Re: [PATCH v2 2/2] Xen: Document the semantic of the pagetable_reserve PVOPS

[David Vrabel <david.vrabel@citrix.com>]

On 14/08/12 13:24, Attilio Rao wrote:
> The informations added on the hook are:
> - Native behaviour
> - Xen specific behaviour
> - Logic behind the Xen specific behaviour

These are implementation details and should be documented with the
implementations (if necessary).

> - PVOPS semantic

This is the interesting stuff.

This particular pvop seems a little odd really.  It might make more
sense if it took a third parameter for pgt_buf_top.

"@pagetable_reserve is used to reserve a range of PFNs used for the
kernel direct mapping page tables and cleans-up any PFNs that ended up
not being used for the tables.

It shall reserve the range (start, end] with memblock_reserve(). It
shall prepare PFNs in the range (end, pgt_buf_top] for general (non page
table) use.

It shall only be called in init_memory_mapping() after the direct
mapping tables have been constructed."

Having said that, I couldn't immediately see where pages in (end,
pgt_buf_top] was getting set RO.  Can you point me to where it's done?

David

> Signed-off-by: Attilio Rao <attilio.rao@citrix.com>
> ---
>  arch/x86/include/asm/x86_init.h |   19 +++++++++++++++++--
>  1 files changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/include/asm/x86_init.h b/arch/x86/include/asm/x86_init.h
> index 38155f6..b22093c 100644
> --- a/arch/x86/include/asm/x86_init.h
> +++ b/arch/x86/include/asm/x86_init.h
> @@ -72,8 +72,23 @@ struct x86_init_oem {
>   * struct x86_init_mapping - platform specific initial kernel pagetable setup
>   * @pagetable_reserve:	reserve a range of addresses for kernel pagetable usage
>   *
> - * For more details on the purpose of this hook, look in
> - * init_memory_mapping and the commit that added it.
> + * It does reserve a range of pages, to be used as pagetable pages.
> + * The start and end parameters are expected to be contained in the
> + * [pgt_buf_start, pgt_buf_top] range.
> + * The native implementation reserves the pages via the memblock_reserve()
> + * interface.
> + * The Xen implementation, besides reserving the range via memblock_reserve(),
> + * also sets RW the remaining pages contained in the ranges
> + * [pgt_buf_start, start) and [end, pgt_buf_top).
> + * This is needed because the range [pgt_buf_start, pgt_buf_top] was
> + * previously mapped read-only by xen_set_pte_init: when running
> + * on Xen all the pagetable pages need to be mapped read-only in order to
> + * avoid protection faults from the hypervisor. However, once the correct
> + * amount of pages is reserved for the pagetables, all the others contained
> + * in the range must be set to RW so that they can be correctly recycled by
> + * the VM subsystem.
> + * This operation is meant to be performed only during init_memory_mapping(),
> + * just after space for the kernel direct mapping tables is found.
>   */
>  struct x86_init_mapping {
>  	void (*pagetable_reserve)(u64 start, u64 end);