[PATCH] drm/xe: Generate oob before compiling anything

[PATCH] drm/xe: Generate oob before compiling anything

[Lucas De Marchi]

Instead of keep adding more dependencies as WAs are needed in different
places of the driver, just add a rule with all the objects so the code
generation happens before anything else.

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 drivers/gpu/drm/xe/Makefile | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
index 6015c9e41f24..a5809b3ec8a5 100644
--- a/drivers/gpu/drm/xe/Makefile
+++ b/drivers/gpu/drm/xe/Makefile
@@ -46,18 +46,6 @@ $(obj)/generated/%_wa_oob.c $(obj)/generated/%_wa_oob.h: $(obj)/xe_gen_wa_oob \
 		 $(srctree)/$(src)/xe_wa_oob.rules
 	$(call cmd,wa_oob)
 
-uses_generated_oob := \
-	$(obj)/xe_gsc.o \
-	$(obj)/xe_guc.o \
-	$(obj)/xe_guc_ads.o \
-	$(obj)/xe_migrate.o \
-	$(obj)/xe_ring_ops.o \
-	$(obj)/xe_vm.o \
-	$(obj)/xe_wa.o \
-	$(obj)/xe_ttm_stolen_mgr.o
-
-$(uses_generated_oob): $(generated_oob)
-
 # Please keep these build lists sorted!
 
 # core driver code
@@ -319,3 +307,7 @@ quiet_cmd_hdrtest = HDRTEST $(patsubst %.hdrtest,%.h,$@)
 
 $(obj)/%.hdrtest: $(src)/%.h FORCE
 	$(call if_changed_dep,hdrtest)
+
+# make sure the generated header is done before any
+# compilation unit is attempted
+$(xe-y): $(generated_oob)
-- 
2.43.0

✓ CI.Patch_applied: success for drm/xe: Generate oob before compiling anything

[Patchwork]

== Series Details ==

Series: drm/xe: Generate oob before compiling anything
URL   : https://patchwork.freedesktop.org/series/132331/
State : success

== Summary ==

=== Applying kernel patches on branch 'drm-tip' with base: ===
Base commit: 64a24a7dc774 drm-tip: 2024y-04m-11d-16h-42m-51s UTC integration manifest
=== git am output follows ===
Applying: drm/xe: Generate oob before compiling anything

✓ CI.checkpatch: success for drm/xe: Generate oob before compiling anything

[Patchwork]

== Series Details ==

Series: drm/xe: Generate oob before compiling anything
URL   : https://patchwork.freedesktop.org/series/132331/
State : success

== Summary ==

+ KERNEL=/kernel
+ git clone https://gitlab.freedesktop.org/drm/maintainer-tools mt
Cloning into 'mt'...
warning: redirecting to https://gitlab.freedesktop.org/drm/maintainer-tools.git/
+ git -C mt rev-list -n1 origin/master
10b531c2aeb176a1a539b4a77216232f97719cec
+ cd /kernel
+ git config --global --add safe.directory /kernel
+ git log -n1
commit 3e494ef639c9dd62f9efe46c3b0e787867bda2bc
Author: Lucas De Marchi <lucas.demarchi@intel.com>
Date:   Thu Apr 11 14:07:03 2024 -0700

    drm/xe: Generate oob before compiling anything
    
    Instead of keep adding more dependencies as WAs are needed in different
    places of the driver, just add a rule with all the objects so the code
    generation happens before anything else.
    
    Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
+ /mt/dim checkpatch 64a24a7dc774886c660a412a0d12c02f4bfec223 drm-intel
3e494ef639c9 drm/xe: Generate oob before compiling anything

✗ CI.KUnit: failure for drm/xe: Generate oob before compiling anything

[Patchwork]

== Series Details ==

Series: drm/xe: Generate oob before compiling anything
URL   : https://patchwork.freedesktop.org/series/132331/
State : failure

== Summary ==

+ trap cleanup EXIT
+ /kernel/tools/testing/kunit/kunit.py run --kunitconfig /kernel/drivers/gpu/drm/xe/.kunitconfig
ERROR:root:../arch/x86/um/user-offsets.c:17:6: warning: no previous prototype for ‘foo’ [-Wmissing-prototypes]
   17 | void foo(void)
      |      ^~~
In file included from ../arch/um/kernel/asm-offsets.c:1:
../arch/x86/um/shared/sysdep/kernel-offsets.h:9:6: warning: no previous prototype for ‘foo’ [-Wmissing-prototypes]
    9 | void foo(void)
      |      ^~~
../arch/x86/um/fault.c:18:5: warning: no previous prototype for ‘arch_fixup’ [-Wmissing-prototypes]
   18 | int arch_fixup(unsigned long address, struct uml_pt_regs *regs)
      |     ^~~~~~~~~~
../arch/x86/um/bugs_64.c:9:6: warning: no previous prototype for ‘arch_check_bugs’ [-Wmissing-prototypes]
    9 | void arch_check_bugs(void)
      |      ^~~~~~~~~~~~~~~
../arch/x86/um/bugs_64.c:13:6: warning: no previous prototype for ‘arch_examine_signal’ [-Wmissing-prototypes]
   13 | void arch_examine_signal(int sig, struct uml_pt_regs *regs)
      |      ^~~~~~~~~~~~~~~~~~~
../arch/x86/um/os-Linux/registers.c:146:15: warning: no previous prototype for ‘get_thread_reg’ [-Wmissing-prototypes]
  146 | unsigned long get_thread_reg(int reg, jmp_buf *buf)
      |               ^~~~~~~~~~~~~~
../arch/um/os-Linux/skas/process.c:107:6: warning: no previous prototype for ‘wait_stub_done’ [-Wmissing-prototypes]
  107 | void wait_stub_done(int pid)
      |      ^~~~~~~~~~~~~~
../arch/x86/um/vdso/um_vdso.c:16:5: warning: no previous prototype for ‘__vdso_clock_gettime’ [-Wmissing-prototypes]
   16 | int __vdso_clock_gettime(clockid_t clock, struct __kernel_old_timespec *ts)
      |     ^~~~~~~~~~~~~~~~~~~~
