| Message ID | 167604864163.2392965.5102660329807283871.stgit@djiang5-mobl3.local (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | [v6] cxl: add RAS status unmasking for CXL | expand |
On Fri, Feb 10, 2023 at 10:04:03AM -0700, Dave Jiang wrote: > By default the CXL RAS mask registers bits are defaulted to 1's and > suppress all error reporting. If the kernel has negotiated ownership > of error handling for CXL then unmask the mask registers by writing 0s. > > PCI_EXP_AER_FLAGS moved to linux/pci.h header to expose to driver. It > allows exposure of system enabled PCI error flags for the driver to decide > which error bits to toggle. Bjorn suggested that the error enabling should > be controlled from the system policy rather than a driver level choice[1]. > > CXL RAS CE and UE masks are checked against PCI_EXP_AER_FLAGS before > unmasking. > > [1]: https://lore.kernel.org/linux-cxl/20230210122952.00006999@Huawei.com/T/#me8c7f39d43029c64ccff5c950b78a2cee8e885af > +static int cxl_pci_ras_unmask(struct pci_dev *pdev) > +{ > + struct pci_host_bridge *host_bridge = pci_find_host_bridge(pdev->bus); > + struct cxl_dev_state *cxlds = pci_get_drvdata(pdev); > + void __iomem *addr; > + u32 orig_val, val, mask; > + > + if (!cxlds->regs.ras) > + return -ENODEV; > + > + /* BIOS has CXL error control */ > + if (!host_bridge->native_cxl_error) > + return -EOPNOTSUPP; > + > + if (PCI_EXP_AER_FLAGS & PCI_EXP_DEVCTL_URRE) { 1) I don't really want to expose PCI_EXP_AER_FLAGS in linux/pci.h. It's basically a convenience part of the AER implementation. 2) I think your intent here is to configure the CXL RAS masking based on what PCIe error reporting is enabled, but doing it by looking at PCI_EXP_AER_FLAGS doesn't seem right. This expression is a compile-time constant that is always true, but we can't rely on devices always being configured that way. We call pci_aer_init() for every device during enumeration, but we only write PCI_EXP_AER_FLAGS if pci_aer_available() and if pcie_aer_is_native(). And there are a bunch of drivers that call pci_disable_pcie_error_reporting(), which *clears* those flags. I'm not sure those drivers *should* be doing that, but they do today. I'm not sure why this needs to be conditional at all, but if it does, maybe you want to read PCI_EXP_DEVCTL and base it on that? > + addr = cxlds->regs.ras + CXL_RAS_UNCORRECTABLE_MASK_OFFSET; > + orig_val = readl(addr); > + > + mask = CXL_RAS_UNCORRECTABLE_MASK_MASK; Weird name ("_MASK_MASK"), but I assume there's a good reason ;) > + if (!cxl_pci_flit_256(pdev)) > + mask &= ~CXL_RAS_UNCORRECTABLE_MASK_F256B_MASK; > + val = orig_val & ~mask; > + writel(val, addr); > + dev_dbg(&pdev->dev, > + "Uncorrectable RAS Errors Mask: %#x -> %#x\n", > + orig_val, val); > + } > if (cxlds->regs.ras) { > - pci_enable_pcie_error_reporting(pdev); > - rc = devm_add_action_or_reset(&pdev->dev, disable_aer, pdev); > - if (rc) > - return rc; > + rc = pci_enable_pcie_error_reporting(pdev); I see you're just adding a check of return value here, but I'm not sure you need to call pci_enable_pcie_error_reporting() in the first place. Isn't the call in the pci_aer_init() path enough? > +++ b/include/uapi/linux/pci_regs.h > @@ -693,6 +693,7 @@ > #define PCI_EXP_LNKCTL2_TX_MARGIN 0x0380 /* Transmit Margin */ > #define PCI_EXP_LNKCTL2_HASD 0x0020 /* HW Autonomous Speed Disable */ > #define PCI_EXP_LNKSTA2 0x32 /* Link Status 2 */ > +#define PCI_EXP_LNKSTA2_FLIT BIT(10) /* Flit Mode Status */ Please spell out the hex constant. This is to match the style of the surrounding code, and it also gives a hint about the size of the register. Bjorn
Bjorn Helgaas wrote: > On Fri, Feb 10, 2023 at 10:04:03AM -0700, Dave Jiang wrote: > > By default the CXL RAS mask registers bits are defaulted to 1's and > > suppress all error reporting. If the kernel has negotiated ownership > > of error handling for CXL then unmask the mask registers by writing 0s. > > > > PCI_EXP_AER_FLAGS moved to linux/pci.h header to expose to driver. It > > allows exposure of system enabled PCI error flags for the driver to decide > > which error bits to toggle. Bjorn suggested that the error enabling should > > be controlled from the system policy rather than a driver level choice[1]. > > > > CXL RAS CE and UE masks are checked against PCI_EXP_AER_FLAGS before > > unmasking. > > > > [1]: https://lore.kernel.org/linux-cxl/20230210122952.00006999@Huawei.com/T/#me8c7f39d43029c64ccff5c950b78a2cee8e885af > > > +static int cxl_pci_ras_unmask(struct pci_dev *pdev) > > +{ > > + struct pci_host_bridge *host_bridge = pci_find_host_bridge(pdev->bus); > > + struct cxl_dev_state *cxlds = pci_get_drvdata(pdev); > > + void __iomem *addr; > > + u32 orig_val, val, mask; > > + > > + if (!cxlds->regs.ras) > > + return -ENODEV; > > + > > + /* BIOS has CXL error control */ > > + if (!host_bridge->native_cxl_error) > > + return -EOPNOTSUPP; > > + > > + if (PCI_EXP_AER_FLAGS & PCI_EXP_DEVCTL_URRE) { > > 1) I don't really want to expose PCI_EXP_AER_FLAGS in linux/pci.h. > It's basically a convenience part of the AER implementation. > > 2) I think your intent here is to configure the CXL RAS masking based > on what PCIe error reporting is enabled, but doing it by looking at > PCI_EXP_AER_FLAGS doesn't seem right. This expression is a > compile-time constant that is always true, but we can't rely on > devices always being configured that way. Oh, I had asked Dave to do that to try to satisfy your request for a system wide policy. So that if someone wanted to modify what errors get unmasked globally just look at that value rather than re-read the register, but it seems I over-intepreted what you and Jonathan were talking about here when you mention "system policy": https://lore.kernel.org/linux-cxl/20221229172731.GA611562@bhelgaas/ > We call pci_aer_init() for every device during enumeration, but we > only write PCI_EXP_AER_FLAGS if pci_aer_available() and if > pcie_aer_is_native(). And there are a bunch of drivers that call > pci_disable_pcie_error_reporting(), which *clears* those flags. I'm > not sure those drivers *should* be doing that, but they do today. > > I'm not sure why this needs to be conditional at all, but if it does, > maybe you want to read PCI_EXP_DEVCTL and base it on that? Re-reading the register works too. Some of this may want to move to a more core location once/if CXL devices beyond the generic Memory Expander Class Code start arriving, but that can be saved for later.
On 2/10/23 3:52 PM, Bjorn Helgaas wrote: > On Fri, Feb 10, 2023 at 10:04:03AM -0700, Dave Jiang wrote: >> By default the CXL RAS mask registers bits are defaulted to 1's and >> suppress all error reporting. If the kernel has negotiated ownership >> of error handling for CXL then unmask the mask registers by writing 0s. >> >> PCI_EXP_AER_FLAGS moved to linux/pci.h header to expose to driver. It >> allows exposure of system enabled PCI error flags for the driver to decide >> which error bits to toggle. Bjorn suggested that the error enabling should >> be controlled from the system policy rather than a driver level choice[1]. >> >> CXL RAS CE and UE masks are checked against PCI_EXP_AER_FLAGS before >> unmasking. >> >> [1]: https://lore.kernel.org/linux-cxl/20230210122952.00006999@Huawei.com/T/#me8c7f39d43029c64ccff5c950b78a2cee8e885af > >> +static int cxl_pci_ras_unmask(struct pci_dev *pdev) >> +{ >> + struct pci_host_bridge *host_bridge = pci_find_host_bridge(pdev->bus); >> + struct cxl_dev_state *cxlds = pci_get_drvdata(pdev); >> + void __iomem *addr; >> + u32 orig_val, val, mask; >> + >> + if (!cxlds->regs.ras) >> + return -ENODEV; >> + >> + /* BIOS has CXL error control */ >> + if (!host_bridge->native_cxl_error) >> + return -EOPNOTSUPP; >> + >> + if (PCI_EXP_AER_FLAGS & PCI_EXP_DEVCTL_URRE) { > > 1) I don't really want to expose PCI_EXP_AER_FLAGS in linux/pci.h. > It's basically a convenience part of the AER implementation. > > 2) I think your intent here is to configure the CXL RAS masking based > on what PCIe error reporting is enabled, but doing it by looking at > PCI_EXP_AER_FLAGS doesn't seem right. This expression is a > compile-time constant that is always true, but we can't rely on > devices always being configured that way. > > We call pci_aer_init() for every device during enumeration, but we > only write PCI_EXP_AER_FLAGS if pci_aer_available() and if > pcie_aer_is_native(). And there are a bunch of drivers that call > pci_disable_pcie_error_reporting(), which *clears* those flags. I'm > not sure those drivers *should* be doing that, but they do today. > > I'm not sure why this needs to be conditional at all, but if it does, > maybe you want to read PCI_EXP_DEVCTL and base it on that? Ok I'll read the PCI_EXP_DEVCTL. Looking to only unmask the relevant RAS reporting if respective PCIe bits are enabled. > >> + addr = cxlds->regs.ras + CXL_RAS_UNCORRECTABLE_MASK_OFFSET; >> + orig_val = readl(addr); >> + >> + mask = CXL_RAS_UNCORRECTABLE_MASK_MASK; > > Weird name ("_MASK_MASK"), but I assume there's a good reason ;) Yes. It's the mask of the error mask register. Is that too much of a mouthful? I can take out the second mask. > >> + if (!cxl_pci_flit_256(pdev)) >> + mask &= ~CXL_RAS_UNCORRECTABLE_MASK_F256B_MASK; >> + val = orig_val & ~mask; >> + writel(val, addr); >> + dev_dbg(&pdev->dev, >> + "Uncorrectable RAS Errors Mask: %#x -> %#x\n", >> + orig_val, val); >> + } > >> if (cxlds->regs.ras) { >> - pci_enable_pcie_error_reporting(pdev); >> - rc = devm_add_action_or_reset(&pdev->dev, disable_aer, pdev); >> - if (rc) >> - return rc; >> + rc = pci_enable_pcie_error_reporting(pdev); > > I see you're just adding a check of return value here, but I'm not > sure you need to call pci_enable_pcie_error_reporting() in the first > place. Isn't the call in the pci_aer_init() path enough? I guess I'm confused by the kernel documentation: " pci_enable_pcie_error_reporting enables the device to send error messages to root port when an error is detected. Note that devices don't enable the error reporting by default, so device drivers need call this function to enable it. " Seems to indicate that driver should always call this if it wants AER reporting? > >> +++ b/include/uapi/linux/pci_regs.h >> @@ -693,6 +693,7 @@ >> #define PCI_EXP_LNKCTL2_TX_MARGIN 0x0380 /* Transmit Margin */ >> #define PCI_EXP_LNKCTL2_HASD 0x0020 /* HW Autonomous Speed Disable */ >> #define PCI_EXP_LNKSTA2 0x32 /* Link Status 2 */ >> +#define PCI_EXP_LNKSTA2_FLIT BIT(10) /* Flit Mode Status */ > > Please spell out the hex constant. This is to match the style of the > surrounding code, and it also gives a hint about the size of the > register. Ok will fix > > Bjorn
On Fri, Feb 10, 2023 at 04:46:15PM -0700, Dave Jiang wrote: > On 2/10/23 3:52 PM, Bjorn Helgaas wrote: > > On Fri, Feb 10, 2023 at 10:04:03AM -0700, Dave Jiang wrote: > > > By default the CXL RAS mask registers bits are defaulted to 1's and > > > suppress all error reporting. If the kernel has negotiated ownership > > > of error handling for CXL then unmask the mask registers by writing 0s. > > > > > > PCI_EXP_AER_FLAGS moved to linux/pci.h header to expose to driver. It > > > allows exposure of system enabled PCI error flags for the driver to decide > > > which error bits to toggle. Bjorn suggested that the error enabling should > > > be controlled from the system policy rather than a driver level choice[1]. > > > > > > CXL RAS CE and UE masks are checked against PCI_EXP_AER_FLAGS before > > > unmasking. > > > > > > [1]: https://lore.kernel.org/linux-cxl/20230210122952.00006999@Huawei.com/T/#me8c7f39d43029c64ccff5c950b78a2cee8e885af > > > > > +static int cxl_pci_ras_unmask(struct pci_dev *pdev) > > > +{ > > > + struct pci_host_bridge *host_bridge = pci_find_host_bridge(pdev->bus); > > > + struct cxl_dev_state *cxlds = pci_get_drvdata(pdev); > > > + void __iomem *addr; > > > + u32 orig_val, val, mask; > > > + > > > + if (!cxlds->regs.ras) > > > + return -ENODEV; > > > + > > > + /* BIOS has CXL error control */ > > > + if (!host_bridge->native_cxl_error) > > > + return -EOPNOTSUPP; > > > + > > > + if (PCI_EXP_AER_FLAGS & PCI_EXP_DEVCTL_URRE) { > > > > 1) I don't really want to expose PCI_EXP_AER_FLAGS in linux/pci.h. > > It's basically a convenience part of the AER implementation. > > > > 2) I think your intent here is to configure the CXL RAS masking based > > on what PCIe error reporting is enabled, but doing it by looking at > > PCI_EXP_AER_FLAGS doesn't seem right. This expression is a > > compile-time constant that is always true, but we can't rely on > > devices always being configured that way. > > > > We call pci_aer_init() for every device during enumeration, but we > > only write PCI_EXP_AER_FLAGS if pci_aer_available() and if > > pcie_aer_is_native(). And there are a bunch of drivers that call > > pci_disable_pcie_error_reporting(), which *clears* those flags. I'm > > not sure those drivers *should* be doing that, but they do today. > > > > I'm not sure why this needs to be conditional at all, but if it does, > > maybe you want to read PCI_EXP_DEVCTL and base it on that? > > Ok I'll read the PCI_EXP_DEVCTL. Looking to only unmask the relevant RAS > reporting if respective PCIe bits are enabled. That sounds OK to me, but leaves the question of those drivers that call pci_disable_pcie_error_reporting() because CXL won't know about that. But maybe that's not a problem, I dunno. > > I see you're just adding a check of return value here, but I'm not > > sure you need to call pci_enable_pcie_error_reporting() in the first > > place. Isn't the call in the pci_aer_init() path enough? > > I guess I'm confused by the kernel documentation: > " > pci_enable_pcie_error_reporting enables the device to send error > messages to root port when an error is detected. Note that devices > don't enable the error reporting by default, so device drivers need > call this function to enable it. > " > > Seems to indicate that driver should always call this if it wants AER > reporting? Oh, thanks for pointing that out! I'll update that doc to match the current code, which *does* enable reporting by default: commit f26e58bf6f54 ("PCI/AER: Enable error reporting when AER is native") Author: Stefan Roese <sr@denx.de> Date: Tue Jan 25 08:18:20 2022 +0100 PCI/AER: Enable error reporting when AER is native If we have native control of AER, set the following error reporting enable bits: - Correctable Error Reporting Enable - Non-Fatal Error Reporting Enable - Fatal Error Reporting Enable - Unsupported Request Reporting Enable Note that these bits are all in the Device Control register and are not AER-specific. This affects all devices with an AER capability, including hot-added devices. Please note that this change is quite invasive, as error reporting now will be enabled for all available PCIe Endpoints, which was previously not the case. When "pci=noaer" is selected, error reporting stays disabled of course.
On Fri, Feb 10, 2023 at 03:38:53PM -0800, Dan Williams wrote: > Bjorn Helgaas wrote: > > On Fri, Feb 10, 2023 at 10:04:03AM -0700, Dave Jiang wrote: > > > By default the CXL RAS mask registers bits are defaulted to 1's and > > > suppress all error reporting. If the kernel has negotiated ownership > > > of error handling for CXL then unmask the mask registers by writing 0s. > > > > > > PCI_EXP_AER_FLAGS moved to linux/pci.h header to expose to driver. It > > > allows exposure of system enabled PCI error flags for the driver to decide > > > which error bits to toggle. Bjorn suggested that the error enabling should > > > be controlled from the system policy rather than a driver level choice[1]. > > > > > > CXL RAS CE and UE masks are checked against PCI_EXP_AER_FLAGS before > > > unmasking. > > > > > > [1]: https://lore.kernel.org/linux-cxl/20230210122952.00006999@Huawei.com/T/#me8c7f39d43029c64ccff5c950b78a2cee8e885af > > > > > +static int cxl_pci_ras_unmask(struct pci_dev *pdev) > > > +{ > > > + struct pci_host_bridge *host_bridge = pci_find_host_bridge(pdev->bus); > > > + struct cxl_dev_state *cxlds = pci_get_drvdata(pdev); > > > + void __iomem *addr; > > > + u32 orig_val, val, mask; > > > + > > > + if (!cxlds->regs.ras) > > > + return -ENODEV; > > > + > > > + /* BIOS has CXL error control */ > > > + if (!host_bridge->native_cxl_error) > > > + return -EOPNOTSUPP; > > > + > > > + if (PCI_EXP_AER_FLAGS & PCI_EXP_DEVCTL_URRE) { > > > > 1) I don't really want to expose PCI_EXP_AER_FLAGS in linux/pci.h. > > It's basically a convenience part of the AER implementation. > > > > 2) I think your intent here is to configure the CXL RAS masking based > > on what PCIe error reporting is enabled, but doing it by looking at > > PCI_EXP_AER_FLAGS doesn't seem right. This expression is a > > compile-time constant that is always true, but we can't rely on > > devices always being configured that way. > > Oh, I had asked Dave to do that to try to satisfy your request for a > system wide policy. So that if someone wanted to modify what errors get > unmasked globally just look at that value rather than re-read the > register, but it seems I over-intepreted what you and Jonathan were > talking about here when you mention "system policy": > > https://lore.kernel.org/linux-cxl/20221229172731.GA611562@bhelgaas/ Yeah, I should have worded that more like "I think the PCI core, not individual drivers, should be responsible for AER configuration." Even if the PCI core does it all, that doesn't mean AER configuration is known at compile-time because it depends on _OSC negotiation and some kernel parameter special cases. Bjorn
On 2/10/23 5:11 PM, Bjorn Helgaas wrote: > On Fri, Feb 10, 2023 at 04:46:15PM -0700, Dave Jiang wrote: >> On 2/10/23 3:52 PM, Bjorn Helgaas wrote: >>> On Fri, Feb 10, 2023 at 10:04:03AM -0700, Dave Jiang wrote: >>>> By default the CXL RAS mask registers bits are defaulted to 1's and >>>> suppress all error reporting. If the kernel has negotiated ownership >>>> of error handling for CXL then unmask the mask registers by writing 0s. >>>> >>>> PCI_EXP_AER_FLAGS moved to linux/pci.h header to expose to driver. It >>>> allows exposure of system enabled PCI error flags for the driver to decide >>>> which error bits to toggle. Bjorn suggested that the error enabling should >>>> be controlled from the system policy rather than a driver level choice[1]. >>>> >>>> CXL RAS CE and UE masks are checked against PCI_EXP_AER_FLAGS before >>>> unmasking. >>>> >>>> [1]: https://lore.kernel.org/linux-cxl/20230210122952.00006999@Huawei.com/T/#me8c7f39d43029c64ccff5c950b78a2cee8e885af >>> >>>> +static int cxl_pci_ras_unmask(struct pci_dev *pdev) >>>> +{ >>>> + struct pci_host_bridge *host_bridge = pci_find_host_bridge(pdev->bus); >>>> + struct cxl_dev_state *cxlds = pci_get_drvdata(pdev); >>>> + void __iomem *addr; >>>> + u32 orig_val, val, mask; >>>> + >>>> + if (!cxlds->regs.ras) >>>> + return -ENODEV; >>>> + >>>> + /* BIOS has CXL error control */ >>>> + if (!host_bridge->native_cxl_error) >>>> + return -EOPNOTSUPP; >>>> + >>>> + if (PCI_EXP_AER_FLAGS & PCI_EXP_DEVCTL_URRE) { >>> >>> 1) I don't really want to expose PCI_EXP_AER_FLAGS in linux/pci.h. >>> It's basically a convenience part of the AER implementation. >>> >>> 2) I think your intent here is to configure the CXL RAS masking based >>> on what PCIe error reporting is enabled, but doing it by looking at >>> PCI_EXP_AER_FLAGS doesn't seem right. This expression is a >>> compile-time constant that is always true, but we can't rely on >>> devices always being configured that way. >>> >>> We call pci_aer_init() for every device during enumeration, but we >>> only write PCI_EXP_AER_FLAGS if pci_aer_available() and if >>> pcie_aer_is_native(). And there are a bunch of drivers that call >>> pci_disable_pcie_error_reporting(), which *clears* those flags. I'm >>> not sure those drivers *should* be doing that, but they do today. >>> >>> I'm not sure why this needs to be conditional at all, but if it does, >>> maybe you want to read PCI_EXP_DEVCTL and base it on that? >> >> Ok I'll read the PCI_EXP_DEVCTL. Looking to only unmask the relevant RAS >> reporting if respective PCIe bits are enabled. > > That sounds OK to me, but leaves the question of those drivers that > call pci_disable_pcie_error_reporting() because CXL won't know about > that. But maybe that's not a problem, I dunno. Currently the CXL subsystem covers the type-3 devices so I don't think it'll be an issue. type-2 may be an issue but it doesn't go through the current driver. Maybe we'll figure out how to deal with that when those show device drivers show up. > >>> I see you're just adding a check of return value here, but I'm not >>> sure you need to call pci_enable_pcie_error_reporting() in the first >>> place. Isn't the call in the pci_aer_init() path enough? >> >> I guess I'm confused by the kernel documentation: >> " >> pci_enable_pcie_error_reporting enables the device to send error >> messages to root port when an error is detected. Note that devices >> don't enable the error reporting by default, so device drivers need >> call this function to enable it. >> " >> >> Seems to indicate that driver should always call this if it wants AER >> reporting? > > Oh, thanks for pointing that out! I'll update that doc to match the > current code, which *does* enable reporting by default: Ah ok. I shall remove the calling of pci_enable_pcie_error_reporting. > > commit f26e58bf6f54 ("PCI/AER: Enable error reporting when AER is native") > Author: Stefan Roese <sr@denx.de> > Date: Tue Jan 25 08:18:20 2022 +0100 > > PCI/AER: Enable error reporting when AER is native > > If we have native control of AER, set the following error reporting enable > bits: > > - Correctable Error Reporting Enable > - Non-Fatal Error Reporting Enable > - Fatal Error Reporting Enable > - Unsupported Request Reporting Enable > > Note that these bits are all in the Device Control register and are not > AER-specific. > > This affects all devices with an AER capability, including hot-added > devices. > > Please note that this change is quite invasive, as error reporting now will > be enabled for all available PCIe Endpoints, which was previously not the > case. > > When "pci=noaer" is selected, error reporting stays disabled of course.
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h index b3964149c77b..d640fe61b893 100644 --- a/drivers/cxl/cxl.h +++ b/drivers/cxl/cxl.h @@ -130,6 +130,7 @@ static inline int ways_to_eiw(unsigned int ways, u8 *eiw) #define CXL_RAS_UNCORRECTABLE_STATUS_MASK (GENMASK(16, 14) | GENMASK(11, 0)) #define CXL_RAS_UNCORRECTABLE_MASK_OFFSET 0x4 #define CXL_RAS_UNCORRECTABLE_MASK_MASK (GENMASK(16, 14) | GENMASK(11, 0)) +#define CXL_RAS_UNCORRECTABLE_MASK_F256B_MASK BIT(8) #define CXL_RAS_UNCORRECTABLE_SEVERITY_OFFSET 0x8 #define CXL_RAS_UNCORRECTABLE_SEVERITY_MASK (GENMASK(16, 14) | GENMASK(11, 0)) #define CXL_RAS_CORRECTABLE_STATUS_OFFSET 0xC diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c index 4cf9a2191602..4bb41cf581df 100644 --- a/drivers/cxl/pci.c +++ b/drivers/cxl/pci.c @@ -642,6 +642,59 @@ static int cxl_event_config(struct pci_host_bridge *host_bridge, return 0; } +/* + * CXL v3.0 6.2.3 Table 6-4 + * The table indicates that if PCIe Flit Mode is set, then CXL is in 256B flits + * mode, otherwise it's 68B flits mode. + */ +static bool cxl_pci_flit_256(struct pci_dev *pdev) +{ + u32 lnksta2; + + pcie_capability_read_dword(pdev, PCI_EXP_LNKSTA2, &lnksta2); + return lnksta2 & PCI_EXP_LNKSTA2_FLIT; +} + +static int cxl_pci_ras_unmask(struct pci_dev *pdev) +{ + struct pci_host_bridge *host_bridge = pci_find_host_bridge(pdev->bus); + struct cxl_dev_state *cxlds = pci_get_drvdata(pdev); + void __iomem *addr; + u32 orig_val, val, mask; + + if (!cxlds->regs.ras) + return -ENODEV; + + /* BIOS has CXL error control */ + if (!host_bridge->native_cxl_error) + return -EOPNOTSUPP; + + if (PCI_EXP_AER_FLAGS & PCI_EXP_DEVCTL_URRE) { + addr = cxlds->regs.ras + CXL_RAS_UNCORRECTABLE_MASK_OFFSET; + orig_val = readl(addr); + + mask = CXL_RAS_UNCORRECTABLE_MASK_MASK; + if (!cxl_pci_flit_256(pdev)) + mask &= ~CXL_RAS_UNCORRECTABLE_MASK_F256B_MASK; + val = orig_val & ~mask; + writel(val, addr); + dev_dbg(&pdev->dev, + "Uncorrectable RAS Errors Mask: %#x -> %#x\n", + orig_val, val); + } + + if (PCI_EXP_AER_FLAGS & PCI_EXP_DEVCTL_CERE) { + addr = cxlds->regs.ras + CXL_RAS_CORRECTABLE_MASK_OFFSET; + orig_val = readl(addr); + val = orig_val & ~CXL_RAS_CORRECTABLE_MASK_MASK; + writel(val, addr); + dev_dbg(&pdev->dev, "Correctable RAS Errors Mask: %#x -> %#x\n", + orig_val, val); + } + + return 0; +} + static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) { struct pci_host_bridge *host_bridge = pci_find_host_bridge(pdev->bus); @@ -734,10 +787,15 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) return rc; if (cxlds->regs.ras) { - pci_enable_pcie_error_reporting(pdev); - rc = devm_add_action_or_reset(&pdev->dev, disable_aer, pdev); - if (rc) - return rc; + rc = pci_enable_pcie_error_reporting(pdev); + if (!rc) { + cxl_pci_ras_unmask(pdev); + rc = devm_add_action_or_reset(&pdev->dev, disable_aer, pdev); + if (rc) + return rc; + } else { + dev_warn(&pdev->dev, "Failed to enable PCIE AER.\n"); + } } pci_save_state(pdev); diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c index 625f7b2cafe4..21af538ee4a0 100644 --- a/drivers/pci/pcie/aer.c +++ b/drivers/pci/pcie/aer.c @@ -214,9 +214,6 @@ void pcie_ecrc_get_policy(char *str) } #endif /* CONFIG_PCIE_ECRC */ -#define PCI_EXP_AER_FLAGS (PCI_EXP_DEVCTL_CERE | PCI_EXP_DEVCTL_NFERE | \ - PCI_EXP_DEVCTL_FERE | PCI_EXP_DEVCTL_URRE) - int pcie_aer_is_native(struct pci_dev *dev) { struct pci_host_bridge *host = pci_find_host_bridge(dev->bus); diff --git a/include/linux/pci.h b/include/linux/pci.h index 22319ea71ab0..508eb3659762 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -2545,4 +2545,6 @@ void pci_ims_free_irq(struct pci_dev *pdev, struct msi_map map); WARN_ONCE(condition, "%s %s: " fmt, \ dev_driver_string(&(pdev)->dev), pci_name(pdev), ##arg) +#define PCI_EXP_AER_FLAGS (PCI_EXP_DEVCTL_CERE | PCI_EXP_DEVCTL_NFERE | \ + PCI_EXP_DEVCTL_FERE | PCI_EXP_DEVCTL_URRE) #endif /* LINUX_PCI_H */ diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h index 85ab1278811e..f19e5ccf5cc1 100644 --- a/include/uapi/linux/pci_regs.h +++ b/include/uapi/linux/pci_regs.h @@ -693,6 +693,7 @@ #define PCI_EXP_LNKCTL2_TX_MARGIN 0x0380 /* Transmit Margin */ #define PCI_EXP_LNKCTL2_HASD 0x0020 /* HW Autonomous Speed Disable */ #define PCI_EXP_LNKSTA2 0x32 /* Link Status 2 */ +#define PCI_EXP_LNKSTA2_FLIT BIT(10) /* Flit Mode Status */ #define PCI_CAP_EXP_ENDPOINT_SIZEOF_V2 0x32 /* end of v2 EPs w/ link */ #define PCI_EXP_SLTCAP2 0x34 /* Slot Capabilities 2 */ #define PCI_EXP_SLTCAP2_IBPD 0x00000001 /* In-band PD Disable Supported */