From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH v5 05/13] ARM: KVM: Inject IRQs and FIQs from userspace Date: Sun, 11 Dec 2011 16:18:48 +0100 Message-ID: <4EE4C9D8.1010601@web.de> References: <20111211102403.21693.6887.stgit@localhost> <20111211102449.21693.12265.stgit@localhost> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig1ABDCE0F48058326EE406810" Cc: android-virt@lists.cs.columbia.edu, kvm@vger.kernel.org, Marc.Zyngier@arm.com, catalin.marinas@arm.com, tech@virtualopensystems.com, avi@redhat.com, peter.maydell@linaro.org To: Christoffer Dall Return-path: Received: from fmmailgate02.web.de ([217.72.192.227]:52579 "EHLO fmmailgate02.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751899Ab1LKPSw (ORCPT ); Sun, 11 Dec 2011 10:18:52 -0500 Received: from moweb002.kundenserver.de (moweb002.kundenserver.de [172.19.20.108]) by fmmailgate02.web.de (Postfix) with ESMTP id 963741BB318CF for ; Sun, 11 Dec 2011 16:18:51 +0100 (CET) In-Reply-To: <20111211102449.21693.12265.stgit@localhost> Sender: kvm-owner@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig1ABDCE0F48058326EE406810 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Just found two, maybe three nits while browsing by: On 2011-12-11 11:24, 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. >=20 > 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 >=20 > This is documented in Documentation/kvm/api.txt. >=20 > 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. >=20 > Note: The custom trace_kvm_irq_line is used despite a generic definitio= n of > trace_kvm_set_irq, since the trace-Kvm_set_irq depends on the x86-speci= fic > define of __HAVE_IOAPIC. Either the trace event should be created > regardless of this define or it should depend on another ifdef clause, > common for both x86 and ARM. However, since the arguments don't really > match those used in ARM, I am yet to be convinced why this is necessary= =2E >=20 > Signed-off-by: Christoffer Dall > --- > Documentation/virtual/kvm/api.txt | 10 ++++++- > arch/arm/include/asm/kvm.h | 8 ++++++ > arch/arm/include/asm/kvm_arm.h | 1 + > arch/arm/kvm/arm.c | 53 +++++++++++++++++++++++++++++= +++++++- > arch/arm/kvm/trace.h | 21 +++++++++++++++ > include/linux/kvm.h | 1 + > 6 files changed, 91 insertions(+), 3 deletions(-) >=20 > diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/= kvm/api.txt > index 7945b0b..4abaa67 100644 > --- a/Documentation/virtual/kvm/api.txt > +++ b/Documentation/virtual/kvm/api.txt > @@ -572,7 +572,7 @@ only go to the IOAPIC. On ia64, a IOSAPIC is creat= ed. > 4.25 KVM_IRQ_LINE > =20 > 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 > @@ -582,6 +582,14 @@ Requires that an interrupt controller model has be= en previously created with > KVM_CREATE_IRQCHIP. Note that edge-triggered interrupts require the l= evel > to be set to 1 and then back to 0. > =20 > +KVM_CREATE_IRQCHIP (except for ARM). Note that edge-triggered interru= pts > +require the level to be set to 1 and then back to 0. You probably wanted to replace the original lines with these two, no? > + > +ARM uses two types of interrupt lines per CPU, ie. IRQ and FIQ. The va= lue 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 { > __u32 irq; /* GSI */ > 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 > =20 > /* > + * KVM_IRQ_LINE macros to set/read IRQ/FIQ for specific VCPU index. > + */ > +enum KVM_ARM_IRQ_LINE_TYPE { > + KVM_ARM_IRQ_LINE =3D 0, > + KVM_ARM_FIQ_LINE =3D 1, > +}; > + > +/* > * Modes used for short-hand mode determinition in the world-switch co= de 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) > =20 > /* 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 89ba18d..fc0bd6b 100644 > --- a/arch/arm/kvm/arm.c > +++ b/arch/arm/kvm/arm.c > @@ -299,6 +299,43 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu,= struct kvm_run *run) > return -EINVAL; > } > =20 > +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 =3D irq_level->irq / 2; > + if (vcpu_idx >=3D KVM_MAX_VCPUS) > + return -EINVAL; > + > + vcpu =3D kvm_get_vcpu(kvm, vcpu_idx); > + if (!vcpu) > + return -EINVAL; > + > + switch (irq_level->irq % 2) { > + case KVM_ARM_IRQ_LINE: > + mask =3D HCR_VI; > + break; > + case KVM_ARM_FIQ_LINE: > + mask =3D HCR_VF; > + break; > + default: > + return -EINVAL; Due to % 2, default is unreachable. Remove the masking? > + } > + > + trace_kvm_irq_line(irq_level->irq % 2, irq_level->level, vcpu_idx); > + > + if (irq_level->level) { > + vcpu->arch.virt_irq |=3D mask; > + vcpu->arch.wait_for_interrupts =3D 0; > + } else > + vcpu->arch.virt_irq &=3D ~mask; No need to protect the bitops on virt_irq? Or what lock does this? > + > + return 0; > +} > + > long kvm_arch_vcpu_ioctl(struct file *filp, > unsigned int ioctl, unsigned long arg) > { > @@ -313,8 +350,20 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm, st= ruct 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 =3D filp->private_data; > + void __user *argp =3D (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: > + return -EINVAL; > + } > } > =20 > /** > 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, > ); > =20 > =20 > +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 =3D type; > + __entry->level =3D level; > + __entry->vcpu_idx =3D vcpu_idx; > + ), > + > + TP_printk("KVM_IRQ_LINE: type: %s, level: %u, vcpu: %u", > + (__entry->type =3D=3D KVM_ARM_IRQ_LINE) ? "IRQ" : "FIQ", > + __entry->level, __entry->vcpu_idx) > +); > + > =20 > #endif /* _TRACE_KVM_H */ > =20 > diff --git a/include/linux/kvm.h b/include/linux/kvm.h > index c3892fc..679abbb 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 =3D (2*vcpu_index). FIQ: irq =3D (2*vcpu_indx + = 1). > */ > union { > __u32 irq; >=20 > -- > 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 >=20 Jan --------------enig1ABDCE0F48058326EE406810 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.16 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk7kydgACgkQitSsb3rl5xQf4ACgwARK4g/cTT4PkER+21kprS/U iJMAoIe6/u9OoKhxbQivjSI9P9kAY8BC =DLz9 -----END PGP SIGNATURE----- --------------enig1ABDCE0F48058326EE406810--