2022-09-16 04:15:35

by Vineet Gupta

[permalink] [raw]
Subject: [PATCH] riscv: ztso: disallow elf binaries needing TSO

As of now the software stack needs work to support ztso. Until that work
is finished, disallow binaries needing TSO.

This patch is needed to help ztso ratification and prolifiration of tso
bits in tooling.

Signed-off-by: Vineet Gupta <[email protected]>
---
arch/riscv/include/asm/elf.h | 11 ++++++++++-
arch/riscv/include/uapi/asm/elf.h | 2 ++
2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/include/asm/elf.h b/arch/riscv/include/asm/elf.h
index f53c40026c7a..37a47352f633 100644
--- a/arch/riscv/include/asm/elf.h
+++ b/arch/riscv/include/asm/elf.h
@@ -26,10 +26,19 @@

#define ELF_DATA ELFDATA2LSB

+/*
+ * Make sure the extensions are compatible.
+ *
+ * In the final incarnation this will get the features as provided by DT and
+ * make sure kernel build is compatible.
+ * For now disallow TSO built binaries.
+ */
+#define rv_ext_ok(x) (!((x)->e_flags & EF_RISCV_TSO))
+
/*
* This is used to ensure we don't load something for the wrong architecture.
*/
-#define elf_check_arch(x) ((x)->e_machine == EM_RISCV)
+#define elf_check_arch(x) ((x)->e_machine == EM_RISCV && rv_ext_ok())

#define CORE_DUMP_USE_REGSET
#define ELF_EXEC_PAGESIZE (PAGE_SIZE)
diff --git a/arch/riscv/include/uapi/asm/elf.h b/arch/riscv/include/uapi/asm/elf.h
index d696d6610231..fa9e4c52c7ac 100644
--- a/arch/riscv/include/uapi/asm/elf.h
+++ b/arch/riscv/include/uapi/asm/elf.h
@@ -32,6 +32,8 @@ typedef union __riscv_fp_state elf_fpregset_t;
#define ELF_RISCV_R_TYPE(r_info) ELF32_R_TYPE(r_info)
#endif

+#define EF_RISCV_TSO (1 << 3)
+
/*
* RISC-V relocation types
*/
--
2.34.1


2022-09-16 04:59:31

by Vineet Gupta

[permalink] [raw]
Subject: [PATCH v2] riscv: ztso: disallow elf binaries needing TSO

As of now the software stack needs work to support ztso. Until that work
is finished, disallow binaries needing TSO.

This patch is needed to help ztso ratification and prolifiration of tso
bits in tooling.

Signed-off-by: Vineet Gupta <[email protected]>
---
Changes since v1
- Build error (and boot tested on qemu)
- Improved the comments a bit
---
arch/riscv/include/asm/elf.h | 11 ++++++++++-
arch/riscv/include/uapi/asm/elf.h | 2 ++
2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/include/asm/elf.h b/arch/riscv/include/asm/elf.h
index f53c40026c7a..b6b4542b3039 100644
--- a/arch/riscv/include/asm/elf.h
+++ b/arch/riscv/include/asm/elf.h
@@ -26,10 +26,19 @@

#define ELF_DATA ELFDATA2LSB

+/*
+ * Make sure the elf being loaded is compatible with extensions.
+ *
+ * In the final incarnation this will get the extension list from DT and
+ * make sure elf can run on given hardware+kernel.
+ * For now disallow TSO built binaries.
+ */
+#define rv_ext_ok(x) (!((x)->e_flags & EF_RISCV_TSO))
+
/*
* This is used to ensure we don't load something for the wrong architecture.
*/
-#define elf_check_arch(x) ((x)->e_machine == EM_RISCV)
+#define elf_check_arch(x) ((x)->e_machine == EM_RISCV && rv_ext_ok(x))

#define CORE_DUMP_USE_REGSET
#define ELF_EXEC_PAGESIZE (PAGE_SIZE)
diff --git a/arch/riscv/include/uapi/asm/elf.h b/arch/riscv/include/uapi/asm/elf.h
index d696d6610231..fa9e4c52c7ac 100644
--- a/arch/riscv/include/uapi/asm/elf.h
+++ b/arch/riscv/include/uapi/asm/elf.h
@@ -32,6 +32,8 @@ typedef union __riscv_fp_state elf_fpregset_t;
#define ELF_RISCV_R_TYPE(r_info) ELF32_R_TYPE(r_info)
#endif

+#define EF_RISCV_TSO (1 << 3)
+
/*
* RISC-V relocation types
*/
--
2.34.1

2022-09-16 06:58:07

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2] riscv: ztso: disallow elf binaries needing TSO

Please don't send html mail to the list :(

2022-09-16 07:09:09

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2] riscv: ztso: disallow elf binaries needing TSO

On 16/09/2022 05:23, Vineet Gupta wrote:
> [You don't often get email from [email protected]. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> As of now the software stack needs work to support ztso. Until that work
> is finished, disallow binaries needing TSO.
>
> This patch is needed to help ztso ratification and prolifiration of tso
> bits in tooling.

I have to admit to being a little confused here, if Ztso is not ratified
why do we need to protect ourselves from it?

>
> Signed-off-by: Vineet Gupta <[email protected]>
> ---
> Changes since v1
> - Build error (and boot tested on qemu)
> - Improved the comments a bit
> ---
> arch/riscv/include/asm/elf.h | 11 ++++++++++-
> arch/riscv/include/uapi/asm/elf.h | 2 ++
> 2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/arch/riscv/include/asm/elf.h b/arch/riscv/include/asm/elf.h
> index f53c40026c7a..b6b4542b3039 100644
> --- a/arch/riscv/include/asm/elf.h
> +++ b/arch/riscv/include/asm/elf.h
> @@ -26,10 +26,19 @@
>
> #define ELF_DATA ELFDATA2LSB
>
> +/*
> + * Make sure the elf being loaded is compatible with extensions.
> + *
> + * In the final incarnation this will get the extension list from DT and
> + * make sure elf can run on given hardware+kernel.
> + * For now disallow TSO built binaries.
> + */
> +#define rv_ext_ok(x) (!((x)->e_flags & EF_RISCV_TSO))
> +
> /*
> * This is used to ensure we don't load something for the wrong architecture.
> */
> -#define elf_check_arch(x) ((x)->e_machine == EM_RISCV)
> +#define elf_check_arch(x) ((x)->e_machine == EM_RISCV && rv_ext_ok(x))
>
> #define CORE_DUMP_USE_REGSET
> #define ELF_EXEC_PAGESIZE (PAGE_SIZE)
> diff --git a/arch/riscv/include/uapi/asm/elf.h b/arch/riscv/include/uapi/asm/elf.h
> index d696d6610231..fa9e4c52c7ac 100644
> --- a/arch/riscv/include/uapi/asm/elf.h
> +++ b/arch/riscv/include/uapi/asm/elf.h
> @@ -32,6 +32,8 @@ typedef union __riscv_fp_state elf_fpregset_t;
> #define ELF_RISCV_R_TYPE(r_info) ELF32_R_TYPE(r_info)
> #endif
>
> +#define EF_RISCV_TSO (1 << 3)

s/EF/ELF ?

Thanks,
Conor.

> +
> /*
> * RISC-V relocation types
> */
> --
> 2.34.1
>
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv

2022-09-16 07:11:41

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2] riscv: ztso: disallow elf binaries needing TSO

