PCI: ACPI: Implement support for _HPX Type 3 tables

PATCH RESEND New Alexandru Gagniuc 2 participants 6 messages Mar 7, 2019 – Apr 23, 2019

Subsystems (2)

ACPI PCI

lore

Share & tools

lore

Alexandru Gagniuc

author

2019-03-07 21:38 UTC #0 lore raw

cover PCI: ACPI: Implement support for _HPX Type 3 tables

Every decade or so, give or take a decade, our ACPI overlords realize that the table they so generously gifted us is not really that useful. Not to be outdone, they make a betterer version of it, which seems like a good idea at the time. This is the story of the _HPX3 (well, most of ACPI really).

Previous _HPX versions had a fixed number of fields, and could only describe a very narrow set of useless register settings. _HPX3 extends that to an arbitrary number of entries, that can cover the entire PCI config space, and will be The Last _HPX Table Ever (TM). For the sadomasochistic, the new ACPI 6.3 spec is here [1].

For our purposes, we had to re-organize code a bit, and the first two patches in the series do this, albeit with minimal noise. The last patch is the actual HPX3 stuff. In case you wonder why the ACPI enums don't match the PCI device types... nobody really thought of it or caught it at the spec writing stage.

In _HPX3 there's also the notion of vendor-specific extensions, and VSEC/DVSEC. They are not implemented here because there's no hardware to test that code path.

[1] https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf

RESEND because the first time 'git send-email' experienced an ID:10T error originating between my keyboard and my chair. As a result of that error, it did not send the cover letter to public mailing lists.

Alexandru Gagniuc (3):
PCI / ACPI: Do not export pci_get_hp_params() PCI / ACPI: Remove the need for 'struct hotplug_params' PCI / ACPI: Implement Type 3 _HPX record

 drivers/pci/pci-acpi.c      | 172 +++++++++++++++++++++++++-----------
 drivers/pci/probe.c         | 130 ++++++++++++++++++++++++---
 include/linux/pci_hotplug.h |  74 +++++++++++++---
 3 files changed, 305 insertions(+), 71 deletions(-)

Signature · 1 line

2.19.2

Alexandru Gagniuc

author

2019-03-07 21:38 UTC #1 lore raw

PATCH RESEND 1/3 PCI / ACPI: Do not export pci_get_hp_params()

This is only used within drivers/pci, and there is no reason to make it available outside of the PCI core.

Signed-off-by Alexandru Gagniuc <[email protected]>

---
 drivers/pci/pci-acpi.c | 1 -
 1 file changed, 1 deletion(-)

drivers/pci/pci-acpi.c +0 -1

index e1949f7efd9c..b25e5fa9d1c9 100644
@@ -366,7 +366,6 @@ int pci_get_hp_params(struct pci_dev *dev, struct hotplug_params *hpp)
 	}
 	return -ENODEV;
 }
-EXPORT_SYMBOL_GPL(pci_get_hp_params);
 
 /**
  * pciehp_is_native - Check whether a hotplug port is handled by the OS

Signature · 1 line

2.19.2

Alexandru Gagniuc

author

2019-03-07 21:38 UTC #2 lore raw

PATCH RESEND 2/3 PCI / ACPI: Remove the need for 'struct hotplug_params'

In reply to Alexandru Gagniuc

We used to first parse all the _HPP and _HPX tables before using the information to program registers of PCIe devices. Up until HPX type 2, there was only one structure of each type, so we could cheat and store it on the stack.

With HPX type 3 we get an arbitrary number of entries, so the above model doesn't scale that well. Instead of parsing all tables at once, parse and program each entry separately. For _HPP and _HPX 0 thru 2, this is functionally equivalent. The change enables the upcoming _HPX3 to integrate more easily.

Signed-off-by Alexandru Gagniuc <[email protected]>

---
 drivers/pci/pci-acpi.c      | 108 +++++++++++++++++++-----------------
 drivers/pci/probe.c         |  16 ++----
 include/linux/pci_hotplug.h |  18 +++---
 3 files changed, 72 insertions(+), 70 deletions(-)

drivers/pci/pci-acpi.c +58 ~~-50~~

index b25e5fa9d1c9..95f4f86d2f34 100644
@@ -119,7 +119,7 @@ phys_addr_t acpi_pci_root_get_mcfg_addr(acpi_handle handle)
 }
 
 static acpi_status decode_type0_hpx_record(union acpi_object *record,
-					   struct hotplug_params *hpx)
+					   struct hpp_type0 *hpx0)
 {
 	int i;
 	union acpi_object *fields = record->package.elements;

@@ -132,12 +132,11 @@ static acpi_status decode_type0_hpx_record(union acpi_object *record,
 		for (i = 2; i < 6; i++)
 			if (fields[i].type != ACPI_TYPE_INTEGER)
 				return AE_ERROR;
-		hpx->t0 = &hpx->type0_data;
-		hpx->t0->revision        = revision;
-		hpx->t0->cache_line_size = fields[2].integer.value;
-		hpx->t0->latency_timer   = fields[3].integer.value;
-		hpx->t0->enable_serr     = fields[4].integer.value;
-		hpx->t0->enable_perr     = fields[5].integer.value;
+		hpx0->revision        = revision;
+		hpx0->cache_line_size = fields[2].integer.value;
+		hpx0->latency_timer   = fields[3].integer.value;
+		hpx0->enable_serr     = fields[4].integer.value;
+		hpx0->enable_perr     = fields[5].integer.value;
 		break;
 	default:
 		printk(KERN_WARNING

@@ -149,7 +148,7 @@ static acpi_status decode_type0_hpx_record(union acpi_object *record,
 }
 
 static acpi_status decode_type1_hpx_record(union acpi_object *record,
-					   struct hotplug_params *hpx)
+					   struct hpp_type1 *hpx1)
 {
 	int i;
 	union acpi_object *fields = record->package.elements;

@@ -162,11 +161,10 @@ static acpi_status decode_type1_hpx_record(union acpi_object *record,
 		for (i = 2; i < 5; i++)
 			if (fields[i].type != ACPI_TYPE_INTEGER)
 				return AE_ERROR;
-		hpx->t1 = &hpx->type1_data;
-		hpx->t1->revision      = revision;
-		hpx->t1->max_mem_read  = fields[2].integer.value;
-		hpx->t1->avg_max_split = fields[3].integer.value;
-		hpx->t1->tot_max_split = fields[4].integer.value;
+		hpx1->revision      = revision;
+		hpx1->max_mem_read  = fields[2].integer.value;
+		hpx1->avg_max_split = fields[3].integer.value;
+		hpx1->tot_max_split = fields[4].integer.value;
 		break;
 	default:
 		printk(KERN_WARNING

@@ -178,7 +176,7 @@ static acpi_status decode_type1_hpx_record(union acpi_object *record,
 }
 
 static acpi_status decode_type2_hpx_record(union acpi_object *record,
-					   struct hotplug_params *hpx)
+					   struct hpp_type2 *hpx2)
 {
 	int i;
 	union acpi_object *fields = record->package.elements;

@@ -191,24 +189,23 @@ static acpi_status decode_type2_hpx_record(union acpi_object *record,
 		for (i = 2; i < 18; i++)
 			if (fields[i].type != ACPI_TYPE_INTEGER)
 				return AE_ERROR;
-		hpx->t2 = &hpx->type2_data;
-		hpx->t2->revision      = revision;
-		hpx->t2->unc_err_mask_and      = fields[2].integer.value;
-		hpx->t2->unc_err_mask_or       = fields[3].integer.value;
-		hpx->t2->unc_err_sever_and     = fields[4].integer.value;
-		hpx->t2->unc_err_sever_or      = fields[5].integer.value;
-		hpx->t2->cor_err_mask_and      = fields[6].integer.value;
-		hpx->t2->cor_err_mask_or       = fields[7].integer.value;
-		hpx->t2->adv_err_cap_and       = fields[8].integer.value;
-		hpx->t2->adv_err_cap_or        = fields[9].integer.value;
-		hpx->t2->pci_exp_devctl_and    = fields[10].integer.value;
-		hpx->t2->pci_exp_devctl_or     = fields[11].integer.value;
-		hpx->t2->pci_exp_lnkctl_and    = fields[12].integer.value;
-		hpx->t2->pci_exp_lnkctl_or     = fields[13].integer.value;
-		hpx->t2->sec_unc_err_sever_and = fields[14].integer.value;
-		hpx->t2->sec_unc_err_sever_or  = fields[15].integer.value;
-		hpx->t2->sec_unc_err_mask_and  = fields[16].integer.value;
-		hpx->t2->sec_unc_err_mask_or   = fields[17].integer.value;
+		hpx2->revision      = revision;
+		hpx2->unc_err_mask_and      = fields[2].integer.value;
+		hpx2->unc_err_mask_or       = fields[3].integer.value;
+		hpx2->unc_err_sever_and     = fields[4].integer.value;
+		hpx2->unc_err_sever_or      = fields[5].integer.value;
+		hpx2->cor_err_mask_and      = fields[6].integer.value;
+		hpx2->cor_err_mask_or       = fields[7].integer.value;
+		hpx2->adv_err_cap_and       = fields[8].integer.value;
+		hpx2->adv_err_cap_or        = fields[9].integer.value;
+		hpx2->pci_exp_devctl_and    = fields[10].integer.value;
+		hpx2->pci_exp_devctl_or     = fields[11].integer.value;
+		hpx2->pci_exp_lnkctl_and    = fields[12].integer.value;
+		hpx2->pci_exp_lnkctl_or     = fields[13].integer.value;
+		hpx2->sec_unc_err_sever_and = fields[14].integer.value;
+		hpx2->sec_unc_err_sever_or  = fields[15].integer.value;
+		hpx2->sec_unc_err_mask_and  = fields[16].integer.value;
+		hpx2->sec_unc_err_mask_or   = fields[17].integer.value;
 		break;
 	default:
 		printk(KERN_WARNING

@@ -219,17 +216,18 @@ static acpi_status decode_type2_hpx_record(union acpi_object *record,
 	return AE_OK;
 }
 
-static acpi_status acpi_run_hpx(acpi_handle handle, struct hotplug_params *hpx)
+static acpi_status acpi_run_hpx(struct pci_dev *dev, acpi_handle handle,
+				const struct hotplug_program_ops *hp_ops)
 {
 	acpi_status status;
 	struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL};
 	union acpi_object *package, *record, *fields;
+	struct hpp_type0 hpx0;
+	struct hpp_type1 hpx1;
+	struct hpp_type2 hpx2;
 	u32 type;
 	int i;
 
-	/* Clear the return buffer with zeros */
-	memset(hpx, 0, sizeof(struct hotplug_params));
-
 	status = acpi_evaluate_object(handle, "_HPX", NULL, &buffer);
 	if (ACPI_FAILURE(status))
 		return status;

@@ -257,19 +255,25 @@ static acpi_status acpi_run_hpx(acpi_handle handle, struct hotplug_params *hpx)
 		type = fields[0].integer.value;
 		switch (type) {
 		case 0:
-			status = decode_type0_hpx_record(record, hpx);
+			memset(&hpx0, 0, sizeof(hpx0));
+			status = decode_type0_hpx_record(record, &hpx0);
 			if (ACPI_FAILURE(status))
 				goto exit;
+			hp_ops->program_type0(dev, &hpx0);
 			break;
 		case 1:
-			status = decode_type1_hpx_record(record, hpx);
+			memset(&hpx1, 0, sizeof(hpx1));
+			status = decode_type1_hpx_record(record, &hpx1);
 			if (ACPI_FAILURE(status))
 				goto exit;
+			hp_ops->program_type1(dev, &hpx1);
 			break;
 		case 2:
-			status = decode_type2_hpx_record(record, hpx);
+			memset(&hpx2, 0, sizeof(hpx2));
+			status = decode_type2_hpx_record(record, &hpx2);
 			if (ACPI_FAILURE(status))
 				goto exit;
+			hp_ops->program_type2(dev, &hpx2);
 			break;
 		default:
 			printk(KERN_ERR "%s: Type %d record not supported\n",

@@ -283,14 +287,16 @@ static acpi_status acpi_run_hpx(acpi_handle handle, struct hotplug_params *hpx)
 	return status;
 }
 
-static acpi_status acpi_run_hpp(acpi_handle handle, struct hotplug_params *hpp)
+static acpi_status acpi_run_hpp(struct pci_dev *dev, acpi_handle handle,
+				const struct hotplug_program_ops *hp_ops)
 {
 	acpi_status status;
 	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
 	union acpi_object *package, *fields;
+	struct hpp_type0 hpp0;
 	int i;
 
-	memset(hpp, 0, sizeof(struct hotplug_params));
+	memset(&hpp0, 0, sizeof(hpp0));
 
 	status = acpi_evaluate_object(handle, "_HPP", NULL, &buffer);
 	if (ACPI_FAILURE(status))

@@ -311,12 +317,13 @@ static acpi_status acpi_run_hpp(acpi_handle handle, struct hotplug_params *hpp)
 		}
 	}
 
