[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v3 07/13] pci: switch pci_conf_read8 to use pci_sbdf_t



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@xxxxxxxxxx>

As far as AMD IOMMU

Acked-by: Brian Woods <brian.woods@xxxxxxx>

> ---
> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Cc: Wei Liu <wl@xxxxxxx>
> Cc: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> Cc: Julien Grall <julien.grall@xxxxxxx>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> Cc: Tim Deegan <tim@xxxxxxx>
> Cc: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>
> Cc: Brian Woods <brian.woods@xxxxxxx>
> Cc: Kevin Tian <kevin.tian@xxxxxxxxx>
> ---
>  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)
> 

-- 
Brian Woods

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.