On Mon, Jun 01, 2020 at 03:04:33PM +0000, Salil Mehta wrote: > Hello, > I could see below within function fdt_add_pmu_nodes() part of > hw/arm/virt.c during virt machine initialization time: > > Observation: > In below function, support of PMU feature is being checked for > each vcpu and if the PMU is found part of the features then PMU > is initialized with in the host/KVM. But if there is even one > vcpu which is found to not support the PMU then loop is exited > and PMU is not initialized for the rest of the vcpus as well. > > Questions: > Q1. Not sure what is the logic of the premature exit and not > continuing with further checks and initialization of other > VCPU PMUs?
KVM requires all VCPUs to have a PMU if one does. If the ARM ARM says it's possible to have PMUs for only some CPUs, then, for TCG, the restriction could be relaxed. I expect it will take more than just removing the check for things to work though. > Q2. Does it even makes sense to have PMUs initialized for some > vcpus and not for others unless we have heterogeneous system? I don't know, but it doesn't sound like a configuration I'd like to see. > Q3. Also, there is a per virt machine knob of vcc->no_pmu. > This is something which user could specify at the init time > and perhaps only once but we don't use it for ARM. Perhaps > should have been used even before entering this function > to enable or disable the support as per user config? It's purpose is to keep users from doing 'pmu=on' on 2.6 machine types. On 2.7 and later machine types if you don't want a PMU you should use 'pmu=off'. > Q4. This function fdt_* looks to be wrongly named. The info > being initialized here shall be used even when ACPI is > being used. Initialization part and FDT info looked > mixed up here if I am right? Agreed. The function has the wrong name. mach-virt has many functions that mix the initialization and fdt building together, but those functions are named something like create_foo(). Patches welcome. Thanks, drew