Re: [PATCH 05/14] hw/i386/vmport: Report VMX type in CMD_GETVERSION

[Michael S. Tsirkin]

On Tue, Mar 10, 2020 at 01:40:24PM +0200, Liran Alon wrote:
> 
> On 10/03/2020 13:23, Michael S. Tsirkin wrote:
> > On Tue, Mar 10, 2020 at 01:18:44PM +0200, Liran Alon wrote:
> > > On 10/03/2020 11:20, Michael S. Tsirkin wrote:
> > > > On Tue, Mar 10, 2020 at 01:54:02AM +0200, Liran Alon wrote:
> > > > > As can be seen from VmCheck_GetVersion() in open-vm-tools code,
> > > > > CMD_GETVERSION should return VMX type in ECX register.
> > > > > 
> > > > > Default is to fake host as VMware ESX server. But user can control
> > > > > this value by "-global vmport.vmx-type=X".
> > > > > 
> > > > > Reviewed-by: Nikita Leshenko <address@hidden>
> > > > > Signed-off-by: Liran Alon <address@hidden>
> > > > > ---
> > > > >    hw/i386/vmport.c | 13 +++++++++++++
> > > > >    1 file changed, 13 insertions(+)
> > > > > 
> > > > > diff --git a/hw/i386/vmport.c b/hw/i386/vmport.c
> > > > > index a2c8ff4b59cf..c03f57f2f636 100644
> > > > > --- a/hw/i386/vmport.c
> > > > > +++ b/hw/i386/vmport.c
> > > > > @@ -36,6 +36,15 @@
> > > > >    #define VMPORT_ENTRIES 0x2c
> > > > >    #define VMPORT_MAGIC   0x564D5868
> > > > > +typedef enum {
> > > > > +   VMX_TYPE_UNSET = 0,
> > > > > +   VMX_TYPE_EXPRESS,    /* Deprecated type used for VMware Express */
> > > > > +   VMX_TYPE_SCALABLE_SERVER,    /* VMware ESX server */
> > > > > +   VMX_TYPE_WGS,        /* Deprecated type used for VMware Server */
> > > > > +   VMX_TYPE_WORKSTATION,
> > > > > +   VMX_TYPE_WORKSTATION_ENTERPRISE /* Deprecated type used for ACE 
> > > > > 1.x */
> > > > > +} VMX_Type;
> > > > > +
> > > > Can names be prefixed with VMPort pls? VMX has specific unrelated 
> > > > meaning.
> > > > 
> > > > Same everywhere.
> > > I didn't thought it matters much given that this enum is only defined
> > > locally in vmport.c.
> > > But sure I can rename it in v2.
> > > 
> > > -Liran
> > Property names matter more.
> You mean to rename "vmx-version" and "vmx-type" to "vmport-vmx-version" and
> "vmport-vmx-type"?
> They are properties of vmport object so it seems redundant no? Also doesn't
> seem consistent which how properties of other objects in QEMU are named.
> (E.g. PVSCSI have "use_msg" property. Not "pvscsi_use_msg").
> But will do as you will suggest. Just asking for guidance of what you are
> looking for.
> 
> -Liran

Sorry - no I'm looking for an alternative to "vmx" everywhere but especially
in property names which need to be maintained. Maybe vm-exec or vmexec
as I suggested separately.

> > 
> > 
> > > > >    #define VMPORT(obj) OBJECT_CHECK(VMPortState, (obj), TYPE_VMPORT)
> > > > >    typedef struct VMPortState {
> > > > > @@ -46,6 +55,7 @@ typedef struct VMPortState {
> > > > >        void *opaque[VMPORT_ENTRIES];
> > > > >        uint32_t vmx_version;
> > > > > +    uint8_t vmx_type;
> > > > >    } VMPortState;
> > > > >    static VMPortState *port_state;
> > > > > @@ -114,6 +124,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;
> > > > > +    cpu->env.regs[R_ECX] = port_state->vmx_type;
> > > > >        return port_state->vmx_version;
> > > > >    }
> > > > > @@ -173,6 +184,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_UINT8("vmx-type", VMPortState, vmx_type,
> > > > > +                      VMX_TYPE_SCALABLE_SERVER),
> > > > >        DEFINE_PROP_END_OF_LIST(),
> > > > >    };
> > > > > -- 
> > > > > 2.20.1