Re: [PATCH 16/17] KVM: PPC: MPIC: Add support for KVM_IRQ_LINE

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 19.04.2013, at 20:51, Scott Wood wrote:

> 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 <agraf@xxxxxxx>
>> ---
>> 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.

Can't we just add an RCU lock in the send_userspace_msi case? I don't think we should handle MSIs any differently from normal IRQs.


Alex

--
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




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux