From: Yi Wang <[email protected]>
We found that it may cost more than 20 milliseconds very accidentally
to enable cap of KVM_CAP_SPLIT_IRQCHIP on a host which has many vms
already.
The reason is that when vmm(qemu/CloudHypervisor) invokes
KVM_CAP_SPLIT_IRQCHIP kvm will call synchronize_srcu_expedited() and
might_sleep and kworker of srcu may cost some delay during this period.
One way makes sence is setup empty irq routing when creating vm and
so that x86/s390 don't need to setup empty/dummy irq routing.
Note: I have no s390 machine so this patch has not been tested
thoroughly on s390 platform. Thanks to Christian for a quick test on
s390 and it still seems to work[1].
Changelog:
----------
v5:
- rename new function to kvm_init_irq_routing as Sean suggested.
- move invok of function of setting up empty before kvm_arch_init_vm.
- Add necessary comment.
v4:
- replace loop with memset when setup empty irq routing table.
v3:
- squash setup empty routing function and use of that into one commit
- drop the comment in s390 part
v2:
- setup empty irq routing in kvm_create_vm
- don't setup irq routing in x86 KVM_CAP_SPLIT_IRQCHIP
- don't setup irq routing in s390 KVM_CREATE_IRQCHIP
v1:
https://lore.kernel.org/kvm/[email protected]/
1. https://lore.kernel.org/lkml/[email protected]/
Yi Wang (3):
KVM: setup empty irq routing when create vm
KVM: x86: don't setup empty irq routing when KVM_CAP_SPLIT_IRQCHIP
KVM: s390: don't setup dummy routing when KVM_CREATE_IRQCHIP
arch/s390/kvm/kvm-s390.c | 9 +--------
arch/x86/kvm/irq.h | 1 -
arch/x86/kvm/irq_comm.c | 5 -----
arch/x86/kvm/x86.c | 3 ---
include/linux/kvm_host.h | 2 ++
virt/kvm/irqchip.c | 23 +++++++++++++++++++++++
virt/kvm/kvm_main.c | 9 ++++++++-
7 files changed, 34 insertions(+), 18 deletions(-)
--
2.39.3
From: Yi Wang <[email protected]>
Add a new function to setup empty irq routing in kvm path, which
can be invoded in non-architecture-specific functions. The difference
compared to the kvm_setup_empty_irq_routing() is this function just
alloc the empty irq routing and does not need synchronize srcu, as
we will call it in kvm_create_vm().
Using the new adding function, we can setup empty irq routing when
kvm_create_vm(), so that x86 and s390 no longer need to set
empty/dummy irq routing when creating an IRQCHIP 'cause it avoid
an synchronize_srcu.
Signed-off-by: Yi Wang <[email protected]>
---
include/linux/kvm_host.h | 2 ++
virt/kvm/irqchip.c | 23 +++++++++++++++++++++++
virt/kvm/kvm_main.c | 9 ++++++++-
3 files changed, 33 insertions(+), 1 deletion(-)
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 48f31dcd318a..a5f12b667ca5 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -2100,6 +2100,7 @@ int kvm_set_irq_routing(struct kvm *kvm,
const struct kvm_irq_routing_entry *entries,
unsigned nr,
unsigned flags);
+int kvm_init_irq_routing(struct kvm *kvm);
int kvm_set_routing_entry(struct kvm *kvm,
struct kvm_kernel_irq_routing_entry *e,
const struct kvm_irq_routing_entry *ue);
@@ -2108,6 +2109,7 @@ void kvm_free_irq_routing(struct kvm *kvm);
#else
static inline void kvm_free_irq_routing(struct kvm *kvm) {}
+int kvm_init_irq_routing(struct kvm *kvm) {}
#endif
diff --git a/virt/kvm/irqchip.c b/virt/kvm/irqchip.c
index 1e567d1f6d3d..ec1fda7fffea 100644
--- a/virt/kvm/irqchip.c
+++ b/virt/kvm/irqchip.c
@@ -237,3 +237,26 @@ int kvm_set_irq_routing(struct kvm *kvm,
return r;
}
+
+/*
+ * Alloc empty irq routing.
+ * Called only during vm creation, because we don't synchronize_srcu here.
+ */
+int kvm_init_irq_routing(struct kvm *kvm)
+{
+ struct kvm_irq_routing_table *new;
+ int chip_size;
+
+ new = kzalloc(struct_size(new, map, 1), GFP_KERNEL_ACCOUNT);
+ if (!new)
+ return -ENOMEM;
+
+ new->nr_rt_entries = 1;
+
+ chip_size = sizeof(int) * KVM_NR_IRQCHIPS * KVM_IRQCHIP_NUM_PINS;
+ memset(new->chip, -1, chip_size);
+
+ RCU_INIT_POINTER(kvm->irq_routing, new);
+
+ return 0;
+}
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index ff0a20565f90..4100ebdd14fe 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1233,6 +1233,11 @@ static struct kvm *kvm_create_vm(unsigned long type, const char *fdname)
goto out_err_no_irq_srcu;
refcount_set(&kvm->users_count, 1);
+
+ r = kvm_init_irq_routing(kvm);
+ if (r)
+ goto out_err_no_irq_routing;
+
for (i = 0; i < kvm_arch_nr_memslot_as_ids(kvm); i++) {
for (j = 0; j < 2; j++) {
slots = &kvm->__memslots[i][j];
@@ -1308,9 +1313,11 @@ static struct kvm *kvm_create_vm(unsigned long type, const char *fdname)
out_err_no_disable:
kvm_arch_destroy_vm(kvm);
out_err_no_arch_destroy_vm:
- WARN_ON_ONCE(!refcount_dec_and_test(&kvm->users_count));
for (i = 0; i < KVM_NR_BUSES; i++)
kfree(kvm_get_bus(kvm, i));
+ kvm_free_irq_routing(kvm);
+out_err_no_irq_routing:
+ WARN_ON_ONCE(!refcount_dec_and_test(&kvm->users_count));
cleanup_srcu_struct(&kvm->irq_srcu);
out_err_no_irq_srcu:
cleanup_srcu_struct(&kvm->srcu);
--
2.39.3
From: Yi Wang <[email protected]>
As we have setup empty irq routing in kvm_create_vm(), there's
no need to setup dummy routing when KVM_CREATE_IRQCHIP.
Signed-off-by: Yi Wang <[email protected]>
---
arch/s390/kvm/kvm-s390.c | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 5147b943a864..ba7fd39bcbf4 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -2998,14 +2998,7 @@ int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
break;
}
case KVM_CREATE_IRQCHIP: {
- struct kvm_irq_routing_entry routing;
-
- r = -EINVAL;
- if (kvm->arch.use_irqchip) {
- /* Set up dummy routing. */
- memset(&routing, 0, sizeof(routing));
- r = kvm_set_irq_routing(kvm, &routing, 0, 0);
- }
+ r = 0;
break;
}
case KVM_SET_DEVICE_ATTR: {
--
2.39.3
From: Yi Wang <[email protected]>
We found that it may cost more than 20 milliseconds very accidentally
to enable cap of KVM_CAP_SPLIT_IRQCHIP on a host which has many vms
already.
The reason is that when vmm(qemu/CloudHypervisor) invokes
KVM_CAP_SPLIT_IRQCHIP kvm will call synchronize_srcu_expedited() and
might_sleep and kworker of srcu may cost some delay during this period.
As we have set up empty irq routing when creating vm, so this is no
need now.
Signed-off-by: Yi Wang <[email protected]>
---
arch/x86/kvm/irq.h | 1 -
arch/x86/kvm/irq_comm.c | 5 -----
arch/x86/kvm/x86.c | 3 ---
3 files changed, 9 deletions(-)
diff --git a/arch/x86/kvm/irq.h b/arch/x86/kvm/irq.h
index c2d7cfe82d00..76d46b2f41dd 100644
--- a/arch/x86/kvm/irq.h
+++ b/arch/x86/kvm/irq.h
@@ -106,7 +106,6 @@ void __kvm_migrate_timers(struct kvm_vcpu *vcpu);
int apic_has_pending_timer(struct kvm_vcpu *vcpu);
int kvm_setup_default_irq_routing(struct kvm *kvm);
-int kvm_setup_empty_irq_routing(struct kvm *kvm);
int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
struct kvm_lapic_irq *irq,
struct dest_map *dest_map);
diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c
index 68f3f6c26046..6ee7ca39466e 100644
--- a/arch/x86/kvm/irq_comm.c
+++ b/arch/x86/kvm/irq_comm.c
@@ -397,11 +397,6 @@ int kvm_setup_default_irq_routing(struct kvm *kvm)
static const struct kvm_irq_routing_entry empty_routing[] = {};
-int kvm_setup_empty_irq_routing(struct kvm *kvm)
-{
- return kvm_set_irq_routing(kvm, empty_routing, 0, 0);
-}
-
void kvm_arch_post_irq_routing_update(struct kvm *kvm)
{
if (!irqchip_split(kvm))
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 91478b769af0..01270182757b 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -6527,9 +6527,6 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
goto split_irqchip_unlock;
if (kvm->created_vcpus)
goto split_irqchip_unlock;
- r = kvm_setup_empty_irq_routing(kvm);
- if (r)
- goto split_irqchip_unlock;
/* Pairs with irqchip_in_kernel. */
smp_wmb();
kvm->arch.irqchip_mode = KVM_IRQCHIP_SPLIT;
--
2.39.3
Am 06.05.24 um 12:17 schrieb Yi Wang:
> From: Yi Wang <[email protected]>
>
> Add a new function to setup empty irq routing in kvm path, which
> can be invoded in non-architecture-specific functions. The difference
> compared to the kvm_setup_empty_irq_routing() is this function just
> alloc the empty irq routing and does not need synchronize srcu, as
> we will call it in kvm_create_vm().
>
> Using the new adding function, we can setup empty irq routing when
> kvm_create_vm(), so that x86 and s390 no longer need to set
> empty/dummy irq routing when creating an IRQCHIP 'cause it avoid
> an synchronize_srcu.
>
> Signed-off-by: Yi Wang <[email protected]>
Acked-by: Christian Borntraeger <[email protected]>
> ---
> include/linux/kvm_host.h | 2 ++
> virt/kvm/irqchip.c | 23 +++++++++++++++++++++++
> virt/kvm/kvm_main.c | 9 ++++++++-
> 3 files changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 48f31dcd318a..a5f12b667ca5 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -2100,6 +2100,7 @@ int kvm_set_irq_routing(struct kvm *kvm,
> const struct kvm_irq_routing_entry *entries,
> unsigned nr,
> unsigned flags);
> +int kvm_init_irq_routing(struct kvm *kvm);
> int kvm_set_routing_entry(struct kvm *kvm,
> struct kvm_kernel_irq_routing_entry *e,
> const struct kvm_irq_routing_entry *ue);
> @@ -2108,6 +2109,7 @@ void kvm_free_irq_routing(struct kvm *kvm);
> #else
>
> static inline void kvm_free_irq_routing(struct kvm *kvm) {}
> +int kvm_init_irq_routing(struct kvm *kvm) {}
>
> #endif
>
> diff --git a/virt/kvm/irqchip.c b/virt/kvm/irqchip.c
> index 1e567d1f6d3d..ec1fda7fffea 100644
> --- a/virt/kvm/irqchip.c
> +++ b/virt/kvm/irqchip.c
> @@ -237,3 +237,26 @@ int kvm_set_irq_routing(struct kvm *kvm,
>
> return r;
> }
> +
> +/*
> + * Alloc empty irq routing.
> + * Called only during vm creation, because we don't synchronize_srcu here.
> + */
> +int kvm_init_irq_routing(struct kvm *kvm)
> +{
> + struct kvm_irq_routing_table *new;
> + int chip_size;
> +
> + new = kzalloc(struct_size(new, map, 1), GFP_KERNEL_ACCOUNT);
> + if (!new)
> + return -ENOMEM;
> +
> + new->nr_rt_entries = 1;
> +
> + chip_size = sizeof(int) * KVM_NR_IRQCHIPS * KVM_IRQCHIP_NUM_PINS;
> + memset(new->chip, -1, chip_size);
> +
> + RCU_INIT_POINTER(kvm->irq_routing, new);
> +
> + return 0;
> +}
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index ff0a20565f90..4100ebdd14fe 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -1233,6 +1233,11 @@ static struct kvm *kvm_create_vm(unsigned long type, const char *fdname)
> goto out_err_no_irq_srcu;
>
> refcount_set(&kvm->users_count, 1);
> +
> + r = kvm_init_irq_routing(kvm);
> + if (r)
> + goto out_err_no_irq_routing;
> +
> for (i = 0; i < kvm_arch_nr_memslot_as_ids(kvm); i++) {
> for (j = 0; j < 2; j++) {
> slots = &kvm->__memslots[i][j];
> @@ -1308,9 +1313,11 @@ static struct kvm *kvm_create_vm(unsigned long type, const char *fdname)
> out_err_no_disable:
> kvm_arch_destroy_vm(kvm);
> out_err_no_arch_destroy_vm:
> - WARN_ON_ONCE(!refcount_dec_and_test(&kvm->users_count));
> for (i = 0; i < KVM_NR_BUSES; i++)
> kfree(kvm_get_bus(kvm, i));
> + kvm_free_irq_routing(kvm);
> +out_err_no_irq_routing:
> + WARN_ON_ONCE(!refcount_dec_and_test(&kvm->users_count));
> cleanup_srcu_struct(&kvm->irq_srcu);
> out_err_no_irq_srcu:
> cleanup_srcu_struct(&kvm->srcu);
Am 06.05.24 um 12:17 schrieb Yi Wang:
> From: Yi Wang <[email protected]>
>
> As we have setup empty irq routing in kvm_create_vm(), there's
> no need to setup dummy routing when KVM_CREATE_IRQCHIP.
>
> Signed-off-by: Yi Wang <[email protected]>
Acked-by: Christian Borntraeger <[email protected]>
> ---
> arch/s390/kvm/kvm-s390.c | 9 +--------
> 1 file changed, 1 insertion(+), 8 deletions(-)
>
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 5147b943a864..ba7fd39bcbf4 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -2998,14 +2998,7 @@ int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
> break;
> }
> case KVM_CREATE_IRQCHIP: {
> - struct kvm_irq_routing_entry routing;
> -
> - r = -EINVAL;
> - if (kvm->arch.use_irqchip) {
> - /* Set up dummy routing. */
> - memset(&routing, 0, sizeof(routing));
> - r = kvm_set_irq_routing(kvm, &routing, 0, 0);
> - }
> + r = 0;
> break;
> }
> case KVM_SET_DEVICE_ATTR: {