From: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
To: liweiwei <liweiwei@iscas.ac.cn>,
qemu-riscv@nongnu.org, qemu-devel@nongnu.org
Cc: palmer@dabbelt.com, alistair.francis@wdc.com,
bin.meng@windriver.com, zhiwei_liu@linux.alibaba.com,
wangjunqiang@iscas.ac.cn, lazyparser@gmail.com
Subject: Re: [PATCH 0/2] target/riscv: Separate implicitly-enabled and explicitly-enabled extensions
Date: Tue, 11 Apr 2023 20:48:04 -0300 [thread overview]
Message-ID: <45032919-7642-870c-33d4-f65c38101edf@ventanamicro.com> (raw)
In-Reply-To: <3ab19e9b-88a3-17bd-169d-7317dfbf36e5@iscas.ac.cn>
On 4/10/23 21:15, 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.
>
> I just find we should also separate the cases here. Zcmp/Zcmt require Zca.
>
> If we disable Zca when misa.C is cleared, whether we need disable Zcmp/Zcmt?
IMO write_misa() shouldn't be able to disable Z-extensions that it can't turn it back
on. E.g. RVV disabling zve64d/f is ok because, if we re-enable RVV, they'll be re-enabled
back.
>
> If yes, then we need another new parameter(to indicate they are diabled by
>
> clearing misa.C ) to decide whether we should re-enable them when misa.C is set.
>
> If not, we should make clearing misa.C failed in this case.
I'd rather fail the operation. write_misa() should always make reversible operations. If
a certain CSR write would put us in a state that we can't go back on, we should fail.
For now I think I'll go back to that cleanup I made, rebase it, get one of Weiwei patches
that fixes the sifive breakage I talked about in the other thread, and see if we can
get that more simple version of write_misa() merged. We can continue these discussions
on top of that code.
Thanks,
Daniel
>
> Regards,
>
> Weiwei Li
>
>>
>> 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?
>>
>>
>> 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(-)
>>>
>
prev parent reply other threads:[~2023-04-11 23:49 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-10 3:35 [PATCH 0/2] target/riscv: Separate implicitly-enabled and explicitly-enabled extensions Weiwei Li
2023-04-10 3:35 ` [PATCH 1/2] target/riscv: Add set_implicit_extensions_from_ext() function Weiwei Li
2023-04-10 13:28 ` Daniel Henrique Barboza
2023-04-12 2:51 ` Alistair Francis
2023-04-10 3:35 ` [PATCH 2/2] target/riscv: Add ext_z*_enabled for implicitly enabled extensions Weiwei Li
2023-04-10 13:48 ` [PATCH 0/2] target/riscv: Separate implicitly-enabled and explicitly-enabled extensions Daniel Henrique Barboza
2023-04-10 14:20 ` liweiwei
2023-04-10 18:35 ` Daniel Henrique Barboza
2023-04-11 0:18 ` liweiwei
2023-04-11 0:15 ` liweiwei
2023-04-11 23:48 ` Daniel Henrique Barboza [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=45032919-7642-870c-33d4-f65c38101edf@ventanamicro.com \
--to=dbarboza@ventanamicro.com \
--cc=alistair.francis@wdc.com \
--cc=bin.meng@windriver.com \
--cc=lazyparser@gmail.com \
--cc=liweiwei@iscas.ac.cn \
--cc=palmer@dabbelt.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-riscv@nongnu.org \
--cc=wangjunqiang@iscas.ac.cn \
--cc=zhiwei_liu@linux.alibaba.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).