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

[Julien Grall <julien@xxxxxxx>]

Hi Ayan,

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.
BTW, I think it should be move under the Kconfig MMU rather than using 
"if MMU".
Cheers,

--
Julien Grall