[PATCH 1/2] rust: pass correct target to bindgen on Usermode Linux

Thomas Weißschuh posted 2 patches 1 year ago
[PATCH 1/2] rust: pass correct target to bindgen on Usermode Linux
Posted by Thomas Weißschuh 1 year ago
Usermode Linux uses "um" as primary architecture name and the underlying
physical architecture is provided in "SUBARCH".
Resolve the target architecture flags through that underlying architecture.
This is the same pattern as used by scripts/Makefile.clang from which
the bindgen flags are derived.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 rust/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/rust/Makefile b/rust/Makefile
index 8fcfd60447bc89ba2c66a4f341288db2387b0956..a94fafb91d7d743c6c1b2248479c0d723964e5c4 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -245,6 +245,7 @@ bindgen_skip_c_flags := -mno-fp-ret-in-387 -mpreferred-stack-boundary=% \
 # Derived from `scripts/Makefile.clang`.
 BINDGEN_TARGET_x86	:= x86_64-linux-gnu
 BINDGEN_TARGET_arm64	:= aarch64-linux-gnu
+BINDGEN_TARGET_um	:= $(BINDGEN_TARGET_$(SUBARCH))
 BINDGEN_TARGET		:= $(BINDGEN_TARGET_$(SRCARCH))
 
 # All warnings are inhibited since GCC builds are very experimental,

-- 
2.48.1

Re: [PATCH 1/2] rust: pass correct target to bindgen on Usermode Linux
Posted by David Gow 10 months, 4 weeks ago
On Sat, 8 Feb 2025 at 21:32, Thomas Weißschuh <linux@weissschuh.net> wrote:
>
> Usermode Linux uses "um" as primary architecture name and the underlying
> physical architecture is provided in "SUBARCH".
> Resolve the target architecture flags through that underlying architecture.
> This is the same pattern as used by scripts/Makefile.clang from which
> the bindgen flags are derived.
>
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---

Is there anything holding this up for the upcoming merge window?

Miguel: I'm assuming you'd rather take this (and possibly [1] as well)
via Rust, but if it goes in via the uml tree, that'd be fine by me,
too.

Cheers,
-- David

[1]: https://lore.kernel.org/linux-um/20250210105353.2238769-2-davidgow@google.com/
Re: [PATCH 1/2] rust: pass correct target to bindgen on Usermode Linux
Posted by Miguel Ojeda 10 months, 4 weeks ago
On Tue, Mar 18, 2025 at 9:07 AM David Gow <davidgow@google.com> wrote:
>
> Is there anything holding this up for the upcoming merge window?
>
> Miguel: I'm assuming you'd rather take this (and possibly [1] as well)
> via Rust, but if it goes in via the uml tree, that'd be fine by me,
> too.

We try to get arch maintainers involved (and everyone else, of
course), i.e. it is up to them. But I see only the list was Cc'd, not
them directly, for some reason -- doing it here.

Ideally, maintainers get involved and pick their own Rust-related
patches, but I can also do so with an Acked-by from them, which would
be nice.

If they don't say anything, then since you have tested it (and you are
a main um user due to KUnit), that should be enough, but then I would
suggest we do it after -rc1 to give them time to react.

Thanks!

Cheers,
Miguel
Re: [PATCH 1/2] rust: pass correct target to bindgen on Usermode Linux
Posted by Johannes Berg 10 months, 4 weeks ago
On Tue, 2025-03-18 at 11:10 +0100, Miguel Ojeda wrote:
> On Tue, Mar 18, 2025 at 9:07 AM David Gow <davidgow@google.com> wrote:
> > 
> > Is there anything holding this up for the upcoming merge window?
> > 
> > Miguel: I'm assuming you'd rather take this (and possibly [1] as well)
> > via Rust, but if it goes in via the uml tree, that'd be fine by me,
> > too.
> 
> We try to get arch maintainers involved (and everyone else, of
> course), i.e. it is up to them. But I see only the list was Cc'd, not
> them directly, for some reason -- doing it here.

The list is fine, but the patch wasn't even CC'ed there, so we don't
have it in our patchwork:
https://patchwork.ozlabs.org/project/linux-um/list/

> Ideally, maintainers get involved and pick their own Rust-related
> patches, but I can also do so with an Acked-by from them, which would
> be nice.

I was just picking up um patches, but given that it was a series, and
changes rust/ rather than arch/um/, I think it's probably better if you
do it, so:

Acked-by: Johannes Berg <johannes@sipsolutions.net>

johannes
Re: [PATCH 1/2] rust: pass correct target to bindgen on Usermode Linux
Posted by Miguel Ojeda 10 months, 4 weeks ago
On Tue, Mar 18, 2025 at 11:14 AM Johannes Berg
<johannes@sipsolutions.net> wrote:
>
> The list is fine, but the patch wasn't even CC'ed there, so we don't
> have it in our patchwork:
> https://patchwork.ozlabs.org/project/linux-um/list/

Ah, right, David added it.

> I was just picking up um patches, but given that it was a series, and
> changes rust/ rather than arch/um/, I think it's probably better if you
> do it, so:
>
> Acked-by: Johannes Berg <johannes@sipsolutions.net>

Thanks for the very quick reply!

I guess they could be picked independently, but I am happy to pick both.

Cheers,
Miguel
Re: [PATCH 1/2] rust: pass correct target to bindgen on Usermode Linux
Posted by David Gow 12 months ago
On Sat, 8 Feb 2025 at 21:32, Thomas Weißschuh <linux@weissschuh.net> wrote:
>
> Usermode Linux uses "um" as primary architecture name and the underlying
> physical architecture is provided in "SUBARCH".
> Resolve the target architecture flags through that underlying architecture.
> This is the same pattern as used by scripts/Makefile.clang from which
> the bindgen flags are derived.
>
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---

(+cc linux-um, but I assume this will probably go in via the Rust tree, anyway.)

Thanks very much: this is enough to get Rust-for-Linux working with
gcc under 64-bit UML on my system.

However, this is actually a bit of a coincidence -- and there are
still some issues with 32-bit UML -- as the UML Rust flags are
currently conditionally set if CC_IS_CLANG. This is my fault (it was
to work around some bugs with older gcc), and I've sent a patch[1] to
fix it. (Though note that 32-bit UML/Rust still hits issues with
atomics in the block driver, so you'll need to disable that for now.)

Regardless, this is a significant improvement, thanks!

Reviewed-by: David Gow <davidgow@googl.ecom>

Thanks,
-- David

[1]: https://lore.kernel.org/rust-for-linux/20250210105353.2238769-2-davidgow@google.com/


>  rust/Makefile | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/rust/Makefile b/rust/Makefile
> index 8fcfd60447bc89ba2c66a4f341288db2387b0956..a94fafb91d7d743c6c1b2248479c0d723964e5c4 100644
> --- a/rust/Makefile
> +++ b/rust/Makefile
> @@ -245,6 +245,7 @@ bindgen_skip_c_flags := -mno-fp-ret-in-387 -mpreferred-stack-boundary=% \
>  # Derived from `scripts/Makefile.clang`.
>  BINDGEN_TARGET_x86     := x86_64-linux-gnu
>  BINDGEN_TARGET_arm64   := aarch64-linux-gnu
> +BINDGEN_TARGET_um      := $(BINDGEN_TARGET_$(SUBARCH))
>  BINDGEN_TARGET         := $(BINDGEN_TARGET_$(SRCARCH))
>
>  # All warnings are inhibited since GCC builds are very experimental,
>
> --
> 2.48.1
>
>