AMD General Hi, Lijo.
" In this commit, the statement is different." " It says FB access is enabled if init_data request is sent." Yes, VF FB access is enabled if init_data request is sent. However, without full GPU access, the guest cannot access the MM_INDEX/MM_DATA registers. With the legacy/existing read path, VF_FB_EN must be enabled so the host can dump early init data into the VF FB, and full GPU access is required so the guest can copy that data through the MM_INDEX/MM_DATA registers. I submitted patch " [PATCH 1/2] drm/amdgpu: read FB through BAR0 when aperture is unavailable " , With the new amdgpu_device_read_fb_via_bar0() path, the guest can read the VF FB without full GPU access Thanks, Chong. -----Original Message----- From: Lazar, Lijo <[email protected]> Sent: Wednesday, July 8, 2026 7:18 PM To: Koenig, Christian <[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 08-Jul-26 4:18 PM, Christian König wrote: > 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. > It could also be because the legacy/existing way used mmindex/data, and that required full access including mmio register access. In this commit, the statement is different. "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" It says FB access is enabled if init_data request is sent. Thanks Lijo > 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. > >> >> >> >> 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; >>>>> >>>> >>> >> >