-	hpp->t0 = &hpp->type0_data;
-	hpp->t0->revision        = 1;
-	hpp->t0->cache_line_size = fields[0].integer.value;
-	hpp->t0->latency_timer   = fields[1].integer.value;
-	hpp->t0->enable_serr     = fields[2].integer.value;
-	hpp->t0->enable_perr     = fields[3].integer.value;
+	hpp0.revision        = 1;
+	hpp0.cache_line_size = fields[0].integer.value;
+	hpp0.latency_timer   = fields[1].integer.value;
+	hpp0.enable_serr     = fields[2].integer.value;
+	hpp0.enable_perr     = fields[3].integer.value;
+
+	hp_ops->program_type0(dev, &hpp0);
 
 exit:
 	kfree(buffer.pointer);

@@ -328,7 +335,8 @@ static acpi_status acpi_run_hpp(acpi_handle handle, struct hotplug_params *hpp)
  * @dev - the pci_dev for which we want parameters
  * @hpp - allocated by the caller
  */
-int pci_get_hp_params(struct pci_dev *dev, struct hotplug_params *hpp)
+int pci_acpi_program_hp_params(struct pci_dev *dev,
+			       const struct hotplug_program_ops *hp_ops)
 {
 	acpi_status status;
 	acpi_handle handle, phandle;

@@ -351,10 +359,10 @@ int pci_get_hp_params(struct pci_dev *dev, struct hotplug_params *hpp)
 	 * this pci dev.
 	 */
 	while (handle) {
-		status = acpi_run_hpx(handle, hpp);
+		status = acpi_run_hpx(dev, handle, hp_ops);
 		if (ACPI_SUCCESS(status))
 			return 0;
-		status = acpi_run_hpp(handle, hpp);
+		status = acpi_run_hpp(dev, handle, hp_ops);
 		if (ACPI_SUCCESS(status))
 			return 0;
 		if (acpi_is_root_bridge(handle))

drivers/pci/probe.c +6 ~~-10~~

index 257b9f6f2ebb..527c209f0c94 100644
@@ -2131,8 +2131,11 @@ static void pci_configure_eetlp_prefix(struct pci_dev *dev)
 
 static void pci_configure_device(struct pci_dev *dev)
 {
-	struct hotplug_params hpp;
-	int ret;
+	static const struct hotplug_program_ops hp_ops = {
+		.program_type0 = program_hpp_type0,
+		.program_type1 = program_hpp_type1,
+		.program_type2 = program_hpp_type2,
+	};
 
 	pci_configure_mps(dev);
 	pci_configure_extended_tags(dev, NULL);

@@ -2140,14 +2143,7 @@ static void pci_configure_device(struct pci_dev *dev)
 	pci_configure_ltr(dev);
 	pci_configure_eetlp_prefix(dev);
 
-	memset(&hpp, 0, sizeof(hpp));
-	ret = pci_get_hp_params(dev, &hpp);
-	if (ret)
-		return;
-
-	program_hpp_type2(dev, hpp.t2);
-	program_hpp_type1(dev, hpp.t1);
-	program_hpp_type0(dev, hpp.t0);
+	pci_acpi_program_hp_params(dev, &hp_ops);
 }
 
 static void pci_release_capabilities(struct pci_dev *dev)

include/linux/pci_hotplug.h +8 ~~-10~~

index 7acc9f91e72b..c85378edf235 100644
@@ -124,26 +124,24 @@ struct hpp_type2 {
 	u32 sec_unc_err_mask_or;
 };
 
-struct hotplug_params {
-	struct hpp_type0 *t0;		/* Type0: NULL if not available */
-	struct hpp_type1 *t1;		/* Type1: NULL if not available */
-	struct hpp_type2 *t2;		/* Type2: NULL if not available */
-	struct hpp_type0 type0_data;
-	struct hpp_type1 type1_data;
-	struct hpp_type2 type2_data;
+struct hotplug_program_ops {
+	void (*program_type0)(struct pci_dev *dev, struct hpp_type0 *hpp);
+	void (*program_type1)(struct pci_dev *dev, struct hpp_type1 *hpp);
+	void (*program_type2)(struct pci_dev *dev, struct hpp_type2 *hpp);
 };
 
 #ifdef CONFIG_ACPI
 #include <linux/acpi.h>
-int pci_get_hp_params(struct pci_dev *dev, struct hotplug_params *hpp);
+int pci_acpi_program_hp_params(struct pci_dev *dev,
+			       const struct hotplug_program_ops *hp_ops);
 bool pciehp_is_native(struct pci_dev *bridge);
 int acpi_get_hp_hw_control_from_firmware(struct pci_dev *bridge);
 bool shpchp_is_native(struct pci_dev *bridge);
 int acpi_pci_check_ejectable(struct pci_bus *pbus, acpi_handle handle);
 int acpi_pci_detect_ejectable(acpi_handle handle);
 #else
-static inline int pci_get_hp_params(struct pci_dev *dev,
-				    struct hotplug_params *hpp)
+int pci_acpi_program_hp_params(struct pci_dev *dev,
+			       const struct hotplug_program_ops *hp_ops);
 {
 	return -ENODEV;
 }

Signature · 1 line

2.19.2

Alexandru Gagniuc

author

2019-03-07 21:38 UTC #3 lore raw

PATCH RESEND 3/3 PCI / ACPI: Implement Type 3 _HPX record

In reply to Alexandru Gagniuc

_HPX Type 3 is intended to be more generic and allow configuration of settings not possible with Type 2 tables. For example, FW could ensure that the completion timeout value is set accordingly throughout the PCI tree.

Implement support for _HPX3 tables.

Signed-off-by Alexandru Gagniuc <[email protected]>

---
 drivers/pci/pci-acpi.c      |  63 ++++++++++++++++++++
 drivers/pci/probe.c         | 114 ++++++++++++++++++++++++++++++++++++
 include/linux/pci_hotplug.h |  56 ++++++++++++++++++
 3 files changed, 233 insertions(+)

drivers/pci/pci-acpi.c +63 -0

index 95f4f86d2f34..03e02dd6c1d9 100644
@@ -216,6 +216,64 @@ static acpi_status decode_type2_hpx_record(union acpi_object *record,
 	return AE_OK;
 }
 
+static void parse_hpx3_register(struct hpx_type3 *hpx3_reg,
+				union acpi_object *reg_fields)
+{
+	hpx3_reg->device_type            = reg_fields[0].integer.value;
+	hpx3_reg->function_type          = reg_fields[1].integer.value;
+	hpx3_reg->config_space_location  = reg_fields[2].integer.value;
+	hpx3_reg->pci_exp_cap_id         = reg_fields[3].integer.value;
+	hpx3_reg->pci_exp_cap_ver        = reg_fields[4].integer.value;
+	hpx3_reg->pci_exp_vendor_id      = reg_fields[5].integer.value;
+	hpx3_reg->dvsec_id               = reg_fields[6].integer.value;
+	hpx3_reg->dvsec_rev              = reg_fields[7].integer.value;
+	hpx3_reg->match_offset           = reg_fields[8].integer.value;
+	hpx3_reg->match_mask_and         = reg_fields[9].integer.value;
+	hpx3_reg->match_value            = reg_fields[10].integer.value;
+	hpx3_reg->reg_offset             = reg_fields[11].integer.value;
+	hpx3_reg->reg_mask_and           = reg_fields[12].integer.value;
+	hpx3_reg->reg_mask_or            = reg_fields[13].integer.value;
+}
+
+static acpi_status program_type3_hpx_record(struct pci_dev *dev,
+					   union acpi_object *record,
+					   const struct hotplug_program_ops *hp_ops)
+{
+	union acpi_object *fields = record->package.elements;
+	u32 desc_count, expected_length, revision;
+	union acpi_object *reg_fields;
+	struct hpx_type3 hpx3;
+	int i;
+
+	revision = fields[1].integer.value;
+	switch (revision) {
+	case 1:
+		desc_count = fields[2].integer.value;
+		expected_length = 3 + desc_count * 14;
+
+		if (record->package.count != expected_length)
+			return AE_ERROR;
+
+		for (i = 2; i < expected_length; i++)
+			if (fields[i].type != ACPI_TYPE_INTEGER)
+				return AE_ERROR;
+
+		for (i = 0; i < desc_count; i++) {
+			reg_fields = fields + 3 + i * 14;
+			parse_hpx3_register(&hpx3, reg_fields);
+			hp_ops->program_type3(dev, &hpx3);
+		}
+
+		break;
+	default:
+		printk(KERN_WARNING
+			"%s: Type 3 Revision %d record not supported\n",
+			__func__, revision);
+		return AE_ERROR;
+	}
+	return AE_OK;
+}
+
 static acpi_status acpi_run_hpx(struct pci_dev *dev, acpi_handle handle,
 				const struct hotplug_program_ops *hp_ops)
 {

@@ -275,6 +333,11 @@ static acpi_status acpi_run_hpx(struct pci_dev *dev, acpi_handle handle,
 				goto exit;
 			hp_ops->program_type2(dev, &hpx2);
 			break;
+		case 3:
+			status = program_type3_hpx_record(dev, record, hp_ops);
+			if (ACPI_FAILURE(status))
+				goto exit;
+			break;
 		default:
 			printk(KERN_ERR "%s: Type %d record not supported\n",
 			       __func__, type);

drivers/pci/probe.c +114 -0

126 lines not on this page · raw message

include/linux/pci_hotplug.h +56 -0

66 lines not on this page · raw message

Signature · 1 line

2.19.2

Alexandru Gagniuc

author

2019-03-15 19:29 UTC #4 lore raw

PATCH 4/4 PCI/ACPI: Advertise HPX type 3 support via _OSC

In reply to Alexandru Gagniuc

_OSC now has a way to inform firmware that OS has the capability to interpret HPX type 3 tables. Since ACPI 6.3 deprecated _OSC specifics, these are now part of the PCI Firmware Specification.

The following is the document describing the changes:
ECN: _HPX and PCIe Completion Timeout related _OSC Enhancements
Date: September 12, 2018
Affected Document: PCI Firmware Specification, Rev. 3.2

Signed-off-by Alexandru Gagniuc <[email protected]>

---

This patch is designed to go on top of the following series "PCI: ACPI: Implement support for _HPX Type 3 tables"

	https://lkml.org/lkml/2019/3/7/790

Although the patch can technically be applied independently, it doesn't make sense without the rest of the series.

The HPX3 capability bit, to my understanding, is designed to be informational. i.e. Setting or not setting it would not affect whether firmware would export HPX3 tables. It may have implications in FFS systems, where FW might need to make sure things like the completion timeout value is set correctly throughout the tree.

My understanding is that firmware will prefer to set these with HPX3, in lieu of firing up SMM on hotplug and other events. Firmware can use the HPX3 support bit to know how to proceed.

 drivers/acpi/pci_root.c | 2 ++
 include/linux/acpi.h    | 3 ++-
 2 files changed, 4 insertions(+), 1 deletion(-)

drivers/acpi/pci_root.c +2 -0

index 707aafc7c2aa..0bee23893297 100644
@@ -145,6 +145,7 @@ static struct pci_osc_bit_struct pci_osc_support_bit[] = {
 	{ OSC_PCI_CLOCK_PM_SUPPORT, "ClockPM" },
 	{ OSC_PCI_SEGMENT_GROUPS_SUPPORT, "Segments" },
 	{ OSC_PCI_MSI_SUPPORT, "MSI" },
+	{ OSC_PCI_HPX_TYPE_3_SUPPORT, "HPX-type3" },
 };
 
 static struct pci_osc_bit_struct pci_osc_control_bit[] = {

@@ -446,6 +447,7 @@ static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm,
 	 * PCI domains, so we indicate this in _OSC support capabilities.
 	 */
 	support = OSC_PCI_SEGMENT_GROUPS_SUPPORT;
+	support |= OSC_PCI_HPX_TYPE_3_SUPPORT;
 	if (pci_ext_cfg_avail())
 		support |= OSC_PCI_EXT_CONFIG_SUPPORT;
 	if (pcie_aspm_support_enabled())

include/linux/acpi.h +2 -1

index 87715f20b69a..028c138b9082 100644
@@ -508,7 +508,8 @@ extern bool osc_pc_lpi_support_confirmed;
 #define OSC_PCI_CLOCK_PM_SUPPORT		0x00000004
 #define OSC_PCI_SEGMENT_GROUPS_SUPPORT		0x00000008
 #define OSC_PCI_MSI_SUPPORT			0x00000010
-#define OSC_PCI_SUPPORT_MASKS			0x0000001f
+#define OSC_PCI_HPX_TYPE_3_SUPPORT		0x00000100
+#define OSC_PCI_SUPPORT_MASKS			0x0000011f
 
 /* PCI Host Bridge _OSC: Capabilities DWORD 3: Control Field */
 #define OSC_PCI_EXPRESS_NATIVE_HP_CONTROL	0x00000001

Signature · 1 line

2.19.2

Bjorn Helgaas

Applied

2019-04-23 21:38 UTC #5 lore raw

Re: [PATCH v1 4/4] PCI/ACPI: Advertise HPX type 3 support via _OSC

Quoted 10 lines · Alexandru Gagniuc

_OSC now has a way to inform firmware that OS has the capability to interpret HPX type 3 tables. Since ACPI 6.3 deprecated _OSC specifics, these are now part of the PCI Firmware Specification.
The following is the document describing the changes:
ECN: _HPX and PCIe Completion Timeout related _OSC Enhancements
Date: September 12, 2018
Affected Document: PCI Firmware Specification, Rev. 3.2
Signed-off-by Alexandru Gagniuc <[email protected]>

Applied to pci/hotplug for v5.2, thanks!

Quoted 60 lines

---
This patch is designed to go on top of the following series "PCI: ACPI: Implement support for _HPX Type 3 tables"
	https://lkml.org/lkml/2019/3/7/790
Although the patch can technically be applied independently, it doesn't make sense without the rest of the series.
The HPX3 capability bit, to my understanding, is designed to be informational. i.e. Setting or not setting it would not affect whether firmware would export HPX3 tables. It may have implications in FFS systems, where FW might need to make sure things like the completion timeout value is set correctly throughout the tree.
My understanding is that firmware will prefer to set these with HPX3, in lieu of firing up SMM on hotplug and other events. Firmware can use the HPX3 support bit to know how to proceed.
 drivers/acpi/pci_root.c | 2 ++
 include/linux/acpi.h    | 3 ++-
 2 files changed, 4 insertions(+), 1 deletion(-)
drivers/acpi/pci_root.c +2 -0
index 707aafc7c2aa..0bee23893297 100644
@@ -145,6 +145,7 @@ static struct pci_osc_bit_struct pci_osc_support_bit[] = {
 	{ OSC_PCI_CLOCK_PM_SUPPORT, "ClockPM" },
 	{ OSC_PCI_SEGMENT_GROUPS_SUPPORT, "Segments" },
 	{ OSC_PCI_MSI_SUPPORT, "MSI" },
+	{ OSC_PCI_HPX_TYPE_3_SUPPORT, "HPX-type3" },
 };
 
 static struct pci_osc_bit_struct pci_osc_control_bit[] = {
@@ -446,6 +447,7 @@ static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm,
 	 * PCI domains, so we indicate this in _OSC support capabilities.
 	 */
 	support = OSC_PCI_SEGMENT_GROUPS_SUPPORT;
+	support |= OSC_PCI_HPX_TYPE_3_SUPPORT;
 	if (pci_ext_cfg_avail())
 		support |= OSC_PCI_EXT_CONFIG_SUPPORT;
 	if (pcie_aspm_support_enabled())
include/linux/acpi.h +2 -1
index 87715f20b69a..028c138b9082 100644
@@ -508,7 +508,8 @@ extern bool osc_pc_lpi_support_confirmed;
 #define OSC_PCI_CLOCK_PM_SUPPORT		0x00000004
 #define OSC_PCI_SEGMENT_GROUPS_SUPPORT		0x00000008
 #define OSC_PCI_MSI_SUPPORT			0x00000010
-#define OSC_PCI_SUPPORT_MASKS			0x0000001f
+#define OSC_PCI_HPX_TYPE_3_SUPPORT		0x00000100
+#define OSC_PCI_SUPPORT_MASKS			0x0000011f
 
 /* PCI Host Bridge _OSC: Capabilities DWORD 3: Control Field */
 #define OSC_PCI_EXPRESS_NATIVE_HP_CONTROL	0x00000001
Signature · 1 line
2.19.2