From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christoffer Dall Subject: Re: [Android-virt] [PATCH v4 05/10] ARM: KVM: Inject IRQs and FIQs from userspace Date: Tue, 9 Aug 2011 13:27:35 +0200 Message-ID: References: <20110806103821.27198.41231.stgit@localhost6.localdomain6> <20110806103933.27198.91264.stgit@localhost6.localdomain6> <4E4106D8.3070305@redhat.com> Mime-Version: 1.0 (Apple Message framework v1244.3) Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 8BIT Cc: Christoffer Dall , tech@virtualopensystems.com, android-virt@lists.cs.columbia.edu, kvm@vger.kernel.org To: Avi Kivity Return-path: Received: from mail-ey0-f174.google.com ([209.85.215.174]:52792 "EHLO mail-ey0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752951Ab1HIL1m convert rfc822-to-8bit (ORCPT ); Tue, 9 Aug 2011 07:27:42 -0400 Received: by eyx24 with SMTP id 24so3365065eyx.19 for ; Tue, 09 Aug 2011 04:27:41 -0700 (PDT) In-Reply-To: <4E4106D8.3070305@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: 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