If I recall correctly, one of the commits that followed this one also added
ValidKernelPteLocal and ValidKernelPdeLocal, which are used for PTEs/PDEs
that shouldn't be global.

Regards,
Alex

On Thu, Oct 15, 2015 at 06:55:51PM -0700, Alex Ionescu wrote:
> This is wrong, wrong, wrong.
>
> 1) The feature needs to be enabled on all processors, not just one. There's
> a reason for that loop.
>
> 2) ValidKernelPte/Pde being marked global will make ALL kernel pages
> global. This is probably not what you want.
>
> Best regards,
> Alex Ionescu
>
> On Wed, Oct 14, 2015 at 12:33 PM, <sginsberg@svn.reactos.org> wrote:
>
> > Author: sginsberg
> > Date: Wed Oct 14 19:33:35 2015
> > New Revision: 69528
> >
> > URL: http://svn.reactos.org/svn/reactos?rev=69528&view=rev
> > Log:
> > [NTOS]
> > Add super-complicated handling of global pages to KeFlushCurrentTb (pretty
> > much the same code which has been in HalpFlushTLB for the past ~6 years).
> > This should be all that is required to make this feature work (everything
> > else being in place already), and *seems* to work fine but is disabled
> > under a switch until tested thoroughly.
> >
> > Global pages, an important optimization that allows for not flushing the
> > whole x86 TLB every time CR3 is changed (typically on context switch to a
> > new process, or during process attach/detach), relies on us doing extra
> > work whenever we do alter a global page. This is likely where any bugs will
> > have to be flushed out!
> >
> > Fixup Ki386EnableGlobalPage while we are at it -- disable/restore
> > interrupts properly, and verify PGE-bit isn't set (nothing should have
> > touched it before this routine, which is responsible for initializing it,
> > so we shouldn't have to disable it). Fix, but disable, the CPU-sync spin as
> > well as there should be no particular reason to do this for PGE-enabling
> > during initialization (no other processor will be messing with PTEs at this
> > stage, as compared to a call to KeFlushEntireTb).
> >
> > Everyone, repeat after me: Global pages are awesome!
> >
> > Modified:
> >     trunk/reactos/ntoskrnl/include/ntoskrnl.h
> >     trunk/reactos/ntoskrnl/ke/i386/cpu.c
> >     trunk/reactos/ntoskrnl/ke/i386/patpge.c
> >     trunk/reactos/ntoskrnl/mm/ARM3/i386/init.c
> >
> > Modified: trunk/reactos/ntoskrnl/include/ntoskrnl.h
> > URL:
> > http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/include/ntoskrnl.h?rev=69528&r1=69527&r2=69528&view=diff
> >
> > ==============================================================================
> > --- trunk/reactos/ntoskrnl/include/ntoskrnl.h   [iso-8859-1] (original)
> > +++ trunk/reactos/ntoskrnl/include/ntoskrnl.h   [iso-8859-1] Wed Oct 14
> > 19:33:35 2015
> > @@ -98,6 +98,14 @@
> >  #define ASSERT NT_ASSERT
> >  #endif
> >
> > +
> > +//
> > +// Switch for enabling global page support
> > +//
> > +
> > +//#define _GLOBAL_PAGES_ARE_AWESOME_
> > +
> > +
> >  /* Internal Headers */
> >  #include "internal/ntoskrnl.h"
> >  #include "config.h"
> >
> > Modified: trunk/reactos/ntoskrnl/ke/i386/cpu.c
> > URL:
> > http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ke/i386/cpu.c?rev=69528&r1=69527&r2=69528&view=diff
> >
> > ==============================================================================
> > --- trunk/reactos/ntoskrnl/ke/i386/cpu.c        [iso-8859-1] (original)
> > +++ trunk/reactos/ntoskrnl/ke/i386/cpu.c        [iso-8859-1] Wed Oct 14
> > 19:33:35 2015
> > @@ -878,8 +878,37 @@
> >  NTAPI
> >  KeFlushCurrentTb(VOID)
> >  {
> > +
> > +#if !defined(_GLOBAL_PAGES_ARE_AWESOME_)
> > +
> >      /* Flush the TLB by resetting CR3 */
> >      __writecr3(__readcr3());
> > +
> > +#else
> > +
> > +    /* Check if global pages are enabled */
> > +    if (KeFeatureBits & KF_GLOBAL_PAGE)
> > +    {
> > +        ULONG Cr4;
> > +
> > +        /* Disable PGE */
> > +        Cr4 = __readcr4() & ~CR4_PGE;
> > +        __writecr4(Cr4);
> > +
> > +        /* Flush everything */
> > +        __writecr3(__readcr3());
> > +
> > +        /* Re-enable PGE */
> > +        __writecr4(Cr4 | CR4_PGE);
> > +    }
> > +    else
> > +    {
> > +        /* No global pages, resetting CR3 is enough */
> > +        __writecr3(__readcr3());
> > +    }
> > +
> > +#endif
> > +
> >  }
> >
> >  VOID
> >
> > Modified: trunk/reactos/ntoskrnl/ke/i386/patpge.c
> > URL:
> > http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ke/i386/patpge.c?rev=69528&r1=69527&r2=69528&view=diff
> >
> > ==============================================================================
> > --- trunk/reactos/ntoskrnl/ke/i386/patpge.c     [iso-8859-1] (original)
> > +++ trunk/reactos/ntoskrnl/ke/i386/patpge.c     [iso-8859-1] Wed Oct 14
> > 19:33:35 2015
> > @@ -17,40 +17,41 @@
> >
> >  /* FUNCTIONS
> > *****************************************************************/
> >
> > +INIT_SECTION
> >  ULONG_PTR
> >  NTAPI
> > -INIT_FUNCTION
> >  Ki386EnableGlobalPage(IN ULONG_PTR Context)
> >  {
> > -    PLONG Count = (PLONG)Context;
> > -    ULONG Cr4, Cr3;
> > +    //PLONG Count;
> > +#if defined(_GLOBAL_PAGES_ARE_AWESOME_)
> > +    ULONG Cr4;
> > +#endif
> > +    BOOLEAN Enable;
> >
> >      /* Disable interrupts */
> > -    _disable();
> > -
> > -    /* Decrease CPU Count and loop until it's reached 0 */
> > -    do {InterlockedDecrement(Count);} while (!*Count);
> > -
> > -    /* Now check if this is the Boot CPU */
> > -    if (!KeGetPcr()->Number)
> > -    {
> > -        /* It is.FIXME: Patch KeFlushCurrentTb */
> > -    }
> > -
> > -    /* Now get CR4 and make sure PGE is masked out */
> > +    Enable = KeDisableInterrupts();
> > +
> > +    /* Spin until other processors are ready */
> > +    //Count = (PLONG)Context;
> > +    //InterlockedDecrement(Count);
> > +    //while (*Count) YieldProcessor();
> > +
> > +#if defined(_GLOBAL_PAGES_ARE_AWESOME_)
> > +
> > +    /* Get CR4 and ensure global pages are disabled */
> >      Cr4 = __readcr4();
> > -    __writecr4(Cr4 & ~CR4_PGE);
> > -
> > -    /* Flush the TLB */
> > -    Cr3 = __readcr3();
> > -    __writecr3(Cr3);
> > +    ASSERT(!(Cr4 & CR4_PGE));
> > +
> > +    /* Reset CR3 to flush the TLB */
> > +    __writecr3(__readcr3());
> >
> >      /* Now enable PGE */
> > -    DPRINT("Global page support detected but not yet taken advantage
> > of\n");
> > -    //__writecr4(Cr4 | CR4_PGE);
> > -
> > -    /* Restore interrupts */
> > -    _enable();
> > +    __writecr4(Cr4 | CR4_PGE);
> > +
> > +#endif
> > +
> > +    /* Restore interrupts and return */
> > +    KeRestoreInterrupts(Enable);
> >      return 0;
> >  }
> >
> >
> > Modified: trunk/reactos/ntoskrnl/mm/ARM3/i386/init.c
> > URL:
> > http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/ARM3/i386/init.c?rev=69528&r1=69527&r2=69528&view=diff
> >
> > ==============================================================================
> > --- trunk/reactos/ntoskrnl/mm/ARM3/i386/init.c  [iso-8859-1] (original)
> > +++ trunk/reactos/ntoskrnl/mm/ARM3/i386/init.c  [iso-8859-1] Wed Oct 14
> > 19:33:35 2015
> > @@ -249,15 +249,18 @@
> >      PMMPFN Pfn1;
> >      ULONG Flags;
> >
> > +#if defined(_GLOBAL_PAGES_ARE_AWESOME_)
> > +
> >      /* Check for global bit */
> > -#if 0
> >      if (KeFeatureBits & KF_GLOBAL_PAGE)
> >      {
> >          /* Set it on the template PTE and PDE */
> >          ValidKernelPte.u.Hard.Global = TRUE;
> >          ValidKernelPde.u.Hard.Global = TRUE;
> >      }
> > +
> >  #endif
> > +
> >      /* Now templates are ready */
> >      TempPte = ValidKernelPte;
> >      TempPde = ValidKernelPde;
> >
> >
> >

> _______________________________________________
> Ros-dev mailing list
> Ros-dev@reactos.org
> http://www.reactos.org/mailman/listinfo/ros-dev


--
Alexander Andrejevic <theflash@sdf.lonestar.org>
SDF Public Access UNIX System - http://sdf.lonestar.org

_______________________________________________
Ros-dev mailing list
Ros-dev@reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev