[PATCH] x86/Kconfig: rust: Patchable function Rust compat

[PATCH] x86/Kconfig: rust: Patchable function Rust compat

[Matthew Maurer]

Rust doesn't yet support patchable entry, but likely will soon. Disable
function padding when Rust is used but doesn't support it, and propagate
the flag when it does.

Signed-off-by: Matthew Maurer <mmaurer@google.com>
---
 arch/x86/Kconfig  | 6 +++++-
 arch/x86/Makefile | 2 ++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 18b9fb7df95b..e9f1814217b5 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2452,6 +2452,10 @@ config CC_HAS_RETURN_THUNK
 config CC_HAS_ENTRY_PADDING
 	def_bool $(cc-option,-fpatchable-function-entry=16,16)
 
+config RUSTC_HAS_ENTRY_PADDING
+	depends RUST
+	def_bool $(rs-option,-Zpatchable-function-entry=16,16)
+
 config FUNCTION_PADDING_CFI
 	int
 	default 59 if FUNCTION_ALIGNMENT_64B
@@ -2469,7 +2473,7 @@ config FUNCTION_PADDING_BYTES
 
 config CALL_PADDING
 	def_bool n
-	depends on CC_HAS_ENTRY_PADDING && OBJTOOL
+	depends on CC_HAS_ENTRY_PADDING && (!RUST || RUST_HAS_ENTRY_PADDING) && OBJTOOL
 	select FUNCTION_ALIGNMENT_16B
 
 config FINEIBT
diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 1a068de12a56..0228af62742e 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -211,7 +211,9 @@ endif
 
 ifdef CONFIG_CALL_PADDING
 PADDING_CFLAGS := -fpatchable-function-entry=$(CONFIG_FUNCTION_PADDING_BYTES),$(CONFIG_FUNCTION_PADDING_BYTES)
+PADDING_RUSTFLAGS := -Zpatchable-function-entry=$(CONFIG_FUNCTION_PADDING_BYTES),$(CONFIG_FUNCTION_PADDING_BYTES)
 KBUILD_CFLAGS += $(PADDING_CFLAGS)
+KBUILD_RUSTFLAGS += $(PADDING_RUSTFLAGS)
 export PADDING_CFLAGS
 endif
 
-- 
2.43.0.472.g3155946c3a-goog

Re: [PATCH] x86/Kconfig: rust: Patchable function Rust compat

[Miguel Ojeda]

On Mon, Dec 11, 2023 at 4:08 PM Matthew Maurer <mmaurer@google.com> wrote:
>
> +       def_bool $(rs-option,-Zpatchable-function-entry=16,16)

We don't have `rs-option` in mainline yet -- missing dependency? We
will likely eventually need it, but currently we only support a single
Rust version anyway, so we could add it (and the flag check itself)
when we upgrade (especially if it is going to be supported soon).

Speaking of which, I can't find the flag in upstream Rust (i.e.
outside the LLVM submodule), and I couldn't find a PR adding it. Could
you please add a `Link:` tag to the tracking issue / PR / ... if it is
submitted / when it is submitted? Or am I very confused?

Cheers,
Miguel

Re: [PATCH] x86/Kconfig: rust: Patchable function Rust compat

[Martin Rodriguez Reboredo]

On 12/11/23 12:07, Matthew Maurer wrote:
> Rust doesn't yet support patchable entry, but likely will soon. Disable
> function padding when Rust is used but doesn't support it, and propagate
> the flag when it does.
> 
> Signed-off-by: Matthew Maurer <mmaurer@google.com>
> ---
> [...]
> diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> index 1a068de12a56..0228af62742e 100644
> --- a/arch/x86/Makefile
> +++ b/arch/x86/Makefile
> @@ -211,7 +211,9 @@ endif
>   
>   ifdef CONFIG_CALL_PADDING
>   PADDING_CFLAGS := -fpatchable-function-entry=$(CONFIG_FUNCTION_PADDING_BYTES),$(CONFIG_FUNCTION_PADDING_BYTES)
> +PADDING_RUSTFLAGS := -Zpatchable-function-entry=$(CONFIG_FUNCTION_PADDING_BYTES),$(CONFIG_FUNCTION_PADDING_BYTES)

