Laszlo Ersek <ler...@redhat.com> 于2019年1月22日周二 上午5:32写道：
> On 01/20/19 08:13, Li Qiang wrote:
> > This is useful to write qtest about fw_cfg file entry.
> >
> > Signed-off-by: Li Qiang <liq...@163.com>
> > ---
> > tests/libqos/fw_cfg.c | 33 +++++++++++++++++++++++++++++++++
> > tests/libqos/fw_cfg.h | 2 ++
> > 2 files changed, 35 insertions(+)
> >
> > diff --git a/tests/libqos/fw_cfg.c b/tests/libqos/fw_cfg.c
> > index d0889d1e22..e2b0cae7cd 100644
> > --- a/tests/libqos/fw_cfg.c
> > +++ b/tests/libqos/fw_cfg.c
> > @@ -16,6 +16,7 @@
> > #include "libqos/fw_cfg.h"
> > #include "libqtest.h"
> > #include "qemu/bswap.h"
> > +#include "standard-headers/linux/qemu_fw_cfg.h"
> >
> > void qfw_cfg_select(QFWCFG *fw_cfg, uint16_t key)
> > {
> > @@ -54,6 +55,38 @@ uint64_t qfw_cfg_get_u64(QFWCFG *fw_cfg, uint16_t key)
> > return le64_to_cpu(value);
> > }
> >
> > +size_t qfw_cfg_get_file(QFWCFG *fw_cfg, const char *filename,
> > + void *data, size_t buflen)
> > +{
> > + uint32_t count;
> > + uint32_t i;
> > + unsigned char *filesbuf = NULL;
> > + uint32_t dsize;
>
> dsize should be size_t
>
>
OK.
> > + struct fw_cfg_file *p;
>
> Please use a better variable name, such as "dir_entry" or similar.
>
>
OK.
> > + size_t filesize = 0;
> > +
> > + qfw_cfg_get(fw_cfg, FW_CFG_FILE_DIR, &count, sizeof(count));
> > + count = be32_to_cpu(count);
> > + dsize = sizeof(uint32_t) + count * sizeof(struct fw_cfg_file);
>
> where does "struct fw_cfg_file" come from?
>
>
>From include/standard-headers/linux/qemu_fw_cfg.h
> ... more precisely: why use it over the "FWCfgFile" typedef? (applies to
> the declaration of "p" as well).
>
>
Ok, I think we should use include/hw/nvram/fw_cfg.h.
Thanks,
Li Qiang
> > + filesbuf = g_malloc0(dsize);
> > + qfw_cfg_get(fw_cfg, FW_CFG_FILE_DIR, filesbuf, dsize);
> > + p = (struct fw_cfg_file *)(filesbuf + sizeof(uint32_t));
> > + for (i = 0; i < count; ++i, ++p) {
> > + if (!strcmp(p->name, filename)) {
> > + uint32_t len = be32_to_cpu(p->size);
> > + uint16_t sel = be16_to_cpu(p->select);
> > + filesize = len;
> > + if (len > buflen) {
> > + len = buflen;
> > + }
> > + qfw_cfg_get(fw_cfg, sel, data, len);
> > + break;
> > + }
> > + }
> > + g_free(filesbuf);
> > + return filesize;
> > +}
> > +
> > static void mm_fw_cfg_select(QFWCFG *fw_cfg, uint16_t key)
> > {
> > qtest_writew(fw_cfg->qts, fw_cfg->base, key);
> > diff --git a/tests/libqos/fw_cfg.h b/tests/libqos/fw_cfg.h
> > index 0353416af0..de73722e67 100644
> > --- a/tests/libqos/fw_cfg.h
> > +++ b/tests/libqos/fw_cfg.h
> > @@ -31,6 +31,8 @@ void qfw_cfg_get(QFWCFG *fw_cfg, uint16_t key, void
> *data, size_t len);
> > uint16_t qfw_cfg_get_u16(QFWCFG *fw_cfg, uint16_t key);
> > uint32_t qfw_cfg_get_u32(QFWCFG *fw_cfg, uint16_t key);
> > uint64_t qfw_cfg_get_u64(QFWCFG *fw_cfg, uint16_t key);
> > +size_t qfw_cfg_get_file(QFWCFG *fw_cfg, const char *filename,
> > + void *data, size_t buflen);
> >
> > QFWCFG *mm_fw_cfg_init(QTestState *qts, uint64_t base);
> > QFWCFG *io_fw_cfg_init(QTestState *qts, uint16_t base);
> >
>
> Looks OK to me otherwise.
>
> Thanks,
> Laszlo
>
