Re: [PATCH v1 2/2] xen/mmu: enable SMMU subsystem only in MMU

[Ayan Kumar Halder <ayankuma@xxxxxxx>]

On 11/11/2024 13:45, Julien Grall wrote:
> Hi Ayan,
Hi Julien,
> On 11/11/2024 13:24, Ayan Kumar Halder wrote:
> > On 11/11/2024 11:12, Julien Grall wrote:
> > > Hi,
> > Hi Julien,
> > > On 08/11/2024 19:59, Ayan Kumar Halder wrote:
> > > > From: Penny Zheng <Penny.Zheng@xxxxxxx>
> > > >
> > > > In Xen, SMMU subsystem is supported for MMU system only. The reason 
> > > > being SMMU
> > > > driver uses the same page tables as MMU.
> > > > Thus, we make it dependent on CONFIG_MMU.
> > > >
> > > > Signed-off-by: Penny Zheng <Penny.Zheng@xxxxxxx>
> > > > Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@xxxxxxx>
> > > > ---
> > > >   xen/arch/arm/Kconfig            | 2 +-
> > > >   xen/drivers/passthrough/Kconfig | 3 ++-
> > > >   2 files changed, 3 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> > > > index 15b2e4a227..3699e148e9 100644
> > > > --- a/xen/arch/arm/Kconfig
> > > > +++ b/xen/arch/arm/Kconfig
> > > > @@ -16,7 +16,7 @@ config ARM
> > > >       select HAS_DEVICE_TREE
> > > >       select HAS_PASSTHROUGH
> > > >       select HAS_UBSAN
> > > > -    select IOMMU_FORCE_PT_SHARE
> > > > +    select IOMMU_FORCE_PT_SHARE if MMU
> > > Realistically, everything under drivers/passthrough is MMU specific. 
> > > So does it actually make any sense to select HAS_PASSTHROUGH right now?
> > Actually we are able to assign devices to different DomUs (eg UART1 
> > to domU1) as long as the device isn't behind an IOMMU. So in our 
> > case, the passthrough device tree has this node
> >          uart@9c0b0000 {
> >              compatible = "arm,pl011\0arm,primecell";
> >              reg = <0x00 0x9c0b0000 0x00 0x10000>;
> >              interrupt-parent = <0x01>;
> >              interrupts = <0x00 0x07 0x04>;
> >              clock-names = "uartclk\0apb_pclk";
> >              clocks = <0x06 0x07>;
> >              xen,path = "/uart@9c0b0000";
> >              xen,reg = <0x00 0x9c0b0000 0x00 0x10000 0x00 0x9c0b0000>;
> >              xen,force-assign-without-iommu;
> So how devices will be protected on an MPU systems?
>
> >          };> So, should we still disable HAS_PASSTHROUGH for MPU ?
>
> While it may work, a lot of code in drivers/passthrough is IOMMU 
> specific (see all the function named iommu_*). So I find really odd 
> that you disable IOMMU_FORCE_PT_SHARE but all the rest is still 
> present...
> I think we need some consistency. If you are planning to do device 
> passthrough without any protection, then I don't think you need any 
> code within drivers/passthrough/ (at least for platform devices).
> Overall, for this patch, I think it would be better to simply select 
> HAS_PASSTHROUGH when MMU is enabled. We can revisit device passthrough 
> once we have the patches on the ML.
Yes, this makes sense. I will wait for Luca to confirm as well.
> BTW, I think it should be move under the Kconfig MMU rather than using 
> "if MMU".
Yes. So for now this should be sufficient.

diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 15b2e4a227..adebf8d5ca 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -14,9 +14,7 @@ config ARM
        select FUNCTION_ALIGNMENT_4B
        select HAS_ALTERNATIVE if HAS_VMAP
        select HAS_DEVICE_TREE
-       select HAS_PASSTHROUGH
        select HAS_UBSAN
-       select IOMMU_FORCE_PT_SHARE

 config ARCH_DEFCONFIG
        string
@@ -78,6 +76,8 @@ config MMU
        bool "MMU"
        select HAS_PMAP
        select HAS_VMAP
+       select HAS_PASSTHROUGH
+       select IOMMU_FORCE_PT_SHARE
        help
          Select it if you plan to run Xen on A-profile Armv7+

- Ayan


> Cheers,