qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/2] expose host-phys-bits to guest
@ 2022-09-08 11:31 Gerd Hoffmann
  2022-09-08 11:31 ` [PATCH v2 1/2] [temporary] reserve bit KVM_HINTS_PHYS_ADDRESS_SIZE_DATA_VALID Gerd Hoffmann
  2022-09-08 11:31 ` [PATCH v2 2/2] [RfC] expose host-phys-bits to guest Gerd Hoffmann
  0 siblings, 2 replies; 8+ messages in thread
From: Gerd Hoffmann @ 2022-09-08 11:31 UTC (permalink / raw)
  To: qemu-devel
  Cc: kvm, Paolo Bonzini, Marcel Apfelbaum, Marcelo Tosatti,
	Richard Henderson, Michael S. Tsirkin, Sergio Lopez,
	Eduardo Habkost, Gerd Hoffmann

When the guest (firmware specifically) knows how big
the address space actually is it can be used better.

Some more background:
  https://bugzilla.redhat.com/show_bug.cgi?id=2084533

This is a RfC series exposes the information via cpuid.

v2:
 - change fvm hint name.
 - better commit message.

take care,
  Gerd

Gerd Hoffmann (2):
  [temporary] reserve bit KVM_HINTS_PHYS_ADDRESS_SIZE_DATA_VALID
  [RfC] expose host-phys-bits to guest

 include/standard-headers/asm-x86/kvm_para.h | 3 ++-
 target/i386/cpu.h                           | 3 ---
 hw/i386/microvm.c                           | 7 ++++++-
 target/i386/cpu.c                           | 3 +--
 target/i386/host-cpu.c                      | 5 ++++-
 target/i386/kvm/kvm.c                       | 1 +
 6 files changed, 14 insertions(+), 8 deletions(-)

-- 
2.37.3



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 1/2] [temporary] reserve bit KVM_HINTS_PHYS_ADDRESS_SIZE_DATA_VALID
  2022-09-08 11:31 [PATCH v2 0/2] expose host-phys-bits to guest Gerd Hoffmann
@ 2022-09-08 11:31 ` Gerd Hoffmann
  2022-09-08 11:31 ` [PATCH v2 2/2] [RfC] expose host-phys-bits to guest Gerd Hoffmann
  1 sibling, 0 replies; 8+ messages in thread
From: Gerd Hoffmann @ 2022-09-08 11:31 UTC (permalink / raw)
  To: qemu-devel
  Cc: kvm, Paolo Bonzini, Marcel Apfelbaum, Marcelo Tosatti,
	Richard Henderson, Michael S. Tsirkin, Sergio Lopez,
	Eduardo Habkost, Gerd Hoffmann

The KVM_HINTS_PHYS_ADDRESS_SIZE_DATA_VALID bit hints to the guest
that the size of the physical address space as advertised by CPUID
leaf 0x80000008 is actually valid and can be used.

Unfortunately this is not the case today with qemu.  Default behavior is
to advertise 40 address bits (which I think comes from the very first x64
opteron processors).  There are lots of intel desktop processors around
which support less than that (36 or 39 depending on age), and when trying
to use the full 40 bit address space on those things go south quickly.

This renders the physical address size information effectively useless
for guests.  This patch paves the way to fix that by adding a hint for
the guest so it knows whenever the physical address size is usable or
not.

The plan for qemu is to set the bit when the physical address size is
valid.  That is the case when qemu is started with the host-phys-bits=on
option set for the cpu.  Eventually qemu can also flip the default for
that option from off to on, unfortunately that isn't easy for backward
compatibility reasons.

The plan for the firmware is to check that bit and when it is set just
query and use the available physical address space.  When the bit is not
set be conservative and try not exceed 36 bits (aka 64G) address space.
The latter is what the firmware does today unconditionally.

[ Temporary qemu patch for RfC patch and testing.  This change must
  actually be done in the linux kernel, then picked up by qemu via
  header file sync ].

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/standard-headers/asm-x86/kvm_para.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/standard-headers/asm-x86/kvm_para.h b/include/standard-headers/asm-x86/kvm_para.h
index f0235e58a1d3..962dabcfdb68 100644
--- a/include/standard-headers/asm-x86/kvm_para.h
+++ b/include/standard-headers/asm-x86/kvm_para.h
@@ -37,7 +37,8 @@
 #define KVM_FEATURE_HC_MAP_GPA_RANGE	16
 #define KVM_FEATURE_MIGRATION_CONTROL	17
 
