Re: [PATCH v5] riscv: mm: Trace TLB flush requests

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] riscv: mm: unaligned cpumask_weight() access in riscv_tlb_flush_request
--

commit d6aa8f46821c6cdff87980399804155c8220831a
Author: Roman 'Hedin' Storozhenko <romeusmeister@gmail.com>

riscv: mm: Trace TLB flush requests

This commit adds the riscv_tlb:riscv_tlb_flush_request tracepoint in
flush_tlb_all() and __flush_tlb_range() to record TLB invalidation
requests along with range, stride, ASID, target CPU mask, and derived
invalidation scope.

> diff --git a/arch/riscv/include/asm/riscv_tlb.h b/arch/riscv/include/asm/riscv_tlb.h
> new file mode 100644
> index 0000000000000..2c438f6bc0d42
> --- /dev/null
> +++ b/arch/riscv/include/asm/riscv_tlb.h
> @@ -0,0 +1,103 @@

[ ... ]

> +TRACE_EVENT(riscv_tlb_flush_request,
> +	    TP_PROTO(unsigned long start, unsigned long size,
> +		     unsigned long stride, unsigned long asid, bool has_mm,
> +		     const struct cpumask *cmask,
> +		     enum riscv_tlb_flush_scope scope),
> +
> +	TP_ARGS(start, size, stride, asid, has_mm, cmask, scope),
> +
> +	TP_STRUCT__entry(__field(unsigned long, start)
> +		__field(unsigned long, size)
> +		__field(unsigned long, stride)
> +		__field(unsigned long, asid)
> +		__field(bool, has_mm)
> +		__field(unsigned int, target_mask_weight)
> +		__cpumask(target_cpus)
> +		__field(u8, scope)
> +	),
> +
> +	TP_fast_assign(__entry->start = start;
> +		__entry->size = size;
> +		__entry->stride = stride;
> +		__entry->asid = asid;
> +		__entry->has_mm = has_mm;
> +		__assign_cpumask(target_cpus, cpumask_bits(cmask));
> +		__entry->target_mask_weight = cpumask_weight((const struct cpumask *)
> +						     __get_cpumask(target_cpus));
> +		__entry->scope = scope;
> +	),

[Severity: High]
Does casting __get_cpumask(target_cpus) to const struct cpumask * and
passing it to cpumask_weight() perform unaligned unsigned long loads?

In DECLARE_EVENT_CLASS() in include/trace/trace_events.h, char __data[]
immediately follows the fields declared in TP_STRUCT__entry():

    struct trace_event_raw_##name {
        struct trace_entry  ent;
        tstruct
        char                __data[];
    };

Because __field(u8, scope) is placed at the end of TP_STRUCT__entry(),
offsetof(struct trace_event_raw_riscv_tlb_flush_request, __data) is 53 on
RV64 (and 37 on RV32). In include/trace/stages/stage5_get_offsets.h,
__dynamic_array() places target_cpus at __data_size +
offsetof(typeof(*entry), __data) without aligning the offset to
sizeof(unsigned long). (Even without the trailing u8 field, the offset on
RV64 would be 52, and ring_buffer_event_data() only guarantees 4-byte
alignment rather than 8-byte alignment.)

When riscv_tlb_flush_request is enabled and triggered via flush_tlb_all()
or __flush_tlb_range(), TP_fast_assign() copies cmask into that unaligned
offset with memcpy() and then calls:

    cpumask_weight((const struct cpumask *)__get_cpumask(target_cpus))
      -> bitmap_weight(cpumask_bits(srcp), small_cpumask_bits)

which dereferences const unsigned long * from an odd byte address. In
addition to triggering UBSAN alignment warnings under
CONFIG_UBSAN_ALIGNMENT, on RISC-V systems without hardware unaligned
access support this raises a misaligned load trap on every tracepoint hit
(and during TP_printk() formatting via trace_print_bitmask_seq()), causing
a kernel oops in do_trap_misaligned() if handle_scalar_misaligned_load()
is disabled (CONFIG_RISCV_SCALAR_MISALIGNED=n or unaligned_enabled=0).

[ ... ]

> +	TP_printk("start=%#lx size=%#lx stride=%#lx asid=%#lx has_mm=%d target_mask_weight=%u target_cpus=%s scope=%s",
> +		  __entry->start, __entry->size, __entry->stride,
> +		  __entry->asid, __entry->has_mm,
> +		  __entry->target_mask_weight, __get_cpumask(target_cpus),
> +		  show_riscv_tlb_flush_scope(__entry->scope))
> +);

[ ... ]

> -- 
> 2.47.3

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261017-tlb_tracepoint-v5-1-256fb5393a7f@gmail.com?part=1