Date: Mon, 16 Mar 2015 15:51:30 -0400 From: Anthony Jenkins <Anthony.B.Jenkins@att.net> To: Ian Smith <smithi@nimnet.asn.au> Cc: freebsd-acpi@freebsd.org Subject: [PATCH] ACPI CMOS region support rev. 5 Message-ID: <55073442.5060005@att.net> In-Reply-To: <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> <20150317041624.K22641@sola.nimnet.asn.au>
next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format. --------------060805080104030201010503 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit 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. Anthony > cheers, Ian > _______________________________________________ > freebsd-acpi@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-acpi > To unsubscribe, send any mail to "freebsd-acpi-unsubscribe@freebsd.org" --------------060805080104030201010503 Content-Type: text/x-patch; name="atrtc_c_rev5.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="atrtc_c_rev5.diff" Index: sys/x86/isa/atrtc.c =================================================================== --- sys/x86/isa/atrtc.c (revision 279957) +++ sys/x86/isa/atrtc.c (working copy) @@ -31,6 +31,7 @@ __FBSDID("$FreeBSD$"); #include "opt_isa.h" +#include "opt_acpi.h" #include <sys/param.h> #include <sys/systm.h> @@ -53,9 +54,17 @@ #include <machine/intr_machdep.h> #include "clock_if.h" +#include <contrib/dev/acpica/include/acpi.h> +#include <contrib/dev/acpica/include/accommon.h> +#include <dev/acpica/acpivar.h> + #define RTC_LOCK do { if (!kdb_active) mtx_lock_spin(&clock_lock); } while (0) #define RTC_UNLOCK do { if (!kdb_active) mtx_unlock_spin(&clock_lock); } while (0) +#define IO_DELAY() (void)inb(0x84) +#define IO_RTC_ADDR (IO_RTC + 0) +#define IO_RTC_DATA (IO_RTC + 1) + int atrtcclock_disable = 0; static int rtc_reg = -1; @@ -62,6 +71,16 @@ static u_char rtc_statusa = RTCSA_DIVIDER | RTCSA_NOPROF; static u_char rtc_statusb = RTCSB_24HR; +#ifndef ATRTC_VERBOSE +#define ATRTC_VERBOSE 1 +#endif + +static unsigned int atrtc_verbose = ATRTC_VERBOSE; +SYSCTL_UINT(_debug, OID_AUTO, atrtc_verbose, CTLFLAG_RWTUN, + &atrtc_verbose, 0, "AT-RTC Debug Level (0-2)"); +#define ATRTC_DBG_PRINTF(level, format, ...) \ + if (atrtc_verbose >= level) printf(format, ##__VA_ARGS__) + /* * RTC support routines */ @@ -73,10 +92,10 @@ RTC_LOCK; if (rtc_reg != reg) { - inb(0x84); + IO_DELAY(); outb(IO_RTC, reg); rtc_reg = reg; - inb(0x84); + IO_DELAY(); } val = inb(IO_RTC + 1); RTC_UNLOCK; @@ -89,16 +108,36 @@ RTC_LOCK; if (rtc_reg != reg) { - inb(0x84); + IO_DELAY(); outb(IO_RTC, reg); rtc_reg = reg; - inb(0x84); + IO_DELAY(); } outb(IO_RTC + 1, val); - inb(0x84); + IO_DELAY(); RTC_UNLOCK; } +static void +acpi_cmos_read(ACPI_PHYSICAL_ADDRESS address, UINT8 *buf, UINT32 buflen) +{ + UINT32 offset; + + for (offset = 0; offset < buflen; ++offset) { + buf[offset] = rtcin(address + offset) & 0xff; + } +} + +static void +acpi_cmos_write(ACPI_PHYSICAL_ADDRESS address, const UINT8 *buf, UINT32 buflen) +{ + UINT32 offset; + + for (offset = 0; offset < buflen; ++offset) { + writertc(address + offset, buf[offset]); + } +} + static __inline int readrtc(int port) { @@ -161,9 +200,72 @@ struct resource *intr_res; void *intr_handler; struct eventtimer et; + ACPI_HANDLE acpi_handle; /* Handle of the PNP0B00 node */ + int acpi_handle_registered; /* 0 = acpi_handle not registered */ }; static int +acpi_check_rtc_access(int is_read, u_long addr, u_long len) +{ + int retval = 1; /* Success */ + + if (is_read) { + /* Reading 0x0C will muck with interrupts */ + if (addr + len - 1 >= 0x0C && addr <= 0x0c) + retval = 0; + } else { + /* Allow single-byte writes to alarm registers and + * addr >= 0x30, else deny. + */ + if (!((len == 1 && (addr <= 5 && (addr & 1))) || addr >= 0x30)) + retval = 0; + } + return retval; +} + +static ACPI_STATUS +acpi_rtc_cmos_handler(UINT32 func, ACPI_PHYSICAL_ADDRESS addr, + UINT32 bitwidth, UINT64 *value, void *context, void *region_context) +{ + struct atrtc_softc *sc; + UINT32 bytewidth = bitwidth >> 3; + + sc = (struct atrtc_softc *)context; + if (!value || !sc) { + ATRTC_DBG_PRINTF(1, "NULL parameter.\n"); + return AE_BAD_PARAMETER; + } + if (bitwidth == 0 || bitwidth > 32 || (bitwidth & 0x07) || + addr + bytewidth - 1 > 63) { + ATRTC_DBG_PRINTF(1, + "Invalid bitwidth (%u) or addr (0x%08lx).\n", + bitwidth, addr); + return AE_BAD_PARAMETER; + } + if (!acpi_check_rtc_access(func == ACPI_READ, addr, bytewidth)) { + ATRTC_DBG_PRINTF(1, "Bad CMOS %s access at addr 0x%08lx.\n", + func == ACPI_READ ? "read" : "write", addr); + return AE_BAD_PARAMETER; + } + + switch (func) { + case ACPI_READ: + acpi_cmos_read(addr, (UINT8 *)value, bytewidth); + break; + case ACPI_WRITE: + acpi_cmos_write(addr, (const UINT8 *)value, bytewidth); + break; + default: + ATRTC_DBG_PRINTF(1, "Invalid function: %d.\n", func); + return AE_BAD_PARAMETER; + } + ATRTC_DBG_PRINTF(1, "%-5s%02u addr=%04lx val=%08x\n", + func == ACPI_READ ? "READ" : "WRITE", bytewidth, + addr, *((UINT32 *)value)); + return AE_OK; +} + +static int rtc_start(struct eventtimer *et, sbintime_t first, sbintime_t period) { @@ -245,10 +347,19 @@ int i; sc = device_get_softc(dev); + sc->acpi_handle = acpi_get_handle(dev); sc->port_res = bus_alloc_resource(dev, SYS_RES_IOPORT, &sc->port_rid, IO_RTC, IO_RTC + 1, 2, RF_ACTIVE); if (sc->port_res == NULL) device_printf(dev, "Warning: Couldn't map I/O.\n"); + if (ACPI_FAILURE(AcpiInstallAddressSpaceHandler(sc->acpi_handle, + ACPI_ADR_SPACE_CMOS, + acpi_rtc_cmos_handler, NULL, sc))) + { + device_printf(dev, "Warning: Couldn't register ACPI CMOS address space handler.\n"); + /* I assume the softc was memset() to 0? */ + } else + sc->acpi_handle_registered = 1; atrtc_start(); clock_register(dev, 1000000); bzero(&sc->et, sizeof(struct eventtimer)); @@ -286,6 +397,17 @@ return(0); } +static int atrtc_detach(device_t dev) +{ + struct atrtc_softc *sc; + + sc = device_get_softc(dev); + if (sc->acpi_handle_registered) + AcpiRemoveAddressSpaceHandler(sc->acpi_handle, + ACPI_ADR_SPACE_CMOS, acpi_rtc_cmos_handler); + return bus_generic_detach(dev); +} + static int atrtc_resume(device_t dev) { @@ -366,7 +488,7 @@ /* Device interface */ DEVMETHOD(device_probe, atrtc_probe), DEVMETHOD(device_attach, atrtc_attach), - DEVMETHOD(device_detach, bus_generic_detach), + DEVMETHOD(device_detach, atrtc_detach), DEVMETHOD(device_shutdown, bus_generic_shutdown), DEVMETHOD(device_suspend, bus_generic_suspend), /* XXX stop statclock? */ --------------060805080104030201010503--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?55073442.5060005>