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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

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.

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

Thanks, Roger.