Re: [Xen-devel] [PATCH v2 12/19] hvmloader: retrieve vNUMA information from hypervisor

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Tue, Dec 09, 2014 at 04:46:22PM +0000, Jan Beulich wrote:
> >>> On 01.12.14 at 16:33, <wei.liu2@xxxxxxxxxx> wrote:
> > @@ -261,6 +262,8 @@ int main(void)
> >  
> >      init_hypercalls();
> >  
> > +    init_vnuma_info();
> 
> This is very early, and I don't think it needs to be - I guess it could be
> done right before doing ACPI stuff?
> 

Yes, it can be moved right before ACPI stuff.

> > --- /dev/null
> > +++ b/tools/firmware/hvmloader/vnuma.c
> > @@ -0,0 +1,84 @@
> > +/*
> > + * vnuma.c: obtain vNUMA information from hypervisor
> > + *
> > + * Copyright (c) 2014 Wei Liu, Citrix Systems (R&D) Ltd.
> > + *
> > + * Redistribution and use in source and binary forms, with or without
> > + * modification, are permitted provided that the following conditions
> > + * are met:
> > + * 1. Redistributions of source code must retain the above copyright
> > + *    notice, this list of conditions and the following disclaimer.
> > + * 2. Redistributions in binary form must reproduce the above copyright
> > + *    notice, this list of conditions and the following disclaimer in the
> > + *    documentation and/or other materials provided with the distribution.
> > + *
> > + * THIS SOFTWARE IS PROVIDED BY AUTHOR AND CONTRIBUTORS ``AS IS'' AND
> > + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> > + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR 
> > PURPOSE
> > + * ARE DISCLAIMED.  IN NO EVENT SHALL AUTHOR OR CONTRIBUTORS BE LIABLE
> > + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR 
> > CONSEQUENTIAL
> > + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
> > + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> > + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, 
> > STRICT
> > + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY 
> > WAY
> > + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> > + * SUCH DAMAGE.
> > + */
> > +
> > +#include "util.h"
> > +#include "hypercall.h"
> > +#include "vnuma.h"
> > +#include <errno.h>
> 
> This is the system header, not the Xen one. See the discussion at
> http://lists.xenproject.org/archives/html/xen-devel/2014-10/msg03206.html
> and perhaps build on the resulting patch
> http://lists.xenproject.org/archives/html/xen-devel/2014-12/msg00013.html.
> 

I will cherry pick that patch and build on top of it.

> > +
> > +unsigned int nr_vnodes, nr_vmemranges;
> > +unsigned int *vcpu_to_vnode, *vdistance;
> > +xen_vmemrange_t *vmemrange;
> > +
[...]
> > + */
> > +
> > +#ifndef __HVMLOADER_VNUMA_H__
> > +#define __HVMLOADER_VNUMA_H__
> > +
> > +#include <xen/memory.h>
> > +
> > +#define MAX_VNODES     64
> > +#define MAX_VDISTANCE  (MAX_VNODES * MAX_VNODES)
> > +#define MAX_VMEMRANGES (MAX_VNODES * 2)
> 
> These look arbitrary - please add a (brief) comment giving some
> rationale so that people needing to change them know eventual
> consequences. Would it perhaps make sense to derive
> MAX_VNODES from HVM_MAX_VCPUS? Additionally I think the

I don't think these two have very strong connection though.

And I remember you saying HVM_MAX_VCPUS will be removed.

> code wouldn't become much more difficult if you didn't build in
> static upper limits.
> 

Yes I can make two hypercalls. First one to retrieve the number of nodes
/ vmemranges configured, allocate memory then fill in those arrays with
second hypercall.

Wei.

> Jan

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