[PATCH] drm/radeon: evergreen/cayman indirect draw support
Marek Olšák
maraeo at gmail.com
Sun Nov 9 04:19:31 PST 2014
Hi Glenn,
First of all, evergreen_cs_track_check only has to be called for the
packets that execute a draw call. It's useless to call it for
INDEX_BASE, INDEX_BUFFER_SIZE, and SET_BASE.
There has never been index buffer bounds checking in the CS checker
and this patch doesn't change that, so no argument there.
There should however be at least indirect buffer bounds checking. You
can save any state before draw packets in struct evergreen_cs_track,
then you can use that at a draw packet to check if all accesses are
within bounds of the indirect buffer.
Since GPU memory isn't protected very well, setting an invalid offset
can read data that doesn't belong to the process.
Marek
On Sat, Nov 8, 2014 at 11:51 PM, Glenn Kennard <glenn.kennard at gmail.com> wrote:
> Signed-off-by: Glenn Kennard <glenn.kennard at gmail.com>
> ---
> See patch sent to mesa-dev at lists.freedesktop.org for userspace usage.
>
> drivers/gpu/drm/radeon/evergreen_cs.c | 76 +++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/radeon/evergreend.h | 1 +
> drivers/gpu/drm/radeon/radeon_drv.c | 3 +-
> 3 files changed, 79 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/radeon/evergreen_cs.c b/drivers/gpu/drm/radeon/evergreen_cs.c
> index 5c8b358..6173df3 100644
> --- a/drivers/gpu/drm/radeon/evergreen_cs.c
> +++ b/drivers/gpu/drm/radeon/evergreen_cs.c
> @@ -1896,6 +1896,19 @@ static int evergreen_packet3_check(struct radeon_cs_parser *p,
> }
> break;
> }
> + case PACKET3_INDEX_BUFFER_SIZE:
> + {
> + if (pkt->count != 0) {
> + DRM_ERROR("bad INDEX_BUFFER_SIZE\n");
> + return -EINVAL;
> + }
> + r = evergreen_cs_track_check(p);
> + if (r) {
> + dev_warn(p->dev, "%s:%d invalid cmd stream\n", __func__, __LINE__);
> + return r;
> + }
> + break;
> + }
> case PACKET3_DRAW_INDEX:
> {
> uint64_t offset;
> @@ -2006,6 +2019,68 @@ static int evergreen_packet3_check(struct radeon_cs_parser *p,
> return r;
> }
> break;
> + case PACKET3_SET_BASE:
> + {
> + /*
> + DW 1 HEADER Header of the packet. Shader_Type in bit 1 of the Header will correspond to the shader type of the Load, see Type-3 Packet.
> + 2 BASE_INDEX Bits [3:0] BASE_INDEX - Base Index specifies which base address is specified in the last two DWs.
> + 0001: DX11 Draw_Index_Indirect Patch Table Base: Base address for Draw_Index_Indirect data.
> + 3 ADDRESS_LO Bits [31:3] - Lower bits of QWORD-Aligned Address. Bits [2:0] - Reserved
> + 4 ADDRESS_HI Bits [31:8] - Reserved. Bits [7:0] - Upper bits of Address [47:32]
> + */
> + uint64_t offset;
> +
> + if (pkt->count != 2) {
> + DRM_ERROR("bad SET_BASE\n");
> + return -EINVAL;
> + }
> +
> + /* currently only supporting setting indirect draw buffer base address */
> + if (radeon_get_ib_value(p, idx) != 1) {
> + DRM_ERROR("bad SET_BASE\n");
> + return -EINVAL;
> + }
> +
> + r = radeon_cs_packet_next_reloc(p, &reloc, 0);
> + if (r) {
> + DRM_ERROR("bad SET_BASE\n");
> + return -EINVAL;
> + }
> +
> + offset = reloc->gpu_offset +
> + radeon_get_ib_value(p, idx+1) +
> + ((u64)(radeon_get_ib_value(p, idx+2) & 0xff) << 32);
> +
> + ib[idx+1] = offset;
> + ib[idx+2] = upper_32_bits(offset) & 0xff;
> +
> + r = evergreen_cs_track_check(p);
> + if (r) {
> + dev_warn(p->dev, "%s:%d invalid cmd stream\n", __func__, __LINE__);
> + return r;
> + }
> + break;
> + }
> + case PACKET3_DRAW_INDIRECT:
> + case PACKET3_DRAW_INDEX_INDIRECT:
> + case PACKET3_DRAW_INDIRECT_MULTI:
> + {
> + /*
> + DW 1 HEADER
> + 2 DATA_OFFSET Bits [31:0] + byte aligned offset where the required data structure starts. Bits 1:0 are zero
> + 3 DRAW_INITIATOR Draw Initiator Register. Written to the VGT_DRAW_INITIATOR register for the assigned context
> + */
> + if (pkt->count != 1) {
> + DRM_ERROR("bad DRAW_INDIRECT\n");
> + return -EINVAL;
> + }
> + r = evergreen_cs_track_check(p);
> + if (r) {
> + dev_warn(p->dev, "%s:%d invalid cmd stream\n", __func__, __LINE__);
> + return r;
> + }
> + break;
> + }
> case PACKET3_DISPATCH_DIRECT:
> if (pkt->count != 3) {
> DRM_ERROR("bad DISPATCH_DIRECT\n");
> @@ -3260,6 +3335,7 @@ static int evergreen_vm_packet3_check(struct radeon_device *rdev,
> case PACKET3_DRAW_INDEX_OFFSET:
> case PACKET3_INDEX_TYPE:
> case PACKET3_DRAW_INDEX:
> + case PACKET3_DRAW_INDIRECT_MULTI:
> case PACKET3_DRAW_INDEX_AUTO:
> case PACKET3_DRAW_INDEX_IMMD:
> case PACKET3_NUM_INSTANCES:
> diff --git a/drivers/gpu/drm/radeon/evergreend.h b/drivers/gpu/drm/radeon/evergreend.h
> index b066d67..5d4c5f2 100644
> --- a/drivers/gpu/drm/radeon/evergreend.h
> +++ b/drivers/gpu/drm/radeon/evergreend.h
> @@ -1553,6 +1553,7 @@
> #define PACKET3_DRAW_INDEX_OFFSET 0x29
> #define PACKET3_INDEX_TYPE 0x2A
> #define PACKET3_DRAW_INDEX 0x2B
> +#define PACKET3_DRAW_INDIRECT_MULTI 0x2C
> #define PACKET3_DRAW_INDEX_AUTO 0x2D
> #define PACKET3_DRAW_INDEX_IMMD 0x2E
> #define PACKET3_NUM_INSTANCES 0x2F
> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
> index dcffa30..fbd1f73 100644
> --- a/drivers/gpu/drm/radeon/radeon_drv.c
> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> @@ -86,9 +86,10 @@
> * 2.39.0 - Add INFO query for number of active CUs
> * 2.40.0 - Add RADEON_GEM_GTT_WC/UC, flush HDP cache before submitting
> * CS to GPU on >= r600
> + * 2.41.0 - evergreen/cayman: Add SET_BASE/DRAW_INDIRECT command parsing support
> */
> #define KMS_DRIVER_MAJOR 2
> -#define KMS_DRIVER_MINOR 40
> +#define KMS_DRIVER_MINOR 41
> #define KMS_DRIVER_PATCHLEVEL 0
> int radeon_driver_load_kms(struct drm_device *dev, unsigned long flags);
> int radeon_driver_unload_kms(struct drm_device *dev);
> --
> 1.9.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
More information about the dri-devel
mailing list