Re: [ros-dev] [ros-diffs] [tkreuzer] 44840: [HAL] Replace the asm implementations of HalpAcquireSystemHardwareSpinLock and HalpReleaseCmosSpinLock with C implementations. The old ones didn't work on SMP, as they were only compiled once as UP.

[Alex Ionescu]

... You don't need them on SMP... sigh.

Please stop replacing code when you have NO idea what you're doing!!!

On 2009-12-31, at 6:45 PM, [e-mail] wrote:
> Author: tkreuzer
> Date: Fri Jan  1 00:45:36 2010
> New Revision: 44840
>
> URL: http://svn.reactos.org/svn/reactos?rev=44840&view=rev
> Log:
> [HAL]
> Replace the asm implementations of HalpAcquireSystemHardwareSpinLock and HalpReleaseCmosSpinLock with C implementations. The old ones didn't work on SMP, as they were only compiled once as UP.
>
> Modified:
>    branches/ros-amd64-bringup/reactos/hal/halx86/generic/amd64/systimer.S
>    branches/ros-amd64-bringup/reactos/hal/halx86/generic/cmos.c
>    branches/ros-amd64-bringup/reactos/hal/halx86/generic/i386/systimer.S
>
> Modified: branches/ros-amd64-bringup/reactos/hal/halx86/generic/amd64/systimer.S
> URL: http://svn.reactos.org/svn/reactos/branches/ros-amd64-bringup/reactos/hal/ha...
> ==============================================================================
> --- branches/ros-amd64-bringup/reactos/hal/halx86/generic/amd64/systimer.S [iso-8859-1] (original)
> +++ branches/ros-amd64-bringup/reactos/hal/halx86/generic/amd64/systimer.S [iso-8859-1] Fri Jan  1 00:45:36 2010
> @@ -14,9 +14,6 @@
>
> .data
>
> -_UnhandledMsg:
> .asciz "\n\x7\x7!!! Unhandled or Unexpected Code at line: %lx!!!\n"
>
> .global _MsgUnimplemented
> _MsgUnimplemented:
> .asciz "WARNING:  %s at %s:%d is UNIMPLEMENTED!\n"
> @@ -26,18 +23,6 @@
>
> .text
> .code64
>
> -.global _HalpReleaseCmosSpinLock
> -.func HalpReleaseCmosSpinLock
> -_HalpReleaseCmosSpinLock:
>
> -.endfunc
>
> -.global _HalpAcquireSystemHardwareSpinLock
> -.func HalpAcquireSystemHardwareSpinLock
> -_HalpAcquireSystemHardwareSpinLock:
>
> -.endfunc
>
> .global _HalpCalibrateStallExecution@0
> .func HalpCalibrateStallExecution@0
>
> Modified: branches/ros-amd64-bringup/reactos/hal/halx86/generic/cmos.c
> URL: http://svn.reactos.org/svn/reactos/branches/ros-amd64-bringup/reactos/hal/ha...
> ==============================================================================
> --- branches/ros-amd64-bringup/reactos/hal/halx86/generic/cmos.c [iso-8859-1] (original)
> +++ branches/ros-amd64-bringup/reactos/hal/halx86/generic/cmos.c [iso-8859-1] Fri Jan  1 00:45:36 2010
> @@ -5,6 +5,7 @@
> PURPOSE:         CMOS Access Routines (Real Time Clock and LastKnownGood)
> PROGRAMMERS:     Alex Ionescu ([e-mail])
>              Eric Kohl ([e-mail])
>              Timo Kreuzer ([e-mail])
>
> */
>
> /* INCLUDES ******************************************************************/
> @@ -17,8 +18,50 @@
>
> KSPIN_LOCK HalpSystemHardwareLock;
> UCHAR HalpCmosCenturyOffset;
> +ULONG HalpSystemHardwareFlags;
>
> /* PRIVATE FUNCTIONS *********************************************************/
>
> +VOID
> +NTAPI
> +HalpAcquireSystemHardwareSpinLock(VOID)
> +{
> ULONG Flags;
> /* Get flags and disable interrupts */
> Flags = __readeflags();
> _disable();
> /* Try to acquire the lock */
> while (InterlockedBitTestAndSet((PLONG)&HalpSystemHardwareLock, 0))
> {
>    /* Lock is held, short wait and try again */
>    YieldProcessor();
> }
> /* We have the lock, save the flags now */
> HalpSystemHardwareFlags = Flags;
>
> +}
>
> +VOID
> +NTAPI
> +HalpReleaseCmosSpinLock(VOID)
> +{
> ULONG Flags;
> /* Get the flags */
> Flags = HalpSystemHardwareFlags;
> /* Release lock and check if we owned it */
> if (!InterlockedBitTestAndReset((PLONG)&HalpSystemHardwareLock, 0))
> {
>    /* The spin lock was not owned! */
>    KeBugCheckEx(SPIN_LOCK_NOT_OWNED, 0, 0, 0, 0);
> }
> /* Restore the flags */
> __writeeflags(Flags);
>
> +}
>
> FORCEINLINE
> UCHAR
>
> Modified: branches/ros-amd64-bringup/reactos/hal/halx86/generic/i386/systimer.S
> URL: http://svn.reactos.org/svn/reactos/branches/ros-amd64-bringup/reactos/hal/ha...
> ==============================================================================
> --- branches/ros-amd64-bringup/reactos/hal/halx86/generic/i386/systimer.S [iso-8859-1] (original)
> +++ branches/ros-amd64-bringup/reactos/hal/halx86/generic/i386/systimer.S [iso-8859-1] Fri Jan  1 00:45:36 2010
> @@ -17,82 +17,11 @@
> _HalpLastPerfCounterHigh:   .long 0
> _HalpPerfCounterLow:        .long 0
> _HalpPerfCounterHigh:       .long 0
> -_HalpSystemHardwareFlags:   .long 0
>
> _UnhandledMsg:
>     .asciz "\n\x7\x7!!! Unhandled or Unexpected Code at line: %lx!!!\n"
>
> /* FUNCTIONS *****************************************************************/
>
> -.global _HalpReleaseCmosSpinLock@0
> -.func HalpReleaseCmosSpinLock@0
> -_HalpReleaseCmosSpinLock@0:
>
> -#ifdef CONFIG_SMP
> /* Save clobbered register */
> push eax
> /* Push saved EFLAGS */
> push _HalpSystemHardwareFlags
> /* Release the lock */
> lea eax, _HalpSystemHardwareLock
> RELEASE_SPINLOCK(eax)
> /* Restore EFLAGS */
> popf
> /* Return */
> pop eax
> ret
>
> -#else
> /* Restore EFLAGS and return */
> push _HalpSystemHardwareFlags
> popf
> ret
>
> -#endif
>
> -.endfunc
>
> -.global _HalpAcquireSystemHardwareSpinLock@0
> -.func HalpAcquireSystemHardwareSpinLock@0
> -_HalpAcquireSystemHardwareSpinLock@0:
>
> -#ifdef CONFIG_SMP
> /* Save clobbered register */
> push eax
>
> -HardwareLock:
> /* Save EFLAGS and disable interrupts */
> pushf
> cli
> /* This is the CMOS lock, acquire it */
> lea eax, _HalpSystemHardwareLock
> ACQUIRE_SPINLOCK(eax, CmosSpin)
> /* We have it, return the flags */
> pop _HalpSystemHardwareFlags
> pop eax
> ret
>
> -CmosSpin:
> /* Restore EFLAGS */
> pushf _HalpSystemHardwareLock
> popf
> /* Spin */
> SPIN_ON_LOCK(eax, HardwareLock)
>
> -#else
> /* Save EFLAGS, disable interrupts and return */
> pushf
> cli
> pop _HalpSystemHardwareFlags
> ret
>
> -#endif
>
> -.endfunc
>
> .global _HalpCalibrateStallExecution@0
> .func HalpCalibrateStallExecution@0

Best regards,
Alex Ionescu