| Message ID | 14624609-019f-2993-261c-d4f45ce78cbe@suse.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | AMD IOMMU: further improvements | expand |
On Thu, Sep 19, 2019 at 03:22:54PM +0200, Jan Beulich wrote: > Rather than doing this every time we set up interrupts for a device > anew (and then in several places) fill this invariant field right after > allocating struct pci_dev. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> LGTM: Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Just one nit below. > --- > v6: New. > > --- > xen/arch/x86/msi.c | 13 +++++-------- > xen/drivers/passthrough/pci.c | 10 ++++++++++ > xen/drivers/vpci/msi.c | 9 ++++----- > xen/include/xen/pci.h | 3 ++- > xen/include/xen/vpci.h | 6 ++---- > 5 files changed, 23 insertions(+), 18 deletions(-) > > --- a/xen/arch/x86/msi.c > +++ b/xen/arch/x86/msi.c > @@ -664,7 +664,7 @@ static int msi_capability_init(struct pc > { > struct msi_desc *entry; > int pos; > - unsigned int i, maxvec, mpos; > + unsigned int i, mpos; > u16 control, seg = dev->seg; > u8 bus = dev->bus; > u8 slot = PCI_SLOT(dev->devfn); > @@ -675,9 +675,8 @@ static int msi_capability_init(struct pc > if ( !pos ) > return -ENODEV; > control = pci_conf_read16(dev->sbdf, msi_control_reg(pos)); > - maxvec = multi_msi_capable(control); > - if ( nvec > maxvec ) > - return maxvec; > + if ( nvec > dev->msi_maxvec ) > + return dev->msi_maxvec; > control &= ~PCI_MSI_FLAGS_QSIZE; > multi_msi_enable(control, nvec); > > @@ -711,7 +710,7 @@ static int msi_capability_init(struct pc > > /* All MSIs are unmasked by default, Mask them all */ > maskbits = pci_conf_read32(dev->sbdf, mpos); > - maskbits |= ~(u32)0 >> (32 - maxvec); > + maskbits |= ~(u32)0 >> (32 - dev->msi_maxvec); GENMASK would be slightly easier to parse IMO (here and below). Thanks, Roger.
On 23.09.2019 16:22, Roger Pau Monné wrote: > On Thu, Sep 19, 2019 at 03:22:54PM +0200, Jan Beulich wrote: >> Rather than doing this every time we set up interrupts for a device >> anew (and then in several places) fill this invariant field right after >> allocating struct pci_dev. >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > > LGTM: > > Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Thanks. > Just one nit below. > >> @@ -711,7 +710,7 @@ static int msi_capability_init(struct pc >> >> /* All MSIs are unmasked by default, Mask them all */ >> maskbits = pci_conf_read32(dev->sbdf, mpos); >> - maskbits |= ~(u32)0 >> (32 - maxvec); >> + maskbits |= ~(u32)0 >> (32 - dev->msi_maxvec); > > GENMASK would be slightly easier to parse IMO (here and below). Besides this being an unrelated change, I'm afraid I'm going to object to use of a macro where it's unclear what its parameters mean: Even the example in xen/bitops.h is so confusing that I can't tell whether "h" is meant to be exclusive or inclusive (looks like the latter is intended). To me the two parameters also look reversed - I'd expect "low" first and "high" second. (ISTR having voiced reservations against its introduction, and I'm happy to see that it's used in Arm code only.) Jan
On Mon, Sep 23, 2019 at 04:41:01PM +0200, Jan Beulich wrote: > On 23.09.2019 16:22, Roger Pau Monné wrote: > > On Thu, Sep 19, 2019 at 03:22:54PM +0200, Jan Beulich wrote: > >> Rather than doing this every time we set up interrupts for a device > >> anew (and then in several places) fill this invariant field right after > >> allocating struct pci_dev. > >> > >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > > > > LGTM: > > > > Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> > > Thanks. > > > Just one nit below. > > > >> @@ -711,7 +710,7 @@ static int msi_capability_init(struct pc > >> > >> /* All MSIs are unmasked by default, Mask them all */ > >> maskbits = pci_conf_read32(dev->sbdf, mpos); > >> - maskbits |= ~(u32)0 >> (32 - maxvec); > >> + maskbits |= ~(u32)0 >> (32 - dev->msi_maxvec); > > > > GENMASK would be slightly easier to parse IMO (here and below). > > Besides this being an unrelated change, I'm afraid I'm going to > object to use of a macro where it's unclear what its parameters > mean: Even the example in xen/bitops.h is so confusing that I > can't tell whether "h" is meant to be exclusive or inclusive > (looks like the latter is intended). To me the two parameters > also look reversed - I'd expect "low" first and "high" second. > (ISTR having voiced reservations against its introduction, and > I'm happy to see that it's used in Arm code only.) I'm not specially trilled to switch to GENMASK, but would you be willing to change u32 to uint32_t? Thanks, Roger.
On 23.09.2019 17:18, Roger Pau Monné wrote: > On Mon, Sep 23, 2019 at 04:41:01PM +0200, Jan Beulich wrote: >> On 23.09.2019 16:22, Roger Pau Monné wrote: >>> On Thu, Sep 19, 2019 at 03:22:54PM +0200, Jan Beulich wrote: >>>> Rather than doing this every time we set up interrupts for a device >>>> anew (and then in several places) fill this invariant field right after >>>> allocating struct pci_dev. >>>> >>>> Signed-off-by: Jan Beulich <jbeulich@suse.com> >>> >>> LGTM: >>> >>> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> >> >> Thanks. >> >>> Just one nit below. >>> >>>> @@ -711,7 +710,7 @@ static int msi_capability_init(struct pc >>>> >>>> /* All MSIs are unmasked by default, Mask them all */ >>>> maskbits = pci_conf_read32(dev->sbdf, mpos); >>>> - maskbits |= ~(u32)0 >> (32 - maxvec); >>>> + maskbits |= ~(u32)0 >> (32 - dev->msi_maxvec); >>> >>> GENMASK would be slightly easier to parse IMO (here and below). >> >> Besides this being an unrelated change, I'm afraid I'm going to >> object to use of a macro where it's unclear what its parameters >> mean: Even the example in xen/bitops.h is so confusing that I >> can't tell whether "h" is meant to be exclusive or inclusive >> (looks like the latter is intended). To me the two parameters >> also look reversed - I'd expect "low" first and "high" second. >> (ISTR having voiced reservations against its introduction, and >> I'm happy to see that it's used in Arm code only.) > > I'm not specially trilled to switch to GENMASK, but would you be > willing to change u32 to uint32_t? Noticing your remark's context, I've done that change already (and I don't know why I missed doing so right away). Jan
> -----Original Message----- > From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of Jan Beulich > Sent: 19 September 2019 14:23 > To: xen-devel@lists.xenproject.org > Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Wei Liu <wl@xen.org>; Suravee Suthikulpanit > <suravee.suthikulpanit@amd.com>; Roger Pau Monne <roger.pau@citrix.com> > Subject: [Xen-devel] [PATCH v6 3/8] x86/PCI: read maximum MSI vector count early > > Rather than doing this every time we set up interrupts for a device > anew (and then in several places) fill this invariant field right after > allocating struct pci_dev. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Paul Durrant <paul.durrant@citrix.com> ...with one nit... > --- > v6: New. > > --- > xen/arch/x86/msi.c | 13 +++++-------- > xen/drivers/passthrough/pci.c | 10 ++++++++++ > xen/drivers/vpci/msi.c | 9 ++++----- > xen/include/xen/pci.h | 3 ++- > xen/include/xen/vpci.h | 6 ++---- > 5 files changed, 23 insertions(+), 18 deletions(-) > > --- a/xen/arch/x86/msi.c > +++ b/xen/arch/x86/msi.c > @@ -664,7 +664,7 @@ static int msi_capability_init(struct pc > { > struct msi_desc *entry; > int pos; > - unsigned int i, maxvec, mpos; > + unsigned int i, mpos; > u16 control, seg = dev->seg; > u8 bus = dev->bus; > u8 slot = PCI_SLOT(dev->devfn); > @@ -675,9 +675,8 @@ static int msi_capability_init(struct pc > if ( !pos ) > return -ENODEV; > control = pci_conf_read16(dev->sbdf, msi_control_reg(pos)); > - maxvec = multi_msi_capable(control); > - if ( nvec > maxvec ) > - return maxvec; > + if ( nvec > dev->msi_maxvec ) > + return dev->msi_maxvec; > control &= ~PCI_MSI_FLAGS_QSIZE; > multi_msi_enable(control, nvec); > > @@ -711,7 +710,7 @@ static int msi_capability_init(struct pc > > /* All MSIs are unmasked by default, Mask them all */ > maskbits = pci_conf_read32(dev->sbdf, mpos); > - maskbits |= ~(u32)0 >> (32 - maxvec); > + maskbits |= ~(u32)0 >> (32 - dev->msi_maxvec); > pci_conf_write32(dev->sbdf, mpos, maskbits); > } > list_add_tail(&entry->list, &dev->msi_list); > @@ -1284,7 +1283,6 @@ int pci_msi_conf_write_intercept(struct > entry = find_msi_entry(pdev, -1, PCI_CAP_ID_MSI); > if ( entry && entry->msi_attrib.maskbit ) > { > - uint16_t cntl; > uint32_t unused; > unsigned int nvec = entry->msi.nvec; > > @@ -1297,8 +1295,7 @@ int pci_msi_conf_write_intercept(struct > if ( reg < entry->msi.mpos || reg >= entry->msi.mpos + 4 || size != 4 ) > return -EACCES; > > - cntl = pci_conf_read16(pdev->sbdf, msi_control_reg(pos)); > - unused = ~(uint32_t)0 >> (32 - multi_msi_capable(cntl)); > + unused = ~(uint32_t)0 >> (32 - pdev->msi_maxvec); > for ( pos = 0; pos < nvec; ++pos, ++entry ) > { > entry->msi_attrib.guest_masked = > --- a/xen/drivers/passthrough/pci.c > +++ b/xen/drivers/passthrough/pci.c > @@ -340,6 +340,16 @@ static struct pci_dev *alloc_pdev(struct > pdev->domain = NULL; > INIT_LIST_HEAD(&pdev->msi_list); > > + Stray blank line here by the looks of it. > + pos = pci_find_cap_offset(pseg->nr, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), > + PCI_CAP_ID_MSI); > + if ( pos ) > + { > + uint16_t ctrl = pci_conf_read16(pdev->sbdf, msi_control_reg(pos)); > + > + pdev->msi_maxvec = multi_msi_capable(ctrl); > + } > + > pos = pci_find_cap_offset(pseg->nr, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), > PCI_CAP_ID_MSIX); > if ( pos ) > --- a/xen/drivers/vpci/msi.c > +++ b/xen/drivers/vpci/msi.c > @@ -27,7 +27,7 @@ static uint32_t control_read(const struc > { > const struct vpci_msi *msi = data; > > - return MASK_INSR(fls(msi->max_vectors) - 1, PCI_MSI_FLAGS_QMASK) | > + return MASK_INSR(fls(pdev->msi_maxvec) - 1, PCI_MSI_FLAGS_QMASK) | > MASK_INSR(fls(msi->vectors) - 1, PCI_MSI_FLAGS_QSIZE) | > (msi->enabled ? PCI_MSI_FLAGS_ENABLE : 0) | > (msi->masking ? PCI_MSI_FLAGS_MASKBIT : 0) | > @@ -40,7 +40,7 @@ static void control_write(const struct p > struct vpci_msi *msi = data; > unsigned int vectors = min_t(uint8_t, > 1u << MASK_EXTR(val, PCI_MSI_FLAGS_QSIZE), > - msi->max_vectors); > + pdev->msi_maxvec); > bool new_enabled = val & PCI_MSI_FLAGS_ENABLE; > > /* > @@ -215,8 +215,7 @@ static int init_msi(struct pci_dev *pdev > * FIXME: I've only been able to test this code with devices using a single > * MSI interrupt and no mask register. > */ > - pdev->vpci->msi->max_vectors = multi_msi_capable(control); > - ASSERT(pdev->vpci->msi->max_vectors <= 32); > + ASSERT(pdev->msi_maxvec <= 32); > > /* The multiple message enable is 0 after reset (1 message enabled). */ > pdev->vpci->msi->vectors = 1; > @@ -298,7 +297,7 @@ void vpci_dump_msi(void) > if ( msi->masking ) > printk(" mask=%08x", msi->mask); > printk(" vectors max: %u enabled: %u\n", > - msi->max_vectors, msi->vectors); > + pdev->msi_maxvec, msi->vectors); > > vpci_msi_arch_print(msi); > } > --- a/xen/include/xen/pci.h > +++ b/xen/include/xen/pci.h > @@ -94,7 +94,8 @@ struct pci_dev { > pci_sbdf_t sbdf; > }; > > - u8 phantom_stride; > + uint8_t msi_maxvec; > + uint8_t phantom_stride; > > nodeid_t node; /* NUMA node */ > > --- a/xen/include/xen/vpci.h > +++ b/xen/include/xen/vpci.h > @@ -99,14 +99,12 @@ struct vpci { > uint32_t mask; > /* Data. */ > uint16_t data; > - /* Maximum number of vectors supported by the device. */ > - uint8_t max_vectors : 6; > + /* Number of vectors configured. */ > + uint8_t vectors : 6; > /* Supports per-vector masking? */ > bool masking : 1; > /* 64-bit address capable? */ > bool address64 : 1; > - /* Number of vectors configured. */ > - uint8_t vectors : 6; > /* Enabled? */ > bool enabled : 1; > /* Arch-specific data. */ > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xenproject.org > https://lists.xenproject.org/mailman/listinfo/xen-devel
On 23.09.2019 17:57, Paul Durrant wrote: >> -----Original Message----- >> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of Jan Beulich >> Sent: 19 September 2019 14:23 >> To: xen-devel@lists.xenproject.org >> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Wei Liu <wl@xen.org>; Suravee Suthikulpanit >> <suravee.suthikulpanit@amd.com>; Roger Pau Monne <roger.pau@citrix.com> >> Subject: [Xen-devel] [PATCH v6 3/8] x86/PCI: read maximum MSI vector count early >> >> Rather than doing this every time we set up interrupts for a device >> anew (and then in several places) fill this invariant field right after >> allocating struct pci_dev. >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > > Reviewed-by: Paul Durrant <paul.durrant@citrix.com> Thanks. >> --- a/xen/drivers/passthrough/pci.c >> +++ b/xen/drivers/passthrough/pci.c >> @@ -340,6 +340,16 @@ static struct pci_dev *alloc_pdev(struct >> pdev->domain = NULL; >> INIT_LIST_HEAD(&pdev->msi_list); >> >> + > > Stray blank line here by the looks of it. Oh, indeed - dropped. Jan >> + pos = pci_find_cap_offset(pseg->nr, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), >> + PCI_CAP_ID_MSI); >> + if ( pos ) >> + { >> + uint16_t ctrl = pci_conf_read16(pdev->sbdf, msi_control_reg(pos)); >> + >> + pdev->msi_maxvec = multi_msi_capable(ctrl); >> + } >> + >> pos = pci_find_cap_offset(pseg->nr, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), >> PCI_CAP_ID_MSIX); >> if ( pos )
On 19/09/2019 14:22, Jan Beulich wrote: > Rather than doing this every time we set up interrupts for a device > anew (and then in several places) fill this invariant field right after > allocating struct pci_dev. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
On 23/09/2019 15:41, Jan Beulich wrote: > On 23.09.2019 16:22, Roger Pau Monné wrote: >> On Thu, Sep 19, 2019 at 03:22:54PM +0200, Jan Beulich wrote: >>> Rather than doing this every time we set up interrupts for a device >>> anew (and then in several places) fill this invariant field right after >>> allocating struct pci_dev. >>> >>> Signed-off-by: Jan Beulich <jbeulich@suse.com> >> LGTM: >> >> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> > Thanks. > >> Just one nit below. >> >>> @@ -711,7 +710,7 @@ static int msi_capability_init(struct pc >>> >>> /* All MSIs are unmasked by default, Mask them all */ >>> maskbits = pci_conf_read32(dev->sbdf, mpos); >>> - maskbits |= ~(u32)0 >> (32 - maxvec); >>> + maskbits |= ~(u32)0 >> (32 - dev->msi_maxvec); >> GENMASK would be slightly easier to parse IMO (here and below). > Besides this being an unrelated change, I'm afraid I'm going to > object to use of a macro where it's unclear what its parameters > mean: Even the example in xen/bitops.h is so confusing that I > can't tell whether "h" is meant to be exclusive or inclusive > (looks like the latter is intended). To me the two parameters > also look reversed - I'd expect "low" first and "high" second. > (ISTR having voiced reservations against its introduction, and > I'm happy to see that it's used in Arm code only.) Furthermore, Linux is having enough problems with it that they were considering abolishing it entirely. Getting the two numbers the wrong way around gets you a mask of 0. It is a very unsafe macro. -1 to any use in Xen, even in the ARM code. (I realise this is not my call, but this clearly expresses my opinion about it.) ~Andrew
On 25.09.2019 15:31, Andrew Cooper wrote: > On 23/09/2019 15:41, Jan Beulich wrote: >> On 23.09.2019 16:22, Roger Pau Monné wrote: >>> On Thu, Sep 19, 2019 at 03:22:54PM +0200, Jan Beulich wrote: >>>> Rather than doing this every time we set up interrupts for a device >>>> anew (and then in several places) fill this invariant field right after >>>> allocating struct pci_dev. >>>> >>>> Signed-off-by: Jan Beulich <jbeulich@suse.com> >>> LGTM: >>> >>> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> >> Thanks. >> >>> Just one nit below. >>> >>>> @@ -711,7 +710,7 @@ static int msi_capability_init(struct pc >>>> >>>> /* All MSIs are unmasked by default, Mask them all */ >>>> maskbits = pci_conf_read32(dev->sbdf, mpos); >>>> - maskbits |= ~(u32)0 >> (32 - maxvec); >>>> + maskbits |= ~(u32)0 >> (32 - dev->msi_maxvec); >>> GENMASK would be slightly easier to parse IMO (here and below). >> Besides this being an unrelated change, I'm afraid I'm going to >> object to use of a macro where it's unclear what its parameters >> mean: Even the example in xen/bitops.h is so confusing that I >> can't tell whether "h" is meant to be exclusive or inclusive >> (looks like the latter is intended). To me the two parameters >> also look reversed - I'd expect "low" first and "high" second. >> (ISTR having voiced reservations against its introduction, and >> I'm happy to see that it's used in Arm code only.) > > Furthermore, Linux is having enough problems with it that they were > considering abolishing it entirely. > > Getting the two numbers the wrong way around gets you a mask of 0. It > is a very unsafe macro. Where, having looked at it some when replying to Roger, it seemed to me that it would have been quite simple to make the macro tolerate either order. Jan
--- a/xen/arch/x86/msi.c +++ b/xen/arch/x86/msi.c @@ -664,7 +664,7 @@ static int msi_capability_init(struct pc { struct msi_desc *entry; int pos; - unsigned int i, maxvec, mpos; + unsigned int i, mpos; u16 control, seg = dev->seg; u8 bus = dev->bus; u8 slot = PCI_SLOT(dev->devfn); @@ -675,9 +675,8 @@ static int msi_capability_init(struct pc if ( !pos ) return -ENODEV; control = pci_conf_read16(dev->sbdf, msi_control_reg(pos)); - maxvec = multi_msi_capable(control); - if ( nvec > maxvec ) - return maxvec; + if ( nvec > dev->msi_maxvec ) + return dev->msi_maxvec; control &= ~PCI_MSI_FLAGS_QSIZE; multi_msi_enable(control, nvec); @@ -711,7 +710,7 @@ static int msi_capability_init(struct pc /* All MSIs are unmasked by default, Mask them all */ maskbits = pci_conf_read32(dev->sbdf, mpos); - maskbits |= ~(u32)0 >> (32 - maxvec); + maskbits |= ~(u32)0 >> (32 - dev->msi_maxvec); pci_conf_write32(dev->sbdf, mpos, maskbits); } list_add_tail(&entry->list, &dev->msi_list); @@ -1284,7 +1283,6 @@ int pci_msi_conf_write_intercept(struct entry = find_msi_entry(pdev, -1, PCI_CAP_ID_MSI); if ( entry && entry->msi_attrib.maskbit ) { - uint16_t cntl; uint32_t unused; unsigned int nvec = entry->msi.nvec; @@ -1297,8 +1295,7 @@ int pci_msi_conf_write_intercept(struct if ( reg < entry->msi.mpos || reg >= entry->msi.mpos + 4 || size != 4 ) return -EACCES; - cntl = pci_conf_read16(pdev->sbdf, msi_control_reg(pos)); - unused = ~(uint32_t)0 >> (32 - multi_msi_capable(cntl)); + unused = ~(uint32_t)0 >> (32 - pdev->msi_maxvec); for ( pos = 0; pos < nvec; ++pos, ++entry ) { entry->msi_attrib.guest_masked = --- a/xen/drivers/passthrough/pci.c +++ b/xen/drivers/passthrough/pci.c @@ -340,6 +340,16 @@ static struct pci_dev *alloc_pdev(struct pdev->domain = NULL; INIT_LIST_HEAD(&pdev->msi_list); + + pos = pci_find_cap_offset(pseg->nr, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), + PCI_CAP_ID_MSI); + if ( pos ) + { + uint16_t ctrl = pci_conf_read16(pdev->sbdf, msi_control_reg(pos)); + + pdev->msi_maxvec = multi_msi_capable(ctrl); + } + pos = pci_find_cap_offset(pseg->nr, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), PCI_CAP_ID_MSIX); if ( pos ) --- a/xen/drivers/vpci/msi.c +++ b/xen/drivers/vpci/msi.c @@ -27,7 +27,7 @@ static uint32_t control_read(const struc { const struct vpci_msi *msi = data; - return MASK_INSR(fls(msi->max_vectors) - 1, PCI_MSI_FLAGS_QMASK) | + return MASK_INSR(fls(pdev->msi_maxvec) - 1, PCI_MSI_FLAGS_QMASK) | MASK_INSR(fls(msi->vectors) - 1, PCI_MSI_FLAGS_QSIZE) | (msi->enabled ? PCI_MSI_FLAGS_ENABLE : 0) | (msi->masking ? PCI_MSI_FLAGS_MASKBIT : 0) | @@ -40,7 +40,7 @@ static void control_write(const struct p struct vpci_msi *msi = data; unsigned int vectors = min_t(uint8_t, 1u << MASK_EXTR(val, PCI_MSI_FLAGS_QSIZE), - msi->max_vectors); + pdev->msi_maxvec); bool new_enabled = val & PCI_MSI_FLAGS_ENABLE; /* @@ -215,8 +215,7 @@ static int init_msi(struct pci_dev *pdev * FIXME: I've only been able to test this code with devices using a single * MSI interrupt and no mask register. */ - pdev->vpci->msi->max_vectors = multi_msi_capable(control); - ASSERT(pdev->vpci->msi->max_vectors <= 32); + ASSERT(pdev->msi_maxvec <= 32); /* The multiple message enable is 0 after reset (1 message enabled). */ pdev->vpci->msi->vectors = 1; @@ -298,7 +297,7 @@ void vpci_dump_msi(void) if ( msi->masking ) printk(" mask=%08x", msi->mask); printk(" vectors max: %u enabled: %u\n", - msi->max_vectors, msi->vectors); + pdev->msi_maxvec, msi->vectors); vpci_msi_arch_print(msi); } --- a/xen/include/xen/pci.h +++ b/xen/include/xen/pci.h @@ -94,7 +94,8 @@ struct pci_dev { pci_sbdf_t sbdf; }; - u8 phantom_stride; + uint8_t msi_maxvec; + uint8_t phantom_stride; nodeid_t node; /* NUMA node */ --- a/xen/include/xen/vpci.h +++ b/xen/include/xen/vpci.h @@ -99,14 +99,12 @@ struct vpci { uint32_t mask; /* Data. */ uint16_t data; - /* Maximum number of vectors supported by the device. */ - uint8_t max_vectors : 6; + /* Number of vectors configured. */ + uint8_t vectors : 6; /* Supports per-vector masking? */ bool masking : 1; /* 64-bit address capable? */ bool address64 : 1; - /* Number of vectors configured. */ - uint8_t vectors : 6; /* Enabled? */ bool enabled : 1; /* Arch-specific data. */
Rather than doing this every time we set up interrupts for a device anew (and then in several places) fill this invariant field right after allocating struct pci_dev. Signed-off-by: Jan Beulich <jbeulich@suse.com> --- v6: New. --- xen/arch/x86/msi.c | 13 +++++-------- xen/drivers/passthrough/pci.c | 10 ++++++++++ xen/drivers/vpci/msi.c | 9 ++++----- xen/include/xen/pci.h | 3 ++- xen/include/xen/vpci.h | 6 ++---- 5 files changed, 23 insertions(+), 18 deletions(-)