Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755221AbdHYKie (ORCPT ); Fri, 25 Aug 2017 06:38:34 -0400 Received: from szxga05-in.huawei.com ([45.249.212.191]:4573 "EHLO szxga05-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755082AbdHYKia (ORCPT ); Fri, 25 Aug 2017 06:38:30 -0400 Subject: Re: [PATCH v11 1/6] ACPI: add APEI/HEST/CPER structures and macros To: Shannon Zhao , , , , , , , , , , , , , , , , , , , , , , , , , , , , , References: <1503066227-18251-1-git-send-email-gengdongjiu@huawei.com> <1503066227-18251-2-git-send-email-gengdongjiu@huawei.com> <599EC7AB.3080604@huawei.com> CC: , , , From: gengdongjiu Message-ID: <69090a96-ce90-4b2b-a419-c8d847d56093@huawei.com> Date: Fri, 25 Aug 2017 18:37:44 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.7.1 MIME-Version: 1.0 In-Reply-To: <599EC7AB.3080604@huawei.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.142.68.147] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A090206.599FFE07.0131,ss=1,re=0.000,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0, ip=0.0.0.0, so=2014-11-16 11:51:01, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: c1335e206e8c45edacacb8cba8c0d529 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 9555 Lines: 257 Shannon, Thanks for the review. please see my reply. On 2017/8/24 20:33, Shannon Zhao wrote: > > > On 2017/8/18 22:23, Dongjiu Geng wrote: >> (1) Add related APEI/HEST table structures and macros, these >> definition refer to ACPI 6.1 and UEFI 2.6 spec. >> (2) Add generic error status block and CPER memory section >> definition, user space only handle memory section errors. >> >> Signed-off-by: Dongjiu Geng >> --- >> include/hw/acpi/acpi-defs.h | 193 ++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 193 insertions(+) >> >> diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h >> index 72be675..3b4bad7 100644 >> --- a/include/hw/acpi/acpi-defs.h >> +++ b/include/hw/acpi/acpi-defs.h >> @@ -297,6 +297,44 @@ typedef struct AcpiMultipleApicTable AcpiMultipleApicTable; >> #define ACPI_APIC_GENERIC_TRANSLATOR 15 >> #define ACPI_APIC_RESERVED 16 /* 16 and greater are reserved */ >> >> +/* UEFI Spec 2.6, "N.2.5 Memory Error Section */ > missing " thanks for the pointing out. > >> +#define UEFI_CPER_MEM_VALID_ERROR_STATUS 0x0001 >> +#define UEFI_CPER_MEM_VALID_PA 0x0002 >> +#define UEFI_CPER_MEM_VALID_PA_MASK 0x0004 >> +#define UEFI_CPER_MEM_VALID_NODE 0x0008 >> +#define UEFI_CPER_MEM_VALID_CARD 0x0010 >> +#define UEFI_CPER_MEM_VALID_MODULE 0x0020 >> +#define UEFI_CPER_MEM_VALID_BANK 0x0040 >> +#define UEFI_CPER_MEM_VALID_DEVICE 0x0080 >> +#define UEFI_CPER_MEM_VALID_ROW 0x0100 >> +#define UEFI_CPER_MEM_VALID_COLUMN 0x0200 >> +#define UEFI_CPER_MEM_VALID_BIT_POSITION 0x0400 >> +#define UEFI_CPER_MEM_VALID_REQUESTOR 0x0800 >> +#define UEFI_CPER_MEM_VALID_RESPONDER 0x1000 >> +#define UEFI_CPER_MEM_VALID_TARGET 0x2000 >> +#define UEFI_CPER_MEM_VALID_ERROR_TYPE 0x4000 >> +#define UEFI_CPER_MEM_VALID_RANK_NUMBER 0x8000 >> +#define UEFI_CPER_MEM_VALID_CARD_HANDLE 0x10000 >> +#define UEFI_CPER_MEM_VALID_MODULE_HANDLE 0x20000 >> +#define UEFI_CPER_MEM_ERROR_TYPE_MULTI_ECC 3 >> + >> +/* From the ACPI 6.1 spec, "18.3.2.9 Hardware Error Notification" */ >> + > It's better to refer to the first spec version of this structure and > same with others you define. do you mean which spec version? the definition is aligned with the linux kernel. > >> +enum AcpiHestNotifyType { >> + ACPI_HEST_NOTIFY_POLLED = 0, >> + ACPI_HEST_NOTIFY_EXTERNAL = 1, >> + ACPI_HEST_NOTIFY_LOCAL = 2, >> + ACPI_HEST_NOTIFY_SCI = 3, >> + ACPI_HEST_NOTIFY_NMI = 4, >> + ACPI_HEST_NOTIFY_CMCI = 5, /* ACPI 5.0 */ >> + ACPI_HEST_NOTIFY_MCE = 6, /* ACPI 5.0 */ >> + ACPI_HEST_NOTIFY_GPIO = 7, /* ACPI 6.0 */ >> + ACPI_HEST_NOTIFY_SEA = 8, /* ACPI 6.1 */ >> + ACPI_HEST_NOTIFY_SEI = 9, /* ACPI 6.1 */ >> + ACPI_HEST_NOTIFY_GSIV = 10, /* ACPI 6.1 */ >> + ACPI_HEST_NOTIFY_RESERVED = 11 /* 11 and greater are reserved */ > In ACPI 6.2, 11 is for Software Delegated Exception, is this useful for > your patchset? it is usefull, for all the error source, I reserved the space for them. Because the space is allocated one time, is not dynamically allocated. so I use the ACPI_HEST_NOTIFY_RESERVED to specify that there is 11 error source. > >> +}; >> + >> /* >> * MADT sub-structures (Follow MULTIPLE_APIC_DESCRIPTION_TABLE) >> */ >> @@ -474,6 +512,161 @@ struct AcpiSystemResourceAffinityTable { >> } QEMU_PACKED; >> typedef struct AcpiSystemResourceAffinityTable AcpiSystemResourceAffinityTable; >> >> +/* Hardware Error Notification, from the ACPI 6.1 >> + * spec, "18.3.2.9 Hardware Error Notification" >> + */ > Use below style for multiple comment lines > /* > * XXX > */ you are right, thanks for the pointing out. > >> +struct AcpiHestNotify { >> + uint8_t type; >> + uint8_t length; >> + uint16_t config_write_enable; >> + uint32_t poll_interval; >> + uint32_t vector; >> + uint32_t polling_threshold_value; >> + uint32_t polling_threshold_window; >> + uint32_t error_threshold_value; >> + uint32_t error_threshold_window; >> +} QEMU_PACKED; >> +typedef struct AcpiHestNotify AcpiHestNotify; >> + >> +/* From ACPI 6.1, sections "18.3.2.1 IA-32 Architecture Machine >> + * Check Exception" through "18.3.2.8 Generic Hardware Error Source version 2". >> + */ >> +enum AcpiHestSourceType { >> + ACPI_HEST_SOURCE_IA32_CHECK = 0, >> + ACPI_HEST_SOURCE_IA32_CORRECTED_CHECK = 1, >> + ACPI_HEST_SOURCE_IA32_NMI = 2, > What's 3, 4, 5 for? the ACPI spec do not use 3, 4, 5, so we not define them. > >> + ACPI_HEST_SOURCE_AER_ROOT_PORT = 6, >> + ACPI_HEST_SOURCE_AER_ENDPOINT = 7, >> + ACPI_HEST_SOURCE_AER_BRIDGE = 8, >> + ACPI_HEST_SOURCE_GENERIC_ERROR = 9, >> + ACPI_HEST_SOURCE_GENERIC_ERROR_V2 = 10, >> + ACPI_HEST_SOURCE_RESERVED = 11 /* 11 and greater are reserved */ >> +}; >> + >> +/* Block status bitmasks from ACPI 6.1, "18.3.2.7.1 Generic Error Data" */ >> +#define ACPI_GEBS_UNCORRECTABLE (1) >> +#define ACPI_GEBS_CORRECTABLE (1 << 1) >> +#define ACPI_GEBS_MULTIPLE_UNCORRECTABLE (1 << 2) >> +#define ACPI_GEBS_MULTIPLE_CORRECTABLE (1 << 3) >> +/* 10 bits, error data entry count */ >> +#define ACPI_GEBS_ERROR_ENTRY_COUNT (0x3FF << 4) >> + >> +/* Generic Hardware Error Source Structure, refer to ACPI 6.1 >> + * "18.3.2.7 Generic Hardware Error Source". in this struct the >> + * "type" field has to be ACPI_HEST_SOURCE_GENERIC_ERROR >> + */ >> + >> +struct AcpiGenericHardwareErrorSource { >> + uint16_t type; >> + uint16_t source_id; >> + uint16_t related_source_id; >> + uint8_t flags; >> + uint8_t enabled; >> + uint32_t number_of_records; >> + uint32_t max_sections_per_record; >> + uint32_t max_raw_data_length; >> + struct AcpiGenericAddress error_status_address; >> + struct AcpiHestNotify notify; >> + uint32_t error_status_block_length; >> +} QEMU_PACKED; >> +typedef struct AcpiGenericHardwareErrorSource AcpiGenericHardwareErrorSource; >> + >> +/* Generic Hardware Error Source, version 2, ACPI 6.1, "18.3.2.8 Generic >> + * Hardware Error Source version 2", in this struct the "type" field has to >> + * be ACPI_HEST_SOURCE_GENERIC_ERROR_V2 >> + */ >> +struct AcpiGenericHardwareErrorSourceV2 { >> + uint16_t type; >> + uint16_t source_id; >> + uint16_t related_source_id; >> + uint8_t flags; >> + uint8_t enabled; >> + uint32_t number_of_records; >> + uint32_t max_sections_per_record; >> + uint32_t max_raw_data_length; >> + struct AcpiGenericAddress error_status_address; >> + struct AcpiHestNotify notify; >> + uint32_t error_status_block_length; >> + struct AcpiGenericAddress read_ack_register; >> + uint64_t read_ack_preserve; >> + uint64_t read_ack_write; >> +} QEMU_PACKED; >> +typedef struct AcpiGenericHardwareErrorSourceV2 >> + AcpiGenericHardwareErrorSourceV2; >> + >> +/* Generic Error Status block, from ACPI 6.1, >> + * "18.3.2.7.1 Generic Error Data" >> + */ >> +struct AcpiGenericErrorStatus { >> + /* It is a bitmask composed of ACPI_GEBS_xxx macros */ >> + uint32_t block_status; >> + uint32_t raw_data_offset; >> + uint32_t raw_data_length; >> + uint32_t data_length; >> + uint32_t error_severity; >> +} QEMU_PACKED; >> +typedef struct AcpiGenericErrorStatus AcpiGenericErrorStatus; >> + >> +enum AcpiGenericErrorSeverity { >> + ACPI_CPER_SEV_RECOVERABLE, >> + ACPI_CPER_SEV_FATAL, >> + ACPI_CPER_SEV_CORRECTED, >> + ACPI_CPER_SEV_NONE, >> +}; >> + >> +/* Generic Error Data entry, revision number is 0x0300, >> + * ACPI 6.1, "18.3.2.7.1 Generic Error Data" >> + */ >> +struct AcpiGenericErrorData { >> + uint8_t section_type_le[16]; >> + /* The "error_severity" fields that they take their >> + * values from AcpiGenericErrorSeverity >> + */ >> + uint32_t error_severity; >> + uint16_t revision; >> + uint8_t validation_bits; >> + uint8_t flags; >> + uint32_t error_data_length; >> + uint8_t fru_id[16]; >> + uint8_t fru_text[20]; >> + uint64_t time_stamp; >> +} QEMU_PACKED; >> +typedef struct AcpiGenericErrorData AcpiGenericErrorData; >> + >> +/* From UEFI 2.6, "N.2.5 Memory Error Section" */ >> +struct UefiCperSecMemErr { >> + uint64_t validation_bits; >> + uint64_t error_status; >> + uint64_t physical_addr; >> + uint64_t physical_addr_mask; >> + uint16_t node; >> + uint16_t card; >> + uint16_t module; >> + uint16_t bank; >> + uint16_t device; >> + uint16_t row; >> + uint16_t column; >> + uint16_t bit_pos; >> + uint64_t requestor_id; >> + uint64_t responder_id; >> + uint64_t target_id; >> + uint8_t error_type; >> + uint8_t reserved; >> + uint16_t rank; >> + uint16_t mem_array_handle; /* card handle in UEFI 2.4 */ >> + uint16_t mem_dev_handle; /* module handle in UEFI 2.4 */ >> +} QEMU_PACKED; >> +typedef struct UefiCperSecMemErr UefiCperSecMemErr; >> + >> +/* >> + * HEST Description Table >> + */ >> +struct AcpiHardwareErrorSourceTable { >> + ACPI_TABLE_HEADER_DEF /* ACPI common table header */ >> + uint32_t error_source_count; >> +} QEMU_PACKED; >> +typedef struct AcpiHardwareErrorSourceTable AcpiHardwareErrorSourceTable; >> + >> #define ACPI_SRAT_PROCESSOR_APIC 0 >> #define ACPI_SRAT_MEMORY 1 >> #define ACPI_SRAT_PROCESSOR_x2APIC 2 >> >