Re: [RFC kvmtool 3/9] arm64: Combine kvm.c

[Alexandru Elisei <alexandru.elisei@arm.com>]

Hi Oliver,

On Fri, Mar 14, 2025 at 03:25:10PM -0700, Oliver Upton wrote:
> Glue together the ARM common and previously arm64-specific bits into one
> source file.
> 
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> ---
>  Makefile                                   |   1 -
>  arm/aarch64/include/kvm/kvm-arch.h         |  22 ---
>  arm/aarch64/kvm.c                          | 212 ---------------------
>  arm/include/{arm-common => kvm}/kvm-arch.h |   4 +
>  arm/kvm.c                                  | 210 ++++++++++++++++++++
>  5 files changed, 214 insertions(+), 235 deletions(-)
>  delete mode 100644 arm/aarch64/include/kvm/kvm-arch.h
>  delete mode 100644 arm/aarch64/kvm.c
>  rename arm/include/{arm-common => kvm}/kvm-arch.h (97%)
> 
> diff --git a/Makefile b/Makefile
> index cf50cf7..72027e0 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -180,7 +180,6 @@ ifeq ($(ARCH), arm64)
>  	OBJS		+= hw/serial.o
>  	OBJS		+= arm/arm-cpu.o
>  	OBJS		+= arm/aarch64/kvm-cpu.o
> -	OBJS		+= arm/aarch64/kvm.o
>  	OBJS		+= arm/pvtime.o
>  	OBJS		+= arm/pmu.o
>  	ARCH_INCLUDE	:= arm/include
> diff --git a/arm/aarch64/include/kvm/kvm-arch.h b/arm/aarch64/include/kvm/kvm-arch.h
> deleted file mode 100644
> index 2d1a4ed..0000000
> diff --git a/arm/aarch64/kvm.c b/arm/aarch64/kvm.c
> deleted file mode 100644
> index 98b2437..0000000
> diff --git a/arm/include/arm-common/kvm-arch.h b/arm/include/kvm/kvm-arch.h
> similarity index 97%
> rename from arm/include/arm-common/kvm-arch.h
> rename to arm/include/kvm/kvm-arch.h
> index 60eec02..b55b3bf 100644
> --- a/arm/include/arm-common/kvm-arch.h
> +++ b/arm/include/kvm/kvm-arch.h
> @@ -84,6 +84,10 @@
>  
>  #define ARCH_HAS_PCI_EXP	1
>  
> +#define MAX_PAGE_SIZE	SZ_64K
> +
> +#define ARCH_HAS_CFG_RAM_ADDRESS	1

Would you mind grouping ARCH_HAS_* defines together?

