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

[Liran Alon <liran.alon@oracle.com>]

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 <nikita.leshchenko@oracle.com>
>>>> Signed-off-by: Liran Alon <liran.alon@oracle.com>
>>>> ---
>>>>    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