* [PATCH 1/2] powerpc/lib: fix redundant inclusion of quad.o
From: Christophe Leroy @ 2019-05-13 10:00 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
quad.o is only for PPC64, and already included in obj64-y,
so it doesn't have to be in obj-y
Fixes: 31bfdb036f12 ("powerpc: Use instruction emulation infrastructure to handle alignment faults")
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/lib/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
index c55f9c27bf79..17fce3738d48 100644
--- a/arch/powerpc/lib/Makefile
+++ b/arch/powerpc/lib/Makefile
@@ -49,7 +49,7 @@ obj64-$(CONFIG_KPROBES_SANITY_TEST) += test_emulate_step.o \
obj-y += checksum_$(BITS).o checksum_wrappers.o \
string_$(BITS).o
-obj-y += sstep.o ldstfp.o quad.o
+obj-y += sstep.o ldstfp.o
obj64-y += quad.o
obj-$(CONFIG_PPC_LIB_RHEAP) += rheap.o
--
2.13.3
^ permalink raw reply related
* [PATCH 2/2] powerpc/lib: only build ldstfp.o when CONFIG_PPC_FPU is set
From: Christophe Leroy @ 2019-05-13 10:00 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <7496da89e027e563cb8e62dc89548525cf53b57e.1557741292.git.christophe.leroy@c-s.fr>
The entire code in ldstfp.o is enclosed into #ifdef CONFIG_PPC_FPU,
so there is no point in building it when this config is not selected.
Fixes: cd64d1697cf0 ("powerpc: mtmsrd not defined")
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/lib/Makefile | 3 ++-
arch/powerpc/lib/ldstfp.S | 4 ----
2 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
index 17fce3738d48..eebc782d89a5 100644
--- a/arch/powerpc/lib/Makefile
+++ b/arch/powerpc/lib/Makefile
@@ -49,7 +49,8 @@ obj64-$(CONFIG_KPROBES_SANITY_TEST) += test_emulate_step.o \
obj-y += checksum_$(BITS).o checksum_wrappers.o \
string_$(BITS).o
-obj-y += sstep.o ldstfp.o
+obj-y += sstep.o
+obj-$(CONFIG_PPC_FPU) += ldstfp.o
obj64-y += quad.o
obj-$(CONFIG_PPC_LIB_RHEAP) += rheap.o
diff --git a/arch/powerpc/lib/ldstfp.S b/arch/powerpc/lib/ldstfp.S
index 32e91994b6b2..e388a3127cb6 100644
--- a/arch/powerpc/lib/ldstfp.S
+++ b/arch/powerpc/lib/ldstfp.S
@@ -18,8 +18,6 @@
#include <asm/asm-compat.h>
#include <linux/errno.h>
-#ifdef CONFIG_PPC_FPU
-
#define STKFRM (PPC_MIN_STKFRM + 16)
/* Get the contents of frN into *p; N is in r3 and p is in r4. */
@@ -241,5 +239,3 @@ _GLOBAL(conv_dp_to_sp)
MTMSRD(r6)
isync
blr
-
-#endif /* CONFIG_PPC_FPU */
--
2.13.3
^ permalink raw reply related
* Re: [PATCH 1/2] perf ioctl: Add check for the sample_period value
From: Ravi Bangoria @ 2019-05-13 10:07 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Ravi Bangoria, maddy, linuxppc-dev, linux-kernel, acme, jolsa
In-Reply-To: <20190513085620.GN2650@hirez.programming.kicks-ass.net>
On 5/13/19 2:26 PM, Peter Zijlstra wrote:
> On Mon, May 13, 2019 at 09:42:13AM +0200, Peter Zijlstra wrote:
>> On Sat, May 11, 2019 at 08:12:16AM +0530, Ravi Bangoria wrote:
>>> Add a check for sample_period value sent from userspace. Negative
>>> value does not make sense. And in powerpc arch code this could cause
>>> a recursive PMI leading to a hang (reported when running perf-fuzzer).
>>>
>>> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
>>> ---
>>> kernel/events/core.c | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>> diff --git a/kernel/events/core.c b/kernel/events/core.c
>>> index abbd4b3b96c2..e44c90378940 100644
>>> --- a/kernel/events/core.c
>>> +++ b/kernel/events/core.c
>>> @@ -5005,6 +5005,9 @@ static int perf_event_period(struct perf_event *event, u64 __user *arg)
>>> if (perf_event_check_period(event, value))
>>> return -EINVAL;
>>>
>>> + if (!event->attr.freq && (value & (1ULL << 63)))
>>> + return -EINVAL;
>>
>> Well, perf_event_attr::sample_period is __u64. Would not be the site
>> using it as signed be the one in error?
>
> You forgot to mention commit: 0819b2e30ccb9, so I guess this just makes
> it consistent and is fine.
>
Yeah, I was about to reply :)
^ permalink raw reply
* [PATCH v3 0/6] soc/fsl/qe: cleanups and new DT binding
From: Rasmus Villemoes @ 2019-05-13 11:14 UTC (permalink / raw)
To: devicetree@vger.kernel.org, Qiang Zhao, Li Yang
Cc: Mark Rutland, linux-kernel@vger.kernel.org, Scott Wood,
Rasmus Villemoes, Rob Herring, linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190501092841.9026-1-rasmus.villemoes@prevas.dk>
This small series consists of some small cleanups and simplifications
of the QUICC engine driver, and introduces a new DT binding that makes
it much easier to support other variants of the QUICC engine IP block
that appears in the wild: There's no reason to expect in general that
the number of valid SNUMs uniquely determines the set of such, so it's
better to simply let the device tree specify the values (and,
implicitly via the array length, also the count).
Which tree should this go through?
v3:
- Move example from commit log into binding document (adapting to
MPC8360 which the existing example pertains to).
- Add more review tags.
- Fix minor style issue.
v2:
- Address comments from Christophe Leroy
- Add his Reviewed-by to 1/6 and 3/6
- Split DT binding update to separate patch as per
Documentation/devicetree/bindings/submitting-patches.txt
Rasmus Villemoes (6):
soc/fsl/qe: qe.c: drop useless static qualifier
soc/fsl/qe: qe.c: reduce static memory footprint by 1.7K
soc/fsl/qe: qe.c: introduce qe_get_device_node helper
dt-bindings: soc/fsl: qe: document new fsl,qe-snums binding
soc/fsl/qe: qe.c: support fsl,qe-snums property
soc/fsl/qe: qe.c: fold qe_get_num_of_snums into qe_snums_init
.../devicetree/bindings/soc/fsl/cpm_qe/qe.txt | 13 +-
drivers/soc/fsl/qe/qe.c | 163 +++++++-----------
2 files changed, 77 insertions(+), 99 deletions(-)
--
2.20.1
^ permalink raw reply
* [PATCH v3 2/6] soc/fsl/qe: qe.c: reduce static memory footprint by 1.7K
From: Rasmus Villemoes @ 2019-05-13 11:14 UTC (permalink / raw)
To: devicetree@vger.kernel.org, Qiang Zhao, Li Yang
Cc: Mark Rutland, linux-kernel@vger.kernel.org, Scott Wood,
Rasmus Villemoes, Rob Herring, linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190513111442.25724-1-rasmus.villemoes@prevas.dk>
The current array of struct qe_snum use 256*4 bytes for just keeping
track of the free/used state of each index, and the struct layout
means there's another 768 bytes of padding. If we just unzip that
structure, the array of snum values just use 256 bytes, while the
free/inuse state can be tracked in a 32 byte bitmap.
So this reduces the .data footprint by 1760 bytes. It also serves as
preparation for introducing another DT binding for specifying the snum
values.
Reviewed-by: Christophe Leroy <christophe.leroy@c-s.fr>
Reviewed-by: Qiang Zhao <qiang.zhao@nxp.com>
Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
drivers/soc/fsl/qe/qe.c | 42 ++++++++++++-----------------------------
1 file changed, 12 insertions(+), 30 deletions(-)
diff --git a/drivers/soc/fsl/qe/qe.c b/drivers/soc/fsl/qe/qe.c
index 855373deb746..4b59109df22b 100644
--- a/drivers/soc/fsl/qe/qe.c
+++ b/drivers/soc/fsl/qe/qe.c
@@ -14,6 +14,7 @@
* Free Software Foundation; either version 2 of the License, or (at your
* option) any later version.
*/
+#include <linux/bitmap.h>
#include <linux/errno.h>
#include <linux/sched.h>
#include <linux/kernel.h>
@@ -43,25 +44,14 @@ static DEFINE_SPINLOCK(qe_lock);
DEFINE_SPINLOCK(cmxgcr_lock);
EXPORT_SYMBOL(cmxgcr_lock);
-/* QE snum state */
-enum qe_snum_state {
- QE_SNUM_STATE_USED,
- QE_SNUM_STATE_FREE
-};
-
-/* QE snum */
-struct qe_snum {
- u8 num;
- enum qe_snum_state state;
-};
-
/* We allocate this here because it is used almost exclusively for
* the communication processor devices.
*/
struct qe_immap __iomem *qe_immr;
EXPORT_SYMBOL(qe_immr);
-static struct qe_snum snums[QE_NUM_OF_SNUM]; /* Dynamically allocated SNUMs */
+static u8 snums[QE_NUM_OF_SNUM]; /* Dynamically allocated SNUMs */
+static DECLARE_BITMAP(snum_state, QE_NUM_OF_SNUM);
static unsigned int qe_num_of_snum;
static phys_addr_t qebase = -1;
@@ -315,10 +305,8 @@ static void qe_snums_init(void)
else
snum_init = snum_init_46;
- for (i = 0; i < qe_num_of_snum; i++) {
- snums[i].num = snum_init[i];
- snums[i].state = QE_SNUM_STATE_FREE;
- }
+ bitmap_zero(snum_state, QE_NUM_OF_SNUM);
+ memcpy(snums, snum_init, qe_num_of_snum);
}
int qe_get_snum(void)
@@ -328,12 +316,10 @@ int qe_get_snum(void)
int i;
spin_lock_irqsave(&qe_lock, flags);
- for (i = 0; i < qe_num_of_snum; i++) {
- if (snums[i].state == QE_SNUM_STATE_FREE) {
- snums[i].state = QE_SNUM_STATE_USED;
- snum = snums[i].num;
- break;
- }
+ i = find_first_zero_bit(snum_state, qe_num_of_snum);
+ if (i < qe_num_of_snum) {
+ set_bit(i, snum_state);
+ snum = snums[i];
}
spin_unlock_irqrestore(&qe_lock, flags);
@@ -343,14 +329,10 @@ EXPORT_SYMBOL(qe_get_snum);
void qe_put_snum(u8 snum)
{
- int i;
+ const u8 *p = memchr(snums, snum, qe_num_of_snum);
- for (i = 0; i < qe_num_of_snum; i++) {
- if (snums[i].num == snum) {
- snums[i].state = QE_SNUM_STATE_FREE;
- break;
- }
- }
+ if (p)
+ clear_bit(p - snums, snum_state);
}
EXPORT_SYMBOL(qe_put_snum);
--
2.20.1
^ permalink raw reply related
* [PATCH v3 3/6] soc/fsl/qe: qe.c: introduce qe_get_device_node helper
From: Rasmus Villemoes @ 2019-05-13 11:14 UTC (permalink / raw)
To: devicetree@vger.kernel.org, Qiang Zhao, Li Yang
Cc: Mark Rutland, linux-kernel@vger.kernel.org, Scott Wood,
Rasmus Villemoes, Rob Herring, linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190513111442.25724-1-rasmus.villemoes@prevas.dk>
The 'try of_find_compatible_node(NULL, NULL, "fsl,qe"), fall back to
of_find_node_by_type(NULL, "qe")' pattern is repeated five
times. Factor it into a common helper.
Reviewed-by: Christophe Leroy <christophe.leroy@c-s.fr>
Reviewed-by: Qiang Zhao <qiang.zhao@nxp.com>
Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
drivers/soc/fsl/qe/qe.c | 71 +++++++++++++++++------------------------
1 file changed, 29 insertions(+), 42 deletions(-)
diff --git a/drivers/soc/fsl/qe/qe.c b/drivers/soc/fsl/qe/qe.c
index 4b59109df22b..4b444846d590 100644
--- a/drivers/soc/fsl/qe/qe.c
+++ b/drivers/soc/fsl/qe/qe.c
@@ -56,6 +56,20 @@ static unsigned int qe_num_of_snum;
static phys_addr_t qebase = -1;
+static struct device_node *qe_get_device_node(void)
+{
+ struct device_node *qe;
+
+ /*
+ * Newer device trees have an "fsl,qe" compatible property for the QE
+ * node, but we still need to support older device trees.
+ */
+ qe = of_find_compatible_node(NULL, NULL, "fsl,qe");
+ if (qe)
+ return qe;
+ return of_find_node_by_type(NULL, "qe");
+}
+
static phys_addr_t get_qe_base(void)
{
struct device_node *qe;
@@ -65,12 +79,9 @@ static phys_addr_t get_qe_base(void)
if (qebase != -1)
return qebase;
- qe = of_find_compatible_node(NULL, NULL, "fsl,qe");
- if (!qe) {
- qe = of_find_node_by_type(NULL, "qe");
- if (!qe)
- return qebase;
- }
+ qe = qe_get_device_node();
+ if (!qe)
+ return qebase;
ret = of_address_to_resource(qe, 0, &res);
if (!ret)
@@ -164,12 +175,9 @@ unsigned int qe_get_brg_clk(void)
if (brg_clk)
return brg_clk;
- qe = of_find_compatible_node(NULL, NULL, "fsl,qe");
- if (!qe) {
- qe = of_find_node_by_type(NULL, "qe");
- if (!qe)
- return brg_clk;
- }
+ qe = qe_get_device_node();
+ if (!qe)
+ return brg_clk;
prop = of_get_property(qe, "brg-frequency", &size);
if (prop && size == sizeof(*prop))
@@ -558,16 +566,9 @@ struct qe_firmware_info *qe_get_firmware_info(void)
initialized = 1;
- /*
- * Newer device trees have an "fsl,qe" compatible property for the QE
- * node, but we still need to support older device trees.
- */
- qe = of_find_compatible_node(NULL, NULL, "fsl,qe");
- if (!qe) {
- qe = of_find_node_by_type(NULL, "qe");
- if (!qe)
- return NULL;
- }
+ qe = qe_get_device_node();
+ if (!qe)
+ return NULL;
/* Find the 'firmware' child node */
fw = of_get_child_by_name(qe, "firmware");
@@ -613,16 +614,9 @@ unsigned int qe_get_num_of_risc(void)
unsigned int num_of_risc = 0;
const u32 *prop;
- qe = of_find_compatible_node(NULL, NULL, "fsl,qe");
- if (!qe) {
- /* Older devices trees did not have an "fsl,qe"
- * compatible property, so we need to look for
- * the QE node by name.
- */
- qe = of_find_node_by_type(NULL, "qe");
- if (!qe)
- return num_of_risc;
- }
+ qe = qe_get_device_node();
+ if (!qe)
+ return num_of_risc;
prop = of_get_property(qe, "fsl,qe-num-riscs", &size);
if (prop && size == sizeof(*prop))
@@ -642,16 +636,9 @@ unsigned int qe_get_num_of_snums(void)
const u32 *prop;
num_of_snums = 28; /* The default number of snum for threads is 28 */
- qe = of_find_compatible_node(NULL, NULL, "fsl,qe");
- if (!qe) {
- /* Older devices trees did not have an "fsl,qe"
- * compatible property, so we need to look for
- * the QE node by name.
- */
- qe = of_find_node_by_type(NULL, "qe");
- if (!qe)
- return num_of_snums;
- }
+ qe = qe_get_device_node();
+ if (!qe)
+ return num_of_snums;
prop = of_get_property(qe, "fsl,qe-num-snums", &size);
if (prop && size == sizeof(*prop)) {
--
2.20.1
^ permalink raw reply related
* [PATCH v3 4/6] dt-bindings: soc/fsl: qe: document new fsl,qe-snums binding
From: Rasmus Villemoes @ 2019-05-13 11:14 UTC (permalink / raw)
To: devicetree@vger.kernel.org, Qiang Zhao, Li Yang
Cc: Mark Rutland, linux-kernel@vger.kernel.org, Scott Wood,
Rasmus Villemoes, Rob Herring, linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190513111442.25724-1-rasmus.villemoes@prevas.dk>
Reading table 4-30, and its footnotes, of the QUICC Engine Block
Reference Manual shows that the set of snum _values_ is not
necessarily just a function of the _number_ of snums, as given in the
fsl,qe-num-snums property.
As an alternative, to make it easier to add support for other variants
of the QUICC engine IP, this introduces a new binding fsl,qe-snums,
which automatically encodes both the number of snums and the actual
values to use.
Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
Rob, thanks for the review of v2. However, since I moved the example
from the commit log to the binding (per Joakim's request), I didn't
add a Reviewed-by tag for this revision.
.../devicetree/bindings/soc/fsl/cpm_qe/qe.txt | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/soc/fsl/cpm_qe/qe.txt b/Documentation/devicetree/bindings/soc/fsl/cpm_qe/qe.txt
index d7afaff5faff..05ec2a838c54 100644
--- a/Documentation/devicetree/bindings/soc/fsl/cpm_qe/qe.txt
+++ b/Documentation/devicetree/bindings/soc/fsl/cpm_qe/qe.txt
@@ -18,7 +18,8 @@ Required properties:
- reg : offset and length of the device registers.
- bus-frequency : the clock frequency for QUICC Engine.
- fsl,qe-num-riscs: define how many RISC engines the QE has.
-- fsl,qe-num-snums: define how many serial number(SNUM) the QE can use for the
+- fsl,qe-snums: This property has to be specified as '/bits/ 8' value,
+ defining the array of serial number (SNUM) values for the virtual
threads.
Optional properties:
@@ -34,6 +35,11 @@ Recommended properties
- brg-frequency : the internal clock source frequency for baud-rate
generators in Hz.
+Deprecated properties
+- fsl,qe-num-snums: define how many serial number(SNUM) the QE can use
+ for the threads. Use fsl,qe-snums instead to not only specify the
+ number of snums, but also their values.
+
Example:
qe@e0100000 {
#address-cells = <1>;
@@ -44,6 +50,11 @@ Example:
reg = <e0100000 480>;
brg-frequency = <0>;
bus-frequency = <179A7B00>;
+ fsl,qe-snums = /bits/ 8 <
+ 0x04 0x05 0x0C 0x0D 0x14 0x15 0x1C 0x1D
+ 0x24 0x25 0x2C 0x2D 0x34 0x35 0x88 0x89
+ 0x98 0x99 0xA8 0xA9 0xB8 0xB9 0xC8 0xC9
+ 0xD8 0xD9 0xE8 0xE9>;
}
* Multi-User RAM (MURAM)
--
2.20.1
^ permalink raw reply related
* [PATCH v3 5/6] soc/fsl/qe: qe.c: support fsl,qe-snums property
From: Rasmus Villemoes @ 2019-05-13 11:15 UTC (permalink / raw)
To: devicetree@vger.kernel.org, Qiang Zhao, Li Yang
Cc: Mark Rutland, linux-kernel@vger.kernel.org, Scott Wood,
Rasmus Villemoes, Rob Herring, linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190513111442.25724-1-rasmus.villemoes@prevas.dk>
Add driver support for the newly introduced fsl,qe-snums property.
Conveniently, of_property_read_variable_u8_array does exactly what we
need: If the property fsl,qe-snums is found (and has an allowed size),
the array of values get copied to snums, and the return value is the
number of snums - we cannot assign directly to num_of_snums, since we
need to check whether the return value is negative.
Reviewed-by: Christophe Leroy <christophe.leroy@c-s.fr>
Reviewed-by: Qiang Zhao <qiang.zhao@nxp.com>
Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
drivers/soc/fsl/qe/qe.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/soc/fsl/qe/qe.c b/drivers/soc/fsl/qe/qe.c
index 4b444846d590..1d27187b251c 100644
--- a/drivers/soc/fsl/qe/qe.c
+++ b/drivers/soc/fsl/qe/qe.c
@@ -283,7 +283,6 @@ EXPORT_SYMBOL(qe_clock_source);
*/
static void qe_snums_init(void)
{
- int i;
static const u8 snum_init_76[] = {
0x04, 0x05, 0x0C, 0x0D, 0x14, 0x15, 0x1C, 0x1D,
0x24, 0x25, 0x2C, 0x2D, 0x34, 0x35, 0x88, 0x89,
@@ -304,7 +303,21 @@ static void qe_snums_init(void)
0x28, 0x29, 0x38, 0x39, 0x48, 0x49, 0x58, 0x59,
0x68, 0x69, 0x78, 0x79, 0x80, 0x81,
};
+ struct device_node *qe;
const u8 *snum_init;
+ int i;
+
+ bitmap_zero(snum_state, QE_NUM_OF_SNUM);
+ qe = qe_get_device_node();
+ if (qe) {
+ i = of_property_read_variable_u8_array(qe, "fsl,qe-snums",
+ snums, 1, QE_NUM_OF_SNUM);
+ of_node_put(qe);
+ if (i > 0) {
+ qe_num_of_snum = i;
+ return;
+ }
+ }
qe_num_of_snum = qe_get_num_of_snums();
@@ -313,7 +326,6 @@ static void qe_snums_init(void)
else
snum_init = snum_init_46;
- bitmap_zero(snum_state, QE_NUM_OF_SNUM);
memcpy(snums, snum_init, qe_num_of_snum);
}
--
2.20.1
^ permalink raw reply related
* [PATCH v3 6/6] soc/fsl/qe: qe.c: fold qe_get_num_of_snums into qe_snums_init
From: Rasmus Villemoes @ 2019-05-13 11:15 UTC (permalink / raw)
To: devicetree@vger.kernel.org, Qiang Zhao, Li Yang
Cc: Mark Rutland, linux-kernel@vger.kernel.org, Scott Wood,
Rasmus Villemoes, Rob Herring, linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190513111442.25724-1-rasmus.villemoes@prevas.dk>
The comment "No QE ever has fewer than 28 SNUMs" is false; e.g. the
MPC8309 has 14. The code path returning -EINVAL is also a recipe for
instant disaster, since the caller (qe_snums_init) uncritically
assigns the return value to the unsigned qe_num_of_snum, and would
thus proceed to attempt to copy 4GB from snum_init_46[] to the snum[]
array.
So fold the handling of the legacy fsl,qe-num-snums into
qe_snums_init, and make sure we do not end up using the snum_init_46
array in cases other than the two where we know it makes sense.
Reviewed-by: Christophe Leroy <christophe.leroy@c-s.fr>
Reviewed-by: Qiang Zhao <qiang.zhao@nxp.com>
Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
drivers/soc/fsl/qe/qe.c | 46 ++++++++++++++---------------------------
1 file changed, 16 insertions(+), 30 deletions(-)
diff --git a/drivers/soc/fsl/qe/qe.c b/drivers/soc/fsl/qe/qe.c
index 1d27187b251c..852060caff24 100644
--- a/drivers/soc/fsl/qe/qe.c
+++ b/drivers/soc/fsl/qe/qe.c
@@ -308,24 +308,33 @@ static void qe_snums_init(void)
int i;
bitmap_zero(snum_state, QE_NUM_OF_SNUM);
+ qe_num_of_snum = 28; /* The default number of snum for threads is 28 */
qe = qe_get_device_node();
if (qe) {
i = of_property_read_variable_u8_array(qe, "fsl,qe-snums",
snums, 1, QE_NUM_OF_SNUM);
- of_node_put(qe);
if (i > 0) {
+ of_node_put(qe);
qe_num_of_snum = i;
return;
}
+ /*
+ * Fall back to legacy binding of using the value of
+ * fsl,qe-num-snums to choose one of the static arrays
+ * above.
+ */
+ of_property_read_u32(qe, "fsl,qe-num-snums", &qe_num_of_snum);
+ of_node_put(qe);
}
- qe_num_of_snum = qe_get_num_of_snums();
-
- if (qe_num_of_snum == 76)
+ if (qe_num_of_snum == 76) {
snum_init = snum_init_76;
- else
+ } else if (qe_num_of_snum == 28 || qe_num_of_snum == 46) {
snum_init = snum_init_46;
-
+ } else {
+ pr_err("QE: unsupported value of fsl,qe-num-snums: %u\n", qe_num_of_snum);
+ return;
+ }
memcpy(snums, snum_init, qe_num_of_snum);
}
@@ -642,30 +651,7 @@ EXPORT_SYMBOL(qe_get_num_of_risc);
unsigned int qe_get_num_of_snums(void)
{
- struct device_node *qe;
- int size;
- unsigned int num_of_snums;
- const u32 *prop;
-
- num_of_snums = 28; /* The default number of snum for threads is 28 */
- qe = qe_get_device_node();
- if (!qe)
- return num_of_snums;
-
- prop = of_get_property(qe, "fsl,qe-num-snums", &size);
- if (prop && size == sizeof(*prop)) {
- num_of_snums = *prop;
- if ((num_of_snums < 28) || (num_of_snums > QE_NUM_OF_SNUM)) {
- /* No QE ever has fewer than 28 SNUMs */
- pr_err("QE: number of snum is invalid\n");
- of_node_put(qe);
- return -EINVAL;
- }
- }
-
- of_node_put(qe);
-
- return num_of_snums;
+ return qe_num_of_snum;
}
EXPORT_SYMBOL(qe_get_num_of_snums);
--
2.20.1
^ permalink raw reply related
* [PATCH v3 1/6] soc/fsl/qe: qe.c: drop useless static qualifier
From: Rasmus Villemoes @ 2019-05-13 11:14 UTC (permalink / raw)
To: devicetree@vger.kernel.org, Qiang Zhao, Li Yang
Cc: Mark Rutland, linux-kernel@vger.kernel.org, Scott Wood,
Rasmus Villemoes, Rob Herring, linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190513111442.25724-1-rasmus.villemoes@prevas.dk>
The local variable snum_init has no reason to have static storage duration.
Reviewed-by: Christophe Leroy <christophe.leroy@c-s.fr>
Reviewed-by: Qiang Zhao <qiang.zhao@nxp.com>
Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
drivers/soc/fsl/qe/qe.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/soc/fsl/qe/qe.c b/drivers/soc/fsl/qe/qe.c
index 612d9c551be5..855373deb746 100644
--- a/drivers/soc/fsl/qe/qe.c
+++ b/drivers/soc/fsl/qe/qe.c
@@ -306,7 +306,7 @@ static void qe_snums_init(void)
0x28, 0x29, 0x38, 0x39, 0x48, 0x49, 0x58, 0x59,
0x68, 0x69, 0x78, 0x79, 0x80, 0x81,
};
- static const u8 *snum_init;
+ const u8 *snum_init;
qe_num_of_snum = qe_get_num_of_snums();
--
2.20.1
^ permalink raw reply related
* [PATCH] powerpc/boot: fix broken way to pass CONFIG options
From: Masahiro Yamada @ 2019-05-13 11:22 UTC (permalink / raw)
To: linuxppc-dev, Michael Ellerman
Cc: Rob Herring, Rodrigo R. Galvao, linux-kernel, Nicholas Piggin,
Mark Greer, Masahiro Yamada, Oliver O'Halloran, Joel Stanley,
Paul Mackerras
Commit 5e9dcb6188a4 ("powerpc/boot: Expose Kconfig symbols to wrapper")
was wrong, but commit e41b93a6be57 ("powerpc/boot: Fix build failures
with -j 1") was also wrong.
Check-in source files never ever depend on build artifacts.
The correct dependency is:
$(obj)/serial.o: $(obj)/autoconf.h
However, copying autoconf.h to arch/power/boot/ is questionable
in the first place.
arch/powerpc/Makefile adopted multiple ways to pass CONFIG options.
arch/powerpc/boot/decompress.c references CONFIG_KERNEL_GZIP and
CONFIG_KERNEL_XZ, which are passed via the command line.
arch/powerpc/boot/serial.c includes the copied autoconf.h to
reference a couple of CONFIG options.
Do not do this.
We should have already learned that including autoconf.h from each
source file is really fragile.
In fact, it is already broken.
arch/powerpc/boot/ppc_asm.h references CONFIG_PPC_8xx, but
arch/powerpc/boot/utils.S is not given any way to access CONFIG
options. So, CONFIG_PPC_8xx is never defined here.
Just pass $(LINUXINCLUDE) and remove all broken code.
I also removed the -traditional flag to make include/linux/kconfig.h
work. I do not understand why it needs to imitate the behavior of
pre-standard C preprocessors.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
arch/powerpc/boot/.gitignore | 2 --
arch/powerpc/boot/Makefile | 14 +++-----------
arch/powerpc/boot/serial.c | 1 -
3 files changed, 3 insertions(+), 14 deletions(-)
diff --git a/arch/powerpc/boot/.gitignore b/arch/powerpc/boot/.gitignore
index 32034a0cc554..6610665fcf5e 100644
--- a/arch/powerpc/boot/.gitignore
+++ b/arch/powerpc/boot/.gitignore
@@ -44,5 +44,3 @@ fdt_sw.c
fdt_wip.c
libfdt.h
libfdt_internal.h
-autoconf.h
-
diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
index 73d1f3562978..b8a82be2af2a 100644
--- a/arch/powerpc/boot/Makefile
+++ b/arch/powerpc/boot/Makefile
@@ -20,9 +20,6 @@
all: $(obj)/zImage
-compress-$(CONFIG_KERNEL_GZIP) := CONFIG_KERNEL_GZIP
-compress-$(CONFIG_KERNEL_XZ) := CONFIG_KERNEL_XZ
-
ifdef CROSS32_COMPILE
BOOTCC := $(CROSS32_COMPILE)gcc
BOOTAR := $(CROSS32_COMPILE)ar
@@ -34,7 +31,7 @@ endif
BOOTCFLAGS := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
-fno-strict-aliasing -O2 -msoft-float -mno-altivec -mno-vsx \
-pipe -fomit-frame-pointer -fno-builtin -fPIC -nostdinc \
- -D$(compress-y)
+ $(LINUXINCLUDE)
ifdef CONFIG_PPC64_BOOT_WRAPPER
BOOTCFLAGS += -m64
@@ -51,7 +48,7 @@ BOOTCFLAGS += -mlittle-endian
BOOTCFLAGS += $(call cc-option,-mabi=elfv2)
endif
-BOOTAFLAGS := -D__ASSEMBLY__ $(BOOTCFLAGS) -traditional -nostdinc
+BOOTAFLAGS := -D__ASSEMBLY__ $(BOOTCFLAGS) -nostdinc
BOOTARFLAGS := -cr$(KBUILD_ARFLAGS)
@@ -202,14 +199,9 @@ $(obj)/empty.c:
$(obj)/zImage.coff.lds $(obj)/zImage.ps3.lds : $(obj)/%: $(srctree)/$(src)/%.S
$(Q)cp $< $@
-$(srctree)/$(src)/serial.c: $(obj)/autoconf.h
-
-$(obj)/autoconf.h: $(obj)/%: $(objtree)/include/generated/%
- $(Q)cp $< $@
-
clean-files := $(zlib-) $(zlibheader-) $(zliblinuxheader-) \
$(zlib-decomp-) $(libfdt) $(libfdtheader) \
- autoconf.h empty.c zImage.coff.lds zImage.ps3.lds zImage.lds
+ empty.c zImage.coff.lds zImage.ps3.lds zImage.lds
quiet_cmd_bootcc = BOOTCC $@
cmd_bootcc = $(BOOTCC) -Wp,-MD,$(depfile) $(BOOTCFLAGS) -c -o $@ $<
diff --git a/arch/powerpc/boot/serial.c b/arch/powerpc/boot/serial.c
index b0491b8c0199..9457863147f9 100644
--- a/arch/powerpc/boot/serial.c
+++ b/arch/powerpc/boot/serial.c
@@ -18,7 +18,6 @@
#include "stdio.h"
#include "io.h"
#include "ops.h"
-#include "autoconf.h"
static int serial_open(void)
{
--
2.17.1
^ permalink raw reply related
* Re: [PATCH, RFC] byteorder: sanity check toolchain vs kernel endianess
From: Michael Ellerman @ 2019-05-13 11:33 UTC (permalink / raw)
To: Dmitry Vyukov, Arnd Bergmann
Cc: linux-arch, linuxppc-dev, Linux Kernel Mailing List,
Nick Kossifidis, Andrew Morton, Linus Torvalds, Christoph Hellwig
In-Reply-To: <CACT4Y+ad5z6z0Dweh5hGwYcUUebPEtqsznmX9enPvYB20J16aA@mail.gmail.com>
Dmitry Vyukov <dvyukov@google.com> writes:
> From: Arnd Bergmann <arnd@arndb.de>
> Date: Sat, May 11, 2019 at 2:51 AM
> To: Dmitry Vyukov
> Cc: Nick Kossifidis, Christoph Hellwig, Linus Torvalds, Andrew Morton,
> linux-arch, Linux Kernel Mailing List, linuxppc-dev
>
>> On Fri, May 10, 2019 at 6:53 AM Dmitry Vyukov <dvyukov@google.com> wrote:
>> > >
>> > > I think it's good to have a sanity check in-place for consistency.
>> >
>> >
>> > Hi,
>> >
>> > This broke our cross-builds from x86. I am using:
>> >
>> > $ powerpc64le-linux-gnu-gcc --version
>> > powerpc64le-linux-gnu-gcc (Debian 7.2.0-7) 7.2.0
>> >
>> > and it says that it's little-endian somehow:
>> >
>> > $ powerpc64le-linux-gnu-gcc -dM -E - < /dev/null | grep BYTE_ORDER
>> > #define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__
>> >
>> > Is it broke compiler? Or I always hold it wrong? Is there some
>> > additional flag I need to add?
>>
>> It looks like a bug in the kernel Makefiles to me. powerpc32 is always
>> big-endian,
>> powerpc64 used to be big-endian but is now usually little-endian. There are
>> often three separate toolchains that default to the respective user
>> space targets
>> (ppc32be, ppc64be, ppc64le), but generally you should be able to build
>> any of the
>> three kernel configurations with any of those compilers, and have the Makefile
>> pass the correct -m32/-m64/-mbig-endian/-mlittle-endian command line options
>> depending on the kernel configuration. It seems that this is not happening
>> here. I have not checked why, but if this is the problem, it should be
>> easy enough
>> to figure out.
>
>
> Thanks! This clears a lot.
> This may be a bug in our magic as we try to build kernel files outside
> of make with own flags (required to extract parts of kernel
> interfaces).
> So don't spend time looking for the Makefile bugs yet.
OK :)
We did have some bugs in the past (~1-2 y/ago) but AFAIK they are all
fixed now. These days I build most of my kernels with a bi-endian 64-bit
toolchain, and switching endian without running `make clean` also works.
cheers
^ permalink raw reply
* Re: [RFC PATCH] powerpc/64/ftrace: mprofile-kernel patch out mflr
From: Michael Ellerman @ 2019-05-13 11:34 UTC (permalink / raw)
To: Naveen N. Rao, linuxppc-dev, Nicholas Piggin
In-Reply-To: <1557729790.fw18xf9mdt.naveen@linux.ibm.com>
"Naveen N. Rao" <naveen.n.rao@linux.ibm.com> writes:
> Michael Ellerman wrote:
>> Nicholas Piggin <npiggin@gmail.com> writes:
>>> The new mprofile-kernel mcount sequence is
>>>
>>> mflr r0
>>> bl _mcount
>>>
>>> Dynamic ftrace patches the branch instruction with a noop, but leaves
>>> the mflr. mflr is executed by the branch unit that can only execute one
>>> per cycle on POWER9 and shared with branches, so it would be nice to
>>> avoid it where possible.
>>>
>>> This patch is a hacky proof of concept to nop out the mflr. Can we do
>>> this or are there races or other issues with it?
>>
>> There's a race, isn't there?
>>
>> We have a function foo which currently has tracing disabled, so the mflr
>> and bl are nop'ed out.
>>
>> CPU 0 CPU 1
>> ==================================
>> bl foo
>> nop (ie. not mflr)
>> -> interrupt
>> something else enable tracing for foo
>> ... patch mflr and branch
>> <- rfi
>> bl _mcount
>>
>> So we end up in _mcount() but with r0 not populated.
>
> Good catch! Looks like we need to patch the mflr with a "b +8" similar
> to what we do in __ftrace_make_nop().
Would that actually make it any faster though? Nick?
cheers
^ permalink raw reply
* Re: [PATCH v3 4/6] dt-bindings: soc/fsl: qe: document new fsl,qe-snums binding
From: Joakim Tjernlund @ 2019-05-13 11:39 UTC (permalink / raw)
To: rasmus.villemoes@prevas.dk, leoyang.li@nxp.com,
qiang.zhao@nxp.com, devicetree@vger.kernel.org
Cc: mark.rutland@arm.com, oss@buserror.net,
linux-kernel@vger.kernel.org, Rasmus.Villemoes@prevas.se,
robh+dt@kernel.org, linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190513111442.25724-5-rasmus.villemoes@prevas.dk>
On Mon, 2019-05-13 at 11:14 +0000, Rasmus Villemoes wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
>
> Reading table 4-30, and its footnotes, of the QUICC Engine Block
> Reference Manual shows that the set of snum _values_ is not
> necessarily just a function of the _number_ of snums, as given in the
> fsl,qe-num-snums property.
>
> As an alternative, to make it easier to add support for other variants
> of the QUICC engine IP, this introduces a new binding fsl,qe-snums,
> which automatically encodes both the number of snums and the actual
> values to use.
>
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> ---
> Rob, thanks for the review of v2. However, since I moved the example
> from the commit log to the binding (per Joakim's request), I didn't
Thanks, looks good now.
> add a Reviewed-by tag for this revision.
>
> .../devicetree/bindings/soc/fsl/cpm_qe/qe.txt | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/soc/fsl/cpm_qe/qe.txt b/Documentation/devicetree/bindings/soc/fsl/cpm_qe/qe.txt
> index d7afaff5faff..05ec2a838c54 100644
> --- a/Documentation/devicetree/bindings/soc/fsl/cpm_qe/qe.txt
> +++ b/Documentation/devicetree/bindings/soc/fsl/cpm_qe/qe.txt
> @@ -18,7 +18,8 @@ Required properties:
> - reg : offset and length of the device registers.
> - bus-frequency : the clock frequency for QUICC Engine.
> - fsl,qe-num-riscs: define how many RISC engines the QE has.
> -- fsl,qe-num-snums: define how many serial number(SNUM) the QE can use for the
> +- fsl,qe-snums: This property has to be specified as '/bits/ 8' value,
> + defining the array of serial number (SNUM) values for the virtual
> threads.
>
> Optional properties:
> @@ -34,6 +35,11 @@ Recommended properties
> - brg-frequency : the internal clock source frequency for baud-rate
> generators in Hz.
>
> +Deprecated properties
> +- fsl,qe-num-snums: define how many serial number(SNUM) the QE can use
> + for the threads. Use fsl,qe-snums instead to not only specify the
> + number of snums, but also their values.
> +
> Example:
> qe@e0100000 {
> #address-cells = <1>;
> @@ -44,6 +50,11 @@ Example:
> reg = <e0100000 480>;
> brg-frequency = <0>;
> bus-frequency = <179A7B00>;
> + fsl,qe-snums = /bits/ 8 <
> + 0x04 0x05 0x0C 0x0D 0x14 0x15 0x1C 0x1D
> + 0x24 0x25 0x2C 0x2D 0x34 0x35 0x88 0x89
> + 0x98 0x99 0xA8 0xA9 0xB8 0xB9 0xC8 0xC9
> + 0xD8 0xD9 0xE8 0xE9>;
> }
>
> * Multi-User RAM (MURAM)
> --
> 2.20.1
>
^ permalink raw reply
* Re: [PATCH] crypto: vmx - fix copy-paste error in CTR mode
From: Michael Ellerman @ 2019-05-13 11:39 UTC (permalink / raw)
To: Herbert Xu, Eric Biggers
Cc: leo.barbosa, Nayna, Stephan Mueller, nayna, omosnacek,
marcelo.cerri, pfsmorigo, linux-crypto, leitao, George Wilson,
linuxppc-dev, Daniel Axtens
In-Reply-To: <20190513005901.tsop4lz26vusr6o4@gondor.apana.org.au>
Herbert Xu <herbert@gondor.apana.org.au> writes:
> On Mon, May 06, 2019 at 08:53:17AM -0700, Eric Biggers wrote:
>>
>> Any progress on this? Someone just reported this again here:
>> https://bugzilla.kernel.org/show_bug.cgi?id=203515
>
> Guys if I don't get a fix for this soon I'll have to disable CTR
> in vmx.
No objection from me.
I'll try and debug it at some point if no one else does, but I can't
make it my top priority sorry.
cheers
^ permalink raw reply
* Re: [PATCH, RFC] byteorder: sanity check toolchain vs kernel endianess
From: Dmitry Vyukov @ 2019-05-13 11:50 UTC (permalink / raw)
To: Michael Ellerman
Cc: linux-arch, Arnd Bergmann, linuxppc-dev,
Linux Kernel Mailing List, Andrew Donnellan, Nick Kossifidis,
Andrew Morton, Linus Torvalds, Christoph Hellwig
In-Reply-To: <87woiutwq4.fsf@concordia.ellerman.id.au>
From: Michael Ellerman <mpe@ellerman.id.au>
Date: Mon, May 13, 2019 at 1:33 PM
To: Dmitry Vyukov, Arnd Bergmann
Cc: Nick Kossifidis, Christoph Hellwig, Linus Torvalds, Andrew Morton,
linux-arch, Linux Kernel Mailing List, linuxppc-dev
> Dmitry Vyukov <dvyukov@google.com> writes:
> > From: Arnd Bergmann <arnd@arndb.de>
> > Date: Sat, May 11, 2019 at 2:51 AM
> > To: Dmitry Vyukov
> > Cc: Nick Kossifidis, Christoph Hellwig, Linus Torvalds, Andrew Morton,
> > linux-arch, Linux Kernel Mailing List, linuxppc-dev
> >
> >> On Fri, May 10, 2019 at 6:53 AM Dmitry Vyukov <dvyukov@google.com> wrote:
> >> > >
> >> > > I think it's good to have a sanity check in-place for consistency.
> >> >
> >> >
> >> > Hi,
> >> >
> >> > This broke our cross-builds from x86. I am using:
> >> >
> >> > $ powerpc64le-linux-gnu-gcc --version
> >> > powerpc64le-linux-gnu-gcc (Debian 7.2.0-7) 7.2.0
> >> >
> >> > and it says that it's little-endian somehow:
> >> >
> >> > $ powerpc64le-linux-gnu-gcc -dM -E - < /dev/null | grep BYTE_ORDER
> >> > #define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__
> >> >
> >> > Is it broke compiler? Or I always hold it wrong? Is there some
> >> > additional flag I need to add?
> >>
> >> It looks like a bug in the kernel Makefiles to me. powerpc32 is always
> >> big-endian,
> >> powerpc64 used to be big-endian but is now usually little-endian. There are
> >> often three separate toolchains that default to the respective user
> >> space targets
> >> (ppc32be, ppc64be, ppc64le), but generally you should be able to build
> >> any of the
> >> three kernel configurations with any of those compilers, and have the Makefile
> >> pass the correct -m32/-m64/-mbig-endian/-mlittle-endian command line options
> >> depending on the kernel configuration. It seems that this is not happening
> >> here. I have not checked why, but if this is the problem, it should be
> >> easy enough
> >> to figure out.
> >
> >
> > Thanks! This clears a lot.
> > This may be a bug in our magic as we try to build kernel files outside
> > of make with own flags (required to extract parts of kernel
> > interfaces).
> > So don't spend time looking for the Makefile bugs yet.
>
> OK :)
>
> We did have some bugs in the past (~1-2 y/ago) but AFAIK they are all
> fixed now. These days I build most of my kernels with a bi-endian 64-bit
> toolchain, and switching endian without running `make clean` also works.
For the record, yes, it turn out to be a problem in our code (a latent
bug). We actually used host (x86) gcc to build as-if ppc code that can
run on the host, so it defined neither LE no BE macros. It just
happened to work in the past :)
+Andrew
^ permalink raw reply
* Re: [PATCH RESEND] powerpc: add simd.h implementation specific to PowerPC
From: Michael Ellerman @ 2019-05-13 11:53 UTC (permalink / raw)
To: Shawn Landden, linuxppc-dev; +Cc: Shawn Landden, Paul Mackerras, linux-kernel
In-Reply-To: <20190513005104.20140-1-shawn@git.icu>
Shawn Landden <shawn@git.icu> writes:
> It is safe to do SIMD in an interrupt on PowerPC.
No it's not sorry :)
> Only disable when there is no SIMD available
> (and this is a static branch).
>
> Tested and works with the WireGuard (Zinc) patch I wrote that needs this.
> Also improves performance of the crypto subsystem that checks this.
>
> Re-sending because this linuxppc-dev didn't seem to accept it.
It did but you were probably moderated as a non-subscriber? In future if
you just wait a while for the moderators to wake up it should come
through. Though having posted once and been approved I think you might
not get moderated at all in future (?).
> Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=203571
> Signed-off-by: Shawn Landden <shawn@git.icu>
> ---
> arch/powerpc/include/asm/simd.h | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
> create mode 100644 arch/powerpc/include/asm/simd.h
>
> diff --git a/arch/powerpc/include/asm/simd.h b/arch/powerpc/include/asm/simd.h
> new file mode 100644
> index 000000000..b3fecb95a
> --- /dev/null
> +++ b/arch/powerpc/include/asm/simd.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +
> +#include <asm/cpu_has_feature.h>
> +
> +/*
> + * may_use_simd - whether it is allowable at this time to issue SIMD
> + * instructions or access the SIMD register file
> + *
> + * As documented in Chapter 6.2.1 Machine Status Save/Restore Registers
> + * of Power ISA (2.07 and 3.0), all registers are saved/restored in an interrupt.
I think the confusion here is that the ISA says:
When various interrupts occur, the state of the machine is saved in the
Machine Status Save/Restore registers (SRR0 and SRR1).
And you've read that to mean all "the state" of the machine, ie.
including GPRs, FPRs etc.
But unfortunately it's not that simple. All the hardware does is write
two 64-bit registers (SRR0 & SRR1) with the information required to be
able to return to the state the CPU was in prior to the interrupt. That
includes (obviously) the PC you were executing at, and what state the
CPU was in (ie. 64/32-bit, MMU on/off, FP on/off etc.). All the saving
of registers etc. is left up to software. It's the RISC way :)
I guess we need to work out why exactly may_use_simd() is returning
false in your kworker.
cheers
^ permalink raw reply
* Re: [PATCH, RFC] byteorder: sanity check toolchain vs kernel endianess
From: Christoph Hellwig @ 2019-05-13 12:04 UTC (permalink / raw)
To: Dmitry Vyukov
Cc: linux-arch, Arnd Bergmann, linuxppc-dev,
Linux Kernel Mailing List, Andrew Donnellan, Nick Kossifidis,
Andrew Morton, Linus Torvalds, Christoph Hellwig
In-Reply-To: <CACT4Y+YT52wGuARxe9RqUsMYGNZTwaBowWWUUawyqTBq4G1NDg@mail.gmail.com>
On Mon, May 13, 2019 at 01:50:19PM +0200, Dmitry Vyukov wrote:
> > We did have some bugs in the past (~1-2 y/ago) but AFAIK they are all
> > fixed now. These days I build most of my kernels with a bi-endian 64-bit
> > toolchain, and switching endian without running `make clean` also works.
>
> For the record, yes, it turn out to be a problem in our code (a latent
> bug). We actually used host (x86) gcc to build as-if ppc code that can
> run on the host, so it defined neither LE no BE macros. It just
> happened to work in the past :)
So Nick was right and these checks actually are useful..
^ permalink raw reply
* Re: [PATCH] vsprintf: Do not break early boot with probing addresses
From: Petr Mladek @ 2019-05-13 12:24 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-arch, linux-s390, Sergey Senozhatsky, Heiko Carstens,
Stephen Rothwell, linuxppc-dev, Rasmus Villemoes, linux-kernel,
Michal Hocko, Sergey Senozhatsky, Martin Schwidefsky,
Andy Shevchenko, Linus Torvalds, Tobin C . Harding
In-Reply-To: <20190510124058.0d44b441@gandalf.local.home>
On Fri 2019-05-10 12:40:58, Steven Rostedt wrote:
> On Fri, 10 May 2019 18:32:58 +0200
> Martin Schwidefsky <schwidefsky@de.ibm.com> wrote:
>
> > On Fri, 10 May 2019 12:24:01 -0400
> > Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> > > On Fri, 10 May 2019 10:42:13 +0200
> > > Petr Mladek <pmladek@suse.com> wrote:
> > >
> > > > static const char *check_pointer_msg(const void *ptr)
> > > > {
> > > > - char byte;
> > > > -
> > > > if (!ptr)
> > > > return "(null)";
> > > >
> > > > - if (probe_kernel_address(ptr, byte))
> > > > + if ((unsigned long)ptr < PAGE_SIZE || IS_ERR_VALUE(ptr))
> > > > return "(efault)";
> > > >
> > >
> > >
> > > < PAGE_SIZE ?
> > >
> > > do you mean: < TASK_SIZE ?
> >
> > The check with < TASK_SIZE would break on s390. The 'ptr' is
> > in the kernel address space, *not* in the user address space.
> > Remember s390 has two separate address spaces for kernel/user
> > the check < TASK_SIZE only makes sense with a __user pointer.
> >
>
> So we allow this to read user addresses? Can't that cause a fault?
I did some quick check and did not found anywhere a user pointer
being dereferenced via vsprintf().
In each case, %s did the check (ptr < PAGE_SIZE) even before this
patchset. The other checks are in %p format modifiers that are
used to print various kernel structures.
Finally, it accesses the pointer directly. I am not completely sure
but I think that it would not work that easily with an address
from the user address space.
Best Regards,
Petr
^ permalink raw reply
* [Bug 203571] Allow use of SIMD in interrupts on PowerPC
From: bugzilla-daemon @ 2019-05-13 12:39 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-203571-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=203571
--- Comment #2 from Shawn Landden (slandden@gmail.com) ---
X86 manages to allow SIMD in interrupts through some very careful code in
arch/x86/kernel/fpu/core.c (starting with irq_fpu_usable())
PowerPC can do the same.
--
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* Re: [PATCH] vsprintf: Do not break early boot with probing addresses
From: Petr Mladek @ 2019-05-13 12:42 UTC (permalink / raw)
To: Andy Shevchenko
Cc: linux-arch@vger.kernel.org, Sergey Senozhatsky, Heiko Carstens,
linux-s390@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
Rasmus Villemoes, linux-kernel@vger.kernel.org, Steven Rostedt,
Michal Hocko, Sergey Senozhatsky, David Laight, Stephen Rothwell,
Linus Torvalds, Martin Schwidefsky, Tobin C . Harding
In-Reply-To: <20190513091320.GK9224@smile.fi.intel.com>
On Mon 2019-05-13 12:13:20, Andy Shevchenko wrote:
> On Mon, May 13, 2019 at 08:52:41AM +0000, David Laight wrote:
> > From: christophe leroy
> > > Sent: 10 May 2019 18:35
> > > Le 10/05/2019 à 18:24, Steven Rostedt a écrit :
> > > > On Fri, 10 May 2019 10:42:13 +0200
> > > > Petr Mladek <pmladek@suse.com> wrote:
>
> > > >> - if (probe_kernel_address(ptr, byte))
> > > >> + if ((unsigned long)ptr < PAGE_SIZE || IS_ERR_VALUE(ptr))
> > > >> return "(efault)";
> >
> > "efault" looks a bit like a spellling mistake for "default".
>
> It's a special, thus it's in parenthesis, though somebody can be
> misguided.
>
> > > Usually, < PAGE_SIZE means NULL pointer dereference (via the member of a
> > > struct)
> >
> > Maybe the caller should pass in a short buffer so that you can return
> > "(err-%d)"
> > or "(null+%#x)" ?
There are many vsprintf() users. I am afraid that nobody would want
to define extra buffers for error messages. It must fit into
the one for the formatted string.
> In both cases it should be limited to the size of pointer (8 or 16
> characters). Something like "(e:%4d)" would work for error codes.
I am afraid that we could get 10 different proposals from a huge
enough group of people. I wanted to start with something simple
(source code). I know that (efault) might be confused with
"default" but it is still just one string to grep.
> The "(null)" is good enough by itself and already an established
> practice..
(efault) made more sense with the probe_kernel_read() that
checked wide range of addresses. Well, I still think that
it makes sense to distinguish a pure NULL. And it still
used also for IS_ERR_VALUE().
Best Regards,
Petr
^ permalink raw reply
* Re: [PATCH] powerpc/boot: fix broken way to pass CONFIG options
From: Masahiro Yamada @ 2019-05-13 13:55 UTC (permalink / raw)
To: linuxppc-dev, Michael Ellerman
Cc: Rob Herring, Rodrigo R. Galvao, Linux Kernel Mailing List,
Nicholas Piggin, Mark Greer, Oliver O'Halloran, Joel Stanley,
Paul Mackerras
In-Reply-To: <20190513112254.22534-1-yamada.masahiro@socionext.com>
On Mon, May 13, 2019 at 9:33 PM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>
> Commit 5e9dcb6188a4 ("powerpc/boot: Expose Kconfig symbols to wrapper")
> was wrong, but commit e41b93a6be57 ("powerpc/boot: Fix build failures
> with -j 1") was also wrong.
>
> Check-in source files never ever depend on build artifacts.
>
> The correct dependency is:
>
> $(obj)/serial.o: $(obj)/autoconf.h
>
> However, copying autoconf.h to arch/power/boot/ is questionable
> in the first place.
>
> arch/powerpc/Makefile adopted multiple ways to pass CONFIG options.
>
> arch/powerpc/boot/decompress.c references CONFIG_KERNEL_GZIP and
> CONFIG_KERNEL_XZ, which are passed via the command line.
>
> arch/powerpc/boot/serial.c includes the copied autoconf.h to
> reference a couple of CONFIG options.
>
> Do not do this.
>
> We should have already learned that including autoconf.h from each
> source file is really fragile.
>
> In fact, it is already broken.
>
> arch/powerpc/boot/ppc_asm.h references CONFIG_PPC_8xx, but
> arch/powerpc/boot/utils.S is not given any way to access CONFIG
> options. So, CONFIG_PPC_8xx is never defined here.
>
> Just pass $(LINUXINCLUDE) and remove all broken code.
>
> I also removed the -traditional flag to make include/linux/kconfig.h
> work. I do not understand why it needs to imitate the behavior of
> pre-standard C preprocessors.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
I re-read my commit log, and I thought it was needlessly
too offensive. Sorry about that.
I will reword the commit log and send v2.
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [PATCH] powerpc/boot: fix broken way to pass CONFIG options
From: Oliver @ 2019-05-13 14:02 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Rob Herring, Rodrigo R. Galvao, Linux Kernel Mailing List,
Nicholas Piggin, Mark Greer, Paul Mackerras, Joel Stanley,
linuxppc-dev
In-Reply-To: <20190513112254.22534-1-yamada.masahiro@socionext.com>
On Mon, May 13, 2019 at 9:23 PM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>
> Commit 5e9dcb6188a4 ("powerpc/boot: Expose Kconfig symbols to wrapper")
> was wrong, but commit e41b93a6be57 ("powerpc/boot: Fix build failures
> with -j 1") was also wrong.
>
> Check-in source files never ever depend on build artifacts.
>
> The correct dependency is:
>
> $(obj)/serial.o: $(obj)/autoconf.h
>
> However, copying autoconf.h to arch/power/boot/ is questionable
> in the first place.
>
> arch/powerpc/Makefile adopted multiple ways to pass CONFIG options.
>
> arch/powerpc/boot/decompress.c references CONFIG_KERNEL_GZIP and
> CONFIG_KERNEL_XZ, which are passed via the command line.
>
> arch/powerpc/boot/serial.c includes the copied autoconf.h to
> reference a couple of CONFIG options.
>
> Do not do this.
>
> We should have already learned that including autoconf.h from each
> source file is really fragile.
>
> In fact, it is already broken.
>
> arch/powerpc/boot/ppc_asm.h references CONFIG_PPC_8xx, but
> arch/powerpc/boot/utils.S is not given any way to access CONFIG
> options. So, CONFIG_PPC_8xx is never defined here.
>
> Just pass $(LINUXINCLUDE) and remove all broken code.
I'm not sure how safe this is. The original reason for the
CONFIG_KERNEL_XZ hack in the makefile was because the kernel headers
couldn't be included directly. The bootwrapper is compiled with a
32bit toolchain when the kernel is compiled for 64bit big endian
because of older systems with broken firmware that can't load 64bit
ELFs directly. When I added XZ support to the wrapper I did experiment
with including the kernel headers directly and couldn't make it work
reliably. I don't remember what the exact reason was, but I think it
was something to do with the generated headers not always matching
what you would expect when compiling for 32bit. It's also possible I
was just being paranoid. Either way it's about time we found a real
fix...
The stuff in serial.c and ppc_asm.h was added later to work around
other issues without anyone thinking too hard about it. Oh well.
> I also removed the -traditional flag to make include/linux/kconfig.h
> work. I do not understand why it needs to imitate the behavior of
> pre-standard C preprocessors.
I'm not sure why it's there either. The boot wrapper was re-written at
some point so it might just be a hold over from the dawn of time.
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
> arch/powerpc/boot/.gitignore | 2 --
> arch/powerpc/boot/Makefile | 14 +++-----------
> arch/powerpc/boot/serial.c | 1 -
> 3 files changed, 3 insertions(+), 14 deletions(-)
>
> diff --git a/arch/powerpc/boot/.gitignore b/arch/powerpc/boot/.gitignore
> index 32034a0cc554..6610665fcf5e 100644
> --- a/arch/powerpc/boot/.gitignore
> +++ b/arch/powerpc/boot/.gitignore
> @@ -44,5 +44,3 @@ fdt_sw.c
> fdt_wip.c
> libfdt.h
> libfdt_internal.h
> -autoconf.h
> -
> diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
> index 73d1f3562978..b8a82be2af2a 100644
> --- a/arch/powerpc/boot/Makefile
> +++ b/arch/powerpc/boot/Makefile
> @@ -20,9 +20,6 @@
>
> all: $(obj)/zImage
>
> -compress-$(CONFIG_KERNEL_GZIP) := CONFIG_KERNEL_GZIP
> -compress-$(CONFIG_KERNEL_XZ) := CONFIG_KERNEL_XZ
> -
> ifdef CROSS32_COMPILE
> BOOTCC := $(CROSS32_COMPILE)gcc
> BOOTAR := $(CROSS32_COMPILE)ar
> @@ -34,7 +31,7 @@ endif
> BOOTCFLAGS := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
> -fno-strict-aliasing -O2 -msoft-float -mno-altivec -mno-vsx \
> -pipe -fomit-frame-pointer -fno-builtin -fPIC -nostdinc \
> - -D$(compress-y)
> + $(LINUXINCLUDE)
>
> ifdef CONFIG_PPC64_BOOT_WRAPPER
> BOOTCFLAGS += -m64
> @@ -51,7 +48,7 @@ BOOTCFLAGS += -mlittle-endian
> BOOTCFLAGS += $(call cc-option,-mabi=elfv2)
> endif
>
> -BOOTAFLAGS := -D__ASSEMBLY__ $(BOOTCFLAGS) -traditional -nostdinc
> +BOOTAFLAGS := -D__ASSEMBLY__ $(BOOTCFLAGS) -nostdinc
>
> BOOTARFLAGS := -cr$(KBUILD_ARFLAGS)
>
> @@ -202,14 +199,9 @@ $(obj)/empty.c:
> $(obj)/zImage.coff.lds $(obj)/zImage.ps3.lds : $(obj)/%: $(srctree)/$(src)/%.S
> $(Q)cp $< $@
>
> -$(srctree)/$(src)/serial.c: $(obj)/autoconf.h
> -
> -$(obj)/autoconf.h: $(obj)/%: $(objtree)/include/generated/%
> - $(Q)cp $< $@
> -
> clean-files := $(zlib-) $(zlibheader-) $(zliblinuxheader-) \
> $(zlib-decomp-) $(libfdt) $(libfdtheader) \
> - autoconf.h empty.c zImage.coff.lds zImage.ps3.lds zImage.lds
> + empty.c zImage.coff.lds zImage.ps3.lds zImage.lds
>
> quiet_cmd_bootcc = BOOTCC $@
> cmd_bootcc = $(BOOTCC) -Wp,-MD,$(depfile) $(BOOTCFLAGS) -c -o $@ $<
> diff --git a/arch/powerpc/boot/serial.c b/arch/powerpc/boot/serial.c
> index b0491b8c0199..9457863147f9 100644
> --- a/arch/powerpc/boot/serial.c
> +++ b/arch/powerpc/boot/serial.c
> @@ -18,7 +18,6 @@
> #include "stdio.h"
> #include "io.h"
> #include "ops.h"
> -#include "autoconf.h"
>
> static int serial_open(void)
> {
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH] powerpc/boot: fix broken way to pass CONFIG options
From: Oliver @ 2019-05-13 14:03 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Rob Herring, Rodrigo R. Galvao, Linux Kernel Mailing List,
Nicholas Piggin, Mark Greer, Paul Mackerras, Joel Stanley,
linuxppc-dev
In-Reply-To: <CAK7LNAQjukukcfz9-SyFhqZOZ1v18CJDVA8Zmn5RL92Q=73ZxA@mail.gmail.com>
On Mon, May 13, 2019 at 11:56 PM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>
> On Mon, May 13, 2019 at 9:33 PM Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
> >
> > Commit 5e9dcb6188a4 ("powerpc/boot: Expose Kconfig symbols to wrapper")
> > was wrong, but commit e41b93a6be57 ("powerpc/boot: Fix build failures
> > with -j 1") was also wrong.
> >
> > Check-in source files never ever depend on build artifacts.
> >
> > The correct dependency is:
> >
> > $(obj)/serial.o: $(obj)/autoconf.h
> >
> > However, copying autoconf.h to arch/power/boot/ is questionable
> > in the first place.
> >
> > arch/powerpc/Makefile adopted multiple ways to pass CONFIG options.
> >
> > arch/powerpc/boot/decompress.c references CONFIG_KERNEL_GZIP and
> > CONFIG_KERNEL_XZ, which are passed via the command line.
> >
> > arch/powerpc/boot/serial.c includes the copied autoconf.h to
> > reference a couple of CONFIG options.
> >
> > Do not do this.
> >
> > We should have already learned that including autoconf.h from each
> > source file is really fragile.
> >
> > In fact, it is already broken.
> >
> > arch/powerpc/boot/ppc_asm.h references CONFIG_PPC_8xx, but
> > arch/powerpc/boot/utils.S is not given any way to access CONFIG
> > options. So, CONFIG_PPC_8xx is never defined here.
> >
> > Just pass $(LINUXINCLUDE) and remove all broken code.
> >
> > I also removed the -traditional flag to make include/linux/kconfig.h
> > work. I do not understand why it needs to imitate the behavior of
> > pre-standard C preprocessors.
> >
> > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> > ---
>
>
> I re-read my commit log, and I thought it was needlessly
> too offensive. Sorry about that.
>
> I will reword the commit log and send v2.
>
No worries. We know the bootwrapper is... not great.
>
>
>
> --
> Best Regards
> Masahiro Yamada
^ permalink raw reply
* Re: [PATCH v6 1/1] iommu: enhance IOMMU dma mode build options
From: Leizhen (ThunderTown) @ 2019-05-13 14:04 UTC (permalink / raw)
To: John Garry, Jean-Philippe Brucker, Robin Murphy, Will Deacon,
Joerg Roedel, Jonathan Corbet, linux-doc, Sebastian Ott,
Gerald Schaefer, Martin Schwidefsky, Heiko Carstens,
Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Tony Luck, Fenghua Yu, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, H . Peter Anvin, David Woodhouse, iommu,
linux-kernel, linux-s390, linuxppc-dev, x86, linux-ia64
Cc: Hanjun Guo
In-Reply-To: <ca30a698-8047-9a86-a2f1-0b3e1c8692bf@huawei.com>
On 2019/5/8 17:42, John Garry wrote:
> On 18/04/2019 14:57, Zhen Lei wrote:
>> First, add build option IOMMU_DEFAULT_{LAZY|STRICT}, so that we have the
>> opportunity to set {lazy|strict} mode as default at build time. Then put
>> the three config options in an choice, make people can only choose one of
>> the three at a time.
>>
>> The default IOMMU dma modes on each ARCHs have no change.
>>
>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
>> ---
>> arch/ia64/kernel/pci-dma.c | 2 +-
>> arch/powerpc/platforms/powernv/pci-ioda.c | 3 ++-
>> arch/s390/pci/pci_dma.c | 2 +-
>> arch/x86/kernel/pci-dma.c | 7 ++---
>> drivers/iommu/Kconfig | 44 ++++++++++++++++++++++++++-----
>> drivers/iommu/amd_iommu_init.c | 3 ++-
>> drivers/iommu/intel-iommu.c | 2 +-
>> drivers/iommu/iommu.c | 3 ++-
>> 8 files changed, 48 insertions(+), 18 deletions(-)
>>
>> diff --git a/arch/ia64/kernel/pci-dma.c b/arch/ia64/kernel/pci-dma.c
>> index fe988c49f01ce6a..655511dbf3c3b34 100644
>> --- a/arch/ia64/kernel/pci-dma.c
>> +++ b/arch/ia64/kernel/pci-dma.c
>> @@ -22,7 +22,7 @@
>> int force_iommu __read_mostly;
>> #endif
>>
>> -int iommu_pass_through;
>> +int iommu_pass_through = IS_ENABLED(CONFIG_IOMMU_DEFAULT_PASSTHROUGH);
>>
>> static int __init pci_iommu_init(void)
>> {
>> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
>> index 3ead4c237ed0ec9..383e082a9bb985c 100644
>> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
>> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
>> @@ -85,7 +85,8 @@ void pe_level_printk(const struct pnv_ioda_pe *pe, const char *level,
>> va_end(args);
>> }
>>
>> -static bool pnv_iommu_bypass_disabled __read_mostly;
>> +static bool pnv_iommu_bypass_disabled __read_mostly =
>> + !IS_ENABLED(CONFIG_IOMMU_DEFAULT_PASSTHROUGH);
>> static bool pci_reset_phbs __read_mostly;
>>
>> static int __init iommu_setup(char *str)
>> diff --git a/arch/s390/pci/pci_dma.c b/arch/s390/pci/pci_dma.c
>> index 9e52d1527f71495..784ad1e0acecfb1 100644
>> --- a/arch/s390/pci/pci_dma.c
>> +++ b/arch/s390/pci/pci_dma.c
>> @@ -17,7 +17,7 @@
>>
>> static struct kmem_cache *dma_region_table_cache;
>> static struct kmem_cache *dma_page_table_cache;
>> -static int s390_iommu_strict;
>> +static int s390_iommu_strict = IS_ENABLED(CONFIG_IOMMU_DEFAULT_STRICT);
>>
>> static int zpci_refresh_global(struct zpci_dev *zdev)
>> {
>> diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
>> index d460998ae828514..fb2bab42a0a3173 100644
>> --- a/arch/x86/kernel/pci-dma.c
>> +++ b/arch/x86/kernel/pci-dma.c
>> @@ -43,11 +43,8 @@
>> * It is also possible to disable by default in kernel config, and enable with
>> * iommu=nopt at boot time.
>> */
>> -#ifdef CONFIG_IOMMU_DEFAULT_PASSTHROUGH
>> -int iommu_pass_through __read_mostly = 1;
>> -#else
>> -int iommu_pass_through __read_mostly;
>> -#endif
>> +int iommu_pass_through __read_mostly =
>> + IS_ENABLED(CONFIG_IOMMU_DEFAULT_PASSTHROUGH);
>>
>> extern struct iommu_table_entry __iommu_table[], __iommu_table_end[];
>>
>> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
>> index 6f07f3b21816c64..8a1f1793cde76b4 100644
>> --- a/drivers/iommu/Kconfig
>> +++ b/drivers/iommu/Kconfig
>> @@ -74,17 +74,47 @@ config IOMMU_DEBUGFS
>> debug/iommu directory, and then populate a subdirectory with
>> entries as required.
>>
>> -config IOMMU_DEFAULT_PASSTHROUGH
>> - bool "IOMMU passthrough by default"
>> +choice
>> + prompt "IOMMU dma mode"
>
> /s/dma/DMA/
OK
>
> And how about add "default", as in "Default IOMMU DMA mode" or "IOMMU default DMA mode"?
Yes. I prefer "IOMMU default DMA mode".
>
>> depends on IOMMU_API
>> - help
>> - Enable passthrough by default, removing the need to pass in
>> - iommu.passthrough=on or iommu=pt through command line. If this
>> - is enabled, you can still disable with iommu.passthrough=off
>> - or iommu=nopt depending on the architecture.
>> + default IOMMU_DEFAULT_PASSTHROUGH if (PPC_POWERNV && PCI)
>> + default IOMMU_DEFAULT_LAZY if (AMD_IOMMU || INTEL_IOMMU || S390_IOMMU)
>> + default IOMMU_DEFAULT_STRICT
>> + help
>> + This option allows IOMMU dma mode to be chose at build time, to
>
> again, capitalize acronyms, i.e. /s/dma/DMA/ (more of these above and below)
OK, I will check it all. Thanks.
>
>> + override the default dma mode of each ARCHs, removing the need to
>> + pass in kernel parameters through command line. You can still use
>> + ARCHs specific boot options to override this option again.
>> +
>> +config IOMMU_DEFAULT_PASSTHROUGH
>
> I think that it may need to be indented, along with the other choices
There is no problem. I referred to mm/Kconfig.
>
>> + bool "passthrough"
>> + help
>> + In this mode, the dma access through IOMMU without any addresses
>> + transformation. That means, the wrong or illegal dma access can not
>
> transformation, or translation?
I copied from somewhere. OK, "translation" will be more clear.
>
>> + be caught, no error information will be reported.
>>
>> If unsure, say N here.
>>
>> +config IOMMU_DEFAULT_LAZY
>> + bool "lazy"
>> + help
>> + Support lazy mode, where for every IOMMU DMA unmap operation, the
>> + flush operation of IOTLB and the free operation of IOVA are deferred.
>> + They are only guaranteed to be done before the related IOVA will be
>> + reused.
>> +
>> +config IOMMU_DEFAULT_STRICT
>> + bool "strict"
>> + help
>> + For every IOMMU DMA unmap operation, the flush operation of IOTLB and
>> + the free operation of IOVA are guaranteed to be done in the unmap
>> + function.
>> +
>> + This mode is safer than the two above, but it maybe slow in some high
>
> slow, or slower? And passthough is not safe, so anything is implicitly safer.
OK. I will change it to "slower".
>
>> + performace scenarios.
>> +
>> +endchoice
>> +
>> config OF_IOMMU
>> def_bool y
>> depends on OF && IOMMU_API
>> diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
>> index ff40ba758cf365e..16c02b08adb4cb2 100644
>> --- a/drivers/iommu/amd_iommu_init.c
>> +++ b/drivers/iommu/amd_iommu_init.c
>> @@ -166,7 +166,8 @@ struct ivmd_header {
>> to handle */
>> LIST_HEAD(amd_iommu_unity_map); /* a list of required unity mappings
>> we find in ACPI */
>> -bool amd_iommu_unmap_flush; /* if true, flush on every unmap */
>> +bool amd_iommu_unmap_flush = IS_ENABLED(CONFIG_IOMMU_DEFAULT_STRICT);
>> + /* if true, flush on every unmap */
>>
>> LIST_HEAD(amd_iommu_list); /* list of all AMD IOMMUs in the
>> system */
>> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
>> index 28cb713d728ceef..0c3cc716210f35a 100644
>> --- a/drivers/iommu/intel-iommu.c
>> +++ b/drivers/iommu/intel-iommu.c
>> @@ -362,7 +362,7 @@ static int domain_detach_iommu(struct dmar_domain *domain,
>>
>> static int dmar_map_gfx = 1;
>> static int dmar_forcedac;
>> -static int intel_iommu_strict;
>> +static int intel_iommu_strict = IS_ENABLED(CONFIG_IOMMU_DEFAULT_STRICT);
>> static int intel_iommu_superpage = 1;
>> static int intel_iommu_sm;
>> static int iommu_identity_mapping;
>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>> index 109de67d5d727c2..0ec5952ac60e2a3 100644
>> --- a/drivers/iommu/iommu.c
>> +++ b/drivers/iommu/iommu.c
>> @@ -43,7 +43,8 @@
>> #else
>> static unsigned int iommu_def_domain_type = IOMMU_DOMAIN_DMA;
>> #endif
>> -static bool iommu_dma_strict __read_mostly = true;
>> +static bool iommu_dma_strict __read_mostly =
>> + IS_ENABLED(CONFIG_IOMMU_DEFAULT_STRICT);
>>
>> struct iommu_callback_data {
>> const struct iommu_ops *ops;
>> --
>> 1.8.3
>>
>>
>>
>> .
>>
>
>
>
> .
>
--
Thanks!
BestRegards
^ permalink raw reply
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