Date:   Fri, 16 Sep 2022 02:54:28 -0700 (PDT)
From:   Palmer Dabbelt <palmer@...osinc.com>
To:     Conor.Dooley@...rochip.com
CC:     Vineet Gupta <vineetg@...osinc.com>,
        linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org,
        cmuellner@...tanamicro.com, linux@...osinc.com
Subject:     Re: [PATCH v2] riscv: ztso: disallow elf binaries needing TSO

On Thu, 15 Sep 2022 23:58:05 PDT (-0700), Conor.Dooley@...rochip.com 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 vineetg@...osinc.com. 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/20220902034352.8825-1-palmer@rivosinc.com/
> 
> 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 <vineetg@...osinc.com>
>>> ---
>>> 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
>>> linux-riscv@...ts.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/linux-riscv
>> 
> 

Powered by blists - more mailing lists