On 13.08.25 20:49, David Francis wrote:
> AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE is a flag that
> specifies that gem memory contains sensitive information and
> should be cleared to prevent snooping.
>
> The COHERENT and UNCACHED gem create flags enable memory
> features related to sharing memory across devices.
>
> These should be settable in GEM_CREATE_IOCTL but weren't.
That needs further explanation. Something like:
For CRIU we need to re-create KFD BOs through the GEM CREATE IOCTL, so allow
those KFD specific flags here as well. This will also aid us in the future and
allows to move the KFD components over using the render node for allocations.
>
> Make a new define AMDGPU_GEM_CREATE_SETTABLE_MASK to
> track which gem flags can be used with gem create, and add
> these flags to it.
This describes what the patch does which should be obvious by reading the
patch. I would drop it.
>
> Signed-off-by: David Francis <[email protected]>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 10 +---------
> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.h | 14 ++++++++++++++
> 2 files changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index e3f65977eeee..aefae3a9e6f4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -442,15 +442,7 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, void
> *data,
> int r;
>
> /* reject invalid gem flags */
> - if (flags & ~(AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED |
> - AMDGPU_GEM_CREATE_NO_CPU_ACCESS |
> - AMDGPU_GEM_CREATE_CPU_GTT_USWC |
> - AMDGPU_GEM_CREATE_VRAM_CLEARED |
> - AMDGPU_GEM_CREATE_VM_ALWAYS_VALID |
> - AMDGPU_GEM_CREATE_EXPLICIT_SYNC |
> - AMDGPU_GEM_CREATE_ENCRYPTED |
> - AMDGPU_GEM_CREATE_GFX12_DCC |
> - AMDGPU_GEM_CREATE_DISCARDABLE))
> + if (flags & ~AMDGPU_GEM_CREATE_SETTABLE_MASK)
> return -EINVAL;
>
> /* reject invalid gem domains */
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.h
> index b51e8f95ee86..b3047d73fe07 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.h
> @@ -71,4 +71,18 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data,
> int amdgpu_gem_metadata_ioctl(struct drm_device *dev, void *data,
> struct drm_file *filp);
>
A comment explaining what this does would be nice to have.
Apart from that looks good to me,
Christian.
> +#define AMDGPU_GEM_CREATE_SETTABLE_MASK
> (AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED | \
> + AMDGPU_GEM_CREATE_NO_CPU_ACCESS | \
> + AMDGPU_GEM_CREATE_CPU_GTT_USWC | \
> + AMDGPU_GEM_CREATE_VRAM_CLEARED | \
> + AMDGPU_GEM_CREATE_VM_ALWAYS_VALID | \
> + AMDGPU_GEM_CREATE_EXPLICIT_SYNC | \
> + AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE | \
> + AMDGPU_GEM_CREATE_ENCRYPTED | \
> + AMDGPU_GEM_CREATE_GFX12_DCC | \
> + AMDGPU_GEM_CREATE_DISCARDABLE | \
> + AMDGPU_GEM_CREATE_COHERENT | \
> + AMDGPU_GEM_CREATE_UNCACHED | \
> + AMDGPU_GEM_CREATE_EXT_COHERENT)
> +
> #endif
