Re: [Xen-devel] [PATCH 1/1] x86/PVHv2: Add memory map pointer to hvm_start_info struct

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Fri, Mar 02, 2018 at 12:54:29PM -0800, Maran Wilson wrote:
> The start info structure that is defined as part of the x86/HVM direct boot
> ABI and used for starting Xen PVH guests would be more versatile if it also
> included a way to pass information about the memory map to the guest. This
> would allow KVM guests to share the same entry point.

I would also like to see Xen modified to make use of this new
memmap_paddr feature. See bootlate_hvm in tools/libxc/xc_dom_x86.c, it
should be quite trivial to add the memmap to the hvm_start_info
crafted there.

> Signed-off-by: Maran Wilson <maran.wilson@xxxxxxxxxx>
> ---
>  xen/include/public/arch-x86/hvm/start_info.h | 51 
> +++++++++++++++++++++++++++-
>  1 file changed, 50 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/include/public/arch-x86/hvm/start_info.h 
> b/xen/include/public/arch-x86/hvm/start_info.h
> index 6484159..ae8dac8 100644
> --- a/xen/include/public/arch-x86/hvm/start_info.h
> +++ b/xen/include/public/arch-x86/hvm/start_info.h
> @@ -33,8 +33,9 @@
>   *    | magic          | Contains the magic value XEN_HVM_START_MAGIC_VALUE
>   *    |                | ("xEn3" with the 0x80 bit of the "E" set).
>   *  4 +----------------+
> - *    | version        | Version of this structure. Current version is 0. New
> + *    | version        | Version of this structure. Current version is 1. New
>   *    |                | versions are guaranteed to be backwards-compatible.
> + *    |                | For PV guests only 0 allowed, for PVH 0 or 1 
> allowed.
>   *  8 +----------------+
>   *    | flags          | SIF_xxx flags.
>   * 12 +----------------+
> @@ -48,6 +49,15 @@
>   * 32 +----------------+
>   *    | rsdp_paddr     | Physical address of the RSDP ACPI data structure.
>   * 40 +----------------+
> + *    | memmap_paddr   | Physical address of the (optional) memory map. Only
> + *    |                | present in version 1 and newer of the structure.
> + * 48 +----------------+
> + *    | memmap_entries | Number of entries in the memory map table. Only
> + *    |                | present in version 1 and newer of the structure.
> + *    |                | Zero if there is no memory map being provided.

Can you place the "present in version 1 and newer" at the end of the
text block?

IMHO setting memmap_paddr to 0 should be the way to signal that
there's no memory map (like it's done for rsdp_paddr), and then the
value of _entries is irrelevant. At which point the "Zero if there is
no memory map being provided" is wrong.

> + * 52 +----------------+
> + *    | reserved       | Version 1 and newer only.
> + * 56 +----------------+
>   *
>   * The layout of each entry in the module structure is the following:
>   *
> @@ -62,10 +72,34 @@
>   *    | reserved       |
>   * 32 +----------------+
>   *
> + * The layout of each entry in the memory map table is as follows:
> + *
> + *  0 +----------------+
> + *    | addr           | Base address
> + *  8 +----------------+
> + *    | size           | Size of mapping in bytes
> + * 16 +----------------+
> + *    | type           | Type of mapping as defined between the hypervisor
> + *    |                | and guest it's starting. E820_TYPE_xxx, for example.

This needs a link to the expected type values (or a reference). Or you
need to spell out the relation between the values and the memory types.

> + * 20 +----------------|
> + *    | reserved       |
> + * 24 +----------------+
> + *
>   * The address and sizes are always a 64bit little endian unsigned integer.
>   *
>   * NB: Xen on x86 will always try to place all the data below the 4GiB
>   * boundary.
> + *
> + * Version numbers of the hvm_start_info structure have evolved like this:
> + *
> + * Version 0:
> + *
> + * Version 1:        Added the memmap_paddr/memmap_entries fields (plus 4 
> bytes of
> + *           padding) to the end of the hvm_start_info struct. These new
> + *           fields can be used to pass a memory map to the guest. The
> + *           memory map is optional and so guests that understand version 1
> + *           of the structure must check that memmap_entries is non-zero
> + *           before trying to read the memory map.

No hard tabs please.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel