On Oct 27, 2014, at 1:18 PM, John Baldwin <j...@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 <j...@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
signature.asc
Description: Message signed with OpenPGP using GPGMail
