Re: [PATCH v4 18/21] arm64: Move "nokaslr" over to the early cpufeature infrastructure
On Mon, 18 Jan 2021 14:46:36 +,
David Brazdil wrote:
>
> On Mon, Jan 18, 2021 at 09:45:30AM +, Marc Zyngier wrote:
> > Given that the early cpufeature infrastructure has borrowed quite
> > a lot of code from the kaslr implementation, let's reimplement
> > the matching of the "nokaslr" option with it.
> >
> > Signed-off-by: Marc Zyngier
> Acked-by: David Brazdil
[...]
> > @@ -126,7 +95,7 @@ u64 __init kaslr_early_init(void)
> > * Check if 'nokaslr' appears on the command line, and
> > * return 0 if that is the case.
> > */
> > - if (is_kaslr_disabled_cmdline(fdt)) {
> > + if (kaslr_feature_val & kaslr_feature_mask & 0xf) {
>
> nit: Isn't the 0xf redundant here? You don't re-mask for VH either.
Actually, I do. See the two back to back ubfx that extract both the
mask and the feature. The "& 0xf" here serves the same purpose.
Is it redundant? At the moment, quite possibly. But since we have
space for 16 "features", this is an indication that we are only using
the first one. I expect that eventually, we'll use it for other
things.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
Re: [PATCH v4 18/21] arm64: Move "nokaslr" over to the early cpufeature infrastructure
On Mon, Jan 18, 2021 at 09:45:30AM +, Marc Zyngier wrote:
> Given that the early cpufeature infrastructure has borrowed quite
> a lot of code from the kaslr implementation, let's reimplement
> the matching of the "nokaslr" option with it.
>
> Signed-off-by: Marc Zyngier
> ---
> arch/arm64/kernel/idreg-override.c | 17 ++
> arch/arm64/kernel/kaslr.c | 37 +++---
> 2 files changed, 20 insertions(+), 34 deletions(-)
>
> diff --git a/arch/arm64/kernel/idreg-override.c
> b/arch/arm64/kernel/idreg-override.c
> index 1db54878b2c4..143fe7b8e3ce 100644
> --- a/arch/arm64/kernel/idreg-override.c
> +++ b/arch/arm64/kernel/idreg-override.c
> @@ -33,8 +33,24 @@ static const struct reg_desc mmfr1 __initdata = {
> },
> };
>
> +extern u64 kaslr_feature_val;
> +extern u64 kaslr_feature_mask;
> +
> +static const struct reg_desc kaslr __initdata = {
> + .name = "kaslr",
We might as well rename this ftr_override or something more generic as
we no longer describe registers here. Otherwise:
Acked-by: Catalin Marinas
[PATCH v4 18/21] arm64: Move "nokaslr" over to the early cpufeature infrastructure
Given that the early cpufeature infrastructure has borrowed quite
a lot of code from the kaslr implementation, let's reimplement
the matching of the "nokaslr" option with it.
Signed-off-by: Marc Zyngier
---
arch/arm64/kernel/idreg-override.c | 17 ++
arch/arm64/kernel/kaslr.c | 37 +++---
2 files changed, 20 insertions(+), 34 deletions(-)
diff --git a/arch/arm64/kernel/idreg-override.c
b/arch/arm64/kernel/idreg-override.c
index 1db54878b2c4..143fe7b8e3ce 100644
--- a/arch/arm64/kernel/idreg-override.c
+++ b/arch/arm64/kernel/idreg-override.c
@@ -33,8 +33,24 @@ static const struct reg_desc mmfr1 __initdata = {
},
};
+extern u64 kaslr_feature_val;
+extern u64 kaslr_feature_mask;
+
+static const struct reg_desc kaslr __initdata = {
+ .name = "kaslr",
+#ifdef CONFIG_RANDOMIZE_BASE
+ .val= _feature_val,
+ .mask = _feature_mask,
+#endif
+ .fields = {
+ { "disabled", 0 },
+ {}
+ },
+};
+
static const struct reg_desc * const regs[] __initdata = {
,
+ ,
};
static const struct {
@@ -43,6 +59,7 @@ static const struct {
} aliases[] __initdata = {
{ "kvm-arm.mode=nvhe", "id_aa64mmfr1.vh=0" },
{ "kvm-arm.mode=protected", "id_aa64mmfr1.vh=0" },
+ { "nokaslr","kaslr.disabled=1" },
};
static int __init find_field(const char *cmdline, const struct reg_desc *reg,
diff --git a/arch/arm64/kernel/kaslr.c b/arch/arm64/kernel/kaslr.c
index 5fc86e7d01a1..dcc4a5aadbe2 100644
--- a/arch/arm64/kernel/kaslr.c
+++ b/arch/arm64/kernel/kaslr.c
@@ -51,39 +51,8 @@ static __init u64 get_kaslr_seed(void *fdt)
return ret;
}
-static __init bool cmdline_contains_nokaslr(const u8 *cmdline)
-{
- const u8 *str;
-
- str = strstr(cmdline, "nokaslr");
- return str == cmdline || (str > cmdline && *(str - 1) == ' ');
-}
-
-static __init bool is_kaslr_disabled_cmdline(void *fdt)
-{
- if (!IS_ENABLED(CONFIG_CMDLINE_FORCE)) {
- int node;
- const u8 *prop;
-
- node = fdt_path_offset(fdt, "/chosen");
- if (node < 0)
- goto out;
-
- prop = fdt_getprop(fdt, node, "bootargs", NULL);
- if (!prop)
- goto out;
-
- if (cmdline_contains_nokaslr(prop))
- return true;
-
- if (IS_ENABLED(CONFIG_CMDLINE_EXTEND))
- goto out;
-
- return false;
- }
-out:
- return cmdline_contains_nokaslr(CONFIG_CMDLINE);
-}
+u64 kaslr_feature_val __initdata;
+u64 kaslr_feature_mask __initdata;
/*
* This routine will be executed with the kernel mapped at its default virtual
@@ -126,7 +95,7 @@ u64 __init kaslr_early_init(void)
* Check if 'nokaslr' appears on the command line, and
* return 0 if that is the case.
*/
- if (is_kaslr_disabled_cmdline(fdt)) {
+ if (kaslr_feature_val & kaslr_feature_mask & 0xf) {
kaslr_status = KASLR_DISABLED_CMDLINE;
return 0;
}
--
2.29.2
Re: [PATCH v4 18/21] arm64: Move "nokaslr" over to the early cpufeature infrastructure
On Mon, Jan 18, 2021 at 09:45:30AM +, Marc Zyngier wrote:
> Given that the early cpufeature infrastructure has borrowed quite
> a lot of code from the kaslr implementation, let's reimplement
> the matching of the "nokaslr" option with it.
>
> Signed-off-by: Marc Zyngier
Acked-by: David Brazdil
> ---
> arch/arm64/kernel/idreg-override.c | 17 ++
> arch/arm64/kernel/kaslr.c | 37 +++---
> 2 files changed, 20 insertions(+), 34 deletions(-)
>
> diff --git a/arch/arm64/kernel/idreg-override.c
> b/arch/arm64/kernel/idreg-override.c
> index 1db54878b2c4..143fe7b8e3ce 100644
> --- a/arch/arm64/kernel/idreg-override.c
> +++ b/arch/arm64/kernel/idreg-override.c
> @@ -33,8 +33,24 @@ static const struct reg_desc mmfr1 __initdata = {
> },
> };
>
> +extern u64 kaslr_feature_val;
> +extern u64 kaslr_feature_mask;
> +
> +static const struct reg_desc kaslr __initdata = {
> + .name = "kaslr",
> +#ifdef CONFIG_RANDOMIZE_BASE
> + .val= _feature_val,
> + .mask = _feature_mask,
> +#endif
> + .fields = {
> + { "disabled", 0 },
> + {}
> + },
> +};
> +
> static const struct reg_desc * const regs[] __initdata = {
> ,
> + ,
> };
>
> static const struct {
> @@ -43,6 +59,7 @@ static const struct {
> } aliases[] __initdata = {
> { "kvm-arm.mode=nvhe", "id_aa64mmfr1.vh=0" },
> { "kvm-arm.mode=protected", "id_aa64mmfr1.vh=0" },
> + { "nokaslr","kaslr.disabled=1" },
> };
>
> static int __init find_field(const char *cmdline, const struct reg_desc *reg,
> diff --git a/arch/arm64/kernel/kaslr.c b/arch/arm64/kernel/kaslr.c
> index 5fc86e7d01a1..dcc4a5aadbe2 100644
> --- a/arch/arm64/kernel/kaslr.c
> +++ b/arch/arm64/kernel/kaslr.c
> @@ -51,39 +51,8 @@ static __init u64 get_kaslr_seed(void *fdt)
> return ret;
> }
>
> -static __init bool cmdline_contains_nokaslr(const u8 *cmdline)
> -{
> - const u8 *str;
> -
> - str = strstr(cmdline, "nokaslr");
> - return str == cmdline || (str > cmdline && *(str - 1) == ' ');
> -}
> -
> -static __init bool is_kaslr_disabled_cmdline(void *fdt)
> -{
> - if (!IS_ENABLED(CONFIG_CMDLINE_FORCE)) {
> - int node;
> - const u8 *prop;
> -
> - node = fdt_path_offset(fdt, "/chosen");
> - if (node < 0)
> - goto out;
> -
> - prop = fdt_getprop(fdt, node, "bootargs", NULL);
> - if (!prop)
> - goto out;
> -
> - if (cmdline_contains_nokaslr(prop))
> - return true;
> -
> - if (IS_ENABLED(CONFIG_CMDLINE_EXTEND))
> - goto out;
> -
> - return false;
> - }
> -out:
> - return cmdline_contains_nokaslr(CONFIG_CMDLINE);
> -}
> +u64 kaslr_feature_val __initdata;
> +u64 kaslr_feature_mask __initdata;
>
> /*
> * This routine will be executed with the kernel mapped at its default
> virtual
> @@ -126,7 +95,7 @@ u64 __init kaslr_early_init(void)
>* Check if 'nokaslr' appears on the command line, and
>* return 0 if that is the case.
>*/
> - if (is_kaslr_disabled_cmdline(fdt)) {
> + if (kaslr_feature_val & kaslr_feature_mask & 0xf) {
nit: Isn't the 0xf redundant here? You don't re-mask for VH either.
> kaslr_status = KASLR_DISABLED_CMDLINE;
> return 0;
> }
> --
> 2.29.2
>
