Re: [PATCH v3] EDAC/versalnet: Report PFN and page offset for DDR errors

On Fri, Jul 10, 2026 at 01:12:17PM +0000, Datta, Shubhrajyoti wrote:
> I have reviewed all the Sashiko comments and identified the valid ones
> (details below).
> 
> Finding 1: Missing null check for
> 
> Comment is accepted will send a patch.

Where is that patch?

> Finding 2: Uninitialized ueinfo when CE and UE flags set simultaneously -- Mostly a false positive
> 
> Looking at rpmsg_cb, CE (error_id 18) and UE (error_id 19) arrive as separate rpmsg messages with distinct error_id values.
> The memset(p, 0, ...) at the top of each case clears the entire ecc_status
>   struct, and p->error_type is set to exactly one of MC5_ERR_TYPE_CE or MC5_ERR_TYPE_UE.
> The get_ddr_info() function then uses if/else if on ISR bits, but the ISR comes from the firmware message for
>   that specific error_id, so only one type should be populated per invocation.

Ok, you can put a comment above get_ddr_info() explaining that it is ok to
alternate between the error severities.

>   Finding 3: memory_failure() with MF_ACTION_REQUIRED from async context
> 
> 
> Sent below patch.
> https://lore.kernel.org/all/[email protected]/

Fixes: tag?

Does it need to go to stable?

Also, you'd need to send me fixes for the issues reported there.

> Regarding the context: rpmsg_cb is a virtio rpmsg callback. Looking at the virtio rpmsg transport, these callbacks run from
> a workqueue (virtio_rpmsg_bus.c uses rpmsg_recv_single() from
>   rpmsg_recv_done() which is a virtqueue callback typically dispatched via a workqueue),
> so it's not hardirq context. memory_failure() does take mf_mutex which can sleep, and this should be fine from a
>   workqueue.

Ok.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette