[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v3 2/2] x86/iommu: avoid MSI address and data writes if IRT index hasn't changed
On 10.03.2025 10:55, Roger Pau Monne wrote: > Attempt to reduce the MSI entry writes, and the associated checking whether > memory decoding and MSI-X is enabled for the PCI device, when the MSI data > hasn't changed. > > When using Interrupt Remapping the MSI entry will contain an index into > the remapping table, and it's in such remapping table where the MSI vector > and destination CPU is stored. As such, when using interrupt remapping, > changes to the interrupt affinity shouldn't result in changes to the MSI > entry, and the MSI entry update can be avoided. > > Signal from the IOMMU update_ire_from_msi hook whether the MSI data or > address fields have changed, and thus need writing to the device registers. > Such signaling is done by returning 1 from the function. Otherwise > returning 0 means no update of the MSI fields, and thus no write > required. > > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx> with two purely cosmetic suggestions and an only loosely related question below. > --- a/xen/arch/x86/hvm/vmx/vmx.c > +++ b/xen/arch/x86/hvm/vmx/vmx.c > @@ -415,7 +415,9 @@ static int cf_check vmx_pi_update_irte(const struct vcpu > *v, > > ASSERT_PDEV_LIST_IS_READ_LOCKED(msi_desc->dev->domain); > > - return iommu_update_ire_from_msi(msi_desc, &msi_desc->msg); > + rc = iommu_update_ire_from_msi(msi_desc, &msi_desc->msg); > + > + return rc < 0 ? rc : 0; Only tangential here, but: Why does this function have a return type of non-void, when neither caller cares? > --- a/xen/drivers/passthrough/amd/iommu_intr.c > +++ b/xen/drivers/passthrough/amd/iommu_intr.c > @@ -492,7 +492,7 @@ static int update_intremap_entry_from_msi_msg( > get_ivrs_mappings(iommu->seg)[alias_id].intremap_table); > } > > - return 0; > + return !fresh ? 0 : 1; > } Simply return fresh; ? > @@ -546,7 +546,7 @@ int cf_check amd_iommu_msi_msg_update_ire( > rc = update_intremap_entry_from_msi_msg(iommu, bdf, nr, > &msi_desc->remap_index, > msg, &data); > - if ( !rc ) > + if ( rc > 0 ) > { > for ( i = 1; i < nr; ++i ) > msi_desc[i].remap_index = msi_desc->remap_index + i; > --- a/xen/drivers/passthrough/vtd/intremap.c > +++ b/xen/drivers/passthrough/vtd/intremap.c > @@ -506,6 +506,7 @@ static int msi_msg_to_remap_entry( > unsigned int index, i, nr = 1; > unsigned long flags; > const struct pi_desc *pi_desc = msi_desc->pi_desc; > + bool alloc = false; > > if ( msi_desc->msi_attrib.type == PCI_CAP_ID_MSI ) > nr = msi_desc->msi.nvec; > @@ -529,6 +530,7 @@ static int msi_msg_to_remap_entry( > index = alloc_remap_entry(iommu, nr); > for ( i = 0; i < nr; ++i ) > msi_desc[i].remap_index = index + i; > + alloc = true; > } > else > index = msi_desc->remap_index; > @@ -601,7 +603,7 @@ static int msi_msg_to_remap_entry( > unmap_vtd_domain_page(iremap_entries); > spin_unlock_irqrestore(&iommu->intremap.lock, flags); > > - return 0; > + return alloc ? 1 : 0; > } Like above, simply return alloc; ? Jan
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |