Re: [Xen-devel] [RFC 00/22] xen/arm: Rework the P2M code to follow break-before-make sequence

["Edgar E. Iglesias" <edgar.iglesias@xxxxxxxxxx>]

On Thu, Jul 28, 2016 at 03:51:23PM +0100, Julien Grall wrote:
> Hello all,
> 
> The ARM architecture mandates the use of a break-before-make sequence when
> changing translation entries if the page table is shared between multiple
> CPUs whenever a valid entry is replaced by another valid entry (see D4.7.1
> in ARM DDI 0487A.j for more details).
> 
> The current P2M code does not respect this sequence and may result to
> break coherency on some processors.
> 
> Adapting the current implementation to use break-before-make sequence would
> imply some code duplication and more TLBs invalidations than necessary.
> For instance, if we are replacing a 4KB page and the current mapping in
> the P2M is using a 1GB superpage, the following steps will happen:
>     1) Shatter the 1GB superpage into a series of 2MB superpages
>     2) Shatter the 2MB superpage into a series of 4KB superpages
>     3) Replace the 4KB page
> 
> As the current implementation is shattering while descending and install
> the mapping before continuing to the next level, Xen would need to issue 3
> TLB invalidation instructions which is clearly inefficient.
> 
> Furthermore, all the operations which modify the page table are using the
> same skeleton. It is more complicated to maintain different code paths than
> having a generic function that set an entry and take care of the break-before-
> make sequence.
> 
> The new implementation is based on the x86 EPT one which, I think, fits
> quite well for the break-before-make sequence whilst keeping the code
> simple.
> 
> I sent this patch series as an RFC because there are still some TODOs
> in the code (mostly sanity check and possible optimization) and I have
> done limited testing. However, I think it is a good shape to start reviewing,
> get more feedback and have wider testing on different board.
> 
> Also, I need to figure out the impact on ARM32 because the domheap is not
> always mapped.
> 
> This series has dependencies on some rework sent separately ([1] and [2]).
> I have provided a branch with all the dependencies and this series applied:
> 
> git://xenbits.xen.org/people/julieng/xen-unstable.git branch p2m-rfc


Hi Julien,

FWIW, I gave this a spin on the ZynqMP and it seems to be working fine.
I tried dom0 and starting a few additional guests. All looks good.

Tested-by: Edgar E. Iglesias <edgar.iglesias@xxxxxxxxxx>

Cheers,
Edgar


> 
> Comments are welcome.
> 
> Yours sincerely,
> 
> Cc: Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>
> Cc: Tamas K Lengyel <tamas@xxxxxxxxxxxxx>
> Cc: Shanker Donthineni <shankerd@xxxxxxxxxxxxxx>
> Cc: Dirk Behme <dirk.behme@xxxxxxxxxxxx>
> Cc: Edgar E. Iglesias <edgar.iglesias@xxxxxxxxxx>
> 
> [1] https://lists.xenproject.org/archives/html/xen-devel/2016-07/msg02936.html
> [2] https://lists.xenproject.org/archives/html/xen-devel/2016-07/msg02830.html
> 
> Julien Grall (22):
>   xen/arm: do_trap_instr_abort_guest: Move the IPA computation out of
>     the switch
>   xen/arm: p2m: Store in p2m_domain whether we need to clean the entry
>   xen/arm: p2m: Rename parameter in p2m_{remove,write}_pte...
>   xen/arm: p2m: Use typesafe gfn in p2m_mem_access_radix_set
>   xen/arm: traps: Move MMIO emulation code in a separate helper
>   xen/arm: traps: Check the P2M before injecting a data/instruction
>     abort
>   xen/arm: p2m: Rework p2m_put_l3_page
>   xen/arm: p2m: Invalidate the TLBs when write unlocking the p2m
>   xen/arm: p2m: Change the type of level_shifts from paddr_t to unsigned
>     int
>   xen/arm: p2m: Move the lookup helpers at the top of the file
>   xen/arm: p2m: Introduce p2m_get_root_pointer and use it in
>     __p2m_lookup
>   xen/arm: p2m: Introduce p2m_get_entry and use it to implement
>     __p2m_lookup
>   xen/arm: p2m: Replace all usage of __p2m_lookup with p2m_get_entry
>   xen/arm: p2m: Re-implement p2m_cache_flush using p2m_get_entry
>   xen/arm: p2m: Re-implement relinquish_p2m_mapping using p2m_get_entry
>   xen/arm: p2m: Make p2m_{valid,table,mapping} helpers inline
>   xen/arm: p2m: Introduce a helper to check if an entry is a superpage
>   xen/arm: p2m: Introduce p2m_set_entry and __p2m_set_entry
>   xen/arm: p2m: Re-implement p2m_remove_using using p2m_set_entry
>   xen/arm: p2m: Re-implement p2m_insert_mapping using p2m_set_entry
>   xen/arm: p2m: Re-implement p2m_set_mem_access using
>     p2m_{set,get}_entry
>   xen/arm: p2m: Do not handle shattering in p2m_create_table
> 
>  xen/arch/arm/domain.c      |    8 +-
>  xen/arch/arm/p2m.c         | 1274 
> ++++++++++++++++++++++----------------------
>  xen/arch/arm/traps.c       |  126 +++--
>  xen/include/asm-arm/p2m.h  |   14 +
>  xen/include/asm-arm/page.h |    4 +
>  5 files changed, 742 insertions(+), 684 deletions(-)
> 
> -- 
> 1.9.1
> 

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