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

[Borislav Petkov]

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.

https://people.kernel.org/tglx/notes-about-netiquette