../arch/x86/um/vdso/um_vdso.c:30:5: warning: no previous prototype for ‘__vdso_gettimeofday’ [-Wmissing-prototypes]
   30 | int __vdso_gettimeofday(struct __kernel_old_timeval *tv, struct timezone *tz)
      |     ^~~~~~~~~~~~~~~~~~~
../arch/x86/um/vdso/um_vdso.c:44:21: warning: no previous prototype for ‘__vdso_time’ [-Wmissing-prototypes]
   44 | __kernel_old_time_t __vdso_time(__kernel_old_time_t *t)
      |                     ^~~~~~~~~~~
../arch/x86/um/vdso/um_vdso.c:57:1: warning: no previous prototype for ‘__vdso_getcpu’ [-Wmissing-prototypes]
   57 | __vdso_getcpu(unsigned *cpu, unsigned *node, struct getcpu_cache *unused)
      | ^~~~~~~~~~~~~
../arch/um/os-Linux/skas/process.c:683:6: warning: no previous prototype for ‘__switch_mm’ [-Wmissing-prototypes]
  683 | void __switch_mm(struct mm_id *mm_idp)
      |      ^~~~~~~~~~~
../arch/x86/um/os-Linux/mcontext.c:7:6: warning: no previous prototype for ‘get_regs_from_mc’ [-Wmissing-prototypes]
    7 | void get_regs_from_mc(struct uml_pt_regs *regs, mcontext_t *mc)
      |      ^~~~~~~~~~~~~~~~
../arch/um/kernel/skas/mmu.c:17:5: warning: no previous prototype for ‘init_new_context’ [-Wmissing-prototypes]
   17 | int init_new_context(struct task_struct *task, struct mm_struct *mm)
      |     ^~~~~~~~~~~~~~~~
../arch/um/kernel/skas/mmu.c:60:6: warning: no previous prototype for ‘destroy_context’ [-Wmissing-prototypes]
   60 | void destroy_context(struct mm_struct *mm)
      |      ^~~~~~~~~~~~~~~
../arch/um/os-Linux/main.c:187:7: warning: no previous prototype for ‘__wrap_malloc’ [-Wmissing-prototypes]
  187 | void *__wrap_malloc(int size)
      |       ^~~~~~~~~~~~~
../arch/um/os-Linux/main.c:208:7: warning: no previous prototype for ‘__wrap_calloc’ [-Wmissing-prototypes]
  208 | void *__wrap_calloc(int n, int size)
      |       ^~~~~~~~~~~~~
../arch/um/os-Linux/main.c:222:6: warning: no previous prototype for ‘__wrap_free’ [-Wmissing-prototypes]
  222 | void __wrap_free(void *ptr)
      |      ^~~~~~~~~~~
../arch/um/kernel/skas/process.c:36:12: warning: no previous prototype for ‘start_uml’ [-Wmissing-prototypes]
   36 | int __init start_uml(void)
      |            ^~~~~~~~~
../arch/um/os-Linux/mem.c:28:6: warning: no previous prototype for ‘kasan_map_memory’ [-Wmissing-prototypes]
   28 | void kasan_map_memory(void *start, size_t len)
      |      ^~~~~~~~~~~~~~~~
../arch/um/os-Linux/mem.c:212:13: warning: no previous prototype for ‘check_tmpexec’ [-Wmissing-prototypes]
  212 | void __init check_tmpexec(void)
      |             ^~~~~~~~~~~~~
../arch/x86/um/ptrace_64.c:111:5: warning: no previous prototype for ‘poke_user’ [-Wmissing-prototypes]
  111 | int poke_user(struct task_struct *child, long addr, long data)
      |     ^~~~~~~~~
../arch/x86/um/ptrace_64.c:171:5: warning: no previous prototype for ‘peek_user’ [-Wmissing-prototypes]
  171 | int peek_user(struct task_struct *child, long addr, long data)
      |     ^~~~~~~~~
../arch/um/kernel/mem.c:202:8: warning: no previous prototype for ‘pgd_alloc’ [-Wmissing-prototypes]
  202 | pgd_t *pgd_alloc(struct mm_struct *mm)
      |        ^~~~~~~~~
../arch/um/kernel/mem.c:215:7: warning: no previous prototype for ‘uml_kmalloc’ [-Wmissing-prototypes]
  215 | void *uml_kmalloc(int size, int flags)
      |       ^~~~~~~~~~~
../arch/um/os-Linux/signal.c:75:6: warning: no previous prototype for ‘sig_handler’ [-Wmissing-prototypes]
   75 | void sig_handler(int sig, struct siginfo *si, mcontext_t *mc)
      |      ^~~~~~~~~~~
../arch/um/os-Linux/signal.c:111:6: warning: no previous prototype for ‘timer_alarm_handler’ [-Wmissing-prototypes]
  111 | void timer_alarm_handler(int sig, struct siginfo *unused_si, mcontext_t *mc)
      |      ^~~~~~~~~~~~~~~~~~~
../arch/um/os-Linux/start_up.c:301:12: warning: no previous prototype for ‘parse_iomem’ [-Wmissing-prototypes]
  301 | int __init parse_iomem(char *str, int *add)
      |            ^~~~~~~~~~~
../arch/x86/um/signal.c:560:6: warning: no previous prototype for ‘sys_rt_sigreturn’ [-Wmissing-prototypes]
  560 | long sys_rt_sigreturn(void)
      |      ^~~~~~~~~~~~~~~~
../arch/um/kernel/process.c:51:5: warning: no previous prototype for ‘pid_to_processor_id’ [-Wmissing-prototypes]
   51 | int pid_to_processor_id(int pid)
      |     ^~~~~~~~~~~~~~~~~~~
