On 2/21/23 14:06, Andrew Jones wrote:
On Tue, Feb 21, 2023 at 12:49:11PM -0300, Daniel Henrique Barboza wrote:
On 2/16/23 22:42, LIU Zhiwei wrote:
On 2023/2/17 5:55, Daniel Henrique Barboza wrote:
At this moment, and apparently since ever, we have no way of enabling
RISCV_FEATURE_MISA. This means that all the code from write_misa(), all
the nuts and bolts that handles how to write this CSR, has always been a
no-op as well because write_misa() will always exit earlier.
This seems to be benign in the majority of cases. Booting an Ubuntu
'virt' guest and logging all the calls to 'write_misa' shows that no
writes to MISA CSR was attempted. Writing MISA, i.e. enabling/disabling
RISC-V extensions after the machine is powered on, seems to be a niche
Before proceeding, let's recap what the spec says about MISA. It is a
CSR that is divided in 3 fields:
- MXL, Machine XLEN, described as "may be writable";
- MXLEN, the XLEN in M-mode, which is given by the setting of MXL or a
fixed value if MISA is zero;
- Extensions is defined as "a WARL field that can contain writable bits
where the implementation allows the supported ISA to be modified"
Thus what we have today (write_misa() being a no-op) is already a valid
spec implementation. We're not obliged to have a particular set of MISA
writable bits, and at this moment we have none.
I see there has been a discussion on this topic. And as no-op has no
harmfulness for current implementation.
However, I still think we should make misa writable as default, which is also a
valid spec implementation.
One reason is that may be we need to dynamic write access for some cpus in the
future. The other is we should
make QEMU a more useful implementation, not just a legal implementation. We
have done in many aspects on this direction.
I prefer your implementation before v4. It's not a complicated implementation.
And I think the other extensions on QEMU currently
can mostly be configurable already.
I don't have a strong opinion in this matter to be honest. My problems with the
existing code are:
- the code is untested. I cannot say that this was never tested, but I can say
this has been mostly untested ever since introduced. Which is normal for a code
- the code is dormant and most likely with bugs, but it's still maintained. For
example we have e91a7227 ("target/riscv: Split misa.mxl and misa.ext") that had
to make changes here. So we have the upkeep but no benefits.
- we don't have an use case for it. Most OSes doesn't seem to care, and afaik no
applications seems to care either.
All this said, I think we can reach a consensus of keeping it if we can at
up with a way of testing it.
Your work is a good step towards to unify the configuration and the check. I
think two more steps we can go further.
1) Remove RVI/RVF and the similar macros, and add fields for them in the
2) Unify the check about configuration. write_misa and cpu_realize_fn can use
the same check function.
As we have done these two steps, I think we can go more closely for the profile
Is this the extension you're taking about?
Zhiwei, I looked it up and at first I don't understand how writing MISA is
related to this profile extension. Are you suggesting that the firmware
can choose to run a specific profile and then the hardware must adapt to
it on the fly? Because if not, then we can implement profiles by just
passing them in the QEMU command line.
This looks like a good reason to keep the code. Let's see if anyone else has an
about it. We can do the improvements you mentioned above as a follow-up (this
really about removing RISC_FEATURE_*) if we decide to keep it.
If we decide to keep it and not guard it by default, then we should test
and fix it now. Also, as we're already aware that it has insufficient
sanity checks for extension dependencies, then we should fix our general
extension dependency checking now too, in order to apply that to this.
IOW, trying to keep this, without some guard on it, opens a can of worms.
My vote is the same as it was before, merge this series and then revisit
this function when someone has a use/test case for it. Nobody said this
was never going to have a different implementation, just that the current
implementation is known-buggy and there's no reason to expose it now.
It wouldn't be not guarded by default. In fact, in case we decide to go back
to what we were doing a couple of versions ago, I would rename the 'misa-w'
attribute to 'x-misa-w'.
The 'x' would be an indication that this is really something experimental and
expectations must be set accordingly if the user decides to enable it. In
reality, what this 'x-misa-w' would do is to give us more time to stabilize
the code inside write_misa(). Ideally we would get rid of it when the code