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: <TYCPR01MB616007723D2C8BA08F5337D2E9F59@TYCPR01MB6160.jpnprd01.prod.outlook.com>
Date:   Wed, 20 Apr 2022 07:54:37 +0000
From:   "ishii.shuuichir@...itsu.com" <ishii.shuuichir@...itsu.com>
To:     'Tyler Baicar' <baicar@...eremail.onmicrosoft.com>,
        'Tyler Baicar' <baicar@...amperecomputing.com>,
        "patches@...erecomputing.com" <patches@...erecomputing.com>,
        "abdulhamid@...amperecomputing.com" 
        <abdulhamid@...amperecomputing.com>,
        "darren@...amperecomputing.com" <darren@...amperecomputing.com>,
        "catalin.marinas@....com" <catalin.marinas@....com>,
        "will@...nel.org" <will@...nel.org>,
        "maz@...nel.org" <maz@...nel.org>,
        "james.morse@....com" <james.morse@....com>,
        "alexandru.elisei@....com" <alexandru.elisei@....com>,
        "suzuki.poulose@....com" <suzuki.poulose@....com>,
        "lorenzo.pieralisi@....com" <lorenzo.pieralisi@....com>,
        "guohanjun@...wei.com" <guohanjun@...wei.com>,
        "sudeep.holla@....com" <sudeep.holla@....com>,
        "rafael@...nel.org" <rafael@...nel.org>,
        "lenb@...nel.org" <lenb@...nel.org>,
        "tony.luck@...el.com" <tony.luck@...el.com>,
        "bp@...en8.de" <bp@...en8.de>,
        "mark.rutland@....com" <mark.rutland@....com>,
        "anshuman.khandual@....com" <anshuman.khandual@....com>,
        "vincenzo.frascino@....com" <vincenzo.frascino@....com>,
        "tabba@...gle.com" <tabba@...gle.com>,
        "marcan@...can.st" <marcan@...can.st>,
        "keescook@...omium.org" <keescook@...omium.org>,
        "jthierry@...hat.com" <jthierry@...hat.com>,
        "masahiroy@...nel.org" <masahiroy@...nel.org>,
        "samitolvanen@...gle.com" <samitolvanen@...gle.com>,
        "john.garry@...wei.com" <john.garry@...wei.com>,
        "daniel.lezcano@...aro.org" <daniel.lezcano@...aro.org>,
        "gor@...ux.ibm.com" <gor@...ux.ibm.com>,
        "zhangshaokun@...ilicon.com" <zhangshaokun@...ilicon.com>,
        "tmricht@...ux.ibm.com" <tmricht@...ux.ibm.com>,
        "dchinner@...hat.com" <dchinner@...hat.com>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "kvmarm@...ts.cs.columbia.edu" <kvmarm@...ts.cs.columbia.edu>,
        "linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
        "linux-edac@...r.kernel.org" <linux-edac@...r.kernel.org>,
        "Vineeth.Pillai@...rosoft.com" <Vineeth.Pillai@...rosoft.com>
CC:     "ishii.shuuichir@...itsu.com" <ishii.shuuichir@...itsu.com>
Subject: RE: [PATCH 1/2] ACPI/AEST: Initial AEST driver

Hi, Tyler.

When do you plan to post the v2 patch series?
Please let me know if you don't mind.

Best regards.

> -----Original Message-----
> From: Tyler Baicar <baicar@...eremail.onmicrosoft.com>
> Sent: Friday, December 17, 2021 8:33 AM
> To: Ishii, Shuuichirou/石井 周一郎 <ishii.shuuichir@...itsu.com>; 'Tyler Baicar'
> <baicar@...amperecomputing.com>; patches@...erecomputing.com;
> abdulhamid@...amperecomputing.com; darren@...amperecomputing.com;
> catalin.marinas@....com; will@...nel.org; maz@...nel.org;
> james.morse@....com; alexandru.elisei@....com; suzuki.poulose@....com;
> lorenzo.pieralisi@....com; guohanjun@...wei.com; sudeep.holla@....com;
> rafael@...nel.org; lenb@...nel.org; tony.luck@...el.com; bp@...en8.de;
> mark.rutland@....com; anshuman.khandual@....com;
> vincenzo.frascino@....com; tabba@...gle.com; marcan@...can.st;
> keescook@...omium.org; jthierry@...hat.com; masahiroy@...nel.org;
> samitolvanen@...gle.com; john.garry@...wei.com; daniel.lezcano@...aro.org;
> gor@...ux.ibm.com; zhangshaokun@...ilicon.com; tmricht@...ux.ibm.com;
> dchinner@...hat.com; tglx@...utronix.de; linux-kernel@...r.kernel.org;
> linux-arm-kernel@...ts.infradead.org; kvmarm@...ts.cs.columbia.edu;
> linux-acpi@...r.kernel.org; linux-edac@...r.kernel.org;
> Vineeth.Pillai@...rosoft.com
> Subject: Re: [PATCH 1/2] ACPI/AEST: Initial AEST driver
> 
> Hi Shuuichirou,
> 
> Thank you for your feedback!
> 
> On 12/9/2021 3:10 AM, ishii.shuuichir@...itsu.com wrote:
> > Hi, Tyler.
> >
> > We would like to make a few comments.
> >
> >> -----Original Message-----
> >> From: Tyler Baicar <baicar@...amperecomputing.com>
> >> Sent: Thursday, November 25, 2021 2:07 AM
> >> To: patches@...erecomputing.com; abdulhamid@...amperecomputing.com;
> >> darren@...amperecomputing.com; catalin.marinas@....com;
> >> will@...nel.org; maz@...nel.org; james.morse@....com;
> >> alexandru.elisei@....com; suzuki.poulose@....com;
> >> lorenzo.pieralisi@....com; guohanjun@...wei.com;
> >> sudeep.holla@....com; rafael@...nel.org; lenb@...nel.org;
> >> tony.luck@...el.com; bp@...en8.de; mark.rutland@....com;
> >> anshuman.khandual@....com; vincenzo.frascino@....com;
> >> tabba@...gle.com; marcan@...can.st; keescook@...omium.org;
> >> jthierry@...hat.com; masahiroy@...nel.org; samitolvanen@...gle.com;
> >> john.garry@...wei.com; daniel.lezcano@...aro.org; gor@...ux.ibm.com;
> >> zhangshaokun@...ilicon.com; tmricht@...ux.ibm.com;
> >> dchinner@...hat.com; tglx@...utronix.de;
> >> linux-kernel@...r.kernel.org; linux-arm-kernel@...ts.infradead.org;
> >> kvmarm@...ts.cs.columbia.edu; linux-acpi@...r.kernel.org;
> >> linux-edac@...r.kernel.org; Ishii, Shuuichirou/石井
> >> 周一郎 <ishii.shuuichir@...itsu.com>; Vineeth.Pillai@...rosoft.com
> >> Cc: Tyler Baicar <baicar@...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@...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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