[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/18 17:57, Andrew Cooper wrote: 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:@@ -80,6 +81,18 @@ static int __init acpi_register_rmrr_unit(struct acpi_rmrr_unit *rmrr) return 0; } +/* Record RMRR mapping to ready expose VM. */ +static int __init rmrr_maps_register(struct acpi_rmrr_unit *rmrr) +{You absolutely need some protection against calling this function more than 128 times, or you need to do dynamic allocation of the storage.This together with ...--- 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.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 e820entry) 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(... } This is just what I mean. You always need to grab all info from acpi_rmrr_units to covert as a e820entry to copy. So I guess you guys mean we should avoid duplicating RMRR many times in global. acpi_rmrr_units is fine enough, so I'd like to do as you expect. Thanks Tiejun But with appropriate error checking. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |