[PATCH RFC V3 08/29] arm/virt: Init PMU at host for all possible vcpus

Salil Mehta via posted 29 patches 5 months, 2 weeks ago
There is a newer version of this series
[PATCH RFC V3 08/29] arm/virt: Init PMU at host for all possible vcpus
Posted by Salil Mehta via 5 months, 2 weeks ago
PMU for all possible vCPUs must be initialized at the VM initialization time.
Refactor existing code to accomodate possible vCPUs. This also assumes that all
processor being used are identical.

Past discussion for reference:
Link: https://lists.gnu.org/archive/html/qemu-devel/2020-06/msg00131.html

Co-developed-by: Salil Mehta <salil.mehta@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
Co-developed-by: Keqian Zhu <zhukeqian1@huawei.com>
Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
 hw/arm/virt.c         | 12 ++++++++----
 include/hw/arm/virt.h |  1 +
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index ac53bfadca..57ec429022 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2045,12 +2045,14 @@ static void finalize_gic_version(VirtMachineState *vms)
  */
 static void virt_cpu_post_init(VirtMachineState *vms, MemoryRegion *sysmem)
 {
+    CPUArchIdList *possible_cpus = vms->parent.possible_cpus;
     int max_cpus = MACHINE(vms)->smp.max_cpus;
-    bool aarch64, pmu, steal_time;
+    bool aarch64, steal_time;
     CPUState *cpu;
+    int n;
 
     aarch64 = object_property_get_bool(OBJECT(first_cpu), "aarch64", NULL);
-    pmu = object_property_get_bool(OBJECT(first_cpu), "pmu", NULL);
+    vms->pmu = object_property_get_bool(OBJECT(first_cpu), "pmu", NULL);
     steal_time = object_property_get_bool(OBJECT(first_cpu),
                                           "kvm-steal-time", NULL);
 
@@ -2077,8 +2079,10 @@ static void virt_cpu_post_init(VirtMachineState *vms, MemoryRegion *sysmem)
             memory_region_add_subregion(sysmem, pvtime_reg_base, pvtime);
         }
 
