Re: [PATCH v2 2/2] [RfC] expose host-phys-bits to guest

From: Gerd Hoffmann <kraxel@redhat.com>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: qemu-devel@nongnu.org, kvm@vger.kernel.org,
	Paolo Bonzini <pbonzini@redhat.com>,
	Marcel Apfelbaum <marcel.apfelbaum@gmail.com>,
	Marcelo Tosatti <mtosatti@redhat.com>,
	Richard Henderson <richard.henderson@linaro.org>,
	Sergio Lopez <slp@redhat.com>,
	Eduardo Habkost <eduardo@habkost.net>
Subject: Re: [PATCH v2 2/2] [RfC] expose host-phys-bits to guest
Date: Fri, 9 Sep 2022 08:06:53 +0200	[thread overview]
Message-ID: <20220909060653.s4cf7caaem3p7ac3@sirius.home.kraxel.org> (raw)
In-Reply-To: <20220909014106-mutt-send-email-mst@kernel.org>

  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;

