Clean up and enable MSI support for assigned device(v2)
cover Clean up and enable MSI support for assigned device(v2)
Hi, Avi
Here is the update of MSI support. The main change is to move kvm_register_irq_ack_notifier() to common code rather than keep it in update_irq(), for it's the basic request of assigned device.
Thanks!
--
regards
Yang, Sheng
PATCH 1/4 Separate update irq to a single function
--- arch/x86/kvm/x86.c | 82 +++++++++++++++++++++++++++++---------------------- 1 files changed, 47 insertions(+), 35 deletions(-)
arch/x86/kvm/x86.c +47 -35
@@ -206,6 +206,40 @@ static void kvm_free_all_assigned_devices(struct kvm *kvm) } } +static int assigned_device_update_irq (struct kvm *kvm, +struct kvm_assigned_dev_kernel *assigned_dev, +struct kvm_assigned_irq *assigned_irq) +{ +if (assigned_dev->irq_requested) { + assigned_dev->guest_irq = assigned_irq->guest_irq; + assigned_dev->ack_notifier.gsi = assigned_irq->guest_irq; +return 0 ; + } +if (irqchip_in_kernel (kvm)) { +if (!capable (CAP_SYS_RAWIO )) +return -EPERM ; + +if (assigned_irq->host_irq) + assigned_dev->host_irq = assigned_irq->host_irq; +else + assigned_dev->host_irq = assigned_dev->dev->irq; + assigned_dev->guest_irq = assigned_dev->guest_irq; + assigned_dev->ack_notifier.gsi = assigned_dev->guest_irq; + +/* Even though this is PCI, we don't want to use shared +* interrupts. Sharing host devices with guest-assigned devices +* on the same interrupt line is not a happy situation: there +* are going to be long delays in accepting, acking, etc. +*/ +if (request_irq (assigned_dev->host_irq, kvm_assigned_dev_intr, +0 ,"kvm_assigned_device" , (void *)assigned_dev)) +return -EIO ; + } + assigned_dev->irq_requested = true; + +return 0 ; +} +static int kvm_vm_ioctl_assign_irq (struct kvm *kvm,struct kvm_assigned_irq *assigned_irq)
@@ -222,44 +256,22 @@ static int kvm_vm_ioctl_assign_irq(struct kvm *kvm,return -EINVAL ; } -if (match->irq_requested) { - match->guest_irq = assigned_irq->guest_irq; - match->ack_notifier.gsi = assigned_irq->guest_irq; -mutex_unlock (&kvm->lock); -return 0 ; - } - -INIT_WORK (&match->interrupt_work, - kvm_assigned_dev_interrupt_work_handler); - -if (irqchip_in_kernel (kvm)) { -if (!capable (CAP_SYS_RAWIO )) { - r = -EPERM ; - goto out_release; - } - -if (assigned_irq->host_irq) - match->host_irq = assigned_irq->host_irq; -else - match->host_irq = match->dev->irq; - match->guest_irq = assigned_irq->guest_irq; - match->ack_notifier.gsi = assigned_irq->guest_irq; - match->ack_notifier.irq_acked = kvm_assigned_dev_ack_irq; -kvm_register_irq_ack_notifier (kvm, &match->ack_notifier); - -/* Even though this is PCI, we don't want to use shared -* interrupts. Sharing host devices with guest-assigned devices -* on the same interrupt line is not a happy situation: there -* are going to be long delays in accepting, acking, etc. -*/ -if (request_irq (match->host_irq, kvm_assigned_dev_intr,0 , -"kvm_assigned_device" , (void *)match)) { - r = -EIO ; - goto out_release; +if (!match->irq_requested) { +INIT_WORK (&match->interrupt_work, + kvm_assigned_dev_interrupt_work_handler); +if (irqchip_in_kernel (kvm)) { + match->ack_notifier.gsi =-1 ; + match->ack_notifier.irq_acked = + kvm_assigned_dev_ack_irq; +kvm_register_irq_ack_notifier (kvm, + &match->ack_notifier); } } - match->irq_requested = true; + r =assigned_device_update_irq (kvm, match, assigned_irq); +if (r) + goto out_release; +mutex_unlock (&kvm->lock);return r; out_release:
Signature · 1 line
1.5.4.5
Re: [PATCH 1/4] Separate update irq to a single function
arch/x86/kvm/x86.c static void kvm_free_all_assigned_devices(struct kvm *kvm) · line 227
Quoted 2 lines · On Wednesday 08 October 2008 16:38:10 Sheng Yang wrote:
arch/x86/kvm/x86.c +2
-0+ assigned_dev->guest_irq = assigned_dev->guest_irq; + assigned_dev->ack_notifier.gsi = assigned_dev->guest_irq;
Found a typo here...
Anyway, I will rebase it later, after Xiantao's patch went to upstream.
--
regards
Yang, Sheng
PATCH 2/4 KVM: x86: Replace irq_requested with guest_intr_type
In reply to Sheng Yang--- arch/x86/kvm/x86.c | 8 ++++---- include/linux/kvm_host.h | 3 ++- 2 files changed, 6 insertions(+), 5 deletions(-)
arch/x86/kvm/x86.c +4 -4
@@ -171,7 +171,7 @@ static void kvm_free_assigned_device(struct kvm *kvm,struct kvm_assigned_dev_kernel *assigned_dev) { -if (irqchip_in_kernel (kvm) && assigned_dev->irq_requested) +if (irqchip_in_kernel (kvm) && assigned_dev->guest_intr_type)free_irq (assigned_dev->host_irq, (void *)assigned_dev);kvm_unregister_irq_ack_notifier (kvm, &assigned_dev->ack_notifier);
@@ -210,7 +210,7 @@ static int assigned_device_update_irq(struct kvm *kvm, struct kvm_assigned_dev_kernel *assigned_dev, struct kvm_assigned_irq *assigned_irq) { -if (assigned_dev->irq_requested) { +if (assigned_dev->guest_intr_type ==KVM_ASSIGNED_DEV_INTR ) { assigned_dev->guest_irq = assigned_irq->guest_irq; assigned_dev->ack_notifier.gsi = assigned_irq->guest_irq;return 0 ;
@@ -235,7 +235,7 @@ static int assigned_device_update_irq(struct kvm *kvm, 0, "kvm_assigned_device", (void *)assigned_dev))return -EIO ; } - assigned_dev->irq_requested = true; + assigned_dev->guest_intr_type =KVM_ASSIGNED_DEV_INTR ;return 0 ; }
@@ -256,7 +256,7 @@ static int kvm_vm_ioctl_assign_irq(struct kvm *kvm,return -EINVAL ; } -if (!match->irq_requested) { +if (!match->guest_intr_type) {INIT_WORK (&match->interrupt_work, kvm_assigned_dev_interrupt_work_handler);if (irqchip_in_kernel (kvm)) {
include/linux/kvm_host.h +2 -1
@@ -301,7 +301,8 @@ struct kvm_assigned_dev_kernel {int host_devfn;int host_irq;int guest_irq; -int irq_requested; +#define KVM_ASSIGNED_DEV_INTR 1 +int guest_intr_type;struct pci_dev *dev;struct kvm *kvm; };
Signature · 1 line
1.5.4.5
PATCH 3/4 x86: Add MSI delivery mode mask
In reply to Sheng Yang--- include/asm-x86/msidef.h | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
include/asm-x86/msidef.h +3 -0
@@ -15,8 +15,11 @@ MSI_DATA_VECTOR_MASK)#define MSI_DATA_DELIVERY_MODE_SHIFT 8 +#define MSI_DATA_DELIVERY_MODE_MASK 0x700#define MSI_DATA_DELIVERY_FIXED (0 << MSI_DATA_DELIVERY_MODE_SHIFT)#define MSI_DATA_DELIVERY_LOWPRI (1 << MSI_DATA_DELIVERY_MODE_SHIFT) +#define MSI_DATA_DELIVERY_FIXED_VAL 0 +#define MSI_DATA_DELIVERY_LOWPRI_VAL 1#define MSI_DATA_LEVEL_SHIFT 14#define MSI_DATA_LEVEL_DEASSERT (0 << MSI_DATA_LEVEL_SHIFT)
Signature · 1 line
1.5.4.5
RE: [PATCH 3/4] x86: Add MSI delivery mode mask
Hi, Sheng
Since these macros are just used by kvm, and considering cross-arch
support, could you move these stuff to include/linux/kvm_host.h ? So they can be shared for x86 and ia64.
Thanks
Xiantao
Quoted 10 lines · Sheng Yang
Signed-off-by Sheng Yang <[email protected]>--- include/asm-x86/msidef.h | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)include/asm-x86/msidef.h +0
-0
include/asm-x86/msidef.h line 19
Quoted 5 lines
include/asm-x86/msidef.h +1
-0@@ -15,8 +15,11 @@ MSI_DATA_VECTOR_MASK)#define MSI_DATA_DELIVERY_MODE_SHIFT 8 +#define MSI_DATA_DELIVERY_MODE_MASK 0x700#define MSI_DATA_DELIVERY_FIXED (0 <<
MSI_DATA_DELIVERY_MODE_SHIFT)
include/asm-x86/msidef.h line 20
Quoted 1 line
include/asm-x86/msidef.h +0
-0#define MSI_DATA_DELIVERY_LOWPRI (1 <<
MSI_DATA_DELIVERY_MODE_SHIFT)
include/asm-x86/msidef.h
Quoted 5 lines
include/asm-x86/msidef.h +2
-0+#define MSI_DATA_DELIVERY_FIXED_VAL 0 +#define MSI_DATA_DELIVERY_LOWPRI_VAL 1#define MSI_DATA_LEVEL_SHIFT 14#define MSI_DATA_LEVEL_DEASSERT (0 <<
MSI_DATA_LEVEL_SHIFT)
Quoted 2 lines
--
1.5.4.5
Re: [PATCH 3/4] x86: Add MSI delivery mode mask
Quoted 6 lines · Xiantao
Hi, Sheng
Since these macros are just used by kvm, and considering cross-arch
support, could you move these stuff toinclude/linux/kvm_host.h? So they can be shared for x86 and ia64.
Thanks
Xiantao
I don't think so... The problem is, we can't assume that IA64 and x86 use same MSI format, and I think we shouldn't do that. And this patch just modify x86 host linux side. So if they can be shared, host kernel side should also share them, then this file(msidef.h) shouldn't put on include/asm-x86/... So for now, probably it's more reasonable to keep it here.
Thanks!
--
regards
Yang, Sheng
Quoted 34 lines
Quoted 10 lines · Sheng Yang
Signed-off-by Sheng Yang <[email protected]>--- include/asm-x86/msidef.h | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)include/asm-x86/msidef.h +0
-0include/asm-x86/msidef.h line 19
Quoted 5 lines
include/asm-x86/msidef.h +1
-0@@ -15,8 +15,11 @@ MSI_DATA_VECTOR_MASK)#define MSI_DATA_DELIVERY_MODE_SHIFT 8 +#define MSI_DATA_DELIVERY_MODE_MASK 0x700#define MSI_DATA_DELIVERY_FIXED (0 <<
MSI_DATA_DELIVERY_MODE_SHIFT)include/asm-x86/msidef.h line 20
Quoted 1 line
include/asm-x86/msidef.h +0
-0#define MSI_DATA_DELIVERY_LOWPRI (1 <<
MSI_DATA_DELIVERY_MODE_SHIFT)include/asm-x86/msidef.h
Quoted 5 lines
include/asm-x86/msidef.h +2
-0+#define MSI_DATA_DELIVERY_FIXED_VAL 0 +#define MSI_DATA_DELIVERY_LOWPRI_VAL 1#define MSI_DATA_LEVEL_SHIFT 14#define MSI_DATA_LEVEL_DEASSERT (0 <<
MSI_DATA_LEVEL_SHIFT)Quoted 2 lines
--
1.5.4.5
RE: [PATCH 3/4] x86: Add MSI delivery mode mask
Quoted 8 lines · Sheng Yang
Quoted 6 lines · Xiantao
Hi, Sheng
Since these macros are just used by kvm, and considering
cross-arch support, could you move these stuff toinclude/linux/kvm_host.h? So they can be shared for x86 and ia64. Thanks
Xiantao
Quoted 6 lines
I don't think so... The problem is, we can't assume that IA64 and x86 use same MSI format, and I think we shouldn't do that. And this patch just modify x86 host linux side. So if they can be shared, host kernel side should also share them, then this file(msidef.h) shouldn't put on
include/asm-x86/... So for now, probably it's more reasonable to keep it here.
You know, device-assigment logic should be shared by x86 and ia64 (so far), so we should make these code arch-indpendent, other than duplicate the work for ia64 side. So maybe you should guarantee it also work on ia64 side before pushing them into upstream.:)
I am not familiar with the detail about the format of MSI in x86 and ia64 side, but anyway they should only have a small gap at the most. Thanks
Xiantao
Quoted 32 lines
Quoted 32 lines
Quoted 9 lines · Sheng Yang
Signed-off-by Sheng Yang <[email protected]>--- include/asm-x86/msidef.h | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)include/asm-x86/msidef.h +1
-0+++ b/include/asm-x86/msidef.hinclude/asm-x86/msidef.h line 19
Quoted 5 lines
include/asm-x86/msidef.h +1
-0@@ -15,8 +15,11 @@ MSI_DATA_VECTOR_MASK)#define MSI_DATA_DELIVERY_MODE_SHIFT 8 +#define MSI_DATA_DELIVERY_MODE_MASK 0x700#define MSI_DATA_DELIVERY_FIXED (0 <<
MSI_DATA_DELIVERY_MODE_SHIFT)include/asm-x86/msidef.h line 20
Quoted 1 line
include/asm-x86/msidef.h +0
-0#define MSI_DATA_DELIVERY_LOWPRI (1 <<
MSI_DATA_DELIVERY_MODE_SHIFT)include/asm-x86/msidef.h
Quoted 5 lines
include/asm-x86/msidef.h +2
-0+#define MSI_DATA_DELIVERY_FIXED_VAL 0 +#define MSI_DATA_DELIVERY_LOWPRI_VAL 1#define MSI_DATA_LEVEL_SHIFT 14#define MSI_DATA_LEVEL_DEASSERT (0 <<
MSI_DATA_LEVEL_SHIFT)Quoted 2 lines
--
1.5.4.5
PATCH 4/4 KVM: Enable MSI for device assignment
In reply to Sheng Yang--- arch/x86/kvm/x86.c | 115 ++++++++++++++++++++++++++++++++++++++++++++-- include/linux/kvm.h | 4 ++ include/linux/kvm_host.h | 3 + virt/kvm/ioapic.c | 2 +- virt/kvm/ioapic.h | 2 + 5 files changed, 120 insertions(+), 6 deletions(-)
arch/x86/kvm/x86.c +110 -5
@@ -41,6 +41,7 @@#include <asm/msr.h> #include <asm/desc.h> #include <asm/mtrr.h> +#include <asm/msidef.h> #define MAX_IO_MSRS 256#define CR0_RESERVED_BITS \
@@ -121,6 +122,50 @@ static struct kvm_assigned_dev_kernel *kvm_find_assigned_dev(struct list_head *hreturn NULL ; } +static void assigned_device_msi_dispatch (struct kvm_assigned_dev_kernel + *assigned_dev) +{ +u8 dest_id = assigned_dev->guest_msi_addr &MSI_ADDR_DEST_ID_MASK ; +u8 vector = assigned_dev->guest_msi_data &MSI_DATA_VECTOR_MASK ; +u8 dest_mode = assigned_dev->guest_msi_addr & + (1 <<MSI_ADDR_DEST_MODE_SHIFT ); +u8 trig_mode = assigned_dev->guest_msi_data & + (1 <<MSI_DATA_TRIGGER_SHIFT ); +u8 delivery_mode = assigned_dev->guest_msi_data & +MSI_DATA_DELIVERY_MODE_MASK ; +u32 deliver_bitmask; +int vcpu_id; +struct kvm_vcpu *vcpu; +struct kvm_ioapic *ioapic =ioapic_irqchip (assigned_dev->kvm); + +BUG_ON (!ioapic); + + deliver_bitmask =ioapic_get_delivery_bitmask (ioapic, + dest_id, dest_mode); +switch (delivery_mode) { +case MSI_DATA_DELIVERY_LOWPRI_VAL : + vcpu =kvm_get_lowest_prio_vcpu (ioapic->kvm, vector, + deliver_bitmask); +if (vcpu !=NULL ) +kvm_apic_set_irq (vcpu, vector, trig_mode); +else +printk (KERN_INFO "Null lowest priority vcpu!\n" ); +break ; +case MSI_DATA_DELIVERY_FIXED_VAL : +for (vcpu_id =0 ; deliver_bitmask !=0 ; vcpu_id++) { +if (!(deliver_bitmask & (1 << vcpu_id))) +continue ; + deliver_bitmask &= ~(1 << vcpu_id); + vcpu = ioapic->kvm->vcpus[vcpu_id]; +if (vcpu) +kvm_apic_set_irq (vcpu, vector, trig_mode); + } +break ; +default : +printk (KERN_INFO "Unsupported MSI delivery mode\n" ); + } +} +static void kvm_assigned_dev_interrupt_work_handler (struct work_struct *work) {struct kvm_assigned_dev_kernel *assigned_dev;
@@ -133,8 +178,12 @@ static void kvm_assigned_dev_interrupt_work_handler(struct work_struct *work)* finer-grained lock, update this */ mutex_lock (&assigned_dev->kvm->lock); -kvm_set_irq (assigned_dev->kvm, - assigned_dev->guest_irq,1 ); +if (assigned_dev->guest_intr_type ==KVM_ASSIGNED_DEV_INTR ) +kvm_set_irq (assigned_dev->kvm, assigned_dev->guest_irq,1 ); +else if (assigned_dev->guest_intr_type ==KVM_ASSIGNED_DEV_MSI ) { +assigned_device_msi_dispatch (assigned_dev); +enable_irq (assigned_dev->host_irq); + }mutex_unlock (&assigned_dev->kvm->lock);kvm_put_kvm (assigned_dev->kvm); }
@@ -173,6 +222,8 @@ static void kvm_free_assigned_device(struct kvm *kvm, {if (irqchip_in_kernel (kvm) && assigned_dev->guest_intr_type)free_irq (assigned_dev->host_irq, (void *)assigned_dev); +if (assigned_dev->guest_intr_type ==KVM_ASSIGNED_DEV_MSI ) +pci_disable_msi (assigned_dev->dev);kvm_unregister_irq_ack_notifier (kvm, &assigned_dev->ack_notifier);
@@ -216,6 +267,11 @@ static int assigned_device_update_irq(struct kvm *kvm,return 0 ; }if (irqchip_in_kernel (kvm)) { +if (assigned_dev->guest_intr_type ==KVM_ASSIGNED_DEV_MSI ) { +free_irq (assigned_dev->host_irq, (void *)kvm); +pci_disable_msi (assigned_dev->dev); + } +if (!capable (CAP_SYS_RAWIO ))return -EPERM ;
@@ -240,6 +296,40 @@ static int assigned_device_update_irq(struct kvm *kvm,return 0 ; } +static int assigned_device_update_msi (struct kvm *kvm, +struct kvm_assigned_dev_kernel *assigned_dev, +struct kvm_assigned_irq *assigned_irq) +{ +int r; + +if (assigned_dev->guest_intr_type ==KVM_ASSIGNED_DEV_MSI ) +return 0 ; + +if (irqchip_in_kernel (kvm)) { +if (assigned_dev->guest_intr_type ==KVM_ASSIGNED_DEV_INTR ) +free_irq (assigned_dev->host_irq, (void *)assigned_dev); + + assigned_dev->guest_msi_addr = assigned_irq->guest_msi_addr; + assigned_dev->guest_msi_data = assigned_irq->guest_msi_data; + + r =pci_enable_msi (assigned_dev->dev); +if (r) +return r; + +if (request_irq (assigned_dev->dev->irq, kvm_assigned_dev_intr, +0 ,"kvm_msi_assigned_device" , + (void *)assigned_dev)) +return -EIO ; + + assigned_dev->host_irq = assigned_dev->dev->irq; + assigned_dev->kvm = kvm; + assigned_dev->guest_intr_type =KVM_ASSIGNED_DEV_MSI ; + } + + assigned_dev->ack_notifier.gsi =-1 ; +return 0 ; +} +static int kvm_vm_ioctl_assign_irq (struct kvm *kvm,struct kvm_assigned_irq *assigned_irq)
@@ -268,9 +358,24 @@ static int kvm_vm_ioctl_assign_irq(struct kvm *kvm, } } - r =assigned_device_update_irq (kvm, match, assigned_irq); -if (r) - goto out_release; +if (assigned_irq->flags &KVM_DEV_IRQ_ASSIGN_ENABLE_MSI ) { +if (assigned_device_update_msi (kvm, match, assigned_irq)) { +printk (KERN_INFO "kvm: fail to enable msi as guest " +"requested, fall back to legacy " +"interrupt\n" ); + r = -EINVAL ; +if (assigned_device_update_irq (kvm, match, + assigned_irq)) { +printk (KERN_INFO "kvm: fail to fall back, " +"release assigned device\n" ); + goto out_release; + } + } + }else { + r =assigned_device_update_irq (kvm, match, assigned_irq); +if (r) + goto out_release; + }mutex_unlock (&kvm->lock);return r;
include/linux/kvm.h +4 -0
@@ -493,9 +493,13 @@ struct kvm_assigned_irq {__u32 assigned_dev_id;__u32 host_irq;__u32 guest_irq; +__u32 guest_msi_data; +__u32 guest_msi_addr; __u32 flags; };#define KVM_DEV_ASSIGN_ENABLE_IOMMU (1 << 0) +#define KVM_DEV_IRQ_ASSIGN_ENABLE_MSI (1 << 0) +#endif
include/linux/kvm_host.h +3 -0
@@ -300,8 +300,11 @@ struct kvm_assigned_dev_kernel {int host_busnr;int host_devfn;int host_irq; +u32 guest_msi_addr; +u32 guest_msi_data;int guest_irq;#define KVM_ASSIGNED_DEV_INTR 1 +#define KVM_ASSIGNED_DEV_MSI 2int guest_intr_type;struct pci_dev *dev;struct kvm *kvm;
virt/kvm/ioapic.c +1 -1
@@ -152,7 +152,7 @@ static void ioapic_inj_nmi(struct kvm_vcpu *vcpu)kvm_inject_nmi (vcpu); } -static u32 ioapic_get_delivery_bitmask (struct kvm_ioapic *ioapic,u8 dest, +u32 ioapic_get_delivery_bitmask (struct kvm_ioapic *ioapic,u8 dest,u8 dest_mode) {u32 mask =0 ;
virt/kvm/ioapic.h +2 -0
@@ -92,5 +92,7 @@ void kvm_ioapic_update_eoi(struct kvm *kvm, int vector, int trigger_mode);int kvm_ioapic_init (struct kvm *kvm);void kvm_ioapic_set_irq (struct kvm_ioapic *ioapic,int irq,int level);void kvm_ioapic_reset (struct kvm_ioapic *ioapic); +u32 ioapic_get_delivery_bitmask (struct kvm_ioapic *ioapic,u8 dest, +u8 dest_mode);#endif
Signature · 1 line
1.5.4.5