linux-kernel - Re: [PATCH 1/3] x86/intel_rdt: Introduce utility to obtain CDP peer

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: <alpine.DEB.2.21.1810030843180.1435@nanos.tec.linutronix.de>
Date:   Wed, 3 Oct 2018 08:46:30 +0200 (CEST)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Reinette Chatre <reinette.chatre@...el.com>
cc:     fenghua.yu@...el.com, tony.luck@...el.com, jithu.joseph@...el.com,
        gavin.hindman@...el.com, dave.hansen@...el.com, mingo@...hat.com,
        hpa@...or.com, x86@...nel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3] x86/intel_rdt: Introduce utility to obtain CDP
 peer

On Wed, 26 Sep 2018, Reinette Chatre wrote:
> + * Return: 0 if a CDP peer was found, <0 on error or if no CDP peer exists.
> + *         If a CDP peer was found, @r_cdp will point to the peer RDT resource
> + *         and @d_cdp will point to the peer RDT domain.
> + */
> +static int __attribute__((unused)) rdt_cdp_peer_get(struct rdt_resource *r,
> +						    struct rdt_domain *d,
> +						    struct rdt_resource **r_cdp,
> +						    struct rdt_domain **d_cdp)
> +{
> +	struct rdt_resource *_r_cdp = NULL;
> +	struct rdt_domain *_d_cdp = NULL;
> +	int ret = 0;
> +
> +	switch (r->rid) {
> +	case RDT_RESOURCE_L3DATA:
> +		_r_cdp = &rdt_resources_all[RDT_RESOURCE_L3CODE];
> +		break;
> +	case RDT_RESOURCE_L3CODE:
> +		_r_cdp =  &rdt_resources_all[RDT_RESOURCE_L3DATA];
> +		break;
> +	case RDT_RESOURCE_L2DATA:
> +		_r_cdp =  &rdt_resources_all[RDT_RESOURCE_L2CODE];
> +		break;
> +	case RDT_RESOURCE_L2CODE:
> +		_r_cdp =  &rdt_resources_all[RDT_RESOURCE_L2DATA];
> +		break;
> +	default:
> +		ret = -ENOENT;
> +		goto out;
> +	}
> +
> +	/*
> +	 * When a new CPU comes online and CDP is enabled then the new
> +	 * RDT domains (if any) associated with both CDP RDT resources
> +	 * are added in the same CPU online routine while the
> +	 * rdtgroup_mutex is held. It should thus not happen for one
> +	 * RDT domain to exist and be associated with its RDT CDP
> +	 * resource but there is no RDT domain associated with the
> +	 * peer RDT CDP resource. Hence the WARN.
> +	 */
> +	_d_cdp = rdt_find_domain(_r_cdp, d->id, NULL);
> +	if (WARN_ON(!_d_cdp)) {
> +		_r_cdp = NULL;
> +		ret = -ENOENT;

While this should never happen, the return value is ambiguous. I'd rather
use EINVAL or such and propagate it further down at the call site.

Thanks,

	tglx