[PATCH v2 0/5] thunderbolt / ACPI: Add support for USB4 _OSC

[PATCH v2 0/5] thunderbolt / ACPI: Add support for USB4 _OSC

[Mika Westerberg]

Hi all,

The just released ACPI 6.4 spec [1] adds a new _OSC method that is used to
negotiate OS support for native USB4 features such as PCIe tunneling. This
patch series adds Linux support for the new _OSC and modifies the
Thunderbolt/USB4 driver accordingly to enable/disable tunneling of
different protocols.

There is an additional setting in the firmware connection manager that
allows the BIOS to disable PCIe tunneling, so we add support for this and
also make the software connection manager to switch to this "nopcie"
security level when the _OSC does not allow PCIe tunneling.

This applies on top of thunderbolt.git/next.

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

The previous version of the patch series can be found here:

  https://lore.kernel.org/linux-usb/20210126155723.9388-1-mika.westerberg@linux.intel.com/

Changes from v1:

  * Dropped patch 1/6. I already applied it to thunderbolt.git/fixes
  * Added ACK from Yehezkel to TBT patches
  * Updated changelog of patch 1/5 and fixed typos too
  * Added Rafael's tag to patch 4/5.

Mario Limonciello (1):
  ACPI: Execute platform _OSC also with query bit clear

Mika Westerberg (4):
  thunderbolt: Add support for PCIe tunneling disabled (SL5)
  thunderbolt: Allow disabling XDomain protocol
  ACPI: Add support for native USB4 control _OSC
  thunderbolt: Add support for native USB4 _OSC

 .../ABI/testing/sysfs-bus-thunderbolt         |   2 +
 Documentation/admin-guide/thunderbolt.rst     |   7 ++
 drivers/acpi/bus.c                            | 119 ++++++++++++++++--
 drivers/thunderbolt/acpi.c                    |  65 ++++++++++
 drivers/thunderbolt/domain.c                  |  16 ++-
 drivers/thunderbolt/icm.c                     |   6 +-
 drivers/thunderbolt/nhi.c                     |  27 +++-
 drivers/thunderbolt/switch.c                  |   6 +-
 drivers/thunderbolt/tb.c                      |  22 +++-
 drivers/thunderbolt/tb.h                      |  13 ++
 drivers/thunderbolt/tunnel.c                  |  10 +-
 drivers/thunderbolt/usb4.c                    |  11 +-
 drivers/thunderbolt/xdomain.c                 |   9 ++
 include/linux/acpi.h                          |  10 ++
 include/linux/thunderbolt.h                   |   3 +
 15 files changed, 298 insertions(+), 28 deletions(-)

-- 
2.29.2

[PATCH v2 2/5] thunderbolt: Allow disabling XDomain protocol

[Mika Westerberg]

This allows disabling XDomain protocol completely if the user does not
plan to use the USB4/Thunderbolt peer-to-peer functionality, or for
security reasons.

XDomain protocol is enabled by default but with this commit it is
possible to disable it by passing "xdomain=0" as module parameter (or
through the kernel command line).

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Acked-by: Yehezkel Bernat <YehezkelShB@gmail.com>
---
 drivers/thunderbolt/domain.c  | 4 +++-
 drivers/thunderbolt/icm.c     | 6 ++++--
 drivers/thunderbolt/tb.c      | 3 +++
 drivers/thunderbolt/tb.h      | 1 +
 drivers/thunderbolt/xdomain.c | 9 +++++++++
 5 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/thunderbolt/domain.c b/drivers/thunderbolt/domain.c
