Hi Lore,
On Wed, 2022-11-30 at 15:46 +0100, Lorenzo Bianconi wrote:
> > SDIO may need addtional 512 bytes to align bus operation. If the
> > tailroom
> > of this skb is not big enough, we would access invalid memory
> > region.
> > For low level operation, take xmit_buf instead of skb to keep valid
> > memory
> > access in SDIO.
> > Note: xmit_buf is big enough for single skb size
> >
> > Fixes: 764dee47e2c1 ("mt76: sdio: move common code in mt76_sdio
> > module")
> > Tested-by: YN Chen <YN.Chen@xxxxxxxxxxxx>
> > Signed-off-by: Deren Wu <deren.wu@xxxxxxxxxxxx>
> > ---
> > drivers/net/wireless/mediatek/mt76/sdio_txrx.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/wireless/mediatek/mt76/sdio_txrx.c
> > b/drivers/net/wireless/mediatek/mt76/sdio_txrx.c
> > index bfc4de50a4d2..ebea5c4e8da5 100644
> > --- a/drivers/net/wireless/mediatek/mt76/sdio_txrx.c
> > +++ b/drivers/net/wireless/mediatek/mt76/sdio_txrx.c
> > @@ -254,7 +254,8 @@ static int mt76s_tx_run_queue(struct mt76_dev
> > *dev, struct mt76_queue *q)
> >
> > if (!test_bit(MT76_STATE_MCU_RUNNING, &dev->phy.state))
> > {
> > __skb_put_zero(e->skb, 4);
> > - err = __mt76s_xmit_queue(dev, e->skb->data,
> > + memcpy(sdio->xmit_buf, e->skb->data, e->skb-
> > >len);
>
> (even if it is not critical for performance) iirc the skb from the
> mcu is
> always linear, I guess we can use __skb_grow() instead. What do you
> think?
>
> Regards,
> Lorenzo
>
_skb_grow() looks good for me. It's a balance solution for this case.
If you have no concern about the patch below, I will post v2 after UT.
:)
if (!test_bit(MT76_STATE_MCU_RUNNING, &dev->phy.state)) {
__skb_put_zero(e->skb, 4);
+ err = __skb_grow(e->skb, roundup(e->skb->len,
+ sdio->func-
>cur_blksize));
+ if (err)
+ return err;
err = __mt76s_xmit_queue(dev, e->skb->data,
e->skb->len);
Regards,
Deren
> > + err = __mt76s_xmit_queue(dev, sdio->xmit_buf,
> > e->skb->len);
> > if (err)
> > return err;
> > --
> > 2.18.0
> >
