Re: [PATCH v15 02/11] fw_cfg: add a public uapi header

From: Michael S. Tsirkin
Date: Mon Feb 26 2018 - 19:06:34 EST


On Thu, Feb 15, 2018 at 10:33:03PM +0100, Marc-André Lureau wrote:
> Create a common header file for well-known values and structures to be
> shared by the Linux kernel with qemu or other projects.
>
> It is based from qemu/docs/specs/fw_cfg.txt which references
> qemu/include/hw/nvram/fw_cfg_keys.h "for the most up-to-date and
> authoritative list" & vmcoreinfo.txt. Those files don't have an
> explicit license, but qemu/hw/nvram/fw_cfg.c is BSD-license, so
> Michael S. Tsirkin suggested to use the same license.
>
> The patch intentionally left out DMA & vmcoreinfo structures &
> defines, which are added in the commits making usage of it.
>
> Suggested-by: Michael S. Tsirkin <mst@xxxxxxxxxx>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>
>
> ---
>
> The related qemu patch making use of it, to be submitted:
> https://github.com/elmarco/qemu/commit/4884fc9e9c4c4467a371e5a40f3181239e1b70f5
> ---
> MAINTAINERS | 1 +
> drivers/firmware/qemu_fw_cfg.c | 22 ++------------
> include/uapi/linux/fw_cfg.h | 66 ++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 69 insertions(+), 20 deletions(-)
> create mode 100644 include/uapi/linux/fw_cfg.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3bdc260e36b7..a66b65f62811 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11352,6 +11352,7 @@ M: "Michael S. Tsirkin" <mst@xxxxxxxxxx>
> L: qemu-devel@xxxxxxxxxx
> S: Maintained
> F: drivers/firmware/qemu_fw_cfg.c
> +F: include/uapi/linux/fw_cfg.h
>
> QIB DRIVER
> M: Dennis Dalessandro <dennis.dalessandro@xxxxxxxxx>

Why fw_cfg.h and not qemu_fw_cfg.h ? fw_cfg.h seems too generic.

> diff --git a/drivers/firmware/qemu_fw_cfg.c b/drivers/firmware/qemu_fw_cfg.c
> index a41b572eeeb1..42601a3eaed5 100644
> --- a/drivers/firmware/qemu_fw_cfg.c
> +++ b/drivers/firmware/qemu_fw_cfg.c
> @@ -32,30 +32,12 @@
> #include <linux/slab.h>
> #include <linux/io.h>
> #include <linux/ioport.h>
> +#include <uapi/linux/fw_cfg.h>
>
> MODULE_AUTHOR("Gabriel L. Somlo <somlo@xxxxxxx>");
> 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 register addresses */
> static bool fw_cfg_is_mmio;
> static phys_addr_t fw_cfg_p_base;
> @@ -597,7 +579,7 @@ MODULE_DEVICE_TABLE(of, fw_cfg_sysfs_mmio_match);
>
> #ifdef CONFIG_ACPI
> static const struct acpi_device_id fw_cfg_sysfs_acpi_match[] = {
> - { "QEMU0002", },
> + { FW_CFG_ACPI_DEVICE_ID, },
> {},
> };
> MODULE_DEVICE_TABLE(acpi, fw_cfg_sysfs_acpi_match);
> diff --git a/include/uapi/linux/fw_cfg.h b/include/uapi/linux/fw_cfg.h
> new file mode 100644
> index 000000000000..c698ac3812f6
> --- /dev/null
> +++ b/include/uapi/linux/fw_cfg.h
> @@ -0,0 +1,66 @@
> +/* SPDX-License-Identifier: BSD-3-Clause */
> +#ifndef _LINUX_FW_CFG_H
> +#define _LINUX_FW_CFG_H
> +
> +#include <linux/types.h>
> +
> +#define FW_CFG_ACPI_DEVICE_ID "QEMU0002"
> +
> +/* selector key values for "well-known" fw_cfg entries */
> +#define FW_CFG_SIGNATURE 0x00
> +#define FW_CFG_ID 0x01
> +#define FW_CFG_UUID 0x02
> +#define FW_CFG_RAM_SIZE 0x03
> +#define FW_CFG_NOGRAPHIC 0x04
> +#define FW_CFG_NB_CPUS 0x05
> +#define FW_CFG_MACHINE_ID 0x06
> +#define FW_CFG_KERNEL_ADDR 0x07
> +#define FW_CFG_KERNEL_SIZE 0x08
> +#define FW_CFG_KERNEL_CMDLINE 0x09
> +#define FW_CFG_INITRD_ADDR 0x0a
> +#define FW_CFG_INITRD_SIZE 0x0b
> +#define FW_CFG_BOOT_DEVICE 0x0c
> +#define FW_CFG_NUMA 0x0d
> +#define FW_CFG_BOOT_MENU 0x0e
> +#define FW_CFG_MAX_CPUS 0x0f
> +#define FW_CFG_KERNEL_ENTRY 0x10
> +#define FW_CFG_KERNEL_DATA 0x11
> +#define FW_CFG_INITRD_DATA 0x12
> +#define FW_CFG_CMDLINE_ADDR 0x13
> +#define FW_CFG_CMDLINE_SIZE 0x14
> +#define FW_CFG_CMDLINE_DATA 0x15
> +#define FW_CFG_SETUP_ADDR 0x16
> +#define FW_CFG_SETUP_SIZE 0x17
> +#define FW_CFG_SETUP_DATA 0x18
> +#define FW_CFG_FILE_DIR 0x19
> +
> +#define FW_CFG_FILE_FIRST 0x20
> +#define FW_CFG_FILE_SLOTS_MIN 0x10
> +
> +#define FW_CFG_WRITE_CHANNEL 0x4000
> +#define FW_CFG_ARCH_LOCAL 0x8000
> +#define FW_CFG_ENTRY_MASK (~(FW_CFG_WRITE_CHANNEL | FW_CFG_ARCH_LOCAL))
> +
> +#define FW_CFG_INVALID 0xffff
> +
> +/* width in bytes of fw_cfg control register */
> +#define FW_CFG_CTL_SIZE 0x02
> +
> +/* fw_cfg "file name" is up to 56 characters (including terminating nul) */
> +#define FW_CFG_MAX_FILE_PATH 56
> +
> +/* size in bytes of fw_cfg signature */
> +#define FW_CFG_SIG_SIZE 4
> +
> +/* FW_CFG_ID bits */
> +#define FW_CFG_VERSION 0x01
> +
> +/* fw_cfg file directory entry type */
> +struct fw_cfg_file {
> + __be32 size;
> + __be16 select;
> + __u16 reserved;
> + char name[FW_CFG_MAX_FILE_PATH];
> +};
> +
> +#endif
> --
> 2.16.1.73.g5832b7e9f2