Signed-off-by: Glenn Kennard glenn.kennard@gmail.com --- See patch sent to mesa-dev@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);
On Sat, Nov 08, 2014 at 11:51:30PM +0100, Glenn Kennard wrote:
Signed-off-by: Glenn Kennard glenn.kennard@gmail.com
NAK insecure. This is missing any kind of boundary checking for the indirect buffer and thus can be abuse.
See patch sent to mesa-dev@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] - Reserved4 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 HEADER2 DATA_OFFSET Bits [31:0] + byte aligned offset where the required data structure starts. Bits 1:0 are zero3 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@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
On Sun, 09 Nov 2014 00:15:49 +0100, Jerome Glisse j.glisse@gmail.com wrote:
On Sat, Nov 08, 2014 at 11:51:30PM +0100, Glenn Kennard wrote:
Signed-off-by: Glenn Kennard glenn.kennard@gmail.com
NAK insecure. This is missing any kind of boundary checking for the indirect buffer and thus can be abuse.
The indirect command buffer is a fixed format 16 or 20 bytes size, read by fixed function hardware, which only contains the following:
For PACKET3_DRAW_INDIRECT:
typedef struct { GLuint count; GLuint primCount; GLuint first; GLuint reservedMustBeZero; } DrawArraysIndirectCommand;
or PACKET3_DRAW_INDEX_INDIRECT:
typedef struct { GLuint count; GLuint primCount; GLuint firstIndex; GLint baseVertex; GLuint reservedMustBeZero; } DrawElementsIndirectCommand;
Please outline an example of how this would be abused.
/Glenn
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@gmail.com wrote:
Signed-off-by: Glenn Kennard glenn.kennard@gmail.com
See patch sent to mesa-dev@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] - Reserved4 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 HEADER2 DATA_OFFSET Bits [31:0] + byte aligned offset where the required data structure starts. Bits 1:0 are zero3 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@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Add the necessary set of commands to support OpenGL indirect draw calls on evergreen/cayman devices that do not have VM.
Signed-off-by: Glenn Kennard glenn.kennard@gmail.com --- Changes since patch V1: * Removed multi draw indirect, not used by current userspace which instead decomposes multi into separate draw indirect calls * Added validation to reject cs if indirect draw buffer is too small for address+offset * Removed useless calls to evergreen_cs_track_check * Reject attempt to call PACKET3_SET_BASE with other value than indirect draw buffer base for chips using VM.
I've left the index buffer logic unchanged, outside of scope for this feature, additional checking of that would be part of support for GL_ARB/KHR_robust_buffer_access_behavior.
drivers/gpu/drm/radeon/evergreen_cs.c | 76 +++++++++++++++++++++++++++++++++++ drivers/gpu/drm/radeon/radeon_drv.c | 3 +- 2 files changed, 78 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/radeon/evergreen_cs.c b/drivers/gpu/drm/radeon/evergreen_cs.c index 924b1b7..5152c1f 100644 --- a/drivers/gpu/drm/radeon/evergreen_cs.c +++ b/drivers/gpu/drm/radeon/evergreen_cs.c @@ -83,6 +83,7 @@ struct evergreen_cs_track { u32 htile_offset; u32 htile_surface; struct radeon_bo *htile_bo; + unsigned long indirect_draw_buffer_size; };
static u32 evergreen_cs_get_aray_mode(u32 tiling_flags) @@ -1896,6 +1897,14 @@ 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; + } + break; + } case PACKET3_DRAW_INDEX: { uint64_t offset; @@ -2006,6 +2015,67 @@ 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] + */ + if (pkt->count != 2) { + DRM_ERROR("bad SET_BASE\n"); + return -EINVAL; + } + + /* currently only supporting setting indirect draw buffer base address */ + if (idx_value != 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; + } + + track->indirect_draw_buffer_size = radeon_bo_size(reloc->robj); + + ib[idx+1] = reloc->gpu_offset; + ib[idx+2] = upper_32_bits(reloc->gpu_offset) & 0xff; + + break; + } + case PACKET3_DRAW_INDIRECT: + case PACKET3_DRAW_INDEX_INDIRECT: + { + u64 size = pkt->opcode == PACKET3_DRAW_INDIRECT ? 16 : 20; + + /* + 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; + } + + if (idx_value + size > track->indirect_draw_buffer_size) { + dev_warn(p->dev, "DRAW_INDIRECT buffer too small %llu + %llu > %lu\n", + idx_value, size, track->indirect_draw_buffer_size); + 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"); @@ -3243,7 +3313,13 @@ static int evergreen_vm_packet3_check(struct radeon_device *rdev,
switch (pkt->opcode) { case PACKET3_NOP: + break; case PACKET3_SET_BASE: + if (idx_value != 1) { + DRM_ERROR("bad SET_BASE"); + return -EINVAL; + } + break; case PACKET3_CLEAR_STATE: case PACKET3_INDEX_BUFFER_SIZE: case PACKET3_DISPATCH_DIRECT: diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c index 4f50fb0..5d684be 100644 --- a/drivers/gpu/drm/radeon/radeon_drv.c +++ b/drivers/gpu/drm/radeon/radeon_drv.c @@ -88,9 +88,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);
Reviewed-by: Marek Olšák marek.olsak@amd.com
Marek
On Sat, Dec 13, 2014 at 3:32 AM, Glenn Kennard glenn.kennard@gmail.com wrote:
Add the necessary set of commands to support OpenGL indirect draw calls on evergreen/cayman devices that do not have VM.
Signed-off-by: Glenn Kennard glenn.kennard@gmail.com
Changes since patch V1:
- Removed multi draw indirect, not used by current userspace which instead decomposes multi into separate draw indirect calls
- Added validation to reject cs if indirect draw buffer is too small for address+offset
- Removed useless calls to evergreen_cs_track_check
- Reject attempt to call PACKET3_SET_BASE with other value than indirect draw buffer base for chips using VM.
I've left the index buffer logic unchanged, outside of scope for this feature, additional checking of that would be part of support for GL_ARB/KHR_robust_buffer_access_behavior.
drivers/gpu/drm/radeon/evergreen_cs.c | 76 +++++++++++++++++++++++++++++++++++ drivers/gpu/drm/radeon/radeon_drv.c | 3 +- 2 files changed, 78 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/radeon/evergreen_cs.c b/drivers/gpu/drm/radeon/evergreen_cs.c index 924b1b7..5152c1f 100644 --- a/drivers/gpu/drm/radeon/evergreen_cs.c +++ b/drivers/gpu/drm/radeon/evergreen_cs.c @@ -83,6 +83,7 @@ struct evergreen_cs_track { u32 htile_offset; u32 htile_surface; struct radeon_bo *htile_bo;
unsigned long indirect_draw_buffer_size;};
static u32 evergreen_cs_get_aray_mode(u32 tiling_flags) @@ -1896,6 +1897,14 @@ 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;}break;} case PACKET3_DRAW_INDEX: { uint64_t offset;@@ -2006,6 +2015,67 @@ 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] - Reserved4 ADDRESS_HI Bits [31:8] - Reserved. Bits [7:0] - Upper bits of Address [47:32]*/if (pkt->count != 2) {DRM_ERROR("bad SET_BASE\n");return -EINVAL;}/* currently only supporting setting indirect draw buffer base address */if (idx_value != 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;}track->indirect_draw_buffer_size = radeon_bo_size(reloc->robj);ib[idx+1] = reloc->gpu_offset;ib[idx+2] = upper_32_bits(reloc->gpu_offset) & 0xff;break;}case PACKET3_DRAW_INDIRECT:case PACKET3_DRAW_INDEX_INDIRECT:{u64 size = pkt->opcode == PACKET3_DRAW_INDIRECT ? 16 : 20;/*DW 1 HEADER2 DATA_OFFSET Bits [31:0] + byte aligned offset where the required data structure starts. Bits 1:0 are zero3 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;}if (idx_value + size > track->indirect_draw_buffer_size) {dev_warn(p->dev, "DRAW_INDIRECT buffer too small %llu + %llu > %lu\n",idx_value, size, track->indirect_draw_buffer_size);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");@@ -3243,7 +3313,13 @@ static int evergreen_vm_packet3_check(struct radeon_device *rdev,
switch (pkt->opcode) { case PACKET3_NOP:
break; case PACKET3_SET_BASE:if (idx_value != 1) {DRM_ERROR("bad SET_BASE");return -EINVAL;}break; case PACKET3_CLEAR_STATE: case PACKET3_INDEX_BUFFER_SIZE: case PACKET3_DISPATCH_DIRECT:diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c index 4f50fb0..5d684be 100644 --- a/drivers/gpu/drm/radeon/radeon_drv.c +++ b/drivers/gpu/drm/radeon/radeon_drv.c @@ -88,9 +88,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@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
On Mon, Dec 15, 2014 at 6:44 AM, Marek Olšák maraeo@gmail.com wrote:
Reviewed-by: Marek Olšák marek.olsak@amd.com
Applied to my -next tree.
Thanks!
Alex
Marek
On Sat, Dec 13, 2014 at 3:32 AM, Glenn Kennard glenn.kennard@gmail.com wrote:
Add the necessary set of commands to support OpenGL indirect draw calls on evergreen/cayman devices that do not have VM.
Signed-off-by: Glenn Kennard glenn.kennard@gmail.com
Changes since patch V1:
- Removed multi draw indirect, not used by current userspace which instead decomposes multi into separate draw indirect calls
- Added validation to reject cs if indirect draw buffer is too small for address+offset
- Removed useless calls to evergreen_cs_track_check
- Reject attempt to call PACKET3_SET_BASE with other value than indirect draw buffer base for chips using VM.
I've left the index buffer logic unchanged, outside of scope for this feature, additional checking of that would be part of support for GL_ARB/KHR_robust_buffer_access_behavior.
drivers/gpu/drm/radeon/evergreen_cs.c | 76 +++++++++++++++++++++++++++++++++++ drivers/gpu/drm/radeon/radeon_drv.c | 3 +- 2 files changed, 78 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/radeon/evergreen_cs.c b/drivers/gpu/drm/radeon/evergreen_cs.c index 924b1b7..5152c1f 100644 --- a/drivers/gpu/drm/radeon/evergreen_cs.c +++ b/drivers/gpu/drm/radeon/evergreen_cs.c @@ -83,6 +83,7 @@ struct evergreen_cs_track { u32 htile_offset; u32 htile_surface; struct radeon_bo *htile_bo;
unsigned long indirect_draw_buffer_size;};
static u32 evergreen_cs_get_aray_mode(u32 tiling_flags) @@ -1896,6 +1897,14 @@ 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;}break;} case PACKET3_DRAW_INDEX: { uint64_t offset;@@ -2006,6 +2015,67 @@ 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] - Reserved4 ADDRESS_HI Bits [31:8] - Reserved. Bits [7:0] - Upper bits of Address [47:32]*/if (pkt->count != 2) {DRM_ERROR("bad SET_BASE\n");return -EINVAL;}/* currently only supporting setting indirect draw buffer base address */if (idx_value != 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;}track->indirect_draw_buffer_size = radeon_bo_size(reloc->robj);ib[idx+1] = reloc->gpu_offset;ib[idx+2] = upper_32_bits(reloc->gpu_offset) & 0xff;break;}case PACKET3_DRAW_INDIRECT:case PACKET3_DRAW_INDEX_INDIRECT:{u64 size = pkt->opcode == PACKET3_DRAW_INDIRECT ? 16 : 20;/*DW 1 HEADER2 DATA_OFFSET Bits [31:0] + byte aligned offset where the required data structure starts. Bits 1:0 are zero3 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;}if (idx_value + size > track->indirect_draw_buffer_size) {dev_warn(p->dev, "DRAW_INDIRECT buffer too small %llu + %llu > %lu\n",idx_value, size, track->indirect_draw_buffer_size);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");@@ -3243,7 +3313,13 @@ static int evergreen_vm_packet3_check(struct radeon_device *rdev,
switch (pkt->opcode) { case PACKET3_NOP:
break; case PACKET3_SET_BASE:if (idx_value != 1) {DRM_ERROR("bad SET_BASE");return -EINVAL;}break; case PACKET3_CLEAR_STATE: case PACKET3_INDEX_BUFFER_SIZE: case PACKET3_DISPATCH_DIRECT:diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c index 4f50fb0..5d684be 100644 --- a/drivers/gpu/drm/radeon/radeon_drv.c +++ b/drivers/gpu/drm/radeon/radeon_drv.c @@ -88,9 +88,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@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel@lists.freedesktop.org
-
Alex Deucher -
Glenn Kennard -
Jerome Glisse -
Marek Olšák