Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

X64 boot fixes #115

Closed
wants to merge 21 commits into from
Closed
Show file tree
Hide file tree
Changes from 17 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion drivers/sac/driver/init.c
Expand Up @@ -18,7 +18,7 @@ DriverEntry(IN PDRIVER_OBJECT DriverObject,
IN PUNICODE_STRING RegistryPath)
{
HEADLESS_RSP_QUERY_INFO HeadlessInformation;
ULONG InfoSize = sizeof(HeadlessInformation);
SIZE_T InfoSize = sizeof(HeadlessInformation);
NTSTATUS Status;
UNICODE_STRING DriverName;
PDEVICE_OBJECT DeviceObject;
Expand Down
55 changes: 30 additions & 25 deletions ntoskrnl/ex/handle.c
Expand Up @@ -18,6 +18,8 @@
LIST_ENTRY HandleTableListHead;
EX_PUSH_LOCK HandleTableListLock;
#define SizeOfHandle(x) (sizeof(HANDLE) * (x))
#define MAKE_HANDLE(x) ((x) << 2)
#define CLEAR_HANDLE(x) MAKE_HANDLE(x.Index)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this defined like that?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, you a right, it's a bit unclear. Probably the best way to write this will be

#define MAKE_HANDLE(x) ((x) << HANDLE_TAG_BITS)

so it will be more obvious.

BTW, the error with handles was quite interesting: it's one of those "32-bit chauvinism" cases, there the code was working just by chance of (sizeof(HANDLE) * (x)) == (x << HANDLE_TAG_BITS). As I can see, there is a whole bunch of similar cases in ReactOS code...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually my point was more about the "CLEAR_HANDLE" definition :P


/* PRIVATE FUNCTIONS *********************************************************/

Expand Down Expand Up @@ -67,12 +69,14 @@ ExpLookupHandleTableEntry(IN PHANDLE_TABLE HandleTable,

/* Get the mid level pointer array */
PointerArray = PointerArray[Handle.HighIndex];
ASSERT(PointerArray == NULL);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean !=?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, probably. Will look into that later


/* Fall through */
case 1:

/* Get the handle array */
HandleArray = PointerArray[Handle.MidIndex];
ASSERT(HandleArray == NULL);

/* Fall through */
case 0:
Expand Down Expand Up @@ -267,13 +271,13 @@ ExpFreeHandleTableEntry(IN PHANDLE_TABLE HandleTable,
InterlockedDecrement(&HandleTable->HandleCount);

/* Mark the handle as free */
NewValue = (ULONG)Handle.Value & ~(SizeOfHandle(1) - 1);
NewValue = (ULONG)CLEAR_HANDLE(Handle);

/* Check if we're FIFO */
if (!HandleTable->StrictFIFO)
{
/* Select a lock index */
i = (NewValue >> 2) % 4;
i = Handle.Index % 4;

/* Select which entry to use */
Free = (HandleTable->HandleTableLock[i].Locked) ?
Expand Down Expand Up @@ -354,7 +358,7 @@ ExpAllocateHandleTable(IN PEPROCESS Process OPTIONAL,
{
/* Set up the free data */
HandleEntry->Value = 0;
HandleEntry->NextFreeTableEntry = (i + 1) * SizeOfHandle(1);
HandleEntry->NextFreeTableEntry = MAKE_HANDLE(i + 1);

/* Move to the next entry */
HandleEntry++;
Expand All @@ -363,11 +367,11 @@ ExpAllocateHandleTable(IN PEPROCESS Process OPTIONAL,
/* Terminate the last entry */
HandleEntry->Value = 0;
HandleEntry->NextFreeTableEntry = 0;
HandleTable->FirstFree = SizeOfHandle(1);
HandleTable->FirstFree = MAKE_HANDLE(1);
}

/* Set the next handle needing pool after our allocated page from above */
HandleTable->NextHandleNeedingPool = LOW_LEVEL_ENTRIES * SizeOfHandle(1);
HandleTable->NextHandleNeedingPool = MAKE_HANDLE(LOW_LEVEL_ENTRIES);

/* Setup the rest of the handle table data */
HandleTable->QuotaProcess = Process;
Expand Down Expand Up @@ -409,12 +413,12 @@ ExpAllocateLowLevelTable(IN PHANDLE_TABLE HandleTable,
{
/* Go to the next entry and the base entry */
HandleEntry++;
Base = HandleTable->NextHandleNeedingPool + SizeOfHandle(2);
Base = HandleTable->NextHandleNeedingPool + MAKE_HANDLE(2);

/* Loop each entry */
for (i = Base;
i < Base + SizeOfHandle(LOW_LEVEL_ENTRIES - 2);
i += SizeOfHandle(1))
i < Base + MAKE_HANDLE(LOW_LEVEL_ENTRIES - 2);
i += MAKE_HANDLE(1))
{
/* Free this entry and move on to the next one */
HandleEntry->NextFreeTableEntry = i;
Expand Down Expand Up @@ -494,7 +498,7 @@ ExpAllocateHandleTableEntrySlow(IN PHANDLE_TABLE HandleTable,

/* Get if the next index can fit in the table */
i = HandleTable->NextHandleNeedingPool /
SizeOfHandle(LOW_LEVEL_ENTRIES);
MAKE_HANDLE(LOW_LEVEL_ENTRIES);
if (i < MID_LEVEL_ENTRIES)
{
/* We need to allocate a new table */
Expand Down Expand Up @@ -539,7 +543,7 @@ ExpAllocateHandleTableEntrySlow(IN PHANDLE_TABLE HandleTable,
ThirdLevel = (PVOID)TableBase;

/* Get the index and check if it can fit */
i = HandleTable->NextHandleNeedingPool / SizeOfHandle(MAX_MID_INDEX);
i = HandleTable->NextHandleNeedingPool / MAKE_HANDLE(MAX_MID_INDEX);
if (i >= HIGH_LEVEL_ENTRIES) return FALSE;

/* Check if there's no mid-level table */
Expand All @@ -556,8 +560,8 @@ ExpAllocateHandleTableEntrySlow(IN PHANDLE_TABLE HandleTable,
else
{
/* We have one, check at which index we should insert our entry */
Index = (HandleTable->NextHandleNeedingPool / SizeOfHandle(1)) -
i * MAX_MID_INDEX;
Index = (HandleTable->NextHandleNeedingPool / MAKE_HANDLE(1)) -
i * MAX_MID_INDEX;
j = Index / LOW_LEVEL_ENTRIES;

/* Allocate a new low level */
Expand All @@ -577,13 +581,13 @@ ExpAllocateHandleTableEntrySlow(IN PHANDLE_TABLE HandleTable,

/* Update the index of the next handle */
Index = InterlockedExchangeAdd((PLONG) &HandleTable->NextHandleNeedingPool,
SizeOfHandle(LOW_LEVEL_ENTRIES));
MAKE_HANDLE(LOW_LEVEL_ENTRIES));

/* Check if need to initialize the table */
if (DoInit)
{
/* Create a new index number */
Index += SizeOfHandle(1);
Index += MAKE_HANDLE(1);

/* Start free index change loop */
for (;;)
Expand Down Expand Up @@ -646,7 +650,7 @@ ExpAllocateHandleTableEntry(IN PHANDLE_TABLE HandleTable,
{
ULONG OldValue, NewValue, NewValue1;
PHANDLE_TABLE_ENTRY Entry;
EXHANDLE Handle;
EXHANDLE Handle, OldHandle;
BOOLEAN Result;
ULONG i;

Expand Down Expand Up @@ -709,7 +713,8 @@ ExpAllocateHandleTableEntry(IN PHANDLE_TABLE HandleTable,
Entry = ExpLookupHandleTableEntry(HandleTable, Handle);

/* Get an available lock and acquire it */
i = ((OldValue & FREE_HANDLE_MASK) >> 2) % 4;
OldHandle.Value = OldValue;
i = OldHandle.Index % 4;
KeEnterCriticalRegion();
ExAcquirePushLockShared(&HandleTable->HandleTableLock[i]);

Expand Down Expand Up @@ -1063,7 +1068,7 @@ ExDupHandleTable(IN PEPROCESS Process,
NewTable->FirstFree = 0;

/* Setup the first handle value */
Handle.Value = SizeOfHandle(1);
Handle.Value = MAKE_HANDLE(1);

/* Enter a critical region and lookup the new entry */
KeEnterCriticalRegion();
Expand Down Expand Up @@ -1125,13 +1130,13 @@ ExDupHandleTable(IN PEPROCESS Process,
}

/* Increase the handle value and move to the next entry */
Handle.Value += SizeOfHandle(1);
Handle.Value += MAKE_HANDLE(1);
NewEntry++;
HandleTableEntry++;
} while (Handle.Value % SizeOfHandle(LOW_LEVEL_ENTRIES));
} while (Handle.Value % MAKE_HANDLE(LOW_LEVEL_ENTRIES));

/* We're done, skip the last entry */
Handle.Value += SizeOfHandle(1);
Handle.Value += MAKE_HANDLE(1);
}

/* Acquire the table lock and insert this new table into the list */
Expand Down Expand Up @@ -1198,7 +1203,7 @@ ExSweepHandleTable(IN PHANDLE_TABLE HandleTable,
PAGED_CODE();

/* Set the initial value and loop the entries */
Handle.Value = SizeOfHandle(1);
Handle.Value = MAKE_HANDLE(1);
while ((HandleTableEntry = ExpLookupHandleTableEntry(HandleTable, Handle)))
{
/* Loop each handle */
Expand All @@ -1214,12 +1219,12 @@ ExSweepHandleTable(IN PHANDLE_TABLE HandleTable,
}

/* Go to the next handle and entry */
Handle.Value += SizeOfHandle(1);
Handle.Value += MAKE_HANDLE(1);
HandleTableEntry++;
} while (Handle.Value % SizeOfHandle(LOW_LEVEL_ENTRIES));
} while (Handle.Value % MAKE_HANDLE(LOW_LEVEL_ENTRIES));

/* Skip past the last entry */
Handle.Value += SizeOfHandle(1);
Handle.Value += MAKE_HANDLE(1);
}
}

Expand Down Expand Up @@ -1271,7 +1276,7 @@ ExEnumHandleTable(IN PHANDLE_TABLE HandleTable,
}

/* Go to the next entry */
Handle.Value += SizeOfHandle(1);
Handle.Value += MAKE_HANDLE(1);
}

/* Leave the critical region and return callback result */
Expand Down
4 changes: 2 additions & 2 deletions ntoskrnl/fsrtl/stackovf.c
Expand Up @@ -115,7 +115,7 @@ FsRtlWorkerThread(IN PVOID StartContext)
KIRQL Irql;
PLIST_ENTRY Entry;
PWORK_QUEUE_ITEM WorkItem;
ULONG QueueId = (ULONG)StartContext;
ULONG_PTR QueueId = (ULONG_PTR)StartContext;

/* Set our priority according to the queue we're dealing with */
KeSetPriorityThread(&PsGetCurrentThread()->Tcb, LOW_REALTIME_PRIORITY + QueueId);
Expand Down Expand Up @@ -149,7 +149,7 @@ NTAPI
INIT_FUNCTION
FsRtlInitializeWorkerThread(VOID)
{
ULONG i;
ULONG_PTR i;
NTSTATUS Status;
HANDLE ThreadHandle;
OBJECT_ATTRIBUTES ObjectAttributes;
Expand Down
2 changes: 1 addition & 1 deletion ntoskrnl/fsrtl/unc.c
Expand Up @@ -71,7 +71,7 @@ FsRtlpIsDfsEnabled(VOID)
return TRUE;
}

return ((ULONG)KeyQueryOutput.KeyInfo.Data != 1);
return ((*(PULONG)KeyQueryOutput.KeyInfo.Data) != 1);
}

NTSTATUS
Expand Down
36 changes: 30 additions & 6 deletions ntoskrnl/include/internal/amd64/ke.h
Expand Up @@ -22,18 +22,42 @@
#define X86_CR4_OSFXSR 0x00000200 /* enable FXSAVE/FXRSTOR instructions */
#define X86_CR4_OSXMMEXCPT 0x00000400 /* enable #XF exception */

/* EDX flags */
#define X86_FEATURE_FPU 0x00000001 /* x87 FPU is present */
#define X86_FEATURE_VME 0x00000002 /* Virtual 8086 Extensions are present */
#define X86_FEATURE_DBG 0x00000004 /* Debugging extensions are present */
#define X86_FEATURE_PSE 0x00000008 /* Page Size Extension is present */
#define X86_FEATURE_TSC 0x00000010 /* time stamp counters are present */
#define X86_FEATURE_PAE 0x00000040 /* physical address extension is present */
#define X86_FEATURE_CX8 0x00000100 /* CMPXCHG8B instruction present */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like these are not even used and all the values are hardcoded in KiGetFeatureBits. Feel free to improve that. Or not :)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simple enough, I think I will do it then. Readablility of code is important.

#define X86_FEATURE_SYSCALL 0x00000800 /* SYSCALL/SYSRET support present */
#define X86_FEATURE_MTTR 0x00001000 /* Memory type range registers are present */
#define X86_FEATURE_PGE 0x00002000 /* Page Global Enable */
#define X86_FEATURE_CMOV 0x00008000 /* "Conditional move" instruction supported */
#define X86_FEATURE_PAT 0x00010000 /* Page Attribute Table is supported */
#define X86_FEATURE_DS 0x00200000 /* Debug Store is present */
#define X86_FEATURE_MMX 0x00800000 /* MMX extension present */
#define X86_FEATURE_FXSR 0x01000000 /* FXSAVE/FXRSTOR instructions present */
#define X86_FEATURE_SSE 0x02000000 /* SSE extension present */
#define X86_FEATURE_SSE2 0x04000000 /* SSE2 extension present */
#define X86_FEATURE_HT 0x10000000 /* Hyper-Threading present */

/* ECX flags */
#define X86_FEATURE_SSE3 0x00000001 /* SSE3 is supported */
#define X86_FEATURE_MONITOR 0x00000008 /* SSE3 Monitor instructions supported */
#define X86_FEATURE_VMX 0x00000020 /* Virtual Machine eXtensions are available */
#define X86_FEATURE_SSSE3 0x00000200 /* Supplemental SSE3 are available */
#define X86_FEATURE_FMA3 0x00001000 /* Fused multiple-add supported */
#define X86_FEATURE_CX16 0x00002000 /* CMPXCHG16B instruction are available */
#define X86_FEATURE_PCID 0x00020000 /* Process Context IDentifiers are supported */
#define X86_FEATURE_SSE41 0x00080000 /* SSE 4.1 is supported */
#define X86_FEATURE_SSE42 0x00100000 /* SSE 4.2 is supported */
#define X86_FEATURE_POPCNT 0x00800000 /* POPCNT instruction is available */
#define X86_FEATURE_XSAVE 0x04000000 /* XSAVE family are available */

/* EDX extended flags */
#define X86_FEATURE_NX 0x00100000 /* NX support present */

#define X86_EXT_FEATURE_SSE3 0x00000001 /* SSE3 extension present */
#define X86_EXT_FEATURE_3DNOW 0x40000000 /* 3DNOW! extension present */

Expand All @@ -47,12 +71,12 @@
#define X86_MSR_CSTAR 0xC0000083
#define X86_MSR_SFMASK 0xC0000084

#define EFER_SCE 0x01
#define EFER_LME 0x10
#define EFER_LMA 0x40
#define EFER_NXE 0x80
#define EFER_SVME 0x100
#define EFER_FFXSR 0x400
#define EFER_SCE 0x0001
#define EFER_LME 0x0100
#define EFER_LMA 0x0400
#define EFER_NXE 0x0800
#define EFER_SVME 0x1000
#define EFER_FFXSR 0x4000
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch! This commit LGTM.


#define AMD64_TSS 9

Expand Down
19 changes: 11 additions & 8 deletions ntoskrnl/include/internal/ex.h
Expand Up @@ -68,22 +68,25 @@ VOID NTAPI ExpDebuggerWorker(IN PVOID Context);
#define HANDLE_LOW_BITS (PAGE_SHIFT - 3)
#define HANDLE_HIGH_BITS (PAGE_SHIFT - 2)
#endif
#define KERNEL_FLAG_BITS (sizeof(PVOID)*8 - 31)
#define HANDLE_TAG_BITS (2)
#define HANDLE_INDEX_BITS (HANDLE_LOW_BITS + 2*HANDLE_HIGH_BITS)
#define KERNEL_FLAG_BITS (sizeof(PVOID)*8 - HANDLE_INDEX_BITS - HANDLE_TAG_BITS)

typedef union _EXHANDLE
{
struct
{
ULONG_PTR TagBits:2;
ULONG_PTR Index:29;
ULONG_PTR TagBits: HANDLE_TAG_BITS;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code style: source alignment broken? (or usage of tabs instead of spaces?)

ULONG_PTR Index: HANDLE_INDEX_BITS;
ULONG_PTR KernelFlag : KERNEL_FLAG_BITS;
};
struct
{
ULONG_PTR TagBits2:2;
ULONG_PTR LowIndex:HANDLE_LOW_BITS;
ULONG_PTR MidIndex:HANDLE_HIGH_BITS;
ULONG_PTR HighIndex:HANDLE_HIGH_BITS;
ULONG_PTR KernelFlag:KERNEL_FLAG_BITS;
ULONG_PTR TagBits2: HANDLE_TAG_BITS;
ULONG_PTR LowIndex: HANDLE_LOW_BITS;
ULONG_PTR MidIndex: HANDLE_HIGH_BITS;
ULONG_PTR HighIndex: HANDLE_HIGH_BITS;
ULONG_PTR KernelFlag2: KERNEL_FLAG_BITS;
};
HANDLE GenericHandleOverlay;
ULONG_PTR Value;
Expand Down
4 changes: 2 additions & 2 deletions ntoskrnl/io/pnpmgr/pnpreport.c
Expand Up @@ -425,7 +425,7 @@ IoReportTargetDeviceChange(IN PDEVICE_OBJECT PhysicalDeviceObject,
/* Check for valid PDO */
if (!IopIsValidPhysicalDeviceObject(PhysicalDeviceObject))
{
KeBugCheckEx(PNP_DETECTED_FATAL_ERROR, 0x2, (ULONG)PhysicalDeviceObject, 0, 0);
KeBugCheckEx(PNP_DETECTED_FATAL_ERROR, 0x2, (ULONG_PTR)PhysicalDeviceObject, 0, 0);
}

/* FileObject must be null. PnP will fill in it */
Expand Down Expand Up @@ -476,7 +476,7 @@ IoReportTargetDeviceChangeAsynchronous(IN PDEVICE_OBJECT PhysicalDeviceObject,
/* Check for valid PDO */
if (!IopIsValidPhysicalDeviceObject(PhysicalDeviceObject))
{
KeBugCheckEx(PNP_DETECTED_FATAL_ERROR, 0x2, (ULONG)PhysicalDeviceObject, 0, 0);
KeBugCheckEx(PNP_DETECTED_FATAL_ERROR, 0x2, (ULONG_PTR)PhysicalDeviceObject, 0, 0);
}

/* FileObject must be null. PnP will fill in it */
Expand Down