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

[Michael S. Tsirkin]

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