From: Andrew Jones <ajones@ventanamicro.com>
To: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Cc: qemu-devel@nongnu.org, qemu-riscv@nongnu.org,
alistair.francis@wdc.com, bmeng@tinylab.org,
liweiwei@iscas.ac.cn, zhiwei_liu@linux.alibaba.com,
palmer@rivosinc.com
Subject: Re: [PATCH v3 16/19] target/riscv/cpu.c: create KVM mock properties
Date: Sat, 24 Jun 2023 09:41:53 +0200 [thread overview]
Message-ID: <20230624-fad369c515a7f4edd9d21c2a@orel> (raw)
In-Reply-To: <c1056ba9-4418-d6ce-536b-93155fff84a2@ventanamicro.com>
On Fri, Jun 23, 2023 at 11:28:03AM -0300, Daniel Henrique Barboza wrote:
...
> > I think we should actually fail with an error when the user tries to
> > enable an extension KVM doesn't support. Otherwise a user may be
> > confused as to why their Zawrs=on didn't provide them a machine with
> > Zawrs. And, when KVM learns how to provide that support to guests
> > (Zawrs is actually on my TODO...), then migrating the same VM to
> > later KVM/QEMU will actually enable the feature, possibly confusing
> > the guest.
> >
> > So, we should probably just not add any extension properties to KVM
> > guests which can't be enabled. Then, as we add support to KVM, we'll
> > add the properties too.
>
> By 'extension properties' do you mean just the flags that enable/disable them,
> like '-cpu, rawrs=<bool>', or also the other properties related to extensions
> that KVM might not support, like 'vlen' and 'elen' from RVV? I'd say that it's
> ok to leave things such as 'vlen' because the user won't be able to enable RVV
> in KVM anyways.
Properties like 'vlen', which have a dependency on an extension, should
probably have their own error checking at cpu finalize features time.
I.e. if 'vlen' is present, but not V, then QEMU should complain. I see
we don't currently do that, though.
>
> And what error do we want to throw? With this patch it's easy to just add an
> Extension zawrs is not available using KVM" error message. Otherwise we can
> not add the property at all and then QEMU will fail with a "property cpu.X not
> found" type of error. Both will error out, question is whether we want to be
> more informative about it.
It's probably best to do the "not available with KVM" error by changing
this patch from adding a no-op setter to an error-out setter. That way,
our story that a riscv machine is the same for both KVM and TCG still
remains, i.e. all properties are still present, but we add the honesty
to the story that not everything works with KVM.
Thanks,
drew
next prev parent reply other threads:[~2023-06-24 7:42 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-22 13:56 [PATCH v3 00/19] target/riscv, KVM: fixes and enhancements Daniel Henrique Barboza
2023-06-22 13:56 ` [PATCH v3 01/19] target/riscv: skip features setup for KVM CPUs Daniel Henrique Barboza
2023-06-22 13:56 ` [PATCH v3 02/19] hw/riscv/virt.c: skip 'mmu-type' FDT if satp mode not set Daniel Henrique Barboza
2023-06-22 13:56 ` [PATCH v3 03/19] target/riscv/cpu.c: restrict 'mvendorid' value Daniel Henrique Barboza
2023-06-22 13:56 ` [PATCH v3 04/19] target/riscv/cpu.c: restrict 'mimpid' value Daniel Henrique Barboza
2023-06-22 13:56 ` [PATCH v3 05/19] target/riscv/cpu.c: restrict 'marchid' value Daniel Henrique Barboza
2023-06-22 13:56 ` [PATCH v3 06/19] target/riscv: use KVM scratch CPUs to init KVM properties Daniel Henrique Barboza
2023-06-22 13:56 ` [PATCH v3 07/19] target/riscv: read marchid/mimpid in kvm_riscv_init_machine_ids() Daniel Henrique Barboza
2023-06-22 13:56 ` [PATCH v3 08/19] target/riscv: handle mvendorid/marchid/mimpid for KVM CPUs Daniel Henrique Barboza
2023-06-22 13:56 ` [PATCH v3 09/19] linux-headers: Update to v6.4-rc1 Daniel Henrique Barboza
2023-06-22 13:56 ` [PATCH v3 10/19] target/riscv/kvm.c: init 'misa_ext_mask' with scratch CPU Daniel Henrique Barboza
2023-06-22 13:56 ` [PATCH v3 11/19] target/riscv/cpu: add misa_ext_info_arr[] Daniel Henrique Barboza
2023-06-23 9:30 ` Andrew Jones
2023-06-22 13:56 ` [PATCH v3 12/19] target/riscv: add KVM specific MISA properties Daniel Henrique Barboza
2023-06-23 9:38 ` Andrew Jones
2023-06-23 14:14 ` Daniel Henrique Barboza
2023-06-24 7:32 ` Andrew Jones
2023-06-25 22:38 ` Daniel Henrique Barboza
2023-06-26 6:24 ` Andrew Jones
2023-06-22 13:56 ` [PATCH v3 13/19] target/riscv/kvm.c: update KVM MISA bits Daniel Henrique Barboza
2023-06-22 13:56 ` [PATCH v3 14/19] target/riscv/kvm.c: add multi-letter extension KVM properties Daniel Henrique Barboza
2023-06-22 13:56 ` [PATCH v3 15/19] target/riscv/cpu.c: remove priv_ver check from riscv_isa_string_ext() Daniel Henrique Barboza
2023-06-23 9:46 ` Andrew Jones
2023-06-22 13:56 ` [PATCH v3 16/19] target/riscv/cpu.c: create KVM mock properties Daniel Henrique Barboza
2023-06-23 9:58 ` Andrew Jones
2023-06-23 14:28 ` Daniel Henrique Barboza
2023-06-24 7:41 ` Andrew Jones [this message]
2023-06-26 17:27 ` Daniel Henrique Barboza
2023-06-22 13:56 ` [PATCH v3 17/19] target/riscv: update multi-letter extension KVM properties Daniel Henrique Barboza
2023-06-22 13:56 ` [PATCH v3 18/19] target/riscv/kvm.c: add kvmconfig_get_cfg_addr() helper Daniel Henrique Barboza
2023-06-22 13:57 ` [PATCH v3 19/19] target/riscv/kvm.c: read/write (cbom|cboz)_blocksize in KVM Daniel Henrique Barboza
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=20230624-fad369c515a7f4edd9d21c2a@orel \
--to=ajones@ventanamicro.com \
--cc=alistair.francis@wdc.com \
--cc=bmeng@tinylab.org \
--cc=dbarboza@ventanamicro.com \
--cc=liweiwei@iscas.ac.cn \
--cc=palmer@rivosinc.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-riscv@nongnu.org \
--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).