* [PATCH] arm64: cpufeature: Trim feature reporting and include PAN emulation
@ 2018-02-20 22:46 Kees Cook
2018-02-21 11:18 ` Dave Martin
0 siblings, 1 reply; 5+ messages in thread
From: Kees Cook @ 2018-02-20 22:46 UTC (permalink / raw)
To: Catalin Marinas
Cc: Will Deacon, Mark Rutland, Suzuki K Poulose, Dave Martin,
linux-arm-kernel, linux-kernel
The PAN emulation notification was only happening for non-boot CPUs
if CPU capabilities had already been configured. This seems to be the
wrong place, as it's system-wide and isn't attached to capabilities,
so its reporting didn't normally happen. Instead, report it once from
the boot CPU. Additionally removes the redundant "feature" word from the
"CPU features:" line.
Before (redundant "feature", and missing PAN emulation report):
SMP: Total of 4 processors activated.
CPU features: detected feature: 32-bit EL0 Support
CPU features: detected feature: Kernel page table isolation (KPTI)
CPU: All CPU(s) started at EL2
After:
SMP: Total of 4 processors activated.
CPU features: detected: 32-bit EL0 Support
CPU features: detected: Kernel page table isolation (KPTI)
CPU features: emulated: Privileged Access Never (PAN) using TTBR0_EL1 switching
CPU: All CPU(s) started at EL2
Signed-off-by: Kees Cook <keescook@chromium.org>
---
arch/arm64/kernel/cpufeature.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 29b1f873e337..6c799ca58b53 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1333,9 +1333,6 @@ static void verify_local_cpu_capabilities(void)
if (system_supports_sve())
verify_sve_features();
-
- if (system_uses_ttbr0_pan())
- pr_info("Emulating Privileged Access Never (PAN) using TTBR0_EL1 switching\n");
}
void check_local_cpu_capabilities(void)
@@ -1360,7 +1357,7 @@ void check_local_cpu_capabilities(void)
static void __init setup_feature_capabilities(void)
{
- update_cpu_capabilities(arm64_features, "detected feature:");
+ update_cpu_capabilities(arm64_features, "detected:");
enable_cpu_capabilities(arm64_features);
}
@@ -1394,6 +1391,9 @@ void __init setup_cpu_features(void)
if (system_supports_32bit_el0())
setup_elf_hwcaps(compat_elf_hwcaps);
+ if (system_uses_ttbr0_pan())
+ pr_info("emulated: Privileged Access Never (PAN) using TTBR0_EL1 switching\n");
+
sve_setup();
/* Advertise that we have computed the system capabilities */
--
2.7.4
--
Kees Cook
Pixel Security
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] arm64: cpufeature: Trim feature reporting and include PAN emulation
2018-02-20 22:46 [PATCH] arm64: cpufeature: Trim feature reporting and include PAN emulation Kees Cook
@ 2018-02-21 11:18 ` Dave Martin
2018-02-21 14:39 ` Mark Rutland
0 siblings, 1 reply; 5+ messages in thread
From: Dave Martin @ 2018-02-21 11:18 UTC (permalink / raw)
To: Kees Cook
Cc: Catalin Marinas, Mark Rutland, Suzuki K Poulose, Will Deacon,
linux-kernel, linux-arm-kernel
On Tue, Feb 20, 2018 at 02:46:24PM -0800, Kees Cook wrote:
> The PAN emulation notification was only happening for non-boot CPUs
> if CPU capabilities had already been configured. This seems to be the
> wrong place, as it's system-wide and isn't attached to capabilities,
> so its reporting didn't normally happen. Instead, report it once from
> the boot CPU. Additionally removes the redundant "feature" word from the
> "CPU features:" line.
>
> Before (redundant "feature", and missing PAN emulation report):
>
> SMP: Total of 4 processors activated.
> CPU features: detected feature: 32-bit EL0 Support
> CPU features: detected feature: Kernel page table isolation (KPTI)
> CPU: All CPU(s) started at EL2
>
> After:
>
> SMP: Total of 4 processors activated.
> CPU features: detected: 32-bit EL0 Support
> CPU features: detected: Kernel page table isolation (KPTI)
> CPU features: emulated: Privileged Access Never (PAN) using TTBR0_EL1 switching
> CPU: All CPU(s) started at EL2
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> arch/arm64/kernel/cpufeature.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index 29b1f873e337..6c799ca58b53 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -1333,9 +1333,6 @@ static void verify_local_cpu_capabilities(void)
>
> if (system_supports_sve())
> verify_sve_features();
> -
> - if (system_uses_ttbr0_pan())
> - pr_info("Emulating Privileged Access Never (PAN) using TTBR0_EL1 switching\n");
> }
>
> void check_local_cpu_capabilities(void)
> @@ -1360,7 +1357,7 @@ void check_local_cpu_capabilities(void)
>
> static void __init setup_feature_capabilities(void)
> {
> - update_cpu_capabilities(arm64_features, "detected feature:");
> + update_cpu_capabilities(arm64_features, "detected:");
Although I get what you're saying about redundant use of the word
"features", this feels like cosmetic churn that is unrelated to the
problem this patch is addressing.
It could be worth reviewing the CPU errata messages and other
miscellaneous printks together to make them less verbose and more
consistent all in one go, but that would be a separate patch...
> enable_cpu_capabilities(arm64_features);
> }
>
> @@ -1394,6 +1391,9 @@ void __init setup_cpu_features(void)
> if (system_supports_32bit_el0())
> setup_elf_hwcaps(compat_elf_hwcaps);
>
> + if (system_uses_ttbr0_pan())
> + pr_info("emulated: Privileged Access Never (PAN) using TTBR0_EL1 switching\n");
> +
Moving this seems sensible. The other option would be to paste it into
update_cpu_capabilities(), but the message would still potentially get
printed multiple times, so that doesn't feel like the right approach.
[...]
Cheers
---Dave
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] arm64: cpufeature: Trim feature reporting and include PAN emulation
2018-02-21 11:18 ` Dave Martin
@ 2018-02-21 14:39 ` Mark Rutland
2018-02-21 16:26 ` Dave Martin
2018-02-21 18:04 ` Kees Cook
0 siblings, 2 replies; 5+ messages in thread
From: Mark Rutland @ 2018-02-21 14:39 UTC (permalink / raw)
To: Dave Martin
Cc: Kees Cook, Catalin Marinas, Suzuki K Poulose, Will Deacon,
linux-kernel, linux-arm-kernel
On Wed, Feb 21, 2018 at 11:18:27AM +0000, Dave Martin wrote:
> On Tue, Feb 20, 2018 at 02:46:24PM -0800, Kees Cook wrote:
> > The PAN emulation notification was only happening for non-boot CPUs
> > if CPU capabilities had already been configured. This seems to be the
> > wrong place, as it's system-wide and isn't attached to capabilities,
> > so its reporting didn't normally happen. Instead, report it once from
> > the boot CPU. Additionally removes the redundant "feature" word from the
> > "CPU features:" line.
> >
> > Before (redundant "feature", and missing PAN emulation report):
> >
> > SMP: Total of 4 processors activated.
> > CPU features: detected feature: 32-bit EL0 Support
> > CPU features: detected feature: Kernel page table isolation (KPTI)
> > CPU: All CPU(s) started at EL2
> >
> > After:
> >
> > SMP: Total of 4 processors activated.
> > CPU features: detected: 32-bit EL0 Support
> > CPU features: detected: Kernel page table isolation (KPTI)
> > CPU features: emulated: Privileged Access Never (PAN) using TTBR0_EL1 switching
> > CPU: All CPU(s) started at EL2
> >
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> > ---
> > arch/arm64/kernel/cpufeature.c | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> > index 29b1f873e337..6c799ca58b53 100644
> > --- a/arch/arm64/kernel/cpufeature.c
> > +++ b/arch/arm64/kernel/cpufeature.c
> > @@ -1333,9 +1333,6 @@ static void verify_local_cpu_capabilities(void)
> >
> > if (system_supports_sve())
> > verify_sve_features();
> > -
> > - if (system_uses_ttbr0_pan())
> > - pr_info("Emulating Privileged Access Never (PAN) using TTBR0_EL1 switching\n");
> > }
> >
> > void check_local_cpu_capabilities(void)
> > @@ -1360,7 +1357,7 @@ void check_local_cpu_capabilities(void)
> >
> > static void __init setup_feature_capabilities(void)
> > {
> > - update_cpu_capabilities(arm64_features, "detected feature:");
> > + update_cpu_capabilities(arm64_features, "detected:");
>
> Although I get what you're saying about redundant use of the word
> "features", this feels like cosmetic churn that is unrelated to the
> problem this patch is addressing.
Given it seems sensible, shall we just split that into a separate patch?
FWIW, for a patch with just this change:
Acked-by: Mark Rutland <mark.rutland@arm.com>
> It could be worth reviewing the CPU errata messages and other
> miscellaneous printks together to make them less verbose and more
> consistent all in one go, but that would be a separate patch...
>
> > enable_cpu_capabilities(arm64_features);
> > }
> >
> > @@ -1394,6 +1391,9 @@ void __init setup_cpu_features(void)
> > if (system_supports_32bit_el0())
> > setup_elf_hwcaps(compat_elf_hwcaps);
> >
> > + if (system_uses_ttbr0_pan())
> > + pr_info("emulated: Privileged Access Never (PAN) using TTBR0_EL1 switching\n");
> > +
>
> Moving this seems sensible. The other option would be to paste it into
> update_cpu_capabilities(), but the message would still potentially get
> printed multiple times, so that doesn't feel like the right approach.
I think that more ideally, we'd give this an entry in the arm64_features array,
but because it's effectively a negative feature, it's a little tricky.
This also looks fine to me, so FWIW:
Acked-by: Mark Rutland <mark.rutland@arm.com>
Mark.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] arm64: cpufeature: Trim feature reporting and include PAN emulation
2018-02-21 14:39 ` Mark Rutland
@ 2018-02-21 16:26 ` Dave Martin
2018-02-21 18:04 ` Kees Cook
1 sibling, 0 replies; 5+ messages in thread
From: Dave Martin @ 2018-02-21 16:26 UTC (permalink / raw)
To: Mark Rutland
Cc: Kees Cook, Suzuki K Poulose, Catalin Marinas, Will Deacon,
linux-kernel, linux-arm-kernel
On Wed, Feb 21, 2018 at 02:39:55PM +0000, Mark Rutland wrote:
> On Wed, Feb 21, 2018 at 11:18:27AM +0000, Dave Martin wrote:
> > On Tue, Feb 20, 2018 at 02:46:24PM -0800, Kees Cook wrote:
> > > The PAN emulation notification was only happening for non-boot CPUs
> > > if CPU capabilities had already been configured. This seems to be the
> > > wrong place, as it's system-wide and isn't attached to capabilities,
> > > so its reporting didn't normally happen. Instead, report it once from
> > > the boot CPU. Additionally removes the redundant "feature" word from the
> > > "CPU features:" line.
> > >
> > > Before (redundant "feature", and missing PAN emulation report):
> > >
> > > SMP: Total of 4 processors activated.
> > > CPU features: detected feature: 32-bit EL0 Support
> > > CPU features: detected feature: Kernel page table isolation (KPTI)
> > > CPU: All CPU(s) started at EL2
> > >
> > > After:
> > >
> > > SMP: Total of 4 processors activated.
> > > CPU features: detected: 32-bit EL0 Support
> > > CPU features: detected: Kernel page table isolation (KPTI)
> > > CPU features: emulated: Privileged Access Never (PAN) using TTBR0_EL1 switching
> > > CPU: All CPU(s) started at EL2
> > >
> > > Signed-off-by: Kees Cook <keescook@chromium.org>
> > > ---
> > > arch/arm64/kernel/cpufeature.c | 8 ++++----
> > > 1 file changed, 4 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> > > index 29b1f873e337..6c799ca58b53 100644
> > > --- a/arch/arm64/kernel/cpufeature.c
> > > +++ b/arch/arm64/kernel/cpufeature.c
> > > @@ -1333,9 +1333,6 @@ static void verify_local_cpu_capabilities(void)
> > >
> > > if (system_supports_sve())
> > > verify_sve_features();
> > > -
> > > - if (system_uses_ttbr0_pan())
> > > - pr_info("Emulating Privileged Access Never (PAN) using TTBR0_EL1 switching\n");
> > > }
> > >
> > > void check_local_cpu_capabilities(void)
> > > @@ -1360,7 +1357,7 @@ void check_local_cpu_capabilities(void)
> > >
> > > static void __init setup_feature_capabilities(void)
> > > {
> > > - update_cpu_capabilities(arm64_features, "detected feature:");
> > > + update_cpu_capabilities(arm64_features, "detected:");
> >
> > Although I get what you're saying about redundant use of the word
> > "features", this feels like cosmetic churn that is unrelated to the
> > problem this patch is addressing.
>
> Given it seems sensible, shall we just split that into a separate patch?
Can do, though I still think it's a bit dubious. The original message
as it stands isn't wrong. I won't lose sleep over it though.
[...]
Cheers
---Dave
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] arm64: cpufeature: Trim feature reporting and include PAN emulation
2018-02-21 14:39 ` Mark Rutland
2018-02-21 16:26 ` Dave Martin
@ 2018-02-21 18:04 ` Kees Cook
1 sibling, 0 replies; 5+ messages in thread
From: Kees Cook @ 2018-02-21 18:04 UTC (permalink / raw)
To: Mark Rutland
Cc: Dave Martin, Catalin Marinas, Suzuki K Poulose, Will Deacon,
LKML, linux-arm-kernel
On Wed, Feb 21, 2018 at 6:39 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> On Wed, Feb 21, 2018 at 11:18:27AM +0000, Dave Martin wrote:
>> On Tue, Feb 20, 2018 at 02:46:24PM -0800, Kees Cook wrote:
>> > The PAN emulation notification was only happening for non-boot CPUs
>> > if CPU capabilities had already been configured. This seems to be the
>> > wrong place, as it's system-wide and isn't attached to capabilities,
>> > so its reporting didn't normally happen. Instead, report it once from
>> > the boot CPU. Additionally removes the redundant "feature" word from the
>> > "CPU features:" line.
>> >
>> > Before (redundant "feature", and missing PAN emulation report):
>> >
>> > SMP: Total of 4 processors activated.
>> > CPU features: detected feature: 32-bit EL0 Support
>> > CPU features: detected feature: Kernel page table isolation (KPTI)
>> > CPU: All CPU(s) started at EL2
>> >
>> > After:
>> >
>> > SMP: Total of 4 processors activated.
>> > CPU features: detected: 32-bit EL0 Support
>> > CPU features: detected: Kernel page table isolation (KPTI)
>> > CPU features: emulated: Privileged Access Never (PAN) using TTBR0_EL1 switching
>> > CPU: All CPU(s) started at EL2
>> >
>> > Signed-off-by: Kees Cook <keescook@chromium.org>
>> > ---
>> > arch/arm64/kernel/cpufeature.c | 8 ++++----
>> > 1 file changed, 4 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
>> > index 29b1f873e337..6c799ca58b53 100644
>> > --- a/arch/arm64/kernel/cpufeature.c
>> > +++ b/arch/arm64/kernel/cpufeature.c
>> > @@ -1333,9 +1333,6 @@ static void verify_local_cpu_capabilities(void)
>> >
>> > if (system_supports_sve())
>> > verify_sve_features();
>> > -
>> > - if (system_uses_ttbr0_pan())
>> > - pr_info("Emulating Privileged Access Never (PAN) using TTBR0_EL1 switching\n");
>> > }
>> >
>> > void check_local_cpu_capabilities(void)
>> > @@ -1360,7 +1357,7 @@ void check_local_cpu_capabilities(void)
>> >
>> > static void __init setup_feature_capabilities(void)
>> > {
>> > - update_cpu_capabilities(arm64_features, "detected feature:");
>> > + update_cpu_capabilities(arm64_features, "detected:");
>>
>> Although I get what you're saying about redundant use of the word
>> "features", this feels like cosmetic churn that is unrelated to the
>> problem this patch is addressing.
>
> Given it seems sensible, shall we just split that into a separate patch?
>
> FWIW, for a patch with just this change:
>
> Acked-by: Mark Rutland <mark.rutland@arm.com>
Sure!
>> It could be worth reviewing the CPU errata messages and other
>> miscellaneous printks together to make them less verbose and more
>> consistent all in one go, but that would be a separate patch...
>>
>> > enable_cpu_capabilities(arm64_features);
>> > }
>> >
>> > @@ -1394,6 +1391,9 @@ void __init setup_cpu_features(void)
>> > if (system_supports_32bit_el0())
>> > setup_elf_hwcaps(compat_elf_hwcaps);
>> >
>> > + if (system_uses_ttbr0_pan())
>> > + pr_info("emulated: Privileged Access Never (PAN) using TTBR0_EL1 switching\n");
>> > +
>>
>> Moving this seems sensible. The other option would be to paste it into
>> update_cpu_capabilities(), but the message would still potentially get
>> printed multiple times, so that doesn't feel like the right approach.
>
> I think that more ideally, we'd give this an entry in the arm64_features array,
> but because it's effectively a negative feature, it's a little tricky.
Yeah, I looked at that but it seemed like it would needlessly burn a
capability bit for no real gain.
> This also looks fine to me, so FWIW:
>
> Acked-by: Mark Rutland <mark.rutland@arm.com>
Thanks, I'll split the patches!
-Kees
--
Kees Cook
Pixel Security
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-02-21 18:04 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-20 22:46 [PATCH] arm64: cpufeature: Trim feature reporting and include PAN emulation Kees Cook
2018-02-21 11:18 ` Dave Martin
2018-02-21 14:39 ` Mark Rutland
2018-02-21 16:26 ` Dave Martin
2018-02-21 18:04 ` Kees Cook
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).