On 09-Jul-26 4:05 PM, 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.



Not just for current, is this also applicable for old host driver? Otherwise, this breaks on old host driver.

Thanks,
Lijo

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;








Reply via email to