Re: git: 8f5406c77f1b - main - x86/xen: implement early init hook

From: Baptiste Daroussin <bapt_at_nours.eu>
Date: Thu, 22 Feb 2024 13:50:51 UTC
Le 22 février 2024 11:31:28 GMT+01:00, "Roger Pau Monné" <royger@FreeBSD.org> a écrit :
>The branch main has been updated by royger:
>
>URL: https://cgit.FreeBSD.org/src/commit/?id=8f5406c77f1b7009c36df4e025fd8789a46d33ce
>
>commit 8f5406c77f1b7009c36df4e025fd8789a46d33ce
>Author:     Roger Pau Monné <royger@FreeBSD.org>
>AuthorDate: 2024-02-02 10:56:32 +0000
>Commit:     Roger Pau Monné <royger@FreeBSD.org>
>CommitDate: 2024-02-22 10:08:05 +0000
>
>    x86/xen: implement early init hook
>    
>    Unify the HVM and PVH early setup, byt making both rely on the hypervisor
>    initialization hook part of identify_hypervisor().
>    
>    The current initialization takes care of the hypercall page, the sahred info
>    page and does any fixup necessary to metadata video console information if
>    FreeBSD is booted as the initial domain (so the video console is handed from
>    Xen into FreeBSD).
>    
>    Note this has the nice side effect of also allowing to use the Xen console on
>    HVM guests, which allows to get rid of the QEMU emulated uart and still get
>    a nice text console.
>    
>    Sponsored by: Cloud Software Group
>    Reviewed by: markj
>    Differential revision: https://reviews.freebsd.org/D43764
>---
> sys/x86/include/xen/xen-os.h |   2 -
> sys/x86/x86/identcpu.c       |  10 ++++-
> sys/x86/xen/hvm.c            | 105 +++++--------------------------------------
> sys/x86/xen/pv.c             |  17 +------
> 4 files changed, 22 insertions(+), 112 deletions(-)
>
>diff --git a/sys/x86/include/xen/xen-os.h b/sys/x86/include/xen/xen-os.h
>index ec0d4b1ab9f1..d3f21e2a6c45 100644
>--- a/sys/x86/include/xen/xen-os.h
>+++ b/sys/x86/include/xen/xen-os.h
>@@ -52,8 +52,6 @@ extern int xen_disable_pv_disks;
> /* tunable for disabling PV nics */
> extern int xen_disable_pv_nics;
> 
>-extern uint32_t xen_cpuid_base;
>-
> /* compatibility for accessing xen_ulong_t with atomics */
> #define	atomic_clear_xen_ulong		atomic_clear_long
> #define	atomic_set_xen_ulong		atomic_set_long
>diff --git a/sys/x86/x86/identcpu.c b/sys/x86/x86/identcpu.c
>index 6df138bccba1..919dda722d71 100644
>--- a/sys/x86/x86/identcpu.c
>+++ b/sys/x86/x86/identcpu.c
>@@ -67,6 +67,10 @@
> #include <x86/isa/icu.h>
> #include <x86/vmware.h>
> 
>+#ifdef XENHVM
>+#include <xen/xen-os.h>
>+#endif
>+
> #ifdef __i386__
> #define	IDENTBLUE_CYRIX486	0
> #define	IDENTBLUE_IBMCPU	1
>@@ -1345,7 +1349,11 @@ static struct {
> 	int		vm_guest;
> 	void		(*init)(void);
> } vm_cpuids[] = {
>-	{ "XenVMMXenVMM",	VM_GUEST_XEN },		/* XEN */
>+	{ "XenVMMXenVMM",	VM_GUEST_XEN,
>+#ifdef XENHVM
>+	  &xen_early_init,
>+#endif
>+	},						/* XEN */
> 	{ "Microsoft Hv",	VM_GUEST_HV },		/* Microsoft Hyper-V */
> 	{ "VMwareVMware",	VM_GUEST_VMWARE },	/* VMware VM */
> 	{ "KVMKVMKVM",		VM_GUEST_KVM },		/* KVM */
>diff --git a/sys/x86/xen/hvm.c b/sys/x86/xen/hvm.c
>index 6336602c8bc4..17500516debf 100644
>--- a/sys/x86/xen/hvm.c
>+++ b/sys/x86/xen/hvm.c
>@@ -104,22 +104,6 @@ void xen_emergency_print(const char *str, size_t size)
> 	outsb(XEN_HVM_DEBUGCONS_IOPORT, str, size);
> }
> 
>-uint32_t xen_cpuid_base;
>-
>-static uint32_t
>-xen_hvm_cpuid_base(void)
>-{
>-	uint32_t base, regs[4];
>-
>-	for (base = 0x40000000; base < 0x40010000; base += 0x100) {
>-		do_cpuid(base, regs);
>-		if (!memcmp("XenVMMXenVMM", &regs[1], 12)
>-		    && (regs[0] - base) >= 2)
>-			return (base);
>-	}
>-	return (0);
>-}
>-
> static void
> hypervisor_quirks(unsigned int major, unsigned int minor)
> {
>@@ -156,38 +140,6 @@ hypervisor_version(void)
> 	hypervisor_quirks(major, minor);
> }
> 
>-/*
>- * Allocate and fill in the hypcall page.
>- */
>-int
>-xen_hvm_init_hypercall_stubs(enum xen_hvm_init_type init_type)
>-{
>-	uint32_t regs[4];
>-
>-	if (xen_cpuid_base != 0)
>-		/* Already setup. */
>-		goto out;
>-
>-	xen_cpuid_base = xen_hvm_cpuid_base();
>-	if (xen_cpuid_base == 0)
>-		return (ENXIO);
>-
>-	/*
>-	 * Find the hypercall pages.
>-	 */
>-	do_cpuid(xen_cpuid_base + 2, regs);
>-	if (regs[0] != 1)
>-		return (EINVAL);
>-
>-	wrmsr(regs[1], (init_type == XEN_HVM_INIT_EARLY)
>-	    ? (vm_paddr_t)((uintptr_t)&hypercall_page - KERNBASE)
>-	    : vtophys(&hypercall_page));
>-
>-out:
>-	hypervisor_version();
>-	return (0);
>-}
>-
> /*
>  * Translate linear to physical address when still running on the bootloader
>  * created page-tables.
>@@ -336,12 +288,14 @@ xen_early_init(void)
> 	uint32_t regs[4];
> 	int rc;
> 
>-	xen_cpuid_base = xen_hvm_cpuid_base();
>-	if (xen_cpuid_base == 0)
>+	if (hv_high < hv_base + 2) {
>+		xc_printf("Invalid maximum leaves for hv_base\n");
>+		vm_guest = VM_GUEST_VM;
> 		return;
>+	}
> 
> 	/* Find the hypercall pages. */
>-	do_cpuid(xen_cpuid_base + 2, regs);
>+	do_cpuid(hv_base + 2, regs);
> 	if (regs[0] != 1) {
> 		xc_printf("Invalid number of hypercall pages %u\n",
> 		    regs[0]);
>@@ -362,33 +316,6 @@ xen_early_init(void)
> 	    fixup_console();
> }
> 
>-static void
>-xen_hvm_init_shared_info_page(void)
>-{
>-	struct xen_add_to_physmap xatp;
>-
>-	if (xen_pv_domain()) {
>-		/*
>-		 * Already setup in the PV case, shared_info is passed inside
>-		 * of the start_info struct at start of day.
>-		 */
>-		return;
>-	}
>-
>-	if (HYPERVISOR_shared_info == NULL) {
>-		HYPERVISOR_shared_info = malloc(PAGE_SIZE, M_XENHVM, M_NOWAIT);
>-		if (HYPERVISOR_shared_info == NULL)
>-			panic("Unable to allocate Xen shared info page");
>-	}
>-
>-	xatp.domid = DOMID_SELF;
>-	xatp.idx = 0;
>-	xatp.space = XENMAPSPACE_shared_info;
>-	xatp.gpfn = vtophys(HYPERVISOR_shared_info) >> PAGE_SHIFT;
>-	if (HYPERVISOR_memory_op(XENMEM_add_to_physmap, &xatp))
>-		panic("HYPERVISOR_memory_op failed");
>-}
>-
> static int
> set_percpu_callback(unsigned int vcpu)
> {
>@@ -505,32 +432,29 @@ xen_hvm_disable_emulated_devices(void)
> static void
> xen_hvm_init(enum xen_hvm_init_type init_type)
> {
>-	int error;
>-	int i;
>+	unsigned int i;
> 
> 	if (!xen_domain() ||
> 	    init_type == XEN_HVM_INIT_CANCELLED_SUSPEND)
> 		return;
> 
>-	error = xen_hvm_init_hypercall_stubs(init_type);
>+	hypervisor_version();
> 
> 	switch (init_type) {
> 	case XEN_HVM_INIT_LATE:
>-		if (error != 0)
>-			return;
>-
> 		setup_xen_features();
> #ifdef SMP
> 		cpu_ops = xen_hvm_cpu_ops;
> #endif
> 		break;
> 	case XEN_HVM_INIT_RESUME:
>-		if (error != 0)
>-			panic("Unable to init Xen hypercall stubs on resume");
>-
> 		/* Clear stale vcpu_info. */
> 		CPU_FOREACH(i)
> 			DPCPU_ID_SET(i, vcpu_info, NULL);
>+
>+		if (map_shared_info() != 0)
>+			panic("cannot map Xen shared info page");
>+
> 		break;
> 	default:
> 		panic("Unsupported HVM initialization type");
>@@ -540,13 +464,6 @@ xen_hvm_init(enum xen_hvm_init_type init_type)
> 	xen_evtchn_needs_ack = false;
> 	xen_hvm_set_callback(NULL);
> 
>-	/*
>-	 * On (PV)HVM domains we need to request the hypervisor to
>-	 * fill the shared info page, for PVH guest the shared_info page
>-	 * is passed inside the start_info struct and is already set, so this
>-	 * functions are no-ops.
>-	 */
>-	xen_hvm_init_shared_info_page();
> 	xen_hvm_disable_emulated_devices();
> } 
> 
>diff --git a/sys/x86/xen/pv.c b/sys/x86/xen/pv.c
>index e33fa41c83d7..0e6492b124b8 100644
>--- a/sys/x86/xen/pv.c
>+++ b/sys/x86/xen/pv.c
>@@ -147,12 +147,9 @@ isxen(void)
> }
> 
> #define CRASH(...) do {					\
>-	if (isxen()) {					\
>+	if (isxen())					\
> 		xc_printf(__VA_ARGS__);			\
>-		HYPERVISOR_shutdown(SHUTDOWN_crash);	\
>-	} else {					\
>-		halt();					\
>-	}						\
>+	halt();						\
> } while (0)
> 
> uint64_t
>@@ -162,16 +159,6 @@ hammer_time_xen(vm_paddr_t start_info_paddr)
> 	uint64_t physfree;
> 	char *kenv;
> 
>-	if (isxen()) {
>-		vm_guest = VM_GUEST_XEN;
>-		xen_early_init();
>-		if (xen_cpuid_base == 0) {
>-			xc_printf(
>-			    "ERROR: failed to initialize hypercall page\n");
>-			HYPERVISOR_shutdown(SHUTDOWN_crash);
>-		}
>-	}
>-
> 	start_info = (struct hvm_start_info *)(start_info_paddr + KERNBASE);
> 	if (start_info->magic != XEN_HVM_START_MAGIC_VALUE) {
> 		CRASH("Unknown magic value in start_info struct: %#x\n",

This breaks i386 kernel build

/home/bapt/worktrees/main/sys/x86/xen/hvm.c:156:47: error: format specifies type 'unsigned long' but the argument has type 'uintptr_t' (aka 'unsigned int') [-Werror,-Wformat]

Bapt