../arch/um/kernel/process.c:87:7: warning: no previous prototype for ‘__switch_to’ [-Wmissing-prototypes]
   87 | void *__switch_to(struct task_struct *from, struct task_struct *to)
      |       ^~~~~~~~~~~
../arch/um/kernel/process.c:140:6: warning: no previous prototype for ‘fork_handler’ [-Wmissing-prototypes]
  140 | void fork_handler(void)
      |      ^~~~~~~~~~~~
../arch/um/kernel/process.c:217:6: warning: no previous prototype for ‘arch_cpu_idle’ [-Wmissing-prototypes]
  217 | void arch_cpu_idle(void)
      |      ^~~~~~~~~~~~~
../arch/um/kernel/process.c:253:5: warning: no previous prototype for ‘copy_to_user_proc’ [-Wmissing-prototypes]
  253 | int copy_to_user_proc(void __user *to, void *from, int size)
      |     ^~~~~~~~~~~~~~~~~
../arch/um/kernel/process.c:263:5: warning: no previous prototype for ‘clear_user_proc’ [-Wmissing-prototypes]
  263 | int clear_user_proc(void __user *buf, int size)
      |     ^~~~~~~~~~~~~~~
../arch/um/kernel/process.c:271:6: warning: no previous prototype for ‘set_using_sysemu’ [-Wmissing-prototypes]
  271 | void set_using_sysemu(int value)
      |      ^~~~~~~~~~~~~~~~
../arch/um/kernel/process.c:278:5: warning: no previous prototype for ‘get_using_sysemu’ [-Wmissing-prototypes]
  278 | int get_using_sysemu(void)
      |     ^~~~~~~~~~~~~~~~
../arch/um/kernel/process.c:316:12: warning: no previous prototype for ‘make_proc_sysemu’ [-Wmissing-prototypes]
  316 | int __init make_proc_sysemu(void)
      |            ^~~~~~~~~~~~~~~~
../arch/um/kernel/process.c:348:15: warning: no previous prototype for ‘arch_align_stack’ [-Wmissing-prototypes]
  348 | unsigned long arch_align_stack(unsigned long sp)
      |               ^~~~~~~~~~~~~~~~
../arch/um/kernel/reboot.c:45:6: warning: no previous prototype for ‘machine_restart’ [-Wmissing-prototypes]
   45 | void machine_restart(char * __unused)
      |      ^~~~~~~~~~~~~~~
../arch/um/kernel/reboot.c:51:6: warning: no previous prototype for ‘machine_power_off’ [-Wmissing-prototypes]
   51 | void machine_power_off(void)
      |      ^~~~~~~~~~~~~~~~~
../arch/um/kernel/reboot.c:57:6: warning: no previous prototype for ‘machine_halt’ [-Wmissing-prototypes]
   57 | void machine_halt(void)
      |      ^~~~~~~~~~~~
../arch/um/kernel/tlb.c:579:6: warning: no previous prototype for ‘flush_tlb_mm_range’ [-Wmissing-prototypes]
  579 | void flush_tlb_mm_range(struct mm_struct *mm, unsigned long start,
      |      ^~~~~~~~~~~~~~~~~~
../arch/um/kernel/tlb.c:594:6: warning: no previous prototype for ‘force_flush_all’ [-Wmissing-prototypes]
  594 | void force_flush_all(void)
      |      ^~~~~~~~~~~~~~~
../arch/um/kernel/um_arch.c:408:19: warning: no previous prototype for ‘read_initrd’ [-Wmissing-prototypes]
  408 | int __init __weak read_initrd(void)
      |                   ^~~~~~~~~~~
../arch/um/kernel/um_arch.c:461:7: warning: no previous prototype for ‘text_poke’ [-Wmissing-prototypes]
  461 | void *text_poke(void *addr, const void *opcode, size_t len)
      |       ^~~~~~~~~
../arch/um/kernel/um_arch.c:473:6: warning: no previous prototype for ‘text_poke_sync’ [-Wmissing-prototypes]
  473 | void text_poke_sync(void)
      |      ^~~~~~~~~~~~~~
../arch/um/kernel/kmsg_dump.c:60:12: warning: no previous prototype for ‘kmsg_dumper_stdout_init’ [-Wmissing-prototypes]
   60 | int __init kmsg_dumper_stdout_init(void)
      |            ^~~~~~~~~~~~~~~~~~~~~~~
../arch/x86/um/syscalls_64.c:48:6: warning: no previous prototype for ‘arch_switch_to’ [-Wmissing-prototypes]
   48 | void arch_switch_to(struct task_struct *to)
      |      ^~~~~~~~~~~~~~
../drivers/gpu/drm/xe/xe_gsc.c:10:10: fatal error: generated/xe_wa_oob.h: No such file or directory
   10 | #include <generated/xe_wa_oob.h>
      |          ^~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[7]: *** [../scripts/Makefile.build:244: drivers/gpu/drm/xe/xe_gsc.o] Error 1
make[7]: *** Waiting for unfinished jobs....
make[6]: *** [../scripts/Makefile.build:485: drivers/gpu/drm/xe] Error 2
make[6]: *** Waiting for unfinished jobs....
../lib/iomap.c:156:5: warning: no previous prototype for ‘ioread64_lo_hi’ [-Wmissing-prototypes]
  156 | u64 ioread64_lo_hi(const void __iomem *addr)
      |     ^~~~~~~~~~~~~~
../lib/iomap.c:163:5: warning: no previous prototype for ‘ioread64_hi_lo’ [-Wmissing-prototypes]
  163 | u64 ioread64_hi_lo(const void __iomem *addr)
      |     ^~~~~~~~~~~~~~
../lib/iomap.c:170:5: warning: no previous prototype for ‘ioread64be_lo_hi’ [-Wmissing-prototypes]
  170 | u64 ioread64be_lo_hi(const void __iomem *addr)
      |     ^~~~~~~~~~~~~~~~
../lib/iomap.c:178:5: warning: no previous prototype for ‘ioread64be_hi_lo’ [-Wmissing-prototypes]
  178 | u64 ioread64be_hi_lo(const void __iomem *addr)
      |     ^~~~~~~~~~~~~~~~
../lib/iomap.c:264:6: warning: no previous prototype for ‘iowrite64_lo_hi’ [-Wmissing-prototypes]
  264 | void iowrite64_lo_hi(u64 val, void __iomem *addr)
      |      ^~~~~~~~~~~~~~~
../lib/iomap.c:272:6: warning: no previous prototype for ‘iowrite64_hi_lo’ [-Wmissing-prototypes]
  272 | void iowrite64_hi_lo(u64 val, void __iomem *addr)
      |      ^~~~~~~~~~~~~~~
../lib/iomap.c:280:6: warning: no previous prototype for ‘iowrite64be_lo_hi’ [-Wmissing-prototypes]
  280 | void iowrite64be_lo_hi(u64 val, void __iomem *addr)
      |      ^~~~~~~~~~~~~~~~~
../lib/iomap.c:288:6: warning: no previous prototype for ‘iowrite64be_hi_lo’ [-Wmissing-prototypes]
  288 | void iowrite64be_hi_lo(u64 val, void __iomem *addr)
      |      ^~~~~~~~~~~~~~~~~
make[5]: *** [../scripts/Makefile.build:485: drivers/gpu/drm] Error 2
make[4]: *** [../scripts/Makefile.build:485: drivers/gpu] Error 2
make[3]: *** [../scripts/Makefile.build:485: drivers] Error 2
make[3]: *** Waiting for unfinished jobs....
make[2]: *** [/kernel/Makefile:1919: .] Error 2
make[1]: *** [/kernel/Makefile:240: __sub-make] Error 2
make: *** [Makefile:240: __sub-make] Error 2

[21:34:13] Configuring KUnit Kernel ...
Generating .config ...
Populating config with:
$ make ARCH=um O=.kunit olddefconfig
[21:34:17] Building KUnit Kernel ...
Populating config with:
$ make ARCH=um O=.kunit olddefconfig
Building with:
$ make ARCH=um O=.kunit --jobs=48
+ cleanup
++ stat -c %u:%g /kernel
+ chown -R 1003:1003 /kernel

Re: [PATCH] drm/xe: Generate oob before compiling anything

[Jani Nikula]

On Thu, 11 Apr 2024, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
> Instead of keep adding more dependencies as WAs are needed in different
> places of the driver, just add a rule with all the objects so the code
> generation happens before anything else.

What's "oob" and why is it used unexplained everywhere?

BR,
Jani.

>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>  drivers/gpu/drm/xe/Makefile | 16 ++++------------
>  1 file changed, 4 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
> index 6015c9e41f24..a5809b3ec8a5 100644
> --- a/drivers/gpu/drm/xe/Makefile
> +++ b/drivers/gpu/drm/xe/Makefile
> @@ -46,18 +46,6 @@ $(obj)/generated/%_wa_oob.c $(obj)/generated/%_wa_oob.h: $(obj)/xe_gen_wa_oob \
>  		 $(srctree)/$(src)/xe_wa_oob.rules
>  	$(call cmd,wa_oob)
>  
> -uses_generated_oob := \
> -	$(obj)/xe_gsc.o \
> -	$(obj)/xe_guc.o \
> -	$(obj)/xe_guc_ads.o \
> -	$(obj)/xe_migrate.o \
> -	$(obj)/xe_ring_ops.o \
> -	$(obj)/xe_vm.o \
> -	$(obj)/xe_wa.o \
> -	$(obj)/xe_ttm_stolen_mgr.o
> -
> -$(uses_generated_oob): $(generated_oob)
> -
>  # Please keep these build lists sorted!
>  
>  # core driver code
> @@ -319,3 +307,7 @@ quiet_cmd_hdrtest = HDRTEST $(patsubst %.hdrtest,%.h,$@)
>  
>  $(obj)/%.hdrtest: $(src)/%.h FORCE
>  	$(call if_changed_dep,hdrtest)
> +
> +# make sure the generated header is done before any
> +# compilation unit is attempted
> +$(xe-y): $(generated_oob)

-- 
Jani Nikula, Intel

Re: [PATCH] drm/xe: Generate oob before compiling anything

[Lucas De Marchi]

On Fri, Apr 12, 2024 at 09:56:05AM +0300, Jani Nikula wrote:
>On Thu, 11 Apr 2024, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
>> Instead of keep adding more dependencies as WAs are needed in different
>> places of the driver, just add a rule with all the objects so the code
>> generation happens before anything else.
>
>What's "oob" and why is it used unexplained everywhere?

from xe_wa.c kernel-doc:

  * platform. There are some basic categories of workarounds, depending on
  * how/when they are applied:

   ( ... )
  * - Other/OOB:  There are WAs that, due to their nature, cannot be applied from    
  *   a central place. Those are peppered around the rest of the code, as needed.    
  *   Workarounds related to the display IP are the main example.                    

This can certainly be improved and the link to wa_oob.c made. But it's
not that it was not explained anywhere :)

Lucas De Marchi

Re: [PATCH] drm/xe: Generate oob before compiling anything

[Jani Nikula]

On Fri, 12 Apr 2024, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
> On Fri, Apr 12, 2024 at 09:56:05AM +0300, Jani Nikula wrote:
>>On Thu, 11 Apr 2024, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
>>> Instead of keep adding more dependencies as WAs are needed in different
>>> places of the driver, just add a rule with all the objects so the code
>>> generation happens before anything else.
>>
>>What's "oob" and why is it used unexplained everywhere?
>
> from xe_wa.c kernel-doc:
>
>   * platform. There are some basic categories of workarounds, depending on
>   * how/when they are applied:
>
>    ( ... )
>   * - Other/OOB:  There are WAs that, due to their nature, cannot be applied from    
>   *   a central place. Those are peppered around the rest of the code, as needed.    
>   *   Workarounds related to the display IP are the main example.                    
>
> This can certainly be improved and the link to wa_oob.c made. But it's
> not that it was not explained anywhere :)

Yeah, but... what does OOB mean? Out-of-band?

BR,
Jani.

-- 
Jani Nikula, Intel

Re: [PATCH] drm/xe: Generate oob before compiling anything

[Lucas De Marchi]

On Fri, Apr 12, 2024 at 05:39:20PM +0300, Jani Nikula wrote:
>On Fri, 12 Apr 2024, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
>> On Fri, Apr 12, 2024 at 09:56:05AM +0300, Jani Nikula wrote:
>>>On Thu, 11 Apr 2024, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
>>>> Instead of keep adding more dependencies as WAs are needed in different
>>>> places of the driver, just add a rule with all the objects so the code
>>>> generation happens before anything else.
>>>
>>>What's "oob" and why is it used unexplained everywhere?
>>
>> from xe_wa.c kernel-doc:
>>
>>   * platform. There are some basic categories of workarounds, depending on
>>   * how/when they are applied:
>>
>>    ( ... )
>>   * - Other/OOB:  There are WAs that, due to their nature, cannot be applied from
>>   *   a central place. Those are peppered around the rest of the code, as needed.
>>   *   Workarounds related to the display IP are the main example.
>>
>> This can certainly be improved and the link to wa_oob.c made. But it's
>> not that it was not explained anywhere :)
>
>Yeah, but... what does OOB mean? Out-of-band?

yes, that's correct :)

Lucas De Marchi

>
>BR,
>Jani.
>
>-- 
>Jani Nikula, Intel

Re: ✗ CI.KUnit: failure for drm/xe: Generate oob before compiling anything

[Lucas De Marchi]

+Masahiro

