On 3/21/2018 2:40 AM, Juergen Gross wrote:
On 21/03/18 10:28, Roger Pau Monné wrote:
On Tue, Mar 20, 2018 at 09:48:56AM -0700, Maran Wilson wrote:
+/*
* C representation of the x86/HVM start info layout.
*
* The canonical definition of this layout is above, this is just a way to
@@ -86,6 +134,14 @@ struct hvm_start_info {
uint64_t cmdline_paddr; /* Physical address of the command line.
*/
uint64_t rsdp_paddr; /* Physical address of the RSDP ACPI data
*/
/* structure.
*/
+ uint64_t memmap_paddr; /* Physical address of an array of */
+ /* hvm_memmap_table_entry. Only present in */
+ /* version 1 and newer of the structure */
+ uint32_t memmap_entries; /* Number of entries in the memmap table. */
+ /* Only present in version 1 and newer of */
+ /* the structure. Value will be zero if */
+ /* there is no memory map being provided. */
+ uint32_t reserved; /* Must be zero for Version 1. */
I would write "Must be zero." only. If at some point we introduce
version 2 we would likely have to fixup this comment to mention
version 1 and version 2.
In case you are going this route I'd suggest to drop the version remarks
for the individual fields and just add a comment like:
/* All following fields only present in version 1 and newer. */
above memmap_paddr.
OK, so combining the above suggestions, I'd have the following. Is the
formatting and alignment of comments what you had in mind and acceptable
to all?
struct hvm_start_info {
uint32_t magic; /* Contains the magic value
0x336ec578 */
/* ("xEn3" with the 0x80 bit of the "E"
set).*/
uint32_t version; /* Version of this
structure. */
uint32_t flags; /* SIF_xxx
flags. */
uint32_t nr_modules; /* Number of modules passed to the
kernel. */
uint64_t modlist_paddr; /* Physical address of an array
of */
/*
hvm_modlist_entry. */
uint64_t cmdline_paddr; /* Physical address of the command
line. */
uint64_t rsdp_paddr; /* Physical address of the RSDP ACPI
data */
/*
structure. */
/* All following fields only present in version 1 and newer */
uint64_t memmap_paddr; /* Physical address of an array
of */
/*
hvm_memmap_table_entry. */
uint32_t memmap_entries; /* Number of entries in the memmap
table. */
/* Value will be zero if there is no
memory */
/* map being
provided. */
uint32_t reserved; /* Must be
zero. */
};
Thanks,
-Maran
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
