Re: [PATCH 04/14] hw/i386/vmport: Introduce vmx-version property

[Michael S. Tsirkin]

On Tue, Mar 10, 2020 at 01:05:02PM +0200, Liran Alon wrote:
> 
> On 10/03/2020 11:32, Michael S. Tsirkin wrote:
> > On Tue, Mar 10, 2020 at 01:54:01AM +0200, Liran Alon wrote:
> > > Instead of hard-coding the VMX version, make it a VMPORT object property.
> > > This would allow user to control it's value via "-global 
> > > vmport.vmx-version=X".
> > > 
> > > Reviewed-by: Nikita Leshenko <address@hidden>
> > > Signed-off-by: Liran Alon <address@hidden>
> > More detail on why this is useful?
> It's more useful than returning a hard-coded "6" as the vmx-version...


Maybe default should be 6 (a bit of explanation why 6 could be nice).


> We have used it to preserve compatibility for some VMware guests that we run
> as-is on top of QEMU/KVM which expects specific vmx-version or else they
> fail to run properly.
> 
> -Liran

Any detail on which guest it is?
Pretending to be a very advanced version has its pitfalls if we
then don't behave the way vmware does, right?
Figuring out the version number is I suspect a bit much to ask of users.


> > 
> > > ---
> > >   hw/i386/vmport.c | 6 +++++-
> > >   1 file changed, 5 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/hw/i386/vmport.c b/hw/i386/vmport.c
> > > index 7c21e56081b0..a2c8ff4b59cf 100644
> > > --- a/hw/i386/vmport.c
> > > +++ b/hw/i386/vmport.c
> > > @@ -44,6 +44,8 @@ typedef struct VMPortState {
> > >       MemoryRegion io;
> > >       VMPortReadFunc *func[VMPORT_ENTRIES];
> > >       void *opaque[VMPORT_ENTRIES];
> > > +
> > > +    uint32_t vmx_version;
> > >   } VMPortState;
> > >   static VMPortState *port_state;
> > > @@ -112,7 +114,7 @@ static uint32_t vmport_cmd_get_version(void *opaque, 
> > > uint32_t addr)
> > >       X86CPU *cpu = X86_CPU(current_cpu);
> > >       cpu->env.regs[R_EBX] = VMPORT_MAGIC;
> > > -    return 6;
> > > +    return port_state->vmx_version;
> > >   }
> > >   static uint32_t vmport_cmd_ram_size(void *opaque, uint32_t addr)
> > > @@ -169,6 +171,8 @@ static void vmport_realizefn(DeviceState *dev, Error 
> > > **errp)
> > >   }
> > >   static Property vmport_properties[] = {
> > > +    /* Default value taken from open-vm-tools code VERSION_MAGIC 
> > > definition */
> > > +    DEFINE_PROP_UINT32("vmx-version", VMPortState, vmx_version, 6),
> > >       DEFINE_PROP_END_OF_LIST(),
> > >   };
> > > -- 
> > > 2.20.1