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

[Datta, Shubhrajyoti]

Public

 > -----Original Message-----
 > From: Borislav Petkov <bp@xxxxxxxxx>
 > Sent: Thursday, May 14, 2026 5:05 PM
 > To: Datta, Shubhrajyoti <shubhrajyoti.datta@xxxxxxx>
 > Cc: linux-kernel@xxxxxxxxxxxxxxx; linux-edac@xxxxxxxxxxxxxxx; git
(AMD-Xilinx)
 > <git@xxxxxxx>; shubhrajyoti.datta@xxxxxxxxx; Tony Luck
 > <tony.luck@xxxxxxxxx>; Potthuri, Sai Krishna
<sai.krishna.potthuri@xxxxxxx>
 > Subject: Re: [PATCH v3] EDAC/versalnet: Report PFN and page offset
for DDR
...
 >
 > https://sashiko.dev/#/patchset/20260511103529.1853270-1-
 > shubhrajyoti.datta%40amd.com
 >
Hi Boris,
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.
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.

  Finding 3: memory_failure() with MF_ACTION_REQUIRED from async context


Sent below patch.
https://lore.kernel.org/all/20260701095040.4139675-1-shubhrajyoti.datta@xxxxxxx/

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.

Also the cdx code holds a mutex
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/cdx/controller/cdx_rpmsg.c?h=v7.2-rc1#n106

cdx_rpmsg_cb -> cdx_mcdi_process_cmd -> holds mutex

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/cdx/controller/mcdi.c?h=v7.2-rc1#n611

This looks like a  a false positive.


 > --
 > Regards/Gruss,
 >     Boris.
 >
 > https://people.kernel.org/tglx/notes-about-netiquette