Re: [PATCH v10 2/4] vpci/rebar: Free Rebar resources when init_rebar() fails

["Chen, Jiqian" <Jiqian.Chen@xxxxxxx>]

On 2025/8/5 16:48, Jan Beulich wrote:
> On 05.08.2025 05:49, Jiqian Chen wrote:
>> --- a/xen/drivers/vpci/rebar.c
>> +++ b/xen/drivers/vpci/rebar.c
>> @@ -49,6 +49,32 @@ static void cf_check rebar_ctrl_write(const struct 
>> pci_dev *pdev,
>>      bar->guest_addr = bar->addr;
>>  }
>>  
>> +static int cf_check cleanup_rebar(const struct pci_dev *pdev)
>> +{
>> +    int rc;
>> +    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) )
>> +    {
>> +        ASSERT_UNREACHABLE();
>> +        return 0;
>> +    }
>> +
>> +    ctrl = pci_conf_read32(pdev->sbdf, rebar_offset + PCI_REBAR_CTRL(0));
>> +    nbars = MASK_EXTR(ctrl, PCI_REBAR_CTRL_NBAR_MASK);
>> +
>> +    rc = vpci_remove_registers(pdev->vpci, rebar_offset + PCI_REBAR_CAP(0),
>> +                               PCI_REBAR_CTRL(nbars - 1));
>> +    if ( rc )
>> +        printk(XENLOG_ERR "%pd %pp: fail to remove Rebar handlers rc=%d\n",
>> +               pdev->domain, &pdev->sbdf, rc);
> 
> MSI and MSI-X (now) have ASSERT_UNREACHABLE() on their respective paths. Is
> there a reason this shouldn't be done here as well?
Will add.

> 
> MSI and MSI-X further have another add-register below here, to ensure the
> control register cannot be written. Again - is there a reason the same
> shouldn't be done here? (If so, I think this may want putting in a comment.)
Since extended capabilities are only exposed to dom0, and dom0 has no 
limitations to access devices.
It since there is not much point in adding such a handler for rebar.

> 
> Jan

-- 
Best regards,
Jiqian Chen.