Re: [Xen-devel] [PATCH v2 13/19] hvmloader: construct SRAT
On 10.12.14 at 11:54, wei.l...@citrix.com wrote: On Wed, Dec 10, 2014 at 08:20:38AM +, Jan Beulich wrote: On 09.12.14 at 19:06, wei.l...@citrix.com wrote: On Tue, Dec 09, 2014 at 04:53:40PM +, Jan Beulich wrote: On 01.12.14 at 16:33, wei.l...@citrix.com wrote: +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? Why? There doesn't seem to be anything that would cause memory -p != size in between during runtime. Except for that original calculation being wrong - that's what I would mean such a check to verify. Do you mean the code is wrong? I don't think so. No, the code looks right. It's just that two disconnected calculations easily get out of sync when someone later changes them. Even if I have +if ( ((unsigned long)memory) - ((unsigned long)p) != size ) +return NULL; + Hvmloader doesn't complain, i.e. I don't see error message printed out in the construct_secondary_tables. Anyway, if the above snippet is what you want, I can add it. I was actually more after a BUG_ON() / ASSERT() kind of thing. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 13/19] hvmloader: construct SRAT
On Wed, Dec 10, 2014 at 11:06:20AM +, Jan Beulich wrote: On 10.12.14 at 11:54, wei.l...@citrix.com wrote: On Wed, Dec 10, 2014 at 08:20:38AM +, Jan Beulich wrote: On 09.12.14 at 19:06, wei.l...@citrix.com wrote: On Tue, Dec 09, 2014 at 04:53:40PM +, Jan Beulich wrote: On 01.12.14 at 16:33, wei.l...@citrix.com wrote: +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? Why? There doesn't seem to be anything that would cause memory -p != size in between during runtime. Except for that original calculation being wrong - that's what I would mean such a check to verify. Do you mean the code is wrong? I don't think so. No, the code looks right. It's just that two disconnected calculations easily get out of sync when someone later changes them. I see. Even if I have +if ( ((unsigned long)memory) - ((unsigned long)p) != size ) +return NULL; + Hvmloader doesn't complain, i.e. I don't see error message printed out in the construct_secondary_tables. Anyway, if the above snippet is what you want, I can add it. I was actually more after a BUG_ON() / ASSERT() kind of thing. Ack. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 13/19] hvmloader: construct SRAT
On 01.12.14 at 16:33, wei.l...@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 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 13/19] hvmloader: construct SRAT
On Tue, Dec 09, 2014 at 04:53:40PM +, Jan Beulich wrote: On 01.12.14 at 16:33, wei.l...@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). Fixed. + +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? Removed. +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? Why? There doesn't seem to be anything that would cause memory -p != size in between during runtime. @@ -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. Fixed. Wei. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 13/19] hvmloader: construct SRAT
Signed-off-by: Wei Liu wei.l...@citrix.com Cc: Jan Beulich jbeul...@suse.com --- Changes in v2: 1. Remove explicit zero initializers. 2. Adapt to new vNUMA retrieval routine. 3. Move SRAT very late in secondary table build. --- tools/firmware/hvmloader/acpi/acpi2_0.h | 53 +++ tools/firmware/hvmloader/acpi/build.c | 71 +++ 2 files changed, 124 insertions(+) diff --git a/tools/firmware/hvmloader/acpi/acpi2_0.h b/tools/firmware/hvmloader/acpi/acpi2_0.h index 7b22d80..6169213 100644 --- a/tools/firmware/hvmloader/acpi/acpi2_0.h +++ b/tools/firmware/hvmloader/acpi/acpi2_0.h @@ -364,6 +364,57 @@ struct acpi_20_madt_intsrcovr { }; /* + * System Resource Affinity Table header definition (SRAT) + */ +struct acpi_20_srat { +struct acpi_header header; +uint32_t table_revision; +uint32_t reserved2[2]; +}; + +#define ACPI_SRAT_TABLE_REVISION 1 + +/* + * System Resource Affinity Table structure types. + */ +#define ACPI_PROCESSOR_AFFINITY 0x0 +#define ACPI_MEMORY_AFFINITY0x1 +struct acpi_20_srat_processor { +uint8_t type; +uint8_t length; +uint8_t domain; +uint8_t apic_id; +uint32_t flags; +uint8_t sapic_id; +uint8_t domain_hi[3]; +uint32_t reserved; +}; + +/* + * Local APIC Affinity Flags. All other bits are reserved and must be 0. + */ +#define ACPI_LOCAL_APIC_AFFIN_ENABLED (1 0) + +struct acpi_20_srat_memory { +uint8_t type; +uint8_t length; +uint32_t domain; +uint16_t reserved; +uint64_t base_address; +uint64_t mem_length; +uint32_t reserved2; +uint32_t flags; +uint64_t reserved3; +}; + +/* + * Memory Affinity Flags. All other bits are reserved and must be 0. + */ +#define ACPI_MEM_AFFIN_ENABLED (1 0) +#define ACPI_MEM_AFFIN_HOTPLUGGABLE (1 1) +#define ACPI_MEM_AFFIN_NONVOLATILE (1 2) + +/* * Table Signatures. */ #define ACPI_2_0_RSDP_SIGNATURE ASCII64('R','S','D',' ','P','T','R',' ') @@ -375,6 +426,7 @@ struct acpi_20_madt_intsrcovr { #define ACPI_2_0_TCPA_SIGNATURE ASCII32('T','C','P','A') #define ACPI_2_0_HPET_SIGNATURE ASCII32('H','P','E','T') #define ACPI_2_0_WAET_SIGNATURE ASCII32('W','A','E','T') +#define ACPI_2_0_SRAT_SIGNATURE ASCII32('S','R','A','T') /* * Table revision numbers. @@ -388,6 +440,7 @@ struct acpi_20_madt_intsrcovr { #define ACPI_2_0_HPET_REVISION 0x01 #define ACPI_2_0_WAET_REVISION 0x01 #define ACPI_1_0_FADT_REVISION 0x01 +#define ACPI_2_0_SRAT_REVISION 0x01 #pragma pack () diff --git a/tools/firmware/hvmloader/acpi/build.c b/tools/firmware/hvmloader/acpi/build.c index 1431296..917b2c9 100644 --- a/tools/firmware/hvmloader/acpi/build.c +++ b/tools/firmware/hvmloader/acpi/build.c @@ -23,6 +23,7 @@ #include ssdt_pm.h #include ../config.h #include ../util.h +#include ../vnuma.h #include xen/hvm/hvm_xs_strings.h #include xen/hvm/params.h @@ -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; + +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; +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; +set_checksum(srat, offsetof(struct acpi_header, checksum), size); + +return srat; +} +