* Re: [PATCH] drm/radeon: evergreen/cayman indirect draw support
2014-11-08 22:51 [PATCH] drm/radeon: evergreen/cayman indirect draw support Glenn Kennard
@ 2014-11-08 23:15 ` Jerome Glisse
2014-11-08 23:50 ` Glenn Kennard
2014-11-09 12:19 ` Marek Olšák
2014-12-13 2:32 ` [PATCH v2] " Glenn Kennard
2 siblings, 1 reply; 7+ messages in thread
From: Jerome Glisse @ 2014-11-08 23:15 UTC (permalink / raw)
To: Glenn Kennard; +Cc: dri-devel
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] - 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@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
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] drm/radeon: evergreen/cayman indirect draw support
2014-11-08 23:15 ` Jerome Glisse
@ 2014-11-08 23:50 ` Glenn Kennard
0 siblings, 0 replies; 7+ messages in thread
From: Glenn Kennard @ 2014-11-08 23:50 UTC (permalink / raw)
To: Jerome Glisse, 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/radeon: evergreen/cayman indirect draw support
2014-11-08 22:51 [PATCH] drm/radeon: evergreen/cayman indirect draw support Glenn Kennard
2014-11-08 23:15 ` Jerome Glisse
@ 2014-11-09 12:19 ` Marek Olšák
2014-12-13 2:32 ` [PATCH v2] " Glenn Kennard
2 siblings, 0 replies; 7+ messages in thread
From: Marek Olšák @ 2014-11-09 12:19 UTC (permalink / raw)
To: Glenn Kennard; +Cc: dri-devel
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] - 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@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
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH v2] drm/radeon: evergreen/cayman indirect draw support
2014-11-08 22:51 [PATCH] drm/radeon: evergreen/cayman indirect draw support Glenn Kennard
2014-11-08 23:15 ` Jerome Glisse
2014-11-09 12:19 ` Marek Olšák
@ 2014-12-13 2:32 ` Glenn Kennard
2014-12-15 11:44 ` Marek Olšák
2 siblings, 1 reply; 7+ messages in thread
From: Glenn Kennard @ 2014-12-13 2:32 UTC (permalink / raw)
To: 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);
--
1.9.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v2] drm/radeon: evergreen/cayman indirect draw support
2014-12-13 2:32 ` [PATCH v2] " Glenn Kennard
@ 2014-12-15 11:44 ` Marek Olšák
2014-12-15 15:59 ` Alex Deucher
0 siblings, 1 reply; 7+ messages in thread
From: Marek Olšák @ 2014-12-15 11:44 UTC (permalink / raw)
To: Glenn Kennard; +Cc: dri-devel
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] - 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);
> --
> 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
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2] drm/radeon: evergreen/cayman indirect draw support
2014-12-15 11:44 ` Marek Olšák
@ 2014-12-15 15:59 ` Alex Deucher
0 siblings, 0 replies; 7+ messages in thread
From: Alex Deucher @ 2014-12-15 15:59 UTC (permalink / raw)
To: Marek Olšák; +Cc: 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] - 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);
>> --
>> 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 mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 7+ messages in thread