Re: [PATCH RFC 09/13] drm/powervr_sgx: Add the client command checks

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] drm/powervr_sgx: missing PDS row extents for bg_pds addresses
  in render
--

commit 2c01add46bb408f66c7c7d092a2528c557b38dee
Author: Joseph Telaak <joseph@telaak.dev>

drm/powervr_sgx: Add the client command checks

This commit adds client command copying, extension chain walking, per-field
validation tables, VM mapping checks, and cross-field rules for PowerVR SGX
commands.

> diff --git a/drivers/gpu/drm/powervr_sgx/pvrsgx_cmd_defs.c b/drivers/gpu/drm/powervr_sgx/pvrsgx_cmd_defs.c
> new file mode 100644
> index 0000000000000..f73c80199b527
> --- /dev/null
> +++ b/drivers/gpu/drm/powervr_sgx/pvrsgx_cmd_defs.c

[ ... ]

> +	{
> +		.name = "pixel_event_pds_addr",
> +		.offset = offsetof(struct pvrsgx_cmd_render_all, base.pixel_event_pds_addr),
> +		.size = 8,
> +		.check = PVRSGX_CMD_CHECK_ADDR,
> +		.heap = PVRSGX_CMD_HEAP_PDS_PIXEL,
> +		.align = 16,
> +		.extent = PVRSGX_CMD_EXTENT_PDS_ROWS,
> +		.extent_field = offsetof(struct pvrsgx_cmd_render_all, base.pixel_event_pds_rows),
> +	},
> +	{
> +		.name = "bg_pds_secondary_addr",
> +		.offset = offsetof(struct pvrsgx_cmd_render_all, base.bg_pds_secondary_addr),
> +		.size = 8,
> +		.check = PVRSGX_CMD_CHECK_ADDR,
> +		.heap = PVRSGX_CMD_HEAP_PDS_PIXEL,
> +		.align = 16,
> +	},
> +	{
> +		.name = "bg_pds_primary_addr",
> +		.offset = offsetof(struct pvrsgx_cmd_render_all, base.bg_pds_primary_addr),
> +		.size = 8,
> +		.check = PVRSGX_CMD_CHECK_ADDR,
> +		.heap = PVRSGX_CMD_HEAP_PDS_PIXEL,
> +		.align = 16,
> +	},

[Severity: Medium]
Should bg_pds_secondary_addr and bg_pds_primary_addr in sgx545_render_fields
also set .extent = PVRSGX_CMD_EXTENT_PDS_ROWS with .extent_field pointing to
base.bg_pds_secondary_rows and base.bg_pds_primary_rows?

mapped_fields() skips address fields where f->extent is 0, so
pvrsgx_cmd_check_mapped() never checks that these background PDS program
extents are mapped in the job's VM.  A render command with an unmapped or
partially mapped background PDS address in the PDS_PIXEL heap will pass
validation and trigger a GPU page fault when executed.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112-powervr-sgx-v1-0-c69b84bac011@telaak.dev?part=9