Re: [PATCH v5 2/3] aspeed/hace: Support AST2600 HACE

[Steven Lee <steven_lee@aspeedtech.com>]

The 04/25/2022 21:46, Cédric Le Goater wrote:
> On 4/22/22 07:19, Steven Lee wrote:
> > +/**
> > + * Check whether the request contains padding message.
> > + *
> > + * @param s             aspeed hace state object
> > + * @param iov           iov of current request
> > + * @param req_len       length of the current request
> > + * @param total_msg_len length of all acc_mode requests(excluding padding msg)
> > + * @param pad_offset    start offset of padding message
> > + */
> > +static bool has_padding(AspeedHACEState *s, struct iovec *iov,
> > +                        hwaddr req_len, uint32_t *total_msg_len,
> > +                        uint32_t *pad_offset)
> > +{
> > +    *total_msg_len = (uint32_t)(ldq_be_p(iov->iov_base + req_len - 8) / 8);
> > +    /*
> > +     * SG_LIST_LEN_LAST asserted in the request length doesn't mean it is the
> > +     * last request. The last request should contain padding message.
> > +     * We check whether message contains padding by
> > +     *   1. Get total message length. If the current message contains
> > +     *      padding, the last 8 bytes are total message length.
> > +     *   2. Check whether the total message length is valid.
> > +     *      If it is valid, the value should less than or equal to
> > +     *      total_req_len.
> > +     *   3. Current request len - padding_size to get padding offset.
> > +     *      The padding message's first byte should be 0x80
> > +     */
> > +    if (*total_msg_len <= s->total_req_len) {
> > +        uint32_t padding_size = s->total_req_len - *total_msg_len;
> > +        uint8_t *padding = iov->iov_base;
> > +        *pad_offset = req_len - padding_size;
> > +        if (padding[*pad_offset] == 0x80) {
> > +            return true;
> > +        }
> > +    }
> > +
> > +    return false;
> > +}
> > +
> > +static int reconstruct_iov(AspeedHACEState *s, struct iovec *iov, int id,
> > +                           uint32_t *pad_offset)
> > +{
> > +    int i, iov_count;
> > +    if (pad_offset != 0) {
> 
> pad_offset can not be NULL. or may be you meant *pad_offset ?
> 
> Thanks,
> 
> C.
> 
> 

Yes, it should be *pad_offset, I will fix it.

Thanks,
Steven

> > +        s->iov_cache[s->iov_count].iov_base = iov[id].iov_base;
> > +        s->iov_cache[s->iov_count].iov_len = *pad_offset;
> > +        ++s->iov_count;
> 
> 
> > +    }
> > +    for (i = 0; i < s->iov_count; i++) {
> > +        iov[i].iov_base = s->iov_cache[i].iov_base;
> > +        iov[i].iov_len = s->iov_cache[i].iov_len;
> > +    }
> > +    iov_count = s->iov_count;
> > +    s->iov_count = 0;
> > +    s->total_req_len = 0;
> > +    return iov_count;
> > +}
> > +
>