[PATCH 05/15] KVM: x86: Fold kvm_setup_default_irq_routing() into kvm_ioapic_init()

Sean Christopherson posted 15 patches 7 months ago
There is a newer version of this series
[PATCH 05/15] KVM: x86: Fold kvm_setup_default_irq_routing() into kvm_ioapic_init()
Posted by Sean Christopherson 7 months ago
Move the default IRQ routing table used for in-kernel I/O APIC routing to
ioapic.c where it belongs, and fold the call to kvm_set_irq_routing() into
kvm_ioapic_init() (the call via kvm_setup_default_irq_routing() is done
immediately after kvm_ioapic_init()).

In addition to making it more obvious that the so called "default" routing
only applies to an in-kernel I/O APIC, getting it out of irq_comm.c will
allow removing irq_comm.c entirely, and will also allow for guarding KVM's
in-kernel I/O APIC emulation with a Kconfig with minimal #ifdefs.

No functional change intended.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/ioapic.c   | 32 ++++++++++++++++++++++++++++++++
 arch/x86/kvm/irq.h      |  1 -
 arch/x86/kvm/irq_comm.c | 32 --------------------------------
 arch/x86/kvm/x86.c      |  6 ------
 4 files changed, 32 insertions(+), 39 deletions(-)

diff --git a/arch/x86/kvm/ioapic.c b/arch/x86/kvm/ioapic.c
index 8c8a8062eb19..dc45ea9f5b9c 100644
--- a/arch/x86/kvm/ioapic.c
+++ b/arch/x86/kvm/ioapic.c
@@ -710,6 +710,32 @@ static const struct kvm_io_device_ops ioapic_mmio_ops = {
 	.write    = ioapic_mmio_write,
 };
 
+#define IOAPIC_ROUTING_ENTRY(irq) \
+	{ .gsi = irq, .type = KVM_IRQ_ROUTING_IRQCHIP,	\
+	  .u.irqchip = { .irqchip = KVM_IRQCHIP_IOAPIC, .pin = (irq) } }
+#define ROUTING_ENTRY1(irq) IOAPIC_ROUTING_ENTRY(irq)
+
+#define PIC_ROUTING_ENTRY(irq) \
+	{ .gsi = irq, .type = KVM_IRQ_ROUTING_IRQCHIP,	\
+	  .u.irqchip = { .irqchip = SELECT_PIC(irq), .pin = (irq) % 8 } }
+#define ROUTING_ENTRY2(irq) \
+	IOAPIC_ROUTING_ENTRY(irq), PIC_ROUTING_ENTRY(irq)
+
+static const struct kvm_irq_routing_entry default_routing[] = {
+	ROUTING_ENTRY2(0), ROUTING_ENTRY2(1),
+	ROUTING_ENTRY2(2), ROUTING_ENTRY2(3),
+	ROUTING_ENTRY2(4), ROUTING_ENTRY2(5),
+	ROUTING_ENTRY2(6), ROUTING_ENTRY2(7),
+	ROUTING_ENTRY2(8), ROUTING_ENTRY2(9),
+	ROUTING_ENTRY2(10), ROUTING_ENTRY2(11),
+	ROUTING_ENTRY2(12), ROUTING_ENTRY2(13),
+	ROUTING_ENTRY2(14), ROUTING_ENTRY2(15),
+	ROUTING_ENTRY1(16), ROUTING_ENTRY1(17),
+	ROUTING_ENTRY1(18), ROUTING_ENTRY1(19),
+	ROUTING_ENTRY1(20), ROUTING_ENTRY1(21),
+	ROUTING_ENTRY1(22), ROUTING_ENTRY1(23),
+};
+
 int kvm_ioapic_init(struct kvm *kvm)
 {
 	struct kvm_ioapic *ioapic;
@@ -731,8 +757,14 @@ int kvm_ioapic_init(struct kvm *kvm)
 	if (ret < 0) {
 		kvm->arch.vioapic = NULL;
 		kfree(ioapic);
+		return ret;
 	}
 
+	ret = kvm_set_irq_routing(kvm, default_routing,
+				  ARRAY_SIZE(default_routing), 0);
+	if (ret)
+		kvm_ioapic_destroy(kvm);
+
 	return ret;
 }
 
