diff mbox series

[v3,kvmtool,11/13] arm/arm64: Consolidate RAM initialization in kvm__init_ram()

Message ID 20220525112345.121321-12-alexandru.elisei@arm.com (mailing list archive)
State New, archived
Headers show
Series arm64: Allow the user to set RAM base address | expand

Commit Message

Alexandru Elisei May 25, 2022, 11:23 a.m. UTC 
From: Julien Grall <julien.grall@arm.com>

RAM initialization is unnecessarily split between kvm__init_ram() and
kvm__arch_init(). Move all code related to RAM initialization to
kvm__init_ram(), making the code easier to follow and to modify.

One thing to note is that the initialization order is slightly altered:
kvm__arch_enable_mte() and gic__create() are now called before mmap'ing the
guest RAM. That is perfectly fine, as they don't use the host's mapping of
the guest memory.

Signed-off-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 arm/kvm.c | 52 ++++++++++++++++++++++++++--------------------------
 1 file changed, 26 insertions(+), 26 deletions(-)

Comments

Andre Przywara June 1, 2022, 1:20 p.m. UTC | #1 
On Wed, 25 May 2022 12:23:43 +0100
Alexandru Elisei <alexandru.elisei@arm.com> wrote:

Hi,

> From: Julien Grall <julien.grall@arm.com>
> 
> RAM initialization is unnecessarily split between kvm__init_ram() and
> kvm__arch_init(). Move all code related to RAM initialization to
> kvm__init_ram(), making the code easier to follow and to modify.
> 
> One thing to note is that the initialization order is slightly altered:
> kvm__arch_enable_mte() and gic__create() are now called before mmap'ing the
> guest RAM. That is perfectly fine, as they don't use the host's mapping of
> the guest memory.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>

I can confirm that this is just a code move, also MTE and GIC init do not
seem to rely on RAM in any way.

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre

> ---
>  arm/kvm.c | 52 ++++++++++++++++++++++++++--------------------------
>  1 file changed, 26 insertions(+), 26 deletions(-)
> 
> diff --git a/arm/kvm.c b/arm/kvm.c
> index bd44aa350796..abcccfabf59e 100644
> --- a/arm/kvm.c
> +++ b/arm/kvm.c
> @@ -26,9 +26,34 @@ bool kvm__arch_cpu_supports_vm(void)
>  
>  void kvm__init_ram(struct kvm *kvm)
>  {
> -	int err;
>  	u64 phys_start, phys_size;
>  	void *host_mem;
> +	int err;
> +
> +	/*
> +	 * Allocate guest memory. We must align our buffer to 64K to
> +	 * correlate with the maximum guest page size for virtio-mmio.
> +	 * If using THP, then our minimal alignment becomes 2M.
> +	 * 2M trumps 64K, so let's go with that.
> +	 */
> +	kvm->ram_size = kvm->cfg.ram_size;
> +	kvm->arch.ram_alloc_size = kvm->ram_size + SZ_2M;
> +	kvm->arch.ram_alloc_start = mmap_anon_or_hugetlbfs(kvm,
> +						kvm->cfg.hugetlbfs_path,
> +						kvm->arch.ram_alloc_size);
> +
> +	if (kvm->arch.ram_alloc_start == MAP_FAILED)
> +		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);
> +
> +	madvise(kvm->arch.ram_alloc_start, kvm->arch.ram_alloc_size,
> +		MADV_MERGEABLE);
> +
> +	madvise(kvm->arch.ram_alloc_start, kvm->arch.ram_alloc_size,
> +		MADV_HUGEPAGE);
>  
>  	phys_start	= ARM_MEMORY_AREA;
>  	phys_size	= kvm->ram_size;
> @@ -59,31 +84,6 @@ void kvm__arch_set_cmdline(char *cmdline, bool video)
>  
>  void kvm__arch_init(struct kvm *kvm)
>  {
> -	/*
> -	 * Allocate guest memory. We must align our buffer to 64K to
> -	 * correlate with the maximum guest page size for virtio-mmio.
> -	 * If using THP, then our minimal alignment becomes 2M.
> -	 * 2M trumps 64K, so let's go with that.
> -	 */
> -	kvm->ram_size = kvm->cfg.ram_size;
> -	kvm->arch.ram_alloc_size = kvm->ram_size + SZ_2M;
> -	kvm->arch.ram_alloc_start = mmap_anon_or_hugetlbfs(kvm,
> -						kvm->cfg.hugetlbfs_path,
> -						kvm->arch.ram_alloc_size);
> -
> -	if (kvm->arch.ram_alloc_start == MAP_FAILED)
> -		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);
> -
> -	madvise(kvm->arch.ram_alloc_start, kvm->arch.ram_alloc_size,
> -		MADV_MERGEABLE);
> -
> -	madvise(kvm->arch.ram_alloc_start, kvm->arch.ram_alloc_size,
> -		MADV_HUGEPAGE);
> -
>  	/* Create the virtual GIC. */
>  	if (gic__create(kvm, kvm->cfg.arch.irqchip))
>  		die("Failed to create virtual GIC");
diff mbox series

