[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-arm] [Qemu-devel] [PATCH 2/3] hw/arm/virt: Add PMU node for vi
From: |
Andrew Jones |
Subject: |
Re: [Qemu-arm] [Qemu-devel] [PATCH 2/3] hw/arm/virt: Add PMU node for virt machine |
Date: |
Fri, 22 Apr 2016 16:32:33 +0200 |
User-agent: |
Mutt/1.5.23.1 (2014-03-12) |
On Fri, Mar 25, 2016 at 05:46:20PM +0800, Shannon Zhao wrote:
> From: Shannon Zhao <address@hidden>
>
> Add a virtual PMU device for virt machine while use PPI 7 for PMU
> overflow interrupt number.
>
> Signed-off-by: Shannon Zhao <address@hidden>
> ---
> hw/arm/virt.c | 31 +++++++++++++++++++++++++++++++
> include/hw/arm/virt.h | 2 ++
> include/sysemu/kvm.h | 1 +
> stubs/kvm.c | 5 +++++
> target-arm/kvm64.c | 51
> +++++++++++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 90 insertions(+)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 95331a5..94c2beb 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -427,6 +427,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;
funny indentation here
> + }
> +
> + kvm_arm_pmu_create(cpu, VIRTUAL_PMU_IRQ + 16);
I think we should have a PPI(irq) ((irq) + 16) type of macro.
> + }
> +
> + 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);
> + }
else what? I guess it's not possible to have has_pmu and !ARM_FEATURE_V8
at the same time right now, but it seems strange to create a /pmu node,
but then only conditionally populate it.
> +}
> +
> static void create_v2m(VirtBoardInfo *vbi, qemu_irq *pic)
> {
> int i;
> @@ -1242,6 +1271,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..864eb49 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -40,6 +40,8 @@
> #define ARCH_TIMER_NS_EL1_IRQ 14
> #define ARCH_TIMER_NS_EL2_IRQ 10
>
> +#define VIRTUAL_PMU_IRQ 7
Can we find a way to make this configurable? a cpu property?
> +
> enum {
> VIRT_FLASH,
> VIRT_MEM,
> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> index 6695fa7..80b6cb3 100644
> --- a/include/sysemu/kvm.h
> +++ b/include/sysemu/kvm.h
> @@ -514,4 +514,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..b97b9ef 100644
> --- a/target-arm/kvm64.c
> +++ b/target-arm/kvm64.c
> @@ -382,6 +382,57 @@ 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;
> +}
> +
> +static void kvm_arm_pmu_init(CPUState *cs)
> +{
> + int err;
> +
> + struct kvm_device_attr attr = {
> + .group = KVM_ARM_VCPU_PMU_V3_CTRL,
> + .attr = KVM_ARM_VCPU_PMU_V3_INIT,
> + .flags = 0,
> + };
> +
> + if (!kvm_arm_pmu_support_ctrl(cs, &attr)) {
> + return;
> + }
I don't think we need to do this check again here. kvm_arm_pmu_init is
only called from a function that already checked for the IRQ attribute,
and both IRQ and INIT were added to the kernel at the same time.
Actually I think we could just opencode kvm_arm_pmu_init in
kvm_arm_pmu_create.
> +
> + 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();
> + }
> +}
> +
> +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();
> + }
> +
> + kvm_arm_pmu_init(cs);
> +}
>
> static inline void set_feature(uint64_t *features, int feature)
> {
> --
> 2.0.4
>
>
>
Thanks,
drew
- Re: [Qemu-arm] [Qemu-devel] [PATCH 2/3] hw/arm/virt: Add PMU node for virt machine,
Andrew Jones <=