[PATCH for 4.21 v7 3/4] xen/riscv: make zbb as mandatory

Oleksii Kurochko posted 4 patches 10 months ago
There is a newer version of this series
[PATCH for 4.21 v7 3/4] xen/riscv: make zbb as mandatory
Posted by Oleksii Kurochko 10 months ago
According to riscv/booting.txt, it is expected that Zbb should be supported.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in v7:
 - new patch.
---
 xen/arch/riscv/arch.mk | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/xen/arch/riscv/arch.mk b/xen/arch/riscv/arch.mk
index 3034da76cb..236ea7c8a6 100644
--- a/xen/arch/riscv/arch.mk
+++ b/xen/arch/riscv/arch.mk
@@ -9,7 +9,7 @@ riscv-abi-$(CONFIG_RISCV_64) := -mabi=lp64
 riscv-march-$(CONFIG_RISCV_64) := rv64
 riscv-march-y += ima
 riscv-march-$(CONFIG_RISCV_ISA_C) += c
-riscv-march-y += _zicsr_zifencei
+riscv-march-y += _zicsr_zifencei_zbb
 
 riscv-generic-flags := $(riscv-abi-y) -march=$(subst $(space),,$(riscv-march-y))
 
@@ -25,13 +25,10 @@ $(eval $(1) := \
 	$(call as-insn,$(CC) $(riscv-generic-flags)_$(1),$(value $(1)-insn),_$(1)))
 endef
 
-zbb-insn := "andn t0$(comma)t0$(comma)t0"
-$(call check-extension,zbb)
-
 zihintpause-insn := "pause"
 $(call check-extension,zihintpause)
 
-extensions := $(zbb) $(zihintpause)
+extensions := $(zihintpause)
 
 extensions := $(subst $(space),,$(extensions))
 
-- 
2.48.1
Re: [PATCH for 4.21 v7 3/4] xen/riscv: make zbb as mandatory
Posted by Jan Beulich 9 months, 3 weeks ago
On 20.02.2025 18:44, Oleksii Kurochko wrote:
> According to riscv/booting.txt, it is expected that Zbb should be supported.
> 
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> ---
> Changes in v7:
>  - new patch.
> ---
>  xen/arch/riscv/arch.mk | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)

Please can you also tidy asm/cmpxchg.h of ANDN_INSN() then?

Jan
Re: [PATCH for 4.21 v7 3/4] xen/riscv: make zbb as mandatory
Posted by Oleksii Kurochko 9 months, 3 weeks ago
On 2/26/25 1:58 PM, Jan Beulich wrote:
> On 20.02.2025 18:44, Oleksii Kurochko wrote:
>> According to riscv/booting.txt, it is expected that Zbb should be supported.
>>
>> Signed-off-by: Oleksii Kurochko<oleksii.kurochko@gmail.com>
>> ---
>> Changes in v7:
>>   - new patch.
>> ---
>>   xen/arch/riscv/arch.mk | 7 ++-----
>>   1 file changed, 2 insertions(+), 5 deletions(-)
> Please can you also tidy asm/cmpxchg.h of ANDN_INSN() then?

Sure, I can leave only:
/*
  * To not face an issue that gas doesn't understand ANDN instruction
  * it is encoded using .insn directive.
  */
#define ANDN_INSN(rd, rs1, rs2)                 \
     ".insn r OP, 0x7, 0x20, " rd ", " rs1 ", " rs2 "\n"

~ Oleksii
Re: [PATCH for 4.21 v7 3/4] xen/riscv: make zbb as mandatory
Posted by Jan Beulich 9 months, 3 weeks ago
On 26.02.2025 15:38, Oleksii Kurochko wrote:
> 
> On 2/26/25 1:58 PM, Jan Beulich wrote:
>> On 20.02.2025 18:44, Oleksii Kurochko wrote:
>>> According to riscv/booting.txt, it is expected that Zbb should be supported.
>>>
>>> Signed-off-by: Oleksii Kurochko<oleksii.kurochko@gmail.com>
>>> ---
>>> Changes in v7:
>>>   - new patch.
>>> ---
>>>   xen/arch/riscv/arch.mk | 7 ++-----
>>>   1 file changed, 2 insertions(+), 5 deletions(-)
>> Please can you also tidy asm/cmpxchg.h of ANDN_INSN() then?
> 
> Sure, I can leave only:
> /*
>   * To not face an issue that gas doesn't understand ANDN instruction
>   * it is encoded using .insn directive.
>   */
> #define ANDN_INSN(rd, rs1, rs2)                 \
>      ".insn r OP, 0x7, 0x20, " rd ", " rs1 ", " rs2 "\n"

Wait, no - why would you? Patch 0.5 is supposed to be setting a baseline
where Zbb is known by the tool chain. With that proper "andn" can be used
wherever we like.

Jan
Re: [PATCH for 4.21 v7 3/4] xen/riscv: make zbb as mandatory
Posted by Oleksii Kurochko 9 months, 3 weeks ago
On 2/26/25 3:49 PM, Jan Beulich wrote:
> On 26.02.2025 15:38, Oleksii Kurochko wrote:
>> On 2/26/25 1:58 PM, Jan Beulich wrote:
>>> On 20.02.2025 18:44, Oleksii Kurochko wrote:
>>>> According to riscv/booting.txt, it is expected that Zbb should be supported.
>>>>
>>>> Signed-off-by: Oleksii Kurochko<oleksii.kurochko@gmail.com>
>>>> ---
>>>> Changes in v7:
>>>>    - new patch.
>>>> ---
>>>>    xen/arch/riscv/arch.mk | 7 ++-----
>>>>    1 file changed, 2 insertions(+), 5 deletions(-)
>>> Please can you also tidy asm/cmpxchg.h of ANDN_INSN() then?
>> Sure, I can leave only:
>> /*
>>    * To not face an issue that gas doesn't understand ANDN instruction
>>    * it is encoded using .insn directive.
>>    */
>> #define ANDN_INSN(rd, rs1, rs2)                 \
>>       ".insn r OP, 0x7, 0x20, " rd ", " rs1 ", " rs2 "\n"
> Wait, no - why would you? Patch 0.5 is supposed to be setting a baseline
> where Zbb is known by the tool chain. With that proper "andn" can be used
> wherever we like.

I thought that Zbb in binutils (part of whic is gas) was added later then
I mentioned in Patch 0.5 but Zbb was added starting from 2.37, so we can
just really use `andn` instruction instead of `.insn` .

Thanks for clarification.

~ Oleksii