On Thu, Dec 05, 2024 at 07:40:48PM +0800, LeoLiu-oc wrote:
> From: LeoLiuoc <LeoLiu-oc@xxxxxxxxxxx>
>
> Call the func pci_acpi_program_hest_aer_params() for every PCIe device,
> the purpose of this function is to extract register value from HEST PCIe
> AER structures and program them into AER Capabilities. This function
> applies to all hardware platforms that has a PCI Express AER structure
> in HEST.
>
> Signed-off-by: LeoLiuoc <LeoLiu-oc@xxxxxxxxxxx>
> ---
> drivers/pci/pci-acpi.c | 103 +++++++++++++++++++++++++++++++++++++++++
> drivers/pci/pci.h | 9 ++++
> drivers/pci/probe.c | 1 +
> 3 files changed, 113 insertions(+)
>
> diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
> index af370628e583..6e29af8e6cc4 100644
> --- a/drivers/pci/pci-acpi.c
> +++ b/drivers/pci/pci-acpi.c
> @@ -19,6 +19,7 @@
> #include <linux/pm_runtime.h>
> #include <linux/pm_qos.h>
> #include <linux/rwsem.h>
> +#include <acpi/apei.h>
> #include "pci.h"
>
> /*
> @@ -806,6 +807,108 @@ int pci_acpi_program_hp_params(struct pci_dev *dev)
> return -ENODEV;
> }
>
> +#ifdef CONFIG_ACPI_APEI
> +/*
> + * program_hest_aer_common() - configure AER common registers for Root Ports,
> + * Endpoints and PCIe to PCI/PCI-X bridges
> + */
> +static void program_hest_aer_common(struct acpi_hest_aer_common aer_common,
> + struct pci_dev *dev, int pos)
> +{
> + u32 uncor_mask;
> + u32 uncor_severity;
> + u32 cor_mask;
> + u32 adv_cap;
> +
> + uncor_mask = aer_common.uncorrectable_mask;
> + uncor_severity = aer_common.uncorrectable_severity;
> + cor_mask = aer_common.correctable_mask;
> + adv_cap = aer_common.advanced_capabilities;
> +
These can be done at the same time as the declarations. Same for the
remaining functions.
> + pci_write_config_dword(dev, pos + PCI_ERR_UNCOR_MASK, uncor_mask);
> + pci_write_config_dword(dev, pos + PCI_ERR_UNCOR_SEVER, uncor_severity);
> + pci_write_config_dword(dev, pos + PCI_ERR_COR_MASK, cor_mask);
> + pci_write_config_dword(dev, pos + PCI_ERR_CAP, adv_cap);
> +}
> +
> +static void program_hest_aer_root(struct acpi_hest_aer_root *aer_root,
> + struct pci_dev *dev, int pos)
> +{
> + u32 root_err_cmd;
> +
> + root_err_cmd = aer_root->root_error_command;
> +
> + pci_write_config_dword(dev, pos + PCI_ERR_ROOT_COMMAND, root_err_cmd);
> +}
> +
> +static void program_hest_aer_bridge(struct acpi_hest_aer_bridge *hest_aer_bridge,
> + struct pci_dev *dev, int pos)
> +{
> + u32 uncor_mask2;
> + u32 uncor_severity2;
> + u32 adv_cap2;
> +
> + uncor_mask2 = hest_aer_bridge->uncorrectable_mask2;
> + uncor_severity2 = hest_aer_bridge->uncorrectable_severity2;
> + adv_cap2 = hest_aer_bridge->advanced_capabilities2;
> +
> + pci_write_config_dword(dev, pos + PCI_ERR_UNCOR_MASK2, uncor_mask2);
> + pci_write_config_dword(dev, pos + PCI_ERR_UNCOR_SEVER2, uncor_severity2);
> + pci_write_config_dword(dev, pos + PCI_ERR_CAP2, adv_cap2);
> +}
> +
> +static void program_hest_aer_params(struct hest_parse_aer_info info)
> +{
> + struct pci_dev *dev;
> + int port_type;
> + int pos;
> + struct acpi_hest_aer_root *hest_aer_root;
> + struct acpi_hest_aer *hest_aer_endpoint;
> + struct acpi_hest_aer_bridge *hest_aer_bridge;
> +
> + dev = info.pci_dev;
> + port_type = pci_pcie_type(dev);
> + pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR);
> + if (!pos)
> + return;
> +
> + switch (port_type) {
> + case PCI_EXP_TYPE_ROOT_PORT:
> + hest_aer_root = info.hest_aer_root_port;
> + program_hest_aer_common(hest_aer_root->aer, dev, pos);
> + program_hest_aer_root(hest_aer_root, dev, pos);
> + break;
> + case PCI_EXP_TYPE_ENDPOINT:
> + hest_aer_endpoint = info.hest_aer_endpoint;
> + program_hest_aer_common(hest_aer_endpoint->aer, dev, pos);
> + break;
> + case PCI_EXP_TYPE_PCI_BRIDGE:
> + hest_aer_bridge = info.hest_aer_bridge;
> + program_hest_aer_common(hest_aer_bridge->aer, dev, pos);
> + program_hest_aer_bridge(hest_aer_bridge, dev, pos);
> + break;
> + default:
> + return;
> + break;
> + }
> +}
> +
> +int pci_acpi_program_hest_aer_params(struct pci_dev *dev)
> +{
> + struct hest_parse_aer_info info = {
> + .pci_dev = dev
> + };
> +
> + if (!pci_is_pcie(dev))
> + return -ENODEV;
> +
> + if (apei_hest_parse(hest_parse_pcie_aer, &info) == 1)
Don't need the "== 1".
> + program_hest_aer_params(info);
> +
> + return 0;
> +}
> +#endif
> +
> /**
> * pciehp_is_native - Check whether a hotplug port is handled by the OS
> * @bridge: Hotplug port to check
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 2e40fc63ba31..78bdc121c905 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -897,6 +897,15 @@ static inline void pci_save_aer_state(struct pci_dev *dev) { }
> static inline void pci_restore_aer_state(struct pci_dev *dev) { }
> #endif
>
> +#ifdef CONFIG_ACPI_APEI
> +int pci_acpi_program_hest_aer_params(struct pci_dev *dev);
The return value is never checked, so this can return void.
> +#else
> +static inline int pci_acpi_program_hest_aer_params(struct pci_dev *dev)
> +{
> + return 0;
> +}
> +#endif
> +
> #ifdef CONFIG_ACPI
> bool pci_acpi_preserve_config(struct pci_host_bridge *bridge);
> int pci_acpi_program_hp_params(struct pci_dev *dev);
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 2e81ab0f5a25..33b8b46ca554 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -2304,6 +2304,7 @@ static void pci_configure_device(struct pci_dev *dev)
> pci_configure_serr(dev);
>
> pci_acpi_program_hp_params(dev);
> + pci_acpi_program_hest_aer_params(dev);
This should not be called here unconditionally.
OS should only write AER registers if granted permission through
_OSC.
It would be more appropriate to call this from pci_aer_init().
Thanks,
Yazen
