Re: [PATCH v2 1/2] time/timekeeping: Fix possible inconsistencies in _COARSE clockids
On Thu, Mar 20, 2025 at 01:03:00PM -0700, John Stultz wrote:
> +static u64 timekeeping_accumulate(struct timekeeper *tk, u64 offset,
> + enum timekeeping_adv_mode mode,
> + unsigned int *clock_set)
> + * Also reset tk::ntp_error as it does not make sense to keep the
> + * old accumulated error around in this case.
> + */
I'm not sure if I still understand the timekeeping code correctly, but
that doesn't seem right to me. At least the comment should explain why
it does not make sense to keep the NTP error.
Resetting the NTP error causes a small time step. An NTP/PTP client
can be setting the frequency very frequently, e.g. up to 128 times per
second and the interval between updates can be random. If the timing
was right, I suspect it could cause a measurable drift. The client
should be able to compensate for it, but why make its job harder by
making the clock less predictable. My expectation for the clock is
that its frequency will not change if the same (or only slightly
different) frequency is set repeatedly by adjtimex().
> + if (mode == TK_ADV_FREQ) {
> + timekeeping_forward(tk, tk->tkr_mono.cycle_last + offset);
> + tk->ntp_error = 0;
> + return 0;
> + }
--
Miroslav Lichvar
Re: [PATCH v15 2/8] remoteproc: Add TEE support
On 12/6/24 23:07, Bjorn Andersson wrote:
> On Thu, Nov 28, 2024 at 09:42:09AM GMT, Arnaud Pouliquen wrote:
>> Add a remoteproc TEE (Trusted Execution Environment) driver
>> that will be probed by the TEE bus. If the associated Trusted
>> application is supported on secure part this driver offers a client
>> interface to load a firmware by the secure part.
>
> If...else?
>
>> This firmware could be authenticated by the secure trusted application.
>>
>
> I would like for this to fully describe how this fits with the bus and
> how it is expected to be used by a specific remoteproc driver.
>
>> Signed-off-by: Arnaud Pouliquen
>> ---
>> Updates vs version v13:
>> - define REMOTEPROC_TEE as bool instead of tristate,
>> - remove the load of the firmware in rproc_tee_parse_fw as we will ensure
>> that the firmware is loaded using the load_fw() operation.
>> ---
>> drivers/remoteproc/Kconfig | 10 +
>> drivers/remoteproc/Makefile | 1 +
>> drivers/remoteproc/remoteproc_tee.c | 508
>> include/linux/remoteproc.h | 4 +
>> include/linux/remoteproc_tee.h | 105 ++
>> 5 files changed, 628 insertions(+)
>> create mode 100644 drivers/remoteproc/remoteproc_tee.c
>> create mode 100644 include/linux/remoteproc_tee.h
[...]
>> +
>> +MODULE_DEVICE_TABLE(tee, rproc_tee_id_table);
>> +
>> +static struct tee_client_driver rproc_tee_fw_driver = {
>> +.id_table = rproc_tee_id_table,
>> +.driver = {
>> +.name = KBUILD_MODNAME,
>> +.bus= &tee_bus_type,
>> +.probe = rproc_tee_probe,
>> +.remove = rproc_tee_remove,
>> +},
>> +};
>> +
>> +static int __init rproc_tee_fw_mod_init(void)
>> +{
>> +return driver_register(&rproc_tee_fw_driver.driver);
>> +}
>> +
>> +static void __exit rproc_tee_fw_mod_exit(void)
>> +{
>> +driver_unregister(&rproc_tee_fw_driver.driver);
>> +}
>> +
>> +module_init(rproc_tee_fw_mod_init);
>> +module_exit(rproc_tee_fw_mod_exit);
>
> Please add an equivalent of the module_platform_driver() macro to tee
> framework instead of open-coding this.
>
It is not possible to use equivalent of the module_platform_driver() macro
as the device is on the TEE bus.
I followed recommendation provided in Documentation/driver-api/tee.rst[1]
Or do you have an alternative in mind?
Thanks,
Arnaud
[1]https://elixir.bootlin.com/linux/v6.14-rc6/source/Documentation/driver-api/tee.rst
>> +
>> +MODULE_DESCRIPTION(" remote processor TEE module");
>> +MODULE_LICENSE("GPL");
>> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
>> index 8fd0d7f63c8e..2e0ddcb2d792 100644
>> --- a/include/linux/remoteproc.h
>> +++ b/include/linux/remoteproc.h
>> @@ -503,6 +503,8 @@ enum rproc_features {
>> RPROC_MAX_FEATURES,
>> };
>>
>> +struct rproc_tee;
>> +
>> /**
>> * struct rproc - represents a physical remote processor device
>> * @node: list node of this rproc object
>> @@ -545,6 +547,7 @@ enum rproc_features {
>> * @cdev: character device of the rproc
>> * @cdev_put_on_release: flag to indicate if remoteproc should be shutdown
>> on @char_dev release
>> * @features: indicate remoteproc features
>> + * @rproc_tee_itf: pointer to the remoteproc tee context
>> */
>> struct rproc {
>> struct list_head node;
>> @@ -586,6 +589,7 @@ struct rproc {
>> struct cdev cdev;
>> bool cdev_put_on_release;
>> DECLARE_BITMAP(features, RPROC_MAX_FEATURES);
>> +struct rproc_tee *rproc_tee_itf;
>
> TEE is just one specific remoteproc implementation, why does it need to
> infest the core data structure? Do you want a stm32_rproc here as well?
>
>> };
>>
>> /**
>> diff --git a/include/linux/remoteproc_tee.h b/include/linux/remoteproc_tee.h
>> new file mode 100644
>> index ..9b498a8eff4d
>> --- /dev/null
>> +++ b/include/linux/remoteproc_tee.h
>> @@ -0,0 +1,105 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +/*
>> + * Copyright(c) 2024 STMicroelectronics
>> + */
>> +
>> +#ifndef REMOTEPROC_TEE_H
>> +#define REMOTEPROC_TEE_H
>> +
>> +#include
>> +#include
>> +#include
>> +
>> +struct rproc;
>> +
>> +/**
>> + * struct rproc_tee - TEE remoteproc structure
>> + * @node: Reference in list
>> + * @rproc: Remoteproc reference
>> + * @parent: Parent device
>
> Isn't that rproc->dev->parent?
>
>> + * @rproc_id: Identifier of the target firmware
>> + * @session_id: TEE session identifier
>> + */
>> +struct rproc_tee {
>
> As far as I can tell this isn't dereferenced outside remoteproc_tee.c,
> can we hide it therein?
>
>> +struct list_head node;
>> +struct rproc *rproc;
>> +struct device *parent;
>> +u32 rproc_id;
>> +u32 session_id;
>> +};
>> +
>> +#if IS_REACHABLE(CONFIG_REMOTEPROC_TEE)
>> +
>> +int rproc_tee_register(struct device *dev, struct rproc *rproc, unsigned
>> int rproc_id);
>> +int rproc_tee
Re: [PATCH net-next v24 18/23] ovpn: implement peer add/get/dump/delete via netlink
2025-03-25, 00:15:48 +0100, Antonio Quartulli wrote:
> On 24/03/2025 11:48, Sabrina Dubroca wrote:
> > Hello Antonio,
> >
> > A few questions wrt the API:
> >
> > 2025-03-18, 02:40:53 +0100, Antonio Quartulli wrote:
> > > +static bool ovpn_nl_attr_sockaddr_remote(struct nlattr **attrs,
> > > + struct sockaddr_storage *ss)
> > > +{
> > > + struct sockaddr_in6 *sin6;
> > > + struct sockaddr_in *sin;
> > > + struct in6_addr *in6;
> > > + __be16 port = 0;
> > > + __be32 *in;
> > > +
> > > + ss->ss_family = AF_UNSPEC;
> > > +
> > > + if (attrs[OVPN_A_PEER_REMOTE_PORT])
> > > + port = nla_get_be16(attrs[OVPN_A_PEER_REMOTE_PORT]);
> >
> > What's the expected behavior if REMOTE_PORT isn't provided? We'll send
> > packets do port 0 (which I'm guessing will get dropped on the other
> > side) until we get a message from the peer and float sets the correct
> > port/address?
>
> I have never seen a packet going out with port 0 :)
It will if you hack into ovpn-cli to skip OVPN_A_PEER_REMOTE_PORT.
I don't know how networks/admins react to such packets.
> But being dropped is most likely what's going to happen.
>
> I'd say this is not something that we expect the user to do:
> if the remote address if specified, the user should specify a non-zero port
> too.
>
> We could add a check to ensure that a port is always specified if the remote
> address is there too, just to avoid the user to shoot himself in the foot.
> But we expect the user to pass an addr:port where the peer is listening to
> (and that can't be a 0 port).
If we expect that (even if a well-behaved userspace would never do
it), I have a preference for enforcing that expectation. Since there's
already a policy rejecting OVPN_A_PEER_REMOTE_PORT == 0, this would be
more consistent IMO.
An alternative would be to select a default (non-zero) port if none is
provided.
> >
> >
> > > +static int ovpn_nl_peer_modify(struct ovpn_peer *peer, struct genl_info
> > > *info,
> > > +struct nlattr **attrs)
> > > +{
> > [...]
> > > + /* when setting the keepalive, both parameters have to be configured */
> > > + if (attrs[OVPN_A_PEER_KEEPALIVE_INTERVAL] &&
> > > + attrs[OVPN_A_PEER_KEEPALIVE_TIMEOUT]) {
> > > + interv = nla_get_u32(attrs[OVPN_A_PEER_KEEPALIVE_INTERVAL]);
> > > + timeout = nla_get_u32(attrs[OVPN_A_PEER_KEEPALIVE_TIMEOUT]);
> > > + ovpn_peer_keepalive_set(peer, interv, timeout);
> >
> > Should we interpret OVPN_A_PEER_KEEPALIVE_INTERVAL = 0 &&
> > OVPN_A_PEER_KEEPALIVE_TIMEOUT == 0 as "disable keepalive/timeout" on
> > an active peer? And maybe "one set to 0, the other set to some
> > non-zero value" as invalid? Setting either value to 0 doesn't seem
> > very useful (timeout = 0 will probably kill the peer immediately, and
> > I suspect interval = 0 would be quite spammy).
> >
>
> Considering "0" as "disable keepalive" is the current intention.
>
> In ovpn_peer_keepalive_work_single() you can see that if either one if 0, we
> just skip the peer:
>
> 1217 /* we expect both timers to be configured at the same time,
> 1218 * therefore bail out if either is not set
> 1219 */
> 1220 if (!peer->keepalive_timeout || !peer->keepalive_interval) {
> 1221 spin_unlock_bh(&peer->lock);
> 1222 return 0;
> 1223 }
>
> does it make sense?
Ah, true. Sorry, I forgot about that. So after _NEW/_SET we'll run
the work once, and that peer will be ignored. And if there's no other
peer requiring keepalive, next_run will be 0 and we don't
reschedule. That's good, thanks.
--
Sabrina
Re: [PATCH] virtio_console: fix order of fields cols and rows
On Mon, Mar 24, 2025 at 12:53:29PM -0700, Daniel Verkamp wrote:
> On Mon, Mar 24, 2025 at 7:43 AM Maximilian Immanuel Brandtner
> wrote:
> >
> > According to section 5.3.6.2 (Multiport Device Operation) of the virtio
> > spec(version 1.2) a control buffer with the event VIRTIO_CONSOLE_RESIZE
> > is followed by a virtio_console_resize struct containing cols then rows.
> > The kernel implements this the wrong way around (rows then cols) resulting
> > in the two values being swapped.
> >
> > Signed-off-by: Maximilian Immanuel Brandtner
> > ---
> > drivers/char/virtio_console.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
> > index 21de774996ad..38af3029da39 100644
> > --- a/drivers/char/virtio_console.c
> > +++ b/drivers/char/virtio_console.c
> > @@ -1579,8 +1579,8 @@ static void handle_control_message(struct
> > virtio_device *vdev,
> > break;
> > case VIRTIO_CONSOLE_RESIZE: {
> > struct {
> > - __virtio16 rows;
> > __virtio16 cols;
> > + __virtio16 rows;
> > } size;
>
> The order of the fields after the patch matches the spec, so from that
> perspective, looks fine:
> Reviewed-by: Daniel Verkamp
>
> Since the driver code has been using the wrong order since support for
> this message was added in 2010, but there is no support for sending
> this message in the current qemu device implementation, I wondered
> what device code was used to test this when it was originally added. I
> dug up what I assume is the corresponding qemu device change from the
> same era, which sends the VIRTIO_CONSOLE_RESIZE message using the
> rows, cols order that matches the kernel driver (and differs from the
> spec):
>
> https://lore.kernel.org/qemu-devel/1273092505-22783-1-git-send-email-amit.s...@redhat.com/
> ("[Qemu-devel] [PATCH] virtio-serial: Send per-console port resize
> notifications to guest", May 6, 2010)
>
> However, I don't believe that patch ever made it into an actual qemu
> release, so it's probably not a compatibility concern. (If there are
> any other device implementations that use the kernel driver order
> rather than the spec order, then maybe this would need more
> consideration, but I don't personally know of any.)
>
> Thanks,
> -- Daniel
I agree. Cc author of the qemu patch for confirmation.
--
MST
Re: [PATCH net-next v24 09/23] ovpn: implement packet processing
2025-03-24, 21:53:02 +0100, Antonio Quartulli wrote:
> On 24/03/2025 12:02, Sabrina Dubroca wrote:
> > 2025-03-18, 02:40:44 +0100, Antonio Quartulli wrote:
> > > +int ovpn_crypto_state_reset(struct ovpn_crypto_state *cs,
> > > + const struct ovpn_peer_key_reset *pkr)
> > > +{
> > > + struct ovpn_crypto_key_slot *old = NULL, *new;
> > > + u8 idx;
> > > +
> > > + if (pkr->slot != OVPN_KEY_SLOT_PRIMARY &&
> > > + pkr->slot != OVPN_KEY_SLOT_SECONDARY)
> > > + return -EINVAL;
> > > +
> > > + new = ovpn_aead_crypto_key_slot_new(&pkr->key);
> > > + if (IS_ERR(new))
> > > + return PTR_ERR(new);
> > > +
> > > + spin_lock_bh(&cs->lock);
> >
> > At this point, should there be a check that we're not installing 2
> > keys with the same key_id at the same time? I expect a well-behaved
> > userspace never does that, but it would confuse
> > ovpn_crypto_key_id_to_slot if it ever happened.
> >
> > ["well, then the tunnel is broken. if userspace sets up a broken
> > config that's not the kernel's problem." is an acceptable answer]
> >
>
> The behaviour of ovpn_crypto_key_id_to_slot() is still "deterministic" as we
> will first lookup the primary key.
>
> Therefore we will simply always use the primary key and never the other,
> which is what we should expect in this situation from the code.
>
> I'd say this is just an ill-formed configuration, yet not invalid.
> As per your statement, I'd say it's userspace's problem.
Ok, sounds good, thanks.
--
Sabrina
[PATCH v16 1/6] remoteproc: core: Introduce rproc_pa_to_va helper
When a resource table is loaded by an external entity such as U-boot or
OP-TEE, we do not necessarily get the device address(da) but the physical
address(pa).
This helper performs similar translation than the rproc_da_to_va()
but based on a physical address.
Signed-off-by: Arnaud Pouliquen
---
drivers/remoteproc/remoteproc_core.c | 46
include/linux/remoteproc.h | 1 +
2 files changed, 47 insertions(+)
diff --git a/drivers/remoteproc/remoteproc_core.c
b/drivers/remoteproc/remoteproc_core.c
index c2cf0d277729..f05122b699b7 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -230,6 +230,52 @@ void *rproc_da_to_va(struct rproc *rproc, u64 da, size_t
len, bool *is_iomem)
}
EXPORT_SYMBOL(rproc_da_to_va);
+/**
+ * rproc_pa_to_va() - lookup the kernel virtual address for a physical address
of a remoteproc
+ * memory
+ *
+ * @rproc: handle of a remote processor
+ * @pa: remoteproc physical address
+ * @len: length of the memory region @pa is pointing to
+ * @is_iomem: optional pointer filled in to indicate if @da is iomapped memory
+ *
+ * This function is a helper function similar to rproc_da_to_va() but it deals
with physical
+ * addresses instead of device addresses.
+ *
+ * Return: a valid kernel address on success or NULL on failure
+ */
+void *rproc_pa_to_va(struct rproc *rproc, phys_addr_t pa, size_t len, bool
*is_iomem)
+{
+ struct rproc_mem_entry *carveout;
+ void *ptr = NULL;
+
+ list_for_each_entry(carveout, &rproc->carveouts, node) {
+ int offset = pa - carveout->dma;
+
+ /* Verify that carveout is allocated */
+ if (!carveout->va)
+ continue;
+
+ /* try next carveout if da is too small */
+ if (offset < 0)
+ continue;
+
+ /* try next carveout if da is too large */
+ if (offset + len > carveout->len)
+ continue;
+
+ ptr = carveout->va + offset;
+
+ if (is_iomem)
+ *is_iomem = carveout->is_iomem;
+
+ break;
+ }
+
+ return ptr;
+}
+EXPORT_SYMBOL(rproc_pa_to_va);
+
/**
* rproc_find_carveout_by_name() - lookup the carveout region by a name
* @rproc: handle of a remote processor
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index b4795698d8c2..8fd0d7f63c8e 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -690,6 +690,7 @@ int rproc_detach(struct rproc *rproc);
int rproc_set_firmware(struct rproc *rproc, const char *fw_name);
void rproc_report_crash(struct rproc *rproc, enum rproc_crash_type type);
void *rproc_da_to_va(struct rproc *rproc, u64 da, size_t len, bool *is_iomem);
+void *rproc_pa_to_va(struct rproc *rproc, phys_addr_t pa, size_t len, bool
*is_iomem);
/* from remoteproc_coredump.c */
void rproc_coredump_cleanup(struct rproc *rproc);
--
2.25.1
[PATCH v16 6/6] remoteproc: stm32: Add support of an OP-TEE TA to load the firmware
The new TEE remoteproc driver is used to manage remote firmware in a
secure, trusted context. The 'st,stm32mp1-m4-tee' compatibility is
introduced to delegate the loading of the firmware to the trusted
execution context. In such cases, the firmware should be signed and
adhere to the image format defined by the TEE.
Signed-off-by: Arnaud Pouliquen
---
drivers/remoteproc/stm32_rproc.c | 57 ++--
1 file changed, 54 insertions(+), 3 deletions(-)
diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
index b1bda314ca85..829dfd440dbf 100644
--- a/drivers/remoteproc/stm32_rproc.c
+++ b/drivers/remoteproc/stm32_rproc.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -255,6 +256,19 @@ static int stm32_rproc_release(struct rproc *rproc)
return 0;
}
+static int stm32_rproc_tee_stop(struct rproc *rproc)
+{
+ int err;
+
+ stm32_rproc_request_shutdown(rproc);
+
+ err = rproc_tee_stop(rproc);
+ if (err)
+ return err;
+
+ return stm32_rproc_release(rproc);
+}
+
static int stm32_rproc_prepare(struct rproc *rproc)
{
struct device *dev = rproc->dev.parent;
@@ -691,8 +705,20 @@ static const struct rproc_ops st_rproc_ops = {
.get_boot_addr = rproc_elf_get_boot_addr,
};
+static const struct rproc_ops st_rproc_tee_ops = {
+ .prepare= stm32_rproc_prepare,
+ .start = rproc_tee_start,
+ .stop = stm32_rproc_tee_stop,
+ .kick = stm32_rproc_kick,
+ .load = rproc_tee_load_fw,
+ .parse_fw = rproc_tee_parse_fw,
+ .find_loaded_rsc_table = rproc_tee_find_loaded_rsc_table,
+ .release_fw = rproc_tee_release_fw,
+};
+
static const struct of_device_id stm32_rproc_match[] = {
{ .compatible = "st,stm32mp1-m4" },
+ { .compatible = "st,stm32mp1-m4-tee" },
{},
};
MODULE_DEVICE_TABLE(of, stm32_rproc_match);
@@ -853,15 +879,36 @@ static int stm32_rproc_probe(struct platform_device *pdev)
struct device_node *np = dev->of_node;
struct rproc *rproc;
unsigned int state;
+ u32 proc_id;
int ret;
ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(32));
if (ret)
return ret;
- rproc = devm_rproc_alloc(dev, np->name, &st_rproc_ops, NULL,
sizeof(*ddata));
- if (!rproc)
- return -ENOMEM;
+ if (of_device_is_compatible(np, "st,stm32mp1-m4-tee")) {
+ /*
+* Delegate the firmware management to the secure context.
+* The firmware loaded has to be signed.
+*/
+ ret = of_property_read_u32(np, "st,proc-id", &proc_id);
+ if (ret) {
+ dev_err(dev, "failed to read st,rproc-id property\n");
+ return ret;
+ }
+
+ rproc = devm_rproc_alloc(dev, np->name, &st_rproc_tee_ops,
NULL, sizeof(*ddata));
+ if (!rproc)
+ return -ENOMEM;
+
+ ret = rproc_tee_register(dev, rproc, proc_id);
+ if (ret)
+ return dev_err_probe(dev, ret, "signed firmware not
supported by TEE\n");
+ } else {
+ rproc = devm_rproc_alloc(dev, np->name, &st_rproc_ops, NULL,
sizeof(*ddata));
+ if (!rproc)
+ return -ENOMEM;
+ }
ddata = rproc->priv;
@@ -913,6 +960,8 @@ static int stm32_rproc_probe(struct platform_device *pdev)
dev_pm_clear_wake_irq(dev);
device_init_wakeup(dev, false);
}
+ rproc_tee_unregister(dev, rproc);
+
return ret;
}
@@ -933,6 +982,8 @@ static void stm32_rproc_remove(struct platform_device *pdev)
dev_pm_clear_wake_irq(dev);
device_init_wakeup(dev, false);
}
+
+ rproc_tee_unregister(dev, rproc);
}
static int stm32_rproc_suspend(struct device *dev)
--
2.25.1
[PATCH v16 0/6] Introduction of a remoteproc tee to load signed firmware
Main updates from version V15[1]:
- Removed the rproc_ops:load_fw() operation introduced in the previous version.
- Returned to managing the remoteproc firmware loading in rproc_tee_parse_fw to
load and authenticate the firmware before getting the resource table.
- Added spinlock and dev_link mechanisms in remoteproc TEE to better manage
bind/unbind.
More details are available in each patch commit message.
[1]
https://lore.kernel.org/linux-remoteproc/20241128084219.2159197-7-arnaud.pouliq...@foss.st.com/T/
Tested-on: commit 0a0ba9924445 ("Linux 6.14-rc7")
Description of the feature:
--
This series proposes the implementation of a remoteproc tee driver to
communicate with a TEE trusted application responsible for authenticating
and loading the remoteproc firmware image in an Arm secure context.
1) Principle:
The remoteproc tee driver provides services to communicate with the OP-TEE
trusted application running on the Trusted Execution Context (TEE).
The trusted application in TEE manages the remote processor lifecycle:
- authenticating and loading firmware images,
- isolating and securing the remote processor memories,
- supporting multi-firmware (e.g., TF-M + Zephyr on a Cortex-M33),
- managing the start and stop of the firmware by the TEE.
2) Format of the signed image:
Refer to:
https://github.com/OP-TEE/optee_os/blob/master/ta/remoteproc/src/remoteproc_core.c#L18-L57
3) OP-TEE trusted application API:
Refer to:
https://github.com/OP-TEE/optee_os/blob/master/ta/remoteproc/include/ta_remoteproc.h
4) OP-TEE signature script
Refer to:
https://github.com/OP-TEE/optee_os/blob/master/scripts/sign_rproc_fw.py
Example of usage:
sign_rproc_fw.py --in --in --out --key
${OP-TEE_PATH}/keys/default.pem
5) Impact on User space Application
No sysfs impact. The user only needs to provide the signed firmware image
instead of the ELF image.
For more information about the implementation, a presentation is available here
(note that the format of the signed image has evolved between the presentation
and the integration in OP-TEE).
https://resources.linaro.org/en/resource/6c5bGvZwUAjX56fvxthxds
Arnaud Pouliquen (6):
remoteproc: core: Introduce rproc_pa_to_va helper
remoteproc: Add TEE support
remoteproc: Introduce release_fw optional operation
dt-bindings: remoteproc: Add compatibility for TEE support
remoteproc: stm32: Create sub-functions to request shutdown and
release
remoteproc: stm32: Add support of an OP-TEE TA to load the firmware
.../bindings/remoteproc/st,stm32-rproc.yaml | 58 +-
drivers/remoteproc/Kconfig| 10 +
drivers/remoteproc/Makefile | 1 +
drivers/remoteproc/remoteproc_core.c | 52 ++
drivers/remoteproc/remoteproc_internal.h | 6 +
drivers/remoteproc/remoteproc_tee.c | 619 ++
drivers/remoteproc/stm32_rproc.c | 139 +++-
include/linux/remoteproc.h| 4 +
include/linux/remoteproc_tee.h| 90 +++
9 files changed, 935 insertions(+), 44 deletions(-)
create mode 100644 drivers/remoteproc/remoteproc_tee.c
create mode 100644 include/linux/remoteproc_tee.h
base-commit: 0a0ba99244455fea8706c4a53f5f66a45d87905d
--
2.25.1
[PATCH v16 5/6] remoteproc: stm32: Create sub-functions to request shutdown and release
To prepare for the support of TEE remoteproc, create sub-functions
that can be used in both cases, with and without remoteproc TEE support.
Signed-off-by: Arnaud Pouliquen
---
drivers/remoteproc/stm32_rproc.c | 82 +++-
1 file changed, 49 insertions(+), 33 deletions(-)
diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
index b02b36a3f515..b1bda314ca85 100644
--- a/drivers/remoteproc/stm32_rproc.c
+++ b/drivers/remoteproc/stm32_rproc.c
@@ -209,6 +209,52 @@ static int stm32_rproc_mbox_idx(struct rproc *rproc, const
unsigned char *name)
return -EINVAL;
}
+static void stm32_rproc_request_shutdown(struct rproc *rproc)
+{
+ struct stm32_rproc *ddata = rproc->priv;
+ int err, idx;
+
+ /* Request shutdown of the remote processor */
+ if (rproc->state != RPROC_OFFLINE && rproc->state != RPROC_CRASHED) {
+ idx = stm32_rproc_mbox_idx(rproc, STM32_MBX_SHUTDOWN);
+ if (idx >= 0 && ddata->mb[idx].chan) {
+ err = mbox_send_message(ddata->mb[idx].chan, "detach");
+ if (err < 0)
+ dev_warn(&rproc->dev, "warning: remote FW
shutdown without ack\n");
+ }
+ }
+}
+
+static int stm32_rproc_release(struct rproc *rproc)
+{
+ struct stm32_rproc *ddata = rproc->priv;
+ unsigned int err = 0;
+
+ /* To allow platform Standby power mode, set remote proc Deep Sleep */
+ if (ddata->pdds.map) {
+ err = regmap_update_bits(ddata->pdds.map, ddata->pdds.reg,
+ddata->pdds.mask, 1);
+ if (err) {
+ dev_err(&rproc->dev, "failed to set pdds\n");
+ return err;
+ }
+ }
+
+ /* Update coprocessor state to OFF if available */
+ if (ddata->m4_state.map) {
+ err = regmap_update_bits(ddata->m4_state.map,
+ddata->m4_state.reg,
+ddata->m4_state.mask,
+M4_STATE_OFF);
+ if (err) {
+ dev_err(&rproc->dev, "failed to set copro state\n");
+ return err;
+ }
+ }
+
+ return 0;
+}
+
static int stm32_rproc_prepare(struct rproc *rproc)
{
struct device *dev = rproc->dev.parent;
@@ -519,17 +565,9 @@ static int stm32_rproc_detach(struct rproc *rproc)
static int stm32_rproc_stop(struct rproc *rproc)
{
struct stm32_rproc *ddata = rproc->priv;
- int err, idx;
+ int err;
- /* request shutdown of the remote processor */
- if (rproc->state != RPROC_OFFLINE && rproc->state != RPROC_CRASHED) {
- idx = stm32_rproc_mbox_idx(rproc, STM32_MBX_SHUTDOWN);
- if (idx >= 0 && ddata->mb[idx].chan) {
- err = mbox_send_message(ddata->mb[idx].chan, "detach");
- if (err < 0)
- dev_warn(&rproc->dev, "warning: remote FW
shutdown without ack\n");
- }
- }
+ stm32_rproc_request_shutdown(rproc);
err = stm32_rproc_set_hold_boot(rproc, true);
if (err)
@@ -541,29 +579,7 @@ static int stm32_rproc_stop(struct rproc *rproc)
return err;
}
- /* to allow platform Standby power mode, set remote proc Deep Sleep */
- if (ddata->pdds.map) {
- err = regmap_update_bits(ddata->pdds.map, ddata->pdds.reg,
-ddata->pdds.mask, 1);
- if (err) {
- dev_err(&rproc->dev, "failed to set pdds\n");
- return err;
- }
- }
-
- /* update coprocessor state to OFF if available */
- if (ddata->m4_state.map) {
- err = regmap_update_bits(ddata->m4_state.map,
-ddata->m4_state.reg,
-ddata->m4_state.mask,
-M4_STATE_OFF);
- if (err) {
- dev_err(&rproc->dev, "failed to set copro state\n");
- return err;
- }
- }
-
- return 0;
+ return stm32_rproc_release(rproc);
}
static void stm32_rproc_kick(struct rproc *rproc, int vqid)
--
2.25.1
[PATCH v16 3/6] remoteproc: Introduce release_fw optional operation
The release_fw operation is the inverse operation of the load, responsible
for releasing the remote processor resources configured from the loading
of the remoteproc firmware (e.g., memories).
The operation is called in the following cases:
- An error occurs on boot of the remote processor.
- An error occurs on recovery start of the remote processor.
- After stopping the remote processor.
This operation is needed for the remoteproc_tee implementation after stop
and on error.
Indeed, as the remoteproc image is loaded when we parse the resource
table, there are many situations where something can go wrong before
the start of the remote processor(resource handling, carveout allocation,
...).
Signed-off-by: Arnaud Pouliquen
---
Updates vs previous version:
- remove the rproc:load_fw() ops introduced in previous version
- remove duplicate call of rproc_release_fw in rproc_fw_boot and rproc_boot
---
drivers/remoteproc/remoteproc_core.c | 6 ++
drivers/remoteproc/remoteproc_internal.h | 6 ++
include/linux/remoteproc.h | 3 +++
3 files changed, 15 insertions(+)
diff --git a/drivers/remoteproc/remoteproc_core.c
b/drivers/remoteproc/remoteproc_core.c
index f05122b699b7..79317d148651 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -1857,6 +1857,8 @@ static int rproc_boot_recovery(struct rproc *rproc)
/* boot the remote processor up again */
ret = rproc_start(rproc, firmware_p);
+ if (ret)
+ rproc_release_fw(rproc);
release_firmware(firmware_p);
@@ -1998,6 +2000,8 @@ int rproc_boot(struct rproc *rproc)
}
ret = rproc_fw_boot(rproc, firmware_p);
+ if (ret)
+ rproc_release_fw(rproc);
release_firmware(firmware_p);
}
@@ -2067,6 +2071,8 @@ int rproc_shutdown(struct rproc *rproc)
rproc_disable_iommu(rproc);
+ rproc_release_fw(rproc);
+
/* Free the copy of the resource table */
kfree(rproc->cached_table);
rproc->cached_table = NULL;
diff --git a/drivers/remoteproc/remoteproc_internal.h
b/drivers/remoteproc/remoteproc_internal.h
index 0cd09e67ac14..c7fb908f8652 100644
--- a/drivers/remoteproc/remoteproc_internal.h
+++ b/drivers/remoteproc/remoteproc_internal.h
@@ -221,4 +221,10 @@ bool rproc_u64_fit_in_size_t(u64 val)
return (val <= (size_t) -1);
}
+static inline void rproc_release_fw(struct rproc *rproc)
+{
+ if (rproc->ops->release_fw)
+ rproc->ops->release_fw(rproc);
+}
+
#endif /* REMOTEPROC_INTERNAL_H */
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index 8fd0d7f63c8e..80128461972b 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -381,6 +381,8 @@ enum rsc_handling_status {
* @panic: optional callback to react to system panic, core will delay
* panic at least the returned number of milliseconds
* @coredump:collect firmware dump after the subsystem is shutdown
+ * @release_fw:optional function to release the loaded firmware,
called after
+ * stopping the remote processor or in case of error
*/
struct rproc_ops {
int (*prepare)(struct rproc *rproc);
@@ -403,6 +405,7 @@ struct rproc_ops {
u64 (*get_boot_addr)(struct rproc *rproc, const struct firmware *fw);
unsigned long (*panic)(struct rproc *rproc);
void (*coredump)(struct rproc *rproc);
+ void (*release_fw)(struct rproc *rproc);
};
/**
--
2.25.1
Re: [PATCH 03/10] samples/livepatch: add module descriptions
On Mon 2025-03-24 18:32:28, Arnd Bergmann wrote:
> From: Arnd Bergmann
>
> Every module should have a description, so add one for each of these modules.
>
> --- a/samples/livepatch/livepatch-callbacks-busymod.c
> +++ b/samples/livepatch/livepatch-callbacks-busymod.c
> @@ -56,4 +56,5 @@ static void livepatch_callbacks_mod_exit(void)
>
> module_init(livepatch_callbacks_mod_init);
> module_exit(livepatch_callbacks_mod_exit);
> +MODULE_DESCRIPTION("Live patching demo for (un)patching callbacks");
This is another support module similar to livepatch-callbacks-mod.c.
I would use the same description, here:
MODULE_DESCRIPTION("Live patching demo for (un)patching callbacks, support
module");
> MODULE_LICENSE("GPL");
> diff --git a/samples/livepatch/livepatch-callbacks-demo.c
> b/samples/livepatch/livepatch-callbacks-demo.c
> index 11c3f4357812..9e69d9caed25 100644
> --- a/samples/livepatch/livepatch-callbacks-demo.c
> +++ b/samples/livepatch/livepatch-callbacks-demo.c
> @@ -192,5 +192,6 @@ static void livepatch_callbacks_demo_exit(void)
>
> module_init(livepatch_callbacks_demo_init);
> module_exit(livepatch_callbacks_demo_exit);
> +MODULE_DESCRIPTION("Live patching demo for (un)patching callbacks");
> MODULE_LICENSE("GPL");
> MODULE_INFO(livepatch, "Y");
> diff --git a/samples/livepatch/livepatch-callbacks-mod.c
> b/samples/livepatch/livepatch-callbacks-mod.c
> index 2a074f422a51..d1851b471ad9 100644
> --- a/samples/livepatch/livepatch-callbacks-mod.c
> +++ b/samples/livepatch/livepatch-callbacks-mod.c
> @@ -38,4 +38,5 @@ static void livepatch_callbacks_mod_exit(void)
>
> module_init(livepatch_callbacks_mod_init);
> module_exit(livepatch_callbacks_mod_exit);
> +MODULE_DESCRIPTION("Live patching demo for (un)patching callbacks, support
> module");
> MODULE_LICENSE("GPL");
The rest looks good. With the above change:
Reviewed-by: Petr Mladek
Thanks a lot for fixing this.
Arnd, should I push this via the livepatch tree or would you prefer to push
the entire patchset together? Both ways work for me.
Best Regards,
Petr
[PATCH v16 2/6] remoteproc: Add TEE support
Add a remoteproc TEE (Trusted Execution Environment) driver that will be probed by the TEE bus. If the associated Trusted application is supported on the secure part, this driver offers a client interface to load firmware by the secure part. This firmware could be authenticated by the secure trusted application. A specificity of the implementation is that the firmware has to be authenticated and optionally decrypted to access the resource table. Consequently, the boot sequence is: 1) rproc_parse_fw --> rproc_tee_parse_fw remoteproc TEE: - Requests the TEE application to authenticate and load the firmware in the remote processor memories. - Requests the TEE application for the address of the resource table. - Creates a copy of the resource table stored in rproc->cached_table. 2) rproc_load_segments --> rproc_tee_load_fw remoteproc TEE: - Requests the TEE application to load the firmware. Nothing is done at the TEE application as the firmware is already loaded. - In case of recovery, the TEE application has to reload the firmware. 3) rproc_tee_get_loaded_rsc_table remoteproc TEE requests the TEE application for the address of the resource table. 4) rproc_start --> rproc_tee_start - Requests the TEE application to start the remote processor. The shutdown sequence is: 5) rproc_stop --> rproc_tee_stop - Requests the TEE application to stop the remote processor. 6) rproc_tee_release_fw This function is used to request the TEE application to perform actions to return to the initial state on stop or on error during the boot sequence. Signed-off-by: Arnaud Pouliquen --- Updates vs previous version: - Come back to v13 managing a load in rproc_tee_parse_fw(). - Replace IS_REACHABLE with IS_ENABLED in remoteproc_tee.h, remoteproc_tee is not supported as module - update REMOTEPROC_TEE config description in Kconfig - manage unbind sequence of the consumer rproc drivers using devlink - remove TA_RPROC_FW_CMD_GET_COREDUMP as not use ( to be treated in a separate thread) - define a static `rproc_tee_ctx` variable instead of allocating it - add spinlock to protect `rproc_tee_ctx` accesses. - add exported function doumentation --- drivers/remoteproc/Kconfig | 10 + drivers/remoteproc/Makefile | 1 + drivers/remoteproc/remoteproc_tee.c | 619 include/linux/remoteproc_tee.h | 90 4 files changed, 720 insertions(+) create mode 100644 drivers/remoteproc/remoteproc_tee.c create mode 100644 include/linux/remoteproc_tee.h diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig index 83962a114dc9..e39265d249d9 100644 --- a/drivers/remoteproc/Kconfig +++ b/drivers/remoteproc/Kconfig @@ -23,6 +23,16 @@ config REMOTEPROC_CDEV It's safe to say N if you don't want to use this interface. +config REMOTEPROC_TEE + bool "Remoteproc support by a TEE application" + depends on OPTEE + help + Support a remote processor that is managed by an application running in a Trusted + Execution Environment (TEE). This application is responsible for loading the remote + processor firmware image and managing its lifecycle. + + It's safe to say N if the remote processor is not managed by a TEE. + config IMX_REMOTEPROC tristate "i.MX remoteproc support" depends on ARCH_MXC diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile index 5ff4e2fee4ab..f77e0abe8349 100644 --- a/drivers/remoteproc/Makefile +++ b/drivers/remoteproc/Makefile @@ -11,6 +11,7 @@ remoteproc-y += remoteproc_sysfs.o remoteproc-y += remoteproc_virtio.o remoteproc-y += remoteproc_elf_loader.o obj-$(CONFIG_REMOTEPROC_CDEV) += remoteproc_cdev.o +obj-$(CONFIG_REMOTEPROC_TEE) += remoteproc_tee.o obj-$(CONFIG_IMX_REMOTEPROC) += imx_rproc.o obj-$(CONFIG_IMX_DSP_REMOTEPROC) += imx_dsp_rproc.o obj-$(CONFIG_INGENIC_VPU_RPROC)+= ingenic_rproc.o diff --git a/drivers/remoteproc/remoteproc_tee.c b/drivers/remoteproc/remoteproc_tee.c new file mode 100644 index ..cf50de7eb952 --- /dev/null +++ b/drivers/remoteproc/remoteproc_tee.c @@ -0,0 +1,619 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (C) STMicroelectronics 2024 + * Author: Arnaud Pouliquen + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +#define MAX_TEE_PARAM_ARRAY_MEMBER 4 + +/* + * Authentication and load of the firmware image in the remote processor memories by the TEE. + * After this step the firmware is loaded in destination memories, which can then be locked to + * prevent access by Linux. + * + * [in] params[0].value.a:remote processor identifier + * [in] params[1].memref: buffer containing a temporary copy of the signed image to load. + */ +#define TA_RPROC_FW_CMD_
[PATCH v3 0/2] selftests: livepatch: test if ftrace can trace a livepatched function
This patchset add ftrace helpers functions and add a new test makes sure that ftrace can trace a function that was introduced by a livepatch. Signed-off-by: Filipe Xavier Suggested-by: Marcos Paulo de Souza Reviewed-by: Marcos Paulo de Souza Acked-by: Miroslav Benes --- Changes in v3: - functions.sh: fixed sed to remove warning from shellcheck and add grep -Fw params. - test-ftrace.sh: change constant to use common SYSFS_KLP_DIR. - Link to v2: https://lore.kernel.org/r/20250318-ftrace-sftest-livepatch-v2-0-60cb0aa95...@gmail.com Changes in v2: - functions.sh: change check traced function to accept a list of functions. - Link to v1: https://lore.kernel.org/r/20250306-ftrace-sftest-livepatch-v1-0-a6f1dfc30...@gmail.com --- Filipe Xavier (2): selftests: livepatch: add new ftrace helpers functions selftests: livepatch: test if ftrace can trace a livepatched function tools/testing/selftests/livepatch/functions.sh | 49 tools/testing/selftests/livepatch/test-ftrace.sh | 34 2 files changed, 83 insertions(+) --- base-commit: 848e076317446f9c663771ddec142d7c2eb4cb43 change-id: 20250306-ftrace-sftest-livepatch-60d9dc472235 Best regards, -- Filipe Xavier
[PATCH v16 4/6] dt-bindings: remoteproc: Add compatibility for TEE support
The "st,stm32mp1-m4-tee" compatible is utilized in a system configuration
where the Cortex-M4 firmware is loaded by the Trusted Execution Environment
(TEE).
For instance, this compatible is used in both the Linux and OP-TEE device
trees:
- In OP-TEE, a node is defined in the device tree with the
"st,stm32mp1-m4-tee" compatible to support signed remoteproc firmware.
Based on DT properties, the OP-TEE remoteproc framework is initiated to
expose a trusted application service to authenticate and load the remote
processor firmware provided by the Linux remoteproc framework, as well
as to start and stop the remote processor.
- In Linux, when the compatibility is set, the Cortex-M resets should not
be declared in the device tree. In such a configuration, the reset is
managed by the OP-TEE remoteproc driver and is no longer accessible from
the Linux kernel.
Associated with this new compatible, add the "st,proc-id" property to
identify the remote processor. This ID is used to define a unique ID,
common between Linux, U-Boot, and OP-TEE, to identify a coprocessor.
This ID will be used in requests to the OP-TEE remoteproc Trusted
Application to specify the remote processor.
Signed-off-by: Arnaud Pouliquen
Reviewed-by: Rob Herring (Arm)
---
.../bindings/remoteproc/st,stm32-rproc.yaml | 58 ---
1 file changed, 50 insertions(+), 8 deletions(-)
diff --git a/Documentation/devicetree/bindings/remoteproc/st,stm32-rproc.yaml
b/Documentation/devicetree/bindings/remoteproc/st,stm32-rproc.yaml
index 370af61d8f28..409123cd4667 100644
--- a/Documentation/devicetree/bindings/remoteproc/st,stm32-rproc.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/st,stm32-rproc.yaml
@@ -16,7 +16,12 @@ maintainers:
properties:
compatible:
-const: st,stm32mp1-m4
+enum:
+ - st,stm32mp1-m4
+ - st,stm32mp1-m4-tee
+description:
+ Use "st,stm32mp1-m4" for the Cortex-M4 coprocessor management by
non-secure context
+ Use "st,stm32mp1-m4-tee" for the Cortex-M4 coprocessor management by
secure context
reg:
description:
@@ -43,6 +48,10 @@ properties:
- description: The offset of the hold boot setting register
- description: The field mask of the hold boot
+ st,proc-id:
+description: remote processor identifier
+$ref: /schemas/types.yaml#/definitions/uint32
+
st,syscfg-tz:
deprecated: true
description:
@@ -142,21 +151,43 @@ properties:
required:
- compatible
- reg
- - resets
allOf:
- if:
properties:
-reset-names:
- not:
-contains:
- const: hold_boot
+compatible:
+ contains:
+const: st,stm32mp1-m4
then:
+ if:
+properties:
+ reset-names:
+not:
+ contains:
+const: hold_boot
+ then:
+required:
+ - st,syscfg-holdboot
+ else:
+properties:
+ st,syscfg-holdboot: false
+required:
+ - reset-names
required:
-- st,syscfg-holdboot
-else:
+- resets
+
+ - if:
+ properties:
+compatible:
+ contains:
+const: st,stm32mp1-m4-tee
+then:
properties:
st,syscfg-holdboot: false
+reset-names: false
+resets: false
+ required:
+- st,proc-id
additionalProperties: false
@@ -188,5 +219,16 @@ examples:
st,syscfg-rsc-tbl = <&tamp 0x144 0x>;
st,syscfg-m4-state = <&tamp 0x148 0x>;
};
+ - |
+#include
+m4@1000 {
+ compatible = "st,stm32mp1-m4-tee";
+ reg = <0x1000 0x4>,
+<0x3000 0x4>,
+<0x3800 0x1>;
+ st,proc-id = <0>;
+ st,syscfg-rsc-tbl = <&tamp 0x144 0x>;
+ st,syscfg-m4-state = <&tamp 0x148 0x>;
+};
...
--
2.25.1
Re: [PATCH net-next v2 0/4] virtio_net: Fixes and improvements
Hello: This series was applied to netdev/net-next.git (main) by Jakub Kicinski : On Fri, 21 Mar 2025 15:48:31 +0900 you wrote: > Jason Wang recently proposed an improvement to struct > virtio_net_rss_config: > https://lore.kernel.org/r/CACGkMEud0Ki8p=z299q7b4qedonpydzbvqhhxcnvk_vo-kd...@mail.gmail.com > > This patch series implements it and also fixes a few minor bugs I found > when writing patches. > > [...] Here is the summary with links: - [net-next,v2,1/4] virtio_net: Split struct virtio_net_rss_config https://git.kernel.org/netdev/net-next/c/976c2696b71d - [net-next,v2,2/4] virtio_net: Fix endian with virtio_net_ctrl_rss https://git.kernel.org/netdev/net-next/c/97841341e302 - [net-next,v2,3/4] virtio_net: Use new RSS config structs https://git.kernel.org/netdev/net-next/c/ed3100e90d0d - [net-next,v2,4/4] virtio_net: Allocate rss_hdr with devres https://git.kernel.org/netdev/net-next/c/4944be2f5ad8 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
Re: [RFC PATCH v3 7/8] perf: arm_pmuv3: Keep out of guest counter partition
James Clark writes:
On 13/02/2025 6:03 pm, Colton Lewis wrote:
If the PMU is partitioned, keep the driver out of the guest counter
partition and only use the host counter partition. Partitioning is
defined by the MDCR_EL2.HPMN register field and saved in
cpu_pmu->hpmn. The range 0..HPMN-1 is accessible by EL1 and EL0 while
HPMN..PMCR.N is reserved for EL2.
Define some macros that take HPMN as an argument and construct
mutually exclusive bitmaps for testing which partition a particular
counter is in. Note that despite their different position in the
bitmap, the cycle and instruction counters are always in the guest
partition.
Signed-off-by: Colton Lewis
---
arch/arm/include/asm/arm_pmuv3.h | 2 +
arch/arm64/include/asm/kvm_pmu.h | 5 +++
arch/arm64/kvm/pmu-part.c| 16 +++
drivers/perf/arm_pmuv3.c | 73 +++-
include/linux/perf/arm_pmuv3.h | 8
5 files changed, 94 insertions(+), 10 deletions(-)
diff --git a/arch/arm/include/asm/arm_pmuv3.h
b/arch/arm/include/asm/arm_pmuv3.h
index 2ec0e5e83fc9..dadd4ddf51af 100644
--- a/arch/arm/include/asm/arm_pmuv3.h
+++ b/arch/arm/include/asm/arm_pmuv3.h
@@ -227,6 +227,8 @@ static inline bool kvm_set_pmuserenr(u64 val)
}
static inline void kvm_vcpu_pmu_resync_el0(void) {}
+static inline void kvm_pmu_host_counters_enable(void) {}
+static inline void kvm_pmu_host_counters_disable(void) {}
/* PMU Version in DFR Register */
#define ARMV8_PMU_DFR_VER_NI0
diff --git a/arch/arm64/include/asm/kvm_pmu.h
b/arch/arm64/include/asm/kvm_pmu.h
index 174b7f376d95..8f25754fde47 100644
--- a/arch/arm64/include/asm/kvm_pmu.h
+++ b/arch/arm64/include/asm/kvm_pmu.h
@@ -25,6 +25,8 @@ void kvm_host_pmu_init(struct arm_pmu *pmu);
u8 kvm_pmu_get_reserved_counters(void);
u8 kvm_pmu_hpmn(u8 nr_counters);
void kvm_pmu_partition(struct arm_pmu *pmu);
+void kvm_pmu_host_counters_enable(void);
+void kvm_pmu_host_counters_disable(void);
#else
@@ -37,6 +39,9 @@ static inline bool kvm_set_pmuserenr(u64 val)
static inline void kvm_vcpu_pmu_resync_el0(void) {}
static inline void kvm_host_pmu_init(struct arm_pmu *pmu) {}
+static inline void kvm_pmu_host_counters_enable(void) {}
+static inline void kvm_pmu_host_counters_disable(void) {}
+
#endif
#endif
diff --git a/arch/arm64/kvm/pmu-part.c b/arch/arm64/kvm/pmu-part.c
index e74fecc67e37..51da65c678f9 100644
--- a/arch/arm64/kvm/pmu-part.c
+++ b/arch/arm64/kvm/pmu-part.c
@@ -45,3 +45,19 @@ void kvm_pmu_partition(struct arm_pmu *pmu)
pmu->partitioned = false;
}
}
+
+void kvm_pmu_host_counters_enable(void)
+{
+ u64 mdcr = read_sysreg(mdcr_el2);
+
+ mdcr |= MDCR_EL2_HPME;
+ write_sysreg(mdcr, mdcr_el2);
+}
+
+void kvm_pmu_host_counters_disable(void)
+{
+ u64 mdcr = read_sysreg(mdcr_el2);
+
+ mdcr &= ~MDCR_EL2_HPME;
+ write_sysreg(mdcr, mdcr_el2);
+}
diff --git a/drivers/perf/arm_pmuv3.c b/drivers/perf/arm_pmuv3.c
index 0e360feb3432..442dcff56d5b 100644
--- a/drivers/perf/arm_pmuv3.c
+++ b/drivers/perf/arm_pmuv3.c
@@ -730,15 +730,19 @@ static void armv8pmu_disable_event_irq(struct
perf_event *event)
armv8pmu_disable_intens(BIT(event->hw.idx));
}
-static u64 armv8pmu_getreset_flags(void)
+static u64 armv8pmu_getreset_flags(struct arm_pmu *cpu_pmu)
{
u64 value;
/* Read */
value = read_pmovsclr();
+ if (cpu_pmu->partitioned)
+ value &= ARMV8_PMU_HOST_CNT_PART(cpu_pmu->hpmn);
+ else
+ value &= ARMV8_PMU_OVERFLOWED_MASK;
+
/* Write to clear flags */
- value &= ARMV8_PMU_OVERFLOWED_MASK;
write_pmovsclr(value);
return value;
@@ -765,6 +769,18 @@ static void armv8pmu_disable_user_access(void)
update_pmuserenr(0);
}
+static bool armv8pmu_is_guest_part(struct arm_pmu *cpu_pmu, u8 idx)
+{
+ return cpu_pmu->partitioned &&
+ (BIT(idx) & ARMV8_PMU_GUEST_CNT_PART(cpu_pmu->hpmn));
+}
+
+static bool armv8pmu_is_host_part(struct arm_pmu *cpu_pmu, u8 idx)
+{
+ return !cpu_pmu->partitioned ||
+ (BIT(idx) & ARMV8_PMU_HOST_CNT_PART(cpu_pmu->hpmn));
+}
+
static void armv8pmu_enable_user_access(struct arm_pmu *cpu_pmu)
{
int i;
@@ -773,6 +789,8 @@ static void armv8pmu_enable_user_access(struct
arm_pmu *cpu_pmu)
if (is_pmuv3p9(cpu_pmu->pmuver)) {
u64 mask = 0;
for_each_set_bit(i, cpuc->used_mask, ARMPMU_MAX_HWEVENTS) {
+ if (armv8pmu_is_guest_part(cpu_pmu, i))
+ continue;
Hi Colton,
Is it possible to keep the guest bits out of used_mask and cntr_mask in
the first place? Then all these loops don't need to have the logic for
is_guest_part()/is_host_part().
It should be possible.
That leads me to wonder about updating the printout:
hw perfevents: enabled with armv8_pmuv3_0 PMU driver,
Re: [PATCH net-next v24 18/23] ovpn: implement peer add/get/dump/delete via netlink
On 25/03/2025 11:56, Sabrina Dubroca wrote:
2025-03-25, 00:15:48 +0100, Antonio Quartulli wrote:
On 24/03/2025 11:48, Sabrina Dubroca wrote:
Hello Antonio,
A few questions wrt the API:
2025-03-18, 02:40:53 +0100, Antonio Quartulli wrote:
+static bool ovpn_nl_attr_sockaddr_remote(struct nlattr **attrs,
+struct sockaddr_storage *ss)
+{
+ struct sockaddr_in6 *sin6;
+ struct sockaddr_in *sin;
+ struct in6_addr *in6;
+ __be16 port = 0;
+ __be32 *in;
+
+ ss->ss_family = AF_UNSPEC;
+
+ if (attrs[OVPN_A_PEER_REMOTE_PORT])
+ port = nla_get_be16(attrs[OVPN_A_PEER_REMOTE_PORT]);
What's the expected behavior if REMOTE_PORT isn't provided? We'll send
packets do port 0 (which I'm guessing will get dropped on the other
side) until we get a message from the peer and float sets the correct
port/address?
I have never seen a packet going out with port 0 :)
It will if you hack into ovpn-cli to skip OVPN_A_PEER_REMOTE_PORT.
I don't know how networks/admins react to such packets.
But being dropped is most likely what's going to happen.
I'd say this is not something that we expect the user to do:
if the remote address if specified, the user should specify a non-zero port
too.
We could add a check to ensure that a port is always specified if the remote
address is there too, just to avoid the user to shoot himself in the foot.
But we expect the user to pass an addr:port where the peer is listening to
(and that can't be a 0 port).
If we expect that (even if a well-behaved userspace would never do
it), I have a preference for enforcing that expectation. Since there's
already a policy rejecting OVPN_A_PEER_REMOTE_PORT == 0, this would be
more consistent IMO.
Ok, makes sense.
An alternative would be to select a default (non-zero) port if none is
provided.
I prefer to enforce having a port, rather tan going with a default that
may bite us down the road.
+static int ovpn_nl_peer_modify(struct ovpn_peer *peer, struct genl_info *info,
+ struct nlattr **attrs)
+{
[...]
+ /* when setting the keepalive, both parameters have to be configured */
+ if (attrs[OVPN_A_PEER_KEEPALIVE_INTERVAL] &&
+ attrs[OVPN_A_PEER_KEEPALIVE_TIMEOUT]) {
+ interv = nla_get_u32(attrs[OVPN_A_PEER_KEEPALIVE_INTERVAL]);
+ timeout = nla_get_u32(attrs[OVPN_A_PEER_KEEPALIVE_TIMEOUT]);
+ ovpn_peer_keepalive_set(peer, interv, timeout);
Should we interpret OVPN_A_PEER_KEEPALIVE_INTERVAL = 0 &&
OVPN_A_PEER_KEEPALIVE_TIMEOUT == 0 as "disable keepalive/timeout" on
an active peer? And maybe "one set to 0, the other set to some
non-zero value" as invalid? Setting either value to 0 doesn't seem
very useful (timeout = 0 will probably kill the peer immediately, and
I suspect interval = 0 would be quite spammy).
Considering "0" as "disable keepalive" is the current intention.
In ovpn_peer_keepalive_work_single() you can see that if either one if 0, we
just skip the peer:
1217 /* we expect both timers to be configured at the same time,
1218 * therefore bail out if either is not set
1219 */
1220 if (!peer->keepalive_timeout || !peer->keepalive_interval) {
1221 spin_unlock_bh(&peer->lock);
1222 return 0;
1223 }
does it make sense?
Ah, true. Sorry, I forgot about that. So after _NEW/_SET we'll run
the work once, and that peer will be ignored. And if there's no other
peer requiring keepalive, next_run will be 0 and we don't
reschedule. That's good, thanks.
Cool,
Regards,
--
Antonio Quartulli
OpenVPN Inc.
Re: [PATCH v2 2/2] KVM: selftests: access_tracking_perf_test: add option to skip the sanity check
On Mon, Mar 24, 2025 at 6:57 PM Maxim Levitsky wrote:
>
> Add an option to skip sanity check of number of still idle pages,
> and set it by default to skip, in case hypervisor or NUMA balancing
> is detected.
>
> Signed-off-by: Maxim Levitsky
Thanks Maxim! I'm still working on a respin of this test with MGLRU
integration, like [1]. Sorry it's taking me so long. I'll apply my
changes on top of yours.
[1]:
https://lore.kernel.org/kvm/20241105184333.2305744-12-jthough...@google.com/
> ---
> .../selftests/kvm/access_tracking_perf_test.c | 33 ---
> .../testing/selftests/kvm/include/test_util.h | 1 +
> tools/testing/selftests/kvm/lib/test_util.c | 7
> 3 files changed, 37 insertions(+), 4 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/access_tracking_perf_test.c
> b/tools/testing/selftests/kvm/access_tracking_perf_test.c
> index 3c7defd34f56..6d50c829f00c 100644
> --- a/tools/testing/selftests/kvm/access_tracking_perf_test.c
> +++ b/tools/testing/selftests/kvm/access_tracking_perf_test.c
> @@ -65,6 +65,8 @@ static int vcpu_last_completed_iteration[KVM_MAX_VCPUS];
> /* Whether to overlap the regions of memory vCPUs access. */
> static bool overlap_memory_access;
>
> +static int warn_on_too_many_idle_pages = -1;
> +
> struct test_params {
> /* The backing source for the region of memory. */
> enum vm_mem_backing_src_type backing_src;
> @@ -184,11 +186,10 @@ static void mark_vcpu_memory_idle(struct kvm_vm *vm,
> * are cached and the guest won't see the "idle" bit cleared.
> */
> if (still_idle >= pages / 10) {
> -#ifdef __x86_64__
> - TEST_ASSERT(this_cpu_has(X86_FEATURE_HYPERVISOR),
> + TEST_ASSERT(warn_on_too_many_idle_pages,
I think this assertion is flipped (or how warn_on_too_many_idle_pages
is being set is flipped, see below).
> "vCPU%d: Too many pages still idle (%lu out of
> %lu)",
> vcpu_idx, still_idle, pages);
> -#endif
> +
> printf("WARNING: vCPU%d: Too many pages still idle (%lu out
> of %lu), "
>"this will affect performance results.\n",
>vcpu_idx, still_idle, pages);
> @@ -342,6 +343,8 @@ static void help(char *name)
> printf(" -v: specify the number of vCPUs to run.\n");
> printf(" -o: Overlap guest memory accesses instead of partitioning\n"
>" them into a separate region of memory for each vCPU.\n");
> + printf(" -w: Skip or force enable the check that after dirtying the
> guest memory, most (90%%) of \n"
> + "it is reported as dirty again (0/1)");
> backing_src_help("-s");
> puts("");
> exit(0);
> @@ -359,7 +362,7 @@ int main(int argc, char *argv[])
>
> guest_modes_append_default();
>
> - while ((opt = getopt(argc, argv, "hm:b:v:os:")) != -1) {
> + while ((opt = getopt(argc, argv, "hm:b:v:os:w:")) != -1) {
> switch (opt) {
> case 'm':
> guest_modes_cmdline(optarg);
> @@ -376,6 +379,11 @@ int main(int argc, char *argv[])
> case 's':
> params.backing_src = parse_backing_src_type(optarg);
> break;
> + case 'w':
> + warn_on_too_many_idle_pages =
> + atoi_non_negative("1 - enable warning, 0 -
> disable",
> + optarg);
We still get a "warning" either way, right? Maybe this should be
called "fail_on_too_many_idle_pages" (in which case the above
assertion is indeed flipped). Or "warn_on_too_many_idle_pages" should
mean *only* warn, i.e., *don't* fail, in which case, below we need to
flip how we set it below.
> + break;
> case 'h':
> default:
> help(argv[0]);
> @@ -386,6 +394,23 @@ int main(int argc, char *argv[])
> page_idle_fd = open("/sys/kernel/mm/page_idle/bitmap", O_RDWR);
> __TEST_REQUIRE(page_idle_fd >= 0,
>"CONFIG_IDLE_PAGE_TRACKING is not enabled");
> + if (warn_on_too_many_idle_pages == -1) {
> +#ifdef __x86_64__
> + if (this_cpu_has(X86_FEATURE_HYPERVISOR)) {
> + printf("Skipping idle page count sanity check,
> because the test is run nested\n");
> + warn_on_too_many_idle_pages = 0;
> + } else
> +#endif
> + if (is_numa_balancing_enabled()) {
> + printf("Skipping idle page count sanity check,
> because NUMA balance is enabled\n");
> + warn_on_too_many_idle_pages = 0;
> + } else {
> + warn_on_too_many_idle_pages = 1;
> + }
> + } else if (!warn_on_too_many_idle_pages) {
> + printf("Skipping idle page count san
Re: [RFC PATCH v3 5/8] KVM: arm64: Introduce module param to partition the PMU
Hi James, Thanks for the review. James Clark writes: On 13/02/2025 6:03 pm, Colton Lewis wrote: For PMUv3, the register MDCR_EL2.HPMN partitiones the PMU counters into two ranges where counters 0..HPMN-1 are accessible by EL1 and, if allowed, EL0 while counters HPMN..N are only accessible by EL2. Introduce a module parameter in KVM to set this register. The name reserved_host_counters reflects the intent to reserve some counters for the host so the guest may eventually be allowed direct access to a subset of PMU functionality for increased performance. Track HPMN and whether the pmu is partitioned in struct arm_pmu because both KVM and the PMUv3 driver will need to know that to handle guests correctly. Due to the difficulty this feature would create for the driver running at EL1 on the host, partitioning is only allowed in VHE mode. Working on nVHE mode would require a hypercall for every register access because the counters reserved for the host by HPMN are now only accessible to EL2. The parameter is only configurable at boot time. Making the parameter configurable on a running system is dangerous due to the difficulty of knowing for sure no counters are in use anywhere so it is safe to reporgram HPMN. Hi Colton, For some high level feedback for the RFC, it probably makes sense to include the other half of the feature at the same time. I think there is a risk that it requires something slightly different than what's here and there ends up being some churn. I agree. That's what I'm working on now. I justed wanted an iteration or two in public so I'm not building on something that needs drastic change later. Other than that I think it looks ok apart from some minor code review nits. Thank you I was also thinking about how BRBE interacts with this. Alex has done some analysis that finds that it's difficult to use BRBE in guests with virtualized counters due to the fact that BRBE freezes on any counter overflow, rather than just guest ones. That leaves the guest with branch blackout windows in the delay between a host counter overflowing and the interrupt being taken and BRBE being restarted. But with HPMN, BRBE does allow freeze on overflow of only one partition or the other (or both, but I don't think we'd want that) e.g.: RNXCWF: If EL2 is implemented, a BRBE freeze event occurs when all of the following are true: * BRBCR_EL1.FZP is 1. * Generation of Branch records is not paused. * PMOVSCLR_EL0[(MDCR_EL2.HPMN-1):0] is nonzero. * The PE is in a BRBE Non-prohibited region. Unfortunately that means we could only let guests use BRBE with a partitioned PMU, which would massively reduce flexibility if hosts have to lose counters just so the guest can use BRBE. I don't know if this is a stupid idea, but instead of having a fixed number for the partition, wouldn't it be nice if we could trap and increment HPMN on the first guest use of a counter, then decrement it on guest exit depending on what's still in use? The host would always assign its counters from the top down, and guests go bottom up if they want PMU passthrough. Maybe it's too complicated or won't work for various reasons, but because of BRBE the counter partitioning changes go from an optimization to almost a necessity. This is a cool idea that would enable useful things. I can think of a few potential problems. 1. Partitioning will give guests direct access to some PMU counter registers. There is no reliable way for KVM to determine what is in use from that state. A counter that is disabled guest at exit might only be so temporarily, which could lead to a lot of thrashing allocating and deallocating counters. 2. HPMN affects reads of PMCR_EL0.N, which is the standard way to determine how many counters there are. If HPMN starts as a low number, guests have no way of knowing there are more counters available. Dynamically changing the counters available could be confusing for guests. 3. If guests were aware they could write beyond HPMN and get the counters allocated to them, nothing stops them from writing at counter N and taking as many counters as possible to starve the host. Signed-off-by: Colton Lewis --- arch/arm64/include/asm/kvm_pmu.h | 4 +++ arch/arm64/kvm/Makefile | 2 +- arch/arm64/kvm/debug.c | 9 -- arch/arm64/kvm/pmu-part.c| 47 arch/arm64/kvm/pmu.c | 2 ++ include/linux/perf/arm_pmu.h | 2 ++ 6 files changed, 62 insertions(+), 4 deletions(-) create mode 100644 arch/arm64/kvm/pmu-part.c diff --git a/arch/arm64/include/asm/kvm_pmu.h b/arch/arm64/include/asm/kvm_pmu.h index 613cddbdbdd8..174b7f376d95 100644 --- a/arch/arm64/include/asm/kvm_pmu.h +++ b/arch/arm64/include/asm/kvm_pmu.h @@ -22,6 +22,10 @@ bool kvm_set_pmuserenr(u64 val); void kvm_vcpu_pmu_resync_el0(void); void kvm_host_pmu_init(struct arm_pmu *pmu); +u8 kvm_pmu_get_reserved_counters(vo
Re: [PATCH v3 1/7] dt-bindings: input: syna,rmi4: document syna,pdt-fallback-desc
On 3/25/25 08:36, Krzysztof Kozlowski wrote: On 24/03/2025 19:00, David Heidelberg wrote: On 10/03/2025 10:45, Krzysztof Kozlowski wrote: On Sat, Mar 08, 2025 at 03:08:37PM +0100, David Heidelberg wrote: From: Caleb Connolly This new property allows devices to specify some register values which are missing on units with third party replacement displays. These displays use unofficial touch ICs which only implement a subset of the RMI4 specification. These are different ICs, so they have their own compatibles. Why this cannot be deduced from the compatible? Yes, but these identify as the originals. It does not matter how they identify. You have the compatible for them. If you cannot add compatible for them, how can you add dedicated property for them? Hi Krzysztof, There are an unknown number of knock-off RMI4 chips which are sold in cheap replacement display panels from multiple vendors. We suspect there's more than one implementation. A new compatible string wouldn't help us, since we use the same DTB on fully original hardware as on hardware with replacement parts. The proposed new property describes configuration registers which are present on original RMI4 chips but missing on the third party ones, the contents of the registers is static. The third party chips were designed to work with a specific downstream driver which doesn't implement the self-describing features of RMI4 and just hardcoded the functionality they expect. Kind regards, Best regards, Krzysztof -- Caleb (they/them)
Re: [PATCH v3 0/2] selftests: livepatch: test if ftrace can trace a livepatched function
On Mon, Mar 24, 2025 at 07:50:17PM -0300, Filipe Xavier wrote: > This patchset add ftrace helpers functions and > add a new test makes sure that ftrace can trace > a function that was introduced by a livepatch. > > Signed-off-by: Filipe Xavier > Suggested-by: Marcos Paulo de Souza > Reviewed-by: Marcos Paulo de Souza > Acked-by: Miroslav Benes > --- > Changes in v3: > - functions.sh: fixed sed to remove warning from shellcheck and add grep -Fw > params. Oh, now I see what shellcheck was complaining about. I missed that there was no '\' line continuation char at the end of the sed line. I thought it was complaining about starting the subsequent lines with the '|' and '>' redirection chars. Good eye on catching that :) > - test-ftrace.sh: change constant to use common SYSFS_KLP_DIR. > - Link to v2: > https://lore.kernel.org/r/20250318-ftrace-sftest-livepatch-v2-0-60cb0aa95...@gmail.com > For both patches: Acked-by: Joe Lawrence Thanks, -- Joe
[no subject]
>From 92c5ac2ffa37f6e4fe0cfa49a20857432bc67718 Mon Sep 17 00:00:00 2001
From: Malaya Kumar Rout
Date: Tue, 25 Mar 2025 18:42:33 +0530
Subject: [PATCH v2] selftests/x86/lam: fix resource leak in do_uring() and
allocate_dsa_pasid()
Exception branch returns without closing
the file descriptors 'file_fd' and 'fd'
Signed-off-by: Malaya Kumar Rout
---
tools/testing/selftests/x86/lam.c | 13 +
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/x86/lam.c
b/tools/testing/selftests/x86/lam.c
index 18d736640ece..eaba0a921322 100644
--- a/tools/testing/selftests/x86/lam.c
+++ b/tools/testing/selftests/x86/lam.c
@@ -682,7 +682,7 @@ int do_uring(unsigned long lam)
return 1;
if (fstat(file_fd, &st) < 0)
- return 1;
+ goto cleanup;
off_t file_sz = st.st_size;
@@ -690,7 +690,7 @@ int do_uring(unsigned long lam)
fi = malloc(sizeof(*fi) + sizeof(struct iovec) * blocks);
if (!fi)
- return 1;
+ goto cleanup;
fi->file_sz = file_sz;
fi->file_fd = file_fd;
@@ -698,7 +698,7 @@ int do_uring(unsigned long lam)
ring = malloc(sizeof(*ring));
if (!ring) {
free(fi);
- return 1;
+ goto cleanup;
}
memset(ring, 0, sizeof(struct io_ring));
@@ -730,6 +730,9 @@ int do_uring(unsigned long lam)
free(fi);
+cleanup:
+ close(file_fd);
+
return ret;
}
@@ -1189,8 +1192,10 @@ void *allocate_dsa_pasid(void)
wq = mmap(NULL, 0x1000, PROT_WRITE,
MAP_SHARED | MAP_POPULATE, fd, 0);
- if (wq == MAP_FAILED)
+ if (wq == MAP_FAILED) {
+ close(fd);
perror("mmap");
+ }
return wq;
}
--
2.43.0
Re: [PATCH v4] hrtimers: Force migrate away hrtimers queued after CPUHP_AP_HRTIMERS_DYING
On Tue, 2025-01-21 at 09:08 -0800, Paul E. McKenney wrote:
> On Sat, Jan 18, 2025 at 12:24:33AM +0100, Frederic Weisbecker wrote:
> > hrtimers are migrated away from the dying CPU to any online target
> > at
> > the CPUHP_AP_HRTIMERS_DYING stage in order not to delay bandwidth
> > timers
> > handling tasks involved in the CPU hotplug forward progress.
> >
> > However wake ups can still be performed by the outgoing CPU after
> > CPUHP_AP_HRTIMERS_DYING. Those can result again in bandwidth timers
> > being armed. Depending on several considerations (crystal ball
> > power management based election, earliest timer already enqueued,
> > timer
> > migration enabled or not), the target may eventually be the current
> > CPU even if offline. If that happens, the timer is eventually
> > ignored.
> >
> > The most notable example is RCU which had to deal with each and
> > every of
> > those wake-ups by deferring them to an online CPU, along with
> > related
> > workarounds:
> >
> > _ e787644caf76 (rcu: Defer RCU kthreads wakeup when CPU is dying)
> > _ 9139f93209d1 (rcu/nocb: Fix RT throttling hrtimer armed from
> > offline CPU)
> > _ f7345ccc62a4 (rcu/nocb: Fix rcuog wake-up from offline softirq)
> >
> > The problem isn't confined to RCU though as the stop machine
> > kthread
> > (which runs CPUHP_AP_HRTIMERS_DYING) reports its completion at the
> > end
> > of its work through cpu_stop_signal_done() and performs a wake up
> > that
> > eventually arms the deadline server timer:
> >
> > WARNING: CPU: 94 PID: 588 at kernel/time/hrtimer.c:1086
> > hrtimer_start_range_ns+0x289/0x2d0
> > CPU: 94 UID: 0 PID: 588 Comm: migration/94 Not tainted
> > Stopper: multi_cpu_stop+0x0/0x120 <-
> > stop_machine_cpuslocked+0x66/0xc0
> > RIP: 0010:hrtimer_start_range_ns+0x289/0x2d0
> > Call Trace:
> >
> > ? hrtimer_start_range_ns
> > start_dl_timer
> > enqueue_dl_entity
> > dl_server_start
> > enqueue_task_fair
> > enqueue_task
> > ttwu_do_activate
> > try_to_wake_up
> > complete
> > cpu_stopper_thread
> > smpboot_thread_fn
> > kthread
> > ret_from_fork
> > ret_from_fork_asm
> >
> >
> > Instead of providing yet another bandaid to work around the
> > situation,
> > fix it from hrtimers infrastructure instead: always migrate away a
> > timer to an online target whenever it is enqueued from an offline
> > CPU.
> >
> > This will also allow to revert all the above RCU disgraceful hacks.
> >
> > Reported-by: Vlad Poenaru
> > Reported-by: Usama Arif
> > Fixes: 5c0930ccaad5 ("hrtimers: Push pending hrtimers away from
> > outgoing CPU earlier")
> > Closes: 20241213203739.1519801-1-usamaarif...@gmail.com
> > Signed-off-by: Frederic Weisbecker
> > Signed-off-by: Paul E. McKenney
>
> This passes over-holiday testing rcutorture, so, perhaps redundantly:
>
> Tested-by: Paul E. McKenney
Hi,
I encountered the same issue even after applying this patch.
Below are the details of the warning and call trace.
migration/3: [ cut here ]
migration/3: WARNING: CPU: 3 PID: 42 at kernel/time/hrtimer.c:1125
enqueue_hrtimer+0x7c/0xec
migration/3: CPU: 3 UID: 0 PID: 42 Comm: migration/3 Tainted: G
OE 6.12.18-android16-0-g59cb5a849beb-4k #1
0b440e43fa7b24aaa3b7e6e5d2b938948e0cacdb
migration/3: Stopper: multi_cpu_stop+0x0/0x184 <-
stop_machine_cpuslocked+0xc0/0x15c
migration/3: Call trace:
migration/3: enqueue_hrtimer+0x7c/0xec
migration/3: hrtimer_start_range_ns+0x54c/0x7b4
migration/3: start_dl_timer+0x110/0x188
migration/3: enqueue_dl_entity+0x478/0x80c
migration/3: dl_server_start+0xf8/0x194
migration/3: enqueue_task_fair+0x6c4/0x924
migration/3: enqueue_task+0x70/0x3a4
migration/3: do_activate_task+0xcc/0x178
migration/3: ttwu_do_activate+0xac/0x2e0
migration/3: try_to_wake_up+0x73c/0xaf4
migration/3: wake_up_process+0x18/0x28
migration/3: kick_pool+0xc4/0x170
migration/3: __queue_work+0x44c/0x698
migration/3: delayed_work_timer_fn+0x20/0x30
migration/3: call_timer_fn+0x4c/0x1d0
migration/3: __run_timer_base+0x278/0x350
migration/3: run_timer_softirq+0x78/0x9c
migration/3: handle_softirqs+0x154/0x42c
migration/3: __do_softirq+0x14/0x20
migration/3: do_softirq+0x10/0x20
migration/3: call_on_irq_stack+0x3c/0x74
migration/3: do_softirq_own_stack+0x1c/0x2c
migration/3: __irq_exit_rcu+0x5c/0xd4
migration/3: irq_exit_rcu+0x10/0x1c
migration/3: el1_interrupt+0x38/0x58
migration/3: el1h_64_irq_handler+0x18/0x24
migration/3: el1h_64_irq+0x68/0x6c
migration/3: multi_cpu_stop+0x15c/0x184
migration/3: cpu_stopper_thread+0xf8/0x1b0
migration/3: smpboot_thread_fn+0x204/0x304
migration/3: kthread+0x110/0x1a4
migration/3: ret_from_fork+0x10/0x20
migration/3: ---[ end trace ]---
kworker/1:1: psci: CPU0 killed (polled 0 ms)
Re: [PATCH 2/3] of: Simplify of_dma_set_restricted_buffer() to use of_for_each_phandle()
Hi,
On Tue, Mar 18, 2025 at 7:29 AM Rob Herring (Arm) wrote:
>
> Simplify of_dma_set_restricted_buffer() by using of_property_present()
> and of_for_each_phandle() iterator.
>
> Signed-off-by: Rob Herring (Arm)
> ---
> drivers/of/device.c | 34 +-
> 1 file changed, 13 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/of/device.c b/drivers/of/device.c
> index edf3be197265..bb4a47d58249 100644
> --- a/drivers/of/device.c
> +++ b/drivers/of/device.c
> @@ -35,44 +35,36 @@ EXPORT_SYMBOL(of_match_device);
> static void
> of_dma_set_restricted_buffer(struct device *dev, struct device_node *np)
> {
> - struct device_node *node, *of_node = dev->of_node;
> - int count, i;
> + struct device_node *of_node = dev->of_node;
> + struct of_phandle_iterator it;
> + int rc, i = 0;
>
> if (!IS_ENABLED(CONFIG_DMA_RESTRICTED_POOL))
> return;
>
> - count = of_property_count_elems_of_size(of_node, "memory-region",
> - sizeof(u32));
> /*
> * If dev->of_node doesn't exist or doesn't contain memory-region, try
> * the OF node having DMA configuration.
> */
> - if (count <= 0) {
> + if (!of_property_present(of_node, "memory-region"))
> of_node = np;
> - count = of_property_count_elems_of_size(
> - of_node, "memory-region", sizeof(u32));
> - }
>
> - for (i = 0; i < count; i++) {
> - node = of_parse_phandle(of_node, "memory-region", i);
> + of_for_each_phandle(&it, rc, of_node, "memory-region", NULL, 0) {
> /*
> * There might be multiple memory regions, but only one
> * restricted-dma-pool region is allowed.
> */
> - if (of_device_is_compatible(node, "restricted-dma-pool") &&
> - of_device_is_available(node)) {
> - of_node_put(node);
> - break;
> + if (of_device_is_compatible(it.node, "restricted-dma-pool") &&
> + of_device_is_available(it.node)) {
> + if (!of_reserved_mem_device_init_by_idx(dev, of_node,
> i)) {
> + of_node_put(it.node);
> + return;
> + }
> }
> - of_node_put(node);
> + i++;
> }
>
> - /*
> -* Attempt to initialize a restricted-dma-pool region if one was
> found.
> -* Note that count can hold a negative error code.
> -*/
> - if (i < count && of_reserved_mem_device_init_by_idx(dev, of_node, i))
> - dev_warn(dev, "failed to initialise \"restricted-dma-pool\"
> memory node\n");
> + dev_warn(dev, "failed to initialise \"restricted-dma-pool\" memory
> node\n");
This changes the behavior. Before this patch, it was:
if a restricted dma pool was found, but initializing it failed, print
a warning.
Whereas now it has become:
print a warning unless a restricted dma pool was found and successfully
initialized.
This change causes the kernel to print out the warning for devices that
don't even do DMA:
simple-pm-bus soc: failed to initialise "restricted-dma-pool" memory node
simple-pm-bus 10006000.syscon: failed to initialise
"restricted-dma-pool" memory node
mtk-tphy soc:t-phy@11c8: failed to initialise
"restricted-dma-pool" memory node
mtk-tphy soc:t-phy@11ca: failed to initialise
"restricted-dma-pool" memory node
mediatek-mipi-tx 11cc.dsi-phy: failed to initialise
"restricted-dma-pool" memory node
mediatek-mipi-tx 11cc.dsi-phy: can't get nvmem_cell_get, ignore it
clk-mt8186-apmixed 1000c000.syscon: failed to initialise
"restricted-dma-pool" memory node
clk-mt8186-topck 1000.syscon: failed to initialise
"restricted-dma-pool" memory node
clk-mt8186-infra-ao 10001000.syscon: failed to initialise
"restricted-dma-pool" memory node
clk-mt8186-cam 1a00.clock-controller: failed to initialise
"restricted-dma-pool" memory node
clk-mt8186-cam 1a04f000.clock-controller: failed to initialise
"restricted-dma-pool" memory node
clk-mt8186-cam 1a06f000.clock-controller: failed to initialise
"restricted-dma-pool" memory node
clk-mt8186-img 1502.clock-controller: failed to initialise
"restricted-dma-pool" memory node
clk-mt8186-img 1582.clock-controller: failed to initialise
"restricted-dma-pool" memory node
clk-mt8186-imp_iic_wrap 11017000.clock-controller: failed to
initialise "restricted-dma-pool" memory node
clk-mt8186-ipe 1c00.clock-controller: failed to initialise
"restricted-dma-pool" memory node
clk-mt8186-mcu c53a000.syscon: failed to initialise
"restricted-dma-pool" memory node
clk-mt8186-mdp 1b00.clock-controller: failed to initialise
"restricted-dma-pool" memory node
clk-mt8186-mfg 1300.clock-controller: failed
Re: [PATCHv5 net-next 1/2] wireguard: selftests: convert iptables to nft
On Sun, Mar 23, 2025 at 10:10:33PM +0100, Phil Sutter wrote:
> On Sat, Mar 22, 2025 at 09:30:15AM +, Hangbin Liu wrote:
> > Convert iptabels to nft as it is the replacement for iptables, which is used
>
>
> Typo, but I would write "Convert the selftest to nft ..." instead since
> that is what you're converting, iptables is just replaced. :)
Hi Jason, I saw net-next is closed. Should I wait for net-next re-open to post
the new version and fix the typo? I'm not sure about the wg branch policy.
Thanks
Hangbin
>
> > by default in most releases.
> >
> > Signed-off-by: Hangbin Liu
> > ---
> > tools/testing/selftests/wireguard/netns.sh | 29 ++
> > 1 file changed, 19 insertions(+), 10 deletions(-)
> >
> > diff --git a/tools/testing/selftests/wireguard/netns.sh
> > b/tools/testing/selftests/wireguard/netns.sh
> > index 55500f901fbc..8b840fef90af 100755
> > --- a/tools/testing/selftests/wireguard/netns.sh
> > +++ b/tools/testing/selftests/wireguard/netns.sh
> > @@ -75,6 +75,11 @@ pp ip netns add $netns1
> > pp ip netns add $netns2
> > ip0 link set up dev lo
> >
> > +# init nft tables
> > +n0 nft add table ip wgtest
> > +n1 nft add table ip wgtest
> > +n2 nft add table ip wgtest
> > +
> > ip0 link add dev wg0 type wireguard
> > ip0 link set wg0 netns $netns1
> > ip0 link add dev wg0 type wireguard
> > @@ -196,13 +201,14 @@ ip1 link set wg0 mtu 1300
> > ip2 link set wg0 mtu 1300
> > n1 wg set wg0 peer "$pub2" endpoint 127.0.0.1:2
> > n2 wg set wg0 peer "$pub1" endpoint 127.0.0.1:1
> > -n0 iptables -A INPUT -m length --length 1360 -j DROP
> > +n0 nft add chain ip wgtest INPUT { type filter hook input priority filter
> > \; policy accept \; }
>
> You may skip the 'policy accept \;' part in all 'add chain' commands as
> this is the default for all chains. Unless you prefer to explicitly
> state the chain policy, of course.
>
> Cheers, Phil
Re: [PATCH 00/19] virtio_ring in order support
On Mon, Mar 24, 2025 at 3:44 PM Lei Yang wrote: > > QE tested this series of patches with virtio-net regression tests, > everything works fine. > Hi Lei, Is it possible to test this series also with virtio-net-pci,...,in_order=on? Thanks! > Tested-by: Lei Yang > > On Mon, Mar 24, 2025 at 1:45 PM Jason Wang wrote: > > > > Hello all: > > > > This sereis tries to implement the VIRTIO_F_IN_ORDER to > > virtio_ring. This is done by introducing virtqueue ops so we can > > implement separate helpers for different virtqueue layout/features > > then the in-order were implmeented on top. > > > > Tests shows 5% imporvemnt in RX PPS with KVM guest + testpmd on the > > host. > > > > Please review. > > > > Thanks > > > > Jason Wang (19): > > virtio_ring: rename virtqueue_reinit_xxx to virtqueue_reset_xxx() > > virtio_ring: switch to use vring_virtqueue in virtqueue_poll variants > > virtio_ring: unify logic of virtqueue_poll() and more_used() > > virtio_ring: switch to use vring_virtqueue for virtqueue resize > > variants > > virtio_ring: switch to use vring_virtqueue for virtqueue_kick_prepare > > variants > > virtio_ring: switch to use vring_virtqueue for virtqueue_add variants > > virtio: switch to use vring_virtqueue for virtqueue_add variants > > virtio_ring: switch to use vring_virtqueue for enable_cb_prepare > > variants > > virtio_ring: use vring_virtqueue for enable_cb_delayed variants > > virtio_ring: switch to use vring_virtqueue for disable_cb variants > > virtio_ring: switch to use vring_virtqueue for detach_unused_buf > > variants > > virtio_ring: use u16 for last_used_idx in virtqueue_poll_split() > > virtio_ring: introduce virtqueue ops > > virtio_ring: determine descriptor flags at one time > > virtio_ring: factor out core logic of buffer detaching > > virtio_ring: factor out core logic for updating last_used_idx > > virtio_ring: move next_avail_idx to vring_virtqueue > > virtio_ring: factor out split indirect detaching logic > > virtio_ring: add in order support > > > > drivers/virtio/virtio_ring.c | 856 ++- > > 1 file changed, 653 insertions(+), 203 deletions(-) > > > > -- > > 2.42.0 > > > > >
[PATCH V2] remoteproc: core: Clear table_sz when rproc_shutdown
From: Peng Fan
There is case as below could trigger kernel dump:
Use U-Boot to start remote processor(rproc) with resource table
published to a fixed address by rproc. After Kernel boots up,
stop the rproc, load a new firmware which doesn't have resource table
,and start rproc.
When starting rproc with a firmware not have resource table,
`memcpy(loaded_table, rproc->cached_table, rproc->table_sz)` will
trigger dump, because rproc->cache_table is set to NULL during the last
stop operation, but rproc->table_sz is still valid.
This issue is found on i.MX8MP and i.MX9.
Dump as below:
Unable to handle kernel NULL pointer dereference at virtual address
Mem abort info:
ESR = 0x9604
EC = 0x25: DABT (current EL), IL = 32 bits
SET = 0, FnV = 0
EA = 0, S1PTW = 0
FSC = 0x04: level 0 translation fault
Data abort info:
ISV = 0, ISS = 0x0004, ISS2 = 0x
CM = 0, WnR = 0, TnD = 0, TagAccess = 0
GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
user pgtable: 4k pages, 48-bit VAs, pgdp=00010af63000
[] pgd=, p4d=
Internal error: Oops: 9604 [#1] PREEMPT SMP
Modules linked in:
CPU: 2 UID: 0 PID: 1060 Comm: sh Not tainted 6.14.0-rc7-next-20250317-dirty #38
Hardware name: NXP i.MX8MPlus EVK board (DT)
pstate: a005 (NzCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : __pi_memcpy_generic+0x110/0x22c
lr : rproc_start+0x88/0x1e0
Call trace:
__pi_memcpy_generic+0x110/0x22c (P)
rproc_boot+0x198/0x57c
state_store+0x40/0x104
dev_attr_store+0x18/0x2c
sysfs_kf_write+0x7c/0x94
kernfs_fop_write_iter+0x120/0x1cc
vfs_write+0x240/0x378
ksys_write+0x70/0x108
__arm64_sys_write+0x1c/0x28
invoke_syscall+0x48/0x10c
el0_svc_common.constprop.0+0xc0/0xe0
do_el0_svc+0x1c/0x28
el0_svc+0x30/0xcc
el0t_64_sync_handler+0x10c/0x138
el0t_64_sync+0x198/0x19c
Clear rproc->table_sz to address the issue.
While at here, also clear rproc->table_sz when rproc_fw_boot and
rproc_detach.
Fixes: 9dc9507f1880 ("remoteproc: Properly deal with the resource table when
detaching")
Signed-off-by: Peng Fan
---
V2:
Clear table_sz when rproc_fw_boot and rproc_detach per Arnaud
drivers/remoteproc/remoteproc_core.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/remoteproc/remoteproc_core.c
b/drivers/remoteproc/remoteproc_core.c
index c2cf0d277729..1efa53d4e0c3 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -1442,6 +1442,7 @@ static int rproc_fw_boot(struct rproc *rproc, const
struct firmware *fw)
kfree(rproc->cached_table);
rproc->cached_table = NULL;
rproc->table_ptr = NULL;
+ rproc->table_sz = 0;
unprepare_rproc:
/* release HW resources if needed */
rproc_unprepare_device(rproc);
@@ -2025,6 +2026,7 @@ int rproc_shutdown(struct rproc *rproc)
kfree(rproc->cached_table);
rproc->cached_table = NULL;
rproc->table_ptr = NULL;
+ rproc->table_sz = 0;
out:
mutex_unlock(&rproc->lock);
return ret;
@@ -2091,6 +2093,7 @@ int rproc_detach(struct rproc *rproc)
kfree(rproc->cached_table);
rproc->cached_table = NULL;
rproc->table_ptr = NULL;
+ rproc->table_sz = 0;
out:
mutex_unlock(&rproc->lock);
return ret;
--
2.37.1
Re: [PATCH] kunit: cs_dsp: Depend on FW_CS_DSP rather then enabling it
On Sat, Mar 22, 2025 at 07:02:26PM +, Richard Fitzgerald wrote: > On 22/3/25 10:11, Richard Fitzgerald wrote: > > I got lucky on all the UM, X86 and ARM builds I tested. > It looks like that bugfix has got lost. > Mark sent an email on 20 Feb to say it has been merged into his > sound tree. But that patch isn't in torvalds/master. It was queued for -next rather than as a fix. signature.asc Description: PGP signature
Re: [PATCH v3 0/2] selftests: livepatch: test if ftrace can trace a livepatched function
On Mon 2025-03-24 19:50:17, Filipe Xavier wrote: > This patchset add ftrace helpers functions and > add a new test makes sure that ftrace can trace > a function that was introduced by a livepatch. > > Signed-off-by: Filipe Xavier > Acked-by: Miroslav Benes For both patches: Reviewed-by: Petr Mladek Tested-by: Petr Mladek Best Regards, Petr
Re: [PATCH v3 0/2] selftests: livepatch: test if ftrace can trace a livepatched function
On Mon 2025-03-24 19:50:17, Filipe Xavier wrote: > This patchset add ftrace helpers functions and > add a new test makes sure that ftrace can trace > a function that was introduced by a livepatch. > > Signed-off-by: Filipe Xavier > Acked-by: Miroslav Benes JFYI, the patchset has been committed into livepatching.git, branch for-6.15/ftrace-test. I had a dilemma whether to push it for 6.15 or postpone it. But it is a selftest and quite trivial. And it has been reviewed by several people. And it seems to work well so I think that we could push it for 6.15. Best Regards, Petr
Re: [PATCH v3 0/2] selftests: livepatch: test if ftrace can trace a livepatched function
On 3/25/25 11:01 AM, Petr Mladek wrote: On Mon 2025-03-24 19:50:17, Filipe Xavier wrote: This patchset add ftrace helpers functions and add a new test makes sure that ftrace can trace a function that was introduced by a livepatch. Signed-off-by: Filipe Xavier Acked-by: Miroslav Benes JFYI, the patchset has been committed into livepatching.git, branch for-6.15/ftrace-test. I had a dilemma whether to push it for 6.15 or postpone it. But it is a selftest and quite trivial. And it has been reviewed by several people. And it seems to work well so I think that we could push it for 6.15. It sounds good to me, thank you all very much for the support. Cheers, Filipe Best Regards, Petr
Re: [PATCH] firmware: cs_dsp: Ensure cs_dsp_load[_coeff]() returns 0 on success
On Sun, 23 Mar 2025 17:05:29 +, Richard Fitzgerald wrote: > Set ret = 0 on successful completion of the processing loop in > cs_dsp_load() and cs_dsp_load_coeff() to ensure that the function > returns 0 on success. > > All normal firmware files will have at least one data block, and > processing this block will set ret == 0, from the result of either > regmap_raw_write() or cs_dsp_parse_coeff(). > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next Thanks! [1/1] firmware: cs_dsp: Ensure cs_dsp_load[_coeff]() returns 0 on success commit: 2593f7e0dc93a898a84220b3fb180d86f1ca8c60 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
Re: [PATCH net-next v2 0/7] selftests/net: Mixed select()+polling mode for TCP-AO tests
On Wed, 19 Mar 2025 03:13:33 + Dmitry Safonov via B4 Relay wrote: > Should fix flaky tcp-ao/connect-deny-ipv6 test. > Begging pardon for the delay since the report and for sending it this > late in the release cycle. Better late than never, thanks a lot! :)
Re: [PATCH 03/10] samples/livepatch: add module descriptions
On 3/25/2025 3:01 AM, Petr Mladek wrote: > Arnd, should I push this via the livepatch tree or would you prefer to push > the entire patchset together? Both ways work for me. My past experience was to let individual maintainers take the ones that apply to their trees, and then Andrew can pick up the stragglers. /jeff
Re: [PATCH v3 0/2] arm64: livepatch: Enable livepatch without sframe
Hi Petr, > On Mar 25, 2025, at 8:53 AM, Petr Mladek wrote: [...] >> >> Given the increasing need of livepatching, and relatively long time before >> sframe is fully ready (for both gcc and clang), we would like to enable >> livepatch without sframe. >> >> Thanks! >> >> [1] >> https://lore.kernel.org/live-patching/20250127213310.2496133-1-wn...@google.com/ >> >> [2] >> https://lore.kernel.org/live-patching/20250129232936.1795412-1-s...@kernel.org/ >> >> [3] >> https://urldefense.com/v3/__https://sourceware.org/bugzilla/show_bug.cgi?id=32589__;!!Bt8RZUm9aw!8CB9ByorcyTxDW3rQ6_GEeMTJN9rHWCydNdIW1FRb_2-LQ6RTrSerZq9E-s8kXEB12JJ_v07xkyc2w$ >> >> [4] >> https://lore.kernel.org/linux-arm-kernel/20241017092538.1859841-1-mark.rutl...@arm.com/ > > Hi, I am sorry but I haven't found time to look at this in time before > the merge window. Is it acceptable to postpone this change to 6.16, > please? Yes, we can postpone this to 6.16. I will resend the patchset after the merge window. Thanks, Song
Re: [PATCH 3/3] selftests: vDSO: chacha: Provide default definition of HWCAP_S390_VXRS
On Tue, Mar 25, 2025 at 07:48:48AM +0100, Thomas Weißschuh wrote:
> On Mon, Mar 24, 2025 at 04:55:13PM +0100, Heiko Carstens wrote:
> > On Mon, Mar 24, 2025 at 03:03:17PM +0100, Thomas Weißschuh wrote:
> > > s390 does not provide a hwcap.h UAPI header.
> > >
> > > Add an inline definition for the constant HWCAP_S390_VXRS until a proper
> > > UAPI header is introduced.
> > >
> > > Fixes: 210860e7f733 ("selftests: vDSO: check cpu caps before running
> > > chacha test")
> > > Signed-off-by: Thomas Weißschuh
> > > ---
> > > tools/testing/selftests/vDSO/vdso_test_chacha.c | 3 +++
> > > 1 file changed, 3 insertions(+)
...
> > > #elif defined(__s390x__)
> > > +#ifndef HWCAP_S390_VXRS
> > > +#define HWCAP_S390_VXRS (1 << 11)
> > > +#endif
> > > static bool cpu_has_capabilities(void)
> > > {
> > > return getauxval(AT_HWCAP) & HWCAP_S390_VXRS;
> >
> > How did this cause a problem?
> >
> > Did you use something different than glibc(-devel) on your test
> > system? Just wondering since glibc-devel provides the define since
> > ages and is also required for getauxval().
>
> I used nolibc (from the kernel tree at tools/include/nolibc/) to make cross
> platform usage of the tests easier. See also [0].
> I got confused by the existence of hwcap.h in the kernel UAPI headers for
> various architectures and didn't check the libc headers.
> So this isn't really a bug right now, and the hwcap changes will only really
> be
> necessary once my other work goes upstream.
Thanks for explaining!
Acked-by: Heiko Carstens
Re: [PATCH 3/3] selftests: vDSO: chacha: Provide default definition of HWCAP_S390_VXRS
On Mon, Mar 24, 2025 at 04:55:13PM +0100, Heiko Carstens wrote:
> On Mon, Mar 24, 2025 at 03:03:17PM +0100, Thomas Weißschuh wrote:
> > s390 does not provide a hwcap.h UAPI header.
> >
> > Add an inline definition for the constant HWCAP_S390_VXRS until a proper
> > UAPI header is introduced.
> >
> > Fixes: 210860e7f733 ("selftests: vDSO: check cpu caps before running chacha
> > test")
> > Signed-off-by: Thomas Weißschuh
> > ---
> > tools/testing/selftests/vDSO/vdso_test_chacha.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/tools/testing/selftests/vDSO/vdso_test_chacha.c
> > b/tools/testing/selftests/vDSO/vdso_test_chacha.c
> > index
> > fd5c5108b42f04ec459d39b74f33edc2ceafbba1..0ce5189718ce35b0a4d69b71559db8379b598b93
> > 100644
> > --- a/tools/testing/selftests/vDSO/vdso_test_chacha.c
> > +++ b/tools/testing/selftests/vDSO/vdso_test_chacha.c
> > @@ -19,6 +19,9 @@ static bool cpu_has_capabilities(void)
> > return getauxval(AT_HWCAP) & HWCAP_ASIMD;
> > }
> > #elif defined(__s390x__)
> > +#ifndef HWCAP_S390_VXRS
> > +#define HWCAP_S390_VXRS(1 << 11)
> > +#endif
> > static bool cpu_has_capabilities(void)
> > {
> > return getauxval(AT_HWCAP) & HWCAP_S390_VXRS;
>
> How did this cause a problem?
>
> Did you use something different than glibc(-devel) on your test
> system? Just wondering since glibc-devel provides the define since
> ages and is also required for getauxval().
I used nolibc (from the kernel tree at tools/include/nolibc/) to make cross
platform usage of the tests easier. See also [0].
I got confused by the existence of hwcap.h in the kernel UAPI headers for
various architectures and didn't check the libc headers.
So this isn't really a bug right now, and the hwcap changes will only really be
necessary once my other work goes upstream.
[0]
https://lore.kernel.org/lkml/20250217-kunit-kselftests-v1-0-42b4524c3...@linutronix.de/
Re: [PATCH 3/3] selftests: vDSO: chacha: Provide default definition of HWCAP_S390_VXRS
On Tue, Mar 25, 2025 at 08:18:40AM +0100, Heiko Carstens wrote:
> On Tue, Mar 25, 2025 at 07:48:48AM +0100, Thomas Weißschuh wrote:
> > On Mon, Mar 24, 2025 at 04:55:13PM +0100, Heiko Carstens wrote:
> > > On Mon, Mar 24, 2025 at 03:03:17PM +0100, Thomas Weißschuh wrote:
> > > > s390 does not provide a hwcap.h UAPI header.
> > > >
> > > > Add an inline definition for the constant HWCAP_S390_VXRS until a proper
> > > > UAPI header is introduced.
> > > >
> > > > Fixes: 210860e7f733 ("selftests: vDSO: check cpu caps before running
> > > > chacha test")
> > > > Signed-off-by: Thomas Weißschuh
> > > > ---
> > > > tools/testing/selftests/vDSO/vdso_test_chacha.c | 3 +++
> > > > 1 file changed, 3 insertions(+)
>
> ...
>
> > > > #elif defined(__s390x__)
> > > > +#ifndef HWCAP_S390_VXRS
> > > > +#define HWCAP_S390_VXRS(1 << 11)
> > > > +#endif
> > > > static bool cpu_has_capabilities(void)
> > > > {
> > > > return getauxval(AT_HWCAP) & HWCAP_S390_VXRS;
> > >
> > > How did this cause a problem?
> > >
> > > Did you use something different than glibc(-devel) on your test
> > > system? Just wondering since glibc-devel provides the define since
> > > ages and is also required for getauxval().
> >
> > I used nolibc (from the kernel tree at tools/include/nolibc/) to make cross
> > platform usage of the tests easier. See also [0].
> > I got confused by the existence of hwcap.h in the kernel UAPI headers for
> > various architectures and didn't check the libc headers.
> > So this isn't really a bug right now, and the hwcap changes will only
> > really be
> > necessary once my other work goes upstream.
>
> Thanks for explaining!
>
> Acked-by: Heiko Carstens
Thanks!
I'll wait for some more feedback and then resend the series with some better
explanations, and without the incorrect Fixes: tags.
If there is pushback for applying any of the patches now, I'll carry them
downstream and will resubmit them as part of the later series which integrates
more of the vDSO selftests with nolibc.
Re: [PATCH] selftests/x86/lam: fix memory leak and resource leak in lam.c
* Peter Zijlstra wrote: > On Mon, Mar 24, 2025 at 06:17:50PM +0530, Malaya Kumar Rout wrote: > > Static Analyis for bench_htab_mem.c with cppcheck:error > > tools/testing/selftests/x86/lam.c:585:3: > > error: Resource leak: file_fd [resourceLeak] > > tools/testing/selftests/x86/lam.c:593:3: > > error: Resource leak: file_fd [resourceLeak] > > tools/testing/selftests/x86/lam.c:600:3: > > error: Memory leak: fi [memleak] > > tools/testing/selftests/x86/lam.c:1066:2: > > error: Resource leak: fd [resourceLeak] > > > > fix the issue by closing the file descriptors and > > releasing the allocated memory. > > > > But but but, doesn't the program just exit on any of those 'errors' > anyway? > > That is, iirc this is a single shot program. While that's true, still proper cleanup of resources is a good practice - and in more complicated tools it's useful to fix even these semi-false-positives, to make sure other warnings don't get missed. Having said that, the error/cleanup control flow here doesn't look overly clean here to begin with, so I'd suggest fixing that (with goto labels or such) - which would fix the file_fd 'leak' as a happy side effect. Thanks, Ingo
Re: [RFC -next 00/10] Add ZC notifications to splice and sendfile
On Thu, Mar 20, 2025 at 11:23:57AM -0700, Joe Damato wrote: > In my other message to Jens I proposed: > - SPLICE_F_ZC for splice to generate zc completion notifications > to the error queue > - Modifying sendfile so that if SO_ZEROCOPY (which already exists) > is set on a network socket, zc completion notifications are > generated. > > In both cases no new system call is needed and both splice and > sendfile become safer to use. > > At some point in the future a mechanism built on top of iouring > introduced as new system calls (sendmsg2, sendfile2, splice2, etc) > can be built. I strongly disagree with this. This is spreading the broken SO_ZEROCOPY to futher places outside the pure networking realm. Don't do that. It also doesn't help that more than 7 years after adding it, SO_ZEROCOPY is still completely undocumented. > > Because sendmsg should never have done that it certainly should not > > spread beyond purely socket specific syscalls. > > I don't know the entire historical context, but I presume sendmsg > did that because there was no other mechanism at the time. At least aio had been around for about 15 years at the point, but networking folks tend to be pretty insular and reinvent things. > It seems like Jens suggested that plumbing this through for splice > was a possibility, but sounds like you disagree. Yes, very strongly. > As mentioned above and in other messages, it seems like it is > possible to improve the networking parts of splice (and therefore > sendfile) to make them safer to use without introducing a new system > call. > > Are you saying that you are against doing that, even if the code is > network specific (but lives in fs/)? Yes. Please take the work and integrate it with the kiocb-based system we use for all other in-kernel I/O that needs completion notifications and which makes it trivial to integate with io_uring instead of spreading an imcompatible and inferior event system.
