Re: [PATCH 4/7] xen/arm: page: Consolidate write_pte() and clarify the documentation

[Julien Grall <julien@xxxxxxx>]

Hi Bertrand,

On 04/07/2023 16:06, Bertrand Marquis wrote:
> > On 19 Jun 2023, at 19:01, Julien Grall <julien@xxxxxxx> wrote:
> >
> > From: Julien Grall <jgrall@xxxxxxxxxx>
> >
> > The implementation of write_pte() is pretty much the same on arm32
> > and arm64. So it would be good to consolidate it as this would help
> > to clarify the requirements when using the helper.
> >
> > Take the opportunity to switch from assembly to call macros. Note there
> > is a difference on arm32 because the option was not specified. But this
> > meant 'sy' (system wide).
> >
> > Futhermore, the requirements for the ISB is incomplete. Per the Arm Arm,
> > (Armv7 DDI406C.d A3.8.3 and Armv8 DDI 0487J.a B2.3.12), DSB will only
> > affect explicit accesses. So an ISB is necessary after DSB to ensure
> > the completion. Having an ISB after each update to the page-tables is
> > probably too much, so let the caller add the instruction when it is
> > convenient.
> >
> > Lastly, the barrier in write_pte() may be too restrictive but I haven't
> > yet find the proper section(s) in the Arm Arm.
> I do not think we will find any proper section for that in the Arm Arm as it
> depends on use cases.
> In ours we might have the page table shared among cores so I think system
> is the way to go.
For SMP systems, like Linux, Xen is assuming that all the cores are in 
the same inner-shareable domain. So 'ish' would be sufficient based on 
just that.
However, I haven't made the change because I am not sure about the 
interaction when the modified entry modified to device memory and we 
have an access after/before.
> > Signed-off-by: Julien Grall <jgrall@xxxxxxxxxx>
> Reviewed-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>
Thanks!

Cheers,

--
Julien Grall