Hi Alireza,
On Fri, Aug 23, 2024 at 01:54:46PM +0100, Alireza Sanaee wrote:
[snip]
> +static int partial_cache_description(MachineState *ms, ACPIPPTTCache* caches,
> + int num_caches)
> +{
> + int level, c;
> +
> + for (level = 1; level < num_caches; level++) {
> + for (c = 0; c < num_caches; c++) {
> + if (caches[c].level != level) {
> + continue;
> + }
> +
> + switch (level) {
> + case 1:
> + /*
> + * L1 cache is assumed to have both L1I and L1D available.
> + * Technically both need to be checked.
> + */
> + if (machine_get_cache_topo_level(ms, SMP_CACHE_L1I) ==
> + CPU_TOPO_LEVEL_DEFAULT) {
This check just concerns L1i, but it looks not covering L1d, is L1d being
missed?
> + assert(machine_get_cache_topo_level(ms, SMP_CACHE_L1D) !=
> + CPU_TOPO_LEVEL_DEFAULT);
I understand you don't want user to configure other different levels for
L1d in this case...If so, it's better to return error (error_steg or
error_report or some other error print ways) to tell user his cache
configuration is invalid.
> + return level;
> + }
> + break;
> + case 2:
> + if (machine_get_cache_topo_level(ms, SMP_CACHE_L2) ==
> + CPU_TOPO_LEVEL_DEFAULT) {
> + return level;
> + }
> + break;
> + case 3:
> + if (machine_get_cache_topo_level(ms, SMP_CACHE_L3) ==
> + CPU_TOPO_LEVEL_DEFAULT) {
> + return level;
> + }
> + break;
> + }
> + }
> + }
> +
> + return 0;
> +}
> +
[snip]
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index b0c68d66a3..b723248ecf 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -3093,6 +3093,11 @@ static void virt_machine_class_init(ObjectClass *oc,
> void *data)
> hc->unplug = virt_machine_device_unplug_cb;
> mc->nvdimm_supported = true;
> mc->smp_props.clusters_supported = true;
> + /* Supported cached */
> + mc->smp_props.cache_supported[SMP_CACHE_L1D] = true;
> + mc->smp_props.cache_supported[SMP_CACHE_L1I] = true;
> + mc->smp_props.cache_supported[SMP_CACHE_L2] = true;
> + mc->smp_props.cache_supported[SMP_CACHE_L3] = true;
> mc->auto_enable_numa_with_memhp = true;
> mc->auto_enable_numa_with_memdev = true;
> /* platform instead of architectural choice */
> diff --git a/hw/core/machine-smp.c b/hw/core/machine-smp.c
> index bf6f2f9107..de95ec9c0f 100644
> --- a/hw/core/machine-smp.c
> +++ b/hw/core/machine-smp.c
> @@ -274,7 +274,11 @@ unsigned int machine_topo_get_threads_per_socket(const
> MachineState *ms)
> CpuTopologyLevel machine_get_cache_topo_level(const MachineState *ms,
> SMPCacheName cache)
> {
> - return ms->smp_cache->props[cache].topo;
> + if (ms->smp_cache) {
> + return ms->smp_cache->props[cache].topo;
> + }
> +
> + return CPU_TOPO_LEVEL_DEFAULT;
> }
>
> static bool machine_check_topo_support(MachineState *ms,
Maybe it's better to split smp-cache support/check on Arm in a seperate
patch.
Regards,
Zhao
