[PATCH v5 1/6] drm/xe: add xe_device_wa infrastructure
Lucas De Marchi
lucas.demarchi at intel.com
Wed Jul 2 21:57:34 UTC 2025
On Wed, Jul 02, 2025 at 12:30:31PM -0700, Matt Atwood wrote:
>There are some workarounds that must be applied before gt init,
>wa_150154044425 for example. Instead of sprinkling them conditionally
>throughout the driver as we did for i915 generate an oob.rules file
>reusing the RTP infrastructure to make these easier to track.
>
>v2: rename xe_soc_wa to xe_device_wa
>v5: derive prefix from argument rather then hard coding the values.
>
>Signed-off-by: Matt Atwood <matthew.s.atwood at intel.com>
>---
> drivers/gpu/drm/xe/Makefile | 11 ++++++-
> drivers/gpu/drm/xe/xe_device_wa_oob.rules | 0
> drivers/gpu/drm/xe/xe_gen_wa_oob.c | 38 ++++++++++++++++++-----
> 3 files changed, 41 insertions(+), 8 deletions(-)
> create mode 100644 drivers/gpu/drm/xe/xe_device_wa_oob.rules
>
>diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
>index 1d97e5b63f4e..48d4aa9d4ecc 100644
>--- a/drivers/gpu/drm/xe/Makefile
>+++ b/drivers/gpu/drm/xe/Makefile
>@@ -21,6 +21,15 @@ $(obj)/generated/%_wa_oob.c $(obj)/generated/%_wa_oob.h: $(obj)/xe_gen_wa_oob \
> $(src)/xe_wa_oob.rules
> $(call cmd,wa_oob)
>
>+generated_device_oob := $(obj)/generated/xe_device_wa_oob.c $(obj)/generated/xe_device_wa_oob.h
>+quiet_cmd_device_wa_oob = GEN $(notdir $(generated_device_oob))
>+ cmd_device_wa_oob = mkdir -p $(@D); $^ $(generated_device_oob)
>+$(obj)/generated/%_device_wa_oob.c $(obj)/generated/%_device_wa_oob.h: $(obj)/xe_gen_wa_oob \
>+ $(src)/xe_device_wa_oob.rules
>+ $(call cmd,device_wa_oob)
>+
>+
>+
drop these several newlines.
> # Please keep these build lists sorted!
>
> # core driver code
>@@ -340,4 +349,4 @@ $(obj)/%.hdrtest: $(src)/%.h FORCE
> $(call if_changed_dep,hdrtest)
>
> uses_generated_oob := $(addprefix $(obj)/, $(xe-y))
>-$(uses_generated_oob): $(obj)/generated/xe_wa_oob.h
>+$(uses_generated_oob): $(obj)/generated/xe_wa_oob.h $(obj)/generated/xe_device_wa_oob.h
>diff --git a/drivers/gpu/drm/xe/xe_device_wa_oob.rules b/drivers/gpu/drm/xe/xe_device_wa_oob.rules
>new file mode 100644
>index 000000000000..e69de29bb2d1
>diff --git a/drivers/gpu/drm/xe/xe_gen_wa_oob.c b/drivers/gpu/drm/xe/xe_gen_wa_oob.c
>index ed9183599e31..341840926f60 100644
>--- a/drivers/gpu/drm/xe/xe_gen_wa_oob.c
>+++ b/drivers/gpu/drm/xe/xe_gen_wa_oob.c
All the changes in this files should be a prep commit: they can be done
regardless of having device_wa_oob
>@@ -18,8 +18,8 @@
> " *\n" \
> " * This file was generated from rules: %s\n" \
> " */\n" \
>- "#ifndef _GENERATED_XE_WA_OOB_\n" \
>- "#define _GENERATED_XE_WA_OOB_\n" \
>+ "#ifndef _GENERATED_%s_\n" \
>+ "#define _GENERATED_%s_\n" \
> "\n" \
> "enum {\n"
>
>@@ -52,7 +52,7 @@ static char *strip(char *line, size_t linelen)
> }
>
> #define MAX_LINE_LEN 4096
>-static int parse(FILE *input, FILE *csource, FILE *cheader)
>+static int parse(FILE *input, FILE *csource, FILE *cheader, char *prefix)
> {
> char line[MAX_LINE_LEN + 1];
> char *name, *prev_name = NULL, *rules;
>@@ -96,7 +96,7 @@ static int parse(FILE *input, FILE *csource, FILE *cheader)
> }
>
> if (name) {
>- fprintf(cheader, "\tXE_WA_OOB_%s = %u,\n", name, idx);
>+ fprintf(cheader, "\t%s_%s = %u,\n", prefix, name, idx);
>
> /* Close previous entry before starting a new one */
> if (idx)
>@@ -118,7 +118,26 @@ static int parse(FILE *input, FILE *csource, FILE *cheader)
> if (idx)
> fprintf(csource, ") },\n");
>
>- fprintf(cheader, "\t_XE_WA_OOB_COUNT = %u\n", idx);
>+ fprintf(cheader, "\t_%s_COUNT = %u\n", prefix, idx);
>+
>+ return 0;
>+}
>+
>+static int fn_to_prefix(const char *fn, char *prefix, size_t size)
>+{
>+ if (!strncpy(prefix, fn, size - 1))
>+ return -ENAMETOOLONG;
I didn't suggest using strncpy(). If you do, then you need to adjust the
error handling. There are 2 problems here:
1) strncpy() doesn't return NULL on error. See CAVEATS section
in strncpy(3) or strlcpy(3) that also documents strncpy and its
return value:
strcpy(3)
strcat(3)
strncpy(3)
strncat(3)
The dst pointer, which is useless.
2) prefix is on the stack in the caller, so it doesn't necessarily
ends with '\0'. So even if you copied at most size - 1, the last
char may not be 0 and you will have an infinity loop below.
Those 2 could be fixed by doing this instead:
diff --git a/drivers/gpu/drm/xe/xe_gen_wa_oob.c b/drivers/gpu/drm/xe/xe_gen_wa_oob.c
index 341840926f602..ed550dca5f439 100644
--- a/drivers/gpu/drm/xe/xe_gen_wa_oob.c
+++ b/drivers/gpu/drm/xe/xe_gen_wa_oob.c
@@ -125,7 +125,7 @@ static int parse(FILE *input, FILE *csource, FILE *cheader, char *prefix)
static int fn_to_prefix(const char *fn, char *prefix, size_t size)
{
- if (!strncpy(prefix, fn, size - 1))
+ if (strlcpy(prefix, basename(fn), size) >= size)
return -ENAMETOOLONG;
for (char *p = prefix; *p; p++) {
@@ -168,8 +168,10 @@ int main(int argc, const char *argv[])
return 1;
}
- if (fn_to_prefix(basename(args[ARGS_CHEADER].fn), prefix, sizeof(prefix)) < 0)
+ if (fn_to_prefix(args[ARGS_CHEADER].fn, prefix, sizeof(prefix)) < 0) {
+ fprintf(stderr, "ERROR: invalid header name: %s\n", args[ARGS_CHEADER].fn);
return 1;
+ }
for (int i = 0; i < _ARGS_COUNT; i++) {
args[i].f = fopen(args[i].fn, args[i].mode);
did you find any issue with strlcpy? I think it's available on all
toolchains, but maybe we need the workaround by including tools/include/linux/string.h
I tried the diff above locally and it worked for me though.
Lucas De Marchi
>+
>+ for (char *p = prefix; *p; p++) {
>+ switch (*p) {
>+ case '.':
>+ *p = '\0';
>+ return 0;
>+ default:
>+ *p = toupper(*p);
>+ break;
>+ }
>+ }
>
> return 0;
> }
>@@ -141,6 +160,7 @@ int main(int argc, const char *argv[])
> [ARGS_CHEADER] = { .fn = argv[3], .mode = "w" },
> };
> int ret = 1;
>+ char prefix[128];
>
> if (argc < 3) {
> fprintf(stderr, "ERROR: wrong arguments\n");
>@@ -148,6 +168,9 @@ int main(int argc, const char *argv[])
> return 1;
> }
>
>+ if (fn_to_prefix(basename(args[ARGS_CHEADER].fn), prefix, sizeof(prefix)) < 0)
>+ return 1;
>+
> for (int i = 0; i < _ARGS_COUNT; i++) {
> args[i].f = fopen(args[i].fn, args[i].mode);
> if (!args[i].f) {
>@@ -157,9 +180,10 @@ int main(int argc, const char *argv[])
> }
> }
>
>- fprintf(args[ARGS_CHEADER].f, HEADER, args[ARGS_INPUT].fn);
>+ fprintf(args[ARGS_CHEADER].f, HEADER, args[ARGS_INPUT].fn, prefix, prefix);
>+
> ret = parse(args[ARGS_INPUT].f, args[ARGS_CSOURCE].f,
>- args[ARGS_CHEADER].f);
>+ args[ARGS_CHEADER].f, prefix);
> if (!ret)
> fprintf(args[ARGS_CHEADER].f, FOOTER);
>
>--
>2.49.0
>
More information about the Intel-xe
mailing list