linux-kernel - Re: [PATCH v4 2/3] acpi, apei, ghes: Make NMI error notification to be GHES architecture extension.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <53B64F3C.6080502@linaro.org>
Date:	Fri, 04 Jul 2014 08:52:44 +0200
From:	Tomasz Nowicki <tomasz.nowicki@...aro.org>
To:	Borislav Petkov <bp@...en8.de>
CC:	rjw@...ysocki.net, lenb@...nel.org, tony.luck@...el.com,
	m.chehab@...sung.com, bp@...e.de, linux-edac@...r.kernel.org,
	x86@...nel.org, linux-acpi@...r.kernel.org,
	linux-kernel@...r.kernel.org, linaro-acpi@...ts.linaro.org,
	rric@...nel.org
Subject: Re: [PATCH v4 2/3] acpi, apei, ghes: Make NMI error notification
 to be GHES architecture extension.

On 02.07.2014 14:16, Borislav Petkov wrote:
> On Tue, Jun 24, 2014 at 01:41:34PM +0200, Tomasz Nowicki wrote:
>> Currently APEI depends on x86 architecture. It is because of NMI hardware
>> error notification of GHES which is currently supported by x86 only.
>> However, many other APEI features can be still used perfectly by other
>> architectures.
>>
>> This commit adds two symbols:
>> 1. HAVE_ACPI_APEI for those archs which support APEI.
>> 2. HAVE_ACPI_APEI_NMI which is used for NMI code isolation in ghes.c
>>     file. NMI related data and functions are grouped so they can be wrapped
>>     inside one #ifdef section. Appropriate function stubs are provided for
>>     !NMI case.
>>
>> Note there is no functional changes for x86 due to hard selected
>> HAVE_ACPI_APEI and HAVE_ACPI_APEI_NMI symbols.
>>
>> Signed-off-by: Tomasz Nowicki <tomasz.nowicki@...aro.org>
>> ---
>>   arch/x86/Kconfig          |    2 +
>>   drivers/acpi/apei/Kconfig |    8 ++-
>>   drivers/acpi/apei/ghes.c  |  149 +++++++++++++++++++++++++++++----------------
>>   include/linux/nmi.h       |    4 ++
>>   4 files changed, 110 insertions(+), 53 deletions(-)
>>
>
> ...
>
>> @@ -894,11 +892,71 @@ static unsigned long ghes_esource_prealloc_size(
>>   	return prealloc_size;
>>   }
>>
>> +static void ghes_estatus_pool_shrink(unsigned long len)
>> +{
>> +	ghes_estatus_pool_size_request -= PAGE_ALIGN(len);
>> +}
>> +
>> +static void ghes_nmi_add(struct ghes *ghes)
>> +{
>> +	unsigned long len;
>> +
>> +	len = ghes_esource_prealloc_size(ghes->generic);
>> +	ghes_estatus_pool_expand(len);
>> +	mutex_lock(&ghes_list_mutex);
>> +	if (list_empty(&ghes_nmi))
>> +		register_nmi_handler(NMI_LOCAL, ghes_notify_nmi, 0, "ghes");
>> +	list_add_rcu(&ghes->list, &ghes_nmi);
>> +	mutex_unlock(&ghes_list_mutex);
>> +}
>> +
>> +static void ghes_nmi_remove(struct ghes *ghes)
>> +{
>> +	unsigned long len;
>> +
>> +	mutex_lock(&ghes_list_mutex);
>> +	list_del_rcu(&ghes->list);
>> +	if (list_empty(&ghes_nmi))
>> +		unregister_nmi_handler(NMI_LOCAL, "ghes");
>> +	mutex_unlock(&ghes_list_mutex);
>> +	/*
>> +	 * To synchronize with NMI handler, ghes can only be
>> +	 * freed after NMI handler finishes.
>> +	 */
>> +	synchronize_rcu();
>> +	len = ghes_esource_prealloc_size(ghes->generic);
>> +	ghes_estatus_pool_shrink(len);
>> +}
>> +
>> +static void ghes_nmi_init_cxt(void)
>> +{
>> +	init_irq_work(&ghes_proc_irq_work, ghes_proc_in_irq);
>> +}
>> +#else
>
> Markers please to know to which CONFIG_ symbol this #else refers to:
>
> #else /* CONFIG_HAVE_ACPI_APEI_NMI */
>
>> +static inline void ghes_nmi_add(struct ghes *ghes)
>> +{
>> +	pr_err(GHES_PFX "ID: %d, trying to add NMI notification which is not supported!\n",
>> +	       ghes->generic->header.source_id);
>> +	BUG();
>> +}
>> +
>> +static inline void ghes_nmi_remove(struct ghes *ghes)
>> +{
>> +	pr_err(GHES_PFX "ID: %d, trying to remove NMI notification which is not supported!\n",
>> +	       ghes->generic->header.source_id);
>> +	BUG();
>> +}
>> +
>> +static inline void ghes_nmi_init_cxt(void)
>> +{
>> +}
>> +#endif
>
> Ditto.
>

Right, I will fix that and thanks for all you acks.

BTW, who else should I asked for review to move upstream process one 
step further?

Regards,
Tomasz
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/