Hi Chong, On 7/10/26 09:00, Li, Chong(Alan) wrote: > AMD General > > Hi, Christian and Alex. > > > "If the ESXi hypervisor has a problem with this then that needs to be handled > on the hypervisor side." > > > This patch is not intended to solve the issue in "ESXi hypervisor". > > This patch aims to reduce the full GPU access time on newer ASICs and speed > up the GPU initialization flow. > > When multiple VFs are initialized, moving the full GPU access request later > shortens the full-access window > and reduces the time other VFs spend waiting for exclusive access.
Well that is at least finally a full explanation for the change. But it makes it clear that this is basically just an optimization and not functional required. > This is also the reason the REQUEST_INIT_DATA flow was introduced, > allow the guest to obtain the early init data without relying on full GPU > access. Yeah and that change was never discussed with Alex or me on the design part. I'm now systematically rejecting anything which hasn't been akwnoledged at least a couple of month pre-hand before the implementation. So this patch here is rejected, end of discussion. Regards, Christian. > > > > > Thanks, > Chong. > > -----Original Message----- > From: Li, Chong(Alan) > Sent: Friday, July 10, 2026 12:06 PM > To: Koenig, Christian <[email protected]>; Deucher, Alexander > <[email protected]>; Lazar, Lijo <[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 > > Hi, Christian and Alex. > > I do not think this patch conflicts with the original design. > The design principle is the same: full GPU access should be limited to the > phase where the guest actually needs exclusive access to hardware resources. > > For the legacy ASIC(like VEGA20 or ALDEBARAN about 6 years ago), the guest > does not send REQUEST_INIT_DATA. > The host prepares the init data only as part of the REQUEST_GPU_INIT flow, so > requesting full GPU access before ip discovery is necessary and should be > preserved. > > For newer ASICs that support REQUEST_INIT_DATA, the ordering is different. > The guest first requests the early init data, and the host replies > INIT_DATA_READY after discovery/VBIOS/PF2VF data has been copied to the > VF-visible FB region. > > In this flow, the guest copies the early init data from the VF-visible FB > region and completes IP early init before accessing hardware resources. > It then requests full GPU access only for the hardware initialization phase. > > So this patch is not changing the legacy design. > It keeps the old behavior when REQUEST_INIT_DATA is not supported, and only > moves full GPU access later for the newer REQUEST_INIT_DATA flow. > > > > > Thanks, > Chong. > > > > > > > -----Original Message----- > From: Koenig, Christian <[email protected]> > Sent: Friday, July 10, 2026 1:32 AM > To: Li, Chong(Alan) <[email protected]>; Lazar, Lijo <[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 > > Hi all, > > I've discussed the issue with Alex today. > > The current approach of keeping full GPU access while discovery has been > intentionally added this way over 6 years ago after some back and forth > discussion. > > So that has been working reliable for quite a while and we don't see any > justification to change that. > > If the ESXi hypervisor has a problem with this then that needs to be handled > on the hypervisor side. > > This patch is rejected. > > Regards, > Christian. > > On 7/9/26 12:35, Li, Chong(Alan) wrote: >> 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; >>>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >> >
