From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id B3FDFC43334 for ; Thu, 16 Jun 2022 17:09:02 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=yckWVeOLtrVLnf34FueAlTdtp2HM5llLKP5FmhKI/QM=; b=iJUl251yFKnq3Y g5M2CGCUsIfarBM74mG7/KDtu0eflj5KaApvXSa+gOJci4f8zRNoVPH+EK+5Kw/T6srVqeTdpoF2K BMMhiVIA4lzYn6UEm6kGM0vwe0Ley8fA0NVQHO7dEsqHcYFIz37WDejRVY4+ZyzZkw7kjChKZs2hB NQyXftt0cyqlwfKh1+w1tXWm5afpthKnYBgcf61I5o2wjebJ4GL/CP0of0cBCn74LufdREUuxoeWZ T/edl71Zh5p+GlY/hlRokgRyKVlrEjmOw8nVaQVVbwWlDghAd+NXsrgTmoyrpJd+CqOZdvLdgDSuV NZ+Mis220fjV4euhviPA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1o1syV-003bFJ-UZ; Thu, 16 Jun 2022 17:08:08 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1o1syR-003bDj-OX for linux-arm-kernel@lists.infradead.org; Thu, 16 Jun 2022 17:08:05 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id F23D411FB; Thu, 16 Jun 2022 10:08:00 -0700 (PDT) Received: from donnerap.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id A30863F73B; Thu, 16 Jun 2022 10:07:59 -0700 (PDT) Date: Thu, 16 Jun 2022 18:07:56 +0100 From: Andre Przywara To: Alexandru Elisei Cc: will@kernel.org, julien.thierry.kdev@gmail.com, maz@kernel.org, suzuki.poulose@arm.com, julien@xen.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, james.morse@arm.com Subject: Re: [PATCH v4 kvmtool 02/12] builtin-run: Always use RAM size in bytes Message-ID: <20220616180756.6402b88d@donnerap.cambridge.arm.com> In-Reply-To: <20220616134828.129006-3-alexandru.elisei@arm.com> References: <20220616134828.129006-1-alexandru.elisei@arm.com> <20220616134828.129006-3-alexandru.elisei@arm.com> Organization: ARM X-Mailer: Claws Mail 3.18.0 (GTK+ 2.24.32; aarch64-unknown-linux-gnu) MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220616_100803_917937_625160BD X-CRM114-Status: GOOD ( 25.16 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Thu, 16 Jun 2022 14:48:18 +0100 Alexandru Elisei wrote: > The user can specify the virtual machine memory size in MB, which is saved > in cfg->ram_size. kvmtool validates it against the host memory size, > converted from bytes to MB. ram_size is then converted to bytes, and this > is how it is used throughout the rest of kvmtool. > > To avoid any confusion about the unit of measurement, especially once the > user is allowed to specify the unit of measurement, always use ram_size in > bytes. > > Signed-off-by: Alexandru Elisei That looks good to me now, many thanks for the changes! I ran a few large VMs (up to 1TB guest), also tested on an ARM32 host, it all seems to still work fine, and no UBSAN messages. Reviewed-by: Andre Przywara Cheers, Andre > --- > builtin-run.c | 19 ++++++++++--------- > include/kvm/kvm-config.h | 7 ++++--- > include/kvm/kvm.h | 2 +- > 3 files changed, 15 insertions(+), 13 deletions(-) > > diff --git a/builtin-run.c b/builtin-run.c > index 0126c9fbcba6..2bf93fe13c92 100644 > --- a/builtin-run.c > +++ b/builtin-run.c > @@ -36,6 +36,7 @@ > > #include > #include > +#include > > #include > #include > @@ -264,7 +265,7 @@ static u64 host_ram_size(void) > return 0; > } > > - return (nr_pages * page_size) >> MB_SHIFT; > + return (u64)nr_pages * page_size; > } > > /* > @@ -278,11 +279,11 @@ static u64 get_ram_size(int nr_cpus) > u64 available; > u64 ram_size; > > - ram_size = 64 * (nr_cpus + 3); > + ram_size = (u64)SZ_64M * (nr_cpus + 3); > > available = host_ram_size() * RAM_SIZE_RATIO; > if (!available) > - available = MIN_RAM_SIZE_MB; > + available = MIN_RAM_SIZE; > > if (ram_size > available) > ram_size = available; > @@ -595,13 +596,13 @@ static struct kvm *kvm_cmd_run_init(int argc, const char **argv) > > if (!kvm->cfg.ram_size) > kvm->cfg.ram_size = get_ram_size(kvm->cfg.nrcpus); > + else > + kvm->cfg.ram_size <<= MB_SHIFT; > > if (kvm->cfg.ram_size > host_ram_size()) > pr_warning("Guest memory size %lluMB exceeds host physical RAM size %lluMB", > - (unsigned long long)kvm->cfg.ram_size, > - (unsigned long long)host_ram_size()); > - > - kvm->cfg.ram_size <<= MB_SHIFT; > + (unsigned long long)kvm->cfg.ram_size >> MB_SHIFT, > + (unsigned long long)host_ram_size() >> MB_SHIFT); > > if (!kvm->cfg.dev) > kvm->cfg.dev = DEFAULT_KVM_DEV; > @@ -676,12 +677,12 @@ static struct kvm *kvm_cmd_run_init(int argc, const char **argv) > if (kvm->cfg.kernel_filename) { > printf(" # %s run -k %s -m %Lu -c %d --name %s\n", KVM_BINARY_NAME, > kvm->cfg.kernel_filename, > - (unsigned long long)kvm->cfg.ram_size / 1024 / 1024, > + (unsigned long long)kvm->cfg.ram_size >> MB_SHIFT, > kvm->cfg.nrcpus, kvm->cfg.guest_name); > } else if (kvm->cfg.firmware_filename) { > printf(" # %s run --firmware %s -m %Lu -c %d --name %s\n", KVM_BINARY_NAME, > kvm->cfg.firmware_filename, > - (unsigned long long)kvm->cfg.ram_size / 1024 / 1024, > + (unsigned long long)kvm->cfg.ram_size >> MB_SHIFT, > kvm->cfg.nrcpus, kvm->cfg.guest_name); > } > > diff --git a/include/kvm/kvm-config.h b/include/kvm/kvm-config.h > index 6a5720c4c7d4..31bc89520d52 100644 > --- a/include/kvm/kvm-config.h > +++ b/include/kvm/kvm-config.h > @@ -5,6 +5,8 @@ > #include "kvm/vfio.h" > #include "kvm/kvm-config-arch.h" > > +#include > + > #define DEFAULT_KVM_DEV "/dev/kvm" > #define DEFAULT_CONSOLE "serial" > #define DEFAULT_NETWORK "user" > @@ -15,14 +17,13 @@ > #define DEFAULT_SCRIPT "none" > #define DEFAULT_SANDBOX_FILENAME "guest/sandbox.sh" > > -#define MIN_RAM_SIZE_MB (64ULL) > -#define MIN_RAM_SIZE_BYTE (MIN_RAM_SIZE_MB << MB_SHIFT) > +#define MIN_RAM_SIZE SZ_64M > > struct kvm_config { > struct kvm_config_arch arch; > struct disk_image_params disk_image[MAX_DISK_IMAGES]; > struct vfio_device_params *vfio_devices; > - u64 ram_size; > + u64 ram_size; /* Guest memory size, in bytes */ > u8 num_net_devices; > u8 num_vfio_devices; > u64 vsock_cid; > diff --git a/include/kvm/kvm.h b/include/kvm/kvm.h > index ad732e56f5ed..7b14b33b50ca 100644 > --- a/include/kvm/kvm.h > +++ b/include/kvm/kvm.h > @@ -87,7 +87,7 @@ struct kvm { > struct kvm_cpu **cpus; > > u32 mem_slots; /* for KVM_SET_USER_MEMORY_REGION */ > - u64 ram_size; > + u64 ram_size; /* Guest memory size, in bytes */ > void *ram_start; > u64 ram_pagesize; > struct mutex mem_banks_lock; _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel