Linux-ACPI Archive on lore.kernel.org
 help / color / Atom feed
* [RFC PATCH] ACPI: Add support for CXL_OSC
@ 2020-07-08 23:57 Sean V Kelley
  2020-07-10 21:53 ` Bjorn Helgaas
  0 siblings, 1 reply; 3+ messages in thread
From: Sean V Kelley @ 2020-07-08 23:57 UTC (permalink / raw)
  To: rjw, bhelgaas, lenb, robert.moore, erik.kaneda
  Cc: linux-pci, linux-acpi, Sean V Kelley

Compute eXpress Link host bridges enumerate to the OS with
ACPI-described support and control capabilities. An _OSC interface
applies to host bridge ACPI devices that originate CXL hierarchies.

These ACPI devices have a _HID of (_CID including) EISAID(“ACPI0016”).
For CXL 2.0 and later hierarchies, CXL _OSC is required. CXL _OSC is
optional for CXL 1.1 hierarchies. A CXL host bridge also originates a
PCIe hierarchy and will have a _CID of EISAID(“PNP0A08”). As such, a CXL
Host Bridge device may expose either CXL _OSC or PCIe _OSC.

CXL end point devices connected behind a CXL bridge are standard PCI or
CXL devices. It is the same device with two labels - ACPI0016 or PNP0A08
label. For example, with a given bridge device ‘PC01’ definition, the OS
will be able to find PCI devices connected behind this bridge.

                      | CPU Root Bus 0x14 |
                      | Downstream Port   |
                       -------------------
                               |
                               | Flex Bus
                               |
                      |                   |  ACPI Device PC01
                      |  Root Bus 0x15    |
                       -------------------
                          |          |
                          |          |
                      | RCiEP |   | RCiEP |
                      |       |   | or EP |
                      | D0 F0 |   | D0 F1 |
                       -------     -------

As a result, it instantiates another PNP0A08 or ACPI0016 device. PN0A08
is therefore legacy support and the ACPI0016 is for a CXL aware OS. A CXL
aware OS would look for CXL first. If the OS evaluates CXL _OSC, it shall
not evaluate PCIe _OSC.

This patch implements support for CXL _OSC for CXL 1.1 hiearchies.  This
is intended as an RFC to open discussion on the feature and its
implementation.

Some caveats and questions to consider:

1) pci_root.c goes through many levels of calls that may need some
changes. These patches attempt to live-with the current state. Should
the first step perhaps be more refactoring to make room so to speak
for the addition of CXL?

2) I avoided rename where possible, and so where you see say OSC_SUPPORT
that implies PCI.

3) I attempted to avoid increasing the complexity of an existing function
by creating a new CXL specific one.  In some cases CXL and PCI handling
coexist in the same function.

As with my past CXL Enumeration RFC there are no fw images or device
drivers yet and so this is not ready for consideration for merge.
However, I would like to get discussion started on the ACPI changes
needed especially if it points to some prior refactoring needs.

DocLink: https://www.computeexpresslink.org/
DocLink: https://uefi.org/

Signed-off-by: Sean V Kelley <sean.v.kelley@linux.intel.com>
---
 drivers/acpi/pci_root.c | 374 ++++++++++++++++++++++++++++------------
 include/acpi/acpi_bus.h |   6 +-
 include/linux/acpi.h    |  18 ++
 3 files changed, 288 insertions(+), 110 deletions(-)

diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index f90e841c59f5..e71fb682072b 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -45,6 +45,7 @@ static int acpi_pci_root_scan_dependent(struct acpi_device *adev)
 
 static const struct acpi_device_id root_device_ids[] = {
 	{"PNP0A03", 0},
+	{"ACPI0016", 0},
 	{"", 0},
 };
 
@@ -135,6 +136,13 @@ static struct pci_osc_bit_struct pci_osc_support_bit[] = {
 	{ OSC_PCI_HPX_TYPE_3_SUPPORT, "HPX-Type3" },
 };
 
+static struct pci_osc_bit_struct cxl_osc_support_bit[] = {
+	{ OSC_CXL_PORT_REG_ACCESS_SUPPORT, "CXLPortRegAccess" },
+	{ OSC_CXL_PORT_DEV_REG_ACCESS_SUPPORT, "CXLPortDevRegAccess" },
+	{ OSC_CXL_PER_SUPPORT, "CXLProtocolErrorReporting" },
+	{ OSC_CXL_NATIVE_HP_SUPPORT, "CXLNativeHotPlug" },
+};
+
 static struct pci_osc_bit_struct pci_osc_control_bit[] = {
 	{ OSC_PCI_EXPRESS_NATIVE_HP_CONTROL, "PCIeHotplug" },
 	{ OSC_PCI_SHPC_NATIVE_HP_CONTROL, "SHPCHotplug" },
@@ -145,6 +153,10 @@ static struct pci_osc_bit_struct pci_osc_control_bit[] = {
 	{ OSC_PCI_EXPRESS_DPC_CONTROL, "DPC" },
 };
 
+static struct pci_osc_bit_struct cxl_osc_control_bit[] = {
+	{ OSC_CXL_MEM_PER_CONTROL, "CXLMemProtocolErrorReport" },
+};
+
 static void decode_osc_bits(struct acpi_pci_root *root, char *msg, u32 word,
 			    struct pci_osc_bit_struct *table, int size)
 {
@@ -161,27 +173,40 @@ static void decode_osc_bits(struct acpi_pci_root *root, char *msg, u32 word,
 	dev_info(&root->device->dev, "_OSC: %s [%s]\n", msg, buf);
 }
 
-static void decode_osc_support(struct acpi_pci_root *root, char *msg, u32 word)
+static void decode_cxl_osc_support(struct acpi_pci_root *root, char *msg, u32 word)
+{
+	decode_osc_bits(root, msg, word, cxl_osc_support_bit,
+			ARRAY_SIZE(cxl_osc_support_bit));
+}
+
+static void decode_cxl_osc_control(struct acpi_pci_root *root, char *msg, u32 word)
+{
+	decode_osc_bits(root, msg, word, cxl_osc_control_bit,
+			ARRAY_SIZE(cxl_osc_control_bit));
+}
+
+static void decode_pci_osc_support(struct acpi_pci_root *root, char *msg, u32 word)
 {
 	decode_osc_bits(root, msg, word, pci_osc_support_bit,
 			ARRAY_SIZE(pci_osc_support_bit));
 }
 
-static void decode_osc_control(struct acpi_pci_root *root, char *msg, u32 word)
+static void decode_pci_osc_control(struct acpi_pci_root *root, char *msg, u32 word)
 {
 	decode_osc_bits(root, msg, word, pci_osc_control_bit,
 			ARRAY_SIZE(pci_osc_control_bit));
 }
 
 static u8 pci_osc_uuid_str[] = "33DB4D5B-1FF7-401C-9657-7441C03DD766";
+static u8 cxl_osc_uuid_str[] = "68F2D50B-C469-4d8A-BD3D-941A103FD3FC";
 
-static acpi_status acpi_pci_run_osc(acpi_handle handle,
-				    const u32 *capbuf, u32 *retval)
+static acpi_status acpi_cap_run_osc(acpi_handle handle,
+				    const u32 *capbuf, u8 *uuid_str, u32 *retval)
 {
 	struct acpi_osc_context context = {
-		.uuid_str = pci_osc_uuid_str,
+		.uuid_str = uuid_str,
 		.rev = 1,
-		.cap.length = 12,
+		.cap.length = 20,
 		.cap.pointer = (void *)capbuf,
 	};
 	acpi_status status;
@@ -194,12 +219,41 @@ static acpi_status acpi_pci_run_osc(acpi_handle handle,
 	return status;
 }
 
+static acpi_status acpi_cxl_query_osc(struct acpi_pci_root *root,
+				      u32 support,
+				      u32 *control)
+{
+	acpi_status status;
+	u32 result, capbuf[5];
+
+	support &= OSC_CXL_SUPPORT_MASKS;
+	support |= root->cxl_osc_support_set;
+
+	capbuf[OSC_QUERY_DWORD] = OSC_QUERY_ENABLE;
+	capbuf[CXL_OSC_SUPPORT_DWORD] = support;
+	if (control) {
+		*control &= OSC_CXL_CONTROL_MASKS;
+		capbuf[CXL_OSC_CONTROL_DWORD] = *control | root->cxl_osc_control_set;
+	} else {
+		/* Run _OSC query only with existing controls. */
+		capbuf[CXL_OSC_CONTROL_DWORD] = root->cxl_osc_control_set;
+	}
+
+	status = acpi_cap_run_osc(root->device->handle, capbuf, cxl_osc_uuid_str, &result);
+	if (ACPI_SUCCESS(status)) {
+		root->cxl_osc_support_set = support;
+		if (control)
+			*control = result;
+	}
+	return status;
+}
+
 static acpi_status acpi_pci_query_osc(struct acpi_pci_root *root,
-					u32 support,
-					u32 *control)
+				      u32 support,
+				      u32 *control)
 {
 	acpi_status status;
-	u32 result, capbuf[3];
+	u32 result, capbuf[5];
 
 	support &= OSC_PCI_SUPPORT_MASKS;
 	support |= root->osc_support_set;
@@ -214,7 +268,7 @@ static acpi_status acpi_pci_query_osc(struct acpi_pci_root *root,
 		capbuf[OSC_CONTROL_DWORD] = root->osc_control_set;
 	}
 
-	status = acpi_pci_run_osc(root->device->handle, capbuf, &result);
+	status = acpi_cap_run_osc(root->device->handle, capbuf, pci_osc_uuid_str, &result);
 	if (ACPI_SUCCESS(status)) {
 		root->osc_support_set = support;
 		if (control)
@@ -223,6 +277,16 @@ static acpi_status acpi_pci_query_osc(struct acpi_pci_root *root,
 	return status;
 }
 
+static acpi_status acpi_cxl_osc_support(struct acpi_pci_root *root, u32 flags)
+{
+	acpi_status status;
+
+	mutex_lock(&osc_lock);
+	status = acpi_cxl_query_osc(root, flags, NULL);
+	mutex_unlock(&osc_lock);
+	return status;
+}
+
 static acpi_status acpi_pci_osc_support(struct acpi_pci_root *root, u32 flags)
 {
 	acpi_status status;
@@ -359,7 +423,7 @@ acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask, u32 req)
 {
 	struct acpi_pci_root *root;
 	acpi_status status = AE_OK;
-	u32 ctrl, capbuf[3];
+	u32 ctrl, capbuf[5];
 
 	if (!mask)
 		return AE_BAD_PARAMETER;
@@ -386,14 +450,14 @@ acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask, u32 req)
 			goto out;
 		if (ctrl == *mask)
 			break;
-		decode_osc_control(root, "platform does not support",
-				   ctrl & ~(*mask));
+		decode_pci_osc_control(root, "platform does not support",
+				       ctrl & ~(*mask));
 		ctrl = *mask;
 	}
 
 	if ((ctrl & req) != req) {
-		decode_osc_control(root, "not requesting control; platform does not support",
-				   req & ~(ctrl));
+		decode_pci_osc_control(root, "not requesting control; platform does not support",
+				       req & ~(ctrl));
 		status = AE_SUPPORT;
 		goto out;
 	}
@@ -401,7 +465,7 @@ acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask, u32 req)
 	capbuf[OSC_QUERY_DWORD] = 0;
 	capbuf[OSC_SUPPORT_DWORD] = root->osc_support_set;
 	capbuf[OSC_CONTROL_DWORD] = ctrl;
-	status = acpi_pci_run_osc(handle, capbuf, mask);
+	status = acpi_cap_run_osc(handle, capbuf, pci_osc_uuid_str, mask);
 	if (ACPI_SUCCESS(status))
 		root->osc_control_set = *mask;
 out:
@@ -410,119 +474,212 @@ acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask, u32 req)
 }
 EXPORT_SYMBOL(acpi_pci_osc_control_set);
 
+/**
+ * acpi_cxl_osc_control_set - Request control of CXL root _OSC features.
+ * @handle: ACPI handle of a PCI root bridge (or PCIe Root Complex).
+ * @mask: Mask of _OSC bits to request control of, place to store control mask.
+ * @req: Mask of _OSC bits the control of is essential to the caller.
+ **/
+acpi_status acpi_cxl_osc_control_set(acpi_handle handle, u32 *mask, u32 req)
+{
+	struct acpi_pci_root *root;
+	acpi_status status = AE_OK;
+	u32 ctrl, capbuf[5];
+
+	if (!mask)
+		return AE_BAD_PARAMETER;
+
+	ctrl = *mask & OSC_CXL_CONTROL_MASKS;
+	if ((ctrl & req) != req)
+		return AE_TYPE;
+
+	root = acpi_pci_find_root(handle);
+	if (!root)
+		return AE_NOT_EXIST;
+
+	mutex_lock(&osc_lock);
+
+	*mask = ctrl | root->cxl_osc_control_set;
+	/* No need to evaluate _OSC if the control was already granted. */
+	if ((root->cxl_osc_control_set & ctrl) == ctrl)
+		goto out;
+
+	/* Need to check the available controls bits before requesting them. */
+	while (*mask) {
+		status = acpi_cxl_query_osc(root, root->cxl_osc_support_set, mask);
+		if (ACPI_FAILURE(status))
+			goto out;
+		if (ctrl == *mask)
+			break;
+		decode_cxl_osc_control(root, "platform does not support",
+				       ctrl & ~(*mask));
+		ctrl = *mask;
+	}
+
+	if ((ctrl & req) != req) {
+		decode_cxl_osc_control(root, "not requesting control; platform does not support",
+				       req & ~(ctrl));
+		status = AE_SUPPORT;
+		goto out;
+	}
+
+	capbuf[OSC_QUERY_DWORD] = 0;
+	capbuf[CXL_OSC_SUPPORT_DWORD] = root->cxl_osc_support_set;
+	capbuf[CXL_OSC_CONTROL_DWORD] = ctrl;
+	status = acpi_cap_run_osc(handle, capbuf, cxl_osc_uuid_str, mask);
+	if (ACPI_SUCCESS(status))
+		root->cxl_osc_control_set = *mask;
+out:
+	mutex_unlock(&osc_lock);
+	return status;
+}
+EXPORT_SYMBOL(acpi_cxl_osc_control_set);
+
 static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm,
-				 bool is_pcie)
+				 bool is_pcie, bool is_cxl)
 {
-	u32 support, control, requested;
+	u32 cxl_support, cxl_control, pci_support, pci_control, requested;
 	acpi_status status;
 	struct acpi_device *device = root->device;
 	acpi_handle handle = device->handle;
 
-	/*
-	 * Apple always return failure on _OSC calls when _OSI("Darwin") has
-	 * been called successfully. We know the feature set supported by the
-	 * platform, so avoid calling _OSC at all
-	 */
-	if (x86_apple_machine) {
-		root->osc_control_set = ~OSC_PCI_EXPRESS_PME_CONTROL;
-		decode_osc_control(root, "OS assumes control of",
-				   root->osc_control_set);
-		return;
-	}
 
-	/*
-	 * All supported architectures that use ACPI have support for
-	 * 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())
-		support |= OSC_PCI_ASPM_SUPPORT | OSC_PCI_CLOCK_PM_SUPPORT;
-	if (pci_msi_enabled())
-		support |= OSC_PCI_MSI_SUPPORT;
-	if (IS_ENABLED(CONFIG_PCIE_EDR))
-		support |= OSC_PCI_EDR_SUPPORT;
-
-	decode_osc_support(root, "OS supports", support);
-	status = acpi_pci_osc_support(root, support);
-	if (ACPI_FAILURE(status)) {
-		*no_aspm = 1;
+	if (is_cxl) {
+		cxl_support = 0; /* No presumed default CXL_OSC support capabilities */
+		cxl_control = 0; /* No presumed default CXL_OSC control capabilities */
 
-		/* _OSC is optional for PCI host bridges */
-		if ((status == AE_NOT_FOUND) && !is_pcie)
+		decode_cxl_osc_support(root, "OS supports", cxl_support);
+		status = acpi_cxl_osc_support(root, cxl_support);
+		if (ACPI_FAILURE(status)) {
+			dev_info(&device->dev, "CXL_OSC failed (%s)\n",
+				 acpi_format_exception(status));
 			return;
+		}
 
-		dev_info(&device->dev, "_OSC failed (%s)%s\n",
-			 acpi_format_exception(status),
-			 pcie_aspm_support_enabled() ? "; disabling ASPM" : "");
-		return;
-	}
+		cxl_control = OSC_CXL_MEM_PER_CONTROL;
+		requested = cxl_control;
+		status = acpi_cxl_osc_control_set(handle, &cxl_control,
+						  OSC_CXL_MEM_PER_CONTROL);
+		if (ACPI_SUCCESS(status)) {
+			decode_cxl_osc_control(root, "OS now controls",
+					       cxl_control);
+		} else {
+			decode_cxl_osc_control(root, "OS requested", requested);
+			decode_cxl_osc_control(root, "platform willing to grant",
+					       cxl_control);
+			dev_info(&device->dev, "_OSC failed (%s)\n",
+				 acpi_format_exception(status));
+		}
 
-	if (pcie_ports_disabled) {
-		dev_info(&device->dev, "PCIe port services disabled; not requesting _OSC control\n");
-		return;
-	}
+	} else { /* PCI/PCIe */
 
-	if ((support & ACPI_PCIE_REQ_SUPPORT) != ACPI_PCIE_REQ_SUPPORT) {
-		decode_osc_support(root, "not requesting OS control; OS requires",
-				   ACPI_PCIE_REQ_SUPPORT);
-		return;
-	}
+		/*
+		 * Apple always return failure on _OSC calls when _OSI("Darwin") has
+		 * been called successfully. We know the feature set supported by the
+		 * platform, so avoid calling _OSC at all
+		 */
+		if (x86_apple_machine) {
+			root->osc_control_set = ~OSC_PCI_EXPRESS_PME_CONTROL;
+			decode_pci_osc_control(root, "OS assumes control of",
+					       root->osc_control_set);
+			return;
+		}
+
+		/*
+		 * All supported architectures that use ACPI have support for
+		 * PCI domains, so we indicate this in _OSC support capabilities.
+		 */
+		pci_support = OSC_PCI_SEGMENT_GROUPS_SUPPORT;
+		pci_support |= OSC_PCI_HPX_TYPE_3_SUPPORT;
+		if (pci_ext_cfg_avail())
+			pci_support |= OSC_PCI_EXT_CONFIG_SUPPORT;
+		if (pcie_aspm_support_enabled())
+			pci_support |= OSC_PCI_ASPM_SUPPORT | OSC_PCI_CLOCK_PM_SUPPORT;
+		if (pci_msi_enabled())
+			pci_support |= OSC_PCI_MSI_SUPPORT;
+		if (IS_ENABLED(CONFIG_PCIE_EDR))
+			pci_support |= OSC_PCI_EDR_SUPPORT;
+
+		decode_pci_osc_support(root, "OS supports", pci_support);
+		status = acpi_pci_osc_support(root, pci_support);
+		if (ACPI_FAILURE(status)) {
+			*no_aspm = 1;
+
+			/* _OSC is optional for PCI host bridges */
+			if (status == AE_NOT_FOUND && !is_pcie)
+				return;
+
+			dev_info(&device->dev, "_OSC failed (%s)%s\n",
+				 acpi_format_exception(status),
+				 pcie_aspm_support_enabled() ? "; disabling ASPM" : "");
+			return;
+		}
+
+		if (pcie_ports_disabled) {
+			dev_info(&device->dev, "PCIe port services disabled; not requesting _OSC control\n");
+			return;
+		}
 
-	control = OSC_PCI_EXPRESS_CAPABILITY_CONTROL
-		| OSC_PCI_EXPRESS_PME_CONTROL;
+		if ((pci_support & ACPI_PCIE_REQ_SUPPORT) != ACPI_PCIE_REQ_SUPPORT) {
+			decode_pci_osc_support(root, "not requesting OS control; OS requires",
+					       ACPI_PCIE_REQ_SUPPORT);
+			return;
+		}
 
-	if (IS_ENABLED(CONFIG_PCIEASPM))
-		control |= OSC_PCI_EXPRESS_LTR_CONTROL;
+		pci_control = OSC_PCI_EXPRESS_CAPABILITY_CONTROL
+			| OSC_PCI_EXPRESS_PME_CONTROL;
 
-	if (IS_ENABLED(CONFIG_HOTPLUG_PCI_PCIE))
-		control |= OSC_PCI_EXPRESS_NATIVE_HP_CONTROL;
+		if (IS_ENABLED(CONFIG_PCIEASPM))
+			pci_control |= OSC_PCI_EXPRESS_LTR_CONTROL;
 
-	if (IS_ENABLED(CONFIG_HOTPLUG_PCI_SHPC))
-		control |= OSC_PCI_SHPC_NATIVE_HP_CONTROL;
+		if (IS_ENABLED(CONFIG_HOTPLUG_PCI_PCIE))
+			pci_control |= OSC_PCI_EXPRESS_NATIVE_HP_CONTROL;
 
-	if (pci_aer_available())
-		control |= OSC_PCI_EXPRESS_AER_CONTROL;
+		if (IS_ENABLED(CONFIG_HOTPLUG_PCI_SHPC))
+			pci_control |= OSC_PCI_SHPC_NATIVE_HP_CONTROL;
 
-	/*
-	 * Per the Downstream Port Containment Related Enhancements ECN to
-	 * the PCI Firmware Spec, r3.2, sec 4.5.1, table 4-5,
-	 * OSC_PCI_EXPRESS_DPC_CONTROL indicates the OS supports both DPC
-	 * and EDR.
-	 */
-	if (IS_ENABLED(CONFIG_PCIE_DPC) && IS_ENABLED(CONFIG_PCIE_EDR))
-		control |= OSC_PCI_EXPRESS_DPC_CONTROL;
+		if (pci_aer_available())
+			pci_control |= OSC_PCI_EXPRESS_AER_CONTROL;
+
+		/*
+		 * Per the Downstream Port Containment Related Enhancements ECN to
+		 * the PCI Firmware Spec, r3.2, sec 4.5.1, table 4-5,
+		 * OSC_PCI_EXPRESS_DPC_CONTROL indicates the OS supports both DPC
+		 * and EDR.
+		 */
+		if (IS_ENABLED(CONFIG_PCIE_DPC) && IS_ENABLED(CONFIG_PCIE_EDR))
+			pci_control |= OSC_PCI_EXPRESS_DPC_CONTROL;
+
+		requested = pci_control;
+		status = acpi_pci_osc_control_set(handle, &pci_control,
+						  OSC_PCI_EXPRESS_CAPABILITY_CONTROL);
+		if (ACPI_SUCCESS(status)) {
+			decode_pci_osc_control(root, "OS now controls", pci_control);
+			if (acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_ASPM) {
+				/*
+				 * We have ASPM control, but the FADT indicates that
+				 * it's unsupported. Leave existing configuration
+				 * intact and prevent the OS from touching it.
+				 */
+				dev_info(&device->dev, "FADT indicates ASPM is unsupported, using BIOS configuration\n");
+				*no_aspm = 1;
+			}
+		} else {
+			decode_pci_osc_control(root, "OS requested", requested);
+			decode_pci_osc_control(root, "platform willing to grant",
+					       pci_control);
+			dev_info(&device->dev, "_OSC failed (%s); disabling ASPM\n",
+				 acpi_format_exception(status));
 
-	requested = control;
-	status = acpi_pci_osc_control_set(handle, &control,
-					  OSC_PCI_EXPRESS_CAPABILITY_CONTROL);
-	if (ACPI_SUCCESS(status)) {
-		decode_osc_control(root, "OS now controls", control);
-		if (acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_ASPM) {
 			/*
-			 * We have ASPM control, but the FADT indicates that
-			 * it's unsupported. Leave existing configuration
-			 * intact and prevent the OS from touching it.
+			 * We want to disable ASPM here, but aspm_disabled
+			 * needs to remain in its state from boot so that we
+			 * properly handle PCIe 1.1 devices.  So we set this
+			 * flag here, to defer the action until after the ACPI
+			 * root scan.
 			 */
-			dev_info(&device->dev, "FADT indicates ASPM is unsupported, using BIOS configuration\n");
 			*no_aspm = 1;
 		}
-	} else {
-		decode_osc_control(root, "OS requested", requested);
-		decode_osc_control(root, "platform willing to grant", control);
-		dev_info(&device->dev, "_OSC failed (%s); disabling ASPM\n",
-			acpi_format_exception(status));
-
-		/*
-		 * We want to disable ASPM here, but aspm_disabled
-		 * needs to remain in its state from boot so that we
-		 * properly handle PCIe 1.1 devices.  So we set this
-		 * flag here, to defer the action until after the ACPI
-		 * root scan.
-		 */
-		*no_aspm = 1;
 	}
 }
 
@@ -536,7 +693,7 @@ static int acpi_pci_root_add(struct acpi_device *device,
 	acpi_handle handle = device->handle;
 	int no_aspm = 0;
 	bool hotadd = system_state == SYSTEM_RUNNING;
-	bool is_pcie;
+	bool is_pcie, is_cxl;
 
 	root = kzalloc(sizeof(struct acpi_pci_root), GFP_KERNEL);
 	if (!root)
@@ -595,7 +752,8 @@ static int acpi_pci_root_add(struct acpi_device *device,
 	root->mcfg_addr = acpi_pci_root_get_mcfg_addr(handle);
 
 	is_pcie = strcmp(acpi_device_hid(device), "PNP0A08") == 0;
-	negotiate_os_control(root, &no_aspm, is_pcie);
+	is_cxl = strcmp(acpi_device_hid(device), "ACPI0016") == 0;
+	negotiate_os_control(root, &no_aspm, is_pcie, is_cxl);
 
 	/*
 	 * TBD: Need PCI interface for enumeration/configuration of roots.
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 5afb6ceb284f..8f3068b6118b 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -577,8 +577,10 @@ struct acpi_pci_root {
 	u16 segment;
 	struct resource secondary;	/* downstream bus range */
 
-	u32 osc_support_set;	/* _OSC state of support bits */
-	u32 osc_control_set;	/* _OSC state of control bits */
+	u32 osc_support_set;		/* _OSC state of support bits */
+	u32 osc_control_set;		/* _OSC state of control bits */
+	u32 cxl_osc_support_set;	/* _OSC state of CXL support bits */
+	u32 cxl_osc_control_set;	/* _OSC state of CXL control bits */
 	phys_addr_t mcfg_addr;
 };
 
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index d661cd0ee64d..7c41535f4dc9 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -527,6 +527,8 @@ acpi_status acpi_run_osc(acpi_handle handle, struct acpi_osc_context *context);
 #define OSC_QUERY_DWORD				0	/* DWORD 1 */
 #define OSC_SUPPORT_DWORD			1	/* DWORD 2 */
 #define OSC_CONTROL_DWORD			2	/* DWORD 3 */
+#define CXL_OSC_SUPPORT_DWORD			3	/* DWORD 4 */
+#define CXL_OSC_CONTROL_DWORD			4	/* DWORD 5 */
 
 /* _OSC Capabilities DWORD 1: Query/Control and Error Returns (generic) */
 #define OSC_QUERY_ENABLE			0x00000001  /* input */
@@ -570,6 +572,20 @@ extern bool osc_pc_lpi_support_confirmed;
 #define OSC_PCI_EXPRESS_DPC_CONTROL		0x00000080
 #define OSC_PCI_CONTROL_MASKS			0x000000bf
 
+/* CXL Host Bridge _OSC: Capabilities DWORD 4: Support Field */
+#define OSC_CXL_PORT_REG_ACCESS_SUPPORT		0x00000001
+#define OSC_CXL_PORT_DEV_REG_ACCESS_SUPPORT	0x00000002
+#define OSC_CXL_PER_SUPPORT			0x00000004
+#define OSC_CXL_NATIVE_HP_SUPPORT		0x00000008
+#define OSC_CXL_SUPPORT_MASKS			0x0000000f
+
+/* CXL Host Bridge _OSC: Capabilities DWORD 5: Control Field */
+#define OSC_CXL_MEM_PER_CONTROL			0x00000001
+#define OSC_CXL_CONTROL_MASKS			0x00000001
+
+/* CXL Host Bridge _OSC: Capabilities DWORD 6: Query/Returns Field */
+#define OSC_CXL_MEM_EXPANDER_PER_CONTROL	0x00000001
+
 #define ACPI_GSB_ACCESS_ATTRIB_QUICK		0x00000002
 #define ACPI_GSB_ACCESS_ATTRIB_SEND_RCV         0x00000004
 #define ACPI_GSB_ACCESS_ATTRIB_BYTE		0x00000006
@@ -581,6 +597,8 @@ extern bool osc_pc_lpi_support_confirmed;
 #define ACPI_GSB_ACCESS_ATTRIB_RAW_BYTES	0x0000000E
 #define ACPI_GSB_ACCESS_ATTRIB_RAW_PROCESS	0x0000000F
 
+extern acpi_status acpi_cxl_osc_control_set(acpi_handle handle,
+					     u32 *mask, u32 req);
 extern acpi_status acpi_pci_osc_control_set(acpi_handle handle,
 					     u32 *mask, u32 req);
 
-- 
2.27.0


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [RFC PATCH] ACPI: Add support for CXL_OSC
  2020-07-08 23:57 [RFC PATCH] ACPI: Add support for CXL_OSC Sean V Kelley
@ 2020-07-10 21:53 ` Bjorn Helgaas
  2020-07-12 21:52   ` Sean V Kelley
  0 siblings, 1 reply; 3+ messages in thread
From: Bjorn Helgaas @ 2020-07-10 21:53 UTC (permalink / raw)
  To: Sean V Kelley
  Cc: rjw, bhelgaas, lenb, robert.moore, erik.kaneda, linux-pci, linux-acpi

On Wed, Jul 08, 2020 at 04:57:42PM -0700, Sean V Kelley wrote:
> Compute eXpress Link host bridges enumerate to the OS with
> ACPI-described support and control capabilities. An _OSC interface
> applies to host bridge ACPI devices that originate CXL hierarchies.
> 
> These ACPI devices have a _HID of (_CID including) EISAID(“ACPI0016”).
> For CXL 2.0 and later hierarchies, CXL _OSC is required. CXL _OSC is
> optional for CXL 1.1 hierarchies. A CXL host bridge also originates a
> PCIe hierarchy and will have a _CID of EISAID(“PNP0A08”). As such, a CXL
> Host Bridge device may expose either CXL _OSC or PCIe _OSC.
> 
> CXL end point devices connected behind a CXL bridge are standard PCI or
> CXL devices. It is the same device with two labels - ACPI0016 or PNP0A08
> label. For example, with a given bridge device ‘PC01’ definition, the OS
> will be able to find PCI devices connected behind this bridge.
> 
>                       | CPU Root Bus 0x14 |
>                       | Downstream Port   |
>                        -------------------
>                                |
>                                | Flex Bus
>                                |
>                       |                   |  ACPI Device PC01
>                       |  Root Bus 0x15    |
>                        -------------------
>                           |          |
>                           |          |
>                       | RCiEP |   | RCiEP |
>                       |       |   | or EP |
>                       | D0 F0 |   | D0 F1 |
>                        -------     -------
> 
> As a result, it instantiates another PNP0A08 or ACPI0016 device. PN0A08
> is therefore legacy support and the ACPI0016 is for a CXL aware OS. A CXL
> aware OS would look for CXL first. If the OS evaluates CXL _OSC, it shall
> not evaluate PCIe _OSC.

s/PN0A08/PNP0A08/

But Linux really doesn't use PNP0A08 at all except to avoid a warning
for conventional PCI host bridges for which we don't have an _OSC.

PNP0A03 is really all Linux uses today.  And presumably your CXL
bridges would also include PNP0A03 in their _CID.

> This patch implements support for CXL _OSC for CXL 1.1 hiearchies.  This
> is intended as an RFC to open discussion on the feature and its
> implementation.
> 
> Some caveats and questions to consider:
> 
> 1) pci_root.c goes through many levels of calls that may need some
> changes. These patches attempt to live-with the current state. Should
> the first step perhaps be more refactoring to make room so to speak
> for the addition of CXL?

negotiate_os_control() is incredibly ugly.  I'd love it if it could be
simplified somehow, but I don't have any good suggestions.  I've tried
and failed several times.

> 2) I avoided rename where possible, and so where you see say OSC_SUPPORT
> that implies PCI.
> 
> 3) I attempted to avoid increasing the complexity of an existing function
> by creating a new CXL specific one.  In some cases CXL and PCI handling
> coexist in the same function.
> 
> As with my past CXL Enumeration RFC there are no fw images or device
> drivers yet and so this is not ready for consideration for merge.
> However, I would like to get discussion started on the ACPI changes
> needed especially if it points to some prior refactoring needs.
> 
> DocLink: https://www.computeexpresslink.org/
> DocLink: https://uefi.org/

More specific references would be useful.  Specific document,
revision, and section number.

Of course, this will all have to be packaged up with users before it
makes sense to merge it.  I know that's obvious :)

> Signed-off-by: Sean V Kelley <sean.v.kelley@linux.intel.com>
> ---
>  drivers/acpi/pci_root.c | 374 ++++++++++++++++++++++++++++------------
>  include/acpi/acpi_bus.h |   6 +-
>  include/linux/acpi.h    |  18 ++
>  3 files changed, 288 insertions(+), 110 deletions(-)
> 
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index f90e841c59f5..e71fb682072b 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -45,6 +45,7 @@ static int acpi_pci_root_scan_dependent(struct acpi_device *adev)
>  
>  static const struct acpi_device_id root_device_ids[] = {
>  	{"PNP0A03", 0},
> +	{"ACPI0016", 0},
>  	{"", 0},
>  };
>  
> @@ -135,6 +136,13 @@ static struct pci_osc_bit_struct pci_osc_support_bit[] = {
>  	{ OSC_PCI_HPX_TYPE_3_SUPPORT, "HPX-Type3" },
>  };
>  
> +static struct pci_osc_bit_struct cxl_osc_support_bit[] = {
> +	{ OSC_CXL_PORT_REG_ACCESS_SUPPORT, "CXLPortRegAccess" },
> +	{ OSC_CXL_PORT_DEV_REG_ACCESS_SUPPORT, "CXLPortDevRegAccess" },
> +	{ OSC_CXL_PER_SUPPORT, "CXLProtocolErrorReporting" },
> +	{ OSC_CXL_NATIVE_HP_SUPPORT, "CXLNativeHotPlug" },
> +};
> +
>  static struct pci_osc_bit_struct pci_osc_control_bit[] = {
>  	{ OSC_PCI_EXPRESS_NATIVE_HP_CONTROL, "PCIeHotplug" },
>  	{ OSC_PCI_SHPC_NATIVE_HP_CONTROL, "SHPCHotplug" },
> @@ -145,6 +153,10 @@ static struct pci_osc_bit_struct pci_osc_control_bit[] = {
>  	{ OSC_PCI_EXPRESS_DPC_CONTROL, "DPC" },
>  };
>  
> +static struct pci_osc_bit_struct cxl_osc_control_bit[] = {
> +	{ OSC_CXL_MEM_PER_CONTROL, "CXLMemProtocolErrorReport" },
> +};
> +
>  static void decode_osc_bits(struct acpi_pci_root *root, char *msg, u32 word,
>  			    struct pci_osc_bit_struct *table, int size)
>  {
> @@ -161,27 +173,40 @@ static void decode_osc_bits(struct acpi_pci_root *root, char *msg, u32 word,
>  	dev_info(&root->device->dev, "_OSC: %s [%s]\n", msg, buf);
>  }
>  
> -static void decode_osc_support(struct acpi_pci_root *root, char *msg, u32 word)
> +static void decode_cxl_osc_support(struct acpi_pci_root *root, char *msg, u32 word)
> +{
> +	decode_osc_bits(root, msg, word, cxl_osc_support_bit,
> +			ARRAY_SIZE(cxl_osc_support_bit));
> +}
> +
> +static void decode_cxl_osc_control(struct acpi_pci_root *root, char *msg, u32 word)
> +{
> +	decode_osc_bits(root, msg, word, cxl_osc_control_bit,
> +			ARRAY_SIZE(cxl_osc_control_bit));
> +}
> +
> +static void decode_pci_osc_support(struct acpi_pci_root *root, char *msg, u32 word)
>  {
>  	decode_osc_bits(root, msg, word, pci_osc_support_bit,
>  			ARRAY_SIZE(pci_osc_support_bit));
>  }
>  
> -static void decode_osc_control(struct acpi_pci_root *root, char *msg, u32 word)
> +static void decode_pci_osc_control(struct acpi_pci_root *root, char *msg, u32 word)
>  {
>  	decode_osc_bits(root, msg, word, pci_osc_control_bit,
>  			ARRAY_SIZE(pci_osc_control_bit));
>  }
>  
>  static u8 pci_osc_uuid_str[] = "33DB4D5B-1FF7-401C-9657-7441C03DD766";
> +static u8 cxl_osc_uuid_str[] = "68F2D50B-C469-4d8A-BD3D-941A103FD3FC";
>  
> -static acpi_status acpi_pci_run_osc(acpi_handle handle,
> -				    const u32 *capbuf, u32 *retval)
> +static acpi_status acpi_cap_run_osc(acpi_handle handle,
> +				    const u32 *capbuf, u8 *uuid_str, u32 *retval)
>  {
>  	struct acpi_osc_context context = {
> -		.uuid_str = pci_osc_uuid_str,
> +		.uuid_str = uuid_str,
>  		.rev = 1,
> -		.cap.length = 12,
> +		.cap.length = 20,
>  		.cap.pointer = (void *)capbuf,
>  	};
>  	acpi_status status;
> @@ -194,12 +219,41 @@ static acpi_status acpi_pci_run_osc(acpi_handle handle,
>  	return status;
>  }
>  
> +static acpi_status acpi_cxl_query_osc(struct acpi_pci_root *root,
> +				      u32 support,
> +				      u32 *control)
> +{
> +	acpi_status status;
> +	u32 result, capbuf[5];
> +
> +	support &= OSC_CXL_SUPPORT_MASKS;
> +	support |= root->cxl_osc_support_set;
> +
> +	capbuf[OSC_QUERY_DWORD] = OSC_QUERY_ENABLE;
> +	capbuf[CXL_OSC_SUPPORT_DWORD] = support;
> +	if (control) {
> +		*control &= OSC_CXL_CONTROL_MASKS;
> +		capbuf[CXL_OSC_CONTROL_DWORD] = *control | root->cxl_osc_control_set;
> +	} else {
> +		/* Run _OSC query only with existing controls. */
> +		capbuf[CXL_OSC_CONTROL_DWORD] = root->cxl_osc_control_set;
> +	}
> +
> +	status = acpi_cap_run_osc(root->device->handle, capbuf, cxl_osc_uuid_str, &result);
> +	if (ACPI_SUCCESS(status)) {
> +		root->cxl_osc_support_set = support;
> +		if (control)
> +			*control = result;
> +	}
> +	return status;
> +}
> +
>  static acpi_status acpi_pci_query_osc(struct acpi_pci_root *root,
> -					u32 support,
> -					u32 *control)
> +				      u32 support,
> +				      u32 *control)
>  {
>  	acpi_status status;
> -	u32 result, capbuf[3];
> +	u32 result, capbuf[5];
>  
>  	support &= OSC_PCI_SUPPORT_MASKS;
>  	support |= root->osc_support_set;
> @@ -214,7 +268,7 @@ static acpi_status acpi_pci_query_osc(struct acpi_pci_root *root,
>  		capbuf[OSC_CONTROL_DWORD] = root->osc_control_set;
>  	}
>  
> -	status = acpi_pci_run_osc(root->device->handle, capbuf, &result);
> +	status = acpi_cap_run_osc(root->device->handle, capbuf, pci_osc_uuid_str, &result);
>  	if (ACPI_SUCCESS(status)) {
>  		root->osc_support_set = support;
>  		if (control)
> @@ -223,6 +277,16 @@ static acpi_status acpi_pci_query_osc(struct acpi_pci_root *root,
>  	return status;
>  }
>  
> +static acpi_status acpi_cxl_osc_support(struct acpi_pci_root *root, u32 flags)
> +{
> +	acpi_status status;
> +
> +	mutex_lock(&osc_lock);
> +	status = acpi_cxl_query_osc(root, flags, NULL);
> +	mutex_unlock(&osc_lock);
> +	return status;
> +}
> +
>  static acpi_status acpi_pci_osc_support(struct acpi_pci_root *root, u32 flags)
>  {
>  	acpi_status status;
> @@ -359,7 +423,7 @@ acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask, u32 req)
>  {
>  	struct acpi_pci_root *root;
>  	acpi_status status = AE_OK;
> -	u32 ctrl, capbuf[3];
> +	u32 ctrl, capbuf[5];
>  
>  	if (!mask)
>  		return AE_BAD_PARAMETER;
> @@ -386,14 +450,14 @@ acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask, u32 req)
>  			goto out;
>  		if (ctrl == *mask)
>  			break;
> -		decode_osc_control(root, "platform does not support",
> -				   ctrl & ~(*mask));
> +		decode_pci_osc_control(root, "platform does not support",
> +				       ctrl & ~(*mask));
>  		ctrl = *mask;
>  	}
>  
>  	if ((ctrl & req) != req) {
> -		decode_osc_control(root, "not requesting control; platform does not support",
> -				   req & ~(ctrl));
> +		decode_pci_osc_control(root, "not requesting control; platform does not support",
> +				       req & ~(ctrl));
>  		status = AE_SUPPORT;
>  		goto out;
>  	}
> @@ -401,7 +465,7 @@ acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask, u32 req)
>  	capbuf[OSC_QUERY_DWORD] = 0;
>  	capbuf[OSC_SUPPORT_DWORD] = root->osc_support_set;
>  	capbuf[OSC_CONTROL_DWORD] = ctrl;
> -	status = acpi_pci_run_osc(handle, capbuf, mask);
> +	status = acpi_cap_run_osc(handle, capbuf, pci_osc_uuid_str, mask);
>  	if (ACPI_SUCCESS(status))
>  		root->osc_control_set = *mask;
>  out:
> @@ -410,119 +474,212 @@ acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask, u32 req)
>  }
>  EXPORT_SYMBOL(acpi_pci_osc_control_set);
>  
> +/**
> + * acpi_cxl_osc_control_set - Request control of CXL root _OSC features.
> + * @handle: ACPI handle of a PCI root bridge (or PCIe Root Complex).
> + * @mask: Mask of _OSC bits to request control of, place to store control mask.
> + * @req: Mask of _OSC bits the control of is essential to the caller.
> + **/
> +acpi_status acpi_cxl_osc_control_set(acpi_handle handle, u32 *mask, u32 req)
> +{
> +	struct acpi_pci_root *root;
> +	acpi_status status = AE_OK;
> +	u32 ctrl, capbuf[5];
> +
> +	if (!mask)
> +		return AE_BAD_PARAMETER;
> +
> +	ctrl = *mask & OSC_CXL_CONTROL_MASKS;
> +	if ((ctrl & req) != req)
> +		return AE_TYPE;
> +
> +	root = acpi_pci_find_root(handle);
> +	if (!root)
> +		return AE_NOT_EXIST;
> +
> +	mutex_lock(&osc_lock);
> +
> +	*mask = ctrl | root->cxl_osc_control_set;
> +	/* No need to evaluate _OSC if the control was already granted. */
> +	if ((root->cxl_osc_control_set & ctrl) == ctrl)
> +		goto out;
> +
> +	/* Need to check the available controls bits before requesting them. */
> +	while (*mask) {
> +		status = acpi_cxl_query_osc(root, root->cxl_osc_support_set, mask);
> +		if (ACPI_FAILURE(status))
> +			goto out;
> +		if (ctrl == *mask)
> +			break;
> +		decode_cxl_osc_control(root, "platform does not support",
> +				       ctrl & ~(*mask));
> +		ctrl = *mask;
> +	}
> +
> +	if ((ctrl & req) != req) {
> +		decode_cxl_osc_control(root, "not requesting control; platform does not support",
> +				       req & ~(ctrl));
> +		status = AE_SUPPORT;
> +		goto out;
> +	}
> +
> +	capbuf[OSC_QUERY_DWORD] = 0;
> +	capbuf[CXL_OSC_SUPPORT_DWORD] = root->cxl_osc_support_set;
> +	capbuf[CXL_OSC_CONTROL_DWORD] = ctrl;
> +	status = acpi_cap_run_osc(handle, capbuf, cxl_osc_uuid_str, mask);
> +	if (ACPI_SUCCESS(status))
> +		root->cxl_osc_control_set = *mask;
> +out:
> +	mutex_unlock(&osc_lock);
> +	return status;
> +}
> +EXPORT_SYMBOL(acpi_cxl_osc_control_set);
> +
>  static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm,
> -				 bool is_pcie)
> +				 bool is_pcie, bool is_cxl)
>  {
> -	u32 support, control, requested;
> +	u32 cxl_support, cxl_control, pci_support, pci_control, requested;
>  	acpi_status status;
>  	struct acpi_device *device = root->device;
>  	acpi_handle handle = device->handle;
>  
> -	/*
> -	 * Apple always return failure on _OSC calls when _OSI("Darwin") has
> -	 * been called successfully. We know the feature set supported by the
> -	 * platform, so avoid calling _OSC at all
> -	 */
> -	if (x86_apple_machine) {
> -		root->osc_control_set = ~OSC_PCI_EXPRESS_PME_CONTROL;
> -		decode_osc_control(root, "OS assumes control of",
> -				   root->osc_control_set);
> -		return;
> -	}
>  
> -	/*
> -	 * All supported architectures that use ACPI have support for
> -	 * 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())
> -		support |= OSC_PCI_ASPM_SUPPORT | OSC_PCI_CLOCK_PM_SUPPORT;
> -	if (pci_msi_enabled())
> -		support |= OSC_PCI_MSI_SUPPORT;
> -	if (IS_ENABLED(CONFIG_PCIE_EDR))
> -		support |= OSC_PCI_EDR_SUPPORT;
> -
> -	decode_osc_support(root, "OS supports", support);
> -	status = acpi_pci_osc_support(root, support);
> -	if (ACPI_FAILURE(status)) {
> -		*no_aspm = 1;
> +	if (is_cxl) {

It would drastically shorten your patch if you used this style
instead:

  if (is_cxl) {
    ...
    return;
  }

  <existing code>

Or, probably even better, just make a new negotiate_cxl_control() and
make the caller choose.

> +		cxl_support = 0; /* No presumed default CXL_OSC support capabilities */
> +		cxl_control = 0; /* No presumed default CXL_OSC control capabilities */
>  
> -		/* _OSC is optional for PCI host bridges */
> -		if ((status == AE_NOT_FOUND) && !is_pcie)
> +		decode_cxl_osc_support(root, "OS supports", cxl_support);
> +		status = acpi_cxl_osc_support(root, cxl_support);
> +		if (ACPI_FAILURE(status)) {
> +			dev_info(&device->dev, "CXL_OSC failed (%s)\n",
> +				 acpi_format_exception(status));
>  			return;
> +		}
>  
> -		dev_info(&device->dev, "_OSC failed (%s)%s\n",
> -			 acpi_format_exception(status),
> -			 pcie_aspm_support_enabled() ? "; disabling ASPM" : "");
> -		return;
> -	}
> +		cxl_control = OSC_CXL_MEM_PER_CONTROL;
> +		requested = cxl_control;
> +		status = acpi_cxl_osc_control_set(handle, &cxl_control,
> +						  OSC_CXL_MEM_PER_CONTROL);
> +		if (ACPI_SUCCESS(status)) {
> +			decode_cxl_osc_control(root, "OS now controls",
> +					       cxl_control);
> +		} else {
> +			decode_cxl_osc_control(root, "OS requested", requested);
> +			decode_cxl_osc_control(root, "platform willing to grant",
> +					       cxl_control);
> +			dev_info(&device->dev, "_OSC failed (%s)\n",
> +				 acpi_format_exception(status));
> +		}
>  
> -	if (pcie_ports_disabled) {
> -		dev_info(&device->dev, "PCIe port services disabled; not requesting _OSC control\n");
> -		return;
> -	}
> +	} else { /* PCI/PCIe */
>  
> -	if ((support & ACPI_PCIE_REQ_SUPPORT) != ACPI_PCIE_REQ_SUPPORT) {
> -		decode_osc_support(root, "not requesting OS control; OS requires",
> -				   ACPI_PCIE_REQ_SUPPORT);
> -		return;
> -	}
> +		/*
> +		 * Apple always return failure on _OSC calls when _OSI("Darwin") has
> +		 * been called successfully. We know the feature set supported by the
> +		 * platform, so avoid calling _OSC at all
> +		 */
> +		if (x86_apple_machine) {
> +			root->osc_control_set = ~OSC_PCI_EXPRESS_PME_CONTROL;
> +			decode_pci_osc_control(root, "OS assumes control of",
> +					       root->osc_control_set);
> +			return;
> +		}
> +
> +		/*
> +		 * All supported architectures that use ACPI have support for
> +		 * PCI domains, so we indicate this in _OSC support capabilities.
> +		 */
> +		pci_support = OSC_PCI_SEGMENT_GROUPS_SUPPORT;
> +		pci_support |= OSC_PCI_HPX_TYPE_3_SUPPORT;
> +		if (pci_ext_cfg_avail())
> +			pci_support |= OSC_PCI_EXT_CONFIG_SUPPORT;
> +		if (pcie_aspm_support_enabled())
> +			pci_support |= OSC_PCI_ASPM_SUPPORT | OSC_PCI_CLOCK_PM_SUPPORT;
> +		if (pci_msi_enabled())
> +			pci_support |= OSC_PCI_MSI_SUPPORT;
> +		if (IS_ENABLED(CONFIG_PCIE_EDR))
> +			pci_support |= OSC_PCI_EDR_SUPPORT;
> +
> +		decode_pci_osc_support(root, "OS supports", pci_support);
> +		status = acpi_pci_osc_support(root, pci_support);
> +		if (ACPI_FAILURE(status)) {
> +			*no_aspm = 1;
> +
> +			/* _OSC is optional for PCI host bridges */
> +			if (status == AE_NOT_FOUND && !is_pcie)
> +				return;
> +
> +			dev_info(&device->dev, "_OSC failed (%s)%s\n",
> +				 acpi_format_exception(status),
> +				 pcie_aspm_support_enabled() ? "; disabling ASPM" : "");
> +			return;
> +		}
> +
> +		if (pcie_ports_disabled) {
> +			dev_info(&device->dev, "PCIe port services disabled; not requesting _OSC control\n");
> +			return;
> +		}
>  
> -	control = OSC_PCI_EXPRESS_CAPABILITY_CONTROL
> -		| OSC_PCI_EXPRESS_PME_CONTROL;
> +		if ((pci_support & ACPI_PCIE_REQ_SUPPORT) != ACPI_PCIE_REQ_SUPPORT) {
> +			decode_pci_osc_support(root, "not requesting OS control; OS requires",
> +					       ACPI_PCIE_REQ_SUPPORT);
> +			return;
> +		}
>  
> -	if (IS_ENABLED(CONFIG_PCIEASPM))
> -		control |= OSC_PCI_EXPRESS_LTR_CONTROL;
> +		pci_control = OSC_PCI_EXPRESS_CAPABILITY_CONTROL
> +			| OSC_PCI_EXPRESS_PME_CONTROL;
>  
> -	if (IS_ENABLED(CONFIG_HOTPLUG_PCI_PCIE))
> -		control |= OSC_PCI_EXPRESS_NATIVE_HP_CONTROL;
> +		if (IS_ENABLED(CONFIG_PCIEASPM))
> +			pci_control |= OSC_PCI_EXPRESS_LTR_CONTROL;
>  
> -	if (IS_ENABLED(CONFIG_HOTPLUG_PCI_SHPC))
> -		control |= OSC_PCI_SHPC_NATIVE_HP_CONTROL;
> +		if (IS_ENABLED(CONFIG_HOTPLUG_PCI_PCIE))
> +			pci_control |= OSC_PCI_EXPRESS_NATIVE_HP_CONTROL;
>  
> -	if (pci_aer_available())
> -		control |= OSC_PCI_EXPRESS_AER_CONTROL;
> +		if (IS_ENABLED(CONFIG_HOTPLUG_PCI_SHPC))
> +			pci_control |= OSC_PCI_SHPC_NATIVE_HP_CONTROL;
>  
> -	/*
> -	 * Per the Downstream Port Containment Related Enhancements ECN to
> -	 * the PCI Firmware Spec, r3.2, sec 4.5.1, table 4-5,
> -	 * OSC_PCI_EXPRESS_DPC_CONTROL indicates the OS supports both DPC
> -	 * and EDR.
> -	 */
> -	if (IS_ENABLED(CONFIG_PCIE_DPC) && IS_ENABLED(CONFIG_PCIE_EDR))
> -		control |= OSC_PCI_EXPRESS_DPC_CONTROL;
> +		if (pci_aer_available())
> +			pci_control |= OSC_PCI_EXPRESS_AER_CONTROL;
> +
> +		/*
> +		 * Per the Downstream Port Containment Related Enhancements ECN to
> +		 * the PCI Firmware Spec, r3.2, sec 4.5.1, table 4-5,
> +		 * OSC_PCI_EXPRESS_DPC_CONTROL indicates the OS supports both DPC
> +		 * and EDR.
> +		 */
> +		if (IS_ENABLED(CONFIG_PCIE_DPC) && IS_ENABLED(CONFIG_PCIE_EDR))
> +			pci_control |= OSC_PCI_EXPRESS_DPC_CONTROL;
> +
> +		requested = pci_control;
> +		status = acpi_pci_osc_control_set(handle, &pci_control,
> +						  OSC_PCI_EXPRESS_CAPABILITY_CONTROL);
> +		if (ACPI_SUCCESS(status)) {
> +			decode_pci_osc_control(root, "OS now controls", pci_control);
> +			if (acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_ASPM) {
> +				/*
> +				 * We have ASPM control, but the FADT indicates that
> +				 * it's unsupported. Leave existing configuration
> +				 * intact and prevent the OS from touching it.
> +				 */
> +				dev_info(&device->dev, "FADT indicates ASPM is unsupported, using BIOS configuration\n");
> +				*no_aspm = 1;
> +			}
> +		} else {
> +			decode_pci_osc_control(root, "OS requested", requested);
> +			decode_pci_osc_control(root, "platform willing to grant",
> +					       pci_control);
> +			dev_info(&device->dev, "_OSC failed (%s); disabling ASPM\n",
> +				 acpi_format_exception(status));
>  
> -	requested = control;
> -	status = acpi_pci_osc_control_set(handle, &control,
> -					  OSC_PCI_EXPRESS_CAPABILITY_CONTROL);
> -	if (ACPI_SUCCESS(status)) {
> -		decode_osc_control(root, "OS now controls", control);
> -		if (acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_ASPM) {
>  			/*
> -			 * We have ASPM control, but the FADT indicates that
> -			 * it's unsupported. Leave existing configuration
> -			 * intact and prevent the OS from touching it.
> +			 * We want to disable ASPM here, but aspm_disabled
> +			 * needs to remain in its state from boot so that we
> +			 * properly handle PCIe 1.1 devices.  So we set this
> +			 * flag here, to defer the action until after the ACPI
> +			 * root scan.
>  			 */
> -			dev_info(&device->dev, "FADT indicates ASPM is unsupported, using BIOS configuration\n");
>  			*no_aspm = 1;
>  		}
> -	} else {
> -		decode_osc_control(root, "OS requested", requested);
> -		decode_osc_control(root, "platform willing to grant", control);
> -		dev_info(&device->dev, "_OSC failed (%s); disabling ASPM\n",
> -			acpi_format_exception(status));
> -
> -		/*
> -		 * We want to disable ASPM here, but aspm_disabled
> -		 * needs to remain in its state from boot so that we
> -		 * properly handle PCIe 1.1 devices.  So we set this
> -		 * flag here, to defer the action until after the ACPI
> -		 * root scan.
> -		 */
> -		*no_aspm = 1;
>  	}
>  }
>  
> @@ -536,7 +693,7 @@ static int acpi_pci_root_add(struct acpi_device *device,
>  	acpi_handle handle = device->handle;
>  	int no_aspm = 0;
>  	bool hotadd = system_state == SYSTEM_RUNNING;
> -	bool is_pcie;
> +	bool is_pcie, is_cxl;
>  
>  	root = kzalloc(sizeof(struct acpi_pci_root), GFP_KERNEL);
>  	if (!root)
> @@ -595,7 +752,8 @@ static int acpi_pci_root_add(struct acpi_device *device,
>  	root->mcfg_addr = acpi_pci_root_get_mcfg_addr(handle);
>  
>  	is_pcie = strcmp(acpi_device_hid(device), "PNP0A08") == 0;
> -	negotiate_os_control(root, &no_aspm, is_pcie);
> +	is_cxl = strcmp(acpi_device_hid(device), "ACPI0016") == 0;
> +	negotiate_os_control(root, &no_aspm, is_pcie, is_cxl);
>  
>  	/*
>  	 * TBD: Need PCI interface for enumeration/configuration of roots.
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index 5afb6ceb284f..8f3068b6118b 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -577,8 +577,10 @@ struct acpi_pci_root {
>  	u16 segment;
>  	struct resource secondary;	/* downstream bus range */
>  
> -	u32 osc_support_set;	/* _OSC state of support bits */
> -	u32 osc_control_set;	/* _OSC state of control bits */
> +	u32 osc_support_set;		/* _OSC state of support bits */
> +	u32 osc_control_set;		/* _OSC state of control bits */
> +	u32 cxl_osc_support_set;	/* _OSC state of CXL support bits */
> +	u32 cxl_osc_control_set;	/* _OSC state of CXL control bits */

Probably "osc_cxl_support_set".  Also for the #defines below.

I assume there will be a CONFIG_PCI_CXL or similar that will wrap much
of this code.

>  	phys_addr_t mcfg_addr;
>  };
>  
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index d661cd0ee64d..7c41535f4dc9 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -527,6 +527,8 @@ acpi_status acpi_run_osc(acpi_handle handle, struct acpi_osc_context *context);
>  #define OSC_QUERY_DWORD				0	/* DWORD 1 */
>  #define OSC_SUPPORT_DWORD			1	/* DWORD 2 */
>  #define OSC_CONTROL_DWORD			2	/* DWORD 3 */
> +#define CXL_OSC_SUPPORT_DWORD			3	/* DWORD 4 */
> +#define CXL_OSC_CONTROL_DWORD			4	/* DWORD 5 */
>  
>  /* _OSC Capabilities DWORD 1: Query/Control and Error Returns (generic) */
>  #define OSC_QUERY_ENABLE			0x00000001  /* input */
> @@ -570,6 +572,20 @@ extern bool osc_pc_lpi_support_confirmed;
>  #define OSC_PCI_EXPRESS_DPC_CONTROL		0x00000080
>  #define OSC_PCI_CONTROL_MASKS			0x000000bf
>  
> +/* CXL Host Bridge _OSC: Capabilities DWORD 4: Support Field */
> +#define OSC_CXL_PORT_REG_ACCESS_SUPPORT		0x00000001
> +#define OSC_CXL_PORT_DEV_REG_ACCESS_SUPPORT	0x00000002
> +#define OSC_CXL_PER_SUPPORT			0x00000004
> +#define OSC_CXL_NATIVE_HP_SUPPORT		0x00000008
> +#define OSC_CXL_SUPPORT_MASKS			0x0000000f
> +
> +/* CXL Host Bridge _OSC: Capabilities DWORD 5: Control Field */
> +#define OSC_CXL_MEM_PER_CONTROL			0x00000001
> +#define OSC_CXL_CONTROL_MASKS			0x00000001
> +
> +/* CXL Host Bridge _OSC: Capabilities DWORD 6: Query/Returns Field */
> +#define OSC_CXL_MEM_EXPANDER_PER_CONTROL	0x00000001
> +
>  #define ACPI_GSB_ACCESS_ATTRIB_QUICK		0x00000002
>  #define ACPI_GSB_ACCESS_ATTRIB_SEND_RCV         0x00000004
>  #define ACPI_GSB_ACCESS_ATTRIB_BYTE		0x00000006
> @@ -581,6 +597,8 @@ extern bool osc_pc_lpi_support_confirmed;
>  #define ACPI_GSB_ACCESS_ATTRIB_RAW_BYTES	0x0000000E
>  #define ACPI_GSB_ACCESS_ATTRIB_RAW_PROCESS	0x0000000F
>  
> +extern acpi_status acpi_cxl_osc_control_set(acpi_handle handle,
> +					     u32 *mask, u32 req);

No need for this to be extern, right?  (I just sent a patch to make
acpi_pci_osc_control_set() static also.)

>  extern acpi_status acpi_pci_osc_control_set(acpi_handle handle,
>  					     u32 *mask, u32 req);
>  
> -- 
> 2.27.0
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [RFC PATCH] ACPI: Add support for CXL_OSC
  2020-07-10 21:53 ` Bjorn Helgaas
