[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/4] tests: fw_cfg: add reboot_timeout test case
From: |
Laszlo Ersek |
Subject: |
Re: [Qemu-devel] [PATCH 3/4] tests: fw_cfg: add reboot_timeout test case |
Date: |
Fri, 26 Apr 2019 18:57:21 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 |
On 04/25/19 12:23, Philippe Mathieu-Daudé wrote:
> On 4/25/19 10:29 AM, Li Qiang wrote:
>>
>>
>> Gerd Hoffmann <address@hidden <mailto:address@hidden>> 于2019年4月
>> 25日周四 下午4:15写道:
>>
>> On Wed, Apr 24, 2019 at 09:16:56AM +0800, Li Qiang wrote:
>> > Thomas Huth <address@hidden <mailto:address@hidden>> 于2019年4
>> 月24日周三 上午12:29写道:
>> >
>> > > Is this endianess-safe? Or do you need to byteswap
>> reboot_timeout if the
>> > > host and guest endianess does not match?
>> >
>> > Good question!
>> >
>> > IIUC, the qemu fw_cfg store the 'file' entry data just in byte stream.
>>
>> No. Integers are defined to be little endian. See fw_cfg_add_i64() for
>> example, there is an explicit cpu_to_le64() call for that.
>>
>>
>>
>> Yes, for the fw_cfg 'integer' entry it is stored as little endian.
>> But for the fw_cfg 'file' entry interpred as an integer, there is no
>> specify the endianess.
>
> I agree with Li, the endianess of 'reboot-timeout' is not clear.
>
> From docs/specs/fw_cfg.txt:
>
> === All Other Data Items ===
>
> Please consult the QEMU source for the most up-to-date
> and authoritative list of selector keys and their respective
> items' purpose, format and writeability.
>
> So checking the git history, this code was introduced in commit
> ac05f3492421, very similar to commit 3d3b8303c6f8 for the
> 'boot-menu-wait' entry, which explicitely use little-endian, so I think
> it is safe to consider it little-endian and add a comment about its
> endianess (referring the previous commits in the patch description).
OVMF consumes "boot-menu-wait", so that one must remain LE.
OVMF doesn't consume "reboot-timeout", so I can't really comment on it.
I guess, if a named fw_cfg file that already exists doesn't explicitly
define the endiannesses of its integer fields, err for safety and opt
for LE.
For new fw_cfg files with integers in them, choose the endianness
explicitly and document it.
Thanks
Laszlo
[Qemu-devel] [PATCH 4/4] tests: fw_cfg: add splash time test case, Li Qiang, 2019/04/20
[Qemu-devel] [PATCH 2/4] tests: fw_cfg: add a function to get the fw_cfg file, Li Qiang, 2019/04/20