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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.03.18 at 12:37, <roger.pau@xxxxxxxxxx> wrote:
> On Fri, Mar 16, 2018 at 05:29:27AM -0600, Jan Beulich wrote:
>> >>> On 16.03.18 at 12:11, <roger.pau@xxxxxxxxxx> wrote:
>> > On Thu, Mar 15, 2018 at 02:33:09PM -0700, Maran Wilson wrote:
>> >> @@ -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. Zero
>> >> + *    |                | if there is no memory map being provided. Only
>> > 
>> > Again (as I've mentioned in previous reviews) the way to signal a
>> > non-present memory map is to set memmap_paddr to 0, not memmap_entries
>> > to 0. This is already covered by the comment at the top of the header,
>> > which states:
>> > 
>> > NOTE: nothing will be loaded at physical address 0, so a 0 value in any
>> > of the address fields should be treated as not present.
>> 
>> I still think it is legitimate to direct consumers to look at the entry
>> count here.
> 
> We have another similar field tuple already, modlist_paddr and
> nr_modules and in that case (according to the current comments) the
> proper way to check if there are modules is to check modlist_paddr !=
> 0 and then get the count from nr_modules.

Well, that's the way it is now, as it's out in the wild already.

> Using this access strategy we avoid adding more comments about
> checking nr_modules != 0 before trying to access modlist_paddr.

I don't follow this argumentation: There is an obvious implication
that only nr_modules entries are valid to access at modlist_paddr.
If nr_modules is zero, no entry is valid to access. Nothing to be
said explicitly in the comment.

Jan


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