Re: [PATCH v4 21/25] xen/riscv: implement IRQ routing for device passthrough

[Jan Beulich <jbeulich@suse.com>]

On 01.07.2026 16:49, Oleksii Kurochko wrote:
> On 6/29/26 5:55 PM, Jan Beulich wrote:
>> On 26.06.2026 17:46, Oleksii Kurochko wrote:
>>> @@ -221,3 +239,215 @@ void do_IRQ(struct cpu_user_regs *regs, unsigned int irq)
>>>       spin_unlock(&desc->lock);
>>>       irq_exit();
>>>   }
>>> +
>>> +static inline struct irq_guest *irq_get_guest_info(struct irq_desc *desc)
>>> +{
>>> +    ASSERT(spin_is_locked(&desc->lock));
>>> +    ASSERT(test_bit(_IRQ_GUEST, &desc->status));
>>> +    ASSERT(desc->action != NULL);
>>> +
>>> +    return desc->action->dev_id;
>>> +}
>>> +
>>> +static inline struct domain *irq_get_domain(struct irq_desc *desc)
>>> +{
>>> +    return irq_get_guest_info(desc)->d;
>>> +}
>>
>> Does this really need a separate helper? (You effectively open-code it
>> anyway in release_guest_irq().)
> 
> Considering that I used irq_get_domain() once at all (even in 
> downstream) I will drop that.

Good.

>> For both functions: "inline" generally wants limiting to header files.
> 
> Regarding inline I think that I don't understand, it isn't in the header 
> file as irq_get_guest_info() is used only in this file.

Right. I didn't as it to move to a header file.

> Do I understand you correctly and it is needed just to drop "inline" for
> irq_get_guest_info()?

Yes. It can be simply static here, and the compiler can figure out whether
to inline it.

>>> +    spin_unlock_irqrestore(&desc->lock, flags);
>>> +
>>> +    release_irq(desc->irq, info);
>>> +    xvfree(info);
>>
>> If, in release_irq(), action isn't freed, it's ->dev_id field will now have
>> a dangling pointer. (I think I did point this out before.)
> 
> It should freed in release_irq() as route_irq_to_guest() always set 
> action->free_on_release = true;

Well, "free_on_release" must exits for a purpose. I.e. there must be, now
or soon, cases where it's set to false. Else simply drop the field.

Jan