[PATCH v12 18/20] PCI/AER: Unmask RCEC internal errors to enable RCH downstream port error handling

[Robert Richter]

AER corrected and uncorrectable internal errors (CIE/UIE) are masked
in their corresponding mask registers per default once in power-up
state. [1][2] Enable internal errors for RCECs to receive CXL
downstream port errors of Restricted CXL Hosts (RCHs).

[1] CXL 3.0 Spec, 12.2.1.1 - RCH Downstream Port Detected Errors
[2] PCIe Base Spec r6.0, 7.8.4.3 Uncorrectable Error Mask Register,
7.8.4.6 Correctable Error Mask Register

Co-developed-by: Terry Bowman 
Signed-off-by: Terry Bowman 
Signed-off-by: Robert Richter 
Acked-by: Bjorn Helgaas 
Reviewed-by: Jonathan Cameron 
Reviewed-by: Dave Jiang 
---
 drivers/pci/pcie/aer.c | 57 ++
 1 file changed, 57 insertions(+)

diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index f1e8494f5bb6..41076cb2956e 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -936,6 +936,30 @@ static bool find_source_device(struct pci_dev *parent,
 
 #ifdef CONFIG_PCIEAER_CXL
 
+/**
+ * pci_aer_unmask_internal_errors - unmask internal errors
+ * @dev: pointer to the pcie_dev data structure
+ *
+ * Unmasks internal errors in the Uncorrectable and Correctable Error
+ * Mask registers.
+ *
+ * Note: AER must be enabled and supported by the device which must be
+ * checked in advance, e.g. with pcie_aer_is_native().
+ */
+static void pci_aer_unmask_internal_errors(struct pci_dev *dev)
+{
+   int aer = dev->aer_cap;
+   u32 mask;
+
+   pci_read_config_dword(dev, aer + PCI_ERR_UNCOR_MASK, );
+   mask &= ~PCI_ERR_UNC_INTN;
+   pci_write_config_dword(dev, aer + PCI_ERR_UNCOR_MASK, mask);
+
+   pci_read_config_dword(dev, aer + PCI_ERR_COR_MASK, );
+   mask &= ~PCI_ERR_COR_INTERNAL;
+   pci_write_config_dword(dev, aer + PCI_ERR_COR_MASK, mask);
+}
+
 static bool is_cxl_mem_dev(struct pci_dev *dev)
 {
/*
@@ -1012,7 +1036,39 @@ static void cxl_rch_handle_error(struct pci_dev *dev, 
struct aer_err_info *info)
pcie_walk_rcec(dev, cxl_rch_handle_error_iter, info);
 }
 
+static int handles_cxl_error_iter(struct pci_dev *dev, void *data)
+{
+   bool *handles_cxl = data;
+
+   if (!*handles_cxl)
+   *handles_cxl = is_cxl_mem_dev(dev) && cxl_error_is_native(dev);
+
+   /* Non-zero terminates iteration */
+   return *handles_cxl;
+}
+
+static bool handles_cxl_errors(struct pci_dev *rcec)
+{
+   bool handles_cxl = false;
+
+   if (pci_pcie_type(rcec) == PCI_EXP_TYPE_RC_EC &&
+   pcie_aer_is_native(rcec))
+   pcie_walk_rcec(rcec, handles_cxl_error_iter, _cxl);
+
+   return handles_cxl;
+}
+
+static void cxl_rch_enable_rcec(struct pci_dev *rcec)
+{
+   if (!handles_cxl_errors(rcec))
+   return;
+
+   pci_aer_unmask_internal_errors(rcec);
+   pci_info(rcec, "CXL: Internal errors unmasked");
+}
+
 #else
+static inline void cxl_rch_enable_rcec(struct pci_dev *dev) { }
 static inline void cxl_rch_handle_error(struct pci_dev *dev,
struct aer_err_info *info) { }
 #endif
@@ -1412,6 +1468,7 @@ static int aer_probe(struct pcie_device *dev)
return status;
}
 
+   cxl_rch_enable_rcec(port);
aer_enable_rootport(rpc);
pci_info(port, "enabled with IRQ %d\n", dev->irq);
return 0;
-- 
2.30.2

[PATCH v12 17/20] PCI/AER: Forward RCH downstream port-detected errors to the CXL.mem dev handler

[Robert Richter]

In Restricted CXL Device (RCD) mode a CXL device is exposed as an
RCiEP, but CXL downstream and upstream ports are not enumerated and
not visible in the PCIe hierarchy. [1] Protocol and link errors from
these non-enumerated ports are signaled as internal AER errors, either
Uncorrectable Internal Error (UIE) or Corrected Internal Errors (CIE)
via an RCEC.

Restricted CXL host (RCH) downstream port-detected errors have the
Requester ID of the RCEC set in the RCEC's AER Error Source ID
register. A CXL handler must then inspect the error status in various
CXL registers residing in the dport's component register space (CXL
RAS capability) or the dport's RCRB (PCIe AER extended
capability). [2]

Errors showing up in the RCEC's error handler must be handled and
connected to the CXL subsystem. Implement this by forwarding the error
to all CXL devices below the RCEC. Since the entire CXL device is
controlled only using PCIe Configuration Space of device 0, function
0, only pass it there [3]. The error handling is limited to currently
supported devices with the Memory Device class code set (CXL Type 3
Device, PCI_CLASS_MEMORY_CXL, 502h), handle downstream port errors in
the device's cxl_pci driver. Support for other CXL Device Types
(e.g. a CXL.cache Device) can be added later.

To handle downstream port errors in addition to errors directed to the
CXL endpoint device, a handler must also inspect the CXL RAS and PCIe
AER capabilities of the CXL downstream port the device is connected
to.

Since CXL downstream port errors are signaled using internal errors,
the handler requires those errors to be unmasked. This is subject of a
follow-on patch.

The reason for choosing this implementation is that the AER service
driver claims the RCEC device, but does not allow it to register a
custom specific handler to support CXL. Connecting the RCEC hard-wired
with a CXL handler does not work, as the CXL subsystem might not be
present all the time. The alternative to add an implementation to the
portdrv to allow the registration of a custom RCEC error handler isn't
worth doing it as CXL would be its only user. Instead, just check for
an CXL RCEC and pass it down to the connected CXL device's error
handler. With this approach the code can entirely be implemented in
the PCIe AER driver and is independent of the CXL subsystem. The CXL
driver only provides the handler.

[1] CXL 3.0 spec: 9.11.8 CXL Devices Attached to an RCH
[2] CXL 3.0 spec, 12.2.1.1 RCH Downstream Port-detected Errors
[3] CXL 3.0 spec, 8.1.3 PCIe DVSEC for CXL Devices

Co-developed-by: Terry Bowman 
Signed-off-by: Terry Bowman 
Signed-off-by: Robert Richter 
Cc: "Oliver O'Halloran" 
Cc: Bjorn Helgaas 
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-...@vger.kernel.org
Acked-by: Bjorn Helgaas 
Reviewed-by: Jonathan Cameron 
Reviewed-by: Dave Jiang 
---
 drivers/pci/pcie/Kconfig |  9 
 drivers/pci/pcie/aer.c   | 93 +++-
 2 files changed, 100 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pcie/Kconfig b/drivers/pci/pcie/Kconfig
index 228652a59f27..8999fcebde6a 100644
--- a/drivers/pci/pcie/Kconfig
+++ b/drivers/pci/pcie/Kconfig
@@ -49,6 +49,15 @@ config PCIEAER_INJECT
  gotten from:
 
https://git.kernel.org/cgit/linux/kernel/git/gong.chen/aer-inject.git/
 
+config PCIEAER_CXL
+   bool "PCI Express CXL RAS support"
+   default y
+   depends on PCIEAER && CXL_PCI
+   help
+ Enables CXL error handling.
+
+ If unsure, say Y.
+
 #
 # PCI Express ECRC
 #
diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index 6593fe3fc555..f1e8494f5bb6 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -934,14 +934,97 @@ static bool find_source_device(struct pci_dev *parent,
return true;
 }
 
+#ifdef CONFIG_PCIEAER_CXL
+
+static bool is_cxl_mem_dev(struct pci_dev *dev)
+{
+   /*
+* The capability, status, and control fields in Device 0,
+* Function 0 DVSEC control the CXL functionality of the
+* entire device (CXL 3.0, 8.1.3).
+*/
+   if (dev->devfn != PCI_DEVFN(0, 0))
+   return false;
+
+   /*
+* CXL Memory Devices must have the 502h class code set (CXL
+* 3.0, 8.1.12.1).
+*/
+   if ((dev->class >> 8) != PCI_CLASS_MEMORY_CXL)
+   return false;
+
+   return true;
+}
+
+static bool cxl_error_is_native(struct pci_dev *dev)
+{
+   struct pci_host_bridge *host = pci_find_host_bridge(dev->bus);
+
+   return (pcie_ports_native || host->native_aer);
+}
+
+static bool is_internal_error(struct aer_err_info *info)
+{
+   if (info->severity == AER_CORRECTABLE)
+   return info->status & PCI_ERR_COR_INTERNAL;
+
+   return info->status & PCI_ERR_UNC_INTN;
+}
+
+static int cxl_rch_handle_error_iter(struct pci_dev *dev, void *data)
+{
+   struct aer_err_info *info = (stru

[PATCH v12 12/20] PCI/AER: Refactor cper_print_aer() for use by CXL driver module

[Robert Richter]

From: Terry Bowman 

The CXL driver plans to use cper_print_aer() for logging restricted CXL
host (RCH) AER errors. cper_print_aer() is not currently exported and
therefore not usable by the CXL drivers built as loadable modules. Export
the cper_print_aer() function. Use the EXPORT_SYMBOL_NS_GPL() variant
to restrict the export to CXL drivers.

The CONFIG_ACPI_APEI_PCIEAER kernel config is currently used to enable
cper_print_aer(). cper_print_aer() logs the AER registers and is
useful in PCIE AER logging outside of APEI. Remove the
CONFIG_ACPI_APEI_PCIEAER dependency to enable cper_print_aer().

The cper_print_aer() function name implies CPER specific use but is useful
in non-CPER cases as well. Rename cper_print_aer() to pci_print_aer().

Also, update cxl_core to import CXL namespace imports.

Co-developed-by: Robert Richter 
Signed-off-by: Terry Bowman 
Signed-off-by: Robert Richter 
Cc: Mahesh J Salgaonkar 
Cc: "Oliver O'Halloran" 
Cc: Bjorn Helgaas 
Cc: linux-...@vger.kernel.org
Reviewed-by: Jonathan Cameron 
Acked-by: Bjorn Helgaas 
Reviewed-by: Dave Jiang 
---
 drivers/cxl/core/port.c | 1 +
 drivers/pci/pcie/aer.c  | 9 +
 include/linux/aer.h | 2 +-
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index 41a8aa56cffd..802e85321a63 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -2101,3 +2101,4 @@ static void cxl_core_exit(void)
 subsys_initcall(cxl_core_init);
 module_exit(cxl_core_exit);
 MODULE_LICENSE("GPL v2");
+MODULE_IMPORT_NS(CXL);
diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index 9c8fd69ae5ad..6593fe3fc555 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -759,9 +759,10 @@ int cper_severity_to_aer(int cper_severity)
}
 }
 EXPORT_SYMBOL_GPL(cper_severity_to_aer);
+#endif
 