-        CPU_FOREACH(cpu) {
-            if (pmu) {
+        for (n = 0; n < possible_cpus->len; n++) {
+            cpu = qemu_get_possible_cpu(n);
+
+            if (vms->pmu) {
                 assert(arm_feature(&ARM_CPU(cpu)->env, ARM_FEATURE_PMU));
                 if (kvm_irqchip_in_kernel()) {
                     kvm_arm_pmu_set_irq(ARM_CPU(cpu), VIRTUAL_PMU_IRQ);
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 36ac5ff4a2..d8dcc89a0d 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -155,6 +155,7 @@ struct VirtMachineState {
     bool ras;
     bool mte;
     bool dtb_randomness;
+    bool pmu;
     OnOffAuto acpi;
     VirtGICType gic_version;
     VirtIOMMUType iommu;
-- 
2.34.1
Re: [PATCH RFC V3 08/29] arm/virt: Init PMU at host for all possible vcpus
Posted by Nicholas Piggin 4 months, 3 weeks ago
On Fri Jun 14, 2024 at 9:36 AM AEST, Salil Mehta wrote:
> PMU for all possible vCPUs must be initialized at the VM initialization time.
> Refactor existing code to accomodate possible vCPUs. This also assumes that all
> processor being used are identical.
>
> Past discussion for reference:
> Link: https://lists.gnu.org/archive/html/qemu-devel/2020-06/msg00131.html

I guess it's something for the ARM people, but there's a lot of
information in there, could it be useful to summarise important
parts here, e.g., from Andrew:

 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 assume he meant ARM arch)

>
> Co-developed-by: Salil Mehta <salil.mehta@huawei.com>
> Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> Co-developed-by: Keqian Zhu <zhukeqian1@huawei.com>
> Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
> Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> ---
>  hw/arm/virt.c         | 12 ++++++++----
>  include/hw/arm/virt.h |  1 +
>  2 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index ac53bfadca..57ec429022 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -2045,12 +2045,14 @@ static void finalize_gic_version(VirtMachineState *vms)
>   */
>  static void virt_cpu_post_init(VirtMachineState *vms, MemoryRegion *sysmem)
>  {
> +    CPUArchIdList *possible_cpus = vms->parent.possible_cpus;
>      int max_cpus = MACHINE(vms)->smp.max_cpus;
> -    bool aarch64, pmu, steal_time;
> +    bool aarch64, steal_time;
>      CPUState *cpu;
> +    int n;
>  
>      aarch64 = object_property_get_bool(OBJECT(first_cpu), "aarch64", NULL);
> -    pmu = object_property_get_bool(OBJECT(first_cpu), "pmu", NULL);
> +    vms->pmu = object_property_get_bool(OBJECT(first_cpu), "pmu", NULL);
>      steal_time = object_property_get_bool(OBJECT(first_cpu),
>                                            "kvm-steal-time", NULL);
>  
> @@ -2077,8 +2079,10 @@ static void virt_cpu_post_init(VirtMachineState *vms, MemoryRegion *sysmem)
>              memory_region_add_subregion(sysmem, pvtime_reg_base, pvtime);
>          }
>  
> -        CPU_FOREACH(cpu) {
> -            if (pmu) {
> +        for (n = 0; n < possible_cpus->len; n++) {
> +            cpu = qemu_get_possible_cpu(n);
> +

Maybe a CPU_FOREACH_POSSIBLE()?

Thanks,
Nick

> +            if (vms->pmu) {
>                  assert(arm_feature(&ARM_CPU(cpu)->env, ARM_FEATURE_PMU));
>                  if (kvm_irqchip_in_kernel()) {
>                      kvm_arm_pmu_set_irq(ARM_CPU(cpu), VIRTUAL_PMU_IRQ);
> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> index 36ac5ff4a2..d8dcc89a0d 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -155,6 +155,7 @@ struct VirtMachineState {
>      bool ras;
>      bool mte;
>      bool dtb_randomness;
> +    bool pmu;
>      OnOffAuto acpi;
>      VirtGICType gic_version;
>      VirtIOMMUType iommu;
RE: [PATCH RFC V3 08/29] arm/virt: Init PMU at host for all possible vcpus
Posted by Salil Mehta via 4 months, 3 weeks ago
HI Nick,

>  From: Nicholas Piggin <npiggin@gmail.com>
>  Sent: Thursday, July 4, 2024 4:08 AM
>  To: Salil Mehta <salil.mehta@huawei.com>; qemu-devel@nongnu.org;
>  qemu-arm@nongnu.org; mst@redhat.com
>  
>  On Fri Jun 14, 2024 at 9:36 AM AEST, Salil Mehta wrote:
>  > PMU for all possible vCPUs must be initialized at the VM initialization time.
>  > Refactor existing code to accomodate possible vCPUs. This also assumes
>  > that all processor being used are identical.
>  >
>  > Past discussion for reference:
>  > Link:
>  > https://lists.gnu.org/archive/html/qemu-devel/2020-06/msg00131.html
>  
>  I guess it's something for the ARM people, but there's a lot of information in
>  there, could it be useful to summarise important parts here, e.g., from
>  Andrew:
>  
>   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 assume he meant ARM arch)


I retained the link just for a reference. Right now it is an assumption that
all vCPUs have similar features. This is reflected in KVM as well. 
(Maybe this might not be the case in future eventually with the advent of
heterogenous computing. Linaro had something going in that direction? 
For now, it looks to be a far-fetched idea). 

I can definitely summarize what was discussed earlier.


>  > Co-developed-by: Salil Mehta <salil.mehta@huawei.com>
>  > Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
>  > Co-developed-by: Keqian Zhu <zhukeqian1@huawei.com>
>  > Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
>  > Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
>  > ---
>  >  hw/arm/virt.c         | 12 ++++++++----
>  >  include/hw/arm/virt.h |  1 +
>  >  2 files changed, 9 insertions(+), 4 deletions(-)
>  >
>  > diff --git a/hw/arm/virt.c b/hw/arm/virt.c index
>  > ac53bfadca..57ec429022 100644
>  > --- a/hw/arm/virt.c
>  > +++ b/hw/arm/virt.c
>  > @@ -2045,12 +2045,14 @@ static void
>  finalize_gic_version(VirtMachineState *vms)
>  >   */
>  >  static void virt_cpu_post_init(VirtMachineState *vms, MemoryRegion
>  > *sysmem)  {
>  > +    CPUArchIdList *possible_cpus = vms->parent.possible_cpus;
>  >      int max_cpus = MACHINE(vms)->smp.max_cpus;
>  > -    bool aarch64, pmu, steal_time;
>  > +    bool aarch64, steal_time;
>  >      CPUState *cpu;
>  > +    int n;
>  >
>  >      aarch64 = object_property_get_bool(OBJECT(first_cpu), "aarch64", NULL);
>  > -    pmu = object_property_get_bool(OBJECT(first_cpu), "pmu", NULL);
>  > +    vms->pmu = object_property_get_bool(OBJECT(first_cpu), "pmu",
>  > + NULL);
>  >      steal_time = object_property_get_bool(OBJECT(first_cpu),
>  >                                            "kvm-steal-time", NULL);
>  >
>  > @@ -2077,8 +2079,10 @@ static void virt_cpu_post_init(VirtMachineState
>  *vms, MemoryRegion *sysmem)
>  >              memory_region_add_subregion(sysmem, pvtime_reg_base,
>  pvtime);
>  >          }
>  >
>  > -        CPU_FOREACH(cpu) {
>  > -            if (pmu) {
>  > +        for (n = 0; n < possible_cpus->len; n++) {
>  > +            cpu = qemu_get_possible_cpu(n);
>  > +
>  
>  Maybe a CPU_FOREACH_POSSIBLE()?


sure.


Thank you
Salil.

>  
>  Thanks,
>  Nick
>  
>  > +            if (vms->pmu) {
>  >                  assert(arm_feature(&ARM_CPU(cpu)->env,
>  ARM_FEATURE_PMU));
>  >                  if (kvm_irqchip_in_kernel()) {
>  >                      kvm_arm_pmu_set_irq(ARM_CPU(cpu),
>  > VIRTUAL_PMU_IRQ); diff --git a/include/hw/arm/virt.h
>  > b/include/hw/arm/virt.h index 36ac5ff4a2..d8dcc89a0d 100644
>  > --- a/include/hw/arm/virt.h
>  > +++ b/include/hw/arm/virt.h
>  > @@ -155,6 +155,7 @@ struct VirtMachineState {
>  >      bool ras;
>  >      bool mte;
>  >      bool dtb_randomness;
>  > +    bool pmu;
>  >      OnOffAuto acpi;
>  >      VirtGICType gic_version;
>  >      VirtIOMMUType iommu;