svn commit: r273214 - in head/sys: amd64/vmm/intel modules/vmm

Warner Losh imp at bsdimp.com
Tue Oct 28 03:04:46 UTC 2014


On Oct 27, 2014, at 1:18 PM, John Baldwin <jhb at FreeBSD.org> wrote:

> On Monday, October 27, 2014 11:36:41 AM Warner Losh wrote:
>> On Oct 27, 2014, at 10:54 AM, John Baldwin <jhb at freebsd.org> wrote:
>>> On Friday, October 17, 2014 01:20:50 PM Warner Losh wrote:
>>>> Author: imp
>>>> Date: Fri Oct 17 13:20:49 2014
>>>> New Revision: 273214
>>>> URL: https://svnweb.freebsd.org/changeset/base/273214
>>>> 
>>>> Log:
>>>> Fix build to not bogusly always rebuild vmm.ko.
>>>> 
>>>> Rename vmx_assym.s to vmx_assym.h to reflect that file's actual use
>>>> and update vmx_support.S's include to match. Add vmx_assym.h to the
>>>> SRCS to that it gets properly added to the dependency list. Add
>>>> vmx_support.S to SRCS as well, so it gets built and needs fewer
>>>> special-case goo. Remove now-redundant special-case goo. Finally,
>>>> vmx_genassym.o doesn't need to depend on a hand expanded ${_ILINKS}
>>>> explicitly, that's all taken care of by beforedepend.
>>>> 
>>>> With these items fixed, we no longer build vmm.ko every single time
>>>> through the modules on a KERNFAST build.
>>> 
>>> So I cheered for this before, but it appears to be broken. :(
>>> 
>>> Namely, I rebuilt world + kernel on my laptop this weekend (it was about a
>>> month old).  My normal setup builds kernels with NO_KERNELCLEAN=yes.  On my
>>> next reboot when I started a bhyve VM I promptly got a panic due to a page
>>> 
>>> fault in this assembly code:
>>> 	/*
>>> 	
>>> 	 * If 'vmx->eptgen[curcpu]' is not identical to 'pmap->pm_eptgen'
>>> 	 * then we must invalidate all mappings associated with this EPTP.
>>> 	 */
>>> 	
>>> 	movq	PM_EPTGEN(%r11), %r10
>>> 	cmpq	%r10, VMX_EPTGEN(%rsi, %rax, 8)
>>> 	je	guest_restore
>>> 
>>> (The 'cmpq' instruction)
>>> 
>>> This change came to mind, so I blew away the 'vmm' module directory and
>>> rebuilt my kernel.  Comparing the assembly of this instruction before and
>>> after used different values for VMX_EPTGEN.  In other words, the
>>> NO_KERNELCLEAN=yes build failed to regenerate vmx_assym.h and the build
>>> used stale values.
>> 
>> Is there a way to force this condition for testing?
> 
> You could checkout an older tree (probably before the recent merge of AMD SVM
> support) and build vmm.ko, then svn update and see if vmx_assym and
> vmx_support.o are updated.
> 
> Actually, this was simpler:
> 
> % cd sys/modules/vmm
> % make depend
> % make vmx_assym.h  # reports nothing to do
> % touch machine/vmm.h  # vmx_genassym.c includes this
> % make vmx_assym.h  # should rebuild, but doesn’t

Thanks!

>>> In particular, if you examine the generated .depend file, you will find
>>> that there are no dependencies recorded for vmx_genassym.o, so it is
>>> never rebuilt if any of the headers it includes are changed.  In my case
>>> the panic happened to be one that was easily diagnosed, but I could
>>> imagine stale assym headers causing very odd crashes that would be quite
>>> hard to track down.  I think these changes should be reverted if we can't
>>> fix the dependencies of the associated object files they are generated
>>> from. :(
>> 
>> Give me a bit and I’ll fix it. There’s a number of implicit dependencies
>> that don’t get recorded in the .depend file, iirc, so that’s not completely
>> conclusive. Not building, though is kinda a big hint that something’s
>> amiss.
> 
> I think the thing here is that for the assym files we don't record any
> dependency info at all.  The main kernel build does record dependencies
> for genassym.o in .depend, so it must be doable.

True.

> In kern.pre.mk:
> 
> GEN_CFILES= $S/$M/$M/genassym.c ${MFILES:T:S/.m$/.c/}
> 
> and those are then explicitly passed to mkdep in kern.post.mk.
> 
> So this fixes it:
> 
> Index: Makefile
> ===================================================================
> --- Makefile	(revision 273555)
> +++ Makefile	(working copy)
> @@ -4,6 +4,7 @@ KMOD=	vmm
> 
> SRCS=	opt_acpi.h opt_ddb.h device_if.h bus_if.h pci_if.h
> SRCS+=	vmx_assym.h svm_assym.h
> +DPSRCS=	vmx_genassym.c svm_genassym.c

That’s the magic I was looking for...

> CFLAGS+= -DVMM_KEEP_STATS -DSMP
> CFLAGS+= -I${.CURDIR}/../../amd64/vmm
> 
> I'll try to track down all the other assym files and fix them as well.

Cool. There’s three more (but I had only fixed two of them, since I didn’t have a i386 build handy).

>> However, -DNO_CLEAN has always been a very-sharp edged tool that will cut
>> you in a number of ways, so there’s no rush to back this out.
> 
> This is the first time in many years that NO_KERNELCLEAN=yes has been a
> problem for me.  (worlds sometimes have issues, but kernels rarely do).
> Also, usually when it breaks it fails to compile, it doesn't compile and
> then panic. :(

But it is current… :)

Warner

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 842 bytes
Desc: Message signed with OpenPGP using GPGMail
URL: <http://lists.freebsd.org/pipermail/svn-src-head/attachments/20141027/658a8701/attachment.sig>


More information about the svn-src-head mailing list