On 03/16/15 19:36, Jung-uk Kim wrote:
> 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
Ahh... style(9) is what I was looking for, thanks. Yeah I was hoping
someone would point me to FreeBSD's official coding style guide.
> but the most serious
> problem is this patch makes atrtc.c dependent on ACPI and it
> practically kills off APM support.
Not quite sure what you mean here... do you mean a non-ACPI build of the
kernel would fail to compile atrtc(4)? Yeah I can fix that.
> Please make it optional (hint:
> sys/conf/files* and sys/conf/options*) although I don't mind killing
> off APM support. ;-)
Well I'd just make the ACPI CMOS handler code conditional on ACPI being
enabled in config(5)... is that what you're looking for (or would that
> Jung-uk Kim
> email@example.com mailing list
> To unsubscribe, send any mail to "freebsd-acpi-unsubscr...@freebsd.org"
firstname.lastname@example.org mailing list
To unsubscribe, send any mail to "freebsd-acpi-unsubscr...@freebsd.org"