On 2025/6/5 23:19, Roger Pau Monné wrote:
> On Mon, May 26, 2025 at 05:45:58PM +0800, Jiqian Chen wrote:
>> When init_msi() fails, current logic return fail and free MSI-related
>> resources in vpci_deassign_device(). But the previous new changes will
>> hide MSI capability and return success, it can't reach
>> vpci_deassign_device() to remove resources if hiding success, so those
>> resources must be removed in cleanup function of MSI.
>>
>> To do that, implement cleanup function for MSI.
>>
>> Signed-off-by: Jiqian Chen <jiqian.c...@amd.com>
>> ---
>> cc: "Roger Pau Monné" <roger....@citrix.com>
>> ---
>> v4->v5 changes:
>> * Change definition "static void cleanup_msi" to "static int cf_check
>> cleanup_msi" since cleanup hook is changed to be int.
>> * Add a read-only register for MSI Control Register in the end of
>> cleanup_msi.
>>
>> v3->v4 changes:
>> * Change function name from fini_msi() to cleanup_msi().
>> * Remove unnecessary comment.
>> * Change to use XFREE to free vpci->msi.
>>
>> v2->v3 changes:
>> * Remove all fail path, and use fini_msi() hook instead.
>> * Change the method to calculating the size of msi registers.
>>
>> v1->v2 changes:
>> * Added a new function fini_msi to free all MSI resources instead of using
>> an array to record registered registers.
>>
>> Best regards,
>> Jiqian Chen.
>> ---
>> xen/drivers/vpci/msi.c | 29 ++++++++++++++++++++++++++++-
>> 1 file changed, 28 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/drivers/vpci/msi.c b/xen/drivers/vpci/msi.c
>> index 2d45c7867de7..4e106c39efae 100644
>> --- 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 )
>
> Possibly same request as the previous patch, msi_pos should be after
> the PCI standard header.
msi_pos <= PCI_CAPABILITY_LIST ?
Or msi_pos < 0x40 ?
>
> Thanks, Roger.
--
Best regards,
Jiqian Chen.
