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 <[email protected]>
---
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
On Mon, Dec 11, 2023 at 4:08 PM Matthew Maurer <[email protected]> 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
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 <[email protected]>
> ---
> [...]
> 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
>
On Mon, Dec 11, 2023 at 7:36 AM Miguel Ojeda
<[email protected]> wrote:
>
> On Mon, Dec 11, 2023 at 4:08 PM Matthew Maurer <[email protected]> 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.
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]
On Mon, Dec 11, 2023 at 5:11 PM Matthew Maurer <[email protected]> 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
On Mon, Dec 11, 2023 at 5:23 PM Martin Rodriguez Reboredo
<[email protected]> 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
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/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
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