Re: [PATCH 1/5] dt-bindings: remoteproc: qcom,sm8550-pas: document the SDX75 PAS
On 6/6/2024 8:19 PM, Krzysztof Kozlowski wrote: On 06/06/2024 16:38, Naina Mehta wrote: Document the MPSS Peripheral Authentication Service on SDX75 platform. Signed-off-by: Naina Mehta --- .../devicetree/bindings/remoteproc/qcom,sm8550-pas.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,sm8550-pas.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,sm8550-pas.yaml index 73fda7565cd1..02e15b1f78ab 100644 --- a/Documentation/devicetree/bindings/remoteproc/qcom,sm8550-pas.yaml +++ b/Documentation/devicetree/bindings/remoteproc/qcom,sm8550-pas.yaml @@ -16,6 +16,7 @@ description: properties: compatible: enum: + - qcom,sdx75-mpss-pas - qcom,sm8550-adsp-pas - qcom,sm8550-cdsp-pas - qcom,sm8550-mpss-pas Missing updates to allOf constraints. Are you sure this is the binding for SDX75? SDX75 diverged from SDX55 due to introduction of separate DTB firmware binary and addition of DSM and Qlink logging memory regions. Considering these additions, SM8550 PAS bindings seem closest. Thanks for catching updates required for allOf constraints. I will add sdx75 compatible in-line with qcom,sm8650-mpss-pas. Please let me know if you recommend creating new bindings for SDX75. Regards, Naina Best regards, Krzysztof
Re: [PATCH v2 1/2] dt-bindings: soc: qcom,smsm: Allow specifying mboxes instead of qcom,ipc
On 06/06/2024 21:18, Luca Weiss wrote: > The qcom,ipc-N properties are essentially providing a reference to a > mailbox, so allow using the mboxes property to do the same in a more > structured way. > > Since multiple SMSM hosts are supported, we need to be able to provide > the correct mailbox for each host. The old qcom,ipc-N properties map to > the mboxes property by index, starting at 0 since that's a valid SMSM > host also. > > Mark the older qcom,ipc-N as deprecated and update the example with > mboxes. > > Signed-off-by: Luca Weiss > --- Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v2 1/1] dt-bindings: remoteproc: imx_rproc: add minItems for power-domain
On 06/06/2024 17:00, Frank Li wrote: > "fsl,imx8qxp-cm4" and "fsl,imx8qm-cm4" need minimum 2 power domains. Keep > the same restriction for other compatible string. > > Signed-off-by: Frank Li > --- > > Notes: > Change from v1 to v2 > - set minitem to 2 at top > - Add imx8qm compatible string also > - use not logic to handle difference compatible string restriction > - update commit message. > > pass dt_binding_check. > > make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- -j8 dt_binding_check > DT_SCHEMA_FILES=fsl,imx-rproc.yaml > SCHEMA Documentation/devicetree/bindings/processed-schema.json > CHKDT Documentation/devicetree/bindings > LINTDocumentation/devicetree/bindings > DTEX > Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.example.dts > DTC_CHK > Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.example.dtb > > .../bindings/remoteproc/fsl,imx-rproc.yaml | 14 ++ > 1 file changed, 14 insertions(+) > > diff --git a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml > b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml > index df36e29d974ca..da108a39df435 100644 > --- a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml > +++ b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml > @@ -59,6 +59,7 @@ properties: > maxItems: 32 > >power-domains: > +minItems: 2 > maxItems: 8 > >fsl,auto-boot: > @@ -99,6 +100,19 @@ allOf: >properties: > fsl,iomuxc-gpr: false > > + - if: > + properties: > +compatible: > + not: > +contains: > + enum: > +- fsl,imx8qxp-cm4 > +- fsl,imx8qm-cm4 > +then: > + properties: > +power-domains: > + minItems: 8 What happened with the "else:"? How many power domains is needed for other devices? Best regards, Krzysztof
Re: [PATCH 0/2] livepatch: Add compiler optimization disclaimer/docs
Hi, On Fri, 31 May 2024, Joe Lawrence wrote: > On 5/31/24 07:23, Miroslav Benes wrote: > > Hi, > > > > On Tue, 21 Jul 2020, Joe Lawrence wrote: > > > >> In light of [PATCH] Revert "kbuild: use -flive-patching when > >> CONFIG_LIVEPATCH is enabled" [1], we should add some loud disclaimers > >> and explanation of the impact compiler optimizations have on > >> livepatching. > >> > >> The first commit provides detailed explanations and examples. The list > >> was taken mostly from Miroslav's LPC talk a few years back. This is a > >> bit rough, so corrections and additional suggestions welcome. Expanding > >> upon the source-based patching approach would be helpful, too. > >> > >> The second commit adds a small README.rst file in the livepatch samples > >> directory pointing the reader to the doc introduced in the first commit. > >> > >> I didn't touch the livepatch kselftests yet as I'm still unsure about > >> how to best account for IPA here. We could add the same README.rst > >> disclaimer here, too, but perhaps we have a chance to do something more. > >> Possibilities range from checking for renamed functions as part of their > >> build, or the selftest scripts, or even adding something to the kernel > >> API. I think we'll have a better idea after reviewing the compiler > >> considerations doc. > > > > thanks to Marcos for resurrecting this. > > > > Joe, do you have an updated version by any chance? Some things have > > changed since July 2020 so it calls for a new review. If there was an > > improved version, it would be easier. If not, no problem at all. > > > > Yea, it's been a little while :) I don't have any newer version than > this one. I can rebase, apply all of the v1 suggestions, and see where > it stands. LMK if you can think of any specifics that could be added. I will walk through the patches first to see if there is something which can/should be changed given the development since then. > For example, CONFIG_KERNEL_IBT will be driving some changes soon, > whether it be klp-convert for source-based patches or vmlinux.o binary > comparison for kpatch-build. True. > I can push a v2 with a few changes, but IIRC, last time we reviewed > this, it kinda begged the question of how someone is creating the > livepatch in the first place. As long as we're fine holding that > thought for a while longer, this doc may still be useful by itself. If I remember correctly, the conclusion was that this doc was beneficial on its own. Miroslav
Re: [PATCH 2/2] ring-buffer: Fix a race between readers and resize checks
On 5/28/24 01:43, Steven Rostedt wrote:
> On Mon, 27 May 2024 11:36:55 +0200
> Petr Pavlu wrote:
>
static void rb_check_pages(struct ring_buffer_per_cpu *cpu_buffer)
{
@@ -2200,8 +2205,13 @@ int ring_buffer_resize(struct trace_buffer *buffer,
unsigned long size,
*/
synchronize_rcu();
for_each_buffer_cpu(buffer, cpu) {
+ unsigned long flags;
+
cpu_buffer = buffer->buffers[cpu];
+ raw_spin_lock_irqsave(&cpu_buffer->reader_lock, flags);
rb_check_pages(cpu_buffer);
+ raw_spin_unlock_irqrestore(&cpu_buffer->reader_lock,
+ flags);
>>>
>>> Putting my RT hat on, I really don't like the above fix. The
>>> rb_check_pages() iterates all subbuffers which makes the time interrupts
>>> are disabled non-deterministic.
>>
>> I see, this applies also to the same rb_check_pages() validation invoked
>> from ring_buffer_read_finish().
>>
>>>
>>> Instead, I would rather have something where we disable readers while we do
>>> the check, and re-enable them.
>>>
>>> raw_spin_lock_irqsave(&cpu_buffer->reader_lock, flags);
>>> cpu_buffer->read_disabled++;
>>> raw_spin_unlock_irqrestore(&cpu_buffer->reader_lock,
>>> flags);
>>>
>>> // Also, don't put flags on a new line. We are allow to go 100 characters
>>> now.
>>
>> Noted.
>>
>>>
>>>
>>> rb_check_pages(cpu_buffer);
>>> raw_spin_lock_irqsave(&cpu_buffer->reader_lock, flags);
>>> cpu_buffer->read_disabled--;
>>> raw_spin_unlock_irqrestore(&cpu_buffer->reader_lock,
>>> flags);
>>>
>>> Or something like that. Yes, that also requires creating a new
>>> "read_disabled" field in the ring_buffer_per_cpu code.
>>
>> I think this would work but I'm personally not immediately sold on this
>> approach. If I understand the idea correctly, readers should then check
>> whether cpu_buffer->read_disabled is set and bail out with some error if
>> that is the case. The rb_check_pages() function is only a self-check
>> code and as such, I feel it doesn't justify disrupting readers with
>> a new error condition and adding more complex locking.
>
> Honestly, this code was never made for more than one reader per
> cpu_buffer. I'm perfectly fine if all check_pages() causes other
> readers to the same per_cpu buffer to get -EBUSY.
>
> Do you really see this being a problem? What use case is there for
> hitting the check_pages() and reading the same cpu buffer at the same
> time?
My main issue is with added complexity to check the new read_disabled
flag. The rb_check_pages() part is simple and you showed what to do. The
readers part is what I struggle with.
I think the read_disabled flag needs to be either checked once in
rb_get_reader_page() where the actual problem with making the list
temporarily inconsistent exists. Or alternatively, it can be checked by
direct or indirect users of rb_get_reader_page() just after they take
the reader_lock.
Looking at the final rb_get_reader_page() function, it currently always
returns a valid reader page unless the buffer doesn't contain any
additional entry or a serious problem is detected by RB_WARN_ON()
checks. This is simple to handle for callers, either they get a reader
page and can continue, or they stop.
Returning -EBUSY means that callers have a new case that they need to
decide what to do about. They need to propagate the error up the chain
or attempt to handle it. This involves ring-buffer functions
rb_advance_reader(), rb_buffer_peek(), ring_buffer_peek(),
ring_buffer_consume(), ring_buffer_read_page()
ring_buffer_map_get_reader() and their callers from other source files.
It is possible to handle this new case in these functions but I'm not
sure if adding this logic is justified. I feel I must have misunderstood
something how it should work?
I was further thinking about alternatives that would possibly provide
a less thorough check but have their complexity limited only to
rb_check_pages(). The already mentioned idea is to have the function to
look only at surrounding nodes where some change in the list occurred.
Another option could be to try traversing the whole list in smaller
parts and give up the reader_lock in between them. This would need some
care to make sure that the operation completes, e.g. the code would need
to bail out if it detects a change on cpu_buffer->pages_read.
Thanks,
Petr
[PATCH -next 1/2] function_graph: Add kernel-doc comments for ftrace_graph_ret_addr() function
Added kernel-doc comments for the ftrace_graph_ret_addr() function to
improve code documentation and readability.
Reported-by: Abaci Robot
Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=9299
Signed-off-by: Yang Li
---
kernel/trace/fgraph.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
index a13551a023aa..4ad33e4cb8da 100644
--- a/kernel/trace/fgraph.c
+++ b/kernel/trace/fgraph.c
@@ -872,6 +872,12 @@ ftrace_graph_get_ret_stack(struct task_struct *task, int
idx)
/**
* ftrace_graph_ret_addr - convert a potentially modified stack return address
*to its original value
+ * @task: pointer to the task_struct of the task being examined
+ * @idx: pointer to a state variable, should be initialized to zero
+ * before the first call
+ * @ret: the current return address found on the stack
+ * @retp: pointer to the return address on the stack, ignored if
+ * HAVE_FUNCTION_GRAPH_RET_ADDR_PTR is not defined
*
* This function can be called by stack unwinding code to convert a found stack
* return address ('ret') to its original value, in case the function graph
--
2.20.1.7.g153144c
[PATCH -next 2/2] ftrace: Add kernel-doc comments for unregister_ftrace_direct() function
Added kernel-doc comments for the unregister_ftrace_direct() function to improve code documentation and readability. Reported-by: Abaci Robot Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=9300 Signed-off-by: Yang Li --- kernel/trace/ftrace.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c index 4aeb1183ea9f..3b0dbd55cc05 100644 --- a/kernel/trace/ftrace.c +++ b/kernel/trace/ftrace.c @@ -5988,6 +5988,8 @@ EXPORT_SYMBOL_GPL(register_ftrace_direct); * unregister_ftrace_direct - Remove calls to custom trampoline * previously registered by register_ftrace_direct for @ops object. * @ops: The address of the struct ftrace_ops object + * @addr: The address of the direct call to remove + * @free_filters: Boolean indicating whether to free the filters * * This is used to remove a direct calls to @addr from the nop locations * of the functions registered in @ops (with by ftrace_set_filter_ip -- 2.20.1.7.g153144c
Re: [PATCH] livepatch: introduce klp_func called interface
Hi,
On Tue, 4 Jun 2024, Song Liu wrote:
> On Tue, May 21, 2024 at 1:04 AM Petr Mladek wrote:
> [...]
> > >
> > > Yes, but the information you get is limited compared to what is available
> > > now. You would obtain the information that a patched function was called
> > > but ftrace could also give you the context and more.
> >
> > Another motivation to use ftrace for testing is that it does not
> > affect the performance in production.
> >
> > We should keep klp_ftrace_handler() as fast as possible so that we
> > could livepatch also performance sensitive functions.
>
> At LPC last year, we discussed about adding a counter to each
> klp_func, like:
>
> struct klp_func {
> ...
> u64 __percpu counter;
> ...
> };
>
> With some static_key (+ sysctl), this should give us a way to estimate
> the overhead of livepatch. If we have the counter, this patch is not
> needed any more. Does this (adding the counter) sound like
> something we still want to pursue?
It would be better than this patch but given what was mentioned in the
thread I wonder if it is possible to use ftrace even for this. See
/sys/kernel/tracing/trace_stat/function*. It already gathers the number of
hits.
Would it be sufficient for you? I guess it depends on what the intention
is. If there is no time limit, klp_func.counter might be better to provide
some kind of overall statistics (but I am not sure if it has any value)
and to avoid having ftrace registered on a live patched function for
infinite period of time. If the intention is to gather data for some
limited period, trace_stat sounds like much better approach to me.
Regards
Miroslav
Re: [PATCH 1/3] rust: add static_call support
Peter Zijlstra wrote:
> On Thu, Jun 06, 2024 at 09:09:00PM +0200, Miguel Ojeda wrote:
> > On Thu, Jun 6, 2024 at 7:19 PM Peter Zijlstra wrote:
> > >
> > > This is absolutely unreadable gibberish -- how am I supposed to keep
> > > this in sync with the rest of the static_call infrastructure?
> >
> > Yeah, they are macros, which look different from "normal" Rust code.
>
> Macros like CPP ?
Yes, this patch series uses declarative macros, which are the closest
that Rust has to the C preprocessor. They are powerful, but just like
CPP, they can become pretty complicated and hard to read if you are
doing something non-trivial.
The macro_rules! block is how you define a new declarative macro.
The ($name:ident($($args:expr),* $(,)?)) part defines the arguments to
the declarative macro. This syntax means:
1. The input starts with any identifier, which we call $name.
2. Then there must be a ( token.
3. Then the $(),* part defines a repetition group. The contents inside
the parenthesises are what is being repeated. The comma means that
repetitions are separated by commas. The asterisk means that the
contents may be repeated any number of times, including zero times.
(Alternatives are + which means repeated at least once, and ? which
means that the contents may be repeated zero or one time.)
4. The contents of the repetition group will be an expression, which we
call $args.
5. After the last expression, we have $(,)? which means that you can
have zero or one commas after the last expression. Rust usually
allows trailing commas in lists, which is why I included it.
6. And finally, you must close the input with a ) token.
So for example, you might invoke the macro like this:
static_call!(tp_func_my_tracepoint(__data, &mut my_task_struct));
Here, $name will be tp_func_my_tracepoint, and the repetition group is
repeated twice, with $args first corresponding to `__data` and then to
`&mut my_task_struct` when the macro is expanded. The $(,)? group is
repeated zero times.
Inside the macro, you will see things such as:
$crate::macros::paste! { $crate::bindings:: [<__SCK__ $name >]; }
The Rust syntax for invoking a macro has an exclamation mark after the
name, so you know that $crate::macros::paste is a macro. The `paste`
macro just emits its input unchanged, except that any identifiers
between [< and >] are concatenated into a single identifier. So if $name
is my_static_key, then the above invocation of paste! emits:
$crate::bindings::__SCK__my_static_key;
The $crate::bindings module is where the output of bindgen goes, so this
should correspond to the C symbol called __SCK__my_static_key.
> > Is there something we could do to help here? I think Alice and others
> > would be happy to explain how it works and/or help maintain it in the
> > future if you prefer.
>
> Write a new language that looks more like C -- pretty please ? :-)
>
> Mostly I would just really like you to just use arm/jump_label.h,
> they're inline functions and should be doable with IR, no weirdo CPP
> involved in this case.
I assume that you're referring to static_key_false here? I don't think
that function can be exposed using IR because it passes the function
argument called key as an "i" argument to an inline assembly block. Any
attempt to compile static_key_false without knowing the value of key at
compile time will surely fail to compile with the
invalid operand for inline asm constraint 'i'
error.
Alice
[PATCH v6 3/5] 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
---
update vs previous version
- merge [PATCH v5 4/7] remoteproc: core introduce rproc_set_rsc_table_on_start
function
as new "st,proc-id" is associated to "st,stm32mp1-m4-tee" compatible
- update commit message
- remove Reviewed-by: Rob Herring as patch is updated
---
.../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
[PATCH v6 5/5] remoteproc: stm32: Add support of an OP-TEE TA to load the firmware
The new TEE remoteproc device 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
---
Update from previous version
- replace find_loaded_rsc_table by find_loaded_rsc_table ops.
---
drivers/remoteproc/stm32_rproc.c | 63 ++--
1 file changed, 60 insertions(+), 3 deletions(-)
diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
index 8cd838df4e92..c1262e1ccc96 100644
--- a/drivers/remoteproc/stm32_rproc.c
+++ b/drivers/remoteproc/stm32_rproc.c
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
#include
#include "remoteproc_internal.h"
@@ -257,6 +258,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 = tee_rproc_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;
@@ -693,8 +707,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 = tee_rproc_start,
+ .stop = stm32_rproc_tee_stop,
+ .kick = stm32_rproc_kick,
+ .load = tee_rproc_load_fw,
+ .parse_fw = tee_rproc_parse_fw,
+ .find_loaded_rsc_table = tee_rproc_find_loaded_rsc_table,
+
+};
+
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,17 +879,42 @@ static int stm32_rproc_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct stm32_rproc *ddata;
struct device_node *np = dev->of_node;
+ struct tee_rproc *trproc = NULL;
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;
+
+ trproc = tee_rproc_register(dev, rproc, proc_id);
+ if (IS_ERR(trproc)) {
+ dev_err_probe(dev, PTR_ERR(trproc),
+ "signed firmware not supported by TEE\n");
+ return PTR_ERR(trproc);
+ }
+ } else {
+ rproc = devm_rproc_alloc(dev, np->name, &st_rproc_ops, NULL,
sizeof(*ddata));
+ if (!rproc)
+ return -ENOMEM;
+ }
ddata = rproc->priv;
@@ -915,6 +966,9 @@ static int stm32_rproc_probe(struct platform_device *pdev)
dev_pm_clear_wake_irq(dev);
device_init_wakeup(dev, false);
}
+ if (trproc)
+ tee_rproc_unregister(trproc);
+
return ret;
}
@@ -935,6 +989,9 @@ static void stm32_rproc_remove(struct platform_device *pdev)
dev_pm_clear_wake_irq(dev);
device_init_wakeup(dev, false);
}
+ if (rproc->tee_interface)
+ tee_rproc_unregister(rproc->tee_interface);
+
}
static int stm32_rproc_suspend(struct device *dev)
--
2.25.1
[PATCH v6 0/5] Introduction of a remoteproc tee to load signed firmware
Main updates from the previous version [1][2]:
--
1) Rework resource table management
- Rework tee_rproc_parse_fw to temporary map the resource table address
to create a cached_table (similar to what is done in
rproc_elf_load_rsc_table()).
- Rename tee_rproc_get_loaded_rsc_table to tee_rproc_find_loaded_rsc_table
- Introduce rproc_pa_to_va() allowing to translate the resource table
physical address to virtual address based on remoteproc carveouts.
2) Merge the 2 "st,stm32-rproc.yaml" bindings patch in one
As the st,rproc-id" is linked to the introduction of the
"st,stm32mp1-m4-tee" compatible, merge following patches to address
Krzysztof concern.
- [PATCH v5 2/7] dt-bindings: remoteproc: Add compatibility for TEE support
- [PATCH v5 3/7] dt-bindings: remoteproc: Add processor identifier property
More details on updates are listed in commits messages.
[1] https://lore.kernel.org/lkml/Zlil4YSjHxb0FRgf@p14s/T/
[2]
https://lore.kernel.org/lkml/20240521122458.3517054-1-arnaud.pouliq...@foss.st.com/
base-commit: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0
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 (5):
remoteproc: core: Introduce rproc_pa_to_va helper
remoteproc: Add TEE support
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 | 74 ++-
drivers/remoteproc/stm32_rproc.c | 147 --
drivers/remoteproc/tee_remoteproc.c | 451 ++
include/linux/remoteproc.h| 7 +
include/linux/tee_remoteproc.h| 99
8 files changed, 801 insertions(+), 46 deletions(-)
create mode 100644 drivers/remoteproc/tee_remoteproc.c
create mode 100644 include/linux/tee_remoteproc.h
base-commit: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0
--
2.25.1
[PATCH v6 1/5] 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 not necessary 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 | 74 +++-
include/linux/remoteproc.h | 3 ++
2 files changed, 75 insertions(+), 2 deletions(-)
diff --git a/drivers/remoteproc/remoteproc_core.c
b/drivers/remoteproc/remoteproc_core.c
index f276956f2c5c..3fdec0336fd6 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -230,6 +230,77 @@ 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
+ *
+ * Some remote processors will ask us to allocate them physically contiguous
+ * memory regions (which we call "carveouts"), and map them to specific
+ * device addresses (which are hardcoded in the firmware). They may also have
+ * dedicated memory regions internal to the processors, and use them either
+ * exclusively or alongside carveouts.
+ *
+ * They may then ask us to copy objects into specific addresses (e.g.
+ * code/data sections) or expose us certain symbols in other device address
+ * (e.g. their trace buffer).
+ *
+ * This function is a helper function with which we can go over the allocated
+ * carveouts and translate specific physical addresses to kernel virtual
addresses
+ * so we can access the referenced memory. This function also allows to perform
+ * translations on the internal remoteproc memory regions through a platform
+ * implementation specific pa_to_va ops, if present.
+ *
+ * Note: phys_to_virt(iommu_iova_to_phys(rproc->domain, da)) will work too,
+ * but only on kernel direct mapped RAM memory. Instead, we're just using
+ * here the output of the DMA API for the carveouts, which should be more
+ * correct.
+ *
+ * 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;
+
+ if (rproc->ops->da_to_va) {
+ ptr = rproc->ops->pa_to_va(rproc, pa, len);
+ if (ptr)
+ goto out;
+ }
+
+ 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;
+ }
+
+out:
+ 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
@@ -724,8 +795,7 @@ static int rproc_alloc_carveout(struct rproc *rproc,
* firmware was compiled with.
*
* In this case, we must use the IOMMU API directly and map
-* the memory to the device address as expected by the remote
-* processor.
+* the memory to the device address as etable
*
* Obviously such remote processor devices should not be configured
* to use the iommu-based DMA API: we expect 'dma' to contain the
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index b4795698d8c2..28aa62a3b505 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -367,6 +367,7 @@ enum rsc_handling_status {
* @detach:detach from a device, leaving it powered up
* @kick: kick a virtqueue (virtqueue id given as a parameter)
* @da_to_va: optional platform hook to perform address translations
+ * @pa_to_va: optional platform hook to perform address translations
* @parse_fw: parse firmware to extract information (e.g. resource table)
* @handle_rsc:optional platform hook to handle vendor resources.
Should return
* RSC_HANDLED if resource was handled, RSC_IGNORED if not handled
@@ -391,6 +392,7 @@ struct rproc_ops {
int (*detach)(struct rproc *rproc);
void (*kick)(struct rproc *rproc, int vqid);
void * (*da_to_va)(struct rproc *rproc, u6
[PATCH v6 4/5] 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 | 84 +++-
1 file changed, 51 insertions(+), 33 deletions(-)
diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
index 88623df7d0c3..8cd838df4e92 100644
--- a/drivers/remoteproc/stm32_rproc.c
+++ b/drivers/remoteproc/stm32_rproc.c
@@ -209,6 +209,54 @@ 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, dummy_data, 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) {
+ /* A dummy data is sent to allow to block on transmit.
*/
+ err = mbox_send_message(ddata->mb[idx].chan,
+ &dummy_data);
+ 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 +567,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 +581,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 v6 2/5] 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 secure part this driver offers a client
interface to load a firmware in the secure part.
This firmware could be authenticated by the secure trusted application.
Signed-off-by: Arnaud Pouliquen
---
update from previous version
- make tee_rproc_get_loaded_rsc_table() local and replace this API by
tee_rproc_find_loaded_rsc_table()
- map and unmap the resource table in tee_rproc_parse_fw to make a cached copy
- use the new rproc_pa_to_va() API to map the resource table memory declared in
carevout
- remove tee_rproc_release_loaded_rsc_table as no more used.
---
drivers/remoteproc/Kconfig | 10 +
drivers/remoteproc/Makefile | 1 +
drivers/remoteproc/tee_remoteproc.c | 451
include/linux/remoteproc.h | 4 +
include/linux/tee_remoteproc.h | 99 ++
5 files changed, 565 insertions(+)
create mode 100644 drivers/remoteproc/tee_remoteproc.c
create mode 100644 include/linux/tee_remoteproc.h
diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index 48845dc8fa85..6c1c07202276 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -365,6 +365,16 @@ config XLNX_R5_REMOTEPROC
It's safe to say N if not interested in using RPU r5f cores.
+
+config TEE_REMOTEPROC
+ tristate "Remoteproc support by a TEE application"
+ depends on OPTEE
+ help
+ Support a remote processor with a TEE application. The Trusted
+ Execution Context is responsible for loading the trusted firmware
+ image and managing the remote processor's lifecycle.
+ This can be either built-in or a loadable module.
+
endif # REMOTEPROC
endmenu
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index 91314a9b43ce..fa8daebce277 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -36,6 +36,7 @@ obj-$(CONFIG_RCAR_REMOTEPROC) += rcar_rproc.o
obj-$(CONFIG_ST_REMOTEPROC)+= st_remoteproc.o
obj-$(CONFIG_ST_SLIM_REMOTEPROC) += st_slim_rproc.o
obj-$(CONFIG_STM32_RPROC) += stm32_rproc.o
+obj-$(CONFIG_TEE_REMOTEPROC) += tee_remoteproc.o
obj-$(CONFIG_TI_K3_DSP_REMOTEPROC) += ti_k3_dsp_remoteproc.o
obj-$(CONFIG_TI_K3_R5_REMOTEPROC) += ti_k3_r5_remoteproc.o
obj-$(CONFIG_XLNX_R5_REMOTEPROC) += xlnx_r5_remoteproc.o
diff --git a/drivers/remoteproc/tee_remoteproc.c
b/drivers/remoteproc/tee_remoteproc.c
new file mode 100644
index ..9455fd9d0d2d
--- /dev/null
+++ b/drivers/remoteproc/tee_remoteproc.c
@@ -0,0 +1,451 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) STMicroelectronics 2024 - All Rights Reserved
+ * Author: Arnaud Pouliquen
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "remoteproc_internal.h"
+
+#define MAX_TEE_PARAM_ARRY_MEMBER 4
+
+/*
+ * Authentication of the firmware and load in the remote processor memory
+ *
+ * [in] params[0].value.a:unique 32bit identifier of the remote processor
+ * [in] params[1].memref: buffer containing the image of the
buffer
+ */
+#define TA_RPROC_FW_CMD_LOAD_FW1
+
+/*
+ * Start the remote processor
+ *
+ * [in] params[0].value.a:unique 32bit identifier of the remote processor
+ */
+#define TA_RPROC_FW_CMD_START_FW 2
+
+/*
+ * Stop the remote processor
+ *
+ * [in] params[0].value.a:unique 32bit identifier of the remote processor
+ */
+#define TA_RPROC_FW_CMD_STOP_FW3
+
+/*
+ * Return the address of the resource table, or 0 if not found
+ * No check is done to verify that the address returned is accessible by
+ * the non secure context. If the resource table is loaded in a protected
+ * memory the access by the non secure context will lead to a data abort.
+ *
+ * [in] params[0].value.a:unique 32bit identifier of the remote processor
+ * [out] params[1].value.a: 32bit LSB resource table memory address
+ * [out] params[1].value.b: 32bit MSB resource table memory address
+ * [out] params[2].value.a: 32bit LSB resource table memory size
+ * [out] params[2].value.b: 32bit MSB resource table memory size
+ */
+#define TA_RPROC_FW_CMD_GET_RSC_TABLE 4
+
+/*
+ * Return the address of the core dump
+ *
+ * [in] params[0].value.a:unique 32bit identifier of the remote processor
+ * [out] params[1].memref: address of the core dump image if exist,
+ * else return Null
+ */
+#define TA_RPROC_FW_CMD_GET_COREDUMP 5
+
+struct tee_rproc_context {
+ struct list_head sessions;
+ struct tee_context *tee_ctx;
+ struct device *dev;
+};
+
+static struct tee_rproc_context *tee_rproc_ctx;
+
+static void tee_rproc_prepare_args(struct tee_rproc *trproc, int cmd,
+ struct
Re: [PATCH 1/3] rust: add static_call support
On Fri, Jun 07, 2024 at 09:43:29AM +, Alice Ryhl wrote:
> Peter Zijlstra wrote:
> > On Thu, Jun 06, 2024 at 09:09:00PM +0200, Miguel Ojeda wrote:
> > > On Thu, Jun 6, 2024 at 7:19 PM Peter Zijlstra
> > > wrote:
> > > >
> > > > This is absolutely unreadable gibberish -- how am I supposed to keep
> > > > this in sync with the rest of the static_call infrastructure?
> > >
> > > Yeah, they are macros, which look different from "normal" Rust code.
> >
> > Macros like CPP ?
>
> Yes, this patch series uses declarative macros, which are the closest
> that Rust has to the C preprocessor. They are powerful, but just like
> CPP, they can become pretty complicated and hard to read if you are
> doing something non-trivial.
>
> The macro_rules! block is how you define a new declarative macro.
I'm sorry, but 30+ years of reading ! as NOT (or factorial) isn't going
to go away. So I'm reading your macros do NOT rule.
> The ($name:ident($($args:expr),* $(,)?)) part defines the arguments to
> the declarative macro. This syntax means:
>
> 1. The input starts with any identifier, which we call $name.
> 2. Then there must be a ( token.
The above exaple fails, because the next token is :ident, whatever the
heck that might be. Also, extra points for line-noise due to lack of
whitespace.
> So for example, you might invoke the macro like this:
>
> static_call!(tp_func_my_tracepoint(__data, &mut my_task_struct));
static_call NOT (blah dog blah);
> Inside the macro, you will see things such as:
> $crate::macros::paste! { $crate::bindings:: [<__SCK__ $name >]; }
>
> The Rust syntax for invoking a macro has an exclamation mark after the
Like I said before, the creator of Rust must've been an esoteric
language freak and must've wanted to make this unreadable on purpose :/
Also, why the white space beteen the :: scope operator and the [< thing?
that's just weird. I would then expect the output to be:
...::bindings:: __SCK__my_static_key
> name, so you know that $crate::macros::paste is a macro. The `paste`
> macro just emits its input unchanged, except that any identifiers
> between [< and >] are concatenated into a single identifier. So if $name
> is my_static_key, then the above invocation of paste! emits:
>
> $crate::bindings::__SCK__my_static_key;
But it doesn't, so it isn't unmodified, it seems to strip whitespace.
> The $crate::bindings module is where the output of bindgen goes, so this
> should correspond to the C symbol called __SCK__my_static_key.
>
> > > Is there something we could do to help here? I think Alice and others
> > > would be happy to explain how it works and/or help maintain it in the
> > > future if you prefer.
> >
> > Write a new language that looks more like C -- pretty please ? :-)
> >
> > Mostly I would just really like you to just use arm/jump_label.h,
> > they're inline functions and should be doable with IR, no weirdo CPP
> > involved in this case.
>
> I assume that you're referring to static_key_false here? I don't think
> that function can be exposed using IR because it passes the function
> argument called key as an "i" argument to an inline assembly block. Any
> attempt to compile static_key_false without knowing the value of key at
> compile time will surely fail to compile with the
>
> invalid operand for inline asm constraint 'i'
>
> error.
You can have clang read the header files and compile them into
Intermediate-Representation, and have it splice the lot into the Rust
crap's IR and voila, compile time.
You just need to extend the rust thing to be able to consume C header
files.
Re: [PATCH 1/3] rust: add static_call support
On Fri, Jun 7, 2024 at 12:52 PM Peter Zijlstra wrote:
>
> I'm sorry, but 30+ years of reading ! as NOT (or factorial) isn't going
> to go away. So I'm reading your macros do NOT rule.
It makes it clear what is macro call or not. They could have gone for
UPPERCASE names (for instance), yes. On the other hand, they do not
work like C macros and are ~hygienic, so it also makes sense to avoid
confusion here.
I mean, I am not suggesting to do a CPP-pass to Rust files, but if
someone really, really wanted to mix them in a single file, it would
be nice to not confuse the two kinds. :)
Generally they feel "closer" to the language (given what they
do/support) compared to the CPP ones, so it also makes sense they
don't "shout" so much compared to UPPERCASE, if that makes sense.
> The above exaple fails, because the next token is :ident, whatever the
> heck that might be. Also, extra points for line-noise due to lack of
> whitespace.
$name:ident means "match what Rust would consider an identifier here
and call it $name for the purposes of this macro".
So, for instance, $x:ident matches:
a
a2
a_b
But it would not match:
2a
a-b
a _b
For the usual reasons why those are not identifiers.
https://godbolt.org/z/G7v4j67dc
fn f(x: i32) -> i32 {
x * 2
}
macro_rules! f {
($x:ident) => { $x * 2 }
}
fn main() {
let a = 42;
let b = f(a); // Function.
let c = f!(a); // Macro.
//let c = f!(a2); // Works, but the variable does not exist.
//let c = f!(2a); // Error: no rules expected the token `2a`.
//let c = f!(a_b); // Works, but the variable does not exist.
//let c = f!(a-b); // Error: no rules expected the token `-`.
//let c = f!(a_ b); // Error: no rules expected the token `b`.
println!("{a} {b} {c}");
}
I hope this makes it clearer.
> You just need to extend the rust thing to be able to consume C header
> files.
I agree, because in practice it is quite useful for a language like
Rust that consuming C header files is "natively" supported.
Though it also has downsides and is a big decision, which is why, like
Alice mentioned, some people agree, and some people don't.
Nevertheless, we have been doing our best for a long time to get the
best we can for the kernel -- just 2 days ago we told the Rust project
in one of our meetings that it would be nice to see that particular
"project goal" from that document realized (among others).
Cheers,
Miguel
[RFC PATCH] ftrace: Skip __fentry__ location of overridden weak functions
ftrace_location() was changed to not only return the __fentry__ location
when called for the __fentry__ location, but also when called for the
sym+0 location after commit aebfd12521d9 ("x86/ibt,ftrace: Search for
__fentry__ location"). That is, if sym+0 location is not __fentry__,
ftrace_location() would find one over the entire size of the sym.
However, there is case that more than one __fentry__ exist in the sym
range (described below) and ftrace_location() would find wrong __fentry__
location by binary searching, which would cause its users like livepatch/
kprobe/bpf to not work properly on this sym!
The case is that, based on current compiler behavior, suppose:
- function A is followed by weak function B1 in same binary file;
- weak function B1 is overridden by function B2;
Then in the final binary file:
- symbol B1 will be removed from symbol table while its instructions are
not removed;
- __fentry__ of B1 will be still in __mcount_loc table;
- function size of A is computed by substracting the symbol address of
A from its next symbol address (see kallsyms_lookup_size_offset()),
but because symbol info of B1 is removed, the next symbol of A is
originally the next symbol of B1. See following example, function
sizeof A will be (symbol_address_C - symbol_address_A):
symbol_address_A
symbol_address_B1 (Not in symbol table)
symbol_address_C
The weak function issue has been discovered in commit b39181f7c690
("ftrace: Add FTRACE_MCOUNT_MAX_OFFSET to avoid adding weak function")
but it didn't resolve the issue in ftrace_location().
There may be following resolutions:
1. Shrink the search range when __fentry__ is not a sym+0 location,
for example use the macro FTRACE_MCOUNT_MAX_OFFSET. This need every
arch to define its own FTRACE_MCOUNT_MAX_OFFSET:
ftrace_location() {
...
if (!offset)
loc = ftrace_location_range(ip, ip + FTRACE_MCOUNT_MAX_OFFSET + 1);
...
}
2. Define arch-specific arch_ftrace_location() based on its own
different cases of __fentry__ position, for example:
ftrace_location() {
...
if (!offset)
loc = arch_ftrace_location(ip);
...
}
3. Skip __fentry__ of non-override weak function in ftrace_process_locs()
then all records in ftrace_pages are valid. The reason why this scheme
may work is that both __mcount_loc and symbol table are sorted and it
can be assumed that one function has only one __fentry__ location. Then
commit b39181f7c690 ("ftrace: Add FTRACE_MCOUNT_MAX_OFFSET to avoid
adding weak function") can be reverted (not do in this patch). However,
looking up size and offset of every record in __mount_loc table will
slow down system boot and module load.
Solution 1 and 2 need every arch to handle the complex fentry location
case, I use solution 3 as RFC.
Fixes: aebfd12521d9 ("x86/ibt,ftrace: Search for __fentry__ location")
Signed-off-by: Zheng Yejian
---
include/linux/module.h | 8
kernel/module/kallsyms.c | 23 +--
kernel/trace/ftrace.c| 20 +---
3 files changed, 38 insertions(+), 13 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index ffa1c603163c..3d5a2165160d 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -954,6 +954,9 @@ unsigned long module_kallsyms_lookup_name(const char *name);
unsigned long find_kallsyms_symbol_value(struct module *mod, const char *name);
+int find_kallsyms_symbol(struct module *mod, unsigned long addr,
+unsigned long *size, unsigned long *offset);
+
#else /* CONFIG_MODULES && CONFIG_KALLSYMS */
static inline int module_kallsyms_on_each_symbol(const char *modname,
@@ -997,6 +1000,11 @@ static inline unsigned long
find_kallsyms_symbol_value(struct module *mod,
return 0;
}
+static inline int find_kallsyms_symbol(struct module *mod, unsigned long addr,
+ unsigned long *size, unsigned long
*offset)
+{
+ return 0;
+}
#endif /* CONFIG_MODULES && CONFIG_KALLSYMS */
#endif /* _LINUX_MODULE_H */
diff --git a/kernel/module/kallsyms.c b/kernel/module/kallsyms.c
index 62fb57bb9f16..d70fb4ead794 100644
--- a/kernel/module/kallsyms.c
+++ b/kernel/module/kallsyms.c
@@ -253,10 +253,10 @@ static const char *kallsyms_symbol_name(struct
mod_kallsyms *kallsyms, unsigned
* Given a module and address, find the corresponding symbol and return its
name
* while providing its size and offset if needed.
*/
-static const char *find_kallsyms_symbol(struct module *mod,
- unsigned long addr,
- unsigned long *size,
- unsigned long *offset)
+static const char *__find_kallsyms_symbol(struct module *mod,
+ unsigned long addr,
+ unsigned long *size,
+
Re: [PATCH v2] rpmsg: qcom_smd: Improve error handling for qcom_smd_parse_edge
On Thu, Jun 06, 2024 at 09:01:36PM +0200, Luca Weiss wrote: > When the mailbox driver has not probed yet, the error message "failed to > parse smd edge" is just going to confuse users, so improve the error > prints a bit. > > Cover the last remaining exits from qcom_smd_parse_edge with proper > error prints, especially the one for the mbox_chan deserved > dev_err_probe to handle EPROBE_DEFER nicely. And add one for ipc_regmap > also to be complete. > > With this done, we can remove the outer print completely. > > Signed-off-by: Luca Weiss > --- > Changes in v2: > - Rebase on qcom for-next, drop dts patches which have been applied > - Improve error printing situation (Bjorn) > - Link to v1: > https://lore.kernel.org/r/20240424-apcs-mboxes-v1-0-6556c47cb...@z3ntu.xyz > --- > drivers/rpmsg/qcom_smd.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
[PATCH net-next v7 06/15] mm: page_frag: add '_va' suffix to page_frag API
Currently the page_frag API is returning 'virtual address'
or 'va' when allocing and expecting 'virtual address' or
'va' as input when freeing.
As we are about to support new use cases that the caller
need to deal with 'struct page' or need to deal with both
'va' and 'struct page'. In order to differentiate the API
handling between 'va' and 'struct page', add '_va' suffix
to the corresponding API mirroring the page_pool_alloc_va()
API of the page_pool. So that callers expecting to deal with
va, page or both va and page may call page_frag_alloc_va*,
page_frag_alloc_pg*, or page_frag_alloc* API accordingly.
CC: Alexander Duyck
Signed-off-by: Yunsheng Lin
---
drivers/net/ethernet/google/gve/gve_rx.c | 4 ++--
drivers/net/ethernet/intel/ice/ice_txrx.c | 2 +-
drivers/net/ethernet/intel/ice/ice_txrx.h | 2 +-
drivers/net/ethernet/intel/ice/ice_txrx_lib.c | 2 +-
.../net/ethernet/intel/ixgbevf/ixgbevf_main.c | 4 ++--
.../marvell/octeontx2/nic/otx2_common.c | 2 +-
drivers/net/ethernet/mediatek/mtk_wed_wo.c| 4 ++--
drivers/nvme/host/tcp.c | 8 +++
drivers/nvme/target/tcp.c | 22 +--
drivers/vhost/net.c | 6 ++---
include/linux/page_frag_cache.h | 21 +-
include/linux/skbuff.h| 2 +-
kernel/bpf/cpumap.c | 2 +-
mm/page_frag_cache.c | 12 +-
mm/page_frag_test.c | 13 ++-
net/core/skbuff.c | 18 +++
net/core/xdp.c| 2 +-
net/rxrpc/txbuf.c | 15 +++--
net/sunrpc/svcsock.c | 6 ++---
19 files changed, 76 insertions(+), 71 deletions(-)
diff --git a/drivers/net/ethernet/google/gve/gve_rx.c
b/drivers/net/ethernet/google/gve/gve_rx.c
index acb73d4d0de6..b6c10100e462 100644
--- a/drivers/net/ethernet/google/gve/gve_rx.c
+++ b/drivers/net/ethernet/google/gve/gve_rx.c
@@ -729,7 +729,7 @@ static int gve_xdp_redirect(struct net_device *dev, struct
gve_rx_ring *rx,
total_len = headroom + SKB_DATA_ALIGN(len) +
SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
- frame = page_frag_alloc(&rx->page_cache, total_len, GFP_ATOMIC);
+ frame = page_frag_alloc_va(&rx->page_cache, total_len, GFP_ATOMIC);
if (!frame) {
u64_stats_update_begin(&rx->statss);
rx->xdp_alloc_fails++;
@@ -742,7 +742,7 @@ static int gve_xdp_redirect(struct net_device *dev, struct
gve_rx_ring *rx,
err = xdp_do_redirect(dev, &new, xdp_prog);
if (err)
- page_frag_free(frame);
+ page_frag_free_va(frame);
return err;
}
diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.c
b/drivers/net/ethernet/intel/ice/ice_txrx.c
index 8bb743f78fcb..399b317c509d 100644
--- a/drivers/net/ethernet/intel/ice/ice_txrx.c
+++ b/drivers/net/ethernet/intel/ice/ice_txrx.c
@@ -126,7 +126,7 @@ ice_unmap_and_free_tx_buf(struct ice_tx_ring *ring, struct
ice_tx_buf *tx_buf)
dev_kfree_skb_any(tx_buf->skb);
break;
case ICE_TX_BUF_XDP_TX:
- page_frag_free(tx_buf->raw_buf);
+ page_frag_free_va(tx_buf->raw_buf);
break;
case ICE_TX_BUF_XDP_XMIT:
xdp_return_frame(tx_buf->xdpf);
diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.h
b/drivers/net/ethernet/intel/ice/ice_txrx.h
index feba314a3fe4..6379f57d8228 100644
--- a/drivers/net/ethernet/intel/ice/ice_txrx.h
+++ b/drivers/net/ethernet/intel/ice/ice_txrx.h
@@ -148,7 +148,7 @@ static inline int ice_skb_pad(void)
* @ICE_TX_BUF_DUMMY: dummy Flow Director packet, unmap and kfree()
* @ICE_TX_BUF_FRAG: mapped skb OR &xdp_buff frag, only unmap DMA
* @ICE_TX_BUF_SKB: &sk_buff, unmap and consume_skb(), update stats
- * @ICE_TX_BUF_XDP_TX: &xdp_buff, unmap and page_frag_free(), stats
+ * @ICE_TX_BUF_XDP_TX: &xdp_buff, unmap and page_frag_free_va(), stats
* @ICE_TX_BUF_XDP_XMIT: &xdp_frame, unmap and xdp_return_frame(), stats
* @ICE_TX_BUF_XSK_TX: &xdp_buff on XSk queue, xsk_buff_free(), stats
*/
diff --git a/drivers/net/ethernet/intel/ice/ice_txrx_lib.c
b/drivers/net/ethernet/intel/ice/ice_txrx_lib.c
index 2719f0e20933..a1a41a14df0d 100644
--- a/drivers/net/ethernet/intel/ice/ice_txrx_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_txrx_lib.c
@@ -250,7 +250,7 @@ ice_clean_xdp_tx_buf(struct device *dev, struct ice_tx_buf
*tx_buf,
switch (tx_buf->type) {
case ICE_TX_BUF_XDP_TX:
- page_frag_free(tx_buf->raw_buf);
+ page_frag_free_va(tx_buf->raw_buf);
break;
case ICE_TX_BUF_XDP_XMIT:
xdp_return_frame_bulk(tx_buf->xdpf, bq);
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
b/drivers/net/ethernet/intel/ixgbevf
[PATCH net-next v7 07/15] mm: page_frag: avoid caller accessing 'page_frag_cache' directly
Use appropriate frag_page API instead of caller accessing
'page_frag_cache' directly.
CC: Alexander Duyck
Signed-off-by: Yunsheng Lin
---
drivers/vhost/net.c | 2 +-
include/linux/page_frag_cache.h | 10 ++
mm/page_frag_test.c | 2 +-
net/core/skbuff.c | 6 +++---
net/rxrpc/conn_object.c | 4 +---
net/rxrpc/local_object.c| 4 +---
net/sunrpc/svcsock.c| 6 ++
7 files changed, 19 insertions(+), 15 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 6691fac01e0d..b2737dc0dc50 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -1325,7 +1325,7 @@ static int vhost_net_open(struct inode *inode, struct
file *f)
vqs[VHOST_NET_VQ_RX]);
f->private_data = n;
- n->pf_cache.va = NULL;
+ page_frag_cache_init(&n->pf_cache);
return 0;
}
diff --git a/include/linux/page_frag_cache.h b/include/linux/page_frag_cache.h
index c6fde197a6eb..6ac3a25089d1 100644
--- a/include/linux/page_frag_cache.h
+++ b/include/linux/page_frag_cache.h
@@ -23,6 +23,16 @@ struct page_frag_cache {
bool pfmemalloc;
};
+static inline void page_frag_cache_init(struct page_frag_cache *nc)
+{
+ nc->va = NULL;
+}
+
+static inline bool page_frag_cache_is_pfmemalloc(struct page_frag_cache *nc)
+{
+ return !!nc->pfmemalloc;
+}
+
void page_frag_cache_drain(struct page_frag_cache *nc);
void __page_frag_cache_drain(struct page *page, unsigned int count);
void *__page_frag_alloc_va_align(struct page_frag_cache *nc,
diff --git a/mm/page_frag_test.c b/mm/page_frag_test.c
index c3cfce87fbbf..8b259e422fae 100644
--- a/mm/page_frag_test.c
+++ b/mm/page_frag_test.c
@@ -341,7 +341,7 @@ static int __init page_frag_test_init(void)
u64 duration;
int ret;
- test_frag.va = NULL;
+ page_frag_cache_init(&test_frag);
atomic_set(&nthreads, 2);
init_completion(&wait);
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 6842fa6a71a5..ac5fe61c0ff2 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -741,12 +741,12 @@ struct sk_buff *__netdev_alloc_skb(struct net_device
*dev, unsigned int len,
if (in_hardirq() || irqs_disabled()) {
nc = this_cpu_ptr(&netdev_alloc_cache);
data = page_frag_alloc_va(nc, len, gfp_mask);
- pfmemalloc = nc->pfmemalloc;
+ pfmemalloc = page_frag_cache_is_pfmemalloc(nc);
} else {
local_bh_disable();
nc = this_cpu_ptr(&napi_alloc_cache.page);
data = page_frag_alloc_va(nc, len, gfp_mask);
- pfmemalloc = nc->pfmemalloc;
+ pfmemalloc = page_frag_cache_is_pfmemalloc(nc);
local_bh_enable();
}
@@ -834,7 +834,7 @@ struct sk_buff *napi_alloc_skb(struct napi_struct *napi,
unsigned int len)
len = SKB_HEAD_ALIGN(len);
data = page_frag_alloc_va(&nc->page, len, gfp_mask);
- pfmemalloc = nc->page.pfmemalloc;
+ pfmemalloc = page_frag_cache_is_pfmemalloc(&nc->page);
}
if (unlikely(!data))
diff --git a/net/rxrpc/conn_object.c b/net/rxrpc/conn_object.c
index 1539d315afe7..694c4df7a1a3 100644
--- a/net/rxrpc/conn_object.c
+++ b/net/rxrpc/conn_object.c
@@ -337,9 +337,7 @@ static void rxrpc_clean_up_connection(struct work_struct
*work)
*/
rxrpc_purge_queue(&conn->rx_queue);
- if (conn->tx_data_alloc.va)
- __page_frag_cache_drain(virt_to_page(conn->tx_data_alloc.va),
- conn->tx_data_alloc.pagecnt_bias);
+ page_frag_cache_drain(&conn->tx_data_alloc);
call_rcu(&conn->rcu, rxrpc_rcu_free_connection);
}
diff --git a/net/rxrpc/local_object.c b/net/rxrpc/local_object.c
index 504453c688d7..a8cffe47cf01 100644
--- a/net/rxrpc/local_object.c
+++ b/net/rxrpc/local_object.c
@@ -452,9 +452,7 @@ void rxrpc_destroy_local(struct rxrpc_local *local)
#endif
rxrpc_purge_queue(&local->rx_queue);
rxrpc_purge_client_connections(local);
- if (local->tx_alloc.va)
- __page_frag_cache_drain(virt_to_page(local->tx_alloc.va),
- local->tx_alloc.pagecnt_bias);
+ page_frag_cache_drain(&local->tx_alloc);
}
/*
diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index 42d20412c1c3..4b1e87187614 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -1609,7 +1609,6 @@ static void svc_tcp_sock_detach(struct svc_xprt *xprt)
static void svc_sock_free(struct svc_xprt *xprt)
{
struct svc_sock *svsk = container_of(xprt, struct svc_sock, sk_xprt);
- struct page_frag_cache *pfc = &svsk->sk_frag_cache;
struct socket *sock = svsk->sk_sock;
trace_svcsock_free(svsk, sock);
@@ -1619,8 +1618,7 @@ static void svc_sock_free(struct svc_xprt *xprt)
sockfd_put(sock);
el
Re: [PATCH] kallsyms, livepatch: Fix livepatch with CONFIG_LTO_CLANG
Hi, On Tue, 4 Jun 2024, Song Liu wrote: > With CONFIG_LTO_CLANG, the compiler may postfix symbols with .llvm. > to avoid symbol duplication. scripts/kallsyms.c sorted the symbols > without these postfixes. The default symbol lookup also removes these > postfixes before comparing symbols. > > On the other hand, livepatch need to look up symbols with the full names. > However, calling kallsyms_on_each_match_symbol with full name (with the > postfix) cannot find the symbol(s). As a result, we cannot livepatch > kernel functions with .llvm. postfix or kernel functions that use > relocation information to symbols with .llvm. postfixes. > > Fix this by calling kallsyms_on_each_match_symbol without the postfix; > and then match the full name (with postfix) in klp_match_callback. > > Signed-off-by: Song Liu > --- > include/linux/kallsyms.h | 13 + > kernel/kallsyms.c| 21 - > kernel/livepatch/core.c | 32 +++- > 3 files changed, 60 insertions(+), 6 deletions(-) I do not like much that something which seems to be kallsyms-internal is leaked out. You need to export cleanup_symbol_name() and there is now a lot of code outside. I would feel much more comfortable if it is all hidden from kallsyms users and kept there. Would it be possible? Moreover, isn't there a similar problem for ftrace, kprobes, ebpf,...? Thank you, Miroslav
[PATCH] kernel/trace: fix possible deadlock in trie_delete_elem
On bpf syscall map operations the bpf_disable_instrumentation function
is called for the reason described in the comment to the function.
The description matches the bug case. The function increments a per CPU
integer variable bpf_prog_active. The variable is not processed in the
bpf trace path. The fix implements a similar processing as for kprobe
handling. The fix degrades the bpf tracing by skipping some eBPF trace
sequences that otherwise might yield deadlock.
Reported-by: syzbot+9d95beb2a3c260622...@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=9d95beb2a3c260622518
Link: https://lore.kernel.org/all/adb08b0614139...@google.com/T/
Signed-off-by: Wojciech Gładysz
---
kernel/trace/bpf_trace.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 6249dac61701..8de2e084b162 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -2391,7 +2391,9 @@ void __bpf_trace_run(struct bpf_raw_tp_link *link, u64
*args)
struct bpf_trace_run_ctx run_ctx;
cant_sleep();
- if (unlikely(this_cpu_inc_return(*(prog->active)) != 1)) {
+
+ // if the instrumentation is not disabled disable recurrence and go
+ if (unlikely(__this_cpu_inc_return(bpf_prog_active) != 1)) {
bpf_prog_inc_misses_counter(prog);
goto out;
}
@@ -2405,7 +2407,7 @@ void __bpf_trace_run(struct bpf_raw_tp_link *link, u64
*args)
bpf_reset_run_ctx(old_run_ctx);
out:
- this_cpu_dec(*(prog->active));
+ __this_cpu_dec(bpf_prog_active);
}
#define UNPACK(...)__VA_ARGS__
--
2.35.3
Re: [PATCH v3 1/4] tracing: add __print_sym() to replace __print_symbolic()
Hi Johannes,
kernel test robot noticed the following build errors:
[auto build test ERROR on mcgrof/modules-next]
[also build test ERROR on arnd-asm-generic/master tip/timers/core net/main
net-next/main linus/master horms-ipvs/master v6.10-rc2 next-20240607]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url:
https://github.com/intel-lab-lkp/linux/commits/Johannes-Berg/tracing-add-__print_sym-to-replace-__print_symbolic/20240607-043503
base: https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux.git
modules-next
patch link:
https://lore.kernel.org/r/20240606203255.49433-7-johannes%40sipsolutions.net
patch subject: [PATCH v3 1/4] tracing: add __print_sym() to replace
__print_symbolic()
config: arc-randconfig-002-20240607
(https://download.01.org/0day-ci/archive/20240607/202406072129.3zzfdolc-...@intel.com/config)
compiler: arc-elf-gcc (GCC) 13.2.0
reproduce (this is a W=1 build):
(https://download.01.org/0day-ci/archive/20240607/202406072129.3zzfdolc-...@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot
| Closes:
https://lore.kernel.org/oe-kbuild-all/202406072129.3zzfdolc-...@intel.com/
All errors (new ones prefixed by >>):
kernel/trace/trace_events.c: In function 'show_sym_list':
>> kernel/trace/trace_events.c:1586:31: error: invalid use of undefined type
>> 'struct module'
1586 | sym_defs = mod->trace_sym_defs;
| ^~
kernel/trace/trace_events.c:1587:33: error: invalid use of undefined type
'struct module'
1587 | n_sym_defs = mod->num_trace_sym_defs;
| ^~
vim +1586 kernel/trace/trace_events.c
1575
1576 /* note: @name is not NUL-terminated */
1577 static void show_sym_list(struct seq_file *m, struct trace_event_call
*call,
1578const char *name, unsigned int name_len)
1579 {
1580 struct trace_sym_def **sym_defs;
1581 unsigned int n_sym_defs, i;
1582
1583 if (call->module) {
1584 struct module *mod = call->module;
1585
> 1586 sym_defs = mod->trace_sym_defs;
1587 n_sym_defs = mod->num_trace_sym_defs;
1588 } else {
1589 sym_defs = __start_ftrace_sym_defs;
1590 n_sym_defs = __stop_ftrace_sym_defs -
__start_ftrace_sym_defs;
1591 }
1592
1593 for (i = 0; i < n_sym_defs; i++) {
1594 unsigned int sym_len;
1595
1596 if (!sym_defs[i])
1597 continue;
1598 if (sym_defs[i]->system != call->class->system)
1599 continue;
1600 sym_len = strlen(sym_defs[i]->symbol_id);
1601 if (name_len != sym_len)
1602 continue;
1603 if (strncmp(sym_defs[i]->symbol_id, name, sym_len))
1604 continue;
1605 if (sym_defs[i]->show)
1606 sym_defs[i]->show(m);
1607 break;
1608 }
1609 }
1610
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH v3 1/4] tracing: add __print_sym() to replace __print_symbolic()
Hi Johannes,
kernel test robot noticed the following build errors:
[auto build test ERROR on mcgrof/modules-next]
[also build test ERROR on arnd-asm-generic/master tip/timers/core net/main
net-next/main linus/master horms-ipvs/master v6.10-rc2 next-20240607]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url:
https://github.com/intel-lab-lkp/linux/commits/Johannes-Berg/tracing-add-__print_sym-to-replace-__print_symbolic/20240607-043503
base: https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux.git
modules-next
patch link:
https://lore.kernel.org/r/20240606203255.49433-7-johannes%40sipsolutions.net
patch subject: [PATCH v3 1/4] tracing: add __print_sym() to replace
__print_symbolic()
config: arm64-randconfig-002-20240607
(https://download.01.org/0day-ci/archive/20240607/202406072141.olmqbch3-...@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project
d7d2d4f53fc79b4b58e8d8d08151b577c3699d4a)
reproduce (this is a W=1 build):
(https://download.01.org/0day-ci/archive/20240607/202406072141.olmqbch3-...@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot
| Closes:
https://lore.kernel.org/oe-kbuild-all/202406072141.olmqbch3-...@intel.com/
All errors (new ones prefixed by >>):
In file included from kernel/trace/trace_events.c:15:
In file included from include/linux/security.h:33:
In file included from include/linux/mm.h:2210:
include/linux/vmstat.h:522:36: warning: arithmetic between different
enumeration types ('enum node_stat_item' and 'enum lru_list')
[-Wenum-enum-conversion]
522 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
| ~~~ ^ ~~~
>> kernel/trace/trace_events.c:1586:17: error: incomplete definition of type
>> 'struct module'
1586 | sym_defs = mod->trace_sym_defs;
|~~~^
include/linux/printk.h:350:8: note: forward declaration of 'struct module'
350 | struct module;
|^
kernel/trace/trace_events.c:1587:19: error: incomplete definition of type
'struct module'
1587 | n_sym_defs = mod->num_trace_sym_defs;
| ~~~^
include/linux/printk.h:350:8: note: forward declaration of 'struct module'
350 | struct module;
|^
1 warning and 2 errors generated.
vim +1586 kernel/trace/trace_events.c
1575
1576 /* note: @name is not NUL-terminated */
1577 static void show_sym_list(struct seq_file *m, struct trace_event_call
*call,
1578const char *name, unsigned int name_len)
1579 {
1580 struct trace_sym_def **sym_defs;
1581 unsigned int n_sym_defs, i;
1582
1583 if (call->module) {
1584 struct module *mod = call->module;
1585
> 1586 sym_defs = mod->trace_sym_defs;
1587 n_sym_defs = mod->num_trace_sym_defs;
1588 } else {
1589 sym_defs = __start_ftrace_sym_defs;
1590 n_sym_defs = __stop_ftrace_sym_defs -
__start_ftrace_sym_defs;
1591 }
1592
1593 for (i = 0; i < n_sym_defs; i++) {
1594 unsigned int sym_len;
1595
1596 if (!sym_defs[i])
1597 continue;
1598 if (sym_defs[i]->system != call->class->system)
1599 continue;
1600 sym_len = strlen(sym_defs[i]->symbol_id);
1601 if (name_len != sym_len)
1602 continue;
1603 if (strncmp(sym_defs[i]->symbol_id, name, sym_len))
1604 continue;
1605 if (sym_defs[i]->show)
1606 sym_defs[i]->show(m);
1607 break;
1608 }
1609 }
1610
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
[PATCH] function_graph: Make fgraph_update_pid_func() a stub for !DYNAMIC_FTRACE
From: "Steven Rostedt (Google)"
When CONFIG_DYNAMIC_FTRACE is not set, the function
fgraph_update_pid_func() doesn't do anything. Currently, most of its logic
is within a "#ifdef CONFIG_DYNAMIC_FTRACE" block, but its variables were
declared outside that, and when DYNAMIC_FTRACE is not set, it produces
unused variable warnings.
Instead, just place it (and the helper function fgraph_pid_func()) within
the #ifdef block and have the header file use a empty stub function for
when DYNAMIC_FTRACE is not defined.
Reported-by: kernel test robot
Closes:
https://lore.kernel.org/oe-kbuild-all/202406071806.brjac5ff-...@intel.com/
Signed-off-by: Steven Rostedt (Google)
---
kernel/trace/fgraph.c | 4 ++--
kernel/trace/ftrace_internal.h | 4
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
index c0e428c87ea5..0859ab112db7 100644
--- a/kernel/trace/fgraph.c
+++ b/kernel/trace/fgraph.c
@@ -1151,6 +1151,7 @@ void ftrace_graph_exit_task(struct task_struct *t)
kfree(ret_stack);
}
+#ifdef CONFIG_DYNAMIC_FTRACE
static int fgraph_pid_func(struct ftrace_graph_ent *trace,
struct fgraph_ops *gops)
{
@@ -1177,7 +1178,6 @@ void fgraph_update_pid_func(void)
if (!(graph_ops.flags & FTRACE_OPS_FL_INITIALIZED))
return;
-#ifdef CONFIG_DYNAMIC_FTRACE
list_for_each_entry(op, &graph_ops.subop_list, list) {
if (op->flags & FTRACE_OPS_FL_PID) {
gops = container_of(op, struct fgraph_ops, ops);
@@ -1187,8 +1187,8 @@ void fgraph_update_pid_func(void)
static_call_update(fgraph_func,
gops->entryfunc);
}
}
-#endif
}
+#endif
/* Allocate a return stack for each task */
static int start_graph_tracing(void)
diff --git a/kernel/trace/ftrace_internal.h b/kernel/trace/ftrace_internal.h
index 4bb1e881154a..3235470e61b3 100644
--- a/kernel/trace/ftrace_internal.h
+++ b/kernel/trace/ftrace_internal.h
@@ -52,7 +52,11 @@ static inline int ftrace_shutdown_subops(struct ftrace_ops
*ops, struct ftrace_o
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
extern int ftrace_graph_active;
+# ifdef CONFIG_DYNAMIC_FTRACE
extern void fgraph_update_pid_func(void);
+# else
+static inline void fgraph_update_pid_func(void) {}
+# endif
#else /* !CONFIG_FUNCTION_GRAPH_TRACER */
# define ftrace_graph_active 0
static inline void fgraph_update_pid_func(void) {}
--
2.43.0
[ANNOUNCE] 4.19.315-rt135
Hello RT-list! I'm pleased to announce the 4.19.315-rt135 stable release. This is just an update to the v4.19.315 stable release, not RT specifc changes. You can get this release via the git tree at: git://git.kernel.org/pub/scm/linux/kernel/git/rt/linux-stable-rt.git branch: v4.19-rt Head SHA1: 66fe9135e37bd5851c55180e2fa301b2968b5588 Or to build 4.19.315-rt135 directly, the following patches should be applied: https://www.kernel.org/pub/linux/kernel/v4.x/linux-4.19.tar.xz https://www.kernel.org/pub/linux/kernel/v4.x/patch-4.19.315.xz https://www.kernel.org/pub/linux/kernel/projects/rt/4.19/older/patch-4.19.315-rt135.patch.xz Signing key fingerprint: 5BF6 7BC5 0826 72CA BB45 ACAE 587C 5ECA 5D0A 306C All keys used for the above files and repositories can be found on the following git repository: git://git.kernel.org/pub/scm/docs/kernel/pgpkeys.git Enjoy! Daniel Changes from v4.19.312-rt134: --- Daniel Wagner (1): Linux 4.19.315-rt135 --- localversion-rt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) --- diff --git a/localversion-rt b/localversion-rt index 6067da4c8c99..e3026053f01e 100644 --- a/localversion-rt +++ b/localversion-rt @@ -1 +1 @@ --rt134 +-rt135
Re: [PATCH v14 02/14] cgroup/misc: Add per resource callbacks for CSS events
On Thu, 06 Jun 2024 20:53:11 -0500, chenridong wrote: I think it is better when _misc_cg_res_alloc fails, it just calls _misc_cg_res_free(cg, index)(add index parameter, it means ending of iterator), so it can avoid calling ->free() that do not call ->alloc(). And in misc_cg_free, just call _misc_cg_res_free(cg, MISC_CG_RES_TYPES) to free all. That makes sense now, Will do that. (BTW you need comment inline :-) Thanks Haitao On 2024/6/6 22:51, Haitao Huang wrote: On Thu, 06 Jun 2024 08:37:31 -0500, chenridong wrote: If _misc_cg_res_alloc fails, maybe some types do not call ->alloc(), but all types ->free() callback >will be called, is that ok? Not sure I understand. Are you suggesting we ignore failures from ->alloc() callback in _misc_cg_res_alloc() as it is per-resource, and have ->free() callback and resource provider of the failing type to handle the failure internally? IIUC, this failure only happens when a specific subcgroup is created (memory running out for allocation) so failing that subcgroup as a whole seems fine to me. Note the root node is static and no pre-resource callbacks invoked by misc. And resource provider handles its own allocations for root. In SGX case we too declare a static object for corresponding root sgx_cgroup struct. Note also misc cgroup (except for setting capacity[res] = 0 at root) is all or nothing so no mechanism to tell user "this resource does not work but others are fine in this particular cgroup." Thanks Haitao -- Using Opera's mail client: http://www.opera.com/mail/
Re: [PATCH v2 2/6] iio: light: stk3310: Implement vdd supply and power it off during suspend
On Sun, Apr 28, 2024 at 05:53:37PM GMT, Jonathan Cameron wrote: > On Tue, 23 Apr 2024 18:33:05 -0400 > Aren Moynihan wrote: > > > From: Ondrej Jirman > > > > VDD power input can be used to completely power off the chip during > > system suspend. Do so if available. > > > > Signed-off-by: Ondrej Jirman > > Signed-off-by: Aren Moynihan > > Suggestions inline. Key thing is take the whole thing devm_ managed > and your life gets much easier. It is mixing the two approaches that > causes problems and often the best plan is to do everything in probe/remove > with devm_ calls to do the cleanup for you. Thank you for writing this up. I've been a bit distracted lately, but I'm hoping to find some time to implement this in a new revision soon. - Aren
Re: [RFC v3 net-next 1/7] net: add rx_sk to trace_kfree_skb
On 6/6/24 9:37 AM, Yan Zhai wrote: > # cat /sys/kernel/debug/tracing/events/skb/kfree_skb/format > name: kfree_skb > ID: 2260 > format: > field:unsigned short common_type; offset:0; > size:2; signed:0; > field:unsigned char common_flags; offset:2; > size:1; signed:0; > field:unsigned char common_preempt_count; offset:3; > size:1; signed:0; > field:int common_pid; offset:4; size:4; signed:1; > > field:void * skbaddr; offset:8; size:8; signed:0; > field:void * location; offset:16; size:8; signed:0; > field:unsigned short protocol; offset:24; size:2; signed:0; > field:enum skb_drop_reason reason; offset:28; > size:4; signed:0; > field:void * rx_skaddr; offset:32; size:8; signed:0; > > Do you think we still need to change the order? > yes. Keeping proper order now ensures no holes creep in with later changes.
Re: [RFC PATCH] ftrace: Skip __fentry__ location of overridden weak functions
On Fri, Jun 07, 2024 at 07:52:11PM +0800, Zheng Yejian wrote:
> ftrace_location() was changed to not only return the __fentry__ location
> when called for the __fentry__ location, but also when called for the
> sym+0 location after commit aebfd12521d9 ("x86/ibt,ftrace: Search for
> __fentry__ location"). That is, if sym+0 location is not __fentry__,
> ftrace_location() would find one over the entire size of the sym.
>
> However, there is case that more than one __fentry__ exist in the sym
> range (described below) and ftrace_location() would find wrong __fentry__
> location by binary searching, which would cause its users like livepatch/
> kprobe/bpf to not work properly on this sym!
>
> The case is that, based on current compiler behavior, suppose:
> - function A is followed by weak function B1 in same binary file;
> - weak function B1 is overridden by function B2;
> Then in the final binary file:
> - symbol B1 will be removed from symbol table while its instructions are
>not removed;
> - __fentry__ of B1 will be still in __mcount_loc table;
> - function size of A is computed by substracting the symbol address of
>A from its next symbol address (see kallsyms_lookup_size_offset()),
>but because symbol info of B1 is removed, the next symbol of A is
>originally the next symbol of B1. See following example, function
>sizeof A will be (symbol_address_C - symbol_address_A):
>
> symbol_address_A
> symbol_address_B1 (Not in symbol table)
> symbol_address_C
>
> The weak function issue has been discovered in commit b39181f7c690
> ("ftrace: Add FTRACE_MCOUNT_MAX_OFFSET to avoid adding weak function")
> but it didn't resolve the issue in ftrace_location().
>
> There may be following resolutions:
Oh gawd, sodding weak functions again.
I would suggest changing scipts/kallsyms.c to emit readily identifiable
symbol names for all the weak junk, eg:
__weak_junk_N
That instantly fixes the immediate problem and Steve's horrid hack can
go away.
Additionally, I would add a boot up pass that would INT3 fill all such
functions and remove/invalidate all
static_call/static_jump/fentry/alternative entry that is inside of them.
Re: [PATCH v2 1/1] dt-bindings: remoteproc: imx_rproc: add minItems for power-domain
On Fri, Jun 07, 2024 at 09:32:26AM +0200, Krzysztof Kozlowski wrote: > On 06/06/2024 17:00, Frank Li wrote: > > "fsl,imx8qxp-cm4" and "fsl,imx8qm-cm4" need minimum 2 power domains. Keep > > the same restriction for other compatible string. > > > > Signed-off-by: Frank Li > > --- > > > > Notes: > > Change from v1 to v2 > > - set minitem to 2 at top > > - Add imx8qm compatible string also > > - use not logic to handle difference compatible string restriction > > - update commit message. > > > > pass dt_binding_check. > > > > make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- -j8 dt_binding_check > > DT_SCHEMA_FILES=fsl,imx-rproc.yaml > > SCHEMA Documentation/devicetree/bindings/processed-schema.json > > CHKDT Documentation/devicetree/bindings > > LINTDocumentation/devicetree/bindings > > DTEX > > Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.example.dts > > DTC_CHK > > Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.example.dtb > > > > .../bindings/remoteproc/fsl,imx-rproc.yaml | 14 ++ > > 1 file changed, 14 insertions(+) > > > > diff --git > > a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml > > b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml > > index df36e29d974ca..da108a39df435 100644 > > --- a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml > > +++ b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml > > @@ -59,6 +59,7 @@ properties: > > maxItems: 32 > > > >power-domains: > > +minItems: 2 > > maxItems: 8 > > > >fsl,auto-boot: > > @@ -99,6 +100,19 @@ allOf: > >properties: > > fsl,iomuxc-gpr: false > > > > + - if: > > + properties: > > +compatible: > > + not: > > +contains: > > + enum: > > +- fsl,imx8qxp-cm4 > > +- fsl,imx8qm-cm4 > > +then: > > + properties: > > +power-domains: > > + minItems: 8 > > What happened with the "else:"? How many power domains is needed for > other devices? So far, only fsl,imx8qxp-cm4 ind fsl,imx8qm-cm4 need power domain (2-8). Power-domains is option property. Can I just remove whole "if"? Frank > > Best regards, > Krzysztof >
Re: [RFC PATCH] ftrace: Skip __fentry__ location of overridden weak functions
On Fri, 7 Jun 2024 17:02:28 +0200 Peter Zijlstra wrote: > > There may be following resolutions: > > Oh gawd, sodding weak functions again. > > I would suggest changing scipts/kallsyms.c to emit readily identifiable > symbol names for all the weak junk, eg: > > __weak_junk_N > > That instantly fixes the immediate problem and Steve's horrid hack can > go away. Right. And when I wrote that hack, I specifically said this should be fixed in kallsyms, and preferably at build time, as that's when the weak functions should all be resolved. -- Steve > > Additionally, I would add a boot up pass that would INT3 fill all such > functions and remove/invalidate all > static_call/static_jump/fentry/alternative entry that is inside of them.
[PATCH v4 0/4] tracing: improve symbolic printing
Before I forget again ... v2 was: - rebased on 6.9-rc1 - always search for __print_sym() and get rid of the DYNPRINT flag and associated code; I think ideally we'll just remove the older __print_symbolic() entirely - use ':' as the separator instead of "//" since that makes searching for it much easier and it's still not a valid char in an identifier - fix RCU v3: - fix #undef issues - fix drop_monitor default - rebase on linux-trace/for-next (there were no conflicts) - move net patches to 3/4 - clarify symbol name matching logic (and remove ")" from it) v4: - fix non-module build and possibly dynamic event handling To recap, it's annoying to have irq/65-iwlwifi:-401 [000]22.79: kfree_skb: ... reason: 0x2 and much nicer to see irq/65-iwlwifi:-401 [000]22.79: kfree_skb: ... reason: RX_DROP_MONITOR but this doesn't work now because __print_symbolic() can only deal with a hard-coded list (which is actually really big.) So here's __print_sym() which doesn't build the list into the kernel image, but creates it at runtime. For userspace, it will look the same as __print_symbolic() (it literally shows __print_symbolic() to userspace) so no changes are needed, but the actual list of values exposed to userspace in there is built dynamically. For SKB drop reasons, this then has all the reasons known when userspace queries the trace format. I guess patch 3/4 should go through net-next, so not sure how to handle this patch series. Or perhaps, as this will not cause conflicts, in fact I've been rebasing it for a long time, go through tracing anyway with an Ack from netdev? But I can also just wait for the trace patch(es) to land and resubmit the net patches after. Assuming this looks good at all :-) Thanks, johannes
[PATCH v4 2/4] tracing/timer: use __print_sym()
From: Johannes Berg
Use the new __print_sym() in the timer tracing, just to show
how to convert something. This adds ~80 bytes of .text for a
saving of ~1.5K of data in my builds.
Note the format changes from
print fmt: "success=%d dependency=%s", REC->success,
__print_symbolic(REC->dependency, { 0, "NONE" }, { (1 << 0), "POSIX_TIMER" }, {
(1 << 1), "PERF_EVENTS" }, { (1 << 2), "SCHED" }, { (1 << 3), "CLOCK_UNSTABLE"
}, { (1 << 4), "RCU" }, { (1 << 5), "RCU_EXP" })
to
print fmt: "success=%d dependency=%s", REC->success,
__print_symbolic(REC->dependency, { 0, "NONE" }, { 1, "POSIX_TIMER" }, { 2,
"PERF_EVENTS" }, { 4, "SCHED" }, { 8, "CLOCK_UNSTABLE" }, { 16, "RCU" }, { 32,
"RCU_EXP" })
since the values are now just printed in the show function as
pure decimal values.
Signed-off-by: Johannes Berg
---
include/trace/events/timer.h | 22 +++---
1 file changed, 7 insertions(+), 15 deletions(-)
diff --git a/include/trace/events/timer.h b/include/trace/events/timer.h
index 1ef58a04fc57..d483abffed78 100644
--- a/include/trace/events/timer.h
+++ b/include/trace/events/timer.h
@@ -402,26 +402,18 @@ TRACE_EVENT(itimer_expire,
#undef tick_dep_mask_name
#undef tick_dep_name_end
-/* The MASK will convert to their bits and they need to be processed too */
-#define tick_dep_name(sdep) TRACE_DEFINE_ENUM(TICK_DEP_BIT_##sdep); \
- TRACE_DEFINE_ENUM(TICK_DEP_MASK_##sdep);
-#define tick_dep_name_end(sdep) TRACE_DEFINE_ENUM(TICK_DEP_BIT_##sdep); \
- TRACE_DEFINE_ENUM(TICK_DEP_MASK_##sdep);
-/* NONE only has a mask defined for it */
-#define tick_dep_mask_name(sdep) TRACE_DEFINE_ENUM(TICK_DEP_MASK_##sdep);
-
-TICK_DEP_NAMES
-
-#undef tick_dep_name
-#undef tick_dep_mask_name
-#undef tick_dep_name_end
-
#define tick_dep_name(sdep) { TICK_DEP_MASK_##sdep, #sdep },
#define tick_dep_mask_name(sdep) { TICK_DEP_MASK_##sdep, #sdep },
#define tick_dep_name_end(sdep) { TICK_DEP_MASK_##sdep, #sdep }
+TRACE_DEFINE_SYM_LIST(tick_dep_names, TICK_DEP_NAMES);
+
+#undef tick_dep_name
+#undef tick_dep_mask_name
+#undef tick_dep_name_end
+
#define show_tick_dep_name(val)\
- __print_symbolic(val, TICK_DEP_NAMES)
+ __print_sym(val, tick_dep_names)
TRACE_EVENT(tick_stop,
--
2.45.2
[PATCH v4 1/4] tracing: add __print_sym() to replace __print_symbolic()
From: Johannes Berg
The way __print_symbolic() works is limited and inefficient
in multiple ways:
- you can only use it with a static list of symbols, but
e.g. the SKB dropreasons are now a dynamic list
- it builds the list in memory _three_ times, so it takes
a lot of memory:
- The print_fmt contains the list (since it's passed to
the macro there). This actually contains the names
_twice_, which is fixed up at runtime.
- TRACE_DEFINE_ENUM() puts a 24-byte struct trace_eval_map
for every entry, plus the string pointed to by it, which
cannot be deduplicated with the strings in the print_fmt
- The in-kernel symbolic printing creates yet another list
of struct trace_print_flags for trace_print_symbols_seq()
- it also requires runtime fixup during init, which is a lot
of string parsing due to the print_fmt fixup
Introduce __print_sym() to - over time - replace the old one.
We can easily extend this also to __print_flags later, but I
cared only about the SKB dropreasons for now, which has only
__print_symbolic().
This new __print_sym() requires only a single list of items,
created by TRACE_DEFINE_SYM_LIST(), or can even use another
already existing list by using TRACE_DEFINE_SYM_FNS() with
lookup and show methods.
Then, instead of doing an init-time fixup, just do this at the
time when userspace reads the print_fmt. This way, dynamically
updated lists are possible.
For userspace, nothing actually changes, because the print_fmt
is shown exactly the same way the old __print_symbolic() was.
This adds about 4k .text in my test builds, but that'll be
more than paid for by the actual conversions.
Signed-off-by: Johannes Berg
---
v2:
- fix RCU
- use ':' as separator to simplify the code, that's
still not valid in a C identifier
v3:
- add missing #undef lines (reported by Simon Horman)
- clarify name is not NUL-terminated and fix logic
for the comparison for that
v4:
- fix non-modular builds and handle TRACE_EVENT_FL_DYNAMIC
---
include/asm-generic/vmlinux.lds.h | 3 +-
include/linux/module.h | 2 +
include/linux/trace_events.h | 7 ++
include/linux/tracepoint.h | 20 +
include/trace/stages/init.h| 55
include/trace/stages/stage2_data_offsets.h | 6 ++
include/trace/stages/stage3_trace_output.h | 9 ++
include/trace/stages/stage7_class_define.h | 3 +
kernel/module/main.c | 3 +
kernel/trace/trace_events.c| 97 +-
kernel/trace/trace_output.c| 45 ++
11 files changed, 247 insertions(+), 3 deletions(-)
diff --git a/include/asm-generic/vmlinux.lds.h
b/include/asm-generic/vmlinux.lds.h
index 5703526d6ebf..8275a06bcaee 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -258,7 +258,8 @@
#define FTRACE_EVENTS()
\
. = ALIGN(8); \
BOUNDED_SECTION(_ftrace_events) \
- BOUNDED_SECTION_BY(_ftrace_eval_map, _ftrace_eval_maps)
+ BOUNDED_SECTION_BY(_ftrace_eval_map, _ftrace_eval_maps) \
+ BOUNDED_SECTION(_ftrace_sym_defs)
#else
#define FTRACE_EVENTS()
#endif
diff --git a/include/linux/module.h b/include/linux/module.h
index ffa1c603163c..7256762d59e1 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -524,6 +524,8 @@ struct module {
unsigned int num_trace_events;
struct trace_eval_map **trace_evals;
unsigned int num_trace_evals;
+ struct trace_sym_def **trace_sym_defs;
+ unsigned int num_trace_sym_defs;
#endif
#ifdef CONFIG_FTRACE_MCOUNT_RECORD
unsigned int num_ftrace_callsites;
diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index 9df3e2973626..2743280c9a46 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -27,6 +27,13 @@ const char *trace_print_flags_seq(struct trace_seq *p, const
char *delim,
const char *trace_print_symbols_seq(struct trace_seq *p, unsigned long val,
const struct trace_print_flags
*symbol_array);
+const char *trace_print_sym_seq(struct trace_seq *p, unsigned long long val,
+ const char *(*lookup)(unsigned long long val));
+const char *trace_sym_lookup(const struct trace_sym_entry *list,
+size_t len, unsigned long long value);
+void trace_sym_show(struct seq_file *m,
+ const struct trace_sym_entry *list, size_t len);
+
#if BITS_PER_LONG == 32
const char *trace_print_flags_seq_u64(struct trace_seq *p, const char *delim,
unsigned long long flags,
diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index 689b6d71590e..cc3b387953d1 100644
--- a/include/linux/tracepoint.h
+++ b/includ
[PATCH v4 4/4] net: drop_monitor: use drop_reason_lookup()
From: Johannes Berg
Now that we have drop_reason_lookup(), we can just use it for
drop_monitor as well, rather than exporting the list itself.
Signed-off-by: Johannes Berg
---
v3:
- look up SKB_DROP_REASON_NOT_SPECIFIED if initial lookup
returns NULL, to preserve previous behaviour
---
include/net/dropreason.h | 4
net/core/drop_monitor.c | 20 +---
net/core/skbuff.c| 6 +++---
3 files changed, 8 insertions(+), 22 deletions(-)
diff --git a/include/net/dropreason.h b/include/net/dropreason.h
index c157070b5303..0e2195ccf2cd 100644
--- a/include/net/dropreason.h
+++ b/include/net/dropreason.h
@@ -38,10 +38,6 @@ struct drop_reason_list {
size_t n_reasons;
};
-/* Note: due to dynamic registrations, access must be under RCU */
-extern const struct drop_reason_list __rcu *
-drop_reasons_by_subsys[SKB_DROP_REASON_SUBSYS_NUM];
-
#ifdef CONFIG_TRACEPOINTS
const char *drop_reason_lookup(unsigned long long value);
void drop_reason_show(struct seq_file *m);
diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c
index 430ed18f8584..fddf6b68bf06 100644
--- a/net/core/drop_monitor.c
+++ b/net/core/drop_monitor.c
@@ -610,9 +610,8 @@ static int net_dm_packet_report_fill(struct sk_buff *msg,
struct sk_buff *skb,
size_t payload_len)
{
struct net_dm_skb_cb *cb = NET_DM_SKB_CB(skb);
- const struct drop_reason_list *list = NULL;
- unsigned int subsys, subsys_reason;
char buf[NET_DM_MAX_SYMBOL_LEN];
+ const char *reason_str;
struct nlattr *attr;
void *hdr;
int rc;
@@ -630,19 +629,10 @@ static int net_dm_packet_report_fill(struct sk_buff *msg,
struct sk_buff *skb,
goto nla_put_failure;
rcu_read_lock();
- subsys = u32_get_bits(cb->reason, SKB_DROP_REASON_SUBSYS_MASK);
- if (subsys < SKB_DROP_REASON_SUBSYS_NUM)
- list = rcu_dereference(drop_reasons_by_subsys[subsys]);
- subsys_reason = cb->reason & ~SKB_DROP_REASON_SUBSYS_MASK;
- if (!list ||
- subsys_reason >= list->n_reasons ||
- !list->reasons[subsys_reason] ||
- strlen(list->reasons[subsys_reason]) > NET_DM_MAX_REASON_LEN) {
- list =
rcu_dereference(drop_reasons_by_subsys[SKB_DROP_REASON_SUBSYS_CORE]);
- subsys_reason = SKB_DROP_REASON_NOT_SPECIFIED;
- }
- if (nla_put_string(msg, NET_DM_ATTR_REASON,
- list->reasons[subsys_reason])) {
+ reason_str = drop_reason_lookup(cb->reason);
+ if (unlikely(!reason_str))
+ reason_str = drop_reason_lookup(SKB_DROP_REASON_NOT_SPECIFIED);
+ if (nla_put_string(msg, NET_DM_ATTR_REASON, reason_str)) {
rcu_read_unlock();
goto nla_put_failure;
}
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index cd1ea6c3e0f8..bd4fb7410284 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -139,13 +139,11 @@ static const struct drop_reason_list drop_reasons_core = {
.n_reasons = ARRAY_SIZE(drop_reasons),
};
-const struct drop_reason_list __rcu *
+static const struct drop_reason_list __rcu *
drop_reasons_by_subsys[SKB_DROP_REASON_SUBSYS_NUM] = {
[SKB_DROP_REASON_SUBSYS_CORE] = RCU_INITIALIZER(&drop_reasons_core),
};
-EXPORT_SYMBOL(drop_reasons_by_subsys);
-#ifdef CONFIG_TRACEPOINTS
const char *drop_reason_lookup(unsigned long long value)
{
unsigned long long subsys_id = value >> SKB_DROP_REASON_SUBSYS_SHIFT;
@@ -162,7 +160,9 @@ const char *drop_reason_lookup(unsigned long long value)
return NULL;
return subsys->reasons[reason];
}
+EXPORT_SYMBOL(drop_reason_lookup);
+#ifdef CONFIG_TRACEPOINTS
void drop_reason_show(struct seq_file *m)
{
u32 subsys_id;
--
2.45.2
[PATCH v4 3/4] net: dropreason: use new __print_sym() in tracing
From: Johannes Berg
The __print_symbolic() could only ever print the core
drop reasons, since that's the way the infrastructure
works. Now that we have __print_sym() with all the
advantages mentioned in that commit, convert to that
and get all the drop reasons from all subsystems. As
we already have a list of them, that's really easy.
This is a little bit of .text (~100 bytes in my build)
and saves a lot of .data (~17k).
Signed-off-by: Johannes Berg
---
include/net/dropreason.h | 5 +
include/trace/events/skb.h | 16 +++---
net/core/skbuff.c | 43 ++
3 files changed, 51 insertions(+), 13 deletions(-)
diff --git a/include/net/dropreason.h b/include/net/dropreason.h
index 56cb7be92244..c157070b5303 100644
--- a/include/net/dropreason.h
+++ b/include/net/dropreason.h
@@ -42,6 +42,11 @@ struct drop_reason_list {
extern const struct drop_reason_list __rcu *
drop_reasons_by_subsys[SKB_DROP_REASON_SUBSYS_NUM];
+#ifdef CONFIG_TRACEPOINTS
+const char *drop_reason_lookup(unsigned long long value);
+void drop_reason_show(struct seq_file *m);
+#endif
+
void drop_reasons_register_subsys(enum skb_drop_reason_subsys subsys,
const struct drop_reason_list *list);
void drop_reasons_unregister_subsys(enum skb_drop_reason_subsys subsys);
diff --git a/include/trace/events/skb.h b/include/trace/events/skb.h
index 07e0715628ec..8a1a63f9e796 100644
--- a/include/trace/events/skb.h
+++ b/include/trace/events/skb.h
@@ -8,15 +8,9 @@
#include
#include
#include
+#include
-#undef FN
-#define FN(reason) TRACE_DEFINE_ENUM(SKB_DROP_REASON_##reason);
-DEFINE_DROP_REASON(FN, FN)
-
-#undef FN
-#undef FNe
-#define FN(reason) { SKB_DROP_REASON_##reason, #reason },
-#define FNe(reason){ SKB_DROP_REASON_##reason, #reason }
+TRACE_DEFINE_SYM_FNS(drop_reason, drop_reason_lookup, drop_reason_show);
/*
* Tracepoint for free an sk_buff:
@@ -44,13 +38,9 @@ TRACE_EVENT(kfree_skb,
TP_printk("skbaddr=%p protocol=%u location=%pS reason: %s",
__entry->skbaddr, __entry->protocol, __entry->location,
- __print_symbolic(__entry->reason,
- DEFINE_DROP_REASON(FN, FNe)))
+ __print_sym(__entry->reason, drop_reason ))
);
-#undef FN
-#undef FNe
-
TRACE_EVENT(consume_skb,
TP_PROTO(struct sk_buff *skb, void *location),
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 466999a7515e..cd1ea6c3e0f8 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -145,6 +145,49 @@ drop_reasons_by_subsys[SKB_DROP_REASON_SUBSYS_NUM] = {
};
EXPORT_SYMBOL(drop_reasons_by_subsys);
+#ifdef CONFIG_TRACEPOINTS
+const char *drop_reason_lookup(unsigned long long value)
+{
+ unsigned long long subsys_id = value >> SKB_DROP_REASON_SUBSYS_SHIFT;
+ u32 reason = value & ~SKB_DROP_REASON_SUBSYS_MASK;
+ const struct drop_reason_list *subsys;
+
+ if (subsys_id >= SKB_DROP_REASON_SUBSYS_NUM)
+ return NULL;
+
+ subsys = rcu_dereference(drop_reasons_by_subsys[subsys_id]);
+ if (!subsys)
+ return NULL;
+ if (reason >= subsys->n_reasons)
+ return NULL;
+ return subsys->reasons[reason];
+}
+
+void drop_reason_show(struct seq_file *m)
+{
+ u32 subsys_id;
+
+ rcu_read_lock();
+ for (subsys_id = 0; subsys_id < SKB_DROP_REASON_SUBSYS_NUM;
subsys_id++) {
+ const struct drop_reason_list *subsys;
+ u32 i;
+
+ subsys = rcu_dereference(drop_reasons_by_subsys[subsys_id]);
+ if (!subsys)
+ continue;
+
+ for (i = 0; i < subsys->n_reasons; i++) {
+ if (!subsys->reasons[i])
+ continue;
+ seq_printf(m, ", { %u, \"%s\" }",
+ (subsys_id << SKB_DROP_REASON_SUBSYS_SHIFT)
| i,
+ subsys->reasons[i]);
+ }
+ }
+ rcu_read_unlock();
+}
+#endif
+
/**
* drop_reasons_register_subsys - register another drop reason subsystem
* @subsys: the subsystem to register, must not be the core
--
2.45.2
Re: [PATCH v6 3/5] dt-bindings: remoteproc: Add compatibility for TEE support
On Fri, 07 Jun 2024 11:33:24 +0200, Arnaud Pouliquen wrote: > 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 > --- > update vs previous version > - merge [PATCH v5 4/7] remoteproc: core introduce > rproc_set_rsc_table_on_start function > as new "st,proc-id" is associated to "st,stm32mp1-m4-tee" compatible > - update commit message > - remove Reviewed-by: Rob Herring as patch is updated > --- > .../bindings/remoteproc/st,stm32-rproc.yaml | 58 --- > 1 file changed, 50 insertions(+), 8 deletions(-) > Reviewed-by: Rob Herring (Arm)
Re: [PATCH] kallsyms, livepatch: Fix livepatch with CONFIG_LTO_CLANG
Hi Miroslav, Thanks for reviewing the patch! On Fri, Jun 7, 2024 at 6:06 AM Miroslav Benes wrote: > > Hi, > > On Tue, 4 Jun 2024, Song Liu wrote: > > > With CONFIG_LTO_CLANG, the compiler may postfix symbols with .llvm. > > to avoid symbol duplication. scripts/kallsyms.c sorted the symbols > > without these postfixes. The default symbol lookup also removes these > > postfixes before comparing symbols. > > > > On the other hand, livepatch need to look up symbols with the full names. > > However, calling kallsyms_on_each_match_symbol with full name (with the > > postfix) cannot find the symbol(s). As a result, we cannot livepatch > > kernel functions with .llvm. postfix or kernel functions that use > > relocation information to symbols with .llvm. postfixes. > > > > Fix this by calling kallsyms_on_each_match_symbol without the postfix; > > and then match the full name (with postfix) in klp_match_callback. > > > > Signed-off-by: Song Liu > > --- > > include/linux/kallsyms.h | 13 + > > kernel/kallsyms.c| 21 - > > kernel/livepatch/core.c | 32 +++- > > 3 files changed, 60 insertions(+), 6 deletions(-) > > I do not like much that something which seems to be kallsyms-internal is > leaked out. You need to export cleanup_symbol_name() and there is now a > lot of code outside. I would feel much more comfortable if it is all > hidden from kallsyms users and kept there. Would it be possible? I think it is possible. Currently, kallsyms_on_each_match_symbol matches symbols without the postfix. We can add a variation or a parameter, so that it matches the full name with post fix. > Moreover, isn't there a similar problem for ftrace, kprobes, ebpf,...? Yes, there is a similar problem with tracing use cases. But the requirements are not the same: For livepatch, we have to point to the exact symbol we want to patch or relocation to. We have sympos API defined to differentiate different symbols with the same name. For tracing, some discrepancy is acceptable. AFAICT, there isn't an API similar to sympos yet. Also, we can play some tricks with tracing. For example, we can use "uniq symbol + offset" to point a kprobe to one of the duplicated symbols. Given livepatch has a well defined API, while the APIs at tracing side may still change, we can change kallsyms to make sure livepatch side works. Work on the tracing side can wait. Thanks, Song
Re: [PATCH] livepatch: introduce klp_func called interface
Hi Miroslav,
On Fri, Jun 7, 2024 at 2:07 AM Miroslav Benes wrote:
>
> Hi,
>
> On Tue, 4 Jun 2024, Song Liu wrote:
>
> > On Tue, May 21, 2024 at 1:04 AM Petr Mladek wrote:
> > [...]
> > > >
> > > > Yes, but the information you get is limited compared to what is
> > > > available
> > > > now. You would obtain the information that a patched function was called
> > > > but ftrace could also give you the context and more.
> > >
> > > Another motivation to use ftrace for testing is that it does not
> > > affect the performance in production.
> > >
> > > We should keep klp_ftrace_handler() as fast as possible so that we
> > > could livepatch also performance sensitive functions.
> >
> > At LPC last year, we discussed about adding a counter to each
> > klp_func, like:
> >
> > struct klp_func {
> > ...
> > u64 __percpu counter;
> > ...
> > };
> >
> > With some static_key (+ sysctl), this should give us a way to estimate
> > the overhead of livepatch. If we have the counter, this patch is not
> > needed any more. Does this (adding the counter) sound like
> > something we still want to pursue?
>
> It would be better than this patch but given what was mentioned in the
> thread I wonder if it is possible to use ftrace even for this. See
> /sys/kernel/tracing/trace_stat/function*. It already gathers the number of
> hits.
I didn't know about the trace_stat API until today. :) It somehow doesn't
exist on some older kernels. (I haven't debugged it.)
> Would it be sufficient for you? I guess it depends on what the intention
> is. If there is no time limit, klp_func.counter might be better to provide
> some kind of overall statistics (but I am not sure if it has any value)
> and to avoid having ftrace registered on a live patched function for
> infinite period of time. If the intention is to gather data for some
> limited period, trace_stat sounds like much better approach to me.
We don't have very urgent use for this. As we discussed, various tracing
tools are sufficient in most cases. I brought this up in the context of the
"called" entry: if we are really adding a new entry, let's do "counter"
instead of "called".
Thanks,
Song
Re: [PATCH v3 2/2] x86/sgx: Resolve EREMOVE page vs EAUG page data race
On Tue, May 28, 2024 at 09:23:13AM -0700, Dave Hansen wrote:
> On 5/17/24 04:06, Dmitrii Kuvaiskii wrote:
> ...
>
> First, why is SGX so special here? How is the SGX problem different
> than what the core mm code does?
Here is my understanding why SGX is so special and why I have to introduce
a new bit SGX_ENCL_PAGE_BEING_REMOVED.
In SGX's removal of the enclave page, two operations must happen
atomically: the PTE entry must be removed and the page must be EREMOVE'd.
Generally, to guarantee atomicity, encl->lock is acquired. Ideally, if
this encl->lock could be acquired at the beginning of
sgx_encl_remove_pages() and be released at the very end of this function,
there would be no EREMOVE page vs EAUG page data race, and my bug fix
(with SGX_ENCL_PAGE_BEING_REMOVED bit) wouldn't be needed.
However, the current implementation of sgx_encl_remove_pages() has to
release encl->lock before removing the PTE entry. Releasing the lock is
required because the function that removes the PTE entry --
sgx_zap_enclave_ptes() -- acquires another, enclave-MM lock:
mmap_read_lock(encl_mm->mm).
The two locks must be taken in this order:
1. mmap_read_lock(encl_mm->mm)
2. mutex_lock(&encl->lock)
This lock order is apparent from e.g. sgx_encl_add_page(). This order also
seems to make intuitive sense: VMA callbacks are called with the MM lock
being held, so the MM lock should be the first in lock order.
So, if sgx_encl_remove_pages() would _not_ release encl->lock before
calling sgx_zap_enclave_ptes(), this would violate the lock order and
might lead to deadlocks. At the same time, releasing encl->lock in the
middle of the two-operations flow leads to a data race that I found in
this patch series.
Quick summary:
- Removing the enclave page requires two operations: removing the PTE and
performing EREMOVE.
- The complete flow of removing the enclave page cannot be protected by a
single encl->lock, because it would violate the lock order and would
lead to deadlocks.
- The current upstream implementation thus breaks the flow into two
critical sections, releasing encl->lock before sgx_zap_enclave_ptes()
and re-acquiring this lock afterwards. This leads to a data race.
- My patch restores "atomicity" of the flow by introducing a new flag
SGX_ENCL_PAGE_BEING_REMOVED.
> > --- a/arch/x86/kernel/cpu/sgx/encl.h
> > +++ b/arch/x86/kernel/cpu/sgx/encl.h
> > @@ -25,6 +25,9 @@
> > /* 'desc' bit marking that the page is being reclaimed. */
> > #define SGX_ENCL_PAGE_BEING_RECLAIMED BIT(3)
> >
> > +/* 'desc' bit marking that the page is being removed. */
> > +#define SGX_ENCL_PAGE_BEING_REMOVEDBIT(2)
>
> Second, convince me that this _needs_ a new bit. Why can't we just have
> a bit that effectively means "return EBUSY if you see this bit when
> handling a fault".
As Haitao mentioned in his reply, the bit SGX_ENCL_PAGE_BEING_RECLAIMED is
also used in reclaimer_writing_to_pcmd(). If we would re-use this bit to
mark a page being removed, reclaimer_writing_to_pcmd() would incorrectly
return 1, meaning that the reclaimer is about to write to the PCMD page,
which is not true.
> > struct sgx_encl_page {
> > unsigned long desc;
> > unsigned long vm_max_prot_bits:8;
> > diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c
> > b/arch/x86/kernel/cpu/sgx/ioctl.c
> > index 5d390df21440..de59219ae794 100644
> > --- a/arch/x86/kernel/cpu/sgx/ioctl.c
> > +++ b/arch/x86/kernel/cpu/sgx/ioctl.c
> > @@ -1142,6 +1142,7 @@ static long sgx_encl_remove_pages(struct sgx_encl
> > *encl,
> > * Do not keep encl->lock because of dependency on
> > * mmap_lock acquired in sgx_zap_enclave_ptes().
> > */
> > + entry->desc |= SGX_ENCL_PAGE_BEING_REMOVED;
>
> This also needs a comment, no matter what.
Ok, I will write something along the lines that we want to prevent a data
race with an EAUG flow, and since we have to release encl->lock (which
would otherwise prevent the data race) we instead set a bit to mark this
enclave page as being in the process of removal, so that the EAUG flow
backs off and retries later.
--
Dmitrii Kuvaiskii
Re: [PATCH v3 2/2] x86/sgx: Resolve EREMOVE page vs EAUG page data race
On 6/3/24 11:42, Haitao Huang wrote: >> Second, convince me that this _needs_ a new bit. Why can't we just have >> a bit that effectively means "return EBUSY if you see this bit when >> handling a fault". > > IIUC, reclaimer_writing_to_pcmd() also uses > SGX_ENCL_PAGE_BEING_RECLAIMED to check if a page is about being > reclaimed in order to prevent its VA slot fro being freed. So I think we > do need separate bit for EREMOVE which does not write to VA slot? I think the bits should be centered around what action the code needs to take and not what is being done to the page. Right now, SGX_ENCL_PAGE_BEING_RECLAIMED has two logical meanings: 1. Don't load the page 2. The page is in the backing store But now folks are suggesting that a new bit is added which means "do #1, but not #2". Let's take a step back and look at what logical outcomes we want in the code and then create the bits based on _that_.
Re: [PATCH v3 0/2] x86/sgx: Fix two data races in EAUG/EREMOVE flows
On Tue, May 28, 2024 at 09:01:10AM -0700, Dave Hansen wrote: > On 5/17/24 04:06, Dmitrii Kuvaiskii wrote: > > We wrote a trivial stress test to reproduce the hangs observed in > > real-world applications. The test stresses #PF-based page allocation and > > SGX_IOC_ENCLAVE_REMOVE_PAGES flows in the SGX driver: > > This seems like something we'd want in the kernel SGX selftests. I looked at tools/testing/selftests/sgx/ and I observe several complications: 1. The stress test requires creation of several threads (at least two, ideally more). However, current SGX selftests are single-threaded. Adding the scaffolding to add multi-threading support to SGX selftests seems like a non-trivial task. 2. Catching the data race would require a for loop with some threshold. - First, there are no such looping tests in current SGX selftests. Is it normal to add such a test? - Second, what would be the threshold to loop for? I.e., after how many iterations should we consider the data race not manifesting, and report success? - Third, the data race may hang the test. Is this something that is allowed in selftests? (I mean the test can have only two outcomes -- either it hangs, meaning the data race was not fixed, or it runs to completion. There is no result that we could EXCEPT or ASSERT on.) Do we still want to add such a selftest? Or could we maybe piggy-back on Gramine CI (that will include the test I mentioned in the cover letter)? -- Dmitrii Kuvaiskii
Re: [PATCH v8 4/5] soc: qcom: add pd-mapper implementation
On 5/11/2024 2:56 PM, Dmitry Baryshkov wrote: Existing userspace protection domain mapper implementation has several issue. It doesn't play well with CONFIG_EXTRA_FIRMWARE, it doesn't reread JSON files if firmware location is changed (or if firmware was not available at the time pd-mapper was started but the corresponding directory is mounted later), etc. Provide in-kernel service implementing protection domain mapping required to work with several services, which are provided by the DSP firmware. This module is loaded automatically by the remoteproc drivers when necessary via the symbol dependency. It uses a root node to match a protection domains map for a particular board. It is not possible to implement it as a 'driver' as there is no corresponding device. Tested-by: Steev Klimaszewski Tested-by: Alexey Minnekhanov Signed-off-by: Dmitry Baryshkov --- drivers/soc/qcom/Kconfig | 11 + drivers/soc/qcom/Makefile | 1 + drivers/soc/qcom/pdr_internal.h | 14 + drivers/soc/qcom/qcom_pd_mapper.c | 676 ++ drivers/soc/qcom/qcom_pdr_msg.c | 34 ++ 5 files changed, 736 insertions(+) Reviewed-by: Chris Lew
Re: [PATCH v8 5/5] remoteproc: qcom: enable in-kernel PD mapper
On 5/11/2024 2:56 PM, Dmitry Baryshkov wrote: Request in-kernel protection domain mapper to be started before starting Qualcomm DSP and release it once DSP is stopped. Once all DSPs are stopped, the PD mapper will be stopped too. Signed-off-by: Dmitry Baryshkov --- drivers/remoteproc/qcom_common.c| 87 + drivers/remoteproc/qcom_common.h| 10 + drivers/remoteproc/qcom_q6v5_adsp.c | 3 ++ drivers/remoteproc/qcom_q6v5_mss.c | 3 ++ drivers/remoteproc/qcom_q6v5_pas.c | 3 ++ drivers/remoteproc/qcom_q6v5_wcss.c | 3 ++ 6 files changed, 109 insertions(+) Thanks for looking into whether this could be implemented as a remoteproc subdevice. Reviewed-by: Chris Lew
Re: [PATCH v6 5/5] remoteproc: stm32: Add support of an OP-TEE TA to load the firmware
Hi Arnaud,
kernel test robot noticed the following build errors:
[auto build test ERROR on 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0]
url:
https://github.com/intel-lab-lkp/linux/commits/Arnaud-Pouliquen/remoteproc-core-Introduce-rproc_pa_to_va-helper/20240607-183305
base: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0
patch link:
https://lore.kernel.org/r/20240607093326.369090-6-arnaud.pouliquen%40foss.st.com
patch subject: [PATCH v6 5/5] remoteproc: stm32: Add support of an OP-TEE TA to
load the firmware
config: i386-buildonly-randconfig-002-20240608
(https://download.01.org/0day-ci/archive/20240608/202406081159.km501g5c-...@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project
617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build):
(https://download.01.org/0day-ci/archive/20240608/202406081159.km501g5c-...@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot
| Closes:
https://lore.kernel.org/oe-kbuild-all/202406081159.km501g5c-...@intel.com/
All errors (new ones prefixed by >>):
In file included from drivers/remoteproc/stm32_rproc.c:23:
>> include/linux/tee_remoteproc.h:94:2: error: expected parameter declarator
94 | WARN_ON(1);
| ^
include/asm-generic/bug.h:122:29: note: expanded from macro 'WARN_ON'
122 | #define WARN_ON(condition) ({
\
| ^
In file included from drivers/remoteproc/stm32_rproc.c:23:
>> include/linux/tee_remoteproc.h:94:2: error: expected ')'
include/asm-generic/bug.h:122:29: note: expanded from macro 'WARN_ON'
122 | #define WARN_ON(condition) ({
\
| ^
include/linux/tee_remoteproc.h:94:2: note: to match this '('
include/asm-generic/bug.h:122:28: note: expanded from macro 'WARN_ON'
122 | #define WARN_ON(condition) ({
\
|^
In file included from drivers/remoteproc/stm32_rproc.c:23:
>> include/linux/tee_remoteproc.h:92:32: error: function cannot return function
>> type 'struct resource_table *()'
92 | tee_rproc_find_loaded_rsc_table(struct rproc *rproc, const struct
firmware *fw)
|^
>> include/linux/tee_remoteproc.h:94:2: error: a function declaration without a
>> prototype is deprecated in all versions of C [-Werror,-Wstrict-prototypes]
94 | WARN_ON(1);
| ^
include/asm-generic/bug.h:122:28: note: expanded from macro 'WARN_ON'
122 | #define WARN_ON(condition) ({
\
|^
In file included from drivers/remoteproc/stm32_rproc.c:23:
>> include/linux/tee_remoteproc.h:96:2: error: expected identifier or '('
96 | return NULL;
| ^
>> include/linux/tee_remoteproc.h:97:1: error: extraneous closing brace ('}')
97 | }
| ^
6 errors generated.
vim +94 include/linux/tee_remoteproc.h
5c0eb7b2737b6e Arnaud Pouliquen 2024-06-07 90
5c0eb7b2737b6e Arnaud Pouliquen 2024-06-07 91 static inline struct
resource_table *
5c0eb7b2737b6e Arnaud Pouliquen 2024-06-07 @92
tee_rproc_find_loaded_rsc_table(struct rproc *rproc, const struct firmware *fw)
5c0eb7b2737b6e Arnaud Pouliquen 2024-06-07 93 /* This shouldn't be
possible */
5c0eb7b2737b6e Arnaud Pouliquen 2024-06-07 @94 WARN_ON(1);
5c0eb7b2737b6e Arnaud Pouliquen 2024-06-07 95
5c0eb7b2737b6e Arnaud Pouliquen 2024-06-07 @96 return NULL;
5c0eb7b2737b6e Arnaud Pouliquen 2024-06-07 @97 }
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH v4 1/4] tracing: add __print_sym() to replace __print_symbolic()
Hi Johannes,
kernel test robot noticed the following build warnings:
[auto build test WARNING on mcgrof/modules-next]
[also build test WARNING on arnd-asm-generic/master tip/timers/core net/main
net-next/main linus/master horms-ipvs/master v6.10-rc2 next-20240607]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url:
https://github.com/intel-lab-lkp/linux/commits/Johannes-Berg/tracing-add-__print_sym-to-replace-__print_symbolic/20240608-000918
base: https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux.git
modules-next
patch link:
https://lore.kernel.org/r/20240607160527.23624-7-johannes%40sipsolutions.net
patch subject: [PATCH v4 1/4] tracing: add __print_sym() to replace
__print_symbolic()
config: i386-buildonly-randconfig-005-20240608
(https://download.01.org/0day-ci/archive/20240608/202406081255.2feqdvbk-...@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project
617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build):
(https://download.01.org/0day-ci/archive/20240608/202406081255.2feqdvbk-...@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot
| Closes:
https://lore.kernel.org/oe-kbuild-all/202406081255.2feqdvbk-...@intel.com/
All warnings (new ones prefixed by >>):
>> kernel/trace/trace_events.c:1583:6: warning: variable 'n_sym_defs' is used
>> uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
1583 | if ((call->flags & TRACE_EVENT_FL_DYNAMIC) || !call->module)
{
| ^~~
kernel/trace/trace_events.c:1595:18: note: uninitialized use occurs here
1595 | for (i = 0; i < n_sym_defs; i++) {
| ^~
kernel/trace/trace_events.c:1583:2: note: remove the 'if' if its condition
is always true
1583 | if ((call->flags & TRACE_EVENT_FL_DYNAMIC) || !call->module)
{
| ^~~~
kernel/trace/trace_events.c:1581:25: note: initialize the variable
'n_sym_defs' to silence this warning
1581 | unsigned int n_sym_defs, i;
|^
| = 0
1 warning generated.
vim +1583 kernel/trace/trace_events.c
1575
1576 /* note: @name is not NUL-terminated */
1577 static void show_sym_list(struct seq_file *m, struct trace_event_call
*call,
1578const char *name, unsigned int name_len)
1579 {
1580 struct trace_sym_def **sym_defs;
1581 unsigned int n_sym_defs, i;
1582
> 1583 if ((call->flags & TRACE_EVENT_FL_DYNAMIC) || !call->module) {
1584 sym_defs = __start_ftrace_sym_defs;
1585 n_sym_defs = __stop_ftrace_sym_defs -
__start_ftrace_sym_defs;
1586 #ifdef CONFIG_MODULES
1587 } else {
1588 struct module *mod = call->module;
1589
1590 sym_defs = mod->trace_sym_defs;
1591 n_sym_defs = mod->num_trace_sym_defs;
1592 #endif /* CONFIG_MODULES */
1593 }
1594
1595 for (i = 0; i < n_sym_defs; i++) {
1596 unsigned int sym_len;
1597
1598 if (!sym_defs[i])
1599 continue;
1600 if (sym_defs[i]->system != call->class->system)
1601 continue;
1602 sym_len = strlen(sym_defs[i]->symbol_id);
1603 if (name_len != sym_len)
1604 continue;
1605 if (strncmp(sym_defs[i]->symbol_id, name, sym_len))
1606 continue;
1607 if (sym_defs[i]->show)
1608 sym_defs[i]->show(m);
1609 break;
1610 }
1611 }
1612
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH v6 5/5] remoteproc: stm32: Add support of an OP-TEE TA to load the firmware
Hi Arnaud,
kernel test robot noticed the following build errors:
[auto build test ERROR on 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0]
url:
https://github.com/intel-lab-lkp/linux/commits/Arnaud-Pouliquen/remoteproc-core-Introduce-rproc_pa_to_va-helper/20240607-183305
base: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0
patch link:
https://lore.kernel.org/r/20240607093326.369090-6-arnaud.pouliquen%40foss.st.com
patch subject: [PATCH v6 5/5] remoteproc: stm32: Add support of an OP-TEE TA to
load the firmware
config: s390-allyesconfig
(https://download.01.org/0day-ci/archive/20240608/202406081214.qfail90a-...@intel.com/config)
compiler: s390-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build):
(https://download.01.org/0day-ci/archive/20240608/202406081214.qfail90a-...@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot
| Closes:
https://lore.kernel.org/oe-kbuild-all/202406081214.qfail90a-...@intel.com/
All errors (new ones prefixed by >>):
In file included from include/linux/bug.h:5,
from include/linux/fortify-string.h:6,
from include/linux/string.h:374,
from include/linux/dma-mapping.h:7,
from drivers/remoteproc/stm32_rproc.c:9:
>> arch/s390/include/asm/bug.h:53:21: error: expected declaration specifiers or
>> '...' before '{' token
53 | #define WARN_ON(x) ({ \
| ^
include/linux/tee_remoteproc.h:94:9: note: in expansion of macro 'WARN_ON'
94 | WARN_ON(1);
| ^~~
In file included from drivers/remoteproc/stm32_rproc.c:23:
include/linux/tee_remoteproc.h:96:9: error: expected identifier or '('
before 'return'
96 | return NULL;
| ^~
include/linux/tee_remoteproc.h:97:1: error: expected identifier or '('
before '}' token
97 | }
| ^
drivers/remoteproc/stm32_rproc.c:717:34: error:
'tee_rproc_find_loaded_rsc_table' undeclared here (not in a function); did you
mean 'rproc_find_loaded_rsc_table'?
717 | .find_loaded_rsc_table = tee_rproc_find_loaded_rsc_table,
| ^~~
| rproc_find_loaded_rsc_table
vim +53 arch/s390/include/asm/bug.h
a9df8e325d0de5 arch/s390/include/asm/bug.h Heiko Carstens 2010-01-13 52
c0007f1a65762e include/asm-s390/bug.h Heiko Carstens 2007-04-27 @53
#define WARN_ON(x) ({ \
fd0cbdd378258f include/asm-s390/bug.h Heiko Carstens 2007-08-02 54
int __ret_warn_on = !!(x); \
c0007f1a65762e include/asm-s390/bug.h Heiko Carstens 2007-04-27 55
if (__builtin_constant_p(__ret_warn_on)) { \
c0007f1a65762e include/asm-s390/bug.h Heiko Carstens 2007-04-27 56
if (__ret_warn_on) \
b2be05273a1744 arch/s390/include/asm/bug.h Ben Hutchings 2010-04-03 57
__WARN(); \
c0007f1a65762e include/asm-s390/bug.h Heiko Carstens 2007-04-27 58
} else {\
c0007f1a65762e include/asm-s390/bug.h Heiko Carstens 2007-04-27 59
if (unlikely(__ret_warn_on))\
b2be05273a1744 arch/s390/include/asm/bug.h Ben Hutchings 2010-04-03 60
__WARN(); \
c0007f1a65762e include/asm-s390/bug.h Heiko Carstens 2007-04-27 61
} \
c0007f1a65762e include/asm-s390/bug.h Heiko Carstens 2007-04-27 62
unlikely(__ret_warn_on);\
c0007f1a65762e include/asm-s390/bug.h Heiko Carstens 2007-04-27 63 })
c0007f1a65762e include/asm-s390/bug.h Heiko Carstens 2007-04-27 64
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: Current status and possible improvements in CONFIG_MODULE_FORCE_UNLOAD
On Thu, Jun 06, 2024 at 06:49:59AM +, Aditya Garg wrote: > Hi > > I am Aditya Garg. I often require using out of tree drivers to support > various hardwares on Linux. Just stop buying hardwarew that requires this, or improve and upstream the drivers to make your life easier instead of making other peoples life worse.
