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

["Chen, Jiqian" <Jiqian.Chen@xxxxxxx>]

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? 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

-- 
Best regards,
Jiqian Chen.