* [PATCH v3] xen/riscv: check whether the assembler has Zbb extension support
@ 2024-04-19 14:23 Oleksii Kurochko
2024-04-22 9:43 ` Jan Beulich
0 siblings, 1 reply; 5+ messages in thread
From: Oleksii Kurochko @ 2024-04-19 14:23 UTC (permalink / raw)
To: xen-devel
Cc: Oleksii Kurochko, Alistair Francis, Bob Eshleman, Connor Davis,
Andrew Cooper, George Dunlap, Jan Beulich, Julien Grall,
Stefano Stabellini
Update the argument of the as-insn for the Zbb case to verify that
Zbb is supported not only by a compiler, but also by an assembler.
Also, check-extenstion(ext_name, "insn") helper macro is introduced
to check whether extension is supported by a compiler and an assembler.
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in V3:
- code style fixes: use "-" insteaf of "_" for names.
- update the commit message.
---
Changes in V2:
- introduce check_extenstion to abstract a check of if extenstion is
supported or not.
- update the commit message.
---
xen/arch/riscv/arch.mk | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/xen/arch/riscv/arch.mk b/xen/arch/riscv/arch.mk
index 53f3575e7d..dd242c91d1 100644
--- a/xen/arch/riscv/arch.mk
+++ b/xen/arch/riscv/arch.mk
@@ -11,9 +11,14 @@ riscv-march-$(CONFIG_RISCV_ISA_C) := $(riscv-march-y)c
riscv-generic-flags := $(riscv-abi-y) -march=$(riscv-march-y)
-zbb := $(call as-insn,$(CC) $(riscv-generic-flags)_zbb,"",_zbb)
-zihintpause := $(call as-insn, \
- $(CC) $(riscv-generic-flags)_zihintpause,"pause",_zihintpause)
+# check-extension: Check whether extenstion is supported by a compiler and
+# an assembler.
+# Usage: $(call check-extension,extension_name,"instr")
+check-extension = $(call as-insn,$(CC) $(riscv-generic-flags)_$(1),$(2),_$(1))
+
+zbb-insn := "andn t0, t0, t0"
+zbb := $(call check-extension,zbb,$(zbb-insn))
+zihintpause := $(call check-extension,zihintpause,"pause")
extensions := $(zbb) $(zihintpause)
--
2.44.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3] xen/riscv: check whether the assembler has Zbb extension support
2024-04-19 14:23 [PATCH v3] xen/riscv: check whether the assembler has Zbb extension support Oleksii Kurochko
@ 2024-04-22 9:43 ` Jan Beulich
2024-04-22 15:41 ` Oleksii
0 siblings, 1 reply; 5+ messages in thread
From: Jan Beulich @ 2024-04-22 9:43 UTC (permalink / raw)
To: Oleksii Kurochko
Cc: Alistair Francis, Bob Eshleman, Connor Davis, Andrew Cooper,
George Dunlap, Julien Grall, Stefano Stabellini, xen-devel
On 19.04.2024 16:23, Oleksii Kurochko wrote:
> Update the argument of the as-insn for the Zbb case to verify that
> Zbb is supported not only by a compiler, but also by an assembler.
>
> Also, check-extenstion(ext_name, "insn") helper macro is introduced
> to check whether extension is supported by a compiler and an assembler.
>
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
despite ...
> --- a/xen/arch/riscv/arch.mk
> +++ b/xen/arch/riscv/arch.mk
> @@ -11,9 +11,14 @@ riscv-march-$(CONFIG_RISCV_ISA_C) := $(riscv-march-y)c
>
> riscv-generic-flags := $(riscv-abi-y) -march=$(riscv-march-y)
>
> -zbb := $(call as-insn,$(CC) $(riscv-generic-flags)_zbb,"",_zbb)
> -zihintpause := $(call as-insn, \
> - $(CC) $(riscv-generic-flags)_zihintpause,"pause",_zihintpause)
> +# check-extension: Check whether extenstion is supported by a compiler and
> +# an assembler.
> +# Usage: $(call check-extension,extension_name,"instr")
> +check-extension = $(call as-insn,$(CC) $(riscv-generic-flags)_$(1),$(2),_$(1))
> +
> +zbb-insn := "andn t0, t0, t0"
> +zbb := $(call check-extension,zbb,$(zbb-insn))
> +zihintpause := $(call check-extension,zihintpause,"pause")
... still not really being happy with this: Either, as said before, zbb-insn
would better be avoided (by using $(comma) as necessary), or you should have
gone yet a step further to fully address my "redundancy" concern. Note how
the two ultimate lines still have 3 (zbb) and 2 (zihintpause) references
respectively, when the goal ought to be to have exactly one. E.g. along the
lines of
$(call check-extension,zbb)
$(call check-extension,zihintpause)
suitably using $(eval ...) (to effect the variable definitions) and defining
both zbb-insn and zihintpause-insn.
But I'll nevertheless put this in as is, unless you tell me you're up to
going the extra step.
Jan
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] xen/riscv: check whether the assembler has Zbb extension support
2024-04-22 9:43 ` Jan Beulich
@ 2024-04-22 15:41 ` Oleksii
2024-04-26 8:26 ` Oleksii
0 siblings, 1 reply; 5+ messages in thread
From: Oleksii @ 2024-04-22 15:41 UTC (permalink / raw)
To: Jan Beulich
Cc: Alistair Francis, Bob Eshleman, Connor Davis, Andrew Cooper,
George Dunlap, Julien Grall, Stefano Stabellini, xen-devel
On Mon, 2024-04-22 at 11:43 +0200, Jan Beulich wrote:
> On 19.04.2024 16:23, Oleksii Kurochko wrote:
> > Update the argument of the as-insn for the Zbb case to verify that
> > Zbb is supported not only by a compiler, but also by an assembler.
> >
> > Also, check-extenstion(ext_name, "insn") helper macro is introduced
> > to check whether extension is supported by a compiler and an
> > assembler.
> >
> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>
> Acked-by: Jan Beulich <jbeulich@suse.com>
> despite ...
>
> > --- a/xen/arch/riscv/arch.mk
> > +++ b/xen/arch/riscv/arch.mk
> > @@ -11,9 +11,14 @@ riscv-march-$(CONFIG_RISCV_ISA_C) :=
> > $(riscv-march-y)c
> >
> > riscv-generic-flags := $(riscv-abi-y) -march=$(riscv-march-y)
> >
> > -zbb := $(call as-insn,$(CC) $(riscv-generic-flags)_zbb,"",_zbb)
> > -zihintpause := $(call as-insn, \
> > - $(CC) $(riscv-generic-
> > flags)_zihintpause,"pause",_zihintpause)
> > +# check-extension: Check whether extenstion is supported by a
> > compiler and
> > +# an assembler.
> > +# Usage: $(call check-extension,extension_name,"instr")
> > +check-extension = $(call as-insn,$(CC) $(riscv-generic-
> > flags)_$(1),$(2),_$(1))
> > +
> > +zbb-insn := "andn t0, t0, t0"
> > +zbb := $(call check-extension,zbb,$(zbb-insn))
> > +zihintpause := $(call check-extension,zihintpause,"pause")
>
> ... still not really being happy with this: Either, as said before,
> zbb-insn
> would better be avoided (by using $(comma) as necessary), or you
> should have
> gone yet a step further to fully address my "redundancy" concern.
> Note how
> the two ultimate lines still have 3 (zbb) and 2 (zihintpause)
> references
> respectively, when the goal ought to be to have exactly one. E.g.
> along the
> lines of
>
> $(call check-extension,zbb)
> $(call check-extension,zihintpause)
>
> suitably using $(eval ...) (to effect the variable definitions) and
> defining
> both zbb-insn and zihintpause-insn.
>
> But I'll nevertheless put this in as is, unless you tell me you're up
> to
> going the extra step.
I am okay with all your suggestions. So the final version will look
like ( if I understood you correctly ):
diff --git a/xen/arch/riscv/arch.mk b/xen/arch/riscv/arch.mk
index dd242c91d1..f172187144 100644
--- a/xen/arch/riscv/arch.mk
+++ b/xen/arch/riscv/arch.mk
@@ -13,12 +13,19 @@ riscv-generic-flags := $(riscv-abi-y) -
march=$(riscv-march-y)
# check-extension: Check whether extenstion is supported by a compiler
and
# an assembler.
-# Usage: $(call check-extension,extension_name,"instr")
-check-extension = $(call as-insn,$(CC) $(riscv-generic-
flags)_$(1),$(2),_$(1))
+# Usage: $(call check-extension,extension_name).
+# it should be defined variable with name: extension-name :=
"insn"
-zbb-insn := "andn t0, t0, t0"
-zbb := $(call check-extension,zbb,$(zbb-insn))
-zihintpause := $(call check-extension,zihintpause,"pause")
+define check-extension =
+$(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)
If the diff above looks good, I'll sent a new patch version.
Thanks.
~ Oleksii
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3] xen/riscv: check whether the assembler has Zbb extension support
2024-04-22 15:41 ` Oleksii
@ 2024-04-26 8:26 ` Oleksii
2024-04-26 10:54 ` Jan Beulich
0 siblings, 1 reply; 5+ messages in thread
From: Oleksii @ 2024-04-26 8:26 UTC (permalink / raw)
To: Jan Beulich
Cc: Alistair Francis, Bob Eshleman, Connor Davis, Andrew Cooper,
George Dunlap, Julien Grall, Stefano Stabellini, xen-devel
On Mon, 2024-04-22 at 17:41 +0200, Oleksii wrote:
> On Mon, 2024-04-22 at 11:43 +0200, Jan Beulich wrote:
> > On 19.04.2024 16:23, Oleksii Kurochko wrote:
> > > Update the argument of the as-insn for the Zbb case to verify
> > > that
> > > Zbb is supported not only by a compiler, but also by an
> > > assembler.
> > >
> > > Also, check-extenstion(ext_name, "insn") helper macro is
> > > introduced
> > > to check whether extension is supported by a compiler and an
> > > assembler.
> > >
> > > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> >
> > Acked-by: Jan Beulich <jbeulich@suse.com>
> > despite ...
> >
> > > --- a/xen/arch/riscv/arch.mk
> > > +++ b/xen/arch/riscv/arch.mk
> > > @@ -11,9 +11,14 @@ riscv-march-$(CONFIG_RISCV_ISA_C) :=
> > > $(riscv-march-y)c
> > >
> > > riscv-generic-flags := $(riscv-abi-y) -march=$(riscv-march-y)
> > >
> > > -zbb := $(call as-insn,$(CC) $(riscv-generic-flags)_zbb,"",_zbb)
> > > -zihintpause := $(call as-insn, \
> > > - $(CC) $(riscv-generic-
> > > flags)_zihintpause,"pause",_zihintpause)
> > > +# check-extension: Check whether extenstion is supported by a
> > > compiler and
> > > +# an assembler.
> > > +# Usage: $(call check-extension,extension_name,"instr")
> > > +check-extension = $(call as-insn,$(CC) $(riscv-generic-
> > > flags)_$(1),$(2),_$(1))
> > > +
> > > +zbb-insn := "andn t0, t0, t0"
> > > +zbb := $(call check-extension,zbb,$(zbb-insn))
> > > +zihintpause := $(call check-extension,zihintpause,"pause")
> >
> > ... still not really being happy with this: Either, as said before,
> > zbb-insn
> > would better be avoided (by using $(comma) as necessary), or you
> > should have
> > gone yet a step further to fully address my "redundancy" concern.
> > Note how
> > the two ultimate lines still have 3 (zbb) and 2 (zihintpause)
> > references
> > respectively, when the goal ought to be to have exactly one. E.g.
> > along the
> > lines of
> >
> > $(call check-extension,zbb)
> > $(call check-extension,zihintpause)
> >
> > suitably using $(eval ...) (to effect the variable definitions) and
> > defining
> > both zbb-insn and zihintpause-insn.
> >
> > But I'll nevertheless put this in as is, unless you tell me you're
> > up
> > to
> > going the extra step.
> I am okay with all your suggestions. So the final version will look
> like ( if I understood you correctly ):
Jan,
Could you please review the changes below? I just want to ensure that
you are okay with them. If you are, I'll add your Acked-by that you
gave to this patch in previous answers.
Thanks in advance.
~ Oleksii
> diff --git a/xen/arch/riscv/arch.mk b/xen/arch/riscv/arch.mk
> index dd242c91d1..f172187144 100644
> --- a/xen/arch/riscv/arch.mk
> +++ b/xen/arch/riscv/arch.mk
> @@ -13,12 +13,19 @@ riscv-generic-flags := $(riscv-abi-y) -
> march=$(riscv-march-y)
>
> # check-extension: Check whether extenstion is supported by a
> compiler
> and
> # an assembler.
> -# Usage: $(call check-extension,extension_name,"instr")
> -check-extension = $(call as-insn,$(CC) $(riscv-generic-
> flags)_$(1),$(2),_$(1))
> +# Usage: $(call check-extension,extension_name).
> +# it should be defined variable with name: extension-name :=
> "insn"
>
> -zbb-insn := "andn t0, t0, t0"
> -zbb := $(call check-extension,zbb,$(zbb-insn))
> -zihintpause := $(call check-extension,zihintpause,"pause")
> +define check-extension =
> +$(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)
>
> If the diff above looks good, I'll sent a new patch version.
>
> Thanks.
>
> ~ Oleksii
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] xen/riscv: check whether the assembler has Zbb extension support
2024-04-26 8:26 ` Oleksii
@ 2024-04-26 10:54 ` Jan Beulich
0 siblings, 0 replies; 5+ messages in thread
From: Jan Beulich @ 2024-04-26 10:54 UTC (permalink / raw)
To: Oleksii
Cc: Alistair Francis, Bob Eshleman, Connor Davis, Andrew Cooper,
George Dunlap, Julien Grall, Stefano Stabellini, xen-devel
On 26.04.2024 10:26, Oleksii wrote:
> On Mon, 2024-04-22 at 17:41 +0200, Oleksii wrote:
>> On Mon, 2024-04-22 at 11:43 +0200, Jan Beulich wrote:
>>> On 19.04.2024 16:23, Oleksii Kurochko wrote:
>>>> Update the argument of the as-insn for the Zbb case to verify
>>>> that
>>>> Zbb is supported not only by a compiler, but also by an
>>>> assembler.
>>>>
>>>> Also, check-extenstion(ext_name, "insn") helper macro is
>>>> introduced
>>>> to check whether extension is supported by a compiler and an
>>>> assembler.
>>>>
>>>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>>>
>>> Acked-by: Jan Beulich <jbeulich@suse.com>
>>> despite ...
>>>
>>>> --- a/xen/arch/riscv/arch.mk
>>>> +++ b/xen/arch/riscv/arch.mk
>>>> @@ -11,9 +11,14 @@ riscv-march-$(CONFIG_RISCV_ISA_C) :=
>>>> $(riscv-march-y)c
>>>>
>>>> riscv-generic-flags := $(riscv-abi-y) -march=$(riscv-march-y)
>>>>
>>>> -zbb := $(call as-insn,$(CC) $(riscv-generic-flags)_zbb,"",_zbb)
>>>> -zihintpause := $(call as-insn, \
>>>> - $(CC) $(riscv-generic-
>>>> flags)_zihintpause,"pause",_zihintpause)
>>>> +# check-extension: Check whether extenstion is supported by a
>>>> compiler and
>>>> +# an assembler.
>>>> +# Usage: $(call check-extension,extension_name,"instr")
>>>> +check-extension = $(call as-insn,$(CC) $(riscv-generic-
>>>> flags)_$(1),$(2),_$(1))
>>>> +
>>>> +zbb-insn := "andn t0, t0, t0"
>>>> +zbb := $(call check-extension,zbb,$(zbb-insn))
>>>> +zihintpause := $(call check-extension,zihintpause,"pause")
>>>
>>> ... still not really being happy with this: Either, as said before,
>>> zbb-insn
>>> would better be avoided (by using $(comma) as necessary), or you
>>> should have
>>> gone yet a step further to fully address my "redundancy" concern.
>>> Note how
>>> the two ultimate lines still have 3 (zbb) and 2 (zihintpause)
>>> references
>>> respectively, when the goal ought to be to have exactly one. E.g.
>>> along the
>>> lines of
>>>
>>> $(call check-extension,zbb)
>>> $(call check-extension,zihintpause)
>>>
>>> suitably using $(eval ...) (to effect the variable definitions) and
>>> defining
>>> both zbb-insn and zihintpause-insn.
>>>
>>> But I'll nevertheless put this in as is, unless you tell me you're
>>> up
>>> to
>>> going the extra step.
Well, as per this v3 went in already. Hence ...
>> I am okay with all your suggestions. So the final version will look
>> like ( if I understood you correctly ):
> Jan,
>
> Could you please review the changes below? I just want to ensure that
> you are okay with them. If you are, I'll add your Acked-by that you
> gave to this patch in previous answers.
... if you now want to further update the logic, it'll be a new patch
anyway. The adjustments below look okay to me, but I'm not going to
insist at this point that this be further fiddled with.
Jan
>> --- a/xen/arch/riscv/arch.mk
>> +++ b/xen/arch/riscv/arch.mk
>> @@ -13,12 +13,19 @@ riscv-generic-flags := $(riscv-abi-y) -
>> march=$(riscv-march-y)
>>
>> # check-extension: Check whether extenstion is supported by a
>> compiler
>> and
>> # an assembler.
>> -# Usage: $(call check-extension,extension_name,"instr")
>> -check-extension = $(call as-insn,$(CC) $(riscv-generic-
>> flags)_$(1),$(2),_$(1))
>> +# Usage: $(call check-extension,extension_name).
>> +# it should be defined variable with name: extension-name :=
>> "insn"
>>
>> -zbb-insn := "andn t0, t0, t0"
>> -zbb := $(call check-extension,zbb,$(zbb-insn))
>> -zihintpause := $(call check-extension,zihintpause,"pause")
>> +define check-extension =
>> +$(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)
>>
>> If the diff above looks good, I'll sent a new patch version.
>>
>> Thanks.
>>
>> ~ Oleksii
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-04-26 10:54 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-19 14:23 [PATCH v3] xen/riscv: check whether the assembler has Zbb extension support Oleksii Kurochko
2024-04-22 9:43 ` Jan Beulich
2024-04-22 15:41 ` Oleksii
2024-04-26 8:26 ` Oleksii
2024-04-26 10:54 ` Jan Beulich
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).