On 2016/1/7 21:56, Marc Zyngier wrote:

On 22/12/15 08:08, Shannon Zhao wrote:

quoted

quoted

From: Shannon Zhao <redacted>

Add a new kvm device type KVM_DEV_TYPE_ARM_PMU_V3 for ARM PMU. Implement
the kvm_device_ops for it.

Signed-off-by: Shannon Zhao <redacted>
---
 Documentation/virtual/kvm/devices/arm-pmu.txt |  24 +++++
 arch/arm64/include/uapi/asm/kvm.h             |   4 +
 include/linux/kvm_host.h                      |   1 +
 include/uapi/linux/kvm.h                      |   2 +
 virt/kvm/arm/pmu.c                            | 128 ++++++++++++++++++++++++++
 virt/kvm/kvm_main.c                           |   4 +
 6 files changed, 163 insertions(+)
 create mode 100644 Documentation/virtual/kvm/devices/arm-pmu.txt

diff --git a/Documentation/virtual/kvm/devices/arm-pmu.txt b/Documentation/virtual/kvm/devices/arm-pmu.txt
new file mode 100644
index 0000000..dda864e
--- /dev/null
+++ b/Documentation/virtual/kvm/devices/arm-pmu.txt

@@ -0,0 +1,24 @@
+ARM Virtual Performance Monitor Unit (vPMU)
+===========================================
+
+Device types supported:
+  KVM_DEV_TYPE_ARM_PMU_V3         ARM Performance Monitor Unit v3
+
+Instantiate one PMU instance for per VCPU through this API.
+
+Groups:
+  KVM_DEV_ARM_PMU_GRP_IRQ
+  Attributes:
+    The attr field of kvm_device_attr encodes one value:
+    bits:     | 63 .... 32 | 31 ....  0 |
+    values:   |  reserved  | vcpu_index |
+    A value describing the PMU overflow interrupt number for the specified
+    vcpu_index vcpu. This interrupt could be a PPI or SPI, but for one VM the
+    interrupt type must be same for each vcpu. As a PPI, the interrupt number is
+    same for all vcpus, while as a SPI it must be different for each vcpu.

I don't see anything enforcing these restrictions in the code (you can
program different PPIs on each vcpu, or the same SPI everywhere, and
nothing will generate an error).

Is that something we want to enforce? Or we're happy just to leave it as
an unsupported corner case?

Yeah, it should add the check. How about below check?

diff --git a/virt/kvm/arm/pmu.c b/virt/kvm/arm/pmu.c
index 0ccf273..c3347e9 100644
--- a/virt/kvm/arm/pmu.c
+++ b/virt/kvm/arm/pmu.c

@@ -435,6 +435,29 @@ static void kvm_arm_pmu_destroy(struct kvm_device *dev)
        kfree(dev);
 }

+static bool irq_is_invalid(kvm_device *dev, int irq, bool is_ppi)
+{
+       int i;
+       struct kvm_vcpu *vcpu;
+       struct kvm *kvm = dev->kvm;
+
+       kvm_for_each_vcpu(i, vcpu, kvm) {
+               struct kvm_pmu *pmu = &vcpu->arch.pmu;
+
+               if (!kvm_arm_pmu_initialized(vcpu))
+                       continue;
+
+               if (is_ppi)
+                       if (pmu->irq_num != irq)
+                               return true;
+               else
+                       if (pmu->irq_num == irq)
+                               return true;
+       }
+
+       return false;
+}
+
 static int kvm_arm_pmu_set_attr(struct kvm_device *dev,
                                struct kvm_device_attr *attr)
 {

@@ -452,7 +475,8 @@ static int kvm_arm_pmu_set_attr(struct kvm_device *dev,
                 * the interrupt number is same for all vcpus, while as

a SPI it
                 * must be different for each vcpu.
                 */
-               if (reg < VGIC_NR_SGIS || reg >=
dev->kvm->arch.vgic.nr_irqs)
+               if (reg < VGIC_NR_SGIS || reg >=
dev->kvm->arch.vgic.nr_irqs ||
+                   irq_is_invalid(dev, reg, reg < VGIC_NR_PRIVATE_IRQS))
                        return -EINVAL;

                return kvm_arm_pmu_irq_access(dev->kvm, attr, &reg, true);

Thanks,
-- 
Shannon