On Thu, Dec 20, 2018 at 03:21:24PM +0000, Julien Grall wrote:
> At the moment the user is only able to give the amount of memory used by
> the guest. The placement of the RAM in the layout is left to the
> software. It would be useful to give more freedom to the user where the
> RAM regions will live in the memory layout and the size of them.
>
> The command line to specify the size of the memory (-m/-mem) is now extended
> in a compatible way to allow specifiying the base address: <size>[@<addr>].
>
> The <addr> is mandatory if the option is specified multiple (i.e the
> user request).
>
> As not all the architecture will support multiple RAM region or even
> specificying the address, it is left to the architecture to enable the
> feature (see MAX_RAM_BANKS and ARCH_SUPPORT_CFG_RAM_BASE).
>
> At the moment no-one is taking advantages of it, this will change in a
> follow-up patch.
>
> Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> ---
> arm/fdt.c | 2 +-
> arm/kvm.c | 33 +++++++++---------
> builtin-run.c | 87 +++++++++++++++++++++++++++++++++++++++++++-----
> include/kvm/kvm-config.h | 16 ++++++++-
> include/kvm/kvm.h | 21 ++++++++++--
> kvm.c | 4 +++
> mips/kvm.c | 40 ++++++++++++----------
> powerpc/kvm.c | 26 ++++++++-------
> x86/bios.c | 8 +++--
> x86/kvm.c | 47 +++++++++++++++-----------
> 10 files changed, 204 insertions(+), 80 deletions(-)
>
> diff --git a/arm/fdt.c b/arm/fdt.c
> index 980015b..6ac0b33 100644
> --- a/arm/fdt.c
> +++ b/arm/fdt.c
> @@ -116,7 +116,7 @@ static int setup_fdt(struct kvm *kvm)
> u8 staging_fdt[FDT_MAX_SIZE];
> u64 mem_reg_prop[] = {
> cpu_to_fdt64(kvm->arch.memory_guest_start),
> - cpu_to_fdt64(kvm->ram_size),
> + cpu_to_fdt64(kvm->ram[0].size),
> };
> struct psci_fns *fns;
> void *fdt = staging_fdt;
> diff --git a/arm/kvm.c b/arm/kvm.c
> index 9623aa5..2a55b41 100644
> --- a/arm/kvm.c
> +++ b/arm/kvm.c
> @@ -27,11 +27,11 @@ bool kvm__arch_cpu_supports_vm(void)
> static void kvm__init_ram(struct kvm *kvm)
> {
> int err;
> - u64 phys_start, phys_size;
> - void *host_mem;
> + u64 phys_start;
> unsigned long alignment;
> /* Convenience aliases */
> const char *hugetlbfs_path = kvm->cfg.hugetlbfs_path;
> + struct kvm_ram_region *ram = &kvm->ram[0];
>
> /*
> * Allocate guest memory. We must align our buffer to 64K to
> @@ -45,8 +45,8 @@ static void kvm__init_ram(struct kvm *kvm)
> alignment = SZ_32M;
> else
> alignment = SZ_2M;
> - kvm->ram_size = kvm->cfg.ram_size;
> - kvm->arch.ram_alloc_size = kvm->ram_size + alignment;
> + ram->size = kvm->cfg.ram[0].size;
> + kvm->arch.ram_alloc_size = ram->size + alignment;
> kvm->arch.ram_alloc_start = mmap_anon_or_hugetlbfs(kvm, hugetlbfs_path,
> kvm->arch.ram_alloc_size);
>
> @@ -54,8 +54,8 @@ static void kvm__init_ram(struct kvm *kvm)
> die("Failed to map %lld bytes for guest memory (%d)",
> kvm->arch.ram_alloc_size, errno);
>
> - kvm->ram_start = (void *)ALIGN((unsigned long)kvm->arch.ram_alloc_start,
> - SZ_2M);
> + ram->start = (void *)ALIGN((unsigned long)kvm->arch.ram_alloc_start,
> + SZ_2M);
>
> madvise(kvm->arch.ram_alloc_start, kvm->arch.ram_alloc_size,
> MADV_MERGEABLE);
> @@ -64,13 +64,11 @@ static void kvm__init_ram(struct kvm *kvm)
> MADV_HUGEPAGE);
>
> phys_start = ARM_MEMORY_AREA;
> - phys_size = kvm->ram_size;
> - host_mem = kvm->ram_start;
>
> - err = kvm__register_ram(kvm, phys_start, phys_size, host_mem);
> + err = kvm__register_ram(kvm, phys_start, ram->size, ram->start);
> if (err)
> die("Failed to register %lld bytes of memory at physical "
> - "address 0x%llx [err %d]", phys_size, phys_start, err);
> + "address 0x%llx [err %d]", ram->size, phys_start, err);
>
> kvm->arch.memory_guest_start = phys_start;
> }
> @@ -92,10 +90,13 @@ void kvm__arch_set_cmdline(char *cmdline, bool video)
>
> static void kvm__arch_sanitize_cfg(struct kvm_config *cfg)
> {
> - if (cfg->ram_size > ARM_MAX_MEMORY(cfg)) {
> - cfg->ram_size = ARM_MAX_MEMORY(cfg);
> + /* Convenience aliases */
> + struct kvm_ram_config *bank0 = &cfg->ram[0];
> +
> + if (bank0->size > ARM_MAX_MEMORY(cfg)) {
> + bank0->size = ARM_MAX_MEMORY(cfg);
> pr_warning("sanitize: Capping memory to %lluMB",
> - cfg->ram_size >> 20);
> + bank0->size >> 20);
> }
> }
>
> @@ -118,14 +119,16 @@ bool kvm__arch_load_kernel_image(struct kvm *kvm, int fd_kernel, int fd_initrd,
> void *pos, *kernel_end, *limit;
> unsigned long guest_addr;
> ssize_t file_size;
> + /* Convenience aliases */
> + struct kvm_ram_region *ram0 = &kvm->ram[0];
>
> /*
> * Linux requires the initrd and dtb to be mapped inside lowmem,
> * so we can't just place them at the top of memory.
> */
> - limit = kvm->ram_start + min(kvm->ram_size, (u64)SZ_256M) - 1;
> + limit = ram0->start + min(ram0->size, (u64)SZ_256M) - 1;
>
> - pos = kvm->ram_start + ARM_KERN_OFFSET(kvm);
> + pos = ram0->start + ARM_KERN_OFFSET(kvm);
> kvm->arch.kern_guest_start = host_to_guest_flat(kvm, pos);
> file_size = read_file(fd_kernel, pos, limit - pos);
> if (file_size < 0) {
> diff --git a/builtin-run.c b/builtin-run.c
> index 443c10b..568af5c 100644
> --- a/builtin-run.c
> +++ b/builtin-run.c
> @@ -87,6 +87,63 @@ void kvm_run_set_wrapper_sandbox(void)
> kvm_run_wrapper = KVM_RUN_SANDBOX;
> }
>
> +static int mem_parser(const struct option *opt, const char *arg, int unset)
> +{
> + struct kvm_config *cfg = opt->value;
> + const char *p = arg;
> + char *next;
> + u64 size, addr;
> + int base;
> +
> + /* parse out size */
> + size = strtoll(p, &next, 10);
> +
> + if (next == p)
> + die("mem: no size specified.\n");
> +
> + if (*next != '@' && *next != '\0')
> + die("mem: unexpected chars after size.\n");
> +
> + if (*next == '\0')
> + p = next;
> + else
> + p = next + 1;
> +
> + /* parse out guest addr */
> + base = 10;
> + if (strcasestr(p, "0x"))
> + base = 16;
> + addr = strtoll(p, &next, base);
I thought strtoll() already handled '0x' prefixes if you pass base = 0?
> + if (next == p && addr == 0) {
> + /*
> + * To keep backward compatibility, the <addr> is not
> + * mandatory for the first bank.
> + */
> + if (cfg->nr_ram > 0)
> + die("mem: <addr> must be specified\n");
> + addr = INVALID_RAM_ADDR;
> + }
> +
> + if ( cfg->nr_ram == MAX_RAM_BANKS )
> + die("mem: Too many banks\n");
> +
> + /*
> + * Allow the architecture to tell whether it is possible to configure
> + * the RAM base address.
> + */
> +#ifndef ARCH_SUPPORT_CFG_RAM_BASE
> + if (addr == INVALID_RAM_ADDR)
> + die("mem: specifying <addr> is not allowed\n");
> +#endif
> +
> + cfg->ram[cfg->nr_ram].base = addr;
> + cfg->ram[cfg->nr_ram].size = size;
How do you avoid adding overlapping regions?
Will
