On 04/19/2013 09:06:27 AM, Alexander Graf wrote:
Now that all pieces are in place for reusing generic irq infrastructure, we can copy x86's implementation of KVM_IRQ_LINE irq injection and simply
reuse it for PPC, as it will work there just as well.

Signed-off-by: Alexander Graf <ag...@suse.de>
---
 arch/powerpc/include/uapi/asm/kvm.h |    1 +
 arch/powerpc/kvm/powerpc.c          |   13 +++++++++++++
 2 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
index 3537bf3..dbb2ac2 100644
--- a/arch/powerpc/include/uapi/asm/kvm.h
+++ b/arch/powerpc/include/uapi/asm/kvm.h
@@ -26,6 +26,7 @@
 #define __KVM_HAVE_SPAPR_TCE
 #define __KVM_HAVE_PPC_SMT
 #define __KVM_HAVE_IRQCHIP
+#define __KVM_HAVE_IRQ_LINE

 struct kvm_regs {
        __u64 pc;
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index c431fea..874c106 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -33,6 +33,7 @@
 #include <asm/cputhreads.h>
 #include <asm/irqflags.h>
 #include "timing.h"
+#include "irq.h"
 #include "../mm/mmu_decl.h"

 #define CREATE_TRACE_POINTS
@@ -945,6 +946,18 @@ static int kvm_vm_ioctl_get_pvinfo(struct kvm_ppc_pvinfo *pvinfo)
        return 0;
 }

+int kvm_vm_ioctl_irq_line(struct kvm *kvm, struct kvm_irq_level *irq_event,
+                         bool line_status)
+{
+       if (!irqchip_in_kernel(kvm))
+               return -ENXIO;
+
+ irq_event->status = kvm_set_irq(kvm, KVM_USERSPACE_IRQ_SOURCE_ID, + irq_event->irq, irq_event->level,
+                                       line_status);
+       return 0;
+}

As Paul noted in the XICS patchset, this could reference an MPIC that has gone away if the user never attached any vcpus and then closed the MPIC fd. It's not a reasonable use case, but it could be used malicously to get the kernel to access a bad pointer. The irqchip_in_kernel check helps somewhat, but it's meant for ensuring that the creation has happened -- it's racy if used for ensuring that destruction hasn't happened.

The problem is rooted in the awkwardness of performing an operation that logically should be on the MPIC fd, but is instead being done on the vm fd.

I think these three steps would fix it (the first two seem like things we should be doing anyway): - During MPIC destruction, make sure MPIC deregisters all routes that reference it. - In kvm_set_irq(), do not release the RCU read lock until after the set() function has been called. - Do not hook up kvm_send_userspace_msi() to MPIC or other new irqchips, as that bypasses the RCU lock. It could be supported as a device fd ioctl if desired, or it could be reworked to operate on an RCU-managed list of MSI handlers, though MPIC really doesn't need this at all.

-Scott
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to