Re: [Xen-devel] [PATCH 12/19] hvmloader: construct SRAT

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 21.11.14 at 16:06, <wei.liu2@xxxxxxxxxx> wrote:
> --- a/tools/firmware/hvmloader/acpi/build.c
> +++ b/tools/firmware/hvmloader/acpi/build.c
> @@ -203,6 +203,66 @@ static struct acpi_20_waet *construct_waet(void)
>      return waet;
>  }
>  
> +static struct acpi_20_srat *construct_srat(void)
> +{
> +    struct acpi_20_srat *srat;
> +    struct acpi_20_srat_processor *processor;
> +    struct acpi_20_srat_memory *memory;
> +    unsigned int size;
> +    void *p;
> +    int i;
> +    uint64_t mem;
> +
> +    size = sizeof(*srat) + sizeof(*processor) * hvm_info->nr_vcpus +
> +        sizeof(*memory) * hvm_info->nr_vmemranges;
> +
> +    p = mem_alloc(size, 16);
> +    if (!p) return NULL;
> +
> +    srat = p;
> +    memset(srat, 0, sizeof(*srat));
> +    srat->header.signature    = ACPI_2_0_SRAT_SIGNATURE;
> +    srat->header.revision     = ACPI_2_0_SRAT_REVISION;
> +    fixed_strcpy(srat->header.oem_id, ACPI_OEM_ID);
> +    fixed_strcpy(srat->header.oem_table_id, ACPI_OEM_TABLE_ID);
> +    srat->header.oem_revision = ACPI_OEM_REVISION;
> +    srat->header.creator_id   = ACPI_CREATOR_ID;
> +    srat->header.creator_revision = ACPI_CREATOR_REVISION;
> +    srat->table_revision      = ACPI_SRAT_TABLE_REVISION;
> +
> +    processor = (struct acpi_20_srat_processor *)(srat + 1);
> +    for ( i = 0; i < hvm_info->nr_vcpus; i++ )
> +    {
> +        memset(processor, 0, sizeof(*processor));
> +        processor->type     = ACPI_PROCESSOR_AFFINITY;
> +        processor->length   = sizeof(*processor);
> +        processor->domain   = hvm_info->vcpu_to_vnode[i];
> +        processor->apic_id  = LAPIC_ID(i);
> +        processor->flags    = ACPI_LOCAL_APIC_AFFIN_ENABLED;
> +        processor->sapic_id = 0;

Either you initialize all fields explicitly and drop the memset(), or
you consistently avoid explicit zero initializers (as being redundant).

> @@ -270,6 +331,13 @@ static int construct_secondary_tables(unsigned long 
> *table_ptrs,
>          table_ptrs[nr_tables++] = (unsigned long)madt;
>      }
>  
> +    if ( hvm_info->nr_nodes > 0 )
> +    {
> +        srat = construct_srat();
> +        if (!srat) return -1;

I don't think failure to set up secondary information (especially when
it requires a variable size table and hence has [slightly] higher
likelihood of table space allocation failing) should result in skipping
other table setup.

Jan


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