On Tue, 6 Jan 2026 15:58:25 +0000 Alireza Sanaee <[email protected]> wrote:
> Add cache topology to PPTT table. > > Signed-off-by: Alireza Sanaee <[email protected]> Some trivial stuff inline around an error message that I think wants a tiny tweak, mostly because I missunderstood it when rereading this! Reviewed-by: Jonathan Cameron <[email protected]> > --- > hw/acpi/aml-build.c | 200 +++++++++++++++++++++++++++++++++++++-- > hw/arm/virt-acpi-build.c | 8 +- > include/hw/acpi/cpu.h | 10 ++ > 3 files changed, 209 insertions(+), 9 deletions(-) > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c > index a711a9600e..580eceb24a 100644 > --- a/hw/acpi/aml-build.c > +++ b/hw/acpi/aml-build.c > * ACPI spec, Revision 6.3 > * 5.2.29 Processor Properties Topology Table (PPTT) > @@ -2150,11 +2249,32 @@ void build_pptt(GArray *table_data, BIOSLinker > *linker, MachineState *ms, > { > MachineClass *mc = MACHINE_GET_CLASS(ms); > CPUArchIdList *cpus = ms->possible_cpus; > - int64_t socket_id = -1, cluster_id = -1, core_id = -1; > - uint32_t socket_offset = 0, cluster_offset = 0, core_offset = 0; > + uint32_t core_data_offset = 0; > + uint32_t core_instr_offset = 0; > + uint32_t cluster_instr_offset = 0; > + uint32_t cluster_data_offset = 0; > + uint32_t node_data_offset = 0; > + uint32_t node_instr_offset = 0; > + int top_node = 3; > + int top_cluster = 3; > + int top_core = 3; > + int bottom_node = 3; > + int bottom_cluster = 3; > + int bottom_core = 3; > + int64_t socket_id = -1; > + int64_t cluster_id = -1; > + int64_t core_id = -1; > + uint32_t socket_offset = 0; > + uint32_t cluster_offset = 0; > + uint32_t core_offset = 0; > uint32_t pptt_start = table_data->len; > uint32_t root_offset; > int n; > + uint32_t priv_rsrc[2]; > + uint32_t num_priv = 0; > + bool cache_available; > + bool llevel; > + > AcpiTable table = { .sig = "PPTT", .rev = 2, > .oem_id = oem_id, .oem_table_id = oem_table_id }; > > @@ -2184,11 +2304,30 @@ void build_pptt(GArray *table_data, BIOSLinker > *linker, MachineState *ms, > socket_id = cpus->cpus[n].props.socket_id; > cluster_id = -1; > core_id = -1; > + bottom_node = top_node; > + num_priv = 0; > + cache_available = machine_defines_cache_at_topo_level( > + ms, CPU_TOPOLOGY_LEVEL_SOCKET); > + llevel = machine_find_lowest_level_cache_at_topo_level( > + ms, &bottom_node, CPU_TOPOLOGY_LEVEL_SOCKET); > + if (cache_available && llevel) { > + build_caches(table_data, pptt_start, num_caches, caches, > + top_node, bottom_node, &node_data_offset, > + &node_instr_offset); > + priv_rsrc[0] = node_instr_offset; > + priv_rsrc[1] = node_data_offset; > + if (node_instr_offset || node_data_offset) { > + num_priv = node_instr_offset == node_data_offset ? 1 : 2; > + } > + > + top_cluster = bottom_node - 1; > + } > + > socket_offset = table_data->len - pptt_start; > build_processor_hierarchy_node(table_data, > (1 << 0) | /* Physical package */ > (1 << 4), /* Identical Implementation */ > - root_offset, socket_id, NULL, 0); > + root_offset, socket_id, priv_rsrc, num_priv); > } > > if (mc->smp_props.clusters_supported && mc->smp_props.has_clusters) { > @@ -2196,21 +2335,68 @@ void build_pptt(GArray *table_data, BIOSLinker > *linker, MachineState *ms, > assert(cpus->cpus[n].props.cluster_id > cluster_id); > cluster_id = cpus->cpus[n].props.cluster_id; > core_id = -1; > + bottom_cluster = top_cluster; > + num_priv = 0; > + cache_available = machine_defines_cache_at_topo_level( > + ms, CPU_TOPOLOGY_LEVEL_CLUSTER); > + llevel = machine_find_lowest_level_cache_at_topo_level( > + ms, &bottom_cluster, CPU_TOPOLOGY_LEVEL_CLUSTER); > + Trivial consistency thing, but in the equivalent socket block above you don't have a blank line here. > + if (cache_available && llevel) { > + build_caches(table_data, pptt_start, num_caches, caches, > + top_cluster, bottom_cluster, > + &cluster_data_offset, > &cluster_instr_offset); > + priv_rsrc[0] = cluster_instr_offset; > + priv_rsrc[1] = cluster_data_offset; > + if (cluster_instr_offset || cluster_data_offset) { > + num_priv = > + cluster_instr_offset == cluster_data_offset ? 1 > : 2; > + } > + top_core = bottom_cluster - 1; > + } else if (top_cluster == bottom_node - 1) { > + /* socket cache but no cluster cache */ > + top_core = bottom_node - 1; > + } > + > cluster_offset = table_data->len - pptt_start; > build_processor_hierarchy_node(table_data, > (0 << 0) | /* Not a physical package */ > (1 << 4), /* Identical Implementation */ > - socket_offset, cluster_id, NULL, 0); > + socket_offset, cluster_id, priv_rsrc, num_priv); > } > } else { > + if (machine_defines_cache_at_topo_level( > + ms, CPU_TOPOLOGY_LEVEL_CLUSTER)) { > + error_setg(&error_fatal, "Not clusters found for the cache"); I'd rewrite this error message as something like "Cache specified for cluster level but no clusters in topology" > + return; > + } > + > cluster_offset = socket_offset; > + top_core = bottom_node - 1; /* there is no cluster */
