Re: [PATCH v2 1/1] vpci: Add resizable bar support

[Jan Beulich <jbeulich@xxxxxxxx>]

On 11.12.2024 11:19, Chen, Jiqian wrote:
> On 2024/12/11 16:53, Jan Beulich wrote:
>> On 11.12.2024 09:43, Roger Pau Monné wrote:
>>> On Wed, Dec 11, 2024 at 09:25:16AM +0100, Jan Beulich wrote:
>>>> On 11.12.2024 08:57, Chen, Jiqian wrote:
>>>>> On 2024/12/10 19:25, Roger Pau Monné wrote:
>>>>>> So you suggest that the capability should be hidden in that case?  We
>>>>>> have logic to hide capabilities, just not used for the hardware
>>>>>> domain.  It would need some extra wiring to be capable of hiding
>>>>>> failed capabilities.
>>>>> Can you give me a guidance on how to hide a failed capability?
>>>>> What codes are current logic to hide capabilities?
>>>>> Then maybe I can add a patch to implement it.
>>>>
>>>> It's really the other way around right now for "normal" capabilities:
>>>> We whitelist what we expose. See init_header()'s logic after checking
>>>> the PCI_STATUS_CAP_LIST bit. Actually past that block there's
>>>>
>>>>         /* Extended capabilities read as zero, write ignore */
>>>>         rc = vpci_add_register(pdev->vpci, vpci_read_val, NULL, 0x100, 4,
>>>>                                (void *)0);
>>>>
>>>> I.e. no extended capabilities are exposed at all right now to DomU-s.
>>>> For Dom0 I guess we shouldn't use whitelisting, but the (extended)
>>>> capability list(s) would need similarly virtualizing to be able to
>>>> hide some.
>>>
>>> Given this capability is only to be exposed to the hw domain (at least
>>> for now), I'm not sure it's fair to ask to add all this
>>> infrastructure as part of adding the new capability.  It would be great
>>> to have it, but doesn't seem fair when there's already MSI and MSI-X
>>> implemented without such support.
>>
>> Well, of course this can also be modeled after MSI/MSI-X, failing
>> assignment when initialization for a capability fails. Yet while for
>> MSI / MSI-X this feels okay-ish (considering that many devices now
>> can't even operate very well without either of the two), I'd expect
>> BAR resizing to not be something that drivers (typically) rely on.
>> "Typically" because iirc Jiqian said the AMD display driver actually
>> does.
> You mean what I said in last version?

Yes.

Jan

> It is "amdgpu driver saves and restores the same pci state during 
> initiazation without disabling memory decoding, that caused Roger's method 
> not work".
> And currently running amdgpu on Xen hypervisor has two scenarios, 
> 1. APU does not rely on ReBar capability, because APU's vram are all CPU 
> accessible.
> 2. But for discrete GPU, they can't work based on current Xen codes, it needs 
> resize Bar to make all vram CPU accessible. That is why I sent this patch to 
> add Rebar support in Xen.
> 
>>
>> Jan
>