-void cper_print_aer(struct pci_dev *dev, int aer_severity,
-   struct aer_capability_regs *aer)
+void pci_print_aer(struct pci_dev *dev, int aer_severity,
+  struct aer_capability_regs *aer)
 {
int layer, agent, tlp_header_valid = 0;
u32 status, mask;
@@ -800,7 +801,7 @@ void cper_print_aer(struct pci_dev *dev, int aer_severity,
trace_aer_event(dev_name(>dev), (status & ~mask),
aer_severity, tlp_header_valid, >header_log);
 }
-#endif
+EXPORT_SYMBOL_NS_GPL(pci_print_aer, CXL);
 
 /**
  * add_error_device - list device to be handled
@@ -996,7 +997,7 @@ static void aer_recover_work_func(struct work_struct *work)
   PCI_SLOT(entry.devfn), PCI_FUNC(entry.devfn));
continue;
}
-   cper_print_aer(pdev, entry.severity, entry.regs);
+   pci_print_aer(pdev, entry.severity, entry.regs);
if (entry.severity == AER_NONFATAL)
pcie_do_recovery(pdev, pci_channel_io_normal,
 aer_root_reset);
diff --git a/include/linux/aer.h b/include/linux/aer.h
index 29cc10220952..f6ea2f57d808 100644
--- a/include/linux/aer.h
+++ b/include/linux/aer.h
@@ -51,7 +51,7 @@ static inline int pci_aer_clear_nonfatal_status(struct 
pci_dev *dev)
 static inline int pcie_aer_is_native(struct pci_dev *dev) { return 0; }
 #endif
 
-void cper_print_aer(struct pci_dev *dev, int aer_severity,
+void pci_print_aer(struct pci_dev *dev, int aer_severity,
struct aer_capability_regs *aer);
 int cper_severity_to_aer(int cper_severity);
 void aer_recover_queue(int domain, unsigned int bus, unsigned int devfn,
-- 
2.30.2

[PATCH v11 17/20] PCI/AER: Forward RCH downstream port-detected errors to the CXL.mem dev handler

[Robert Richter]

In Restricted CXL Device (RCD) mode a CXL device is exposed as an
RCiEP, but CXL downstream and upstream ports are not enumerated and
not visible in the PCIe hierarchy. [1] Protocol and link errors from
these non-enumerated ports are signaled as internal AER errors, either
Uncorrectable Internal Error (UIE) or Corrected Internal Errors (CIE)
via an RCEC.

Restricted CXL host (RCH) downstream port-detected errors have the
Requester ID of the RCEC set in the RCEC's AER Error Source ID
register. A CXL handler must then inspect the error status in various
CXL registers residing in the dport's component register space (CXL
RAS capability) or the dport's RCRB (PCIe AER extended
capability). [2]

Errors showing up in the RCEC's error handler must be handled and
connected to the CXL subsystem. Implement this by forwarding the error
to all CXL devices below the RCEC. Since the entire CXL device is
controlled only using PCIe Configuration Space of device 0, function
0, only pass it there [3]. The error handling is limited to currently
supported devices with the Memory Device class code set (CXL Type 3
Device, PCI_CLASS_MEMORY_CXL, 502h), handle downstream port errors in
the device's cxl_pci driver. Support for other CXL Device Types
(e.g. a CXL.cache Device) can be added later.

To handle downstream port errors in addition to errors directed to the
CXL endpoint device, a handler must also inspect the CXL RAS and PCIe
AER capabilities of the CXL downstream port the device is connected
to.

Since CXL downstream port errors are signaled using internal errors,
the handler requires those errors to be unmasked. This is subject of a
follow-on patch.

The reason for choosing this implementation is that the AER service
driver claims the RCEC device, but does not allow it to register a
custom specific handler to support CXL. Connecting the RCEC hard-wired
with a CXL handler does not work, as the CXL subsystem might not be
present all the time. The alternative to add an implementation to the
portdrv to allow the registration of a custom RCEC error handler isn't
worth doing it as CXL would be its only user. Instead, just check for
an CXL RCEC and pass it down to the connected CXL device's error
handler. With this approach the code can entirely be implemented in
the PCIe AER driver and is independent of the CXL subsystem. The CXL
driver only provides the handler.

[1] CXL 3.0 spec: 9.11.8 CXL Devices Attached to an RCH
[2] CXL 3.0 spec, 12.2.1.1 RCH Downstream Port-detected Errors
[3] CXL 3.0 spec, 8.1.3 PCIe DVSEC for CXL Devices

Co-developed-by: Terry Bowman 
Signed-off-by: Terry Bowman 
Signed-off-by: Robert Richter 
Cc: "Oliver O'Halloran" 
Cc: Bjorn Helgaas 
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-...@vger.kernel.org
Acked-by: Bjorn Helgaas 
Reviewed-by: Jonathan Cameron 
Reviewed-by: Dave Jiang 
---
 drivers/pci/pcie/Kconfig |  9 
 drivers/pci/pcie/aer.c   | 96 +++-
 2 files changed, 103 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pcie/Kconfig b/drivers/pci/pcie/Kconfig
index 228652a59f27..8999fcebde6a 100644
--- a/drivers/pci/pcie/Kconfig
+++ b/drivers/pci/pcie/Kconfig
@@ -49,6 +49,15 @@ config PCIEAER_INJECT
  gotten from:
 
https://git.kernel.org/cgit/linux/kernel/git/gong.chen/aer-inject.git/
 
+config PCIEAER_CXL
+   bool "PCI Express CXL RAS support"
+   default y
+   depends on PCIEAER && CXL_PCI
+   help
+ Enables CXL error handling.
+
+ If unsure, say Y.
+
 #
 # PCI Express ECRC
 #
diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index 6593fe3fc555..9f420733996b 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -934,14 +934,100 @@ static bool find_source_device(struct pci_dev *parent,
return true;
 }
 
+#ifdef CONFIG_PCIEAER_CXL
+
+static bool is_cxl_mem_dev(struct pci_dev *dev)
+{
+   /*
+* The capability, status, and control fields in Device 0,
+* Function 0 DVSEC control the CXL functionality of the
+* entire device (CXL 3.0, 8.1.3).
+*/
+   if (dev->devfn != PCI_DEVFN(0, 0))
+   return false;
+
+   /*
+* CXL Memory Devices must have the 502h class code set (CXL
+* 3.0, 8.1.12.1).
+*/
+   if ((dev->class >> 8) != PCI_CLASS_MEMORY_CXL)
+   return false;
+
+   return true;
+}
+
+static bool cxl_error_is_native(struct pci_dev *dev)
+{
+   struct pci_host_bridge *host = pci_find_host_bridge(dev->bus);
+
+   if (pcie_ports_native)
+   return true;
+
+   return host->native_aer && host->native_cxl_error;
+}
+
+static bool is_internal_error(struct aer_err_info *info)
+{
+   if (info->severity == AER_CORRECTABLE)
+   return info->status & PCI_ERR_COR_INTERNAL;
+
+   return info->status & PCI_ERR_UNC_INTN;
+}
+
+static int cxl_rch_handle_error_iter(struc

[PATCH v11 18/20] PCI/AER: Unmask RCEC internal errors to enable RCH downstream port error handling

[Robert Richter]

AER corrected and uncorrectable internal errors (CIE/UIE) are masked
in their corresponding mask registers per default once in power-up
state. [1][2] Enable internal errors for RCECs to receive CXL
downstream port errors of Restricted CXL Hosts (RCHs).

[1] CXL 3.0 Spec, 12.2.1.1 - RCH Downstream Port Detected Errors
[2] PCIe Base Spec r6.0, 7.8.4.3 Uncorrectable Error Mask Register,
7.8.4.6 Correctable Error Mask Register

Co-developed-by: Terry Bowman 
Signed-off-by: Terry Bowman 
Signed-off-by: Robert Richter 
Acked-by: Bjorn Helgaas 
Reviewed-by: Jonathan Cameron 
Reviewed-by: Dave Jiang 
---
 drivers/pci/pcie/aer.c | 57 ++
 1 file changed, 57 insertions(+)

diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index 9f420733996b..de63cda8f453 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -936,6 +936,30 @@ static bool find_source_device(struct pci_dev *parent,
 
 #ifdef CONFIG_PCIEAER_CXL
 
+/**
+ * pci_aer_unmask_internal_errors - unmask internal errors
+ * @dev: pointer to the pcie_dev data structure
+ *
+ * Unmasks internal errors in the Uncorrectable and Correctable Error
+ * Mask registers.
+ *
+ * Note: AER must be enabled and supported by the device which must be
+ * checked in advance, e.g. with pcie_aer_is_native().
+ */
+static void pci_aer_unmask_internal_errors(struct pci_dev *dev)
+{
+   int aer = dev->aer_cap;
+   u32 mask;
+
+   pci_read_config_dword(dev, aer + PCI_ERR_UNCOR_MASK, );
+   mask &= ~PCI_ERR_UNC_INTN;
+   pci_write_config_dword(dev, aer + PCI_ERR_UNCOR_MASK, mask);
+
+   pci_read_config_dword(dev, aer + PCI_ERR_COR_MASK, );
+   mask &= ~PCI_ERR_COR_INTERNAL;
+   pci_write_config_dword(dev, aer + PCI_ERR_COR_MASK, mask);
+}
+
 static bool is_cxl_mem_dev(struct pci_dev *dev)
 {
/*
@@ -1015,7 +1039,39 @@ static void cxl_rch_handle_error(struct pci_dev *dev, 
struct aer_err_info *info)
pcie_walk_rcec(dev, cxl_rch_handle_error_iter, info);
 }
 
+static int handles_cxl_error_iter(struct pci_dev *dev, void *data)
+{
+   bool *handles_cxl = data;
+
+   if (!*handles_cxl)
+   *handles_cxl = is_cxl_mem_dev(dev) && cxl_error_is_native(dev);
+
+   /* Non-zero terminates iteration */
+   return *handles_cxl;
+}
+
+static bool handles_cxl_errors(struct pci_dev *rcec)
+{
+   bool handles_cxl = false;
+
+   if (pci_pcie_type(rcec) == PCI_EXP_TYPE_RC_EC &&
+   pcie_aer_is_native(rcec))
+   pcie_walk_rcec(rcec, handles_cxl_error_iter, _cxl);
+
+   return handles_cxl;
+}
+
+static void cxl_rch_enable_rcec(struct pci_dev *rcec)
+{
+   if (!handles_cxl_errors(rcec))
+   return;
+
+   pci_aer_unmask_internal_errors(rcec);
+   pci_info(rcec, "CXL: Internal errors unmasked");
+}
+
 #else
+static inline void cxl_rch_enable_rcec(struct pci_dev *dev) { }
 static inline void cxl_rch_handle_error(struct pci_dev *dev,
struct aer_err_info *info) { }
 #endif
@@ -1415,6 +1471,7 @@ static int aer_probe(struct pcie_device *dev)
return status;
}
 
+   cxl_rch_enable_rcec(port);
aer_enable_rootport(rpc);
pci_info(port, "enabled with IRQ %d\n", dev->irq);
return 0;
-- 
2.30.2

[PATCH v11 12/20] PCI/AER: Refactor cper_print_aer() for use by CXL driver module

[Robert Richter]

From: Terry Bowman 

The CXL driver plans to use cper_print_aer() for logging restricted CXL
host (RCH) AER errors. cper_print_aer() is not currently exported and
therefore not usable by the CXL drivers built as loadable modules. Export
the cper_print_aer() function. Use the EXPORT_SYMBOL_NS_GPL() variant
to restrict the export to CXL drivers.

The CONFIG_ACPI_APEI_PCIEAER kernel config is currently used to enable
cper_print_aer(). cper_print_aer() logs the AER registers and is
useful in PCIE AER logging outside of APEI. Remove the
CONFIG_ACPI_APEI_PCIEAER dependency to enable cper_print_aer().

The cper_print_aer() function name implies CPER specific use but is useful
in non-CPER cases as well. Rename cper_print_aer() to pci_print_aer().

Also, update cxl_core to import CXL namespace imports.

Co-developed-by: Robert Richter 
Signed-off-by: Terry Bowman 
Signed-off-by: Robert Richter 
Cc: Mahesh J Salgaonkar 
Cc: "Oliver O'Halloran" 
Cc: Bjorn Helgaas 
Cc: linux-...@vger.kernel.org
Reviewed-by: Jonathan Cameron 
Acked-by: Bjorn Helgaas 
Reviewed-by: Dave Jiang 
---
 drivers/cxl/core/port.c | 1 +
 drivers/pci/pcie/aer.c  | 9 +
 include/linux/aer.h | 2 +-
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index 41a8aa56cffd..802e85321a63 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -2101,3 +2101,4 @@ static void cxl_core_exit(void)
 subsys_initcall(cxl_core_init);
 module_exit(cxl_core_exit);
 MODULE_LICENSE("GPL v2");
+MODULE_IMPORT_NS(CXL);
diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index 9c8fd69ae5ad..6593fe3fc555 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -759,9 +759,10 @@ int cper_severity_to_aer(int cper_severity)
}
 }
 EXPORT_SYMBOL_GPL(cper_severity_to_aer);
+#endif
 
-void cper_print_aer(struct pci_dev *dev, int aer_severity,
-   struct aer_capability_regs *aer)
+void pci_print_aer(struct pci_dev *dev, int aer_severity,
+  struct aer_capability_regs *aer)
 {
int layer, agent, tlp_header_valid = 0;
u32 status, mask;
@@ -800,7 +801,7 @@ void cper_print_aer(struct pci_dev *dev, int aer_severity,
trace_aer_event(dev_name(>dev), (status & ~mask),
aer_severity, tlp_header_valid, >header_log);
 }
-#endif
+EXPORT_SYMBOL_NS_GPL(pci_print_aer, CXL);
 
 /**
  * add_error_device - list device to be handled
@@ -996,7 +997,7 @@ static void aer_recover_work_func(struct work_struct *work)
   PCI_SLOT(entry.devfn), PCI_FUNC(entry.devfn));
continue;
}
-   cper_print_aer(pdev, entry.severity, entry.regs);
+   pci_print_aer(pdev, entry.severity, entry.regs);
if (entry.severity == AER_NONFATAL)
pcie_do_recovery(pdev, pci_channel_io_normal,
 aer_root_reset);
diff --git a/include/linux/aer.h b/include/linux/aer.h
index 29cc10220952..f6ea2f57d808 100644
--- a/include/linux/aer.h
+++ b/include/linux/aer.h
@@ -51,7 +51,7 @@ static inline int pci_aer_clear_nonfatal_status(struct 
pci_dev *dev)
 static inline int pcie_aer_is_native(struct pci_dev *dev) { return 0; }
 #endif
 
-void cper_print_aer(struct pci_dev *dev, int aer_severity,
+void pci_print_aer(struct pci_dev *dev, int aer_severity,
struct aer_capability_regs *aer);
 int cper_severity_to_aer(int cper_severity);
 void aer_recover_queue(int domain, unsigned int bus, unsigned int devfn,
-- 
2.30.2

Re: [PATCH v4 22/23] PCI/AER: Forward RCH downstream port-detected errors to the CXL.mem dev handler

[Robert Richter]

On 25.05.23 17:01:01, Bjorn Helgaas wrote:
> On Thu, May 25, 2023 at 11:29:58PM +0200, Robert Richter wrote:
> > eOn 24.05.23 16:32:35, Bjorn Helgaas wrote:
> > > On Tue, May 23, 2023 at 06:22:13PM -0500, Terry Bowman wrote:
> > > > From: Robert Richter 
> > > > 
> > > > In Restricted CXL Device (RCD) mode a CXL device is exposed as an
> > > > RCiEP, but CXL downstream and upstream ports are not enumerated and
> > > > not visible in the PCIe hierarchy. Protocol and link errors are sent
> > > > to an RCEC.
> > > >
> > > > Restricted CXL host (RCH) downstream port-detected errors are signaled
> > > > as internal AER errors, either Uncorrectable Internal Error (UIE) or
> > > > Corrected Internal Errors (CIE). 
> > > 
> > > From the parallelism with RCD above, I first thought that RCH devices
> > > were non-RCD mode and *were* enumerated as part of the PCIe hierarchy,
> > > but actually I suspect it's more like the following?
> > > 
> > >   ... but CXL downstream and upstream ports are not enumerated and not
> > >   visible in the PCIe hierarchy.
> > > 
> > >   Protocol and link errors from these non-enumerated ports are
> > >   signaled as internal AER errors ... via a CXL RCEC.
> > 
> > Exactly, except the RCEC is standard PCIe and also must not
> > necessarily on the same PCI bus as the CXL RCiEPs are.
> 
> So make it "RCEC" instead of "CXL RCEC", I guess?  PCIe r6.0, sec
> 7.9.10.3, allows an RCEC to be associated with RCiEPs on different
> buses, so nothing to see there.

Yes, nothing special. This makes it more difficult to check if the
RCEC has CXL devices connected, but still it is feasible.

> 
> > > > The error source is the id of the RCEC.
> > > 
> > > This seems odd; I assume this refers to the RCEC's AER Error Source
> > > Identification register, and the ERR_COR or ERR_FATAL/NONFATAL Source
> > > Identification would ordinarily be the Requester ID of the RCiEP that
> > > "sent" the Error Message.  But you're saying it's actually the ID of
> > > the *RCEC*, not the RCiEP?
> > 
> > Right, the downstream port has its own AER ext capability in
> > non-config (io mapped) RCRB register range. Errors originating from
> > there are signaled as internal AER errors via the RCEC *with* the
> > RCEC's Requester ID. Code walks through all associated CXL endpoints,
> > determines the dport and checks its AER.
> > 
> > There is also an RDPAS structure defined in CXL but that is only a
> > different way to provide the RCEC to dport association instead of
> > using the RCEC's Endpoint Association Extended Capability. In the end
> > we get all associated RCHs and check the AER of all their dports.
> > 
> > The upstream port is signaled using the RCiEP's AER. CXL spec is
> > strict here: "Upstream Port RCRB shall not implement the AER Extended
> > Capability." The RCiEP's requestor ID is used then and its config
> > space the AER is in.
> > 
> > CXL.cachemem errors are reported with the RCiEP as requester
> > too. Status is in the CXL RAS cap and the UIE or CIE is set
> > respectively in the AER status of the RCiEP.
> >
> > > We're going to call pci_aer_handle_error() as well, to handle the
> > > non-internal errors, and I'm pretty sure that path expects the RCiEP
> > > ID there.
> > > 
> > > Whatever the answer, I'm not sure this sentence is actually relevant
> > > to this patch, since this patch doesn't read PCI_ERR_ROOT_ERR_SRC or
> > > look at struct aer_err_source.id.
> > 
> > The source id is used in aer_process_err_devices() which finally calls
> > handle_error_source() for the device with the requestor id. This is
> > the place where cxl_rch_handle_error() checks if it is an RCEC that
> > received an internal error and has cxl devices connected to it. Then,
> > the request is forwarded to the cxl_mem handler which also needs to
> > check the dport now. That is, pcie_walk_rcec() in
> > cxl_rch_handle_error() is called with the RCEC's pci handle,
> > cxl_rch_handle_error_iter() with the RCiEP's pci handle.
> 
> I'm still not sure this is relevant.  Isn't that last sentence just
> the way we always use pcie_walk_rcec()?
> 
> If there's something *different* here about CXL, and it's important to
> this patch, sure.  But I don't see that yet.  Maybe a comment in the
> code if you think it's important to clarify something there.

The importance I see is that internal errors of an RCEC indicate an
AER error in an RCH's downstream port. Thus, once that happens, all
involved dports must be checked. Internal errors are typically
non-standard and implementation defined, but here it is CXL standard.

-Robert

Re: [PATCH v4 22/23] PCI/AER: Forward RCH downstream port-detected errors to the CXL.mem dev handler

[Robert Richter]

eOn 24.05.23 16:32:35, Bjorn Helgaas wrote:
> On Tue, May 23, 2023 at 06:22:13PM -0500, Terry Bowman wrote:
> > From: Robert Richter 
> > 
> > In Restricted CXL Device (RCD) mode a CXL device is exposed as an
> > RCiEP, but CXL downstream and upstream ports are not enumerated and
> > not visible in the PCIe hierarchy. Protocol and link errors are sent
> > to an RCEC.
> >
> > Restricted CXL host (RCH) downstream port-detected errors are signaled
> > as internal AER errors, either Uncorrectable Internal Error (UIE) or
> > Corrected Internal Errors (CIE). 
> 
> From the parallelism with RCD above, I first thought that RCH devices
> were non-RCD mode and *were* enumerated as part of the PCIe hierarchy,
> but actually I suspect it's more like the following?
> 
>   ... but CXL downstream and upstream ports are not enumerated and not
>   visible in the PCIe hierarchy.
> 
>   Protocol and link errors from these non-enumerated ports are
>   signaled as internal AER errors ... via a CXL RCEC.

Exactly, except the RCEC is standard PCIe and also must not
necessarily on the same PCI bus as the CXL RCiEPs are.

> 
> > The error source is the id of the RCEC.
> 
> This seems odd; I assume this refers to the RCEC's AER Error Source
> Identification register, and the ERR_COR or ERR_FATAL/NONFATAL Source
> Identification would ordinarily be the Requester ID of the RCiEP that
> "sent" the Error Message.  But you're saying it's actually the ID of
> the *RCEC*, not the RCiEP?

Right, the downstream port has it's own AER ext capability in
non-config (io mapped) RCRB regsister range. Errors originating from
there are signaled as internal AER errors via the RCEC *with* the
RCEC's Requester ID. Code walks through all associated CXL endpoints,
determines the dport and checks its AER.

There is also an RDPAS structure defined in CXL but that is only a
different way to provide the RCEC to dport association instead of
using the RCEC's Endpoint Association Extended Capability. In the end
we get all associated RCHs and check the AER of all their dports.

The upstream port is signaled using the RCiEP's AER. CXL spec is
strict here: "Upstream Port RCRB shall not implement the AER Extended
Capability." The RCiEP's requestor ID is used then and it's config
space the AER is in.

CXL.cachemem errors are reported with the RCiEP as requester
too. Status is in the CXL RAS cap and the UIE or CIE is set
respectively in the AER status of the RCiEP.

> 
> We're going to call pci_aer_handle_error() as well, to handle the
> non-internal errors, and I'm pretty sure that path expects the RCiEP
> ID there.
> 
> Whatever the answer, I'm not sure this sentence is actually relevant
> to this patch, since this patch doesn't read PCI_ERR_ROOT_ERR_SRC or
> look at struct aer_err_source.id.

The source id is used in aer_process_err_devices() which finally calls
handle_error_source() for the device with the requestor id. This is
the place where cxl_rch_handle_error() checks if it is an RCEC that
recieved an internal error and has cxl devices connected to it. Then,
the request is forwarded to the cxl_mem handler which also needs to
check the dport now. That is, pcie_walk_rcec() in
cxl_rch_handle_error() is called with the RCEC's pci handle,
cxl_rch_handle_error_iter() with the RCiEP's pci handle..

> 
> > A CXL handler must then inspect the error status in various CXL
> > registers residing in the dport's component register space (CXL RAS
> > capability) or the dport's RCRB (PCIe AER extended capability). [1]
> > 
> > Errors showing up in the RCEC's error handler must be handled and
> > connected to the CXL subsystem. Implement this by forwarding the error
> > to all CXL devices below the RCEC. Since the entire CXL device is
> > controlled only using PCIe Configuration Space of device 0, function
> > 0, only pass it there [2]. The error handling is limited to currently
> > supported devices with the Memory Device class code set
> > (PCI_CLASS_MEMORY_CXL, 502h), where the handler can be implemented in
> > the existing cxl_pci driver. Support of CXL devices (e.g. a CXL.cache
> > device) can be enabled later.
> 
> I assume the Memory Devices are CXL devices, so maybe "Error handling
> for *other* CXL devices ... can be enabled later"?  
> 
> IIUC, this happens via cxl_rch_handle_error_iter() calling
> pci_error_handlers for CXL RCiEPs.  Maybe the is_cxl_mem_dev() check
> belongs inside those handlers, since that driver claimed the RCiEP and
> should know its functionality?  Maybe is_internal_error() and
> cxl_error_is_native(), too?

The check is outside the handlers on purpose. A corresponding handler
is needed, it is cxl_pci_driver, see the class code in
cxl_mem_pci_tbl. As the han

Re: [PATCH v3 5/6] PCI/AER: Forward RCH downstream port-detected errors to the CXL.mem dev handler

[Robert Richter]

Bjorn,

On 18.04.23 00:00:58, Robert Richter wrote:
> On 14.04.23 16:32:54, Bjorn Helgaas wrote:
> > On Thu, Apr 13, 2023 at 01:40:52PM +0200, Robert Richter wrote:
> > > On 12.04.23 17:02:33, Bjorn Helgaas wrote:
> > > > On Tue, Apr 11, 2023 at 01:03:01PM -0500, Terry Bowman wrote:

> > I'm mostly interested in the PCI entities involved because that's all
> > aer.c can deal with.  For the above, I think the PCI core only knows
> > about these:
> > 
> >   00:00.0 RCEC  with AER, RCEC EA includes 00:01.0
> >   00:01.0 RCiEP with AER
> > 
> > aer_irq() would handle AER interrupts from 00:00.0.
> > cxl_handle_error() would be called for 00:00.0 and would call
> > handle_error_source() for everything below it (only 00:01.0 here).
> > 
> > > > The current code uses pcie_walk_rcec() in this path, which basically
> > > > searches below a Root Port or RCEC for devices that have an AER error
> > > > status bit set, add them to the e_info[] list, and call
> > > > handle_error_source() for each one:
> > > 
> > > For reference, this series adds support to handle RCH downstream
> > > port-detected errors as described in CXL 3.0, 12.2.1.1.
> > > 
> > > This flow looks correct to me, see comments inline.
> > 
> > We seem to be on the same page here, so I'll trim it out.
> > 
> > > ...
> > > > So we insert cxl_handle_error() in handle_error_source(), where it
> > > > gets called for the RCEC, and then it uses pcie_walk_rcec() again to
> > > > forcibly call handle_error_source() for *every* device "below" the
> > > > RCEC (even though they don't have AER error status bits set).
> > > 
> > > The CXL device contains the links to the dport's caps. Also, there can
> > > be multiple RCs with CXL devs connected to it. So we must search for
> > > all CXL devices now, determine the corresponding dport and inspect
> > > both, PCIe AER and CXL RAS caps.
> > > 
> > > > Then handle_error_source() ultimately calls the CXL driver err_handler
> > > > entry points (.cor_error_detected(), .error_detected(), etc), which
> > > > can look at the CXL-specific error status in the CXL RAS or RCRB or
> > > > whatever.
> > > 
> > > The AER driver (portdrv) does not have the knowledge of CXL internals.
> > > Thus the approach is to pass dport errors to the cxl_mem driver to
> > > handle it there in addition to cxl mem dev errors.
> > > 
> > > > So this basically looks like a workaround for the fact that the AER
> > > > code only calls handle_error_source() when it finds AER error status,
> > > > and CXL doesn't *set* that AER error status.  There's not that much
> > > > code here, but it seems like a quite a bit of complexity in an area
> > > > that is already pretty complicated.
> > 
> > My main point here (correct me if I got this wrong) is that:
> > 
> >   - A RCEC generates an AER interrupt
> > 
> >   - find_source_device() searches all devices below the RCEC and
> > builds a list everything for which to call handle_error_source()
> 
> find_source_device() does not walk the RCEC if the error source is the
> RCEC itself (note that find_device_iter() is called for the root/rcec
> device first and exits early then).
> 
> > 
> >   - cxl_handle_error() *again* looks at all devices below the same
> > RCEC and calls handle_error_source() for each one
> > 
> > So the main difference here is that the existing flow only calls
> > handle_error_source() when it finds an error logged in an AER status
> > register, while the new CXL flow calls handle_error_source() for
> > *every* device below the RCEC.
> 
> That is limited as much as possible:
> 
>  * The RCEC walk to handle CXL dport errors is done only in case of
>internal errors, for an RCEC only (not a port) (check in
>cxl_handle_error()).
> 
>  * Internal errors are only enabled for RCECs connected to CXL devices
>(handles_cxl_errors()).
> 
>  * The handler is only called if it is a CXL memory device (class code
>set and zero devfn) (check in cxl_handle_error_iter()).
> 
> An optimization I see here is to convert some runtime checks to cached
> values determined during device enumeration (CXL device list, RCEC is
> associated with CXL devices). Some sort of RCEC-to-CXL-dev
> association, similar to rcec->rcec_ea.
> 
> > 
> > I think it's OK to do that, but the almost recursive structure and the
> > unusual reference counting ma

Re: [PATCH v3 5/6] PCI/AER: Forward RCH downstream port-detected errors to the CXL.mem dev handler

[Robert Richter]

Dan,

thanks for review, see comments inline.

On 17.04.23 18:01:41, Dan Williams wrote:
> Terry Bowman wrote:
> > From: Robert Richter 
> > 
> > In Restricted CXL Device (RCD) mode a CXL device is exposed as an
> > RCiEP, but CXL downstream and upstream ports are not enumerated and
> > not visible in the PCIe hierarchy. Protocol and link errors are sent
> > to an RCEC.
> > 
> > Restricted CXL host (RCH) downstream port-detected errors are signaled
> > as internal AER errors, either Uncorrectable Internal Error (UIE) or
> > Corrected Internal Errors (CIE). The error source is the id of the
> > RCEC. A CXL handler must then inspect the error status in various CXL
> > registers residing in the dport's component register space (CXL RAS
> > cap) or the dport's RCRB (AER ext cap). [1]
> > 
> > Errors showing up in the RCEC's error handler must be handled and
> > connected to the CXL subsystem. Implement this by forwarding the error
> > to all CXL devices below the RCEC. Since the entire CXL device is
> > controlled only using PCIe Configuration Space of device 0, Function
> > 0, only pass it there [2]. These devices have the Memory Device class
> > code set (PCI_CLASS_MEMORY_CXL, 502h) and the existing cxl_pci driver
> > can implement the handler. In addition to errors directed to the CXL
> > endpoint device, the handler must also inspect the CXL downstream
> > port's CXL RAS and PCIe AER external capabilities that is connected to
> > the device.
> > 
> > Since CXL downstream port errors are signaled using internal errors,
> > the handler requires those errors to be unmasked. This is subject of a
> > follow-on patch.
> > 
> > The reason for choosing this implementation is that a CXL RCEC device
> > is bound to the AER port driver, but the driver does not allow it to
> > register a custom specific handler to support CXL. Connecting the RCEC
> > hard-wired with a CXL handler does not work, as the CXL subsystem
> > might not be present all the time. The alternative to add an
> > implementation to the portdrv to allow the registration of a custom
> > RCEC error handler isn't worth doing it as CXL would be its only user.
> > Instead, just check for an CXL RCEC and pass it down to the connected
> > CXL device's error handler. With this approach the code can entirely
> > be implemented in the PCIe AER driver and is independent of the CXL
> > subsystem. The CXL driver only provides the handler.
> > 
> > [1] CXL 3.0 spec, 12.2.1.1 RCH Downstream Port-detected Errors
> > [2] CXL 3.0 spec, 8.1.3 PCIe DVSEC for CXL Devices
> > 
> > Co-developed-by: Terry Bowman 
> > Signed-off-by: Robert Richter 
> > Signed-off-by: Terry Bowman 
> > Cc: "Oliver O'Halloran" 
> > Cc: Bjorn Helgaas 
> > Cc: Mahesh J Salgaonkar 
> > Cc: linuxppc-dev@lists.ozlabs.org
> > Cc: linux-...@vger.kernel.org
> > ---
> >  drivers/pci/pcie/Kconfig |  8 ++
> >  drivers/pci/pcie/aer.c   | 61 
> >  2 files changed, 69 insertions(+)
> > 
> > diff --git a/drivers/pci/pcie/Kconfig b/drivers/pci/pcie/Kconfig
> > index 228652a59f27..b0dbd864d3a3 100644
> > --- a/drivers/pci/pcie/Kconfig
> > +++ b/drivers/pci/pcie/Kconfig
> > @@ -49,6 +49,14 @@ config PCIEAER_INJECT
> >   gotten from:
> >  
> > https://git.kernel.org/cgit/linux/kernel/git/gong.chen/aer-inject.git/
> >  
> > +config PCIEAER_CXL
> > +   bool "PCI Express CXL RAS support"
> > +   default y
> > +   depends on PCIEAER && CXL_PCI
> > +   help
> > + This enables CXL error handling for Restricted CXL Hosts
> > + (RCHs).
> > +
> >  #
> >  # PCI Express ECRC
> >  #
> > diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> > index 7a25b62d9e01..171a08fd8ebd 100644
> > --- a/drivers/pci/pcie/aer.c
> > +++ b/drivers/pci/pcie/aer.c
> > @@ -946,6 +946,65 @@ static bool find_source_device(struct pci_dev *parent,
> > return true;
> >  }
> >  
> > +#ifdef CONFIG_PCIEAER_CXL
> > +
> > +static bool is_cxl_mem_dev(struct pci_dev *dev)
> > +{
> > +   /*
> > +* A CXL device is controlled only using PCIe Configuration
> > +* Space of device 0, Function 0.
> > +*/
> > +   if (dev->devfn != PCI_DEVFN(0, 0))
> > +   return false;
> > +
> > +   /* Right now there is only a CXL.mem driver */
> > +   if ((dev->class >> 8) != PCI_CLASS_MEMORY_CXL)
> > +   return false;
> > +
> > +   

Re: [PATCH v3 5/6] PCI/AER: Forward RCH downstream port-detected errors to the CXL.mem dev handler

[Robert Richter]

On 14.04.23 16:32:54, Bjorn Helgaas wrote:
> On Thu, Apr 13, 2023 at 01:40:52PM +0200, Robert Richter wrote:
> > On 12.04.23 17:02:33, Bjorn Helgaas wrote:
> > > On Tue, Apr 11, 2023 at 01:03:01PM -0500, Terry Bowman wrote:
> > > > From: Robert Richter 
> 
> > ...
> > Let's assume just a simple CXL RCH topology:
> > 
> > PCI hierarchy:
> > 
> >   -
> >   | ACPI0016  |--   Host bridge (CXL host)
> >   | - CEDT| |
> >---|   - RCRB base | |
> >|  - :
> >|   |
> >|   |
> >|  --- -
> >|  | RCiEP   |.| RCEC  | Endpoint (CXL dev)
> >|  | - BDF   | | - BDF |
> >|  |   | - PCIe AER  | -
> >|  |   | - CXL dvsec |
> >|  |   |   (v2: reg loc) |
> >|  |   |   - Comp regs   |
> >|  |   | - CXL RAS   |
> >|  |   ---
> >:  :
> >   
> > CXL hierarchy:
> > 
> >::
> >:  --|
> >|  | CXL root port  |<
> >|  ||
> >|->| - dport RCRB   |<
> >|  |   - PCIe AER   ||
> >|  |   - Comp regs  ||
> >|  | - CXL RAS  ||
> >|  --|
> >|  : |
> >|  |   --|
> >|  --->| CXL endpoint   |-
> >|  | (v1: RCRB) |
> >-->| - uport RCRB   |
> >   |   - Comp regs  |
> >   | - CXL RAS  |
> >   --
> > 
> > Dport detected errors are reported using PCIe AER and CXL RAS caps in
> > the dports RCRB.
> > 
> > Uport detected errors are reported using RCiEP's PCIe AER cap and
> > either the uport's RCRB RAS cap or the RAS cap of the comp regs
> > located using CXL DVSEC register locator.
> > 
> > In all cases the RCEC is used with either the RCEC (dport errors) or
> > the RCiEP (uport errors) error source id (BDF: bus, dev, func).
> 
> I'm mostly interested in the PCI entities involved because that's all
> aer.c can deal with.  For the above, I think the PCI core only knows
> about these:
> 
>   00:00.0 RCEC  with AER, RCEC EA includes 00:01.0
>   00:01.0 RCiEP with AER
> 
> aer_irq() would handle AER interrupts from 00:00.0.
> cxl_handle_error() would be called for 00:00.0 and would call
> handle_error_source() for everything below it (only 00:01.0 here).
> 
> > > The current code uses pcie_walk_rcec() in this path, which basically
> > > searches below a Root Port or RCEC for devices that have an AER error
> > > status bit set, add them to the e_info[] list, and call
> > > handle_error_source() for each one:
> > 
> > For reference, this series adds support to handle RCH downstream
> > port-detected errors as described in CXL 3.0, 12.2.1.1.
> > 
> > This flow looks correct to me, see comments inline.
> 
> We seem to be on the same page here, so I'll trim it out.
> 
> > ...
> > > So we insert cxl_handle_error() in handle_error_source(), where it
> > > gets called for the RCEC, and then it uses pcie_walk_rcec() again to
> > > forcibly call handle_error_source() for *every* device "below" the
> > > RCEC (even though they don't have AER error status bits set).
> > 
> > The CXL device contains the links to the dport's caps. Also, there can
> > be multiple RCs with CXL devs connected to it. So we must search for
> > all CXL devices now, determine the corresponding dport and inspect
> > both, PCIe AER and CXL RAS caps.
> > 
> > > Then handle_error_source() ultimately calls the CXL driver err_handler
> > > entry points (.cor_error_detected(), .error_detected(), etc), which
> > > can look at the CXL-specific error status in the CXL RAS or RCRB or
> > > whatever.
> > 
> > The AER driver (portdrv) does not have the knowledge of CXL internals.
> > Thus the approach is to pass dport errors to the cxl_mem driver to
> > handle it there in addition to cxl mem dev errors.
> > 
> > > So this basically looks like a workarou

Re: [PATCH v3 5/6] PCI/AER: Forward RCH downstream port-detected errors to the CXL.mem dev handler

[Robert Richter]

Hi Jonathan,

On 17.04.23 17:54:31, Jonathan Cameron wrote:
> On Fri, 14 Apr 2023 16:35:05 +0200
> Robert Richter  wrote:
> 
> > On 14.04.23 13:19:50, Jonathan Cameron wrote:
> > > On Tue, 11 Apr 2023 13:03:01 -0500
> > > Terry Bowman  wrote:
> > >   
> > > > From: Robert Richter 
> > > > 
> > > > In Restricted CXL Device (RCD) mode a CXL device is exposed as an
> > > > RCiEP, but CXL downstream and upstream ports are not enumerated and
> > > > not visible in the PCIe hierarchy. Protocol and link errors are sent
> > > > to an RCEC.
> > > > 
> > > > Restricted CXL host (RCH) downstream port-detected errors are signaled
> > > > as internal AER errors, either Uncorrectable Internal Error (UIE) or
> > > > Corrected Internal Errors (CIE). The error source is the id of the
> > > > RCEC. A CXL handler must then inspect the error status in various CXL
> > > > registers residing in the dport's component register space (CXL RAS
> > > > cap) or the dport's RCRB (AER ext cap). [1]
> > > > 
> > > > Errors showing up in the RCEC's error handler must be handled and
> > > > connected to the CXL subsystem. Implement this by forwarding the error
> > > > to all CXL devices below the RCEC. Since the entire CXL device is
> > > > controlled only using PCIe Configuration Space of device 0, Function
> > > > 0, only pass it there [2]. These devices have the Memory Device class
> > > > code set (PCI_CLASS_MEMORY_CXL, 502h) and the existing cxl_pci driver
> > > > can implement the handler.  
> > > 
> > > This comment implies only class code compliant drivers.  Sure we don't
> > > have drivers for anything else yet, but we should try to avoid saying
> > > there won't be any (which I think above implies).
> > > 
> > > You have a comment in the code, but maybe relaxing the description above
> > > to "currently support devices have..."  
> > 
> > It is used here to identify CXL memory devices and limit the
> > enablement to those. The spec requires this to be set for CXL mem devs
> > (see cxl 3.0, 8.1.12.2).
> > 
> > There could be other CXL devices (e.g. cache), but other drivers are
> > not yet implemented. That is what I am referring to. The check makes
> > sure there is actually a driver with a handler for it (cxl_pci).
> 
> Understood on intent. My worry is that the above can be read as a
> statement on hardware restrictions, rathe than on what software currently
> implements.  Meh. Minor point so I don't care that much!
> Unlikely anyone will read the patch description after it merges anyway ;)

I have updated the description ...

> > > > diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> > > > index 7a25b62d9e01..171a08fd8ebd 100644
> > > > --- a/drivers/pci/pcie/aer.c
> > > > +++ b/drivers/pci/pcie/aer.c
> > > > @@ -946,6 +946,65 @@ static bool find_source_device(struct pci_dev 
> > > > *parent,
> > > > return true;
> > > >  }
> > > >  
> > > > +#ifdef CONFIG_PCIEAER_CXL
> > > > +
> > > > +static bool is_cxl_mem_dev(struct pci_dev *dev)
> > > > +{
> > > > +   /*
> > > > +* A CXL device is controlled only using PCIe Configuration
> > > > +* Space of device 0, Function 0.  
> > > 
> > > That's not true in general.   Definitely true that CXL protocol
> > > error reporting is controlled only using this Devfn, but
> > > more generally there could be other stuff in later functions.
> > > So perhaps make the comment more specific.  
> > 
> > I actually mean CXL device in RCD mode here (seen as RCiEP in the PCI
> > hierarchy).
> > 
> > The spec says (cxl 3.0, 8.1.3):
> > 
> > """
> > In either case [(RCD and non-RCD)], the capability, status, and
> > control fields in Device 0, Function 0 DVSEC control the CXL
> > functionality of the entire device.
> 
> > """
> > 
> > So dev 0, func 0 must contain a CXL PCIe DVSEC. Thus it is a CXL
> > device and able to handle CXL AER errors. The limitation to the first
> > device prevents the handler from being run multiple times for the same
> > event.
> 
> Fine with limitation.  Text says "device is controlled only using".
> That is true for what you are controlling here, but other aspects of the
> device are controlled via whatever interface they like.
> 
> Perhaps just quote the specification as you have done in your reply. Then it
> is clear that we mean just these registers.

... and comments.

Thanks,

-Robert

Re: [PATCH v3 6/6] PCI/AER: Unmask RCEC internal errors to enable RCH downstream port error handling

[Robert Richter]

On 14.04.23 12:55:43, Jonathan Cameron wrote:
> On Fri, 14 Apr 2023 13:21:37 +0200
> Robert Richter  wrote:

> > The version I have ready after addressing Bjorn's comments is pretty
> > much the same, apart from error checking of the read/writes.
> > 
> > From your patch proposed you will need it in aer.c too and we do not
> > need to export it.
> 
> I think for the other components we'll want to call it from 
> cxl_pci_ras_unmask()
> so an export needed.
> 
> I also wonder if a more generic function would be better as seems likely
> similar code will be needed for errors other than this pair.

There are only a few masked by default, but not only internals. Will
consider that and also make it easy to export later once needed.

Thanks,

-Robert

Re: [PATCH v3 5/6] PCI/AER: Forward RCH downstream port-detected errors to the CXL.mem dev handler

[Robert Richter]

On 14.04.23 13:19:50, Jonathan Cameron wrote:
> On Tue, 11 Apr 2023 13:03:01 -0500
> Terry Bowman  wrote:
> 
> > From: Robert Richter 
> > 
> > In Restricted CXL Device (RCD) mode a CXL device is exposed as an
> > RCiEP, but CXL downstream and upstream ports are not enumerated and
> > not visible in the PCIe hierarchy. Protocol and link errors are sent
> > to an RCEC.
> > 
> > Restricted CXL host (RCH) downstream port-detected errors are signaled
> > as internal AER errors, either Uncorrectable Internal Error (UIE) or
> > Corrected Internal Errors (CIE). The error source is the id of the
> > RCEC. A CXL handler must then inspect the error status in various CXL
> > registers residing in the dport's component register space (CXL RAS
> > cap) or the dport's RCRB (AER ext cap). [1]
> > 
> > Errors showing up in the RCEC's error handler must be handled and
> > connected to the CXL subsystem. Implement this by forwarding the error
> > to all CXL devices below the RCEC. Since the entire CXL device is
> > controlled only using PCIe Configuration Space of device 0, Function
> > 0, only pass it there [2]. These devices have the Memory Device class
> > code set (PCI_CLASS_MEMORY_CXL, 502h) and the existing cxl_pci driver
> > can implement the handler.
> 
> This comment implies only class code compliant drivers.  Sure we don't
> have drivers for anything else yet, but we should try to avoid saying
> there won't be any (which I think above implies).
> 
> You have a comment in the code, but maybe relaxing the description above
> to "currently support devices have..."

It is used here to identify CXL memory devices and limit the
enablement to those. The spec requires this to be set for CXL mem devs
(see cxl 3.0, 8.1.12.2).

There could be other CXL devices (e.g. cache), but other drivers are
not yet implemented. That is what I am referring to. The check makes
sure there is actually a driver with a handler for it (cxl_pci).

> 
> > In addition to errors directed to the CXL
> > endpoint device, the handler must also inspect the CXL downstream
> > port's CXL RAS and PCIe AER external capabilities that is connected to
> > the device.
> > 
> > Since CXL downstream port errors are signaled using internal errors,
> > the handler requires those errors to be unmasked. This is subject of a
> > follow-on patch.
> > 
> > The reason for choosing this implementation is that a CXL RCEC device
> > is bound to the AER port driver, but the driver does not allow it to
> > register a custom specific handler to support CXL. Connecting the RCEC
> > hard-wired with a CXL handler does not work, as the CXL subsystem
> > might not be present all the time. The alternative to add an
> > implementation to the portdrv to allow the registration of a custom
> > RCEC error handler isn't worth doing it as CXL would be its only user.
> > Instead, just check for an CXL RCEC and pass it down to the connected
> > CXL device's error handler. With this approach the code can entirely
> > be implemented in the PCIe AER driver and is independent of the CXL
> > subsystem. The CXL driver only provides the handler.
> > 
> > [1] CXL 3.0 spec, 12.2.1.1 RCH Downstream Port-detected Errors
> > [2] CXL 3.0 spec, 8.1.3 PCIe DVSEC for CXL Devices
> > 
> > Co-developed-by: Terry Bowman 
> > Signed-off-by: Robert Richter 
> > Signed-off-by: Terry Bowman 
> > Cc: "Oliver O'Halloran" 
> > Cc: Bjorn Helgaas 
> > Cc: Mahesh J Salgaonkar 
> > Cc: linuxppc-dev@lists.ozlabs.org
> > Cc: linux-...@vger.kernel.org
> 
> Generally looks good to me.  A few trivial comments inline.
> 
> > ---
> >  drivers/pci/pcie/Kconfig |  8 ++
> >  drivers/pci/pcie/aer.c   | 61 
> >  2 files changed, 69 insertions(+)
> > 
> > diff --git a/drivers/pci/pcie/Kconfig b/drivers/pci/pcie/Kconfig
> > index 228652a59f27..b0dbd864d3a3 100644
> > --- a/drivers/pci/pcie/Kconfig
> > +++ b/drivers/pci/pcie/Kconfig
> > @@ -49,6 +49,14 @@ config PCIEAER_INJECT
> >   gotten from:
> >  
> > https://git.kernel.org/cgit/linux/kernel/git/gong.chen/aer-inject.git/
> >  
> > +config PCIEAER_CXL
> > +   bool "PCI Express CXL RAS support"
> 
> Description makes this sound too general. I'd mentioned restricted
> hosts even in the menu option title.
> 
> 
> > +   default y
> > +   depends on PCIEAER && CXL_PCI
> > +   help
> > + This enables CXL error handling for Restricted CXL Hosts
> > + (RCHs).
> 
> Spec term is probably fin

Re: [PATCH v3 6/6] PCI/AER: Unmask RCEC internal errors to enable RCH downstream port error handling

[Robert Richter]

On 13.04.23 18:05:08, Jonathan Cameron wrote:
> On Thu, 13 Apr 2023 15:38:07 +0200
> Robert Richter  wrote:
> 
> > On 12.04.23 16:29:01, Bjorn Helgaas wrote:
> > > On Tue, Apr 11, 2023 at 01:03:02PM -0500, Terry Bowman wrote:  

> > > With the exception of this function, this patch looks like all CXL
> > > code that maybe could be with other CXL code.  Would require making
> > > pcie_walk_rcec() available outside drivers/pci, I guess.  
> > 
> > Even this is CXL code, it implements AER support and fits better here
> > around AER code. Export of pcie_walk_rcec() (and others?) is not the
> > main issue here. CXL drivers can come as modules and would need to
> > register a hook at the aer handler.  This would add even more
> > complexity here. In contrast, current solution just adds two functions
> > for enablement and handling which are empty stubs if code is disabled.
> > 
> > I could move that code to aer_cxl.c similar to aer_inject.c. Since the
> > CXL part is small compared to the remaining aer code I left it in
> > aer.c. Also, it is guarded by #ifdef which additionally encapsulates
> > it.
> > 
> 
> To throw another option in there (what Bjorn suggested IIRC for the more
> general case..) 
> 
> Just enable internal errors always.  No need to know if they are CXL
> or something else.
> 
> There will/might be fallout and it will be fun.

I left the fun part to others. :-)

If some PCI root port goes crazy it tears down the whole system, would
avoid that.

Since internal error are implementation specific, I would only enable
them once a handler exists. What's why enablement is limited to CXL
RCECs only.

-Robert

Re: [PATCH v3 6/6] PCI/AER: Unmask RCEC internal errors to enable RCH downstream port error handling

[Robert Richter]

On 13.04.23 15:52:36, Ira Weiny wrote:
> Jonathan Cameron wrote:
> > On Wed, 12 Apr 2023 16:29:01 -0500
> > Bjorn Helgaas  wrote:
> > 
> > > On Tue, Apr 11, 2023 at 01:03:02PM -0500, Terry Bowman wrote:
> > > > From: Robert Richter 
> > > > 

> > > > +static int __cxl_unmask_internal_errors(struct pci_dev *rcec)
> > > > +{
> > > > +   int aer, rc;
> > > > +   u32 mask;
> > > > +
> > > > +   /*
> > > > +* Internal errors are masked by default, unmask RCEC's here
> > > > +* PCI6.0 7.8.4.3 Uncorrectable Error Mask Register (Offset 08h)
> > > > +* PCI6.0 7.8.4.6 Correctable Error Mask Register (Offset 14h)
> > > > +*/  
> > > 
> > > Unmasking internal errors doesn't have anything specific to do with
> > > CXL, so I don't think it should have "cxl" in the function name.
> > > Maybe something like "pci_aer_unmask_internal_errors()".
> > 
> > This reminds me.  Not sure we resolved earlier discussion on changing
> > the system wide policy to turn these on 
> > https://lore.kernel.org/linux-cxl/20221229172731.GA611562@bhelgaas/
> > which needs pretty much the same thing.
> > 
> > Ira, I think you were picking this one up?
> > https://lore.kernel.org/linux-cxl/63e5fb533f304_13244829412@iweiny-mobl.notmuch/
> 
> After this discussion I posted an RFC to enable those errors.
> 
> https://lore.kernel.org/all/20230209-cxl-pci-aer-v1-1-f9a817fa4...@intel.com/
> 
> Unfortunately the prevailing opinion was that this was unsafe.  And no one
> piped up with a reason to pursue the alternative of a pci core call to enable
> them as needed.
> 
> So I abandoned the work.
> 
> I think the direction things where headed was to have a call like:
> 
> int pci_enable_pci_internal_errors(struct pci_dev *dev)
> {
>   int pos_cap_err;
>   u32 reg;
> 
>   if (!pcie_aer_is_native(dev))
>   return -EIO;
> 
>   pos_cap_err = dev->aer_cap;
> 
>   /* Unmask correctable and uncorrectable (non-fatal) internal errors */
>   pci_read_config_dword(dev, pos_cap_err + PCI_ERR_COR_MASK, );
>   reg &= ~PCI_ERR_COR_INTERNAL;
>   pci_write_config_dword(dev, pos_cap_err + PCI_ERR_COR_MASK, reg);
>   
>   pci_read_config_dword(dev, pos_cap_err + PCI_ERR_UNCOR_SEVER, );
>   reg &= ~PCI_ERR_UNC_INTN;
>   pci_write_config_dword(dev, pos_cap_err + PCI_ERR_UNCOR_SEVER, reg);
>   
>   pci_read_config_dword(dev, pos_cap_err + PCI_ERR_UNCOR_MASK, );
>   reg &= ~PCI_ERR_UNC_INTN;
>   pci_write_config_dword(dev, pos_cap_err + PCI_ERR_UNCOR_MASK, reg);
> 
>   return 0;
> }
> 
> ... and call this from the cxl code where it is needed.

The version I have ready after addressing Bjorn's comments is pretty
much the same, apart from error checking of the read/writes.

>From your patch proposed you will need it in aer.c too and we do not
need to export it.

This patch only enables it for (CXL) RCECs. You might want to extend
this for CXL endpoints (and ports?) then.

> 
> Is this an acceptable direction?  Terry is welcome to steal the above from my
> patch and throw it into the PCI core.
> 
> Looking at the current state of things I think cxl_pci_ras_unmask() may
> actually be broken now without calling something like the above.  For that I
> dropped the ball.

Thanks,

-Robert

> 
> Ira

Re: [PATCH v3 6/6] PCI/AER: Unmask RCEC internal errors to enable RCH downstream port error handling

[Robert Richter]

On 12.04.23 16:29:01, Bjorn Helgaas wrote:
> On Tue, Apr 11, 2023 at 01:03:02PM -0500, Terry Bowman wrote:
> > From: Robert Richter 
> > 
> > RCEC AER corrected and uncorrectable internal errors (CIE/UIE) are
> > disabled by default.
> 
> "Disabled by default" just means "the power-up state of CIE/UIC is
> that they are masked", right?  It doesn't mean that Linux normally
> masks them.

Yes, will change the wording here.

> > [1][2] Enable them to receive CXL downstream port
> > errors of a Restricted CXL Host (RCH).
> > 
> > [1] CXL 3.0 Spec, 12.2.1.1 - RCH Downstream Port Detected Errors
> > [2] PCIe Base Spec 6.0, 7.8.4.3 Uncorrectable Error Mask Register,
> > 7.8.4.6 Correctable Error Mask Register
> > 
> > Co-developed-by: Terry Bowman 
> > Signed-off-by: Robert Richter 
> > Signed-off-by: Terry Bowman 
> > Cc: "Oliver O'Halloran" 
> > Cc: Bjorn Helgaas 
> > Cc: Mahesh J Salgaonkar 
> > Cc: linuxppc-dev@lists.ozlabs.org
> > Cc: linux-...@vger.kernel.org
> > ---
> >  drivers/pci/pcie/aer.c | 73 ++
> >  1 file changed, 73 insertions(+)
> > 
> > diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> > index 171a08fd8ebd..3973c731e11d 100644
> > --- a/drivers/pci/pcie/aer.c
> > +++ b/drivers/pci/pcie/aer.c
> > @@ -1000,7 +1000,79 @@ static void cxl_handle_error(struct pci_dev *dev, 
> > struct aer_err_info *info)
> > pcie_walk_rcec(dev, cxl_handle_error_iter, info);
> >  }
> >  
> > +static bool cxl_error_is_native(struct pci_dev *dev)
> > +{
> > +   struct pci_host_bridge *host = pci_find_host_bridge(dev->bus);
> > +
> > +   if (pcie_ports_native)
> > +   return true;
> > +
> > +   return host->native_aer && host->native_cxl_error;
> > +}
> > +
> > +static int handles_cxl_error_iter(struct pci_dev *dev, void *data)
> > +{
> > +   int *handles_cxl = data;
> > +
> > +   *handles_cxl = is_cxl_mem_dev(dev) && cxl_error_is_native(dev);
> > +
> > +   return *handles_cxl;
> > +}
> > +
> > +static bool handles_cxl_errors(struct pci_dev *rcec)
> > +{
> > +   int handles_cxl = 0;
> > +
> > +   if (!rcec->aer_cap)
> > +   return false;
> > +
> > +   if (pci_pcie_type(rcec) == PCI_EXP_TYPE_RC_EC)
> > +   pcie_walk_rcec(rcec, handles_cxl_error_iter, _cxl);
> > +
> > +   return !!handles_cxl;
> > +}
> > +
> > +static int __cxl_unmask_internal_errors(struct pci_dev *rcec)
> > +{
> > +   int aer, rc;
> > +   u32 mask;
> > +
> > +   /*
> > +* Internal errors are masked by default, unmask RCEC's here
> > +* PCI6.0 7.8.4.3 Uncorrectable Error Mask Register (Offset 08h)
> > +* PCI6.0 7.8.4.6 Correctable Error Mask Register (Offset 14h)
> > +*/
> 
> Unmasking internal errors doesn't have anything specific to do with
> CXL, so I don't think it should have "cxl" in the function name.
> Maybe something like "pci_aer_unmask_internal_errors()".

Since it is static I renamed it to aer_unmask_internal_errors() and
also moved it to the beginning of the #ifdef block for easier later
reuse.

> 
> This also has nothing special to do with RCECs, so I think we should
> refer to the device as "dev" as is typical in this file.

Changed.

> 
> I think this needs to check pcie_aer_is_native() as is done by
> pci_aer_clear_nonfatal_status() and other functions that write the AER
> Capability.

Also added the check to aer_unmask_internal_errors(). There was a
check for native_* in handles_cxl_errors() already, but only for the
pci devs of the RCEC. I added a check of the RCEC there too.

> 
> With the exception of this function, this patch looks like all CXL
> code that maybe could be with other CXL code.  Would require making
> pcie_walk_rcec() available outside drivers/pci, I guess.

Even this is CXL code, it implements AER support and fits better here
around AER code. Export of pcie_walk_rcec() (and others?) is not the
main issue here. CXL drivers can come as modules and would need to
register a hook at the aer handler.  This would add even more
complexity here. In contrast, current solution just adds two functions
for enablement and handling which are empty stubs if code is disabled.

I could move that code to aer_cxl.c similar to aer_inject.c. Since the
CXL part is small compared to the remaining aer code I left it in
aer.c. Also, it is guarded by #ifdef which additionally encapsulates
it.

> 
> > +   aer = rcec->aer_cap;
> >

Re: [PATCH v3 5/6] PCI/AER: Forward RCH downstream port-detected errors to the CXL.mem dev handler

[Robert Richter]

Bjorn,

thanks for your detailed review.

On 12.04.23 17:02:33, Bjorn Helgaas wrote:
> On Tue, Apr 11, 2023 at 01:03:01PM -0500, Terry Bowman wrote:
> > From: Robert Richter 
> > 
> > In Restricted CXL Device (RCD) mode a CXL device is exposed as an
> > RCiEP, but CXL downstream and upstream ports are not enumerated and
> > not visible in the PCIe hierarchy. Protocol and link errors are sent
> > to an RCEC.
> > 
> > Restricted CXL host (RCH) downstream port-detected errors are signaled
> > as internal AER errors, either Uncorrectable Internal Error (UIE) or
> > Corrected Internal Errors (CIE). The error source is the id of the
> > RCEC. A CXL handler must then inspect the error status in various CXL
> > registers residing in the dport's component register space (CXL RAS
> > cap) or the dport's RCRB (AER ext cap). [1]
> > 
> > Errors showing up in the RCEC's error handler must be handled and
> > connected to the CXL subsystem. Implement this by forwarding the error
> > to all CXL devices below the RCEC. Since the entire CXL device is
> > controlled only using PCIe Configuration Space of device 0, Function
> > 0,
> 
> Capitalize "device" and "Function" the same way (also appears in
> comment below).

Changed that.

> 
> > only pass it there [2]. These devices have the Memory Device class
> > code set (PCI_CLASS_MEMORY_CXL, 502h) and the existing cxl_pci driver
> > can implement the handler. In addition to errors directed to the CXL
> > endpoint device, the handler must also inspect the CXL downstream
> > port's CXL RAS and PCIe AER external capabilities that is connected to
> 
> "AER external capabilities" --  is that referring to the "AER
> *Extended* capability"?  If so, we usually don't bother including the
> "extended" part because it's usually not relevant.  But if you intended
> "external", I don't know what it means.

Right, "extended" is meant here, but I will drop it to also fit with
the 'CXL RAS capability'.

> 
> > the device.
> > 
> > Since CXL downstream port errors are signaled using internal errors,
> > the handler requires those errors to be unmasked. This is subject of a
> > follow-on patch.
> > 
> > The reason for choosing this implementation is that a CXL RCEC device
> > is bound to the AER port driver, but the driver does not allow it to
> > register a custom specific handler to support CXL. Connecting the RCEC
> > hard-wired with a CXL handler does not work, as the CXL subsystem
> > might not be present all the time. The alternative to add an
> > implementation to the portdrv to allow the registration of a custom
> > RCEC error handler isn't worth doing it as CXL would be its only user.
> > Instead, just check for an CXL RCEC and pass it down to the connected
> > CXL device's error handler. With this approach the code can entirely
> > be implemented in the PCIe AER driver and is independent of the CXL
> > subsystem. The CXL driver only provides the handler.
> 
> Can you make this more concrete with an example topology so we can
> work through how this all works?  Correct me when I go off the rails
> here:

Let's assume just a simple CXL RCH topology:

PCI hierarchy:

-
| ACPI0016  |   Host bridge (CXL host)
| - CEDT|   |
   -|   - RCRB base |   |
   |-   :
   | |
   | |
   |--- -
   || RCiEP   |.| RCEC  |   Endpoint (CXL dev)
   || - BDF   | | - BDF |
   ||   | - PCIe AER  | -
   ||   | - CXL dvsec |
   ||   |   (v2: reg loc) |
   ||   |   - Comp regs   |
   ||   | - CXL RAS   |
   ||   ---
   ::

CXL hierarchy:

   ::
   :--  |
   || CXL root port  |<--
   |||
   |--->| - dport RCRB   |<--
   ||   - PCIe AER   |  |
   ||   - Comp regs  |  |
   || - CXL RAS  |  |
   |--  |
   |:   |
   ||   --  |
   |--->| CXL endpoint   |---
   || (v1: RCRB) |
   >| - uport RCRB   |
|   - Comp regs  |
|   

Re: [PATCH v2 4/5] cxl/pci: Forward RCH downstream port-detected errors to the CXL.mem dev handler

[Robert Richter]

On 28.03.23 12:21:04, Bjorn Helgaas wrote:
> [+cc linux-pci, more error handling folks; beginning of thread at
> https://lore.kernel.org/all/20230323213808.398039-1-terry.bow...@amd.com/]
> 
> On Mon, Mar 27, 2023 at 11:51:39PM +0200, Robert Richter wrote:
> > On 24.03.23 17:36:56, Bjorn Helgaas wrote:
> 
> > > > The CXL device driver is then responsible to
> > > > enable error reporting in the RCEC's AER cap
> > > 
> > > I don't know exactly what you mean by "error reporting in the RCEC's
> > > AER cap", but IIUC, for non-Root Port devices, generation of ERR_COR/
> > > ERR_NONFATAL/ERR_FATAL messages is controlled by the Device Control
> > > register and should already be enabled by pci_aer_init().
> > > 
> > > Maybe you mean setting AER mask/severity specifically for Internal
> > > Errors?  I'm hoping to get as much of AER management as we can in the
> > 
> > Richt, this is implemented in patch #5 in function
> > rcec_enable_aer_ints().
> 
> I think we should add a PCI core interface for this so we can enforce
> the AER ownership question (all the crud like pcie_aer_is_native()) in
> one place.

Do you mean, code around functions rcec_enable_aer_ints() should be
moved to aer.c and the cxl handler then just assumes it is enabled
already? That looks feasible.

> 
> > > PCI core and out of drivers, so maybe we need a new PCI interface to
> > > do that.
> > > 
> > > In any event, I assume this sort of configuration would be an
> > > enumeration-time thing, while *this* patch is a run-time thing, so
> > > maybe this information belongs with a different patch?
> > 
> > Do you mean once a Restricted CXL host (RCH) is detected, the internal
> > errors should be enabled in the device mask, all this done during
> > device enumeration? But wouldn't interrupts being enabled then before
> > the CXL device is ready?
> 
> I'm not sure what you mean by "before the CXL device is ready."  What
> makes a CXL device ready, and how do we know when it is ready?

The cxl_pci driver must be bound to a device which then further
creates a CXL mem dev. With that binding we can determine the
connected CXL dports from the cxl endpoints (which are seen as PCIe
endpoints) to inspect the CXL RAS caps (in the CXL component reg
space) and the PCIe AER caps (in the RCRB of the dport).

> 
> pci_aer_init() turns on PCI_EXP_DEVCTL_CERE, PCI_EXP_DEVCTL_FERE, etc
> as soon as we enumerate the device, before any driver claims the
> device.  I'm wondering whether we can do this PCI_ERR_COR_INTERNAL and
> PCI_ERR_UNC_INTN fiddling around the same time?

Yes, if the CXL device is not yet bound, there is no handler attached
and AER errors are only handled on a PCI level. Though, we need to
make sure the status is cleared.

> 
> > > I haven't worked all the way through this, but I thought Sean Kelley's
> > > and Qiuxu Zhuo's work was along the same line and might cover this,
> > > e.g.,
> > > 
> > >   a175102b0a82 ("PCI/ERR: Recover from RCEC AER errors")
> > >   579086225502 ("PCI/ERR: Recover from RCiEP AER errors")
> > >   af113553d961 ("PCI/AER: Add pcie_walk_rcec() to RCEC AER handling")
> > > 
> > > But I guess maybe it's not quite the same case?
> > 
> > Actually, we use this code to handle errors that are reported to the
> > RCEC and only implement here the CXL specifics. That is, checking if
> > the RCEC receives something from a CXL downstream port and forwarding
> > that to a CXL handler (this patch). The handler then checks the AER
> > err cap in the RCRB of all CXL downstream ports associated to the RCEC
> > (not visible in the PCI hierarchy), but discovered through the :00.0
> > RCiEP (patch #5).
> 
> There are two calls to pcie_walk_rcec():
> 
>   1) The existing one in find_source_device()
>   2) The one you add in handle_cxl_error()
> 
> Does the call in handle_cxl_error() look at devices that the existing
> call in find_source_device() does not?  I'm trying to understand why
> we need both calls.

In case of a dport error, e_info will only contain the RCEC's id after
running find_source_device(). Thus, only the RCEC's handler would be
called. The portdrv is already bound to the device and currently
doesn't have a handler attached.

As described, due to cross dependencies between cxl and the portdrv,
instead of implementing a handler in the portdrv, we decided to
forward errors to the CXL endpoint driver and handle it there. So now,
in handle_cxl_error(), we check if the error source is an RCEC
attached to a CXL bus and we forward e

Re: gcc trunk fails to build kernel on PowerPC64 due to oprofile warnings

[Robert Richter]

On 26.01.17 10:46:43, William Cohen wrote:
> From 7e46dbd7dc5bc941926a4a63c28ccebf46493e8d Mon Sep 17 00:00:00 2001
> From: William Cohen 
> Date: Thu, 26 Jan 2017 10:33:59 -0500
> Subject: [PATCH] Avoid hypthetical string truncation in oprofile stats buffer
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> Increased the size of an internal oprofile driver buffer ensuring that
> the string was never truncated for any possible int value to avoid the
> following gcc-7 compiler error on ppc when building the kernel:

Please test gcc7 for other archs first. I don't think this is the only
change needed to avoid this warning in oprofile code.

Thanks,

-Robert

Re: [PATCH v10 3/8] dt, numa: adding numa dt binding implementation.

[Robert Richter]

On 11.02.16 08:50:41, Rob Herring wrote:
> On Tue, Feb 2, 2016 at 4:09 AM, Ganapatrao Kulkarni
>  wrote:
> > dt node parsing for numa topology is done using device property
> > numa-node-id and device node distance-map.
> 
> How is it that powerpc doesn't need flat DT parsing for NUMA? Both
> arches are memblock based and the binding is similar IIRC, so you
> should be able to find a way to do this with the unflattened tree.
> Ideally, there should be some common code shared as well.
> 
> Do you have a git tree with this series?

I have pushed it here for you:

 
https://git.kernel.org/cgit/linux/kernel/git/rric/linux.git/log/?h=thunder/numa-v10

-Robert
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v10 0/8] arm64, numa: Add numa support for arm64 platforms

[Robert Richter]

On 02.02.16 15:39:15, Ganapatrao Kulkarni wrote:

> Ganapatrao Kulkarni (8):
>   arm64, numa: adding numa support for arm64 platforms.
>   Documentation, dt, numa: dt bindings for numa.
>   dt, numa: adding numa dt binding implementation.
>   arm64, numa : Enable numa dt for arm64 platforms.
>   arm64, dt, thunderx: Add initial dts for Cavium Thunderx in 2 node
> topology.
>   arm64, mm, numa: Adding numa balancing support for arm64.
>   topology, cleanup: Avoid redefinition of cpumask_of_pcibus in asm
> header files.
>   numa, mm, cleanup: remove redundant NODE_DATA macro from asm header
> files.

I have tested the whole series on single and dual node systems for
devicetree and acpi (with Hanjun's acpi numa v3 patches ported on
top).

Tested-by: Robert Richter <rrich...@cavium.com>

-Robert

> 
>  Documentation/devicetree/bindings/numa.txt  | 272 
>  arch/arm64/Kconfig  |  26 +
>  arch/arm64/boot/dts/cavium/Makefile |   2 +-
>  arch/arm64/boot/dts/cavium/thunder-88xx-2n.dts  |  83 +++
>  arch/arm64/boot/dts/cavium/thunder-88xx-2n.dtsi | 806 
> 
>  arch/arm64/include/asm/mmzone.h |  10 +
>  arch/arm64/include/asm/numa.h   |  45 ++
>  arch/arm64/include/asm/pgtable.h|  18 +
>  arch/arm64/include/asm/topology.h   |   7 +
>  arch/arm64/kernel/pci.c |  10 +
>  arch/arm64/kernel/setup.c   |   4 +
>  arch/arm64/kernel/smp.c |   4 +
>  arch/arm64/mm/Makefile  |   1 +
>  arch/arm64/mm/init.c|  34 +-
>  arch/arm64/mm/mmu.c |   1 +
>  arch/arm64/mm/numa.c| 404 
>  arch/ia64/include/asm/topology.h|   4 -
>  arch/m32r/include/asm/mmzone.h  |   4 +-
>  arch/metag/include/asm/mmzone.h |   4 +-
>  arch/metag/include/asm/topology.h   |   3 -
>  arch/powerpc/include/asm/mmzone.h   |   8 +-
>  arch/powerpc/include/asm/topology.h |   4 -
>  arch/s390/include/asm/mmzone.h  |   6 +-
>  arch/s390/include/asm/pci.h |   2 +-
>  arch/s390/include/asm/topology.h|   1 +
>  arch/sh/include/asm/mmzone.h|   4 +-
>  arch/sh/include/asm/topology.h  |   3 -
>  arch/sparc/include/asm/mmzone.h |   6 +-
>  arch/tile/include/asm/pci.h |   2 -
>  arch/tile/include/asm/topology.h|   3 +
>  arch/x86/include/asm/mmzone.h   |   3 +-
>  arch/x86/include/asm/mmzone_32.h|   5 -
>  arch/x86/include/asm/mmzone_64.h|  17 -
>  arch/x86/include/asm/pci.h  |   2 +-
>  arch/x86/include/asm/topology.h |   1 +
>  drivers/of/Kconfig  |  11 +
>  drivers/of/Makefile |   1 +
>  drivers/of/of_numa.c| 207 ++
>  include/asm-generic/mmzone.h|  24 +
>  include/asm-generic/topology.h  |   4 +-
>  include/linux/of.h  |   4 +
>  41 files changed, 1986 insertions(+), 74 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/numa.txt
>  create mode 100644 arch/arm64/boot/dts/cavium/thunder-88xx-2n.dts
>  create mode 100644 arch/arm64/boot/dts/cavium/thunder-88xx-2n.dtsi
>  create mode 100644 arch/arm64/include/asm/mmzone.h
>  create mode 100644 arch/arm64/include/asm/numa.h
>  create mode 100644 arch/arm64/mm/numa.c
>  delete mode 100644 arch/x86/include/asm/mmzone_64.h
>  create mode 100644 drivers/of/of_numa.c
>  create mode 100644 include/asm-generic/mmzone.h
> 
> -- 
> 1.8.1.4
> 
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v13 04/14] perf, tools: Allow events with dot

[Robert Richter]

On 03.06.15 12:32:04, Jiri Olsa wrote:
 On Tue, Jun 02, 2015 at 10:12:04AM -0700, Sukadev Bhattiprolu wrote:
  From: Andi Kleen a...@linux.intel.com
  
  The Intel events use a dot to separate event name and unit mask.
  Allow dot in names in the scanner, and remove special handling
  of dot as EOF. Also remove the hack in jevents to replace dot
  with underscore. This way dotted events can be specified
  directly by the user.
  
  I'm not fully sure this change to the scanner is correct
  (what was the dot special case good for?), but I haven't
  found anything that breaks with it so far at least.
 
 can't see anything either
 
 Robert,
 does it ring a bell? seems like you introduced it ;-)

It is not a dot, it is a regex to handle any other char or eof to
return from event state to INITIAL and rescan the text again.

The change below does not only add the dot for config but also for
names of groups, pmus and tracepoints (might not be complete, see
PE_NAME).

To be sure this is only valid in config syntax, add

 name_config   [a-zA-Z_*?][a-zA-Z0-9_*?.]*

and change name to name_config in config.

This should be cleaned up with name_minus by merging both.

I also think we can remove PE_NAME from event_term: at all. We need to
return PE_TERM then instead from config.

If you need help with the changes, I could then prepare something and
send it to you, let me know.

-Robert

 
 thanks,
 jirka
 
 
  
  V2: Add the dot to name too, to handle events outside cpu//
  Acked-by: Namhyung Kim namhy...@kernel.org
  Signed-off-by: Andi Kleen a...@linux.intel.com
  Signed-off-by: Sukadev Bhattiprolu suka...@linux.vnet.ibm.com
  ---
   tools/perf/util/parse-events.l |5 ++---
   1 file changed, 2 insertions(+), 3 deletions(-)
  
  diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
  index 09e738f..13cef3c 100644
  --- a/tools/perf/util/parse-events.l
  +++ b/tools/perf/util/parse-events.l
  @@ -119,8 +119,8 @@ event   [^,{}/]+
   num_dec[0-9]+
   num_hex0x[a-fA-F0-9]+
   num_raw_hex[a-fA-F0-9]+
  -name   [a-zA-Z_*?][a-zA-Z0-9_*?]*
  -name_minus [a-zA-Z_*?][a-zA-Z0-9\-_*?]*
  +name   [a-zA-Z_*?][a-zA-Z0-9_*?.]*
  +name_minus [a-zA-Z_*?][a-zA-Z0-9\-_*?.]*
   /* If you add a modifier you need to update check_modifier() */
   modifier_event [ukhpGHSDI]+
   modifier_bp[rwx]{1,3}
  @@ -165,7 +165,6 @@ modifier_bp [rwx]{1,3}
  return PE_EVENT_NAME;
  }
   
  -.  |
   EOF{
  BEGIN(INITIAL);
  REWIND(0);
  -- 
  1.7.9.5
  
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 1/2] oprofile: Disable oprofile NMI timer on ppc64

[Robert Richter]

On 09.04.15 12:52:55, Anton Blanchard wrote:
 We want to enable the hard lockup detector on ppc64, but right now
 that enables the oprofile NMI timer too.
 
 We'd prefer not to enable the oprofile NMI timer, it adds another
 element to our PMU testing and it requires us to increase our
 exported symbols (eg cpu_khz).
 
 Modify the config entry for OPROFILE_NMI_TIMER to disable it on PPC64.
 
 Signed-off-by: Anton Blanchard an...@samba.org

Acked-by: Robert Richter r...@kernel.org

Please pass this patch along with patch 2 of this series (ppc tree or
so).

Thanks,

-Robert

 ---
  arch/Kconfig | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/arch/Kconfig b/arch/Kconfig
 index 05d7a8a..0cc605d 100644
 --- a/arch/Kconfig
 +++ b/arch/Kconfig
 @@ -32,7 +32,7 @@ config HAVE_OPROFILE
  
  config OPROFILE_NMI_TIMER
   def_bool y
 - depends on PERF_EVENTS  HAVE_PERF_EVENTS_NMI
 + depends on PERF_EVENTS  HAVE_PERF_EVENTS_NMI  !PPC64
  
  config KPROBES
   bool Kprobes
 -- 
 2.1.0
 
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 1/2] oprofile: Add HAVE_OPROFILE_NMI_TIMER

[Robert Richter]

On 21.01.15 22:54:08, Anton Blanchard wrote:
 HAVE_PERF_EVENTS_NMI is used for two things - the oprofile NMI timer
 and the hard lockup detector.
 
 Create HAVE_OPROFILE_NMI_TIMER so an architecture can select them
 separately. On ppc64 we want to add the hard lockup detector, but not
 the oprofile NMI timer fallback.

No, this option should depend on HAVE_PERF_EVENTS_NMI. It uses a perf
counter internally, so if perf supports some sort of 'soft' nmi,
oprofile nmi timer would also work well with it.

I also don't see a reason, why you don't want to support oprofile NMI
timer. Is there any?

 @@ -30,9 +30,12 @@ config OPROFILE_EVENT_MULTIPLEX

  config OPROFILE_NMI_TIMER
   def_bool y
 - depends on PERF_EVENTS  HAVE_PERF_EVENTS_NMI
 + depends on PERF_EVENTS  HAVE_OPROFILE_NMI_TIMER

I understand that you might want to disable NMI_TIMER, though I really
don't see a reason if oprofile is enabled and can support it.

If you don't want NMI_TIMER being enabled, then (order of preference):

 * disable it with oprofile (OPROFILE dependency needed for
   NMI_TIMER), or

 * make the default value for NMI_TIMER !PPC64 and add a prompt to let
   the user select/deselect it, or

 * disable OPROFILE_NMI_TIMER by adding a !PPC64 dependency.

