On 25 August 2014 21:19, Ard Biesheuvel <ard.biesheu...@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.

-- 
Ard.




>    // 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);
> --
> 1.8.3.2
>

------------------------------------------------------------------------------
Slashdot TV.  
Video for Nerds.  Stuff that matters.
http://tv.slashdot.org/
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel

Reply via email to