Re: [PATCH 10/20] function_graph: Have the instances use their own ftrace_ops for filtering
On Thu, 30 May 2024 22:30:57 -0400
Steven Rostedt wrote:
> On Fri, 24 May 2024 22:37:02 -0400
> Steven Rostedt wrote:
>
> > From: "Steven Rostedt (VMware)"
> >
> > Allow for instances to have their own ftrace_ops part of the fgraph_ops
> > that makes the funtion_graph tracer filter on the set_ftrace_filter file
> > of the instance and not the top instance.
> >
> > Note that this also requires to update ftrace_graph_func() to call new
> > function_graph_enter_ops() instead of function_graph_enter() so that
> > it avoid pushing on shadow stack multiple times on the same function.
>
> So I found a major design flaw in this patch.
>
> >
> > Co-developed with Masami Hiramatsu:
> > Link:
> > https://lore.kernel.org/linux-trace-kernel/171509102088.162236.15758883237657317789.stgit@devnote2
> >
> > Signed-off-by: Steven Rostedt (VMware)
> > Signed-off-by: Masami Hiramatsu (Google)
> > Signed-off-by: Steven Rostedt (Google)
> > ---
>
> > diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
> > index 8da0e66ca22d..998558cb8f15 100644
> > --- a/arch/x86/kernel/ftrace.c
> > +++ b/arch/x86/kernel/ftrace.c
> > @@ -648,9 +648,24 @@ void ftrace_graph_func(unsigned long ip, unsigned long
> > parent_ip,
> >struct ftrace_ops *op, struct ftrace_regs *fregs)
> > {
> > struct pt_regs *regs = >regs;
> > - unsigned long *stack = (unsigned long *)kernel_stack_pointer(regs);
> > + unsigned long *parent = (unsigned long *)kernel_stack_pointer(regs);
> > + struct fgraph_ops *gops = container_of(op, struct fgraph_ops, ops);
> > + int bit;
> > +
> > + if (unlikely(ftrace_graph_is_dead()))
> > + return;
> > +
> > + if (unlikely(atomic_read(>tracing_graph_pause)))
> > + return;
> >
> > - prepare_ftrace_return(ip, (unsigned long *)stack, 0);
> > + bit = ftrace_test_recursion_trylock(ip, *parent);
> > + if (bit < 0)
> > + return;
> > +
> > + if (!function_graph_enter_ops(*parent, ip, 0, parent, gops))
>
> So each registered graph ops has its own ftrace_ops which gets
> registered with ftrace, so this function does get called in a loop (by
> the ftrace iterator function). This means that we would need that code
> to detect the function_graph_enter_ops() getting called multiple times
> for the same function. This means each fgraph_ops gits its own retstack
> on the shadow stack.
Ah, that is my concern and the reason why I added bitmap and stack reuse
code in the ftrace_push_return_trace().
>
> I find this a waste of shadow stack resources, and also complicates the
> code with having to deal with tail calls and all that.
>
> BUT! There's good news! I also thought about another way of handling
> this. I have something working, but requires a bit of rewriting the
> code. I should have something out in a day or two.
Hmm, I just wonder why you don't reocver my bitmap check and stack
reusing code. Are there any problem on it? (Too complicated?)
Thanks,
>
> -- Steve
>
>
> > + *parent = (unsigned long)_to_handler;
> > +
> > + ftrace_test_recursion_unlock(bit);
> > }
> > #endif
> >
>
--
Masami Hiramatsu (Google)
Re: [PATCH 0/3] tracing: Fix some selftest issues
On Wed, 29 May 2024 11:01:43 -0500 Tom Zanussi wrote: > Hi Masami, > > On Wed, 2024-05-29 at 08:38 +0900, Masami Hiramatsu wrote: > > On Wed, 29 May 2024 01:46:40 +0900 > > Masami Hiramatsu (Google) wrote: > > > > > On Mon, 27 May 2024 19:29:07 -0400 > > > Steven Rostedt wrote: > > > > > > > On Sun, 26 May 2024 19:10:57 +0900 > > > > "Masami Hiramatsu (Google)" wrote: > > > > > > > > > Hi, > > > > > > > > > > Here is a series of some fixes/improvements for the test > > > > > modules and boot > > > > > time selftest of kprobe events. I found a WARNING message with > > > > > some boot > > > > > time selftest configuration, which came from the combination of > > > > > embedded > > > > > kprobe generate API tests module and ftrace boot-time selftest. > > > > > So the main > > > > > problem is that the test module should not be built-in. But I > > > > > also think > > > > > this WARNING message is useless (because there are warning > > > > > messages already) > > > > > and the cleanup code is redundant. This series fixes those > > > > > issues. > > > > > > > > Note, when I enable trace tests as builtin instead of modules, I > > > > just > > > > disable the bootup self tests when it detects this. This helps > > > > with > > > > doing tests via config options than having to add user space code > > > > that > > > > loads modules. > > > > > > > > Could you do something similar? > > > > > > OK, in that case, I would like to move the test cleanup code in > > > module_exit function into the end of module_init function. > > > It looks there is no reason to split those into 2 parts. > > > > Wait, I would like to hear Tom's opinion. I found following usage > > comments > > in the code. > > > > * Following that are a few examples using the created events to test > > * various ways of tracing a synthetic event. > > * > > * To test, select CONFIG_SYNTH_EVENT_GEN_TEST and build the module. > > * Then: > > * > > * # insmod kernel/trace/synth_event_gen_test.ko > > * # cat /sys/kernel/tracing/trace > > * > > * You should see several events in the trace buffer - > > * "create_synth_test", "empty_synth_test", and several instances of > > * "gen_synth_test". > > * > > * To remove the events, remove the module: > > * > > * # rmmod synth_event_gen_test > > > > Tom, is that intended behavior ? and are you expected to reuse these > > events outside of the module? e.g. load the test module and run some > > test script in user space which uses those events? > > > > Yeah, this module was meant as a sample module showing how to create > and generate synthetic events in-kernel. > > So the interested user insmods the module, looks at the trace stream > and sees, ok the events are there as expected, so it does work, great, > let's remove the module to get rid of them and go write our own. > > Having both the creation and cleanup in module_init() wouldn't allow > the user the opportunity to do that i.e. verify the results by reading > the trace file. So, in summary, it is designed to be a module. Steve, I think these tests should be kept as modules. There are many reason to do so. - This test is designed to be used as module. - This can conflict with other boot time selftest if it is embedded. - We can make these tests and boot time selftest mutable exclusive but if we make these tests as modules, we can build and run both tests safely. - Embedding these tests leave new events when the kernel boot, which user must be cleaned up by manual. What would you think? Thank you, > > Tom > > > As far as I can see, those tests are not intended to be embedded in > > the > > kernel because those are expected to be removed. > > > > Thank you, > > > > > > > > Thank you, > > > > > > > > > > > -- Steve > > > > > > > > > > > > > > > > > > Thank you, > > > > > > > > > > --- > > > > > > > > > > Masami Hiramatsu (Google) (3): > > > > > tracing: Build event generation tests only as modules > > > > > tracing/kprobe: Remove unneeded WARN_ON_ONCE() in > > > > > selftests > > > > > tracing/kprobe: Remove cleanup code unrelated to selftest > > > > > > > > > > > > > > -- > > > Masami Hiramatsu (Google) > > > > > > -- Masami Hiramatsu (Google)
Re: [PATCH 10/20] function_graph: Have the instances use their own ftrace_ops for filtering
On Fri, 24 May 2024 22:37:02 -0400
Steven Rostedt wrote:
> From: "Steven Rostedt (VMware)"
>
> Allow for instances to have their own ftrace_ops part of the fgraph_ops
> that makes the funtion_graph tracer filter on the set_ftrace_filter file
> of the instance and not the top instance.
>
> Note that this also requires to update ftrace_graph_func() to call new
> function_graph_enter_ops() instead of function_graph_enter() so that
> it avoid pushing on shadow stack multiple times on the same function.
So I found a major design flaw in this patch.
>
> Co-developed with Masami Hiramatsu:
> Link:
> https://lore.kernel.org/linux-trace-kernel/171509102088.162236.15758883237657317789.stgit@devnote2
>
> Signed-off-by: Steven Rostedt (VMware)
> Signed-off-by: Masami Hiramatsu (Google)
> Signed-off-by: Steven Rostedt (Google)
> ---
> diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
> index 8da0e66ca22d..998558cb8f15 100644
> --- a/arch/x86/kernel/ftrace.c
> +++ b/arch/x86/kernel/ftrace.c
> @@ -648,9 +648,24 @@ void ftrace_graph_func(unsigned long ip, unsigned long
> parent_ip,
> struct ftrace_ops *op, struct ftrace_regs *fregs)
> {
> struct pt_regs *regs = >regs;
> - unsigned long *stack = (unsigned long *)kernel_stack_pointer(regs);
> + unsigned long *parent = (unsigned long *)kernel_stack_pointer(regs);
> + struct fgraph_ops *gops = container_of(op, struct fgraph_ops, ops);
> + int bit;
> +
> + if (unlikely(ftrace_graph_is_dead()))
> + return;
> +
> + if (unlikely(atomic_read(>tracing_graph_pause)))
> + return;
>
> - prepare_ftrace_return(ip, (unsigned long *)stack, 0);
> + bit = ftrace_test_recursion_trylock(ip, *parent);
> + if (bit < 0)
> + return;
> +
> + if (!function_graph_enter_ops(*parent, ip, 0, parent, gops))
So each registered graph ops has its own ftrace_ops which gets
registered with ftrace, so this function does get called in a loop (by
the ftrace iterator function). This means that we would need that code
to detect the function_graph_enter_ops() getting called multiple times
for the same function. This means each fgraph_ops gits its own retstack
on the shadow stack.
I find this a waste of shadow stack resources, and also complicates the
code with having to deal with tail calls and all that.
BUT! There's good news! I also thought about another way of handling
this. I have something working, but requires a bit of rewriting the
code. I should have something out in a day or two.
-- Steve
> + *parent = (unsigned long)_to_handler;
> +
> + ftrace_test_recursion_unlock(bit);
> }
> #endif
>
Re: [PATCH] livepatch: introduce klp_func called interface
Hi Bros, How about my patch? I do think it is a viable feature to show the state of the patched function. If we add an unlikely branch test before we set the 'called' state, once this function is called, there maybe no negative effect to the performance. Please give me some advice. Regards, Wardenjohn
Re: [PATCH net-next V2] virtio-net: synchronize operstate with admin state on up/down
On Thu, May 30, 2024 at 9:09 PM Michael S. Tsirkin wrote:
>
> On Thu, May 30, 2024 at 06:29:51PM +0800, Jason Wang wrote:
> > On Thu, May 30, 2024 at 2:10 PM Michael S. Tsirkin wrote:
> > >
> > > On Thu, May 30, 2024 at 11:20:55AM +0800, Jason Wang wrote:
> > > > This patch synchronize operstate with admin state per RFC2863.
> > > >
> > > > This is done by trying to toggle the carrier upon open/close and
> > > > synchronize with the config change work. This allows propagate status
> > > > correctly to stacked devices like:
> > > >
> > > > ip link add link enp0s3 macvlan0 type macvlan
> > > > ip link set link enp0s3 down
> > > > ip link show
> > > >
> > > > Before this patch:
> > > >
> > > > 3: enp0s3: mtu 1500 qdisc pfifo_fast state DOWN
> > > > mode DEFAULT group default qlen 1000
> > > > link/ether 00:00:05:00:00:09 brd ff:ff:ff:ff:ff:ff
> > > > ..
> > > > 5: macvlan0@enp0s3: mtu 1500
> > > > qdisc noqueue state UP mode DEFAULT group default qlen 1000
> > > > link/ether b2:a9:c5:04:da:53 brd ff:ff:ff:ff:ff:ff
> > > >
> > > > After this patch:
> > > >
> > > > 3: enp0s3: mtu 1500 qdisc pfifo_fast state DOWN
> > > > mode DEFAULT group default qlen 1000
> > > > link/ether 00:00:05:00:00:09 brd ff:ff:ff:ff:ff:ff
> > > > ...
> > > > 5: macvlan0@enp0s3: mtu 1500
> > > > qdisc noqueue state LOWERLAYERDOWN mode DEFAULT group default qlen 1000
> > > > link/ether b2:a9:c5:04:da:53 brd ff:ff:ff:ff:ff:ff
> > > >
> > > > Cc: Venkat Venkatsubra
> > > > Cc: Gia-Khanh Nguyen
> > > > Reviewed-by: Xuan Zhuo
> > > > Acked-by: Michael S. Tsirkin
> > > > Signed-off-by: Jason Wang
> > > > ---
> > > > Changes since V1:
> > > > - rebase
> > > > - add ack/review tags
> > >
> > >
> > >
> > >
> > >
> > > > ---
> > > > drivers/net/virtio_net.c | 94 +++-
> > > > 1 file changed, 63 insertions(+), 31 deletions(-)
> > > >
> > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > > index 4a802c0ea2cb..69e4ae353c51 100644
> > > > --- a/drivers/net/virtio_net.c
> > > > +++ b/drivers/net/virtio_net.c
> > > > @@ -433,6 +433,12 @@ struct virtnet_info {
> > > > /* The lock to synchronize the access to refill_enabled */
> > > > spinlock_t refill_lock;
> > > >
> > > > + /* Is config change enabled? */
> > > > + bool config_change_enabled;
> > > > +
> > > > + /* The lock to synchronize the access to config_change_enabled */
> > > > + spinlock_t config_change_lock;
> > > > +
> > > > /* Work struct for config space updates */
> > > > struct work_struct config_work;
> > > >
> > >
> > >
> > > But we already have dev->config_lock and dev->config_enabled.
> > >
> > > And it actually works better - instead of discarding config
> > > change events it defers them until enabled.
> > >
> >
> > Yes but then both virtio-net driver and virtio core can ask to enable
> > and disable and then we need some kind of synchronization which is
> > non-trivial.
>
> Well for core it happens on bring up path before driver works
> and later on tear down after it is gone.
> So I do not think they ever do it at the same time.
For example, there could be a suspend/resume when the admin state is down.
>
>
> > And device enabling on the core is different from bringing the device
> > up in the networking subsystem. Here we just delay to deal with the
> > config change interrupt on ndo_open(). (E.g try to ack announce is
> > meaningless when the device is down).
> >
> > Thanks
>
> another thing is that it is better not to re-read all config
> on link up if there was no config interrupt - less vm exits.
Yes, but it should not matter much as it's done in the ndo_open().
Thanks
>
> --
> MST
>
Re: [PATCHv6 bpf-next 1/9] x86/shstk: Make return uprobe work with shadow stack
On Tue, 2024-05-21 at 12:48 +0200, Jiri Olsa wrote:
> Currently the application with enabled shadow stack will crash
> if it sets up return uprobe. The reason is the uretprobe kernel
> code changes the user space task's stack, but does not update
> shadow stack accordingly.
>
> Adding new functions to update values on shadow stack and using
> them in uprobe code to keep shadow stack in sync with uretprobe
> changes to user stack.
>
> Fixes: 8b1c23543436 ("x86/shstk: Add return uprobe support")
> Signed-off-by: Jiri Olsa
> ---
Acked-by: Rick Edgecombe
Re: [PATCH v8 0/5] soc: qcom: add in-kernel pd-mapper implementation
I've tested this applied on top of kernel 6.8.11 on an X13s over the past week and it's been working well. -- classabbyamp
[PATCH 4/4] EDAC/mce_amd: Add support for FRU Text in MCA
From: Yazen Ghannam
A new "FRU Text in MCA" feature is defined where the Field Replaceable
Unit (FRU) Text for a device is represented by a string in the new
MCA_SYND1 and MCA_SYND2 registers. This feature is supported per MCA
bank, and it is advertised by the McaFruTextInMca bit (MCA_CONFIG[9]).
The FRU Text is populated dynamically for each individual error state
(MCA_STATUS, MCA_ADDR, et al.). This handles the case where an MCA bank
covers multiple devices, for example, a Unified Memory Controller (UMC)
bank that manages two DIMMs.
Print the FRU Text string, if available, when decoding an MCA error.
Also, add field for MCA_CONFIG MSR in struct mce_hw_err as vendor specific
error information and save the value of the MSR. The very value can then be
exported through tracepoint for userspace tools like rasdaemon to print FRU
Text, if available.
Note: Checkpatch checks/warnings are ignored to maintain coding style.
[Yazen: Add Avadhut as co-developer for wrapper changes. ]
Co-developed-by: Avadhut Naik
Signed-off-by: Avadhut Naik
Signed-off-by: Yazen Ghannam
---
arch/x86/include/asm/mce.h | 2 ++
arch/x86/kernel/cpu/mce/apei.c | 2 ++
arch/x86/kernel/cpu/mce/core.c | 3 +++
drivers/edac/mce_amd.c | 21 ++---
4 files changed, 21 insertions(+), 7 deletions(-)
diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h
index 290d32c84ffd..a513a2dce458 100644
--- a/arch/x86/include/asm/mce.h
+++ b/arch/x86/include/asm/mce.h
@@ -61,6 +61,7 @@
* - TCC bit is present in MCx_STATUS.
*/
#define MCI_CONFIG_MCAX0x1
+#define MCI_CONFIG_FRUTEXT BIT_ULL(9)
#define MCI_IPID_MCATYPE 0x
#define MCI_IPID_HWID 0xFFF
@@ -199,6 +200,7 @@ struct mce_hw_err {
struct {
u64 synd1;
u64 synd2;
+ u64 config;
} amd;
} vi;
};
diff --git a/arch/x86/kernel/cpu/mce/apei.c b/arch/x86/kernel/cpu/mce/apei.c
index 8fd4c42ddc06..2671b3a5f24b 100644
--- a/arch/x86/kernel/cpu/mce/apei.c
+++ b/arch/x86/kernel/cpu/mce/apei.c
@@ -154,6 +154,8 @@ int apei_smca_report_x86_error(struct cper_ia_proc_ctx
*ctx_info, u64 lapic_id)
fallthrough;
/* MCA_CONFIG */
case 4:
+ err.vi.amd.config = *(i_mce + 3);
+ fallthrough;
/* MCA_MISC0 */
case 3:
m->misc = *(i_mce + 2);
diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
index 253d7d0d3331..a2a588c6383e 100644
--- a/arch/x86/kernel/cpu/mce/core.c
+++ b/arch/x86/kernel/cpu/mce/core.c
@@ -207,6 +207,8 @@ static void __print_mce(struct mce_hw_err *err)
pr_cont("SYND2 %llx ", err->vi.amd.synd2);
if (m->ipid)
pr_cont("IPID %llx ", m->ipid);
+ if (err->vi.amd.config)
+ pr_cont("CONFIG %llx ", err->vi.amd.config);
}
pr_cont("\n");
@@ -680,6 +682,7 @@ static noinstr void mce_read_aux(struct mce_hw_err *err,
int i)
if (mce_flags.smca) {
m->ipid = mce_rdmsrl(MSR_AMD64_SMCA_MCx_IPID(i));
+ err->vi.amd.config = mce_rdmsrl(MSR_AMD64_SMCA_MCx_CONFIG(i));
if (m->status & MCI_STATUS_SYNDV) {
m->synd = mce_rdmsrl(MSR_AMD64_SMCA_MCx_SYND(i));
diff --git a/drivers/edac/mce_amd.c b/drivers/edac/mce_amd.c
index 69e12cb2f0de..6ae6b89b1a1e 100644
--- a/drivers/edac/mce_amd.c
+++ b/drivers/edac/mce_amd.c
@@ -795,6 +795,7 @@ amd_decode_mce(struct notifier_block *nb, unsigned long
val, void *data)
struct mce_hw_err *err = (struct mce_hw_err *)data;
struct mce *m = >m;
unsigned int fam = x86_family(m->cpuid);
+ u64 mca_config = err->vi.amd.config;
int ecc;
if (m->kflags & MCE_HANDLED_CEC)
@@ -814,11 +815,7 @@ amd_decode_mce(struct notifier_block *nb, unsigned long
val, void *data)
((m->status & MCI_STATUS_PCC) ? "PCC" : "-"));
if (boot_cpu_has(X86_FEATURE_SMCA)) {
- u32 low, high;
- u32 addr = MSR_AMD64_SMCA_MCx_CONFIG(m->bank);
-
- if (!rdmsr_safe(addr, , ) &&
- (low & MCI_CONFIG_MCAX))
+ if (mca_config & MCI_CONFIG_MCAX)
pr_cont("|%s", ((m->status & MCI_STATUS_TCC) ? "TCC" :
"-"));
pr_cont("|%s", ((m->status & MCI_STATUS_SYNDV) ? "SyndV" :
"-"));
@@ -853,8 +850,18 @@ amd_decode_mce(struct notifier_block *nb, unsigned long
val, void *data)
if (m->status & MCI_STATUS_SYNDV) {
pr_cont(", Syndrome: 0x%016llx\n", m->synd);
- pr_emerg(HW_ERR "Syndrome1: 0x%016llx, Syndrome2:
0x%016llx",
-err->vi.amd.synd1, err->vi.amd.synd2);
+ if (mca_config & MCI_CONFIG_FRUTEXT) {
+ char
[PATCH 3/4] x86/mce/apei: Handle variable register array size
From: Yazen Ghannam
ACPI Boot Error Record Table (BERT) is being used by the kernel to
report errors that occurred in a previous boot. On some modern AMD
systems, these very errors within the BERT are reported through the
x86 Common Platform Error Record (CPER) format which consists of one
or more Processor Context Information Structures. These context
structures provide a starting address and represent an x86 MSR range
in which the data constitutes a contiguous set of MSRs starting from,
and including the starting address.
It's common, for AMD systems that implement this behavior, that the
MSR range represents the MCAX register space used for the Scalable MCA
feature. The apei_smca_report_x86_error() function decodes and passes
this information through the MCE notifier chain. However, this function
assumes a fixed register size based on the original HW/FW implementation.
This assumption breaks with the addition of two new MCAX registers viz.
MCA_SYND1 and MCA_SYND2. These registers are added at the end of the
MCAX register space, so they won't be included when decoding the CPER
data.
Rework apei_smca_report_x86_error() to support a variable register array
size. This covers any case where the MSR context information starts at
the MCAX address for MCA_STATUS and ends at any other register within
the MCAX register space.
Add code comments indicating the MCAX register at each offset.
[Yazen: Add Avadhut as co-developer for wrapper changes.]
Co-developed-by: Avadhut Naik
Signed-off-by: Avadhut Naik
Signed-off-by: Yazen Ghannam
---
arch/x86/kernel/cpu/mce/apei.c | 73 +++---
1 file changed, 59 insertions(+), 14 deletions(-)
diff --git a/arch/x86/kernel/cpu/mce/apei.c b/arch/x86/kernel/cpu/mce/apei.c
index 4cd6312423c6..8fd4c42ddc06 100644
--- a/arch/x86/kernel/cpu/mce/apei.c
+++ b/arch/x86/kernel/cpu/mce/apei.c
@@ -69,7 +69,7 @@ EXPORT_SYMBOL_GPL(apei_mce_report_mem_error);
int apei_smca_report_x86_error(struct cper_ia_proc_ctx *ctx_info, u64 lapic_id)
{
const u64 *i_mce = ((const u64 *) (ctx_info + 1));
- unsigned int cpu;
+ unsigned int cpu, num_registers;
struct mce_hw_err err;
struct mce *m =
@@ -91,16 +91,12 @@ int apei_smca_report_x86_error(struct cper_ia_proc_ctx
*ctx_info, u64 lapic_id)
return -EINVAL;
/*
-* The register array size must be large enough to include all the
-* SMCA registers which need to be extracted.
-*
* The number of registers in the register array is determined by
* Register Array Size/8 as defined in UEFI spec v2.8, sec N.2.4.2.2.
-* The register layout is fixed and currently the raw data in the
-* register array includes 6 SMCA registers which the kernel can
-* extract.
+* Ensure that the array size includes at least 1 register.
*/
- if (ctx_info->reg_arr_size < 48)
+ num_registers = ctx_info->reg_arr_size >> 3;
+ if (!num_registers)
return -EINVAL;
for_each_possible_cpu(cpu) {
@@ -115,12 +111,61 @@ int apei_smca_report_x86_error(struct cper_ia_proc_ctx
*ctx_info, u64 lapic_id)
mce_prep_record_per_cpu(cpu, m);
m->bank = (ctx_info->msr_addr >> 4) & 0xFF;
- m->status = *i_mce;
- m->addr = *(i_mce + 1);
- m->misc = *(i_mce + 2);
- /* Skipping MCA_CONFIG */
- m->ipid = *(i_mce + 4);
- m->synd = *(i_mce + 5);
+
+ /*
+* The SMCA register layout is fixed and includes 16 registers.
+* The end of the array may be variable, but the beginning is known.
+* Switch on the number of registers. Cap the number of registers to
+* expected max (15).
+*/
+ if (num_registers > 15)
+ num_registers = 15;
+
+ switch (num_registers) {
+ /* MCA_SYND2 */
+ case 15:
+ err.vi.amd.synd2 = *(i_mce + 14);
+ fallthrough;
+ /* MCA_SYND1 */
+ case 14:
+ err.vi.amd.synd1 = *(i_mce + 13);
+ fallthrough;
+ /* MCA_MISC4 */
+ case 13:
+ /* MCA_MISC3 */
+ case 12:
+ /* MCA_MISC2 */
+ case 11:
+ /* MCA_MISC1 */
+ case 10:
+ /* MCA_DEADDR */
+ case 9:
+ /* MCA_DESTAT */
+ case 8:
+ /* reserved */
+ case 7:
+ /* MCA_SYND */
+ case 6:
+ m->synd = *(i_mce + 5);
+ fallthrough;
+ /* MCA_IPID */
+ case 5:
+ m->ipid = *(i_mce + 4);
+ fallthrough;
+ /* MCA_CONFIG */
+ case 4:
+ /* MCA_MISC0 */
+ case 3:
+ m->misc = *(i_mce + 2);
+ fallthrough;
+ /* MCA_ADDR */
+ case 2:
+ m->addr = *(i_mce + 1);
+ fallthrough;
+ /* MCA_STATUS */
+ case 1:
+ m->status = *i_mce;
+ }
mce_log();
--
2.34.1
[PATCH 2/4] x86/mce, EDAC/mce_amd: Add support for new MCA_SYND{1,2} registers
From: Avadhut Naik
AMD's Scalable MCA systems viz. Genoa will include two new registers:
MCA_SYND1 and MCA_SYND2.
These registers will include supplemental error information in addition
to the existing MCA_SYND register. The data within the registers is
considered valid if MCA_STATUS[SyndV] is set.
Add fields for these registers as vendor-specific error information
in struct mce_hw_err. Save and print these registers wherever
MCA_STATUS[SyndV]/MCA_SYND is currently used.
Also, modify the mce_record tracepoint to export these new registers
through __dynamic_array. While the sizeof() operator has been used to
determine the size of this __dynamic_array, the same, if needed in the
future can be substituted by caching the size of vendor-specific error
information as part of struct mce_hw_err.
Note: Checkpatch warnings/errors are ignored to maintain coding style.
[Yazen: Drop Yazen's Co-developed-by tag and moved SoB tag.]
Signed-off-by: Avadhut Naik
Signed-off-by: Yazen Ghannam
---
arch/x86/include/asm/mce.h | 12
arch/x86/kernel/cpu/mce/core.c | 24 +---
drivers/edac/mce_amd.c | 10 +++---
include/trace/events/mce.h | 9 +++--
4 files changed, 43 insertions(+), 12 deletions(-)
diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h
index 83923d8a43b0..290d32c84ffd 100644
--- a/arch/x86/include/asm/mce.h
+++ b/arch/x86/include/asm/mce.h
@@ -122,6 +122,9 @@
#define MSR_AMD64_SMCA_MC0_DESTAT 0xc0002008
#define MSR_AMD64_SMCA_MC0_DEADDR 0xc0002009
#define MSR_AMD64_SMCA_MC0_MISC1 0xc000200a
+/* Registers MISC2 to MISC4 are at offsets B to D. */
+#define MSR_AMD64_SMCA_MC0_SYND1 0xc000200e
+#define MSR_AMD64_SMCA_MC0_SYND2 0xc000200f
#define MSR_AMD64_SMCA_MCx_CTL(x) (MSR_AMD64_SMCA_MC0_CTL + 0x10*(x))
#define MSR_AMD64_SMCA_MCx_STATUS(x) (MSR_AMD64_SMCA_MC0_STATUS + 0x10*(x))
#define MSR_AMD64_SMCA_MCx_ADDR(x) (MSR_AMD64_SMCA_MC0_ADDR + 0x10*(x))
@@ -132,6 +135,8 @@
#define MSR_AMD64_SMCA_MCx_DESTAT(x) (MSR_AMD64_SMCA_MC0_DESTAT + 0x10*(x))
#define MSR_AMD64_SMCA_MCx_DEADDR(x) (MSR_AMD64_SMCA_MC0_DEADDR + 0x10*(x))
#define MSR_AMD64_SMCA_MCx_MISCy(x, y) ((MSR_AMD64_SMCA_MC0_MISC1 + y) +
(0x10*(x)))
+#define MSR_AMD64_SMCA_MCx_SYND1(x)(MSR_AMD64_SMCA_MC0_SYND1 + 0x10*(x))
+#define MSR_AMD64_SMCA_MCx_SYND2(x)(MSR_AMD64_SMCA_MC0_SYND2 + 0x10*(x))
#define XEC(x, mask) (((x) >> 16) & mask)
@@ -189,6 +194,13 @@ enum mce_notifier_prios {
struct mce_hw_err {
struct mce m;
+
+ union vendor_info {
+ struct {
+ u64 synd1;
+ u64 synd2;
+ } amd;
+ } vi;
};
struct notifier_block;
diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
index 2f93c825e0fc..253d7d0d3331 100644
--- a/arch/x86/kernel/cpu/mce/core.c
+++ b/arch/x86/kernel/cpu/mce/core.c
@@ -201,6 +201,10 @@ static void __print_mce(struct mce_hw_err *err)
if (mce_flags.smca) {
if (m->synd)
pr_cont("SYND %llx ", m->synd);
+ if (err->vi.amd.synd1)
+ pr_cont("SYND1 %llx ", err->vi.amd.synd1);
+ if (err->vi.amd.synd2)
+ pr_cont("SYND2 %llx ", err->vi.amd.synd2);
if (m->ipid)
pr_cont("IPID %llx ", m->ipid);
}
@@ -651,8 +655,10 @@ static struct notifier_block mce_default_nb = {
/*
* Read ADDR and MISC registers.
*/
-static noinstr void mce_read_aux(struct mce *m, int i)
+static noinstr void mce_read_aux(struct mce_hw_err *err, int i)
{
+ struct mce *m = >m;
+
if (m->status & MCI_STATUS_MISCV)
m->misc = mce_rdmsrl(mca_msr_reg(i, MCA_MISC));
@@ -675,8 +681,11 @@ static noinstr void mce_read_aux(struct mce *m, int i)
if (mce_flags.smca) {
m->ipid = mce_rdmsrl(MSR_AMD64_SMCA_MCx_IPID(i));
- if (m->status & MCI_STATUS_SYNDV)
+ if (m->status & MCI_STATUS_SYNDV) {
m->synd = mce_rdmsrl(MSR_AMD64_SMCA_MCx_SYND(i));
+ err->vi.amd.synd1 =
mce_rdmsrl(MSR_AMD64_SMCA_MCx_SYND1(i));
+ err->vi.amd.synd2 =
mce_rdmsrl(MSR_AMD64_SMCA_MCx_SYND2(i));
+ }
}
}
@@ -815,7 +824,7 @@ void machine_check_poll(enum mcp_flags flags, mce_banks_t
*b)
if (flags & MCP_DONTLOG)
goto clear_it;
- mce_read_aux(m, i);
+ mce_read_aux(, i);
m->severity = mce_severity(m, NULL, NULL, false);
/*
* Don't get the IP here because it's unlikely to
@@ -954,9 +963,10 @@ static __always_inline void quirk_zen_ifu(int bank, struct
mce *m, struct pt_reg
* Do a quick check if any of the events requires a panic.
* This decides if we keep the events around or clear them.
*/
[PATCH 1/4] x86/mce: Add wrapper for struct mce to export vendor specific info
From: Avadhut Naik
Currently, exporting new additional machine check error information
involves adding new fields for the same at the end of the struct mce.
This additional information can then be consumed through mcelog or
tracepoint.
However, as new MSRs are being added (and will be added in the future)
by CPU vendors on their newer CPUs with additional machine check error
information to be exported, the size of struct mce will balloon on some
CPUs, unnecessarily, since those fields are vendor-specific. Moreover,
different CPU vendors may export the additional information in varying
sizes.
The problem particularly intensifies since struct mce is exposed to
userspace as part of UAPI. It's bloating through vendor-specific data
should be avoided to limit the information being sent out to userspace.
Add a new structure mce_hw_err to wrap the existing struct mce. The same
will prevent its ballooning since vendor-specifc data, if any, can now be
exported through a union within the wrapper structure and through
__dynamic_array in mce_record tracepoint.
Furthermore, new internal kernel fields can be added to the wrapper
struct without impacting the user space API.
Note: Some Checkpatch checks have been ignored to maintain coding style.
[Yazen: Add last commit message paragraph. Rebase on other MCA updates.]
Suggested-by: Borislav Petkov (AMD)
Signed-off-by: Avadhut Naik
Signed-off-by: Yazen Ghannam
---
arch/x86/include/asm/mce.h | 6 +-
arch/x86/kernel/cpu/mce/apei.c | 46 ---
arch/x86/kernel/cpu/mce/core.c | 168 +---
arch/x86/kernel/cpu/mce/dev-mcelog.c| 2 +-
arch/x86/kernel/cpu/mce/genpool.c | 20 +--
arch/x86/kernel/cpu/mce/inject.c| 4 +-
arch/x86/kernel/cpu/mce/internal.h | 4 +-
drivers/acpi/acpi_extlog.c | 2 +-
drivers/acpi/nfit/mce.c | 2 +-
drivers/edac/i7core_edac.c | 2 +-
drivers/edac/igen6_edac.c | 2 +-
drivers/edac/mce_amd.c | 2 +-
drivers/edac/pnd2_edac.c| 2 +-
drivers/edac/sb_edac.c | 2 +-
drivers/edac/skx_common.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 2 +-
drivers/ras/amd/fmpm.c | 2 +-
drivers/ras/cec.c | 2 +-
include/trace/events/mce.h | 42 +++---
19 files changed, 173 insertions(+), 141 deletions(-)
diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h
index 3b9970117a0f..83923d8a43b0 100644
--- a/arch/x86/include/asm/mce.h
+++ b/arch/x86/include/asm/mce.h
@@ -187,6 +187,10 @@ enum mce_notifier_prios {
MCE_PRIO_HIGHEST = MCE_PRIO_CEC
};
+struct mce_hw_err {
+ struct mce m;
+};
+
struct notifier_block;
extern void mce_register_decode_chain(struct notifier_block *nb);
extern void mce_unregister_decode_chain(struct notifier_block *nb);
@@ -222,7 +226,7 @@ static inline int apei_smca_report_x86_error(struct
cper_ia_proc_ctx *ctx_info,
#endif
void mce_prep_record(struct mce *m);
-void mce_log(struct mce *m);
+void mce_log(struct mce_hw_err *err);
DECLARE_PER_CPU(struct device *, mce_device);
/* Maximum number of MCA banks per CPU. */
diff --git a/arch/x86/kernel/cpu/mce/apei.c b/arch/x86/kernel/cpu/mce/apei.c
index 0cbadfaf2400..4cd6312423c6 100644
--- a/arch/x86/kernel/cpu/mce/apei.c
+++ b/arch/x86/kernel/cpu/mce/apei.c
@@ -28,9 +28,12 @@
void apei_mce_report_mem_error(int severity, struct cper_sec_mem_err *mem_err)
{
- struct mce m;
+ struct mce_hw_err err;
+ struct mce *m =
int lsb;
+ memset(, 0, sizeof(struct mce_hw_err));
+
if (!(mem_err->validation_bits & CPER_MEM_VALID_PA))
return;
@@ -44,22 +47,22 @@ void apei_mce_report_mem_error(int severity, struct
cper_sec_mem_err *mem_err)
else
lsb = PAGE_SHIFT;
- mce_prep_record();
- m.bank = -1;
+ mce_prep_record(m);
+ m->bank = -1;
/* Fake a memory read error with unknown channel */
- m.status = MCI_STATUS_VAL | MCI_STATUS_EN | MCI_STATUS_ADDRV |
MCI_STATUS_MISCV | 0x9f;
- m.misc = (MCI_MISC_ADDR_PHYS << 6) | lsb;
+ m->status = MCI_STATUS_VAL | MCI_STATUS_EN | MCI_STATUS_ADDRV |
MCI_STATUS_MISCV | 0x9f;
+ m->misc = (MCI_MISC_ADDR_PHYS << 6) | lsb;
if (severity >= GHES_SEV_RECOVERABLE)
- m.status |= MCI_STATUS_UC;
+ m->status |= MCI_STATUS_UC;
if (severity >= GHES_SEV_PANIC) {
- m.status |= MCI_STATUS_PCC;
- m.tsc = rdtsc();
+ m->status |= MCI_STATUS_PCC;
+ m->tsc = rdtsc();
}
- m.addr = mem_err->physical_addr;
- mce_log();
+ m->addr = mem_err->physical_addr;
+ mce_log();
}
EXPORT_SYMBOL_GPL(apei_mce_report_mem_error);
@@ -67,7 +70,10 @@ int apei_smca_report_x86_error(struct cper_ia_proc_ctx
*ctx_info,
[PATCH 0/4] MCE wrapper and support for new SMCA syndrome MSRs
This patchset adds a new wrapper for struct mce to prevent its bloating
and export vendor specific error information. Additionally, support is
also introduced for two new "syndrome" MSRs used in newer AMD Scalable
MCA (SMCA) systems. Also, a new "FRU Text in MCA" feature that uses these
new "syndrome" MSRs has been addded.
Patch 1 adds the new wrapper structure mce_hw_err for the struct mce
while also modifying the mce_record tracepoint to use the new wrapper.
Patch 2 adds support for the new "syndrome" registers. They are read/printed
wherever the existing MCA_SYND register is used.
Patch 3 updates the function that pulls MCA information from UEFI x86
Common Platform Error Records (CPERs) to handle systems that support the
new registers.
Patch 4 adds support to the AMD MCE decoder module to detect and use the
"FRU Text in MCA" feature which leverages the new registers.
NOTE:
This set was initially submitted as part of the larger MCA Updates set.
v1:
https://lore.kernel.org/linux-edac/20231118193248.1296798-1-yazen.ghan...@amd.com/
v2:
https://lore.kernel.org/linux-edac/20240404151359.47970-1-yazen.ghan...@amd.com/
However, since the MCA Updates set has been split up into smaller sets,
this set, going forward, will be submitted independently.
Having said that, this set set depends on and applies cleanly on top of
the below two sets.
1:
https://lore.kernel.org/linux-edac/20240521125434.1555845-1-yazen.ghan...@amd.com/
2:
https://lore.kernel.org/linux-edac/20240523155641.2805411-1-yazen.ghan...@amd.com/
Avadhut Naik (2):
x86/mce: Add wrapper for struct mce to export vendor specific info
x86/mce, EDAC/mce_amd: Add support for new MCA_SYND{1,2} registers
Yazen Ghannam (2):
x86/mce/apei: Handle variable register array size
EDAC/mce_amd: Add support for FRU Text in MCA
arch/x86/include/asm/mce.h | 20 ++-
arch/x86/kernel/cpu/mce/apei.c | 111 ++
arch/x86/kernel/cpu/mce/core.c | 191 ++--
arch/x86/kernel/cpu/mce/dev-mcelog.c| 2 +-
arch/x86/kernel/cpu/mce/genpool.c | 20 +--
arch/x86/kernel/cpu/mce/inject.c| 4 +-
arch/x86/kernel/cpu/mce/internal.h | 4 +-
drivers/acpi/acpi_extlog.c | 2 +-
drivers/acpi/nfit/mce.c | 2 +-
drivers/edac/i7core_edac.c | 2 +-
drivers/edac/igen6_edac.c | 2 +-
drivers/edac/mce_amd.c | 27 +++-
drivers/edac/pnd2_edac.c| 2 +-
drivers/edac/sb_edac.c | 2 +-
drivers/edac/skx_common.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 2 +-
drivers/ras/amd/fmpm.c | 2 +-
drivers/ras/cec.c | 2 +-
include/trace/events/mce.h | 51 ---
19 files changed, 286 insertions(+), 164 deletions(-)
--
2.34.1
Re: [PATCH v2 2/6] livepatch: Add klp-convert tool
On Thu, May 16, 2024 at 03:30:05PM +0200, Lukas Hruska wrote: > Livepatches need to access external symbols which can't be handled > by the normal relocation mechanism. It is needed for two types > of symbols: > > + Symbols which can be local for the original livepatched function. > The alternative implementation in the livepatch sees them > as external symbols. > > + Symbols in modules which are exported via EXPORT_SYMBOL*(). They > must be handled special way otherwise the livepatch module would > depend on the livepatched one. Loading such livepatch would cause > loading the other module as well. > > The address of these symbols can be found via kallsyms. Or they can > be relocated using livepatch specific relocation sections as specified > in Documentation/livepatch/module-elf-format.txt. > > Currently, there is no trivial way to embed the required information as > requested in the final livepatch elf object. klp-convert solves this > problem by using annotations in the elf object to convert the relocation > accordingly to the specification, enabling it to be handled by the > livepatch loader. > > Given the above, create scripts/livepatch to hold tools developed for > livepatches and add source files for klp-convert there. > > Allow to annotate such external symbols in the livepatch by a macro > KLP_RELOC_SYMBOL(). It will create symbol with all needed > metadata. For example: > > extern char *saved_command_line \ > KLP_RELOC_SYMBOL(vmlinux, vmlinux, saved_command_line, 0); > > would create symbol > > $>readelf -r -W : > Relocation section '.rela.text' at offset 0x32e60 contains 10 entries: > Offset Info Type Symbol's Value > Symbol's Name + Addend > [...] > 0068 003c0002 R_X86_64_PC32 > .klp.sym.rela.vmlinux.vmlinux.saved_command_line,0 - 4 > [...] > > Also add scripts/livepatch/klp-convert. The tool transforms symbols > created by KLP_RELOC_SYMBOL() to object specific rela sections > and rela entries which would later be proceed when the livepatch > or the livepatched object is loaded. > > For example, klp-convert would replace the above symbol with: > > $> readelf -r -W > Relocation section '.klp.rela.vmlinux.text' at offset 0x5cb60 contains 1 > entry: > Offset Info Type Symbol's Value > Symbol's Name + Addend > 0068 003c0002 R_X86_64_PC32 > .klp.sym.vmlinux.saved_command_line,0 - 4 > > klp-convert relies on libelf and on a list implementation. Add files > scripts/livepatch/elf.c and scripts/livepatch/elf.h, which are a libelf > interfacing layer and scripts/livepatch/list.h, which is a list > implementation. > > Update Makefiles to correctly support the compilation of the new tool, > update MAINTAINERS file and add a .gitignore file. > > [jpoim...@redhat.com: initial version] > Signed-off-by: Josh Poimboeuf > [joe.lawre...@redhat.com: clean-up and fixes] > Signed-off-by: Joe Lawrence > [lhru...@suse.cz: klp-convert code, minimal approach] > Signed-off-by: Lukas Hruska > Reviewed-by: Marcos Paulo de Souza > --- > MAINTAINERS | 1 + > include/linux/livepatch.h | 19 + > scripts/Makefile| 1 + > scripts/livepatch/.gitignore| 1 + > scripts/livepatch/Makefile | 5 + > scripts/livepatch/elf.c | 817 > scripts/livepatch/elf.h | 73 +++ > scripts/livepatch/klp-convert.c | 284 +++ > scripts/livepatch/klp-convert.h | 23 + > scripts/livepatch/list.h| 391 +++ > 10 files changed, 1615 insertions(+) > create mode 100644 scripts/livepatch/.gitignore > create mode 100644 scripts/livepatch/Makefile > create mode 100644 scripts/livepatch/elf.c > create mode 100644 scripts/livepatch/elf.h > create mode 100644 scripts/livepatch/klp-convert.c > create mode 100644 scripts/livepatch/klp-convert.h > create mode 100644 scripts/livepatch/list.h > > diff --git a/MAINTAINERS b/MAINTAINERS > index 130b8b0bd4f7..d2facc1f4e15 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -12618,6 +12618,7 @@ F:include/uapi/linux/livepatch.h > F: kernel/livepatch/ > F: kernel/module/livepatch.c > F: samples/livepatch/ > +F: scripts/livepatch/ > F: tools/testing/selftests/livepatch/ > > LLC (802.2) > diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h > index 9b9b38e89563..83bbcd1c43fd 100644 > --- a/include/linux/livepatch.h > +++ b/include/linux/livepatch.h > @@ -235,6 +235,25 @@ int klp_apply_section_relocs(struct module *pmod, > Elf_Shdr *sechdrs, >unsigned int symindex, unsigned int secindex, >const char *objname); > > +/** > + * KLP_RELOC_SYMBOL_POS - define relocation for external symbols > + * > + * @LP_OBJ_NAME: name of the livepatched object
Re: [PATCH v3 3/3] arm64: dts: qcom: sm7225-fairphone-fp4: Enable USB role switching
On Thu, May 30, 2024 at 05:05:49PM +0200, Luca Weiss wrote: > Configure the Type-C and VBUS regulator on PM7250B and wire it up to the > USB PHY, so that USB role and orientation switching works. > > For now USB Power Delivery properties are skipped / disabled, so that > the (presumably) bootloader-configured charger doesn't get messed with >From my understanding the no-pd, typec-power-opmode disable PD negotiation over the USB-C. If a device gets connected to the power supply, it will still negotiate something like 5V / 1.5A. > and we can charge the phone with at least some amount of power. > > Reviewed-by: Konrad Dybcio > Signed-off-by: Luca Weiss > --- > .../dts/qcom/sm6350-sony-xperia-lena-pdx213.dts| 1 + > arch/arm64/boot/dts/qcom/sm6350.dtsi | 50 +++ > arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts | 58 > +- > 3 files changed, 108 insertions(+), 1 deletion(-) > Usually the SoC changes come in a separate patch, but I won't insist on that. Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH v3 2/3] arm64: dts: qcom: pm7250b: Add a TCPM description
On Thu, May 30, 2024 at 05:05:48PM +0200, Luca Weiss wrote: > Type-C port management functionality lives inside of the PMIC block on > pm7250b. > > The Type-C port management logic controls orientation detection, > vbus/vconn sense and to send/receive Type-C Power Domain messages. > > Reviewed-by: Bryan O'Donoghue > Reviewed-by: Konrad Dybcio > Signed-off-by: Luca Weiss > --- > arch/arm64/boot/dts/qcom/pm7250b.dtsi | 40 > +++ > 1 file changed, 40 insertions(+) > Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH v3 1/3] arm64: dts: qcom: pm7250b: Add node for PMIC VBUS booster
On Thu, May 30, 2024 at 05:05:47PM +0200, Luca Weiss wrote: > Add the required DTS node for the USB VBUS output regulator, which is > available on PM7250B. This will provide the VBUS source to connected > peripherals. > > Reviewed-by: Bryan O'Donoghue > Signed-off-by: Luca Weiss > --- > arch/arm64/boot/dts/qcom/pm7250b.dtsi | 6 ++ > 1 file changed, 6 insertions(+) > Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
[PATCH v2] dt-bindings: remoteproc: k3-dsp: correct optional sram properties for AM62A SoCs
The C7xv-dsp on AM62A have 32KB L1 I-cache and a 64KB L1 D-cache. It does not have an addressable l1dram . So, remove this optional sram property from the bindings to fix device tree build warnings. Signed-off-by: Hari Nagalla --- Changes from v1 to v2: *) Kept back memory-regions property, as it is unrelated to this patch correcting the sram property for AM62A C7xv-dsp nodes. DT binding check log: https://paste.sr.ht/~hnagalla/cb26237560a572a17c0874b687353e00b400285a v1: https://lore.kernel.org/all/20230810110545.11644-1-hnaga...@ti.com/ .../bindings/remoteproc/ti,k3-dsp-rproc.yaml | 15 ++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml index 9768db8663eb..771cfceb5458 100644 --- a/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml +++ b/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml @@ -111,7 +111,6 @@ else: properties: compatible: enum: - - ti,am62a-c7xv-dsp - ti,j721e-c71-dsp - ti,j721s2-c71-dsp then: @@ -124,6 +123,20 @@ else: items: - const: l2sram - const: l1dram + else: +if: + properties: +compatible: + enum: +- ti,am62a-c7xv-dsp +then: + properties: +reg: + items: +- description: Address and Size of the L2 SRAM internal memory region +reg-names: + items: +- const: l2sram required: - compatible -- 2.34.1
Re: [PATCH v5 5/7] remoteproc: core: support of the tee interface
On Thu, May 30, 2024 at 09:42:26AM +0200, Arnaud POULIQUEN wrote:
> Hello Mathieu,
>
> On 5/29/24 22:35, Mathieu Poirier wrote:
> > On Wed, May 29, 2024 at 09:13:26AM +0200, Arnaud POULIQUEN wrote:
> >> Hello Mathieu,
> >>
> >> On 5/28/24 23:30, Mathieu Poirier wrote:
> >>> On Tue, May 21, 2024 at 10:09:59AM +0200, Arnaud Pouliquen wrote:
> 1) on start:
> - Using the TEE loader, the resource table is loaded by an external
> entity.
> In such case the resource table address is not find from the firmware but
> provided by the TEE remoteproc framework.
> Use the rproc_get_loaded_rsc_table instead of rproc_find_loaded_rsc_table
> - test that rproc->cached_table is not null before performing the memcpy
>
> 2)on stop
> The use of the cached_table seems mandatory:
> - during recovery sequence to have a snapshot of the resource table
> resources used,
> - on stop to allow for the deinitialization of resources after the
> the remote processor has been shutdown.
> However if the TEE interface is being used, we first need to unmap the
> table_ptr before setting it to rproc->cached_table.
> The update of rproc->table_ptr to rproc->cached_table is performed in
> tee_remoteproc.
>
> Signed-off-by: Arnaud Pouliquen
> ---
> drivers/remoteproc/remoteproc_core.c | 31 +---
> 1 file changed, 23 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/remoteproc/remoteproc_core.c
> b/drivers/remoteproc/remoteproc_core.c
> index 42bca01f3bde..3a642151c983 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -1267,6 +1267,7 @@ EXPORT_SYMBOL(rproc_resource_cleanup);
> static int rproc_set_rsc_table_on_start(struct rproc *rproc, const
> struct firmware *fw)
> {
> struct resource_table *loaded_table;
> +struct device *dev = >dev;
>
> /*
> * The starting device has been given the rproc->cached_table
> as the
> @@ -1276,12 +1277,21 @@ static int rproc_set_rsc_table_on_start(struct
> rproc *rproc, const struct firmwa
> * this information to device memory. We also update the
> table_ptr so
> * that any subsequent changes will be applied to the loaded
> version.
> */
> -loaded_table = rproc_find_loaded_rsc_table(rproc, fw);
> -if (loaded_table) {
> -memcpy(loaded_table, rproc->cached_table,
> rproc->table_sz);
> -rproc->table_ptr = loaded_table;
> +if (rproc->tee_interface) {
> +loaded_table = rproc_get_loaded_rsc_table(rproc,
> >table_sz);
> +if (IS_ERR(loaded_table)) {
> +dev_err(dev, "can't get resource table\n");
> +return PTR_ERR(loaded_table);
> +}
> +} else {
> +loaded_table = rproc_find_loaded_rsc_table(rproc, fw);
> }
>
> +if (loaded_table && rproc->cached_table)
> +memcpy(loaded_table, rproc->cached_table,
> rproc->table_sz);
> +
> >>>
> >>> Why is this not part of the else {} above as it was the case before this
> >>> patch?
> >>> And why was an extra check for ->cached_table added?
> >>
> >> Here we have to cover 2 use cases if rproc->tee_interface is set.
> >> 1) The remote processor is in stop state
> >> - loaded_table points to the resource table in the remote memory and
> >> - rproc->cached_table is null
> >> => no memcopy
> >> 2) crash recovery
> >> - loaded_table points to the resource table in the remote memory
> >> - rproc-cached_table point to a copy of the resource table
> >
> > A cached_table exists because it was created in
> > rproc_reset_rsc_table_on_stop().
> > But as the comment says [1], that part of the code was meant to be used for
> > the
> > attach()/detach() use case. Mixing both will become extremely confusing and
> > impossible to maintain.
>
> i am not sure to understand your point here... the cached_table table was
> already existing for the "normal" case[2]. Seems to me that the cache table is
> needed on stop in all scenarios.
>
> [2]
> https://elixir.bootlin.com/linux/v4.20.17/source/drivers/remoteproc/remoteproc_core.c#L1402
>
> >
> > I think the TEE scenario should be as similar as the "normal" one where TEE
> > is
> > not involved. To that end, I suggest to create a cached_table in
> > tee_rproc_parse_fw(), exactly the same way it is done in
> > rproc_elf_load_rsc_table(). That way the code path in
> > rproc_set_rsc_table_on_start() become very similar and we have a
> > cached_table to
> > work with when the remote processor is recovered. In fact we may not need
> >
[PATCH v3 3/3] arm64: dts: qcom: sm7225-fairphone-fp4: Enable USB role switching
Configure the Type-C and VBUS regulator on PM7250B and wire it up to the
USB PHY, so that USB role and orientation switching works.
For now USB Power Delivery properties are skipped / disabled, so that
the (presumably) bootloader-configured charger doesn't get messed with
and we can charge the phone with at least some amount of power.
Reviewed-by: Konrad Dybcio
Signed-off-by: Luca Weiss
---
.../dts/qcom/sm6350-sony-xperia-lena-pdx213.dts| 1 +
arch/arm64/boot/dts/qcom/sm6350.dtsi | 50 +++
arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts | 58 +-
3 files changed, 108 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/sm6350-sony-xperia-lena-pdx213.dts
b/arch/arm64/boot/dts/qcom/sm6350-sony-xperia-lena-pdx213.dts
index 6e44d280..88ee04973a2f 100644
--- a/arch/arm64/boot/dts/qcom/sm6350-sony-xperia-lena-pdx213.dts
+++ b/arch/arm64/boot/dts/qcom/sm6350-sony-xperia-lena-pdx213.dts
@@ -375,6 +375,7 @@ _1 {
};
_1_dwc3 {
+ /delete-property/ usb-role-switch;
maximum-speed = "super-speed";
dr_mode = "peripheral";
};
diff --git a/arch/arm64/boot/dts/qcom/sm6350.dtsi
b/arch/arm64/boot/dts/qcom/sm6350.dtsi
index acf0b0f73af9..1ac626d963b8 100644
--- a/arch/arm64/boot/dts/qcom/sm6350.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm6350.dtsi
@@ -1715,10 +1715,39 @@ usb_1_qmpphy: phy@88e8000 {
< GCC_USB3_DP_PHY_PRIM_BCR>;
reset-names = "phy", "common";
+ orientation-switch;
+
#clock-cells = <1>;
#phy-cells = <1>;
status = "disabled";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+
+ usb_1_qmpphy_out: endpoint {
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+
+ usb_1_qmpphy_usb_ss_in: endpoint {
+ remote-endpoint =
<_1_dwc3_ss_out>;
+ };
+ };
+
+ port@2 {
+ reg = <2>;
+
+ usb_1_qmpphy_dp_in: endpoint {
+ };
+ };
+ };
};
dc_noc: interconnect@916 {
@@ -1894,6 +1923,27 @@ usb_1_dwc3: usb@a60 {
snps,hird-threshold = /bits/ 8 <0x10>;
phys = <_1_hsphy>, <_1_qmpphy
QMP_USB43DP_USB3_PHY>;
phy-names = "usb2-phy", "usb3-phy";
+ usb-role-switch;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+
+ usb_1_dwc3_hs_out: endpoint {
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+
+ usb_1_dwc3_ss_out: endpoint {
+ remote-endpoint =
<_1_qmpphy_usb_ss_in>;
+ };
+ };
+ };
};
};
diff --git a/arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts
b/arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts
index bc67e8c1fe4d..d2632f011353 100644
--- a/arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts
+++ b/arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts
@@ -19,6 +19,7 @@
#include
#include
#include
+#include
#include "sm7225.dtsi"
#include "pm6150l.dtsi"
#include "pm6350.dtsi"
@@ -543,6 +544,53 @@ conn-therm@1 {
};
};
+_typec {
+ vdd-pdphy-supply = <_l3a>;
+
+ status = "okay";
+
+ connector {
+ compatible = "usb-c-connector";
+
+ power-role = "dual";
+ data-role = "dual";
+ self-powered;
+
+ /*
+* Disable USB Power Delivery for now, seems to need extra work
+* to support role switching while also letting the battery
+* charge still - without charger driver
+*/
+ typec-power-opmode = "default";
+
[PATCH v3 0/3] Add TCPM support for PM7250B and Fairphone 4
This series adds support for Type-C Port Management on the Fairphone 4 which enables USB role switching and orientation switching. This enables a user for example to plug in a USB stick or a USB keyboard to the Type-C port. To: Bjorn Andersson To: Konrad Dybcio To: Rob Herring To: Krzysztof Kozlowski To: Conor Dooley Cc: ~postmarketos/upstream...@lists.sr.ht Cc: phone-de...@vger.kernel.org Cc: linux-arm-...@vger.kernel.org Cc: devicet...@vger.kernel.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Luca Weiss Changes in v3: - Disable pm7250b typec node by default since on some platforms the ADSP firmware will manage it and not Linux (Bjorn) - Move usb-role-switch and orientation-switch to dtsi (Konrad) - update sony-lena also as per https://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git/commit/?id=dad66630a083263b513448426523a3b52a959c79 - Link to v2: https://lore.kernel.org/r/20240329-fp4-tcpm-v2-0-d7f8cd165...@fairphone.com Changes in v2: - Move disabled as last property for pm7250b_vbus - Update USB graph to newer version, connect both HS and SS signals - Update FP4 Type-C properties, try to keep phone charging intact by disabling USB PD for now - Pick up tags - Drop patches that landed in linux-next already - Link to v1: https://lore.kernel.org/r/20240322-fp4-tcpm-v1-0-c5644099d...@fairphone.com --- Luca Weiss (3): arm64: dts: qcom: pm7250b: Add node for PMIC VBUS booster arm64: dts: qcom: pm7250b: Add a TCPM description arm64: dts: qcom: sm7225-fairphone-fp4: Enable USB role switching arch/arm64/boot/dts/qcom/pm7250b.dtsi | 46 + .../dts/qcom/sm6350-sony-xperia-lena-pdx213.dts| 1 + arch/arm64/boot/dts/qcom/sm6350.dtsi | 50 +++ arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts | 58 +- 4 files changed, 154 insertions(+), 1 deletion(-) --- base-commit: 4adcd8f0525c79f058c10e3ecaaba74932f6 change-id: 20240322-fp4-tcpm-2ad68ef55346 Best regards, -- Luca Weiss
[PATCH v3 2/3] arm64: dts: qcom: pm7250b: Add a TCPM description
Type-C port management functionality lives inside of the PMIC block on
pm7250b.
The Type-C port management logic controls orientation detection,
vbus/vconn sense and to send/receive Type-C Power Domain messages.
Reviewed-by: Bryan O'Donoghue
Reviewed-by: Konrad Dybcio
Signed-off-by: Luca Weiss
---
arch/arm64/boot/dts/qcom/pm7250b.dtsi | 40 +++
1 file changed, 40 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/pm7250b.dtsi
b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
index 4faed25a787f..7dc7262f1537 100644
--- a/arch/arm64/boot/dts/qcom/pm7250b.dtsi
+++ b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
@@ -51,6 +51,46 @@ pm7250b_vbus: usb-vbus-regulator@1100 {
status = "disabled";
};
+ pm7250b_typec: typec@1500 {
+ compatible = "qcom,pm7250b-typec", "qcom,pm8150b-typec";
+ reg = <0x1500>,
+ <0x1700>;
+ interrupts = ,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+;
+ interrupt-names = "or-rid-detect-change",
+ "vpd-detect",
+ "cc-state-change",
+ "vconn-oc",
+ "vbus-change",
+ "attach-detach",
+ "legacy-cable-detect",
+ "try-snk-src-detect",
+ "sig-tx",
+ "sig-rx",
+ "msg-tx",
+ "msg-rx",
+ "msg-tx-failed",
+ "msg-tx-discarded",
+ "msg-rx-discarded",
+ "fr-swap";
+ vdd-vbus-supply = <_vbus>;
+ status = "disabled";
+ };
+
pm7250b_temp: temp-alarm@2400 {
compatible = "qcom,spmi-temp-alarm";
reg = <0x2400>;
--
2.45.1
[PATCH v3 1/3] arm64: dts: qcom: pm7250b: Add node for PMIC VBUS booster
Add the required DTS node for the USB VBUS output regulator, which is
available on PM7250B. This will provide the VBUS source to connected
peripherals.
Reviewed-by: Bryan O'Donoghue
Signed-off-by: Luca Weiss
---
arch/arm64/boot/dts/qcom/pm7250b.dtsi | 6 ++
1 file changed, 6 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/pm7250b.dtsi
b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
index 3bf7cf5d1700..4faed25a787f 100644
--- a/arch/arm64/boot/dts/qcom/pm7250b.dtsi
+++ b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
@@ -45,6 +45,12 @@ pmic@PM7250B_SID {
#address-cells = <1>;
#size-cells = <0>;
+ pm7250b_vbus: usb-vbus-regulator@1100 {
+ compatible = "qcom,pm7250b-vbus-reg",
"qcom,pm8150b-vbus-reg";
+ reg = <0x1100>;
+ status = "disabled";
+ };
+
pm7250b_temp: temp-alarm@2400 {
compatible = "qcom,spmi-temp-alarm";
reg = <0x2400>;
--
2.45.1
Re: [PATCH 2/3] remoteproc: k3-r5: Acquire mailbox handle during probe
On 5/30/24 4:07 AM, Beleswar Padhi wrote:
Acquire the mailbox handle during device probe and do not release handle
in stop/detach routine or error paths. This removes the redundant
requests for mbox handle later during rproc start/attach. This also
allows to defer remoteproc driver's probe if mailbox is not probed yet.
Signed-off-by: Beleswar Padhi
---
drivers/remoteproc/ti_k3_r5_remoteproc.c | 66
1 file changed, 21 insertions(+), 45 deletions(-)
diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c
b/drivers/remoteproc/ti_k3_r5_remoteproc.c
index 26362a509ae3..157e8fd57665 100644
--- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
@@ -391,6 +391,7 @@ static int k3_r5_rproc_request_mbox(struct rproc *rproc)
struct mbox_client *client = >client;
struct device *dev = kproc->dev;
int ret;
+ long err;
client->dev = dev;
client->tx_done = NULL;
@@ -400,10 +401,9 @@ static int k3_r5_rproc_request_mbox(struct rproc *rproc)
kproc->mbox = mbox_request_channel(client, 0);
if (IS_ERR(kproc->mbox)) {
- ret = -EBUSY;
- dev_err(dev, "mbox_request_channel failed: %ld\n",
- PTR_ERR(kproc->mbox));
- return ret;
+ err = PTR_ERR(kproc->mbox);
+ dev_err(dev, "mbox_request_channel failed: %ld\n", err);
+ return (err == -EPROBE_DEFER) ? -EPROBE_DEFER : -EBUSY;
Why turn all other errors into EBUSY? If you just return the error as-is you
can simply make these 3 lines just:
return dev_err_probe(dev, PTR_ERR(kproc->mbox), "mbox_request_channel
failed\n");
}
/*
@@ -552,10 +552,6 @@ static int k3_r5_rproc_start(struct rproc *rproc)
u32 boot_addr;
int ret;
- ret = k3_r5_rproc_request_mbox(rproc);
- if (ret)
- return ret;
-
boot_addr = rproc->bootaddr;
/* TODO: add boot_addr sanity checking */
dev_dbg(dev, "booting R5F core using boot addr = 0x%x\n", boot_addr);
@@ -564,7 +560,7 @@ static int k3_r5_rproc_start(struct rproc *rproc)
core = kproc->core;
ret = ti_sci_proc_set_config(core->tsp, boot_addr, 0, 0);
if (ret)
- goto put_mbox;
+ goto out;
The label "out" doesn't do anything, just directly `return ret;` here and
in the other cases below.
/* unhalt/run all applicable cores */
if (cluster->mode == CLUSTER_MODE_LOCKSTEP) {
@@ -581,12 +577,12 @@ static int k3_r5_rproc_start(struct rproc *rproc)
dev_err(dev, "%s: can not start core 1 before core 0\n",
__func__);
ret = -EPERM;
- goto put_mbox;
+ goto out;
}
ret = k3_r5_core_run(core);
if (ret)
- goto put_mbox;
+ goto out;
}
return 0;
@@ -596,8 +592,7 @@ static int k3_r5_rproc_start(struct rproc *rproc)
if (k3_r5_core_halt(core))
dev_warn(core->dev, "core halt back failed\n");
}
-put_mbox:
- mbox_free_channel(kproc->mbox);
+out:
return ret;
}
@@ -658,8 +653,6 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
goto out;
}
- mbox_free_channel(kproc->mbox);
-
return 0;
unroll_core_halt:
@@ -674,42 +667,21 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
/*
* Attach to a running R5F remote processor (IPC-only mode)
*
- * The R5F attach callback only needs to request the mailbox, the remote
- * processor is already booted, so there is no need to issue any TI-SCI
- * commands to boot the R5F cores in IPC-only mode. This callback is invoked
- * only in IPC-only mode.
+ * The R5F attach callback is a NOP. The remote processor is already booted,
and
+ * all required resources have been acquired during probe routine, so there is
+ * no need to issue any TI-SCI commands to boot the R5F cores in IPC-only mode.
+ * This callback is invoked only in IPC-only mode and exists for sanity sake.
*/
-static int k3_r5_rproc_attach(struct rproc *rproc)
-{
- struct k3_r5_rproc *kproc = rproc->priv;
- struct device *dev = kproc->dev;
- int ret;
-
- ret = k3_r5_rproc_request_mbox(rproc);
- if (ret)
- return ret;
-
- dev_info(dev, "R5F core initialized in IPC-only mode\n");
- return 0;
-}
+static int k3_r5_rproc_attach(struct rproc *rproc) { return 0; }
/*
* Detach from a running R5F remote processor (IPC-only mode)
*
- * The R5F detach callback performs the opposite operation to attach callback
- * and only needs to release the mailbox, the R5F cores are not stopped and
- * will be left in booted state in IPC-only mode. This callback is invoked
- * only in IPC-only mode.
+ * The R5F detach callback is a
Re: Bug in Kernel 6.8.x, 6.9.x Causing Trace/Panic During Shutdown/Reboot
On Thu, 30 May 2024 16:02:37 +0300
Ilkka Naulapää wrote:
> applied your patch and here's the output.
>
Unfortunately, it doesn't give me any new information. I added one more
BUG on, want to try this? Otherwise, I'm pretty much at a lost. :-/
-- Steve
diff --git a/fs/tracefs/inode.c b/fs/tracefs/inode.c
index de5b72216b1a..a090495e78c9 100644
--- a/fs/tracefs/inode.c
+++ b/fs/tracefs/inode.c
@@ -39,13 +39,17 @@ static struct inode *tracefs_alloc_inode(struct super_block
*sb)
return NULL;
ti->flags = 0;
+ ti->magic = 20240823;
return >vfs_inode;
}
static void tracefs_free_inode(struct inode *inode)
{
- kmem_cache_free(tracefs_inode_cachep, get_tracefs(inode));
+ struct tracefs_inode *ti = get_tracefs(inode);
+
+ BUG_ON(ti->magic != 20240823);
+ kmem_cache_free(tracefs_inode_cachep, ti);
}
static ssize_t default_read_file(struct file *file, char __user *buf,
@@ -147,16 +151,6 @@ static const struct inode_operations
tracefs_dir_inode_operations = {
.rmdir = tracefs_syscall_rmdir,
};
-struct inode *tracefs_get_inode(struct super_block *sb)
-{
- struct inode *inode = new_inode(sb);
- if (inode) {
- inode->i_ino = get_next_ino();
- inode->i_atime = inode->i_mtime =
inode_set_ctime_current(inode);
- }
- return inode;
-}
-
struct tracefs_mount_opts {
kuid_t uid;
kgid_t gid;
@@ -384,6 +378,7 @@ static void tracefs_dentry_iput(struct dentry *dentry,
struct inode *inode)
return;
ti = get_tracefs(inode);
+ BUG_ON(ti->magic != 20240823);
if (ti && ti->flags & TRACEFS_EVENT_INODE)
eventfs_set_ef_status_free(dentry);
iput(inode);
@@ -568,6 +563,18 @@ struct dentry *eventfs_end_creating(struct dentry *dentry)
return dentry;
}
+struct inode *tracefs_get_inode(struct super_block *sb)
+{
+ struct inode *inode = new_inode(sb);
+
+ BUG_ON(sb->s_op != _super_operations);
+ if (inode) {
+ inode->i_ino = get_next_ino();
+ inode->i_atime = inode->i_mtime =
inode_set_ctime_current(inode);
+ }
+ return inode;
+}
+
/**
* tracefs_create_file - create a file in the tracefs filesystem
* @name: a pointer to a string containing the name of the file to create.
diff --git a/fs/tracefs/internal.h b/fs/tracefs/internal.h
index 69c2b1d87c46..9059b8b11bb6 100644
--- a/fs/tracefs/internal.h
+++ b/fs/tracefs/internal.h
@@ -9,12 +9,15 @@ enum {
struct tracefs_inode {
unsigned long flags;
void*private;
+ unsigned long magic;
struct inodevfs_inode;
};
static inline struct tracefs_inode *get_tracefs(const struct inode *inode)
{
- return container_of(inode, struct tracefs_inode, vfs_inode);
+ struct tracefs_inode *ti = container_of(inode, struct tracefs_inode,
vfs_inode);
+ BUG_ON(ti->magic != 20240823);
+ return ti;
}
struct dentry *tracefs_start_creating(const char *name, struct dentry *parent);
Re: [PATCH net-next V2] virtio-net: synchronize operstate with admin state on up/down
On Thu, May 30, 2024 at 06:29:51PM +0800, Jason Wang wrote:
> On Thu, May 30, 2024 at 2:10 PM Michael S. Tsirkin wrote:
> >
> > On Thu, May 30, 2024 at 11:20:55AM +0800, Jason Wang wrote:
> > > This patch synchronize operstate with admin state per RFC2863.
> > >
> > > This is done by trying to toggle the carrier upon open/close and
> > > synchronize with the config change work. This allows propagate status
> > > correctly to stacked devices like:
> > >
> > > ip link add link enp0s3 macvlan0 type macvlan
> > > ip link set link enp0s3 down
> > > ip link show
> > >
> > > Before this patch:
> > >
> > > 3: enp0s3: mtu 1500 qdisc pfifo_fast state DOWN
> > > mode DEFAULT group default qlen 1000
> > > link/ether 00:00:05:00:00:09 brd ff:ff:ff:ff:ff:ff
> > > ..
> > > 5: macvlan0@enp0s3: mtu 1500
> > > qdisc noqueue state UP mode DEFAULT group default qlen 1000
> > > link/ether b2:a9:c5:04:da:53 brd ff:ff:ff:ff:ff:ff
> > >
> > > After this patch:
> > >
> > > 3: enp0s3: mtu 1500 qdisc pfifo_fast state DOWN
> > > mode DEFAULT group default qlen 1000
> > > link/ether 00:00:05:00:00:09 brd ff:ff:ff:ff:ff:ff
> > > ...
> > > 5: macvlan0@enp0s3: mtu 1500
> > > qdisc noqueue state LOWERLAYERDOWN mode DEFAULT group default qlen 1000
> > > link/ether b2:a9:c5:04:da:53 brd ff:ff:ff:ff:ff:ff
> > >
> > > Cc: Venkat Venkatsubra
> > > Cc: Gia-Khanh Nguyen
> > > Reviewed-by: Xuan Zhuo
> > > Acked-by: Michael S. Tsirkin
> > > Signed-off-by: Jason Wang
> > > ---
> > > Changes since V1:
> > > - rebase
> > > - add ack/review tags
> >
> >
> >
> >
> >
> > > ---
> > > drivers/net/virtio_net.c | 94 +++-
> > > 1 file changed, 63 insertions(+), 31 deletions(-)
> > >
> > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > index 4a802c0ea2cb..69e4ae353c51 100644
> > > --- a/drivers/net/virtio_net.c
> > > +++ b/drivers/net/virtio_net.c
> > > @@ -433,6 +433,12 @@ struct virtnet_info {
> > > /* The lock to synchronize the access to refill_enabled */
> > > spinlock_t refill_lock;
> > >
> > > + /* Is config change enabled? */
> > > + bool config_change_enabled;
> > > +
> > > + /* The lock to synchronize the access to config_change_enabled */
> > > + spinlock_t config_change_lock;
> > > +
> > > /* Work struct for config space updates */
> > > struct work_struct config_work;
> > >
> >
> >
> > But we already have dev->config_lock and dev->config_enabled.
> >
> > And it actually works better - instead of discarding config
> > change events it defers them until enabled.
> >
>
> Yes but then both virtio-net driver and virtio core can ask to enable
> and disable and then we need some kind of synchronization which is
> non-trivial.
Well for core it happens on bring up path before driver works
and later on tear down after it is gone.
So I do not think they ever do it at the same time.
> And device enabling on the core is different from bringing the device
> up in the networking subsystem. Here we just delay to deal with the
> config change interrupt on ndo_open(). (E.g try to ack announce is
> meaningless when the device is down).
>
> Thanks
another thing is that it is better not to re-read all config
on link up if there was no config interrupt - less vm exits.
--
MST
Re: How to trace kvm:kvm_exit using fprobe?
Hi don, Thanks for reporting! I confirmed that the raw tracepoint probe event does not work on the tracepoint in the module. (not only kvm but also other modules) Let me fix the issue. Thank you, On Tue, 21 May 2024 17:28:48 -0700 don wrote: > Hi, Mr Masami Hiramatsu， > I see your presentation "Function Parameters with BTF Function tracing with > parameters", > and I started using fprobe and dynamic_events to trace functions. > > I have a question regarding tracepoints using fprobe/dynamic_events > I can trace the kvm:kvm_exit event using trace-cmd: > > *trace-cmd stream -e kvm:kvm_exit* > But if I echo to dynamic_event will get "Invalid argument" error: > # cd /sys/kernel/debug/tracing > > *# echo 't:kvm kvm_exit' >> dynamic_events*-bash: echo: write error: > Invalid argument > > How to solve this problem? > > Thanks, > don -- Masami Hiramatsu (Google)
Re: [PATCH net-next V2] virtio-net: synchronize operstate with admin state on up/down
On Thu, May 30, 2024 at 6:29 PM Jason Wang wrote:
>
> On Thu, May 30, 2024 at 2:10 PM Michael S. Tsirkin wrote:
> >
> > On Thu, May 30, 2024 at 11:20:55AM +0800, Jason Wang wrote:
> > > This patch synchronize operstate with admin state per RFC2863.
> > >
> > > This is done by trying to toggle the carrier upon open/close and
> > > synchronize with the config change work. This allows propagate status
> > > correctly to stacked devices like:
> > >
> > > ip link add link enp0s3 macvlan0 type macvlan
> > > ip link set link enp0s3 down
> > > ip link show
> > >
> > > Before this patch:
> > >
> > > 3: enp0s3: mtu 1500 qdisc pfifo_fast state DOWN
> > > mode DEFAULT group default qlen 1000
> > > link/ether 00:00:05:00:00:09 brd ff:ff:ff:ff:ff:ff
> > > ..
> > > 5: macvlan0@enp0s3: mtu 1500
> > > qdisc noqueue state UP mode DEFAULT group default qlen 1000
> > > link/ether b2:a9:c5:04:da:53 brd ff:ff:ff:ff:ff:ff
> > >
> > > After this patch:
> > >
> > > 3: enp0s3: mtu 1500 qdisc pfifo_fast state DOWN
> > > mode DEFAULT group default qlen 1000
> > > link/ether 00:00:05:00:00:09 brd ff:ff:ff:ff:ff:ff
> > > ...
> > > 5: macvlan0@enp0s3: mtu 1500
> > > qdisc noqueue state LOWERLAYERDOWN mode DEFAULT group default qlen 1000
> > > link/ether b2:a9:c5:04:da:53 brd ff:ff:ff:ff:ff:ff
> > >
> > > Cc: Venkat Venkatsubra
> > > Cc: Gia-Khanh Nguyen
> > > Reviewed-by: Xuan Zhuo
> > > Acked-by: Michael S. Tsirkin
> > > Signed-off-by: Jason Wang
> > > ---
> > > Changes since V1:
> > > - rebase
> > > - add ack/review tags
> >
> >
> >
> >
> >
> > > ---
> > > drivers/net/virtio_net.c | 94 +++-
> > > 1 file changed, 63 insertions(+), 31 deletions(-)
> > >
> > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > index 4a802c0ea2cb..69e4ae353c51 100644
> > > --- a/drivers/net/virtio_net.c
> > > +++ b/drivers/net/virtio_net.c
> > > @@ -433,6 +433,12 @@ struct virtnet_info {
> > > /* The lock to synchronize the access to refill_enabled */
> > > spinlock_t refill_lock;
> > >
> > > + /* Is config change enabled? */
> > > + bool config_change_enabled;
> > > +
> > > + /* The lock to synchronize the access to config_change_enabled */
> > > + spinlock_t config_change_lock;
> > > +
> > > /* Work struct for config space updates */
> > > struct work_struct config_work;
> > >
> >
> >
> > But we already have dev->config_lock and dev->config_enabled.
> >
> > And it actually works better - instead of discarding config
> > change events it defers them until enabled.
> >
>
> Yes but then both virtio-net driver and virtio core can ask to enable
> and disable and then we need some kind of synchronization which is
> non-trivial.
>
> And device enabling on the core is different from bringing the device
> up in the networking subsystem. Here we just delay to deal with the
> config change interrupt on ndo_open(). (E.g try to ack announce is
> meaningless when the device is down).
Or maybe you meant to make config_enabled a nest counter?
Thanks
>
> Thanks
Re: [PATCH v2] sched/rt: Clean up usage of rt_task()
On 05/29/24 12:55, Sebastian Andrzej Siewior wrote:
> On 2024-05-29 11:34:09 [+0100], Qais Yousef wrote:
> > > behaviour. But then it is insistent which matters only in the RT case.
> > > Puh. Any sched folks regarding policy?
> >
> > I am not sure I understood you here. Could you rephrase please?
>
> Right now a SCHED_OTHER task boosted to a realtime priority gets
> slack=0. In the !RT scenario everything is fine.
> For RT the slack=0 also happens but the init of the timer looks at the
> policy instead at the possible boosted priority and uses a different
> clock attribute. This can lead to a delayed wake up (so avoiding the
> slack does not solve the problem).
>
> This is not consistent because IMHO the clock setup & slack should be
> handled equally. So I am asking the sched folks for a policy and I am
> leaning towards looking at task-policy in this case instead of prio
> because you shouldn't do anything that can delay.
Can't we do that based on is_soft/is_hard flag in hrtimer struct when we apply
the slack in hrtimer_set_expires_range_ns() instead?
(not compile tested even)
diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h
index aa1e65ccb615..e001f20bbea9 100644
--- a/include/linux/hrtimer.h
+++ b/include/linux/hrtimer.h
@@ -102,12 +102,16 @@ static inline void hrtimer_set_expires(struct hrtimer
*timer, ktime_t time)
static inline void hrtimer_set_expires_range(struct hrtimer *timer, ktime_t
time, ktime_t delta)
{
+ if (timer->is_soft || timer->is_hard)
+ delta = 0;
timer->_softexpires = time;
timer->node.expires = ktime_add_safe(time, delta);
}
static inline void hrtimer_set_expires_range_ns(struct hrtimer *timer, ktime_t
time, u64 delta)
{
+ if (timer->is_soft || timer->is_hard)
+ delta = 0;
timer->_softexpires = time;
timer->node.expires = ktime_add_safe(time, ns_to_ktime(delta));
}
[PATCH V2] net: qrtr: ns: Ignore ENODEV failures in ns
From: Chris Lew
Ignore the ENODEV failures returned by kernel_sendmsg(). These errors
indicate that either the local port has been closed or the remote has
gone down. Neither of these scenarios are fatal and will eventually be
handled through packets that are later queued on the control port.
Signed-off-by: Chris Lew
Signed-off-by: Sarannya Sasikumar
Reviewed-by: Simon Horman
---
Changes from previous revision:
Changed return type of service_announce_del from int to void.
net/qrtr/ns.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/net/qrtr/ns.c b/net/qrtr/ns.c
index 654a3cc0d347..e821101e7a4b 100644
--- a/net/qrtr/ns.c
+++ b/net/qrtr/ns.c
@@ -132,7 +132,7 @@ static int service_announce_new(struct sockaddr_qrtr *dest,
return kernel_sendmsg(qrtr_ns.sock, , , 1, sizeof(pkt));
}
-static int service_announce_del(struct sockaddr_qrtr *dest,
+static void service_announce_del(struct sockaddr_qrtr *dest,
struct qrtr_server *srv)
{
struct qrtr_ctrl_pkt pkt;
@@ -157,10 +157,10 @@ static int service_announce_del(struct sockaddr_qrtr
*dest,
msg.msg_namelen = sizeof(*dest);
ret = kernel_sendmsg(qrtr_ns.sock, , , 1, sizeof(pkt));
- if (ret < 0)
+ if (ret < 0 && ret != -ENODEV)
pr_err("failed to announce del service\n");
- return ret;
+ return;
}
static void lookup_notify(struct sockaddr_qrtr *to, struct qrtr_server *srv,
@@ -188,7 +188,7 @@ static void lookup_notify(struct sockaddr_qrtr *to, struct
qrtr_server *srv,
msg.msg_namelen = sizeof(*to);
ret = kernel_sendmsg(qrtr_ns.sock, , , 1, sizeof(pkt));
- if (ret < 0)
+ if (ret < 0 && ret != -ENODEV)
pr_err("failed to send lookup notification\n");
}
@@ -207,6 +207,9 @@ static int announce_servers(struct sockaddr_qrtr *sq)
xa_for_each(>servers, index, srv) {
ret = service_announce_new(sq, srv);
if (ret < 0) {
+ if (ret == -ENODEV)
+ continue;
+
pr_err("failed to announce new service\n");
return ret;
}
@@ -369,7 +372,7 @@ static int ctrl_cmd_bye(struct sockaddr_qrtr *from)
msg.msg_namelen = sizeof(sq);
ret = kernel_sendmsg(qrtr_ns.sock, , , 1, sizeof(pkt));
- if (ret < 0) {
+ if (ret < 0 && ret != -ENODEV) {
pr_err("failed to send bye cmd\n");
return ret;
}
@@ -443,7 +446,7 @@ static int ctrl_cmd_del_client(struct sockaddr_qrtr *from,
msg.msg_namelen = sizeof(sq);
ret = kernel_sendmsg(qrtr_ns.sock, , , 1, sizeof(pkt));
- if (ret < 0) {
+ if (ret < 0 && ret != -ENODEV) {
pr_err("failed to send del client cmd\n");
return ret;
}
--
2.25.1
Re: [PATCH net-next V2] virtio-net: synchronize operstate with admin state on up/down
On Thu, May 30, 2024 at 2:10 PM Michael S. Tsirkin wrote:
>
> On Thu, May 30, 2024 at 11:20:55AM +0800, Jason Wang wrote:
> > This patch synchronize operstate with admin state per RFC2863.
> >
> > This is done by trying to toggle the carrier upon open/close and
> > synchronize with the config change work. This allows propagate status
> > correctly to stacked devices like:
> >
> > ip link add link enp0s3 macvlan0 type macvlan
> > ip link set link enp0s3 down
> > ip link show
> >
> > Before this patch:
> >
> > 3: enp0s3: mtu 1500 qdisc pfifo_fast state DOWN mode
> > DEFAULT group default qlen 1000
> > link/ether 00:00:05:00:00:09 brd ff:ff:ff:ff:ff:ff
> > ..
> > 5: macvlan0@enp0s3: mtu 1500 qdisc
> > noqueue state UP mode DEFAULT group default qlen 1000
> > link/ether b2:a9:c5:04:da:53 brd ff:ff:ff:ff:ff:ff
> >
> > After this patch:
> >
> > 3: enp0s3: mtu 1500 qdisc pfifo_fast state DOWN mode
> > DEFAULT group default qlen 1000
> > link/ether 00:00:05:00:00:09 brd ff:ff:ff:ff:ff:ff
> > ...
> > 5: macvlan0@enp0s3: mtu 1500
> > qdisc noqueue state LOWERLAYERDOWN mode DEFAULT group default qlen 1000
> > link/ether b2:a9:c5:04:da:53 brd ff:ff:ff:ff:ff:ff
> >
> > Cc: Venkat Venkatsubra
> > Cc: Gia-Khanh Nguyen
> > Reviewed-by: Xuan Zhuo
> > Acked-by: Michael S. Tsirkin
> > Signed-off-by: Jason Wang
> > ---
> > Changes since V1:
> > - rebase
> > - add ack/review tags
>
>
>
>
>
> > ---
> > drivers/net/virtio_net.c | 94 +++-
> > 1 file changed, 63 insertions(+), 31 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 4a802c0ea2cb..69e4ae353c51 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -433,6 +433,12 @@ struct virtnet_info {
> > /* The lock to synchronize the access to refill_enabled */
> > spinlock_t refill_lock;
> >
> > + /* Is config change enabled? */
> > + bool config_change_enabled;
> > +
> > + /* The lock to synchronize the access to config_change_enabled */
> > + spinlock_t config_change_lock;
> > +
> > /* Work struct for config space updates */
> > struct work_struct config_work;
> >
>
>
> But we already have dev->config_lock and dev->config_enabled.
>
> And it actually works better - instead of discarding config
> change events it defers them until enabled.
>
Yes but then both virtio-net driver and virtio core can ask to enable
and disable and then we need some kind of synchronization which is
non-trivial.
And device enabling on the core is different from bringing the device
up in the networking subsystem. Here we just delay to deal with the
config change interrupt on ndo_open(). (E.g try to ack announce is
meaningless when the device is down).
Thanks
[PATCH 1/3] remoteproc: k3-r5: Use devm_rproc_alloc() helper
Use the device lifecycle managed allocation function. This helps prevent
mistakes like freeing out of order in cleanup functions and forgetting
to free on error paths.
Signed-off-by: Beleswar Padhi
---
drivers/remoteproc/ti_k3_r5_remoteproc.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c
b/drivers/remoteproc/ti_k3_r5_remoteproc.c
index 50e486bcfa10..26362a509ae3 100644
--- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
@@ -1258,8 +1258,8 @@ static int k3_r5_cluster_rproc_init(struct
platform_device *pdev)
goto out;
}
- rproc = rproc_alloc(cdev, dev_name(cdev), _r5_rproc_ops,
- fw_name, sizeof(*kproc));
+ rproc = devm_rproc_alloc(cdev, dev_name(cdev), _r5_rproc_ops,
+fw_name, sizeof(*kproc));
if (!rproc) {
ret = -ENOMEM;
goto out;
@@ -1351,7 +1351,6 @@ static int k3_r5_cluster_rproc_init(struct
platform_device *pdev)
err_add:
k3_r5_reserved_mem_exit(kproc);
err_config:
- rproc_free(rproc);
core->rproc = NULL;
out:
/* undo core0 upon any failures on core1 in split-mode */
@@ -1398,7 +1397,6 @@ static void k3_r5_cluster_rproc_exit(void *data)
k3_r5_reserved_mem_exit(kproc);
- rproc_free(rproc);
core->rproc = NULL;
}
}
--
2.34.1
[PATCH 0/3] Defer TI's Remoteproc's Probe until Mailbox is Probed
Hello All, This series adds deferred probe functionality in the TI's Remoteproc drivers. The remoteproc drivers are dependent on the omap-mailbox driver for mbox functionalities. Sometimes, the remoteproc driver could be probed before the mailbox driver leading to rproc boot failures. Thus, defer the probe routine of remoteproc drivers until mailbox driver is probed by checking the mbox_request_channel handle in probe. Also, use the acquired mbox handle in probe during rproc start/attach routine instead of re-requesting. Do not free mbox handle during stop/detach routine or error paths. This makes our k3_rproc_attach() & k3_rproc_detach() functions NOP. Also, use the devm_rproc_alloc() helper to automatically free created rprocs incase of a probe defer. Beleswar Padhi (3): remoteproc: k3-r5: Use devm_rproc_alloc() helper remoteproc: k3-r5: Acquire mailbox handle during probe remoteproc: k3-dsp: Acquire mailbox handle during probe routine drivers/remoteproc/ti_k3_dsp_remoteproc.c | 67 +++-- drivers/remoteproc/ti_k3_r5_remoteproc.c | 72 --- 2 files changed, 44 insertions(+), 95 deletions(-) -- 2.34.1
[PATCH 3/3] remoteproc: k3-dsp: Acquire mailbox handle during probe routine
Acquire the mailbox handle during device probe and do not release handle
in stop/detach routine or error paths. This removes the redundant
requests for mbox handle later during rproc start/attach. This also
allows to defer remoteproc driver's probe if mailbox is not probed yet.
Signed-off-by: Beleswar Padhi
---
drivers/remoteproc/ti_k3_dsp_remoteproc.c | 67 +++
1 file changed, 21 insertions(+), 46 deletions(-)
diff --git a/drivers/remoteproc/ti_k3_dsp_remoteproc.c
b/drivers/remoteproc/ti_k3_dsp_remoteproc.c
index 3555b535b168..88cda609a5eb 100644
--- a/drivers/remoteproc/ti_k3_dsp_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_dsp_remoteproc.c
@@ -222,6 +222,7 @@ static int k3_dsp_rproc_request_mbox(struct rproc *rproc)
struct mbox_client *client = >client;
struct device *dev = kproc->dev;
int ret;
+ long err;
client->dev = dev;
client->tx_done = NULL;
@@ -231,10 +232,9 @@ static int k3_dsp_rproc_request_mbox(struct rproc *rproc)
kproc->mbox = mbox_request_channel(client, 0);
if (IS_ERR(kproc->mbox)) {
- ret = -EBUSY;
- dev_err(dev, "mbox_request_channel failed: %ld\n",
- PTR_ERR(kproc->mbox));
- return ret;
+ err = PTR_ERR(kproc->mbox);
+ dev_err(dev, "mbox_request_channel failed: %ld\n", err);
+ return (err == -EPROBE_DEFER) ? -EPROBE_DEFER : -EBUSY;
}
/*
@@ -315,31 +315,25 @@ static int k3_dsp_rproc_start(struct rproc *rproc)
u32 boot_addr;
int ret;
- ret = k3_dsp_rproc_request_mbox(rproc);
- if (ret)
- return ret;
-
boot_addr = rproc->bootaddr;
if (boot_addr & (kproc->data->boot_align_addr - 1)) {
dev_err(dev, "invalid boot address 0x%x, must be aligned on a
0x%x boundary\n",
boot_addr, kproc->data->boot_align_addr);
ret = -EINVAL;
- goto put_mbox;
+ goto out;
}
dev_err(dev, "booting DSP core using boot addr = 0x%x\n", boot_addr);
ret = ti_sci_proc_set_config(kproc->tsp, boot_addr, 0, 0);
if (ret)
- goto put_mbox;
+ goto out;
ret = k3_dsp_rproc_release(kproc);
if (ret)
- goto put_mbox;
+ goto out;
return 0;
-
-put_mbox:
- mbox_free_channel(kproc->mbox);
+out:
return ret;
}
@@ -353,8 +347,6 @@ static int k3_dsp_rproc_stop(struct rproc *rproc)
{
struct k3_dsp_rproc *kproc = rproc->priv;
- mbox_free_channel(kproc->mbox);
-
k3_dsp_rproc_reset(kproc);
return 0;
@@ -363,42 +355,21 @@ static int k3_dsp_rproc_stop(struct rproc *rproc)
/*
* Attach to a running DSP remote processor (IPC-only mode)
*
- * This rproc attach callback only needs to request the mailbox, the remote
- * processor is already booted, so there is no need to issue any TI-SCI
- * commands to boot the DSP core. This callback is invoked only in IPC-only
- * mode.
+ * This rproc attach callback is a NOP. The remote processor is already booted,
+ * and all required resources have been acquired during probe routine, so there
+ * is no need to issue any TI-SCI commands to boot the DSP core. This callback
+ * is invoked only in IPC-only mode and exists for sanity sake.
*/
-static int k3_dsp_rproc_attach(struct rproc *rproc)
-{
- struct k3_dsp_rproc *kproc = rproc->priv;
- struct device *dev = kproc->dev;
- int ret;
-
- ret = k3_dsp_rproc_request_mbox(rproc);
- if (ret)
- return ret;
-
- dev_info(dev, "DSP initialized in IPC-only mode\n");
- return 0;
-}
+static int k3_dsp_rproc_attach(struct rproc *rproc) { return 0; }
/*
* Detach from a running DSP remote processor (IPC-only mode)
*
- * This rproc detach callback performs the opposite operation to attach
callback
- * and only needs to release the mailbox, the DSP core is not stopped and will
- * be left to continue to run its booted firmware. This callback is invoked
only
- * in IPC-only mode.
+ * This rproc detach callback is a NOP. The DSP core is not stopped and will be
+ * left to continue to run its booted firmware. This callback is invoked only
in
+ * IPC-only mode and exists for sanity sake.
*/
-static int k3_dsp_rproc_detach(struct rproc *rproc)
-{
- struct k3_dsp_rproc *kproc = rproc->priv;
- struct device *dev = kproc->dev;
-
- mbox_free_channel(kproc->mbox);
- dev_info(dev, "DSP deinitialized in IPC-only mode\n");
- return 0;
-}
+static int k3_dsp_rproc_detach(struct rproc *rproc) { return 0; }
/*
* This function implements the .get_loaded_rsc_table() callback and is used
@@ -697,6 +668,10 @@ static int k3_dsp_rproc_probe(struct platform_device *pdev)
kproc->dev = dev;
kproc->data = data;
+ ret = k3_dsp_rproc_request_mbox(rproc);
+
[PATCH 2/3] remoteproc: k3-r5: Acquire mailbox handle during probe
Acquire the mailbox handle during device probe and do not release handle
in stop/detach routine or error paths. This removes the redundant
requests for mbox handle later during rproc start/attach. This also
allows to defer remoteproc driver's probe if mailbox is not probed yet.
Signed-off-by: Beleswar Padhi
---
drivers/remoteproc/ti_k3_r5_remoteproc.c | 66
1 file changed, 21 insertions(+), 45 deletions(-)
diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c
b/drivers/remoteproc/ti_k3_r5_remoteproc.c
index 26362a509ae3..157e8fd57665 100644
--- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
@@ -391,6 +391,7 @@ static int k3_r5_rproc_request_mbox(struct rproc *rproc)
struct mbox_client *client = >client;
struct device *dev = kproc->dev;
int ret;
+ long err;
client->dev = dev;
client->tx_done = NULL;
@@ -400,10 +401,9 @@ static int k3_r5_rproc_request_mbox(struct rproc *rproc)
kproc->mbox = mbox_request_channel(client, 0);
if (IS_ERR(kproc->mbox)) {
- ret = -EBUSY;
- dev_err(dev, "mbox_request_channel failed: %ld\n",
- PTR_ERR(kproc->mbox));
- return ret;
+ err = PTR_ERR(kproc->mbox);
+ dev_err(dev, "mbox_request_channel failed: %ld\n", err);
+ return (err == -EPROBE_DEFER) ? -EPROBE_DEFER : -EBUSY;
}
/*
@@ -552,10 +552,6 @@ static int k3_r5_rproc_start(struct rproc *rproc)
u32 boot_addr;
int ret;
- ret = k3_r5_rproc_request_mbox(rproc);
- if (ret)
- return ret;
-
boot_addr = rproc->bootaddr;
/* TODO: add boot_addr sanity checking */
dev_dbg(dev, "booting R5F core using boot addr = 0x%x\n", boot_addr);
@@ -564,7 +560,7 @@ static int k3_r5_rproc_start(struct rproc *rproc)
core = kproc->core;
ret = ti_sci_proc_set_config(core->tsp, boot_addr, 0, 0);
if (ret)
- goto put_mbox;
+ goto out;
/* unhalt/run all applicable cores */
if (cluster->mode == CLUSTER_MODE_LOCKSTEP) {
@@ -581,12 +577,12 @@ static int k3_r5_rproc_start(struct rproc *rproc)
dev_err(dev, "%s: can not start core 1 before core 0\n",
__func__);
ret = -EPERM;
- goto put_mbox;
+ goto out;
}
ret = k3_r5_core_run(core);
if (ret)
- goto put_mbox;
+ goto out;
}
return 0;
@@ -596,8 +592,7 @@ static int k3_r5_rproc_start(struct rproc *rproc)
if (k3_r5_core_halt(core))
dev_warn(core->dev, "core halt back failed\n");
}
-put_mbox:
- mbox_free_channel(kproc->mbox);
+out:
return ret;
}
@@ -658,8 +653,6 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
goto out;
}
- mbox_free_channel(kproc->mbox);
-
return 0;
unroll_core_halt:
@@ -674,42 +667,21 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
/*
* Attach to a running R5F remote processor (IPC-only mode)
*
- * The R5F attach callback only needs to request the mailbox, the remote
- * processor is already booted, so there is no need to issue any TI-SCI
- * commands to boot the R5F cores in IPC-only mode. This callback is invoked
- * only in IPC-only mode.
+ * The R5F attach callback is a NOP. The remote processor is already booted,
and
+ * all required resources have been acquired during probe routine, so there is
+ * no need to issue any TI-SCI commands to boot the R5F cores in IPC-only mode.
+ * This callback is invoked only in IPC-only mode and exists for sanity sake.
*/
-static int k3_r5_rproc_attach(struct rproc *rproc)
-{
- struct k3_r5_rproc *kproc = rproc->priv;
- struct device *dev = kproc->dev;
- int ret;
-
- ret = k3_r5_rproc_request_mbox(rproc);
- if (ret)
- return ret;
-
- dev_info(dev, "R5F core initialized in IPC-only mode\n");
- return 0;
-}
+static int k3_r5_rproc_attach(struct rproc *rproc) { return 0; }
/*
* Detach from a running R5F remote processor (IPC-only mode)
*
- * The R5F detach callback performs the opposite operation to attach callback
- * and only needs to release the mailbox, the R5F cores are not stopped and
- * will be left in booted state in IPC-only mode. This callback is invoked
- * only in IPC-only mode.
+ * The R5F detach callback is a NOP. The R5F cores are not stopped and will be
+ * left in booted state in IPC-only mode. This callback is invoked only in
+ * IPC-only mode and exists for sanity sake.
*/
-static int k3_r5_rproc_detach(struct rproc *rproc)
-{
- struct k3_r5_rproc *kproc = rproc->priv;
- struct device *dev = kproc->dev;
-
-
Re: [PATCH] mctp i2c: Add rx trace
Hi Tal, > > > mctp-i2c rx implementation doesn't call > > > __i2c_transfer which calls the i2c reply trace function. > > > > No, but we can trace the i2c rx path through the trace_i2c_slave > > tracepoint. It is a little messier than tracing trace_i2c_write, > > but > > has been sufficient with the debugging I've needed in the past. > > Oh, I missed that. > I had to test it with an older kernel without i2c_slave tracing > so I looked only at the regular i2c and mctp trace paths. OK! That tracepoint was (coincidentally) added in 5.18, same as the MCTP-over-i2c transport. So we should have coverage for both features on upstream kernels, at least. > > > Add an mctp_reply trace function that will be used instead. > > > > Can you elaborate a little on what you were/are looking to inspect > > here? (mainly: which packet fields are you interested in?) That > > will > > help to determine the best approach here. > > Sure, I basically wanted to trace the i2c packet buffer in a simple > way. OK - did you specifically need the i2c transport headers? Since the MCTP interfaces are regular net devices, the easiest way to trace generic MCTP transfers is generally via a packet capture (tcpdump, wireshark, etc). Cheers, Jeremy
Re: [PATCH v5 5/7] remoteproc: core: support of the tee interface
Hello Mathieu,
On 5/29/24 22:35, Mathieu Poirier wrote:
> On Wed, May 29, 2024 at 09:13:26AM +0200, Arnaud POULIQUEN wrote:
>> Hello Mathieu,
>>
>> On 5/28/24 23:30, Mathieu Poirier wrote:
>>> On Tue, May 21, 2024 at 10:09:59AM +0200, Arnaud Pouliquen wrote:
1) on start:
- Using the TEE loader, the resource table is loaded by an external entity.
In such case the resource table address is not find from the firmware but
provided by the TEE remoteproc framework.
Use the rproc_get_loaded_rsc_table instead of rproc_find_loaded_rsc_table
- test that rproc->cached_table is not null before performing the memcpy
2)on stop
The use of the cached_table seems mandatory:
- during recovery sequence to have a snapshot of the resource table
resources used,
- on stop to allow for the deinitialization of resources after the
the remote processor has been shutdown.
However if the TEE interface is being used, we first need to unmap the
table_ptr before setting it to rproc->cached_table.
The update of rproc->table_ptr to rproc->cached_table is performed in
tee_remoteproc.
Signed-off-by: Arnaud Pouliquen
---
drivers/remoteproc/remoteproc_core.c | 31 +---
1 file changed, 23 insertions(+), 8 deletions(-)
diff --git a/drivers/remoteproc/remoteproc_core.c
b/drivers/remoteproc/remoteproc_core.c
index 42bca01f3bde..3a642151c983 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -1267,6 +1267,7 @@ EXPORT_SYMBOL(rproc_resource_cleanup);
static int rproc_set_rsc_table_on_start(struct rproc *rproc, const struct
firmware *fw)
{
struct resource_table *loaded_table;
+ struct device *dev = >dev;
/*
* The starting device has been given the rproc->cached_table as the
@@ -1276,12 +1277,21 @@ static int rproc_set_rsc_table_on_start(struct
rproc *rproc, const struct firmwa
* this information to device memory. We also update the table_ptr so
* that any subsequent changes will be applied to the loaded version.
*/
- loaded_table = rproc_find_loaded_rsc_table(rproc, fw);
- if (loaded_table) {
- memcpy(loaded_table, rproc->cached_table, rproc->table_sz);
- rproc->table_ptr = loaded_table;
+ if (rproc->tee_interface) {
+ loaded_table = rproc_get_loaded_rsc_table(rproc,
>table_sz);
+ if (IS_ERR(loaded_table)) {
+ dev_err(dev, "can't get resource table\n");
+ return PTR_ERR(loaded_table);
+ }
+ } else {
+ loaded_table = rproc_find_loaded_rsc_table(rproc, fw);
}
+ if (loaded_table && rproc->cached_table)
+ memcpy(loaded_table, rproc->cached_table, rproc->table_sz);
+
>>>
>>> Why is this not part of the else {} above as it was the case before this
>>> patch?
>>> And why was an extra check for ->cached_table added?
>>
>> Here we have to cover 2 use cases if rproc->tee_interface is set.
>> 1) The remote processor is in stop state
>> - loaded_table points to the resource table in the remote memory and
>> - rproc->cached_table is null
>> => no memcopy
>> 2) crash recovery
>> - loaded_table points to the resource table in the remote memory
>> - rproc-cached_table point to a copy of the resource table
>
> A cached_table exists because it was created in
> rproc_reset_rsc_table_on_stop().
> But as the comment says [1], that part of the code was meant to be used for
> the
> attach()/detach() use case. Mixing both will become extremely confusing and
> impossible to maintain.
i am not sure to understand your point here... the cached_table table was
already existing for the "normal" case[2]. Seems to me that the cache table is
needed on stop in all scenarios.
[2]
https://elixir.bootlin.com/linux/v4.20.17/source/drivers/remoteproc/remoteproc_core.c#L1402
>
> I think the TEE scenario should be as similar as the "normal" one where TEE is
> not involved. To that end, I suggest to create a cached_table in
> tee_rproc_parse_fw(), exactly the same way it is done in
> rproc_elf_load_rsc_table(). That way the code path in
> rproc_set_rsc_table_on_start() become very similar and we have a cached_table
> to
> work with when the remote processor is recovered. In fact we may not need
> rproc_set_rsc_table_on_start() at all but that needs to be asserted.
This is was I proposed in my V4 [3]. Could you please confirm that this aligns
with what you have in mind?
In such a case, should I keep the updates below in
rproc_reset_rsc_table_on_stop(), or should I revert to using rproc->rsc_table to
store the pointer to the resource table in tee_remoteproc for the associated
memory map/unmap?"
[3]
Re: [PATCH net-next V2] virtio-net: synchronize operstate with admin state on up/down
On Thu, May 30, 2024 at 11:20:55AM +0800, Jason Wang wrote:
> This patch synchronize operstate with admin state per RFC2863.
>
> This is done by trying to toggle the carrier upon open/close and
> synchronize with the config change work. This allows propagate status
> correctly to stacked devices like:
>
> ip link add link enp0s3 macvlan0 type macvlan
> ip link set link enp0s3 down
> ip link show
>
> Before this patch:
>
> 3: enp0s3: mtu 1500 qdisc pfifo_fast state DOWN mode
> DEFAULT group default qlen 1000
> link/ether 00:00:05:00:00:09 brd ff:ff:ff:ff:ff:ff
> ..
> 5: macvlan0@enp0s3: mtu 1500 qdisc
> noqueue state UP mode DEFAULT group default qlen 1000
> link/ether b2:a9:c5:04:da:53 brd ff:ff:ff:ff:ff:ff
>
> After this patch:
>
> 3: enp0s3: mtu 1500 qdisc pfifo_fast state DOWN mode
> DEFAULT group default qlen 1000
> link/ether 00:00:05:00:00:09 brd ff:ff:ff:ff:ff:ff
> ...
> 5: macvlan0@enp0s3: mtu 1500 qdisc
> noqueue state LOWERLAYERDOWN mode DEFAULT group default qlen 1000
> link/ether b2:a9:c5:04:da:53 brd ff:ff:ff:ff:ff:ff
>
> Cc: Venkat Venkatsubra
> Cc: Gia-Khanh Nguyen
> Reviewed-by: Xuan Zhuo
> Acked-by: Michael S. Tsirkin
> Signed-off-by: Jason Wang
> ---
> Changes since V1:
> - rebase
> - add ack/review tags
> ---
> drivers/net/virtio_net.c | 94 +++-
> 1 file changed, 63 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 4a802c0ea2cb..69e4ae353c51 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -433,6 +433,12 @@ struct virtnet_info {
> /* The lock to synchronize the access to refill_enabled */
> spinlock_t refill_lock;
>
> + /* Is config change enabled? */
> + bool config_change_enabled;
> +
> + /* The lock to synchronize the access to config_change_enabled */
> + spinlock_t config_change_lock;
> +
> /* Work struct for config space updates */
> struct work_struct config_work;
>
But we already have dev->config_lock and dev->config_enabled.
And it actually works better - instead of discarding config
change events it defers them until enabled.
> @@ -623,6 +629,20 @@ static void disable_delayed_refill(struct virtnet_info
> *vi)
> spin_unlock_bh(>refill_lock);
> }
>
> +static void enable_config_change(struct virtnet_info *vi)
> +{
> + spin_lock_irq(>config_change_lock);
> + vi->config_change_enabled = true;
> + spin_unlock_irq(>config_change_lock);
> +}
> +
> +static void disable_config_change(struct virtnet_info *vi)
> +{
> + spin_lock_irq(>config_change_lock);
> + vi->config_change_enabled = false;
> + spin_unlock_irq(>config_change_lock);
> +}
> +
> static void enable_rx_mode_work(struct virtnet_info *vi)
> {
> rtnl_lock();
> @@ -2421,6 +2441,25 @@ static int virtnet_enable_queue_pair(struct
> virtnet_info *vi, int qp_index)
> return err;
> }
>
> +static void virtnet_update_settings(struct virtnet_info *vi)
> +{
> + u32 speed;
> + u8 duplex;
> +
> + if (!virtio_has_feature(vi->vdev, VIRTIO_NET_F_SPEED_DUPLEX))
> + return;
> +
> + virtio_cread_le(vi->vdev, struct virtio_net_config, speed, );
> +
> + if (ethtool_validate_speed(speed))
> + vi->speed = speed;
> +
> + virtio_cread_le(vi->vdev, struct virtio_net_config, duplex, );
> +
> + if (ethtool_validate_duplex(duplex))
> + vi->duplex = duplex;
> +}
> +
> static int virtnet_open(struct net_device *dev)
> {
> struct virtnet_info *vi = netdev_priv(dev);
> @@ -2439,6 +2478,18 @@ static int virtnet_open(struct net_device *dev)
> goto err_enable_qp;
> }
>
> + /* Assume link up if device can't report link status,
> +otherwise get link status from config. */
> + netif_carrier_off(dev);
> + if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_STATUS)) {
> + enable_config_change(vi);
> + schedule_work(>config_work);
> + } else {
> + vi->status = VIRTIO_NET_S_LINK_UP;
> + virtnet_update_settings(vi);
> + netif_carrier_on(dev);
> + }
> +
> return 0;
>
> err_enable_qp:
> @@ -2875,12 +2926,19 @@ static int virtnet_close(struct net_device *dev)
> disable_delayed_refill(vi);
> /* Make sure refill_work doesn't re-enable napi! */
> cancel_delayed_work_sync(>refill);
> + /* Make sure config notification doesn't schedule config work */
> + disable_config_change(vi);
> + /* Make sure status updating is cancelled */
> + cancel_work_sync(>config_work);
>
> for (i = 0; i < vi->max_queue_pairs; i++) {
> virtnet_disable_queue_pair(vi, i);
> cancel_work_sync(>rq[i].dim.work);
> }
>
> + vi->status &= ~VIRTIO_NET_S_LINK_UP;
> + netif_carrier_off(dev);
> +
> return 0;
> }
>
> @@ -4583,25 +4641,6 @@ static void
