* [PATCH 01/10] KVM: Move ack notifier register and IRQ sourcd ID request
2008-10-31 4:50 [PATCH 0/10][v4] Enable MSI for KVM assigned device kernel part Sheng Yang
@ 2008-10-31 4:50 ` Sheng Yang
2008-10-31 4:50 ` [PATCH 02/10] KVM: Separate update irq to a single function Sheng Yang
` (8 subsequent siblings)
9 siblings, 0 replies; 32+ messages in thread
From: Sheng Yang @ 2008-10-31 4:50 UTC (permalink / raw)
To: Avi Kivity; +Cc: kvm, Sheng Yang
Signed-off-by: Sheng Yang <sheng@linux.intel.com>
---
virt/kvm/kvm_main.c | 30 +++++++++++++++++++-----------
1 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 4f43abe..4647a7b 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -190,16 +190,31 @@ static int kvm_vm_ioctl_assign_irq(struct kvm *kvm,
return -EINVAL;
}
- if (match->irq_requested) {
+ if (!match->irq_requested) {
+ INIT_WORK(&match->interrupt_work,
+ kvm_assigned_dev_interrupt_work_handler);
+ if (irqchip_in_kernel(kvm)) {
+ /* Register ack nofitier */
+ match->ack_notifier.gsi = -1;
+ match->ack_notifier.irq_acked =
+ kvm_assigned_dev_ack_irq;
+ kvm_register_irq_ack_notifier(kvm,
+ &match->ack_notifier);
+
+ /* Request IRQ source ID */
+ r = kvm_request_irq_source_id(kvm);
+ if (r < 0)
+ goto out_release;
+ else
+ match->irq_source_id = r;
+ }
+ } else {
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;
@@ -212,13 +227,6 @@ static int kvm_vm_ioctl_assign_irq(struct kvm *kvm,
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);
- r = kvm_request_irq_source_id(kvm);
- if (r < 0)
- goto out_release;
- else
- match->irq_source_id = r;
/* Even though this is PCI, we don't want to use shared
* interrupts. Sharing host devices with guest-assigned devices
--
1.5.4.5
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH 02/10] KVM: Separate update irq to a single function
2008-10-31 4:50 [PATCH 0/10][v4] Enable MSI for KVM assigned device kernel part Sheng Yang
2008-10-31 4:50 ` [PATCH 01/10] KVM: Move ack notifier register and IRQ sourcd ID request Sheng Yang
@ 2008-10-31 4:50 ` Sheng Yang
2008-10-31 4:50 ` [PATCH 03/10] KVM: Replace irq_requested with more generic irq_requested_type Sheng Yang
` (7 subsequent siblings)
9 siblings, 0 replies; 32+ messages in thread
From: Sheng Yang @ 2008-10-31 4:50 UTC (permalink / raw)
To: Avi Kivity; +Cc: kvm, Sheng Yang
Signed-off-by: Sheng Yang <sheng@linux.intel.com>
---
virt/kvm/kvm_main.c | 68 ++++++++++++++++++++++++++++----------------------
1 files changed, 38 insertions(+), 30 deletions(-)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 4647a7b..7d261b1 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -174,6 +174,41 @@ void kvm_free_all_assigned_devices(struct kvm *kvm)
}
}
+static int assigned_device_update_intx(struct kvm *kvm,
+ struct kvm_assigned_dev_kernel *adev,
+ struct kvm_assigned_irq *airq)
+{
+ if (adev->irq_requested) {
+ adev->guest_irq = airq->guest_irq;
+ adev->ack_notifier.gsi = airq->guest_irq;
+ return 0;
+ }
+
+ if (irqchip_in_kernel(kvm)) {
+ if (!capable(CAP_SYS_RAWIO))
+ return -EPERM;
+
+ if (airq->host_irq)
+ adev->host_irq = airq->host_irq;
+ else
+ adev->host_irq = adev->dev->irq;
+ adev->guest_irq = airq->guest_irq;
+ adev->ack_notifier.gsi = airq->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(adev->host_irq, kvm_assigned_dev_intr,
+ 0, "kvm_assigned_intx_device", (void *)adev))
+ return -EIO;
+ }
+
+ adev->irq_requested = true;
+ return 0;
+}
+
static int kvm_vm_ioctl_assign_irq(struct kvm *kvm,
struct kvm_assigned_irq
*assigned_irq)
@@ -208,39 +243,12 @@ static int kvm_vm_ioctl_assign_irq(struct kvm *kvm,
else
match->irq_source_id = r;
}
- } else {
- match->guest_irq = assigned_irq->guest_irq;
- match->ack_notifier.gsi = assigned_irq->guest_irq;
- mutex_unlock(&kvm->lock);
- return 0;
}
- 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;
-
- /* 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;
- }
- }
+ r = assigned_device_update_intx(kvm, match, assigned_irq);
+ if (r)
+ goto out_release;
- match->irq_requested = true;
mutex_unlock(&kvm->lock);
return r;
out_release:
--
1.5.4.5
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH 03/10] KVM: Replace irq_requested with more generic irq_requested_type
2008-10-31 4:50 [PATCH 0/10][v4] Enable MSI for KVM assigned device kernel part Sheng Yang
2008-10-31 4:50 ` [PATCH 01/10] KVM: Move ack notifier register and IRQ sourcd ID request Sheng Yang
2008-10-31 4:50 ` [PATCH 02/10] KVM: Separate update irq to a single function Sheng Yang
@ 2008-10-31 4:50 ` Sheng Yang
2008-10-31 4:50 ` [PATCH 04/10] KVM: Clean up assigned_device_update_irq Sheng Yang
` (6 subsequent siblings)
9 siblings, 0 replies; 32+ messages in thread
From: Sheng Yang @ 2008-10-31 4:50 UTC (permalink / raw)
To: Avi Kivity; +Cc: kvm, Sheng Yang
Separate guest irq type and host irq type, for we can support guest using INTx
with host using MSI (but not opposite combination).
Signed-off-by: Sheng Yang <sheng@linux.intel.com>
---
include/linux/kvm_host.h | 4 +++-
virt/kvm/kvm_main.c | 9 +++++----
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 3a0fb77..c3d4b96 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -307,7 +307,9 @@ struct kvm_assigned_dev_kernel {
int host_devfn;
int host_irq;
int guest_irq;
- int irq_requested;
+#define KVM_ASSIGNED_DEV_GUEST_INTX (1 << 0)
+#define KVM_ASSIGNED_DEV_HOST_INTX (1 << 8)
+ unsigned long irq_requested_type;
int irq_source_id;
struct pci_dev *dev;
struct kvm *kvm;
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 7d261b1..ed1b243 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -140,7 +140,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->irq_requested_type)
free_irq(assigned_dev->host_irq, (void *)assigned_dev);
kvm_unregister_irq_ack_notifier(&assigned_dev->ack_notifier);
@@ -178,7 +178,7 @@ static int assigned_device_update_intx(struct kvm *kvm,
struct kvm_assigned_dev_kernel *adev,
struct kvm_assigned_irq *airq)
{
- if (adev->irq_requested) {
+ if (adev->irq_requested_type & KVM_ASSIGNED_DEV_GUEST_INTX) {
adev->guest_irq = airq->guest_irq;
adev->ack_notifier.gsi = airq->guest_irq;
return 0;
@@ -205,7 +205,8 @@ static int assigned_device_update_intx(struct kvm *kvm,
return -EIO;
}
- adev->irq_requested = true;
+ adev->irq_requested_type = KVM_ASSIGNED_DEV_GUEST_INTX |
+ KVM_ASSIGNED_DEV_HOST_INTX;
return 0;
}
@@ -225,7 +226,7 @@ static int kvm_vm_ioctl_assign_irq(struct kvm *kvm,
return -EINVAL;
}
- if (!match->irq_requested) {
+ if (!match->irq_requested_type) {
INIT_WORK(&match->interrupt_work,
kvm_assigned_dev_interrupt_work_handler);
if (irqchip_in_kernel(kvm)) {
--
1.5.4.5
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH 04/10] KVM: Clean up assigned_device_update_irq
2008-10-31 4:50 [PATCH 0/10][v4] Enable MSI for KVM assigned device kernel part Sheng Yang
` (2 preceding siblings ...)
2008-10-31 4:50 ` [PATCH 03/10] KVM: Replace irq_requested with more generic irq_requested_type Sheng Yang
@ 2008-10-31 4:50 ` Sheng Yang
2008-10-31 4:50 ` [PATCH 05/10] KVM: Add fields for MSI device assignment Sheng Yang
` (5 subsequent siblings)
9 siblings, 0 replies; 32+ messages in thread
From: Sheng Yang @ 2008-10-31 4:50 UTC (permalink / raw)
To: Avi Kivity; +Cc: kvm, Sheng Yang
Signed-off-by: Sheng Yang <sheng@linux.intel.com>
---
virt/kvm/kvm_main.c | 10 ++++------
1 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index ed1b243..c69c108 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -178,11 +178,11 @@ static int assigned_device_update_intx(struct kvm *kvm,
struct kvm_assigned_dev_kernel *adev,
struct kvm_assigned_irq *airq)
{
- if (adev->irq_requested_type & KVM_ASSIGNED_DEV_GUEST_INTX) {
- adev->guest_irq = airq->guest_irq;
- adev->ack_notifier.gsi = airq->guest_irq;
+ adev->guest_irq = airq->guest_irq;
+ adev->ack_notifier.gsi = airq->guest_irq;
+
+ if (adev->irq_requested_type & KVM_ASSIGNED_DEV_HOST_INTX)
return 0;
- }
if (irqchip_in_kernel(kvm)) {
if (!capable(CAP_SYS_RAWIO))
@@ -192,8 +192,6 @@ static int assigned_device_update_intx(struct kvm *kvm,
adev->host_irq = airq->host_irq;
else
adev->host_irq = adev->dev->irq;
- adev->guest_irq = airq->guest_irq;
- adev->ack_notifier.gsi = airq->guest_irq;
/* Even though this is PCI, we don't want to use shared
* interrupts. Sharing host devices with guest-assigned devices
--
1.5.4.5
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH 05/10] KVM: Add fields for MSI device assignment
2008-10-31 4:50 [PATCH 0/10][v4] Enable MSI for KVM assigned device kernel part Sheng Yang
` (3 preceding siblings ...)
2008-10-31 4:50 ` [PATCH 04/10] KVM: Clean up assigned_device_update_irq Sheng Yang
@ 2008-10-31 4:50 ` Sheng Yang
2008-11-04 11:05 ` Avi Kivity
2008-10-31 4:50 ` [PATCH 06/10] KVM: Export ioapic_get_delivery_bitmask Sheng Yang
` (4 subsequent siblings)
9 siblings, 1 reply; 32+ messages in thread
From: Sheng Yang @ 2008-10-31 4:50 UTC (permalink / raw)
To: Avi Kivity; +Cc: kvm, Sheng Yang
Prepared for kvm_arch_assigned_device_msi_dispatch().
Signed-off-by: Sheng Yang <sheng@linux.intel.com>
---
include/linux/kvm.h | 6 +++++-
include/linux/kvm_host.h | 4 ++++
2 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/include/linux/kvm.h b/include/linux/kvm.h
index 44fd7fa..8ec74b8 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -506,11 +506,15 @@ struct kvm_assigned_irq {
__u32 host_irq;
__u32 guest_irq;
__u32 flags;
+ __u32 guest_msi_data;
+ __u32 guest_msi_addr;
union {
- __u32 reserved[12];
+ __u32 reserved[10];
};
};
#define KVM_DEV_ASSIGN_ENABLE_IOMMU (1 << 0)
+#define KVM_DEV_IRQ_ASSIGN_ENABLE_MSI (1 << 0)
+
#endif
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index c3d4b96..2e9a9a8 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -307,8 +307,12 @@ struct kvm_assigned_dev_kernel {
int host_devfn;
int host_irq;
int guest_irq;
+ u32 guest_msi_addr;
+ u32 guest_msi_data;
#define KVM_ASSIGNED_DEV_GUEST_INTX (1 << 0)
+#define KVM_ASSIGNED_DEV_GUEST_MSI (1 << 1)
#define KVM_ASSIGNED_DEV_HOST_INTX (1 << 8)
+#define KVM_ASSIGNED_DEV_HOST_MSI (1 << 9)
unsigned long irq_requested_type;
int irq_source_id;
struct pci_dev *dev;
--
1.5.4.5
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH 06/10] KVM: Export ioapic_get_delivery_bitmask
2008-10-31 4:50 [PATCH 0/10][v4] Enable MSI for KVM assigned device kernel part Sheng Yang
` (4 preceding siblings ...)
2008-10-31 4:50 ` [PATCH 05/10] KVM: Add fields for MSI device assignment Sheng Yang
@ 2008-10-31 4:50 ` Sheng Yang
2008-11-04 11:07 ` Avi Kivity
2008-10-31 4:50 ` [PATCH 07/10] x86: Add MSI delivery mode value Sheng Yang
` (3 subsequent siblings)
9 siblings, 1 reply; 32+ messages in thread
From: Sheng Yang @ 2008-10-31 4:50 UTC (permalink / raw)
To: Avi Kivity; +Cc: kvm, Sheng Yang
It would be used for MSI in device assignment, for MSI dispatch.
Signed-off-by: Sheng Yang <sheng@linux.intel.com>
---
virt/kvm/ioapic.c | 4 ++--
virt/kvm/ioapic.h | 2 ++
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
index c8f939c..1e0ff8c 100644
--- a/virt/kvm/ioapic.c
+++ b/virt/kvm/ioapic.c
@@ -153,8 +153,8 @@ static void ioapic_inj_nmi(struct kvm_vcpu *vcpu)
kvm_vcpu_kick(vcpu);
}
-static u32 ioapic_get_delivery_bitmask(struct kvm_ioapic *ioapic, u8 dest,
- u8 dest_mode)
+u32 ioapic_get_delivery_bitmask(struct kvm_ioapic *ioapic, u8 dest,
+ u8 dest_mode)
{
u32 mask = 0;
int i;
diff --git a/virt/kvm/ioapic.h b/virt/kvm/ioapic.h
index cd7ae76..42972f5 100644
--- a/virt/kvm/ioapic.h
+++ b/virt/kvm/ioapic.h
@@ -85,5 +85,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
--
1.5.4.5
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 06/10] KVM: Export ioapic_get_delivery_bitmask
2008-10-31 4:50 ` [PATCH 06/10] KVM: Export ioapic_get_delivery_bitmask Sheng Yang
@ 2008-11-04 11:07 ` Avi Kivity
2008-11-04 11:09 ` Sheng Yang
0 siblings, 1 reply; 32+ messages in thread
From: Avi Kivity @ 2008-11-04 11:07 UTC (permalink / raw)
To: Sheng Yang; +Cc: kvm
Sheng Yang wrote:
> It would be used for MSI in device assignment, for MSI dispatch.
>
> -static u32 ioapic_get_delivery_bitmask(struct kvm_ioapic *ioapic, u8 dest,
> - u8 dest_mode)
> +u32 ioapic_get_delivery_bitmask(struct kvm_ioapic *ioapic, u8 dest,
> + u8 dest_mode)
>
Need to be prefixed with kvm_, otherwise it might be confused with the
host ioapic.
I'm a little worried about locking; we'll want to have a separate lock
for the ioapic, and callers of this function will need to know about
it. But we can handle that later.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 06/10] KVM: Export ioapic_get_delivery_bitmask
2008-11-04 11:07 ` Avi Kivity
@ 2008-11-04 11:09 ` Sheng Yang
0 siblings, 0 replies; 32+ messages in thread
From: Sheng Yang @ 2008-11-04 11:09 UTC (permalink / raw)
To: Avi Kivity; +Cc: kvm
On Tuesday 04 November 2008 19:07:19 Avi Kivity wrote:
> Sheng Yang wrote:
> > It would be used for MSI in device assignment, for MSI dispatch.
> >
> > -static u32 ioapic_get_delivery_bitmask(struct kvm_ioapic *ioapic, u8
> > dest, - u8 dest_mode)
> > +u32 ioapic_get_delivery_bitmask(struct kvm_ioapic *ioapic, u8 dest,
> > + u8 dest_mode)
>
> Need to be prefixed with kvm_, otherwise it might be confused with the
> host ioapic.
>
> I'm a little worried about locking; we'll want to have a separate lock
> for the ioapic, and callers of this function will need to know about
> it. But we can handle that later.
Yeah, agree. And after kvm_apic_local_deliver() was exported, I also a little
worry about locking on apic side. I would like take a look at them later.
--
regards
Yang, Sheng
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 07/10] x86: Add MSI delivery mode value
2008-10-31 4:50 [PATCH 0/10][v4] Enable MSI for KVM assigned device kernel part Sheng Yang
` (5 preceding siblings ...)
2008-10-31 4:50 ` [PATCH 06/10] KVM: Export ioapic_get_delivery_bitmask Sheng Yang
@ 2008-10-31 4:50 ` Sheng Yang
2008-11-04 11:09 ` Avi Kivity
2008-10-31 4:50 ` [PATCH 08/10] KVM: Add assigned_device_msi_dispatch() Sheng Yang
` (2 subsequent siblings)
9 siblings, 1 reply; 32+ messages in thread
From: Sheng Yang @ 2008-10-31 4:50 UTC (permalink / raw)
To: Avi Kivity; +Cc: kvm, Sheng Yang
Signed-off-by: Sheng Yang <sheng@linux.intel.com>
---
arch/x86/include/asm/msidef.h | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/arch/x86/include/asm/msidef.h b/arch/x86/include/asm/msidef.h
index 6706b30..32edc7f 100644
--- a/arch/x86/include/asm/msidef.h
+++ b/arch/x86/include/asm/msidef.h
@@ -17,6 +17,8 @@
#define MSI_DATA_DELIVERY_MODE_SHIFT 8
#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)
--
1.5.4.5
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 07/10] x86: Add MSI delivery mode value
2008-10-31 4:50 ` [PATCH 07/10] x86: Add MSI delivery mode value Sheng Yang
@ 2008-11-04 11:09 ` Avi Kivity
2008-11-04 11:37 ` Sheng Yang
0 siblings, 1 reply; 32+ messages in thread
From: Avi Kivity @ 2008-11-04 11:09 UTC (permalink / raw)
To: Sheng Yang; +Cc: kvm
Sheng Yang wrote:
> Signed-off-by: Sheng Yang <sheng@linux.intel.com>
> ---
> arch/x86/include/asm/msidef.h | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/arch/x86/include/asm/msidef.h b/arch/x86/include/asm/msidef.h
> index 6706b30..32edc7f 100644
> --- a/arch/x86/include/asm/msidef.h
> +++ b/arch/x86/include/asm/msidef.h
> @@ -17,6 +17,8 @@
> #define MSI_DATA_DELIVERY_MODE_SHIFT 8
> #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
>
Shouldn't _FIXED be defined in terms of _FIXED_VAL then?
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 07/10] x86: Add MSI delivery mode value
2008-11-04 11:09 ` Avi Kivity
@ 2008-11-04 11:37 ` Sheng Yang
0 siblings, 0 replies; 32+ messages in thread
From: Sheng Yang @ 2008-11-04 11:37 UTC (permalink / raw)
To: Avi Kivity; +Cc: kvm
On Tuesday 04 November 2008 19:09:15 Avi Kivity wrote:
> Sheng Yang wrote:
> > Signed-off-by: Sheng Yang <sheng@linux.intel.com>
> > ---
> > arch/x86/include/asm/msidef.h | 2 ++
> > 1 files changed, 2 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/msidef.h
> > b/arch/x86/include/asm/msidef.h index 6706b30..32edc7f 100644
> > --- a/arch/x86/include/asm/msidef.h
> > +++ b/arch/x86/include/asm/msidef.h
> > @@ -17,6 +17,8 @@
> > #define MSI_DATA_DELIVERY_MODE_SHIFT 8
> > #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
>
> Shouldn't _FIXED be defined in terms of _FIXED_VAL then?
Yes... It's a compromise... Well, if you think it's OK, I would make it more
elegant (rename macro on host side, I would do the same thing to IA64 sooner
or later).
--
regards
Yang, Sheng
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 08/10] KVM: Add assigned_device_msi_dispatch()
2008-10-31 4:50 [PATCH 0/10][v4] Enable MSI for KVM assigned device kernel part Sheng Yang
` (6 preceding siblings ...)
2008-10-31 4:50 ` [PATCH 07/10] x86: Add MSI delivery mode value Sheng Yang
@ 2008-10-31 4:50 ` Sheng Yang
2008-11-04 11:17 ` Avi Kivity
2008-10-31 4:50 ` [PATCH 09/10] KVM: Enable MSI for device assignment Sheng Yang
2008-10-31 4:50 ` [PATCH 10/10] KVM: MSI to INTx translate Sheng Yang
9 siblings, 1 reply; 32+ messages in thread
From: Sheng Yang @ 2008-10-31 4:50 UTC (permalink / raw)
To: Avi Kivity; +Cc: kvm, Sheng Yang
The function is used to dispatch MSI to lapic according to MSI message
address and message data.
(Any way to replace macro GET_VAL_FROM_SHIFT()?)
Signed-off-by: Sheng Yang <sheng@linux.intel.com>
---
virt/kvm/kvm_main.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 55 insertions(+), 0 deletions(-)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index c69c108..d23f555 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -47,6 +47,10 @@
#include <asm/uaccess.h>
#include <asm/pgtable.h>
+#ifdef CONFIG_X86
+#include <asm/msidef.h>
+#endif
+
#ifdef KVM_COALESCED_MMIO_PAGE_OFFSET
#include "coalesced_mmio.h"
#endif
@@ -78,6 +82,57 @@ static long kvm_vcpu_ioctl(struct file *file, unsigned int ioctl,
bool kvm_rebooting;
#ifdef KVM_CAP_DEVICE_ASSIGNMENT
+
+#ifdef CONFIG_X86
+static void assigned_device_msi_dispatch(struct kvm_assigned_dev_kernel *dev)
+{
+#define GET_VAL_FROM_SHIFT(v, s) (((v) & (1 << s)) >> s)
+ int vcpu_id;
+ struct kvm_vcpu *vcpu;
+ struct kvm_ioapic *ioapic = ioapic_irqchip(dev->kvm);
+ u8 dest_id = (dev->guest_msi_addr & MSI_ADDR_DEST_ID_MASK)
+ >> MSI_ADDR_DEST_ID_SHIFT;
+ u8 vector = (dev->guest_msi_data & MSI_DATA_VECTOR_MASK)
+ >> MSI_DATA_VECTOR_SHIFT;
+ u8 dest_mode = GET_VAL_FROM_SHIFT(dev->guest_msi_addr,
+ MSI_ADDR_DEST_MODE_SHIFT);
+ u8 trig_mode = GET_VAL_FROM_SHIFT(dev->guest_msi_data,
+ MSI_DATA_TRIGGER_SHIFT);
+ u8 delivery_mode = GET_VAL_FROM_SHIFT(dev->guest_msi_data,
+ MSI_DATA_DELIVERY_MODE_SHIFT);
+ u32 deliver_bitmask;
+
+ 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 "kvm: 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 "kvm: unsupported MSI delivery mode\n");
+ }
+}
+#else
+static void assigned_device_msi_dispatch(struct kvm_assigned_dev_kernel *dev) {}
+#endif
+
static struct kvm_assigned_dev_kernel *kvm_find_assigned_dev(struct list_head *head,
int assigned_dev_id)
{
--
1.5.4.5
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 08/10] KVM: Add assigned_device_msi_dispatch()
2008-10-31 4:50 ` [PATCH 08/10] KVM: Add assigned_device_msi_dispatch() Sheng Yang
@ 2008-11-04 11:17 ` Avi Kivity
2008-11-04 11:17 ` Sheng Yang
0 siblings, 1 reply; 32+ messages in thread
From: Avi Kivity @ 2008-11-04 11:17 UTC (permalink / raw)
To: Sheng Yang; +Cc: kvm
Sheng Yang wrote:
> The function is used to dispatch MSI to lapic according to MSI message
> address and message data.
>
> (Any way to replace macro GET_VAL_FROM_SHIFT()?)
>
> +#ifdef CONFIG_X86
> +#include <asm/msidef.h>
> +#endif
>
The include can be unconditional.
> +
> +#ifdef CONFIG_X86
> +static void assigned_device_msi_dispatch(struct kvm_assigned_dev_kernel *dev)
> +{
> +#define GET_VAL_FROM_SHIFT(v, s) (((v) & (1 << s)) >> s)
>
test_bit() ?
> + int vcpu_id;
> + struct kvm_vcpu *vcpu;
> + struct kvm_ioapic *ioapic = ioapic_irqchip(dev->kvm);
> + u8 dest_id = (dev->guest_msi_addr & MSI_ADDR_DEST_ID_MASK)
> + >> MSI_ADDR_DEST_ID_SHIFT;
> + u8 vector = (dev->guest_msi_data & MSI_DATA_VECTOR_MASK)
> + >> MSI_DATA_VECTOR_SHIFT;
> + u8 dest_mode = GET_VAL_FROM_SHIFT(dev->guest_msi_addr,
> + MSI_ADDR_DEST_MODE_SHIFT);
> + u8 trig_mode = GET_VAL_FROM_SHIFT(dev->guest_msi_data,
> + MSI_DATA_TRIGGER_SHIFT);
> + u8 delivery_mode = GET_VAL_FROM_SHIFT(dev->guest_msi_data,
> + MSI_DATA_DELIVERY_MODE_SHIFT);
>
int is usually better than u8.
> + u32 deliver_bitmask;
> +
> + case MSI_DATA_DELIVERY_FIXED_VAL:
> + for (vcpu_id = 0; deliver_bitmask != 0; vcpu_id++) {
>
__ffs() and __clear_bit() can make this a bit faster. Can be done later
though.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 08/10] KVM: Add assigned_device_msi_dispatch()
2008-11-04 11:17 ` Avi Kivity
@ 2008-11-04 11:17 ` Sheng Yang
0 siblings, 0 replies; 32+ messages in thread
From: Sheng Yang @ 2008-11-04 11:17 UTC (permalink / raw)
To: Avi Kivity; +Cc: kvm
On Tuesday 04 November 2008 19:17:03 Avi Kivity wrote:
> Sheng Yang wrote:
> > The function is used to dispatch MSI to lapic according to MSI message
> > address and message data.
> >
> > (Any way to replace macro GET_VAL_FROM_SHIFT()?)
> >
> >
> > +#ifdef CONFIG_X86
> > +#include <asm/msidef.h>
> > +#endif
>
> The include can be unconditional.
it's pity that IA64 don't have this head file... I will separate the msi.c and
add this later.
>
> > +
> > +#ifdef CONFIG_X86
> > +static void assigned_device_msi_dispatch(struct kvm_assigned_dev_kernel
> > *dev) +{
> > +#define GET_VAL_FROM_SHIFT(v, s) (((v) & (1 << s)) >> s)
>
> test_bit() ?
Oh, indeed here. :)
>
> > + int vcpu_id;
> > + struct kvm_vcpu *vcpu;
> > + struct kvm_ioapic *ioapic = ioapic_irqchip(dev->kvm);
> > + u8 dest_id = (dev->guest_msi_addr & MSI_ADDR_DEST_ID_MASK)
> > + >> MSI_ADDR_DEST_ID_SHIFT;
> > + u8 vector = (dev->guest_msi_data & MSI_DATA_VECTOR_MASK)
> > + >> MSI_DATA_VECTOR_SHIFT;
> > + u8 dest_mode = GET_VAL_FROM_SHIFT(dev->guest_msi_addr,
> > + MSI_ADDR_DEST_MODE_SHIFT);
> > + u8 trig_mode = GET_VAL_FROM_SHIFT(dev->guest_msi_data,
> > + MSI_DATA_TRIGGER_SHIFT);
> > + u8 delivery_mode = GET_VAL_FROM_SHIFT(dev->guest_msi_data,
> > + MSI_DATA_DELIVERY_MODE_SHIFT);
>
> int is usually better than u8.
OK.
>
> > + u32 deliver_bitmask;
> > +
> > + case MSI_DATA_DELIVERY_FIXED_VAL:
> > + for (vcpu_id = 0; deliver_bitmask != 0; vcpu_id++) {
>
> __ffs() and __clear_bit() can make this a bit faster. Can be done later
> though.
OK, also would optimize IOAPIC side later.
--
regards
Yang, Sheng
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 09/10] KVM: Enable MSI for device assignment
2008-10-31 4:50 [PATCH 0/10][v4] Enable MSI for KVM assigned device kernel part Sheng Yang
` (7 preceding siblings ...)
2008-10-31 4:50 ` [PATCH 08/10] KVM: Add assigned_device_msi_dispatch() Sheng Yang
@ 2008-10-31 4:50 ` Sheng Yang
2008-11-04 11:23 ` Avi Kivity
2008-10-31 4:50 ` [PATCH 10/10] KVM: MSI to INTx translate Sheng Yang
9 siblings, 1 reply; 32+ messages in thread
From: Sheng Yang @ 2008-10-31 4:50 UTC (permalink / raw)
To: Avi Kivity; +Cc: kvm, Sheng Yang
We enable guest MSI and host MSI support in this patch. The userspace want to
enable MSI should set KVM_DEV_IRQ_ASSIGN_ENABLE_MSI in the assigned_irq's
flag. Function would return -ENOTTY if can't enable MSI, userspace shouldn't
set MSI Enable bit when KVM_ASSIGN_IRQ return -ENOTTY with
KVM_DEV_IRQ_ASSIGN_ENABLE_MSI.
Signed-off-by: Sheng Yang <sheng@linux.intel.com>
---
virt/kvm/kvm_main.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++----
1 files changed, 74 insertions(+), 6 deletions(-)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index d23f555..3ae8653 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -159,9 +159,15 @@ 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->irq_source_id,
- assigned_dev->guest_irq, 1);
+ if (assigned_dev->irq_requested_type & KVM_ASSIGNED_DEV_GUEST_INTX)
+ kvm_set_irq(assigned_dev->kvm,
+ assigned_dev->irq_source_id,
+ assigned_dev->guest_irq, 1);
+ else if (assigned_dev->irq_requested_type &
+ KVM_ASSIGNED_DEV_GUEST_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);
}
@@ -197,6 +203,8 @@ static void kvm_free_assigned_device(struct kvm *kvm,
{
if (irqchip_in_kernel(kvm) && assigned_dev->irq_requested_type)
free_irq(assigned_dev->host_irq, (void *)assigned_dev);
+ if (assigned_dev->irq_requested_type & KVM_ASSIGNED_DEV_HOST_MSI)
+ pci_disable_msi(assigned_dev->dev);
kvm_unregister_irq_ack_notifier(&assigned_dev->ack_notifier);
kvm_free_irq_source_id(kvm, assigned_dev->irq_source_id);
@@ -240,6 +248,11 @@ static int assigned_device_update_intx(struct kvm *kvm,
return 0;
if (irqchip_in_kernel(kvm)) {
+ if (adev->irq_requested_type & KVM_ASSIGNED_DEV_HOST_MSI) {
+ free_irq(adev->host_irq, (void *)kvm);
+ pci_disable_msi(adev->dev);
+ }
+
if (!capable(CAP_SYS_RAWIO))
return -EPERM;
@@ -263,6 +276,40 @@ static int assigned_device_update_intx(struct kvm *kvm,
return 0;
}
+#ifdef CONFIG_X86
+static int assigned_device_update_msi(struct kvm *kvm,
+ struct kvm_assigned_dev_kernel *adev,
+ struct kvm_assigned_irq *airq)
+{
+ int r;
+
+ adev->guest_msi_addr = airq->guest_msi_addr;
+ adev->guest_msi_data = airq->guest_msi_data;
+ adev->ack_notifier.gsi = -1;
+
+ if (adev->irq_requested_type & KVM_ASSIGNED_DEV_HOST_MSI)
+ return 0;
+
+ if (irqchip_in_kernel(kvm)) {
+ if (adev->irq_requested_type & KVM_ASSIGNED_DEV_HOST_INTX)
+ free_irq(adev->host_irq, (void *)adev);
+
+ r = pci_enable_msi(adev->dev);
+ if (r)
+ return r;
+
+ adev->host_irq = adev->dev->irq;
+ if (request_irq(adev->host_irq, kvm_assigned_dev_intr, 0,
+ "kvm_assigned_msi_device", (void *)adev))
+ return -EIO;
+ }
+
+ adev->irq_requested_type = KVM_ASSIGNED_DEV_GUEST_MSI |
+ KVM_ASSIGNED_DEV_HOST_MSI;
+ return 0;
+}
+#endif
+
static int kvm_vm_ioctl_assign_irq(struct kvm *kvm,
struct kvm_assigned_irq
*assigned_irq)
@@ -299,9 +346,30 @@ static int kvm_vm_ioctl_assign_irq(struct kvm *kvm,
}
}
- r = assigned_device_update_intx(kvm, match, assigned_irq);
- if (r)
- goto out_release;
+ if (assigned_irq->flags & KVM_DEV_IRQ_ASSIGN_ENABLE_MSI) {
+#ifdef CONFIG_X86
+ r = assigned_device_update_msi(kvm, match, assigned_irq);
+ if (r) {
+ printk(KERN_WARNING "kvm: failed to enable "
+ "MSI device!\n");
+ goto out_release;
+ }
+#else
+ r = -ENOTTY;
+#endif
+ } else if (assigned_irq->host_irq == 0 && match->dev->irq == 0) {
+ /* Host device IRQ 0 means don't support INTx */
+ printk(KERN_WARNING "kvm: wait device to enable MSI!\n");
+ r = 0;
+ } else {
+ /* Non-sharing INTx mode */
+ r = assigned_device_update_intx(kvm, match, assigned_irq);
+ if (r) {
+ printk(KERN_WARNING "kvm: failed to enable "
+ "INTx device!\n");
+ goto out_release;
+ }
+ }
mutex_unlock(&kvm->lock);
return r;
--
1.5.4.5
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 09/10] KVM: Enable MSI for device assignment
2008-10-31 4:50 ` [PATCH 09/10] KVM: Enable MSI for device assignment Sheng Yang
@ 2008-11-04 11:23 ` Avi Kivity
2008-11-04 13:25 ` Sheng Yang
0 siblings, 1 reply; 32+ messages in thread
From: Avi Kivity @ 2008-11-04 11:23 UTC (permalink / raw)
To: Sheng Yang; +Cc: kvm
Sheng Yang wrote:
> We enable guest MSI and host MSI support in this patch. The userspace want to
> enable MSI should set KVM_DEV_IRQ_ASSIGN_ENABLE_MSI in the assigned_irq's
> flag. Function would return -ENOTTY if can't enable MSI, userspace shouldn't
> set MSI Enable bit when KVM_ASSIGN_IRQ return -ENOTTY with
> KVM_DEV_IRQ_ASSIGN_ENABLE_MSI.
>
Need a KVM_CAP_MSI so that userspace can tell before trying.
> + if (assigned_dev->irq_requested_type & KVM_ASSIGNED_DEV_GUEST_INTX)
> + kvm_set_irq(assigned_dev->kvm,
> + assigned_dev->irq_source_id,
> + assigned_dev->guest_irq, 1);
> + else if (assigned_dev->irq_requested_type &
> + KVM_ASSIGNED_DEV_GUEST_MSI) {
> + assigned_device_msi_dispatch(assigned_dev);
> + enable_irq(assigned_dev->host_irq);
> + }
>
Hmm. This means the only path that can trigger msi is device
assignment. But we also want virtio-pci to use msi.
How about modifying kvm_set_irq() to dispatch either msi or normal
irqs? We'd need a 'struct kvm_guest_irq' if we can't define the msi
using the bits available in gsi (and also define a new KVM_IRQ_LINE... I
really hope we can fit the msi info in gsi).
The device-assignment-independent parts should be in a separate patch.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 09/10] KVM: Enable MSI for device assignment
2008-11-04 11:23 ` Avi Kivity
@ 2008-11-04 13:25 ` Sheng Yang
2008-11-04 13:32 ` Avi Kivity
0 siblings, 1 reply; 32+ messages in thread
From: Sheng Yang @ 2008-11-04 13:25 UTC (permalink / raw)
To: Avi Kivity; +Cc: Sheng Yang, kvm
On Tue, Nov 04, 2008 at 01:23:04PM +0200, Avi Kivity wrote:
> Sheng Yang wrote:
>> We enable guest MSI and host MSI support in this patch. The userspace want to
>> enable MSI should set KVM_DEV_IRQ_ASSIGN_ENABLE_MSI in the assigned_irq's
>> flag. Function would return -ENOTTY if can't enable MSI, userspace shouldn't
>> set MSI Enable bit when KVM_ASSIGN_IRQ return -ENOTTY with
>> KVM_DEV_IRQ_ASSIGN_ENABLE_MSI.
>>
>
> Need a KVM_CAP_MSI so that userspace can tell before trying.
>
>> + if (assigned_dev->irq_requested_type & KVM_ASSIGNED_DEV_GUEST_INTX)
>> + kvm_set_irq(assigned_dev->kvm,
>> + assigned_dev->irq_source_id,
>> + assigned_dev->guest_irq, 1);
>> + else if (assigned_dev->irq_requested_type &
>> + KVM_ASSIGNED_DEV_GUEST_MSI) {
>> + assigned_device_msi_dispatch(assigned_dev);
>> + enable_irq(assigned_dev->host_irq);
>> + }
>>
>
> Hmm. This means the only path that can trigger msi is device
> assignment. But we also want virtio-pci to use msi.
>
> How about modifying kvm_set_irq() to dispatch either msi or normal irqs?
> We'd need a 'struct kvm_guest_irq' if we can't define the msi using the
> bits available in gsi (and also define a new KVM_IRQ_LINE... I really
> hope we can fit the msi info in gsi).
>
> The device-assignment-independent parts should be in a separate patch.
One question here: did the architectures other than X86 and IA64 need
dispatch MSI support? If so, I am afraid reuse assigned_device_msi_dispatch
(or similar solution) won't be elegant (though I already lean toward a arch
specific dispatch function).
I am not familiar with virtio-pci and how it works, it would be good if
it can be illustrated a little more. But I guess what you means is only set
gsi can result in kvm_set_irq() deliver the MSI correctly. I think this can
be done. Associating gsi with guest_msi_addr and guest_msi_data in a linked
list, for gsi >= IOAPIC_PINS, kvm_set_irq() checked the list and deliver the
interrupt. Of course, some lock should be taken to maintain the list, maybe
kvm->lock in first step as we used for kvm_set_irq (kvm->lock become more
and more bigger...).
--
regards
Yang, Sheng
>
> --
> error compiling committee.c: too many arguments to function
>
> --
> 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
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 09/10] KVM: Enable MSI for device assignment
2008-11-04 13:25 ` Sheng Yang
@ 2008-11-04 13:32 ` Avi Kivity
2008-11-04 13:58 ` Sheng Yang
0 siblings, 1 reply; 32+ messages in thread
From: Avi Kivity @ 2008-11-04 13:32 UTC (permalink / raw)
To: Avi Kivity, Sheng Yang, kvm
Sheng Yang wrote:
> One question here: did the architectures other than X86 and IA64 need
> dispatch MSI support?
s390 doesn't. ppc may, one day in the future. Not now I think.
> If so, I am afraid reuse assigned_device_msi_dispatch
> (or similar solution) won't be elegant (though I already lean toward a arch
> specific dispatch function).
>
> I am not familiar with virtio-pci and how it works, it would be good if
> it can be illustrated a little more.
For this context, it's just a pci device. qemu calls KVM_IRQ_LINE to
inject an irq, and the pci layer in the guest delivers the irq to the
virtio-pci driver.
In fact, it's not just for pci. We could msi-enable e1000 and get
improved performance there as well.
> But I guess what you means is only set
> gsi can result in kvm_set_irq() deliver the MSI correctly. I think this can
> be done. Associating gsi with guest_msi_addr and guest_msi_data in a linked
> list, for gsi >= IOAPIC_PINS,
Let's say, (gsi >> 24) == 1. That gives us 16M potential MSIs.
> kvm_set_irq() checked the list and deliver the
> interrupt. Of course, some lock should be taken to maintain the list, maybe
> kvm->lock in first step as we used for kvm_set_irq (kvm->lock become more
> and more bigger...).
>
I think kvm_set_irq() already takes kvm->lock.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 09/10] KVM: Enable MSI for device assignment
2008-11-04 13:32 ` Avi Kivity
@ 2008-11-04 13:58 ` Sheng Yang
2008-11-04 14:35 ` Avi Kivity
0 siblings, 1 reply; 32+ messages in thread
From: Sheng Yang @ 2008-11-04 13:58 UTC (permalink / raw)
To: Avi Kivity; +Cc: Sheng Yang, kvm
On Tue, Nov 04, 2008 at 03:32:49PM +0200, Avi Kivity wrote:
> Sheng Yang wrote:
>> One question here: did the architectures other than X86 and IA64 need
>> dispatch MSI support?
>
> s390 doesn't. ppc may, one day in the future. Not now I think.
OK, so reuse is enough for today.
>
>> If so, I am afraid reuse assigned_device_msi_dispatch
>> (or similar solution) won't be elegant (though I already lean toward a arch
>> specific dispatch function).
>>
>> I am not familiar with virtio-pci and how it works, it would be good if
>> it can be illustrated a little more.
>
> For this context, it's just a pci device. qemu calls KVM_IRQ_LINE to
> inject an irq, and the pci layer in the guest delivers the irq to the
> virtio-pci driver.
Got it.
>
> In fact, it's not just for pci. We could msi-enable e1000 and get
> improved performance there as well.
E1000? Don't understand... Sounds like INTx->MSI...
>
>> But I guess what you means is only set
>> gsi can result in kvm_set_irq() deliver the MSI correctly. I think this can
>> be done. Associating gsi with guest_msi_addr and guest_msi_data in a linked
>> list, for gsi >= IOAPIC_PINS,
> Let's say, (gsi >> 24) == 1. That gives us 16M potential MSIs.
Confused... GSI become a bitmap?
>
>> kvm_set_irq() checked the list and deliver the
>> interrupt. Of course, some lock should be taken to maintain the list, maybe
>> kvm->lock in first step as we used for kvm_set_irq (kvm->lock become more
>> and more bigger...).
>>
>
> I think kvm_set_irq() already takes kvm->lock.
Yes. So I means maybe we can improve the granularity of the lock in interrupt
handling part in the future.
--
regards
Yang, Sheng
>
> --
> error compiling committee.c: too many arguments to function
>
> --
> 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
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 09/10] KVM: Enable MSI for device assignment
2008-11-04 13:58 ` Sheng Yang
@ 2008-11-04 14:35 ` Avi Kivity
2008-11-04 14:57 ` Sheng Yang
0 siblings, 1 reply; 32+ messages in thread
From: Avi Kivity @ 2008-11-04 14:35 UTC (permalink / raw)
To: Avi Kivity, Sheng Yang, kvm
Sheng Yang wrote:
>> In fact, it's not just for pci. We could msi-enable e1000 and get
>> improved performance there as well.
>>
>
> E1000? Don't understand... Sounds like INTx->MSI...
>
There are msi-capable e1000 cards. If we upgrade qemu's e1000 to
support msi, and if the guest supports msi, it will be msi all the way.
>>> But I guess what you means is only set
>>> gsi can result in kvm_set_irq() deliver the MSI correctly. I think this can
>>> be done. Associating gsi with guest_msi_addr and guest_msi_data in a linked
>>> list, for gsi >= IOAPIC_PINS,
>>>
>> Let's say, (gsi >> 24) == 1. That gives us 16M potential MSIs.
>>
>
> Confused... GSI become a bitmap?
>
A structure...
struct gsi {
u32 n : 24;
u32 type : 8; // 0 -> classic gsi, 1 -> msi
};
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 09/10] KVM: Enable MSI for device assignment
2008-11-04 14:35 ` Avi Kivity
@ 2008-11-04 14:57 ` Sheng Yang
2008-11-04 15:01 ` Avi Kivity
0 siblings, 1 reply; 32+ messages in thread
From: Sheng Yang @ 2008-11-04 14:57 UTC (permalink / raw)
To: Avi Kivity; +Cc: Sheng Yang, kvm
On Tue, Nov 04, 2008 at 04:35:19PM +0200, Avi Kivity wrote:
> Sheng Yang wrote:
>>> In fact, it's not just for pci. We could msi-enable e1000 and get
>>> improved performance there as well.
>>>
>>
>> E1000? Don't understand... Sounds like INTx->MSI...
>>
>
> There are msi-capable e1000 cards. If we upgrade qemu's e1000 to
> support msi, and if the guest supports msi, it will be msi all the way.
Oh, forgot we got e1000 emulation in qemu...
>
>>>> But I guess what you means is only set
>>>> gsi can result in kvm_set_irq() deliver the MSI correctly. I think this can
>>>> be done. Associating gsi with guest_msi_addr and guest_msi_data in a linked
>>>> list, for gsi >= IOAPIC_PINS,
>>>>
>>> Let's say, (gsi >> 24) == 1. That gives us 16M potential MSIs.
>>>
>>
>> Confused... GSI become a bitmap?
>>
>
> A structure...
>
> struct gsi {
> u32 n : 24;
> u32 type : 8; // 0 -> classic gsi, 1 -> msi
> };
I just don't understand why we need 24 bits for n? GSI can be that wide?
--
regards
Yang, Sheng
>
>
> --
> error compiling committee.c: too many arguments to function
>
> --
> 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
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 09/10] KVM: Enable MSI for device assignment
2008-11-04 14:57 ` Sheng Yang
@ 2008-11-04 15:01 ` Avi Kivity
2008-11-04 15:14 ` Sheng Yang
0 siblings, 1 reply; 32+ messages in thread
From: Avi Kivity @ 2008-11-04 15:01 UTC (permalink / raw)
To: Avi Kivity, Sheng Yang, kvm
Sheng Yang wrote:
>> struct gsi {
>> u32 n : 24;
>> u32 type : 8; // 0 -> classic gsi, 1 -> msi
>> };
>>
>
> I just don't understand why we need 24 bits for n? GSI can be that wide?
>
I don't think so, but who knows what we'll need for type == 2?
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 09/10] KVM: Enable MSI for device assignment
2008-11-04 15:01 ` Avi Kivity
@ 2008-11-04 15:14 ` Sheng Yang
0 siblings, 0 replies; 32+ messages in thread
From: Sheng Yang @ 2008-11-04 15:14 UTC (permalink / raw)
To: Avi Kivity; +Cc: Sheng Yang, kvm
On Tue, Nov 04, 2008 at 05:01:04PM +0200, Avi Kivity wrote:
> Sheng Yang wrote:
>>> struct gsi {
>>> u32 n : 24;
>>> u32 type : 8; // 0 -> classic gsi, 1 -> msi
>>> };
>>>
>>
>> I just don't understand why we need 24 bits for n? GSI can be that wide?
>>
>
> I don't think so, but who knows what we'll need for type == 2?
Oh, OK, don't take it too seriously. I just took it too seriously to think
that you got some certain defines to support magic number 24. :)
--
regards
Yang, Sheng
> --
> error compiling committee.c: too many arguments to function
>
> --
> 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
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 10/10] KVM: MSI to INTx translate
2008-10-31 4:50 [PATCH 0/10][v4] Enable MSI for KVM assigned device kernel part Sheng Yang
` (8 preceding siblings ...)
2008-10-31 4:50 ` [PATCH 09/10] KVM: Enable MSI for device assignment Sheng Yang
@ 2008-10-31 4:50 ` Sheng Yang
2008-11-04 11:25 ` Avi Kivity
9 siblings, 1 reply; 32+ messages in thread
From: Sheng Yang @ 2008-10-31 4:50 UTC (permalink / raw)
To: Avi Kivity; +Cc: kvm, Sheng Yang
Now we use MSI as default one, and translate MSI to INTx when guest need INTx
rather than MSI. For legacy device, we provide support for non-sharing host
IRQ.
Signed-off-by: Sheng Yang <sheng@linux.intel.com>
---
virt/kvm/kvm_main.c | 52 ++++++++++++++++++++++++++++----------------------
1 files changed, 29 insertions(+), 23 deletions(-)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 3ae8653..5aa975e 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -241,6 +241,8 @@ static int assigned_device_update_intx(struct kvm *kvm,
struct kvm_assigned_dev_kernel *adev,
struct kvm_assigned_irq *airq)
{
+ BUG_ON(!!adev->irq_requested_type & KVM_ASSIGNED_DEV_HOST_MSI);
+
adev->guest_irq = airq->guest_irq;
adev->ack_notifier.gsi = airq->guest_irq;
@@ -248,11 +250,6 @@ static int assigned_device_update_intx(struct kvm *kvm,
return 0;
if (irqchip_in_kernel(kvm)) {
- if (adev->irq_requested_type & KVM_ASSIGNED_DEV_HOST_MSI) {
- free_irq(adev->host_irq, (void *)kvm);
- pci_disable_msi(adev->dev);
- }
-
if (!capable(CAP_SYS_RAWIO))
return -EPERM;
@@ -281,31 +278,32 @@ static int assigned_device_update_msi(struct kvm *kvm,
struct kvm_assigned_dev_kernel *adev,
struct kvm_assigned_irq *airq)
{
- int r;
+ BUG_ON(!!adev->irq_requested_type & KVM_ASSIGNED_DEV_HOST_INTX);
- adev->guest_msi_addr = airq->guest_msi_addr;
- adev->guest_msi_data = airq->guest_msi_data;
- adev->ack_notifier.gsi = -1;
+ if (airq->flags & KVM_DEV_IRQ_ASSIGN_ENABLE_MSI) {
+ adev->irq_requested_type |= KVM_ASSIGNED_DEV_GUEST_MSI;
+ adev->irq_requested_type &= ~KVM_ASSIGNED_DEV_GUEST_INTX;
+ adev->guest_msi_addr = airq->guest_msi_addr;
+ adev->guest_msi_data = airq->guest_msi_data;
+ adev->ack_notifier.gsi = -1;
+ } else {
+ adev->irq_requested_type |= KVM_ASSIGNED_DEV_GUEST_INTX;
+ adev->irq_requested_type &= ~KVM_ASSIGNED_DEV_GUEST_MSI;
+ adev->guest_irq = airq->guest_irq;
+ adev->ack_notifier.gsi = airq->guest_irq;
+ }
if (adev->irq_requested_type & KVM_ASSIGNED_DEV_HOST_MSI)
return 0;
if (irqchip_in_kernel(kvm)) {
- if (adev->irq_requested_type & KVM_ASSIGNED_DEV_HOST_INTX)
- free_irq(adev->host_irq, (void *)adev);
-
- r = pci_enable_msi(adev->dev);
- if (r)
- return r;
-
adev->host_irq = adev->dev->irq;
if (request_irq(adev->host_irq, kvm_assigned_dev_intr, 0,
"kvm_assigned_msi_device", (void *)adev))
return -EIO;
}
- adev->irq_requested_type = KVM_ASSIGNED_DEV_GUEST_MSI |
- KVM_ASSIGNED_DEV_HOST_MSI;
+ adev->irq_requested_type |= KVM_ASSIGNED_DEV_HOST_MSI;
return 0;
}
#endif
@@ -343,11 +341,18 @@ static int kvm_vm_ioctl_assign_irq(struct kvm *kvm,
goto out_release;
else
match->irq_source_id = r;
+
+#ifdef CONFIG_X86
+ /* Determine host device irq type, we can know the
+ * result from dev->msi_enabled */
+ pci_enable_msi(match->dev);
+#endif
}
}
- if (assigned_irq->flags & KVM_DEV_IRQ_ASSIGN_ENABLE_MSI) {
+ if (match->dev->msi_enabled) {
#ifdef CONFIG_X86
+ /* Device support MSI */
r = assigned_device_update_msi(kvm, match, assigned_irq);
if (r) {
printk(KERN_WARNING "kvm: failed to enable "
@@ -358,9 +363,10 @@ static int kvm_vm_ioctl_assign_irq(struct kvm *kvm,
r = -ENOTTY;
#endif
} else if (assigned_irq->host_irq == 0 && match->dev->irq == 0) {
- /* Host device IRQ 0 means don't support INTx */
- printk(KERN_WARNING "kvm: wait device to enable MSI!\n");
- r = 0;
+ /* Device only support MSI but can't enable */
+ printk(KERN_WARNING "kvm: failed to enable MSI device!\n");
+ r = -ENOTTY;
+ goto out_release;
} else {
/* Non-sharing INTx mode */
r = assigned_device_update_intx(kvm, match, assigned_irq);
--
1.5.4.5
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 10/10] KVM: MSI to INTx translate
2008-10-31 4:50 ` [PATCH 10/10] KVM: MSI to INTx translate Sheng Yang
@ 2008-11-04 11:25 ` Avi Kivity
2008-11-04 11:40 ` Sheng Yang
0 siblings, 1 reply; 32+ messages in thread
From: Avi Kivity @ 2008-11-04 11:25 UTC (permalink / raw)
To: Sheng Yang; +Cc: kvm
Sheng Yang wrote:
> Now we use MSI as default one, and translate MSI to INTx when guest need INTx
> rather than MSI. For legacy device, we provide support for non-sharing host
> IRQ.
>
>
Won't we run into trouble with edge-triggered vs level-triggered.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 10/10] KVM: MSI to INTx translate
2008-11-04 11:25 ` Avi Kivity
@ 2008-11-04 11:40 ` Sheng Yang
2008-11-04 11:57 ` Avi Kivity
0 siblings, 1 reply; 32+ messages in thread
From: Sheng Yang @ 2008-11-04 11:40 UTC (permalink / raw)
To: Avi Kivity; +Cc: kvm
On Tuesday 04 November 2008 19:25:34 Avi Kivity wrote:
> Sheng Yang wrote:
> > Now we use MSI as default one, and translate MSI to INTx when guest need
> > INTx rather than MSI. For legacy device, we provide support for
> > non-sharing host IRQ.
>
> Won't we run into trouble with edge-triggered vs level-triggered.
Yes, in theory, we got a little trouble, but not that much.
The mechanism based on a assumption: every time guest got a interrupt, it
would deassert the interrupt source sooner or later. So we take the Ack action
as the same as deassert the IRQ line here (unmasked the IRQ).
So the biggest known issue is, after guest got the interrupt, rather than
deassert it, guest driver took the advantage of level triggered interrupt, and
just want to deassert after it receive it for e.g. 10 times. Then we would got
the trouble. But it's so tricky and we don't know any device by now take this
way yet. Of course they can, so we also suggest to use a whitelist/blacklist
for this approach. But this should work in most condition.
Yeah, it's aggressive, but we think it's reasonable, and tested OK (in my
limited environment). Also I heard that Hyper-V or Vmware did this.
Any more concerns? Hope we didn't neglect something.
--
regards
Yang, Sheng
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 10/10] KVM: MSI to INTx translate
2008-11-04 11:40 ` Sheng Yang
@ 2008-11-04 11:57 ` Avi Kivity
2008-11-04 12:55 ` Sheng Yang
0 siblings, 1 reply; 32+ messages in thread
From: Avi Kivity @ 2008-11-04 11:57 UTC (permalink / raw)
To: Sheng Yang; +Cc: kvm
Sheng Yang wrote:
> Yes, in theory, we got a little trouble, but not that much.
>
> The mechanism based on a assumption: every time guest got a interrupt, it
> would deassert the interrupt source sooner or later. So we take the Ack action
> as the same as deassert the IRQ line here (unmasked the IRQ).
>
> So the biggest known issue is, after guest got the interrupt, rather than
> deassert it, guest driver took the advantage of level triggered interrupt, and
> just want to deassert after it receive it for e.g. 10 times. Then we would got
> the trouble. But it's so tricky and we don't know any device by now take this
> way yet. Of course they can, so we also suggest to use a whitelist/blacklist
> for this approach. But this should work in most condition.
>
> Yeah, it's aggressive, but we think it's reasonable, and tested OK (in my
> limited environment). Also I heard that Hyper-V or Vmware did this.
>
> Any more concerns? Hope we didn't neglect something.
>
Please add a module parameter to disable msi->pciirq conversion in that
case. So if someone runs into trouble, we can try out the alternative.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 10/10] KVM: MSI to INTx translate
2008-11-04 11:57 ` Avi Kivity
@ 2008-11-04 12:55 ` Sheng Yang
0 siblings, 0 replies; 32+ messages in thread
From: Sheng Yang @ 2008-11-04 12:55 UTC (permalink / raw)
To: Avi Kivity; +Cc: Sheng Yang, kvm
On Tue, Nov 04, 2008 at 01:57:56PM +0200, Avi Kivity wrote:
> Sheng Yang wrote:
>> Yes, in theory, we got a little trouble, but not that much.
>>
>> The mechanism based on a assumption: every time guest got a interrupt,
>> it would deassert the interrupt source sooner or later. So we take the
>> Ack action as the same as deassert the IRQ line here (unmasked the
>> IRQ).
>>
>> So the biggest known issue is, after guest got the interrupt, rather
>> than deassert it, guest driver took the advantage of level triggered
>> interrupt, and just want to deassert after it receive it for e.g. 10
>> times. Then we would got the trouble. But it's so tricky and we don't
>> know any device by now take this way yet. Of course they can, so we
>> also suggest to use a whitelist/blacklist for this approach. But this
>> should work in most condition.
>>
>> Yeah, it's aggressive, but we think it's reasonable, and tested OK (in
>> my limited environment). Also I heard that Hyper-V or Vmware did this.
>>
>> Any more concerns? Hope we didn't neglect something.
>>
>
> Please add a module parameter to disable msi->pciirq conversion in that
> case. So if someone runs into trouble, we can try out the alternative.
Yes of course. :)
--
regards
Yang, Sheng
> --
> error compiling committee.c: too many arguments to function
>
> --
> 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
^ permalink raw reply [flat|nested] 32+ messages in thread