From: gengdongjiu <gengdongjiu@huawei.com>
To: James Morse <james.morse@arm.com>
Cc: <christoffer.dall@linaro.org>, <marc.zyngier@arm.com>,
<rkrcmar@redhat.com>, <linux@armlinux.org.uk>,
<catalin.marinas@arm.com>, <will.deacon@arm.com>,
<lenb@kernel.org>, <robert.moore@intel.com>, <lv.zheng@intel.com>,
<mark.rutland@arm.com>, <xiexiuqi@huawei.com>,
<cov@codeaurora.org>, <david.daney@cavium.com>,
<suzuki.poulose@arm.com>, <stefan@hello-penguin.com>,
<Dave.Martin@arm.com>, <kristina.martsenko@arm.com>,
<wangkefeng.wang@huawei.com>, <tbaicar@codeaurora.org>,
<ard.biesheuvel@linaro.org>, <mingo@kernel.org>, <bp@suse.de>,
<shiju.jose@huawei.com>, <zjzhang@codeaurora.org>,
<linux-arm-kernel@lists.infradead.org>,
<kvmarm@lists.cs.columbia.edu>, <kvm@vger.kernel.org>,
<linux-kernel@vger.kernel.org>, <linux-acpi@vger.kernel.org>,
<devel@acpica.org>, <mst@redhat.com>, <john.garry@huawei.com>,
<jonathan.cameron@huawei.com>,
<shameerali.kolothum.thodi@huawei.com>,
<huangdaode@hisilicon.com>, <wangzhou1@hisilicon.com>,
<huangshaoyu@huawei.com>, <wuquanming@huawei.com>,
<linuxarm@huawei.com>, <zhengqiang10@huawei.com>
Subject: Re: [PATCH v6 3/7] acpi: apei: remove the unused code
Date: Mon, 11 Sep 2017 20:04:21 +0800 [thread overview]
Message-ID: <b06c680c-93a3-7e20-1118-c6a9a24704df@huawei.com> (raw)
In-Reply-To: <59B2DEBA.6030608@arm.com>
James,
Thanks for the review.
On 2017/9/9 2:17, James Morse wrote:
> Hi gengdongjiu,
>
> On 04/09/17 12:43, gengdongjiu wrote:
>> On 2017/9/1 1:50, James Morse wrote:
>>> On 28/08/17 11:38, Dongjiu Geng wrote:
>>>> In current code logic, the two functions ghes_sea_add() and
>>>> ghes_sea_remove() are only called when CONFIG_ACPI_APEI_SEA
>>>> is defined. If not, it will return errors in the ghes_probe()
>>>> and not contiue. Hence, remove the unnecessary handling when
>>>> CONFIG_ACPI_APEI_SEI is not defined.
>>>
>>> This doesn't match what the patch does. I get this feeling this is needed for
>>> some future patch you haven't included.
>>
>> James, let check the code, when the ghes_probe, if the CONFIG_ACPI_APEI_SEA is not defined.
>> it will return -ENOTSUPP and goto error, and the ghes_sea_add has no chance to execute.
>> similar, if the probe is failed, it should not have chance to execute ghes_sea_remove.
>
> It's the 'unnecessary handling when CONFIG_ACPI_APEI_SEI' in the commit message
> that confuses me: this patch doesn't reference that Kconfig symbol. I guess that
> sentence needs removing for this v6?
thanks for the pointing out, That needs to be removed for v6.
>
> Re-reading without that part of the commit-message:
>
> You're relying on the compiler's dead-code elimination to spot unused static
> functions and silently drop them. Great!
> (there is the small risk that gcc 3.2[0] can't do this, x86 still has to support
> this gcc version)
>
> As this is just clean-up patch can you break it out of this series, it isn't
> needed to add support for SEI.
sure, I will.
>
> (This series adds support for what should be an APEI notification, but the only
> code that touches APEI removes some code from a different notification method.)
understand.
>
>
>>> Setting NOTIFY_SEI as the GHES entry's notification type means the OS should
>>> check the GHES->ErrorStatusAddress for CPER records when it receives an
>>> SError-Interrupt, as it may be a notification of an error from this error source.
>
>> previously I added the NOTIFY_SEI support,
>
> (Yes, I saw that in v5 and expected this series to add some APEI support code )
>
>
>> but consider the error address in CPER is not accurate and calling memory_failure() may not make sense.
>> so I remove it.
>
> 'not accurate'... this is going to be a problem, but lets keep that discussion
> on the cover-letter.
Ok.
>
>
>>> If you aren't handling the notification, why is this is in the HEST at all?
>>> (and if its not: its not firmware-first)
>
>> For the SEI notification, may be we can parse and handle the CPER record other than the Error physical address
>
> Sure, but I only see this cleanup patch in this series, where does APEI learn
> about NOTIFY_SEI? As this is nothing will ever touch those CPER records, if
> you're using GHESv2 firmware will be prevented from delivering subsequent
> notifications.
James, whether it is possible you can review the previous v5 patch which adds the support for NOTIFY_SEI? thanks in advancecIn that patch, I share the SEI notification handling with the SEA notification handling to avoid duplicated code.
https://www.spinics.net/lists/arm-kernel/msg601767.html
>
>
> Thanks,
>
> James
>
> [0]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/admin-guide/README.rst#n251
>
> .
>
next prev parent reply other threads:[~2017-09-11 12:09 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-28 10:38 [PATCH v6 0/7] Add RAS virtualization support for SEA/SEI notification type in KVM Dongjiu Geng
2017-08-28 10:38 ` [PATCH v6 1/7] arm64: cpufeature: Detect CPU RAS Extentions Dongjiu Geng
2017-08-31 17:44 ` James Morse
2017-09-04 11:20 ` gengdongjiu
2017-08-28 10:38 ` [PATCH v6 2/7] KVM: arm64: Save ESR_EL2 on guest SError Dongjiu Geng
2017-08-28 10:38 ` [PATCH v6 3/7] acpi: apei: remove the unused code Dongjiu Geng
2017-08-31 17:50 ` James Morse
2017-09-04 11:43 ` gengdongjiu
2017-09-08 18:17 ` James Morse
2017-09-11 12:04 ` gengdongjiu [this message]
2017-09-14 12:35 ` James Morse
2017-09-14 12:51 ` gengdongjiu
2017-08-28 10:38 ` [PATCH v6 4/7] arm64: kvm: support user space to query RAS extension feature Dongjiu Geng
2017-08-31 18:04 ` James Morse
2017-09-05 7:18 ` gengdongjiu
2017-09-07 16:31 ` James Morse
2017-08-28 10:38 ` [PATCH v6 5/7] arm64: kvm: route synchronous external abort exceptions to el2 Dongjiu Geng
2017-09-07 16:31 ` James Morse
2017-09-13 8:12 ` gengdongjiu
2017-09-14 11:12 ` gengdongjiu
2017-09-14 12:36 ` James Morse
2017-10-16 11:44 ` James Morse
2017-10-16 13:44 ` gengdongjiu
2017-08-28 10:38 ` [PATCH v6 6/7] KVM: arm64: allow get exception information from userspace Dongjiu Geng
2017-09-07 16:30 ` James Morse
2017-09-13 7:32 ` gengdongjiu
2017-09-14 13:00 ` James Morse
2017-09-18 13:36 ` gengdongjiu
2017-09-22 16:39 ` James Morse
2017-09-21 7:55 ` gengdongjiu
2017-09-22 16:51 ` James Morse
2017-09-27 11:07 ` gengdongjiu
2017-09-27 15:37 ` gengdongjiu
2017-10-06 17:31 ` James Morse
2017-10-19 7:49 ` gengdongjiu
2017-08-28 10:38 ` [PATCH v6 7/7] arm64: kvm: handle SEI notification and pass the virtual syndrome Dongjiu Geng
2017-08-31 17:43 ` [PATCH v6 0/7] Add RAS virtualization support for SEA/SEI notification type in KVM James Morse
2017-09-04 11:10 ` gengdongjiu
2017-09-07 16:32 ` James Morse
2017-09-06 11:19 ` Peter Maydell
2017-09-06 11:29 ` gengdongjiu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=b06c680c-93a3-7e20-1118-c6a9a24704df@huawei.com \
--to=gengdongjiu@huawei.com \
--cc=Dave.Martin@arm.com \
--cc=ard.biesheuvel@linaro.org \
--cc=bp@suse.de \
--cc=catalin.marinas@arm.com \
--cc=christoffer.dall@linaro.org \
--cc=cov@codeaurora.org \
--cc=david.daney@cavium.com \
--cc=devel@acpica.org \
--cc=huangdaode@hisilicon.com \
--cc=huangshaoyu@huawei.com \
--cc=james.morse@arm.com \
--cc=john.garry@huawei.com \
--cc=jonathan.cameron@huawei.com \
--cc=kristina.martsenko@arm.com \
--cc=kvm@vger.kernel.org \
--cc=kvmarm@lists.cs.columbia.edu \
--cc=lenb@kernel.org \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@armlinux.org.uk \
--cc=linuxarm@huawei.com \
--cc=lv.zheng@intel.com \
--cc=marc.zyngier@arm.com \
--cc=mark.rutland@arm.com \
--cc=mingo@kernel.org \
--cc=mst@redhat.com \
--cc=rkrcmar@redhat.com \
--cc=robert.moore@intel.com \
--cc=shameerali.kolothum.thodi@huawei.com \
--cc=shiju.jose@huawei.com \
--cc=stefan@hello-penguin.com \
--cc=suzuki.poulose@arm.com \
--cc=tbaicar@codeaurora.org \
--cc=wangkefeng.wang@huawei.com \
--cc=wangzhou1@hisilicon.com \
--cc=will.deacon@arm.com \
--cc=wuquanming@huawei.com \
--cc=xiexiuqi@huawei.com \
--cc=zhengqiang10@huawei.com \
--cc=zjzhang@codeaurora.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).