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
