Re: [Qemu-devel] [PATCH 3/4] tests: fw_cfg: add reboot_timeout test case

[Li Qiang]

Thomas Huth <address@hidden> 于2019年4月24日周三 上午12:29写道：

> On 20/04/2019 12.00, Li Qiang wrote:
> > Signed-off-by: Li Qiang <address@hidden>
> > ---
> >  tests/fw_cfg-test.c | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> >
> > diff --git a/tests/fw_cfg-test.c b/tests/fw_cfg-test.c
> > index c22503619f..9f75dbb5f4 100644
> > --- a/tests/fw_cfg-test.c
> > +++ b/tests/fw_cfg-test.c
> > @@ -174,6 +174,24 @@ static void test_fw_cfg_boot_menu(void)
> >      qtest_quit(s);
> >  }
> >
> > +static void test_fw_cfg_reboot_timeout(void)
> > +{
> > +    QFWCFG *fw_cfg;
> > +    QTestState *s;
> > +    uint32_t reboot_timeout = 0;
> > +    size_t filesize;
> > +
> > +    s = qtest_init("-boot reboot-timeout=15");
> > +    fw_cfg = pc_fw_cfg_init(s);
> > +
> > +    filesize = qfw_cfg_get_file(fw_cfg, "etc/boot-fail-wait",
> > +                     &reboot_timeout, sizeof(reboot_timeout));
> > +    g_assert_cmpint(filesize, ==, sizeof(reboot_timeout));
> > +    g_assert_cmpint(reboot_timeout, ==, 15);
>
> 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. So
there is no need
to consider the file's endianess. If when the file entry is stored without
considering endianess,
it will be ok to ignore the endianess. So for this patch, the 'reboot_timeout'
is ok as when we stored
it we don't consider endianess. But for 'splash-time', when we stored it
we convert it to little endian
so when we fetches it, we should also convert it from little endian to
cpu-endian.

So this raises another question.
Should we consider the endianness for this 'file' entry used for 'integer
value'?
I read the seabios related code function 'qemu_cfg_read_file' and
'romfile_loadint'.
It doesn't consider endianess. This seems we should drop the 'endianess
convertion' in 'fw_cfg_bootsplash'
when we store 'etc/boot-menu-wait'.
However what's if the emulation target has a different endianess for the
host target. I think there will be inconsistent.
So I think we can have following choose:

1. Don't consider the endianess, this is ok when emaulated target and host
target has the same endianess.
2. definite an endianess for the 'file' entry used as an integer value.
When we store this kind of data we uses the definited endianess
and when we fetch it out, we also do an endianess convertion. This will
change the seabios I think.

Gerd, Philippe

What do you think of this?

Thanks,
Li Qiang





>  Thomas
>