| Message ID | 20250202134840.40102-1-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [for-4.21] x86/msi: Change __msi_set_enable() to take pci_sbdf_t | expand |
On 02.02.2025 14:48, Andrew Cooper wrote: > This removes the unnecessary work of splitting a 32-bit number across > 4 registers, and recombining later. Bloat-o-meter reports: > > add/remove: 0/0 grow/shrink: 0/9 up/down: 0/-295 (-295) > Function old new delta > enable_iommu 1748 1732 -16 > iommu_msi_unmask 98 81 -17 > iommu_msi_mask 100 83 -17 > disable_iommu 286 269 -17 > __msi_set_enable 81 50 -31 > __pci_disable_msi 178 146 -32 > pci_cleanup_msi 268 229 -39 > pci_enable_msi 1063 1019 -44 > pci_restore_msi_state 1116 1034 -82 > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> I was actually thinking to do the same. And we have more of such conversions to be done. Jan
On 03/02/2025 8:42 am, Jan Beulich wrote: > On 02.02.2025 14:48, Andrew Cooper wrote: >> This removes the unnecessary work of splitting a 32-bit number across >> 4 registers, and recombining later. Bloat-o-meter reports: >> >> add/remove: 0/0 grow/shrink: 0/9 up/down: 0/-295 (-295) >> Function old new delta >> enable_iommu 1748 1732 -16 >> iommu_msi_unmask 98 81 -17 >> iommu_msi_mask 100 83 -17 >> disable_iommu 286 269 -17 >> __msi_set_enable 81 50 -31 >> __pci_disable_msi 178 146 -32 >> pci_cleanup_msi 268 229 -39 >> pci_enable_msi 1063 1019 -44 >> pci_restore_msi_state 1116 1034 -82 >> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > Reviewed-by: Jan Beulich <jbeulich@suse.com> Thanks. > I was actually thinking to do the same. And we have more of such conversions > to be done. Yes. This happened to be an easy one I spotted while reviewing your series, that I could do on the train. AMD IOMMU should move to a pci_sbdf_t too. Right amd_iommu's seg and bdf fields are opposite way to a pci_sbdf_t. But it occurs to me (having just been at a conference where people were asking for easy introductory work), that stuff like this is a good candidate. I'll see about doing a gitlab ticket. ~Andrew
diff --git a/xen/arch/x86/include/asm/msi.h b/xen/arch/x86/include/asm/msi.h index 63adb19820e8..5bb9abd3eb6f 100644 --- a/xen/arch/x86/include/asm/msi.h +++ b/xen/arch/x86/include/asm/msi.h @@ -237,7 +237,7 @@ struct arch_msix { void early_msi_init(void); void msi_compose_msg(unsigned vector, const cpumask_t *cpu_mask, struct msi_msg *msg); -void __msi_set_enable(u16 seg, u8 bus, u8 slot, u8 func, int pos, int enable); +void __msi_set_enable(pci_sbdf_t sbdf, int pos, int enable); void cf_check mask_msi_irq(struct irq_desc *desc); void cf_check unmask_msi_irq(struct irq_desc *desc); void guest_mask_msi_irq(struct irq_desc *desc, bool mask); diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c index e2360579deda..52117d97b522 100644 --- a/xen/arch/x86/msi.c +++ b/xen/arch/x86/msi.c @@ -267,28 +267,22 @@ void cf_check set_msi_affinity(struct irq_desc *desc, const cpumask_t *mask) write_msi_msg(msi_desc, &msg); } -void __msi_set_enable(u16 seg, u8 bus, u8 slot, u8 func, int pos, int enable) +void __msi_set_enable(pci_sbdf_t sbdf, int pos, int enable) { - uint16_t control = pci_conf_read16(PCI_SBDF(seg, bus, slot, func), - pos + PCI_MSI_FLAGS); + uint16_t control = pci_conf_read16(sbdf, pos + PCI_MSI_FLAGS); control &= ~PCI_MSI_FLAGS_ENABLE; if ( enable ) control |= PCI_MSI_FLAGS_ENABLE; - pci_conf_write16(PCI_SBDF(seg, bus, slot, func), - pos + PCI_MSI_FLAGS, control); + pci_conf_write16(sbdf, pos + PCI_MSI_FLAGS, control); } static void msi_set_enable(struct pci_dev *dev, int enable) { unsigned int pos = dev->msi_pos; - u16 seg = dev->seg; - u8 bus = dev->bus; - u8 slot = PCI_SLOT(dev->devfn); - u8 func = PCI_FUNC(dev->devfn); if ( pos ) - __msi_set_enable(seg, bus, slot, func, pos, enable); + __msi_set_enable(dev->sbdf, pos, enable); } static void msix_set_enable(struct pci_dev *dev, int enable) diff --git a/xen/drivers/passthrough/amd/iommu_init.c b/xen/drivers/passthrough/amd/iommu_init.c index 05fd3bde6e29..f1076bf11d62 100644 --- a/xen/drivers/passthrough/amd/iommu_init.c +++ b/xen/drivers/passthrough/amd/iommu_init.c @@ -409,8 +409,9 @@ static void iommu_reset_log(struct amd_iommu *iommu, static void amd_iommu_msi_enable(struct amd_iommu *iommu, int flag) { - __msi_set_enable(iommu->seg, PCI_BUS(iommu->bdf), PCI_SLOT(iommu->bdf), - PCI_FUNC(iommu->bdf), iommu->msi.msi_attrib.pos, flag); + pci_sbdf_t sbdf = { .seg = iommu->seg, .bdf = iommu->bdf }; + + __msi_set_enable(sbdf, iommu->msi.msi_attrib.pos, flag); } static void cf_check iommu_msi_unmask(struct irq_desc *desc)
This removes the unnecessary work of splitting a 32-bit number across 4 registers, and recombining later. Bloat-o-meter reports: add/remove: 0/0 grow/shrink: 0/9 up/down: 0/-295 (-295) Function old new delta enable_iommu 1748 1732 -16 iommu_msi_unmask 98 81 -17 iommu_msi_mask 100 83 -17 disable_iommu 286 269 -17 __msi_set_enable 81 50 -31 __pci_disable_msi 178 146 -32 pci_cleanup_msi 268 229 -39 pci_enable_msi 1063 1019 -44 pci_restore_msi_state 1116 1034 -82 Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <jbeulich@suse.com> CC: Roger Pau Monné <roger.pau@citrix.com> --- xen/arch/x86/include/asm/msi.h | 2 +- xen/arch/x86/msi.c | 14 ++++---------- xen/drivers/passthrough/amd/iommu_init.c | 5 +++-- 3 files changed, 8 insertions(+), 13 deletions(-)