Received-SPF: pass (google.com: domain of linux-kernel+bounces-207345-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Sun, 9 Jun 2024 13:22:40 +0200
From: Borislav Petkov <bp@alien8.de>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Anvin <hpa@zytor.com>, Ingo Molnar <mingo@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	Josh Poimboeuf <jpoimboe@kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	the arch/x86 maintainers <x86@kernel.org>,
	linux-arch <linux-arch@vger.kernel.org>
Subject: Re: [PATCH] x86: add 'runtime constant' infrastructure
Message-ID: <20240609112240.GBZmWQgNQXguD_8Nc8@fat_crate.local>
References: <20240608193504.429644-2-torvalds@linux-foundation.org>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20240608193504.429644-2-torvalds@linux-foundation.org>

On Sat, Jun 08, 2024 at 12:35:05PM -0700, Linus Torvalds wrote:
> Ingo / Peter / Borislav - I enabled this for 32-bit x86 too, because it
> was literally trivial (had to remove a "q" from "movq").  I did a
> test-build and it looks find, but I didn't actually try to boot it. 

Will do once you have your final version. I still have an Atom, 32-bit
only laptop lying around here.

> +#define runtime_const_ptr(sym) ({				\
> +	typeof(sym) __ret;					\
> +	asm("mov %1,%0\n1:\n"					\
> +		".pushsection runtime_ptr_" #sym ",\"a\"\n\t"	\
> +		".long 1b - %c2 - .\n\t"			\
> +		".popsection"					\
> +		:"=r" (__ret)					\
> +		:"i" ((unsigned long)0x0123456789abcdefull),	\
> +		 "i" (sizeof(long)));				\
> +	__ret; })

You might wanna use asm symbolic names for the operands so that it is
more readable:

#define runtime_const_ptr(sym) ({                                               \
        typeof(sym) __ret;                                                      \
        asm("mov %[constant] ,%[__ret]\n1:\n"                                   \
                ".pushsection runtime_ptr_" #sym ",\"a\"\n\t"                   \
                ".long 1b - %c[sizeoflong] - .\n\t"                             \
                ".popsection"                                                   \
                : [__ret] "=r" (__ret)                                          \
                : [constant] "i" ((unsigned long)0x0123456789abcdefull),        \
                  [sizeoflong] "i" (sizeof(long)));                             \
        __ret; })

For example.

> +// The 'typeof' will create at _least_ a 32-bit type, but
> +// will happily also take a bigger type and the 'shrl' will
> +// clear the upper bits

Can we pls use the multiline comments, like you do below in the same
file.

Otherwise, it looks ok to me and it boots in a guest.

I'll take the final version for a spin on real hw in a couple of days,
once the review dust settles.

Thx.

-- 
Regards/Gruss,
    Boris.

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