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

[Julien Grall <julien@xxxxxxx>]

Hi,

On 23/08/2023 19:08, Julien Grall wrote:
> > > > 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.
> > > > Add #ifdef CONFIG_HAS_MMU to p2m_write_unlock() since future MPU
> > > > work does not need p2m_tlb_flush_sync().
> > > And there are no specific barrier required? Overall, I am not sure I 
> > > like the #ifdef rather than providing a stub helper.
> > I think for MPU systems we don’t need to flush the TLB, hence the #ifdef.
> I wasn't necessarily thinking about a TLB flush but instead a DSB/DMB. 
> At least for the MMU case, I think that in theory we need a DSB when the 
> there is no TLB flush to ensure new entry in the page-tables are seen 
> before p2m_write_unlock() completes.
> So far we are getting away because write_pte() always have a barrier 
> after. But at some point, I would like to remove it as this is a massive 
> hammer.
> > Do you mean we should
> > provide a stub helper of p2m_tlb_flush_sync() for MPU? If so I think 
> > maybe the naming of this stub
> > helper is not really ideal?
> See above. I am trying to understand the expected sequence when updating 
> the MPU tables. Are you going to add barriers after every update to the 
> entry?
I have looked at your branch mpu_v5. In theory the P2M can be modified 
at any point of the life-cycle of the domain. This means that another 
pCPU may have the regions loaded.
If that's the case then you would likely want to ensure the entries are 
synchronized. The easiest way would be to pause/unpause the domain when 
taking/releasing the lock. There might be other way, but this indicates 
that helper would be needed for the MPU.
That said, it is not clear to me if there is any use-case where you 
would want to update the P2M at runtime. If you have known, then you 
might be able to simply return an error if the P2M is modified after the 
domain was created.
Cheers,

--
Julien Grall