| Message ID | 20110806103933.27198.91264.stgit@localhost6.localdomain6 (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 08/06/2011 01:39 PM, Christoffer Dall wrote: > Userspace can inject IRQs and FIQs through the KVM_IRQ_LINE VM ioctl. > This ioctl is used since the sematics are in fact two lines that can be > either raised or lowered on the VCPU - the IRQ and FIQ lines. > > KVM needs to know which VCPU it must operate on and whether the FIQ or > IRQ line is raised/lowered. Hence both pieces of information is packed > in the kvm_irq_level->irq field. The irq fild value will be: > IRQ: vcpu_index * 2 > FIQ: (vcpu_index * 2) + 1 > > This is documented in Documentation/kvm/api.txt. > > The effect of the ioctl is simply to simply raise/lower the > corresponding virt_irq field on the VCPU struct, which will cause the > world-switch code to raise/lower virtual interrupts when running the > guest on next switch. The wait_for_interrupt flag is also cleared for > raised IRQs causing an idle VCPU to become active again. Note x86 starts out with a default configuration and allows updating it via KVM_SET_GSI_ROUTING. You may need this in the future if you decide to implement an irq controller in the kernel. > +static int kvm_arch_vm_ioctl_irq_line(struct kvm *kvm, > + struct kvm_irq_level *irq_level) > +{ > + u32 mask; > + unsigned int vcpu_idx; > + struct kvm_vcpu *vcpu; > + > + vcpu_idx = irq_level->irq / 2; > + if (vcpu_idx>= KVM_MAX_VCPUS) > + return -EINVAL; > + > + vcpu = kvm_get_vcpu(kvm, vcpu_idx); > + if (!vcpu) > + return -EINVAL; > + > + switch (irq_level->irq % 2) { > + case KVM_ARM_IRQ_LINE: > + mask = HCR_VI; > + break; > + case KVM_ARM_FIQ_LINE: > + mask = HCR_VF; > + break; > + default: > + return -EINVAL; > + } > + > + trace_kvm_irq_line(irq_level->irq % 2, irq_level->level, vcpu_idx); Please reuse trace_kvm_set_irq(). You can decode vcpu/type in a trace-cmd plugin. > + > + if (irq_level->level) { > + vcpu->arch.virt_irq |= mask; > + vcpu->arch.wait_for_interrupts = 0; > + } else > + vcpu->arch.virt_irq&= ~mask; > + This seems to be non-smp-safe? Do you need atomic ops and barriers here? And a wakeup? Unlike KVM_INTERRUPT, KVM_IRQ_LINE is designed to be used asynchronously wrt the vcpu. > + return 0; > +} > + > long kvm_arch_vcpu_ioctl(struct file *filp, > unsigned int ioctl, unsigned long arg) > { > @@ -312,8 +349,21 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm, struct kvm_dirty_log *log) > long kvm_arch_vm_ioctl(struct file *filp, > unsigned int ioctl, unsigned long arg) > { > - printk(KERN_ERR "kvm_arch_vm_ioctl: Unsupported ioctl (%d)\n", ioctl); > - return -EINVAL; > + struct kvm *kvm = filp->private_data; > + void __user *argp = (void __user *)arg; > + > + switch (ioctl) { > + case KVM_IRQ_LINE: { > + struct kvm_irq_level irq_event; > + > + if (copy_from_user(&irq_event, argp, sizeof irq_event)) > + return -EFAULT; > + return kvm_arch_vm_ioctl_irq_line(kvm,&irq_event); > + } > + default: > + kvm_err(-EINVAL, "Unsupported ioctl (%d)", ioctl); Please remove for the final code, we don't want a user spamming the kernel log. > + return -EINVAL; > + } > } > >
On Aug 9, 2011, at 12:07 PM, Avi Kivity wrote: > On 08/06/2011 01:39 PM, Christoffer Dall wrote: >> Userspace can inject IRQs and FIQs through the KVM_IRQ_LINE VM ioctl. >> This ioctl is used since the sematics are in fact two lines that can be >> either raised or lowered on the VCPU - the IRQ and FIQ lines. >> >> KVM needs to know which VCPU it must operate on and whether the FIQ or >> IRQ line is raised/lowered. Hence both pieces of information is packed >> in the kvm_irq_level->irq field. The irq fild value will be: >> IRQ: vcpu_index * 2 >> FIQ: (vcpu_index * 2) + 1 >> >> This is documented in Documentation/kvm/api.txt. >> >> The effect of the ioctl is simply to simply raise/lower the >> corresponding virt_irq field on the VCPU struct, which will cause the >> world-switch code to raise/lower virtual interrupts when running the >> guest on next switch. The wait_for_interrupt flag is also cleared for >> raised IRQs causing an idle VCPU to become active again. > > Note x86 starts out with a default configuration and allows updating it > via KVM_SET_GSI_ROUTING. You may need this in the future if you decide > to implement an irq controller in the kernel. Will probably happen some time. Noted. > >> +static int kvm_arch_vm_ioctl_irq_line(struct kvm *kvm, >> + struct kvm_irq_level *irq_level) >> +{ >> + u32 mask; >> + unsigned int vcpu_idx; >> + struct kvm_vcpu *vcpu; >> + >> + vcpu_idx = irq_level->irq / 2; >> + if (vcpu_idx>= KVM_MAX_VCPUS) >> + return -EINVAL; >> + >> + vcpu = kvm_get_vcpu(kvm, vcpu_idx); >> + if (!vcpu) >> + return -EINVAL; >> + >> + switch (irq_level->irq % 2) { >> + case KVM_ARM_IRQ_LINE: >> + mask = HCR_VI; >> + break; >> + case KVM_ARM_FIQ_LINE: >> + mask = HCR_VF; >> + break; >> + default: >> + return -EINVAL; >> + } >> + >> + trace_kvm_irq_line(irq_level->irq % 2, irq_level->level, vcpu_idx); > > Please reuse trace_kvm_set_irq(). You can decode vcpu/type in a > trace-cmd plugin. OK > >> + >> + if (irq_level->level) { >> + vcpu->arch.virt_irq |= mask; >> + vcpu->arch.wait_for_interrupts = 0; >> + } else >> + vcpu->arch.virt_irq&= ~mask; >> + > > This seems to be non-smp-safe? Do you need atomic ops and barriers > here? And a wakeup? The whole thing is not SMP tested yet, so I took some shortcuts. I only recently got hold of a SMP model and SMP support will be a focus area for the next series. Thanks for pin-pointing this though. > > Unlike KVM_INTERRUPT, KVM_IRQ_LINE is designed to be used asynchronously > wrt the vcpu. > >> + return 0; >> +} >> + >> long kvm_arch_vcpu_ioctl(struct file *filp, >> unsigned int ioctl, unsigned long arg) >> { >> @@ -312,8 +349,21 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm, struct kvm_dirty_log *log) >> long kvm_arch_vm_ioctl(struct file *filp, >> unsigned int ioctl, unsigned long arg) >> { >> - printk(KERN_ERR "kvm_arch_vm_ioctl: Unsupported ioctl (%d)\n", ioctl); >> - return -EINVAL; >> + struct kvm *kvm = filp->private_data; >> + void __user *argp = (void __user *)arg; >> + >> + switch (ioctl) { >> + case KVM_IRQ_LINE: { >> + struct kvm_irq_level irq_event; >> + >> + if (copy_from_user(&irq_event, argp, sizeof irq_event)) >> + return -EFAULT; >> + return kvm_arch_vm_ioctl_irq_line(kvm,&irq_event); >> + } >> + default: >> + kvm_err(-EINVAL, "Unsupported ioctl (%d)", ioctl); > > Please remove for the final code, we don't want a user spamming the > kernel log. OK. Good point. > >> + return -EINVAL; >> + } >> } >> >> > > -- > error compiling committee.c: too many arguments to function > > _______________________________________________ > Android-virt mailing list > Android-virt@lists.cs.columbia.edu > https://lists.cs.columbia.edu/cucslists/listinfo/android-virt -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/09/2011 02:27 PM, Christoffer Dall wrote: > > > >> + > >> + if (irq_level->level) { > >> + vcpu->arch.virt_irq |= mask; > >> + vcpu->arch.wait_for_interrupts = 0; > >> + } else > >> + vcpu->arch.virt_irq&= ~mask; > >> + > > > > This seems to be non-smp-safe? Do you need atomic ops and barriers > > here? And a wakeup? > > The whole thing is not SMP tested yet, so I took some shortcuts. I only recently got hold of a SMP model and SMP support will be a focus area for the next series. Thanks for pin-pointing this though. Note even a single vcpu guest on an smp host needs this.
On Aug 9, 2011, at 1:37 PM, Avi Kivity wrote: > On 08/09/2011 02:27 PM, Christoffer Dall wrote: >> > >> >> + >> >> + if (irq_level->level) { >> >> + vcpu->arch.virt_irq |= mask; >> >> + vcpu->arch.wait_for_interrupts = 0; >> >> + } else >> >> + vcpu->arch.virt_irq&= ~mask; >> >> + >> > >> > This seems to be non-smp-safe? Do you need atomic ops and barriers >> > here? And a wakeup? >> >> The whole thing is not SMP tested yet, so I took some shortcuts. I only recently got hold of a SMP model and SMP support will be a focus area for the next series. Thanks for pin-pointing this though. > > Note even a single vcpu guest on an smp host needs this. yep, I am aware. It's on my to-do list. Thanks. > > -- > error compiling committee.c: too many arguments to function > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/kvm/api.txt b/Documentation/kvm/api.txt index 9bef4e4..1ed5554 100644 --- a/Documentation/kvm/api.txt +++ b/Documentation/kvm/api.txt @@ -534,15 +534,20 @@ only go to the IOAPIC. On ia64, a IOSAPIC is created. 4.25 KVM_IRQ_LINE Capability: KVM_CAP_IRQCHIP -Architectures: x86, ia64 +Architectures: x86, ia64, arm Type: vm ioctl Parameters: struct kvm_irq_level Returns: 0 on success, -1 on error Sets the level of a GSI input to the interrupt controller model in the kernel. Requires that an interrupt controller model has been previously created with -KVM_CREATE_IRQCHIP. Note that edge-triggered interrupts require the level -to be set to 1 and then back to 0. +KVM_CREATE_IRQCHIP (except for ARM). Note that edge-triggered interrupts +require the level to be set to 1 and then back to 0. + +ARM uses two types of interrupt lines per CPU, ie. IRQ and FIQ. The value of the +irq field should be (VCPU_INDEX * 2) for IRQs and ((VCPU_INDEX * 2) + 1) for +FIQs. Level is used to raise/lower the line. See arch/arm/include/asm/kvm.h for +convenience macros. struct kvm_irq_level { union { diff --git a/arch/arm/include/asm/kvm.h b/arch/arm/include/asm/kvm.h index 87dc33b..8935062 100644 --- a/arch/arm/include/asm/kvm.h +++ b/arch/arm/include/asm/kvm.h @@ -20,6 +20,14 @@ #include <asm/types.h> /* + * KVM_IRQ_LINE macros to set/read IRQ/FIQ for specific VCPU index. + */ +enum KVM_ARM_IRQ_LINE_TYPE { + KVM_ARM_IRQ_LINE = 0, + KVM_ARM_FIQ_LINE = 1, +}; + +/* * Modes used for short-hand mode determinition in the world-switch code and * in emulation code. * diff --git a/arch/arm/include/asm/kvm_arm.h b/arch/arm/include/asm/kvm_arm.h index 835abd1..e378a37 100644 --- a/arch/arm/include/asm/kvm_arm.h +++ b/arch/arm/include/asm/kvm_arm.h @@ -49,6 +49,7 @@ #define HCR_VM 1 #define HCR_GUEST_MASK (HCR_TSC | HCR_TWE | HCR_TWI | HCR_VM | HCR_AMO | \ HCR_AMO | HCR_IMO | HCR_FMO | HCR_SWIO) +#define HCR_VIRT_EXCP_MASK (HCR_VA | HCR_VI | HCR_VF) /* Hyp System Control Register (HSCTLR) bits */ #define HSCTLR_TE (1 << 30) diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c index 3db6794..071912e 100644 --- a/arch/arm/kvm/arm.c +++ b/arch/arm/kvm/arm.c @@ -297,6 +297,43 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run) return -EINVAL; } +static int kvm_arch_vm_ioctl_irq_line(struct kvm *kvm, + struct kvm_irq_level *irq_level) +{ + u32 mask; + unsigned int vcpu_idx; + struct kvm_vcpu *vcpu; + + vcpu_idx = irq_level->irq / 2; + if (vcpu_idx >= KVM_MAX_VCPUS) + return -EINVAL; + + vcpu = kvm_get_vcpu(kvm, vcpu_idx); + if (!vcpu) + return -EINVAL; + + switch (irq_level->irq % 2) { + case KVM_ARM_IRQ_LINE: + mask = HCR_VI; + break; + case KVM_ARM_FIQ_LINE: + mask = HCR_VF; + break; + default: + return -EINVAL; + } + + trace_kvm_irq_line(irq_level->irq % 2, irq_level->level, vcpu_idx); + + if (irq_level->level) { + vcpu->arch.virt_irq |= mask; + vcpu->arch.wait_for_interrupts = 0; + } else + vcpu->arch.virt_irq &= ~mask; + + return 0; +} + long kvm_arch_vcpu_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg) { @@ -312,8 +349,21 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm, struct kvm_dirty_log *log) long kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg) { - printk(KERN_ERR "kvm_arch_vm_ioctl: Unsupported ioctl (%d)\n", ioctl); - return -EINVAL; + struct kvm *kvm = filp->private_data; + void __user *argp = (void __user *)arg; + + switch (ioctl) { + case KVM_IRQ_LINE: { + struct kvm_irq_level irq_event; + + if (copy_from_user(&irq_event, argp, sizeof irq_event)) + return -EFAULT; + return kvm_arch_vm_ioctl_irq_line(kvm, &irq_event); + } + default: + kvm_err(-EINVAL, "Unsupported ioctl (%d)", ioctl); + return -EINVAL; + } } /** diff --git a/arch/arm/kvm/trace.h b/arch/arm/kvm/trace.h index f8869c1..ac64e3a 100644 --- a/arch/arm/kvm/trace.h +++ b/arch/arm/kvm/trace.h @@ -40,6 +40,27 @@ TRACE_EVENT(kvm_exit, ); +TRACE_EVENT(kvm_irq_line, + TP_PROTO(unsigned int type, unsigned int level, unsigned int vcpu_idx), + TP_ARGS(type, level, vcpu_idx), + + TP_STRUCT__entry( + __field( unsigned int, type ) + __field( unsigned int, level ) + __field( unsigned int, vcpu_idx ) + ), + + TP_fast_assign( + __entry->type = type; + __entry->level = level; + __entry->vcpu_idx = vcpu_idx; + ), + + TP_printk("KVM_IRQ_LINE: type: %s, level: %u, vcpu: %u", + (__entry->type == KVM_ARM_IRQ_LINE) ? "IRQ" : "FIQ", + __entry->level, __entry->vcpu_idx) +); + #endif /* _TRACE_KVM_H */ diff --git a/include/linux/kvm.h b/include/linux/kvm.h index ea2dc1a..4e85b4a 100644 --- a/include/linux/kvm.h +++ b/include/linux/kvm.h @@ -111,6 +111,7 @@ struct kvm_irq_level { * ACPI gsi notion of irq. * For IA-64 (APIC model) IOAPIC0: irq 0-23; IOAPIC1: irq 24-47.. * For X86 (standard AT mode) PIC0/1: irq 0-15. IOAPIC0: 0-23.. + * For ARM: IRQ: irq = (2*vcpu_index). FIQ: irq = (2*vcpu_indx + 1). */ union { __u32 irq;
Userspace can inject IRQs and FIQs through the KVM_IRQ_LINE VM ioctl. This ioctl is used since the sematics are in fact two lines that can be either raised or lowered on the VCPU - the IRQ and FIQ lines. KVM needs to know which VCPU it must operate on and whether the FIQ or IRQ line is raised/lowered. Hence both pieces of information is packed in the kvm_irq_level->irq field. The irq fild value will be: IRQ: vcpu_index * 2 FIQ: (vcpu_index * 2) + 1 This is documented in Documentation/kvm/api.txt. The effect of the ioctl is simply to simply raise/lower the corresponding virt_irq field on the VCPU struct, which will cause the world-switch code to raise/lower virtual interrupts when running the guest on next switch. The wait_for_interrupt flag is also cleared for raised IRQs causing an idle VCPU to become active again. Signed-off-by: Christoffer Dall <c.dall@virtualopensystems.com> --- Documentation/kvm/api.txt | 11 ++++++-- arch/arm/include/asm/kvm.h | 8 ++++++ arch/arm/include/asm/kvm_arm.h | 1 + arch/arm/kvm/arm.c | 54 +++++++++++++++++++++++++++++++++++++++- arch/arm/kvm/trace.h | 21 ++++++++++++++++ include/linux/kvm.h | 1 + 6 files changed, 91 insertions(+), 5 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html