Hi Julien, > On Aug 22, 2023, at 16:18, Julien Grall <jul...@xen.org> wrote: > > On 22/08/2023 03:11, Henry Wang wrote: >> Hi Julien, > > Hi, > >>> On Aug 22, 2023, at 05:34, Julien Grall <jul...@xen.org> wrote: >>> >>> Hi, >>> >>> On 14/08/2023 05:25, Henry Wang wrote: >>>> From: Penny Zheng <penny.zh...@arm.com> >>>> SMMU subsystem is only supported in MMU system, so we make it dependent >>>> on CONFIG_HAS_MMU. >>> >>> "only supported" as in it doesn't work with Xen or the HW is not supporting >>> it? >> I think currently there are no hardware combination of MPU + SMMU, but >> theoretically I think this is a valid combination since SMMU supports the >> linear >> mapping. So would below reword looks good to you: >> “Currently the hardware use case of connecting SMMU to MPU system is rarely >> seen, so we make CONFIG_ARM_SMMU and CONFIG_ARM_SMMU_V3 >> dependent on CONFIG_MMU." > > I read this as there might be MPU system with SMMU in development. What you > want to explain is why we can't let the developper to select the SMMU driver > on an MPU system. > > From my understanding this is because the drivers are expecting to use the > page-tables and the concept doesn't exist in the MPU system. So the drivers > are not ready for the MPU.
I agree. > >>> >>> Also, I am not entirely convinced that anything in passthrough would >>> properly work with MPU. At least none of the IOMMU drivers are. So I would >>> consider to completely disable HAS_PASSTHROUGH. >> I agree, do you think adding below addition diff to this patch makes sense >> to you? > > I think it should be a replacement because none of the IOMMU drivers works > for the MPU. So I would rather prefer if we avoid adding "depends on" on all > of them (even if there are only 3) for now. I am a bit confused, I read your above explanation to the commit message as you agree with the idea that making SMMU driver not selectable by MPU system. If we replace this with “select HAS_PASSTHROUGH if MMU”, the SMMU driver will be selectable by MPU system. But... > >> If so I guess would also need to mention this in commit message. > > Did you confirm that Xen MPU still build without HAS_PASSTHROUGH? …this is a good catch, no MPU is not buildable without HAS_PASSTHROUGH, we will have: ``` In file included from ./include/xen/sched.h:12, from ./include/xen/iocap.h:10, from arch/arm/p2m.c:3: arch/arm/p2m.c: In function 'p2m_set_way_flush': ./include/xen/iommu.h:366:40: error: 'struct domain' has no member named 'iommu' 366 | #define dom_iommu(d) (&(d)->iommu) | ^~ ./include/xen/iommu.h:371:36: note: in expansion of macro 'dom_iommu' 371 | #define iommu_use_hap_pt(d) (dom_iommu(d)->hap_pt_share) | ^~~~~~~~~ arch/arm/p2m.c:617:10: note: in expansion of macro 'iommu_use_hap_pt' 617 | if ( iommu_use_hap_pt(current->domain) ) | ^~~~~~~~~~~~~~~~ In file included from ./include/xen/sched.h:12, from ./arch/arm/include/asm/grant_table.h:7, from ./include/xen/grant_table.h:29, from arch/arm/domain.c:4: arch/arm/domain.c: In function 'arch_domain_creation_finished': ./include/xen/iommu.h:366:40: error: 'struct domain' has no member named 'iommu' 366 | #define dom_iommu(d) (&(d)->iommu) | ^~ ./include/xen/iommu.h:371:36: note: in expansion of macro 'dom_iommu' 371 | #define iommu_use_hap_pt(d) (dom_iommu(d)->hap_pt_share) | ^~~~~~~~~ arch/arm/domain.c:880:11: note: in expansion of macro 'iommu_use_hap_pt' 880 | if ( !iommu_use_hap_pt(d) ) | ^~~~~~~~~~~~~~~~ CC arch/arm/shutdown.o CC lib/strlen.o CC arch/arm/smp.o CC arch/arm/smpboot.o CC common/xmalloc_tlsf.o CC lib/strncasecmp.o make[2]: *** [Rules.mk:247: arch/arm/p2m.o] Error 1 make[2]: *** Waiting for unfinished jobs.... ``` Kind regards, Henry > > Cheers, > > -- > Julien Grall