On 16/09/2022 07:34, Conor Dooley wrote:
> On 16/09/2022 05:23, Vineet Gupta wrote:
>> [You don't often get email from [email protected]. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>>
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> As of now the software stack needs work to support ztso. Until that work
>> is finished, disallow binaries needing TSO.
>>
>> This patch is needed to help ztso ratification and prolifiration of tso
>> bits in tooling.
>
> I have to admit to being a little confused here, if Ztso is not ratified
> why do we need to protect ourselves from it?

Also, since this is not marked as a fix, why would we not just apply the
patchset from Palmer that looks like a more complete version of this
patch:
https://lore.kernel.org/linux-riscv/[email protected]/

Maybe you could offer an R-b or some comments on that patch instead?

Thanks,
Conor.

>
>>
>> Signed-off-by: Vineet Gupta <[email protected]>
>> ---
>> Changes since v1
>>    - Build error (and boot tested on qemu)
>>    - Improved the comments a bit
>> ---
>>   arch/riscv/include/asm/elf.h      | 11 ++++++++++-
>>   arch/riscv/include/uapi/asm/elf.h |  2 ++
>>   2 files changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/riscv/include/asm/elf.h b/arch/riscv/include/asm/elf.h
>> index f53c40026c7a..b6b4542b3039 100644
>> --- a/arch/riscv/include/asm/elf.h
>> +++ b/arch/riscv/include/asm/elf.h
>> @@ -26,10 +26,19 @@
>>
>>   #define ELF_DATA       ELFDATA2LSB
>>
>> +/*
>> + * Make sure the elf being loaded is compatible with extensions.
>> + *
>> + * In the final incarnation this will get the extension list from DT and
>> + * make sure elf can run on given hardware+kernel.
>> + * For now disallow TSO built binaries.
>> + */
>> +#define rv_ext_ok(x)   (!((x)->e_flags & EF_RISCV_TSO))
>> +
>>   /*
>>    * This is used to ensure we don't load something for the wrong architecture.
>>    */
>> -#define elf_check_arch(x) ((x)->e_machine == EM_RISCV)
>> +#define elf_check_arch(x) ((x)->e_machine == EM_RISCV && rv_ext_ok(x))
>>
>>   #define CORE_DUMP_USE_REGSET
>>   #define ELF_EXEC_PAGESIZE      (PAGE_SIZE)
>> diff --git a/arch/riscv/include/uapi/asm/elf.h b/arch/riscv/include/uapi/asm/elf.h
>> index d696d6610231..fa9e4c52c7ac 100644
>> --- a/arch/riscv/include/uapi/asm/elf.h
>> +++ b/arch/riscv/include/uapi/asm/elf.h
>> @@ -32,6 +32,8 @@ typedef union __riscv_fp_state elf_fpregset_t;
>>   #define ELF_RISCV_R_TYPE(r_info)       ELF32_R_TYPE(r_info)
>>   #endif
>>
>> +#define EF_RISCV_TSO           (1 << 3)
>
> s/EF/ELF ?
>
> Thanks,
> Conor.
>
>> +
>>   /*
>>    * RISC-V relocation types
>>    */
>> --
>> 2.34.1
>>
>>
>> _______________________________________________
>> linux-riscv mailing list
>> [email protected]
>> http://lists.infradead.org/mailman/listinfo/linux-riscv
>

2022-09-16 09:58:59

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [PATCH v2] riscv: ztso: disallow elf binaries needing TSO

On Thu, 15 Sep 2022 23:58:05 PDT (-0700), [email protected] wrote:
> On 16/09/2022 07:34, Conor Dooley wrote:
>> On 16/09/2022 05:23, Vineet Gupta wrote:
>>> [You don't often get email from [email protected]. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>>>
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> As of now the software stack needs work to support ztso. Until that work
>>> is finished, disallow binaries needing TSO.
>>>
>>> This patch is needed to help ztso ratification and prolifiration of tso
>>> bits in tooling.
>>
>> I have to admit to being a little confused here, if Ztso is not ratified
>> why do we need to protect ourselves from it?
>
> Also, since this is not marked as a fix, why would we not just apply the
> patchset from Palmer that looks like a more complete version of this
> patch:
> https://lore.kernel.org/linux-riscv/[email protected]/
>
> Maybe you could offer an R-b or some comments on that patch instead?

IMO that's the better way to go, Ztso is frozen so we can expect
software to start using it. Might as well let it run if the machine is
going to be TSO anyway (and IIRC some of the T-Head stuff either is or
can be TSO, so we'll need to detection soon anyway).

I was also planning on re-spinning the QEMU stuff so we can test it, but
given how simple the Linux patch is I think it's fine to just go ahead
and take it.

Happy to hear any comments, though.

>
> Thanks,
> Conor.
>
>>
>>>
>>> Signed-off-by: Vineet Gupta <[email protected]>
>>> ---
>>> Changes since v1
>>>    - Build error (and boot tested on qemu)
>>>    - Improved the comments a bit
>>> ---
>>>   arch/riscv/include/asm/elf.h      | 11 ++++++++++-
>>>   arch/riscv/include/uapi/asm/elf.h |  2 ++
>>>   2 files changed, 12 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/riscv/include/asm/elf.h b/arch/riscv/include/asm/elf.h
>>> index f53c40026c7a..b6b4542b3039 100644
>>> --- a/arch/riscv/include/asm/elf.h
>>> +++ b/arch/riscv/include/asm/elf.h
>>> @@ -26,10 +26,19 @@
>>>
>>>   #define ELF_DATA       ELFDATA2LSB
>>>
>>> +/*
>>> + * Make sure the elf being loaded is compatible with extensions.
>>> + *
>>> + * In the final incarnation this will get the extension list from DT and
>>> + * make sure elf can run on given hardware+kernel.
>>> + * For now disallow TSO built binaries.
>>> + */
>>> +#define rv_ext_ok(x)   (!((x)->e_flags & EF_RISCV_TSO))
>>> +
>>>   /*
>>>    * This is used to ensure we don't load something for the wrong architecture.
>>>    */
>>> -#define elf_check_arch(x) ((x)->e_machine == EM_RISCV)
>>> +#define elf_check_arch(x) ((x)->e_machine == EM_RISCV && rv_ext_ok(x))
>>>
>>>   #define CORE_DUMP_USE_REGSET
>>>   #define ELF_EXEC_PAGESIZE      (PAGE_SIZE)
>>> diff --git a/arch/riscv/include/uapi/asm/elf.h b/arch/riscv/include/uapi/asm/elf.h
>>> index d696d6610231..fa9e4c52c7ac 100644
>>> --- a/arch/riscv/include/uapi/asm/elf.h
>>> +++ b/arch/riscv/include/uapi/asm/elf.h
>>> @@ -32,6 +32,8 @@ typedef union __riscv_fp_state elf_fpregset_t;
>>>   #define ELF_RISCV_R_TYPE(r_info)       ELF32_R_TYPE(r_info)
>>>   #endif
>>>
>>> +#define EF_RISCV_TSO           (1 << 3)
>>
>> s/EF/ELF ?
>>
>> Thanks,
>> Conor.
>>
>>> +
>>>   /*
>>>    * RISC-V relocation types
>>>    */
>>> --
>>> 2.34.1
>>>
>>>
>>> _______________________________________________
>>> linux-riscv mailing list
>>> [email protected]
>>> http://lists.infradead.org/mailman/listinfo/linux-riscv
>>
>

2022-09-16 16:44:28

by Vineet Gupta

[permalink] [raw]
Subject: Re: [PATCH v2] riscv: ztso: disallow elf binaries needing TSO



On 9/15/22 23:58, [email protected] wrote:
> Also, since this is not marked as a fix, why would we not just apply the
> patchset from Palmer that looks like a more complete version of this
> patch:
> https://lore.kernel.org/linux-riscv/[email protected]/
>
> Maybe you could offer an R-b or some comments on that patch instead?

Sure I didn't know about that patch and my idea was just to get the ball
rolling so tooling at least can start having tso bits as last time
around that was the blocker to getting stuff in binutils and gcc etc.

-Vineet

2022-09-16 16:47:20

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2] riscv: ztso: disallow elf binaries needing TSO

On 16/09/2022 17:28, Vineet Gupta wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On 9/15/22 23:58, [email protected] wrote:
>> Also, since this is not marked as a fix, why would we not just apply the
>> patchset from Palmer that looks like a more complete version of this
>> patch:
>> https://lore.kernel.org/linux-riscv/[email protected]/
>>
>> Maybe you could offer an R-b or some comments on that patch instead?
>
> Sure I didn't know about that patch and my idea was just to get the ball
> rolling so tooling at least can start having tso bits as last time
> around that was the blocker to getting stuff in binutils and gcc etc.

Ye nw, sure I completely forgot what the order was between frozen,
public review & ratification was ;)