On Thu, Apr 11, 2024 at 09:34:41PM GMT, Patchwork wrote:
>== Series Details ==
>
>Series: drm/xe: Generate oob before compiling anything
>URL   : https://patchwork.freedesktop.org/series/132331/
>State : failure
>
>== Summary ==
>
>+ trap cleanup EXIT
>+ /kernel/tools/testing/kunit/kunit.py run --kunitconfig /kernel/drivers/gpu/drm/xe/.kunitconfig
>ERROR:root:../arch/x86/um/user-offsets.c:17:6: warning: no previous prototype for ‘foo’ [-Wmissing-prototypes]
>   17 | void foo(void)
>      |      ^~~
>In file included from ../arch/um/kernel/asm-offsets.c:1:
>../arch/x86/um/shared/sysdep/kernel-offsets.h:9:6: warning: no previous prototype for ‘foo’ [-Wmissing-prototypes]
>    9 | void foo(void)
>      |      ^~~
>../arch/x86/um/fault.c:18:5: warning: no previous prototype for ‘arch_fixup’ [-Wmissing-prototypes]
>   18 | int arch_fixup(unsigned long address, struct uml_pt_regs *regs)
>      |     ^~~~~~~~~~
>../arch/x86/um/bugs_64.c:9:6: warning: no previous prototype for ‘arch_check_bugs’ [-Wmissing-prototypes]
>    9 | void arch_check_bugs(void)
>      |      ^~~~~~~~~~~~~~~
>../arch/x86/um/bugs_64.c:13:6: warning: no previous prototype for ‘arch_examine_signal’ [-Wmissing-prototypes]
>   13 | void arch_examine_signal(int sig, struct uml_pt_regs *regs)
>      |      ^~~~~~~~~~~~~~~~~~~
>../arch/x86/um/os-Linux/registers.c:146:15: warning: no previous prototype for ‘get_thread_reg’ [-Wmissing-prototypes]
>  146 | unsigned long get_thread_reg(int reg, jmp_buf *buf)
>      |               ^~~~~~~~~~~~~~
>../arch/um/os-Linux/skas/process.c:107:6: warning: no previous prototype for ‘wait_stub_done’ [-Wmissing-prototypes]
>  107 | void wait_stub_done(int pid)
>      |      ^~~~~~~~~~~~~~
>../arch/x86/um/vdso/um_vdso.c:16:5: warning: no previous prototype for ‘__vdso_clock_gettime’ [-Wmissing-prototypes]
>   16 | int __vdso_clock_gettime(clockid_t clock, struct __kernel_old_timespec *ts)
>      |     ^~~~~~~~~~~~~~~~~~~~
>../arch/x86/um/vdso/um_vdso.c:30:5: warning: no previous prototype for ‘__vdso_gettimeofday’ [-Wmissing-prototypes]
>   30 | int __vdso_gettimeofday(struct __kernel_old_timeval *tv, struct timezone *tz)
>      |     ^~~~~~~~~~~~~~~~~~~
>../arch/x86/um/vdso/um_vdso.c:44:21: warning: no previous prototype for ‘__vdso_time’ [-Wmissing-prototypes]
>   44 | __kernel_old_time_t __vdso_time(__kernel_old_time_t *t)
>      |                     ^~~~~~~~~~~
>../arch/x86/um/vdso/um_vdso.c:57:1: warning: no previous prototype for ‘__vdso_getcpu’ [-Wmissing-prototypes]
>   57 | __vdso_getcpu(unsigned *cpu, unsigned *node, struct getcpu_cache *unused)
>      | ^~~~~~~~~~~~~
>../arch/um/os-Linux/skas/process.c:683:6: warning: no previous prototype for ‘__switch_mm’ [-Wmissing-prototypes]
>  683 | void __switch_mm(struct mm_id *mm_idp)
>      |      ^~~~~~~~~~~
>../arch/x86/um/os-Linux/mcontext.c:7:6: warning: no previous prototype for ‘get_regs_from_mc’ [-Wmissing-prototypes]
>    7 | void get_regs_from_mc(struct uml_pt_regs *regs, mcontext_t *mc)
>      |      ^~~~~~~~~~~~~~~~
>../arch/um/kernel/skas/mmu.c:17:5: warning: no previous prototype for ‘init_new_context’ [-Wmissing-prototypes]
>   17 | int init_new_context(struct task_struct *task, struct mm_struct *mm)
>      |     ^~~~~~~~~~~~~~~~
>../arch/um/kernel/skas/mmu.c:60:6: warning: no previous prototype for ‘destroy_context’ [-Wmissing-prototypes]
>   60 | void destroy_context(struct mm_struct *mm)
>      |      ^~~~~~~~~~~~~~~
>../arch/um/os-Linux/main.c:187:7: warning: no previous prototype for ‘__wrap_malloc’ [-Wmissing-prototypes]
>  187 | void *__wrap_malloc(int size)
>      |       ^~~~~~~~~~~~~
>../arch/um/os-Linux/main.c:208:7: warning: no previous prototype for ‘__wrap_calloc’ [-Wmissing-prototypes]
>  208 | void *__wrap_calloc(int n, int size)
>      |       ^~~~~~~~~~~~~
>../arch/um/os-Linux/main.c:222:6: warning: no previous prototype for ‘__wrap_free’ [-Wmissing-prototypes]
>  222 | void __wrap_free(void *ptr)
>      |      ^~~~~~~~~~~
>../arch/um/kernel/skas/process.c:36:12: warning: no previous prototype for ‘start_uml’ [-Wmissing-prototypes]
>   36 | int __init start_uml(void)
>      |            ^~~~~~~~~
>../arch/um/os-Linux/mem.c:28:6: warning: no previous prototype for ‘kasan_map_memory’ [-Wmissing-prototypes]
>   28 | void kasan_map_memory(void *start, size_t len)
>      |      ^~~~~~~~~~~~~~~~
>../arch/um/os-Linux/mem.c:212:13: warning: no previous prototype for ‘check_tmpexec’ [-Wmissing-prototypes]
>  212 | void __init check_tmpexec(void)
>      |             ^~~~~~~~~~~~~
>../arch/x86/um/ptrace_64.c:111:5: warning: no previous prototype for ‘poke_user’ [-Wmissing-prototypes]
>  111 | int poke_user(struct task_struct *child, long addr, long data)
>      |     ^~~~~~~~~
>../arch/x86/um/ptrace_64.c:171:5: warning: no previous prototype for ‘peek_user’ [-Wmissing-prototypes]
>  171 | int peek_user(struct task_struct *child, long addr, long data)
>      |     ^~~~~~~~~
>../arch/um/kernel/mem.c:202:8: warning: no previous prototype for ‘pgd_alloc’ [-Wmissing-prototypes]
>  202 | pgd_t *pgd_alloc(struct mm_struct *mm)
>      |        ^~~~~~~~~
>../arch/um/kernel/mem.c:215:7: warning: no previous prototype for ‘uml_kmalloc’ [-Wmissing-prototypes]
>  215 | void *uml_kmalloc(int size, int flags)
>      |       ^~~~~~~~~~~
>../arch/um/os-Linux/signal.c:75:6: warning: no previous prototype for ‘sig_handler’ [-Wmissing-prototypes]
>   75 | void sig_handler(int sig, struct siginfo *si, mcontext_t *mc)
>      |      ^~~~~~~~~~~
>../arch/um/os-Linux/signal.c:111:6: warning: no previous prototype for ‘timer_alarm_handler’ [-Wmissing-prototypes]
>  111 | void timer_alarm_handler(int sig, struct siginfo *unused_si, mcontext_t *mc)
>      |      ^~~~~~~~~~~~~~~~~~~
>../arch/um/os-Linux/start_up.c:301:12: warning: no previous prototype for ‘parse_iomem’ [-Wmissing-prototypes]
>  301 | int __init parse_iomem(char *str, int *add)
>      |            ^~~~~~~~~~~
>../arch/x86/um/signal.c:560:6: warning: no previous prototype for ‘sys_rt_sigreturn’ [-Wmissing-prototypes]
>  560 | long sys_rt_sigreturn(void)
>      |      ^~~~~~~~~~~~~~~~
>../arch/um/kernel/process.c:51:5: warning: no previous prototype for ‘pid_to_processor_id’ [-Wmissing-prototypes]
>   51 | int pid_to_processor_id(int pid)
>      |     ^~~~~~~~~~~~~~~~~~~
>../arch/um/kernel/process.c:87:7: warning: no previous prototype for ‘__switch_to’ [-Wmissing-prototypes]
>   87 | void *__switch_to(struct task_struct *from, struct task_struct *to)
>      |       ^~~~~~~~~~~
>../arch/um/kernel/process.c:140:6: warning: no previous prototype for ‘fork_handler’ [-Wmissing-prototypes]
>  140 | void fork_handler(void)
>      |      ^~~~~~~~~~~~
>../arch/um/kernel/process.c:217:6: warning: no previous prototype for ‘arch_cpu_idle’ [-Wmissing-prototypes]
>  217 | void arch_cpu_idle(void)
>      |      ^~~~~~~~~~~~~
>../arch/um/kernel/process.c:253:5: warning: no previous prototype for ‘copy_to_user_proc’ [-Wmissing-prototypes]
>  253 | int copy_to_user_proc(void __user *to, void *from, int size)
>      |     ^~~~~~~~~~~~~~~~~
>../arch/um/kernel/process.c:263:5: warning: no previous prototype for ‘clear_user_proc’ [-Wmissing-prototypes]
>  263 | int clear_user_proc(void __user *buf, int size)
>      |     ^~~~~~~~~~~~~~~
>../arch/um/kernel/process.c:271:6: warning: no previous prototype for ‘set_using_sysemu’ [-Wmissing-prototypes]
>  271 | void set_using_sysemu(int value)
>      |      ^~~~~~~~~~~~~~~~
>../arch/um/kernel/process.c:278:5: warning: no previous prototype for ‘get_using_sysemu’ [-Wmissing-prototypes]
>  278 | int get_using_sysemu(void)
>      |     ^~~~~~~~~~~~~~~~
>../arch/um/kernel/process.c:316:12: warning: no previous prototype for ‘make_proc_sysemu’ [-Wmissing-prototypes]
>  316 | int __init make_proc_sysemu(void)
>      |            ^~~~~~~~~~~~~~~~
>../arch/um/kernel/process.c:348:15: warning: no previous prototype for ‘arch_align_stack’ [-Wmissing-prototypes]
>  348 | unsigned long arch_align_stack(unsigned long sp)
>      |               ^~~~~~~~~~~~~~~~
>../arch/um/kernel/reboot.c:45:6: warning: no previous prototype for ‘machine_restart’ [-Wmissing-prototypes]
>   45 | void machine_restart(char * __unused)
>      |      ^~~~~~~~~~~~~~~
>../arch/um/kernel/reboot.c:51:6: warning: no previous prototype for ‘machine_power_off’ [-Wmissing-prototypes]
>   51 | void machine_power_off(void)
>      |      ^~~~~~~~~~~~~~~~~
>../arch/um/kernel/reboot.c:57:6: warning: no previous prototype for ‘machine_halt’ [-Wmissing-prototypes]
>   57 | void machine_halt(void)
>      |      ^~~~~~~~~~~~
>../arch/um/kernel/tlb.c:579:6: warning: no previous prototype for ‘flush_tlb_mm_range’ [-Wmissing-prototypes]
>  579 | void flush_tlb_mm_range(struct mm_struct *mm, unsigned long start,
>      |      ^~~~~~~~~~~~~~~~~~
>../arch/um/kernel/tlb.c:594:6: warning: no previous prototype for ‘force_flush_all’ [-Wmissing-prototypes]
>  594 | void force_flush_all(void)
>      |      ^~~~~~~~~~~~~~~
>../arch/um/kernel/um_arch.c:408:19: warning: no previous prototype for ‘read_initrd’ [-Wmissing-prototypes]
>  408 | int __init __weak read_initrd(void)
>      |                   ^~~~~~~~~~~
>../arch/um/kernel/um_arch.c:461:7: warning: no previous prototype for ‘text_poke’ [-Wmissing-prototypes]
>  461 | void *text_poke(void *addr, const void *opcode, size_t len)
>      |       ^~~~~~~~~
>../arch/um/kernel/um_arch.c:473:6: warning: no previous prototype for ‘text_poke_sync’ [-Wmissing-prototypes]
>  473 | void text_poke_sync(void)
>      |      ^~~~~~~~~~~~~~
>../arch/um/kernel/kmsg_dump.c:60:12: warning: no previous prototype for ‘kmsg_dumper_stdout_init’ [-Wmissing-prototypes]
>   60 | int __init kmsg_dumper_stdout_init(void)
>      |            ^~~~~~~~~~~~~~~~~~~~~~~
>../arch/x86/um/syscalls_64.c:48:6: warning: no previous prototype for ‘arch_switch_to’ [-Wmissing-prototypes]
>   48 | void arch_switch_to(struct task_struct *to)
>      |      ^~~~~~~~~~~~~~
>../drivers/gpu/drm/xe/xe_gsc.c:10:10: fatal error: generated/xe_wa_oob.h: No such file or directory
>   10 | #include <generated/xe_wa_oob.h>
>      |          ^~~~~~~~~~~~~~~~~~~~~~~


So, I'm trying to make this generic so the generated .h/.c are done
before anything else is attempted. I'm puzzled that this works for me
for x86-64 with xe as a module or built-in, but for some reason is
failing when using ARCH=um.

Any idea what's going on?

thanks
Lucas De Marchi

>compilation terminated.
>make[7]: *** [../scripts/Makefile.build:244: drivers/gpu/drm/xe/xe_gsc.o] Error 1
>make[7]: *** Waiting for unfinished jobs....
>make[6]: *** [../scripts/Makefile.build:485: drivers/gpu/drm/xe] Error 2
>make[6]: *** Waiting for unfinished jobs....
>../lib/iomap.c:156:5: warning: no previous prototype for ‘ioread64_lo_hi’ [-Wmissing-prototypes]
>  156 | u64 ioread64_lo_hi(const void __iomem *addr)
>      |     ^~~~~~~~~~~~~~
>../lib/iomap.c:163:5: warning: no previous prototype for ‘ioread64_hi_lo’ [-Wmissing-prototypes]
>  163 | u64 ioread64_hi_lo(const void __iomem *addr)
>      |     ^~~~~~~~~~~~~~
>../lib/iomap.c:170:5: warning: no previous prototype for ‘ioread64be_lo_hi’ [-Wmissing-prototypes]
>  170 | u64 ioread64be_lo_hi(const void __iomem *addr)
>      |     ^~~~~~~~~~~~~~~~
>../lib/iomap.c:178:5: warning: no previous prototype for ‘ioread64be_hi_lo’ [-Wmissing-prototypes]
>  178 | u64 ioread64be_hi_lo(const void __iomem *addr)
>      |     ^~~~~~~~~~~~~~~~
>../lib/iomap.c:264:6: warning: no previous prototype for ‘iowrite64_lo_hi’ [-Wmissing-prototypes]
>  264 | void iowrite64_lo_hi(u64 val, void __iomem *addr)
>      |      ^~~~~~~~~~~~~~~
>../lib/iomap.c:272:6: warning: no previous prototype for ‘iowrite64_hi_lo’ [-Wmissing-prototypes]
>  272 | void iowrite64_hi_lo(u64 val, void __iomem *addr)
>      |      ^~~~~~~~~~~~~~~
>../lib/iomap.c:280:6: warning: no previous prototype for ‘iowrite64be_lo_hi’ [-Wmissing-prototypes]
>  280 | void iowrite64be_lo_hi(u64 val, void __iomem *addr)
>      |      ^~~~~~~~~~~~~~~~~
>../lib/iomap.c:288:6: warning: no previous prototype for ‘iowrite64be_hi_lo’ [-Wmissing-prototypes]
>  288 | void iowrite64be_hi_lo(u64 val, void __iomem *addr)
>      |      ^~~~~~~~~~~~~~~~~
>make[5]: *** [../scripts/Makefile.build:485: drivers/gpu/drm] Error 2
>make[4]: *** [../scripts/Makefile.build:485: drivers/gpu] Error 2
>make[3]: *** [../scripts/Makefile.build:485: drivers] Error 2
>make[3]: *** Waiting for unfinished jobs....
>make[2]: *** [/kernel/Makefile:1919: .] Error 2
>make[1]: *** [/kernel/Makefile:240: __sub-make] Error 2
>make: *** [Makefile:240: __sub-make] Error 2
>
>[21:34:13] Configuring KUnit Kernel ...
>Generating .config ...
>Populating config with:
>$ make ARCH=um O=.kunit olddefconfig
>[21:34:17] Building KUnit Kernel ...
>Populating config with:
>$ make ARCH=um O=.kunit olddefconfig
>Building with:
>$ make ARCH=um O=.kunit --jobs=48
>+ cleanup
>++ stat -c %u:%g /kernel
>+ chown -R 1003:1003 /kernel
>
>

Re: [PATCH] drm/xe: Generate oob before compiling anything

[Jani Nikula]

On Thu, 11 Apr 2024, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
> Instead of keep adding more dependencies as WAs are needed in different
> places of the driver, just add a rule with all the objects so the code
> generation happens before anything else.
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>  drivers/gpu/drm/xe/Makefile | 16 ++++------------
>  1 file changed, 4 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
> index 6015c9e41f24..a5809b3ec8a5 100644
> --- a/drivers/gpu/drm/xe/Makefile
> +++ b/drivers/gpu/drm/xe/Makefile
> @@ -46,18 +46,6 @@ $(obj)/generated/%_wa_oob.c $(obj)/generated/%_wa_oob.h: $(obj)/xe_gen_wa_oob \
>  		 $(srctree)/$(src)/xe_wa_oob.rules
>  	$(call cmd,wa_oob)
>  
> -uses_generated_oob := \
> -	$(obj)/xe_gsc.o \
> -	$(obj)/xe_guc.o \
> -	$(obj)/xe_guc_ads.o \
> -	$(obj)/xe_migrate.o \
> -	$(obj)/xe_ring_ops.o \
> -	$(obj)/xe_vm.o \
> -	$(obj)/xe_wa.o \
> -	$(obj)/xe_ttm_stolen_mgr.o
> -
> -$(uses_generated_oob): $(generated_oob)
> -
>  # Please keep these build lists sorted!
>  
>  # core driver code
> @@ -319,3 +307,7 @@ quiet_cmd_hdrtest = HDRTEST $(patsubst %.hdrtest,%.h,$@)
>  
>  $(obj)/%.hdrtest: $(src)/%.h FORCE
>  	$(call if_changed_dep,hdrtest)
> +
> +# make sure the generated header is done before any
> +# compilation unit is attempted
> +$(xe-y): $(generated_oob)

I'm sorry, I don't understand why this would ensure the order.

Moreover, I think there should be a way to express the dependency
accurately and automatically. I'm just not sure how. I guess it's the
system include <generated/...> that prevents the usual dep handling from
working.

BR,
Jani.


-- 
Jani Nikula, Intel

Re: [PATCH] drm/xe: Generate oob before compiling anything

[Lucas De Marchi]

On Thu, Apr 25, 2024 at 02:19:45PM GMT, Jani Nikula wrote:
>On Thu, 11 Apr 2024, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
>> Instead of keep adding more dependencies as WAs are needed in different
>> places of the driver, just add a rule with all the objects so the code
>> generation happens before anything else.
>>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>> ---
>>  drivers/gpu/drm/xe/Makefile | 16 ++++------------
>>  1 file changed, 4 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
>> index 6015c9e41f24..a5809b3ec8a5 100644
>> --- a/drivers/gpu/drm/xe/Makefile
>> +++ b/drivers/gpu/drm/xe/Makefile
>> @@ -46,18 +46,6 @@ $(obj)/generated/%_wa_oob.c $(obj)/generated/%_wa_oob.h: $(obj)/xe_gen_wa_oob \
>>  		 $(srctree)/$(src)/xe_wa_oob.rules
>>  	$(call cmd,wa_oob)
>>
>> -uses_generated_oob := \
>> -	$(obj)/xe_gsc.o \
>> -	$(obj)/xe_guc.o \
>> -	$(obj)/xe_guc_ads.o \
>> -	$(obj)/xe_migrate.o \
>> -	$(obj)/xe_ring_ops.o \
>> -	$(obj)/xe_vm.o \
>> -	$(obj)/xe_wa.o \
>> -	$(obj)/xe_ttm_stolen_mgr.o
>> -
>> -$(uses_generated_oob): $(generated_oob)
>> -
>>  # Please keep these build lists sorted!
>>
>>  # core driver code
>> @@ -319,3 +307,7 @@ quiet_cmd_hdrtest = HDRTEST $(patsubst %.hdrtest,%.h,$@)
>>
>>  $(obj)/%.hdrtest: $(src)/%.h FORCE
>>  	$(call if_changed_dep,hdrtest)
>> +
>> +# make sure the generated header is done before any
>> +# compilation unit is attempted
>> +$(xe-y): $(generated_oob)
>
>I'm sorry, I don't understand why this would ensure the order.

$(xe-y) should expand to every single .o listed here. So this is adding
a dependency "every single .o depends on the generated files. Or at
least it was the intention and appeared to work fine for x86, but no UM.
Before we had:

$(uses_generated_oob): $(generated_oob)

with $(uses_generated_oob) being crafted by hand.

>
>Moreover, I think there should be a way to express the dependency
>accurately and automatically. I'm just not sure how. I guess it's the
>system include <generated/...> that prevents the usual dep handling from
>working.

why? there's nothing special with "generated" or instructing the
compiler to look at the include paths (without looking at the current
dir)

Lucas De Marchi

>
>BR,
>Jani.
>
>
>-- 
>Jani Nikula, Intel