Hi Thomas,

GTDT looks OK now.

Regards.
Alexei.

> -----Original Message-----
> From: Thomas Abraham <thomas.abra...@arm.com>
> Sent: 24 May 2018 12:36
> To: Alexei Fedorov <alexei.fedo...@arm.com>
> Cc: Thomas Abraham <thomas.abra...@arm.com>; edk2-devel@lists.01.org;
> leif.lindh...@linaro.org; ard.biesheu...@linaro.org
> Subject: Re: [edk2] [PATCH edk2-platforms v6 5/9] Platform/ARM/Sgi: add the
> initial set of acpi tables
>
> Hi Alexei,
>
> On Wed, May 23, 2018 at 4:20 PM, Alexei Fedorov <alexei.fedo...@arm.com>
> wrote:
> > Please see my comment in-lined.
> >
> >> -----Original Message-----
> >> From: edk2-devel <edk2-devel-boun...@lists.01.org> On Behalf Of
> >> Thomas Abraham
> >> Sent: 23 May 2018 06:46
> >> To: edk2-devel@lists.01.org
> >> Cc: leif.lindh...@linaro.org; ard.biesheu...@linaro.org
> >> Subject: [edk2] [PATCH edk2-platforms v6 5/9] Platform/ARM/Sgi: add
> >> the initial set of acpi tables
> >>
> >> From: Daniil Egranov <daniil.egra...@arm.com>
> >>
> >> Add the initial set of Acpi tables for the SGI-575 platform. These
> >> tables conform to the ACPI specification version 6.1. Some of the
> >> mandatory tables required for SBBR v1.0 compilance are not included in this
> initial set of Acpi tables.
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Daniil Egranov <daniil.egra...@arm.com>
> >> Signed-off-by: Thomas Abraham <thomas.abra...@arm.com>
> >> ---
> >>  .../ARM/SgiPkg/AcpiTables/Sgi575/AcpiTables.inf    |  51 ++++++
> >>  Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dbg2.aslc    |  90 +++++++++++
> >>  Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dsdt.asl     |  99 ++++++++++++
> >>  Platform/ARM/SgiPkg/AcpiTables/Sgi575/Fadt.aslc    |  87 +++++++++++
> >>  Platform/ARM/SgiPkg/AcpiTables/Sgi575/Gtdt.aslc    | 151
> >> ++++++++++++++++++
> >>  Platform/ARM/SgiPkg/AcpiTables/Sgi575/Madt.aslc    | 173
> >> +++++++++++++++++++++
> >>  Platform/ARM/SgiPkg/AcpiTables/Sgi575/Spcr.aslc    |  77 +++++++++
> >>  .../ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c   |   7 +
> >>  .../ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf |   6 +
> >>  Platform/ARM/SgiPkg/Include/SgiAcpiHeader.h        |  41 +++++
> >>  .../ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf |   4 +
> >>  11 files changed, 786 insertions(+)
> >>  create mode 100644
> >> Platform/ARM/SgiPkg/AcpiTables/Sgi575/AcpiTables.inf
> >>  create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dbg2.aslc
> >>  create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dsdt.asl
> >>  create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Fadt.aslc
> >>  create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Gtdt.aslc
> >>  create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Madt.aslc
> >>  create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Spcr.aslc
> >>  create mode 100644 Platform/ARM/SgiPkg/Include/SgiAcpiHeader.h
> >>
>
> <snip>
>
> >> diff --git a/Platform/ARM/SgiPkg/AcpiTables/Sgi575/Gtdt.aslc
> >> b/Platform/ARM/SgiPkg/AcpiTables/Sgi575/Gtdt.aslc
> >> new file mode 100644
> >> index 0000000..40657c9
> >> --- /dev/null
> >> +++ b/Platform/ARM/SgiPkg/AcpiTables/Sgi575/Gtdt.aslc
> >> @@ -0,0 +1,151 @@
> >> +/** @file
> >> +*  Generic Timer Description Table (GTDT)
> >> +*
> >> +*  Copyright (c) 2018, ARM Limited. All rights reserved.
> >> +*
> >> +*  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
> >> +*  http://opensource.org/licenses/bsd-license.php
> >> +*
> >> +*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> >> +BASIS,
> >> +*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> >> EXPRESS OR IMPLIED.
> >> +*
> >> +**/
> >> +
> >> +#include "SgiAcpiHeader.h"
> >> +#include <Library/AcpiLib.h>
> >> +#include <Library/PcdLib.h>
> >> +#include <IndustryStandard/Acpi61.h>
> >> +
> >> +#define SGI_PLATFORM_WATCHDOG_COUNT       2
> >> +#define SGI_TIMER_FRAMES_COUNT            2
> >> +
> >> +#define SYSTEM_TIMER_BASE_ADDRESS         0xFFFFFFFFFFFFFFFF
> >> +#define GTDT_GLOBAL_FLAGS                 0
> >> +#define GTDT_GTIMER_FLAGS
> >> EFI_ACPI_6_1_GTDT_TIMER_FLAG_TIMER_INTERRUPT_POLARITY
> >> +
> >> +#define SGI_GT_BLOCK_CTL_BASE             0x2A810000
> >> +#define SGI_GT_BLOCK_FRAME1_CTL_BASE      0x2A820000
> >> +#define SGI_GT_BLOCK_FRAME1_CTL_EL0_BASE  0xFFFFFFFFFFFFFFFF
> >> +#define SGI_GT_BLOCK_FRAME1_GSIV          0x5B
> >> +
> >> +#define SGI_GT_BLOCK_FRAME0_CTL_BASE      0x2A830000
> >> +#define SGI_GT_BLOCK_FRAME0_CTL_EL0_BASE  0xFFFFFFFFFFFFFFFF
> >> +#define SGI_GT_BLOCK_FRAME0_GSIV          0x5C
> >> +
> >> +#define SGI_GTX_TIMER_FLAGS               0
> >> +#define GTX_TIMER_SECURE
> >> EFI_ACPI_6_1_GTDT_GT_BLOCK_COMMON_FLAG_SECURE_TIMER
> >> +#define GTX_TIMER_NON_SECURE              0
> >> +#define GTX_TIMER_SAVE_CONTEXT
> >> EFI_ACPI_6_1_GTDT_GT_BLOCK_COMMON_FLAG_ALWAYS_ON_CAPABILITY
> >> +#define SGI_GTX_COMMON_FLAGS_S            (GTX_TIMER_SAVE_CONTEXT |
> >> GTX_TIMER_SECURE)
> >> +#define SGI_GTX_COMMON_FLAGS_NS           (GTX_TIMER_SAVE_CONTEXT
> |
> >> GTX_TIMER_NON_SECURE)
> >> +
> >> +#define EFI_ACPI_6_1_SBSA_GENERIC_WATCHDOG_STRUCTURE_INIT(
> \
> >> +  RefreshFramePhysicalAddress, ControlFramePhysicalAddress,       \
> >> +  WatchdogTimerGSIV, WatchdogTimerFlags)                          \
> >> +  {                                                               \
> >> +    EFI_ACPI_6_1_GTDT_SBSA_GENERIC_WATCHDOG,                      \
> >> +    sizeof (EFI_ACPI_6_1_GTDT_SBSA_GENERIC_WATCHDOG_STRUCTURE),
> \
> >> +    EFI_ACPI_RESERVED_WORD,                                       \
> >> +    RefreshFramePhysicalAddress,                                  \
> >> +    ControlFramePhysicalAddress,                                  \
> >> +    WatchdogTimerGSIV,                                            \
> >> +    WatchdogTimerFlags                                            \
> >> +  }
> >> +
> >> +#pragma pack (1)
> >> +
> >> +typedef struct {
> >> +  EFI_ACPI_6_1_GENERIC_TIMER_DESCRIPTION_TABLE       Gtdt;
> >> +  EFI_ACPI_6_1_GTDT_GT_BLOCK_STRUCTURE               GtBlock;
> >> +  EFI_ACPI_6_1_GTDT_GT_BLOCK_TIMER_STRUCTURE
> >> Frames[SGI_TIMER_FRAMES_COUNT];
> >> +  EFI_ACPI_6_1_GTDT_SBSA_GENERIC_WATCHDOG_STRUCTURE
> >> +Watchdogs[SGI_PLATFORM_WATCHDOG_COUNT];
> >> +} EFI_ACPI_6_1_GENERIC_TIMER_DESCRIPTION_TABLES;
> >> +
> >> +#pragma pack ()
> >> +
> >> +STATIC EFI_ACPI_6_1_GENERIC_TIMER_DESCRIPTION_TABLES Gtdt = {
> >> +  {
> >> +    ARM_ACPI_HEADER (
> >> +      EFI_ACPI_6_1_GENERIC_TIMER_DESCRIPTION_TABLE_SIGNATURE,
> >> +      EFI_ACPI_6_1_GENERIC_TIMER_DESCRIPTION_TABLES,
> >> +      EFI_ACPI_6_1_GENERIC_TIMER_DESCRIPTION_TABLE_REVISION
> >> +    ),
> >> +    SYSTEM_TIMER_BASE_ADDRESS,                    // UINT64  
> >> PhysicalAddress
> >> +    0,                                            // UINT32  Reserved
> >> +    FixedPcdGet32 (PcdArmArchTimerSecIntrNum),    // UINT32
> >> SecurePL1TimerGSIV
> >> +    GTDT_GTIMER_FLAGS,                            // UINT32  
> >> SecurePL1TimerFlags
> >> +    FixedPcdGet32 (PcdArmArchTimerIntrNum),       // UINT32
> >> NonSecurePL1TimerGSIV
> >> +    GTDT_GTIMER_FLAGS,                            // UINT32
> NonSecurePL1TimerFlags
> >> +    FixedPcdGet32 (PcdArmArchTimerVirtIntrNum),   // UINT32
> VirtualTimerGSIV
> >> +    GTDT_GTIMER_FLAGS,                            // UINT32  
> >> VirtualTimerFlags
> >> +    FixedPcdGet32 (PcdArmArchTimerHypIntrNum),    // UINT32
> >> NonSecurePL2TimerGSIV
> >> +    GTDT_GTIMER_FLAGS,                            // UINT32
> NonSecurePL2TimerFlags
> >> +    0xFFFFFFFFFFFFFFFF,                           // UINT64
> CntReadBasePhysicalAddress
> >> +    SGI_PLATFORM_WATCHDOG_COUNT,                  // UINT32
> >> PlatformTimerCount
> >
> > This is incorrect, because doesn't take in account GT block.
>
> Thanks for your review. This has been fixed now. Can you please have a look at
> the updated patch.
>
> Thanks,
> Thomas.
>
> <snip>
IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to