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

[Julien Grall <julien@xxxxxxx>]

On 07/11/2022 09:57, Penny Zheng wrote:
> Hi Julien
Hi Penny,

> > -----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).
Cheers,

[1] 20221022150422.17707-1-julien@xxxxxxx

--
Julien Grall