diff mbox

[v2,2/3] hw/arm/virt: Add PMU node for virt machine

Message ID 1461405855-15576-3-git-send-email-zhaoshenglong@huawei.com
State Superseded
Headers show

Commit Message

Shannon Zhao April 23, 2016, 10:04 a.m. UTC 
From: Shannon Zhao <shannon.zhao@linaro.org>


Add a virtual PMU device for virt machine while use PPI 7 for PMU
overflow interrupt number.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>

---
 hw/arm/virt.c         | 31 +++++++++++++++++++++++++++++++
 include/hw/arm/virt.h |  4 ++++
 include/sysemu/kvm.h  |  1 +
 stubs/kvm.c           |  5 +++++
 target-arm/kvm64.c    | 39 +++++++++++++++++++++++++++++++++++++++
 5 files changed, 80 insertions(+)

-- 
2.0.4

Comments

Andrew Jones April 24, 2016, 5:28 p.m. UTC | #1 
On Sat, Apr 23, 2016 at 06:04:14PM +0800, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>

> 

> Add a virtual PMU device for virt machine while use PPI 7 for PMU

> overflow interrupt number.

> 

> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>

> ---

>  hw/arm/virt.c         | 31 +++++++++++++++++++++++++++++++

>  include/hw/arm/virt.h |  4 ++++

>  include/sysemu/kvm.h  |  1 +

>  stubs/kvm.c           |  5 +++++

>  target-arm/kvm64.c    | 39 +++++++++++++++++++++++++++++++++++++++

>  5 files changed, 80 insertions(+)

> 

> diff --git a/hw/arm/virt.c b/hw/arm/virt.c

> index 56d35c7..d77b314 100644

> --- a/hw/arm/virt.c

> +++ b/hw/arm/virt.c

> @@ -428,6 +428,35 @@ static void fdt_add_gic_node(VirtBoardInfo *vbi, int type)

>      qemu_fdt_setprop_cell(vbi->fdt, "/intc", "phandle", vbi->gic_phandle);

>  }

>  

> +static void fdt_add_pmu_nodes(const VirtBoardInfo *vbi)

> +{

> +    CPUState *cpu;

> +    ARMCPU *armcpu;

> +    uint32_t irqflags = GIC_FDT_IRQ_FLAGS_LEVEL_HI;

> +

> +    CPU_FOREACH(cpu) {

> +        armcpu = ARM_CPU(cpu);

> +        if (!armcpu->has_pmu) {

> +            return;

> +        }

> +

> +        kvm_arm_pmu_create(cpu, PPI(VIRTUAL_PMU_IRQ));

> +    }

> +

> +    irqflags = deposit32(irqflags, GIC_FDT_IRQ_PPI_CPU_START,

> +                         GIC_FDT_IRQ_PPI_CPU_WIDTH, (1 << vbi->smp_cpus) - 1);


The shift will overflow when configuring a machine to have more than
32 cpus. You should confirm smp_cpus is <= GIC_FDT_IRQ_PPI_CPU_WIDTH
before generating a mask this way, otherwise you can just pass in 0xff.

> +

> +    armcpu = ARM_CPU(qemu_get_cpu(0));

> +    qemu_fdt_add_subnode(vbi->fdt, "/pmu");

> +    if (arm_feature(&armcpu->env, ARM_FEATURE_V8)) {

> +        const char compat[] = "arm,armv8-pmuv3";

> +        qemu_fdt_setprop(vbi->fdt, "/pmu", "compatible",

> +                         compat, sizeof(compat));

> +        qemu_fdt_setprop_cells(vbi->fdt, "/pmu", "interrupts",

> +                               GIC_FDT_IRQ_TYPE_PPI, VIRTUAL_PMU_IRQ, irqflags);

> +    }

> +}

> +

>  static void create_v2m(VirtBoardInfo *vbi, qemu_irq *pic)

