linux-kernel - Re: [PATCH 1/1] arm: rust: Enable Rust support for ARMv7

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0a45a87f-84d1-4fb7-adaf-c612af68cd40@gmail.com>
Date: Sat, 8 Jun 2024 15:57:36 +0200
From: Christian Schrefl <chrisi.schrefl@...il.com>
To: Miguel Ojeda <miguel.ojeda.sandonis@...il.com>,
 Jamie Cunliffe <jamie.cunliffe@....com>
Cc: linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 rust-for-linux@...r.kernel.org, Miguel Ojeda <ojeda@...nel.org>,
 Sven Van Asbroeck <thesven73@...il.com>
Subject: Re: [PATCH 1/1] arm: rust: Enable Rust support for ARMv7

On 08.06.24 2:16 PM, Miguel Ojeda wrote:
> On Sat, Jun 8, 2024 at 12:30 AM Christian Schrefl
> <chrisi.schrefl@...il.com> wrote:
>>
>> This is based on the code by Sven Van Asbroeck from the original
>> rust branch and inspired by the AArch version by Jamie Cunliffe.
> 
> Thanks for mentioning that!
> 
> (AArch64 perhaps?)

Ah yes I meant AArch64. Fixed it locally.

>> I have tested the rust samples and a custom simple MMIO module
>> on on hardware (De1SoC FPGA + Arm A9 CPU).
> 
> Very nice to have tested it in real hardware, thanks!
> 
> (Duplicate "on on").

I fixed that locally, but forgot to run format-patch again, sorry! 

>> +``arm``        Maintained        ARMv7 Little Endian only.
> 
> How hard would it be to test and maintain other versions, eventually?
> I met Jamie recently, he may have some ideas here.

I think it would just be setting the correct rust target,
but I have no hardware to test it and I think its best to
only do v7 for the initial support.

>> -BINDGEN_TARGET_arm64   := aarch64-linux-gnu
>> +BINDGEN_TARGET_arm64:= aarch64-linux-gnu
> 
> Spurious change?

I've changed that so the := aligns with the others, but I can
remove that for v2 if you want

>> +# Depending on how the architecute defines ARCH_SLAB_MINALIGN, bindgen might generate a binding.
>> +# Disable this here as there is a const that will always be generated in bindings_helper.c
> 
> Typo in "architecute". Also, I guess you mean `.h` instead. Please use
> Markdown formatting for consistency with the rest of the file.

Fixed.

>> +        panic!("arm uses the builtin rustc target");
> 
> Since ARM64 puts the name of the target in the message, it would be
> nice to do the same (or perhaps it can be removed from the ARM64 one
> otherwise).

I can add the target here but then the message would have to depend
on the sub architecture if we support v6 in the future. 

> Cheers,
> Miguel