RE: [PATCH 0/1] Add XSAVE layout description to Core files for debuggers to support varying XSAVE layouts

From: Willgerodt, Felix
Date: Fri Mar 15 2024 - 04:43:45 EST


> -----Original Message-----
> From: Borislav Petkov <bp@xxxxxxxxx>
> Sent: Donnerstag, 14. März 2024 17:34
> To: Willgerodt, Felix <felix.willgerodt@xxxxxxxxx>
> Cc: Vignesh Balasubramanian <vigbalas@xxxxxxx>; linux-
> kernel@xxxxxxxxxxxxxxx; linux-toolchains@xxxxxxxxxxxxxxx;
> mpe@xxxxxxxxxxxxxx; npiggin@xxxxxxxxx; christophe.leroy@xxxxxxxxxx;
> aneesh.kumar@xxxxxxxxxx; naveen.n.rao@xxxxxxxxxxxxx;
> ebiederm@xxxxxxxxxxxx; keescook@xxxxxxxxxxxx; x86@xxxxxxxxxx;
> linuxppc-dev@xxxxxxxxxxxxxxxx; linux-mm@xxxxxxxxx;
> jinisusan.george@xxxxxxx; matz@xxxxxxx; binutils@xxxxxxxxxxxxxx;
> jhb@xxxxxxxxxxx
> Subject: Re: [PATCH 0/1] Add XSAVE layout description to Core files for
> debuggers to support varying XSAVE layouts
>
> On Thu, Mar 14, 2024 at 04:25:44PM +0000, Willgerodt, Felix wrote:
> > I am wondering if it wouldn't be easier for everyone if corefiles would just
> > contain space for all possible XSAVE components?
>
> You mean we should shuffle out from the kernel 8K of AMX state even if
> nothing uses it or the machine doesn't even support it?
>
> That's silly.

I don't think it is so silly ;) Let me elaborate.

I was mostly trying to suggest an "easy fix" for the MPX issue.
I also said that we could drop features from the end of the list if the CPU
doesn't support them. Yes it is still wasteful, but kind of was the status quo
in the past afaik. And yes, if new states come after AMX it could get more wasteful.

Though in another email here Dave said that the offsets in XSAVE
are "fixed in practice". That makes me wonder. Even if we add CPUID to corefiles,
will Intel CPUs benefit? (I am not saying it isn't worth changing even if Intel CPUs don't.)
E.g. will we actually get rid of the 8k that you are complaining about?
I don't know the answer to be honest. If XSAVE offsets are "fixed in practice",
I don't see how we would benefit.

And how would you check that "nothing uses AMX", if the state exist according
to CPUID?

> Please have a look at this:
>
> +struct xfeat_component {
> + u32 xfeat_type;
> + u32 xfeat_sz;
> + u32 xfeat_off;
> + u32 xfeat_flags;
> +} __packed;
>
> What is wrong with having a blob of such xfeat_component things
> describing the XSTATE buffer and parsing it in gdb?

I didn't say it is wrong or that I am opposing it. In fact CPUID in
corefiles could be useful for other scenarios as well.
Though all consumers will need to add to their implementation.
I assume LLDB and GDB aren't the only consumers.

Felix
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928