>  {

>      int i;

> @@ -1246,6 +1275,8 @@ static void machvirt_init(MachineState *machine)

>  

>      create_gic(vbi, pic, gic_version, vms->secure);

>  

> +    fdt_add_pmu_nodes(vbi);

> +

>      create_uart(vbi, pic, VIRT_UART, sysmem);

>  

>      if (vms->secure) {

> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h

> index ecd8589..b50f095 100644

> --- a/include/hw/arm/virt.h

> +++ b/include/hw/arm/virt.h

> @@ -40,6 +40,10 @@

>  #define ARCH_TIMER_NS_EL1_IRQ 14

>  #define ARCH_TIMER_NS_EL2_IRQ 10

>  

> +#define VIRTUAL_PMU_IRQ 7

> +

> +#define PPI(irq) ((irq) + 16)

> +

>  enum {

>      VIRT_FLASH,

>      VIRT_MEM,

> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h

> index 0e18f15..90c2c54 100644

> --- a/include/sysemu/kvm.h

> +++ b/include/sysemu/kvm.h

> @@ -523,4 +523,5 @@ int kvm_set_one_reg(CPUState *cs, uint64_t id, void *source);

>   * Returns: 0 on success, or a negative errno on failure.

>   */

>  int kvm_get_one_reg(CPUState *cs, uint64_t id, void *target);

> +void kvm_arm_pmu_create(CPUState *cs, int irq);

>  #endif

> diff --git a/stubs/kvm.c b/stubs/kvm.c

> index ddd6204..58a348a 100644

> --- a/stubs/kvm.c

> +++ b/stubs/kvm.c

> @@ -6,3 +6,8 @@ int kvm_arch_irqchip_create(MachineState *ms, KVMState *s)

>  {

>      return 0;

>  }

> +

> +void kvm_arm_pmu_create(CPUState *cs, int irq)

> +{

> +    return;

> +}

> diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c

> index b364789..faec4fa 100644

> --- a/target-arm/kvm64.c

> +++ b/target-arm/kvm64.c

> @@ -382,6 +382,45 @@ static CPUWatchpoint *find_hw_watchpoint(CPUState *cpu, target_ulong addr)

>      return NULL;

>  }

>  

> +static bool kvm_arm_pmu_support_ctrl(CPUState *cs, struct kvm_device_attr *attr)

> +{

> +    return kvm_vcpu_ioctl(cs, KVM_HAS_DEVICE_ATTR, attr) == 0;

> +}

> +

> +void kvm_arm_pmu_create(CPUState *cs, int irq)

> +{

> +    int err;

> +

> +    struct kvm_device_attr attr = {

> +        .group = KVM_ARM_VCPU_PMU_V3_CTRL,

> +        .addr = (intptr_t)&irq,

> +        .attr = KVM_ARM_VCPU_PMU_V3_IRQ,

> +        .flags = 0,

> +    };

> +

> +    if (!kvm_arm_pmu_support_ctrl(cs, &attr)) {

> +        return;

> +    }

> +

> +    err = kvm_vcpu_ioctl(cs, KVM_SET_DEVICE_ATTR, &attr);

> +    if (err < 0) {

> +        fprintf(stderr, "KVM_{SET/GET}_DEVICE_ATTR failed: %s\n",


You know it's a KVM_SET_DEVICE_ATTR here, so why report SET/GET?

> +                strerror(-err));

> +        abort();

> +    }

> +

> +    attr.group = KVM_ARM_VCPU_PMU_V3_CTRL;

> +    attr.attr = KVM_ARM_VCPU_PMU_V3_INIT;

> +    attr.addr = 0;

> +    attr.flags = 0;

> +

> +    err = kvm_vcpu_ioctl(cs, KVM_SET_DEVICE_ATTR, &attr);

> +    if (err < 0) {

> +        fprintf(stderr, "KVM_{SET/GET}_DEVICE_ATTR failed: %s\n",


Same SET comment as above.

> +                strerror(-err));

> +        abort();

> +    }

> +}

>  

>  static inline void set_feature(uint64_t *features, int feature)

>  {

> -- 

> 2.0.4

> 

>


Thanks,
drew
Andrew Jones April 25, 2016, 9:05 a.m. UTC | #2 
On Mon, Apr 25, 2016 at 02:52:45PM +0800, Shannon Zhao wrote:
> 

> 

> On 2016/4/25 1:28, Andrew Jones wrote:

> > On Sat, Apr 23, 2016 at 06:04:14PM +0800, Shannon Zhao wrote:

> >> > From: Shannon Zhao <shannon.zhao@linaro.org>

> >> > 

> >> > Add a virtual PMU device for virt machine while use PPI 7 for PMU

> >> > overflow interrupt number.

> >> > 

> >> > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>

> >> > ---

> >> >  hw/arm/virt.c         | 31 +++++++++++++++++++++++++++++++

> >> >  include/hw/arm/virt.h |  4 ++++

> >> >  include/sysemu/kvm.h  |  1 +

> >> >  stubs/kvm.c           |  5 +++++

> >> >  target-arm/kvm64.c    | 39 +++++++++++++++++++++++++++++++++++++++

> >> >  5 files changed, 80 insertions(+)

> >> > 

> >> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c

> >> > index 56d35c7..d77b314 100644

> >> > --- a/hw/arm/virt.c

> >> > +++ b/hw/arm/virt.c

> >> > @@ -428,6 +428,35 @@ static void fdt_add_gic_node(VirtBoardInfo *vbi, int type)

> >> >      qemu_fdt_setprop_cell(vbi->fdt, "/intc", "phandle", vbi->gic_phandle);

> >> >  }

> >> >  

> >> > +static void fdt_add_pmu_nodes(const VirtBoardInfo *vbi)

> >> > +{

> >> > +    CPUState *cpu;

> >> > +    ARMCPU *armcpu;

> >> > +    uint32_t irqflags = GIC_FDT_IRQ_FLAGS_LEVEL_HI;

> >> > +

> >> > +    CPU_FOREACH(cpu) {

> >> > +        armcpu = ARM_CPU(cpu);

> >> > +        if (!armcpu->has_pmu) {

> >> > +            return;

> >> > +        }

> >> > +

> >> > +        kvm_arm_pmu_create(cpu, PPI(VIRTUAL_PMU_IRQ));

> >> > +    }

> >> > +

> >> > +    irqflags = deposit32(irqflags, GIC_FDT_IRQ_PPI_CPU_START,

> >> > +                         GIC_FDT_IRQ_PPI_CPU_WIDTH, (1 << vbi->smp_cpus) - 1);

> > The shift will overflow when configuring a machine to have more than

> > 32 cpus. You should confirm smp_cpus is <= GIC_FDT_IRQ_PPI_CPU_WIDTH

> > before generating a mask this way, otherwise you can just pass in 0xff.

> > 

> Ah, right. So it could check the gictype like what we do in

> fdt_add_timer_nodes.


Oh, now that I look closer we definitely need that condition. Otherwise
we're losing the LEVEL_HI flag that irqflags is initialized to for no
reason.

Thanks,
drew
diff mbox

Patch

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 56d35c7..d77b314 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -428,6 +428,35 @@  static void fdt_add_gic_node(VirtBoardInfo *vbi, int type)
     qemu_fdt_setprop_cell(vbi->fdt, "/intc", "phandle", vbi->gic_phandle);
 }
 
+static void fdt_add_pmu_nodes(const VirtBoardInfo *vbi)
+{
+    CPUState *cpu;
+    ARMCPU *armcpu;
+    uint32_t irqflags = GIC_FDT_IRQ_FLAGS_LEVEL_HI;
+
+    CPU_FOREACH(cpu) {
+        armcpu = ARM_CPU(cpu);
+        if (!armcpu->has_pmu) {
+            return;
+        }
+
+        kvm_arm_pmu_create(cpu, PPI(VIRTUAL_PMU_IRQ));
+    }
+
+    irqflags = deposit32(irqflags, GIC_FDT_IRQ_PPI_CPU_START,
+                         GIC_FDT_IRQ_PPI_CPU_WIDTH, (1 << vbi->smp_cpus) - 1);
+
+    armcpu = ARM_CPU(qemu_get_cpu(0));
+    qemu_fdt_add_subnode(vbi->fdt, "/pmu");
+    if (arm_feature(&armcpu->env, ARM_FEATURE_V8)) {
+        const char compat[] = "arm,armv8-pmuv3";
+        qemu_fdt_setprop(vbi->fdt, "/pmu", "compatible",
+                         compat, sizeof(compat));
+        qemu_fdt_setprop_cells(vbi->fdt, "/pmu", "interrupts",
+                               GIC_FDT_IRQ_TYPE_PPI, VIRTUAL_PMU_IRQ, irqflags);
+    }
+}
+
 static void create_v2m(VirtBoardInfo *vbi, qemu_irq *pic)
 {
     int i;
@@ -1246,6 +1275,8 @@  static void machvirt_init(MachineState *machine)
 
     create_gic(vbi, pic, gic_version, vms->secure);
 
+    fdt_add_pmu_nodes(vbi);
+
     create_uart(vbi, pic, VIRT_UART, sysmem);
 
     if (vms->secure) {
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index ecd8589..b50f095 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -40,6 +40,10 @@ 
 #define ARCH_TIMER_NS_EL1_IRQ 14
 #define ARCH_TIMER_NS_EL2_IRQ 10
 
+#define VIRTUAL_PMU_IRQ 7
+
+#define PPI(irq) ((irq) + 16)
+
 enum {
     VIRT_FLASH,
     VIRT_MEM,
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 0e18f15..90c2c54 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -523,4 +523,5 @@  int kvm_set_one_reg(CPUState *cs, uint64_t id, void *source);
  * Returns: 0 on success, or a negative errno on failure.
  */
 int kvm_get_one_reg(CPUState *cs, uint64_t id, void *target);
+void kvm_arm_pmu_create(CPUState *cs, int irq);
 #endif
diff --git a/stubs/kvm.c b/stubs/kvm.c
index ddd6204..58a348a 100644
--- a/stubs/kvm.c
+++ b/stubs/kvm.c
@@ -6,3 +6,8 @@  int kvm_arch_irqchip_create(MachineState *ms, KVMState *s)
 {
     return 0;
 }
+
+void kvm_arm_pmu_create(CPUState *cs, int irq)
+{
+    return;
+}
diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
index b364789..faec4fa 100644
--- a/target-arm/kvm64.c
+++ b/target-arm/kvm64.c
@@ -382,6 +382,45 @@  static CPUWatchpoint *find_hw_watchpoint(CPUState *cpu, target_ulong addr)
     return NULL;
 }
 
+static bool kvm_arm_pmu_support_ctrl(CPUState *cs, struct kvm_device_attr *attr)
+{
+    return kvm_vcpu_ioctl(cs, KVM_HAS_DEVICE_ATTR, attr) == 0;
+}
+
+void kvm_arm_pmu_create(CPUState *cs, int irq)
+{
+    int err;
+
+    struct kvm_device_attr attr = {
+        .group = KVM_ARM_VCPU_PMU_V3_CTRL,
+        .addr = (intptr_t)&irq,
+        .attr = KVM_ARM_VCPU_PMU_V3_IRQ,
+        .flags = 0,
+    };
+
+    if (!kvm_arm_pmu_support_ctrl(cs, &attr)) {
+        return;
+    }
+
+    err = kvm_vcpu_ioctl(cs, KVM_SET_DEVICE_ATTR, &attr);
+    if (err < 0) {
+        fprintf(stderr, "KVM_{SET/GET}_DEVICE_ATTR failed: %s\n",
+                strerror(-err));
+        abort();
+    }
+
+    attr.group = KVM_ARM_VCPU_PMU_V3_CTRL;
+    attr.attr = KVM_ARM_VCPU_PMU_V3_INIT;
+    attr.addr = 0;
+    attr.flags = 0;
+
+    err = kvm_vcpu_ioctl(cs, KVM_SET_DEVICE_ATTR, &attr);
+    if (err < 0) {
+        fprintf(stderr, "KVM_{SET/GET}_DEVICE_ATTR failed: %s\n",
+                strerror(-err));
+        abort();
+    }
+}
 
 static inline void set_feature(uint64_t *features, int feature)
 {