Re: [Xen-devel] [PATCH v1 06/20] acpi/hvmloader: Collect processor and NUMA info in hvmloader

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 05.07.16 at 21:05, <boris.ostrovsky@xxxxxxxxxx> wrote:
> No need for ACPI code to rely on hvm_info.

Perhaps better "... on the hvm_info variable"?

> @@ -118,9 +118,9 @@ static struct acpi_20_madt *construct_madt(struct 
> acpi_info *info)
>      io_apic->ioapic_addr = IOAPIC_BASE_ADDRESS;
>  
>      lapic = (struct acpi_20_madt_lapic *)(io_apic + 1);
> -    info->nr_cpus = hvm_info->nr_vcpus;
> -    info->madt_lapic0_addr = (uint32_t)lapic;
> -    for ( i = 0; i < hvm_info->nr_vcpus; i++ )
> +    config->ainfo.nr_cpus =config-> hvminfo->nr_vcpus;

Misplaced space.

>      memory = (struct acpi_20_srat_memory *)processor;
> -    for ( i = 0; i < nr_vmemranges; i++ )
> +    for ( i = 0; i < config->numa.nr_vmemranges; i++ )
>      {
>          memory->type          = ACPI_MEMORY_AFFINITY;
>          memory->length        = sizeof(*memory);
> -        memory->domain        = vmemrange[i].nid;
> +        memory->domain        = config->numa.vmemrange[i].nid;
>          memory->flags         = ACPI_MEM_AFFIN_ENABLED;
> -        memory->base_address  = vmemrange[i].start;
> -        memory->mem_length    = vmemrange[i].end - vmemrange[i].start;
> +        memory->base_address  = config->numa.vmemrange[i].start;
> +        memory->mem_length    = config->numa.vmemrange[i].end -
> +            config->numa.vmemrange[i].start;

I'd prefer for the two config-> of this expression to align with one
another.

> --- a/tools/firmware/hvmloader/acpi/libacpi.h
> +++ b/tools/firmware/hvmloader/acpi/libacpi.h
> @@ -28,6 +28,8 @@
>  #ifndef __LIBACPI_H__
>  #define __LIBACPI_H__
>  
> +#include <xen/memory.h>

I think this can be avoided if ...

> @@ -51,6 +53,14 @@ struct acpi_info {
>      uint64_t pci_hi_min, pci_hi_len; /* 24, 32 - PCI I/O hole boundaries */
>  };
>  
> +struct acpi_numa {
> +    uint32_t nr_vmemranges;
> +    uint32_t nr_vnodes;
> +    unsigned int *vcpu_to_vnode;
> +    unsigned int *vdistance;
> +    xen_vmemrange_t *vmemrange;

... you use struct xen_vmemrange * here.

Also I think the two pointed to types can and should be const
qualified.

And then - any reason not to put this ...

> @@ -66,6 +76,9 @@ struct acpi_config {
>          uint32_t pt_length;
>      } pt;
>  
> +    struct acpi_numa numa;

... right here, perhaps even omitting the structure tag?

Jan


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