* [PATCH v3 1/4] firmware: introduce sysfs driver for QEMU's fw_cfg device
2015-10-03 23:28 [PATCH v3 0/4] SysFS driver for QEMU fw_cfg device Gabriel L. Somlo
@ 2015-10-03 23:28 ` Gabriel L. Somlo
2015-10-04 1:34 ` kbuild test robot
` (3 more replies)
2015-10-03 23:28 ` [PATCH v3 2/4] firmware: use acpi to detect QEMU fw_cfg device for sysfs fw_cfg driver Gabriel L. Somlo
` (3 subsequent siblings)
4 siblings, 4 replies; 26+ messages in thread
From: Gabriel L. Somlo @ 2015-10-03 23:28 UTC (permalink / raw)
To: gregkh, paul, galak, will.deacon, agross, mark.rutland, zajec5,
hanjun.guo, catalin.marinas, linux-api, linux-kernel
Cc: peter.maydell, matt.fleming, mst, jordan.l.justen, kernelnewbies,
qemu-devel, leif.lindholm, ard.biesheuvel, kraxel, pbonzini,
lersek
From: Gabriel Somlo <somlo@cmu.edu>
Make fw_cfg entries of type "file" available via sysfs. Entries
are listed under /sys/firmware/qemu_fw_cfg/by_key, in folders
named after each entry's selector key. Filename, selector value,
and size read-only attributes are included for each entry. Also,
a "raw" attribute allows retrieval of the full binary content of
each entry.
This patch also provides a documentation file outlining the
guest-side "hardware" interface exposed by the QEMU fw_cfg device.
Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
---
.../ABI/testing/sysfs-firmware-qemu_fw_cfg | 167 ++++++++
drivers/firmware/Kconfig | 10 +
drivers/firmware/Makefile | 1 +
drivers/firmware/qemu_fw_cfg.c | 456 +++++++++++++++++++++
4 files changed, 634 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-firmware-qemu_fw_cfg
create mode 100644 drivers/firmware/qemu_fw_cfg.c
diff --git a/Documentation/ABI/testing/sysfs-firmware-qemu_fw_cfg b/Documentation/ABI/testing/sysfs-firmware-qemu_fw_cfg
new file mode 100644
index 0000000..f1ef44e
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-firmware-qemu_fw_cfg
@@ -0,0 +1,167 @@
+What: /sys/firmware/qemu_fw_cfg/
+Date: August 2015
+Contact: Gabriel Somlo <somlo@cmu.edu>
+Description:
+ Several different architectures supported by QEMU (x86, arm,
+ sun4*, ppc/mac) are provisioned with a firmware configuration
+ (fw_cfg) device, used by the host to provide configuration data
+ to the starting guest. While most of this data is meant for use
+ by the guest firmware, starting with QEMU v2.4, guest VMs may
+ be given arbitrary fw_cfg entries supplied directly on the
+ command line, which therefore may be of interest to userspace.
+
+ === Guest-side Hardware Interface ===
+
+ The fw_cfg device is available to guest VMs as a register pair
+ (control and data), accessible as either a IO ports or as MMIO
+ addresses, depending on the architecture.
+
+ --- Control Register ---
+
+ Width: 16-bit
+ Access: Write-Only
+ Endianness: LE (if IOport) or BE (if MMIO)
+
+ A write to the control register selects the index for one of
+ the firmware configuration items (or "blobs") available on the
+ fw_cfg device, which can subsequently be read from the data
+ register.
+
+ Each time the control register is written, an data offset
+ internal to the fw_cfg device will be set to zero. This data
+ offset impacts which portion of the selected fw_cfg blob is
+ accessed by reading the data register, as explained below.
+
+ --- Data Register ---
+
+ Width: 8-bit (if IOport), or 8/16/32/64-bit (if MMIO)
+ Access: Read-Only
+ Endianness: string preserving
+
+ The data register allows access to an array of bytes which
+ represent the fw_cfg blob last selected by a write to the
+ control register.
+
+ Immediately following a write to the control register, the data
+ offset will be set to zero. Each successful read access to the
+ data register will increment the data offset by the appropriate
+ access width.
+
+ Each fw_cfg blob has a maximum associated data length. Once the
+ data offset exceeds this maximum length, any subsequent reads
+ via the data register will return 0x00.
+
+ An N-byte wide read of the data register will return the next
+ available N bytes of the selected fw_cfg blob, as a substring,
+ in increasing address order, similar to memcpy(), zero-padded
+ if necessary should the maximum data length of the selected
+ item be reached, as described above.
+
+ --- Per-arch Register Details ---
+
+ -------------------------------------------------------------
+ arch access base ctrl ctrl data max.
+ mode address offset endian offset data
+ (bytes) (bytes)
+ -------------------------------------------------------------
+ x86* IOport 0x510 0 LE 1 1
+ arm MMIO 0x9020000 8 BE 0 8
+ sun4u IOport 0x510 0 LE 1 1
+ sun4m MMIO 0xd00000510 0 BE 2 1
+ ppc/mac MMIO 0xf0000510 0 BE 2 1
+ -------------------------------------------------------------
+
+ NOTE 1. On platforms where the fw_cfg registers are exposed as
+ IO ports, the data port number will always be one greater than
+ the port number of the control register. I.e., the two ports
+ are overlapping, and can not be mapped separately.
+
+ === Firmware Configuration Items of Interest ===
+
+ Originally, the index key, size, and formatting of blobs in
+ fw_cfg was hard coded by mutual agreement between QEMU on the
+ host side, and the guest-side firmware. Later on, a file
+ transfer interface was added: by reading a special blob, the
+ fw_cfg consumer can retrieve a list of records containing the
+ name, selector key, and size of further fw_cfg blobs made
+ available by the host. Below we describe three fw_cfg blobs
+ of interest to the sysfs driver.
+
+ --- Signature (Key 0x0000, FW_CFG_SIGNATURE) ---
+
+ The presence of the fw_cfg device can be verified by selecting
+ the signature blob by writing 0x0000 to the control register,
+ and reading four bytes from the data register. If the fw_cfg
+ device is present, the four bytes read will match the ASCII
+ characters "QEMU".
+
+ --- Revision (Key 0x0001, FW_CFG_ID) ---
+
+ A 32-bit little-endian unsigned integer, this item is used as
+ an interface revision number.
+
+ --- File Directory (Key 0x0019, FW_CFG_FILE_DIR) ---
+
+ Any fw_cfg blobs stored at key 0x0020 FW_CFG_FILE_FIRST() or
+ higher will have an associated entry in this "directory" blob,
+ which facilitates the discovery of available items by software
+ (e.g. BIOS) running on the guest. The format of the directory
+ blob is shown below.
+
+ NOTE: All integers are stored in big-endian format!
+
+ /* the entire file directory "blob" */
+ struct FWCfgFiles {
+ u32 count; /* total number of entries */
+ struct FWCfgFile f[]; /* entry array, see below */
+ };
+
+ /* an individual directory entry, 64 bytes total */
+ struct FWCfgFile {
+ u32 size; /* size of referenced blob */
+ u16 select; /* selector key for referenced blob */
+ u16 reserved;
+ char name[56]; /* blob name, nul-terminated ASCII */
+ };
+
+ === SysFS fw_cfg Interface ===
+
+ The fw_cfg sysfs interface described in this document is only
+ intended to display discoverable blobs (i.e., those registered
+ with the file directory), as there is no way to determine the
+ presence or size of "legacy" blobs (with selector keys between
+ 0x0002 and 0x0018) programmatically.
+
+ All fw_cfg information is shown under:
+
+ /sys/firmware/qemu_fw_cfg/
+
+ The only legacy blob displayed is the fw_cfg device revision:
+
+ /sys/firmware/qemu_fw_cfg/rev
+
+ --- Discoverable fw_cfg blobs by selector key ---
+
+ All discoverable blobs listed in the fw_cfg file directory are
+ displayed as entries named after their unique selector key
+ value, e.g.:
+
+ /sys/firmware/qemu_fw_cfg/by_key/32
+ /sys/firmware/qemu_fw_cfg/by_key/33
+ /sys/firmware/qemu_fw_cfg/by_key/34
+ ...
+
+ Each such fw_cfg sysfs entry has the following values exported
+ as attributes:
+
+ name : The 56-byte nul-terminated ASCII string used as the
+ blob's 'file name' in the fw_cfg directory.
+ size : The length of the blob, as given in the fw_cfg
+ directory.
+ key : The value of the blob's selector key as given in the
+ fw_cfg directory. This value is the same as used in
+ the parent directory name.
+ raw : The raw bytes of the blob, obtained by selecting the
+ entry via the control register, and reading a number
+ of bytes equal to the blob size from the data
+ register.
diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index 665efca..0466e80 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -135,6 +135,16 @@ config ISCSI_IBFT
detect iSCSI boot parameters dynamically during system boot, say Y.
Otherwise, say N.
+config FW_CFG_SYSFS
+ tristate "QEMU fw_cfg device support in sysfs"
+ depends on SYSFS
+ default n
+ help
+ Say Y or M here to enable the exporting of the QEMU firmware
+ configuration (fw_cfg) file entries via sysfs. Entries are
+ found under /sys/firmware/fw_cfg when this option is enabled
+ and loaded.
+
config QCOM_SCM
bool
depends on ARM || ARM64
diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
index 2ee8347..efba22a 100644
--- a/drivers/firmware/Makefile
+++ b/drivers/firmware/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_DMIID) += dmi-id.o
obj-$(CONFIG_ISCSI_IBFT_FIND) += iscsi_ibft_find.o
obj-$(CONFIG_ISCSI_IBFT) += iscsi_ibft.o
obj-$(CONFIG_FIRMWARE_MEMMAP) += memmap.o
+obj-$(CONFIG_FW_CFG_SYSFS) += qemu_fw_cfg.o
obj-$(CONFIG_QCOM_SCM) += qcom_scm.o
obj-$(CONFIG_QCOM_SCM_64) += qcom_scm-64.o
obj-$(CONFIG_QCOM_SCM_32) += qcom_scm-32.o
diff --git a/drivers/firmware/qemu_fw_cfg.c b/drivers/firmware/qemu_fw_cfg.c
new file mode 100644
index 0000000..3a67a16
--- /dev/null
+++ b/drivers/firmware/qemu_fw_cfg.c
@@ -0,0 +1,456 @@
+/*
+ * drivers/firmware/qemu_fw_cfg.c
+ *
+ * Expose entries from QEMU's firmware configuration (fw_cfg) device in
+ * sysfs (read-only, under "/sys/firmware/qemu_fw_cfg/...").
+ *
+ * Copyright 2015 Carnegie Mellon University
+ */
+
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/io.h>
+#include <linux/ioport.h>
+
+MODULE_AUTHOR("Gabriel L. Somlo <somlo@cmu.edu>");
+MODULE_DESCRIPTION("QEMU fw_cfg sysfs support");
+MODULE_LICENSE("GPL");
+
+/* selector key values for "well-known" fw_cfg entries */
+#define FW_CFG_SIGNATURE 0x00
+#define FW_CFG_ID 0x01
+#define FW_CFG_FILE_DIR 0x19
+
+/* size in bytes of fw_cfg signature */
+#define FW_CFG_SIG_SIZE 4
+
+/* fw_cfg "file name" is up to 56 characters (including terminating nul) */
+#define FW_CFG_MAX_FILE_PATH 56
+
+/* fw_cfg file directory entry type */
+struct fw_cfg_file {
+ u32 size;
+ u16 select;
+ u16 reserved;
+ char name[FW_CFG_MAX_FILE_PATH];
+};
+
+/* fw_cfg device i/o access options type */
+struct fw_cfg_access {
+ const char *name;
+ phys_addr_t base;
+ u8 size;
+ u8 ctrl_offset;
+ u8 data_offset;
+ bool is_mmio;
+};
+
+/* table of fw_cfg device i/o access options for known architectures */
+static struct fw_cfg_access fw_cfg_modes[] = {
+ {
+ .name = "fw_cfg IOport on i386, sun4u",
+ .base = 0x510,
+ .size = 0x02,
+ .ctrl_offset = 0x00,
+ .data_offset = 0x01,
+ .is_mmio = false,
+ }, {
+ .name = "fw_cfg MMIO on arm",
+ .base = 0x9020000,
+ .size = 0x0a,
+ .ctrl_offset = 0x08,
+ .data_offset = 0x00,
+ .is_mmio = true,
+ }, {
+ .name = "fw_cfg MMIO on sun4m",
+ .base = 0xd00000510,
+ .size = 0x03,
+ .ctrl_offset = 0x00,
+ .data_offset = 0x02,
+ .is_mmio = true,
+ }, {
+ .name = "fw_cfg MMIO on ppc/mac",
+ .base = 0xf0000510,
+ .size = 0x03,
+ .ctrl_offset = 0x00,
+ .data_offset = 0x02,
+ .is_mmio = true,
+ }, { } /* END */
+};
+
+/* fw_cfg device i/o currently selected option set */
+static struct fw_cfg_access *fw_cfg_mode;
+
+/* fw_cfg device i/o register addresses */
+static void __iomem *fw_cfg_dev_base;
+static void __iomem *fw_cfg_reg_ctrl;
+static void __iomem *fw_cfg_reg_data;
+
+/* atomic access to fw_cfg device (potentially slow i/o, so using mutex) */
+static DEFINE_MUTEX(fw_cfg_dev_lock);
+
+/* pick appropriate endianness for selector key */
+static inline u16 fw_cfg_sel_endianness(u16 key)
+{
+ return fw_cfg_mode->is_mmio ? cpu_to_be16(key) : cpu_to_le16(key);
+}
+
+/* type for fw_cfg "directory scan" visitor/callback function */
+typedef int (*fw_cfg_file_callback)(const struct fw_cfg_file *f);
+
+/* run a given callback on each fw_cfg directory entry */
+static int fw_cfg_scan_dir(fw_cfg_file_callback callback)
+{
+ int ret = 0;
+ u32 count, i;
+ struct fw_cfg_file f;
+
+ mutex_lock(&fw_cfg_dev_lock);
+ iowrite16(fw_cfg_sel_endianness(FW_CFG_FILE_DIR), fw_cfg_reg_ctrl);
+ ioread8_rep(fw_cfg_reg_data, &count, sizeof(count));
+ for (i = 0; i < be32_to_cpu(count); i++) {
+ ioread8_rep(fw_cfg_reg_data, &f, sizeof(f));
+ ret = callback(&f);
+ if (ret)
+ break;
+ }
+ mutex_unlock(&fw_cfg_dev_lock);
+ return ret;
+}
+
+/* read chunk of given fw_cfg blob (caller responsible for sanity-check) */
+static inline void fw_cfg_read_blob(u16 key,
+ void *buf, loff_t pos, size_t count)
+{
+ mutex_lock(&fw_cfg_dev_lock);
+ iowrite16(fw_cfg_sel_endianness(key), fw_cfg_reg_ctrl);
+ while (pos-- > 0)
+ ioread8(fw_cfg_reg_data);
+ ioread8_rep(fw_cfg_reg_data, buf, count);
+ mutex_unlock(&fw_cfg_dev_lock);
+}
+
+/* clean up fw_cfg device i/o */
+static void fw_cfg_io_cleanup(void)
+{
+ if (fw_cfg_mode->is_mmio) {
+ iounmap(fw_cfg_dev_base);
+ release_mem_region(fw_cfg_mode->base, fw_cfg_mode->size);
+ } else {
+ ioport_unmap(fw_cfg_dev_base);
+ release_region(fw_cfg_mode->base, fw_cfg_mode->size);
+ }
+}
+
+/* probe and map fw_cfg device */
+static int __init fw_cfg_io_probe(void)
+{
+ char sig[FW_CFG_SIG_SIZE];
+
+ for (fw_cfg_mode = &fw_cfg_modes[0];
+ fw_cfg_mode->base; fw_cfg_mode++) {
+
+ phys_addr_t base = fw_cfg_mode->base;
+ u8 size = fw_cfg_mode->size;
+
+ /* reserve and map mmio or ioport region */
+ if (fw_cfg_mode->is_mmio) {
+ if (!request_mem_region(base, size, fw_cfg_mode->name))
+ continue;
+ fw_cfg_dev_base = ioremap(base, size);
+ if (!fw_cfg_dev_base) {
+ release_mem_region(base, size);
+ continue;
+ }
+ } else {
+ if (!request_region(base, size, fw_cfg_mode->name))
+ continue;
+ fw_cfg_dev_base = ioport_map(base, size);
+ if (!fw_cfg_dev_base) {
+ release_region(base, size);
+ continue;
+ }
+ }
+
+ /* set control and data register addresses */
+ fw_cfg_reg_ctrl = fw_cfg_dev_base + fw_cfg_mode->ctrl_offset;
+ fw_cfg_reg_data = fw_cfg_dev_base + fw_cfg_mode->data_offset;
+
+ /* verify fw_cfg device signature */
+ fw_cfg_read_blob(FW_CFG_SIGNATURE, sig, 0, FW_CFG_SIG_SIZE);
+ if (memcmp(sig, "QEMU", FW_CFG_SIG_SIZE) == 0)
+ /* success, we're done */
+ return 0;
+
+ /* clean up before probing next access mode */
+ fw_cfg_io_cleanup();
+ }
+
+ return -ENODEV;
+}
+
+/* fw_cfg revision attribute, in /sys/firmware/qemu_fw_cfg top-level dir. */
+static u32 fw_cfg_rev;
+
+static ssize_t fw_cfg_showrev(struct kobject *k, struct attribute *a, char *buf)
+{
+ return sprintf(buf, "%u\n", fw_cfg_rev);
+}
+
+static const struct {
+ struct attribute attr;
+ ssize_t (*show)(struct kobject *k, struct attribute *a, char *buf);
+} fw_cfg_rev_attr = {
+ .attr = { .name = "rev", .mode = S_IRUSR },
+ .show = fw_cfg_showrev,
+};
+
+/* fw_cfg_sysfs_entry type */
+struct fw_cfg_sysfs_entry {
+ struct kobject kobj;
+ struct fw_cfg_file f;
+ struct list_head list;
+};
+
+/* get fw_cfg_sysfs_entry from kobject member */
+static inline struct fw_cfg_sysfs_entry *to_entry(struct kobject *kobj)
+{
+ return container_of(kobj, struct fw_cfg_sysfs_entry, kobj);
+}
+
+/* fw_cfg_sysfs_attribute type */
+struct fw_cfg_sysfs_attribute {
+ struct attribute attr;
+ ssize_t (*show)(struct fw_cfg_sysfs_entry *entry, char *buf);
+};
+
+/* get fw_cfg_sysfs_attribute from attribute member */
+static inline struct fw_cfg_sysfs_attribute *to_attr(struct attribute *attr)
+{
+ return container_of(attr, struct fw_cfg_sysfs_attribute, attr);
+}
+
+/* global cache of fw_cfg_sysfs_entry objects */
+static LIST_HEAD(fw_cfg_entry_cache);
+
+/* kobjects removed lazily by kernel, mutual exclusion needed */
+static DEFINE_SPINLOCK(fw_cfg_cache_lock);
+
+static inline void fw_cfg_sysfs_cache_enlist(struct fw_cfg_sysfs_entry *entry)
+{
+ spin_lock(&fw_cfg_cache_lock);
+ list_add_tail(&entry->list, &fw_cfg_entry_cache);
+ spin_unlock(&fw_cfg_cache_lock);
+}
+
+static inline void fw_cfg_sysfs_cache_delist(struct fw_cfg_sysfs_entry *entry)
+{
+ spin_lock(&fw_cfg_cache_lock);
+ list_del(&entry->list);
+ spin_unlock(&fw_cfg_cache_lock);
+}
+
+static void fw_cfg_sysfs_cache_cleanup(void)
+{
+ struct fw_cfg_sysfs_entry *entry, *next;
+
+ list_for_each_entry_safe(entry, next, &fw_cfg_entry_cache, list) {
+ /* will end up invoking fw_cfg_sysfs_cache_delist()
+ * via each object's release() method (i.e. destructor)
+ */
+ kobject_put(&entry->kobj);
+ }
+}
+
+/* default_attrs: per-entry attributes and show methods */
+
+#define FW_CFG_SYSFS_ATTR(_attr) \
+struct fw_cfg_sysfs_attribute fw_cfg_sysfs_attr_##_attr = { \
+ .attr = { .name = __stringify(_attr), .mode = S_IRUSR }, \
+ .show = fw_cfg_sysfs_show_##_attr, \
+}
+
+static ssize_t fw_cfg_sysfs_show_size(struct fw_cfg_sysfs_entry *e, char *buf)
+{
+ return sprintf(buf, "%u\n", e->f.size);
+}
+
+static ssize_t fw_cfg_sysfs_show_key(struct fw_cfg_sysfs_entry *e, char *buf)
+{
+ return sprintf(buf, "%u\n", e->f.select);
+}
+
+static ssize_t fw_cfg_sysfs_show_name(struct fw_cfg_sysfs_entry *e, char *buf)
+{
+ return sprintf(buf, "%s\n", e->f.name);
+}
+
+static FW_CFG_SYSFS_ATTR(size);
+static FW_CFG_SYSFS_ATTR(key);
+static FW_CFG_SYSFS_ATTR(name);
+
+static struct attribute *fw_cfg_sysfs_entry_attrs[] = {
+ &fw_cfg_sysfs_attr_size.attr,
+ &fw_cfg_sysfs_attr_key.attr,
+ &fw_cfg_sysfs_attr_name.attr,
+ NULL,
+};
+
+/* sysfs_ops: find fw_cfg_[entry, attribute] and call appropriate show method */
+static ssize_t fw_cfg_sysfs_attr_show(struct kobject *kobj, struct attribute *a,
+ char *buf)
+{
+ struct fw_cfg_sysfs_entry *entry = to_entry(kobj);
+ struct fw_cfg_sysfs_attribute *attr = to_attr(a);
+
+ return attr->show(entry, buf);
+}
+
+static const struct sysfs_ops fw_cfg_sysfs_attr_ops = {
+ .show = fw_cfg_sysfs_attr_show,
+};
+
+/* release: destructor, to be called via kobject_put() */
+static void fw_cfg_sysfs_release_entry(struct kobject *kobj)
+{
+ struct fw_cfg_sysfs_entry *entry = to_entry(kobj);
+
+ fw_cfg_sysfs_cache_delist(entry);
+ kfree(entry);
+}
+
+/* kobj_type: ties together all properties required to register an entry */
+static struct kobj_type fw_cfg_sysfs_entry_ktype = {
+ .default_attrs = fw_cfg_sysfs_entry_attrs,
+ .sysfs_ops = &fw_cfg_sysfs_attr_ops,
+ .release = fw_cfg_sysfs_release_entry,
+};
+
+/* raw-read method and attribute */
+static ssize_t fw_cfg_sysfs_read_raw(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr,
+ char *buf, loff_t pos, size_t count)
+{
+ struct fw_cfg_sysfs_entry *entry = to_entry(kobj);
+
+ if (pos > entry->f.size)
+ return -EINVAL;
+
+ if (count > entry->f.size - pos)
+ count = entry->f.size - pos;
+
+ fw_cfg_read_blob(entry->f.select, buf, pos, count);
+ return count;
+}
+
+static struct bin_attribute fw_cfg_sysfs_attr_raw = {
+ .attr = { .name = "raw", .mode = 0400 },
+ .read = fw_cfg_sysfs_read_raw,
+};
+
+/* kobjects & kset representing top-level, by_key, and by_name folders */
+static struct kobject *fw_cfg_top_ko;
+static struct kobject *fw_cfg_sel_ko;
+
+/* callback function to register an individual fw_cfg file */
+static int __init fw_cfg_register_file(const struct fw_cfg_file *f)
+{
+ int err;
+ struct fw_cfg_sysfs_entry *entry;
+
+ /* allocate new entry */
+ entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+ if (!entry)
+ return -ENOMEM;
+
+ /* set file entry information */
+ entry->f.size = be32_to_cpu(f->size);
+ entry->f.select = be16_to_cpu(f->select);
+ strcpy(entry->f.name, f->name);
+
+ /* register entry under "/sys/firmware/qemu_fw_cfg/by_key/" */
+ err = kobject_init_and_add(&entry->kobj, &fw_cfg_sysfs_entry_ktype,
+ fw_cfg_sel_ko, "%d", entry->f.select);
+ if (err)
+ goto err_register;
+
+ /* add raw binary content access */
+ err = sysfs_create_bin_file(&entry->kobj, &fw_cfg_sysfs_attr_raw);
+ if (err)
+ goto err_add_raw;
+
+ /* success, add entry to global cache */
+ fw_cfg_sysfs_cache_enlist(entry);
+ return 0;
+
+err_add_raw:
+ kobject_del(&entry->kobj);
+err_register:
+ kfree(entry);
+ return err;
+}
+
+/* unregister top-level or by_key folder */
+static inline void fw_cfg_kobj_cleanup(struct kobject *kobj)
+{
+ kobject_del(kobj);
+ kobject_put(kobj);
+}
+
+static int __init fw_cfg_sysfs_init(void)
+{
+ int err;
+
+ /* probe for the fw_cfg "hardware" */
+ err = fw_cfg_io_probe();
+ if (err)
+ return err;
+
+ /* create /sys/firmware/qemu_fw_cfg/ and its subdirectories */
+ err = -ENOMEM;
+ fw_cfg_top_ko = kobject_create_and_add("qemu_fw_cfg", firmware_kobj);
+ if (!fw_cfg_top_ko)
+ goto err_top;
+ fw_cfg_sel_ko = kobject_create_and_add("by_key", fw_cfg_top_ko);
+ if (!fw_cfg_sel_ko)
+ goto err_sel;
+
+ /* get revision number, add matching top-level attribute */
+ fw_cfg_read_blob(FW_CFG_ID, &fw_cfg_rev, 0, sizeof(fw_cfg_rev));
+ fw_cfg_rev = le32_to_cpu(fw_cfg_rev);
+ err = sysfs_create_file(fw_cfg_top_ko, &fw_cfg_rev_attr.attr);
+ if (err)
+ goto err_rev;
+
+ /* process fw_cfg file directory entry, registering each file */
+ err = fw_cfg_scan_dir(fw_cfg_register_file);
+ if (err)
+ goto err_scan;
+
+ /* success */
+ pr_debug("fw_cfg: loaded.\n");
+ return 0;
+
+err_scan:
+ fw_cfg_sysfs_cache_cleanup();
+ sysfs_remove_file(fw_cfg_top_ko, &fw_cfg_rev_attr.attr);
+err_rev:
+ fw_cfg_kobj_cleanup(fw_cfg_sel_ko);
+err_sel:
+ fw_cfg_kobj_cleanup(fw_cfg_top_ko);
+err_top:
+ fw_cfg_io_cleanup();
+ return err;
+}
+
+static void __exit fw_cfg_sysfs_exit(void)
+{
+ pr_debug("fw_cfg: unloading.\n");
+ fw_cfg_sysfs_cache_cleanup();
+ fw_cfg_kobj_cleanup(fw_cfg_sel_ko);
+ fw_cfg_kobj_cleanup(fw_cfg_top_ko);
+ fw_cfg_io_cleanup();
+}
+
+module_init(fw_cfg_sysfs_init);
+module_exit(fw_cfg_sysfs_exit);
--
2.4.3
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v3 1/4] firmware: introduce sysfs driver for QEMU's fw_cfg device
2015-10-03 23:28 ` [PATCH v3 1/4] firmware: introduce sysfs driver for QEMU's " Gabriel L. Somlo
@ 2015-10-04 1:34 ` kbuild test robot
2015-10-06 8:40 ` [Qemu-devel] " Stefan Hajnoczi
` (2 subsequent siblings)
3 siblings, 0 replies; 26+ messages in thread
From: kbuild test robot @ 2015-10-04 1:34 UTC (permalink / raw)
To: Gabriel L. Somlo
Cc: kbuild-all, gregkh, paul, galak, will.deacon, agross,
mark.rutland, zajec5, hanjun.guo, catalin.marinas, linux-api,
linux-kernel, kernelnewbies, matt.fleming, lersek,
jordan.l.justen, mst, peter.maydell, leif.lindholm,
ard.biesheuvel, pbonzini, kraxel, qemu-devel
Hi Gabriel,
[auto build test results on v4.3-rc3 -- if it's inappropriate base, please ignore]
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__
sparse warnings: (new ones prefixed by >>)
drivers/firmware/qemu_fw_cfg.c:66:25: sparse: constant 0xd00000510 is so big it is long
>> drivers/firmware/qemu_fw_cfg.c:95:39: sparse: restricted __be16 degrades to integer
>> drivers/firmware/qemu_fw_cfg.c:95:58: sparse: restricted __le16 degrades to integer
>> drivers/firmware/qemu_fw_cfg.c:111:25: sparse: cast to restricted __be32
>> drivers/firmware/qemu_fw_cfg.c:111:25: sparse: cast to restricted __be32
>> drivers/firmware/qemu_fw_cfg.c:111:25: sparse: cast to restricted __be32
>> drivers/firmware/qemu_fw_cfg.c:111:25: sparse: cast to restricted __be32
>> drivers/firmware/qemu_fw_cfg.c:111:25: sparse: cast to restricted __be32
>> drivers/firmware/qemu_fw_cfg.c:111:25: sparse: cast to restricted __be32
>> drivers/firmware/qemu_fw_cfg.c:95:39: sparse: restricted __be16 degrades to integer
>> drivers/firmware/qemu_fw_cfg.c:95:58: sparse: restricted __le16 degrades to integer
>> drivers/firmware/qemu_fw_cfg.c:95:39: sparse: restricted __be16 degrades to integer
>> drivers/firmware/qemu_fw_cfg.c:95:58: sparse: restricted __le16 degrades to integer
drivers/firmware/qemu_fw_cfg.c:367:25: sparse: cast to restricted __be32
drivers/firmware/qemu_fw_cfg.c:367:25: sparse: cast to restricted __be32
drivers/firmware/qemu_fw_cfg.c:367:25: sparse: cast to restricted __be32
drivers/firmware/qemu_fw_cfg.c:367:25: sparse: cast to restricted __be32
drivers/firmware/qemu_fw_cfg.c:367:25: sparse: cast to restricted __be32
drivers/firmware/qemu_fw_cfg.c:367:25: sparse: cast to restricted __be32
>> drivers/firmware/qemu_fw_cfg.c:368:27: sparse: cast to restricted __be16
>> drivers/firmware/qemu_fw_cfg.c:368:27: sparse: cast to restricted __be16
>> drivers/firmware/qemu_fw_cfg.c:368:27: sparse: cast to restricted __be16
>> drivers/firmware/qemu_fw_cfg.c:368:27: sparse: cast to restricted __be16
>> drivers/firmware/qemu_fw_cfg.c:95:39: sparse: restricted __be16 degrades to integer
>> drivers/firmware/qemu_fw_cfg.c:95:58: sparse: restricted __le16 degrades to integer
>> drivers/firmware/qemu_fw_cfg.c:420:22: sparse: cast to restricted __le32
vim +95 drivers/firmware/qemu_fw_cfg.c
60 .size = 0x0a,
61 .ctrl_offset = 0x08,
62 .data_offset = 0x00,
63 .is_mmio = true,
64 }, {
65 .name = "fw_cfg MMIO on sun4m",
> 66 .base = 0xd00000510,
67 .size = 0x03,
68 .ctrl_offset = 0x00,
69 .data_offset = 0x02,
70 .is_mmio = true,
71 }, {
72 .name = "fw_cfg MMIO on ppc/mac",
73 .base = 0xf0000510,
74 .size = 0x03,
75 .ctrl_offset = 0x00,
76 .data_offset = 0x02,
77 .is_mmio = true,
78 }, { } /* END */
79 };
80
81 /* fw_cfg device i/o currently selected option set */
82 static struct fw_cfg_access *fw_cfg_mode;
83
84 /* fw_cfg device i/o register addresses */
85 static void __iomem *fw_cfg_dev_base;
86 static void __iomem *fw_cfg_reg_ctrl;
87 static void __iomem *fw_cfg_reg_data;
88
89 /* atomic access to fw_cfg device (potentially slow i/o, so using mutex) */
90 static DEFINE_MUTEX(fw_cfg_dev_lock);
91
92 /* pick appropriate endianness for selector key */
93 static inline u16 fw_cfg_sel_endianness(u16 key)
94 {
> 95 return fw_cfg_mode->is_mmio ? cpu_to_be16(key) : cpu_to_le16(key);
96 }
97
98 /* type for fw_cfg "directory scan" visitor/callback function */
99 typedef int (*fw_cfg_file_callback)(const struct fw_cfg_file *f);
100
101 /* run a given callback on each fw_cfg directory entry */
102 static int fw_cfg_scan_dir(fw_cfg_file_callback callback)
103 {
104 int ret = 0;
105 u32 count, i;
106 struct fw_cfg_file f;
107
108 mutex_lock(&fw_cfg_dev_lock);
109 iowrite16(fw_cfg_sel_endianness(FW_CFG_FILE_DIR), fw_cfg_reg_ctrl);
110 ioread8_rep(fw_cfg_reg_data, &count, sizeof(count));
> 111 for (i = 0; i < be32_to_cpu(count); i++) {
112 ioread8_rep(fw_cfg_reg_data, &f, sizeof(f));
113 ret = callback(&f);
114 if (ret)
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [Qemu-devel] [PATCH v3 1/4] firmware: introduce sysfs driver for QEMU's fw_cfg device
2015-10-03 23:28 ` [PATCH v3 1/4] firmware: introduce sysfs driver for QEMU's " Gabriel L. Somlo
2015-10-04 1:34 ` kbuild test robot
@ 2015-10-06 8:40 ` Stefan Hajnoczi
[not found] ` <1443914889-9619-2-git-send-email-somlo-D+Gtc/HYRWM@public.gmane.org>
2015-10-06 17:54 ` Andy Lutomirski
3 siblings, 0 replies; 26+ messages in thread
From: Stefan Hajnoczi @ 2015-10-06 8:40 UTC (permalink / raw)
To: Gabriel L. Somlo
Cc: gregkh, paul, galak, will.deacon, agross, mark.rutland, zajec5,
hanjun.guo, catalin.marinas, linux-api, linux-kernel,
peter.maydell, matt.fleming, mst, jordan.l.justen, kernelnewbies,
qemu-devel, leif.lindholm, ard.biesheuvel, kraxel, pbonzini,
lersek
On Sat, Oct 03, 2015 at 07:28:06PM -0400, Gabriel L. Somlo wrote:
> +/* read chunk of given fw_cfg blob (caller responsible for sanity-check) */
> +static inline void fw_cfg_read_blob(u16 key,
> + void *buf, loff_t pos, size_t count)
> +{
> + mutex_lock(&fw_cfg_dev_lock);
> + iowrite16(fw_cfg_sel_endianness(key), fw_cfg_reg_ctrl);
> + while (pos-- > 0)
> + ioread8(fw_cfg_reg_data);
> + ioread8_rep(fw_cfg_reg_data, buf, count);
> + mutex_unlock(&fw_cfg_dev_lock);
> +}
Have you had a chance to play with Marc Mari's fw_cfg DMA interface
patches? They should make this operation much faster.
https://www.mail-archive.com/qemu-devel@nongnu.org/msg325541.html
Stefan
^ permalink raw reply [flat|nested] 26+ messages in thread[parent not found: <1443914889-9619-2-git-send-email-somlo-D+Gtc/HYRWM@public.gmane.org>]
* Re: [PATCH v3 1/4] firmware: introduce sysfs driver for QEMU's fw_cfg device
[not found] ` <1443914889-9619-2-git-send-email-somlo-D+Gtc/HYRWM@public.gmane.org>
@ 2015-10-06 12:53 ` Laszlo Ersek
0 siblings, 0 replies; 26+ messages in thread
From: Laszlo Ersek @ 2015-10-06 12:53 UTC (permalink / raw)
To: Gabriel L. Somlo
Cc: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
paul-DWxLp4Yu+b8AvxtiuMwx3w, galak-sgV2jX0FEOL9JmXXK+q4OQ,
will.deacon-5wv7dgnIgG8, agross-sgV2jX0FEOL9JmXXK+q4OQ,
mark.rutland-5wv7dgnIgG8, zajec5-Re5JQEeQqe8AvxtiuMwx3w,
hanjun.guo-QSEj5FYQhm4dnm+yROfE0A, catalin.marinas-5wv7dgnIgG8,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
kernelnewbies-7JyXY6prKcjpASu1u0TL5ti2O/JbrIOy,
matt.fleming-ral2JQCrhuEAvxtiuMwx3w,
jordan.l.justen-ral2JQCrhuEAvxtiuMwx3w,
mst-H+wXaHxf7aLQT0dZR+AlfA, peter.maydell-QSEj5FYQhm4dnm+yROfE0A,
leif.lindholm-QSEj5FYQhm4dnm+yROfE0A,
ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A,
pbonzini-H+wXaHxf7aLQT0dZR+AlfA, kraxel-H+wXaHxf7aLQT0dZR+AlfA,
qemu-devel-qX2TKyscuCcdnm+yROfE0A
On 10/04/15 01:28, Gabriel L. Somlo wrote:
> From: Gabriel Somlo <somlo-D+Gtc/HYRWM@public.gmane.org>
>
> Make fw_cfg entries of type "file" available via sysfs. Entries
> are listed under /sys/firmware/qemu_fw_cfg/by_key, in folders
> named after each entry's selector key. Filename, selector value,
> and size read-only attributes are included for each entry. Also,
> a "raw" attribute allows retrieval of the full binary content of
> each entry.
>
> This patch also provides a documentation file outlining the
> guest-side "hardware" interface exposed by the QEMU fw_cfg device.
>
> Signed-off-by: Gabriel Somlo <somlo-D+Gtc/HYRWM@public.gmane.org>
> ---
> .../ABI/testing/sysfs-firmware-qemu_fw_cfg | 167 ++++++++
> drivers/firmware/Kconfig | 10 +
> drivers/firmware/Makefile | 1 +
> drivers/firmware/qemu_fw_cfg.c | 456 +++++++++++++++++++++
> 4 files changed, 634 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-firmware-qemu_fw_cfg
> create mode 100644 drivers/firmware/qemu_fw_cfg.c
>
> diff --git a/Documentation/ABI/testing/sysfs-firmware-qemu_fw_cfg b/Documentation/ABI/testing/sysfs-firmware-qemu_fw_cfg
> new file mode 100644
> index 0000000..f1ef44e
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-firmware-qemu_fw_cfg
> @@ -0,0 +1,167 @@
> +What: /sys/firmware/qemu_fw_cfg/
> +Date: August 2015
> +Contact: Gabriel Somlo <somlo-D+Gtc/HYRWM@public.gmane.org>
> +Description:
> + Several different architectures supported by QEMU (x86, arm,
> + sun4*, ppc/mac) are provisioned with a firmware configuration
> + (fw_cfg) device, used by the host to provide configuration data
> + to the starting guest. While most of this data is meant for use
> + by the guest firmware, starting with QEMU v2.4, guest VMs may
> + be given arbitrary fw_cfg entries supplied directly on the
> + command line, which therefore may be of interest to userspace.
> +
> + === Guest-side Hardware Interface ===
> +
> + The fw_cfg device is available to guest VMs as a register pair
> + (control and data), accessible as either a IO ports or as MMIO
> + addresses, depending on the architecture.
> +
> + --- Control Register ---
> +
> + Width: 16-bit
> + Access: Write-Only
> + Endianness: LE (if IOport) or BE (if MMIO)
> +
> + A write to the control register selects the index for one of
> + the firmware configuration items (or "blobs") available on the
> + fw_cfg device, which can subsequently be read from the data
> + register.
> +
> + Each time the control register is written, an data offset
> + internal to the fw_cfg device will be set to zero. This data
> + offset impacts which portion of the selected fw_cfg blob is
> + accessed by reading the data register, as explained below.
> +
> + --- Data Register ---
> +
> + Width: 8-bit (if IOport), or 8/16/32/64-bit (if MMIO)
> + Access: Read-Only
> + Endianness: string preserving
> +
> + The data register allows access to an array of bytes which
> + represent the fw_cfg blob last selected by a write to the
> + control register.
> +
> + Immediately following a write to the control register, the data
> + offset will be set to zero. Each successful read access to the
> + data register will increment the data offset by the appropriate
> + access width.
> +
> + Each fw_cfg blob has a maximum associated data length. Once the
> + data offset exceeds this maximum length, any subsequent reads
> + via the data register will return 0x00.
> +
> + An N-byte wide read of the data register will return the next
> + available N bytes of the selected fw_cfg blob, as a substring,
> + in increasing address order, similar to memcpy(), zero-padded
> + if necessary should the maximum data length of the selected
> + item be reached, as described above.
> +
> + --- Per-arch Register Details ---
> +
> + -------------------------------------------------------------
> + arch access base ctrl ctrl data max.
> + mode address offset endian offset data
> + (bytes) (bytes)
> + -------------------------------------------------------------
> + x86* IOport 0x510 0 LE 1 1
> + arm MMIO 0x9020000 8 BE 0 8
> + sun4u IOport 0x510 0 LE 1 1
> + sun4m MMIO 0xd00000510 0 BE 2 1
> + ppc/mac MMIO 0xf0000510 0 BE 2 1
> + -------------------------------------------------------------
> +
> + NOTE 1. On platforms where the fw_cfg registers are exposed as
> + IO ports, the data port number will always be one greater than
> + the port number of the control register. I.e., the two ports
> + are overlapping, and can not be mapped separately.
> +
> + === Firmware Configuration Items of Interest ===
> +
> + Originally, the index key, size, and formatting of blobs in
> + fw_cfg was hard coded by mutual agreement between QEMU on the
> + host side, and the guest-side firmware. Later on, a file
> + transfer interface was added: by reading a special blob, the
> + fw_cfg consumer can retrieve a list of records containing the
> + name, selector key, and size of further fw_cfg blobs made
> + available by the host. Below we describe three fw_cfg blobs
> + of interest to the sysfs driver.
> +
> + --- Signature (Key 0x0000, FW_CFG_SIGNATURE) ---
> +
> + The presence of the fw_cfg device can be verified by selecting
> + the signature blob by writing 0x0000 to the control register,
> + and reading four bytes from the data register. If the fw_cfg
> + device is present, the four bytes read will match the ASCII
> + characters "QEMU".
> +
> + --- Revision (Key 0x0001, FW_CFG_ID) ---
> +
> + A 32-bit little-endian unsigned integer, this item is used as
> + an interface revision number.
> +
> + --- File Directory (Key 0x0019, FW_CFG_FILE_DIR) ---
> +
> + Any fw_cfg blobs stored at key 0x0020 FW_CFG_FILE_FIRST() or
> + higher will have an associated entry in this "directory" blob,
> + which facilitates the discovery of available items by software
> + (e.g. BIOS) running on the guest. The format of the directory
> + blob is shown below.
> +
> + NOTE: All integers are stored in big-endian format!
> +
> + /* the entire file directory "blob" */
> + struct FWCfgFiles {
> + u32 count; /* total number of entries */
> + struct FWCfgFile f[]; /* entry array, see below */
> + };
> +
> + /* an individual directory entry, 64 bytes total */
> + struct FWCfgFile {
> + u32 size; /* size of referenced blob */
> + u16 select; /* selector key for referenced blob */
> + u16 reserved;
> + char name[56]; /* blob name, nul-terminated ASCII */
> + };
I think the above is somewhat redundant with regard to
"Documentation/devicetree/bindings/arm/fw-cfg.txt". That may not
necessarily be a problem, I'd just like to make you aware of that file
too. Perhaps that file should be updated too.
... In any case, I wanted to call your attention to that file, because
it's been now raised that the driver should handle both ACPI and DT
guests. In DT, the device is already exposed -- that's how the UEFI
guest firmware for aarch64 guests learns about it.
So, if you'd like to follow the example of
"drivers/virtio/virtio_mmio.c" (which I think would be a good idea), in
order to recognize the device in both DT and ACPI guests, then on the
QEMU side, you won't have to do anything for DT, because
"qemu,fw-cfg-mmio" is already exposed in create_fw_cfg() [hw/arm/virt.c].
Thanks
Laszlo
> +
> + === SysFS fw_cfg Interface ===
> +
> + The fw_cfg sysfs interface described in this document is only
> + intended to display discoverable blobs (i.e., those registered
> + with the file directory), as there is no way to determine the
> + presence or size of "legacy" blobs (with selector keys between
> + 0x0002 and 0x0018) programmatically.
> +
> + All fw_cfg information is shown under:
> +
> + /sys/firmware/qemu_fw_cfg/
> +
> + The only legacy blob displayed is the fw_cfg device revision:
> +
> + /sys/firmware/qemu_fw_cfg/rev
> +
> + --- Discoverable fw_cfg blobs by selector key ---
> +
> + All discoverable blobs listed in the fw_cfg file directory are
> + displayed as entries named after their unique selector key
> + value, e.g.:
> +
> + /sys/firmware/qemu_fw_cfg/by_key/32
> + /sys/firmware/qemu_fw_cfg/by_key/33
> + /sys/firmware/qemu_fw_cfg/by_key/34
> + ...
> +
> + Each such fw_cfg sysfs entry has the following values exported
> + as attributes:
> +
> + name : The 56-byte nul-terminated ASCII string used as the
> + blob's 'file name' in the fw_cfg directory.
> + size : The length of the blob, as given in the fw_cfg
> + directory.
> + key : The value of the blob's selector key as given in the
> + fw_cfg directory. This value is the same as used in
> + the parent directory name.
> + raw : The raw bytes of the blob, obtained by selecting the
> + entry via the control register, and reading a number
> + of bytes equal to the blob size from the data
> + register.
> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
> index 665efca..0466e80 100644
> --- a/drivers/firmware/Kconfig
> +++ b/drivers/firmware/Kconfig
> @@ -135,6 +135,16 @@ config ISCSI_IBFT
> detect iSCSI boot parameters dynamically during system boot, say Y.
> Otherwise, say N.
>
> +config FW_CFG_SYSFS
> + tristate "QEMU fw_cfg device support in sysfs"
> + depends on SYSFS
> + default n
> + help
> + Say Y or M here to enable the exporting of the QEMU firmware
> + configuration (fw_cfg) file entries via sysfs. Entries are
> + found under /sys/firmware/fw_cfg when this option is enabled
> + and loaded.
> +
> config QCOM_SCM
> bool
> depends on ARM || ARM64
> diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
> index 2ee8347..efba22a 100644
> --- a/drivers/firmware/Makefile
> +++ b/drivers/firmware/Makefile
> @@ -12,6 +12,7 @@ obj-$(CONFIG_DMIID) += dmi-id.o
> obj-$(CONFIG_ISCSI_IBFT_FIND) += iscsi_ibft_find.o
> obj-$(CONFIG_ISCSI_IBFT) += iscsi_ibft.o
> obj-$(CONFIG_FIRMWARE_MEMMAP) += memmap.o
> +obj-$(CONFIG_FW_CFG_SYSFS) += qemu_fw_cfg.o
> obj-$(CONFIG_QCOM_SCM) += qcom_scm.o
> obj-$(CONFIG_QCOM_SCM_64) += qcom_scm-64.o
> obj-$(CONFIG_QCOM_SCM_32) += qcom_scm-32.o
> diff --git a/drivers/firmware/qemu_fw_cfg.c b/drivers/firmware/qemu_fw_cfg.c
> new file mode 100644
> index 0000000..3a67a16
> --- /dev/null
> +++ b/drivers/firmware/qemu_fw_cfg.c
> @@ -0,0 +1,456 @@
> +/*
> + * drivers/firmware/qemu_fw_cfg.c
> + *
> + * Expose entries from QEMU's firmware configuration (fw_cfg) device in
> + * sysfs (read-only, under "/sys/firmware/qemu_fw_cfg/...").
> + *
> + * Copyright 2015 Carnegie Mellon University
> + */
> +
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/io.h>
> +#include <linux/ioport.h>
> +
> +MODULE_AUTHOR("Gabriel L. Somlo <somlo-D+Gtc/HYRWM@public.gmane.org>");
> +MODULE_DESCRIPTION("QEMU fw_cfg sysfs support");
> +MODULE_LICENSE("GPL");
> +
> +/* selector key values for "well-known" fw_cfg entries */
> +#define FW_CFG_SIGNATURE 0x00
> +#define FW_CFG_ID 0x01
> +#define FW_CFG_FILE_DIR 0x19
> +
> +/* size in bytes of fw_cfg signature */
> +#define FW_CFG_SIG_SIZE 4
> +
> +/* fw_cfg "file name" is up to 56 characters (including terminating nul) */
> +#define FW_CFG_MAX_FILE_PATH 56
> +
> +/* fw_cfg file directory entry type */
> +struct fw_cfg_file {
> + u32 size;
> + u16 select;
> + u16 reserved;
> + char name[FW_CFG_MAX_FILE_PATH];
> +};
> +
> +/* fw_cfg device i/o access options type */
> +struct fw_cfg_access {
> + const char *name;
> + phys_addr_t base;
> + u8 size;
> + u8 ctrl_offset;
> + u8 data_offset;
> + bool is_mmio;
> +};
> +
> +/* table of fw_cfg device i/o access options for known architectures */
> +static struct fw_cfg_access fw_cfg_modes[] = {
> + {
> + .name = "fw_cfg IOport on i386, sun4u",
> + .base = 0x510,
> + .size = 0x02,
> + .ctrl_offset = 0x00,
> + .data_offset = 0x01,
> + .is_mmio = false,
> + }, {
> + .name = "fw_cfg MMIO on arm",
> + .base = 0x9020000,
> + .size = 0x0a,
> + .ctrl_offset = 0x08,
> + .data_offset = 0x00,
> + .is_mmio = true,
> + }, {
> + .name = "fw_cfg MMIO on sun4m",
> + .base = 0xd00000510,
> + .size = 0x03,
> + .ctrl_offset = 0x00,
> + .data_offset = 0x02,
> + .is_mmio = true,
> + }, {
> + .name = "fw_cfg MMIO on ppc/mac",
> + .base = 0xf0000510,
> + .size = 0x03,
> + .ctrl_offset = 0x00,
> + .data_offset = 0x02,
> + .is_mmio = true,
> + }, { } /* END */
> +};
> +
> +/* fw_cfg device i/o currently selected option set */
> +static struct fw_cfg_access *fw_cfg_mode;
> +
> +/* fw_cfg device i/o register addresses */
> +static void __iomem *fw_cfg_dev_base;
> +static void __iomem *fw_cfg_reg_ctrl;
> +static void __iomem *fw_cfg_reg_data;
> +
> +/* atomic access to fw_cfg device (potentially slow i/o, so using mutex) */
> +static DEFINE_MUTEX(fw_cfg_dev_lock);
> +
> +/* pick appropriate endianness for selector key */
> +static inline u16 fw_cfg_sel_endianness(u16 key)
> +{
> + return fw_cfg_mode->is_mmio ? cpu_to_be16(key) : cpu_to_le16(key);
> +}
> +
> +/* type for fw_cfg "directory scan" visitor/callback function */
> +typedef int (*fw_cfg_file_callback)(const struct fw_cfg_file *f);
> +
> +/* run a given callback on each fw_cfg directory entry */
> +static int fw_cfg_scan_dir(fw_cfg_file_callback callback)
> +{
> + int ret = 0;
> + u32 count, i;
> + struct fw_cfg_file f;
> +
> + mutex_lock(&fw_cfg_dev_lock);
> + iowrite16(fw_cfg_sel_endianness(FW_CFG_FILE_DIR), fw_cfg_reg_ctrl);
> + ioread8_rep(fw_cfg_reg_data, &count, sizeof(count));
> + for (i = 0; i < be32_to_cpu(count); i++) {
> + ioread8_rep(fw_cfg_reg_data, &f, sizeof(f));
> + ret = callback(&f);
> + if (ret)
> + break;
> + }
> + mutex_unlock(&fw_cfg_dev_lock);
> + return ret;
> +}
> +
> +/* read chunk of given fw_cfg blob (caller responsible for sanity-check) */
> +static inline void fw_cfg_read_blob(u16 key,
> + void *buf, loff_t pos, size_t count)
> +{
> + mutex_lock(&fw_cfg_dev_lock);
> + iowrite16(fw_cfg_sel_endianness(key), fw_cfg_reg_ctrl);
> + while (pos-- > 0)
> + ioread8(fw_cfg_reg_data);
> + ioread8_rep(fw_cfg_reg_data, buf, count);
> + mutex_unlock(&fw_cfg_dev_lock);
> +}
> +
> +/* clean up fw_cfg device i/o */
> +static void fw_cfg_io_cleanup(void)
> +{
> + if (fw_cfg_mode->is_mmio) {
> + iounmap(fw_cfg_dev_base);
> + release_mem_region(fw_cfg_mode->base, fw_cfg_mode->size);
> + } else {
> + ioport_unmap(fw_cfg_dev_base);
> + release_region(fw_cfg_mode->base, fw_cfg_mode->size);
> + }
> +}
> +
> +/* probe and map fw_cfg device */
> +static int __init fw_cfg_io_probe(void)
> +{
> + char sig[FW_CFG_SIG_SIZE];
> +
> + for (fw_cfg_mode = &fw_cfg_modes[0];
> + fw_cfg_mode->base; fw_cfg_mode++) {
> +
> + phys_addr_t base = fw_cfg_mode->base;
> + u8 size = fw_cfg_mode->size;
> +
> + /* reserve and map mmio or ioport region */
> + if (fw_cfg_mode->is_mmio) {
> + if (!request_mem_region(base, size, fw_cfg_mode->name))
> + continue;
> + fw_cfg_dev_base = ioremap(base, size);
> + if (!fw_cfg_dev_base) {
> + release_mem_region(base, size);
> + continue;
> + }
> + } else {
> + if (!request_region(base, size, fw_cfg_mode->name))
> + continue;
> + fw_cfg_dev_base = ioport_map(base, size);
> + if (!fw_cfg_dev_base) {
> + release_region(base, size);
> + continue;
> + }
> + }
> +
> + /* set control and data register addresses */
> + fw_cfg_reg_ctrl = fw_cfg_dev_base + fw_cfg_mode->ctrl_offset;
> + fw_cfg_reg_data = fw_cfg_dev_base + fw_cfg_mode->data_offset;
> +
> + /* verify fw_cfg device signature */
> + fw_cfg_read_blob(FW_CFG_SIGNATURE, sig, 0, FW_CFG_SIG_SIZE);
> + if (memcmp(sig, "QEMU", FW_CFG_SIG_SIZE) == 0)
> + /* success, we're done */
> + return 0;
> +
> + /* clean up before probing next access mode */
> + fw_cfg_io_cleanup();
> + }
> +
> + return -ENODEV;
> +}
> +
> +/* fw_cfg revision attribute, in /sys/firmware/qemu_fw_cfg top-level dir. */
> +static u32 fw_cfg_rev;
> +
> +static ssize_t fw_cfg_showrev(struct kobject *k, struct attribute *a, char *buf)
> +{
> + return sprintf(buf, "%u\n", fw_cfg_rev);
> +}
> +
> +static const struct {
> + struct attribute attr;
> + ssize_t (*show)(struct kobject *k, struct attribute *a, char *buf);
> +} fw_cfg_rev_attr = {
> + .attr = { .name = "rev", .mode = S_IRUSR },
> + .show = fw_cfg_showrev,
> +};
> +
> +/* fw_cfg_sysfs_entry type */
> +struct fw_cfg_sysfs_entry {
> + struct kobject kobj;
> + struct fw_cfg_file f;
> + struct list_head list;
> +};
> +
> +/* get fw_cfg_sysfs_entry from kobject member */
> +static inline struct fw_cfg_sysfs_entry *to_entry(struct kobject *kobj)
> +{
> + return container_of(kobj, struct fw_cfg_sysfs_entry, kobj);
> +}
> +
> +/* fw_cfg_sysfs_attribute type */
> +struct fw_cfg_sysfs_attribute {
> + struct attribute attr;
> + ssize_t (*show)(struct fw_cfg_sysfs_entry *entry, char *buf);
> +};
> +
> +/* get fw_cfg_sysfs_attribute from attribute member */
> +static inline struct fw_cfg_sysfs_attribute *to_attr(struct attribute *attr)
> +{
> + return container_of(attr, struct fw_cfg_sysfs_attribute, attr);
> +}
> +
> +/* global cache of fw_cfg_sysfs_entry objects */
> +static LIST_HEAD(fw_cfg_entry_cache);
> +
> +/* kobjects removed lazily by kernel, mutual exclusion needed */
> +static DEFINE_SPINLOCK(fw_cfg_cache_lock);
> +
> +static inline void fw_cfg_sysfs_cache_enlist(struct fw_cfg_sysfs_entry *entry)
> +{
> + spin_lock(&fw_cfg_cache_lock);
> + list_add_tail(&entry->list, &fw_cfg_entry_cache);
> + spin_unlock(&fw_cfg_cache_lock);
> +}
> +
> +static inline void fw_cfg_sysfs_cache_delist(struct fw_cfg_sysfs_entry *entry)
> +{
> + spin_lock(&fw_cfg_cache_lock);
> + list_del(&entry->list);
> + spin_unlock(&fw_cfg_cache_lock);
> +}
> +
> +static void fw_cfg_sysfs_cache_cleanup(void)
> +{
> + struct fw_cfg_sysfs_entry *entry, *next;
> +
> + list_for_each_entry_safe(entry, next, &fw_cfg_entry_cache, list) {
> + /* will end up invoking fw_cfg_sysfs_cache_delist()
> + * via each object's release() method (i.e. destructor)
> + */
> + kobject_put(&entry->kobj);
> + }
> +}
> +
> +/* default_attrs: per-entry attributes and show methods */
> +
> +#define FW_CFG_SYSFS_ATTR(_attr) \
> +struct fw_cfg_sysfs_attribute fw_cfg_sysfs_attr_##_attr = { \
> + .attr = { .name = __stringify(_attr), .mode = S_IRUSR }, \
> + .show = fw_cfg_sysfs_show_##_attr, \
> +}
> +
> +static ssize_t fw_cfg_sysfs_show_size(struct fw_cfg_sysfs_entry *e, char *buf)
> +{
> + return sprintf(buf, "%u\n", e->f.size);
> +}
> +
> +static ssize_t fw_cfg_sysfs_show_key(struct fw_cfg_sysfs_entry *e, char *buf)
> +{
> + return sprintf(buf, "%u\n", e->f.select);
> +}
> +
> +static ssize_t fw_cfg_sysfs_show_name(struct fw_cfg_sysfs_entry *e, char *buf)
> +{
> + return sprintf(buf, "%s\n", e->f.name);
> +}
> +
> +static FW_CFG_SYSFS_ATTR(size);
> +static FW_CFG_SYSFS_ATTR(key);
> +static FW_CFG_SYSFS_ATTR(name);
> +
> +static struct attribute *fw_cfg_sysfs_entry_attrs[] = {
> + &fw_cfg_sysfs_attr_size.attr,
> + &fw_cfg_sysfs_attr_key.attr,
> + &fw_cfg_sysfs_attr_name.attr,
> + NULL,
> +};
> +
> +/* sysfs_ops: find fw_cfg_[entry, attribute] and call appropriate show method */
> +static ssize_t fw_cfg_sysfs_attr_show(struct kobject *kobj, struct attribute *a,
> + char *buf)
> +{
> + struct fw_cfg_sysfs_entry *entry = to_entry(kobj);
> + struct fw_cfg_sysfs_attribute *attr = to_attr(a);
> +
> + return attr->show(entry, buf);
> +}
> +
> +static const struct sysfs_ops fw_cfg_sysfs_attr_ops = {
> + .show = fw_cfg_sysfs_attr_show,
> +};
> +
> +/* release: destructor, to be called via kobject_put() */
> +static void fw_cfg_sysfs_release_entry(struct kobject *kobj)
> +{
> + struct fw_cfg_sysfs_entry *entry = to_entry(kobj);
> +
> + fw_cfg_sysfs_cache_delist(entry);
> + kfree(entry);
> +}
> +
> +/* kobj_type: ties together all properties required to register an entry */
> +static struct kobj_type fw_cfg_sysfs_entry_ktype = {
> + .default_attrs = fw_cfg_sysfs_entry_attrs,
> + .sysfs_ops = &fw_cfg_sysfs_attr_ops,
> + .release = fw_cfg_sysfs_release_entry,
> +};
> +
> +/* raw-read method and attribute */
> +static ssize_t fw_cfg_sysfs_read_raw(struct file *filp, struct kobject *kobj,
> + struct bin_attribute *bin_attr,
> + char *buf, loff_t pos, size_t count)
> +{
> + struct fw_cfg_sysfs_entry *entry = to_entry(kobj);
> +
> + if (pos > entry->f.size)
> + return -EINVAL;
> +
> + if (count > entry->f.size - pos)
> + count = entry->f.size - pos;
> +
> + fw_cfg_read_blob(entry->f.select, buf, pos, count);
> + return count;
> +}
> +
> +static struct bin_attribute fw_cfg_sysfs_attr_raw = {
> + .attr = { .name = "raw", .mode = 0400 },
> + .read = fw_cfg_sysfs_read_raw,
> +};
> +
> +/* kobjects & kset representing top-level, by_key, and by_name folders */
> +static struct kobject *fw_cfg_top_ko;
> +static struct kobject *fw_cfg_sel_ko;
> +
> +/* callback function to register an individual fw_cfg file */
> +static int __init fw_cfg_register_file(const struct fw_cfg_file *f)
> +{
> + int err;
> + struct fw_cfg_sysfs_entry *entry;
> +
> + /* allocate new entry */
> + entry = kzalloc(sizeof(*entry), GFP_KERNEL);
> + if (!entry)
> + return -ENOMEM;
> +
> + /* set file entry information */
> + entry->f.size = be32_to_cpu(f->size);
> + entry->f.select = be16_to_cpu(f->select);
> + strcpy(entry->f.name, f->name);
> +
> + /* register entry under "/sys/firmware/qemu_fw_cfg/by_key/" */
> + err = kobject_init_and_add(&entry->kobj, &fw_cfg_sysfs_entry_ktype,
> + fw_cfg_sel_ko, "%d", entry->f.select);
> + if (err)
> + goto err_register;
> +
> + /* add raw binary content access */
> + err = sysfs_create_bin_file(&entry->kobj, &fw_cfg_sysfs_attr_raw);
> + if (err)
> + goto err_add_raw;
> +
> + /* success, add entry to global cache */
> + fw_cfg_sysfs_cache_enlist(entry);
> + return 0;
> +
> +err_add_raw:
> + kobject_del(&entry->kobj);
> +err_register:
> + kfree(entry);
> + return err;
> +}
> +
> +/* unregister top-level or by_key folder */
> +static inline void fw_cfg_kobj_cleanup(struct kobject *kobj)
> +{
> + kobject_del(kobj);
> + kobject_put(kobj);
> +}
> +
> +static int __init fw_cfg_sysfs_init(void)
> +{
> + int err;
> +
> + /* probe for the fw_cfg "hardware" */
> + err = fw_cfg_io_probe();
> + if (err)
> + return err;
> +
> + /* create /sys/firmware/qemu_fw_cfg/ and its subdirectories */
> + err = -ENOMEM;
> + fw_cfg_top_ko = kobject_create_and_add("qemu_fw_cfg", firmware_kobj);
> + if (!fw_cfg_top_ko)
> + goto err_top;
> + fw_cfg_sel_ko = kobject_create_and_add("by_key", fw_cfg_top_ko);
> + if (!fw_cfg_sel_ko)
> + goto err_sel;
> +
> + /* get revision number, add matching top-level attribute */
> + fw_cfg_read_blob(FW_CFG_ID, &fw_cfg_rev, 0, sizeof(fw_cfg_rev));
> + fw_cfg_rev = le32_to_cpu(fw_cfg_rev);
> + err = sysfs_create_file(fw_cfg_top_ko, &fw_cfg_rev_attr.attr);
> + if (err)
> + goto err_rev;
> +
> + /* process fw_cfg file directory entry, registering each file */
> + err = fw_cfg_scan_dir(fw_cfg_register_file);
> + if (err)
> + goto err_scan;
> +
> + /* success */
> + pr_debug("fw_cfg: loaded.\n");
> + return 0;
> +
> +err_scan:
> + fw_cfg_sysfs_cache_cleanup();
> + sysfs_remove_file(fw_cfg_top_ko, &fw_cfg_rev_attr.attr);
> +err_rev:
> + fw_cfg_kobj_cleanup(fw_cfg_sel_ko);
> +err_sel:
> + fw_cfg_kobj_cleanup(fw_cfg_top_ko);
> +err_top:
> + fw_cfg_io_cleanup();
> + return err;
> +}
> +
> +static void __exit fw_cfg_sysfs_exit(void)
> +{
> + pr_debug("fw_cfg: unloading.\n");
> + fw_cfg_sysfs_cache_cleanup();
> + fw_cfg_kobj_cleanup(fw_cfg_sel_ko);
> + fw_cfg_kobj_cleanup(fw_cfg_top_ko);
> + fw_cfg_io_cleanup();
> +}
> +
> +module_init(fw_cfg_sysfs_init);
> +module_exit(fw_cfg_sysfs_exit);
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 1/4] firmware: introduce sysfs driver for QEMU's fw_cfg device
2015-10-03 23:28 ` [PATCH v3 1/4] firmware: introduce sysfs driver for QEMU's " Gabriel L. Somlo
` (2 preceding siblings ...)
[not found] ` <1443914889-9619-2-git-send-email-somlo-D+Gtc/HYRWM@public.gmane.org>
@ 2015-10-06 17:54 ` Andy Lutomirski
[not found] ` <CALCETrXdTf-85zzGYN28iGhcUMwE-h0o_GQ1XSFK-6q0AZNvgw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
3 siblings, 1 reply; 26+ messages in thread
From: Andy Lutomirski @ 2015-10-06 17:54 UTC (permalink / raw)
To: Gabriel L. Somlo
Cc: Greg KH, paul, galak, Will Deacon, agross, Mark Rutland, zajec5,
hanjun.guo, Catalin Marinas, Linux API,
linux-kernel@vger.kernel.org, kernelnewbies, Matt Fleming,
Laszlo Ersek, Jordan Justen (Intel address), Michael S. Tsirkin,
peter.maydell, Leif Lindholm, Ard Biesheuvel, Paolo Bonzini,
kraxel, qemu-devel@nongnu.org Developers
On Sat, Oct 3, 2015 at 4:28 PM, Gabriel L. Somlo <somlo@cmu.edu> wrote:
> From: Gabriel Somlo <somlo@cmu.edu>
>
> Make fw_cfg entries of type "file" available via sysfs. Entries
> are listed under /sys/firmware/qemu_fw_cfg/by_key, in folders
> named after each entry's selector key. Filename, selector value,
> and size read-only attributes are included for each entry. Also,
> a "raw" attribute allows retrieval of the full binary content of
> each entry.
>
> This patch also provides a documentation file outlining the
> guest-side "hardware" interface exposed by the QEMU fw_cfg device.
>
What's the status of "by_name"? There's a single (presumably
incorrect) mention of it in a comment in this patch.
I would prefer if the kernel populated by_name itself rather than
deferring that to udev, since I'd like to use this facility in virtme,
and I'd like to use fw_cfg very early on boot before I even start
udev.
--Andy
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v3 2/4] firmware: use acpi to detect QEMU fw_cfg device for sysfs fw_cfg driver
2015-10-03 23:28 [PATCH v3 0/4] SysFS driver for QEMU fw_cfg device Gabriel L. Somlo
2015-10-03 23:28 ` [PATCH v3 1/4] firmware: introduce sysfs driver for QEMU's " Gabriel L. Somlo
@ 2015-10-03 23:28 ` Gabriel L. Somlo
[not found] ` <1443914889-9619-3-git-send-email-somlo-D+Gtc/HYRWM@public.gmane.org>
2015-10-03 23:28 ` [PATCH v3 3/4] kobject: export kset_find_obj() for module use Gabriel L. Somlo
` (2 subsequent siblings)
4 siblings, 1 reply; 26+ messages in thread
From: Gabriel L. Somlo @ 2015-10-03 23:28 UTC (permalink / raw)
To: gregkh, paul, galak, will.deacon, agross, mark.rutland, zajec5,
hanjun.guo, catalin.marinas, linux-api, linux-kernel
Cc: peter.maydell, matt.fleming, mst, jordan.l.justen, kernelnewbies,
qemu-devel, leif.lindholm, ard.biesheuvel, kraxel, pbonzini,
lersek
From: Gabriel Somlo <somlo@cmu.edu>
Instead of blindly probing fw_cfg registers at known IOport and MMIO
locations, use the ACPI subsystem to determine whether a QEMU fw_cfg
device is present, and, if found, to initialize it.
This limits portability to architectures which support ACPI (x86 and
UEFI-enabled aarch64), but avoids touching hardware registers before
being certain that our device is present.
NOTE: The standard way to verify the presence of fw_cfg on arm VMs
would have been to use the device tree, but that would have left out
x86, which is the primary architecture targeted by this patch.
Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
---
.../ABI/testing/sysfs-firmware-qemu_fw_cfg | 4 +
drivers/firmware/Kconfig | 2 +-
drivers/firmware/qemu_fw_cfg.c | 201 +++++++++++----------
3 files changed, 113 insertions(+), 94 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-firmware-qemu_fw_cfg b/Documentation/ABI/testing/sysfs-firmware-qemu_fw_cfg
index f1ef44e..e9761bf 100644
--- a/Documentation/ABI/testing/sysfs-firmware-qemu_fw_cfg
+++ b/Documentation/ABI/testing/sysfs-firmware-qemu_fw_cfg
@@ -76,6 +76,10 @@ Description:
the port number of the control register. I.e., the two ports
are overlapping, and can not be mapped separately.
+ NOTE 2. QEMU publishes the register details in the device tree
+ on arm guests, and in ACPI (under _HID "QEMU0002") on x86 and
+ select arm (aarch64) VM types.
+
=== Firmware Configuration Items of Interest ===
Originally, the index key, size, and formatting of blobs in
diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index 0466e80..bc12d31 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -137,7 +137,7 @@ config ISCSI_IBFT
config FW_CFG_SYSFS
tristate "QEMU fw_cfg device support in sysfs"
- depends on SYSFS
+ depends on SYSFS && ACPI
default n
help
Say Y or M here to enable the exporting of the QEMU firmware
diff --git a/drivers/firmware/qemu_fw_cfg.c b/drivers/firmware/qemu_fw_cfg.c
index 3a67a16..f935afb 100644
--- a/drivers/firmware/qemu_fw_cfg.c
+++ b/drivers/firmware/qemu_fw_cfg.c
@@ -8,6 +8,7 @@
*/
#include <linux/module.h>
+#include <linux/acpi.h>
#include <linux/slab.h>
#include <linux/io.h>
#include <linux/ioport.h>
@@ -35,53 +36,10 @@ struct fw_cfg_file {
char name[FW_CFG_MAX_FILE_PATH];
};
-/* fw_cfg device i/o access options type */
-struct fw_cfg_access {
- const char *name;
- phys_addr_t base;
- u8 size;
- u8 ctrl_offset;
- u8 data_offset;
- bool is_mmio;
-};
-
-/* table of fw_cfg device i/o access options for known architectures */
-static struct fw_cfg_access fw_cfg_modes[] = {
- {
- .name = "fw_cfg IOport on i386, sun4u",
- .base = 0x510,
- .size = 0x02,
- .ctrl_offset = 0x00,
- .data_offset = 0x01,
- .is_mmio = false,
- }, {
- .name = "fw_cfg MMIO on arm",
- .base = 0x9020000,
- .size = 0x0a,
- .ctrl_offset = 0x08,
- .data_offset = 0x00,
- .is_mmio = true,
- }, {
- .name = "fw_cfg MMIO on sun4m",
- .base = 0xd00000510,
- .size = 0x03,
- .ctrl_offset = 0x00,
- .data_offset = 0x02,
- .is_mmio = true,
- }, {
- .name = "fw_cfg MMIO on ppc/mac",
- .base = 0xf0000510,
- .size = 0x03,
- .ctrl_offset = 0x00,
- .data_offset = 0x02,
- .is_mmio = true,
- }, { } /* END */
-};
-
-/* fw_cfg device i/o currently selected option set */
-static struct fw_cfg_access *fw_cfg_mode;
-
/* fw_cfg device i/o register addresses */
+static bool fw_cfg_is_mmio;
+static phys_addr_t fw_cfg_phys_base;
+static u32 fw_cfg_phys_size;
static void __iomem *fw_cfg_dev_base;
static void __iomem *fw_cfg_reg_ctrl;
static void __iomem *fw_cfg_reg_data;
@@ -92,7 +50,7 @@ static DEFINE_MUTEX(fw_cfg_dev_lock);
/* pick appropriate endianness for selector key */
static inline u16 fw_cfg_sel_endianness(u16 key)
{
- return fw_cfg_mode->is_mmio ? cpu_to_be16(key) : cpu_to_le16(key);
+ return fw_cfg_is_mmio ? cpu_to_be16(key) : cpu_to_le16(key);
}
/* type for fw_cfg "directory scan" visitor/callback function */
@@ -133,60 +91,100 @@ static inline void fw_cfg_read_blob(u16 key,
/* clean up fw_cfg device i/o */
static void fw_cfg_io_cleanup(void)
{
- if (fw_cfg_mode->is_mmio) {
+ if (fw_cfg_is_mmio) {
iounmap(fw_cfg_dev_base);
- release_mem_region(fw_cfg_mode->base, fw_cfg_mode->size);
+ release_mem_region(fw_cfg_phys_base, fw_cfg_phys_size);
} else {
ioport_unmap(fw_cfg_dev_base);
- release_region(fw_cfg_mode->base, fw_cfg_mode->size);
+ release_region(fw_cfg_phys_base, fw_cfg_phys_size);
}
}
-/* probe and map fw_cfg device */
-static int __init fw_cfg_io_probe(void)
+/* configure fw_cfg device i/o from ACPI _CRS method */
+static acpi_status fw_cfg_walk_crs(struct acpi_resource *r, void *context)
+{
+ struct acpi_resource_io *io;
+ struct acpi_resource_fixed_memory32 *mmio;
+
+ switch (r->type) {
+ case ACPI_RESOURCE_TYPE_END_TAG:
+ return AE_OK;
+ case ACPI_RESOURCE_TYPE_IO:
+ io = &r->data.io;
+ /* physical base addr should NOT be already set */
+ if (fw_cfg_phys_base)
+ return AE_ERROR;
+ if (!request_region(io->minimum,
+ io->address_length, "fw_cfg_io"))
+ return AE_ERROR;
+ fw_cfg_dev_base = ioport_map(io->minimum, io->address_length);
+ if (!fw_cfg_dev_base) {
+ release_region(io->minimum, io->address_length);
+ return AE_ERROR;
+ }
+ fw_cfg_phys_base = io->minimum;
+ fw_cfg_phys_size = io->address_length;
+ fw_cfg_is_mmio = false;
+ /* set register addresses (pc/i386 offsets) */
+ fw_cfg_reg_ctrl = fw_cfg_dev_base + 0x00;
+ fw_cfg_reg_data = fw_cfg_dev_base + 0x01;
+ return AE_OK;
+ case ACPI_RESOURCE_TYPE_FIXED_MEMORY32:
+ mmio = &r->data.fixed_memory32;
+ /* physical base addr should NOT be already set */
+ if (fw_cfg_phys_base)
+ return AE_ERROR;
+ /* MMIO and ACPI, but not on ARM ?!?! */
+ if (mmio->address_length < 0x0a)
+ return AE_ERROR;
+ if (!request_mem_region(mmio->address,
+ mmio->address_length, "fw_cfg_mem"))
+ return AE_ERROR;
+ fw_cfg_dev_base = ioremap(mmio->address, mmio->address_length);
+ if (!fw_cfg_dev_base) {
+ release_mem_region(mmio->address, mmio->address_length);
+ return AE_ERROR;
+ }
+ fw_cfg_phys_base = mmio->address;
+ fw_cfg_phys_size = mmio->address_length;
+ fw_cfg_is_mmio = true;
+ /* set register addresses (arm offsets) */
+ fw_cfg_reg_ctrl = fw_cfg_dev_base + 0x08;
+ fw_cfg_reg_data = fw_cfg_dev_base + 0x00;
+ return AE_OK;
+ default:
+ return AE_ERROR;
+ }
+}
+
+/* initialize fw_cfg device i/o from ACPI data */
+static int fw_cfg_acpi_init(struct acpi_device *dev)
{
char sig[FW_CFG_SIG_SIZE];
+ acpi_status status;
+ int err;
- for (fw_cfg_mode = &fw_cfg_modes[0];
- fw_cfg_mode->base; fw_cfg_mode++) {
-
- phys_addr_t base = fw_cfg_mode->base;
- u8 size = fw_cfg_mode->size;
-
- /* reserve and map mmio or ioport region */
- if (fw_cfg_mode->is_mmio) {
- if (!request_mem_region(base, size, fw_cfg_mode->name))
- continue;
- fw_cfg_dev_base = ioremap(base, size);
- if (!fw_cfg_dev_base) {
- release_mem_region(base, size);
- continue;
- }
- } else {
- if (!request_region(base, size, fw_cfg_mode->name))
- continue;
- fw_cfg_dev_base = ioport_map(base, size);
- if (!fw_cfg_dev_base) {
- release_region(base, size);
- continue;
- }
- }
+ err = acpi_bus_get_status(dev);
+ if (err < 0)
+ return err;
- /* set control and data register addresses */
- fw_cfg_reg_ctrl = fw_cfg_dev_base + fw_cfg_mode->ctrl_offset;
- fw_cfg_reg_data = fw_cfg_dev_base + fw_cfg_mode->data_offset;
+ if (!(dev->status.enabled && dev->status.functional))
+ return -ENODEV;
- /* verify fw_cfg device signature */
- fw_cfg_read_blob(FW_CFG_SIGNATURE, sig, 0, FW_CFG_SIG_SIZE);
- if (memcmp(sig, "QEMU", FW_CFG_SIG_SIZE) == 0)
- /* success, we're done */
- return 0;
+ /* extract device i/o details from _CRS */
+ status = acpi_walk_resources(dev->handle, METHOD_NAME__CRS,
+ fw_cfg_walk_crs, NULL);
+ if (status != AE_OK || !fw_cfg_phys_base)
+ return -ENODEV;
- /* clean up before probing next access mode */
+ /* verify fw_cfg device signature */
+ fw_cfg_read_blob(FW_CFG_SIGNATURE, sig, 0, FW_CFG_SIG_SIZE);
+ if (memcmp(sig, "QEMU", FW_CFG_SIG_SIZE) != 0) {
fw_cfg_io_cleanup();
+ return -ENODEV;
}
- return -ENODEV;
+ return 0;
}
/* fw_cfg revision attribute, in /sys/firmware/qemu_fw_cfg top-level dir. */
@@ -353,7 +351,7 @@ static struct kobject *fw_cfg_top_ko;
static struct kobject *fw_cfg_sel_ko;
/* callback function to register an individual fw_cfg file */
-static int __init fw_cfg_register_file(const struct fw_cfg_file *f)
+static int fw_cfg_register_file(const struct fw_cfg_file *f)
{
int err;
struct fw_cfg_sysfs_entry *entry;
@@ -397,12 +395,12 @@ static inline void fw_cfg_kobj_cleanup(struct kobject *kobj)
kobject_put(kobj);
}
-static int __init fw_cfg_sysfs_init(void)
+static int fw_cfg_sysfs_add(struct acpi_device *dev)
{
int err;
- /* probe for the fw_cfg "hardware" */
- err = fw_cfg_io_probe();
+ /* initialize fw_cfg device i/o from ACPI data */
+ err = fw_cfg_acpi_init(dev);
if (err)
return err;
@@ -443,14 +441,31 @@ err_top:
return err;
}
-static void __exit fw_cfg_sysfs_exit(void)
+static int fw_cfg_sysfs_remove(struct acpi_device *dev)
{
pr_debug("fw_cfg: unloading.\n");
fw_cfg_sysfs_cache_cleanup();
fw_cfg_kobj_cleanup(fw_cfg_sel_ko);
fw_cfg_kobj_cleanup(fw_cfg_top_ko);
fw_cfg_io_cleanup();
+ return 0;
}
-module_init(fw_cfg_sysfs_init);
-module_exit(fw_cfg_sysfs_exit);
+static const struct acpi_device_id fw_cfg_sysfs_device_ids[] = {
+ { "QEMU0002", 0 },
+ { "", 0 },
+};
+MODULE_DEVICE_TABLE(acpi, fw_cfg_sysfs_device_ids);
+
+static struct acpi_driver fw_cfg_sysfs_driver = {
+ .name = "fw_cfg",
+ .class = "QEMU",
+ .ids = fw_cfg_sysfs_device_ids,
+ .ops = {
+ .add = fw_cfg_sysfs_add,
+ .remove = fw_cfg_sysfs_remove,
+ },
+ .owner = THIS_MODULE,
+};
+
+module_acpi_driver(fw_cfg_sysfs_driver);
--
2.4.3
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v3 3/4] kobject: export kset_find_obj() for module use
2015-10-03 23:28 [PATCH v3 0/4] SysFS driver for QEMU fw_cfg device Gabriel L. Somlo
2015-10-03 23:28 ` [PATCH v3 1/4] firmware: introduce sysfs driver for QEMU's " Gabriel L. Somlo
2015-10-03 23:28 ` [PATCH v3 2/4] firmware: use acpi to detect QEMU fw_cfg device for sysfs fw_cfg driver Gabriel L. Somlo
@ 2015-10-03 23:28 ` Gabriel L. Somlo
2015-10-03 23:28 ` [PATCH v3 4/4] firmware: create directory hierarchy for sysfs fw_cfg entries Gabriel L. Somlo
[not found] ` <1443914889-9619-1-git-send-email-somlo-D+Gtc/HYRWM@public.gmane.org>
4 siblings, 0 replies; 26+ messages in thread
From: Gabriel L. Somlo @ 2015-10-03 23:28 UTC (permalink / raw)
To: gregkh, paul, galak, will.deacon, agross, mark.rutland, zajec5,
hanjun.guo, catalin.marinas, linux-api, linux-kernel
Cc: peter.maydell, matt.fleming, mst, jordan.l.justen, kernelnewbies,
qemu-devel, leif.lindholm, ard.biesheuvel, kraxel, pbonzini,
lersek
From: Gabriel Somlo <somlo@cmu.edu>
Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
---
lib/kobject.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/lib/kobject.c b/lib/kobject.c
index 3e3a5c3..bea2c9b 100644
--- a/lib/kobject.c
+++ b/lib/kobject.c
@@ -842,6 +842,7 @@ struct kobject *kset_find_obj(struct kset *kset, const char *name)
spin_unlock(&kset->list_lock);
return ret;
}
+EXPORT_SYMBOL(kset_find_obj);
static void kset_release(struct kobject *kobj)
{
--
2.4.3
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v3 4/4] firmware: create directory hierarchy for sysfs fw_cfg entries
2015-10-03 23:28 [PATCH v3 0/4] SysFS driver for QEMU fw_cfg device Gabriel L. Somlo
` (2 preceding siblings ...)
2015-10-03 23:28 ` [PATCH v3 3/4] kobject: export kset_find_obj() for module use Gabriel L. Somlo
@ 2015-10-03 23:28 ` Gabriel L. Somlo
[not found] ` <1443914889-9619-1-git-send-email-somlo-D+Gtc/HYRWM@public.gmane.org>
4 siblings, 0 replies; 26+ messages in thread
From: Gabriel L. Somlo @ 2015-10-03 23:28 UTC (permalink / raw)
To: gregkh, paul, galak, will.deacon, agross, mark.rutland, zajec5,
hanjun.guo, catalin.marinas, linux-api, linux-kernel
Cc: peter.maydell, matt.fleming, mst, jordan.l.justen, kernelnewbies,
qemu-devel, leif.lindholm, ard.biesheuvel, kraxel, pbonzini,
lersek
From: Gabriel Somlo <somlo@cmu.edu>
Each fw_cfg entry of type "file" has an associated 56-char,
nul-terminated ASCII string which represents its name. While
the fw_cfg device doesn't itself impose any specific naming
convention, QEMU developers have traditionally used path name
semantics (i.e. "etc/acpi/rsdp") to descriptively name the
various fw_cfg "blobs" passed into the guest.
This patch attempts, on a best effort basis, to create a
directory hierarchy representing the content of fw_cfg file
names, under /sys/firmware/qemu_fw_cfg/by_name.
Upon successful creation of all directories representing the
"dirname" portion of a fw_cfg file, a symlink will be created
to represent the "basename", pointing at the appropriate
/sys/firmware/qemu_fw_cfg/by_key entry. If a file name is not
suitable for this procedure (e.g., if its basename or dirname
components collide with an already existing dirname component
or basename, respectively) the corresponding fw_cfg blob is
skipped and will remain available in sysfs only by its selector
key value.
Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
---
.../ABI/testing/sysfs-firmware-qemu_fw_cfg | 42 +++++++++
drivers/firmware/qemu_fw_cfg.c | 104 +++++++++++++++++++++
2 files changed, 146 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-firmware-qemu_fw_cfg b/Documentation/ABI/testing/sysfs-firmware-qemu_fw_cfg
index e9761bf..2e3eaba 100644
--- a/Documentation/ABI/testing/sysfs-firmware-qemu_fw_cfg
+++ b/Documentation/ABI/testing/sysfs-firmware-qemu_fw_cfg
@@ -169,3 +169,45 @@ Description:
entry via the control register, and reading a number
of bytes equal to the blob size from the data
register.
+
+ --- Listing fw_cfg blobs by file name ---
+
+ While the fw_cfg device does not impose any specific naming
+ convention on the blobs registered in the file directory,
+ QEMU developers have traditionally used path name semantics
+ to give each blob a descriptive name. For example:
+
+ "bootorder"
+ "genroms/kvmvapic.bin"
+ "etc/e820"
+ "etc/boot-fail-wait"
+ "etc/system-states"
+ "etc/table-loader"
+ "etc/acpi/rsdp"
+ "etc/acpi/tables"
+ "etc/smbios/smbios-tables"
+ "etc/smbios/smbios-anchor"
+ ...
+
+ In addition to the listing by unique selector key described
+ above, the fw_cfg sysfs driver also attempts to build a tree
+ of directories matching the path name components of fw_cfg
+ blob names, ending in symlinks to the by_key entry for each
+ "basename", as illustrated below (assume current directory is
+ /sys/firmware):
+
+ qemu_fw_cfg/by_name/bootorder -> ../by_key/38
+ qemu_fw_cfg/by_name/etc/e820 -> ../../by_key/35
+ qemu_fw_cfg/by_name/etc/acpi/rsdp -> ../../../by_key/41
+ ...
+
+ Construction of the directory tree and symlinks is done on a
+ "best-effort" basis, as there is no guarantee that components
+ of fw_cfg blob names are always "well behaved". I.e., there is
+ the possibility that a symlink (basename) will conflict with
+ a dirname component of another fw_cfg blob, in which case the
+ creation of the offending /sys/firmware/qemu_fw_cfg/by_name
+ entry will be skipped.
+
+ The authoritative list of entries will continue to be found
+ under the /sys/firmware/qemu_fw_cfg/by_key directory.
diff --git a/drivers/firmware/qemu_fw_cfg.c b/drivers/firmware/qemu_fw_cfg.c
index f935afb..880f8ff 100644
--- a/drivers/firmware/qemu_fw_cfg.c
+++ b/drivers/firmware/qemu_fw_cfg.c
@@ -346,9 +346,104 @@ static struct bin_attribute fw_cfg_sysfs_attr_raw = {
.read = fw_cfg_sysfs_read_raw,
};
+/*
+ * Create a kset subdirectory matching each '/' delimited dirname token
+ * in 'name', starting with sysfs kset/folder 'dir'; At the end, create
+ * a symlink directed at the given 'target'.
+ * NOTE: We do this on a best-effort basis, since 'name' is not guaranteed
+ * to be a well-behaved path name. Whenever a symlink vs. kset directory
+ * name collision occurs, the kernel will issue big scary warnings while
+ * refusing to add the offending link or directory. We follow up with our
+ * own, slightly less scary error messages explaining the situation :)
+ */
+static int __init fw_cfg_build_symlink(struct kset *dir,
+ struct kobject *target,
+ const char *name)
+{
+ int ret;
+ struct kset *subdir;
+ struct kobject *ko;
+ char *name_copy, *p, *tok;
+
+ if (!dir || !target || !name || !*name)
+ return -EINVAL;
+
+ /* clone a copy of name for parsing */
+ name_copy = p = kstrdup(name, GFP_KERNEL);
+ if (!name_copy)
+ return -ENOMEM;
+
+ /* create folders for each dirname token, then symlink for basename */
+ while ((tok = strsep(&p, "/")) && *tok) {
+
+ /* last (basename) token? If so, add symlink here */
+ if (!p || !*p) {
+ ret = sysfs_create_link(&dir->kobj, target, tok);
+ break;
+ }
+
+ /* does the current dir contain an item named after tok ? */
+ ko = kset_find_obj(dir, tok);
+ if (ko) {
+ /* drop reference added by kset_find_obj */
+ kobject_put(ko);
+
+ /* ko MUST be a kset - we're about to use it as one ! */
+ if (ko->ktype != dir->kobj.ktype) {
+ ret = -EINVAL;
+ break;
+ }
+
+ /* descend into already existing subdirectory */
+ dir = to_kset(ko);
+ } else {
+ /* create new subdirectory kset */
+ subdir = kzalloc(sizeof(struct kset), GFP_KERNEL);
+ if (!subdir) {
+ ret = -ENOMEM;
+ break;
+ }
+ subdir->kobj.kset = dir;
+ subdir->kobj.ktype = dir->kobj.ktype;
+ ret = kobject_set_name(&subdir->kobj, "%s", tok);
+ if (ret) {
+ kfree(subdir);
+ break;
+ }
+ ret = kset_register(subdir);
+ if (ret) {
+ kfree(subdir);
+ break;
+ }
+
+ /* descend into newly created subdirectory */
+ dir = subdir;
+ }
+ }
+
+ /* we're done with cloned copy of name */
+ kfree(name_copy);
+ return ret;
+}
+
+/* recursively unregister fw_cfg/by_name/ kset directory tree */
+static void fw_cfg_kset_unregister_recursive(struct kset *kset)
+{
+ struct kobject *k, *next;
+
+ list_for_each_entry_safe(k, next, &kset->list, entry)
+ /* all set members are ksets too, but check just in case... */
+ if (k->ktype == kset->kobj.ktype)
+ fw_cfg_kset_unregister_recursive(to_kset(k));
+
+ /* symlinks are cleanly and automatically removed with the directory */
+ kset_unregister(kset);
+}
+
/* kobjects & kset representing top-level, by_key, and by_name folders */
static struct kobject *fw_cfg_top_ko;
static struct kobject *fw_cfg_sel_ko;
+static struct kset *fw_cfg_fname_kset;
/* callback function to register an individual fw_cfg file */
static int fw_cfg_register_file(const struct fw_cfg_file *f)
@@ -377,6 +472,9 @@ static int fw_cfg_register_file(const struct fw_cfg_file *f)
if (err)
goto err_add_raw;
+ /* try adding "/sys/firmware/qemu_fw_cfg/by_name/" symlink */
+ fw_cfg_build_symlink(fw_cfg_fname_kset, &entry->kobj, entry->f.name);
+
/* success, add entry to global cache */
fw_cfg_sysfs_cache_enlist(entry);
return 0;
@@ -412,6 +510,9 @@ static int fw_cfg_sysfs_add(struct acpi_device *dev)
fw_cfg_sel_ko = kobject_create_and_add("by_key", fw_cfg_top_ko);
if (!fw_cfg_sel_ko)
goto err_sel;
+ fw_cfg_fname_kset = kset_create_and_add("by_name", NULL, fw_cfg_top_ko);
+ if (!fw_cfg_fname_kset)
+ goto err_name;
/* get revision number, add matching top-level attribute */
fw_cfg_read_blob(FW_CFG_ID, &fw_cfg_rev, 0, sizeof(fw_cfg_rev));
@@ -433,6 +534,8 @@ err_scan:
fw_cfg_sysfs_cache_cleanup();
sysfs_remove_file(fw_cfg_top_ko, &fw_cfg_rev_attr.attr);
err_rev:
+ fw_cfg_kset_unregister_recursive(fw_cfg_fname_kset);
+err_name:
fw_cfg_kobj_cleanup(fw_cfg_sel_ko);
err_sel:
fw_cfg_kobj_cleanup(fw_cfg_top_ko);
@@ -445,6 +548,7 @@ static int fw_cfg_sysfs_remove(struct acpi_device *dev)
{
pr_debug("fw_cfg: unloading.\n");
fw_cfg_sysfs_cache_cleanup();
+ fw_cfg_kset_unregister_recursive(fw_cfg_fname_kset);
fw_cfg_kobj_cleanup(fw_cfg_sel_ko);
fw_cfg_kobj_cleanup(fw_cfg_top_ko);
fw_cfg_io_cleanup();
--
2.4.3
^ permalink raw reply related [flat|nested] 26+ messages in thread[parent not found: <1443914889-9619-1-git-send-email-somlo-D+Gtc/HYRWM@public.gmane.org>]
* Re: [PATCH v3 0/4] SysFS driver for QEMU fw_cfg device
[not found] ` <1443914889-9619-1-git-send-email-somlo-D+Gtc/HYRWM@public.gmane.org>
@ 2015-10-05 10:00 ` Mark Rutland
2015-10-05 11:48 ` Paolo Bonzini
2015-10-05 12:40 ` Gabriel L. Somlo
0 siblings, 2 replies; 26+ messages in thread
From: Mark Rutland @ 2015-10-05 10:00 UTC (permalink / raw)
To: Gabriel L. Somlo
Cc: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
paul-DWxLp4Yu+b8AvxtiuMwx3w, galak-sgV2jX0FEOL9JmXXK+q4OQ,
will.deacon-5wv7dgnIgG8, agross-sgV2jX0FEOL9JmXXK+q4OQ,
zajec5-Re5JQEeQqe8AvxtiuMwx3w, hanjun.guo-QSEj5FYQhm4dnm+yROfE0A,
catalin.marinas-5wv7dgnIgG8, linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
kernelnewbies-7JyXY6prKcjpASu1u0TL5ti2O/JbrIOy,
matt.fleming-ral2JQCrhuEAvxtiuMwx3w,
lersek-H+wXaHxf7aLQT0dZR+AlfA,
jordan.l.justen-ral2JQCrhuEAvxtiuMwx3w,
mst-H+wXaHxf7aLQT0dZR+AlfA, peter.maydell-QSEj5FYQhm4dnm+yROfE0A,
leif.lindholm-QSEj5FYQhm4dnm+yROfE0A,
ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A,
pbonzini-H+wXaHxf7aLQT0dZR+AlfA, kraxel-H+wXaHxf7aLQT0dZR+AlfA,
qemu-devel-qX2TKyscuCcdnm+yROfE0A
On Sat, Oct 03, 2015 at 07:28:05PM -0400, Gabriel L. Somlo wrote:
> From: "Gabriel Somlo" <somlo-D+Gtc/HYRWM@public.gmane.org>
>
> Allow access to QEMU firmware blobs, passed into the guest VM via
> the fw_cfg device, through SysFS entries. Blob meta-data (e.g. name,
> size, and fw_cfg key), as well as the raw binary blob data may be
> accessed.
>
> The SysFS access location is /sys/firmware/qemu_fw_cfg/... and was
> selected based on overall similarity to the type of information
> exposed under /sys/firmware/dmi/entries/...
What is the intended use of these?
Some of the keys in the example look like they'd come from other sources
(e.g. the *-tables entries), while others look like kernel/bootloader
configuration options (e.g. etc/boot-fail-wait, bootorder) -- I'm
concerned about redundancy here.
> NEW (since v2): Using ACPI to detect the presence and details of the
> fw_cfg virtual hardware device.
>
> Device Tree has been suggested by Ard as a comment on v2 of this
> patch, but after some deliberation I decided to go with ACPI,
> since it's supported on both x86 and some (uefi-enabled) versions
> of aarch64. I really don't see how I'd reasonably use *both* DT (on
> ARM) *and* ACPI (on x86), and after all I'm mostly concerned with
> x86, but originally wanted to maximize portability (which is where
> the register probing in earlier versions came from).
There are defintitely going to be arm64 VMs that don't use ACPI, so we
may need DT support depending on what the intended use is.
I'm not sure I follow what the difficulty with supporting DT in addition
to ACPI is? It looks like all you need is a compatible string and a reg
entry.
Thanks,
Mark.
> A patch set generating an ACPI device node for qemu's fw_cfg is
> currently under review on the qemu-devel list:
>
> http://lists.nongnu.org/archive/html/qemu-devel/2015-09/msg06946.html
> (sorry, gmane appears down at the moment...)
>
> In consequence:
>
> - Patch 1/4 is mostly the same as in v2;
> - Patch 2/4 switches device initialization from register
> probing to using ACPI; this is a separate patch only to
> illustrate the transition from probing to ACPI, and I'm
> assuming it will end up squashed on top of patch 1/4 in
> the final version.
>
> - Patches 3/4 and 4/4 add a "human-readable" directory
> hierarchy built from tokenizing fw_cfg blob names into
> '/'-separated components, with symlinks to each 'by_key'
> blob folder (same as in earlier versions). At Greg's
> suggestion I tried to build this folder hierarchy and
> leaf symlinks using udev rules, but so far I haven't been
> successful in figuring that out. If udev turns out to
> be applicable after all, these two patches can be dropped
> from this series.
>
> In other words, patches 1 and 2 give us the following "by_key" listing
> of blobs contained in the qemu fw_cfg device (example pulled from a PC
> qemu guest running Fedora 22), with the value of each "name" attribute
> shown on the right:
>
> $ tree /sys/firmware/qemu_fw_cfg/
> /sys/firmware/qemu_fw_cfg/
> |-- by_key
> | |-- 32
> | | |-- key
> | | |-- name ("etc/boot-fail-wait")
> | | |-- raw
> | | `-- size
> | |-- 33
> | | |-- key
> | | |-- name ("etc/smbios/smbios-tables")
> | | |-- raw
> | | `-- size
> | |-- 34
> | | |-- key
> | | |-- name ("etc/smbios/smbios-anchor")
> | | |-- raw
> | | `-- size
> | |-- 35
> | | |-- key
> | | |-- name ("etc/e820")
> | | |-- raw
> | | `-- size
> | |-- 36
> | | |-- key
> | | |-- name ("genroms/kvmvapic.bin")
> | | |-- raw
> | | `-- size
> | |-- 37
> | | |-- key
> | | |-- name ("etc/system-states")
> | | |-- raw
> | | `-- size
> | |-- 38
> | | |-- key
> | | |-- name ("etc/acpi/tables")
> | | |-- raw
> | | `-- size
> | |-- 39
> | | |-- key
> | | |-- name ("etc/table-loader")
> | | |-- raw
> | | `-- size
> | |-- 40
> | | |-- key
> | | |-- name ("etc/tpm/log")
> | | |-- raw
> | | `-- size
> | |-- 41
> | | |-- key
> | | |-- name ("etc/acpi/rsdp")
> | | |-- raw
> | | `-- size
> | `-- 42
> | |-- key
> | |-- name ("bootorder")
> | |-- raw
> | `-- size
> |
> ...
>
> Additionally, patches 3 and 4 (mostly 4) give us the following
> "user friendly" directory hierarchy as a complement to the above,
> based on tokenizing each blob name into symlink-tipped (sub)directories:
>
> ...
> |-- by_name
> | |-- bootorder -> ../by_key/42
> | |-- etc
> | | |-- acpi
> | | | |-- rsdp -> ../../../by_key/41
> | | | `-- tables -> ../../../by_key/38
> | | |-- boot-fail-wait -> ../../by_key/32
> | | |-- e820 -> ../../by_key/35
> | | |-- smbios
> | | | |-- smbios-anchor -> ../../../by_key/34
> | | | `-- smbios-tables -> ../../../by_key/33
> | | |-- system-states -> ../../by_key/37
> | | |-- table-loader -> ../../by_key/39
> | | `-- tpm
> | | `-- log -> ../../../by_key/40
> | `-- genroms
> | `-- kvmvapic.bin -> ../../by_key/36
> `-- rev
>
> The trick is to figure out how to replace patches 3 and 4 with a
> udev rule that would read the contents of each "name" attribute,
> and build the "by_name" hierarchy and symlinks in userspace.
>
> I tried:
>
> $ udevadm info -a -p /sys/firmware/qemu_fw_cfg/by_key/33
>
> looking at device '/firmware/qemu_fw_cfg/by_key/33':
> KERNEL=="33"
> SUBSYSTEM==""
> DRIVER==""
> ATTR{key}=="33"
> ATTR{name}=="etc/smbios/smbios-tables"
> ATTR{size}=="388"
>
> looking at parent device '/firmware/qemu_fw_cfg/by_key':
> KERNELS=="by_key"
> SUBSYSTEMS==""
> DRIVERS==""
>
> looking at parent device '/firmware/qemu_fw_cfg':
> KERNELS=="qemu_fw_cfg"
> SUBSYSTEMS==""
> DRIVERS==""
> ATTRS{rev}=="1"
>
> Then I tried creating a file, /usr/lib/udev/rules.d/99-qemu-fw-cfg.rules
> containing the following line:
>
> KERNELS=="qemu_fw_cfg", ATTRS{rev}=="1", SYMLINK="%p/%s{name}"
>
> but NOTHING happens when I insert/remove qemu_fw_cfg.ko. I also tried:
>
> KERNELS=="qemu_fw_cfg", ATTRS{rev}=="1", PROGRAM="/foo %k"
>
> where "/foo" basically did "echo $* > /tmp/bar", but no /tmp/bar file ever
> showed up as a consequence of inserting/removing the qemu_fw_cfg.ko module.
>
> At this point, I need help figuring out whether udev is really what would
> get the second, user-friendly, "by_name" /sysfs directory tree created,
> and how I'd go about that...
>
> Thanks much,
> --Gabriel
>
> Gabriel Somlo (4):
> firmware: introduce sysfs driver for QEMU's fw_cfg device
> firmware: use acpi to detect QEMU fw_cfg device for sysfs fw_cfg
> driver
> kobject: export kset_find_obj() for module use
> firmware: create directory hierarchy for sysfs fw_cfg entries
>
> .../ABI/testing/sysfs-firmware-qemu_fw_cfg | 213 ++++++++
> drivers/firmware/Kconfig | 10 +
> drivers/firmware/Makefile | 1 +
> drivers/firmware/qemu_fw_cfg.c | 575 +++++++++++++++++++++
> lib/kobject.c | 1 +
> 5 files changed, 800 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-firmware-qemu_fw_cfg
> create mode 100644 drivers/firmware/qemu_fw_cfg.c
>
> --
> 2.4.3
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v3 0/4] SysFS driver for QEMU fw_cfg device
2015-10-05 10:00 ` [PATCH v3 0/4] SysFS driver for QEMU fw_cfg device Mark Rutland
@ 2015-10-05 11:48 ` Paolo Bonzini
2015-10-05 12:23 ` Mark Rutland
2015-10-05 12:40 ` Gabriel L. Somlo
1 sibling, 1 reply; 26+ messages in thread
From: Paolo Bonzini @ 2015-10-05 11:48 UTC (permalink / raw)
To: Mark Rutland, Gabriel L. Somlo
Cc: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
paul-DWxLp4Yu+b8AvxtiuMwx3w, galak-sgV2jX0FEOL9JmXXK+q4OQ,
will.deacon-5wv7dgnIgG8, agross-sgV2jX0FEOL9JmXXK+q4OQ,
zajec5-Re5JQEeQqe8AvxtiuMwx3w, hanjun.guo-QSEj5FYQhm4dnm+yROfE0A,
catalin.marinas-5wv7dgnIgG8, linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
kernelnewbies-7JyXY6prKcjpASu1u0TL5ti2O/JbrIOy,
matt.fleming-ral2JQCrhuEAvxtiuMwx3w,
lersek-H+wXaHxf7aLQT0dZR+AlfA,
jordan.l.justen-ral2JQCrhuEAvxtiuMwx3w,
mst-H+wXaHxf7aLQT0dZR+AlfA, peter.maydell-QSEj5FYQhm4dnm+yROfE0A,
leif.lindholm-QSEj5FYQhm4dnm+yROfE0A,
ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A,
kraxel-H+wXaHxf7aLQT0dZR+AlfA, qemu-devel-qX2TKyscuCcdnm+yROfE0A
On 05/10/2015 12:00, Mark Rutland wrote:
> Some of the keys in the example look like they'd come from other sources
> (e.g. the *-tables entries), while others look like kernel/bootloader
> configuration options (e.g. etc/boot-fail-wait, bootorder) -- I'm
> concerned about redundancy here.
The redundancy is because the firmware and the bootloader actually
_consume_ these fw_cfg strings to produce the others (the ACPI tables,
the kernel configuration options).
On the other hand, hiding some strings just because they ought to have
been consumed already makes little sense.
Paolo
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 0/4] SysFS driver for QEMU fw_cfg device
2015-10-05 11:48 ` Paolo Bonzini
@ 2015-10-05 12:23 ` Mark Rutland
2015-10-05 12:43 ` Gabriel L. Somlo
0 siblings, 1 reply; 26+ messages in thread
From: Mark Rutland @ 2015-10-05 12:23 UTC (permalink / raw)
To: Paolo Bonzini
Cc: peter.maydell, paul, zajec5, catalin.marinas, jordan.l.justen,
kernelnewbies, gregkh, Gabriel L. Somlo, galak, will.deacon,
linux-kernel, leif.lindholm, qemu-devel, ard.biesheuvel,
matt.fleming, mst, hanjun.guo, agross, linux-api, lersek, kraxel
On Mon, Oct 05, 2015 at 01:48:52PM +0200, Paolo Bonzini wrote:
>
>
> On 05/10/2015 12:00, Mark Rutland wrote:
> > Some of the keys in the example look like they'd come from other sources
> > (e.g. the *-tables entries), while others look like kernel/bootloader
> > configuration options (e.g. etc/boot-fail-wait, bootorder) -- I'm
> > concerned about redundancy here.
>
> The redundancy is because the firmware and the bootloader actually
> _consume_ these fw_cfg strings to produce the others (the ACPI tables,
> the kernel configuration options).
>
> On the other hand, hiding some strings just because they ought to have
> been consumed already makes little sense.
Sure. However, I'm concerned that providing redundant interfaces for
those could lead to people grabbing information from here (because it's
convenient) rather than the existing canonical locations, which means we
get more software that works on fewer systems for no good reason.
What I couldn't figure out was what _additional_ information this
provided; it looked like a mixed bag of details we could already get
from disparate sources. If that's all it does, then it seems to me like
it doesn't add any benefit and potentially makes things worse.
So what do we get from this interface that we cannot get elsewhere, and
why is this the best way of exposing it?
Mark.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 0/4] SysFS driver for QEMU fw_cfg device
2015-10-05 12:23 ` Mark Rutland
@ 2015-10-05 12:43 ` Gabriel L. Somlo
[not found] ` <20151005124346.GG1977-h65ZQ0r4j6KKUezXOiBB2eW1CriLhL8O@public.gmane.org>
0 siblings, 1 reply; 26+ messages in thread
From: Gabriel L. Somlo @ 2015-10-05 12:43 UTC (permalink / raw)
To: Mark Rutland
Cc: Paolo Bonzini, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
paul-DWxLp4Yu+b8AvxtiuMwx3w, galak-sgV2jX0FEOL9JmXXK+q4OQ,
will.deacon-5wv7dgnIgG8, agross-sgV2jX0FEOL9JmXXK+q4OQ,
zajec5-Re5JQEeQqe8AvxtiuMwx3w, hanjun.guo-QSEj5FYQhm4dnm+yROfE0A,
catalin.marinas-5wv7dgnIgG8, linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
kernelnewbies-7JyXY6prKcjpASu1u0TL5ti2O/JbrIOy,
matt.fleming-ral2JQCrhuEAvxtiuMwx3w,
lersek-H+wXaHxf7aLQT0dZR+AlfA,
jordan.l.justen-ral2JQCrhuEAvxtiuMwx3w,
mst-H+wXaHxf7aLQT0dZR+AlfA, peter.maydell-QSEj5FYQhm4dnm+yROfE0A,
leif.lindholm-QSEj5FYQhm4dnm+yROfE0A,
ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A,
kraxel-H+wXaHxf7aLQT0dZR+AlfA, qemu-devel-qX2TKyscuCcdnm+yROfE0A
On Mon, Oct 05, 2015 at 01:23:33PM +0100, Mark Rutland wrote:
> On Mon, Oct 05, 2015 at 01:48:52PM +0200, Paolo Bonzini wrote:
> >
> >
> > On 05/10/2015 12:00, Mark Rutland wrote:
> > > Some of the keys in the example look like they'd come from other sources
> > > (e.g. the *-tables entries), while others look like kernel/bootloader
> > > configuration options (e.g. etc/boot-fail-wait, bootorder) -- I'm
> > > concerned about redundancy here.
> >
> > The redundancy is because the firmware and the bootloader actually
> > _consume_ these fw_cfg strings to produce the others (the ACPI tables,
> > the kernel configuration options).
> >
> > On the other hand, hiding some strings just because they ought to have
> > been consumed already makes little sense.
>
> Sure. However, I'm concerned that providing redundant interfaces for
> those could lead to people grabbing information from here (because it's
> convenient) rather than the existing canonical locations, which means we
> get more software that works on fewer systems for no good reason.
>
> What I couldn't figure out was what _additional_ information this
> provided; it looked like a mixed bag of details we could already get
> from disparate sources. If that's all it does, then it seems to me like
> it doesn't add any benefit and potentially makes things worse.
>
> So what do we get from this interface that we cannot get elsewhere, and
> why is this the best way of exposing it?
Starting with qemu 2.4, it is possible to insert arbitrary named
blobs into fw_cfg from the qemu command line. *Those* entries
might be interesting to userspace, which is why it might be handy
to access to fw_cfg blobs in general.
Thanks,
--Gabriel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 0/4] SysFS driver for QEMU fw_cfg device
2015-10-05 10:00 ` [PATCH v3 0/4] SysFS driver for QEMU fw_cfg device Mark Rutland
2015-10-05 11:48 ` Paolo Bonzini
@ 2015-10-05 12:40 ` Gabriel L. Somlo
[not found] ` <20151005124042.GF1977-h65ZQ0r4j6KKUezXOiBB2eW1CriLhL8O@public.gmane.org>
2015-10-05 13:05 ` Mark Rutland
1 sibling, 2 replies; 26+ messages in thread
From: Gabriel L. Somlo @ 2015-10-05 12:40 UTC (permalink / raw)
To: Mark Rutland
Cc: peter.maydell, paul, matt.fleming, catalin.marinas,
jordan.l.justen, kernelnewbies, gregkh, zajec5, will.deacon,
linux-kernel, leif.lindholm, qemu-devel, ard.biesheuvel, galak,
mst, hanjun.guo, agross, pbonzini, linux-api, lersek, kraxel
On Mon, Oct 05, 2015 at 11:00:36AM +0100, Mark Rutland wrote:
> On Sat, Oct 03, 2015 at 07:28:05PM -0400, Gabriel L. Somlo wrote:
> > From: "Gabriel Somlo" <somlo@cmu.edu>
> >
> > Allow access to QEMU firmware blobs, passed into the guest VM via
> > the fw_cfg device, through SysFS entries. Blob meta-data (e.g. name,
> > size, and fw_cfg key), as well as the raw binary blob data may be
> > accessed.
> >
> > The SysFS access location is /sys/firmware/qemu_fw_cfg/... and was
> > selected based on overall similarity to the type of information
> > exposed under /sys/firmware/dmi/entries/...
>
> What is the intended use of these?
>
> Some of the keys in the example look like they'd come from other sources
> (e.g. the *-tables entries), while others look like kernel/bootloader
> configuration options (e.g. etc/boot-fail-wait, bootorder) -- I'm
> concerned about redundancy here.
Paolo already answered that (more eloquently than I would have) so I'll
leave it at that, for now...
>
> > NEW (since v2): Using ACPI to detect the presence and details of the
> > fw_cfg virtual hardware device.
> >
> > Device Tree has been suggested by Ard as a comment on v2 of this
> > patch, but after some deliberation I decided to go with ACPI,
> > since it's supported on both x86 and some (uefi-enabled) versions
> > of aarch64. I really don't see how I'd reasonably use *both* DT (on
> > ARM) *and* ACPI (on x86), and after all I'm mostly concerned with
> > x86, but originally wanted to maximize portability (which is where
> > the register probing in earlier versions came from).
>
> There are defintitely going to be arm64 VMs that don't use ACPI, so we
> may need DT support depending on what the intended use is.
>
> I'm not sure I follow what the difficulty with supporting DT in addition
> to ACPI is? It looks like all you need is a compatible string and a reg
> entry.
Bearing in mind that I have almost no experience with arm:
I started out by probing all possible port-io and mmio locations where
fw_cfg registers might have been found, from a "classic" module_init
method.
Arm has DT, which as far as I understand will answer the following two
questions: 1. Do I have fw_cfg ? 2. If yes, what address range does it use ?
So that I could continue using a classic module_init, but won't need
to probe for the device.
PC (my primary architecture, the one I actually care about) does not
have DT. If I want to share the same code, I can't probe, so if I try
DT and don't find fw_cfg there (or somehow DT is no-op-ed out because
I'm on a PC guest), I could somehow look it up in ACPI the same way
(i.e., use ACPI as sort of a stand-in for DT).
But all ACPI-enabled drivers I could find use dedicated macros (i.e.
no more classic module_init() and module_exit(), but rather
module_acpi_driver() with .add and .remove methods on an acpi_driver
object, etc.) Not sure how I'd glue DT back into something like that.
In addition, Michael's comment earlier in the thread suggests that
even my current acpi version isn't sufficiently "orthodox" w.r.t.
ACPI, and I should be providing the hardware access routine as
an ACPI/AML routine, to avoid race conditions with the rest of ACPI,
and for encapsulation. I.e. it's even rude to use the fw_cfg node's
ACPI _CRS method (the part where I'd be treating it like a DT stand-in
only to query fw_cfg's hardware specifics).
So far, all the information I've been able to pull together points
away from a dual DT + ACPI all-in-one solution for fw_cfg. If you know
of an example where that's done in an acceptable way, please let
me know so I can use it for inspiration...
Thanks much,
--Gabriel
>
> > A patch set generating an ACPI device node for qemu's fw_cfg is
> > currently under review on the qemu-devel list:
> >
> > http://lists.nongnu.org/archive/html/qemu-devel/2015-09/msg06946.html
> > (sorry, gmane appears down at the moment...)
> >
> > In consequence:
> >
> > - Patch 1/4 is mostly the same as in v2;
> > - Patch 2/4 switches device initialization from register
> > probing to using ACPI; this is a separate patch only to
> > illustrate the transition from probing to ACPI, and I'm
> > assuming it will end up squashed on top of patch 1/4 in
> > the final version.
> >
> > - Patches 3/4 and 4/4 add a "human-readable" directory
> > hierarchy built from tokenizing fw_cfg blob names into
> > '/'-separated components, with symlinks to each 'by_key'
> > blob folder (same as in earlier versions). At Greg's
> > suggestion I tried to build this folder hierarchy and
> > leaf symlinks using udev rules, but so far I haven't been
> > successful in figuring that out. If udev turns out to
> > be applicable after all, these two patches can be dropped
> > from this series.
> >
> > In other words, patches 1 and 2 give us the following "by_key" listing
> > of blobs contained in the qemu fw_cfg device (example pulled from a PC
> > qemu guest running Fedora 22), with the value of each "name" attribute
> > shown on the right:
> >
> > $ tree /sys/firmware/qemu_fw_cfg/
> > /sys/firmware/qemu_fw_cfg/
> > |-- by_key
> > | |-- 32
> > | | |-- key
> > | | |-- name ("etc/boot-fail-wait")
> > | | |-- raw
> > | | `-- size
> > | |-- 33
> > | | |-- key
> > | | |-- name ("etc/smbios/smbios-tables")
> > | | |-- raw
> > | | `-- size
> > | |-- 34
> > | | |-- key
> > | | |-- name ("etc/smbios/smbios-anchor")
> > | | |-- raw
> > | | `-- size
> > | |-- 35
> > | | |-- key
> > | | |-- name ("etc/e820")
> > | | |-- raw
> > | | `-- size
> > | |-- 36
> > | | |-- key
> > | | |-- name ("genroms/kvmvapic.bin")
> > | | |-- raw
> > | | `-- size
> > | |-- 37
> > | | |-- key
> > | | |-- name ("etc/system-states")
> > | | |-- raw
> > | | `-- size
> > | |-- 38
> > | | |-- key
> > | | |-- name ("etc/acpi/tables")
> > | | |-- raw
> > | | `-- size
> > | |-- 39
> > | | |-- key
> > | | |-- name ("etc/table-loader")
> > | | |-- raw
> > | | `-- size
> > | |-- 40
> > | | |-- key
> > | | |-- name ("etc/tpm/log")
> > | | |-- raw
> > | | `-- size
> > | |-- 41
> > | | |-- key
> > | | |-- name ("etc/acpi/rsdp")
> > | | |-- raw
> > | | `-- size
> > | `-- 42
> > | |-- key
> > | |-- name ("bootorder")
> > | |-- raw
> > | `-- size
> > |
> > ...
> >
> > Additionally, patches 3 and 4 (mostly 4) give us the following
> > "user friendly" directory hierarchy as a complement to the above,
> > based on tokenizing each blob name into symlink-tipped (sub)directories:
> >
> > ...
> > |-- by_name
> > | |-- bootorder -> ../by_key/42
> > | |-- etc
> > | | |-- acpi
> > | | | |-- rsdp -> ../../../by_key/41
> > | | | `-- tables -> ../../../by_key/38
> > | | |-- boot-fail-wait -> ../../by_key/32
> > | | |-- e820 -> ../../by_key/35
> > | | |-- smbios
> > | | | |-- smbios-anchor -> ../../../by_key/34
> > | | | `-- smbios-tables -> ../../../by_key/33
> > | | |-- system-states -> ../../by_key/37
> > | | |-- table-loader -> ../../by_key/39
> > | | `-- tpm
> > | | `-- log -> ../../../by_key/40
> > | `-- genroms
> > | `-- kvmvapic.bin -> ../../by_key/36
> > `-- rev
> >
> > The trick is to figure out how to replace patches 3 and 4 with a
> > udev rule that would read the contents of each "name" attribute,
> > and build the "by_name" hierarchy and symlinks in userspace.
> >
> > I tried:
> >
> > $ udevadm info -a -p /sys/firmware/qemu_fw_cfg/by_key/33
> >
> > looking at device '/firmware/qemu_fw_cfg/by_key/33':
> > KERNEL=="33"
> > SUBSYSTEM==""
> > DRIVER==""
> > ATTR{key}=="33"
> > ATTR{name}=="etc/smbios/smbios-tables"
> > ATTR{size}=="388"
> >
> > looking at parent device '/firmware/qemu_fw_cfg/by_key':
> > KERNELS=="by_key"
> > SUBSYSTEMS==""
> > DRIVERS==""
> >
> > looking at parent device '/firmware/qemu_fw_cfg':
> > KERNELS=="qemu_fw_cfg"
> > SUBSYSTEMS==""
> > DRIVERS==""
> > ATTRS{rev}=="1"
> >
> > Then I tried creating a file, /usr/lib/udev/rules.d/99-qemu-fw-cfg.rules
> > containing the following line:
> >
> > KERNELS=="qemu_fw_cfg", ATTRS{rev}=="1", SYMLINK="%p/%s{name}"
> >
> > but NOTHING happens when I insert/remove qemu_fw_cfg.ko. I also tried:
> >
> > KERNELS=="qemu_fw_cfg", ATTRS{rev}=="1", PROGRAM="/foo %k"
> >
> > where "/foo" basically did "echo $* > /tmp/bar", but no /tmp/bar file ever
> > showed up as a consequence of inserting/removing the qemu_fw_cfg.ko module.
> >
> > At this point, I need help figuring out whether udev is really what would
> > get the second, user-friendly, "by_name" /sysfs directory tree created,
> > and how I'd go about that...
> >
> > Thanks much,
> > --Gabriel
> >
> > Gabriel Somlo (4):
> > firmware: introduce sysfs driver for QEMU's fw_cfg device
> > firmware: use acpi to detect QEMU fw_cfg device for sysfs fw_cfg
> > driver
> > kobject: export kset_find_obj() for module use
> > firmware: create directory hierarchy for sysfs fw_cfg entries
> >
> > .../ABI/testing/sysfs-firmware-qemu_fw_cfg | 213 ++++++++
> > drivers/firmware/Kconfig | 10 +
> > drivers/firmware/Makefile | 1 +
> > drivers/firmware/qemu_fw_cfg.c | 575 +++++++++++++++++++++
> > lib/kobject.c | 1 +
> > 5 files changed, 800 insertions(+)
> > create mode 100644 Documentation/ABI/testing/sysfs-firmware-qemu_fw_cfg
> > create mode 100644 drivers/firmware/qemu_fw_cfg.c
> >
> > --
> > 2.4.3
> >
^ permalink raw reply [flat|nested] 26+ messages in thread[parent not found: <20151005124042.GF1977-h65ZQ0r4j6KKUezXOiBB2eW1CriLhL8O@public.gmane.org>]
* Re: [PATCH v3 0/4] SysFS driver for QEMU fw_cfg device
[not found] ` <20151005124042.GF1977-h65ZQ0r4j6KKUezXOiBB2eW1CriLhL8O@public.gmane.org>
@ 2015-10-05 12:50 ` Peter Maydell
[not found] ` <CAFEAcA9qVSsmPW_s3wRSBykGYWU-2PS2ZNCagnoe0FGCg8mqzQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
0 siblings, 1 reply; 26+ messages in thread
From: Peter Maydell @ 2015-10-05 12:50 UTC (permalink / raw)
To: Gabriel L. Somlo
Cc: Mark Rutland, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
paul-DWxLp4Yu+b8AvxtiuMwx3w, Kumar Gala, Will Deacon,
agross-sgV2jX0FEOL9JmXXK+q4OQ, zajec5-Re5JQEeQqe8AvxtiuMwx3w,
Hanjun Guo, Catalin Marinas,
linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
lkml - Kernel Mailing List,
kernelnewbies-7JyXY6prKcjpASu1u0TL5ti2O/JbrIOy, Matt Fleming,
Laszlo Ersek, Jordan Justen, Michael S. Tsirkin, Leif Lindholm,
Ard Biesheuvel, Paolo Bonzini, Gerd Hoffmann, QEMU Developers
On 5 October 2015 at 13:40, Gabriel L. Somlo <somlo-D+Gtc/HYRWM@public.gmane.org> wrote:
> In addition, Michael's comment earlier in the thread suggests that
> even my current acpi version isn't sufficiently "orthodox" w.r.t.
> ACPI, and I should be providing the hardware access routine as
> an ACPI/AML routine, to avoid race conditions with the rest of ACPI,
> and for encapsulation. I.e. it's even rude to use the fw_cfg node's
> ACPI _CRS method (the part where I'd be treating it like a DT stand-in
> only to query fw_cfg's hardware specifics).
If you want to try to support "firmware might also be reading
fw_cfg at the same time as the kernel" this is a (painful)
problem regardless of how the kernel figures out whether a
fw_cfg device is present. I had assumed that one of the design
assumptions of this series was that firmware would only
read the fw_cfg before booting the guest kernel and never touch
it afterwards. If it might touch it later then letting the
guest kernel also mess with fw_cfg seems like a really bad idea.
thanks
-- PMM
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 0/4] SysFS driver for QEMU fw_cfg device
2015-10-05 12:40 ` Gabriel L. Somlo
[not found] ` <20151005124042.GF1977-h65ZQ0r4j6KKUezXOiBB2eW1CriLhL8O@public.gmane.org>
@ 2015-10-05 13:05 ` Mark Rutland
2015-10-06 7:18 ` Laszlo Ersek
1 sibling, 1 reply; 26+ messages in thread
From: Mark Rutland @ 2015-10-05 13:05 UTC (permalink / raw)
To: Gabriel L. Somlo
Cc: gregkh, paul, galak, will.deacon, agross, zajec5, hanjun.guo,
catalin.marinas, linux-api, linux-kernel, kernelnewbies,
matt.fleming, lersek, jordan.l.justen, mst, peter.maydell,
leif.lindholm, ard.biesheuvel, pbonzini, kraxel, qemu-devel
> > I'm not sure I follow what the difficulty with supporting DT in addition
> > to ACPI is? It looks like all you need is a compatible string and a reg
> > entry.
>
> Bearing in mind that I have almost no experience with arm:
>
> I started out by probing all possible port-io and mmio locations where
> fw_cfg registers might have been found, from a "classic" module_init
> method.
>
> Arm has DT, which as far as I understand will answer the following two
> questions: 1. Do I have fw_cfg ? 2. If yes, what address range does it use ?
> So that I could continue using a classic module_init, but won't need
> to probe for the device.
>
> PC (my primary architecture, the one I actually care about) does not
> have DT. If I want to share the same code, I can't probe, so if I try
> DT and don't find fw_cfg there (or somehow DT is no-op-ed out because
> I'm on a PC guest), I could somehow look it up in ACPI the same way
> (i.e., use ACPI as sort of a stand-in for DT).
I'd imagine that it's simple to have something in your probe path like:
if (pdev->dev.of_node)
parse_dt(pdev);
else
parse_acpi(pdev);
> But all ACPI-enabled drivers I could find use dedicated macros (i.e.
> no more classic module_init() and module_exit(), but rather
> module_acpi_driver() with .add and .remove methods on an acpi_driver
> object, etc.) Not sure how I'd glue DT back into something like that.
You don't have to use those macros, and can simply use the classic
module_{init,exit} functions, calling the requisite acpi driver
registration functions at module {init,exit} time.
> In addition, Michael's comment earlier in the thread suggests that
> even my current acpi version isn't sufficiently "orthodox" w.r.t.
> ACPI, and I should be providing the hardware access routine as
> an ACPI/AML routine, to avoid race conditions with the rest of ACPI,
> and for encapsulation. I.e. it's even rude to use the fw_cfg node's
> ACPI _CRS method (the part where I'd be treating it like a DT stand-in
> only to query fw_cfg's hardware specifics).
As Peter stated, this sounds very much like it rules out sharing the
interface with FW generally (and is certainly scary).
> So far, all the information I've been able to pull together points
> away from a dual DT + ACPI all-in-one solution for fw_cfg. If you know
> of an example where that's done in an acceptable way, please let
> me know so I can use it for inspiration...
I'm not immediately aware, but I would imagine you could search for
files that had both an of_match_table and a acpi_bus_register_driver
call.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v3 0/4] SysFS driver for QEMU fw_cfg device
2015-10-05 13:05 ` Mark Rutland
@ 2015-10-06 7:18 ` Laszlo Ersek
0 siblings, 0 replies; 26+ messages in thread
From: Laszlo Ersek @ 2015-10-06 7:18 UTC (permalink / raw)
To: Mark Rutland, Gabriel L. Somlo
Cc: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
paul-DWxLp4Yu+b8AvxtiuMwx3w, galak-sgV2jX0FEOL9JmXXK+q4OQ,
will.deacon-5wv7dgnIgG8, agross-sgV2jX0FEOL9JmXXK+q4OQ,
zajec5-Re5JQEeQqe8AvxtiuMwx3w, hanjun.guo-QSEj5FYQhm4dnm+yROfE0A,
catalin.marinas-5wv7dgnIgG8, linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
kernelnewbies-7JyXY6prKcjpASu1u0TL5ti2O/JbrIOy,
matt.fleming-ral2JQCrhuEAvxtiuMwx3w,
jordan.l.justen-ral2JQCrhuEAvxtiuMwx3w,
mst-H+wXaHxf7aLQT0dZR+AlfA, peter.maydell-QSEj5FYQhm4dnm+yROfE0A,
leif.lindholm-QSEj5FYQhm4dnm+yROfE0A,
ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A,
pbonzini-H+wXaHxf7aLQT0dZR+AlfA, kraxel-H+wXaHxf7aLQT0dZR+AlfA,
qemu-devel-qX2TKyscuCcdnm+yROfE0A
On 10/05/15 15:05, Mark Rutland wrote:
>>> I'm not sure I follow what the difficulty with supporting DT in addition
>>> to ACPI is? It looks like all you need is a compatible string and a reg
>>> entry.
>>
>> Bearing in mind that I have almost no experience with arm:
>>
>> I started out by probing all possible port-io and mmio locations where
>> fw_cfg registers might have been found, from a "classic" module_init
>> method.
>>
>> Arm has DT, which as far as I understand will answer the following two
>> questions: 1. Do I have fw_cfg ? 2. If yes, what address range does it use ?
>> So that I could continue using a classic module_init, but won't need
>> to probe for the device.
>>
>> PC (my primary architecture, the one I actually care about) does not
>> have DT. If I want to share the same code, I can't probe, so if I try
>> DT and don't find fw_cfg there (or somehow DT is no-op-ed out because
>> I'm on a PC guest), I could somehow look it up in ACPI the same way
>> (i.e., use ACPI as sort of a stand-in for DT).
>
> I'd imagine that it's simple to have something in your probe path like:
>
> if (pdev->dev.of_node)
> parse_dt(pdev);
> else
> parse_acpi(pdev);
>
>> But all ACPI-enabled drivers I could find use dedicated macros (i.e.
>> no more classic module_init() and module_exit(), but rather
>> module_acpi_driver() with .add and .remove methods on an acpi_driver
>> object, etc.) Not sure how I'd glue DT back into something like that.
>
> You don't have to use those macros, and can simply use the classic
> module_{init,exit} functions, calling the requisite acpi driver
> registration functions at module {init,exit} time.
>
>> In addition, Michael's comment earlier in the thread suggests that
>> even my current acpi version isn't sufficiently "orthodox" w.r.t.
>> ACPI, and I should be providing the hardware access routine as
>> an ACPI/AML routine, to avoid race conditions with the rest of ACPI,
>> and for encapsulation. I.e. it's even rude to use the fw_cfg node's
>> ACPI _CRS method (the part where I'd be treating it like a DT stand-in
>> only to query fw_cfg's hardware specifics).
>
> As Peter stated, this sounds very much like it rules out sharing the
> interface with FW generally (and is certainly scary).
>
>> So far, all the information I've been able to pull together points
>> away from a dual DT + ACPI all-in-one solution for fw_cfg. If you know
>> of an example where that's done in an acceptable way, please let
>> me know so I can use it for inspiration...
>
> I'm not immediately aware, but I would imagine you could search for
> files that had both an of_match_table and a acpi_bus_register_driver
> call.
One file that I think is an example for this (and I have looked at
before) is: "drivers/virtio/virtio_mmio.c".
Virtio-mmio is supposed to be enumerable in both ACPI and DT virtual
machines. For the QEMU side, grep QEMU for "LNRO0005" vs. "virtio,mmio".
Thanks
Laszlo
^ permalink raw reply [flat|nested] 26+ messages in thread