From owner-freebsd-current@freebsd.org Fri Apr 29 21:17:18 2016 Return-Path: Delivered-To: freebsd-current@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 7A61EB212D9 for ; Fri, 29 Apr 2016 21:17:18 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from bigwig.baldwin.cx (bigwig.baldwin.cx [IPv6:2001:470:1f11:75::1]) (using TLSv1 with cipher DHE-RSA-CAMELLIA256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 4DB7D19F7; Fri, 29 Apr 2016 21:17:18 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from ralph.baldwin.cx (c-73-231-226-104.hsd1.ca.comcast.net [73.231.226.104]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id D3AD8B987; Fri, 29 Apr 2016 17:17:16 -0400 (EDT) From: John Baldwin To: Jung-uk Kim Cc: freebsd-current@freebsd.org, jenkins-admin@freebsd.org, Kurt Lidl Subject: Re: Build failed in Jenkins: FreeBSD_HEAD #220 Date: Fri, 29 Apr 2016 14:17:01 -0700 Message-ID: <4214221.s09rEQldUq@ralph.baldwin.cx> User-Agent: KMail/4.14.3 (FreeBSD/10.2-STABLE; KDE/4.14.3; amd64; ; ) 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> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Fri, 29 Apr 2016 17:17:16 -0400 (EDT) X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 29 Apr 2016 21:17:18 -0000 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 > ... > >> 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