Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1211436 > unrolled thread

Re: [PATCH 2/2] acpi, apei: use appropriate pgprot_t to map GHES memory

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 2/2] acpi, apei: use appropriate pgprot_t to map GHES  memory Ingo Molnar <mingo@kernel.org> - 2015-08-22 11:30 +0200
    Re: [PATCH 2/2] acpi, apei: use appropriate pgprot_t to map GHES  memory "Zhang, Jonathan Zhixiong" <zjzhang@codeaurora.org> - 2015-08-24 20:30 +0200
      Re: [PATCH 2/2] acpi, apei: use appropriate pgprot_t to map GHES  memory Ingo Molnar <mingo@kernel.org> - 2015-08-25 11:10 +0200
        Re: [PATCH 2/2] acpi, apei: use appropriate pgprot_t to map GHES  memory "Zhang, Jonathan Zhixiong" <zjzhang@codeaurora.org> - 2015-08-25 19:40 +0200

#1211436 — Re: [PATCH 2/2] acpi, apei: use appropriate pgprot_t to map GHES memory

* Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org> wrote:

> From: "Jonathan (Zhixiong) Zhang" <zjzhang@codeaurora.org>
> 
> With ACPI APEI firmware first handling, generic hardware error
> record is updated by firmware in GHES memory region. On an arm64
> platform, firmware updates GHES memory region with uncached
> access attribute, and then Linux reads stale data from cache.

This paragraph *still* doesn't parse for me. It's not any English
I can recognize: what is a 'With ACPI APEI firmware first handling'?

> With current code, GHES memory region is mapped with PAGE_KERNEL
> based on the assumption that cache coherency of GHES memory region
> is maintained by firmware on all platforms. This assumption is
> not true for above mentioned arm64 platform.
> 
> Instead GHES memory region should be mapped with page protection type
> according to what is returned from arch_apei_get_mem_attribute().

... plus what this changelog still doesn't mention is the most important part of 
any bug fix description: how does the user notice this in practice and why does he 
care?

Thanks,

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

[toc] | [next] | [standalone]

#1212401

On 8/22/2015 2:24 AM, Ingo Molnar wrote:
>
> * Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org> wrote:
>
>> From: "Jonathan (Zhixiong) Zhang" <zjzhang@codeaurora.org>
>>
>> With ACPI APEI firmware first handling, generic hardware error
>> record is updated by firmware in GHES memory region. On an arm64
>> platform, firmware updates GHES memory region with uncached
>> access attribute, and then Linux reads stale data from cache.
>
> This paragraph *still* doesn't parse for me. It's not any English
> I can recognize: what is a 'With ACPI APEI firmware first handling'?
APEI is ACPI Platform Error Interface; it is part of ACPI spec,
defining the aspect of hardware error handling. "firmware first
handling" is a terminology used in APEI. It describes such mechanism
that when hardware error happens, firmware intersects/handles such
hardware error, formulates hardware error record and writes the record
to GHES memory region, notifies the kernel through NMI/interrupt, then
the kernel GHES driver grabs the error record from the GHES memory
region.

>
>> With current code, GHES memory region is mapped with PAGE_KERNEL
>> based on the assumption that cache coherency of GHES memory region
>> is maintained by firmware on all platforms. This assumption is
>> not true for above mentioned arm64 platform.
>>
>> Instead GHES memory region should be mapped with page protection type
>> according to what is returned from arch_apei_get_mem_attribute().
>
> ... plus what this changelog still doesn't mention is the most important part of
> any bug fix description: how does the user notice this in practice and why does he
> care?
The changelog mentioned that Linux would read stale data from cache.
When stale data is read, kernel reports there is no new hardware error
when there actually is. This may lead to further damage in various
scenarios, such as error propagation caused data corruption.
>
> Thanks,
>
> 	Ingo
> --
> To unsubscribe from this list: send the line "unsubscribe linux-efi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

-- 
Jonathan (Zhixiong) Zhang
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1212861

* Zhang, Jonathan Zhixiong <zjzhang@codeaurora.org> wrote:

