* [RFC PATCH 0/2] acpi: add support for CXL _OSC
@ 2022-03-17 0:27 Vishal Verma
2022-03-17 0:27 ` [RFC PATCH 1/2] PCI/ACPI: Use CXL _OSC instead of PCIe _OSC Vishal Verma
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Vishal Verma @ 2022-03-17 0:27 UTC (permalink / raw)
To: linux-cxl
Cc: linux-acpi, Jonathan Cameron, Dan Williams, Rafael J. Wysocki,
Robert Moore, Bjorn Helgaas, Vishal Verma
Add support for using the CXL definition of _OSC where applicable, and
negotiating CXL specific support and control bits.
Patch 1 adds the new CXL _OSC UUID, and uses it instead of the PCI UUID
when a root port is CXL enabled. It provides a fallback method for
CXL-1.1 devices that may not implement the CXL-2.0 _OSC.
Patch 2 performs negotiation for the CXL specific _OSC support and
control bits.
I've tested these against a custom qemu[1], which adds the CXL _OSC (in
addition to other CXL support). Specifically, _OSC support is added
here[2].
[1]: https://gitlab.com/jic23/qemu/-/tree/cxl-v7-draft-2-for-test
[2]: https://gitlab.com/jic23/qemu/-/commit/31c85054b84645dfbd9e9bb14aa35286141c14cf
Dan Williams (1):
PCI/ACPI: Use CXL _OSC instead of PCIe _OSC
Vishal Verma (1):
acpi/pci_root: negotiate CXL _OSC
include/linux/acpi.h | 11 +++
include/acpi/acpi_bus.h | 7 +-
drivers/acpi/pci_root.c | 201 ++++++++++++++++++++++++++++++++++------
3 files changed, 187 insertions(+), 32 deletions(-)
base-commit: 74be98774dfbc5b8b795db726bd772e735d2edd4
--
2.35.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [RFC PATCH 1/2] PCI/ACPI: Use CXL _OSC instead of PCIe _OSC
2022-03-17 0:27 [RFC PATCH 0/2] acpi: add support for CXL _OSC Vishal Verma
@ 2022-03-17 0:27 ` Vishal Verma
2022-03-17 1:47 ` Dan Williams
2022-03-17 0:27 ` [RFC PATCH 2/2] acpi/pci_root: negotiate CXL _OSC Vishal Verma
2022-03-17 15:19 ` [RFC PATCH 0/2] acpi: add support for " Jonathan Cameron
2 siblings, 1 reply; 14+ messages in thread
From: Vishal Verma @ 2022-03-17 0:27 UTC (permalink / raw)
To: linux-cxl
Cc: linux-acpi, Jonathan Cameron, Dan Williams, Rafael J. Wysocki,
Robert Moore, Bjorn Helgaas, Rafael J. Wysocki
From: Dan Williams <dan.j.williams@intel.com>
In preparation for negotiating OS control of CXL _OSC features, do the
minimal enabling to use CXL _OSC to handle the base PCIe feature
negotiation. Recall that CXL _OSC is a super-set of PCIe _OSC and the
CXL 2.0 specification mandates: "If a CXL Host Bridge device exposes CXL
_OSC, CXL aware OSPM shall evaluate CXL _OSC and not evaluate PCIe
_OSC."
A new ->cxl_osc_disable attribute is added for cases where platform
firmware publishes ACPI0016, but does not also publish CXL _OSC.
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Robert Moore <robert.moore@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
include/acpi/acpi_bus.h | 1 +
drivers/acpi/pci_root.c | 62 +++++++++++++++++++++++++++++++----------
2 files changed, 48 insertions(+), 15 deletions(-)
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index ca88c4706f2b..768ef1584055 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -585,6 +585,7 @@ struct acpi_pci_root {
struct acpi_device * device;
struct pci_bus *bus;
u16 segment;
+ bool cxl_osc_disable;
struct resource secondary; /* downstream bus range */
u32 osc_support_set; /* _OSC state of support bits */
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index b76db99cced3..2d834504096b 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -170,20 +170,47 @@ static void decode_osc_control(struct acpi_pci_root *root, char *msg, u32 word)
ARRAY_SIZE(pci_osc_control_bit));
}
-static u8 pci_osc_uuid_str[] = "33DB4D5B-1FF7-401C-9657-7441C03DD766";
+static bool is_pcie(struct acpi_pci_root *root)
+{
+ return strcmp(acpi_device_hid(root->device), "PNP0A08") == 0;
+}
-static acpi_status acpi_pci_run_osc(acpi_handle handle,
+static bool is_cxl(struct acpi_pci_root *root)
+{
+ if (root->cxl_osc_disable)
+ return false;
+ return strcmp(acpi_device_hid(root->device), "ACPI0016") == 0;
+}
+
+static u8 pci_osc_uuid_str[] = "33DB4D5B-1FF7-401C-9657-7441C03DD766";
+static u8 cxl_osc_uuid_str[] = "68F2D50B-C469-4d8A-BD3D-941A103FD3FC";
+
+static char *to_uuid(struct acpi_pci_root *root)
+{
+ if (is_cxl(root))
+ return cxl_osc_uuid_str;
+ return pci_osc_uuid_str;
+}
+
+static int cap_length(struct acpi_pci_root *root)
+{
+ if (is_cxl(root))
+ return sizeof(u32) * 6;
+ return sizeof(u32) * 3;
+}
+
+static acpi_status acpi_pci_run_osc(struct acpi_pci_root *root,
const u32 *capbuf, u32 *retval)
{
struct acpi_osc_context context = {
- .uuid_str = pci_osc_uuid_str,
+ .uuid_str = to_uuid(root),
.rev = 1,
- .cap.length = 12,
+ .cap.length = cap_length(root),
.cap.pointer = (void *)capbuf,
};
acpi_status status;
- status = acpi_run_osc(handle, &context);
+ status = acpi_run_osc(root->device->handle, &context);
if (ACPI_SUCCESS(status)) {
*retval = *((u32 *)(context.ret.pointer + 8));
kfree(context.ret.pointer);
@@ -196,7 +223,7 @@ static acpi_status acpi_pci_query_osc(struct acpi_pci_root *root,
u32 *control)
{
acpi_status status;
- u32 result, capbuf[3];
+ u32 result, capbuf[6];
support |= root->osc_support_set;
@@ -204,10 +231,18 @@ static acpi_status acpi_pci_query_osc(struct acpi_pci_root *root,
capbuf[OSC_SUPPORT_DWORD] = support;
capbuf[OSC_CONTROL_DWORD] = *control | root->osc_control_set;
- status = acpi_pci_run_osc(root->device->handle, capbuf, &result);
+retry:
+ status = acpi_pci_run_osc(root, capbuf, &result);
if (ACPI_SUCCESS(status)) {
root->osc_support_set = support;
*control = result;
+ } else if (is_cxl(root)) {
+ /*
+ * CXL _OSC is optional on CXL 1.1 hosts. Fall back to PCIe _OSC
+ * upon any failure using CXL _OSC.
+ */
+ root->cxl_osc_disable = true;
+ goto retry;
}
return status;
}
@@ -338,7 +373,7 @@ static acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask, u32 s
u32 req = OSC_PCI_EXPRESS_CAPABILITY_CONTROL;
struct acpi_pci_root *root;
acpi_status status;
- u32 ctrl, capbuf[3];
+ u32 ctrl, capbuf[6];
if (!mask)
return AE_BAD_PARAMETER;
@@ -375,7 +410,7 @@ static acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask, u32 s
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_pci_run_osc(root, capbuf, mask);
if (ACPI_FAILURE(status))
return status;
@@ -454,8 +489,7 @@ static bool os_control_query_checks(struct acpi_pci_root *root, u32 support)
return true;
}
-static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm,
- bool is_pcie)
+static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm)
{
u32 support, control = 0, requested = 0;
acpi_status status;
@@ -506,7 +540,7 @@ static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm,
*no_aspm = 1;
/* _OSC is optional for PCI host bridges */
- if ((status == AE_NOT_FOUND) && !is_pcie)
+ if ((status == AE_NOT_FOUND) && !is_pcie(root))
return;
if (control) {
@@ -529,7 +563,6 @@ 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;
root = kzalloc(sizeof(struct acpi_pci_root), GFP_KERNEL);
if (!root)
@@ -587,8 +620,7 @@ 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);
+ negotiate_os_control(root, &no_aspm);
/*
* TBD: Need PCI interface for enumeration/configuration of roots.
--
2.35.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [RFC PATCH 2/2] acpi/pci_root: negotiate CXL _OSC
2022-03-17 0:27 [RFC PATCH 0/2] acpi: add support for CXL _OSC Vishal Verma
2022-03-17 0:27 ` [RFC PATCH 1/2] PCI/ACPI: Use CXL _OSC instead of PCIe _OSC Vishal Verma
@ 2022-03-17 0:27 ` Vishal Verma
2022-03-17 3:19 ` Dan Williams
` (2 more replies)
2022-03-17 15:19 ` [RFC PATCH 0/2] acpi: add support for " Jonathan Cameron
2 siblings, 3 replies; 14+ messages in thread
From: Vishal Verma @ 2022-03-17 0:27 UTC (permalink / raw)
To: linux-cxl
Cc: linux-acpi, Jonathan Cameron, Dan Williams, Rafael J. Wysocki,
Robert Moore, Bjorn Helgaas, Vishal Verma, Rafael J. Wysocki
Add full support for negotiating _OSC as defined in the CXL 2.0 spec, as
applicable to CXL-enabled platforms. Advertise support for the CXL
features we support - 'CXL 2.0 port/device register access', 'Protocol
Error Reporting', and 'CL Native Hot Plug'. Request control for 'CXL
Memory Error Reporting'. The requests are dependent on CONFIG_* based
pre-requisites, and prior PCI enabling, similar to how the standard PCI
_OSC bits are determined.
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Robert Moore <robert.moore@intel.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
include/linux/acpi.h | 11 +++
include/acpi/acpi_bus.h | 6 +-
drivers/acpi/pci_root.c | 147 ++++++++++++++++++++++++++++++++++------
3 files changed, 143 insertions(+), 21 deletions(-)
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 6274758648e3..1717ccc265d7 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -554,6 +554,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 OSC_CXL_SUPPORT_DWORD 3 /* DWORD 4 */
+#define OSC_CXL_CONTROL_DWORD 4 /* DWORD 5 */
/* _OSC Capabilities DWORD 1: Query/Control and Error Returns (generic) */
#define OSC_QUERY_ENABLE 0x00000001 /* input */
@@ -607,6 +609,15 @@ extern u32 osc_sb_native_usb4_control;
#define OSC_PCI_EXPRESS_LTR_CONTROL 0x00000020
#define OSC_PCI_EXPRESS_DPC_CONTROL 0x00000080
+/* CXL _OSC: Capabilities DWORD 4: Support Field */
+#define OSC_CXL_1_1_PORT_REG_ACCESS_SUPPORT 0x00000001
+#define OSC_CXL_2_0_PORT_DEV_REG_ACCESS_SUPPORT 0x00000002
+#define OSC_CXL_PER_SUPPORT 0x00000004
+#define OSC_CXL_NATIVE_HP_SUPPORT 0x00000008
+
+/* CXL _OSC: Capabilities DWORD 5: Control Field */
+#define OSC_CXL_ERROR_REPORTING_CONTROL 0x00000001
+
#define ACPI_GSB_ACCESS_ATTRIB_QUICK 0x00000002
#define ACPI_GSB_ACCESS_ATTRIB_SEND_RCV 0x00000004
#define ACPI_GSB_ACCESS_ATTRIB_BYTE 0x00000006
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 768ef1584055..5776d4c1509a 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -588,8 +588,10 @@ struct acpi_pci_root {
bool cxl_osc_disable;
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/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 2d834504096b..c916318b11a0 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -142,6 +142,17 @@ static struct pci_osc_bit_struct pci_osc_control_bit[] = {
{ OSC_PCI_EXPRESS_DPC_CONTROL, "DPC" },
};
+static struct pci_osc_bit_struct cxl_osc_support_bit[] = {
+ { OSC_CXL_1_1_PORT_REG_ACCESS_SUPPORT, "CXL11PortRegAccess" },
+ { OSC_CXL_2_0_PORT_DEV_REG_ACCESS_SUPPORT, "CXL20PortDevRegAccess" },
+ { OSC_CXL_PER_SUPPORT, "CXLProtocolErrorReporting" },
+ { OSC_CXL_NATIVE_HP_SUPPORT, "CXLNativeHotPlug" },
+};
+
+static struct pci_osc_bit_struct cxl_osc_control_bit[] = {
+ { OSC_CXL_ERROR_REPORTING_CONTROL, "CXLMemErrorReporting" },
+};
+
static void decode_osc_bits(struct acpi_pci_root *root, char *msg, u32 word,
struct pci_osc_bit_struct *table, int size)
{
@@ -170,6 +181,18 @@ static void decode_osc_control(struct acpi_pci_root *root, char *msg, u32 word)
ARRAY_SIZE(pci_osc_control_bit));
}
+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 bool is_pcie(struct acpi_pci_root *root)
{
return strcmp(acpi_device_hid(root->device), "PNP0A08") == 0;
@@ -199,8 +222,19 @@ static int cap_length(struct acpi_pci_root *root)
return sizeof(u32) * 3;
}
+static u32 acpi_osc_ctx_get_pci_control(struct acpi_osc_context *context)
+{
+ return *((u32 *)(context->ret.pointer + 8));
+}
+
+static u32 acpi_osc_ctx_get_cxl_control(struct acpi_osc_context *context)
+{
+ return *((u32 *)(context->ret.pointer + 16));
+}
+
static acpi_status acpi_pci_run_osc(struct acpi_pci_root *root,
- const u32 *capbuf, u32 *retval)
+ const u32 *capbuf, u32 *pci_control,
+ u32 *cxl_control)
{
struct acpi_osc_context context = {
.uuid_str = to_uuid(root),
@@ -212,18 +246,20 @@ static acpi_status acpi_pci_run_osc(struct acpi_pci_root *root,
status = acpi_run_osc(root->device->handle, &context);
if (ACPI_SUCCESS(status)) {
- *retval = *((u32 *)(context.ret.pointer + 8));
+ *pci_control = acpi_osc_ctx_get_pci_control(&context);
+ if (is_cxl(root))
+ *cxl_control = acpi_osc_ctx_get_cxl_control(&context);
kfree(context.ret.pointer);
}
return status;
}
-static acpi_status acpi_pci_query_osc(struct acpi_pci_root *root,
- u32 support,
- u32 *control)
+static acpi_status acpi_pci_query_osc(struct acpi_pci_root *root, u32 support,
+ u32 *control, u32 cxl_support,
+ u32 *cxl_control)
{
acpi_status status;
- u32 result, capbuf[6];
+ u32 pci_result, cxl_result, capbuf[8];
support |= root->osc_support_set;
@@ -231,11 +267,21 @@ static acpi_status acpi_pci_query_osc(struct acpi_pci_root *root,
capbuf[OSC_SUPPORT_DWORD] = support;
capbuf[OSC_CONTROL_DWORD] = *control | root->osc_control_set;
+ if (is_cxl(root)) {
+ cxl_support |= root->cxl_osc_support_set;
+ capbuf[OSC_CXL_SUPPORT_DWORD] = cxl_support;
+ capbuf[OSC_CXL_CONTROL_DWORD] = *cxl_control | root->cxl_osc_control_set;
+ }
+
retry:
- status = acpi_pci_run_osc(root, capbuf, &result);
+ status = acpi_pci_run_osc(root, capbuf, &pci_result, &cxl_result);
if (ACPI_SUCCESS(status)) {
root->osc_support_set = support;
- *control = result;
+ *control = pci_result;
+ if (is_cxl(root)) {
+ root->cxl_osc_support_set = cxl_support;
+ *cxl_control = cxl_result;
+ }
} else if (is_cxl(root)) {
/*
* CXL _OSC is optional on CXL 1.1 hosts. Fall back to PCIe _OSC
@@ -358,6 +404,8 @@ EXPORT_SYMBOL_GPL(acpi_get_pci_dev);
* @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.
* @support: _OSC supported capability.
+ * @cxl_mask: Mask of CXL _OSC control bits, place to store control mask.
+ * @cxl_support: CXL _OSC supported capability.
*
* Run _OSC query for @mask and if that is successful, compare the returned
* mask of control bits with @req. If all of the @req bits are set in the
@@ -368,12 +416,14 @@ EXPORT_SYMBOL_GPL(acpi_get_pci_dev);
* _OSC bits the BIOS has granted control of, but its contents are meaningless
* on failure.
**/
-static acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask, u32 support)
+static acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask,
+ u32 support, u32 *cxl_mask,
+ u32 cxl_support)
{
u32 req = OSC_PCI_EXPRESS_CAPABILITY_CONTROL;
struct acpi_pci_root *root;
acpi_status status;
- u32 ctrl, capbuf[6];
+ u32 ctrl, cxl_ctrl, capbuf[8];
if (!mask)
return AE_BAD_PARAMETER;
@@ -385,20 +435,35 @@ static acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask, u32 s
ctrl = *mask;
*mask |= root->osc_control_set;
+ if (is_cxl(root)) {
+ cxl_ctrl = *cxl_mask;
+ *mask |= root->osc_control_set;
+ }
+
/* Need to check the available controls bits before requesting them. */
do {
- status = acpi_pci_query_osc(root, support, mask);
+ status = acpi_pci_query_osc(root, support, mask, cxl_support,
+ cxl_mask);
if (ACPI_FAILURE(status))
return status;
- if (ctrl == *mask)
- break;
- decode_osc_control(root, "platform does not support",
- ctrl & ~(*mask));
+ if (is_cxl(root)) {
+ if ((ctrl == *mask) && (cxl_ctrl == *cxl_mask))
+ break;
+ decode_cxl_osc_control(root, "platform does not support",
+ cxl_ctrl & ~(*cxl_mask));
+ } else {
+ if (ctrl == *mask)
+ break;
+ decode_osc_control(root, "platform does not support",
+ ctrl & ~(*mask));
+ }
ctrl = *mask;
- } while (*mask);
+ cxl_ctrl = *cxl_mask;
+ } while (*mask || *cxl_mask);
/* No need to request _OSC if the control was already granted. */
- if ((root->osc_control_set & ctrl) == ctrl)
+ if (((root->osc_control_set & ctrl) == ctrl) &&
+ ((root->cxl_osc_control_set & cxl_ctrl) == cxl_ctrl))
return AE_OK;
if ((ctrl & req) != req) {
@@ -410,11 +475,17 @@ static acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask, u32 s
capbuf[OSC_QUERY_DWORD] = 0;
capbuf[OSC_SUPPORT_DWORD] = root->osc_support_set;
capbuf[OSC_CONTROL_DWORD] = ctrl;
- status = acpi_pci_run_osc(root, capbuf, mask);
+ if (is_cxl(root)) {
+ capbuf[OSC_CXL_SUPPORT_DWORD] = root->cxl_osc_support_set;
+ capbuf[OSC_CXL_CONTROL_DWORD] = cxl_ctrl;
+ }
+
+ status = acpi_pci_run_osc(root, capbuf, mask, cxl_mask);
if (ACPI_FAILURE(status))
return status;
root->osc_control_set = *mask;
+ root->cxl_osc_control_set = *cxl_mask;
return AE_OK;
}
@@ -440,6 +511,18 @@ static u32 calculate_support(void)
return support;
}
+static u32 calculate_cxl_support(void)
+{
+ u32 support;
+
+ support = OSC_CXL_2_0_PORT_DEV_REG_ACCESS_SUPPORT;
+ support |= OSC_CXL_PER_SUPPORT;
+ if (IS_ENABLED(CONFIG_HOTPLUG_PCI_PCIE))
+ support |= OSC_CXL_NATIVE_HP_SUPPORT;
+
+ return support;
+}
+
static u32 calculate_control(void)
{
u32 control;
@@ -471,6 +554,16 @@ static u32 calculate_control(void)
return control;
}
+static u32 calculate_cxl_control(void)
+{
+ u32 control;
+
+ if (pci_aer_available())
+ control |= OSC_CXL_ERROR_REPORTING_CONTROL;
+
+ return control;
+}
+
static bool os_control_query_checks(struct acpi_pci_root *root, u32 support)
{
struct acpi_device *device = root->device;
@@ -492,6 +585,7 @@ static bool os_control_query_checks(struct acpi_pci_root *root, u32 support)
static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm)
{
u32 support, control = 0, requested = 0;
+ u32 cxl_support, cxl_control = 0, cxl_requested = 0;
acpi_status status;
struct acpi_device *device = root->device;
acpi_handle handle = device->handle;
@@ -515,10 +609,20 @@ static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm)
if (os_control_query_checks(root, support))
requested = control = calculate_control();
- status = acpi_pci_osc_control_set(handle, &control, support);
+ if (is_cxl(root)) {
+ cxl_support = calculate_cxl_support();
+ decode_cxl_osc_support(root, "OS supports", cxl_support);
+ cxl_requested = cxl_control = calculate_cxl_control();
+ }
+
+ status = acpi_pci_osc_control_set(handle, &control, support,
+ &cxl_control, cxl_support);
if (ACPI_SUCCESS(status)) {
if (control)
decode_osc_control(root, "OS now controls", control);
+ if (cxl_control)
+ decode_cxl_osc_control(root, "OS now controls",
+ cxl_control);
if (acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_ASPM) {
/*
@@ -547,6 +651,11 @@ static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm)
decode_osc_control(root, "OS requested", requested);
decode_osc_control(root, "platform willing to grant", control);
}
+ if (cxl_control) {
+ decode_cxl_osc_control(root, "OS requested", cxl_requested);
+ decode_cxl_osc_control(root, "platform willing to grant",
+ cxl_control);
+ }
dev_info(&device->dev, "_OSC: platform retains control of PCIe features (%s)\n",
acpi_format_exception(status));
--
2.35.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [RFC PATCH 1/2] PCI/ACPI: Use CXL _OSC instead of PCIe _OSC
2022-03-17 0:27 ` [RFC PATCH 1/2] PCI/ACPI: Use CXL _OSC instead of PCIe _OSC Vishal Verma
@ 2022-03-17 1:47 ` Dan Williams
2022-03-17 15:40 ` Jonathan Cameron
0 siblings, 1 reply; 14+ messages in thread
From: Dan Williams @ 2022-03-17 1:47 UTC (permalink / raw)
To: Vishal Verma
Cc: linux-cxl, Linux ACPI, Jonathan Cameron, Rafael J. Wysocki,
Robert Moore, Bjorn Helgaas, Rafael J. Wysocki
On Wed, Mar 16, 2022 at 5:27 PM Vishal Verma <vishal.l.verma@intel.com> wrote:
>
> From: Dan Williams <dan.j.williams@intel.com>
>
> In preparation for negotiating OS control of CXL _OSC features, do the
> minimal enabling to use CXL _OSC to handle the base PCIe feature
> negotiation. Recall that CXL _OSC is a super-set of PCIe _OSC and the
> CXL 2.0 specification mandates: "If a CXL Host Bridge device exposes CXL
> _OSC, CXL aware OSPM shall evaluate CXL _OSC and not evaluate PCIe
> _OSC."
>
> A new ->cxl_osc_disable attribute is added for cases where platform
> firmware publishes ACPI0016, but does not also publish CXL _OSC.
It's been a couple weeks since I wrote this... looking at it now I
would rewrite this to:
Rather than pass a boolean flag alongside @root to all the helper
functions that need to consider PCIe specifics, add is_pcie() and
is_cxl() helper functions to check the flavor of @root. This also
allows for dynamic fallback to PCIe _OSC in cases where an attempt to
use CXL _OXC fails. This can happen on CXL 1.1 platforms that publish
ACPI0016 devices to indicate CXL host bridges, but do not publish the
optional CXL _OSC method. CXL _OSC is mandatory for CXL 2.0 hosts.
>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Robert Moore <robert.moore@intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Always include your own sign-off when forwarding a patch.
> ---
> include/acpi/acpi_bus.h | 1 +
> drivers/acpi/pci_root.c | 62 +++++++++++++++++++++++++++++++----------
> 2 files changed, 48 insertions(+), 15 deletions(-)
>
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index ca88c4706f2b..768ef1584055 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -585,6 +585,7 @@ struct acpi_pci_root {
> struct acpi_device * device;
> struct pci_bus *bus;
> u16 segment;
> + bool cxl_osc_disable;
> struct resource secondary; /* downstream bus range */
>
> u32 osc_support_set; /* _OSC state of support bits */
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index b76db99cced3..2d834504096b 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -170,20 +170,47 @@ static void decode_osc_control(struct acpi_pci_root *root, char *msg, u32 word)
> ARRAY_SIZE(pci_osc_control_bit));
> }
>
> -static u8 pci_osc_uuid_str[] = "33DB4D5B-1FF7-401C-9657-7441C03DD766";
> +static bool is_pcie(struct acpi_pci_root *root)
> +{
> + return strcmp(acpi_device_hid(root->device), "PNP0A08") == 0;
> +}
>
> -static acpi_status acpi_pci_run_osc(acpi_handle handle,
> +static bool is_cxl(struct acpi_pci_root *root)
> +{
> + if (root->cxl_osc_disable)
> + return false;
> + return strcmp(acpi_device_hid(root->device), "ACPI0016") == 0;
> +}
> +
> +static u8 pci_osc_uuid_str[] = "33DB4D5B-1FF7-401C-9657-7441C03DD766";
> +static u8 cxl_osc_uuid_str[] = "68F2D50B-C469-4d8A-BD3D-941A103FD3FC";
> +
> +static char *to_uuid(struct acpi_pci_root *root)
> +{
> + if (is_cxl(root))
> + return cxl_osc_uuid_str;
> + return pci_osc_uuid_str;
> +}
> +
> +static int cap_length(struct acpi_pci_root *root)
> +{
> + if (is_cxl(root))
> + return sizeof(u32) * 6;
> + return sizeof(u32) * 3;
> +}
> +
> +static acpi_status acpi_pci_run_osc(struct acpi_pci_root *root,
> const u32 *capbuf, u32 *retval)
> {
> struct acpi_osc_context context = {
> - .uuid_str = pci_osc_uuid_str,
> + .uuid_str = to_uuid(root),
> .rev = 1,
> - .cap.length = 12,
> + .cap.length = cap_length(root),
> .cap.pointer = (void *)capbuf,
> };
> acpi_status status;
>
> - status = acpi_run_osc(handle, &context);
> + status = acpi_run_osc(root->device->handle, &context);
> if (ACPI_SUCCESS(status)) {
> *retval = *((u32 *)(context.ret.pointer + 8));
> kfree(context.ret.pointer);
> @@ -196,7 +223,7 @@ static acpi_status acpi_pci_query_osc(struct acpi_pci_root *root,
> u32 *control)
> {
> acpi_status status;
> - u32 result, capbuf[3];
> + u32 result, capbuf[6];
>
> support |= root->osc_support_set;
>
> @@ -204,10 +231,18 @@ static acpi_status acpi_pci_query_osc(struct acpi_pci_root *root,
> capbuf[OSC_SUPPORT_DWORD] = support;
> capbuf[OSC_CONTROL_DWORD] = *control | root->osc_control_set;
>
> - status = acpi_pci_run_osc(root->device->handle, capbuf, &result);
> +retry:
> + status = acpi_pci_run_osc(root, capbuf, &result);
> if (ACPI_SUCCESS(status)) {
> root->osc_support_set = support;
> *control = result;
> + } else if (is_cxl(root)) {
> + /*
> + * CXL _OSC is optional on CXL 1.1 hosts. Fall back to PCIe _OSC
> + * upon any failure using CXL _OSC.
> + */
> + root->cxl_osc_disable = true;
> + goto retry;
> }
> return status;
> }
> @@ -338,7 +373,7 @@ static acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask, u32 s
> u32 req = OSC_PCI_EXPRESS_CAPABILITY_CONTROL;
> struct acpi_pci_root *root;
> acpi_status status;
> - u32 ctrl, capbuf[3];
> + u32 ctrl, capbuf[6];
>
> if (!mask)
> return AE_BAD_PARAMETER;
> @@ -375,7 +410,7 @@ static acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask, u32 s
> 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_pci_run_osc(root, capbuf, mask);
> if (ACPI_FAILURE(status))
> return status;
>
> @@ -454,8 +489,7 @@ static bool os_control_query_checks(struct acpi_pci_root *root, u32 support)
> return true;
> }
>
> -static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm,
> - bool is_pcie)
> +static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm)
> {
> u32 support, control = 0, requested = 0;
> acpi_status status;
> @@ -506,7 +540,7 @@ static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm,
> *no_aspm = 1;
>
> /* _OSC is optional for PCI host bridges */
> - if ((status == AE_NOT_FOUND) && !is_pcie)
> + if ((status == AE_NOT_FOUND) && !is_pcie(root))
> return;
>
> if (control) {
> @@ -529,7 +563,6 @@ 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;
>
> root = kzalloc(sizeof(struct acpi_pci_root), GFP_KERNEL);
> if (!root)
> @@ -587,8 +620,7 @@ 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);
> + negotiate_os_control(root, &no_aspm);
>
> /*
> * TBD: Need PCI interface for enumeration/configuration of roots.
> --
> 2.35.1
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH 2/2] acpi/pci_root: negotiate CXL _OSC
2022-03-17 0:27 ` [RFC PATCH 2/2] acpi/pci_root: negotiate CXL _OSC Vishal Verma
@ 2022-03-17 3:19 ` Dan Williams
2022-03-17 3:49 ` Verma, Vishal L
2022-03-17 3:25 ` kernel test robot
2022-03-17 16:10 ` Jonathan Cameron
2 siblings, 1 reply; 14+ messages in thread
From: Dan Williams @ 2022-03-17 3:19 UTC (permalink / raw)
To: Vishal Verma
Cc: linux-cxl, Linux ACPI, Jonathan Cameron, Rafael J. Wysocki,
Robert Moore, Bjorn Helgaas, Rafael J. Wysocki
On Wed, Mar 16, 2022 at 5:27 PM Vishal Verma <vishal.l.verma@intel.com> wrote:
>
> Add full support for negotiating _OSC as defined in the CXL 2.0 spec, as
> applicable to CXL-enabled platforms. Advertise support for the CXL
> features we support - 'CXL 2.0 port/device register access', 'Protocol
> Error Reporting', and 'CL Native Hot Plug'. Request control for 'CXL
> Memory Error Reporting'. The requests are dependent on CONFIG_* based
> pre-requisites, and prior PCI enabling, similar to how the standard PCI
> _OSC bits are determined.
Might want to add a clarification here of why this just reflects the
PCIe support into the similar CXL fields. For example:
The CXL specification does not define any additional constraints on
the hotplug flow beyond PCIe native hotplug, so a kernel that supports
native PCIe hotplug, supports CXL hotplug. For error handling protocol
and link errors just use PCIe AER. There is nascent support for
amending AER events with CXL specific status [1], but there's
otherwise no additional OS responsibility for CXL errors beyond PCIe
AER. CXL Memory Errors behave the same as typical memory errors so
CONFIG_MEMORY_FAILURE is sufficient to indicate support to platform
firmware.
[1]: https://lore.kernel.org/linux-cxl/164740402242.3912056.8303625392871313860.stgit@dwillia2-desk3.amr.corp.intel.com/
>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Robert Moore <robert.moore@intel.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> ---
> include/linux/acpi.h | 11 +++
> include/acpi/acpi_bus.h | 6 +-
> drivers/acpi/pci_root.c | 147 ++++++++++++++++++++++++++++++++++------
> 3 files changed, 143 insertions(+), 21 deletions(-)
>
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 6274758648e3..1717ccc265d7 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -554,6 +554,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 OSC_CXL_SUPPORT_DWORD 3 /* DWORD 4 */
> +#define OSC_CXL_CONTROL_DWORD 4 /* DWORD 5 */
>
> /* _OSC Capabilities DWORD 1: Query/Control and Error Returns (generic) */
> #define OSC_QUERY_ENABLE 0x00000001 /* input */
> @@ -607,6 +609,15 @@ extern u32 osc_sb_native_usb4_control;
> #define OSC_PCI_EXPRESS_LTR_CONTROL 0x00000020
> #define OSC_PCI_EXPRESS_DPC_CONTROL 0x00000080
>
> +/* CXL _OSC: Capabilities DWORD 4: Support Field */
> +#define OSC_CXL_1_1_PORT_REG_ACCESS_SUPPORT 0x00000001
> +#define OSC_CXL_2_0_PORT_DEV_REG_ACCESS_SUPPORT 0x00000002
> +#define OSC_CXL_PER_SUPPORT 0x00000004
> +#define OSC_CXL_NATIVE_HP_SUPPORT 0x00000008
> +
> +/* CXL _OSC: Capabilities DWORD 5: Control Field */
> +#define OSC_CXL_ERROR_REPORTING_CONTROL 0x00000001
> +
> #define ACPI_GSB_ACCESS_ATTRIB_QUICK 0x00000002
> #define ACPI_GSB_ACCESS_ATTRIB_SEND_RCV 0x00000004
> #define ACPI_GSB_ACCESS_ATTRIB_BYTE 0x00000006
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index 768ef1584055..5776d4c1509a 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -588,8 +588,10 @@ struct acpi_pci_root {
> bool cxl_osc_disable;
> 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/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index 2d834504096b..c916318b11a0 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -142,6 +142,17 @@ static struct pci_osc_bit_struct pci_osc_control_bit[] = {
> { OSC_PCI_EXPRESS_DPC_CONTROL, "DPC" },
> };
>
> +static struct pci_osc_bit_struct cxl_osc_support_bit[] = {
> + { OSC_CXL_1_1_PORT_REG_ACCESS_SUPPORT, "CXL11PortRegAccess" },
> + { OSC_CXL_2_0_PORT_DEV_REG_ACCESS_SUPPORT, "CXL20PortDevRegAccess" },
> + { OSC_CXL_PER_SUPPORT, "CXLProtocolErrorReporting" },
> + { OSC_CXL_NATIVE_HP_SUPPORT, "CXLNativeHotPlug" },
> +};
> +
> +static struct pci_osc_bit_struct cxl_osc_control_bit[] = {
> + { OSC_CXL_ERROR_REPORTING_CONTROL, "CXLMemErrorReporting" },
> +};
> +
> static void decode_osc_bits(struct acpi_pci_root *root, char *msg, u32 word,
> struct pci_osc_bit_struct *table, int size)
> {
> @@ -170,6 +181,18 @@ static void decode_osc_control(struct acpi_pci_root *root, char *msg, u32 word)
> ARRAY_SIZE(pci_osc_control_bit));
> }
>
> +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 bool is_pcie(struct acpi_pci_root *root)
> {
> return strcmp(acpi_device_hid(root->device), "PNP0A08") == 0;
> @@ -199,8 +222,19 @@ static int cap_length(struct acpi_pci_root *root)
> return sizeof(u32) * 3;
> }
>
> +static u32 acpi_osc_ctx_get_pci_control(struct acpi_osc_context *context)
> +{
> + return *((u32 *)(context->ret.pointer + 8));
> +}
> +
> +static u32 acpi_osc_ctx_get_cxl_control(struct acpi_osc_context *context)
> +{
> + return *((u32 *)(context->ret.pointer + 16));
> +}
> +
> static acpi_status acpi_pci_run_osc(struct acpi_pci_root *root,
> - const u32 *capbuf, u32 *retval)
> + const u32 *capbuf, u32 *pci_control,
> + u32 *cxl_control)
> {
> struct acpi_osc_context context = {
> .uuid_str = to_uuid(root),
> @@ -212,18 +246,20 @@ static acpi_status acpi_pci_run_osc(struct acpi_pci_root *root,
>
> status = acpi_run_osc(root->device->handle, &context);
> if (ACPI_SUCCESS(status)) {
> - *retval = *((u32 *)(context.ret.pointer + 8));
> + *pci_control = acpi_osc_ctx_get_pci_control(&context);
> + if (is_cxl(root))
> + *cxl_control = acpi_osc_ctx_get_cxl_control(&context);
> kfree(context.ret.pointer);
> }
> return status;
> }
>
> -static acpi_status acpi_pci_query_osc(struct acpi_pci_root *root,
> - u32 support,
> - u32 *control)
> +static acpi_status acpi_pci_query_osc(struct acpi_pci_root *root, u32 support,
> + u32 *control, u32 cxl_support,
> + u32 *cxl_control)
> {
> acpi_status status;
> - u32 result, capbuf[6];
> + u32 pci_result, cxl_result, capbuf[8];
>
> support |= root->osc_support_set;
>
> @@ -231,11 +267,21 @@ static acpi_status acpi_pci_query_osc(struct acpi_pci_root *root,
> capbuf[OSC_SUPPORT_DWORD] = support;
> capbuf[OSC_CONTROL_DWORD] = *control | root->osc_control_set;
>
> + if (is_cxl(root)) {
> + cxl_support |= root->cxl_osc_support_set;
> + capbuf[OSC_CXL_SUPPORT_DWORD] = cxl_support;
> + capbuf[OSC_CXL_CONTROL_DWORD] = *cxl_control | root->cxl_osc_control_set;
> + }
> +
> retry:
> - status = acpi_pci_run_osc(root, capbuf, &result);
> + status = acpi_pci_run_osc(root, capbuf, &pci_result, &cxl_result);
> if (ACPI_SUCCESS(status)) {
> root->osc_support_set = support;
> - *control = result;
> + *control = pci_result;
> + if (is_cxl(root)) {
> + root->cxl_osc_support_set = cxl_support;
> + *cxl_control = cxl_result;
> + }
> } else if (is_cxl(root)) {
> /*
> * CXL _OSC is optional on CXL 1.1 hosts. Fall back to PCIe _OSC
> @@ -358,6 +404,8 @@ EXPORT_SYMBOL_GPL(acpi_get_pci_dev);
> * @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.
> * @support: _OSC supported capability.
> + * @cxl_mask: Mask of CXL _OSC control bits, place to store control mask.
> + * @cxl_support: CXL _OSC supported capability.
> *
> * Run _OSC query for @mask and if that is successful, compare the returned
> * mask of control bits with @req. If all of the @req bits are set in the
> @@ -368,12 +416,14 @@ EXPORT_SYMBOL_GPL(acpi_get_pci_dev);
> * _OSC bits the BIOS has granted control of, but its contents are meaningless
> * on failure.
> **/
> -static acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask, u32 support)
> +static acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask,
> + u32 support, u32 *cxl_mask,
> + u32 cxl_support)
> {
> u32 req = OSC_PCI_EXPRESS_CAPABILITY_CONTROL;
> struct acpi_pci_root *root;
> acpi_status status;
> - u32 ctrl, capbuf[6];
> + u32 ctrl, cxl_ctrl, capbuf[8];
>
> if (!mask)
> return AE_BAD_PARAMETER;
> @@ -385,20 +435,35 @@ static acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask, u32 s
> ctrl = *mask;
> *mask |= root->osc_control_set;
>
> + if (is_cxl(root)) {
> + cxl_ctrl = *cxl_mask;
> + *mask |= root->osc_control_set;
> + }
> +
> /* Need to check the available controls bits before requesting them. */
> do {
> - status = acpi_pci_query_osc(root, support, mask);
> + status = acpi_pci_query_osc(root, support, mask, cxl_support,
> + cxl_mask);
> if (ACPI_FAILURE(status))
> return status;
> - if (ctrl == *mask)
> - break;
> - decode_osc_control(root, "platform does not support",
> - ctrl & ~(*mask));
> + if (is_cxl(root)) {
> + if ((ctrl == *mask) && (cxl_ctrl == *cxl_mask))
> + break;
> + decode_cxl_osc_control(root, "platform does not support",
> + cxl_ctrl & ~(*cxl_mask));
> + } else {
> + if (ctrl == *mask)
> + break;
> + decode_osc_control(root, "platform does not support",
> + ctrl & ~(*mask));
> + }
> ctrl = *mask;
> - } while (*mask);
> + cxl_ctrl = *cxl_mask;
> + } while (*mask || *cxl_mask);
>
> /* No need to request _OSC if the control was already granted. */
> - if ((root->osc_control_set & ctrl) == ctrl)
> + if (((root->osc_control_set & ctrl) == ctrl) &&
> + ((root->cxl_osc_control_set & cxl_ctrl) == cxl_ctrl))
> return AE_OK;
>
> if ((ctrl & req) != req) {
> @@ -410,11 +475,17 @@ static acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask, u32 s
> capbuf[OSC_QUERY_DWORD] = 0;
> capbuf[OSC_SUPPORT_DWORD] = root->osc_support_set;
> capbuf[OSC_CONTROL_DWORD] = ctrl;
> - status = acpi_pci_run_osc(root, capbuf, mask);
> + if (is_cxl(root)) {
> + capbuf[OSC_CXL_SUPPORT_DWORD] = root->cxl_osc_support_set;
> + capbuf[OSC_CXL_CONTROL_DWORD] = cxl_ctrl;
> + }
> +
> + status = acpi_pci_run_osc(root, capbuf, mask, cxl_mask);
> if (ACPI_FAILURE(status))
> return status;
>
> root->osc_control_set = *mask;
> + root->cxl_osc_control_set = *cxl_mask;
> return AE_OK;
> }
>
> @@ -440,6 +511,18 @@ static u32 calculate_support(void)
> return support;
> }
>
> +static u32 calculate_cxl_support(void)
> +{
> + u32 support;
> +
> + support = OSC_CXL_2_0_PORT_DEV_REG_ACCESS_SUPPORT;
> + support |= OSC_CXL_PER_SUPPORT;
I would expect this one to be gated by pci_aer_available() since these
errors are reported by PCIe AER.
Perhaps also s/PER/PORT_ERROR/? I keep reading PER like 'per' as in 'per-cpu'.
> + if (IS_ENABLED(CONFIG_HOTPLUG_PCI_PCIE))
> + support |= OSC_CXL_NATIVE_HP_SUPPORT;
> +
> + return support;
> +}
> +
> static u32 calculate_control(void)
> {
> u32 control;
> @@ -471,6 +554,16 @@ static u32 calculate_control(void)
> return control;
> }
>
> +static u32 calculate_cxl_control(void)
> +{
> + u32 control;
> +
> + if (pci_aer_available())
> + control |= OSC_CXL_ERROR_REPORTING_CONTROL;
In this case the error handling is for memory errors, so this one should be:
if (IS_ENABLED(CONFIG_MEMORY_FAILURE))
control |= OSC_CXL_ERROR_REPORTING_CONTROL;
...other than that looks good to me.
> +
> + return control;
> +}
> +
> static bool os_control_query_checks(struct acpi_pci_root *root, u32 support)
> {
> struct acpi_device *device = root->device;
> @@ -492,6 +585,7 @@ static bool os_control_query_checks(struct acpi_pci_root *root, u32 support)
> static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm)
> {
> u32 support, control = 0, requested = 0;
> + u32 cxl_support, cxl_control = 0, cxl_requested = 0;
> acpi_status status;
> struct acpi_device *device = root->device;
> acpi_handle handle = device->handle;
> @@ -515,10 +609,20 @@ static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm)
> if (os_control_query_checks(root, support))
> requested = control = calculate_control();
>
> - status = acpi_pci_osc_control_set(handle, &control, support);
> + if (is_cxl(root)) {
> + cxl_support = calculate_cxl_support();
> + decode_cxl_osc_support(root, "OS supports", cxl_support);
> + cxl_requested = cxl_control = calculate_cxl_control();
> + }
> +
> + status = acpi_pci_osc_control_set(handle, &control, support,
> + &cxl_control, cxl_support);
> if (ACPI_SUCCESS(status)) {
> if (control)
> decode_osc_control(root, "OS now controls", control);
> + if (cxl_control)
> + decode_cxl_osc_control(root, "OS now controls",
> + cxl_control);
>
> if (acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_ASPM) {
> /*
> @@ -547,6 +651,11 @@ static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm)
> decode_osc_control(root, "OS requested", requested);
> decode_osc_control(root, "platform willing to grant", control);
> }
> + if (cxl_control) {
> + decode_cxl_osc_control(root, "OS requested", cxl_requested);
> + decode_cxl_osc_control(root, "platform willing to grant",
> + cxl_control);
> + }
>
> dev_info(&device->dev, "_OSC: platform retains control of PCIe features (%s)\n",
> acpi_format_exception(status));
> --
> 2.35.1
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH 2/2] acpi/pci_root: negotiate CXL _OSC
2022-03-17 0:27 ` [RFC PATCH 2/2] acpi/pci_root: negotiate CXL _OSC Vishal Verma
2022-03-17 3:19 ` Dan Williams
@ 2022-03-17 3:25 ` kernel test robot
2022-03-17 16:10 ` Jonathan Cameron
2 siblings, 0 replies; 14+ messages in thread
From: kernel test robot @ 2022-03-17 3:25 UTC (permalink / raw)
To: Vishal Verma; +Cc: llvm, kbuild-all
Hi Vishal,
[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on 74be98774dfbc5b8b795db726bd772e735d2edd4]
url: https://github.com/0day-ci/linux/commits/Vishal-Verma/acpi-add-support-for-CXL-_OSC/20220317-082840
base: 74be98774dfbc5b8b795db726bd772e735d2edd4
config: x86_64-randconfig-a001 (https://download.01.org/0day-ci/archive/20220317/202203171123.Jy0945To-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project a6ec1e3d798f8eab43fb3a91028c6ab04e115fcb)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/8020d862c22d52b61a590343b2202e5d05332100
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Vishal-Verma/acpi-add-support-for-CXL-_OSC/20220317-082840
git checkout 8020d862c22d52b61a590343b2202e5d05332100
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/acpi/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/acpi/pci_root.c:562:3: warning: variable 'control' is uninitialized when used here [-Wuninitialized]
control |= OSC_CXL_ERROR_REPORTING_CONTROL;
^~~~~~~
drivers/acpi/pci_root.c:559:13: note: initialize the variable 'control' to silence this warning
u32 control;
^
= 0
>> drivers/acpi/pci_root.c:612:6: warning: variable 'cxl_support' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
if (is_cxl(root)) {
^~~~~~~~~~~~
drivers/acpi/pci_root.c:619:22: note: uninitialized use occurs here
&cxl_control, cxl_support);
^~~~~~~~~~~
drivers/acpi/pci_root.c:612:2: note: remove the 'if' if its condition is always true
if (is_cxl(root)) {
^~~~~~~~~~~~~~~~~~
drivers/acpi/pci_root.c:588:17: note: initialize the variable 'cxl_support' to silence this warning
u32 cxl_support, cxl_control = 0, cxl_requested = 0;
^
= 0
2 warnings generated.
vim +/control +562 drivers/acpi/pci_root.c
556
557 static u32 calculate_cxl_control(void)
558 {
559 u32 control;
560
561 if (pci_aer_available())
> 562 control |= OSC_CXL_ERROR_REPORTING_CONTROL;
563
564 return control;
565 }
566
567 static bool os_control_query_checks(struct acpi_pci_root *root, u32 support)
568 {
569 struct acpi_device *device = root->device;
570
571 if (pcie_ports_disabled) {
572 dev_info(&device->dev, "PCIe port services disabled; not requesting _OSC control\n");
573 return false;
574 }
575
576 if ((support & ACPI_PCIE_REQ_SUPPORT) != ACPI_PCIE_REQ_SUPPORT) {
577 decode_osc_support(root, "not requesting OS control; OS requires",
578 ACPI_PCIE_REQ_SUPPORT);
579 return false;
580 }
581
582 return true;
583 }
584
585 static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm)
586 {
587 u32 support, control = 0, requested = 0;
588 u32 cxl_support, cxl_control = 0, cxl_requested = 0;
589 acpi_status status;
590 struct acpi_device *device = root->device;
591 acpi_handle handle = device->handle;
592
593 /*
594 * Apple always return failure on _OSC calls when _OSI("Darwin") has
595 * been called successfully. We know the feature set supported by the
596 * platform, so avoid calling _OSC at all
597 */
598 if (x86_apple_machine) {
599 root->osc_control_set = ~OSC_PCI_EXPRESS_PME_CONTROL;
600 decode_osc_control(root, "OS assumes control of",
601 root->osc_control_set);
602 return;
603 }
604
605 support = calculate_support();
606
607 decode_osc_support(root, "OS supports", support);
608
609 if (os_control_query_checks(root, support))
610 requested = control = calculate_control();
611
> 612 if (is_cxl(root)) {
613 cxl_support = calculate_cxl_support();
614 decode_cxl_osc_support(root, "OS supports", cxl_support);
615 cxl_requested = cxl_control = calculate_cxl_control();
616 }
617
618 status = acpi_pci_osc_control_set(handle, &control, support,
619 &cxl_control, cxl_support);
620 if (ACPI_SUCCESS(status)) {
621 if (control)
622 decode_osc_control(root, "OS now controls", control);
623 if (cxl_control)
624 decode_cxl_osc_control(root, "OS now controls",
625 cxl_control);
626
627 if (acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_ASPM) {
628 /*
629 * We have ASPM control, but the FADT indicates that
630 * it's unsupported. Leave existing configuration
631 * intact and prevent the OS from touching it.
632 */
633 dev_info(&device->dev, "FADT indicates ASPM is unsupported, using BIOS configuration\n");
634 *no_aspm = 1;
635 }
636 } else {
637 /*
638 * We want to disable ASPM here, but aspm_disabled
639 * needs to remain in its state from boot so that we
640 * properly handle PCIe 1.1 devices. So we set this
641 * flag here, to defer the action until after the ACPI
642 * root scan.
643 */
644 *no_aspm = 1;
645
646 /* _OSC is optional for PCI host bridges */
647 if ((status == AE_NOT_FOUND) && !is_pcie(root))
648 return;
649
650 if (control) {
651 decode_osc_control(root, "OS requested", requested);
652 decode_osc_control(root, "platform willing to grant", control);
653 }
654 if (cxl_control) {
655 decode_cxl_osc_control(root, "OS requested", cxl_requested);
656 decode_cxl_osc_control(root, "platform willing to grant",
657 cxl_control);
658 }
659
660 dev_info(&device->dev, "_OSC: platform retains control of PCIe features (%s)\n",
661 acpi_format_exception(status));
662 }
663 }
664
---
0-DAY CI Kernel Test Service
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH 2/2] acpi/pci_root: negotiate CXL _OSC
2022-03-17 3:19 ` Dan Williams
@ 2022-03-17 3:49 ` Verma, Vishal L
0 siblings, 0 replies; 14+ messages in thread
From: Verma, Vishal L @ 2022-03-17 3:49 UTC (permalink / raw)
To: Williams, Dan J
Cc: rafael, Moore, Robert, linux-cxl, Jonathan.Cameron, Wysocki,
Rafael J, bhelgaas, linux-acpi
On Wed, 2022-03-16 at 20:19 -0700, Dan Williams wrote:
> On Wed, Mar 16, 2022 at 5:27 PM Vishal Verma <vishal.l.verma@intel.com> wrote:
> >
> > Add full support for negotiating _OSC as defined in the CXL 2.0 spec, as
> > applicable to CXL-enabled platforms. Advertise support for the CXL
> > features we support - 'CXL 2.0 port/device register access', 'Protocol
> > Error Reporting', and 'CL Native Hot Plug'. Request control for 'CXL
> > Memory Error Reporting'. The requests are dependent on CONFIG_* based
> > pre-requisites, and prior PCI enabling, similar to how the standard PCI
> > _OSC bits are determined.
>
> Might want to add a clarification here of why this just reflects the
> PCIe support into the similar CXL fields. For example:
>
> The CXL specification does not define any additional constraints on
> the hotplug flow beyond PCIe native hotplug, so a kernel that supports
> native PCIe hotplug, supports CXL hotplug. For error handling protocol
> and link errors just use PCIe AER. There is nascent support for
> amending AER events with CXL specific status [1], but there's
> otherwise no additional OS responsibility for CXL errors beyond PCIe
> AER. CXL Memory Errors behave the same as typical memory errors so
> CONFIG_MEMORY_FAILURE is sufficient to indicate support to platform
> firmware.
>
> [1]: https://lore.kernel.org/linux-cxl/164740402242.3912056.8303625392871313860.stgit@dwillia2-desk3.amr.corp.intel.com/
Yes that sounds good, will add.
> >
[..]
> > @@ -440,6 +511,18 @@ static u32 calculate_support(void)
> > return support;
> > }
> >
> > +static u32 calculate_cxl_support(void)
> > +{
> > + u32 support;
> > +
> > + support = OSC_CXL_2_0_PORT_DEV_REG_ACCESS_SUPPORT;
> > + support |= OSC_CXL_PER_SUPPORT;
>
> I would expect this one to be gated by pci_aer_available() since these
> errors are reported by PCIe AER.
>
> Perhaps also s/PER/PORT_ERROR/? I keep reading PER like 'per' as in 'per-cpu'.
Expanding the acronym sounds good, though it is Protocol Error
Reporting, not Port.
>
> > + if (IS_ENABLED(CONFIG_HOTPLUG_PCI_PCIE))
> > + support |= OSC_CXL_NATIVE_HP_SUPPORT;
> > +
> > + return support;
> > +}
> > +
> > static u32 calculate_control(void)
> > {
> > u32 control;
> > @@ -471,6 +554,16 @@ static u32 calculate_control(void)
> > return control;
> > }
> >
> > +static u32 calculate_cxl_control(void)
> > +{
> > + u32 control;
> > +
> > + if (pci_aer_available())
> > + control |= OSC_CXL_ERROR_REPORTING_CONTROL;
>
> In this case the error handling is for memory errors, so this one should be:
>
> if (IS_ENABLED(CONFIG_MEMORY_FAILURE))
> control |= OSC_CXL_ERROR_REPORTING_CONTROL;
Makes sense, I'll change to this.
>
> ...other than that looks good to me.
Thanks for the review!
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH 0/2] acpi: add support for CXL _OSC
2022-03-17 0:27 [RFC PATCH 0/2] acpi: add support for CXL _OSC Vishal Verma
2022-03-17 0:27 ` [RFC PATCH 1/2] PCI/ACPI: Use CXL _OSC instead of PCIe _OSC Vishal Verma
2022-03-17 0:27 ` [RFC PATCH 2/2] acpi/pci_root: negotiate CXL _OSC Vishal Verma
@ 2022-03-17 15:19 ` Jonathan Cameron
2022-03-18 19:52 ` Verma, Vishal L
2 siblings, 1 reply; 14+ messages in thread
From: Jonathan Cameron @ 2022-03-17 15:19 UTC (permalink / raw)
To: Vishal Verma
Cc: linux-cxl, linux-acpi, Dan Williams, Rafael J. Wysocki,
Robert Moore, Bjorn Helgaas
On Wed, 16 Mar 2022 18:27:02 -0600
Vishal Verma <vishal.l.verma@intel.com> wrote:
> Add support for using the CXL definition of _OSC where applicable, and
> negotiating CXL specific support and control bits.
>
> Patch 1 adds the new CXL _OSC UUID, and uses it instead of the PCI UUID
> when a root port is CXL enabled. It provides a fallback method for
> CXL-1.1 devices that may not implement the CXL-2.0 _OSC.
_OSC is implemented by the firmware of a host not the device so perhaps
rephrase this.
>
> Patch 2 performs negotiation for the CXL specific _OSC support and
> control bits.
>
> I've tested these against a custom qemu[1], which adds the CXL _OSC (in
> addition to other CXL support). Specifically, _OSC support is added
> here[2].
>
> [1]: https://gitlab.com/jic23/qemu/-/tree/cxl-v7-draft-2-for-test
> [2]: https://gitlab.com/jic23/qemu/-/commit/31c85054b84645dfbd9e9bb14aa35286141c14cf
Glad that worked :) I was wondering if it was correct.
There are some issues with that code raised in a recent review, so good
to have this to test against it going forwards.
Thanks,
Jonathan
>
> Dan Williams (1):
> PCI/ACPI: Use CXL _OSC instead of PCIe _OSC
>
> Vishal Verma (1):
> acpi/pci_root: negotiate CXL _OSC
>
> include/linux/acpi.h | 11 +++
> include/acpi/acpi_bus.h | 7 +-
> drivers/acpi/pci_root.c | 201 ++++++++++++++++++++++++++++++++++------
> 3 files changed, 187 insertions(+), 32 deletions(-)
>
>
> base-commit: 74be98774dfbc5b8b795db726bd772e735d2edd4
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH 1/2] PCI/ACPI: Use CXL _OSC instead of PCIe _OSC
2022-03-17 1:47 ` Dan Williams
@ 2022-03-17 15:40 ` Jonathan Cameron
0 siblings, 0 replies; 14+ messages in thread
From: Jonathan Cameron @ 2022-03-17 15:40 UTC (permalink / raw)
To: Dan Williams
Cc: Vishal Verma, linux-cxl, Linux ACPI, Rafael J. Wysocki,
Robert Moore, Bjorn Helgaas, Rafael J. Wysocki
On Wed, 16 Mar 2022 18:47:11 -0700
Dan Williams <dan.j.williams@intel.com> wrote:
> On Wed, Mar 16, 2022 at 5:27 PM Vishal Verma <vishal.l.verma@intel.com> wrote:
> >
> > From: Dan Williams <dan.j.williams@intel.com>
> >
> > In preparation for negotiating OS control of CXL _OSC features, do the
> > minimal enabling to use CXL _OSC to handle the base PCIe feature
> > negotiation. Recall that CXL _OSC is a super-set of PCIe _OSC and the
> > CXL 2.0 specification mandates: "If a CXL Host Bridge device exposes CXL
> > _OSC, CXL aware OSPM shall evaluate CXL _OSC and not evaluate PCIe
> > _OSC."
> >
> > A new ->cxl_osc_disable attribute is added for cases where platform
> > firmware publishes ACPI0016, but does not also publish CXL _OSC.
>
> It's been a couple weeks since I wrote this... looking at it now I
> would rewrite this to:
>
> Rather than pass a boolean flag alongside @root to all the helper
> functions that need to consider PCIe specifics, add is_pcie() and
> is_cxl() helper functions to check the flavor of @root. This also
> allows for dynamic fallback to PCIe _OSC in cases where an attempt to
> use CXL _OXC fails. This can happen on CXL 1.1 platforms that publish
> ACPI0016 devices to indicate CXL host bridges, but do not publish the
> optional CXL _OSC method. CXL _OSC is mandatory for CXL 2.0 hosts.
>
> >
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> > Cc: Robert Moore <robert.moore@intel.com>
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>
> Always include your own sign-off when forwarding a patch.
>
Subject to Dan's rewording above, this looks good to me.
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> > ---
> > include/acpi/acpi_bus.h | 1 +
> > drivers/acpi/pci_root.c | 62 +++++++++++++++++++++++++++++++----------
> > 2 files changed, 48 insertions(+), 15 deletions(-)
> >
> > diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> > index ca88c4706f2b..768ef1584055 100644
> > --- a/include/acpi/acpi_bus.h
> > +++ b/include/acpi/acpi_bus.h
> > @@ -585,6 +585,7 @@ struct acpi_pci_root {
> > struct acpi_device * device;
> > struct pci_bus *bus;
> > u16 segment;
> > + bool cxl_osc_disable;
> > struct resource secondary; /* downstream bus range */
> >
> > u32 osc_support_set; /* _OSC state of support bits */
> > diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> > index b76db99cced3..2d834504096b 100644
> > --- a/drivers/acpi/pci_root.c
> > +++ b/drivers/acpi/pci_root.c
> > @@ -170,20 +170,47 @@ static void decode_osc_control(struct acpi_pci_root *root, char *msg, u32 word)
> > ARRAY_SIZE(pci_osc_control_bit));
> > }
> >
> > -static u8 pci_osc_uuid_str[] = "33DB4D5B-1FF7-401C-9657-7441C03DD766";
> > +static bool is_pcie(struct acpi_pci_root *root)
> > +{
> > + return strcmp(acpi_device_hid(root->device), "PNP0A08") == 0;
> > +}
> >
> > -static acpi_status acpi_pci_run_osc(acpi_handle handle,
> > +static bool is_cxl(struct acpi_pci_root *root)
> > +{
> > + if (root->cxl_osc_disable)
> > + return false;
> > + return strcmp(acpi_device_hid(root->device), "ACPI0016") == 0;
> > +}
> > +
> > +static u8 pci_osc_uuid_str[] = "33DB4D5B-1FF7-401C-9657-7441C03DD766";
> > +static u8 cxl_osc_uuid_str[] = "68F2D50B-C469-4d8A-BD3D-941A103FD3FC";
> > +
> > +static char *to_uuid(struct acpi_pci_root *root)
> > +{
> > + if (is_cxl(root))
> > + return cxl_osc_uuid_str;
> > + return pci_osc_uuid_str;
> > +}
> > +
> > +static int cap_length(struct acpi_pci_root *root)
> > +{
> > + if (is_cxl(root))
> > + return sizeof(u32) * 6;
> > + return sizeof(u32) * 3;
> > +}
> > +
> > +static acpi_status acpi_pci_run_osc(struct acpi_pci_root *root,
> > const u32 *capbuf, u32 *retval)
> > {
> > struct acpi_osc_context context = {
> > - .uuid_str = pci_osc_uuid_str,
> > + .uuid_str = to_uuid(root),
> > .rev = 1,
> > - .cap.length = 12,
> > + .cap.length = cap_length(root),
> > .cap.pointer = (void *)capbuf,
> > };
> > acpi_status status;
> >
> > - status = acpi_run_osc(handle, &context);
> > + status = acpi_run_osc(root->device->handle, &context);
> > if (ACPI_SUCCESS(status)) {
> > *retval = *((u32 *)(context.ret.pointer + 8));
> > kfree(context.ret.pointer);
> > @@ -196,7 +223,7 @@ static acpi_status acpi_pci_query_osc(struct acpi_pci_root *root,
> > u32 *control)
> > {
> > acpi_status status;
> > - u32 result, capbuf[3];
> > + u32 result, capbuf[6];
> >
> > support |= root->osc_support_set;
> >
> > @@ -204,10 +231,18 @@ static acpi_status acpi_pci_query_osc(struct acpi_pci_root *root,
> > capbuf[OSC_SUPPORT_DWORD] = support;
> > capbuf[OSC_CONTROL_DWORD] = *control | root->osc_control_set;
> >
> > - status = acpi_pci_run_osc(root->device->handle, capbuf, &result);
> > +retry:
> > + status = acpi_pci_run_osc(root, capbuf, &result);
> > if (ACPI_SUCCESS(status)) {
> > root->osc_support_set = support;
> > *control = result;
> > + } else if (is_cxl(root)) {
> > + /*
> > + * CXL _OSC is optional on CXL 1.1 hosts. Fall back to PCIe _OSC
> > + * upon any failure using CXL _OSC.
> > + */
> > + root->cxl_osc_disable = true;
> > + goto retry;
> > }
> > return status;
> > }
> > @@ -338,7 +373,7 @@ static acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask, u32 s
> > u32 req = OSC_PCI_EXPRESS_CAPABILITY_CONTROL;
> > struct acpi_pci_root *root;
> > acpi_status status;
> > - u32 ctrl, capbuf[3];
> > + u32 ctrl, capbuf[6];
> >
> > if (!mask)
> > return AE_BAD_PARAMETER;
> > @@ -375,7 +410,7 @@ static acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask, u32 s
> > 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_pci_run_osc(root, capbuf, mask);
> > if (ACPI_FAILURE(status))
> > return status;
> >
> > @@ -454,8 +489,7 @@ static bool os_control_query_checks(struct acpi_pci_root *root, u32 support)
> > return true;
> > }
> >
> > -static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm,
> > - bool is_pcie)
> > +static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm)
> > {
> > u32 support, control = 0, requested = 0;
> > acpi_status status;
> > @@ -506,7 +540,7 @@ static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm,
> > *no_aspm = 1;
> >
> > /* _OSC is optional for PCI host bridges */
> > - if ((status == AE_NOT_FOUND) && !is_pcie)
> > + if ((status == AE_NOT_FOUND) && !is_pcie(root))
> > return;
> >
> > if (control) {
> > @@ -529,7 +563,6 @@ 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;
> >
> > root = kzalloc(sizeof(struct acpi_pci_root), GFP_KERNEL);
> > if (!root)
> > @@ -587,8 +620,7 @@ 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);
> > + negotiate_os_control(root, &no_aspm);
> >
> > /*
> > * TBD: Need PCI interface for enumeration/configuration of roots.
> > --
> > 2.35.1
> >
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH 2/2] acpi/pci_root: negotiate CXL _OSC
2022-03-17 0:27 ` [RFC PATCH 2/2] acpi/pci_root: negotiate CXL _OSC Vishal Verma
2022-03-17 3:19 ` Dan Williams
2022-03-17 3:25 ` kernel test robot
@ 2022-03-17 16:10 ` Jonathan Cameron
2022-03-18 21:16 ` Verma, Vishal L
2 siblings, 1 reply; 14+ messages in thread
From: Jonathan Cameron @ 2022-03-17 16:10 UTC (permalink / raw)
To: Vishal Verma
Cc: linux-cxl, linux-acpi, Dan Williams, Rafael J. Wysocki,
Robert Moore, Bjorn Helgaas, Rafael J. Wysocki
On Wed, 16 Mar 2022 18:27:04 -0600
Vishal Verma <vishal.l.verma@intel.com> wrote:
> Add full support for negotiating _OSC as defined in the CXL 2.0 spec, as
> applicable to CXL-enabled platforms. Advertise support for the CXL
> features we support - 'CXL 2.0 port/device register access', 'Protocol
> Error Reporting', and 'CL Native Hot Plug'. Request control for 'CXL
> Memory Error Reporting'. The requests are dependent on CONFIG_* based
> pre-requisites, and prior PCI enabling, similar to how the standard PCI
> _OSC bits are determined.
>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Robert Moore <robert.moore@intel.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
Hi Vishal,
A few minor queries inline.
Jonathan
> ---
> include/linux/acpi.h | 11 +++
> include/acpi/acpi_bus.h | 6 +-
> drivers/acpi/pci_root.c | 147 ++++++++++++++++++++++++++++++++++------
> 3 files changed, 143 insertions(+), 21 deletions(-)
>
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 6274758648e3..1717ccc265d7 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -554,6 +554,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 OSC_CXL_SUPPORT_DWORD 3 /* DWORD 4 */
> +#define OSC_CXL_CONTROL_DWORD 4 /* DWORD 5 */
>
> /* _OSC Capabilities DWORD 1: Query/Control and Error Returns (generic) */
> #define OSC_QUERY_ENABLE 0x00000001 /* input */
> @@ -607,6 +609,15 @@ extern u32 osc_sb_native_usb4_control;
> #define OSC_PCI_EXPRESS_LTR_CONTROL 0x00000020
> #define OSC_PCI_EXPRESS_DPC_CONTROL 0x00000080
>
> +/* CXL _OSC: Capabilities DWORD 4: Support Field */
> +#define OSC_CXL_1_1_PORT_REG_ACCESS_SUPPORT 0x00000001
> +#define OSC_CXL_2_0_PORT_DEV_REG_ACCESS_SUPPORT 0x00000002
> +#define OSC_CXL_PER_SUPPORT 0x00000004
> +#define OSC_CXL_NATIVE_HP_SUPPORT 0x00000008
> +
> +/* CXL _OSC: Capabilities DWORD 5: Control Field */
> +#define OSC_CXL_ERROR_REPORTING_CONTROL 0x00000001
> +
> #define ACPI_GSB_ACCESS_ATTRIB_QUICK 0x00000002
> #define ACPI_GSB_ACCESS_ATTRIB_SEND_RCV 0x00000004
> #define ACPI_GSB_ACCESS_ATTRIB_BYTE 0x00000006
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index 768ef1584055..5776d4c1509a 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -588,8 +588,10 @@ struct acpi_pci_root {
> bool cxl_osc_disable;
> 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/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index 2d834504096b..c916318b11a0 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -142,6 +142,17 @@ static struct pci_osc_bit_struct pci_osc_control_bit[] = {
> { OSC_PCI_EXPRESS_DPC_CONTROL, "DPC" },
> };
>
> +static struct pci_osc_bit_struct cxl_osc_support_bit[] = {
> + { OSC_CXL_1_1_PORT_REG_ACCESS_SUPPORT, "CXL11PortRegAccess" },
> + { OSC_CXL_2_0_PORT_DEV_REG_ACCESS_SUPPORT, "CXL20PortDevRegAccess" },
> + { OSC_CXL_PER_SUPPORT, "CXLProtocolErrorReporting" },
> + { OSC_CXL_NATIVE_HP_SUPPORT, "CXLNativeHotPlug" },
> +};
> +
> +static struct pci_osc_bit_struct cxl_osc_control_bit[] = {
> + { OSC_CXL_ERROR_REPORTING_CONTROL, "CXLMemErrorReporting" },
> +};
> +
> static void decode_osc_bits(struct acpi_pci_root *root, char *msg, u32 word,
> struct pci_osc_bit_struct *table, int size)
> {
> @@ -170,6 +181,18 @@ static void decode_osc_control(struct acpi_pci_root *root, char *msg, u32 word)
> ARRAY_SIZE(pci_osc_control_bit));
> }
>
> +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 bool is_pcie(struct acpi_pci_root *root)
> {
> return strcmp(acpi_device_hid(root->device), "PNP0A08") == 0;
> @@ -199,8 +222,19 @@ static int cap_length(struct acpi_pci_root *root)
> return sizeof(u32) * 3;
> }
>
> +static u32 acpi_osc_ctx_get_pci_control(struct acpi_osc_context *context)
> +{
> + return *((u32 *)(context->ret.pointer + 8));
Use the defines for the offsets? sizeof(u32) * OSC_CONTROL_DWORD for example
> +}
> +
> +static u32 acpi_osc_ctx_get_cxl_control(struct acpi_osc_context *context)
> +{
> + return *((u32 *)(context->ret.pointer + 16));
As above + sizeof(u32) * OSC_CXL_CONTROL_DWORD)
> +}
> +
> static acpi_status acpi_pci_run_osc(struct acpi_pci_root *root,
> - const u32 *capbuf, u32 *retval)
> + const u32 *capbuf, u32 *pci_control,
> + u32 *cxl_control)
> {
> struct acpi_osc_context context = {
> .uuid_str = to_uuid(root),
> @@ -212,18 +246,20 @@ static acpi_status acpi_pci_run_osc(struct acpi_pci_root *root,
>
> status = acpi_run_osc(root->device->handle, &context);
> if (ACPI_SUCCESS(status)) {
> - *retval = *((u32 *)(context.ret.pointer + 8));
> + *pci_control = acpi_osc_ctx_get_pci_control(&context);
> + if (is_cxl(root))
> + *cxl_control = acpi_osc_ctx_get_cxl_control(&context);
> kfree(context.ret.pointer);
> }
> return status;
> }
>
> -static acpi_status acpi_pci_query_osc(struct acpi_pci_root *root,
> - u32 support,
> - u32 *control)
> +static acpi_status acpi_pci_query_osc(struct acpi_pci_root *root, u32 support,
> + u32 *control, u32 cxl_support,
> + u32 *cxl_control)
> {
> acpi_status status;
> - u32 result, capbuf[6];
> + u32 pci_result, cxl_result, capbuf[8];
Nice to set capbuf size off one of the defines if possible, though I'm not
sure why it is 8 (or why it was 6 before for that mater). I think it should be 5.
>
> support |= root->osc_support_set;
>
> @@ -231,11 +267,21 @@ static acpi_status acpi_pci_query_osc(struct acpi_pci_root *root,
> capbuf[OSC_SUPPORT_DWORD] = support;
> capbuf[OSC_CONTROL_DWORD] = *control | root->osc_control_set;
>
> + if (is_cxl(root)) {
> + cxl_support |= root->cxl_osc_support_set;
> + capbuf[OSC_CXL_SUPPORT_DWORD] = cxl_support;
> + capbuf[OSC_CXL_CONTROL_DWORD] = *cxl_control | root->cxl_osc_control_set;
> + }
> +
> retry:
> - status = acpi_pci_run_osc(root, capbuf, &result);
> + status = acpi_pci_run_osc(root, capbuf, &pci_result, &cxl_result);
> if (ACPI_SUCCESS(status)) {
> root->osc_support_set = support;
> - *control = result;
> + *control = pci_result;
> + if (is_cxl(root)) {
> + root->cxl_osc_support_set = cxl_support;
> + *cxl_control = cxl_result;
> + }
> } else if (is_cxl(root)) {
> /*
> * CXL _OSC is optional on CXL 1.1 hosts. Fall back to PCIe _OSC
> @@ -358,6 +404,8 @@ EXPORT_SYMBOL_GPL(acpi_get_pci_dev);
> * @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.
> * @support: _OSC supported capability.
> + * @cxl_mask: Mask of CXL _OSC control bits, place to store control mask.
> + * @cxl_support: CXL _OSC supported capability.
> *
> * Run _OSC query for @mask and if that is successful, compare the returned
> * mask of control bits with @req. If all of the @req bits are set in the
> @@ -368,12 +416,14 @@ EXPORT_SYMBOL_GPL(acpi_get_pci_dev);
> * _OSC bits the BIOS has granted control of, but its contents are meaningless
> * on failure.
> **/
> -static acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask, u32 support)
> +static acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask,
> + u32 support, u32 *cxl_mask,
> + u32 cxl_support)
> {
> u32 req = OSC_PCI_EXPRESS_CAPABILITY_CONTROL;
> struct acpi_pci_root *root;
> acpi_status status;
> - u32 ctrl, capbuf[6];
> + u32 ctrl, cxl_ctrl, capbuf[8];
As above, why 8?
>
> if (!mask)
> return AE_BAD_PARAMETER;
> @@ -385,20 +435,35 @@ static acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask, u32 s
> ctrl = *mask;
> *mask |= root->osc_control_set;
>
> + if (is_cxl(root)) {
> + cxl_ctrl = *cxl_mask;
Odd spacing
> + *mask |= root->osc_control_set;
> + }
> +
> /* Need to check the available controls bits before requesting them. */
> do {
> - status = acpi_pci_query_osc(root, support, mask);
> + status = acpi_pci_query_osc(root, support, mask, cxl_support,
> + cxl_mask);
> if (ACPI_FAILURE(status))
> return status;
> - if (ctrl == *mask)
> - break;
> - decode_osc_control(root, "platform does not support",
> - ctrl & ~(*mask));
> + if (is_cxl(root)) {
> + if ((ctrl == *mask) && (cxl_ctrl == *cxl_mask))
> + break;
> + decode_cxl_osc_control(root, "platform does not support",
> + cxl_ctrl & ~(*cxl_mask));
> + } else {
> + if (ctrl == *mask)
> + break;
> + decode_osc_control(root, "platform does not support",
> + ctrl & ~(*mask));
> + }
> ctrl = *mask;
> - } while (*mask);
> + cxl_ctrl = *cxl_mask;
> + } while (*mask || *cxl_mask);
>
> /* No need to request _OSC if the control was already granted. */
> - if ((root->osc_control_set & ctrl) == ctrl)
> + if (((root->osc_control_set & ctrl) == ctrl) &&
> + ((root->cxl_osc_control_set & cxl_ctrl) == cxl_ctrl))
> return AE_OK;
>
> if ((ctrl & req) != req) {
> @@ -410,11 +475,17 @@ static acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask, u32 s
> capbuf[OSC_QUERY_DWORD] = 0;
> capbuf[OSC_SUPPORT_DWORD] = root->osc_support_set;
> capbuf[OSC_CONTROL_DWORD] = ctrl;
> - status = acpi_pci_run_osc(root, capbuf, mask);
> + if (is_cxl(root)) {
> + capbuf[OSC_CXL_SUPPORT_DWORD] = root->cxl_osc_support_set;
> + capbuf[OSC_CXL_CONTROL_DWORD] = cxl_ctrl;
> + }
> +
> + status = acpi_pci_run_osc(root, capbuf, mask, cxl_mask);
> if (ACPI_FAILURE(status))
> return status;
>
> root->osc_control_set = *mask;
> + root->cxl_osc_control_set = *cxl_mask;
> return AE_OK;
> }
>
> @@ -440,6 +511,18 @@ static u32 calculate_support(void)
> return support;
> }
>
> +static u32 calculate_cxl_support(void)
> +{
> + u32 support;
> +
> + support = OSC_CXL_2_0_PORT_DEV_REG_ACCESS_SUPPORT;
> + support |= OSC_CXL_PER_SUPPORT;
> + if (IS_ENABLED(CONFIG_HOTPLUG_PCI_PCIE))
> + support |= OSC_CXL_NATIVE_HP_SUPPORT;
> +
> + return support;
> +}
> +
> static u32 calculate_control(void)
> {
> u32 control;
> @@ -471,6 +554,16 @@ static u32 calculate_control(void)
> return control;
> }
>
> +static u32 calculate_cxl_control(void)
> +{
> + u32 control;
> +
> + if (pci_aer_available())
> + control |= OSC_CXL_ERROR_REPORTING_CONTROL;
> +
> + return control;
> +}
> +
> static bool os_control_query_checks(struct acpi_pci_root *root, u32 support)
> {
> struct acpi_device *device = root->device;
> @@ -492,6 +585,7 @@ static bool os_control_query_checks(struct acpi_pci_root *root, u32 support)
> static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm)
> {
> u32 support, control = 0, requested = 0;
> + u32 cxl_support, cxl_control = 0, cxl_requested = 0;
> acpi_status status;
> struct acpi_device *device = root->device;
> acpi_handle handle = device->handle;
> @@ -515,10 +609,20 @@ static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm)
> if (os_control_query_checks(root, support))
> requested = control = calculate_control();
>
> - status = acpi_pci_osc_control_set(handle, &control, support);
> + if (is_cxl(root)) {
> + cxl_support = calculate_cxl_support();
> + decode_cxl_osc_support(root, "OS supports", cxl_support);
> + cxl_requested = cxl_control = calculate_cxl_control();
> + }
> +
> + status = acpi_pci_osc_control_set(handle, &control, support,
> + &cxl_control, cxl_support);
> if (ACPI_SUCCESS(status)) {
> if (control)
> decode_osc_control(root, "OS now controls", control);
> + if (cxl_control)
> + decode_cxl_osc_control(root, "OS now controls",
> + cxl_control);
>
> if (acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_ASPM) {
> /*
> @@ -547,6 +651,11 @@ static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm)
> decode_osc_control(root, "OS requested", requested);
> decode_osc_control(root, "platform willing to grant", control);
> }
> + if (cxl_control) {
> + decode_cxl_osc_control(root, "OS requested", cxl_requested);
> + decode_cxl_osc_control(root, "platform willing to grant",
> + cxl_control);
> + }
>
> dev_info(&device->dev, "_OSC: platform retains control of PCIe features (%s)\n",
> acpi_format_exception(status));
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH 0/2] acpi: add support for CXL _OSC
2022-03-17 15:19 ` [RFC PATCH 0/2] acpi: add support for " Jonathan Cameron
@ 2022-03-18 19:52 ` Verma, Vishal L
0 siblings, 0 replies; 14+ messages in thread
From: Verma, Vishal L @ 2022-03-18 19:52 UTC (permalink / raw)
To: Jonathan.Cameron
Cc: Williams, Dan J, Moore, Robert, linux-cxl, Wysocki, Rafael J,
bhelgaas, linux-acpi
On Thu, 2022-03-17 at 15:19 +0000, Jonathan Cameron wrote:
> On Wed, 16 Mar 2022 18:27:02 -0600
> Vishal Verma <vishal.l.verma@intel.com> wrote:
>
> > Add support for using the CXL definition of _OSC where applicable, and
> > negotiating CXL specific support and control bits.
> >
> > Patch 1 adds the new CXL _OSC UUID, and uses it instead of the PCI UUID
> > when a root port is CXL enabled. It provides a fallback method for
> > CXL-1.1 devices that may not implement the CXL-2.0 _OSC.
>
> _OSC is implemented by the firmware of a host not the device so perhaps
> rephrase this.
Yes good point - I'll reword to say "CXL-1.1 platforms"
>
> >
> > Patch 2 performs negotiation for the CXL specific _OSC support and
> > control bits.
> >
> > I've tested these against a custom qemu[1], which adds the CXL _OSC (in
> > addition to other CXL support). Specifically, _OSC support is added
> > here[2].
> >
> > [1]: https://gitlab.com/jic23/qemu/-/tree/cxl-v7-draft-2-for-test
> > [2]: https://gitlab.com/jic23/qemu/-/commit/31c85054b84645dfbd9e9bb14aa35286141c14cf
>
> Glad that worked :) I was wondering if it was correct.
> There are some issues with that code raised in a recent review, so good
> to have this to test against it going forwards.
Thanks for taking a look and moving the qemu series forward!
>
> Thanks,
>
> Jonathan
>
> >
> > Dan Williams (1):
> > PCI/ACPI: Use CXL _OSC instead of PCIe _OSC
> >
> > Vishal Verma (1):
> > acpi/pci_root: negotiate CXL _OSC
> >
> > include/linux/acpi.h | 11 +++
> > include/acpi/acpi_bus.h | 7 +-
> > drivers/acpi/pci_root.c | 201 ++++++++++++++++++++++++++++++++++------
> > 3 files changed, 187 insertions(+), 32 deletions(-)
> >
> >
> > base-commit: 74be98774dfbc5b8b795db726bd772e735d2edd4
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH 2/2] acpi/pci_root: negotiate CXL _OSC
2022-03-17 16:10 ` Jonathan Cameron
@ 2022-03-18 21:16 ` Verma, Vishal L
0 siblings, 0 replies; 14+ messages in thread
From: Verma, Vishal L @ 2022-03-18 21:16 UTC (permalink / raw)
To: Jonathan.Cameron
Cc: Williams, Dan J, Moore, Robert, linux-cxl, Wysocki, Rafael J,
rafael, bhelgaas, linux-acpi
On Thu, 2022-03-17 at 16:10 +0000, Jonathan Cameron wrote:
> On Wed, 16 Mar 2022 18:27:04 -0600
> Vishal Verma <vishal.l.verma@intel.com> wrote:
>
> > Add full support for negotiating _OSC as defined in the CXL 2.0 spec, as
> > applicable to CXL-enabled platforms. Advertise support for the CXL
> > features we support - 'CXL 2.0 port/device register access', 'Protocol
> > Error Reporting', and 'CL Native Hot Plug'. Request control for 'CXL
> > Memory Error Reporting'. The requests are dependent on CONFIG_* based
> > pre-requisites, and prior PCI enabling, similar to how the standard PCI
> > _OSC bits are determined.
> >
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> > Cc: Robert Moore <robert.moore@intel.com>
> > Cc: Dan Williams <dan.j.williams@intel.com>
> > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> Hi Vishal,
>
> A few minor queries inline.
>
> Jonathan
Thanks for reviewing Jonathan - fixed up most of the things, see below.
>
[..]
> >
> > +static u32 acpi_osc_ctx_get_pci_control(struct acpi_osc_context *context)
> > +{
> > + return *((u32 *)(context->ret.pointer + 8));
>
> Use the defines for the offsets? sizeof(u32) * OSC_CONTROL_DWORD for example
>
> > +}
> > +
> > +static u32 acpi_osc_ctx_get_cxl_control(struct acpi_osc_context *context)
> > +{
> > + return *((u32 *)(context->ret.pointer + 16));
>
> As above + sizeof(u32) * OSC_CXL_CONTROL_DWORD)
Makes sense, done.
>
> > +}
> > +
> > static acpi_status acpi_pci_run_osc(struct acpi_pci_root *root,
> > - const u32 *capbuf, u32 *retval)
> > + const u32 *capbuf, u32 *pci_control,
> > + u32 *cxl_control)
> > {
> > struct acpi_osc_context context = {
> > .uuid_str = to_uuid(root),
> > @@ -212,18 +246,20 @@ static acpi_status acpi_pci_run_osc(struct acpi_pci_root *root,
> >
> > status = acpi_run_osc(root->device->handle, &context);
> > if (ACPI_SUCCESS(status)) {
> > - *retval = *((u32 *)(context.ret.pointer + 8));
> > + *pci_control = acpi_osc_ctx_get_pci_control(&context);
> > + if (is_cxl(root))
> > + *cxl_control = acpi_osc_ctx_get_cxl_control(&context);
> > kfree(context.ret.pointer);
> > }
> > return status;
> > }
> >
> > -static acpi_status acpi_pci_query_osc(struct acpi_pci_root *root,
> > - u32 support,
> > - u32 *control)
> > +static acpi_status acpi_pci_query_osc(struct acpi_pci_root *root, u32 support,
> > + u32 *control, u32 cxl_support,
> > + u32 *cxl_control)
> > {
> > acpi_status status;
> > - u32 result, capbuf[6];
> > + u32 pci_result, cxl_result, capbuf[8];
>
> Nice to set capbuf size off one of the defines if possible, though I'm not
> sure why it is 8 (or why it was 6 before for that mater). I think it should be 5.
Yep, I'm not sure why these were 6. I've added a new define and set it
to 5. Perhaps someone from ACPI might comment if there was a reason for
the extra padding. Rafael or Robert?
>
> >
> > support |= root->osc_support_set;
> >
> > @@ -231,11 +267,21 @@ static acpi_status acpi_pci_query_osc(struct acpi_pci_root *root,
> > capbuf[OSC_SUPPORT_DWORD] = support;
> > capbuf[OSC_CONTROL_DWORD] = *control | root->osc_control_set;
> >
> > + if (is_cxl(root)) {
> > + cxl_support |= root->cxl_osc_support_set;
> > + capbuf[OSC_CXL_SUPPORT_DWORD] = cxl_support;
> > + capbuf[OSC_CXL_CONTROL_DWORD] = *cxl_control | root->cxl_osc_control_set;
> > + }
> > +
> > retry:
> > - status = acpi_pci_run_osc(root, capbuf, &result);
> > + status = acpi_pci_run_osc(root, capbuf, &pci_result, &cxl_result);
> > if (ACPI_SUCCESS(status)) {
> > root->osc_support_set = support;
> > - *control = result;
> > + *control = pci_result;
> > + if (is_cxl(root)) {
> > + root->cxl_osc_support_set = cxl_support;
> > + *cxl_control = cxl_result;
> > + }
> > } else if (is_cxl(root)) {
> > /*
> > * CXL _OSC is optional on CXL 1.1 hosts. Fall back to PCIe _OSC
> > @@ -358,6 +404,8 @@ EXPORT_SYMBOL_GPL(acpi_get_pci_dev);
> > * @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.
> > * @support: _OSC supported capability.
> > + * @cxl_mask: Mask of CXL _OSC control bits, place to store control mask.
> > + * @cxl_support: CXL _OSC supported capability.
> > *
> > * Run _OSC query for @mask and if that is successful, compare the returned
> > * mask of control bits with @req. If all of the @req bits are set in the
> > @@ -368,12 +416,14 @@ EXPORT_SYMBOL_GPL(acpi_get_pci_dev);
> > * _OSC bits the BIOS has granted control of, but its contents are meaningless
> > * on failure.
> > **/
> > -static acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask, u32 support)
> > +static acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask,
> > + u32 support, u32 *cxl_mask,
> > + u32 cxl_support)
> > {
> > u32 req = OSC_PCI_EXPRESS_CAPABILITY_CONTROL;
> > struct acpi_pci_root *root;
> > acpi_status status;
> > - u32 ctrl, capbuf[6];
> > + u32 ctrl, cxl_ctrl, capbuf[8];
>
> As above, why 8?
>
> >
> > if (!mask)
> > return AE_BAD_PARAMETER;
> > @@ -385,20 +435,35 @@ static acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask, u32 s
> > ctrl = *mask;
> > *mask |= root->osc_control_set;
> >
> > + if (is_cxl(root)) {
> > + cxl_ctrl = *cxl_mask;
>
> Odd spacing
Fixed.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH 2/2] acpi/pci_root: negotiate CXL _OSC
@ 2022-03-18 0:33 kernel test robot
0 siblings, 0 replies; 14+ messages in thread
From: kernel test robot @ 2022-03-18 0:33 UTC (permalink / raw)
To: kbuild
[-- Attachment #1: Type: text/plain, Size: 28527 bytes --]
CC: llvm(a)lists.linux.dev
CC: kbuild-all(a)lists.01.org
BCC: lkp(a)intel.com
In-Reply-To: <20220317002704.1835870-3-vishal.l.verma@intel.com>
References: <20220317002704.1835870-3-vishal.l.verma@intel.com>
TO: Vishal Verma <vishal.l.verma@intel.com>
Hi Vishal,
[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on 74be98774dfbc5b8b795db726bd772e735d2edd4]
url: https://github.com/0day-ci/linux/commits/Vishal-Verma/acpi-add-support-for-CXL-_OSC/20220317-082840
base: 74be98774dfbc5b8b795db726bd772e735d2edd4
:::::: branch date: 24 hours ago
:::::: commit date: 24 hours ago
config: i386-randconfig-c001 (https://download.01.org/0day-ci/archive/20220318/202203180801.eBPKbPgj-lkp(a)intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project a6ec1e3d798f8eab43fb3a91028c6ab04e115fcb)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/8020d862c22d52b61a590343b2202e5d05332100
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Vishal-Verma/acpi-add-support-for-CXL-_OSC/20220317-082840
git checkout 8020d862c22d52b61a590343b2202e5d05332100
# save the config file to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=i386 clang-analyzer
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
clang-analyzer warnings: (new ones prefixed by >>)
drivers/mfd/wm831x-irq.c:517:7: note: 'primary' is equal to 8192
if (primary == WM831X_GP_INT &&
^
include/linux/compiler.h:56:47: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^~~~
include/linux/compiler.h:58:86: note: expanded from macro '__trace_if_var'
#define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
^~~~
include/linux/compiler.h:69:3: note: expanded from macro '__trace_if_value'
(cond) ? \
^~~~
drivers/mfd/wm831x-irq.c:517:7: note: Left side of '&&' is true
if (primary == WM831X_GP_INT &&
^
drivers/mfd/wm831x-irq.c:517:3: note: '?' condition is false
if (primary == WM831X_GP_INT &&
^
include/linux/compiler.h:56:28: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^
include/linux/compiler.h:58:69: note: expanded from macro '__trace_if_var'
#define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
^
include/linux/compiler.h:69:2: note: expanded from macro '__trace_if_value'
(cond) ? \
^
drivers/mfd/wm831x-irq.c:517:3: note: Taking false branch
if (primary == WM831X_GP_INT &&
^
include/linux/compiler.h:56:23: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^
drivers/mfd/wm831x-irq.c:528:7: note: 'primary' is equal to 8192
if (primary == WM831X_GP_INT &&
^
include/linux/compiler.h:56:47: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^~~~
include/linux/compiler.h:58:52: note: expanded from macro '__trace_if_var'
#define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
^~~~
drivers/mfd/wm831x-irq.c:528:7: note: Left side of '&&' is true
if (primary == WM831X_GP_INT &&
^
drivers/mfd/wm831x-irq.c:528:3: note: Assuming the condition is false
if (primary == WM831X_GP_INT &&
^
include/linux/compiler.h:56:45: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/compiler.h:58:52: note: expanded from macro '__trace_if_var'
#define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
^~~~
drivers/mfd/wm831x-irq.c:528:3: note: '?' condition is false
if (primary == WM831X_GP_INT &&
^
include/linux/compiler.h:56:28: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^
include/linux/compiler.h:58:31: note: expanded from macro '__trace_if_var'
#define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
^
drivers/mfd/wm831x-irq.c:528:7: note: 'primary' is equal to 8192
if (primary == WM831X_GP_INT &&
^
include/linux/compiler.h:56:47: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^~~~
include/linux/compiler.h:58:86: note: expanded from macro '__trace_if_var'
#define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
^~~~
include/linux/compiler.h:69:3: note: expanded from macro '__trace_if_value'
(cond) ? \
^~~~
drivers/mfd/wm831x-irq.c:528:7: note: Left side of '&&' is true
if (primary == WM831X_GP_INT &&
^
drivers/mfd/wm831x-irq.c:528:3: note: '?' condition is true
if (primary == WM831X_GP_INT &&
^
include/linux/compiler.h:56:28: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^
include/linux/compiler.h:58:69: note: expanded from macro '__trace_if_var'
#define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
^
include/linux/compiler.h:69:2: note: expanded from macro '__trace_if_value'
(cond) ? \
^
drivers/mfd/wm831x-irq.c:528:3: note: Taking true branch
if (primary == WM831X_GP_INT &&
^
include/linux/compiler.h:56:23: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^
drivers/mfd/wm831x-irq.c:531:21: note: The result of the left shift is undefined due to shifting by '4294967295', which is greater or equal to the width of type 'int'
while (!(ret & 1 << (i - WM831X_IRQ_GPIO_1))) {
^ ~~~~~~~~~~~~~~~~~~~~~~~
8 warnings generated.
>> drivers/acpi/pci_root.c:283:17: warning: Assigned value is garbage or undefined [clang-analyzer-core.uninitialized.Assign]
*cxl_control = cxl_result;
^
drivers/acpi/pci_root.c:428:6: note: Assuming 'mask' is non-null
if (!mask)
^
include/linux/compiler.h:56:47: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^~~~
include/linux/compiler.h:58:52: note: expanded from macro '__trace_if_var'
#define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
^~~~
drivers/acpi/pci_root.c:428:2: note: '?' condition is false
if (!mask)
^
include/linux/compiler.h:56:28: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^
include/linux/compiler.h:58:31: note: expanded from macro '__trace_if_var'
#define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
^
drivers/acpi/pci_root.c:428:7: note: 'mask' is non-null
if (!mask)
^
include/linux/compiler.h:56:47: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^~~~
include/linux/compiler.h:58:86: note: expanded from macro '__trace_if_var'
#define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
^~~~
include/linux/compiler.h:69:3: note: expanded from macro '__trace_if_value'
(cond) ? \
^~~~
drivers/acpi/pci_root.c:428:2: note: '?' condition is false
if (!mask)
^
include/linux/compiler.h:56:28: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^
include/linux/compiler.h:58:69: note: expanded from macro '__trace_if_var'
#define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
^
include/linux/compiler.h:69:2: note: expanded from macro '__trace_if_value'
(cond) ? \
^
drivers/acpi/pci_root.c:428:2: note: Taking false branch
if (!mask)
^
include/linux/compiler.h:56:23: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^
drivers/acpi/pci_root.c:432:6: note: Assuming 'root' is non-null
if (!root)
^
include/linux/compiler.h:56:47: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^~~~
include/linux/compiler.h:58:52: note: expanded from macro '__trace_if_var'
#define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
^~~~
drivers/acpi/pci_root.c:432:2: note: '?' condition is false
if (!root)
^
include/linux/compiler.h:56:28: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^
include/linux/compiler.h:58:31: note: expanded from macro '__trace_if_var'
#define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
^
drivers/acpi/pci_root.c:432:7: note: 'root' is non-null
if (!root)
^
include/linux/compiler.h:56:47: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^~~~
include/linux/compiler.h:58:86: note: expanded from macro '__trace_if_var'
#define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
^~~~
include/linux/compiler.h:69:3: note: expanded from macro '__trace_if_value'
(cond) ? \
^~~~
drivers/acpi/pci_root.c:432:2: note: '?' condition is false
if (!root)
^
include/linux/compiler.h:56:28: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^
include/linux/compiler.h:58:69: note: expanded from macro '__trace_if_var'
#define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
^
include/linux/compiler.h:69:2: note: expanded from macro '__trace_if_value'
(cond) ? \
^
drivers/acpi/pci_root.c:432:2: note: Taking false branch
if (!root)
^
include/linux/compiler.h:56:23: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^
drivers/acpi/pci_root.c:438:2: note: '?' condition is false
if (is_cxl(root)) {
--
^
include/linux/compiler.h:58:69: note: expanded from macro '__trace_if_var'
#define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
^
include/linux/compiler.h:69:2: note: expanded from macro '__trace_if_value'
(cond) ? \
^
drivers/acpi/pci_root.c:447:3: note: Taking false branch
if (ACPI_FAILURE(status))
^
include/linux/compiler.h:56:23: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^
drivers/acpi/pci_root.c:449:3: note: '?' condition is false
if (is_cxl(root)) {
^
include/linux/compiler.h:56:28: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^
include/linux/compiler.h:58:31: note: expanded from macro '__trace_if_var'
#define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
^
drivers/acpi/pci_root.c:449:3: note: '?' condition is false
if (is_cxl(root)) {
^
include/linux/compiler.h:56:28: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^
include/linux/compiler.h:58:69: note: expanded from macro '__trace_if_var'
#define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
^
include/linux/compiler.h:69:2: note: expanded from macro '__trace_if_value'
(cond) ? \
^
drivers/acpi/pci_root.c:449:3: note: Taking false branch
if (is_cxl(root)) {
^
include/linux/compiler.h:56:23: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^
drivers/acpi/pci_root.c:455:8: note: Assuming the condition is true
if (ctrl == *mask)
^
include/linux/compiler.h:56:47: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^~~~
include/linux/compiler.h:58:52: note: expanded from macro '__trace_if_var'
#define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
^~~~
drivers/acpi/pci_root.c:455:4: note: '?' condition is false
if (ctrl == *mask)
^
include/linux/compiler.h:56:28: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^
include/linux/compiler.h:58:31: note: expanded from macro '__trace_if_var'
#define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
^
drivers/acpi/pci_root.c:455:4: note: '?' condition is true
if (ctrl == *mask)
^
include/linux/compiler.h:56:28: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^
include/linux/compiler.h:58:69: note: expanded from macro '__trace_if_var'
#define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
^
include/linux/compiler.h:69:2: note: expanded from macro '__trace_if_value'
(cond) ? \
^
drivers/acpi/pci_root.c:455:4: note: Taking true branch
if (ctrl == *mask)
^
include/linux/compiler.h:56:23: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^
drivers/acpi/pci_root.c:456:5: note: Execution continues on line 465
break;
^
drivers/acpi/pci_root.c:465:7: note: Assuming the condition is true
if (((root->osc_control_set & ctrl) == ctrl) &&
^
include/linux/compiler.h:56:47: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^~~~
include/linux/compiler.h:58:52: note: expanded from macro '__trace_if_var'
#define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
^~~~
drivers/acpi/pci_root.c:465:6: note: Left side of '&&' is true
if (((root->osc_control_set & ctrl) == ctrl) &&
^
drivers/acpi/pci_root.c:466:34: note: The right operand of '&' is a garbage value
((root->cxl_osc_control_set & cxl_ctrl) == cxl_ctrl))
^
include/linux/compiler.h:56:47: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^~~~
include/linux/compiler.h:58:52: note: expanded from macro '__trace_if_var'
#define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
^~~~
>> drivers/acpi/pci_root.c:564:2: warning: Undefined or garbage value returned to caller [clang-analyzer-core.uninitialized.UndefReturn]
return control;
^
drivers/acpi/pci_root.c:598:2: note: Assuming 'x86_apple_machine' is false
if (x86_apple_machine) {
^
include/linux/compiler.h:56:45: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/compiler.h:58:52: note: expanded from macro '__trace_if_var'
#define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
^~~~
drivers/acpi/pci_root.c:598:2: note: '?' condition is false
if (x86_apple_machine) {
^
include/linux/compiler.h:56:28: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^
include/linux/compiler.h:58:31: note: expanded from macro '__trace_if_var'
#define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
^
drivers/acpi/pci_root.c:598:6: note: 'x86_apple_machine' is false
if (x86_apple_machine) {
^
include/linux/compiler.h:56:47: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^~~~
include/linux/compiler.h:58:86: note: expanded from macro '__trace_if_var'
#define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
^~~~
include/linux/compiler.h:69:3: note: expanded from macro '__trace_if_value'
(cond) ? \
^~~~
drivers/acpi/pci_root.c:598:2: note: '?' condition is false
if (x86_apple_machine) {
^
include/linux/compiler.h:56:28: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^
include/linux/compiler.h:58:69: note: expanded from macro '__trace_if_var'
#define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
^
include/linux/compiler.h:69:2: note: expanded from macro '__trace_if_value'
(cond) ? \
^
drivers/acpi/pci_root.c:598:2: note: Taking false branch
if (x86_apple_machine) {
^
include/linux/compiler.h:56:23: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^
drivers/acpi/pci_root.c:609:2: note: '?' condition is false
if (os_control_query_checks(root, support))
^
include/linux/compiler.h:56:28: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^
include/linux/compiler.h:58:31: note: expanded from macro '__trace_if_var'
#define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
^
drivers/acpi/pci_root.c:609:2: note: '?' condition is false
if (os_control_query_checks(root, support))
^
include/linux/compiler.h:56:28: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^
include/linux/compiler.h:58:69: note: expanded from macro '__trace_if_var'
#define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
^
include/linux/compiler.h:69:2: note: expanded from macro '__trace_if_value'
(cond) ? \
^
drivers/acpi/pci_root.c:609:2: note: Taking false branch
if (os_control_query_checks(root, support))
^
include/linux/compiler.h:56:23: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^
drivers/acpi/pci_root.c:612:2: note: '?' condition is false
if (is_cxl(root)) {
^
include/linux/compiler.h:56:28: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^
include/linux/compiler.h:58:31: note: expanded from macro '__trace_if_var'
#define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
^
drivers/acpi/pci_root.c:612:2: note: '?' condition is true
if (is_cxl(root)) {
^
include/linux/compiler.h:56:28: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^
include/linux/compiler.h:58:69: note: expanded from macro '__trace_if_var'
#define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
^
include/linux/compiler.h:69:2: note: expanded from macro '__trace_if_value'
(cond) ? \
^
drivers/acpi/pci_root.c:612:2: note: Taking true branch
if (is_cxl(root)) {
vim +283 drivers/acpi/pci_root.c
63f10f0f6df4e4 Kenji Kaneshige 2009-02-09 256
8020d862c22d52 Vishal Verma 2022-03-16 257 static acpi_status acpi_pci_query_osc(struct acpi_pci_root *root, u32 support,
8020d862c22d52 Vishal Verma 2022-03-16 258 u32 *control, u32 cxl_support,
8020d862c22d52 Vishal Verma 2022-03-16 259 u32 *cxl_control)
63f10f0f6df4e4 Kenji Kaneshige 2009-02-09 260 {
63f10f0f6df4e4 Kenji Kaneshige 2009-02-09 261 acpi_status status;
8020d862c22d52 Vishal Verma 2022-03-16 262 u32 pci_result, cxl_result, capbuf[8];
ab8e8957a2ae21 Rafael J. Wysocki 2010-08-21 263
ab8e8957a2ae21 Rafael J. Wysocki 2010-08-21 264 support |= root->osc_support_set;
63f10f0f6df4e4 Kenji Kaneshige 2009-02-09 265
b938a229c85a56 Bjorn Helgaas 2013-09-05 266 capbuf[OSC_QUERY_DWORD] = OSC_QUERY_ENABLE;
b938a229c85a56 Bjorn Helgaas 2013-09-05 267 capbuf[OSC_SUPPORT_DWORD] = support;
b938a229c85a56 Bjorn Helgaas 2013-09-05 268 capbuf[OSC_CONTROL_DWORD] = *control | root->osc_control_set;
63f10f0f6df4e4 Kenji Kaneshige 2009-02-09 269
8020d862c22d52 Vishal Verma 2022-03-16 270 if (is_cxl(root)) {
8020d862c22d52 Vishal Verma 2022-03-16 271 cxl_support |= root->cxl_osc_support_set;
8020d862c22d52 Vishal Verma 2022-03-16 272 capbuf[OSC_CXL_SUPPORT_DWORD] = cxl_support;
8020d862c22d52 Vishal Verma 2022-03-16 273 capbuf[OSC_CXL_CONTROL_DWORD] = *cxl_control | root->cxl_osc_control_set;
8020d862c22d52 Vishal Verma 2022-03-16 274 }
8020d862c22d52 Vishal Verma 2022-03-16 275
53da48751c13ff Dan Williams 2022-03-16 276 retry:
8020d862c22d52 Vishal Verma 2022-03-16 277 status = acpi_pci_run_osc(root, capbuf, &pci_result, &cxl_result);
63f10f0f6df4e4 Kenji Kaneshige 2009-02-09 278 if (ACPI_SUCCESS(status)) {
ab8e8957a2ae21 Rafael J. Wysocki 2010-08-21 279 root->osc_support_set = support;
8020d862c22d52 Vishal Verma 2022-03-16 280 *control = pci_result;
8020d862c22d52 Vishal Verma 2022-03-16 281 if (is_cxl(root)) {
8020d862c22d52 Vishal Verma 2022-03-16 282 root->cxl_osc_support_set = cxl_support;
8020d862c22d52 Vishal Verma 2022-03-16 @283 *cxl_control = cxl_result;
8020d862c22d52 Vishal Verma 2022-03-16 284 }
53da48751c13ff Dan Williams 2022-03-16 285 } else if (is_cxl(root)) {
53da48751c13ff Dan Williams 2022-03-16 286 /*
53da48751c13ff Dan Williams 2022-03-16 287 * CXL _OSC is optional on CXL 1.1 hosts. Fall back to PCIe _OSC
53da48751c13ff Dan Williams 2022-03-16 288 * upon any failure using CXL _OSC.
53da48751c13ff Dan Williams 2022-03-16 289 */
53da48751c13ff Dan Williams 2022-03-16 290 root->cxl_osc_disable = true;
53da48751c13ff Dan Williams 2022-03-16 291 goto retry;
ab8e8957a2ae21 Rafael J. Wysocki 2010-08-21 292 }
63f10f0f6df4e4 Kenji Kaneshige 2009-02-09 293 return status;
63f10f0f6df4e4 Kenji Kaneshige 2009-02-09 294 }
63f10f0f6df4e4 Kenji Kaneshige 2009-02-09 295
---
0-DAY CI Kernel Test Service
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH 2/2] acpi/pci_root: negotiate CXL _OSC
@ 2022-03-17 5:48 kernel test robot
0 siblings, 0 replies; 14+ messages in thread
From: kernel test robot @ 2022-03-17 5:48 UTC (permalink / raw)
To: kbuild
[-- Attachment #1: Type: text/plain, Size: 25259 bytes --]
CC: kbuild-all(a)lists.01.org
BCC: lkp(a)intel.com
In-Reply-To: <20220317002704.1835870-3-vishal.l.verma@intel.com>
References: <20220317002704.1835870-3-vishal.l.verma@intel.com>
TO: Vishal Verma <vishal.l.verma@intel.com>
Hi Vishal,
[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on 74be98774dfbc5b8b795db726bd772e735d2edd4]
url: https://github.com/0day-ci/linux/commits/Vishal-Verma/acpi-add-support-for-CXL-_OSC/20220317-082840
base: 74be98774dfbc5b8b795db726bd772e735d2edd4
:::::: branch date: 5 hours ago
:::::: commit date: 5 hours ago
config: x86_64-randconfig-m001 (https://download.01.org/0day-ci/archive/20220317/202203171337.1O6VuoTj-lkp(a)intel.com/config)
compiler: gcc-9 (Ubuntu 9.4.0-1ubuntu1~20.04) 9.4.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
New smatch warnings:
drivers/acpi/pci_root.c:450 acpi_pci_osc_control_set() error: uninitialized symbol 'cxl_ctrl'.
drivers/acpi/pci_root.c:564 calculate_cxl_control() error: uninitialized symbol 'control'.
drivers/acpi/pci_root.c:619 negotiate_os_control() error: uninitialized symbol 'cxl_support'.
Old smatch warnings:
drivers/acpi/pci_root.c:453 acpi_pci_osc_control_set() error: uninitialized symbol 'cxl_ctrl'.
drivers/acpi/pci_root.c:466 acpi_pci_osc_control_set() error: uninitialized symbol 'cxl_ctrl'.
drivers/acpi/pci_root.c:480 acpi_pci_osc_control_set() error: uninitialized symbol 'cxl_ctrl'.
vim +/cxl_ctrl +450 drivers/acpi/pci_root.c
2f7bbceb5b6aa93 Alexander Chiang 2009-06-10 401
63f10f0f6df4e4e Kenji Kaneshige 2009-02-09 402 /**
75fb60f26befb59 Rafael J. Wysocki 2010-08-23 403 * acpi_pci_osc_control_set - Request control of PCI root _OSC features.
75fb60f26befb59 Rafael J. Wysocki 2010-08-23 404 * @handle: ACPI handle of a PCI root bridge (or PCIe Root Complex).
75fb60f26befb59 Rafael J. Wysocki 2010-08-23 405 * @mask: Mask of _OSC bits to request control of, place to store control mask.
843438deebe247f Yang Li 2021-12-23 406 * @support: _OSC supported capability.
8020d862c22d52b Vishal Verma 2022-03-16 407 * @cxl_mask: Mask of CXL _OSC control bits, place to store control mask.
8020d862c22d52b Vishal Verma 2022-03-16 408 * @cxl_support: CXL _OSC supported capability.
63f10f0f6df4e4e Kenji Kaneshige 2009-02-09 409 *
75fb60f26befb59 Rafael J. Wysocki 2010-08-23 410 * Run _OSC query for @mask and if that is successful, compare the returned
75fb60f26befb59 Rafael J. Wysocki 2010-08-23 411 * mask of control bits with @req. If all of the @req bits are set in the
75fb60f26befb59 Rafael J. Wysocki 2010-08-23 412 * returned mask, run _OSC request for it.
75fb60f26befb59 Rafael J. Wysocki 2010-08-23 413 *
75fb60f26befb59 Rafael J. Wysocki 2010-08-23 414 * The variable at the @mask address may be modified regardless of whether or
75fb60f26befb59 Rafael J. Wysocki 2010-08-23 415 * not the function returns success. On success it will contain the mask of
75fb60f26befb59 Rafael J. Wysocki 2010-08-23 416 * _OSC bits the BIOS has granted control of, but its contents are meaningless
75fb60f26befb59 Rafael J. Wysocki 2010-08-23 417 * on failure.
63f10f0f6df4e4e Kenji Kaneshige 2009-02-09 418 **/
8020d862c22d52b Vishal Verma 2022-03-16 419 static acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask,
8020d862c22d52b Vishal Verma 2022-03-16 420 u32 support, u32 *cxl_mask,
8020d862c22d52b Vishal Verma 2022-03-16 421 u32 cxl_support)
63f10f0f6df4e4e Kenji Kaneshige 2009-02-09 422 {
6bc779ee05d4fa6 Joerg Roedel 2021-08-24 423 u32 req = OSC_PCI_EXPRESS_CAPABILITY_CONTROL;
75fb60f26befb59 Rafael J. Wysocki 2010-08-23 424 struct acpi_pci_root *root;
866e61fc40c96e7 Bjorn Helgaas 2020-06-02 425 acpi_status status;
8020d862c22d52b Vishal Verma 2022-03-16 426 u32 ctrl, cxl_ctrl, capbuf[8];
63f10f0f6df4e4e Kenji Kaneshige 2009-02-09 427
75fb60f26befb59 Rafael J. Wysocki 2010-08-23 428 if (!mask)
75fb60f26befb59 Rafael J. Wysocki 2010-08-23 429 return AE_BAD_PARAMETER;
75fb60f26befb59 Rafael J. Wysocki 2010-08-23 430
63f10f0f6df4e4e Kenji Kaneshige 2009-02-09 431 root = acpi_pci_find_root(handle);
63f10f0f6df4e4e Kenji Kaneshige 2009-02-09 432 if (!root)
63f10f0f6df4e4e Kenji Kaneshige 2009-02-09 433 return AE_NOT_EXIST;
63f10f0f6df4e4e Kenji Kaneshige 2009-02-09 434
6bc779ee05d4fa6 Joerg Roedel 2021-08-24 435 ctrl = *mask;
6bc779ee05d4fa6 Joerg Roedel 2021-08-24 436 *mask |= root->osc_control_set;
63f10f0f6df4e4e Kenji Kaneshige 2009-02-09 437
8020d862c22d52b Vishal Verma 2022-03-16 438 if (is_cxl(root)) {
8020d862c22d52b Vishal Verma 2022-03-16 439 cxl_ctrl = *cxl_mask;
8020d862c22d52b Vishal Verma 2022-03-16 440 *mask |= root->osc_control_set;
8020d862c22d52b Vishal Verma 2022-03-16 441 }
8020d862c22d52b Vishal Verma 2022-03-16 442
75fb60f26befb59 Rafael J. Wysocki 2010-08-23 443 /* Need to check the available controls bits before requesting them. */
6bc779ee05d4fa6 Joerg Roedel 2021-08-24 444 do {
8020d862c22d52b Vishal Verma 2022-03-16 445 status = acpi_pci_query_osc(root, support, mask, cxl_support,
8020d862c22d52b Vishal Verma 2022-03-16 446 cxl_mask);
63f10f0f6df4e4e Kenji Kaneshige 2009-02-09 447 if (ACPI_FAILURE(status))
866e61fc40c96e7 Bjorn Helgaas 2020-06-02 448 return status;
8020d862c22d52b Vishal Verma 2022-03-16 449 if (is_cxl(root)) {
8020d862c22d52b Vishal Verma 2022-03-16 @450 if ((ctrl == *mask) && (cxl_ctrl == *cxl_mask))
8020d862c22d52b Vishal Verma 2022-03-16 451 break;
8020d862c22d52b Vishal Verma 2022-03-16 452 decode_cxl_osc_control(root, "platform does not support",
8020d862c22d52b Vishal Verma 2022-03-16 453 cxl_ctrl & ~(*cxl_mask));
8020d862c22d52b Vishal Verma 2022-03-16 454 } else {
75fb60f26befb59 Rafael J. Wysocki 2010-08-23 455 if (ctrl == *mask)
75fb60f26befb59 Rafael J. Wysocki 2010-08-23 456 break;
955f14b4ed0648d Bjorn Helgaas 2013-09-05 457 decode_osc_control(root, "platform does not support",
955f14b4ed0648d Bjorn Helgaas 2013-09-05 458 ctrl & ~(*mask));
8020d862c22d52b Vishal Verma 2022-03-16 459 }
75fb60f26befb59 Rafael J. Wysocki 2010-08-23 460 ctrl = *mask;
8020d862c22d52b Vishal Verma 2022-03-16 461 cxl_ctrl = *cxl_mask;
8020d862c22d52b Vishal Verma 2022-03-16 462 } while (*mask || *cxl_mask);
6bc779ee05d4fa6 Joerg Roedel 2021-08-24 463
6bc779ee05d4fa6 Joerg Roedel 2021-08-24 464 /* No need to request _OSC if the control was already granted. */
8020d862c22d52b Vishal Verma 2022-03-16 465 if (((root->osc_control_set & ctrl) == ctrl) &&
8020d862c22d52b Vishal Verma 2022-03-16 466 ((root->cxl_osc_control_set & cxl_ctrl) == cxl_ctrl))
6bc779ee05d4fa6 Joerg Roedel 2021-08-24 467 return AE_OK;
2b8fd9186d9275b Rafael J. Wysocki 2010-08-23 468
75fb60f26befb59 Rafael J. Wysocki 2010-08-23 469 if ((ctrl & req) != req) {
955f14b4ed0648d Bjorn Helgaas 2013-09-05 470 decode_osc_control(root, "not requesting control; platform does not support",
955f14b4ed0648d Bjorn Helgaas 2013-09-05 471 req & ~(ctrl));
866e61fc40c96e7 Bjorn Helgaas 2020-06-02 472 return AE_SUPPORT;
63f10f0f6df4e4e Kenji Kaneshige 2009-02-09 473 }
63f10f0f6df4e4e Kenji Kaneshige 2009-02-09 474
b938a229c85a567 Bjorn Helgaas 2013-09-05 475 capbuf[OSC_QUERY_DWORD] = 0;
b938a229c85a567 Bjorn Helgaas 2013-09-05 476 capbuf[OSC_SUPPORT_DWORD] = root->osc_support_set;
b938a229c85a567 Bjorn Helgaas 2013-09-05 477 capbuf[OSC_CONTROL_DWORD] = ctrl;
8020d862c22d52b Vishal Verma 2022-03-16 478 if (is_cxl(root)) {
8020d862c22d52b Vishal Verma 2022-03-16 479 capbuf[OSC_CXL_SUPPORT_DWORD] = root->cxl_osc_support_set;
8020d862c22d52b Vishal Verma 2022-03-16 480 capbuf[OSC_CXL_CONTROL_DWORD] = cxl_ctrl;
8020d862c22d52b Vishal Verma 2022-03-16 481 }
8020d862c22d52b Vishal Verma 2022-03-16 482
8020d862c22d52b Vishal Verma 2022-03-16 483 status = acpi_pci_run_osc(root, capbuf, mask, cxl_mask);
866e61fc40c96e7 Bjorn Helgaas 2020-06-02 484 if (ACPI_FAILURE(status))
63f10f0f6df4e4e Kenji Kaneshige 2009-02-09 485 return status;
866e61fc40c96e7 Bjorn Helgaas 2020-06-02 486
866e61fc40c96e7 Bjorn Helgaas 2020-06-02 487 root->osc_control_set = *mask;
8020d862c22d52b Vishal Verma 2022-03-16 488 root->cxl_osc_control_set = *cxl_mask;
866e61fc40c96e7 Bjorn Helgaas 2020-06-02 489 return AE_OK;
63f10f0f6df4e4e Kenji Kaneshige 2009-02-09 490 }
63f10f0f6df4e4e Kenji Kaneshige 2009-02-09 491
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 492 static u32 calculate_support(void)
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 493 {
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 494 u32 support;
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 495
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 496 /*
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 497 * All supported architectures that use ACPI have support for
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 498 * PCI domains, so we indicate this in _OSC support capabilities.
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 499 */
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 500 support = OSC_PCI_SEGMENT_GROUPS_SUPPORT;
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 501 support |= OSC_PCI_HPX_TYPE_3_SUPPORT;
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 502 if (pci_ext_cfg_avail())
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 503 support |= OSC_PCI_EXT_CONFIG_SUPPORT;
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 504 if (pcie_aspm_support_enabled())
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 505 support |= OSC_PCI_ASPM_SUPPORT | OSC_PCI_CLOCK_PM_SUPPORT;
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 506 if (pci_msi_enabled())
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 507 support |= OSC_PCI_MSI_SUPPORT;
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 508 if (IS_ENABLED(CONFIG_PCIE_EDR))
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 509 support |= OSC_PCI_EDR_SUPPORT;
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 510
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 511 return support;
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 512 }
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 513
8020d862c22d52b Vishal Verma 2022-03-16 514 static u32 calculate_cxl_support(void)
8020d862c22d52b Vishal Verma 2022-03-16 515 {
8020d862c22d52b Vishal Verma 2022-03-16 516 u32 support;
8020d862c22d52b Vishal Verma 2022-03-16 517
8020d862c22d52b Vishal Verma 2022-03-16 518 support = OSC_CXL_2_0_PORT_DEV_REG_ACCESS_SUPPORT;
8020d862c22d52b Vishal Verma 2022-03-16 519 support |= OSC_CXL_PER_SUPPORT;
8020d862c22d52b Vishal Verma 2022-03-16 520 if (IS_ENABLED(CONFIG_HOTPLUG_PCI_PCIE))
8020d862c22d52b Vishal Verma 2022-03-16 521 support |= OSC_CXL_NATIVE_HP_SUPPORT;
8020d862c22d52b Vishal Verma 2022-03-16 522
8020d862c22d52b Vishal Verma 2022-03-16 523 return support;
8020d862c22d52b Vishal Verma 2022-03-16 524 }
8020d862c22d52b Vishal Verma 2022-03-16 525
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 526 static u32 calculate_control(void)
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 527 {
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 528 u32 control;
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 529
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 530 control = OSC_PCI_EXPRESS_CAPABILITY_CONTROL
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 531 | OSC_PCI_EXPRESS_PME_CONTROL;
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 532
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 533 if (IS_ENABLED(CONFIG_PCIEASPM))
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 534 control |= OSC_PCI_EXPRESS_LTR_CONTROL;
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 535
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 536 if (IS_ENABLED(CONFIG_HOTPLUG_PCI_PCIE))
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 537 control |= OSC_PCI_EXPRESS_NATIVE_HP_CONTROL;
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 538
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 539 if (IS_ENABLED(CONFIG_HOTPLUG_PCI_SHPC))
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 540 control |= OSC_PCI_SHPC_NATIVE_HP_CONTROL;
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 541
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 542 if (pci_aer_available())
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 543 control |= OSC_PCI_EXPRESS_AER_CONTROL;
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 544
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 545 /*
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 546 * Per the Downstream Port Containment Related Enhancements ECN to
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 547 * the PCI Firmware Spec, r3.2, sec 4.5.1, table 4-5,
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 548 * OSC_PCI_EXPRESS_DPC_CONTROL indicates the OS supports both DPC
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 549 * and EDR.
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 550 */
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 551 if (IS_ENABLED(CONFIG_PCIE_DPC) && IS_ENABLED(CONFIG_PCIE_EDR))
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 552 control |= OSC_PCI_EXPRESS_DPC_CONTROL;
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 553
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 554 return control;
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 555 }
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 556
8020d862c22d52b Vishal Verma 2022-03-16 557 static u32 calculate_cxl_control(void)
8020d862c22d52b Vishal Verma 2022-03-16 558 {
8020d862c22d52b Vishal Verma 2022-03-16 559 u32 control;
8020d862c22d52b Vishal Verma 2022-03-16 560
8020d862c22d52b Vishal Verma 2022-03-16 561 if (pci_aer_available())
8020d862c22d52b Vishal Verma 2022-03-16 562 control |= OSC_CXL_ERROR_REPORTING_CONTROL;
8020d862c22d52b Vishal Verma 2022-03-16 563
8020d862c22d52b Vishal Verma 2022-03-16 @564 return control;
8020d862c22d52b Vishal Verma 2022-03-16 565 }
8020d862c22d52b Vishal Verma 2022-03-16 566
87f1f87a16818c3 Joerg Roedel 2021-08-24 567 static bool os_control_query_checks(struct acpi_pci_root *root, u32 support)
87f1f87a16818c3 Joerg Roedel 2021-08-24 568 {
87f1f87a16818c3 Joerg Roedel 2021-08-24 569 struct acpi_device *device = root->device;
87f1f87a16818c3 Joerg Roedel 2021-08-24 570
87f1f87a16818c3 Joerg Roedel 2021-08-24 571 if (pcie_ports_disabled) {
87f1f87a16818c3 Joerg Roedel 2021-08-24 572 dev_info(&device->dev, "PCIe port services disabled; not requesting _OSC control\n");
87f1f87a16818c3 Joerg Roedel 2021-08-24 573 return false;
87f1f87a16818c3 Joerg Roedel 2021-08-24 574 }
87f1f87a16818c3 Joerg Roedel 2021-08-24 575
87f1f87a16818c3 Joerg Roedel 2021-08-24 576 if ((support & ACPI_PCIE_REQ_SUPPORT) != ACPI_PCIE_REQ_SUPPORT) {
87f1f87a16818c3 Joerg Roedel 2021-08-24 577 decode_osc_support(root, "not requesting OS control; OS requires",
87f1f87a16818c3 Joerg Roedel 2021-08-24 578 ACPI_PCIE_REQ_SUPPORT);
87f1f87a16818c3 Joerg Roedel 2021-08-24 579 return false;
87f1f87a16818c3 Joerg Roedel 2021-08-24 580 }
87f1f87a16818c3 Joerg Roedel 2021-08-24 581
87f1f87a16818c3 Joerg Roedel 2021-08-24 582 return true;
87f1f87a16818c3 Joerg Roedel 2021-08-24 583 }
87f1f87a16818c3 Joerg Roedel 2021-08-24 584
53da48751c13ffc Dan Williams 2022-03-16 585 static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm)
^1da177e4c3f415 Linus Torvalds 2005-04-16 586 {
6bc779ee05d4fa6 Joerg Roedel 2021-08-24 587 u32 support, control = 0, requested = 0;
8020d862c22d52b Vishal Verma 2022-03-16 588 u32 cxl_support, cxl_control = 0, cxl_requested = 0;
3e43abb012d45dc Bjorn Helgaas 2013-09-05 589 acpi_status status;
3e43abb012d45dc Bjorn Helgaas 2013-09-05 590 struct acpi_device *device = root->device;
bfe2414aecca03d Jiang Liu 2013-05-28 591 acpi_handle handle = device->handle;
^1da177e4c3f415 Linus Torvalds 2005-04-16 592
7bc5a2bad0b8d9d Matthew Garrett 2014-09-20 593 /*
7bc5a2bad0b8d9d Matthew Garrett 2014-09-20 594 * Apple always return failure on _OSC calls when _OSI("Darwin") has
7bc5a2bad0b8d9d Matthew Garrett 2014-09-20 595 * been called successfully. We know the feature set supported by the
7bc5a2bad0b8d9d Matthew Garrett 2014-09-20 596 * platform, so avoid calling _OSC at all
7bc5a2bad0b8d9d Matthew Garrett 2014-09-20 597 */
630b3aff8a51c90 Lukas Wunner 2017-08-01 598 if (x86_apple_machine) {
7bc5a2bad0b8d9d Matthew Garrett 2014-09-20 599 root->osc_control_set = ~OSC_PCI_EXPRESS_PME_CONTROL;
7bc5a2bad0b8d9d Matthew Garrett 2014-09-20 600 decode_osc_control(root, "OS assumes control of",
7bc5a2bad0b8d9d Matthew Garrett 2014-09-20 601 root->osc_control_set);
7bc5a2bad0b8d9d Matthew Garrett 2014-09-20 602 return;
7bc5a2bad0b8d9d Matthew Garrett 2014-09-20 603 }
7bc5a2bad0b8d9d Matthew Garrett 2014-09-20 604
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 605 support = calculate_support();
955f14b4ed0648d Bjorn Helgaas 2013-09-05 606
955f14b4ed0648d Bjorn Helgaas 2013-09-05 607 decode_osc_support(root, "OS supports", support);
de18966228ed4b4 Bjorn Helgaas 2013-09-05 608
6bc779ee05d4fa6 Joerg Roedel 2021-08-24 609 if (os_control_query_checks(root, support))
4c6f6060b7c4fe0 Joerg Roedel 2021-08-24 610 requested = control = calculate_control();
ac1c8e35a3262d0 Kuppuswamy Sathyanarayanan 2020-03-23 611
8020d862c22d52b Vishal Verma 2022-03-16 612 if (is_cxl(root)) {
8020d862c22d52b Vishal Verma 2022-03-16 613 cxl_support = calculate_cxl_support();
8020d862c22d52b Vishal Verma 2022-03-16 614 decode_cxl_osc_support(root, "OS supports", cxl_support);
8020d862c22d52b Vishal Verma 2022-03-16 615 cxl_requested = cxl_control = calculate_cxl_control();
8020d862c22d52b Vishal Verma 2022-03-16 616 }
8020d862c22d52b Vishal Verma 2022-03-16 617
8020d862c22d52b Vishal Verma 2022-03-16 618 status = acpi_pci_osc_control_set(handle, &control, support,
8020d862c22d52b Vishal Verma 2022-03-16 @619 &cxl_control, cxl_support);
eca67315e0e0d5f Naga Chumbalkar 2011-03-21 620 if (ACPI_SUCCESS(status)) {
6bc779ee05d4fa6 Joerg Roedel 2021-08-24 621 if (control)
955f14b4ed0648d Bjorn Helgaas 2013-09-05 622 decode_osc_control(root, "OS now controls", control);
8020d862c22d52b Vishal Verma 2022-03-16 623 if (cxl_control)
8020d862c22d52b Vishal Verma 2022-03-16 624 decode_cxl_osc_control(root, "OS now controls",
8020d862c22d52b Vishal Verma 2022-03-16 625 cxl_control);
6bc779ee05d4fa6 Joerg Roedel 2021-08-24 626
b8178f130e25c1b Bjorn Helgaas 2013-04-01 627 if (acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_ASPM) {
b8178f130e25c1b Bjorn Helgaas 2013-04-01 628 /*
387d37577fdd05e Matthew Garrett 2015-04-07 629 * We have ASPM control, but the FADT indicates that
387d37577fdd05e Matthew Garrett 2015-04-07 630 * it's unsupported. Leave existing configuration
387d37577fdd05e Matthew Garrett 2015-04-07 631 * intact and prevent the OS from touching it.
b8178f130e25c1b Bjorn Helgaas 2013-04-01 632 */
387d37577fdd05e Matthew Garrett 2015-04-07 633 dev_info(&device->dev, "FADT indicates ASPM is unsupported, using BIOS configuration\n");
387d37577fdd05e Matthew Garrett 2015-04-07 634 *no_aspm = 1;
b8178f130e25c1b Bjorn Helgaas 2013-04-01 635 }
eca67315e0e0d5f Naga Chumbalkar 2011-03-21 636 } else {
3dc48af310709b8 Neil Horman 2013-08-29 637 /*
3dc48af310709b8 Neil Horman 2013-08-29 638 * We want to disable ASPM here, but aspm_disabled
3dc48af310709b8 Neil Horman 2013-08-29 639 * needs to remain in its state from boot so that we
3dc48af310709b8 Neil Horman 2013-08-29 640 * properly handle PCIe 1.1 devices. So we set this
3dc48af310709b8 Neil Horman 2013-08-29 641 * flag here, to defer the action until after the ACPI
3dc48af310709b8 Neil Horman 2013-08-29 642 * root scan.
3dc48af310709b8 Neil Horman 2013-08-29 643 */
3e43abb012d45dc Bjorn Helgaas 2013-09-05 644 *no_aspm = 1;
6bc779ee05d4fa6 Joerg Roedel 2021-08-24 645
6bc779ee05d4fa6 Joerg Roedel 2021-08-24 646 /* _OSC is optional for PCI host bridges */
53da48751c13ffc Dan Williams 2022-03-16 647 if ((status == AE_NOT_FOUND) && !is_pcie(root))
6bc779ee05d4fa6 Joerg Roedel 2021-08-24 648 return;
6bc779ee05d4fa6 Joerg Roedel 2021-08-24 649
6bc779ee05d4fa6 Joerg Roedel 2021-08-24 650 if (control) {
6bc779ee05d4fa6 Joerg Roedel 2021-08-24 651 decode_osc_control(root, "OS requested", requested);
6bc779ee05d4fa6 Joerg Roedel 2021-08-24 652 decode_osc_control(root, "platform willing to grant", control);
6bc779ee05d4fa6 Joerg Roedel 2021-08-24 653 }
8020d862c22d52b Vishal Verma 2022-03-16 654 if (cxl_control) {
8020d862c22d52b Vishal Verma 2022-03-16 655 decode_cxl_osc_control(root, "OS requested", cxl_requested);
8020d862c22d52b Vishal Verma 2022-03-16 656 decode_cxl_osc_control(root, "platform willing to grant",
8020d862c22d52b Vishal Verma 2022-03-16 657 cxl_control);
8020d862c22d52b Vishal Verma 2022-03-16 658 }
6bc779ee05d4fa6 Joerg Roedel 2021-08-24 659
6bc779ee05d4fa6 Joerg Roedel 2021-08-24 660 dev_info(&device->dev, "_OSC: platform retains control of PCIe features (%s)\n",
6bc779ee05d4fa6 Joerg Roedel 2021-08-24 661 acpi_format_exception(status));
eca67315e0e0d5f Naga Chumbalkar 2011-03-21 662 }
3e43abb012d45dc Bjorn Helgaas 2013-09-05 663 }
3e43abb012d45dc Bjorn Helgaas 2013-09-05 664
---
0-DAY CI Kernel Test Service
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2022-03-18 21:16 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-17 0:27 [RFC PATCH 0/2] acpi: add support for CXL _OSC Vishal Verma
2022-03-17 0:27 ` [RFC PATCH 1/2] PCI/ACPI: Use CXL _OSC instead of PCIe _OSC Vishal Verma
2022-03-17 1:47 ` Dan Williams
2022-03-17 15:40 ` Jonathan Cameron
2022-03-17 0:27 ` [RFC PATCH 2/2] acpi/pci_root: negotiate CXL _OSC Vishal Verma
2022-03-17 3:19 ` Dan Williams
2022-03-17 3:49 ` Verma, Vishal L
2022-03-17 3:25 ` kernel test robot
2022-03-17 16:10 ` Jonathan Cameron
2022-03-18 21:16 ` Verma, Vishal L
2022-03-17 15:19 ` [RFC PATCH 0/2] acpi: add support for " Jonathan Cameron
2022-03-18 19:52 ` Verma, Vishal L
2022-03-17 5:48 [RFC PATCH 2/2] acpi/pci_root: negotiate " kernel test robot
2022-03-18 0:33 kernel test robot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.