On 2025/6/18 22:37, Jan Beulich wrote:
> On 12.06.2025 11:29, Jiqian Chen wrote:
>> --- a/xen/drivers/vpci/msi.c
>> +++ b/xen/drivers/vpci/msi.c
>> @@ -193,6 +193,33 @@ static void cf_check mask_write(
>> msi->mask = val;
>> }
>>
>> +static int cf_check cleanup_msi(struct pci_dev *pdev)
>> +{
>> + int rc;
>> + unsigned int end, size;
>> + 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;
>> +
>> + size = end - ctrl;
>> +
>> + rc = vpci_remove_registers(vpci, ctrl, size);
>> + if ( rc )
>> + return rc;
>> +
>> + XFREE(vpci->msi);
>> +
>> + return vpci_add_register(pdev->vpci, vpci_hw_read16, NULL, ctrl, 2,
>> NULL);
>
> Why not use the local variable "vpci" here?
That's my fault, will change in next version. And the same in msix.c
>
> I also think this (re)adding warrants a comment, as to why we do this.
I write a draft as below, could you help to modify?
/*
* The driver may not traverse the capability list and think device
* supports MSI by default. So here let the control register of MSI
* be Read-Only is to ensure MSI disabled.
*/
>
> Jan
--
Best regards,
Jiqian Chen.
