Hi Andrew,
On 2020-09-08 10:53, Andrew Jones wrote:
Hi Marc,
On Tue, Sep 08, 2020 at 08:58:26AM +0100, Marc Zyngier wrote:
The PMU emulation error handling is pretty messy when dealing with
attributes. Let's refactor it so that we have less duplication,
and that it is easy to extend later on.
A functional change is that kvm_arm_pmu_v3_init() used to return
-ENXIO when the PMU feature wasn't set. The error is now reported
as -ENODEV, matching the documentation.
Hmm, I didn't think we could make changes like that, since some
userspace
somewhere may now depend on the buggy interface.
Well, this is the whole point of this patch: discussing whether
this change is acceptable or whether existing VMMs are relying
on such behaviour. We *could* leave it as is, but I thought I'd
bring it up!
That said, I'm not really
against the change, but maybe it should go as a separate patch.
Sure, why not.
-ENXIO is still returned
when the interrupt isn't properly configured.
Signed-off-by: Marc Zyngier <maz@xxxxxxxxxx>
---
arch/arm64/kvm/pmu-emul.c | 21 +++++++++------------
1 file changed, 9 insertions(+), 12 deletions(-)
diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c
index f0d0312c0a55..93d797df42c6 100644
--- a/arch/arm64/kvm/pmu-emul.c
+++ b/arch/arm64/kvm/pmu-emul.c
@@ -735,15 +735,6 @@ int kvm_arm_pmu_v3_enable(struct kvm_vcpu *vcpu)
static int kvm_arm_pmu_v3_init(struct kvm_vcpu *vcpu)
{
- if (!kvm_arm_support_pmu_v3())
- return -ENODEV;
-
- if (!test_bit(KVM_ARM_VCPU_PMU_V3, vcpu->arch.features))
- return -ENXIO;
-
- if (vcpu->arch.pmu.created)
- return -EBUSY;
-
if (irqchip_in_kernel(vcpu->kvm)) {
int ret;
@@ -796,6 +787,15 @@ static bool pmu_irq_is_valid(struct kvm *kvm, int
irq)
int kvm_arm_pmu_v3_set_attr(struct kvm_vcpu *vcpu, struct
kvm_device_attr *attr)
{
+ if (!kvm_arm_support_pmu_v3())
+ return -ENODEV;
+
+ if (!test_bit(KVM_ARM_VCPU_PMU_V3, vcpu->arch.features))
+ return -ENODEV;
nit: could combine these two if's w/ an ||
This was made to make the userspace visible change obvious to the
reviewer. Now that you have noticed it, I'm happy to merge these
two! ;-)
Thanks,
M.
--
Jazz is not dead. It just smells funny...
