[PATCH] rpmsg_ns: Work around TI non-standard message
Texas Instruments ships a patch in their vendor kernels,
which adds a new NS message that includes a description field.
While TI is free to do whatever they want in their copy of the kernel,
it becomes a mess when people switch to a mainline kernel and want
to use their existing DSP programs with it.
To make it easier to migrate to a mainline kernel,
let's make the kernel aware of their non-standard extension but
briefly ignore the description field.
[0]
https://patchwork.kernel.org/project/linux-remoteproc/patch/20190815231448.10100-1-s-a...@ti.com/
[1]
https://stash.phytec.com/projects/PUB/repos/linux-phytec-ti/commits/aeded1f439effc84aa9f4e341a6e92ce1844ab98#drivers/rpmsg/virtio_rpmsg_bus.c
Cc: o...@wizery.com
Cc: s-a...@ti.com
Cc: t-kri...@ti.com
Signed-off-by: Richard Weinberger
---
FWIW, this is a forward port of a patch I'm using on v6.6.
Thanks,
//richard
---
drivers/rpmsg/rpmsg_ns.c | 30 ++
include/linux/rpmsg/ns.h | 8
2 files changed, 30 insertions(+), 8 deletions(-)
diff --git a/drivers/rpmsg/rpmsg_ns.c b/drivers/rpmsg/rpmsg_ns.c
index bde8c8d433e0a..2fb3721eb0141 100644
--- a/drivers/rpmsg/rpmsg_ns.c
+++ b/drivers/rpmsg/rpmsg_ns.c
@@ -31,10 +31,11 @@ EXPORT_SYMBOL(rpmsg_ns_register_device);
static int rpmsg_ns_cb(struct rpmsg_device *rpdev, void *data, int len,
void *priv, u32 src)
{
- struct rpmsg_ns_msg *msg = data;
struct rpmsg_device *newch;
struct rpmsg_channel_info chinfo;
struct device *dev = rpdev->dev.parent;
+ __rpmsg32 ns_addr, ns_flags;
+ char *ns_name;
int ret;
#if defined(CONFIG_DYNAMIC_DEBUG)
@@ -42,23 +43,36 @@ static int rpmsg_ns_cb(struct rpmsg_device *rpdev, void
*data, int len,
data, len, true);
#endif
- if (len != sizeof(*msg)) {
+ if (len == sizeof(struct rpmsg_ns_msg)) {
+ struct rpmsg_ns_msg *msg = data;
+
+ ns_addr = msg->addr;
+ ns_flags = msg->flags;
+ ns_name = msg->name;
+ } else if (len == sizeof(struct __rpmsg_ns_msg_ti)) {
+ struct __rpmsg_ns_msg_ti *msg = data;
+
+ ns_addr = msg->addr;
+ ns_flags = msg->flags;
+ ns_name = msg->name;
+ dev_warn(dev, "non-standard ns msg found\n");
+ } else {
dev_err(dev, "malformed ns msg (%d)\n", len);
return -EINVAL;
}
/* don't trust the remote processor for null terminating the name */
- msg->name[RPMSG_NAME_SIZE - 1] = '\0';
+ ns_name[RPMSG_NAME_SIZE - 1] = '\0';
- strscpy_pad(chinfo.name, msg->name, sizeof(chinfo.name));
+ strscpy_pad(chinfo.name, ns_name, sizeof(chinfo.name));
chinfo.src = RPMSG_ADDR_ANY;
- chinfo.dst = rpmsg32_to_cpu(rpdev, msg->addr);
+ chinfo.dst = rpmsg32_to_cpu(rpdev, ns_addr);
dev_info(dev, "%sing channel %s addr 0x%x\n",
-rpmsg32_to_cpu(rpdev, msg->flags) & RPMSG_NS_DESTROY ?
-"destroy" : "creat", msg->name, chinfo.dst);
+rpmsg32_to_cpu(rpdev, ns_flags) & RPMSG_NS_DESTROY ?
+"destroy" : "creat", ns_name, chinfo.dst);
- if (rpmsg32_to_cpu(rpdev, msg->flags) & RPMSG_NS_DESTROY) {
+ if (rpmsg32_to_cpu(rpdev, ns_flags) & RPMSG_NS_DESTROY) {
ret = rpmsg_release_channel(rpdev, &chinfo);
if (ret)
dev_err(dev, "rpmsg_destroy_channel failed: %d\n", ret);
diff --git a/include/linux/rpmsg/ns.h b/include/linux/rpmsg/ns.h
index a7804edd6d58f..60fca84ad4cea 100644
--- a/include/linux/rpmsg/ns.h
+++ b/include/linux/rpmsg/ns.h
@@ -26,6 +26,14 @@ struct rpmsg_ns_msg {
__rpmsg32 flags;
} __packed;
+/* Non-standard extended ns message by Texas Instruments */
+struct __rpmsg_ns_msg_ti {
+ char name[RPMSG_NAME_SIZE];
+ char desc[RPMSG_NAME_SIZE]; /* ignored */
+ u32 addr;
+ u32 flags;
+} __packed;
+
/**
* enum rpmsg_ns_flags - dynamic name service announcement flags
*
--
2.35.3
Re: [PATCH net-next v7] ptp: Add support for the AMZNC10C 'vmclock' device
On Sun, Oct 06, 2024 at 08:17:58AM +0100, David Woodhouse wrote: > From: David Woodhouse > > The vmclock device addresses the problem of live migration with > precision clocks. The tolerances of a hardware counter (e.g. TSC) are > typically around ±50PPM. A guest will use NTP/PTP/PPS to discipline that > counter against an external source of 'real' time, and track the precise > frequency of the counter as it changes with environmental conditions. > > When a guest is live migrated, anything it knows about the frequency of > the underlying counter becomes invalid. It may move from a host where > the counter running at -50PPM of its nominal frequency, to a host where > it runs at +50PPM. There will also be a step change in the value of the > counter, as the correctness of its absolute value at migration is > limited by the accuracy of the source and destination host's time > synchronization. > > In its simplest form, the device merely advertises a 'disruption_marker' > which indicates that the guest should throw away any NTP synchronization > it thinks it has, and start again. > > Because the shared memory region can be exposed all the way to userspace > through the /dev/vmclock0 node, applications can still use time from a > fast vDSO 'system call', and check the disruption marker to be sure that > their timestamp is indeed truthful. > > The structure also allows for the precise time, as known by the host, to > be exposed directly to guests so that they don't have to wait for NTP to > resync from scratch. The PTP driver consumes this information if present. > Like the KVM PTP clock, this PTP driver can convert TSC-based cross > timestamps into KVM clock values. Unlike the KVM PTP clock, it does so > only when such is actually helpful. > > The values and fields are based on the nascent virtio-rtc specification, > and the intent is that a version (hopefully precisely this version) of > this structure will be included as an optional part of that spec. In the > meantime, this driver supports the simple ACPI form of the device which > is being shipped in certain commercial hypervisors (and submitted for > inclusion in QEMU). > > Signed-off-by: David Woodhouse Acked-by: Richard Cochran
Re: [PATCH 0/4] remoteproc: k3-r5: Introduce suspend to ram support
Le 01/07/2024 à 11:59, Hari Nagalla a écrit : On 6/21/24 10:00, Richard Genoud wrote: Richard Genoud (4): remoteproc: k3-r5: Fix IPC-only mode detection remoteproc: k3-r5: Introduce PM suspend/resume handlers remoteproc: k3-r5: k3_r5_rproc_stop: code reorder remoteproc: k3-r5: support for graceful stop of remote cores IMO, the patches are better reordered as below (since there is a LPM rearch in progress) patch1 - Independent of other patches and is clearly a bug fix. patch3,4 - Support for graceful shutdown. A separate feature used by customers wanting to change FW at runtime and is independent of suspend/resume. patch 2 - suspend/resume handlers.. Indeed, patches 2, 3, 4 still need some internal discussions. But I think patch 1 could be taken as is, since it's a bug fix. Regards, Richard
Re: [PATCH 3/4] remoteproc: k3-r5: k3_r5_rproc_stop: code reorder
Le 01/07/2024 à 18:35, Mathieu Poirier a écrit :
On Mon, Jul 01, 2024 at 10:03:22AM +0200, Richard GENOUD wrote:
Le 28/06/2024 à 23:18, Mathieu Poirier a écrit :
On Fri, Jun 21, 2024 at 05:00:57PM +0200, Richard Genoud wrote:
In the next commit, a RP_MBOX_SHUTDOWN message will be sent in
k3_r5_rproc_stop() to the remote proc (in lockstep on not)
Thus, the sanity check "do not allow core 0 to stop before core 1"
should be moved at the beginning of the function so that the generic case
can be dealt with.
In order to have an easier patch to review, those actions are broke in
two patches:
- this patch: moving the sanity check at the beginning (No functional
change).
- next patch: doing the real job (sending shutdown messages to remote
procs before halting them).
Basically, we had:
- cluster_mode actions
- !cluster_mode sanity check
- !cluster_mode actions
And now:
- !cluster_mode sanity check
- cluster_mode actions
- !cluster_mode actions
Signed-off-by: Richard Genoud
---
drivers/remoteproc/ti_k3_r5_remoteproc.c | 24 ++--
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c
b/drivers/remoteproc/ti_k3_r5_remoteproc.c
index 1f18b08618c8..a2ead87952c7 100644
--- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
@@ -636,16 +636,8 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
struct k3_r5_core *core1, *core = kproc->core;
int ret;
- /* halt all applicable cores */
- if (cluster->mode == CLUSTER_MODE_LOCKSTEP) {
- list_for_each_entry(core, &cluster->cores, elem) {
- ret = k3_r5_core_halt(core);
- if (ret) {
- core = list_prev_entry(core, elem);
- goto unroll_core_halt;
- }
- }
- } else {
+
+ if (cluster->mode != CLUSTER_MODE_LOCKSTEP) {
/* do not allow core 0 to stop before core 1 */
core1 = list_last_entry(&cluster->cores, struct k3_r5_core,
elem);
@@ -656,6 +648,18 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
ret = -EPERM;
goto out;
}
+ }
+
+ /* halt all applicable cores */
+ if (cluster->mode == CLUSTER_MODE_LOCKSTEP) {
+ list_for_each_entry(core, &cluster->cores, elem) {
+ ret = k3_r5_core_halt(core);
+ if (ret) {
+ core = list_prev_entry(core, elem);
+ goto unroll_core_halt;
+ }
+ }
+ } else {
ret = k3_r5_core_halt(core);
if (ret)
With this patch, the "else" in this "if" condition is coupled with the "if" from
the lockstep mode, making the code extremaly hard to read. The original code
has a k3_r5_core_halt() in both "if" conditions, making the condition
independent from one another.
I'm not sure I understand what you mean.
With your patch applied I get the following: https://pastebin.com/yTZ0pKcS
Let's say the R5 is in split mode and k3_r5_rproc_stop() called on core1. The
if() that deal with that condition is on line 10, while the function that halts
the remote processor is online 34, part of the else clause that handles lockstep
mode. The two if() clauses are entangled and nothing good can come out of that.
Ok, I see your point now.
Thanks !
Anyway, I'm not happy with this diff, it doesn't reflect what was intended.
(which is moving the check "core 0 should not be stop before core 1" at the
beginning)
Tweaking around with the diff algorithms, I came with something way easier to
read I think:
diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c
b/drivers/remoteproc/ti_k3_r5_remoteproc.c
index 1f18b08618c8..a2ead87952c7 100644
--- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
@@ -636,6 +636,20 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
struct k3_r5_core *core1, *core = kproc->core;
int ret;
+
+ if (cluster->mode != CLUSTER_MODE_LOCKSTEP) {
+ /* do not allow core 0 to stop before core 1 */
+ core1 = list_last_entry(&cluster->cores, struct k3_r5_core,
+ elem);
+ if (core != core1 && core1->rproc->state != RPROC_OFFLINE &&
+ core1->rproc->state != RPROC_SUSPENDED) {
+ dev_err(dev, "%s: can not stop core 0 before core 1\n",
+ __func__);
+ ret = -EPERM;
+ goto out;
+ }
+
Re: [PATCH 4/4] remoteproc: k3-r5: support for graceful stop of remote cores
Le 29/06/2024 à 00:50, Andrew Davis a écrit :
On 6/21/24 10:00 AM, Richard Genoud wrote:
Introduce software IPC handshake between the K3-R5 remote proc driver
and the R5 MCU to gracefully stop/reset the remote core.
Upon a stop request, K3-R5 remote proc driver sends a RP_MBOX_SHUTDOWN
mailbox message to the remote R5 core.
The remote core is expected to:
- relinquish all the resources acquired through Device Manager (DM)
- disable its interrupts
- send back a mailbox acknowledgment RP_MBOX_SHUDOWN_ACK
- enter WFI state.
Meanwhile, the K3-R5 remote proc driver does:
- wait for the RP_MBOX_SHUTDOWN_ACK from the remote core
- wait for the remote proc to enter WFI state
- reset the remote core through device manager
Based on work from: Hari Nagalla
Signed-off-by: Richard Genoud
---
drivers/remoteproc/omap_remoteproc.h | 9 +-
drivers/remoteproc/ti_k3_r5_remoteproc.c | 40
2 files changed, 48 insertions(+), 1 deletion(-)
diff --git a/drivers/remoteproc/omap_remoteproc.h
b/drivers/remoteproc/omap_remoteproc.h
index 828e13256c02..c008f11fa2a4 100644
--- a/drivers/remoteproc/omap_remoteproc.h
+++ b/drivers/remoteproc/omap_remoteproc.h
@@ -42,6 +42,11 @@
* @RP_MBOX_SUSPEND_CANCEL: a cancel suspend response from a remote
processor
* on a suspend request
*
+ * @RP_MBOX_SHUTDOWN: shutdown request for the remote processor
+ *
+ * @RP_MBOX_SHUTDOWN_ACK: successful response from remote processor
for a
+ * shutdown request. The remote processor should be in WFI state
short after.
+ *
* Introduce new message definitions if any here.
*
* @RP_MBOX_END_MSG: Indicates end of known/defined messages from
remote core
@@ -59,7 +64,9 @@ enum omap_rp_mbox_messages {
RP_MBOX_SUSPEND_SYSTEM = 0xFF11,
RP_MBOX_SUSPEND_ACK = 0xFF12,
RP_MBOX_SUSPEND_CANCEL = 0xFF13,
- RP_MBOX_END_MSG = 0xFF14,
+ RP_MBOX_SHUTDOWN = 0xFF14,
+ RP_MBOX_SHUTDOWN_ACK = 0xFF15,
+ RP_MBOX_END_MSG = 0xFF16,
};
#endif /* _OMAP_RPMSG_H */
diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c
b/drivers/remoteproc/ti_k3_r5_remoteproc.c
index a2ead87952c7..918a15e1dd9a 100644
--- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
@@ -21,6 +21,7 @@
#include
#include
#include
+#include
#include
#include
@@ -172,8 +173,23 @@ struct k3_r5_rproc {
struct k3_r5_core *core;
struct k3_r5_mem *rmem;
int num_rmems;
+ struct completion shutdown_complete;
};
+/*
+ * This will return true if the remote core is in Wait For Interrupt
state.
+ */
+static bool k3_r5_is_core_in_wfi(struct k3_r5_core *core)
+{
+ int ret;
+ u64 boot_vec;
+ u32 cfg, ctrl, stat;
+
+ ret = ti_sci_proc_get_status(core->tsp, &boot_vec, &cfg, &ctrl,
&stat);
+
+ return !ret ? !!(stat & PROC_BOOT_STATUS_FLAG_R5_WFI) : false;
Too fancy for me :) Just return if (ret) right after get_status().
Ok, too much punctuation :)
Looks like this function is called in a polling loop, if
ti_sci_proc_get_status() fails once, it won't get better,
no need to keep checking, we should just error out of
the polling loop.
Ok
Thanks !
Andrew
+}
+
/**
* k3_r5_rproc_mbox_callback() - inbound mailbox message handler
* @client: mailbox client pointer used for requesting the mailbox
channel
@@ -209,6 +225,10 @@ static void k3_r5_rproc_mbox_callback(struct
mbox_client *client, void *data)
case RP_MBOX_ECHO_REPLY:
dev_info(dev, "received echo reply from %s\n", name);
break;
+ case RP_MBOX_SHUTDOWN_ACK:
+ dev_dbg(dev, "received shutdown_ack from %s\n", name);
+ complete(&kproc->shutdown_complete);
+ break;
default:
/* silently handle all other valid messages */
if (msg >= RP_MBOX_READY && msg < RP_MBOX_END_MSG)
@@ -634,6 +654,7 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
struct k3_r5_cluster *cluster = kproc->cluster;
struct device *dev = kproc->dev;
struct k3_r5_core *core1, *core = kproc->core;
+ bool wfi;
int ret;
@@ -650,6 +671,24 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
}
}
+ /* Send SHUTDOWN message to remote proc */
+ reinit_completion(&kproc->shutdown_complete);
+ ret = mbox_send_message(kproc->mbox, (void *)RP_MBOX_SHUTDOWN);
+ if (ret < 0) {
+ dev_err(dev, "Sending SHUTDOWN message failed: %d. Halting
core anyway.\n", ret);
+ } else {
+ ret = wait_for_completion_timeout(&kproc->shutdown_complete,
+ msecs_to_jiffies(1000));
+ if (ret == 0) {
+ dev_err(dev, "Timeout waiting SHUTDOWN_ACK message.
Halting core anyway.\n");
+ } else {
+ ret = readx_poll_timeout(k3_r5_is_core_in_wfi, core,
+
Re: [PATCH 4/4] remoteproc: k3-r5: support for graceful stop of remote cores
[adding Vibhore in Cc]
Le 28/06/2024 à 23:20, Mathieu Poirier a écrit :
On Fri, Jun 21, 2024 at 05:00:58PM +0200, Richard Genoud wrote:
Introduce software IPC handshake between the K3-R5 remote proc driver
and the R5 MCU to gracefully stop/reset the remote core.
Upon a stop request, K3-R5 remote proc driver sends a RP_MBOX_SHUTDOWN
mailbox message to the remote R5 core.
The remote core is expected to:
- relinquish all the resources acquired through Device Manager (DM)
- disable its interrupts
- send back a mailbox acknowledgment RP_MBOX_SHUDOWN_ACK
- enter WFI state.
Meanwhile, the K3-R5 remote proc driver does:
- wait for the RP_MBOX_SHUTDOWN_ACK from the remote core
- wait for the remote proc to enter WFI state
- reset the remote core through device manager
Based on work from: Hari Nagalla
Why is this needed now and what happens to system with a new kernel driver and
an older K3R5 firmware?
Without this patch, the IPC firwmares from recent TI PDK prevent the
suspend:
platform 5d0.r5f: ti-sci processor set_control failed: -19
remoteproc remoteproc1: can't stop rproc: -19
platform 5c0.r5f: Failed to shutdown rproc (-19)
platform 5c0.r5f: k3_r5_rproc_suspend failed (-19)
With a new kernel driver and an old firmware, this will add a timeout
before stopping it, and the message:
platform 5c0.r5f: Timeout waiting SHUTDOWN_ACK message. Halting core
anyway.
(tested on old FW 09.00.00.01 and new FW 09.02.01.18, on J7200)
Regards,
Richard
Thanks,
Mathieu
Signed-off-by: Richard Genoud
---
drivers/remoteproc/omap_remoteproc.h | 9 +-
drivers/remoteproc/ti_k3_r5_remoteproc.c | 40
2 files changed, 48 insertions(+), 1 deletion(-)
diff --git a/drivers/remoteproc/omap_remoteproc.h
b/drivers/remoteproc/omap_remoteproc.h
index 828e13256c02..c008f11fa2a4 100644
--- a/drivers/remoteproc/omap_remoteproc.h
+++ b/drivers/remoteproc/omap_remoteproc.h
@@ -42,6 +42,11 @@
* @RP_MBOX_SUSPEND_CANCEL: a cancel suspend response from a remote processor
* on a suspend request
*
+ * @RP_MBOX_SHUTDOWN: shutdown request for the remote processor
+ *
+ * @RP_MBOX_SHUTDOWN_ACK: successful response from remote processor for a
+ * shutdown request. The remote processor should be in WFI state short after.
+ *
* Introduce new message definitions if any here.
*
* @RP_MBOX_END_MSG: Indicates end of known/defined messages from remote core
@@ -59,7 +64,9 @@ enum omap_rp_mbox_messages {
RP_MBOX_SUSPEND_SYSTEM = 0xFF11,
RP_MBOX_SUSPEND_ACK = 0xFF12,
RP_MBOX_SUSPEND_CANCEL = 0xFF13,
- RP_MBOX_END_MSG = 0xFF14,
+ RP_MBOX_SHUTDOWN= 0xFF14,
+ RP_MBOX_SHUTDOWN_ACK= 0xFF15,
+ RP_MBOX_END_MSG = 0xFF16,
};
#endif /* _OMAP_RPMSG_H */
diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c
b/drivers/remoteproc/ti_k3_r5_remoteproc.c
index a2ead87952c7..918a15e1dd9a 100644
--- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
@@ -21,6 +21,7 @@
#include
#include
#include
+#include
#include
#include
@@ -172,8 +173,23 @@ struct k3_r5_rproc {
struct k3_r5_core *core;
struct k3_r5_mem *rmem;
int num_rmems;
+ struct completion shutdown_complete;
};
+/*
+ * This will return true if the remote core is in Wait For Interrupt state.
+ */
+static bool k3_r5_is_core_in_wfi(struct k3_r5_core *core)
+{
+ int ret;
+ u64 boot_vec;
+ u32 cfg, ctrl, stat;
+
+ ret = ti_sci_proc_get_status(core->tsp, &boot_vec, &cfg, &ctrl, &stat);
+
+ return !ret ? !!(stat & PROC_BOOT_STATUS_FLAG_R5_WFI) : false;
+}
+
/**
* k3_r5_rproc_mbox_callback() - inbound mailbox message handler
* @client: mailbox client pointer used for requesting the mailbox channel
@@ -209,6 +225,10 @@ static void k3_r5_rproc_mbox_callback(struct mbox_client
*client, void *data)
case RP_MBOX_ECHO_REPLY:
dev_info(dev, "received echo reply from %s\n", name);
break;
+ case RP_MBOX_SHUTDOWN_ACK:
+ dev_dbg(dev, "received shutdown_ack from %s\n", name);
+ complete(&kproc->shutdown_complete);
+ break;
default:
/* silently handle all other valid messages */
if (msg >= RP_MBOX_READY && msg < RP_MBOX_END_MSG)
@@ -634,6 +654,7 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
struct k3_r5_cluster *cluster = kproc->cluster;
struct device *dev = kproc->dev;
struct k3_r5_core *core1, *core = kproc->core;
+ bool wfi;
int ret;
@@ -650,6 +671,24 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
}
}
+ /* Send SHUTDOWN message to remote proc */
+ reinit_completion(&kproc->shutdown_complete);
+
Re: [PATCH 3/4] remoteproc: k3-r5: k3_r5_rproc_stop: code reorder
Le 28/06/2024 à 23:18, Mathieu Poirier a écrit :
On Fri, Jun 21, 2024 at 05:00:57PM +0200, Richard Genoud wrote:
In the next commit, a RP_MBOX_SHUTDOWN message will be sent in
k3_r5_rproc_stop() to the remote proc (in lockstep on not)
Thus, the sanity check "do not allow core 0 to stop before core 1"
should be moved at the beginning of the function so that the generic case
can be dealt with.
In order to have an easier patch to review, those actions are broke in
two patches:
- this patch: moving the sanity check at the beginning (No functional
change).
- next patch: doing the real job (sending shutdown messages to remote
procs before halting them).
Basically, we had:
- cluster_mode actions
- !cluster_mode sanity check
- !cluster_mode actions
And now:
- !cluster_mode sanity check
- cluster_mode actions
- !cluster_mode actions
Signed-off-by: Richard Genoud
---
drivers/remoteproc/ti_k3_r5_remoteproc.c | 24 ++--
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c
b/drivers/remoteproc/ti_k3_r5_remoteproc.c
index 1f18b08618c8..a2ead87952c7 100644
--- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
@@ -636,16 +636,8 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
struct k3_r5_core *core1, *core = kproc->core;
int ret;
- /* halt all applicable cores */
- if (cluster->mode == CLUSTER_MODE_LOCKSTEP) {
- list_for_each_entry(core, &cluster->cores, elem) {
- ret = k3_r5_core_halt(core);
- if (ret) {
- core = list_prev_entry(core, elem);
- goto unroll_core_halt;
- }
- }
- } else {
+
+ if (cluster->mode != CLUSTER_MODE_LOCKSTEP) {
/* do not allow core 0 to stop before core 1 */
core1 = list_last_entry(&cluster->cores, struct k3_r5_core,
elem);
@@ -656,6 +648,18 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
ret = -EPERM;
goto out;
}
+ }
+
+ /* halt all applicable cores */
+ if (cluster->mode == CLUSTER_MODE_LOCKSTEP) {
+ list_for_each_entry(core, &cluster->cores, elem) {
+ ret = k3_r5_core_halt(core);
+ if (ret) {
+ core = list_prev_entry(core, elem);
+ goto unroll_core_halt;
+ }
+ }
+ } else {
ret = k3_r5_core_halt(core);
if (ret)
With this patch, the "else" in this "if" condition is coupled with the "if" from
the lockstep mode, making the code extremaly hard to read. The original code
has a k3_r5_core_halt() in both "if" conditions, making the condition
independent from one another.
I'm not sure I understand what you mean.
Anyway, I'm not happy with this diff, it doesn't reflect what was intended.
(which is moving the check "core 0 should not be stop before core 1" at the
beginning)
Tweaking around with the diff algorithms, I came with something way easier to
read I think:
diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c
b/drivers/remoteproc/ti_k3_r5_remoteproc.c
index 1f18b08618c8..a2ead87952c7 100644
--- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
@@ -636,6 +636,20 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
struct k3_r5_core *core1, *core = kproc->core;
int ret;
+
+ if (cluster->mode != CLUSTER_MODE_LOCKSTEP) {
+ /* do not allow core 0 to stop before core 1 */
+ core1 = list_last_entry(&cluster->cores, struct k3_r5_core,
+ elem);
+ if (core != core1 && core1->rproc->state != RPROC_OFFLINE &&
+ core1->rproc->state != RPROC_SUSPENDED) {
+ dev_err(dev, "%s: can not stop core 0 before core 1\n",
+ __func__);
+ ret = -EPERM;
+ goto out;
+ }
+ }
+
/* halt all applicable cores */
if (cluster->mode == CLUSTER_MODE_LOCKSTEP) {
list_for_each_entry(core, &cluster->cores, elem) {
@@ -646,16 +660,6 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
}
}
} else {
- /* do not allow core 0 to stop before core 1 */
- core1 = list_last_entry(&cluster->cores, struct k3_r5_core,
- elem);
- if (core != core1 &&
Re: [PATCH 2/4] remoteproc: k3-r5: Introduce PM suspend/resume handlers
Le 28/06/2024 à 22:48, Mathieu Poirier a écrit :
On Fri, Jun 21, 2024 at 05:00:56PM +0200, Richard Genoud wrote:
This patch adds the support for system suspend/resume to the ti_k3_R5
remoteproc driver.
In order to save maximum power, the approach here is to shutdown
completely the cores that were started by the kernel (i.e. those in
RUNNING state).
Those which were started before the kernel (in attached mode) will be
detached.
The pm_notifier mechanism is used here because the remote procs firmwares
have to be reloaded at resume, and thus the driver must have access to
the file system were the firmware is stored.
On suspend, the running remote procs are stopped, the attached remote
procs are detached and processor control released.
On resume, the reverse operation is done.
Based on work from: Hari Nagalla
Signed-off-by: Richard Genoud
---
drivers/remoteproc/ti_k3_r5_remoteproc.c | 123 ++-
1 file changed, 121 insertions(+), 2 deletions(-)
diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c
b/drivers/remoteproc/ti_k3_r5_remoteproc.c
index 39a47540c590..1f18b08618c8 100644
--- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
#include
#include
@@ -112,6 +113,7 @@ struct k3_r5_cluster {
struct list_head cores;
wait_queue_head_t core_transition;
const struct k3_r5_soc_data *soc_data;
+ struct notifier_block pm_notifier;
};
/**
@@ -577,7 +579,8 @@ static int k3_r5_rproc_start(struct rproc *rproc)
/* do not allow core 1 to start before core 0 */
core0 = list_first_entry(&cluster->cores, struct k3_r5_core,
elem);
- if (core != core0 && core0->rproc->state == RPROC_OFFLINE) {
+ if (core != core0 && (core0->rproc->state == RPROC_OFFLINE ||
+ core0->rproc->state == RPROC_SUSPENDED)) {
If I understand correctly, this is to address a possible race condition between
user space wanting to start core1 via sysfs while the system is being suspended.
Is this correct? If so, please add a comment to explain what is going on.
Otherwise a comment is obviously needed.
Yes, you're right, I'll add a comment on the race condition at suspend.
dev_err(dev, "%s: can not start core 1 before core 0\n",
__func__);
ret = -EPERM;
@@ -646,7 +649,8 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
/* do not allow core 0 to stop before core 1 */
core1 = list_last_entry(&cluster->cores, struct k3_r5_core,
elem);
- if (core != core1 && core1->rproc->state != RPROC_OFFLINE) {
+ if (core != core1 && core1->rproc->state != RPROC_OFFLINE &&
+ core1->rproc->state != RPROC_SUSPENDED) {
dev_err(dev, "%s: can not stop core 0 before core 1\n",
__func__);
ret = -EPERM;
@@ -1238,6 +1242,117 @@ static int k3_r5_rproc_configure_mode(struct
k3_r5_rproc *kproc)
return ret;
}
+static int k3_r5_rproc_suspend(struct k3_r5_rproc *kproc)
+{
+ unsigned int rproc_state = kproc->rproc->state;
+ int ret;
+
+ if (rproc_state != RPROC_RUNNING && rproc_state != RPROC_ATTACHED)
+ return 0;
+
+ if (rproc_state == RPROC_RUNNING)
+ ret = rproc_shutdown(kproc->rproc);
+ else
+ ret = rproc_detach(kproc->rproc);
+
+ if (ret) {
+ dev_err(kproc->dev, "Failed to %s rproc (%d)\n",
+ (rproc_state == RPROC_RUNNING) ? "shutdown" : "detach",
+ ret);
+ return ret;
+ }
+
+ kproc->rproc->state = RPROC_SUSPENDED;
+
+ return ret;
+}
+
+static int k3_r5_rproc_resume(struct k3_r5_rproc *kproc)
+{
+ int ret;
+
+ if (kproc->rproc->state != RPROC_SUSPENDED)
+ return 0;
+
+ ret = k3_r5_rproc_configure_mode(kproc);
+ if (ret < 0)
+ return -EBUSY;
+
+ /*
+* ret > 0 for IPC-only mode
+* ret == 0 for remote proc mode
+*/
+ if (ret == 0) {
+ /*
+* remote proc looses its configuration when powered off.
+* So, we have to configure it again on resume.
+*/
+ ret = k3_r5_rproc_configure(kproc);
+ if (ret < 0) {
+ dev_err(kproc->dev, "k3_r5_rproc_configure failed
(%d)\n", ret);
+ return -EBUSY;
+
Re: [RFC PATCH v2] ptp: Add vDSO-style vmclock support
On Tue, Jun 25, 2024 at 11:34:24PM +0200, Thomas Gleixner wrote: > There is effort underway to expose PTP clocks to user space via > VDSO. That sounds interesting. Has anything been posted? Thanks, Richard
[PATCH 1/4] remoteproc: k3-r5: Fix IPC-only mode detection
ret variable was used to test reset status, get from
reset_control_status() call. But this variable was overwritten by
ti_sci_proc_get_status() a few lines bellow.
And as ti_sci_proc_get_status() returns 0 or a negative value (in this
latter case, followed by a return), the expression !ret was always true,
Clearly, this was not what was intended:
In the comment above it's said that "requires both local and module
resets to be deasserted"; if reset_control_status() returns 0 it means
that the reset line is deasserted.
So, it's pretty clear that the return value of reset_control_status()
was intended to be used instead of ti_sci_proc_get_status() return
value.
This could lead in an incorrect IPC-only mode detection if reset line is
asserted (so reset_control_status() return > 0) and c_state != 0 and
halted == 0.
In this case, the old code would have detected an IPC-only mode instead
of a mismatched mode.
Fixes: 1168af40b1ad ("remoteproc: k3-r5: Add support for IPC-only mode for all
R5Fs")
Signed-off-by: Richard Genoud
---
drivers/remoteproc/ti_k3_r5_remoteproc.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c
b/drivers/remoteproc/ti_k3_r5_remoteproc.c
index 50e486bcfa10..39a47540c590 100644
--- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
@@ -1144,6 +1144,7 @@ static int k3_r5_rproc_configure_mode(struct k3_r5_rproc
*kproc)
u32 atcm_enable, btcm_enable, loczrama;
struct k3_r5_core *core0;
enum cluster_mode mode = cluster->mode;
+ int reset_ctrl_status;
int ret;
core0 = list_first_entry(&cluster->cores, struct k3_r5_core, elem);
@@ -1160,11 +1161,11 @@ static int k3_r5_rproc_configure_mode(struct
k3_r5_rproc *kproc)
r_state, c_state);
}
- ret = reset_control_status(core->reset);
- if (ret < 0) {
+ reset_ctrl_status = reset_control_status(core->reset);
+ if (reset_ctrl_status < 0) {
dev_err(cdev, "failed to get initial local reset status, ret =
%d\n",
- ret);
- return ret;
+ reset_ctrl_status);
+ return reset_ctrl_status;
}
/*
@@ -1199,7 +1200,7 @@ static int k3_r5_rproc_configure_mode(struct k3_r5_rproc
*kproc)
* irrelevant if module reset is asserted (POR value has local reset
* deasserted), and is deemed as remoteproc mode
*/
- if (c_state && !ret && !halted) {
+ if (c_state && !reset_ctrl_status && !halted) {
dev_info(cdev, "configured R5F for IPC-only mode\n");
kproc->rproc->state = RPROC_DETACHED;
ret = 1;
@@ -1217,7 +1218,7 @@ static int k3_r5_rproc_configure_mode(struct k3_r5_rproc
*kproc)
ret = 0;
} else {
dev_err(cdev, "mismatched mode: local_reset = %s, module_reset
= %s, core_state = %s\n",
- !ret ? "deasserted" : "asserted",
+ !reset_ctrl_status ? "deasserted" : "asserted",
c_state ? "deasserted" : "asserted",
halted ? "halted" : "unhalted");
ret = -EINVAL;
[PATCH 4/4] remoteproc: k3-r5: support for graceful stop of remote cores
Introduce software IPC handshake between the K3-R5 remote proc driver
and the R5 MCU to gracefully stop/reset the remote core.
Upon a stop request, K3-R5 remote proc driver sends a RP_MBOX_SHUTDOWN
mailbox message to the remote R5 core.
The remote core is expected to:
- relinquish all the resources acquired through Device Manager (DM)
- disable its interrupts
- send back a mailbox acknowledgment RP_MBOX_SHUDOWN_ACK
- enter WFI state.
Meanwhile, the K3-R5 remote proc driver does:
- wait for the RP_MBOX_SHUTDOWN_ACK from the remote core
- wait for the remote proc to enter WFI state
- reset the remote core through device manager
Based on work from: Hari Nagalla
Signed-off-by: Richard Genoud
---
drivers/remoteproc/omap_remoteproc.h | 9 +-
drivers/remoteproc/ti_k3_r5_remoteproc.c | 40
2 files changed, 48 insertions(+), 1 deletion(-)
diff --git a/drivers/remoteproc/omap_remoteproc.h
b/drivers/remoteproc/omap_remoteproc.h
index 828e13256c02..c008f11fa2a4 100644
--- a/drivers/remoteproc/omap_remoteproc.h
+++ b/drivers/remoteproc/omap_remoteproc.h
@@ -42,6 +42,11 @@
* @RP_MBOX_SUSPEND_CANCEL: a cancel suspend response from a remote processor
* on a suspend request
*
+ * @RP_MBOX_SHUTDOWN: shutdown request for the remote processor
+ *
+ * @RP_MBOX_SHUTDOWN_ACK: successful response from remote processor for a
+ * shutdown request. The remote processor should be in WFI state short after.
+ *
* Introduce new message definitions if any here.
*
* @RP_MBOX_END_MSG: Indicates end of known/defined messages from remote core
@@ -59,7 +64,9 @@ enum omap_rp_mbox_messages {
RP_MBOX_SUSPEND_SYSTEM = 0xFF11,
RP_MBOX_SUSPEND_ACK = 0xFF12,
RP_MBOX_SUSPEND_CANCEL = 0xFF13,
- RP_MBOX_END_MSG = 0xFF14,
+ RP_MBOX_SHUTDOWN= 0xFF14,
+ RP_MBOX_SHUTDOWN_ACK= 0xFF15,
+ RP_MBOX_END_MSG = 0xFF16,
};
#endif /* _OMAP_RPMSG_H */
diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c
b/drivers/remoteproc/ti_k3_r5_remoteproc.c
index a2ead87952c7..918a15e1dd9a 100644
--- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
@@ -21,6 +21,7 @@
#include
#include
#include
+#include
#include
#include
@@ -172,8 +173,23 @@ struct k3_r5_rproc {
struct k3_r5_core *core;
struct k3_r5_mem *rmem;
int num_rmems;
+ struct completion shutdown_complete;
};
+/*
+ * This will return true if the remote core is in Wait For Interrupt state.
+ */
+static bool k3_r5_is_core_in_wfi(struct k3_r5_core *core)
+{
+ int ret;
+ u64 boot_vec;
+ u32 cfg, ctrl, stat;
+
+ ret = ti_sci_proc_get_status(core->tsp, &boot_vec, &cfg, &ctrl, &stat);
+
+ return !ret ? !!(stat & PROC_BOOT_STATUS_FLAG_R5_WFI) : false;
+}
+
/**
* k3_r5_rproc_mbox_callback() - inbound mailbox message handler
* @client: mailbox client pointer used for requesting the mailbox channel
@@ -209,6 +225,10 @@ static void k3_r5_rproc_mbox_callback(struct mbox_client
*client, void *data)
case RP_MBOX_ECHO_REPLY:
dev_info(dev, "received echo reply from %s\n", name);
break;
+ case RP_MBOX_SHUTDOWN_ACK:
+ dev_dbg(dev, "received shutdown_ack from %s\n", name);
+ complete(&kproc->shutdown_complete);
+ break;
default:
/* silently handle all other valid messages */
if (msg >= RP_MBOX_READY && msg < RP_MBOX_END_MSG)
@@ -634,6 +654,7 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
struct k3_r5_cluster *cluster = kproc->cluster;
struct device *dev = kproc->dev;
struct k3_r5_core *core1, *core = kproc->core;
+ bool wfi;
int ret;
@@ -650,6 +671,24 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
}
}
+ /* Send SHUTDOWN message to remote proc */
+ reinit_completion(&kproc->shutdown_complete);
+ ret = mbox_send_message(kproc->mbox, (void *)RP_MBOX_SHUTDOWN);
+ if (ret < 0) {
+ dev_err(dev, "Sending SHUTDOWN message failed: %d. Halting core
anyway.\n", ret);
+ } else {
+ ret = wait_for_completion_timeout(&kproc->shutdown_complete,
+ msecs_to_jiffies(1000));
+ if (ret == 0) {
+ dev_err(dev, "Timeout waiting SHUTDOWN_ACK message.
Halting core anyway.\n");
+ } else {
+ ret = readx_poll_timeout(k3_r5_is_core_in_wfi, core,
+wfi, wfi, 200, 2000);
+ if (ret)
+ dev_err(dev, "Timeout waiting for remote proc
to be in WFI state. Halting core anyway.\n"
[PATCH 3/4] remoteproc: k3-r5: k3_r5_rproc_stop: code reorder
In the next commit, a RP_MBOX_SHUTDOWN message will be sent in
k3_r5_rproc_stop() to the remote proc (in lockstep on not)
Thus, the sanity check "do not allow core 0 to stop before core 1"
should be moved at the beginning of the function so that the generic case
can be dealt with.
In order to have an easier patch to review, those actions are broke in
two patches:
- this patch: moving the sanity check at the beginning (No functional
change).
- next patch: doing the real job (sending shutdown messages to remote
procs before halting them).
Basically, we had:
- cluster_mode actions
- !cluster_mode sanity check
- !cluster_mode actions
And now:
- !cluster_mode sanity check
- cluster_mode actions
- !cluster_mode actions
Signed-off-by: Richard Genoud
---
drivers/remoteproc/ti_k3_r5_remoteproc.c | 24 ++--
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c
b/drivers/remoteproc/ti_k3_r5_remoteproc.c
index 1f18b08618c8..a2ead87952c7 100644
--- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
@@ -636,16 +636,8 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
struct k3_r5_core *core1, *core = kproc->core;
int ret;
- /* halt all applicable cores */
- if (cluster->mode == CLUSTER_MODE_LOCKSTEP) {
- list_for_each_entry(core, &cluster->cores, elem) {
- ret = k3_r5_core_halt(core);
- if (ret) {
- core = list_prev_entry(core, elem);
- goto unroll_core_halt;
- }
- }
- } else {
+
+ if (cluster->mode != CLUSTER_MODE_LOCKSTEP) {
/* do not allow core 0 to stop before core 1 */
core1 = list_last_entry(&cluster->cores, struct k3_r5_core,
elem);
@@ -656,6 +648,18 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
ret = -EPERM;
goto out;
}
+ }
+
+ /* halt all applicable cores */
+ if (cluster->mode == CLUSTER_MODE_LOCKSTEP) {
+ list_for_each_entry(core, &cluster->cores, elem) {
+ ret = k3_r5_core_halt(core);
+ if (ret) {
+ core = list_prev_entry(core, elem);
+ goto unroll_core_halt;
+ }
+ }
+ } else {
ret = k3_r5_core_halt(core);
if (ret)
[PATCH 2/4] remoteproc: k3-r5: Introduce PM suspend/resume handlers
This patch adds the support for system suspend/resume to the ti_k3_R5
remoteproc driver.
In order to save maximum power, the approach here is to shutdown
completely the cores that were started by the kernel (i.e. those in
RUNNING state).
Those which were started before the kernel (in attached mode) will be
detached.
The pm_notifier mechanism is used here because the remote procs firmwares
have to be reloaded at resume, and thus the driver must have access to
the file system were the firmware is stored.
On suspend, the running remote procs are stopped, the attached remote
procs are detached and processor control released.
On resume, the reverse operation is done.
Based on work from: Hari Nagalla
Signed-off-by: Richard Genoud
---
drivers/remoteproc/ti_k3_r5_remoteproc.c | 123 ++-
1 file changed, 121 insertions(+), 2 deletions(-)
diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c
b/drivers/remoteproc/ti_k3_r5_remoteproc.c
index 39a47540c590..1f18b08618c8 100644
--- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
#include
#include
@@ -112,6 +113,7 @@ struct k3_r5_cluster {
struct list_head cores;
wait_queue_head_t core_transition;
const struct k3_r5_soc_data *soc_data;
+ struct notifier_block pm_notifier;
};
/**
@@ -577,7 +579,8 @@ static int k3_r5_rproc_start(struct rproc *rproc)
/* do not allow core 1 to start before core 0 */
core0 = list_first_entry(&cluster->cores, struct k3_r5_core,
elem);
- if (core != core0 && core0->rproc->state == RPROC_OFFLINE) {
+ if (core != core0 && (core0->rproc->state == RPROC_OFFLINE ||
+ core0->rproc->state == RPROC_SUSPENDED)) {
dev_err(dev, "%s: can not start core 1 before core 0\n",
__func__);
ret = -EPERM;
@@ -646,7 +649,8 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
/* do not allow core 0 to stop before core 1 */
core1 = list_last_entry(&cluster->cores, struct k3_r5_core,
elem);
- if (core != core1 && core1->rproc->state != RPROC_OFFLINE) {
+ if (core != core1 && core1->rproc->state != RPROC_OFFLINE &&
+ core1->rproc->state != RPROC_SUSPENDED) {
dev_err(dev, "%s: can not stop core 0 before core 1\n",
__func__);
ret = -EPERM;
@@ -1238,6 +1242,117 @@ static int k3_r5_rproc_configure_mode(struct
k3_r5_rproc *kproc)
return ret;
}
+static int k3_r5_rproc_suspend(struct k3_r5_rproc *kproc)
+{
+ unsigned int rproc_state = kproc->rproc->state;
+ int ret;
+
+ if (rproc_state != RPROC_RUNNING && rproc_state != RPROC_ATTACHED)
+ return 0;
+
+ if (rproc_state == RPROC_RUNNING)
+ ret = rproc_shutdown(kproc->rproc);
+ else
+ ret = rproc_detach(kproc->rproc);
+
+ if (ret) {
+ dev_err(kproc->dev, "Failed to %s rproc (%d)\n",
+ (rproc_state == RPROC_RUNNING) ? "shutdown" : "detach",
+ ret);
+ return ret;
+ }
+
+ kproc->rproc->state = RPROC_SUSPENDED;
+
+ return ret;
+}
+
+static int k3_r5_rproc_resume(struct k3_r5_rproc *kproc)
+{
+ int ret;
+
+ if (kproc->rproc->state != RPROC_SUSPENDED)
+ return 0;
+
+ ret = k3_r5_rproc_configure_mode(kproc);
+ if (ret < 0)
+ return -EBUSY;
+
+ /*
+* ret > 0 for IPC-only mode
+* ret == 0 for remote proc mode
+*/
+ if (ret == 0) {
+ /*
+* remote proc looses its configuration when powered off.
+* So, we have to configure it again on resume.
+*/
+ ret = k3_r5_rproc_configure(kproc);
+ if (ret < 0) {
+ dev_err(kproc->dev, "k3_r5_rproc_configure failed
(%d)\n", ret);
+ return -EBUSY;
+ }
+ }
+
+ return rproc_boot(kproc->rproc);
+}
+
+static int k3_r5_cluster_pm_notifier_call(struct notifier_block *bl,
+ unsigned long state, void *unused)
+{
+ struct k3_r5_cluster *cluster = container_of(bl, struct k3_r5_cluster,
+pm_notifier);
+ struct k3_r5_core *core;
+ int ret;
+
+ switch (state) {
+ case PM_HIBERNATION_PREPARE
[PATCH 0/4] remoteproc: k3-r5: Introduce suspend to ram support
This series enables the suspend to ram with R5F remote processors on TI K3 platform. The 1st patch is actually a fix, independent from the others The 2nd patch introduces the suspend/resume handlers. On suspend, the running rprocs will be stopped (or detached) and then re-loaded in resume. The logic behind this is: - shutdown the cores that Linux started to save power in suspend. - detach the cores that were started before Linux. Then, the 3rd and 4th patches introduce the graceful shutdown of remote procs. This will give them a chance to release resources and shut down in a civilized manner. Without this series, the suspend fails with: omap-mailbox 31f81000.mailbox: fifo 1 has unexpected unread messages omap-mailbox 31f81000.mailbox: PM: dpm_run_callback(): platform_pm_suspend returns -16 omap-mailbox 31f81000.mailbox: PM: platform_pm_suspend returned -16 after 16328 usecs omap-mailbox 31f81000.mailbox: PM: failed to suspend: error -16 Patches 2 and 4 are based on work from Hari Nagalla . @Hari, please feel free to add your Co-developed-by:/Signed-off-by: Tested on J7200X SoM Series is based on v6.10-rc4 Richard Genoud (4): remoteproc: k3-r5: Fix IPC-only mode detection remoteproc: k3-r5: Introduce PM suspend/resume handlers remoteproc: k3-r5: k3_r5_rproc_stop: code reorder remoteproc: k3-r5: support for graceful stop of remote cores drivers/remoteproc/omap_remoteproc.h | 9 +- drivers/remoteproc/ti_k3_r5_remoteproc.c | 196 +-- 2 files changed, 188 insertions(+), 17 deletions(-)
[PATCH v2] mm: add alloc_contig_migrate_range allocation statistics
alloc_contig_migrate_range has every information to be able to
understand big contiguous allocation latency. For example, how many
pages are migrated, how many times they were needed to unmap from
page tables.
This patch adds the trace event to collect the allocation statistics.
In the field, it was quite useful to understand CMA allocation
latency.
Signed-off-by: Richard Chang
---
* from v1 -
https://lore.kernel.org/linux-mm/20240226100045.2083962-1-richard...@google.com/
* Move the trace event int field to the end of the longs - rostedt
* Do the calculation only when tracing is enabled - rostedt
include/trace/events/kmem.h | 38 +
mm/internal.h | 3 ++-
mm/page_alloc.c | 32 ++-
mm/page_isolation.c | 2 +-
4 files changed, 68 insertions(+), 7 deletions(-)
diff --git a/include/trace/events/kmem.h b/include/trace/events/kmem.h
index 58688768ef0f..6e62cc64cd92 100644
--- a/include/trace/events/kmem.h
+++ b/include/trace/events/kmem.h
@@ -304,6 +304,44 @@ TRACE_EVENT(mm_page_alloc_extfrag,
__entry->change_ownership)
);
+TRACE_EVENT(mm_alloc_contig_migrate_range_info,
+
+ TP_PROTO(unsigned long start,
+unsigned long end,
+unsigned long nr_migrated,
+unsigned long nr_reclaimed,
+unsigned long nr_mapped,
+int migratetype),
+
+ TP_ARGS(start, end, nr_migrated, nr_reclaimed, nr_mapped, migratetype),
+
+ TP_STRUCT__entry(
+ __field(unsigned long, start)
+ __field(unsigned long, end)
+ __field(unsigned long, nr_migrated)
+ __field(unsigned long, nr_reclaimed)
+ __field(unsigned long, nr_mapped)
+ __field(int, migratetype)
+ ),
+
+ TP_fast_assign(
+ __entry->start = start;
+ __entry->end = end;
+ __entry->nr_migrated = nr_migrated;
+ __entry->nr_reclaimed = nr_reclaimed;
+ __entry->nr_mapped = nr_mapped;
+ __entry->migratetype = migratetype;
+ ),
+
+ TP_printk("start=0x%lx end=0x%lx migratetype=%d nr_migrated=%lu
nr_reclaimed=%lu nr_mapped=%lu",
+ __entry->start,
+ __entry->end,
+ __entry->migratetype,
+ __entry->nr_migrated,
+ __entry->nr_reclaimed,
+ __entry->nr_mapped)
+);
+
/*
* Required for uniquely and securely identifying mm in rss_stat tracepoint.
*/
diff --git a/mm/internal.h b/mm/internal.h
index f309a010d50f..e114c647e278 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -537,7 +537,8 @@ isolate_migratepages_range(struct compact_control *cc,
unsigned long low_pfn, unsigned long end_pfn);
int __alloc_contig_migrate_range(struct compact_control *cc,
- unsigned long start, unsigned long end);
+ unsigned long start, unsigned long end,
+ int migratetype);
/* Free whole pageblock and set its migration type to MIGRATE_CMA. */
void init_cma_reserved_pageblock(struct page *page);
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 150d4f23b010..da169fa20d8e 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6219,9 +6219,14 @@ static void alloc_contig_dump_pages(struct list_head
*page_list)
}
}
-/* [start, end) must belong to a single zone. */
+/*
+ * [start, end) must belong to a single zone.
+ * @migratetype: using migratetype to filter the type of migration in
+ * trace_mm_alloc_contig_migrate_range_info.
+ */
int __alloc_contig_migrate_range(struct compact_control *cc,
- unsigned long start, unsigned long end)
+ unsigned long start, unsigned long end,
+ int migratetype)
{
/* This function is based on compact_zone() from compaction.c. */
unsigned int nr_reclaimed;
@@ -6232,6 +6237,10 @@ int __alloc_contig_migrate_range(struct compact_control
*cc,
.nid = zone_to_nid(cc->zone),
.gfp_mask = GFP_USER | __GFP_MOVABLE | __GFP_RETRY_MAYFAIL,
};
+ struct page *page;
+ unsigned long total_mapped = 0;
+ unsigned long total_migrated = 0;
+ unsigned long total_reclaimed = 0;
lru_cache_disable();
@@ -6257,9 +6266,18 @@ int __alloc_contig_migrate_range(struct compact_control
*cc,
&cc->migratepages);
cc->nr_migratepages -= nr_reclaimed;
+ if (trace_mm_alloc_contig_migrate_range_info_enabled()) {
+ total_reclaimed += nr_reclaimed;
+
[PATCH] mm: add alloc_contig_migrate_range allocation statistics
alloc_contig_migrate_range has every information to be able to
understand big contiguous allocation latency. For example, how many
pages are migrated, how many times they were needed to unmap from
page tables.
This patch adds the trace event to collect the allocation statistics.
In the field, it was quite useful to understand CMA allocation
latency.
Signed-off-by: Richard Chang
---
include/trace/events/kmem.h | 39 +
mm/internal.h | 3 ++-
mm/page_alloc.c | 30 +++-
mm/page_isolation.c | 2 +-
4 files changed, 67 insertions(+), 7 deletions(-)
diff --git a/include/trace/events/kmem.h b/include/trace/events/kmem.h
index 58688768ef0f..964704d76f9f 100644
--- a/include/trace/events/kmem.h
+++ b/include/trace/events/kmem.h
@@ -304,6 +304,45 @@ TRACE_EVENT(mm_page_alloc_extfrag,
__entry->change_ownership)
);
+TRACE_EVENT(mm_alloc_contig_migrate_range_info,
+
+ TP_PROTO(unsigned long start,
+unsigned long end,
+int migratetype,
+unsigned long nr_migrated,
+unsigned long nr_reclaimed,
+unsigned long nr_mapped),
+
+ TP_ARGS(start, end, migratetype,
+ nr_migrated, nr_reclaimed, nr_mapped),
+
+ TP_STRUCT__entry(
+ __field(unsigned long, start)
+ __field(unsigned long, end)
+ __field(int, migratetype)
+ __field(unsigned long, nr_migrated)
+ __field(unsigned long, nr_reclaimed)
+ __field(unsigned long, nr_mapped)
+ ),
+
+ TP_fast_assign(
+ __entry->start = start;
+ __entry->end = end;
+ __entry->migratetype = migratetype;
+ __entry->nr_migrated = nr_migrated;
+ __entry->nr_reclaimed = nr_reclaimed;
+ __entry->nr_mapped = nr_mapped;
+ ),
+
+ TP_printk("start=0x%lx end=0x%lx migratetype=%d nr_migrated=%lu
nr_reclaimed=%lu nr_mapped=%lu",
+ __entry->start,
+ __entry->end,
+ __entry->migratetype,
+ __entry->nr_migrated,
+ __entry->nr_reclaimed,
+ __entry->nr_mapped)
+);
+
/*
* Required for uniquely and securely identifying mm in rss_stat tracepoint.
*/
diff --git a/mm/internal.h b/mm/internal.h
index f309a010d50f..e114c647e278 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -537,7 +537,8 @@ isolate_migratepages_range(struct compact_control *cc,
unsigned long low_pfn, unsigned long end_pfn);
int __alloc_contig_migrate_range(struct compact_control *cc,
- unsigned long start, unsigned long end);
+ unsigned long start, unsigned long end,
+ int migratetype);
/* Free whole pageblock and set its migration type to MIGRATE_CMA. */
void init_cma_reserved_pageblock(struct page *page);
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 150d4f23b010..f840bc785afa 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6219,9 +6219,14 @@ static void alloc_contig_dump_pages(struct list_head
*page_list)
}
}
-/* [start, end) must belong to a single zone. */
+/*
+ * [start, end) must belong to a single zone.
+ * @migratetype: using migratetype to filter the type of migration in
+ * trace_mm_alloc_contig_migrate_range_info.
+ */
int __alloc_contig_migrate_range(struct compact_control *cc,
- unsigned long start, unsigned long end)
+ unsigned long start, unsigned long end,
+ int migratetype)
{
/* This function is based on compact_zone() from compaction.c. */
unsigned int nr_reclaimed;
@@ -6232,6 +6237,10 @@ int __alloc_contig_migrate_range(struct compact_control
*cc,
.nid = zone_to_nid(cc->zone),
.gfp_mask = GFP_USER | __GFP_MOVABLE | __GFP_RETRY_MAYFAIL,
};
+ struct page *page;
+ unsigned long total_mapped = 0;
+ unsigned long total_migrated = 0;
+ unsigned long total_reclaimed = 0;
lru_cache_disable();
@@ -6257,9 +6266,16 @@ int __alloc_contig_migrate_range(struct compact_control
*cc,
&cc->migratepages);
cc->nr_migratepages -= nr_reclaimed;
+ total_reclaimed += nr_reclaimed;
+ list_for_each_entry(page, &cc->migratepages, lru)
+ total_mapped += page_mapcount(page);
+
ret = migrate_pages(&cc->migratepages, alloc_migration_target,
NULL, (unsigned long)&mtc, cc->mode, MR_CONTIG_RANGE,
NULL);
+
Re: Question about the ipi_raise filter usage and output
On Wed, Feb 7, 2024 at 10:28 AM richard clark
wrote:
>
> On Tue, Feb 6, 2024 at 5:39 PM Valentin Schneider wrote:
> >
> > You should have access to the generic fields which include the CPU from
> > which the event happens. Any of "CPU", "cpu" or "common_cpu" would match
> > this.
> >
> > So if you're on a recent enough kernel (v6.6 or above AFAICT), you should
> > be able to do something like so:
> >
> > trace-cmd record -e 'ipi_raise' -f 'CPU & CPUS{7-42}' ./foo.sh
> >
> > If you just want to match a single CPU, or are on an older kernel, this
> > should work as well:
> >
> > trace-cmd record -e 'ipi_raise' -f 'CPU == 42' ./foo.sh
> >
> > For example on a QEMU x86 environment:
> >
> > # trace-cmd record -e 'call_function*' -f 'CPU & CPUS{3}' hackbench
> > Running in process mode with 10 groups using 40 file descriptors each (==
> > 400 tasks)
> > Each sender will pass 100 messages of 100 bytes
> > Time: 0.396
> > CPU0 data recorded at offset=0x738000
> > 0 bytes in size
> > CPU1 data recorded at offset=0x738000
> > 0 bytes in size
> > CPU2 data recorded at offset=0x738000
> > 0 bytes in size
> > CPU3 data recorded at offset=0x738000
> > 4096 bytes in size
> >
> > # trace-cmd report
> > CPU 0 is empty
> > CPU 1 is empty
> > CPU 2 is empty
> > cpus=4
> > -0 [003]29.704387: call_function_single_entry:
> > vector=251
> > -0 [003]29.704388: call_function_single_exit:
> > vector=251
> > -0 [003]29.705950: call_function_single_entry:
> > vector=251
> > -0 [003]29.705951: call_function_single_exit:
> > vector=251
> > -0 [003]29.706462: call_function_single_entry:
> > vector=251
> > -0 [003]29.706463: call_function_single_exit:
> > vector=251
> > hackbench-962 [003]29.706501: call_function_single_entry:
> > vector=251
> > hackbench-962 [003]29.706502: call_function_single_exit:
> > vector=251
> > hackbench-955 [003]29.706521: call_function_single_entry:
> > vector=251
> > hackbench-955 [003]29.706522: call_function_single_exit:
> > vector=251
> > -0 [003]30.101812: call_function_single_entry:
> > vector=251
> > -0 [003]30.101814: call_function_single_exit:
> > vector=251
> > -0 [003]30.101897: call_function_single_entry:
> > vector=251
> > -0 [003]30.101898: call_function_single_exit:
> > vector=251
> > -0 [003]30.101985: call_function_single_entry:
> > vector=251
> > -0 [003]30.101986: call_function_single_exit:
> > vector=251
> > -0 [003]30.102072: call_function_single_entry:
> > vector=251
> > -0 [003]30.102072: call_function_single_exit:
> > vector=251
> > -0 [003]30.102161: call_function_single_entry:
> > vector=251
> > -0 [003]30.102161: call_function_single_exit:
> > vector=251
> > -0 [003]30.102250: call_function_single_entry:
> > vector=251
> > -0 [003]30.102251: call_function_single_exit:
> > vector=251
> > -0 [003]30.102372: call_function_single_entry:
> > vector=251
> > -0 [003]30.102372: call_function_single_exit:
> > vector=251
> >
> >
> > CPU 0 is empty
> > CPU 1 is empty
> > CPU 2 is empty
> > cpus=4
> > -0 [003] 1067.718304: call_function_single_entry:
> > vector=251
> > -0 [003] 1067.718309: call_function_single_exit:
> > vector=251
> >
> > and that behaves the same as
> >
> > trace-cmd record -e 'call_function*' -f 'CPU == 3' hackbench
> >
> Thanks, # trace-cmd record -e 'ipi' -f 'CPU==10 || CPU==11' -f
> 'reason=="Function call interrupts"' works:
> CPU0 data recorded at offset=0x336000
> 0 bytes in size
> CPU1 data recorded at offset=0x336000
> 0 bytes in size
> CPU2 data recorded at offset=0x336000
> 0 bytes in size
> CPU3 data recorded at offset=0x336000
> 0 bytes in size
> CPU4 data recorded at offset=0x336000
> 0
Re: Question about the ipi_raise filter usage and output
On Tue, Feb 6, 2024 at 5:39 PM Valentin Schneider wrote:
>
> You should have access to the generic fields which include the CPU from
> which the event happens. Any of "CPU", "cpu" or "common_cpu" would match
> this.
>
> So if you're on a recent enough kernel (v6.6 or above AFAICT), you should
> be able to do something like so:
>
> trace-cmd record -e 'ipi_raise' -f 'CPU & CPUS{7-42}' ./foo.sh
>
> If you just want to match a single CPU, or are on an older kernel, this
> should work as well:
>
> trace-cmd record -e 'ipi_raise' -f 'CPU == 42' ./foo.sh
>
> For example on a QEMU x86 environment:
>
> # trace-cmd record -e 'call_function*' -f 'CPU & CPUS{3}' hackbench
> Running in process mode with 10 groups using 40 file descriptors each (==
> 400 tasks)
> Each sender will pass 100 messages of 100 bytes
> Time: 0.396
> CPU0 data recorded at offset=0x738000
> 0 bytes in size
> CPU1 data recorded at offset=0x738000
> 0 bytes in size
> CPU2 data recorded at offset=0x738000
> 0 bytes in size
> CPU3 data recorded at offset=0x738000
> 4096 bytes in size
>
> # trace-cmd report
> CPU 0 is empty
> CPU 1 is empty
> CPU 2 is empty
> cpus=4
> -0 [003]29.704387: call_function_single_entry:
> vector=251
> -0 [003]29.704388: call_function_single_exit:
> vector=251
> -0 [003]29.705950: call_function_single_entry:
> vector=251
> -0 [003]29.705951: call_function_single_exit:
> vector=251
> -0 [003]29.706462: call_function_single_entry:
> vector=251
> -0 [003]29.706463: call_function_single_exit:
> vector=251
> hackbench-962 [003]29.706501: call_function_single_entry:
> vector=251
> hackbench-962 [003]29.706502: call_function_single_exit:
> vector=251
> hackbench-955 [003]29.706521: call_function_single_entry:
> vector=251
> hackbench-955 [003]29.706522: call_function_single_exit:
> vector=251
> -0 [003]30.101812: call_function_single_entry:
> vector=251
> -0 [003]30.101814: call_function_single_exit:
> vector=251
> -0 [003]30.101897: call_function_single_entry:
> vector=251
> -0 [003]30.101898: call_function_single_exit:
> vector=251
> -0 [003]30.101985: call_function_single_entry:
> vector=251
> -0 [003]30.101986: call_function_single_exit:
> vector=251
> -0 [003]30.102072: call_function_single_entry:
> vector=251
> -0 [003]30.102072: call_function_single_exit:
> vector=251
> -0 [003]30.102161: call_function_single_entry:
> vector=251
> -0 [003]30.102161: call_function_single_exit:
> vector=251
> -0 [003]30.102250: call_function_single_entry:
> vector=251
> -0 [003]30.102251: call_function_single_exit:
> vector=251
> -0 [003]30.102372: call_function_single_entry:
> vector=251
> -0 [003]30.102372: call_function_single_exit:
> vector=251
>
>
> CPU 0 is empty
> CPU 1 is empty
> CPU 2 is empty
> cpus=4
> -0 [003] 1067.718304: call_function_single_entry:
> vector=251
> -0 [003] 1067.718309: call_function_single_exit:
> vector=251
>
> and that behaves the same as
>
> trace-cmd record -e 'call_function*' -f 'CPU == 3' hackbench
>
Thanks, # trace-cmd record -e 'ipi' -f 'CPU==10 || CPU==11' -f
'reason=="Function call interrupts"' works:
CPU0 data recorded at offset=0x336000
0 bytes in size
CPU1 data recorded at offset=0x336000
0 bytes in size
CPU2 data recorded at offset=0x336000
0 bytes in size
CPU3 data recorded at offset=0x336000
0 bytes in size
CPU4 data recorded at offset=0x336000
0 bytes in size
CPU5 data recorded at offset=0x336000
0 bytes in size
CPU6 data recorded at offset=0x336000
0 bytes in size
CPU7 data recorded at offset=0x336000
0 bytes in size
CPU8 data recorded at offset=0x336000
0 bytes in size
CPU9 data recorded at offset=0x336000
0 bytes in size
CPU10 data recorded at offset=0x336000
4096 bytes in size
CPU11 data recorded at offset=0x337000
4096 bytes in size
# trace-cmd report
CPU 0 is empty
CPU 1 is empty
CPU 2 is empty
CPU 3 is empty
CPU 4 is empty
CPU 5 is empty
CPU 6 is empty
CPU 7 is empty
CPU 8 is empty
CPU 9 is empty
cpus=12
insmod-8519 [010] 170847.580062: ipi_raise:
target_mask=,0bff (Function call interrupts)
-0 [011] 170847.580070: ipi_entry:
(Function call interrupts)
-0 [011] 170847.580071: ipi_exit:
(Function call interrupts)
insmod-8519 [010] 170847.580078: ipi_raise:
target_mask=,0bff (Function call interrupts)
-0 [011] 170847.580080: ipi_entry:
(Function call interrupts)
-0 [011]
Re: Question about the ipi_raise filter usage and output
On Tue, Feb 6, 2024 at 12:05 AM Valentin Schneider wrote:
>
> On 05/02/24 14:39, Mark Rutland wrote:
> > [adding Valentin]
> >
>
> Thanks!
>
> > On Mon, Feb 05, 2024 at 08:06:09AM -0500, Steven Rostedt wrote:
> >> On Mon, 5 Feb 2024 10:28:57 +
> >> Mark Rutland wrote:
> >>
> >> > > I try to write below:
> >> > > echo 'target_cpus == 11 && reason == "Function call interrupts"' >
> >> > > events/ipi/ipi_raise/filter
> >> >
> >> > The '=' checks if the target_cpus bitmap *only* contains CPU 11. If the
> >> > cpumask
> >> > contains other CPUs, the filter will skip the call.
> >> >
> >> > I believe you can use '&' to check whether a cpumask contains a CPU, e.g.
> >> >
> >> >'target_cpus & 11'
> >>
> >> 11 == 0xb = b1011
> >>
> >> So the above would only be true for CPUs 0,1 and 3 ;-)
> >
> > Sorry, I misunderstood the scalar logic and thought that we treated:
> >
> > '$mask $OP $scalar', e.g. 'target_cpus & 11'
> >
> > .. as a special case meaning a cpumask with that scalar bit set, i.e.
> >
> > '$mask $OP CPUS{$scalar}', e.g. 'target_cpus & CPUS{11}'
> >
> > .. but evidently I was wrong.
> >
> >> I think you meant: 'target_cpus & 0x800'
> >>
> >> I tried "1 << 11' but it appears to not allow shifts. I wonder if we
> >> should add that?
> >
> > Hmm... shouldn't we make 'CPUS{11}' work for that?
> >
>
> It /should/ already be the case, the user input with the curly braces is
> parsed as a cpulist. So CPUS{11} really does mean CPU11, not a hex value to
> be interpreted as a cpumask.
>
> However...
>
> > From a quick test (below), that doesn't seem to work, though I think it
> > probably should?
> > Have I completely misunderstood how this is supposed to work, or is that a
> > bug?
> >
>
> The CPUS{} thingie only works with an event field that is either declared as a
> cpumask (__cpumask) or a scalar. That's not the case for ipi_raise, the
> target_cpus event field is saved as a "raw" bitmask.
>
> There /should/ have been a warning about the event filter though, but I
> think it's not happening because I'm allowing more than just FILTER_CPUMASK
> in parse_pred() to make it work for scalars. I'll go poke around some more.
>
> Generally for this sort of IPI investigation I'd recommend using the newer
> trace_ipi_send_cpu() and trace_ipi_send_cpumask() (for which CPUS{}
> filtering works).
> If it's only the function call interrupts you're interesting in, have a
> look at trace_csd_queue_cpu().
This should be supported by newer version kernels like v6.5, but I am
using v6.1 and this trace event has not been supported yet... so ipi
is more suitable for me. ipi_entry and ipi_exit is ok, but seems the
filter doesn't support a specific cpu, maybe we need to add this?
>
> > Mark.
>
Re: Question about the ipi_raise filter usage and output
Hi Steve,
On Mon, Feb 5, 2024 at 6:38 PM Steven Rostedt wrote:
>
> On Mon, 5 Feb 2024 17:57:29 +0800
> richard clark wrote:
>
> > I try to write below:
> > echo 'target_cpus == 11 && reason == "Function call interrupts"' >
> > events/ipi/ipi_raise/filter
>
> You mean when it is sent only to CPU 11? Not when the event is
> happening on CPU 11. Like the above example, the event was triggered on
> CPU 10, but the mask was for all the other CPUs.
>
> If you are looking for just CPU 11, you can do:
>
> echo 'target_cpus == 0x800 && reason == "Function call interrupts"'
>
Seems both 'target_cpus == 0x800 && reason == "Function call
interrupts"' and 'target_cpus & 0x800 && reason == "Function call
interrupts"' don't work:
# cat events/ipi/ipi_raise/enable
1
# cat events/ipi/ipi_raise/filter
target_cpus == 0x800 && reason == "Function call interrupts"
The kernel module code snippet:
void ipi_func_run_cpu(void *info)
{
pr_info("remote function runs on cpu[%d].\n", smp_processor_id());
}
static int __init ipi_send_init(void)
{
int target = (smp_processor_id() + 1) % nr_cpu_ids;
int ret = smp_call_function_single(target, ipi_func_run_cpu,
NULL, true);
pr_info("ipi cpu[%d --> %d] ret = %d\n", smp_processor_id(),
target, ret);
return 0;
}
...
module_init(ipi_send_init);
module_exit(ipi_send_exit);
$ sudo taskset -c 10 insmod ipi_send.ko
$ dmesg
...
[84931.864273] remote function runs on cpu[11].
[84931.864282] ipi cpu[10 --> 11] ret = 0
The 'cat trace' will output the below message with 'reason ==
"Function call interrupts"' filter:
...
sudo-5726[007] dn.h1.. 84302.833545: ipi_raise:
target_mask=,0001 (Function call interrupts)
sudo-5726[007] dn.h2.. 84302.837544: ipi_raise:
target_mask=,0001 (Function call interrupts)
insmod-5727[011] dn.h1.. 84302.841545: ipi_raise:
target_mask=,0001 (Function call interrupts)
insmod-5727[010] 1.. 84302.843966: ipi_raise:
target_mask=,0bff (Function call interrupts)
insmod-5727[010] 1.. 84302.843975: ipi_raise:
target_mask=,0bff (Function call interrupts)
insmod-5727[010] 1.. 84302.844184: ipi_raise:
target_mask=,0800 (Function call interrupts)
...
I find that 'target_cpus == 0xfff && reason == "Function call
interrupts"' doesn't have output in the buffer, but 'target_cpus &
0xfff && reason == "Function call interrupts"' does. I also tried to
use 'target_cpus & 0xf00 && reason == "Function call interrupts"' in
my case, the trace buffer has nothing after the kmod inserted.
Any comments?
>
>
> -- Steve
Question about the ipi_raise filter usage and output
Hi guys, With the ipi_raise event enabled and filtered with: echo 'reason == "Function call interrupts"' > filter, then the 'cat trace' output below messages: ... insmod-3355[010] 1.. 24479.230381: ipi_raise: target_mask=,0bff (Function call interrupts) ... The above output is triggered by my kernel module where it will smp cross call a remote function from cpu#10 to cpu#11, for the 'target_mask' value, what does the ',0bff' mean? ~~ Another question is for the filter, I'd like to catch the IPI only happening on cpu#11 *AND* a remote function call, so how to write the 'target_cpus' in the filter expression? I try to write below: echo 'target_cpus == 11 && reason == "Function call interrupts"' > events/ipi/ipi_raise/filter But the 'cat trace' doesn't show anything about cpu#11 IPI info, although both the /proc/interrupts and the smp_processor_id() in the remote function shows there's IPI sent to the cpu#11. Any suggestions? Thank you!
Re: [PATCH 2/5] UML: remove unused cmd_vdso_install
- Ursprüngliche Mail - > Von: "masahiroy" > An: "linux-kbuild" > CC: "linux-kernel" , "linux-arm-kernel" > , > linux-c...@vger.kernel.org, "linux-parisc" , > linux-ri...@lists.infradead.org, > linux-s...@vger.kernel.org, "linux-um" , > "loongarch" , > "sparclinux" , "x86" , > "masahiroy" , "anton ivanov" > , "bp" , "dave hansen" > , "hpa" > , "mingo" , "Johannes Berg" > , "richard" , > "tglx" > Gesendet: Montag, 9. Oktober 2023 14:42:07 > Betreff: [PATCH 2/5] UML: remove unused cmd_vdso_install > You cannot run this code because arch/um/Makefile does not define the > vdso_install target. > > It appears that this code was blindly copied from another architecture. > > Remove the dead code. > > Signed-off-by: Masahiro Yamada Acked-by: Richard Weinberger Thanks, //richard
Re: [PATCH] ptp: Don't print an error if ptp_kvm is not supported
On Tue, Apr 20, 2021 at 02:24:19PM +0100, Jon Hunter wrote:
> Commit 300bb1fe7671 ("ptp: arm/arm64: Enable ptp_kvm for arm/arm64")
> enable ptp_kvm support for ARM platforms and for any ARM platform that
> does not support this, the following error message is displayed ...
>
> ERR KERN fail to initialize ptp_kvm
>
> For platforms that do not support ptp_kvm this error is a bit misleading
> and so fix this by only printing this message if the error returned by
> kvm_arch_ptp_init() is not -EOPNOTSUPP. Note that -EOPNOTSUPP is only
> returned by ARM platforms today if ptp_kvm is not supported.
>
> Fixes: 300bb1fe7671 ("ptp: arm/arm64: Enable ptp_kvm for arm/arm64")
> Signed-off-by: Jon Hunter
Acked-by: Richard Cochran
Re: [net-next 1/3] net: dsa: optimize tx timestamp request handling
On Sun, Apr 18, 2021 at 12:18:42PM +0300, Vladimir Oltean wrote: > > How about not passing "clone" back to DSA as an argument by reference, > but instead require the driver to populate DSA_SKB_CB(skb)->clone if it > needs to do so? > > Also, how about changing the return type to void? Returning true or > false makes no difference. +1 Thanks, Richard
Re: [net-next 1/3] net: dsa: optimize tx timestamp request handling
On Fri, Apr 16, 2021 at 08:36:53PM +0800, Yangbo Lu wrote:
> @@ -628,9 +620,7 @@ static netdev_tx_t dsa_slave_xmit(struct sk_buff *skb,
> struct net_device *dev)
>
> DSA_SKB_CB(skb)->clone = NULL;
>
> - /* Identify PTP protocol packets, clone them, and pass them to the
> - * switch driver
> - */
> + /* Handle tx timestamp request if has */
"if has" what?
> dsa_skb_tx_timestamp(p, skb);
>
> if (dsa_realloc_skb(skb, dev)) {
> --
> 2.25.1
>
Thanks,
Richard
Re: [net-next 1/3] net: dsa: optimize tx timestamp request handling
On Fri, Apr 16, 2021 at 08:36:53PM +0800, Yangbo Lu wrote: > Optimization could be done on dsa_skb_tx_timestamp(), and dsa device > drivers should adapt to it. > > - Check SKBTX_HW_TSTAMP request flag at the very beginning, instead of in > port_txtstamp, so that most skbs not requiring tx timestamp just return. > > - No longer to identify PTP packets, and limit tx timestamping only for PTP > packets. If device driver likes, let device driver do. > > - It is a waste to clone skb directly in dsa_skb_tx_timestamp(). > For one-step timestamping, a clone is not needed. For any failure of > port_txtstamp (this may usually happen), the skb clone has to be freed. > So put skb cloning into port_txtstamp where it really needs. This patch changes three things ^^^ at once. These are AFAICT independent changes, and so please break this one patch into three for easier review. Thanks, Richard
Re: [PATCH 00/13] [RFC] Rust support
On Thu, Apr 15, 2021 at 2:41 AM wrote: > Regarding compilers, we support Clang-built kernels as well as > `LLVM=1` builds where possible (i.e. as long as supported by > the ClangBuiltLinux project). We also maintain some configurations > of GCC-built kernels working, but they are not intended to be used > at the present time. Having a `bindgen` backend for GCC would be > ideal to improve support for those builds. Sp this effectively means gcc is a second class citizen and even if gcc is supported at some point one needs a super recent gcc *and* rust toolchain to build a rust-enabeled kernel? I understand that this is right now not a big deal, but as soon a non-trival subsystem is rust-only people are forced to upgrade. Don't get me wrong, I'm all for having rust support in Linux. But I'm a bit worried about new dependencies on compiler toolchains. As someone who works a lot with long supported embedded systems I learned that as soon an application gains a hard dependency on clang or rust I'm in trouble. -- Thanks, //richard
Re: [PATCH v2] time: Fix overwrite err unexpected in clock_adjtime32
On Wed, Apr 14, 2021 at 03:04:49AM +, Chen Jun wrote:
> the correct error is covered by put_old_timex32.
>
> Fixes: 3a4d44b61625 ("ntp: Move adjtimex related compat syscalls to native
> counterparts")
> Signed-off-by: Chen Jun
Reviewed-by: Richard Cochran
Re: [PATCH net-next v4 1/1] net: stmmac: Add support for external trigger timestamping
On Wed, Apr 14, 2021 at 08:16:17AM +0800, Wong Vee Khee wrote: > From: Tan Tee Min > > The Synopsis MAC controller supports auxiliary snapshot feature that > allows user to store a snapshot of the system time based on an external > event. > > This patch add supports to the above mentioned feature. Users will be > able to triggered capturing the time snapshot from user-space using > application such as testptp or any other applications that uses the > PTP_EXTTS_REQUEST ioctl request. > > Cc: Richard Cochran > Signed-off-by: Tan Tee Min > Co-developed-by: Wong Vee Khee > Signed-off-by: Wong Vee Khee Acked-by: Richard Cochran
RE: [PATCH] arm64: dts: imx8mq-evk: add one regulator used to power up pcie phy
Hi Shawn: Regarding Lucas' advice, this patch should be split out and post for you to pick up into DT tree. Since the other two patches are accepted by PCIe tree now. Can you help to pick up this patch? Thanks in advanced. https://patchwork.kernel.org/project/linux-pci/patch/1616661882-26487-3-git-send-email-hongxing@nxp.com/ https://patchwork.kernel.org/project/linux-pci/patch/1617091701-6444-2-git-send-email-hongxing@nxp.com/ Best Regards Richard Zhu > -Original Message- > From: Richard Zhu > Sent: Wednesday, April 14, 2021 10:26 AM > To: shawn...@kernel.org > Cc: l.st...@pengutronix.de; dl-linux-imx ; > linux-arm-ker...@lists.infradead.org; linux-kernel@vger.kernel.org; Richard > Zhu > Subject: [PATCH] arm64: dts: imx8mq-evk: add one regulator used to power > up pcie phy > > Both 1.8v and 3.3v power supplies can be used by i.MX8MQ PCIe PHY. > In default, the PCIE_VPH voltage is suggested to be 1.8v refer to data sheet. > When PCIE_VPH is supplied by 3.3v in the HW schematic design, the > VREG_BYPASS bits of GPR registers should be cleared from default value 1b'1 > to 1b'0. Thus, the internal 3v3 to 1v8 translator would be turned on. > > Signed-off-by: Richard Zhu > Reviewed-by: Lucas Stach > --- > arch/arm64/boot/dts/freescale/imx8mq-evk.dts | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm64/boot/dts/freescale/imx8mq-evk.dts > b/arch/arm64/boot/dts/freescale/imx8mq-evk.dts > index 85b045253a0e..4d2035e3dd7c 100644 > --- a/arch/arm64/boot/dts/freescale/imx8mq-evk.dts > +++ b/arch/arm64/boot/dts/freescale/imx8mq-evk.dts > @@ -318,6 +318,7 @@ ><&clk IMX8MQ_CLK_PCIE1_PHY>, ><&pcie0_refclk>; > clock-names = "pcie", "pcie_aux", "pcie_phy", "pcie_bus"; > + vph-supply = <&vgen5_reg>; > status = "okay"; > }; > > -- > 2.17.1
[PATCH] arm64: dts: imx8mq-evk: add one regulator used to power up pcie phy
Both 1.8v and 3.3v power supplies can be used by i.MX8MQ PCIe PHY. In default, the PCIE_VPH voltage is suggested to be 1.8v refer to data sheet. When PCIE_VPH is supplied by 3.3v in the HW schematic design, the VREG_BYPASS bits of GPR registers should be cleared from default value 1b'1 to 1b'0. Thus, the internal 3v3 to 1v8 translator would be turned on. Signed-off-by: Richard Zhu Reviewed-by: Lucas Stach --- arch/arm64/boot/dts/freescale/imx8mq-evk.dts | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm64/boot/dts/freescale/imx8mq-evk.dts b/arch/arm64/boot/dts/freescale/imx8mq-evk.dts index 85b045253a0e..4d2035e3dd7c 100644 --- a/arch/arm64/boot/dts/freescale/imx8mq-evk.dts +++ b/arch/arm64/boot/dts/freescale/imx8mq-evk.dts @@ -318,6 +318,7 @@ <&clk IMX8MQ_CLK_PCIE1_PHY>, <&pcie0_refclk>; clock-names = "pcie", "pcie_aux", "pcie_phy", "pcie_bus"; + vph-supply = <&vgen5_reg>; status = "okay"; }; -- 2.17.1
[PATCH] arm64: dts: imx8mq-evk: add one regulator used to power up
The other two patches are accepted into PCIe tree. It's time to post it for Shawn to pick up into the imx DT tree. https://patchwork.kernel.org/project/linux-pci/patch/1616661882-26487-3-git-send-email-hongxing@nxp.com/ arch/arm64/boot/dts/freescale/imx8mq-evk.dts | 1 + 1 file changed, 1 insertion(+) [PATCH] arm64: dts: imx8mq-evk: add one regulator used to power up
[GIT PULL] MTD fixes for 5.12-rc7
Linus, The following changes since commit a38fd8748464831584a19438cbb3082b5a2dab15: Linux 5.12-rc2 (2021-03-05 17:33:41 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git tags/fixes-for-5.12-rc7 for you to fetch changes up to 2fb164f0ce95e504e2688b4f984893c29ebd19ab: mtd: rawnand: mtk: Fix WAITRDY break condition and timeout (2021-03-11 12:23:11 +0100) This pull request contains the following bug fix for MTD: - nand: Fix WAITRDY break condition and timeout in mtk driver Hauke Mehrtens (1): mtd: rawnand: mtk: Fix WAITRDY break condition and timeout drivers/mtd/nand/raw/mtk_nand.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Re: [PATCHv5 0/7] Extend Intel service layer, FPGA manager and region
Hi Moritz, On 3/28/21 12:20 PM, Moritz Fischer wrote: Tom, On Sun, Mar 28, 2021 at 08:40:24AM -0700, Tom Rix wrote: On 3/27/21 11:09 AM, Moritz Fischer wrote: Hi Richard, Russ, On Thu, Feb 25, 2021 at 01:07:14PM +, Gong, Richard wrote: Hi Moritz, Sorry for asking. When you have chance, can you help review the version 5 patchset submitted on 02/09/21? Regards, Richard -Original Message- From: richard.g...@linux.intel.com Sent: Tuesday, February 9, 2021 4:20 PM To: m...@kernel.org; t...@redhat.com; gre...@linuxfoundation.org; linux-f...@vger.kernel.org; linux-kernel@vger.kernel.org Cc: Gong, Richard Subject: [PATCHv5 0/7] Extend Intel service layer, FPGA manager and region From: Richard Gong This is 5th submission of Intel service layer and FPGA patches, which includes the missing standalone patch in the 4th submission. This submission includes additional changes for Intel service layer driver to get the firmware version running at FPGA SoC device. Then FPGA manager driver, one of Intel service layer driver's client, can decide whether to handle the newly added bitstream authentication function based on the retrieved firmware version. So that we can maintain FPGA manager driver the back compatible. Bitstream authentication makes sure a signed bitstream has valid signatures. The customer sends the bitstream via FPGA framework and overlay, the firmware will authenticate the bitstream but not program the bitstream to device. If the authentication passes, the bitstream will be programmed into QSPI flash and will be expected to boot without issues. Extend Intel service layer, FPGA manager and region drivers to support the bitstream authentication feature. Richard Gong (7): firmware: stratix10-svc: reset COMMAND_RECONFIG_FLAG_PARTIAL to 0 firmware: stratix10-svc: add COMMAND_AUTHENTICATE_BITSTREAM flag firmware: stratix10-svc: extend SVC driver to get the firmware version fpga: fpga-mgr: add FPGA_MGR_BITSTREAM_AUTHENTICATE flag fpga: of-fpga-region: add authenticate-fpga-config property dt-bindings: fpga: add authenticate-fpga-config property fpga: stratix10-soc: extend driver for bitstream authentication .../devicetree/bindings/fpga/fpga-region.txt | 10 drivers/firmware/stratix10-svc.c | 12 - drivers/fpga/of-fpga-region.c | 24 ++--- drivers/fpga/stratix10-soc.c | 62 +++--- include/linux/firmware/intel/stratix10-smc.h | 21 +++- .../linux/firmware/intel/stratix10-svc-client.h| 11 +++- include/linux/fpga/fpga-mgr.h | 3 ++ 7 files changed, 125 insertions(+), 18 deletions(-) -- 2.7.4 Apologies for the epic delay in getting back to this, I took another look at this patchset and Russ' patchset. TL;DR I'm not really a fan of using device-tree overlays for this (and again, apologies, I should've voiced this earlier ...). Anyways, let's find a common API for this and Russ' work, they're trying to achieve the same / similar thing, they should use the same API. I'd like to re-invetigate the possiblity to extend FPGA Manager with 'secure update' ops that work for both these use-cases (and I susspect hte XRT patchset will follow with a similar requirement, right after). The xrt patchset makes heavy use of device trees. What is the general guidance for device tree usage ? I'm not generally against using device tree, it has its place. To describe hardware (and hardware *changes* with overlays) :) What I don't like about this particular implementation w.r.t device-tree usage is that it uses DT overlays as a mechanism to program the flash -- in place of having an API to do so. One could add device-nodes during the DT overlay application, while the FPGA doesn't actually get programmed with a new runtime image -- meaning live DT and actual hardware state diverged -- worst case it'd crash. So when roughly at the same time (from the same company even) we have two patchsets that do similar things with radically different APIs I think we should pause, and reflect on whether we can come up with something that works for both :) I discussed with Russ and studies his patches, came to realize that the work we had to accomplish was not same or similar. What I want to achieve is to verify the identity of the bitstream, which is like doing a "dry-run" to FPGA configuration. Performing FPGA configuration (full or partial) through the device tree overlay is a method widely used by our customers. Russ's approach utilizes a different user API which is a set of sysfs files. If we depart from device tree overlay, then the end-user must utilize 2 different mechanism or APIs (device tree overlay is used for full/partial configuration, and sysfs is used for bitstream authentication). Similarly low-level FPGA manager driver
Re: [PATCH] time: Fix overwrite err unexpected in clock_adjtime32
On Mon, Apr 12, 2021 at 02:52:11PM +, chenjun (AM) wrote:
> 在 2021/4/12 22:20, Richard Cochran 写道:
> > On Mon, Apr 12, 2021 at 12:45:51PM +, Chen Jun wrote:
> >> the correct error is covered by put_old_timex32.
> >
> > Well, the non-negative return code (TIME_OK, TIME_INS, etc) is
> > clobbered by put_old_timex32().
> >
> >> Fixes: f1f1d5ebd10f ("posix-timers: Introduce a syscall for clock tuning.")
> >
> > This is not the correct commit for the "Fixes" tag. Please find the
> > actual commit that introduced the issue.
> >
> > In commit f1f1d5ebd10f the code looked like this...
> >
> > long compat_sys_clock_adjtime(clockid_t which_clock,
> > struct compat_timex __user *utp)
> > {
> > struct timex txc;
> > mm_segment_t oldfs;
> > int err, ret;
> >
> > err = compat_get_timex(&txc, utp);
> > if (err)
> > return err;
> >
> > oldfs = get_fs();
> > set_fs(KERNEL_DS);
> > ret = sys_clock_adjtime(which_clock, (struct timex __user *)
> > &txc);
> > set_fs(oldfs);
> >
> > err = compat_put_timex(utp, &txc);
> > if (err)
> > return err;
> >
> > return ret;
> > }
Look at the code ^^^
> The implement of clock_adjtime32 is similar to compat_sys_clock_adjtime.
> And I think f1f1d5ebd10 introduced the problem actually.
See how 'ret' and 'err' are two separate variables? It makes a difference.
Thanks,
Richard
Re: [PATCH] time: Fix overwrite err unexpected in clock_adjtime32
On Mon, Apr 12, 2021 at 12:45:51PM +, Chen Jun wrote:
> the correct error is covered by put_old_timex32.
Well, the non-negative return code (TIME_OK, TIME_INS, etc) is
clobbered by put_old_timex32().
> Fixes: f1f1d5ebd10f ("posix-timers: Introduce a syscall for clock tuning.")
This is not the correct commit for the "Fixes" tag. Please find the
actual commit that introduced the issue.
In commit f1f1d5ebd10f the code looked like this...
long compat_sys_clock_adjtime(clockid_t which_clock,
struct compat_timex __user *utp)
{
struct timex txc;
mm_segment_t oldfs;
int err, ret;
err = compat_get_timex(&txc, utp);
if (err)
return err;
oldfs = get_fs();
set_fs(KERNEL_DS);
ret = sys_clock_adjtime(which_clock, (struct timex __user *)
&txc);
set_fs(oldfs);
err = compat_put_timex(utp, &txc);
if (err)
return err;
return ret;
}
Thanks,
Richard
> Signed-off-by: Chen Jun
> ---
> kernel/time/posix-timers.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c
> index bf540f5a..dd5697d 100644
> --- a/kernel/time/posix-timers.c
> +++ b/kernel/time/posix-timers.c
> @@ -1191,8 +1191,8 @@ SYSCALL_DEFINE2(clock_adjtime32, clockid_t, which_clock,
>
> err = do_clock_adjtime(which_clock, &ktx);
>
> - if (err >= 0)
> - err = put_old_timex32(utp, &ktx);
> + if (err >= 0 && put_old_timex32(utp, &ktx))
> + return -EFAULT;
>
> return err;
> }
> --
> 2.9.4
>
Re: [PATCH net-next v3 1/1] net: stmmac: Add support for external trigger timestamping
On Sun, Apr 11, 2021 at 10:40:28AM +0800, Wong Vee Khee wrote:
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
> b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
> index 60566598d644..60e17fd24aba 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
> @@ -296,6 +296,13 @@ static int intel_crosststamp(ktime_t *device,
>
> intel_priv = priv->plat->bsp_priv;
>
> + /* Both internal crosstimestamping and external triggered event
> + * timestamping cannot be run concurrently.
> + */
> + if (priv->plat->ext_snapshot_en)
> + return -EBUSY;
> +
> + mutex_lock(&priv->aux_ts_lock);
Lock, then ...
> /* Enable Internal snapshot trigger */
> acr_value = readl(ptpaddr + PTP_ACR);
> acr_value &= ~PTP_ACR_MASK;
> @@ -321,6 +328,7 @@ static int intel_crosststamp(ktime_t *device,
> acr_value = readl(ptpaddr + PTP_ACR);
> acr_value |= PTP_ACR_ATSFC;
> writel(acr_value, ptpaddr + PTP_ACR);
> + mutex_unlock(&priv->aux_ts_lock);
unlock, then ...
> /* Trigger Internal snapshot signal
>* Create a rising edge by just toggle the GPO1 to low
> @@ -355,6 +363,8 @@ static int intel_crosststamp(ktime_t *device,
> *system = convert_art_to_tsc(art_time);
> }
>
> + /* Release the mutex */
> + mutex_unlock(&priv->aux_ts_lock);
unlock again? Huh?
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> index c49debb62b05..abadcd8cdc41 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> @@ -239,6 +239,9 @@ struct stmmac_priv {
> int use_riwt;
> int irq_wake;
> spinlock_t ptp_lock;
> + /* Mutex lock for Auxiliary Snapshots */
> + struct mutex aux_ts_lock;
In the comment, please be specific about which data are protected.
For example:
/* Protects auxiliary snapshot registers from concurrent access. */
> @@ -163,6 +166,43 @@ static void get_ptptime(void __iomem *ptpaddr, u64
> *ptp_time)
> *ptp_time = ns;
> }
>
> +static void timestamp_interrupt(struct stmmac_priv *priv)
> +{
> + struct ptp_clock_event event;
> + unsigned long flags;
> + u32 num_snapshot;
> + u32 ts_status;
> + u32 tsync_int;
Please group same types together (u32) in a one-line list.
> + u64 ptp_time;
> + int i;
> +
> + tsync_int = readl(priv->ioaddr + GMAC_INT_STATUS) & GMAC_INT_TSIE;
> +
> + if (!tsync_int)
> + return;
> +
> + /* Read timestamp status to clear interrupt from either external
> + * timestamp or start/end of PPS.
> + */
> + ts_status = readl(priv->ioaddr + GMAC_TIMESTAMP_STATUS);
Reading this register has a side effect of clearing status? If so,
doesn't it need protection against concurrent access?
The function, intel_crosststamp() also reads this bit.
> + if (!priv->plat->ext_snapshot_en)
> + return;
Doesn't this test come too late? Setting ts_status just cleared the
bit used by the other code path.
> + num_snapshot = (ts_status & GMAC_TIMESTAMP_ATSNS_MASK) >>
> +GMAC_TIMESTAMP_ATSNS_SHIFT;
> +
> + for (i = 0; i < num_snapshot; i++) {
> + spin_lock_irqsave(&priv->ptp_lock, flags);
> + get_ptptime(priv->ptpaddr, &ptp_time);
> + spin_unlock_irqrestore(&priv->ptp_lock, flags);
> + event.type = PTP_CLOCK_EXTTS;
> + event.index = 0;
> + event.timestamp = ptp_time;
> + ptp_clock_event(priv->ptp_clock, &event);
> + }
> +}
> +
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
> b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
> index b164ae22e35f..d668c33a0746 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
> @@ -135,9 +135,13 @@ static int stmmac_enable(struct ptp_clock_info *ptp,
> {
> struct stmmac_priv *priv =
> container_of(ptp, struct stmmac_priv, ptp_clock_ops);
> + void __iomem *ptpaddr = priv->ptpaddr;
> + void __iomem *ioaddr = priv->hw->pcsr;
> struct stmmac_pps_cfg *cfg;
> int ret = -EOPNOTSUPP;
> unsigned long flags;
> + u32 intr_value;
> + u32 acr_value;
Please group same types together.
Thanks,
Richard
Re: [PATCH] ubifs: default to zstd compression
On Mon, Apr 5, 2021 at 5:36 PM Rui Salvaterra wrote: > > Compared to lzo and zlib, zstd is the best all-around performer, both in terms > of speed and compression ratio. Set it as the default, if available. I was about to NACK this patch but by looking at the diff I realized that you change the default compressor only for the default filesystem as created by UBIFS itself. Queued for the merge window. :-) -- Thanks, //richard
Re: [PATCH 4/6] fs/jffs2: Delete obsolete TODO file
On Tue, Mar 30, 2021 at 9:07 AM Wang Qing wrote: > > The TODO file here has not been updated for 14 years, and the function > development described in the file have been implemented or abandoned. > > Its existence will mislead developers seeking to view outdated information. Did you check whether all items in this list are really outdated? Nobody shall ever blindly follow a TODO list without checking first which points are still valid or not. Removing that file does not magically solve the issues it describes. -- Thanks, //richard
Re: [PATCH] ubifs: fix read fail but return ok
On Wed, Mar 31, 2021 at 8:29 AM wangfangpeng wrote: > do_readpage() may return err, but ubifs_readpage() always return ok. > The vfs will ignore the err happen in ubifs. Are you sure about that? In case of an error UBIFS sets the error flag of the page and does not mark it as uptodate, so vfs will emit -EIO. At least this is the theory. :-) -- Thanks, //richard
Re: [PATCH v1 1/1] ubifs: only check replay with inode type to judge if inode linked
On Tue, Mar 16, 2021 at 10:00 AM wrote:
>
> From: Guochun Mao
>
> Conside the following case, it just write a big file into flash,
> when complete writing, delete the file, and then power off promptly.
> Next time power on, we'll get a replay list like:
> ...
> LEB 1105:211344 len 4144 deletion 0 sqnum 428783 key type 1 inode 80
> LEB 15:233544 len 160 deletion 1 sqnum 428785 key type 0 inode 80
> LEB 1105:215488 len 4144 deletion 0 sqnum 428787 key type 1 inode 80
> ...
> In the replay list, data nodes' deletion are 0, and the inode node's
> deletion is 1. In current logic, the file's dentry will be removed,
> but inode and the flash space it occupied will be reserved.
> User will see that much free space been disappeared.
>
> We only need to check the deletion value of the following inode type
> node of the replay entry.
>
> Signed-off-by: Guochun Mao
> ---
> fs/ubifs/replay.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fs/ubifs/replay.c b/fs/ubifs/replay.c
> index 0f8a6a16421b..1929ec63a0cb 100644
> --- a/fs/ubifs/replay.c
> +++ b/fs/ubifs/replay.c
> @@ -223,7 +223,8 @@ static bool inode_still_linked(struct ubifs_info *c,
> struct replay_entry *rino)
> */
> list_for_each_entry_reverse(r, &c->replay_list, list) {
> ubifs_assert(c, r->sqnum >= rino->sqnum);
> - if (key_inum(c, &r->key) == key_inum(c, &rino->key))
> + if (key_inum(c, &r->key) == key_inum(c, &rino->key) &&
> + key_type(c, &r->key) == UBIFS_INO_KEY)
This change makes sense. Thanks a lot for hunting this down.
It will be part of the merge window.
--
Thanks,
//richard
Re: [PATCH v19 3/7] ptp: Reorganize ptp_kvm.c to make it arch-independent
On Wed, Apr 07, 2021 at 10:28:44AM +0100, Marc Zyngier wrote: > On Tue, 30 Mar 2021 15:54:26 +0100, > Marc Zyngier wrote: > > > > From: Jianyong Wu > > > > Currently, the ptp_kvm module contains a lot of x86-specific code. > > Let's move this code into a new arch-specific file in the same directory, > > and rename the arch-independent file to ptp_kvm_common.c. > > > > Reviewed-by: Andre Przywara > > Signed-off-by: Jianyong Wu > > Signed-off-by: Marc Zyngier > > Link: https://lore.kernel.org/r/20201209060932.212364-4-jianyong...@arm.com > > Richard, Paolo, > > Can I get an Ack on this and patch #7? We're getting pretty close to > the next merge window, and this series has been going on for a couple > of years now... For both patches: Acked-by: Richard Cochran
[PATCH] audit: drop /proc/PID/loginuid documentation Format field
Drop the "Format:" field from the /proc/PID/loginuid documentation and
integrate the information into the Description field since it is not
recognized by the "./scripts/get_abi.pl validate" command which causes a
warning. Documentation/ABI/README describes the valid fields.
Reported-by: Mauro Carvalho Chehab
Signed-off-by: Richard Guy Briggs
---
.../ABI/stable/procfs-audit_loginuid | 22 +--
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/Documentation/ABI/stable/procfs-audit_loginuid
b/Documentation/ABI/stable/procfs-audit_loginuid
index 9d55a3ff4b34..cda405178391 100644
--- a/Documentation/ABI/stable/procfs-audit_loginuid
+++ b/Documentation/ABI/stable/procfs-audit_loginuid
@@ -2,26 +2,26 @@ What: Audit Login UID
Date: 2005-02-01
KernelVersion: 2.6.11-rc2 1e2d1492e178 ("[PATCH] audit: handle loginuid
through proc")
Contact: linux-au...@redhat.com
-Format:%u
Users: audit and login applications
Description:
The /proc/$pid/loginuid pseudofile is written to set and
- read to get the audit login UID of process $pid. If it is
- unset, permissions are not needed to set it. The accessor must
- have CAP_AUDIT_CONTROL in the initial user namespace to write
- it if it has been set. It cannot be written again if
- AUDIT_FEATURE_LOGINUID_IMMUTABLE is enabled. It cannot be
- unset if AUDIT_FEATURE_ONLY_UNSET_LOGINUID is enabled.
-
+ read to get the audit login UID of process $pid as a
+ decimal unsigned int (%u, u32). If it is unset,
+ permissions are not needed to set it. The accessor must
+ have CAP_AUDIT_CONTROL in the initial user namespace to
+ write it if it has been set. It cannot be written again
+ if AUDIT_FEATURE_LOGINUID_IMMUTABLE is enabled. It
+ cannot be unset if AUDIT_FEATURE_ONLY_UNSET_LOGINUID is
+ enabled.
What: Audit Login Session ID
Date: 2008-03-13
KernelVersion: 2.6.25-rc7 1e0bd7550ea9 ("[PATCH] export sessionid alongside
the loginuid in procfs")
Contact: linux-au...@redhat.com
-Format:%u
Users: audit and login applications
Description:
The /proc/$pid/sessionid pseudofile is read to get the
- audit login session ID of process $pid. It is set
- automatically, serially assigned with each new login.
+ audit login session ID of process $pid as a decimal
+ unsigned int (%u, u32). It is set automatically,
+ serially assigned with each new login.
--
2.27.0
Re: [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
Sumit, - Ursprüngliche Mail - > Von: "Sumit Garg" > IIUC, this would require support for multiple trusted keys backends at > runtime but currently the trusted keys subsystem only supports a > single backend which is selected via kernel module parameter during > boot. > > So the trusted keys framework needs to evolve to support multiple > trust sources at runtime but I would like to understand the use-cases > first. IMO, selecting the best trust source available on a platform > for trusted keys should be a one time operation, so why do we need to > have other backends available at runtime as well? I thought about devices with a TPM-Chip and CAAM. IMHO allowing only one backend at the same time is a little over simplified. Thanks, //richard
Re: [PATCH v3 1/2] audit: document /proc/PID/loginuid
On 2021-04-01 09:57, Paul Moore wrote:
> On Thu, Apr 1, 2021 at 9:48 AM Mauro Carvalho Chehab
> wrote:
> > Em Thu, 18 Mar 2021 15:19:10 -0400
> > Richard Guy Briggs escreveu:
> > > Describe the /proc/PID/loginuid interface in Documentation/ABI/stable that
> > > was added 2005-02-01 by commit 1e2d1492e178 ("[PATCH] audit: handle
> > > loginuid through proc")
> > >
> > > Signed-off-by: Richard Guy Briggs
> > > ---
> > > Documentation/ABI/stable/procfs-audit_loginuid | 15 +++
> > > 1 file changed, 15 insertions(+)
> > > create mode 100644 Documentation/ABI/stable/procfs-audit_loginuid
> > >
> > > diff --git a/Documentation/ABI/stable/procfs-audit_loginuid
> > > b/Documentation/ABI/stable/procfs-audit_loginuid
> > > new file mode 100644
> > > index ..e7c100b9ab18
> > > --- /dev/null
> > > +++ b/Documentation/ABI/stable/procfs-audit_loginuid
> > > @@ -0,0 +1,15 @@
> > > +What:Audit Login UID
> > > +Date:2005-02-01
> > > +KernelVersion: 2.6.11-rc2 1e2d1492e178 ("[PATCH] audit: handle
> > > loginuid through proc")
> > > +Contact: linux-au...@redhat.com
> > > +Format: %u
> >
> > The ABI definition doesn't include a "Format:" symbol. See:
> >
> > Documentation/ABI/README
> >
> > For the valid ones.
> >
> > This change causes a warning at the ABI parser:
> >
> >
> > $ ./scripts/get_abi.pl validate
> > Warning: file Documentation/ABI/stable/procfs-audit_loginuid#5:
> > tag 'contact' is invalid. Line
> > Format: %u
> > Warning: file Documentation/ABI/stable/procfs-audit_loginuid#21:
> > tag 'contact' is invalid. Line
> > Format: %u
> >
> > You should either drop it or add it to the parser and to the README
> > file, if the ABI maintainers are ok with such new field.
>
> Thanks Mauro, I didn't realize there were tools that parsed these files.
>
> Richard, please post a patch that drops the 'Format:' line from the
> newly added audit files as soon as possible so I can merge it into
> audit/next.
Ok. I'll roll it into the description so we don't lose that
information.
> paul moore
- RGB
--
Richard Guy Briggs
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
Re: [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
Ahmad, - Ursprüngliche Mail - > Von: "Ahmad Fatoum" >> But using LUKS would mean that cryptsetup has access to the plain disc >> encryption key material? >> This would be a no-go for many systems out there, key material must not >> accessible to userspace. >> I know, distrusting userspace root is not easy, but doable. :) > > The LUKS2 format supports tokens. I see no reason why the encrypted blob > couldn't be stored there along with the usual metadata. cryptsetup would > then load it as kernel trusted key and use it for dmcrypt decryption. > > This will mean we have to part ways with features such as having multiple > keys, but I think it's worth it to have a plug and play solution for > trusted keys. Ah, now I can follow your thoughts! Yes, that would be nice to have. :) I kind of assumed you want to use LUKS with passphrases and CAAM blobs. Thanks, //richard
Re: [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
Sumit, - Ursprüngliche Mail - > Von: "Sumit Garg" > In this case why would one prefer to use CAAM when you have standards > compliant TPM-Chip which additionally offers sealing to specific PCR > (integrity measurement) values. I don't think we can dictate what good/sane solutions are and which are not. Both CAAM and TPM have pros and cons, I don't see why supporting both is a bad idea. >> > IMHO allowing only one backend at the same time is a little over >> > simplified. >> >> It is, but I'd rather leave this until it's actually needed. >> What can be done now is adopting a format for the exported keys that would >> make this extension seamless in future. >> > > +1 As long we don't make multiple backends at runtime impossible I'm fine and will happily add support for it when needed. :-) Thanks, //richard
Re: [PATCH v5] audit: log nftables configuration change events once per table
On 2021-04-01 15:24, Phil Sutter wrote: > On Fri, Mar 26, 2021 at 01:38:59PM -0400, Richard Guy Briggs wrote: > > Reduce logging of nftables events to a level similar to iptables. > > Restore the table field to list the table, adding the generation. > > > > Indicate the op as the most significant operation in the event. > > > > A couple of sample events: > > > > type=PROCTITLE msg=audit(2021-03-18 09:30:49.801:143) : > > proctitle=/usr/bin/python3 -s /usr/sbin/firewalld --nofork --nopid > > type=SYSCALL msg=audit(2021-03-18 09:30:49.801:143) : arch=x86_64 > > syscall=sendmsg success=yes exit=172 a0=0x6 a1=0x7ffdcfcbe650 a2=0x0 > > a3=0x7ffdcfcbd52c items=0 ppid=1 pid=367 auid=unset uid=root gid=root > > euid=root suid=root fsuid=root egid=roo > > t sgid=root fsgid=root tty=(none) ses=unset comm=firewalld > > exe=/usr/bin/python3.9 subj=system_u:system_r:firewalld_t:s0 key=(null) > > type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.801:143) : > > table=firewalld:2 family=ipv6 entries=1 op=nft_register_table pid=367 > > subj=system_u:system_r:firewalld_t:s0 comm=firewalld > > type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.801:143) : > > table=firewalld:2 family=ipv4 entries=1 op=nft_register_table pid=367 > > subj=system_u:system_r:firewalld_t:s0 comm=firewalld > > type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.801:143) : > > table=firewalld:2 family=inet entries=1 op=nft_register_table pid=367 > > subj=system_u:system_r:firewalld_t:s0 comm=firewalld > > > > type=PROCTITLE msg=audit(2021-03-18 09:30:49.839:144) : > > proctitle=/usr/bin/python3 -s /usr/sbin/firewalld --nofork --nopid > > type=SYSCALL msg=audit(2021-03-18 09:30:49.839:144) : arch=x86_64 > > syscall=sendmsg success=yes exit=22792 a0=0x6 a1=0x7ffdcfcbe650 a2=0x0 > > a3=0x7ffdcfcbd52c items=0 ppid=1 pid=367 auid=unset uid=root gid=root > > euid=root suid=root fsuid=root egid=r > > oot sgid=root fsgid=root tty=(none) ses=unset comm=firewalld > > exe=/usr/bin/python3.9 subj=system_u:system_r:firewalld_t:s0 key=(null) > > type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.839:144) : > > table=firewalld:3 family=ipv6 entries=30 op=nft_register_chain pid=367 > > subj=system_u:system_r:firewalld_t:s0 comm=firewalld > > type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.839:144) : > > table=firewalld:3 family=ipv4 entries=30 op=nft_register_chain pid=367 > > subj=system_u:system_r:firewalld_t:s0 comm=firewalld > > type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.839:144) : > > table=firewalld:3 family=inet entries=165 op=nft_register_chain pid=367 > > subj=system_u:system_r:firewalld_t:s0 comm=firewalld > > > > The issue was originally documented in > > https://github.com/linux-audit/audit-kernel/issues/124 > > > > Signed-off-by: Richard Guy Briggs > > Tested this patch to make sure it eliminates the slowdown of > iptables-nft when auditd is running. With this applied, neither > iptables-nft-restore nor 'iptables-nft -F' show a significant > difference in run-time between running or stopped auditd, at least for > large rulesets. Individual calls suffer from added audit logging, but > that's expected of course. > > Tested-by: Phil Sutter Excellent, thanks Phil for helping nail this one down and confirming the fix. > Thanks, Phil - RGB -- Richard Guy Briggs Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada IRC: rgb, SunRaycer Voice: +1.647.777.2635, Internal: (81) 32635
Re: [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
Ahmad, - Ursprüngliche Mail - > Von: "Ahmad Fatoum" >> I don't want you to force to use cryptsetup. > > I'd love to use cryptsetup with LUKS and trusted keys eventually. I'll take But using LUKS would mean that cryptsetup has access to the plain disc encryption key material? This would be a no-go for many systems out there, key material must not accessible to userspace. I know, distrusting userspace root is not easy, but doable. :) Thanks, //richard
Re: [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
Ahmad, - Ursprüngliche Mail - > Do you mean systemd-cryptsetup? It looks to me like it's just a way to supply > the keyphrase. With trusted keys and a keyphrase unknown to userspace, this > won't work. Nah, I meant existing scripts/service Files. > I don't (yet) see the utility of it without LUKS. Perhaps a command dump on > how > to do the same I did with dmsetup, but with cryptsetup plain instead could > help me to see the benefits? My reasoning is simple, why do I need a different tool when there is already one that could do the task too? Usually the systems I get my hands on use already dm-crypt with cryptsetup in some way. So I have the tooling already in my initramfs, etc.. and need to adopt the callers of cryptsetup a little. If I need all of a sudden different/additional tooling, it means more work, more docs to write, more hassle with crypto/system reviewers, etc... I don't want you to force to use cryptsetup. The only goal was pointing out that it can be done with cryptsetup and that there is already code such that no work is done twice. One the kernel side it does not matter. Thanks, //richard
Re: [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
Ahmad, - Ursprüngliche Mail - > Von: "Ahmad Fatoum" >> I'm pretty sure with minimal changes it will work with your recent approach >> too. > > I am using dmsetup directly in my project. I am not familiar with cryptsetup > plain. What benefits do you see with this over direct dmsetup? cryptsetup is the de-facto standard to setup encrypted block devices. There is a lot of existing tooling around cryptsetup already (systemd, etc..), so being able to use CAAM keys for dm-crypt with cryptsetup seems natural to me. Plain mode allows setting up dm-crypt without LUKS. Thanks, //richard
Re: [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
Ahmad, - Ursprüngliche Mail - > Von: "Ahmad Fatoum" >> That way existing blobs can also be used with this implementation. >> IIRC the NXP vendor tree uses "SECURE_KEY" as default modifier. > > Being binary compatible with other implementations is not an objective > for this patch set. If you need to migrate I'd suggest to get out a > clear text password and side-load it into the trusted key framework. Compatibility is only one argument, IMHO the much stronger argument is that there are people out there that want to salt the CAAM blob with a key modifier of their own choice. Thanks, //richard
Re: [PATCH v5] audit: log nftables configuration change events once per table
On 2021-03-31 22:46, Pablo Neira Ayuso wrote:
> On Fri, Mar 26, 2021 at 01:38:59PM -0400, Richard Guy Briggs wrote:
> > @@ -8006,12 +7966,65 @@ static void nft_commit_notify(struct net *net, u32
> > portid)
> > WARN_ON_ONCE(!list_empty(&net->nft.notify_list));
> > }
> >
> > +static int nf_tables_commit_audit_alloc(struct list_head *adl,
> > +struct nft_table *table)
> > +{
> > + struct nft_audit_data *adp;
> > +
> > + list_for_each_entry(adp, adl, list) {
> > + if (adp->table == table)
> > + return 0;
> > + }
> > + adp = kzalloc(sizeof(*adp), GFP_KERNEL);
> > + if (!adp)
> > + return -ENOMEM;
> > + adp->table = table;
> > + INIT_LIST_HEAD(&adp->list);
>
> This INIT_LIST_HEAD is not required for an object that is going to be
> inserted into the 'adl' list.
>
> > + list_add(&adp->list, adl);
>
> If no objections, I'll amend this patch. I'll include the UAF fix and
> remove this unnecessary INIT_LIST_HEAD.
Ok, so it is harmless other than being code noise and overhead, thanks again.
- RGB
--
Richard Guy Briggs
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
Re: [PATCH v5] audit: log nftables configuration change events once per table
On 2021-03-31 22:22, Pablo Neira Ayuso wrote:
> On Fri, Mar 26, 2021 at 01:38:59PM -0400, Richard Guy Briggs wrote:
> > Reduce logging of nftables events to a level similar to iptables.
> > Restore the table field to list the table, adding the generation.
> >
> > Indicate the op as the most significant operation in the event.
>
> There's a UAF, Florian reported. I'm attaching an incremental fix.
>
> nf_tables_commit_audit_collect() refers to the trans object which
> might have been already released.
Got it. Thanks Pablo. I didn't see it when running nft-test.py Where
was it reported? Here I tried to stay out of the way by putting that
call at the end of the loop but that was obviously a mistake in
hindsight. :-)
> commit e4d272948d25b66d86fc241cefd95281bfb1079e
> Author: Pablo Neira Ayuso
> Date: Wed Mar 31 22:19:51 2021 +0200
>
> netfilter: nf_tables: use-after-free
>
> Signed-off-by: Pablo Neira Ayuso
>
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index 5dd4bb7cabf5..01674c0d9103 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -8063,6 +8063,8 @@ static int nf_tables_commit(struct net *net, struct
> sk_buff *skb)
> net->nft.gencursor = nft_gencursor_next(net);
>
> list_for_each_entry_safe(trans, next, &net->nft.commit_list, list) {
> + nf_tables_commit_audit_collect(&adl, trans->ctx.table,
> +trans->msg_type);
> switch (trans->msg_type) {
> case NFT_MSG_NEWTABLE:
> if (nft_trans_table_update(trans)) {
> @@ -8211,8 +8213,6 @@ static int nf_tables_commit(struct net *net, struct
> sk_buff *skb)
> }
> break;
> }
> - nf_tables_commit_audit_collect(&adl, trans->ctx.table,
> -trans->msg_type);
> }
>
> nft_commit_notify(net, NETLINK_CB(skb).portid);
- RGB
--
Richard Guy Briggs
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
Re: [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
James,
- Ursprüngliche Mail -
> Von: "James Bottomley"
> Well, yes. For the TPM, there's a defined ASN.1 format for the keys:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/jejb/openssl_tpm2_engine.git/tree/tpm2-asn.h
>
> and part of the design of the file is that it's distinguishable either
> in DER or PEM (by the guards) format so any crypto application can know
> it's dealing with a TPM key simply by inspecting the file. I think you
> need the same thing for CAAM and any other format.
>
> We're encouraging new ASN.1 formats to be of the form
>
> SEQUENCE {
>type OBJECT IDENTIFIER
>... key specific fields ...
> }
>
> Where you choose a defined OID to represent the key and that means
> every key even in DER form begins with a unique binary signature.
I like this idea.
Ahmad, what do you think?
That way we could also get rid off the kernel parameter and all the fall back
logic,
given that we find a way to reliable detect TEE blobs too...
Thanks,
//richard
Re: [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
James, - Ursprüngliche Mail - > Von: "James Bottomley" >> On Wed, Mar 17, 2021 at 3:08 PM Ahmad Fatoum > > wrote: >> > keyctl add trusted $KEYNAME "load $(cat ~/kmk.blob)" @s >> >> Is there a reason why we can't pass the desired backend name in the >> trusted key parameters? >> e.g. >> keyctl add trusted $KEYNAME "backendtype caam load $(cat ~/kmk.blob)" >> @s > > Why would you want to in the load? The blob should be type specific, > so a TPM key shouldn't load as a CAAM key and vice versa ... and if > they're not they need to be made so before the patches go upstream. I fear right now there is no good way to detect whether a blob is desired for CAAM or TPM. > I could possibly see that you might want to be type specific in the > create, but once you're simply loading an already created key, the > trusted key subsystem should be able to figure what to do on its own. So you have some kind of container format in mind which denotes the type of the blob? Thanks, //richard
Re: [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
Ahmad, On Tue, Mar 16, 2021 at 6:24 PM Ahmad Fatoum wrote: > +#define KEYMOD "kernel:trusted" why is the CAAM key modifier hard coded? I'd love to have way to pass my own modifier. That way existing blobs can also be used with this implementation. IIRC the NXP vendor tree uses "SECURE_KEY" as default modifier. -- Thanks, //richard
RE: Re: [PATCH] clk: imx8mp: Remove the none exist pcie clocks
> -Original Message- > From: Stephen Boyd > Sent: Wednesday, March 31, 2021 10:17 AM > To: Richard Zhu ; Abel Vesa ; > Jacky Bai ; shawn...@kernel.org > Cc: dl-linux-imx ; linux-arm-ker...@lists.infradead.org; > linux-kernel@vger.kernel.org; linux-...@vger.kernel.org; Richard Zhu > > Subject: Re: [PATCH] clk: imx8mp: Remove the none exist pcie clocks > Quoting Richard Zhu (2021-03-15 01:17:47) > > In the i.MX8MP PCIe design, the PCIe PHY REF clock comes from external > > OSC or internal system PLL. It is configured in the IOMUX_GPR14 > > register directly, and can't be contolled by CCM at all. > > Remove the PCIE PHY clock from clock driver to clean up codes. > > There is only one PCIe in i.MX8MP, remove the none exist second PCIe > > related clocks. > > Remove the none exsits clocks IDs together. > > > > Signed-off-by: Richard Zhu > > Reviewed-by: Jason Liu > > --- > > Any Fixes tag? [Richard Zhu] Hi Stephen: The codes are changed refer to the RM document updates. It doesn't fix a bug introduced from previous commit. Best Regards Richard Zhu
Re: [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
Ahmad, On Wed, Mar 17, 2021 at 3:08 PM Ahmad Fatoum wrote: > keyctl add trusted $KEYNAME "load $(cat ~/kmk.blob)" @s Is there a reason why we can't pass the desired backend name in the trusted key parameters? e.g. keyctl add trusted $KEYNAME "backendtype caam load $(cat ~/kmk.blob)" @s -- Thanks, //richard
Re: [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
Ahmad, On Wed, Mar 17, 2021 at 3:08 PM Ahmad Fatoum wrote: > TABLE="0 $BLOCKS crypt $ALGO :32:trusted:$KEYNAME 0 $DEV 0 1 > allow_discards" > echo $TABLE | dmsetup create mydev > echo $TABLE | dmsetup load mydev Do you also plan to add support for this to cryptsetup? David and I have added (rough) support for our CAAM/DCP based keyrings to cryptsetup: https://github.com/sigma-star/cryptsetup/tree/rw/plain I'm pretty sure with minimal changes it will work with your recent approach too. -- Thanks, //richard
Re: [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
Ahmad, On Wed, Mar 17, 2021 at 3:03 PM Ahmad Fatoum wrote: > > I didn't closely follow the previous discussions, but is a module > > parameter really the right approach? > > Is there also a way to set it via something like device tree? > > Compiled-on sources are considered in the order: tpm, tee then caam. > Module parameters are the only override currently available. Okay. So in the ideal case only one of these backends is compiled in, but the list can get long. I'm asking because David and I currently port another caam-like mechanism to the most recent kernel which will also hook in there. Out driver adds trusted keys support (with caam alike blobs) for i.mx SoCs that come with DCP instead of CAAM. Patches will hopefully materialize soon. -- Thanks, //richard
Re: [PATCH] firmware: stratix10-svc: extend SVC driver to get the firmware version
Hi Moritz,
On 3/30/21 11:15 AM, Moritz Fischer wrote:
Hi Richard,
On Tue, Mar 30, 2021 at 09:33:05AM -0500, richard.g...@linux.intel.com wrote:
From: Richard Gong
Extend Intel service layer driver to get the firmware version running at
FPGA device. Therefore FPGA manager driver, one of Intel service layer
driver's client, can decide whether to handle the newly added bitstream
authentication function based on the retrieved firmware version.
Signed-off-by: Richard Gong
Acked-by: Moritz Fischr
---
drivers/firmware/stratix10-svc.c| 12 ++--
include/linux/firmware/intel/stratix10-smc.h| 21 +++--
include/linux/firmware/intel/stratix10-svc-client.h | 4
3 files changed, 33 insertions(+), 4 deletions(-)
diff --git a/drivers/firmware/stratix10-svc.c b/drivers/firmware/stratix10-svc.c
index 3aa489d..1443bbd 100644
--- a/drivers/firmware/stratix10-svc.c
+++ b/drivers/firmware/stratix10-svc.c
@@ -306,6 +306,7 @@ static void svc_thread_recv_status_ok(struct
stratix10_svc_data *p_data,
break;
case COMMAND_RSU_RETRY:
case COMMAND_RSU_MAX_RETRY:
+ case COMMAND_FIRMWARE_VERSION:
cb_data->status = BIT(SVC_STATUS_OK);
cb_data->kaddr1 = &res.a1;
break;
@@ -422,6 +423,11 @@ static int svc_normal_to_secure_thread(void *data)
a1 = 0;
a2 = 0;
break;
+ case COMMAND_FIRMWARE_VERSION:
+ a0 = INTEL_SIP_SMC_FIRMWARE_VERSION;
+ a1 = 0;
+ a2 = 0;
+ break;
default:
pr_warn("it shouldn't happen\n");
break;
@@ -487,11 +493,13 @@ static int svc_normal_to_secure_thread(void *data)
/*
* be compatible with older version firmware which
-* doesn't support RSU notify or retry
+* doesn't support RSU notify, retry or bitstream
+* authentication.
*/
if ((pdata->command == COMMAND_RSU_RETRY) ||
(pdata->command == COMMAND_RSU_MAX_RETRY) ||
- (pdata->command == COMMAND_RSU_NOTIFY)) {
+ (pdata->command == COMMAND_RSU_NOTIFY) ||
+ (pdata->command == COMMAND_FIRMWARE_VERSION)) {
cbdata->status =
BIT(SVC_STATUS_NO_SUPPORT);
cbdata->kaddr1 = NULL;
diff --git a/include/linux/firmware/intel/stratix10-smc.h
b/include/linux/firmware/intel/stratix10-smc.h
index c3e5ab0..505fcca 100644
--- a/include/linux/firmware/intel/stratix10-smc.h
+++ b/include/linux/firmware/intel/stratix10-smc.h
@@ -321,8 +321,6 @@
INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_FPGA_CONFIG_COMPLETED_WRITE)
#define INTEL_SIP_SMC_ECC_DBE \
INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_ECC_DBE)
-#endif
-
/**
* Request INTEL_SIP_SMC_RSU_NOTIFY
*
@@ -404,3 +402,22 @@
INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_FPGA_CONFIG_COMPLETED_WRITE)
#define INTEL_SIP_SMC_FUNCID_RSU_MAX_RETRY 18
#define INTEL_SIP_SMC_RSU_MAX_RETRY \
INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_RSU_MAX_RETRY)
+
+/**
+ * Request INTEL_SIP_SMC_FIRMWARE_VERSION
+ *
+ * Sync call used to query the version of running firmware
+ *
+ * Call register usage:
+ * a0 INTEL_SIP_SMC_FIRMWARE_VERSION
+ * a1-a7 not used
+ *
+ * Return status:
+ * a0 INTEL_SIP_SMC_STATUS_OK or INTEL_SIP_SMC_STATUS_ERROR
+ * a1 running firmware version
+ */
+#define INTEL_SIP_SMC_FUNCID_FIRMWARE_VERSION 31
+#define INTEL_SIP_SMC_FIRMWARE_VERSION \
+ INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_FIRMWARE_VERSION)
+
+#endif
diff --git a/include/linux/firmware/intel/stratix10-svc-client.h
b/include/linux/firmware/intel/stratix10-svc-client.h
index 19781b0f..18c1841 100644
--- a/include/linux/firmware/intel/stratix10-svc-client.h
+++ b/include/linux/firmware/intel/stratix10-svc-client.h
@@ -104,6 +104,9 @@ struct stratix10_svc_chan;
*
* @COMMAND_RSU_DCMF_VERSION: query firmware for the DCMF version, return
status
* is SVC_STATUS_OK or SVC_STATUS_ERROR
+ *
+ * @COMMAND_FIRMWARE_VERSION: query running firmware version, return status
+ * is SVC_STATUS_OK or SVC_STATUS_ERROR
*/
enum stratix10_svc_command_code {
COMMAND_NOOP = 0,
@@ -117,6 +120,7 @@ enum stratix10_svc_command_code {
COMMAND_RSU_RETRY,
COMMAND_RSU_MAX_RETRY,
COMMAND_RSU_DCMF_VERSION,
+ COMMAND_FIRMWARE_VERSION,
};
/**
--
2.7.4
Let's hold off on this patch until we have sorted the rest of this patch
series out. As it stands it doesn't have a in-tree user.
OK,
Regards,
Richard
Thanks,
Moritz
Re: [PATCH] firmware: stratix10-svc: extend SVC driver to get the firmware version
Hi David, On 3/30/21 9:19 AM, David Laight wrote: From: richard.g...@linux.intel.com Sent: 30 March 2021 15:33 Extend Intel service layer driver to get the firmware version running at FPGA device. Therefore FPGA manager driver, one of Intel service layer driver's client, can decide whether to handle the newly added bitstream authentication function based on the retrieved firmware version. Using the version number to detect features is just plain wrong. You should use something like a bitmap of supported features. Firmware, which runs at EL3, returns the version to Intel service layer driver in a 64-bit value at the register. Intel service layer driver runs at EL1. Regards, Richard David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
[PATCH] firmware: stratix10-svc: extend SVC driver to get the firmware version
From: Richard Gong
Extend Intel service layer driver to get the firmware version running at
FPGA device. Therefore FPGA manager driver, one of Intel service layer
driver's client, can decide whether to handle the newly added bitstream
authentication function based on the retrieved firmware version.
Signed-off-by: Richard Gong
Acked-by: Moritz Fischr
---
drivers/firmware/stratix10-svc.c| 12 ++--
include/linux/firmware/intel/stratix10-smc.h| 21 +++--
include/linux/firmware/intel/stratix10-svc-client.h | 4
3 files changed, 33 insertions(+), 4 deletions(-)
diff --git a/drivers/firmware/stratix10-svc.c b/drivers/firmware/stratix10-svc.c
index 3aa489d..1443bbd 100644
--- a/drivers/firmware/stratix10-svc.c
+++ b/drivers/firmware/stratix10-svc.c
@@ -306,6 +306,7 @@ static void svc_thread_recv_status_ok(struct
stratix10_svc_data *p_data,
break;
case COMMAND_RSU_RETRY:
case COMMAND_RSU_MAX_RETRY:
+ case COMMAND_FIRMWARE_VERSION:
cb_data->status = BIT(SVC_STATUS_OK);
cb_data->kaddr1 = &res.a1;
break;
@@ -422,6 +423,11 @@ static int svc_normal_to_secure_thread(void *data)
a1 = 0;
a2 = 0;
break;
+ case COMMAND_FIRMWARE_VERSION:
+ a0 = INTEL_SIP_SMC_FIRMWARE_VERSION;
+ a1 = 0;
+ a2 = 0;
+ break;
default:
pr_warn("it shouldn't happen\n");
break;
@@ -487,11 +493,13 @@ static int svc_normal_to_secure_thread(void *data)
/*
* be compatible with older version firmware which
-* doesn't support RSU notify or retry
+* doesn't support RSU notify, retry or bitstream
+* authentication.
*/
if ((pdata->command == COMMAND_RSU_RETRY) ||
(pdata->command == COMMAND_RSU_MAX_RETRY) ||
- (pdata->command == COMMAND_RSU_NOTIFY)) {
+ (pdata->command == COMMAND_RSU_NOTIFY) ||
+ (pdata->command == COMMAND_FIRMWARE_VERSION)) {
cbdata->status =
BIT(SVC_STATUS_NO_SUPPORT);
cbdata->kaddr1 = NULL;
diff --git a/include/linux/firmware/intel/stratix10-smc.h
b/include/linux/firmware/intel/stratix10-smc.h
index c3e5ab0..505fcca 100644
--- a/include/linux/firmware/intel/stratix10-smc.h
+++ b/include/linux/firmware/intel/stratix10-smc.h
@@ -321,8 +321,6 @@
INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_FPGA_CONFIG_COMPLETED_WRITE)
#define INTEL_SIP_SMC_ECC_DBE \
INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_ECC_DBE)
-#endif
-
/**
* Request INTEL_SIP_SMC_RSU_NOTIFY
*
@@ -404,3 +402,22 @@
INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_FPGA_CONFIG_COMPLETED_WRITE)
#define INTEL_SIP_SMC_FUNCID_RSU_MAX_RETRY 18
#define INTEL_SIP_SMC_RSU_MAX_RETRY \
INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_RSU_MAX_RETRY)
+
+/**
+ * Request INTEL_SIP_SMC_FIRMWARE_VERSION
+ *
+ * Sync call used to query the version of running firmware
+ *
+ * Call register usage:
+ * a0 INTEL_SIP_SMC_FIRMWARE_VERSION
+ * a1-a7 not used
+ *
+ * Return status:
+ * a0 INTEL_SIP_SMC_STATUS_OK or INTEL_SIP_SMC_STATUS_ERROR
+ * a1 running firmware version
+ */
+#define INTEL_SIP_SMC_FUNCID_FIRMWARE_VERSION 31
+#define INTEL_SIP_SMC_FIRMWARE_VERSION \
+ INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_FIRMWARE_VERSION)
+
+#endif
diff --git a/include/linux/firmware/intel/stratix10-svc-client.h
b/include/linux/firmware/intel/stratix10-svc-client.h
index 19781b0f..18c1841 100644
--- a/include/linux/firmware/intel/stratix10-svc-client.h
+++ b/include/linux/firmware/intel/stratix10-svc-client.h
@@ -104,6 +104,9 @@ struct stratix10_svc_chan;
*
* @COMMAND_RSU_DCMF_VERSION: query firmware for the DCMF version, return
status
* is SVC_STATUS_OK or SVC_STATUS_ERROR
+ *
+ * @COMMAND_FIRMWARE_VERSION: query running firmware version, return status
+ * is SVC_STATUS_OK or SVC_STATUS_ERROR
*/
enum stratix10_svc_command_code {
COMMAND_NOOP = 0,
@@ -117,6 +120,7 @@ enum stratix10_svc_command_code {
COMMAND_RSU_RETRY,
COMMAND_RSU_MAX_RETRY,
COMMAND_RSU_DCMF_VERSION,
+ COMMAND_FIRMWARE_VERSION,
};
/**
--
2.7.4
[PATCH] A patch for Intel service layer driver
From: Richard Gong Hi Greg, Please take this stratix10-svc patch, which has been reviewed on the mailing list and applied cleanly on current linux-next and char-misc-testing. Thanks, Richard Richard Gong (1): firmware: stratix10-svc: extend SVC driver to get the firmware version drivers/firmware/stratix10-svc.c| 12 ++-- include/linux/firmware/intel/stratix10-smc.h| 21 +++-- include/linux/firmware/intel/stratix10-svc-client.h | 4 3 files changed, 33 insertions(+), 4 deletions(-) -- 2.7.4
[PATCH v8 1/4] lib: vsprintf: scanf: Negative number must have field width > 1
If a signed number field starts with a '-' the field width must be > 1,
or unlimited, to allow at least one digit after the '-'.
This patch adds a check for this. If a signed field starts with '-'
and field_width == 1 the scanf will quit.
It is ok for a signed number field to have a field width of 1 if it
starts with a digit. In that case the single digit can be converted.
Signed-off-by: Richard Fitzgerald
Reviewed-by: Petr Mladek
Acked-by: Andy Shevchenko
---
lib/vsprintf.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 41ddc353ebb8..f78651e9b030 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -3466,8 +3466,12 @@ int vsscanf(const char *buf, const char *fmt, va_list
args)
str = skip_spaces(str);
digit = *str;
- if (is_sign && digit == '-')
+ if (is_sign && digit == '-') {
+ if (field_width == 1)
+ break;
+
digit = *(str + 1);
+ }
if (!digit
|| (base == 16 && !isxdigit(digit))
--
2.20.1
[PATCH v8 2/4] lib: vsprintf: Fix handling of number field widths in vsscanf
The existing code attempted to handle numbers by doing a strto[u]l(),
ignoring the field width, and then repeatedly dividing to extract the
field out of the full converted value. If the string contains a run of
valid digits longer than will fit in a long or long long, this would
overflow and no amount of dividing can recover the correct value.
This patch fixes vsscanf() to obey number field widths when parsing
the number.
A new _parse_integer_limit() is added that takes a limit for the number
of characters to parse. The number field conversion in vsscanf is changed
to use this new function.
If a number starts with a radix prefix, the field width must be long
enough for at last one digit after the prefix. If not, it will be handled
like this:
sscanf("0x4", "%1i", &i): i=0, scanning continues with the 'x'
sscanf("0x4", "%2i", &i): i=0, scanning continues with the '4'
This is consistent with the observed behaviour of userland sscanf.
Note that this patch does NOT fix the problem of a single field value
overflowing the target type. So for example:
sscanf("123456789abcdef", "%x", &i);
Will not produce the correct result because the value obviously overflows
INT_MAX. But sscanf will report a successful conversion.
Note that where a very large number is used to mean "unlimited", the value
INT_MAX is used for consistency with the behaviour of vsnprintf().
Signed-off-by: Richard Fitzgerald
Reviewed-by: Petr Mladek
---
Changed since v6/v7:
Reverted unnecessary change in simple_strtoul(). The existing code that calls
simple_strtoull() is ok.
This is a trivial change so I've kept Petr's Reviewed-by.
---
lib/kstrtox.c | 13 ++--
lib/kstrtox.h | 2 ++
lib/vsprintf.c | 86 +-
3 files changed, 62 insertions(+), 39 deletions(-)
diff --git a/lib/kstrtox.c b/lib/kstrtox.c
index a118b0b1e9b2..0b5fe8b41173 100644
--- a/lib/kstrtox.c
+++ b/lib/kstrtox.c
@@ -39,20 +39,22 @@ const char *_parse_integer_fixup_radix(const char *s,
unsigned int *base)
/*
* Convert non-negative integer string representation in explicitly given radix
- * to an integer.
+ * to an integer. A maximum of max_chars characters will be converted.
+ *
* Return number of characters consumed maybe or-ed with overflow bit.
* If overflow occurs, result integer (incorrect) is still returned.
*
* Don't you dare use this function.
*/
-unsigned int _parse_integer(const char *s, unsigned int base, unsigned long
long *p)
+unsigned int _parse_integer_limit(const char *s, unsigned int base, unsigned
long long *p,
+ size_t max_chars)
{
unsigned long long res;
unsigned int rv;
res = 0;
rv = 0;
- while (1) {
+ while (max_chars--) {
unsigned int c = *s;
unsigned int lc = c | 0x20; /* don't tolower() this line */
unsigned int val;
@@ -82,6 +84,11 @@ unsigned int _parse_integer(const char *s, unsigned int
base, unsigned long long
return rv;
}
+unsigned int _parse_integer(const char *s, unsigned int base, unsigned long
long *p)
+{
+ return _parse_integer_limit(s, base, p, INT_MAX);
+}
+
static int _kstrtoull(const char *s, unsigned int base, unsigned long long
*res)
{
unsigned long long _res;
diff --git a/lib/kstrtox.h b/lib/kstrtox.h
index 3b4637bcd254..158c400ca865 100644
--- a/lib/kstrtox.h
+++ b/lib/kstrtox.h
@@ -4,6 +4,8 @@
#define KSTRTOX_OVERFLOW (1U << 31)
const char *_parse_integer_fixup_radix(const char *s, unsigned int *base);
+unsigned int _parse_integer_limit(const char *s, unsigned int base, unsigned
long long *res,
+ size_t max_chars);
unsigned int _parse_integer(const char *s, unsigned int base, unsigned long
long *res);
#endif
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index f78651e9b030..e3b318c06623 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -53,29 +53,43 @@
#include
#include "kstrtox.h"
-/**
- * simple_strtoull - convert a string to an unsigned long long
- * @cp: The start of the string
- * @endp: A pointer to the end of the parsed string will be placed here
- * @base: The number base to use
- *
- * This function has caveats. Please use kstrtoull instead.
- */
-unsigned long long simple_strtoull(const char *cp, char **endp, unsigned int
base)
+static unsigned long long simple_strntoull(const char *startp, size_t
max_chars,
+ char **endp, unsigned int base)
{
- unsigned long long result;
+ const char *cp;
+ unsigned long long result = 0ULL;
+ size_t prefix_chars;
unsigned int rv;
- cp = _parse_integer_fixup_radix(cp, &base);
- rv = _parse_integer(cp, base, &result);
- /* FIXME */
- cp += (rv &
[PATCH v8 4/4] selftests: lib: Add wrapper script for test_scanf
Adds a wrapper shell script for the test_scanf module. Signed-off-by: Richard Fitzgerald Reviewed-by: Petr Mladek Acked-by: Andy Shevchenko --- Changed since v6: Fixed typo in tools/testing/selftests/lib/config: -CONFIG_TEST_SCANTF=m +CONFIG_TEST_SCANF=m As this is a trivial change I have kept Petr Mladek's Reviewed-by and Andy Shevchenko's ack. --- tools/testing/selftests/lib/Makefile | 2 +- tools/testing/selftests/lib/config | 1 + tools/testing/selftests/lib/scanf.sh | 4 3 files changed, 6 insertions(+), 1 deletion(-) create mode 100755 tools/testing/selftests/lib/scanf.sh diff --git a/tools/testing/selftests/lib/Makefile b/tools/testing/selftests/lib/Makefile index a105f094676e..ee71fc99d5b5 100644 --- a/tools/testing/selftests/lib/Makefile +++ b/tools/testing/selftests/lib/Makefile @@ -4,6 +4,6 @@ # No binaries, but make sure arg-less "make" doesn't trigger "run_tests" all: -TEST_PROGS := printf.sh bitmap.sh prime_numbers.sh strscpy.sh +TEST_PROGS := printf.sh bitmap.sh prime_numbers.sh scanf.sh strscpy.sh include ../lib.mk diff --git a/tools/testing/selftests/lib/config b/tools/testing/selftests/lib/config index b80ee3f6e265..645839b50b0a 100644 --- a/tools/testing/selftests/lib/config +++ b/tools/testing/selftests/lib/config @@ -1,4 +1,5 @@ CONFIG_TEST_PRINTF=m +CONFIG_TEST_SCANF=m CONFIG_TEST_BITMAP=m CONFIG_PRIME_NUMBERS=m CONFIG_TEST_STRSCPY=m diff --git a/tools/testing/selftests/lib/scanf.sh b/tools/testing/selftests/lib/scanf.sh new file mode 100755 index ..b59b8ba561c3 --- /dev/null +++ b/tools/testing/selftests/lib/scanf.sh @@ -0,0 +1,4 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0 +# Tests the scanf infrastructure using test_scanf kernel module. +$(dirname $0)/../kselftest/module.sh "scanf" test_scanf -- 2.20.1
[PATCH v8 3/4] lib: test_scanf: Add tests for sscanf number conversion
Adds test_sscanf to test various number conversion cases, as
number conversion was previously broken.
This also tests the simple_strtoxxx() functions exported from
vsprintf.c.
Signed-off-by: Richard Fitzgerald
Acked-by: Andy Shevchenko
---
Changed since v6:
Use the KSTM_MODULE_GLOBALS define:
-static unsigned total_tests __initdata;
-static unsigned failed_tests __initdata;
+KSTM_MODULE_GLOBALS();
As this is a trivial change I have kept Andy Shevchenko's ack.
---
MAINTAINERS | 1 +
lib/Kconfig.debug | 3 +
lib/Makefile | 1 +
lib/test_scanf.c | 751 ++
4 files changed, 756 insertions(+)
create mode 100644 lib/test_scanf.c
diff --git a/MAINTAINERS b/MAINTAINERS
index fb2a3633b719..712812b2142b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19225,6 +19225,7 @@ S: Maintained
T: git git://git.kernel.org/pub/scm/linux/kernel/git/pmladek/printk.git
F: Documentation/core-api/printk-formats.rst
F: lib/test_printf.c
+F: lib/test_scanf.c
F: lib/vsprintf.c
VT1211 HARDWARE MONITOR DRIVER
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 2779c29d9981..4cfaf2a817f9 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2123,6 +2123,9 @@ config TEST_KSTRTOX
config TEST_PRINTF
tristate "Test printf() family of functions at runtime"
+config TEST_SCANF
+ tristate "Test scanf() family of functions at runtime"
+
config TEST_BITMAP
tristate "Test bitmap_*() family of functions at runtime"
help
diff --git a/lib/Makefile b/lib/Makefile
index b5307d3eec1a..5433d3f43361 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -83,6 +83,7 @@ obj-$(CONFIG_TEST_USER_COPY) += test_user_copy.o
obj-$(CONFIG_TEST_STATIC_KEYS) += test_static_keys.o
obj-$(CONFIG_TEST_STATIC_KEYS) += test_static_key_base.o
obj-$(CONFIG_TEST_PRINTF) += test_printf.o
+obj-$(CONFIG_TEST_SCANF) += test_scanf.o
obj-$(CONFIG_TEST_BITMAP) += test_bitmap.o
obj-$(CONFIG_TEST_STRSCPY) += test_strscpy.o
obj-$(CONFIG_TEST_UUID) += test_uuid.o
diff --git a/lib/test_scanf.c b/lib/test_scanf.c
new file mode 100644
index ..8d577aec6c28
--- /dev/null
+++ b/lib/test_scanf.c
@@ -0,0 +1,751 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Test cases for sscanf facility.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "../tools/testing/selftests/kselftest_module.h"
+
+#define BUF_SIZE 1024
+
+KSTM_MODULE_GLOBALS();
+static char *test_buffer __initdata;
+static char *fmt_buffer __initdata;
+static struct rnd_state rnd_state __initdata;
+
+typedef int (*check_fn)(const void *check_data, const char *string,
+ const char *fmt, int n_args, va_list ap);
+
+static void __scanf(4, 6) __init
+_test(check_fn fn, const void *check_data, const char *string, const char *fmt,
+ int n_args, ...)
+{
+ va_list ap, ap_copy;
+ int ret;
+
+ total_tests++;
+
+ va_start(ap, n_args);
+ va_copy(ap_copy, ap);
+ ret = vsscanf(string, fmt, ap_copy);
+ va_end(ap_copy);
+
+ if (ret != n_args) {
+ pr_warn("vsscanf(\"%s\", \"%s\", ...) returned %d expected
%d\n",
+ string, fmt, ret, n_args);
+ goto fail;
+ }
+
+ ret = (*fn)(check_data, string, fmt, n_args, ap);
+ if (ret)
+ goto fail;
+
+ va_end(ap);
+
+ return;
+
+fail:
+ failed_tests++;
+ va_end(ap);
+}
+
+#define _check_numbers_template(arg_fmt, expect, str, fmt, n_args, ap)
\
+do {
\
+ pr_debug("\"%s\", \"%s\" ->\n", str, fmt);
\
+ for (; n_args > 0; n_args--, expect++) {
\
+ typeof(*expect) got = *va_arg(ap, typeof(expect));
\
+ pr_debug("\t" arg_fmt "\n", got);
\
+ if (got != *expect) {
\
+ pr_warn("vsscanf(\"%s\", \"%s\", ...) expected "
arg_fmt " got " arg_fmt "\n", \
+ str, fmt, *expect, got);
\
+ return 1;
\
+ }
\
+ }
\
+ return 0;
\
+} while (0)
+
+static int __init check_ull(const void *check_data, const char *string,
+
[RESEND v4 2/2] PCI: imx: clear vreg bypass when pcie vph voltage is 3v3
Both 1.8v and 3.3v power supplies can be used by i.MX8MQ PCIe PHY.
In default, the PCIE_VPH voltage is suggested to be 1.8v refer to data
sheet. When PCIE_VPH is supplied by 3.3v in the HW schematic design,
the VREG_BYPASS bits of GPR registers should be cleared from default
value 1b'1 to 1b'0. Thus, the internal 3v3 to 1v8 translator would be
turned on.
Signed-off-by: Richard Zhu
Reviewed-by: Lucas Stach
---
drivers/pci/controller/dwc/pci-imx6.c | 20
1 file changed, 20 insertions(+)
diff --git a/drivers/pci/controller/dwc/pci-imx6.c
b/drivers/pci/controller/dwc/pci-imx6.c
index 853ea8e82952..94b43b4ecca1 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -37,6 +37,7 @@
#define IMX8MQ_GPR_PCIE_REF_USE_PADBIT(9)
#define IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_ENBIT(10)
#define IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE BIT(11)
+#define IMX8MQ_GPR_PCIE_VREG_BYPASSBIT(12)
#define IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPEGENMASK(11, 8)
#define IMX8MQ_PCIE2_BASE_ADDR 0x33c0
@@ -80,6 +81,7 @@ struct imx6_pcie {
u32 tx_swing_full;
u32 tx_swing_low;
struct regulator*vpcie;
+ struct regulator*vph;
void __iomem*phy_base;
/* power domain for pcie */
@@ -621,6 +623,17 @@ static void imx6_pcie_init_phy(struct imx6_pcie *imx6_pcie)
imx6_pcie_grp_offset(imx6_pcie),
IMX8MQ_GPR_PCIE_REF_USE_PAD,
IMX8MQ_GPR_PCIE_REF_USE_PAD);
+ /*
+* Regarding the datasheet, the PCIE_VPH is suggested
+* to be 1.8V. If the PCIE_VPH is supplied by 3.3V, the
+* VREG_BYPASS should be cleared to zero.
+*/
+ if (imx6_pcie->vph &&
+ regulator_get_voltage(imx6_pcie->vph) > 300)
+ regmap_update_bits(imx6_pcie->iomuxc_gpr,
+ imx6_pcie_grp_offset(imx6_pcie),
+ IMX8MQ_GPR_PCIE_VREG_BYPASS,
+ 0);
break;
case IMX7D:
regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
@@ -1130,6 +1143,13 @@ static int imx6_pcie_probe(struct platform_device *pdev)
imx6_pcie->vpcie = NULL;
}
+ imx6_pcie->vph = devm_regulator_get_optional(&pdev->dev, "vph");
+ if (IS_ERR(imx6_pcie->vph)) {
+ if (PTR_ERR(imx6_pcie->vph) != -ENODEV)
+ return PTR_ERR(imx6_pcie->vph);
+ imx6_pcie->vph = NULL;
+ }
+
platform_set_drvdata(pdev, imx6_pcie);
ret = imx6_pcie_attach_pd(dev);
--
2.17.1
[RESEND v4 0/2] add one regulator used to power up pcie phy
Changes: Only add "Reviewed-by: Lucas Stach " in the first patch. No other changes. Make it easier to be integrated later. [RESEND v4 1/2] dt-bindings: imx6q-pcie: add one regulator used to [RESEND v4 2/2] PCI: imx: clear vreg bypass when pcie vph voltage is
[RESEND v4 1/2] dt-bindings: imx6q-pcie: add one regulator used to power up pcie phy
Both 1.8v and 3.3v power supplies can be used by i.MX8MQ PCIe PHY. In default, the PCIE_VPH voltage is suggested to be 1.8v refer to data sheet. When PCIE_VPH is supplied by 3.3v in the HW schematic design, the VREG_BYPASS bits of GPR registers should be cleared from default value 1b'1 to 1b'0. Thus, the internal 3v3 to 1v8 translator would be turned on. Signed-off-by: Richard Zhu Reviewed-by: Lucas Stach --- Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt index de4b2baf91e8..d8971ab99274 100644 --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt @@ -38,6 +38,9 @@ Optional properties: The regulator will be enabled when initializing the PCIe host and disabled either as part of the init process or when shutting down the host. +- vph-supply: Should specify the regulator in charge of VPH one of the three + PCIe PHY powers. This regulator can be supplied by both 1.8v and 3.3v voltage + supplies. Additional required properties for imx6sx-pcie: - clock names: Must include the following additional entries: -- 2.17.1
Re: [PATCH v7 2/4] lib: vsprintf: Fix handling of number field widths in vsscanf
On 29/03/2021 14:36, Andy Shevchenko wrote:
On Mon, Mar 29, 2021 at 01:08:22PM +0100, Richard Fitzgerald wrote:
The existing code attempted to handle numbers by doing a strto[u]l(),
ignoring the field width, and then repeatedly dividing to extract the
field out of the full converted value. If the string contains a run of
valid digits longer than will fit in a long or long long, this would
overflow and no amount of dividing can recover the correct value.
This patch fixes vsscanf() to obey number field widths when parsing
the number.
A new _parse_integer_limit() is added that takes a limit for the number
of characters to parse. The number field conversion in vsscanf is changed
to use this new function.
If a number starts with a radix prefix, the field width must be long
enough for at last one digit after the prefix. If not, it will be handled
like this:
sscanf("0x4", "%1i", &i): i=0, scanning continues with the 'x'
sscanf("0x4", "%2i", &i): i=0, scanning continues with the '4'
This is consistent with the observed behaviour of userland sscanf.
Note that this patch does NOT fix the problem of a single field value
overflowing the target type. So for example:
sscanf("123456789abcdef", "%x", &i);
Will not produce the correct result because the value obviously overflows
INT_MAX. But sscanf will report a successful conversion.
Note that where a very large number is used to mean "unlimited", the value
INT_MAX is used for consistency with the behaviour of vsnprintf().
...
unsigned long simple_strtoul(const char *cp, char **endp, unsigned int base)
{
- return simple_strtoull(cp, endp, base);
+ return simple_strntoull(cp, INT_MAX, endp, base);
Why do you need this change?
I agree it's not necessary. I changed it between V1 and V2 but I can't
remember what the reason was.
Re: [Bug 212265] New: clock_gettime(CLOCK_TAI, ...) should return an error when TAI has not been configured
On Mon, Mar 29, 2021 at 04:57:55PM +0200, Thomas Gleixner wrote:
> I think adjtimex is the right place and not yet another random file
> somewhere. Something like the below.
Perfect.
Acked-by: Richard Cochran
> ---
> include/uapi/linux/timex.h |7 +--
> kernel/time/ntp.c |4 +++-
> 2 files changed, 8 insertions(+), 3 deletions(-)
>
> --- a/include/uapi/linux/timex.h
> +++ b/include/uapi/linux/timex.h
> @@ -188,9 +188,12 @@ struct __kernel_timex {
> #define STA_MODE 0x4000 /* mode (0 = PLL, 1 = FLL) (ro) */
> #define STA_CLK 0x8000 /* clock source (0 = A, 1 = B) (ro) */
>
> +#define STA_TAISET 0x1 /* TAI offset was set via adjtimex (ro) */
> +
> /* read-only bits */
> -#define STA_RONLY (STA_PPSSIGNAL | STA_PPSJITTER | STA_PPSWANDER | \
> - STA_PPSERROR | STA_CLOCKERR | STA_NANO | STA_MODE | STA_CLK)
> +#define STA_RONLY (STA_PPSSIGNAL | STA_PPSJITTER | STA_PPSWANDER | \
> +STA_PPSERROR | STA_CLOCKERR | STA_NANO | STA_MODE | \
> +STA_CLK | STA_TAISET)
>
> /*
> * Clock states (time_state)
> --- a/kernel/time/ntp.c
> +++ b/kernel/time/ntp.c
> @@ -741,8 +741,10 @@ static inline void process_adjtimex_mode
> }
>
> if (txc->modes & ADJ_TAI &&
> - txc->constant >= 0 && txc->constant <= MAX_TAI_OFFSET)
> + txc->constant >= 0 && txc->constant <= MAX_TAI_OFFSET) {
> *time_tai = txc->constant;
> + time_status |= STA_TAISET;
> + }
>
> if (txc->modes & ADJ_OFFSET)
> ntp_update_offset(txc->offset);
Re: [Bug 212265] New: clock_gettime(CLOCK_TAI, ...) should return an error when TAI has not been configured
On Mon, Mar 29, 2021 at 11:16:48AM +0200, Miroslav Lichvar wrote: > There are at least two issues with handling a zero offset as a special > value. One is that zero could potentially be a valid value in distant > future. I not losing sleep over that, but > The other is that the kernel updates the offset when a leap > second is inserted/deleted even if the original offset is zero, so > checking for zero (in the kernel or an application) works only until > the first leap second after boot. oh, I didn't think of that. I hate leap seconds. Good thing Earth is picking up the pace again! > The kernel would need to set a flag that the offset was set. Returning > an error in clock_gettime() until the offset is set sounds reasonable > to me, but I have no idea how many of the existing applications it > would break. I think it wiser to provide another way, sysfs or something else. Thanks, Richard
Re: [Bug 212265] New: clock_gettime(CLOCK_TAI, ...) should return an error when TAI has not been configured
On Mon, Mar 29, 2021 at 11:16:48AM +0200, Miroslav Lichvar wrote: > On Fri, Mar 26, 2021 at 08:28:59PM -0700, Richard Cochran wrote: > > Using ntpd on Debian, the service will set the offset, but only after > > synchronization with the upstream server has been established, and > > this takes about five minutes, IIRC. > > With the iburst option it shouldn't take more than 10 seconds. There > might be an issue wrt stepping the clock when the initial offset is > large. Really? Debian has # pool.ntp.org maps to about 1000 low-stratum NTP servers. Your server will # pick a different set every time it starts up. Please consider joining the # pool: <http://www.pool.ntp.org/join.html> pool 0.debian.pool.ntp.org iburst pool 1.debian.pool.ntp.org iburst pool 2.debian.pool.ntp.org iburst pool 3.debian.pool.ntp.org iburst I guess I'll measure again, but I'm pretty sure it took a long time to get to TAI being set. > In Fedora and derived distros using chrony by default the > TAI-UTC offset should be set right on the first update of the clock as > expected. (Maybe it is time to switch to chrony ;) Thanks, Richard
Re: [Bug 212265] New: clock_gettime(CLOCK_TAI, ...) should return an error when TAI has not been configured
On Mon, Mar 29, 2021 at 11:56:31AM +0200, Daphne Preston-Kendal wrote: > > The other is that the kernel updates the offset when a leap > > second is inserted/deleted even if the original offset is zero, so > > checking for zero (in the kernel or an application) works only until > > the first leap second after boot. > > This is a problem and definitely speaks for having a way to tell whether > CLOCK_TAI has been set up at all. +1 Thanks, Richard
Re: perf does not resolve plt symbols from libstdc++ right (.plt.sec problem)
On Mon, 29 Mar 2021, H.J. Lu wrote: > On Mon, Mar 29, 2021 at 2:38 AM Richard Biener wrote: > > > > On Mon, 29 Mar 2021, Jiri Slaby wrote: > > > > > Any ideas on this? > > > > > > On 11. 01. 21, 7:31, Jiri Slaby wrote: > > > > Hi, > > > > > > > > this e-mails is a follow-up of my report at: > > > > https://bugzilla.suse.com/show_bug.cgi?id=1180681 > > > > > > > > There is a problem with *@plt symbols in some libraries, they are > > > > unresolved > > > > by perf (memcmp@plt in this case): > > > > > 0.26% main2/usr/lib64/libstdc++.so.6.0.280xa51a0 > > > > l [.] 0x000a51a0 > > > > > > > > On the other hand, plt symbols in other libraries are fine (memset@plt > > > > in > > > > this case): > > > > > 0.17% main2/usr/lib64/libantlr4-runtime.so.4.8 0x4ed10 > > > > l [.] memset@plt > > > > > > > > I dumped memcmp's .plt.rela entries in perf: > > > > /usr/lib64/libantlr4-runtime.so.4.8: 154th addr=4e9d0 plt_off=4e020 > > > > hdr=10 > > > > entry=10 > > > > /usr/lib64/libstdc++.so.6.0.28: 772th addr=a1070 plt_off=9e020 hdr=10 > > > > entry=10 > > > > > > > > The difference (offset) of stdc++'s memcmp is 0xa51a0 (correct) - > > > > 0xa1070 > > > > (perf's computed) = 0x4130. > > > > > > > > The problem is perf assumes nth entry of .plt.rela to correspond to nth > > > > function in .plt, but memcmp is in .plt.sec in libstdc++.so: > > > > > > > > >Relocation section '.rela.plt' at offset 0x97900 contains 1018 > > > > entries: > > > > > Offset Info Type Symbol's > > > > Value Symbol's Name + Addend > > > > > ... > > > > > 001dc838 00780007 R_X86_64_JUMP_SLOT > > > > memcmp@GLIBC_2.2.5 + 0 > > > > > > > > Perf does this with the rela entries: > > > > https://github.com/torvalds/linux/blob/f5e6c330254ae691f6d7befe61c786eb5056007e/tools/perf/util/symbol-elf.c#L385 > > > > > > > > It takes a symbol index from sym.r_info. Then it resolves its name from > > > > .dynsym, appending "@plt" to it. Then this name is added to perf's > > > > symbol > > > > table along with address which is computed as .rela.plt index > > > > multiplied by > > > > entry size (shdr_plt.sh_entsize) plus plt header (shdr_plt.sh_entsize on > > > > x86_64 too). > > > > > > > > And from this comes (almost) the offset above: > > > > >$ objdump -h /usr/lib64/libstdc++.so.6|grep -E ' .plt(\.sec)? ' > > > > > 12 .plt 3fb0 0009e020 0009e020 > > > > 0009e020 2**4 > > > > > 14 .plt.sec 3fa0 000a2160 000a2160 > > > > 000a2160 2**4 > > > > > > > > 0xa2160-0x9e020 = 0x4140. I assume the 0x10 difference is that perf adds > > > > shdr_plt.sh_entsize (0x10) to the offset to skip the first .plt entry > > > > (header). > > > > > > > > Richard writes: > > > > == > > > > .plt.sec is IIRC the "second" (sec) PLT entry - the one that will be > > > > used on > > > > the second call (and on). This is used / emitted for ELF object > > > > instrumented for Intel CET. The details escape me for the moment but I > > > > hope > > > > the x86 ABI documents this (and the constraints) in detail. > > > > I just checked and the x86_64 psABI doesn't say anything about .plt.sec > > The second PLT is documented in section 13.2 Dynamic Linking in x86-64 > psABI. Please see elf_x86_64_get_synthetic_symtab in binutils for PLT symbol > processing. Hmm, google pointed me to https://gitlab.com/x86-psABIs/ and that version does not have a section 13 (but the last is section 12 on MPX). There's also references to a pdf which contain the section but that's on github and the github page says gitlab is the home... So I'm a bit confused here. Richard. -- Richard Biener SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
[PATCH v7 4/4] selftests: lib: Add wrapper script for test_scanf
Adds a wrapper shell script for the test_scanf module. Signed-off-by: Richard Fitzgerald Reviewed-by: Petr Mladek Acked-by: Andy Shevchenko --- Changes since v6: Fixed typo in tools/testing/selftests/lib/config: -CONFIG_TEST_SCANTF=m +CONFIG_TEST_SCANF=m As this is a trivial change I have kept Petr Mladek's Reviewed-by and Andy Shevchenko's ack. --- tools/testing/selftests/lib/Makefile | 2 +- tools/testing/selftests/lib/config | 1 + tools/testing/selftests/lib/scanf.sh | 4 3 files changed, 6 insertions(+), 1 deletion(-) create mode 100755 tools/testing/selftests/lib/scanf.sh diff --git a/tools/testing/selftests/lib/Makefile b/tools/testing/selftests/lib/Makefile index a105f094676e..ee71fc99d5b5 100644 --- a/tools/testing/selftests/lib/Makefile +++ b/tools/testing/selftests/lib/Makefile @@ -4,6 +4,6 @@ # No binaries, but make sure arg-less "make" doesn't trigger "run_tests" all: -TEST_PROGS := printf.sh bitmap.sh prime_numbers.sh strscpy.sh +TEST_PROGS := printf.sh bitmap.sh prime_numbers.sh scanf.sh strscpy.sh include ../lib.mk diff --git a/tools/testing/selftests/lib/config b/tools/testing/selftests/lib/config index b80ee3f6e265..776c8c42e78d 100644 --- a/tools/testing/selftests/lib/config +++ b/tools/testing/selftests/lib/config @@ -1,4 +1,5 @@ CONFIG_TEST_PRINTF=m +CONFIG_TEST_SCANF=m CONFIG_TEST_BITMAP=m CONFIG_PRIME_NUMBERS=m CONFIG_TEST_STRSCPY=m diff --git a/tools/testing/selftests/lib/scanf.sh b/tools/testing/selftests/lib/scanf.sh new file mode 100755 index ..b59b8ba561c3 --- /dev/null +++ b/tools/testing/selftests/lib/scanf.sh @@ -0,0 +1,4 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0 +# Tests the scanf infrastructure using test_scanf kernel module. +$(dirname $0)/../kselftest/module.sh "scanf" test_scanf -- 2.20.1
[PATCH RESEND] lib: crc8: Pointer to data block should be const
crc8() does not change the data passed to it, so the pointer argument
should be declared const. This avoids callers that receive const data
having to cast it to a non-const pointer to call crc8().
Signed-off-by: Richard Fitzgerald
Acked-by: Randy Dunlap
---
No maintainer listed for this file so including Andrew Morton on the
'to' list.
---
include/linux/crc8.h | 2 +-
lib/crc8.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/crc8.h b/include/linux/crc8.h
index 13c8dabb0441..674045c59a04 100644
--- a/include/linux/crc8.h
+++ b/include/linux/crc8.h
@@ -96,6 +96,6 @@ void crc8_populate_msb(u8 table[CRC8_TABLE_SIZE], u8
polynomial);
* Williams, Ross N., rossross.net
* (see URL http://www.ross.net/crc/download/crc_v3.txt).
*/
-u8 crc8(const u8 table[CRC8_TABLE_SIZE], u8 *pdata, size_t nbytes, u8 crc);
+u8 crc8(const u8 table[CRC8_TABLE_SIZE], const u8 *pdata, size_t nbytes, u8
crc);
#endif /* __CRC8_H_ */
diff --git a/lib/crc8.c b/lib/crc8.c
index 595a5a75e3cd..1ad8e501d9b6 100644
--- a/lib/crc8.c
+++ b/lib/crc8.c
@@ -71,7 +71,7 @@ EXPORT_SYMBOL(crc8_populate_lsb);
* @nbytes: number of bytes in data buffer.
* @crc: previous returned crc8 value.
*/
-u8 crc8(const u8 table[CRC8_TABLE_SIZE], u8 *pdata, size_t nbytes, u8 crc)
+u8 crc8(const u8 table[CRC8_TABLE_SIZE], const u8 *pdata, size_t nbytes, u8
crc)
{
/* loop over the buffer data */
while (nbytes-- > 0)
--
2.20.1
[PATCH v7 1/4] lib: vsprintf: scanf: Negative number must have field width > 1
If a signed number field starts with a '-' the field width must be > 1,
or unlimited, to allow at least one digit after the '-'.
This patch adds a check for this. If a signed field starts with '-'
and field_width == 1 the scanf will quit.
It is ok for a signed number field to have a field width of 1 if it
starts with a digit. In that case the single digit can be converted.
Signed-off-by: Richard Fitzgerald
Reviewed-by: Petr Mladek
Acked-by: Andy Shevchenko
---
lib/vsprintf.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 41ddc353ebb8..f78651e9b030 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -3466,8 +3466,12 @@ int vsscanf(const char *buf, const char *fmt, va_list
args)
str = skip_spaces(str);
digit = *str;
- if (is_sign && digit == '-')
+ if (is_sign && digit == '-') {
+ if (field_width == 1)
+ break;
+
digit = *(str + 1);
+ }
if (!digit
|| (base == 16 && !isxdigit(digit))
--
2.20.1
[PATCH v7 3/4] lib: test_scanf: Add tests for sscanf number conversion
Adds test_sscanf to test various number conversion cases, as
number conversion was previously broken.
This also tests the simple_strtoxxx() functions exported from
vsprintf.c.
Signed-off-by: Richard Fitzgerald
Acked-by: Andy Shevchenko
---
Changed since v6:
Use the KSTM_MODULE_GLOBALS define:
-static unsigned total_tests __initdata;
-static unsigned failed_tests __initdata;
+KSTM_MODULE_GLOBALS();
As this is a trivial change I have kept Andy Shevchenko's ack.
---
MAINTAINERS | 1 +
lib/Kconfig.debug | 3 +
lib/Makefile | 1 +
lib/test_scanf.c | 751 ++
4 files changed, 756 insertions(+)
create mode 100644 lib/test_scanf.c
diff --git a/MAINTAINERS b/MAINTAINERS
index fb2a3633b719..712812b2142b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19225,6 +19225,7 @@ S: Maintained
T: git git://git.kernel.org/pub/scm/linux/kernel/git/pmladek/printk.git
F: Documentation/core-api/printk-formats.rst
F: lib/test_printf.c
+F: lib/test_scanf.c
F: lib/vsprintf.c
VT1211 HARDWARE MONITOR DRIVER
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 2779c29d9981..4cfaf2a817f9 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2123,6 +2123,9 @@ config TEST_KSTRTOX
config TEST_PRINTF
tristate "Test printf() family of functions at runtime"
+config TEST_SCANF
+ tristate "Test scanf() family of functions at runtime"
+
config TEST_BITMAP
tristate "Test bitmap_*() family of functions at runtime"
help
diff --git a/lib/Makefile b/lib/Makefile
index b5307d3eec1a..5433d3f43361 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -83,6 +83,7 @@ obj-$(CONFIG_TEST_USER_COPY) += test_user_copy.o
obj-$(CONFIG_TEST_STATIC_KEYS) += test_static_keys.o
obj-$(CONFIG_TEST_STATIC_KEYS) += test_static_key_base.o
obj-$(CONFIG_TEST_PRINTF) += test_printf.o
+obj-$(CONFIG_TEST_SCANF) += test_scanf.o
obj-$(CONFIG_TEST_BITMAP) += test_bitmap.o
obj-$(CONFIG_TEST_STRSCPY) += test_strscpy.o
obj-$(CONFIG_TEST_UUID) += test_uuid.o
diff --git a/lib/test_scanf.c b/lib/test_scanf.c
new file mode 100644
index ..8d577aec6c28
--- /dev/null
+++ b/lib/test_scanf.c
@@ -0,0 +1,751 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Test cases for sscanf facility.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "../tools/testing/selftests/kselftest_module.h"
+
+#define BUF_SIZE 1024
+
+KSTM_MODULE_GLOBALS();
+static char *test_buffer __initdata;
+static char *fmt_buffer __initdata;
+static struct rnd_state rnd_state __initdata;
+
+typedef int (*check_fn)(const void *check_data, const char *string,
+ const char *fmt, int n_args, va_list ap);
+
+static void __scanf(4, 6) __init
+_test(check_fn fn, const void *check_data, const char *string, const char *fmt,
+ int n_args, ...)
+{
+ va_list ap, ap_copy;
+ int ret;
+
+ total_tests++;
+
+ va_start(ap, n_args);
+ va_copy(ap_copy, ap);
+ ret = vsscanf(string, fmt, ap_copy);
+ va_end(ap_copy);
+
+ if (ret != n_args) {
+ pr_warn("vsscanf(\"%s\", \"%s\", ...) returned %d expected
%d\n",
+ string, fmt, ret, n_args);
+ goto fail;
+ }
+
+ ret = (*fn)(check_data, string, fmt, n_args, ap);
+ if (ret)
+ goto fail;
+
+ va_end(ap);
+
+ return;
+
+fail:
+ failed_tests++;
+ va_end(ap);
+}
+
+#define _check_numbers_template(arg_fmt, expect, str, fmt, n_args, ap)
\
+do {
\
+ pr_debug("\"%s\", \"%s\" ->\n", str, fmt);
\
+ for (; n_args > 0; n_args--, expect++) {
\
+ typeof(*expect) got = *va_arg(ap, typeof(expect));
\
+ pr_debug("\t" arg_fmt "\n", got);
\
+ if (got != *expect) {
\
+ pr_warn("vsscanf(\"%s\", \"%s\", ...) expected "
arg_fmt " got " arg_fmt "\n", \
+ str, fmt, *expect, got);
\
+ return 1;
\
+ }
\
+ }
\
+ return 0;
\
+} while (0)
+
+static int __init check_ull(const void *check_data, const char *string,
+
[PATCH v7 2/4] lib: vsprintf: Fix handling of number field widths in vsscanf
The existing code attempted to handle numbers by doing a strto[u]l(),
ignoring the field width, and then repeatedly dividing to extract the
field out of the full converted value. If the string contains a run of
valid digits longer than will fit in a long or long long, this would
overflow and no amount of dividing can recover the correct value.
This patch fixes vsscanf() to obey number field widths when parsing
the number.
A new _parse_integer_limit() is added that takes a limit for the number
of characters to parse. The number field conversion in vsscanf is changed
to use this new function.
If a number starts with a radix prefix, the field width must be long
enough for at last one digit after the prefix. If not, it will be handled
like this:
sscanf("0x4", "%1i", &i): i=0, scanning continues with the 'x'
sscanf("0x4", "%2i", &i): i=0, scanning continues with the '4'
This is consistent with the observed behaviour of userland sscanf.
Note that this patch does NOT fix the problem of a single field value
overflowing the target type. So for example:
sscanf("123456789abcdef", "%x", &i);
Will not produce the correct result because the value obviously overflows
INT_MAX. But sscanf will report a successful conversion.
Note that where a very large number is used to mean "unlimited", the value
INT_MAX is used for consistency with the behaviour of vsnprintf().
Signed-off-by: Richard Fitzgerald
Reviewed-by: Petr Mladek
---
lib/kstrtox.c | 13 ++--
lib/kstrtox.h | 2 ++
lib/vsprintf.c | 88 +-
3 files changed, 63 insertions(+), 40 deletions(-)
diff --git a/lib/kstrtox.c b/lib/kstrtox.c
index a118b0b1e9b2..0b5fe8b41173 100644
--- a/lib/kstrtox.c
+++ b/lib/kstrtox.c
@@ -39,20 +39,22 @@ const char *_parse_integer_fixup_radix(const char *s,
unsigned int *base)
/*
* Convert non-negative integer string representation in explicitly given radix
- * to an integer.
+ * to an integer. A maximum of max_chars characters will be converted.
+ *
* Return number of characters consumed maybe or-ed with overflow bit.
* If overflow occurs, result integer (incorrect) is still returned.
*
* Don't you dare use this function.
*/
-unsigned int _parse_integer(const char *s, unsigned int base, unsigned long
long *p)
+unsigned int _parse_integer_limit(const char *s, unsigned int base, unsigned
long long *p,
+ size_t max_chars)
{
unsigned long long res;
unsigned int rv;
res = 0;
rv = 0;
- while (1) {
+ while (max_chars--) {
unsigned int c = *s;
unsigned int lc = c | 0x20; /* don't tolower() this line */
unsigned int val;
@@ -82,6 +84,11 @@ unsigned int _parse_integer(const char *s, unsigned int
base, unsigned long long
return rv;
}
+unsigned int _parse_integer(const char *s, unsigned int base, unsigned long
long *p)
+{
+ return _parse_integer_limit(s, base, p, INT_MAX);
+}
+
static int _kstrtoull(const char *s, unsigned int base, unsigned long long
*res)
{
unsigned long long _res;
diff --git a/lib/kstrtox.h b/lib/kstrtox.h
index 3b4637bcd254..158c400ca865 100644
--- a/lib/kstrtox.h
+++ b/lib/kstrtox.h
@@ -4,6 +4,8 @@
#define KSTRTOX_OVERFLOW (1U << 31)
const char *_parse_integer_fixup_radix(const char *s, unsigned int *base);
+unsigned int _parse_integer_limit(const char *s, unsigned int base, unsigned
long long *res,
+ size_t max_chars);
unsigned int _parse_integer(const char *s, unsigned int base, unsigned long
long *res);
#endif
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index f78651e9b030..c51167df527d 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -53,29 +53,43 @@
#include
#include "kstrtox.h"
-/**
- * simple_strtoull - convert a string to an unsigned long long
- * @cp: The start of the string
- * @endp: A pointer to the end of the parsed string will be placed here
- * @base: The number base to use
- *
- * This function has caveats. Please use kstrtoull instead.
- */
-unsigned long long simple_strtoull(const char *cp, char **endp, unsigned int
base)
+static unsigned long long simple_strntoull(const char *startp, size_t
max_chars,
+ char **endp, unsigned int base)
{
- unsigned long long result;
+ const char *cp;
+ unsigned long long result = 0ULL;
+ size_t prefix_chars;
unsigned int rv;
- cp = _parse_integer_fixup_radix(cp, &base);
- rv = _parse_integer(cp, base, &result);
- /* FIXME */
- cp += (rv & ~KSTRTOX_OVERFLOW);
+ cp = _parse_integer_fixup_radix(startp, &base);
+ prefix_chars = cp - startp;
+ if (prefix_chars < max_chars) {
+ rv = _parse_inte
Re: [PATCH] MAINTAINERS: add self as reviewer to INTEL STRATIX10 FIRMWARE DRIVERS
Hi Tom, On 3/27/21 10:19 AM, t...@redhat.com wrote: From: Tom Rix The Intel stratix 10 is a fpga. I review fpga's. So I want to help in this related subsystem. Intel Stratix10 service layer driver is not a FPGA. Intel FPGA SoC is composed of ARM Cortex hard processor system (HPS) and Secure Device Manager (SDM). SDM is the hardware which does FPGA configuration, remote system update (RSU), crypto service (FCS), warm reset and other features. To meet the whole system security needs and support virtual machine requesting communication with SDM, only the secure world of software (EL3) can interfaces with SDM. All software entities running on the other exception level must channel through the EL3 software whenever it needs service from SDM. Intel Stratix 10 service layer driver, added into Opensource kernel mainlines since version 5.0, interfaces with the service provides at EL1 (FPGA manager, RSU and FPGA crypto service drivers as today) and managers secure monitor call (SMC) to communicate with the secure monitor software at the secure world (EL3). Regards, Richard Signed-off-by: Tom Rix --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 67b104202602..00828de0a7bc 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -9266,6 +9266,7 @@ F:tools/power/x86/intel-speed-select/ INTEL STRATIX10 FIRMWARE DRIVERS M:Richard Gong +R: Tom RixL: linux-kernel@vger.kernel.org S:Maintained F:Documentation/ABI/testing/sysfs-devices-platform-stratix10-rsu
Re: perf does not resolve plt symbols from libstdc++ right (.plt.sec problem)
On Mon, 29 Mar 2021, Jiri Slaby wrote: > Any ideas on this? > > On 11. 01. 21, 7:31, Jiri Slaby wrote: > > Hi, > > > > this e-mails is a follow-up of my report at: > > https://bugzilla.suse.com/show_bug.cgi?id=1180681 > > > > There is a problem with *@plt symbols in some libraries, they are unresolved > > by perf (memcmp@plt in this case): > > > 0.26% main2 /usr/lib64/libstdc++.so.6.0.28 0xa51a0 > > l [.] 0x000a51a0 > > > > On the other hand, plt symbols in other libraries are fine (memset@plt in > > this case): > > > 0.17% main2 /usr/lib64/libantlr4-runtime.so.4.8 0x4ed10 > > l [.] memset@plt > > > > I dumped memcmp's .plt.rela entries in perf: > > /usr/lib64/libantlr4-runtime.so.4.8: 154th addr=4e9d0 plt_off=4e020 hdr=10 > > entry=10 > > /usr/lib64/libstdc++.so.6.0.28: 772th addr=a1070 plt_off=9e020 hdr=10 > > entry=10 > > > > The difference (offset) of stdc++'s memcmp is 0xa51a0 (correct) - 0xa1070 > > (perf's computed) = 0x4130. > > > > The problem is perf assumes nth entry of .plt.rela to correspond to nth > > function in .plt, but memcmp is in .plt.sec in libstdc++.so: > > > > >Relocation section '.rela.plt' at offset 0x97900 contains 1018 entries: > > > Offset Info Type Symbol's > > Value Symbol's Name + Addend > > > ... > > > 001dc838 00780007 R_X86_64_JUMP_SLOT > > memcmp@GLIBC_2.2.5 + 0 > > > > Perf does this with the rela entries: > > https://github.com/torvalds/linux/blob/f5e6c330254ae691f6d7befe61c786eb5056007e/tools/perf/util/symbol-elf.c#L385 > > > > > > It takes a symbol index from sym.r_info. Then it resolves its name from > > .dynsym, appending "@plt" to it. Then this name is added to perf's symbol > > table along with address which is computed as .rela.plt index multiplied by > > entry size (shdr_plt.sh_entsize) plus plt header (shdr_plt.sh_entsize on > > x86_64 too). > > > > And from this comes (almost) the offset above: > > >$ objdump -h /usr/lib64/libstdc++.so.6|grep -E ' .plt(\.sec)? ' > > > 12 .plt 3fb0 0009e020 0009e020 > > 0009e020 2**4 > > > 14 .plt.sec 3fa0 000a2160 000a2160 > > 000a2160 2**4 > > > > 0xa2160-0x9e020 = 0x4140. I assume the 0x10 difference is that perf adds > > shdr_plt.sh_entsize (0x10) to the offset to skip the first .plt entry > > (header). > > > > Richard writes: > > == > > .plt.sec is IIRC the "second" (sec) PLT entry - the one that will be used on > > the second call (and on). This is used / emitted for ELF object > > instrumented for Intel CET. The details escape me for the moment but I hope > > the x86 ABI documents this (and the constraints) in detail. I just checked and the x86_64 psABI doesn't say anything about .plt.sec > > == > > > > How should perf find out whether to consider .plt or .plt.sec? Or generally, > > how to properly find an address of *@plt symbols like memcmp@plt above? > > thanks, > > > -- Richard Biener SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
[PATCH v4 0/2] add one regulator used to power up pcie phy
Changes: v3 -> v4 Split the DTS changes to a standalone patch from this patch-set. And would post to Shawn to take it, after the other two are accepted by PCIe tree. Refine the DT binding descriptions refer to Lucas' suggestion. Use "Regarding" to replace the "Regarding to" in the comments refer to Krzysztof's suggestion. v2 -> v3: Refine the DT binding descriptions, and the condition adjustment in the codes. v1 -> v2: Don't use the boolean property to specify the different power supply of PCIe PHY. Use one regulator as a supply to the PCIe controller node, and the regulator APIs to get the voltage of it. Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt | 3 +++ drivers/pci/controller/dwc/pci-imx6.c| 20 2 files changed, 23 insertions(+) [PATCH v4 1/2] dt-bindings: imx6q-pcie: add one regulator used to [PATCH v4 2/2] PCI: imx: clear vreg bypass when pcie vph voltage is
[PATCH] arm64: dts: imx8mq-evk: add one regulator used to power up pcie phy
Both 1.8v and 3.3v power supplies can be used by i.MX8MQ PCIe PHY. In default, the PCIE_VPH voltage is suggested to be 1.8v refer to data sheet. When PCIE_VPH is supplied by 3.3v in the HW schematic design, the VREG_BYPASS bits of GPR registers should be cleared from default value 1b'1 to 1b'0. Thus, the internal 3v3 to 1v8 translator would be turned on. Signed-off-by: Richard Zhu Reviewed-by: Lucas Stach --- arch/arm64/boot/dts/freescale/imx8mq-evk.dts | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm64/boot/dts/freescale/imx8mq-evk.dts b/arch/arm64/boot/dts/freescale/imx8mq-evk.dts index 85b045253a0e..4d2035e3dd7c 100644 --- a/arch/arm64/boot/dts/freescale/imx8mq-evk.dts +++ b/arch/arm64/boot/dts/freescale/imx8mq-evk.dts @@ -318,6 +318,7 @@ <&clk IMX8MQ_CLK_PCIE1_PHY>, <&pcie0_refclk>; clock-names = "pcie", "pcie_aux", "pcie_phy", "pcie_bus"; + vph-supply = <&vgen5_reg>; status = "okay"; }; -- 2.17.1
[PATCH v4 2/2] PCI: imx: clear vreg bypass when pcie vph voltage is 3v3
Both 1.8v and 3.3v power supplies can be used by i.MX8MQ PCIe PHY.
In default, the PCIE_VPH voltage is suggested to be 1.8v refer to data
sheet. When PCIE_VPH is supplied by 3.3v in the HW schematic design,
the VREG_BYPASS bits of GPR registers should be cleared from default
value 1b'1 to 1b'0. Thus, the internal 3v3 to 1v8 translator would be
turned on.
Signed-off-by: Richard Zhu
Reviewed-by: Lucas Stach
---
drivers/pci/controller/dwc/pci-imx6.c | 20
1 file changed, 20 insertions(+)
diff --git a/drivers/pci/controller/dwc/pci-imx6.c
b/drivers/pci/controller/dwc/pci-imx6.c
index 853ea8e82952..94b43b4ecca1 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -37,6 +37,7 @@
#define IMX8MQ_GPR_PCIE_REF_USE_PADBIT(9)
#define IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_ENBIT(10)
#define IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE BIT(11)
+#define IMX8MQ_GPR_PCIE_VREG_BYPASSBIT(12)
#define IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPEGENMASK(11, 8)
#define IMX8MQ_PCIE2_BASE_ADDR 0x33c0
@@ -80,6 +81,7 @@ struct imx6_pcie {
u32 tx_swing_full;
u32 tx_swing_low;
struct regulator*vpcie;
+ struct regulator*vph;
void __iomem*phy_base;
/* power domain for pcie */
@@ -621,6 +623,17 @@ static void imx6_pcie_init_phy(struct imx6_pcie *imx6_pcie)
imx6_pcie_grp_offset(imx6_pcie),
IMX8MQ_GPR_PCIE_REF_USE_PAD,
IMX8MQ_GPR_PCIE_REF_USE_PAD);
+ /*
+* Regarding the datasheet, the PCIE_VPH is suggested
+* to be 1.8V. If the PCIE_VPH is supplied by 3.3V, the
+* VREG_BYPASS should be cleared to zero.
+*/
+ if (imx6_pcie->vph &&
+ regulator_get_voltage(imx6_pcie->vph) > 300)
+ regmap_update_bits(imx6_pcie->iomuxc_gpr,
+ imx6_pcie_grp_offset(imx6_pcie),
+ IMX8MQ_GPR_PCIE_VREG_BYPASS,
+ 0);
break;
case IMX7D:
regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
@@ -1130,6 +1143,13 @@ static int imx6_pcie_probe(struct platform_device *pdev)
imx6_pcie->vpcie = NULL;
}
+ imx6_pcie->vph = devm_regulator_get_optional(&pdev->dev, "vph");
+ if (IS_ERR(imx6_pcie->vph)) {
+ if (PTR_ERR(imx6_pcie->vph) != -ENODEV)
+ return PTR_ERR(imx6_pcie->vph);
+ imx6_pcie->vph = NULL;
+ }
+
platform_set_drvdata(pdev, imx6_pcie);
ret = imx6_pcie_attach_pd(dev);
--
2.17.1
[PATCH v4 1/2] dt-bindings: imx6q-pcie: add one regulator used to power up pcie phy
Both 1.8v and 3.3v power supplies can be used by i.MX8MQ PCIe PHY. In default, the PCIE_VPH voltage is suggested to be 1.8v refer to data sheet. When PCIE_VPH is supplied by 3.3v in the HW schematic design, the VREG_BYPASS bits of GPR registers should be cleared from default value 1b'1 to 1b'0. Thus, the internal 3v3 to 1v8 translator would be turned on. Signed-off-by: Richard Zhu --- Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt index de4b2baf91e8..d8971ab99274 100644 --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt @@ -38,6 +38,9 @@ Optional properties: The regulator will be enabled when initializing the PCIe host and disabled either as part of the init process or when shutting down the host. +- vph-supply: Should specify the regulator in charge of VPH one of the three + PCIe PHY powers. This regulator can be supplied by both 1.8v and 3.3v voltage + supplies. Additional required properties for imx6sx-pcie: - clock names: Must include the following additional entries: -- 2.17.1
RE: Re: [PATCH v3 2/3] arm64: dts: imx8mq-evk: add one regulator used to power up pcie phy
> -Original Message- > From: Lucas Stach > Sent: Friday, March 26, 2021 5:40 PM > To: Richard Zhu ; andrew.smir...@gmail.com; > shawn...@kernel.org; k...@linux.com; bhelg...@google.com; > ste...@agner.ch; lorenzo.pieral...@arm.com > Cc: linux-...@vger.kernel.org; dl-linux-imx ; > linux-arm-ker...@lists.infradead.org; linux-kernel@vger.kernel.org; > ker...@pengutronix.de > Subject: Re: [PATCH v3 2/3] arm64: dts: imx8mq-evk: add one regulator > used to power up pcie phy > Am Donnerstag, dem 25.03.2021 um 16:44 +0800 schrieb Richard Zhu: > > Both 1.8v and 3.3v power supplies can be used by i.MX8MQ PCIe PHY. > > In default, the PCIE_VPH voltage is suggested to be 1.8v refer to data > > sheet. When PCIE_VPH is supplied by 3.3v in the HW schematic design, > > the VREG_BYPASS bits of GPR registers should be cleared from default > > value 1b'1 to 1b'0. Thus, the internal 3v3 to 1v8 translator would be > > turned on. > > > > Signed-off-by: Richard Zhu > > Reviewed-by: Lucas Stach > > I guess you need to split this patch out of the series and post it for Shawn > to > pick up into the imx DT tree, after the other two patches of the series have > been accepted into the PCIe tree. [Richard Zhu] Sure it is. Shawn had been included in this review loop. Would split this patch out of this set, and post it for Shawn lonely, after the other two Patches are accepted into the PCIe tree. > > Regards, > Lucas > > > --- > > arch/arm64/boot/dts/freescale/imx8mq-evk.dts | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mq-evk.dts > > b/arch/arm64/boot/dts/freescale/imx8mq-evk.dts > > index 85b045253a0e..4d2035e3dd7c 100644 > > --- a/arch/arm64/boot/dts/freescale/imx8mq-evk.dts > > +++ b/arch/arm64/boot/dts/freescale/imx8mq-evk.dts > > @@ -318,6 +318,7 @@ > ><&clk IMX8MQ_CLK_PCIE1_PHY>, > ><&pcie0_refclk>; > > clock-names = "pcie", "pcie_aux", "pcie_phy", "pcie_bus"; > > + vph-supply = <&vgen5_reg>; > > status = "okay"; > > }; > > > > > > > > >
RE:Re: [PATCH v3 1/3] dt-bindings: imx6q-pcie: add one regulator used to power up pcie phy
> -Original Message- > From: Lucas Stach > Sent: Friday, March 26, 2021 5:38 PM > To: Richard Zhu ; andrew.smir...@gmail.com; > shawn...@kernel.org; k...@linux.com; bhelg...@google.com; > ste...@agner.ch; lorenzo.pieral...@arm.com > Cc: linux-...@vger.kernel.org; dl-linux-imx ; > linux-arm-ker...@lists.infradead.org; linux-kernel@vger.kernel.org; > ker...@pengutronix.de > Subject: Re: [PATCH v3 1/3] dt-bindings: imx6q-pcie: add one regulator > used to power up pcie phy > Am Donnerstag, dem 25.03.2021 um 16:44 +0800 schrieb Richard Zhu: > > Both 1.8v and 3.3v power supplies can be used by i.MX8MQ PCIe PHY. > > In default, the PCIE_VPH voltage is suggested to be 1.8v refer to data > > sheet. When PCIE_VPH is supplied by 3.3v in the HW schematic design, > > the VREG_BYPASS bits of GPR registers should be cleared from default > > value 1b'1 to 1b'0. Thus, the internal 3v3 to 1v8 translator would be > > turned on. > > > > Signed-off-by: Richard Zhu > > --- > > Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt > > b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt > > index de4b2baf91e8..e6d1886144ce 100644 > > --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt > > +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt > > @@ -38,6 +38,9 @@ Optional properties: > >The regulator will be enabled when initializing the PCIe host and > >disabled either as part of the init process or when shutting down the > >host. > > +- vph-supply: Should specify the regulator in charge of VPH one of > > +the three > > + PCIe PHY powers. This regulator can be supplied by both 1.8v and > > +3.3v voltage > > + supplies. Might be used to distinguish different HW board designs. > > Please just get rid of the last sentence. All DT properties are used in one > way > or the other to distinguish different HW designs, so no need to mention this. [Richard Zhu] Thanks, would remove the last sentence later. > > Regards, > Lucas
Re: [Bug 212265] New: clock_gettime(CLOCK_TAI, ...) should return an error when TAI has not been configured
On Fri, Mar 26, 2021 at 12:13:43PM +0100, Thomas Gleixner wrote: > On Sat, Mar 13 2021 at 17:44, bugzilla-daemon wrote: > > Unfortunately, although the majority of distributions ship with a leap > > second file from the zoneinfo database, many or most of them (I have > > Arch here) do not configure ntpd to know about it, so ntpd does not > > set things up properly for CLOCK_TAI to work. I'm not sure about "many or most" distros. In Debian, the ntp package depends on tzdata, and the default /etc/ntp.conf does use the leap seconds file. > That would be a user visible change and might hit existing user space by > surprise, so that's not a necessarily a good option. Agreed. > and the kernel on it's own has no way to check for and retrieve an > up-to-date version. That's why it is delegated to user space. Right, the kernel can't make any assumptions about the TAI-UTC offset. > I hope the NTP/TAI wizards have some more insight/opinions on this. I agree that ntpd and the current distros don't handle this very well, but all the pieces are there to allow user space to handle TAI and leap seconds as reasonably as possible. The fundamental issue is that there is no way to determine the TAI-UTC offset without some kind of input from the real world. Even with GPS, after a cold boot you cannot know the offset immediately, because the leap second information is broadcast in the almanac only every 12.5 minutes, and so you can be left in suspense for a long time. Using ntpd on Debian, the service will set the offset, but only after synchronization with the upstream server has been established, and this takes about five minutes, IIRC. If waiting 5 or 12.5 minutes is too long for your requirements, you can boot strap the time from RTC [1] and then consult the leap seconds table [2] to set the TAI-UTC offset in the kernel via adjtimex(). Unfortunately there is no user space utility for setting TAI-UTC, but I hacked one 'adjtimex' program for this purpose: https://github.com/richardcochran/ntpclient-2015 Getting back to the original point of the kernel returning an error, I don't see a need for this. Applications that require correct leap seconds can simply call adjtimex() and wait until the initial zero value is changed by ntpd/etc to the correct offset. That isn't fundamentally harder than calling clock_gettime() and waiting until the error would go away. Thanks, Richard 1. Assuming the RTC was set and has a fresh battery, and assuming no leap seconds occurred while your computer was off! 2. Assuming the RTC value is not newer than the expiration date of the leap seconds file.
[PATCH v5] audit: log nftables configuration change events once per table
Reduce logging of nftables events to a level similar to iptables.
Restore the table field to list the table, adding the generation.
Indicate the op as the most significant operation in the event.
A couple of sample events:
type=PROCTITLE msg=audit(2021-03-18 09:30:49.801:143) :
proctitle=/usr/bin/python3 -s /usr/sbin/firewalld --nofork --nopid
type=SYSCALL msg=audit(2021-03-18 09:30:49.801:143) : arch=x86_64
syscall=sendmsg success=yes exit=172 a0=0x6 a1=0x7ffdcfcbe650 a2=0x0
a3=0x7ffdcfcbd52c items=0 ppid=1 pid=367 auid=unset uid=root gid=root euid=root
suid=root fsuid=root egid=roo
t sgid=root fsgid=root tty=(none) ses=unset comm=firewalld
exe=/usr/bin/python3.9 subj=system_u:system_r:firewalld_t:s0 key=(null)
type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.801:143) : table=firewalld:2
family=ipv6 entries=1 op=nft_register_table pid=367
subj=system_u:system_r:firewalld_t:s0 comm=firewalld
type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.801:143) : table=firewalld:2
family=ipv4 entries=1 op=nft_register_table pid=367
subj=system_u:system_r:firewalld_t:s0 comm=firewalld
type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.801:143) : table=firewalld:2
family=inet entries=1 op=nft_register_table pid=367
subj=system_u:system_r:firewalld_t:s0 comm=firewalld
type=PROCTITLE msg=audit(2021-03-18 09:30:49.839:144) :
proctitle=/usr/bin/python3 -s /usr/sbin/firewalld --nofork --nopid
type=SYSCALL msg=audit(2021-03-18 09:30:49.839:144) : arch=x86_64
syscall=sendmsg success=yes exit=22792 a0=0x6 a1=0x7ffdcfcbe650 a2=0x0
a3=0x7ffdcfcbd52c items=0 ppid=1 pid=367 auid=unset uid=root gid=root euid=root
suid=root fsuid=root egid=r
oot sgid=root fsgid=root tty=(none) ses=unset comm=firewalld
exe=/usr/bin/python3.9 subj=system_u:system_r:firewalld_t:s0 key=(null)
type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.839:144) : table=firewalld:3
family=ipv6 entries=30 op=nft_register_chain pid=367
subj=system_u:system_r:firewalld_t:s0 comm=firewalld
type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.839:144) : table=firewalld:3
family=ipv4 entries=30 op=nft_register_chain pid=367
subj=system_u:system_r:firewalld_t:s0 comm=firewalld
type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.839:144) : table=firewalld:3
family=inet entries=165 op=nft_register_chain pid=367
subj=system_u:system_r:firewalld_t:s0 comm=firewalld
The issue was originally documented in
https://github.com/linux-audit/audit-kernel/issues/124
Signed-off-by: Richard Guy Briggs
---
Changelog:
v5:
(sorry for all the noise...)
- fix kbuild missing prototype warning in
nf_tables_commit_audit_{alloc,collect,log}()
v4:
- move nf_tables_commit_audit_log() before nf_tables_commit_release() [fw]
- move nft2audit_op[] from audit.h to nf_tables_api.c
v3:
- fix function braces, reduce parameter scope [pna]
- pre-allocate nft_audit_data per table in step 1, bail on ENOMEM [pna]
v2:
- convert NFT ops to array indicies in nft2audit_op[] [ps]
- use linux lists [pna]
- use functions for each of collection and logging of audit data [pna]
---
net/netfilter/nf_tables_api.c | 187 +++---
1 file changed, 104 insertions(+), 83 deletions(-)
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index c1eb5cdb3033..ef51abe3a6d7 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -66,6 +66,41 @@ static const struct rhashtable_params nft_objname_ht_params
= {
.automatic_shrinking= true,
};
+struct nft_audit_data {
+ struct nft_table *table;
+ int entries;
+ int op;
+ struct list_head list;
+};
+
+static const u8 nft2audit_op[NFT_MSG_MAX] = { // enum nf_tables_msg_types
+ [NFT_MSG_NEWTABLE] = AUDIT_NFT_OP_TABLE_REGISTER,
+ [NFT_MSG_GETTABLE] = AUDIT_NFT_OP_INVALID,
+ [NFT_MSG_DELTABLE] = AUDIT_NFT_OP_TABLE_UNREGISTER,
+ [NFT_MSG_NEWCHAIN] = AUDIT_NFT_OP_CHAIN_REGISTER,
+ [NFT_MSG_GETCHAIN] = AUDIT_NFT_OP_INVALID,
+ [NFT_MSG_DELCHAIN] = AUDIT_NFT_OP_CHAIN_UNREGISTER,
+ [NFT_MSG_NEWRULE] = AUDIT_NFT_OP_RULE_REGISTER,
+ [NFT_MSG_GETRULE] = AUDIT_NFT_OP_INVALID,
+ [NFT_MSG_DELRULE] = AUDIT_NFT_OP_RULE_UNREGISTER,
+ [NFT_MSG_NEWSET]= AUDIT_NFT_OP_SET_REGISTER,
+ [NFT_MSG_GETSET]= AUDIT_NFT_OP_INVALID,
+ [NFT_MSG_DELSET]= AUDIT_NFT_OP_SET_UNREGISTER,
+ [NFT_MSG_NEWSETELEM]= AUDIT_NFT_OP_SETELEM_REGISTER,
+ [NFT_MSG_GETSETELEM]= AUDIT_NFT_OP_INVALID,
+ [NFT_MSG_DELSETELEM]= AUDIT_NFT_OP_SETELEM_UNREGISTER,
+ [NFT_MSG_NEWGEN]= AUDIT_NFT_OP_GEN_REGISTER,
+ [NFT_MSG_GETGEN]= AUDIT_NFT_OP_INVALID,
+ [NFT_MSG_TRACE] = AUDIT_NFT_OP_INVALID,
+ [NFT_MSG_NEWOBJ]= AUDIT_NFT_OP_OBJ_REGISTER,
+ [NFT_MSG_GETOBJ]= AUDIT_NFT_OP_INVALID,
+ [NFT_MSG_DELOBJ]= AUDIT_NFT_OP_OBJ_UNREGISTER,
+ [NFT_MSG_GETOBJ_RESET
RE: Re: [PATCH v3 3/3] PCI: imx: clear vreg bypass when pcie vph voltage is 3v3
> -Original Message- > From: Krzysztof Wilczyński > Sent: Friday, March 26, 2021 3:46 PM > To: Richard Zhu > Cc: l.st...@pengutronix.de; andrew.smir...@gmail.com; > shawn...@kernel.org; bhelg...@google.com; ste...@agner.ch; > lorenzo.pieral...@arm.com; linux-...@vger.kernel.org; dl-linux-imx > ; linux-arm-ker...@lists.infradead.org; > linux-kernel@vger.kernel.org; ker...@pengutronix.de > Subject: Re: [PATCH v3 3/3] PCI: imx: clear vreg bypass when pcie vph > voltage is 3v3 > Hi, > > > + /* > > + * Regarding to the datasheet, the PCIE_VPH is suggested > > + * to be 1.8V. If the PCIE_VPH is supplied by 3.3V, the > > + * VREG_BYPASS should be cleared to zero. > > + */ > [...] > > A small nitpick here. What about the following: > > Regarding the data sheet, the PCIE_VPH is suggested to be 1.8V. > If the PCIE_VPH is supplied with 3.3V, the VREG_BYPASS should be > cleared to zero. > > What do you think? [Richard Zhu] Hi Krzysztof: Thanks for your comments. You're right. It should be "Regarding something", not the "Regarding to something" Thanks. 😊 Best Regards Richard Zhu > > Krzysztof
[PATCH v3 0/3] add one regulator used to power up pcie phy
Changes: v2 -> v3: Refine the DT binding descriptions, and the condition adjustment in the codes. v1 -> v2: Don't use the boolean property to specify the different power supply of PCIe PHY. Use one regulator as a supply to the PCIe controller node, and the regulator APIs to get the voltage of it. Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt | 3 +++ arch/arm64/boot/dts/freescale/imx8mq-evk.dts | 1 + drivers/pci/controller/dwc/pci-imx6.c| 20 3 files changed, 24 insertions(+) [PATCH v3 1/3] dt-bindings: imx6q-pcie: add one regulator used to [PATCH v3 2/3] arm64: dts: imx8mq-evk: add one regulator used to [PATCH v3 3/3] PCI: imx: clear vreg bypass when pcie vph voltage is
[PATCH v3 1/3] dt-bindings: imx6q-pcie: add one regulator used to power up pcie phy
Both 1.8v and 3.3v power supplies can be used by i.MX8MQ PCIe PHY. In default, the PCIE_VPH voltage is suggested to be 1.8v refer to data sheet. When PCIE_VPH is supplied by 3.3v in the HW schematic design, the VREG_BYPASS bits of GPR registers should be cleared from default value 1b'1 to 1b'0. Thus, the internal 3v3 to 1v8 translator would be turned on. Signed-off-by: Richard Zhu --- Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt index de4b2baf91e8..e6d1886144ce 100644 --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt @@ -38,6 +38,9 @@ Optional properties: The regulator will be enabled when initializing the PCIe host and disabled either as part of the init process or when shutting down the host. +- vph-supply: Should specify the regulator in charge of VPH one of the three + PCIe PHY powers. This regulator can be supplied by both 1.8v and 3.3v voltage + supplies. Might be used to distinguish different HW board designs. Additional required properties for imx6sx-pcie: - clock names: Must include the following additional entries: -- 2.17.1
[PATCH v3 2/3] arm64: dts: imx8mq-evk: add one regulator used to power up pcie phy
Both 1.8v and 3.3v power supplies can be used by i.MX8MQ PCIe PHY. In default, the PCIE_VPH voltage is suggested to be 1.8v refer to data sheet. When PCIE_VPH is supplied by 3.3v in the HW schematic design, the VREG_BYPASS bits of GPR registers should be cleared from default value 1b'1 to 1b'0. Thus, the internal 3v3 to 1v8 translator would be turned on. Signed-off-by: Richard Zhu --- arch/arm64/boot/dts/freescale/imx8mq-evk.dts | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm64/boot/dts/freescale/imx8mq-evk.dts b/arch/arm64/boot/dts/freescale/imx8mq-evk.dts index 85b045253a0e..4d2035e3dd7c 100644 --- a/arch/arm64/boot/dts/freescale/imx8mq-evk.dts +++ b/arch/arm64/boot/dts/freescale/imx8mq-evk.dts @@ -318,6 +318,7 @@ <&clk IMX8MQ_CLK_PCIE1_PHY>, <&pcie0_refclk>; clock-names = "pcie", "pcie_aux", "pcie_phy", "pcie_bus"; + vph-supply = <&vgen5_reg>; status = "okay"; }; -- 2.17.1
[PATCH v3 3/3] PCI: imx: clear vreg bypass when pcie vph voltage is 3v3
Both 1.8v and 3.3v power supplies can be used by i.MX8MQ PCIe PHY.
In default, the PCIE_VPH voltage is suggested to be 1.8v refer to data
sheet. When PCIE_VPH is supplied by 3.3v in the HW schematic design,
the VREG_BYPASS bits of GPR registers should be cleared from default
value 1b'1 to 1b'0. Thus, the internal 3v3 to 1v8 translator would be
turned on.
Signed-off-by: Richard Zhu
---
drivers/pci/controller/dwc/pci-imx6.c | 20
1 file changed, 20 insertions(+)
diff --git a/drivers/pci/controller/dwc/pci-imx6.c
b/drivers/pci/controller/dwc/pci-imx6.c
index 853ea8e82952..d9d534f0840f 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -37,6 +37,7 @@
#define IMX8MQ_GPR_PCIE_REF_USE_PADBIT(9)
#define IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_ENBIT(10)
#define IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE BIT(11)
+#define IMX8MQ_GPR_PCIE_VREG_BYPASSBIT(12)
#define IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPEGENMASK(11, 8)
#define IMX8MQ_PCIE2_BASE_ADDR 0x33c0
@@ -80,6 +81,7 @@ struct imx6_pcie {
u32 tx_swing_full;
u32 tx_swing_low;
struct regulator*vpcie;
+ struct regulator*vph;
void __iomem*phy_base;
/* power domain for pcie */
@@ -621,6 +623,17 @@ static void imx6_pcie_init_phy(struct imx6_pcie *imx6_pcie)
imx6_pcie_grp_offset(imx6_pcie),
IMX8MQ_GPR_PCIE_REF_USE_PAD,
IMX8MQ_GPR_PCIE_REF_USE_PAD);
+ /*
+* Regarding to the datasheet, the PCIE_VPH is suggested
+* to be 1.8V. If the PCIE_VPH is supplied by 3.3V, the
+* VREG_BYPASS should be cleared to zero.
+*/
+ if (imx6_pcie->vph &&
+ regulator_get_voltage(imx6_pcie->vph) > 300)
+ regmap_update_bits(imx6_pcie->iomuxc_gpr,
+ imx6_pcie_grp_offset(imx6_pcie),
+ IMX8MQ_GPR_PCIE_VREG_BYPASS,
+ 0);
break;
case IMX7D:
regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
@@ -1130,6 +1143,13 @@ static int imx6_pcie_probe(struct platform_device *pdev)
imx6_pcie->vpcie = NULL;
}
+ imx6_pcie->vph = devm_regulator_get_optional(&pdev->dev, "vph");
+ if (IS_ERR(imx6_pcie->vph)) {
+ if (PTR_ERR(imx6_pcie->vph) != -ENODEV)
+ return PTR_ERR(imx6_pcie->vph);
+ imx6_pcie->vph = NULL;
+ }
+
platform_set_drvdata(pdev, imx6_pcie);
ret = imx6_pcie_attach_pd(dev);
--
2.17.1
[PATCH v4] audit: log nftables configuration change events once per table
Reduce logging of nftables events to a level similar to iptables.
Restore the table field to list the table, adding the generation.
Indicate the op as the most significant operation in the event.
A couple of sample events:
type=PROCTITLE msg=audit(2021-03-18 09:30:49.801:143) :
proctitle=/usr/bin/python3 -s /usr/sbin/firewalld --nofork --nopid
type=SYSCALL msg=audit(2021-03-18 09:30:49.801:143) : arch=x86_64
syscall=sendmsg success=yes exit=172 a0=0x6 a1=0x7ffdcfcbe650 a2=0x0
a3=0x7ffdcfcbd52c items=0 ppid=1 pid=367 auid=unset uid=root gid=root euid=root
suid=root fsuid=root egid=roo
t sgid=root fsgid=root tty=(none) ses=unset comm=firewalld
exe=/usr/bin/python3.9 subj=system_u:system_r:firewalld_t:s0 key=(null)
type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.801:143) : table=firewalld:2
family=ipv6 entries=1 op=nft_register_table pid=367
subj=system_u:system_r:firewalld_t:s0 comm=firewalld
type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.801:143) : table=firewalld:2
family=ipv4 entries=1 op=nft_register_table pid=367
subj=system_u:system_r:firewalld_t:s0 comm=firewalld
type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.801:143) : table=firewalld:2
family=inet entries=1 op=nft_register_table pid=367
subj=system_u:system_r:firewalld_t:s0 comm=firewalld
type=PROCTITLE msg=audit(2021-03-18 09:30:49.839:144) :
proctitle=/usr/bin/python3 -s /usr/sbin/firewalld --nofork --nopid
type=SYSCALL msg=audit(2021-03-18 09:30:49.839:144) : arch=x86_64
syscall=sendmsg success=yes exit=22792 a0=0x6 a1=0x7ffdcfcbe650 a2=0x0
a3=0x7ffdcfcbd52c items=0 ppid=1 pid=367 auid=unset uid=root gid=root euid=root
suid=root fsuid=root egid=r
oot sgid=root fsgid=root tty=(none) ses=unset comm=firewalld
exe=/usr/bin/python3.9 subj=system_u:system_r:firewalld_t:s0 key=(null)
type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.839:144) : table=firewalld:3
family=ipv6 entries=30 op=nft_register_chain pid=367
subj=system_u:system_r:firewalld_t:s0 comm=firewalld
type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.839:144) : table=firewalld:3
family=ipv4 entries=30 op=nft_register_chain pid=367
subj=system_u:system_r:firewalld_t:s0 comm=firewalld
type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.839:144) : table=firewalld:3
family=inet entries=165 op=nft_register_chain pid=367
subj=system_u:system_r:firewalld_t:s0 comm=firewalld
The issue was originally documented in
https://github.com/linux-audit/audit-kernel/issues/124
Signed-off-by: Richard Guy Briggs
---
Changelog:
v4:
- move nf_tables_commit_audit_log() before nf_tables_commit_release() [fw]
- move nft2audit_op[] from audit.h to nf_tables_api.c
v3:
- fix function braces, reduce parameter scope [pna]
- pre-allocate nft_audit_data per table in step 1, bail on ENOMEM [pna]
v2:
- convert NFT ops to array indicies in nft2audit_op[] [ps]
- use linux lists [pna]
- use functions for each of collection and logging of audit data [pna]
---
net/netfilter/nf_tables_api.c | 187 +++---
1 file changed, 104 insertions(+), 83 deletions(-)
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index c1eb5cdb3033..9c930fe72005 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -66,6 +66,41 @@ static const struct rhashtable_params nft_objname_ht_params
= {
.automatic_shrinking= true,
};
+struct nft_audit_data {
+ struct nft_table *table;
+ int entries;
+ int op;
+ struct list_head list;
+};
+
+static const u8 nft2audit_op[NFT_MSG_MAX] = { // enum nf_tables_msg_types
+ [NFT_MSG_NEWTABLE] = AUDIT_NFT_OP_TABLE_REGISTER,
+ [NFT_MSG_GETTABLE] = AUDIT_NFT_OP_INVALID,
+ [NFT_MSG_DELTABLE] = AUDIT_NFT_OP_TABLE_UNREGISTER,
+ [NFT_MSG_NEWCHAIN] = AUDIT_NFT_OP_CHAIN_REGISTER,
+ [NFT_MSG_GETCHAIN] = AUDIT_NFT_OP_INVALID,
+ [NFT_MSG_DELCHAIN] = AUDIT_NFT_OP_CHAIN_UNREGISTER,
+ [NFT_MSG_NEWRULE] = AUDIT_NFT_OP_RULE_REGISTER,
+ [NFT_MSG_GETRULE] = AUDIT_NFT_OP_INVALID,
+ [NFT_MSG_DELRULE] = AUDIT_NFT_OP_RULE_UNREGISTER,
+ [NFT_MSG_NEWSET]= AUDIT_NFT_OP_SET_REGISTER,
+ [NFT_MSG_GETSET]= AUDIT_NFT_OP_INVALID,
+ [NFT_MSG_DELSET]= AUDIT_NFT_OP_SET_UNREGISTER,
+ [NFT_MSG_NEWSETELEM]= AUDIT_NFT_OP_SETELEM_REGISTER,
+ [NFT_MSG_GETSETELEM]= AUDIT_NFT_OP_INVALID,
+ [NFT_MSG_DELSETELEM]= AUDIT_NFT_OP_SETELEM_UNREGISTER,
+ [NFT_MSG_NEWGEN]= AUDIT_NFT_OP_GEN_REGISTER,
+ [NFT_MSG_GETGEN]= AUDIT_NFT_OP_INVALID,
+ [NFT_MSG_TRACE] = AUDIT_NFT_OP_INVALID,
+ [NFT_MSG_NEWOBJ]= AUDIT_NFT_OP_OBJ_REGISTER,
+ [NFT_MSG_GETOBJ]= AUDIT_NFT_OP_INVALID,
+ [NFT_MSG_DELOBJ]= AUDIT_NFT_OP_OBJ_UNREGISTER,
+ [NFT_MSG_GETOBJ_RESET] = AUDIT_NFT_OP_OBJ_RESET,
+ [NFT_MSG_NEWFLOWTABLE] = AUDIT_NFT_OP_FLOWTABLE_REGISTER,
+ [NFT_MSG_GETFLOWTABLE
Re: [PATCH v3] audit: log nftables configuration change events once per table
On 2021-03-24 12:32, Paul Moore wrote:
> On Tue, Mar 23, 2021 at 4:05 PM Richard Guy Briggs wrote:
> >
> > Reduce logging of nftables events to a level similar to iptables.
> > Restore the table field to list the table, adding the generation.
> >
> > Indicate the op as the most significant operation in the event.
> >
> > A couple of sample events:
> >
> > type=PROCTITLE msg=audit(2021-03-18 09:30:49.801:143) :
> > proctitle=/usr/bin/python3 -s /usr/sbin/firewalld --nofork --nopid
> > type=SYSCALL msg=audit(2021-03-18 09:30:49.801:143) : arch=x86_64
> > syscall=sendmsg success=yes exit=172 a0=0x6 a1=0x7ffdcfcbe650 a2=0x0
> > a3=0x7ffdcfcbd52c items=0 ppid=1 pid=367 auid=unset uid=root gid=root
> > euid=root suid=root fsuid=root egid=roo
> > t sgid=root fsgid=root tty=(none) ses=unset comm=firewalld
> > exe=/usr/bin/python3.9 subj=system_u:system_r:firewalld_t:s0 key=(null)
> > type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.801:143) :
> > table=firewalld:2 family=ipv6 entries=1 op=nft_register_table pid=367
> > subj=system_u:system_r:firewalld_t:s0 comm=firewalld
> > type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.801:143) :
> > table=firewalld:2 family=ipv4 entries=1 op=nft_register_table pid=367
> > subj=system_u:system_r:firewalld_t:s0 comm=firewalld
> > type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.801:143) :
> > table=firewalld:2 family=inet entries=1 op=nft_register_table pid=367
> > subj=system_u:system_r:firewalld_t:s0 comm=firewalld
> >
> > type=PROCTITLE msg=audit(2021-03-18 09:30:49.839:144) :
> > proctitle=/usr/bin/python3 -s /usr/sbin/firewalld --nofork --nopid
> > type=SYSCALL msg=audit(2021-03-18 09:30:49.839:144) : arch=x86_64
> > syscall=sendmsg success=yes exit=22792 a0=0x6 a1=0x7ffdcfcbe650 a2=0x0
> > a3=0x7ffdcfcbd52c items=0 ppid=1 pid=367 auid=unset uid=root gid=root
> > euid=root suid=root fsuid=root egid=r
> > oot sgid=root fsgid=root tty=(none) ses=unset comm=firewalld
> > exe=/usr/bin/python3.9 subj=system_u:system_r:firewalld_t:s0 key=(null)
> > type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.839:144) :
> > table=firewalld:3 family=ipv6 entries=30 op=nft_register_chain pid=367
> > subj=system_u:system_r:firewalld_t:s0 comm=firewalld
> > type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.839:144) :
> > table=firewalld:3 family=ipv4 entries=30 op=nft_register_chain pid=367
> > subj=system_u:system_r:firewalld_t:s0 comm=firewalld
> > type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.839:144) :
> > table=firewalld:3 family=inet entries=165 op=nft_register_chain pid=367
> > subj=system_u:system_r:firewalld_t:s0 comm=firewalld
> >
> > The issue was originally documented in
> > https://github.com/linux-audit/audit-kernel/issues/124
> >
> > Signed-off-by: Richard Guy Briggs
> > ---
> > Changelog:
> > v3:
> > - fix function braces, reduce parameter scope
> > - pre-allocate nft_audit_data per table in step 1, bail on ENOMEM
> >
> > v2:
> > - convert NFT ops to array indicies in nft2audit_op[]
> > - use linux lists
> > - use functions for each of collection and logging of audit data
> > ---
> > include/linux/audit.h | 28 ++
> > net/netfilter/nf_tables_api.c | 160 --
> > 2 files changed, 105 insertions(+), 83 deletions(-)
>
> ...
>
> > diff --git a/include/linux/audit.h b/include/linux/audit.h
> > index 82b7c1116a85..5fafcf4c13de 100644
> > --- a/include/linux/audit.h
> > +++ b/include/linux/audit.h
> > @@ -118,6 +118,34 @@ enum audit_nfcfgop {
> > AUDIT_NFT_OP_INVALID,
> > };
> >
> > +static const u8 nft2audit_op[NFT_MSG_MAX] = { // enum nf_tables_msg_types
> > + [NFT_MSG_NEWTABLE] = AUDIT_NFT_OP_TABLE_REGISTER,
> > + [NFT_MSG_GETTABLE] = AUDIT_NFT_OP_INVALID,
> > + [NFT_MSG_DELTABLE] = AUDIT_NFT_OP_TABLE_UNREGISTER,
> > + [NFT_MSG_NEWCHAIN] = AUDIT_NFT_OP_CHAIN_REGISTER,
> > + [NFT_MSG_GETCHAIN] = AUDIT_NFT_OP_INVALID,
> > + [NFT_MSG_DELCHAIN] = AUDIT_NFT_OP_CHAIN_UNREGISTER,
> > + [NFT_MSG_NEWRULE] = AUDIT_NFT_OP_RULE_REGISTER,
> > + [NFT_MSG_GETRULE] = AUDIT_NFT_OP_INVALID,
> > + [NFT_MSG_DELRULE] = AUDIT_NFT_OP_RULE_UNREGISTER,
> > + [NFT_MSG_NEWSET]= AUDIT_NFT_OP_SET_REGISTER,
> > + [NFT_MSG_GETSET]= AUDIT_NFT_OP_INVALID,
> > + [NFT_MSG_DELSET]= AUDIT_NFT_OP_SET_UNREGISTER,
> > + [NFT_MSG_NEWSETELEM]= AUDIT_NFT_OP_SETELEM_