-#define KVM_HINTS_REALTIME      0
+#define KVM_HINTS_REALTIME                      0
+#define KVM_HINTS_PHYS_ADDRESS_SIZE_DATA_VALID  1
 
 /* The last 8 bits are used to indicate how to interpret the flags field
  * in pvclock structure. If no bits are set, all flags are ignored.
-- 
2.37.3



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 2/2] [RfC] expose host-phys-bits to guest
  2022-09-08 11:31 [PATCH v2 0/2] expose host-phys-bits to guest Gerd Hoffmann
  2022-09-08 11:31 ` [PATCH v2 1/2] [temporary] reserve bit KVM_HINTS_PHYS_ADDRESS_SIZE_DATA_VALID Gerd Hoffmann
@ 2022-09-08 11:31 ` Gerd Hoffmann
  2022-09-08 14:19   ` Michael S. Tsirkin
  1 sibling, 1 reply; 8+ messages in thread
From: Gerd Hoffmann @ 2022-09-08 11:31 UTC (permalink / raw)
  To: qemu-devel
  Cc: kvm, Paolo Bonzini, Marcel Apfelbaum, Marcelo Tosatti,
	Richard Henderson, Michael S. Tsirkin, Sergio Lopez,
	Eduardo Habkost, Gerd Hoffmann

Move "host-phys-bits" property from cpu->host_phys_bits to
cpu->env.features[FEAT_KVM_HINTS] (KVM_HINTS_PHYS_ADDRESS_SIZE_DATA_VALID bit).

This has the effect that the guest can see whenever host-phys-bits
is turned on or not and act accordingly.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 target/i386/cpu.h      | 3 ---
 hw/i386/microvm.c      | 7 ++++++-
 target/i386/cpu.c      | 3 +--
 target/i386/host-cpu.c | 5 ++++-
 target/i386/kvm/kvm.c  | 1 +
 5 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 82004b65b944..b9c6d3d9cac6 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1898,9 +1898,6 @@ struct ArchCPU {
     /* if true fill the top bits of the MTRR_PHYSMASKn variable range */
     bool fill_mtrr_mask;
 
-    /* if true override the phys_bits value with a value read from the host */
-    bool host_phys_bits;
-
     /* if set, limit maximum value for phys_bits when host_phys_bits is true */
     uint8_t host_phys_bits_limit;
 
diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
index 52cafa003d8a..316bbc8ef946 100644
--- a/hw/i386/microvm.c
+++ b/hw/i386/microvm.c
@@ -54,6 +54,8 @@
 #include "kvm/kvm_i386.h"
 #include "hw/xen/start_info.h"
 
+#include "standard-headers/asm-x86/kvm_para.h"
+
 #define MICROVM_QBOOT_FILENAME "qboot.rom"
 #define MICROVM_BIOS_FILENAME  "bios-microvm.bin"
 
@@ -424,7 +426,10 @@ static void microvm_device_pre_plug_cb(HotplugHandler *hotplug_dev,
 {
     X86CPU *cpu = X86_CPU(dev);
 
-    cpu->host_phys_bits = true; /* need reliable phys-bits */
+    /* need reliable phys-bits */
+    cpu->env.features[FEAT_KVM_HINTS] |=
+        (1 << KVM_HINTS_PHYS_ADDRESS_SIZE_DATA_VALID);
+
     x86_cpu_pre_plug(hotplug_dev, dev, errp);
 }
 
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 1db1278a599b..d60f4498a3c3 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -778,7 +778,7 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
     [FEAT_KVM_HINTS] = {
         .type = CPUID_FEATURE_WORD,
         .feat_names = {
-            "kvm-hint-dedicated", NULL, NULL, NULL,
+            "kvm-hint-dedicated", "host-phys-bits", NULL, NULL,
             NULL, NULL, NULL, NULL,
             NULL, NULL, NULL, NULL,
             NULL, NULL, NULL, NULL,
@@ -7016,7 +7016,6 @@ static Property x86_cpu_properties[] = {
     DEFINE_PROP_BOOL("x-force-features", X86CPU, force_features, false),
     DEFINE_PROP_BOOL("kvm", X86CPU, expose_kvm, true),
     DEFINE_PROP_UINT32("phys-bits", X86CPU, phys_bits, 0),
-    DEFINE_PROP_BOOL("host-phys-bits", X86CPU, host_phys_bits, false),
     DEFINE_PROP_UINT8("host-phys-bits-limit", X86CPU, host_phys_bits_limit, 0),
     DEFINE_PROP_BOOL("fill-mtrr-mask", X86CPU, fill_mtrr_mask, true),
     DEFINE_PROP_UINT32("level-func7", X86CPU, env.cpuid_level_func7,
diff --git a/target/i386/host-cpu.c b/target/i386/host-cpu.c
index 10f8aba86e53..a1d6b3ac962e 100644
--- a/target/i386/host-cpu.c
+++ b/target/i386/host-cpu.c
@@ -13,6 +13,8 @@
 #include "qapi/error.h"
 #include "sysemu/sysemu.h"
 
+#include "standard-headers/asm-x86/kvm_para.h"
+
 /* Note: Only safe for use on x86(-64) hosts */
 static uint32_t host_cpu_phys_bits(void)
 {
@@ -68,7 +70,8 @@ static uint32_t host_cpu_adjust_phys_bits(X86CPU *cpu)
         warned = true;
     }
 
-    if (cpu->host_phys_bits) {
+    if (cpu->env.features[FEAT_KVM_HINTS] &
+        (1 << KVM_HINTS_PHYS_ADDRESS_SIZE_DATA_VALID)) {
         /* The user asked for us to use the host physical bits */
         phys_bits = host_phys_bits;
         if (cpu->host_phys_bits_limit &&
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index a1fd1f53791d..3335c57b21b2 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -459,6 +459,7 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s, uint32_t function,
         }
     } else if (function == KVM_CPUID_FEATURES && reg == R_EDX) {
         ret |= 1U << KVM_HINTS_REALTIME;
+        ret |= 1U << KVM_HINTS_PHYS_ADDRESS_SIZE_DATA_VALID;
     }
 
     return ret;
-- 
2.37.3



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/2] [RfC] expose host-phys-bits to guest
  2022-09-08 11:31 ` [PATCH v2 2/2] [RfC] expose host-phys-bits to guest Gerd Hoffmann
@ 2022-09-08 14:19   ` Michael S. Tsirkin
  2022-09-09  5:18     ` Gerd Hoffmann
  0 siblings, 1 reply; 8+ messages in thread
From: Michael S. Tsirkin @ 2022-09-08 14:19 UTC (permalink / raw)
  To: Gerd Hoffmann
  Cc: qemu-devel, kvm, Paolo Bonzini, Marcel Apfelbaum, Marcelo Tosatti,
	Richard Henderson, Sergio Lopez, Eduardo Habkost

On Thu, Sep 08, 2022 at 01:31:09PM +0200, Gerd Hoffmann wrote:
> Move "host-phys-bits" property from cpu->host_phys_bits to
> cpu->env.features[FEAT_KVM_HINTS] (KVM_HINTS_PHYS_ADDRESS_SIZE_DATA_VALID bit).
> 
> This has the effect that the guest can see whenever host-phys-bits
> is turned on or not and act accordingly.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  target/i386/cpu.h      | 3 ---
>  hw/i386/microvm.c      | 7 ++++++-
>  target/i386/cpu.c      | 3 +--
>  target/i386/host-cpu.c | 5 ++++-
>  target/i386/kvm/kvm.c  | 1 +
>  5 files changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 82004b65b944..b9c6d3d9cac6 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -1898,9 +1898,6 @@ struct ArchCPU {
>      /* if true fill the top bits of the MTRR_PHYSMASKn variable range */
>      bool fill_mtrr_mask;
>  
> -    /* if true override the phys_bits value with a value read from the host */
> -    bool host_phys_bits;
> -
>      /* if set, limit maximum value for phys_bits when host_phys_bits is true */
>      uint8_t host_phys_bits_limit;
>  
> diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
> index 52cafa003d8a..316bbc8ef946 100644
> --- a/hw/i386/microvm.c
> +++ b/hw/i386/microvm.c
> @@ -54,6 +54,8 @@
>  #include "kvm/kvm_i386.h"
>  #include "hw/xen/start_info.h"
>  
> +#include "standard-headers/asm-x86/kvm_para.h"
> +
>  #define MICROVM_QBOOT_FILENAME "qboot.rom"
>  #define MICROVM_BIOS_FILENAME  "bios-microvm.bin"
>  
> @@ -424,7 +426,10 @@ static void microvm_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>  {
>      X86CPU *cpu = X86_CPU(dev);
>  
> -    cpu->host_phys_bits = true; /* need reliable phys-bits */
> +    /* need reliable phys-bits */
> +    cpu->env.features[FEAT_KVM_HINTS] |=
> +        (1 << KVM_HINTS_PHYS_ADDRESS_SIZE_DATA_VALID);
> +

Do we need compat machinery for this?

>      x86_cpu_pre_plug(hotplug_dev, dev, errp);
>  }
>  
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 1db1278a599b..d60f4498a3c3 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -778,7 +778,7 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
>      [FEAT_KVM_HINTS] = {
>          .type = CPUID_FEATURE_WORD,
>          .feat_names = {
> -            "kvm-hint-dedicated", NULL, NULL, NULL,
> +            "kvm-hint-dedicated", "host-phys-bits", NULL, NULL,
>              NULL, NULL, NULL, NULL,
>              NULL, NULL, NULL, NULL,
>              NULL, NULL, NULL, NULL,
> @@ -7016,7 +7016,6 @@ static Property x86_cpu_properties[] = {
>      DEFINE_PROP_BOOL("x-force-features", X86CPU, force_features, false),
>      DEFINE_PROP_BOOL("kvm", X86CPU, expose_kvm, true),
>      DEFINE_PROP_UINT32("phys-bits", X86CPU, phys_bits, 0),
> -    DEFINE_PROP_BOOL("host-phys-bits", X86CPU, host_phys_bits, false),
>      DEFINE_PROP_UINT8("host-phys-bits-limit", X86CPU, host_phys_bits_limit, 0),
>      DEFINE_PROP_BOOL("fill-mtrr-mask", X86CPU, fill_mtrr_mask, true),
>      DEFINE_PROP_UINT32("level-func7", X86CPU, env.cpuid_level_func7,
> diff --git a/target/i386/host-cpu.c b/target/i386/host-cpu.c
> index 10f8aba86e53..a1d6b3ac962e 100644
> --- a/target/i386/host-cpu.c
> +++ b/target/i386/host-cpu.c
> @@ -13,6 +13,8 @@
>  #include "qapi/error.h"
>  #include "sysemu/sysemu.h"
>  
> +#include "standard-headers/asm-x86/kvm_para.h"
> +
>  /* Note: Only safe for use on x86(-64) hosts */
>  static uint32_t host_cpu_phys_bits(void)
>  {
> @@ -68,7 +70,8 @@ static uint32_t host_cpu_adjust_phys_bits(X86CPU *cpu)
>          warned = true;
>      }
>  
> -    if (cpu->host_phys_bits) {
> +    if (cpu->env.features[FEAT_KVM_HINTS] &
> +        (1 << KVM_HINTS_PHYS_ADDRESS_SIZE_DATA_VALID)) {
>          /* The user asked for us to use the host physical bits */
>          phys_bits = host_phys_bits;
>          if (cpu->host_phys_bits_limit &&

I think we still want to key this one off host_phys_bits
so it works for e.g. hyperv emulation too.

> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> index a1fd1f53791d..3335c57b21b2 100644
> --- a/target/i386/kvm/kvm.c
> +++ b/target/i386/kvm/kvm.c
> @@ -459,6 +459,7 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s, uint32_t function,
>          }
>      } else if (function == KVM_CPUID_FEATURES && reg == R_EDX) {
>          ret |= 1U << KVM_HINTS_REALTIME;
> +        ret |= 1U << KVM_HINTS_PHYS_ADDRESS_SIZE_DATA_VALID;
>      }
>  
>      return ret;
> -- 
> 2.37.3



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/2] [RfC] expose host-phys-bits to guest
  2022-09-08 14:19   ` Michael S. Tsirkin
@ 2022-09-09  5:18     ` Gerd Hoffmann
  2022-09-09  5:51       ` Michael S. Tsirkin
  0 siblings, 1 reply; 8+ messages in thread
From: Gerd Hoffmann @ 2022-09-09  5:18 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: qemu-devel, kvm, Paolo Bonzini, Marcel Apfelbaum, Marcelo Tosatti,
	Richard Henderson, Sergio Lopez, Eduardo Habkost

  Hi,

> > @@ -424,7 +426,10 @@ static void microvm_device_pre_plug_cb(HotplugHandler *hotplug_dev,
> >  {
> >      X86CPU *cpu = X86_CPU(dev);
> >  
> > -    cpu->host_phys_bits = true; /* need reliable phys-bits */
> > +    /* need reliable phys-bits */
> > +    cpu->env.features[FEAT_KVM_HINTS] |=
> > +        (1 << KVM_HINTS_PHYS_ADDRESS_SIZE_DATA_VALID);
> > +
> 
> Do we need compat machinery for this?

Don't think so, microvm has no versioned machine types anyway.

> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c

> >      [FEAT_KVM_HINTS] = {
> >          .type = CPUID_FEATURE_WORD,
> >          .feat_names = {
> > -            "kvm-hint-dedicated", NULL, NULL, NULL,
> > +            "kvm-hint-dedicated", "host-phys-bits", NULL, NULL,

> > -    DEFINE_PROP_BOOL("host-phys-bits", X86CPU, host_phys_bits, false),

> > -    if (cpu->host_phys_bits) {
> > +    if (cpu->env.features[FEAT_KVM_HINTS] &
> > +        (1 << KVM_HINTS_PHYS_ADDRESS_SIZE_DATA_VALID)) {
> >          /* The user asked for us to use the host physical bits */
> >          phys_bits = host_phys_bits;
> >          if (cpu->host_phys_bits_limit &&
> 
> I think we still want to key this one off host_phys_bits
> so it works for e.g. hyperv emulation too.

I think that should be the case.  The chunks above change the
host-phys-bits option from setting cpu->host_phys_bits to setting
the FEAT_KVM_HINTS bit.  That should also happen with hyperv emulation
enabled, and the bit should also be visible to the guest then, just at
another location (base 0x40000100 instead of 0x40000000).

take care,
  Gerd



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/2] [RfC] expose host-phys-bits to guest
  2022-09-09  5:18     ` Gerd Hoffmann
@ 2022-09-09  5:51       ` Michael S. Tsirkin
  2022-09-09  6:06         ` Gerd Hoffmann
  0 siblings, 1 reply; 8+ messages in thread
From: Michael S. Tsirkin @ 2022-09-09  5:51 UTC (permalink / raw)
  To: Gerd Hoffmann
  Cc: qemu-devel, kvm, Paolo Bonzini, Marcel Apfelbaum, Marcelo Tosatti,
	Richard Henderson, Sergio Lopez, Eduardo Habkost

On Fri, Sep 09, 2022 at 07:18:17AM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> > > @@ -424,7 +426,10 @@ static void microvm_device_pre_plug_cb(HotplugHandler *hotplug_dev,
> > >  {
> > >      X86CPU *cpu = X86_CPU(dev);
> > >  
> > > -    cpu->host_phys_bits = true; /* need reliable phys-bits */
> > > +    /* need reliable phys-bits */
> > > +    cpu->env.features[FEAT_KVM_HINTS] |=
> > > +        (1 << KVM_HINTS_PHYS_ADDRESS_SIZE_DATA_VALID);
> > > +
> > 
> > Do we need compat machinery for this?
> 
> Don't think so, microvm has no versioned machine types anyway.
> 
> > > --- a/target/i386/cpu.c
> > > +++ b/target/i386/cpu.c
> 
> > >      [FEAT_KVM_HINTS] = {
> > >          .type = CPUID_FEATURE_WORD,
> > >          .feat_names = {
> > > -            "kvm-hint-dedicated", NULL, NULL, NULL,
> > > +            "kvm-hint-dedicated", "host-phys-bits", NULL, NULL,
> 
> > > -    DEFINE_PROP_BOOL("host-phys-bits", X86CPU, host_phys_bits, false),
> 
> > > -    if (cpu->host_phys_bits) {
> > > +    if (cpu->env.features[FEAT_KVM_HINTS] &
> > > +        (1 << KVM_HINTS_PHYS_ADDRESS_SIZE_DATA_VALID)) {
> > >          /* The user asked for us to use the host physical bits */
> > >          phys_bits = host_phys_bits;
> > >          if (cpu->host_phys_bits_limit &&
> > 
> > I think we still want to key this one off host_phys_bits
> > so it works for e.g. hyperv emulation too.
> 
> I think that should be the case.  The chunks above change the
> host-phys-bits option from setting cpu->host_phys_bits to setting
> the FEAT_KVM_HINTS bit.  That should also happen with hyperv emulation
> enabled, and the bit should also be visible to the guest then, just at
> another location (base 0x40000100 instead of 0x40000000).
> 
> take care,
>   Gerd


You are right, I forgot. Hmm, ok. What about !cpu->expose_kvm ?

We have

    if (!kvm_enabled() || !cpu->expose_kvm) {
        env->features[FEAT_KVM] = 0;
    }   
        
This is quick grep, I didn't check whether this is called
after the point where you currently use it, but
it frankly seems fragile to pass a generic user specified flag
inside a cpuid where everyone pokes at it.


-- 
MST



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/2] [RfC] expose host-phys-bits to guest
  2022-09-09  5:51       ` Michael S. Tsirkin
@ 2022-09-09  6:06         ` Gerd Hoffmann
  2022-09-09  6:13           ` Michael S. Tsirkin
  0 siblings, 1 reply; 8+ messages in thread
From: Gerd Hoffmann @ 2022-09-09  6:06 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: qemu-devel, kvm, Paolo Bonzini, Marcel Apfelbaum, Marcelo Tosatti,
	Richard Henderson, Sergio Lopez, Eduardo Habkost

  Hi,

> > > I think we still want to key this one off host_phys_bits
> > > so it works for e.g. hyperv emulation too.
> > 
> > I think that should be the case.  The chunks above change the
> > host-phys-bits option from setting cpu->host_phys_bits to setting
> > the FEAT_KVM_HINTS bit.  That should also happen with hyperv emulation
> > enabled, and the bit should also be visible to the guest then, just at
> > another location (base 0x40000100 instead of 0x40000000).
> > 
> > take care,
> >   Gerd
> 
> 
> You are right, I forgot. Hmm, ok. What about !cpu->expose_kvm ?
> 
> We have
> 
>     if (!kvm_enabled() || !cpu->expose_kvm) {
>         env->features[FEAT_KVM] = 0;
>     }   
>         
> This is quick grep, I didn't check whether this is called
> after the point where you currently use it, but
> it frankly seems fragile to pass a generic user specified flag
> inside a cpuid where everyone pokes at it.

I tried to avoid keeping the state of the host_phys_bits option at
multiple places.  Maybe that wasn't a good idea after all.  How about
doing this instead:

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 1db1278a599b..279fde095d7c 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6219,6 +6219,11 @@ void x86_cpu_expand_features(X86CPU *cpu, Error **errp)
         env->features[FEAT_KVM] = 0;
     }
 
+    if (kvm_enabled() && cpu->host_phys_bits) {
+        env->features[FEAT_KVM_HINTS] |=
+            (1U << KVM_HINTS_PHYS_ADDRESS_SIZE_DATA_VALID);
+    }
+
     x86_cpu_enable_xsave_components(cpu);
 
     /* CPUID[EAX=7,ECX=0].EBX always increased level automatically: */
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index a1fd1f53791d..3335c57b21b2 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -459,6 +459,7 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s, uint32_t function,
         }
     } else if (function == KVM_CPUID_FEATURES && reg == R_EDX) {
         ret |= 1U << KVM_HINTS_REALTIME;
+        ret |= 1U << KVM_HINTS_PHYS_ADDRESS_SIZE_DATA_VALID;
     }
 
     return ret;



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/2] [RfC] expose host-phys-bits to guest
  2022-09-09  6:06         ` Gerd Hoffmann
@ 2022-09-09  6:13           ` Michael S. Tsirkin
  0 siblings, 0 replies; 8+ messages in thread
From: Michael S. Tsirkin @ 2022-09-09  6:13 UTC (permalink / raw)
  To: Gerd Hoffmann
  Cc: qemu-devel, kvm, Paolo Bonzini, Marcel Apfelbaum, Marcelo Tosatti,
	Richard Henderson, Sergio Lopez, Eduardo Habkost

On Fri, Sep 09, 2022 at 08:06:53AM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> > > > I think we still want to key this one off host_phys_bits
> > > > so it works for e.g. hyperv emulation too.
> > > 
> > > I think that should be the case.  The chunks above change the
> > > host-phys-bits option from setting cpu->host_phys_bits to setting
> > > the FEAT_KVM_HINTS bit.  That should also happen with hyperv emulation
> > > enabled, and the bit should also be visible to the guest then, just at
> > > another location (base 0x40000100 instead of 0x40000000).
> > > 
> > > take care,
> > >   Gerd
> > 
> > 
> > You are right, I forgot. Hmm, ok. What about !cpu->expose_kvm ?
> > 
> > We have
> > 
> >     if (!kvm_enabled() || !cpu->expose_kvm) {
> >         env->features[FEAT_KVM] = 0;
> >     }   
> >         
> > This is quick grep, I didn't check whether this is called
> > after the point where you currently use it, but
> > it frankly seems fragile to pass a generic user specified flag
> > inside a cpuid where everyone pokes at it.
> 
> I tried to avoid keeping the state of the host_phys_bits option at
> multiple places.  Maybe that wasn't a good idea after all.  How about
> doing this instead:
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 1db1278a599b..279fde095d7c 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -6219,6 +6219,11 @@ void x86_cpu_expand_features(X86CPU *cpu, Error **errp)
>          env->features[FEAT_KVM] = 0;
>      }
>  
> +    if (kvm_enabled() && cpu->host_phys_bits) {
> +        env->features[FEAT_KVM_HINTS] |=
> +            (1U << KVM_HINTS_PHYS_ADDRESS_SIZE_DATA_VALID);
> +    }
> +
>      x86_cpu_enable_xsave_components(cpu);
>  
>      /* CPUID[EAX=7,ECX=0].EBX always increased level automatically: */
> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> index a1fd1f53791d..3335c57b21b2 100644
> --- a/target/i386/kvm/kvm.c
> +++ b/target/i386/kvm/kvm.c
> @@ -459,6 +459,7 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s, uint32_t function,
>          }
>      } else if (function == KVM_CPUID_FEATURES && reg == R_EDX) {
>          ret |= 1U << KVM_HINTS_REALTIME;
> +        ret |= 1U << KVM_HINTS_PHYS_ADDRESS_SIZE_DATA_VALID;
>      }
>  
>      return ret;


/me nods.
That seems much more straight-forward.

-- 
MST



^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-09-09  6:23 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-09-08 11:31 [PATCH v2 0/2] expose host-phys-bits to guest Gerd Hoffmann
2022-09-08 11:31 ` [PATCH v2 1/2] [temporary] reserve bit KVM_HINTS_PHYS_ADDRESS_SIZE_DATA_VALID Gerd Hoffmann
2022-09-08 11:31 ` [PATCH v2 2/2] [RfC] expose host-phys-bits to guest Gerd Hoffmann
2022-09-08 14:19   ` Michael S. Tsirkin
2022-09-09  5:18     ` Gerd Hoffmann
2022-09-09  5:51       ` Michael S. Tsirkin
2022-09-09  6:06         ` Gerd Hoffmann
2022-09-09  6:13           ` Michael S. Tsirkin

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).