Re: [PATCH 4/4] remoteproc: k3-r5: support for graceful stop of remote cores
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, _vec, , , );
+
+ return !ret ? !!(stat & PROC_BOOT_STATUS_FLAG_R5_WFI) : false;
Too fancy for me :) Just return if (ret) right after get_status().
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.
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(>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(>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(>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,
Re: [RFC PATCH v2] ptp: Add vDSO-style vmclock support
On 28 June 2024 17:38:15 BST, Peter Hilber wrote: >On 28.06.24 14:15, David Woodhouse wrote: >> On Fri, 2024-06-28 at 13:33 +0200, Peter Hilber wrote: >>> On 27.06.24 16:52, David Woodhouse wrote: I already added a flags field, so this might look something like: /* * Smearing flags. The UTC clock exposed through this structure * is only ever true UTC, but a guest operating system may * choose to offer a monotonic smeared clock to its users. This * merely offers a hint about what kind of smearing to perform, * for consistency with systems in the nearby environment. */ #define VMCLOCK_FLAGS_SMEAR_UTC_SLS (1<<5) /* draft-kuhn-leapsecond-00.txt */ (UTC-SLS is probably a bad example but are there formal definitions for anything else?) >>> >>> I think it could also be more generic, like flags for linear smearing, >>> cosine smearing(?), and smear_start_sec and smear_end_sec fields (relative >>> to the leap second start). That could also represent UTC-SLS, and >>> noon-to-noon, and it would be well-defined. >>> >>> This should reduce the likelihood that the guest doesn't know the smearing >>> variant. >> >> I'm wary of making it too generic. That would seem to encourage a >> *proliferation* of false "UTC-like" clocks. >> >> It's bad enough that we do smearing at all, let alone that we don't >> have a single definition of how to do it. >> >> I made the smearing hint a full uint8_t instead of using bits in flags, >> in the end. That gives us a full 255 ways of lying to users about what >> the time is, so we're unlikely to run out. And it's easy enough to add >> a new VMCLOCK_SMEARING_XXX type to the 'registry' for any new methods >> that get invented. >> >> > >My concern is that the registry update may come after a driver has already >been implemented, so that it may be hard to ensure that the smearing which >has been chosen is actually implemented. Well yes, but why in the name of all that is holy would anyone want to invent *new* ways to lie to users about the time? If we capture the existing ones as we write this, surely it's a good thing that there's a barrier to entry for adding more? >But the error bounds could be large or missing. I am trying to address use >cases where the host steps or slews the clock as well. The host is absolutely intended to be skewing the clock to keep it accurate as the frequency of the underlying oscillator changes, and the seq_count field will change every time the host does so. Do we need to handle steps differently? Or just let the guest deal with it? If an NTP server suddenly steps the time it reports, what does the client do?
Re: [PATCH 0/4] remoteproc: k3-r5: Introduce suspend to ram support
On Fri, 21 Jun 2024 at 09:01, Richard Genoud wrote: > > 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(-) > Nishanth, Vignesh, Hari and Andrew - I will wait for you guys to review this patch before moving forward. Thanks, Mathieu
Re: [PATCH 4/4] remoteproc: k3-r5: support for graceful stop of remote cores
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?
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, _vec, , , );
> +
> + 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(>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(>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(>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,
Re: [PATCH 3/4] remoteproc: k3-r5: k3_r5_rproc_stop: code reorder
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, >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(>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, >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.
Re: [PATCH v9 8/8] arm64: dts: qcom: Enable Q6v5 WCSS for ipq8074 SoC
Hi Gokul, kernel test robot noticed the following build warnings: [auto build test WARNING on remoteproc/rproc-next] [also build test WARNING on clk/clk-next robh/for-next linus/master v6.10-rc5 next-20240627] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Gokul-Sriram-Palanisamy/remoteproc-qcom-Add-PRNG-proxy-clock/20240625-162317 base: git://git.kernel.org/pub/scm/linux/kernel/git/remoteproc/linux.git rproc-next patch link: https://lore.kernel.org/r/20240621114659.2958170-9-quic_gokulsri%40quicinc.com patch subject: [PATCH v9 8/8] arm64: dts: qcom: Enable Q6v5 WCSS for ipq8074 SoC config: arm64-randconfig-051-20240627 (https://download.01.org/0day-ci/archive/20240629/202406290444.4w2fba5x-...@intel.com/config) compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project ad79a14c9e5ec4a369eed4adf567c22cc029863f) dtschema version: 2024.6.dev3+g650bf2d reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240629/202406290444.4w2fba5x-...@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot | Closes: https://lore.kernel.org/oe-kbuild-all/202406290444.4w2fba5x-...@intel.com/ dtcheck warnings: (new ones prefixed by >>) arch/arm64/boot/dts/qcom/ipq8074-hk01.dtb: phy@59000: 'vdda-pll-supply' is a required property from schema $id: http://devicetree.org/schemas/phy/qcom,qusb2-phy.yaml# arch/arm64/boot/dts/qcom/ipq8074-hk01.dtb: phy@59000: 'vdda-phy-dpdm-supply' is a required property from schema $id: http://devicetree.org/schemas/phy/qcom,qusb2-phy.yaml# arch/arm64/boot/dts/qcom/ipq8074-hk01.dtb: phy@79000: 'vdd-supply' is a required property from schema $id: http://devicetree.org/schemas/phy/qcom,qusb2-phy.yaml# arch/arm64/boot/dts/qcom/ipq8074-hk01.dtb: phy@79000: 'vdda-pll-supply' is a required property from schema $id: http://devicetree.org/schemas/phy/qcom,qusb2-phy.yaml# arch/arm64/boot/dts/qcom/ipq8074-hk01.dtb: phy@79000: 'vdda-phy-dpdm-supply' is a required property from schema $id: http://devicetree.org/schemas/phy/qcom,qusb2-phy.yaml# >> arch/arm64/boot/dts/qcom/ipq8074-hk01.dtb: /soc@0/remoteproc@cd0: failed >> to match any schema with compatible: ['qcom,ipq8074-wcss-pil'] -- >> arch/arm64/boot/dts/qcom/ipq8074-hk10-c1.dtb: /soc@0/remoteproc@cd0: >> failed to match any schema with compatible: ['qcom,ipq8074-wcss-pil'] -- >> arch/arm64/boot/dts/qcom/ipq8074-hk10-c2.dtb: /soc@0/remoteproc@cd0: >> failed to match any schema with compatible: ['qcom,ipq8074-wcss-pil'] -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
Re: [PATCH 2/4] remoteproc: k3-r5: Introduce PM suspend/resume handlers
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(>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.
> 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(>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
[ANNOUNCE] 5.10.219-rt111
Hello RT-list! I'm pleased to announce the 5.10.219-rt111 stable release. This release is just an update to the new stable 5.10.219 version and no RT changes have been made. You can get this release via the git tree at: git://git.kernel.org/pub/scm/linux/kernel/git/rt/linux-stable-rt.git branch: v5.10-rt Head SHA1: 4a4ea2ea1cc624964d53cf22fa5f92a9f43708bb Or to build 5.10.219-rt111 directly, the following patches should be applied: https://www.kernel.org/pub/linux/kernel/v5.x/linux-5.10.tar.xz https://www.kernel.org/pub/linux/kernel/v5.x/patch-5.10.219.xz https://www.kernel.org/pub/linux/kernel/projects/rt/5.10/older/patch-5.10.219-rt111.patch.xz Signing key fingerprint: 9354 0649 9972 8D31 D464 D140 F394 A423 F8E6 7C26 All keys used for the above files and repositories can be found on the following git repository: git://git.kernel.org/pub/scm/docs/kernel/pgpkeys.git Enjoy! Luis
Re: [PATCH 1/4] remoteproc: k3-r5: Fix IPC-only mode detection
Nishanth, Vignesh, Hari and Andrew - please have a look at this patch.
Thanks,
Mathieu
On Fri, 28 Jun 2024 at 13:53, Mathieu Poirier
wrote:
>
> Good day,
>
> On Fri, Jun 21, 2024 at 05:00:55PM +0200, Richard Genoud wrote:
> > 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.
> >
>
> Your assessment seems to be correct. That said I'd like to have an RB or a TB
> from someone in the TI delegation - guys please have a look.
>
> Thanks,
> Mathieu
>
> > 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(>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;
Re: [PATCH 1/4] remoteproc: k3-r5: Fix IPC-only mode detection
Good day,
On Fri, Jun 21, 2024 at 05:00:55PM +0200, Richard Genoud wrote:
> 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.
>
Your assessment seems to be correct. That said I'd like to have an RB or a TB
from someone in the TI delegation - guys please have a look.
Thanks,
Mathieu
> 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(>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;
Re: [PATCH] kallsyms, livepatch: Fix livepatch with CONFIG_LTO_CLANG
Hi Luis, On Fri, Jun 28, 2024 at 10:36 AM Luis Chamberlain wrote: > > On Fri, Jun 28, 2024 at 02:23:49PM +0200, Miroslav Benes wrote: > > On Fri, 7 Jun 2024, Song Liu wrote: > > > > > Hi Miroslav, > > > > > > Thanks for reviewing the patch! > > > > > > I think it is possible. Currently, kallsyms_on_each_match_symbol matches > > > symbols without the postfix. We can add a variation or a parameter, so > > > that it matches the full name with post fix. > > > > I think it might be better. > > > > Luis, what is your take on this? > > > > If I am not mistaken, there was a patch set to address this. Luis might > > remember more. > > Yeah this is a real issue outside of CONFIG_LTO_CLANG, Rust modules is > another example where instead of symbol names they want to use full > hashes. So, as I hinted to you Sami, can we knock two birds with one stone > here and move CONFIG_LTO_CLANG to use the same strategy as Rust so we > have two users instead of just one? I'm all for finding generic solutions, but perhaps I've missed the patch set Miroslav mentioned because I'm not quite sure how these problems are related. LTO makes duplicate symbol names globally unique by appending a postfix to them, which complicates looking up symbols by name. Rust, on the other hand, has a problem with CONFIG_MODVERSIONS because the long symbol names it generates cannot fit in the small buffer in struct modversion_info. The only reason we proposed storing a cryptographic hash in modversion_info was to avoid breaking userspace tools that parse this data structure, but AFAIK nobody wants to use hashed symbol names anywhere else. In fact, if there's a better solution for addressing modversion_info limitations, I would be happy not to hash anything. Sami
[syzbot] [virt?] [net?] upstream test error: KMSAN: uninit-value in virtnet_poll
Hello, syzbot found the following issue on: HEAD commit:626737a5791b Merge tag 'pinctrl-v6.10-2' of git://git.kern.. git tree: upstream console output: https://syzkaller.appspot.com/x/log.txt?x=1373f72e98 kernel config: https://syzkaller.appspot.com/x/.config?x=12ff58d525e7b8f9 dashboard link: https://syzkaller.appspot.com/bug?extid=35b9a14142dd62084eb9 compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40 userspace arch: i386 Downloadable assets: disk image: https://storage.googleapis.com/syzbot-assets/b5c2e4152e89/disk-626737a5.raw.xz vmlinux: https://storage.googleapis.com/syzbot-assets/4847a4cfa180/vmlinux-626737a5.xz kernel image: https://storage.googleapis.com/syzbot-assets/18f05d5ddcb1/bzImage-626737a5.xz IMPORTANT: if you fix the issue, please add the following tag to the commit: Reported-by: syzbot+35b9a14142dd62084...@syzkaller.appspotmail.com = BUG: KMSAN: uninit-value in receive_mergeable drivers/net/virtio_net.c:1847 [inline] BUG: KMSAN: uninit-value in receive_buf+0x2620/0x6070 drivers/net/virtio_net.c:1973 virtnet_receive drivers/net/virtio_net.c:2277 [inline] virtnet_poll+0xd1c/0x23c0 drivers/net/virtio_net.c:2380 __napi_poll+0xe7/0x980 net/core/dev.c:6722 handle_softirqs+0x1ce/0x800 kernel/softirq.c:554 common_interrupt+0x94/0xa0 arch/x86/kernel/irq.c:278 asm_common_interrupt+0x2b/0x40 arch/x86/include/asm/idtentry.h:693 kmsan_get_metadata+0x189/0x1d0 kmsan_get_shadow_origin_ptr+0x4d/0xb0 mm/kmsan/shadow.c:102 get_shadow_origin_ptr mm/kmsan/instrumentation.c:36 [inline] __msan_metadata_ptr_for_load_8+0x24/0x40 mm/kmsan/instrumentation.c:92 unwind_get_return_address_ptr+0x6a/0x100 arch/x86/kernel/unwind_frame.c:28 update_stack_state+0x206/0x270 arch/x86/kernel/unwind_frame.c:251 unwind_next_frame+0x19a/0x470 arch/x86/kernel/unwind_frame.c:315 arch_stack_walk+0x1ec/0x2d0 arch/x86/kernel/stacktrace.c:25 stack_trace_save+0xaa/0xe0 kernel/stacktrace.c:122 kmsan_save_stack_with_flags mm/kmsan/core.c:74 [inline] kmsan_internal_poison_memory+0x49/0x90 mm/kmsan/core.c:58 kmsan_slab_alloc+0xdf/0x160 mm/kmsan/hooks.c:68 slab_post_alloc_hook mm/slub.c:3947 [inline] slab_alloc_node mm/slub.c:4001 [inline] __do_kmalloc_node mm/slub.c:4121 [inline] __kmalloc_noprof+0x660/0xf30 mm/slub.c:4135 kmalloc_noprof include/linux/slab.h:664 [inline] tomoyo_realpath_from_path+0x104/0xaa0 security/tomoyo/realpath.c:251 tomoyo_get_realpath security/tomoyo/file.c:151 [inline] tomoyo_check_open_permission+0x1ef/0xc50 security/tomoyo/file.c:771 tomoyo_file_open+0x271/0x360 security/tomoyo/tomoyo.c:334 security_file_open+0x9a/0xc60 security/security.c:2962 do_dentry_open+0x5b1/0x22b0 fs/open.c:942 vfs_open+0x49/0x60 fs/open.c:1089 do_open fs/namei.c:3650 [inline] path_openat+0x4ab0/0x5b70 fs/namei.c:3807 do_filp_open+0x20e/0x590 fs/namei.c:3834 do_sys_openat2+0x1bf/0x2f0 fs/open.c:1405 do_sys_open fs/open.c:1420 [inline] __do_sys_openat fs/open.c:1436 [inline] __se_sys_openat fs/open.c:1431 [inline] __x64_sys_openat+0x2a1/0x310 fs/open.c:1431 x64_sys_call+0x128b/0x3b90 arch/x86/include/generated/asm/syscalls_64.h:258 do_syscall_x64 arch/x86/entry/common.c:52 [inline] do_syscall_64+0xcd/0x1e0 arch/x86/entry/common.c:83 entry_SYSCALL_64_after_hwframe+0x77/0x7f Uninit was created at: __alloc_pages_noprof+0x9d6/0xe70 mm/page_alloc.c:4701 alloc_pages_mpol_noprof+0x299/0x990 mm/mempolicy.c:2265 alloc_pages_noprof+0x1bf/0x1e0 mm/mempolicy.c:2336 skb_page_frag_refill+0x2bf/0x7c0 net/core/sock.c:2920 virtnet_rq_alloc+0x43/0xbb0 drivers/net/virtio_net.c:882 add_recvbuf_mergeable drivers/net/virtio_net.c:2128 [inline] try_fill_recv+0x3f0/0x2f50 drivers/net/virtio_net.c:2173 virtnet_open+0x1cc/0xb00 drivers/net/virtio_net.c:2452 __dev_open+0x546/0x6f0 net/core/dev.c:1472 __dev_change_flags+0x309/0x9a0 net/core/dev.c:8781 dev_change_flags+0x8e/0x1d0 net/core/dev.c:8853 devinet_ioctl+0x13ec/0x22c0 net/ipv4/devinet.c:1177 inet_ioctl+0x4bd/0x6d0 net/ipv4/af_inet.c:1003 sock_do_ioctl+0xb7/0x540 net/socket.c:1222 sock_ioctl+0x727/0xd70 net/socket.c:1341 vfs_ioctl fs/ioctl.c:51 [inline] __do_sys_ioctl fs/ioctl.c:907 [inline] __se_sys_ioctl+0x261/0x450 fs/ioctl.c:893 __x64_sys_ioctl+0x96/0xe0 fs/ioctl.c:893 x64_sys_call+0x18c0/0x3b90 arch/x86/include/generated/asm/syscalls_64.h:17 do_syscall_x64 arch/x86/entry/common.c:52 [inline] do_syscall_64+0xcd/0x1e0 arch/x86/entry/common.c:83 entry_SYSCALL_64_after_hwframe+0x77/0x7f CPU: 0 PID: 4794 Comm: rm Not tainted 6.10.0-rc5-syzkaller-00012-g626737a5791b #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 06/07/2024 = --- This report is generated by a bot. It may contain errors. See https://goo.gl/tpsmEJ for more information about syzbot. syzbot engineers can be reached at syzkal...@googlegroups.com. syzbot will keep track of this issue.
Re: [PATCH] remoteproc: mediatek: Don't attempt to remap l1tcm memory if missing
On Thu, Jun 27, 2024 at 05:20:55PM -0400, Nícolas F. R. A. Prado wrote:
> The current code doesn't check whether platform_get_resource_byname()
> succeeded to get the l1tcm memory, which is optional, before attempting
> to map it. This results in the following error message when it is
> missing:
>
> mtk-scp 1050.scp: error -EINVAL: invalid resource (null)
>
> Add a check so that the remapping is only attempted if the memory region
> exists. This also allows to simplify the logic handling failure to
> remap, since a failure then is always a failure.
>
> Fixes: ca23ecfdbd44 ("remoteproc/mediatek: support L1TCM")
> Signed-off-by: Nícolas F. R. A. Prado
> ---
> drivers/remoteproc/mtk_scp.c | 12 +---
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c
> index b885a9a041e4..b17757900cd7 100644
> --- a/drivers/remoteproc/mtk_scp.c
> +++ b/drivers/remoteproc/mtk_scp.c
> @@ -1344,14 +1344,12 @@ static int scp_probe(struct platform_device *pdev)
>
> /* l1tcm is an optional memory region */
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "l1tcm");
> - scp_cluster->l1tcm_base = devm_ioremap_resource(dev, res);
> - if (IS_ERR(scp_cluster->l1tcm_base)) {
> - ret = PTR_ERR(scp_cluster->l1tcm_base);
> - if (ret != -EINVAL)
> - return dev_err_probe(dev, ret, "Failed to map l1tcm
> memory\n");
> + if (res) {
> + scp_cluster->l1tcm_base = devm_ioremap_resource(dev, res);
> + if (IS_ERR(scp_cluster->l1tcm_base))
> + return dev_err_probe(dev,
> PTR_ERR(scp_cluster->l1tcm_base),
> + "Failed to map l1tcm memory\n");
>
> - scp_cluster->l1tcm_base = NULL;
> - } else {
Much better - I have applied this patch.
Regards,
Mathieu
> scp_cluster->l1tcm_size = resource_size(res);
> scp_cluster->l1tcm_phys = res->start;
> }
>
> ---
> base-commit: 0fc4bfab2cd45f9acb86c4f04b5191e114e901ed
> change-id: 20240627-scp-invalid-resource-l1tcm-9f7cf45c17e6
>
> Best regards,
> --
> Nícolas F. R. A. Prado
>
Re: [PATCH v7 0/5] initial support for Marvell 88PM886 PMIC
Lee Jones, 2024-06-28T15:41:39+01:00: > On Fri, 31 May 2024 19:34:55 +0200, Karel Balej wrote: > > the following implements basic support for Marvell's 88PM886 PMIC which > > is found for instance as a component of the samsung,coreprimevelte > > smartphone which inspired this and also serves as a testing platform. > > > > The code for the MFD is based primarily on this old series [1] with the > > addition of poweroff based on the smartphone's downstream kernel tree > > [2]. The onkey and regulators drivers are based on the latter. I am not > > in possesion of the datasheet. > > > > [...] > > Applied, thanks! Thank you and thank you and everybody else for all the feedback and reviews, I appreciate it. K. B.
[PATCH] mailmap: Update Luca Weiss's email address
I'm slowly migrating my mail to a new domain, add an entry to map the mail address. Just for clarity, my work-related @fairphone.com email stays unchanged. Signed-off-by: Luca Weiss --- Since my email address also appears in a bunch of drivers and arm(64) files, and two devicetree binding files, how are those normally handled? Just ignore them and let mailmap handle everything relevant? --- .mailmap | 1 + 1 file changed, 1 insertion(+) diff --git a/.mailmap b/.mailmap index a6c619e22efc..e169a99ce7c7 100644 --- a/.mailmap +++ b/.mailmap @@ -385,6 +385,7 @@ Li Yang Lior David Lorenzo Pieralisi Luca Ceresoli +Luca Weiss Lukasz Luba Luo Jie Maciej W. Rozycki --- base-commit: 642a16ca7994a50d7de85715996a8ce171a5bdfb change-id: 20240628-mailmap-3528f7365abb Best regards, -- Luca Weiss
[PATCH] soc: qcom: smsm: Add missing mailbox dependency to Kconfig
Since the smsm driver got the ability to interact with the mailbox using
the mailbox subsystem and not just syscon, we need to add the dependency
to kconfig as well to avoid compile errors.
Fixes: 75287992f58a ("soc: qcom: smsm: Support using mailbox interface")
Reported-by: kernel test robot
Closes:
https://lore.kernel.org/oe-kbuild-all/202406180006.z397c67h-...@intel.com/
Signed-off-by: Luca Weiss
---
drivers/soc/qcom/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index 5af33b0e3470..60efecd16380 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -194,6 +194,7 @@ config QCOM_SMP2P
config QCOM_SMSM
tristate "Qualcomm Shared Memory State Machine"
+ depends on MAILBOX
depends on QCOM_SMEM
select QCOM_SMEM_STATE
select IRQ_DOMAIN
---
base-commit: 642a16ca7994a50d7de85715996a8ce171a5bdfb
change-id: 20240628-smsm-kconfig-6a01783472f0
Best regards,
--
Luca Weiss
Re: [PATCH V3 2/2] soc: qcom: smp2p: Introduce tracepoint support
Hi Sudeepgoud,
kernel test robot noticed the following build errors:
[auto build test ERROR on linus/master]
[also build test ERROR on v6.10-rc5 next-20240627]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url:
https://github.com/intel-lab-lkp/linux/commits/Sudeepgoud-Patil/soc-qcom-smp2p-Use-devname-for-interrupt-descriptions/20240628-061654
base: linus/master
patch link:
https://lore.kernel.org/r/20240627104831.4176799-3-quic_sudeepgo%40quicinc.com
patch subject: [PATCH V3 2/2] soc: qcom: smp2p: Introduce tracepoint support
config: arc-allmodconfig
(https://download.01.org/0day-ci/archive/20240629/202406290037.kajgvuwb-...@intel.com/config)
compiler: arceb-elf-gcc (GCC) 13.2.0
reproduce (this is a W=1 build):
(https://download.01.org/0day-ci/archive/20240629/202406290037.kajgvuwb-...@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot
| Closes:
https://lore.kernel.org/oe-kbuild-all/202406290037.kajgvuwb-...@intel.com/
All errors (new ones prefixed by >>):
In file included from include/trace/trace_events.h:419,
from include/trace/define_trace.h:102,
from drivers/soc/qcom/trace-smp2p.h:98,
from drivers/soc/qcom/smp2p.c:165:
>> drivers/soc/qcom/./trace-smp2p.h:25:1: error: macro "__assign_str" passed 2
>> arguments, but takes just 1
25 | );
| ^~
In file included from include/trace/trace_events.h:375:
include/trace/stages/stage6_event_callback.h:34: note: macro "__assign_str"
defined here
34 | #define __assign_str(dst)
\
|
drivers/soc/qcom/./trace-smp2p.h: In function
'trace_event_raw_event_smp2p_ssr_ack':
>> drivers/soc/qcom/./trace-smp2p.h:22:17: error: '__assign_str' undeclared
>> (first use in this function)
22 | __assign_str(dev_name, dev_name(dev));
| ^~~~
include/trace/trace_events.h:402:11: note: in definition of macro
'DECLARE_EVENT_CLASS'
402 | { assign; }
\
| ^~
include/trace/trace_events.h:44:30: note: in expansion of macro 'PARAMS'
44 | PARAMS(assign), \
| ^~
drivers/soc/qcom/./trace-smp2p.h:15:1: note: in expansion of macro
'TRACE_EVENT'
15 | TRACE_EVENT(smp2p_ssr_ack,
| ^~~
drivers/soc/qcom/./trace-smp2p.h:21:9: note: in expansion of macro
'TP_fast_assign'
21 | TP_fast_assign(
| ^~
drivers/soc/qcom/./trace-smp2p.h:22:17: note: each undeclared identifier is
reported only once for each function it appears in
22 | __assign_str(dev_name, dev_name(dev));
| ^~~~
include/trace/trace_events.h:402:11: note: in definition of macro
'DECLARE_EVENT_CLASS'
402 | { assign; }
\
| ^~
include/trace/trace_events.h:44:30: note: in expansion of macro 'PARAMS'
44 | PARAMS(assign), \
| ^~
drivers/soc/qcom/./trace-smp2p.h:15:1: note: in expansion of macro
'TRACE_EVENT'
15 | TRACE_EVENT(smp2p_ssr_ack,
| ^~~
drivers/soc/qcom/./trace-smp2p.h:21:9: note: in expansion of macro
'TP_fast_assign'
21 | TP_fast_assign(
| ^~
drivers/soc/qcom/./trace-smp2p.h: At top level:
drivers/soc/qcom/./trace-smp2p.h:42:1: error: macro "__assign_str" passed 2
arguments, but takes just 1
42 | );
| ^~
include/trace/stages/stage6_event_callback.h:34: note: macro "__assign_str"
defined here
34 | #define __assign_str(dst)
\
|
drivers/soc/qcom/./trace-smp2p.h: In function
'trace_event_raw_event_smp2p_negotiate':
drivers/soc/qcom/./trace-smp2p.h:35:17: error: '__assign_str' undeclared
(first use in this function)
35 | __assign_str(dev_name, dev_name(dev));
| ^~~~
include/trace/trace_events.h:402:11: note: in definition of macro
'DECLARE_EVENT_CLASS'
402 | { assign; }
\
| ^~
include/trace/trace_events.h:44:30: note: in expansion of macro 'PARAMS'
44 | PARAMS(assign),
Re: [PATCH] kallsyms, livepatch: Fix livepatch with CONFIG_LTO_CLANG
On Fri, Jun 28, 2024 at 02:23:49PM +0200, Miroslav Benes wrote: > On Fri, 7 Jun 2024, Song Liu wrote: > > > Hi Miroslav, > > > > Thanks for reviewing the patch! > > > > On Fri, Jun 7, 2024 at 6:06 AM Miroslav Benes wrote: > > > > > > Hi, > > > > > > On Tue, 4 Jun 2024, Song Liu wrote: > > > > > > > With CONFIG_LTO_CLANG, the compiler may postfix symbols with > > > > .llvm. > > > > to avoid symbol duplication. scripts/kallsyms.c sorted the symbols > > > > without these postfixes. The default symbol lookup also removes these > > > > postfixes before comparing symbols. > > > > > > > > On the other hand, livepatch need to look up symbols with the full > > > > names. > > > > However, calling kallsyms_on_each_match_symbol with full name (with the > > > > postfix) cannot find the symbol(s). As a result, we cannot livepatch > > > > kernel functions with .llvm. postfix or kernel functions that use > > > > relocation information to symbols with .llvm. postfixes. > > > > > > > > Fix this by calling kallsyms_on_each_match_symbol without the postfix; > > > > and then match the full name (with postfix) in klp_match_callback. > > > > > > > > Signed-off-by: Song Liu > > > > --- > > > > include/linux/kallsyms.h | 13 + > > > > kernel/kallsyms.c| 21 - > > > > kernel/livepatch/core.c | 32 +++- > > > > 3 files changed, 60 insertions(+), 6 deletions(-) > > > > > > I do not like much that something which seems to be kallsyms-internal is > > > leaked out. You need to export cleanup_symbol_name() and there is now a > > > lot of code outside. I would feel much more comfortable if it is all > > > hidden from kallsyms users and kept there. Would it be possible? > > > > I think it is possible. Currently, kallsyms_on_each_match_symbol matches > > symbols without the postfix. We can add a variation or a parameter, so > > that it matches the full name with post fix. > > I think it might be better. > > Luis, what is your take on this? > > If I am not mistaken, there was a patch set to address this. Luis might > remember more. Yeah this is a real issue outside of CONFIG_LTO_CLANG, Rust modules is another example where instead of symbol names they want to use full hashes. So, as I hinted to you Sami, can we knock two birds with one stone here and move CONFIG_LTO_CLANG to use the same strategy as Rust so we have two users instead of just one? Then we resolve this. In fact what I suggested was even to allow even non-Rust, and in this case even with gcc to enable this world. This gives much more wider scope of testing / review / impact of these sorts of changes and world view and it would resolve the Rust case, the live patch CONFIG_LTO_CLANG world too. Thoughts? Luis
Re: [PATCH 6.10.0-rc2] kernel/module: avoid panic on loading broken module
On Fri, Jun 21, 2024 at 04:05:27PM +0200, Daniel von Kirschten wrote: > Am 18.06.2024 um 21:58 schrieb Luis Chamberlain: > > On Thu, Jun 06, 2024 at 03:31:49PM +0200, Daniel v. Kirschten wrote: > > > If a module is being loaded, and the .gnu.linkonce.this_module section > > > in the module's ELF file does not have the WRITE flag, the kernel will > > > map the finished module struct of that module as read-only. > > > This causes a kernel panic when the struct is written to the first time > > > after it has been marked read-only. Currently this happens in > > > complete_formation in kernel/module/main.c:2765 when the module's state is > > > set to MODULE_STATE_COMING, just after setting up the memory protections. > > > > How did you find this issue? > > In a university course I got the assignment to manually craft a loadable .ko > file, given only a regular object file, without using Kbuild. During testing > my module files, most of them were simply (correctly) rejected by the kernel > with an appropriate error message, but at some point I ran into this exact > kernel panic, and investigated it to understand why my module file was > invalid. OK, then the commit log should describe that this doesn't fix any known real world issue, but rather a custom crafted module without the regular module build system. > > > Down the line, this seems to lead to unpredictable freezes when trying to > > > load other modules - I guess this is due to some structures not being > > > cleaned up properly, but I didn't investigate this further. > > > > > > A check already exists which verifies that .gnu.linkonce.this_module > > > is ALLOC. This patch simply adds an analogous check for WRITE. > > > > Can you check to ensure our modules generated have a respective check to > > ensure this check exists at build time? That would proactively inform > > userspace when a built module is not built correctly, and the tool > > responsible can be identified. > > See above - I don't think it's possible to create such a broken module file > with any of "official" tools. That should be clearly stated on the commit log. > I haven't looked too deeply into how Kbuild > actually builds modules, but as far as I know, the user doesn't even come > into contact with this_module w Consider that a next level university assignment and is more useful to the world than this debug message. Because above you suggest "I don't think", go out and now be sure. > hen using the regular toolchain, because > Kbuild is responsible for creating the .this_module section. And Kbuild of > course creates it with the correct flags. So if I understand correctly, ... > this > problem can only occur when the module was built by some external tooling > (or manually, in my case). Who would create custom modules without the Linux kernel module build system, and what uses does that provide? It seems you are proving why this would be terribly silly thing to do. Now, the *value* your change has is it can prevent a crash in case of a corrupted module, which *can* occur, consider an odd filesystem live corruption, at least this would be caught at module load attempt and not crash. That's worth committing for this reason but your commit log really needs much more clarity. Why? Because stupid bots want to assign stupid CVEs for anything that seems like a security issue and this could escalate to such type of things. Providing clarity helps system integrators decide if they want to backport this sort of patch. Providing clarify on the chances of this happening and how we think it can happen helps a lot. If you want to be more proactive, try to enhance userspace kmod modprobe so that this is also verified. Luis
Re: [PATCH v3] module: Add log info for verifying module signature
On Fri, Jun 28, 2024 at 10:39:23AM +, Yusong Gao wrote:
> Add log information in kernel-space when loading module failures.
> Try to load the unsigned module and the module with bad signature
> when set 1 to /sys/module/module/parameters/sig_enforce.
>
> Unsigned module case:
> (linux) insmod unsigned.ko
> [ 18.714661] Loading of unsigned module is rejected
> insmod: can't insert 'unsigned.ko': Key was rejected by service
> (linux)
>
> Bad signature module case:
> (linux) insmod bad_signature.ko
> insmod: can't insert 'bad_signature.ko': Key was rejected by service
> (linux)
>
> There have different logging behavior the bad signature case only log
> in user-space, add log info for fatal errors in module_sig_check().
>
> Signed-off-by: Yusong Gao
> ---
> V3: Clarify the message type and the error code meaning.
> V2: Change print level from notice to debug.
> ---
> kernel/module/signing.c | 27 +++
> 1 file changed, 27 insertions(+)
>
> diff --git a/kernel/module/signing.c b/kernel/module/signing.c
> index a2ff4242e623..826cdab8e3e4 100644
> --- a/kernel/module/signing.c
> +++ b/kernel/module/signing.c
> @@ -67,6 +67,31 @@ int mod_verify_sig(const void *mod, struct load_info *info)
> NULL, NULL);
> }
>
> +static const char *mod_decode_error(int errno)
> +{
> + char *errstr = "Unrecognized error";
This is not safe. You can just extend the existing debug switch for
strict module loading and re-use the variable there and use that,
for example
diff --git a/kernel/module/signing.c b/kernel/module/signing.c
index a2ff4242e623..9111822116e6 100644
--- a/kernel/module/signing.c
+++ b/kernel/module/signing.c
@@ -106,6 +106,9 @@ int module_sig_check(struct load_info *info, int flags)
case -ENOKEY:
reason = "module with unavailable key";
break;
+ case -EKEYREJECTED:
+ reason = "Key was rejected by service";
+ break;
default:
/*
@@ -113,6 +116,7 @@ int module_sig_check(struct load_info *info, int flags)
* unparseable signatures, and signature check failures --
* even if signatures aren't required.
*/
+ pr_debug("Verifying module signature failed: %s\n", reason);
return err;
}
Also certs/system_keyring.c already has a lot of pr_devel stuff too, so
do we really need this?
Luis
Re: [RFC PATCH v2] ptp: Add vDSO-style vmclock support
On 28.06.24 14:15, David Woodhouse wrote: > On Fri, 2024-06-28 at 13:33 +0200, Peter Hilber wrote: >> On 27.06.24 16:52, David Woodhouse wrote: >>> I already added a flags field, so this might look something like: >>> >>> /* >>> * Smearing flags. The UTC clock exposed through this structure >>> * is only ever true UTC, but a guest operating system may >>> * choose to offer a monotonic smeared clock to its users. This >>> * merely offers a hint about what kind of smearing to perform, >>> * for consistency with systems in the nearby environment. >>> */ >>> #define VMCLOCK_FLAGS_SMEAR_UTC_SLS (1<<5) /* draft-kuhn-leapsecond-00.txt >>> */ >>> >>> (UTC-SLS is probably a bad example but are there formal definitions for >>> anything else?) >> >> I think it could also be more generic, like flags for linear smearing, >> cosine smearing(?), and smear_start_sec and smear_end_sec fields (relative >> to the leap second start). That could also represent UTC-SLS, and >> noon-to-noon, and it would be well-defined. >> >> This should reduce the likelihood that the guest doesn't know the smearing >> variant. > > I'm wary of making it too generic. That would seem to encourage a > *proliferation* of false "UTC-like" clocks. > > It's bad enough that we do smearing at all, let alone that we don't > have a single definition of how to do it. > > I made the smearing hint a full uint8_t instead of using bits in flags, > in the end. That gives us a full 255 ways of lying to users about what > the time is, so we're unlikely to run out. And it's easy enough to add > a new VMCLOCK_SMEARING_XXX type to the 'registry' for any new methods > that get invented. > > My concern is that the registry update may come after a driver has already been implemented, so that it may be hard to ensure that the smearing which has been chosen is actually implemented. > + /* > + * This field changes to another non-repeating value when the CPU > + * counter is disrupted, for example on live migration. > + */ > + uint64_t disruption_marker; The field could also change when the clock is stepped (leap seconds excepted), or when the clock frequency is slewed. >>> >>> I'm not sure. The concept of the disruption marker is that it tells the >>> guest to throw away any calibration of the counter that the guest has >>> done for *itself* (with NTP, other PTP devices, etc.). >>> >>> One mode for this device would be not to populate the clock fields at >>> all, but *only* to signal disruption when it occurs. So the guest can >>> abort transactions until it's resynced its clocks (to avoid incurring >>> fines if breaking databases, etc.). >>> >>> Exposing the host timekeeping through the structure means that the >>> migrated guest can keep working because it can trust the timekeeping >>> performed by the (new) host and exposed to it. >>> >>> If the counter is actually varying in frequency over time, and the host >>> is slewing the clock frequency that it reports, that *isn't* a step >>> change and doesn't mean that the guest should throw away any >>> calibration that it's been doing for itself. One hopes that the guest >>> would have detected the *same* frequency change, and be adapting for >>> itself. So I don't think that should indicate a disruption. >>> >>> I think the same is even true if the clock is stepped by the host. The >>> actual *counter* hasn't changed, so the guest is better off ignoring >>> the vacillating host and continuing to derive its idea of time from the >>> hardware counter itself, as calibrated against some external NTP/PTP >>> sources. Surely we actively *don't* to tell the guest to throw its own >>> calibrations away, in this case? >> >> In case the guest is also considering other time sources, it might indeed >> not be a good idea to mix host clock changes into the hardware counter >> disruption marker. >> >> But if the vmclock is the authoritative source of time, it can still be >> helpful to know about such changes, maybe through another marker. > > Could that be the existing seq_count field? > > Skewing the counter_period_frac_sec as the underlying oscillator speeds > up and slows down is perfectly normal and expected, and we already > expect the seq_count to change when that happens. > > Maybe step changes are different, but arguably if the time advertised > by the host steps *outside* the error bounds previously advertised, > that's just broken? But the error bounds could be large or missing. I am trying to address use cases where the host steps or slews the clock as well. > > Depending on how the clock information is fed, a change in seq_count > may even result in non-monotonicity. If the underlying oscillator has > sped up and the structure is updated accordingly, the time calculated > the moment *before* that update may appear later than the time > calculated immediately after it. > > It's up
Re: [PATCH 06/12] uprobes: add batch uprobe register/unregister APIs
On Thu, Jun 27, 2024 at 11:28 PM Masami Hiramatsu wrote: > > On Thu, 27 Jun 2024 09:47:10 -0700 > Andrii Nakryiko wrote: > > > On Thu, Jun 27, 2024 at 6:04 AM Masami Hiramatsu > > wrote: > > > > > > On Mon, 24 Jun 2024 17:21:38 -0700 > > > Andrii Nakryiko wrote: > > > > > > > -static int __uprobe_register(struct inode *inode, loff_t offset, > > > > - loff_t ref_ctr_offset, struct > > > > uprobe_consumer *uc) > > > > +int uprobe_register_batch(struct inode *inode, int cnt, > > > > + uprobe_consumer_fn get_uprobe_consumer, void > > > > *ctx) > > > > > > Is this interface just for avoiding memory allocation? Can't we just > > > allocate a temporary array of *uprobe_consumer instead? > > > > Yes, exactly, to avoid the need for allocating another array that > > would just contain pointers to uprobe_consumer. Consumers would never > > just have an array of `struct uprobe_consumer *`, because > > uprobe_consumer struct is embedded in some other struct, so the array > > interface isn't the most convenient. > > OK, I understand it. > > > > > If you feel strongly, I can do an array, but this necessitates > > allocating an extra array *and keeping it* for the entire duration of > > BPF multi-uprobe link (attachment) existence, so it feels like a > > waste. This is because we don't want to do anything that can fail in > > the detachment logic (so no temporary array allocation there). > > No need to change it, that sounds reasonable. > Great, thanks. > > > > Anyways, let me know how you feel about keeping this callback. > > IMHO, maybe the interface function is better to change to > `uprobe_consumer *next_uprobe_consumer(void **data)`. If caller > side uses a linked list of structure, index access will need to > follow the list every time. This would be problematic. Note how we call get_uprobe_consumer(i, ctx) with i going from 0 to N in multiple independent loops. So if we are only allowed to ask for the next consumer, then uprobe_register_batch and uprobe_unregister_batch would need to build its own internal index and remember ith instance. Which again means more allocations and possibly failing uprobe_unregister_batch(), which isn't great. For now this API works well, I propose to keep it as is. For linked list case consumers would need to allocate one extra array or pay the price of O(N) search (which might be ok, depending on how many uprobes are being attached). But we don't have such consumers right now, thankfully. > > Thank you, > > > > > > > > > > Thank you, > > > > > > -- > > > Masami Hiramatsu (Google) > > > -- > Masami Hiramatsu (Google)
Re: [PATCH v2 0/2] ARM: dts: qcom-msm8226-samsung-ms013g: Add initial device tree
On Thu, 27 Jun 2024 19:30:30 +, Raymond Hackley wrote: > Samsung Galaxy Grand 2 is a phone based on MSM8226. It's similar to the > other Samsung devices based on MSM8226 with only a few minor differences. > > The device trees contain initial support with: > - GPIO keys > - Regulator haptic > - SDHCI (internal and external storage) > - UART (on USB connector via the TI TSU6721 MUIC) > - Regulators > - Touchscreen > - Accelerometer > > --- > v2: Adjust l3, l15, l22 and l27 regulator voltages. Sort nodes. > Set regulator-allow-set-load for vqmmc supplies. > > > > My bot found new DTB warnings on the .dts files added or changed in this series. Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings are fixed by another series. Ultimately, it is up to the platform maintainer whether these warnings are acceptable or not. No need to reply unless the platform maintainer has comments. If you already ran DT checks and didn't see these error(s), then make sure dt-schema is up to date: pip3 install dtschema --upgrade New warnings running 'make CHECK_DTBS=y qcom/qcom-msm8226-samsung-ms013g.dtb' for 20240627193013.1800-1-raymondhack...@protonmail.com: arch/arm/boot/dts/qcom/qcom-msm8226-samsung-ms013g.dtb: syscon@f9011000: compatible: 'anyOf' conditional failed, one must be fixed: ['syscon'] is too short 'syscon' is not one of ['al,alpine-sysfabric-service', 'allwinner,sun8i-a83t-system-controller', 'allwinner,sun8i-h3-system-controller', 'allwinner,sun8i-v3s-system-controller', 'allwinner,sun50i-a64-system-controller', 'altr,l3regs', 'altr,sdr-ctl', 'amd,pensando-elba-syscon', 'amlogic,meson-mx-assist', 'amlogic,meson-mx-bootrom', 'amlogic,meson8-analog-top', 'amlogic,meson8b-analog-top', 'amlogic,meson8-pmu', 'amlogic,meson8b-pmu', 'apm,xgene-csw', 'apm,xgene-efuse', 'apm,xgene-mcb', 'apm,xgene-rb', 'apm,xgene-scu', 'atmel,sama5d2-sfrbu', 'atmel,sama5d3-nfc-io', 'atmel,sama5d3-sfrbu', 'atmel,sama5d4-sfrbu', 'axis,artpec6-syscon', 'brcm,cru-clkset', 'brcm,sr-cdru', 'brcm,sr-mhb', 'cirrus,ep7209-syscon1', 'cirrus,ep7209-syscon2', 'cirrus,ep7209-syscon3', 'cnxt,cx92755-uc', 'freecom,fsg-cs2-system-controller', 'fsl,imx93-aonmix-ns-syscfg', 'fsl,imx93-wakeupmix-syscfg', 'fsl,ls1088a-reset', 'fsl,vf610-anatop', 'fsl,vf610-mscm-cpucfg', 'hisilicon,dsa-subctrl', 'hisilicon,hi6220-sramctr l', 'hisilicon,hip04-ppe', 'hisilicon,pcie-sas-subctrl', 'hisilicon,peri-subctrl', 'hpe,gxp-sysreg', 'intel,lgm-syscon', 'loongson,ls1b-syscon', 'loongson,ls1c-syscon', 'lsi,axxia-syscon', 'marvell,armada-3700-cpu-misc', 'marvell,armada-3700-nb-pm', 'marvell,armada-3700-avs', 'marvell,armada-3700-usb2-host-misc', 'marvell,dove-global-config', 'mediatek,mt2701-pctl-a-syscfg', 'mediatek,mt2712-pctl-a-syscfg', 'mediatek,mt6397-pctl-pmic-syscfg', 'mediatek,mt8135-pctl-a-syscfg', 'mediatek,mt8135-pctl-b-syscfg', 'mediatek,mt8173-pctl-a-syscfg', 'mediatek,mt8365-syscfg', 'microchip,lan966x-cpu-syscon', 'microchip,sam9x60-sfr', 'microchip,sama7g5-ddr3phy', 'microchip,sparx5-cpu-syscon', 'mscc,ocelot-cpu-syscon', 'mstar,msc313-pmsleep', 'nuvoton,ma35d1-sys', 'nuvoton,wpcm450-shm', 'rockchip,px30-qos', 'rockchip,rk3036-qos', 'rockchip,rk3066-qos', 'rockchip,rk3128-qos', 'rockchip,rk3228-qos', 'rockchip,rk3288-qos', 'rockchip,rk3368-qos', 'rockchip,rk3399-qos', 'rockchip,rk3568-qos', 'rockchi p,rk3588-qos', 'rockchip,rv1126-qos', 'st,spear1340-misc', 'stericsson,nomadik-pmu', 'starfive,jh7100-sysmain', 'ti,am62-opp-efuse-table', 'ti,am62-usb-phy-ctrl', 'ti,am625-dss-oldi-io-ctrl', 'ti,am62p-cpsw-mac-efuse', 'ti,am654-dss-oldi-io-ctrl', 'ti,am654-serdes-ctrl', 'ti,j784s4-pcie-ctrl', 'ti,keystone-pllctrl'] from schema $id: http://devicetree.org/schemas/mfd/syscon.yaml#
Re: [PATCH v3 2/2] rust: add tracepoint support
On Wed, Jun 26, 2024 at 8:43 PM Steven Rostedt wrote: > > On Wed, 26 Jun 2024 10:48:23 +0200 > Alice Ryhl wrote: > > > > > > > Because your hooks/rust_binder.h and events/rust_binder.h use the same > > > TRACE_SYSTEM name? Could you try something like: > > > > > > #define TRACE_SYSTEM rust_binder_hook > > > > > > in your hooks/rust_binder.h? > > > > I was able to get it to work by moving the includes into two different > > .c files. I don't think changing TRACE_SYSTEM works because it must > > match the filename. > > Try to use: > > #define TRACE_SYSTEM_VAR rust_binder_hook_other_name > > in one. Then that is used as the variable for that file. Thanks. I also made a change to restore the value of DEFINE_RUST_DO_TRACE after define_trace.h Alice
Re: [PATCH v7 0/5] initial support for Marvell 88PM886 PMIC
On Fri, 28 Jun 2024, Lee Jones wrote: > On Fri, 31 May 2024 19:34:55 +0200, Karel Balej wrote: > > the following implements basic support for Marvell's 88PM886 PMIC which > > is found for instance as a component of the samsung,coreprimevelte > > smartphone which inspired this and also serves as a testing platform. > > > > The code for the MFD is based primarily on this old series [1] with the > > addition of poweroff based on the smartphone's downstream kernel tree > > [2]. The onkey and regulators drivers are based on the latter. I am not > > in possesion of the datasheet. > > > > [...] > > Applied, thanks! > > [1/5] dt-bindings: mfd: add entry for Marvell 88PM886 PMIC > commit: c4725350a9f76fbec45cbbfffb20be2e574eb6ef > [2/5] mfd: add driver for Marvell 88PM886 PMIC > commit: 860f8e3beac0b800bbe20f23c5f3440b1c470b8f > [3/5] regulator: add regulators driver for Marvell 88PM886 PMIC > commit: 5d1a5144396e9570efea02d467df0a68fd28db6f > [4/5] input: add onkey driver for Marvell 88PM886 PMIC > commit: 914089db309ccc590314b6c21df5a1f812e9ab0b > [5/5] MAINTAINERS: add myself for Marvell 88PM886 PMIC > commit: f53d3efa366b1754f0389944401bb53397d22468 Submitted for build testing. If all is good, I'll send out a PR for the other maintainers soon. Note to self: ib-mfd-input-regulator-6.11 -- Lee Jones [李琼斯]
Re: [PATCH v7 0/5] initial support for Marvell 88PM886 PMIC
On Fri, 31 May 2024 19:34:55 +0200, Karel Balej wrote: > the following implements basic support for Marvell's 88PM886 PMIC which > is found for instance as a component of the samsung,coreprimevelte > smartphone which inspired this and also serves as a testing platform. > > The code for the MFD is based primarily on this old series [1] with the > addition of poweroff based on the smartphone's downstream kernel tree > [2]. The onkey and regulators drivers are based on the latter. I am not > in possesion of the datasheet. > > [...] Applied, thanks! [1/5] dt-bindings: mfd: add entry for Marvell 88PM886 PMIC commit: c4725350a9f76fbec45cbbfffb20be2e574eb6ef [2/5] mfd: add driver for Marvell 88PM886 PMIC commit: 860f8e3beac0b800bbe20f23c5f3440b1c470b8f [3/5] regulator: add regulators driver for Marvell 88PM886 PMIC commit: 5d1a5144396e9570efea02d467df0a68fd28db6f [4/5] input: add onkey driver for Marvell 88PM886 PMIC commit: 914089db309ccc590314b6c21df5a1f812e9ab0b [5/5] MAINTAINERS: add myself for Marvell 88PM886 PMIC commit: f53d3efa366b1754f0389944401bb53397d22468 -- Lee Jones [李琼斯]
Re: [PATCH net-next v3 1/3] vsock: add support for SIOCOUTQ ioctl for all vsock socket types.
Hi Luigi,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 50b70845fc5c22cf7e7d25b57d57b3dca1725aa5]
url:
https://github.com/intel-lab-lkp/linux/commits/Luigi-Leonardi-via-B4-Relay/vsock-add-support-for-SIOCOUTQ-ioctl-for-all-vsock-socket-types/20240627-023902
base: 50b70845fc5c22cf7e7d25b57d57b3dca1725aa5
patch link:
https://lore.kernel.org/r/20240626-ioctl_next-v3-1-63be5bf19a40%40outlook.com
patch subject: [PATCH net-next v3 1/3] vsock: add support for SIOCOUTQ ioctl
for all vsock socket types.
config: i386-randconfig-141-20240628
(https://download.01.org/0day-ci/archive/20240628/202406282144.dxr5kwiu-...@intel.com/config)
compiler: gcc-8 (Ubuntu 8.4.0-3ubuntu2) 8.4.0
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot
| Closes:
https://lore.kernel.org/oe-kbuild-all/202406282144.dxr5kwiu-...@intel.com/
smatch warnings:
net/vmw_vsock/af_vsock.c:1321 vsock_do_ioctl() warn: unsigned 'n_bytes' is
never less than zero.
vim +/n_bytes +1321 net/vmw_vsock/af_vsock.c
1295
1296 static int vsock_do_ioctl(struct socket *sock, unsigned int cmd,
1297int __user *arg)
1298 {
1299 struct sock *sk = sock->sk;
1300 struct vsock_sock *vsk;
1301 int retval;
1302
1303 vsk = vsock_sk(sk);
1304
1305 switch (cmd) {
1306 case SIOCOUTQ: {
1307 size_t n_bytes;
1308
1309 if (!vsk->transport || !vsk->transport->unsent_bytes) {
1310 retval = -EOPNOTSUPP;
1311 break;
1312 }
1313
1314 if (vsk->transport->unsent_bytes) {
1315 if (sock_type_connectible(sk->sk_type) &&
sk->sk_state == TCP_LISTEN) {
1316 retval = -EINVAL;
1317 break;
1318 }
1319
1320 n_bytes = vsk->transport->unsent_bytes(vsk);
> 1321 if (n_bytes < 0) {
1322 retval = n_bytes;
1323 break;
1324 }
1325
1326 retval = put_user(n_bytes, arg);
1327 }
1328 break;
1329 }
1330 default:
1331 retval = -ENOIOCTLCMD;
1332 }
1333
1334 return retval;
1335 }
1336
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
[PATCH v4 2/2] rust: add tracepoint support
Make it possible to have Rust code call into tracepoints defined by C
code. It is still required that the tracepoint is declared in a C
header, and that this header is included in the input to bindgen.
Signed-off-by: Alice Ryhl
---
include/linux/tracepoint.h | 18 +++-
include/trace/define_trace.h| 12 +++
rust/bindings/bindings_helper.h | 1 +
rust/kernel/lib.rs | 1 +
rust/kernel/tracepoint.rs | 47 +
5 files changed, 78 insertions(+), 1 deletion(-)
diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index 689b6d71590e..d82af4d77c9f 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -238,6 +238,20 @@ static inline struct tracepoint
*tracepoint_ptr_deref(tracepoint_ptr_t *p)
#define __DECLARE_TRACE_RCU(name, proto, args, cond)
#endif
+/*
+ * Declare an exported function that Rust code can call to trigger this
+ * tracepoint. This function does not include the static branch; that is done
+ * in Rust to avoid a function call when the tracepoint is disabled.
+ */
+#define DEFINE_RUST_DO_TRACE(name, proto, args)
+#define DEFINE_RUST_DO_TRACE_REAL(name, proto, args) \
+ notrace void rust_do_trace_##name(proto)\
+ { \
+ __DO_TRACE(name,\
+ TP_ARGS(args), \
+ cpu_online(raw_smp_processor_id()), 0); \
+ }
+
/*
* Make sure the alignment of the structure in the __tracepoints section will
* not add unwanted padding between the beginning of the section and the
@@ -253,6 +267,7 @@ static inline struct tracepoint
*tracepoint_ptr_deref(tracepoint_ptr_t *p)
extern int __traceiter_##name(data_proto); \
DECLARE_STATIC_CALL(tp_func_##name, __traceiter_##name);\
extern struct tracepoint __tracepoint_##name; \
+ extern void rust_do_trace_##name(proto);\
static inline void trace_##name(proto) \
{ \
if (static_key_false(&__tracepoint_##name.key)) \
@@ -337,7 +352,8 @@ static inline struct tracepoint
*tracepoint_ptr_deref(tracepoint_ptr_t *p)
void __probestub_##_name(void *__data, proto) \
{ \
} \
- DEFINE_STATIC_CALL(tp_func_##_name, __traceiter_##_name);
+ DEFINE_STATIC_CALL(tp_func_##_name, __traceiter_##_name); \
+ DEFINE_RUST_DO_TRACE(_name, TP_PROTO(proto), TP_ARGS(args))
#define DEFINE_TRACE(name, proto, args)\
DEFINE_TRACE_FN(name, NULL, NULL, PARAMS(proto), PARAMS(args));
diff --git a/include/trace/define_trace.h b/include/trace/define_trace.h
index 00723935dcc7..08ed5ce63a96 100644
--- a/include/trace/define_trace.h
+++ b/include/trace/define_trace.h
@@ -72,6 +72,13 @@
#define DECLARE_TRACE(name, proto, args) \
DEFINE_TRACE(name, PARAMS(proto), PARAMS(args))
+/* If requested, create helpers for calling these tracepoints from Rust. */
+#ifdef CREATE_RUST_TRACE_POINTS
+#undef DEFINE_RUST_DO_TRACE
+#define DEFINE_RUST_DO_TRACE(name, proto, args)\
+ DEFINE_RUST_DO_TRACE_REAL(name, PARAMS(proto), PARAMS(args))
+#endif
+
#undef TRACE_INCLUDE
#undef __TRACE_INCLUDE
@@ -129,6 +136,11 @@
# undef UNDEF_TRACE_INCLUDE_PATH
#endif
+#ifdef CREATE_RUST_TRACE_POINTS
+# undef DEFINE_RUST_DO_TRACE
+# define DEFINE_RUST_DO_TRACE(name, proto, args)
+#endif
+
/* We may be processing more files */
#define CREATE_TRACE_POINTS
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index ddb5644d4fd9..d442f9ccfc2c 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -15,6 +15,7 @@
#include
#include
#include
+#include
#include
#include
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index fffd4e1dd1c1..9ae90eb69020 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -46,6 +46,7 @@
pub mod sync;
pub mod task;
pub mod time;
+pub mod tracepoint;
pub mod types;
pub mod workqueue;
diff --git a/rust/kernel/tracepoint.rs b/rust/kernel/tracepoint.rs
new file mode 100644
index ..1005f09e0330
--- /dev/null
+++ b/rust/kernel/tracepoint.rs
@@ -0,0 +1,47 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2024 Google LLC.
+
+//! Logic for tracepoints.
+
+/// Declare the Rust entry point for a tracepoint.
+#[macro_export]
+macro_rules! declare_trace {
+($($(#[$attr:meta])* $pub:vis fn $name:ident($($argname:ident :
$argtyp:ty),* $(,)?);)*) => {$(
+$(
[PATCH v4 1/2] rust: add static_key_false
Add just enough support for static key so that we can use it from
tracepoints. Tracepoints rely on `static_key_false` even though it is
deprecated, so we add the same functionality to Rust.
It is not possible to use the existing C implementation of
arch_static_branch because it passes the argument `key` to inline
assembly as an 'i' parameter, so any attempt to add a C helper for this
function will fail to compile because the value of `key` must be known
at compile-time.
Signed-off-by: Alice Ryhl
---
rust/kernel/arch/arm64/jump_label.rs | 34
rust/kernel/arch/loongarch/jump_label.rs | 35 +
rust/kernel/arch/mod.rs | 24
rust/kernel/arch/riscv/jump_label.rs | 38
rust/kernel/arch/x86/jump_label.rs | 35 +
rust/kernel/lib.rs | 2 ++
rust/kernel/static_key.rs| 32 +++
scripts/Makefile.build | 2 +-
8 files changed, 201 insertions(+), 1 deletion(-)
diff --git a/rust/kernel/arch/arm64/jump_label.rs
b/rust/kernel/arch/arm64/jump_label.rs
new file mode 100644
index ..5eede2245718
--- /dev/null
+++ b/rust/kernel/arch/arm64/jump_label.rs
@@ -0,0 +1,34 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2024 Google LLC.
+
+//! Arm64 Rust implementation of jump_label.h
+
+/// arm64 implementation of arch_static_branch
+#[macro_export]
+#[cfg(target_arch = "aarch64")]
+macro_rules! arch_static_branch {
+($key:path, $keytyp:ty, $field:ident, $branch:expr) => {'my_label: {
+core::arch::asm!(
+r#"
+1: nop
+
+.pushsection __jump_table, "aw"
+.align 3
+.long 1b - ., {0} - .
+.quad {1} + {2} + {3} - .
+.popsection
+"#,
+label {
+break 'my_label true;
+},
+sym $key,
+const ::core::mem::offset_of!($keytyp, $field),
+const $crate::arch::bool_to_int($branch),
+);
+
+break 'my_label false;
+}};
+}
+
+pub use arch_static_branch;
diff --git a/rust/kernel/arch/loongarch/jump_label.rs
b/rust/kernel/arch/loongarch/jump_label.rs
new file mode 100644
index ..8d31318aeb11
--- /dev/null
+++ b/rust/kernel/arch/loongarch/jump_label.rs
@@ -0,0 +1,35 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2024 Google LLC.
+
+//! Loongarch Rust implementation of jump_label.h
+
+/// loongarch implementation of arch_static_branch
+#[doc(hidden)]
+#[macro_export]
+#[cfg(target_arch = "loongarch64")]
+macro_rules! arch_static_branch {
+($key:path, $keytyp:ty, $field:ident, $branch:expr) => {'my_label: {
+core::arch::asm!(
+r#"
+1: nop
+
+.pushsection __jump_table, "aw"
+.align 3
+.long 1b - ., {0} - .
+.quad {1} + {2} + {3} - .
+.popsection
+"#,
+label {
+break 'my_label true;
+},
+sym $key,
+const ::core::mem::offset_of!($keytyp, $field),
+const $crate::arch::bool_to_int($branch),
+);
+
+break 'my_label false;
+}};
+}
+
+pub use arch_static_branch;
diff --git a/rust/kernel/arch/mod.rs b/rust/kernel/arch/mod.rs
new file mode 100644
index ..14271d2530e9
--- /dev/null
+++ b/rust/kernel/arch/mod.rs
@@ -0,0 +1,24 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2024 Google LLC.
+
+//! Architecture specific code.
+
+#[cfg_attr(target_arch = "aarch64", path = "arm64")]
+#[cfg_attr(target_arch = "x86_64", path = "x86")]
+#[cfg_attr(target_arch = "loongarch64", path = "loongarch")]
+#[cfg_attr(target_arch = "riscv64", path = "riscv")]
+mod inner {
+pub mod jump_label;
+}
+
+pub use self::inner::*;
+
+/// A helper used by inline assembly to pass a boolean to as a `const`
parameter.
+///
+/// Using this function instead of a cast lets you assert that the input is a
boolean, rather than
+/// some other type that can be cast to an integer.
+#[doc(hidden)]
+pub const fn bool_to_int(b: bool) -> i32 {
+b as i32
+}
diff --git a/rust/kernel/arch/riscv/jump_label.rs
b/rust/kernel/arch/riscv/jump_label.rs
new file mode 100644
index ..2672e0c6f033
--- /dev/null
+++ b/rust/kernel/arch/riscv/jump_label.rs
@@ -0,0 +1,38 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2024 Google LLC.
+
+//! RiscV Rust implementation of jump_label.h
+
+/// riscv implementation of arch_static_branch
+#[macro_export]
+#[cfg(target_arch = "riscv64")]
+macro_rules! arch_static_branch {
+($key:path, $keytyp:ty, $field:ident, $branch:expr) => {'my_label: {
+core::arch::asm!(
+r#"
+.align 2
+.option push
+.option norelax
+.option norvc
+1: nop
+
[PATCH v4 0/2] Tracepoints and static branch in Rust
An important part of a production ready Linux kernel driver is
tracepoints. So to write production ready Linux kernel drivers in Rust,
we must be able to call tracepoints from Rust code. This patch series
adds support for calling tracepoints declared in C from Rust.
To use the tracepoint support, you must:
1. Declare the tracepoint in a C header file as usual.
2. Add #define CREATE_RUST_TRACE_POINTS next to your
#define CREATE_TRACE_POINTS.
2. Make sure that the header file is visible to bindgen.
3. Use the declare_trace! macro in your Rust code to generate Rust
functions that call into the tracepoint.
For example, the kernel has a tracepoint called `sched_kthread_stop`. It
is declared like this:
TRACE_EVENT(sched_kthread_stop,
TP_PROTO(struct task_struct *t),
TP_ARGS(t),
TP_STRUCT__entry(
__array(char, comm, TASK_COMM_LEN )
__field(pid_t, pid )
),
TP_fast_assign(
memcpy(__entry->comm, t->comm, TASK_COMM_LEN);
__entry->pid= t->pid;
),
TP_printk("comm=%s pid=%d", __entry->comm, __entry->pid)
);
To call the above tracepoint from Rust code, you must first ensure that
the Rust helper for the tracepoint is generated. To do this, you would
modify kernel/sched/core.c by adding #define CREATE_RUST_TRACE_POINTS.
Next, you would include include/trace/events/sched.h in
rust/bindings/bindings_helper.h so that the exported C functions are
visible to Rust, and then you would declare the tracepoint in Rust:
declare_trace! {
fn sched_kthread_stop(task: *mut task_struct);
}
This will define an inline Rust function that checks the static key,
calling into rust_do_trace_##name if the tracepoint is active. Since
these tracepoints often take raw pointers as arguments, it may be
convenient to wrap it in a safe wrapper:
mod raw {
declare_trace! {
fn sched_kthread_stop(task: *mut task_struct);
}
}
#[inline]
pub fn trace_sched_kthread_stop(task: ) {
// SAFETY: The pointer to `task` is valid.
unsafe { raw::sched_kthread_stop(task.as_raw()) }
}
A future expansion of the tracepoint support could generate these safe
versions automatically, but that is left as future work for now.
This is intended for use in the Rust Binder driver, which was originally
sent as an RFC [1]. The RFC did not include tracepoint support, but you
can see how it will be used in Rust Binder at [2]. The author has
verified that the tracepoint support works on Android devices.
This implementation implements support for static keys in Rust so that
the actual static branch happens in the Rust object file. However, the
__DO_TRACE body remains in C code. See v1 for an implementation where
__DO_TRACE is also implemented in Rust.
Link:
https://lore.kernel.org/rust-for-linux/20231101-rust-binder-v1-0-08ba9197f...@google.com/
[1]
Link: https://r.android.com/3119993 [2]
Signed-off-by: Alice Ryhl
---
Changes in v4:
- Move arch-specific code into rust/kernel/arch.
- Restore DEFINE_RUST_DO_TRACE at end of define_trace.h
- Link to v3:
https://lore.kernel.org/r/20240621-tracepoint-v3-0-9e44eeea2...@google.com
Changes in v3:
- Support for Rust static_key on loongarch64 and riscv64.
- Avoid failing compilation on architectures that are missing Rust
static_key support when the archtectures does not actually use it.
- Link to v2:
https://lore.kernel.org/r/20240610-tracepoint-v2-0-faebad81b...@google.com
Changes in v2:
- Call into C code for __DO_TRACE.
- Drop static_call patch, as it is no longer needed.
- Link to v1:
https://lore.kernel.org/r/20240606-tracepoint-v1-0-6551627bf...@google.com
---
Alice Ryhl (2):
rust: add static_key_false
rust: add tracepoint support
include/linux/tracepoint.h | 18 +++-
include/trace/define_trace.h | 12
rust/bindings/bindings_helper.h | 1 +
rust/kernel/arch/arm64/jump_label.rs | 34 +++
rust/kernel/arch/loongarch/jump_label.rs | 35
rust/kernel/arch/mod.rs | 24
rust/kernel/arch/riscv/jump_label.rs | 38 ++
rust/kernel/arch/x86/jump_label.rs | 35
rust/kernel/lib.rs | 3 ++
rust/kernel/static_key.rs| 32 ++
rust/kernel/tracepoint.rs| 47
scripts/Makefile.build | 2 +-
12 files changed, 279 insertions(+), 2 deletions(-)
---
base-commit: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0
change-id: 20240606-tracepoint-31e15b90e471
Best regards,
--
Alice Ryhl
Re: [PATCH 13/14] tracefs: Convert to new uid/gid option parsing helpers
On Thu, 27 Jun 2024 19:40:44 -0500 Eric Sandeen wrote: > Convert to new uid/gid option parsing helpers > > Signed-off-by: Eric Sandeen Acked-by: Steven Rostedt (Google) -- Steve
Re: [PATCH] kallsyms, livepatch: Fix livepatch with CONFIG_LTO_CLANG
On Fri, 7 Jun 2024, Song Liu wrote: > Hi Miroslav, > > Thanks for reviewing the patch! > > On Fri, Jun 7, 2024 at 6:06 AM Miroslav Benes wrote: > > > > Hi, > > > > On Tue, 4 Jun 2024, Song Liu wrote: > > > > > With CONFIG_LTO_CLANG, the compiler may postfix symbols with .llvm. > > > to avoid symbol duplication. scripts/kallsyms.c sorted the symbols > > > without these postfixes. The default symbol lookup also removes these > > > postfixes before comparing symbols. > > > > > > On the other hand, livepatch need to look up symbols with the full names. > > > However, calling kallsyms_on_each_match_symbol with full name (with the > > > postfix) cannot find the symbol(s). As a result, we cannot livepatch > > > kernel functions with .llvm. postfix or kernel functions that use > > > relocation information to symbols with .llvm. postfixes. > > > > > > Fix this by calling kallsyms_on_each_match_symbol without the postfix; > > > and then match the full name (with postfix) in klp_match_callback. > > > > > > Signed-off-by: Song Liu > > > --- > > > include/linux/kallsyms.h | 13 + > > > kernel/kallsyms.c| 21 - > > > kernel/livepatch/core.c | 32 +++- > > > 3 files changed, 60 insertions(+), 6 deletions(-) > > > > I do not like much that something which seems to be kallsyms-internal is > > leaked out. You need to export cleanup_symbol_name() and there is now a > > lot of code outside. I would feel much more comfortable if it is all > > hidden from kallsyms users and kept there. Would it be possible? > > I think it is possible. Currently, kallsyms_on_each_match_symbol matches > symbols without the postfix. We can add a variation or a parameter, so > that it matches the full name with post fix. I think it might be better. Luis, what is your take on this? > > Moreover, isn't there a similar problem for ftrace, kprobes, ebpf,...? > > Yes, there is a similar problem with tracing use cases. But the requirements > are not the same: > > For livepatch, we have to point to the exact symbol we want to patch or > relocation to. We have sympos API defined to differentiate different symbols > with the same name. Yes. In fact, sympos may be used to solve even this problem. The user would disregard .llvm. suffix and they are suddenly in the same situation which sympos aims to solve. I will not argue with you if say it is cumbersome. > For tracing, some discrepancy is acceptable. AFAICT, there isn't an API > similar to sympos yet. Also, we can play some tricks with tracing. For > example, we can use "uniq symbol + offset" to point a kprobe to one of > the duplicated symbols. If I am not mistaken, there was a patch set to address this. Luis might remember more. Regards, Miroslav
Re: [RFC PATCH v2] ptp: Add vDSO-style vmclock support
On Fri, 2024-06-28 at 13:33 +0200, Peter Hilber wrote: > On 27.06.24 16:52, David Woodhouse wrote: > > I already added a flags field, so this might look something like: > > > > /* > > * Smearing flags. The UTC clock exposed through this structure > > * is only ever true UTC, but a guest operating system may > > * choose to offer a monotonic smeared clock to its users. This > > * merely offers a hint about what kind of smearing to perform, > > * for consistency with systems in the nearby environment. > > */ > > #define VMCLOCK_FLAGS_SMEAR_UTC_SLS (1<<5) /* draft-kuhn-leapsecond-00.txt > > */ > > > > (UTC-SLS is probably a bad example but are there formal definitions for > > anything else?) > > I think it could also be more generic, like flags for linear smearing, > cosine smearing(?), and smear_start_sec and smear_end_sec fields (relative > to the leap second start). That could also represent UTC-SLS, and > noon-to-noon, and it would be well-defined. > > This should reduce the likelihood that the guest doesn't know the smearing > variant. I'm wary of making it too generic. That would seem to encourage a *proliferation* of false "UTC-like" clocks. It's bad enough that we do smearing at all, let alone that we don't have a single definition of how to do it. I made the smearing hint a full uint8_t instead of using bits in flags, in the end. That gives us a full 255 ways of lying to users about what the time is, so we're unlikely to run out. And it's easy enough to add a new VMCLOCK_SMEARING_XXX type to the 'registry' for any new methods that get invented. > > > > + /* > > > > + * This field changes to another non-repeating value when the > > > > CPU > > > > + * counter is disrupted, for example on live migration. > > > > + */ > > > > + uint64_t disruption_marker; > > > > > > The field could also change when the clock is stepped (leap seconds > > > excepted), or when the clock frequency is slewed. > > > > I'm not sure. The concept of the disruption marker is that it tells the > > guest to throw away any calibration of the counter that the guest has > > done for *itself* (with NTP, other PTP devices, etc.). > > > > One mode for this device would be not to populate the clock fields at > > all, but *only* to signal disruption when it occurs. So the guest can > > abort transactions until it's resynced its clocks (to avoid incurring > > fines if breaking databases, etc.). > > > > Exposing the host timekeeping through the structure means that the > > migrated guest can keep working because it can trust the timekeeping > > performed by the (new) host and exposed to it. > > > > If the counter is actually varying in frequency over time, and the host > > is slewing the clock frequency that it reports, that *isn't* a step > > change and doesn't mean that the guest should throw away any > > calibration that it's been doing for itself. One hopes that the guest > > would have detected the *same* frequency change, and be adapting for > > itself. So I don't think that should indicate a disruption. > > > > I think the same is even true if the clock is stepped by the host. The > > actual *counter* hasn't changed, so the guest is better off ignoring > > the vacillating host and continuing to derive its idea of time from the > > hardware counter itself, as calibrated against some external NTP/PTP > > sources. Surely we actively *don't* to tell the guest to throw its own > > calibrations away, in this case? > > In case the guest is also considering other time sources, it might indeed > not be a good idea to mix host clock changes into the hardware counter > disruption marker. > > But if the vmclock is the authoritative source of time, it can still be > helpful to know about such changes, maybe through another marker. Could that be the existing seq_count field? Skewing the counter_period_frac_sec as the underlying oscillator speeds up and slows down is perfectly normal and expected, and we already expect the seq_count to change when that happens. Maybe step changes are different, but arguably if the time advertised by the host steps *outside* the error bounds previously advertised, that's just broken? Depending on how the clock information is fed, a change in seq_count may even result in non-monotonicity. If the underlying oscillator has sped up and the structure is updated accordingly, the time calculated the moment *before* that update may appear later than the time calculated immediately after it. It's up to the guest operating system to feed that information into its own timekeeping system and skew towards correctness instead of stepping the time it reports to its users. smime.p7s Description: S/MIME cryptographic signature
[PATCH v2 6/6] riscv: ftrace: support PREEMPT
Now, we can safely enable dynamic ftrace with kernel preemption. Signed-off-by: Andy Chiu --- arch/riscv/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 55c70efbad0a..881ea466ff52 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -139,7 +139,7 @@ config RISCV select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL select HAVE_FUNCTION_GRAPH_TRACER select HAVE_FUNCTION_GRAPH_RETVAL if HAVE_FUNCTION_GRAPH_TRACER - select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION + select HAVE_FUNCTION_TRACER if !XIP_KERNEL select HAVE_EBPF_JIT if MMU select HAVE_GUP_FAST if MMU select HAVE_FUNCTION_ARG_ACCESS_API -- 2.43.0
[PATCH v2 5/6] riscv: vector: Support calling schedule() for preemptible Vector
Each function entry implies a call to ftrace infrastructure. And it may
call into schedule in some cases. So, it is possible for preemptible
kernel-mode Vector to implicitly call into schedule. Since all V-regs
are caller-saved, it is possible to drop all V context when a thread
voluntarily call schedule(). Besides, we currently don't pass argument
through vector register, so we don't have to save/restore V-regs in
ftrace trampoline.
Signed-off-by: Andy Chiu
---
arch/riscv/include/asm/processor.h | 5 +
arch/riscv/include/asm/vector.h| 22 +++---
2 files changed, 24 insertions(+), 3 deletions(-)
diff --git a/arch/riscv/include/asm/processor.h
b/arch/riscv/include/asm/processor.h
index 68c3432dc6ea..02598e168659 100644
--- a/arch/riscv/include/asm/processor.h
+++ b/arch/riscv/include/asm/processor.h
@@ -95,6 +95,10 @@ struct pt_regs;
* Thus, the task does not own preempt_v. Any use of Vector will have to
* save preempt_v, if dirty, and fallback to non-preemptible kernel-mode
* Vector.
+ * - bit 29: The thread voluntarily calls schedule() while holding an active
+ *preempt_v. All preempt_v context should be dropped in such case because
+ *V-regs are caller-saved. Only sstatus.VS=ON is persisted across a
+ *schedule() call.
* - bit 30: The in-kernel preempt_v context is saved, and requries to be
*restored when returning to the context that owns the preempt_v.
* - bit 31: The in-kernel preempt_v context is dirty, as signaled by the
@@ -109,6 +113,7 @@ struct pt_regs;
#define RISCV_PREEMPT_V0x0100
#define RISCV_PREEMPT_V_DIRTY 0x8000
#define RISCV_PREEMPT_V_NEED_RESTORE 0x4000
+#define RISCV_PREEMPT_V_IN_SCHEDULE0x2000
/* CPU-specific state of a task */
struct thread_struct {
diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h
index be7d309cca8a..fbf17aba92c1 100644
--- a/arch/riscv/include/asm/vector.h
+++ b/arch/riscv/include/asm/vector.h
@@ -75,6 +75,11 @@ static __always_inline void riscv_v_disable(void)
csr_clear(CSR_SSTATUS, SR_VS);
}
+static __always_inline bool riscv_v_is_on(void)
+{
+ return !!(csr_read(CSR_SSTATUS) & SR_VS);
+}
+
static __always_inline void __vstate_csr_save(struct __riscv_v_ext_state *dest)
{
asm volatile (
@@ -243,6 +248,11 @@ static inline void __switch_to_vector(struct task_struct
*prev,
struct pt_regs *regs;
if (riscv_preempt_v_started(prev)) {
+ if (riscv_v_is_on()) {
+ WARN_ON(prev->thread.riscv_v_flags &
RISCV_V_CTX_DEPTH_MASK);
+ riscv_v_disable();
+ prev->thread.riscv_v_flags |=
RISCV_PREEMPT_V_IN_SCHEDULE;
+ }
if (riscv_preempt_v_dirty(prev)) {
__riscv_v_vstate_save(>thread.kernel_vstate,
prev->thread.kernel_vstate.datap);
@@ -253,10 +263,16 @@ static inline void __switch_to_vector(struct task_struct
*prev,
riscv_v_vstate_save(>thread.vstate, regs);
}
- if (riscv_preempt_v_started(next))
- riscv_preempt_v_set_restore(next);
- else
+ if (riscv_preempt_v_started(next)) {
+ if (next->thread.riscv_v_flags & RISCV_PREEMPT_V_IN_SCHEDULE) {
+ next->thread.riscv_v_flags &=
~RISCV_PREEMPT_V_IN_SCHEDULE;
+ riscv_v_enable();
+ } else {
+ riscv_preempt_v_set_restore(next);
+ }
+ } else {
riscv_v_vstate_set_restore(next, task_pt_regs(next));
+ }
}
void riscv_v_vstate_ctrl_init(struct task_struct *tsk);
--
2.43.0
[PATCH v2 4/6] riscv: ftrace: do not use stop_machine to update code
Now it is safe to remove dependency from stop_machine() for us to patch
code in ftrace.
Signed-off-by: Andy Chiu
---
arch/riscv/kernel/ftrace.c | 53 --
1 file changed, 4 insertions(+), 49 deletions(-)
diff --git a/arch/riscv/kernel/ftrace.c b/arch/riscv/kernel/ftrace.c
index 5ebe412280ef..57a6558e212e 100644
--- a/arch/riscv/kernel/ftrace.c
+++ b/arch/riscv/kernel/ftrace.c
@@ -13,23 +13,13 @@
#include
#ifdef CONFIG_DYNAMIC_FTRACE
-void ftrace_arch_code_modify_prepare(void) __acquires(_mutex)
+void arch_ftrace_update_code(int command)
{
mutex_lock(_mutex);
-
- /*
-* The code sequences we use for ftrace can't be patched while the
-* kernel is running, so we need to use stop_machine() to modify them
-* for now. This doesn't play nice with text_mutex, we use this flag
-* to elide the check.
-*/
- riscv_patch_in_stop_machine = true;
-}
-
-void ftrace_arch_code_modify_post_process(void) __releases(_mutex)
-{
- riscv_patch_in_stop_machine = false;
+ command |= FTRACE_MAY_SLEEP;
+ ftrace_modify_all_code(command);
mutex_unlock(_mutex);
+ flush_icache_all();
}
static int ftrace_check_current_call(unsigned long hook_pos,
@@ -155,41 +145,6 @@ int ftrace_update_ftrace_func(ftrace_func_t func)
return __ftrace_modify_call_site(_call_dest, func, true);
}
-struct ftrace_modify_param {
- int command;
- atomic_t cpu_count;
-};
-
-static int __ftrace_modify_code(void *data)
-{
- struct ftrace_modify_param *param = data;
-
- if (atomic_inc_return(>cpu_count) == num_online_cpus()) {
- ftrace_modify_all_code(param->command);
- /*
-* Make sure the patching store is effective *before* we
-* increment the counter which releases all waiting CPUs
-* by using the release variant of atomic increment. The
-* release pairs with the call to local_flush_icache_all()
-* on the waiting CPU.
-*/
- atomic_inc_return_release(>cpu_count);
- } else {
- while (atomic_read(>cpu_count) <= num_online_cpus())
- cpu_relax();
-
- local_flush_icache_all();
- }
-
- return 0;
-}
-
-void arch_ftrace_update_code(int command)
-{
- struct ftrace_modify_param param = { command, ATOMIC_INIT(0) };
-
- stop_machine(__ftrace_modify_code, , cpu_online_mask);
-}
#endif
#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
--
2.43.0
[PATCH v2 3/6] riscv: ftrace: prepare ftrace for atomic code patching
We use an AUIPC+JALR pair to jump into a ftrace trampoline. Since
instruction fetch can break down to 4 byte at a time, it is impossible
to update two instructions without a race. In order to mitigate it, we
initialize the patchable entry to AUIPC + NOP4. Then, the run-time code
patching can change NOP4 to JALR to eable/disable ftrcae from a
function. This limits the reach of each ftrace entry to +-2KB displacing
from ftrace_caller.
Starting from the trampoline, we add a level of indirection for it to
reach ftrace caller target. Now, it loads the target address from a
memory location, then perform the jump. This enable the kernel to update
the target atomically.
The ordering of reading/updating the targert address should be guarded
by generic ftrace code, where it sends smp_rmb ipi.
Signed-off-by: Andy Chiu
---
arch/riscv/include/asm/ftrace.h | 4 +++
arch/riscv/kernel/ftrace.c | 80 ++---
arch/riscv/kernel/mcount-dyn.S | 9 +++--
3 files changed, 62 insertions(+), 31 deletions(-)
diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h
index 5f81c53dbfd9..7199383f8c02 100644
--- a/arch/riscv/include/asm/ftrace.h
+++ b/arch/riscv/include/asm/ftrace.h
@@ -81,6 +81,7 @@ struct dyn_arch_ftrace {
#define JALR_T0(0x000282e7)
#define AUIPC_T0 (0x0297)
#define NOP4 (0x0013)
+#define JALR_RANGE (JALR_SIGN_MASK - 1)
#define to_jalr_t0(offset) \
(((offset & JALR_OFFSET_MASK) << JALR_SHIFT) | JALR_T0)
@@ -118,6 +119,9 @@ do {
\
* Let auipc+jalr be the basic *mcount unit*, so we make it 8 bytes here.
*/
#define MCOUNT_INSN_SIZE 8
+#define MCOUNT_AUIPC_SIZE 4
+#define MCOUNT_JALR_SIZE 4
+#define MCOUNT_NOP4_SIZE 4
#ifndef __ASSEMBLY__
struct dyn_ftrace;
diff --git a/arch/riscv/kernel/ftrace.c b/arch/riscv/kernel/ftrace.c
index 4b95c574fd04..5ebe412280ef 100644
--- a/arch/riscv/kernel/ftrace.c
+++ b/arch/riscv/kernel/ftrace.c
@@ -64,42 +64,64 @@ static int ftrace_check_current_call(unsigned long hook_pos,
return 0;
}
-static int __ftrace_modify_call(unsigned long hook_pos, unsigned long target,
- bool enable, bool ra)
+static int __ftrace_modify_call(unsigned long hook_pos, unsigned long target,
bool validate)
{
unsigned int call[2];
- unsigned int nops[2] = {NOP4, NOP4};
+ unsigned int replaced[2];
+
+ make_call_t0(hook_pos, target, call);
- if (ra)
- make_call_ra(hook_pos, target, call);
- else
- make_call_t0(hook_pos, target, call);
+ if (validate) {
+ /*
+* Read the text we want to modify;
+* return must be -EFAULT on read error
+*/
+ if (copy_from_kernel_nofault(replaced, (void *)hook_pos,
+MCOUNT_INSN_SIZE))
+ return -EFAULT;
+
+ if (replaced[0] != call[0]) {
+ pr_err("%p: expected (%08x) but got (%08x)\n",
+ (void *)hook_pos, call[0], replaced[0]);
+ return -EINVAL;
+ }
+ }
- /* Replace the auipc-jalr pair at once. Return -EPERM on write error. */
- if (patch_insn_write((void *)hook_pos, enable ? call : nops,
MCOUNT_INSN_SIZE))
+ /* Replace the jalr at once. Return -EPERM on write error. */
+ if (patch_insn_write((void *)(hook_pos + MCOUNT_AUIPC_SIZE), call + 1,
MCOUNT_JALR_SIZE))
return -EPERM;
return 0;
}
-int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
+static int __ftrace_modify_call_site(ftrace_func_t *hook_pos, ftrace_func_t
target, bool enable)
{
- unsigned int call[2];
+ ftrace_func_t call = target;
+ ftrace_func_t nops = _stub;
- make_call_t0(rec->ip, addr, call);
-
- if (patch_insn_write((void *)rec->ip, call, MCOUNT_INSN_SIZE))
- return -EPERM;
+ WRITE_ONCE(*hook_pos, enable ? call : nops);
return 0;
}
+int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
+{
+ unsigned long distance, orig_addr;
+
+ orig_addr = (unsigned long)_caller;
+ distance = addr > orig_addr ? addr - orig_addr : orig_addr - addr;
+ if (distance > JALR_RANGE)
+ return -EINVAL;
+
+ return __ftrace_modify_call(rec->ip, addr, false);
+}
+
int ftrace_make_nop(struct module *mod, struct dyn_ftrace *rec,
unsigned long addr)
{
- unsigned int nops[2] = {NOP4, NOP4};
+ unsigned int nops[1] = {NOP4};
- if (patch_insn_write((void *)rec->ip, nops, MCOUNT_INSN_SIZE))
+ if (patch_insn_write((void *)(rec->ip + MCOUNT_AUIPC_SIZE), nops,
[PATCH v2 2/6] riscv: ftrace: align patchable functions to 4 Byte boundary
We are changing ftrace code patching in order to remove dependency from stop_machine() and enable kernel preemption. This requires us to align functions entry at a 4-B align address. However, -falign-functions on older versions of GCC alone was not strong enoungh to align all functions. In fact, cold functions are not aligned after turning on optimizations. We consider this is a bug in GCC and turn off guess-branch-probility as a workaround to align all functions. GCC bug id: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88345 The option -fmin-function-alignment is able to align all functions properly on newer versions of gcc. So, we add a cc-option to test if the toolchain supports it. Suggested-by: Evgenii Shatokhin Signed-off-by: Andy Chiu --- Changelog v2: - Use CC_HAS_MIN_FUNCTION_ALIGNMENT and it friends to prevent reinventing wheels (Nathan) --- arch/riscv/Kconfig | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 704d4683bcfa..55c70efbad0a 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -133,6 +133,7 @@ config RISCV select HAVE_DEBUG_KMEMLEAK select HAVE_DMA_CONTIGUOUS if MMU select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && (CLANG_SUPPORTS_DYNAMIC_FTRACE || GCC_SUPPORTS_DYNAMIC_FTRACE) + select FUNCTION_ALIGNMENT_4B if HAVE_DYNAMIC_FTRACE && RISCV_ISA_C select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS select HAVE_DYNAMIC_FTRACE_WITH_ARGS if HAVE_DYNAMIC_FTRACE select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL @@ -208,6 +209,7 @@ config CLANG_SUPPORTS_DYNAMIC_FTRACE config GCC_SUPPORTS_DYNAMIC_FTRACE def_bool CC_IS_GCC depends on $(cc-option,-fpatchable-function-entry=8) + depends on CC_HAS_MIN_FUNCTION_ALIGNMENT || !RISCV_ISA_C config HAVE_SHADOW_CALL_STACK def_bool $(cc-option,-fsanitize=shadow-call-stack) -- 2.43.0
[PATCH v2 1/6] riscv: ftrace: support fastcc in Clang for WITH_ARGS
Some caller-saved registers which are not defined as function arguments
in the ABI can still be passed as arguments when the kernel is compiled
with Clang. As a result, we must save and restore those registers to
prevent ftrace from clobbering them.
- [1]: https://reviews.llvm.org/D68559
Reported-by: Evgenii Shatokhin
Closes:
https://lore.kernel.org/linux-riscv/7e7c7914-445d-426d-89a0-59a9199c4...@yadro.com/
Acked-by: Nathan Chancellor
Signed-off-by: Andy Chiu
---
arch/riscv/include/asm/ftrace.h | 7 +++
arch/riscv/kernel/asm-offsets.c | 7 +++
arch/riscv/kernel/mcount-dyn.S | 16 ++--
3 files changed, 28 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h
index 9eb31a7ea0aa..5f81c53dbfd9 100644
--- a/arch/riscv/include/asm/ftrace.h
+++ b/arch/riscv/include/asm/ftrace.h
@@ -144,6 +144,13 @@ struct ftrace_regs {
unsigned long a5;
unsigned long a6;
unsigned long a7;
+#ifdef CONFIG_CC_IS_CLANG
+ unsigned long t2;
+ unsigned long t3;
+ unsigned long t4;
+ unsigned long t5;
+ unsigned long t6;
+#endif
};
};
};
diff --git a/arch/riscv/kernel/asm-offsets.c b/arch/riscv/kernel/asm-offsets.c
index b09ca5f944f7..db5a26fcc9ae 100644
--- a/arch/riscv/kernel/asm-offsets.c
+++ b/arch/riscv/kernel/asm-offsets.c
@@ -497,6 +497,13 @@ void asm_offsets(void)
DEFINE(FREGS_SP,offsetof(struct ftrace_regs, sp));
DEFINE(FREGS_S0,offsetof(struct ftrace_regs, s0));
DEFINE(FREGS_T1,offsetof(struct ftrace_regs, t1));
+#ifdef CONFIG_CC_IS_CLANG
+ DEFINE(FREGS_T2,offsetof(struct ftrace_regs, t2));
+ DEFINE(FREGS_T3,offsetof(struct ftrace_regs, t3));
+ DEFINE(FREGS_T4,offsetof(struct ftrace_regs, t4));
+ DEFINE(FREGS_T5,offsetof(struct ftrace_regs, t5));
+ DEFINE(FREGS_T6,offsetof(struct ftrace_regs, t6));
+#endif
DEFINE(FREGS_A0,offsetof(struct ftrace_regs, a0));
DEFINE(FREGS_A1,offsetof(struct ftrace_regs, a1));
DEFINE(FREGS_A2,offsetof(struct ftrace_regs, a2));
diff --git a/arch/riscv/kernel/mcount-dyn.S b/arch/riscv/kernel/mcount-dyn.S
index 745dd4c4a69c..e988bd26b28b 100644
--- a/arch/riscv/kernel/mcount-dyn.S
+++ b/arch/riscv/kernel/mcount-dyn.S
@@ -96,7 +96,13 @@
REG_S x8, FREGS_S0(sp)
#endif
REG_S x6, FREGS_T1(sp)
-
+#ifdef CONFIG_CC_IS_CLANG
+ REG_S x7, FREGS_T2(sp)
+ REG_S x28, FREGS_T3(sp)
+ REG_S x29, FREGS_T4(sp)
+ REG_S x30, FREGS_T5(sp)
+ REG_S x31, FREGS_T6(sp)
+#endif
// save the arguments
REG_S x10, FREGS_A0(sp)
REG_S x11, FREGS_A1(sp)
@@ -115,7 +121,13 @@
REG_L x8, FREGS_S0(sp)
#endif
REG_L x6, FREGS_T1(sp)
-
+#ifdef CONFIG_CC_IS_CLANG
+ REG_L x7, FREGS_T2(sp)
+ REG_L x28, FREGS_T3(sp)
+ REG_L x29, FREGS_T4(sp)
+ REG_L x30, FREGS_T5(sp)
+ REG_L x31, FREGS_T6(sp)
+#endif
// restore the arguments
REG_L x10, FREGS_A0(sp)
REG_L x11, FREGS_A1(sp)
--
2.43.0
[PATCH v2 0/6] riscv: ftrace: atmoic patching and preempt improvements
This series makes atmoic code patching possible in riscv ftrace. A
direct benefit of this is that we can get rid of stop_machine() when
patching function entries. This also makes it possible to run ftrace
with full kernel preemption. Before this series, the kernel initializes
patchable function entries to NOP4 + NOP4. To start tracing, it updates
entries to AUIPC + JALR while holding other cores in stop_machine.
stop_machine() is required because it is impossible to update 2
instructions, and be seen atomically. And preemption must have to be
prevented, as kernel preemption allows process to be scheduled out while
executing on one of these instruction pairs.
This series addresses the problem by initializing the first NOP4 to
AUIPC. So, atmoic patching is possible because the kernel only has to
update one instruction. As long as the instruction is naturally aligned,
then it is expected to be updated atomically.
However, the address range of the ftrace trampoline is limited to +-2K
from ftrace_caller after appplying this series. This issue is expected
to be solved by Puranjay's CALL_OPS, where it adds 8B naturally align
data in front of pacthable functions and can use it to direct execution
out to any custom trampolines.
The series is composed by three parts. The first part cleans up the
existing issues when the kernel is compiled with clang.The second part
modifies the ftrace code patching mechanism (2-4) as mentioned above.
Then prepare ftrace to be able to run with kernel preemption (5,6)
This series is tested after applying the following ftrace/patching in
the fixes branch:
- commit 57a369b6f2ee ("riscv: patch: Flush the icache right after
patching to avoid illegal insns")
- commit a2bd3a5b4b63 ("riscv: stacktrace: convert arch_stack_walk() to
noinstr")
Changes in v2:
- Drop patch 1 as it is merged through fixes.
- Drop patch 2, which converts kernel_text_address into notrace. As
users can prevent tracing it by configuring the tracefs.
- Use a more generic way in kconfig to align functions.
- Link to v1:
https://lore.kernel.org/r/20240613-dev-andyc-dyn-ftrace-v4-v1-0-1a538e12c...@sifive.com
---
Andy Chiu (6):
riscv: ftrace: support fastcc in Clang for WITH_ARGS
riscv: ftrace: align patchable functions to 4 Byte boundary
riscv: ftrace: prepare ftrace for atomic code patching
riscv: ftrace: do not use stop_machine to update code
riscv: vector: Support calling schedule() for preemptible Vector
riscv: ftrace: support PREEMPT
arch/riscv/Kconfig | 4 +-
arch/riscv/include/asm/ftrace.h| 11 +++
arch/riscv/include/asm/processor.h | 5 ++
arch/riscv/include/asm/vector.h| 22 +-
arch/riscv/kernel/asm-offsets.c| 7 ++
arch/riscv/kernel/ftrace.c | 133 -
arch/riscv/kernel/mcount-dyn.S | 25 +--
7 files changed, 121 insertions(+), 86 deletions(-)
---
base-commit: a2bd3a5b4b63b95aea7dbf61d9395cd6696a2bc0
change-id: 20240613-dev-andyc-dyn-ftrace-v4-941d4a00ea19
Best regards,
--
Andy Chiu
Re: [RFC PATCH v2] ptp: Add vDSO-style vmclock support
On Fri, 2024-06-28 at 13:33 +0200, Peter Hilber wrote: > > > > > /* > > * What time is exposed in the time_sec/time_frac_sec fields? > > */ > > uint8_t time_type; > > #define VMCLOCK_TIME_UNKNOWN0 /* Invalid / no time > > exposed */ > > #define VMCLOCK_TIME_UTC1 /* Since 1970-01-01 > > 00:00:00z */ > > #define VMCLOCK_TIME_TAI2 /* Since 1970-01-01 > > 00:00:00z */ > > #define VMCLOCK_TIME_MONOTONIC 3 /* Since undefined epoch */ > > > > /* Bit shift for counter_period_frac_sec and its error rate */ > > uint8_t counter_period_shift; > > > > /* > > * Unlike in NTP, this can indicate a leap second in the past. This > > * is needed to allow guests to derive an imprecise clock with > > * smeared leap seconds for themselves, as some modes of smearing > > * need the adjustments to continue even after the moment at which > > * the leap second should have occurred. > > */ > > int8_t leapsecond_direction; > > uint64_t leapsecond_tai_sec; /* Since 1970-01-01 00:00:00z */ > > > > /* > > * Paired values of counter and UTC at a given point in time. > > */ > > uint64_t counter_value; > > uint64_t time_sec; /* Since 1970-01-01 00:00:00z */ > > Nitpick: The comment is not valid any more for TIME_MONOTONIC. Ah yes, I "moved" that comment up to the UTC/TAI time_type values, but neglected to actually delete it from here. Fixed; thanks. smime.p7s Description: S/MIME cryptographic signature
Re: [RFC PATCH v2] ptp: Add vDSO-style vmclock support
On 27.06.24 18:03, David Woodhouse wrote:
>
> I've updated the tree at
> https://git.infradead.org/users/dwmw2/linux.git/shortlog/refs/heads/vmclock
> (but not yet the qemu one).
>
> I think I've taken into account all your comments apart from the one
> about non-64-bit counters wrapping. I reduced the seq_count to 32 bit
> to make room for a 32-bit flags field, added the time type
> (UTC/TAI/MONOTONIC) and a smearing hint, with some straw man
> definitions for smearing algorithms for which I could actually find
> definitions.
>
> The structure now looks like this:
>
>
> struct vmclock_abi {
[...]
>
> /*
>* What time is exposed in the time_sec/time_frac_sec fields?
>*/
> uint8_t time_type;
> #define VMCLOCK_TIME_UNKNOWN 0 /* Invalid / no time exposed */
> #define VMCLOCK_TIME_UTC 1 /* Since 1970-01-01 00:00:00z */
> #define VMCLOCK_TIME_TAI 2 /* Since 1970-01-01 00:00:00z */
> #define VMCLOCK_TIME_MONOTONIC3 /* Since undefined
> epoch */
>
> /* Bit shift for counter_period_frac_sec and its error rate */
> uint8_t counter_period_shift;
>
> /*
>* Unlike in NTP, this can indicate a leap second in the past. This
>* is needed to allow guests to derive an imprecise clock with
>* smeared leap seconds for themselves, as some modes of smearing
>* need the adjustments to continue even after the moment at which
>* the leap second should have occurred.
>*/
> int8_t leapsecond_direction;
> uint64_t leapsecond_tai_sec; /* Since 1970-01-01 00:00:00z */
>
> /*
>* Paired values of counter and UTC at a given point in time.
>*/
> uint64_t counter_value;
> uint64_t time_sec; /* Since 1970-01-01 00:00:00z */
Nitpick: The comment is not valid any more for TIME_MONOTONIC.
Re: [RFC PATCH v2] ptp: Add vDSO-style vmclock support
On 27.06.24 16:52, David Woodhouse wrote:
> On Thu, 2024-06-27 at 15:50 +0200, Peter Hilber wrote:
>> On 25.06.24 21:01, David Woodhouse wrote:
>>> From: David Woodhouse
>>>
>>> The vmclock "device" provides a shared memory region with precision clock
>>> information. By using shared memory, it is safe across Live Migration.
>>>
>>> Like the KVM PTP clock, this 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 memory region of the device is also exposed to userspace so it can be
>>> read or memory mapped by application which need reliable notification of
>>> clock disruptions.
>>>
>>> Signed-off-by: David Woodhouse
>>> ---
>>>
>>> v2:
>>> • Add gettimex64() support
>>> • Convert TSC values to KVM clock when appropriate
>>> • Require int128 support
>>> • Add counter_period_shift
>>> • Add timeout when seq_count is invalid
>>> • Add flags field
>>> • Better comments in vmclock ABI structure
>>> • Explicitly forbid smearing (as clock rates would need to change)
>>
>> Leap second smearing information could still be conveyed through the
>> vmclock_abi. AFAIU, to cover the popular smearing variants, it should be
>> enough to indicate whether the driver should apply linear or cosine
>> smearing, and the start time and end time.
>
> Yes. The clock information actually conveyed through the {counter,
> time, rate} tuple should never be smeared, and should only ever be UTC.
>
> But we could provide a hint to the guest operating system about what
> type of smearing to perform, *if* it chooses to offer a clock other
> than the standard CLOCK_REALTIME to its users.
>
> I already added a flags field, so this might look something like:
>
> /*
> * Smearing flags. The UTC clock exposed through this structure
> * is only ever true UTC, but a guest operating system may
> * choose to offer a monotonic smeared clock to its users. This
> * merely offers a hint about what kind of smearing to perform,
> * for consistency with systems in the nearby environment.
> */
> #define VMCLOCK_FLAGS_SMEAR_UTC_SLS (1<<5) /* draft-kuhn-leapsecond-00.txt */
>
>
> (UTC-SLS is probably a bad example but are there formal definitions for
> anything else?)
>
>
I think it could also be more generic, like flags for linear smearing,
cosine smearing(?), and smear_start_sec and smear_end_sec fields (relative
to the leap second start). That could also represent UTC-SLS, and
noon-to-noon, and it would be well-defined.
This should reduce the likelihood that the guest doesn't know the smearing
variant.
[...]
>>> diff --git a/include/uapi/linux/vmclock.h b/include/uapi/linux/vmclock.h
>>> new file mode 100644
>>> index ..cf0f22205e79
>>> --- /dev/null
>>> +++ b/include/uapi/linux/vmclock.h
>>> @@ -0,0 +1,138 @@
>>> +/* SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR
>>> BSD-2-Clause) */
>>> +
>>> +/*
>>> + * This structure provides a vDSO-style clock to VM guests, exposing the
>>> + * relationship (or lack thereof) between the CPU clock (TSC, timebase,
>>> arch
>>> + * counter, etc.) and real time. It is designed to address the problem of
>>> + * live migration, which other clock enlightenments do not.
>>> + *
>>> + * When a guest is live migrated, this affects the clock in two ways.
>>> + *
>>> + * First, even between identical hosts the actual frequency of the
>>> underlying
>>> + * counter will change within the tolerances of its specification
>>> (typically
>>> + * ±50PPM, or 4 seconds a day). The frequency also varies over time on the
>>> + * same host, but can be tracked by NTP as it generally varies slowly. With
>>> + * live migration there is a step change in the frequency, with no warning.
>>> + *
>>> + * Second, there may be a step change in the value of the counter itself,
>>> as
>>> + * its accuracy is limited by the precision of the NTP synchronization on
>>> the
>>> + * source and destination hosts.
>>> + *
>>> + * So any calibration (NTP, PTP, etc.) which the guest has done on the
>>> source
>>> + * host before migration is invalid, and needs to be redone on the new
>>> host.
>>> + *
>>> + * In its most basic mode, this structure provides only an indication to
>>> the
>>> + * guest that live migration has occurred. This allows the guest to know
>>> that
>>> + * its clock is invalid and take remedial action. For applications that
>>> need
>>> + * reliable accurate timestamps (e.g. distributed databases), the structure
>>> + * can be mapped all the way to userspace. This allows the application to
>>> see
>>> + * directly for itself that the clock is disrupted and take appropriate
>>> + * action, even when using a vDSO-style method to get the time instead of a
>>> + * system call.
>>> + *
>>> + * In its more advanced mode. this structure can also be used to expose the
>>> + * precise relationship of the CPU counter to
Re: [PATCH net-next v3 2/3] vsock/virtio: add SIOCOUTQ support for all virtio based transports
On Wed, Jun 26, 2024 at 02:08:36PM GMT, Luigi Leonardi via B4 Relay wrote:
From: Luigi Leonardi
Introduce support for stream_bytes_unsent and seqpacket_bytes_unsent
ioctl for virtio_transport, vhost_vsock and vsock_loopback.
For all transports the unsent bytes counter is incremented
in virtio_transport_get_credit.
In the virtio_transport (G2H) the counter is decremented each
time the host notifies the guest that it consumed the skbuffs.
In vhost-vsock (H2G) the counter is decremented after the skbuff
is queued in the virtqueue.
In vsock_loopback the counter is decremented after the skbuff is
dequeued.
Signed-off-by: Luigi Leonardi
---
drivers/vhost/vsock.c | 4 +++-
include/linux/virtio_vsock.h| 7 +++
net/vmw_vsock/virtio_transport.c| 4 +++-
net/vmw_vsock/virtio_transport_common.c | 35 +
net/vmw_vsock/vsock_loopback.c | 7 +++
5 files changed, 55 insertions(+), 2 deletions(-)
diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index ec20ecff85c7..dba8b3ea37bf 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -244,7 +244,7 @@ vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
restart_tx = true;
}
- consume_skb(skb);
+ virtio_transport_consume_skb_sent(skb, true);
}
} while(likely(!vhost_exceeds_weight(vq, ++pkts, total_len)));
if (added)
@@ -451,6 +451,8 @@ static struct virtio_transport vhost_transport = {
.notify_buffer_size = virtio_transport_notify_buffer_size,
.notify_set_rcvlowat =
virtio_transport_notify_set_rcvlowat,
+ .unsent_bytes = virtio_transport_bytes_unsent,
The callback is named `unsent_bytes`, I'd use something similar also
in the function name, so `virtio_transport_unsent_bytes`, or the
opposite renaming the callback, as you prefer, but I'd use the same
for both.
+
.read_skb = virtio_transport_read_skb,
},
diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
index c82089dee0c8..e74c12878213 100644
--- a/include/linux/virtio_vsock.h
+++ b/include/linux/virtio_vsock.h
@@ -134,6 +134,8 @@ struct virtio_vsock_sock {
u32 peer_fwd_cnt;
u32 peer_buf_alloc;
Can you remove this extra empty line, so it's clear that it is
protected by tx_lock?
+ size_t bytes_unsent;
+
/* Protected by rx_lock */
u32 fwd_cnt;
u32 last_fwd_cnt;
@@ -193,6 +195,11 @@ s64 virtio_transport_stream_has_data(struct vsock_sock
*vsk);
s64 virtio_transport_stream_has_space(struct vsock_sock *vsk);
u32 virtio_transport_seqpacket_has_data(struct vsock_sock *vsk);
+size_t virtio_transport_bytes_unsent(struct vsock_sock *vsk);
+
+void virtio_transport_consume_skb_sent(struct sk_buff *skb,
+ bool consume);
+
int virtio_transport_do_socket_init(struct vsock_sock *vsk,
struct vsock_sock *psk);
int
diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
index 43d405298857..fc62d2818c2c 100644
--- a/net/vmw_vsock/virtio_transport.c
+++ b/net/vmw_vsock/virtio_transport.c
@@ -311,7 +311,7 @@ static void virtio_transport_tx_work(struct work_struct
*work)
virtqueue_disable_cb(vq);
while ((skb = virtqueue_get_buf(vq, )) != NULL) {
- consume_skb(skb);
+ virtio_transport_consume_skb_sent(skb, true);
added = true;
}
} while (!virtqueue_enable_cb(vq));
@@ -540,6 +540,8 @@ static struct virtio_transport virtio_transport = {
.notify_buffer_size = virtio_transport_notify_buffer_size,
.notify_set_rcvlowat =
virtio_transport_notify_set_rcvlowat,
+ .unsent_bytes = virtio_transport_bytes_unsent,
+
.read_skb = virtio_transport_read_skb,
},
diff --git a/net/vmw_vsock/virtio_transport_common.c
b/net/vmw_vsock/virtio_transport_common.c
index 16ff976a86e3..3a7fa36f306b 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -463,6 +463,26 @@ void virtio_transport_inc_tx_pkt(struct virtio_vsock_sock
*vvs, struct sk_buff *
}
EXPORT_SYMBOL_GPL(virtio_transport_inc_tx_pkt);
+void virtio_transport_consume_skb_sent(struct sk_buff *skb, bool consume)
+{
+ struct sock *s = skb->sk;
+
+ if (s && skb->len) {
+ struct vsock_sock *vs = vsock_sk(s);
+ struct virtio_vsock_sock *vvs;
+
+ vvs = vs->trans;
+
+ spin_lock_bh(>tx_lock);
+ vvs->bytes_unsent -= skb->len;
+ spin_unlock_bh(>tx_lock);
+ }
+
+ if (consume)
+ consume_skb(skb);
+}
[PATCH v2] ring-buffer: Align meta-page to sub-buffers for improved TLB usage
Previously, the mapped ring-buffer layout caused misalignment between
the meta-page and sub-buffers when the sub-buffer size was not a
multiple of PAGE_SIZE. This prevented hardware with larger TLB entries
from utilizing them effectively.
Add a padding with the zero-page between the meta-page and sub-buffers.
Also update the ring-buffer map_test to verify that padding.
Signed-off-by: Vincent Donnefort
--
This is based on the mm-unstable branch [1] as it depends on David's work [2]
for allowing the zero-page in vm_insert_page().
[1] https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git
[2] https://lore.kernel.org/all/20240522125713.775114-1-da...@redhat.com
v1 -> v2:
* Fix unsequenced modification and access to 'p' (s390 build)
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 7345a8b625fb..c1116e76fe17 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -6148,10 +6148,10 @@ static void rb_setup_ids_meta_page(struct
ring_buffer_per_cpu *cpu_buffer,
/* install subbuf ID to kern VA translation */
cpu_buffer->subbuf_ids = subbuf_ids;
- meta->meta_page_size = PAGE_SIZE;
meta->meta_struct_len = sizeof(*meta);
meta->nr_subbufs = nr_subbufs;
meta->subbuf_size = cpu_buffer->buffer->subbuf_size + BUF_PAGE_HDR_SIZE;
+ meta->meta_page_size = meta->subbuf_size;
rb_update_meta_page(cpu_buffer);
}
@@ -6238,6 +6238,12 @@ static int __rb_map_vma(struct ring_buffer_per_cpu
*cpu_buffer,
!(vma->vm_flags & VM_MAYSHARE))
return -EPERM;
+ subbuf_order = cpu_buffer->buffer->subbuf_order;
+ subbuf_pages = 1 << subbuf_order;
+
+ if (subbuf_order && pgoff % subbuf_pages)
+ return -EINVAL;
+
/*
* Make sure the mapping cannot become writable later. Also tell the VM
* to not touch these pages (VM_DONTCOPY | VM_DONTEXPAND).
@@ -6247,11 +6253,8 @@ static int __rb_map_vma(struct ring_buffer_per_cpu
*cpu_buffer,
lockdep_assert_held(_buffer->mapping_lock);
- subbuf_order = cpu_buffer->buffer->subbuf_order;
- subbuf_pages = 1 << subbuf_order;
-
nr_subbufs = cpu_buffer->nr_pages + 1; /* + reader-subbuf */
- nr_pages = ((nr_subbufs) << subbuf_order) - pgoff + 1; /* + meta-page */
+ nr_pages = ((nr_subbufs + 1) << subbuf_order) - pgoff; /* + meta-page */
vma_pages = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
if (!vma_pages || vma_pages > nr_pages)
@@ -6264,20 +6267,24 @@ static int __rb_map_vma(struct ring_buffer_per_cpu
*cpu_buffer,
return -ENOMEM;
if (!pgoff) {
+ unsigned long meta_page_padding;
+
pages[p++] = virt_to_page(cpu_buffer->meta_page);
/*
-* TODO: Align sub-buffers on their size, once
-* vm_insert_pages() supports the zero-page.
+* Pad with the zero-page to align the meta-page with the
+* sub-buffers.
*/
- } else {
- /* Skip the meta-page */
- pgoff--;
+ meta_page_padding = subbuf_pages - 1;
+ while (meta_page_padding-- && p < nr_pages) {
+ unsigned long __maybe_unused zero_addr =
+ vma->vm_start + (PAGE_SIZE * p);
- if (pgoff % subbuf_pages) {
- err = -EINVAL;
- goto out;
+ pages[p++] = ZERO_PAGE(zero_addr);
}
+ } else {
+ /* Skip the meta-page */
+ pgoff -= subbuf_pages;
s += pgoff / subbuf_pages;
}
diff --git a/tools/testing/selftests/ring-buffer/map_test.c
b/tools/testing/selftests/ring-buffer/map_test.c
index a9006fa7097e..4bb0192e43f3 100644
--- a/tools/testing/selftests/ring-buffer/map_test.c
+++ b/tools/testing/selftests/ring-buffer/map_test.c
@@ -228,6 +228,20 @@ TEST_F(map, data_mmap)
data = mmap(NULL, data_len, PROT_READ, MAP_SHARED,
desc->cpu_fd, meta_len);
ASSERT_EQ(data, MAP_FAILED);
+
+ /* Verify meta-page padding */
+ if (desc->meta->meta_page_size > getpagesize()) {
+ void *addr;
+
+ data_len = desc->meta->meta_page_size;
+ data = mmap(NULL, data_len,
+ PROT_READ, MAP_SHARED, desc->cpu_fd, 0);
+ ASSERT_NE(data, MAP_FAILED);
+
+ addr = (void *)((unsigned long)data + getpagesize());
+ ASSERT_EQ(*((int *)addr), 0);
+ munmap(data, data_len);
+ }
}
FIXTURE(snapshot) {
base-commit: c65920c76a977c2b73c3a8b03b4c0c00cc1285ed
--
2.45.2.803.g4e1b14247a-goog
[PATCH v3] module: Add log info for verifying module signature
Add log information in kernel-space when loading module failures.
Try to load the unsigned module and the module with bad signature
when set 1 to /sys/module/module/parameters/sig_enforce.
Unsigned module case:
(linux) insmod unsigned.ko
[ 18.714661] Loading of unsigned module is rejected
insmod: can't insert 'unsigned.ko': Key was rejected by service
(linux)
Bad signature module case:
(linux) insmod bad_signature.ko
insmod: can't insert 'bad_signature.ko': Key was rejected by service
(linux)
There have different logging behavior the bad signature case only log
in user-space, add log info for fatal errors in module_sig_check().
Signed-off-by: Yusong Gao
---
V3: Clarify the message type and the error code meaning.
V2: Change print level from notice to debug.
---
kernel/module/signing.c | 27 +++
1 file changed, 27 insertions(+)
diff --git a/kernel/module/signing.c b/kernel/module/signing.c
index a2ff4242e623..826cdab8e3e4 100644
--- a/kernel/module/signing.c
+++ b/kernel/module/signing.c
@@ -67,6 +67,31 @@ int mod_verify_sig(const void *mod, struct load_info *info)
NULL, NULL);
}
+static const char *mod_decode_error(int errno)
+{
+ char *errstr = "Unrecognized error";
+
+ switch (errno) {
+ case -ENOMEM:
+ errstr = "Out of memory";
+ break;
+ case -EINVAL:
+ errstr = "Invalid argument";
+ break;
+ case -EBADMSG:
+ errstr = "Invaild module signature format";
+ break;
+ case -EMSGSIZE:
+ errstr = "Message too long";
+ break;
+ case -EKEYREJECTED:
+ errstr = "Key was rejected by service";
+ break;
+ }
+
+ return errstr;
+}
+
int module_sig_check(struct load_info *info, int flags)
{
int err = -ENODATA;
@@ -113,6 +138,8 @@ int module_sig_check(struct load_info *info, int flags)
* unparseable signatures, and signature check failures --
* even if signatures aren't required.
*/
+ pr_debug("Verifying module signature failed: %s\n",
+mod_decode_error(err));
return err;
}
--
2.34.1
Re: [PATCH] remoteproc: mediatek: Don't attempt to remap l1tcm memory if missing
Il 27/06/24 23:20, Nícolas F. R. A. Prado ha scritto:
The current code doesn't check whether platform_get_resource_byname()
succeeded to get the l1tcm memory, which is optional, before attempting
to map it. This results in the following error message when it is
missing:
mtk-scp 1050.scp: error -EINVAL: invalid resource (null)
Add a check so that the remapping is only attempted if the memory region
exists. This also allows to simplify the logic handling failure to
remap, since a failure then is always a failure.
Fixes: ca23ecfdbd44 ("remoteproc/mediatek: support L1TCM")
Signed-off-by: Nícolas F. R. A. Prado
Reviewed-by: AngeloGioacchino Del Regno
Re: [PATCH V3 2/2] soc: qcom: smp2p: Introduce tracepoint support
On 6/27/2024 4:18 PM, Sudeepgoud Patil wrote:
This commit introduces tracepoint support for smp2p, enabling
logging of communication between local and remote processors.
These tracepoints include information about the remote subsystem
name, negotiation details, supported features, bit change
notifications, and ssr activity. These logs are useful for
debugging issues between subsystems.
Signed-off-by: Sudeepgoud Patil
Reviewed-by: Deepak Kumar Singh
---
drivers/soc/qcom/Makefile | 1 +
drivers/soc/qcom/smp2p.c | 9
drivers/soc/qcom/trace-smp2p.h | 98 ++
3 files changed, 108 insertions(+)
create mode 100644 drivers/soc/qcom/trace-smp2p.h
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index ca0bece0dfff..30c1bf645501 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -23,6 +23,7 @@ qcom_rpmh-y += rpmh.o
obj-$(CONFIG_QCOM_SMD_RPM)+= rpm-proc.o smd-rpm.o
obj-$(CONFIG_QCOM_SMEM) +=smem.o
obj-$(CONFIG_QCOM_SMEM_STATE) += smem_state.o
+CFLAGS_smp2p.o := -I$(src)
obj-$(CONFIG_QCOM_SMP2P) += smp2p.o
obj-$(CONFIG_QCOM_SMSM) += smsm.o
obj-$(CONFIG_QCOM_SOCINFO)+= socinfo.o
diff --git a/drivers/soc/qcom/smp2p.c b/drivers/soc/qcom/smp2p.c
index 696c2a8387d0..4aa61b0f11ad 100644
--- a/drivers/soc/qcom/smp2p.c
+++ b/drivers/soc/qcom/smp2p.c
@@ -161,6 +161,9 @@ struct qcom_smp2p {
struct list_head outbound;
};
+#define CREATE_TRACE_POINTS
+#include "trace-smp2p.h"
+
static void qcom_smp2p_kick(struct qcom_smp2p *smp2p)
{
/* Make sure any updated data is written before the kick */
@@ -192,6 +195,7 @@ static void qcom_smp2p_do_ssr_ack(struct qcom_smp2p *smp2p)
struct smp2p_smem_item *out = smp2p->out;
u32 val;
+ trace_smp2p_ssr_ack(smp2p->dev);
smp2p->ssr_ack = !smp2p->ssr_ack;
val = out->flags & ~BIT(SMP2P_FLAGS_RESTART_ACK_BIT);
@@ -214,6 +218,7 @@ static void qcom_smp2p_negotiate(struct qcom_smp2p *smp2p)
smp2p->ssr_ack_enabled = true;
smp2p->negotiation_done = true;
+ trace_smp2p_negotiate(smp2p->dev, out->features);
}
}
@@ -252,6 +257,8 @@ static void qcom_smp2p_notify_in(struct qcom_smp2p *smp2p)
status = val ^ entry->last_value;
entry->last_value = val;
+ trace_smp2p_notify_in(entry, status, val);
+
/* No changes of this entry? */
if (!status)
continue;
@@ -415,6 +422,8 @@ static int smp2p_update_bits(void *data, u32 mask, u32
value)
writel(val, entry->value);
spin_unlock_irqrestore(>lock, flags);
+ trace_smp2p_update_bits(entry, orig, val);
+
if (val != orig)
qcom_smp2p_kick(entry->smp2p);
diff --git a/drivers/soc/qcom/trace-smp2p.h b/drivers/soc/qcom/trace-smp2p.h
new file mode 100644
index ..fa985a0d7615
--- /dev/null
+++ b/drivers/soc/qcom/trace-smp2p.h
@@ -0,0 +1,98 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM qcom_smp2p
+
+#if !defined(__QCOM_SMP2P_TRACE_H__) || defined(TRACE_HEADER_MULTI_READ)
+#define __QCOM_SMP2P_TRACE_H__
+
+#include
+#include
+
+TRACE_EVENT(smp2p_ssr_ack,
+ TP_PROTO(const struct device *dev),
+ TP_ARGS(dev),
+ TP_STRUCT__entry(
+ __string(dev_name, dev_name(dev))
+ ),
+ TP_fast_assign(
+ __assign_str(dev_name, dev_name(dev));
+ ),
+ TP_printk("%s: SSR detected", __get_str(dev_name))
+);
+
+TRACE_EVENT(smp2p_negotiate,
+ TP_PROTO(const struct device *dev, unsigned int features),
+ TP_ARGS(dev, features),
+ TP_STRUCT__entry(
+ __string(dev_name, dev_name(dev))
+ __field(u32, out_features)
+ ),
+ TP_fast_assign(
+ __assign_str(dev_name, dev_name(dev));
+ __entry->out_features = features;
+ ),
+ TP_printk("%s: state=open out_features=%s", __get_str(dev_name),
+ __print_flags(__entry->out_features, "|",
+ {SMP2P_FEATURE_SSR_ACK, "SMP2P_FEATURE_SSR_ACK"})
+ )
+);
+
+TRACE_EVENT(smp2p_notify_in,
+ TP_PROTO(struct smp2p_entry *smp2p_entry, unsigned long status, u32
val),
+ TP_ARGS(smp2p_entry, status, val),
+ TP_STRUCT__entry(
+ __string(dev_name, dev_name(smp2p_entry->smp2p->dev))
+ __string(client_name, smp2p_entry->name)
+ __field(unsigned long, status)
+ __field(u32, val)
+ ),
+ TP_fast_assign(
+ __assign_str(dev_name, dev_name(smp2p_entry->smp2p->dev));
+ __assign_str(client_name, smp2p_entry->name);
+ __entry->status = status;
+ __entry->val = val;
+ ),
+ TP_printk("%s: %s:
Re: [PATCH net-next v3 1/3] vsock: add support for SIOCOUTQ ioctl for all vsock socket types.
nit: in theory in this patch we don't support it for any of the
transports, so I wouldn't confuse and take that part out of the title.
WDYT with someting like:
vsock: add support for SIOCOUTQ ioctl
On Wed, Jun 26, 2024 at 02:08:35PM GMT, Luigi Leonardi via B4 Relay
wrote:
From: Luigi Leonardi
Add support for ioctl(s) for SOCK_STREAM SOCK_SEQPACKET and SOCK_DGRAM
in AF_VSOCK.
The only ioctl available is SIOCOUTQ/TIOCOUTQ, which returns the number
of unsent bytes in the socket. This information is transport-specific
and is delegated to them using a callback.
Suggested-by: Daan De Meyer
Signed-off-by: Luigi Leonardi
---
include/net/af_vsock.h | 3 +++
net/vmw_vsock/af_vsock.c | 60 +---
2 files changed, 60 insertions(+), 3 deletions(-)
diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
index 535701efc1e5..7b5375ae7827 100644
--- a/include/net/af_vsock.h
+++ b/include/net/af_vsock.h
@@ -169,6 +169,9 @@ struct vsock_transport {
void (*notify_buffer_size)(struct vsock_sock *, u64 *);
int (*notify_set_rcvlowat)(struct vsock_sock *vsk, int val);
+ /* SIOCOUTQ ioctl */
+ size_t (*unsent_bytes)(struct vsock_sock *vsk);
If you want to return also errors, maybe better returning ssize_t.
This should fix one of the error reported by kernel bots.
+
/* Shutdown. */
int (*shutdown)(struct vsock_sock *, int);
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index 4b040285aa78..d6140d73d122 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -112,6 +112,7 @@
#include
#include
#include
+#include
static int __vsock_bind(struct sock *sk, struct sockaddr_vm *addr);
static void vsock_sk_destruct(struct sock *sk);
@@ -1292,6 +1293,59 @@ int vsock_dgram_recvmsg(struct socket *sock, struct
msghdr *msg,
}
EXPORT_SYMBOL_GPL(vsock_dgram_recvmsg);
+static int vsock_do_ioctl(struct socket *sock, unsigned int cmd,
+ int __user *arg)
+{
+ struct sock *sk = sock->sk;
+ struct vsock_sock *vsk;
+ int retval;
+
+ vsk = vsock_sk(sk);
+
+ switch (cmd) {
+ case SIOCOUTQ: {
+ size_t n_bytes;
+
+ if (!vsk->transport || !vsk->transport->unsent_bytes) {
+ retval = -EOPNOTSUPP;
+ break;
+ }
+
+ if (vsk->transport->unsent_bytes) {
This if is not necessary after the check we did earlier, right?
Removing it should fix the other issue reported by the bot.
+ if (sock_type_connectible(sk->sk_type) && sk->sk_state
== TCP_LISTEN) {
+ retval = -EINVAL;
+ break;
+ }
+
+ n_bytes = vsk->transport->unsent_bytes(vsk);
+ if (n_bytes < 0) {
+ retval = n_bytes;
+ break;
+ }
+
+ retval = put_user(n_bytes, arg);
+ }
+ break;
+ }
+ default:
+ retval = -ENOIOCTLCMD;
+ }
+
+ return retval;
+}
+
+static int vsock_ioctl(struct socket *sock, unsigned int cmd,
+ unsigned long arg)
+{
+ int ret;
+
+ lock_sock(sock->sk);
+ ret = vsock_do_ioctl(sock, cmd, (int __user *)arg);
+ release_sock(sock->sk);
+
+ return ret;
+}
+
static const struct proto_ops vsock_dgram_ops = {
.family = PF_VSOCK,
.owner = THIS_MODULE,
@@ -1302,7 +1356,7 @@ static const struct proto_ops vsock_dgram_ops = {
.accept = sock_no_accept,
.getname = vsock_getname,
.poll = vsock_poll,
- .ioctl = sock_no_ioctl,
+ .ioctl = vsock_ioctl,
.listen = sock_no_listen,
.shutdown = vsock_shutdown,
.sendmsg = vsock_dgram_sendmsg,
@@ -2286,7 +2340,7 @@ static const struct proto_ops vsock_stream_ops = {
.accept = vsock_accept,
.getname = vsock_getname,
.poll = vsock_poll,
- .ioctl = sock_no_ioctl,
+ .ioctl = vsock_ioctl,
.listen = vsock_listen,
.shutdown = vsock_shutdown,
.setsockopt = vsock_connectible_setsockopt,
@@ -2308,7 +2362,7 @@ static const struct proto_ops vsock_seqpacket_ops = {
.accept = vsock_accept,
.getname = vsock_getname,
.poll = vsock_poll,
- .ioctl = sock_no_ioctl,
+ .ioctl = vsock_ioctl,
.listen = vsock_listen,
.shutdown = vsock_shutdown,
.setsockopt = vsock_connectible_setsockopt,
--
2.45.2
Re: [PATCH] arm64: dts: qcom: sm7225-fairphone-fp4: Name the regulators
On Thu, Jun 27, 2024 at 03:15:54PM GMT, Luca Weiss wrote: > Without explicitly specifying names for the regulators they are named > based on the DeviceTree node name. This results in multiple regulators > with the same name, making debug prints and regulator_summary impossible > to reason about. > > Signed-off-by: Luca Weiss > --- > arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts | 34 > +++ > 1 file changed, 34 insertions(+) > Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH 06/12] uprobes: add batch uprobe register/unregister APIs
On Thu, 27 Jun 2024 09:47:10 -0700 Andrii Nakryiko wrote: > On Thu, Jun 27, 2024 at 6:04 AM Masami Hiramatsu wrote: > > > > On Mon, 24 Jun 2024 17:21:38 -0700 > > Andrii Nakryiko wrote: > > > > > -static int __uprobe_register(struct inode *inode, loff_t offset, > > > - loff_t ref_ctr_offset, struct uprobe_consumer > > > *uc) > > > +int uprobe_register_batch(struct inode *inode, int cnt, > > > + uprobe_consumer_fn get_uprobe_consumer, void *ctx) > > > > Is this interface just for avoiding memory allocation? Can't we just > > allocate a temporary array of *uprobe_consumer instead? > > Yes, exactly, to avoid the need for allocating another array that > would just contain pointers to uprobe_consumer. Consumers would never > just have an array of `struct uprobe_consumer *`, because > uprobe_consumer struct is embedded in some other struct, so the array > interface isn't the most convenient. OK, I understand it. > > If you feel strongly, I can do an array, but this necessitates > allocating an extra array *and keeping it* for the entire duration of > BPF multi-uprobe link (attachment) existence, so it feels like a > waste. This is because we don't want to do anything that can fail in > the detachment logic (so no temporary array allocation there). No need to change it, that sounds reasonable. > > Anyways, let me know how you feel about keeping this callback. IMHO, maybe the interface function is better to change to `uprobe_consumer *next_uprobe_consumer(void **data)`. If caller side uses a linked list of structure, index access will need to follow the list every time. Thank you, > > > > > Thank you, > > > > -- > > Masami Hiramatsu (Google) -- Masami Hiramatsu (Google)
Re: [PATCH net-next v3 1/3] vsock: add support for SIOCOUTQ ioctl for all vsock socket types.
Hi Luigi,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 50b70845fc5c22cf7e7d25b57d57b3dca1725aa5]
url:
https://github.com/intel-lab-lkp/linux/commits/Luigi-Leonardi-via-B4-Relay/vsock-add-support-for-SIOCOUTQ-ioctl-for-all-vsock-socket-types/20240627-023902
base: 50b70845fc5c22cf7e7d25b57d57b3dca1725aa5
patch link:
https://lore.kernel.org/r/20240626-ioctl_next-v3-1-63be5bf19a40%40outlook.com
patch subject: [PATCH net-next v3 1/3] vsock: add support for SIOCOUTQ ioctl
for all vsock socket types.
config: i386-randconfig-141-20240628
(https://download.01.org/0day-ci/archive/20240628/202406281355.d1jnvgbc-...@intel.com/config)
compiler: gcc-8 (Ubuntu 8.4.0-3ubuntu2) 8.4.0
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot
| Closes:
https://lore.kernel.org/oe-kbuild-all/202406281355.d1jnvgbc-...@intel.com/
smatch warnings:
net/vmw_vsock/af_vsock.c:1321 vsock_do_ioctl() warn: unsigned 'n_bytes' is
never less than zero.
vim +/n_bytes +1321 net/vmw_vsock/af_vsock.c
1295
1296 static int vsock_do_ioctl(struct socket *sock, unsigned int cmd,
1297int __user *arg)
1298 {
1299 struct sock *sk = sock->sk;
1300 struct vsock_sock *vsk;
1301 int retval;
1302
1303 vsk = vsock_sk(sk);
1304
1305 switch (cmd) {
1306 case SIOCOUTQ: {
1307 size_t n_bytes;
1308
1309 if (!vsk->transport || !vsk->transport->unsent_bytes) {
1310 retval = -EOPNOTSUPP;
1311 break;
1312 }
1313
1314 if (vsk->transport->unsent_bytes) {
1315 if (sock_type_connectible(sk->sk_type) &&
sk->sk_state == TCP_LISTEN) {
1316 retval = -EINVAL;
1317 break;
1318 }
1319
1320 n_bytes = vsk->transport->unsent_bytes(vsk);
> 1321 if (n_bytes < 0) {
1322 retval = n_bytes;
1323 break;
1324 }
1325
1326 retval = put_user(n_bytes, arg);
1327 }
1328 break;
1329 }
1330 default:
1331 retval = -ENOIOCTLCMD;
1332 }
1333
1334 return retval;
1335 }
1336
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
