Re: [Xen-devel] [PATCH v6 01/11] pci: introduce a type to store a SBDF

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Roger Pau Monne [mailto:roger.pau@xxxxxxxxxx]
> Sent: 19 September 2017 16:29
> To: xen-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: konrad.wilk@xxxxxxxxxx; boris.ostrovsky@xxxxxxxxxx; Roger Pau Monne
> <roger.pau@xxxxxxxxxx>; Paul Durrant <Paul.Durrant@xxxxxxxxxx>; Jan
> Beulich <jbeulich@xxxxxxxx>; Andrew Cooper
> <Andrew.Cooper3@xxxxxxxxxx>; George Dunlap
> <George.Dunlap@xxxxxxxxxx>; Ian Jackson <Ian.Jackson@xxxxxxxxxx>;
> Stefano Stabellini <sstabellini@xxxxxxxxxx>; Tim (Xen.org) <tim@xxxxxxx>;
> Wei Liu <wei.liu2@xxxxxxxxxx>
> Subject: [PATCH v6 01/11] pci: introduce a type to store a SBDF
> 
> That provides direct access to all the members that constitute a SBDF.
> The only function switched to use it is hvm_pci_decode_addr, because
> it makes following patches simpler.
> 
> Suggested-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> ---
> Cc: Paul Durrant <paul.durrant@xxxxxxxxxx>
> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Cc: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> Cc: Tim Deegan <tim@xxxxxxx>
> Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
> ---
> Changes since v5:
>  - New in this version.
> ---
>  xen/arch/x86/hvm/io.c        | 10 ++--------
>  xen/arch/x86/hvm/ioreq.c     | 12 +++++-------
>  xen/include/asm-x86/hvm/io.h |  4 ++--
>  xen/include/xen/pci.h        | 20 ++++++++++++++++++++
>  4 files changed, 29 insertions(+), 17 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c
> index bf41954f59..4e49e59012 100644
> --- a/xen/arch/x86/hvm/io.c
> +++ b/xen/arch/x86/hvm/io.c
> @@ -257,17 +257,11 @@ void register_g2m_portio_handler(struct domain
> *d)
>  }
> 
>  unsigned int hvm_pci_decode_addr(unsigned int cf8, unsigned int addr,
> -                                 unsigned int *bus, unsigned int *slot,
> -                                 unsigned int *func)
> +                                 pci_sbdf_t *bdf)

I'd prefer the pointer name to be 'sbdf' rather than 'bdf', but otherwise...

Reviewed-by: Paul Durrant <paul.durrant@xxxxxxxxxx>

>  {
> -    unsigned int bdf;
> -
>      ASSERT(CF8_ENABLED(cf8));
> 
> -    bdf = CF8_BDF(cf8);
> -    *bus = PCI_BUS(bdf);
> -    *slot = PCI_SLOT(bdf);
> -    *func = PCI_FUNC(bdf);
> +    bdf->sbdf = CF8_BDF(cf8);
>      /*
>       * NB: the lower 2 bits of the register address are fetched from the
>       * offset into the 0xcfc register when reading/writing to it.
> diff --git a/xen/arch/x86/hvm/ioreq.c b/xen/arch/x86/hvm/ioreq.c
> index 752976d16d..3e7a88e053 100644
> --- a/xen/arch/x86/hvm/ioreq.c
> +++ b/xen/arch/x86/hvm/ioreq.c
> @@ -1177,17 +1177,15 @@ struct hvm_ioreq_server
> *hvm_select_ioreq_server(struct domain *d,
>           (p->addr & ~3) == 0xcfc &&
>           CF8_ENABLED(cf8) )
>      {
> -        uint32_t sbdf, x86_fam;
> -        unsigned int bus, slot, func, reg;
> +        uint32_t x86_fam;
> +        pci_sbdf_t bdf;
> +        unsigned int reg;
> 
> -        reg = hvm_pci_decode_addr(cf8, p->addr, &bus, &slot, &func);
> +        reg = hvm_pci_decode_addr(cf8, p->addr, &bdf);
> 
>          /* PCI config data cycle */
> -
> -        sbdf = XEN_DMOP_PCI_SBDF(0, bus, slot, func);
> -
>          type = XEN_DMOP_IO_RANGE_PCI;
> -        addr = ((uint64_t)sbdf << 32) | reg;
> +        addr = ((uint64_t)bdf.bdf << 32) | reg;
>          /* AMD extended configuration space access? */
>          if ( CF8_ADDR_HI(cf8) &&
>               d->arch.cpuid->x86_vendor == X86_VENDOR_AMD &&
> diff --git a/xen/include/asm-x86/hvm/io.h b/xen/include/asm-x86/hvm/io.h
> index 51659b6c7f..2ff1c96883 100644
> --- a/xen/include/asm-x86/hvm/io.h
> +++ b/xen/include/asm-x86/hvm/io.h
> @@ -20,6 +20,7 @@
>  #define __ASM_X86_HVM_IO_H__
> 
>  #include <xen/mm.h>
> +#include <xen/pci.h>
>  #include <asm/hvm/vpic.h>
>  #include <asm/hvm/vioapic.h>
>  #include <public/hvm/ioreq.h>
> @@ -151,8 +152,7 @@ extern void hvm_dpci_msi_eoi(struct domain *d, int
> vector);
> 
>  /* Decode a PCI port IO access into a bus/slot/func/reg. */
>  unsigned int hvm_pci_decode_addr(unsigned int cf8, unsigned int addr,
> -                                 unsigned int *bus, unsigned int *slot,
> -                                 unsigned int *func);
> +                                 pci_sbdf_t *bdf);
> 
>  /*
>   * HVM port IO handler that performs forwarding of guest IO ports into
> machine
> diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
> index 43f21251a5..dd5ec43a70 100644
> --- a/xen/include/xen/pci.h
> +++ b/xen/include/xen/pci.h
> @@ -38,6 +38,26 @@
>  #define PCI_SBDF2(s,bdf) ((((s) & 0xffff) << 16) | ((bdf) & 0xffff))
>  #define PCI_SBDF3(s,b,df) ((((s) & 0xffff) << 16) | PCI_BDF2(b, df))
> 
> +typedef union {
> +    uint32_t sbdf;
> +    struct {
> +        union {
> +            uint16_t bdf;
> +            struct {
> +                union {
> +                    struct {
> +                        uint8_t func : 3,
> +                                dev  : 5;
> +                    };
> +                    uint8_t     extfunc;
> +                };
> +                uint8_t         bus;
> +            };
> +        };
> +        uint16_t                seg;
> +    };
> +} pci_sbdf_t;
> +
>  struct pci_dev_info {
>      /*
>       * VF's 'is_extfn' field is used to indicate whether its PF is an 
> extended
> --
> 2.11.0 (Apple Git-81)

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel