Re: [PATCH 08/67] KVM: x86: Pass new routing entries and irqfd when updating IRTEs

[Sairaj Kodilkar]

On 4/11/2025 7:31 PM, Sean Christopherson wrote:
> On Fri, Apr 11, 2025, Arun Kodilkar, Sairaj wrote:
> > On 4/5/2025 1:08 AM, Sean Christopherson wrote:
> > > +int avic_pi_update_irte(struct kvm_kernel_irqfd *irqfd, struct kvm *kvm,
> > > +			unsigned int host_irq, uint32_t guest_irq,
> > > +			struct kvm_kernel_irq_routing_entry *new)
> > >    {
> > >    	struct kvm_kernel_irq_routing_entry *e;
> > >    	struct kvm_irq_routing_table *irq_rt;
> > >    	bool enable_remapped_mode = true;
> > > +	bool set = !!new;
> > >    	int idx, ret = 0;
> > >    	if (!kvm_arch_has_assigned_device(kvm) || !kvm_arch_has_irq_bypass())
> > > @@ -925,6 +919,8 @@ int avic_pi_update_irte(struct kvm *kvm, unsigned int host_irq,
> > >    		if (e->type != KVM_IRQ_ROUTING_MSI)
> > >    			continue;
> > > +		WARN_ON_ONCE(new && memcmp(e, new, sizeof(*new)));
> > > +
> >
> > Hi Sean,
> >
> > In kvm_irq_routing_update() function, its possible that there are
> > multiple entries in the `kvm_irq_routing_table`,
>
> Not if one of them is an MSI.  In setup_routing_entry():
>
> 	/*
> 	 * Do not allow GSI to be mapped to the same irqchip more than once.
> 	 * Allow only one to one mapping between GSI and non-irqchip routing.
> 	 */
> 	hlist_for_each_entry(ei, &rt->map[gsi], link)
> 		if (ei->type != KVM_IRQ_ROUTING_IRQCHIP ||
> 		    ue->type != KVM_IRQ_ROUTING_IRQCHIP ||
> 		    ue->u.irqchip.irqchip == ei->irqchip.irqchip)
> 			return -EINVAL;
>
> > and `irqfd_update()` ends up setting up the new entry type to 0 instead of
> > copying the entry.
> >
> > if (n_entries == 1)
> >      irqfd->irq_entry = *e;
> > else
> >      irqfd->irq_entry.type = 0;
> >
> > Since irqfd_update() did not copy the entry to irqfd->entries, the "new"
> > will not match entry "e" obtained from irq_rt, which can trigger a false
> > WARN_ON.
>
> And since there can only be one MSI, if there are multiple routing entries, then
> the WARN won't be reached thanks to the continue that's just above:
>
> 		if (e->type != KVM_IRQ_ROUTING_MSI)
> 			continue;

Thanks.. I understand it now. I did not see complete code hence the 
confusion. Sorry about that.

Regards
Sairaj kodilkar