Re: [PATCH edk2-platforms v1 30/38] Hisilicon/D06: add apei driver

[Leif Lindholm <leif.lindholm@linaro.org>]

On Fri, Aug 10, 2018 at 10:46:34AM +0800, Ming wrote:
> >> --- /dev/null
> >> +++ b/Silicon/Hisilicon/Hi1620/Drivers/Apei/Apei.inf

> >> +[sources.common]
> >> +  Apei.c
> > 
> > Apei.h?
> 
> As Apei.h is included by Apei.c, so I think Apei.h
> is no need to add here.

Will the build system still generate correct build dependencies if
Apei.h is modified?

> >> +  Bert/bert.c
> >> +  Bert/bert.h

And if so, why are these other .h files explicitly specified?

> >> +  Einj/einj.c
> >> +  Einj/einj.h
> >> +  Erst/erst.c
> >> +  Erst/erst.h
> >> +  Hest/hest.c
> >> +  Hest/hest.h
> >> +  ErrorSource/Ghes.c
> >> +  ErrorSource/Ghes.h
> >> +  OemApeiHi1620.c

> >> diff --git a/Silicon/Hisilicon/Hi1620/Drivers/Apei/Einj/einj.h b/Silicon/Hisilicon/Hi1620/Drivers/Apei/Einj/einj.h
> >> new file mode 100644
> >> index 0000000000..a5342b600f
> >> --- /dev/null
> >> +++ b/Silicon/Hisilicon/Hi1620/Drivers/Apei/Einj/einj.h
> >> @@ -0,0 +1,154 @@
> >> +/** @file
> >> +*
> >> +*  Copyright (c) 2017 Hisilicon 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.
> >> +*
> >> +**/
> >> +
> >> +#ifndef _EINJ_H_
> >> +#define _EINJ_H_
> >> +
> >> +#include "Apei.h"
> >> +
> >> +#define EINJ_ACTION_NO             10
> >> +#define EINJ_BEGIN_OPERATION_VALUE 0xFFFF
> >> +#define EINJ_END_OPERATION_VALUE   0
> >> +#define EINJ_WRITE_MASK            0xFFFFFFFF
> >> +#define EINJ_READ_VALUE            0xFFFF
> >> +#define EINJ_READ_MASK             0xFFFFFFFF
> >> +
> >> +#define EINJ_TRIGGER_ERROR_ACTION_NO                                    1
> >> +
> >> +#define EFI_ACPI_EINJ_SET_ERROR_TYPE_WITH_ADDRESS                       0x08
> >> +#define EFI_ACPI_EINJ_GET_EXCUTE_OPERATION_TIMINGS                      0x09
> >> +
> >> +
> >> +extern EFI_ACPI_TABLE_PROTOCOL            *mAcpiTableProtocol;
> >> +extern EFI_ACPI_SDT_PROTOCOL              *mAcpiSdtProtocol;
> >> +extern APEI_TRUSTFIRMWARE_STRUCTURE       *mApeiTrustfirmwareStruc;
> >> +
> >> +//
> >> +// Error Type Definition
> >> +//
> >> +#define EINJ_PROCESSOR_CORRECTABLE                                      BIT0
> >> +#define EINJ_PROCESSOR_UNCORRECTABLE_NONFATAL                           BIT1
> >> +#define EINJ_PROCESSOR_UNCORRECTABLE_FATAL                              BIT2
> >> +#define EINJ_MEMORY_CORRECTABLE                                         BIT3
> >> +#define EINJ_MEMORY_UNCORRECTABLE_NONFATAL                              BIT4
> >> +#define EINJ_MEMORY_UNCORRECTABLE_FATAL                                 BIT5
> >> +#define EINJ_PCIE_CORRECTABLE                                           BIT6
> >> +#define EINJ_PCIE_UNCORRECTABLE_NONFATAL                                BIT7
> >> +#define EINJ_PCIE_UNCORRECTABLE_FATAL                                   BIT8
> >> +#define EINJ_PLATFORM_CORRECTABLE                                       BIT9
> >> +#define EINJ_PLATFORM_UNCORRECTABLE_NONFATAL                            BIT10
> >> +#define EINJ_PLATFORM_UNCORRECTABLE_FATAL                               BIT11
> >> +#define EINJ_VENDOR_DEFINED_ERROR_TYPE                                  BIT31
> >> +
> >> +//
> >> +#define EINJ_PROCESSOR_APIC_VALID                                       BIT0
> >> +#define EINJ_MEMORY_ADDRESS_VALID                                       BIT1
> >> +#define EINJ_PCIE_SBDF_VALID                                            BIT2
> >> +
> >> +//CPER
> >> +#define EFI_GENERIC_ERROR_PROC_TYPE_ARM                                 0x02
> >> +
> >> +#define EFI_GENERIC_ERROR_PROC_ISA_ARM32                                0x03
> >> +#define EFI_GENERIC_ERROR_PROC_ISA_ARM64                                0x04
> > 
> > Surely these should be added to MdePkg/Include/Guid/Cper.h instead?
> > Please submit a patch.
> 
> OK, should this macro be add to MdePkg/Include/Guid/Cper.h ?
> EFI_GENERIC_ERROR_PROC_TYPE_ARM

Yes please - I could have been more clear on that.

/
    Leif