| Message ID | 20241113080027.244240-1-Jiqian.Chen@amd.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | vpci: Add resizable bar support | expand |
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: 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;
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; >
On Wed, Nov 13, 2024 at 10:00:33AM +0000, Chen, Jiqian wrote: > 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? Yes please, I have no way to test it. Let's see what others think about the different approaches. > And I have one question: where does your patch do writing the resizing size into hardware? dom0 has unrestricted access to the resize capability, so the value written by dom0 is propagated to the hardware without modification. I would be wary of exposing the resize capability to untrusted domains, as allowing a domU to change the size of BARs can lead to overlapping if the hardware domain hasn't accounted for the increase in BAR size. Thanks, Roger.
On 13.11.2024 11:30, Roger Pau Monné wrote: > On Wed, Nov 13, 2024 at 10:00:33AM +0000, Chen, Jiqian wrote: >> 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? > > Yes please, I have no way to test it. Let's see what others think > about the different approaches. I'd certainly prefer your simpler form, if it's safe and fits the needs. >> And I have one question: where does your patch do writing the resizing size into hardware? > > dom0 has unrestricted access to the resize capability, so the value > written by dom0 is propagated to the hardware without modification. > > I would be wary of exposing the resize capability to untrusted > domains, as allowing a domU to change the size of BARs can lead to > overlapping if the hardware domain hasn't accounted for the increase > in BAR size. Question is how the feature is used in practice: If it was a driver to request the re-size, I'd have a hard time seeing how we could make that work without intercepting accesses to the capability for DomU-s (implying to expose it in the first place, of course). Jan
On Wed, Nov 13, 2024 at 11:36:46AM +0100, Jan Beulich wrote: > On 13.11.2024 11:30, Roger Pau Monné wrote: > > On Wed, Nov 13, 2024 at 10:00:33AM +0000, Chen, Jiqian wrote: > >> 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? > > > > Yes please, I have no way to test it. Let's see what others think > > about the different approaches. > > I'd certainly prefer your simpler form, if it's safe and fits the needs. > > >> And I have one question: where does your patch do writing the resizing size into hardware? > > > > dom0 has unrestricted access to the resize capability, so the value > > written by dom0 is propagated to the hardware without modification. > > > > I would be wary of exposing the resize capability to untrusted > > domains, as allowing a domU to change the size of BARs can lead to > > overlapping if the hardware domain hasn't accounted for the increase > > in BAR size. > > Question is how the feature is used in practice: If it was a driver to > request the re-size, I'd have a hard time seeing how we could make that > work without intercepting accesses to the capability for DomU-s (implying > to expose it in the first place, of course). Question is also whether the capability is required for guests, as in OS drivers requesting it to be present for proper operation. I haven't given much thought about how to expose to domUs. The current patch doesn't attempt to expose to domUs either, as the capability is not added to the 'supported_caps' array. Thanks, Roger.
On 13.11.2024 11:56, Roger Pau Monné wrote: > On Wed, Nov 13, 2024 at 11:36:46AM +0100, Jan Beulich wrote: >> On 13.11.2024 11:30, Roger Pau Monné wrote: >>> On Wed, Nov 13, 2024 at 10:00:33AM +0000, Chen, Jiqian wrote: >>>> 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? >>> >>> Yes please, I have no way to test it. Let's see what others think >>> about the different approaches. >> >> I'd certainly prefer your simpler form, if it's safe and fits the needs. >> >>>> And I have one question: where does your patch do writing the resizing size into hardware? >>> >>> dom0 has unrestricted access to the resize capability, so the value >>> written by dom0 is propagated to the hardware without modification. >>> >>> I would be wary of exposing the resize capability to untrusted >>> domains, as allowing a domU to change the size of BARs can lead to >>> overlapping if the hardware domain hasn't accounted for the increase >>> in BAR size. >> >> Question is how the feature is used in practice: If it was a driver to >> request the re-size, I'd have a hard time seeing how we could make that >> work without intercepting accesses to the capability for DomU-s (implying >> to expose it in the first place, of course). > > Question is also whether the capability is required for guests, as in > OS drivers requesting it to be present for proper operation. > > I haven't given much thought about how to expose to domUs. The > current patch doesn't attempt to expose to domUs either, as the > capability is not added to the 'supported_caps' array. Hmm, I see. Yet then adding support to vPCI, but limited to Dom0, ends up odd in two ways: Another aspect that'll need dealing with for DomU-s, and the same functionality remaining unavailable (or at least not properly available, with all possible side effects) to PV Dom0. Jan
On Wed, Nov 13, 2024 at 12:01:23PM +0100, Jan Beulich wrote: > On 13.11.2024 11:56, Roger Pau Monné wrote: > > On Wed, Nov 13, 2024 at 11:36:46AM +0100, Jan Beulich wrote: > >> On 13.11.2024 11:30, Roger Pau Monné wrote: > >>> On Wed, Nov 13, 2024 at 10:00:33AM +0000, Chen, Jiqian wrote: > >>>> 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? > >>> > >>> Yes please, I have no way to test it. Let's see what others think > >>> about the different approaches. > >> > >> I'd certainly prefer your simpler form, if it's safe and fits the needs. > >> > >>>> And I have one question: where does your patch do writing the resizing size into hardware? > >>> > >>> dom0 has unrestricted access to the resize capability, so the value > >>> written by dom0 is propagated to the hardware without modification. > >>> > >>> I would be wary of exposing the resize capability to untrusted > >>> domains, as allowing a domU to change the size of BARs can lead to > >>> overlapping if the hardware domain hasn't accounted for the increase > >>> in BAR size. > >> > >> Question is how the feature is used in practice: If it was a driver to > >> request the re-size, I'd have a hard time seeing how we could make that > >> work without intercepting accesses to the capability for DomU-s (implying > >> to expose it in the first place, of course). > > > > Question is also whether the capability is required for guests, as in > > OS drivers requesting it to be present for proper operation. > > > > I haven't given much thought about how to expose to domUs. The > > current patch doesn't attempt to expose to domUs either, as the > > capability is not added to the 'supported_caps' array. > > Hmm, I see. Yet then adding support to vPCI, but limited to Dom0, ends up > odd in two ways: Another aspect that'll need dealing with for DomU-s, and > the same functionality remaining unavailable (or at least not properly > available, with all possible side effects) to PV Dom0. I think resizable BARs should just work for PV dom0, as Xen allows PV dom0 to map almost all physical memory. Xen doesn't require knowing the BAR positions and sizes like it does for PVH dom0. Note that resizable BAR capability is not exposed to domUs now either when using QEMU based pci-passthrough. Thanks, Roger.
On 13.11.2024 12:24, Roger Pau Monné wrote: > On Wed, Nov 13, 2024 at 12:01:23PM +0100, Jan Beulich wrote: >> On 13.11.2024 11:56, Roger Pau Monné wrote: >>> On Wed, Nov 13, 2024 at 11:36:46AM +0100, Jan Beulich wrote: >>>> On 13.11.2024 11:30, Roger Pau Monné wrote: >>>>> On Wed, Nov 13, 2024 at 10:00:33AM +0000, Chen, Jiqian wrote: >>>>>> 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? >>>>> >>>>> Yes please, I have no way to test it. Let's see what others think >>>>> about the different approaches. >>>> >>>> I'd certainly prefer your simpler form, if it's safe and fits the needs. >>>> >>>>>> And I have one question: where does your patch do writing the resizing size into hardware? >>>>> >>>>> dom0 has unrestricted access to the resize capability, so the value >>>>> written by dom0 is propagated to the hardware without modification. >>>>> >>>>> I would be wary of exposing the resize capability to untrusted >>>>> domains, as allowing a domU to change the size of BARs can lead to >>>>> overlapping if the hardware domain hasn't accounted for the increase >>>>> in BAR size. >>>> >>>> Question is how the feature is used in practice: If it was a driver to >>>> request the re-size, I'd have a hard time seeing how we could make that >>>> work without intercepting accesses to the capability for DomU-s (implying >>>> to expose it in the first place, of course). >>> >>> Question is also whether the capability is required for guests, as in >>> OS drivers requesting it to be present for proper operation. >>> >>> I haven't given much thought about how to expose to domUs. The >>> current patch doesn't attempt to expose to domUs either, as the >>> capability is not added to the 'supported_caps' array. >> >> Hmm, I see. Yet then adding support to vPCI, but limited to Dom0, ends up >> odd in two ways: Another aspect that'll need dealing with for DomU-s, and >> the same functionality remaining unavailable (or at least not properly >> available, with all possible side effects) to PV Dom0. > > I think resizable BARs should just work for PV dom0, as Xen allows PV > dom0 to map almost all physical memory. Xen doesn't require knowing > the BAR positions and sizes like it does for PVH dom0. Does it really not need to know in any (corner) case? Are there guarantees that e.g. MSI-X table or PBA can't move when the size of the BAR covering them changes? > Note that resizable BAR capability is not exposed to domUs now either > when using QEMU based pci-passthrough. Of course. Jan
On Wed, Nov 13, 2024 at 12:29:11PM +0100, Jan Beulich wrote: > On 13.11.2024 12:24, Roger Pau Monné wrote: > > On Wed, Nov 13, 2024 at 12:01:23PM +0100, Jan Beulich wrote: > >> On 13.11.2024 11:56, Roger Pau Monné wrote: > >>> On Wed, Nov 13, 2024 at 11:36:46AM +0100, Jan Beulich wrote: > >>>> On 13.11.2024 11:30, Roger Pau Monné wrote: > >>>>> On Wed, Nov 13, 2024 at 10:00:33AM +0000, Chen, Jiqian wrote: > >>>>>> 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? > >>>>> > >>>>> Yes please, I have no way to test it. Let's see what others think > >>>>> about the different approaches. > >>>> > >>>> I'd certainly prefer your simpler form, if it's safe and fits the needs. > >>>> > >>>>>> And I have one question: where does your patch do writing the resizing size into hardware? > >>>>> > >>>>> dom0 has unrestricted access to the resize capability, so the value > >>>>> written by dom0 is propagated to the hardware without modification. > >>>>> > >>>>> I would be wary of exposing the resize capability to untrusted > >>>>> domains, as allowing a domU to change the size of BARs can lead to > >>>>> overlapping if the hardware domain hasn't accounted for the increase > >>>>> in BAR size. > >>>> > >>>> Question is how the feature is used in practice: If it was a driver to > >>>> request the re-size, I'd have a hard time seeing how we could make that > >>>> work without intercepting accesses to the capability for DomU-s (implying > >>>> to expose it in the first place, of course). > >>> > >>> Question is also whether the capability is required for guests, as in > >>> OS drivers requesting it to be present for proper operation. > >>> > >>> I haven't given much thought about how to expose to domUs. The > >>> current patch doesn't attempt to expose to domUs either, as the > >>> capability is not added to the 'supported_caps' array. > >> > >> Hmm, I see. Yet then adding support to vPCI, but limited to Dom0, ends up > >> odd in two ways: Another aspect that'll need dealing with for DomU-s, and > >> the same functionality remaining unavailable (or at least not properly > >> available, with all possible side effects) to PV Dom0. > > > > I think resizable BARs should just work for PV dom0, as Xen allows PV > > dom0 to map almost all physical memory. Xen doesn't require knowing > > the BAR positions and sizes like it does for PVH dom0. > > Does it really not need to know in any (corner) case? Are there guarantees > that e.g. MSI-X table or PBA can't move when the size of the BAR covering > them changes? Those are the same guarantees that Xen has from a PV dom0 not moving the position of the BARs after having MSI-X enabled for example. IOW: dom0 should set the BAR size before enabling MSI-X, just like it does for the BAR positions currently. The specification doesn't mention anything about MSI-X table or PBA explicitly not changing it's offset if the BAR where it resides is resized, so I don't think we have any guarantees. Albeit I would think it's unexpected for the MSI-X BIR or offset to change as a result of a BAR resize. There's an implementation note in the PCI specification that the BAR should be resized during resource allocation, prior to assigning the base address to the BAR. Thanks, Roger.
On Thu, Nov 14, 2024 at 06:11:46AM +0000, Chen, Jiqian wrote: > On 2024/11/13 18:30, Roger Pau Monné wrote: > > On Wed, Nov 13, 2024 at 10:00:33AM +0000, Chen, Jiqian wrote: > >> 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? > > > > Yes please, I have no way to test it. Let's see what others think > > about the different approaches. > There are some errors with your method. > I attached the dmesg and xl dmesg logs. > From the dmesg logs, it seems that 0000:03:00.0 has addresse overlap with 0000:03:00.1 Do you have the output of lspci with the BAR sizes/positions before and after the resizing? > > I think there is a place that needs to be modified regarding your method, > although this modification does not help with the above-mentioned errors, > it is that whether to support resizing is specific to which bar, rather than just determining whether there is a Rebar capability. Do we really need such fine-grained information? It should be harmless (even if not strictly necessary) to size all BARs on the device before enabling memory decoding, even if some of them do not support resizing. I might have to provide a patch with additional messages to see what's going on. Thanks, Roger.
On Thu, Nov 14, 2024 at 04:52:18PM +0100, Roger Pau Monné wrote: > On Thu, Nov 14, 2024 at 06:11:46AM +0000, Chen, Jiqian wrote: > > On 2024/11/13 18:30, Roger Pau Monné wrote: > > > On Wed, Nov 13, 2024 at 10:00:33AM +0000, Chen, Jiqian wrote: > > >> 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? > > > > > > Yes please, I have no way to test it. Let's see what others think > > > about the different approaches. > > There are some errors with your method. > > I attached the dmesg and xl dmesg logs. > > From the dmesg logs, it seems that 0000:03:00.0 has addresse overlap with 0000:03:00.1 > > Do you have the output of lspci with the BAR sizes/positions before > and after the resizing? > > > > > I think there is a place that needs to be modified regarding your method, > > although this modification does not help with the above-mentioned errors, > > it is that whether to support resizing is specific to which bar, rather than just determining whether there is a Rebar capability. > > Do we really need such fine-grained information? It should be > harmless (even if not strictly necessary) to size all BARs on the > device before enabling memory decoding, even if some of them do not > support resizing. > > I might have to provide a patch with additional messages to see what's > going on. One nit that I've noticed with the patch I gave you previously is that the check for a size change in modify_bars() should be done ahead of pci_check_bar(), otherwise the check is possibly using an outdated size. I've also added a debug message to notify when a BAR register is written and report the new address. This is done unconditionally, but if you think it's too chatty you can limit to only printing for the device that has the ReBAR capability. Thanks, Roger. diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c index ef6c965c081c..d49d3588993b 100644 --- a/xen/drivers/vpci/header.c +++ b/xen/drivers/vpci/header.c @@ -346,6 +346,30 @@ static int modify_bars(const struct pci_dev *pdev, uint16_t cmd, bool rom_only) bar->enabled == !!(cmd & PCI_COMMAND_MEMORY) ) continue; + 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); + } + } + if ( !pci_check_bar(pdev, _mfn(start), _mfn(end)) ) { printk(XENLOG_G_WARNING @@ -601,6 +625,9 @@ static void cf_check bar_write( /* Update guest address, so hardware domain BAR is identity mapped. */ bar->guest_addr = bar->addr; +gprintk(XENLOG_DEBUG, "%pp: updated BAR%zu address: %#lx\n", + &pdev->sbdf, bar - pdev->vpci->header.bars + hi, bar->addr); + /* Make sure Xen writes back the same value for the BAR RO bits. */ if ( !hi ) { @@ -870,6 +897,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;
On Thu, Nov 14, 2024 at 06:11:46AM +0000, Chen, Jiqian wrote: > On 2024/11/13 18:30, Roger Pau Monné wrote: > > On Wed, Nov 13, 2024 at 10:00:33AM +0000, Chen, Jiqian wrote: > >> 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? > > > > Yes please, I have no way to test it. Let's see what others think > > about the different approaches. > There are some errors with your method. > I attached the dmesg and xl dmesg logs. > From the dmesg logs, it seems that 0000:03:00.0 has addresse overlap with 0000:03:00.1 I'm attempting to go over Linux dmesg, and so far got the following information about the BAR positions for 0000:03:00.0 and 0000:03:00.1: [ 1.101522] pci 0000:03:00.0: [1002:73ff] type 00 class 0x030000 PCIe Legacy Endpoint [ 1.102416] pci 0000:03:00.0: BAR 0 [mem 0xfc90000000-0xfc9fffffff 64bit pref] [ 1.105592] pci 0000:03:00.0: BAR 2 [mem 0xfca0000000-0xfca01fffff 64bit pref] [ 1.106665] pci 0000:03:00.0: BAR 4 [io 0x2000-0x20ff] [ 1.109858] pci 0000:03:00.0: BAR 5 [mem 0xd0600000-0xd06fffff] [ 1.112962] pci 0000:03:00.0: ROM [mem 0xfffe0000-0xffffffff pref] [ 1.113543] pci 0000:03:00.0: PME# supported from D1 D2 D3hot D3cold [ 1.114031] pci 0000:03:00.0: 63.008 Gb/s available PCIe bandwidth, limited by 8.0 GT/s PCIe x8 link at 0000:00:01.1 (capable of 252.048 Gb/s with 16.0 GT/s PCIe x16 link) [ 1.115074] pci 0000:03:00.1: [1002:ab28] type 00 class 0x040300 PCIe Legacy Endpoint [ 1.115207] pci 0000:03:00.1: BAR 0 [mem 0xd0700000-0xd0703fff] [ 1.115962] pci 0000:03:00.1: PME# supported from D1 D2 D3hot D3cold [ 120.049678] amdgpu 0000:03:00.0: BAR 2 [mem 0xfca0000000-0xfca01fffff 64bit pref]: releasing [ 120.049682] amdgpu 0000:03:00.0: BAR 0 [mem 0xfc90000000-0xfc9fffffff 64bit pref]: releasing [ 120.049815] amdgpu 0000:03:00.0: BAR 0 [mem 0xa00000000-0xbffffffff 64bit pref]: assigned [ 120.049833] amdgpu 0000:03:00.0: BAR 2 [mem 0x900000000-0x9001fffff 64bit pref]: assigned So the only BAR from 0000:03:00.1 is always at 0xd0700000-0xd0703fff, and BARs for 0000:03:00.0 are relocated from: 0xfca0000000-0xfca01fffff -> 0x900000000-0x9001fffff (same size) 0xfc90000000-0xfc9fffffff -> 0xa00000000-0xbffffffff (resized) So I see no overlap between the BARs of 0000:03:00.0 and 0000:03:00.1, but maybe I'm missing some position changes, following Linux dmesg is not easy. Regards, Roger.
diff --git a/xen/drivers/vpci/Makefile b/xen/drivers/vpci/Makefile index 1a1413b93e76..a7c8a30a8956 100644 --- a/xen/drivers/vpci/Makefile +++ b/xen/drivers/vpci/Makefile @@ -1,2 +1,2 @@ -obj-y += vpci.o header.o +obj-y += vpci.o header.o rebar.o obj-$(CONFIG_HAS_PCI_MSI) += msi.o msix.o diff --git a/xen/drivers/vpci/rebar.c b/xen/drivers/vpci/rebar.c new file mode 100644 index 000000000000..84dbd84b0745 --- /dev/null +++ b/xen/drivers/vpci/rebar.c @@ -0,0 +1,89 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (C) 2024 Advanced Micro Devices, Inc. All Rights Reserved. + * + * Author: Jiqian Chen <Jiqian.Chen@amd.com> + */ + +#include <xen/hypercall.h> +#include <xen/vpci.h> + +/* + * The number value of the BAR Size in PCI_REBAR_CTRL register reprent: + * 0 1 MB (2^20 bytes) + * 1 2 MB (2^21 bytes) + * 2 4 MB (2^22 bytes) + * ... + * 43 8 EB (2^63 bytes) + */ +#define PCI_REBAR_CTRL_BAR_UNIT (1ULL << 20) + +static void cf_check rebar_ctrl_write(const struct pci_dev *pdev, + unsigned int reg, + uint32_t val, + void *data) +{ + uint32_t ctrl, index; + struct vpci_bar *bars = pdev->vpci->header.bars; + + ctrl = pci_conf_read32(pdev->sbdf, reg); + if ( ctrl == val ) + return; + + ctrl &= ~PCI_REBAR_CTRL_BAR_SIZE; + if ( ctrl != ( val & ~PCI_REBAR_CTRL_BAR_SIZE ) ) + return; + + index = ctrl & PCI_REBAR_CTRL_BAR_IDX; + bars[index].size = (1 << ((val & PCI_REBAR_CTRL_BAR_SIZE) >> + PCI_REBAR_CTRL_BAR_SHIFT)) * + PCI_REBAR_CTRL_BAR_UNIT; + + pci_conf_write32(pdev->sbdf, reg, val); +} + +static int cf_check init_rebar(struct pci_dev *pdev) +{ + unsigned int rebar_offset; + uint32_t ctrl, nbars; + int rc = 0; + + rebar_offset = pci_find_ext_capability(pdev->sbdf, PCI_EXT_CAP_ID_REBAR); + + if ( !rebar_offset ) + return rc; + + ctrl = pci_conf_read32(pdev->sbdf, rebar_offset + PCI_REBAR_CTRL); + nbars = (ctrl & PCI_REBAR_CTRL_NBAR_MASK) >> PCI_REBAR_CTRL_NBAR_SHIFT; + + for ( int i = 0; i < nbars; i++, rebar_offset += 8 ) { + rc = vpci_add_register(pdev->vpci, vpci_hw_read32, NULL, + rebar_offset + PCI_REBAR_CAP, 4, NULL); + if ( rc ) { + printk("%s: %pp: add register for PCI_REBAR_CAP failed (rc=%d)\n", + __func__, &pdev->sbdf, rc); + break; + } + + rc = vpci_add_register(pdev->vpci, vpci_hw_read32, rebar_ctrl_write, + rebar_offset + PCI_REBAR_CTRL, 4, NULL); + if ( rc ) { + printk("%s: %pp: add register for PCI_REBAR_CTRL failed (rc=%d)\n", + __func__, &pdev->sbdf, rc); + break; + } + } + + return rc; +} +REGISTER_VPCI_INIT(init_rebar, VPCI_PRIORITY_LOW); + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * tab-width: 4 + * indent-tabs-mode: nil + * End: + */ diff --git a/xen/include/xen/pci_regs.h b/xen/include/xen/pci_regs.h index 250ba106dbd3..5d2aa130916e 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 /* Resizable BAR */ /* Advanced Error Reporting */ #define PCI_ERR_UNCOR_STATUS 4 /* Uncorrectable Error Status */ @@ -541,6 +542,16 @@ #define PCI_VNDR_HEADER_REV(x) (((x) >> 16) & 0xf) #define PCI_VNDR_HEADER_LEN(x) (((x) >> 20) & 0xfff) +/* Resizable BARs */ +#define PCI_REBAR_CAP 4 /* capability register */ +#define PCI_REBAR_CAP_SIZES 0x00FFFFF0 /* supported BAR sizes */ +#define PCI_REBAR_CTRL 8 /* control register */ +#define PCI_REBAR_CTRL_BAR_IDX 0x00000007 /* BAR index */ +#define PCI_REBAR_CTRL_NBAR_MASK 0x000000E0 /* # of resizable BARs */ +#define PCI_REBAR_CTRL_NBAR_SHIFT 5 /* shift for # of BARs */ +#define PCI_REBAR_CTRL_BAR_SIZE 0x00001F00 /* BAR size */ +#define PCI_REBAR_CTRL_BAR_SHIFT 8 /* shift for BAR size */ + /* * Hypertransport sub capability types *
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. Signed-off-by: Jiqian Chen <Jiqian.Chen@amd.com> --- xen/drivers/vpci/Makefile | 2 +- xen/drivers/vpci/rebar.c | 89 ++++++++++++++++++++++++++++++++++++++ xen/include/xen/pci_regs.h | 11 +++++ 3 files changed, 101 insertions(+), 1 deletion(-) create mode 100644 xen/drivers/vpci/rebar.c