Re: [PATCH 4/5] KVM: arm64: Unmap vLPIs affected by changes to GSI routing information

[Marc Zyngier <maz@kernel.org>]

On Fri, 23 May 2025 17:08:09 +0100,
Oliver Upton <oliver.upton@linux.dev> wrote:
> 
> KVM's interrupt infrastructure is dodgy at best, allowing for some ugly
> 'off label' usage of the various UAPIs. In one example, userspace can
> change the routing entry of a particular "GSI" after configuring
> irqbypass with KVM_IRQFD. KVM/arm64 is oblivious to this, and winds up
> preserving the stale translation in cases where vLPIs are configured.
> 
> Honor userspace's intentions and tear down the vLPI mapping if affected
> by a "GSI" routing change. Make no attempt to reconstruct vLPIs if the
> new target is an MSI and just fall back to software injection.
> 
> Tested-by: Sweet Tea Dorminy <sweettea@google.com>
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> ---
>  arch/arm64/kvm/arm.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 1de49b48e35e..505d504b52b5 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -2790,6 +2790,7 @@ int kvm_arch_irq_bypass_add_producer(struct irq_bypass_consumer *cons,
>  	return kvm_vgic_v4_set_forwarding(irqfd->kvm, prod->irq,
>  					  &irqfd->irq_entry);
>  }
> +
>  void kvm_arch_irq_bypass_del_producer(struct irq_bypass_consumer *cons,
>  				      struct irq_bypass_producer *prod)
>  {
> @@ -2803,6 +2804,28 @@ void kvm_arch_irq_bypass_del_producer(struct irq_bypass_consumer *cons,
>  	kvm_vgic_v4_unset_forwarding(irqfd->kvm, prod->irq);
>  }
>  
> +bool kvm_arch_irqfd_route_changed(struct kvm_kernel_irq_routing_entry *old,
> +				  struct kvm_kernel_irq_routing_entry *new)
> +{
> +	if (new->type != KVM_IRQ_ROUTING_MSI)
> +		return true;
> +
> +	return memcmp(&old->msi, &new->msi, sizeof(new->msi));
> +}
> +
> +int kvm_arch_update_irqfd_routing(struct kvm *kvm, unsigned int host_irq,
> +				  uint32_t guest_irq, bool set)

If we're adding this, can we take out the trash and get rid of this
'set' parameter? Its only purpose is to be set to '1' and fed to the
x86-specific stuff. How about this:

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index df5b99ea1f181..2d69609c1ec00 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -13613,9 +13613,9 @@ void kvm_arch_irq_bypass_del_producer(struct irq_bypass_consumer *cons,
 }
 
 int kvm_arch_update_irqfd_routing(struct kvm *kvm, unsigned int host_irq,
-				   uint32_t guest_irq, bool set)
+				   uint32_t guest_irq)
 {
-	return kvm_x86_call(pi_update_irte)(kvm, host_irq, guest_irq, set);
+	return kvm_x86_call(pi_update_irte)(kvm, host_irq, guest_irq, true);
 }
 
 bool kvm_arch_irqfd_route_changed(struct kvm_kernel_irq_routing_entry *old,
diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
index 11e5d1e3f12ea..2e60d0bf02e2f 100644
--- a/virt/kvm/eventfd.c
+++ b/virt/kvm/eventfd.c
@@ -287,7 +287,7 @@ void __attribute__((weak)) kvm_arch_irq_bypass_start(
 
 int  __attribute__((weak)) kvm_arch_update_irqfd_routing(
 				struct kvm *kvm, unsigned int host_irq,
-				uint32_t guest_irq, bool set)
+				uint32_t guest_irq)
 {
 	return 0;
 }
@@ -621,7 +621,7 @@ void kvm_irq_routing_update(struct kvm *kvm)
 		    kvm_arch_irqfd_route_changed(&old, &irqfd->irq_entry)) {
 			int ret = kvm_arch_update_irqfd_routing(
 					irqfd->kvm, irqfd->producer->irq,
-					irqfd->gsi, 1);
+					irqfd->gsi);
 			WARN_ON(ret);
 		}
 #endif


Thanks,

	M.

-- 
Jazz isn't dead. It just smells funny.