It seems that at a glance there's no discussion around this, neither an
issue, branch, PR or commit. Do you happen to have a link to it?

>   KBUILD_CFLAGS += $(PADDING_CFLAGS)
> +KBUILD_RUSTFLAGS += $(PADDING_RUSTFLAGS)
>   export PADDING_CFLAGS
>   endif
>   

Re: [PATCH] x86/Kconfig: rust: Patchable function Rust compat

[Matthew Maurer]

On Mon, Dec 11, 2023 at 7:36 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Mon, Dec 11, 2023 at 4:08 PM Matthew Maurer <mmaurer@google.com> wrote:
> >
> > +       def_bool $(rs-option,-Zpatchable-function-entry=16,16)
>
> We don't have `rs-option` in mainline yet -- missing dependency? We
> will likely eventually need it, but currently we only support a single
> Rust version anyway, so we could add it (and the flag check itself)
> when we upgrade (especially if it is going to be supported soon).
Sorry, I just realized this was only in a local patch. I hadn't sent
it previously because,
as you pointed out, we currently only support one compiler revision.

I was taking this approach because Android's compilers can have patches
backported onto them when needed, so our 1.73.0 could have this flag
and make use
of it.
>
>
> Speaking of which, I can't find the flag in upstream Rust (i.e.
> outside the LLVM submodule), and I couldn't find a PR adding it. Could
> you please add a `Link:` tag to the tracking issue / PR / ... if it is
> submitted / when it is submitted? Or am I very confused?
I haven't uploaded it yet. I'm hoping to send it up later today. I can
wait until it's
uploaded for a v2 of the patch series so I can link to it directly.
>
> Cheers,
> Miguel

If I don't get the PR for `-Zpatchable-function-entry` done in a
timely fashion, I'll send
up an alternate version of this patch that just makes it depend on
!RUST, as this can
currently cause random runtime failures if features which assume
patchable entry are
used with Rust.

Re: Martin's comments (unfortunately they aren't on the same email so
I can't reply inline)
would you like me to file an issue against the R4L repository about
this before sending a v2?
I thought that repository was just for staging/discussion, and this
didn't seem likely to need it.

Re: [PATCH] x86/Kconfig: rust: Patchable function Rust compat

[Martin Rodriguez Reboredo]

On 12/11/23 13:11, Matthew Maurer wrote:
> [...]
> Sorry, I just realized this was only in a local patch. I hadn't sent
> it previously because,
> as you pointed out, we currently only support one compiler revision.
> 
> I was taking this approach because Android's compilers can have patches
> backported onto them when needed, so our 1.73.0 could have this flag
> and make use
> of it.
>>
>>
>> Speaking of which, I can't find the flag in upstream Rust (i.e.
>> outside the LLVM submodule), and I couldn't find a PR adding it. Could
>> you please add a `Link:` tag to the tracking issue / PR / ... if it is
>> submitted / when it is submitted? Or am I very confused?
> I haven't uploaded it yet. I'm hoping to send it up later today. I can
> wait until it's
> uploaded for a v2 of the patch series so I can link to it directly.

If you can send the patch then it'll be wonderful, no hurries though.

> [...]
> 
> Re: Martin's comments (unfortunately they aren't on the same email so
> I can't reply inline)
> would you like me to file an issue against the R4L repository about
> this before sending a v2?
> I thought that repository was just for staging/discussion, and this
> didn't seem likely to need it.
> 

About issues, commits or PRs I was referring to issues in
`rust-lang/rust`. When we use unstable compiler features in R4L they
get tracked in [1].

Link: https://github.com/Rust-for-Linux/linux/issues/355 [1]

Re: [PATCH] x86/Kconfig: rust: Patchable function Rust compat

[Miguel Ojeda]

On Mon, Dec 11, 2023 at 5:11 PM Matthew Maurer <mmaurer@google.com> wrote:
>
> Sorry, I just realized this was only in a local patch. I hadn't sent
> it previously because,
> as you pointed out, we currently only support one compiler revision.

No worries at all, and thanks!

> Re: Martin's comments (unfortunately they aren't on the same email so
> I can't reply inline)
> would you like me to file an issue against the R4L repository about
> this before sending a v2?
> I thought that repository was just for staging/discussion, and this
> didn't seem likely to need it.

