[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, tkreuzer at svn.reactos.org 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/halx86/generic/amd64/systimer.S?rev=44840&r1=44839&r2=44840&view=diff
> ==============================================================================
> --- 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 at 0
> .func HalpCalibrateStallExecution at 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/halx86/generic/cmos.c?rev=44840&r1=44839&r2=44840&view=diff
> ==============================================================================
> --- 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 (alex.ionescu at reactos.org)
>  *                  Eric Kohl (ekohl at abo.rhein-zeitung.de)
> + *                  Timo Kreuzer (timo.kreuzer at reactos.org)
>  */
> 
> /* 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/halx86/generic/i386/systimer.S?rev=44840&r1=44839&r2=44840&view=diff
> ==============================================================================
> --- 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 at 0
> -.func HalpReleaseCmosSpinLock at 0
> -_HalpReleaseCmosSpinLock at 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 at 0
> -.func HalpAcquireSystemHardwareSpinLock at 0
> -_HalpAcquireSystemHardwareSpinLock at 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 at 0
> .func HalpCalibrateStallExecution at 0
> 
> 

Best regards,
Alex Ionescu