On 2023/4/11 02:35, Daniel Henrique Barboza wrote:
On 4/10/23 11:20, liweiwei wrote:
On 2023/4/10 21:48, Daniel Henrique Barboza wrote:
Hi,
On 4/10/23 00:35, Weiwei Li wrote:
The patch tries to separate the multi-letter extensions that may
implicitly-enabled by misa.EXT from the explicitly-enabled cases,
so that the misa.EXT can truely disabled by write_misa().
With this separation, the implicitly-enabled zve64d/f and zve32f
extensions will no work if we clear misa.V. And clear misa.V will
have no effect on the explicitly-enalbed zve64d/f and zve32f
extensions.
For this particular case of write_misa() I'm not sure if we need all
that. If we want
to grant user choice on write_misa(), let's say that the user wants
to enable/disable
RVV, we can enable/disable all RVV related Z-extensions by hand.
It's just a matter
of writing enable/disable code that write_misa() would use.
In the end, write_misa() is also an user choice. If write_misa()
wants to disable RVV,
this means that the user wants to disable RVV, so it doesn't matter
whether the user
enabled zve32f on the command line or not - we disable zve32f as
well. Same thing for
RVC and its related Z-extensions.
Yeah. It's also a choice. It's a question whether we take C_Zca and C
as the same user choice.
If we consider them as different, then this patch works. And this
patch can bypass the priv version problem.
The reason why I didn't do this particular code for RVC and RVV is
because we have
pending work in the ML that I would like to get it merged first. And
there's a few
caveats we need to decide what to do, e.g. what if the user disables
F but V is
enabled? Do we refuse write_misa()? Do we disable RVV?
In section 3.1.1 of privilege spec:
"If an ISA feature x depends on an ISA feature y, then attempting to
enable feature x but disable
feature y results in both features being disabled."
Even though there is also another description in the following
content of the same section:
"An implementation may impose additional constraints on the
collective setting of two or more misa
fields, in which case they function collectively as a single WARL
field. An attempt to write an
unsupported combination causes those bits to be set to some supported
combination."
I think the former description is more explicit.
Yeah. Disabling a MISA bit should disable all dependencies of the bit
and there's
not much to discuss about it.
As far as the current write_misa() impl in the mailing list goes,
we're refusing to
disable the bit (e.g. the validation would fail if RVF is disabled
while keeping all
its dependencies, write_misa() becomes a no-op).
If we decide to give users this kind of control I believe we should
disregard all user
choice during boot and enable/disable extensions as required.
Sorry, I don't get your idea here. Why should we disregard all user
choice during boot?
Regards,
Weiwei Li
Thanks,
Daniel
Regards,
Weiwei Li
All this said, patch 1 is still a good addition to make it easier to
distinguish
the Z-extensions we're enabling due to MISA bits. I believe we
should use
set_implicit_extensions_from_ext() in the future for all similar
situations.
Thanks,
Daniel
Weiwei Li (2):
target/riscv: Add set_implicit_extensions_from_ext() function
target/riscv: Add ext_z*_enabled for implicitly enabled extensions
target/riscv/cpu.c | 73
+++++++++++++++----------
target/riscv/cpu.h | 8 +++
target/riscv/cpu_helper.c | 2 +-
target/riscv/csr.c | 2 +-
target/riscv/insn_trans/trans_rvd.c.inc | 2 +-
target/riscv/insn_trans/trans_rvf.c.inc | 2 +-
target/riscv/insn_trans/trans_rvi.c.inc | 5 +-
target/riscv/insn_trans/trans_rvv.c.inc | 16 +++---
target/riscv/translate.c | 4 +-
9 files changed, 68 insertions(+), 46 deletions(-)
