Sync tools headers with the kernel source

[PATCH 1/9] tools/include: Sync uapi/drm/i915_drm.h with the kernel sources

Posted by Namhyung Kim 1 year, 10 months ago

To pick up changes from:

   b112364867499 ("drm/i915: Add GuC submission interface version query")
   5cf0fbf763741 ("drm/i915: Add some boring kerneldoc")

This should be used to beautify DRM syscall arguments and it addresses
these tools/perf build warnings:

  Warning: Kernel ABI header differences:
    diff -u tools/include/uapi/drm/i915_drm.h include/uapi/drm/i915_drm.h

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: David Airlie <airlied@gmail.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/include/uapi/drm/i915_drm.h | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/tools/include/uapi/drm/i915_drm.h b/tools/include/uapi/drm/i915_drm.h
index fd4f9574d177..2ee338860b7e 100644
--- a/tools/include/uapi/drm/i915_drm.h
+++ b/tools/include/uapi/drm/i915_drm.h
@@ -3013,6 +3013,7 @@ struct drm_i915_query_item {
 	 *  - %DRM_I915_QUERY_MEMORY_REGIONS (see struct drm_i915_query_memory_regions)
 	 *  - %DRM_I915_QUERY_HWCONFIG_BLOB (see `GuC HWCONFIG blob uAPI`)
 	 *  - %DRM_I915_QUERY_GEOMETRY_SUBSLICES (see struct drm_i915_query_topology_info)
+	 *  - %DRM_I915_QUERY_GUC_SUBMISSION_VERSION (see struct drm_i915_query_guc_submission_version)
 	 */
 	__u64 query_id;
 #define DRM_I915_QUERY_TOPOLOGY_INFO		1
@@ -3021,6 +3022,7 @@ struct drm_i915_query_item {
 #define DRM_I915_QUERY_MEMORY_REGIONS		4
 #define DRM_I915_QUERY_HWCONFIG_BLOB		5
 #define DRM_I915_QUERY_GEOMETRY_SUBSLICES	6
+#define DRM_I915_QUERY_GUC_SUBMISSION_VERSION	7
 /* Must be kept compact -- no holes and well documented */
 
 	/**
@@ -3566,6 +3568,20 @@ struct drm_i915_query_memory_regions {
 	struct drm_i915_memory_region_info regions[];
 };
 
+/**
+ * struct drm_i915_query_guc_submission_version - query GuC submission interface version
+ */
+struct drm_i915_query_guc_submission_version {
+	/** @branch: Firmware branch version. */
+	__u32 branch;
+	/** @major: Firmware major version. */
+	__u32 major;
+	/** @minor: Firmware minor version. */
+	__u32 minor;
+	/** @patch: Firmware patch version. */
+	__u32 patch;
+};
+
 /**
  * DOC: GuC HWCONFIG blob uAPI
  *
-- 
2.44.0.478.gd926399ef9-goog

Re: [PATCH 1/9] tools/include: Sync uapi/drm/i915_drm.h with the kernel sources

Posted by Jani Nikula 1 year, 10 months ago

On Mon, 08 Apr 2024, Namhyung Kim <namhyung@kernel.org> wrote:
> To pick up changes from:
>
>    b112364867499 ("drm/i915: Add GuC submission interface version query")
>    5cf0fbf763741 ("drm/i915: Add some boring kerneldoc")
>
> This should be used to beautify DRM syscall arguments and it addresses
> these tools/perf build warnings:
>
>   Warning: Kernel ABI header differences:
>     diff -u tools/include/uapi/drm/i915_drm.h include/uapi/drm/i915_drm.h

All these years and I never realized there are header copies
there. But... why copies?

BR,
Jani.


-- 
Jani Nikula, Intel

Re: [PATCH 1/9] tools/include: Sync uapi/drm/i915_drm.h with the kernel sources

Posted by Ingo Molnar 1 year, 10 months ago

* Jani Nikula <jani.nikula@linux.intel.com> wrote:

> On Mon, 08 Apr 2024, Namhyung Kim <namhyung@kernel.org> wrote:
> > To pick up changes from:
> >
> >    b112364867499 ("drm/i915: Add GuC submission interface version query")
> >    5cf0fbf763741 ("drm/i915: Add some boring kerneldoc")
> >
> > This should be used to beautify DRM syscall arguments and it addresses
> > these tools/perf build warnings:
> >
> >   Warning: Kernel ABI header differences:
> >     diff -u tools/include/uapi/drm/i915_drm.h include/uapi/drm/i915_drm.h
> 
> All these years and I never realized there are header copies
> there. But... why copies?

It's better than all the alternatives we tried so far:

 - Symbolic links and direct #includes: this was the original approach but 
   was pushed back on from the kernel side, when tooling modified the 
   headers and broke them accidentally for kernel builds.

 - Duplicate self-defined ABI headers like glibc: double the maintenance 
   burden, double the chance for mistakes, plus there's no tech-driven 
   notification mechanism to look at new kernel side changes.

What we are doing now is a third option:

 - A software-enforced copy-on-write mechanism of kernel headers to 
   tooling, driven by non-fatal warnings on the tooling side build when 
   kernel headers get modified:

    Warning: Kernel ABI header differences:
      diff -u tools/include/uapi/drm/i915_drm.h include/uapi/drm/i915_drm.h
      diff -u tools/include/uapi/linux/fs.h include/uapi/linux/fs.h
      diff -u tools/include/uapi/linux/kvm.h include/uapi/linux/kvm.h
      ...

   The tooling policy is to always pick up the kernel side headers as-is, 
   and integate them into the tooling build. The warnings above serve as a 
   notification to tooling maintainers that there's changes on the kernel 
   side.

We've been using this for many years now, and it might seem hacky, but 
works surprisingly well.

Does this make sense to you?

Thanks,

	Ingo

Re: [PATCH 1/9] tools/include: Sync uapi/drm/i915_drm.h with the kernel sources

Posted by Jani Nikula 1 year, 10 months ago

On Tue, 09 Apr 2024, Ingo Molnar <mingo@kernel.org> wrote:
> * Jani Nikula <jani.nikula@linux.intel.com> wrote:
>
>> On Mon, 08 Apr 2024, Namhyung Kim <namhyung@kernel.org> wrote:
>> > To pick up changes from:
>> >
>> >    b112364867499 ("drm/i915: Add GuC submission interface version query")
>> >    5cf0fbf763741 ("drm/i915: Add some boring kerneldoc")
>> >
>> > This should be used to beautify DRM syscall arguments and it addresses
>> > these tools/perf build warnings:
>> >
>> >   Warning: Kernel ABI header differences:
>> >     diff -u tools/include/uapi/drm/i915_drm.h include/uapi/drm/i915_drm.h
>> 
>> All these years and I never realized there are header copies
>> there. But... why copies?
>
> It's better than all the alternatives we tried so far:
>
>  - Symbolic links and direct #includes: this was the original approach but 
>    was pushed back on from the kernel side, when tooling modified the 
>    headers and broke them accidentally for kernel builds.
>
>  - Duplicate self-defined ABI headers like glibc: double the maintenance 
>    burden, double the chance for mistakes, plus there's no tech-driven 
>    notification mechanism to look at new kernel side changes.
>
> What we are doing now is a third option:
>
>  - A software-enforced copy-on-write mechanism of kernel headers to 
>    tooling, driven by non-fatal warnings on the tooling side build when 
>    kernel headers get modified:
>
>     Warning: Kernel ABI header differences:
>       diff -u tools/include/uapi/drm/i915_drm.h include/uapi/drm/i915_drm.h
>       diff -u tools/include/uapi/linux/fs.h include/uapi/linux/fs.h
>       diff -u tools/include/uapi/linux/kvm.h include/uapi/linux/kvm.h
>       ...
>
>    The tooling policy is to always pick up the kernel side headers as-is, 
>    and integate them into the tooling build. The warnings above serve as a 
>    notification to tooling maintainers that there's changes on the kernel 
>    side.
>
> We've been using this for many years now, and it might seem hacky, but 
> works surprisingly well.
>
> Does this make sense to you?

Yes, although there are probably pieces of the puzzle I'm missing.
Thanks for the explanation! (That might work almost as-is copied to
tools/include/uapi/README. ;)

It's also kind of funny to find this kind of back alleys of the kernel
repo I've never wandered to before.

BR,
Jani.

-- 
Jani Nikula, Intel

Re: [PATCH 1/9] tools/include: Sync uapi/drm/i915_drm.h with the kernel sources

Posted by Namhyung Kim 1 year, 10 months ago

Hello,

On Tue, Apr 9, 2024 at 3:14 AM Jani Nikula <jani.nikula@linux.intel.com> wrote:
>
> On Tue, 09 Apr 2024, Ingo Molnar <mingo@kernel.org> wrote:
> > * Jani Nikula <jani.nikula@linux.intel.com> wrote:
> >
> >> On Mon, 08 Apr 2024, Namhyung Kim <namhyung@kernel.org> wrote:
> >> > To pick up changes from:
> >> >
> >> >    b112364867499 ("drm/i915: Add GuC submission interface version query")
> >> >    5cf0fbf763741 ("drm/i915: Add some boring kerneldoc")
> >> >
> >> > This should be used to beautify DRM syscall arguments and it addresses
> >> > these tools/perf build warnings:
> >> >
> >> >   Warning: Kernel ABI header differences:
> >> >     diff -u tools/include/uapi/drm/i915_drm.h include/uapi/drm/i915_drm.h
> >>
> >> All these years and I never realized there are header copies
> >> there. But... why copies?
> >
> > It's better than all the alternatives we tried so far:
> >
> >  - Symbolic links and direct #includes: this was the original approach but
> >    was pushed back on from the kernel side, when tooling modified the
> >    headers and broke them accidentally for kernel builds.
> >
> >  - Duplicate self-defined ABI headers like glibc: double the maintenance
> >    burden, double the chance for mistakes, plus there's no tech-driven
> >    notification mechanism to look at new kernel side changes.
> >
> > What we are doing now is a third option:
> >
> >  - A software-enforced copy-on-write mechanism of kernel headers to
> >    tooling, driven by non-fatal warnings on the tooling side build when
> >    kernel headers get modified:
> >
> >     Warning: Kernel ABI header differences:
> >       diff -u tools/include/uapi/drm/i915_drm.h include/uapi/drm/i915_drm.h
> >       diff -u tools/include/uapi/linux/fs.h include/uapi/linux/fs.h
> >       diff -u tools/include/uapi/linux/kvm.h include/uapi/linux/kvm.h
> >       ...
> >
> >    The tooling policy is to always pick up the kernel side headers as-is,
> >    and integate them into the tooling build. The warnings above serve as a
> >    notification to tooling maintainers that there's changes on the kernel
> >    side.
> >
> > We've been using this for many years now, and it might seem hacky, but
> > works surprisingly well.
> >
> > Does this make sense to you?
>
> Yes, although there are probably pieces of the puzzle I'm missing.
> Thanks for the explanation! (That might work almost as-is copied to
> tools/include/uapi/README. ;)
>
> It's also kind of funny to find this kind of back alleys of the kernel
> repo I've never wandered to before.

I have some explanation in the cover letter of the series but I forgot
to mention that in each commit message.  Probably I can update the
explanation with Ingo's reply.

Thanks,
Namhyung

Re: [PATCH 1/9] tools/include: Sync uapi/drm/i915_drm.h with the kernel sources

Posted by Arnaldo Carvalho de Melo 1 year, 10 months ago

On Tue, Apr 09, 2024 at 08:58:55AM -0700, Namhyung Kim wrote:
> Hello,
> 
> On Tue, Apr 9, 2024 at 3:14 AM Jani Nikula <jani.nikula@linux.intel.com> wrote:
> >
> > On Tue, 09 Apr 2024, Ingo Molnar <mingo@kernel.org> wrote:
> > > * Jani Nikula <jani.nikula@linux.intel.com> wrote:
> > >
> > >> On Mon, 08 Apr 2024, Namhyung Kim <namhyung@kernel.org> wrote:
> > >> > To pick up changes from:
> > >> >
> > >> >    b112364867499 ("drm/i915: Add GuC submission interface version query")
> > >> >    5cf0fbf763741 ("drm/i915: Add some boring kerneldoc")
> > >> >
> > >> > This should be used to beautify DRM syscall arguments and it addresses
> > >> > these tools/perf build warnings:
> > >> >
> > >> >   Warning: Kernel ABI header differences:
> > >> >     diff -u tools/include/uapi/drm/i915_drm.h include/uapi/drm/i915_drm.h
> > >>
> > >> All these years and I never realized there are header copies
> > >> there. But... why copies?
> > >
> > > It's better than all the alternatives we tried so far:
> > >
> > >  - Symbolic links and direct #includes: this was the original approach but
> > >    was pushed back on from the kernel side, when tooling modified the
> > >    headers and broke them accidentally for kernel builds.
> > >
> > >  - Duplicate self-defined ABI headers like glibc: double the maintenance
> > >    burden, double the chance for mistakes, plus there's no tech-driven
> > >    notification mechanism to look at new kernel side changes.
> > >
> > > What we are doing now is a third option:
> > >
> > >  - A software-enforced copy-on-write mechanism of kernel headers to
> > >    tooling, driven by non-fatal warnings on the tooling side build when
> > >    kernel headers get modified:
> > >
> > >     Warning: Kernel ABI header differences:
> > >       diff -u tools/include/uapi/drm/i915_drm.h include/uapi/drm/i915_drm.h
> > >       diff -u tools/include/uapi/linux/fs.h include/uapi/linux/fs.h
> > >       diff -u tools/include/uapi/linux/kvm.h include/uapi/linux/kvm.h
> > >       ...
> > >
> > >    The tooling policy is to always pick up the kernel side headers as-is,
> > >    and integate them into the tooling build. The warnings above serve as a
> > >    notification to tooling maintainers that there's changes on the kernel
> > >    side.
> > >
> > > We've been using this for many years now, and it might seem hacky, but
> > > works surprisingly well.
> > >
> > > Does this make sense to you?
> >
> > Yes, although there are probably pieces of the puzzle I'm missing.
> > Thanks for the explanation! (That might work almost as-is copied to
> > tools/include/uapi/README. ;)
> >
> > It's also kind of funny to find this kind of back alleys of the kernel
> > repo I've never wandered to before.
> 
> I have some explanation in the cover letter of the series but I forgot
> to mention that in each commit message.  Probably I can update the
> explanation with Ingo's reply.

I think we can combine Ingo's with the reply I used and you adopted and
continue to have it when sending the update messages, probably keep it
in the cover letter (the combined text) and add a link to each
individual update:

"Please see tools/include/README.kernel-copies."

The recommendation that developers shouldn't update the copy seems
important to have there as well.

- Arnaldo