Re: [PATCH v4 0/3] target/i386: Restrict system-specific features from user emulation

[Philippe Mathieu-Daudé]

On 12/9/23 19:25, Paolo Bonzini wrote:


 > However, the dependency of user-mode emulation on KVM is really an
 > implementation detail of QEMU.  It's very much baked into
linux-user and
 > hard to remove, but I'm not sure it's a good idea to add more #ifdef
 > CONFIG_USER_ONLY around KVM code.

Do you rather v3 then?

https://lore.kernel.org/qemu-devel/20230911142729.25548-1-phi...@linaro.org/ 



No, if we want a small patch it is better to replace kvm_enabled() with 
CONFIG_KVM, and also follow Kevin's suggestion to make it fail at 
compile time.


For target common code (shared between user/system), CONFIG_KVM is not
a replacement for !CONFIG_USER_ONLY, since it can be always enabled;
which is why we defer to runtime check with kvm_enabled().

Having stub prototypes was done because we expected the compiler to 
remove the dead code.


Paolo

Re: [PATCH v4 0/3] target/i386: Restrict system-specific features from user emulation

[Paolo Bonzini]

Il mar 12 set 2023, 18:44 Philippe Mathieu-Daudé  ha
scritto:

> I can try to improve it with your comments, but I have no idea of
> x86 CPU features.
>

I will take a look.

> However, the dependency of user-mode emulation on KVM is really an
> > implementation detail of QEMU.  It's very much baked into linux-user and
> > hard to remove, but I'm not sure it's a good idea to add more #ifdef
> > CONFIG_USER_ONLY around KVM code.
>
> Do you rather v3 then?
>
>
> https://lore.kernel.org/qemu-devel/20230911142729.25548-1-phi...@linaro.org/


No, if we want a small patch it is better to replace kvm_enabled() with
CONFIG_KVM, and also follow Kevin's suggestion to make it fail at compile
time.

Having stub prototypes was done because we expected the compiler to remove
the dead code.

Paolo

Re: [PATCH v4 0/3] target/i386: Restrict system-specific features from user emulation

[Philippe Mathieu-Daudé]

On 12/9/23 16:07, Paolo Bonzini wrote:

On 9/11/23 23:13, Philippe Mathieu-Daudé wrote:

Too many system-specific code (and in particular KVM related)
is pulled in user-only build. This led to adding unjustified
stubs as kludge to unagressive linker non-optimizations.

This series restrict x86 system-specific features to sysemu,
so we don't require any stub, and remove all x86 KVM declarations
from user emulation code (to trigger compile failure instead of
link one).

Philippe Mathieu-Daudé (3):
   target/i386: Check kvm_hyperv_expand_features() return value
   RFC target/i386: Restrict system-specific features from user emulation
   target/i386: Prohibit target specific KVM prototypes on user emulation


At least, patch 2 should be changed so that the #ifdef'ery is done at a 
higher level.


I can try to improve it with your comments, but I have no idea of
x86 CPU features.

However, the dependency of user-mode emulation on KVM is really an 
implementation detail of QEMU.  It's very much baked into linux-user and 
hard to remove, but I'm not sure it's a good idea to add more #ifdef 
CONFIG_USER_ONLY around KVM code.


Do you rather v3 then?

https://lore.kernel.org/qemu-devel/20230911142729.25548-1-phi...@linaro.org/

Re: [PATCH v4 0/3] target/i386: Restrict system-specific features from user emulation

[Paolo Bonzini]

On 9/11/23 23:13, Philippe Mathieu-Daudé wrote:

Too many system-specific code (and in particular KVM related)
is pulled in user-only build. This led to adding unjustified
stubs as kludge to unagressive linker non-optimizations.

This series restrict x86 system-specific features to sysemu,
so we don't require any stub, and remove all x86 KVM declarations
from user emulation code (to trigger compile failure instead of
link one).

Philippe Mathieu-Daudé (3):
   target/i386: Check kvm_hyperv_expand_features() return value
   RFC target/i386: Restrict system-specific features from user emulation
   target/i386: Prohibit target specific KVM prototypes on user emulation


At least, patch 2 should be changed so that the #ifdef'ery is done at a 
higher level.


However, the dependency of user-mode emulation on KVM is really an 
implementation detail of QEMU.  It's very much baked into linux-user and 
hard to remove, but I'm not sure it's a good idea to add more #ifdef 
CONFIG_USER_ONLY around KVM code.


Paolo

[PATCH v4 0/3] target/i386: Restrict system-specific features from user emulation

[Philippe Mathieu-Daudé]

Too many system-specific code (and in particular KVM related)
is pulled in user-only build. This led to adding unjustified
stubs as kludge to unagressive linker non-optimizations.

This series restrict x86 system-specific features to sysemu,
so we don't require any stub, and remove all x86 KVM declarations
from user emulation code (to trigger compile failure instead of
link one).

Philippe Mathieu-Daudé (3):
  target/i386: Check kvm_hyperv_expand_features() return value
  RFC target/i386: Restrict system-specific features from user emulation
  target/i386: Prohibit target specific KVM prototypes on user emulation

 target/i386/kvm/kvm_i386.h |  4 +++
 target/i386/cpu.c  | 58 +-
 2 files changed, 55 insertions(+), 7 deletions(-)

-- 
2.41.0