> 
> 
> On 8/22/2015 2:24 AM, Ingo Molnar wrote:
> >
> >* Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org> wrote:
> >
> >>From: "Jonathan (Zhixiong) Zhang" <zjzhang@codeaurora.org>
> >>
> >>With ACPI APEI firmware first handling, generic hardware error
> >>record is updated by firmware in GHES memory region. On an arm64
> >>platform, firmware updates GHES memory region with uncached
> >>access attribute, and then Linux reads stale data from cache.
> >
> >This paragraph *still* doesn't parse for me. It's not any English
> >I can recognize: what is a 'With ACPI APEI firmware first handling'?
> APEI is ACPI Platform Error Interface; it is part of ACPI spec,
> defining the aspect of hardware error handling. "firmware first
> handling" is a terminology used in APEI. It describes such mechanism
> that when hardware error happens, firmware intersects/handles such
> hardware error, formulates hardware error record and writes the record
> to GHES memory region, notifies the kernel through NMI/interrupt, then
> the kernel GHES driver grabs the error record from the GHES memory
> region.

Argh. So how about translating that to English and putting that misnomer into 
scare quotes, and saying something like:

  If the ACPI APEI firmware handles the error first (called "firmware first 
  handling"), the generic hardware error record is updated by the firmware in the 
  GHES memory region.

( Also note all the missing articles I added for readability. The rest of the 
  changelog is missing articles as well. )

> > ... plus what this changelog still doesn't mention is the most important part 
> > of any bug fix description: how does the user notice this in practice and why 
> > does he care?
>
> The changelog mentioned that Linux would read stale data from cache. When stale 
> data is read, kernel reports there is no new hardware error when there actually 
> is.

Note that this is the most valuable sentence so far, in this whole changelog and 
discussion. And we needed how many emails to get to this point?

obviously saying 'stale data' in itself does not mean much - it could mean a 
harmless inconsistency nobody really cares about, or in fact it could mean 
something more serious:

> [...] This may lead to further damage in various scenarios, such as error 
> propagation caused data corruption.

Please outline this better. How users are affected in practice is far more 
important than any other detail.

Thanks,

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

[toc] | [prev] | [next] | [standalone]

#1213204

On 8/25/2015 1:59 AM, Ingo Molnar wrote:
>
> * Zhang, Jonathan Zhixiong <zjzhang@codeaurora.org> wrote:
>
>>
>>
>> On 8/22/2015 2:24 AM, Ingo Molnar wrote:
>>>
>>> * Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org> wrote:
>>>
>>>> From: "Jonathan (Zhixiong) Zhang" <zjzhang@codeaurora.org>
>>>>
>>>> With ACPI APEI firmware first handling, generic hardware error
>>>> record is updated by firmware in GHES memory region. On an arm64
>>>> platform, firmware updates GHES memory region with uncached
>>>> access attribute, and then Linux reads stale data from cache.
>>>
>>> This paragraph *still* doesn't parse for me. It's not any English
>>> I can recognize: what is a 'With ACPI APEI firmware first handling'?
>> APEI is ACPI Platform Error Interface; it is part of ACPI spec,
>> defining the aspect of hardware error handling. "firmware first
>> handling" is a terminology used in APEI. It describes such mechanism
>> that when hardware error happens, firmware intersects/handles such
>> hardware error, formulates hardware error record and writes the record
>> to GHES memory region, notifies the kernel through NMI/interrupt, then
>> the kernel GHES driver grabs the error record from the GHES memory
>> region.
>
> Argh. So how about translating that to English and putting that misnomer into
> scare quotes, and saying something like:
>
>    If the ACPI APEI firmware handles the error first (called "firmware first
>    handling"), the generic hardware error record is updated by the firmware in the
>    GHES memory region.
>
> ( Also note all the missing articles I added for readability. The rest of the
>    changelog is missing articles as well. )
Thank you very much, Ingo. Input are taken.
>
>>> ... plus what this changelog still doesn't mention is the most important part
>>> of any bug fix description: how does the user notice this in practice and why
>>> does he care?
>>
>> The changelog mentioned that Linux would read stale data from cache. When stale
>> data is read, kernel reports there is no new hardware error when there actually
>> is.
>
> Note that this is the most valuable sentence so far, in this whole changelog and
> discussion. And we needed how many emails to get to this point?
>
> obviously saying 'stale data' in itself does not mean much - it could mean a
> harmless inconsistency nobody really cares about, or in fact it could mean
> something more serious:
Sure, makes sense.
>
>> [...] This may lead to further damage in various scenarios, such as error
>> propagation caused data corruption.
>
> Please outline this better. How users are affected in practice is far more
> important than any other detail.
Yes, will do. I just sent out an update for your review.
>
> Thanks,
>
> 	Ingo
>

-- 
Jonathan (Zhixiong) Zhang
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web