[PATCH v2 0/2] soc: qcom: QMI helpers supports for big endian

[PATCH v2 0/2] soc: qcom: QMI helpers supports for big endian

[Alexander Wilhelm]

Fix QMI encoding and decoding for variable length elements to support big
endian platforms. Also fix endiannes for QMI header.

Alexander Wilhelm (2):
  soc: qcom: QMI encoding/decoding for big endian
  soc: qcom: fix endianness for QMI header

 drivers/soc/qcom/qmi_encdec.c    | 52 +++++++++++++++++++++++++-------
 drivers/soc/qcom/qmi_interface.c |  6 ++--
 include/linux/soc/qcom/qmi.h     |  6 ++--
 3 files changed, 47 insertions(+), 17 deletions(-)


base-commit: a5806cd506af5a7c19bcd596e4708b5c464bfd21
---
Changes in v2:
  -  Make type cast explicit
-- 
2.34.1

[PATCH v2 1/2] soc: qcom: QMI encoding/decoding for big endian

[Alexander Wilhelm]

The QMI_DATA_LEN type may have different sizes. Taking the element's
address of that type and interpret it as a smaller sized ones works fine
for little endian platforms but not for big endian ones. Instead use
temporary variables of smaller sized types and cast them correctly to
support big endian platforms.

Signed-off-by: Alexander Wilhelm <alexander.wilhelm@westermo.com>
---
 drivers/soc/qcom/qmi_encdec.c | 46 +++++++++++++++++++++++++++++------
 1 file changed, 38 insertions(+), 8 deletions(-)