diff --git a/arch/x86/kvm/irq.h b/arch/x86/kvm/irq.h
index 33dd5666b656..f6134289523e 100644
--- a/arch/x86/kvm/irq.h
+++ b/arch/x86/kvm/irq.h
@@ -107,7 +107,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_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 b85e4be2ddff..998c4a34d87c 100644
--- a/arch/x86/kvm/irq_comm.c
+++ b/arch/x86/kvm/irq_comm.c
@@ -334,38 +334,6 @@ bool kvm_intr_is_single_vcpu(struct kvm *kvm, struct kvm_lapic_irq *irq,
 }
 EXPORT_SYMBOL_GPL(kvm_intr_is_single_vcpu);
 
-#define IOAPIC_ROUTING_ENTRY(irq) \
-	{ .gsi = irq, .type = KVM_IRQ_ROUTING_IRQCHIP,	\
-	  .u.irqchip = { .irqchip = KVM_IRQCHIP_IOAPIC, .pin = (irq) } }
-#define ROUTING_ENTRY1(irq) IOAPIC_ROUTING_ENTRY(irq)
-
-#define PIC_ROUTING_ENTRY(irq) \
-	{ .gsi = irq, .type = KVM_IRQ_ROUTING_IRQCHIP,	\
-	  .u.irqchip = { .irqchip = SELECT_PIC(irq), .pin = (irq) % 8 } }
-#define ROUTING_ENTRY2(irq) \
-	IOAPIC_ROUTING_ENTRY(irq), PIC_ROUTING_ENTRY(irq)
-
-static const struct kvm_irq_routing_entry default_routing[] = {
-	ROUTING_ENTRY2(0), ROUTING_ENTRY2(1),
-	ROUTING_ENTRY2(2), ROUTING_ENTRY2(3),
-	ROUTING_ENTRY2(4), ROUTING_ENTRY2(5),
-	ROUTING_ENTRY2(6), ROUTING_ENTRY2(7),
-	ROUTING_ENTRY2(8), ROUTING_ENTRY2(9),
-	ROUTING_ENTRY2(10), ROUTING_ENTRY2(11),
-	ROUTING_ENTRY2(12), ROUTING_ENTRY2(13),
-	ROUTING_ENTRY2(14), ROUTING_ENTRY2(15),
-	ROUTING_ENTRY1(16), ROUTING_ENTRY1(17),
-	ROUTING_ENTRY1(18), ROUTING_ENTRY1(19),
-	ROUTING_ENTRY1(20), ROUTING_ENTRY1(21),
-	ROUTING_ENTRY1(22), ROUTING_ENTRY1(23),
-};
-
-int kvm_setup_default_irq_routing(struct kvm *kvm)
-{
-	return kvm_set_irq_routing(kvm, default_routing,
-				   ARRAY_SIZE(default_routing), 0);
-}
-
 void kvm_scan_ioapic_irq(struct kvm_vcpu *vcpu, u32 dest_id, u16 dest_mode,
 			 u8 vector, unsigned long *ioapic_handled_vectors)
 {
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index f9f798f286ce..4a9c252c9dab 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -7118,12 +7118,6 @@ int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
 			goto create_irqchip_unlock;
 		}
 
-		r = kvm_setup_default_irq_routing(kvm);
-		if (r) {
-			kvm_ioapic_destroy(kvm);
-			kvm_pic_destroy(kvm);
-			goto create_irqchip_unlock;
-		}
 		/* Write kvm->irq_routing before enabling irqchip_in_kernel. */
 		smp_wmb();
 		kvm->arch.irqchip_mode = KVM_IRQCHIP_KERNEL;