@ 2020-07-12 21:52   ` Sean V Kelley
  0 siblings, 0 replies; 3+ messages in thread
From: Sean V Kelley @ 2020-07-12 21:52 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: rjw, bhelgaas, lenb, robert.moore, erik.kaneda, linux-pci, linux-acpi

Hi,

Thanks for for your feedback, my comments below:

On 10 Jul 2020, at 14:53, Bjorn Helgaas wrote:

> On Wed, Jul 08, 2020 at 04:57:42PM -0700, Sean V Kelley wrote:
>> Compute eXpress Link host bridges enumerate to the OS with
>> ACPI-described support and control capabilities. An _OSC interface
>> applies to host bridge ACPI devices that originate CXL hierarchies.
>>
>> These ACPI devices have a _HID of (_CID including) 
>> EISAID(“ACPI0016”).
>> For CXL 2.0 and later hierarchies, CXL _OSC is required. CXL _OSC is
>> optional for CXL 1.1 hierarchies. A CXL host bridge also originates a
>> PCIe hierarchy and will have a _CID of EISAID(“PNP0A08”). As 
>> such, a CXL
>> Host Bridge device may expose either CXL _OSC or PCIe _OSC.
>>
>> CXL end point devices connected behind a CXL bridge are standard PCI 
>> or
>> CXL devices. It is the same device with two labels - ACPI0016 or 
>> PNP0A08
>> label. For example, with a given bridge device ‘PC01’ definition, 
>> the OS
>> will be able to find PCI devices connected behind this bridge.
>>
>>                       | CPU Root Bus 0x14 |
>>                       | Downstream Port   |
>>                        -------------------
>>                                |
>>                                | Flex Bus
>>                                |
>>                       |                   |  ACPI Device PC01
>>                       |  Root Bus 0x15    |
>>                        -------------------
>>                           |          |
>>                           |          |
>>                       | RCiEP |   | RCiEP |
>>                       |       |   | or EP |
>>                       | D0 F0 |   | D0 F1 |
>>                        -------     -------
>>
>> As a result, it instantiates another PNP0A08 or ACPI0016 device. 
>> PN0A08
>> is therefore legacy support and the ACPI0016 is for a CXL aware OS. A 
>> CXL
>> aware OS would look for CXL first. If the OS evaluates CXL _OSC, it 
>> shall
>> not evaluate PCIe _OSC.
>
> s/PN0A08/PNP0A08/
>
> But Linux really doesn't use PNP0A08 at all except to avoid a warning
> for conventional PCI host bridges for which we don't have an _OSC.
>
> PNP0A03 is really all Linux uses today.  And presumably your CXL
> bridges would also include PNP0A03 in their _CID.

Yes it’s used as a part of the _CID as well.  According to the spec - 
"ACPI-6_3-ECR-CXL OSC", the Host Bridge created for CXL can have a _CID 
of "PNP0A08”. However, this _CID value of “PNP0A08” by itself will 
result in an OSC method not being evaluated by an OS without any CXL 
awareness. As a result, a legacy OS cannot fully support the PCIe 
features used by CXL devices.  While the BIOS vendor can use the _CID of 
“PNP0A03” it’s best to fix the spec.  So that is a request 
underway (modification of Spec "ACPI-6_3-ECR-CXL OSC.docx" (and 
consequently BIOS) to ensure primacy of _CID "PNP0A03" with regards to 
"PNP0A08”)


>
>> This patch implements support for CXL _OSC for CXL 1.1 hiearchies.  
>> This
>> is intended as an RFC to open discussion on the feature and its
>> implementation.
>>
>> Some caveats and questions to consider:
>>
>> 1) pci_root.c goes through many levels of calls that may need some
>> changes. These patches attempt to live-with the current state. Should
>> the first step perhaps be more refactoring to make room so to speak
>> for the addition of CXL?
>
> negotiate_os_control() is incredibly ugly.  I'd love it if it could be
> simplified somehow, but I don't have any good suggestions.  I've tried
> and failed several times.

I’ll have a further look to perhaps suggest some changes.

>
>> 2) I avoided rename where possible, and so where you see say 
>> OSC_SUPPORT
>> that implies PCI.
>>
>> 3) I attempted to avoid increasing the complexity of an existing 
>> function
>> by creating a new CXL specific one.  In some cases CXL and PCI 
>> handling
>> coexist in the same function.
>>
>> As with my past CXL Enumeration RFC there are no fw images or device
>> drivers yet and so this is not ready for consideration for merge.
>> However, I would like to get discussion started on the ACPI changes
>> needed especially if it points to some prior refactoring needs.
>>
>> DocLink: https://www.computeexpresslink.org/
>> DocLink: https://uefi.org/
>
> More specific references would be useful.  Specific document,
> revision, and section number.

Understood, will do.

>
> Of course, this will all have to be packaged up with users before it
> makes sense to merge it.  I know that's obvious :)

Agree!

>
>> Signed-off-by: Sean V Kelley <sean.v.kelley@linux.intel.com>
>> ---
>>  drivers/acpi/pci_root.c | 374 
>> ++++++++++++++++++++++++++++------------
>>  include/acpi/acpi_bus.h |   6 +-
>>  include/linux/acpi.h    |  18 ++
>>  3 files changed, 288 insertions(+), 110 deletions(-)
>>
>> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
>> index f90e841c59f5..e71fb682072b 100644
>> --- a/drivers/acpi/pci_root.c
>> +++ b/drivers/acpi/pci_root.c
>> @@ -45,6 +45,7 @@ static int acpi_pci_root_scan_dependent(struct 
>> acpi_device *adev)
>>
>>  static const struct acpi_device_id root_device_ids[] = {
>>  	{"PNP0A03", 0},
>> +	{"ACPI0016", 0},
>>  	{"", 0},
>>  };
>>
>> @@ -135,6 +136,13 @@ static struct pci_osc_bit_struct 
>> pci_osc_support_bit[] = {
>>  	{ OSC_PCI_HPX_TYPE_3_SUPPORT, "HPX-Type3" },
>>  };
>>
>> +static struct pci_osc_bit_struct cxl_osc_support_bit[] = {
>> +	{ OSC_CXL_PORT_REG_ACCESS_SUPPORT, "CXLPortRegAccess" },
>> +	{ OSC_CXL_PORT_DEV_REG_ACCESS_SUPPORT, "CXLPortDevRegAccess" },
>> +	{ OSC_CXL_PER_SUPPORT, "CXLProtocolErrorReporting" },
>> +	{ OSC_CXL_NATIVE_HP_SUPPORT, "CXLNativeHotPlug" },
>> +};
>> +
>>  static struct pci_osc_bit_struct pci_osc_control_bit[] = {
>>  	{ OSC_PCI_EXPRESS_NATIVE_HP_CONTROL, "PCIeHotplug" },
>>  	{ OSC_PCI_SHPC_NATIVE_HP_CONTROL, "SHPCHotplug" },
>> @@ -145,6 +153,10 @@ static struct pci_osc_bit_struct 
>> pci_osc_control_bit[] = {
>>  	{ OSC_PCI_EXPRESS_DPC_CONTROL, "DPC" },
>>  };
>>
>> +static struct pci_osc_bit_struct cxl_osc_control_bit[] = {
>> +	{ OSC_CXL_MEM_PER_CONTROL, "CXLMemProtocolErrorReport" },
>> +};
>> +
>>  static void decode_osc_bits(struct acpi_pci_root *root, char *msg, 
>> u32 word,
>>  			    struct pci_osc_bit_struct *table, int size)
>>  {
>> @@ -161,27 +173,40 @@ static void decode_osc_bits(struct 
>> acpi_pci_root *root, char *msg, u32 word,
>>  	dev_info(&root->device->dev, "_OSC: %s [%s]\n", msg, buf);
>>  }
>>
>> -static void decode_osc_support(struct acpi_pci_root *root, char 
>> *msg, u32 word)
>> +static void decode_cxl_osc_support(struct acpi_pci_root *root, char 
>> *msg, u32 word)
>> +{
>> +	decode_osc_bits(root, msg, word, cxl_osc_support_bit,
>> +			ARRAY_SIZE(cxl_osc_support_bit));
>> +}
>> +
>> +static void decode_cxl_osc_control(struct acpi_pci_root *root, char 
>> *msg, u32 word)
>> +{
>> +	decode_osc_bits(root, msg, word, cxl_osc_control_bit,
>> +			ARRAY_SIZE(cxl_osc_control_bit));
>> +}
>> +
>> +static void decode_pci_osc_support(struct acpi_pci_root *root, char 
>> *msg, u32 word)
>>  {
>>  	decode_osc_bits(root, msg, word, pci_osc_support_bit,
>>  			ARRAY_SIZE(pci_osc_support_bit));
>>  }
>>
>> -static void decode_osc_control(struct acpi_pci_root *root, char 
>> *msg, u32 word)
>> +static void decode_pci_osc_control(struct acpi_pci_root *root, char 
>> *msg, u32 word)
>>  {
>>  	decode_osc_bits(root, msg, word, pci_osc_control_bit,
>>  			ARRAY_SIZE(pci_osc_control_bit));
>>  }
>>
>>  static u8 pci_osc_uuid_str[] = 
>> "33DB4D5B-1FF7-401C-9657-7441C03DD766";
>> +static u8 cxl_osc_uuid_str[] = 
>> "68F2D50B-C469-4d8A-BD3D-941A103FD3FC";
>>
>> -static acpi_status acpi_pci_run_osc(acpi_handle handle,
>> -				    const u32 *capbuf, u32 *retval)
>> +static acpi_status acpi_cap_run_osc(acpi_handle handle,
>> +				    const u32 *capbuf, u8 *uuid_str, u32 *retval)
>>  {
>>  	struct acpi_osc_context context = {
>> -		.uuid_str = pci_osc_uuid_str,
>> +		.uuid_str = uuid_str,
>>  		.rev = 1,
>> -		.cap.length = 12,
>> +		.cap.length = 20,
>>  		.cap.pointer = (void *)capbuf,
>>  	};
>>  	acpi_status status;
>> @@ -194,12 +219,41 @@ static acpi_status acpi_pci_run_osc(acpi_handle 
>> handle,
>>  	return status;
>>  }
>>
>> +static acpi_status acpi_cxl_query_osc(struct acpi_pci_root *root,
>> +				      u32 support,
>> +				      u32 *control)
>> +{
>> +	acpi_status status;
>> +	u32 result, capbuf[5];
>> +
>> +	support &= OSC_CXL_SUPPORT_MASKS;
>> +	support |= root->cxl_osc_support_set;
>> +
>> +	capbuf[OSC_QUERY_DWORD] = OSC_QUERY_ENABLE;
>> +	capbuf[CXL_OSC_SUPPORT_DWORD] = support;
>> +	if (control) {
>> +		*control &= OSC_CXL_CONTROL_MASKS;
>> +		capbuf[CXL_OSC_CONTROL_DWORD] = *control | 
>> root->cxl_osc_control_set;
>> +	} else {
>> +		/* Run _OSC query only with existing controls. */
>> +		capbuf[CXL_OSC_CONTROL_DWORD] = root->cxl_osc_control_set;
>> +	}
>> +
>> +	status = acpi_cap_run_osc(root->device->handle, capbuf, 
>> cxl_osc_uuid_str, &result);
>> +	if (ACPI_SUCCESS(status)) {
>> +		root->cxl_osc_support_set = support;
>> +		if (control)
>> +			*control = result;
>> +	}
>> +	return status;
>> +}
>> +
>>  static acpi_status acpi_pci_query_osc(struct acpi_pci_root *root,
>> -					u32 support,
>> -					u32 *control)
>> +				      u32 support,
>> +				      u32 *control)
>>  {
>>  	acpi_status status;
>> -	u32 result, capbuf[3];
>> +	u32 result, capbuf[5];
>>
>>  	support &= OSC_PCI_SUPPORT_MASKS;
>>  	support |= root->osc_support_set;
>> @@ -214,7 +268,7 @@ static acpi_status acpi_pci_query_osc(struct 
>> acpi_pci_root *root,
>>  		capbuf[OSC_CONTROL_DWORD] = root->osc_control_set;
>>  	}
>>
>> -	status = acpi_pci_run_osc(root->device->handle, capbuf, &result);
>> +	status = acpi_cap_run_osc(root->device->handle, capbuf, 
>> pci_osc_uuid_str, &result);
>>  	if (ACPI_SUCCESS(status)) {
>>  		root->osc_support_set = support;
>>  		if (control)
>> @@ -223,6 +277,16 @@ static acpi_status acpi_pci_query_osc(struct 
>> acpi_pci_root *root,
>>  	return status;
>>  }
>>
>> +static acpi_status acpi_cxl_osc_support(struct acpi_pci_root *root, 
>> u32 flags)
>> +{
>> +	acpi_status status;
>> +
>> +	mutex_lock(&osc_lock);
>> +	status = acpi_cxl_query_osc(root, flags, NULL);
>> +	mutex_unlock(&osc_lock);
>> +	return status;
>> +}
>> +
>>  static acpi_status acpi_pci_osc_support(struct acpi_pci_root *root, 
>> u32 flags)
>>  {
>>  	acpi_status status;
>> @@ -359,7 +423,7 @@ acpi_status acpi_pci_osc_control_set(acpi_handle 
>> handle, u32 *mask, u32 req)
>>  {
>>  	struct acpi_pci_root *root;
>>  	acpi_status status = AE_OK;
>> -	u32 ctrl, capbuf[3];
>> +	u32 ctrl, capbuf[5];
>>
>>  	if (!mask)
>>  		return AE_BAD_PARAMETER;
>> @@ -386,14 +450,14 @@ acpi_status 
>> acpi_pci_osc_control_set(acpi_handle handle, u32 *mask, u32 req)
>>  			goto out;
>>  		if (ctrl == *mask)
>>  			break;
>> -		decode_osc_control(root, "platform does not support",
>> -				   ctrl & ~(*mask));
>> +		decode_pci_osc_control(root, "platform does not support",
>> +				       ctrl & ~(*mask));
>>  		ctrl = *mask;
>>  	}
>>
>>  	if ((ctrl & req) != req) {
>> -		decode_osc_control(root, "not requesting control; platform does 
>> not support",
>> -				   req & ~(ctrl));
>> +		decode_pci_osc_control(root, "not requesting control; platform 
>> does not support",
>> +				       req & ~(ctrl));
>>  		status = AE_SUPPORT;
>>  		goto out;
>>  	}
>> @@ -401,7 +465,7 @@ acpi_status acpi_pci_osc_control_set(acpi_handle 
>> handle, u32 *mask, u32 req)
>>  	capbuf[OSC_QUERY_DWORD] = 0;
>>  	capbuf[OSC_SUPPORT_DWORD] = root->osc_support_set;
>>  	capbuf[OSC_CONTROL_DWORD] = ctrl;
>> -	status = acpi_pci_run_osc(handle, capbuf, mask);
>> +	status = acpi_cap_run_osc(handle, capbuf, pci_osc_uuid_str, mask);
>>  	if (ACPI_SUCCESS(status))
>>  		root->osc_control_set = *mask;
>>  out:
>> @@ -410,119 +474,212 @@ acpi_status 
>> acpi_pci_osc_control_set(acpi_handle handle, u32 *mask, u32 req)
>>  }
>>  EXPORT_SYMBOL(acpi_pci_osc_control_set);
>>
>> +/**
>> + * acpi_cxl_osc_control_set - Request control of CXL root _OSC 
>> features.
>> + * @handle: ACPI handle of a PCI root bridge (or PCIe Root Complex).
>> + * @mask: Mask of _OSC bits to request control of, place to store 
>> control mask.
>> + * @req: Mask of _OSC bits the control of is essential to the 
>> caller.
>> + **/
>> +acpi_status acpi_cxl_osc_control_set(acpi_handle handle, u32 *mask, 
>> u32 req)
>> +{
>> +	struct acpi_pci_root *root;
>> +	acpi_status status = AE_OK;
>> +	u32 ctrl, capbuf[5];
>> +
>> +	if (!mask)
>> +		return AE_BAD_PARAMETER;
>> +
>> +	ctrl = *mask & OSC_CXL_CONTROL_MASKS;
>> +	if ((ctrl & req) != req)
>> +		return AE_TYPE;
>> +
>> +	root = acpi_pci_find_root(handle);
>> +	if (!root)
>> +		return AE_NOT_EXIST;
>> +
>> +	mutex_lock(&osc_lock);
>> +
>> +	*mask = ctrl | root->cxl_osc_control_set;
>> +	/* No need to evaluate _OSC if the control was already granted. */
>> +	if ((root->cxl_osc_control_set & ctrl) == ctrl)
>> +		goto out;
>> +
>> +	/* Need to check the available controls bits before requesting 
>> them. */
>> +	while (*mask) {
>> +		status = acpi_cxl_query_osc(root, root->cxl_osc_support_set, 
>> mask);
>> +		if (ACPI_FAILURE(status))
>> +			goto out;
>> +		if (ctrl == *mask)
>> +			break;
>> +		decode_cxl_osc_control(root, "platform does not support",
>> +				       ctrl & ~(*mask));
>> +		ctrl = *mask;
>> +	}
>> +
>> +	if ((ctrl & req) != req) {
>> +		decode_cxl_osc_control(root, "not requesting control; platform 
>> does not support",
>> +				       req & ~(ctrl));
>> +		status = AE_SUPPORT;
>> +		goto out;
>> +	}
>> +
>> +	capbuf[OSC_QUERY_DWORD] = 0;
>> +	capbuf[CXL_OSC_SUPPORT_DWORD] = root->cxl_osc_support_set;
>> +	capbuf[CXL_OSC_CONTROL_DWORD] = ctrl;
>> +	status = acpi_cap_run_osc(handle, capbuf, cxl_osc_uuid_str, mask);
>> +	if (ACPI_SUCCESS(status))
>> +		root->cxl_osc_control_set = *mask;
>> +out:
>> +	mutex_unlock(&osc_lock);
>> +	return status;
>> +}
>> +EXPORT_SYMBOL(acpi_cxl_osc_control_set);
>> +
>>  static void negotiate_os_control(struct acpi_pci_root *root, int 
>> *no_aspm,
>> -				 bool is_pcie)
>> +				 bool is_pcie, bool is_cxl)
>>  {
>> -	u32 support, control, requested;
>> +	u32 cxl_support, cxl_control, pci_support, pci_control, requested;
>>  	acpi_status status;
>>  	struct acpi_device *device = root->device;
>>  	acpi_handle handle = device->handle;
>>
>> -	/*
>> -	 * Apple always return failure on _OSC calls when _OSI("Darwin") 
>> has
>> -	 * been called successfully. We know the feature set supported by 
>> the
>> -	 * platform, so avoid calling _OSC at all
>> -	 */
>> -	if (x86_apple_machine) {
>> -		root->osc_control_set = ~OSC_PCI_EXPRESS_PME_CONTROL;
>> -		decode_osc_control(root, "OS assumes control of",
>> -				   root->osc_control_set);
>> -		return;
>> -	}
>>
>> -	/*
>> -	 * All supported architectures that use ACPI have support for
>> -	 * 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())
>> -		support |= OSC_PCI_ASPM_SUPPORT | OSC_PCI_CLOCK_PM_SUPPORT;
>> -	if (pci_msi_enabled())
>> -		support |= OSC_PCI_MSI_SUPPORT;
>> -	if (IS_ENABLED(CONFIG_PCIE_EDR))
>> -		support |= OSC_PCI_EDR_SUPPORT;
>> -
>> -	decode_osc_support(root, "OS supports", support);
>> -	status = acpi_pci_osc_support(root, support);
>> -	if (ACPI_FAILURE(status)) {
>> -		*no_aspm = 1;
>> +	if (is_cxl) {
>
> It would drastically shorten your patch if you used this style
> instead:
>
>   if (is_cxl) {
>     ...
>     return;
>   }
>
>   <existing code>
>
> Or, probably even better, just make a new negotiate_cxl_control() and
> make the caller choose.

Agree, and I like the latter.  Will experiment.

>
>> +		cxl_support = 0; /* No presumed default CXL_OSC support 
>> capabilities */
>> +		cxl_control = 0; /* No presumed default CXL_OSC control 
>> capabilities */
>>
>> -		/* _OSC is optional for PCI host bridges */
>> -		if ((status == AE_NOT_FOUND) && !is_pcie)
>> +		decode_cxl_osc_support(root, "OS supports", cxl_support);
>> +		status = acpi_cxl_osc_support(root, cxl_support);
>> +		if (ACPI_FAILURE(status)) {
>> +			dev_info(&device->dev, "CXL_OSC failed (%s)\n",
>> +				 acpi_format_exception(status));
>>  			return;
>> +		}
>>
>> -		dev_info(&device->dev, "_OSC failed (%s)%s\n",
>> -			 acpi_format_exception(status),
>> -			 pcie_aspm_support_enabled() ? "; disabling ASPM" : "");
>> -		return;
>> -	}
>> +		cxl_control = OSC_CXL_MEM_PER_CONTROL;
>> +		requested = cxl_control;
>> +		status = acpi_cxl_osc_control_set(handle, &cxl_control,
>> +						  OSC_CXL_MEM_PER_CONTROL);
>> +		if (ACPI_SUCCESS(status)) {
>> +			decode_cxl_osc_control(root, "OS now controls",
>> +					       cxl_control);
>> +		} else {
>> +			decode_cxl_osc_control(root, "OS requested", requested);
>> +			decode_cxl_osc_control(root, "platform willing to grant",
>> +					       cxl_control);
>> +			dev_info(&device->dev, "_OSC failed (%s)\n",
>> +				 acpi_format_exception(status));
>> +		}
>>
>> -	if (pcie_ports_disabled) {
>> -		dev_info(&device->dev, "PCIe port services disabled; not 
>> requesting _OSC control\n");
>> -		return;
>> -	}
>> +	} else { /* PCI/PCIe */
>>
>> -	if ((support & ACPI_PCIE_REQ_SUPPORT) != ACPI_PCIE_REQ_SUPPORT) {
>> -		decode_osc_support(root, "not requesting OS control; OS requires",
>> -				   ACPI_PCIE_REQ_SUPPORT);
>> -		return;
>> -	}
>> +		/*
>> +		 * Apple always return failure on _OSC calls when _OSI("Darwin") 
>> has
>> +		 * been called successfully. We know the feature set supported by 
>> the
>> +		 * platform, so avoid calling _OSC at all
>> +		 */
>> +		if (x86_apple_machine) {
>> +			root->osc_control_set = ~OSC_PCI_EXPRESS_PME_CONTROL;
>> +			decode_pci_osc_control(root, "OS assumes control of",
>> +					       root->osc_control_set);
>> +			return;
>> +		}
>> +
>> +		/*
>> +		 * All supported architectures that use ACPI have support for
>> +		 * PCI domains, so we indicate this in _OSC support capabilities.
>> +		 */
>> +		pci_support = OSC_PCI_SEGMENT_GROUPS_SUPPORT;
>> +		pci_support |= OSC_PCI_HPX_TYPE_3_SUPPORT;
>> +		if (pci_ext_cfg_avail())
>> +			pci_support |= OSC_PCI_EXT_CONFIG_SUPPORT;
>> +		if (pcie_aspm_support_enabled())
>> +			pci_support |= OSC_PCI_ASPM_SUPPORT | OSC_PCI_CLOCK_PM_SUPPORT;
>> +		if (pci_msi_enabled())
>> +			pci_support |= OSC_PCI_MSI_SUPPORT;
>> +		if (IS_ENABLED(CONFIG_PCIE_EDR))
>> +			pci_support |= OSC_PCI_EDR_SUPPORT;
>> +
>> +		decode_pci_osc_support(root, "OS supports", pci_support);
>> +		status = acpi_pci_osc_support(root, pci_support);
>> +		if (ACPI_FAILURE(status)) {
>> +			*no_aspm = 1;
>> +
>> +			/* _OSC is optional for PCI host bridges */
>> +			if (status == AE_NOT_FOUND && !is_pcie)
>> +				return;
>> +
>> +			dev_info(&device->dev, "_OSC failed (%s)%s\n",
>> +				 acpi_format_exception(status),
>> +				 pcie_aspm_support_enabled() ? "; disabling ASPM" : "");
>> +			return;
>> +		}
>> +
>> +		if (pcie_ports_disabled) {
>> +			dev_info(&device->dev, "PCIe port services disabled; not 
>> requesting _OSC control\n");
>> +			return;
>> +		}
>>
>> -	control = OSC_PCI_EXPRESS_CAPABILITY_CONTROL
>> -		| OSC_PCI_EXPRESS_PME_CONTROL;
>> +		if ((pci_support & ACPI_PCIE_REQ_SUPPORT) != 
>> ACPI_PCIE_REQ_SUPPORT) {
>> +			decode_pci_osc_support(root, "not requesting OS control; OS 
>> requires",
>> +					       ACPI_PCIE_REQ_SUPPORT);
>> +			return;
>> +		}
>>
>> -	if (IS_ENABLED(CONFIG_PCIEASPM))
>> -		control |= OSC_PCI_EXPRESS_LTR_CONTROL;
>> +		pci_control = OSC_PCI_EXPRESS_CAPABILITY_CONTROL
>> +			| OSC_PCI_EXPRESS_PME_CONTROL;
>>
>> -	if (IS_ENABLED(CONFIG_HOTPLUG_PCI_PCIE))
>> -		control |= OSC_PCI_EXPRESS_NATIVE_HP_CONTROL;
>> +		if (IS_ENABLED(CONFIG_PCIEASPM))
>> +			pci_control |= OSC_PCI_EXPRESS_LTR_CONTROL;
>>
>> -	if (IS_ENABLED(CONFIG_HOTPLUG_PCI_SHPC))
>> -		control |= OSC_PCI_SHPC_NATIVE_HP_CONTROL;
>> +		if (IS_ENABLED(CONFIG_HOTPLUG_PCI_PCIE))
>> +			pci_control |= OSC_PCI_EXPRESS_NATIVE_HP_CONTROL;
>>
>> -	if (pci_aer_available())
>> -		control |= OSC_PCI_EXPRESS_AER_CONTROL;
>> +		if (IS_ENABLED(CONFIG_HOTPLUG_PCI_SHPC))
>> +			pci_control |= OSC_PCI_SHPC_NATIVE_HP_CONTROL;
>>
>> -	/*
>> -	 * Per the Downstream Port Containment Related Enhancements ECN to
>> -	 * the PCI Firmware Spec, r3.2, sec 4.5.1, table 4-5,
>> -	 * OSC_PCI_EXPRESS_DPC_CONTROL indicates the OS supports both DPC
>> -	 * and EDR.
>> -	 */
>> -	if (IS_ENABLED(CONFIG_PCIE_DPC) && IS_ENABLED(CONFIG_PCIE_EDR))
>> -		control |= OSC_PCI_EXPRESS_DPC_CONTROL;
>> +		if (pci_aer_available())
>> +			pci_control |= OSC_PCI_EXPRESS_AER_CONTROL;
>> +
>> +		/*
>> +		 * Per the Downstream Port Containment Related Enhancements ECN to
>> +		 * the PCI Firmware Spec, r3.2, sec 4.5.1, table 4-5,
>> +		 * OSC_PCI_EXPRESS_DPC_CONTROL indicates the OS supports both DPC
>> +		 * and EDR.
>> +		 */
>> +		if (IS_ENABLED(CONFIG_PCIE_DPC) && IS_ENABLED(CONFIG_PCIE_EDR))
>> +			pci_control |= OSC_PCI_EXPRESS_DPC_CONTROL;
>> +
>> +		requested = pci_control;
>> +		status = acpi_pci_osc_control_set(handle, &pci_control,
>> +						  OSC_PCI_EXPRESS_CAPABILITY_CONTROL);
>> +		if (ACPI_SUCCESS(status)) {
>> +			decode_pci_osc_control(root, "OS now controls", pci_control);
>> +			if (acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_ASPM) {
>> +				/*
>> +				 * We have ASPM control, but the FADT indicates that
>> +				 * it's unsupported. Leave existing configuration
>> +				 * intact and prevent the OS from touching it.
>> +				 */
>> +				dev_info(&device->dev, "FADT indicates ASPM is unsupported, 
>> using BIOS configuration\n");
>> +				*no_aspm = 1;
>> +			}
>> +		} else {
>> +			decode_pci_osc_control(root, "OS requested", requested);
>> +			decode_pci_osc_control(root, "platform willing to grant",
>> +					       pci_control);
>> +			dev_info(&device->dev, "_OSC failed (%s); disabling ASPM\n",
>> +				 acpi_format_exception(status));
>>
>> -	requested = control;
>> -	status = acpi_pci_osc_control_set(handle, &control,
>> -					  OSC_PCI_EXPRESS_CAPABILITY_CONTROL);
>> -	if (ACPI_SUCCESS(status)) {
>> -		decode_osc_control(root, "OS now controls", control);
>> -		if (acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_ASPM) {
>>  			/*
>> -			 * We have ASPM control, but the FADT indicates that
>> -			 * it's unsupported. Leave existing configuration
>> -			 * intact and prevent the OS from touching it.
>> +			 * We want to disable ASPM here, but aspm_disabled
>> +			 * needs to remain in its state from boot so that we
>> +			 * properly handle PCIe 1.1 devices.  So we set this
>> +			 * flag here, to defer the action until after the ACPI
>> +			 * root scan.
>>  			 */
>> -			dev_info(&device->dev, "FADT indicates ASPM is unsupported, using 
>> BIOS configuration\n");
>>  			*no_aspm = 1;
>>  		}
>> -	} else {
>> -		decode_osc_control(root, "OS requested", requested);
>> -		decode_osc_control(root, "platform willing to grant", control);
>> -		dev_info(&device->dev, "_OSC failed (%s); disabling ASPM\n",
>> -			acpi_format_exception(status));
>> -
>> -		/*
>> -		 * We want to disable ASPM here, but aspm_disabled
>> -		 * needs to remain in its state from boot so that we
>> -		 * properly handle PCIe 1.1 devices.  So we set this
>> -		 * flag here, to defer the action until after the ACPI
>> -		 * root scan.
>> -		 */
>> -		*no_aspm = 1;
>>  	}
>>  }
>>
>> @@ -536,7 +693,7 @@ static int acpi_pci_root_add(struct acpi_device 
>> *device,
>>  	acpi_handle handle = device->handle;
>>  	int no_aspm = 0;
>>  	bool hotadd = system_state == SYSTEM_RUNNING;
>> -	bool is_pcie;
>> +	bool is_pcie, is_cxl;
>>
>>  	root = kzalloc(sizeof(struct acpi_pci_root), GFP_KERNEL);
>>  	if (!root)
>> @@ -595,7 +752,8 @@ static int acpi_pci_root_add(struct acpi_device 
>> *device,
>>  	root->mcfg_addr = acpi_pci_root_get_mcfg_addr(handle);
>>
>>  	is_pcie = strcmp(acpi_device_hid(device), "PNP0A08") == 0;
>> -	negotiate_os_control(root, &no_aspm, is_pcie);
>> +	is_cxl = strcmp(acpi_device_hid(device), "ACPI0016") == 0;
>> +	negotiate_os_control(root, &no_aspm, is_pcie, is_cxl);
>>
>>  	/*
>>  	 * TBD: Need PCI interface for enumeration/configuration of roots.
>> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
>> index 5afb6ceb284f..8f3068b6118b 100644
>> --- a/include/acpi/acpi_bus.h
>> +++ b/include/acpi/acpi_bus.h
>> @@ -577,8 +577,10 @@ struct acpi_pci_root {
>>  	u16 segment;
>>  	struct resource secondary;	/* downstream bus range */
>>
>> -	u32 osc_support_set;	/* _OSC state of support bits */
>> -	u32 osc_control_set;	/* _OSC state of control bits */
>> +	u32 osc_support_set;		/* _OSC state of support bits */
>> +	u32 osc_control_set;		/* _OSC state of control bits */
>> +	u32 cxl_osc_support_set;	/* _OSC state of CXL support bits */
>> +	u32 cxl_osc_control_set;	/* _OSC state of CXL control bits */
>
> Probably "osc_cxl_support_set".  Also for the #defines below.
>
> I assume there will be a CONFIG_PCI_CXL or similar that will wrap much
> of this code.
>
>>  	phys_addr_t mcfg_addr;
>>  };
>>
>> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
>> index d661cd0ee64d..7c41535f4dc9 100644
>> --- a/include/linux/acpi.h
>> +++ b/include/linux/acpi.h
>> @@ -527,6 +527,8 @@ acpi_status acpi_run_osc(acpi_handle handle, 
>> struct acpi_osc_context *context);
>>  #define OSC_QUERY_DWORD				0	/* DWORD 1 */
>>  #define OSC_SUPPORT_DWORD			1	/* DWORD 2 */
>>  #define OSC_CONTROL_DWORD			2	/* DWORD 3 */
>> +#define CXL_OSC_SUPPORT_DWORD			3	/* DWORD 4 */
>> +#define CXL_OSC_CONTROL_DWORD			4	/* DWORD 5 */
>>
>>  /* _OSC Capabilities DWORD 1: Query/Control and Error Returns 
>> (generic) */
>>  #define OSC_QUERY_ENABLE			0x00000001  /* input */
>> @@ -570,6 +572,20 @@ extern bool osc_pc_lpi_support_confirmed;
>>  #define OSC_PCI_EXPRESS_DPC_CONTROL		0x00000080
>>  #define OSC_PCI_CONTROL_MASKS			0x000000bf
>>
>> +/* CXL Host Bridge _OSC: Capabilities DWORD 4: Support Field */
>> +#define OSC_CXL_PORT_REG_ACCESS_SUPPORT		0x00000001
>> +#define OSC_CXL_PORT_DEV_REG_ACCESS_SUPPORT	0x00000002
>> +#define OSC_CXL_PER_SUPPORT			0x00000004
>> +#define OSC_CXL_NATIVE_HP_SUPPORT		0x00000008
>> +#define OSC_CXL_SUPPORT_MASKS			0x0000000f
>> +
>> +/* CXL Host Bridge _OSC: Capabilities DWORD 5: Control Field */
>> +#define OSC_CXL_MEM_PER_CONTROL			0x00000001
>> +#define OSC_CXL_CONTROL_MASKS			0x00000001
>> +
>> +/* CXL Host Bridge _OSC: Capabilities DWORD 6: Query/Returns Field 
>> */
>> +#define OSC_CXL_MEM_EXPANDER_PER_CONTROL	0x00000001
>> +
>>  #define ACPI_GSB_ACCESS_ATTRIB_QUICK		0x00000002
>>  #define ACPI_GSB_ACCESS_ATTRIB_SEND_RCV         0x00000004
>>  #define ACPI_GSB_ACCESS_ATTRIB_BYTE		0x00000006
>> @@ -581,6 +597,8 @@ extern bool osc_pc_lpi_support_confirmed;
>>  #define ACPI_GSB_ACCESS_ATTRIB_RAW_BYTES	0x0000000E
>>  #define ACPI_GSB_ACCESS_ATTRIB_RAW_PROCESS	0x0000000F
>>
>> +extern acpi_status acpi_cxl_osc_control_set(acpi_handle handle,
>> +					     u32 *mask, u32 req);
>
> No need for this to be extern, right?  (I just sent a patch to make
> acpi_pci_osc_control_set() static also.)

Good, because I was wondering the same…

Thanks,

Sean

>
>>  extern acpi_status acpi_pci_osc_control_set(acpi_handle handle,
>>  					     u32 *mask, u32 req);
>>
>> -- 
>> 2.27.0
>>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-08 23:57 [RFC PATCH] ACPI: Add support for CXL_OSC Sean V Kelley
2020-07-10 21:53 ` Bjorn Helgaas
2020-07-12 21:52   ` Sean V Kelley

Linux-ACPI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-acpi/0 linux-acpi/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-acpi linux-acpi/ https://lore.kernel.org/linux-acpi \
		linux-acpi@vger.kernel.org
	public-inbox-index linux-acpi

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-acpi


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git