diff --git a/drivers/soc/qcom/qmi_encdec.c b/drivers/soc/qcom/qmi_encdec.c
index bb09eff85cff..dafe0a4c202e 100644
--- a/drivers/soc/qcom/qmi_encdec.c
+++ b/drivers/soc/qcom/qmi_encdec.c
@@ -304,6 +304,8 @@ static int qmi_encode(const struct qmi_elem_info *ei_array, void *out_buf,
 	const void *buf_src;
 	int encode_tlv = 0;
 	int rc;
+	u8 val8;
+	u16 val16;
 
 	if (!ei_array)
 		return 0;
@@ -338,7 +340,6 @@ static int qmi_encode(const struct qmi_elem_info *ei_array, void *out_buf,
 			break;
 
 		case QMI_DATA_LEN:
-			memcpy(&data_len_value, buf_src, temp_ei->elem_size);
 			data_len_sz = temp_ei->elem_size == sizeof(u8) ?
 					sizeof(u8) : sizeof(u16);
 			/* Check to avoid out of range buffer access */
@@ -348,8 +349,17 @@ static int qmi_encode(const struct qmi_elem_info *ei_array, void *out_buf,
 				       __func__);
 				return -ETOOSMALL;
 			}
-			rc = qmi_encode_basic_elem(buf_dst, &data_len_value,
-						   1, data_len_sz);
+			if (data_len_sz == sizeof(u8)) {
+				val8 = *(u8 *)buf_src;
+				data_len_value = (u32)val8;
+				rc = qmi_encode_basic_elem(buf_dst, &val8,
+							   1, data_len_sz);
+			} else {
+				val16 = *(u16 *)buf_src;
+				data_len_value = (u32)le16_to_cpu(val16);
+				rc = qmi_encode_basic_elem(buf_dst, &val16,
+							   1, data_len_sz);
+			}
 			UPDATE_ENCODE_VARIABLES(temp_ei, buf_dst,
 						encoded_bytes, tlv_len,
 						encode_tlv, rc);
@@ -523,14 +533,23 @@ static int qmi_decode_string_elem(const struct qmi_elem_info *ei_array,
 	u32 string_len = 0;
 	u32 string_len_sz = 0;
 	const struct qmi_elem_info *temp_ei = ei_array;
+	u8 val8;
+	u16 val16;
 
 	if (dec_level == 1) {
 		string_len = tlv_len;
 	} else {
 		string_len_sz = temp_ei->elem_len <= U8_MAX ?
 				sizeof(u8) : sizeof(u16);
-		rc = qmi_decode_basic_elem(&string_len, buf_src,
-					   1, string_len_sz);
+		if (string_len_sz == sizeof(u8)) {
+			rc = qmi_decode_basic_elem(&val8, buf_src,
+						   1, string_len_sz);
+			string_len = (u32)val8;
+		} else {
+			rc = qmi_decode_basic_elem(&val16, buf_src,
+						   1, string_len_sz);
+			string_len = (u32)val16;
+		}
 		decoded_bytes += rc;
 	}
 
@@ -604,6 +623,9 @@ static int qmi_decode(const struct qmi_elem_info *ei_array, void *out_c_struct,
 	u32 decoded_bytes = 0;
 	const void *buf_src = in_buf;
 	int rc;
+	u8 val8;
+	u16 val16;
+	u32 val32;
 
 	while (decoded_bytes < in_buf_len) {
 		if (dec_level >= 2 && temp_ei->data_type == QMI_EOTI)
@@ -642,9 +664,17 @@ static int qmi_decode(const struct qmi_elem_info *ei_array, void *out_c_struct,
 		if (temp_ei->data_type == QMI_DATA_LEN) {
 			data_len_sz = temp_ei->elem_size == sizeof(u8) ?
 					sizeof(u8) : sizeof(u16);
-			rc = qmi_decode_basic_elem(&data_len_value, buf_src,
-						   1, data_len_sz);
-			memcpy(buf_dst, &data_len_value, sizeof(u32));
+			if (data_len_sz == sizeof(u8)) {
+				rc = qmi_decode_basic_elem(&val8, buf_src,
+							   1, data_len_sz);
+				data_len_value = (u32)val8;
+			} else {
+				rc = qmi_decode_basic_elem(&val16, buf_src,
+							   1, data_len_sz);
+				data_len_value = (u32)val16;
+			}
+			val32 = cpu_to_le32(data_len_value);
+			memcpy(buf_dst, &val32, sizeof(u32));
 			temp_ei = temp_ei + 1;
 			buf_dst = out_c_struct + temp_ei->offset;
 			tlv_len -= data_len_sz;
-- 
2.34.1

Re: [PATCH v2 1/2] soc: qcom: QMI encoding/decoding for big endian

[Dmitry Baryshkov]

On Thu, May 22, 2025 at 04:35:29PM +0200, Alexander Wilhelm wrote:
> The QMI_DATA_LEN type may have different sizes. Taking the element's
> address of that type and interpret it as a smaller sized ones works fine
> for little endian platforms but not for big endian ones. Instead use
> temporary variables of smaller sized types and cast them correctly to
> support big endian platforms.
> 
> Signed-off-by: Alexander Wilhelm <alexander.wilhelm@westermo.com>
> ---
>  drivers/soc/qcom/qmi_encdec.c | 46 +++++++++++++++++++++++++++++------
>  1 file changed, 38 insertions(+), 8 deletions(-)


Fixes: 9b8a11e82615 ("soc: qcom: Introduce QMI encoder/decoder")


Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

-- 
With best wishes
Dmitry

[PATCH v2 2/2] soc: qcom: fix endianness for QMI header

[Alexander Wilhelm]

The members of QMI header have to be swapped on big endian platforms. Use
__le16 types instead of u16 ones.

Signed-off-by: Alexander Wilhelm <alexander.wilhelm@westermo.com>
---
 drivers/soc/qcom/qmi_encdec.c    | 6 +++---
 drivers/soc/qcom/qmi_interface.c | 6 +++---
 include/linux/soc/qcom/qmi.h     | 6 +++---
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/soc/qcom/qmi_encdec.c b/drivers/soc/qcom/qmi_encdec.c
index dafe0a4c202e..7660a960fb45 100644
--- a/drivers/soc/qcom/qmi_encdec.c
+++ b/drivers/soc/qcom/qmi_encdec.c
@@ -776,9 +776,9 @@ void *qmi_encode_message(int type, unsigned int msg_id, size_t *len,
 
 	hdr = msg;
 	hdr->type = type;
-	hdr->txn_id = txn_id;
-	hdr->msg_id = msg_id;
-	hdr->msg_len = msglen;
+	hdr->txn_id = cpu_to_le16(txn_id);
+	hdr->msg_id = cpu_to_le16(msg_id);
+	hdr->msg_len = cpu_to_le16(msglen);
 
 	*len = sizeof(*hdr) + msglen;
 
diff --git a/drivers/soc/qcom/qmi_interface.c b/drivers/soc/qcom/qmi_interface.c
index bc6d6379d8b1..6500f863aae5 100644
--- a/drivers/soc/qcom/qmi_interface.c
+++ b/drivers/soc/qcom/qmi_interface.c
@@ -400,7 +400,7 @@ static void qmi_invoke_handler(struct qmi_handle *qmi, struct sockaddr_qrtr *sq,
 
 	for (handler = qmi->handlers; handler->fn; handler++) {
 		if (handler->type == hdr->type &&
-		    handler->msg_id == hdr->msg_id)
+		    handler->msg_id == le16_to_cpu(hdr->msg_id))
 			break;
 	}
 
@@ -488,7 +488,7 @@ static void qmi_handle_message(struct qmi_handle *qmi,
 	/* If this is a response, find the matching transaction handle */
 	if (hdr->type == QMI_RESPONSE) {
 		mutex_lock(&qmi->txn_lock);
-		txn = idr_find(&qmi->txns, hdr->txn_id);
+		txn = idr_find(&qmi->txns, le16_to_cpu(hdr->txn_id));
 
 		/* Ignore unexpected responses */
 		if (!txn) {
@@ -514,7 +514,7 @@ static void qmi_handle_message(struct qmi_handle *qmi,
 	} else {
 		/* Create a txn based on the txn_id of the incoming message */
 		memset(&tmp_txn, 0, sizeof(tmp_txn));
-		tmp_txn.id = hdr->txn_id;
+		tmp_txn.id = le16_to_cpu(hdr->txn_id);
 
 		qmi_invoke_handler(qmi, sq, &tmp_txn, buf, len);
 	}
diff --git a/include/linux/soc/qcom/qmi.h b/include/linux/soc/qcom/qmi.h
index 469e02d2aa0d..291cdc7ef49c 100644
--- a/include/linux/soc/qcom/qmi.h
+++ b/include/linux/soc/qcom/qmi.h
@@ -24,9 +24,9 @@ struct socket;
  */
 struct qmi_header {
 	u8 type;
-	u16 txn_id;
-	u16 msg_id;
-	u16 msg_len;
+	__le16 txn_id;
+	__le16 msg_id;
+	__le16 msg_len;
 } __packed;
 
 #define QMI_REQUEST	0
-- 
2.34.1

Re: [PATCH v2 2/2] soc: qcom: fix endianness for QMI header

[Dmitry Baryshkov]

On Thu, May 22, 2025 at 04:35:30PM +0200, Alexander Wilhelm wrote:
> The members of QMI header have to be swapped on big endian platforms. Use
> __le16 types instead of u16 ones.
> 
> Signed-off-by: Alexander Wilhelm <alexander.wilhelm@westermo.com>
> ---
>  drivers/soc/qcom/qmi_encdec.c    | 6 +++---
>  drivers/soc/qcom/qmi_interface.c | 6 +++---
>  include/linux/soc/qcom/qmi.h     | 6 +++---
>  3 files changed, 9 insertions(+), 9 deletions(-)

Just out of curiosity, is there a usecase for running QMI helpers on BE
platforms?

-- 
With best wishes
Dmitry

Re: [PATCH v2 2/2] soc: qcom: fix endianness for QMI header

[Alexander Wilhelm]

Am Thu, May 22, 2025 at 10:09:13PM +0300 schrieb Dmitry Baryshkov:
> On Thu, May 22, 2025 at 04:35:30PM +0200, Alexander Wilhelm wrote:
> > The members of QMI header have to be swapped on big endian platforms. Use
> > __le16 types instead of u16 ones.
> > 
> > Signed-off-by: Alexander Wilhelm <alexander.wilhelm@westermo.com>
> > ---
> >  drivers/soc/qcom/qmi_encdec.c    | 6 +++---
> >  drivers/soc/qcom/qmi_interface.c | 6 +++---
> >  include/linux/soc/qcom/qmi.h     | 6 +++---
> >  3 files changed, 9 insertions(+), 9 deletions(-)
> 
> Just out of curiosity, is there a usecase for running QMI helpers on BE
> platforms?

I'm not familiar with modems, but the wireless drivers ath11k and ath12k use the
QMI helpers while transferring the firmware and boardfile to the module. As an
example here is the log by probing ath12k device on big endian without a patch:

    ath12k_pci 0001:01:00.0: BAR 0: assigned [mem 0xc00000000-0xc001fffff 64bit]
    ath12k_pci 0001:01:00.0: MSI vectors: 1
    ath12k_pci 0001:01:00.0: Hardware name: qcn9274 hw2.0
    ath12k_pci 0001:01:00.0: FW Ind register request failed, result: 256, err: 512
    ath12k_pci 0001:01:00.0: qmi failed to send FW indication QMI:-22

Similar behavior is seen with ath11k drivers.


Best regards
Alexander Wilhelm

Re: [PATCH v2 2/2] soc: qcom: fix endianness for QMI header

[Dmitry Baryshkov]

On Fri, May 23, 2025 at 07:57:50AM +0200, Alexander Wilhelm wrote:
> Am Thu, May 22, 2025 at 10:09:13PM +0300 schrieb Dmitry Baryshkov:
> > On Thu, May 22, 2025 at 04:35:30PM +0200, Alexander Wilhelm wrote:
> > > The members of QMI header have to be swapped on big endian platforms. Use
> > > __le16 types instead of u16 ones.
> > > 
> > > Signed-off-by: Alexander Wilhelm <alexander.wilhelm@westermo.com>
> > > ---
> > >  drivers/soc/qcom/qmi_encdec.c    | 6 +++---
> > >  drivers/soc/qcom/qmi_interface.c | 6 +++---
> > >  include/linux/soc/qcom/qmi.h     | 6 +++---
> > >  3 files changed, 9 insertions(+), 9 deletions(-)
> > 
> > Just out of curiosity, is there a usecase for running QMI helpers on BE
> > platforms?
> 
> I'm not familiar with modems, but the wireless drivers ath11k and ath12k use the
> QMI helpers while transferring the firmware and boardfile to the module. As an
> example here is the log by probing ath12k device on big endian without a patch:
> 
>     ath12k_pci 0001:01:00.0: BAR 0: assigned [mem 0xc00000000-0xc001fffff 64bit]
>     ath12k_pci 0001:01:00.0: MSI vectors: 1
>     ath12k_pci 0001:01:00.0: Hardware name: qcn9274 hw2.0
>     ath12k_pci 0001:01:00.0: FW Ind register request failed, result: 256, err: 512
>     ath12k_pci 0001:01:00.0: qmi failed to send FW indication QMI:-22
> 
> Similar behavior is seen with ath11k drivers.

Ack, thanks.
-- 
With best wishes
Dmitry

Re: [PATCH v2 2/2] soc: qcom: fix endianness for QMI header

[Dmitry Baryshkov]

On Thu, May 22, 2025 at 04:35:30PM +0200, Alexander Wilhelm wrote:
> The members of QMI header have to be swapped on big endian platforms. Use
> __le16 types instead of u16 ones.
> 
> Signed-off-by: Alexander Wilhelm <alexander.wilhelm@westermo.com>
> ---
>  drivers/soc/qcom/qmi_encdec.c    | 6 +++---
>  drivers/soc/qcom/qmi_interface.c | 6 +++---
>  include/linux/soc/qcom/qmi.h     | 6 +++---
>  3 files changed, 9 insertions(+), 9 deletions(-)

Fixes: 9b8a11e82615 ("soc: qcom: Introduce QMI encoder/decoder")
Fixes: 3830d0771ef6 ("soc: qcom: Introduce QMI helpers")

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>



-- 
With best wishes
Dmitry

Re: [PATCH v2 0/2] soc: qcom: QMI helpers supports for big endian

[Bjorn Andersson]

On Thu, 22 May 2025 16:35:28 +0200, Alexander Wilhelm wrote:
> Fix QMI encoding and decoding for variable length elements to support big
> endian platforms. Also fix endiannes for QMI header.
> 
> Alexander Wilhelm (2):
>   soc: qcom: QMI encoding/decoding for big endian
>   soc: qcom: fix endianness for QMI header
> 
> [...]

Applied, thanks!

[1/2] soc: qcom: QMI encoding/decoding for big endian
      commit: 3ced38da5f7de4c260f9eaa86fc805827953243a
[2/2] soc: qcom: fix endianness for QMI header
      commit: 07a4688833b237331e5045f90fc546c085b28c86

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>