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

[Wei Liu <wei.liu2@xxxxxxxxxx>]

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.

Wei.


> Jan

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