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.



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

Reply via email to