From: Maximilian Luz <luzmaximilian@gmail.com>
To: Hans de Goede <hdegoede@redhat.com>
Cc: Maximilian Luz <luzmaximilian@gmail.com>,
Mark Gross <markgross@kernel.org>,
platform-driver-x86@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: [PATCH 1/9] platform/surface: aggregator: Ignore command messages not intended for us
Date: Fri, 2 Dec 2022 23:33:19 +0100 [thread overview]
Message-ID: <20221202223327.690880-2-luzmaximilian@gmail.com> (raw)
In-Reply-To: <20221202223327.690880-1-luzmaximilian@gmail.com>
It is possible that we (the host/kernel driver) receive command messages
that are not intended for us. Ignore those for now.
The whole story is a bit more complicated: It is possible to enable
debug output on SAM, which is sent via SSH command messages. By default
this output is sent to a debug connector, with its own target ID
(TID=0x03). It is possible to override the target of the debug output
and set it to the host/kernel driver. This, however, does not change the
original target ID of the message. Meaning, we receive messages with
TID=0x03 (debug) but expect to only receive messages with TID=0x00
(host).
The problem is that the different target ID also comes with a different
scope of request IDs. In particular, these do not follow the standard
event rules (i.e. do not fall into a set of small reserved values).
Therefore, current message handling interprets them as responses to
pending requests and tries to match them up via the request ID. However,
these debug output messages are not in fact responses, and therefore
this will at best fail to find the request and at worst pass on the
wrong data as response for a request.
Therefore ignore any command messages not intended for us (host) for
now. We can implement support for the debug messages once we have a
better understanding of them.
Note that this may also provide a bit more stability and avoid some
driver confusion in case any other targets want to talk to us in the
future, since we don't yet know what to do with those as well. A warning
for the dropped messages should suffice for now and also give us a
chance of discovering new targets if they come along without any
potential for bugs/instabilities.
Fixes: c167b9c7e3d6 ("platform/surface: Add Surface Aggregator subsystem")
Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
---
Note: I've simplified this commit so that it can be applied
independently of the rest of the series for easier backporting.
---
.../surface/aggregator/ssh_request_layer.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/platform/surface/aggregator/ssh_request_layer.c b/drivers/platform/surface/aggregator/ssh_request_layer.c
index f5565570f16c..69132976d297 100644
--- a/drivers/platform/surface/aggregator/ssh_request_layer.c
+++ b/drivers/platform/surface/aggregator/ssh_request_layer.c
@@ -916,6 +916,20 @@ static void ssh_rtl_rx_command(struct ssh_ptl *p, const struct ssam_span *data)
if (sshp_parse_command(dev, data, &command, &command_data))
return;
+ /*
+ * Check if the message was intended for us. If not, drop it.
+ *
+ * Note: We will need to change this to handle debug messages. On newer
+ * generation devices, these seem to be sent to tid_out=0x03. We as
+ * host can still receive them as they can be forwarded via an override
+ * option on SAM, but doing so does not change tid_out=0x00.
+ */
+ if (command->tid_out != 0x00) {
+ rtl_warn(rtl, "rtl: dropping message not intended for us (tid = %#04x)\n",
+ command->tid_out);
+ return;
+ }
+
if (ssh_rqid_is_event(get_unaligned_le16(&command->rqid)))
ssh_rtl_rx_event(rtl, command, &command_data);
else
--
2.38.1
next prev parent reply other threads:[~2022-12-02 22:33 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-02 22:33 [PATCH 0/9] platform/surface: aggregator: Improve target/source handling in SSH messages Maximilian Luz
2022-12-02 22:33 ` Maximilian Luz [this message]
2023-01-12 18:12 ` [PATCH 1/9] platform/surface: aggregator: Ignore command messages not intended for us Hans de Goede
2022-12-02 22:33 ` [PATCH 2/9] platform/surface: aggregator: Improve documentation and handling of message target and source IDs Maximilian Luz
2022-12-02 22:33 ` [PATCH 3/9] platform/surface: aggregator: Add target and source IDs to command trace events Maximilian Luz
2022-12-02 22:33 ` [PATCH 4/9] platform/surface: aggregator_hub: Use target-ID enum instead of hard-coding values Maximilian Luz
2022-12-02 22:33 ` [PATCH 5/9] platform/surface: aggregator_tabletsw: " Maximilian Luz
2022-12-02 22:33 ` [PATCH 6/9] platform/surface: dtx: " Maximilian Luz
2022-12-02 22:33 ` [PATCH 7/9] HID: surface-hid: " Maximilian Luz
2022-12-02 22:33 ` [PATCH 8/9] platform/surface: aggregator: Enforce use of target-ID enum in device ID macros Maximilian Luz
2022-12-03 0:41 ` Sebastian Reichel
2022-12-02 22:33 ` [PATCH 9/9] platform/surface: aggregator_registry: Fix target-ID of base-hub Maximilian Luz
2022-12-08 16:03 ` [PATCH 0/9] platform/surface: aggregator: Improve target/source handling in SSH messages Hans de Goede
2022-12-08 16:18 ` Maximilian Luz
2022-12-08 16:24 ` Benjamin Tissoires
2022-12-08 16:38 ` Hans de Goede
2022-12-08 16:48 ` Maximilian Luz
2022-12-08 18:25 ` Benjamin Tissoires
2023-01-23 15:37 ` Hans de Goede
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20221202223327.690880-2-luzmaximilian@gmail.com \
--to=luzmaximilian@gmail.com \
--cc=hdegoede@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=markgross@kernel.org \
--cc=platform-driver-x86@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox