Re: [PATCH v7 8/8] xen/arm: mmu: move MMU specific P2M code to mmu/p2m.{c,h}

[Julien Grall <julien@xxxxxxx>]

On 14/10/2023 02:26, Henry Wang wrote:
> Hi Julien,
Hi Henry,

> > On Oct 14, 2023, at 02:22, Julien Grall <julien@xxxxxxx> wrote:
> >
> > Hi Henry,
> >
> > On 09/10/2023 02:03, Henry Wang wrote:
> > > From: Penny Zheng <penny.zheng@xxxxxxx>
> > > Current P2M implementation is designed for MMU system only.
> > > We move the MMU-specific codes into mmu/p2m.c, and only keep generic
> > > codes in p2m.c, like VMID allocator, etc. We also move MMU-specific
> > > definitions and declarations to mmu/p2m.h, such as p2m_tlb_flush_sync().
> > > Also expose previously static functions p2m_vmid_allocator_init(),
> > > p2m_alloc_vmid(), and setup_virt_paging_one() for further MPU usage.
> > > With the code movement, global variable max_vmid is used in multiple
> > > files instead of a single file (and will be used in MPU P2M
> > > implementation), declare it in the header and remove the "static" of
> > > this variable.
> > > Signed-off-by: Penny Zheng <penny.zheng@xxxxxxx>
> > > Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
> > > Signed-off-by: Henry Wang <Henry.Wang@xxxxxxx>
> > Some remarks about some of the code not moved:
> > * struct p2m_domain: The bulk of the fields seems to be MMU specific. So 
> > depending on the number of common fields we either want to split or move the 
> > structure to p2m_domain. I would be ok to wait until the MPU code is present.
> > * p2m_type_t: It is not yet clear how this will apply to the MPU. I am ok to 
> > wait before moving it.
> Agree with both here, let’s continue the discussion in the actual MPU patch for 
> P2M
> then, but I am then a bit confused about...
>
> > * p2m_cache_flush_range(): I expect the code will need some change because you 
> > may get large chunk of memory for the MPU.
> > * p2m_set_way_flush()/p2m_toggle_cache(): This was a giant hack to support 
> > cache flush operations via set/way. To make it efficient, we track the pages 
> > that have been touched and only flush them. For the MPU, this would not work. 
> > Can we attempt to not emulate the instructions?
> …these two remarks here, do you expect me to do some changes with these three
> functions in this patch? Or we can defer the required changes to the MPU patch 
> for
> P2M?
My original intention was to ask to move them right now. But if it is 
unclear whether they would be used, then it would be best to defer until 
we have a better understanding.
> I think I am highly likely to make a mistake here but I took a look at the MPU
> implementation [1] and it looks like the MPU code can use these tree functions
> without changes - probably because these functions are simply used by
> (1) domctl and we only have dom0less DomUs on MPU
> (2) trap handlers
> which means these functions are simply not called…
I am not sure I fully understand why would the trap handlers not called. 
Is this suggesting that a dom0less domUs can not use set/way instructions?
Cheers,

--
Julien Grall