[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 5/5] fw_cfg: insert fw_cfg file blobs via qem
From: |
Gabriel L. Somlo |
Subject: |
Re: [Qemu-devel] [PATCH v2 5/5] fw_cfg: insert fw_cfg file blobs via qemu cmdline |
Date: |
Fri, 20 Mar 2015 14:47:56 -0400 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
On Fri, Mar 20, 2015 at 02:01:25PM -0400, Gabriel L. Somlo wrote:
> On Thu, Mar 19, 2015 at 06:38:53PM +0100, Laszlo Ersek wrote:
> > Hmmmm... that's messy, again. fw_cfg is built into the qemu binary only
> > if you have CONFIG_SOFTMMU. I guess something like this should work:
> >
> > #ifdef CONFIG_SOFTMMU
> > /* okay, fw_cfg_find() is linked into the binary */
> >
> > if (fw_cfg_find()) {
> > /* okay, the board has set up fw_cfg in its MachineClass init
> > * function
> > */
> >
> > if (qemu_opts_foreach(qemu_find_opts("fw_cfg"), parse_fw_cfg,
> > NULL, 1)) {
> > exit(1);
> > }
> > }
> > #endif
> >
> > You should definitely wait & see what Markus & Gerd think about the above.
>
> I didn't use #ifdef around fw_cfg_find() -- I think the underlying
> object_resolve_path() will return NULL whether fw_cfg support isn't
> compiled in or if the fw_cfg device hasn't been initialized, so
> that should not be necessary [*].
>
> [*] Oh, wait. Right now, fw_cfg_find() is a function defined inside
> fw_cfg.c, so if that won't get compiled, we lose... How about turning
> it into a macro or inline function in fw_cfg.h instead ? Something like
>
> static inline FWCfgState *fw_cfg_find(void)
> {
> return FW_CFG(object_resolve_path(FW_CFG_PATH, NULL));
> }
>
> should work nicely -- any thoughts ?
That patch, by the way, would look something like below. At this
point, "#ifdef CONFIG_SOFTMMU" definitely looks cleaner, although
now we'd have the (fw_cfg <-> softmmu) relationship codified *both*
here, and in hw/Makefile.objs ("devices-dirs-$(CONFIG_SOFTMMU) += nvram/"),
which is why I'm not 100% thrilled by the idea...
Thanks,
--Gabriel
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 8d4ea25..7a7439a 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -31,14 +31,10 @@
#include "qemu/config-file.h"
#define FW_CFG_SIZE 2
-#define FW_CFG_NAME "fw_cfg"
-#define FW_CFG_PATH "/machine/" FW_CFG_NAME
-#define TYPE_FW_CFG "fw_cfg"
#define TYPE_FW_CFG_IO "fw_cfg_io"
#define TYPE_FW_CFG_MEM "fw_cfg_mem"
-#define FW_CFG(obj) OBJECT_CHECK(FWCfgState, (obj), TYPE_FW_CFG)
#define FW_CFG_IO(obj) OBJECT_CHECK(FWCfgIoState, (obj), TYPE_FW_CFG_IO)
#define FW_CFG_MEM(obj) OBJECT_CHECK(FWCfgMemState, (obj), TYPE_FW_CFG_MEM)
@@ -633,11 +629,6 @@ FWCfgState *fw_cfg_init_mem(hwaddr ctl_addr, hwaddr
data_addr)
}
-FWCfgState *fw_cfg_find(void)
-{
- return FW_CFG(object_resolve_path(FW_CFG_PATH, NULL));
-}
-
static void fw_cfg_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
index b2e10c2..10c7771 100644
--- a/include/hw/nvram/fw_cfg.h
+++ b/include/hw/nvram/fw_cfg.h
@@ -7,6 +7,7 @@
#include "exec/hwaddr.h"
#include "qemu/typedefs.h"
+#include "qom/object.h"
#endif
#define FW_CFG_SIGNATURE 0x00
@@ -81,7 +82,16 @@ FWCfgState *fw_cfg_init_mem(hwaddr ctl_addr, hwaddr
data_addr);
FWCfgState *fw_cfg_init_mem_wide(hwaddr ctl_addr, hwaddr data_addr,
uint32_t data_width);
-FWCfgState *fw_cfg_find(void);
+#define FW_CFG_NAME "fw_cfg"
+#define FW_CFG_PATH "/machine/" FW_CFG_NAME
+#define TYPE_FW_CFG "fw_cfg"
+
+#define FW_CFG(obj) OBJECT_CHECK(FWCfgState, (obj), TYPE_FW_CFG)
+
+static inline FWCfgState *fw_cfg_find(void)
+{
+ return FW_CFG(object_resolve_path(FW_CFG_PATH, NULL));
+}
#endif /* NO_QEMU_PROTOS */
[Qemu-devel] [PATCH v2 1/5] fw_cfg: add documentation file (docs/specs/fw_cfg.txt), Gabriel L. Somlo, 2015/03/18