| Message ID | 20170814142850.39133-2-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> -----Original Message----- > From: Roger Pau Monne [mailto:roger.pau@citrix.com] > Sent: 14 August 2017 15:29 > To: xen-devel@lists.xenproject.org > Cc: boris.ostrovsky@oracle.com; konrad.wilk@oracle.com; Roger Pau Monne > <roger.pau@citrix.com>; Paul Durrant <Paul.Durrant@citrix.com>; Jan > Beulich <jbeulich@suse.com>; Andrew Cooper > <Andrew.Cooper3@citrix.com> > Subject: [PATCH v5 01/11] x86/pci: introduce hvm_pci_decode_addr > > And use it in the ioreq code to decode accesses to the PCI IO ports > into bus, slot, function and register values. > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Paul Durrant <paul.durrant@citrix.com> > --- > Cc: Paul Durrant <paul.durrant@citrix.com> > Cc: Jan Beulich <jbeulich@suse.com> > Cc: Andrew Cooper <andrew.cooper3@citrix.com> > --- > Changes since v4: > - New in this version. > --- > xen/arch/x86/hvm/io.c | 19 +++++++++++++++++++ > xen/arch/x86/hvm/ioreq.c | 12 +++++------- > xen/include/asm-x86/hvm/io.h | 5 +++++ > 3 files changed, 29 insertions(+), 7 deletions(-) > > diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c > index 214ab307c4..074cba89da 100644 > --- a/xen/arch/x86/hvm/io.c > +++ b/xen/arch/x86/hvm/io.c > @@ -256,6 +256,25 @@ void register_g2m_portio_handler(struct domain > *d) > handler->ops = &g2m_portio_ops; > } > > +unsigned int hvm_pci_decode_addr(unsigned int cf8, unsigned int addr, > + unsigned int *bus, unsigned int *slot, > + unsigned int *func) > +{ > + unsigned long bdf; > + > + ASSERT(CF8_ENABLED(cf8)); > + > + bdf = CF8_BDF(cf8); > + *bus = PCI_BUS(bdf); > + *slot = PCI_SLOT(bdf); > + *func = PCI_FUNC(bdf); > + /* > + * NB: the lower 2 bits of the register address are fetched from the > + * offset into the 0xcfc register when reading/writing to it. > + */ > + return CF8_ADDR_LO(cf8) | (addr & 3); > +} > + > /* > * Local variables: > * mode: C > diff --git a/xen/arch/x86/hvm/ioreq.c b/xen/arch/x86/hvm/ioreq.c > index b2a8b0e986..752976d16d 100644 > --- a/xen/arch/x86/hvm/ioreq.c > +++ b/xen/arch/x86/hvm/ioreq.c > @@ -1178,18 +1178,16 @@ struct hvm_ioreq_server > *hvm_select_ioreq_server(struct domain *d, > CF8_ENABLED(cf8) ) > { > uint32_t sbdf, x86_fam; > + unsigned int bus, slot, func, reg; > + > + reg = hvm_pci_decode_addr(cf8, p->addr, &bus, &slot, &func); > > /* PCI config data cycle */ > > - sbdf = XEN_DMOP_PCI_SBDF(0, > - PCI_BUS(CF8_BDF(cf8)), > - PCI_SLOT(CF8_BDF(cf8)), > - PCI_FUNC(CF8_BDF(cf8))); > + sbdf = XEN_DMOP_PCI_SBDF(0, bus, slot, func); > > type = XEN_DMOP_IO_RANGE_PCI; > - addr = ((uint64_t)sbdf << 32) | > - CF8_ADDR_LO(cf8) | > - (p->addr & 3); > + addr = ((uint64_t)sbdf << 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 2484eb1c75..51659b6c7f 100644 > --- a/xen/include/asm-x86/hvm/io.h > +++ b/xen/include/asm-x86/hvm/io.h > @@ -149,6 +149,11 @@ void stdvga_deinit(struct domain *d); > > 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); > + > /* > * HVM port IO handler that performs forwarding of guest IO ports into > machine > * IO ports. > -- > 2.11.0 (Apple Git-81)
>>> On 14.08.17 at 16:28, <roger.pau@citrix.com> wrote: > --- a/xen/arch/x86/hvm/io.c > +++ b/xen/arch/x86/hvm/io.c > @@ -256,6 +256,25 @@ void register_g2m_portio_handler(struct domain *d) > handler->ops = &g2m_portio_ops; > } > > +unsigned int hvm_pci_decode_addr(unsigned int cf8, unsigned int addr, > + unsigned int *bus, unsigned int *slot, > + unsigned int *func) > +{ > + unsigned long bdf; Is there a need for this being unsigned long instead of unsigned int? If not, I'd be fine changing this while committing. Jan
On Thu, Aug 24, 2017 at 09:46:16AM -0600, Jan Beulich wrote: > >>> On 14.08.17 at 16:28, <roger.pau@citrix.com> wrote: > > --- a/xen/arch/x86/hvm/io.c > > +++ b/xen/arch/x86/hvm/io.c > > @@ -256,6 +256,25 @@ void register_g2m_portio_handler(struct domain *d) > > handler->ops = &g2m_portio_ops; > > } > > > > +unsigned int hvm_pci_decode_addr(unsigned int cf8, unsigned int addr, > > + unsigned int *bus, unsigned int *slot, > > + unsigned int *func) > > +{ > > + unsigned long bdf; > > Is there a need for this being unsigned long instead of unsigned int? > If not, I'd be fine changing this while committing. It's my mistake it certainly needs to be unsigned int. I've wrongly fixed this in patch 2. Thanks, Roger.
diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c index 214ab307c4..074cba89da 100644 --- a/xen/arch/x86/hvm/io.c +++ b/xen/arch/x86/hvm/io.c @@ -256,6 +256,25 @@ void register_g2m_portio_handler(struct domain *d) handler->ops = &g2m_portio_ops; } +unsigned int hvm_pci_decode_addr(unsigned int cf8, unsigned int addr, + unsigned int *bus, unsigned int *slot, + unsigned int *func) +{ + unsigned long bdf; + + ASSERT(CF8_ENABLED(cf8)); + + bdf = CF8_BDF(cf8); + *bus = PCI_BUS(bdf); + *slot = PCI_SLOT(bdf); + *func = PCI_FUNC(bdf); + /* + * NB: the lower 2 bits of the register address are fetched from the + * offset into the 0xcfc register when reading/writing to it. + */ + return CF8_ADDR_LO(cf8) | (addr & 3); +} + /* * Local variables: * mode: C diff --git a/xen/arch/x86/hvm/ioreq.c b/xen/arch/x86/hvm/ioreq.c index b2a8b0e986..752976d16d 100644 --- a/xen/arch/x86/hvm/ioreq.c +++ b/xen/arch/x86/hvm/ioreq.c @@ -1178,18 +1178,16 @@ struct hvm_ioreq_server *hvm_select_ioreq_server(struct domain *d, CF8_ENABLED(cf8) ) { uint32_t sbdf, x86_fam; + unsigned int bus, slot, func, reg; + + reg = hvm_pci_decode_addr(cf8, p->addr, &bus, &slot, &func); /* PCI config data cycle */ - sbdf = XEN_DMOP_PCI_SBDF(0, - PCI_BUS(CF8_BDF(cf8)), - PCI_SLOT(CF8_BDF(cf8)), - PCI_FUNC(CF8_BDF(cf8))); + sbdf = XEN_DMOP_PCI_SBDF(0, bus, slot, func); type = XEN_DMOP_IO_RANGE_PCI; - addr = ((uint64_t)sbdf << 32) | - CF8_ADDR_LO(cf8) | - (p->addr & 3); + addr = ((uint64_t)sbdf << 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 2484eb1c75..51659b6c7f 100644 --- a/xen/include/asm-x86/hvm/io.h +++ b/xen/include/asm-x86/hvm/io.h @@ -149,6 +149,11 @@ void stdvga_deinit(struct domain *d); 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); + /* * HVM port IO handler that performs forwarding of guest IO ports into machine * IO ports.
And use it in the ioreq code to decode accesses to the PCI IO ports into bus, slot, function and register values. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- Cc: Paul Durrant <paul.durrant@citrix.com> Cc: Jan Beulich <jbeulich@suse.com> Cc: Andrew Cooper <andrew.cooper3@citrix.com> --- Changes since v4: - New in this version. --- xen/arch/x86/hvm/io.c | 19 +++++++++++++++++++ xen/arch/x86/hvm/ioreq.c | 12 +++++------- xen/include/asm-x86/hvm/io.h | 5 +++++ 3 files changed, 29 insertions(+), 7 deletions(-)