kfd_criu_restore_queue() takes the control stack size of a restored
queue from the checkpoint image, without the check create-queue does in
kfd_queue_acquire_buffers(). On gfx9 with CWSR, that size, per XCC,
sizes the MQD buffer in allocate_mqd() and is the ceiling
get_wave_state() uses to bound its copy of the control stack. A value of
0xFFFFFFFF reaches a kmalloc() above KMALLOC_MAX_SIZE and trips a page
allocator warning. Without CWSR the MQD comes from the GTT
sub-allocator, sized for the MQD alone, yet restore_mqd() still copies
the image's control stack one page after it.
Take the per-XCC control stack size from topology instead, as
create-queue requires, and 0 for queues without a control stack
(non-compute, or CWSR disabled). Reject an image that carries more
control stack than that per XCC.
The MQD in the image keeps its own cp_hqd_cntl_stack_size and
cp_hqd_cntl_stack_offset, which CWSR uses to place the control stack
after the MQD. Set the size to the allocated size in the gfx9
restore_mqd(), as init_mqd() does, and keep the offset within it, so
CWSR cannot access past the control stack of the new allocation.
Fixes: 3a9822d7bd62 ("drm/amdkfd: CRIU checkpoint and restore queue control
stack")
Signed-off-by: William Palacek <[email protected]>
---
.../gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c | 3 ++
.../amd/amdkfd/kfd_process_queue_manager.c | 33 ++++++++++++++-----
2 files changed, 27 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
index 6e6bc1ec0b64..c02dd9abc3ad 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
@@ -462,6 +462,9 @@ static void restore_mqd(struct mqd_manager *mm, void **mqd,
addr = mqd_mem_obj->gpu_addr;
memcpy(m, mqd_src, sizeof(*m));
+ /* CWSR must not access past the control stack of this allocation */
+ m->cp_hqd_cntl_stack_size = qp->ctl_stack_size;
+ m->cp_hqd_cntl_stack_offset = min(m->cp_hqd_cntl_stack_offset,
qp->ctl_stack_size);
*mqd = m;
if (gart_addr)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
index a15a826519ad..b41494775767 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
@@ -26,6 +26,7 @@
#include <linux/list.h>
#include "kfd_device_queue_manager.h"
#include "kfd_priv.h"
+#include "kfd_topology.h"
#include "kfd_kernel_queue.h"
#include "amdgpu_amdkfd.h"
#include "amdgpu_reset.h"
@@ -983,7 +984,7 @@ int kfd_criu_checkpoint_queues(struct kfd_process *p,
}
static void set_queue_properties_from_criu(struct queue_properties *qp,
- struct kfd_criu_queue_priv_data
*q_data, uint32_t num_xcc)
+ struct kfd_criu_queue_priv_data
*q_data)
{
qp->is_interop = false;
qp->queue_percent = q_data->q_percent;
@@ -996,11 +997,6 @@ static void set_queue_properties_from_criu(struct
queue_properties *qp,
qp->eop_ring_buffer_size = q_data->eop_ring_buffer_size;
qp->ctx_save_restore_area_address =
q_data->ctx_save_restore_area_address;
qp->ctx_save_restore_area_size = q_data->ctx_save_restore_area_size;
- if (q_data->type == KFD_QUEUE_TYPE_COMPUTE)
- qp->ctl_stack_size = q_data->ctl_stack_size / num_xcc;
- else
- qp->ctl_stack_size = q_data->ctl_stack_size;
-
qp->type = q_data->type;
qp->format = q_data->format;
}
@@ -1012,6 +1008,7 @@ int kfd_criu_restore_queue(struct kfd_process *p,
{
uint8_t *mqd, *ctl_stack, *q_extra_data = NULL;
struct kfd_criu_queue_priv_data *q_data;
+ struct kfd_topology_device *topo_dev;
struct kfd_process_device *pdd;
uint64_t q_extra_data_size;
struct queue_properties qp;
@@ -1048,6 +1045,27 @@ int kfd_criu_restore_queue(struct kfd_process *p,
goto exit;
}
+ memset(&qp, 0, sizeof(qp));
+ set_queue_properties_from_criu(&qp, q_data);
+
+ /*
+ * Size the control stack as create-queue does, not from the image.
+ * Only compute queues with CWSR have one, one per XCC.
+ */
+ if (qp.type == KFD_QUEUE_TYPE_COMPUTE && pdd->dev->kfd->cwsr_enabled) {
+ topo_dev = kfd_topology_device_by_id(pdd->dev->id);
+ if (!topo_dev) {
+ ret = -EINVAL;
+ goto exit;
+ }
+ qp.ctl_stack_size = topo_dev->node_props.ctl_stack_size;
+ }
+
+ if (q_data->ctl_stack_size > (u64)qp.ctl_stack_size *
NUM_XCC(pdd->dev->xcc_mask)) {
+ ret = -EINVAL;
+ goto exit;
+ }
+
*priv_data_offset += sizeof(*q_data);
q_extra_data_size = (uint64_t)q_data->ctl_stack_size + q_data->mqd_size;
@@ -1077,9 +1095,6 @@ int kfd_criu_restore_queue(struct kfd_process *p,
mqd = q_extra_data;
ctl_stack = mqd + q_data->mqd_size;
- memset(&qp, 0, sizeof(qp));
- set_queue_properties_from_criu(&qp, q_data,
NUM_XCC(pdd->dev->xcc_mask));
-
print_queue_properties(&qp);
ret = pqm_create_queue(&p->pqm, pdd->dev, &qp, &queue_id, q_data, mqd,
ctl_stack, NULL);
--
2.43.0
