Re: [PATCH 1/3] MIPS: Add vulnerabilities infrastructure

2020-12-31 Thread Jiaxun Yang




在 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 
---
  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    0x2000 /* Loongson EXTensions */
  #define MIPS_ASE_LOONGSON_EXT2    0x4000 /* 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 ..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 
+ *  MIPS CPU vulnerabilities
+ */
+
+#include 
+
+#include 
+#include 
+
+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.v

Re: [PATCH 1/3] MIPS: Add vulnerabilities infrastructure

2020-12-31 Thread 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 
---
  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 intguestid_mask;
unsigned intguestid_cache;
  
+	/* Vulnerabilities */

+   unsigned intvulnerabilities; /* Vulnerabilities states that 
we known */
+   unsigned intvulnerable; /* Vulnerabilities affated */
+   unsigned intmitigations; /* 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.



+
  #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 0x2000 /* Loongson EXTensions */
  #define MIPS_ASE_LOONGSON_EXT20x4000 /* 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 ..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 
+ *  MIPS CPU vulnerabilities
+ */
+
+#include 
+
+#include 
+#include 
+
+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 *

Re: [PATCH 1/3] MIPS: Add vulnerabilities infrastructure

2020-12-30 Thread Huacai Chen
Hi, Jiaxun,

On Wed, Dec 30, 2020 at 11:25 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
>
> Signed-off-by: Jiaxun Yang 
> ---
>  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 intguestid_mask;
> unsigned intguestid_cache;
>
> +   /* Vulnerabilities */
> +   unsigned intvulnerabilities; /* Vulnerabilities states 
> that we known */
> +   unsigned intvulnerable; /* Vulnerabilities affated */
What is "affated"? maybe "affected"?

Huacai

> +   unsigned intmitigations; /* Mitigations */
> +
>  #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  0x2000 /* Loongson EXTensions */
>  #define MIPS_ASE_LOONGSON_EXT2 0x4000 /* 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)
> +
>  #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 ..fc73da6214fe
> --- /dev/null
> +++ b/arch/mips/kernel/vulnbl.c
> @@ -0,0 +1,46 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + *  Copyright (C) 2020, Jiaxun Yang 
> + *  MIPS CPU vulnerabilities
> + */
> +
> +#include 
> +
> +#include 
> +#include 
> +
> +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");
> +}
> +
> +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");
> +}
> +
> +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");
> +}
> --
> 2.30.0
>


[PATCH 1/3] MIPS: Add vulnerabilities infrastructure

2020-12-29 Thread Jiaxun Yang
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

Signed-off-by: Jiaxun Yang 
---
 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 intguestid_mask;
unsigned intguestid_cache;
 
+   /* Vulnerabilities */
+   unsigned intvulnerabilities; /* Vulnerabilities states that 
we known */
+   unsigned intvulnerable; /* Vulnerabilities affated */
+   unsigned intmitigations; /* Mitigations */
+
 #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  0x2000 /* Loongson EXTensions */
 #define MIPS_ASE_LOONGSON_EXT2 0x4000 /* 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)
+
 #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 ..fc73da6214fe
--- /dev/null
+++ b/arch/mips/kernel/vulnbl.c
@@ -0,0 +1,46 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ *  Copyright (C) 2020, Jiaxun Yang 
+ *  MIPS CPU vulnerabilities
+ */
+
+#include 
+
+#include 
+#include 
+
+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");
+}
+
+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");
+}
+
+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");
+}
-- 
2.30.0