diff mbox

[edk2,04/10] ArmPkg: allow dynamically discovered virtual timer interrupt

Message ID 1408994367-11143-5-git-send-email-ard.biesheuvel@linaro.org
State New
Headers show

Commit Message

Ard Biesheuvel Aug. 25, 2014, 7:19 p.m. UTC 
To support booting on virtual machines whose interrupt routing is
discovered from the device tree, allow the interrupt numbers to
be redeclared as PcdsDynamic by the platform .dsc
---
 ArmPkg/ArmPkg.dec                                |  2 ++
 ArmPkg/Drivers/TimerDxe/TimerDxe.c               |  6 +++++-
 ArmPkg/Library/ArmLib/AArch64/AArch64ArchTimer.c | 16 ++++++++--------
 3 files changed, 15 insertions(+), 9 deletions(-)

Comments

Ard Biesheuvel Aug. 26, 2014, 11:23 a.m. UTC | #1 
On 25 August 2014 21:19, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> To support booting on virtual machines whose interrupt routing is
> discovered from the device tree, allow the interrupt numbers to
> be redeclared as PcdsDynamic by the platform .dsc
> ---
>  ArmPkg/ArmPkg.dec                                |  2 ++
>  ArmPkg/Drivers/TimerDxe/TimerDxe.c               |  6 +++++-
>  ArmPkg/Library/ArmLib/AArch64/AArch64ArchTimer.c | 16 ++++++++--------
>  3 files changed, 15 insertions(+), 9 deletions(-)
>
[...]
> diff --git a/ArmPkg/Drivers/TimerDxe/TimerDxe.c b/ArmPkg/Drivers/TimerDxe/TimerDxe.c
> index be346bf89c51..670c4431e5a1 100644
> --- a/ArmPkg/Drivers/TimerDxe/TimerDxe.c
> +++ b/ArmPkg/Drivers/TimerDxe/TimerDxe.c
[...]
> @@ -343,6 +345,8 @@ TimerInitialize (
>    Status = TimerDriverSetTimerPeriod (&gTimer, 0);
>    ASSERT_EFI_ERROR (Status);
>
> +  mArmArchTimerTimerFreq = ArmArchTimerGetTimerFreq() / 1000000;
> +

@Olivier: this is a change that slipped in with the others, but this
is something I meant to ask you about:
since we already establish in
ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.c that
ArmArchTimerGetTimerFreq () returns a sane value, is there any reason
to keep using the PCD here? In case of virtualization, it is much
easier to just read the register than to have yet another dynamic PCD
and set it in the KVM platform lib.
diff mbox

Patch

diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
index 1d896892dafc..9229c07df53c 100644
--- a/ArmPkg/ArmPkg.dec
+++ b/ArmPkg/ArmPkg.dec
@@ -136,6 +136,8 @@ 
   # Maximum file size for TFTP servers that do not support 'tsize' extension
   gArmTokenSpaceGuid.PcdMaxTftpFileSize|0x01000000|UINT32|0x00000000
 
+
+[PcdsFixedAtBuild.common,PcdsDynamic.common]
   #
   # ARM Architectural Timer
   #
diff --git a/ArmPkg/Drivers/TimerDxe/TimerDxe.c b/ArmPkg/Drivers/TimerDxe/TimerDxe.c
index be346bf89c51..670c4431e5a1 100644
--- a/ArmPkg/Drivers/TimerDxe/TimerDxe.c
+++ b/ArmPkg/Drivers/TimerDxe/TimerDxe.c
@@ -36,6 +36,8 @@  EFI_EVENT             EfiExitBootServicesEvent = (EFI_EVENT)NULL;
 // The current period of the timer interrupt
 UINT64 mTimerPeriod = 0;
 
+UINTN mArmArchTimerTimerFreq = 0;
+
 // Cached copy of the Hardware Interrupt protocol instance
 EFI_HARDWARE_INTERRUPT_PROTOCOL *gInterrupt = NULL;
 
@@ -144,7 +146,7 @@  TimerDriverSetTimerPeriod (
     // Convert TimerPeriod to micro sec units
     TimerTicks = DivU64x32 (TimerPeriod, 10);
 
-    TimerTicks = MultU64x32 (TimerTicks, (PcdGet32(PcdArmArchTimerFreqInHz)/1000000));
+    TimerTicks = MultU64x32 (TimerTicks, mArmArchTimerTimerFreq);
 
     ArmArchTimerSetTimerVal((UINTN)TimerTicks);
 
@@ -343,6 +345,8 @@  TimerInitialize (
   Status = TimerDriverSetTimerPeriod (&gTimer, 0);
   ASSERT_EFI_ERROR (Status);
 
+  mArmArchTimerTimerFreq = ArmArchTimerGetTimerFreq() / 1000000;
+
   // Install secure and Non-secure interrupt handlers
   // Note: Because it is not possible to determine the security state of the
   // CPU dynamically, we just install interrupt handler for both sec and non-sec
diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64ArchTimer.c b/ArmPkg/Library/ArmLib/AArch64/AArch64ArchTimer.c
index 232d745b3d06..6487dcc441e4 100644
--- a/ArmPkg/Library/ArmLib/AArch64/AArch64ArchTimer.c
+++ b/ArmPkg/Library/ArmLib/AArch64/AArch64ArchTimer.c
@@ -175,7 +175,7 @@  ArmArchTimerEnableTimer (
 {
   UINTN TimerCtrlReg;
 
-  if (PcdGetBool(PcdArmArchTimerUseVirtual) == TRUE) {
+  if (FixedPcdGetBool(PcdArmArchTimerUseVirtual) == TRUE) {
     ArmArchTimerReadReg (CntvCtl, (VOID *)&TimerCtrlReg);
     /* FIXME: We need to clear IMASK when under KVM because KVM sets it. Unclear if this is a bug ... */
     TimerCtrlReg |= ARM_ARCH_TIMER_ENABLE;
@@ -196,7 +196,7 @@  ArmArchTimerDisableTimer (
 {
   UINTN TimerCtrlReg;
 
-  if (PcdGetBool(PcdArmArchTimerUseVirtual) == TRUE) {
+  if (FixedPcdGetBool(PcdArmArchTimerUseVirtual) == TRUE) {
     ArmArchTimerReadReg (CntvCtl, (VOID *)&TimerCtrlReg);
     TimerCtrlReg &= ~ARM_ARCH_TIMER_ENABLE;
     ArmArchTimerWriteReg (CntvCtl, (VOID *)&TimerCtrlReg);
@@ -234,7 +234,7 @@  ArmArchTimerGetTimerVal (
     )
 {
   UINTN ArchTimerVal;
-  if (PcdGetBool(PcdArmArchTimerUseVirtual) == TRUE) {
+  if (FixedPcdGetBool(PcdArmArchTimerUseVirtual) == TRUE) {
     ArmArchTimerReadReg (CntvTval, (VOID *)&ArchTimerVal);
   } else {
     ArmArchTimerReadReg (CntpTval, (VOID *)&ArchTimerVal);
@@ -250,7 +250,7 @@  ArmArchTimerSetTimerVal (
     IN   UINTN   Val
     )
 {
-  if (PcdGetBool(PcdArmArchTimerUseVirtual) == TRUE) {
+  if (FixedPcdGetBool(PcdArmArchTimerUseVirtual) == TRUE) {
     ArmArchTimerWriteReg (CntvTval, (VOID *)&Val);
   } else {
     ArmArchTimerWriteReg (CntpTval, (VOID *)&Val);
@@ -264,7 +264,7 @@  ArmArchTimerGetSystemCount (
     )
 {
   UINT64 SystemCount;
-  if (PcdGetBool(PcdArmArchTimerUseVirtual) == TRUE) {
+  if (FixedPcdGetBool(PcdArmArchTimerUseVirtual) == TRUE) {
     ArmArchTimerReadReg (CntvCt, (VOID *)&SystemCount);
   } else {
     ArmArchTimerReadReg (CntPct, (VOID *)&SystemCount);
@@ -281,7 +281,7 @@  ArmArchTimerGetTimerCtrlReg (
 {
   UINTN  Val;
 
-  if (PcdGetBool(PcdArmArchTimerUseVirtual) == TRUE) {
+  if (FixedPcdGetBool(PcdArmArchTimerUseVirtual) == TRUE) {
     ArmArchTimerReadReg (CntvCtl, (VOID *)&Val);
   } else {
     ArmArchTimerReadReg (CntpCtl, (VOID *)&Val);
@@ -296,7 +296,7 @@  ArmArchTimerSetTimerCtrlReg (
     UINTN Val
     )
 {
-  if (PcdGetBool(PcdArmArchTimerUseVirtual) == TRUE) {
+  if (FixedPcdGetBool(PcdArmArchTimerUseVirtual) == TRUE) {
     ArmArchTimerWriteReg (CntvCtl, (VOID *)&Val);
   } else {
     ArmArchTimerWriteReg (CntpCtl, (VOID *)&Val);
@@ -309,7 +309,7 @@  ArmArchTimerSetCompareVal (
     IN   UINT64   Val
     )
 {
-  if (PcdGetBool(PcdArmArchTimerUseVirtual) == TRUE) {
+  if (FixedPcdGetBool(PcdArmArchTimerUseVirtual) == TRUE) {
     ArmArchTimerWriteReg (CntvCval, (VOID *)&Val);
   } else {
     ArmArchTimerWriteReg (CntpCval, (VOID *)&Val);