* Re: [PATCH v12 00/11] Appended signatures support for IMA appraisal
From: Thiago Jung Bauermann @ 2019-08-27 1:04 UTC (permalink / raw)
To: Jordan Hand
Cc: linux-integrity, linux-security-module, keyrings, linux-crypto,
linuxppc-dev, linux-doc, linux-kernel, Mimi Zohar,
Dmitry Kasatkin, James Morris, Serge E. Hallyn, David Howells,
David Woodhouse, Jessica Yu, Herbert Xu, David S. Miller,
Jonathan Corbet, AKASHI, Takahiro
In-Reply-To: <9682b5d0-1634-2dd0-2cbb-eb1fa8ba7423@linux.microsoft.com>
Hello Jordan,
Jordan Hand <jorhand@linux.microsoft.com> writes:
> On 6/27/19 7:19 PM, Thiago Jung Bauermann wrote:
>> On the OpenPOWER platform, secure boot and trusted boot are being
>> implemented using IMA for taking measurements and verifying signatures.
>> Since the kernel image on Power servers is an ELF binary, kernels are
>> signed using the scripts/sign-file tool and thus use the same signature
>> format as signed kernel modules.
>>
>> This patch series adds support in IMA for verifying those signatures.
>> It adds flexibility to OpenPOWER secure boot, because it allows it to boot
>> kernels with the signature appended to them as well as kernels where the
>> signature is stored in the IMA extended attribute.
>
> I know this is pretty late, but I just wanted to let you know that I
> tested this patch set on x86_64 with QEMU.
>
> That is, I enrolled a key to _ima keyring, signed my kernel and modules
> with appended signatures (with scripts/sign-file), set the IMA policy to
> appraise and measure my kernel and modules. Also tested kexec appraisal.
>
> You can add my tested-by if you'd like.
Thanks for testing!
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply
* [RFC v1 0/2] RCU dyntick nesting counter cleanups
From: Joel Fernandes (Google) @ 2019-08-27 1:33 UTC (permalink / raw)
To: linux-kernel
Cc: Joel Fernandes (Google), Frederic Weisbecker, Jonathan Corbet,
Josh Triplett, kernel-team, Lai Jiangshan, linux-doc,
Mathieu Desnoyers, Mauro Carvalho Chehab, Paul E. McKenney, rcu,
Steven Rostedt
These patches clean up the usage of dynticks nesting counters simplifying the
code, while preserving the usecases.
It is a much needed simplification, makes the code less confusing, and prevents
future bugs such as those that arise from forgetting that the
dynticks_nmi_nesting counter is not a simple counter and can be "crowbarred" in
common situations.
Several nights of rcutorture testing with CONFIG_RCU_EQS_DEBUG on all RCU
kernel configurations have survived without any splats.
Further testing is in progress, hence marked as RFC!
thanks,
- Joel
Joel Fernandes (Google) (2):
rcu/tree: Clean up dynticks counter usage
rcu/tree: Remove dynticks_nmi_nesting counter
.../Data-Structures/Data-Structures.rst | 31 ++----
Documentation/RCU/stallwarn.txt | 6 +-
kernel/rcu/rcu.h | 4 -
kernel/rcu/tree.c | 98 +++++++++----------
kernel/rcu/tree.h | 4 +-
kernel/rcu/tree_stall.h | 4 +-
6 files changed, 64 insertions(+), 83 deletions(-)
--
2.23.0.187.g17f5b7556c-goog
^ permalink raw reply
* [RFC v1 1/2] rcu/tree: Clean up dynticks counter usage
From: Joel Fernandes (Google) @ 2019-08-27 1:33 UTC (permalink / raw)
To: linux-kernel
Cc: Joel Fernandes (Google), Frederic Weisbecker, Jonathan Corbet,
Josh Triplett, kernel-team, Lai Jiangshan, linux-doc,
Mathieu Desnoyers, Mauro Carvalho Chehab, Paul E. McKenney, rcu,
Steven Rostedt
The dynticks counter are confusing due to crowbar writes of
DYNTICK_IRQ_NONIDLE whose purpose is to detect half-interrupts (i.e. we
see rcu_irq_enter() but not rcu_irq_exit() due to a usermode upcall) and
if so then do a reset of the dyntick_nmi_nesting counters. This patch
tries to get rid of DYNTICK_IRQ_NONIDLE while still keeping the code
working, fully functional, and less confusing. The confusion recently
has even led to patches forgetting that DYNTICK_IRQ_NONIDLE was written
to which wasted lots of time.
The patch has the following changes:
(1) Use dynticks_nesting instead of dynticks_nmi_nesting for determining
outer most "EQS exit". This is needed to detect in
rcu_nmi_enter_common() if we have already EQS-exited, such as because of
a syscall. Currently we rely on a forced write of DYNTICK_IRQ_NONIDLE
from rcu_eqs_exit() for this purpose. This is one purpose of the
DYNTICK_IRQ_NONIDLE write (other than detecting half-interrupts).
However, we do not need to do that. dyntick_nesting already tells us that
we have EQS-exited so just use that thus removing the dependence of
dynticks_nmi_nesting for this purpose.
(2) Keep dynticks_nmi_nesting around because:
(a) rcu_is_cpu_rrupt_from_idle() needs to be able to detect first
interrupt nesting level.
(b) We need to detect half-interrupts till we are sure they're not an
issue. However, change the comparison to DYNTICK_IRQ_NONIDLE with 0.
(3) Since we got rid of DYNTICK_IRQ_NONIDLE, we also do cheaper
comparisons with zero instead for the code that keeps the tick on in
rcu_nmi_enter_common().
In the next patch, both of the concerns of (2) will be addressed and
then we can get rid of dynticks_nmi_nesting, however one step at a time.
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
kernel/rcu/rcu.h | 4 ----
kernel/rcu/tree.c | 60 ++++++++++++++++++++++++++++-------------------
2 files changed, 36 insertions(+), 28 deletions(-)
diff --git a/kernel/rcu/rcu.h b/kernel/rcu/rcu.h
index aeec70fda82c..046833f3784b 100644
--- a/kernel/rcu/rcu.h
+++ b/kernel/rcu/rcu.h
@@ -12,10 +12,6 @@
#include <trace/events/rcu.h>
-/* Offset to allow distinguishing irq vs. task-based idle entry/exit. */
-#define DYNTICK_IRQ_NONIDLE ((LONG_MAX / 2) + 1)
-
-
/*
* Grace-period counter management.
*/
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 68ebf0eb64c8..255cd6835526 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -81,7 +81,7 @@
static DEFINE_PER_CPU_SHARED_ALIGNED(struct rcu_data, rcu_data) = {
.dynticks_nesting = 1,
- .dynticks_nmi_nesting = DYNTICK_IRQ_NONIDLE,
+ .dynticks_nmi_nesting = 0,
.dynticks = ATOMIC_INIT(RCU_DYNTICK_CTRL_CTR),
};
struct rcu_state rcu_state = {
@@ -558,17 +558,18 @@ EXPORT_SYMBOL_GPL(rcutorture_get_gp_data);
/*
* Enter an RCU extended quiescent state, which can be either the
* idle loop or adaptive-tickless usermode execution.
- *
- * We crowbar the ->dynticks_nmi_nesting field to zero to allow for
- * the possibility of usermode upcalls having messed up our count
- * of interrupt nesting level during the prior busy period.
*/
static void rcu_eqs_enter(bool user)
{
struct rcu_data *rdp = this_cpu_ptr(&rcu_data);
- WARN_ON_ONCE(rdp->dynticks_nmi_nesting != DYNTICK_IRQ_NONIDLE);
- WRITE_ONCE(rdp->dynticks_nmi_nesting, 0);
+ /* Entering usermode/idle from interrupt is not handled. These would
+ * mean usermode upcalls or idle entry happened from interrupts. But,
+ * reset the counter if we warn.
+ */
+ if (WARN_ON_ONCE(rdp->dynticks_nmi_nesting != 0))
+ WRITE_ONCE(rdp->dynticks_nmi_nesting, 0);
+
WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
rdp->dynticks_nesting == 0);
if (rdp->dynticks_nesting != 1) {
@@ -642,23 +643,27 @@ static __always_inline void rcu_nmi_exit_common(bool irq)
* (We are exiting an NMI handler, so RCU better be paying attention
* to us!)
*/
+ WARN_ON_ONCE(rdp->dynticks_nesting <= 0);
WARN_ON_ONCE(rdp->dynticks_nmi_nesting <= 0);
WARN_ON_ONCE(rcu_dynticks_curr_cpu_in_eqs());
+ WRITE_ONCE(rdp->dynticks_nmi_nesting, /* No store tearing. */
+ rdp->dynticks_nmi_nesting - 1);
/*
* If the nesting level is not 1, the CPU wasn't RCU-idle, so
* leave it in non-RCU-idle state.
*/
- if (rdp->dynticks_nmi_nesting != 1) {
- trace_rcu_dyntick(TPS("--="), rdp->dynticks_nmi_nesting, rdp->dynticks_nmi_nesting - 2, rdp->dynticks);
- WRITE_ONCE(rdp->dynticks_nmi_nesting, /* No store tearing. */
- rdp->dynticks_nmi_nesting - 2);
+ if (rdp->dynticks_nesting != 1) {
+ trace_rcu_dyntick(TPS("--="), rdp->dynticks_nesting,
+ rdp->dynticks_nesting - 2, rdp->dynticks);
+ WRITE_ONCE(rdp->dynticks_nesting, /* No store tearing. */
+ rdp->dynticks_nesting - 2);
return;
}
/* This NMI interrupted an RCU-idle CPU, restore RCU-idleness. */
- trace_rcu_dyntick(TPS("Startirq"), rdp->dynticks_nmi_nesting, 0, rdp->dynticks);
- WRITE_ONCE(rdp->dynticks_nmi_nesting, 0); /* Avoid store tearing. */
+ trace_rcu_dyntick(TPS("Startirq"), rdp->dynticks_nesting, 0, rdp->dynticks);
+ WRITE_ONCE(rdp->dynticks_nesting, 0); /* Avoid store tearing. */
if (irq)
rcu_prepare_for_idle();
@@ -723,10 +728,6 @@ void rcu_irq_exit_irqson(void)
/*
* Exit an RCU extended quiescent state, which can be either the
* idle loop or adaptive-tickless usermode execution.
- *
- * We crowbar the ->dynticks_nmi_nesting field to DYNTICK_IRQ_NONIDLE to
- * allow for the possibility of usermode upcalls messing up our count of
- * interrupt nesting level during the busy period that is just now starting.
*/
static void rcu_eqs_exit(bool user)
{
@@ -747,8 +748,13 @@ static void rcu_eqs_exit(bool user)
trace_rcu_dyntick(TPS("End"), rdp->dynticks_nesting, 1, rdp->dynticks);
WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) && !user && !is_idle_task(current));
WRITE_ONCE(rdp->dynticks_nesting, 1);
- WARN_ON_ONCE(rdp->dynticks_nmi_nesting);
- WRITE_ONCE(rdp->dynticks_nmi_nesting, DYNTICK_IRQ_NONIDLE);
+
+ /* Exiting usermode/idle from interrupt is not handled. These would
+ * mean usermode upcalls or idle exit happened from interrupts. But,
+ * reset the counter if we warn.
+ */
+ if (WARN_ON_ONCE(rdp->dynticks_nmi_nesting != 0))
+ WRITE_ONCE(rdp->dynticks_nmi_nesting, 0);
}
/**
@@ -804,6 +810,7 @@ static __always_inline void rcu_nmi_enter_common(bool irq)
long incby = 2;
/* Complain about underflow. */
+ WARN_ON_ONCE(rdp->dynticks_nesting < 0);
WARN_ON_ONCE(rdp->dynticks_nmi_nesting < 0);
/*
@@ -826,16 +833,21 @@ static __always_inline void rcu_nmi_enter_common(bool irq)
incby = 1;
} else if (tick_nohz_full_cpu(rdp->cpu) &&
- rdp->dynticks_nmi_nesting == DYNTICK_IRQ_NONIDLE &&
- rdp->rcu_urgent_qs && !rdp->rcu_forced_tick) {
+ !rdp->dynticks_nmi_nesting && rdp->rcu_urgent_qs &&
+ !rdp->rcu_forced_tick) {
rdp->rcu_forced_tick = true;
tick_dep_set_cpu(rdp->cpu, TICK_DEP_BIT_RCU);
}
+
trace_rcu_dyntick(incby == 1 ? TPS("Endirq") : TPS("++="),
- rdp->dynticks_nmi_nesting,
- rdp->dynticks_nmi_nesting + incby, rdp->dynticks);
+ rdp->dynticks_nesting,
+ rdp->dynticks_nesting + incby, rdp->dynticks);
+
+ WRITE_ONCE(rdp->dynticks_nesting, /* Prevent store tearing. */
+ rdp->dynticks_nesting + incby);
+
WRITE_ONCE(rdp->dynticks_nmi_nesting, /* Prevent store tearing. */
- rdp->dynticks_nmi_nesting + incby);
+ rdp->dynticks_nmi_nesting + 1);
barrier();
}
--
2.23.0.187.g17f5b7556c-goog
^ permalink raw reply related
* [RFC v1 2/2] rcu/tree: Remove dynticks_nmi_nesting counter
From: Joel Fernandes (Google) @ 2019-08-27 1:33 UTC (permalink / raw)
To: linux-kernel
Cc: Joel Fernandes (Google), Frederic Weisbecker, Jonathan Corbet,
Josh Triplett, kernel-team, Lai Jiangshan, linux-doc,
Mathieu Desnoyers, Mauro Carvalho Chehab, Paul E. McKenney, rcu,
Steven Rostedt
The dynticks_nmi_nesting counter serves 4 purposes:
(a) rcu_is_cpu_rrupt_from_idle() needs to be able to detect first
interrupt nesting level.
(b) We need to detect half-interrupts till we are sure they're not an
issue. However, change the comparison to DYNTICK_IRQ_NONIDLE with 0.
(c) When a quiescent state report is needed from a nohz_full CPU.
The nesting counter detects we are a first level interrupt.
For (a) we can just use dyntick_nesting == 1 to determine this. Only the
outermost interrupt that interrupted an RCU-idle state can set it to 1.
For (b), this warning condition has not occurred for several kernel
releases. But we still keep the warning but change it to use
in_interrupt() instead of the nesting counter. In a later year, we can
remove the warning.
For (c), the nest check is not really necessary since forced_tick would
have been set to true in the outermost interrupt, so the nested/NMI
interrupts will check forced_tick anyway, and bail.
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
.../Data-Structures/Data-Structures.rst | 31 +++------
Documentation/RCU/stallwarn.txt | 6 +-
kernel/rcu/tree.c | 64 +++++++------------
kernel/rcu/tree.h | 4 +-
kernel/rcu/tree_stall.h | 4 +-
5 files changed, 41 insertions(+), 68 deletions(-)
diff --git a/Documentation/RCU/Design/Data-Structures/Data-Structures.rst b/Documentation/RCU/Design/Data-Structures/Data-Structures.rst
index 4a48e20a46f2..a5a907f434a1 100644
--- a/Documentation/RCU/Design/Data-Structures/Data-Structures.rst
+++ b/Documentation/RCU/Design/Data-Structures/Data-Structures.rst
@@ -936,10 +936,9 @@ This portion of the rcu_data structure is declared as follows:
::
1 long dynticks_nesting;
- 2 long dynticks_nmi_nesting;
- 3 atomic_t dynticks;
- 4 bool rcu_need_heavy_qs;
- 5 bool rcu_urgent_qs;
+ 2 atomic_t dynticks;
+ 3 bool rcu_need_heavy_qs;
+ 4 bool rcu_urgent_qs;
These fields in the rcu_data structure maintain the per-CPU dyntick-idle
state for the corresponding CPU. The fields may be accessed only from
@@ -948,26 +947,14 @@ the corresponding CPU (and from tracing) unless otherwise stated.
The ``->dynticks_nesting`` field counts the nesting depth of process
execution, so that in normal circumstances this counter has value zero
or one. NMIs, irqs, and tracers are counted by the
-``->dynticks_nmi_nesting`` field. Because NMIs cannot be masked, changes
+``->dynticks_nesting`` field as well. Because NMIs cannot be masked, changes
to this variable have to be undertaken carefully using an algorithm
provided by Andy Lutomirski. The initial transition from idle adds one,
and nested transitions add two, so that a nesting level of five is
-represented by a ``->dynticks_nmi_nesting`` value of nine. This counter
+represented by a ``->dynticks_nesting`` value of nine. This counter
can therefore be thought of as counting the number of reasons why this
-CPU cannot be permitted to enter dyntick-idle mode, aside from
-process-level transitions.
-
-However, it turns out that when running in non-idle kernel context, the
-Linux kernel is fully capable of entering interrupt handlers that never
-exit and perhaps also vice versa. Therefore, whenever the
-``->dynticks_nesting`` field is incremented up from zero, the
-``->dynticks_nmi_nesting`` field is set to a large positive number, and
-whenever the ``->dynticks_nesting`` field is decremented down to zero,
-the the ``->dynticks_nmi_nesting`` field is set to zero. Assuming that
-the number of misnested interrupts is not sufficient to overflow the
-counter, this approach corrects the ``->dynticks_nmi_nesting`` field
-every time the corresponding CPU enters the idle loop from process
-context.
+CPU cannot be permitted to enter dyntick-idle mode. It counts both the
+process-level and interrupt transitions.
The ``->dynticks`` field counts the corresponding CPU's transitions to
and from either dyntick-idle or user mode, so that this counter has an
@@ -1000,7 +987,9 @@ code.
+-----------------------------------------------------------------------+
| Because this would fail in the presence of interrupts whose handlers |
| never return and of handlers that manage to return from a made-up |
-| interrupt. |
+| interrupt. NOTE: The counters have now been combined however |
+| a temporary warning has been left to make sure this condition never |
+| occurs. |
+-----------------------------------------------------------------------+
Additional fields are present for some special-purpose builds, and are
diff --git a/Documentation/RCU/stallwarn.txt b/Documentation/RCU/stallwarn.txt
index f48f4621ccbc..585f73009a56 100644
--- a/Documentation/RCU/stallwarn.txt
+++ b/Documentation/RCU/stallwarn.txt
@@ -173,8 +173,8 @@ For non-RCU-tasks flavors of RCU, when a CPU detects that it is stalling,
it will print a message similar to the following:
INFO: rcu_sched detected stalls on CPUs/tasks:
- 2-...: (3 GPs behind) idle=06c/0/0 softirq=1453/1455 fqs=0
- 16-...: (0 ticks this GP) idle=81c/0/0 softirq=764/764 fqs=0
+ 2-...: (3 GPs behind) idle=06c/0 softirq=1453/1455 fqs=0
+ 16-...: (0 ticks this GP) idle=81c/0 softirq=764/764 fqs=0
(detected by 32, t=2603 jiffies, g=7075, q=625)
This message indicates that CPU 32 detected that CPUs 2 and 16 were both
@@ -225,7 +225,7 @@ an estimate of the total number of RCU callbacks queued across all CPUs
In kernels with CONFIG_RCU_FAST_NO_HZ, more information is printed
for each CPU:
- 0: (64628 ticks this GP) idle=dd5/3fffffffffffffff/0 softirq=82/543 last_accelerate: a345/d342 Nonlazy posted: ..D
+ 0: (64628 ticks this GP) idle=dd5/3fffffffffffffff softirq=82/543 last_accelerate: a345/d342 Nonlazy posted: ..D
The "last_accelerate:" prints the low-order 16 bits (in hex) of the
jiffies counter when this CPU last invoked rcu_try_advance_all_cbs()
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 255cd6835526..1465a3e406f8 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -81,7 +81,6 @@
static DEFINE_PER_CPU_SHARED_ALIGNED(struct rcu_data, rcu_data) = {
.dynticks_nesting = 1,
- .dynticks_nmi_nesting = 0,
.dynticks = ATOMIC_INIT(RCU_DYNTICK_CTRL_CTR),
};
struct rcu_state rcu_state = {
@@ -392,15 +391,9 @@ static int rcu_is_cpu_rrupt_from_idle(void)
/* Check for counter underflows */
RCU_LOCKDEP_WARN(__this_cpu_read(rcu_data.dynticks_nesting) < 0,
"RCU dynticks_nesting counter underflow!");
- RCU_LOCKDEP_WARN(__this_cpu_read(rcu_data.dynticks_nmi_nesting) <= 0,
- "RCU dynticks_nmi_nesting counter underflow/zero!");
- /* Are we at first interrupt nesting level? */
- if (__this_cpu_read(rcu_data.dynticks_nmi_nesting) != 1)
- return false;
-
- /* Does CPU appear to be idle from an RCU standpoint? */
- return __this_cpu_read(rcu_data.dynticks_nesting) == 0;
+ /* Are we the outermost interrupt that arrived when RCU was idle? */
+ return __this_cpu_read(rcu_data.dynticks_nesting) == 1;
}
#define DEFAULT_RCU_BLIMIT 10 /* Maximum callbacks per rcu_do_batch ... */
@@ -564,11 +557,10 @@ static void rcu_eqs_enter(bool user)
struct rcu_data *rdp = this_cpu_ptr(&rcu_data);
/* Entering usermode/idle from interrupt is not handled. These would
- * mean usermode upcalls or idle entry happened from interrupts. But,
- * reset the counter if we warn.
+ * mean usermode upcalls or idle exit happened from interrupts. Remove
+ * the warning by 2020.
*/
- if (WARN_ON_ONCE(rdp->dynticks_nmi_nesting != 0))
- WRITE_ONCE(rdp->dynticks_nmi_nesting, 0);
+ WARN_ON_ONCE(in_interrupt());
WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
rdp->dynticks_nesting == 0);
@@ -627,9 +619,8 @@ void rcu_user_enter(void)
/*
* If we are returning from the outermost NMI handler that interrupted an
- * RCU-idle period, update rdp->dynticks and rdp->dynticks_nmi_nesting
- * to let the RCU grace-period handling know that the CPU is back to
- * being RCU-idle.
+ * RCU-idle period, update rdp->dynticks to let the RCU grace-period handling
+ * know that the CPU is back to being RCU-idle.
*
* If you add or remove a call to rcu_nmi_exit_common(), be sure to test
* with CONFIG_RCU_EQS_DEBUG=y.
@@ -639,16 +630,13 @@ static __always_inline void rcu_nmi_exit_common(bool irq)
struct rcu_data *rdp = this_cpu_ptr(&rcu_data);
/*
- * Check for ->dynticks_nmi_nesting underflow and bad ->dynticks.
+ * Check for ->dynticks_nesting underflow and bad ->dynticks.
* (We are exiting an NMI handler, so RCU better be paying attention
* to us!)
*/
WARN_ON_ONCE(rdp->dynticks_nesting <= 0);
- WARN_ON_ONCE(rdp->dynticks_nmi_nesting <= 0);
WARN_ON_ONCE(rcu_dynticks_curr_cpu_in_eqs());
- WRITE_ONCE(rdp->dynticks_nmi_nesting, /* No store tearing. */
- rdp->dynticks_nmi_nesting - 1);
/*
* If the nesting level is not 1, the CPU wasn't RCU-idle, so
* leave it in non-RCU-idle state.
@@ -750,11 +738,10 @@ static void rcu_eqs_exit(bool user)
WRITE_ONCE(rdp->dynticks_nesting, 1);
/* Exiting usermode/idle from interrupt is not handled. These would
- * mean usermode upcalls or idle exit happened from interrupts. But,
- * reset the counter if we warn.
+ * mean usermode upcalls or idle exit happened from interrupts. Remove
+ * the warning by 2020.
*/
- if (WARN_ON_ONCE(rdp->dynticks_nmi_nesting != 0))
- WRITE_ONCE(rdp->dynticks_nmi_nesting, 0);
+ WARN_ON_ONCE(in_interrupt());
}
/**
@@ -795,14 +782,13 @@ void rcu_user_exit(void)
* rcu_nmi_enter_common - inform RCU of entry to NMI context
* @irq: Is this call from rcu_irq_enter?
*
- * If the CPU was idle from RCU's viewpoint, update rdp->dynticks and
- * rdp->dynticks_nmi_nesting to let the RCU grace-period handling know
- * that the CPU is active. This implementation permits nested NMIs, as
- * long as the nesting level does not overflow an int. (You will probably
- * run out of stack space first.)
+ * If the CPU was idle from RCU's viewpoint, update rdp->dynticks to let the
+ * RCU grace-period handling know that the CPU is active. This implementation
+ * permits nested NMIs, as long as the nesting level does not overflow a long.
+ * (You will probably run out of stack space first.)
*
- * If you add or remove a call to rcu_nmi_enter_common(), be sure to test
- * with CONFIG_RCU_EQS_DEBUG=y.
+ * If you add or remove a call to rcu_nmi_enter_common(), be sure to test with
+ * CONFIG_RCU_EQS_DEBUG=y.
*/
static __always_inline void rcu_nmi_enter_common(bool irq)
{
@@ -811,15 +797,16 @@ static __always_inline void rcu_nmi_enter_common(bool irq)
/* Complain about underflow. */
WARN_ON_ONCE(rdp->dynticks_nesting < 0);
- WARN_ON_ONCE(rdp->dynticks_nmi_nesting < 0);
/*
* If idle from RCU viewpoint, atomically increment ->dynticks
- * to mark non-idle and increment ->dynticks_nmi_nesting by one.
- * Otherwise, increment ->dynticks_nmi_nesting by two. This means
- * if ->dynticks_nmi_nesting is equal to one, we are guaranteed
+ * to mark non-idle and increment ->dynticks_nesting by one.
+ * Otherwise, increment ->dynticks_nesting by two. This means
+ * if ->dynticks_nesting is equal to one, we are guaranteed
* to be in the outermost NMI handler that interrupted an RCU-idle
- * period (observation due to Andy Lutomirski).
+ * period (observation due to Andy Lutomirski). An exception
+ * is if the interrupt arrived in kernel mode; in this case we would
+ * be the outermost interrupt but still increment by 2 which is Ok.
*/
if (rcu_dynticks_curr_cpu_in_eqs()) {
@@ -832,8 +819,7 @@ static __always_inline void rcu_nmi_enter_common(bool irq)
rcu_cleanup_after_idle();
incby = 1;
- } else if (tick_nohz_full_cpu(rdp->cpu) &&
- !rdp->dynticks_nmi_nesting && rdp->rcu_urgent_qs &&
+ } else if (tick_nohz_full_cpu(rdp->cpu) && rdp->rcu_urgent_qs &&
!rdp->rcu_forced_tick) {
rdp->rcu_forced_tick = true;
tick_dep_set_cpu(rdp->cpu, TICK_DEP_BIT_RCU);
@@ -846,8 +832,6 @@ static __always_inline void rcu_nmi_enter_common(bool irq)
WRITE_ONCE(rdp->dynticks_nesting, /* Prevent store tearing. */
rdp->dynticks_nesting + incby);
- WRITE_ONCE(rdp->dynticks_nmi_nesting, /* Prevent store tearing. */
- rdp->dynticks_nmi_nesting + 1);
barrier();
}
diff --git a/kernel/rcu/tree.h b/kernel/rcu/tree.h
index 055c31781d3a..ad7d3e31c5cf 100644
--- a/kernel/rcu/tree.h
+++ b/kernel/rcu/tree.h
@@ -176,8 +176,8 @@ struct rcu_data {
/* 3) dynticks interface. */
int dynticks_snap; /* Per-GP tracking for dynticks. */
- long dynticks_nesting; /* Track process nesting level. */
- long dynticks_nmi_nesting; /* Track irq/NMI nesting level. */
+ long dynticks_nesting; /* Track dyntick (non-IDLE) nesting
+ * level for kernel entry and interrupt. */
atomic_t dynticks; /* Even value for idle, else odd. */
bool rcu_need_heavy_qs; /* GP old, so heavy quiescent state! */
bool rcu_urgent_qs; /* GP old need light quiescent state. */
diff --git a/kernel/rcu/tree_stall.h b/kernel/rcu/tree_stall.h
index 841ab43f3e60..0676460107d0 100644
--- a/kernel/rcu/tree_stall.h
+++ b/kernel/rcu/tree_stall.h
@@ -313,7 +313,7 @@ static void print_cpu_stall_info(int cpu)
}
print_cpu_stall_fast_no_hz(fast_no_hz, cpu);
delta = rcu_seq_ctr(rdp->mynode->gp_seq - rdp->rcu_iw_gp_seq);
- pr_err("\t%d-%c%c%c%c: (%lu %s) idle=%03x/%ld/%#lx softirq=%u/%u fqs=%ld %s\n",
+ pr_err("\t%d-%c%c%c%c: (%lu %s) idle=%03x/%ld softirq=%u/%u fqs=%ld %s\n",
cpu,
"O."[!!cpu_online(cpu)],
"o."[!!(rdp->grpmask & rdp->mynode->qsmaskinit)],
@@ -323,7 +323,7 @@ static void print_cpu_stall_info(int cpu)
"!."[!delta],
ticks_value, ticks_title,
rcu_dynticks_snap(rdp) & 0xfff,
- rdp->dynticks_nesting, rdp->dynticks_nmi_nesting,
+ rdp->dynticks_nesting,
rdp->softirq_snap, kstat_softirqs_cpu(RCU_SOFTIRQ, cpu),
READ_ONCE(rcu_state.n_force_qs) - rcu_state.n_force_qs_gpstart,
fast_no_hz);
--
2.23.0.187.g17f5b7556c-goog
^ permalink raw reply related
* Re: [RFC v1 2/2] rcu/tree: Remove dynticks_nmi_nesting counter
From: Joel Fernandes @ 2019-08-27 1:40 UTC (permalink / raw)
To: LKML
Cc: Frederic Weisbecker, Jonathan Corbet, Josh Triplett, kernel-team,
Lai Jiangshan, open list:DOCUMENTATION, Mathieu Desnoyers,
Mauro Carvalho Chehab, Paul E. McKenney, rcu, Steven Rostedt
In-Reply-To: <5d648897.1c69fb81.5e60a.fc70@mx.google.com>
On Mon, Aug 26, 2019 at 9:34 PM Joel Fernandes (Google)
<joel@joelfernandes.org> wrote:
>
> The dynticks_nmi_nesting counter serves 4 purposes:
>
And actually, I meant 3 purposes ;-) :-P
thanks,
- Joel
> (a) rcu_is_cpu_rrupt_from_idle() needs to be able to detect first
> interrupt nesting level.
>
> (b) We need to detect half-interrupts till we are sure they're not an
> issue. However, change the comparison to DYNTICK_IRQ_NONIDLE with 0.
>
> (c) When a quiescent state report is needed from a nohz_full CPU.
> The nesting counter detects we are a first level interrupt.
>
> For (a) we can just use dyntick_nesting == 1 to determine this. Only the
> outermost interrupt that interrupted an RCU-idle state can set it to 1.
>
> For (b), this warning condition has not occurred for several kernel
> releases. But we still keep the warning but change it to use
> in_interrupt() instead of the nesting counter. In a later year, we can
> remove the warning.
>
> For (c), the nest check is not really necessary since forced_tick would
> have been set to true in the outermost interrupt, so the nested/NMI
> interrupts will check forced_tick anyway, and bail.
>
> Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> ---
> .../Data-Structures/Data-Structures.rst | 31 +++------
> Documentation/RCU/stallwarn.txt | 6 +-
> kernel/rcu/tree.c | 64 +++++++------------
> kernel/rcu/tree.h | 4 +-
> kernel/rcu/tree_stall.h | 4 +-
> 5 files changed, 41 insertions(+), 68 deletions(-)
>
> diff --git a/Documentation/RCU/Design/Data-Structures/Data-Structures.rst b/Documentation/RCU/Design/Data-Structures/Data-Structures.rst
> index 4a48e20a46f2..a5a907f434a1 100644
> --- a/Documentation/RCU/Design/Data-Structures/Data-Structures.rst
> +++ b/Documentation/RCU/Design/Data-Structures/Data-Structures.rst
> @@ -936,10 +936,9 @@ This portion of the rcu_data structure is declared as follows:
> ::
>
> 1 long dynticks_nesting;
> - 2 long dynticks_nmi_nesting;
> - 3 atomic_t dynticks;
> - 4 bool rcu_need_heavy_qs;
> - 5 bool rcu_urgent_qs;
> + 2 atomic_t dynticks;
> + 3 bool rcu_need_heavy_qs;
> + 4 bool rcu_urgent_qs;
>
> These fields in the rcu_data structure maintain the per-CPU dyntick-idle
> state for the corresponding CPU. The fields may be accessed only from
> @@ -948,26 +947,14 @@ the corresponding CPU (and from tracing) unless otherwise stated.
> The ``->dynticks_nesting`` field counts the nesting depth of process
> execution, so that in normal circumstances this counter has value zero
> or one. NMIs, irqs, and tracers are counted by the
> -``->dynticks_nmi_nesting`` field. Because NMIs cannot be masked, changes
> +``->dynticks_nesting`` field as well. Because NMIs cannot be masked, changes
> to this variable have to be undertaken carefully using an algorithm
> provided by Andy Lutomirski. The initial transition from idle adds one,
> and nested transitions add two, so that a nesting level of five is
> -represented by a ``->dynticks_nmi_nesting`` value of nine. This counter
> +represented by a ``->dynticks_nesting`` value of nine. This counter
> can therefore be thought of as counting the number of reasons why this
> -CPU cannot be permitted to enter dyntick-idle mode, aside from
> -process-level transitions.
> -
> -However, it turns out that when running in non-idle kernel context, the
> -Linux kernel is fully capable of entering interrupt handlers that never
> -exit and perhaps also vice versa. Therefore, whenever the
> -``->dynticks_nesting`` field is incremented up from zero, the
> -``->dynticks_nmi_nesting`` field is set to a large positive number, and
> -whenever the ``->dynticks_nesting`` field is decremented down to zero,
> -the the ``->dynticks_nmi_nesting`` field is set to zero. Assuming that
> -the number of misnested interrupts is not sufficient to overflow the
> -counter, this approach corrects the ``->dynticks_nmi_nesting`` field
> -every time the corresponding CPU enters the idle loop from process
> -context.
> +CPU cannot be permitted to enter dyntick-idle mode. It counts both the
> +process-level and interrupt transitions.
>
> The ``->dynticks`` field counts the corresponding CPU's transitions to
> and from either dyntick-idle or user mode, so that this counter has an
> @@ -1000,7 +987,9 @@ code.
> +-----------------------------------------------------------------------+
> | Because this would fail in the presence of interrupts whose handlers |
> | never return and of handlers that manage to return from a made-up |
> -| interrupt. |
> +| interrupt. NOTE: The counters have now been combined however |
> +| a temporary warning has been left to make sure this condition never |
> +| occurs. |
> +-----------------------------------------------------------------------+
>
> Additional fields are present for some special-purpose builds, and are
> diff --git a/Documentation/RCU/stallwarn.txt b/Documentation/RCU/stallwarn.txt
> index f48f4621ccbc..585f73009a56 100644
> --- a/Documentation/RCU/stallwarn.txt
> +++ b/Documentation/RCU/stallwarn.txt
> @@ -173,8 +173,8 @@ For non-RCU-tasks flavors of RCU, when a CPU detects that it is stalling,
> it will print a message similar to the following:
>
> INFO: rcu_sched detected stalls on CPUs/tasks:
> - 2-...: (3 GPs behind) idle=06c/0/0 softirq=1453/1455 fqs=0
> - 16-...: (0 ticks this GP) idle=81c/0/0 softirq=764/764 fqs=0
> + 2-...: (3 GPs behind) idle=06c/0 softirq=1453/1455 fqs=0
> + 16-...: (0 ticks this GP) idle=81c/0 softirq=764/764 fqs=0
> (detected by 32, t=2603 jiffies, g=7075, q=625)
>
> This message indicates that CPU 32 detected that CPUs 2 and 16 were both
> @@ -225,7 +225,7 @@ an estimate of the total number of RCU callbacks queued across all CPUs
> In kernels with CONFIG_RCU_FAST_NO_HZ, more information is printed
> for each CPU:
>
> - 0: (64628 ticks this GP) idle=dd5/3fffffffffffffff/0 softirq=82/543 last_accelerate: a345/d342 Nonlazy posted: ..D
> + 0: (64628 ticks this GP) idle=dd5/3fffffffffffffff softirq=82/543 last_accelerate: a345/d342 Nonlazy posted: ..D
>
> The "last_accelerate:" prints the low-order 16 bits (in hex) of the
> jiffies counter when this CPU last invoked rcu_try_advance_all_cbs()
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index 255cd6835526..1465a3e406f8 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -81,7 +81,6 @@
>
> static DEFINE_PER_CPU_SHARED_ALIGNED(struct rcu_data, rcu_data) = {
> .dynticks_nesting = 1,
> - .dynticks_nmi_nesting = 0,
> .dynticks = ATOMIC_INIT(RCU_DYNTICK_CTRL_CTR),
> };
> struct rcu_state rcu_state = {
> @@ -392,15 +391,9 @@ static int rcu_is_cpu_rrupt_from_idle(void)
> /* Check for counter underflows */
> RCU_LOCKDEP_WARN(__this_cpu_read(rcu_data.dynticks_nesting) < 0,
> "RCU dynticks_nesting counter underflow!");
> - RCU_LOCKDEP_WARN(__this_cpu_read(rcu_data.dynticks_nmi_nesting) <= 0,
> - "RCU dynticks_nmi_nesting counter underflow/zero!");
>
> - /* Are we at first interrupt nesting level? */
> - if (__this_cpu_read(rcu_data.dynticks_nmi_nesting) != 1)
> - return false;
> -
> - /* Does CPU appear to be idle from an RCU standpoint? */
> - return __this_cpu_read(rcu_data.dynticks_nesting) == 0;
> + /* Are we the outermost interrupt that arrived when RCU was idle? */
> + return __this_cpu_read(rcu_data.dynticks_nesting) == 1;
> }
>
> #define DEFAULT_RCU_BLIMIT 10 /* Maximum callbacks per rcu_do_batch ... */
> @@ -564,11 +557,10 @@ static void rcu_eqs_enter(bool user)
> struct rcu_data *rdp = this_cpu_ptr(&rcu_data);
>
> /* Entering usermode/idle from interrupt is not handled. These would
> - * mean usermode upcalls or idle entry happened from interrupts. But,
> - * reset the counter if we warn.
> + * mean usermode upcalls or idle exit happened from interrupts. Remove
> + * the warning by 2020.
> */
> - if (WARN_ON_ONCE(rdp->dynticks_nmi_nesting != 0))
> - WRITE_ONCE(rdp->dynticks_nmi_nesting, 0);
> + WARN_ON_ONCE(in_interrupt());
>
> WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
> rdp->dynticks_nesting == 0);
> @@ -627,9 +619,8 @@ void rcu_user_enter(void)
>
> /*
> * If we are returning from the outermost NMI handler that interrupted an
> - * RCU-idle period, update rdp->dynticks and rdp->dynticks_nmi_nesting
> - * to let the RCU grace-period handling know that the CPU is back to
> - * being RCU-idle.
> + * RCU-idle period, update rdp->dynticks to let the RCU grace-period handling
> + * know that the CPU is back to being RCU-idle.
> *
> * If you add or remove a call to rcu_nmi_exit_common(), be sure to test
> * with CONFIG_RCU_EQS_DEBUG=y.
> @@ -639,16 +630,13 @@ static __always_inline void rcu_nmi_exit_common(bool irq)
> struct rcu_data *rdp = this_cpu_ptr(&rcu_data);
>
> /*
> - * Check for ->dynticks_nmi_nesting underflow and bad ->dynticks.
> + * Check for ->dynticks_nesting underflow and bad ->dynticks.
> * (We are exiting an NMI handler, so RCU better be paying attention
> * to us!)
> */
> WARN_ON_ONCE(rdp->dynticks_nesting <= 0);
> - WARN_ON_ONCE(rdp->dynticks_nmi_nesting <= 0);
> WARN_ON_ONCE(rcu_dynticks_curr_cpu_in_eqs());
>
> - WRITE_ONCE(rdp->dynticks_nmi_nesting, /* No store tearing. */
> - rdp->dynticks_nmi_nesting - 1);
> /*
> * If the nesting level is not 1, the CPU wasn't RCU-idle, so
> * leave it in non-RCU-idle state.
> @@ -750,11 +738,10 @@ static void rcu_eqs_exit(bool user)
> WRITE_ONCE(rdp->dynticks_nesting, 1);
>
> /* Exiting usermode/idle from interrupt is not handled. These would
> - * mean usermode upcalls or idle exit happened from interrupts. But,
> - * reset the counter if we warn.
> + * mean usermode upcalls or idle exit happened from interrupts. Remove
> + * the warning by 2020.
> */
> - if (WARN_ON_ONCE(rdp->dynticks_nmi_nesting != 0))
> - WRITE_ONCE(rdp->dynticks_nmi_nesting, 0);
> + WARN_ON_ONCE(in_interrupt());
> }
>
> /**
> @@ -795,14 +782,13 @@ void rcu_user_exit(void)
> * rcu_nmi_enter_common - inform RCU of entry to NMI context
> * @irq: Is this call from rcu_irq_enter?
> *
> - * If the CPU was idle from RCU's viewpoint, update rdp->dynticks and
> - * rdp->dynticks_nmi_nesting to let the RCU grace-period handling know
> - * that the CPU is active. This implementation permits nested NMIs, as
> - * long as the nesting level does not overflow an int. (You will probably
> - * run out of stack space first.)
> + * If the CPU was idle from RCU's viewpoint, update rdp->dynticks to let the
> + * RCU grace-period handling know that the CPU is active. This implementation
> + * permits nested NMIs, as long as the nesting level does not overflow a long.
> + * (You will probably run out of stack space first.)
> *
> - * If you add or remove a call to rcu_nmi_enter_common(), be sure to test
> - * with CONFIG_RCU_EQS_DEBUG=y.
> + * If you add or remove a call to rcu_nmi_enter_common(), be sure to test with
> + * CONFIG_RCU_EQS_DEBUG=y.
> */
> static __always_inline void rcu_nmi_enter_common(bool irq)
> {
> @@ -811,15 +797,16 @@ static __always_inline void rcu_nmi_enter_common(bool irq)
>
> /* Complain about underflow. */
> WARN_ON_ONCE(rdp->dynticks_nesting < 0);
> - WARN_ON_ONCE(rdp->dynticks_nmi_nesting < 0);
>
> /*
> * If idle from RCU viewpoint, atomically increment ->dynticks
> - * to mark non-idle and increment ->dynticks_nmi_nesting by one.
> - * Otherwise, increment ->dynticks_nmi_nesting by two. This means
> - * if ->dynticks_nmi_nesting is equal to one, we are guaranteed
> + * to mark non-idle and increment ->dynticks_nesting by one.
> + * Otherwise, increment ->dynticks_nesting by two. This means
> + * if ->dynticks_nesting is equal to one, we are guaranteed
> * to be in the outermost NMI handler that interrupted an RCU-idle
> - * period (observation due to Andy Lutomirski).
> + * period (observation due to Andy Lutomirski). An exception
> + * is if the interrupt arrived in kernel mode; in this case we would
> + * be the outermost interrupt but still increment by 2 which is Ok.
> */
> if (rcu_dynticks_curr_cpu_in_eqs()) {
>
> @@ -832,8 +819,7 @@ static __always_inline void rcu_nmi_enter_common(bool irq)
> rcu_cleanup_after_idle();
>
> incby = 1;
> - } else if (tick_nohz_full_cpu(rdp->cpu) &&
> - !rdp->dynticks_nmi_nesting && rdp->rcu_urgent_qs &&
> + } else if (tick_nohz_full_cpu(rdp->cpu) && rdp->rcu_urgent_qs &&
> !rdp->rcu_forced_tick) {
> rdp->rcu_forced_tick = true;
> tick_dep_set_cpu(rdp->cpu, TICK_DEP_BIT_RCU);
> @@ -846,8 +832,6 @@ static __always_inline void rcu_nmi_enter_common(bool irq)
> WRITE_ONCE(rdp->dynticks_nesting, /* Prevent store tearing. */
> rdp->dynticks_nesting + incby);
>
> - WRITE_ONCE(rdp->dynticks_nmi_nesting, /* Prevent store tearing. */
> - rdp->dynticks_nmi_nesting + 1);
> barrier();
> }
>
> diff --git a/kernel/rcu/tree.h b/kernel/rcu/tree.h
> index 055c31781d3a..ad7d3e31c5cf 100644
> --- a/kernel/rcu/tree.h
> +++ b/kernel/rcu/tree.h
> @@ -176,8 +176,8 @@ struct rcu_data {
>
> /* 3) dynticks interface. */
> int dynticks_snap; /* Per-GP tracking for dynticks. */
> - long dynticks_nesting; /* Track process nesting level. */
> - long dynticks_nmi_nesting; /* Track irq/NMI nesting level. */
> + long dynticks_nesting; /* Track dyntick (non-IDLE) nesting
> + * level for kernel entry and interrupt. */
> atomic_t dynticks; /* Even value for idle, else odd. */
> bool rcu_need_heavy_qs; /* GP old, so heavy quiescent state! */
> bool rcu_urgent_qs; /* GP old need light quiescent state. */
> diff --git a/kernel/rcu/tree_stall.h b/kernel/rcu/tree_stall.h
> index 841ab43f3e60..0676460107d0 100644
> --- a/kernel/rcu/tree_stall.h
> +++ b/kernel/rcu/tree_stall.h
> @@ -313,7 +313,7 @@ static void print_cpu_stall_info(int cpu)
> }
> print_cpu_stall_fast_no_hz(fast_no_hz, cpu);
> delta = rcu_seq_ctr(rdp->mynode->gp_seq - rdp->rcu_iw_gp_seq);
> - pr_err("\t%d-%c%c%c%c: (%lu %s) idle=%03x/%ld/%#lx softirq=%u/%u fqs=%ld %s\n",
> + pr_err("\t%d-%c%c%c%c: (%lu %s) idle=%03x/%ld softirq=%u/%u fqs=%ld %s\n",
> cpu,
> "O."[!!cpu_online(cpu)],
> "o."[!!(rdp->grpmask & rdp->mynode->qsmaskinit)],
> @@ -323,7 +323,7 @@ static void print_cpu_stall_info(int cpu)
> "!."[!delta],
> ticks_value, ticks_title,
> rcu_dynticks_snap(rdp) & 0xfff,
> - rdp->dynticks_nesting, rdp->dynticks_nmi_nesting,
> + rdp->dynticks_nesting,
> rdp->softirq_snap, kstat_softirqs_cpu(RCU_SOFTIRQ, cpu),
> READ_ONCE(rcu_state.n_force_qs) - rcu_state.n_force_qs_gpstart,
> fast_no_hz);
> --
> 2.23.0.187.g17f5b7556c-goog
>
^ permalink raw reply
* Re: [RFC PATCH v3 01/19] skc: Add supplemental kernel cmdline support
From: Masami Hiramatsu @ 2019-08-27 2:37 UTC (permalink / raw)
To: Rob Herring
Cc: Steven Rostedt, Frank Rowand, Ingo Molnar, Namhyung Kim, Tim Bird,
Jiri Olsa, Arnaldo Carvalho de Melo, Tom Zanussi, Andrew Morton,
Thomas Gleixner, Greg Kroah-Hartman, Alexey Dobriyan,
Jonathan Corbet, Linus Torvalds, Linux Doc Mailing List,
linux-fsdevel, linux-kernel@vger.kernel.org
In-Reply-To: <CAL_Jsq+Pm4D_fm+iG9UfGSObx2fSXshZuMW4QKwGePbg4RUEjA@mail.gmail.com>
Hi Rob,
Thank you for your comment!
On Mon, 26 Aug 2019 08:27:48 -0500
Rob Herring <robh+dt@kernel.org> wrote:
> On Sun, Aug 25, 2019 at 10:15 PM Masami Hiramatsu <mhiramat@kernel.org> wrote:
> >
> > Supplemental kernel command line (SKC) allows admin to pass a
> > tree-structured supplemental kernel commandline file (SKC file)
> > when boot up kernel. This expands the kernel command line in
> > efficient way.
> >
> > SKC file will contain some key-value commands, e.g.
> >
> > key.word = value1;
> > another.key.word = value2;
> >
> > It can fold same keys with braces, also you can write array
> > data. For example,
> >
> > key {
> > word1 {
> > setting1 = data;
> > setting2;
> > }
> > word2.array = "val1", "val2";
> > }
>
> Why invent a custom file format? You could use YAML (or JSON):
Yeah, actually my early idea was using JSON, since it is widely used and
many good tools. However, I thought that is not human friendly format :(.
I would like to give an easy to read/write but structured interface.
>
> key:
> word1:
> setting1: data
> setting2: true
> word2:
> - val1
> - val2
(Ah, in above example "array" is just a part of key, and is not
a reserved word.)
> That would allow you to define a schema for defined options and can
> easily be manipulated with python (or any language with dictionaries
> and lists). That does imply adding a YAML parser to the kernel which
> I'm not sure is a great idea. There is a C parser lib, but working
> with YAML in C is not that great compared to python.
Yes, using plain YAML maybe requires user-space coverter to some
other format.
>
> Another option would be using the DTS format, but as a separate file.
> That's not unprecedented as u-boot FIT image is a DTB. Then the kernel
> already has the parser. And you could still have schema now.
Yeah, that is what I consider at first. I discussed it with Frank at
OSSJ, but he suggested to not use DTS, nor touch current parser in kernel.
So I finally convinced not using DTS.
> A new interface will take a lot of bootloader work to make it easy to
> use given the user has to manually load some file in the bootloader
> and know a good address to load it to.
Right, that is what I have to do next if this is accepted. As I shown, I
modified Qemu and Grub. (Since U-Boot is very flexible, it is easy to
load skc file and modify bootargs by manual.)
What I found was, since the bootloaders already supported loading DTB,
it would not be so hard to add loading another file :) (curiously, the
most complicated part was modifying kernel cmdline)
> Between that and rebuilding the
> kernel with the configuration, I'd pick rebuilding the kernel. Perhaps
> this version will highlight that the original proposal was not so bad.
Maybe for embedded, yes. For admins who use vendor kernel, no.
> Another thought, maybe you could process the configuration file that's
> in a readable/editable format into a flat representation that could
> simply be added to the kernel command line:
(BTW, it is easy to make a flat representation data as you can see
in /proc/sup_cmdline, which is added by [2/19])
>
> key.word1.setting1=data key.word1.setting2 key.word2=val1,val2
>
> That would then use an existing interface and probably simplify the
> kernel parsing.
Hmm, if it is just for passing extended arguments, that will be enough
(that was my first version of SKC, here
https://github.com/mhiramat/skc/tree/5f0429c244d1c9f8f84711bc33e1e6f90df62df8 )
But I found that was not enough flexible for my usage. For expressing
complex ftrace settings (e.g. nesting options, some options related to
other options etc.), I need tree-structured data, something like Devicetree.
Thank you,
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH v3] powerpc/fadump: sysfs for fadump memory reservation
From: Michael Ellerman @ 2019-08-27 3:19 UTC (permalink / raw)
To: Hari Bathini, Sourabh Jain
Cc: mahesh, linux-doc, linuxppc-dev, linux-kernel, corbet, gregkh
In-Reply-To: <f8e9cbdd-1926-081d-c8e6-f9d55408fe51@linux.ibm.com>
Hari Bathini <hbathini@linux.ibm.com> writes:
> On 26/08/19 4:14 PM, Sourabh Jain wrote:
>> On 8/26/19 3:46 PM, Sourabh Jain wrote:
>>> On 8/26/19 3:29 PM, Hari Bathini wrote:
>>>> On 10/08/19 11:29 PM, Sourabh Jain wrote:
>>>>> Add a sys interface to allow querying the memory reserved by
>>>>> fadump for saving the crash dump.
>>>>>
>>>>> Add an ABI doc entry for new sysfs interface.
>>>>> - /sys/kernel/fadump_mem_reserved
>>>>>
>>>>> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
>>>>> ---
>>>>> Changelog:
>>>>> v1 -> v2:
>>>>> - Added ABI doc for new sysfs interface.
>>>>>
>>>>> v2 -> v3:
>>>>> - Updated the ABI documentation.
>>>>> ---
>>>>>
>>>>> Documentation/ABI/testing/sysfs-kernel-fadump | 6 ++++++
>>>>
>>>> Shouldn't this be Documentation/ABI/testing/sysfs-kernel-fadump_mem_reserved?
>>
>> How about documenting fadump_mem_reserved and other sysfs attributes suggested
>> by you in a single file Documentation/ABI/testing/sysfs-kernel-fadump?
>
> I wouldn't mind that but please do check if it is breaking a convention..
AIUI a file named like that would hold the documentation for the files
inside a directory called /sys/kernel/fadump.
And in fact that's probably where these files should live, rather than
just dropped directly into /sys/kernel.
cheers
^ permalink raw reply
* Re: [PATCH v3] powerpc/fadump: sysfs for fadump memory reservation
From: Hari Bathini @ 2019-08-27 6:02 UTC (permalink / raw)
To: Michael Ellerman, Sourabh Jain
Cc: corbet, mahesh, linux-doc, linux-kernel, gregkh, linuxppc-dev
In-Reply-To: <87sgpn2t2w.fsf@concordia.ellerman.id.au>
On 27/08/19 8:49 AM, Michael Ellerman wrote:
> Hari Bathini <hbathini@linux.ibm.com> writes:
>> On 26/08/19 4:14 PM, Sourabh Jain wrote:
>>> On 8/26/19 3:46 PM, Sourabh Jain wrote:
>>>> On 8/26/19 3:29 PM, Hari Bathini wrote:
>>>>> On 10/08/19 11:29 PM, Sourabh Jain wrote:
>>>>>> Add a sys interface to allow querying the memory reserved by
>>>>>> fadump for saving the crash dump.
>>>>>>
>>>>>> Add an ABI doc entry for new sysfs interface.
>>>>>> - /sys/kernel/fadump_mem_reserved
>>>>>>
>>>>>> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
>>>>>> ---
>>>>>> Changelog:
>>>>>> v1 -> v2:
>>>>>> - Added ABI doc for new sysfs interface.
>>>>>>
>>>>>> v2 -> v3:
>>>>>> - Updated the ABI documentation.
>>>>>> ---
>>>>>>
>>>>>> Documentation/ABI/testing/sysfs-kernel-fadump | 6 ++++++
>>>>>
>>>>> Shouldn't this be Documentation/ABI/testing/sysfs-kernel-fadump_mem_reserved?
>>>
>>> How about documenting fadump_mem_reserved and other sysfs attributes suggested
>>> by you in a single file Documentation/ABI/testing/sysfs-kernel-fadump?
>>
>> I wouldn't mind that but please do check if it is breaking a convention..
>
> AIUI a file named like that would hold the documentation for the files
> inside a directory called /sys/kernel/fadump.
>
> And in fact that's probably where these files should live, rather than
> just dropped directly into /sys/kernel.
Michael, could that be corrected now by introducing new sysfs files for FADump in
/sys/kernel/fadump/.
Also, duplicating current /sys/kernel/fadump_* files as /sys/kernel/fadump/* files
& eventually dropping /sys/kernel/fadump_* files sometime later..
- Hari
^ permalink raw reply
* Re: [PATCH V3 0/3] KVM/Hyper-V: Add Hyper-V direct tlb flush support
From: Vitaly Kuznetsov @ 2019-08-27 6:41 UTC (permalink / raw)
To: lantianyu1986
Cc: Tianyu Lan, kvm, linux-doc, linux-hyperv, linux-kernel, pbonzini,
rkrcmar, corbet, kys, haiyangz, sthemmin, sashal, tglx, mingo, bp,
hpa, x86, michael.h.kelley
In-Reply-To: <20190819131737.26942-1-Tianyu.Lan@microsoft.com>
lantianyu1986@gmail.com writes:
> From: Tianyu Lan <Tianyu.Lan@microsoft.com>
>
> This patchset is to add Hyper-V direct tlb support in KVM. Hyper-V
> in L0 can delegate L1 hypervisor to handle tlb flush request from
> L2 guest when direct tlb flush is enabled in L1.
>
> Patch 2 introduces new cap KVM_CAP_HYPERV_DIRECT_TLBFLUSH to enable
> feature from user space. User space should enable this feature only
> when Hyper-V hypervisor capability is exposed to guest and KVM profile
> is hided. There is a parameter conflict between KVM and Hyper-V hypercall.
> We hope L2 guest doesn't use KVM hypercall when the feature is
> enabled. Detail please see comment of new API
> "KVM_CAP_HYPERV_DIRECT_TLBFLUSH"
I was thinking about this for awhile and I think I have a better
proposal. Instead of adding this new capability let's enable direct TLB
flush when KVM guest enables Hyper-V Hypercall page (writes to
HV_X64_MSR_HYPERCALL) - this guarantees that the guest doesn't need KVM
hypercalls as we can't handle both KVM-style and Hyper-V-style
hypercalls simultaneously and kvm_emulate_hypercall() does:
if (kvm_hv_hypercall_enabled(vcpu->kvm))
return kvm_hv_hypercall(vcpu);
What do you think?
(and instead of adding the capability we can add kvm.ko module parameter
to enable direct tlb flush unconditionally, like
'hv_direct_tlbflush=-1/0/1' with '-1' being the default (autoselect
based on Hyper-V hypercall enablement, '0' - permanently disabled, '1' -
permanenetly enabled)).
--
Vitaly
^ permalink raw reply
* [PATCH 5.2 127/162] x86/CPU/AMD: Clear RDRAND CPUID bit on AMD family 15h/16h
From: Greg Kroah-Hartman @ 2019-08-27 7:50 UTC (permalink / raw)
To: linux-kernel
Cc: Greg Kroah-Hartman, stable, Tom Lendacky, Borislav Petkov,
Andrew Cooper, Andrew Morton, Chen Yu, H. Peter Anvin,
Ingo Molnar, Jonathan Corbet, Josh Poimboeuf, Juergen Gross,
Kees Cook, linux-doc@vger.kernel.org, linux-pm@vger.kernel.org,
Nathan Chancellor, Paolo Bonzini, Pavel Machek, Rafael J. Wysocki,
Thomas Gleixner, x86@kernel.org
In-Reply-To: <20190827072738.093683223@linuxfoundation.org>
From: Tom Lendacky <thomas.lendacky@amd.com>
commit c49a0a80137c7ca7d6ced4c812c9e07a949f6f24 upstream.
There have been reports of RDRAND issues after resuming from suspend on
some AMD family 15h and family 16h systems. This issue stems from a BIOS
not performing the proper steps during resume to ensure RDRAND continues
to function properly.
RDRAND support is indicated by CPUID Fn00000001_ECX[30]. This bit can be
reset by clearing MSR C001_1004[62]. Any software that checks for RDRAND
support using CPUID, including the kernel, will believe that RDRAND is
not supported.
Update the CPU initialization to clear the RDRAND CPUID bit for any family
15h and 16h processor that supports RDRAND. If it is known that the family
15h or family 16h system does not have an RDRAND resume issue or that the
system will not be placed in suspend, the "rdrand=force" kernel parameter
can be used to stop the clearing of the RDRAND CPUID bit.
Additionally, update the suspend and resume path to save and restore the
MSR C001_1004 value to ensure that the RDRAND CPUID setting remains in
place after resuming from suspend.
Note, that clearing the RDRAND CPUID bit does not prevent a processor
that normally supports the RDRAND instruction from executing it. So any
code that determined the support based on family and model won't #UD.
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Chen Yu <yu.c.chen@intel.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: "linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>
Cc: "linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>
Cc: Nathan Chancellor <natechancellor@gmail.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: <stable@vger.kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: "x86@kernel.org" <x86@kernel.org>
Link: https://lkml.kernel.org/r/7543af91666f491547bd86cebb1e17c66824ab9f.1566229943.git.thomas.lendacky@amd.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
Documentation/admin-guide/kernel-parameters.txt | 7 +
arch/x86/include/asm/msr-index.h | 1
arch/x86/kernel/cpu/amd.c | 66 ++++++++++++++++++
arch/x86/power/cpu.c | 86 ++++++++++++++++++++----
4 files changed, 147 insertions(+), 13 deletions(-)
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -4055,6 +4055,13 @@
Run specified binary instead of /init from the ramdisk,
used for early userspace startup. See initrd.
+ rdrand= [X86]
+ force - Override the decision by the kernel to hide the
+ advertisement of RDRAND support (this affects
+ certain AMD processors because of buggy BIOS
+ support, specifically around the suspend/resume
+ path).
+
rdt= [HW,X86,RDT]
Turn on/off individual RDT features. List is:
cmt, mbmtotal, mbmlocal, l3cat, l3cdp, l2cat, l2cdp,
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -372,6 +372,7 @@
#define MSR_AMD64_PATCH_LEVEL 0x0000008b
#define MSR_AMD64_TSC_RATIO 0xc0000104
#define MSR_AMD64_NB_CFG 0xc001001f
+#define MSR_AMD64_CPUID_FN_1 0xc0011004
#define MSR_AMD64_PATCH_LOADER 0xc0010020
#define MSR_AMD64_OSVW_ID_LENGTH 0xc0010140
#define MSR_AMD64_OSVW_STATUS 0xc0010141
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -804,6 +804,64 @@ static void init_amd_ln(struct cpuinfo_x
msr_set_bit(MSR_AMD64_DE_CFG, 31);
}
+static bool rdrand_force;
+
+static int __init rdrand_cmdline(char *str)
+{
+ if (!str)
+ return -EINVAL;
+
+ if (!strcmp(str, "force"))
+ rdrand_force = true;
+ else
+ return -EINVAL;
+
+ return 0;
+}
+early_param("rdrand", rdrand_cmdline);
+
+static void clear_rdrand_cpuid_bit(struct cpuinfo_x86 *c)
+{
+ /*
+ * Saving of the MSR used to hide the RDRAND support during
+ * suspend/resume is done by arch/x86/power/cpu.c, which is
+ * dependent on CONFIG_PM_SLEEP.
+ */
+ if (!IS_ENABLED(CONFIG_PM_SLEEP))
+ return;
+
+ /*
+ * The nordrand option can clear X86_FEATURE_RDRAND, so check for
+ * RDRAND support using the CPUID function directly.
+ */
+ if (!(cpuid_ecx(1) & BIT(30)) || rdrand_force)
+ return;
+
+ msr_clear_bit(MSR_AMD64_CPUID_FN_1, 62);
+
+ /*
+ * Verify that the CPUID change has occurred in case the kernel is
+ * running virtualized and the hypervisor doesn't support the MSR.
+ */
+ if (cpuid_ecx(1) & BIT(30)) {
+ pr_info_once("BIOS may not properly restore RDRAND after suspend, but hypervisor does not support hiding RDRAND via CPUID.\n");
+ return;
+ }
+
+ clear_cpu_cap(c, X86_FEATURE_RDRAND);
+ pr_info_once("BIOS may not properly restore RDRAND after suspend, hiding RDRAND via CPUID. Use rdrand=force to reenable.\n");
+}
+
+static void init_amd_jg(struct cpuinfo_x86 *c)
+{
+ /*
+ * Some BIOS implementations do not restore proper RDRAND support
+ * across suspend and resume. Check on whether to hide the RDRAND
+ * instruction support via CPUID.
+ */
+ clear_rdrand_cpuid_bit(c);
+}
+
static void init_amd_bd(struct cpuinfo_x86 *c)
{
u64 value;
@@ -818,6 +876,13 @@ static void init_amd_bd(struct cpuinfo_x
wrmsrl_safe(MSR_F15H_IC_CFG, value);
}
}
+
+ /*
+ * Some BIOS implementations do not restore proper RDRAND support
+ * across suspend and resume. Check on whether to hide the RDRAND
+ * instruction support via CPUID.
+ */
+ clear_rdrand_cpuid_bit(c);
}
static void init_amd_zn(struct cpuinfo_x86 *c)
@@ -860,6 +925,7 @@ static void init_amd(struct cpuinfo_x86
case 0x10: init_amd_gh(c); break;
case 0x12: init_amd_ln(c); break;
case 0x15: init_amd_bd(c); break;
+ case 0x16: init_amd_jg(c); break;
case 0x17: init_amd_zn(c); break;
}
--- a/arch/x86/power/cpu.c
+++ b/arch/x86/power/cpu.c
@@ -12,6 +12,7 @@
#include <linux/smp.h>
#include <linux/perf_event.h>
#include <linux/tboot.h>
+#include <linux/dmi.h>
#include <asm/pgtable.h>
#include <asm/proto.h>
@@ -23,7 +24,7 @@
#include <asm/debugreg.h>
#include <asm/cpu.h>
#include <asm/mmu_context.h>
-#include <linux/dmi.h>
+#include <asm/cpu_device_id.h>
#ifdef CONFIG_X86_32
__visible unsigned long saved_context_ebx;
@@ -397,15 +398,14 @@ static int __init bsp_pm_check_init(void
core_initcall(bsp_pm_check_init);
-static int msr_init_context(const u32 *msr_id, const int total_num)
+static int msr_build_context(const u32 *msr_id, const int num)
{
- int i = 0;
+ struct saved_msrs *saved_msrs = &saved_context.saved_msrs;
struct saved_msr *msr_array;
+ int total_num;
+ int i, j;
- if (saved_context.saved_msrs.array || saved_context.saved_msrs.num > 0) {
- pr_err("x86/pm: MSR quirk already applied, please check your DMI match table.\n");
- return -EINVAL;
- }
+ total_num = saved_msrs->num + num;
msr_array = kmalloc_array(total_num, sizeof(struct saved_msr), GFP_KERNEL);
if (!msr_array) {
@@ -413,19 +413,30 @@ static int msr_init_context(const u32 *m
return -ENOMEM;
}
- for (i = 0; i < total_num; i++) {
- msr_array[i].info.msr_no = msr_id[i];
+ if (saved_msrs->array) {
+ /*
+ * Multiple callbacks can invoke this function, so copy any
+ * MSR save requests from previous invocations.
+ */
+ memcpy(msr_array, saved_msrs->array,
+ sizeof(struct saved_msr) * saved_msrs->num);
+
+ kfree(saved_msrs->array);
+ }
+
+ for (i = saved_msrs->num, j = 0; i < total_num; i++, j++) {
+ msr_array[i].info.msr_no = msr_id[j];
msr_array[i].valid = false;
msr_array[i].info.reg.q = 0;
}
- saved_context.saved_msrs.num = total_num;
- saved_context.saved_msrs.array = msr_array;
+ saved_msrs->num = total_num;
+ saved_msrs->array = msr_array;
return 0;
}
/*
- * The following section is a quirk framework for problematic BIOSen:
+ * The following sections are a quirk framework for problematic BIOSen:
* Sometimes MSRs are modified by the BIOSen after suspended to
* RAM, this might cause unexpected behavior after wakeup.
* Thus we save/restore these specified MSRs across suspend/resume
@@ -440,7 +451,7 @@ static int msr_initialize_bdw(const stru
u32 bdw_msr_id[] = { MSR_IA32_THERM_CONTROL };
pr_info("x86/pm: %s detected, MSR saving is needed during suspending.\n", d->ident);
- return msr_init_context(bdw_msr_id, ARRAY_SIZE(bdw_msr_id));
+ return msr_build_context(bdw_msr_id, ARRAY_SIZE(bdw_msr_id));
}
static const struct dmi_system_id msr_save_dmi_table[] = {
@@ -455,9 +466,58 @@ static const struct dmi_system_id msr_sa
{}
};
+static int msr_save_cpuid_features(const struct x86_cpu_id *c)
+{
+ u32 cpuid_msr_id[] = {
+ MSR_AMD64_CPUID_FN_1,
+ };
+
+ pr_info("x86/pm: family %#hx cpu detected, MSR saving is needed during suspending.\n",
+ c->family);
+
+ return msr_build_context(cpuid_msr_id, ARRAY_SIZE(cpuid_msr_id));
+}
+
+static const struct x86_cpu_id msr_save_cpu_table[] = {
+ {
+ .vendor = X86_VENDOR_AMD,
+ .family = 0x15,
+ .model = X86_MODEL_ANY,
+ .feature = X86_FEATURE_ANY,
+ .driver_data = (kernel_ulong_t)msr_save_cpuid_features,
+ },
+ {
+ .vendor = X86_VENDOR_AMD,
+ .family = 0x16,
+ .model = X86_MODEL_ANY,
+ .feature = X86_FEATURE_ANY,
+ .driver_data = (kernel_ulong_t)msr_save_cpuid_features,
+ },
+ {}
+};
+
+typedef int (*pm_cpu_match_t)(const struct x86_cpu_id *);
+static int pm_cpu_check(const struct x86_cpu_id *c)
+{
+ const struct x86_cpu_id *m;
+ int ret = 0;
+
+ m = x86_match_cpu(msr_save_cpu_table);
+ if (m) {
+ pm_cpu_match_t fn;
+
+ fn = (pm_cpu_match_t)m->driver_data;
+ ret = fn(m);
+ }
+
+ return ret;
+}
+
static int pm_check_save_msr(void)
{
dmi_check_system(msr_save_dmi_table);
+ pm_cpu_check(msr_save_cpu_table);
+
return 0;
}
^ permalink raw reply
* [PATCH 4.19 72/98] x86/CPU/AMD: Clear RDRAND CPUID bit on AMD family 15h/16h
From: Greg Kroah-Hartman @ 2019-08-27 7:50 UTC (permalink / raw)
To: linux-kernel
Cc: Greg Kroah-Hartman, stable, Tom Lendacky, Borislav Petkov,
Andrew Cooper, Andrew Morton, Chen Yu, H. Peter Anvin,
Ingo Molnar, Jonathan Corbet, Josh Poimboeuf, Juergen Gross,
Kees Cook, linux-doc@vger.kernel.org, linux-pm@vger.kernel.org,
Nathan Chancellor, Paolo Bonzini, Pavel Machek, Rafael J. Wysocki,
Thomas Gleixner, x86@kernel.org
In-Reply-To: <20190827072718.142728620@linuxfoundation.org>
From: Tom Lendacky <thomas.lendacky@amd.com>
commit c49a0a80137c7ca7d6ced4c812c9e07a949f6f24 upstream.
There have been reports of RDRAND issues after resuming from suspend on
some AMD family 15h and family 16h systems. This issue stems from a BIOS
not performing the proper steps during resume to ensure RDRAND continues
to function properly.
RDRAND support is indicated by CPUID Fn00000001_ECX[30]. This bit can be
reset by clearing MSR C001_1004[62]. Any software that checks for RDRAND
support using CPUID, including the kernel, will believe that RDRAND is
not supported.
Update the CPU initialization to clear the RDRAND CPUID bit for any family
15h and 16h processor that supports RDRAND. If it is known that the family
15h or family 16h system does not have an RDRAND resume issue or that the
system will not be placed in suspend, the "rdrand=force" kernel parameter
can be used to stop the clearing of the RDRAND CPUID bit.
Additionally, update the suspend and resume path to save and restore the
MSR C001_1004 value to ensure that the RDRAND CPUID setting remains in
place after resuming from suspend.
Note, that clearing the RDRAND CPUID bit does not prevent a processor
that normally supports the RDRAND instruction from executing it. So any
code that determined the support based on family and model won't #UD.
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Chen Yu <yu.c.chen@intel.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: "linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>
Cc: "linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>
Cc: Nathan Chancellor <natechancellor@gmail.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: <stable@vger.kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: "x86@kernel.org" <x86@kernel.org>
Link: https://lkml.kernel.org/r/7543af91666f491547bd86cebb1e17c66824ab9f.1566229943.git.thomas.lendacky@amd.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
Documentation/admin-guide/kernel-parameters.txt | 7 +
arch/x86/include/asm/msr-index.h | 1
arch/x86/kernel/cpu/amd.c | 66 ++++++++++++++++++
arch/x86/power/cpu.c | 86 ++++++++++++++++++++----
4 files changed, 147 insertions(+), 13 deletions(-)
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3948,6 +3948,13 @@
Run specified binary instead of /init from the ramdisk,
used for early userspace startup. See initrd.
+ rdrand= [X86]
+ force - Override the decision by the kernel to hide the
+ advertisement of RDRAND support (this affects
+ certain AMD processors because of buggy BIOS
+ support, specifically around the suspend/resume
+ path).
+
rdt= [HW,X86,RDT]
Turn on/off individual RDT features. List is:
cmt, mbmtotal, mbmlocal, l3cat, l3cdp, l2cat, l2cdp,
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -334,6 +334,7 @@
#define MSR_AMD64_PATCH_LEVEL 0x0000008b
#define MSR_AMD64_TSC_RATIO 0xc0000104
#define MSR_AMD64_NB_CFG 0xc001001f
+#define MSR_AMD64_CPUID_FN_1 0xc0011004
#define MSR_AMD64_PATCH_LOADER 0xc0010020
#define MSR_AMD64_OSVW_ID_LENGTH 0xc0010140
#define MSR_AMD64_OSVW_STATUS 0xc0010141
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -799,6 +799,64 @@ static void init_amd_ln(struct cpuinfo_x
msr_set_bit(MSR_AMD64_DE_CFG, 31);
}
+static bool rdrand_force;
+
+static int __init rdrand_cmdline(char *str)
+{
+ if (!str)
+ return -EINVAL;
+
+ if (!strcmp(str, "force"))
+ rdrand_force = true;
+ else
+ return -EINVAL;
+
+ return 0;
+}
+early_param("rdrand", rdrand_cmdline);
+
+static void clear_rdrand_cpuid_bit(struct cpuinfo_x86 *c)
+{
+ /*
+ * Saving of the MSR used to hide the RDRAND support during
+ * suspend/resume is done by arch/x86/power/cpu.c, which is
+ * dependent on CONFIG_PM_SLEEP.
+ */
+ if (!IS_ENABLED(CONFIG_PM_SLEEP))
+ return;
+
+ /*
+ * The nordrand option can clear X86_FEATURE_RDRAND, so check for
+ * RDRAND support using the CPUID function directly.
+ */
+ if (!(cpuid_ecx(1) & BIT(30)) || rdrand_force)
+ return;
+
+ msr_clear_bit(MSR_AMD64_CPUID_FN_1, 62);
+
+ /*
+ * Verify that the CPUID change has occurred in case the kernel is
+ * running virtualized and the hypervisor doesn't support the MSR.
+ */
+ if (cpuid_ecx(1) & BIT(30)) {
+ pr_info_once("BIOS may not properly restore RDRAND after suspend, but hypervisor does not support hiding RDRAND via CPUID.\n");
+ return;
+ }
+
+ clear_cpu_cap(c, X86_FEATURE_RDRAND);
+ pr_info_once("BIOS may not properly restore RDRAND after suspend, hiding RDRAND via CPUID. Use rdrand=force to reenable.\n");
+}
+
+static void init_amd_jg(struct cpuinfo_x86 *c)
+{
+ /*
+ * Some BIOS implementations do not restore proper RDRAND support
+ * across suspend and resume. Check on whether to hide the RDRAND
+ * instruction support via CPUID.
+ */
+ clear_rdrand_cpuid_bit(c);
+}
+
static void init_amd_bd(struct cpuinfo_x86 *c)
{
u64 value;
@@ -813,6 +871,13 @@ static void init_amd_bd(struct cpuinfo_x
wrmsrl_safe(MSR_F15H_IC_CFG, value);
}
}
+
+ /*
+ * Some BIOS implementations do not restore proper RDRAND support
+ * across suspend and resume. Check on whether to hide the RDRAND
+ * instruction support via CPUID.
+ */
+ clear_rdrand_cpuid_bit(c);
}
static void init_amd_zn(struct cpuinfo_x86 *c)
@@ -855,6 +920,7 @@ static void init_amd(struct cpuinfo_x86
case 0x10: init_amd_gh(c); break;
case 0x12: init_amd_ln(c); break;
case 0x15: init_amd_bd(c); break;
+ case 0x16: init_amd_jg(c); break;
case 0x17: init_amd_zn(c); break;
}
--- a/arch/x86/power/cpu.c
+++ b/arch/x86/power/cpu.c
@@ -13,6 +13,7 @@
#include <linux/smp.h>
#include <linux/perf_event.h>
#include <linux/tboot.h>
+#include <linux/dmi.h>
#include <asm/pgtable.h>
#include <asm/proto.h>
@@ -24,7 +25,7 @@
#include <asm/debugreg.h>
#include <asm/cpu.h>
#include <asm/mmu_context.h>
-#include <linux/dmi.h>
+#include <asm/cpu_device_id.h>
#ifdef CONFIG_X86_32
__visible unsigned long saved_context_ebx;
@@ -398,15 +399,14 @@ static int __init bsp_pm_check_init(void
core_initcall(bsp_pm_check_init);
-static int msr_init_context(const u32 *msr_id, const int total_num)
+static int msr_build_context(const u32 *msr_id, const int num)
{
- int i = 0;
+ struct saved_msrs *saved_msrs = &saved_context.saved_msrs;
struct saved_msr *msr_array;
+ int total_num;
+ int i, j;
- if (saved_context.saved_msrs.array || saved_context.saved_msrs.num > 0) {
- pr_err("x86/pm: MSR quirk already applied, please check your DMI match table.\n");
- return -EINVAL;
- }
+ total_num = saved_msrs->num + num;
msr_array = kmalloc_array(total_num, sizeof(struct saved_msr), GFP_KERNEL);
if (!msr_array) {
@@ -414,19 +414,30 @@ static int msr_init_context(const u32 *m
return -ENOMEM;
}
- for (i = 0; i < total_num; i++) {
- msr_array[i].info.msr_no = msr_id[i];
+ if (saved_msrs->array) {
+ /*
+ * Multiple callbacks can invoke this function, so copy any
+ * MSR save requests from previous invocations.
+ */
+ memcpy(msr_array, saved_msrs->array,
+ sizeof(struct saved_msr) * saved_msrs->num);
+
+ kfree(saved_msrs->array);
+ }
+
+ for (i = saved_msrs->num, j = 0; i < total_num; i++, j++) {
+ msr_array[i].info.msr_no = msr_id[j];
msr_array[i].valid = false;
msr_array[i].info.reg.q = 0;
}
- saved_context.saved_msrs.num = total_num;
- saved_context.saved_msrs.array = msr_array;
+ saved_msrs->num = total_num;
+ saved_msrs->array = msr_array;
return 0;
}
/*
- * The following section is a quirk framework for problematic BIOSen:
+ * The following sections are a quirk framework for problematic BIOSen:
* Sometimes MSRs are modified by the BIOSen after suspended to
* RAM, this might cause unexpected behavior after wakeup.
* Thus we save/restore these specified MSRs across suspend/resume
@@ -441,7 +452,7 @@ static int msr_initialize_bdw(const stru
u32 bdw_msr_id[] = { MSR_IA32_THERM_CONTROL };
pr_info("x86/pm: %s detected, MSR saving is needed during suspending.\n", d->ident);
- return msr_init_context(bdw_msr_id, ARRAY_SIZE(bdw_msr_id));
+ return msr_build_context(bdw_msr_id, ARRAY_SIZE(bdw_msr_id));
}
static const struct dmi_system_id msr_save_dmi_table[] = {
@@ -456,9 +467,58 @@ static const struct dmi_system_id msr_sa
{}
};
+static int msr_save_cpuid_features(const struct x86_cpu_id *c)
+{
+ u32 cpuid_msr_id[] = {
+ MSR_AMD64_CPUID_FN_1,
+ };
+
+ pr_info("x86/pm: family %#hx cpu detected, MSR saving is needed during suspending.\n",
+ c->family);
+
+ return msr_build_context(cpuid_msr_id, ARRAY_SIZE(cpuid_msr_id));
+}
+
+static const struct x86_cpu_id msr_save_cpu_table[] = {
+ {
+ .vendor = X86_VENDOR_AMD,
+ .family = 0x15,
+ .model = X86_MODEL_ANY,
+ .feature = X86_FEATURE_ANY,
+ .driver_data = (kernel_ulong_t)msr_save_cpuid_features,
+ },
+ {
+ .vendor = X86_VENDOR_AMD,
+ .family = 0x16,
+ .model = X86_MODEL_ANY,
+ .feature = X86_FEATURE_ANY,
+ .driver_data = (kernel_ulong_t)msr_save_cpuid_features,
+ },
+ {}
+};
+
+typedef int (*pm_cpu_match_t)(const struct x86_cpu_id *);
+static int pm_cpu_check(const struct x86_cpu_id *c)
+{
+ const struct x86_cpu_id *m;
+ int ret = 0;
+
+ m = x86_match_cpu(msr_save_cpu_table);
+ if (m) {
+ pm_cpu_match_t fn;
+
+ fn = (pm_cpu_match_t)m->driver_data;
+ ret = fn(m);
+ }
+
+ return ret;
+}
+
static int pm_check_save_msr(void)
{
dmi_check_system(msr_save_dmi_table);
+ pm_cpu_check(msr_save_cpu_table);
+
return 0;
}
^ permalink raw reply
* [PATCH 4.14 46/62] x86/CPU/AMD: Clear RDRAND CPUID bit on AMD family 15h/16h
From: Greg Kroah-Hartman @ 2019-08-27 7:50 UTC (permalink / raw)
To: linux-kernel
Cc: Greg Kroah-Hartman, stable, Tom Lendacky, Borislav Petkov,
Andrew Cooper, Andrew Morton, Chen Yu, H. Peter Anvin,
Ingo Molnar, Jonathan Corbet, Josh Poimboeuf, Juergen Gross,
Kees Cook, linux-doc@vger.kernel.org, linux-pm@vger.kernel.org,
Nathan Chancellor, Paolo Bonzini, Pavel Machek, Rafael J. Wysocki,
Thomas Gleixner, x86@kernel.org
In-Reply-To: <20190827072659.803647352@linuxfoundation.org>
From: Tom Lendacky <thomas.lendacky@amd.com>
commit c49a0a80137c7ca7d6ced4c812c9e07a949f6f24 upstream.
There have been reports of RDRAND issues after resuming from suspend on
some AMD family 15h and family 16h systems. This issue stems from a BIOS
not performing the proper steps during resume to ensure RDRAND continues
to function properly.
RDRAND support is indicated by CPUID Fn00000001_ECX[30]. This bit can be
reset by clearing MSR C001_1004[62]. Any software that checks for RDRAND
support using CPUID, including the kernel, will believe that RDRAND is
not supported.
Update the CPU initialization to clear the RDRAND CPUID bit for any family
15h and 16h processor that supports RDRAND. If it is known that the family
15h or family 16h system does not have an RDRAND resume issue or that the
system will not be placed in suspend, the "rdrand=force" kernel parameter
can be used to stop the clearing of the RDRAND CPUID bit.
Additionally, update the suspend and resume path to save and restore the
MSR C001_1004 value to ensure that the RDRAND CPUID setting remains in
place after resuming from suspend.
Note, that clearing the RDRAND CPUID bit does not prevent a processor
that normally supports the RDRAND instruction from executing it. So any
code that determined the support based on family and model won't #UD.
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Chen Yu <yu.c.chen@intel.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: "linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>
Cc: "linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>
Cc: Nathan Chancellor <natechancellor@gmail.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: <stable@vger.kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: "x86@kernel.org" <x86@kernel.org>
Link: https://lkml.kernel.org/r/7543af91666f491547bd86cebb1e17c66824ab9f.1566229943.git.thomas.lendacky@amd.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
Documentation/admin-guide/kernel-parameters.txt | 7 +
arch/x86/include/asm/msr-index.h | 1
arch/x86/kernel/cpu/amd.c | 66 ++++++++++++++++++
arch/x86/power/cpu.c | 86 ++++++++++++++++++++----
4 files changed, 147 insertions(+), 13 deletions(-)
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3788,6 +3788,13 @@
Run specified binary instead of /init from the ramdisk,
used for early userspace startup. See initrd.
+ rdrand= [X86]
+ force - Override the decision by the kernel to hide the
+ advertisement of RDRAND support (this affects
+ certain AMD processors because of buggy BIOS
+ support, specifically around the suspend/resume
+ path).
+
rdt= [HW,X86,RDT]
Turn on/off individual RDT features. List is:
cmt, mbmtotal, mbmlocal, l3cat, l3cdp, l2cat, mba.
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -334,6 +334,7 @@
#define MSR_AMD64_PATCH_LEVEL 0x0000008b
#define MSR_AMD64_TSC_RATIO 0xc0000104
#define MSR_AMD64_NB_CFG 0xc001001f
+#define MSR_AMD64_CPUID_FN_1 0xc0011004
#define MSR_AMD64_PATCH_LOADER 0xc0010020
#define MSR_AMD64_OSVW_ID_LENGTH 0xc0010140
#define MSR_AMD64_OSVW_STATUS 0xc0010141
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -772,6 +772,64 @@ static void init_amd_ln(struct cpuinfo_x
msr_set_bit(MSR_AMD64_DE_CFG, 31);
}
+static bool rdrand_force;
+
+static int __init rdrand_cmdline(char *str)
+{
+ if (!str)
+ return -EINVAL;
+
+ if (!strcmp(str, "force"))
+ rdrand_force = true;
+ else
+ return -EINVAL;
+
+ return 0;
+}
+early_param("rdrand", rdrand_cmdline);
+
+static void clear_rdrand_cpuid_bit(struct cpuinfo_x86 *c)
+{
+ /*
+ * Saving of the MSR used to hide the RDRAND support during
+ * suspend/resume is done by arch/x86/power/cpu.c, which is
+ * dependent on CONFIG_PM_SLEEP.
+ */
+ if (!IS_ENABLED(CONFIG_PM_SLEEP))
+ return;
+
+ /*
+ * The nordrand option can clear X86_FEATURE_RDRAND, so check for
+ * RDRAND support using the CPUID function directly.
+ */
+ if (!(cpuid_ecx(1) & BIT(30)) || rdrand_force)
+ return;
+
+ msr_clear_bit(MSR_AMD64_CPUID_FN_1, 62);
+
+ /*
+ * Verify that the CPUID change has occurred in case the kernel is
+ * running virtualized and the hypervisor doesn't support the MSR.
+ */
+ if (cpuid_ecx(1) & BIT(30)) {
+ pr_info_once("BIOS may not properly restore RDRAND after suspend, but hypervisor does not support hiding RDRAND via CPUID.\n");
+ return;
+ }
+
+ clear_cpu_cap(c, X86_FEATURE_RDRAND);
+ pr_info_once("BIOS may not properly restore RDRAND after suspend, hiding RDRAND via CPUID. Use rdrand=force to reenable.\n");
+}
+
+static void init_amd_jg(struct cpuinfo_x86 *c)
+{
+ /*
+ * Some BIOS implementations do not restore proper RDRAND support
+ * across suspend and resume. Check on whether to hide the RDRAND
+ * instruction support via CPUID.
+ */
+ clear_rdrand_cpuid_bit(c);
+}
+
static void init_amd_bd(struct cpuinfo_x86 *c)
{
u64 value;
@@ -786,6 +844,13 @@ static void init_amd_bd(struct cpuinfo_x
wrmsrl_safe(MSR_F15H_IC_CFG, value);
}
}
+
+ /*
+ * Some BIOS implementations do not restore proper RDRAND support
+ * across suspend and resume. Check on whether to hide the RDRAND
+ * instruction support via CPUID.
+ */
+ clear_rdrand_cpuid_bit(c);
}
static void init_amd_zn(struct cpuinfo_x86 *c)
@@ -828,6 +893,7 @@ static void init_amd(struct cpuinfo_x86
case 0x10: init_amd_gh(c); break;
case 0x12: init_amd_ln(c); break;
case 0x15: init_amd_bd(c); break;
+ case 0x16: init_amd_jg(c); break;
case 0x17: init_amd_zn(c); break;
}
--- a/arch/x86/power/cpu.c
+++ b/arch/x86/power/cpu.c
@@ -13,6 +13,7 @@
#include <linux/smp.h>
#include <linux/perf_event.h>
#include <linux/tboot.h>
+#include <linux/dmi.h>
#include <asm/pgtable.h>
#include <asm/proto.h>
@@ -24,7 +25,7 @@
#include <asm/debugreg.h>
#include <asm/cpu.h>
#include <asm/mmu_context.h>
-#include <linux/dmi.h>
+#include <asm/cpu_device_id.h>
#ifdef CONFIG_X86_32
__visible unsigned long saved_context_ebx;
@@ -398,15 +399,14 @@ static int __init bsp_pm_check_init(void
core_initcall(bsp_pm_check_init);
-static int msr_init_context(const u32 *msr_id, const int total_num)
+static int msr_build_context(const u32 *msr_id, const int num)
{
- int i = 0;
+ struct saved_msrs *saved_msrs = &saved_context.saved_msrs;
struct saved_msr *msr_array;
+ int total_num;
+ int i, j;
- if (saved_context.saved_msrs.array || saved_context.saved_msrs.num > 0) {
- pr_err("x86/pm: MSR quirk already applied, please check your DMI match table.\n");
- return -EINVAL;
- }
+ total_num = saved_msrs->num + num;
msr_array = kmalloc_array(total_num, sizeof(struct saved_msr), GFP_KERNEL);
if (!msr_array) {
@@ -414,19 +414,30 @@ static int msr_init_context(const u32 *m
return -ENOMEM;
}
- for (i = 0; i < total_num; i++) {
- msr_array[i].info.msr_no = msr_id[i];
+ if (saved_msrs->array) {
+ /*
+ * Multiple callbacks can invoke this function, so copy any
+ * MSR save requests from previous invocations.
+ */
+ memcpy(msr_array, saved_msrs->array,
+ sizeof(struct saved_msr) * saved_msrs->num);
+
+ kfree(saved_msrs->array);
+ }
+
+ for (i = saved_msrs->num, j = 0; i < total_num; i++, j++) {
+ msr_array[i].info.msr_no = msr_id[j];
msr_array[i].valid = false;
msr_array[i].info.reg.q = 0;
}
- saved_context.saved_msrs.num = total_num;
- saved_context.saved_msrs.array = msr_array;
+ saved_msrs->num = total_num;
+ saved_msrs->array = msr_array;
return 0;
}
/*
- * The following section is a quirk framework for problematic BIOSen:
+ * The following sections are a quirk framework for problematic BIOSen:
* Sometimes MSRs are modified by the BIOSen after suspended to
* RAM, this might cause unexpected behavior after wakeup.
* Thus we save/restore these specified MSRs across suspend/resume
@@ -441,7 +452,7 @@ static int msr_initialize_bdw(const stru
u32 bdw_msr_id[] = { MSR_IA32_THERM_CONTROL };
pr_info("x86/pm: %s detected, MSR saving is needed during suspending.\n", d->ident);
- return msr_init_context(bdw_msr_id, ARRAY_SIZE(bdw_msr_id));
+ return msr_build_context(bdw_msr_id, ARRAY_SIZE(bdw_msr_id));
}
static const struct dmi_system_id msr_save_dmi_table[] = {
@@ -456,9 +467,58 @@ static const struct dmi_system_id msr_sa
{}
};
+static int msr_save_cpuid_features(const struct x86_cpu_id *c)
+{
+ u32 cpuid_msr_id[] = {
+ MSR_AMD64_CPUID_FN_1,
+ };
+
+ pr_info("x86/pm: family %#hx cpu detected, MSR saving is needed during suspending.\n",
+ c->family);
+
+ return msr_build_context(cpuid_msr_id, ARRAY_SIZE(cpuid_msr_id));
+}
+
+static const struct x86_cpu_id msr_save_cpu_table[] = {
+ {
+ .vendor = X86_VENDOR_AMD,
+ .family = 0x15,
+ .model = X86_MODEL_ANY,
+ .feature = X86_FEATURE_ANY,
+ .driver_data = (kernel_ulong_t)msr_save_cpuid_features,
+ },
+ {
+ .vendor = X86_VENDOR_AMD,
+ .family = 0x16,
+ .model = X86_MODEL_ANY,
+ .feature = X86_FEATURE_ANY,
+ .driver_data = (kernel_ulong_t)msr_save_cpuid_features,
+ },
+ {}
+};
+
+typedef int (*pm_cpu_match_t)(const struct x86_cpu_id *);
+static int pm_cpu_check(const struct x86_cpu_id *c)
+{
+ const struct x86_cpu_id *m;
+ int ret = 0;
+
+ m = x86_match_cpu(msr_save_cpu_table);
+ if (m) {
+ pm_cpu_match_t fn;
+
+ fn = (pm_cpu_match_t)m->driver_data;
+ ret = fn(m);
+ }
+
+ return ret;
+}
+
static int pm_check_save_msr(void)
{
dmi_check_system(msr_save_dmi_table);
+ pm_cpu_check(msr_save_cpu_table);
+
return 0;
}
^ permalink raw reply
* Re: [PATCH v2 2/3] fbdev: fbmem: allow overriding the number of bootup logos
From: Geert Uytterhoeven @ 2019-08-27 8:36 UTC (permalink / raw)
To: Peter Rosin
Cc: linux-kernel@vger.kernel.org, Bartlomiej Zolnierkiewicz,
Jonathan Corbet, dri-devel@lists.freedesktop.org,
linux-fbdev@vger.kernel.org, linux-doc@vger.kernel.org,
Matthew Wilcox
In-Reply-To: <20190826195740.29415-3-peda@axentia.se>
Hi Peter,
On Mon, Aug 26, 2019 at 10:46 PM Peter Rosin <peda@axentia.se> wrote:
> Probably most useful if you only want one logo regardless of how many
> CPU cores you have.
>
> Signed-off-by: Peter Rosin <peda@axentia.se>
Thanks for your patch!
> --- a/Documentation/fb/fbcon.rst
> +++ b/Documentation/fb/fbcon.rst
> @@ -174,6 +174,11 @@ C. Boot options
> displayed due to multiple CPUs, the collected line of logos is moved
> as a whole.
>
> +9. fbcon=logo-count:<n>
> +
> + The value 'n' overrides the number of bootup logos. Zero gives the
> + default, which is the number of online cpus.
Isn't that a bit unexpected for the user?
What about making -1 the default (auto), and zero meaning no logos?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH v3 01/10] KVM: arm64: Document PV-time interface
From: Christoffer Dall @ 2019-08-27 8:44 UTC (permalink / raw)
To: Steven Price
Cc: Marc Zyngier, Will Deacon, linux-arm-kernel, kvmarm, linux-kernel,
kvm, Catalin Marinas, linux-doc, Russell King, Paolo Bonzini
In-Reply-To: <20190821153656.33429-2-steven.price@arm.com>
On Wed, Aug 21, 2019 at 04:36:47PM +0100, Steven Price wrote:
> Introduce a paravirtualization interface for KVM/arm64 based on the
> "Arm Paravirtualized Time for Arm-Base Systems" specification DEN 0057A.
>
> This only adds the details about "Stolen Time" as the details of "Live
> Physical Time" have not been fully agreed.
>
> User space can specify a reserved area of memory for the guest and
> inform KVM to populate the memory with information on time that the host
> kernel has stolen from the guest.
>
> A hypercall interface is provided for the guest to interrogate the
> hypervisor's support for this interface and the location of the shared
> memory structures.
>
> Signed-off-by: Steven Price <steven.price@arm.com>
> ---
> Documentation/virt/kvm/arm/pvtime.txt | 100 ++++++++++++++++++++++++++
> 1 file changed, 100 insertions(+)
> create mode 100644 Documentation/virt/kvm/arm/pvtime.txt
>
> diff --git a/Documentation/virt/kvm/arm/pvtime.txt b/Documentation/virt/kvm/arm/pvtime.txt
> new file mode 100644
> index 000000000000..1ceb118694e7
> --- /dev/null
> +++ b/Documentation/virt/kvm/arm/pvtime.txt
> @@ -0,0 +1,100 @@
> +Paravirtualized time support for arm64
> +======================================
> +
> +Arm specification DEN0057/A defined a standard for paravirtualised time
> +support for AArch64 guests:
> +
> +https://developer.arm.com/docs/den0057/a
> +
> +KVM/arm64 implements the stolen time part of this specification by providing
> +some hypervisor service calls to support a paravirtualized guest obtaining a
> +view of the amount of time stolen from its execution.
> +
> +Two new SMCCC compatible hypercalls are defined:
> +
> +PV_FEATURES 0xC5000020
> +PV_TIME_ST 0xC5000022
> +
> +These are only available in the SMC64/HVC64 calling convention as
> +paravirtualized time is not available to 32 bit Arm guests. The existence of
> +the PV_FEATURES hypercall should be probed using the SMCCC 1.1 ARCH_FEATURES
> +mechanism before calling it.
> +
> +PV_FEATURES
> + Function ID: (uint32) : 0xC5000020
> + PV_func_id: (uint32) : Either PV_TIME_LPT or PV_TIME_ST
> + Return value: (int32) : NOT_SUPPORTED (-1) or SUCCESS (0) if the relevant
> + PV-time feature is supported by the hypervisor.
> +
> +PV_TIME_ST
> + Function ID: (uint32) : 0xC5000022
> + Return value: (int64) : IPA of the stolen time data structure for this
> + (V)CPU. On failure:
> + NOT_SUPPORTED (-1)
> +
> +The IPA returned by PV_TIME_ST should be mapped by the guest as normal memory
> +with inner and outer write back caching attributes, in the inner shareable
> +domain. A total of 16 bytes from the IPA returned are guaranteed to be
> +meaningfully filled by the hypervisor (see structure below).
> +
> +PV_TIME_ST returns the structure for the calling VCPU.
> +
> +Stolen Time
> +-----------
> +
> +The structure pointed to by the PV_TIME_ST hypercall is as follows:
> +
> + Field | Byte Length | Byte Offset | Description
> + ----------- | ----------- | ----------- | --------------------------
> + Revision | 4 | 0 | Must be 0 for version 0.1
> + Attributes | 4 | 4 | Must be 0
> + Stolen time | 8 | 8 | Stolen time in unsigned
> + | | | nanoseconds indicating how
> + | | | much time this VCPU thread
> + | | | was involuntarily not
> + | | | running on a physical CPU.
> +
> +The structure will be updated by the hypervisor prior to scheduling a VCPU. It
> +will be present within a reserved region of the normal memory given to the
> +guest. The guest should not attempt to write into this memory. There is a
> +structure per VCPU of the guest.
> +
> +User space interface
> +====================
> +
> +User space can request that KVM provide the paravirtualized time interface to
> +a guest by creating a KVM_DEV_TYPE_ARM_PV_TIME device, for example:
> +
I feel it would be more consistent to have the details of this in
Documentation/virt/kvm/devices/arm-pv-time.txt and refer to this
document from here.
> + struct kvm_create_device pvtime_device = {
> + .type = KVM_DEV_TYPE_ARM_PV_TIME,
> + .attr = 0,
> + .flags = 0,
> + };
> +
> + pvtime_fd = ioctl(vm_fd, KVM_CREATE_DEVICE, &pvtime_device);
> +
> +Creation of the device should be done after creating the vCPUs of the virtual
> +machine.
> +
> +The IPA of the structures must be given to KVM. This is the base address
> +of an array of stolen time structures (one for each VCPU). The base address
> +must be page aligned. The size must be at least 64 * number of VCPUs and be a
> +multiple of PAGE_SIZE.
> +
> +The memory for these structures should be added to the guest in the usual
> +manner (e.g. using KVM_SET_USER_MEMORY_REGION).
> +
> +For example:
> +
> + struct kvm_dev_arm_st_region region = {
> + .gpa = <IPA of guest base address>,
> + .size = <size in bytes>
> + };
> +
> + struct kvm_device_attr st_base = {
> + .group = KVM_DEV_ARM_PV_TIME_PADDR,
> + .attr = KVM_DEV_ARM_PV_TIME_ST,
> + .addr = (u64)®ion
> + };
> +
> + ioctl(pvtime_fd, KVM_SET_DEVICE_ATTR, &st_base);
> --
> 2.20.1
>
Thanks,
Christoffer
^ permalink raw reply
* Re: [PATCH v2 2/3] fbdev: fbmem: allow overriding the number of bootup logos
From: Peter Rosin @ 2019-08-27 8:54 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: linux-kernel@vger.kernel.org, Bartlomiej Zolnierkiewicz,
Jonathan Corbet, dri-devel@lists.freedesktop.org,
linux-fbdev@vger.kernel.org, linux-doc@vger.kernel.org,
Matthew Wilcox
In-Reply-To: <CAMuHMdVx77aOyUVhZ2_N76VAP+AJ3X8w-gdHLjnjUEeRKcZmOA@mail.gmail.com>
On 2019-08-27 10:36, Geert Uytterhoeven wrote:
> Hi Peter,
>
> On Mon, Aug 26, 2019 at 10:46 PM Peter Rosin <peda@axentia.se> wrote:
>> Probably most useful if you only want one logo regardless of how many
>> CPU cores you have.
>>
>> Signed-off-by: Peter Rosin <peda@axentia.se>
>
> Thanks for your patch!
>
>> --- a/Documentation/fb/fbcon.rst
>> +++ b/Documentation/fb/fbcon.rst
>> @@ -174,6 +174,11 @@ C. Boot options
>> displayed due to multiple CPUs, the collected line of logos is moved
>> as a whole.
>>
>> +9. fbcon=logo-count:<n>
>> +
>> + The value 'n' overrides the number of bootup logos. Zero gives the
>> + default, which is the number of online cpus.
>
> Isn't that a bit unexpected for the user?
> What about making -1 the default (auto), and zero meaning no logos?
I just naively assumed there was some other mechanism to disable it.
Sigh, I'll take a look. v3 coming up...
Cheers,
Peter
^ permalink raw reply
* Re: [PATCH v3 01/10] KVM: arm64: Document PV-time interface
From: Christoffer Dall @ 2019-08-27 8:57 UTC (permalink / raw)
To: Steven Price
Cc: Marc Zyngier, Will Deacon, linux-arm-kernel, kvmarm, linux-kernel,
kvm, Catalin Marinas, linux-doc, Russell King, Paolo Bonzini
In-Reply-To: <20190821153656.33429-2-steven.price@arm.com>
On Wed, Aug 21, 2019 at 04:36:47PM +0100, Steven Price wrote:
> Introduce a paravirtualization interface for KVM/arm64 based on the
> "Arm Paravirtualized Time for Arm-Base Systems" specification DEN 0057A.
>
> This only adds the details about "Stolen Time" as the details of "Live
> Physical Time" have not been fully agreed.
>
> User space can specify a reserved area of memory for the guest and
> inform KVM to populate the memory with information on time that the host
> kernel has stolen from the guest.
>
> A hypercall interface is provided for the guest to interrogate the
> hypervisor's support for this interface and the location of the shared
> memory structures.
>
> Signed-off-by: Steven Price <steven.price@arm.com>
> ---
> Documentation/virt/kvm/arm/pvtime.txt | 100 ++++++++++++++++++++++++++
> 1 file changed, 100 insertions(+)
> create mode 100644 Documentation/virt/kvm/arm/pvtime.txt
>
> diff --git a/Documentation/virt/kvm/arm/pvtime.txt b/Documentation/virt/kvm/arm/pvtime.txt
> new file mode 100644
> index 000000000000..1ceb118694e7
> --- /dev/null
> +++ b/Documentation/virt/kvm/arm/pvtime.txt
> @@ -0,0 +1,100 @@
> +Paravirtualized time support for arm64
> +======================================
> +
> +Arm specification DEN0057/A defined a standard for paravirtualised time
> +support for AArch64 guests:
> +
> +https://developer.arm.com/docs/den0057/a
> +
> +KVM/arm64 implements the stolen time part of this specification by providing
> +some hypervisor service calls to support a paravirtualized guest obtaining a
> +view of the amount of time stolen from its execution.
> +
> +Two new SMCCC compatible hypercalls are defined:
> +
> +PV_FEATURES 0xC5000020
> +PV_TIME_ST 0xC5000022
> +
> +These are only available in the SMC64/HVC64 calling convention as
> +paravirtualized time is not available to 32 bit Arm guests. The existence of
> +the PV_FEATURES hypercall should be probed using the SMCCC 1.1 ARCH_FEATURES
> +mechanism before calling it.
> +
> +PV_FEATURES
> + Function ID: (uint32) : 0xC5000020
> + PV_func_id: (uint32) : Either PV_TIME_LPT or PV_TIME_ST
> + Return value: (int32) : NOT_SUPPORTED (-1) or SUCCESS (0) if the relevant
> + PV-time feature is supported by the hypervisor.
> +
> +PV_TIME_ST
> + Function ID: (uint32) : 0xC5000022
> + Return value: (int64) : IPA of the stolen time data structure for this
> + (V)CPU. On failure:
> + NOT_SUPPORTED (-1)
> +
> +The IPA returned by PV_TIME_ST should be mapped by the guest as normal memory
> +with inner and outer write back caching attributes, in the inner shareable
> +domain. A total of 16 bytes from the IPA returned are guaranteed to be
> +meaningfully filled by the hypervisor (see structure below).
> +
> +PV_TIME_ST returns the structure for the calling VCPU.
> +
> +Stolen Time
> +-----------
> +
> +The structure pointed to by the PV_TIME_ST hypercall is as follows:
> +
> + Field | Byte Length | Byte Offset | Description
> + ----------- | ----------- | ----------- | --------------------------
> + Revision | 4 | 0 | Must be 0 for version 0.1
> + Attributes | 4 | 4 | Must be 0
> + Stolen time | 8 | 8 | Stolen time in unsigned
> + | | | nanoseconds indicating how
> + | | | much time this VCPU thread
> + | | | was involuntarily not
> + | | | running on a physical CPU.
> +
> +The structure will be updated by the hypervisor prior to scheduling a VCPU. It
> +will be present within a reserved region of the normal memory given to the
> +guest. The guest should not attempt to write into this memory. There is a
> +structure per VCPU of the guest.
> +
> +User space interface
> +====================
> +
> +User space can request that KVM provide the paravirtualized time interface to
> +a guest by creating a KVM_DEV_TYPE_ARM_PV_TIME device, for example:
> +
> + struct kvm_create_device pvtime_device = {
> + .type = KVM_DEV_TYPE_ARM_PV_TIME,
> + .attr = 0,
> + .flags = 0,
> + };
> +
> + pvtime_fd = ioctl(vm_fd, KVM_CREATE_DEVICE, &pvtime_device);
> +
> +Creation of the device should be done after creating the vCPUs of the virtual
> +machine.
> +
> +The IPA of the structures must be given to KVM. This is the base address
> +of an array of stolen time structures (one for each VCPU). The base address
> +must be page aligned. The size must be at least 64 * number of VCPUs and be a
> +multiple of PAGE_SIZE.
> +
> +The memory for these structures should be added to the guest in the usual
> +manner (e.g. using KVM_SET_USER_MEMORY_REGION).
> +
> +For example:
> +
> + struct kvm_dev_arm_st_region region = {
> + .gpa = <IPA of guest base address>,
> + .size = <size in bytes>
> + };
This feel fragile; how are you handling userspace creating VCPUs after
setting this up, the GPA overlapping guest memory, etc. Is the
philosophy here that the VMM can mess up the VM if it wants, but that
this should never lead attacks on the host (we better hope not) and so
we don't care?
It seems to me setting the IPA per vcpu throught the VCPU device would
avoid a lot of these issues. See
Documentation/virt/kvm/devices/vcpu.txt.
Thanks,
Christoffer
> +
> + struct kvm_device_attr st_base = {
> + .group = KVM_DEV_ARM_PV_TIME_PADDR,
> + .attr = KVM_DEV_ARM_PV_TIME_ST,
> + .addr = (u64)®ion
> + };
> +
> + ioctl(pvtime_fd, KVM_SET_DEVICE_ATTR, &st_base);
> --
> 2.20.1
>
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply
* Re: [PATCH v2 2/3] fbdev: fbmem: allow overriding the number of bootup logos
From: Geert Uytterhoeven @ 2019-08-27 9:18 UTC (permalink / raw)
To: Peter Rosin
Cc: linux-kernel@vger.kernel.org, Bartlomiej Zolnierkiewicz,
Jonathan Corbet, dri-devel@lists.freedesktop.org,
linux-fbdev@vger.kernel.org, linux-doc@vger.kernel.org,
Matthew Wilcox
In-Reply-To: <a31ff144-f037-3580-08b5-aa368572c69d@axentia.se>
Hi Peter,
On Tue, Aug 27, 2019 at 10:54 AM Peter Rosin <peda@axentia.se> wrote:
> On 2019-08-27 10:36, Geert Uytterhoeven wrote:
> > On Mon, Aug 26, 2019 at 10:46 PM Peter Rosin <peda@axentia.se> wrote:
> >> Probably most useful if you only want one logo regardless of how many
> >> CPU cores you have.
> >>
> >> Signed-off-by: Peter Rosin <peda@axentia.se>
> >
> > Thanks for your patch!
> >
> >> --- a/Documentation/fb/fbcon.rst
> >> +++ b/Documentation/fb/fbcon.rst
> >> @@ -174,6 +174,11 @@ C. Boot options
> >> displayed due to multiple CPUs, the collected line of logos is moved
> >> as a whole.
> >>
> >> +9. fbcon=logo-count:<n>
> >> +
> >> + The value 'n' overrides the number of bootup logos. Zero gives the
> >> + default, which is the number of online cpus.
> >
> > Isn't that a bit unexpected for the user?
> > What about making -1 the default (auto), and zero meaning no logos?
>
> I just naively assumed there was some other mechanism to disable it.
That was my first thought, too, but I couldn't find one.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* [PATCH] Documentation: Rename rcu_node_context_switch() to rcu_note_context_switch()
From: Sebastian Andrzej Siewior @ 2019-08-27 9:36 UTC (permalink / raw)
To: Paul E. McKenney
Cc: Joel Fernandes, Scott Wood, linux-kernel, Thomas Gleixner,
Peter Zijlstra, Juri Lelli, Clark Williams, Josh Triplett,
Steven Rostedt, Mathieu Desnoyers, Lai Jiangshan, Jonathan Corbet,
rcu, linux-doc
While Paul was explaning some RCU magic I noticed a typo in
rcu_note_context_switch().
Replace rcu_node_context_switch() with rcu_note_context_switch().
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
.../RCU/Design/Memory-Ordering/Tree-RCU-Memory-Ordering.html | 2 +-
Documentation/RCU/Design/Memory-Ordering/TreeRCU-gp.svg | 2 +-
Documentation/RCU/Design/Memory-Ordering/TreeRCU-qs.svg | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/Documentation/RCU/Design/Memory-Ordering/Tree-RCU-Memory-Ordering.html b/Documentation/RCU/Design/Memory-Ordering/Tree-RCU-Memory-Ordering.html
index c64f8d26609fb..54db02b74f636 100644
--- a/Documentation/RCU/Design/Memory-Ordering/Tree-RCU-Memory-Ordering.html
+++ b/Documentation/RCU/Design/Memory-Ordering/Tree-RCU-Memory-Ordering.html
@@ -481,7 +481,7 @@ section that the grace period must wait on.
</table>
<p>If the CPU does a context switch, a quiescent state will be
-noted by <tt>rcu_node_context_switch()</tt> on the left.
+noted by <tt>rcu_note_context_switch()</tt> on the left.
On the other hand, if the CPU takes a scheduler-clock interrupt
while executing in usermode, a quiescent state will be noted by
<tt>rcu_sched_clock_irq()</tt> on the right.
diff --git a/Documentation/RCU/Design/Memory-Ordering/TreeRCU-gp.svg b/Documentation/RCU/Design/Memory-Ordering/TreeRCU-gp.svg
index 2bcd742d6e491..069f6f8371c20 100644
--- a/Documentation/RCU/Design/Memory-Ordering/TreeRCU-gp.svg
+++ b/Documentation/RCU/Design/Memory-Ordering/TreeRCU-gp.svg
@@ -3880,7 +3880,7 @@
font-style="normal"
y="-4418.6582"
x="3745.7725"
- xml:space="preserve">rcu_node_context_switch()</text>
+ xml:space="preserve">rcu_note_context_switch()</text>
</g>
<g
transform="translate(1881.1886,54048.57)"
diff --git a/Documentation/RCU/Design/Memory-Ordering/TreeRCU-qs.svg b/Documentation/RCU/Design/Memory-Ordering/TreeRCU-qs.svg
index 779c9ac31a527..7d6c5f7e505c6 100644
--- a/Documentation/RCU/Design/Memory-Ordering/TreeRCU-qs.svg
+++ b/Documentation/RCU/Design/Memory-Ordering/TreeRCU-qs.svg
@@ -753,7 +753,7 @@
font-style="normal"
y="-4418.6582"
x="3745.7725"
- xml:space="preserve">rcu_node_context_switch()</text>
+ xml:space="preserve">rcu_note_context_switch()</text>
</g>
<g
transform="translate(3131.2648,-585.6713)"
--
2.23.0
^ permalink raw reply related
* [PATCH v4 1/2] Documentation: perf: Update documentation for ThunderX2 PMU uncore driver
From: Ganapatrao Kulkarni @ 2019-08-27 10:00 UTC (permalink / raw)
To: linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
Cc: will@kernel.org, mark.rutland@arm.com, corbet@lwn.net,
Jayachandran Chandrasekharan Nair, Robert Richter, Jan Glauber,
gklkml16@gmail.com
In-Reply-To: <1566899990-18277-1-git-send-email-gkulkarni@marvell.com>
From: Ganapatrao Kulkarni <ganapatrao.kulkarni@marvell.com>
Add documentation for Cavium Coherent Processor Interconnect (CCPI2) PMU.
Signed-off-by: Ganapatrao Kulkarni <gkulkarni@marvell.com>
---
.../admin-guide/perf/thunderx2-pmu.rst | 20 ++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/Documentation/admin-guide/perf/thunderx2-pmu.rst b/Documentation/admin-guide/perf/thunderx2-pmu.rst
index 08e33675853a..01f158238ae1 100644
--- a/Documentation/admin-guide/perf/thunderx2-pmu.rst
+++ b/Documentation/admin-guide/perf/thunderx2-pmu.rst
@@ -3,24 +3,26 @@ Cavium ThunderX2 SoC Performance Monitoring Unit (PMU UNCORE)
=============================================================
The ThunderX2 SoC PMU consists of independent, system-wide, per-socket
-PMUs such as the Level 3 Cache (L3C) and DDR4 Memory Controller (DMC).
+PMUs such as the Level 3 Cache (L3C), DDR4 Memory Controller (DMC) and
+Cavium Coherent Processor Interconnect (CCPI2).
The DMC has 8 interleaved channels and the L3C has 16 interleaved tiles.
Events are counted for the default channel (i.e. channel 0) and prorated
to the total number of channels/tiles.
-The DMC and L3C support up to 4 counters. Counters are independently
-programmable and can be started and stopped individually. Each counter
-can be set to a different event. Counters are 32-bit and do not support
-an overflow interrupt; they are read every 2 seconds.
+The DMC and L3C support up to 4 counters, while the CCPI2 supports up to 8
+counters. Counters are independently programmable to different events and
+can be started and stopped individually. None of the counters support an
+overflow interrupt. DMC and L3C counters are 32-bit and read every 2 seconds.
+The CCPI2 counters are 64-bit and assumed not to overflow in normal operation.
PMU UNCORE (perf) driver:
The thunderx2_pmu driver registers per-socket perf PMUs for the DMC and
-L3C devices. Each PMU can be used to count up to 4 events
-simultaneously. The PMUs provide a description of their available events
-and configuration options under sysfs, see
-/sys/devices/uncore_<l3c_S/dmc_S/>; S is the socket id.
+L3C devices. Each PMU can be used to count up to 4 (DMC/L3C) or up to 8
+(CCPI2) events simultaneously. The PMUs provide a description of their
+available events and configuration options under sysfs, see
+/sys/devices/uncore_<l3c_S/dmc_S/ccpi2_S/>; S is the socket id.
The driver does not support sampling, therefore "perf record" will not
work. Per-task perf sessions are also not supported.
--
2.17.1
^ permalink raw reply related
* [PATCH v4 0/2] Add CCPI2 PMU support
From: Ganapatrao Kulkarni @ 2019-08-27 10:00 UTC (permalink / raw)
To: linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
Cc: will@kernel.org, mark.rutland@arm.com, corbet@lwn.net,
Jayachandran Chandrasekharan Nair, Robert Richter, Jan Glauber,
gklkml16@gmail.com
Add Cavium Coherent Processor Interconnect (CCPI2) PMU
support in ThunderX2 Uncore driver.
v4:
Update with review comments [2].
Changed Counter read to 2 word read since single dword read is misbhehaving(hw issue).
[2] https://lkml.org/lkml/2019/7/23/231
v3: Rebased to 5.3-rc1
v2: Updated with review comments [1]
[1] https://lkml.org/lkml/2019/6/14/965
v1: initial patch
Ganapatrao Kulkarni (2):
Documentation: perf: Update documentation for ThunderX2 PMU uncore
driver
drivers/perf: Add CCPI2 PMU support in ThunderX2 UNCORE driver.
.../admin-guide/perf/thunderx2-pmu.rst | 20 +-
drivers/perf/thunderx2_pmu.c | 258 +++++++++++++++---
2 files changed, 236 insertions(+), 42 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH v4 2/2] drivers/perf: Add CCPI2 PMU support in ThunderX2 UNCORE driver.
From: Ganapatrao Kulkarni @ 2019-08-27 10:00 UTC (permalink / raw)
To: linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
Cc: will@kernel.org, mark.rutland@arm.com, corbet@lwn.net,
Jayachandran Chandrasekharan Nair, Robert Richter, Jan Glauber,
gklkml16@gmail.com
In-Reply-To: <1566899990-18277-1-git-send-email-gkulkarni@marvell.com>
CCPI2 is a low-latency high-bandwidth serial interface for inter socket
connectivity of ThunderX2 processors.
CCPI2 PMU supports up to 8 counters per socket. Counters are
independently programmable to different events and can be started and
stopped individually. The CCPI2 counters are 64-bit and do not overflow
in normal operation.
Signed-off-by: Ganapatrao Kulkarni <gkulkarni@marvell.com>
---
drivers/perf/thunderx2_pmu.c | 258 ++++++++++++++++++++++++++++++-----
1 file changed, 225 insertions(+), 33 deletions(-)
diff --git a/drivers/perf/thunderx2_pmu.c b/drivers/perf/thunderx2_pmu.c
index 43d76c85da56..d290d8ebbd53 100644
--- a/drivers/perf/thunderx2_pmu.c
+++ b/drivers/perf/thunderx2_pmu.c
@@ -16,23 +16,36 @@
* they need to be sampled before overflow(i.e, at every 2 seconds).
*/
-#define TX2_PMU_MAX_COUNTERS 4
+#define TX2_PMU_DMC_L3C_MAX_COUNTERS 4
+#define TX2_PMU_CCPI2_MAX_COUNTERS 8
+#define TX2_PMU_MAX_COUNTERS TX2_PMU_CCPI2_MAX_COUNTERS
+
+
#define TX2_PMU_DMC_CHANNELS 8
#define TX2_PMU_L3_TILES 16
#define TX2_PMU_HRTIMER_INTERVAL (2 * NSEC_PER_SEC)
-#define GET_EVENTID(ev) ((ev->hw.config) & 0x1f)
-#define GET_COUNTERID(ev) ((ev->hw.idx) & 0x3)
+#define GET_EVENTID(ev, mask) ((ev->hw.config) & mask)
+#define GET_COUNTERID(ev, mask) ((ev->hw.idx) & mask)
/* 1 byte per counter(4 counters).
* Event id is encoded in bits [5:1] of a byte,
*/
#define DMC_EVENT_CFG(idx, val) ((val) << (((idx) * 8) + 1))
+/* bits[3:0] to select counters, are indexed from 8 to 15. */
+#define CCPI2_COUNTER_OFFSET 8
+
#define L3C_COUNTER_CTL 0xA8
#define L3C_COUNTER_DATA 0xAC
#define DMC_COUNTER_CTL 0x234
#define DMC_COUNTER_DATA 0x240
+#define CCPI2_PERF_CTL 0x108
+#define CCPI2_COUNTER_CTL 0x10C
+#define CCPI2_COUNTER_SEL 0x12c
+#define CCPI2_COUNTER_DATA_L 0x130
+#define CCPI2_COUNTER_DATA_H 0x134
+
/* L3C event IDs */
#define L3_EVENT_READ_REQ 0xD
#define L3_EVENT_WRITEBACK_REQ 0xE
@@ -51,15 +64,28 @@
#define DMC_EVENT_READ_TXNS 0xF
#define DMC_EVENT_MAX 0x10
+#define CCPI2_EVENT_REQ_PKT_SENT 0x3D
+#define CCPI2_EVENT_SNOOP_PKT_SENT 0x65
+#define CCPI2_EVENT_DATA_PKT_SENT 0x105
+#define CCPI2_EVENT_GIC_PKT_SENT 0x12D
+#define CCPI2_EVENT_MAX 0x200
+
+#define CCPI2_PERF_CTL_ENABLE BIT(0)
+#define CCPI2_PERF_CTL_START BIT(1)
+#define CCPI2_PERF_CTL_RESET BIT(4)
+#define CCPI2_EVENT_LEVEL_RISING_EDGE BIT(10)
+#define CCPI2_EVENT_TYPE_EDGE_SENSITIVE BIT(11)
+
enum tx2_uncore_type {
PMU_TYPE_L3C,
PMU_TYPE_DMC,
+ PMU_TYPE_CCPI2,
PMU_TYPE_INVALID,
};
/*
- * pmu on each socket has 2 uncore devices(dmc and l3c),
- * each device has 4 counters.
+ * Each socket has 3 uncore devices associated with a PMU. The DMC and
+ * L3C have 4 32-bit counters and the CCPI2 has 8 64-bit counters.
*/
struct tx2_uncore_pmu {
struct hlist_node hpnode;
@@ -69,8 +95,10 @@ struct tx2_uncore_pmu {
int node;
int cpu;
u32 max_counters;
+ u32 counters_mask;
u32 prorate_factor;
u32 max_events;
+ u32 events_mask;
u64 hrtimer_interval;
void __iomem *base;
DECLARE_BITMAP(active_counters, TX2_PMU_MAX_COUNTERS);
@@ -92,7 +120,21 @@ static inline struct tx2_uncore_pmu *pmu_to_tx2_pmu(struct pmu *pmu)
return container_of(pmu, struct tx2_uncore_pmu, pmu);
}
-PMU_FORMAT_ATTR(event, "config:0-4");
+#define TX2_PMU_FORMAT_ATTR(_var, _name, _format) \
+static ssize_t \
+__tx2_pmu_##_var##_show(struct device *dev, \
+ struct device_attribute *attr, \
+ char *page) \
+{ \
+ BUILD_BUG_ON(sizeof(_format) >= PAGE_SIZE); \
+ return sprintf(page, _format "\n"); \
+} \
+ \
+static struct device_attribute format_attr_##_var = \
+ __ATTR(_name, 0444, __tx2_pmu_##_var##_show, NULL)
+
+TX2_PMU_FORMAT_ATTR(event, event, "config:0-4");
+TX2_PMU_FORMAT_ATTR(event_ccpi2, event, "config:0-9");
static struct attribute *l3c_pmu_format_attrs[] = {
&format_attr_event.attr,
@@ -104,6 +146,11 @@ static struct attribute *dmc_pmu_format_attrs[] = {
NULL,
};
+static struct attribute *ccpi2_pmu_format_attrs[] = {
+ &format_attr_event_ccpi2.attr,
+ NULL,
+};
+
static const struct attribute_group l3c_pmu_format_attr_group = {
.name = "format",
.attrs = l3c_pmu_format_attrs,
@@ -114,6 +161,11 @@ static const struct attribute_group dmc_pmu_format_attr_group = {
.attrs = dmc_pmu_format_attrs,
};
+static const struct attribute_group ccpi2_pmu_format_attr_group = {
+ .name = "format",
+ .attrs = ccpi2_pmu_format_attrs,
+};
+
/*
* sysfs event attributes
*/
@@ -164,6 +216,19 @@ static struct attribute *dmc_pmu_events_attrs[] = {
NULL,
};
+TX2_EVENT_ATTR(req_pktsent, CCPI2_EVENT_REQ_PKT_SENT);
+TX2_EVENT_ATTR(snoop_pktsent, CCPI2_EVENT_SNOOP_PKT_SENT);
+TX2_EVENT_ATTR(data_pktsent, CCPI2_EVENT_DATA_PKT_SENT);
+TX2_EVENT_ATTR(gic_pktsent, CCPI2_EVENT_GIC_PKT_SENT);
+
+static struct attribute *ccpi2_pmu_events_attrs[] = {
+ &tx2_pmu_event_attr_req_pktsent.attr.attr,
+ &tx2_pmu_event_attr_snoop_pktsent.attr.attr,
+ &tx2_pmu_event_attr_data_pktsent.attr.attr,
+ &tx2_pmu_event_attr_gic_pktsent.attr.attr,
+ NULL,
+};
+
static const struct attribute_group l3c_pmu_events_attr_group = {
.name = "events",
.attrs = l3c_pmu_events_attrs,
@@ -174,6 +239,11 @@ static const struct attribute_group dmc_pmu_events_attr_group = {
.attrs = dmc_pmu_events_attrs,
};
+static const struct attribute_group ccpi2_pmu_events_attr_group = {
+ .name = "events",
+ .attrs = ccpi2_pmu_events_attrs,
+};
+
/*
* sysfs cpumask attributes
*/
@@ -213,6 +283,13 @@ static const struct attribute_group *dmc_pmu_attr_groups[] = {
NULL
};
+static const struct attribute_group *ccpi2_pmu_attr_groups[] = {
+ &ccpi2_pmu_format_attr_group,
+ &pmu_cpumask_attr_group,
+ &ccpi2_pmu_events_attr_group,
+ NULL
+};
+
static inline u32 reg_readl(unsigned long addr)
{
return readl((void __iomem *)addr);
@@ -245,33 +322,58 @@ static void init_cntr_base_l3c(struct perf_event *event,
struct tx2_uncore_pmu *tx2_pmu)
{
struct hw_perf_event *hwc = &event->hw;
+ u32 cmask;
+
+ tx2_pmu = pmu_to_tx2_pmu(event->pmu);
+ cmask = tx2_pmu->counters_mask;
/* counter ctrl/data reg offset at 8 */
hwc->config_base = (unsigned long)tx2_pmu->base
- + L3C_COUNTER_CTL + (8 * GET_COUNTERID(event));
+ + L3C_COUNTER_CTL + (8 * GET_COUNTERID(event, cmask));
hwc->event_base = (unsigned long)tx2_pmu->base
- + L3C_COUNTER_DATA + (8 * GET_COUNTERID(event));
+ + L3C_COUNTER_DATA + (8 * GET_COUNTERID(event, cmask));
}
static void init_cntr_base_dmc(struct perf_event *event,
struct tx2_uncore_pmu *tx2_pmu)
{
struct hw_perf_event *hwc = &event->hw;
+ u32 cmask;
+
+ tx2_pmu = pmu_to_tx2_pmu(event->pmu);
+ cmask = tx2_pmu->counters_mask;
hwc->config_base = (unsigned long)tx2_pmu->base
+ DMC_COUNTER_CTL;
/* counter data reg offset at 0xc */
hwc->event_base = (unsigned long)tx2_pmu->base
- + DMC_COUNTER_DATA + (0xc * GET_COUNTERID(event));
+ + DMC_COUNTER_DATA + (0xc * GET_COUNTERID(event, cmask));
+}
+
+static void init_cntr_base_ccpi2(struct perf_event *event,
+ struct tx2_uncore_pmu *tx2_pmu)
+{
+ struct hw_perf_event *hwc = &event->hw;
+ u32 cmask;
+
+ cmask = tx2_pmu->counters_mask;
+
+ hwc->config_base = (unsigned long)tx2_pmu->base
+ + CCPI2_COUNTER_CTL + (4 * GET_COUNTERID(event, cmask));
+ hwc->event_base = (unsigned long)tx2_pmu->base;
}
static void uncore_start_event_l3c(struct perf_event *event, int flags)
{
- u32 val;
+ u32 val, emask;
struct hw_perf_event *hwc = &event->hw;
+ struct tx2_uncore_pmu *tx2_pmu;
+
+ tx2_pmu = pmu_to_tx2_pmu(event->pmu);
+ emask = tx2_pmu->events_mask;
/* event id encoded in bits [07:03] */
- val = GET_EVENTID(event) << 3;
+ val = GET_EVENTID(event, emask) << 3;
reg_writel(val, hwc->config_base);
local64_set(&hwc->prev_count, 0);
reg_writel(0, hwc->event_base);
@@ -284,10 +386,17 @@ static inline void uncore_stop_event_l3c(struct perf_event *event)
static void uncore_start_event_dmc(struct perf_event *event, int flags)
{
- u32 val;
+ u32 val, cmask, emask;
struct hw_perf_event *hwc = &event->hw;
- int idx = GET_COUNTERID(event);
- int event_id = GET_EVENTID(event);
+ struct tx2_uncore_pmu *tx2_pmu;
+ int idx, event_id;
+
+ tx2_pmu = pmu_to_tx2_pmu(event->pmu);
+ cmask = tx2_pmu->counters_mask;
+ emask = tx2_pmu->events_mask;
+
+ idx = GET_COUNTERID(event, cmask);
+ event_id = GET_EVENTID(event, emask);
/* enable and start counters.
* 8 bits for each counter, bits[05:01] of a counter to set event type.
@@ -302,9 +411,14 @@ static void uncore_start_event_dmc(struct perf_event *event, int flags)
static void uncore_stop_event_dmc(struct perf_event *event)
{
- u32 val;
+ u32 val, cmask;
struct hw_perf_event *hwc = &event->hw;
- int idx = GET_COUNTERID(event);
+ struct tx2_uncore_pmu *tx2_pmu;
+ int idx;
+
+ tx2_pmu = pmu_to_tx2_pmu(event->pmu);
+ cmask = tx2_pmu->counters_mask;
+ idx = GET_COUNTERID(event, cmask);
/* clear event type(bits[05:01]) to stop counter */
val = reg_readl(hwc->config_base);
@@ -312,27 +426,72 @@ static void uncore_stop_event_dmc(struct perf_event *event)
reg_writel(val, hwc->config_base);
}
+static void uncore_start_event_ccpi2(struct perf_event *event, int flags)
+{
+ u32 emask;
+ struct hw_perf_event *hwc = &event->hw;
+ struct tx2_uncore_pmu *tx2_pmu;
+
+ tx2_pmu = pmu_to_tx2_pmu(event->pmu);
+ emask = tx2_pmu->events_mask;
+
+ /* Bit [09:00] to set event id.
+ * Bits [10], set level to rising edge.
+ * Bits [11], set type to edge sensitive.
+ */
+ reg_writel((CCPI2_EVENT_TYPE_EDGE_SENSITIVE |
+ CCPI2_EVENT_LEVEL_RISING_EDGE |
+ GET_EVENTID(event, emask)), hwc->config_base);
+
+ /* reset[4], enable[0] and start[1] counters */
+ reg_writel(CCPI2_PERF_CTL_RESET |
+ CCPI2_PERF_CTL_START |
+ CCPI2_PERF_CTL_ENABLE,
+ hwc->event_base + CCPI2_PERF_CTL);
+ local64_set(&event->hw.prev_count, 0ULL);
+}
+
+static void uncore_stop_event_ccpi2(struct perf_event *event)
+{
+ struct hw_perf_event *hwc = &event->hw;
+
+ /* disable and stop counter */
+ reg_writel(0, hwc->event_base + CCPI2_PERF_CTL);
+}
+
static void tx2_uncore_event_update(struct perf_event *event)
{
- s64 prev, delta, new = 0;
+ u64 prev, delta, new = 0;
struct hw_perf_event *hwc = &event->hw;
struct tx2_uncore_pmu *tx2_pmu;
enum tx2_uncore_type type;
u32 prorate_factor;
+ u32 cmask, emask;
tx2_pmu = pmu_to_tx2_pmu(event->pmu);
type = tx2_pmu->type;
+ cmask = tx2_pmu->counters_mask;
+ emask = tx2_pmu->events_mask;
prorate_factor = tx2_pmu->prorate_factor;
-
- new = reg_readl(hwc->event_base);
- prev = local64_xchg(&hwc->prev_count, new);
-
- /* handles rollover of 32 bit counter */
- delta = (u32)(((1UL << 32) - prev) + new);
+ if (type == PMU_TYPE_CCPI2) {
+ reg_writel(CCPI2_COUNTER_OFFSET +
+ GET_COUNTERID(event, cmask),
+ hwc->event_base + CCPI2_COUNTER_SEL);
+ new = reg_readl(hwc->event_base + CCPI2_COUNTER_DATA_H);
+ new = (new << 32) +
+ reg_readl(hwc->event_base + CCPI2_COUNTER_DATA_L);
+ prev = local64_xchg(&hwc->prev_count, new);
+ delta = new - prev;
+ } else {
+ new = reg_readl(hwc->event_base);
+ prev = local64_xchg(&hwc->prev_count, new);
+ /* handles rollover of 32 bit counter */
+ delta = (u32)(((1UL << 32) - prev) + new);
+ }
/* DMC event data_transfers granularity is 16 Bytes, convert it to 64 */
if (type == PMU_TYPE_DMC &&
- GET_EVENTID(event) == DMC_EVENT_DATA_TRANSFERS)
+ GET_EVENTID(event, emask) == DMC_EVENT_DATA_TRANSFERS)
delta = delta/4;
/* L3C and DMC has 16 and 8 interleave channels respectively.
@@ -351,6 +510,7 @@ static enum tx2_uncore_type get_tx2_pmu_type(struct acpi_device *adev)
} devices[] = {
{"CAV901D", PMU_TYPE_L3C},
{"CAV901F", PMU_TYPE_DMC},
+ {"CAV901E", PMU_TYPE_CCPI2},
{"", PMU_TYPE_INVALID}
};
@@ -380,7 +540,8 @@ static bool tx2_uncore_validate_event(struct pmu *pmu,
* Make sure the group of events can be scheduled at once
* on the PMU.
*/
-static bool tx2_uncore_validate_event_group(struct perf_event *event)
+static bool tx2_uncore_validate_event_group(struct perf_event *event,
+ int max_counters)
{
struct perf_event *sibling, *leader = event->group_leader;
int counters = 0;
@@ -403,7 +564,7 @@ static bool tx2_uncore_validate_event_group(struct perf_event *event)
* If the group requires more counters than the HW has,
* it cannot ever be scheduled.
*/
- return counters <= TX2_PMU_MAX_COUNTERS;
+ return counters <= max_counters;
}
@@ -439,7 +600,7 @@ static int tx2_uncore_event_init(struct perf_event *event)
hwc->config = event->attr.config;
/* Validate the group */
- if (!tx2_uncore_validate_event_group(event))
+ if (!tx2_uncore_validate_event_group(event, tx2_pmu->max_counters))
return -EINVAL;
return 0;
@@ -456,6 +617,10 @@ static void tx2_uncore_event_start(struct perf_event *event, int flags)
tx2_pmu->start_event(event, flags);
perf_event_update_userpage(event);
+ /* No hrtimer needed for CCPI2, 64-bit counters */
+ if (!tx2_pmu->hrtimer.function)
+ return;
+
/* Start timer for first event */
if (bitmap_weight(tx2_pmu->active_counters,
tx2_pmu->max_counters) == 1) {
@@ -510,11 +675,13 @@ static void tx2_uncore_event_del(struct perf_event *event, int flags)
{
struct tx2_uncore_pmu *tx2_pmu = pmu_to_tx2_pmu(event->pmu);
struct hw_perf_event *hwc = &event->hw;
+ u32 cmask;
+ cmask = tx2_pmu->counters_mask;
tx2_uncore_event_stop(event, PERF_EF_UPDATE);
/* clear the assigned counter */
- free_counter(tx2_pmu, GET_COUNTERID(event));
+ free_counter(tx2_pmu, GET_COUNTERID(event, cmask));
perf_event_update_userpage(event);
tx2_pmu->events[hwc->idx] = NULL;
@@ -580,8 +747,10 @@ static int tx2_uncore_pmu_add_dev(struct tx2_uncore_pmu *tx2_pmu)
cpu_online_mask);
tx2_pmu->cpu = cpu;
- hrtimer_init(&tx2_pmu->hrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
- tx2_pmu->hrtimer.function = tx2_hrtimer_callback;
+
+ if (tx2_pmu->hrtimer.function)
+ hrtimer_init(&tx2_pmu->hrtimer,
+ CLOCK_MONOTONIC, HRTIMER_MODE_REL);
ret = tx2_uncore_pmu_register(tx2_pmu);
if (ret) {
@@ -653,10 +822,13 @@ static struct tx2_uncore_pmu *tx2_uncore_pmu_init_dev(struct device *dev,
switch (tx2_pmu->type) {
case PMU_TYPE_L3C:
- tx2_pmu->max_counters = TX2_PMU_MAX_COUNTERS;
+ tx2_pmu->max_counters = TX2_PMU_DMC_L3C_MAX_COUNTERS;
+ tx2_pmu->counters_mask = 0x3;
tx2_pmu->prorate_factor = TX2_PMU_L3_TILES;
tx2_pmu->max_events = L3_EVENT_MAX;
+ tx2_pmu->events_mask = 0x1f;
tx2_pmu->hrtimer_interval = TX2_PMU_HRTIMER_INTERVAL;
+ tx2_pmu->hrtimer.function = tx2_hrtimer_callback;
tx2_pmu->attr_groups = l3c_pmu_attr_groups;
tx2_pmu->name = devm_kasprintf(dev, GFP_KERNEL,
"uncore_l3c_%d", tx2_pmu->node);
@@ -665,10 +837,13 @@ static struct tx2_uncore_pmu *tx2_uncore_pmu_init_dev(struct device *dev,
tx2_pmu->stop_event = uncore_stop_event_l3c;
break;
case PMU_TYPE_DMC:
- tx2_pmu->max_counters = TX2_PMU_MAX_COUNTERS;
+ tx2_pmu->max_counters = TX2_PMU_DMC_L3C_MAX_COUNTERS;
+ tx2_pmu->counters_mask = 0x3;
tx2_pmu->prorate_factor = TX2_PMU_DMC_CHANNELS;
tx2_pmu->max_events = DMC_EVENT_MAX;
+ tx2_pmu->events_mask = 0x1f;
tx2_pmu->hrtimer_interval = TX2_PMU_HRTIMER_INTERVAL;
+ tx2_pmu->hrtimer.function = tx2_hrtimer_callback;
tx2_pmu->attr_groups = dmc_pmu_attr_groups;
tx2_pmu->name = devm_kasprintf(dev, GFP_KERNEL,
"uncore_dmc_%d", tx2_pmu->node);
@@ -676,6 +851,21 @@ static struct tx2_uncore_pmu *tx2_uncore_pmu_init_dev(struct device *dev,
tx2_pmu->start_event = uncore_start_event_dmc;
tx2_pmu->stop_event = uncore_stop_event_dmc;
break;
+ case PMU_TYPE_CCPI2:
+ /* CCPI2 has 8 counters */
+ tx2_pmu->max_counters = TX2_PMU_CCPI2_MAX_COUNTERS;
+ tx2_pmu->counters_mask = 0x7;
+ tx2_pmu->prorate_factor = 1;
+ tx2_pmu->max_events = CCPI2_EVENT_MAX;
+ tx2_pmu->events_mask = 0x1ff;
+ tx2_pmu->attr_groups = ccpi2_pmu_attr_groups;
+ tx2_pmu->name = devm_kasprintf(dev, GFP_KERNEL,
+ "uncore_ccpi2_%d", tx2_pmu->node);
+ tx2_pmu->init_cntr_base = init_cntr_base_ccpi2;
+ tx2_pmu->start_event = uncore_start_event_ccpi2;
+ tx2_pmu->stop_event = uncore_stop_event_ccpi2;
+ tx2_pmu->hrtimer.function = NULL;
+ break;
case PMU_TYPE_INVALID:
devm_kfree(dev, tx2_pmu);
return NULL;
@@ -744,7 +934,9 @@ static int tx2_uncore_pmu_offline_cpu(unsigned int cpu,
if (cpu != tx2_pmu->cpu)
return 0;
- hrtimer_cancel(&tx2_pmu->hrtimer);
+ if (tx2_pmu->hrtimer.function)
+ hrtimer_cancel(&tx2_pmu->hrtimer);
+
cpumask_copy(&cpu_online_mask_temp, cpu_online_mask);
cpumask_clear_cpu(cpu, &cpu_online_mask_temp);
new_cpu = cpumask_any_and(
--
2.17.1
^ permalink raw reply related
* [PATCH v3 0/3] Add possibility to specify the number of displayed logos
From: Peter Rosin @ 2019-08-27 11:09 UTC (permalink / raw)
To: linux-kernel@vger.kernel.org
Cc: Peter Rosin, Bartlomiej Zolnierkiewicz, Jonathan Corbet,
dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org,
linux-doc@vger.kernel.org, Matthew Wilcox, Geert Uytterhoeven
Hi!
The first patch fixes the fact that there are two items numbered "4" in
the list of fbcon options. This bug is a teenager...
The second patch extends that list with a new option that allows the
user to display any number of logos (that fits on the screen). I need it
to limit the display to only one logo instead of one for each CPU core.
Changes since v2
- make -1 the default and make 0 disable the logo [Geert Uytterhoeven]
- cpu -> CPU
Changes since v1
- do not needlessly export fb_logo_count [Matthew Wilcox]
- added patch 3/3, which removes the export of fb_center_logo
Cheers,
Peter
Peter Rosin (3):
fbdev: fix numbering of fbcon options
fbdev: fbmem: allow overriding the number of bootup logos
fbdev: fbmem: avoid exporting fb_center_logo
Documentation/fb/fbcon.rst | 13 +++++++++----
drivers/video/fbdev/core/fbcon.c | 7 +++++++
drivers/video/fbdev/core/fbmem.c | 13 +++++++++----
include/linux/fb.h | 1 +
4 files changed, 26 insertions(+), 8 deletions(-)
--
2.11.0
^ permalink raw reply
* [PATCH v3 1/3] fbdev: fix numbering of fbcon options
From: Peter Rosin @ 2019-08-27 11:09 UTC (permalink / raw)
To: linux-kernel@vger.kernel.org
Cc: Peter Rosin, Bartlomiej Zolnierkiewicz, Jonathan Corbet,
dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org,
linux-doc@vger.kernel.org, Matthew Wilcox, Geert Uytterhoeven
In-Reply-To: <20190827110854.12574-1-peda@axentia.se>
Three shall be the number thou shalt count, and the number of the
counting shall be three. Four shalt thou not count...
One! Two! Five!
Fixes: efb985f6b265 ("[PATCH] fbcon: Console Rotation - Add framebuffer console documentation")
Signed-off-by: Peter Rosin <peda@axentia.se>
---
Documentation/fb/fbcon.rst | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/Documentation/fb/fbcon.rst b/Documentation/fb/fbcon.rst
index ebca41785abe..65ba40255137 100644
--- a/Documentation/fb/fbcon.rst
+++ b/Documentation/fb/fbcon.rst
@@ -127,7 +127,7 @@ C. Boot options
is typically located on the same video card. Thus, the consoles that
are controlled by the VGA console will be garbled.
-4. fbcon=rotate:<n>
+5. fbcon=rotate:<n>
This option changes the orientation angle of the console display. The
value 'n' accepts the following:
@@ -152,21 +152,21 @@ C. Boot options
Actually, the underlying fb driver is totally ignorant of console
rotation.
-5. fbcon=margin:<color>
+6. fbcon=margin:<color>
This option specifies the color of the margins. The margins are the
leftover area at the right and the bottom of the screen that are not
used by text. By default, this area will be black. The 'color' value
is an integer number that depends on the framebuffer driver being used.
-6. fbcon=nodefer
+7. fbcon=nodefer
If the kernel is compiled with deferred fbcon takeover support, normally
the framebuffer contents, left in place by the firmware/bootloader, will
be preserved until there actually is some text is output to the console.
This option causes fbcon to bind immediately to the fbdev device.
-7. fbcon=logo-pos:<location>
+8. fbcon=logo-pos:<location>
The only possible 'location' is 'center' (without quotes), and when
given, the bootup logo is moved from the default top-left corner
--
2.11.0
^ permalink raw reply related
* [PATCH v3 2/3] fbdev: fbmem: allow overriding the number of bootup logos
From: Peter Rosin @ 2019-08-27 11:09 UTC (permalink / raw)
To: linux-kernel@vger.kernel.org
Cc: Peter Rosin, Bartlomiej Zolnierkiewicz, Jonathan Corbet,
dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org,
linux-doc@vger.kernel.org, Matthew Wilcox, Geert Uytterhoeven
In-Reply-To: <20190827110854.12574-1-peda@axentia.se>
Probably most useful if you want no logo at all, or if you only want one
logo regardless of how many CPU cores you have.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
Documentation/fb/fbcon.rst | 5 +++++
drivers/video/fbdev/core/fbcon.c | 7 +++++++
drivers/video/fbdev/core/fbmem.c | 12 +++++++++---
include/linux/fb.h | 1 +
4 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/Documentation/fb/fbcon.rst b/Documentation/fb/fbcon.rst
index 65ba40255137..e57a3d1d085a 100644
--- a/Documentation/fb/fbcon.rst
+++ b/Documentation/fb/fbcon.rst
@@ -174,6 +174,11 @@ C. Boot options
displayed due to multiple CPUs, the collected line of logos is moved
as a whole.
+9. fbcon=logo-count:<n>
+
+ The value 'n' overrides the number of bootup logos. 0 disables the
+ logo, and -1 gives the default which is the number of online CPUs.
+
C. Attaching, Detaching and Unloading
Before going on to how to attach, detach and unload the framebuffer console, an
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index c9235a2f42f8..bb6ae995c2e5 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -536,6 +536,13 @@ static int __init fb_console_setup(char *this_opt)
fb_center_logo = true;
continue;
}
+
+ if (!strncmp(options, "logo-count:", 11)) {
+ options += 11;
+ if (*options)
+ fb_logo_count = simple_strtol(options, &options, 0);
+ continue;
+ }
}
return 1;
}
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 64dd732021d8..c7ddcb72025b 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -56,6 +56,8 @@ EXPORT_SYMBOL(num_registered_fb);
bool fb_center_logo __read_mostly;
EXPORT_SYMBOL(fb_center_logo);
+int fb_logo_count __read_mostly = -1;
+
static struct fb_info *get_fb_info(unsigned int idx)
{
struct fb_info *fb_info;
@@ -620,7 +622,7 @@ int fb_prepare_logo(struct fb_info *info, int rotate)
memset(&fb_logo, 0, sizeof(struct logo_data));
if (info->flags & FBINFO_MISC_TILEBLITTING ||
- info->fbops->owner)
+ info->fbops->owner || !fb_logo_count)
return 0;
if (info->fix.visual == FB_VISUAL_DIRECTCOLOR) {
@@ -686,10 +688,14 @@ int fb_prepare_logo(struct fb_info *info, int rotate)
int fb_show_logo(struct fb_info *info, int rotate)
{
+ unsigned int count;
int y;
- y = fb_show_logo_line(info, rotate, fb_logo.logo, 0,
- num_online_cpus());
+ if (!fb_logo_count)
+ return 0;
+
+ count = fb_logo_count < 0 ? num_online_cpus() : fb_logo_count;
+ y = fb_show_logo_line(info, rotate, fb_logo.logo, 0, count);
y = fb_show_extra_logos(info, y, rotate);
return y;
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 303771264644..e37f72b2efca 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -630,6 +630,7 @@ extern int fb_new_modelist(struct fb_info *info);
extern struct fb_info *registered_fb[FB_MAX];
extern int num_registered_fb;
extern bool fb_center_logo;
+extern int fb_logo_count;
extern struct class *fb_class;
#define for_each_registered_fb(i) \
--
2.11.0
^ permalink raw reply related
* [PATCH v3 3/3] fbdev: fbmem: avoid exporting fb_center_logo
From: Peter Rosin @ 2019-08-27 11:09 UTC (permalink / raw)
To: linux-kernel@vger.kernel.org
Cc: Peter Rosin, Bartlomiej Zolnierkiewicz, Jonathan Corbet,
dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org,
linux-doc@vger.kernel.org, Matthew Wilcox, Geert Uytterhoeven
In-Reply-To: <20190827110854.12574-1-peda@axentia.se>
The variable is only ever used from fbcon.c which is linked into the
same module. Therefore, the export is not needed.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/video/fbdev/core/fbmem.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index c7ddcb72025b..d45e59ac351b 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -54,7 +54,6 @@ int num_registered_fb __read_mostly;
EXPORT_SYMBOL(num_registered_fb);
bool fb_center_logo __read_mostly;
-EXPORT_SYMBOL(fb_center_logo);
int fb_logo_count __read_mostly = -1;
--
2.11.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox