RE: [PATCH v6 10/11] xen/arm64: introduce helpers for MPU enable/disable

[Penny Zheng <Penny.Zheng@xxxxxxx>]

> -----Original Message-----
> From: Julien Grall <julien@xxxxxxx>
> Sent: Monday, November 7, 2022 6:38 PM
> To: Penny Zheng <Penny.Zheng@xxxxxxx>; Wei Chen
> <Wei.Chen@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: nd <nd@xxxxxxx>; Stefano Stabellini <sstabellini@xxxxxxxxxx>; Bertrand
> Marquis <Bertrand.Marquis@xxxxxxx>; Volodymyr Babchuk
> <Volodymyr_Babchuk@xxxxxxxx>
> Subject: Re: [PATCH v6 10/11] xen/arm64: introduce helpers for MPU
> enable/disable
> 
> 
> 
> On 07/11/2022 09:57, Penny Zheng wrote:
> > Hi Julien
> 
> Hi Penny,

Hi Julien

> 
> >
> >> -----Original Message-----
> >> From: Xen-devel <xen-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf Of
> >> Julien Grall
> >> Sent: Monday, November 7, 2022 4:56 AM
> >> To: Wei Chen <Wei.Chen@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx
> >> Cc: nd <nd@xxxxxxx>; Penny Zheng <Penny.Zheng@xxxxxxx>; Stefano
> >> Stabellini <sstabellini@xxxxxxxxxx>; Bertrand Marquis
> >> <Bertrand.Marquis@xxxxxxx>; Volodymyr Babchuk
> >> <Volodymyr_Babchuk@xxxxxxxx>
> >> Subject: Re: [PATCH v6 10/11] xen/arm64: introduce helpers for MPU
> >> enable/disable
> >>
> >> Hi Wei,
> >>
> >> On 04/11/2022 10:07, Wei Chen wrote:
> >>> From: Penny Zheng <penny.zheng@xxxxxxx>
> >>>
> >>> We need some helpers for Xen to enable/disable MPU in boot-time and
> >>> runtime. For MPU enable helper, we know that it's an essential
> >>> requirement of MPU system. But for MPU disable, we need to use it
> >>> for some special situations. For example, in the progress of
> >>> tranferring from boot-time to runtime, we need to update the MPU
> >>> protection regions configuration, but we can't modify an MPU
> >>> protection region if there is some data accessed by Xen. But in
> >>> boot-time all of Xen text, data and BSS are in one MPU protection
> >>> region, if Xen want to update this protection region, above restriction 
> >>> will
> be triggered.
> >>
> >> This raises the following question: Why can't we create the split
> >> regions right now?
> >>
> >
> > The reason why we are not creating the split regions right now is that
> > we are trying to go the same path MMU goes.
> 
> The MMU code is going to change pretty soon (see [1] for some ground
> work). The runtime page-tables for CPU0 will be created in assembly code
> and never switched after (aside when using cache coloring).
> 
> Although, I don't think I will apply the proper permissions in assembly (this 
> is
> a bit trickier than with the MPU).
> 
> > Then we could reuse as much
> > same interfaces as we could, in order to not leave #ifdef
> > CONFIG_HAS_MPU all over the place.
> Do you have a list of those interfaces that would require #ifdef?
> 
> >
> >> In particular, disabling the MMU/Cache is fairly risky because you
> >> need to ensure that anything in the cache you care about have been
> >> written back to the RAM).
> >>
> >
> > Hope I could understand your concern totally, you are worrying about
> > stale info left in the cache, even if it's always 1:1 mapping on the
> > MPU system, memory attributes could be different before and after?
> 
> No. I am more concerned about...
> 
> > So it is never enough that we only flush the variables which we will
> > use during the disabling time, it should be everything in the
> > cache...:/
> 
> ... this. We don't only need to flush before they are accessed but also after 
> if
> they are modified.
> 
> It is possible to do it correctly, but it requires to be very careful.
> So if we can avoid disabling the cache/MPU then it will be a lot better.
> 
> >
> > Since in current design, there are two time points in boot time where
> > we will disable MPU/Cache to configure MPU.
> >
> > One is in setup_mm, here, we will map XEN components by components,
> > each MPU memory region for a different component.
> > The other is near the end of boot time, we will reorg the whole MPU
> > memory region layout before going runtime, and we will keep unchanging
> regions in the front and flexible ones in the rear.
> 
> You should not need any reorg if you map the boot-only section towards in
> the higher slot index (or just after the fixed ones).
> 

"in the higher slot index" is really shining a light in my mind ;) And I'll try 
to enable it
in v2.

> Cheers,
> 
> [1] 20221022150422.17707-1-julien@xxxxxxx
> 
> --
> Julien Grall