Re: [PATCH v2 13/19] hvmloader: construct SRAT

["Jan Beulich" <JBeulich@suse.com>]

>>> On 01.12.14 at 16:33, <wei.liu2@citrix.com> wrote:
> @@ -203,6 +204,65 @@ 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) * nr_vmemranges;
> +
> +    p = mem_alloc(size, 16);
> +    if (!p) return NULL;

Coding style (despite you likely copied it from elsewhere).

> +
> +    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   = vcpu_to_vnode[i];
> +        processor->apic_id  = LAPIC_ID(i);
> +        processor->flags    = ACPI_LOCAL_APIC_AFFIN_ENABLED;
> +        processor++;
> +    }
> +
> +    memory = (struct acpi_20_srat_memory *)processor;
> +    for ( i = 0; i < nr_vmemranges; i++ )
> +    {
> +        mem = vmemrange[i].end - vmemrange[i].start;

Do you really need this helper variable?

> +        memset(memory, 0, sizeof(*memory));
> +        memory->type          = ACPI_MEMORY_AFFINITY;
> +        memory->length        = sizeof(*memory);
> +        memory->domain        = vmemrange[i].nid;
> +        memory->flags         = ACPI_MEM_AFFIN_ENABLED;
> +        memory->base_address  = vmemrange[i].start;
> +        memory->mem_length    = mem;
> +        memory++;
> +    }
> +
> +    srat->header.length = size;

Mind checking size == memory - p here?

> @@ -346,6 +407,16 @@ static int construct_secondary_tables(unsigned long *table_ptrs,
>          }
>      }
>  
> +    /* SRAT */
> +    if ( nr_vnodes > 0 )
> +    {
> +        srat = construct_srat();
> +        if (srat)

Coding style again.

Jan