On 10/30/2020 12:56 AM, Andrew Jones wrote:
> On Tue, Oct 20, 2020 at 09:14:36PM +0800, Ying Fang wrote:
>> Add the Processor Properties Topology Table (PPTT) to present CPU topology
>> information to the guest.
>>
>> Signed-off-by: Andrew Jones <drjones@redhat.com>
>
> I don't know why I have an s-o-b here. I guess it's because this code
> looks nearly identical to what I wrote, except for using the new and,
> IMO, unnecessary build_socket_hierarchy and build_smt_hierarchy functions.
>
> IMHO, you should drop the last patch and just take
>
> https://github.com/rhdrjones/qemu/commit/439b38d67ca1f2cbfa5b9892a822b651ebd05c11
>
> as it is, unless it needs to be fixed somehow
>
> Thanks,
> drew
This patch is based on your branch however it is slightly modified.
As described in:
[RFC,v2,08/13] hw/acpi/aml-build: add processor hierarchy node structure
The wrapper function build_socket_hierarchy and build_smt_hierarchy are
introduced to make later patch much more readable and make preparations
for cache hierarchy.
Hope it won't make you confused. I will drop your branch patch and
give details in the commit message in the next post.
Thanks,
Ying
>
>> Signed-off-by: Ying Fang <fangying1@huawei.com>
>> ---
>> hw/arm/virt-acpi-build.c | 42 ++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 42 insertions(+)
>>
>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
>> index fae5a26741..e1f3ea50ad 100644
>> --- a/hw/arm/virt-acpi-build.c
>> +++ b/hw/arm/virt-acpi-build.c
>> @@ -429,6 +429,42 @@ build_srat(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>> "SRAT", table_data->len - srat_start, 3, NULL, NULL);
>> }
>>
>> +static void build_pptt(GArray *table_data, BIOSLinker *linker, MachineState *ms)
>> +{
>> + int pptt_start = table_data->len;
>> + int uid = 0, cpus = 0, socket;
>> + unsigned int smp_cores = ms->smp.cores;
>> + unsigned int smp_threads = ms->smp.threads;
>> +
>> + acpi_data_push(table_data, sizeof(AcpiTableHeader));
>> +
>> + for (socket = 0; cpus < ms->possible_cpus->len; socket++) {
>> + uint32_t socket_offset = table_data->len - pptt_start;
>> + int core;
>> +
>> + build_socket_hierarchy(table_data, 0, socket);
>> +
>> + for (core = 0; core < smp_cores; core++) {
>> + uint32_t core_offset = table_data->len - pptt_start;
>> + int thread;
>> +
>> + if (smp_threads <= 1) {
>> + build_processor_hierarchy(table_data, 2, socket_offset, uid++);
>> + } else {
>> + build_processor_hierarchy(table_data, 0, socket_offset, core);
>> + for (thread = 0; thread < smp_threads; thread++) {
>> + build_smt_hierarchy(table_data, core_offset, uid++);
>> + }
>> + }
>> + }
>> + cpus += smp_cores * smp_threads;
>> + }
>> +
>> + build_header(linker, table_data,
>> + (void *)(table_data->data + pptt_start), "PPTT",
>> + table_data->len - pptt_start, 2, NULL, NULL);
>> +}
>> +
>> /* GTDT */
>> static void
>> build_gtdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>> @@ -669,6 +705,7 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
>> unsigned dsdt, xsdt;
>> GArray *tables_blob = tables->table_data;
>> MachineState *ms = MACHINE(vms);
>> + bool cpu_topology_enabled = !vmc->ignore_cpu_topology;
>>
>> table_offsets = g_array_new(false, true /* clear */,
>> sizeof(uint32_t));
>> @@ -688,6 +725,11 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
>> acpi_add_table(table_offsets, tables_blob);
>> build_madt(tables_blob, tables->linker, vms);
>>
>> + if (cpu_topology_enabled) {
>> + acpi_add_table(table_offsets, tables_blob);
>> + build_pptt(tables_blob, tables->linker, ms);
>> + }
>> +
>> acpi_add_table(table_offsets, tables_blob);
>> build_gtdt(tables_blob, tables->linker, vms);
>>
>> --
>> 2.23.0
>>
>>
>
> .
>