> +
>  static inline bool arm_addr_in_ioport_region(u64 phys_addr)
>  {
>  	u64 limit = KVM_IOPORT_AREA + ARM_IOPORT_SIZE;
> diff --git a/arm/kvm.c b/arm/kvm.c
> index cc0cc4f..5e7fe77 100644
> --- a/arm/kvm.c
> +++ b/arm/kvm.c
> @@ -7,10 +7,19 @@
>  
>  #include "arm-common/gic.h"
>  
> +#include <linux/byteorder.h>
> +#include <linux/cpumask.h>
>  #include <linux/kernel.h>
>  #include <linux/kvm.h>
>  #include <linux/sizes.h>
>  
> +#include <asm/image.h>
> +
> +#include <kvm/util.h>

kvm/util.h is already included at the top of the file. Otherwise looks good
to me.

Thanks,
Alex

> +
> +
> +static struct arm64_image_header *kernel_header;
> +
>  struct kvm_ext kvm_req_ext[] = {
>  	{ DEFINE_KVM_EXT(KVM_CAP_IRQCHIP) },
>  	{ DEFINE_KVM_EXT(KVM_CAP_ONE_REG) },
> @@ -87,6 +96,33 @@ void kvm__arch_set_cmdline(char *cmdline, bool video)
>  {
>  }
>  
> +static void kvm__arch_enable_mte(struct kvm *kvm)
> +{
> +	struct kvm_enable_cap cap = {
> +		.cap = KVM_CAP_ARM_MTE,
> +	};
> +
> +	if (kvm->cfg.arch.aarch32_guest) {
> +		pr_debug("MTE is incompatible with AArch32");
> +		return;
> +	}
> +
> +	if (kvm->cfg.arch.mte_disabled) {
> +		pr_debug("MTE disabled by user");
> +		return;
> +	}
> +
> +	if (!kvm__supports_extension(kvm, KVM_CAP_ARM_MTE)) {
> +		pr_debug("MTE capability not available");
> +		return;
> +	}
> +
> +	if (ioctl(kvm->vm_fd, KVM_ENABLE_CAP, &cap))
> +		die_perror("KVM_ENABLE_CAP(KVM_CAP_ARM_MTE)");
> +
> +	pr_debug("MTE capability enabled");
> +}
> +
>  void kvm__arch_init(struct kvm *kvm)
>  {
>  	/* Create the virtual GIC. */
> @@ -96,6 +132,90 @@ void kvm__arch_init(struct kvm *kvm)
>  	kvm__arch_enable_mte(kvm);
>  }
>  
> +static u64 kvm__arch_get_payload_region_size(struct kvm *kvm)
> +{
> +	if (kvm->cfg.arch.aarch32_guest)
> +		return SZ_256M;
> +
> +	return SZ_512M;
> +}
> +
> +/*
> + * Return the TEXT_OFFSET value that the guest kernel expects. Note
> + * that pre-3.17 kernels expose this value using the native endianness
> + * instead of Little-Endian. BE kernels of this vintage may fail to
> + * boot. See Documentation/arm64/booting.rst in your local kernel tree.
> + */
> +static u64 kvm__arch_get_kern_offset(struct kvm *kvm)
> +{
> +	const char *debug_str;
> +
> +	/* the 32bit kernel offset is a well known value */
> +	if (kvm->cfg.arch.aarch32_guest)
> +		return 0x8000;
> +
> +	if (!kernel_header) {
> +		debug_str = "Kernel header is missing";
> +		goto default_offset;
> +	}
> +
> +	if (!le64_to_cpu(kernel_header->image_size)) {
> +		debug_str = "Image size is 0";
> +		goto default_offset;
> +	}
> +
> +	return le64_to_cpu(kernel_header->text_offset);
> +
> +default_offset:
> +	pr_debug("%s, assuming TEXT_OFFSET to be 0x80000", debug_str);
> +	return 0x80000;
> +}
> +
> +static void kvm__arch_read_kernel_header(struct kvm *kvm, int fd)
> +{
> +	const char *debug_str;
> +	off_t cur_offset;
> +	ssize_t size;
> +
> +	if (kvm->cfg.arch.aarch32_guest)
> +		return;
> +
> +	kernel_header = malloc(sizeof(*kernel_header));
> +	if (!kernel_header)
> +		return;
> +
> +	cur_offset = lseek(fd, 0, SEEK_CUR);
> +	if (cur_offset == (off_t)-1 || lseek(fd, 0, SEEK_SET) == (off_t)-1) {
> +		debug_str = "Failed to seek in kernel image file";
> +		goto fail;
> +	}
> +
> +	size = xread(fd, kernel_header, sizeof(*kernel_header));
> +	if (size < 0 || (size_t)size < sizeof(*kernel_header))
> +		die("Failed to read kernel image header");
> +
> +	lseek(fd, cur_offset, SEEK_SET);
> +
> +	if (memcmp(&kernel_header->magic, ARM64_IMAGE_MAGIC, sizeof(kernel_header->magic))) {
> +		debug_str = "Kernel image magic not matching";
> +		kernel_header = NULL;
> +		goto fail;
> +	}
> +
> +	return;
> +
> +fail:
> +	pr_debug("%s, using defaults", debug_str);
> +}
> +
> +static u64 kvm__arch_get_kernel_size(struct kvm *kvm)
> +{
> +	if (kvm->cfg.arch.aarch32_guest || !kernel_header)
> +		return 0;
> +
> +	return le64_to_cpu(kernel_header->image_size);
> +}
> +
>  #define FDT_ALIGN	SZ_2M
>  #define INITRD_ALIGN	4
>  bool kvm__arch_load_kernel_image(struct kvm *kvm, int fd_kernel, int fd_initrd,
> @@ -264,3 +384,93 @@ int kvm__arch_setup_firmware(struct kvm *kvm)
>  {
>  	return 0;
>  }
> +
> +int vcpu_affinity_parser(const struct option *opt, const char *arg, int unset)
> +{
> +	struct kvm *kvm = opt->ptr;
> +	const char *cpulist = arg;
> +	cpumask_t *cpumask;
> +	int cpu, ret;
> +
> +	kvm->cfg.arch.vcpu_affinity = cpulist;
> +
> +	cpumask = calloc(1, cpumask_size());
> +	if (!cpumask)
> +		die_perror("calloc");
> +
> +	ret = cpulist_parse(cpulist, cpumask);
> +	if (ret) {
> +		free(cpumask);
> +		return ret;
> +	}
> +
> +	kvm->arch.vcpu_affinity_cpuset = CPU_ALLOC(NR_CPUS);
> +	if (!kvm->arch.vcpu_affinity_cpuset)
> +		die_perror("CPU_ALLOC");
> +	CPU_ZERO_S(CPU_ALLOC_SIZE(NR_CPUS), kvm->arch.vcpu_affinity_cpuset);
> +
> +	for_each_cpu(cpu, cpumask)
> +		CPU_SET(cpu, kvm->arch.vcpu_affinity_cpuset);
> +
> +	return 0;
> +}
> +
> +void kvm__arch_validate_cfg(struct kvm *kvm)
> +{
> +
> +	if (kvm->cfg.ram_addr < ARM_MEMORY_AREA) {
> +		die("RAM address is below the I/O region ending at %luGB",
> +		    ARM_MEMORY_AREA >> 30);
> +	}
> +
> +	if (kvm->cfg.arch.aarch32_guest &&
> +	    kvm->cfg.ram_addr + kvm->cfg.ram_size > SZ_4G) {
> +		die("RAM extends above 4GB");
> +	}
> +}
> +
> +u64 kvm__arch_default_ram_address(void)
> +{
> +	return ARM_MEMORY_AREA;
> +}
> +
> +static int kvm__arch_get_ipa_limit(struct kvm *kvm)
> +{
> +	int ret;
> +
> +	ret = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION, KVM_CAP_ARM_VM_IPA_SIZE);
> +	if (ret <= 0)
> +		ret = 0;
> +
> +	return ret;
> +}
> +
> +int kvm__get_vm_type(struct kvm *kvm)
> +{
> +	unsigned int ipa_bits, max_ipa_bits;
> +	unsigned long max_ipa;
> +
> +	/* If we're running on an old kernel, use 0 as the VM type */
> +	max_ipa_bits = kvm__arch_get_ipa_limit(kvm);
> +	if (!max_ipa_bits)
> +		return 0;
> +
> +	/* Otherwise, compute the minimal required IPA size */
> +	max_ipa = kvm->cfg.ram_addr + kvm->cfg.ram_size - 1;
> +	ipa_bits = max(32, fls_long(max_ipa));
> +	pr_debug("max_ipa %lx ipa_bits %d max_ipa_bits %d",
> +		 max_ipa, ipa_bits, max_ipa_bits);
> +
> +	if (ipa_bits > max_ipa_bits)
> +		die("Memory too large for this system (needs %d bits, %d available)", ipa_bits, max_ipa_bits);
> +
> +	return KVM_VM_TYPE_ARM_IPA_SIZE(ipa_bits);
> +}
> +
> +static int kvm__arch_free_kernel_header(struct kvm *kvm)
> +{
> +	free(kernel_header);
> +
> +	return 0;
> +}
> +late_exit(kvm__arch_free_kernel_header);
> -- 
> 2.39.5
> 
>