From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Chen, Tiejun" Subject: Re: [RFC][v3][PATCH 1/6] xen:x86: record RMRR mappings Date: Tue, 19 Aug 2014 10:28:05 +0800 Message-ID: <53F2B635.9030208@intel.com> References: <1408091238-18364-1-git-send-email-tiejun.chen@intel.com> <1408091238-18364-2-git-send-email-tiejun.chen@intel.com> <53EDD539.90403@citrix.com> <53EE436502000078000BA911@mail.emea.novell.com> <53F1AE4C.5050002@intel.com> <53F1CE0D.7080005@citrix.com> <53F2003B02000078000BAAC3@mail.emea.novell.com> <53F2B313.5010408@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <53F2B313.5010408@intel.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich , andrew.cooper3@citrix.com Cc: kevin.tian@intel.com, ian.campbell@citrix.com, stefano.stabellini@eu.citrix.com, ian.jackson@eu.citrix.com, xen-devel@lists.xen.org, yang.z.zhang@intel.com List-Id: xen-devel@lists.xenproject.org On 2014/8/19 10:14, Chen, Tiejun wrote: > On 2014/8/18 20:31, Jan Beulich wrote: >>>>>> Andrew Cooper 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, 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 >>>> ) 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@lists.xen.org > http://lists.xen.org/xen-devel > >