Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 16 Mar 2015 19:36:06 -0400
From:      Jung-uk Kim <jkim@FreeBSD.org>
To:        Anthony Jenkins <Anthony.B.Jenkins@att.net>,  Ian Smith <smithi@nimnet.asn.au>
Cc:        freebsd-acpi@freebsd.org
Subject:   Re: [PATCH] ACPI CMOS region support rev. 5
Message-ID:  <550768E6.6000801@FreeBSD.org>
In-Reply-To: <55073442.5060005@att.net>
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> <20150317041624.K22641@sola.nimnet.asn.au> <55073442.5060005@att.net>

next in thread | previous in thread | raw e-mail | index | archive | help
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 03/16/2015 15:51, Anthony Jenkins wrote:
> On 03/16/2015 01:49 PM, Ian Smith wrote:
>> 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.
> 
> Oh yeah, forgot about that!
> 
>> I guess the Big Kids will start playing once this hits bugzilla?
>> :)
> 
> I'm just glad I get to learn how to commit stuff to FreeBSD.
> 
> Here's another iteration...comments welcome.  Think I got (most) 
> everything in there.  I need to unit test acpi_check_rtc_access()
> to make sure it DTRT.

I see that there are several minor style(9) bugs but the most serious
problem is this patch makes atrtc.c dependent on ACPI and it
practically kills off APM support.  Please make it optional (hint:
sys/conf/files* and sys/conf/options*) although I don't mind killing
off APM support. ;-)

Jung-uk Kim
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQEcBAEBCAAGBQJVB2jgAAoJEHyflib82/FG2i4IAIVjf2fN6HcxVBzWbB5SWBGl
d4ZircYjOkq5ld8jqVBuZP+En5Jm94JABo1Hp3XV4z8GNzCT29jB8STh4WmpU8Zu
A6kURXJjAPyUZbbQKtRr1YzTOfzttUNBBnPPbFvyAZG0vLEjCwZnx/2t7yrO2A/I
7PgbW5Qtl1TTRYux/eF6zFEWo2nPrK70Rr8dKCYTUlJnYorz3YVuSkM1PrjJUjom
6C0t3N3X5BxziuW/NRwWWWCf2EOkAR3Mdefo/eFASm8+n4GTCpxflnWPuy6NjIYY
1NSmqGurTcFoyQ9igzl7J8gWteqwaPMjlpAp0GCU1ADpkhrBmcU7dFK9C7jcOaE=
=ZKAN
-----END PGP SIGNATURE-----



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?550768E6.6000801>