On Tue, May 23, 2017 at 05:52:01PM +0100, Marc Zyngier wrote: > On 16/05/17 19:45, Christoffer Dall wrote: > > Since we got support for devices in userspace which allows reporting the > > PMU overflow output status to userspace, we should actually allow > > creating the PMU on systems without an in-kernel irqchip, which in turn > > requires us to slightly clarify error codes for the ABI and move things > > around for the initialization phase. > > > > Signed-off-by: Christoffer Dall <[email protected]> > > --- > > Documentation/virtual/kvm/devices/vcpu.txt | 16 +++++++++------- > > virt/kvm/arm/pmu.c | 30 > > ++++++++++++++++++++---------- > > 2 files changed, 29 insertions(+), 17 deletions(-) > > > > diff --git a/Documentation/virtual/kvm/devices/vcpu.txt > > b/Documentation/virtual/kvm/devices/vcpu.txt > > index 02f5068..352af6e 100644 > > --- a/Documentation/virtual/kvm/devices/vcpu.txt > > +++ b/Documentation/virtual/kvm/devices/vcpu.txt > > @@ -16,7 +16,9 @@ Parameters: in kvm_device_attr.addr the address for PMU > > overflow interrupt is a > > Returns: -EBUSY: The PMU overflow interrupt is already set > > -ENXIO: The overflow interrupt not set when attempting to get it > > -ENODEV: PMUv3 not supported > > - -EINVAL: Invalid PMU overflow interrupt number supplied > > + -EINVAL: Invalid PMU overflow interrupt number supplied or > > + trying to set the IRQ number without using an in-kernel > > + irqchip. > > > > A value describing the PMUv3 (Performance Monitor Unit v3) overflow > > interrupt > > number for this vcpu. This interrupt could be a PPI or SPI, but the > > interrupt > > @@ -25,11 +27,11 @@ all vcpus, while as an SPI it must be a separate number > > per vcpu. > > > > 1.2 ATTRIBUTE: KVM_ARM_VCPU_PMU_V3_INIT > > Parameters: no additional parameter in kvm_device_attr.addr > > -Returns: -ENODEV: PMUv3 not supported > > - -ENXIO: PMUv3 not properly configured as required prior to > > calling this > > - attribute > > +Returns: -ENODEV: PMUv3 not supported or GIC not initialized > > + -ENXIO: PMUv3 not properly configured or in-kernel irqchip not > > + conigured as required prior to calling this attribute > > configured > > > -EBUSY: PMUv3 already initialized > > > > -Request the initialization of the PMUv3. This must be done after creating > > the > > -in-kernel irqchip. Creating a PMU with a userspace irqchip is currently > > not > > -supported. > > +Request the initialization of the PMUv3. If using the PMUv3 with an > > in-kernel > > +virtual GIC implementation, this must be done after initializing the > > in-kernel > > +irqchip. > > diff --git a/virt/kvm/arm/pmu.c b/virt/kvm/arm/pmu.c > > index 4b43e7f..7209185 100644 > > --- a/virt/kvm/arm/pmu.c > > +++ b/virt/kvm/arm/pmu.c > > @@ -456,21 +456,25 @@ static int kvm_arm_pmu_v3_init(struct kvm_vcpu *vcpu) > > if (!kvm_arm_support_pmu_v3()) > > return -ENODEV; > > > > - /* > > - * We currently require an in-kernel VGIC to use the PMU emulation, > > - * because we do not support forwarding PMU overflow interrupts to > > - * userspace yet. > > - */ > > - if (!irqchip_in_kernel(vcpu->kvm) || !vgic_initialized(vcpu->kvm)) > > - return -ENODEV; > > - > > - if (!test_bit(KVM_ARM_VCPU_PMU_V3, vcpu->arch.features) || > > - !kvm_arm_pmu_irq_initialized(vcpu)) > > + if (!test_bit(KVM_ARM_VCPU_PMU_V3, vcpu->arch.features)) > > return -ENXIO; > > > > if (kvm_arm_pmu_v3_ready(vcpu)) > > return -EBUSY; > > > > + if (irqchip_in_kernel(vcpu->kvm)) { > > + /* > > + * If using the PMU with an in-kernel virtual GIC > > + * implementation, we require the GIC to be already > > + * initialized when initializing the PMU. > > + */ > > + if (!vgic_initialized(vcpu->kvm)) > > + return -ENODEV; > > + > > + if (!kvm_arm_pmu_irq_initialized(vcpu)) > > + return -ENXIO; > > + } > > + > > Do we also need to prevent a vgic to be created if the PMU has been > initialized beforehand? >
Sigh. We probably have to. I don't like having a cross-VGIC-PMU check, but we could do something like setting a flag on the kvm struct so that irqchip_in_user() always return true, and if that is set, it is not possible to create the VGIC. Alternatively we can make the PMU init a no-op, and try to enable it via the first-vcpu-run path, like the timer, and check that everything lines up then (i.e. you have in-kernel irqchip with a non-conflicting irq number or you have a userspace irqchip). Thougths? Thanks, -Christoffer _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
