[PATCH kvmtool v2 0/3] Align value generated by get_ram_size() to the host's page size

[PATCH kvmtool v2 0/3] Align value generated by get_ram_size() to the host's page size

[Fuad Tabba]

Hi,

This patch series ensures that the value returned by
get_ram_size() is aligned to the host's page size. Without that,
KVM_SET_USER_MEMORY_REGION could fail when passed an unaligned
value.

Changes from V1:
- Factor out code for getting _SC_PHYS_PAGES into a function (Will)
- Apply the scaling down of the ramsize to the number of pages (Will)

Cheers,
/fuad

Fixes: 18bd8c3bd2a7 ("kvm tools: Don't use all of host RAM for guests by default")
Signed-off-by: Fuad Tabba <tabba@google.com>

Fuad Tabba (3):
  Factor out getting the host page size
  Factor out getting the number of physical memory host pages
  Apply scaling down the calculated guest ram size to the number of
    pages

 builtin-run.c | 44 +++++++++++++++++++++++++++-----------------
 1 file changed, 27 insertions(+), 17 deletions(-)


base-commit: bd4ba57156dad39349edfb2338bdc2f4ed3c0bae
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH kvmtool v2 1/3] Factor out getting the host page size

[Fuad Tabba]

Factor out getting the page size of the host into a separate
function. This will be used in a subsequent patch.

No functional change intended.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 builtin-run.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/builtin-run.c b/builtin-run.c
index b1a27fd..2801735 100644
--- a/builtin-run.c
+++ b/builtin-run.c
@@ -360,9 +360,21 @@ static void kernel_usage_with_options(void)
 		KVM_BINARY_NAME);
 }
 
+static long host_page_size(void)
+{
+	long page_size = sysconf(_SC_PAGE_SIZE);
+
+	if (page_size < 0) {
+		pr_warning("sysconf(_SC_PAGE_SIZE) failed");
+		return 0;
+	}
+
+	return page_size;
+}
+
 static u64 host_ram_size(void)
 {
-	long page_size;
+	long page_size = host_page_size();
 	long nr_pages;
 
 	nr_pages	= sysconf(_SC_PHYS_PAGES);
@@ -371,12 +383,6 @@ static u64 host_ram_size(void)
 		return 0;
 	}
 
-	page_size	= sysconf(_SC_PAGE_SIZE);
-	if (page_size < 0) {
-		pr_warning("sysconf(_SC_PAGE_SIZE) failed");
-		return 0;
-	}
-
 	return (u64)nr_pages * page_size;
 }
 
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH kvmtool v2 2/3] Factor out getting the number of physical memory host pages

[Fuad Tabba]

Factor out getting the number of physical pages available for the
host into a separate function. This will be used in a subsequent
patch.

No functional change intended.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 builtin-run.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/builtin-run.c b/builtin-run.c
index 2801735..44ea690 100644
--- a/builtin-run.c
+++ b/builtin-run.c
@@ -372,17 +372,23 @@ static long host_page_size(void)
 	return page_size;
 }
 
-static u64 host_ram_size(void)
+static long host_ram_nrpages(void)
 {
-	long page_size = host_page_size();
-	long nr_pages;
+	long nr_pages = sysconf(_SC_PHYS_PAGES);
 
-	nr_pages	= sysconf(_SC_PHYS_PAGES);
 	if (nr_pages < 0) {
 		pr_warning("sysconf(_SC_PHYS_PAGES) failed");
 		return 0;
 	}
 
+	return nr_pages;
+}
+
+static u64 host_ram_size(void)
+{
+	long page_size = host_page_size();
+	long nr_pages = host_ram_nrpages();
+
 	return (u64)nr_pages * page_size;
 }
 
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH kvmtool v2 3/3] Apply scaling down the calculated guest ram size to the number of pages

[Fuad Tabba]

Calculate the guest ram size based a ratio proportional to the
number of pages available, rather than the amount of memory
available in bytes, in the host. This is to ensure that the
result is always page-aligned.

If the result of get_ram_size() isn't aligned to the host page
size, it triggers an error in __kvm_set_memory_region(), called
via the KVM_SET_USER_MEMORY_REGION ioctl, which requires the size
to be page-aligned.

Fixes: 18bd8c3bd2a7 ("kvm tools: Don't use all of host RAM for guests by default")
Signed-off-by: Fuad Tabba <tabba@google.com>
---
 builtin-run.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/builtin-run.c b/builtin-run.c
index 44ea690..21373d4 100644
--- a/builtin-run.c
+++ b/builtin-run.c
@@ -400,17 +400,15 @@ static u64 host_ram_size(void)
 
 static u64 get_ram_size(int nr_cpus)
 {
-	u64 available;
-	u64 ram_size;
+	long nr_pages_available = host_ram_nrpages() * RAM_SIZE_RATIO;
+	u64 ram_size = (u64)SZ_64M * (nr_cpus + 3);
+	u64 available = MIN_RAM_SIZE;
 
-	ram_size	= (u64)SZ_64M * (nr_cpus + 3);
-
-	available	= host_ram_size() * RAM_SIZE_RATIO;
-	if (!available)
-		available = MIN_RAM_SIZE;
+	if (nr_pages_available)
+		available = nr_pages_available * host_page_size();
 
 	if (ram_size > available)
-		ram_size	= available;
+		ram_size = available;
 
 	return ram_size;
 }
-- 
2.41.0.255.g8b1d071c50-goog

Re: [PATCH kvmtool v2 0/3] Align value generated by get_ram_size() to the host's page size

[Will Deacon]

On Mon, 17 Jul 2023 13:12:29 +0100, Fuad Tabba wrote:
> This patch series ensures that the value returned by
> get_ram_size() is aligned to the host's page size. Without that,
> KVM_SET_USER_MEMORY_REGION could fail when passed an unaligned
> value.
> 
> Changes from V1:
> - Factor out code for getting _SC_PHYS_PAGES into a function (Will)
> - Apply the scaling down of the ramsize to the number of pages (Will)
> 
> [...]

Applied to kvmtool (master), thanks!

[1/3] Factor out getting the host page size
      https://git.kernel.org/will/kvmtool/c/b6bae725decc
[2/3] Factor out getting the number of physical memory host pages
      https://git.kernel.org/will/kvmtool/c/834e5ed62fb8
[3/3] Apply scaling down the calculated guest ram size to the number of pages
      https://git.kernel.org/will/kvmtool/c/63643b11ce7d

Cheers,
-- 
Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev