Re: [Xen-devel] [RFC][v3][PATCH 1/6] xen:x86: record RMRR mappings

["Chen, Tiejun" <tiejun.chen@xxxxxxxxx>]

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.
> 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?
Thanks
Tiejun

> patch here just clones existing information.
>
> Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel