From 07b31b2204f83afbcfbe04a1899de344adafe9e7 Mon Sep 17 00:00:00 2001 From: Boris Brezillon Date: Tue, 3 Sep 2024 18:09:56 +0200 Subject: [PATCH] pan/cs: Add a facility to restrict register access If the CS register file is used in a stateful mode, some of the registers will survive the RUN_xxx boundary, and even serve as a context for subsequent RUN_xxx calls. Having a way to restrict register access outside specific sections of code is a handy debug feature, so let's expose the necessary bits to allow that. It takes the form of a reg permission callback that gets called every time a register serves are a source or destination, to make sure this is allowed in this context. Signed-off-by: Boris Brezillon Reviewed-by: Rebecca Mckeever Reviewed-by: Mary Guillemard Reviewed-by: Lars-Ivar Hesselberg Simonsen Reviewed-by: John Anthony Acked-by: Erik Faye-Lund Part-of: --- src/panfrost/lib/genxml/cs_builder.h | 53 +++++++++++++++++++++------- 1 file changed, 41 insertions(+), 12 deletions(-) diff --git a/src/panfrost/lib/genxml/cs_builder.h b/src/panfrost/lib/genxml/cs_builder.h index 463510ad96a..4868459f93f 100644 --- a/src/panfrost/lib/genxml/cs_builder.h +++ b/src/panfrost/lib/genxml/cs_builder.h @@ -68,6 +68,17 @@ struct cs_load_store_tracker { uint8_t sb_slot; }; +enum cs_reg_perm { + CS_REG_NO_ACCESS = 0, + CS_REG_RD = BITFIELD_BIT(1), + CS_REG_WR = BITFIELD_BIT(2), + CS_REG_RW = CS_REG_RD | CS_REG_WR, +}; + +struct cs_builder; + +typedef enum cs_reg_perm (*reg_perm_cb_t)(struct cs_builder *b, unsigned reg); + struct cs_builder_conf { /* Number of 32-bit registers in the hardware register file */ uint8_t nr_registers; @@ -81,6 +92,9 @@ struct cs_builder_conf { /* Optional load/store tracker. */ struct cs_load_store_tracker *ls_tracker; + /* Optional register access checker. */ + reg_perm_cb_t reg_perm; + /* Cookie passed back to alloc_buffer() */ void *cookie; }; @@ -291,15 +305,22 @@ cs_to_reg_tuple(struct cs_index idx, ASSERTED unsigned expected_size) static inline unsigned cs_src_tuple(struct cs_builder *b, struct cs_index src, ASSERTED unsigned count) { - struct cs_load_store_tracker *ls_tracker = b->conf.ls_tracker; unsigned reg = cs_to_reg_tuple(src, count); - if (likely(!ls_tracker)) - return reg; + if (unlikely(b->conf.reg_perm)) { + for (unsigned i = reg; i < reg + count; i++) { + assert((b->conf.reg_perm(b, i) & CS_REG_RD) || + !"Trying to read a restricted register"); + } + } - for (unsigned i = reg; i < reg + count; i++) { - if (BITSET_TEST(ls_tracker->pending_loads, i)) - assert(!"register used as a source before flushing loads\n"); + struct cs_load_store_tracker *ls_tracker = b->conf.ls_tracker; + + if (unlikely(ls_tracker)) { + for (unsigned i = reg; i < reg + count; i++) { + if (BITSET_TEST(ls_tracker->pending_loads, i)) + assert(!"register used as a source before flushing loads\n"); + } } return reg; @@ -320,15 +341,23 @@ cs_src64(struct cs_builder *b, struct cs_index src) static inline unsigned cs_dst_tuple(struct cs_builder *b, struct cs_index dst, ASSERTED unsigned count) { - struct cs_load_store_tracker *ls_tracker = b->conf.ls_tracker; unsigned reg = cs_to_reg_tuple(dst, count); - if (likely(!ls_tracker)) - return reg; + if (unlikely(b->conf.reg_perm)) { + for (unsigned i = reg; i < reg + count; i++) { + assert((b->conf.reg_perm(b, i) & CS_REG_WR) || + !"Trying to write a restricted register"); + } + } - for (unsigned i = reg; i < reg + count; i++) { - if (BITSET_TEST(ls_tracker->pending_stores, i)) - assert(!"register reused as a destination before flushing stores\n"); + struct cs_load_store_tracker *ls_tracker = b->conf.ls_tracker; + + if (unlikely(ls_tracker)) { + for (unsigned i = reg; i < reg + count; i++) { + if (BITSET_TEST(ls_tracker->pending_stores, i)) + assert( + !"register reused as a destination before flushing stores\n"); + } } return reg;