Re: [PATCH 1/2] soc: qcom: qmi: Fix "invalid data length" in encoder

On Wed, Feb 18, 2026 at 12:53:03PM -0600, Bjorn Andersson wrote:
> On Wed, Feb 18, 2026 at 10:00:22AM -0800, Jeff Johnson wrote:
> > On 2/16/2026 7:25 AM, Bjorn Andersson wrote:
> > > It might very well be that the underlying bug is my expectation that
> > > elem_size should be reflected in the struct and not only in the encoded
> > > message, and hence what I wrote in https://github.com/linux-msm/qmic.
> > > Perhaps the length-specifier of an array should always be u32?
> > > 
> > > @Chris, what does the downstream generator produce here?
> > 
> > Is this behavior just constrained to QMI_DATA_LEN TLVs?
> > 
> > I'm looking at downstream Android WLAN code and it has the same discrepancy,
> > so it appears the code generator is always producing a u32 member in the host
> > struct to hold a QMI_DATA_LEN member even though the actual element size as
> > defined in the qmi_elem_info array is either sizeof(u8) or sizeof(u16).
> > 
> > Does this issue get fixed if we change the member in the host struct, i.e. for
> > the issue mentioned (that I chopped off) modify:
> > struct qmi_wlanfw_request_mem_ind_msg_v01 {
> > -	u32 mem_seg_len;
> > +	u8 mem_seg_len;
> > 
> 
> Yes, that would fix the problem.
> 
> But if your elem_info arrays are coming from the downstream tool, then I
> think the correct way to fix this would be to move in the other
> direction and turn some those u8/u16 members elsewhere into u32.
> 
> I have the downstream code generator source here somewhere...
> 

I reviewed the downstream code generator source and documentation.

We do generate tables matching the ath12k c-structures, i.e. variable
length arrays are always prefixed with an uint32_t field - not a
uint8_t or uint16_t based on elem_size.

Looking back at the original implementation of the in-kernel
qmi_encode(), we only read elem_size bytes from the c-structure, but we
do so into the (little-endian) uint32_t on the stack, from which we
encode the message and act upon the result.

In qmi_decode() we decode elem_size bytes from the message into the
(little endian) uint32_t and then write 4 bytes to the c-structure.


The fix would as such seem to be to just update the length fields to be
all uint32_t. The problem I see with this is that qmic [1] is the only
publicly available code generator, and if we change it to always
generate uint32_t length members, we also need to fix the
encoder/decoder in libqrtr [2] - which will be an ABI breaking change.

If we go the other way around, the drawback is that we no longer support
the c-structures generated by the proprietary code generator.

Worth pointing out is that the structure of the c-code is an ABI between
the encoder/decoder, the code generator and the client - it does not
affect the wire format.

[1] https://github.com/linux-msm/qmic
[2] https://github.com/linux-msm/qrtr

Regards,
Bjorn

> Regards,
> Bjorn