Re: [PATCH v7 38/41] x86/fpu: Add helper for initing features

Borislav Petkov
2023-03-13T11:03:49Z

On Mon, Mar 13, 2023 at 02:45:08AM +0000, Edgecombe, Rick P wrote:
> These two are from the existing code. Basically they get extracted into
> a new function.

I know but you can fix them while at it.

> I did it up, and it makes the caller code cleaner. But I'm not sure
> what to think of it. Is this not mixing two operations together? Today
> get_xsave_addr() pretty much just gets a buffer offset with some
> checks. Now it would compute the offset and also silently go off and
> changes the buffer.

Ok, so why don't you write the call site this way instead:

cetregs = get_xsave_addr(xsave, XFEATURE_CET_USER);
if (!cetregs) {
if (xfeature_saved(xsave, XFEATURE_CET_USER)) {
WARN("something's wrong with this buffer")
return ...;
}

/* Not saved, initialize it */
init_xfeature(xsave, XFEATURE_CET_USER));
}

cetregs = get_xsave_addr(xsave, XFEATURE_CET_USER);
if (!cetregs) {
WARN_ON("WTF")
return -ENODEV;
}

Now it is clear what happens and it is a common code pattern of trying
to get something and initializing it if it wasn't initialized yet, and
then retrying...

Hmm?

--
Regards/Gruss,
Boris.