http://defect.opensolaris.org/bz/show_bug.cgi?id=6499





--- Comment #6 from Bill Holler <bill.holler at sun.com>  2009-02-13 18:46:57 
---
(In reply to comment #2)
> More feedback:
> 
> cpu_idle.c:
>          * start the LAPIC Timer agin before leaving this function.
> again

done


> hpet_acpi.c:
> 
> hpet_init_proxys()
> proxies
> 
> As we discussed on the phone, something in a "top-of-file" comment about
> 1) HPET interrupt doesn't cause interrupt on the CPU, but rather "wake from
> HLT", until the "code after HLT" reenables IF, and 2) one cpu is tasked with
> managing HPET wakeups for all C-stated CPUs

done.  Big block comment placed in hpet_acpi.c which includes this info.


> Presumably apic_regops.c should not be in this wad?

This file will be removed from the changeset.


> cpuid.c:
> 
> 1) we really weren't looking for CPUID_TSC_INVARIANCE before?  Doesn't Mark
> look for that for p-state support?

No.  C-state and P-state Invariance are two seperate CPU properties.
CPUs can have any combination of these properties.
P-states used family/model to determine P-state invariant TSC.

For example Intel Penryn processors have P-state invariant TSC and
C-state variant TSC.

The define what changed to include CSTATE in the name, so this will be
more clear.


> 2) TSC_INVARIANCE applies to C-, P-, and T- state TSC invariance; the comment
> should note that

As above: C-state and P-state Invariance are two seperate CPU properties.
Comment changed to be more C-state specific.


> 3) why is the #define here, and not in, say, x86_archext.h?

done.  Moved into x86_archext.h


> cpu_idle.c
> 
>  211          * Set our mcpu_mwait here, so we can tell if anyone trys to
> tries

done


>  223          * If this CPU is online, and there's multiple CPUs
> there are

done


>  224          * in the system, then we should notate our halting
> note

done


>  238          * (if anyone) should be awoken. We therefore need to first
> I would say "awakened", but I see both in online dictionaries.  I dunno.

done


>  274          * start the LAPIC Timer agin before leaving this function.
> "again" again.  You might want to global-search for 'agin'.

done


>  350         /* DTRACE_PROBE1(idle__state__transition, uint_t,
> (uint_t)cs_type); */
> Is this an old version of the probe, or?...(ah, see mp_machdep.c below)

done.  Bringup code deleted.


> mp_machdep.c:
> I don't these are in your wad, but I notice
>          * Set our mcpu_mwait here, so we can tell if anyone trys to
> tries

done


>          * in the system, then we should notate our halting
> note

done


>          * (if anyone) should be awoken. We therefore need to first
> awakened

done


> I sense a theme :)
> 
> In your changes:
> 
> + * XXX: for now this in not guarded from xpv.
>  
> Why not?  and should it be before putback?

done.  Moved into guard.  This was discussed with XEN team a while back.
Guarding C-state implementation from xpv is prefered.


> XXX_highbit looks vestigial; presumably lint catches this

Done.  Bringup code deleted.


> + * XXX: DTRACE_PROBE1 did not work in acpi_cpu_idle which is linked into
> 
> ah, now I understand line 350 in cpu_idle.c above

done.  Removed.


> Can cstate_wakeup() take over from cpu_wakeup()?

We do not know what AMD will do with C-state support.

-- 
Configure bugmail: http://defect.opensolaris.org/bz/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

Reply via email to