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
