Re: [PATCH 07/14] hw/i386/vmport: Add support for CMD_GETBIOSUUID

[Liran Alon]

On 10/03/2020 14:01, Michael S. Tsirkin wrote:
> On Tue, Mar 10, 2020 at 04:44:54AM -0700, Liran Alon wrote:
> > On 10/03/2020 11:22, Michael S. Tsirkin wrote:
> > > On Tue, Mar 10, 2020 at 01:54:04AM +0200, Liran Alon wrote:
> > > > This is VMware documented functionallity that some guests rely on.
> > > > Returns the BIOS UUID of the current virtual machine.
> > > >
> > > > Reviewed-by: Nikita Leshenko <address@hidden>
> > > > Signed-off-by: Liran Alon <address@hidden>
> > > > ---
> > > >    hw/i386/vmport.c     | 14 ++++++++++++++
> > > >    include/hw/i386/pc.h |  1 +
> > > >    2 files changed, 15 insertions(+)
> > > >
> > > > diff --git a/hw/i386/vmport.c b/hw/i386/vmport.c
> > > > index 2ae5afc42b50..7687f3368a55 100644
> > > > --- a/hw/i386/vmport.c
> > > > +++ b/hw/i386/vmport.c
> > > > @@ -26,6 +26,7 @@
> > > >    #include "hw/i386/pc.h"
> > > >    #include "hw/input/i8042.h"
> > > >    #include "hw/qdev-properties.h"
> > > > +#include "sysemu/sysemu.h"
> > > >    #include "sysemu/hw_accel.h"
> > > >    #include "qemu/log.h"
> > > >    #include "trace.h"
> > > > @@ -121,6 +122,18 @@ static uint32_t vmport_cmd_get_version(void *opaque, 
> > > > uint32_t addr)
> > > >        return port_state->vmx_version;
> > > >    }
> > > > +static uint32_t vmport_cmd_get_bios_uuid(void *opaque, uint32_t addr)
> > > > +{
> > > > +    X86CPU *cpu = X86_CPU(current_cpu);
> > > > +    uint32_t *uuid_parts = (uint32_t*)(qemu_uuid.data);
> BTW missing space before * here.
Yes. I'm fixing this in v2. So the Patchew bot email.
> > > > +
> > > > +    cpu->env.regs[R_EAX] = uuid_parts[0];
> > > > +    cpu->env.regs[R_EBX] = uuid_parts[1];
> > > > +    cpu->env.regs[R_ECX] = uuid_parts[2];
> > > > +    cpu->env.regs[R_EDX] = uuid_parts[3];
> > > > +    return cpu->env.regs[R_EAX];
> > > > +}
> > > > +
> > > Should be LE here?
> > No. This is how the UUID is expected to be returned to guest.
> >
> > -Liran
> Um *how* is it expected to be returned? IIUC this takes network order
> byte data and handles it as host endian. Assuming it's right on an LE
> host it isn't on a BE host.  So I am guessing you want le32_to_cpu here.

Oh I see. Because you say this code can also executes on a BE host using 
TCG.

qemu_uuid.data is in the byte order specified by RFC4122, which is 
indeed network-order.
The byte order of each register should be LE (As in x86 host).

E,g. For VMware's uuid.bios = " 56 4d 3e 7a 92 ee 4c 46-e8 0d 86 f3 68 
a0 cb e7", this should return:
EAX: 7a3e4d56
EBX: 464cee92
ECX: f3860de8
EDX: e7cba068

So I think you are right this should be le32_to_cpu(). i.e. Treat the 
uuid_part[x] as little-endian and convert it to native CPU format.

I always get confused in this :)

Thanks,
-Liran