On 1/14/25 8:33 AM, Jan Beulich wrote:
+ RISCV_ISA_EXT_DATA(i, RISCV_ISA_EXT_i),
+ RISCV_ISA_EXT_DATA(m, RISCV_ISA_EXT_m),
+ RISCV_ISA_EXT_DATA(a, RISCV_ISA_EXT_a),
+ RISCV_ISA_EXT_DATA(f, RISCV_ISA_EXT_f),
+ RISCV_ISA_EXT_DATA(d, RISCV_ISA_EXT_d),
+ RISCV_ISA_EXT_DATA(q, RISCV_ISA_EXT_q),
+ RISCV_ISA_EXT_DATA(h, RISCV_ISA_EXT_h),
+ RISCV_ISA_EXT_DATA(zicntr, RISCV_ISA_EXT_ZICNTR),
+ RISCV_ISA_EXT_DATA(zicsr, RISCV_ISA_EXT_ZICSR),
+ RISCV_ISA_EXT_DATA(zifencei, RISCV_ISA_EXT_ZIFENCEI),
+ RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE),
+ RISCV_ISA_EXT_DATA(zihpm, RISCV_ISA_EXT_ZIHPM),
+ RISCV_ISA_EXT_DATA(zbb, RISCV_ISA_EXT_ZBB),
Isn't it kind of implied that with the presence of Zbb, B should also be
present?
My interpretation of the RISC-V Bitmanip Extension spec is that the 'B'
extension is essentially a collection of
the Zba, Zbb, Zbs, and other extensions, but it isn't an extension by itself.
The following is mentioned in the spec:
The bit-manipulation (bitmanip) extension collection is comprised of
several component extensions to the base
RISC-V architecture that are intended to provide some combination of code
size reduction, performance
improvement, and energy reduction. While the instructions are intended to
have general use, some instructions
are more useful in some domains than others. Hence, several smaller
bitmanip extensions are provided, rather
than one large extension. Each of these smaller extensions is grouped by
common function and use case, and
each of which has its own Zb*-extension name.
Still the doc has '"B" Extension for Bit Manipulation' as the title of the
chapter.
And gas accepts B as an extension (e.g. ".option arch, +b").
I think it is fine.
B, in this case, just represents Zba, Zbb, Zbc, Zbs and that all of them are
supported at the same time.
But I see chips that doesn't have B because it doesn't have one of those
extensions.
+ /*
+ * Workaround for invalid single-letter 's' & 'u' (QEMU).
+ * No need to set the bit in riscv_isa as 's' & 'u' are
+ * not valid ISA extensions. It works unless the first
+ * multi-letter extension in the ISA string begins with
+ * "Su" and is not prefixed with an underscore.
+ */
+ if ( ext[-1] != '_' && ext[1] == 'u' )
+ {
+ ++isa;
+ ext_err = true;
+ break;
+ }
I'm afraid I don't understand this; the comment raises more questions
than it answers.
Some details could be found here about these QEMU workaround from LK view:
https://lore.kernel.org/linux-riscv/ae93358e-e117-b43d-faad-772c529f8...@irq.a4lg.com/#t
This leads to the following fix in QEMU:
https://patchwork.kernel.org/project/qemu-devel/patch/dee09d708405075420b29115c1e9e87910b8da55.1648270894.git.research_tra...@irq.a4lg.com/#24792587
Considering QEMU's patch, these workaround isn't needed anymore since QEMU 7.1
( it has been released30 Aug 2022 ) probably we could update the
QEMU version on our CI and just drop these changes.
Or, at least, update the comment with the links mentioned above and add a message
that these changes are needed only for QEMU < 7.1.
Am I right that we don't have something like GCC_VERSION in Xen but for QEMU?
How could there be? At the time of building Xen we know what compiler
version is in use, but we clearly don't know under what qemu versions
it might later be run.
Agree with that, there is no any sense for having something similar as
GCC_VERSIOB but
for QEMU. Then I will just update the comment around this workaround with some
clarifications.
+ riscv_isa_parse_string(isa, this_isa);
+
+ if ( bitmap_empty(riscv_isa, RISCV_ISA_EXT_MAX) )
+ bitmap_copy(riscv_isa, this_isa, RISCV_ISA_EXT_MAX);
+ else
+ bitmap_and(riscv_isa, riscv_isa, this_isa, RISCV_ISA_EXT_MAX);
What if the first instance had no extensions at all? You'll then copy what
the second instance say, ending up with extensions not supported by one of
the CPUs.
I think that it's impossible that there is no extensions at all and it should be
considered as a bug of provided riscv,isa property. Thereby it should be enough
to
add BUG_ON(!bitmap_empty(this_isa, RISCV_ISA_EXT_MAX)) before if-condition.
Well, you can of course make such an assumption. I don't think though that
it's technically impossible to have an extension-less environment. Xen
won't be able to run there, though (we'll require H at the very least aiui,
and I'm sure we really also require Zicsr).
I would like to clarify some things. I think we are counting by word
'extension' different things.
I am including to this `Base ISA` ( and likely it is incorrect to do so ( or,at
least, confusing )
and I will try not to do that in the future. I am using it in this manner
because `Base ISA` is
included to the table 74. Standard ISA extension names in Unpriv spec ) then it
is impossible to
have an extension-less environment because the spec mentions the following:
A RISC-V ISA is defined as a base integer ISA, which must be present in any
implementation, plus
optional extensions to the base ISA.
So, at least, r{32,64,128}i should written in riscv,isa property of DTS file
and that is the reason why
this_isa can't be empty, and thereby riscv_isa will be initialized with, at
least, `i` ending up with only `i`
supported by all CPUs.
But if not count `I` as an extension and just as base ISA then it is really
technically possible to come up with
extension-less environment. But anyway as you mentioned we still need for Xen
some extensions. ( btw, thanks, I missed to
add Zicsr to required_extensions[] ).
Thanks.
~ Oleksii
