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;
