Re: [PATCH v7 7/8] vpci/msi: Free MSI resources when init_msi() fails

[Jan Beulich <jbeulich@xxxxxxxx>]

On 09.07.2025 08:10, Chen, Jiqian wrote:
> On 2025/7/8 23:13, Jan Beulich wrote:
>> On 04.07.2025 09:08, Jiqian Chen wrote:
>>> @@ -193,6 +234,42 @@ static void cf_check mask_write(
>>>      msi->mask = val;
>>>  }
>>>  
>>> +static int cf_check cleanup_msi(const struct pci_dev *pdev)
>>> +{
>>> +    int rc;
>>> +    unsigned int end;
>>> +    struct vpci *vpci = pdev->vpci;
>>> +    const unsigned int msi_pos = pdev->msi_pos;
>>> +    const unsigned int ctrl = msi_control_reg(msi_pos);
>>> +
>>> +    if ( !msi_pos || !vpci->msi )
>>> +        return 0;
>>> +
>>> +    if ( vpci->msi->masking )
>>> +        end = msi_pending_bits_reg(msi_pos, vpci->msi->address64);
>>> +    else
>>> +        end = msi_mask_bits_reg(msi_pos, vpci->msi->address64) - 2;
>>
>> What's this "- 2" for? If there's no masking support, you want to cut off
>> _at_ the mask register, not 2 bytes ahead of it? Just like you cut off at
>> the pending bits register when there is masking support.
> "-2" here is to cut the reserved 2 bytes of Message Data if there is no 
> masking support.

Hmm, init_msi() doesn't intercept Extended Message Data when present. I
think that's wrong there, leading to the oddity here. (Imo such use of
hard coded numbers would almost always want to be accompanied by a
comment.)

Jan