Re: merge bouyer-xenpvh to HEAD

[Maxime Villard <max%m00nbsd.net@localhost>]

Le 22/04/2020 à 19:07, Manuel Bouyer a écrit :
> Hello,
> As you've probably noticed, I've been working in the bouyer-xenpvh on
> improving the work started by cherry@ on Xen PVHVM support. The main goal
> was to be able to have it in GENERIC, so that we don't need to distribute
> yet another kernel. This is also a step toward PVH support.
> Right now in the branch there is a GENERIC_XENHVM config, but it's
> only to ease checking that GENERIC without Xen support stil builds and boot.
> It has been tested on amd64 native, PV domU, HVM domU and PV dom0.
> It has been tested on i386 native, PV domU and HVM domU.
> 
> The branch covers src/common and src/sys but the changes are in
> sys/arch/{amd64,i386,x86,xen}/ (there is a XEN -> XENPV in
> common/lib/libc/arch too).
> You can see the diff with
> cvs rdiff -kk -u -rbouyer-xenpvh-base1 -rbouyer-xenpvh src/sys
> 
> In amd64 and i386, the main change is that there is less #ifdef XEN
> in assembly (and some headers and C files in x86), as we can now use the
> native ci_ipending for Xen events too.
> I also took the opportunity to factor some common code, like cpu_initclocks()
> and associated function pointers.
> Of course the */conf/ files had to be adjusted too, but there is less
> duplicate entries now. But there are !xenpv conditions in non-xen file.*
> 
> Also, on amd64 the cli/sti based spllower is gone, we always use the
> cmpxchg8 based one.
> 
> vm_guest got new types and associated sysctl value: XenPV, XenPVH.
> XenHVM and XenPVHVM.
> 
> There are 3 new functions pointers to struct pic:
> pic_intr_get_devname, pic_intr_get_assigned, pic_intr_get_count.
> This is a step towrard better integration between native interrupts and
> events, and allows Xen events to show up in 'intrctl list' on PVHVM.
> 
> On the Xen PV side, this gives us __HAVE_FAST_SOFTINTS and kernel
> preemption (except on dom0 where MULTIPROCESSOR is still disabled,
> some dom0 stuff is still not MP-safe yet).
> For this I had to move parts of x86/x86/intr.c to a new x86/x86/x86_softintr.c.
> 
> 
> If there's no objection I'd like to merge this to HEAD this week-end.

Two late remarks.

In locore.S:

	+	movl	$VM_GUEST_XENPV, _C_LABEL(vm_guest)

vm_guest being an enum, I don't know if the "movl" is correct. The compiler
could make vm_guest an uint8_t. I think it should be switched to a fixed-
size type.

In vector.S:

	+	jmp _C_LABEL(Xhypervisor_pvhvm_callback)
	+	TEXT_USER_BEGIN
	 IDTVEC(hypervisor_pvhvm_callback)
	 	pushq	$0		/* Dummy error code */
	 	pushq	$T_ASTFLT
	 	INTRENTRY
	-	/* sti?? */
	+	movl    CPUVAR(ILEVEL),%edi
	+	pushq   %rdi /* for Xdoreti */
	+	incl	CPUVAR(IDEPTH)
	...

All the code after INTRENTRY should be in a separate function in the .text
section, because with Meltdown, the code between TEXT_USER_BEGIN and
TEXT_USER_END is leaked to userland. Here your change leaks more information
than necessary.

You can use intr_lapic_ltimer as an example, we leave right after the
INTRENTRY.

Thanks,
Maxime