On 5/8/25 07:09, Samuel Zhang wrote:
> For virtual machine with vGPUs in SRIOV single device mode and XGMI
> is enabled, XGMI physical node ids may change when waking up from
> hiberation with different vGPU devices. So update XGMI info on resume.
>
> Signed-off-by: Jiang Liu <[email protected]>
> Signed-off-by: Samuel Zhang <[email protected]>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 24 ++++++++++++++++++++++
> drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h | 4 ++++
> drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 6 ++++++
> 3 files changed, 34 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index d477a901af84..843a3b0a9a07 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -4478,6 +4478,7 @@ int amdgpu_device_init(struct amdgpu_device *adev,
> r = adev->gfxhub.funcs->get_xgmi_info(adev);
> if (r)
> return r;
> + adev->gmc.xgmi.prev_physical_node_id =
> adev->gmc.xgmi.physical_node_id;
> }
>
> /* enable PCIE atomic ops */
> @@ -5040,6 +5041,26 @@ int amdgpu_device_suspend(struct drm_device *dev, bool
> notify_clients)
> return 0;
> }
>
> +static int amdgpu_device_update_xgmi_info(struct amdgpu_device *adev)
> +{
> + int r;
> +
> + /* Get xgmi info again for sriov to detect device changes */
> + if (amdgpu_sriov_vf(adev) &&
> + !(adev->flags & AMD_IS_APU) &&
> + adev->gmc.xgmi.supported &&
> + !adev->gmc.xgmi.connected_to_cpu) {
> + adev->gmc.xgmi.prev_physical_node_id =
> adev->gmc.xgmi.physical_node_id;
> + r = adev->gfxhub.funcs->get_xgmi_info(adev);
> + if (r)
> + return r;
> +
> + dev_info(adev->dev, "xgmi node, old id %d, new id %d\n",
> + adev->gmc.xgmi.prev_physical_node_id,
> adev->gmc.xgmi.physical_node_id);
> + }
> + return 0;
> +}
> +
> /**
> * amdgpu_device_resume - initiate device resume
> *
> @@ -5059,6 +5080,9 @@ int amdgpu_device_resume(struct drm_device *dev, bool
> notify_clients)
> r = amdgpu_virt_request_full_gpu(adev, true);
> if (r)
> return r;
> + r = amdgpu_device_update_xgmi_info(adev);
> + if (r)
> + return r;
> }
>
> if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
> index 32dabba4062f..1387901576f1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
> @@ -89,6 +89,7 @@ struct amdgpu_xgmi {
> u64 node_segment_size;
> /* physical node (0-3) */
> unsigned physical_node_id;
> + unsigned prev_physical_node_id;
> /* number of nodes (0-4) */
> unsigned num_physical_nodes;
> /* gpu list in the same hive */
> @@ -101,6 +102,9 @@ struct amdgpu_xgmi {
> uint8_t max_width;
> };
>
> +#define amdgpu_xmgi_is_node_changed(adev) \
> + (adev->gmc.xgmi.prev_physical_node_id !=
> adev->gmc.xgmi.physical_node_id)
Please drop that function and the related checks.
If this is necessary we want to update the relevant parameters all the time and
not just when something changed.
Regards,
Christian.
> +
> struct amdgpu_hive_info *amdgpu_get_xgmi_hive(struct amdgpu_device *adev);
> void amdgpu_put_xgmi_hive(struct amdgpu_hive_info *hive);
> int amdgpu_xgmi_update_topology(struct amdgpu_hive_info *hive, struct
> amdgpu_device *adev);
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index 59385da80185..7c0ca2721eb3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -2533,6 +2533,12 @@ static int gmc_v9_0_resume(struct amdgpu_ip_block
> *ip_block)
> struct amdgpu_device *adev = ip_block->adev;
> int r;
>
> + if (amdgpu_xmgi_is_node_changed(adev)) {
> + adev->vm_manager.vram_base_offset =
> adev->gfxhub.funcs->get_mc_fb_offset(adev);
> + adev->vm_manager.vram_base_offset +=
> + adev->gmc.xgmi.physical_node_id *
> adev->gmc.xgmi.node_segment_size;
> + }
> +
> /* If a reset is done for NPS mode switch, read the memory range
> * information again.
> */
