Re: [Xen-devel] [RFC 21/22] xen/arm: p2m: Re-implement p2m_set_mem_access using p2m_{set, get}_entry

[Julien Grall <julien.grall@xxxxxxx>]

Hi Razvan,

On 28/07/16 16:04, Razvan Cojocaru wrote:
> On 07/28/2016 05:51 PM, Julien Grall wrote:
> > The function p2m_set_mem_access can be re-implemented using the generic
> > functions p2m_get_entry and __p2m_set_entry.
> >
> > Note that because of the implementation of p2m_get_entry, a TLB
> > invalidation instruction will be issued for each 4KB page. Therefore the
> > performance of memaccess will be impacted, however the function is now
> > safe on all the processors.
> >
> > Also the function apply_p2m_changes is dropped completely as it is not
> > unused anymore.
> >
> > Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> > Cc: Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>
> > Cc: Tamas K Lengyel <tamas@xxxxxxxxxxxxx>
> >
> > ---
> >     I have not ran any performance test with memaccess for now, but I
> >     expect an important and unavoidable impact because of how memaccess
> >     has been designed to workaround hardware limitation. Note that might
> >     be possible to re-work memaccess work on superpage but this should
> >     be done in a separate patch.
> > ---
> >  xen/arch/arm/p2m.c | 329 +++++++----------------------------------------------
> >  1 file changed, 38 insertions(+), 291 deletions(-)
> Thanks for the CC!
>
> This seems to only impact ARM, are there any planned changes for x86
> along these lines as well?
Actually, it might be possible to remove the TLB for each 4KB entry in 
the memaccess case.
After I read again multiple time the ARM ARM (D4-1732 in ARM DDI 
0487A.i) and spoke with various ARM folks, changing the permission (i.e 
read, write, execute) does not require the break-before-make sequence.
However, I noticed a latent bug in the memaccess code when the 
permission restriction are removed/changed.
In the current implementation (i.e without this series), the TLB 
invalidation is deferred until the p2m is released. Until that time, a 
vCPU may still run with the previous permission and trap into the 
hypervisor the permission fault. However, as the permission as changed, 
p2m_memaccess_check may fail and we will inject an abort to the guest.
The problem is very similar to [1]. This will still be true with this 
series applied if I relaxed the use of the break-before-make sequence. 
The two ways I can see to fix this are either try again the instruction 
(we would trap again if the permission was not correct) or keep the 
break-before-make.
The former will be cleaner given than stage-2 permission fault can only 
happen because of memaccess for now. Any opinions?
Regards,

[1] 
https://lists.xenproject.org/archives/html/xen-devel/2016-07/msg03133.html
--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel