[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



>>>> 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. the
>>
>> Yes, 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(...
>}

... 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.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.