On Thu, 2002-11-28 at 21:59, John Levon wrote:
> On Thu, Nov 28, 2002 at 09:35:35PM -0500, graydon hoare wrote:
> > if you want, ok. I did it this way because I separate out a number of
> > logically distinct updates I make to a register value ("lower this
> > flag", "set this field", etc) which I wanted to perform in separate
> > statements, for readability, but I didn't want to incur a register write
> > for each of them.
> But we only need to write back to the CCCR once anyway.
oh down there sure, but see pmc_setup_one_p4_counter.
anyways I'm not explicitly disagreeing with your opinion on the matter,
I don't really *like* the macros either. I'm just explaining why out of
all the not-so-great macros I could have written, I wrote those.
> sure, but the real reason behind the uppercase convention is to make
> them an ad-hoc namespace. That's not needed here, and they contain
> enough logic as to distract visually.
oh. I thought it was more so you knew they were macros, and didn't
assume they had function-call semantics. well, anyways, again, I'm happy
to lower-case 'em. can I do it in the next (separate) patch?
> why are we testing ovf AND counter-positivity ? Isn't the latter on its
> own good enough (and then we can just have a single cccr_reset_ovf();)
we have to read the CCCR anyways to write back to it, I figured I'd
check its OVF flag as well, considering the hardware is not actually
behaving as documented, and checking overflow in the counter is not
officially described anywhere in the p4 arch docs either :)
> spacing. Also, please give a reference to the erratum inside the code,
> and does it work if this moved inside the overflow check if() (and can
> we avoid the re-read altogether in that case ?)
unfortunately, there are *two* error cases I'm trying to compensate for
here: one is when I didn't find any counters with their CCCR:OVF flag
set ("spurious NMIs"), which I compensate for by heuristically checking
positivity of counter; and one is when I found an overflowed counter,
wrote back to it, and the counter wasn't updated properly (hence the
I tried a couple variations on the logic, trying to omit different parts
or combine them, and was usually rewarded either with lockups or dropped
overflows. I'd be willing to try some more precise variants if you have
one in mind, of course, but I don't know exactly which variant you mean
by this description. I'm gradually losing hope that it will ever behave
according to the logic of its spec.