From owner-freebsd-acpi@FreeBSD.ORG Mon Mar 16 17:49:29 2015 Return-Path: Delivered-To: freebsd-acpi@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 2F4EDBC8 for ; Mon, 16 Mar 2015 17:49:29 +0000 (UTC) Received: from sola.nimnet.asn.au (paqi.nimnet.asn.au [115.70.110.159]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id F294D8CF for ; Mon, 16 Mar 2015 17:49:27 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by sola.nimnet.asn.au (8.14.2/8.14.2) with ESMTP id t2GHnIFR086406; Tue, 17 Mar 2015 04:49:18 +1100 (EST) (envelope-from smithi@nimnet.asn.au) Date: Tue, 17 Mar 2015 04:49:18 +1100 (EST) From: Ian Smith To: Anthony Jenkins Subject: Re: [PATCH] ACPI CMOS region support rev. 4 In-Reply-To: <5506FBE3.1000009@att.net> Message-ID: <20150317041624.K22641@sola.nimnet.asn.au> References: <20150222180817.GD27984@strugglingcoder.info> <54EB8C21.2080600@att.net> <2401337.2oUs7iAbtB@ralph.baldwin.cx> <54EF3D5D.4010106@att.net> <20150227222203.P38620@sola.nimnet.asn.au> <20150228125857.D1277@besplex.bde.org> <54F14368.4020807@att.net> <20150302002647.W42658@sola.nimnet.asn.au> <54F5E53D.1090601@att.net> <20150306025800.U46361@sola.nimnet.asn.au> <54F9D7E6.4050807@att.net> <5504FF32.3020202@att.net> <20150317001401.X22641@sola.nimnet.asn.au> <5506F00A.3030708@att.net> <5506FBE3.1000009@att.net> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Cc: freebsd-acpi@freebsd.org X-BeenThere: freebsd-acpi@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: ACPI and power management development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 16 Mar 2015 17:49:29 -0000 On Mon, 16 Mar 2015 11:50:59 -0400, Anthony Jenkins wrote: > On 03/16/2015 11:00 AM, Anthony Jenkins wrote: > > On 03/16/2015 09:59 AM, Ian Smith wrote: > >> On Sat, 14 Mar 2015 23:40:34 -0400, Anthony Jenkins wrote: > >>> + if (!acpi_check_rtc_byteaccess(function == ACPI_READ, address)) > >>> + return AE_BAD_PARAMETER; > >> acpi_check_rtc_byteaccess() needs to be called per byte of 1, 2 or 4 > >> bytes - or pass it 'bytes' also, and loop over each of them within? > >> ======= > >> > >> Otherwise (for example) a 2 byte read from 0x0b or 4 byte read from > >> 0x09-0x0b will read 0x0c (clearing interrupts), or a 2 or 4 byte write > >> to (say) 0x01 will also write to 0x02 and 0x04 (clobbering the time). > > Right, this is an (incorrect) hybrid of a few attempts, probably from > > around the time I lost my SSD and only had a single backup copy of my > > work to go from. In one revision I had disallowed all multibyte > > accesses (width > 8) since IMHO it was more consistent/correct with the > > suggested locking. I wasn't ignoring your suggestion, just making one > > or a few changes at a time (generally the simpler ones). > > Okay now I remember why I was reluctant to do this - suppose ACPIBIOS > does a multibyte op on a set of bytes whose last byte fails > acpi_check_rtc_byteaccess(). I will have already performed n-1 > accesses. At one point I had a revision (acpi_check_rtc_access()?) that > permitted/denied the entire request (it took the starting address and > byte length), but I guess that got lost too. I'll just recreate it... Yep, validating all access before doing any sounds like the way to go. Also, bytes = width >> 3 is ok, since you then affirm !(width & 0x07), so non-multiples of 8 bits are invalidated anyway. You should still check that width (or bytes) > 0, even if 0 should never be passed. I guess the Big Kids will start playing once this hits bugzilla? :) cheers, Ian