Date: Fri, 29 Apr 2016 14:17:01 -0700 From: John Baldwin <jhb@freebsd.org> To: Jung-uk Kim <jkim@freebsd.org> Cc: freebsd-current@freebsd.org, jenkins-admin@freebsd.org, Kurt Lidl <lidl@pix.net> Subject: Re: Build failed in Jenkins: FreeBSD_HEAD #220 Message-ID: <4214221.s09rEQldUq@ralph.baldwin.cx> In-Reply-To: <00078caa-226f-77ec-a847-030789483f74@FreeBSD.org> References: <1930955076.5.1461942541071.JavaMail.jenkins@jenkins-9.freebsd.org> <4410839.umQIpSnpkP@ralph.baldwin.cx> <00078caa-226f-77ec-a847-030789483f74@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Friday, April 29, 2016 05:01:46 PM Jung-uk Kim wrote: > On 04/29/16 04:02 PM, John Baldwin wrote: > > On Friday, April 29, 2016 06:58:41 PM jenkins-admin@FreeBSD.org wrote: > >> See <https://jenkins.FreeBSD.org/job/FreeBSD_HEAD/220/>; > ... > >> Unhandled inl 0x0402 at 0xffffffff8039c2bd > > > > This is a read of the PM1 enable register. In bhyve this is treated as a 16-bit > > register, so only inw/outw is permitted. > > > > Hmm, PM1a is defined as 16-bits in the spec, so this seems to be an error in > > the recent ACPICA import. I note that the bitwidth calculations changed in > > hwregs.c in the most recent import. Perhaps those are incorrect? > > > > I suspect that this might also explain the issue Kurt Lidl reported of a VM > > shutting down after spewing errors related to FixedEvents and GPEs. > > Unfortunately, there were too many recent upstream commits to this file. :-( > > At least, we should take a closer look at the following commits: > > https://github.com/acpica/acpica/commit/6cb97888 > https://github.com/acpica/acpica/commit/3d8583a0 > https://github.com/acpica/acpica/commit/48eea5e7 > https://github.com/acpica/acpica/commit/41f6aefa > https://github.com/acpica/acpica/commit/c23034a3 > https://github.com/acpica/acpica/commit/c49a751b I bet it is getting confused because PM1_STATUS, etc. are 32-bit "psuedo" registers split across 2 16-bit registers and it's probably using the psuedo register size somewhere. Either that or the hardcoded '32' bits for I/O ports when AccessWidth isn't specified. Ah, that looks likely! In tbfadt.c the code creates psuedo GAS entries from the FADT data: /* * Calculate separate GAS structs for the PM1x (A/B) Status and Enable * registers. These addresses do not appear (directly) in the FADT, so it * is useful to pre-calculate them from the PM1 Event Block definitions. * * The PM event blocks are split into two register blocks, first is the * PM Status Register block, followed immediately by the PM Enable * Register block. Each is of length (Pm1EventLength/2) * * Note: The PM1A event block is required by the ACPI specification. * However, the PM1B event block is optional and is rarely, if ever, * used. */ for (i = 0; i < ACPI_FADT_PM_INFO_ENTRIES; i++) { Source64 = ACPI_ADD_PTR (ACPI_GENERIC_ADDRESS, &AcpiGbl_FADT, FadtPmInfoTable[i].Source); if (Source64->Address) { AcpiTbInitGenericAddress (FadtPmInfoTable[i].Target, Source64->SpaceId, Pm1RegisterByteWidth, Source64->Address + (FadtPmInfoTable[i].RegisterNum * Pm1RegisterByteWidth), "PmRegisters", 0); } } The helper function used always uses an AccessWidth of 0: static void AcpiTbInitGenericAddress ( ACPI_GENERIC_ADDRESS *GenericAddress, ... GenericAddress->SpaceId = SpaceId; GenericAddress->BitWidth = BitWidth; GenericAddress->BitOffset = 0; GenericAddress->AccessWidth = 0; /* Access width ANY */ } That AccessWidth of 0 for the PM1 blocks means that the function that computes AccessWidth falls into the case of always using 32 bits for I/O access. (For memory it uses MaxBitWidth.) You'll have to talk to the Intel guy who broke this to find out how he'd like to fix it (not hardcode 32, or fix the AccessWidth). Apparently they didn't bother to actually test these changes on any real machines with PM1 blocks though. :( (And/or the hardware they tested against isn't strict and permits 32-bit accesses to these 16-bit registers.) -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4214221.s09rEQldUq>