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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 02.07.2026 11:33, Oleksii Kurochko wrote:
> 
> 
> On 7/2/26 8:38 AM, Jan Beulich wrote:
> 
>>>>> +    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.
> 
> I can't simply remove this field because it is part of the common 
> `struct irqaction` and is used by other arc-s. (I assume that it is not 
> what you fully meant...)
> 
> IIUC, this field is used to determine whether an irqaction is 
> heap-allocated (and therefore should be freed) or statically allocated 
> (and therefore should not be freed).

Yes. However, all uses of the field are in arch-specific code. So in
principle it could be #ifdef-ed out for RISC-V. There may be a better
option, though:

> At the moment, all IRQ actions are heap-allocated (on RISC-V), so 
> free_on_release should always be set to true. In particular, the code 
> snippet you asked about releases a guest interrupt, and guest interrupt 
> actions are always heap-allocated. As a result, when release_guest_irq() 
> calls release_irq(), the associated irqaction will be freed.
> 
> So, from what I can see, the current behavior is correct and I think it 
> should be left as it is. Do you have any concerns about this?

If you only ever have the field set to true, use ASSERT() in place of
if().

Jan