On 19/04/2021 12:16, Jan Beulich wrote:
On 19.04.2021 10:40, Roger Pau Monné wrote:
On Mon, Apr 19, 2021 at 07:16:52AM +0000, Rahul Singh wrote:
Thanks you everyone for reviewing the code. I will summarise what I have
understood from all the comments
and what I will be doing for the next version of the patch. Please let me know
your view on this.
1. Create a separate non-arch specific file "msi-intercept.c" for the below
newly introduced function and
compile that file if CONFIG_PCI_MSI_INTERCEPT is
enabled.CONFIG_PCI_MSI_INTERCEPT will be
enabled for x86 by default.
Everything up to here wants to be separate from ...
Also Mention in the commit message that these function will be needed for Xen to
support MSI interrupt within XEN.
pdev_msi_initi(..)
pdev_msi_deiniti(..)
... this (if all of these functions really are needed beyond the
purpose of intercepting MSI accesses).
I would drop the last 'i' from both function names above, as we use
init/deinit in the rest of the code base.
+1
pdev_dump_msi(..),
pdev_msix_assign(..)
2. Create separate patch for iommu_update_ire_from_msi() related code. There
are two suggestion please help me which one to choose.
- Move the iommu_update_ire_from_msi() function to asm-x86/iommu.h and also move the hook from iommu_ops under CONFIG_X86.
I would go for this one.
Strictly speaking this isn't x86-specific and hence shouldn't move there.
It merely depends on whether full MSI support is wanted by an arch.
As I pointed out before, Arm doesn't use the IOMMU to setup the MSIs. So
the naming and using an IOMMU callback is definitely wrong for Arm.
I'd
therefore guard the declaration by an #ifdef (if needed at all - have a
declaration without implementation isn't really that problematic). For
the definition question is going to be whether you introduce another new
file for the pdev_*() functions above. If not, #ifdef may again be better
than moving to an x86-specific file.
AFAIK, this helper is only called by x86 specific code and it will not
be used as-is by Arm.
I can't tell for other arch (e.g RISCv, PowerPC). However... we can take
the decision to move the code back to common back when it is necessary.
For the time being, I think move this code in x86 is a lot better than
#ifdef or keep the code in common code.
Cheers,
--
Julien Grall