AMD General Hi, Lijo.
Yes, the current GIM driver ensures the early init data is placed within the VF’s visible BAR0 aperture. Thanks, Chong. -----Original Message----- From: Lazar, Lijo <[email protected]> Sent: Thursday, July 9, 2026 3:56 PM To: Li, Chong(Alan) <[email protected]>; Koenig, Christian <[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 6:12 PM, Li, Chong(Alan) wrote: > 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 > Yes, I saw that one. One other question - does host driver guarantee that discovery data will always be copied to visible BAR aperture of VF? If not, it will still require mmindex/data regardless of FB access enablement. Thanks, Lijo > > 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; >>>>>> >>>>> >>>> >>> >> >
