Re: [Xen-devel] [PATCH RFC 18/20] libxc/acpi: Build ACPI tables for HVMlite guests

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Mon, Jun 06, 2016 at 11:15:22AM -0400, Boris Ostrovsky wrote:
[...]
> > +static int init_acpi_config(struct xc_dom_image *dom,
> > +                            struct acpi_config *config)
> > +{
> > +    xc_interface *xch = dom->xch;
> > +    uint32_t domid = dom->guest_domid;
> > +    xc_dominfo_t info;
> > +    int i, rc;
> > +
> > +    memset(config, 0, sizeof(*config));
> > +
> > +    config->dsdt_anycpu = config->dsdt_15cpu = dsdt_empty;
> > +    config->dsdt_anycpu_len = config->dsdt_15cpu_len = dsdt_empty_len;
> > +
> > +    rc = xc_domain_getinfo(xch, domid, 1, &info);
> > +    if ( rc < 0 )
> > +    {
> > +        DOMPRINTF("%s: getdomaininfo failed (rc=%d)", __FUNCTION__, rc);
> > +        return rc;
> > +    }
> > +
> > +    config->apic_mode = 1;
> > +
> > +    if ( dom->nr_vnodes )
> > +    {
> > +        struct acpi_numa *numa = &config->numa;
> > +
> > +        numa->vmemrange = calloc(dom->nr_vmemranges,
> > +                                 sizeof(*numa->vmemrange));
> > +        numa->vdistance = calloc(dom->nr_vnodes,
> > +                                 sizeof(*numa->vdistance));
> > +        numa->vcpu_to_vnode = calloc(config->nr_vcpus,
> > +                                     sizeof(*numa->vcpu_to_vnode));
> > +        if ( !numa->vmemrange || !numa->vdistance || !numa->vcpu_to_vnode )
> > +        {
> > +            DOMPRINTF("%s: Out of memory", __FUNCTION__);
> > +            free(numa->vmemrange);
> > +            free(numa->vdistance);
> > +            free(numa->vcpu_to_vnode);
> > +            return -ENOMEM;
> > +        }
> > +
> > +        rc = xc_domain_getvnuma(xch, domid, &numa->nr_vnodes,
> > +                                &numa->nr_vmemranges,
> > +                                &config->nr_vcpus, numa->vmemrange,
> > +                                numa->vdistance, numa->vcpu_to_vnode);
> > +
> > +       if ( rc )
> > +        {
> > +            DOMPRINTF("%s: xc_domain_getvnuma failed (rc=%d)", 
> > __FUNCTION__, rc);
> > +            return rc;
> > +        }
> > +    }
> > +    else
> > +        config->nr_vcpus = info.max_vcpu_id + 1;
> > This looks wrong, at least it is not immediately clear why you would
> > want to do this.
> 
> 
> Why is this wrong? If we have one VCPU max_vcpu_id will be zero, won't it?
> 

I'm not saying it is absolutely wrong, just that I don't quite
understand why it is coded this way.

It's guarded by dom->nr_vnodes and the code is not immediately clear why
you want to do that. Perhaps you can add a comment here?

Wei.

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