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
