Re: [PATCH v6 1/3] printk: Add line-buffered printk() API.
On (11/09/18 15:10), Petr Mladek wrote:
> >
> > If I'm not mistaken, this is for the futute "printk injection" work.
>
> The above code only tries to push complete lines to the main log buffer
> and consoles ASAP. It sounds like a Good Idea(tm).
Probably it is. So *quite likely* I'm wrong here.
Hmm... Thinking out loud.
At the same time, splitting a single logbuf entry gives a chance to other
events to mix in. Example:
pr_cont("Foo:");
pr_cont("\nbar");
pr_cont("\n");
Previously it could been stored in one logbuf entry (one logstore,
one console_trylock_spinning), which means that it could have been
printed in one go:
call_console_drivers()
spin_trylock_irqsave(&port->lock, flags);
uart_console_write( "Foo:\nbar\n");
spin_unlock_irqrestore(&port->lock, flags);
While with buffered printk, it will be store in two logbuf entries,
and printed in two goes:
call_console_drivers()
spin_trylock_irqsave(&port->lock, flags);
uart_console_write( "Foo:\nbar\n");
spin_unlock_irqrestore(&port->lock, flags);
<< ... console driver IRQ, TX port->state->xmit chars ... >>>
call_console_drivers()
spin_trylock_irqsave(&port->lock, flags);
uart_console_write( "Foo:\nbar\n");
spin_unlock_irqrestore(&port->lock, flags);
So, *technically*, we now let more events to happens between printk-s.
But, let's look at some of pr_cont() usage examples.
E.g. dump() from arch/h8300/kernel/traps.c. The code in question looks
as follows:
pr_info("\nKERNEL STACK:");
tp = ((unsigned char *) fp) - 0x40;
for (sp = (unsigned long *) tp, i = 0; (i < 0x40); i += 4) {
if ((i % 0x10) == 0)
pr_info("\n%08x: ", (int) (tp + i));
pr_info("%08x ", (int) *sp++);
}
pr_info("\n");
dmesg
[ 15.260099] : 0010 0040 0090
0010:0100 0190 0240 0310
0020:0400 0510 0640 0790
0030:0900 0a90 0c40 0e10
So we have the entire KERNEL STACK stored as a single line, in
a single logbuf entry.
cat /dev/kmsg
4,687,15260099,c;\x0a: 0010 0040 0090 \x0a0010:
0100 0190 0240 0310 \x0a0020:0400 0510
0640 0790 \x0a0030:0900 0a90 0c40 0e10
Shall we consider this as a "reference" representation: data that
pr_cont(), ideally, would have stored as a single logbuf entry? And
then compare the "reference" representation and what buffered printk
does.
Buffered printk always stores this as several lines, IOW several
logbuf entries.
cat /dev/kmsg
4,690,15260152,-;: 0010 0040 0090
4,691,15260154,-;0010:0100 0190 0240 0310
4,692,15260157,-;0020:0400 0510 0640 0790
4,694,15260161,-;0030:0900 0a90 0c40 0e10
So if we will have concurrent printk()-s happening on other CPUs,
then the KERNEL STACK data block still can be a bit hard to read:
[ 15.260152] : 0010 0040 0090
... foo bar foo bar
... foo bar foo bar
...
[ 15.260154] 0010:0100 0190 0240 0310
... foo bar foo bar
... foo bar foo bar
...
... and so on; you got the idea.
> No, please note that the for cycle searches for '\n' from the end
> of the string.
You are right, I didn't notice that. Indeed.
Tetsuo, lockdep report with buffered printks looks a bit different:
kernel: Possible unsafe locking scenario:
kernel:CPU0CPU1
kernel:
kernel: lock(bar_lock);
kernel:lock(
kernel: foo_lock);
kernel:lock(bar_lock);
kernel: lock(foo_lock);
kernel:
> > len = vsprintf();
> > if (len && text[len - 1] == '\n' || overflow)
> > flush();
>
> I had the same idea. Tetsuo ignored it. I looked for more arguments
> and found that '\n' is used in the middle of several pr_cont()
> calls
OK, so we probably can have that new semantics. But we might split
something that possibly was meant to be a single line with a bunch
of \n in the middle.
-ss
[RFCv3 PATCH 1/6] uacce: Add documents for WarpDrive/uacce
From: Kenneth Lee WarpDrive is a general accelerator framework for the user application to access the hardware without going through the kernel in data path. The kernel component to provide kernel facility to driver for expose the user interface is called uacce. It a short name for "Unified/User-space-access-intended Accelerator Framework". This patch add document to explain how it works. Signed-off-by: Kenneth Lee --- Documentation/warpdrive/warpdrive.rst | 260 +++ Documentation/warpdrive/wd-arch.svg | 764 Documentation/warpdrive/wd.svg | 526 ++ Documentation/warpdrive/wd_q_addr_space.svg | 359 + 4 files changed, 1909 insertions(+) create mode 100644 Documentation/warpdrive/warpdrive.rst create mode 100644 Documentation/warpdrive/wd-arch.svg create mode 100644 Documentation/warpdrive/wd.svg create mode 100644 Documentation/warpdrive/wd_q_addr_space.svg diff --git a/Documentation/warpdrive/warpdrive.rst b/Documentation/warpdrive/warpdrive.rst new file mode 100644 index ..ef84d3a2d462 --- /dev/null +++ b/Documentation/warpdrive/warpdrive.rst @@ -0,0 +1,260 @@ +Introduction of WarpDrive += + +*WarpDrive* is a general accelerator framework for the user application to +access the hardware without going through the kernel in data path. + +It can be used as the quick channel for accelerators, network adaptors or +other hardware for application in user space. + +This may make some implementation simpler. E.g. you can reuse most of the +*netdev* driver in kernel and just share some ring buffer to the user space +driver for *DPDK* [4] or *ODP* [5]. Or you can combine the RSA accelerator with +the *netdev* in the user space as a https reversed proxy, etc. + +*WarpDrive* takes the hardware accelerator as a heterogeneous processor which +can share particular load from the CPU: + +.. image:: wd.svg +:alt: WarpDrive Concept + +The virtual concept, queue, is used to manage the requests sent to the +accelerator. The application send requests to the queue by writing to some +particular address, while the hardware takes the requests directly from the +address and send feedback accordingly. + +The format of the queue may differ from hardware to hardware. But the +application need not to make any system call for the communication. + +*WarpDrive* tries to create a shared virtual address space for all involved +accelerators. Within this space, the requests sent to queue can refer to any +virtual address, which will be valid to the application and all involved +accelerators. + +The name *WarpDrive* is simply a cool and general name meaning the framework +makes the application faster. It includes general user library, kernel +management module and drivers for the hardware. In kernel, the management +module is called *uacce*, meaning "Unified/User-space-access-intended +Accelerator Framework". + + +How does it work + + +*WarpDrive* uses *mmap* and *IOMMU* to play the trick. + +*Uacce* creates a chrdev for the device registered to it. A "queue" will be +created when the chrdev is opened. The application access the queue by mmap +different address region of the queue file. + +The following figure demonstrated the queue file address space: + +.. image:: wd_q_addr_space.svg +:alt: WarpDrive Queue Address Space + +The first region of the space, device region, is used for the application to +write request or read answer to or from the hardware. + +Normally, there can be three types of device regions mmio and memory regions. +It is recommended to use common memory for request/answer descriptors and use +the mmio space for device notification, such as doorbell. But of course, this +is all up to the interface designer. + +There can be two types of device memory regions, kernel-only and user-shared. +This will be explained in the "kernel APIs" section. + +The Static Share Virtual Memory region is necessary only when the device IOMMU +does not support "Share Virtual Memory". This will be explained after the +*IOMMU* idea. + + +Architecture + + +The full *WarpDrive* architecture is represented in the following class +diagram: + +.. image:: wd-arch.svg +:alt: WarpDrive Architecture + + +The user API + + +We adopt a polling style interface in the user space: :: + +int wd_request_queue(struct wd_queue *q); +void wd_release_queue(struct wd_queue *q); + +int wd_send(struct wd_queue *q, void *req); +int wd_recv(struct wd_queue *q, void **req); +int wd_recv_sync(struct wd_queue *q, void **req); +void wd_flush(struct wd_queue *q); + +wd_recv_sync() is a wrapper to its non-sync version. It will trapped into +kernel and waits until the queue become available. + +If the queue do not support SVA/SVM. The following helper function +can be used to create Static Virtual Share Memory: :: + +void *wd_preserve_share_m
Re: [PATCH 0/4] patches for FPGA
Hello Alan, Am 07.11.2018 18:51, schrieb Alan Tull: Hi Greg, Please take these four small fpga fixes patches. They have been reviewed on the mailing list and apply cleanly on current linux-next and char-misc-testing. Thanks, Alan Anatolij Gustschin (1): fpga: altera-cvp: fix 'bad IO access' on x86_64 Andreas Puhm (1): fpga: altera-cvp: Fix registration for CvP incapable devices Mike Looijmans (1): zynq-fpga: Only route PR via PCAP when required YueHaibing (1): fpga: dfl: fme: remove set but not used variable 'priv' drivers/fpga/altera-cvp.c | 15 +-- drivers/fpga/dfl-fme-pr.c | 2 -- drivers/fpga/zynq-fpga.c | 4 3 files changed, 17 insertions(+), 4 deletions(-) How is the backporting strategy for FPGA manager related bugfixes and drivers? Best regards Eric
[PATCH 0/3] Qualcomm AOSS QMP side channel binding and driver
Add binding and driver for the QMP based side-channel communication mechanism to the AOSS, which is used to control resources not exposed through the RPMh interface. Currently implemented is a genpd provider, but pending some improvements in the thermal framework a cooling device will be added at a later point. Bjorn Andersson (3): dt-bindings: soc: qcom: Add AOSS QMP binding soc: qcom: Add AOSS QMP communication driver soc: qcom: Add AOSS QMP genpd provider .../bindings/soc/qcom/qcom,aoss-qmp.txt | 63 drivers/soc/qcom/Kconfig | 15 + drivers/soc/qcom/Makefile | 2 + drivers/soc/qcom/aoss-qmp-pd.c| 135 drivers/soc/qcom/aoss-qmp.c | 313 ++ include/dt-bindings/power/qcom-aoss-qmp.h | 15 + include/linux/soc/qcom/aoss-qmp.h | 12 + 7 files changed, 555 insertions(+) create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,aoss-qmp.txt create mode 100644 drivers/soc/qcom/aoss-qmp-pd.c create mode 100644 drivers/soc/qcom/aoss-qmp.c create mode 100644 include/dt-bindings/power/qcom-aoss-qmp.h create mode 100644 include/linux/soc/qcom/aoss-qmp.h -- 2.18.0
[PATCH 1/3] dt-bindings: soc: qcom: Add AOSS QMP binding
Add binding for the QMP based side-channel communication mechanism to
the AOSS, which is used to control resources not exposed through the
RPMh interface.
Signed-off-by: Bjorn Andersson
---
.../bindings/soc/qcom/qcom,aoss-qmp.txt | 63 +++
include/dt-bindings/power/qcom-aoss-qmp.h | 15 +
2 files changed, 78 insertions(+)
create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,aoss-qmp.txt
create mode 100644 include/dt-bindings/power/qcom-aoss-qmp.h
diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,aoss-qmp.txt
b/Documentation/devicetree/bindings/soc/qcom/qcom,aoss-qmp.txt
new file mode 100644
index ..e3c8cb4372f2
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,aoss-qmp.txt
@@ -0,0 +1,63 @@
+Qualcomm Always-On Subsystem side channel binding
+
+This binding describes the hardware component responsible for side channel
+requests to the always-on subsystem (AOSS), used for certain power management
+requests that is not handled by the standard RPMh interface. Each client in the
+SoC has it's own block of message RAM and IRQ for communication with the AOSS.
+The protocol used to communicate in the message RAM is known as QMP.
+
+- compatible:
+ Usage: required
+ Value type:
+ Definition: must be "qcom,sdm845-aoss-qmp"
+
+- reg:
+ Usage: required
+ Value type:
+ Definition: the base address and size of the message RAM for this
+ client's communication with the AOSS
+
+- interrupts:
+ Usage: required
+ Value type:
+ Definition: should specify the AOSS message IRQ for this client
+
+- mboxes:
+ Usage: required
+ Value type:
+ Definition: reference to the mailbox representing the outgoing doorbell
+ in APCS for this client, as described in mailbox/mailbox.txt
+
+= AOSS Power-domains
+The AOSS side channel exposes control over a set of resources, used to control
+a set of debug related clocks and to affect the low power state of resources
+related to the secondary subsystems. These resources are described as a set of
+power-domains in a subnode of hte AOSS side channel node.
+
+- compatible:
+ Usage: required
+ Value type:
+ Definition: must be "qcom,sdm845-aoss-qmp-pd"
+
+- #power-domain-cells:
+ Usage: required
+ Value type:
+ Definition: must be 1
+
+= EXAMPLE
+
+The following example represents the AOSS side-channel message RAM and the
+mechanism exposing the power-domains, as found in SDM845.
+
+ aoss_qmp: qmp@c30 {
+ compatible = "qcom,sdm845-aoss-qmp";
+ reg = <0x0c30 0x10>;
+ interrupts = ;
+
+ mboxes = <&apss_shared 0>;
+
+ aoss_qmp_pd: power-controller {
+ compatible = "qcom,sdm845-aoss-qmp-pd";
+ #power-domain-cells = <1>;
+ };
+ };
diff --git a/include/dt-bindings/power/qcom-aoss-qmp.h
b/include/dt-bindings/power/qcom-aoss-qmp.h
new file mode 100644
index ..7d8ac1a4f90c
--- /dev/null
+++ b/include/dt-bindings/power/qcom-aoss-qmp.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c) 2018, Linaro Ltd. */
+
+#ifndef __DT_BINDINGS_POWER_QCOM_AOSS_QMP_H
+#define __DT_BINDINGS_POWER_QCOM_AOSS_QMP_H
+
+#define AOSS_QMP_QDSS_CLK 0
+#define AOSS_QMP_LS_CDSP 1
+#define AOSS_QMP_LS_LPASS 2
+#define AOSS_QMP_LS_MODEM 3
+#define AOSS_QMP_LS_SLPI 4
+#define AOSS_QMP_LS_SPSS 5
+#define AOSS_QMP_LS_VENUS 6
+
+#endif
--
2.18.0
[PATCH 2/3] soc: qcom: Add AOSS QMP communication driver
The AOSS QMP driver is used to communicate with the AOSS for certain
side-channel requests, that are not enabled through the RPMh interface.
The communication is a very simple synchronous mechanism of messages
being written in message RAM and a doorbell in the AOSS is rung. As the
AOSS has processed the message length is cleared and an interrupt is
fired by the AOSS as acknowledgment.
Signed-off-by: Bjorn Andersson
---
drivers/soc/qcom/Kconfig | 7 +
drivers/soc/qcom/Makefile | 1 +
drivers/soc/qcom/aoss-qmp.c | 313 ++
include/linux/soc/qcom/aoss-qmp.h | 12 ++
4 files changed, 333 insertions(+)
create mode 100644 drivers/soc/qcom/aoss-qmp.c
create mode 100644 include/linux/soc/qcom/aoss-qmp.h
diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index a51458022d21..ba08fc00d7f5 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -3,6 +3,13 @@
#
menu "Qualcomm SoC drivers"
+config QCOM_AOSS_QMP
+ tristate "Qualcomm AOSS Messaging Driver"
+ help
+ This driver provides the means for communicating with the
+ micro-controller in the AOSS, using QMP, to control certain resource
+ that are not exposed through RPMh.
+
config QCOM_COMMAND_DB
bool "Qualcomm Command DB"
depends on ARCH_QCOM || COMPILE_TEST
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index 67cb85d0373c..d0d7fdc94d9a 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -1,5 +1,6 @@
# SPDX-License-Identifier: GPL-2.0
CFLAGS_rpmh-rsc.o := -I$(src)
+obj-$(CONFIG_QCOM_AOSS_QMP) += aoss-qmp.o
obj-$(CONFIG_QCOM_GENI_SE) += qcom-geni-se.o
obj-$(CONFIG_QCOM_COMMAND_DB) += cmd-db.o
obj-$(CONFIG_QCOM_GLINK_SSR) +=glink_ssr.o
diff --git a/drivers/soc/qcom/aoss-qmp.c b/drivers/soc/qcom/aoss-qmp.c
new file mode 100644
index ..acc5677a06ed
--- /dev/null
+++ b/drivers/soc/qcom/aoss-qmp.c
@@ -0,0 +1,313 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018, Linaro Ltd
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define QMP_DESC_MAGIC 0x0
+#define QMP_DESC_VERSION 0x4
+#define QMP_DESC_FEATURES 0x8
+
+#define QMP_DESC_UCORE_LINK_STATE 0xc
+#define QMP_DESC_UCORE_LINK_STATE_ACK 0x10
+#define QMP_DESC_UCORE_CH_STATE0x14
+#define QMP_DESC_UCORE_CH_STATE_ACK0x18
+#define QMP_DESC_UCORE_MBOX_SIZE 0x1c
+#define QMP_DESC_UCORE_MBOX_OFFSET 0x20
+
+#define QMP_DESC_MCORE_LINK_STATE 0x24
+#define QMP_DESC_MCORE_LINK_STATE_ACK 0x28
+#define QMP_DESC_MCORE_CH_STATE0x2c
+#define QMP_DESC_MCORE_CH_STATE_ACK0x30
+#define QMP_DESC_MCORE_MBOX_SIZE 0x34
+#define QMP_DESC_MCORE_MBOX_OFFSET 0x38
+
+#define QMP_STATE_UP 0x
+#define QMP_STATE_DOWN 0x
+
+#define QMP_MAGIC 0x4d41494c
+#define QMP_VERSION1
+
+/**
+ * struct qmp - driver state for QMP implementation
+ * @msgram: iomem referencing the message RAM used for communication
+ * @dev: reference to QMP device
+ * @mbox_client: mailbox client used to ring the doorbell on transmit
+ * @mbox_chan: mailbox channel used to ring the doorbell on transmit
+ * @offset: offset within @msgram where messages should be written
+ * @size: maximum size of the messages to be transmitted
+ * @event: wait_queue for synchronization with the IRQ
+ * @tx_lock: provides syncrhonization between multiple callers of qmp_send()
+ */
+struct qmp {
+ void __iomem *msgram;
+ struct device *dev;
+
+ struct mbox_client mbox_client;
+ struct mbox_chan *mbox_chan;
+
+ size_t offset;
+ size_t size;
+
+ wait_queue_head_t event;
+
+ struct mutex tx_lock;
+};
+
+static void qmp_kick(struct qmp *qmp)
+{
+ mbox_send_message(qmp->mbox_chan, NULL);
+ mbox_client_txdone(qmp->mbox_chan, 0);
+}
+
+static bool qmp_magic_valid(struct qmp *qmp)
+{
+ return readl(qmp->msgram + QMP_DESC_MAGIC) == QMP_MAGIC;
+}
+
+static bool qmp_link_acked(struct qmp *qmp)
+{
+ return readl(qmp->msgram + QMP_DESC_MCORE_LINK_STATE_ACK) ==
QMP_STATE_UP;
+}
+
+static bool qmp_mcore_channel_acked(struct qmp *qmp)
+{
+ return readl(qmp->msgram + QMP_DESC_MCORE_CH_STATE_ACK) == QMP_STATE_UP;
+}
+
+static bool qmp_ucore_channel_up(struct qmp *qmp)
+{
+ return readl(qmp->msgram + QMP_DESC_UCORE_CH_STATE) == QMP_STATE_UP;
+}
+
+static int qmp_open(struct qmp *qmp)
+{
+ int ret;
+ u32 val;
+
+ ret = wait_event_timeout(qmp->event, qmp_magic_valid(qmp), HZ);
+ if (!ret) {
+ dev_err(qmp->dev, "QMP magic doesn't match\n");
+ return -ETIMEDOUT;
+ }
+
+ val = readl(qmp->msgram + QMP_DESC_VERSION);
+ if (val != QMP_VERSION) {
+ dev_err(qmp->dev, "unsupported QMP version %d\n", val);
+ return -EINVAL;
+ }
+
+
[PATCH 3/3] soc: qcom: Add AOSS QMP genpd provider
The AOSS QMP genpd provider implements control over power-related
resources related to low-power state associated with the remoteprocs in
the system as well as control over a set of clocks related to debug
hardware in the SoC.
Signed-off-by: Bjorn Andersson
---
drivers/soc/qcom/Kconfig | 8 ++
drivers/soc/qcom/Makefile | 1 +
drivers/soc/qcom/aoss-qmp-pd.c | 135 +
3 files changed, 144 insertions(+)
create mode 100644 drivers/soc/qcom/aoss-qmp-pd.c
diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index ba08fc00d7f5..e1eda3d59748 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -10,6 +10,14 @@ config QCOM_AOSS_QMP
micro-controller in the AOSS, using QMP, to control certain resource
that are not exposed through RPMh.
+config QCOM_AOSS_QMP_PD
+ tristate "Qualcomm AOSS Messaging Power Domain driver"
+ depends on QCOM_AOSS_QMP
+ help
+ This driver provides the means of controlling the AOSSs handling of
+ low-power state for resources related to the remoteproc subsystems as
+ well as controlling the debug clocks.
+
config QCOM_COMMAND_DB
bool "Qualcomm Command DB"
depends on ARCH_QCOM || COMPILE_TEST
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index d0d7fdc94d9a..ebfa414a5b77 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -1,6 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
CFLAGS_rpmh-rsc.o := -I$(src)
obj-$(CONFIG_QCOM_AOSS_QMP) += aoss-qmp.o
+obj-$(CONFIG_QCOM_AOSS_QMP_PD) += aoss-qmp-pd.o
obj-$(CONFIG_QCOM_GENI_SE) += qcom-geni-se.o
obj-$(CONFIG_QCOM_COMMAND_DB) += cmd-db.o
obj-$(CONFIG_QCOM_GLINK_SSR) +=glink_ssr.o
diff --git a/drivers/soc/qcom/aoss-qmp-pd.c b/drivers/soc/qcom/aoss-qmp-pd.c
new file mode 100644
index ..467d0db4abfa
--- /dev/null
+++ b/drivers/soc/qcom/aoss-qmp-pd.c
@@ -0,0 +1,135 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018, Linaro Ltd
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+struct qmp_pd {
+ struct qmp *qmp;
+
+ struct generic_pm_domain pd;
+
+ const char *name;
+};
+
+#define to_qmp_pd_resource(res) container_of(res, struct qmp_pd, pd)
+
+struct qmp_pd_resource {
+ const char *name;
+ int (*on)(struct generic_pm_domain *domain);
+ int (*off)(struct generic_pm_domain *domain);
+};
+
+static int qmp_pd_clock_toggle(struct qmp_pd *res, bool enable)
+{
+ char buf[96];
+ size_t n;
+
+ n = snprintf(buf, sizeof(buf), "{class: clock, res: %s, val: %d}",
+res->name, !!enable);
+ return qmp_send(res->qmp, buf, n);
+}
+
+static int qmp_pd_clock_on(struct generic_pm_domain *domain)
+{
+ return qmp_pd_clock_toggle(to_qmp_pd_resource(domain), true);
+}
+
+static int qmp_pd_clock_off(struct generic_pm_domain *domain)
+{
+ return qmp_pd_clock_toggle(to_qmp_pd_resource(domain), false);
+}
+
+static int qmp_pd_image_toggle(struct qmp_pd *res, bool enable)
+{
+ char buf[96];
+ size_t n;
+
+ n = snprintf(buf, sizeof(buf),
+"{class: image, res: load_state, name: %s, val: %s}",
+res->name, enable ? "on" : "off");
+ return qmp_send(res->qmp, buf, sizeof(buf));
+}
+
+static int qmp_pd_image_on(struct generic_pm_domain *domain)
+{
+ return qmp_pd_image_toggle(to_qmp_pd_resource(domain), true);
+}
+
+static int qmp_pd_image_off(struct generic_pm_domain *domain)
+{
+ return qmp_pd_image_toggle(to_qmp_pd_resource(domain), false);
+}
+
+static const struct qmp_pd_resource sdm845_resources[] = {
+ [AOSS_QMP_QDSS_CLK] = { "qdss", qmp_pd_clock_on, qmp_pd_clock_off },
+ [AOSS_QMP_LS_CDSP] = { "cdsp", qmp_pd_image_on, qmp_pd_image_off },
+ [AOSS_QMP_LS_LPASS] = { "adsp", qmp_pd_image_on, qmp_pd_image_off },
+ [AOSS_QMP_LS_MODEM] = { "modem", qmp_pd_image_on, qmp_pd_image_off },
+ [AOSS_QMP_LS_SLPI] = { "slpi", qmp_pd_image_on, qmp_pd_image_off },
+ [AOSS_QMP_LS_SPSS] = { "spss", qmp_pd_image_on, qmp_pd_image_off },
+ [AOSS_QMP_LS_VENUS] = { "venus", qmp_pd_image_on, qmp_pd_image_off },
+};
+
+static int qmp_pd_probe(struct platform_device *pdev)
+{
+ struct genpd_onecell_data *data;
+ struct qmp_pd *res;
+ struct qmp *qmp;
+ size_t num = ARRAY_SIZE(sdm845_resources);
+ int i;
+
+ qmp = dev_get_drvdata(pdev->dev.parent);
+ if (!qmp)
+ return -EINVAL;
+
+ res = devm_kcalloc(&pdev->dev, num, sizeof(*res), GFP_KERNEL);
+ if (!res)
+ return -ENOMEM;
+
+ data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ data->domains = devm_kcalloc(&pdev->dev, num, sizeof(*data->domains),
+GFP_KERNEL);
+
+ for (i = 0; i < num; i++) {
RE: [PATCH] ftrace: remove KASAN poison in ftrace_ops_test()
> -Original Message- > From: Mark Rutland [mailto:mark.rutl...@arm.com] > Sent: Monday, November 12, 2018 1:32 AM > To: Zhang Zhizhou(张治洲) > Cc: rost...@goodmis.org; mi...@redhat.com; linux- > ker...@vger.kernel.org; zhizho...@gmail.com > Subject: Re: [PATCH] ftrace: remove KASAN poison in ftrace_ops_test() > > On Sun, Nov 11, 2018 at 11:10:17PM +0800, Zhizhou Zhang wrote: > > ftrace_ops_test() passed local varible parameter to > > hash_contains_ip(), which could result KASAN stack-out-of-bounds > warning. > > > > Signed-off-by: Zhizhou Zhang > > --- > > kernel/trace/ftrace.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c index > > f536f60..6e11f90 100644 > > --- a/kernel/trace/ftrace.c > > +++ b/kernel/trace/ftrace.c > > @@ -1522,6 +1522,8 @@ ftrace_ops_test(struct ftrace_ops *ops, unsigned > long ip, void *regs) > > rcu_assign_pointer(hash.filter_hash, ops->func_hash->filter_hash); > > rcu_assign_pointer(hash.notrace_hash, ops->func_hash- > >notrace_hash); > > > > + kasan_unpoison_task_stack(current); > > This is extremely heavy-handed, and will mask legitimate stack-out-of- > bounds errors. > Yes. I tried to find ways to mark this function no need to poison, but failed. My first thought was just make this function not poison its stack. > Passing a stack-local variable by reference *should not* result in KASAN > warnings unless KASAN itself is broken. Can you please give an example > report when this occurs? > Please excuse my lack of knowledge. Did you mean if a function passing a stack-local variable as parameter to another function, the compiler won't poison the stack-local variable? The KASAN report is based on 4.4.145, I'm not sure for the latest kernel: c3 0 (swapper/3) == c3 0 (swapper/3) BUG: KASAN: stack-out-of-bounds in hash_contains_ip.isra.4+0x44/0xd0 c3 0 (swapper/3) Read of size 8 at addr ffc0d204bc20 by task swapper/3/0 c3 0 (swapper/3) c3 0 (swapper/3) CPU: 3 PID: 0 Comm: swapper/3 Not tainted 4.4.145+ #168 c3 0 (swapper/3) Hardware name: ASR AQUILAC EVB (DT) c3 0 (swapper/3) Call trace: c3 0 (swapper/3) [] dump_backtrace+0x0/0x418 c3 0 (swapper/3) [] show_stack+0x28/0x38 c3 0 (swapper/3) [] dump_stack+0xe8/0x13c c3 0 (swapper/3) [] print_address_description+0x8c/0x2b0 c3 0 (swapper/3) [] kasan_report+0x210/0x330 c3 0 (swapper/3) [] __asan_load8+0x84/0x98 c3 0 (swapper/3) [] hash_contains_ip.isra.4+0x44/0xd0 c3 0 (swapper/3) [] ftrace_ops_test.isra.5+0xa8/0xe0 c3 0 (swapper/3) [] ftrace_ops_no_ops+0xdc/0x218 c3 0 (swapper/3) [] ftrace_graph_call+0x0/0x14 c3 0 (swapper/3) [] scale_cpu_capacity+0x24/0x70 c3 0 (swapper/3) [] sync_entity_load_avg+0xb0/0x4c0 c3 0 (swapper/3) [] remove_entity_load_avg+0x2c/0x80 c3 0 (swapper/3) [] task_dead_fair+0x20/0x30 c3 0 (swapper/3) [] finish_task_switch+0x1f4/0x2a0 c3 0 (swapper/3) [] __schedule+0x4cc/0xe80 c3 0 (swapper/3) [] schedule+0x70/0x110 c3 0 (swapper/3) [] schedule_preempt_disabled+0x24/0x70 c3 0 (swapper/3) [] cpu_startup_entry+0x198/0x538 c3 0 (swapper/3) [] secondary_start_kernel+0x258/0x2f0 c3 0 (swapper/3) [<0001032e803c>] 0x1032e803c c3 0 (swapper/3) c3 0 (swapper/3) The buggy address belongs to the page: c3 0 (swapper/3) page:ffbdc34812c0 count:0 mapcount:0 mapping: (null) index:0x0 c3 0 (swapper/3) flags: 0x0() c3 0 (swapper/3) page dumped because: kasan: bad access detected c3 0 (swapper/3) c3 0 (swapper/3) Memory state around the buggy address: c3 0 (swapper/3) ffc0d204bb00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 c3 0 (swapper/3) ffc0d204bb80: 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1 c3 0 (swapper/3) >ffc0d204bc00: f1 f1 f1 f1 f3 f3 f3 f3 00 00 00 00 00 00 00 00 c3 0 (swapper/3)^ c3 0 (swapper/3) ffc0d204bc80: 00 00 00 00 00 00 f2 f2 00 00 00 00 00 00 00 00 c3 0 (swapper/3) ffc0d204bd00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 c3 0 (swapper/3) == c3 1 (init) init: Untracked pid 3103 killed by signal 14 c4 972 (syz-executor) asr-wdt d408.watchdog: feed asr_wdt(soc_wdt). c5 431 (kworker/u16:8) C0_DFC: Time 117510987492 (1248 -> 1600); c5 431 (kworker/u16:8) C0_DFC: Time 117582358799 (1600 -> 1248); c5 431 (kworker/u16:8) C1_DFC: Time 51133656270 (1900 -> 1248); c5 431 (kworker/u16:8) C1_DFC: Time 51133864462 (1248 -> 1600); c5 431 (kworker/u16:8) DDR_DFC: Time 0 (0 -> 0); c5 431 (kworker/u16:8) DDR_DFC: Time 0 (0 -> 0); c5 431 (kworker/u16:8) CLK: GPU(31200, 0);VPU(83200, 0);ISP(31200, 0); c5 431 (kworker/u16:8) DVC: clst0(11); clst1(11); m2(11); peri(1); d1p(1); c5 431 (kworker/u16:8) Temp: cpu(82000); gpu(64000); ddr(68000); local(64000); battery(0); pmic(0) c5 431 (kworker/u16:8) Battery: soc(57648); i_battery(-941556); c5 431 (kworker/u16:8) 88pm88x-battery 88pm88x-batt
Re: Official Linux system wrapper library?
* Daniel Colascione: > If the kernel provides a system call, libc should provide a C wrapper > for it, even if in the opinion of the libc maintainers, that system > call is flawed. It's not that simple, I think. What about bdflush? socketcall? getxpid? osf_gettimeofday? set_robust_list? There are quite a few irregularities, and some editorial discretion appears to be unavoidable. Even if we were to provide perfectly consistent system call wrappers under separate names, we'd still expose different calling conventions for things like off_t to applications, which would make using some of the system calls quite difficult and surprisingly non-portable. Thanks, Florian
Re: [PATCH v2] exec: make de_thread() freezable
On 11/12, Chanho Min wrote:
>
> @@ -1083,7 +1084,7 @@ static int de_thread(struct task_struct *tsk)
> while (sig->notify_count) {
> __set_current_state(TASK_KILLABLE);
> spin_unlock_irq(lock);
> - schedule();
> + freezable_schedule();
> if (unlikely(__fatal_signal_pending(tsk)))
> goto killed;
> spin_lock_irq(lock);
> @@ -,7 +1112,7 @@ static int de_thread(struct task_struct *tsk)
> __set_current_state(TASK_KILLABLE);
> write_unlock_irq(&tasklist_lock);
> cgroup_threadgroup_change_end(tsk);
> - schedule();
> + freezable_schedule();
> if (unlikely(__fatal_signal_pending(tsk)))
> goto killed;
> }
Thanks, looks good to me.
Acked-by: Oleg Nesterov
Re: [PATCH v2 1/4] dt-bindings: hwlock: Document STM32 hwspinlock bindings
Hi Benjamin
On 11/8/18 10:04 AM, Benjamin Gaignard wrote:
Add bindings for STM32 hardware spinlock device
Signed-off-by: Benjamin Gaignard
---
version 2 :
- change clock name from hwspinlock to hsem to be align with hardware
documentation
.../bindings/hwlock/st,stm32-hwspinlock.txt| 23 ++
1 file changed, 23 insertions(+)
create mode 100644
Documentation/devicetree/bindings/hwlock/st,stm32-hwspinlock.txt
diff --git a/Documentation/devicetree/bindings/hwlock/st,stm32-hwspinlock.txt
b/Documentation/devicetree/bindings/hwlock/st,stm32-hwspinlock.txt
new file mode 100644
index ..6e933b218574
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwlock/st,stm32-hwspinlock.txt
@@ -0,0 +1,23 @@
+STM32 Hardware Spinlock Device Binding
+-
+
+Required properties :
+- compatible : should be "st,stm32-hwspinlock".
+- reg : the register address of hwspinlock.
+- #hwlock-cells : hwlock users only use the hwlock id to represent a specific
+ hwlock, so the number of cells should be <1> here.
+- clock-names : Must contain "hwspinlock".
"hwspinlock" --> "hsem" ?
+- clocks : Must contain a phandle entry for the clock in clock-names, see the
+ common clock bindings.
+
+Please look at the generic hwlock binding for usage information for consumers,
+"Documentation/devicetree/bindings/hwlock/hwlock.txt"
+
+Example of hwlock provider:
+ hwspinlock@4c00 {
+ compatible = "st,stm32-hwspinlock";
+ #hwlock-cells = <1>;
+ reg = <0x4c00 0x400>;
+ clocks = <&rcc HSEM>;
+ clock-names = "hsem";
+ };
Re: [PATCH 3.16 298/366] kthread, tracing: Don't expose half-written comm when creating kthreads
On 11/11/18 8:49 PM, Ben Hutchings wrote:
> 3.16.61-rc1 review patch. If anyone has any objections, please let me know.
>
> --
>
> From: Snild Dolkow
>
> commit 3e536e222f2930534c252c1cc7ae799c725c5ff9 upstream.
>
> There is a window for racing when printing directly to task->comm,
> allowing other threads to see a non-terminated string. The vsnprintf
> function fills the buffer, counts the truncated chars, then finally
> writes the \0 at the end.
>
> creator other
> vsnprintf:
> fill (not terminated)
> count the resttrace_sched_waking(p):
> ... memcpy(comm, p->comm, TASK_COMM_LEN)
> write \0
>
> The consequences depend on how 'other' uses the string. In our case,
> it was copied into the tracing system's saved cmdlines, a buffer of
> adjacent TASK_COMM_LEN-byte buffers (note the 'n' where 0 should be):
>
> crash-arm64> x/1024s savedcmd->saved_cmdlines | grep 'evenk'
> 0xffd5b3818640: "irq/497-pwr_evenkworker/u16:12"
>
> ...and a strcpy out of there would cause stack corruption:
>
> [224761.522292] Kernel panic - not syncing: stack-protector:
> Kernel stack is corrupted in: ff9bf9783c78
>
> crash-arm64> kbt | grep 'comm\|trace_print_context'
> #6 0xff9bf9783c78 in trace_print_context+0x18c(+396)
> comm (char [16]) = "irq/497-pwr_even"
>
> crash-arm64> rd 0xffd4d0e17d14 8
> ffd4d0e17d14: 2f717269 5f7277702d373934 irq/497-pwr_
> ffd4d0e17d24: 726f776b6e657665 3a3631752f72656b evenkworker/u16:
> ffd4d0e17d34: f9780248ff003231 cede60e0ff9b 12..H.x..`..
> ffd4d0e17d44: cede60c8ffd4 0fd4 .`..
>
> The workaround in e09e28671 (use strlcpy in __trace_find_cmdline) was
> likely needed because of this same bug.
>
> Solved by vsnprintf:ing to a local buffer, then using set_task_comm().
> This way, there won't be a window where comm is not terminated.
>
> Link: http://lkml.kernel.org/r/20180726071539.188015-1-sn...@sony.com
>
> Fixes: bc0c38d139ec7 ("ftrace: latency tracer infrastructure")
> Reviewed-by: Steven Rostedt (VMware)
> Signed-off-by: Snild Dolkow
> Signed-off-by: Steven Rostedt (VMware)
> [bwh: Backported to 3.16: adjust context]
> Signed-off-by: Ben Hutchings
> ---
> kernel/kthread.c | 8 +++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> --- a/kernel/kthread.c
> +++ b/kernel/kthread.c
> @@ -309,10 +309,16 @@ struct task_struct *kthread_create_on_no
> if (!IS_ERR(task)) {
> static const struct sched_param param = { .sched_priority = 0 };
> va_list args;
> + char name[TASK_COMM_LEN];
>
> + /*
> + * task is already visible to other tasks, so updating
> + * COMM must be protected.
> + */
> va_start(args, namefmt);
> - vsnprintf(task->comm, sizeof(task->comm), namefmt, args);
> + vsnprintf(name, sizeof(name), namefmt, args);
> va_end(args);
> + set_task_comm(task, name);
> /*
>* root may have changed our (kthreadd's) priority or CPU mask.
>* The kernel thread should not inherit these properties.
>
Reviewed-by: Snild Dolkow
Re: [PATCH 2/2] phy: ocelot-serdes: fix out-of-bounds read
Hi, On 17/10/18 9:07 PM, Gustavo A. R. Silva wrote: > Hi Kishon, > > On 10/16/18 10:48 AM, Kishon Vijay Abraham I wrote: >> Hi, >> >> On Tuesday 16 October 2018 02:16 PM, Gustavo A. R. Silva wrote: >>> Hi, >>> >>> On 10/9/18 9:28 AM, Quentin Schulz wrote: Hi Gustavo, On Tue, Oct 09, 2018 at 12:22:33AM +0200, Gustavo A. R. Silva wrote: > Currently, there is an out-of-bounds read on array ctrl->phys, > once variable i reaches the maximum array size of SERDES_MAX > in the for loop. > > Fix this by changing the condition in the for loop from > i <= SERDES_MAX to i < SERDES_MAX. > Reviewed-by: Quentin Schulz >>> >>> Friendly ping. Who can you take this? >> >> This can go during the 4.20 -rc cycle. >> > > Should I resend the following patch to you, so the whole series is > applied to your phy tree? > > https://lore.kernel.org/patchwork/patch/997326/ This is merged by David Miller. Thanks Kishon
Re: [PATCH] x86: remove gcc-x86_*-has-stack-protector.sh checks
On Sun, Nov 11, 2018 at 9:06 PM, Masahiro Yamada
wrote:
> gcc-x86_64-has-stack-protector.sh was introduced by commit 4f7fd4d7a791
> ("[PATCH] Add the -fstack-protector option to the CFLAGS") in 2006
> to work around buggy compilers.
>
> gcc-x86_32-has-stack-protector.sh was introduced by commit 60a5317ff0f4
> ("x86: implement x86_32 stack protector"), which did not clearly state
> whether compilers were still producing broken code at that time.
>
> Now, the minimum reuquired GCC version is 4.6, which was released in
> 2011. Probably, we can dump these old compiler checks.
NAK. We need to keep this because we've seen recent regressions with
stack protection (e.g. gcc briefly used global instead of tls for the
canary, which silently broke the use of stack protectors). Since the
gcc/kernel "API" for the canary is so fragile we need to keep these
tests to make sure things end up where they're expected.
-Kees
>
> Signed-off-by: Masahiro Yamada
> ---
>
> arch/x86/Kconfig | 10 +-
> scripts/gcc-x86_32-has-stack-protector.sh | 4
> scripts/gcc-x86_64-has-stack-protector.sh | 4
> 3 files changed, 1 insertion(+), 17 deletions(-)
> delete mode 100755 scripts/gcc-x86_32-has-stack-protector.sh
> delete mode 100755 scripts/gcc-x86_64-has-stack-protector.sh
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 9d734f3..7240d50 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -187,7 +187,7 @@ config X86
> select HAVE_REGS_AND_STACK_ACCESS_API
> select HAVE_RELIABLE_STACKTRACE if X86_64 &&
> (UNWINDER_FRAME_POINTER || UNWINDER_ORC) && STACK_VALIDATION
> select HAVE_FUNCTION_ARG_ACCESS_API
> - select HAVE_STACKPROTECTOR if CC_HAS_SANE_STACKPROTECTOR
> + select HAVE_STACKPROTECTOR
> select HAVE_STACK_VALIDATIONif X86_64
> select HAVE_RSEQ
> select HAVE_SYSCALL_TRACEPOINTS
> @@ -352,14 +352,6 @@ config PGTABLE_LEVELS
> default 3 if X86_PAE
> default 2
>
> -config CC_HAS_SANE_STACKPROTECTOR
> - bool
> - default
> $(success,$(srctree)/scripts/gcc-x86_64-has-stack-protector.sh $(CC)) if 64BIT
> - default
> $(success,$(srctree)/scripts/gcc-x86_32-has-stack-protector.sh $(CC))
> - help
> - We have to make sure stack protector is unconditionally disabled if
> - the compiler produces broken code.
> -
> menu "Processor type and features"
>
> config ZONE_DMA
> diff --git a/scripts/gcc-x86_32-has-stack-protector.sh
> b/scripts/gcc-x86_32-has-stack-protector.sh
> deleted file mode 100755
> index f5c1194..000
> --- a/scripts/gcc-x86_32-has-stack-protector.sh
> +++ /dev/null
> @@ -1,4 +0,0 @@
> -#!/bin/sh
> -# SPDX-License-Identifier: GPL-2.0
> -
> -echo "int foo(void) { char X[200]; return 3; }" | $* -S -x c -c -m32 -O0
> -fstack-protector - -o - 2> /dev/null | grep -q "%gs"
> diff --git a/scripts/gcc-x86_64-has-stack-protector.sh
> b/scripts/gcc-x86_64-has-stack-protector.sh
> deleted file mode 100755
> index 75e4e22..000
> --- a/scripts/gcc-x86_64-has-stack-protector.sh
> +++ /dev/null
> @@ -1,4 +0,0 @@
> -#!/bin/sh
> -# SPDX-License-Identifier: GPL-2.0
> -
> -echo "int foo(void) { char X[200]; return 3; }" | $* -S -x c -c -m64 -O0
> -mcmodel=kernel -fno-PIE -fstack-protector - -o - 2> /dev/null | grep -q "%gs"
> --
> 2.7.4
>
--
Kees Cook
Re: [PATCH 1/2] RISC-V: Request stat64 on RV32
David Abdurachmanov 於 2018年11月12日 週一 下午2:19寫道：
>
> On Mon, Nov 12, 2018 at 5:10 AM Zong Li wrote:
> >
> > The stat64 family that is used on 32-bit architectures to replace
> > newstat.
> >
> > Since commit 67314ec7b0250290cc85eaa7a2f88a8ddb9e8547 ("RISC-V: Request
> > newstat syscalls"), the RV32 build fail with undeclared 'sys_fstatat64'
> >
> > Signed-off-by: Zong Li
> > ---
> > arch/riscv/include/asm/unistd.h | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/riscv/include/asm/unistd.h
> > b/arch/riscv/include/asm/unistd.h
> > index eff7aa9..a4aade9 100644
> > --- a/arch/riscv/include/asm/unistd.h
> > +++ b/arch/riscv/include/asm/unistd.h
> > @@ -18,5 +18,6 @@
> >
> > #define __ARCH_WANT_NEW_STAT
> > #define __ARCH_WANT_SYS_CLONE
> > +#define __ARCH_WANT_STAT64
> > #include
> > #include
> > --
>
> See:
> http://lists.infradead.org/pipermail/linux-riscv/2018-November/002087.html
>
> The plan is not to have old stat syscalls and support statx on
> riscv32, which is y2038 safe.
>
> The issue you see is a bug in include/uapi/asm-generic/unistd.h.
> Marcin (CC) already sent a patch to Arnd (CC) IIRC. Basically without
> __ARCH_WANT_NEW_STAT or __ARCH_WANT_STAT64 two macros are not defined:
> __NR3264_fstatat and __NR3264_fstat. Which is later used (without any
> guards):
>
> 763 #define __NR_newfstatat __NR3264_fstatat
> 764 #define __NR_fstat __NR3264_fstat
Nice. Thank you.
soa up to MAY 2018
Good day to you! Here is the soa up to MAY 2018 Pls find the updated statement as attached for your checking. Kindly confirm it and provide your payment schedule to us. Looking forward to hear from you, thank you! Kindly check and reply us as soon as possible! Best Regards, G.C. Solanki Office No.2, Talib Musa Bldg., Bur Dubai, P.O. Box 62760, Dubai, U.A.E. Tel: +9714 3887842 Fax: +9714 3887843 Mobile: +97150 2723120 59874100.xls Description: 59874100.xls
Re: KASAN: use-after-free Read in nbp_vlan_rcu_free
On 12 November 2018 06:51:02 CET, syzbot wrote: >Hello, > >syzbot found the following crash on: > >HEAD commit:e12e00e388de Merge tag 'kbuild-fixes-v4.20' of >git://git.k.. >git tree: upstream >console output: >https://syzkaller.appspot.com/x/log.txt?x=14cdb6f540 >kernel config: >https://syzkaller.appspot.com/x/.config?x=8f215f21f041a0d7 >dashboard link: >https://syzkaller.appspot.com/bug?extid=04681da557a0e49a52e5 >compiler: gcc (GCC) 8.0.1 20180413 (experimental) > >Unfortunately, I don't have any reproducer for this crash yet. > >IMPORTANT: if you fix the bug, please add the following tag to the >commit: >Reported-by: syzbot+04681da557a0e49a5...@syzkaller.appspotmail.com Thanks, I'm about to fly out for LPC. Will take a look in a day.
Re: [PATCH] perf/x86/intel: Init early callchain for bts event
On Mon, Nov 12, 2018 at 01:26:37AM +0100, Peter Zijlstra wrote:
> On Sun, Nov 11, 2018 at 07:16:50PM +0100, Jiri Olsa wrote:
> > Vince reported crash in bts flush code when touching the
> > callchain data, which was supposed to be initialized
> > as an 'early' callchain data.
> >
> > BUG: unable to handle kernel NULL pointer dereference at
> > ...
>
> > It was triggered by fuzzer by can be easilt reproduced by:
> > # perf record -e cpu/branch-instructions/p -g -c 1
> >
> > The problem is that bts drain code does not initialize sample's
> > early callchain data and calls perf_prepare_sample with NULL
> > sample->callchain, even if it's expected to exist via
> > __PERF_SAMPLE_CALLCHAIN_EARLY sample type bit.
>
> Not sure that is the actual problem, nor that this:
>
> > @@ -612,6 +614,9 @@ int intel_pmu_drain_bts_buffer(void)
> >
> > perf_sample_data_init(&data, 0, event->hw.last_period);
> >
> > + if (event->attr.sample_type & __PERF_SAMPLE_CALLCHAIN_EARLY)
> > + data.callchain = &__empty_callchain;
> > +
> > /*
> > * BTS leaks kernel addresses in branches across the cpl boundary,
> > * such as traps or system calls, so unless the user is asking for
>
> is the right fix.
>
> If you look at commit:
>
> 6cbc304f2f36 ("perf/x86/intel: Fix unwind errors from PEBS entries (mk-II)")
>
> Then the right fix would be to do perf_callchain() from the BTS drain
> code -- if '/p'.
>
> Because prior to that commit, we would do a perf_callchain() in
> intel_pmu_drain_bts_buffer()'s call to perf_prepare_sample(), which
> would do an actual stack unwind for a branch entry.
>
> With your patch, we get an empty stack for every entry.
>
> Which is a change in behaviour...
I thought there's no callchain anyway, because we use zero-ed regs
>
> Now arguably, this is really stupid behaviour. Who in his right mind
> wants callchain output on BTS entries. And even if they do, BTS +
> precise_ip is nonsensical.
>
> So in my mind disallowing precise_ip on BTS would be the simplest fix.
>
> Hmm?
sounds ok, will post it
thanks,
jirka
Re: [RFC PATCH 01/13] arm: Fix mutual exclusion in arch_gettimeoffset
On Mon, Nov 12, 2018 at 03:12:39PM +1100, Finn Thain wrote:
> Implementations of arch_gettimeoffset are generally not re-entrant
> and assume that interrupts have been disabled. Unfortunately this
> pre-condition got broken in v2.6.32.
>
> Fixes: 5cfc8ee0bb51 ("ARM: convert arm to arch_gettimeoffset()")
> Signed-off-by: Finn Thain
This looks like a sensible fix for -stable backporting. But with m68k
converted over it might be time for these two arm platforms to either
be converted to the clocksource API or to be dropped..
Re: [PATCH v2] scripts/kconfig/merge_config: don't redefine 'y' to 'm'
On Sun, 11 Nov 2018 at 05:43, Masahiro Yamada
wrote:
>
> On Fri, Nov 9, 2018 at 4:45 AM Anders Roxell wrote:
> >
> > In today's merge_config.sh the order of the config fragment files dictates
> > the output of a config option. With this approach we will get different
> > .config files depending on the order of the config fragment files.
> >
> > So doing something like:
> > $ ./merge/kconfig/merge_config.sh selftest.config drm.config
> >
> > Where selftest.config defines DRM=y and drm.config defines DRM=m, the
> > result will be "DRM=m".
> >
> > Rework to add a switch to get builtin '=y' precedence over modules '=m',
> > this will result in "DRM=y". If we do something like this:
> >
> > $ ./merge/kconfig/merge_config.sh -y selftest.config drm.config
> >
> > Suggested-by: Arnd Bergmann
> > Signed-off-by: Anders Roxell
> > ---
> > scripts/kconfig/merge_config.sh | 34 +++--
> > 1 file changed, 28 insertions(+), 6 deletions(-)
> >
> > diff --git a/scripts/kconfig/merge_config.sh
> > b/scripts/kconfig/merge_config.sh
> > index da66e7742282..fcd18f642fc7 100755
> > --- a/scripts/kconfig/merge_config.sh
> > +++ b/scripts/kconfig/merge_config.sh
> > @@ -22,6 +22,7 @@
> >
> > clean_up() {
> > rm -f $TMP_FILE
> > + rm -f $MERGE_FILE
> > exit
> > }
> > trap clean_up HUP INT TERM
> > @@ -32,6 +33,7 @@ usage() {
> > echo " -monly merge the fragments, do not execute the make
> > command"
> > echo " -nuse allnoconfig instead of alldefconfig"
> > echo " -rlist redundant entries when merging fragments"
> > + echo " -ymake builtin have precedence over modules"
> > echo " -Odir to put generated output files. Consider setting
> > \$KCONFIG_CONFIG instead."
> > echo
> > echo "Used prefix: '$CONFIG_PREFIX'. You can redefine it with
> > \$CONFIG_ environment variable."
> > @@ -40,6 +42,8 @@ usage() {
> > RUNMAKE=true
> > ALLTARGET=alldefconfig
> > WARNREDUN=false
> > +BUILTIN=false
> > +BUILTIN_FLAG=false
>
>
> Could you move the initialization of BUILTIN_FLAG
> into the inner for-loop ?
>
>
>
> > OUTPUT=.
> > CONFIG_PREFIX=${CONFIG_-CONFIG_}
> >
> > @@ -64,6 +68,11 @@ while true; do
> > shift
> > continue
> > ;;
> > + "-y")
> > + BUILTIN=true
> > + shift
> > + continue
> > + ;;
> > "-O")
> > if [ -d $2 ];then
> > OUTPUT=$(echo $2 | sed 's/\/*$//')
> > @@ -105,13 +114,15 @@ MERGE_LIST=$*
> > SED_CONFIG_EXP="s/^\(# \)\{0,1\}\(${CONFIG_PREFIX}[a-zA-Z0-9_]*\)[=
> > ].*/\2/p"
> >
> > TMP_FILE=$(mktemp ./.tmp.config.XX)
> > +MERGE_FILE=$(mktemp ./.merge_tmp.config.XX)
> >
> > echo "Using $INITFILE as base"
> > cat $INITFILE > $TMP_FILE
> >
> > # Merge files, printing warnings on overridden values
> > -for MERGE_FILE in $MERGE_LIST ; do
> > - echo "Merging $MERGE_FILE"
> > +for ORIG_MERGE_FILE in $MERGE_LIST ; do
> > + cat $ORIG_MERGE_FILE > $MERGE_FILE
>
>
> This 'cat' should be moved after the check
> of the presence of '$ORIG_MERGE_FILE'.
>
>
> > + echo "Merging $ORIG_MERGE_FILE"
> > if [ ! -r "$MERGE_FILE" ]; then
>
> This check always returns false now.
>
>
> > echo "The merge file '$MERGE_FILE' does not exist. Exit."
> > >&2
> > exit 1
> > @@ -122,15 +133,26 @@ for MERGE_FILE in $MERGE_LIST ; do
> > grep -q -w $CFG $TMP_FILE || continue
> > PREV_VAL=$(grep -w $CFG $TMP_FILE)
> > NEW_VAL=$(grep -w $CFG $MERGE_FILE)
>
> Could you add 'BUILTIN_FLAG=false' here?
>
>
>
> > - if [ "x$PREV_VAL" != "x$NEW_VAL" ] ; then
> > - echo Value of $CFG is redefined by fragment
> > $MERGE_FILE:
> > + if [ "$BUILTIN" = "true" ] && [ "${NEW_VAL#CONFIG_*=}" =
> > "m" ] && [ "${PREV_VAL#CONFIG_*=}" = "y" ]; then
> > + echo Previous value: $PREV_VAL
> > + echo New value: $NEW_VAL
> > + echo -y passed, will not demote y to m
> > + echo
> > + BUILTIN_FLAG=true
> > + elif [ "x$PREV_VAL" != "x$NEW_VAL" ] ; then
> > + echo Value of $CFG is redefined by fragment
> > $ORIG_MERGE_FILE:
> > echo Previous value: $PREV_VAL
> > echo New value: $NEW_VAL
> > echo
> > elif [ "$WARNREDUN" = "true" ]; then
> > - echo Value of $CFG is redundant by fragment
> > $MERGE_FILE:
> > + echo Value of $CFG is redundant by fragment
> > $ORIG_MERGE_FILE:
> > + fi
> > + if [ "$BUILTIN_FLAG" = "false" ]; then
> > + sed -i "/$CFG[ =]/d" $TMP_FILE
> > +
[PATCH v3] scripts/kconfig/merge_config: don't redefine 'y' to 'm'
In today's merge_config.sh the order of the config fragment files dictates
the output of a config option. With this approach we will get different
.config files depending on the order of the config fragment files.
So doing something like:
$ ./merge/kconfig/merge_config.sh selftest.config drm.config
Where selftest.config defines DRM=y and drm.config defines DRM=m, the
result will be "DRM=m".
Rework to add a switch to get builtin '=y' precedence over modules '=m',
this will result in "DRM=y". If we do something like this:
$ ./merge/kconfig/merge_config.sh -y selftest.config drm.config
Suggested-by: Arnd Bergmann
Signed-off-by: Anders Roxell
---
scripts/kconfig/merge_config.sh | 37 ++---
1 file changed, 29 insertions(+), 8 deletions(-)
diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh
index 0ef906499646..9b89791b202c 100755
--- a/scripts/kconfig/merge_config.sh
+++ b/scripts/kconfig/merge_config.sh
@@ -22,6 +22,7 @@
clean_up() {
rm -f $TMP_FILE
+ rm -f $MERGE_FILE
exit
}
trap clean_up HUP INT TERM
@@ -32,6 +33,7 @@ usage() {
echo " -monly merge the fragments, do not execute the make command"
echo " -nuse allnoconfig instead of alldefconfig"
echo " -rlist redundant entries when merging fragments"
+ echo " -ymake builtin have precedence over modules"
echo " -Odir to put generated output files. Consider setting
\$KCONFIG_CONFIG instead."
echo
echo "Used prefix: '$CONFIG_PREFIX'. You can redefine it with \$CONFIG_
environment variable."
@@ -40,6 +42,7 @@ usage() {
RUNMAKE=true
ALLTARGET=alldefconfig
WARNREDUN=false
+BUILTIN=false
OUTPUT=.
CONFIG_PREFIX=${CONFIG_-CONFIG_}
@@ -64,6 +67,11 @@ while true; do
shift
continue
;;
+ "-y")
+ BUILTIN=true
+ shift
+ continue
+ ;;
"-O")
if [ -d $2 ];then
OUTPUT=$(echo $2 | sed 's/\/*$//')
@@ -106,32 +114,45 @@
SED_CONFIG_EXP1="s/^\(${CONFIG_PREFIX}[a-zA-Z0-9_]*\)=.*/\1/p"
SED_CONFIG_EXP2="s/^# \(${CONFIG_PREFIX}[a-zA-Z0-9_]*\) is not set$/\1/p"
TMP_FILE=$(mktemp ./.tmp.config.XX)
+MERGE_FILE=$(mktemp ./.merge_tmp.config.XX)
echo "Using $INITFILE as base"
cat $INITFILE > $TMP_FILE
# Merge files, printing warnings on overridden values
-for MERGE_FILE in $MERGE_LIST ; do
- echo "Merging $MERGE_FILE"
- if [ ! -r "$MERGE_FILE" ]; then
- echo "The merge file '$MERGE_FILE' does not exist. Exit." >&2
+for ORIG_MERGE_FILE in $MERGE_LIST ; do
+ echo "Merging $ORIG_MERGE_FILE"
+ if [ ! -r "$ORIG_MERGE_FILE" ]; then
+ echo "The merge file '$ORIG_MERGE_FILE' does not exist. Exit."
>&2
exit 1
fi
+ cat $ORIG_MERGE_FILE > $MERGE_FILE
CFG_LIST=$(sed -n -e "$SED_CONFIG_EXP1" -e "$SED_CONFIG_EXP2"
$MERGE_FILE)
for CFG in $CFG_LIST ; do
grep -q -w $CFG $TMP_FILE || continue
PREV_VAL=$(grep -w $CFG $TMP_FILE)
NEW_VAL=$(grep -w $CFG $MERGE_FILE)
- if [ "x$PREV_VAL" != "x$NEW_VAL" ] ; then
- echo Value of $CFG is redefined by fragment $MERGE_FILE:
+ BUILTIN_FLAG=false
+ if [ "$BUILTIN" = "true" ] && [ "${NEW_VAL#CONFIG_*=}" = "m" ]
&& [ "${PREV_VAL#CONFIG_*=}" = "y" ]; then
+ echo Previous value: $PREV_VAL
+ echo New value: $NEW_VAL
+ echo -y passed, will not demote y to m
+ echo
+ BUILTIN_FLAG=true
+ elif [ "x$PREV_VAL" != "x$NEW_VAL" ] ; then
+ echo Value of $CFG is redefined by fragment
$ORIG_MERGE_FILE:
echo Previous value: $PREV_VAL
echo New value: $NEW_VAL
echo
elif [ "$WARNREDUN" = "true" ]; then
- echo Value of $CFG is redundant by fragment $MERGE_FILE:
+ echo Value of $CFG is redundant by fragment
$ORIG_MERGE_FILE:
+ fi
+ if [ "$BUILTIN_FLAG" = "false" ]; then
+ sed -i "/$CFG[ =]/d" $TMP_FILE
+ else
+ sed -i "/$CFG[ =]/d" $MERGE_FILE
fi
- sed -i "/$CFG[ =]/d" $TMP_FILE
done
cat $MERGE_FILE >> $TMP_FILE
done
--
2.19.1
Re: WARNING: CPU: 0 PID: 0 at drivers/irqchip/irq-gic-v3-its.c
On Fri, 09 Nov 2018 18:41:03 +,
Qian Cai wrote:
>
>
>
> > On Nov 9, 2018, at 12:41 PM, Marc Zyngier wrote:
> >
> > On 09/11/18 17:28, Sudeep Holla wrote:
> >> On Fri, Nov 9, 2018 at 4:10 PM Marc Zyngier wrote:
> >>>
> >> [...]
> >>
> >>>
> >>> See bb42ca474010 and d003d029cea8 for details.
> >>>
> >>> Now, activating this workaround leads to lockdep being really angry,
> >>> most likely because the cpus_read_lock is not taken, which is a change
> >>> in behaviour...
> >>>
> >>> I'm trying to dig into this now.
> >>>
> >>
> >> Yes we found similar issue in kernel/sched/core.c sched_init_smp
> >> There's a fix with detailed description in -next
> >> (Commit 40fa3780bac2 ("sched/core: Take the hotplug lock in
> >> sched_init_smp()")
> >>
> >> The behaviour changed since commit cb538267ea1e ("jump_label/lockdep:
> >> Assert we hold the hotplug lock for _cpuslocked() operations")
> >
> > I indeed came to the same conclusion, but the fix is slightly less than
> > obvious. I have the following arm64-specific crap, but it is pretty
> > terrible:
> >
> > diff --git a/arch/arm64/kernel/time.c b/arch/arm64/kernel/time.c
> > index f258636273c9..9e96e9eaca9b 100644
> > --- a/arch/arm64/kernel/time.c
> > +++ b/arch/arm64/kernel/time.c
> > @@ -36,6 +36,7 @@
> > #include
> > #include
> > #include
> > +#include
> >
> > #include
> >
> > @@ -69,7 +70,9 @@ void __init time_init(void)
> > u32 arch_timer_rate;
> >
> > of_clk_init(NULL);
> > + cpus_read_lock();
> > timer_probe();
> > + cpus_read_unlock();
> >
> > tick_setup_hrtimer_broadcast();
> >
> > Qian, can you please let me know if this helps? If it does, we'll have
> > to think of something a bit better…
> After applied the above patch, the original warning is gone but there
> Is now a new warning.
[...]
Which was ful;ly expected, given that I've taken the cpu lock at some
semi-random location. I'll try to talk to PeterZ this week to try and
solve this.
Thanks,
M.
--
Jazz is not dead, it just smell funny.
Re: [PATCH] scripts/setlocalversion: Improve -dirty check with git-status --no-optional-locks
On Sun, Nov 11, 2018 at 9:59 PM Alexander Kapshuk
wrote:
>
> On Sun, Nov 11, 2018 at 7:41 PM Genki Sky wrote:
> >
> > Hi Alexander,
> >
> > On Sun, 11 Nov 2018 16:48:38 +0200, Alexander Kapshuk
> > wrote:
> > > Piping the output of the git command to grep and using the return status
> > > of grep as the test condition within the if block, would be sufficient
> > > to determine whether or not '-dirty' should be printed.
> > >
> > > Sample run:
> > > % if git --no-optional-locks \
> > > status -uno --porcelain \
> > > 2>/dev/null |
> > > grep -qv '^.. scripts/package'
> > > then
> > > printf '%s' -dirty
> > > fi
> >
> > I don't think this works well for us. We need to check whether
> > --no-optional-locks is available before using the output to determine
> > whether the tree is dirty or not. If it's not available, we have to
> > fall back on diff-index. Let me know if I'm misreading you.
>
> It was I who failed to read the proposed patch in its entirety in the
> first place. I did not get the full picture as a result. My apologies.
>
> Would something like this work for you?
>
> local git_status
> git_status=$(git --no-optional-locks status -uno --porcelain 2>/dev/null)
> test $? -eq 0 ||
> git_status=$(git diff-index --name-only HEAD)
> if printf '%s' "$git_status" | grep -qv scripts/package; then
> printf '%s' -dirty
> fi
An even simpler approach would be:
{
git --no-optional-locks status -uno --porcelain 2>/dev/null ||
git diff-index --name-only HEAD
} | grep -qv scripts/package &&
printf '%s' -dirty
Sample run:
cmd
sh: cmd: command not found
{
cmd 2>/dev/null ||
date
} | grep -q 2018 &&
printf '%s' ok
ok
Re: [PATCH v4 06/10] x86/alternative: use temporary mm for text poking
On Mon, Nov 12, 2018 at 04:46:46AM +0100, Ingo Molnar wrote: > > * Peter Zijlstra wrote: > > > On Sun, Nov 11, 2018 at 08:53:07PM +, Nadav Amit wrote: > > > > > >> + /* > > > >> + * The lock is not really needed, but this allows to avoid > > > >> open-coding. > > > >> + */ > > > >> + ptep = get_locked_pte(poking_mm, poking_addr, &ptl); > > > >> + > > > >> + /* > > > >> + * If we failed to allocate a PTE, fail. This should *never* > > > >> happen, > > > >> + * since we preallocate the PTE. > > > >> + */ > > > >> + if (WARN_ON_ONCE(!ptep)) > > > >> + goto out; > > > > > > > > Since we hard rely on init getting that right; can't we simply get rid > > > > of this? > > > > > > This is a repeated complaint of yours, which I do not feel comfortable > > > with. > > > One day someone will run some static analysis tool and start finding that > > > all these checks are missing. > > > > > > The question is why do you care about them. > > > > Mostly because they should not be happening, ever. > > Since get_locked_pte() might in principle return NULL, it's an entirely > routine pattern to check the return for NULL. This will save reviewer > time in the future. The reviewer can read a comment. > > > If it is because they affect the > > > generated code and make it less efficient, I can fully understand and > > > perhaps > > > we should have something like PARANOID_WARN_ON_ONCE() which compiles into > > > nothing > > > unless a certain debug option is set. > > > > > > If it is about the way the source code looks - I guess it doesn’t sore my > > > eyes as hard as some other stuff, and I cannot do much about it (other > > > than > > > removing it as you asked). > > > > And yes on the above two points. It adds both runtime overhead (albeit > > trivially small) and code complexity. > > It's trivially small cycle level overhead in something that will be > burdened by two TLB flushes anyway is is utterly slow. The code complexity not so much. > > > >> +out: > > > >> + if (memcmp(addr, opcode, len)) > > > >> + r = -EFAULT; > > > > > > > > How could this ever fail? And how can we reliably recover from that? > > > > > > This code has been there before (with slightly uglier code). Before this > > > patch, a BUG_ON() was used here. However, I noticed that kgdb actually > > > checks that text_poke() succeeded after calling it and gracefully fail. > > > However, this was useless, since text_poke() would panic before kgdb gets > > > the chance to do anything (see patch 7). > > > > Yes, I know it was there before, and I did see kgdb do it too. But aside > > from that out-label case, which we also should never hit, how can we > > realistically ever fail that memcmp()? > > > > If we fail here, something is _seriously_ buggered. > > So wouldn't it be better to just document and verify our assumptions of > this non-trivial code by using return values intelligently? The thing is, I don't think there is realistically anything the caller can do; our text is not what we expect it to be, that is a fairly fundamentally buggered situation to be in. I'm fine with validating it; I'm as paranoid as the next guy; but passing along that information seems pointless. At best we can try poking again, but that's not going to help much if it failed the first time around.
Re: [PATCH 01/10] irqdomain: Add interface to request an irq domain
Julien,
On 09.11.18 09:05:11, Julien Thierry wrote:
> On 08/11/18 15:05, Richter, Robert wrote:
> >>>+static void irq_domain_handle_requests(struct fwnode_handle *fwnode,
> >>>+enum irq_domain_bus_token bus_token)
> >>>+{
> >>>+ struct irq_domain *domain;
> >>>+ struct irq_domain_request *request;
> >>>+
> >>>+ if (!fwnode)
> >>>+ return;
> >>>+redo:
> >>>+ domain = irq_find_matching_fwnode(fwnode, bus_token);
> >>>+ if (!domain)
> >>>+ return;
> >>>+
> >>>+ mutex_lock(&irq_domain_mutex);
> >>>+
> >>
> >>Why do we need to take the mutex before checking the domain fields?
> >>Can't we delay it?
> >
> >The list is protected by the mutex. irq_find_matching_fwnode() also
> >accesses the list and must use the same mutex.
> >
> >>
> >>>+ if ((domain->fwnode != fwnode) && (domain->bus_token != bus_token)) {
> >>
> >>Why do we even need that check?
> >
> >The domain token might have changed after irq_find_matching_fwnode()
> >and before mutex_lock(), we do a recheck here. Some sort of try-lock.
> >
> >Note: I found the check being wrong here, it needs to be corrected to:
> >
> > if ((domain->fwnode != fwnode) || (domain->bus_token != bus_token)) {
> >
> >>
> >>Isn't the point of passing fwnode and bus_token to
> >>irq_find_matching_fwnode to find a domain with those properties?
> >
> >Yes, but properties may change and also the list itself.
>
> Hmmm, that check is unrelated to the list, you're just checking the
> domain you just retrieved.
>
> Can you clarify which properties may change? If the irq_domain fields
> can change (I guess if e.g. another cpu modifies the domain with
> irq_domain_update_bus_token), they could still be changed between the
> moment you retrieved the domain and the moment you call the handler. Why
> is that not an issue if we worried about properties changing before
> removing the request from the list?
>
> Maybe some comment would help here.
The check makes sure we can use the irq_domain_requests list for the
serialization of irq domain updates. Suppose the following:
Thread1 Thread2
~ mutex
fwnode
token1
handler1request_fwnode()
~
domain1
fwnode
token1 find_fwnode()
~
domain1
fwnode
token1 find_fwnode()
~
domain1
fwnode
token1
handler1call_handler()
~
domain1
fwnode
token2 update_token()
~
domain2
fwnode
token1 update_token()
~
fwnode
token1
handler1request_fwnode(), reschedule request
~
domain1 < called with wrong domain, should be
domain2
fwnode
token1
handler1call_handler()
~
The check handles a corner case and as such the conditions for
triggering it are rare and might look a bit constructed, but it *can*
happen. So see the check more like an assertion in the code that does
not hurt much. How about the following comment:?
/*
* For serialization of irq domain updates make sure to handle
* (and remove) the request only if the domain still matches
* the request.
*/
>
> >
> >>
> >>>+ mutex_unlock(&irq_domain_mutex);
> >>>+ goto redo;
> >>>+ }
> >>>+
> >>>+ list_for_each_entry(request, &irq_domain_requests, list) {
> >>
> >>Shouldn't you use list_for_each_safe if you want to remove elements of
> >>the list inside the loop?
> >
> >No, we do a complete redo again without further iterating the list. We
> >need to do this since the handler must be called with the mutex
> >unlocked (to be able to manipulate the irq domain list in the callback
> >and to be in non-atomic context). After we unlocked the mutex, we must
> >restart again as the list may have changed.
> >
> >>
> >>>+ if (request->fwnode != fwnode ||
> >>>+ request->bus_token != bus_token)
> >>>+ continue;
> >>>+
> >>>+ list_del(&request->list);
> >>>+ mutex_unlock(&irq_domain_mutex);
> >>>+
> >>>+ irq_domain_call_handler(domain, request->callback,
> >>>+ request->name, request->priv);
> >>>+ irq_domain_free_request(request);
> >>>+
> >>>+ goto redo;
> >>>+ }
> >>>+
> >>>+ mutex_unlock(&irq_domain_mutex);
> >>>+}
> >>>+
> >>>+static int __init irq_domain_drain_requests(void)
> >>>+{
> >>>+ struct irq_domain_request *request;
> >>>+ struct irq_domain *domain;
> >>>+ int ret = 0;
> >>>+redo:
Re: [RFC PATCH v1 2/2] proc: add /proc//thread_state
On Mon, Nov 12, 2018 at 06:31:47AM +0100, Ingo Molnar wrote: > > * Peter Zijlstra wrote: > > > On Thu, Nov 08, 2018 at 07:32:46AM +0100, Ingo Molnar wrote: > > > > > > * Aubrey Li wrote: > > > > > > > Expose the per-task cpu specific thread state value, it's helpful > > > > for userland to classify and schedule the tasks by different policies > > > > > > That's pretty vague - what exactly would use this information? I'm sure > > > you have a usecase in mind - could you please describe it? > > > > Yeah, "thread_state" is a pretty terrible name for this. The use-case is > > detectoring which tasks use AVX3 such that a userspace component (think > > job scheduler) can cluster them together. > > I'd prefer the kernel to do such clustering... I think that is a next step. Also, while the kernel can do this at a best effort basis, it cannot take into account things the kernel doesn't know about, like high priority job peak load etc.., things a job scheduler would know. Then again, a job scheduler would likely already know about the AVX state anyway.
Re: [PATCH tip/core/rcu 23/41] sched: Replace synchronize_sched() with synchronize_rcu()
On Sun, Nov 11, 2018 at 06:24:55PM -0800, Paul E. McKenney wrote:
> > > There were quite a few commits involved in making this happen. Perhaps
> > > the most pertinent are these:
> > >
> > > 3e3100989869 ("rcu: Defer reporting RCU-preempt quiescent states when
> > > disabled")
> > > 45975c7d21a1 ("rcu: Define RCU-sched API in terms of RCU for Tree RCU
> > > PREEMPT builds")
> >
> > The latter; it does not mention that this will possible make
> > synchronize_sched() quite a bit more expensive on PREEMPT=y builds :/
>
> In theory, sure. In practice, people have switched any number of
> things from RCU-sched to RCU and back without problems.
Still, better safe than sorry. It was a rather big change in behaviour,
so it wouldn't have been strange to call that out.
> > But for PREEMPT=y synchronize_sched() can be quite a bit shorter than
> > synchronize_rcu(), since we don't have to wait for preempted read side
> > stuff.
>
> Again, there are quite a few places that have managed that transition
> without issue. Why do you expect this change to have problems that have
> not been seen elsewhere?
I'm not, I'm just taking issue with the Changelog.
> > Again, the patch didn't say that.
> >
> > If the Changelog would've read something like:
> >
> > "Since synchronize_sched() is now equivalent to synchronize_rcu(),
> > replace the synchronize_sched() usage such that we can eventually remove
> > the interface."
> >
> > It would've been clear that the patch is a nop and what the purpose
> > was.
>
> I can easily make that change.
Please, sufficient doesn't imply necessary etc.. A changelog should
always clarify why we do the patch.
Re: [RFC PATCH 06/13] m68k: Drop ARCH_USES_GETTIMEOFFSET
Hi Finn,
Thanks for your patch!
On Mon, Nov 12, 2018 at 5:46 AM Finn Thain wrote:
> The functions that implement arch_gettimeoffset are re-used by
> new clocksource drivers in subsequent patches.
Disabling this first affects functionality during bisection, right?
> --- a/arch/m68k/amiga/config.c
> +++ b/arch/m68k/amiga/config.c
> @@ -95,8 +95,6 @@ static char amiga_model_name[13] = "Amiga ";
> static void amiga_sched_init(irq_handler_t handler);
> static void amiga_get_model(char *model);
> static void amiga_get_hardware_list(struct seq_file *m);
> -/* amiga specific timer functions */
> -static u32 amiga_gettimeoffset(void);
> extern void amiga_mksound(unsigned int count, unsigned int ticks);
> static void amiga_reset(void);
> extern void amiga_init_sound(void);
> @@ -386,7 +384,6 @@ void __init config_amiga(void)
> mach_init_IRQ= amiga_init_IRQ;
> mach_get_model = amiga_get_model;
> mach_get_hardware_list = amiga_get_hardware_list;
> - arch_gettimeoffset = amiga_gettimeoffset;
In addition, won't this lead to "function defined statically but not called'
warnings?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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
Re: [RFC PATCH 07/13] m68k: amiga: Convert to clocksource API
Hi Finn,
On Mon, Nov 12, 2018 at 5:46 AM Finn Thain wrote:
> Add a platform clocksource by adapting the existing arch_gettimeoffset
> implementation.
>
> Signed-off-by: Finn Thain
Thanks for your patch!
> --- a/arch/m68k/amiga/config.c
> +++ b/arch/m68k/amiga/config.c
> @@ -17,6 +17,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -461,7 +462,30 @@ void __init config_amiga(void)
> *(unsigned char *)ZTWO_VADDR(0xde0002) |= 0x80;
> }
>
> +static u64 amiga_read_clk(struct clocksource *cs);
> +
> +static struct clocksource amiga_clk = {
> + .name = "ciab",
> + .rating = 250,
> + .read = amiga_read_clk,
> + .mask = CLOCKSOURCE_MASK(32),
> + .flags = CLOCK_SOURCE_IS_CONTINUOUS,
> +};
> +
> static unsigned short jiffy_ticks;
> +static u32 clk_total;
Shouldn't clk_total be u64?
> +
> +static irqreturn_t ciab_timer_handler(int irq, void *data)
> +{
> + irq_handler_t timer_routine = data;
> + unsigned long flags;
> +
> + local_irq_save(flags);
> + clk_total += jiffy_ticks;
> + local_irq_restore(flags);
> +
> + return timer_routine(irq, data);
> +}
[...]
> -static u32 amiga_gettimeoffset(void)
> +static u64 amiga_read_clk(struct clocksource *cs)
> {
> unsigned long flags;
> unsigned short hi, lo, hi2;
> @@ -512,14 +537,13 @@ static u32 amiga_gettimeoffset(void)
> if (ticks > jiffy_ticks / 2)
> /* check for pending interrupt */
> if (cia_set_irq(&ciab_base, 0) & CIA_ICR_TA)
> - offset = 1;
> + offset = jiffy_ticks;
>
> local_irq_restore(flags);
>
> ticks = jiffy_ticks - ticks;
> - ticks = (1 * ticks) / jiffy_ticks;
>
> - return (ticks + offset) * 1000;
> + return clk_total + ticks + offset;
... to return a full 64-bit value here?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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
nohz: ignore explicit nohz=off in case of nohz_full running
Hi,
It seems "nohz=" is not compatible with "nohz_full=" or "isolcpus=" by now.
when both "nohz=off" and "nohz_full=[cpulist]" or "isolcpus=nohz,domain" is
configured in cmdline,
we enter an ambiguous status, that is, we do unbound timers, watchdog, RCU
isolation, even show nohz_full cpu in sysfs,
but we never enter tickless mode because one-shot mode is disable by "nohz=off"
in tick_nohz_switch_to_nohz().
**
[How I resolve it]
**
Since we want to centralize the isolation management by housekeeping subsystem,
we regard "nohz_full=" or "isolcpus=" as the controller.
Just ignore explicit nohz=off when nohz_full=[cpulist] or isolcpus=nohz,domain
is configured.
---
kernel/time/tick-sched.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c index
5b33e2f..d30d783 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -414,6 +414,8 @@ void __init tick_nohz_init(void)
if (!tick_nohz_full_running)
return;
+ if (!tick_nohz_enabled)
+ pr_warn("NO_HZ: NOHZ is disable explicitly, ignore it in case
of NOHZ_FULL running\n");
/*
* Full dynticks uses irq work to drive the tick rescheduling on safe
* locking contexts. But then we need irq work to raise its own @@
-1182,7 +1184,7 @@ static void tick_nohz_handler(struct clock_event_device *dev)
static inline void tick_nohz_activate(struct tick_sched *ts, int mode) {
- if (!tick_nohz_enabled)
+ if (!tick_nohz_enabled && !tick_nohz_full_enabled())
return;
ts->nohz_mode = mode;
/* One update is enough */
@@ -1198,7 +1200,7 @@ static void tick_nohz_switch_to_nohz(void)
struct tick_sched *ts = this_cpu_ptr(&tick_cpu_sched);
ktime_t next;
- if (!tick_nohz_enabled)
+ if (!tick_nohz_enabled && !tick_nohz_full_enabled())
return;
if (tick_switch_to_oneshot(tick_nohz_handler))
--
1.7.12.4
[PATCH] backlight: pwm_bl: fix devicetree parsing with auto-generated brightness tables
From: Heiko Stuebner
Commit 88ba95bedb79 ("backlight: pwm_bl: Compute brightness of LED linearly
to human eye") made the parse-dt function return early when using an auto-
generated brightness-table, but didn't take into account that some more
settings were handled below the brightness handling, like power-on-delays
and also setting the pdata enable-gpio to -EINVAL.
This surfaces for example in the case of a backlight without any
enable-gpio which then tries to use gpio-0 in error.
Fix this by simply moving the trailing settings above the brightness
handling.
Fixes: 88ba95bedb79 ("backlight: pwm_bl: Compute brightness of LED linearly to
human eye")
Signed-off-by: Heiko Stuebner
---
drivers/video/backlight/pwm_bl.c | 19 ++-
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index bcd08b41765d..b7b5b31f3824 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -272,6 +272,16 @@ static int pwm_backlight_parse_dt(struct device *dev,
memset(data, 0, sizeof(*data));
+ /*
+* These values are optional and set as 0 by default, the out values
+* are modified only if a valid u32 value can be decoded.
+*/
+ of_property_read_u32(node, "post-pwm-on-delay-ms",
+&data->post_pwm_on_delay);
+ of_property_read_u32(node, "pwm-off-delay-ms", &data->pwm_off_delay);
+
+ data->enable_gpio = -EINVAL;
+
/*
* Determine the number of brightness levels, if this property is not
* set a default table of brightness levels will be used.
@@ -384,15 +394,6 @@ static int pwm_backlight_parse_dt(struct device *dev,
data->max_brightness--;
}
- /*
-* These values are optional and set as 0 by default, the out values
-* are modified only if a valid u32 value can be decoded.
-*/
- of_property_read_u32(node, "post-pwm-on-delay-ms",
-&data->post_pwm_on_delay);
- of_property_read_u32(node, "pwm-off-delay-ms", &data->pwm_off_delay);
-
- data->enable_gpio = -EINVAL;
return 0;
}
--
2.18.0
Re: [RFC PATCH 06/13] m68k: Drop ARCH_USES_GETTIMEOFFSET
On Mon, 12 Nov 2018, Geert Uytterhoeven wrote:
> Hi Finn,
>
> Thanks for your patch!
>
> On Mon, Nov 12, 2018 at 5:46 AM Finn Thain wrote:
> > The functions that implement arch_gettimeoffset are re-used by
> > new clocksource drivers in subsequent patches.
>
> Disabling this first affects functionality during bisection, right?
>
It means that all platforms have to use the 'jiffies' clocksource.
At the end of the series, only apollo, q40, sun3 & sun3x continue to use
that clocksource.
> > --- a/arch/m68k/amiga/config.c
> > +++ b/arch/m68k/amiga/config.c
> > @@ -95,8 +95,6 @@ static char amiga_model_name[13] = "Amiga ";
> > static void amiga_sched_init(irq_handler_t handler);
> > static void amiga_get_model(char *model);
> > static void amiga_get_hardware_list(struct seq_file *m);
> > -/* amiga specific timer functions */
> > -static u32 amiga_gettimeoffset(void);
> > extern void amiga_mksound(unsigned int count, unsigned int ticks);
> > static void amiga_reset(void);
> > extern void amiga_init_sound(void);
> > @@ -386,7 +384,6 @@ void __init config_amiga(void)
> > mach_init_IRQ= amiga_init_IRQ;
> > mach_get_model = amiga_get_model;
> > mach_get_hardware_list = amiga_get_hardware_list;
> > - arch_gettimeoffset = amiga_gettimeoffset;
>
> In addition, won't this lead to "function defined statically but not
> called' warnings?
>
I expect so. I haven't compile-tested each step in the series; but I'll do
that before I send v2. Thanks for the reminder.
There are no new warnings at the end of the series.
--
> Gr{oetje,eeting}s,
>
> Geert
>
>
Re: [PATCH 0/3] ARM: dts: stm32: add dmas to stm32mp157c timers
Hi Fabrice On 10/3/18 5:11 PM, Fabrice Gasnier wrote: This series adds dmas description to stm32mp157c device tree file. But dmas are kept disabled by default on all boards. They are only necessary for PWM capture. So, spare them for other usage by default. Fabrice Gasnier (3): ARM: dts: stm32: Add dmas to timer on stm32mp157c ARM: dts: stm32: don't use timers dmas on stm32mp157c-ed1 ARM: dts: stm32: don't use timers dmas on stm32mp157c-ev1 arch/arm/boot/dts/stm32mp157c-ed1.dts | 3 ++ arch/arm/boot/dts/stm32mp157c-ev1.dts | 7 + arch/arm/boot/dts/stm32mp157c.dtsi| 58 +++ 3 files changed, 68 insertions(+) Series applied on stm32-next. Thanks! Alex
Re: [RFC PATCH 07/13] m68k: amiga: Convert to clocksource API
On Mon, 12 Nov 2018, Geert Uytterhoeven wrote:
> Hi Finn,
>
> On Mon, Nov 12, 2018 at 5:46 AM Finn Thain wrote:
> > Add a platform clocksource by adapting the existing arch_gettimeoffset
> > implementation.
> >
> > Signed-off-by: Finn Thain
>
> Thanks for your patch!
>
Thanks for your review.
> > --- a/arch/m68k/amiga/config.c
> > +++ b/arch/m68k/amiga/config.c
> > @@ -17,6 +17,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -461,7 +462,30 @@ void __init config_amiga(void)
> > *(unsigned char *)ZTWO_VADDR(0xde0002) |= 0x80;
> > }
> >
> > +static u64 amiga_read_clk(struct clocksource *cs);
> > +
> > +static struct clocksource amiga_clk = {
> > + .name = "ciab",
> > + .rating = 250,
> > + .read = amiga_read_clk,
> > + .mask = CLOCKSOURCE_MASK(32),
> > + .flags = CLOCK_SOURCE_IS_CONTINUOUS,
> > +};
> > +
> > static unsigned short jiffy_ticks;
> > +static u32 clk_total;
>
> Shouldn't clk_total be u64?
>
Well, it was intentionally u32, and amiga_clk.mask was intentionally
CLOCKSOURCE_MASK(32)...
> > +
> > +static irqreturn_t ciab_timer_handler(int irq, void *data)
> > +{
> > + irq_handler_t timer_routine = data;
> > + unsigned long flags;
> > +
> > + local_irq_save(flags);
> > + clk_total += jiffy_ticks;
> > + local_irq_restore(flags);
> > +
> > + return timer_routine(irq, data);
> > +}
>
> [...]
>
> > -static u32 amiga_gettimeoffset(void)
> > +static u64 amiga_read_clk(struct clocksource *cs)
> > {
> > unsigned long flags;
> > unsigned short hi, lo, hi2;
> > @@ -512,14 +537,13 @@ static u32 amiga_gettimeoffset(void)
> > if (ticks > jiffy_ticks / 2)
> > /* check for pending interrupt */
> > if (cia_set_irq(&ciab_base, 0) & CIA_ICR_TA)
> > - offset = 1;
> > + offset = jiffy_ticks;
> >
> > local_irq_restore(flags);
> >
> > ticks = jiffy_ticks - ticks;
> > - ticks = (1 * ticks) / jiffy_ticks;
> >
> > - return (ticks + offset) * 1000;
> > + return clk_total + ticks + offset;
>
> ... to return a full 64-bit value here?
>
It's certainly possible. Is it better? It sounds like a question about the
cost of wrap-around in the clocksource core vs. the cost of 64-bit
additions in the arch code.
Looking at __clocksource_update_freq_scale() there are some consequences
to choosing a 32-bit mask --
* For clocksources which have a mask > 32-bit
* we need to limit the max sleep time to have a good
* conversion precision.
I don't know the answer, sorry.
--
> Gr{oetje,eeting}s,
>
> Geert
>
>
Re: [PATCH v4 4/4] dt-bindings: iio: adc: Add docs for ad7124
On Du, 2018-11-11 at 12:19 +, Jonathan Cameron wrote:
>
> On Fri, 9 Nov 2018 17:43:00 +0200
> Stefan Popa wrote:
>
> >
> >
> > Add support for Analog Devices AD7124 4-channels and 8-channels ADC.
> >
> > Signed-off-by: Stefan Popa
> Your example still includes the other things that I think you have now
> dropped.
> gain and odr.
>
You are right! I am sorry about this.
>
> >
> >
> > ---
> > Changes in v2:
> > - Nothing changed.
> > Changes in v3:
> > - Removed the "adi,channels" property.
> > - Used the "reg" property to get the channel number and "adi,diff-
> > channels"
> > for the differential pins. The "adi,channel-number" property was
> > removed.
> > - adi,bipolar is of boolean type.
> > Changes in v4:
> > - Used the bipolar and diff-channels properties defined in the new
> > adc.txt doc.
> >
> > .../devicetree/bindings/iio/adc/adi,ad7124.txt | 81
> > ++
> > MAINTAINERS| 1 +
> > 2 files changed, 82 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/iio/adc/adi,ad7124.txt
> >
> > diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7124.txt
> > b/Documentation/devicetree/bindings/iio/adc/adi,ad7124.txt
> > new file mode 100644
> > index 000..fa0c43b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7124.txt
> > @@ -0,0 +1,81 @@
> > +Analog Devices AD7124 ADC device driver
> > +
> > +Required properties for the AD7124:
> > + - compatible: Must be one of "adi,ad7124-4" or "adi,ad7124-8"
> > + - reg: SPI chip select number for the device
> > + - spi-max-frequency: Max SPI frequency to use
> > + see: Documentation/devicetree/bindings/spi/spi-bus.txt
> > + - clocks: phandle to the master clock (mclk)
> > + see: Documentation/devicetree/bindings/clock/clock-
> > bindings.txt
> > + - clock-names: Must be "mclk".
> > + - interrupts: IRQ line for the ADC
> > + see: Documentation/devicetree/bindings/interrupt-
> > controller/interrupts.txt
> Given the driver doesn't currently use it, should perhaps be optional?
>
The interrupt is actually required by ad_sigma_delta.
>
> >
> >
> > +
> > + Required properties:
> > + * #address-cells: Must be 1.
> > + * #size-cells: Must be 0.
> > +
> > + Subnode(s) represent the external channels which are
> > connected to the ADC.
> > + Each subnode represents one channel and has the following
> > properties:
> > + Required properties:
> > + * reg: The channel number. It can have up to 4
> > channels on ad7124-4
> > + and 8 channels on ad7124-8, numbered from 0
> > to 15.
> > + * diff-channels: see:
> > Documentation/devicetree/bindings/iio/adc/adc.txt
> > +
> > + Optional properties:
> > + * bipolar: see:
> > Documentation/devicetree/bindings/iio/adc/adc.txt
> > + * adi,reference-select: Select the reference
> > source to use when
> > + converting on the the specific channel.
> > Valid values are:
> > + 0: REFIN1(+)/REFIN1(−).
> > + 1: REFIN2(+)/REFIN2(−).
> > + 3: AVDD
> > + If this field is left empty, internal
> > reference is selected.
> > +
> > +Optional properties:
> > + - refin1-supply: refin1 supply can be used as reference for
> > conversion.
> > + - refin2-supply: refin2 supply can be used as reference for
> > conversion.
> > + - avdd-supply: avdd supply can be used as reference for
> > conversion.
> > +
> > +Example:
> > + adc@0 {
> > + compatible = "adi,ad7124-4";
> > + reg = <0>;
> > + spi-max-frequency = <500>;
> > + interrupts = <25 2>;
> > + interrupt-parent = <&gpio>;
> > + refin1-supply = <&adc_vref>;
> > + clocks = <&ad7124_mclk>;
> > + clock-names = "mclk";
> > +
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + channel@0 {
> > + reg = <0>;
> > + adi,diff-channels = <0 1>;
> > + adi,reference-select = <0>;
> > + adi,gain = <2>;
> > + adi,odr-hz = <10>;
> I think you have dropped these two..
>
> >
> >
> > + };
> > +
> > + channel@1 {
> > + reg = <1>;
> > + adi,bipolar;
> > + adi,diff-channels = <2 3>;
> > + adi,reference-select = <0>;
> > + adi,gain = <4>;
> > + adi,odr-hz = <50>;
> > + };
> > +
> > + channel@2 {
> > + reg = <2>;
> > + adi,diff-channels = <4 5>;
> > + adi,gain = <128>;
> > + adi,odr-hz = <19200>;
> > + };
> > +
> > + channel@3
[PATCH V4] binder: ipc namespace support for android binder
Currently android's binder is not isolated by ipc namespace. Since binder
is a form of IPC and therefore should be tied to ipc namespace. With this
patch, we can run multiple instances of android container on one host.
This patch move "binder_procs" and "binder_context" into ipc_namespace,
driver will find the context from it when opening. For debugfs, binder_proc
is namespace-aware, but not for binder dead nodes, binder_stats and
binder_transaction_log_entry (we added ipc inum to trace it).
Signed-off-by: chouryzhou
Reviewed-by: Davidlohr Bueso
---
drivers/android/binder.c | 133 --
include/linux/ipc_namespace.h | 15 +
ipc/namespace.c | 10 +++-
3 files changed, 125 insertions(+), 33 deletions(-)
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index cb30a524d16d..453265505b04 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -67,6 +67,8 @@
#include
#include
#include
+#include
+#include
#include
#include
#include
@@ -80,13 +82,21 @@
#include "binder_alloc.h"
#include "binder_trace.h"
+
+#ifndef CONFIG_IPC_NS
+static struct ipc_namespace binder_ipc_ns = {
+ .ns.inum = PROC_IPC_INIT_INO,
+};
+
+#define ipcns (&binder_ipc_ns)
+#else
+#define ipcns (current->nsproxy->ipc_ns)
+#endif
+
static HLIST_HEAD(binder_deferred_list);
static DEFINE_MUTEX(binder_deferred_lock);
static HLIST_HEAD(binder_devices);
-static HLIST_HEAD(binder_procs);
-static DEFINE_MUTEX(binder_procs_lock);
-
static HLIST_HEAD(binder_dead_nodes);
static DEFINE_SPINLOCK(binder_dead_nodes_lock);
@@ -233,6 +243,7 @@ struct binder_transaction_log_entry {
uint32_t return_error;
uint32_t return_error_param;
const char *context_name;
+ unsigned int ipc_inum;
};
struct binder_transaction_log {
atomic_t cur;
@@ -263,19 +274,66 @@ static struct binder_transaction_log_entry
*binder_transaction_log_add(
}
struct binder_context {
+ struct hlist_node hlist;
struct binder_node *binder_context_mgr_node;
struct mutex context_mgr_node_lock;
kuid_t binder_context_mgr_uid;
+ int device;
const char *name;
};
struct binder_device {
struct hlist_node hlist;
struct miscdevice miscdev;
- struct binder_context context;
};
+void binder_exit_ns(struct ipc_namespace *ns)
+{
+ struct binder_context *context;
+ struct hlist_node *tmp;
+
+ mutex_destroy(&ns->binder_procs_lock);
+ hlist_for_each_entry_safe(context, tmp, &ns->binder_contexts, hlist) {
+ mutex_destroy(&context->context_mgr_node_lock);
+ hlist_del(&context->hlist);
+ kfree(context);
+ }
+}
+
+int binder_init_ns(struct ipc_namespace *ns)
+{
+ int ret;
+ struct binder_device *device;
+
+ mutex_init(&ns->binder_procs_lock);
+ INIT_HLIST_HEAD(&ns->binder_procs);
+ INIT_HLIST_HEAD(&ns->binder_contexts);
+
+ hlist_for_each_entry(device, &binder_devices, hlist) {
+ struct binder_context *context;
+
+ context = kzalloc(sizeof(*context), GFP_KERNEL);
+ if (!context) {
+ ret = -ENOMEM;
+ goto err;
+ }
+
+ context->device = device->miscdev.minor;
+ context->name = device->miscdev.name;
+ context->binder_context_mgr_uid = INVALID_UID;
+ mutex_init(&context->context_mgr_node_lock);
+
+ hlist_add_head(&context->hlist, &ns->binder_contexts);
+ }
+
+ return 0;
+err:
+ binder_exit_ns(ns);
+ return ret;
+}
+
+
/**
* struct binder_work - work enqueued on a worklist
* @entry: node enqueued on list
@@ -2728,6 +2786,7 @@ static void binder_transaction(struct binder_proc *proc,
e->data_size = tr->data_size;
e->offsets_size = tr->offsets_size;
e->context_name = proc->context->name;
+ e->ipc_inum = ipcns->ns.inum;
if (reply) {
binder_inner_proc_lock(proc);
@@ -4922,6 +4981,7 @@ static int binder_open(struct inode *nodp, struct file
*filp)
{
struct binder_proc *proc;
struct binder_device *binder_dev;
+ struct binder_context *context;
binder_debug(BINDER_DEBUG_OPEN_CLOSE, "%s: %d:%d\n", __func__,
current->group_leader->pid, current->pid);
@@ -4937,7 +4997,15 @@ static int binder_open(struct inode *nodp, struct file
*filp)
proc->default_priority = task_nice(current);
binder_dev = container_of(filp->private_data, struct binder_device,
miscdev);
- proc->context = &binder_dev->context;
+ hlist_for_each_entry(context, &ipcns->binder_contexts, hlist) {
+ if (context->device == binder_dev->miscdev.minor) {
+ proc->context = context;
+ br
Re: [PATCH RFC 0/3] Static calls
On Mon, 12 Nov 2018 at 06:31, Josh Poimboeuf wrote: > > On Mon, Nov 12, 2018 at 06:02:41AM +0100, Ingo Molnar wrote: > > > > * Josh Poimboeuf wrote: > > > > > On Fri, Nov 09, 2018 at 08:28:11AM +0100, Ingo Molnar wrote: > > > > > - I'm not sure about the objtool approach. Objtool is (currently) > > > > > x86-64 only, which means we have to use the "unoptimized" version > > > > > everywhere else. I may experiment with a GCC plugin instead. > > > > > > > > I'd prefer the objtool approach. It's a pretty reliable first-principles > > > > approach while GCC plugin would have to be replicated for Clang and any > > > > other compilers, etc. > > > > > > The benefit of a plugin is that we'd only need two of them: GCC and > > > Clang. And presumably, they'd share a lot of code. > > > Having looked into this, I don't think they will share any code at all, to be honest. Perhaps some macros and string templates, that's all. > > > The prospect of porting objtool to all architectures is going to be much > > > more of a daunting task (though we are at least already considering it > > > for some arches). > > > > Which architectures would benefit from ORC support the most? > > According to my (limited and potentially flawed) knowledge, I think > arm64 would benefit the most performance-wise, whereas powerpc and s390 > gains would be quite a bit less. > What would arm64 gain from ORC and/or objtool? > We may have to port objtool to arm64 anyway, for live patching. Is this about the reliable stack traces, i.e., the ability to detect non-leaf functions that don't create stack frames? I think we should be able to manage this without objtool on arm64 tbh. > But > that will be a lot more work than it took for Ard to write a GCC plugin. > > > I really think that hard reliance on GCC plugins is foolish > > Funny, I feel the same way about hard reliance on objtool :-) > I tend to agree here. I think objtool is a necessary evil (as are compiler plugins, for that matter) which I hope does not spread to other architectures. But the main difference is that the GCC plugin is only ~50 lines (for this particular use case, and minus another 50 lines of boilerplate), whereas objtool (AIUI) duplicates lots and lots of functionality of the compiler, assembler and/or linker, to mangle relocations, create new sections etc etc. Porting this to other architectures is going to be a major maintenance effort, especially when I think of, e.g., 32-bit ARM with its Thumb2 quirks and other idiosyncrasies that are currently hidden in the toolchain. Other architectures should be first class citizens if objtool gains support for them, which means that the x86 people that own it currently are on the hook for testing their changes against architectures they are not familiar with. This obviously applies equally to compiler plugins, but those have a lot more focus. > > - but maybe Clang's plugin infrastructure is a guarantee that it > > remains a sane and usable interface. > > Hopefully so. If it breaks, we could always write another tool, as the > work is straightforward. Or we could make it an objtool subcommand > which works on all arches. > > > > > All other usecases are bonus, but it would certainly be interesting to > > > > investigate the impact of using these APIs for tracing: that too is a > > > > feature enabled everywhere but utilized only by a small fraction of > > > > Linux > > > > users - so literally every single cycle or instruction saved or hot-path > > > > shortened is a major win. > > > > > > With retpolines, and with tracepoints enabled, it's definitely a major > > > win. Steve measured an 8.9% general slowdown on hackbench caused by > > > retpolines. > > > > How much of that slowdown is reversed? > > In theory, it should reverse all of the slowdown, and actually may even > speed it up a little. Steve is working on measuring that now. > > -- > Josh
[PATCH] mfd: at91-usart: Add platform dependency
It doesn't make sense to present option MFD_AT91_USART by default if
not building an AT91 kernel, as the drivers which depend on it are
not available.
Signed-off-by: Jean Delvare
Fixes: 7d3aa342cef7 ("mfd: at91-usart: Add MFD driver for USART")
Cc: Radu Pirea
Cc: Lee Jones
---
As a side note, maybe it would make sense to have SPI_AT91_USART
select MFD_AT91_USART instead of depend on it, so that
MFD_AT91_USART could be hidden?
drivers/mfd/Kconfig |1 +
1 file changed, 1 insertion(+)
--- linux-4.20-rc2.orig/drivers/mfd/Kconfig 2018-11-12 09:34:20.096038788
+0100
+++ linux-4.20-rc2/drivers/mfd/Kconfig 2018-11-12 10:39:51.102170181 +0100
@@ -102,6 +102,7 @@ config MFD_AAT2870_CORE
config MFD_AT91_USART
tristate "AT91 USART Driver"
select MFD_CORE
+ depends on ARCH_AT91 || COMPILE_TEST
help
Select this to get support for AT91 USART IP. This is a wrapper
over at91-usart-serial driver and usart-spi-driver. Only one function
--
Jean Delvare
SUSE L3 Support
Re: [PATCH v2] exec: make de_thread() freezable
On Mon 2018-11-12 12:54:45, Chanho Min wrote: > Suspend fails due to the exec family of functions blocking the freezer. > The casue is that de_thread() sleeps in TASK_UNINTERRUPTIBLE waiting for > all sub-threads to die, and we have the deadlock if one of them is frozen. > This also can occur with the schedule() waiting for the group thread leader > to exit if it is frozen. > > In our machine, it causes freeze timeout as bellows. > > Freezing of tasks failed after 20.010 seconds (1 tasks refusing to freeze, > wq_busy=0): > setcpushares-ls D ffc8ed70 0 5817 1483 0x004d > Call trace: > [] __switch_to+0x88/0xa0 > [] __schedule+0x1bc/0x720 > [] schedule+0x40/0xa8 > [] flush_old_exec+0xdc/0x640 > [] load_elf_binary+0x2a8/0x1090 > [] search_binary_handler+0x9c/0x240 > [] load_script+0x20c/0x228 > [] search_binary_handler+0x9c/0x240 > [] do_execveat_common.isra.14+0x4f8/0x6e8 > [] compat_SyS_execve+0x38/0x48 > [] el0_svc_naked+0x24/0x28 > > To fix this, make de_thread() freezable. It looks safe and works fine. > > Changes in v2: > - changes for the same reason in "if (!thread_group_leader(tsk))" branch. >(reported by Oleg) > > Suggested-by: Oleg Nesterov > Signed-off-by: Chanho Min Acked-by: Pavel Machek -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
RE: [PATCH] slab.h: Avoid using & for logical and of booleans
From: Vlastimil Babka [mailto:vba...@suse.cz]
> Sent: 09 November 2018 19:16
...
> This? Not terribly elegant, but I don't see a nicer way right now...
Maybe just have two copies of the function body?
static __always_inline enum kmalloc_cache_type kmalloc_type(gfp_t flags)
{
#ifndef CONFIG_ZONE_DMA
return flags & __GFP_RECLAIMABLE ? KMALLOC_RECLAIM : KMALLOC_NORMAL;
#else
if (likely((flags & (__GFP_DMA | __GFP_RECLAIMABLE)) == 0))
return KMALLOC_NORMAL;
return flags & __GFP_DMA ? KMALLOC_DMA : KMALLOC_RECLAIM;
#endif
}
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT,
UK
Registration No: 1397386 (Wales)
[PATCH] mm: cleancache: fix corruption on missed inode invalidation
If all pages are deleted from the mapping by memory reclaim and also
moved to the cleancache:
__delete_from_page_cache
(no shadow case)
unaccount_page_cache_page
cleancache_put_page
page_cache_delete
mapping->nrpages -= nr
(nrpages becomes 0)
We don't clean the cleancache for an inode after final file truncation
(removal).
truncate_inode_pages_final
check (nrpages || nrexceptional) is false
no truncate_inode_pages
no cleancache_invalidate_inode(mapping)
These way when reading the new file created with same inode we may get
these trash leftover pages from cleancache and see wrong data instead of
the contents of the new file.
Fix it by always doing truncate_inode_pages which is already ready for
nrpages == 0 && nrexceptional == 0 case and just invalidates inode.
Fixes: commit 91b0abe36a7b ("mm + fs: store shadow entries in page cache")
To: Andrew Morton
Cc: Johannes Weiner
Cc: Mel Gorman
Cc: Jan Kara
Cc: Matthew Wilcox
Cc: Andi Kleen
Cc: linux...@kvack.org
Cc: linux-kernel@vger.kernel.org
Reviewed-by: Vasily Averin
Reviewed-by: Andrey Ryabinin
Signed-off-by: Pavel Tikhomirov
---
mm/truncate.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/mm/truncate.c b/mm/truncate.c
index 45d68e90b703..4c56c19e76eb 100644
--- a/mm/truncate.c
+++ b/mm/truncate.c
@@ -517,9 +517,9 @@ void truncate_inode_pages_final(struct address_space
*mapping)
*/
xa_lock_irq(&mapping->i_pages);
xa_unlock_irq(&mapping->i_pages);
-
- truncate_inode_pages(mapping, 0);
}
+
+ truncate_inode_pages(mapping, 0);
}
EXPORT_SYMBOL(truncate_inode_pages_final);
--
2.17.1
Re: [PATCH 1/3] soundwire: intel: constify snd_soc_dai_ops structures
On 27-10-18, 15:34, Julia Lawall wrote: > The snd_soc_dai_ops structures are only stored in the ops field of a > snd_soc_dai_driver structure, so make the snd_soc_dai_ops structures > const as well. Applied, thanks -- ~Vinod
Re: [PATCH v2 0/2] phy: cadence: Add driver and dt-bindings for Sierra PHY
Hi Alan, On 31/10/18 10:03 PM, Alan Douglas wrote: > The Cadence Sierra PHY supports a number of different protocols. This > series adds a driver with support for USB3 and PCIe modes. > > Only one clock frequency is currently supported, so the value of clock > provided in device tree is ignored. > > Changes since v1: > * Moved subnode resets into each subnode in devicetree bindings > * Modified driver to put subnode resets on exit > * Changed driver filename to phy-cdns-sierra.c > > Changes since RFC v2: > * Devicetree bindings modified as suggested by Rob Herring. > * Tidy up and correction of return paths in probe function. > > Changes since RFC v1: > * Each group of lanes is now treated as a subnode, and a generic PHY > device is created for each group. > * General cleanup based on comments > * A reset is now required for each subnode. The complete PHY block > is taken out of reset at initial probe, and remains out of reset. > * Added a binding to allow for hardware configuration of PHY registers It's not applying to my -next. Can you rebase this series to -rc1? Thanks Kishon > > > Alan Douglas (2): > dt-bindings: phy: Document cadence Sierra PHY bindings > phy: cadence: Add driver for Sierra PHY > > .../devicetree/bindings/phy/cdns-sierra-phy.txt| 68 > drivers/phy/Kconfig| 1 + > drivers/phy/Makefile | 1 + > drivers/phy/cadence/Kconfig| 9 + > drivers/phy/cadence/Makefile | 2 + > drivers/phy/cadence/phy-cdns-sierra.c | 395 > + > 6 files changed, 476 insertions(+) > create mode 100644 Documentation/devicetree/bindings/phy/cdns-sierra-phy.txt > create mode 100644 drivers/phy/cadence/Kconfig > create mode 100644 drivers/phy/cadence/Makefile > create mode 100644 drivers/phy/cadence/phy-cdns-sierra.c >
Inquiry 12/11/2018
Hi,friend, This is Daniel Murray and i am from Sinara Group Co.Ltd Group Co.,LTD in Russia. We are glad to know about your company from the web and we are interested in your products. Could you kindly send us your Latest catalog and price list for our trial order. Best Regards, Daniel Murray Purchasing Manager
[PATCH] kmemleak: Turn kmemleak_lock to raw spinlock on RT
From: He Zhe kmemleak_lock, as a rwlock on RT, can possibly be held in atomic context and causes the follow BUG. BUG: scheduling while atomic: migration/15/132/0x0002 Modules linked in: iTCO_wdt iTCO_vendor_support intel_rapl pcc_cpufreq pnd2_edac intel_powerclamp coretemp crct10dif_pclmul crct10dif_common aesni_intel matroxfb_base aes_x86_64 matroxfb_g450 matroxfb_accel crypto_simd matroxfb_DAC1064 cryptd glue_helper g450_pll matroxfb_misc i2c_ismt i2c_i801 acpi_cpufreq Preemption disabled at: [] cpu_stopper_thread+0x71/0x100 CPU: 15 PID: 132 Comm: migration/15 Not tainted 4.19.0-rt1-preempt-rt #1 Hardware name: Intel Corp. Harcuvar/Server, BIOS HAVLCRB1.X64.0015.D62.1708310404 08/31/2017 Call Trace: dump_stack+0x4f/0x6a ? cpu_stopper_thread+0x71/0x100 __schedule_bug.cold.16+0x38/0x55 __schedule+0x484/0x6c0 schedule+0x3d/0xe0 rt_spin_lock_slowlock_locked+0x118/0x2a0 rt_spin_lock_slowlock+0x57/0x90 __rt_spin_lock+0x26/0x30 __write_rt_lock+0x23/0x1a0 ? intel_pmu_cpu_dying+0x67/0x70 rt_write_lock+0x2a/0x30 find_and_remove_object+0x1e/0x80 delete_object_full+0x10/0x20 kmemleak_free+0x32/0x50 kfree+0x104/0x1f0 ? x86_pmu_starting_cpu+0x30/0x30 intel_pmu_cpu_dying+0x67/0x70 x86_pmu_dying_cpu+0x1a/0x30 cpuhp_invoke_callback+0x92/0x700 take_cpu_down+0x70/0xa0 multi_cpu_stop+0x62/0xc0 ? cpu_stop_queue_work+0x130/0x130 cpu_stopper_thread+0x79/0x100 smpboot_thread_fn+0x20f/0x2d0 kthread+0x121/0x140 ? sort_range+0x30/0x30 ? kthread_park+0x90/0x90 ret_from_fork+0x35/0x40 And on v4.18 stable tree the following call trace, caused by grabbing kmemleak_lock again, is also observed. kernel BUG at kernel/locking/rtmutex.c:1048! invalid opcode: [#1] PREEMPT SMP PTI CPU: 5 PID: 689 Comm: mkfs.ext4 Not tainted 4.18.16-rt9-preempt-rt #1 Hardware name: Intel Corp. Harcuvar/Server, BIOS HAVLCRB1.X64.0015.D62.1708310404 08/31/2017 RIP: 0010:rt_spin_lock_slowlock_locked+0x277/0x2a0 Code: e8 5e 64 61 ff e9 bc fe ff ff e8 54 64 61 ff e9 b7 fe ff ff 0f 0b e8 98 57 53 ff e9 43 fe ff ff e8 8e 57 53 ff e9 74 ff ff ff <0f> 0b 0f 0b 0f 0b 48 8b 43 10 48 85 c0 74 06 48 3b 58 38 75 0b 49 RSP: 0018:936846d4f3b0 EFLAGS: 00010046 RAX: 8e3680361e00 RBX: 83a8b240 RCX: 0001 RDX: RSI: 8e3680361e00 RDI: 83a8b258 RBP: 936846d4f3e8 R08: 8e3680361e01 R09: 82adfdf0 R10: 827ede18 R11: R12: 936846d4f3f8 R13: 8e3680361e00 R14: 936846d4f3f8 R15: 0246 FS: 7fc8b6bfd780() GS:8e369f34() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: 55fb5659e000 CR3: 0007fdd14000 CR4: 003406e0 Call Trace: ? preempt_count_add+0x74/0xc0 rt_spin_lock_slowlock+0x57/0x90 ? __kernel_text_address+0x12/0x40 ? __save_stack_trace+0x75/0x100 __rt_spin_lock+0x26/0x30 __write_rt_lock+0x23/0x1a0 rt_write_lock+0x2a/0x30 create_object+0x17d/0x2b0 kmemleak_alloc+0x34/0x50 kmem_cache_alloc+0x146/0x220 ? mempool_alloc_slab+0x15/0x20 mempool_alloc_slab+0x15/0x20 mempool_alloc+0x65/0x170 sg_pool_alloc+0x21/0x60 __sg_alloc_table+0x101/0x160 ? sg_free_table_chained+0x30/0x30 sg_alloc_table_chained+0x8b/0xb0 scsi_init_sgtable+0x31/0x90 scsi_init_io+0x44/0x130 sd_setup_write_same16_cmnd+0xef/0x150 sd_init_command+0x6bf/0xaa0 ? cgroup_base_stat_cputime_account_end.isra.0+0x26/0x60 ? elv_rb_del+0x2a/0x40 scsi_setup_cmnd+0x8e/0x140 scsi_prep_fn+0x5d/0x140 blk_peek_request+0xda/0x2f0 scsi_request_fn+0x33/0x550 ? cfq_rb_erase+0x23/0x40 __blk_run_queue+0x43/0x60 cfq_insert_request+0x2f3/0x5d0 __elv_add_request+0x160/0x290 blk_flush_plug_list+0x204/0x230 schedule+0x87/0xe0 __write_rt_lock+0x18b/0x1a0 rt_write_lock+0x2a/0x30 create_object+0x17d/0x2b0 kmemleak_alloc+0x34/0x50 __kmalloc_node+0x1cd/0x340 alloc_request_size+0x30/0x70 mempool_alloc+0x65/0x170 ? ioc_lookup_icq+0x54/0x70 get_request+0x4e3/0x8d0 ? wait_woken+0x80/0x80 blk_queue_bio+0x153/0x470 generic_make_request+0x1dc/0x3f0 submit_bio+0x49/0x140 ? next_bio+0x38/0x40 submit_bio_wait+0x59/0x90 blkdev_issue_discard+0x7a/0xd0 ? _raw_spin_unlock_irqrestore+0x18/0x50 blk_ioctl_discard+0xc7/0x110 blkdev_ioctl+0x57e/0x960 ? __wake_up+0x13/0x20 block_ioctl+0x3d/0x50 do_vfs_ioctl+0xa8/0x610 ? vfs_write+0x166/0x1b0 ksys_ioctl+0x67/0x90 __x64_sys_ioctl+0x1a/0x20 do_syscall_64+0x4d/0xf0 entry_SYSCALL_64_after_hwframe+0x44/0xa9 kmemleak is an error detecting feature. We would not expect as good performance as without it. As there is no raw rwlock defining helpers, we turn kmemleak_lock to a raw spinlock. Signed-off-by: He Zhe Cc: sta...@vger.kernel.org Cc: catalin.mari...@arm.com Cc: bige...@linutronix.de Cc: t...@linutronix.de Cc: rost...@goodmis.org --- mm/kmemleak.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/mm/kmemleak.c b/mm/kmemleak.c index 17dd
Re: [PATCH AUTOSEL 4.4 07/32] cpupower: Fix coredump on VMWare
On 10/31/18 9:11 PM, Sasha Levin wrote:
From: Prarit Bhargava
[ Upstream commit f69ffc5d3db8f1f03fd6d1df5930f9a1fbd787b6 ]
cpupower crashes on VMWare guests. The guests have the AMD PStateDef MSR
(0xC0010064 + state number) set to zero. As a result fid and did are zero
and the crash occurs because of a divide by zero (cof = fid/did). This
can be prevented by checking the enable bit in the PStateDef MSR before
calculating cof. By doing this the value of pstate[i] remains zero and
the value can be tested before displaying the active Pstates.
Check the enable bit in the PstateDef register for all supported families
and only print out enabled Pstates.
Signed-off-by: Prarit Bhargava
Cc: Shuah Khan
Cc: Stafford Horne
Signed-off-by: Shuah Khan (Samsung OSG)
Signed-off-by: Sasha Levin
---
tools/power/cpupower/utils/cpufreq-info.c | 2 ++
tools/power/cpupower/utils/helpers/amd.c | 5 +
2 files changed, 7 insertions(+)
diff --git a/tools/power/cpupower/utils/cpufreq-info.c
b/tools/power/cpupower/utils/cpufreq-info.c
index 0e6764330241..f9b064ac8d94 100644
--- a/tools/power/cpupower/utils/cpufreq-info.c
+++ b/tools/power/cpupower/utils/cpufreq-info.c
@@ -200,6 +200,8 @@ static int get_boost_mode(unsigned int cpu)
printf(_("Boost States: %d\n"), b_states);
printf(_("Total States: %d\n"), pstate_no);
for (i = 0; i < pstate_no; i++) {
+ if (!pstates[i])
+ continue;
if (i < b_states)
printf(_("Pstate-Pb%d: %luMHz (boost state)"
"\n"), i, pstates[i]);
diff --git a/tools/power/cpupower/utils/helpers/amd.c
b/tools/power/cpupower/utils/helpers/amd.c
index 6437ef39aeea..82adfb33d7a5 100644
--- a/tools/power/cpupower/utils/helpers/amd.c
+++ b/tools/power/cpupower/utils/helpers/amd.c
@@ -103,6 +103,11 @@ int decode_pstates(unsigned int cpu, unsigned int
cpu_family,
}
if (read_msr(cpu, MSR_AMD_PSTATE + i, &pstate.val))
return -1;
+ if ((cpu_family == 0x17) && (!pstate.fam17h_bits.en))
+ continue;
+ else if (!pstate.bits.en)
+ continue;
+
pstates[i] = get_cof(cpu_family, pstate);
}
*no = i;
Sasha,
This commit is causing:
$ make V=1 -C tools/power/cpupower all
gcc -fPIC -DVERSION=\"4.4.163.99.g0d9e7b\" -DPACKAGE=\"cpupower\"
-DPACKAGE_BUGREPORT=\"linux...@vger.kernel.org\" -D_GNU_SOURCE -pipe
-DNLS -Wall -Wchar-subscripts -Wpointer-arith -Wsign-compare
-Wno-pointer-sign -Wdeclaration-after-statement -Wshadow -O1 -g -DDEBUG
-I./lib -I ./utils -o utils/helpers/amd.o -c utils/helpers/amd.c
utils/helpers/amd.c: In function ‘decode_pstates’:
utils/helpers/amd.c:106:39: error: ‘union msr_pstate’ has no member
named ‘fam17h_bits’
if ((cpu_family == 0x17) && (!pstate.fam17h_bits.en))
I think it should be dropped from v4.4 and v4.9 RCs, since this is a fix
for a CPU that isn't supported for those 2.
Thanks
--
Rafael D. Tinoco
Linaro Kernel Validation
Re: [PATCH AUTOSEL 4.9 11/48] cpupower: Fix coredump on VMWare
On 10/31/18 9:10 PM, Sasha Levin wrote:
From: Prarit Bhargava
[ Upstream commit f69ffc5d3db8f1f03fd6d1df5930f9a1fbd787b6 ]
cpupower crashes on VMWare guests. The guests have the AMD PStateDef MSR
(0xC0010064 + state number) set to zero. As a result fid and did are zero
and the crash occurs because of a divide by zero (cof = fid/did). This
can be prevented by checking the enable bit in the PStateDef MSR before
calculating cof. By doing this the value of pstate[i] remains zero and
the value can be tested before displaying the active Pstates.
Check the enable bit in the PstateDef register for all supported families
and only print out enabled Pstates.
Signed-off-by: Prarit Bhargava
Cc: Shuah Khan
Cc: Stafford Horne
Signed-off-by: Shuah Khan (Samsung OSG)
Signed-off-by: Sasha Levin
---
tools/power/cpupower/utils/cpufreq-info.c | 2 ++
tools/power/cpupower/utils/helpers/amd.c | 5 +
2 files changed, 7 insertions(+)
diff --git a/tools/power/cpupower/utils/cpufreq-info.c
b/tools/power/cpupower/utils/cpufreq-info.c
index 590d12a25f6e..8d7580e4b299 100644
--- a/tools/power/cpupower/utils/cpufreq-info.c
+++ b/tools/power/cpupower/utils/cpufreq-info.c
@@ -202,6 +202,8 @@ static int get_boost_mode(unsigned int cpu)
printf(_("Boost States: %d\n"), b_states);
printf(_("Total States: %d\n"), pstate_no);
for (i = 0; i < pstate_no; i++) {
+ if (!pstates[i])
+ continue;
if (i < b_states)
printf(_("Pstate-Pb%d: %luMHz (boost state)"
"\n"), i, pstates[i]);
diff --git a/tools/power/cpupower/utils/helpers/amd.c
b/tools/power/cpupower/utils/helpers/amd.c
index 6437ef39aeea..82adfb33d7a5 100644
--- a/tools/power/cpupower/utils/helpers/amd.c
+++ b/tools/power/cpupower/utils/helpers/amd.c
@@ -103,6 +103,11 @@ int decode_pstates(unsigned int cpu, unsigned int
cpu_family,
}
if (read_msr(cpu, MSR_AMD_PSTATE + i, &pstate.val))
return -1;
+ if ((cpu_family == 0x17) && (!pstate.fam17h_bits.en))
+ continue;
+ else if (!pstate.bits.en)
+ continue;
+
pstates[i] = get_cof(cpu_family, pstate);
}
*no = i;
Sasha,
This commit is causing:
$ make V=1 -C tools/power/cpupower all
gcc -fPIC -DVERSION=\"4.9.136.142.g36c7703\" -DPACKAGE=\"cpupower\"
-DPACKAGE_BUGREPORT=\"linux...@vger.kernel.org\" -D_GNU_SOURCE -pipe
-DNLS -Wall -Wchar-subscripts -Wpointer-arith -Wsign-compare
-Wno-pointer-sign -Wdeclaration-after-statement -Wshadow -O1 -g -DDEBUG
-I./lib -I ./utils -o utils/helpers/amd.o -c utils/helpers/amd.c
utils/helpers/amd.c: In function ‘decode_pstates’:
utils/helpers/amd.c:106:39: error: ‘union msr_pstate’ has no member
named ‘fam17h_bits’
if ((cpu_family == 0x17) && (!pstate.fam17h_bits.en))
I think it should be dropped from v4.4 and v4.9 RCs, since this is a fix
for a CPU that isn't supported for those 2.
Thanks
--
Rafael D. Tinoco
Linaro Kernel Validation
Re: [PATCH v2 2/3] drivers/irqchip: Add Actions external interrupts support
On 11/8/18 6:03 PM, Marc Zyngier wrote:
On 26/08/18 16:20, Parthiban Nallathambi wrote:
Hello Marc,
Thanks for your feedback.
On 8/13/18 1:46 PM, Marc Zyngier wrote:
On 12/08/18 13:22, Parthiban Nallathambi wrote:
Actions Semi Owl family SoC's S500, S700 and S900 provides support
for 3 external interrupt controllers through SIRQ pins.
Each line can be independently configured as interrupt and triggers
on either of the edges (raising or falling) or either of the levels
(high or low) . Each line can also be masked independently.
Signed-off-by: Parthiban Nallathambi
Signed-off-by: Saravanan Sekar
---
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-owl-sirq.c | 305 +
2 files changed, 306 insertions(+)
create mode 100644 drivers/irqchip/irq-owl-sirq.c
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 15f268f646bf..072c4409e7c4 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_ATH79) += irq-ath79-misc.o
obj-$(CONFIG_ARCH_BCM2835)+= irq-bcm2835.o
obj-$(CONFIG_ARCH_BCM2835)+= irq-bcm2836.o
obj-$(CONFIG_ARCH_EXYNOS) += exynos-combiner.o
+obj-$(CONFIG_ARCH_ACTIONS) += irq-owl-sirq.o
obj-$(CONFIG_FARADAY_FTINTC010) += irq-ftintc010.o
obj-$(CONFIG_ARCH_HIP04) += irq-hip04.o
obj-$(CONFIG_ARCH_LPC32XX)+= irq-lpc32xx.o
diff --git a/drivers/irqchip/irq-owl-sirq.c b/drivers/irqchip/irq-owl-sirq.c
new file mode 100644
index ..b69301388300
--- /dev/null
+++ b/drivers/irqchip/irq-owl-sirq.c
@@ -0,0 +1,305 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *
+ * Actions Semi Owl SoCs SIRQ interrupt controller driver
+ *
+ * Copyright (C) 2014 Actions Semi Inc.
+ * David Liu
+ *
+ * Author: Parthiban Nallathambi
+ * Author: Saravanan Sekar
+ */
+
+#include
+#include
+#include
+#include
+
+#include
+
+#define INTC_GIC_INTERRUPT_PIN 13
Why isn't that coming from the DT?
DT numbering is taken irqchip local, by which hwirq is directly used to
calculate the offset into register when it is shared. Even if this is
directly from DT, I need the value to offset into the register. So maintianed
inside the driver.
This is normally shown as a property from DT, and is relative to the
parent irqchip. And I don't understand what you mean by "offset into the
register". The only use of this is to allocate the corresponding GIC
We have two SoC's (s500, s700) with shared external interrupt control
register and one (s900) with dedicated register for each external
interrupt line. So the DT property "actions,sirq-offset" was introduced
to access the register.
In case of s500, s700 when it's shared, the idea is to use the "hwirq"
variable value to offset into the control register INTC_EXTCTL. Even if
3 cell GIC value is directly used like
interrupts = ;
then hwirq - 13 is needed internally everywhere in this driver.
In short, this value is defined inside driver for the ease of referring
the offset with the register.
Yes, it is possible to change the driver logic and use 3 cell interrupts
from DT.
interrupt, and this definitely shouldn't be harcoded.
Should it make sense to move it to DT and use another macro (different name)
for offsetting?
+#define INTC_EXTCTL_PENDINGBIT(0)
+#define INTC_EXTCTL_CLK_SELBIT(4)
+#define INTC_EXTCTL_EN BIT(5)
+#defineINTC_EXTCTL_TYPE_MASK GENMASK(6, 7)
+#defineINTC_EXTCTL_TYPE_HIGH 0
+#defineINTC_EXTCTL_TYPE_LOWBIT(6)
+#defineINTC_EXTCTL_TYPE_RISING BIT(7)
+#defineINTC_EXTCTL_TYPE_FALLING(BIT(6) | BIT(7))
+
+#define get_sirq_offset(x) chip_data->sirq[x].offset
+
+/* Per SIRQ data */
+struct owl_sirq {
+ u16 offset;
+ /* software is responsible to clear interrupt pending bit when
+* type is edge triggered. This value is for per SIRQ line.
+*/
Please follow the normal multi-line comment style:
/*
* This is a comment, starting with a capital letter and ending with
* a full stop.
*/
Sure, thanks.
+ bool type_edge;
+};
+
+struct owl_sirq_chip_data {
+ void __iomem *base;
+ raw_spinlock_t lock;
+ /* some SoC's share the register for all SIRQ lines, so maintain
+* register is shared or not here. This value is from DT.
+*/
+ bool shared_reg;
+ struct owl_sirq *sirq;
Given that this driver handles at most 3 interrupts, do we need the
overhead of a pointer and an additional allocation, while we could store
all of this data in the space taken by the pointer itself?
Something like:
u16 offset[3];
u8 trigger; // Bit mask indicating edge-triggered interrupts
and we're done.
Sure, I will modify with one allocation.
+};
+
+static struct owl_sirq_chip_data *sirq_data;
+
+static unsigne
[tip:sched/core] sched/core: Clean up the #ifdef block in add_nr_running()
Commit-ID: 3e184501083c38fa091f640acb13af17a21fd228
Gitweb: https://git.kernel.org/tip/3e184501083c38fa091f640acb13af17a21fd228
Author: Viresh Kumar
AuthorDate: Tue, 6 Nov 2018 11:12:57 +0530
Committer: Ingo Molnar
CommitDate: Mon, 12 Nov 2018 11:18:06 +0100
sched/core: Clean up the #ifdef block in add_nr_running()
There is no point in keeping the conditional statement of the #if block
outside of the #ifdef block, while all of its body is contained within
the #ifdef block.
Move the conditional statement under the #ifdef block as well.
Signed-off-by: Viresh Kumar
Cc: Daniel Lezcano
Cc: Linus Torvalds
Cc: Peter Zijlstra
Cc: Thomas Gleixner
Cc: Vincent Guittot
Link:
http://lkml.kernel.org/r/78cbd78a615d6f9fdcd3327f1ead68470f92593e.1541482935.git.viresh.ku...@linaro.org
Signed-off-by: Ingo Molnar
---
kernel/sched/sched.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index b7a3147874e3..e0e052a50fcd 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1801,12 +1801,12 @@ static inline void add_nr_running(struct rq *rq,
unsigned count)
rq->nr_running = prev_nr + count;
- if (prev_nr < 2 && rq->nr_running >= 2) {
#ifdef CONFIG_SMP
+ if (prev_nr < 2 && rq->nr_running >= 2) {
if (!READ_ONCE(rq->rd->overload))
WRITE_ONCE(rq->rd->overload, 1);
-#endif
}
+#endif
sched_update_tick_dependency(rq);
}
Re: [PATCH 02/24] leds: core: Add support for composing LED class device names
Hi! > >> It's overcomplicating the naming again. In every case you can tweak > >> the function name to eth0_link, eth1_link etc. > > > > Well, but in such case it would be good to keep existing scheme. > > > > My system looks like this: > > > > input16::capslocktpacpi::bay_activetpacpi::standby > > input16::numlock tpacpi::dock_active tpacpi::thinklight > > input16::scrolllock tpacpi::dock_batttpacpi::thinkvantage > > input5::capslock tpacpi::dock_status1 tpacpi::unknown_led > > input5::numlock tpacpi::dock_status2 tpacpi::unknown_led2 > > input5::scrolllock tpacpi:green:batttpacpi::unknown_led3 > > > > I agree that we should get rid of "tpacpi:" part in some cases. But > > it does not make sense to get rid of "input16:" part -- it tells you > > if the LED is on USB or on built-in keyboard. > > > > Ideally, tpacpi::thinklight would be input5::frontlight (as it is > > frontlight for the keyboard). > > > > Yes we should simplify, but it still needs to work in all cases. > > Well, label is not being removed. You still can use it an the old > fashion, even when using new led_compose_name(). > > Maybe removing the description of the old LED naming from > Documentation/leds/leds-class.txt was too drastic move. > I'll keep it next to the new one, and only add a note that > it is kept only for backwards compatibility. I agree that removing it is "just too drastic". But it is not just for backwards compatibility. See my examples above, it is needed to tell which device the LED is asociated with, and it is absolutely required for USB devices (for example). And even for "embedded" stuff like routers, we want eth0:green:link, eth0:yellow:activity and not some kind of hack. Ideally, colors would come from fixed list, functions would come from fixed list, and device part would come from name used elsewhere in the kernel. (And yes, it probably means we should have something in device tree to link LED to its device. device = "name" would be good start...) Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
[tip:x86/mm] x86/mm/fault: Allow stack access below %rsp
Commit-ID: 1d8ca3be86ebc6a38dad8236f45c7a9c61681e78
Gitweb: https://git.kernel.org/tip/1d8ca3be86ebc6a38dad8236f45c7a9c61681e78
Author: Waiman Long
AuthorDate: Tue, 6 Nov 2018 15:12:29 -0500
Committer: Ingo Molnar
CommitDate: Mon, 12 Nov 2018 11:06:19 +0100
x86/mm/fault: Allow stack access below %rsp
The current x86 page fault handler allows stack access below the stack
pointer if it is no more than 64k+256 bytes. Any access beyond the 64k+
limit will cause a segmentation fault.
The gcc -fstack-check option generates code to probe the stack for
large stack allocation to see if the stack is accessible. The newer gcc
does that while updating the %rsp simultaneously. Older gcc's like gcc4
doesn't do that. As a result, an application compiled with an old gcc
and the -fstack-check option may fail to start at all:
$ cat test.c
int main() {
char tmp[1024*128];
printf("### ok\n");
return 0;
}
$ gcc -fstack-check -g -o test test.c
$ ./test
Segmentation fault
The old binary was working in older kernels where expand_stack() was
somehow called before the check. But it is not working in newer kernels.
Besides, the 64k+ limit check is kind of crude and will not catch a
lot of mistakes that userspace applications may be misbehaving anyway.
I think the kernel isn't the right place for this kind of tests. We
should leave it to userspace instrumentation tools to perform them.
The 64k+ limit check is now removed to just let expand_stack() decide
if a segmentation fault should happen, when the RLIMIT_STACK limit is
exceeded, for example.
Signed-off-by: Waiman Long
Cc: Andy Lutomirski
Cc: Borislav Petkov
Cc: Brian Gerst
Cc: Dave Hansen
Cc: Denys Vlasenko
Cc: H. Peter Anvin
Cc: Linus Torvalds
Cc: Peter Zijlstra
Cc: Rik van Riel
Cc: Thomas Gleixner
Link:
http://lkml.kernel.org/r/1541535149-31963-1-git-send-email-long...@redhat.com
Signed-off-by: Ingo Molnar
---
arch/x86/mm/fault.c | 12
1 file changed, 12 deletions(-)
diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index 71d4b9d4d43f..29525cf21100 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -1380,18 +1380,6 @@ retry:
bad_area(regs, sw_error_code, address);
return;
}
- if (sw_error_code & X86_PF_USER) {
- /*
-* Accessing the stack below %sp is always a bug.
-* The large cushion allows instructions like enter
-* and pusha to work. ("enter $65535, $31" pushes
-* 32 pointers and then decrements %sp by 65535.)
-*/
- if (unlikely(address + 65536 + 32 * sizeof(unsigned long) <
regs->sp)) {
- bad_area(regs, sw_error_code, address);
- return;
- }
- }
if (unlikely(expand_stack(vma, address))) {
bad_area(regs, sw_error_code, address);
return;
[RFC PATCH v3 4/4] arm64: dts: add msm8996 compatible to gicv3
Signed-off-by: Srinivas Kandagatla
---
arch/arm64/boot/dts/qcom/msm8996.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi
b/arch/arm64/boot/dts/qcom/msm8996.dtsi
index 16c3fc0f4e69..e38da221f9a9 100644
--- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
@@ -902,7 +902,7 @@
};
intc: interrupt-controller@9bc {
- compatible = "arm,gic-v3";
+ compatible = "qcom,msm8996-gic-v3", "arm,gic-v3";
#interrupt-cells = <3>;
interrupt-controller;
#redistributor-regions = <1>;
--
2.19.1
[RFC PATCH v3 2/4] irqchip/gic: common: add support to device tree based quirks
This patch adds support to device tree based quirks based on
device tree compatible string.
Signed-off-by: Srinivas Kandagatla
---
drivers/irqchip/irq-gic-common.c | 12
drivers/irqchip/irq-gic-common.h | 3 +++
2 files changed, 15 insertions(+)
diff --git a/drivers/irqchip/irq-gic-common.c b/drivers/irqchip/irq-gic-common.c
index 01e673c680cd..3c93c6f4d1f1 100644
--- a/drivers/irqchip/irq-gic-common.c
+++ b/drivers/irqchip/irq-gic-common.c
@@ -36,6 +36,18 @@ void gic_set_kvm_info(const struct gic_kvm_info *info)
gic_kvm_info = info;
}
+void gic_enable_of_quirks(const struct device_node *np,
+ const struct gic_quirk *quirks, void *data)
+{
+ for (; quirks->desc; quirks++) {
+ if (!of_device_is_compatible(np, quirks->compatible))
+ continue;
+ if (quirks->init(data))
+ pr_info("GIC: enabling workaround for %s\n",
+ quirks->desc);
+ }
+}
+
void gic_enable_quirks(u32 iidr, const struct gic_quirk *quirks,
void *data)
{
diff --git a/drivers/irqchip/irq-gic-common.h b/drivers/irqchip/irq-gic-common.h
index 3919cd7c5285..97e58fb6c232 100644
--- a/drivers/irqchip/irq-gic-common.h
+++ b/drivers/irqchip/irq-gic-common.h
@@ -23,6 +23,7 @@
struct gic_quirk {
const char *desc;
+ const char *compatible;
bool (*init)(void *data);
u32 iidr;
u32 mask;
@@ -35,6 +36,8 @@ void gic_dist_config(void __iomem *base, int gic_irqs,
void gic_cpu_config(void __iomem *base, void (*sync_access)(void));
void gic_enable_quirks(u32 iidr, const struct gic_quirk *quirks,
void *data);
+void gic_enable_of_quirks(const struct device_node *np,
+ const struct gic_quirk *quirks, void *data);
void gic_set_kvm_info(const struct gic_kvm_info *info);
--
2.19.1
[RFC PATCH v3 0/4] irqchip/gic-v3: Add support to DT based quirk for msm8996
Access to GICR_WAKER is restricted on msm8996 SoC in Hypervisor. There are many devices out there with this restriction in place and there has been no update to this firmware since last few years, making those devices totally unusable for upstream development. My previous attempts to add quick based on IIDR register value seems to be flawed by the fact that the value conflicted with other SoCs. Last Suggestion by Marc Z using compatible seems to be the only way to apply quirks required for msm8996 based SoCs. Here is the patchset which add new compatible for msm8996 gicv3 and add support gic_enable_of_quirks() followed by the actual quirk required for msm8996. Without this quirk many qcom SoCs (atleast 3 that I know) are unable to boot mainline. Thanks, Srini Srinivas Kandagatla (4): dt-bindings/gic-v3: Add msm8996 compatible string irqchip/gic: common: add support to device tree based quirks irqchip: gic-v3: Add quirk for msm8996 secured registers arm64: dts: add msm8996 compatible to gicv3 .../interrupt-controller/arm,gic-v3.txt | 4 ++- arch/arm64/boot/dts/qcom/msm8996.dtsi | 2 +- drivers/irqchip/irq-gic-common.c | 12 + drivers/irqchip/irq-gic-common.h | 3 +++ drivers/irqchip/irq-gic-v3.c | 26 +++ 5 files changed, 45 insertions(+), 2 deletions(-) -- 2.19.1
[RFC PATCH v3 1/4] dt-bindings/gic-v3: Add msm8996 compatible string
Access to GICR_WAKER is restricted on msm8996 SoC in Hypervisor. There are many devices out there with this restriction in place and there has been no update to this firmware since last few years, making those devices totally unusable for upstream development. IIDR register value conflicts with other SoCs, using compatible seems to be the only way to apply quirks required for msm8996 based SoCs. Without this quirk many qcom SoCs (atleast 3 that I know) are unable to boot mainline. Signed-off-by: Srinivas Kandagatla --- .../devicetree/bindings/interrupt-controller/arm,gic-v3.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt b/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt index 3ea78c4ef887..b83bb8249074 100644 --- a/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt +++ b/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt @@ -7,7 +7,9 @@ Interrupts (LPI). Main node required properties: -- compatible : should at least contain "arm,gic-v3". +- compatible : should at least contain "arm,gic-v3" or either + "qcom,msm8996-gic-v3", "arm,gic-v3" for msm8996 SoCs + to address SoC specific bugs/quirks - interrupt-controller : Identifies the node as an interrupt controller - #interrupt-cells : Specifies the number of cells needed to encode an interrupt source. Must be a single cell with a value of at least 3. -- 2.19.1
[RFC PATCH v3 3/4] irqchip: gic-v3: Add quirk for msm8996 secured registers
Access to GICR_WAKER is restricted on msm8996 SoC in Hypervisor.
Its been more than 2+ years of wait for this to be fixed, which has
no hopes to be fixed. This change was introduced for the "lead device"
on msm8996 platform. It looks like all publicly available msm8996 and
other qcom SoCs have this implementation.
So add a quirk to not access this register on msm8996.
With this quirk MSM8996 can at least boot out of mainline,
which can help community to work with boards based on MSM8996 and other
SoCs with have this restrictions. This Quirk is based on device tree
compatible string.
Without this patch Qualcomm DB820c board reboots when GICR_WAKER
is accessed.
Signed-off-by: Srinivas Kandagatla
---
drivers/irqchip/irq-gic-v3.c | 26 ++
1 file changed, 26 insertions(+)
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index 8f87f40c9460..4bd3bbe1b7ce 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -41,6 +41,8 @@
#include "irq-gic-common.h"
+#define FLAGS_WORKAROUND_GICR_WAKER_MSM8996(1ULL << 0)
+
struct redist_region {
void __iomem*redist_base;
phys_addr_t phys_base;
@@ -55,6 +57,7 @@ struct gic_chip_data {
struct irq_domain *domain;
u64 redist_stride;
u32 nr_redist_regions;
+ u64 flags;
boolhas_rss;
unsigned intirq_nr;
struct partition_desc *ppi_descs[16];
@@ -139,6 +142,9 @@ static void gic_enable_redist(bool enable)
u32 count = 100;/* 1s! */
u32 val;
+ if (gic_data.flags & FLAGS_WORKAROUND_GICR_WAKER_MSM8996)
+ return;
+
rbase = gic_data_rdist_rd_base();
val = readl_relaxed(rbase + GICR_WAKER);
@@ -1067,6 +1073,23 @@ static const struct irq_domain_ops partition_domain_ops
= {
.select = gic_irq_domain_select,
};
+static bool __maybe_unused gic_enable_quirk_msm8996(void *data)
+{
+ struct gic_chip_data *d = data;
+
+ d->flags |= FLAGS_WORKAROUND_GICR_WAKER_MSM8996;
+
+ return true;
+}
+
+static const struct gic_quirk gic_quirks[] = {
+ {
+ .desc = "GICv3: Qualcomm MSM8996 skip GICR_WAKER Read/Write",
+ .compatible = "qcom,msm8996-gic-v3", /* MSM8996 */
+ .init = gic_enable_quirk_msm8996,
+ },
+};
+
static int __init gic_init_bases(void __iomem *dist_base,
struct redist_region *rdist_regs,
u32 nr_redist_regions,
@@ -1126,6 +1149,9 @@ static int __init gic_init_bases(void __iomem *dist_base,
gic_update_vlpi_properties();
+ if (is_of_node(handle))
+ gic_enable_of_quirks(to_of_node(handle), gic_quirks, &gic_data);
+
gic_smp_init();
gic_dist_init();
gic_cpu_init();
--
2.19.1
Re: [PATCH 4.9 000/141] 4.9.137-stable review
On 11/11/18 8:24 PM, Greg Kroah-Hartman wrote: This is the start of the stable review cycle for the 4.9.137 release. There are 141 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know. Responses should be made by Tue Nov 13 22:15:38 UTC 2018. Anything received after that time might be too late. The whole patch series can be found in one patch at: https://www.kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.9.137-rc1.gz or in the git tree and branch at: git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git linux-4.9.y and the diffstat can be found below. thanks, greg k-h - Pseudo-Shortlog of commits: Greg Kroah-Hartman Linux 4.9.137-rc1 ... Prarit Bhargava cpupower: Fix coredump on VMWare Greg, maybe... commit ef8d3a128c1f1de7ffdedb2f14e846e10fd3fec3 Author: Prarit Bhargava Date: Mon Oct 8 12:06:19 2018 cpupower: Fix coredump on VMWare [ Upstream commit f69ffc5d3db8f1f03fd6d1df5930f9a1fbd787b6 ] should be dropped from v4.9 (and v4.4) since it causes: $ make V=1 -C tools/power/cpupower all gcc -fPIC -DVERSION=\"4.9.136.142.g36c7703\" -DPACKAGE=\"cpupower\" -DPACKAGE_BUGREPORT=\"linux...@vger.kernel.org\" -D_GNU_SOURCE -pipe -DNLS -Wall -Wchar-subscripts -Wpointer-arith -Wsign-compare -Wno-pointer-sign -Wdeclaration-after-statement -Wshadow -O1 -g -DDEBUG -I./lib -I ./utils -o utils/helpers/amd.o -c utils/helpers/amd.c utils/helpers/amd.c: In function ‘decode_pstates’: utils/helpers/amd.c:106:39: error: ‘union msr_pstate’ has no member named ‘fam17h_bits’ if ((cpu_family == 0x17) && (!pstate.fam17h_bits.en)) Due to nonexistent CPU (flag) support. Replied to Sasha on specific commits for both, v4.4 and v4.9. Thanks! -- Rafael D. Tinoco Linaro Kernel Validation
Re: [PATCH v6 1/3] printk: Add line-buffered printk() API.
On 2018/11/12 16:59, Sergey Senozhatsky wrote: > Tetsuo, lockdep report with buffered printks looks a bit different: > > kernel: Possible unsafe locking scenario: > kernel:CPU0CPU1 > kernel: > kernel: lock(bar_lock); > kernel:lock( > kernel: foo_lock); > kernel:lock(bar_lock); > kernel: lock(foo_lock); > kernel: > Yes. That is because vprintk_buffered() eliminated KERN_CONT. Since there was pending partial printk() output, vprintk_buffered() should not eliminate KERN_CONT. Petr Mladek commented 1. The mixing of normal and buffered printk calls is a bit confusing and error prone. It would make sense to use the buffered printk everywhere in the given section of code even when it is not strictly needed. and I made a draft version for how the code would look like if we try to avoid the mixing of normal and buffered printk calls. The required changes seem to be too large to apply tree wide. And I suggested try_buffered_printk().
Re: [PATCH v4 06/10] ARM: defconfig: Use the new FSL QSPI driver under the SPI framework
On 08.11.18 09:34, Boris Brezillon wrote: > On Wed, 7 Nov 2018 16:36:13 + > Schrempf Frieder wrote: > >> Hi Olof, >> >> On 07.11.18 17:20, Olof Johansson wrote: >>> On Wed, Nov 7, 2018 at 6:44 AM Frieder Schrempf >>> wrote: From: Frieder Schrempf The new driver at spi/spi-fsl-qspi.c replaces the old SPI NOR driver at mtd/fsl-quadspi.c. Switch to the new driver in the defconfigs. Signed-off-by: Frieder Schrempf >>> >>> Hi Frieder, >>> >>> This patch is part of a series that I didn't see the rest of, but in >>> general we prefer to merge these through arm-soc even if the driver >>> goes in through another tree. The way we'd prefer to handle it is that >>> once the driver lands, we'll take the config option change to turn it >>> on. To avoid our branches to break until both sides have landed, it >>> might be a good idea to keep both drivers on for a short while (one >>> release). >>> >>> So, I'm not going to ack this since we avoid taking defconfig changes >>> through driver trees (these two defconfigs tend to churn a lot and we >>> don't want to create merge conflicts where we don't have to), but >>> we'll be happy to pick it up when the time comes. >> >> Ok, thank you for explaining the common practice. I will drop the config >> changes for the next version and send it separately when the time is ready. >> >> Both the old driver and the new one use the same compatible strings for >> probing. Wouldn't that cause problems if both drivers are enabled for a >> while, or am I missing something? > > Or maybe we should not introduce a new Kconfig option and just reuse > the old one. It probably requires re-ordering patches a bit (patch 1 > should be moved after patch 5). Then you have 2 choices: > > 1/ merge patch 1 and 6 so that the new driver effectively replaces the > old one but uses the same Kconfig option > 2/ remove the ability to compile the old driver when the new one is > introduced: remove the line from drivers/mtd/spi-nor Makefile and > move the Kconfig entry from drivers/mtd/spi-nor/Kconfig to > drivers/spi/Kconfig. And remove the old code in a separate patch > > I'm fine either way, but option #2 will probably make the patch > introducing the new driver bigger and hurt readability. I think having both drivers in the tree for a while wouldn't be so bad. So if any compatibility issues come up with the new driver, people can still use the old one. Therefore I think I will drop the patches that change the defconfig and remove the old driver code and keep the different Kconfig options. And maybe add an exclusive dependency in Kconfig, so both drivers can not be enabled at the same time. Does this make sense?
[PATCH 3/4] power: supply: sc2731_charger: Avoid repeated charge/discharge
Add info->charging validation to avoid repeated charge or discharge
operation.
Signed-off-by: Baolin Wang
---
drivers/power/supply/sc2731_charger.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/power/supply/sc2731_charger.c
b/drivers/power/supply/sc2731_charger.c
index a012d6c..49b3f0c 100644
--- a/drivers/power/supply/sc2731_charger.c
+++ b/drivers/power/supply/sc2731_charger.c
@@ -328,7 +328,7 @@ static void sc2731_charger_work(struct work_struct *data)
mutex_lock(&info->lock);
- if (info->limit > 0) {
+ if (info->limit > 0 && !info->charging) {
/* set current limitation and start to charge */
ret = sc2731_charger_set_current_limit(info, info->limit);
if (ret)
@@ -343,7 +343,7 @@ static void sc2731_charger_work(struct work_struct *data)
goto out;
info->charging = true;
- } else {
+ } else if (!info->limit && info->charging) {
/* Stop charging */
info->charging = false;
sc2731_charger_stop_charge(info);
--
1.7.9.5
[PATCH 4/4] power: supply: sc2731_charger: Free battery information
Free battery information in case of adding battery OCV tables. Signed-off-by: Baolin Wang --- drivers/power/supply/sc2731_charger.c |2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/power/supply/sc2731_charger.c b/drivers/power/supply/sc2731_charger.c index 49b3f0c..335cb85 100644 --- a/drivers/power/supply/sc2731_charger.c +++ b/drivers/power/supply/sc2731_charger.c @@ -408,6 +408,8 @@ static int sc2731_charger_hw_init(struct sc2731_charger_info *info) vol_val = (term_voltage - 4200) / 100; else vol_val = 0; + + power_supply_put_battery_info(info->psy_usb, &bat_info); } /* Set charge termination current */ -- 1.7.9.5
[PATCH 1/4] power: supply: sc2731_charger: Add one work to charge/discharge
Since the USB notifier context is atomic, we can not start or stop charging
in atomic context. Thus this patch adds one work to help to charge or
discharge.
Signed-off-by: Baolin Wang
---
drivers/power/supply/sc2731_charger.c | 30 ++
1 file changed, 22 insertions(+), 8 deletions(-)
diff --git a/drivers/power/supply/sc2731_charger.c
b/drivers/power/supply/sc2731_charger.c
index 525a820..393ba98 100644
--- a/drivers/power/supply/sc2731_charger.c
+++ b/drivers/power/supply/sc2731_charger.c
@@ -57,9 +57,11 @@ struct sc2731_charger_info {
struct usb_phy *usb_phy;
struct notifier_block usb_notify;
struct power_supply *psy_usb;
+ struct work_struct work;
struct mutex lock;
bool charging;
u32 base;
+ u32 limit;
};
static void sc2731_charger_stop_charge(struct sc2731_charger_info *info)
@@ -318,22 +320,21 @@ static int sc2731_charger_property_is_writeable(struct
power_supply *psy,
.property_is_writeable = sc2731_charger_property_is_writeable,
};
-static int sc2731_charger_usb_change(struct notifier_block *nb,
-unsigned long limit, void *data)
+static void sc2731_charger_work(struct work_struct *data)
{
struct sc2731_charger_info *info =
- container_of(nb, struct sc2731_charger_info, usb_notify);
- int ret = 0;
+ container_of(data, struct sc2731_charger_info, work);
+ int ret;
mutex_lock(&info->lock);
- if (limit > 0) {
+ if (info->limit > 0) {
/* set current limitation and start to charge */
- ret = sc2731_charger_set_current_limit(info, limit);
+ ret = sc2731_charger_set_current_limit(info, info->limit);
if (ret)
goto out;
- ret = sc2731_charger_set_current(info, limit);
+ ret = sc2731_charger_set_current(info, info->limit);
if (ret)
goto out;
@@ -350,7 +351,19 @@ static int sc2731_charger_usb_change(struct notifier_block
*nb,
out:
mutex_unlock(&info->lock);
- return ret;
+}
+
+static int sc2731_charger_usb_change(struct notifier_block *nb,
+unsigned long limit, void *data)
+{
+ struct sc2731_charger_info *info =
+ container_of(nb, struct sc2731_charger_info, usb_notify);
+
+ info->limit = limit;
+
+ schedule_work(&info->work);
+
+ return NOTIFY_OK;
}
static int sc2731_charger_hw_init(struct sc2731_charger_info *info)
@@ -432,6 +445,7 @@ static int sc2731_charger_probe(struct platform_device
*pdev)
mutex_init(&info->lock);
info->dev = &pdev->dev;
+ INIT_WORK(&info->work, sc2731_charger_work);
info->regmap = dev_get_regmap(pdev->dev.parent, NULL);
if (!info->regmap) {
--
1.7.9.5
[PATCH 2/4] power: supply: sc2731_charger: Add charger status detection
The USB charger status can be notified before the charger driver registers
the USB phy notifier, so we should check the charger status in probe() in
case we missed the USB charger notification.
Signed-off-by: Baolin Wang
---
drivers/power/supply/sc2731_charger.c | 20
1 file changed, 20 insertions(+)
diff --git a/drivers/power/supply/sc2731_charger.c
b/drivers/power/supply/sc2731_charger.c
index 393ba98..a012d6c 100644
--- a/drivers/power/supply/sc2731_charger.c
+++ b/drivers/power/supply/sc2731_charger.c
@@ -432,6 +432,24 @@ static int sc2731_charger_hw_init(struct
sc2731_charger_info *info)
return ret;
}
+static void sc2731_charger_detect_status(struct sc2731_charger_info *info)
+{
+ unsigned int min, max;
+
+ /*
+* If the USB charger status has been USB_CHARGER_PRESENT before
+* registering the notifier, we should start to charge with getting
+* the charge current.
+*/
+ if (info->usb_phy->chg_state != USB_CHARGER_PRESENT)
+ return;
+
+ usb_phy_get_charger_current(info->usb_phy, &min, &max);
+ info->limit = min;
+
+ schedule_work(&info->work);
+}
+
static int sc2731_charger_probe(struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
@@ -486,6 +504,8 @@ static int sc2731_charger_probe(struct platform_device
*pdev)
return ret;
}
+ sc2731_charger_detect_status(info);
+
return 0;
}
--
1.7.9.5
Re: [PATCH v2 1/3] dt-bindings: interrupt-controller: Actions external interrupt controller
On 8/13/18 6:34 AM, Manivannan Sadhasivam wrote:
Hi Parthiban,
On Sun, Aug 12, 2018 at 02:22:13PM +0200, Parthiban Nallathambi wrote:
Actions Semi OWL family SoC's provides support for external interrupt
controller to be connected and controlled using SIRQ pins. S500, S700
and S900 provides 3 SIRQ lines and works independently for 3 external
interrupt controllers.
Signed-off-by: Parthiban Nallathambi
Signed-off-by: Saravanan Sekar
---
.../interrupt-controller/actions,owl-sirq.txt | 46 ++
1 file changed, 46 insertions(+)
create mode 100644
Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.txt
diff --git
a/Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.txt
b/Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.txt
new file mode 100644
index ..4b8437751331
--- /dev/null
+++
b/Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.txt
@@ -0,0 +1,46 @@
+Actions Semi Owl SoCs SIRQ interrupt controller
+
+S500, S700 and S900 SoC's from Actions provides 3 SPI's from GIC,
+in which external interrupt controller can be connected. 3 SPI's
+45, 46, 47 from GIC are directly exposed as SIRQ. It has
+the following properties:
We should really document the driver here. What it does? and how the
hierarchy is handled with GIC? etc...
+
+- inputs three interrupt signal from external interrupt controller
+
+Required properties:
+
+- compatible: should be "actions,owl-sirq"
+- reg: physical base address of the controller and length of memory mapped.
...length of memory mapped region?
Yes it is.
+- interrupt-controller: identifies the node as an interrupt controller
+- #interrupt-cells: specifies the number of cells needed to encode an interrupt
+ source, should be 2.
+- actions,sirq-shared-reg: Applicable for S500 and S700 where SIRQ register
+ details are maintained at same offset/register.
+- actions,sirq-offset: register offset for SIRQ interrupts. When registers are
+ shared, all the three offsets will be same (S500 and S700).
+- actions,sirq-clk-sel: external interrupt controller can be either
+ connected to 32Khz or 24Mhz external/internal clock. This needs
Hertz should be specified as Hz.
+ to be configured for per SIRQ line. Failing defaults to 32Khz clock.
What value needs to be specified for selecting 24MHz clock? You should
mention the available options this property supports.
Ok, this property will be removed here and leave to default 24MHz for
all the SoC's.
+
+Example for S900:
+
+sirq: interrupt-controller@e01b {
+ compatible = "actions,owl-sirq";
+ reg = <0 0xe01b 0 0x1000>;
could be: reg = <0x0 0xe01b 0x0 0x1000>;
Agreed!
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ actions,sirq-clk-sel = <0 0 0>;
+ actions,sirq-offset = <0x200 0x528 0x52c>;
+};
+
+Example for S500 and S700:
+
+sirq: interrupt-controller@e01b {
+ compatible = "actions,owl-sirq";
+ reg = <0 0xe01b 0 0x1000>;
For S500, reg base is 0xb01b.
Yes, agreed!
Thanks,
Parthi
Thanks
Mani
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ actions,sirq-shared-reg;
+ actions,sirq-clk-sel = <0 0 0>;
+ actions,sirq-offset = <0x200 0x200 0x200>;
+};
--
2.14.4
Re: [PATCH v4 06/10] ARM: defconfig: Use the new FSL QSPI driver under the SPI framework
On Mon, 12 Nov 2018 10:46:45 + Schrempf Frieder wrote: > On 08.11.18 09:34, Boris Brezillon wrote: > > On Wed, 7 Nov 2018 16:36:13 + > > Schrempf Frieder wrote: > > > >> Hi Olof, > >> > >> On 07.11.18 17:20, Olof Johansson wrote: > >>> On Wed, Nov 7, 2018 at 6:44 AM Frieder Schrempf > >>> wrote: > > From: Frieder Schrempf > > The new driver at spi/spi-fsl-qspi.c replaces the old SPI NOR driver > at mtd/fsl-quadspi.c. Switch to the new driver in the defconfigs. > > Signed-off-by: Frieder Schrempf > >>> > >>> Hi Frieder, > >>> > >>> This patch is part of a series that I didn't see the rest of, but in > >>> general we prefer to merge these through arm-soc even if the driver > >>> goes in through another tree. The way we'd prefer to handle it is that > >>> once the driver lands, we'll take the config option change to turn it > >>> on. To avoid our branches to break until both sides have landed, it > >>> might be a good idea to keep both drivers on for a short while (one > >>> release). > >>> > >>> So, I'm not going to ack this since we avoid taking defconfig changes > >>> through driver trees (these two defconfigs tend to churn a lot and we > >>> don't want to create merge conflicts where we don't have to), but > >>> we'll be happy to pick it up when the time comes. > >> > >> Ok, thank you for explaining the common practice. I will drop the config > >> changes for the next version and send it separately when the time is ready. > >> > >> Both the old driver and the new one use the same compatible strings for > >> probing. Wouldn't that cause problems if both drivers are enabled for a > >> while, or am I missing something? > > > > Or maybe we should not introduce a new Kconfig option and just reuse > > the old one. It probably requires re-ordering patches a bit (patch 1 > > should be moved after patch 5). Then you have 2 choices: > > > > 1/ merge patch 1 and 6 so that the new driver effectively replaces the > > old one but uses the same Kconfig option > > 2/ remove the ability to compile the old driver when the new one is > > introduced: remove the line from drivers/mtd/spi-nor Makefile and > > move the Kconfig entry from drivers/mtd/spi-nor/Kconfig to > > drivers/spi/Kconfig. And remove the old code in a separate patch > > > > I'm fine either way, but option #2 will probably make the patch > > introducing the new driver bigger and hurt readability. > > I think having both drivers in the tree for a while wouldn't be so bad. > So if any compatibility issues come up with the new driver, people can > still use the old one. Except that's not what happens in practice. Believe me, I tried this approach several times, and people keep using the old driver until they're forced to switch to the new one. So you actually don't address the problem, you just delay it a bit, and you'll have to fix regressions anyway. > > Therefore I think I will drop the patches that change the defconfig and > remove the old driver code and keep the different Kconfig options. And > maybe add an exclusive dependency in Kconfig, so both drivers can not be > enabled at the same time. > > Does this make sense? I'd really prefer to have the removal of the old driver in the same release the new driver is introduced but if that's not possible, let's have a clear plan, like "introduce new driver in release X, remove the old one in release X+1".
Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
On Mon, 12 Nov 2018 11:55:36 +0100
Martin Lund wrote:
> Hi Naga,
>
> Just a few review comments for the v12 version.
>
> On Fri, Nov 9, 2018 at 6:00 AM Naga Sureshkumar Relli
> wrote:
> > +#define PKT_OFST 0x00
> > +#define PKT_CNT_SHIFT 12
> > +
> > +#define MEM_ADDR1_OFST 0x04
> > +#define MEM_ADDR2_OFST 0x08
>
> For the sake of readability I think *_OFFSET is preferred, especially
> since the driver already includes short macro names. I think this is
> similar to the EVNT vs EVENT point.
> The majority of drivers in the Linux kernel do not shorten OFFSET -> OFST.
>
>
> > +static void anfc_rw_pio_op(struct mtd_info *mtd, u8 *buf, int len,
> > + bool do_read, int prog, int pktcount, int
> > pktsize)
> > +{
> > + struct nand_chip *chip = mtd_to_nand(mtd);
> > + struct anfc_nand_controller *nfc = to_anfc(chip->controller);
> > + struct anfc_nand_chip *achip = to_anfc_nand(chip);
> > + u32 *bufptr = (u32 *)buf;
> > + u32 cnt = 0, intr = 0;
> > +
> > + anfc_config_dma(nfc, 0);
> > +
> > + if (pktsize == 0)
> > + pktsize = len;
> > +
> > + anfc_setpktszcnt(nfc, pktsize, pktcount);
> > +
> > + if (!achip->strength)
> > + intr = MBIT_ERROR;
> > +
> > + if (do_read)
> > + intr |= READ_READY;
> > + else
> > + intr |= WRITE_READY;
> > + anfc_enable_intrs(nfc, intr);
> > + writel(prog, nfc->base + PROG_OFST);
> > + while (cnt < pktcount) {
> > + anfc_wait_for_event(nfc);
> > + cnt++;
> > + if (cnt == pktcount)
> > + anfc_enable_intrs(nfc, XFER_COMPLETE);
> > + if (do_read)
> > + ioread32_rep(nfc->base + DATA_PORT_OFST, bufptr,
> > +pktsize / 4);
> > + else
> > + iowrite32_rep(nfc->base + DATA_PORT_OFST, bufptr,
> > + pktsize / 4);
> > + bufptr += (pktsize / 4);
> > + if (cnt < pktcount)
> > + anfc_enable_intrs(nfc, intr);
> > + }
> > + anfc_wait_for_event(nfc);
> > +}
>
> Throughout the driver all calls to anfc_wait_for_event() ignores the
> timeout return value. It would be nice to see some error handling in
> case it times out - at minimum consider printing out an error message
> since timeout on NAND operations are fairly critical and should
> generally not occur. Perhaps even an argument can be made for
> returning -ETIMEDOUT in case of timeout.
Yes please, check anfc_wait_for_event() return code and propagate the
error to the upper layer.
Re: [PATCH v4 01/10] Fix "x86/alternatives: Lockdep-enforce text_mutex in text_poke*()"
On Mon, 12 Nov 2018, Masami Hiramatsu wrote: > > text_mutex is currently expected to be held before text_poke() is > > called, but we kgdb does not take the mutex, and instead *supposedly* > > ensures the lock is not taken and will not be acquired by any other > > core while text_poke() is running. > > > > The reason for the "supposedly" comment is that it is not entirely clear > > that this would be the case if gdb_do_roundup is zero. > > > > This patch creates two wrapper functions, text_poke() and > > text_poke_kgdb() which do or do not run the lockdep assertion > > respectively. > > > > While we are at it, change the return code of text_poke() to something > > meaningful. One day, callers might actually respect it and the existing > > BUG_ON() when patching fails could be removed. For kgdb, the return > > value can actually be used. > > Hm, this looks reasonable and good to me. > > Reviewed-by: Masami Hiramatsu Yes, I guess this is much better than putting the 'enforcement by comment' back in place :) Acked-by: Jiri Kosina Thanks. -- Jiri Kosina SUSE Labs
Re: [PATCH v4 2/3] arm64: implement live patching
On Thu, Nov 08, 2018 at 01:42:35PM +0100, Ard Biesheuvel wrote: > On 26 October 2018 at 16:21, Torsten Duwe wrote: > > /* The program counter just after the ftrace call site */ > > str lr, [x9, #S_PC] > > + > > /* The stack pointer as it was on ftrace_caller entry... */ > > add x28, fp, #16 > > str x28, [x9, #S_SP] > > Please drop this hunk Sure. I missed that one during cleanup. > > @@ -233,6 +234,10 @@ ftrace_common: ^^ > > ldr x28, [fp, 8] > > str x28, [x9, #S_LR]/* to pt_regs.r[30] */ > > > > +#if defined(CONFIG_LIVEPATCH) && defined(CONFIG_FUNCTION_GRAPH_TRACER) > > + mov x28, lr /* remember old return address */ > > +#endif > > + > > ldr_l x2, function_trace_op, x0 > > ldr x1, [fp, #8] > > sub x0, lr, #8 /* function entry == IP */ > > @@ -245,6 +250,17 @@ ftrace_call: > > > > bl ftrace_stub > > > > +#if defined(CONFIG_LIVEPATCH) && defined(CONFIG_FUNCTION_GRAPH_TRACER) > > + /* Is the trace function a live patcher an has messed with > > +* the return address? > > +*/ > > + add x9, sp, #16 /* advance to pt_regs for restore */ > > + ldr x0, [x9, #S_PC] > > + cmp x0, x28 /* compare with the value we remembered */ > > + /* to not call graph tracer's "call" mechanism twice! */ > > + b.neftrace_common_return > > Is ftrace_common_return guaranteed to be in range? Conditional > branches have only -/+ 1 MB range IIRC. It's the same function. A "1f" would do the same job, but the long label is a talking identifier that saves a comment. I'd more be worried about the return from the graph trace caller, which happens to be the _next_ function ;-) If ftrace_caller or graph_caller grow larger than a meg, something else is _very_ wrong. > > +#endif > > + > > #ifdef CONFIG_FUNCTION_GRAPH_TRACER > > Can we fold these #ifdef blocks together (i.e, incorporate the > conditional livepatch sequence here) I'll see how to make it fit. But remember some people might want ftrace but no live patching capability. Thanks for the review! Torsten
Re: [PATCH v2 1/3] dt-bindings: interrupt-controller: Actions external interrupt controller
On 8/13/18 9:44 PM, Rob Herring wrote:
On Sun, Aug 12, 2018 at 02:22:13PM +0200, Parthiban Nallathambi wrote:
Actions Semi OWL family SoC's provides support for external interrupt
controller to be connected and controlled using SIRQ pins. S500, S700
and S900 provides 3 SIRQ lines and works independently for 3 external
interrupt controllers.
Signed-off-by: Parthiban Nallathambi
Signed-off-by: Saravanan Sekar
---
.../interrupt-controller/actions,owl-sirq.txt | 46 ++
1 file changed, 46 insertions(+)
create mode 100644
Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.txt
diff --git
a/Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.txt
b/Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.txt
new file mode 100644
index ..4b8437751331
--- /dev/null
+++
b/Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.txt
@@ -0,0 +1,46 @@
+Actions Semi Owl SoCs SIRQ interrupt controller
+
+S500, S700 and S900 SoC's from Actions provides 3 SPI's from GIC,
+in which external interrupt controller can be connected. 3 SPI's
+45, 46, 47 from GIC are directly exposed as SIRQ. It has
+the following properties:
+
+- inputs three interrupt signal from external interrupt controller
+
+Required properties:
+
+- compatible: should be "actions,owl-sirq"
+- reg: physical base address of the controller and length of memory mapped.
+- interrupt-controller: identifies the node as an interrupt controller
+- #interrupt-cells: specifies the number of cells needed to encode an interrupt
+ source, should be 2.
+- actions,sirq-shared-reg: Applicable for S500 and S700 where SIRQ register
+ details are maintained at same offset/register.
+- actions,sirq-offset: register offset for SIRQ interrupts. When registers are
+ shared, all the three offsets will be same (S500 and S700).
You should have more specific compatible strings if there are
differences and these settings can be implied by them.
This to meant to get the register offset because s500, s700 uses the
same external interrupt controller register to provide three or more
interrupt line. But this is not the case for s900.
So should it be "actions,sirq-offset-reg"?
+- actions,sirq-clk-sel: external interrupt controller can be either
+ connected to 32Khz or 24Mhz external/internal clock. This needs
+ to be configured for per SIRQ line. Failing defaults to 32Khz clock.
What are the valid values?
+
+Example for S900:
+
+sirq: interrupt-controller@e01b {
+ compatible = "actions,owl-sirq";
+ reg = <0 0xe01b 0 0x1000>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ actions,sirq-clk-sel = <0 0 0>;
If 0 is 32khz, then having this is pointless. But I can't tell what the
values correspond to.
Thanks, clock selection will be removed and defaults to 24MHz.
+ actions,sirq-offset = <0x200 0x528 0x52c>;
+};
+
+Example for S500 and S700:
+
+sirq: interrupt-controller@e01b {
+ compatible = "actions,owl-sirq";
+ reg = <0 0xe01b 0 0x1000>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ actions,sirq-shared-reg;
+ actions,sirq-clk-sel = <0 0 0>;
+ actions,sirq-offset = <0x200 0x200 0x200>;
+};
--
2.14.4
--
Thanks,
Parthiban N
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-22 Fax: (+49)-8142-66989-80 Email: p...@denx.de
Re: [PATCH v2] mmc: sdhci-pci: Try "cd" for card-detect lookup before using NULL
On 29 October 2018 at 23:17, Rajat Jain wrote:
> Problem:
>
> The card detect IRQ does not work with modern BIOS (that want
> to use _DSD to provide the card detect GPIO to the driver).
>
> Details:
>
> The mmc core provides the mmc_gpiod_request_cd() API to let host drivers
> request the gpio descriptor for the "card detect" pin.
> This pin is specified in the ACPI for the SDHC device:
>
> * Either as a resource using _CRS. This is a method used by legacy BIOS.
>(The driver needs to tell which resource index).
>
> * Or as a named property ("cd-gpios"/"cd-gpio") in _DSD (which internally
>points to an entry in _CRS). This way, the driver can lookup using a
>string. This is what modern BIOS prefer to use.
>
> This API finally results in a call to the following code:
>
> struct gpio_desc *acpi_find_gpio(..., const char *con_id,...)
> {
> ...
>/* Lookup gpio (using "-gpio") in the _DSD */
> ...
>if (!acpi_can_fallback_to_crs(adev, con_id))
> return ERR_PTR(-ENOENT);
> ...
>/* Falling back to _CRS is allowed, Lookup gpio in the _CRS */
> ...
> }
>
> Note that this means that if the ACPI has _DSD properties, the kernel
> will never use _CRS for the lookup (Because acpi_can_fallback_to_crs()
> will always be false for any device hat has _DSD entries).
>
> The SDHCI driver is thus currently broken on a modern BIOS, even if
> BIOS provides both _CRS (for index based lookup) and _DSD entries (for
> string based lookup). Ironically, none of these will be used for the
> lookup currently because:
>
> * Since the con_id is NULL, acpi_find_gpio() does not find a matching
> entry in DSDT. (The _DSDT entry has the property name = "cd-gpios")
>
> * Because ACPI contains DSDT entries, thus acpi_can_fallback_to_crs()
> returns false (because device properties have been populated from
> _DSD), thus the _CRS is never used for the lookup.
>
> Fix:
>
> Try "cd" for lookup in the _DSD before falling back to using NULL so
> as to try looking up in the _CRS.
>
> I've tested this patch successfully with both Legacy BIOS (that
> provide only _CRS method) as well as modern BIOS (that provide both
> _CRS and _DSD). Also the use of "cd" appears to be fairly consistent
> across other users of this API (other MMC host controller drivers).
>
> Link: https://lkml.org/lkml/2018/9/25/1113
> Signed-off-by: Rajat Jain
Applied for fixes, thanks!
Should I add a stable tag to this as well?
Kind regards
Uffe
> ---
> v2: Fix the commit log to take care of Andy's comments.
>
> drivers/mmc/host/sdhci-pci-core.c | 7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci-pci-core.c
> b/drivers/mmc/host/sdhci-pci-core.c
> index 7bfd366d970d..e5c695b3 100644
> --- a/drivers/mmc/host/sdhci-pci-core.c
> +++ b/drivers/mmc/host/sdhci-pci-core.c
> @@ -1762,8 +1762,13 @@ static struct sdhci_pci_slot *sdhci_pci_probe_slot(
> device_init_wakeup(&pdev->dev, true);
>
> if (slot->cd_idx >= 0) {
> - ret = mmc_gpiod_request_cd(host->mmc, NULL, slot->cd_idx,
> + ret = mmc_gpiod_request_cd(host->mmc, "cd", slot->cd_idx,
>slot->cd_override_level, 0, NULL);
> + if (ret && ret != -EPROBE_DEFER)
> + ret = mmc_gpiod_request_cd(host->mmc, NULL,
> + slot->cd_idx,
> + slot->cd_override_level,
> + 0, NULL);
> if (ret == -EPROBE_DEFER)
> goto remove;
>
> --
> 2.19.1.568.g152ad8e336-goog
>
Re: [PATCH] mmc: sdhci: Convert sdhci_allocate_bounce_buffer() to return void
On 25 October 2018 at 04:12, Chunyan Zhang wrote:
> The function sdhci_allocate_bounce_buffer() always return zero at
> present, so there's no need to have a return value, that will also make
> error path easier.
>
> CC: Linus Walleij
> Signed-off-by: Chunyan Zhang
Applied for next, thanks!
Kind regards
Uffe
> ---
> drivers/mmc/host/sdhci.c | 15 +--
> 1 file changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 1b3fbd9..e94f4aa 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -3408,7 +3408,7 @@ void __sdhci_read_caps(struct sdhci_host *host, u16
> *ver, u32 *caps, u32 *caps1)
> }
> EXPORT_SYMBOL_GPL(__sdhci_read_caps);
>
> -static int sdhci_allocate_bounce_buffer(struct sdhci_host *host)
> +static void sdhci_allocate_bounce_buffer(struct sdhci_host *host)
> {
> struct mmc_host *mmc = host->mmc;
> unsigned int max_blocks;
> @@ -3446,7 +3446,7 @@ static int sdhci_allocate_bounce_buffer(struct
> sdhci_host *host)
> * Exiting with zero here makes sure we proceed with
> * mmc->max_segs == 1.
> */
> - return 0;
> + return;
> }
>
> host->bounce_addr = dma_map_single(mmc->parent,
> @@ -3456,7 +3456,7 @@ static int sdhci_allocate_bounce_buffer(struct
> sdhci_host *host)
> ret = dma_mapping_error(mmc->parent, host->bounce_addr);
> if (ret)
> /* Again fall back to max_segs == 1 */
> - return 0;
> + return;
> host->bounce_buffer_size = bounce_size;
>
> /* Lie about this since we're bouncing */
> @@ -3466,8 +3466,6 @@ static int sdhci_allocate_bounce_buffer(struct
> sdhci_host *host)
>
> pr_info("%s bounce up to %u segments into one, max segment size %u
> bytes\n",
> mmc_hostname(mmc), max_blocks, bounce_size);
> -
> - return 0;
> }
>
> int sdhci_setup_host(struct sdhci_host *host)
> @@ -3987,12 +3985,9 @@ int sdhci_setup_host(struct sdhci_host *host)
> */
> mmc->max_blk_count = (host->quirks & SDHCI_QUIRK_NO_MULTIBLOCK) ? 1 :
> 65535;
>
> - if (mmc->max_segs == 1) {
> + if (mmc->max_segs == 1)
> /* This may alter mmc->*_blk_* parameters */
> - ret = sdhci_allocate_bounce_buffer(host);
> - if (ret)
> - return ret;
> - }
> + sdhci_allocate_bounce_buffer(host);
>
> return 0;
>
> --
> 2.7.4
>
Re: [PATCH v4 2/3] arm64: implement live patching
On Mon, 12 Nov 2018 at 12:01, Torsten Duwe wrote: > > On Thu, Nov 08, 2018 at 01:42:35PM +0100, Ard Biesheuvel wrote: > > On 26 October 2018 at 16:21, Torsten Duwe wrote: > > > /* The program counter just after the ftrace call site */ > > > str lr, [x9, #S_PC] > > > + > > > /* The stack pointer as it was on ftrace_caller entry... */ > > > add x28, fp, #16 > > > str x28, [x9, #S_SP] > > > > Please drop this hunk > > Sure. I missed that one during cleanup. > > > > @@ -233,6 +234,10 @@ ftrace_common: > ^^ > > > ldr x28, [fp, 8] > > > str x28, [x9, #S_LR]/* to pt_regs.r[30] */ > > > > > > +#if defined(CONFIG_LIVEPATCH) && defined(CONFIG_FUNCTION_GRAPH_TRACER) > > > + mov x28, lr /* remember old return address */ > > > +#endif > > > + > > > ldr_l x2, function_trace_op, x0 > > > ldr x1, [fp, #8] > > > sub x0, lr, #8 /* function entry == IP */ > > > @@ -245,6 +250,17 @@ ftrace_call: > > > > > > bl ftrace_stub > > > > > > +#if defined(CONFIG_LIVEPATCH) && defined(CONFIG_FUNCTION_GRAPH_TRACER) > > > + /* Is the trace function a live patcher an has messed with > > > +* the return address? > > > +*/ > > > + add x9, sp, #16 /* advance to pt_regs for restore */ > > > + ldr x0, [x9, #S_PC] > > > + cmp x0, x28 /* compare with the value we remembered */ > > > + /* to not call graph tracer's "call" mechanism twice! */ > > > + b.neftrace_common_return > > > > Is ftrace_common_return guaranteed to be in range? Conditional > > branches have only -/+ 1 MB range IIRC. > > It's the same function. A "1f" would do the same job, but the long label > is a talking identifier that saves a comment. I'd more be worried about > the return from the graph trace caller, which happens to be the _next_ > function ;-) > > If ftrace_caller or graph_caller grow larger than a meg, something else is > _very_ wrong. > Ah ok. I confused myself into thinking that ftrace_common_return() was defined in another compilation unit > > > +#endif > > > + > > > #ifdef CONFIG_FUNCTION_GRAPH_TRACER > > > > Can we fold these #ifdef blocks together (i.e, incorporate the > > conditional livepatch sequence here) > > I'll see how to make it fit. But remember some people might want ftrace > but no live patching capability. > Sure. I simply mean turning this #if defined(CONFIG_LIVEPATCH) && defined(CONFIG_FUNCTION_GRAPH_TRACER) #endif #ifdef CONFIG_FUNCTION_GRAPH_TRACER #endif into #ifdef CONFIG_FUNCTION_GRAPH_TRACER #ifdef CONFIG_LIVEPATCH #endif #endif
Re: [RFC PATCH RESEND] tools/perf: Add Hygon Dhyana support
On Mon, Nov 12, 2018 at 03:40:51PM +0800, Pu Wen wrote:
> The tool perf is useful for the performance analysis on the Hygon Dhyana
> platform. But right now there is no Hygon support for it to analyze the
> KVM guest os data. So add Hygon Dhyana support to it by checking vendor
acme, pls fix that to "OS" when applying. Thx.
> string to share the code path of AMD.
>
> Signed-off-by: Pu Wen
> ---
> tools/perf/arch/x86/util/kvm-stat.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Acked-by: Borislav Petkov
> diff --git a/tools/perf/arch/x86/util/kvm-stat.c
> b/tools/perf/arch/x86/util/kvm-stat.c
> index b32409a..081353d 100644
> --- a/tools/perf/arch/x86/util/kvm-stat.c
> +++ b/tools/perf/arch/x86/util/kvm-stat.c
> @@ -156,7 +156,7 @@ int cpu_isa_init(struct perf_kvm_stat *kvm, const char
> *cpuid)
> if (strstr(cpuid, "Intel")) {
> kvm->exit_reasons = vmx_exit_reasons;
> kvm->exit_reasons_isa = "VMX";
> - } else if (strstr(cpuid, "AMD")) {
> + } else if (strstr(cpuid, "AMD") || strstr(cpuid, "Hygon")) {
> kvm->exit_reasons = svm_exit_reasons;
> kvm->exit_reasons_isa = "SVM";
> } else
> --
> 2.7.4
>
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Naga,
Just a few review comments for the v12 version.
On Fri, Nov 9, 2018 at 6:00 AM Naga Sureshkumar Relli
wrote:
> +#define PKT_OFST 0x00
> +#define PKT_CNT_SHIFT 12
> +
> +#define MEM_ADDR1_OFST 0x04
> +#define MEM_ADDR2_OFST 0x08
For the sake of readability I think *_OFFSET is preferred, especially
since the driver already includes short macro names. I think this is
similar to the EVNT vs EVENT point.
The majority of drivers in the Linux kernel do not shorten OFFSET -> OFST.
> +static void anfc_rw_pio_op(struct mtd_info *mtd, u8 *buf, int len,
> + bool do_read, int prog, int pktcount, int pktsize)
> +{
> + struct nand_chip *chip = mtd_to_nand(mtd);
> + struct anfc_nand_controller *nfc = to_anfc(chip->controller);
> + struct anfc_nand_chip *achip = to_anfc_nand(chip);
> + u32 *bufptr = (u32 *)buf;
> + u32 cnt = 0, intr = 0;
> +
> + anfc_config_dma(nfc, 0);
> +
> + if (pktsize == 0)
> + pktsize = len;
> +
> + anfc_setpktszcnt(nfc, pktsize, pktcount);
> +
> + if (!achip->strength)
> + intr = MBIT_ERROR;
> +
> + if (do_read)
> + intr |= READ_READY;
> + else
> + intr |= WRITE_READY;
> + anfc_enable_intrs(nfc, intr);
> + writel(prog, nfc->base + PROG_OFST);
> + while (cnt < pktcount) {
> + anfc_wait_for_event(nfc);
> + cnt++;
> + if (cnt == pktcount)
> + anfc_enable_intrs(nfc, XFER_COMPLETE);
> + if (do_read)
> + ioread32_rep(nfc->base + DATA_PORT_OFST, bufptr,
> +pktsize / 4);
> + else
> + iowrite32_rep(nfc->base + DATA_PORT_OFST, bufptr,
> + pktsize / 4);
> + bufptr += (pktsize / 4);
> + if (cnt < pktcount)
> + anfc_enable_intrs(nfc, intr);
> + }
> + anfc_wait_for_event(nfc);
> +}
Throughout the driver all calls to anfc_wait_for_event() ignores the
timeout return value. It would be nice to see some error handling in
case it times out - at minimum consider printing out an error message
since timeout on NAND operations are fairly critical and should
generally not occur. Perhaps even an argument can be made for
returning -ETIMEDOUT in case of timeout.
I'm putting a focus on this because I see the original non-upstream
Arasan driver sometimes timeout on NAND operations when I stress test
it via the UBI stress test. Not sure what the cause for the timeout is
yet but either way an error message would have been helpful.
Br, Martin
Re: [PATCH] HID: input: Ignore battery reported by Symbol DS4308
On Fri, Nov 9, 2018 at 9:50 AM Benjamin Tissoires
wrote:
>
> On Fri, Nov 9, 2018 at 12:59 AM Benson Leung wrote:
> >
> > The Motorola/Zebra Symbol DS4308-HD is a handheld USB barcode scanner
> > which does not have a battery, but reports one anyway that always has
> > capacity 2.
> >
> > Let's apply the IGNORE quirk to prevent it from being treated like a
> > power supply so that userspaces don't get confused that this
> > accessory is almost out of power and warn the user that they need to charge
> > their wired barcode scanner.
> >
> > Reported here: https://bugs.chromium.org/p/chromium/issues/detail?id=804720
> >
> > Signed-off-by: Benson Leung
> > ---
>
> Reviewed-by: Benjamin Tissoires
And applied, thanks
>
> > drivers/hid/hid-ids.h | 1 +
> > drivers/hid/hid-input.c | 3 +++
> > 2 files changed, 4 insertions(+)
> >
> > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> > index c0d668944dbe..07f6348b9f86 100644
> > --- a/drivers/hid/hid-ids.h
> > +++ b/drivers/hid/hid-ids.h
> > @@ -1043,6 +1043,7 @@
> > #define USB_VENDOR_ID_SYMBOL 0x05e0
> > #define USB_DEVICE_ID_SYMBOL_SCANNER_1 0x0800
> > #define USB_DEVICE_ID_SYMBOL_SCANNER_2 0x1300
> > +#define USB_DEVICE_ID_SYMBOL_SCANNER_3 0x1200
> >
> > #define USB_VENDOR_ID_SYNAPTICS0x06cb
> > #define USB_DEVICE_ID_SYNAPTICS_TP 0x0001
> > diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> > index a2f74e6adc70..44ea8e7c71a9 100644
> > --- a/drivers/hid/hid-input.c
> > +++ b/drivers/hid/hid-input.c
> > @@ -325,6 +325,9 @@ static const struct hid_device_id hid_battery_quirks[]
> > = {
> > { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_ELECOM,
> > USB_DEVICE_ID_ELECOM_BM084),
> > HID_BATTERY_QUIRK_IGNORE },
> > + { HID_USB_DEVICE(USB_VENDOR_ID_SYMBOL,
> > + USB_DEVICE_ID_SYMBOL_SCANNER_3),
> > + HID_BATTERY_QUIRK_IGNORE },
> > {}
> > };
> >
> > --
> > 2.19.1.930.g4563a0d9d0-goog
> >
Re: [PATCH v4 06/10] ARM: defconfig: Use the new FSL QSPI driver under the SPI framework
On 12.11.18 11:56, Boris Brezillon wrote: > On Mon, 12 Nov 2018 10:46:45 + > Schrempf Frieder wrote: > >> On 08.11.18 09:34, Boris Brezillon wrote: >>> On Wed, 7 Nov 2018 16:36:13 + >>> Schrempf Frieder wrote: >>> Hi Olof, On 07.11.18 17:20, Olof Johansson wrote: > On Wed, Nov 7, 2018 at 6:44 AM Frieder Schrempf > wrote: >> >> From: Frieder Schrempf >> >> The new driver at spi/spi-fsl-qspi.c replaces the old SPI NOR driver >> at mtd/fsl-quadspi.c. Switch to the new driver in the defconfigs. >> >> Signed-off-by: Frieder Schrempf > > Hi Frieder, > > This patch is part of a series that I didn't see the rest of, but in > general we prefer to merge these through arm-soc even if the driver > goes in through another tree. The way we'd prefer to handle it is that > once the driver lands, we'll take the config option change to turn it > on. To avoid our branches to break until both sides have landed, it > might be a good idea to keep both drivers on for a short while (one > release). > > So, I'm not going to ack this since we avoid taking defconfig changes > through driver trees (these two defconfigs tend to churn a lot and we > don't want to create merge conflicts where we don't have to), but > we'll be happy to pick it up when the time comes. Ok, thank you for explaining the common practice. I will drop the config changes for the next version and send it separately when the time is ready. Both the old driver and the new one use the same compatible strings for probing. Wouldn't that cause problems if both drivers are enabled for a while, or am I missing something? >>> >>> Or maybe we should not introduce a new Kconfig option and just reuse >>> the old one. It probably requires re-ordering patches a bit (patch 1 >>> should be moved after patch 5). Then you have 2 choices: >>> >>> 1/ merge patch 1 and 6 so that the new driver effectively replaces the >>> old one but uses the same Kconfig option >>> 2/ remove the ability to compile the old driver when the new one is >>> introduced: remove the line from drivers/mtd/spi-nor Makefile and >>> move the Kconfig entry from drivers/mtd/spi-nor/Kconfig to >>> drivers/spi/Kconfig. And remove the old code in a separate patch >>> >>> I'm fine either way, but option #2 will probably make the patch >>> introducing the new driver bigger and hurt readability. >> >> I think having both drivers in the tree for a while wouldn't be so bad. >> So if any compatibility issues come up with the new driver, people can >> still use the old one. > > Except that's not what happens in practice. Believe me, I tried this > approach several times, and people keep using the old driver until > they're forced to switch to the new one. So you actually don't address > the problem, you just delay it a bit, and you'll have to fix > regressions anyway. Ok, I see. >> Therefore I think I will drop the patches that change the defconfig and >> remove the old driver code and keep the different Kconfig options. And >> maybe add an exclusive dependency in Kconfig, so both drivers can not be >> enabled at the same time. >> >> Does this make sense? > > I'd really prefer to have the removal of the old driver in the same > release the new driver is introduced but if that's not possible, let's > have a clear plan, like "introduce new driver in release X, remove the > old one in release X+1". We can do it as you suggested. I will think about whether to use option #1 or #2. With #1 we will have the removal of the old driver and adding the new driver in one single patch. With #2 we will have the disabling of the old driver via Makefile in the same patch thats adding the new driver.
Re: [PATCH] misc: genwqe: should return proper error value.
On 2018-09-20 04:29, zhong jiang wrote:
The function should return -EFAULT when copy_from_user fails. Even
though the caller does not distinguish them. but we should keep
backward
compatibility.
Signed-off-by: zhong jiang
---
drivers/misc/genwqe/card_utils.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/misc/genwqe/card_utils.c
b/drivers/misc/genwqe/card_utils.c
index 8679e0b..f0961ec 100644
--- a/drivers/misc/genwqe/card_utils.c
+++ b/drivers/misc/genwqe/card_utils.c
@@ -298,7 +298,7 @@ static int genwqe_sgl_size(int num_pages)
int genwqe_alloc_sync_sgl(struct genwqe_dev *cd, struct genwqe_sgl
*sgl,
void __user *user_addr, size_t user_size, int write)
{
- int rc;
+ int ret = -ENOMEM;
struct pci_dev *pci_dev = cd->pci_dev;
sgl->fpage_offs = offset_in_page((unsigned long)user_addr);
@@ -318,7 +318,7 @@ int genwqe_alloc_sync_sgl(struct genwqe_dev *cd,
struct genwqe_sgl *sgl,
if (get_order(sgl->sgl_size) > MAX_ORDER) {
dev_err(&pci_dev->dev,
"[%s] err: too much memory requested!\n", __func__);
- return -ENOMEM;
+ return ret;
}
sgl->sgl = __genwqe_alloc_consistent(cd, sgl->sgl_size,
@@ -326,7 +326,7 @@ int genwqe_alloc_sync_sgl(struct genwqe_dev *cd,
struct genwqe_sgl *sgl,
if (sgl->sgl == NULL) {
dev_err(&pci_dev->dev,
"[%s] err: no memory available!\n", __func__);
- return -ENOMEM;
+ return ret;
}
/* Only use buffering on incomplete pages */
@@ -339,7 +339,7 @@ int genwqe_alloc_sync_sgl(struct genwqe_dev *cd,
struct genwqe_sgl *sgl,
/* Sync with user memory */
if (copy_from_user(sgl->fpage + sgl->fpage_offs,
user_addr, sgl->fpage_size)) {
- rc = -EFAULT;
+ ret = -EFAULT;
goto err_out;
}
}
@@ -352,7 +352,7 @@ int genwqe_alloc_sync_sgl(struct genwqe_dev *cd,
struct genwqe_sgl *sgl,
/* Sync with user memory */
if (copy_from_user(sgl->lpage, user_addr + user_size -
sgl->lpage_size, sgl->lpage_size)) {
- rc = -EFAULT;
+ ret = -EFAULT;
goto err_out2;
}
}
@@ -374,7 +374,8 @@ int genwqe_alloc_sync_sgl(struct genwqe_dev *cd,
struct genwqe_sgl *sgl,
sgl->sgl = NULL;
sgl->sgl_dma_addr = 0;
sgl->sgl_size = 0;
- return -ENOMEM;
+
+ return ret;
}
int genwqe_setup_sgl(struct genwqe_dev *cd, struct genwqe_sgl *sgl,
Thanks for the contribution. Should be fine to change that and have
a more precise failure information.
Signed-off-by: Frank Haverkamp
Re: [PATCH v2] mmc: sdhci-pci: Try "cd" for card-detect lookup before using NULL
On Mon, Nov 12, 2018 at 1:05 PM Ulf Hansson wrote:
>
> On 29 October 2018 at 23:17, Rajat Jain wrote:
> > Problem:
> >
> > The card detect IRQ does not work with modern BIOS (that want
> > to use _DSD to provide the card detect GPIO to the driver).
> >
> > Details:
> >
> > The mmc core provides the mmc_gpiod_request_cd() API to let host drivers
> > request the gpio descriptor for the "card detect" pin.
> > This pin is specified in the ACPI for the SDHC device:
> >
> > * Either as a resource using _CRS. This is a method used by legacy BIOS.
> >(The driver needs to tell which resource index).
> >
> > * Or as a named property ("cd-gpios"/"cd-gpio") in _DSD (which internally
> >points to an entry in _CRS). This way, the driver can lookup using a
> >string. This is what modern BIOS prefer to use.
> >
> > This API finally results in a call to the following code:
> >
> > struct gpio_desc *acpi_find_gpio(..., const char *con_id,...)
> > {
> > ...
> >/* Lookup gpio (using "-gpio") in the _DSD */
> > ...
> >if (!acpi_can_fallback_to_crs(adev, con_id))
> > return ERR_PTR(-ENOENT);
> > ...
> >/* Falling back to _CRS is allowed, Lookup gpio in the _CRS */
> > ...
> > }
> >
> > Note that this means that if the ACPI has _DSD properties, the kernel
> > will never use _CRS for the lookup (Because acpi_can_fallback_to_crs()
> > will always be false for any device hat has _DSD entries).
> >
> > The SDHCI driver is thus currently broken on a modern BIOS, even if
> > BIOS provides both _CRS (for index based lookup) and _DSD entries (for
> > string based lookup). Ironically, none of these will be used for the
> > lookup currently because:
> >
> > * Since the con_id is NULL, acpi_find_gpio() does not find a matching
> > entry in DSDT. (The _DSDT entry has the property name = "cd-gpios")
> >
> > * Because ACPI contains DSDT entries, thus acpi_can_fallback_to_crs()
> > returns false (because device properties have been populated from
> > _DSD), thus the _CRS is never used for the lookup.
> >
> > Fix:
> >
> > Try "cd" for lookup in the _DSD before falling back to using NULL so
> > as to try looking up in the _CRS.
> >
> > I've tested this patch successfully with both Legacy BIOS (that
> > provide only _CRS method) as well as modern BIOS (that provide both
> > _CRS and _DSD). Also the use of "cd" appears to be fairly consistent
> > across other users of this API (other MMC host controller drivers).
> >
> > Link: https://lkml.org/lkml/2018/9/25/1113
> > Signed-off-by: Rajat Jain
>
> Applied for fixes, thanks!
>
> Should I add a stable tag to this as well?
If you go with that it might make sense to have Fixes tag against
commit f10e4bf6632b5be11cea875b66ba959833a69258
Author: Andy Shevchenko
Date: Tue May 23 20:03:19 2017 +0300
gpio: acpi: Even more tighten up ACPI GPIO lookups
--
With Best Regards,
Andy Shevchenko
Re: [PATCH] mm: cleancache: fix corruption on missed inode invalidation
On Mon 12-11-18 12:57:34, Pavel Tikhomirov wrote:
> If all pages are deleted from the mapping by memory reclaim and also
> moved to the cleancache:
>
> __delete_from_page_cache
> (no shadow case)
> unaccount_page_cache_page
> cleancache_put_page
> page_cache_delete
> mapping->nrpages -= nr
> (nrpages becomes 0)
>
> We don't clean the cleancache for an inode after final file truncation
> (removal).
>
> truncate_inode_pages_final
> check (nrpages || nrexceptional) is false
> no truncate_inode_pages
> no cleancache_invalidate_inode(mapping)
>
> These way when reading the new file created with same inode we may get
> these trash leftover pages from cleancache and see wrong data instead of
> the contents of the new file.
>
> Fix it by always doing truncate_inode_pages which is already ready for
> nrpages == 0 && nrexceptional == 0 case and just invalidates inode.
>
> Fixes: commit 91b0abe36a7b ("mm + fs: store shadow entries in page cache")
> To: Andrew Morton
> Cc: Johannes Weiner
> Cc: Mel Gorman
> Cc: Jan Kara
> Cc: Matthew Wilcox
> Cc: Andi Kleen
> Cc: linux...@kvack.org
> Cc: linux-kernel@vger.kernel.org
> Reviewed-by: Vasily Averin
> Reviewed-by: Andrey Ryabinin
> Signed-off-by: Pavel Tikhomirov
> ---
> mm/truncate.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
The patch looks good but can you add a short comment before the
truncate_inode_pages() call explaining why it needs to be called always?
Something like:
/*
* Cleancache needs notification even if there are no pages or
* shadow entries...
*/
Otherwise you can add:
Reviewed-by: Jan Kara
Honza
>
> diff --git a/mm/truncate.c b/mm/truncate.c
> index 45d68e90b703..4c56c19e76eb 100644
> --- a/mm/truncate.c
> +++ b/mm/truncate.c
> @@ -517,9 +517,9 @@ void truncate_inode_pages_final(struct address_space
> *mapping)
>*/
> xa_lock_irq(&mapping->i_pages);
> xa_unlock_irq(&mapping->i_pages);
> -
> - truncate_inode_pages(mapping, 0);
> }
> +
> + truncate_inode_pages(mapping, 0);
> }
> EXPORT_SYMBOL(truncate_inode_pages_final);
>
> --
> 2.17.1
>
--
Jan Kara
SUSE Labs, CR
[PATCH v4 4/4] irqdesc: Add domain handler for NMIs
NMI handling code should be executed between calls to nmi_enter and
nmi_exit.
Add a separate domain handler to properly setup NMI context when handling
an interrupt requested as NMI.
Signed-off-by: Julien Thierry
Cc: Thomas Gleixner
Cc: Marc Zyngier
Cc: Will Deacon
Cc: Peter Zijlstra
---
include/linux/irqdesc.h | 5 +
kernel/irq/irqdesc.c| 35 +++
2 files changed, 40 insertions(+)
diff --git a/include/linux/irqdesc.h b/include/linux/irqdesc.h
index dd1e40d..ba05b0d 100644
--- a/include/linux/irqdesc.h
+++ b/include/linux/irqdesc.h
@@ -171,6 +171,11 @@ static inline int handle_domain_irq(struct irq_domain
*domain,
{
return __handle_domain_irq(domain, hwirq, true, regs);
}
+
+#ifdef CONFIG_IRQ_DOMAIN
+int handle_domain_nmi(struct irq_domain *domain, unsigned int hwirq,
+ struct pt_regs *regs);
+#endif
#endif
/* Test to see if a driver has successfully requested an irq */
diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
index 578d0e5..58934ea 100644
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -665,6 +665,41 @@ int __handle_domain_irq(struct irq_domain *domain,
unsigned int hwirq,
set_irq_regs(old_regs);
return ret;
}
+
+#ifdef CONFIG_IRQ_DOMAIN
+/**
+ * handle_domain_nmi - Invoke the handler for a HW irq belonging to a domain
+ * @domain:The domain where to perform the lookup
+ * @hwirq: The HW irq number to convert to a logical one
+ * @regs: Register file coming from the low-level handling code
+ *
+ * Returns:0 on success, or -EINVAL if conversion has failed
+ */
+int handle_domain_nmi(struct irq_domain *domain, unsigned int hwirq,
+ struct pt_regs *regs)
+{
+ struct pt_regs *old_regs = set_irq_regs(regs);
+ unsigned int irq;
+ int ret = 0;
+
+ nmi_enter();
+
+ irq = irq_find_mapping(domain, hwirq);
+
+ /*
+* ack_bad_irq is not NMI-safe, just report
+* an invalid interrupt.
+*/
+ if (likely(irq))
+ generic_handle_irq(irq);
+ else
+ ret = -EINVAL;
+
+ nmi_exit();
+ set_irq_regs(old_regs);
+ return ret;
+}
+#endif
#endif
/* Dynamic interrupt handling */
--
1.9.1
[PATCH v4 1/4] genirq: Provide basic NMI management for interrupt lines
Add functionality to allocate interrupt lines that will deliver IRQs
as Non-Maskable Interrupts. These allocations are only successful if
the irqchip provides the necessary support and allows NMI delivery for the
interrupt line.
Interrupt lines allocated for NMI delivery must be enabled/disabled through
enable_nmi/disable_nmi_nosync to keep their state consistent.
To treat a PERCPU IRQ as NMI, the interrupt must not be shared nor threaded,
the irqchip directly managing the IRQ must be the root irqchip and the
irqchip cannot be behind a slow bus.
Signed-off-by: Julien Thierry
Cc: Thomas Gleixner
Cc: Peter Zijlstra
Cc: Ingo Molnar
Cc: Marc Zyngier
---
include/linux/interrupt.h | 9 ++
include/linux/irq.h | 7 ++
kernel/irq/debugfs.c | 6 +-
kernel/irq/internals.h| 2 +
kernel/irq/manage.c | 228 +-
5 files changed, 249 insertions(+), 3 deletions(-)
diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index 1d6711c..b15e40c 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -156,6 +156,10 @@ struct irqaction {
unsigned long flags, const char *devname,
void __percpu *percpu_dev_id);
+extern int __must_check
+request_nmi(unsigned int irq, irq_handler_t handler, unsigned long flags,
+ const char *name, void *dev);
+
static inline int __must_check
request_percpu_irq(unsigned int irq, irq_handler_t handler,
const char *devname, void __percpu *percpu_dev_id)
@@ -167,6 +171,8 @@ struct irqaction {
extern const void *free_irq(unsigned int, void *);
extern void free_percpu_irq(unsigned int, void __percpu *);
+extern const void *free_nmi(unsigned int irq, void *dev_id);
+
struct device;
extern int __must_check
@@ -217,6 +223,9 @@ struct irqaction {
extern bool irq_percpu_is_enabled(unsigned int irq);
extern void irq_wake_thread(unsigned int irq, void *dev_id);
+extern void disable_nmi_nosync(unsigned int irq);
+extern void enable_nmi(unsigned int irq);
+
/* The following three functions are for the core kernel use only. */
extern void suspend_device_irqs(void);
extern void resume_device_irqs(void);
diff --git a/include/linux/irq.h b/include/linux/irq.h
index c9bffda..e0880a08 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -441,6 +441,8 @@ static inline irq_hw_number_t irqd_to_hwirq(struct irq_data
*d)
* @irq_set_vcpu_affinity: optional to target a vCPU in a virtual machine
* @ipi_send_single: send a single IPI to destination cpus
* @ipi_send_mask: send an IPI to destination cpus in cpumask
+ * @irq_nmi_setup: function called from core code before enabling an NMI
+ * @irq_nmi_teardown: function called from core code after disabling an NMI
* @flags: chip specific flags
*/
struct irq_chip {
@@ -489,6 +491,9 @@ struct irq_chip {
void(*ipi_send_single)(struct irq_data *data, unsigned int
cpu);
void(*ipi_send_mask)(struct irq_data *data, const struct
cpumask *dest);
+ int (*irq_nmi_setup)(struct irq_data *data);
+ void(*irq_nmi_teardown)(struct irq_data *data);
+
unsigned long flags;
};
@@ -504,6 +509,7 @@ struct irq_chip {
* IRQCHIP_ONESHOT_SAFE: One shot does not require mask/unmask
* IRQCHIP_EOI_THREADED: Chip requires eoi() on unmask in threaded mode
* IRQCHIP_SUPPORTS_LEVEL_MSI Chip can provide two doorbells for Level MSIs
+ * IRQCHIP_SUPPORTS_NMI: Chip can deliver NMIs, only for root irqchips
*/
enum {
IRQCHIP_SET_TYPE_MASKED = (1 << 0),
@@ -514,6 +520,7 @@ enum {
IRQCHIP_ONESHOT_SAFE= (1 << 5),
IRQCHIP_EOI_THREADED= (1 << 6),
IRQCHIP_SUPPORTS_LEVEL_MSI = (1 << 7),
+ IRQCHIP_SUPPORTS_NMI= (1 << 8),
};
#include
diff --git a/kernel/irq/debugfs.c b/kernel/irq/debugfs.c
index 6f63613..59a04d2 100644
--- a/kernel/irq/debugfs.c
+++ b/kernel/irq/debugfs.c
@@ -56,6 +56,7 @@ static void irq_debug_show_masks(struct seq_file *m, struct
irq_desc *desc) { }
BIT_MASK_DESCR(IRQCHIP_ONESHOT_SAFE),
BIT_MASK_DESCR(IRQCHIP_EOI_THREADED),
BIT_MASK_DESCR(IRQCHIP_SUPPORTS_LEVEL_MSI),
+ BIT_MASK_DESCR(IRQCHIP_SUPPORTS_NMI),
};
static void
@@ -140,6 +141,7 @@ static void irq_debug_show_masks(struct seq_file *m, struct
irq_desc *desc) { }
BIT_MASK_DESCR(IRQS_WAITING),
BIT_MASK_DESCR(IRQS_PENDING),
BIT_MASK_DESCR(IRQS_SUSPENDED),
+ BIT_MASK_DESCR(IRQS_NMI),
};
@@ -203,8 +205,8 @@ static ssize_t irq_debug_write(struct file *file, const
char __user *user_buf,
chip_bus_lock(desc);
raw_spin_lock_irqsave(&desc->lock, flags);
- if (irq_settings_is_level(desc)) {
- /* Can't do level, sorry */
+ if (irq_settings_is_level(desc)
[PATCH v4 2/4] genirq: Provide NMI management for percpu_devid interrupts
Add support for percpu_devid interrupts treated as NMIs.
Percpu_devid NMIs need to be setup/torn down on each CPU they target.
The same restrictions as for global NMIs still apply for percpu_devid NMIs.
Signed-off-by: Julien Thierry
Cc: Thomas Gleixner
Cc: Peter Zijlstra
Cc: Ingo Molnar
Cc: Marc Zyngier
---
include/linux/interrupt.h | 9 +++
kernel/irq/manage.c | 149 ++
2 files changed, 158 insertions(+)
diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index b15e40c..375442e 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -168,10 +168,15 @@ struct irqaction {
devname, percpu_dev_id);
}
+extern int __must_check
+request_percpu_nmi(unsigned int irq, irq_handler_t handler,
+ const char *devname, void __percpu *dev);
+
extern const void *free_irq(unsigned int, void *);
extern void free_percpu_irq(unsigned int, void __percpu *);
extern const void *free_nmi(unsigned int irq, void *dev_id);
+extern void free_percpu_nmi(unsigned int irq, void __percpu *percpu_dev_id);
struct device;
@@ -224,7 +229,11 @@ struct irqaction {
extern void irq_wake_thread(unsigned int irq, void *dev_id);
extern void disable_nmi_nosync(unsigned int irq);
+extern void disable_percpu_nmi(unsigned int irq);
extern void enable_nmi(unsigned int irq);
+extern void enable_percpu_nmi(unsigned int irq, unsigned int type);
+extern int ready_percpu_nmi(unsigned int irq);
+extern void teardown_percpu_nmi(unsigned int irq);
/* The following three functions are for the core kernel use only. */
extern void suspend_device_irqs(void);
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index 0ef7825..0d2d770 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -2182,6 +2182,11 @@ void enable_percpu_irq(unsigned int irq, unsigned int
type)
}
EXPORT_SYMBOL_GPL(enable_percpu_irq);
+void enable_percpu_nmi(unsigned int irq, unsigned int type)
+{
+ enable_percpu_irq(irq, type);
+}
+
/**
* irq_percpu_is_enabled - Check whether the per cpu irq is enabled
* @irq: Linux irq number to check for
@@ -2221,6 +2226,11 @@ void disable_percpu_irq(unsigned int irq)
}
EXPORT_SYMBOL_GPL(disable_percpu_irq);
+void disable_percpu_nmi(unsigned int irq)
+{
+ disable_percpu_irq(irq);
+}
+
/*
* Internal function to unregister a percpu irqaction.
*/
@@ -2252,6 +2262,8 @@ static struct irqaction *__free_percpu_irq(unsigned int
irq, void __percpu *dev_
/* Found it - now remove it from the list of entries: */
desc->action = NULL;
+ desc->istate &= ~IRQS_NMI;
+
raw_spin_unlock_irqrestore(&desc->lock, flags);
unregister_handler_proc(irq, action);
@@ -2305,6 +2317,19 @@ void free_percpu_irq(unsigned int irq, void __percpu
*dev_id)
}
EXPORT_SYMBOL_GPL(free_percpu_irq);
+void free_percpu_nmi(unsigned int irq, void __percpu *dev_id)
+{
+ struct irq_desc *desc = irq_to_desc(irq);
+
+ if (!desc || !irq_settings_is_per_cpu_devid(desc))
+ return;
+
+ if (WARN_ON(!(desc->istate & IRQS_NMI)))
+ return;
+
+ kfree(__free_percpu_irq(irq, dev_id));
+}
+
/**
* setup_percpu_irq - setup a per-cpu interrupt
* @irq: Interrupt line to setup
@@ -2395,6 +2420,130 @@ int __request_percpu_irq(unsigned int irq,
irq_handler_t handler,
EXPORT_SYMBOL_GPL(__request_percpu_irq);
/**
+ * request_percpu_nmi - allocate a percpu interrupt line for NMI delivery
+ * @irq: Interrupt line to allocate
+ * @handler: Function to be called when the IRQ occurs.
+ * @name: An ascii name for the claiming device
+ * @dev_id: A percpu cookie passed back to the handler function
+ *
+ * This call allocates interrupt resources and enables the
+ * interrupt on the local CPU. If the interrupt is supposed to be
+ * enabled on other CPUs, it has to be done on each CPU using
+ * enable_percpu_nmi().
+ *
+ * Dev_id must be globally unique. It is a per-cpu variable, and
+ * the handler gets called with the interrupted CPU's instance of
+ * that variable.
+ *
+ * Interrupt lines requested for NMI delivering should have auto enabling
+ * setting disabled.
+ *
+ * If the interrupt line cannot be used to deliver NMIs, function
+ * will fail returning a negative value.
+ */
+int request_percpu_nmi(unsigned int irq, irq_handler_t handler,
+ const char *name, void __percpu *dev_id)
+{
+ struct irqaction *action;
+ struct irq_desc *desc;
+ unsigned long flags;
+ int retval;
+
+ if (!handler)
+ return -EINVAL;
+
+ desc = irq_to_desc(irq);
+
+ if (!desc || !irq_settings_can_request(desc) ||
+ !irq_settings_is_per_cpu_devid(desc) ||
+ irq_settings_can_autoenable(desc) ||
+ !irq_supports_nmi(desc))
+ return -EINVAL;
+
+ /*
[PATCH v4 3/4] genirq: Provide NMI handlers
Provide flow handlers that are NMI safe for interrupts and percpu_devid
interrupts.
Signed-off-by: Julien Thierry
Cc: Thomas Gleixner
Cc: Marc Zyngier
Cc: Peter Zijlstra
---
include/linux/irq.h | 3 +++
kernel/irq/chip.c | 54 +
2 files changed, 57 insertions(+)
diff --git a/include/linux/irq.h b/include/linux/irq.h
index e0880a08..a1225dd 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -600,6 +600,9 @@ static inline int irq_set_parent(int irq, int parent_irq)
extern void handle_bad_irq(struct irq_desc *desc);
extern void handle_nested_irq(unsigned int irq);
+extern void handle_fasteoi_nmi(struct irq_desc *desc);
+extern void handle_percpu_devid_fasteoi_nmi(struct irq_desc *desc);
+
extern int irq_chip_compose_msi_msg(struct irq_data *data, struct msi_msg
*msg);
extern int irq_chip_pm_get(struct irq_data *data);
extern int irq_chip_pm_put(struct irq_data *data);
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index a2b3d9d..c332c17 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -730,6 +730,37 @@ void handle_fasteoi_irq(struct irq_desc *desc)
EXPORT_SYMBOL_GPL(handle_fasteoi_irq);
/**
+ * handle_fasteoi_nmi - irq handler for NMI interrupt lines
+ * @desc: the interrupt description structure for this irq
+ *
+ * A simple NMI-safe handler, considering the restrictions
+ * from request_nmi.
+ *
+ * Only a single callback will be issued to the chip: an ->eoi()
+ * call when the interrupt has been serviced. This enables support
+ * for modern forms of interrupt handlers, which handle the flow
+ * details in hardware, transparently.
+ */
+void handle_fasteoi_nmi(struct irq_desc *desc)
+{
+ struct irq_chip *chip = irq_desc_get_chip(desc);
+ struct irqaction *action = desc->action;
+ unsigned int irq = irq_desc_get_irq(desc);
+ irqreturn_t res;
+
+ trace_irq_handler_entry(irq, action);
+ /*
+* NMIs cannot be shared, there is only one action.
+*/
+ res = action->handler(irq, action->dev_id);
+ trace_irq_handler_exit(irq, action, res);
+
+ if (chip->irq_eoi)
+ chip->irq_eoi(&desc->irq_data);
+}
+EXPORT_SYMBOL_GPL(handle_fasteoi_nmi);
+
+/**
* handle_edge_irq - edge type IRQ handler
* @desc: the interrupt description structure for this irq
*
@@ -908,6 +939,29 @@ void handle_percpu_devid_irq(struct irq_desc *desc)
chip->irq_eoi(&desc->irq_data);
}
+/**
+ * handle_percpu_devid_fasteoi_nmi - Per CPU local NMI handler with per cpu
+ * dev ids
+ * @desc: the interrupt description structure for this irq
+ *
+ * Similar to handle_fasteoi_nmi, but handling the dev_id cookie
+ * as a percpu pointer.
+ */
+void handle_percpu_devid_fasteoi_nmi(struct irq_desc *desc)
+{
+ struct irq_chip *chip = irq_desc_get_chip(desc);
+ struct irqaction *action = desc->action;
+ unsigned int irq = irq_desc_get_irq(desc);
+ irqreturn_t res;
+
+ trace_irq_handler_entry(irq, action);
+ res = action->handler(irq, raw_cpu_ptr(action->percpu_dev_id));
+ trace_irq_handler_exit(irq, action, res);
+
+ if (chip->irq_eoi)
+ chip->irq_eoi(&desc->irq_data);
+}
+
static void
__irq_do_set_handler(struct irq_desc *desc, irq_flow_handler_t handle,
int is_chained, const char *name)
--
1.9.1
[PATCH v4 0/4] Provide core API for NMIs
Hi, This patch series provides a way for irqchips to define some IRQs as NMIs. As we hope (maybe) to get the arm64 use case for this into next, this version is simply a rebase of the previous one. Changes since v3[1]: - Rebased on v4.20-rc2 Changes since v2[2]: - Fix ARM build error reported by kbuild bot - Fix documentation warnings reported by kbuild bot Changes since v1[3]: - Rebased to v4.19-rc1 - Fix some coding style issues Changes since RFC[4]: - Rebased to v4.18-rc6 - Remove disable_nmi, one should call disable_nmi_nosync instead - Remove code related to affinity balancing from NMI functions - Require PERCPU configuration for NMIs [1] https://lkml.org/lkml/2018/8/31/173 [2] https://lkml.org/lkml/2018/8/28/661 [3] https://lkml.org/lkml/2018/7/24/321 [4] https://lkml.org/lkml/2018/7/9/768 Cheers, Julien --> Julien Thierry (4): genirq: Provide basic NMI management for interrupt lines genirq: Provide NMI management for percpu_devid interrupts genirq: Provide NMI handlers irqdesc: Add domain handler for NMIs include/linux/interrupt.h | 18 +++ include/linux/irq.h | 10 ++ include/linux/irqdesc.h | 5 + kernel/irq/chip.c | 54 +++ kernel/irq/debugfs.c | 6 +- kernel/irq/internals.h| 2 + kernel/irq/irqdesc.c | 35 + kernel/irq/manage.c | 377 +- 8 files changed, 504 insertions(+), 3 deletions(-) -- 1.9.1
Re: [PATCH] mm: cleancache: fix corruption on missed inode invalidation
On 11/12/18 2:31 PM, Jan Kara wrote:
> On Mon 12-11-18 12:57:34, Pavel Tikhomirov wrote:
>> If all pages are deleted from the mapping by memory reclaim and also
>> moved to the cleancache:
>>
>> __delete_from_page_cache
>> (no shadow case)
>> unaccount_page_cache_page
>> cleancache_put_page
>> page_cache_delete
>> mapping->nrpages -= nr
>> (nrpages becomes 0)
>>
>> We don't clean the cleancache for an inode after final file truncation
>> (removal).
>>
>> truncate_inode_pages_final
>> check (nrpages || nrexceptional) is false
>> no truncate_inode_pages
>> no cleancache_invalidate_inode(mapping)
>>
>> These way when reading the new file created with same inode we may get
>> these trash leftover pages from cleancache and see wrong data instead of
>> the contents of the new file.
>>
>> Fix it by always doing truncate_inode_pages which is already ready for
>> nrpages == 0 && nrexceptional == 0 case and just invalidates inode.
>>
>> Fixes: commit 91b0abe36a7b ("mm + fs: store shadow entries in page cache")
>> To: Andrew Morton
>> Cc: Johannes Weiner
>> Cc: Mel Gorman
>> Cc: Jan Kara
>> Cc: Matthew Wilcox
>> Cc: Andi Kleen
>> Cc: linux...@kvack.org
>> Cc: linux-kernel@vger.kernel.org
>> Reviewed-by: Vasily Averin
>> Reviewed-by: Andrey Ryabinin
>> Signed-off-by: Pavel Tikhomirov
>> ---
>> mm/truncate.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> The patch looks good but can you add a short comment before the
> truncate_inode_pages() call explaining why it needs to be called always?
> Something like:
>
>/*
> * Cleancache needs notification even if there are no pages or
> * shadow entries...
> */
Or we can just call cleancache_invalidate_inode(mapping) on else branch,
so the code would be more self-explanatory, and also avoid
function call in no-cleancache setups, which should the most of setups.
linux-next: Signed-off-by missing for commit in the nios2 tree
Hi Ley,
Commit
a070f18a62df ("nios2: TLBMISC writes do not require PID bits to be set")
is missing a Signed-off-by from its committer.
--
Cheers,
Stephen Rothwell
pgprVLgNY0QsC.pgp
Description: OpenPGP digital signature
Re: [PATCH] HID: hidraw: enforce minors_lock locking via lockdep
On Fri, 9 Nov 2018, Benjamin Tissoires wrote: > On Thu, Nov 8, 2018 at 10:38 PM Jiri Kosina wrote: > > > > From: Jiri Kosina > > > > lockdep is much more powerful enforcing the locking rules than code > > comments, > > so let's switch to it. > > > > Signed-off-by: Jiri Kosina > > --- > > Looks good to me. > > Reviewed-by: Benjamin Tissoires I actually missed the comment removal for hidraw_get_report(). I've ammeded the patch and comitted, keeping your tag, please shout if you disagree :) Thanks. -- Jiri Kosina SUSE Labs
Re: [PATCH v4 1/3] arm64: implement ftrace with regs
On Thu, Nov 08, 2018 at 01:12:42PM +0100, Ard Biesheuvel wrote:
>
> On 26 October 2018 at 16:21, Torsten Duwe wrote:
> > @@ -162,6 +165,114 @@ ftrace_graph_call://
> > ftrace_graph_cal
> >
> > mcount_exit
> > ENDPROC(ftrace_caller)
> > +#else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
> > +
> > +/*
> > + * Since no -pg or similar compiler flag is used, there should really be
> > + * no reference to _mcount; so do not define one. Only some value for
> > + * MCOUNT_ADDR is needed for comparison. Let it point here to have some
> > + * sort of magic value that can be recognised when debugging.
> > + */
> > + .global _mcount
> > +_mcount:
> > + ret /* make it differ from regs caller */
> > +
> > +ENTRY(ftrace_regs_caller)
> > + /* callee's preliminary stack frame: */
> > + stp fp, x9, [sp, #-16]!
>
> Does the 'fp' alias for x29 work with older assemblers? I guess it
> does not matter gor GCC 8+ code, but be careful when you rewrite
> existing stuff.
I had gotten the impression the fp alias was there ever since, so I used
it for readability. Thanks for the notification, I'll double check.
> > + mov fp, sp
> > +
> > + /* our stack frame: */
> > + stp fp, lr, [sp, #-S_FRAME_SIZE]!
>
> If sizeof(struct pt_regs) == S_FRAME_SIZE), you should subtract 16
> additional bytes here
This is intentional :-]
At the end of pt_regs there's a "stackframe", which now aligns with the
"preliminary" frame I create for the callee. Please tell me what the struct
member is good for if not for an actual callee stack frame...
I thought it was a neat idea.
> > +
> > +ftrace_common:
> > + /*
> > +* At this point we have 2 new stack frames, and x9 pointing
> > +* at a pt_regs which we can populate as needed.
> > +*/
> > +
> > + /* save function arguments */
> > + stp x0, x1, [x9]
> > + stp x2, x3, [x9, #S_X2]
> > + stp x4, x5, [x9, #S_X4]
> > + stp x6, x7, [x9, #S_X6]
> > + stp x8, x9, [x9, #S_X8]
> > +
>
> x9 is not a function argument, and if it were, you'd have clobbered it
> by now. Please use a single 'str' and store x8 only
This way the x9 slot in pt_regs will be undefined. Is that ok with everybody?
> > +ftrace_common_return:
> > + add x9, sp, #16 /* advance to pt_regs for restore */
> > +
> > + ldp x0, x1, [x9]
> > + ldp x2, x3, [x9, #S_X2]
> > + ldp x4, x5, [x9, #S_X4]
> > + ldp x6, x7, [x9, #S_X6]
> > + ldp x8, x9, [x9, #S_X8]
> > +
>
> Same as above. It also deserves a mention that you are relying on the
> absence of IPA-RA, and so x9..x18 are guaranteed to be dead at
> function entry, and so they don't need to be restored here.
Sure, I can quote some ABI spec here :-/
I just wish all arm code was such well documented.
> > --- a/arch/arm64/kernel/ftrace.c
> > +++ b/arch/arm64/kernel/ftrace.c
> > @@ -65,18 +65,61 @@ int ftrace_update_ftrace_func(ftrace_fun
> > return ftrace_modify_code(pc, 0, new, false);
> > }
> >
> > +#ifdef CONFIG_ARM64_MODULE_PLTS
> > +static int install_ftrace_trampoline(struct module *mod, unsigned long
> > *addr)
> > +{
> > + struct plt_entry trampoline, *mod_trampoline;
> > + trampoline = get_plt_entry(*addr);
> > +
> > + if (*addr == FTRACE_ADDR)
> > + mod_trampoline = mod->arch.ftrace_trampoline;
> > + else if (*addr == FTRACE_REGS_ADDR)
> > + mod_trampoline = mod->arch.ftrace_regs_trampoline;
>
> Could we do something like
>
> if (*addr == FTRACE_ADDR)
> mod_trampoline = &mod->arch.ftrace_trampoline[0];
> else if (*addr == FTRACE_REGS_ADDR)
> mod_trampoline = &mod->arch.ftrace_trampoline[1];
>
> and get rid of the additional struct field and pointer?
"0" and "1" won't make it obvious which one has the regs tracing, but besides
that, I like the idea of making this a small array. Other opinions?
Torsten
Re: [PATCH] mm: cleancache: fix corruption on missed inode invalidation
On Mon 12-11-18 14:40:06, Andrey Ryabinin wrote:
>
>
> On 11/12/18 2:31 PM, Jan Kara wrote:
> > On Mon 12-11-18 12:57:34, Pavel Tikhomirov wrote:
> >> If all pages are deleted from the mapping by memory reclaim and also
> >> moved to the cleancache:
> >>
> >> __delete_from_page_cache
> >> (no shadow case)
> >> unaccount_page_cache_page
> >> cleancache_put_page
> >> page_cache_delete
> >> mapping->nrpages -= nr
> >> (nrpages becomes 0)
> >>
> >> We don't clean the cleancache for an inode after final file truncation
> >> (removal).
> >>
> >> truncate_inode_pages_final
> >> check (nrpages || nrexceptional) is false
> >> no truncate_inode_pages
> >> no cleancache_invalidate_inode(mapping)
> >>
> >> These way when reading the new file created with same inode we may get
> >> these trash leftover pages from cleancache and see wrong data instead of
> >> the contents of the new file.
> >>
> >> Fix it by always doing truncate_inode_pages which is already ready for
> >> nrpages == 0 && nrexceptional == 0 case and just invalidates inode.
> >>
> >> Fixes: commit 91b0abe36a7b ("mm + fs: store shadow entries in page cache")
> >> To: Andrew Morton
> >> Cc: Johannes Weiner
> >> Cc: Mel Gorman
> >> Cc: Jan Kara
> >> Cc: Matthew Wilcox
> >> Cc: Andi Kleen
> >> Cc: linux...@kvack.org
> >> Cc: linux-kernel@vger.kernel.org
> >> Reviewed-by: Vasily Averin
> >> Reviewed-by: Andrey Ryabinin
> >> Signed-off-by: Pavel Tikhomirov
> >> ---
> >> mm/truncate.c | 4 ++--
> >> 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > The patch looks good but can you add a short comment before the
> > truncate_inode_pages() call explaining why it needs to be called always?
> > Something like:
> >
> > /*
> > * Cleancache needs notification even if there are no pages or
> > * shadow entries...
> > */
>
> Or we can just call cleancache_invalidate_inode(mapping) on else branch,
> so the code would be more self-explanatory, and also avoid
> function call in no-cleancache setups, which should the most of setups.
That is workable for me as well although I'd be somewhat worried that if we
have calls to inform cleancache about final inode teardown in two different
places, they can get out of sync easily. So I somewhat prefer the current
solution + comment.
Honza
--
Jan Kara
SUSE Labs, CR
[PATCH v6 03/24] arm64: cpufeature: Add cpufeature for IRQ priority masking
Add a cpufeature indicating whether a cpu supports masking interrupts
by priority.
The feature will be properly enabled in a later patch.
Signed-off-by: Julien Thierry
Cc: Catalin Marinas
Cc: Will Deacon
Cc: Marc Zyngier
Cc: Suzuki K Poulose
---
arch/arm64/include/asm/cpucaps.h| 3 ++-
arch/arm64/include/asm/cpufeature.h | 6 ++
arch/arm64/kernel/cpufeature.c | 23 +++
3 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h
index 6e2d254..f367e5c 100644
--- a/arch/arm64/include/asm/cpucaps.h
+++ b/arch/arm64/include/asm/cpucaps.h
@@ -54,7 +54,8 @@
#define ARM64_HAS_CRC3233
#define ARM64_SSBS 34
#define ARM64_WORKAROUND_1188873 35
+#define ARM64_HAS_IRQ_PRIO_MASKING 36
-#define ARM64_NCAPS36
+#define ARM64_NCAPS37
#endif /* __ASM_CPUCAPS_H */
diff --git a/arch/arm64/include/asm/cpufeature.h
b/arch/arm64/include/asm/cpufeature.h
index 7e2ec64..a6e063f 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -514,6 +514,12 @@ static inline bool system_supports_cnp(void)
cpus_have_const_cap(ARM64_HAS_CNP);
}
+static inline bool system_supports_irq_prio_masking(void)
+{
+ return IS_ENABLED(CONFIG_ARM64_PSEUDO_NMI) &&
+ cpus_have_const_cap(ARM64_HAS_IRQ_PRIO_MASKING);
+}
+
#define ARM64_SSBD_UNKNOWN -1
#define ARM64_SSBD_FORCE_DISABLE 0
#define ARM64_SSBD_KERNEL 1
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 03a9d96..1b5b553 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1145,6 +1145,14 @@ static void cpu_clear_disr(const struct
arm64_cpu_capabilities *__unused)
}
#endif /* CONFIG_ARM64_RAS_EXTN */
+#ifdef CONFIG_ARM64_PSEUDO_NMI
+static bool can_use_gic_priorities(const struct arm64_cpu_capabilities *entry,
+ int scope)
+{
+ return false;
+}
+#endif
+
static const struct arm64_cpu_capabilities arm64_features[] = {
{
.desc = "GIC system register CPU interface",
@@ -1368,6 +1376,21 @@ static void cpu_clear_disr(const struct
arm64_cpu_capabilities *__unused)
.cpu_enable = cpu_enable_cnp,
},
#endif
+#ifdef CONFIG_ARM64_PSEUDO_NMI
+ {
+ /*
+* Depends on having GICv3
+*/
+ .desc = "IRQ priority masking",
+ .capability = ARM64_HAS_IRQ_PRIO_MASKING,
+ .type = ARM64_CPUCAP_STRICT_BOOT_CPU_FEATURE,
+ .matches = can_use_gic_priorities,
+ .sys_reg = SYS_ID_AA64PFR0_EL1,
+ .field_pos = ID_AA64PFR0_GIC_SHIFT,
+ .sign = FTR_UNSIGNED,
+ .min_field_value = 1,
+ },
+#endif
{},
};
--
1.9.1
[PATCH v6 15/24] arm64: Switch to PMR masking when starting CPUs
Once the boot CPU has been prepared or a new secondary CPU has been
brought up, use ICC_PMR_EL1 to mask interrupts on that CPU and clear
PSR.I bit.
Since ICC_PMR_EL1 is initialized at CPU bringup, avoid overwriting
it in the GICv3 driver.
Signed-off-by: Julien Thierry
Suggested-by: Daniel Thompson
Cc: Catalin Marinas
Cc: Will Deacon
Cc: James Morse
Cc: Marc Zyngier
---
arch/arm64/kernel/smp.c | 27 +++
drivers/irqchip/irq-gic-v3.c | 8 +++-
2 files changed, 34 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 8dc9dde..e495360 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -35,6 +35,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -175,6 +176,25 @@ int __cpu_up(unsigned int cpu, struct task_struct *idle)
return ret;
}
+static void init_gic_priority_masking(void)
+{
+ u32 gic_sre = gic_read_sre();
+ u32 cpuflags;
+
+ if (WARN_ON(!(gic_sre & ICC_SRE_EL1_SRE)))
+ return;
+
+ WARN_ON(!irqs_disabled());
+
+ gic_write_pmr(GIC_PRIO_IRQOFF);
+
+ cpuflags = read_sysreg(daif);
+
+ /* We can only unmask PSR.I if we can take aborts */
+ if (!(cpuflags & PSR_A_BIT))
+ write_sysreg(cpuflags & ~PSR_I_BIT, daif);
+}
+
/*
* This is the secondary CPU boot entry. We're using this CPUs
* idle thread stack, but a set of temporary page tables.
@@ -211,6 +231,9 @@ asmlinkage notrace void secondary_start_kernel(void)
*/
check_local_cpu_capabilities();
+ if (system_supports_irq_prio_masking())
+ init_gic_priority_masking();
+
if (cpu_ops[cpu]->cpu_postboot)
cpu_ops[cpu]->cpu_postboot();
@@ -421,6 +444,10 @@ void __init smp_prepare_boot_cpu(void)
* and/or scheduling is enabled.
*/
apply_boot_alternatives();
+
+ /* Conditionally switch to GIC PMR for interrupt masking */
+ if (system_supports_irq_prio_masking())
+ init_gic_priority_masking();
}
static u64 __init of_get_cpu_mpidr(struct device_node *dn)
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index dbf5247..7f0b2e8 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -414,6 +414,9 @@ static u32 gic_get_pribits(void)
static bool gic_has_group0(void)
{
u32 val;
+ u32 old_pmr;
+
+ old_pmr = gic_read_pmr();
/*
* Let's find out if Group0 is under control of EL3 or not by
@@ -429,6 +432,8 @@ static bool gic_has_group0(void)
gic_write_pmr(BIT(8 - gic_get_pribits()));
val = gic_read_pmr();
+ gic_write_pmr(old_pmr);
+
return val != 0;
}
@@ -590,7 +595,8 @@ static void gic_cpu_sys_reg_init(void)
group0 = gic_has_group0();
/* Set priority mask register */
- write_gicreg(DEFAULT_PMR_VALUE, ICC_PMR_EL1);
+ if (!gic_prio_masking_enabled())
+ write_gicreg(DEFAULT_PMR_VALUE, ICC_PMR_EL1);
/*
* Some firmwares hand over to the kernel with the BPR changed from
--
1.9.1
[PATCH v6 16/24] arm64: gic-v3: Implement arch support for priority masking
Implement architecture specific primitive allowing the GICv3 driver to
use priorities to mask interrupts.
Lower the default priority of interrupts to a value maskable with
priority mask used for PMR. This is safe to do as both arm and arm64
only use one priority and are do not currently care about which priority
value it is, as long as all interrupts use the same priority.
Signed-off-by: Julien Thierry
Suggested-by: Daniel Thompson
Cc: Marc Zyngier
Cc: Catalin Marinas
Cc: Will Deacon
---
arch/arm64/include/asm/arch_gicv3.h| 8
include/linux/irqchip/arm-gic-common.h | 2 +-
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/arm64/include/asm/arch_gicv3.h
b/arch/arm64/include/asm/arch_gicv3.h
index 3f8d5f4..154612a 100644
--- a/arch/arm64/include/asm/arch_gicv3.h
+++ b/arch/arm64/include/asm/arch_gicv3.h
@@ -22,6 +22,7 @@
#ifndef __ASSEMBLY__
+#include
#include
#include
#include
@@ -162,14 +163,13 @@ static inline bool gic_prio_masking_enabled(void)
static inline void gic_pmr_mask_irqs(void)
{
- /* Should not get called yet. */
- WARN_ON_ONCE(true);
+ BUILD_BUG_ON(GICD_INT_DEF_PRI <= GIC_PRIO_IRQOFF);
+ gic_write_pmr(GIC_PRIO_IRQOFF);
}
static inline void gic_arch_enable_irqs(void)
{
- /* Should not get called yet. */
- WARN_ON_ONCE(true);
+ asm volatile ("msr daifclr, #2" : : : "memory");
}
#endif /* __ASSEMBLY__ */
diff --git a/include/linux/irqchip/arm-gic-common.h
b/include/linux/irqchip/arm-gic-common.h
index 9a1a479..2c9a4b3 100644
--- a/include/linux/irqchip/arm-gic-common.h
+++ b/include/linux/irqchip/arm-gic-common.h
@@ -13,7 +13,7 @@
#include
#include
-#define GICD_INT_DEF_PRI 0xa0
+#define GICD_INT_DEF_PRI 0xc0
#define GICD_INT_DEF_PRI_X4((GICD_INT_DEF_PRI << 24) |\
(GICD_INT_DEF_PRI << 16) |\
(GICD_INT_DEF_PRI << 8) |\
--
1.9.1
[PATCH v6 12/24] arm64: alternative: Allow alternative status checking per cpufeature
In preparation for the application of alternatives at different points
during the boot process, provide the possibility to check whether
alternatives for a feature of interest was already applied instead of
having a global boolean for all alternatives.
Make VHE enablement code check for the VHE feature instead of considering
all alternatives.
Signed-off-by: Julien Thierry
Cc: Catalin Marinas
Cc: Will Deacon
Cc: Suzuki K Poulose
Cc: Marc Zyngier
Cc: Christoffer Dall
---
arch/arm64/include/asm/alternative.h | 3 +--
arch/arm64/kernel/alternative.c | 21 +
arch/arm64/kernel/cpufeature.c | 2 +-
3 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/arch/arm64/include/asm/alternative.h
b/arch/arm64/include/asm/alternative.h
index 4b650ec..9806a23 100644
--- a/arch/arm64/include/asm/alternative.h
+++ b/arch/arm64/include/asm/alternative.h
@@ -14,8 +14,6 @@
#include
#include
-extern int alternatives_applied;
-
struct alt_instr {
s32 orig_offset;/* offset to original instruction */
s32 alt_offset; /* offset to replacement instruction */
@@ -28,6 +26,7 @@ typedef void (*alternative_cb_t)(struct alt_instr *alt,
__le32 *origptr, __le32 *updptr, int nr_inst);
void __init apply_alternatives_all(void);
+bool alternative_is_applied(u16 cpufeature);
#ifdef CONFIG_MODULES
void apply_alternatives_module(void *start, size_t length);
diff --git a/arch/arm64/kernel/alternative.c b/arch/arm64/kernel/alternative.c
index b5d6039..c947d22 100644
--- a/arch/arm64/kernel/alternative.c
+++ b/arch/arm64/kernel/alternative.c
@@ -32,13 +32,23 @@
#define ALT_ORIG_PTR(a)__ALT_PTR(a, orig_offset)
#define ALT_REPL_PTR(a)__ALT_PTR(a, alt_offset)
-int alternatives_applied;
+static int all_alternatives_applied;
+
+static DECLARE_BITMAP(applied_alternatives, ARM64_NCAPS);
struct alt_region {
struct alt_instr *begin;
struct alt_instr *end;
};
+bool alternative_is_applied(u16 cpufeature)
+{
+ if (WARN_ON(cpufeature >= ARM64_NCAPS))
+ return false;
+
+ return test_bit(cpufeature, applied_alternatives);
+}
+
/*
* Check if the target PC is within an alternative block.
*/
@@ -192,6 +202,9 @@ static void __apply_alternatives(void *alt_region, bool
is_module)
dsb(ish);
__flush_icache_all();
isb();
+
+ /* We applied all that was available */
+ bitmap_copy(applied_alternatives, cpu_hwcaps, ARM64_NCAPS);
}
}
@@ -208,14 +221,14 @@ static int __apply_alternatives_multi_stop(void *unused)
/* We always have a CPU 0 at this point (__init) */
if (smp_processor_id()) {
- while (!READ_ONCE(alternatives_applied))
+ while (!READ_ONCE(all_alternatives_applied))
cpu_relax();
isb();
} else {
- BUG_ON(alternatives_applied);
+ BUG_ON(all_alternatives_applied);
__apply_alternatives(®ion, false);
/* Barriers provided by the cache flushing */
- WRITE_ONCE(alternatives_applied, 1);
+ WRITE_ONCE(all_alternatives_applied, 1);
}
return 0;
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 1b5b553..dcf5d14 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1068,7 +1068,7 @@ static void cpu_copy_el2regs(const struct
arm64_cpu_capabilities *__unused)
* that, freshly-onlined CPUs will set tpidr_el2, so we don't need to
* do anything here.
*/
- if (!alternatives_applied)
+ if (!alternative_is_applied(ARM64_HAS_VIRT_HOST_EXTN))
write_sysreg(read_sysreg(tpidr_el1), tpidr_el2);
}
#endif
--
1.9.1
[PATCH v6 11/24] arm64: daifflags: Include PMR in daifflags restore operations
The addition of PMR should not bypass the semantics of daifflags. When DA_F are set, I bit is also set as no interrupts (even of higher priority) is allowed. When DA_F are cleared, I bit is cleared and interrupt enabling/disabling goes through ICC_PMR_EL1. Signed-off-by: Julien Thierry Cc: Catalin Marinas Cc: Will Deacon Cc: James Morse --- arch/arm64/include/asm/daifflags.h | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/arch/arm64/include/asm/daifflags.h b/arch/arm64/include/asm/daifflags.h index 546bc39..31936b3 100644 --- a/arch/arm64/include/asm/daifflags.h +++ b/arch/arm64/include/asm/daifflags.h @@ -18,8 +18,14 @@ #include -#define DAIF_PROCCTX 0 -#define DAIF_PROCCTX_NOIRQ PSR_I_BIT +#include + +#define DAIF_PROCCTX MAKE_ARCH_FLAGS(0, GIC_PRIO_IRQON) + +#define DAIF_PROCCTX_NOIRQ \ + (system_supports_irq_prio_masking() ? \ + MAKE_ARCH_FLAGS(0, GIC_PRIO_IRQOFF) : \ + MAKE_ARCH_FLAGS(PSR_I_BIT, GIC_PRIO_IRQON)) /* mask/save/unmask/restore all exceptions, including interrupts. */ static inline void local_daif_mask(void) -- 1.9.1
[PATCH v6 22/24] arm64: Skip preemption when exiting an NMI
Handling of an NMI should not set any TIF flags. For NMIs received from EL0 the current exit path is safe to use. However, an NMI received at EL1 could have interrupted some task context that has set the TIF_NEED_RESCHED flag. Preempting a task should not happen as a result of an NMI. Skip preemption after handling an NMI from EL1. Signed-off-by: Julien Thierry Cc: Catalin Marinas Cc: Will Deacon Cc: Marc Zyngier --- arch/arm64/kernel/entry.S | 8 1 file changed, 8 insertions(+) diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S index eb8120e..e02ee55 100644 --- a/arch/arm64/kernel/entry.S +++ b/arch/arm64/kernel/entry.S @@ -636,6 +636,14 @@ el1_irq: #ifdef CONFIG_PREEMPT ldr w24, [tsk, #TSK_TI_PREEMPT] // get preempt count +alternative_if ARM64_HAS_IRQ_PRIO_MASKING + /* +* DA_F were cleared at start of handling. If anything is set in DAIF, +* we come back from an NMI, so skip preemption +*/ + mrs x0, daif + orr w24, w24, w0 +alternative_else_nop_endif cbnzw24, 1f // preempt count != 0 ldr x0, [tsk, #TSK_TI_FLAGS]// get flags tbz x0, #TIF_NEED_RESCHED, 1f // needs rescheduling? -- 1.9.1
[PATCH v6 19/24] irqchip/gic: Add functions to access irq priorities
Add accessors to the GIC distributor/redistributors priority registers.
Signed-off-by: Julien Thierry
Cc: Thomas Gleixner
Cc: Jason Cooper
Cc: Marc Zyngier
---
drivers/irqchip/irq-gic-common.c | 10 ++
drivers/irqchip/irq-gic-common.h | 2 ++
2 files changed, 12 insertions(+)
diff --git a/drivers/irqchip/irq-gic-common.c b/drivers/irqchip/irq-gic-common.c
index 01e673c..910746f 100644
--- a/drivers/irqchip/irq-gic-common.c
+++ b/drivers/irqchip/irq-gic-common.c
@@ -98,6 +98,16 @@ int gic_configure_irq(unsigned int irq, unsigned int type,
return ret;
}
+void gic_set_irq_prio(unsigned int irq, void __iomem *base, u8 prio)
+{
+ writeb_relaxed(prio, base + GIC_DIST_PRI + irq);
+}
+
+u8 gic_get_irq_prio(unsigned int irq, void __iomem *base)
+{
+ return readb_relaxed(base + GIC_DIST_PRI + irq);
+}
+
void gic_dist_config(void __iomem *base, int gic_irqs,
void (*sync_access)(void))
{
diff --git a/drivers/irqchip/irq-gic-common.h b/drivers/irqchip/irq-gic-common.h
index 3919cd7..1586dbd 100644
--- a/drivers/irqchip/irq-gic-common.h
+++ b/drivers/irqchip/irq-gic-common.h
@@ -35,6 +35,8 @@ void gic_dist_config(void __iomem *base, int gic_irqs,
void gic_cpu_config(void __iomem *base, void (*sync_access)(void));
void gic_enable_quirks(u32 iidr, const struct gic_quirk *quirks,
void *data);
+void gic_set_irq_prio(unsigned int irq, void __iomem *base, u8 prio);
+u8 gic_get_irq_prio(unsigned int irq, void __iomem *base);
void gic_set_kvm_info(const struct gic_kvm_info *info);
--
1.9.1
[PATCH v6 17/24] irqchip/gic-v3: Detect if GIC can support pseudo-NMIs
The values non secure EL1 needs to use for PMR and RPR registers depends on
the value of SCR_EL3.FIQ.
The values non secure EL1 sees from the distributor and redistributor
depend on whether security is enabled for the GIC or not.
To avoid having to deal with two sets of values for PMR
masking/unmasking, only enable pseudo-NMIs when GIC has non-secure view
of priorities.
Also, add firmware requirements related to SCR_EL3.
Signed-off-by: Julien Thierry
Cc: Catalin Marinas
Cc: Will Deacon
Cc: Jonathan Corbet
Cc: Thomas Gleixner
Cc: Jason Cooper
Cc: Marc Zyngier
---
Documentation/arm64/booting.txt | 5
drivers/irqchip/irq-gic-v3.c| 58 -
2 files changed, 57 insertions(+), 6 deletions(-)
diff --git a/Documentation/arm64/booting.txt b/Documentation/arm64/booting.txt
index 8d0df62..e387938 100644
--- a/Documentation/arm64/booting.txt
+++ b/Documentation/arm64/booting.txt
@@ -188,6 +188,11 @@ Before jumping into the kernel, the following conditions
must be met:
the kernel image will be entered must be initialised by software at a
higher exception level to prevent execution in an UNKNOWN state.
+ - SCR_EL3.FIQ must have the same value across all CPUs the kernel is
+executing on.
+ - The value of SCR_EL3.FIQ must be the same as the one present at boot
+time whenever the kernel is executing.
+
For systems with a GICv3 interrupt controller to be used in v3 mode:
- If EL3 is present:
ICC_SRE_EL3.Enable (bit 3) must be initialiased to 0b1.
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index 7f0b2e8..5245791 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -63,6 +63,31 @@ struct gic_chip_data {
static struct gic_chip_data gic_data __read_mostly;
static DEFINE_STATIC_KEY_TRUE(supports_deactivate_key);
+/*
+ * The behaviours of RPR and PMR registers differ depending on the value of
+ * SCR_EL3.FIQ, and the behaviour of non-secure priority registers of the
+ * distributor and redistributors depends on whether security is enabled in the
+ * GIC.
+ *
+ * When security is enabled, non-secure priority values from the
(re)distributor
+ * are presented to the GIC CPUIF as follow:
+ * (GIC_(R)DIST_PRI[irq] >> 1) | 0x80;
+ *
+ * If SCR_EL3.FIQ == 1, the values writen to/read from PMR and RPR at
non-secure
+ * EL1 are subject to a similar operation thus matching the priorities
presented
+ * from the (re)distributor when security is enabled.
+ *
+ * see GICv3/GICv4 Architecture Specification (IHI0069D):
+ * - section 4.8.1 Non-secure accesses to register fields for Secure interrupt
+ * priorities.
+ * - Figure 4-7 Secure read of the priority field for a Non-secure Group 1
+ * interrupt.
+ *
+ * For now, we only support pseudo-NMIs if we have non-secure view of
+ * priorities.
+ */
+static DEFINE_STATIC_KEY_FALSE(supports_pseudo_nmis);
+
static struct gic_kvm_info gic_v3_kvm_info;
static DEFINE_PER_CPU(bool, has_rss);
@@ -226,6 +251,12 @@ static void gic_unmask_irq(struct irq_data *d)
gic_poke_irq(d, GICD_ISENABLER);
}
+static inline bool gic_supports_nmi(void)
+{
+ return IS_ENABLED(CONFIG_ARM64_PSEUDO_NMI) &&
+ static_branch_likely(&supports_pseudo_nmis);
+}
+
static int gic_irq_set_irqchip_state(struct irq_data *d,
enum irqchip_irq_state which, bool val)
{
@@ -572,6 +603,12 @@ static void gic_update_vlpi_properties(void)
!gic_data.rdists.has_direct_lpi ? "no " : "");
}
+/* Check whether it's single security state view */
+static inline bool gic_dist_security_disabled(void)
+{
+ return readl_relaxed(gic_data.dist_base + GICD_CTLR) & GICD_CTLR_DS;
+}
+
static void gic_cpu_sys_reg_init(void)
{
int i, cpu = smp_processor_id();
@@ -597,6 +634,9 @@ static void gic_cpu_sys_reg_init(void)
/* Set priority mask register */
if (!gic_prio_masking_enabled())
write_gicreg(DEFAULT_PMR_VALUE, ICC_PMR_EL1);
+ else if (gic_supports_nmi() && group0)
+ /* Mismatch configuration with boot CPU */
+ WARN_ON(!gic_dist_security_disabled());
/*
* Some firmwares hand over to the kernel with the BPR changed from
@@ -851,12 +891,6 @@ static int gic_set_affinity(struct irq_data *d, const
struct cpumask *mask_val,
#endif
#ifdef CONFIG_CPU_PM
-/* Check whether it's single security state view */
-static bool gic_dist_security_disabled(void)
-{
- return readl_relaxed(gic_data.dist_base + GICD_CTLR) & GICD_CTLR_DS;
-}
-
static int gic_cpu_pm_notifier(struct notifier_block *self,
unsigned long cmd, void *v)
{
@@ -1100,6 +1134,11 @@ static int partition_domain_translate(struct irq_domain
*d,
.select = gic_irq_domain_select,
};
+static void gic_enable_nmi_support(void)
+{
+ static_branch_enable(&supports_pseudo_nmis);
+}
+
static int __init gic_init_bases
[PATCH v6 18/24] irqchip/gic-v3: Handle pseudo-NMIs
Provide a higher priority to be used for pseudo-NMIs. When such an
interrupt is received, keep interrupts fully disabled at CPU level to
prevent receiving other pseudo-NMIs while handling the current one.
Signed-off-by: Julien Thierry
Cc: Thomas Gleixner
Cc: Jason Cooper
Cc: Marc Zyngier
---
drivers/irqchip/irq-gic-v3.c | 35 +--
1 file changed, 29 insertions(+), 6 deletions(-)
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index 5245791..c763f1a 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -41,6 +41,8 @@
#include "irq-gic-common.h"
+#define GICD_INT_NMI_PRI 0xa0
+
struct redist_region {
void __iomem*redist_base;
phys_addr_t phys_base;
@@ -375,6 +377,16 @@ static u64 gic_mpidr_to_affinity(unsigned long mpidr)
return aff;
}
+static inline void gic_deactivate_unexpected_irq(u32 irqnr)
+{
+ if (static_branch_likely(&supports_deactivate_key)) {
+ if (irqnr < 8192)
+ gic_write_dir(irqnr);
+ } else {
+ gic_write_eoir(irqnr);
+ }
+}
+
static asmlinkage void __exception_irq_entry gic_handle_irq(struct pt_regs
*regs)
{
u32 irqnr;
@@ -384,6 +396,22 @@ static asmlinkage void __exception_irq_entry
gic_handle_irq(struct pt_regs *regs
if (likely(irqnr > 15 && irqnr < 1020) || irqnr >= 8192) {
int err;
+ if (gic_supports_nmi() &&
+ unlikely(gic_read_rpr() == GICD_INT_NMI_PRI)) {
+ if (static_branch_likely(&supports_deactivate_key))
+ gic_write_eoir(irqnr);
+ /*
+* Leave the PSR.I bit set to prevent other NMIs to be
+* received while handling this one.
+* PSR.I will be restored when we ERET to the
+* interrupted context.
+*/
+ err = handle_domain_nmi(gic_data.domain, irqnr, regs);
+ if (err)
+ gic_deactivate_unexpected_irq(irqnr);
+ return;
+ }
+
if (gic_prio_masking_enabled()) {
gic_pmr_mask_irqs();
gic_arch_enable_irqs();
@@ -397,12 +425,7 @@ static asmlinkage void __exception_irq_entry
gic_handle_irq(struct pt_regs *regs
err = handle_domain_irq(gic_data.domain, irqnr, regs);
if (err) {
WARN_ONCE(true, "Unexpected interrupt received!\n");
- if (static_branch_likely(&supports_deactivate_key)) {
- if (irqnr < 8192)
- gic_write_dir(irqnr);
- } else {
- gic_write_eoir(irqnr);
- }
+ gic_deactivate_unexpected_irq(irqnr);
}
return;
}
--
1.9.1
[PATCH v6 20/24] irqchip/gic-v3: Allow interrupts to be set as pseudo-NMI
Implement NMI callbacks for GICv3 irqchip. Install NMI safe handlers
when setting up interrupt line as NMI.
Only SPIs and PPIs are allowed to be set up as NMI.
Signed-off-by: Julien Thierry
Cc: Thomas Gleixner
Cc: Jason Cooper
Cc: Marc Zyngier
---
drivers/irqchip/irq-gic-v3.c | 84
1 file changed, 84 insertions(+)
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index c763f1a..f22ae49 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include
#include
@@ -90,6 +91,9 @@ struct gic_chip_data {
*/
static DEFINE_STATIC_KEY_FALSE(supports_pseudo_nmis);
+/* ppi_nmi_refs[n] == number of cpus having ppi[n + 16] set as NMI */
+static refcount_t ppi_nmi_refs[16];
+
static struct gic_kvm_info gic_v3_kvm_info;
static DEFINE_PER_CPU(bool, has_rss);
@@ -314,6 +318,72 @@ static int gic_irq_get_irqchip_state(struct irq_data *d,
return 0;
}
+static int gic_irq_nmi_setup(struct irq_data *d)
+{
+ struct irq_desc *desc = irq_to_desc(d->irq);
+
+ if (!gic_supports_nmi())
+ return -EINVAL;
+
+ if (gic_peek_irq(d, GICD_ISENABLER)) {
+ pr_err("Cannot set NMI property of enabled IRQ %u\n", d->irq);
+ return -EINVAL;
+ }
+
+ /*
+* A secondary irq_chip should be in charge of LPI request,
+* it should not be possible to get there
+*/
+ if (WARN_ON(gic_irq(d) >= 8192))
+ return -EINVAL;
+
+ /* desc lock should already be held */
+ if (gic_irq(d) < 32) {
+ /* Setting up PPI as NMI, only switch handler for first NMI */
+ if (!refcount_inc_not_zero(&ppi_nmi_refs[gic_irq(d) - 16])) {
+ refcount_set(&ppi_nmi_refs[gic_irq(d) - 16], 1);
+ desc->handle_irq = handle_percpu_devid_fasteoi_nmi;
+ }
+ } else {
+ desc->handle_irq = handle_fasteoi_nmi;
+ }
+
+ gic_set_irq_prio(gic_irq(d), gic_dist_base(d), GICD_INT_NMI_PRI);
+
+ return 0;
+}
+
+static void gic_irq_nmi_teardown(struct irq_data *d)
+{
+ struct irq_desc *desc = irq_to_desc(d->irq);
+
+ if (WARN_ON(!gic_supports_nmi()))
+ return;
+
+ if (gic_peek_irq(d, GICD_ISENABLER)) {
+ pr_err("Cannot set NMI property of enabled IRQ %u\n", d->irq);
+ return;
+ }
+
+ /*
+* A secondary irq_chip should be in charge of LPI request,
+* it should not be possible to get there
+*/
+ if (WARN_ON(gic_irq(d) >= 8192))
+ return;
+
+ /* desc lock should already be held */
+ if (gic_irq(d) < 32) {
+ /* Tearing down NMI, only switch handler for last NMI */
+ if (refcount_dec_and_test(&ppi_nmi_refs[gic_irq(d) - 16]))
+ desc->handle_irq = handle_percpu_devid_irq;
+ } else {
+ desc->handle_irq = handle_fasteoi_irq;
+ }
+
+ gic_set_irq_prio(gic_irq(d), gic_dist_base(d), GICD_INT_DEF_PRI);
+}
+
static void gic_eoi_irq(struct irq_data *d)
{
gic_write_eoir(gic_irq(d));
@@ -950,6 +1020,8 @@ static inline void gic_cpu_pm_init(void) { }
.irq_set_affinity = gic_set_affinity,
.irq_get_irqchip_state = gic_irq_get_irqchip_state,
.irq_set_irqchip_state = gic_irq_set_irqchip_state,
+ .irq_nmi_setup = gic_irq_nmi_setup,
+ .irq_nmi_teardown = gic_irq_nmi_teardown,
.flags = IRQCHIP_SET_TYPE_MASKED |
IRQCHIP_SKIP_SET_WAKE |
IRQCHIP_MASK_ON_SUSPEND,
@@ -965,6 +1037,8 @@ static inline void gic_cpu_pm_init(void) { }
.irq_get_irqchip_state = gic_irq_get_irqchip_state,
.irq_set_irqchip_state = gic_irq_set_irqchip_state,
.irq_set_vcpu_affinity = gic_irq_set_vcpu_affinity,
+ .irq_nmi_setup = gic_irq_nmi_setup,
+ .irq_nmi_teardown = gic_irq_nmi_teardown,
.flags = IRQCHIP_SET_TYPE_MASKED |
IRQCHIP_SKIP_SET_WAKE |
IRQCHIP_MASK_ON_SUSPEND,
@@ -1159,7 +1233,17 @@ static int partition_domain_translate(struct irq_domain
*d,
static void gic_enable_nmi_support(void)
{
+ int i;
+
+ for (i = 0; i < 16; i++)
+ refcount_set(&ppi_nmi_refs[i], 0);
+
static_branch_enable(&supports_pseudo_nmis);
+
+ if (static_branch_likely(&supports_deactivate_key))
+ gic_eoimode1_chip.flags |= IRQCHIP_SUPPORTS_NMI;
+ else
+ gic_chip.flags |= IRQCHIP_SUPPORTS_NMI;
}
static int __init gic_init_bases(void __iomem *dist_base,
--
1.9.1
RE: scsi_set_medium_removal timeout issue
Hi Alan: >-Original Message- >From: Alan Stern [mailto:st...@rowland.harvard.edu] >Sent: Wednesday, October 31, 2018 10:20 PM >To: Zengtao (B) >Cc: j...@linux.vnet.ibm.com; martin.peter...@oracle.com; >gre...@linuxfoundation.org; linux-s...@vger.kernel.org; >linux-kernel@vger.kernel.org; linux-...@vger.kernel.org; >usb-stor...@lists.one-eyed-alien.net >Subject: RE: scsi_set_medium_removal timeout issue > >On Wed, 31 Oct 2018, Zengtao (B) wrote: > >> Hi: >> >> >-Original Message- >> >From: Alan Stern [mailto:st...@rowland.harvard.edu] >> >Sent: Tuesday, October 30, 2018 10:08 PM >> >To: Zengtao (B) >> >Cc: j...@linux.vnet.ibm.com; martin.peter...@oracle.com; >> >gre...@linuxfoundation.org; linux-s...@vger.kernel.org; >> >linux-kernel@vger.kernel.org; linux-...@vger.kernel.org; >> >usb-stor...@lists.one-eyed-alien.net >> >Subject: Re: scsi_set_medium_removal timeout issue >> > >> >On Tue, 30 Oct 2018, Zengtao (B) wrote: >> > >> >> Hi >> >> >> >> I have recently met a scsi_set_medium_removal timeout issue, and >> >> it's related to both SCSI and USB MASS storage. >> >> Since i am not an expert in either scsi or usb mass storage, i am >> >> writing to report the issue and ask for a solution from you guys. >> >> >> >> My test scenario is as follow: >> >> 1.Linux HOST-Linux mass storage gadget(the back store is a >> >> flash >> >partition). >> >> 2.Host mount the device. >> >> 3.Host writes some data to the Mass storage device. >> >> 4.Host Unmount the device. >> >> Both Linux kernels(Host and Device) are Linux 4.9. >> >> Some has reported the same issue a long time ago, but it remains >there. >> >> https://www.spinics.net/lists/linux-usb/msg53739.html >> >> >> >> For the issue itself, there is my observation: >> >> In the step 4, the Host will issue an >> >PREVENT_ALLOW_MEDIUM_REMOVAL scsi command. >> >> and and timeout happens due to the device 's very slow >> >fsg_lun_fsync_sub. >> > >> >Something is wrong here. Before sending PREVENT-ALLOW MEDIUM >> >REMOVAL, the host should issue SYNCHRONIZE CACHE. This will force >> >fsg_lun_fsync_sub to run, and the host should allow a long timeout >> >for this command. Then when PREVENT-ALLOW MEDIUM REMOVAL >is sent, >> >nothing will need to be flushed. >> > >> >> Definitely, I haven't seen the SYNCHRONIZE CACHE from the host, it >> directly issued the PREVENT-ALLOW MEDIUM REMOVAL, so maybe >something >> wrong with the scsi layer or something wrong with the mass storage >class driver? > >Or it could be something else. Can you please post the dmesg log from >the host, showing what happens when the device is first plugged in? > I have enabled the SCSI log for the host, please refer to the attachment. Thanks. Zengtao ~ # ~ # ~ # ~ # ~ # mkfs.vfat /dev/sda1 moun~ # ~ # mount /dev/sda1 /mnt ~ # ~ # time dd if=/dev/zero of=/mnt/test0 bs=16k count=5120 5120+0 records in 5120+0 records out 83886080 bytes (80.0MB) copied, 2.970369 seconds, 26.9MB/s real0m 2.97s user0m 0.01s sys 0m 0.76s ~ # ~ # umount /mnt sd 0:0:0:0: [sda] tag#0 Done: SUCCESS Result: hostbyte=DID_OK driverbyte=DRIVER_OK sd 0:0:0:0: [sda] tag#0 CDB: Write(10) 2a 00 00 02 12 f6 00 00 80 00 sd 0:0:0:0: [sda] tag#0 scsi host busy 1 failed 0 sd 0:0:0:0: Notifying upper driver of completion (result 0) sd 0:0:0:0: [sda] tag#0 Send: scmd 0xce13b800 sd 0:0:0:0: [sda] tag#0 CDB: Write(10) 2a 00 00 02 13 76 00 00 f0 00 sd 0:0:0:0: [sda] tag#0 Done: SUCCESS Result: hostbyte=DID_OK driverbyte=DRIVER_OK sd 0:0:0:0: [sda] tag#0 CDB: Write(10) 2a 00 00 02 13 76 00 00 f0 00 sd 0:0:0:0: [sda] tag#0 scsi host busy 1 failed 0 sd 0:0:0:0: Notifying upper driver of completion (result 0) sd 0:0:0:0: [sda] tag#0 Send: scmd 0xce13b700 sd 0:0:0:0: [sda] tag#0 CDB: Write(10) 2a 00 00 02 14 66 00 00 f0 00 sd 0:0:0:0: [sda] tag#0 Done: SUCCESS Result: hostbyte=DID_OK driverbyte=DRIVER_OK sd 0:0:0:0: [sda] tag#0 CDB: Write(10) 2a 00 00 02 14 66 00 00 f0 00 sd 0:0:0:0: [sda] tag#0 scsi host busy 1 failed 0 sd 0:0:0:0: Notifying upper driver of completion (result 0) sd 0:0:0:0: [sda] tag#0 Send: scmd 0xce13b800 sd 0:0:0:0: [sda] tag#0 CDB: Write(10) 2a 00 00 02 15 56 00 00 f0 00 sd 0:0:0:0: [sda] tag#0 Done: SUCCESS Result: hostbyte=DID_OK driverbyte=DRIVER_OK sd 0:0:0:0: [sda] tag#0 CDB: Write(10) 2a 00 00 02 15 56 00 00 f0 00 sd 0:0:0:0: [sda] tag#0 scsi host busy 1 failed 0 sd 0:0:0:0: Notifying upper driver of completion (result 0) sd 0:0:0:0: [sda] tag#0 Send: scmd 0xce13b700 sd 0:0:0:0: [sda] tag#0 CDB: Write(10) 2a 00 00 02 16 46 00 00 f0 00 sd 0:0:0:0: [sda] tag#0 Done: SUCCESS Result: hostbyte=DID_OK driverbyte=DRIVER_OK sd 0:0:0:0: [sda] tag#0 CDB: Write(10) 2a 00 00 02 16 46 00 00 f0 00 sd 0:0:0:0: [sda] tag#0 scsi host busy 1 failed 0 sd 0:0:0:0: Notifying upper driver of completion (result 0) sd 0:0:0:0: [sda] tag#0 Send: scmd 0xce13b800 sd 0:0:0:0: [sda] tag#0 CDB: Write(10) 2a 00 00 02 17 36 00 00 f0 00 sd 0:0:0:0: [sda] tag#0 Done: SUCCESS Result: hostbyte=DID_OK dr
