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
