[AMD Official Use Only - AMD Internal Distribution Only]
Review-by: Emily Deng <Emily.Deng@xxxxxxx>
>-----Original Message-----
>From: Li, Yunxiang (Teddy) <Yunxiang.Li@xxxxxxx>
>Sent: Friday, May 31, 2024 5:48 AM
>To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx
>Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Koenig, Christian
><Christian.Koenig@xxxxxxx>; Li, Yunxiang (Teddy) <Yunxiang.Li@xxxxxxx>;
>Chang, HaiJun <HaiJun.Chang@xxxxxxx>; Deng, Emily
><Emily.Deng@xxxxxxx>
>Subject: [PATCH v3 2/8] drm/amdgpu: fix sriov host flr handler
>
>We send back the ready to reset message before we stop anything. This is
>wrong. Move it to when we are actually ready for the FLR to happen.
>
>In the current state since we take tens of seconds to stop everything, it is
>very likely that host would give up waiting and reset the GPU before we send
>ready, so it would be the same as before. But this gets rid of the hack with
>reset_domain locking and also let us know how slow the reset actually is on
>the host. The pre-reset speed can thus be improved later.
>
>Signed-off-by: Yunxiang Li <Yunxiang.Li@xxxxxxx>
>---
>v3: still call amdgpu_virt_fini_data_exchange right away, it could take
> awhile for the reset to grab it's lock and call this function in
> pre_reset so during this time the thread will read garbage.
>
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 ++
> drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 14 ++++++++
> drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h | 2 ++
> drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c | 39 +++++++++-------------
> drivers/gpu/drm/amd/amdgpu/mxgpu_nv.c | 39 +++++++++-------------
> drivers/gpu/drm/amd/amdgpu/mxgpu_vi.c | 6 ----
> 6 files changed, 50 insertions(+), 52 deletions(-)
>
>diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>index bf1a6593dc5e..eb77b4ec3cb4 100644
>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>@@ -5069,6 +5069,8 @@ static int amdgpu_device_reset_sriov(struct
>amdgpu_device *adev,
> struct amdgpu_hive_info *hive = NULL;
>
> if (test_bit(AMDGPU_HOST_FLR, &reset_context->flags)) {
>+ amdgpu_virt_ready_to_reset(adev);
>+ amdgpu_virt_wait_reset(adev);
> clear_bit(AMDGPU_HOST_FLR, &reset_context->flags);
> r = amdgpu_virt_request_full_gpu(adev, true);
> } else {
>diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
>b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
>index 3cf8416f8cb0..44450507c140 100644
>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
>@@ -152,6 +152,20 @@ void amdgpu_virt_request_init_data(struct
>amdgpu_device *adev)
> DRM_WARN("host doesn't support REQ_INIT_DATA
>handshake\n"); }
>
>+/**
>+ * amdgpu_virt_ready_to_reset() - send ready to reset to host
>+ * @adev: amdgpu device.
>+ * Send ready to reset message to GPU hypervisor to signal we have
>+stopped GPU
>+ * activity and is ready for host FLR
>+ */
>+void amdgpu_virt_ready_to_reset(struct amdgpu_device *adev) {
>+ struct amdgpu_virt *virt = &adev->virt;
>+
>+ if (virt->ops && virt->ops->reset_gpu)
>+ virt->ops->ready_to_reset(adev);
>+}
>+
> /**
> * amdgpu_virt_wait_reset() - wait for reset gpu completed
> * @adev: amdgpu device.
>diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
>b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
>index 642f1fd287d8..66de5380d9a1 100644
>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
>@@ -88,6 +88,7 @@ struct amdgpu_virt_ops {
> int (*rel_full_gpu)(struct amdgpu_device *adev, bool init);
> int (*req_init_data)(struct amdgpu_device *adev);
> int (*reset_gpu)(struct amdgpu_device *adev);
>+ void (*ready_to_reset)(struct amdgpu_device *adev);
> int (*wait_reset)(struct amdgpu_device *adev);
> void (*trans_msg)(struct amdgpu_device *adev, enum idh_request
>req,
> u32 data1, u32 data2, u32 data3);
>@@ -345,6 +346,7 @@ int amdgpu_virt_request_full_gpu(struct
>amdgpu_device *adev, bool init); int amdgpu_virt_release_full_gpu(struct
>amdgpu_device *adev, bool init); int amdgpu_virt_reset_gpu(struct
>amdgpu_device *adev); void amdgpu_virt_request_init_data(struct
>amdgpu_device *adev);
>+void amdgpu_virt_ready_to_reset(struct amdgpu_device *adev);
> int amdgpu_virt_wait_reset(struct amdgpu_device *adev); int
>amdgpu_virt_alloc_mm_table(struct amdgpu_device *adev); void
>amdgpu_virt_free_mm_table(struct amdgpu_device *adev); diff --git
>a/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c
>b/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c
>index f4c47492e0cd..6b71ee85ee65 100644
>--- a/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c
>+++ b/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c
>@@ -249,38 +249,30 @@ static int xgpu_ai_set_mailbox_ack_irq(struct
>amdgpu_device *adev,
> return 0;
> }
>
>-static void xgpu_ai_mailbox_flr_work(struct work_struct *work)
>+static void xgpu_ai_ready_to_reset(struct amdgpu_device *adev)
> {
>- struct amdgpu_virt *virt = container_of(work, struct amdgpu_virt,
>flr_work);
>- struct amdgpu_device *adev = container_of(virt, struct
>amdgpu_device, virt);
>- int timeout = AI_MAILBOX_POLL_FLR_TIMEDOUT;
>-
>- /* block amdgpu_gpu_recover till msg FLR COMPLETE received,
>- * otherwise the mailbox msg will be ruined/reseted by
>- * the VF FLR.
>- */
>- if (atomic_cmpxchg(&adev->reset_domain->in_gpu_reset, 0, 1) != 0)
>- return;
>-
>- down_write(&adev->reset_domain->sem);
>-
>- amdgpu_virt_fini_data_exchange(adev);
>-
> xgpu_ai_mailbox_trans_msg(adev, IDH_READY_TO_RESET, 0, 0, 0);
>+}
>
>+static int xgpu_ai_wait_reset(struct amdgpu_device *adev) {
>+ int timeout = AI_MAILBOX_POLL_FLR_TIMEDOUT;
> do {
> if (xgpu_ai_mailbox_peek_msg(adev) ==
>IDH_FLR_NOTIFICATION_CMPL)
>- goto flr_done;
>-
>+ return 0;
> msleep(10);
> timeout -= 10;
> } while (timeout > 1);
>-
> dev_warn(adev->dev, "waiting IDH_FLR_NOTIFICATION_CMPL
>timeout\n");
>+ return -ETIME;
>+}
>
>-flr_done:
>- atomic_set(&adev->reset_domain->in_gpu_reset, 0);
>- up_write(&adev->reset_domain->sem);
>+static void xgpu_ai_mailbox_flr_work(struct work_struct *work) {
>+ struct amdgpu_virt *virt = container_of(work, struct amdgpu_virt,
>flr_work);
>+ struct amdgpu_device *adev = container_of(virt, struct
>amdgpu_device,
>+virt);
>+
>+ amdgpu_virt_fini_data_exchange(adev);
>
> /* Trigger recovery for world switch failure if no TDR */
> if (amdgpu_device_should_recover_gpu(adev)
>@@ -417,7 +409,8 @@ const struct amdgpu_virt_ops xgpu_ai_virt_ops = {
> .req_full_gpu = xgpu_ai_request_full_gpu_access,
> .rel_full_gpu = xgpu_ai_release_full_gpu_access,
> .reset_gpu = xgpu_ai_request_reset,
>- .wait_reset = NULL,
>+ .ready_to_reset = xgpu_ai_ready_to_reset,
>+ .wait_reset = xgpu_ai_wait_reset,
> .trans_msg = xgpu_ai_mailbox_trans_msg,
> .req_init_data = xgpu_ai_request_init_data,
> .ras_poison_handler = xgpu_ai_ras_poison_handler, diff --git
>a/drivers/gpu/drm/amd/amdgpu/mxgpu_nv.c
>b/drivers/gpu/drm/amd/amdgpu/mxgpu_nv.c
>index 37b49a5ed2a1..22af30a15a5f 100644
>--- a/drivers/gpu/drm/amd/amdgpu/mxgpu_nv.c
>+++ b/drivers/gpu/drm/amd/amdgpu/mxgpu_nv.c
>@@ -282,38 +282,30 @@ static int xgpu_nv_set_mailbox_ack_irq(struct
>amdgpu_device *adev,
> return 0;
> }
>
>-static void xgpu_nv_mailbox_flr_work(struct work_struct *work)
>+static void xgpu_nv_ready_to_reset(struct amdgpu_device *adev)
> {
>- struct amdgpu_virt *virt = container_of(work, struct amdgpu_virt,
>flr_work);
>- struct amdgpu_device *adev = container_of(virt, struct
>amdgpu_device, virt);
>- int timeout = NV_MAILBOX_POLL_FLR_TIMEDOUT;
>-
>- /* block amdgpu_gpu_recover till msg FLR COMPLETE received,
>- * otherwise the mailbox msg will be ruined/reseted by
>- * the VF FLR.
>- */
>- if (atomic_cmpxchg(&adev->reset_domain->in_gpu_reset, 0, 1) != 0)
>- return;
>-
>- down_write(&adev->reset_domain->sem);
>-
>- amdgpu_virt_fini_data_exchange(adev);
>-
> xgpu_nv_mailbox_trans_msg(adev, IDH_READY_TO_RESET, 0, 0, 0);
>+}
>
>+static int xgpu_nv_wait_reset(struct amdgpu_device *adev) {
>+ int timeout = NV_MAILBOX_POLL_FLR_TIMEDOUT;
> do {
> if (xgpu_nv_mailbox_peek_msg(adev) ==
>IDH_FLR_NOTIFICATION_CMPL)
>- goto flr_done;
>-
>+ return 0;
> msleep(10);
> timeout -= 10;
> } while (timeout > 1);
>-
> dev_warn(adev->dev, "waiting IDH_FLR_NOTIFICATION_CMPL
>timeout\n");
>+ return -ETIME;
>+}
>
>-flr_done:
>- atomic_set(&adev->reset_domain->in_gpu_reset, 0);
>- up_write(&adev->reset_domain->sem);
>+static void xgpu_nv_mailbox_flr_work(struct work_struct *work) {
>+ struct amdgpu_virt *virt = container_of(work, struct amdgpu_virt,
>flr_work);
>+ struct amdgpu_device *adev = container_of(virt, struct
>amdgpu_device,
>+virt);
>+
>+ amdgpu_virt_fini_data_exchange(adev);
>
> /* Trigger recovery for world switch failure if no TDR */
> if (amdgpu_device_should_recover_gpu(adev)
>@@ -455,7 +447,8 @@ const struct amdgpu_virt_ops xgpu_nv_virt_ops = {
> .rel_full_gpu = xgpu_nv_release_full_gpu_access,
> .req_init_data = xgpu_nv_request_init_data,
> .reset_gpu = xgpu_nv_request_reset,
>- .wait_reset = NULL,
>+ .ready_to_reset = xgpu_nv_ready_to_reset,
>+ .wait_reset = xgpu_nv_wait_reset,
> .trans_msg = xgpu_nv_mailbox_trans_msg,
> .ras_poison_handler = xgpu_nv_ras_poison_handler, }; diff --git
>a/drivers/gpu/drm/amd/amdgpu/mxgpu_vi.c
>b/drivers/gpu/drm/amd/amdgpu/mxgpu_vi.c
>index 78cd07744ebe..e1d63bed84bf 100644
>--- a/drivers/gpu/drm/amd/amdgpu/mxgpu_vi.c
>+++ b/drivers/gpu/drm/amd/amdgpu/mxgpu_vi.c
>@@ -515,12 +515,6 @@ static void xgpu_vi_mailbox_flr_work(struct
>work_struct *work)
> struct amdgpu_virt *virt = container_of(work, struct amdgpu_virt,
>flr_work);
> struct amdgpu_device *adev = container_of(virt, struct
>amdgpu_device, virt);
>
>- /* wait until RCV_MSG become 3 */
>- if (xgpu_vi_poll_msg(adev, IDH_FLR_NOTIFICATION_CMPL)) {
>- pr_err("failed to receive FLR_CMPL\n");
>- return;
>- }
>-
> /* Trigger recovery due to world switch failure */
> if (amdgpu_device_should_recover_gpu(adev)) {
> struct amdgpu_reset_context reset_context;
>--
>2.34.1
