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

[Luca Fancellu <Luca.Fancellu@xxxxxxx>]

Hi Ayan,


> On 11 Nov 2024, at 16:00, Ayan Kumar Halder <ayankuma@xxxxxxx> wrote:
> 
> 
> 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.

It makes sense to don’t compile all that stuff, anyway we are using some 
functions from drivers/passthrough/device_tree.c to pass the pl011 to the 
guests, we will try to handle them later in the series then.

Cheers,
Luca