ACPI patch (was Re: 'microuptime() went backwards ...' using ACPI...)
Ok, here is a patch that executes a brute-force solution to the asynchronous counter problem. Basically it figures out a mask and then the timer code loops until two masked reads yield the same value, guarenteeing that we haven't caught the timer during a carry. On my system, the mask I got was: 0xFFFC which means I lost only 2 bits of accuracy in order to be able to retrieve accurate counter values. This gives my particular box an approximately 1uS accuracy. I think this may be the only safe way to do it. It looks like it is possible to catch the ripple and/or fast-carry on *any* bit, with the statistical chance of it occuring on higher bits dropping by 2x per bit. My proposed (tested) patch is below. Mike? acpi_timer0: 32-bit timer at 3.579545MHz mask fffc port 0x808-0x80b on acpi0 -Matt Index: acpi_timer.c === RCS file: /home/ncvs/src/sys/dev/acpica/acpi_timer.c,v retrieving revision 1.11 diff -u -r1.11 acpi_timer.c --- acpi_timer.c8 Jan 2002 06:45:56 - 1.11 +++ acpi_timer.c17 Feb 2002 23:26:29 - @@ -56,6 +56,7 @@ MODULE_NAME(TIMER) static device_tacpi_timer_dev; +static u_int32_t acpi_timer_mask; struct resource*acpi_timer_reg; #define TIMER_READ bus_space_read_4(rman_get_bustag(acpi_timer_reg), \ rman_get_bushandle(acpi_timer_reg),\ @@ -113,7 +114,7 @@ acpi_timer_identify(driver_t *driver, device_t parent) { device_t dev; -char desc[40]; +char desc[48]; intrid; FUNCTION_TRACE(__func__); @@ -138,14 +139,32 @@ if (getenv(debug.acpi.timer_test) != NULL) acpi_timer_test(); -acpi_timer_timecounter.tc_get_timecount = acpi_timer_get_timecount; +acpi_timer_timecounter.tc_get_timecount = acpi_timer_get_timecount_safe; acpi_timer_timecounter.tc_frequency = acpi_timer_frequency; tc_init(acpi_timer_timecounter); -sprintf(desc, %d-bit timer at 3.579545MHz, AcpiGbl_FADT-TmrValExt ? 32 : 24); +for (acpi_timer_mask = 0x; acpi_timer_mask; acpi_timer_mask = 1) { + u_int32_t u1; + u_int32_t u2; + int count = 10; + + u1 = TIMER_READ; + u2 = TIMER_READ; + while (count ((u1 ^ u2) acpi_timer_mask)) { + u1 = u2; + u2 = TIMER_READ; + --count; + } + if (count) + break; +} +acpi_timer_mask = 1; + +sprintf(desc, %d-bit timer at 3.579545MHz mask %08x, + (AcpiGbl_FADT-TmrValExt ? 32 : 24), acpi_timer_mask); device_set_desc_copy(dev, desc); -#if 0 +#if 0 { u_int64_t first; @@ -192,16 +211,22 @@ static unsigned acpi_timer_get_timecount_safe(struct timecounter *tc) { -unsigned u1, u2, u3; +u_int32_t u1; +u_int32_t u2; +u1 = TIMER_READ; u2 = TIMER_READ; -u3 = TIMER_READ; -do { +while ((u1 ^ u2) acpi_timer_mask) { u1 = u2; - u2 = u3; - u3 = TIMER_READ; -} while (u1 u2 || u2 u3); -return (u2); + u2 = TIMER_READ; +} +#if 0 /* DEBUGGING */ +if (u2 u1) { + u_int32_t u3 = TIMER_READ; + printf(ACPI TIMER LSB MISREAD %08x %08x %08x\n, u1, u2, u3); +} +#endif +return(u2 acpi_timer_mask); } /* To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: ACPI patch (was Re: 'microuptime() went backwards ...' using ACPI...)
Ok, here is a patch that executes a brute-force solution to the asynchronous counter problem. Basically it figures out a mask and then the timer code loops until two masked reads yield the same value, guarenteeing that we haven't caught the timer during a carry. On my system, the mask I got was: 0xFFFC which means I lost only 2 bits of accuracy in order to be able to retrieve accurate counter values. This gives my particular box an approximately 1uS accuracy. I think this may be the only safe way to do it. It looks like it is possible to catch the ripple and/or fast-carry on *any* bit, with the statistical chance of it occuring on higher bits dropping by 2x per bit. My proposed (tested) patch is below. Mike? I have some reservations about this, because I'm not sure that 10 successive reads will catch the ripple-counter problem that the old PIIX4s have. I think that if this code detects a need for the mask technique, it should set the handler to a function that will deal with the mask. If it doesn't, it should assume that we need the 'safe' function as it currently exists (I'm not sure why it's not the default, unless there's a log message to explain it, it must have been a mistake on my part). I'd also like to see the description include the number of bits, rather than the mask, if we are using a mask. Thanks for taking the time to track this one down. -- To announce that there must be no criticism of the president, or that we are to stand by the president, right or wrong, is not only unpatriotic and servile, but is morally treasonable to the American public. - Theodore Roosevelt To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: ACPI patch (was Re: 'microuptime() went backwards ...' using ACPI...)
: :I have some reservations about this, because I'm not sure that 10 :successive reads will catch the ripple-counter problem that the old PIIX4s :have. Just goes to show that I need to document my code :-) Those reads are not detecting the ripple-counter problem, they are figuring out the mask required to guarentee that two successive reads will return the same counter value so the counter read in the later code doesn't end up in an infinite loop. i.e. on a slow cpu the mask might have to remove more bits because the counter increments a number of times between reads. On a fast cpu, fewer bits would have to be chopped off. That's all. :I'd also like to see the description include the number of bits, rather :than the mask, if we are using a mask. : :Thanks for taking the time to track this one down. Well, I don't want to spend too much time on it because this was incidental to other work I'm doing on -current. If you think it's worth comitting I'll commit it, otherwise you can use it as a template for your own fix/commit. It would be nice if some sort of solution were comitted to the tree relatively soon. -Matt Matthew Dillon [EMAIL PROTECTED] To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message