AMD General
>-----Original Message-----
>From: Koenig, Christian <[email protected]>
>Sent: Wednesday, July 8, 2026 6:49 PM
>To: Li, Chong(Alan) <[email protected]>; Lazar, Lijo <[email protected]>; amd-
>[email protected]
>Cc: Deng, Emily <[email protected]>; Chang, HaiJun
><[email protected]>; Skvortsov, Victor <[email protected]>; Cursor
><[email protected]>
>Subject: Re: [PATCH] drm/amdgpu: improve the amdgpu device init progress in
>sriov
>mode
>
>Hi Chong,
>
>On 7/8/26 12:25, Li, Chong(Alan) wrote:
>> AMD General
>>
>> Hi, Christian.
>>
>> The code " early_full_gpu_access = (adev->virt.req_init_data_ver == 0);" is
>> related
>to the ASIC type, not the host platform:
>>
>>
>> adev->virt.req_init_data_ver is initialized by
>> amdgpu_virt_request_init_data().
>>
>> Some legacy ASICs, such as CHIP_VEGA20 and CHIP_ALDEBARAN, do not
>send amdgpu_virt_request_init_data() to the host.
>>
>> For those ASICs, the host dumps the early init data only after the guest
>> requests full
>GPU access.
>> Therefore, those ASICs still need to request full GPU access before the
>> guest driver
>can read the early init data from the VF FB.
>
>No, exactly that is not the case as far as I can see. See the commit I
>mentioned below.
>
>According to that starting from SIENNA_CICHLID the full GPU access is mandatory
>for reading the discovery table.
>
>So I absolutely clearly have to reject your patch here.
>
>As far as I can see this is a hack for ESXi and that is not going to fly.
>
>Regards,
>Christian.
Hi Christian, that patch " drm/amdgpu: invoke req full access early enough " is
not correct since FB access right has already been granted by the init data
event. Maybe that patch was only intended for the initial SIENNA_CICHLID
bring-up.
Emily Deng
Best Wishes
>>
>>
>>
>> static bool amdgpu_virt_init_req_data(struct amdgpu_device *adev,
>> u32 reg)
>> {
>> ...
>> case CHIP_VEGA20:
>> case CHIP_ARCTURUS:
>> case CHIP_ALDEBARAN:
>> soc15_set_virt_ops(adev);
>> break;
>> case CHIP_NAVI10:
>> case CHIP_NAVI12:
>> case CHIP_SIENNA_CICHLID:
>> case CHIP_IP_DISCOVERY:
>> nv_set_virt_ops(adev);
>> /* try send GPU_INIT_DATA request to host */
>> amdgpu_virt_request_init_data(adev);
>> break;
>> ...
>> }
>>
>> Thanks,
>> Chong.
>>
>> -----Original Message-----
>> From: Koenig, Christian <[email protected]>
>> Sent: Wednesday, July 8, 2026 6:24 PM
>> To: Lazar, Lijo <[email protected]>; Li, Chong(Alan)
>> <[email protected]>; [email protected]
>> Cc: Deng, Emily <[email protected]>; Chang, HaiJun
>> <[email protected]>; Skvortsov, Victor <[email protected]>;
>> Cursor <[email protected]>
>> Subject: Re: [PATCH] drm/amdgpu: improve the amdgpu device init
>> progress in sriov mode
>>
>> On 7/8/26 12:03, Lazar, Lijo wrote:
>>>
>>>
>>> On 08-Jul-26 3:23 PM, Christian König wrote:
>>>> On 7/8/26 11:35, Li, Chong(Alan) wrote:
>>>>> AMD General
>>>>>
>>>>> Hi, Christian.
>>>>>
>>>>> This version of the patch is not host-platform dependent; all host
>>>>> platforms follow
>the same path.
>>>>>
>>>>> I verified this patch with KVM in my local environment and with ESXi on a
>borrowed server.
>>>>>
>>>>> Distinguishing the host platform was a mistake, and I have
>>>>> explained the situation at the end of the patch email.
>>>>>
>>>>> I have pasted the content below:
>>>>>
>>>>> Hi, Christian and Lijo.
>>>>>
>>>>> Sorry, I made a mistake.
>>>>> Host access to the VF FB is not platform-dependent. After the
>>>>> host driver
>sets VF_FB_EN in response to GPU_INIT_DATA, the host can access the VF FB.
>>>>>
>>>>> I borrowed an ESXi server and tested this patch there.
>>>>> The amdgpu driver works normally.
>>>>>
>>>>> I rewrote the background. Is anything still unclear?
>>>>
>>>> You still have this check here in the code: "early_full_gpu_access = (adev-
>>virt.req_init_data_ver == 0);".
>>>>
>>>> As far as I can see that is Hypervisor specific and a NO-GO.
>>>>
>>>
>>> I think this should be seen as a policy set by host driver on when a guest
>>> operation
>is allowed. Host driver could do it based on other conditions also, not
>specific to
>hypervisor environment.
>>
>> No, this was intentionally changed in 2020 because of a new feature. See this
>commit here:
>>
>> commit 00a979f3d69e0c275e88c741b854dbe0d5238ae0
>> Author: Wenhui Sheng <[email protected]>
>> Date: Tue Jun 23 13:43:49 2020 +0800
>>
>> drm/amdgpu: invoke req full access early enough
>>
>> From SIENNA_CICHLID, HW introduce a new protection
>> feature which can control the FB, doorbell and MMIO
>> write access for VF, so guest driver should request
>> full access before ip discovery, or we couldn't access
>> ip discovery data in FB.
>>
>> Signed-off-by: Wenhui Sheng <[email protected]>
>> Reviewed-by: Hawking Zhang <[email protected]>
>> Signed-off-by: Alex Deucher <[email protected]>
>>
>> So as far as I can see this change here will break older hypervisor versions
>> and that
>is absolutely clear reason to NAK it.
>>
>> Regards,
>> Christian.
>>
>>>
>>> Thanks,
>>> Lijo
>>>
>>>> Regards,
>>>> Christian.
>>>>
>>>>>
>>>>> Thanks,
>>>>> Chong.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> -----Original Message-----
>>>>> From: Koenig, Christian <[email protected]>
>>>>> Sent: Wednesday, July 8, 2026 5:05 PM
>>>>> To: Li, Chong(Alan) <[email protected]>;
>>>>> [email protected]
>>>>> Cc: Deng, Emily <[email protected]>; Chang, HaiJun
>>>>> <[email protected]>; Skvortsov, Victor
>>>>> <[email protected]>; Lazar, Lijo <[email protected]>;
>>>>> Cursor <[email protected]>
>>>>> Subject: Re: [PATCH] drm/amdgpu: improve the amdgpu device init
>>>>> progress in sriov mode
>>>>>
>>>>> On 7/8/26 10:25, chong li wrote:
>>>>>> v2:
>>>>>> Some legacy ASICs do not send amdgpu_virt_request_init_data().
>>>>>> Only keep the full GPU access request early when request_init_data is not
>sent.
>>>>>>
>>>>>> v1:
>>>>>> Move the initialization of non-GPU resources out of the full GPU
>>>>>> access region during AMDGPU device initialization.
>>>>>>
>>>>>> Background:
>>>>>> In SR-IOV, the guest sends GPU_INIT_DATA, then the host enables
>>>>>> VF_FB_EN and places early initialization data, such as IP
>>>>>> discovery, VBIOS, and PF-VF exchange data, in the VF FB. The guest
>>>>>> should then be able to read this data before requesting full GPU access.
>>>>>>
>>>>>> Before this patch, the VF still requested full GPU access in
>>>>>> amdgpu_device_ip_early_init(). At that point TTM is not
>>>>>> initialized yet, so the normal VRAM aperture mapping is
>>>>>> unavailable and the guest falls back to MM_INDEX/MM_DATA register
>>>>>> access. That register path requires full GPU access.
>>>>>>
>>>>>> Use the BAR0 framebuffer read path,
>>>>>> amdgpu_device_read_fb_via_bar0(), for the early init-data copy
>>>>>> instead of MM_INDEX/MM_DATA. This lets the driver delay the full
>>>>>> GPU access request until after the early init data has been copied.
>>>>>
>>>>> That looks like it goes into the right direction, but as far as I can see
>>>>> it is still an
>ESXi specific change.
>>>>>
>>>>> So once more: We can't adjust the driver to the hypervisor!
>>>>>
>>>>> Either this works on all hypervisors or I have to reject the change.
>>>>>
>>>>> Regards,
>>>>> Christian.
>>>>>
>>>>>>
>>>>>> Signed-off-by: chong li <[email protected]>
>>>>>> Co-authored-by: Cursor <[email protected]>
>>>>>> ---
>>>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 21
>>>>>> +++++++++++++++++----
>>>>>> drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c | 4 ++--
>>>>>> 2 files changed, 19 insertions(+), 6 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>>>> index 610d82b79de3..ac66796e8634 100644
>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>>>> @@ -38,6 +38,10 @@
>>>>>> #include <linux/apple-gmux.h>
>>>>>> #include <linux/nospec.h>
>>>>>>
>>>>>> +#ifdef CONFIG_X86
>>>>>> +#include <asm/hypervisor.h>
>>>>>> +#endif
>>>>>> +
>>>>>> #include <drm/drm_atomic_helper.h>
>>>>>> #include <drm/drm_client_event.h>
>>>>>> #include <drm/drm_crtc_helper.h> @@ -1987,16 +1991,19 @@ static
>>>>>> int amdgpu_device_ip_early_init(struct
>>>>>> amdgpu_device *adev) {
>>>>>> struct amdgpu_ip_block *ip_block;
>>>>>> struct pci_dev *parent;
>>>>>> - bool total, skip_bios;
>>>>>> + bool total, skip_bios, early_full_gpu_access = false;
>>>>>> uint32_t bios_flags;
>>>>>> int i, r;
>>>>>>
>>>>>> amdgpu_device_enable_virtual_display(adev);
>>>>>>
>>>>>> if (amdgpu_sriov_vf(adev)) {
>>>>>> - r = amdgpu_virt_request_full_gpu(adev, true);
>>>>>> - if (r)
>>>>>> - return r;
>>>>>> + early_full_gpu_access =
>>>>>> +(adev->virt.req_init_data_ver == 0);
>>>>>> + if (early_full_gpu_access) {
>>>>>> + r = amdgpu_virt_request_full_gpu(adev, true);
>>>>>> + if (r)
>>>>>> + return r;
>>>>>> + }
>>>>>>
>>>>>> r = amdgpu_virt_init_critical_region(adev);
>>>>>> if (r)
>>>>>> @@ -2159,6 +2166,12 @@ static int
>>>>>> amdgpu_device_ip_early_init(struct amdgpu_device *adev)
>>>>>> if (!total)
>>>>>> return -ENODEV;
>>>>>>
>>>>>> + if (amdgpu_sriov_vf(adev) && !early_full_gpu_access) {
>>>>>> + r = amdgpu_virt_request_full_gpu(adev, true);
>>>>>> + if (r)
>>>>>> + return r;
>>>>>> + }
>>>>>> +
>>>>>> if (adev->gmc.xgmi.supported)
>>>>>> amdgpu_xgmi_early_init(adev);
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c
>>>>>> b/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c
>>>>>> index 9a40107a0869..340703d89d6b 100644
>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c
>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c
>>>>>> @@ -185,8 +185,8 @@ static int xgpu_ai_send_access_requests(struct
>>>>>> amdgpu_device *adev,
>>>>>> } else if (req == IDH_REQ_GPU_INIT_DATA){
>>>>>> /* Dummy REQ_GPU_INIT_DATA handling */
>>>>>> r = xgpu_ai_poll_msg(adev,
>>>>>> IDH_REQ_GPU_INIT_DATA_READY);
>>>>>> - /* version set to 0 since dummy */
>>>>>> - adev->virt.req_init_data_ver = 0;
>>>>>> + /* Version is set to 1 since GPU_CRIT_REGION_V1 */
>>>>>> + adev->virt.req_init_data_ver = GPU_CRIT_REGION_V1;
>>>>>> }
>>>>>>
>>>>>> return 0;
>>>>>
>>>>
>>>
>>
