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

[WIP] The ultimate ros amd64 bringup #361

Closed
wants to merge 54 commits into from

Conversation

tkreuzer
Copy link
Contributor

@tkreuzer tkreuzer commented Feb 5, 2018

Make ReactOS boot to user mode on x64

This fixes a large number of issues and implements missing stuff.

TODO

  • Fix resource conflict issues properly

…atePebOrTeb()

The size is in bytes, not in pages! On x86 we got away with it, since PEB and TEB require only a single page and the 1 passed to MiInsertVadEx() was aligned up to PAGE_SIZE. On x64 this doesn't work, since the size is 2 pages.
…nd FFFFF68000000000 in MiInitSystemMemoryAreas()
…mGetPageFileMapping, remove obsolete functions.
…port for win32k syscalls in KiSystemCallHandler
mov r9, [rax + PeKernelCallbackTable]

; Call the routine
call qword ptr [r9 + r8 * 8]
Copy link
Member

Choose a reason for hiding this comment

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

You need to reserve 32 bytes of stack for it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I dont't. (It's part of the callback frame that has been allocated by the kernel).

Copy link
Member

Choose a reason for hiding this comment

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

Ah, didn't see that. Sounds good.

@@ -12,7 +12,7 @@
//#define NDEBUG
#include <debug.h>

//#include "x86emu.h"
#include <fast486.h>
Copy link
Member

Choose a reason for hiding this comment

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

Whoa nice!

@@ -994,8 +994,8 @@ HalpBuildAcpiResourceList(IN PIO_RESOURCE_REQUIREMENTS_LIST ResourceList)

/* Get the interrupt number */
Interrupt = HalpPicVectorRedirect[HalpFixedAcpiDescTable.sci_int_vector];
ResourceList->List[0].Descriptors[0].u.Interrupt.MinimumVector = Interrupt;
ResourceList->List[0].Descriptors[0].u.Interrupt.MaximumVector = Interrupt;
ResourceList->List[0].Descriptors[0].u.Interrupt.MinimumVector = Interrupt + PRIMARY_VECTOR_BASE;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is a hack that I applied, which ensures that at least HAL is loaded properly and doesn't have any resource conflicts. I have doubts that what you claim about untranslated resources is true, since you can see in the registry on Windows that e.g. "HKEY_LOCAL_MACHINE\HARDWARE\RESOURCEMAP\Hardware Abstraction Layer\ACPI x64 platform.Raw" contains all vectors starting from 0 and ".Translated" contains the same vectors.

@@ -152,8 +152,8 @@ HalpInitDma(VOID)
* Check if Extended DMA is available. We're just going to do a random
* read and write.
*/
WRITE_PORT_UCHAR((PUCHAR)FIELD_OFFSET(EISA_CONTROL, DmaController2Pages.Channel2), 0x2A);
if (READ_PORT_UCHAR((PUCHAR)FIELD_OFFSET(EISA_CONTROL, DmaController2Pages.Channel2)) == 0x2A)
WRITE_PORT_UCHAR((PUCHAR)(ULONG_PTR)FIELD_OFFSET(EISA_CONTROL, DmaController2Pages.Channel2), 0x2A);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dunno, maybe the base address is 0?


/* Finally switch RSP to the new stack.
We pass OldStackBase to make sure it is not lost. */
OldStackBase = KiSwitchKernelStackHelper(StackOffset, OldStackBase);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, and if we are honest, we can't do it this way on x86 either, unless all the code that leads to this is entirely written in assembly. For x64 I have a solution that is "by the book", i.e. it returns to asm code, then switches the stack and repeats the system call handling.

PULONG_PTR NewStack;
PULONG_PTR UserStackPointer;
NTSTATUS CallbackStatus;
#ifdef _M_IX86
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that was the plan. This looks generally pretty portable to me.

/* Check if we have GDI Batch operations */
if (GdiBatchCount)
{
*UserStackPointer -= 256;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Much stack, so large, very structure.

@@ -15,13 +15,13 @@
#include <debug.h>

#include <mm/ARM3/miarm.h>
#include <fltkernel.h>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It contains a few nice macros like Add2Ptr, PtrOffset, ROUND_TO_SIZE, IS_ALIGNED, We should probably put those somewhere separate and include them from fltkernel.h and ntoskrnl.h, so we don't need the whole clusterfuck, but still have those macros available.

MiSessionViewStart = (PCHAR)MiSessionViewEnd - MmSessionViewSize;
/* Session view is below the session working set */
MmSessionViewSize = MI_SESSION_VIEW_SIZE;
MiSessionViewEnd = (PUCHAR)MiSessionImageStart - MI_SESSION_WORKING_SET_SIZE;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not to self: use MiSessionSpaceWs here.

@@ -702,6 +703,10 @@ MiInitMachineDependent(IN PLOADER_PARAMETER_BLOCK LoaderBlock)
/* Map the PFN database pages */
MiBuildPfnDatabase(LoaderBlock);

/* Initialize the bogus address space */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I probably copy-pasted this comment from i386. It's not "bogus", but it's an address space that was manually initialized. Just don't ask too many questions! It is like it is. Alex made it, so it must be right.

@@ -680,7 +642,7 @@ MmCreateProcessAddressSpace(IN ULONG MinWs,
PdePte.u.Hard.PageFrameNumber = HyperPfn;
*SystemPte = PdePte;
__invlpg(PageTablePointer);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like it is a trailing whitespace.

@@ -62,7 +62,6 @@ if(ARCH STREQUAL "i386")
math/i386/cisqrt.c)
elseif(ARCH STREQUAL "amd64")
list(APPEND MSVCRTEX_ASM_SOURCE
except/amd64/chkstk_asm.s
except/amd64/chkstk_ms.s)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nothing I guess. It might be from the time when GCC handled things differently, but IIRC this is not the case anymore for x86 and I don't care for GCC/x64.

@@ -451,7 +451,8 @@ pretty_print_option(unsigned int code, unsigned char *data, int len,
static char optbuf[32768]; /* XXX */
int hunksize = 0, numhunk = -1, numelem = 0;
char fmtbuf[32], *op = optbuf;
int i, j, k, opleft = sizeof(optbuf);
int i, j, k;
Copy link
Member

Choose a reason for hiding this comment

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

Something's off about the indentation here.

@EricKohl EricKohl removed their request for review February 19, 2018 19:32
@@ -462,7 +462,7 @@ elseif(ARCH STREQUAL "amd64")
math/amd64/log.S
math/amd64/log10.S
math/amd64/pow.S
math/amd64/sqrt.S
# math/amd64/sqrt.S
Copy link
Contributor

Choose a reason for hiding this comment

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

What's wrong with it ?

#else
PointerPde = MiAddressToPde(HYPER_SPACE);
#endif
PointerPde = MiAddressToPde((PVOID)HYPER_SPACE);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't we add the cast to the definition of HYPER_SPACE ? What's the point of having it defined as an integer when everybody knows it's an address ?


/* Adjust the PCR fields */
__addgsqword(FIELD_OFFSET(KPCR, NtTib.StackBase), StackOffset);
__addgsqword(FIELD_OFFSET(KIPCR, Prcb.RspBase), StackOffset);
Copy link
Contributor

Choose a reason for hiding this comment

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

I fail to see how, except for those two lines, this function is specific to amd64. Can't we factor out a general x86 version ?

@@ -1381,7 +1381,7 @@ MmDeleteProcessAddressSpace2(IN PEPROCESS Process)
MiDecrementShareCount(Pfn1, PageFrameIndex);

/* Page table is now dead. Bye bye... */
ASSERT((Pfn1->u3.e2.ReferenceCount == 0) || (Pfn1->u3.e1.WriteInProgress));
///ASSERT((Pfn1->u3.e2.ReferenceCount == 0) || (Pfn1->u3.e1.WriteInProgress));
Copy link
Contributor

Choose a reason for hiding this comment

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

Please #ifdef it out, with a relevant comment.

@@ -321,7 +349,11 @@ IntInt10CallBios(
FALSE,
NULL);

#ifdef _M_AMD64
Status = x86BiosCall(0x10, &BiosContext) ? STATUS_SUCCESS : STATUS_UNSUCCESSFUL;;
Copy link
Contributor

Choose a reason for hiding this comment

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

Double trailing semi-colon.

@GeoB99
Copy link
Member

GeoB99 commented Mar 25, 2018

Apologise for my ignorance but I believe when 64-bit support gets released out, only 64-bit software can be installed partly because of the lack of WOW support. Am I correct?

@nkrapivin
Copy link
Contributor

Yup, you're right. Just find some XP x64 only apps and here you go.

@sanchaez sanchaez added enhancement For PRs with an enhancement/new feature. review pending For PRs undergoing review. labels Apr 7, 2018
@Mouvedia
Copy link

Mouvedia commented Apr 8, 2018

@nkrapivin is there an issue for the Windows on Windows support?

@ROCKNROLLKID
Copy link

So what are we doing with this PR?

@HBelusca
Copy link
Contributor

@ROCKNROLLKID : @tkreuzer is slowly reintegrating it in the master.

@tkreuzer
Copy link
Contributor Author

Closing for now.

@tkreuzer tkreuzer closed this Aug 19, 2018
@binarymaster binarymaster removed the review pending For PRs undergoing review. label Jun 11, 2023
@binarymaster binarymaster added this to the x64 bringup milestone Jun 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement For PRs with an enhancement/new feature.
Projects
None yet