|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [edk2-devel] [PATCH v2 27/31] OvmfPkg/XenOvmf: Introduce XenTimerDxe
On 04/09/19 13:08, Anthony PERARD wrote: > "PcAtChipsetPkg/8254TimerDxe" is replaced with a Xen-specific > EFI_TIMER_ARCH_PROTOCOL implementation. Also remove > 8259InterruptControllerDxe as it is not used anymore. > > This Timer uses the local APIC timer as time source as it can work on > both a Xen PVH guest and an HVM one. > > Based on the "PcAtChipsetPkg/8254TimerDxe" implementation. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx> > --- > > Notes: > v2: > - Use InitializeApicTimer instead of WriteLocalApicReg > - rework comments (remove many that don't apply) > - remove unused includes, and libs > - have a macro to the timervector. > - cleanup, copyright > - rework calculation of TimerCount, value to be use by the APIC timer > - check for overflow of TimerPeriod, with the apic timer, the period can > be up to about 42s on Xen (or even higher by changing the DivideValue). > > OvmfPkg/XenOvmf.dsc > | 3 +- > OvmfPkg/XenOvmf.fdf > | 3 +- > PcAtChipsetPkg/8254TimerDxe/8254Timer.inf => > OvmfPkg/XenTimerDxe/XenTimerDxe.inf | 27 +++--- > PcAtChipsetPkg/8254TimerDxe/Timer.h => OvmfPkg/XenTimerDxe/XenTimerDxe.h > | 32 +++---- > PcAtChipsetPkg/8254TimerDxe/Timer.c => OvmfPkg/XenTimerDxe/XenTimerDxe.c > | 100 ++++++-------------- > 5 files changed, 55 insertions(+), 110 deletions(-) For OVMF in general, the primary residence of 8254TimerDxe is now under OvmfPkg, due to <https://bugzilla.tianocore.org/show_bug.cgi?id=1496>. However, as long as the module continues to exist under PcAtChipsetPkg (i.e. until <https://bugzilla.tianocore.org/show_bug.cgi?id=1604> is completed), I think it is OK as the copy-origin here. > > diff --git a/OvmfPkg/XenOvmf.dsc b/OvmfPkg/XenOvmf.dsc > index 54601c697f..35af05715b 100644 > --- a/OvmfPkg/XenOvmf.dsc > +++ b/OvmfPkg/XenOvmf.dsc > @@ -560,10 +560,9 @@ [Components] > MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf > > MdeModulePkg/Universal/EbcDxe/EbcDxe.inf > - PcAtChipsetPkg/8259InterruptControllerDxe/8259.inf > + OvmfPkg/XenTimerDxe/XenTimerDxe.inf > UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf > UefiCpuPkg/CpuDxe/CpuDxe.inf > - PcAtChipsetPkg/8254TimerDxe/8254Timer.inf > OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf > OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf > MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf { > diff --git a/OvmfPkg/XenOvmf.fdf b/OvmfPkg/XenOvmf.fdf > index f9e58befd6..d614bdce1d 100644 > --- a/OvmfPkg/XenOvmf.fdf > +++ b/OvmfPkg/XenOvmf.fdf > @@ -284,10 +284,9 @@ [FV.DXEFV] > INF MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf > INF MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf > INF MdeModulePkg/Universal/EbcDxe/EbcDxe.inf > -INF PcAtChipsetPkg/8259InterruptControllerDxe/8259.inf > +INF OvmfPkg/XenTimerDxe/XenTimerDxe.inf > INF UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf > INF UefiCpuPkg/CpuDxe/CpuDxe.inf > -INF PcAtChipsetPkg/8254TimerDxe/8254Timer.inf > INF OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf > INF OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf > INF MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf > diff --git a/PcAtChipsetPkg/8254TimerDxe/8254Timer.inf > b/OvmfPkg/XenTimerDxe/XenTimerDxe.inf > similarity index 64% > copy from PcAtChipsetPkg/8254TimerDxe/8254Timer.inf > copy to OvmfPkg/XenTimerDxe/XenTimerDxe.inf > index 46cf01de39..5ad80b9368 100644 > --- a/PcAtChipsetPkg/8254TimerDxe/8254Timer.inf > +++ b/OvmfPkg/XenTimerDxe/XenTimerDxe.inf > @@ -1,7 +1,9 @@ > ## @file > -# 8254 timer driver that provides Timer Arch protocol. > +# Local APIC timer driver that provides Timer Arch protocol. > +# > +# Copyright (c) 2005 - 2014, Intel Corporation. All rights reserved.<BR> > +# Copyright (c) 2019, Citrix Systems, Inc. > # > -# Copyright (c) 2005 - 2018, Intel Corporation. All rights reserved.<BR> > # This program and the accompanying materials > # are licensed and made available under the terms and conditions of the BSD > License > # which accompanies this distribution. The full text of the license may be > found at I'm sure this is well-intended (you simply created the patch before the Intel copyright notice was bumped under PcAtChipsetPkg), but now this diff appears a bit questionable, due to "downgrading" the Intel copyright. I understand it's technically correct (because the original code, being copied & customized here, matches the copyright notice too). (1) Still, could you please refresh this patch so that the Intel copyright is not touched in the "find-copies-harder" diff? (Covering all new files added in this patch.) ... Such a refresh would be justified by <https://bugzilla.tianocore.org/show_bug.cgi?id=1373> anyway -- we should preferably add new files only with SPDX license IDs now. Otherwise I'd be OK to ACK the patch. Thanks Laszlo > @@ -14,9 +16,8 @@ > > [Defines] > INF_VERSION = 0x00010005 > - BASE_NAME = Timer > - MODULE_UNI_FILE = Timer.uni > - FILE_GUID = f2765dec-6b41-11d5-8e71-00902707b35e > + BASE_NAME = XenTimerDxe > + FILE_GUID = 52fe8196-f9de-4d07-b22f-51f77a0e7c41 > MODULE_TYPE = DXE_DRIVER > VERSION_STRING = 1.0 > > @@ -25,24 +26,24 @@ [Defines] > [Packages] > MdePkg/MdePkg.dec > IntelFrameworkPkg/IntelFrameworkPkg.dec > + UefiCpuPkg/UefiCpuPkg.dec > + OvmfPkg/OvmfPkg.dec > > [LibraryClasses] > UefiBootServicesTableLib > BaseLib > DebugLib > UefiDriverEntryPoint > - IoLib > + LocalApicLib > > [Sources] > - Timer.h > - Timer.c > + XenTimerDxe.h > + XenTimerDxe.c > > [Protocols] > gEfiCpuArchProtocolGuid ## CONSUMES > - gEfiLegacy8259ProtocolGuid ## CONSUMES > gEfiTimerArchProtocolGuid ## PRODUCES > - > +[Pcd] > + gEfiMdePkgTokenSpaceGuid.PcdFSBClock ## CONSUMES > [Depex] > - gEfiCpuArchProtocolGuid AND gEfiLegacy8259ProtocolGuid > -[UserExtensions.TianoCore."ExtraFiles"] > - TimerExtra.uni > + gEfiCpuArchProtocolGuid > diff --git a/PcAtChipsetPkg/8254TimerDxe/Timer.h > b/OvmfPkg/XenTimerDxe/XenTimerDxe.h > similarity index 89% > copy from PcAtChipsetPkg/8254TimerDxe/Timer.h > copy to OvmfPkg/XenTimerDxe/XenTimerDxe.h > index 9d70e3aa19..747259c162 100644 > --- a/PcAtChipsetPkg/8254TimerDxe/Timer.h > +++ b/OvmfPkg/XenTimerDxe/XenTimerDxe.h > @@ -1,7 +1,9 @@ > /** @file > Private data structures > > -Copyright (c) 2005 - 2018, Intel Corporation. All rights reserved.<BR> > +Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.<BR> > +Copyright (c) 2019, Citrix Systems, Inc. > + > This program and the accompanying materials > are licensed and made available under the terms and conditions of the BSD > License > which accompanies this distribution. The full text of the license may be > found at > @@ -17,34 +19,24 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER > EXPRESS OR IMPLIED. > #include <PiDxe.h> > > #include <Protocol/Cpu.h> > -#include <Protocol/Legacy8259.h> > #include <Protocol/Timer.h> > > +#include <Register/LocalApic.h> > + > #include <Library/UefiBootServicesTableLib.h> > #include <Library/BaseLib.h> > #include <Library/DebugLib.h> > -#include <Library/IoLib.h> > +#include <Library/LocalApicLib.h> > +#include <Library/PcdLib.h> > > -// > -// The PCAT 8253/8254 has an input clock at 1.193182 MHz and Timer 0 is > -// initialized as a 16 bit free running counter that generates an > interrupt(IRQ0) > -// each time the counter rolls over. > -// > -// 65536 counts > -// ---------------- * 1,000,000 uS/S = 54925.4 uS = 549254 * 100 ns > -// 1,193,182 Hz > -// > - > -// > -// The maximum tick duration for 8254 timer > -// > -#define MAX_TIMER_TICK_DURATION 549254 > -// > // The default timer tick duration is set to 10 ms = 100000 100 ns units > // > #define DEFAULT_TIMER_TICK_DURATION 100000 > -#define TIMER_CONTROL_PORT 0x43 > -#define TIMER0_COUNT_PORT 0x40 > + > +// > +// The Timer Vector use for interrupt > +// > +#define LOCAL_APIC_TIMER_VECTOR 32 > > // > // Function Prototypes > diff --git a/PcAtChipsetPkg/8254TimerDxe/Timer.c > b/OvmfPkg/XenTimerDxe/XenTimerDxe.c > similarity index 77% > copy from PcAtChipsetPkg/8254TimerDxe/Timer.c > copy to OvmfPkg/XenTimerDxe/XenTimerDxe.c > index 60799aadd3..fb39ce0cd3 100644 > --- a/PcAtChipsetPkg/8254TimerDxe/Timer.c > +++ b/OvmfPkg/XenTimerDxe/XenTimerDxe.c > @@ -1,7 +1,9 @@ > /** @file > Timer Architectural Protocol as defined in the DXE CIS > > -Copyright (c) 2005 - 2018, Intel Corporation. All rights reserved.<BR> > +Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.<BR> > +Copyright (c) 2019, Citrix Systems, Inc. > + > This program and the accompanying materials > are licensed and made available under the terms and conditions of the BSD > License > which accompanies this distribution. The full text of the license may be > found at > @@ -12,7 +14,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER > EXPRESS OR IMPLIED. > > **/ > > -#include "Timer.h" > +#include "XenTimerDxe.h" > > // > // The handle onto which the Timer Architectural Protocol will be installed > @@ -34,11 +36,6 @@ EFI_TIMER_ARCH_PROTOCOL mTimer = { > // > EFI_CPU_ARCH_PROTOCOL *mCpu; > > -// > -// Pointer to the Legacy 8259 Protocol instance > -// > -EFI_LEGACY_8259_PROTOCOL *mLegacy8259; > - > // > // The notification function to call on every timer interrupt. > // A bug in the compiler prevents us from initializing this here. > @@ -54,22 +51,7 @@ volatile UINT64 mTimerPeriod = 0; > // Worker Functions > // > /** > - Sets the counter value for Timer #0 in a legacy 8254 timer. > - > - @param Count The 16-bit counter value to program into Timer #0 of the > legacy 8254 timer. > -**/ > -VOID > -SetPitCount ( > - IN UINT16 Count > - ) > -{ > - IoWrite8 (TIMER_CONTROL_PORT, 0x36); > - IoWrite8 (TIMER0_COUNT_PORT, (UINT8)(Count & 0xff)); > - IoWrite8 (TIMER0_COUNT_PORT, (UINT8)((Count >> 8) & 0xff)); > -} > - > -/** > - 8254 Timer #0 Interrupt Handler. > + Interrupt Handler. > > @param InterruptType The type of interrupt that occurred > @param SystemContext A pointer to the system context when the interrupt > occurred > @@ -85,7 +67,7 @@ TimerInterruptHandler ( > > OriginalTPL = gBS->RaiseTPL (TPL_HIGH_LEVEL); > > - mLegacy8259->EndOfInterrupt (mLegacy8259, Efi8259Irq0); > + SendApicEoi(); > > if (mTimerNotifyFunction != NULL) { > // > @@ -187,49 +169,41 @@ TimerDriverSetTimerPeriod ( > ) > { > UINT64 TimerCount; > + UINT32 TimerFrequency; > + UINTN DivideValue = 1; > > - // > - // The basic clock is 1.19318 MHz or 0.119318 ticks per 100 ns. > - // TimerPeriod * 0.119318 = 8254 timer divisor. Using integer arithmetic > - // TimerCount = (TimerPeriod * 119318)/1000000. > - // > - // Round up to next highest integer. This guarantees that the timer is > - // equal to or slightly longer than the requested time. > - // TimerCount = ((TimerPeriod * 119318) + 500000)/1000000 > - // > - // Note that a TimerCount of 0 is equivalent to a count of 65,536 > - // > - // Since TimerCount is limited to 16 bits for IA32, TimerPeriod is limited > - // to 20 bits. > - // > if (TimerPeriod == 0) { > // > // Disable timer interrupt for a TimerPeriod of 0 > // > - mLegacy8259->DisableIrq (mLegacy8259, Efi8259Irq0); > + DisableApicTimerInterrupt(); > } else { > + TimerFrequency = PcdGet32(PcdFSBClock) / DivideValue; > > // > - // Convert TimerPeriod into 8254 counts > + // Convert TimerPeriod into local APIC counts > // > - TimerCount = DivU64x32 (MultU64x32 (119318, (UINT32) TimerPeriod) + > 500000, 1000000); > + // TimerPeriod is in 100ns > + // TimerPeriod/10000000 will be in seconds. > + TimerCount = DivU64x32 (MultU64x32 (TimerPeriod, TimerFrequency), > + 10000000); > > - // > // Check for overflow > - // > - if (TimerCount >= 65536) { > - TimerCount = 0; > - TimerPeriod = MAX_TIMER_TICK_DURATION; > + if (TimerCount > MAX_UINT32) { > + TimerCount = MAX_UINT32; > + /* TimerPeriod = (MAX_UINT32 / TimerFrequency) * 10000000; */ > + TimerPeriod = 429496730; > } > + > // > - // Program the 8254 timer with the new count value > + // Program the timer with the new count value > // > - SetPitCount ((UINT16) TimerCount); > + InitializeApicTimer(DivideValue, TimerCount, TRUE, > LOCAL_APIC_TIMER_VECTOR); > > // > // Enable timer interrupt > // > - mLegacy8259->EnableIrq (mLegacy8259, Efi8259Irq0, FALSE); > + EnableApicTimerInterrupt(); > } > // > // Save the new timer period > @@ -294,16 +268,9 @@ TimerDriverGenerateSoftInterrupt ( > IN EFI_TIMER_ARCH_PROTOCOL *This > ) > { > - EFI_STATUS Status; > - UINT16 IRQMask; > EFI_TPL OriginalTPL; > > - // > - // If the timer interrupt is enabled, then the registered handler will be > invoked. > - // > - Status = mLegacy8259->GetMask (mLegacy8259, NULL, NULL, &IRQMask, NULL); > - ASSERT_EFI_ERROR (Status); > - if ((IRQMask & 0x1) == 0) { > + if (GetApicTimerInterruptState()) { > // > // Invoke the registered handler > // > @@ -343,7 +310,6 @@ TimerDriverInitialize ( > ) > { > EFI_STATUS Status; > - UINT32 TimerVector; > > // > // Initialize the pointer to our notify function. > @@ -361,12 +327,6 @@ TimerDriverInitialize ( > Status = gBS->LocateProtocol (&gEfiCpuArchProtocolGuid, NULL, (VOID **) > &mCpu); > ASSERT_EFI_ERROR (Status); > > - // > - // Find the Legacy8259 protocol. > - // > - Status = gBS->LocateProtocol (&gEfiLegacy8259ProtocolGuid, NULL, (VOID **) > &mLegacy8259); > - ASSERT_EFI_ERROR (Status); > - > // > // Force the timer to be disabled > // > @@ -374,16 +334,10 @@ TimerDriverInitialize ( > ASSERT_EFI_ERROR (Status); > > // > - // Get the interrupt vector number corresponding to IRQ0 from the 8259 > driver > + // Install interrupt handler for Local APIC Timer > // > - TimerVector = 0; > - Status = mLegacy8259->GetVector (mLegacy8259, Efi8259Irq0, (UINT8 *) > &TimerVector); > - ASSERT_EFI_ERROR (Status); > - > - // > - // Install interrupt handler for 8254 Timer #0 (ISA IRQ0) > - // > - Status = mCpu->RegisterInterruptHandler (mCpu, TimerVector, > TimerInterruptHandler); > + Status = mCpu->RegisterInterruptHandler (mCpu, LOCAL_APIC_TIMER_VECTOR, > + TimerInterruptHandler); > ASSERT_EFI_ERROR (Status); > > // > _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |