[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 13.03.18 at 17:55, <maran.wilson@xxxxxxxxxx> wrote: > On 3/13/2018 9:34 AM, Jan Beulich wrote: >>>>> On 13.03.18 at 17:20, <maran.wilson@xxxxxxxxxx> wrote: >>> On 3/13/2018 3:50 AM, Roger Pau Monné wrote: >>>> On Fri, Mar 02, 2018 at 12:54:29PM -0800, Maran Wilson wrote: >>>>> @@ -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). >> But Roger makes a valid remark here. Statements like >> "E820_TYPE_xxx, for example" are simply to vague for a stable public >> interface. > > How about "For example, E820 types like E820_RAM, E820_ACPI, etc as > defined in xen/include/asm-x86/e820.h of the Xen tree" ? No, that's still "for example". You need to spell out (in the abstract part) and provide C constants (in the C implementation part) for the types currently permitted. Their 1:1 relationship with E820_* constants could/should then be documented with a couple of BUILD_BUG_ON()s. Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |