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