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