On Wed, Jul 19, 2017 at 09:39:57AM -0400, Andrew Jones wrote: > If a KVM PMU init or set-irq attr call fails we just silently stop > the PMU DT node generation. The only way they could fail, though, > is if the attr's respective KVM has-attr call fails. But that should > never happen if KVM advertises the PMU capability, because both > attrs have been available since the capability was introduced. Let's > just abort if this should-never-happen stuff does happen, because, > if it does, then something is obviously horribly wrong. > > Signed-off-by: Andrew Jones <drjo...@redhat.com>
Reviewed-by: Christoffer Dall <cd...@linaro.org> > --- > hw/arm/virt.c | 9 +++------ > target/arm/kvm32.c | 3 +-- > target/arm/kvm64.c | 28 ++++++++++++++++++++-------- > target/arm/kvm_arm.h | 15 ++++----------- > 4 files changed, 28 insertions(+), 27 deletions(-) > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > index a215330444da..4bc50964d52b 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -496,13 +496,10 @@ static void fdt_add_pmu_nodes(const VirtMachineState > *vms) > return; > } > if (kvm_enabled()) { > - if (kvm_irqchip_in_kernel() && > - !kvm_arm_pmu_set_irq(cpu, PPI(VIRTUAL_PMU_IRQ))) { > - return; > - } > - if (!kvm_arm_pmu_init(cpu)) { > - return; > + if (kvm_irqchip_in_kernel()) { > + kvm_arm_pmu_set_irq(cpu, PPI(VIRTUAL_PMU_IRQ)); > } > + kvm_arm_pmu_init(cpu); > } > } > > diff --git a/target/arm/kvm32.c b/target/arm/kvm32.c > index e3aab89a1a94..717a2562670b 100644 > --- a/target/arm/kvm32.c > +++ b/target/arm/kvm32.c > @@ -522,10 +522,9 @@ bool kvm_arm_hw_debug_active(CPUState *cs) > return false; > } > > -int kvm_arm_pmu_set_irq(CPUState *cs, int irq) > +void kvm_arm_pmu_set_irq(CPUState *cs, int irq) > { > qemu_log_mask(LOG_UNIMP, "%s: not implemented\n", __func__); > - return 0; > } > > int kvm_arm_pmu_init(CPUState *cs) > diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c > index ec7d85314acc..6554c30007a4 100644 > --- a/target/arm/kvm64.c > +++ b/target/arm/kvm64.c > @@ -387,30 +387,36 @@ static bool kvm_arm_pmu_set_attr(CPUState *cs, struct > kvm_device_attr *attr) > > err = kvm_vcpu_ioctl(cs, KVM_HAS_DEVICE_ATTR, attr); > if (err != 0) { > + error_report("PMU: KVM_HAS_DEVICE_ATTR: %s", strerror(-err)); > return false; > } > > err = kvm_vcpu_ioctl(cs, KVM_SET_DEVICE_ATTR, attr); > - if (err < 0) { > - fprintf(stderr, "KVM_SET_DEVICE_ATTR failed: %s\n", > - strerror(-err)); > - abort(); > + if (err != 0) { > + error_report("PMU: KVM_SET_DEVICE_ATTR: %s", strerror(-err)); > + return false; > } > > return true; > } > > -int kvm_arm_pmu_init(CPUState *cs) > +void kvm_arm_pmu_init(CPUState *cs) > { > struct kvm_device_attr attr = { > .group = KVM_ARM_VCPU_PMU_V3_CTRL, > .attr = KVM_ARM_VCPU_PMU_V3_INIT, > }; > > - return kvm_arm_pmu_set_attr(cs, &attr); > + if (!ARM_CPU(cs)->has_pmu) { > + return; > + } > + if (!kvm_arm_pmu_set_attr(cs, &attr)) { > + error_report("failed to init PMU"); > + abort(); > + } > } > > -int kvm_arm_pmu_set_irq(CPUState *cs, int irq) > +void kvm_arm_pmu_set_irq(CPUState *cs, int irq) > { > struct kvm_device_attr attr = { > .group = KVM_ARM_VCPU_PMU_V3_CTRL, > @@ -418,7 +424,13 @@ int kvm_arm_pmu_set_irq(CPUState *cs, int irq) > .attr = KVM_ARM_VCPU_PMU_V3_IRQ, > }; > > - return kvm_arm_pmu_set_attr(cs, &attr); > + if (!ARM_CPU(cs)->has_pmu) { > + return; > + } > + if (!kvm_arm_pmu_set_attr(cs, &attr)) { > + error_report("failed to set irq for PMU"); > + abort(); > + } > } > > static inline void set_feature(uint64_t *features, int feature) > diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h > index cab5ea9be55c..ff53e9fafb7a 100644 > --- a/target/arm/kvm_arm.h > +++ b/target/arm/kvm_arm.h > @@ -195,8 +195,8 @@ int kvm_arm_sync_mpstate_to_qemu(ARMCPU *cpu); > > int kvm_arm_vgic_probe(void); > > -int kvm_arm_pmu_set_irq(CPUState *cs, int irq); > -int kvm_arm_pmu_init(CPUState *cs); > +void kvm_arm_pmu_set_irq(CPUState *cs, int irq); > +void kvm_arm_pmu_init(CPUState *cs); > > #else > > @@ -205,15 +205,8 @@ static inline int kvm_arm_vgic_probe(void) > return 0; > } > > -static inline int kvm_arm_pmu_set_irq(CPUState *cs, int irq) > -{ > - return 0; > -} > - > -static inline int kvm_arm_pmu_init(CPUState *cs) > -{ > - return 0; > -} > +static inline void kvm_arm_pmu_set_irq(CPUState *cs, int irq) {} > +static inline void kvm_arm_pmu_init(CPUState *cs) {} > > #endif > > -- > 1.8.3.1 >