From e545c54c39eba583df20ad368257dbafb2f27d17 Mon Sep 17 00:00:00 2001 From: dv Date: Tue, 3 May 2022 21:39:18 +0000 Subject: [PATCH] vmm/vmd/vmctl: standardize memory units to bytes At different points in the vm lifecycle vmm(4), vmctl(8), and vmd(8) refer to a vm's memory range sizes in either bytes or megabytes. This is needlessly complex. Switch to using bytes everywhere and adjust types and constants accordingly. While this makes it possible to specify vm's with memory in fractions of megabytes, the logic requiring whole megabyte values remains. Feedback from deraadt@, mlarkin@, and Matthew Martin. ok mlarkin@ --- sys/arch/amd64/amd64/vmm.c | 4 ++-- sys/arch/amd64/include/vmmvar.h | 4 ++-- usr.sbin/vmctl/main.c | 31 +++++++++++++++++++++++-------- usr.sbin/vmctl/vmctl.c | 6 +++--- usr.sbin/vmctl/vmctl.h | 6 +++--- usr.sbin/vmd/parse.y | 30 +++++++++++++++++++++++------- usr.sbin/vmd/vm.c | 12 +++++------- usr.sbin/vmd/vmd.h | 4 ++-- 8 files changed, 63 insertions(+), 34 deletions(-) diff --git a/sys/arch/amd64/amd64/vmm.c b/sys/arch/amd64/amd64/vmm.c index 765fc19bca5..d218407f5bb 100644 --- a/sys/arch/amd64/amd64/vmm.c +++ b/sys/arch/amd64/amd64/vmm.c @@ -1,4 +1,4 @@ -/* $OpenBSD: vmm.c,v 1.306 2022/04/27 14:23:37 dv Exp $ */ +/* $OpenBSD: vmm.c,v 1.307 2022/05/03 21:39:18 dv Exp $ */ /* * Copyright (c) 2014 Mike Larkin * @@ -1575,7 +1575,7 @@ vm_create_check_mem_ranges(struct vm_create_params *vcp) { size_t i, memsize = 0; struct vm_mem_range *vmr, *pvmr; - const paddr_t maxgpa = (uint64_t)VMM_MAX_VM_MEM_SIZE * 1024 * 1024; + const paddr_t maxgpa = VMM_MAX_VM_MEM_SIZE; if (vcp->vcp_nmemranges == 0 || vcp->vcp_nmemranges > VMM_MAX_MEM_RANGES) diff --git a/sys/arch/amd64/include/vmmvar.h b/sys/arch/amd64/include/vmmvar.h index 94bb172832d..f798674dd0f 100644 --- a/sys/arch/amd64/include/vmmvar.h +++ b/sys/arch/amd64/include/vmmvar.h @@ -1,4 +1,4 @@ -/* $OpenBSD: vmmvar.h,v 1.74 2021/09/13 22:16:27 dv Exp $ */ +/* $OpenBSD: vmmvar.h,v 1.75 2022/05/03 21:39:19 dv Exp $ */ /* * Copyright (c) 2014 Mike Larkin * @@ -31,7 +31,7 @@ #define VMM_MAX_KERNEL_PATH 128 #define VMM_MAX_VCPUS 512 #define VMM_MAX_VCPUS_PER_VM 64 -#define VMM_MAX_VM_MEM_SIZE 32768 +#define VMM_MAX_VM_MEM_SIZE 32L * 1024 * 1024 * 1024 /* 32 GiB */ #define VMM_MAX_NICS_PER_VM 4 #define VMM_PCI_MMIO_BAR_BASE 0xF0000000ULL diff --git a/usr.sbin/vmctl/main.c b/usr.sbin/vmctl/main.c index 0f7e4329a00..37fc4dc642a 100644 --- a/usr.sbin/vmctl/main.c +++ b/usr.sbin/vmctl/main.c @@ -1,4 +1,4 @@ -/* $OpenBSD: main.c,v 1.68 2021/07/12 15:09:22 beck Exp $ */ +/* $OpenBSD: main.c,v 1.69 2022/05/03 21:39:18 dv Exp $ */ /* * Copyright (c) 2015 Reyk Floeter @@ -404,23 +404,38 @@ parse_network(struct parse_result *res, char *word) int parse_size(struct parse_result *res, char *word) { - long long val = 0; + char result[FMT_SCALED_STRSIZE]; + long long val = 0; if (word != NULL) { if (scan_scaled(word, &val) != 0) { - warn("invalid size: %s", word); + warn("invalid memory size: %s", word); return (-1); } } if (val < (1024 * 1024)) { - warnx("size must be at least one megabyte"); + warnx("memory size must be at least 1MB"); return (-1); - } else - res->size = val / 1024 / 1024; + } - if ((res->size * 1024 * 1024) != val) - warnx("size rounded to %lld megabytes", res->size); + if (val > VMM_MAX_VM_MEM_SIZE) { + if (fmt_scaled(VMM_MAX_VM_MEM_SIZE, result) == 0) + warnx("memory size too large (limit is %s)", result); + else + warnx("memory size too large"); + return (-1); + } + + /* Round down to the megabyte. */ + res->size = (val / (1024 * 1024)) * (1024 * 1024); + + if (res->size != (size_t)val) { + if (fmt_scaled(res->size, result) == 0) + warnx("memory size rounded to %s", result); + else + warnx("memory size rounded to %zu bytes", res->size); + } return (0); } diff --git a/usr.sbin/vmctl/vmctl.c b/usr.sbin/vmctl/vmctl.c index 4c0b62fc6e1..b1977f95051 100644 --- a/usr.sbin/vmctl/vmctl.c +++ b/usr.sbin/vmctl/vmctl.c @@ -1,4 +1,4 @@ -/* $OpenBSD: vmctl.c,v 1.79 2021/06/10 19:50:05 dv Exp $ */ +/* $OpenBSD: vmctl.c,v 1.80 2022/05/03 21:39:18 dv Exp $ */ /* * Copyright (c) 2014 Mike Larkin @@ -73,7 +73,7 @@ struct imsgbuf *ibuf; * ENOMEM if a memory allocation failure occurred. */ int -vm_start(uint32_t start_id, const char *name, int memsize, int nnics, +vm_start(uint32_t start_id, const char *name, size_t memsize, int nnics, char **nics, int ndisks, char **disks, int *disktypes, char *kernel, char *iso, char *instance, unsigned int bootdevice) { @@ -122,7 +122,7 @@ vm_start(uint32_t start_id, const char *name, int memsize, int nnics, /* * XXX: vmd(8) fills in the actual memory ranges. vmctl(8) - * just passes in the actual memory size in MB here. + * just passes in the actual memory size here. */ vcp->vcp_nmemranges = 1; vcp->vcp_memranges[0].vmr_size = memsize; diff --git a/usr.sbin/vmctl/vmctl.h b/usr.sbin/vmctl/vmctl.h index 4fd2b787deb..6d84c7b1c6c 100644 --- a/usr.sbin/vmctl/vmctl.h +++ b/usr.sbin/vmctl/vmctl.h @@ -1,4 +1,4 @@ -/* $OpenBSD: vmctl.h,v 1.34 2021/01/27 07:21:12 deraadt Exp $ */ +/* $OpenBSD: vmctl.h,v 1.35 2022/05/03 21:39:18 dv Exp $ */ /* * Copyright (c) 2015 Reyk Floeter @@ -48,7 +48,7 @@ struct parse_result { char *name; char *path; char *isopath; - long long size; + size_t size; int nifs; char **nets; int nnets; @@ -93,7 +93,7 @@ int open_imagefile(int, const char *, int, int create_imagefile(int, const char *, const char *, long, const char **); int create_raw_imagefile(const char *, long); int create_qc2_imagefile(const char *, const char *, long); -int vm_start(uint32_t, const char *, int, int, char **, int, +int vm_start(uint32_t, const char *, size_t, int, char **, int, char **, int *, char *, char *, char *, unsigned int); int vm_start_complete(struct imsg *, int *, int); void terminate_vm(uint32_t, const char *, unsigned int); diff --git a/usr.sbin/vmd/parse.y b/usr.sbin/vmd/parse.y index ebebbf24750..90d74e28401 100644 --- a/usr.sbin/vmd/parse.y +++ b/usr.sbin/vmd/parse.y @@ -1,4 +1,4 @@ -/* $OpenBSD: parse.y,v 1.59 2021/10/15 15:01:29 naddy Exp $ */ +/* $OpenBSD: parse.y,v 1.60 2022/05/03 21:39:18 dv Exp $ */ /* * Copyright (c) 2007-2016 Reyk Floeter @@ -1248,25 +1248,41 @@ symget(const char *nam) ssize_t parse_size(char *word, int64_t val) { + char result[FMT_SCALED_STRSIZE]; ssize_t size; long long res; if (word != NULL) { if (scan_scaled(word, &res) != 0) { - log_warn("invalid size: %s", word); + log_warn("invalid memory size: %s", word); return (-1); } val = (int64_t)res; } if (val < (1024 * 1024)) { - log_warnx("size must be at least one megabyte"); + log_warnx("memory size must be at least 1MB"); return (-1); - } else - size = val / 1024 / 1024; + } - if ((size * 1024 * 1024) != val) - log_warnx("size rounded to %zd megabytes", size); + if (val > VMM_MAX_VM_MEM_SIZE) { + if (fmt_scaled(VMM_MAX_VM_MEM_SIZE, result) == 0) + log_warnx("memory size too large (limit is %s)", + result); + else + log_warnx("memory size too large"); + return (-1); + } + + /* Round down to the megabyte. */ + size = (val / (1024 * 1024)) * (1024 * 1024); + + if (size != val) { + if (fmt_scaled(size, result) == 0) + log_warnx("memory size rounded to %s", result); + else + log_warnx("memory size rounded to %zd bytes", size); + } return ((ssize_t)size); } diff --git a/usr.sbin/vmd/vm.c b/usr.sbin/vmd/vm.c index 55d938ed1d1..d952ba4d8d0 100644 --- a/usr.sbin/vmd/vm.c +++ b/usr.sbin/vmd/vm.c @@ -1,4 +1,4 @@ -/* $OpenBSD: vm.c,v 1.68 2022/03/01 21:46:19 dv Exp $ */ +/* $OpenBSD: vm.c,v 1.69 2022/05/03 21:39:18 dv Exp $ */ /* * Copyright (c) 2015 Mike Larkin @@ -233,7 +233,7 @@ loadfile_bios(gzFile fp, off_t size, struct vcpu_reg_state *vrs) if (gzseek(fp, 0, SEEK_SET) == -1) return (-1); - /* The BIOS image must end at 1M */ + /* The BIOS image must end at 1MB */ if ((off = 1048576 - size) < 0) return (-1); @@ -871,15 +871,13 @@ vcpu_reset(uint32_t vmid, uint32_t vcpu_id, struct vcpu_reg_state *vrs) void create_memory_map(struct vm_create_params *vcp) { - size_t len, mem_bytes, mem_mb; + size_t len, mem_bytes; - mem_mb = vcp->vcp_memranges[0].vmr_size; + mem_bytes = vcp->vcp_memranges[0].vmr_size; vcp->vcp_nmemranges = 0; - if (mem_mb < 1 || mem_mb > VMM_MAX_VM_MEM_SIZE) + if (mem_bytes == 0 || mem_bytes > VMM_MAX_VM_MEM_SIZE) return; - mem_bytes = mem_mb * 1024 * 1024; - /* First memory region: 0 - LOWMEM_KB (DOS low mem) */ len = LOWMEM_KB * 1024; vcp->vcp_memranges[0].vmr_gpa = 0x0; diff --git a/usr.sbin/vmd/vmd.h b/usr.sbin/vmd/vmd.h index 5f33b648317..31bc642afdb 100644 --- a/usr.sbin/vmd/vmd.h +++ b/usr.sbin/vmd/vmd.h @@ -1,4 +1,4 @@ -/* $OpenBSD: vmd.h,v 1.108 2022/01/04 15:22:53 claudio Exp $ */ +/* $OpenBSD: vmd.h,v 1.109 2022/05/03 21:39:18 dv Exp $ */ /* * Copyright (c) 2015 Mike Larkin @@ -56,7 +56,7 @@ #define MAX_TAP 256 #define NR_BACKLOG 5 #define VMD_SWITCH_TYPE "bridge" -#define VM_DEFAULT_MEMORY 512 +#define VM_DEFAULT_MEMORY 512 * 1024 * 1024 /* 512 MiB */ #define VMD_DEFAULT_STAGGERED_START_DELAY 30 -- 2.20.1