* [PATCH v3 5/6] drm/vmwgfx: Use vmware_hypercall API
From: Alexey Makhalov @ 2023-12-19 21:57 UTC (permalink / raw)
To: linux-kernel, virtualization, bp, hpa, dave.hansen, mingo, tglx
Cc: x86, netdev, richardcochran, linux-input, dmitry.torokhov, zackr,
linux-graphics-maintainer, pv-drivers, namit, timothym, akaher,
jsipek, dri-devel, daniel, airlied, tzimmermann, mripard,
maarten.lankhorst, horms, kirill.shutemov
In-Reply-To: <20231219215751.9445-1-alexey.makhalov@broadcom.com>
From: Alexey Makhalov <amakhalov@vmware.com>
Switch from VMWARE_HYPERCALL macro to vmware_hypercall API.
Eliminate arch specific code.
drivers/gpu/drm/vmwgfx/vmwgfx_msg_arm64.h: implement arm64 variant
of vmware_hypercall. And keep it here until introduction of ARM64
VMWare hypervisor interface.
Signed-off-by: Alexey Makhalov <amakhalov@vmware.com>
Reviewed-by: Nadav Amit <namit@vmware.com>
Reviewed-by: Zack Rusin <zackr@vmware.com>
---
drivers/gpu/drm/vmwgfx/vmwgfx_msg.c | 173 +++++++------------
drivers/gpu/drm/vmwgfx/vmwgfx_msg_arm64.h | 197 +++++++++++++++-------
drivers/gpu/drm/vmwgfx/vmwgfx_msg_x86.h | 185 --------------------
3 files changed, 197 insertions(+), 358 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
index 2651fe0ef518..1f15990d3934 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
@@ -48,8 +48,6 @@
#define RETRIES 3
-#define VMW_HYPERVISOR_MAGIC 0x564D5868
-
#define VMW_PORT_CMD_MSG 30
#define VMW_PORT_CMD_HB_MSG 0
#define VMW_PORT_CMD_OPEN_CHANNEL (MSG_TYPE_OPEN << 16 | VMW_PORT_CMD_MSG)
@@ -104,20 +102,18 @@ static const char* const mksstat_kern_name_desc[MKSSTAT_KERN_COUNT][2] =
*/
static int vmw_open_channel(struct rpc_channel *channel, unsigned int protocol)
{
- unsigned long eax, ebx, ecx, edx, si = 0, di = 0;
+ u32 ecx, edx, esi, edi;
- VMW_PORT(VMW_PORT_CMD_OPEN_CHANNEL,
- (protocol | GUESTMSG_FLAG_COOKIE), si, di,
- 0,
- VMW_HYPERVISOR_MAGIC,
- eax, ebx, ecx, edx, si, di);
+ vmware_hypercall6(VMW_PORT_CMD_OPEN_CHANNEL,
+ (protocol | GUESTMSG_FLAG_COOKIE), 0,
+ &ecx, &edx, &esi, &edi);
if ((HIGH_WORD(ecx) & MESSAGE_STATUS_SUCCESS) == 0)
return -EINVAL;
channel->channel_id = HIGH_WORD(edx);
- channel->cookie_high = si;
- channel->cookie_low = di;
+ channel->cookie_high = esi;
+ channel->cookie_low = edi;
return 0;
}
@@ -133,17 +129,13 @@ static int vmw_open_channel(struct rpc_channel *channel, unsigned int protocol)
*/
static int vmw_close_channel(struct rpc_channel *channel)
{
- unsigned long eax, ebx, ecx, edx, si, di;
-
- /* Set up additional parameters */
- si = channel->cookie_high;
- di = channel->cookie_low;
+ u32 ecx;
- VMW_PORT(VMW_PORT_CMD_CLOSE_CHANNEL,
- 0, si, di,
- channel->channel_id << 16,
- VMW_HYPERVISOR_MAGIC,
- eax, ebx, ecx, edx, si, di);
+ vmware_hypercall5(VMW_PORT_CMD_CLOSE_CHANNEL,
+ 0, channel->channel_id << 16,
+ channel->cookie_high,
+ channel->cookie_low,
+ &ecx);
if ((HIGH_WORD(ecx) & MESSAGE_STATUS_SUCCESS) == 0)
return -EINVAL;
@@ -163,24 +155,18 @@ static int vmw_close_channel(struct rpc_channel *channel)
static unsigned long vmw_port_hb_out(struct rpc_channel *channel,
const char *msg, bool hb)
{
- unsigned long si, di, eax, ebx, ecx, edx;
+ u32 ebx, ecx;
unsigned long msg_len = strlen(msg);
/* HB port can't access encrypted memory. */
if (hb && !cc_platform_has(CC_ATTR_MEM_ENCRYPT)) {
- unsigned long bp = channel->cookie_high;
- u32 channel_id = (channel->channel_id << 16);
-
- si = (uintptr_t) msg;
- di = channel->cookie_low;
-
- VMW_PORT_HB_OUT(
+ vmware_hypercall_hb_out(
(MESSAGE_STATUS_SUCCESS << 16) | VMW_PORT_CMD_HB_MSG,
- msg_len, si, di,
- VMWARE_HYPERVISOR_HB | channel_id |
- VMWARE_HYPERVISOR_OUT,
- VMW_HYPERVISOR_MAGIC, bp,
- eax, ebx, ecx, edx, si, di);
+ msg_len,
+ channel->channel_id << 16,
+ (uintptr_t) msg, channel->cookie_low,
+ channel->cookie_high,
+ &ebx);
return ebx;
}
@@ -194,14 +180,13 @@ static unsigned long vmw_port_hb_out(struct rpc_channel *channel,
memcpy(&word, msg, bytes);
msg_len -= bytes;
msg += bytes;
- si = channel->cookie_high;
- di = channel->cookie_low;
-
- VMW_PORT(VMW_PORT_CMD_MSG | (MSG_TYPE_SENDPAYLOAD << 16),
- word, si, di,
- channel->channel_id << 16,
- VMW_HYPERVISOR_MAGIC,
- eax, ebx, ecx, edx, si, di);
+
+ vmware_hypercall5(VMW_PORT_CMD_MSG |
+ (MSG_TYPE_SENDPAYLOAD << 16),
+ word, channel->channel_id << 16,
+ channel->cookie_high,
+ channel->cookie_low,
+ &ecx);
}
return ecx;
@@ -220,22 +205,17 @@ static unsigned long vmw_port_hb_out(struct rpc_channel *channel,
static unsigned long vmw_port_hb_in(struct rpc_channel *channel, char *reply,
unsigned long reply_len, bool hb)
{
- unsigned long si, di, eax, ebx, ecx, edx;
+ u32 ebx, ecx, edx;
/* HB port can't access encrypted memory */
if (hb && !cc_platform_has(CC_ATTR_MEM_ENCRYPT)) {
- unsigned long bp = channel->cookie_low;
- u32 channel_id = (channel->channel_id << 16);
-
- si = channel->cookie_high;
- di = (uintptr_t) reply;
-
- VMW_PORT_HB_IN(
+ vmware_hypercall_hb_in(
(MESSAGE_STATUS_SUCCESS << 16) | VMW_PORT_CMD_HB_MSG,
- reply_len, si, di,
- VMWARE_HYPERVISOR_HB | channel_id,
- VMW_HYPERVISOR_MAGIC, bp,
- eax, ebx, ecx, edx, si, di);
+ reply_len,
+ channel->channel_id << 16,
+ channel->cookie_high,
+ (uintptr_t) reply, channel->cookie_low,
+ &ebx);
return ebx;
}
@@ -245,14 +225,13 @@ static unsigned long vmw_port_hb_in(struct rpc_channel *channel, char *reply,
while (reply_len) {
unsigned int bytes = min_t(unsigned long, reply_len, 4);
- si = channel->cookie_high;
- di = channel->cookie_low;
-
- VMW_PORT(VMW_PORT_CMD_MSG | (MSG_TYPE_RECVPAYLOAD << 16),
- MESSAGE_STATUS_SUCCESS, si, di,
- channel->channel_id << 16,
- VMW_HYPERVISOR_MAGIC,
- eax, ebx, ecx, edx, si, di);
+ vmware_hypercall7(VMW_PORT_CMD_MSG |
+ (MSG_TYPE_RECVPAYLOAD << 16),
+ MESSAGE_STATUS_SUCCESS,
+ channel->channel_id << 16,
+ channel->cookie_high,
+ channel->cookie_low,
+ &ebx, &ecx, &edx);
if ((HIGH_WORD(ecx) & MESSAGE_STATUS_SUCCESS) == 0)
break;
@@ -276,22 +255,18 @@ static unsigned long vmw_port_hb_in(struct rpc_channel *channel, char *reply,
*/
static int vmw_send_msg(struct rpc_channel *channel, const char *msg)
{
- unsigned long eax, ebx, ecx, edx, si, di;
+ u32 ebx, ecx;
size_t msg_len = strlen(msg);
int retries = 0;
while (retries < RETRIES) {
retries++;
- /* Set up additional parameters */
- si = channel->cookie_high;
- di = channel->cookie_low;
-
- VMW_PORT(VMW_PORT_CMD_SENDSIZE,
- msg_len, si, di,
- channel->channel_id << 16,
- VMW_HYPERVISOR_MAGIC,
- eax, ebx, ecx, edx, si, di);
+ vmware_hypercall5(VMW_PORT_CMD_SENDSIZE,
+ msg_len, channel->channel_id << 16,
+ channel->cookie_high,
+ channel->cookie_low,
+ &ecx);
if ((HIGH_WORD(ecx) & MESSAGE_STATUS_SUCCESS) == 0) {
/* Expected success. Give up. */
@@ -329,7 +304,7 @@ STACK_FRAME_NON_STANDARD(vmw_send_msg);
static int vmw_recv_msg(struct rpc_channel *channel, void **msg,
size_t *msg_len)
{
- unsigned long eax, ebx, ecx, edx, si, di;
+ u32 ebx, ecx, edx;
char *reply;
size_t reply_len;
int retries = 0;
@@ -341,15 +316,11 @@ static int vmw_recv_msg(struct rpc_channel *channel, void **msg,
while (retries < RETRIES) {
retries++;
- /* Set up additional parameters */
- si = channel->cookie_high;
- di = channel->cookie_low;
-
- VMW_PORT(VMW_PORT_CMD_RECVSIZE,
- 0, si, di,
- channel->channel_id << 16,
- VMW_HYPERVISOR_MAGIC,
- eax, ebx, ecx, edx, si, di);
+ vmware_hypercall7(VMW_PORT_CMD_RECVSIZE,
+ 0, channel->channel_id << 16,
+ channel->cookie_high,
+ channel->cookie_low,
+ &ebx, &ecx, &edx);
if ((HIGH_WORD(ecx) & MESSAGE_STATUS_SUCCESS) == 0) {
DRM_ERROR("Failed to get reply size for host message.\n");
@@ -384,16 +355,12 @@ static int vmw_recv_msg(struct rpc_channel *channel, void **msg,
reply[reply_len] = '\0';
-
- /* Ack buffer */
- si = channel->cookie_high;
- di = channel->cookie_low;
-
- VMW_PORT(VMW_PORT_CMD_RECVSTATUS,
- MESSAGE_STATUS_SUCCESS, si, di,
- channel->channel_id << 16,
- VMW_HYPERVISOR_MAGIC,
- eax, ebx, ecx, edx, si, di);
+ vmware_hypercall5(VMW_PORT_CMD_RECVSTATUS,
+ MESSAGE_STATUS_SUCCESS,
+ channel->channel_id << 16,
+ channel->cookie_high,
+ channel->cookie_low,
+ &ecx);
if ((HIGH_WORD(ecx) & MESSAGE_STATUS_SUCCESS) == 0) {
kfree(reply);
@@ -652,13 +619,7 @@ static inline void reset_ppn_array(PPN64 *arr, size_t size)
*/
static inline void hypervisor_ppn_reset_all(void)
{
- unsigned long eax, ebx, ecx, edx, si = 0, di = 0;
-
- VMW_PORT(VMW_PORT_CMD_MKSGS_RESET,
- 0, si, di,
- 0,
- VMW_HYPERVISOR_MAGIC,
- eax, ebx, ecx, edx, si, di);
+ vmware_hypercall1(VMW_PORT_CMD_MKSGS_RESET, 0);
}
/**
@@ -669,13 +630,7 @@ static inline void hypervisor_ppn_reset_all(void)
*/
static inline void hypervisor_ppn_add(PPN64 pfn)
{
- unsigned long eax, ebx, ecx, edx, si = 0, di = 0;
-
- VMW_PORT(VMW_PORT_CMD_MKSGS_ADD_PPN,
- (unsigned long)pfn, si, di,
- 0,
- VMW_HYPERVISOR_MAGIC,
- eax, ebx, ecx, edx, si, di);
+ vmware_hypercall1(VMW_PORT_CMD_MKSGS_ADD_PPN, (unsigned long)pfn);
}
/**
@@ -686,13 +641,7 @@ static inline void hypervisor_ppn_add(PPN64 pfn)
*/
static inline void hypervisor_ppn_remove(PPN64 pfn)
{
- unsigned long eax, ebx, ecx, edx, si = 0, di = 0;
-
- VMW_PORT(VMW_PORT_CMD_MKSGS_REMOVE_PPN,
- (unsigned long)pfn, si, di,
- 0,
- VMW_HYPERVISOR_MAGIC,
- eax, ebx, ecx, edx, si, di);
+ vmware_hypercall1(VMW_PORT_CMD_MKSGS_REMOVE_PPN, (unsigned long)pfn);
}
#if IS_ENABLED(CONFIG_DRM_VMWGFX_MKSSTATS)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_msg_arm64.h b/drivers/gpu/drm/vmwgfx/vmwgfx_msg_arm64.h
index 4f40167ad61f..29bd0af83038 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_msg_arm64.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_msg_arm64.h
@@ -34,6 +34,8 @@
#define VMWARE_HYPERVISOR_HB BIT(0)
#define VMWARE_HYPERVISOR_OUT BIT(1)
+#define VMWARE_HYPERVISOR_MAGIC 0x564D5868
+
#define X86_IO_MAGIC 0x86
#define X86_IO_W7_SIZE_SHIFT 0
@@ -45,86 +47,159 @@
#define X86_IO_W7_IMM_SHIFT 5
#define X86_IO_W7_IMM_MASK (0xff << X86_IO_W7_IMM_SHIFT)
-static inline void vmw_port(unsigned long cmd, unsigned long in_ebx,
- unsigned long in_si, unsigned long in_di,
- unsigned long flags, unsigned long magic,
- unsigned long *eax, unsigned long *ebx,
- unsigned long *ecx, unsigned long *edx,
- unsigned long *si, unsigned long *di)
+static inline
+unsigned long vmware_hypercall1(unsigned long cmd, unsigned long in1)
{
- register u64 x0 asm("x0") = magic;
- register u64 x1 asm("x1") = in_ebx;
+ register u64 x0 asm("x0") = VMWARE_HYPERVISOR_MAGIC;
+ register u64 x1 asm("x1") = in1;
register u64 x2 asm("x2") = cmd;
- register u64 x3 asm("x3") = flags | VMWARE_HYPERVISOR_PORT;
- register u64 x4 asm("x4") = in_si;
- register u64 x5 asm("x5") = in_di;
+ register u64 x3 asm("x3") = VMWARE_HYPERVISOR_PORT;
+ register u64 x7 asm("x7") = ((u64)X86_IO_MAGIC << 32) |
+ X86_IO_W7_WITH |
+ X86_IO_W7_DIR |
+ (2 << X86_IO_W7_SIZE_SHIFT);
+ asm_inline volatile (
+ "mrs xzr, mdccsr_el0; "
+ : "+r" (x0)
+ : "r" (x1), "r" (x2), "r" (x3), "r" (x7)
+ : "memory");
+
+ return x0;
+}
+
+static inline
+unsigned long vmware_hypercall5(unsigned long cmd, unsigned long in1,
+ unsigned long in3, unsigned long in4,
+ unsigned long in5, uint32_t *out2)
+{
+ register u64 x0 asm("x0") = VMWARE_HYPERVISOR_MAGIC;
+ register u64 x1 asm("x1") = in1;
+ register u64 x2 asm("x2") = cmd;
+ register u64 x3 asm("x3") = in3 | VMWARE_HYPERVISOR_PORT;
+ register u64 x4 asm("x4") = in4;
+ register u64 x5 asm("x5") = in5;
register u64 x7 asm("x7") = ((u64)X86_IO_MAGIC << 32) |
X86_IO_W7_WITH |
X86_IO_W7_DIR |
(2 << X86_IO_W7_SIZE_SHIFT);
- asm volatile("mrs xzr, mdccsr_el0 \n\t"
- : "+r"(x0), "+r"(x1), "+r"(x2),
- "+r"(x3), "+r"(x4), "+r"(x5)
- : "r"(x7)
- :);
- *eax = x0;
- *ebx = x1;
- *ecx = x2;
- *edx = x3;
- *si = x4;
- *di = x5;
+ asm_inline volatile (
+ "mrs xzr, mdccsr_el0; "
+ : "+r" (x0), "+r" (x2)
+ : "r" (x1), "r" (x3), "r" (x4), "r" (x5), "r" (x7)
+ : "memory");
+
+ *out2 = x2;
+ return x0;
}
-static inline void vmw_port_hb(unsigned long cmd, unsigned long in_ecx,
- unsigned long in_si, unsigned long in_di,
- unsigned long flags, unsigned long magic,
- unsigned long bp, u32 w7dir,
- unsigned long *eax, unsigned long *ebx,
- unsigned long *ecx, unsigned long *edx,
- unsigned long *si, unsigned long *di)
+static inline
+unsigned long vmware_hypercall6(unsigned long cmd, unsigned long in1,
+ unsigned long in3, uint32_t *out2,
+ uint32_t *out3, uint32_t *out4,
+ uint32_t *out5)
{
- register u64 x0 asm("x0") = magic;
+ register u64 x0 asm("x0") = VMWARE_HYPERVISOR_MAGIC;
+ register u64 x1 asm("x1") = in1;
+ register u64 x2 asm("x2") = cmd;
+ register u64 x3 asm("x3") = in3 | VMWARE_HYPERVISOR_PORT;
+ register u64 x4 asm("x4");
+ register u64 x5 asm("x5");
+ register u64 x7 asm("x7") = ((u64)X86_IO_MAGIC << 32) |
+ X86_IO_W7_WITH |
+ X86_IO_W7_DIR |
+ (2 << X86_IO_W7_SIZE_SHIFT);
+
+ asm_inline volatile (
+ "mrs xzr, mdccsr_el0; "
+ : "+r" (x0), "+r" (x2), "+r" (x3), "=r" (x4), "=r" (x5)
+ : "r" (x1), "r" (x7)
+ : "memory");
+
+ *out2 = x2;
+ *out3 = x3;
+ *out4 = x4;
+ *out5 = x5;
+ return x0;
+}
+
+static inline
+unsigned long vmware_hypercall7(unsigned long cmd, unsigned long in1,
+ unsigned long in3, unsigned long in4,
+ unsigned long in5, uint32_t *out1,
+ uint32_t *out2, uint32_t *out3)
+{
+ register u64 x0 asm("x0") = VMWARE_HYPERVISOR_MAGIC;
+ register u64 x1 asm("x1") = in1;
+ register u64 x2 asm("x2") = cmd;
+ register u64 x3 asm("x3") = in3 | VMWARE_HYPERVISOR_PORT;
+ register u64 x4 asm("x4") = in4;
+ register u64 x5 asm("x5") = in5;
+ register u64 x7 asm("x7") = ((u64)X86_IO_MAGIC << 32) |
+ X86_IO_W7_WITH |
+ X86_IO_W7_DIR |
+ (2 << X86_IO_W7_SIZE_SHIFT);
+
+ asm_inline volatile (
+ "mrs xzr, mdccsr_el0; "
+ : "+r" (x0), "+r" (x1), "+r" (x2), "+r" (x3)
+ : "r" (x4), "r" (x5), "r" (x7)
+ : "memory");
+
+ *out1 = x1;
+ *out2 = x2;
+ *out3 = x3;
+ return x0;
+}
+
+static inline
+unsigned long vmware_hypercall_hb(unsigned long cmd, unsigned long in2,
+ unsigned long in3, unsigned long in4,
+ unsigned long in5, unsigned long in6,
+ uint32_t *out1, int dir)
+{
+ register u64 x0 asm("x0") = VMWARE_HYPERVISOR_MAGIC;
register u64 x1 asm("x1") = cmd;
- register u64 x2 asm("x2") = in_ecx;
- register u64 x3 asm("x3") = flags | VMWARE_HYPERVISOR_PORT_HB;
- register u64 x4 asm("x4") = in_si;
- register u64 x5 asm("x5") = in_di;
- register u64 x6 asm("x6") = bp;
+ register u64 x2 asm("x2") = in2;
+ register u64 x3 asm("x3") = in3 | VMWARE_HYPERVISOR_PORT_HB;
+ register u64 x4 asm("x4") = in4;
+ register u64 x5 asm("x5") = in5;
+ register u64 x6 asm("x6") = in6;
register u64 x7 asm("x7") = ((u64)X86_IO_MAGIC << 32) |
X86_IO_W7_STR |
X86_IO_W7_WITH |
- w7dir;
-
- asm volatile("mrs xzr, mdccsr_el0 \n\t"
- : "+r"(x0), "+r"(x1), "+r"(x2),
- "+r"(x3), "+r"(x4), "+r"(x5)
- : "r"(x6), "r"(x7)
- :);
- *eax = x0;
- *ebx = x1;
- *ecx = x2;
- *edx = x3;
- *si = x4;
- *di = x5;
-}
+ dir;
-#define VMW_PORT(cmd, in_ebx, in_si, in_di, flags, magic, eax, ebx, ecx, edx, \
- si, di) \
- vmw_port(cmd, in_ebx, in_si, in_di, flags, magic, &eax, &ebx, &ecx, \
- &edx, &si, &di)
+ asm_inline volatile (
+ "mrs xzr, mdccsr_el0; "
+ : "+r" (x0), "+r" (x1)
+ : "r" (x2), "r" (x3), "r" (x4), "r" (x5),
+ "r" (x6), "r" (x7)
+ : "memory");
-#define VMW_PORT_HB_OUT(cmd, in_ecx, in_si, in_di, flags, magic, bp, eax, ebx, \
- ecx, edx, si, di) \
- vmw_port_hb(cmd, in_ecx, in_si, in_di, flags, magic, bp, \
- 0, &eax, &ebx, &ecx, &edx, &si, &di)
+ *out1 = x1;
+ return x0;
+}
-#define VMW_PORT_HB_IN(cmd, in_ecx, in_si, in_di, flags, magic, bp, eax, ebx, \
- ecx, edx, si, di) \
- vmw_port_hb(cmd, in_ecx, in_si, in_di, flags, magic, bp, \
- X86_IO_W7_DIR, &eax, &ebx, &ecx, &edx, &si, &di)
+static inline
+unsigned long vmware_hypercall_hb_out(unsigned long cmd, unsigned long in2,
+ unsigned long in3, unsigned long in4,
+ unsigned long in5, unsigned long in6,
+ uint32_t *out1)
+{
+ return vmware_hypercall_hb(cmd, in2, in3, in4, in5, in6, out1, 0);
+}
+static inline
+unsigned long vmware_hypercall_hb_in(unsigned long cmd, unsigned long in2,
+ unsigned long in3, unsigned long in4,
+ unsigned long in5, unsigned long in6,
+ uint32_t *out1)
+{
+ return vmware_hypercall_hb(cmd, in2, in3, in4, in5, in6, out1,
+ X86_IO_W7_DIR);
+}
#endif
#endif /* _VMWGFX_MSG_ARM64_H */
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_msg_x86.h b/drivers/gpu/drm/vmwgfx/vmwgfx_msg_x86.h
index 23899d743a90..13304d34cc6e 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_msg_x86.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_msg_x86.h
@@ -37,191 +37,6 @@
#include <asm/vmware.h>
-/**
- * Hypervisor-specific bi-directional communication channel. Should never
- * execute on bare metal hardware. The caller must make sure to check for
- * supported hypervisor before using these macros.
- *
- * The last two parameters are both input and output and must be initialized.
- *
- * @cmd: [IN] Message Cmd
- * @in_ebx: [IN] Message Len, through EBX
- * @in_si: [IN] Input argument through SI, set to 0 if not used
- * @in_di: [IN] Input argument through DI, set ot 0 if not used
- * @flags: [IN] hypercall flags + [channel id]
- * @magic: [IN] hypervisor magic value
- * @eax: [OUT] value of EAX register
- * @ebx: [OUT] e.g. status from an HB message status command
- * @ecx: [OUT] e.g. status from a non-HB message status command
- * @edx: [OUT] e.g. channel id
- * @si: [OUT]
- * @di: [OUT]
- */
-#define VMW_PORT(cmd, in_ebx, in_si, in_di, \
- flags, magic, \
- eax, ebx, ecx, edx, si, di) \
-({ \
- asm volatile (VMWARE_HYPERCALL : \
- "=a"(eax), \
- "=b"(ebx), \
- "=c"(ecx), \
- "=d"(edx), \
- "=S"(si), \
- "=D"(di) : \
- "a"(magic), \
- "b"(in_ebx), \
- "c"(cmd), \
- "d"(flags), \
- "S"(in_si), \
- "D"(in_di) : \
- "memory"); \
-})
-
-
-/**
- * Hypervisor-specific bi-directional communication channel. Should never
- * execute on bare metal hardware. The caller must make sure to check for
- * supported hypervisor before using these macros.
- *
- * The last 3 parameters are both input and output and must be initialized.
- *
- * @cmd: [IN] Message Cmd
- * @in_ecx: [IN] Message Len, through ECX
- * @in_si: [IN] Input argument through SI, set to 0 if not used
- * @in_di: [IN] Input argument through DI, set to 0 if not used
- * @flags: [IN] hypercall flags + [channel id]
- * @magic: [IN] hypervisor magic value
- * @bp: [IN]
- * @eax: [OUT] value of EAX register
- * @ebx: [OUT] e.g. status from an HB message status command
- * @ecx: [OUT] e.g. status from a non-HB message status command
- * @edx: [OUT] e.g. channel id
- * @si: [OUT]
- * @di: [OUT]
- */
-#ifdef __x86_64__
-
-#define VMW_PORT_HB_OUT(cmd, in_ecx, in_si, in_di, \
- flags, magic, bp, \
- eax, ebx, ecx, edx, si, di) \
-({ \
- asm volatile ( \
- UNWIND_HINT_SAVE \
- "push %%rbp;" \
- UNWIND_HINT_UNDEFINED \
- "mov %12, %%rbp;" \
- VMWARE_HYPERCALL_HB_OUT \
- "pop %%rbp;" \
- UNWIND_HINT_RESTORE : \
- "=a"(eax), \
- "=b"(ebx), \
- "=c"(ecx), \
- "=d"(edx), \
- "=S"(si), \
- "=D"(di) : \
- "a"(magic), \
- "b"(cmd), \
- "c"(in_ecx), \
- "d"(flags), \
- "S"(in_si), \
- "D"(in_di), \
- "r"(bp) : \
- "memory", "cc"); \
-})
-
-
-#define VMW_PORT_HB_IN(cmd, in_ecx, in_si, in_di, \
- flags, magic, bp, \
- eax, ebx, ecx, edx, si, di) \
-({ \
- asm volatile ( \
- UNWIND_HINT_SAVE \
- "push %%rbp;" \
- UNWIND_HINT_UNDEFINED \
- "mov %12, %%rbp;" \
- VMWARE_HYPERCALL_HB_IN \
- "pop %%rbp;" \
- UNWIND_HINT_RESTORE : \
- "=a"(eax), \
- "=b"(ebx), \
- "=c"(ecx), \
- "=d"(edx), \
- "=S"(si), \
- "=D"(di) : \
- "a"(magic), \
- "b"(cmd), \
- "c"(in_ecx), \
- "d"(flags), \
- "S"(in_si), \
- "D"(in_di), \
- "r"(bp) : \
- "memory", "cc"); \
-})
-
-#elif defined(__i386__)
-
-/*
- * In the 32-bit version of this macro, we store bp in a memory location
- * because we've ran out of registers.
- * Now we can't reference that memory location while we've modified
- * %esp or %ebp, so we first push it on the stack, just before we push
- * %ebp, and then when we need it we read it from the stack where we
- * just pushed it.
- */
-#define VMW_PORT_HB_OUT(cmd, in_ecx, in_si, in_di, \
- flags, magic, bp, \
- eax, ebx, ecx, edx, si, di) \
-({ \
- asm volatile ("push %12;" \
- "push %%ebp;" \
- "mov 0x04(%%esp), %%ebp;" \
- VMWARE_HYPERCALL_HB_OUT \
- "pop %%ebp;" \
- "add $0x04, %%esp;" : \
- "=a"(eax), \
- "=b"(ebx), \
- "=c"(ecx), \
- "=d"(edx), \
- "=S"(si), \
- "=D"(di) : \
- "a"(magic), \
- "b"(cmd), \
- "c"(in_ecx), \
- "d"(flags), \
- "S"(in_si), \
- "D"(in_di), \
- "m"(bp) : \
- "memory", "cc"); \
-})
-
-
-#define VMW_PORT_HB_IN(cmd, in_ecx, in_si, in_di, \
- flags, magic, bp, \
- eax, ebx, ecx, edx, si, di) \
-({ \
- asm volatile ("push %12;" \
- "push %%ebp;" \
- "mov 0x04(%%esp), %%ebp;" \
- VMWARE_HYPERCALL_HB_IN \
- "pop %%ebp;" \
- "add $0x04, %%esp;" : \
- "=a"(eax), \
- "=b"(ebx), \
- "=c"(ecx), \
- "=d"(edx), \
- "=S"(si), \
- "=D"(di) : \
- "a"(magic), \
- "b"(cmd), \
- "c"(in_ecx), \
- "d"(flags), \
- "S"(in_si), \
- "D"(in_di), \
- "m"(bp) : \
- "memory", "cc"); \
-})
-#endif /* defined(__i386__) */
-
#endif /* defined(__i386__) || defined(__x86_64__) */
#endif /* _VMWGFX_MSG_X86_H */
--
2.39.0
^ permalink raw reply related
* [PATCH v3 6/6] x86/vmware: Add TDX hypercall support
From: Alexey Makhalov @ 2023-12-19 21:57 UTC (permalink / raw)
To: linux-kernel, virtualization, bp, hpa, dave.hansen, mingo, tglx
Cc: x86, netdev, richardcochran, linux-input, dmitry.torokhov, zackr,
linux-graphics-maintainer, pv-drivers, namit, timothym, akaher,
jsipek, dri-devel, daniel, airlied, tzimmermann, mripard,
maarten.lankhorst, horms, kirill.shutemov
In-Reply-To: <20231219215751.9445-1-alexey.makhalov@broadcom.com>
From: Alexey Makhalov <amakhalov@vmware.com>
VMware hypercalls use I/O port, VMCALL or VMMCALL instructions.
Add __tdx_hypercall path to support TDX guests.
No change in high bandwidth hypercalls, as only low bandwidth
ones are supported for TDX guests.
Co-developed-by: Tim Merrifield <timothym@vmware.com>
Signed-off-by: Tim Merrifield <timothym@vmware.com>
Signed-off-by: Alexey Makhalov <amakhalov@vmware.com>
Reviewed-by: Nadav Amit <namit@vmware.com>
---
arch/x86/include/asm/vmware.h | 83 +++++++++++++++++++++++++++++++++++
arch/x86/kernel/cpu/vmware.c | 24 ++++++++++
2 files changed, 107 insertions(+)
diff --git a/arch/x86/include/asm/vmware.h b/arch/x86/include/asm/vmware.h
index 719e41260ece..cad6f5b371a8 100644
--- a/arch/x86/include/asm/vmware.h
+++ b/arch/x86/include/asm/vmware.h
@@ -34,12 +34,65 @@
#define VMWARE_CMD_GETHZ 45
#define VMWARE_CMD_GETVCPU_INFO 68
#define VMWARE_CMD_STEALCLOCK 91
+/*
+ * Hypercall command mask:
+ * bits[6:0] command, range [0, 127]
+ * bits[19:16] sub-command, range [0, 15]
+ */
+#define VMWARE_CMD_MASK 0xf007fU
#define CPUID_VMWARE_FEATURES_ECX_VMMCALL BIT(0)
#define CPUID_VMWARE_FEATURES_ECX_VMCALL BIT(1)
extern u8 vmware_hypercall_mode;
+#define VMWARE_TDX_VENDOR_LEAF 0x1af7e4909ULL
+#define VMWARE_TDX_HCALL_FUNC 1
+
+extern unsigned long vmware_tdx_hypercall(unsigned long cmd,
+ struct tdx_module_args *args);
+
+/*
+ * TDCALL[TDG.VP.VMCALL] uses rax (arg0) and rcx (arg2), while the use of
+ * rbp (arg6) is discouraged by the TDX specification. Therefore, we
+ * remap those registers to r12, r13 and r14, respectively.
+ */
+static inline
+unsigned long vmware_tdx_hypercall_args(unsigned long cmd, unsigned long in1,
+ unsigned long in3, unsigned long in4,
+ unsigned long in5, unsigned long in6,
+ uint32_t *out1, uint32_t *out2,
+ uint32_t *out3, uint32_t *out4,
+ uint32_t *out5, uint32_t *out6)
+{
+ unsigned long ret;
+
+ struct tdx_module_args args = {
+ .rbx = in1,
+ .rdx = in3,
+ .rsi = in4,
+ .rdi = in5,
+ .r14 = in6,
+ };
+
+ ret = vmware_tdx_hypercall(cmd, &args);
+
+ if (out1)
+ *out1 = args.rbx;
+ if (out2)
+ *out2 = args.r13;
+ if (out3)
+ *out3 = args.rdx;
+ if (out4)
+ *out4 = args.rsi;
+ if (out5)
+ *out5 = args.rdi;
+ if (out6)
+ *out6 = args.r14;
+
+ return ret;
+}
+
/*
* The low bandwidth call. The low word of edx is presumed to have OUT bit
* set. The high word of edx may contain input data from the caller.
@@ -67,6 +120,11 @@ unsigned long vmware_hypercall1(unsigned long cmd, unsigned long in1)
{
unsigned long out0;
+ if (cpu_feature_enabled(X86_FEATURE_TDX_GUEST))
+ return vmware_tdx_hypercall_args(cmd, in1, 0, 0, 0, 0,
+ NULL, NULL, NULL,
+ NULL, NULL, NULL);
+
asm_inline volatile (VMWARE_HYPERCALL
: "=a" (out0)
: [port] "i" (VMWARE_HYPERVISOR_PORT),
@@ -85,6 +143,11 @@ unsigned long vmware_hypercall3(unsigned long cmd, unsigned long in1,
{
unsigned long out0;
+ if (cpu_feature_enabled(X86_FEATURE_TDX_GUEST))
+ return vmware_tdx_hypercall_args(cmd, in1, 0, 0, 0, 0,
+ out1, out2, NULL,
+ NULL, NULL, NULL);
+
asm_inline volatile (VMWARE_HYPERCALL
: "=a" (out0), "=b" (*out1), "=c" (*out2)
: [port] "i" (VMWARE_HYPERVISOR_PORT),
@@ -104,6 +167,11 @@ unsigned long vmware_hypercall4(unsigned long cmd, unsigned long in1,
{
unsigned long out0;
+ if (cpu_feature_enabled(X86_FEATURE_TDX_GUEST))
+ return vmware_tdx_hypercall_args(cmd, in1, 0, 0, 0, 0,
+ out1, out2, out3,
+ NULL, NULL, NULL);
+
asm_inline volatile (VMWARE_HYPERCALL
: "=a" (out0), "=b" (*out1), "=c" (*out2), "=d" (*out3)
: [port] "i" (VMWARE_HYPERVISOR_PORT),
@@ -123,6 +191,11 @@ unsigned long vmware_hypercall5(unsigned long cmd, unsigned long in1,
{
unsigned long out0;
+ if (cpu_feature_enabled(X86_FEATURE_TDX_GUEST))
+ return vmware_tdx_hypercall_args(cmd, in1, in3, in4, in5, 0,
+ NULL, out2, NULL,
+ NULL, NULL, NULL);
+
asm_inline volatile (VMWARE_HYPERCALL
: "=a" (out0), "=c" (*out2)
: [port] "i" (VMWARE_HYPERVISOR_PORT),
@@ -145,6 +218,11 @@ unsigned long vmware_hypercall6(unsigned long cmd, unsigned long in1,
{
unsigned long out0;
+ if (cpu_feature_enabled(X86_FEATURE_TDX_GUEST))
+ return vmware_tdx_hypercall_args(cmd, in1, in3, 0, 0, 0,
+ NULL, out2, out3,
+ out4, out5, NULL);
+
asm_inline volatile (VMWARE_HYPERCALL
: "=a" (out0), "=c" (*out2), "=d" (*out3), "=S" (*out4),
"=D" (*out5)
@@ -166,6 +244,11 @@ unsigned long vmware_hypercall7(unsigned long cmd, unsigned long in1,
{
unsigned long out0;
+ if (cpu_feature_enabled(X86_FEATURE_TDX_GUEST))
+ return vmware_tdx_hypercall_args(cmd, in1, in3, in4, in5, 0,
+ out1, out2, out3,
+ NULL, NULL, NULL);
+
asm_inline volatile (VMWARE_HYPERCALL
: "=a" (out0), "=b" (*out1), "=c" (*out2), "=d" (*out3)
: [port] "i" (VMWARE_HYPERVISOR_PORT),
diff --git a/arch/x86/kernel/cpu/vmware.c b/arch/x86/kernel/cpu/vmware.c
index 3aa1adaed18f..ef07ab7a07e1 100644
--- a/arch/x86/kernel/cpu/vmware.c
+++ b/arch/x86/kernel/cpu/vmware.c
@@ -428,6 +428,30 @@ static bool __init vmware_legacy_x2apic_available(void)
(eax & BIT(VCPU_LEGACY_X2APIC));
}
+#ifdef CONFIG_INTEL_TDX_GUEST
+unsigned long vmware_tdx_hypercall(unsigned long cmd,
+ struct tdx_module_args *args)
+{
+ if (!hypervisor_is_type(X86_HYPER_VMWARE))
+ return 0;
+
+ if (cmd & ~VMWARE_CMD_MASK) {
+ pr_warn("Out of range command %x\n", cmd);
+ return 0;
+ }
+
+ args->r10 = VMWARE_TDX_VENDOR_LEAF;
+ args->r11 = VMWARE_TDX_HCALL_FUNC;
+ args->r12 = VMWARE_HYPERVISOR_MAGIC;
+ args->r13 = cmd;
+
+ __tdx_hypercall(args);
+
+ return args->r12;
+}
+EXPORT_SYMBOL_GPL(vmware_tdx_hypercall);
+#endif
+
#ifdef CONFIG_AMD_MEM_ENCRYPT
static void vmware_sev_es_hcall_prepare(struct ghcb *ghcb,
struct pt_regs *regs)
--
2.39.0
^ permalink raw reply related
* Re: [PATCH 1/2] HID: wacom: Correct behavior when processing some confidence == false touches
From: Jiri Kosina @ 2023-12-19 23:03 UTC (permalink / raw)
To: Gerecke, Jason
Cc: linux-input, Benjamin Tissoires, Aaron Skomra, Jason Gerecke,
Joshua Dickens, Ping Cheng, Tatsunosuke Tobita,
Aaron Armstrong Skomra, Joshua Dickens, Ping Cheng, stable
In-Reply-To: <20231219213344.38434-1-jason.gerecke@wacom.com>
Jason,
both patches applied to hid.git#for-6.8/wacom.
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* [PATCH] HID: sensor-hub: Enable hid core report processing for all devices
From: Yauhen Kharuzhy @ 2023-12-19 23:15 UTC (permalink / raw)
To: linux-input, linux-iio
Cc: Daniel Thompson, linux-kernel, Jiri Kosina, Jonathan Cameron,
Srinivas Pandruvada, Benjamin Tissoires, Yauhen Kharuzhy
After the commit 666cf30a589a ("HID: sensor-hub: Allow multi-function
sensor devices") hub devices are claimed by hidraw driver in hid_connect().
This causes stoppping of processing HID reports by hid core due to
optimization.
In such case, the hid-sensor-custom driver cannot match a known custom
sensor in hid_sensor_custom_get_known() because it try to check custom
properties which weren't filled from the report because hid core didn't
parsed it.
As result, custom sensors like hinge angle sensor and LISS sensors
don't work.
Mark the sensor hub devices claimed by some driver to avoid hidraw-related
optimizations.
Signed-off-by: Yauhen Kharuzhy <jekhor@gmail.com>
---
drivers/hid/hid-sensor-hub.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
index 2eba152e8b90..26e93a331a51 100644
--- a/drivers/hid/hid-sensor-hub.c
+++ b/drivers/hid/hid-sensor-hub.c
@@ -632,7 +632,7 @@ static int sensor_hub_probe(struct hid_device *hdev,
}
INIT_LIST_HEAD(&hdev->inputs);
- ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+ ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT | HID_CONNECT_DRIVER);
if (ret) {
hid_err(hdev, "hw start failed\n");
return ret;
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v3 2/6] x86/vmware: Introduce vmware_hypercall API
From: kirill.shutemov @ 2023-12-19 23:20 UTC (permalink / raw)
To: Alexey Makhalov
Cc: linux-kernel, virtualization, bp, hpa, dave.hansen, mingo, tglx,
x86, netdev, richardcochran, linux-input, dmitry.torokhov, zackr,
linux-graphics-maintainer, pv-drivers, namit, timothym, akaher,
jsipek, dri-devel, daniel, airlied, tzimmermann, mripard,
maarten.lankhorst, horms
In-Reply-To: <20231219215751.9445-3-alexey.makhalov@broadcom.com>
On Tue, Dec 19, 2023 at 01:57:47PM -0800, Alexey Makhalov wrote:
> +static inline
> +unsigned long vmware_hypercall1(unsigned long cmd, unsigned long in1)
...
> +static inline
> +unsigned long vmware_hypercall3(unsigned long cmd, unsigned long in1,
> + uint32_t *out1, uint32_t *out2)
...
> +static inline
> +unsigned long vmware_hypercall4(unsigned long cmd, unsigned long in1,
> + uint32_t *out1, uint32_t *out2,
> + uint32_t *out3)
...
> +static inline
> +unsigned long vmware_hypercall5(unsigned long cmd, unsigned long in1,
> + unsigned long in3, unsigned long in4,
> + unsigned long in5, uint32_t *out2)
...
> +static inline
> +unsigned long vmware_hypercall6(unsigned long cmd, unsigned long in1,
> + unsigned long in3, uint32_t *out2,
> + uint32_t *out3, uint32_t *out4,
> + uint32_t *out5)
...
> +static inline
> +unsigned long vmware_hypercall7(unsigned long cmd, unsigned long in1,
> + unsigned long in3, unsigned long in4,
> + unsigned long in5, uint32_t *out1,
> + uint32_t *out2, uint32_t *out3)
Naming is weird. The number in the name doesn't help much as there seems
no system on how many of the parameters are ins and outs.
Why these combinations of ins/outs are supported?
And as an outsider, I'm curious where in2 got lost :P
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
* Re: [PATCH v3 6/6] x86/vmware: Add TDX hypercall support
From: kirill.shutemov @ 2023-12-19 23:23 UTC (permalink / raw)
To: Alexey Makhalov
Cc: linux-kernel, virtualization, bp, hpa, dave.hansen, mingo, tglx,
x86, netdev, richardcochran, linux-input, dmitry.torokhov, zackr,
linux-graphics-maintainer, pv-drivers, namit, timothym, akaher,
jsipek, dri-devel, daniel, airlied, tzimmermann, mripard,
maarten.lankhorst, horms
In-Reply-To: <20231219215751.9445-7-alexey.makhalov@broadcom.com>
On Tue, Dec 19, 2023 at 01:57:51PM -0800, Alexey Makhalov wrote:
> diff --git a/arch/x86/kernel/cpu/vmware.c b/arch/x86/kernel/cpu/vmware.c
> index 3aa1adaed18f..ef07ab7a07e1 100644
> --- a/arch/x86/kernel/cpu/vmware.c
> +++ b/arch/x86/kernel/cpu/vmware.c
> @@ -428,6 +428,30 @@ static bool __init vmware_legacy_x2apic_available(void)
> (eax & BIT(VCPU_LEGACY_X2APIC));
> }
>
> +#ifdef CONFIG_INTEL_TDX_GUEST
> +unsigned long vmware_tdx_hypercall(unsigned long cmd,
> + struct tdx_module_args *args)
> +{
> + if (!hypervisor_is_type(X86_HYPER_VMWARE))
> + return 0;
> +
> + if (cmd & ~VMWARE_CMD_MASK) {
> + pr_warn("Out of range command %x\n", cmd);
> + return 0;
Is zero success? Shouldn't it be an error?
> + }
> +
> + args->r10 = VMWARE_TDX_VENDOR_LEAF;
> + args->r11 = VMWARE_TDX_HCALL_FUNC;
> + args->r12 = VMWARE_HYPERVISOR_MAGIC;
> + args->r13 = cmd;
> +
> + __tdx_hypercall(args);
> +
> + return args->r12;
> +}
> +EXPORT_SYMBOL_GPL(vmware_tdx_hypercall);
> +#endif
> +
> #ifdef CONFIG_AMD_MEM_ENCRYPT
> static void vmware_sev_es_hcall_prepare(struct ghcb *ghcb,
> struct pt_regs *regs)
> --
> 2.39.0
>
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
* Re: [RFC] dt-bindings: input: Clarify that abs_min must be less than abs_max
From: Peter Hutterer @ 2023-12-19 23:48 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Artur Rojek, Chris Morgan, linux-input, devicetree, conor+dt,
krzysztof.kozlowski+dt, robh+dt, Chris Morgan, Paul Cercueil
In-Reply-To: <ZYH-a3TCBuJ00GvG@google.com>
On Tue, Dec 19, 2023 at 12:34:51PM -0800, Dmitry Torokhov wrote:
> Sorry, meant to add Peter Hutterer to the conversation, but forgot
> before hitting send...
Thx for the CC, I only saw the other patch and had missed this one.
> On Tue, Dec 19, 2023 at 12:32:44PM -0800, Dmitry Torokhov wrote:
> > On Fri, Dec 15, 2023 at 12:19:51PM +0100, Artur Rojek wrote:
> > > On 2023-12-15 03:40, Chris Morgan wrote:
> > > > From: Chris Morgan <macromorgan@hotmail.com>
> > > >
> > > > uinput refuses to work with abs devices where the min value is greater
> > > > than the max value. uinput_validate_absinfo() returns -EINVAL if this
> > > > is the case and prevents using uinput on such a device. Since uinput
> > > > has worked this way since at least kernel 2.6 (or prior) I presume that
> > > > this is the correct way of doing things, and that this documentation
> > > > needs to be clarified that min must always be less than max.
> > > >
> > > > uinput is used in my use case to bind together adc-joystick devices
> > > > with gpio-keys devices to create a single unified gamepad for
> > > > userspace.
> > > >
> > > > Note that there are several boards that will need to be corrected,
> > > > all but a few of them I maintain. Submitting as an RFC for now to get
> > > > comments from the input team and the original author in case there is
> > > > something I am missing.
> > > >
> > > > Fixes: 7956b0d4694f ("dt-bindings: input: Add docs for ADC driven
> > > > joystick")
> > > >
> > > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > > > ---
> > > > Documentation/devicetree/bindings/input/adc-joystick.yaml | 5 +++--
> > > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/input/adc-joystick.yaml
> > > > b/Documentation/devicetree/bindings/input/adc-joystick.yaml
> > > > index 6c244d66f8ce..8f5cdd5ef190 100644
> > > > --- a/Documentation/devicetree/bindings/input/adc-joystick.yaml
> > > > +++ b/Documentation/devicetree/bindings/input/adc-joystick.yaml
> > > > @@ -73,8 +73,9 @@ patternProperties:
> > > > description: >
> > > > Minimum and maximum values produced by the axis.
> > > > For an ABS_X axis this will be the left-most and right-most
> > > > - inclination of the joystick. If min > max, it is left to
> > > > userspace to
> > > > - treat the axis as inverted.
> > > > + inclination of the joystick. The axis must always be
> > > > expressed as
> > > > + min < max, if the axis is inverted it is left to userspace to
> > > > handle
> > > > + the inversion.
> > >
> > > Hi Chris,
> > >
> > > Device Tree is supposed to depict the actual state of the hardware.
> > > I worded the adc-joytick's adc-range property specifically, so that it
> > > covers a case of GCW Zero hardware [1], which has a joystick, where the
> > > ABS_X axis reports increasing values for the left-wards inclination of
> > > the joystick, and decreasing values for the right-wards inclination. You
> > > are saying that there are even more boards that need to be corrected -
> > > those are all situations, where DT depicts the actual behavior of the
> > > hardware.
> > > What you are trying to do is change hardware description, because of how
> > > a driver in an OS works. You should instead fix behavior of said driver,
> > > even if nobody stumbled upon that issue since 2.6 :) We fixed libSDL [2]
> > > for the same reason.
> >
> > We have several places in the kernel (such as mousedev and joydev) where
> > we expect that max is greater or equal to min if they are specified. I
> > am sure that at least some userspace components also have this
> > assumption. In general, we expect min to be a minimum value reported and
> > max being maximum value reported, and orientation expressed via
> > different properties (see [1]).
> >
> > Since we codified min > max as inversion for adc-joystick devices in the
> > bindings, I think we need to handle this *in that driver* and leave the
> > rest alone.
> >
> > [1] Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
I quickly checked libevdev and libinput and neither of them have checks
for min < max but the base assumption is there. So we'll get
entertaining results if that stops being the case.
Cheers,
Peter
^ permalink raw reply
* Re: [PATCH] input: uinput: Drop checks for abs_min > abs_max
From: Peter Hutterer @ 2023-12-19 23:51 UTC (permalink / raw)
To: Chris Morgan
Cc: linux-input, dmitry.torokhov, svv, biswarupp, paul, contact,
Chris Morgan
In-Reply-To: <20231218171653.141941-1-macroalpha82@gmail.com>
On Mon, Dec 18, 2023 at 11:16:53AM -0600, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
>
> Stop checking if the minimum abs value is greater than the maximum abs
> value. When the axis is inverted this condition is allowed. Without
> relaxing this check, it is not possible to use uinput on devices in
> userspace with an inverted axis, such as the adc-joystick found on
> many handheld gaming devices.
As mentioned in the other thread [1] a fair bit of userspace relies on
that general assumption so removing it will likely cause all sorts of
issues.
Cheers,
Petre
[1] https://lore.kernel.org/linux-input/20231219234803.GA3396969@quokka/T/#t
>
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> ---
> drivers/input/misc/uinput.c | 9 +--------
> 1 file changed, 1 insertion(+), 8 deletions(-)
>
> diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
> index d98212d55108..e90dbf2c0b34 100644
> --- a/drivers/input/misc/uinput.c
> +++ b/drivers/input/misc/uinput.c
> @@ -403,14 +403,7 @@ static int uinput_validate_absinfo(struct input_dev *dev, unsigned int code,
> min = abs->minimum;
> max = abs->maximum;
>
> - if ((min != 0 || max != 0) && max < min) {
> - printk(KERN_DEBUG
> - "%s: invalid abs[%02x] min:%d max:%d\n",
> - UINPUT_NAME, code, min, max);
> - return -EINVAL;
> - }
> -
> - if (!check_sub_overflow(max, min, &range) && abs->flat > range) {
> + if (!check_sub_overflow(max, min, &range) && abs->flat > abs(range)) {
> printk(KERN_DEBUG
> "%s: abs_flat #%02x out of range: %d (min:%d/max:%d)\n",
> UINPUT_NAME, code, abs->flat, min, max);
> --
> 2.34.1
>
>
^ permalink raw reply
* Re: [PATCH v3 2/6] x86/vmware: Introduce vmware_hypercall API
From: Alexey Makhalov @ 2023-12-20 0:17 UTC (permalink / raw)
To: kirill.shutemov
Cc: linux-kernel, virtualization, bp, hpa, dave.hansen, mingo, tglx,
x86, netdev, richardcochran, linux-input, dmitry.torokhov, zackr,
linux-graphics-maintainer, pv-drivers, namit, timothym, akaher,
jsipek, dri-devel, daniel, airlied, tzimmermann, mripard,
maarten.lankhorst, horms
In-Reply-To: <20231219232023.u4dyuvbzbh565grk@box.shutemov.name>
On 12/19/23 3:20 PM, kirill.shutemov@linux.intel.com wrote:
> On Tue, Dec 19, 2023 at 01:57:47PM -0800, Alexey Makhalov wrote:
>> +static inline
>> +unsigned long vmware_hypercall1(unsigned long cmd, unsigned long in1)
> ...
>> +static inline
>> +unsigned long vmware_hypercall3(unsigned long cmd, unsigned long in1,
>> + uint32_t *out1, uint32_t *out2)
> ...
>> +static inline
>> +unsigned long vmware_hypercall4(unsigned long cmd, unsigned long in1,
>> + uint32_t *out1, uint32_t *out2,
>> + uint32_t *out3)
> ...
>> +static inline
>> +unsigned long vmware_hypercall5(unsigned long cmd, unsigned long in1,
>> + unsigned long in3, unsigned long in4,
>> + unsigned long in5, uint32_t *out2)
> ...
>> +static inline
>> +unsigned long vmware_hypercall6(unsigned long cmd, unsigned long in1,
>> + unsigned long in3, uint32_t *out2,
>> + uint32_t *out3, uint32_t *out4,
>> + uint32_t *out5)
> ...
>> +static inline
>> +unsigned long vmware_hypercall7(unsigned long cmd, unsigned long in1,
>> + unsigned long in3, unsigned long in4,
>> + unsigned long in5, uint32_t *out1,
>> + uint32_t *out2, uint32_t *out3)
>
> Naming is weird. The number in the name doesn't help much as there seems
> no system on how many of the parameters are ins and outs.
There was internal discussion on hypercall API naming. One of proposals
was using 2 digits - number of input and number of output arguments.
And it definitely looked weird. So, we agreed to have just single number
- total number of arguments excluding cmd.
>
> Why these combinations of ins/outs are supported?
VMware hypercalls can use up to 6 ins and 6 outs for LB and 7 ins and 7
outs for HB calls. The mapping to x86 registers is below:
in0/out0 - rax
in1/out1 - rbx
in2/out2 - rcx
in3/out3 - rdx
in4/out4 - rsi
in5/out5 - rdi
in6/out6 - rbp (only used in high bandwidth hypercalls)
args 0, 2 and 6 are remapped to r12, r13 and r14 for TDX.
There is a standard on some arguments such as cmd on in2, magic on in0
and output value is on out0. While other arguments are not standardized
across hypercall.
Theoreticaly max hypercall, in term of number of arguments:
vmware_hypercall9(cmd, in1, in3, in4, in5, *out1, *out2, *out3, *out4,
*out5)
But there is no such called in a linux kernel.
Current combination of hypercalls covers all current and future (not yet
upstreamed) callers, with round up to next number in some cases.
>
> And as an outsider, I'm curious where in2 got lost :P
>
'lost' arguments:
in0 - indirectly initialized inside hypercall function.
out0 - return value from the hypercall.
[LB hypercalls] in2 <- input cmd
[HB hypercalls] in1 <- input cmd
Regards,
--Alexey
^ permalink raw reply
* Re: [RFC] dt-bindings: input: Clarify that abs_min must be less than abs_max
From: Paul Cercueil @ 2023-12-20 0:23 UTC (permalink / raw)
To: Dmitry Torokhov, Artur Rojek
Cc: Chris Morgan, linux-input, devicetree, conor+dt,
krzysztof.kozlowski+dt, robh+dt, Chris Morgan
In-Reply-To: <ZYH97KVDO4lFsbmi@google.com>
Hi Dmitry,
Le mardi 19 décembre 2023 à 12:32 -0800, Dmitry Torokhov a écrit :
> On Fri, Dec 15, 2023 at 12:19:51PM +0100, Artur Rojek wrote:
> > On 2023-12-15 03:40, Chris Morgan wrote:
> > > From: Chris Morgan <macromorgan@hotmail.com>
> > >
> > > uinput refuses to work with abs devices where the min value is
> > > greater
> > > than the max value. uinput_validate_absinfo() returns -EINVAL if
> > > this
> > > is the case and prevents using uinput on such a device. Since
> > > uinput
> > > has worked this way since at least kernel 2.6 (or prior) I
> > > presume that
> > > this is the correct way of doing things, and that this
> > > documentation
> > > needs to be clarified that min must always be less than max.
> > >
> > > uinput is used in my use case to bind together adc-joystick
> > > devices
> > > with gpio-keys devices to create a single unified gamepad for
> > > userspace.
> > >
> > > Note that there are several boards that will need to be
> > > corrected,
> > > all but a few of them I maintain. Submitting as an RFC for now to
> > > get
> > > comments from the input team and the original author in case
> > > there is
> > > something I am missing.
> > >
> > > Fixes: 7956b0d4694f ("dt-bindings: input: Add docs for ADC driven
> > > joystick")
> > >
> > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > > ---
> > > Documentation/devicetree/bindings/input/adc-joystick.yaml | 5
> > > +++--
> > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/input/adc-
> > > joystick.yaml
> > > b/Documentation/devicetree/bindings/input/adc-joystick.yaml
> > > index 6c244d66f8ce..8f5cdd5ef190 100644
> > > --- a/Documentation/devicetree/bindings/input/adc-joystick.yaml
> > > +++ b/Documentation/devicetree/bindings/input/adc-joystick.yaml
> > > @@ -73,8 +73,9 @@ patternProperties:
> > > description: >
> > > Minimum and maximum values produced by the axis.
> > > For an ABS_X axis this will be the left-most and
> > > right-most
> > > - inclination of the joystick. If min > max, it is left
> > > to
> > > userspace to
> > > - treat the axis as inverted.
> > > + inclination of the joystick. The axis must always be
> > > expressed as
> > > + min < max, if the axis is inverted it is left to
> > > userspace to
> > > handle
> > > + the inversion.
> >
> > Hi Chris,
> >
> > Device Tree is supposed to depict the actual state of the hardware.
> > I worded the adc-joytick's adc-range property specifically, so that
> > it
> > covers a case of GCW Zero hardware [1], which has a joystick,
> > where the
> > ABS_X axis reports increasing values for the left-wards inclination
> > of
> > the joystick, and decreasing values for the right-wards
> > inclination. You
> > are saying that there are even more boards that need to be
> > corrected -
> > those are all situations, where DT depicts the actual behavior of
> > the
> > hardware.
> > What you are trying to do is change hardware description, because
> > of how
> > a driver in an OS works. You should instead fix behavior of said
> > driver,
> > even if nobody stumbled upon that issue since 2.6 :) We fixed
> > libSDL [2]
> > for the same reason.
>
> We have several places in the kernel (such as mousedev and joydev)
> where
> we expect that max is greater or equal to min if they are specified.
> I
> am sure that at least some userspace components also have this
> assumption. In general, we expect min to be a minimum value reported
> and
> max being maximum value reported, and orientation expressed via
> different properties (see [1]).
You can express all you want in DT properties the orientation of the
axis, it does not carry to userspace. As far as I can see there is
absolutely no way to tell userspace that an axis is inverted.
Cheers,
-Paul
>
> Since we codified min > max as inversion for adc-joystick devices in
> the
> bindings, I think we need to handle this *in that driver* and leave
> the
> rest alone.
>
> [1]
> Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
>
> Thanks.
>
^ permalink raw reply
* Re: [PATCH v3 6/6] x86/vmware: Add TDX hypercall support
From: Alexey Makhalov @ 2023-12-20 0:27 UTC (permalink / raw)
To: kirill.shutemov
Cc: linux-kernel, virtualization, bp, hpa, dave.hansen, mingo, tglx,
x86, netdev, richardcochran, linux-input, dmitry.torokhov, zackr,
linux-graphics-maintainer, pv-drivers, namit, timothym, akaher,
jsipek, dri-devel, daniel, airlied, tzimmermann, mripard,
maarten.lankhorst, horms
In-Reply-To: <20231219232323.euweerulgsgbodx5@box.shutemov.name>
On 12/19/23 3:23 PM, kirill.shutemov@linux.intel.com wrote:
> On Tue, Dec 19, 2023 at 01:57:51PM -0800, Alexey Makhalov wrote:
>> diff --git a/arch/x86/kernel/cpu/vmware.c b/arch/x86/kernel/cpu/vmware.c
>> index 3aa1adaed18f..ef07ab7a07e1 100644
>> --- a/arch/x86/kernel/cpu/vmware.c
>> +++ b/arch/x86/kernel/cpu/vmware.c
>> @@ -428,6 +428,30 @@ static bool __init vmware_legacy_x2apic_available(void)
>> (eax & BIT(VCPU_LEGACY_X2APIC));
>> }
>>
>> +#ifdef CONFIG_INTEL_TDX_GUEST
>> +unsigned long vmware_tdx_hypercall(unsigned long cmd,
>> + struct tdx_module_args *args)
>> +{
>> + if (!hypervisor_is_type(X86_HYPER_VMWARE))
>> + return 0;
>> +
>> + if (cmd & ~VMWARE_CMD_MASK) {
>> + pr_warn("Out of range command %x\n", cmd);
>> + return 0;
>
> Is zero success? Shouldn't it be an error?
VMware hypercalls do not have a standard way of signalling an error.
To generalize expectations from the caller perspective of any existing
hypercalls: error (including hypercall is not supported or disabled) is
when return value is 0 and out1/2 are unchanged or equal to in1/in2.
All existing vmware_hypercall callers will gracefully handle returned 0.
But they should never hit this path, as 0 bail out was introduced as a
protection for the case where exported vmware_tdx_hypercall is used by
random caller (not following VMware hypercall ABI).
>
>> + }
>> +
>> + args->r10 = VMWARE_TDX_VENDOR_LEAF;
>> + args->r11 = VMWARE_TDX_HCALL_FUNC;
>> + args->r12 = VMWARE_HYPERVISOR_MAGIC;
>> + args->r13 = cmd;
>> +
>> + __tdx_hypercall(args);
>> +
>> + return args->r12;
>> +}
>> +EXPORT_SYMBOL_GPL(vmware_tdx_hypercall);
>> +#endif
>> +
>> #ifdef CONFIG_AMD_MEM_ENCRYPT
>> static void vmware_sev_es_hcall_prepare(struct ghcb *ghcb,
>> struct pt_regs *regs)
>> --
>> 2.39.0
>>
>
^ permalink raw reply
* Re: [PATCH] input: uinput: Drop checks for abs_min > abs_max
From: Paul Cercueil @ 2023-12-20 0:38 UTC (permalink / raw)
To: Peter Hutterer, Chris Morgan
Cc: linux-input, dmitry.torokhov, svv, biswarupp, contact,
Chris Morgan
In-Reply-To: <20231219235149.GA3401344@quokka>
Hi Peter,
Le mercredi 20 décembre 2023 à 09:51 +1000, Peter Hutterer a écrit :
> On Mon, Dec 18, 2023 at 11:16:53AM -0600, Chris Morgan wrote:
> > From: Chris Morgan <macromorgan@hotmail.com>
> >
> > Stop checking if the minimum abs value is greater than the maximum
> > abs
> > value. When the axis is inverted this condition is allowed. Without
> > relaxing this check, it is not possible to use uinput on devices in
> > userspace with an inverted axis, such as the adc-joystick found on
> > many handheld gaming devices.
>
> As mentioned in the other thread [1] a fair bit of userspace relies
> on
> that general assumption so removing it will likely cause all sorts of
> issues.
There is some userspace that works with it though, so why restrict it
artificially?
The fact that some other userspace code wouldn't work with it sounds a
bit irrelevant. They just never encountered that min>max usage before.
And removing this check won't cause all sort of issues, why would it?
It's not like the current software actively probes min>max and crash
badly if it doesn't return -EINVAL...
Cheers,
-Paul
>
> Cheers,
> Petre
>
> [1]
> https://lore.kernel.org/linux-input/20231219234803.GA3396969@quokka/T/#t
> >
> > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > ---
> > drivers/input/misc/uinput.c | 9 +--------
> > 1 file changed, 1 insertion(+), 8 deletions(-)
> >
> > diff --git a/drivers/input/misc/uinput.c
> > b/drivers/input/misc/uinput.c
> > index d98212d55108..e90dbf2c0b34 100644
> > --- a/drivers/input/misc/uinput.c
> > +++ b/drivers/input/misc/uinput.c
> > @@ -403,14 +403,7 @@ static int uinput_validate_absinfo(struct
> > input_dev *dev, unsigned int code,
> > min = abs->minimum;
> > max = abs->maximum;
> >
> > - if ((min != 0 || max != 0) && max < min) {
> > - printk(KERN_DEBUG
> > - "%s: invalid abs[%02x] min:%d max:%d\n",
> > - UINPUT_NAME, code, min, max);
> > - return -EINVAL;
> > - }
> > -
> > - if (!check_sub_overflow(max, min, &range) && abs->flat >
> > range) {
> > + if (!check_sub_overflow(max, min, &range) && abs->flat >
> > abs(range)) {
> > printk(KERN_DEBUG
> > "%s: abs_flat #%02x out of range: %d
> > (min:%d/max:%d)\n",
> > UINPUT_NAME, code, abs->flat, min, max);
> > --
> > 2.34.1
> >
> >
^ permalink raw reply
* Re: [PATCH v3 2/6] x86/vmware: Introduce vmware_hypercall API
From: kirill.shutemov @ 2023-12-20 0:51 UTC (permalink / raw)
To: Alexey Makhalov
Cc: linux-kernel, virtualization, bp, hpa, dave.hansen, mingo, tglx,
x86, netdev, richardcochran, linux-input, dmitry.torokhov, zackr,
linux-graphics-maintainer, pv-drivers, namit, timothym, akaher,
jsipek, dri-devel, daniel, airlied, tzimmermann, mripard,
maarten.lankhorst, horms
In-Reply-To: <75eed318-2d22-429d-ab95-80610ba82934@broadcom.com>
On Tue, Dec 19, 2023 at 04:17:40PM -0800, Alexey Makhalov wrote:
>
>
> On 12/19/23 3:20 PM, kirill.shutemov@linux.intel.com wrote:
> > On Tue, Dec 19, 2023 at 01:57:47PM -0800, Alexey Makhalov wrote:
> > > +static inline
> > > +unsigned long vmware_hypercall1(unsigned long cmd, unsigned long in1)
> > ...
> > > +static inline
> > > +unsigned long vmware_hypercall3(unsigned long cmd, unsigned long in1,
> > > + uint32_t *out1, uint32_t *out2)
> > ...
> > > +static inline
> > > +unsigned long vmware_hypercall4(unsigned long cmd, unsigned long in1,
> > > + uint32_t *out1, uint32_t *out2,
> > > + uint32_t *out3)
> > ...
> > > +static inline
> > > +unsigned long vmware_hypercall5(unsigned long cmd, unsigned long in1,
> > > + unsigned long in3, unsigned long in4,
> > > + unsigned long in5, uint32_t *out2)
> > ...
> > > +static inline
> > > +unsigned long vmware_hypercall6(unsigned long cmd, unsigned long in1,
> > > + unsigned long in3, uint32_t *out2,
> > > + uint32_t *out3, uint32_t *out4,
> > > + uint32_t *out5)
> > ...
> > > +static inline
> > > +unsigned long vmware_hypercall7(unsigned long cmd, unsigned long in1,
> > > + unsigned long in3, unsigned long in4,
> > > + unsigned long in5, uint32_t *out1,
> > > + uint32_t *out2, uint32_t *out3)
> >
> > Naming is weird. The number in the name doesn't help much as there seems
> > no system on how many of the parameters are ins and outs.
>
> There was internal discussion on hypercall API naming. One of proposals was
> using 2 digits - number of input and number of output arguments.
> And it definitely looked weird. So, we agreed to have just single number -
> total number of arguments excluding cmd.
Have you considered naming them by number of input parameters? Number of
output parameters as demanded by users.
So vmware_hypercall4() will become vmware_hypercall1() and current
vmware_hypercall1() and vmware_hypercall3() will go away.
It is still awful, but /maybe/ better that this, I donno.
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
* Re: [PATCH v3 6/6] x86/vmware: Add TDX hypercall support
From: kirill.shutemov @ 2023-12-20 1:00 UTC (permalink / raw)
To: Alexey Makhalov
Cc: linux-kernel, virtualization, bp, hpa, dave.hansen, mingo, tglx,
x86, netdev, richardcochran, linux-input, dmitry.torokhov, zackr,
linux-graphics-maintainer, pv-drivers, namit, timothym, akaher,
jsipek, dri-devel, daniel, airlied, tzimmermann, mripard,
maarten.lankhorst, horms
In-Reply-To: <ba679460-827d-40b1-bc78-bcee1c013f36@broadcom.com>
On Tue, Dec 19, 2023 at 04:27:51PM -0800, Alexey Makhalov wrote:
>
>
> On 12/19/23 3:23 PM, kirill.shutemov@linux.intel.com wrote:
> > On Tue, Dec 19, 2023 at 01:57:51PM -0800, Alexey Makhalov wrote:
> > > diff --git a/arch/x86/kernel/cpu/vmware.c b/arch/x86/kernel/cpu/vmware.c
> > > index 3aa1adaed18f..ef07ab7a07e1 100644
> > > --- a/arch/x86/kernel/cpu/vmware.c
> > > +++ b/arch/x86/kernel/cpu/vmware.c
> > > @@ -428,6 +428,30 @@ static bool __init vmware_legacy_x2apic_available(void)
> > > (eax & BIT(VCPU_LEGACY_X2APIC));
> > > }
> > > +#ifdef CONFIG_INTEL_TDX_GUEST
> > > +unsigned long vmware_tdx_hypercall(unsigned long cmd,
> > > + struct tdx_module_args *args)
> > > +{
> > > + if (!hypervisor_is_type(X86_HYPER_VMWARE))
> > > + return 0;
BTW, don't you want to warn here to? We don't expect vmware hypercalls to
be called by non-vmware guest, do we?
> > > +
> > > + if (cmd & ~VMWARE_CMD_MASK) {
> > > + pr_warn("Out of range command %x\n", cmd);
> > > + return 0;
> >
> > Is zero success? Shouldn't it be an error?
>
> VMware hypercalls do not have a standard way of signalling an error.
> To generalize expectations from the caller perspective of any existing
> hypercalls: error (including hypercall is not supported or disabled) is when
> return value is 0 and out1/2 are unchanged or equal to in1/in2.
You are talking about signaling errors over hypercall transport. But if
kernel can see that something is wrong why cannot it signal the issue
clearly to caller. It is going to be in-kernel convention.
And to very least, it has to be pr_warn_once().
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
* Re: [RFC] dt-bindings: input: Clarify that abs_min must be less than abs_max
From: Dmitry Torokhov @ 2023-12-20 1:22 UTC (permalink / raw)
To: Paul Cercueil
Cc: Artur Rojek, Chris Morgan, linux-input, devicetree, conor+dt,
krzysztof.kozlowski+dt, robh+dt, Chris Morgan
In-Reply-To: <fb38504f15148c78c7a91850daf8c247d592b0a0.camel@crapouillou.net>
On Wed, Dec 20, 2023 at 01:23:31AM +0100, Paul Cercueil wrote:
> Hi Dmitry,
>
> Le mardi 19 décembre 2023 à 12:32 -0800, Dmitry Torokhov a écrit :
> > On Fri, Dec 15, 2023 at 12:19:51PM +0100, Artur Rojek wrote:
> > > On 2023-12-15 03:40, Chris Morgan wrote:
> > > > From: Chris Morgan <macromorgan@hotmail.com>
> > > >
> > > > uinput refuses to work with abs devices where the min value is
> > > > greater
> > > > than the max value. uinput_validate_absinfo() returns -EINVAL if
> > > > this
> > > > is the case and prevents using uinput on such a device. Since
> > > > uinput
> > > > has worked this way since at least kernel 2.6 (or prior) I
> > > > presume that
> > > > this is the correct way of doing things, and that this
> > > > documentation
> > > > needs to be clarified that min must always be less than max.
> > > >
> > > > uinput is used in my use case to bind together adc-joystick
> > > > devices
> > > > with gpio-keys devices to create a single unified gamepad for
> > > > userspace.
> > > >
> > > > Note that there are several boards that will need to be
> > > > corrected,
> > > > all but a few of them I maintain. Submitting as an RFC for now to
> > > > get
> > > > comments from the input team and the original author in case
> > > > there is
> > > > something I am missing.
> > > >
> > > > Fixes: 7956b0d4694f ("dt-bindings: input: Add docs for ADC driven
> > > > joystick")
> > > >
> > > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > > > ---
> > > > Documentation/devicetree/bindings/input/adc-joystick.yaml | 5
> > > > +++--
> > > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/input/adc-
> > > > joystick.yaml
> > > > b/Documentation/devicetree/bindings/input/adc-joystick.yaml
> > > > index 6c244d66f8ce..8f5cdd5ef190 100644
> > > > --- a/Documentation/devicetree/bindings/input/adc-joystick.yaml
> > > > +++ b/Documentation/devicetree/bindings/input/adc-joystick.yaml
> > > > @@ -73,8 +73,9 @@ patternProperties:
> > > > description: >
> > > > Minimum and maximum values produced by the axis.
> > > > For an ABS_X axis this will be the left-most and
> > > > right-most
> > > > - inclination of the joystick. If min > max, it is left
> > > > to
> > > > userspace to
> > > > - treat the axis as inverted.
> > > > + inclination of the joystick. The axis must always be
> > > > expressed as
> > > > + min < max, if the axis is inverted it is left to
> > > > userspace to
> > > > handle
> > > > + the inversion.
> > >
> > > Hi Chris,
> > >
> > > Device Tree is supposed to depict the actual state of the hardware.
> > > I worded the adc-joytick's adc-range property specifically, so that
> > > it
> > > covers a case of GCW Zero hardware [1], which has a joystick,
> > > where the
> > > ABS_X axis reports increasing values for the left-wards inclination
> > > of
> > > the joystick, and decreasing values for the right-wards
> > > inclination. You
> > > are saying that there are even more boards that need to be
> > > corrected -
> > > those are all situations, where DT depicts the actual behavior of
> > > the
> > > hardware.
> > > What you are trying to do is change hardware description, because
> > > of how
> > > a driver in an OS works. You should instead fix behavior of said
> > > driver,
> > > even if nobody stumbled upon that issue since 2.6 :) We fixed
> > > libSDL [2]
> > > for the same reason.
> >
> > We have several places in the kernel (such as mousedev and joydev)
> > where
> > we expect that max is greater or equal to min if they are specified.
> > I
> > am sure that at least some userspace components also have this
> > assumption. In general, we expect min to be a minimum value reported
> > and
> > max being maximum value reported, and orientation expressed via
> > different properties (see [1]).
>
> You can express all you want in DT properties the orientation of the
> axis, it does not carry to userspace. As far as I can see there is
> absolutely no way to tell userspace that an axis is inverted.
That is true, and I will argue that it is a feature. Kernel's task is to
normalize the data stream and present it to userspace so that it does
not have to deal with differences of hardware. This abstraction often
breaks, but when we can keep it, we should.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] input: uinput: Drop checks for abs_min > abs_max
From: Dmitry Torokhov @ 2023-12-20 1:53 UTC (permalink / raw)
To: Paul Cercueil
Cc: Peter Hutterer, Chris Morgan, linux-input, svv, biswarupp,
contact, Chris Morgan
In-Reply-To: <f77b98bf015bf3f8716422ac70c4fd6051e66376.camel@crapouillou.net>
Hi Paul,
On Wed, Dec 20, 2023 at 01:38:39AM +0100, Paul Cercueil wrote:
> Hi Peter,
>
> Le mercredi 20 décembre 2023 à 09:51 +1000, Peter Hutterer a écrit :
> > On Mon, Dec 18, 2023 at 11:16:53AM -0600, Chris Morgan wrote:
> > > From: Chris Morgan <macromorgan@hotmail.com>
> > >
> > > Stop checking if the minimum abs value is greater than the maximum
> > > abs
> > > value. When the axis is inverted this condition is allowed. Without
> > > relaxing this check, it is not possible to use uinput on devices in
> > > userspace with an inverted axis, such as the adc-joystick found on
> > > many handheld gaming devices.
> >
> > As mentioned in the other thread [1] a fair bit of userspace relies
> > on
> > that general assumption so removing it will likely cause all sorts of
> > issues.
>
> There is some userspace that works with it though, so why restrict it
> artificially?
>
> The fact that some other userspace code wouldn't work with it sounds a
> bit irrelevant. They just never encountered that min>max usage before.
>
> And removing this check won't cause all sort of issues, why would it?
> It's not like the current software actively probes min>max and crash
> badly if it doesn't return -EINVAL...
It will cause weird movements because calculations expect min be the
minimum, and max the maximum, and not encode left/right or up/down.
Putting this into adc joystick binding was a mistake.
This is the definition of absinfo:
/**
* struct input_absinfo - used by EVIOCGABS/EVIOCSABS ioctls
* @value: latest reported value for the axis.
* @minimum: specifies minimum value for the axis.
* @maximum: specifies maximum value for the axis.
* @fuzz: specifies fuzz value that is used to filter noise from
* the event stream.
* @flat: values that are within this value will be discarded by
* joydev interface and reported as 0 instead.
* @resolution: specifies resolution for the values reported for
* the axis.
*
* Note that input core does not clamp reported values to the
* [minimum, maximum] limits, such task is left to userspace.
...
*/
(We should change wording of the last sentence to "... does not always
clamp ..." since when we do inversion/swap we do clamp values.)
And note that the userspace that can handle swapped min and max will
work fine if the kernel provides normalized data, but other software
will/may not work.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v3 6/6] x86/vmware: Add TDX hypercall support
From: Alexey Makhalov @ 2023-12-20 3:02 UTC (permalink / raw)
To: kirill.shutemov
Cc: linux-kernel, virtualization, bp, hpa, dave.hansen, mingo, tglx,
x86, netdev, richardcochran, linux-input, dmitry.torokhov, zackr,
linux-graphics-maintainer, pv-drivers, namit, timothym, akaher,
jsipek, dri-devel, daniel, airlied, tzimmermann, mripard,
maarten.lankhorst, horms
In-Reply-To: <20231220010000.y5ybey76xjckvh6y@box.shutemov.name>
On 12/19/23 5:00 PM, kirill.shutemov@linux.intel.com wrote:
> On Tue, Dec 19, 2023 at 04:27:51PM -0800, Alexey Makhalov wrote:
>>
>>
>> On 12/19/23 3:23 PM, kirill.shutemov@linux.intel.com wrote:
>>> On Tue, Dec 19, 2023 at 01:57:51PM -0800, Alexey Makhalov wrote:
>>>> diff --git a/arch/x86/kernel/cpu/vmware.c b/arch/x86/kernel/cpu/vmware.c
>>>> index 3aa1adaed18f..ef07ab7a07e1 100644
>>>> --- a/arch/x86/kernel/cpu/vmware.c
>>>> +++ b/arch/x86/kernel/cpu/vmware.c
>>>> @@ -428,6 +428,30 @@ static bool __init vmware_legacy_x2apic_available(void)
>>>> (eax & BIT(VCPU_LEGACY_X2APIC));
>>>> }
>>>> +#ifdef CONFIG_INTEL_TDX_GUEST
>>>> +unsigned long vmware_tdx_hypercall(unsigned long cmd,
>>>> + struct tdx_module_args *args)
>>>> +{
>>>> + if (!hypervisor_is_type(X86_HYPER_VMWARE))
>>>> + return 0;
>
> BTW, don't you want to warn here to? We don't expect vmware hypercalls to
> be called by non-vmware guest, do we?
The answer is below...
>
>>>> +
>>>> + if (cmd & ~VMWARE_CMD_MASK) {
>>>> + pr_warn("Out of range command %x\n", cmd);
>>>> + return 0;
>>>
>>> Is zero success? Shouldn't it be an error?
>>
>> VMware hypercalls do not have a standard way of signalling an error.
>> To generalize expectations from the caller perspective of any existing
>> hypercalls: error (including hypercall is not supported or disabled) is when
>> return value is 0 and out1/2 are unchanged or equal to in1/in2.
>
> You are talking about signaling errors over hypercall transport. But if
> kernel can see that something is wrong why cannot it signal the issue
> clearly to caller. It is going to be in-kernel convention.These "return 0" blocks were introduced to protect against non-vmware
guest or arbitrary modules trying to use __tdx_hypercall via exported
vmware_tdx_hypercall function. In this case, it will be NOOP behavior
with no or minor side effects.
From valid vmware_hypercall callers point of view, there is no such
thing as a hypercall not available. Once guest detection code recognizes
VMWare hypervisor via cpuid, it will start using hypercalls in
accordance to per-call API.
Valid VMware guest code will never go into first return, no warning
required.
Second return can be hit in rare cases for example during development
phase, or, hypothetical case, when cmd was dynamically generated.
That's why we have a warning warning only for the second condition.
While speaking about it, I'm started to lean towards your
recommendation. Yes, we can return standard error code such as -EINVAL
or just -1 instead of "return 0" in this function. And it will be
algorithmically correct. As if Vmware guest caller provide out of range
cmd - it is not documented behavior.
Speaking of additional in-kernel convention for passing additional
parameter if error happens, it does not makes sense for me because:
1. existing caller codes analyze output argument to recognize error
error response from the hypervisor. Adding one additional check for
in-kernel errors just for TDX path which will be never hit by valid code
in production is an unnecessary overhead.
2. It will definitely add an overhead as an error code will require one
more output value, or out0 should be moved from return in-register value
to return by pointer function argument.
Summarizing, overloading vmware_tdx_hypercall return value by arg0 (from
the hypervisor) and kernel error (-1 or any other) seems like reasonable
change.
>
> And to very least, it has to be pr_warn_once().
>
Good catch! Will change it.
Thanks,
--Alexey
^ permalink raw reply
* Re: [PATCH v3 2/6] x86/vmware: Introduce vmware_hypercall API
From: Alexey Makhalov @ 2023-12-20 3:30 UTC (permalink / raw)
To: kirill.shutemov
Cc: linux-kernel, virtualization, bp, hpa, dave.hansen, mingo, tglx,
x86, netdev, richardcochran, linux-input, dmitry.torokhov, zackr,
linux-graphics-maintainer, pv-drivers, namit, timothym, akaher,
jsipek, dri-devel, daniel, airlied, tzimmermann, mripard,
maarten.lankhorst, horms
In-Reply-To: <20231220005156.2rymnxu5bv6wdwlx@box.shutemov.name>
On 12/19/23 4:51 PM, kirill.shutemov@linux.intel.com wrote:
> On Tue, Dec 19, 2023 at 04:17:40PM -0800, Alexey Makhalov wrote:
>>
>>
>> On 12/19/23 3:20 PM, kirill.shutemov@linux.intel.com wrote:
>>> On Tue, Dec 19, 2023 at 01:57:47PM -0800, Alexey Makhalov wrote:
>>>> +static inline
>>>> +unsigned long vmware_hypercall1(unsigned long cmd, unsigned long in1)
>>> ...
>>>> +static inline
>>>> +unsigned long vmware_hypercall3(unsigned long cmd, unsigned long in1,
>>>> + uint32_t *out1, uint32_t *out2)
>>> ...
>>>> +static inline
>>>> +unsigned long vmware_hypercall4(unsigned long cmd, unsigned long in1,
>>>> + uint32_t *out1, uint32_t *out2,
>>>> + uint32_t *out3)
>>> ...
>>>> +static inline
>>>> +unsigned long vmware_hypercall5(unsigned long cmd, unsigned long in1,
>>>> + unsigned long in3, unsigned long in4,
>>>> + unsigned long in5, uint32_t *out2)
>>> ...
>>>> +static inline
>>>> +unsigned long vmware_hypercall6(unsigned long cmd, unsigned long in1,
>>>> + unsigned long in3, uint32_t *out2,
>>>> + uint32_t *out3, uint32_t *out4,
>>>> + uint32_t *out5)
>>> ...
>>>> +static inline
>>>> +unsigned long vmware_hypercall7(unsigned long cmd, unsigned long in1,
>>>> + unsigned long in3, unsigned long in4,
>>>> + unsigned long in5, uint32_t *out1,
>>>> + uint32_t *out2, uint32_t *out3)
>>>
>>> Naming is weird. The number in the name doesn't help much as there seems
>>> no system on how many of the parameters are ins and outs.
>>
>> There was internal discussion on hypercall API naming. One of proposals was
>> using 2 digits - number of input and number of output arguments.
>> And it definitely looked weird. So, we agreed to have just single number -
>> total number of arguments excluding cmd.
>
> Have you considered naming them by number of input parameters? Number of
> output parameters as demanded by users.
>
> So vmware_hypercall4() will become vmware_hypercall1() and current
> vmware_hypercall1() and vmware_hypercall3() will go away.
>
> It is still awful, but /maybe/ better that this, I donno.
>
Deprecating vmware_hypercall1 and vmware_hypercall3 in favor of
vmware_hypercall4 will generate less efficient code for the caller of
first ones.
Using current vmware_hypercall4 instead of vmware_hypercall1 will force
the caller to allocate additional variables (register or on stack
memory) for hypercall asm inline to put additional output registers on.
And specifically to 'usage' of *out3 - compiler will unnecessary
'clobber' useful rdx, when hypervisor will keep it unchanged.
Unfortunately VMware hypercall ABI is not as beautiful as KVM one,
especially in number of output arguments and their ordering. rbp
register usage as an argument is a separate bummer((. So we have to work
with what we have.
Current set of functions includes only 6 functions (for LB), which is
the optimum between readability, maintainability and performance. It
covers all current kernel callers and all new callers from yet to be
upstreamed patches that we have in Photon OS including 2 patches for x86
and arm64 guest support.
Regards,
--Alexey
^ permalink raw reply
* [PATCH 0/7] HID: hid-steam: Upstream more SteamOS patches
From: Vicki Pfau @ 2023-12-20 3:36 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-input; +Cc: Vicki Pfau
This is a slew of patches that have been in testing for a while in SteamOS
betas in one form or another. Most of them are pretty straight-forward, though
I expect the gamepad-only mode may be preferred to be offloaded to a userspace
daemon. Right now, the gamepad-only mode is handled by Steam when it's running,
but has utility when it's not running too, given the presence of Lizard Mode
(the keyboard/mouse emulation system).
Vicki Pfau (7):
HID: hid-steam: Avoid overwriting smoothing parameter
HID: hid-steam: Disable watchdog instead of using a heartbeat
HID: hid-steam: Clean up locking
HID: hid-steam: Make client_opened a counter
HID: hid-steam: Update list of identifiers from SDL
HID: hid-steam: Better handling of serial number length
HID: hid-steam: Add gamepad-only mode switched to by holding options
drivers/hid/hid-steam.c | 547 ++++++++++++++++++++++++++++------------
1 file changed, 391 insertions(+), 156 deletions(-)
--
2.42.0
^ permalink raw reply
* [PATCH 1/7] HID: hid-steam: Avoid overwriting smoothing parameter
From: Vicki Pfau @ 2023-12-20 3:38 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-input; +Cc: Vicki Pfau
In-Reply-To: <20231220033609.2132033-1-vi@endrift.com>
The original implementation of this driver incorrectly guessed the function of
this register. It's not only unnecessary to write to this register for lizard
mode but actually counter-productive since it overwrites whatever previous
value was intentionally set, for example by Steam.
Signed-off-by: Vicki Pfau <vi@endrift.com>
---
drivers/hid/hid-steam.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
index b110818fc945..7aefd52e945a 100644
--- a/drivers/hid/hid-steam.c
+++ b/drivers/hid/hid-steam.c
@@ -340,9 +340,6 @@ static void steam_set_lizard_mode(struct steam_device *steam, bool enable)
steam_send_report_byte(steam, STEAM_CMD_DEFAULT_MAPPINGS);
/* enable mouse */
steam_send_report_byte(steam, STEAM_CMD_DEFAULT_MOUSE);
- steam_write_registers(steam,
- STEAM_REG_RPAD_MARGIN, 0x01, /* enable margin */
- 0);
cancel_delayed_work_sync(&steam->heartbeat);
} else {
@@ -351,7 +348,6 @@ static void steam_set_lizard_mode(struct steam_device *steam, bool enable)
if (steam->quirks & STEAM_QUIRK_DECK) {
steam_write_registers(steam,
- STEAM_REG_RPAD_MARGIN, 0x00, /* disable margin */
STEAM_REG_LPAD_MODE, 0x07, /* disable mouse */
STEAM_REG_RPAD_MODE, 0x07, /* disable mouse */
STEAM_REG_LPAD_CLICK_PRESSURE, 0xFFFF, /* disable clicky pad */
@@ -365,7 +361,6 @@ static void steam_set_lizard_mode(struct steam_device *steam, bool enable)
schedule_delayed_work(&steam->heartbeat, 5 * HZ);
} else {
steam_write_registers(steam,
- STEAM_REG_RPAD_MARGIN, 0x00, /* disable margin */
STEAM_REG_LPAD_MODE, 0x07, /* disable mouse */
STEAM_REG_RPAD_MODE, 0x07, /* disable mouse */
0);
--
2.42.0
^ permalink raw reply related
* [PATCH 2/7] HID: hid-steam: Disable watchdog instead of using a heartbeat
From: Vicki Pfau @ 2023-12-20 3:38 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-input; +Cc: Vicki Pfau
In-Reply-To: <20231220033837.2135194-1-vi@endrift.com>
The Steam Deck has a setting that controls whether or not the watchdog is
enabled, so instead of using a heartbeat to keep the watchdog from triggering,
this commit changes the behavior to simply disable the watchdog instead.
---
drivers/hid/hid-steam.c | 30 ++----------------------------
1 file changed, 2 insertions(+), 28 deletions(-)
diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
index 7aefd52e945a..efd297e0ea8c 100644
--- a/drivers/hid/hid-steam.c
+++ b/drivers/hid/hid-steam.c
@@ -101,6 +101,7 @@ static LIST_HEAD(steam_devices);
#define STEAM_REG_GYRO_MODE 0x30
#define STEAM_REG_LPAD_CLICK_PRESSURE 0x34
#define STEAM_REG_RPAD_CLICK_PRESSURE 0x35
+#define STEAM_REG_WATCHDOG_ENABLE 0x47
/* Raw event identifiers */
#define STEAM_EV_INPUT_DATA 0x01
@@ -134,7 +135,6 @@ struct steam_device {
struct power_supply __rcu *battery;
u8 battery_charge;
u16 voltage;
- struct delayed_work heartbeat;
struct work_struct rumble_work;
u16 rumble_left;
u16 rumble_right;
@@ -340,8 +340,6 @@ static void steam_set_lizard_mode(struct steam_device *steam, bool enable)
steam_send_report_byte(steam, STEAM_CMD_DEFAULT_MAPPINGS);
/* enable mouse */
steam_send_report_byte(steam, STEAM_CMD_DEFAULT_MOUSE);
-
- cancel_delayed_work_sync(&steam->heartbeat);
} else {
/* disable esc, enter, cursor */
steam_send_report_byte(steam, STEAM_CMD_CLEAR_MAPPINGS);
@@ -352,13 +350,8 @@ static void steam_set_lizard_mode(struct steam_device *steam, bool enable)
STEAM_REG_RPAD_MODE, 0x07, /* disable mouse */
STEAM_REG_LPAD_CLICK_PRESSURE, 0xFFFF, /* disable clicky pad */
STEAM_REG_RPAD_CLICK_PRESSURE, 0xFFFF, /* disable clicky pad */
+ STEAM_REG_WATCHDOG_ENABLE, 0, /* disable watchdog that tests if Steam is active */
0);
- /*
- * The Steam Deck has a watchdog that automatically enables
- * lizard mode if it doesn't see any traffic for too long
- */
- if (!work_busy(&steam->heartbeat.work))
- schedule_delayed_work(&steam->heartbeat, 5 * HZ);
} else {
steam_write_registers(steam,
STEAM_REG_LPAD_MODE, 0x07, /* disable mouse */
@@ -733,22 +726,6 @@ static bool steam_is_valve_interface(struct hid_device *hdev)
return !list_empty(&rep_enum->report_list);
}
-static void steam_lizard_mode_heartbeat(struct work_struct *work)
-{
- struct steam_device *steam = container_of(work, struct steam_device,
- heartbeat.work);
-
- mutex_lock(&steam->mutex);
- if (!steam->client_opened && steam->client_hdev) {
- steam_send_report_byte(steam, STEAM_CMD_CLEAR_MAPPINGS);
- steam_write_registers(steam,
- STEAM_REG_RPAD_MODE, 0x07, /* disable mouse */
- 0);
- schedule_delayed_work(&steam->heartbeat, 5 * HZ);
- }
- mutex_unlock(&steam->mutex);
-}
-
static int steam_client_ll_parse(struct hid_device *hdev)
{
struct steam_device *steam = hdev->driver_data;
@@ -887,7 +864,6 @@ static int steam_probe(struct hid_device *hdev,
steam->quirks = id->driver_data;
INIT_WORK(&steam->work_connect, steam_work_connect_cb);
INIT_LIST_HEAD(&steam->list);
- INIT_DEFERRABLE_WORK(&steam->heartbeat, steam_lizard_mode_heartbeat);
INIT_WORK(&steam->rumble_work, steam_haptic_rumble_cb);
steam->client_hdev = steam_create_client_hid(hdev);
@@ -944,7 +920,6 @@ static int steam_probe(struct hid_device *hdev,
hid_destroy_device(steam->client_hdev);
client_hdev_fail:
cancel_work_sync(&steam->work_connect);
- cancel_delayed_work_sync(&steam->heartbeat);
cancel_work_sync(&steam->rumble_work);
steam_alloc_fail:
hid_err(hdev, "%s: failed with error %d\n",
@@ -965,7 +940,6 @@ static void steam_remove(struct hid_device *hdev)
mutex_lock(&steam->mutex);
steam->client_hdev = NULL;
steam->client_opened = false;
- cancel_delayed_work_sync(&steam->heartbeat);
mutex_unlock(&steam->mutex);
cancel_work_sync(&steam->work_connect);
if (steam->quirks & STEAM_QUIRK_WIRELESS) {
--
2.42.0
^ permalink raw reply related
* [PATCH 3/7] HID: hid-steam: Clean up locking
From: Vicki Pfau @ 2023-12-20 3:38 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-input; +Cc: Vicki Pfau
In-Reply-To: <20231220033837.2135194-1-vi@endrift.com>
This cleans up the locking logic so that the spinlock is consistently used for
access to a small handful of struct variables, and the mutex is exclusively and
consistently used for ensuring that mutliple threads aren't trying to
send/receive reports at the same time. Previously, only some report
transactions were guarded by this mutex, potentially breaking atomicity. The
mutex has been renamed to reflect this usage.
Signed-off-by: Vicki Pfau <vi@endrift.com>
---
drivers/hid/hid-steam.c | 122 +++++++++++++++++++++++-----------------
1 file changed, 69 insertions(+), 53 deletions(-)
diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
index efd297e0ea8c..57cb58941c9f 100644
--- a/drivers/hid/hid-steam.c
+++ b/drivers/hid/hid-steam.c
@@ -124,7 +124,7 @@ struct steam_device {
struct list_head list;
spinlock_t lock;
struct hid_device *hdev, *client_hdev;
- struct mutex mutex;
+ struct mutex report_mutex;
bool client_opened;
struct input_dev __rcu *input;
unsigned long quirks;
@@ -267,21 +267,26 @@ static int steam_get_serial(struct steam_device *steam)
* Send: 0xae 0x15 0x01
* Recv: 0xae 0x15 0x01 serialnumber (10 chars)
*/
- int ret;
+ int ret = 0;
u8 cmd[] = {STEAM_CMD_GET_SERIAL, 0x15, 0x01};
u8 reply[3 + STEAM_SERIAL_LEN + 1];
+ mutex_lock(&steam->report_mutex);
ret = steam_send_report(steam, cmd, sizeof(cmd));
if (ret < 0)
- return ret;
+ goto out;
ret = steam_recv_report(steam, reply, sizeof(reply));
if (ret < 0)
- return ret;
- if (reply[0] != 0xae || reply[1] != 0x15 || reply[2] != 0x01)
- return -EIO;
+ goto out;
+ if (reply[0] != 0xae || reply[1] != 0x15 || reply[2] != 0x01) {
+ ret = -EIO;
+ goto out;
+ }
reply[3 + STEAM_SERIAL_LEN] = 0;
strscpy(steam->serial_no, reply + 3, sizeof(steam->serial_no));
- return 0;
+out:
+ mutex_unlock(&steam->report_mutex);
+ return ret;
}
/*
@@ -291,13 +296,18 @@ static int steam_get_serial(struct steam_device *steam)
*/
static inline int steam_request_conn_status(struct steam_device *steam)
{
- return steam_send_report_byte(steam, STEAM_CMD_REQUEST_COMM_STATUS);
+ int ret;
+ mutex_lock(&steam->report_mutex);
+ ret = steam_send_report_byte(steam, STEAM_CMD_REQUEST_COMM_STATUS);
+ mutex_unlock(&steam->report_mutex);
+ return ret;
}
static inline int steam_haptic_rumble(struct steam_device *steam,
u16 intensity, u16 left_speed, u16 right_speed,
u8 left_gain, u8 right_gain)
{
+ int ret;
u8 report[11] = {STEAM_CMD_HAPTIC_RUMBLE, 9};
report[3] = intensity & 0xFF;
@@ -309,7 +319,10 @@ static inline int steam_haptic_rumble(struct steam_device *steam,
report[9] = left_gain;
report[10] = right_gain;
- return steam_send_report(steam, report, sizeof(report));
+ mutex_lock(&steam->report_mutex);
+ ret = steam_send_report(steam, report, sizeof(report));
+ mutex_unlock(&steam->report_mutex);
+ return ret;
}
static void steam_haptic_rumble_cb(struct work_struct *work)
@@ -336,11 +349,14 @@ static int steam_play_effect(struct input_dev *dev, void *data,
static void steam_set_lizard_mode(struct steam_device *steam, bool enable)
{
if (enable) {
+ mutex_lock(&steam->report_mutex);
/* enable esc, enter, cursors */
steam_send_report_byte(steam, STEAM_CMD_DEFAULT_MAPPINGS);
/* enable mouse */
steam_send_report_byte(steam, STEAM_CMD_DEFAULT_MOUSE);
+ mutex_unlock(&steam->report_mutex);
} else {
+ mutex_lock(&steam->report_mutex);
/* disable esc, enter, cursor */
steam_send_report_byte(steam, STEAM_CMD_CLEAR_MAPPINGS);
@@ -352,11 +368,13 @@ static void steam_set_lizard_mode(struct steam_device *steam, bool enable)
STEAM_REG_RPAD_CLICK_PRESSURE, 0xFFFF, /* disable clicky pad */
STEAM_REG_WATCHDOG_ENABLE, 0, /* disable watchdog that tests if Steam is active */
0);
+ mutex_unlock(&steam->report_mutex);
} else {
steam_write_registers(steam,
STEAM_REG_LPAD_MODE, 0x07, /* disable mouse */
STEAM_REG_RPAD_MODE, 0x07, /* disable mouse */
0);
+ mutex_unlock(&steam->report_mutex);
}
}
}
@@ -364,22 +382,29 @@ static void steam_set_lizard_mode(struct steam_device *steam, bool enable)
static int steam_input_open(struct input_dev *dev)
{
struct steam_device *steam = input_get_drvdata(dev);
+ unsigned long flags;
+ bool set_lizard_mode;
- mutex_lock(&steam->mutex);
- if (!steam->client_opened && lizard_mode)
+ spin_lock_irqsave(&steam->lock, flags);
+ set_lizard_mode = !steam->client_opened && lizard_mode;
+ spin_unlock_irqrestore(&steam->lock, flags);
+ if (set_lizard_mode)
steam_set_lizard_mode(steam, false);
- mutex_unlock(&steam->mutex);
+
return 0;
}
static void steam_input_close(struct input_dev *dev)
{
struct steam_device *steam = input_get_drvdata(dev);
+ unsigned long flags;
+ bool set_lizard_mode;
- mutex_lock(&steam->mutex);
- if (!steam->client_opened && lizard_mode)
+ spin_lock_irqsave(&steam->lock, flags);
+ set_lizard_mode = !steam->client_opened && lizard_mode;
+ spin_unlock_irqrestore(&steam->lock, flags);
+ if (set_lizard_mode)
steam_set_lizard_mode(steam, true);
- mutex_unlock(&steam->mutex);
}
static enum power_supply_property steam_battery_props[] = {
@@ -624,6 +649,7 @@ static int steam_register(struct steam_device *steam)
{
int ret;
bool client_opened;
+ unsigned long flags;
/*
* This function can be called several times in a row with the
@@ -636,11 +662,9 @@ static int steam_register(struct steam_device *steam)
* Unlikely, but getting the serial could fail, and it is not so
* important, so make up a serial number and go on.
*/
- mutex_lock(&steam->mutex);
if (steam_get_serial(steam) < 0)
strscpy(steam->serial_no, "XXXXXXXXXX",
sizeof(steam->serial_no));
- mutex_unlock(&steam->mutex);
hid_info(steam->hdev, "Steam Controller '%s' connected",
steam->serial_no);
@@ -655,15 +679,13 @@ static int steam_register(struct steam_device *steam)
mutex_unlock(&steam_devices_lock);
}
- mutex_lock(&steam->mutex);
+ spin_lock_irqsave(&steam->lock, flags);
client_opened = steam->client_opened;
- if (!client_opened)
+ spin_unlock_irqrestore(&steam->lock, flags);
+ if (!client_opened) {
steam_set_lizard_mode(steam, lizard_mode);
- mutex_unlock(&steam->mutex);
-
- if (!client_opened)
ret = steam_input_register(steam);
- else
+ } else
ret = 0;
return ret;
@@ -746,10 +768,11 @@ static void steam_client_ll_stop(struct hid_device *hdev)
static int steam_client_ll_open(struct hid_device *hdev)
{
struct steam_device *steam = hdev->driver_data;
+ unsigned long flags;
- mutex_lock(&steam->mutex);
+ spin_lock_irqsave(&steam->lock, flags);
steam->client_opened = true;
- mutex_unlock(&steam->mutex);
+ spin_unlock_irqrestore(&steam->lock, flags);
steam_input_unregister(steam);
@@ -764,17 +787,14 @@ static void steam_client_ll_close(struct hid_device *hdev)
bool connected;
spin_lock_irqsave(&steam->lock, flags);
- connected = steam->connected;
+ steam->client_opened = false;
+ connected = steam->connected && !steam->client_opened;
spin_unlock_irqrestore(&steam->lock, flags);
- mutex_lock(&steam->mutex);
- steam->client_opened = false;
- if (connected)
+ if (connected) {
steam_set_lizard_mode(steam, lizard_mode);
- mutex_unlock(&steam->mutex);
-
- if (connected)
steam_input_register(steam);
+ }
}
static int steam_client_ll_raw_request(struct hid_device *hdev,
@@ -860,19 +880,12 @@ static int steam_probe(struct hid_device *hdev,
steam->hdev = hdev;
hid_set_drvdata(hdev, steam);
spin_lock_init(&steam->lock);
- mutex_init(&steam->mutex);
+ mutex_init(&steam->report_mutex);
steam->quirks = id->driver_data;
INIT_WORK(&steam->work_connect, steam_work_connect_cb);
INIT_LIST_HEAD(&steam->list);
INIT_WORK(&steam->rumble_work, steam_haptic_rumble_cb);
- steam->client_hdev = steam_create_client_hid(hdev);
- if (IS_ERR(steam->client_hdev)) {
- ret = PTR_ERR(steam->client_hdev);
- goto client_hdev_fail;
- }
- steam->client_hdev->driver_data = steam;
-
/*
* With the real steam controller interface, do not connect hidraw.
* Instead, create the client_hid and connect that.
@@ -881,10 +894,6 @@ static int steam_probe(struct hid_device *hdev,
if (ret)
goto hid_hw_start_fail;
- ret = hid_add_device(steam->client_hdev);
- if (ret)
- goto client_hdev_add_fail;
-
ret = hid_hw_open(hdev);
if (ret) {
hid_err(hdev,
@@ -910,15 +919,26 @@ static int steam_probe(struct hid_device *hdev,
}
}
+ steam->client_hdev = steam_create_client_hid(hdev);
+ if (IS_ERR(steam->client_hdev)) {
+ ret = PTR_ERR(steam->client_hdev);
+ goto client_hdev_fail;
+ }
+ steam->client_hdev->driver_data = steam;
+
+ ret = hid_add_device(steam->client_hdev);
+ if (ret)
+ goto client_hdev_add_fail;
+
return 0;
-input_register_fail:
-hid_hw_open_fail:
client_hdev_add_fail:
hid_hw_stop(hdev);
-hid_hw_start_fail:
- hid_destroy_device(steam->client_hdev);
client_hdev_fail:
+ hid_destroy_device(steam->client_hdev);
+input_register_fail:
+hid_hw_open_fail:
+hid_hw_start_fail:
cancel_work_sync(&steam->work_connect);
cancel_work_sync(&steam->rumble_work);
steam_alloc_fail:
@@ -936,12 +956,10 @@ static void steam_remove(struct hid_device *hdev)
return;
}
+ cancel_work_sync(&steam->work_connect);
hid_destroy_device(steam->client_hdev);
- mutex_lock(&steam->mutex);
steam->client_hdev = NULL;
steam->client_opened = false;
- mutex_unlock(&steam->mutex);
- cancel_work_sync(&steam->work_connect);
if (steam->quirks & STEAM_QUIRK_WIRELESS) {
hid_info(hdev, "Steam wireless receiver disconnected");
}
@@ -1408,10 +1426,8 @@ static int steam_param_set_lizard_mode(const char *val,
mutex_lock(&steam_devices_lock);
list_for_each_entry(steam, &steam_devices, list) {
- mutex_lock(&steam->mutex);
if (!steam->client_opened)
steam_set_lizard_mode(steam, lizard_mode);
- mutex_unlock(&steam->mutex);
}
mutex_unlock(&steam_devices_lock);
return 0;
--
2.42.0
^ permalink raw reply related
* [PATCH 4/7] HID: hid-steam: Make client_opened a counter
From: Vicki Pfau @ 2023-12-20 3:38 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-input; +Cc: Vicki Pfau
In-Reply-To: <20231220033837.2135194-1-vi@endrift.com>
The client_opened variable was used to track if the hidraw was opened by any
clients to silence keyboard/mouse events while opened. However, there was no
counting of how many clients were opened, so opening two at the same time and
then closing one would fool the driver into thinking it had no remaining opened
clients.
Signed-off-by: Vicki Pfau <vi@endrift.com>
---
drivers/hid/hid-steam.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
index 57cb58941c9f..667b5b09f931 100644
--- a/drivers/hid/hid-steam.c
+++ b/drivers/hid/hid-steam.c
@@ -125,7 +125,7 @@ struct steam_device {
spinlock_t lock;
struct hid_device *hdev, *client_hdev;
struct mutex report_mutex;
- bool client_opened;
+ unsigned long client_opened;
struct input_dev __rcu *input;
unsigned long quirks;
struct work_struct work_connect;
@@ -648,7 +648,7 @@ static void steam_battery_unregister(struct steam_device *steam)
static int steam_register(struct steam_device *steam)
{
int ret;
- bool client_opened;
+ unsigned long client_opened;
unsigned long flags;
/*
@@ -771,7 +771,7 @@ static int steam_client_ll_open(struct hid_device *hdev)
unsigned long flags;
spin_lock_irqsave(&steam->lock, flags);
- steam->client_opened = true;
+ steam->client_opened++;
spin_unlock_irqrestore(&steam->lock, flags);
steam_input_unregister(steam);
@@ -787,7 +787,7 @@ static void steam_client_ll_close(struct hid_device *hdev)
bool connected;
spin_lock_irqsave(&steam->lock, flags);
- steam->client_opened = false;
+ steam->client_opened--;
connected = steam->connected && !steam->client_opened;
spin_unlock_irqrestore(&steam->lock, flags);
@@ -959,7 +959,7 @@ static void steam_remove(struct hid_device *hdev)
cancel_work_sync(&steam->work_connect);
hid_destroy_device(steam->client_hdev);
steam->client_hdev = NULL;
- steam->client_opened = false;
+ steam->client_opened = 0;
if (steam->quirks & STEAM_QUIRK_WIRELESS) {
hid_info(hdev, "Steam wireless receiver disconnected");
}
--
2.42.0
^ permalink raw reply related
* [PATCH 5/7] HID: hid-steam: Update list of identifiers from SDL
From: Vicki Pfau @ 2023-12-20 3:38 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-input; +Cc: Vicki Pfau
In-Reply-To: <20231220033837.2135194-1-vi@endrift.com>
SDL includes a list of settings (formerly called registers in this driver),
reports (formerly cmds), and various other identifiers that were provided by
Valve. This commit imports a significant chunk of that list as well as
replacing most of the guessed names and a handful of magic constants. It also
replaces bitmask definitions that used hex with the BIT macro.
Signed-off-by: Vicki Pfau <vi@endrift.com>
---
drivers/hid/hid-steam.c | 286 +++++++++++++++++++++++++++++++---------
1 file changed, 221 insertions(+), 65 deletions(-)
diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
index 667b5b09f931..4f5c647f04dd 100644
--- a/drivers/hid/hid-steam.c
+++ b/drivers/hid/hid-steam.c
@@ -71,51 +71,207 @@ static LIST_HEAD(steam_devices);
/*
* Commands that can be sent in a feature report.
- * Thanks to Valve for some valuable hints.
+ * Thanks to Valve and SDL for the names.
*/
-#define STEAM_CMD_SET_MAPPINGS 0x80
-#define STEAM_CMD_CLEAR_MAPPINGS 0x81
-#define STEAM_CMD_GET_MAPPINGS 0x82
-#define STEAM_CMD_GET_ATTRIB 0x83
-#define STEAM_CMD_GET_ATTRIB_LABEL 0x84
-#define STEAM_CMD_DEFAULT_MAPPINGS 0x85
-#define STEAM_CMD_FACTORY_RESET 0x86
-#define STEAM_CMD_WRITE_REGISTER 0x87
-#define STEAM_CMD_CLEAR_REGISTER 0x88
-#define STEAM_CMD_READ_REGISTER 0x89
-#define STEAM_CMD_GET_REGISTER_LABEL 0x8a
-#define STEAM_CMD_GET_REGISTER_MAX 0x8b
-#define STEAM_CMD_GET_REGISTER_DEFAULT 0x8c
-#define STEAM_CMD_SET_MODE 0x8d
-#define STEAM_CMD_DEFAULT_MOUSE 0x8e
-#define STEAM_CMD_FORCEFEEDBAK 0x8f
-#define STEAM_CMD_REQUEST_COMM_STATUS 0xb4
-#define STEAM_CMD_GET_SERIAL 0xae
-#define STEAM_CMD_HAPTIC_RUMBLE 0xeb
-
-/* Some useful register ids */
-#define STEAM_REG_LPAD_MODE 0x07
-#define STEAM_REG_RPAD_MODE 0x08
-#define STEAM_REG_RPAD_MARGIN 0x18
-#define STEAM_REG_LED 0x2d
-#define STEAM_REG_GYRO_MODE 0x30
-#define STEAM_REG_LPAD_CLICK_PRESSURE 0x34
-#define STEAM_REG_RPAD_CLICK_PRESSURE 0x35
-#define STEAM_REG_WATCHDOG_ENABLE 0x47
-
-/* Raw event identifiers */
-#define STEAM_EV_INPUT_DATA 0x01
-#define STEAM_EV_CONNECT 0x03
-#define STEAM_EV_BATTERY 0x04
-#define STEAM_EV_DECK_INPUT_DATA 0x09
+enum {
+ ID_SET_DIGITAL_MAPPINGS = 0x80,
+ ID_CLEAR_DIGITAL_MAPPINGS = 0x81,
+ ID_GET_DIGITAL_MAPPINGS = 0x82,
+ ID_GET_ATTRIBUTES_VALUES = 0x83,
+ ID_GET_ATTRIBUTE_LABEL = 0x84,
+ ID_SET_DEFAULT_DIGITAL_MAPPINGS = 0x85,
+ ID_FACTORY_RESET = 0x86,
+ ID_SET_SETTINGS_VALUES = 0x87,
+ ID_CLEAR_SETTINGS_VALUES = 0x88,
+ ID_GET_SETTINGS_VALUES = 0x89,
+ ID_GET_SETTING_LABEL = 0x8A,
+ ID_GET_SETTINGS_MAXS = 0x8B,
+ ID_GET_SETTINGS_DEFAULTS = 0x8C,
+ ID_SET_CONTROLLER_MODE = 0x8D,
+ ID_LOAD_DEFAULT_SETTINGS = 0x8E,
+ ID_TRIGGER_HAPTIC_PULSE = 0x8F,
+ ID_TURN_OFF_CONTROLLER = 0x9F,
+
+ ID_GET_DEVICE_INFO = 0xA1,
+
+ ID_CALIBRATE_TRACKPADS = 0xA7,
+ ID_RESERVED_0 = 0xA8,
+ ID_SET_SERIAL_NUMBER = 0xA9,
+ ID_GET_TRACKPAD_CALIBRATION = 0xAA,
+ ID_GET_TRACKPAD_FACTORY_CALIBRATION = 0xAB,
+ ID_GET_TRACKPAD_RAW_DATA = 0xAC,
+ ID_ENABLE_PAIRING = 0xAD,
+ ID_GET_STRING_ATTRIBUTE = 0xAE,
+ ID_RADIO_ERASE_RECORDS = 0xAF,
+ ID_RADIO_WRITE_RECORD = 0xB0,
+ ID_SET_DONGLE_SETTING = 0xB1,
+ ID_DONGLE_DISCONNECT_DEVICE = 0xB2,
+ ID_DONGLE_COMMIT_DEVICE = 0xB3,
+ ID_DONGLE_GET_WIRELESS_STATE = 0xB4,
+ ID_CALIBRATE_GYRO = 0xB5,
+ ID_PLAY_AUDIO = 0xB6,
+ ID_AUDIO_UPDATE_START = 0xB7,
+ ID_AUDIO_UPDATE_DATA = 0xB8,
+ ID_AUDIO_UPDATE_COMPLETE = 0xB9,
+ ID_GET_CHIPID = 0xBA,
+
+ ID_CALIBRATE_JOYSTICK = 0xBF,
+ ID_CALIBRATE_ANALOG_TRIGGERS = 0xC0,
+ ID_SET_AUDIO_MAPPING = 0xC1,
+ ID_CHECK_GYRO_FW_LOAD = 0xC2,
+ ID_CALIBRATE_ANALOG = 0xC3,
+ ID_DONGLE_GET_CONNECTED_SLOTS = 0xC4,
+
+ ID_RESET_IMU = 0xCE,
+
+ ID_TRIGGER_HAPTIC_CMD = 0xEA,
+ ID_TRIGGER_RUMBLE_CMD = 0xEB,
+};
+
+/* Settings IDs */
+enum {
+ /* 0 */
+ SETTING_MOUSE_SENSITIVITY,
+ SETTING_MOUSE_ACCELERATION,
+ SETTING_TRACKBALL_ROTATION_ANGLE,
+ SETTING_HAPTIC_INTENSITY_UNUSED,
+ SETTING_LEFT_GAMEPAD_STICK_ENABLED,
+ SETTING_RIGHT_GAMEPAD_STICK_ENABLED,
+ SETTING_USB_DEBUG_MODE,
+ SETTING_LEFT_TRACKPAD_MODE,
+ SETTING_RIGHT_TRACKPAD_MODE,
+ SETTING_MOUSE_POINTER_ENABLED,
+
+ /* 10 */
+ SETTING_DPAD_DEADZONE,
+ SETTING_MINIMUM_MOMENTUM_VEL,
+ SETTING_MOMENTUM_DECAY_AMMOUNT,
+ SETTING_TRACKPAD_RELATIVE_MODE_TICKS_PER_PIXEL,
+ SETTING_HAPTIC_INCREMENT,
+ SETTING_DPAD_ANGLE_SIN,
+ SETTING_DPAD_ANGLE_COS,
+ SETTING_MOMENTUM_VERTICAL_DIVISOR,
+ SETTING_MOMENTUM_MAXIMUM_VELOCITY,
+ SETTING_TRACKPAD_Z_ON,
+
+ /* 20 */
+ SETTING_TRACKPAD_Z_OFF,
+ SETTING_SENSITIVY_SCALE_AMMOUNT,
+ SETTING_LEFT_TRACKPAD_SECONDARY_MODE,
+ SETTING_RIGHT_TRACKPAD_SECONDARY_MODE,
+ SETTING_SMOOTH_ABSOLUTE_MOUSE,
+ SETTING_STEAMBUTTON_POWEROFF_TIME,
+ SETTING_UNUSED_1,
+ SETTING_TRACKPAD_OUTER_RADIUS,
+ SETTING_TRACKPAD_Z_ON_LEFT,
+ SETTING_TRACKPAD_Z_OFF_LEFT,
+
+ /* 30 */
+ SETTING_TRACKPAD_OUTER_SPIN_VEL,
+ SETTING_TRACKPAD_OUTER_SPIN_RADIUS,
+ SETTING_TRACKPAD_OUTER_SPIN_HORIZONTAL_ONLY,
+ SETTING_TRACKPAD_RELATIVE_MODE_DEADZONE,
+ SETTING_TRACKPAD_RELATIVE_MODE_MAX_VEL,
+ SETTING_TRACKPAD_RELATIVE_MODE_INVERT_Y,
+ SETTING_TRACKPAD_DOUBLE_TAP_BEEP_ENABLED,
+ SETTING_TRACKPAD_DOUBLE_TAP_BEEP_PERIOD,
+ SETTING_TRACKPAD_DOUBLE_TAP_BEEP_COUNT,
+ SETTING_TRACKPAD_OUTER_RADIUS_RELEASE_ON_TRANSITION,
+
+ /* 40 */
+ SETTING_RADIAL_MODE_ANGLE,
+ SETTING_HAPTIC_INTENSITY_MOUSE_MODE,
+ SETTING_LEFT_DPAD_REQUIRES_CLICK,
+ SETTING_RIGHT_DPAD_REQUIRES_CLICK,
+ SETTING_LED_BASELINE_BRIGHTNESS,
+ SETTING_LED_USER_BRIGHTNESS,
+ SETTING_ENABLE_RAW_JOYSTICK,
+ SETTING_ENABLE_FAST_SCAN,
+ SETTING_IMU_MODE,
+ SETTING_WIRELESS_PACKET_VERSION,
+
+ /* 50 */
+ SETTING_SLEEP_INACTIVITY_TIMEOUT,
+ SETTING_TRACKPAD_NOISE_THRESHOLD,
+ SETTING_LEFT_TRACKPAD_CLICK_PRESSURE,
+ SETTING_RIGHT_TRACKPAD_CLICK_PRESSURE,
+ SETTING_LEFT_BUMPER_CLICK_PRESSURE,
+ SETTING_RIGHT_BUMPER_CLICK_PRESSURE,
+ SETTING_LEFT_GRIP_CLICK_PRESSURE,
+ SETTING_RIGHT_GRIP_CLICK_PRESSURE,
+ SETTING_LEFT_GRIP2_CLICK_PRESSURE,
+ SETTING_RIGHT_GRIP2_CLICK_PRESSURE,
+
+ /* 60 */
+ SETTING_PRESSURE_MODE,
+ SETTING_CONTROLLER_TEST_MODE,
+ SETTING_TRIGGER_MODE,
+ SETTING_TRACKPAD_Z_THRESHOLD,
+ SETTING_FRAME_RATE,
+ SETTING_TRACKPAD_FILT_CTRL,
+ SETTING_TRACKPAD_CLIP,
+ SETTING_DEBUG_OUTPUT_SELECT,
+ SETTING_TRIGGER_THRESHOLD_PERCENT,
+ SETTING_TRACKPAD_FREQUENCY_HOPPING,
+
+ /* 70 */
+ SETTING_HAPTICS_ENABLED,
+ SETTING_STEAM_WATCHDOG_ENABLE,
+ SETTING_TIMP_TOUCH_THRESHOLD_ON,
+ SETTING_TIMP_TOUCH_THRESHOLD_OFF,
+ SETTING_FREQ_HOPPING,
+ SETTING_TEST_CONTROL,
+ SETTING_HAPTIC_MASTER_GAIN_DB,
+ SETTING_THUMB_TOUCH_THRESH,
+ SETTING_DEVICE_POWER_STATUS,
+ SETTING_HAPTIC_INTENSITY,
+
+ /* 80 */
+ SETTING_STABILIZER_ENABLED,
+ SETTING_TIMP_MODE_MTE,
+};
+
+/* Input report identifiers */
+enum
+{
+ ID_CONTROLLER_STATE = 1,
+ ID_CONTROLLER_DEBUG = 2,
+ ID_CONTROLLER_WIRELESS = 3,
+ ID_CONTROLLER_STATUS = 4,
+ ID_CONTROLLER_DEBUG2 = 5,
+ ID_CONTROLLER_SECONDARY_STATE = 6,
+ ID_CONTROLLER_BLE_STATE = 7,
+ ID_CONTROLLER_DECK_STATE = 9
+};
+
+/* String attribute idenitifiers */
+enum {
+ ATTRIB_STR_BOARD_SERIAL,
+ ATTRIB_STR_UNIT_SERIAL,
+};
/* Values for GYRO_MODE (bitmask) */
-#define STEAM_GYRO_MODE_OFF 0x0000
-#define STEAM_GYRO_MODE_STEERING 0x0001
-#define STEAM_GYRO_MODE_TILT 0x0002
-#define STEAM_GYRO_MODE_SEND_ORIENTATION 0x0004
-#define STEAM_GYRO_MODE_SEND_RAW_ACCEL 0x0008
-#define STEAM_GYRO_MODE_SEND_RAW_GYRO 0x0010
+enum {
+ SETTING_GYRO_MODE_OFF = 0,
+ SETTING_GYRO_MODE_STEERING = BIT(0),
+ SETTING_GYRO_MODE_TILT = BIT(1),
+ SETTING_GYRO_MODE_SEND_ORIENTATION = BIT(2),
+ SETTING_GYRO_MODE_SEND_RAW_ACCEL = BIT(3),
+ SETTING_GYRO_MODE_SEND_RAW_GYRO = BIT(4),
+};
+
+/* Trackpad modes */
+enum {
+ TRACKPAD_ABSOLUTE_MOUSE,
+ TRACKPAD_RELATIVE_MOUSE,
+ TRACKPAD_DPAD_FOUR_WAY_DISCRETE,
+ TRACKPAD_DPAD_FOUR_WAY_OVERLAP,
+ TRACKPAD_DPAD_EIGHT_WAY,
+ TRACKPAD_RADIAL_MODE,
+ TRACKPAD_ABSOLUTE_DPAD,
+ TRACKPAD_NONE,
+ TRACKPAD_GESTURE_KEYBOARD,
+};
/* Other random constants */
#define STEAM_SERIAL_LEN 10
@@ -226,13 +382,13 @@ static inline int steam_send_report_byte(struct steam_device *steam, u8 cmd)
return steam_send_report(steam, &cmd, 1);
}
-static int steam_write_registers(struct steam_device *steam,
+static int steam_write_settings(struct steam_device *steam,
/* u8 reg, u16 val */...)
{
/* Send: 0x87 len (reg valLo valHi)* */
u8 reg;
u16 val;
- u8 cmd[64] = {STEAM_CMD_WRITE_REGISTER, 0x00};
+ u8 cmd[64] = {ID_SET_SETTINGS_VALUES, 0x00};
int ret;
va_list args;
@@ -268,7 +424,7 @@ static int steam_get_serial(struct steam_device *steam)
* Recv: 0xae 0x15 0x01 serialnumber (10 chars)
*/
int ret = 0;
- u8 cmd[] = {STEAM_CMD_GET_SERIAL, 0x15, 0x01};
+ u8 cmd[] = {ID_GET_STRING_ATTRIBUTE, 0x15, ATTRIB_STR_UNIT_SERIAL};
u8 reply[3 + STEAM_SERIAL_LEN + 1];
mutex_lock(&steam->report_mutex);
@@ -278,7 +434,7 @@ static int steam_get_serial(struct steam_device *steam)
ret = steam_recv_report(steam, reply, sizeof(reply));
if (ret < 0)
goto out;
- if (reply[0] != 0xae || reply[1] != 0x15 || reply[2] != 0x01) {
+ if (reply[0] != ID_GET_STRING_ATTRIBUTE || reply[1] != 0x15 || reply[2] != ATTRIB_STR_UNIT_SERIAL) {
ret = -EIO;
goto out;
}
@@ -298,7 +454,7 @@ static inline int steam_request_conn_status(struct steam_device *steam)
{
int ret;
mutex_lock(&steam->report_mutex);
- ret = steam_send_report_byte(steam, STEAM_CMD_REQUEST_COMM_STATUS);
+ ret = steam_send_report_byte(steam, ID_DONGLE_GET_WIRELESS_STATE);
mutex_unlock(&steam->report_mutex);
return ret;
}
@@ -308,7 +464,7 @@ static inline int steam_haptic_rumble(struct steam_device *steam,
u8 left_gain, u8 right_gain)
{
int ret;
- u8 report[11] = {STEAM_CMD_HAPTIC_RUMBLE, 9};
+ u8 report[11] = {ID_TRIGGER_RUMBLE_CMD, 9};
report[3] = intensity & 0xFF;
report[4] = intensity >> 8;
@@ -351,28 +507,28 @@ static void steam_set_lizard_mode(struct steam_device *steam, bool enable)
if (enable) {
mutex_lock(&steam->report_mutex);
/* enable esc, enter, cursors */
- steam_send_report_byte(steam, STEAM_CMD_DEFAULT_MAPPINGS);
- /* enable mouse */
- steam_send_report_byte(steam, STEAM_CMD_DEFAULT_MOUSE);
+ steam_send_report_byte(steam, ID_SET_DEFAULT_DIGITAL_MAPPINGS);
+ /* reset settings */
+ steam_send_report_byte(steam, ID_LOAD_DEFAULT_SETTINGS);
mutex_unlock(&steam->report_mutex);
} else {
mutex_lock(&steam->report_mutex);
/* disable esc, enter, cursor */
- steam_send_report_byte(steam, STEAM_CMD_CLEAR_MAPPINGS);
+ steam_send_report_byte(steam, ID_CLEAR_DIGITAL_MAPPINGS);
if (steam->quirks & STEAM_QUIRK_DECK) {
- steam_write_registers(steam,
- STEAM_REG_LPAD_MODE, 0x07, /* disable mouse */
- STEAM_REG_RPAD_MODE, 0x07, /* disable mouse */
- STEAM_REG_LPAD_CLICK_PRESSURE, 0xFFFF, /* disable clicky pad */
- STEAM_REG_RPAD_CLICK_PRESSURE, 0xFFFF, /* disable clicky pad */
- STEAM_REG_WATCHDOG_ENABLE, 0, /* disable watchdog that tests if Steam is active */
+ steam_write_settings(steam,
+ SETTING_LEFT_TRACKPAD_MODE, TRACKPAD_NONE, /* disable mouse */
+ SETTING_RIGHT_TRACKPAD_MODE, TRACKPAD_NONE, /* disable mouse */
+ SETTING_LEFT_TRACKPAD_CLICK_PRESSURE, 0xFFFF, /* disable haptic click */
+ SETTING_RIGHT_TRACKPAD_CLICK_PRESSURE, 0xFFFF, /* disable haptic click */
+ SETTING_STEAM_WATCHDOG_ENABLE, 0, /* disable watchdog that tests if Steam is active */
0);
mutex_unlock(&steam->report_mutex);
} else {
- steam_write_registers(steam,
- STEAM_REG_LPAD_MODE, 0x07, /* disable mouse */
- STEAM_REG_RPAD_MODE, 0x07, /* disable mouse */
+ steam_write_settings(steam,
+ SETTING_LEFT_TRACKPAD_MODE, TRACKPAD_NONE, /* disable mouse */
+ SETTING_RIGHT_TRACKPAD_MODE, TRACKPAD_NONE, /* disable mouse */
0);
mutex_unlock(&steam->report_mutex);
}
@@ -1362,7 +1518,7 @@ static int steam_raw_event(struct hid_device *hdev,
return 0;
switch (data[2]) {
- case STEAM_EV_INPUT_DATA:
+ case ID_CONTROLLER_STATE:
if (steam->client_opened)
return 0;
rcu_read_lock();
@@ -1371,7 +1527,7 @@ static int steam_raw_event(struct hid_device *hdev,
steam_do_input_event(steam, input, data);
rcu_read_unlock();
break;
- case STEAM_EV_DECK_INPUT_DATA:
+ case ID_CONTROLLER_DECK_STATE:
if (steam->client_opened)
return 0;
rcu_read_lock();
@@ -1380,7 +1536,7 @@ static int steam_raw_event(struct hid_device *hdev,
steam_do_deck_input_event(steam, input, data);
rcu_read_unlock();
break;
- case STEAM_EV_CONNECT:
+ case ID_CONTROLLER_WIRELESS:
/*
* The payload of this event is a single byte:
* 0x01: disconnected.
@@ -1395,7 +1551,7 @@ static int steam_raw_event(struct hid_device *hdev,
break;
}
break;
- case STEAM_EV_BATTERY:
+ case ID_CONTROLLER_STATUS:
if (steam->quirks & STEAM_QUIRK_WIRELESS) {
rcu_read_lock();
battery = rcu_dereference(steam->battery);
--
2.42.0
^ permalink raw reply related
* [PATCH 7/7] HID: hid-steam: Add gamepad-only mode switched to by holding options
From: Vicki Pfau @ 2023-12-20 3:38 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-input; +Cc: Vicki Pfau
In-Reply-To: <20231220033837.2135194-1-vi@endrift.com>
This commit adds a hotkey to switch between "gamepad" mode (mouse and keyboard
disabled) and "desktop" mode (gamepad disabled) by holding down the options
button (mapped here as the start button). This mirrors the behavior of the
official Steam client.
This also adds and uses a function for generating haptic pulses, as Steam also
does when engaging this hotkey.
Signed-off-by: Vicki Pfau <vi@endrift.com>
---
drivers/hid/hid-steam.c | 113 ++++++++++++++++++++++++++++++++++++----
1 file changed, 103 insertions(+), 10 deletions(-)
diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
index a0ed8812e7ea..b3c4e50e248a 100644
--- a/drivers/hid/hid-steam.c
+++ b/drivers/hid/hid-steam.c
@@ -273,6 +273,11 @@ enum {
TRACKPAD_GESTURE_KEYBOARD,
};
+/* Pad identifiers for the deck */
+#define STEAM_PAD_LEFT 0
+#define STEAM_PAD_RIGHT 1
+#define STEAM_PAD_BOTH 2
+
/* Other random constants */
#define STEAM_SERIAL_LEN 0x15
@@ -291,6 +296,9 @@ struct steam_device {
struct power_supply __rcu *battery;
u8 battery_charge;
u16 voltage;
+ struct delayed_work mode_switch;
+ bool did_mode_switch;
+ bool gamepad_mode;
struct work_struct rumble_work;
u16 rumble_left;
u16 rumble_right;
@@ -460,6 +468,37 @@ static inline int steam_request_conn_status(struct steam_device *steam)
return ret;
}
+/*
+ * Send a haptic pulse to the trackpads
+ * Duration and interval are measured in microseconds, count is the number
+ * of pulses to send for duration time with interval microseconds between them
+ * and gain is measured in decibels, ranging from -24 to +6
+ */
+static inline int steam_haptic_pulse(struct steam_device *steam, u8 pad,
+ u16 duration, u16 interval, u16 count, u8 gain)
+{
+ int ret;
+ u8 report[10] = {ID_TRIGGER_HAPTIC_PULSE, 8};
+
+ /* Left and right are swapped on this report for legacy reasons */
+ if (pad < STEAM_PAD_BOTH)
+ pad ^= 1;
+
+ report[2] = pad;
+ report[3] = duration & 0xFF;
+ report[4] = duration >> 8;
+ report[5] = interval & 0xFF;
+ report[6] = interval >> 8;
+ report[7] = count & 0xFF;
+ report[8] = count >> 8;
+ report[9] = gain;
+
+ mutex_lock(&steam->report_mutex);
+ ret = steam_send_report(steam, report, sizeof(report));
+ mutex_unlock(&steam->report_mutex);
+ return ret;
+}
+
static inline int steam_haptic_rumble(struct steam_device *steam,
u16 intensity, u16 left_speed, u16 right_speed,
u8 left_gain, u8 right_gain)
@@ -505,6 +544,9 @@ static int steam_play_effect(struct input_dev *dev, void *data,
static void steam_set_lizard_mode(struct steam_device *steam, bool enable)
{
+ if (steam->gamepad_mode)
+ enable = false;
+
if (enable) {
mutex_lock(&steam->report_mutex);
/* enable esc, enter, cursors */
@@ -542,11 +584,18 @@ static int steam_input_open(struct input_dev *dev)
unsigned long flags;
bool set_lizard_mode;
- spin_lock_irqsave(&steam->lock, flags);
- set_lizard_mode = !steam->client_opened && lizard_mode;
- spin_unlock_irqrestore(&steam->lock, flags);
- if (set_lizard_mode)
- steam_set_lizard_mode(steam, false);
+ /*
+ * Disabling lizard mode automatically is only done on the Steam
+ * Controller. On the Steam Deck, this is toggled manually by holding
+ * the options button instead, handled by steam_mode_switch_cb.
+ */
+ if (!(steam->quirks & STEAM_QUIRK_DECK)) {
+ spin_lock_irqsave(&steam->lock, flags);
+ set_lizard_mode = !steam->client_opened && lizard_mode;
+ spin_unlock_irqrestore(&steam->lock, flags);
+ if (set_lizard_mode)
+ steam_set_lizard_mode(steam, false);
+ }
return 0;
}
@@ -557,11 +606,13 @@ static void steam_input_close(struct input_dev *dev)
unsigned long flags;
bool set_lizard_mode;
- spin_lock_irqsave(&steam->lock, flags);
- set_lizard_mode = !steam->client_opened && lizard_mode;
- spin_unlock_irqrestore(&steam->lock, flags);
- if (set_lizard_mode)
- steam_set_lizard_mode(steam, true);
+ if (!(steam->quirks & STEAM_QUIRK_DECK)) {
+ spin_lock_irqsave(&steam->lock, flags);
+ set_lizard_mode = !steam->client_opened && lizard_mode;
+ spin_unlock_irqrestore(&steam->lock, flags);
+ if (set_lizard_mode)
+ steam_set_lizard_mode(steam, true);
+ }
}
static enum power_supply_property steam_battery_props[] = {
@@ -886,6 +937,34 @@ static void steam_work_connect_cb(struct work_struct *work)
}
}
+static void steam_mode_switch_cb(struct work_struct *work)
+{
+ struct steam_device *steam = container_of(to_delayed_work(work),
+ struct steam_device, mode_switch);
+ unsigned long flags;
+ bool client_opened;
+ steam->gamepad_mode = !steam->gamepad_mode;
+ if (!lizard_mode)
+ return;
+
+ if (steam->gamepad_mode)
+ steam_set_lizard_mode(steam, false);
+ else {
+ spin_lock_irqsave(&steam->lock, flags);
+ client_opened = steam->client_opened;
+ spin_unlock_irqrestore(&steam->lock, flags);
+ if (!client_opened)
+ steam_set_lizard_mode(steam, lizard_mode);
+ }
+
+ steam_haptic_pulse(steam, STEAM_PAD_RIGHT, 0x190, 0, 1, 0);
+ if (steam->gamepad_mode) {
+ steam_haptic_pulse(steam, STEAM_PAD_LEFT, 0x14D, 0x14D, 0x2D, 0);
+ } else {
+ steam_haptic_pulse(steam, STEAM_PAD_LEFT, 0x1F4, 0x1F4, 0x1E, 0);
+ }
+}
+
static bool steam_is_valve_interface(struct hid_device *hdev)
{
struct hid_report_enum *rep_enum;
@@ -1040,6 +1119,7 @@ static int steam_probe(struct hid_device *hdev,
mutex_init(&steam->report_mutex);
steam->quirks = id->driver_data;
INIT_WORK(&steam->work_connect, steam_work_connect_cb);
+ INIT_DELAYED_WORK(&steam->mode_switch, steam_mode_switch_cb);
INIT_LIST_HEAD(&steam->list);
INIT_WORK(&steam->rumble_work, steam_haptic_rumble_cb);
@@ -1097,6 +1177,7 @@ static int steam_probe(struct hid_device *hdev,
hid_hw_open_fail:
hid_hw_start_fail:
cancel_work_sync(&steam->work_connect);
+ cancel_delayed_work_sync(&steam->mode_switch);
cancel_work_sync(&steam->rumble_work);
steam_alloc_fail:
hid_err(hdev, "%s: failed with error %d\n",
@@ -1113,6 +1194,7 @@ static void steam_remove(struct hid_device *hdev)
return;
}
+ cancel_delayed_work_sync(&steam->mode_switch);
cancel_work_sync(&steam->work_connect);
hid_destroy_device(steam->client_hdev);
steam->client_hdev = NULL;
@@ -1398,6 +1480,17 @@ static void steam_do_deck_input_event(struct steam_device *steam,
b13 = data[13];
b14 = data[14];
+ if (!(b9 & BIT(6)) && steam->did_mode_switch) {
+ steam->did_mode_switch = false;
+ cancel_delayed_work_sync(&steam->mode_switch);
+ } else if (!steam->client_opened && (b9 & BIT(6)) && !steam->did_mode_switch) {
+ steam->did_mode_switch = true;
+ schedule_delayed_work(&steam->mode_switch, 45 * HZ / 100);
+ }
+
+ if (!steam->gamepad_mode)
+ return;
+
lpad_touched = b10 & BIT(3);
rpad_touched = b10 & BIT(4);
--
2.42.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox