Since x86-64 time handling is not overflow-safe, these are the
adjustments needed for allowing debuggers to update time after
having halted the system for perhaps extended periods of time.
Note that this depends on the HPET definitions adjustments, which
aren't in 2.6.14, but have supposedly been merged already into 2.6.15.
From: Jan Beulich <[email protected]>
(actual patch attached)
On Wednesday 09 November 2005 15:08, Jan Beulich wrote:
> Since x86-64 time handling is not overflow-safe, these are the
> adjustments needed for allowing debuggers to update time after
> having halted the system for perhaps extended periods of time.
>
> Note that this depends on the HPET definitions adjustments, which
> aren't in 2.6.14, but have supposedly been merged already into 2.6.15.
>
> From: Jan Beulich <[email protected]>
<no code quoting because that is hard with your attachments>
In general thanks for the overflow fixes. I understand the code
had generic problems in this area and it's good someone tackling them.
But many details should be changed.
The magic ICH6 HPET enable code has to go. It looks far too fragile and might
break something else. I also don't want direct PCI config space accesses like
this. We'll have to wait for ACPI HPET support I think. I think I'll remove
the original hack for AMD 8111 too - it was only for testing anyways. If you
still want to use it you have to keep it as a private patch.
Please remove the ifdefs too. 64bit HPET support would be fine, but
only as a runtime mechanism, not compile time.
Can you remove debugger_jiffies please?
The code has to handle long delays anyways (e.g. if someone uses a target
probe), so we cannot rely on such hacks anyways.
I don't quite understand why the SMP case should be different from UP
in that ifdef. Can you explain? It shouldn't in theory.
/* When the TSC gets reset during AP startup, the code below would
+ incorrectly think we lost a huge amount of ticks. */
That is outdated - the TSCs are not reset anymore since 2.6.12.
Please remove code for handling that.
The union in vxtime_data is ugly - can it be avoided?
Vojtech should probably review that one too when you repost.
-Andi
>The magic ICH6 HPET enable code has to go. It looks far too fragile and might
>break something else. I also don't want direct PCI config space accesses like
>this. We'll have to wait for ACPI HPET support I think. I think I'll remove
>the original hack for AMD 8111 too - it was only for testing anyways. If you
>still want to use it you have to keep it as a private patch.
That's fine with me. However, it was the only way for me to test the HPET code... It's not enabled anyway by default, but if you want the whole hack go away that's fine with me.
>Please remove the ifdefs too. 64bit HPET support would be fine, but
>only as a runtime mechanism, not compile time.
This I added only for the purpose of not affecting existing code in existing configurations. If the code is generally acceptable, then I'll be more than happy to convert it.
>Can you remove debugger_jiffies please?
>The code has to handle long delays anyways (e.g. if someone uses a target
>probe), so we cannot rely on such hacks anyways.
As above, I introduced this only to not affect existing code. If the added latency is no problem, then of course only the overflow safe code path should be kept, and then debugger_jiffies is completely unnecessary.
>I don't quite understand why the SMP case should be different from UP
>in that ifdef. Can you explain? It shouldn't in theory.
>
>
>/* When the TSC gets reset during AP startup, the code below would
>+ incorrectly think we lost a huge amount of ticks. */
>That is outdated - the TSCs are not reset anymore since 2.6.12.
>Please remove code for handling that.
Good to know, I didn't notice that. The conditional was so that the reset TSC would be considered a rolled-over one, resulting in a huge number of lost ticks.
>The union in vxtime_data is ugly - can it be avoided?
Yes, if the whole code is to become overflow-safe, then it'll just need to be a 64-bit field.
Jan
On Thu, Nov 10, 2005 at 02:19:03PM +0100, Andi Kleen wrote:
> Please remove the ifdefs too. 64bit HPET support would be fine, but
> only as a runtime mechanism, not compile time.
>
> Can you remove debugger_jiffies please?
> The code has to handle long delays anyways (e.g. if someone uses a target
> probe), so we cannot rely on such hacks anyways.
>
> I don't quite understand why the SMP case should be different from UP
> in that ifdef. Can you explain? It shouldn't in theory.
>
>
> /* When the TSC gets reset during AP startup, the code below would
> + incorrectly think we lost a huge amount of ticks. */
> That is outdated - the TSCs are not reset anymore since 2.6.12.
> Please remove code for handling that.
>
> The union in vxtime_data is ugly - can it be avoided?
>
> Vojtech should probably review that one too when you repost.
I'd like to take a look at the patch as it is, but it seems my spam
filter ate it, and LKML.org doesn't archive binary attachments.
I've done some work on making x86-64 time handling overflow save in the
last few days, so I'm quite interested in what you needed to change.
--
Vojtech Pavlik
SuSE Labs, SuSE CR
On Thursday 10 November 2005 15:23, Jan Beulich wrote:
>
> >Please remove the ifdefs too. 64bit HPET support would be fine, but
> >only as a runtime mechanism, not compile time.
>
> This I added only for the purpose of not affecting existing code in
> existing configurations. If the code is generally acceptable, then I'll be
> more than happy to convert it.
We can't use 64bit HPET everywhere because quite some chipsets only
support 32bit HPET. So it has to be a runtime switch depending on the
capabilities of the hardware.
>
> >Can you remove debugger_jiffies please?
> >The code has to handle long delays anyways (e.g. if someone uses a target
> >probe), so we cannot rely on such hacks anyways.
>
> As above, I introduced this only to not affect existing code. If the added
> latency is no problem, then of course only the overflow safe code path
> should be kept, and then debugger_jiffies is completely unnecessary.
Hmm - didn't notice anything particularly slow. Or what were you thinking
about regarding the latency? And it should only run at each timer interrupt,
so it isn't a fast path. So I guess it's best to run it always.
-Andi
On Fri, Nov 11, 2005 at 03:12:15AM +0100, Andi Kleen wrote:
> On Thursday 10 November 2005 15:23, Jan Beulich wrote:
>
> >
> > >Please remove the ifdefs too. 64bit HPET support would be fine, but
> > >only as a runtime mechanism, not compile time.
> >
> > This I added only for the purpose of not affecting existing code in
> > existing configurations. If the code is generally acceptable, then I'll be
> > more than happy to convert it.
>
> We can't use 64bit HPET everywhere because quite some chipsets only
> support 32bit HPET. So it has to be a runtime switch depending on the
> capabilities of the hardware.
Is there any advantage to using 64-bit HPET? It's read is even slower
(and thus even more unusable in a vsyscall than a 32-bit HPET read), and
the missing 32 bits can be quite easily added should we need them for
computations, which I doubt.
> > >Can you remove debugger_jiffies please?
> > >The code has to handle long delays anyways (e.g. if someone uses a target
> > >probe), so we cannot rely on such hacks anyways.
> >
> > As above, I introduced this only to not affect existing code. If the added
> > latency is no problem, then of course only the overflow safe code path
> > should be kept, and then debugger_jiffies is completely unnecessary.
>
> Hmm - didn't notice anything particularly slow. Or what were you thinking
> about regarding the latency? And it should only run at each timer interrupt,
> so it isn't a fast path. So I guess it's best to run it always.
--
Vojtech Pavlik
SuSE Labs, SuSE CR
On Saturday 12 November 2005 10:22, Vojtech Pavlik wrote:
> Is there any advantage to using 64-bit HPET?
Yes - it can tolerate long delays between ticks, e.g. caused by noidletick /
debuggers / target probes / smm etc. At least the first case will be fairly
important soon.
> It's read is even slower
Why? The read should be on cache line granuality and there shouldn't
be any difference in theory.
-Andi
On Sat, Nov 12, 2005 at 06:21:11PM +0100, Andi Kleen wrote:
> On Saturday 12 November 2005 10:22, Vojtech Pavlik wrote:
>
> > Is there any advantage to using 64-bit HPET?
>
> Yes - it can tolerate long delays between ticks, e.g. caused by noidletick /
> debuggers / target probes / smm etc. At least the first case will be fairly
> important soon.
A 32-bit 14 MHz HPET counter will overflow in approximately 5 minutes. I
don't think going 64-bit makes sense for noidletick, but for debuggers,
etc, it could make a good sense indeed.
> > It's read is even slower
>
> Why? The read should be on cache line granuality and there shouldn't
> be any difference in theory.
I'll try to measure this. Indeed, in theory there shouldn't be a
significant difference.
--
Vojtech Pavlik
SuSE Labs, SuSE CR
Vojtech Pavlik wrote:
> On Sat, Nov 12, 2005 at 06:21:11PM +0100, Andi Kleen wrote:
>
>>On Saturday 12 November 2005 10:22, Vojtech Pavlik wrote:
>>
>>
>>>Is there any advantage to using 64-bit HPET?
>>
>>Yes - it can tolerate long delays between ticks, e.g. caused by noidletick /
>>debuggers / target probes / smm etc. At least the first case will be fairly
>>important soon.
>
>
> A 32-bit 14 MHz HPET counter will overflow in approximately 5 minutes. I
> don't think going 64-bit makes sense for noidletick, but for debuggers,
> etc, it could make a good sense indeed.
>
>
>>>It's read is even slower
>>
>>Why? The read should be on cache line granuality and there shouldn't
>>be any difference in theory.
>
>
> I'll try to measure this. Indeed, in theory there shouldn't be a
> significant difference.
>
Doesn't this depend on the atomic nature of the 64-bit read? If it is really two 32-bit reads one
would need to do extra work to make sure the two parts belong together.
--
George Anzinger [email protected]
HRT (High-res-timers): http://sourceforge.net/projects/high-res-timers/
On Tuesday 15 November 2005 01:38, George Anzinger wrote:
> Doesn't this depend on the atomic nature of the 64-bit read? If it is really two 32-bit reads one
> would need to do extra work to make sure the two parts belong together.
Please take a look at the Subject.
-Andi
On Tue, Nov 15, 2005 at 02:05:24AM +0100, Andi Kleen wrote:
> On Tuesday 15 November 2005 01:38, George Anzinger wrote:
>
> > Doesn't this depend on the atomic nature of the 64-bit read? If it
> > is really two 32-bit reads one would need to do extra work to make
> > sure the two parts belong together.
>
> Please take a look at the Subject.
Still, the HPET doesn't necessarily have to be a 64-bit device. At least
on AMD systems, where it's implemented in AMD8111 Thor, it's bundled
together with other, 32-bit PCI devices like USB. On the other hand, the
8111 HPET doesn't implement a 64-bit conter, and it's likely that the
Intel implementation in the northbridges (or *CH, as Intel prefers
calling the things) is likely native 64-bit.
--
Vojtech Pavlik
SuSE Labs, SuSE CR
>>> Vojtech Pavlik <[email protected]> 15.11.05 08:50:26 >>>
>On Tue, Nov 15, 2005 at 02:05:24AM +0100, Andi Kleen wrote:
>
>> On Tuesday 15 November 2005 01:38, George Anzinger wrote:
>>
>> > Doesn't this depend on the atomic nature of the 64-bit read? If
it
>> > is really two 32-bit reads one would need to do extra work to
make
>> > sure the two parts belong together.
>>
>> Please take a look at the Subject.
>
>Still, the HPET doesn't necessarily have to be a 64-bit device. At
least
>on AMD systems, where it's implemented in AMD8111 Thor, it's bundled
>together with other, 32-bit PCI devices like USB. On the other hand,
the
>8111 HPET doesn't implement a 64-bit conter, and it's likely that the
>Intel implementation in the northbridges (or *CH, as Intel prefers
>calling the things) is likely native 64-bit.
And even then there is no guarantee there isn't something somewhere
converting the 64-bit to pairs of 32-bit accesses. Thus one would in
fact need extra information about the platform to know whether the
entire bus path is 64-bit in order to safely do 64-bit accesses.
However, the changes as I proposed them (and will resubmit hopefully
later today with the adjustments requested by Andi) do intentionally not
do any 64-bit reads, they only use 64-bit writes when resetting counter
or comparator (where all that's needed from the platform is that the
writes to the low 32 bits are done first, which x86 architecturally
guarantees).
Jan