KVM & genirq: Enable adaptive IRQ sharing for passed-through devices
Subsystems (5)
v2 cover KVM & genirq: Enable adaptive IRQ sharing for passed-through devices
Second try to allow adaptive interrupt handlers according to the line's current sharing situation. This one converts the explicit notifier interface into several extension of existing genirq APIs:
- introduce a interrupt status word that can be read by interrupt handlers to obtain the current line sharing state
- introduce IRQF_ADAPTIVE: handlers registered with this flag will be informed about an upcoming line sharing by calling the interrupt handler itself (instead of a notifier callback)
- introduce IRQF_COND_ONESHOT: IRQF_ONESHOT semantics if the line is not shared, standard semantics otherwise (removes the need to re-register handlers to switch between IRQF_SHARED and IRQF_ONESHOT)
The result may look simpler on first glance than v1, but it comes with more subtle race scenarios IMO. I thought them through, hopefully catching all, but I would appreciate any skeptical review.
I also tried to replace line-level disabling from the interrupt handler with signaling genirq to keep the line masked on handler return. However, I finally dropped this idea as it turns out to be very hard (if not impossible) to properly synchronize the generic tail (in genirq) with the driver that may want to re-enable the line at the same time (depending on the driver's internal state). But if anyone sees a magically simple way to achieve this, I'm still all ears.
Finally, the last patch in this series makes use of the new interface for KVM's PCI pass-through subsystem. KVM has to keep the interrupt source disabled while calling into the guest to handle the event. This can be done at device or line level. The former is required to share the interrupt line, the latter is an order of magnitude faster.
Beside pass-through support of KVM, further users of this new interface could become VFIO (not yet mainline) and uio_pci_generic which have to resolve the same conflict.
Note: The KVM patch depends on
http://thread.gmane.org/gmane.comp.emulators.kvm.devel/64515
Jan Kiszka (4):
genirq: Introduce driver-readable IRQ status word
genirq: Inform handler about line sharing state
genirq: Add support for IRQF_COND_ONESHOT
KVM: Allow host IRQ sharing for passed-through PCI 2.3 devices
Documentation/kvm/api.txt | 27 ++++ arch/x86/kvm/x86.c | 1 + include/linux/interrupt.h | 15 ++ include/linux/irq.h | 2 + include/linux/irqdesc.h | 2 + include/linux/kvm.h | 6 + include/linux/kvm_host.h | 10 ++- kernel/irq/irqdesc.c | 2 + kernel/irq/manage.c | 74 ++++++++++- virt/kvm/assigned-dev.c | 336 ++++++++++++++++++++++++++++++++++++++++----- 10 files changed, 437 insertions(+), 38 deletions(-)
PATCH v2 1/4 genirq: Introduce driver-readable IRQ status word
From: Jan Kiszka <[email protected]>
This associates a status word with every IRQ descriptor. Drivers can obtain its content via get_irq_status(irq). First use case will be propagating the interrupt sharing state.
--- include/linux/interrupt.h | 2 ++ include/linux/irq.h | 2 ++ kernel/irq/manage.c | 15 +++++++++++++++ 3 files changed, 19 insertions(+), 0 deletions(-)
include/linux/interrupt.h +2 -0
@@ -126,6 +126,8 @@ struct irqaction {extern irqreturn_t no_action (int cpl,void *dev_id); +extern unsigned long get_irq_status (unsigned long irq); +#ifdef CONFIG_GENERIC_HARDIRQS extern int __must_check request_threaded_irq(unsigned int irq, irq_handler_t handler,
include/linux/irq.h +2 -0
@@ -96,6 +96,7 @@ struct msi_desc;* methods, to allow shared chip implementations * @msi_desc: MSI descriptor * @affinity: IRQ affinity on SMP +* @status: driver-readable status flags (IRQS_*) * * The fields here need to overlay the ones in irq_desc until we * cleaned up the direct references and switched everything over to
@@ -108,6 +109,7 @@ struct irq_data {void *handler_data;void *chip_data;struct msi_desc *msi_desc; +unsigned long status;#ifdef CONFIG_SMP cpumask_var_t affinity;#endif
kernel/irq/manage.c +15 -0
@@ -1157,3 +1157,18 @@ int request_any_context_irq(unsigned int irq, irq_handler_t handler,return !ret ?IRQC_IS_HARDIRQ : ret; }EXPORT_SYMBOL_GPL (request_any_context_irq); + +/** +* get_irq_status - read interrupt line status word +* @irq: Interrupt line of the status word +* +* This returns the current content of the status word associated with +* the given interrupt line. See IRQS_* flags for details. +*/ +unsigned long get_irq_status (unsigned long irq) +{ +struct irq_desc *desc =irq_to_desc (irq); + +return desc ? desc->irq_data.status :0 ; +} +EXPORT_SYMBOL_GPL (get_irq_status);
Signature · 1 line
1.7.1
Re: [PATCH v2 1/4] genirq: Introduce driver-readable IRQ status word
Quoted 39 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
This associates a status word with every IRQ descriptor. Drivers can obtain its content via
get_irq_status(irq). First use case will be propagating the interrupt sharing state.Signed-off-by Jan Kiszka <[email protected]>--- include/linux/interrupt.h | 2 ++ include/linux/irq.h | 2 ++ kernel/irq/manage.c | 15 +++++++++++++++ 3 files changed, 19 insertions(+), 0 deletions(-)include/linux/interrupt.h +2
-0@@ -126,6 +126,8 @@ struct irqaction {extern irqreturn_t no_action (int cpl,void *dev_id); +extern unsigned long get_irq_status (unsigned long irq); +#ifdef CONFIG_GENERIC_HARDIRQS extern int __must_check request_threaded_irq(unsigned int irq, irq_handler_t handler,include/linux/irq.h +1
-0@@ -96,6 +96,7 @@ struct msi_desc;* methods, to allow shared chip implementations * @msi_desc: MSI descriptor * @affinity: IRQ affinity on SMP +* @status: driver-readable status flags (IRQS_*) * * The fields here need to overlay the ones in irq_desc until we * cleaned up the direct references and switched everything over to
include/linux/irq.h struct irq_data { · line 112
Quoted 4 lines
include/linux/irq.h +1
-0@@ -108,6 +109,7 @@ struct irq_data {void *handler_data;void *chip_data;struct msi_desc *msi_desc; +unsigned long status;
That breaks the current code which has irq_data embedded and shadowed in irq_desc for migratory reasons until all users are fixed up and the GENERIC_HARDIRQS_NO_DEPRECATED sections are cleaned up. I know it's ugly, but that was the only way not to rewrite the world in one go. :) Just move it below affinity.
Also we should name it different than status, drv_status perhaps, to avoid confusion with the irq_desc status.
include/linux/irq.h struct irq_data { · line 115
Quoted 3 lines
include/linux/irq.h +0
-0#ifdef CONFIG_SMP cpumask_var_t affinity;#endif
Thanks,
tglx
Re: [PATCH v2 1/4] genirq: Introduce driver-readable IRQ status word
Quoted 52 lines · Thomas Gleixner
Quoted 39 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
This associates a status word with every IRQ descriptor. Drivers can obtain its content via
get_irq_status(irq). First use case will be propagating the interrupt sharing state.Signed-off-by Jan Kiszka <[email protected]>--- include/linux/interrupt.h | 2 ++ include/linux/irq.h | 2 ++ kernel/irq/manage.c | 15 +++++++++++++++ 3 files changed, 19 insertions(+), 0 deletions(-)include/linux/interrupt.h +2
-0@@ -126,6 +126,8 @@ struct irqaction {extern irqreturn_t no_action (int cpl,void *dev_id); +extern unsigned long get_irq_status (unsigned long irq); +#ifdef CONFIG_GENERIC_HARDIRQS extern int __must_check request_threaded_irq(unsigned int irq, irq_handler_t handler,include/linux/irq.h +1
-0@@ -96,6 +96,7 @@ struct msi_desc;* methods, to allow shared chip implementations * @msi_desc: MSI descriptor * @affinity: IRQ affinity on SMP +* @status: driver-readable status flags (IRQS_*) * * The fields here need to overlay the ones in irq_desc until we * cleaned up the direct references and switched everything over to include/linux/irq.h struct irq_data { · line 112
Quoted 4 lines
include/linux/irq.h +1
-0@@ -108,6 +109,7 @@ struct irq_data {void *handler_data;void *chip_data;struct msi_desc *msi_desc; +unsigned long status;That breaks the current code which has
irq_dataembedded and shadowed inirq_descfor migratory reasons until all users are fixed up and theGENERIC_HARDIRQS_NO_DEPRECATEDsections are cleaned up. I know it's ugly, but that was the only way not to rewrite the world in one go. :) Just move it below affinity.
Quoted 2 lines
Also we should name it different than status,
drv_statusperhaps, to avoid confusion with theirq_descstatus.
OK, will address both in a succeeding round (just waiting for potential further comments).
Thanks,
Jan
Re: [PATCH v2 1/4] genirq: Introduce driver-readable IRQ status word
Quoted 6 lines · Jan Kiszka
Quoted 2 lines · Thomas Gleixner
Also we should name it different than status,
drv_statusperhaps, to avoid confusion with theirq_descstatus.OK, will address both in a succeeding round (just waiting for potential further comments).
No further comments from my side ATM.
Thanks,
tglx
PATCH v2 2/4 genirq: Inform handler about line sharing state
In reply to Jan KiszkaFrom: Jan Kiszka <[email protected]>
This enabled interrupt handlers to retrieve the current line sharing state via the new interrupt status word so that they can adapt to it.
The switch from shared to exclusive is generally uncritical and can thus be performed on demand. However, preparing a line for shared mode may require preparational steps of the currently registered handler. It can therefore request an ahead-of-time notification via IRQF_ADAPTIVE. The notification consists of an exceptional handler invocation with IRQS_MAKE_SHAREABLE set in the status word.
--- include/linux/interrupt.h | 10 ++++++++++ include/linux/irqdesc.h | 2 ++ kernel/irq/irqdesc.c | 2 ++ kernel/irq/manage.c | 44 +++++++++++++++++++++++++++++++++++++++++--- 4 files changed, 55 insertions(+), 3 deletions(-)
include/linux/interrupt.h +10 -0
@@ -55,6 +55,7 @@* Used by threaded interrupts which need to keep the * irq line disabled until the threaded handler has been run. * IRQF_NO_SUSPEND - Do not disable this IRQ during suspend +* IRQF_ADAPTIVE - Request notification about upcoming interrupt line sharing * */ #define IRQF_DISABLED 0x00000020
@@ -67,6 +68,7 @@#define IRQF_IRQPOLL 0x00001000#define IRQF_ONESHOT 0x00002000#define IRQF_NO_SUSPEND 0x00004000 +#define IRQF_ADAPTIVE 0x00008000#define IRQF_TIMER (__IRQF_TIMER | IRQF_NO_SUSPEND)
@@ -126,6 +128,14 @@ struct irqaction {extern irqreturn_t no_action (int cpl,void *dev_id); +/* +* Driver-readable IRQ line status flags: +* IRQS_SHARED - line is shared between multiple handlers +* IRQS_MAKE_SHAREABLE - in the process of making an exclusive line shareable +*/ +#define IRQS_SHARED 0x00000001 +#define IRQS_MAKE_SHAREABLE 0x00000002 +extern unsigned long get_irq_status (unsigned long irq);#ifdef CONFIG_GENERIC_HARDIRQS
include/linux/irqdesc.h +2 -0
@@ -29,6 +29,7 @@ struct timer_rand_state;* @wait_for_threads: wait queue for sync_irq to wait for threaded handlers * @dir: /proc/irq/ procfs entry * @name: flow handler name for /proc/interrupts output +* @register_lock: protects registration & release, for unshared->shared */ struct irq_desc {
@@ -80,6 +81,7 @@ struct irq_desc {struct proc_dir_entry *dir;#endif const char *name; +struct mutex register_lock; }____cacheline_internodealigned_in_smp ;#ifndef CONFIG_SPARSE_IRQ
kernel/irq/irqdesc.c +2 -0
@@ -143,6 +143,7 @@ static struct irq_desc *alloc_desc(int irq, int node)raw_spin_lock_init (&desc->lock);lockdep_set_class (&desc->lock, &irq_desc_lock_class); +mutex_init (&desc->register_lock);desc_set_defaults (irq, desc, node);
@@ -254,6 +255,7 @@ int __init early_irq_init(void)alloc_masks (desc + i,GFP_KERNEL , node);desc_smp_init (desc + i, node);lockdep_set_class (&desc[i].lock, &irq_desc_lock_class); +mutex_init (&desc[i].register_lock); }return arch_early_irq_init (); }
kernel/irq/manage.c +41 -3
94 lines not on this page · raw message
Signature · 1 line
1.7.1
Re: [PATCH v2 2/4] genirq: Inform handler about line sharing state
Quoted 60 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
This enabled interrupt handlers to retrieve the current line sharing state via the new interrupt status word so that they can adapt to it.
The switch from shared to exclusive is generally uncritical and can thus be performed on demand. However, preparing a line for shared mode may require preparational steps of the currently registered handler. It can therefore request an ahead-of-time notification via
IRQF_ADAPTIVE. The notification consists of an exceptional handler invocation withIRQS_MAKE_SHAREABLEset in the status word.Signed-off-by Jan Kiszka <[email protected]>--- include/linux/interrupt.h | 10 ++++++++++ include/linux/irqdesc.h | 2 ++ kernel/irq/irqdesc.c | 2 ++ kernel/irq/manage.c | 44 +++++++++++++++++++++++++++++++++++++++++--- 4 files changed, 55 insertions(+), 3 deletions(-)include/linux/interrupt.h +10
-0@@ -55,6 +55,7 @@* Used by threaded interrupts which need to keep the * irq line disabled until the threaded handler has been run. * IRQF_NO_SUSPEND - Do not disable this IRQ during suspend +* IRQF_ADAPTIVE - Request notification about upcoming interrupt line sharing * */ #define IRQF_DISABLED 0x00000020@@ -67,6 +68,7 @@#define IRQF_IRQPOLL 0x00001000#define IRQF_ONESHOT 0x00002000#define IRQF_NO_SUSPEND 0x00004000 +#define IRQF_ADAPTIVE 0x00008000#define IRQF_TIMER (__IRQF_TIMER | IRQF_NO_SUSPEND)@@ -126,6 +128,14 @@ struct irqaction {extern irqreturn_t no_action (int cpl,void *dev_id); +/* +* Driver-readable IRQ line status flags: +* IRQS_SHARED - line is shared between multiple handlers +* IRQS_MAKE_SHAREABLE - in the process of making an exclusive line shareable +*/ +#define IRQS_SHARED 0x00000001 +#define IRQS_MAKE_SHAREABLE 0x00000002 +extern unsigned long get_irq_status (unsigned long irq);#ifdef CONFIG_GENERIC_HARDIRQS include/linux/irqdesc.h +0
-0
include/linux/irqdesc.h struct timer_rand_state; · line 32
Quoted 4 lines
include/linux/irqdesc.h +1
-0@@ -29,6 +29,7 @@ struct timer_rand_state;* @wait_for_threads: wait queue for sync_irq to wait for threaded handlers * @dir: /proc/irq/ procfs entry * @name: flow handler name for /proc/interrupts output +* @register_lock: protects registration & release, for unshared->shared
I think we can make that a global mutex. request/free_irq are not hotpath operations which require a mutex per irq descriptor.
Thanks,
tglx
Re: [PATCH v2 2/4] genirq: Inform handler about line sharing state
Quoted 56 lines · Thomas Gleixner
Quoted 54 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
This enabled interrupt handlers to retrieve the current line sharing state via the new interrupt status word so that they can adapt to it.
The switch from shared to exclusive is generally uncritical and can thus be performed on demand. However, preparing a line for shared mode may require preparational steps of the currently registered handler. It can therefore request an ahead-of-time notification via
IRQF_ADAPTIVE. The notification consists of an exceptional handler invocation withIRQS_MAKE_SHAREABLEset in the status word.Signed-off-by Jan Kiszka <[email protected]>--- include/linux/interrupt.h | 10 ++++++++++ include/linux/irqdesc.h | 2 ++ kernel/irq/irqdesc.c | 2 ++ kernel/irq/manage.c | 44 +++++++++++++++++++++++++++++++++++++++++--- 4 files changed, 55 insertions(+), 3 deletions(-)include/linux/interrupt.h +10
-0@@ -55,6 +55,7 @@* Used by threaded interrupts which need to keep the * irq line disabled until the threaded handler has been run. * IRQF_NO_SUSPEND - Do not disable this IRQ during suspend +* IRQF_ADAPTIVE - Request notification about upcoming interrupt line sharing * */ #define IRQF_DISABLED 0x00000020@@ -67,6 +68,7 @@#define IRQF_IRQPOLL 0x00001000#define IRQF_ONESHOT 0x00002000#define IRQF_NO_SUSPEND 0x00004000 +#define IRQF_ADAPTIVE 0x00008000#define IRQF_TIMER (__IRQF_TIMER | IRQF_NO_SUSPEND)@@ -126,6 +128,14 @@ struct irqaction {extern irqreturn_t no_action (int cpl,void *dev_id); +/* +* Driver-readable IRQ line status flags: +* IRQS_SHARED - line is shared between multiple handlers +* IRQS_MAKE_SHAREABLE - in the process of making an exclusive line shareable +*/ +#define IRQS_SHARED 0x00000001 +#define IRQS_MAKE_SHAREABLE 0x00000002 +extern unsigned long get_irq_status (unsigned long irq);
Quoted 14 lines
include/linux/irqdesc.h struct timer_rand_state; · line 32
Quoted 10 lines
include/linux/interrupt.h +0
-0#ifdef CONFIG_GENERIC_HARDIRQS include/linux/irqdesc.h +1
-0@@ -29,6 +29,7 @@ struct timer_rand_state;* @wait_for_threads: wait queue for sync_irq to wait for threaded handlers * @dir: /proc/irq/ procfs entry * @name: flow handler name for /proc/interrupts output +* @register_lock: protects registration & release, for unshared->shared I think we can make that a global mutex. request/free_irq are not hotpath operations which require a mutex per irq descriptor.
Agreed, will change this.
Jan
PATCH v2 3/4 genirq: Add support for IRQF_COND_ONESHOT
In reply to Jan KiszkaFrom: Jan Kiszka <[email protected]>
Provide an adaptive version of IRQF_ONESHOT: If the line is exclusively used, IRQF_COND_ONESHOT provides the same semantics as IRQF_ONESHOT. If it is shared, the line will be unmasked directly after the hardirq handler, just as if IRQF_COND_ONESHOT was not provided.
--- include/linux/interrupt.h | 3 +++ kernel/irq/manage.c | 19 ++++++++++++++++--- 2 files changed, 19 insertions(+), 3 deletions(-)
include/linux/interrupt.h +3 -0
@@ -56,6 +56,8 @@* irq line disabled until the threaded handler has been run. * IRQF_NO_SUSPEND - Do not disable this IRQ during suspend * IRQF_ADAPTIVE - Request notification about upcoming interrupt line sharing +* IRQF_COND_ONESHOT - If line is not shared, keep interrupt disabled after +* hardirq handler finshed. * */ #define IRQF_DISABLED 0x00000020
@@ -69,6 +71,7 @@#define IRQF_ONESHOT 0x00002000#define IRQF_NO_SUSPEND 0x00004000#define IRQF_ADAPTIVE 0x00008000 +#define IRQF_COND_ONESHOT 0x00010000#define IRQF_TIMER (__IRQF_TIMER | IRQF_NO_SUSPEND)
kernel/irq/manage.c +16 -3
@@ -580,7 +580,7 @@ static int irq_thread(void *data)struct sched_param param = { .sched_priority =MAX_USER_RT_PRIO /2 , };struct irqaction *action = data;struct irq_desc *desc =irq_to_desc (action->irq); -int wake, oneshot = desc->status &IRQ_ONESHOT ; +int wake, oneshot;sched_setscheduler (current,SCHED_FIFO , ¶m); current->irqaction = action;
@@ -603,6 +603,7 @@ static int irq_thread(void *data) desc->status |=IRQ_PENDING ;raw_spin_unlock_irq (&desc->lock); }else { + oneshot = desc->status &IRQ_ONESHOT ;raw_spin_unlock_irq (&desc->lock); action->thread_fn (action->irq, action->dev_id);
@@ -756,6 +757,15 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new) shared =1 ; desc->irq_data.status |=IRQS_SHARED ; + desc->status &= ~IRQ_ONESHOT ; + +/* Unmask if the interrupt was masked due to oneshot mode. */ +if ((desc->status & + (IRQ_INPROGRESS |IRQ_DISABLED |IRQ_MASKED )) == +IRQ_MASKED ) { + desc->irq_data.chip->irq_unmask (&desc->irq_data); + desc->status &= ~IRQ_MASKED ; + } }if (!shared) {
@@ -780,7 +790,7 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new) desc->status &= ~(IRQ_AUTODETECT |IRQ_WAITING |IRQ_ONESHOT |IRQ_INPROGRESS |IRQ_SPURIOUS_DISABLED ); -if (new->flags &IRQF_ONESHOT ) +if (new->flags & (IRQF_ONESHOT |IRQF_COND_ONESHOT )) desc->status |=IRQ_ONESHOT ;if (!(desc->status &IRQ_NOAUTOEN )) {
@@ -931,8 +941,11 @@ static struct irqaction *__free_irq(unsigned int irq, void *dev_id) desc->irq_data.chip->irq_shutdown (&desc->irq_data); else desc->irq_data.chip->irq_disable(&desc->irq_data); - }else if (!desc->action->next) + }else if (!desc->action->next) { single_handler = true; +if (desc->action->flags &IRQF_COND_ONESHOT ) + desc->status |=IRQ_ONESHOT ; + }#ifdef CONFIG_SMP /* make sure affinity_hint is cleaned up */
Signature · 1 line
1.7.1
PATCH v2 4/4 KVM: Allow host IRQ sharing for passed-through PCI 2.3 devices
In reply to Jan KiszkaFrom: Jan Kiszka <[email protected]>
PCI 2.3 allows to generically disable IRQ sources at device level. This enables us to share IRQs of such devices on the host side when passing them to a guest.
However, IRQ disabling via the PCI config space is more costly than masking the line via disable_irq. Therefore we register the IRQ in adaptive mode and switch between line and device level disabling on demand.
This feature is optional, user space has to request it explicitly as it also has to inform us about its view of PCI_COMMAND_INTX_DISABLE. That way, we can avoid unmasking the interrupt and signaling it if the guest masked it via the PCI config space.
--- Documentation/kvm/api.txt | 27 ++++ arch/x86/kvm/x86.c | 1 + include/linux/kvm.h | 6 + include/linux/kvm_host.h | 10 ++- virt/kvm/assigned-dev.c | 336 ++++++++++++++++++++++++++++++++++++++++----- 5 files changed, 346 insertions(+), 34 deletions(-)
Documentation/kvm/api.txt +27 -0
@@ -1112,6 +1112,14 @@ following flags are specified: /* Depends on KVM_CAP_IOMMU */ #define KVM_DEV_ASSIGN_ENABLE_IOMMU (1 << 0) +/* The following two depend on KVM_CAP_PCI_2_3 */ +#define KVM_DEV_ASSIGN_PCI_2_3 (1 << 1) +#define KVM_DEV_ASSIGN_MASK_INTX (1 << 2) + +If KVM_DEV_ASSIGN_PCI_2_3 is set, the kernel will manage legacy INTx interrupts +via the PCI-2.3-compliant device-level mask, but only if IRQ sharing with other +assigned or host devices requires it. KVM_DEV_ASSIGN_MASK_INTX specifies the +guest's view on the INTx mask, see KVM_ASSIGN_SET_INTX_MASK for details. 4.48 KVM_DEASSIGN_PCI_DEVICE
@@ -1263,6 +1271,25 @@ struct kvm_assigned_msix_entry { __u16 padding[3]; }; +4.54 KVM_ASSIGN_SET_INTX_MASK + +Capability: KVM_CAP_PCI_2_3 +Architectures: x86 +Type: vm ioctl +Parameters: struct kvm_assigned_pci_dev (in) +Returns: 0 on success, -1 on error + +Informs the kernel about the guest's view on the INTx mask. As long as the +guest masks the legacy INTx, the kernel will refrain from unmasking it at +hardware level and will not assert the guest's IRQ line. User space is still +responsible for applying this state to the assigned device's real config space. +To avoid that the kernel overwrites the state user space wants to set, +KVM_ASSIGN_SET_INTX_MASK has to be called prior to updating the config space. + +See KVM_ASSIGN_DEV_IRQ for the data structure. The target device is specified +by assigned_dev_id. In the flags field, only KVM_DEV_ASSIGN_MASK_INTX is +evaluated. + 5. The kvm_run structure Application code obtains a pointer to the kvm_run structure by
arch/x86/kvm/x86.c +1 -0
@@ -1965,6 +1965,7 @@ int kvm_dev_ioctl_check_extension(long ext)case KVM_CAP_X86_ROBUST_SINGLESTEP :case KVM_CAP_XSAVE :case KVM_CAP_ASYNC_PF : +case KVM_CAP_PCI_2_3 : r =1 ;break ;case KVM_CAP_COALESCED_MMIO :
include/linux/kvm.h +6 -0
@@ -541,6 +541,7 @@ struct kvm_ppc_pvinfo {#define KVM_CAP_PPC_GET_PVINFO 57#define KVM_CAP_PPC_IRQ_LEVEL 58#define KVM_CAP_ASYNC_PF 59 +#define KVM_CAP_PCI_2_3 60#ifdef KVM_CAP_IRQ_ROUTING
@@ -677,6 +678,9 @@ struct kvm_clock_data {#define KVM_SET_PIT2 _IOW(KVMIO, 0xa0, struct kvm_pit_state2)/* Available with KVM_CAP_PPC_GET_PVINFO */ #define KVM_PPC_GET_PVINFO _IOW(KVMIO, 0xa1, struct kvm_ppc_pvinfo) +/* Available with KVM_CAP_PCI_2_3 */ +#define KVM_ASSIGN_SET_INTX_MASK _IOW(KVMIO, 0xa2, \ + struct kvm_assigned_pci_dev)/* * ioctls for vcpu fds
@@ -742,6 +746,8 @@ struct kvm_clock_data {#define KVM_SET_XCRS _IOW(KVMIO, 0xa7, struct kvm_xcrs)#define KVM_DEV_ASSIGN_ENABLE_IOMMU (1 << 0) +#define KVM_DEV_ASSIGN_PCI_2_3 (1 << 1) +#define KVM_DEV_ASSIGN_MASK_INTX (1 << 2)struct kvm_assigned_pci_dev {__u32 assigned_dev_id;
include/linux/kvm_host.h +9 -1
@@ -477,6 +477,12 @@ struct kvm_irq_ack_notifier {void (*irq_acked)(struct kvm_irq_ack_notifier *kian); }; +enum kvm_intx_state { +KVM_INTX_ENABLED , +KVM_INTX_LINE_DISABLED , +KVM_INTX_DEVICE_DISABLED , +}; +struct kvm_assigned_dev_kernel {struct kvm_irq_ack_notifier ack_notifier;struct list_head list;
@@ -486,7 +492,7 @@ struct kvm_assigned_dev_kernel {int host_devfn;unsigned int entries_nr;int host_irq; -bool host_irq_disabled; +unsigned long last_irq_status;struct msix_entry *host_msix_entries;int guest_irq;struct msix_entry *guest_msix_entries;
@@ -496,6 +502,8 @@ struct kvm_assigned_dev_kernel {struct pci_dev *dev;struct kvm *kvm;spinlock_t intx_lock; +spinlock_t intx_mask_lock; +enum kvm_intx_state intx_state;char irq_name[32 ]; };
virt/kvm/assigned-dev.c +303 -33
441 lines not on this page · raw message
Signature · 1 line
1.7.1
Re: [PATCH v2 4/4] KVM: Allow host IRQ sharing for passed-through PCI 2.3 devices
Quoted 6 lines · Jan Kiszka
From: Jan Kiszka<[email protected]>
PCI 2.3 allows to generically disable IRQ sources at device level. This enables us to share IRQs of such devices on the host side when passing them to a guest.
Quoted 9 lines
However, IRQ disabling via the PCI config space is more costly than masking the line via
disable_irq. Therefore we register the IRQ in adaptive mode and switch between line and device level disabling on demand.This feature is optional, user space has to request it explicitly as it also has to inform us about its view of
PCI_COMMAND_INTX_DISABLE. That way, we can avoid unmasking the interrupt and signaling it if the guest masked it via the PCI config space.
Looks fine.
Review
Quoted 2 lines
(quoted diff) +2
-0+ ret =IRQ_NONE; +
Danger, whitespace error detected. Initiating self-destruct sequence.
Signature · 1 line
error compiling committee.c: too many arguments to function
Quoted 3 lines · Jan Kiszka
The result may look simpler on first glance than v1, but it comes with more subtle race scenarios IMO. I thought them through, hopefully catching all, but I would appreciate any skeptical review.
Thought about the races till my head hurt, and yes, they all seem to be handled correctly. FWIW
Quoted 7 lines · Michael S. Tsirkin
Quoted 3 lines · Jan Kiszka
The result may look simpler on first glance than v1, but it comes with more subtle race scenarios IMO. I thought them through, hopefully catching all, but I would appreciate any skeptical review.
Thought about the races till my head hurt, and yes, they all seem to be handled correctly. FWIW
Ouch, I'm endlessly sorry for causing this pain.
Quoted 3 lines
Reviewed-by Michael S. Tsirkin <[email protected]>
Thanks!
Jan