Re: [PATCH for-4.21?] AMD/IOMMU: unshare IRQ .ack and .disable handlers

[Jan Beulich <jbeulich@xxxxxxxx>]

On 21.10.2025 11:59, Jan Beulich wrote:
> On 21.10.2025 11:48, Roger Pau Monné wrote:
>> On Mon, Oct 20, 2025 at 04:16:13PM +0200, Jan Beulich wrote:
>>> @@ -442,6 +440,13 @@ static unsigned int cf_check iommu_msi_s
>>>      return 0;
>>>  }
>>>  
>>> +static void cf_check iommu_msi_ack(struct irq_desc *desc)
>>> +{
>>> +    irq_complete_move(desc);
>>> +    iommu_msi_mask(desc);
>>> +    move_masked_irq(desc);
>>
>> Not sure it matters much, as I don't expect IOMMU interrupts to move
>> around frequently, but do we really need to mask the source?  The
>> update of the interrupt would be done atomically, as we know IOMMU is
>> available.
> 
> First I wanted to keep things in sync with other, similar functions. Then
> the masking here may be not only about the moving, but also about this
> actually being the .ack handler. In fact, when taking into account the
> combination of both aspects, don't we need to deal with the case of this
> being the last IRQ on the "old" vector, with us wanting to prevent another
> IRQ on the "new" vector until we actually handled the IRQ? The LAPIC ack
> (only done in the .end handler) wouldn't guard against that, if the "new"
> vector is a higher priority one than the "old" one.

... or on a different CPU.

Then again such nesting is avoided by generic logic, in particular via the
IRQ_INPROGRESS flag (in combination with the IRQ_PENDING one). So perhaps
you're right and the masking could be avoided (not just here).

Jan