[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <d82ec97f-d171-4217-d0c5-0a96b6510284@flygoat.com>
Date: Fri, 1 Jan 2021 00:12:23 +0800
From: Jiaxun Yang <jiaxun.yang@...goat.com>
To: WANG Xuerui <kernel@...0n.name>,
"open list:MIPS" <linux-mips@...r.kernel.org>
Cc: Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
Serge Semin <Sergey.Semin@...kalelectronics.ru>,
Alexey Malahov <Alexey.Malahov@...kalelectronics.ru>,
周琰杰 (Zhou Yanjie)
<zhouyanjie@...yeetech.com>, Paul Burton <paulburton@...nel.org>,
Paul Cercueil <paul@...pouillou.net>,
Tiezhu Yang <yangtiezhu@...ngson.cn>,
Huacai Chen <chenhc@...ote.com>, YunQiang Su <syq@...ian.org>,
Liangliang Huang <huanglllzu@...il.com>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3] MIPS: Add vulnerabilities infrastructure
在 2020/12/31 23:38, WANG Xuerui 写道:
> Hi Jiaxun,
>
> Overall a nice step towards a more conformant arch/mips! Some nits
> below though.
>
>
> On 12/30/20 11:23 AM, Jiaxun Yang wrote:
>> Add infrastructure to display CPU vulnerabilities.
>> As most MIPS CPU vendors are dead today and we can't confirm
>> vulnerabilities states with them, we'll display vulnerabilities
>> as "Unknown" by default and override them in cpu-probe.c
> Add trailing period.
>>
>> Signed-off-by: Jiaxun Yang <jiaxun.yang@...goat.com>
>> ---
>> arch/mips/Kconfig | 1 +
>> arch/mips/include/asm/cpu-info.h | 5 ++++
>> arch/mips/include/asm/cpu.h | 7 +++++
>> arch/mips/kernel/Makefile | 2 +-
>> arch/mips/kernel/vulnbl.c | 46 ++++++++++++++++++++++++++++++++
>> 5 files changed, 60 insertions(+), 1 deletion(-)
>> create mode 100644 arch/mips/kernel/vulnbl.c
>>
>> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
>> index ef5b2a177b1b..524053b8f769 100644
>> --- a/arch/mips/Kconfig
>> +++ b/arch/mips/Kconfig
>> @@ -24,6 +24,7 @@ config MIPS
>> select GENERIC_CLOCKEVENTS
>> select GENERIC_CMOS_UPDATE
>> select GENERIC_CPU_AUTOPROBE
>> + select GENERIC_CPU_VULNERABILITIES
>> select GENERIC_GETTIMEOFDAY
>> select GENERIC_IOMAP
>> select GENERIC_IRQ_PROBE
>> diff --git a/arch/mips/include/asm/cpu-info.h
>> b/arch/mips/include/asm/cpu-info.h
>> index a600670d00e9..1a964dbfc0a8 100644
>> --- a/arch/mips/include/asm/cpu-info.h
>> +++ b/arch/mips/include/asm/cpu-info.h
>> @@ -106,6 +106,11 @@ struct cpuinfo_mips {
>> unsigned int guestid_mask;
>> unsigned int guestid_cache;
>> + /* Vulnerabilities */
>> + unsigned int vulnerabilities; /* Vulnerabilities states
>> that we known */
>> + unsigned int vulnerable; /* Vulnerabilities affated */
>> + unsigned int mitigations; /* Mitigations */
>
> Could you make the field names a little clearer? Like "known_mask",
> "affected_mask" and "mitigated_mask"?
>
> Also I wonder if removing the first mask is okay, since if a bit is
> neither "affected" nor "mitigated" then it must belong to the
> "unknown" case.
Actually we have no way to mitigate them in kernel for now :-(
>
>> +
>> #ifdef CONFIG_CPU_LOONGSON3_CPUCFG_EMULATION
>> /* CPUCFG data for this CPU, synthesized at probe time.
>> *
>> diff --git a/arch/mips/include/asm/cpu.h b/arch/mips/include/asm/cpu.h
>> index f5b04e8f6061..3414c9f5464e 100644
>> --- a/arch/mips/include/asm/cpu.h
>> +++ b/arch/mips/include/asm/cpu.h
>> @@ -447,4 +447,11 @@ enum cpu_type_enum {
>> #define MIPS_ASE_LOONGSON_EXT 0x00002000 /* Loongson EXTensions */
>> #define MIPS_ASE_LOONGSON_EXT2 0x00004000 /* Loongson EXTensions
>> R2 */
>> +/*
>> + * CPU security vulnerabilities
>> + */
>> +#define MIPS_VULNBL_MELTDOWN BIT(0)
>> +#define MIPS_VULNBL_SPECTRE_V1 BIT(1)
>> +#define MIPS_VULNBL_SPECTRE_V2 BIT(2)
> Looking at the arch/x86 vulnerabilities code, I tend to think that
> "VULNBL" is not (rather ugly) shorthand for "vulnerability", but
> "vulnerability blacklist" (!), because they have "VULNWL" for
> apparently "whitelist". So I suggest writing out "VULNERABILITY" fully
> for clarity.
>> +
>> #endif /* _ASM_CPU_H */
>> diff --git a/arch/mips/kernel/Makefile b/arch/mips/kernel/Makefile
>> index 13a26d254829..39abc8ead5e0 100644
>> --- a/arch/mips/kernel/Makefile
>> +++ b/arch/mips/kernel/Makefile
>> @@ -8,7 +8,7 @@ extra-y := head.o vmlinux.lds
>> obj-y += cmpxchg.o cpu-probe.o branch.o elf.o entry.o
>> genex.o idle.o irq.o \
>> process.o prom.o ptrace.o reset.o setup.o signal.o \
>> syscall.o time.o topology.o traps.o unaligned.o watch.o \
>> - vdso.o cacheinfo.o
>> + vdso.o cacheinfo.o vulnbl.o
>> ifdef CONFIG_FUNCTION_TRACER
>> CFLAGS_REMOVE_ftrace.o = -pg
>> diff --git a/arch/mips/kernel/vulnbl.c b/arch/mips/kernel/vulnbl.c
>> new file mode 100644
>> index 000000000000..fc73da6214fe
>> --- /dev/null
>> +++ b/arch/mips/kernel/vulnbl.c
> Same with this filename.
>> @@ -0,0 +1,46 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) 2020, Jiaxun Yang <jiaxun.yang@...goat.com>
>> + * MIPS CPU vulnerabilities
>> + */
>> +
>> +#include <linux/device.h>
>> +
>> +#include <asm/cpu-info.h>
>> +#include <asm/cpu.h>
>> +
>> +ssize_t cpu_show_meltdown(struct device *dev,
>> + struct device_attribute *attr, char *buf)
>> +{
>> + if (!(boot_cpu_data.vulnerabilities & MIPS_VULNBL_MELTDOWN))
>> + return sprintf(buf, "Unknown\n");
>> +
>> + if (!(boot_cpu_data.vulnerable & MIPS_VULNBL_MELTDOWN))
>> + return sprintf(buf, "Not affected\n");
>> +
>> + return sprintf(buf, "Affected\n");
> Be consistent with other arches and use "Vulnerable"?
>> +}
>> +
>> +ssize_t cpu_show_spectre_v1(struct device *dev,
>> + struct device_attribute *attr, char *buf)
>> +{
>> + if (!(boot_cpu_data.vulnerabilities & MIPS_VULNBL_SPECTRE_V1))
>> + return sprintf(buf, "Unknown\n");
>> +
>> + if (!(boot_cpu_data.vulnerable & MIPS_VULNBL_SPECTRE_V1))
>> + return sprintf(buf, "Not affected\n");
>> +
>> + return sprintf(buf, "Affected\n");
> Same as above.
>> +}
>> +
>> +ssize_t cpu_show_spectre_v2(struct device *dev,
>> + struct device_attribute *attr, char *buf)
>> +{
>> + if (!(boot_cpu_data.vulnerabilities & MIPS_VULNBL_SPECTRE_V2))
>> + return sprintf(buf, "Unknown\n");
>> +
>> + if (!(boot_cpu_data.vulnerable & MIPS_VULNBL_SPECTRE_V2))
>> + return sprintf(buf, "Not affected\n");
>> +
>> + return sprintf(buf, "Affected\n");
> Same as above.
>> +}
Powered by blists - more mailing lists