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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 21.01.2025 10:29, Roger Pau Monné wrote:
> On Tue, Jan 21, 2025 at 09:10:26AM +0000, Chen, Jiqian wrote:
>> 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.
> 
> Hm, let's wait for Jan to confirm, but even if handler cannot be setup
> for some of the registers, it's better than just allowing dom0
> unmediated access to the capability.

I remained silent on this because I accepted this middle ground as ...

> None of this is ideal, but it seems to be the option that gives dom0
> most options to successfully boot.

... perhaps the most reasonable compromise.

Jan