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, tkreuzer(a)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/h…
>  ==============================================================================
>  --- 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/h…
>  ==============================================================================
>  --- 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(a)reactos.org)
>   *                  Eric Kohl (ekohl(a)abo.rhein-zeitung.de)
>  + *                  Timo Kreuzer (timo.kreuzer(a)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/h…
>  ==============================================================================
>  --- 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