linux-kernel - Re: [PATCH v7 0/5] Add support for debugfs based RAS DES feature in PCIe DW

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z7yniizCTdBvUBI0@ryzen>
Date: Mon, 24 Feb 2025 18:08:26 +0100
From: Niklas Cassel <cassel@...nel.org>
To: Shradha Todi <shradha.t@...sung.com>
Cc: linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org,
	linux-perf-users@...r.kernel.org, manivannan.sadhasivam@...aro.org,
	lpieralisi@...nel.org, kw@...ux.com, robh@...nel.org,
	bhelgaas@...gle.com, jingoohan1@...il.com,
	Jonathan.Cameron@...wei.com, fan.ni@...sung.com,
	nifan.cxl@...il.com, a.manzanares@...sung.com,
	pankaj.dubey@...sung.com, 18255117159@....com,
	xueshuai@...ux.alibaba.com, renyu.zj@...ux.alibaba.com,
	will@...nel.org, mark.rutland@....com
Subject: Re: [PATCH v7 0/5] Add support for debugfs based RAS DES feature in
 PCIe DW

Hello Shradha,

On Fri, Feb 21, 2025 at 06:45:43PM +0530, Shradha Todi wrote:
> DesignWare controller provides a vendor specific extended capability
> called RASDES as an IP feature. This extended capability  provides
> hardware information like:
>  - Debug registers to know the state of the link or controller. 
>  - Error injection mechanisms to inject various PCIe errors including
>    sequence number, CRC
>  - Statistical counters to know how many times a particular event
>    occurred
> 
> However, in Linux we do not have any generic or custom support to be
> able to use this feature in an efficient manner. This is the reason we
> are proposing this framework. Debug and bring up time of high-speed IPs
> are highly dependent on costlier hardware analyzers and this solution
> will in some ways help to reduce the HW analyzer usage.
> 
> The debugfs entries can be used to get information about underlying
> hardware and can be shared with user space. Separate debugfs entries has
> been created to cater to all the DES hooks provided by the controller.
> The debugfs entries interacts with the RASDES registers in the required
> sequence and provides the meaningful data to the user. This eases the
> effort to understand and use the register information for debugging.
> 
> This series creates a generic debugfs framework for DesignWare PCIe
> controllers where other debug features apart from RASDES can also be
> added as and when required.
> 
> v7:
>     - Moved the patches to make finding VSEC IDs common from Mani's patchset [1]
>       into this series to remove dependancy as discussed
>     - Addressed style related change requests from v6

I tested this series, and one thing that I noticed:

# for f in /sys/kernel/debug/dwc_pcie_a40000000.pcie/rasdes_event_counter/*/counter_enable; do echo 1 > $f; done

# grep "" /sys/kernel/debug/dwc_pcie_a40000000.pcie/rasdes_event_counter/*/* | grep Disabled
/sys/kernel/debug/dwc_pcie_a40000000.pcie/rasdes_event_counter/ctl_skp_os_parity_err/counter_enable:Counter Disabled
/sys/kernel/debug/dwc_pcie_a40000000.pcie/rasdes_event_counter/deskew_uncompleted_err/counter_enable:Counter Disabled
/sys/kernel/debug/dwc_pcie_a40000000.pcie/rasdes_event_counter/framing_err_in_l0/counter_enable:Counter Disabled
/sys/kernel/debug/dwc_pcie_a40000000.pcie/rasdes_event_counter/margin_crc_parity_err/counter_enable:Counter Disabled
/sys/kernel/debug/dwc_pcie_a40000000.pcie/rasdes_event_counter/retimer_parity_err_1st/counter_enable:Counter Disabled
/sys/kernel/debug/dwc_pcie_a40000000.pcie/rasdes_event_counter/retimer_parity_err_2nd/counter_enable:Counter Disabled

that there are some events that cannot be enabled when testing on my platform,
rk3588, perhaps this is because my version of the DWC IP does not have these
events.

(Because all the other events can be enabled successfully:
# grep "" /sys/kernel/debug/dwc_pcie_a40000000.pcie/rasdes_event_counter/*/* | grep Enabled | wc -l
29
)


So the question is, how do we want to handle that?

E.g. counter_enable_write() could theoretically read back the
dw_pcie_readl_dbi(pci, rinfo->ras_cap_offset + RAS_DES_EVENT_COUNTER_CTRL_REG);
register after doing the
ww_pcie_writel_dbi(pci, rinfo->ras_cap_offset + RAS_DES_EVENT_COUNTER_CTRL_REG, val);

to actually check if it could enable the event.

If counter_enable_write() could not enable the specific event, should it
perhaps return a failure to user space?

Or, do we want to keep the current behavior of just letting counter_enable_write()
return success, even for events that are not supported by the specific DWC PCIe
implementation?


Kind regards,
Niklas