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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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)?

Jan


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