| Message ID | 1461405855-15576-3-git-send-email-zhaoshenglong@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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. Thanks,
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 --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) {