[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



On 3/13/2018 3:50 AM, Roger Pau Monné wrote:
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.

Yes, that is being worked on as we speak. Should have a new set of patches out shortly.

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?

Sure, I'll do that.

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.

This field was discussed a good deal in v2 of the linux patches. I had originally defined this to be a specific type field, matching the x86/Linux definition for e820 memory mapping types. But Jan Beulich successfully argued that we should keep the definition of this particular interface agnostic to architecture and OS and not limit the field to specific values. I believe the central idea behind Jan's argument was to keep the interface x86-agnostic as well as preserving the option to add additional memory mapping types in the future without them being sanctioned by whoever maintains E820 type assignments.

That's why I changed the comment wording to what it is now. Basically spelling out the fact that this field simply needs to be agreed upon between the producer and the consumer since a hypervisor should generally know what type of guest it is starting. And I mentioned e820_type_xxx as the *example* of one such implementation, since that is the most obvious use case and the e820 types are part of the ACPI standard (and thus easy to find/reference).

+ * 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.

OK, will clean that up.

Thanks,
-Maran


Thanks, Roger.


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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.