Re: [vNUMA v2][PATCH 2/8] public interface

[Andre Przywara <andre.przywara@amd.com>]

Dulloor wrote:
> Interface definition. Structure that will be shared with hvmloader (with HVMs)
> and directly with the VMs (with PV).
> 
> -dulloor
> 
> Signed-off-by : Dulloor <dulloor@gmail.com>
> 

> +/* vnodes are 1GB-aligned */
> +#define XEN_MIN_VNODE_SHIFT (30)
Why that? Do you mean guest memory here? Isn't that a bit restrictive?
What if the remaining system resources do not allow this?
What about a 5GB guest on 2 nodes?
In AMD hardware there is minimum shift of 16MB, so I think 24 bit would
be better.

+struct xen_vnode_info {
+    uint8_t mnode_id; 	/* physical node vnode is allocated from */
+    uint32_t start; 	/* start of the vnode range (in pages) */
+    uint32_t end; 		/* end of the vnode range (in pages) */
+};
+
> +struct xen_domain_numa_info {
> +    uint8_t version;    /* Interface version */
> +    uint8_t type;       /* VM memory allocation scheme (see above) */
> +
> +    uint8_t nr_vcpus;
Isn't that redundant with info stored somewhere else (for instance
in the hvm_info table)?
> +    uint8_t nr_vnodes;
> +    /* data[] has the following entries :
> +     * //Only (nr_vnodes) entries are filled, each sizeof(struct xen_vnode_info)
> +     * struct xen_vnode_info vnode_info[nr_vnodes]; 
Why would the guest need that info (physical node, start and end) here?
Wouldn't be just the size of the node's memory sufficient?

Regards,
Andre.
> +     * //Only (nr_vcpus) entries are filled, each sizeof(uint8_t)
> +     * uint8_t vcpu_to_vnode[nr_vcpus];
> +     * //Only (nr_vnodes*nr_vnodes) entries are filled, each sizeof(uint8_t)
> +     * uint8_t vnode_distance[nr_vnodes*nr_vnodes];
> +     */
> +	uint8_t data[0];
> +};
> +
> +#endif

-- 
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 448-3567-12