On Wed, Jan 28, 2026 at 7:30 AM Lancelot SIX <[email protected]> wrote:
>
> >>>>> + /*
> >>>>> + * Only control stack and save area size details checked.
> >>>>> Address validation needs to be
> >>>>> + * carried out separately.
> >>>>> + */
> >>>>> + if (cwsr_params->ctl_stack_sz !=
> >>>>> + (cwsr_info->xcc_ctl_stack_sz * num_xcc)) {
> >>>>> + dev_dbg(adev->dev,
> >>>>> + "queue ctl stack size 0x%x not equal to
> >>>>> node ctl stack size 0x%x\n",
> >>>>> + cwsr_params->ctl_stack_sz,
> >>>>> + num_xcc * cwsr_info->xcc_ctl_stack_sz);
> >>>>> + return -EINVAL;
> >>>>> + }
> >>>>> +
> >>>>> + if (cwsr_params->cwsr_sz < (cwsr_info->xcc_cwsr_sz *
> >>>>> num_xcc)) {
> >>>>> + dev_dbg(adev->dev,
> >>>>> + "queue cwsr size 0x%x not equal to node
> >>>>> cwsr size 0x%x\n",
> >>>>> + cwsr_params->cwsr_sz, num_xcc * cwsr_info-
> >>>>> >xcc_cwsr_sz);
> >>>>> + return -EINVAL;
> >>>>> + }
> >>
> >> cwsr_params->cwsr_sz has no upper bound check. Can this cause an
> >> overflow elsewhere?
> >>
> >
> > We could restrict to a max limit of 2 * cwsr size required. Adding
> > David/Lancelot as well.
> >
> > Thanks,
> > Lijo
> >
>
> Hi,
>
> The CWSR size should allow room for userspace to choose the amount
> allocated for use by the debugger. I am not sure what limit would make
> sense, as I can't really predict what will be needed in the future, but
> I really don't see how we could need more than the cwsr size (which
> itself can contain the entire state of what is running on the queue).
>
We can always make it bigger in the future if we need it. Unbounded
seems ripe for an overflow somewhere.
Alex
> Best,
> Lancelot.
>
> cc Jonathan/Laurent
>
> >> Alex
> >>
> >>
> >> Alex
> >>
> >>>>> +
> >>>>> + return 0;
> >>>>> +}
> >>>>> +
> >>>>> void amdgpu_cwsr_free(struct amdgpu_device *adev, struct
> >>>>> amdgpu_vm *vm,
> >>>>> struct amdgpu_cwsr_trap_obj **trap_obj)
> >>>>> {
> >>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cwsr.h b/drivers/
> >>>>> gpu/drm/amd/amdgpu/amdgpu_cwsr.h
> >>>>> index 3c80d057bbed..96b03a8ed99b 100644
> >>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cwsr.h
> >>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cwsr.h
> >>>>> @@ -56,6 +56,13 @@ struct amdgpu_cwsr_info {
> >>>>> uint32_t xcc_cwsr_sz;
> >>>>> };
> >>>>>
> >>>>> +struct amdgpu_cwsr_params {
> >>>>> + uint64_t ctx_save_area_address;
> >>>>> + /* cwsr size info */
> >>>>> + uint32_t ctl_stack_sz;
> >>>>> + uint32_t cwsr_sz;
> >>>>> +};
> >>>>> +
> >>>>> int amdgpu_cwsr_init(struct amdgpu_device *adev);
> >>>>> void amdgpu_cwsr_fini(struct amdgpu_device *adev);
> >>>>>
> >>>>> @@ -68,4 +75,8 @@ static inline bool amdgpu_cwsr_is_enabled(struct
> >>>>> amdgpu_device *adev)
> >>>>> return adev->cwsr_info != NULL;
> >>>>> }
> >>>>>
> >>>>> +uint32_t amdgpu_cwsr_size_needed(struct amdgpu_device *adev, int
> >>>>> num_xcc);
> >>>>> +int amdgpu_cwsr_validate_params(struct amdgpu_device *adev,
> >>>>> + struct amdgpu_cwsr_params
> >>>>> *cwsr_params,
> >>>>> + int num_xcc);
> >>>>> #endif
> >>>>> --
> >>>>> 2.49.0
> >>>>>
> >>>
> >
>