Patch

diff --git a/arm/kvm.c b/arm/kvm.c
index bd44aa350796..abcccfabf59e 100644
--- a/arm/kvm.c
+++ b/arm/kvm.c
@@ -26,9 +26,34 @@  bool kvm__arch_cpu_supports_vm(void)
 
 void kvm__init_ram(struct kvm *kvm)
 {
-	int err;
 	u64 phys_start, phys_size;
 	void *host_mem;
+	int err;
+
+	/*
+	 * Allocate guest memory. We must align our buffer to 64K to
+	 * correlate with the maximum guest page size for virtio-mmio.
+	 * If using THP, then our minimal alignment becomes 2M.
+	 * 2M trumps 64K, so let's go with that.
+	 */
+	kvm->ram_size = kvm->cfg.ram_size;
+	kvm->arch.ram_alloc_size = kvm->ram_size + SZ_2M;
+	kvm->arch.ram_alloc_start = mmap_anon_or_hugetlbfs(kvm,
+						kvm->cfg.hugetlbfs_path,
+						kvm->arch.ram_alloc_size);
+
+	if (kvm->arch.ram_alloc_start == MAP_FAILED)
+		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);
+
+	madvise(kvm->arch.ram_alloc_start, kvm->arch.ram_alloc_size,
+		MADV_MERGEABLE);
+
+	madvise(kvm->arch.ram_alloc_start, kvm->arch.ram_alloc_size,
+		MADV_HUGEPAGE);
 
 	phys_start	= ARM_MEMORY_AREA;
 	phys_size	= kvm->ram_size;
@@ -59,31 +84,6 @@  void kvm__arch_set_cmdline(char *cmdline, bool video)
 
 void kvm__arch_init(struct kvm *kvm)
 {
-	/*
-	 * Allocate guest memory. We must align our buffer to 64K to
-	 * correlate with the maximum guest page size for virtio-mmio.
-	 * If using THP, then our minimal alignment becomes 2M.
-	 * 2M trumps 64K, so let's go with that.
-	 */
-	kvm->ram_size = kvm->cfg.ram_size;
-	kvm->arch.ram_alloc_size = kvm->ram_size + SZ_2M;
-	kvm->arch.ram_alloc_start = mmap_anon_or_hugetlbfs(kvm,
-						kvm->cfg.hugetlbfs_path,
-						kvm->arch.ram_alloc_size);
-
-	if (kvm->arch.ram_alloc_start == MAP_FAILED)
-		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);
-
-	madvise(kvm->arch.ram_alloc_start, kvm->arch.ram_alloc_size,
-		MADV_MERGEABLE);
-
-	madvise(kvm->arch.ram_alloc_start, kvm->arch.ram_alloc_size,
-		MADV_HUGEPAGE);
-
 	/* Create the virtual GIC. */
 	if (gic__create(kvm, kvm->cfg.arch.irqchip))
 		die("Failed to create virtual GIC");