| Message ID | 20201030022230.21949-1-Lulu_Su@wistron.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/2] mowgli: Limit slot1 to Gen3 by default | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch master (233ade2f6ffdfa406100276784eb447d062fe8e7) |
| snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot | success | Test snowpatch/job/snowpatch-skiboot on branch master |
| snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot-dco | success | Signed-off-by present |
On 30/10/2020 03:22, Lulu Su wrote: > From: LuluTHSu<Lulu_Su@wistron.com> > > Refer to the spec. of mowgli, limit the slot to Gen3 speed. > For mowgli platform spec. > > Cc:skiboot-stable@lists.ozlabs.org > Signed-off-by: LuluTHSu<Lulu_Su@wistron.com> > --- > hw/phb4.c | 21 +++++++++++++++++++++ > include/phb4.h | 1 + > platforms/astbmc/mowgli.c | 16 ++++++++++++++++ > 3 files changed, 38 insertions(+) > > diff --git a/hw/phb4.c b/hw/phb4.c > index 17a233f..de10bb0 100644 > --- a/hw/phb4.c > +++ b/hw/phb4.c > @@ -2991,6 +2991,27 @@ static unsigned int phb4_get_max_link_speed(struct phb4 *p, struct dt_node *np) > return max_link_speed; > } > > +/* > + * Has the same effect as the ibm,max-link-speed property. > + * i.e. sets the default link speed, while allowing NVRAM > + * overrides, etc to still take effect. > + */ > +void phb4_set_dt_max_link_speed(struct phb4 *p, int new_max) > +{ > + uint64_t scr; > + int max; > + > + /* take into account nvram settings, etc */ > + if (pcie_max_link_speed) > + max = pcie_max_link_speed; > + else > + max = new_max; > + > + scr = phb4_read_reg(p, PHB_PCIE_SCR); > + scr = SETFIELD(PHB_PCIE_SCR_MAXLINKSPEED, scr, max); > + phb4_write_reg(p, PHB_PCIE_SCR, scr); > +} > + This patch has already been merged, but I think there's a problem here. This should work to limit the link speed at boot, when the link is trained for the first time. However, since we're writing directly in the register, skiboot won't memorize the limit and on the next link reset/retrain (due to a EEH recovery or PCI hotplug for example), then that is lost. And mowgli_setup_phb() won't be called either on that path. It seems there's an easier way of handling it: phb4_get_max_link_speed() will honor any limit found in the "ibm,max-link-speed" property of the PHB in the device tree. So it should be enough to just set/add that property for PHB 0. I'm not sure what control you have on the device tree generation on that platform, but worse case, you could add a fixup in mowgli.c, for example when probe() is called. Fred
On 11/9/20 10:06 PM, Frederic Barrat wrote: > > > On 30/10/2020 03:22, Lulu Su wrote: >> From: LuluTHSu<Lulu_Su@wistron.com> >> >> Refer to the spec. of mowgli, limit the slot to Gen3 speed. >> For mowgli platform spec. >> >> Cc:skiboot-stable@lists.ozlabs.org >> Signed-off-by: LuluTHSu<Lulu_Su@wistron.com> >> --- >> hw/phb4.c | 21 +++++++++++++++++++++ >> include/phb4.h | 1 + >> platforms/astbmc/mowgli.c | 16 ++++++++++++++++ >> 3 files changed, 38 insertions(+) >> >> diff --git a/hw/phb4.c b/hw/phb4.c >> index 17a233f..de10bb0 100644 >> --- a/hw/phb4.c >> +++ b/hw/phb4.c >> @@ -2991,6 +2991,27 @@ static unsigned int phb4_get_max_link_speed(struct phb4 >> *p, struct dt_node *np) >> return max_link_speed; >> } >> +/* >> + * Has the same effect as the ibm,max-link-speed property. >> + * i.e. sets the default link speed, while allowing NVRAM >> + * overrides, etc to still take effect. >> + */ >> +void phb4_set_dt_max_link_speed(struct phb4 *p, int new_max) >> +{ >> + uint64_t scr; >> + int max; >> + >> + /* take into account nvram settings, etc */ >> + if (pcie_max_link_speed) >> + max = pcie_max_link_speed; >> + else >> + max = new_max; >> + >> + scr = phb4_read_reg(p, PHB_PCIE_SCR); >> + scr = SETFIELD(PHB_PCIE_SCR_MAXLINKSPEED, scr, max); >> + phb4_write_reg(p, PHB_PCIE_SCR, scr); >> +} >> + > > > This patch has already been merged, but I think there's a problem here. This > should work to limit the link speed at boot, when the link is trained for the > first time. However, since we're writing directly in the register, skiboot won't > memorize the limit and on the next link reset/retrain (due to a EEH recovery or > PCI hotplug for example), then that is lost. And mowgli_setup_phb() won't be > called either on that path. Lulu, Can you guys fix this issue? -Vasant
Hi Vasant, Thank you for your reminder. I have a question for your opinion: Finally, there is a branch used by mowgli in ibm-op-release/op-build: release-mowgli, Maybe I can refer to the previous approach https://github.com/ibm-op-release/op-build/blob/release-mowgli/openpower/configs/mihawk_defconfig#L3 , build a dedicated mowgli-patches, and add the modification of phb4_get_max_link_speed(), set max_link_speed to GEN3. What do you think? Hi Frederic, Thanks for your advice, I tried to make some modifications (attached files) and verified through EEH, the max-link-speed of this modification will not be lost, Do you have any suggestions for this modification? But as you mentioned, there is an easier way to achieve this. I would like to ask how to set/add this property (ibm, max-link-speed) for PHB 0? If there is any problem, please feel free to contact me. Thank you very much. Best regards, Lulu Su -----Original Message----- From: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> Sent: Wednesday, December 2, 2020 3:04 PM To: Frederic Barrat <fbarrat@linux.ibm.com>; Lulu Su/WHQ/Wistron <Lulu_Su@wistron.com>; skiboot@lists.ozlabs.org; Stewart Smith <stewart@linux.vnet.ibm.com> Cc: Oliver OHalloran <oliveroh@au1.ibm.com>; skiboot-stable@lists.ozlabs.org; Klaus Heinrich Kiwi <klausk@br.ibm.com> Subject: Re: [Skiboot] [PATCH 1/2] mowgli: Limit slot1 to Gen3 by default On 11/9/20 10:06 PM, Frederic Barrat wrote: > > > On 30/10/2020 03:22, Lulu Su wrote: >> From: LuluTHSu<Lulu_Su@wistron.com> >> >> Refer to the spec. of mowgli, limit the slot to Gen3 speed. >> For mowgli platform spec. >> >> Cc:skiboot-stable@lists.ozlabs.org >> Signed-off-by: LuluTHSu<Lulu_Su@wistron.com> >> --- >> hw/phb4.c | 21 +++++++++++++++++++++ >> include/phb4.h | 1 + >> platforms/astbmc/mowgli.c | 16 ++++++++++++++++ >> 3 files changed, 38 insertions(+) >> >> diff --git a/hw/phb4.c b/hw/phb4.c >> index 17a233f..de10bb0 100644 >> --- a/hw/phb4.c >> +++ b/hw/phb4.c >> @@ -2991,6 +2991,27 @@ static unsigned int >> phb4_get_max_link_speed(struct phb4 *p, struct dt_node *np) >> return max_link_speed; >> } >> +/* >> + * Has the same effect as the ibm,max-link-speed property. >> + * i.e. sets the default link speed, while allowing NVRAM >> + * overrides, etc to still take effect. >> + */ >> +void phb4_set_dt_max_link_speed(struct phb4 *p, int new_max) { >> + uint64_t scr; >> + int max; >> + >> + /* take into account nvram settings, etc */ >> + if (pcie_max_link_speed) >> + max = pcie_max_link_speed; >> + else >> + max = new_max; >> + >> + scr = phb4_read_reg(p, PHB_PCIE_SCR); >> + scr = SETFIELD(PHB_PCIE_SCR_MAXLINKSPEED, scr, max); >> + phb4_write_reg(p, PHB_PCIE_SCR, scr); } >> + > > > This patch has already been merged, but I think there's a problem > here. This should work to limit the link speed at boot, when the link > is trained for the first time. However, since we're writing directly > in the register, skiboot won't memorize the limit and on the next link > reset/retrain (due to a EEH recovery or PCI hotplug for example), then > that is lost. And mowgli_setup_phb() won't be called either on that path. Lulu, Can you guys fix this issue? -Vasant
Hi, Attached is a patch for you to try. You will also need to revert the previous patch (5262cdd1b99f77bca5951fc8132f9795ef0c2b87), since we no longer need the phb4_set_dt_max_link_speed() function. Fred On 08/12/2020 12:50, Lulu_Su@wistron.com wrote: > Hi Vasant, > > Thank you for your reminder. I have a question for your opinion: > Finally, there is a branch used by mowgli in ibm-op-release/op-build: release-mowgli, > Maybe I can refer to the previous approach https://github.com/ibm-op-release/op-build/blob/release-mowgli/openpower/configs/mihawk_defconfig#L3 , > build a dedicated mowgli-patches, and add the modification of phb4_get_max_link_speed(), set max_link_speed to GEN3. > What do you think? > > > > Hi Frederic, > > Thanks for your advice, > I tried to make some modifications (attached files) and verified through EEH, the max-link-speed of this modification will not be lost, > Do you have any suggestions for this modification? > > But as you mentioned, there is an easier way to achieve this. > I would like to ask how to set/add this property (ibm, max-link-speed) for PHB 0? > > If there is any problem, please feel free to contact me. > Thank you very much. > > Best regards, > Lulu Su > > -----Original Message----- > From: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> > Sent: Wednesday, December 2, 2020 3:04 PM > To: Frederic Barrat <fbarrat@linux.ibm.com>; Lulu Su/WHQ/Wistron <Lulu_Su@wistron.com>; skiboot@lists.ozlabs.org; Stewart Smith <stewart@linux.vnet.ibm.com> > Cc: Oliver OHalloran <oliveroh@au1.ibm.com>; skiboot-stable@lists.ozlabs.org; Klaus Heinrich Kiwi <klausk@br.ibm.com> > Subject: Re: [Skiboot] [PATCH 1/2] mowgli: Limit slot1 to Gen3 by default > > On 11/9/20 10:06 PM, Frederic Barrat wrote: >> >> >> On 30/10/2020 03:22, Lulu Su wrote: >>> From: LuluTHSu<Lulu_Su@wistron.com> >>> >>> Refer to the spec. of mowgli, limit the slot to Gen3 speed. >>> For mowgli platform spec. >>> >>> Cc:skiboot-stable@lists.ozlabs.org >>> Signed-off-by: LuluTHSu<Lulu_Su@wistron.com> >>> --- >>> hw/phb4.c | 21 +++++++++++++++++++++ >>> include/phb4.h | 1 + >>> platforms/astbmc/mowgli.c | 16 ++++++++++++++++ >>> 3 files changed, 38 insertions(+) >>> >>> diff --git a/hw/phb4.c b/hw/phb4.c >>> index 17a233f..de10bb0 100644 >>> --- a/hw/phb4.c >>> +++ b/hw/phb4.c >>> @@ -2991,6 +2991,27 @@ static unsigned int >>> phb4_get_max_link_speed(struct phb4 *p, struct dt_node *np) >>> return max_link_speed; >>> } >>> +/* >>> + * Has the same effect as the ibm,max-link-speed property. >>> + * i.e. sets the default link speed, while allowing NVRAM >>> + * overrides, etc to still take effect. >>> + */ >>> +void phb4_set_dt_max_link_speed(struct phb4 *p, int new_max) { >>> + uint64_t scr; >>> + int max; >>> + >>> + /* take into account nvram settings, etc */ >>> + if (pcie_max_link_speed) >>> + max = pcie_max_link_speed; >>> + else >>> + max = new_max; >>> + >>> + scr = phb4_read_reg(p, PHB_PCIE_SCR); >>> + scr = SETFIELD(PHB_PCIE_SCR_MAXLINKSPEED, scr, max); >>> + phb4_write_reg(p, PHB_PCIE_SCR, scr); } >>> + >> >> >> This patch has already been merged, but I think there's a problem >> here. This should work to limit the link speed at boot, when the link >> is trained for the first time. However, since we're writing directly >> in the register, skiboot won't memorize the limit and on the next link >> reset/retrain (due to a EEH recovery or PCI hotplug for example), then >> that is lost. And mowgli_setup_phb() won't be called either on that path. > > > Lulu, > > Can you guys fix this issue? > > -Vasant > > --------------------------------------------------------------------------------------------------------------------------------------------------------------- > This email contains confidential or legally privileged information and is for the sole use of its intended recipient. > Any unauthorized review, use, copying or distribution of this email or the content of this email is strictly prohibited. > If you are not the intended recipient, you may reply to the sender and should delete this e-mail immediately. > --------------------------------------------------------------------------------------------------------------------------------------------------------------- >
Hi Frederic, The attached patch has been modified according to what you mentioned. And use EEH to verify, the result is the same as expected. If you think there is no problem, I will upload this patch to Skiboot. Thank you very much for your methods and suggestions. Lulu -----Original Message----- From: Frederic Barrat <fbarrat@linux.ibm.com> Sent: Wednesday, December 9, 2020 2:33 AM To: Lulu Su/WHQ/Wistron <Lulu_Su@wistron.com>; hegdevasant@linux.vnet.ibm.com; skiboot@lists.ozlabs.org; stewart@linux.vnet.ibm.com Cc: oliveroh@au1.ibm.com; skiboot-stable@lists.ozlabs.org; klausk@br.ibm.com Subject: Re: [Skiboot] [PATCH 1/2] mowgli: Limit slot1 to Gen3 by default Hi, Attached is a patch for you to try. You will also need to revert the previous patch (5262cdd1b99f77bca5951fc8132f9795ef0c2b87), since we no longer need the phb4_set_dt_max_link_speed() function. Fred On 08/12/2020 12:50, Lulu_Su@wistron.com wrote: > Hi Vasant, > > Thank you for your reminder. I have a question for your opinion: > Finally, there is a branch used by mowgli in ibm-op-release/op-build: > release-mowgli, Maybe I can refer to the previous approach > https://github.com/ibm-op-release/op-build/blob/release-mowgli/openpower/configs/mihawk_defconfig#L3 , build a dedicated mowgli-patches, and add the modification of phb4_get_max_link_speed(), set max_link_speed to GEN3. > What do you think? > > > > Hi Frederic, > > Thanks for your advice, > I tried to make some modifications (attached files) and verified > through EEH, the max-link-speed of this modification will not be lost, Do you have any suggestions for this modification? > > But as you mentioned, there is an easier way to achieve this. > I would like to ask how to set/add this property (ibm, max-link-speed) for PHB 0? > > If there is any problem, please feel free to contact me. > Thank you very much. > > Best regards, > Lulu Su > > -----Original Message----- > From: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> > Sent: Wednesday, December 2, 2020 3:04 PM > To: Frederic Barrat <fbarrat@linux.ibm.com>; Lulu Su/WHQ/Wistron > <Lulu_Su@wistron.com>; skiboot@lists.ozlabs.org; Stewart Smith > <stewart@linux.vnet.ibm.com> > Cc: Oliver OHalloran <oliveroh@au1.ibm.com>; > skiboot-stable@lists.ozlabs.org; Klaus Heinrich Kiwi > <klausk@br.ibm.com> > Subject: Re: [Skiboot] [PATCH 1/2] mowgli: Limit slot1 to Gen3 by > default > > On 11/9/20 10:06 PM, Frederic Barrat wrote: >> >> >> On 30/10/2020 03:22, Lulu Su wrote: >>> From: LuluTHSu<Lulu_Su@wistron.com> >>> >>> Refer to the spec. of mowgli, limit the slot to Gen3 speed. >>> For mowgli platform spec. >>> >>> Cc:skiboot-stable@lists.ozlabs.org >>> Signed-off-by: LuluTHSu<Lulu_Su@wistron.com> >>> --- >>> hw/phb4.c | 21 +++++++++++++++++++++ >>> include/phb4.h | 1 + >>> platforms/astbmc/mowgli.c | 16 ++++++++++++++++ >>> 3 files changed, 38 insertions(+) >>> >>> diff --git a/hw/phb4.c b/hw/phb4.c >>> index 17a233f..de10bb0 100644 >>> --- a/hw/phb4.c >>> +++ b/hw/phb4.c >>> @@ -2991,6 +2991,27 @@ static unsigned int >>> phb4_get_max_link_speed(struct phb4 *p, struct dt_node *np) >>> return max_link_speed; >>> } >>> +/* >>> + * Has the same effect as the ibm,max-link-speed property. >>> + * i.e. sets the default link speed, while allowing NVRAM >>> + * overrides, etc to still take effect. >>> + */ >>> +void phb4_set_dt_max_link_speed(struct phb4 *p, int new_max) { >>> + uint64_t scr; >>> + int max; >>> + >>> + /* take into account nvram settings, etc */ >>> + if (pcie_max_link_speed) >>> + max = pcie_max_link_speed; >>> + else >>> + max = new_max; >>> + >>> + scr = phb4_read_reg(p, PHB_PCIE_SCR); >>> + scr = SETFIELD(PHB_PCIE_SCR_MAXLINKSPEED, scr, max); >>> + phb4_write_reg(p, PHB_PCIE_SCR, scr); } >>> + >> >> >> This patch has already been merged, but I think there's a problem >> here. This should work to limit the link speed at boot, when the link >> is trained for the first time. However, since we're writing directly >> in the register, skiboot won't memorize the limit and on the next >> link reset/retrain (due to a EEH recovery or PCI hotplug for >> example), then that is lost. And mowgli_setup_phb() won't be called either on that path. > > > Lulu, > > Can you guys fix this issue? > > -Vasant > > ---------------------------------------------------------------------- > ---------------------------------------------------------------------- > ------------------- This email contains confidential or legally > privileged information and is for the sole use of its intended recipient. > Any unauthorized review, use, copying or distribution of this email or the content of this email is strictly prohibited. > If you are not the intended recipient, you may reply to the sender and should delete this e-mail immediately. > ---------------------------------------------------------------------- > ---------------------------------------------------------------------- > ------------------- >
Hi Lulu, The resulting patch looks good to me. But please submit it as 2 separate patches: patch 1/2 for the revert and patch 2/2 for the code change. It makes it easier to track what is going on. Fred On 09/12/2020 06:26, Lulu_Su@wistron.com wrote: > Hi Frederic, > > The attached patch has been modified according to what you mentioned. > And use EEH to verify, the result is the same as expected. > If you think there is no problem, I will upload this patch to Skiboot. > Thank you very much for your methods and suggestions. > > Lulu > -----Original Message----- > From: Frederic Barrat <fbarrat@linux.ibm.com> > Sent: Wednesday, December 9, 2020 2:33 AM > To: Lulu Su/WHQ/Wistron <Lulu_Su@wistron.com>; hegdevasant@linux.vnet.ibm.com; skiboot@lists.ozlabs.org; stewart@linux.vnet.ibm.com > Cc: oliveroh@au1.ibm.com; skiboot-stable@lists.ozlabs.org; klausk@br.ibm.com > Subject: Re: [Skiboot] [PATCH 1/2] mowgli: Limit slot1 to Gen3 by default > > Hi, > > Attached is a patch for you to try. You will also need to revert the previous patch (5262cdd1b99f77bca5951fc8132f9795ef0c2b87), since we no longer need the phb4_set_dt_max_link_speed() function. > > Fred > > > On 08/12/2020 12:50, Lulu_Su@wistron.com wrote: >> Hi Vasant, >> >> Thank you for your reminder. I have a question for your opinion: >> Finally, there is a branch used by mowgli in ibm-op-release/op-build: >> release-mowgli, Maybe I can refer to the previous approach >> https://github.com/ibm-op-release/op-build/blob/release-mowgli/openpower/configs/mihawk_defconfig#L3 , build a dedicated mowgli-patches, and add the modification of phb4_get_max_link_speed(), set max_link_speed to GEN3. >> What do you think? >> >> >> >> Hi Frederic, >> >> Thanks for your advice, >> I tried to make some modifications (attached files) and verified >> through EEH, the max-link-speed of this modification will not be lost, Do you have any suggestions for this modification? >> >> But as you mentioned, there is an easier way to achieve this. >> I would like to ask how to set/add this property (ibm, max-link-speed) for PHB 0? >> >> If there is any problem, please feel free to contact me. >> Thank you very much. >> >> Best regards, >> Lulu Su >> >> -----Original Message----- >> From: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> >> Sent: Wednesday, December 2, 2020 3:04 PM >> To: Frederic Barrat <fbarrat@linux.ibm.com>; Lulu Su/WHQ/Wistron >> <Lulu_Su@wistron.com>; skiboot@lists.ozlabs.org; Stewart Smith >> <stewart@linux.vnet.ibm.com> >> Cc: Oliver OHalloran <oliveroh@au1.ibm.com>; >> skiboot-stable@lists.ozlabs.org; Klaus Heinrich Kiwi >> <klausk@br.ibm.com> >> Subject: Re: [Skiboot] [PATCH 1/2] mowgli: Limit slot1 to Gen3 by >> default >> >> On 11/9/20 10:06 PM, Frederic Barrat wrote: >>> >>> >>> On 30/10/2020 03:22, Lulu Su wrote: >>>> From: LuluTHSu<Lulu_Su@wistron.com> >>>> >>>> Refer to the spec. of mowgli, limit the slot to Gen3 speed. >>>> For mowgli platform spec. >>>> >>>> Cc:skiboot-stable@lists.ozlabs.org >>>> Signed-off-by: LuluTHSu<Lulu_Su@wistron.com> >>>> --- >>>> hw/phb4.c | 21 +++++++++++++++++++++ >>>> include/phb4.h | 1 + >>>> platforms/astbmc/mowgli.c | 16 ++++++++++++++++ >>>> 3 files changed, 38 insertions(+) >>>> >>>> diff --git a/hw/phb4.c b/hw/phb4.c >>>> index 17a233f..de10bb0 100644 >>>> --- a/hw/phb4.c >>>> +++ b/hw/phb4.c >>>> @@ -2991,6 +2991,27 @@ static unsigned int >>>> phb4_get_max_link_speed(struct phb4 *p, struct dt_node *np) >>>> return max_link_speed; >>>> } >>>> +/* >>>> + * Has the same effect as the ibm,max-link-speed property. >>>> + * i.e. sets the default link speed, while allowing NVRAM >>>> + * overrides, etc to still take effect. >>>> + */ >>>> +void phb4_set_dt_max_link_speed(struct phb4 *p, int new_max) { >>>> + uint64_t scr; >>>> + int max; >>>> + >>>> + /* take into account nvram settings, etc */ >>>> + if (pcie_max_link_speed) >>>> + max = pcie_max_link_speed; >>>> + else >>>> + max = new_max; >>>> + >>>> + scr = phb4_read_reg(p, PHB_PCIE_SCR); >>>> + scr = SETFIELD(PHB_PCIE_SCR_MAXLINKSPEED, scr, max); >>>> + phb4_write_reg(p, PHB_PCIE_SCR, scr); } >>>> + >>> >>> >>> This patch has already been merged, but I think there's a problem >>> here. This should work to limit the link speed at boot, when the link >>> is trained for the first time. However, since we're writing directly >>> in the register, skiboot won't memorize the limit and on the next >>> link reset/retrain (due to a EEH recovery or PCI hotplug for >>> example), then that is lost. And mowgli_setup_phb() won't be called either on that path. >> >> >> Lulu, >> >> Can you guys fix this issue? >> >> -Vasant >> >> ---------------------------------------------------------------------- >> ---------------------------------------------------------------------- >> ------------------- This email contains confidential or legally >> privileged information and is for the sole use of its intended recipient. >> Any unauthorized review, use, copying or distribution of this email or the content of this email is strictly prohibited. >> If you are not the intended recipient, you may reply to the sender and should delete this e-mail immediately. >> ---------------------------------------------------------------------- >> ---------------------------------------------------------------------- >> ------------------- >>
Hi Frederic, Okay, I modified this patch as you said and resubmitted it. Thank you very much. Lulu Su -----Original Message----- From: Frederic Barrat <fbarrat@linux.ibm.com> Sent: Wednesday, December 9, 2020 3:30 PM To: Lulu Su/WHQ/Wistron <Lulu_Su@wistron.com>; hegdevasant@linux.vnet.ibm.com; skiboot@lists.ozlabs.org; stewart@linux.vnet.ibm.com Cc: skiboot-stable@lists.ozlabs.org; klausk@br.ibm.com Subject: Re: [Skiboot] [PATCH 1/2] mowgli: Limit slot1 to Gen3 by default Hi Lulu, The resulting patch looks good to me. But please submit it as 2 separate patches: patch 1/2 for the revert and patch 2/2 for the code change. It makes it easier to track what is going on. Fred On 09/12/2020 06:26, Lulu_Su@wistron.com wrote: > Hi Frederic, > > The attached patch has been modified according to what you mentioned. > And use EEH to verify, the result is the same as expected. > If you think there is no problem, I will upload this patch to Skiboot. > Thank you very much for your methods and suggestions. > > Lulu > -----Original Message----- > From: Frederic Barrat <fbarrat@linux.ibm.com> > Sent: Wednesday, December 9, 2020 2:33 AM > To: Lulu Su/WHQ/Wistron <Lulu_Su@wistron.com>; > hegdevasant@linux.vnet.ibm.com; skiboot@lists.ozlabs.org; > stewart@linux.vnet.ibm.com > Cc: oliveroh@au1.ibm.com; skiboot-stable@lists.ozlabs.org; > klausk@br.ibm.com > Subject: Re: [Skiboot] [PATCH 1/2] mowgli: Limit slot1 to Gen3 by > default > > Hi, > > Attached is a patch for you to try. You will also need to revert the previous patch (5262cdd1b99f77bca5951fc8132f9795ef0c2b87), since we no longer need the phb4_set_dt_max_link_speed() function. > > Fred > > > On 08/12/2020 12:50, Lulu_Su@wistron.com wrote: >> Hi Vasant, >> >> Thank you for your reminder. I have a question for your opinion: >> Finally, there is a branch used by mowgli in ibm-op-release/op-build: >> release-mowgli, Maybe I can refer to the previous approach >> https://github.com/ibm-op-release/op-build/blob/release-mowgli/openpower/configs/mihawk_defconfig#L3 , build a dedicated mowgli-patches, and add the modification of phb4_get_max_link_speed(), set max_link_speed to GEN3. >> What do you think? >> >> >> >> Hi Frederic, >> >> Thanks for your advice, >> I tried to make some modifications (attached files) and verified >> through EEH, the max-link-speed of this modification will not be lost, Do you have any suggestions for this modification? >> >> But as you mentioned, there is an easier way to achieve this. >> I would like to ask how to set/add this property (ibm, max-link-speed) for PHB 0? >> >> If there is any problem, please feel free to contact me. >> Thank you very much. >> >> Best regards, >> Lulu Su >> >> -----Original Message----- >> From: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> >> Sent: Wednesday, December 2, 2020 3:04 PM >> To: Frederic Barrat <fbarrat@linux.ibm.com>; Lulu Su/WHQ/Wistron >> <Lulu_Su@wistron.com>; skiboot@lists.ozlabs.org; Stewart Smith >> <stewart@linux.vnet.ibm.com> >> Cc: Oliver OHalloran <oliveroh@au1.ibm.com>; >> skiboot-stable@lists.ozlabs.org; Klaus Heinrich Kiwi >> <klausk@br.ibm.com> >> Subject: Re: [Skiboot] [PATCH 1/2] mowgli: Limit slot1 to Gen3 by >> default >> >> On 11/9/20 10:06 PM, Frederic Barrat wrote: >>> >>> >>> On 30/10/2020 03:22, Lulu Su wrote: >>>> From: LuluTHSu<Lulu_Su@wistron.com> >>>> >>>> Refer to the spec. of mowgli, limit the slot to Gen3 speed. >>>> For mowgli platform spec. >>>> >>>> Cc:skiboot-stable@lists.ozlabs.org >>>> Signed-off-by: LuluTHSu<Lulu_Su@wistron.com> >>>> --- >>>> hw/phb4.c | 21 +++++++++++++++++++++ >>>> include/phb4.h | 1 + >>>> platforms/astbmc/mowgli.c | 16 ++++++++++++++++ >>>> 3 files changed, 38 insertions(+) >>>> >>>> diff --git a/hw/phb4.c b/hw/phb4.c >>>> index 17a233f..de10bb0 100644 >>>> --- a/hw/phb4.c >>>> +++ b/hw/phb4.c >>>> @@ -2991,6 +2991,27 @@ static unsigned int >>>> phb4_get_max_link_speed(struct phb4 *p, struct dt_node *np) >>>> return max_link_speed; >>>> } >>>> +/* >>>> + * Has the same effect as the ibm,max-link-speed property. >>>> + * i.e. sets the default link speed, while allowing NVRAM >>>> + * overrides, etc to still take effect. >>>> + */ >>>> +void phb4_set_dt_max_link_speed(struct phb4 *p, int new_max) { >>>> + uint64_t scr; >>>> + int max; >>>> + >>>> + /* take into account nvram settings, etc */ >>>> + if (pcie_max_link_speed) >>>> + max = pcie_max_link_speed; >>>> + else >>>> + max = new_max; >>>> + >>>> + scr = phb4_read_reg(p, PHB_PCIE_SCR); >>>> + scr = SETFIELD(PHB_PCIE_SCR_MAXLINKSPEED, scr, max); >>>> + phb4_write_reg(p, PHB_PCIE_SCR, scr); } >>>> + >>> >>> >>> This patch has already been merged, but I think there's a problem >>> here. This should work to limit the link speed at boot, when the >>> link is trained for the first time. However, since we're writing >>> directly in the register, skiboot won't memorize the limit and on >>> the next link reset/retrain (due to a EEH recovery or PCI hotplug >>> for example), then that is lost. And mowgli_setup_phb() won't be called either on that path. >> >> >> Lulu, >> >> Can you guys fix this issue? >> >> -Vasant >> >> --------------------------------------------------------------------- >> - >> --------------------------------------------------------------------- >> - >> ------------------- This email contains confidential or legally >> privileged information and is for the sole use of its intended recipient. >> Any unauthorized review, use, copying or distribution of this email or the content of this email is strictly prohibited. >> If you are not the intended recipient, you may reply to the sender and should delete this e-mail immediately. >> --------------------------------------------------------------------- >> - >> --------------------------------------------------------------------- >> - >> ------------------- >>
diff --git a/hw/phb4.c b/hw/phb4.c index 17a233f..de10bb0 100644 --- a/hw/phb4.c +++ b/hw/phb4.c @@ -2991,6 +2991,27 @@ static unsigned int phb4_get_max_link_speed(struct phb4 *p, struct dt_node *np) return max_link_speed; } +/* + * Has the same effect as the ibm,max-link-speed property. + * i.e. sets the default link speed, while allowing NVRAM + * overrides, etc to still take effect. + */ +void phb4_set_dt_max_link_speed(struct phb4 *p, int new_max) +{ + uint64_t scr; + int max; + + /* take into account nvram settings, etc */ + if (pcie_max_link_speed) + max = pcie_max_link_speed; + else + max = new_max; + + scr = phb4_read_reg(p, PHB_PCIE_SCR); + scr = SETFIELD(PHB_PCIE_SCR_MAXLINKSPEED, scr, max); + phb4_write_reg(p, PHB_PCIE_SCR, scr); +} + static void phb4_assert_perst(struct pci_slot *slot, bool assert) { struct phb4 *p = phb_to_phb4(slot->phb); diff --git a/include/phb4.h b/include/phb4.h index abba2d9..47a46b7 100644 --- a/include/phb4.h +++ b/include/phb4.h @@ -259,4 +259,5 @@ static inline int phb4_get_opal_id(unsigned int chip_id, unsigned int index) void phb4_pec2_dma_engine_realloc(struct phb4 *p); +void phb4_set_dt_max_link_speed(struct phb4 *p, int new_max); #endif /* __PHB4_H */ diff --git a/platforms/astbmc/mowgli.c b/platforms/astbmc/mowgli.c index 9bfe7a4..0246bff 100644 --- a/platforms/astbmc/mowgli.c +++ b/platforms/astbmc/mowgli.c @@ -12,6 +12,8 @@ #include <psi.h> #include <npu-regs.h> #include <secvar.h> +#include <pci.h> +#include <phb4.h> #include "astbmc.h" @@ -71,6 +73,19 @@ static int mowgli_secvar_init(void) return secvar_main(secboot_tpm_driver, edk2_compatible_v1); } +/* + * Limit PHB0/(pec0) to gen3 speeds. + */ +static void mowgli_setup_phb(struct phb *phb, unsigned int __unused index) +{ + struct phb4 *p = phb_to_phb4(phb); + + if (p->pec == 0) { + phb4_set_dt_max_link_speed(p, 3); + prlog(PR_DEBUG, "Mowgli: Force the PHB%d Speed to Gen3.\n", p->pec); + } +} + DECLARE_PLATFORM(mowgli) = { .name = "Mowgli", .probe = mowgli_probe, @@ -80,6 +95,7 @@ DECLARE_PLATFORM(mowgli) = { .bmc = &bmc_plat_ast2500_openbmc, .pci_get_slot_info = slot_table_get_slot_info, .pci_probe_complete = check_all_slot_table, + .pci_setup_phb = mowgli_setup_phb, .cec_power_down = astbmc_ipmi_power_down, .cec_reboot = astbmc_ipmi_reboot, .elog_commit = ipmi_elog_commit,