[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [RFC][v3][PATCH 1/6] xen:x86: record RMRR mappings
On 2014/8/19 10:14, Chen, Tiejun wrote: On 2014/8/18 20:31, Jan Beulich wrote:Andrew Cooper <andrew.cooper3@xxxxxxxxxx> 08/18/14 11:57 AM >>>On 18/08/14 08:42, Chen, Tiejun wrote:On 2014/8/16 0:29, Jan Beulich wrote:On 15.08.14 at 11:39, <andrew.cooper3@xxxxxxxxxx> wrote:--- a/xen/include/asm-x86/e820.h +++ b/xen/include/asm-x86/e820.h @@ -23,6 +23,8 @@ struct e820map { struct e820entry map[E820MAX]; }; +typedef struct e820map rmrr_maps_t;This type is a single map of RMRR regions, not multiple maps. rmrr_map_t please.... this once again stresses what I stated previously: Piggybacking on the E820 handling here is just the wrong approach. There's really no correlation with E820 other than us wanting to use the gathered information for (among other things) adjusting the guest E820 table. But that doesn't in any way require any re-use of non-suitable data structures.Why are you saying this is not suitable? We need a structure to represent a RMRR entry including three fields, start, size and type, and especially, essentially RMRR entry belongs to e820 table as one entry.Not in Xen. Only as reported to guests, in which case an e820-like structure is most appropriate.E280-like yes, but ...In fact I don't see the need for this first patch anyway, as RMRRs are already being put on a linked list as they get found. I.e. theYes, that list, acpi_rmrr_unit, can be exposed here. But before you copy to guest, don't you need to grab those fields from that list then convert them as a suitable structure (mostly this is still same as ) to be copied into a buffer?Yes, but the hypercall handler can do this which avoids all need to store an intermediate representation in Xen. list_for_each_entry(rmrr, &acpi_rmrr_units, list) {>e820entry e;>e.start = ...>copy_to_guest_offset(...}... as said before, I don't think using the E820 structure as-is is the right approach: Neither do we need byte-granular fields, nor do we need a type here.Please don't say simply that e820entry is not suitable, what's your preferred structure here? Looks you are saying something like, struct __packed rmrr_entry { uint64_t addr; uint64_t size; }; but compare that to the existing e820entry, struct __packed e820entry { uint64_t addr; uint64_t size; uint32_t type; }; Another concern is that we always use xen_memory_map for the hypercall, struct xen_memory_map { /* * On call the number of entries which can be stored in buffer. On * return the number of entries which have been stored in * buffer. */ unsigned int nr_entries; /* * Entries in the buffer are in the same format as returned by the * BIOS INT 0x15 EAX=0xE820 call. */ XEN_GUEST_HANDLE(void) buffer; }; As it comments above, theoretical e820 is expected in buffer. Thanks Tiejun Anyway, please show me your ideal structure then I'd like to follow-up that since it's no big deal. Thanks Tiejun _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |