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