| Message ID | 20230725061008.1504292-1-jiaxun.yang@flygoat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] pci: loongson: Workaround MIPS firmware MRRS settings | expand |
在 2023/7/25 14:10, Jiaxun Yang 写道: > This is a partial revert of commit 8b3517f88ff2 ("PCI: > loongson: Prevent LS7A MRRS increases") for MIPS based Loongson. > > There are many MIPS based Loongson systems in wild that > shipped with firmware which does not set maximum MRRS properly. > > Limiting MRRS to 256 for all as MIPS Loongson comes with higher > MRRS support is considered rare. > > Cc: stable@vger.kernel.org > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217680 > Fixes: 8b3517f88ff2 ("PCI: loongson: Prevent LS7A MRRS increases") > Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com> Ping? I expect this patch to go through PCI fixes tree. Thanks - Jiaxun > --- > v2: Rename quirk name to: loongson_old_mrrs_quirk > --- > drivers/pci/controller/pci-loongson.c | 38 +++++++++++++++++++++++++++ > 1 file changed, 38 insertions(+) > > diff --git a/drivers/pci/controller/pci-loongson.c b/drivers/pci/controller/pci-loongson.c > index fe0f732f6e43..d0f68b102d10 100644 > --- a/drivers/pci/controller/pci-loongson.c > +++ b/drivers/pci/controller/pci-loongson.c > @@ -108,6 +108,44 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON, > DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON, > DEV_LS7A_PCIE_PORT6, loongson_mrrs_quirk); > > +#ifdef CONFIG_MIPS > +static void loongson_old_mrrs_quirk(struct pci_dev *pdev) > +{ > + struct pci_bus *bus = pdev->bus; > + struct pci_dev *bridge; > + static const struct pci_device_id bridge_devids[] = { > + { PCI_VDEVICE(LOONGSON, DEV_LS2K_PCIE_PORT0) }, > + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT0) }, > + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT1) }, > + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT2) }, > + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT3) }, > + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT4) }, > + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT5) }, > + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT6) }, > + { 0, }, > + }; > + > + /* look for the matching bridge */ > + while (!pci_is_root_bus(bus)) { > + bridge = bus->self; > + bus = bus->parent; > + /* > + * There are still some wild MIPS Loongson firmware won't > + * set MRRS properly. Limiting MRRS to 256 as MIPS Loongson > + * comes with higher MRRS support is considered rare. > + */ > + if (pci_match_id(bridge_devids, bridge)) { > + if (pcie_get_readrq(pdev) > 256) { > + pci_info(pdev, "limiting MRRS to 256\n"); > + pcie_set_readrq(pdev, 256); > + } > + break; > + } > + } > +} > +DECLARE_PCI_FIXUP_ENABLE(PCI_ANY_ID, PCI_ANY_ID, loongson_old_mrrs_quirk); > +#endif > + > static void loongson_pci_pin_quirk(struct pci_dev *pdev) > { > pdev->pin = 1 + (PCI_FUNC(pdev->devfn) & 3);
Hi, Jiaxun, On Sun, Aug 6, 2023 at 10:20 AM Jiaxun Yang <jiaxun.yang@flygoat.com> wrote: > > > > 在 2023/7/25 14:10, Jiaxun Yang 写道: > > This is a partial revert of commit 8b3517f88ff2 ("PCI: > > loongson: Prevent LS7A MRRS increases") for MIPS based Loongson. > > > > There are many MIPS based Loongson systems in wild that > > shipped with firmware which does not set maximum MRRS properly. > > > > Limiting MRRS to 256 for all as MIPS Loongson comes with higher > > MRRS support is considered rare. > > > > Cc: stable@vger.kernel.org > > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217680 > > Fixes: 8b3517f88ff2 ("PCI: loongson: Prevent LS7A MRRS increases") > > Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com> > > Ping? > I expect this patch to go through PCI fixes tree. Can we do it like this by modifying the existing loongson_mrrs_quirk()? static void loongson_mrrs_quirk(struct pci_dev *pdev) { /* * Some Loongson PCIe ports have h/w limitations of maximum read * request size. They can't handle anything larger than this. So * force this limit on any devices attached under these ports. */ struct pci_host_bridge *bridge = pci_find_host_bridge(pdev->bus); #ifdef CONFIG_MIPS set_pcie_ports_to_mrrs_256_to_emulate_the_firmware_behavior(); #endif bridge->no_inc_mrrs = 1; } > > Thanks > - Jiaxun > > > --- > > v2: Rename quirk name to: loongson_old_mrrs_quirk > > --- > > drivers/pci/controller/pci-loongson.c | 38 +++++++++++++++++++++++++++ > > 1 file changed, 38 insertions(+) > > > > diff --git a/drivers/pci/controller/pci-loongson.c b/drivers/pci/controller/pci-loongson.c > > index fe0f732f6e43..d0f68b102d10 100644 > > --- a/drivers/pci/controller/pci-loongson.c > > +++ b/drivers/pci/controller/pci-loongson.c > > @@ -108,6 +108,44 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON, > > DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON, > > DEV_LS7A_PCIE_PORT6, loongson_mrrs_quirk); > > > > +#ifdef CONFIG_MIPS > > +static void loongson_old_mrrs_quirk(struct pci_dev *pdev) > > +{ > > + struct pci_bus *bus = pdev->bus; > > + struct pci_dev *bridge; > > + static const struct pci_device_id bridge_devids[] = { > > + { PCI_VDEVICE(LOONGSON, DEV_LS2K_PCIE_PORT0) }, > > + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT0) }, > > + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT1) }, > > + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT2) }, > > + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT3) }, > > + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT4) }, > > + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT5) }, > > + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT6) }, > > + { 0, }, > > + }; > > + > > + /* look for the matching bridge */ > > + while (!pci_is_root_bus(bus)) { > > + bridge = bus->self; > > + bus = bus->parent; > > + /* > > + * There are still some wild MIPS Loongson firmware won't > > + * set MRRS properly. Limiting MRRS to 256 as MIPS Loongson > > + * comes with higher MRRS support is considered rare. > > + */ > > + if (pci_match_id(bridge_devids, bridge)) { > > + if (pcie_get_readrq(pdev) > 256) { > > + pci_info(pdev, "limiting MRRS to 256\n"); > > + pcie_set_readrq(pdev, 256); > > + } > > + break; > > + } > > + } > > +} > > +DECLARE_PCI_FIXUP_ENABLE(PCI_ANY_ID, PCI_ANY_ID, loongson_old_mrrs_quirk); > > +#endif > > + > > static void loongson_pci_pin_quirk(struct pci_dev *pdev) > > { > > pdev->pin = 1 + (PCI_FUNC(pdev->devfn) & 3); >
在 2023/8/6 22:30, Huacai Chen 写道: > Hi, Jiaxun, > > On Sun, Aug 6, 2023 at 10:20 AM Jiaxun Yang <jiaxun.yang@flygoat.com> wrote: >> >> >> 在 2023/7/25 14:10, Jiaxun Yang 写道: >>> This is a partial revert of commit 8b3517f88ff2 ("PCI: >>> loongson: Prevent LS7A MRRS increases") for MIPS based Loongson. >>> >>> There are many MIPS based Loongson systems in wild that >>> shipped with firmware which does not set maximum MRRS properly. >>> >>> Limiting MRRS to 256 for all as MIPS Loongson comes with higher >>> MRRS support is considered rare. >>> >>> Cc: stable@vger.kernel.org >>> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217680 >>> Fixes: 8b3517f88ff2 ("PCI: loongson: Prevent LS7A MRRS increases") >>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com> >> Ping? >> I expect this patch to go through PCI fixes tree. > Can we do it like this by modifying the existing loongson_mrrs_quirk()? Hmm, I'm not sure this will work, since loongson_mrrs_quirk only run on bridges but the old quirk should run on every single device. Thanks Jiaxun > > static void loongson_mrrs_quirk(struct pci_dev *pdev) > { > /* > * Some Loongson PCIe ports have h/w limitations of maximum read > * request size. They can't handle anything larger than this. So > * force this limit on any devices attached under these ports. > */ > struct pci_host_bridge *bridge = pci_find_host_bridge(pdev->bus); > > #ifdef CONFIG_MIPS > set_pcie_ports_to_mrrs_256_to_emulate_the_firmware_behavior(); > #endif > > bridge->no_inc_mrrs = 1; > } > >> Thanks >> - Jiaxun >> >>> --- >>> v2: Rename quirk name to: loongson_old_mrrs_quirk >>> --- >>> drivers/pci/controller/pci-loongson.c | 38 +++++++++++++++++++++++++++ >>> 1 file changed, 38 insertions(+) >>> >>> diff --git a/drivers/pci/controller/pci-loongson.c b/drivers/pci/controller/pci-loongson.c >>> index fe0f732f6e43..d0f68b102d10 100644 >>> --- a/drivers/pci/controller/pci-loongson.c >>> +++ b/drivers/pci/controller/pci-loongson.c >>> @@ -108,6 +108,44 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON, >>> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON, >>> DEV_LS7A_PCIE_PORT6, loongson_mrrs_quirk); >>> >>> +#ifdef CONFIG_MIPS >>> +static void loongson_old_mrrs_quirk(struct pci_dev *pdev) >>> +{ >>> + struct pci_bus *bus = pdev->bus; >>> + struct pci_dev *bridge; >>> + static const struct pci_device_id bridge_devids[] = { >>> + { PCI_VDEVICE(LOONGSON, DEV_LS2K_PCIE_PORT0) }, >>> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT0) }, >>> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT1) }, >>> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT2) }, >>> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT3) }, >>> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT4) }, >>> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT5) }, >>> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT6) }, >>> + { 0, }, >>> + }; >>> + >>> + /* look for the matching bridge */ >>> + while (!pci_is_root_bus(bus)) { >>> + bridge = bus->self; >>> + bus = bus->parent; >>> + /* >>> + * There are still some wild MIPS Loongson firmware won't >>> + * set MRRS properly. Limiting MRRS to 256 as MIPS Loongson >>> + * comes with higher MRRS support is considered rare. >>> + */ >>> + if (pci_match_id(bridge_devids, bridge)) { >>> + if (pcie_get_readrq(pdev) > 256) { >>> + pci_info(pdev, "limiting MRRS to 256\n"); >>> + pcie_set_readrq(pdev, 256); >>> + } >>> + break; >>> + } >>> + } >>> +} >>> +DECLARE_PCI_FIXUP_ENABLE(PCI_ANY_ID, PCI_ANY_ID, loongson_old_mrrs_quirk); >>> +#endif >>> + >>> static void loongson_pci_pin_quirk(struct pci_dev *pdev) >>> { >>> pdev->pin = 1 + (PCI_FUNC(pdev->devfn) & 3);
Hi, Thorsten here, the Linux kernel's regression tracker. Top-posting for once, to make this easily accessible to everyone. What's the status here? From my point it looks like this regression that was ported six weeks ago is still not fixed -- and it seems nothing has happened since three weeks now. But maybe I'm just missing something, that's why I'm asking. Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) -- Everything you wanna know about Linux kernel regression tracking: https://linux-regtracking.leemhuis.info/about/#tldr If I did something stupid, please tell me, as explained on that page. #regzbot poke On 08.08.23 09:38, Jiaxun Yang wrote: > > > 在 2023/8/6 22:30, Huacai Chen 写道: >> Hi, Jiaxun, >> >> On Sun, Aug 6, 2023 at 10:20 AM Jiaxun Yang <jiaxun.yang@flygoat.com> >> wrote: >>> >>> >>> 在 2023/7/25 14:10, Jiaxun Yang 写道: >>>> This is a partial revert of commit 8b3517f88ff2 ("PCI: >>>> loongson: Prevent LS7A MRRS increases") for MIPS based Loongson. >>>> >>>> There are many MIPS based Loongson systems in wild that >>>> shipped with firmware which does not set maximum MRRS properly. >>>> >>>> Limiting MRRS to 256 for all as MIPS Loongson comes with higher >>>> MRRS support is considered rare. >>>> >>>> Cc: stable@vger.kernel.org >>>> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217680 >>>> Fixes: 8b3517f88ff2 ("PCI: loongson: Prevent LS7A MRRS increases") >>>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com> >>> Ping? >>> I expect this patch to go through PCI fixes tree. >> Can we do it like this by modifying the existing loongson_mrrs_quirk()? > > Hmm, I'm not sure this will work, since loongson_mrrs_quirk only run on > bridges > but the old quirk should run on every single device. > > Thanks > Jiaxun > >> >> static void loongson_mrrs_quirk(struct pci_dev *pdev) >> { >> /* >> * Some Loongson PCIe ports have h/w limitations of maximum read >> * request size. They can't handle anything larger than this. So >> * force this limit on any devices attached under these ports. >> */ >> struct pci_host_bridge *bridge = >> pci_find_host_bridge(pdev->bus); >> >> #ifdef CONFIG_MIPS >> set_pcie_ports_to_mrrs_256_to_emulate_the_firmware_behavior(); >> #endif >> >> bridge->no_inc_mrrs = 1; >> } >> >>> Thanks >>> - Jiaxun >>> >>>> --- >>>> v2: Rename quirk name to: loongson_old_mrrs_quirk >>>> --- >>>> drivers/pci/controller/pci-loongson.c | 38 >>>> +++++++++++++++++++++++++++ >>>> 1 file changed, 38 insertions(+) >>>> >>>> diff --git a/drivers/pci/controller/pci-loongson.c >>>> b/drivers/pci/controller/pci-loongson.c >>>> index fe0f732f6e43..d0f68b102d10 100644 >>>> --- a/drivers/pci/controller/pci-loongson.c >>>> +++ b/drivers/pci/controller/pci-loongson.c >>>> @@ -108,6 +108,44 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON, >>>> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON, >>>> DEV_LS7A_PCIE_PORT6, loongson_mrrs_quirk); >>>> >>>> +#ifdef CONFIG_MIPS >>>> +static void loongson_old_mrrs_quirk(struct pci_dev *pdev) >>>> +{ >>>> + struct pci_bus *bus = pdev->bus; >>>> + struct pci_dev *bridge; >>>> + static const struct pci_device_id bridge_devids[] = { >>>> + { PCI_VDEVICE(LOONGSON, DEV_LS2K_PCIE_PORT0) }, >>>> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT0) }, >>>> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT1) }, >>>> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT2) }, >>>> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT3) }, >>>> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT4) }, >>>> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT5) }, >>>> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT6) }, >>>> + { 0, }, >>>> + }; >>>> + >>>> + /* look for the matching bridge */ >>>> + while (!pci_is_root_bus(bus)) { >>>> + bridge = bus->self; >>>> + bus = bus->parent; >>>> + /* >>>> + * There are still some wild MIPS Loongson firmware won't >>>> + * set MRRS properly. Limiting MRRS to 256 as MIPS >>>> Loongson >>>> + * comes with higher MRRS support is considered rare. >>>> + */ >>>> + if (pci_match_id(bridge_devids, bridge)) { >>>> + if (pcie_get_readrq(pdev) > 256) { >>>> + pci_info(pdev, "limiting MRRS to 256\n"); >>>> + pcie_set_readrq(pdev, 256); >>>> + } >>>> + break; >>>> + } >>>> + } >>>> +} >>>> +DECLARE_PCI_FIXUP_ENABLE(PCI_ANY_ID, PCI_ANY_ID, >>>> loongson_old_mrrs_quirk); >>>> +#endif >>>> + >>>> static void loongson_pci_pin_quirk(struct pci_dev *pdev) >>>> { >>>> pdev->pin = 1 + (PCI_FUNC(pdev->devfn) & 3); >
在 2023/8/29 20:26, Linux regression tracking (Thorsten Leemhuis) 写道: > Hi, Thorsten here, the Linux kernel's regression tracker. Top-posting > for once, to make this easily accessible to everyone. > > What's the status here? From my point it looks like this regression that > was ported six weeks ago is still not fixed -- and it seems nothing has > happened since three weeks now. But maybe I'm just missing something, > that's why I'm asking. Hi Thorsten, Thanks for reaching back, unfortunately there is no progress so far. + Huacai, what's your opinion here? I do think this fix is in best shape for now. Thanks - Jiaxun > > Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) > -- > Everything you wanna know about Linux kernel regression tracking: > https://linux-regtracking.leemhuis.info/about/#tldr > If I did something stupid, please tell me, as explained on that page. > > #regzbot poke > > On 08.08.23 09:38, Jiaxun Yang wrote: >> >> 在 2023/8/6 22:30, Huacai Chen 写道: >>> Hi, Jiaxun, >>> >>> On Sun, Aug 6, 2023 at 10:20 AM Jiaxun Yang <jiaxun.yang@flygoat.com> >>> wrote: >>>> >>>> 在 2023/7/25 14:10, Jiaxun Yang 写道: >>>>> This is a partial revert of commit 8b3517f88ff2 ("PCI: >>>>> loongson: Prevent LS7A MRRS increases") for MIPS based Loongson. >>>>> >>>>> There are many MIPS based Loongson systems in wild that >>>>> shipped with firmware which does not set maximum MRRS properly. >>>>> >>>>> Limiting MRRS to 256 for all as MIPS Loongson comes with higher >>>>> MRRS support is considered rare. >>>>> >>>>> Cc: stable@vger.kernel.org >>>>> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217680 >>>>> Fixes: 8b3517f88ff2 ("PCI: loongson: Prevent LS7A MRRS increases") >>>>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com> >>>> Ping? >>>> I expect this patch to go through PCI fixes tree. >>> Can we do it like this by modifying the existing loongson_mrrs_quirk()? >> Hmm, I'm not sure this will work, since loongson_mrrs_quirk only run on >> bridges >> but the old quirk should run on every single device. >> >> Thanks >> Jiaxun >> >>> static void loongson_mrrs_quirk(struct pci_dev *pdev) >>> { >>> /* >>> * Some Loongson PCIe ports have h/w limitations of maximum read >>> * request size. They can't handle anything larger than this. So >>> * force this limit on any devices attached under these ports. >>> */ >>> struct pci_host_bridge *bridge = >>> pci_find_host_bridge(pdev->bus); >>> >>> #ifdef CONFIG_MIPS >>> set_pcie_ports_to_mrrs_256_to_emulate_the_firmware_behavior(); >>> #endif >>> >>> bridge->no_inc_mrrs = 1; >>> } >>> >>>> Thanks >>>> - Jiaxun >>>> >>>>> --- >>>>> v2: Rename quirk name to: loongson_old_mrrs_quirk >>>>> --- >>>>> drivers/pci/controller/pci-loongson.c | 38 >>>>> +++++++++++++++++++++++++++ >>>>> 1 file changed, 38 insertions(+) >>>>> >>>>> diff --git a/drivers/pci/controller/pci-loongson.c >>>>> b/drivers/pci/controller/pci-loongson.c >>>>> index fe0f732f6e43..d0f68b102d10 100644 >>>>> --- a/drivers/pci/controller/pci-loongson.c >>>>> +++ b/drivers/pci/controller/pci-loongson.c >>>>> @@ -108,6 +108,44 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON, >>>>> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON, >>>>> DEV_LS7A_PCIE_PORT6, loongson_mrrs_quirk); >>>>> >>>>> +#ifdef CONFIG_MIPS >>>>> +static void loongson_old_mrrs_quirk(struct pci_dev *pdev) >>>>> +{ >>>>> + struct pci_bus *bus = pdev->bus; >>>>> + struct pci_dev *bridge; >>>>> + static const struct pci_device_id bridge_devids[] = { >>>>> + { PCI_VDEVICE(LOONGSON, DEV_LS2K_PCIE_PORT0) }, >>>>> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT0) }, >>>>> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT1) }, >>>>> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT2) }, >>>>> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT3) }, >>>>> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT4) }, >>>>> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT5) }, >>>>> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT6) }, >>>>> + { 0, }, >>>>> + }; >>>>> + >>>>> + /* look for the matching bridge */ >>>>> + while (!pci_is_root_bus(bus)) { >>>>> + bridge = bus->self; >>>>> + bus = bus->parent; >>>>> + /* >>>>> + * There are still some wild MIPS Loongson firmware won't >>>>> + * set MRRS properly. Limiting MRRS to 256 as MIPS >>>>> Loongson >>>>> + * comes with higher MRRS support is considered rare. >>>>> + */ >>>>> + if (pci_match_id(bridge_devids, bridge)) { >>>>> + if (pcie_get_readrq(pdev) > 256) { >>>>> + pci_info(pdev, "limiting MRRS to 256\n"); >>>>> + pcie_set_readrq(pdev, 256); >>>>> + } >>>>> + break; >>>>> + } >>>>> + } >>>>> +} >>>>> +DECLARE_PCI_FIXUP_ENABLE(PCI_ANY_ID, PCI_ANY_ID, >>>>> loongson_old_mrrs_quirk); >>>>> +#endif >>>>> + >>>>> static void loongson_pci_pin_quirk(struct pci_dev *pdev) >>>>> { >>>>> pdev->pin = 1 + (PCI_FUNC(pdev->devfn) & 3);
Hi, Jiaxun, On Tue, Aug 8, 2023 at 3:38 PM Jiaxun Yang <jiaxun.yang@flygoat.com> wrote: > > > > 在 2023/8/6 22:30, Huacai Chen 写道: > > Hi, Jiaxun, > > > > On Sun, Aug 6, 2023 at 10:20 AM Jiaxun Yang <jiaxun.yang@flygoat.com> wrote: > >> > >> > >> 在 2023/7/25 14:10, Jiaxun Yang 写道: > >>> This is a partial revert of commit 8b3517f88ff2 ("PCI: > >>> loongson: Prevent LS7A MRRS increases") for MIPS based Loongson. > >>> > >>> There are many MIPS based Loongson systems in wild that > >>> shipped with firmware which does not set maximum MRRS properly. > >>> > >>> Limiting MRRS to 256 for all as MIPS Loongson comes with higher > >>> MRRS support is considered rare. > >>> > >>> Cc: stable@vger.kernel.org > >>> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217680 > >>> Fixes: 8b3517f88ff2 ("PCI: loongson: Prevent LS7A MRRS increases") > >>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com> > >> Ping? > >> I expect this patch to go through PCI fixes tree. > > Can we do it like this by modifying the existing loongson_mrrs_quirk()? > > Hmm, I'm not sure this will work, since loongson_mrrs_quirk only run on > bridges But it is worth to try, and you can walk the children to set mrrs when the quirk runs on bridges, I think. > but the old quirk should run on every single device. Your current patch has a drawback that both quirks will run for MIPS, and their order is random (though it may cause nothing, but not elegant). Huacai > > Thanks > Jiaxun > > > > > static void loongson_mrrs_quirk(struct pci_dev *pdev) > > { > > /* > > * Some Loongson PCIe ports have h/w limitations of maximum read > > * request size. They can't handle anything larger than this. So > > * force this limit on any devices attached under these ports. > > */ > > struct pci_host_bridge *bridge = pci_find_host_bridge(pdev->bus); > > > > #ifdef CONFIG_MIPS > > set_pcie_ports_to_mrrs_256_to_emulate_the_firmware_behavior(); > > #endif > > > > bridge->no_inc_mrrs = 1; > > } > > > >> Thanks > >> - Jiaxun > >> > >>> --- > >>> v2: Rename quirk name to: loongson_old_mrrs_quirk > >>> --- > >>> drivers/pci/controller/pci-loongson.c | 38 +++++++++++++++++++++++++++ > >>> 1 file changed, 38 insertions(+) > >>> > >>> diff --git a/drivers/pci/controller/pci-loongson.c b/drivers/pci/controller/pci-loongson.c > >>> index fe0f732f6e43..d0f68b102d10 100644 > >>> --- a/drivers/pci/controller/pci-loongson.c > >>> +++ b/drivers/pci/controller/pci-loongson.c > >>> @@ -108,6 +108,44 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON, > >>> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON, > >>> DEV_LS7A_PCIE_PORT6, loongson_mrrs_quirk); > >>> > >>> +#ifdef CONFIG_MIPS > >>> +static void loongson_old_mrrs_quirk(struct pci_dev *pdev) > >>> +{ > >>> + struct pci_bus *bus = pdev->bus; > >>> + struct pci_dev *bridge; > >>> + static const struct pci_device_id bridge_devids[] = { > >>> + { PCI_VDEVICE(LOONGSON, DEV_LS2K_PCIE_PORT0) }, > >>> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT0) }, > >>> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT1) }, > >>> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT2) }, > >>> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT3) }, > >>> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT4) }, > >>> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT5) }, > >>> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT6) }, > >>> + { 0, }, > >>> + }; > >>> + > >>> + /* look for the matching bridge */ > >>> + while (!pci_is_root_bus(bus)) { > >>> + bridge = bus->self; > >>> + bus = bus->parent; > >>> + /* > >>> + * There are still some wild MIPS Loongson firmware won't > >>> + * set MRRS properly. Limiting MRRS to 256 as MIPS Loongson > >>> + * comes with higher MRRS support is considered rare. > >>> + */ > >>> + if (pci_match_id(bridge_devids, bridge)) { > >>> + if (pcie_get_readrq(pdev) > 256) { > >>> + pci_info(pdev, "limiting MRRS to 256\n"); > >>> + pcie_set_readrq(pdev, 256); > >>> + } > >>> + break; > >>> + } > >>> + } > >>> +} > >>> +DECLARE_PCI_FIXUP_ENABLE(PCI_ANY_ID, PCI_ANY_ID, loongson_old_mrrs_quirk); > >>> +#endif > >>> + > >>> static void loongson_pci_pin_quirk(struct pci_dev *pdev) > >>> { > >>> pdev->pin = 1 + (PCI_FUNC(pdev->devfn) & 3); >
在 2023/8/31 14:22, Huacai Chen 写道: [...] > But it is worth to try, and you can walk the children to set mrrs when > the quirk runs on bridges, I think. No, this will break hotplug, I had managed to get hotplug work on 2K1000. Also we have no guarantee on order of discovering devices. >> but the old quirk should run on every single device. > Your current patch has a drawback that both quirks will run for MIPS, > and their order is random (though it may cause nothing, but not > elegant). Actually loongson_mrrs_quirk is declared by DECLARE_PCI_FIXUP_EARLY but loongson_old_mrrs_quirk is declared by DECLARE_PCI_FIXUP_ENABLE, which means loongson_old_mrrs_quirk always runs after loongson_mrrs_quirk as expected. Thanks - Jiaxun [...]
On Thu, Aug 31, 2023 at 4:58 PM Jiaxun Yang <jiaxun.yang@flygoat.com> wrote: > > > > 在 2023/8/31 14:22, Huacai Chen 写道: > > [...] > > But it is worth to try, and you can walk the children to set mrrs when > > the quirk runs on bridges, I think. > No, this will break hotplug, I had managed to get hotplug work on 2K1000. > Also we have no guarantee on order of discovering devices. > >> but the old quirk should run on every single device. > > Your current patch has a drawback that both quirks will run for MIPS, > > and their order is random (though it may cause nothing, but not > > elegant). > Actually loongson_mrrs_quirk is declared by DECLARE_PCI_FIXUP_EARLY but > loongson_old_mrrs_quirk is declared by DECLARE_PCI_FIXUP_ENABLE, > which means loongson_old_mrrs_quirk always runs after loongson_mrrs_quirk as > expected. Then I think it is better to put this quirk in arch/mips/loongson64/, just as pci_fixup_radeon(). Huacai > > Thanks > - Jiaxun > > [...]
在 2023/8/31 17:42, Huacai Chen 写道: > On Thu, Aug 31, 2023 at 4:58 PM Jiaxun Yang <jiaxun.yang@flygoat.com> wrote: >> >> >> 在 2023/8/31 14:22, Huacai Chen 写道: >> >> [...] >>> But it is worth to try, and you can walk the children to set mrrs when >>> the quirk runs on bridges, I think. >> No, this will break hotplug, I had managed to get hotplug work on 2K1000. >> Also we have no guarantee on order of discovering devices. >>>> but the old quirk should run on every single device. >>> Your current patch has a drawback that both quirks will run for MIPS, >>> and their order is random (though it may cause nothing, but not >>> elegant). >> Actually loongson_mrrs_quirk is declared by DECLARE_PCI_FIXUP_EARLY but >> loongson_old_mrrs_quirk is declared by DECLARE_PCI_FIXUP_ENABLE, >> which means loongson_old_mrrs_quirk always runs after loongson_mrrs_quirk as >> expected. > Then I think it is better to put this quirk in arch/mips/loongson64/, > just as pci_fixup_radeon(). Hmm, spiting two quirks with similar functionality into two places doesn't sound like a good idea. PCI folks, what do you think? Thanks. - Jiaxun [...]
在 2023/8/31 18:04, Jiaxun Yang 写道: > > > 在 2023/8/31 17:42, Huacai Chen 写道: >> On Thu, Aug 31, 2023 at 4:58 PM Jiaxun Yang <jiaxun.yang@flygoat.com> >> wrote: >>> >>> >>> 在 2023/8/31 14:22, Huacai Chen 写道: >>> >>> [...] >>>> But it is worth to try, and you can walk the children to set mrrs when >>>> the quirk runs on bridges, I think. >>> No, this will break hotplug, I had managed to get hotplug work on >>> 2K1000. >>> Also we have no guarantee on order of discovering devices. >>>>> but the old quirk should run on every single device. >>>> Your current patch has a drawback that both quirks will run for MIPS, >>>> and their order is random (though it may cause nothing, but not >>>> elegant). >>> Actually loongson_mrrs_quirk is declared by DECLARE_PCI_FIXUP_EARLY but >>> loongson_old_mrrs_quirk is declared by DECLARE_PCI_FIXUP_ENABLE, >>> which means loongson_old_mrrs_quirk always runs after >>> loongson_mrrs_quirk as >>> expected. >> Then I think it is better to put this quirk in arch/mips/loongson64/, >> just as pci_fixup_radeon(). Huacai, if you are happy with changes in this patch, do you mind to give out a conditional R-b tag? Since this regression is in tree for a while, I really don't want to see it lapse into next release. > > Hmm, spiting two quirks with similar functionality into two places > doesn't sound > like a good idea. > > PCI folks, what do you think? Bjorn, Lorenzo and Krzysztof, do you mind to give some input on this fix? Thanks - Jiaxun > > Thanks. > - Jiaxun > > [...]
On Tue, Aug 08, 2023 at 03:38:19PM +0800, Jiaxun Yang wrote: > > > 在 2023/8/6 22:30, Huacai Chen 写道: > > Hi, Jiaxun, > > > > On Sun, Aug 6, 2023 at 10:20 AM Jiaxun Yang <jiaxun.yang@flygoat.com> wrote: > > > > > > > > > 在 2023/7/25 14:10, Jiaxun Yang 写道: > > > > This is a partial revert of commit 8b3517f88ff2 ("PCI: > > > > loongson: Prevent LS7A MRRS increases") for MIPS based Loongson. > > > > > > > > There are many MIPS based Loongson systems in wild that > > > > shipped with firmware which does not set maximum MRRS properly. > > > > > > > > Limiting MRRS to 256 for all as MIPS Loongson comes with higher > > > > MRRS support is considered rare. > > > > > > > > Cc: stable@vger.kernel.org > > > > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217680 > > > > Fixes: 8b3517f88ff2 ("PCI: loongson: Prevent LS7A MRRS increases") > > > > Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com> > > > Ping? > > > I expect this patch to go through PCI fixes tree. > > Can we do it like this by modifying the existing loongson_mrrs_quirk()? > > Hmm, I'm not sure this will work, since loongson_mrrs_quirk only run on > bridges > but the old quirk should run on every single device. > Why do you need to walk through every single device instead of just bridges? I'm not the maintainer, but my suggestion is to go for Huacai Chen's solution. This avoids iterating over bridges/devices two times. Also, please rename firmware to BIOS, as firmware commonly represents the software running on PCIe endpoint devices. - Mani > Thanks > Jiaxun > > > > > static void loongson_mrrs_quirk(struct pci_dev *pdev) > > { > > /* > > * Some Loongson PCIe ports have h/w limitations of maximum read > > * request size. They can't handle anything larger than this. So > > * force this limit on any devices attached under these ports. > > */ > > struct pci_host_bridge *bridge = pci_find_host_bridge(pdev->bus); > > > > #ifdef CONFIG_MIPS > > set_pcie_ports_to_mrrs_256_to_emulate_the_firmware_behavior(); > > #endif > > > > bridge->no_inc_mrrs = 1; > > } > > > > > Thanks > > > - Jiaxun > > > > > > > --- > > > > v2: Rename quirk name to: loongson_old_mrrs_quirk > > > > --- > > > > drivers/pci/controller/pci-loongson.c | 38 +++++++++++++++++++++++++++ > > > > 1 file changed, 38 insertions(+) > > > > > > > > diff --git a/drivers/pci/controller/pci-loongson.c b/drivers/pci/controller/pci-loongson.c > > > > index fe0f732f6e43..d0f68b102d10 100644 > > > > --- a/drivers/pci/controller/pci-loongson.c > > > > +++ b/drivers/pci/controller/pci-loongson.c > > > > @@ -108,6 +108,44 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON, > > > > DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON, > > > > DEV_LS7A_PCIE_PORT6, loongson_mrrs_quirk); > > > > > > > > +#ifdef CONFIG_MIPS > > > > +static void loongson_old_mrrs_quirk(struct pci_dev *pdev) > > > > +{ > > > > + struct pci_bus *bus = pdev->bus; > > > > + struct pci_dev *bridge; > > > > + static const struct pci_device_id bridge_devids[] = { > > > > + { PCI_VDEVICE(LOONGSON, DEV_LS2K_PCIE_PORT0) }, > > > > + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT0) }, > > > > + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT1) }, > > > > + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT2) }, > > > > + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT3) }, > > > > + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT4) }, > > > > + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT5) }, > > > > + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT6) }, > > > > + { 0, }, > > > > + }; > > > > + > > > > + /* look for the matching bridge */ > > > > + while (!pci_is_root_bus(bus)) { > > > > + bridge = bus->self; > > > > + bus = bus->parent; > > > > + /* > > > > + * There are still some wild MIPS Loongson firmware won't > > > > + * set MRRS properly. Limiting MRRS to 256 as MIPS Loongson > > > > + * comes with higher MRRS support is considered rare. > > > > + */ > > > > + if (pci_match_id(bridge_devids, bridge)) { > > > > + if (pcie_get_readrq(pdev) > 256) { > > > > + pci_info(pdev, "limiting MRRS to 256\n"); > > > > + pcie_set_readrq(pdev, 256); > > > > + } > > > > + break; > > > > + } > > > > + } > > > > +} > > > > +DECLARE_PCI_FIXUP_ENABLE(PCI_ANY_ID, PCI_ANY_ID, loongson_old_mrrs_quirk); > > > > +#endif > > > > + > > > > static void loongson_pci_pin_quirk(struct pci_dev *pdev) > > > > { > > > > pdev->pin = 1 + (PCI_FUNC(pdev->devfn) & 3); >
在 2023/9/7 9:18, Manivannan Sadhasivam 写道: [...] > Why do you need to walk through every single device instead of just bridges? > I'm not the maintainer, but my suggestion is to go for Huacai Chen's solution. Hi Mani, Thanks for your reply, unfortunately Huacai's solution is impractical in this case. The problem we have, is firmware (or BIOS) setting improper MRRS for devices attached under those bridges. So we have to fix up MRRS for every single device. We can't iterate child device in bridge quirk because there is no guarantee that bridge will be probed before it's child device, partly due to hotplug. This quirk has been in tree for a while, until Huacai refactored it and broke some systems in 8b3517f88ff2 ("PCI: loongson: Prevent LS7A MRRS increases"). Also to note that ks_pcie_quirk in drivers/pci/controller/dwc/pci-keystone.c uses similar approach. > This avoids iterating over bridges/devices two times. > > Also, please rename firmware to BIOS, as firmware commonly represents the > software running on PCIe endpoint devices. Ack, will fix in next reversion. Thanks - Jiaxun > > - Mani [...]
On Thu, Sep 07, 2023 at 11:13:00AM +0800, Jiaxun Yang wrote: > > > 在 2023/9/7 9:18, Manivannan Sadhasivam 写道: > [...] > > Why do you need to walk through every single device instead of just bridges? > > I'm not the maintainer, but my suggestion is to go for Huacai Chen's solution. > Hi Mani, > > Thanks for your reply, unfortunately Huacai's solution is impractical in > this case. > > The problem we have, is firmware (or BIOS) setting improper MRRS for devices > attached under those bridges. So we have to fix up MRRS for every single > device. > We can't iterate child device in bridge quirk because there is no guarantee > that > bridge will be probed before it's child device, partly due to hotplug. > Okay, this clarifies and also warrants improvement in commit message. You could also use pci_walk_bus() after pci_host_probe() to iterate over the child devices under root bridge and set MRRS. IMO that would look neat. - Mani > This quirk has been in tree for a while, until Huacai refactored it and > broke some > systems in 8b3517f88ff2 ("PCI: loongson: Prevent LS7A MRRS increases"). > > Also to note that ks_pcie_quirk in drivers/pci/controller/dwc/pci-keystone.c > uses similar approach. > > This avoids iterating over bridges/devices two times. > > > > Also, please rename firmware to BIOS, as firmware commonly represents the > > software running on PCIe endpoint devices. > Ack, will fix in next reversion. > > Thanks > - Jiaxun > > > > - Mani > [...]
[CCing the regression list, as it should be in the loop for regressions: https://docs.kernel.org/admin-guide/reporting-regressions.html] On 07.09.23 07:08, Manivannan Sadhasivam wrote: > On Thu, Sep 07, 2023 at 11:13:00AM +0800, Jiaxun Yang wrote: >> 在 2023/9/7 9:18, Manivannan Sadhasivam 写道: >> [...] >>> Why do you need to walk through every single device instead of just bridges? >>> I'm not the maintainer, but my suggestion is to go for Huacai Chen's solution. >> >> Thanks for your reply, unfortunately Huacai's solution is impractical in >> this case. >> >> The problem we have, is firmware (or BIOS) setting improper MRRS for devices >> attached under those bridges. So we have to fix up MRRS for every single >> device. >> We can't iterate child device in bridge quirk because there is no guarantee >> that >> bridge will be probed before it's child device, partly due to hotplug. > > Okay, this clarifies and also warrants improvement in commit message. > > You could also use pci_walk_bus() after pci_host_probe() to iterate over the > child devices under root bridge and set MRRS. IMO that would look neat. Hi, Thorsten here, the Linux kernel's regression tracker. What's the status here? The regression that was supposed to be fixed by the patched that started this thread was reported 9 weeks ago[1] and the culprit made it to many stable kernels as well. Would be really good to finally fix this, as a regression like this should ideally be fixed within 2 to 3 weeks (in both mainline and stable). With a revert if necessary -- is this maybe still a option, or would that cause more trouble then it solved (I guess that's the case). [1] https://bugzilla.kernel.org/show_bug.cgi?id=217680 Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) -- Everything you wanna know about Linux kernel regression tracking: https://linux-regtracking.leemhuis.info/about/#tldr If I did something stupid, please tell me, as explained on that page. >> This quirk has been in tree for a while, until Huacai refactored it and >> broke some >> systems in 8b3517f88ff2 ("PCI: loongson: Prevent LS7A MRRS increases"). >> >> Also to note that ks_pcie_quirk in drivers/pci/controller/dwc/pci-keystone.c >> uses similar approach. >>> This avoids iterating over bridges/devices two times. >>> >>> Also, please rename firmware to BIOS, as firmware commonly represents the >>> software running on PCIe endpoint devices. >> Ack, will fix in next reversion. >> >> Thanks >> - Jiaxun >>> >>> - Mani >> [...] >
Hi, Thorsten here, the Linux kernel's regression tracker. Top-posting for once, to make this easily accessible to everyone. Just wondering what's up here. The patch this thread is about afaics was supposed to fix a regression reported in July (https://bugzilla.kernel.org/show_bug.cgi?id=217680 ), but has made not steps closer to get mainlined during the past few weeks. Is there a reason, or did it maybe fell through the cracks? Jiaxun Yang, from it quick look it seems like you wanted to post a v3, but never did so; but I might be mistaken there. Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) -- Everything you wanna know about Linux kernel regression tracking: https://linux-regtracking.leemhuis.info/about/#tldr If I did something stupid, please tell me, as explained on that page. #regzbot poke On 20.09.23 14:33, Linux regression tracking (Thorsten Leemhuis) wrote: > [CCing the regression list, as it should be in the loop for regressions: > https://docs.kernel.org/admin-guide/reporting-regressions.html] > > On 07.09.23 07:08, Manivannan Sadhasivam wrote: >> On Thu, Sep 07, 2023 at 11:13:00AM +0800, Jiaxun Yang wrote: >>> 在 2023/9/7 9:18, Manivannan Sadhasivam 写道: >>> [...] >>>> Why do you need to walk through every single device instead of just bridges? >>>> I'm not the maintainer, but my suggestion is to go for Huacai Chen's solution. >>> >>> Thanks for your reply, unfortunately Huacai's solution is impractical in >>> this case. >>> >>> The problem we have, is firmware (or BIOS) setting improper MRRS for devices >>> attached under those bridges. So we have to fix up MRRS for every single >>> device. >>> We can't iterate child device in bridge quirk because there is no guarantee >>> that >>> bridge will be probed before it's child device, partly due to hotplug. >> >> Okay, this clarifies and also warrants improvement in commit message. >> >> You could also use pci_walk_bus() after pci_host_probe() to iterate over the >> child devices under root bridge and set MRRS. IMO that would look neat. > > Hi, Thorsten here, the Linux kernel's regression tracker. What's the > status here? The regression that was supposed to be fixed by the patched > that started this thread was reported 9 weeks ago[1] and the culprit > made it to many stable kernels as well. Would be really good to finally > fix this, as a regression like this should ideally be fixed within 2 to > 3 weeks (in both mainline and stable). With a revert if necessary -- is > this maybe still a option, or would that cause more trouble then it > solved (I guess that's the case). > > [1] https://bugzilla.kernel.org/show_bug.cgi?id=217680 > > Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) > -- > Everything you wanna know about Linux kernel regression tracking: > https://linux-regtracking.leemhuis.info/about/#tldr > If I did something stupid, please tell me, as explained on that page. > >>> This quirk has been in tree for a while, until Huacai refactored it and >>> broke some >>> systems in 8b3517f88ff2 ("PCI: loongson: Prevent LS7A MRRS increases"). >>> >>> Also to note that ks_pcie_quirk in drivers/pci/controller/dwc/pci-keystone.c >>> uses similar approach. >>>> This avoids iterating over bridges/devices two times. >>>> >>>> Also, please rename firmware to BIOS, as firmware commonly represents the >>>> software running on PCIe endpoint devices. >>> Ack, will fix in next reversion. >>> >>> Thanks >>> - Jiaxun >>>> >>>> - Mani >>> [...] >>
在2023年10月20日十月 下午12:04，Thorsten Leemhuis写道： > Hi, Thorsten here, the Linux kernel's regression tracker. Top-posting > for once, to make this easily accessible to everyone. > > Just wondering what's up here. The patch this thread is about afaics was > supposed to fix a regression reported in July > (https://bugzilla.kernel.org/show_bug.cgi?id=217680 ), but has made not > steps closer to get mainlined during the past few weeks. Is there a > reason, or did it maybe fell through the cracks? > > Jiaxun Yang, from it quick look it seems like you wanted to post a v3, > but never did so; but I might be mistaken there. I just figure out the problem of this implementation. Will resend and explain. Thanks - Jiaxun > > Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) > -- > Everything you wanna know about Linux kernel regression tracking: > https://linux-regtracking.leemhuis.info/about/#tldr > If I did something stupid, please tell me, as explained on that page. > > #regzbot poke > > On 20.09.23 14:33, Linux regression tracking (Thorsten Leemhuis) wrote: >> [CCing the regression list, as it should be in the loop for regressions: >> https://docs.kernel.org/admin-guide/reporting-regressions.html] >> >> On 07.09.23 07:08, Manivannan Sadhasivam wrote: >>> On Thu, Sep 07, 2023 at 11:13:00AM +0800, Jiaxun Yang wrote: >>>> 在 2023/9/7 9:18, Manivannan Sadhasivam 写道: >>>> [...] >>>>> Why do you need to walk through every single device instead of just bridges? >>>>> I'm not the maintainer, but my suggestion is to go for Huacai Chen's solution. >>>> >>>> Thanks for your reply, unfortunately Huacai's solution is impractical in >>>> this case. >>>> >>>> The problem we have, is firmware (or BIOS) setting improper MRRS for devices >>>> attached under those bridges. So we have to fix up MRRS for every single >>>> device. >>>> We can't iterate child device in bridge quirk because there is no guarantee >>>> that >>>> bridge will be probed before it's child device, partly due to hotplug. >>> >>> Okay, this clarifies and also warrants improvement in commit message. >>> >>> You could also use pci_walk_bus() after pci_host_probe() to iterate over the >>> child devices under root bridge and set MRRS. IMO that would look neat. >> >> Hi, Thorsten here, the Linux kernel's regression tracker. What's the >> status here? The regression that was supposed to be fixed by the patched >> that started this thread was reported 9 weeks ago[1] and the culprit >> made it to many stable kernels as well. Would be really good to finally >> fix this, as a regression like this should ideally be fixed within 2 to >> 3 weeks (in both mainline and stable). With a revert if necessary -- is >> this maybe still a option, or would that cause more trouble then it >> solved (I guess that's the case). >> >> [1] https://bugzilla.kernel.org/show_bug.cgi?id=217680 >> >> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) >> -- >> Everything you wanna know about Linux kernel regression tracking: >> https://linux-regtracking.leemhuis.info/about/#tldr >> If I did something stupid, please tell me, as explained on that page. >> >>>> This quirk has been in tree for a while, until Huacai refactored it and >>>> broke some >>>> systems in 8b3517f88ff2 ("PCI: loongson: Prevent LS7A MRRS increases"). >>>> >>>> Also to note that ks_pcie_quirk in drivers/pci/controller/dwc/pci-keystone.c >>>> uses similar approach. >>>>> This avoids iterating over bridges/devices two times. >>>>> >>>>> Also, please rename firmware to BIOS, as firmware commonly represents the >>>>> software running on PCIe endpoint devices. >>>> Ack, will fix in next reversion. >>>> >>>> Thanks >>>> - Jiaxun >>>>> >>>>> - Mani >>>> [...] >>>
在2023年9月7日九月 上午6:08，Manivannan Sadhasivam写道： > On Thu, Sep 07, 2023 at 11:13:00AM +0800, Jiaxun Yang wrote: >> >> >> 在 2023/9/7 9:18, Manivannan Sadhasivam 写道: >> [...] >> > Why do you need to walk through every single device instead of just bridges? >> > I'm not the maintainer, but my suggestion is to go for Huacai Chen's solution. >> Hi Mani, >> >> Thanks for your reply, unfortunately Huacai's solution is impractical in >> this case. >> >> The problem we have, is firmware (or BIOS) setting improper MRRS for devices >> attached under those bridges. So we have to fix up MRRS for every single >> device. >> We can't iterate child device in bridge quirk because there is no guarantee >> that >> bridge will be probed before it's child device, partly due to hotplug. >> > > Okay, this clarifies and also warrants improvement in commit message. > > You could also use pci_walk_bus() after pci_host_probe() to iterate over the > child devices under root bridge and set MRRS. IMO that would look neat. This won't work as disabling device will reset MRRS and you have to set it again, while pci_host_probe will only run at init. So quirk at device enablement is still the best solution. Thanks - Jiaxun > > - Mani > >> This quirk has been in tree for a while, until Huacai refactored it and >> broke some >> systems in 8b3517f88ff2 ("PCI: loongson: Prevent LS7A MRRS increases"). >> >> Also to note that ks_pcie_quirk in drivers/pci/controller/dwc/pci-keystone.c >> uses similar approach. >> > This avoids iterating over bridges/devices two times. >> > >> > Also, please rename firmware to BIOS, as firmware commonly represents the >> > software running on PCIe endpoint devices. >> Ack, will fix in next reversion. >> >> Thanks >> - Jiaxun >> > >> > - Mani >> [...] > > -- > மணிவண்ணன் சதாசிவம்
diff --git a/drivers/pci/controller/pci-loongson.c b/drivers/pci/controller/pci-loongson.c index fe0f732f6e43..d0f68b102d10 100644 --- a/drivers/pci/controller/pci-loongson.c +++ b/drivers/pci/controller/pci-loongson.c @@ -108,6 +108,44 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON, DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON, DEV_LS7A_PCIE_PORT6, loongson_mrrs_quirk); +#ifdef CONFIG_MIPS +static void loongson_old_mrrs_quirk(struct pci_dev *pdev) +{ + struct pci_bus *bus = pdev->bus; + struct pci_dev *bridge; + static const struct pci_device_id bridge_devids[] = { + { PCI_VDEVICE(LOONGSON, DEV_LS2K_PCIE_PORT0) }, + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT0) }, + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT1) }, + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT2) }, + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT3) }, + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT4) }, + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT5) }, + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT6) }, + { 0, }, + }; + + /* look for the matching bridge */ + while (!pci_is_root_bus(bus)) { + bridge = bus->self; + bus = bus->parent; + /* + * There are still some wild MIPS Loongson firmware won't + * set MRRS properly. Limiting MRRS to 256 as MIPS Loongson + * comes with higher MRRS support is considered rare. + */ + if (pci_match_id(bridge_devids, bridge)) { + if (pcie_get_readrq(pdev) > 256) { + pci_info(pdev, "limiting MRRS to 256\n"); + pcie_set_readrq(pdev, 256); + } + break; + } + } +} +DECLARE_PCI_FIXUP_ENABLE(PCI_ANY_ID, PCI_ANY_ID, loongson_old_mrrs_quirk); +#endif + static void loongson_pci_pin_quirk(struct pci_dev *pdev) { pdev->pin = 1 + (PCI_FUNC(pdev->devfn) & 3);
This is a partial revert of commit 8b3517f88ff2 ("PCI: loongson: Prevent LS7A MRRS increases") for MIPS based Loongson. There are many MIPS based Loongson systems in wild that shipped with firmware which does not set maximum MRRS properly. Limiting MRRS to 256 for all as MIPS Loongson comes with higher MRRS support is considered rare. Cc: stable@vger.kernel.org Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217680 Fixes: 8b3517f88ff2 ("PCI: loongson: Prevent LS7A MRRS increases") Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com> --- v2: Rename quirk name to: loongson_old_mrrs_quirk --- drivers/pci/controller/pci-loongson.c | 38 +++++++++++++++++++++++++++ 1 file changed, 38 insertions(+)