index a1c79c9c4f66..89ae614eaba2 100644
--- a/drivers/thunderbolt/domain.c
+++ b/drivers/thunderbolt/domain.c
@@ -412,7 +412,9 @@ static bool tb_domain_event_cb(void *data, enum tb_cfg_pkg_type type,
 	switch (type) {
 	case TB_CFG_PKG_XDOMAIN_REQ:
 	case TB_CFG_PKG_XDOMAIN_RESP:
-		return tb_xdomain_handle_request(tb, type, buf, size);
+		if (tb_is_xdomain_enabled())
+			return tb_xdomain_handle_request(tb, type, buf, size);
+		break;
 
 	default:
 		tb->cm_ops->handle_event(tb, type, buf, size);
diff --git a/drivers/thunderbolt/icm.c b/drivers/thunderbolt/icm.c
index 14340ec61703..8cd7b3054d14 100644
--- a/drivers/thunderbolt/icm.c
+++ b/drivers/thunderbolt/icm.c
@@ -1701,10 +1701,12 @@ static void icm_handle_notification(struct work_struct *work)
 			icm->device_disconnected(tb, n->pkg);
 			break;
 		case ICM_EVENT_XDOMAIN_CONNECTED:
-			icm->xdomain_connected(tb, n->pkg);
+			if (tb_is_xdomain_enabled())
+				icm->xdomain_connected(tb, n->pkg);
 			break;
 		case ICM_EVENT_XDOMAIN_DISCONNECTED:
-			icm->xdomain_disconnected(tb, n->pkg);
+			if (tb_is_xdomain_enabled())
+				icm->xdomain_disconnected(tb, n->pkg);
 			break;
 		case ICM_EVENT_RTD3_VETO:
 			icm->rtd3_veto(tb, n->pkg);
diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
index 413955aa6a94..ad3c285026d5 100644
--- a/drivers/thunderbolt/tb.c
+++ b/drivers/thunderbolt/tb.c
@@ -179,6 +179,9 @@ static void tb_scan_xdomain(struct tb_port *port)
 	struct tb_xdomain *xd;
 	u64 route;
 
+	if (!tb_is_xdomain_enabled())
+		return;
+
 	route = tb_downstream_route(port);
 	xd = tb_xdomain_find_by_route(tb, route);
 	if (xd) {
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index 31468de658e4..01fe849e9d4c 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -953,6 +953,7 @@ static inline u64 tb_downstream_route(struct tb_port *port)
 	       | ((u64) port->port << (port->sw->config.depth * 8));
 }
 
+bool tb_is_xdomain_enabled(void);
 bool tb_xdomain_handle_request(struct tb *tb, enum tb_cfg_pkg_type type,
 			       const void *buf, size_t size);
 struct tb_xdomain *tb_xdomain_alloc(struct tb *tb, struct device *parent,
diff --git a/drivers/thunderbolt/xdomain.c b/drivers/thunderbolt/xdomain.c
index 6e8bea6a7d39..b242161868c2 100644
--- a/drivers/thunderbolt/xdomain.c
+++ b/drivers/thunderbolt/xdomain.c
@@ -30,6 +30,10 @@ struct xdomain_request_work {
 	struct tb *tb;
 };
 
+static bool tb_xdomain_enabled = true;
+module_param_named(xdomain, tb_xdomain_enabled, bool, 0444);
+MODULE_PARM_DESC(xdomain, "allow XDomain protocol (default: true)");
+
 /* Serializes access to the properties and protocol handlers below */
 static DEFINE_MUTEX(xdomain_lock);
 
@@ -47,6 +51,11 @@ static const uuid_t tb_xdp_uuid =
 	UUID_INIT(0xb638d70e, 0x42ff, 0x40bb,
 		  0x97, 0xc2, 0x90, 0xe2, 0xc0, 0xb2, 0xff, 0x07);
 
+bool tb_is_xdomain_enabled(void)
+{
+	return tb_xdomain_enabled;
+}
+
 static bool tb_xdomain_match(const struct tb_cfg_request *req,
 			     const struct ctl_pkg *pkg)
 {
-- 
2.29.2

[PATCH v2 4/5] ACPI: Add support for native USB4 control _OSC

[Mika Westerberg]

ACPI 6.4 introduced a new _OSC capability that is used negotiate native
connection manager support. Connection manager is the entity that is
responsible for tunneling over the USB4 fabric. If the platform rejects
the native access then firmware based connection manager is used.

The new _OSC also includes a set of bits that can be used to disable
certain tunnel types such as PCIe for security reasons for instance.

This implements the new USB4 _OSC so that we try to negotiate native
USB4 support if the Thunderbolt/USB4 (CONFIG_USB4) driver is enabled.
Drivers can determine what was negotiated by checking two new variables
exposed in this patch.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/bus.c   | 76 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/acpi.h | 10 ++++++
 2 files changed, 86 insertions(+)

diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index a52cb28c40d8..9c3fe08e8f18 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -281,6 +281,12 @@ bool osc_sb_apei_support_acked;
 bool osc_pc_lpi_support_confirmed;
 EXPORT_SYMBOL_GPL(osc_pc_lpi_support_confirmed);
 
+/*
+ * ACPI 6.4 Operating System Capabilities for USB.
+ */
+bool osc_sb_native_usb4_support_confirmed;
+EXPORT_SYMBOL_GPL(osc_sb_native_usb4_support_confirmed);
+
 static u8 sb_uuid_str[] = "0811B06E-4A27-44F9-8D60-3CBBC22E7B48";
 static void acpi_bus_osc_negotiate_platform_control(void)
 {
@@ -317,6 +323,9 @@ static void acpi_bus_osc_negotiate_platform_control(void)
 	if (IS_ENABLED(CONFIG_SCHED_MC_PRIO))
 		capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_CPC_DIVERSE_HIGH_SUPPORT;
 
+	if (IS_ENABLED(CONFIG_USB4))
+		capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_NATIVE_USB4_SUPPORT;
+
 	if (!ghes_disable)
 		capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_APEI_SUPPORT;
 	if (ACPI_FAILURE(acpi_get_handle(NULL, "\\_SB", &handle)))
@@ -348,8 +357,74 @@ static void acpi_bus_osc_negotiate_platform_control(void)
 			capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_APEI_SUPPORT;
 		osc_pc_lpi_support_confirmed =
 			capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_PCLPI_SUPPORT;
+		osc_sb_native_usb4_support_confirmed =
+			capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_NATIVE_USB4_SUPPORT;
+	}
+
+	kfree(context.ret.pointer);
+}
+
+/*
+ * Native control of USB4 capabilities. If any of the tunneling bits is
+ * set it means OS is in control and we use software based connection
+ * manager.
+ */
+u32 osc_sb_native_usb4_control;
+EXPORT_SYMBOL_GPL(osc_sb_native_usb4_control);
+
+static void acpi_bus_decode_usb_osc(const char *msg, u32 bits)
+{
+	printk(KERN_INFO PREFIX "%s USB3%c DisplayPort%c PCIe%c XDomain%c\n", msg,
+	       (bits & OSC_USB_USB3_TUNNELING) ? '+' : '-',
+	       (bits & OSC_USB_DP_TUNNELING) ? '+' : '-',
+	       (bits & OSC_USB_PCIE_TUNNELING) ? '+' : '-',
+	       (bits & OSC_USB_XDOMAIN) ? '+' : '-');
+}
+
+static u8 sb_usb_uuid_str[] = "23A0D13A-26AB-486C-9C5F-0FFA525A575A";
+static void acpi_bus_osc_negotiate_usb_control(void)
+{
+	u32 capbuf[3];
+	struct acpi_osc_context context = {
+		.uuid_str = sb_usb_uuid_str,
+		.rev = 1,
+		.cap.length = sizeof(capbuf),
+		.cap.pointer = capbuf,
+	};
+	acpi_handle handle;
+	acpi_status status;
+	u32 control;
+
+	if (!osc_sb_native_usb4_support_confirmed)
+		return;
+
+	if (ACPI_FAILURE(acpi_get_handle(NULL, "\\_SB", &handle)))
+		return;
+
+	control = OSC_USB_USB3_TUNNELING | OSC_USB_DP_TUNNELING |
+		  OSC_USB_PCIE_TUNNELING | OSC_USB_XDOMAIN;
+
+	capbuf[OSC_QUERY_DWORD] = 0;
+	capbuf[OSC_SUPPORT_DWORD] = 0;
+	capbuf[OSC_CONTROL_DWORD] = control;
+
+	status = acpi_run_osc(handle, &context);
+	if (ACPI_FAILURE(status))
+		return;
+
+	if (context.ret.length != sizeof(capbuf)) {
+		printk(KERN_INFO PREFIX "USB4 _OSC: returned invalid length buffer\n");
+		goto out_free;
 	}
 
+	osc_sb_native_usb4_control =
+		control & ((u32 *)context.ret.pointer)[OSC_CONTROL_DWORD];
+
+	acpi_bus_decode_usb_osc("USB4 _OSC: OS supports", control);
+	acpi_bus_decode_usb_osc("USB4 _OSC: OS controls",
+				osc_sb_native_usb4_control);
+
+out_free:
 	kfree(context.ret.pointer);
 }
 
@@ -1188,6 +1263,7 @@ static int __init acpi_bus_init(void)
 	 * so it must be run after ACPI_FULL_INITIALIZATION
 	 */
 	acpi_bus_osc_negotiate_platform_control();
+	acpi_bus_osc_negotiate_usb_control();
 
 	/*
 	 * _PDC control method may load dynamic SSDT tables,
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 2630c2e953f7..ac68c2d4e393 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -546,9 +546,19 @@ acpi_status acpi_run_osc(acpi_handle handle, struct acpi_osc_context *context);
 #define OSC_SB_OSLPI_SUPPORT			0x00000100
 #define OSC_SB_CPC_DIVERSE_HIGH_SUPPORT		0x00001000
 #define OSC_SB_GENERIC_INITIATOR_SUPPORT	0x00002000
+#define OSC_SB_NATIVE_USB4_SUPPORT		0x00040000
 
 extern bool osc_sb_apei_support_acked;
 extern bool osc_pc_lpi_support_confirmed;
+extern bool osc_sb_native_usb4_support_confirmed;
+
+/* USB4 Capabilities */
+#define OSC_USB_USB3_TUNNELING			0x00000001
+#define OSC_USB_DP_TUNNELING			0x00000002
+#define OSC_USB_PCIE_TUNNELING			0x00000004
+#define OSC_USB_XDOMAIN				0x00000008
+
+extern u32 osc_sb_native_usb4_control;
 
 /* PCI Host Bridge _OSC: Capabilities DWORD 2: Support Field */
 #define OSC_PCI_EXT_CONFIG_SUPPORT		0x00000001
-- 
2.29.2

Re: [PATCH v2 3/5] ACPI: Execute platform _OSC also with query bit clear

[Mika Westerberg]

Hi Rafael,

I wonder if you are OK with this patch?

Thanks!

On Fri, Jan 29, 2021 at 11:32:39AM +0300, Mika Westerberg wrote:
> From: Mario Limonciello <mario.limonciello@dell.com>
> 
> The platform _OSC can change the hardware state when query bit is not
> set. According to ACPI spec it is recommended that the OS runs _OSC with
> query bit set until the platform does not mask any of the capabilities.
> Then it should run it with query bit clear in order to actually commit
> the changes. Linux has not been doing this for the reasons that there
> has not been anything to commit, until now.
> 
> The ACPI 6.4 introduced _OSC for USB4 to allow the OS to negotiate
> native control over USB4 tunneling. The platform might implement this so
> that it only activates the software connection manager path when the OS
> calls the _OSC with the query bit clear. Otherwise it may default to the
> firmware connection manager, for instance.
> 
> For this reason modify the _OSC support so that we first execute it with
> query bit set, then use the returned value as base of the features we
> want to control and run the _OSC again with query bit clear. This also
> follows what Windows is doing.
> 
> Also rename the function to better match what it does.
> 
> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---
>  drivers/acpi/bus.c | 43 +++++++++++++++++++++++++++++++------------
>  1 file changed, 31 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> index 1682f8b454a2..a52cb28c40d8 100644
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -282,9 +282,9 @@ bool osc_pc_lpi_support_confirmed;
>  EXPORT_SYMBOL_GPL(osc_pc_lpi_support_confirmed);
>  
>  static u8 sb_uuid_str[] = "0811B06E-4A27-44F9-8D60-3CBBC22E7B48";
> -static void acpi_bus_osc_support(void)
> +static void acpi_bus_osc_negotiate_platform_control(void)
>  {
> -	u32 capbuf[2];
> +	u32 capbuf[2], *capbuf_ret;
>  	struct acpi_osc_context context = {
>  		.uuid_str = sb_uuid_str,
>  		.rev = 1,
> @@ -321,17 +321,36 @@ static void acpi_bus_osc_support(void)
>  		capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_APEI_SUPPORT;
>  	if (ACPI_FAILURE(acpi_get_handle(NULL, "\\_SB", &handle)))
>  		return;
> -	if (ACPI_SUCCESS(acpi_run_osc(handle, &context))) {
> -		u32 *capbuf_ret = context.ret.pointer;
> -		if (context.ret.length > OSC_SUPPORT_DWORD) {
> -			osc_sb_apei_support_acked =
> -				capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_APEI_SUPPORT;
> -			osc_pc_lpi_support_confirmed =
> -				capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_PCLPI_SUPPORT;
> -		}
> +
> +	if (ACPI_FAILURE(acpi_run_osc(handle, &context)))
> +		return;
> +
> +	capbuf_ret = context.ret.pointer;
> +	if (context.ret.length <= OSC_SUPPORT_DWORD) {
>  		kfree(context.ret.pointer);
> +		return;
>  	}
> -	/* do we need to check other returned cap? Sounds no */
> +
> +	/*
> +	 * Now run _OSC again with query flag clear and with the caps
> +	 * supported by both the OS and the platform.
> +	 */
> +	capbuf[OSC_QUERY_DWORD] = 0;
> +	capbuf[OSC_SUPPORT_DWORD] = capbuf_ret[OSC_SUPPORT_DWORD];
> +	kfree(context.ret.pointer);
> +
> +	if (ACPI_FAILURE(acpi_run_osc(handle, &context)))
> +		return;
> +
> +	capbuf_ret = context.ret.pointer;
> +	if (context.ret.length > OSC_SUPPORT_DWORD) {
> +		osc_sb_apei_support_acked =
> +			capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_APEI_SUPPORT;
> +		osc_pc_lpi_support_confirmed =
> +			capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_PCLPI_SUPPORT;
> +	}
> +
> +	kfree(context.ret.pointer);
>  }
>  
>  /* --------------------------------------------------------------------------
> @@ -1168,7 +1187,7 @@ static int __init acpi_bus_init(void)
>  	 * _OSC method may exist in module level code,
>  	 * so it must be run after ACPI_FULL_INITIALIZATION
>  	 */
> -	acpi_bus_osc_support();
> +	acpi_bus_osc_negotiate_platform_control();
>  
>  	/*
>  	 * _PDC control method may load dynamic SSDT tables,
> -- 
> 2.29.2

Re: [PATCH v2 3/5] ACPI: Execute platform _OSC also with query bit clear

[Rafael J. Wysocki]

On Wed, Feb 3, 2021 at 9:16 AM Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
>
> Hi Rafael,
>
> I wonder if you are OK with this patch?

It looks good to me now, please feel free to add my ACK to it.

Thanks!

>
> On Fri, Jan 29, 2021 at 11:32:39AM +0300, Mika Westerberg wrote:
> > From: Mario Limonciello <mario.limonciello@dell.com>
> >
> > The platform _OSC can change the hardware state when query bit is not
> > set. According to ACPI spec it is recommended that the OS runs _OSC with
> > query bit set until the platform does not mask any of the capabilities.
> > Then it should run it with query bit clear in order to actually commit
> > the changes. Linux has not been doing this for the reasons that there
> > has not been anything to commit, until now.
> >
> > The ACPI 6.4 introduced _OSC for USB4 to allow the OS to negotiate
> > native control over USB4 tunneling. The platform might implement this so
> > that it only activates the software connection manager path when the OS
> > calls the _OSC with the query bit clear. Otherwise it may default to the
> > firmware connection manager, for instance.
> >
> > For this reason modify the _OSC support so that we first execute it with
> > query bit set, then use the returned value as base of the features we
> > want to control and run the _OSC again with query bit clear. This also
> > follows what Windows is doing.
> >
> > Also rename the function to better match what it does.
> >
> > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > ---
> >  drivers/acpi/bus.c | 43 +++++++++++++++++++++++++++++++------------
> >  1 file changed, 31 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> > index 1682f8b454a2..a52cb28c40d8 100644
> > --- a/drivers/acpi/bus.c
> > +++ b/drivers/acpi/bus.c
> > @@ -282,9 +282,9 @@ bool osc_pc_lpi_support_confirmed;
> >  EXPORT_SYMBOL_GPL(osc_pc_lpi_support_confirmed);
> >
> >  static u8 sb_uuid_str[] = "0811B06E-4A27-44F9-8D60-3CBBC22E7B48";
> > -static void acpi_bus_osc_support(void)
> > +static void acpi_bus_osc_negotiate_platform_control(void)
> >  {
> > -     u32 capbuf[2];
> > +     u32 capbuf[2], *capbuf_ret;
> >       struct acpi_osc_context context = {
> >               .uuid_str = sb_uuid_str,
> >               .rev = 1,
> > @@ -321,17 +321,36 @@ static void acpi_bus_osc_support(void)
> >               capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_APEI_SUPPORT;
> >       if (ACPI_FAILURE(acpi_get_handle(NULL, "\\_SB", &handle)))
> >               return;
> > -     if (ACPI_SUCCESS(acpi_run_osc(handle, &context))) {
> > -             u32 *capbuf_ret = context.ret.pointer;
> > -             if (context.ret.length > OSC_SUPPORT_DWORD) {
> > -                     osc_sb_apei_support_acked =
> > -                             capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_APEI_SUPPORT;
> > -                     osc_pc_lpi_support_confirmed =
> > -                             capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_PCLPI_SUPPORT;
> > -             }
> > +
> > +     if (ACPI_FAILURE(acpi_run_osc(handle, &context)))
> > +             return;
> > +
> > +     capbuf_ret = context.ret.pointer;
> > +     if (context.ret.length <= OSC_SUPPORT_DWORD) {
> >               kfree(context.ret.pointer);
> > +             return;
> >       }
> > -     /* do we need to check other returned cap? Sounds no */
> > +
> > +     /*
> > +      * Now run _OSC again with query flag clear and with the caps
> > +      * supported by both the OS and the platform.
> > +      */
> > +     capbuf[OSC_QUERY_DWORD] = 0;
> > +     capbuf[OSC_SUPPORT_DWORD] = capbuf_ret[OSC_SUPPORT_DWORD];
> > +     kfree(context.ret.pointer);
> > +
> > +     if (ACPI_FAILURE(acpi_run_osc(handle, &context)))
> > +             return;
> > +
> > +     capbuf_ret = context.ret.pointer;
> > +     if (context.ret.length > OSC_SUPPORT_DWORD) {
> > +             osc_sb_apei_support_acked =
> > +                     capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_APEI_SUPPORT;
> > +             osc_pc_lpi_support_confirmed =
> > +                     capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_PCLPI_SUPPORT;
> > +     }
> > +
> > +     kfree(context.ret.pointer);
> >  }
> >
> >  /* --------------------------------------------------------------------------
> > @@ -1168,7 +1187,7 @@ static int __init acpi_bus_init(void)
> >        * _OSC method may exist in module level code,
> >        * so it must be run after ACPI_FULL_INITIALIZATION
> >        */
> > -     acpi_bus_osc_support();
> > +     acpi_bus_osc_negotiate_platform_control();
> >
> >       /*
> >        * _PDC control method may load dynamic SSDT tables,
> > --
> > 2.29.2

Re: [PATCH v2 3/5] ACPI: Execute platform _OSC also with query bit clear

[Mika Westerberg]

On Wed, Feb 03, 2021 at 02:56:25PM +0100, Rafael J. Wysocki wrote:
> On Wed, Feb 3, 2021 at 9:16 AM Mika Westerberg
> <mika.westerberg@linux.intel.com> wrote:
> >
> > Hi Rafael,
> >
> > I wonder if you are OK with this patch?
> 
> It looks good to me now, please feel free to add my ACK to it.

Thanks!

Re: [PATCH v2 0/5] thunderbolt / ACPI: Add support for USB4 _OSC

[Mika Westerberg]

On Fri, Jan 29, 2021 at 11:32:36AM +0300, Mika Westerberg wrote:
> Hi all,
> 
> The just released ACPI 6.4 spec [1] adds a new _OSC method that is used to
> negotiate OS support for native USB4 features such as PCIe tunneling. This
> patch series adds Linux support for the new _OSC and modifies the
> Thunderbolt/USB4 driver accordingly to enable/disable tunneling of
> different protocols.
> 
> There is an additional setting in the firmware connection manager that
> allows the BIOS to disable PCIe tunneling, so we add support for this and
> also make the software connection manager to switch to this "nopcie"
> security level when the _OSC does not allow PCIe tunneling.
> 
> This applies on top of thunderbolt.git/next.
> 
> [1] https://uefi.org/sites/default/files/resources/ACPI_Spec_6_4_Jan22.pdf
> 
> The previous version of the patch series can be found here:
> 
>   https://lore.kernel.org/linux-usb/20210126155723.9388-1-mika.westerberg@linux.intel.com/
> 
> Changes from v1:
> 
>   * Dropped patch 1/6. I already applied it to thunderbolt.git/fixes
>   * Added ACK from Yehezkel to TBT patches
>   * Updated changelog of patch 1/5 and fixed typos too
>   * Added Rafael's tag to patch 4/5.
> 
> Mario Limonciello (1):
>   ACPI: Execute platform _OSC also with query bit clear
> 
> Mika Westerberg (4):
>   thunderbolt: Add support for PCIe tunneling disabled (SL5)
>   thunderbolt: Allow disabling XDomain protocol
>   ACPI: Add support for native USB4 control _OSC
>   thunderbolt: Add support for native USB4 _OSC

All applied to thunderbolt.git/next.

Re: [PATCH v2 3/5] ACPI: Execute platform _OSC also with query bit clear

[joeyli]

Hi Mika,

There have some machines be found on openSUSE Tumbleweed that this patch
causes that SSDT tables can not be dynamic loaded. The symptom is that
dmesg shows '_CPC not found' because SSDT table did not dynamic load.

[    1.149107] ACPI BIOS Error (bug): Could not resolve symbol [\_PR.PR00._CPC], AE_NOT_FOUND (20210105/psargs-330)

Looks that the firmware didn't response OSC_SB_CPCV2_SUPPORT after
kernel changed to new behavior. The openSUSE bug is here:

Bug 1185513 - ACPI BIOS Error after upgrade to 5.12.0-1-default 
https://bugzilla.suse.com/show_bug.cgi?id=1185513

Could you please help to give any suggestion?

Thanks a lot!
Joey Lee

On Wed, Feb 03, 2021 at 10:14:15AM +0200, Mika Westerberg wrote:
> Hi Rafael,
> 
> I wonder if you are OK with this patch?
> 
> Thanks!
> 
> On Fri, Jan 29, 2021 at 11:32:39AM +0300, Mika Westerberg wrote:
> > From: Mario Limonciello <mario.limonciello@dell.com>
> > 
> > The platform _OSC can change the hardware state when query bit is not
> > set. According to ACPI spec it is recommended that the OS runs _OSC with
> > query bit set until the platform does not mask any of the capabilities.
> > Then it should run it with query bit clear in order to actually commit
> > the changes. Linux has not been doing this for the reasons that there
> > has not been anything to commit, until now.
> > 
> > The ACPI 6.4 introduced _OSC for USB4 to allow the OS to negotiate
> > native control over USB4 tunneling. The platform might implement this so
> > that it only activates the software connection manager path when the OS
> > calls the _OSC with the query bit clear. Otherwise it may default to the
> > firmware connection manager, for instance.
> > 
> > For this reason modify the _OSC support so that we first execute it with
> > query bit set, then use the returned value as base of the features we
> > want to control and run the _OSC again with query bit clear. This also
> > follows what Windows is doing.
> > 
> > Also rename the function to better match what it does.
> > 
> > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > ---
> >  drivers/acpi/bus.c | 43 +++++++++++++++++++++++++++++++------------
> >  1 file changed, 31 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> > index 1682f8b454a2..a52cb28c40d8 100644
> > --- a/drivers/acpi/bus.c
> > +++ b/drivers/acpi/bus.c
> > @@ -282,9 +282,9 @@ bool osc_pc_lpi_support_confirmed;
> >  EXPORT_SYMBOL_GPL(osc_pc_lpi_support_confirmed);
> >  
> >  static u8 sb_uuid_str[] = "0811B06E-4A27-44F9-8D60-3CBBC22E7B48";
> > -static void acpi_bus_osc_support(void)
> > +static void acpi_bus_osc_negotiate_platform_control(void)
> >  {
> > -	u32 capbuf[2];
> > +	u32 capbuf[2], *capbuf_ret;
> >  	struct acpi_osc_context context = {
> >  		.uuid_str = sb_uuid_str,
> >  		.rev = 1,
> > @@ -321,17 +321,36 @@ static void acpi_bus_osc_support(void)
> >  		capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_APEI_SUPPORT;
> >  	if (ACPI_FAILURE(acpi_get_handle(NULL, "\\_SB", &handle)))
> >  		return;
> > -	if (ACPI_SUCCESS(acpi_run_osc(handle, &context))) {
> > -		u32 *capbuf_ret = context.ret.pointer;
> > -		if (context.ret.length > OSC_SUPPORT_DWORD) {
> > -			osc_sb_apei_support_acked =
> > -				capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_APEI_SUPPORT;
> > -			osc_pc_lpi_support_confirmed =
> > -				capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_PCLPI_SUPPORT;
> > -		}
> > +
> > +	if (ACPI_FAILURE(acpi_run_osc(handle, &context)))
> > +		return;
> > +
> > +	capbuf_ret = context.ret.pointer;
> > +	if (context.ret.length <= OSC_SUPPORT_DWORD) {
> >  		kfree(context.ret.pointer);
> > +		return;
> >  	}
> > -	/* do we need to check other returned cap? Sounds no */
> > +
> > +	/*
> > +	 * Now run _OSC again with query flag clear and with the caps
> > +	 * supported by both the OS and the platform.
> > +	 */
> > +	capbuf[OSC_QUERY_DWORD] = 0;
> > +	capbuf[OSC_SUPPORT_DWORD] = capbuf_ret[OSC_SUPPORT_DWORD];
> > +	kfree(context.ret.pointer);
> > +
> > +	if (ACPI_FAILURE(acpi_run_osc(handle, &context)))
> > +		return;
> > +
> > +	capbuf_ret = context.ret.pointer;
> > +	if (context.ret.length > OSC_SUPPORT_DWORD) {
> > +		osc_sb_apei_support_acked =
> > +			capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_APEI_SUPPORT;
> > +		osc_pc_lpi_support_confirmed =
> > +			capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_PCLPI_SUPPORT;
> > +	}
> > +
> > +	kfree(context.ret.pointer);
> >  }
> >  
> >  /* --------------------------------------------------------------------------
> > @@ -1168,7 +1187,7 @@ static int __init acpi_bus_init(void)
> >  	 * _OSC method may exist in module level code,
> >  	 * so it must be run after ACPI_FULL_INITIALIZATION
> >  	 */
> > -	acpi_bus_osc_support();
> > +	acpi_bus_osc_negotiate_platform_control();
> >  
> >  	/*
> >  	 * _PDC control method may load dynamic SSDT tables,
> > -- 
> > 2.29.2

Re: [PATCH v2 3/5] ACPI: Execute platform _OSC also with query bit clear

[Mika Westerberg]

Hi,

On Mon, Jun 07, 2021 at 08:31:10PM +0800, joeyli wrote:
> Hi Mika,
> 
> There have some machines be found on openSUSE Tumbleweed that this patch
> causes that SSDT tables can not be dynamic loaded. The symptom is that
> dmesg shows '_CPC not found' because SSDT table did not dynamic load.
> 
> [    1.149107] ACPI BIOS Error (bug): Could not resolve symbol [\_PR.PR00._CPC], AE_NOT_FOUND (20210105/psargs-330)
> 
> Looks that the firmware didn't response OSC_SB_CPCV2_SUPPORT after
> kernel changed to new behavior. The openSUSE bug is here:
> 
> Bug 1185513 - ACPI BIOS Error after upgrade to 5.12.0-1-default 
> https://bugzilla.suse.com/show_bug.cgi?id=1185513
> 
> Could you please help to give any suggestion?

There is another one that Red Hat reported here:

https://bugzilla.kernel.org/show_bug.cgi?id=213023

The Bugzilla entry also has a patch attached [1] from Hans, can you try it
out and see if that fixes the issue?

[1] https://bugzilla.kernel.org/attachment.cgi?id=297195

Thanks!

Re: [PATCH v2 3/5] ACPI: Execute platform _OSC also with query bit clear

[joeyli]

Hi Mika,

On Mon, Jun 07, 2021 at 07:10:59PM +0300, Mika Westerberg wrote:
> Hi,
> 
> On Mon, Jun 07, 2021 at 08:31:10PM +0800, joeyli wrote:
> > Hi Mika,
> > 
> > There have some machines be found on openSUSE Tumbleweed that this patch
> > causes that SSDT tables can not be dynamic loaded. The symptom is that
> > dmesg shows '_CPC not found' because SSDT table did not dynamic load.
> > 
> > [    1.149107] ACPI BIOS Error (bug): Could not resolve symbol [\_PR.PR00._CPC], AE_NOT_FOUND (20210105/psargs-330)
> > 
> > Looks that the firmware didn't response OSC_SB_CPCV2_SUPPORT after
> > kernel changed to new behavior. The openSUSE bug is here:
> > 
> > Bug 1185513 - ACPI BIOS Error after upgrade to 5.12.0-1-default 
> > https://bugzilla.suse.com/show_bug.cgi?id=1185513
> > 
> > Could you please help to give any suggestion?
> 
> There is another one that Red Hat reported here:
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=213023
> 
> The Bugzilla entry also has a patch attached [1] from Hans, can you try it
> out and see if that fixes the issue?
> 
> [1] https://bugzilla.kernel.org/attachment.cgi?id=297195
>

Thanks for your information! Hans's patch looks reasonable.

I will backport to openSUSE TW and let bug reporter helps to test it. We will
add comment on kernel.org bugzilla.

Thanks a lot!
Joey Lee