https://git.reactos.org/?p=reactos.git;a=commitdiff;h=b7a149fcec7f3626e671f1da658589f8c219503f
commit b7a149fcec7f3626e671f1da658589f8c219503f Author: Timo Kreuzer <[email protected]> AuthorDate: Sat Jun 5 15:39:22 2021 +0200 Commit: Timo Kreuzer <[email protected]> CommitDate: Thu Jun 17 23:27:44 2021 +0200 [HAL:APIC] Code fixes * Fix some broken code * Add some ASSERTs * Use ApicWriteIORedirectionEntry where appropriate * Use KeQueryInterruptHandler/KeRegisterInterruptHandler to save/restore the old handler instead of saving the KIDTENTRY * Move HalpProfileInterruptHandler to apictimer.c and implement it * Use READ/WRITE_REGISTER macros * Add some symbolic names --- hal/halx86/apic/apic.c | 40 +++++++++++++++++++++++----------------- hal/halx86/apic/apicp.h | 8 ++++++-- hal/halx86/apic/apictimer.c | 7 +++++++ hal/halx86/apic/rtctimer.c | 7 ------- hal/halx86/apic/tsc.c | 10 ++++------ 5 files changed, 40 insertions(+), 32 deletions(-) diff --git a/hal/halx86/apic/apic.c b/hal/halx86/apic/apic.c index 2811cc45535..65a6d2e4d20 100644 --- a/hal/halx86/apic/apic.c +++ b/hal/halx86/apic/apic.c @@ -94,8 +94,9 @@ ULONG IOApicRead(UCHAR Register) { /* Select the register, then do the read */ - *(volatile UCHAR *)(IOAPIC_BASE + IOAPIC_IOREGSEL) = Register; - return *(volatile ULONG *)(IOAPIC_BASE + IOAPIC_IOWIN); + ASSERT(Register <= 0x3F); + WRITE_REGISTER_ULONG((PULONG)(IOAPIC_BASE + IOAPIC_IOREGSEL), Register); + return READ_REGISTER_ULONG((PULONG)(IOAPIC_BASE + IOAPIC_IOWIN)); } FORCEINLINE @@ -103,8 +104,9 @@ VOID IOApicWrite(UCHAR Register, ULONG Value) { /* Select the register, then do the write */ - *(volatile UCHAR *)(IOAPIC_BASE + IOAPIC_IOREGSEL) = Register; - *(volatile ULONG *)(IOAPIC_BASE + IOAPIC_IOWIN) = Value; + ASSERT(Register <= 0x3F); + WRITE_REGISTER_ULONG((PULONG)(IOAPIC_BASE + IOAPIC_IOREGSEL), Register); + WRITE_REGISTER_ULONG((PULONG)(IOAPIC_BASE + IOAPIC_IOWIN), Value); } FORCEINLINE @@ -113,6 +115,7 @@ ApicWriteIORedirectionEntry( UCHAR Index, IOAPIC_REDIRECTION_REGISTER ReDirReg) { + ASSERT(Index < APIC_MAX_IRQ); IOApicWrite(IOAPIC_REDTBL + 2 * Index, ReDirReg.Long0); IOApicWrite(IOAPIC_REDTBL + 2 * Index + 1, ReDirReg.Long1); } @@ -124,6 +127,7 @@ ApicReadIORedirectionEntry( { IOAPIC_REDIRECTION_REGISTER ReDirReg; + ASSERT(Index < APIC_MAX_IRQ); ReDirReg.Long0 = IOApicRead(IOAPIC_REDTBL + 2 * Index); ReDirReg.Long1 = IOApicRead(IOAPIC_REDTBL + 2 * Index + 1); @@ -344,7 +348,7 @@ HalpAllocateSystemInterrupt( Vector = IrqlToTpr(Irql) & 0xF0; /* Find an empty vector */ - while (HalpVectorToIndex[Vector] != 0xFF) + while (HalpVectorToIndex[Vector] != APIC_FREE_VECTOR) { Vector++; @@ -372,8 +376,7 @@ HalpAllocateSystemInterrupt( ReDirReg.Destination = 0; /* Initialize entry */ - IOApicWrite(IOAPIC_REDTBL + 2 * Irq, ReDirReg.Long0); - IOApicWrite(IOAPIC_REDTBL + 2 * Irq + 1, ReDirReg.Long1); + ApicWriteIORedirectionEntry(Irq, ReDirReg); return Vector; } @@ -410,17 +413,16 @@ ApicInitializeIOApic(VOID) ReDirReg.Destination = 0; /* Loop all table entries */ - for (Index = 0; Index < 24; Index++) + for (Index = 0; Index < APIC_MAX_IRQ; Index++) { /* Initialize entry */ - IOApicWrite(IOAPIC_REDTBL + 2 * Index, ReDirReg.Long0); - IOApicWrite(IOAPIC_REDTBL + 2 * Index + 1, ReDirReg.Long1); + ApicWriteIORedirectionEntry(Index, ReDirReg); } /* Init the vactor to index table */ for (Vector = 0; Vector <= 255; Vector++) { - HalpVectorToIndex[Vector] = 0xFF; + HalpVectorToIndex[Vector] = APIC_FREE_VECTOR; } // HACK: Allocate all IRQs, should rather do that on demand @@ -437,7 +439,7 @@ ApicInitializeIOApic(VOID) ReDirReg.TriggerMode = APIC_TGM_Edge; ReDirReg.Mask = 0; ReDirReg.Destination = ApicRead(APIC_ID); - IOApicWrite(IOAPIC_REDTBL + 2 * APIC_CLOCK_INDEX, ReDirReg.Long0); + ApicWriteIORedirectionEntry(APIC_CLOCK_INDEX, ReDirReg); } VOID @@ -457,9 +459,10 @@ HalpInitializePICs(IN BOOLEAN EnableInterrupts) ApicInitializeIOApic(); /* Manually reserve some vectors */ + HalpVectorToIndex[APC_VECTOR] = APIC_RESERVED_VECTOR; + HalpVectorToIndex[DISPATCH_VECTOR] = APIC_RESERVED_VECTOR; HalpVectorToIndex[APIC_CLOCK_VECTOR] = 8; - HalpVectorToIndex[APC_VECTOR] = 99; - HalpVectorToIndex[DISPATCH_VECTOR] = 99; + HalpVectorToIndex[APIC_SPURIOUS_VECTOR] = APIC_RESERVED_VECTOR; /* Set interrupt handlers in the IDT */ KeRegisterInterruptHandler(APIC_CLOCK_VECTOR, HalpClockInterrupt); @@ -669,7 +672,7 @@ HalDisableSystemInterrupt( ReDirReg.Mask = 1; /* Write back lower dword */ - IOApicWrite(IOAPIC_REDTBL + 2 * Irql, ReDirReg.Long0); + IOApicWrite(IOAPIC_REDTBL + 2 * Index, ReDirReg.Long0); } #ifndef _M_AMD64 @@ -704,8 +707,8 @@ HalBeginSystemInterrupt( /* Get the irq for this vector */ Index = HalpVectorToIndex[Vector]; - /* Check if its valid */ - if (Index != 0xff) + /* Check if it's valid */ + if (Index < APIC_MAX_IRQ) { /* Read the I/O redirection entry */ RedirReg = ApicReadIORedirectionEntry(Index); @@ -715,6 +718,9 @@ HalBeginSystemInterrupt( } else { + /* This should be a reserved vector! */ + ASSERT(Index == APIC_RESERVED_VECTOR); + /* Re-request the interrupt to be handled later */ ApicRequestInterrupt(Vector, APIC_TGM_Edge); } diff --git a/hal/halx86/apic/apicp.h b/hal/halx86/apic/apicp.h index ee8eec3d665..d3e1ece84aa 100644 --- a/hal/halx86/apic/apicp.h +++ b/hal/halx86/apic/apicp.h @@ -25,6 +25,10 @@ #define TprToIrql(Tpr) (HalVectorToIRQL[Tpr >> 4]) #endif +#define APIC_MAX_IRQ 24 +#define APIC_FREE_VECTOR 0xFF +#define APIC_RESERVED_VECTOR 0xFE + /* The IMCR is supported by two read/writable or write-only I/O ports, 22h and 23h, which receive address and data respectively. To access the IMCR, write a value of 70h to I/O port 22h, which selects the IMCR. @@ -284,14 +288,14 @@ FORCEINLINE ULONG ApicRead(ULONG Offset) { - return *(volatile ULONG *)(APIC_BASE + Offset); + return READ_REGISTER_ULONG((PULONG)(APIC_BASE + Offset)); } FORCEINLINE VOID ApicWrite(ULONG Offset, ULONG Value) { - *(volatile ULONG *)(APIC_BASE + Offset) = Value; + WRITE_REGISTER_ULONG((PULONG)(APIC_BASE + Offset), Value); } VOID diff --git a/hal/halx86/apic/apictimer.c b/hal/halx86/apic/apictimer.c index 15f87a58499..ec1e0f1ecef 100644 --- a/hal/halx86/apic/apictimer.c +++ b/hal/halx86/apic/apictimer.c @@ -61,6 +61,13 @@ ApicInitializeTimer(ULONG Cpu) // KeSetTimeIncrement } +VOID +FASTCALL +HalpProfileInterruptHandler(_In_ PKTRAP_FRAME TrapFrame) +{ + KeProfileInterruptWithSource(TrapFrame, ProfileTime); +} + /* PUBLIC FUNCTIONS ***********************************************************/ diff --git a/hal/halx86/apic/rtctimer.c b/hal/halx86/apic/rtctimer.c index 817e6e2de88..2d30ad55c43 100644 --- a/hal/halx86/apic/rtctimer.c +++ b/hal/halx86/apic/rtctimer.c @@ -167,13 +167,6 @@ HalpClockInterruptHandler(IN PKTRAP_FRAME TrapFrame) KeUpdateSystemTime(TrapFrame, LastIncrement, Irql); } -VOID -FASTCALL -HalpProfileInterruptHandler(IN PKTRAP_FRAME TrapFrame) -{ - __debugbreak(); -} - ULONG NTAPI HalSetTimeIncrement(IN ULONG Increment) diff --git a/hal/halx86/apic/tsc.c b/hal/halx86/apic/tsc.c index ce06b2f6d8e..1ff881206fd 100644 --- a/hal/halx86/apic/tsc.c +++ b/hal/halx86/apic/tsc.c @@ -55,8 +55,7 @@ NTAPI HalpInitializeTsc(VOID) { ULONG_PTR Flags; - KIDTENTRY OldIdtEntry, *IdtPointer; - PKPCR Pcr = KeGetPcr(); + PVOID PreviousHandler; UCHAR RegisterA, RegisterB; /* Check if the CPU supports RDTSC */ @@ -79,8 +78,7 @@ HalpInitializeTsc(VOID) HalpWriteCmos(RTC_REGISTER_A, RegisterA); /* Save old IDT entry */ - IdtPointer = KiGetIdtEntry(Pcr, HalpRtcClockVector); - OldIdtEntry = *IdtPointer; + PreviousHandler = KeQueryInterruptHandler(HalpRtcClockVector); /* Set the calibration ISR */ KeRegisterInterruptHandler(HalpRtcClockVector, TscCalibrationISR); @@ -105,8 +103,8 @@ HalpInitializeTsc(VOID) /* Disable the timer interrupt */ HalDisableSystemInterrupt(HalpRtcClockVector, CLOCK_LEVEL); - /* Restore old IDT entry */ - *IdtPointer = OldIdtEntry; + /* Restore the previous handler */ + KeRegisterInterruptHandler(HalpRtcClockVector, PreviousHandler); /* Calculate an average, using simplified linear regression */ HalpCpuClockFrequency.QuadPart = DoLinearRegression(NUM_SAMPLES - 1,
