[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 16/19] target/riscv/cpu.c: create KVM mock properties

From: Daniel Henrique Barboza
Subject: Re: [PATCH v3 16/19] target/riscv/cpu.c: create KVM mock properties
Date: Mon, 26 Jun 2023 14:27:44 -0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.12.0

On 6/24/23 04:41, Andrew Jones wrote:
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.

We do some checks during realize() in riscv_validate_set_extensions(), via
riscv_cpu_validate_v(). However, 'is the user set vlen but not V we should error
out' is not possible because we don't have a 'user_set' flag for this option, 
we can't say if vlen = 128 is the default, untouched value, or if the user
set vlen = 128 in the command line.

I think we're getting closer to a point where we can "export" the KVM config
options model to TCG. Let's see if we can pull something off for the next

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.

Got it. I'll add an error message in v4. Thanks,



reply via email to

[Prev in Thread] Current Thread [Next in Thread]