On Tue, 4 Dec 2018 11:14:33 +0000
Julien Thierry <julien.thierry@xxxxxxx> wrote:
Hi,
> Add an option to let user load files as non-volatile memory.
>
> An additional range of addresses is reserved for nv memory.
> Loaded files must be a multiple of the system page size.
>
> Users can chose whether the data written by the guest modifies the
> original file.
>
> Signed-off-by: Julien Thierry <julien.thierry@xxxxxxx>
> ---
> arm/fdt.c | 43 ++++++++++
> arm/include/arm-common/kvm-arch.h | 5 +-
> arm/include/arm-common/kvm-config-arch.h | 18 ++++-
> arm/kvm.c | 134
> +++++++++++++++++++++++++++++++ 4 files changed, 198 insertions(+), 2
> deletions(-)
>
> diff --git a/arm/fdt.c b/arm/fdt.c
> index 2936986..fd482ce 100644
> --- a/arm/fdt.c
> +++ b/arm/fdt.c
> @@ -72,6 +72,46 @@ static void generate_irq_prop(void *fdt, u8 irq,
> enum irq_type irq_type) _FDT(fdt_property(fdt, "interrupts",
> irq_prop, sizeof(irq_prop))); }
>
> +static void generate_nvmem_node(void *fdt, struct kvm_nv_mem *nvmem)
> +{
> + char *buf;
> + int len;
> + const u64 reg_prop[] = {
> + cpu_to_fdt64(nvmem->map_addr),
> + cpu_to_fdt64(nvmem->size)
> + };
> +
> + /* Name length + '@' + 8 hex characters + '\0' */
> + len = strlen(nvmem->name) + 10;
> + buf = malloc(len);
> + snprintf(buf, len, "%s@%llx", nvmem->name,
> + nvmem->map_addr);
> + _FDT(fdt_begin_node(fdt, buf));
> + free(buf);
> +
> + /* Name length + "kvmtool," + '\0' */
> + len = strlen(nvmem->name) + 9;
> + buf = malloc(len);
> + snprintf(buf, len, "kvmtool,%s", nvmem->name);
> + _FDT(fdt_property_string(fdt, "compatible", buf));
As discussed in person, it doesn't sound right to (ab)use the
compatible name for this. This one should describe a device type, not
an instance of it.
So I would suggest to use a fixed compatible name (say:
"kvmtool,flash"), then put the designator in a property.
flash@7f000000 {
compatible = "kvmtool,flash";
label = "environment";
reg = <0 0x7f000000 0 0x10000>;
};
So the label property would reflect the user provided name.
Also there is the generic "read-only" property, which we might want to
use in case "wb" is not provided.
I am not overly happy with inventing a new compatible name for such a
generic device, but couldn't find anything better (mtd-ram, mtd-rom,
cfi-flash were close, but not a complete fit). Also given that the idea
is to just communicate this from kvmtool to EDK II (without Linux ever
picking this up), I guess this solution works.
Cheers,
Andre,
> + free(buf);
> +
> + _FDT(fdt_property(fdt, "reg", reg_prop, sizeof(reg_prop)));
> +
> + _FDT(fdt_end_node(fdt));
> +}
> +
> +static void generate_nvmem_nodes(void *fdt, struct kvm *kvm)
> +{
> + struct list_head *nvmem_node;
> +
> + list_for_each(nvmem_node, &kvm->cfg.arch.nvmem_list)
> + generate_nvmem_node(fdt,
> + container_of(nvmem_node,
> + struct kvm_nv_mem,
> + node));
> +}
> +
> struct psci_fns {
> u32 cpu_suspend;
> u32 cpu_off;
> @@ -210,6 +250,9 @@ static int setup_fdt(struct kvm *kvm)
> _FDT(fdt_property_cell(fdt, "migrate", fns->migrate));
> _FDT(fdt_end_node(fdt));
>
> + /* Non volatile memories */
> + generate_nvmem_nodes(fdt, kvm);
> +
> /* Finalise. */
> _FDT(fdt_end_node(fdt));
> _FDT(fdt_finish(fdt));
> diff --git a/arm/include/arm-common/kvm-arch.h
> b/arm/include/arm-common/kvm-arch.h index b9d486d..f425d86 100644
> --- a/arm/include/arm-common/kvm-arch.h
> +++ b/arm/include/arm-common/kvm-arch.h
> @@ -10,6 +10,7 @@
> #define ARM_IOPORT_AREA _AC(0x0000000000000000, UL)
> #define ARM_MMIO_AREA _AC(0x0000000000010000, UL)
> #define ARM_AXI_AREA _AC(0x0000000040000000, UL)
> +#define ARM_NVRAM_AREA _AC(0x000000007f000000, UL)
> #define ARM_MEMORY_AREA _AC(0x0000000080000000, UL)
>
> #define ARM_LOMAP_MAX_MEMORY ((1ULL << 32) - ARM_MEMORY_AREA)
> @@ -24,9 +25,11 @@
> #define ARM_IOPORT_SIZE (ARM_MMIO_AREA -
> ARM_IOPORT_AREA) #define ARM_VIRTIO_MMIO_SIZE (ARM_AXI_AREA -
> (ARM_MMIO_AREA + ARM_GIC_SIZE)) #define ARM_PCI_CFG_SIZE (1ULL
> << 24) -#define ARM_PCI_MMIO_SIZE (ARM_MEMORY_AREA - \
> +#define ARM_PCI_MMIO_SIZE (ARM_NVRAM_AREA - \
> (ARM_AXI_AREA + ARM_PCI_CFG_SIZE))
>
> +#define ARM_NVRAM_SIZE (ARM_MEMORY_AREA -
> ARM_NVRAM_AREA) +
> #define KVM_IOPORT_AREA ARM_IOPORT_AREA
> #define KVM_PCI_CFG_AREA ARM_AXI_AREA
> #define KVM_PCI_MMIO_AREA (KVM_PCI_CFG_AREA +
> ARM_PCI_CFG_SIZE) diff --git
> a/arm/include/arm-common/kvm-config-arch.h
> b/arm/include/arm-common/kvm-config-arch.h index 5734c46..d5742cb
> 100644 --- a/arm/include/arm-common/kvm-config-arch.h +++
> b/arm/include/arm-common/kvm-config-arch.h @@ -1,8 +1,18 @@
> #ifndef ARM_COMMON__KVM_CONFIG_ARCH_H
> #define ARM_COMMON__KVM_CONFIG_ARCH_H
>
> +#include <linux/list.h>
> #include "kvm/parse-options.h"
>
> +struct kvm_nv_mem {
> + char *data_file;
> + char *name;
> + ssize_t size;
> + u64 map_addr;
> + bool write_back;
> + struct list_head node;
> +};
> +
> struct kvm_config_arch {
> const char *dump_dtb_filename;
> unsigned int force_cntfrq;
> @@ -12,9 +22,11 @@ struct kvm_config_arch {
> u64 kaslr_seed;
> enum irqchip_type irqchip;
> u64 fw_addr;
> + struct list_head nvmem_list;
> };
>
> int irqchip_parser(const struct option *opt, const char *arg, int
> unset); +int nvmem_parser(const struct option *opt, const char *arg,
> int unset);
> #define OPT_ARCH_RUN(pfx,
> cfg) \
> pfx,
> \ @@ -33,6 +45,10 @@ int irqchip_parser(const struct option *opt,
> const char *arg, int unset); "Type of interrupt controller to emulate
> in the guest", \ irqchip_parser,
> NULL), \ OPT_U64('\0',
> "firmware-address", &(cfg)->fw_addr, \
> - "Address where firmware should be loaded"),
> + "Address where firmware should be
> loaded"), \
> + OPT_CALLBACK('\0', "nvmem",
> NULL, \
> +
> "<file>,<name>[,wb]", \
> + "Load <file> as non-volatile memory
> kvmtool,<name>", \
> + nvmem_parser, kvm),
>
> #endif /* ARM_COMMON__KVM_CONFIG_ARCH_H */
> diff --git a/arm/kvm.c b/arm/kvm.c
> index 1a2cfdc..00675d9 100644
> --- a/arm/kvm.c
> +++ b/arm/kvm.c
> @@ -18,6 +18,53 @@ struct kvm_ext kvm_req_ext[] = {
> { 0, 0 },
> };
>
> +int nvmem_parser(const struct option *opt, const char *arg, int
> unset) +{
> + struct kvm *kvm = (struct kvm*) opt->ptr;
> + struct kvm_nv_mem *nvmem;
> + struct stat st;
> + const char *ptr;
> + uint32_t len;
> +
> + nvmem = calloc(sizeof (*nvmem), 1);
> +
> + if (!nvmem)
> + die("nvmem: cannot add non-volatile memory");
> +
> + ptr = strstr(arg, ",");
> +
> + if (!ptr)
> + die("nvmem: missing name for non-volatile memory");
> +
> + len = ptr - arg + 1;
> + nvmem->data_file = malloc(len);
> + strncpy(nvmem->data_file, arg, len);
> + nvmem->data_file[len - 1] = '\0';
> +
> + if (stat(nvmem->data_file, &st))
> + die("nvmem: failed to stat data file");
> + nvmem->size = st.st_size;
> +
> + arg = arg + len;
> + for (ptr = arg; *ptr != '\0' && *ptr != ','; ++ptr)
> + ;
> + len = ptr - arg + 1;
> + nvmem->name = malloc(len);
> + strncpy(nvmem->name, arg, len);
> + nvmem->name[len - 1] = '\0';
> +
> + if (*ptr == ',') {
> + if (!strcmp(ptr + 1, "wb"))
> + nvmem->write_back = true;
> + else
> + die("firmware-data: invalid option %s", ptr
> + 1);
> + }
> +
> + list_add(&nvmem->node, &kvm->cfg.arch.nvmem_list);
> +
> + return 0;
> +}
> +
> bool kvm__arch_cpu_supports_vm(void)
> {
> /* The KVM capability check is enough. */
> @@ -59,6 +106,7 @@ void kvm__arch_set_cmdline(char *cmdline, bool
> video)
> void kvm__arch_reset(struct kvm *kvm)
> {
> + INIT_LIST_HEAD(&kvm->cfg.arch.nvmem_list);
> }
>
> void kvm__arch_init(struct kvm *kvm, const char *hugetlbfs_path, u64
> ram_size) @@ -238,3 +286,89 @@ int kvm__arch_setup_firmware(struct
> kvm *kvm) {
> return 0;
> }
> +
> +static int setup_nvmem(struct kvm *kvm)
> +{
> + u64 map_address = ARM_NVRAM_AREA;
> + const u64 limit = ARM_NVRAM_AREA + ARM_NVRAM_SIZE;
> + struct list_head *nvmem_node;
> + const int pagesize = getpagesize();
> +
> + list_for_each(nvmem_node, &kvm->cfg.arch.nvmem_list) {
> + struct kvm_nv_mem *nvmem = container_of(nvmem_node,
> + struct
> kvm_nv_mem,
> + node);
> + void *user_addr;
> + int fd;
> +
> + if (map_address + nvmem->size > limit)
> + die("cannot map file %s in non-volatile
> memory, no space left",
> + nvmem->data_file);
> +
> + if (nvmem->size & (pagesize - 1))
> + die("size of non-volatile memory files must
> be a multiple of system page size (= %d)",
> + pagesize);
> +
> + user_addr = mmap(NULL, nvmem->size, PROT_RW,
> MAP_ANON_NORESERVE, -1, 0);
> + if (user_addr == MAP_FAILED)
> + die("cannot create mapping for file %s",
> + nvmem->data_file);
> +
> + fd = open(nvmem->data_file, O_RDONLY);
> + if (fd < 0)
> + die("cannot read file %s", nvmem->data_file);
> + if (read_file(fd, user_addr, nvmem->size) < 0)
> + die("failed to map nv memory data %s",
> + nvmem->data_file);
> + close(fd);
> +
> + if (kvm__register_dev_mem(kvm, map_address,
> nvmem->size,
> + user_addr))
> + die("failed to register nv memory mapping
> for guest"); +
> + nvmem->map_addr = map_address;
> + map_address += nvmem->size;
> + }
> +
> + return 0;
> +}
> +firmware_init(setup_nvmem);
> +
> +static int flush_nv_mem(struct kvm *kvm)
> +{
> + struct list_head *nvmem_node;
> + int err = 0;
> +
> + list_for_each(nvmem_node, &kvm->cfg.arch.nvmem_list) {
> + struct kvm_nv_mem *nvmem = container_of(nvmem_node,
> + struct
> kvm_nv_mem,
> + node);
> + void *host_pos;
> +
> + host_pos = guest_flat_to_host(kvm,
> + nvmem->map_addr);
> +
> + if (nvmem->write_back) {
> + int fd;
> +
> + fd = open(nvmem->data_file, O_WRONLY);
> + if (fd < 0) {
> + pr_err("failed to open firmware data
> file for writting");
> + err = -1;
> + continue;
> + }
> +
> + if (write_in_full(fd, host_pos, nvmem->size)
> < 0) {
> + pr_err("failed to flush firmware
> data to file");
> + err = -1;
> + }
> + close(fd);
> + }
> +
> + if (munmap(host_pos, nvmem->size))
> + err = -1;
> + }
> +
> + return err;
> +}
> +firmware_exit(flush_nv_mem);
