Linux-ACPI Archive mirror
 help / color / mirror / Atom feed
From: Tyler Baicar <baicar@amperemail.onmicrosoft.com>
To: "ishii.shuuichir@fujitsu.com" <ishii.shuuichir@fujitsu.com>,
	'Tyler Baicar' <baicar@os.amperecomputing.com>,
	"patches@amperecomputing.com" <patches@amperecomputing.com>,
	"abdulhamid@os.amperecomputing.com" 
	<abdulhamid@os.amperecomputing.com>,
	"darren@os.amperecomputing.com" <darren@os.amperecomputing.com>,
	"catalin.marinas@arm.com" <catalin.marinas@arm.com>,
	"will@kernel.org" <will@kernel.org>,
	"maz@kernel.org" <maz@kernel.org>,
	"james.morse@arm.com" <james.morse@arm.com>,
	"alexandru.elisei@arm.com" <alexandru.elisei@arm.com>,
	"suzuki.poulose@arm.com" <suzuki.poulose@arm.com>,
	"lorenzo.pieralisi@arm.com" <lorenzo.pieralisi@arm.com>,
	"guohanjun@huawei.com" <guohanjun@huawei.com>,
	"sudeep.holla@arm.com" <sudeep.holla@arm.com>,
	"rafael@kernel.org" <rafael@kernel.org>,
	"lenb@kernel.org" <lenb@kernel.org>,
	"tony.luck@intel.com" <tony.luck@intel.com>,
	"bp@alien8.de" <bp@alien8.de>,
	"mark.rutland@arm.com" <mark.rutland@arm.com>,
	"anshuman.khandual@arm.com" <anshuman.khandual@arm.com>,
	"vincenzo.frascino@arm.com" <vincenzo.frascino@arm.com>,
	"tabba@google.com" <tabba@google.com>,
	"marcan@marcan.st" <marcan@marcan.st>,
	"keescook@chromium.org" <keescook@chromium.org>,
	"jthierry@redhat.com" <jthierry@redhat.com>,
	"masahiroy@kernel.org" <masahiroy@kernel.org>,
	"samitolvanen@google.com" <samitolvanen@google.com>,
	"john.garry@huawei.com" <john.garry@huawei.com>,
	"daniel.lezcano@linaro.org" <daniel.lezcano@linaro.org>,
	"gor@linux.ibm.com" <gor@linux.ibm.com>,
	"zhangshaokun@hisilicon.com" <zhangshaokun@hisilicon.com>,
	"tmricht@linux.ibm.com" <tmricht@linux.ibm.com>,
	"dchinner@redhat.com" <dchinner@redhat.com>,
	"tglx@linutronix.de" <tglx@linutronix.de>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"kvmarm@lists.cs.columbia.edu" <kvmarm@lists.cs.columbia.edu>,
	"linux-acpi@vger.kernel.org" <linux-acpi@vger.kernel.org>,
	"linux-edac@vger.kernel.org" <linux-edac@vger.kernel.org>,
	"Vineeth.Pillai@microsoft.com" <Vineeth.Pillai@microsoft.com>
Subject: Re: [PATCH 1/2] ACPI/AEST: Initial AEST driver
Date: Thu, 16 Dec 2021 18:33:11 -0500	[thread overview]
Message-ID: <9330bbfb-d016-0283-a5ed-e2f4d5446759@amperemail.onmicrosoft.com> (raw)
In-Reply-To: <TYCPR01MB6160D05580A6E8C9510D25A5E9709@TYCPR01MB6160.jpnprd01.prod.outlook.com>

Hi Shuuichirou,

Thank you for your feedback!

On 12/9/2021 3:10 AM, ishii.shuuichir@fujitsu.com wrote:
> Hi, Tyler.
>
> We would like to make a few comments.
>
>> -----Original Message-----
>> From: Tyler Baicar <baicar@os.amperecomputing.com>
>> Sent: Thursday, November 25, 2021 2:07 AM
>> To: patches@amperecomputing.com; abdulhamid@os.amperecomputing.com;
>> darren@os.amperecomputing.com; catalin.marinas@arm.com; will@kernel.org;
>> maz@kernel.org; james.morse@arm.com; alexandru.elisei@arm.com;
>> suzuki.poulose@arm.com; lorenzo.pieralisi@arm.com; guohanjun@huawei.com;
>> sudeep.holla@arm.com; rafael@kernel.org; lenb@kernel.org;
>> tony.luck@intel.com; bp@alien8.de; mark.rutland@arm.com;
>> anshuman.khandual@arm.com; vincenzo.frascino@arm.com;
>> tabba@google.com; marcan@marcan.st; keescook@chromium.org;
>> jthierry@redhat.com; masahiroy@kernel.org; samitolvanen@google.com;
>> john.garry@huawei.com; daniel.lezcano@linaro.org; gor@linux.ibm.com;
>> zhangshaokun@hisilicon.com; tmricht@linux.ibm.com; dchinner@redhat.com;
>> tglx@linutronix.de; linux-kernel@vger.kernel.org;
>> linux-arm-kernel@lists.infradead.org; kvmarm@lists.cs.columbia.edu;
>> linux-acpi@vger.kernel.org; linux-edac@vger.kernel.org; Ishii, Shuuichirou/石井
>> 周一郎 <ishii.shuuichir@fujitsu.com>; Vineeth.Pillai@microsoft.com
>> Cc: Tyler Baicar <baicar@os.amperecomputing.com>
>> Subject: [PATCH 1/2] ACPI/AEST: Initial AEST driver
>>
>> Add support for parsing the ARM Error Source Table and basic handling of
>> errors reported through both memory mapped and system register interfaces.
>>
>> Assume system register interfaces are only registered with private
>> peripheral interrupts (PPIs); otherwise there is no guarantee the
>> core handling the error is the core which took the error and has the
>> syndrome info in its system registers.
>>
>> Add logging for all detected errors and trigger a kernel panic if there is
>> any uncorrected error present.
>>
>> Signed-off-by: Tyler Baicar <baicar@os.amperecomputing.com>
>> ---
> [...]
>
>> +static int __init aest_init_node(struct acpi_aest_hdr *node)
>> +{
>> +	union acpi_aest_processor_data *proc_data;
>> +	union aest_node_spec *node_spec;
>> +	struct aest_node_data *data;
>> +	int ret;
>> +
>> +	data = kzalloc(sizeof(struct aest_node_data), GFP_KERNEL);
>> +	if (!data)
>> +		return -ENOMEM;
>> +
>> +	data->node_type = node->type;
>> +
>> +	node_spec = ACPI_ADD_PTR(union aest_node_spec, node,
>> node->node_specific_offset);
>> +
>> +	switch (node->type) {
>> +	case ACPI_AEST_PROCESSOR_ERROR_NODE:
>> +		memcpy(&data->data, node_spec, sizeof(struct
>> acpi_aest_processor));
>> +		break;
>> +	case ACPI_AEST_MEMORY_ERROR_NODE:
>> +		memcpy(&data->data, node_spec, sizeof(struct
>> acpi_aest_memory));
>> +		break;
>> +	case ACPI_AEST_SMMU_ERROR_NODE:
>> +		memcpy(&data->data, node_spec, sizeof(struct
>> acpi_aest_smmu));
>> +		break;
>> +	case ACPI_AEST_VENDOR_ERROR_NODE:
>> +		memcpy(&data->data, node_spec, sizeof(struct
>> acpi_aest_vendor));
>> +		break;
>> +	case ACPI_AEST_GIC_ERROR_NODE:
>> +		memcpy(&data->data, node_spec, sizeof(struct
>> acpi_aest_gic));
>> +		break;
>> +	default:
>> +		kfree(data);
>> +		return -EINVAL;
>> +	}
>> +
>> +	if (node->type == ACPI_AEST_PROCESSOR_ERROR_NODE) {
>> +		proc_data = ACPI_ADD_PTR(union acpi_aest_processor_data,
>> node_spec,
>> +					 sizeof(acpi_aest_processor));
>> +
>> +		switch (data->data.processor.resource_type) {
>> +		case ACPI_AEST_CACHE_RESOURCE:
>> +			memcpy(&data->proc_data, proc_data,
>> +			       sizeof(struct acpi_aest_processor_cache));
>> +			break;
>> +		case ACPI_AEST_TLB_RESOURCE:
>> +			memcpy(&data->proc_data, proc_data,
>> +			       sizeof(struct acpi_aest_processor_tlb));
>> +			break;
>> +		case ACPI_AEST_GENERIC_RESOURCE:
>> +			memcpy(&data->proc_data, proc_data,
>> +			       sizeof(struct acpi_aest_processor_generic));
>> +			break;
>> +		}
>> +	}
>> +
>> +	ret = aest_init_interface(node, data);
>> +	if (ret) {
>> +		kfree(data);
>> +		return ret;
>> +	}
>> +
>> +	return aest_init_interrupts(node, data);
> If aest_init_interrupts() failed, is it necessary to release
> the data pointer acquired by kzalloc?
aest_init_interrupts() returns an error if any of the interrupts in the 
interrupt list fails, but it's possible that some interrupts in the list 
registered successfully. So we attempt to keep chugging along in that 
scenario because some interrupts may be enabled and registered with the 
interface successfully. If any interrupt registration fails, there will 
be a print notifying that there was a failure when initializing that node.
>> +}
>> +
>> +static void aest_count_ppi(struct acpi_aest_hdr *node)
>> +{
>> +	struct acpi_aest_node_interrupt *interrupt;
>> +	int i;
>> +
>> +	interrupt = ACPI_ADD_PTR(struct acpi_aest_node_interrupt, node,
>> +				 node->node_interrupt_offset);
>> +
>> +	for (i = 0; i < node->node_interrupt_count; i++, interrupt++) {
>> +		if (interrupt->gsiv >= 16 && interrupt->gsiv < 32)
>> +			num_ppi++;
>> +	}
>> +}
>> +
>> +static int aest_starting_cpu(unsigned int cpu)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < num_ppi; i++)
>> +		enable_percpu_irq(ppi_irqs[i], IRQ_TYPE_NONE);
>> +
>> +	return 0;
>> +}
>> +
>> +static int aest_dying_cpu(unsigned int cpu)
>> +{
> Wouldn't it be better to execute disable_percpu_irq(), which is paired
> with enable_percpu_irq(), in aest_dying_cpu()?

Good point. I will add that in the next version.

Thanks,

Tyler


  reply	other threads:[~2021-12-16 23:33 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-24 17:07 [PATCH 0/2] ARM Error Source Table Support Tyler Baicar
2021-11-24 17:07 ` [PATCH 1/2] ACPI/AEST: Initial AEST driver Tyler Baicar
2021-11-24 18:09   ` Marc Zyngier
2021-11-29 20:39     ` Darren Hart
2021-11-30  9:45       ` Marc Zyngier
2021-11-30 16:41         ` Darren Hart
2021-12-16 22:05           ` Tyler Baicar
2021-12-16 23:42             ` Sudeep Holla
2021-11-24 18:51   ` Mark Rutland
2021-12-16 23:22     ` Tyler Baicar
2021-12-09  8:10   ` ishii.shuuichir
2021-12-16 23:33     ` Tyler Baicar [this message]
2022-04-20  7:54       ` ishii.shuuichir
2022-05-09 13:37         ` Tyler Baicar
2022-05-09 23:23           ` ishii.shuuichir
2022-12-07  5:44           ` Ruidong Tian
2021-11-24 17:07 ` [PATCH 2/2] trace, ras: add ARM RAS extension trace event Tyler Baicar
  -- strict thread matches above, loose matches on Subject: below --
2024-03-04 11:15 [PATCH 0/2] ARM Error Source Table V1 Support Ruidong Tian
2024-03-04 11:15 ` [PATCH 1/2] ACPI/AEST: Initial AEST driver Ruidong Tian
2024-03-04 12:07   ` Marc Zyngier
2024-03-08  4:49     ` Ruidong Tian
     [not found]     ` <aaad88c3-333d-4714-a9ca-3b66c8a5d9c8@linux.alibaba.com>
2024-03-09 10:33       ` Marc Zyngier
2024-03-12  9:53         ` Ruidong Tian

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=9330bbfb-d016-0283-a5ed-e2f4d5446759@amperemail.onmicrosoft.com \
    --to=baicar@amperemail.onmicrosoft.com \
    --cc=Vineeth.Pillai@microsoft.com \
    --cc=abdulhamid@os.amperecomputing.com \
    --cc=alexandru.elisei@arm.com \
    --cc=anshuman.khandual@arm.com \
    --cc=baicar@os.amperecomputing.com \
    --cc=bp@alien8.de \
    --cc=catalin.marinas@arm.com \
    --cc=daniel.lezcano@linaro.org \
    --cc=darren@os.amperecomputing.com \
    --cc=dchinner@redhat.com \
    --cc=gor@linux.ibm.com \
    --cc=guohanjun@huawei.com \
    --cc=ishii.shuuichir@fujitsu.com \
    --cc=james.morse@arm.com \
    --cc=john.garry@huawei.com \
    --cc=jthierry@redhat.com \
    --cc=keescook@chromium.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-edac@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=marcan@marcan.st \
    --cc=mark.rutland@arm.com \
    --cc=masahiroy@kernel.org \
    --cc=maz@kernel.org \
    --cc=patches@amperecomputing.com \
    --cc=rafael@kernel.org \
    --cc=samitolvanen@google.com \
    --cc=sudeep.holla@arm.com \
    --cc=suzuki.poulose@arm.com \
    --cc=tabba@google.com \
    --cc=tglx@linutronix.de \
    --cc=tmricht@linux.ibm.com \
    --cc=tony.luck@intel.com \
    --cc=vincenzo.frascino@arm.com \
    --cc=will@kernel.org \
    --cc=zhangshaokun@hisilicon.com \
    /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 read-only IMAP folder(s) and NNTP newsgroup(s).