Linux-EDAC Archive on lore.kernel.org
 help / color / Atom feed
From: Shiju Jose <shiju.jose@huawei.com>
To: James Morse <james.morse@arm.com>
Cc: Borislav Petkov <bp@alien8.de>,
	"linux-edac@vger.kernel.org" <linux-edac@vger.kernel.org>,
	"linux-acpi@vger.kernel.org" <linux-acpi@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"tony.luck@intel.com" <tony.luck@intel.com>,
	"rjw@rjwysocki.net" <rjw@rjwysocki.net>,
	"lenb@kernel.org" <lenb@kernel.org>,
	Linuxarm <linuxarm@huawei.com>,
	Jonathan Cameron <jonathan.cameron@huawei.com>
Subject: RE: [RFC PATCH 0/7] RAS/CEC: Extend CEC for errors count check on short time period
Date: Tue, 6 Oct 2020 16:13:56 +0000
Message-ID: <8d826b53a3fc453ba1c468aaf8eb2e75@huawei.com> (raw)
In-Reply-To: <59950d44-906b-684f-c876-e09c76e5f827@arm.com>

Hi James,

Thanks for the reply and the information shared.

>-----Original Message-----
>From: James Morse [mailto:james.morse@arm.com]
>Sent: 02 October 2020 18:33
>To: Shiju Jose <shiju.jose@huawei.com>
>Cc: Borislav Petkov <bp@alien8.de>; linux-edac@vger.kernel.org; linux-
>acpi@vger.kernel.org; linux-kernel@vger.kernel.org; tony.luck@intel.com;
>rjw@rjwysocki.net; lenb@kernel.org; Linuxarm <linuxarm@huawei.com>
>Subject: Re: [RFC PATCH 0/7] RAS/CEC: Extend CEC for errors count check on
>short time period
>
>Hi Shiju,
>
>On 02/10/2020 16:38, Shiju Jose wrote:
>>> -----Original Message-----
>>> From: Borislav Petkov [mailto:bp@alien8.de]
>>> Sent: 02 October 2020 13:44
>>> To: Shiju Jose <shiju.jose@huawei.com>
>>> Cc: linux-edac@vger.kernel.org; linux-acpi@vger.kernel.org; linux-
>>> kernel@vger.kernel.org; tony.luck@intel.com; rjw@rjwysocki.net;
>>> james.morse@arm.com; lenb@kernel.org; Linuxarm
><linuxarm@huawei.com>
>>> Subject: Re: [RFC PATCH 0/7] RAS/CEC: Extend CEC for errors count
>>> check on short time period
>>>
>>> On Fri, Oct 02, 2020 at 01:22:28PM +0100, Shiju Jose wrote:
>>>> Open Questions based on the feedback from Boris, 1. ARM processor
>>>> error types are cache/TLB/bus errors.
>>>>    [Reference N2.4.4.1 ARM Processor Error Information UEFI Spec
>>>> v2.8] Any of the above error types should not be consider for the
>>>> error collection and CPU core isolation?
>
>Boris' earlier example was that Bus errors have very little to do with the CPU.
>It may just be that this CPU is handling the IRQs for a fault device, and thus
>receiving the errors. irqbalance could change that anytime.
>
>I'd prefer we just stick with the caches for now.
>
[...]

>
>>> Open question from James with my reply to it:
>>>
>>> On Thu, Oct 01, 2020 at 06:16:03PM +0100, James Morse wrote:
>>>> If the corrected-count is available somewhere, can't this policy be
>>>> made in user-space?
>
>> The error count is present in the struct cper_arm_err_info, the fields
>> of this structure  are not reported to the user-space through trace events?
>
>> Presently the fields of table struct cper_sec_proc_arm only are
>> reported to the user-space through trace-arm-event.
>> Also there can be multiple cper_arm_err_info per cper_sec_proc_arm.
>> Thus I think this need reporting through a new trace event?
>
>I think it would be more useful to feed this into edac like ghes.c already does
>for memory errors. These would end up as corrected errors counts on devices
>for L3 or whatever.
>
>This saves fixing your user-space component to the arm specific CPER record
>format, or even firmware-first, meaning its useful to the widest number of
>people.
>
>
>> Also the logical index of a CPU which I think need to extract from the
>'mpidr' field of
>> struct cper_sec_proc_arm using platform dependent kernel function
>get_logical_index().
>> Thus cpu index also need to report to the user space.
>
>I thought you were talking about caches. These structures have a 'level' for
>cache errors.
>
>Certainly you need a way of knowing which cache it is, and from that number
>you should also be able to work out which the CPUs it is attached to.
>
>x86 already has a way of doing this:
>https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Docu
>mentation/x86/resctrl_ui.rst#n327
>
>arm64 doesn't have anything equivalent, but my current proposal for MPAM
>is here:
>https://git.kernel.org/pub/scm/linux/kernel/git/morse/linux.git/commit/?h=
>mpam/snapshot/feb&id=ce3148bd39509ac8b12f5917f0f92ce014a5b22f
>
>I was hoping the PPTT table would grow something we could use as an ID, but
>I've not seen anything yet.

Please find following pseudo code we added for the kernel side to make sure
we correctly understand your suggestions.

1. Create edac device and edac device sysfs entries for the online CPU caches.
/drivers/edac/edac_device.c
struct edac_device_ctl_info  *edac_device_add_cache(unsigned int id, u8 level, u8 type) {
	...
	/* Check edac entry for cache already present */       
	edev_cache = find_edac_device_cache(id, level, type);
	if (edev_cache)
		return edev_cache;
 
	edev_cache = edac_device_alloc_ctrl_info(...);
 	if (!edev_cache)
		return NULL;

	rc = edac_device_add_device(edev_cache);
 	if (rc)
		goto exit;

 	/* store edev_cache for future use */
 	...
	return edev_cache;

 exit:
	...
	return NULL; 
 }

/drivers/base/cacheinfo.c
int cache_create_edac_entries(u64 mpidr, u8 cache_level, u8 cache_type)
{ 
	...
	/* Get cacheinfo for each online cpus */
	for_each_online_cpu(i) {
		struct cpu_cacheinfo *cpu_ci = get_cpu_cacheinfo(i);
		if (!cpu_ci || !cpu_ci->id)
			continue;
        		... 
		/*Add  the edac entry for the CPU cache */
		edev_cache = edac_device_add_cache(cpu_ci->id, cpu_ci ->level, cpu_ci ->type)
		if (!edev_cache)
			break;
		...
	}
	...	
}
     
unsigned int cache_get_cache_id(u64 proc_id, u8 cache_level, u8 cache_type)
{ 
	unsigned int cache_id = 0;
	...
	/* Walk looking for matching cache node */   
	for_each_online_cpu(i) {
		struct cpu_cacheinfo *cpu_ci = get_cpu_cacheinfo(i);
		if (!cpu_ci || !cpu_ci->id)
			continue;

		id = CONV(proc_id);  /* need to check */
		if((id == cpu_ci->id) && (cache_level == cpu_ci->level) && (cache_type == cpu_ci->type))  {
			cache_id = cpu_ci->id;
			break;
		}
	}
	return cache_id;
}

2. Store CPU CE count in the edac sysfs entry for the CPU cache.

drivers/edac/ghes_edac.c
void ghes_edac_report_cpu_error(int cache_id, u8 cache_level, u8 cache_type , uint32 ce_count)
{
	...
	/* Check edac entry for cache already present, if not add new entry */       
	edev_cache = find_edac_device_cache(cache_id, cache_level, cache_type);
	if (!edev_cache) {
		/*Add  the edac entry for the cache */
		edev_cache = edac_device_add_cache(cache_id, cache_level, cache_type);
		if (!edev_cache)
			return;
	}

	/* Store the ce_count to /sys/devices/system/edac/ cpu/cpu<no>/L<N>cache/ce_count */
	edac_device_handle_ce_count(edev_cache, ce_count, ...)
}
 
drivers/acpi/apei/ghes.c
void ghes_handle_arm_hw_error(struct acpi_hest_generic_data *gdata) {
 	...
 	if (sec_sev != GHES_SEV_CORRECTED)
 		return;
 	mpidr = cper_sec_proc_arm->mpidr;    
 	for(i = 0; i < cper_sec_proc_arm->err_info_num; i++) {
 		if(cper_sec_proc_info->type != CPER_ARM_CACHE_ERROR) 
 			continue; 
 		ce_count = cper_arm_err_info->multiple_error + 1;
		cache_type = cper_arm_err_info->type;
		cache_level = cper_arm_err_info->error_info<24: 22>;  
		cache_id = cache_get_cache_id(mpidr, cache_level, cache_type);
 		if (!cache_id)
 			return;
		ghes_edac_report_cpu_error(cache_id, cache_level, cache_type , ce_count);
	}
              ...
	return;	
}

>
>
>>> You mean rasdaemon goes and offlines CPUs when certain thresholds are
>>> reached? Sure. It would be much more flexible too.
>
[...]
>
>
>Thanks,
>
>James

Thanks,
Shiju


  parent reply index

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-02 12:22 Shiju Jose
2020-10-02 12:22 ` [RFC PATCH 1/7] RAS/CEC: Replace the macro PFN with ELEM_NO Shiju Jose
2020-10-02 12:22 ` [RFC PATCH 2/7] RAS/CEC: Replace pfns_poisoned with elems_poisoned Shiju Jose
2020-10-02 12:22 ` [RFC PATCH 3/7] RAS/CEC: Move X86 MCE specific code under CONFIG_X86_MCE Shiju Jose
2020-10-02 12:22 ` [RFC PATCH 4/7] RAS/CEC: Modify cec_mod_work() for common use Shiju Jose
2020-10-02 12:22 ` [RFC PATCH 5/7] RAS/CEC: Add support for errors count check on short time period Shiju Jose
2020-10-02 12:22 ` [RFC PATCH 6/7] RAS/CEC: Add CPU Correctable Error Collector to isolate an erroneous CPU core Shiju Jose
2020-10-02 12:22 ` [RFC PATCH 7/7] ACPI / APEI: Add reporting ARM64 CPU correctable errors to the CEC Shiju Jose
2020-10-02 12:43 ` [RFC PATCH 0/7] RAS/CEC: Extend CEC for errors count check on short time period Borislav Petkov
2020-10-02 15:38   ` Shiju Jose
2020-10-02 17:33     ` James Morse
2020-10-02 18:02       ` Borislav Petkov
2020-10-06 16:13       ` Shiju Jose [this message]
2020-10-07 16:45         ` James Morse
2020-10-02 16:04   ` Luck, Tony

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=8d826b53a3fc453ba1c468aaf8eb2e75@huawei.com \
    --to=shiju.jose@huawei.com \
    --cc=bp@alien8.de \
    --cc=james.morse@arm.com \
    --cc=jonathan.cameron@huawei.com \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-edac@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxarm@huawei.com \
    --cc=rjw@rjwysocki.net \
    --cc=tony.luck@intel.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

Linux-EDAC Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-edac/0 linux-edac/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-edac linux-edac/ https://lore.kernel.org/linux-edac \
		linux-edac@vger.kernel.org
	public-inbox-index linux-edac

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-edac


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git