What we normally do is putting them in the "wanted features" sub-lists
(to track implementation/merging into upstream Rust). Then, when we
start using them, we put them into the main list at
https://github.com/Rust-for-Linux/linux/issues/2 (to track
stabilization).

Then we also add version numbers when they get implemented/stabilized,
and then on the version upgrades I like to go through the lists and
mention things in the commit message that got solved (that we needed).

Therefore, since the feature is still getting implemented, I have
added it now to https://github.com/Rust-for-Linux/linux/issues/355 --
when the PR / Tracking Issue are created, we can add the links.

Cheers,
Miguel

Re: [PATCH] x86/Kconfig: rust: Patchable function Rust compat

[Miguel Ojeda]

On Mon, Dec 11, 2023 at 5:23 PM Martin Rodriguez Reboredo
<yakoyoku@gmail.com> wrote:
>
> About issues, commits or PRs I was referring to issues in
> `rust-lang/rust`. When we use unstable compiler features in R4L they
> get tracked in [1].
>
> Link: https://github.com/Rust-for-Linux/linux/issues/355 [1]

Almost right :) In that one we put compiler things that we "want",
i.e. they may not even implemented in upstream Rust. When we start
using them (or we intend to use it and they have a
flag/feature/cfg...) it is when we (also) put them in the "main" list
of unstable features in issue #2.

Cheers,
Miguel

Re: [PATCH] x86/Kconfig: rust: Patchable function Rust compat

[kernel test robot]

Hi Matthew,

kernel test robot noticed the following build errors:

[auto build test ERROR on tip/x86/core]
[also build test ERROR on peterz-queue/sched/core tip/master tip/auto-latest linus/master v6.7-rc5 next-20231212]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Matthew-Maurer/x86-Kconfig-rust-Patchable-function-Rust-compat/20231211-230934
base:   tip/x86/core
patch link:    https://lore.kernel.org/r/20231211150753.293883-1-mmaurer%40google.com
patch subject: [PATCH] x86/Kconfig: rust: Patchable function Rust compat
config: x86_64-rhel-8.3-bpf (attached as .config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231212/202312122055.m7s1ccIE-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202312122055.m7s1ccIE-lkp@intel.com/

All errors (new ones prefixed by >>):

>> arch/x86/Kconfig:2443: syntax error
   arch/x86/Kconfig:2442: invalid statement
   arch/x86/Kconfig:2443: invalid statement
   make[3]: *** [scripts/kconfig/Makefile:77: oldconfig] Error 1
   make[2]: *** [Makefile:685: oldconfig] Error 2
   make[1]: *** [Makefile:234: __sub-make] Error 2
   make[1]: Target 'oldconfig' not remade because of errors.
   make: *** [Makefile:234: __sub-make] Error 2
   make: Target 'oldconfig' not remade because of errors.
--
>> arch/x86/Kconfig:2443: syntax error
   arch/x86/Kconfig:2442: invalid statement
   arch/x86/Kconfig:2443: invalid statement
   make[3]: *** [scripts/kconfig/Makefile:77: olddefconfig] Error 1
   make[2]: *** [Makefile:685: olddefconfig] Error 2
   make[1]: *** [Makefile:234: __sub-make] Error 2
   make[1]: Target 'olddefconfig' not remade because of errors.
   make: *** [Makefile:234: __sub-make] Error 2
   make: Target 'olddefconfig' not remade because of errors.


vim +2443 arch/x86/Kconfig

  2431	
  2432	config CC_HAS_SLS
  2433		def_bool $(cc-option,-mharden-sls=all)
  2434	
  2435	config CC_HAS_RETURN_THUNK
  2436		def_bool $(cc-option,-mfunction-return=thunk-extern)
  2437	
  2438	config CC_HAS_ENTRY_PADDING
  2439		def_bool $(cc-option,-fpatchable-function-entry=16,16)
  2440	
  2441	config RUSTC_HAS_ENTRY_PADDING
  2442		depends RUST
> 2443		def_bool $(rs-option,-Zpatchable-function-entry=16,16)
  2444	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki