linux-kernel - Re: [PATCH] powerpc/features: Remove CPU_FTR

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: <7b1f7fdd-0256-3370-13ab-d808b9fe9b02@linux.ibm.com>
Date:   Wed, 14 Oct 2020 08:49:26 +0530
From:   "Aneesh Kumar K.V" <aneesh.kumar@...ux.ibm.com>
To:     Michael Ellerman <mpe@...erman.id.au>,
        Christophe Leroy <christophe.leroy@...roup.eu>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Paul Mackerras <paulus@...ba.org>
Cc:     linuxppc-dev@...ts.ozlabs.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] powerpc/features: Remove CPU_FTR_NODSISRALIGN

On 10/13/20 3:45 PM, Michael Ellerman wrote:
> Christophe Leroy <christophe.leroy@...roup.eu> writes:
>> Le 13/10/2020 à 09:23, Aneesh Kumar K.V a écrit :
>>> Christophe Leroy <christophe.leroy@...roup.eu> writes:
>>>
>>>> CPU_FTR_NODSISRALIGN has not been used since
>>>> commit 31bfdb036f12 ("powerpc: Use instruction emulation
>>>> infrastructure to handle alignment faults")
>>>>
>>>> Remove it.
>>>>
>>>> Signed-off-by: Christophe Leroy <christophe.leroy@...roup.eu>
>>>> ---
>>>>    arch/powerpc/include/asm/cputable.h | 22 ++++++++++------------
>>>>    arch/powerpc/kernel/dt_cpu_ftrs.c   |  8 --------
>>>>    arch/powerpc/kernel/prom.c          |  2 +-
>>>>    3 files changed, 11 insertions(+), 21 deletions(-)
>>>>
>>>> diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c b/arch/powerpc/kernel/dt_cpu_ftrs.c
>>>> index 1098863e17ee..c598961d9f15 100644
>>>> --- a/arch/powerpc/kernel/dt_cpu_ftrs.c
>>>> +++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
>>>> @@ -273,13 +273,6 @@ static int __init feat_enable_idle_nap(struct dt_cpu_feature *f)
>>>>    	return 1;
>>>>    }
>>>>    
>>>> -static int __init feat_enable_align_dsisr(struct dt_cpu_feature *f)
>>>> -{
>>>> -	cur_cpu_spec->cpu_features &= ~CPU_FTR_NODSISRALIGN;
>>>> -
>>>> -	return 1;
>>>> -}
>>>> -
>>>>    static int __init feat_enable_idle_stop(struct dt_cpu_feature *f)
>>>>    {
>>>>    	u64 lpcr;
>>>> @@ -641,7 +634,6 @@ static struct dt_cpu_feature_match __initdata
>>>>    	{"tm-suspend-hypervisor-assist", feat_enable, CPU_FTR_P9_TM_HV_ASSIST},
>>>>    	{"tm-suspend-xer-so-bug", feat_enable, CPU_FTR_P9_TM_XER_SO_BUG},
>>>>    	{"idle-nap", feat_enable_idle_nap, 0},
>>>> -	{"alignment-interrupt-dsisr", feat_enable_align_dsisr, 0},
> 
> Rather than removing it entirely, I'd rather we left a comment, so that
> it's obvious that we are ignoring that feature on purpose, not because
> we forget about it.
> 
> eg:
> 
> diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c b/arch/powerpc/kernel/dt_cpu_ftrs.c
> index f204ad79b6b5..45cb7e59bd13 100644
> --- a/arch/powerpc/kernel/dt_cpu_ftrs.c
> +++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
> @@ -640,7 +640,7 @@ static struct dt_cpu_feature_match __initdata
>   	{"tm-suspend-hypervisor-assist", feat_enable, CPU_FTR_P9_TM_HV_ASSIST},
>   	{"tm-suspend-xer-so-bug", feat_enable, CPU_FTR_P9_TM_XER_SO_BUG},
>   	{"idle-nap", feat_enable_idle_nap, 0},
> -	{"alignment-interrupt-dsisr", feat_enable_align_dsisr, 0},
> +	// "alignment-interrupt-dsisr" ignored
>   	{"idle-stop", feat_enable_idle_stop, 0},
>   	{"machine-check-power8", feat_enable_mce_power8, 0},
>   	{"performance-monitor-power8", feat_enable_pmu_power8, 0},
> 


why not do it as
static int __init feat_enable_align_dsisr(struct dt_cpu_feature *f)
{
	/* This feature should not be enabled */
#ifdef DEBUG
	WARN(1);
#endif

	return 1;
}


-aneesh