Re: [Xen-devel] [PATCH 11/19] hvmloader: add new fields for vNUMA information

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 24.11.14 at 11:07, <wei.liu2@xxxxxxxxxx> wrote:
> On Mon, Nov 24, 2014 at 09:58:29AM +0000, Jan Beulich wrote:
>> >>> On 21.11.14 at 16:06, <wei.liu2@xxxxxxxxxx> wrote:
>> > --- a/xen/include/public/hvm/hvm_info_table.h
>> > +++ b/xen/include/public/hvm/hvm_info_table.h
>> > @@ -32,6 +32,17 @@
>> >  /* Maximum we can support with current vLAPIC ID mapping. */
>> >  #define HVM_MAX_VCPUS        128
>> >  
>> > +#define HVM_MAX_NODES         16
>> > +#define HVM_MAX_LOCALITIES    (HVM_MAX_NODES * HVM_MAX_NODES)
>> > +
>> > +#define HVM_MAX_VMEMRANGES    64
>> > +struct hvm_info_vmemrange {
>> > +    uint64_t start;
>> > +    uint64_t end;
>> > +    uint32_t flags;
>> > +    uint32_t nid;
>> > +};
>> > +
>> >  struct hvm_info_table {
>> >      char        signature[8]; /* "HVM INFO" */
>> >      uint32_t    length;
>> > @@ -67,6 +78,14 @@ struct hvm_info_table {
>> >  
>> >      /* Bitmap of which CPUs are online at boot time. */
>> >      uint8_t     vcpu_online[(HVM_MAX_VCPUS + 7)/8];
>> > +
>> > +    /* Virtual NUMA information */
>> > +    uint32_t    nr_nodes;
>> > +    uint8_t     vcpu_to_vnode[HVM_MAX_VCPUS];
>> > +    uint32_t    nr_vmemranges;
>> > +    struct hvm_info_vmemrange vmemranges[HVM_MAX_VMEMRANGES];
>> > +    uint64_t    nr_localities;
>> > +    uint8_t     localities[HVM_MAX_LOCALITIES];
>> >  };
>> >  
>> >  #endif /* __XEN_PUBLIC_HVM_HVM_INFO_TABLE_H__ */
>> 
>> Is this really the right place? This is a public interface, which we
>> shouldn't modify in ways making future changes more cumbersome.
>> In particular, once we finally get the LAPIC ID brokenness fixed,
>> HVM_MAX_VCPUS won't need to be limited to 128 anymore. And
>> we likely would want to keep things simple an retain the bitmap
>> where it currently sits, just extending its size. With all of the data
>> above (supposedly, or we made a mistake somewhere) being
>> retrievable via hypercall, what is the rationale for doing things
>> this way in the first place (the lack of any kind of description is of
>> course not really helpful here)?
>> 
> 
> My thought was that this is interface between libxl and hvmloader, and
> the way it's used suggests that this is canonical way of doing things. I
> might have got this wrong in the first place.
> 
> So I take it you're of the opinion this piece of information should be
> retrieved via hypercall in hvmloader, right? That's OK (and even better)
> for me.

Yes - that other interface should be used only for things that
can't be communicated to the guest in another (sane) way.

Jan


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