linux-kernel - Re: [PATCH v5 2/4] Add debugfs based silicon debug support in DWC

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: <Z5DKOHiPd-yc7MCc@ryzen>
Date: Wed, 22 Jan 2025 11:36:40 +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,
	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, a.manzanares@...sung.com,
	pankaj.dubey@...sung.com, quic_nitegupt@...cinc.com,
	quic_krichai@...cinc.com, gost.dev@...sung.com
Subject: Re: [PATCH v5 2/4] Add debugfs based silicon debug support in DWC

Hello Shradha,

On Tue, Jan 21, 2025 at 04:44:19PM +0530, Shradha Todi wrote:

This is the suggested format of your new feature:

> +
> +struct dwc_pcie_vendor_id {
> +	u16 vendor_id;
> +	u16 vsec_rasdes_cap_id;
> +};
> +
> +static const struct dwc_pcie_vendor_id dwc_pcie_vendor_ids[] = {
> +	{PCI_VENDOR_ID_SAMSUNG,	0x2},
> +	{} /* terminator */
> +};


You might know of the drivers/perf driver which also exposes RAS information
for DWC. That driver uses the following format for supported entries:

+struct dwc_pcie_pmu_vsec_id {
+	u16 vendor_id;
+	u16 vsec_id;
+	u8 vsec_rev;
 };

+/*
+ * VSEC IDs are allocated by the vendor, so a given ID may mean different
+ * things to different vendors.  See PCIe r6.0, sec 7.9.5.2.
+ */
+static const struct dwc_pcie_pmu_vsec_id dwc_pcie_pmu_vsec_ids[] = {
+	{ .vendor_id = PCI_VENDOR_ID_ALIBABA,
+	  .vsec_id = 0x02, .vsec_rev = 0x4 },
+	{ .vendor_id = PCI_VENDOR_ID_AMPERE,
+	  .vsec_id = 0x02, .vsec_rev = 0x4 },
+	{ .vendor_id = PCI_VENDOR_ID_QCOM,
+	  .vsec_id = 0x02, .vsec_rev = 0x4 },
 	{} /* terminator */
 };

See:
https://lore.kernel.org/all/20241209222938.3219364-1-helgaas@kernel.org/


I think it would be a good idea for your feature to use the exact same
format for supported entries, so that entries can simply be copy pasted
between the two drivers.

(Considering that both of these drivers are simply exposing the RAS
information in different ways, having an entry in one of the two drivers
should mean that that entry should work/be applicable for the other
driver as well.)


You might also want to add support for Samsung in the drivers/perf driver.


Kind regards,
Niklas