-Robert
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: perf: POWER-event translation questions

[Robert Richter]

On 09.11.12 11:26:26, Stephane Eranian wrote:
 On Thu, Nov 8, 2012 at 2:10 AM, Sukadev Bhattiprolu
 suka...@linux.vnet.ibm.com wrote:

  2. Can we allow hyphens in the {name} token  (please see my change to
 util/parse-events.l below). With this change, I can run:
 
 The current code does not support this but Andi fixed that in his HSW patch
 and I use it for the PEBS-LL patch series as well.

perf stat -e cpu/cmplu-stall-bru /tmp/nop
 
 without any changes to the user level tool (parse-events.l) I have
 tested some common cases, not sure if it will break something :-)

But ...

  diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
  index c87efc1..1967bb2 100644
  --- a/tools/perf/util/parse-events.l
  +++ b/tools/perf/util/parse-events.l
  @@ -80,7 +80,7 @@ event [^,{}/]+
   num_dec[0-9]+
   num_hex0x[a-fA-F0-9]+
   num_raw_hex[a-fA-F0-9]+
  -name   [a-zA-Z_*?][a-zA-Z0-9_*?]*
  +name   [-a-zA-Z_*?][-a-zA-Z0-9_*?]*
 ^

... I wouldn't allow hyphens at the beginning of a name.

And, I am wondering if parsing reserved names like 'cpu-cycles' works
too, e.g. cpu/cpu-cycles-foobar/. There are many reserved words in the
lexer with hyphens in it. This might cause unexpected problems.

-Robert
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [RFC][PATCH] perf: Add a few generic stalled-cycles events

[Robert Richter]

Sukadev,

On 15.10.12 17:55:34, Robert Richter wrote:
 On 11.10.12 18:28:39, Sukadev Bhattiprolu wrote:
  +  { .type = PERF_TYPE_HARDWARE, .config = 
  PERF_COUNT_HW_STALLED_CYCLES_FIXED_POINT },
  +  { .type = PERF_TYPE_HARDWARE, .config = 
  PERF_COUNT_HW_STALLED_CYCLES_LOAD_STORE },
  +  { .type = PERF_TYPE_HARDWARE, .config = 
  PERF_COUNT_HW_STALLED_CYCLES_INSTRUCTION_FETCH },
  +  { .type = PERF_TYPE_HARDWARE, .config = 
  PERF_COUNT_HW_STALLED_CYCLES_BRANCH },
 
 Instead of adding new hardware event types I would prefer to use raw
 events in conjunction with sysfs, see e.g. the intel-uncore
 implementation. Something like:
 
  $ find /sys/bus/event_source/devices/cpu/events/
  ...
  /sys/bus/event_source/devices/cpu/events/stalled-cycles-fixed-point
  /sys/bus/event_source/devices/cpu/events/stalled-cycles-load-store
  /sys/bus/event_source/devices/cpu/events/stalled-cycles-instruction-fetch
  /sys/bus/event_source/devices/cpu/events/stalled-cycles-branch
  ...
  $ cat /sys/bus/event_source/devices/cpu/events/stalled-cycles-fixed-point
  event=0xff,umask=0x00
 
 Perf tool works then out-of-the-box with:
 
  $ perf record -e cpu/stalled-cycles-fixed-point/ ...

I refer here to arch/x86/kernel/cpu/perf_event_intel_uncore.c (should
be in v3.7-rc1 or tip:perf/core). See the INTEL_UNCORE_EVENT_DESC()
macro and 'if (type-event_descs) ...' in uncore_type_init(). The code
should be reworked to be non-architectural.

PMU registration is implemented for a longer time already for all
architectures and pmu types:

 /sys/bus/event_source/devices/*

But

 /sys/bus/event_source/devices/*/events/

exists only for a small number of pmus. Perf tool support of this was
implemented with:

 a6146d5 perf/tool: Add PMU event alias support

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [RFC][PATCH] perf: Add a few generic stalled-cycles events

[Robert Richter]

On 11.10.12 18:28:39, Sukadev Bhattiprolu wrote:
 +  { .type = PERF_TYPE_HARDWARE, .config = 
 PERF_COUNT_HW_STALLED_CYCLES_FIXED_POINT },
 +  { .type = PERF_TYPE_HARDWARE, .config = 
 PERF_COUNT_HW_STALLED_CYCLES_LOAD_STORE },
 +  { .type = PERF_TYPE_HARDWARE, .config = 
 PERF_COUNT_HW_STALLED_CYCLES_INSTRUCTION_FETCH },
 +  { .type = PERF_TYPE_HARDWARE, .config = 
 PERF_COUNT_HW_STALLED_CYCLES_BRANCH },

Instead of adding new hardware event types I would prefer to use raw
events in conjunction with sysfs, see e.g. the intel-uncore
implementation. Something like:

 $ find /sys/bus/event_source/devices/cpu/events/
 ...
 /sys/bus/event_source/devices/cpu/events/stalled-cycles-fixed-point
 /sys/bus/event_source/devices/cpu/events/stalled-cycles-load-store
 /sys/bus/event_source/devices/cpu/events/stalled-cycles-instruction-fetch
 /sys/bus/event_source/devices/cpu/events/stalled-cycles-branch
 ...
 $ cat /sys/bus/event_source/devices/cpu/events/stalled-cycles-fixed-point
 event=0xff,umask=0x00

Perf tool works then out-of-the-box with:

 $ perf record -e cpu/stalled-cycles-fixed-point/ ...

The event string can easily be reused by other architectures as a
quasi standard.

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: oprofile callgraph support missing for common cpus

[Robert Richter]

On 17.11.11 22:27:46, Joakim Tjernlund wrote:
   For my e300c2 cpu both if's are false and I don't get support for 
   callgraph/backtrace
   support. Moving the ops-backtrace = op_powerpc_backtrace; to the top
   enables backtrace for me.

 It sure seems to work :)

Backtrace support also works in timer mode. This is already
implemented on arm and sh (out of my mind, have to look at the code to
be sure). This works if the architectural initialization code sets up
ops-backtrace even on failure, which is exactly what you proposed.

 Question, what is the sample rate for timer based oprofile?
 Is it HZ depended?

Yes, it uses the high resolution timer and sets it to HZ which is 4ms
on most architectures. (I did some measurements and for some reason I
got 8ms on x86 though it should be 4ms, but didn't debug this yet.)

Unless there are volunteers I will queue up a patch for v3.3 to enable
backtrace support in timer mode.

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] oprofile, powerpc: Handle events that raise an exception without overflowing

[Robert Richter]

On 23.05.11 10:22:40, Eric B Munson wrote:
 Commit 0837e3242c73566fc1c0196b4ec61779c25ffc93 fixes a situation on POWER7
 where events can roll back if a specualtive event doesn't actually complete.
 This can raise a performance monitor exception.  We need to catch this to 
 ensure
 that we reset the PMC.  In all cases the PMC will be less than 256 cycles from
 overflow.
 
 This patch lifts Anton's fix for the problem in perf and applies it to 
 oprofile
 as well.
 
 Signed-off-by: Eric B Munson emun...@mgebm.net
 Cc: sta...@kernel.org # as far back as it applies cleanly
 ---
  arch/powerpc/oprofile/op_model_power4.c |   24 +++-
  1 files changed, 23 insertions(+), 1 deletions(-)

I applied the fix to oprofile/urgent.

Thanks Eric,

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] powerpc/oprofile: fix potential buffer overrun in op_model_cell.c

[Robert Richter]

On 01.06.10 15:43:34, Denis Kirjanov wrote:
 Fix potential initial_lfsr buffer overrun.
 Writing past the end of the buffer could happen when index == ENTRIES
 
 Signed-off-by: Denis Kirjanov dkirja...@kernel.org

Patch applied to

 git://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile.git urgent

Thanks Denis.

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center
email: robert.rich...@amd.com

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [Patch 0/3] Overview, OProfile SPU event profiling support for IBM Cell processor

[Robert Richter]

On 12.01.09 08:15:27, Carl Love wrote:
 
 On Sun, 2009-01-11 at 10:31 +1100, Benjamin Herrenschmidt wrote:
  On Thu, 2009-01-08 at 16:26 -0800, Carl Love wrote:
   I pulled down the git tree, compiled and installed it.  I tested it
   against the OProfile testsuite, which includes SPU event profiling
   tests.  Everything passed.  The patch I submitted was against a 2.6.26
   tree.  You are now on a 2.6.28 tree so perhaps that is why the patch did
   not apply cleanly.  The patch has been out there for some time.  
  
  When you submited it on Dec 2, it should have been against whatever was
  the latest upstream at the time, not 2.6.26.
  
  Cheers,
  Ben.
 
 Ben, Arnd and Robert:
 
 The patch was against the latest Arnd Cell Kernel tree at the time it
 was posted. The OProfile for cell patches have all been submitted,
 accepted by Arnd (CELL Kernel maintainer) and pushed up stream by Arnd.
 Arnd has waited for the OProfile maintainer to review and approve the
 change before Arnd would push it upstream.  

The patches are already upstream in v2.6.29-rc1. Hope this is ok. I
had added them to the oprofile tree and the tree was merged be Linus.

-Robert

 So, at this point, Robert's (OProfile kernel maintainer) and Maynard's
 (OProfile user space maintainer) approval for the patches.  
 
 The question now goes to Arnd and Robert, who is going to push the
 patches upstream?  Looks like Robert is ready to do it, so Arnd do you
 approve the patches?  Would you like to have Robert push the patches
 upstream or would you prefer to do it?  I think we need an
 answer/agreement on this.  

Arnd, I thought the patches were ok for you. If there are still some
concerns, we have to make delta patches.

See also here:

 git log -4 -p 25006644e6042aab4bb7cdc4bfc5777cd3141df7

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center
email: robert.rich...@amd.com

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [Patch 0/3] Overview, OProfile SPU event profiling support for IBM Cell processor

[Robert Richter]

On 08.01.09 16:26:31, Carl Love wrote:
   git://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile.git cell
  
  The patches did not apply cleanly. I had to change the path to
  arch/powerpc/include/asm/, fix cell/pr_util.h and did some whitespace
  cleanups. Please run your tests on the cell branch.

Thanks Carl, could you also test the 'next' branch on your
architecture, please? This is the latest version of patches to go
upstream (containing your patches also).

Thanks,

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center
email: robert.rich...@amd.com

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [Patch 0/3] Overview, OProfile SPU event profiling support for IBM Cell processor

[Robert Richter]

On 01.12.08 16:18:26, Carl Love wrote:
 This is a rework of the previously posted set of patches.
 
 Patch 1 is the user level patch to add the SPU events to the user
 OProfile tool.  
 
 Patch 2 is a kernel patch to do code clean up and restructuring to make
 it easier to add the new SPU event profiling support.  This patch makes
 no functional changes.
 
 Patch 3 is a kernel patch to add the SPU event profiling support.

I applied patch 2  3 to:

 git://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile.git cell

The patches did not apply cleanly. I had to change the path to
arch/powerpc/include/asm/, fix cell/pr_util.h and did some whitespace
cleanups. Please run your tests on the cell branch.

Thanks,

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center
email: robert.rich...@amd.com

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [UPDATED PATCH VER2] Cell OProfile: Incorrect local array size in activate spu profiling function

[Robert Richter]

On 29.10.08 08:06:45, Carl Love wrote:
 Updated the patch to address comments by Michael Ellerman.  
 Specifically, changed upper limit in for loop to 
 ARRAY_SIZE() macro and added a check to make sure the 
 number of events specified by the user, which is used as
 the max for indexing various arrays, is no bigger then the
 declared size of the arrays.
 
 The size of the pm_signal_local array should be equal to the
 number of SPUs being configured in the array.  Currently, the
 array is of size 4 (NR_PHYS_CTRS) but being indexed by a for 
 loop from 0 to 7 (NUM_SPUS_PER_NODE).  
 
 Signed-off-by: Carl Love [EMAIL PROTECTED]

I applied your patch to oprofile/oprofile-for-tip. Thanks Carl.

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center
email: [EMAIL PROTECTED]

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [Cbe-oss-dev] [PATCH] Cell OProfile: Incorrect local array size in activate spu profiling function

[Robert Richter]

On 24.10.08 11:47:29, Carl Love wrote:
 
 The size of the pm_signal_local array should be equal to the
 number of SPUs being configured in the call.  Currently, the
 array is of size 4 (NR_PHYS_CTRS) but being indexed by a for 
 loop from 0 to 7 (NUM_SPUS_PER_NODE).  
 
 Signed-off-by: Carl Love [EMAIL PROTECTED]

I applied your patch to oprofile/oprofile-for-tip. Thanks Carl.

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center
email: [EMAIL PROTECTED]

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [git pull] Please pull from powerpc.git next branch

[Robert Richter]

Ben,

there are still these OProfile patches for powerpc pending:

Carl Love (1):
  powerpc/cell/oprofile: fix mutex locking for spu-oprofile

Roel Kluin (1):
  powerpc/cell/oprofile: vma_map: fix test on overlay_tbl_offset

Can you or Paul send them upstream? You can pull from here:

git://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile.git powerpc-for-paul

Thanks,

-Robert

On 15.10.08 12:16:36, Benjamin Herrenschmidt wrote:
 Hi Linus !
 
 Here's the powerpc main batch for 2.6.28. It's a bit late mostly due to both
 Paul and I being distracted by other things at the wrong time, and me trying
 to run some more tests ( fixing regressions) before sending it all to you.
 
 There's a bunch of stuff in there, most of it in arch/powerpc, but you'll
 notice a few things touching files out of it. Here's a short summary of
 these:
 
  - These are just a trivial change of CONFIG_PPC_MERGE - CONFIG_PPC
 since the former is no longer useful now that arch/ppc is gone
 
   drivers/block/floppy.c
   drivers/char/ipmi/ipmi_si_intf.c
   drivers/i2c/busses/i2c-pca-isa.c
   drivers/input/serio/i8042-io.h
   drivers/pnp/isapnp/core.c
   drivers/pnp/pnpbios/core.c
 
  - Some other trivial #include fixes for the move of of_device.h from
asm/ to linux/
 
   drivers/hwmon/ams/ams.h
   sound/aoa/soundbus/soundbus.h
 
  - The math-emu changes are two fold. Some trivial compile warning
fixes and some changes to improve exception reporting acked by
davem. Now powerpc uses the generic math-emu code too
 
  - Some powerpc specific drivers. They should all have appropriate acks
with the possible exception of the pata_of_platform.c one which I
merged in while jeff was away and hvc_console for which I believe
we are still maintainers of.
 
  - Some drivers/of additions that should (hopefully) be of no impact to
other users of the OF stuff
 
  - a procfs change removing our obsolete ppc_htab stuff
 
 And that should be it, hopefully I didn't miss anything :-) 
 
 I did a merge with your tree to fixup a couple of conflicts so it should
 be a smooth merge on your side.
 
 Cheers,
 Ben.
 
 The following changes since commit 8acd3a60bcca17c6d89c73cee3ad6057eb83ba1e:
   Linus Torvalds (1):
 Merge branch 'for-2.6.28' of git://linux-nfs.org/~bfields/linux
 
 are available in the git repository at:
 
   git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc.git next
 
 Adrian Bunk (1):
   powerpc: Use bcd2bin/bin2bcd
 
 Anton Vorontsov (12):
   powerpc/83xx: mpc836x_mds: add support for the nor flash
   powerpc/fsl_soc: remove mpc83xx_wdt code
   OF: add fsl,mcu-mpc8349emitx to the exception list
   powerpc: Fix no interrupt handling in pata_of_platform
   of: Add new helper of_parse_phandles_with_args()
   powerpc/QE: move QE_GPIO Kconfig symbol into the platforms/Kconfig
   powerpc/83xx: don't probe broken PCI on mpc837x_mds boards
   powerpc/83xx: add DS1374 RTC support for the MPC837xE-MDS boards
   OF: add fsl,mcu-mpc8349emitx to the exception list
   i2c: MPC8349E-mITX Power Management and GPIO expander driver
   powerpc/83xx: add NAND support for the MPC8360E-RDK boards
   powerpc: fix fsl_upm nand driver modular build
 
 Becky Bruce (10):
   powerpc: Rename PTE_SIZE to HPTE_SIZE
   powerpc/85xx: fix build warning, remove silly cast
   cpm_uart: Pass actual dev ptr to dma_* in ucc and cpm_uart serial
   powerpc: Rename dma_64.c to dma.c
   powerpc: Move iommu dma ops from dma.c to dma-iommu.c
   powerpc: Drop archdata numa_node
   powerpc: Merge 32 and 64-bit dma code
   powerpc: Make dma_addr_t a u64 if CONFIG_PHYS_64BIT is set
   POWERPC: Allow 32-bit hashed pgtable code to support 36-bit physical
   powerpc: Drop redundant machine type print in show_cpuinfo
 
 Benjamin Herrenschmidt (16):
   powerpc: Turn get/set_hard_smp_proccessor_id into inlines
   powerpc: Expose PMCs  cache topology in sysfs on 32-bit
   Merge commit 'kumar/kumar-dma'
   Merge commit 'kumar/kumar-mmu'
   Merge commit 'jwb/jwb-next'
   powerpc: Fix sysfs pci mmap on 32-bit machines with 64-bit PCI
   Merge commit 'jk/jk-merge'
   Merge commit 'gcl/gcl-next'
   Merge commit 'kumar/kumar-next'
   powerpc: Fix DMA offset for non-coherent DMA
   powerpc/pci: Improve detection of unassigned bridge resources
   powerpc: Fix link errors on 32-bit machines using legacy DMA
   powerpc: Fix 32-bit SMP boot on CHRP
   powerpc/chrp: Fix detection of Python PCI host bridge on IBM CHRPs
   powerpc: Fix CHRP PCI config access for indirect_pci
   Merge commit 'origin'
 
 Chandru (1):
   powerpc: Add support for dynamic reconfiguration memory in kexec/kdump 
 kernels
 
 Christian Borntraeger (1):
   hvc_console: Fix free_irq in spinlocked section
 
 Christoph Hellwig (1):
   powerpc: Use sys_pause for 32-bit pause entry point
 
 

Re: [Cbe-oss-dev] powerpc/cell/oprofile: fix mutex locking for spu-oprofile

[Robert Richter]

On 13.10.08 16:53:28, Arnd Bergmann wrote:
 On Monday 25 August 2008, Arnd Bergmann wrote:
  On Monday 25 August 2008, Paul Mackerras wrote:
   
Since rc4 is out now, I understand if you feel more comfortable with
putting the patch into -next instead of -merge.
   
   Linus has been getting stricter about only putting in fixes for
   regressions and serious bugs (see his recent email to Dave Airlie on
   LKML for instance).  I assume that the corruption is just in the data
   that is supplied to userspace and doesn't extend to any kernel data
   structures.
  
  That's right, please queue it for -next then.
 
 I just realized that this patch never made it into powerpc-next after
 all, neither benh nor paulus version. Whoever is handling it today,
 could you please pull
 
  master.kernel.org:/pub/scm/linux/kernel/git/arnd/cell-2.6.git merge
 
 to get this commit below. I have rebased it on top of the current
 benh/powerpc/next branch.

All powerpc oprofile patches are in this branch:

git://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile.git powerpc-for-paul

Pending patches are:

Carl Love (1):
  powerpc/cell/oprofile: fix mutex locking for spu-oprofile

Roel Kluin (1):
  powerpc/cell/oprofile: vma_map: fix test on overlay_tbl_offset

Please pull from there.

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center
email: [EMAIL PROTECTED]

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH] powerpc/cell/oprofile: vma_map: fix test on overlay_tbl_offset

[Robert Richter]

On 07.10.08 10:38:33, Arnd Bergmann wrote:
 From: Roel Kluin [EMAIL PROTECTED]
 
 Offset is unsigned and when an address isn't found in the vma map
 vma_map_lookup() returns the vma physical address + 0x1000.
 
 vma_map_lookup used to return 0x on a failed lookup, but
 a change was made to return the vma physical address + 0x1000
 There are two callers of vam_map_lookup: one of them correctly 
 deals with this new return value, but the other (below) did not.  
 
 Signed-off-by: Roel Kluin [EMAIL PROTECTED]
 Acked-by: Maynard Johnson [EMAIL PROTECTED]
 Signed-off-by: Arnd Bergmann [EMAIL PROTECTED]
 Cc: [EMAIL PROTECTED]
 Cc: Carl Love [EMAIL PROTECTED]

The patch has been applied to the powerpc-for-paul branch of
git://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile.git

Thanks Roel and Arnd,

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center
email: [EMAIL PROTECTED]

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: powerpc/cell/oprofile: fix mutex locking for spu-oprofile

[Robert Richter]

I am fine with the changes with the exception of removing
add_event_entry() from include/linux/oprofile.h. Though there is no
usage of the function also in other architectures anymore, this change
in the API should be discussed on the oprofile mailing list. Please
separate the change in a different patch and submit it to the mailing
list. If there are no objections then, this change can go upstream as
well.

-Robert

On 11.08.08 09:25:07, Arnd Bergmann wrote:
 From: Carl Love [EMAIL PROTECTED]
 
 The issue is the SPU code is not holding the kernel mutex lock while
 adding samples to the kernel buffer.
 
 This patch creates per SPU buffers to hold the data.  Data
 is added to the buffers from in interrupt context.  The data
 is periodically pushed to the kernel buffer via a new Oprofile
 function oprofile_put_buff(). The oprofile_put_buff() function
 is called via a work queue enabling the funtion to acquire the
 mutex lock.
 
 The existing user controls for adjusting the per CPU buffer
 size is used to control the size of the per SPU buffers.
 Similarly, overflows of the SPU buffers are reported by
 incrementing the per CPU buffer stats.  This eliminates the
 need to have architecture specific controls for the per SPU
 buffers which is not acceptable to the OProfile user tool
 maintainer.
 
 The export of the oprofile add_event_entry() is removed as it
 is no longer needed given this patch.
 
 Note, this patch has not addressed the issue of indexing arrays
 by the spu number.  This still needs to be fixed as the spu
 numbering is not guarenteed to be 0 to max_num_spus-1.
 
 Signed-off-by: Carl Love [EMAIL PROTECTED]
 Signed-off-by: Maynard Johnson [EMAIL PROTECTED]
 Signed-off-by: Arnd Bergmann [EMAIL PROTECTED]
 ---
  arch/powerpc/oprofile/cell/pr_util.h   |   13 ++
  arch/powerpc/oprofile/cell/spu_profiler.c  |4 +-
  arch/powerpc/oprofile/cell/spu_task_sync.c |  236 
 +---
  drivers/oprofile/buffer_sync.c |   24 +++
  drivers/oprofile/cpu_buffer.c  |   15 ++-
  drivers/oprofile/event_buffer.h|7 +
  include/linux/oprofile.h   |   16 +-
  7 files changed, 279 insertions(+), 36 deletions(-)
 
 diff --git a/arch/powerpc/oprofile/cell/pr_util.h 
 b/arch/powerpc/oprofile/cell/pr_util.h
 index 22e4e8d..628009c 100644
 --- a/arch/powerpc/oprofile/cell/pr_util.h
 +++ b/arch/powerpc/oprofile/cell/pr_util.h
 @@ -24,6 +24,11 @@
  #define SKIP_GENERIC_SYNC 0
  #define SYNC_START_ERROR -1
  #define DO_GENERIC_SYNC 1
 +#define SPUS_PER_NODE   8
 +#define DEFAULT_TIMER_EXPIRE  (HZ / 10)
 +
 +extern struct delayed_work spu_work;
 +extern int spu_prof_running;
  
  struct spu_overlay_info {/* map of sections within an SPU overlay */
   unsigned int vma;   /* SPU virtual memory address from elf */
 @@ -62,6 +67,14 @@ struct vma_to_fileoffset_map { /* map of sections 
 within an SPU program */
  
  };
  
 +struct spu_buffer {
 + int last_guard_val;
 + int ctx_sw_seen;
 + unsigned long *buff;
 + unsigned int head, tail;
 +};
 +
 +
  /* The three functions below are for maintaining and accessing
   * the vma-to-fileoffset map.
   */
 diff --git a/arch/powerpc/oprofile/cell/spu_profiler.c 
 b/arch/powerpc/oprofile/cell/spu_profiler.c
 index 380d7e2..6edaebd 100644
 --- a/arch/powerpc/oprofile/cell/spu_profiler.c
 +++ b/arch/powerpc/oprofile/cell/spu_profiler.c
 @@ -23,12 +23,11 @@
  
  static u32 *samples;
  
 -static int spu_prof_running;
 +int spu_prof_running;
  static unsigned int profiling_interval;
  
  #define NUM_SPU_BITS_TRBUF 16
  #define SPUS_PER_TB_ENTRY   4
 -#define SPUS_PER_NODE 8
  
  #define SPU_PC_MASK   0x
  
 @@ -208,6 +207,7 @@ int start_spu_profiling(unsigned int cycles_reset)
  
   spu_prof_running = 1;
   hrtimer_start(timer, kt, HRTIMER_MODE_REL);
 + schedule_delayed_work(spu_work, DEFAULT_TIMER_EXPIRE);
  
   return 0;
  }
 diff --git a/arch/powerpc/oprofile/cell/spu_task_sync.c 
 b/arch/powerpc/oprofile/cell/spu_task_sync.c
 index 2a9b4a0..2949126 100644
 --- a/arch/powerpc/oprofile/cell/spu_task_sync.c
 +++ b/arch/powerpc/oprofile/cell/spu_task_sync.c
 @@ -35,7 +35,102 @@ static DEFINE_SPINLOCK(buffer_lock);
  static DEFINE_SPINLOCK(cache_lock);
  static int num_spu_nodes;
  int spu_prof_num_nodes;
 -int last_guard_val[MAX_NUMNODES * 8];
 +
 +struct spu_buffer spu_buff[MAX_NUMNODES * SPUS_PER_NODE];
 +struct delayed_work spu_work;
 +static unsigned max_spu_buff;
 +
 +static void spu_buff_add(unsigned long int value, int spu)
 +{
 + /* spu buff is a circular buffer.  Add entries to the
 +  * head.  Head is the index to store the next value.
 +  * The buffer is full when there is one available entry
 +  * in the queue, i.e. head and tail can't be equal.
 +  * That way we can tell the difference between the
 +  * buffer being full versus empty.
 +  *
 +  *  ASSUPTION: the buffer_lock is held when this function
 + 

Re: powerpc/cell/oprofile: fix mutex locking for spu-oprofile

[Robert Richter]

On 20.08.08 14:05:31, Arnd Bergmann wrote:
 On Wednesday 20 August 2008, Robert Richter wrote:
  I am fine with the changes with the exception of removing
  add_event_entry() from include/linux/oprofile.h. Though there is no
  usage of the function also in other architectures anymore, this change
  in the API should be discussed on the oprofile mailing list. Please
  separate the change in a different patch and submit it to the mailing
  list. If there are no objections then, this change can go upstream as
  well.
 
 As an explanation, the removal of add_event_entry is the whole point
 of this patch. add_event_entry must only be called with buffer_mutex
 held, but buffer_mutex itself is not exported.

Thanks for pointing this out.

 I'm pretty sure that no other user of add_event_entry exists, as it
 was exported specifically for the SPU support and that never worked.
 Any other (theoretical) code using it would be broken in the same way
 and need a corresponding fix.
 
 We can easily leave the declaration in place, but I'd recommend removing
 it eventually. If you prefer to keep it, how about marking it as
 __deprecated?

No, since this is broken by design we remove it. The patch can go
upstream as it is.

Thanks,

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center
email: [EMAIL PROTECTED]

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: powerpc/cell/oprofile: fix mutex locking for spu-oprofile

[Robert Richter]

On 11.08.08 09:25:07, Arnd Bergmann wrote:
 From: Carl Love [EMAIL PROTECTED]
 
 The issue is the SPU code is not holding the kernel mutex lock while
 adding samples to the kernel buffer.
 
 This patch creates per SPU buffers to hold the data.  Data
 is added to the buffers from in interrupt context.  The data
 is periodically pushed to the kernel buffer via a new Oprofile
 function oprofile_put_buff(). The oprofile_put_buff() function
 is called via a work queue enabling the funtion to acquire the
 mutex lock.
 
 The existing user controls for adjusting the per CPU buffer
 size is used to control the size of the per SPU buffers.
 Similarly, overflows of the SPU buffers are reported by
 incrementing the per CPU buffer stats.  This eliminates the
 need to have architecture specific controls for the per SPU
 buffers which is not acceptable to the OProfile user tool
 maintainer.
 
 The export of the oprofile add_event_entry() is removed as it
 is no longer needed given this patch.
 
 Note, this patch has not addressed the issue of indexing arrays
 by the spu number.  This still needs to be fixed as the spu
 numbering is not guarenteed to be 0 to max_num_spus-1.
 
 Signed-off-by: Carl Love [EMAIL PROTECTED]
 Signed-off-by: Maynard Johnson [EMAIL PROTECTED]
 Signed-off-by: Arnd Bergmann [EMAIL PROTECTED]

Acked-by: Robert Richter [EMAIL PROTECTED]

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center
email: [EMAIL PROTECTED]

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: powerpc/cell/oprofile: avoid double free of profile buffer

[Robert Richter]

On 11.08.08 09:25:43, Arnd Bergmann wrote:
 From: Carl Love [EMAIL PROTECTED]
 
 If an error occurs on opcontrol start, the event and per cpu buffers
 are released.  If later opcontrol shutdown is called then the free
 function will be called again to free buffers that no longer
 exist.  This results in a kernel oops.  The following changes
 prevent the call to delete buffers that don't exist.
 
 Signed-off-by: Carl Love [EMAIL PROTECTED]
 Signed-off-by: Arnd Bergmann [EMAIL PROTECTED]

Acked-by: Robert Richter [EMAIL PROTECTED]

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center
email: [EMAIL PROTECTED]

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev