| Message ID | 20190607092232.83179-8-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | pci: expand usage of pci_sbdf_t | expand |
>>> On 07.06.19 at 11:22, <roger.pau@citrix.com> wrote: > --- a/xen/arch/x86/cpu/amd.c > +++ b/xen/arch/x86/cpu/amd.c > @@ -420,12 +420,12 @@ static void disable_c1_ramping(void) > nr_nodes = ((pci_conf_read32(0, 0, 0x18, 0x0, 0x60)>>4)&0x07)+1; > for (node = 0; node < nr_nodes; node++) { > /* PMM7: bus=0, dev=0x18+node, function=0x3, register=0x87. */ > - pmm7 = pci_conf_read8(0, 0, 0x18+node, 0x3, 0x87); > + pmm7 = pci_conf_read8(PCI_SBDF(0, 0, 0x18 + node, 3), 0x87); You drop a pointless 0x here, but ... > /* Invalid read means we've updated every Northbridge. */ > if (pmm7 == 0xFF) > break; > pmm7 &= 0xFC; /* clear pmm7[1:0] */ > - pci_conf_write8(0, 0, 0x18+node, 0x3, 0x87, pmm7); > + pci_conf_write8(0, 0, 0x18 + node, 0x3, 0x87, pmm7); ... you leave one in place here - is this intentional? (Of course that's easy enough to adjust while committing.) Anyway, Reviewed-by: Jan Beulich <jbeulich@suse.com> Jan
On Fri, Jun 07, 2019 at 11:22:26AM +0200, Roger Pau Monne wrote: > This reduces the number of parameters of the function to two, and > simplifies some of the calling sites. > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> As far as AMD IOMMU Acked-by: Brian Woods <brian.woods@amd.com> > --- > Cc: Jan Beulich <jbeulich@suse.com> > Cc: Andrew Cooper <andrew.cooper3@citrix.com> > Cc: Wei Liu <wl@xen.org> > Cc: George Dunlap <George.Dunlap@eu.citrix.com> > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > Cc: Julien Grall <julien.grall@arm.com> > Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > Cc: Stefano Stabellini <sstabellini@kernel.org> > Cc: Tim Deegan <tim@xen.org> > Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> > Cc: Brian Woods <brian.woods@amd.com> > Cc: Kevin Tian <kevin.tian@intel.com> > --- > xen/arch/x86/cpu/amd.c | 4 ++-- > xen/arch/x86/msi.c | 2 +- > xen/arch/x86/x86_64/pci.c | 25 ++++++++++++------------ > xen/drivers/char/ehci-dbgp.c | 5 +++-- > xen/drivers/char/ns16550.c | 6 ++++-- > xen/drivers/passthrough/amd/iommu_init.c | 2 +- > xen/drivers/passthrough/pci.c | 21 ++++++++------------ > xen/drivers/passthrough/vtd/dmar.c | 6 +++--- > xen/drivers/passthrough/vtd/quirks.c | 6 +++--- > xen/drivers/pci/pci.c | 9 ++++----- > xen/drivers/video/vga.c | 3 +-- > xen/drivers/vpci/header.c | 3 +-- > xen/drivers/vpci/vpci.c | 8 +++----- > xen/include/xen/pci.h | 4 +--- > 14 files changed, 47 insertions(+), 57 deletions(-) > > diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c > index 8404cf290f..3c069391f4 100644 > --- a/xen/arch/x86/cpu/amd.c > +++ b/xen/arch/x86/cpu/amd.c > @@ -420,12 +420,12 @@ static void disable_c1_ramping(void) > nr_nodes = ((pci_conf_read32(0, 0, 0x18, 0x0, 0x60)>>4)&0x07)+1; > for (node = 0; node < nr_nodes; node++) { > /* PMM7: bus=0, dev=0x18+node, function=0x3, register=0x87. */ > - pmm7 = pci_conf_read8(0, 0, 0x18+node, 0x3, 0x87); > + pmm7 = pci_conf_read8(PCI_SBDF(0, 0, 0x18 + node, 3), 0x87); > /* Invalid read means we've updated every Northbridge. */ > if (pmm7 == 0xFF) > break; > pmm7 &= 0xFC; /* clear pmm7[1:0] */ > - pci_conf_write8(0, 0, 0x18+node, 0x3, 0x87, pmm7); > + pci_conf_write8(0, 0, 0x18 + node, 0x3, 0x87, pmm7); > printk ("AMD: Disabling C1 Clock Ramping Node #%x\n", node); > } > } > diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c > index babc4147c4..67339edc68 100644 > --- a/xen/arch/x86/msi.c > +++ b/xen/arch/x86/msi.c > @@ -800,7 +800,7 @@ static u64 read_pci_mem_bar(u16 seg, u8 bus, u8 slot, u8 func, u8 bir, int vf) > disp = vf * pdev->vf_rlen[bir]; > limit = PCI_SRIOV_NUM_BARS; > } > - else switch ( pci_conf_read8(seg, bus, slot, func, > + else switch ( pci_conf_read8(PCI_SBDF(seg, bus, slot, func), > PCI_HEADER_TYPE) & 0x7f ) > { > case PCI_HEADER_TYPE_NORMAL: > diff --git a/xen/arch/x86/x86_64/pci.c b/xen/arch/x86/x86_64/pci.c > index 6e3f5cf203..b70383fb03 100644 > --- a/xen/arch/x86/x86_64/pci.c > +++ b/xen/arch/x86/x86_64/pci.c > @@ -8,27 +8,26 @@ > #include <xen/pci.h> > #include <asm/io.h> > > -#define PCI_CONF_ADDRESS(bus, dev, func, reg) \ > - (0x80000000 | (bus << 16) | (dev << 11) | (func << 8) | (reg & ~3)) > +#define PCI_CONF_ADDRESS(sbdf, reg) \ > + (0x80000000 | ((sbdf).bdf << 8) | ((reg) & ~3)) > > -uint8_t pci_conf_read8( > - unsigned int seg, unsigned int bus, unsigned int dev, unsigned int func, > - unsigned int reg) > +uint8_t pci_conf_read8(pci_sbdf_t sbdf, unsigned int reg) > { > - u32 value; > + uint32_t value; > > - if ( seg || reg > 255 ) > + if ( sbdf.seg || reg > 255 ) > { > - pci_mmcfg_read(seg, bus, PCI_DEVFN(dev, func), reg, 1, &value); > + pci_mmcfg_read(sbdf.seg, sbdf.bus, sbdf.devfn, reg, 1, &value); > return value; > } > - else > - { > - BUG_ON((bus > 255) || (dev > 31) || (func > 7)); > - return pci_conf_read(PCI_CONF_ADDRESS(bus, dev, func, reg), reg & 3, 1); > - } > + > + return pci_conf_read(PCI_CONF_ADDRESS(sbdf, reg), reg & 3, 1); > } > > +#undef PCI_CONF_ADDRESS > +#define PCI_CONF_ADDRESS(bus, dev, func, reg) \ > + (0x80000000 | (bus << 16) | (dev << 11) | (func << 8) | (reg & ~3)) > + > uint16_t pci_conf_read16( > unsigned int seg, unsigned int bus, unsigned int dev, unsigned int func, > unsigned int reg) > diff --git a/xen/drivers/char/ehci-dbgp.c b/xen/drivers/char/ehci-dbgp.c > index 475dc41767..71f0aaa6ac 100644 > --- a/xen/drivers/char/ehci-dbgp.c > +++ b/xen/drivers/char/ehci-dbgp.c > @@ -713,7 +713,7 @@ static unsigned int __init find_dbgp(struct ehci_dbgp *dbgp, > cap = __find_dbgp(bus, slot, func); > if ( !cap || ehci_num-- ) > { > - if ( !func && !(pci_conf_read8(0, bus, slot, func, > + if ( !func && !(pci_conf_read8(PCI_SBDF(0, bus, slot, func), > PCI_HEADER_TYPE) & 0x80) ) > break; > continue; > @@ -1312,7 +1312,8 @@ static void __init ehci_dbgp_init_preirq(struct serial_port *port) > offset = (debug_port >> 16) & 0xfff; > > /* double check if the mem space is enabled */ > - dbgp->pci_cr = pci_conf_read8(0, dbgp->bus, dbgp->slot, dbgp->func, > + dbgp->pci_cr = pci_conf_read8(PCI_SBDF(0, dbgp->bus, dbgp->slot, > + dbgp->func), > PCI_COMMAND); > if ( !(dbgp->pci_cr & PCI_COMMAND_MEMORY) ) > { > diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c > index 189e121b7e..547270d0e1 100644 > --- a/xen/drivers/char/ns16550.c > +++ b/xen/drivers/char/ns16550.c > @@ -1188,8 +1188,10 @@ pci_uart_config(struct ns16550 *uart, bool_t skip_amt, unsigned int idx) > uart->bar64 = bar_64; > uart->io_size = max(8U << param->reg_shift, > param->uart_offset); > - uart->irq = pci_conf_read8(0, b, d, f, PCI_INTERRUPT_PIN) ? > - pci_conf_read8(0, b, d, f, PCI_INTERRUPT_LINE) : 0; > + uart->irq = pci_conf_read8(PCI_SBDF(0, b, d, f), > + PCI_INTERRUPT_PIN) ? > + pci_conf_read8(PCI_SBDF(0, b, d, f), > + PCI_INTERRUPT_LINE) : 0; > > return 0; > } > diff --git a/xen/drivers/passthrough/amd/iommu_init.c b/xen/drivers/passthrough/amd/iommu_init.c > index 72ea8824b0..30de684f6d 100644 > --- a/xen/drivers/passthrough/amd/iommu_init.c > +++ b/xen/drivers/passthrough/amd/iommu_init.c > @@ -1241,7 +1241,7 @@ static bool_t __init amd_sp5100_erratum28(void) > if (vendor_id != 0x1002 || dev_id != 0x4385) > continue; > > - byte = pci_conf_read8(0, bus, 0x14, 0, 0xad); > + byte = pci_conf_read8(PCI_SBDF(0, bus, 0x14, 0), 0xad); > if ( (byte >> 3) & 1 ) > { > printk(XENLOG_WARNING "AMD-Vi: SP5100 erratum 28 detected, disabling IOMMU.\n" > diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c > index e88689425d..340e957954 100644 > --- a/xen/drivers/passthrough/pci.c > +++ b/xen/drivers/passthrough/pci.c > @@ -260,7 +260,7 @@ static void check_pdev(const struct pci_dev *pdev) > } > } > > - switch ( pci_conf_read8(seg, bus, dev, func, PCI_HEADER_TYPE) & 0x7f ) > + switch ( pci_conf_read8(pdev->sbdf, PCI_HEADER_TYPE) & 0x7f ) > { > case PCI_HEADER_TYPE_BRIDGE: > if ( !bridge_ctl_mask ) > @@ -370,10 +370,8 @@ static struct pci_dev *alloc_pdev(struct pci_seg *pseg, u8 bus, u8 devfn) > > case DEV_TYPE_PCIe2PCI_BRIDGE: > case DEV_TYPE_LEGACY_PCI_BRIDGE: > - sec_bus = pci_conf_read8(pseg->nr, bus, PCI_SLOT(devfn), > - PCI_FUNC(devfn), PCI_SECONDARY_BUS); > - sub_bus = pci_conf_read8(pseg->nr, bus, PCI_SLOT(devfn), > - PCI_FUNC(devfn), PCI_SUBORDINATE_BUS); > + sec_bus = pci_conf_read8(pdev->sbdf, PCI_SECONDARY_BUS); > + sub_bus = pci_conf_read8(pdev->sbdf, PCI_SUBORDINATE_BUS); > > spin_lock(&pseg->bus2bridge_lock); > for ( ; sec_bus <= sub_bus; sec_bus++ ) > @@ -436,16 +434,12 @@ static void free_pdev(struct pci_seg *pseg, struct pci_dev *pdev) > /* update bus2bridge */ > switch ( pdev->type ) > { > - u8 dev, func, sec_bus, sub_bus; > + uint8_t sec_bus, sub_bus; > > case DEV_TYPE_PCIe2PCI_BRIDGE: > case DEV_TYPE_LEGACY_PCI_BRIDGE: > - dev = PCI_SLOT(pdev->devfn); > - func = PCI_FUNC(pdev->devfn); > - sec_bus = pci_conf_read8(pseg->nr, pdev->bus, dev, func, > - PCI_SECONDARY_BUS); > - sub_bus = pci_conf_read8(pseg->nr, pdev->bus, dev, func, > - PCI_SUBORDINATE_BUS); > + sec_bus = pci_conf_read8(pdev->sbdf, PCI_SECONDARY_BUS); > + sub_bus = pci_conf_read8(pdev->sbdf, PCI_SUBORDINATE_BUS); > > spin_lock(&pseg->bus2bridge_lock); > for ( ; sec_bus <= sub_bus; sec_bus++ ) > @@ -1082,7 +1076,8 @@ static int __init _scan_pci_devices(struct pci_seg *pseg, void *arg) > return -ENOMEM; > } > > - if ( !func && !(pci_conf_read8(pseg->nr, bus, dev, func, > + if ( !func && !(pci_conf_read8(PCI_SBDF(pseg->nr, bus, dev, > + func), > PCI_HEADER_TYPE) & 0x80) ) > break; > } > diff --git a/xen/drivers/passthrough/vtd/dmar.c b/xen/drivers/passthrough/vtd/dmar.c > index b858fe7c80..9c94deac0b 100644 > --- a/xen/drivers/passthrough/vtd/dmar.c > +++ b/xen/drivers/passthrough/vtd/dmar.c > @@ -348,7 +348,7 @@ static int __init acpi_parse_dev_scope( > > while ( --depth > 0 ) > { > - bus = pci_conf_read8(seg, bus, path->dev, path->fn, > + bus = pci_conf_read8(PCI_SBDF(seg, bus, path->dev, path->fn), > PCI_SECONDARY_BUS); > path++; > } > @@ -356,9 +356,9 @@ static int __init acpi_parse_dev_scope( > switch ( acpi_scope->entry_type ) > { > case ACPI_DMAR_SCOPE_TYPE_BRIDGE: > - sec_bus = pci_conf_read8(seg, bus, path->dev, path->fn, > + sec_bus = pci_conf_read8(PCI_SBDF(seg, bus, path->dev, path->fn), > PCI_SECONDARY_BUS); > - sub_bus = pci_conf_read8(seg, bus, path->dev, path->fn, > + sub_bus = pci_conf_read8(PCI_SBDF(seg, bus, path->dev, path->fn), > PCI_SUBORDINATE_BUS); > if ( iommu_verbose ) > printk(VTDPREFIX > diff --git a/xen/drivers/passthrough/vtd/quirks.c b/xen/drivers/passthrough/vtd/quirks.c > index d6db862678..ff73b0e7f4 100644 > --- a/xen/drivers/passthrough/vtd/quirks.c > +++ b/xen/drivers/passthrough/vtd/quirks.c > @@ -92,8 +92,8 @@ static void __init cantiga_b3_errata_init(void) > if ( vid != 0x8086 ) > return; > > - did_hi = pci_conf_read8(0, 0, IGD_DEV, 0, 3); > - rid = pci_conf_read8(0, 0, IGD_DEV, 0, 8); > + did_hi = pci_conf_read8(PCI_SBDF(0, 0, IGD_DEV, 0), 3); > + rid = pci_conf_read8(PCI_SBDF(0, 0, IGD_DEV, 0), 8); > > if ( (did_hi == 0x2A) && (rid == 0x7) ) > is_cantiga_b3 = 1; > @@ -281,7 +281,7 @@ static void __init tylersburg_intremap_quirk(void) > { > /* Match on System Management Registers on Device 20 Function 0 */ > device = pci_conf_read32(0, bus, 20, 0, PCI_VENDOR_ID); > - rev = pci_conf_read8(0, bus, 20, 0, PCI_REVISION_ID); > + rev = pci_conf_read8(PCI_SBDF(0, bus, 20, 0), PCI_REVISION_ID); > > if ( rev == 0x13 && device == 0x342e8086 ) > { > diff --git a/xen/drivers/pci/pci.c b/xen/drivers/pci/pci.c > index 1c808d6632..e3f883fc5c 100644 > --- a/xen/drivers/pci/pci.c > +++ b/xen/drivers/pci/pci.c > @@ -21,12 +21,12 @@ int pci_find_cap_offset(u16 seg, u8 bus, u8 dev, u8 func, u8 cap) > > while ( max_cap-- ) > { > - pos = pci_conf_read8(seg, bus, dev, func, pos); > + pos = pci_conf_read8(PCI_SBDF(seg, bus, dev, func), pos); > if ( pos < 0x40 ) > break; > > pos &= ~3; > - id = pci_conf_read8(seg, bus, dev, func, pos + PCI_CAP_LIST_ID); > + id = pci_conf_read8(PCI_SBDF(seg, bus, dev, func), pos + PCI_CAP_LIST_ID); > > if ( id == 0xff ) > break; > @@ -46,13 +46,12 @@ int pci_find_next_cap(u16 seg, u8 bus, unsigned int devfn, u8 pos, int cap) > > while ( ttl-- ) > { > - pos = pci_conf_read8(seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), pos); > + pos = pci_conf_read8(PCI_SBDF3(seg, bus, devfn), pos); > if ( pos < 0x40 ) > break; > > pos &= ~3; > - id = pci_conf_read8(seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), > - pos + PCI_CAP_LIST_ID); > + id = pci_conf_read8(PCI_SBDF3(seg, bus, devfn), pos + PCI_CAP_LIST_ID); > > if ( id == 0xff ) > break; > diff --git a/xen/drivers/video/vga.c b/xen/drivers/video/vga.c > index 6a64fd9013..78533ad0b1 100644 > --- a/xen/drivers/video/vga.c > +++ b/xen/drivers/video/vga.c > @@ -136,8 +136,7 @@ void __init video_endboot(void) > b = 0; > break; > case 1: > - switch ( pci_conf_read8(0, b, PCI_SLOT(df), > - PCI_FUNC(df), > + switch ( pci_conf_read8(PCI_SBDF3(0, b, df), > PCI_HEADER_TYPE) ) > { > case PCI_HEADER_TYPE_BRIDGE: > diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c > index 258b91deed..564c7b6a7d 100644 > --- a/xen/drivers/vpci/header.c > +++ b/xen/drivers/vpci/header.c > @@ -463,8 +463,7 @@ static int init_bars(struct pci_dev *pdev) > struct vpci_bar *bars = header->bars; > int rc; > > - switch ( pci_conf_read8(pdev->seg, pdev->bus, slot, func, PCI_HEADER_TYPE) > - & 0x7f ) > + switch ( pci_conf_read8(pdev->sbdf, PCI_HEADER_TYPE) & 0x7f ) > { > case PCI_HEADER_TYPE_NORMAL: > num_bars = PCI_HEADER_NORMAL_NR_BARS; > diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c > index ca598675ea..c4030333a5 100644 > --- a/xen/drivers/vpci/vpci.c > +++ b/xen/drivers/vpci/vpci.c > @@ -222,8 +222,7 @@ static uint32_t vpci_read_hw(pci_sbdf_t sbdf, unsigned int reg, > */ > if ( reg & 1 ) > { > - data = pci_conf_read8(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn, > - reg); > + data = pci_conf_read8(sbdf, reg); > data |= pci_conf_read16(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn, > reg + 1) << 8; > } > @@ -231,8 +230,7 @@ static uint32_t vpci_read_hw(pci_sbdf_t sbdf, unsigned int reg, > { > data = pci_conf_read16(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn, > reg); > - data |= pci_conf_read8(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn, > - reg + 2) << 16; > + data |= pci_conf_read8(sbdf, reg + 2) << 16; > } > break; > > @@ -241,7 +239,7 @@ static uint32_t vpci_read_hw(pci_sbdf_t sbdf, unsigned int reg, > break; > > case 1: > - data = pci_conf_read8(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn, reg); > + data = pci_conf_read8(sbdf, reg); > break; > > default: > diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h > index 05ee115715..b2a62cb366 100644 > --- a/xen/include/xen/pci.h > +++ b/xen/include/xen/pci.h > @@ -172,9 +172,7 @@ struct pci_dev *pci_get_pdev_by_domain(const struct domain *, int seg, > int bus, int devfn); > void pci_check_disable_device(u16 seg, u8 bus, u8 devfn); > > -uint8_t pci_conf_read8( > - unsigned int seg, unsigned int bus, unsigned int dev, unsigned int func, > - unsigned int reg); > +uint8_t pci_conf_read8(pci_sbdf_t sbdf, unsigned int reg); > uint16_t pci_conf_read16( > unsigned int seg, unsigned int bus, unsigned int dev, unsigned int func, > unsigned int reg); > -- > 2.20.1 (Apple Git-117) >
> From: Roger Pau Monne [mailto:roger.pau@citrix.com] > Sent: Friday, June 7, 2019 5:22 PM > > This reduces the number of parameters of the function to two, and > simplifies some of the calling sites. > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c index 8404cf290f..3c069391f4 100644 --- a/xen/arch/x86/cpu/amd.c +++ b/xen/arch/x86/cpu/amd.c @@ -420,12 +420,12 @@ static void disable_c1_ramping(void) nr_nodes = ((pci_conf_read32(0, 0, 0x18, 0x0, 0x60)>>4)&0x07)+1; for (node = 0; node < nr_nodes; node++) { /* PMM7: bus=0, dev=0x18+node, function=0x3, register=0x87. */ - pmm7 = pci_conf_read8(0, 0, 0x18+node, 0x3, 0x87); + pmm7 = pci_conf_read8(PCI_SBDF(0, 0, 0x18 + node, 3), 0x87); /* Invalid read means we've updated every Northbridge. */ if (pmm7 == 0xFF) break; pmm7 &= 0xFC; /* clear pmm7[1:0] */ - pci_conf_write8(0, 0, 0x18+node, 0x3, 0x87, pmm7); + pci_conf_write8(0, 0, 0x18 + node, 0x3, 0x87, pmm7); printk ("AMD: Disabling C1 Clock Ramping Node #%x\n", node); } } diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c index babc4147c4..67339edc68 100644 --- a/xen/arch/x86/msi.c +++ b/xen/arch/x86/msi.c @@ -800,7 +800,7 @@ static u64 read_pci_mem_bar(u16 seg, u8 bus, u8 slot, u8 func, u8 bir, int vf) disp = vf * pdev->vf_rlen[bir]; limit = PCI_SRIOV_NUM_BARS; } - else switch ( pci_conf_read8(seg, bus, slot, func, + else switch ( pci_conf_read8(PCI_SBDF(seg, bus, slot, func), PCI_HEADER_TYPE) & 0x7f ) { case PCI_HEADER_TYPE_NORMAL: diff --git a/xen/arch/x86/x86_64/pci.c b/xen/arch/x86/x86_64/pci.c index 6e3f5cf203..b70383fb03 100644 --- a/xen/arch/x86/x86_64/pci.c +++ b/xen/arch/x86/x86_64/pci.c @@ -8,27 +8,26 @@ #include <xen/pci.h> #include <asm/io.h> -#define PCI_CONF_ADDRESS(bus, dev, func, reg) \ - (0x80000000 | (bus << 16) | (dev << 11) | (func << 8) | (reg & ~3)) +#define PCI_CONF_ADDRESS(sbdf, reg) \ + (0x80000000 | ((sbdf).bdf << 8) | ((reg) & ~3)) -uint8_t pci_conf_read8( - unsigned int seg, unsigned int bus, unsigned int dev, unsigned int func, - unsigned int reg) +uint8_t pci_conf_read8(pci_sbdf_t sbdf, unsigned int reg) { - u32 value; + uint32_t value; - if ( seg || reg > 255 ) + if ( sbdf.seg || reg > 255 ) { - pci_mmcfg_read(seg, bus, PCI_DEVFN(dev, func), reg, 1, &value); + pci_mmcfg_read(sbdf.seg, sbdf.bus, sbdf.devfn, reg, 1, &value); return value; } - else - { - BUG_ON((bus > 255) || (dev > 31) || (func > 7)); - return pci_conf_read(PCI_CONF_ADDRESS(bus, dev, func, reg), reg & 3, 1); - } + + return pci_conf_read(PCI_CONF_ADDRESS(sbdf, reg), reg & 3, 1); } +#undef PCI_CONF_ADDRESS +#define PCI_CONF_ADDRESS(bus, dev, func, reg) \ + (0x80000000 | (bus << 16) | (dev << 11) | (func << 8) | (reg & ~3)) + uint16_t pci_conf_read16( unsigned int seg, unsigned int bus, unsigned int dev, unsigned int func, unsigned int reg) diff --git a/xen/drivers/char/ehci-dbgp.c b/xen/drivers/char/ehci-dbgp.c index 475dc41767..71f0aaa6ac 100644 --- a/xen/drivers/char/ehci-dbgp.c +++ b/xen/drivers/char/ehci-dbgp.c @@ -713,7 +713,7 @@ static unsigned int __init find_dbgp(struct ehci_dbgp *dbgp, cap = __find_dbgp(bus, slot, func); if ( !cap || ehci_num-- ) { - if ( !func && !(pci_conf_read8(0, bus, slot, func, + if ( !func && !(pci_conf_read8(PCI_SBDF(0, bus, slot, func), PCI_HEADER_TYPE) & 0x80) ) break; continue; @@ -1312,7 +1312,8 @@ static void __init ehci_dbgp_init_preirq(struct serial_port *port) offset = (debug_port >> 16) & 0xfff; /* double check if the mem space is enabled */ - dbgp->pci_cr = pci_conf_read8(0, dbgp->bus, dbgp->slot, dbgp->func, + dbgp->pci_cr = pci_conf_read8(PCI_SBDF(0, dbgp->bus, dbgp->slot, + dbgp->func), PCI_COMMAND); if ( !(dbgp->pci_cr & PCI_COMMAND_MEMORY) ) { diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c index 189e121b7e..547270d0e1 100644 --- a/xen/drivers/char/ns16550.c +++ b/xen/drivers/char/ns16550.c @@ -1188,8 +1188,10 @@ pci_uart_config(struct ns16550 *uart, bool_t skip_amt, unsigned int idx) uart->bar64 = bar_64; uart->io_size = max(8U << param->reg_shift, param->uart_offset); - uart->irq = pci_conf_read8(0, b, d, f, PCI_INTERRUPT_PIN) ? - pci_conf_read8(0, b, d, f, PCI_INTERRUPT_LINE) : 0; + uart->irq = pci_conf_read8(PCI_SBDF(0, b, d, f), + PCI_INTERRUPT_PIN) ? + pci_conf_read8(PCI_SBDF(0, b, d, f), + PCI_INTERRUPT_LINE) : 0; return 0; } diff --git a/xen/drivers/passthrough/amd/iommu_init.c b/xen/drivers/passthrough/amd/iommu_init.c index 72ea8824b0..30de684f6d 100644 --- a/xen/drivers/passthrough/amd/iommu_init.c +++ b/xen/drivers/passthrough/amd/iommu_init.c @@ -1241,7 +1241,7 @@ static bool_t __init amd_sp5100_erratum28(void) if (vendor_id != 0x1002 || dev_id != 0x4385) continue; - byte = pci_conf_read8(0, bus, 0x14, 0, 0xad); + byte = pci_conf_read8(PCI_SBDF(0, bus, 0x14, 0), 0xad); if ( (byte >> 3) & 1 ) { printk(XENLOG_WARNING "AMD-Vi: SP5100 erratum 28 detected, disabling IOMMU.\n" diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c index e88689425d..340e957954 100644 --- a/xen/drivers/passthrough/pci.c +++ b/xen/drivers/passthrough/pci.c @@ -260,7 +260,7 @@ static void check_pdev(const struct pci_dev *pdev) } } - switch ( pci_conf_read8(seg, bus, dev, func, PCI_HEADER_TYPE) & 0x7f ) + switch ( pci_conf_read8(pdev->sbdf, PCI_HEADER_TYPE) & 0x7f ) { case PCI_HEADER_TYPE_BRIDGE: if ( !bridge_ctl_mask ) @@ -370,10 +370,8 @@ static struct pci_dev *alloc_pdev(struct pci_seg *pseg, u8 bus, u8 devfn) case DEV_TYPE_PCIe2PCI_BRIDGE: case DEV_TYPE_LEGACY_PCI_BRIDGE: - sec_bus = pci_conf_read8(pseg->nr, bus, PCI_SLOT(devfn), - PCI_FUNC(devfn), PCI_SECONDARY_BUS); - sub_bus = pci_conf_read8(pseg->nr, bus, PCI_SLOT(devfn), - PCI_FUNC(devfn), PCI_SUBORDINATE_BUS); + sec_bus = pci_conf_read8(pdev->sbdf, PCI_SECONDARY_BUS); + sub_bus = pci_conf_read8(pdev->sbdf, PCI_SUBORDINATE_BUS); spin_lock(&pseg->bus2bridge_lock); for ( ; sec_bus <= sub_bus; sec_bus++ ) @@ -436,16 +434,12 @@ static void free_pdev(struct pci_seg *pseg, struct pci_dev *pdev) /* update bus2bridge */ switch ( pdev->type ) { - u8 dev, func, sec_bus, sub_bus; + uint8_t sec_bus, sub_bus; case DEV_TYPE_PCIe2PCI_BRIDGE: case DEV_TYPE_LEGACY_PCI_BRIDGE: - dev = PCI_SLOT(pdev->devfn); - func = PCI_FUNC(pdev->devfn); - sec_bus = pci_conf_read8(pseg->nr, pdev->bus, dev, func, - PCI_SECONDARY_BUS); - sub_bus = pci_conf_read8(pseg->nr, pdev->bus, dev, func, - PCI_SUBORDINATE_BUS); + sec_bus = pci_conf_read8(pdev->sbdf, PCI_SECONDARY_BUS); + sub_bus = pci_conf_read8(pdev->sbdf, PCI_SUBORDINATE_BUS); spin_lock(&pseg->bus2bridge_lock); for ( ; sec_bus <= sub_bus; sec_bus++ ) @@ -1082,7 +1076,8 @@ static int __init _scan_pci_devices(struct pci_seg *pseg, void *arg) return -ENOMEM; } - if ( !func && !(pci_conf_read8(pseg->nr, bus, dev, func, + if ( !func && !(pci_conf_read8(PCI_SBDF(pseg->nr, bus, dev, + func), PCI_HEADER_TYPE) & 0x80) ) break; } diff --git a/xen/drivers/passthrough/vtd/dmar.c b/xen/drivers/passthrough/vtd/dmar.c index b858fe7c80..9c94deac0b 100644 --- a/xen/drivers/passthrough/vtd/dmar.c +++ b/xen/drivers/passthrough/vtd/dmar.c @@ -348,7 +348,7 @@ static int __init acpi_parse_dev_scope( while ( --depth > 0 ) { - bus = pci_conf_read8(seg, bus, path->dev, path->fn, + bus = pci_conf_read8(PCI_SBDF(seg, bus, path->dev, path->fn), PCI_SECONDARY_BUS); path++; } @@ -356,9 +356,9 @@ static int __init acpi_parse_dev_scope( switch ( acpi_scope->entry_type ) { case ACPI_DMAR_SCOPE_TYPE_BRIDGE: - sec_bus = pci_conf_read8(seg, bus, path->dev, path->fn, + sec_bus = pci_conf_read8(PCI_SBDF(seg, bus, path->dev, path->fn), PCI_SECONDARY_BUS); - sub_bus = pci_conf_read8(seg, bus, path->dev, path->fn, + sub_bus = pci_conf_read8(PCI_SBDF(seg, bus, path->dev, path->fn), PCI_SUBORDINATE_BUS); if ( iommu_verbose ) printk(VTDPREFIX diff --git a/xen/drivers/passthrough/vtd/quirks.c b/xen/drivers/passthrough/vtd/quirks.c index d6db862678..ff73b0e7f4 100644 --- a/xen/drivers/passthrough/vtd/quirks.c +++ b/xen/drivers/passthrough/vtd/quirks.c @@ -92,8 +92,8 @@ static void __init cantiga_b3_errata_init(void) if ( vid != 0x8086 ) return; - did_hi = pci_conf_read8(0, 0, IGD_DEV, 0, 3); - rid = pci_conf_read8(0, 0, IGD_DEV, 0, 8); + did_hi = pci_conf_read8(PCI_SBDF(0, 0, IGD_DEV, 0), 3); + rid = pci_conf_read8(PCI_SBDF(0, 0, IGD_DEV, 0), 8); if ( (did_hi == 0x2A) && (rid == 0x7) ) is_cantiga_b3 = 1; @@ -281,7 +281,7 @@ static void __init tylersburg_intremap_quirk(void) { /* Match on System Management Registers on Device 20 Function 0 */ device = pci_conf_read32(0, bus, 20, 0, PCI_VENDOR_ID); - rev = pci_conf_read8(0, bus, 20, 0, PCI_REVISION_ID); + rev = pci_conf_read8(PCI_SBDF(0, bus, 20, 0), PCI_REVISION_ID); if ( rev == 0x13 && device == 0x342e8086 ) { diff --git a/xen/drivers/pci/pci.c b/xen/drivers/pci/pci.c index 1c808d6632..e3f883fc5c 100644 --- a/xen/drivers/pci/pci.c +++ b/xen/drivers/pci/pci.c @@ -21,12 +21,12 @@ int pci_find_cap_offset(u16 seg, u8 bus, u8 dev, u8 func, u8 cap) while ( max_cap-- ) { - pos = pci_conf_read8(seg, bus, dev, func, pos); + pos = pci_conf_read8(PCI_SBDF(seg, bus, dev, func), pos); if ( pos < 0x40 ) break; pos &= ~3; - id = pci_conf_read8(seg, bus, dev, func, pos + PCI_CAP_LIST_ID); + id = pci_conf_read8(PCI_SBDF(seg, bus, dev, func), pos + PCI_CAP_LIST_ID); if ( id == 0xff ) break; @@ -46,13 +46,12 @@ int pci_find_next_cap(u16 seg, u8 bus, unsigned int devfn, u8 pos, int cap) while ( ttl-- ) { - pos = pci_conf_read8(seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), pos); + pos = pci_conf_read8(PCI_SBDF3(seg, bus, devfn), pos); if ( pos < 0x40 ) break; pos &= ~3; - id = pci_conf_read8(seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), - pos + PCI_CAP_LIST_ID); + id = pci_conf_read8(PCI_SBDF3(seg, bus, devfn), pos + PCI_CAP_LIST_ID); if ( id == 0xff ) break; diff --git a/xen/drivers/video/vga.c b/xen/drivers/video/vga.c index 6a64fd9013..78533ad0b1 100644 --- a/xen/drivers/video/vga.c +++ b/xen/drivers/video/vga.c @@ -136,8 +136,7 @@ void __init video_endboot(void) b = 0; break; case 1: - switch ( pci_conf_read8(0, b, PCI_SLOT(df), - PCI_FUNC(df), + switch ( pci_conf_read8(PCI_SBDF3(0, b, df), PCI_HEADER_TYPE) ) { case PCI_HEADER_TYPE_BRIDGE: diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c index 258b91deed..564c7b6a7d 100644 --- a/xen/drivers/vpci/header.c +++ b/xen/drivers/vpci/header.c @@ -463,8 +463,7 @@ static int init_bars(struct pci_dev *pdev) struct vpci_bar *bars = header->bars; int rc; - switch ( pci_conf_read8(pdev->seg, pdev->bus, slot, func, PCI_HEADER_TYPE) - & 0x7f ) + switch ( pci_conf_read8(pdev->sbdf, PCI_HEADER_TYPE) & 0x7f ) { case PCI_HEADER_TYPE_NORMAL: num_bars = PCI_HEADER_NORMAL_NR_BARS; diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c index ca598675ea..c4030333a5 100644 --- a/xen/drivers/vpci/vpci.c +++ b/xen/drivers/vpci/vpci.c @@ -222,8 +222,7 @@ static uint32_t vpci_read_hw(pci_sbdf_t sbdf, unsigned int reg, */ if ( reg & 1 ) { - data = pci_conf_read8(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn, - reg); + data = pci_conf_read8(sbdf, reg); data |= pci_conf_read16(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn, reg + 1) << 8; } @@ -231,8 +230,7 @@ static uint32_t vpci_read_hw(pci_sbdf_t sbdf, unsigned int reg, { data = pci_conf_read16(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn, reg); - data |= pci_conf_read8(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn, - reg + 2) << 16; + data |= pci_conf_read8(sbdf, reg + 2) << 16; } break; @@ -241,7 +239,7 @@ static uint32_t vpci_read_hw(pci_sbdf_t sbdf, unsigned int reg, break; case 1: - data = pci_conf_read8(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn, reg); + data = pci_conf_read8(sbdf, reg); break; default: diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h index 05ee115715..b2a62cb366 100644 --- a/xen/include/xen/pci.h +++ b/xen/include/xen/pci.h @@ -172,9 +172,7 @@ struct pci_dev *pci_get_pdev_by_domain(const struct domain *, int seg, int bus, int devfn); void pci_check_disable_device(u16 seg, u8 bus, u8 devfn); -uint8_t pci_conf_read8( - unsigned int seg, unsigned int bus, unsigned int dev, unsigned int func, - unsigned int reg); +uint8_t pci_conf_read8(pci_sbdf_t sbdf, unsigned int reg); uint16_t pci_conf_read16( unsigned int seg, unsigned int bus, unsigned int dev, unsigned int func, unsigned int reg);
This reduces the number of parameters of the function to two, and simplifies some of the calling sites. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- Cc: Jan Beulich <jbeulich@suse.com> Cc: Andrew Cooper <andrew.cooper3@citrix.com> Cc: Wei Liu <wl@xen.org> Cc: George Dunlap <George.Dunlap@eu.citrix.com> Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Julien Grall <julien.grall@arm.com> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Cc: Stefano Stabellini <sstabellini@kernel.org> Cc: Tim Deegan <tim@xen.org> Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> Cc: Brian Woods <brian.woods@amd.com> Cc: Kevin Tian <kevin.tian@intel.com> --- xen/arch/x86/cpu/amd.c | 4 ++-- xen/arch/x86/msi.c | 2 +- xen/arch/x86/x86_64/pci.c | 25 ++++++++++++------------ xen/drivers/char/ehci-dbgp.c | 5 +++-- xen/drivers/char/ns16550.c | 6 ++++-- xen/drivers/passthrough/amd/iommu_init.c | 2 +- xen/drivers/passthrough/pci.c | 21 ++++++++------------ xen/drivers/passthrough/vtd/dmar.c | 6 +++--- xen/drivers/passthrough/vtd/quirks.c | 6 +++--- xen/drivers/pci/pci.c | 9 ++++----- xen/drivers/video/vga.c | 3 +-- xen/drivers/vpci/header.c | 3 +-- xen/drivers/vpci/vpci.c | 8 +++----- xen/include/xen/pci.h | 4 +--- 14 files changed, 47 insertions(+), 57 deletions(-)