linux-kernel - Re: [PATCH 2/3] usb: xhci-mtk: fix UAS issue by XHCI_BROKEN

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 17 Dec 2020 10:19:17 +0800
From:   Chunfeng Yun <chunfeng.yun@...iatek.com>
To:     Nicolas Boichat <drinkcat@...omium.org>
CC:     Rob Herring <robh+dt@...nel.org>,
        Mathias Nyman <mathias.nyman@...el.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Matthias Brugger <matthias.bgg@...il.com>,
        <linux-usb@...r.kernel.org>,
        linux-arm Mailing List <linux-arm-kernel@...ts.infradead.org>,
        "moderated list:ARM/Mediatek SoC support" 
        <linux-mediatek@...ts.infradead.org>,
        Devicetree List <devicetree@...r.kernel.org>,
        lkml <linux-kernel@...r.kernel.org>,
        "Hsin-Yi Wang" <hsinyi@...omium.org>,
        Ikjoon Jang <ikjn@...omium.org>
Subject: Re: [PATCH 2/3] usb: xhci-mtk: fix UAS issue by XHCI_BROKEN_STREAMS
 quirk

On Wed, 2020-12-16 at 20:28 +0800, Nicolas Boichat wrote:
> On Wed, Dec 16, 2020 at 7:53 PM Chunfeng Yun <chunfeng.yun@...iatek.com> wrote:
> >
> > The 0.96 xHCI controller on some platforms does not support
> > bulk stream even HCCPARAMS says supporting, due to MaxPSASize
> > is set a non-zero default value by mistake, here use
> > XHCI_BROKEN_STREAMS quirk to fix it.
> >
> > Fixes: 94a631d91ad3 ("usb: xhci-mtk: check hcc_params after adding primary hcd")
> > Signed-off-by: Chunfeng Yun <chunfeng.yun@...iatek.com>
> > ---
> >  drivers/usb/host/xhci-mtk.c | 7 ++++++-
> >  drivers/usb/host/xhci-mtk.h | 1 +
> >  2 files changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> > index 8f321f39ab96..08dab974d847 100644
> > --- a/drivers/usb/host/xhci-mtk.c
> > +++ b/drivers/usb/host/xhci-mtk.c
> > @@ -395,6 +395,9 @@ static void xhci_mtk_quirks(struct device *dev, struct xhci_hcd *xhci)
> >         xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
> >         if (mtk->lpm_support)
> >                 xhci->quirks |= XHCI_LPM_SUPPORT;
> > +
> > +       if (mtk->broken_streams)
> > +               xhci->quirks |= XHCI_BROKEN_STREAMS;
> >  }
> >
> >  /* called during probe() after chip reset completes */
> > @@ -460,6 +463,8 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> >                 return ret;
> >
> >         mtk->lpm_support = of_property_read_bool(node, "usb3-lpm-capable");
> > +       mtk->broken_streams =
> > +               of_property_read_bool(node, "mediatek,broken_streams_quirk");
> 
> Would it be better to add a data field to struct of_device_id
> mtk_xhci_of_match, and enable this quirk on mediatek,mt8173-xhci only?
This is the common issue for all SoCs (before 2016.06) with 0.96 xHCI
when the controller don't support bulk stream. If enable this quirk only
for mt8173, then for other SoCs, the compatible need include
"mediatek,mt8173-xhci" in dts, this may be not flexible for some cases,
e.g. a new SoC has the broken stream as mt8173, but also has another
different quirk, the way you suggested will not handle it.
And I plan to remove "mediatek,mt8173-xhci" in mtk_xhci_of_match after
converting the binding to YMAL.

> 
> (IMHO usb3-lpm-capable detection should also be done in the same way)
I prefer to provide a property for common issues, and use the way you
suggested for the issue only happened at a specific SoC.

Thank you

> 
> Thanks,
> 
> >         /* optional property, ignore the error if it does not exist */
> >         of_property_read_u32(node, "mediatek,u3p-dis-msk",
> >                              &mtk->u3p_dis_msk);
> > @@ -546,7 +551,7 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> >         if (ret)
> >                 goto put_usb3_hcd;
> >
> > -       if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
> > +       if (!mtk->broken_streams && HCC_MAX_PSA(xhci->hcc_params) >= 4)
> >                 xhci->shared_hcd->can_do_streams = 1;
> >
> >         ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
> > diff --git a/drivers/usb/host/xhci-mtk.h b/drivers/usb/host/xhci-mtk.h
> > index a93cfe817904..86aa4978915e 100644
> > --- a/drivers/usb/host/xhci-mtk.h
> > +++ b/drivers/usb/host/xhci-mtk.h
> > @@ -147,6 +147,7 @@ struct xhci_hcd_mtk {
> >         struct phy **phys;
> >         int num_phys;
> >         bool lpm_support;
> > +       bool broken_streams;
> >         /* usb remote wakeup */
> >         bool uwk_en;
> >         struct regmap *uwk;
> > --
> > 2.18.0