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: {
On Mon, May 06, 2024, Yi Wang wrote:
> 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;
This is wrong, KVM_CREATE_IRQCHIP should fail with -EINVAL if kvm->arch.use_irqchip
is false.
There's also a functional change here, though I highly doubt it negatively affects
userspace. Nothing in s390 prevents invoking KVM_CREATE_IRQCHIP after
KVM_SET_GSI_ROUTING, so userspace could very theoretically use KVM_CREATE_IRQCHIP
to reset to empty IRQ routing.
Christian, if it works for you, I'll massage it to this when applying.
--
From: Yi Wang <[email protected]>
Date: Mon, 6 May 2024 18:17:51 +0800
Subject: [PATCH] KVM: s390: Don't re-setup dummy routing when
KVM_CREATE_IRQCHIP
Now that KVM sets up empty irq routing in kvm_create_vm(), there's
no need to setup dummy routing when KVM_CREATE_IRQCHIP.
Note, userspace could very theoretically use KVM_CREATE_IRQCHIP after
KVM_SET_GSI_ROUTING to reset to empty IRQ routing, but it's extremely
unlikely any VMM does that, e.g. the main reason s390 does anything for
KVM_CREATE_IRQCHIP is to that s390 doesn't need to be special cased by the
VMM.
Signed-off-by: Yi Wang <[email protected]>
Acked-by: Christian Borntraeger <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
[sean: keep use_irqchip check, call out KVM_SET_GSI_ROUTING impact]
Signed-off-by: Sean Christopherson <[email protected]>
---
arch/s390/kvm/kvm-s390.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 82e9631cd9ef..4641083ee100 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -2996,14 +2996,9 @@ 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);
- }
+ if (kvm->arch.use_irqchip)
+ r = 0;
break;
}
case KVM_SET_DEVICE_ATTR: {
base-commit: 9a859becf1b7f6879466e8c0ebee492b236f2080
--
On Mon, May 06, 2024, Yi Wang wrote:
> 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[] = {};
empty_routing is unused after this patch and can+should be removed, e.g.
arch/x86/kvm/irq_comm.c:398:43: error: ‘empty_routing’ defined but not used [-Werror=unused-const-variable=]
398 | static const struct kvm_irq_routing_entry empty_routing[] = {};
I'll fix this up when applying, too.
On Tue, Jun 11, 2024 at 8:25 AM Sean Christopherson <[email protected]> wrote:
>
> On Mon, May 06, 2024, Yi Wang wrote:
> > 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[] = {};
>
> empty_routing is unused after this patch and can+should be removed, e.g.
>
> arch/x86/kvm/irq_comm.c:398:43: error: ‘empty_routing’ defined but not used [-Werror=unused-const-variable=]
> 398 | static const struct kvm_irq_routing_entry empty_routing[] = {};
>
> I'll fix this up when applying, too.
Many thanks for your work, Sean.
---
Best wishes
Yi Wang
Am 10.06.24 um 18:19 schrieb Sean Christopherson:
> On Mon, May 06, 2024, Yi Wang wrote:
>> 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;
>
> This is wrong, KVM_CREATE_IRQCHIP should fail with -EINVAL if kvm->arch.use_irqchip
> is false.
>
> There's also a functional change here, though I highly doubt it negatively affects
> userspace. Nothing in s390 prevents invoking KVM_CREATE_IRQCHIP after
> KVM_SET_GSI_ROUTING, so userspace could very theoretically use KVM_CREATE_IRQCHIP
> to reset to empty IRQ routing.
>
> Christian, if it works for you, I'll massage it to this when applying.
Oops. Yes please do so.
On Mon, 06 May 2024 18:17:48 +0800, Yi Wang wrote:
> 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.
>
> [...]
Applied to kvm-x86 generic, with a lot of fixup. A sanity check on the end
result would be much appreciated. Thanks!
[1/3] KVM: Setup empty IRQ routing when creating a VM
https://github.com/kvm-x86/linux/commit/fbe4a7e881d4
[2/3] KVM: x86: Don't re-setup empty IRQ routing when KVM_CAP_SPLIT_IRQCHIP
https://github.com/kvm-x86/linux/commit/e3c89f5dd11d
[3/3] KVM: s390: Don't re-setup dummy routing when KVM_CREATE_IRQCHIP
https://github.com/kvm-x86/linux/commit/c4201bd24f4a
--
https://github.com/kvm-x86/linux/tree/next
On Wed, Jun 12, 2024 at 9:19 AM Sean Christopherson <[email protected]> wrote:
>
> On Mon, 06 May 2024 18:17:48 +0800, Yi Wang wrote:
> > 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.
> >
> > [...]
>
> Applied to kvm-x86 generic, with a lot of fixup. A sanity check on the end
> result would be much appreciated. Thanks!
The code LGTM. I did some tests in my Intel machine and found no problems.
Thanks for your work :)
>
> [1/3] KVM: Setup empty IRQ routing when creating a VM
> https://github.com/kvm-x86/linux/commit/fbe4a7e881d4
> [2/3] KVM: x86: Don't re-setup empty IRQ routing when KVM_CAP_SPLIT_IRQCHIP
> https://github.com/kvm-x86/linux/commit/e3c89f5dd11d
> [3/3] KVM: s390: Don't re-setup dummy routing when KVM_CREATE_IRQCHIP
> https://github.com/kvm-x86/linux/commit/c4201bd24f4a
>
> --
> https://github.com/kvm-x86/linux/tree/next
---
Best wishes
Yi Wang