On 2024/11/13 17:30, Roger Pau Monné wrote:
> On Wed, Nov 13, 2024 at 04:00:27PM +0800, Jiqian Chen wrote:
>> Some devices, like discrete GPU of amd, support resizable bar capability,
>> but vpci of Xen doesn't support this feature, so they fail to resize bars
>> and then cause probing failure.
>>
>> According to PCIe spec, each bar that support resizing has two registers,
>> PCI_REBAR_CAP and PCI_REBAR_CTRL, so add these two registers and their
>> corresponding handler into vpci.
>>
>> PCI_REBAR_CAP is RO, only provide reading.
>>
>> PCI_REBAR_CTRL only has bar size is RW, so add write function to support
>> setting the new size.
>
> I think the logic to handle resizable BAR could be much simpler. Some
> time ago I've made a patch to add support for it, but due to lack of
> hardware on my side to test it I've never submitted it.
>
> My approach would be to detect the presence of the
> PCI_EXT_CAP_ID_REBAR capability in init_header(), and if the
> capability is present force the sizing of BARs each time they are
> mapped in modify_bars(). I don't think we need to trap accesses to
> the capability itself, as resizing can only happen when memory
> decoding is not enabled for the device. It's enough to fetch the size
> of the BARs ahead of each enabling of memory decoding.
>
> Note that memory decoding implies mapping the BARs into the p2m, which
> is already an expensive operation, the extra sizing is unlikely to
> make much of a difference performance wise.
>
> I've found the following on my git tree and rebased on top of staging:
OK.
Do you need me to validate your patch in my environment?
And I have one question: where does your patch do writing the resizing size
into hardware?
>
> diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
> index ef6c965c081c..045aa4bdadc8 100644
> --- a/xen/drivers/vpci/header.c
> +++ b/xen/drivers/vpci/header.c
> @@ -356,6 +356,30 @@ static int modify_bars(const struct pci_dev *pdev,
> uint16_t cmd, bool rom_only)
>
> ASSERT(rangeset_is_empty(bar->mem));
>
> + if ( bar->type != VPCI_BAR_ROM && header->bars_resizable &&
> + (cmd & PCI_COMMAND_MEMORY) )
> + {
> + uint64_t addr, size;
> +
> + pci_size_mem_bar(pdev->sbdf, PCI_BASE_ADDRESS_0 + i * 4,
> + &addr, &size, 0);
> +
> + if ( bar->addr != addr )
> + printk(XENLOG_G_ERR
> + "%pp: BAR#%u address mismatch %#lx vs %#lx\n",
> + &pdev->sbdf, i, bar->addr, addr);
> +
> + if ( bar->size != size )
> + {
> + printk(XENLOG_G_DEBUG
> + "%pp: detected BAR#%u size change (%#lx -> %#lx)\n",
> + &pdev->sbdf, i, bar->size, size);
> + bar->size = size;
> + end = PFN_DOWN(bar->addr + size - 1);
> + end_guest = PFN_DOWN(bar->guest_addr + size - 1);
> + }
> + }
> +
> /*
> * Make sure that the guest set address has the same page offset
> * as the physical address on the host or otherwise things won't
> work as
> @@ -870,6 +894,9 @@ static int cf_check init_header(struct pci_dev *pdev)
> if ( pdev->ignore_bars )
> return 0;
>
> + header->bars_resizable = pci_find_ext_capability(pdev->sbdf,
> + PCI_EXT_CAP_ID_REBAR);
> +
> cmd = pci_conf_read16(pdev->sbdf, PCI_COMMAND);
>
> /*
> diff --git a/xen/include/xen/pci_regs.h b/xen/include/xen/pci_regs.h
> index 250ba106dbd3..c543a2b86778 100644
> --- a/xen/include/xen/pci_regs.h
> +++ b/xen/include/xen/pci_regs.h
> @@ -459,6 +459,7 @@
> #define PCI_EXT_CAP_ID_ARI 14
> #define PCI_EXT_CAP_ID_ATS 15
> #define PCI_EXT_CAP_ID_SRIOV 16
> +#define PCI_EXT_CAP_ID_REBAR 21
>
> /* Advanced Error Reporting */
> #define PCI_ERR_UNCOR_STATUS 4 /* Uncorrectable Error Status */
> diff --git a/xen/include/xen/vpci.h b/xen/include/xen/vpci.h
> index 41e7c3bc2791..45ebc1bb3356 100644
> --- a/xen/include/xen/vpci.h
> +++ b/xen/include/xen/vpci.h
> @@ -129,6 +129,8 @@ struct vpci {
> * upon to know whether BARs are mapped into the guest p2m.
> */
> bool bars_mapped : 1;
> + /* Device has the Resizable BARs capability. */
> + bool bars_resizable : 1;
> /* FIXME: currently there's no support for SR-IOV. */
> } header;
>
--
Best regards,
Jiqian Chen.
