On 2025/6/5 23:14, Roger Pau Monné wrote:
> On Mon, May 26, 2025 at 05:45:57PM +0800, Jiqian Chen wrote:
>> When init_rebar() fails, current logic return fail and free Rebar-related
>> resources in vpci_deassign_device(). But the previous new changes will
>> hide Rebar 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 Rebar.
>>
>> To do that, implement cleanup function for Rebar.
>>
>> Signed-off-by: Jiqian Chen <jiqian.c...@amd.com>
>
> LGTM, just one nit about a bounds check.
>
>> ---
>> cc: "Roger Pau Monné" <roger....@citrix.com>
>> ---
>> v4->v5 changes:
>> * Change definition "static void cleanup_rebar" to "static int cf_check
>> cleanup_rebar" since cleanup hook is changed to be int.
>>
>> v3->v4 changes:
>> * Change function name from fini_rebar() to cleanup_rebar().
>> * Change the error number to be E2BIG and ENXIO in init_rebar().
>>
>> v2->v3 changes:
>> * Use fini_rebar() to remove all register instead of in the failure path of
>> init_rebar();
>>
>> v1->v2 changes:
>> * Called vpci_remove_registers() to remove all possible registered registers
>> instead of using a array to record all registered register.
>>
>> Best regards,
>> Jiqian Chen.
>> ---
>> xen/drivers/vpci/rebar.c | 35 ++++++++++++++++++++++++-----------
>> 1 file changed, 24 insertions(+), 11 deletions(-)
>>
>> diff --git a/xen/drivers/vpci/rebar.c b/xen/drivers/vpci/rebar.c
>> index 9cafd80ca2c9..4b1892fab3d6 100644
>> --- a/xen/drivers/vpci/rebar.c
>> +++ b/xen/drivers/vpci/rebar.c
>> @@ -49,6 +49,26 @@ static void cf_check rebar_ctrl_write(const struct
>> pci_dev *pdev,
>> bar->guest_addr = bar->addr;
>> }
>>
>> +static int cf_check cleanup_rebar(struct pci_dev *pdev)
>> +{
>> + uint32_t ctrl;
>> + unsigned int nbars;
>> + unsigned int rebar_offset = pci_find_ext_capability(pdev->sbdf,
>> +
>> PCI_EXT_CAP_ID_REBAR);
>> +
>> + if ( !rebar_offset || !is_hardware_domain(pdev->domain) )
>
> I think you could check rebar_offset < PCI_CFG_SPACE_SIZE to be more
> accurate?
OK.
Do I need to change in init_rebar()?
>
> Thanks, Roger.
--
Best regards,
Jiqian Chen.
