On 03/12/12 13:25, Will Deacon wrote:
> On Sat, Nov 10, 2012 at 03:45:18PM +0000, Christoffer Dall wrote:
>> From: Marc Zyngier <marc.zyngier@xxxxxxx>
>>
>> Plug the interrupt injection code. Interrupts can now be generated
>> from user space.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>
>> Signed-off-by: Christoffer Dall <c.dall@xxxxxxxxxxxxxxxxxxxxxx>
>> ---
>> arch/arm/include/asm/kvm_vgic.h | 8 +++
>> arch/arm/kvm/arm.c | 29 +++++++++++++
>> arch/arm/kvm/vgic.c | 90 +++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 127 insertions(+)
>>
>> diff --git a/arch/arm/include/asm/kvm_vgic.h b/arch/arm/include/asm/kvm_vgic.h
>> index 7229324..6e3d303 100644
>> --- a/arch/arm/include/asm/kvm_vgic.h
>> +++ b/arch/arm/include/asm/kvm_vgic.h
>> @@ -241,6 +241,8 @@ struct kvm_exit_mmio;
>> int kvm_vgic_set_addr(struct kvm *kvm, unsigned long type, u64 addr);
>> void kvm_vgic_sync_to_cpu(struct kvm_vcpu *vcpu);
>> void kvm_vgic_sync_from_cpu(struct kvm_vcpu *vcpu);
>> +int kvm_vgic_inject_irq(struct kvm *kvm, int cpuid, unsigned int irq_num,
>> + bool level);
>> int kvm_vgic_vcpu_pending_irq(struct kvm_vcpu *vcpu);
>> bool vgic_handle_mmio(struct kvm_vcpu *vcpu, struct kvm_run *run,
>> struct kvm_exit_mmio *mmio);
>> @@ -271,6 +273,12 @@ static inline void kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu) {}
>> static inline void kvm_vgic_sync_to_cpu(struct kvm_vcpu *vcpu) {}
>> static inline void kvm_vgic_sync_from_cpu(struct kvm_vcpu *vcpu) {}
>>
>> +static inline int kvm_vgic_inject_irq(struct kvm *kvm, int cpuid,
>> + const struct kvm_irq_level *irq)
>> +{
>> + return 0;
>> +}
>> +
>> static inline int kvm_vgic_vcpu_pending_irq(struct kvm_vcpu *vcpu)
>> {
>> return 0;
>> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
>> index 3ac1aab..f43da01 100644
>> --- a/arch/arm/kvm/arm.c
>> +++ b/arch/arm/kvm/arm.c
>> @@ -764,10 +764,31 @@ int kvm_vm_ioctl_irq_line(struct kvm *kvm, struct kvm_irq_level *irq_level)
>>
>> switch (irq_type) {
>> case KVM_ARM_IRQ_TYPE_CPU:
>> + if (irqchip_in_kernel(kvm))
>> + return -ENXIO;
>> +
>> if (irq_num > KVM_ARM_IRQ_CPU_FIQ)
>> return -EINVAL;
>>
>> return vcpu_interrupt_line(vcpu, irq_num, level);
>> +#ifdef CONFIG_KVM_ARM_VGIC
>> + case KVM_ARM_IRQ_TYPE_PPI:
>> + if (!irqchip_in_kernel(kvm))
>> + return -ENXIO;
>> +
>> + if (irq_num < 16 || irq_num > 31)
>> + return -EINVAL;
>
> It's our favourite two numbers again! :)
I already fixed a number of them. Probably missed this one though.
>> +
>> + return kvm_vgic_inject_irq(kvm, vcpu->vcpu_id, irq_num, level);
>> + case KVM_ARM_IRQ_TYPE_SPI:
>> + if (!irqchip_in_kernel(kvm))
>> + return -ENXIO;
>> +
>> + if (irq_num < 32 || irq_num > KVM_ARM_IRQ_GIC_MAX)
>> + return -EINVAL;
>> +
>> + return kvm_vgic_inject_irq(kvm, 0, irq_num, level);
>> +#endif
>> }
>>
>> return -EINVAL;
>> @@ -849,6 +870,14 @@ long kvm_arch_vm_ioctl(struct file *filp,
>> void __user *argp = (void __user *)arg;
>>
>> switch (ioctl) {
>> +#ifdef CONFIG_KVM_ARM_VGIC
>> + case KVM_CREATE_IRQCHIP: {
>> + if (vgic_present)
>> + return kvm_vgic_create(kvm);
>> + else
>> + return -EINVAL;
>
> ENXIO? At least, that's what you use when setting the GIC addresses.
-EINVAL seems to be one of the values other archs are using. -ENXIO is
not one of them for KVM_CREATE_IRQCHIP. Doesn't mean they are right, but
for the sake of keeping userspace happy, I'm not really inclined to
change this.
Christoffer?
>> + }
>> +#endif
>> case KVM_SET_DEVICE_ADDRESS: {
>> struct kvm_device_address dev_addr;
>>
>> diff --git a/arch/arm/kvm/vgic.c b/arch/arm/kvm/vgic.c
>> index dda5623..70040bb 100644
>> --- a/arch/arm/kvm/vgic.c
>> +++ b/arch/arm/kvm/vgic.c
>> @@ -75,6 +75,7 @@
>> #define ACCESS_WRITE_MASK(x) ((x) & (3 << 1))
>>
>> static void vgic_update_state(struct kvm *kvm);
>> +static void vgic_kick_vcpus(struct kvm *kvm);
>> static void vgic_dispatch_sgi(struct kvm_vcpu *vcpu, u32 reg);
>>
>> static inline int vgic_irq_is_edge(struct vgic_dist *dist, int irq)
>> @@ -542,6 +543,9 @@ bool vgic_handle_mmio(struct kvm_vcpu *vcpu, struct kvm_run *run, struct kvm_exi
>> kvm_prepare_mmio(run, mmio);
>> kvm_handle_mmio_return(vcpu, run);
>>
>> + if (updated_state)
>> + vgic_kick_vcpus(vcpu->kvm);
>> +
>> return true;
>> }
>>
>> @@ -867,6 +871,92 @@ int kvm_vgic_vcpu_pending_irq(struct kvm_vcpu *vcpu)
>> return test_bit(vcpu->vcpu_id, &dist->irq_pending_on_cpu);
>> }
>>
>> +static void vgic_kick_vcpus(struct kvm *kvm)
>> +{
>> + struct kvm_vcpu *vcpu;
>> + int c;
>> +
>> + /*
>> + * We've injected an interrupt, time to find out who deserves
>> + * a good kick...
>> + */
>> + kvm_for_each_vcpu(c, vcpu, kvm) {
>> + if (kvm_vgic_vcpu_pending_irq(vcpu))
>> + kvm_vcpu_kick(vcpu);
>> + }
>> +}
>> +
>> +static bool vgic_update_irq_state(struct kvm *kvm, int cpuid,
>> + unsigned int irq_num, bool level)
>> +{
>> + struct vgic_dist *dist = &kvm->arch.vgic;
>> + struct kvm_vcpu *vcpu;
>> + int is_edge, is_level, state;
>> + int enabled;
>> + bool ret = true;
>> +
>> + spin_lock(&dist->lock);
>> +
>> + is_edge = vgic_irq_is_edge(dist, irq_num);
>> + is_level = !is_edge;
>> + state = vgic_bitmap_get_irq_val(&dist->irq_state, cpuid, irq_num);
>> +
>> + /*
>> + * Only inject an interrupt if:
>> + * - level triggered and we change level
>> + * - edge triggered and we have a rising edge
>> + */
>> + if ((is_level && !(state ^ level)) || (is_edge && (state || !level))) {
>> + ret = false;
>> + goto out;
>> + }
>
> Eek, more of the edge/level combo. Can this be be restructured so that we
> have vgic_update_{edge,level}_irq_state, which are called from here
> appropriately?
I'll have a look.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
