Re: [PATCH v5] vpci: Add resizable bar support

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

On 2025/1/21 16:46, Roger Pau Monné wrote:
> On Tue, Jan 14, 2025 at 11:26:36AM +0800, Jiqian Chen wrote:
>> +    ctrl = pci_conf_read32(pdev->sbdf, rebar_offset + PCI_REBAR_CTRL(0));
>> +    nbars = MASK_EXTR(ctrl, PCI_REBAR_CTRL_NBAR_MASK);
>> +    for ( unsigned int i = 0; i < nbars; i++ )
>> +    {
>> +        int rc;
>> +        struct vpci_bar *bar;
>> +        unsigned int index;
>> +
>> +        ctrl = pci_conf_read32(pdev->sbdf, rebar_offset + 
>> PCI_REBAR_CTRL(i));
>> +        index = ctrl & PCI_REBAR_CTRL_BAR_IDX;
>> +        if ( index >= PCI_HEADER_NORMAL_NR_BARS )
>> +        {
>> +            printk(XENLOG_ERR "%pd %pp: too big BAR number %u in 
>> REBAR_CTRL\n",
>> +                   pdev->domain, &pdev->sbdf, index);
>> +            continue;
>> +        }
>> +
>> +        bar = &pdev->vpci->header.bars[index];
>> +        if ( bar->type != VPCI_BAR_MEM64_LO && bar->type != VPCI_BAR_MEM32 )
>> +        {
>> +            printk(XENLOG_ERR "%pd %pp: BAR%u is not in memory space\n",
>> +                   pdev->domain, &pdev->sbdf, index);
>> +            continue;
>> +        }
>> +
>> +        rc = vpci_add_register(pdev->vpci, vpci_hw_read32, vpci_hw_write32,
>> +                               rebar_offset + PCI_REBAR_CAP(i), 4, NULL);
>> +        if ( rc )
>> +        {
>> +            /*
>> +             * TODO: for failed pathes, need to hide ReBar capability
>> +             * from hardware domain instead of returning an error.
>> +             */
>> +            printk(XENLOG_ERR "%pd %pp: fail to add reg of REBAR_CAP 
>> rc=%d\n",
>> +                   pdev->domain, &pdev->sbdf, rc);
>> +            return rc;
>> +        }
>> +
>> +        rc = vpci_add_register(pdev->vpci, vpci_hw_read32, rebar_ctrl_write,
>> +                               rebar_offset + PCI_REBAR_CTRL(i), 4, bar);
>> +        if ( rc )
>> +        {
>> +            printk(XENLOG_ERR "%pd %pp: fail to add reg of REBAR_CTRL 
>> rc=%d\n",
>> +                   pdev->domain, &pdev->sbdf, rc);
>> +            return rc;
> 
> I think we said we wanted to attempt to continue here, rather than
> returning an error and thus removing all vPCI handlers from the
> device?
I thought the result of your discussion with Jan was that I only needed to 
change the above two error paths to be "continue".
If these two also need to be changed, I will modify them in the next version.


-- 
Best regards,
Jiqian Chen.