-- 
2.49.0.1101.gccaa498523-goog
Re: [PATCH 05/15] KVM: x86: Fold kvm_setup_default_irq_routing() into kvm_ioapic_init()
Posted by Paolo Bonzini 6 months, 2 weeks ago
On 5/20/25 01:27, Sean Christopherson wrote:
> Move the default IRQ routing table used for in-kernel I/O APIC routing to
> ioapic.c where it belongs, and fold the call to kvm_set_irq_routing() into
> kvm_ioapic_init() (the call via kvm_setup_default_irq_routing() is done
> immediately after kvm_ioapic_init()).
> 
> In addition to making it more obvious that the so called "default" routing
> only applies to an in-kernel I/O APIC, getting it out of irq_comm.c will
> allow removing irq_comm.c entirely, and will also allow for guarding KVM's
> in-kernel I/O APIC emulation with a Kconfig with minimal #ifdefs.
> 
> No functional change intended.

Well, it also applies to the PIC.  Even though the IOAPIC and PIC (and 
PIT) do come in a bundle, it's a bit weird to have the PIC routing 
entries initialized by kvm_ioapic_init().  Please keep 
kvm_setup_default_irq_routine() a separate function.

Paolo

> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>   arch/x86/kvm/ioapic.c   | 32 ++++++++++++++++++++++++++++++++
>   arch/x86/kvm/irq.h      |  1 -
>   arch/x86/kvm/irq_comm.c | 32 --------------------------------
>   arch/x86/kvm/x86.c      |  6 ------
>   4 files changed, 32 insertions(+), 39 deletions(-)
> 
> diff --git a/arch/x86/kvm/ioapic.c b/arch/x86/kvm/ioapic.c
> index 8c8a8062eb19..dc45ea9f5b9c 100644
> --- a/arch/x86/kvm/ioapic.c
> +++ b/arch/x86/kvm/ioapic.c
> @@ -710,6 +710,32 @@ static const struct kvm_io_device_ops ioapic_mmio_ops = {
>   	.write    = ioapic_mmio_write,
>   };
>   
> +#define IOAPIC_ROUTING_ENTRY(irq) \
> +	{ .gsi = irq, .type = KVM_IRQ_ROUTING_IRQCHIP,	\
> +	  .u.irqchip = { .irqchip = KVM_IRQCHIP_IOAPIC, .pin = (irq) } }
> +#define ROUTING_ENTRY1(irq) IOAPIC_ROUTING_ENTRY(irq)
> +
> +#define PIC_ROUTING_ENTRY(irq) \
> +	{ .gsi = irq, .type = KVM_IRQ_ROUTING_IRQCHIP,	\
> +	  .u.irqchip = { .irqchip = SELECT_PIC(irq), .pin = (irq) % 8 } }
> +#define ROUTING_ENTRY2(irq) \
> +	IOAPIC_ROUTING_ENTRY(irq), PIC_ROUTING_ENTRY(irq)
> +
> +static const struct kvm_irq_routing_entry default_routing[] = {
> +	ROUTING_ENTRY2(0), ROUTING_ENTRY2(1),
> +	ROUTING_ENTRY2(2), ROUTING_ENTRY2(3),
> +	ROUTING_ENTRY2(4), ROUTING_ENTRY2(5),
> +	ROUTING_ENTRY2(6), ROUTING_ENTRY2(7),
> +	ROUTING_ENTRY2(8), ROUTING_ENTRY2(9),
> +	ROUTING_ENTRY2(10), ROUTING_ENTRY2(11),
> +	ROUTING_ENTRY2(12), ROUTING_ENTRY2(13),
> +	ROUTING_ENTRY2(14), ROUTING_ENTRY2(15),
> +	ROUTING_ENTRY1(16), ROUTING_ENTRY1(17),
> +	ROUTING_ENTRY1(18), ROUTING_ENTRY1(19),
> +	ROUTING_ENTRY1(20), ROUTING_ENTRY1(21),
> +	ROUTING_ENTRY1(22), ROUTING_ENTRY1(23),
> +};
> +
>   int kvm_ioapic_init(struct kvm *kvm)
>   {
>   	struct kvm_ioapic *ioapic;
> @@ -731,8 +757,14 @@ int kvm_ioapic_init(struct kvm *kvm)
>   	if (ret < 0) {
>   		kvm->arch.vioapic = NULL;
>   		kfree(ioapic);
> +		return ret;
>   	}
>   
> +	ret = kvm_set_irq_routing(kvm, default_routing,
> +				  ARRAY_SIZE(default_routing), 0);
> +	if (ret)
> +		kvm_ioapic_destroy(kvm);
> +
>   	return ret;
>   }
>   
> diff --git a/arch/x86/kvm/irq.h b/arch/x86/kvm/irq.h
> index 33dd5666b656..f6134289523e 100644
> --- a/arch/x86/kvm/irq.h
> +++ b/arch/x86/kvm/irq.h
> @@ -107,7 +107,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_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 b85e4be2ddff..998c4a34d87c 100644
> --- a/arch/x86/kvm/irq_comm.c
> +++ b/arch/x86/kvm/irq_comm.c
> @@ -334,38 +334,6 @@ bool kvm_intr_is_single_vcpu(struct kvm *kvm, struct kvm_lapic_irq *irq,
>   }
>   EXPORT_SYMBOL_GPL(kvm_intr_is_single_vcpu);
>   
> -#define IOAPIC_ROUTING_ENTRY(irq) \
> -	{ .gsi = irq, .type = KVM_IRQ_ROUTING_IRQCHIP,	\
> -	  .u.irqchip = { .irqchip = KVM_IRQCHIP_IOAPIC, .pin = (irq) } }
> -#define ROUTING_ENTRY1(irq) IOAPIC_ROUTING_ENTRY(irq)
> -
> -#define PIC_ROUTING_ENTRY(irq) \
> -	{ .gsi = irq, .type = KVM_IRQ_ROUTING_IRQCHIP,	\
> -	  .u.irqchip = { .irqchip = SELECT_PIC(irq), .pin = (irq) % 8 } }
> -#define ROUTING_ENTRY2(irq) \
> -	IOAPIC_ROUTING_ENTRY(irq), PIC_ROUTING_ENTRY(irq)
> -
> -static const struct kvm_irq_routing_entry default_routing[] = {
> -	ROUTING_ENTRY2(0), ROUTING_ENTRY2(1),
> -	ROUTING_ENTRY2(2), ROUTING_ENTRY2(3),
> -	ROUTING_ENTRY2(4), ROUTING_ENTRY2(5),
> -	ROUTING_ENTRY2(6), ROUTING_ENTRY2(7),
> -	ROUTING_ENTRY2(8), ROUTING_ENTRY2(9),
> -	ROUTING_ENTRY2(10), ROUTING_ENTRY2(11),
> -	ROUTING_ENTRY2(12), ROUTING_ENTRY2(13),
> -	ROUTING_ENTRY2(14), ROUTING_ENTRY2(15),
> -	ROUTING_ENTRY1(16), ROUTING_ENTRY1(17),
> -	ROUTING_ENTRY1(18), ROUTING_ENTRY1(19),
> -	ROUTING_ENTRY1(20), ROUTING_ENTRY1(21),
> -	ROUTING_ENTRY1(22), ROUTING_ENTRY1(23),
> -};
> -
> -int kvm_setup_default_irq_routing(struct kvm *kvm)
> -{
> -	return kvm_set_irq_routing(kvm, default_routing,
> -				   ARRAY_SIZE(default_routing), 0);
> -}
> -
>   void kvm_scan_ioapic_irq(struct kvm_vcpu *vcpu, u32 dest_id, u16 dest_mode,
>   			 u8 vector, unsigned long *ioapic_handled_vectors)
>   {
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index f9f798f286ce..4a9c252c9dab 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -7118,12 +7118,6 @@ int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
>   			goto create_irqchip_unlock;
>   		}
>   
> -		r = kvm_setup_default_irq_routing(kvm);
> -		if (r) {
> -			kvm_ioapic_destroy(kvm);
> -			kvm_pic_destroy(kvm);
> -			goto create_irqchip_unlock;
> -		}
>   		/* Write kvm->irq_routing before enabling irqchip_in_kernel. */
>   		smp_wmb();
>   		kvm->arch.irqchip_mode = KVM_IRQCHIP_KERNEL;