| Message ID | d4113e25-fb4e-2d8a-be08-8f05b48d4413@linux.vnet.ibm.com |
|---|---|
| State | Changes Requested |
| Headers | show |
Vasant Hegde <hegdevasant@linux.vnet.ibm.com> writes: > On 04/03/2017 07:27 AM, Pridhiviraj Paidipeddi wrote: >> Currently fsp-rtc reads/writes the cached RTC TOD on an fsp >> reset. Use latest fsp_in_rr() function to properly read the cached rtc >> value when fsp reset initiated by the hir. >> > > .../... > > Looks like we are not triggering fsp_start_rr() for host initiated FSP R/R. > While this patch works for > RTC we may hit issues in some other place. > > Can you try with below patch? > > diff --git a/hw/fsp/fsp.c b/hw/fsp/fsp.c > index a0c5a78..7321e63 100644 > --- a/hw/fsp/fsp.c > +++ b/hw/fsp/fsp.c > @@ -1571,6 +1571,7 @@ static void __fsp_poll(bool interrupt) > /* Handle host initiated resets */ > if (fsp_in_hir(fsp)) { > fsp_hir_poll(fsp, iop->psi); > + fsp_start_rr(fsp); > return; > } So, it looks like I pretty successfully ignored non-POWER9 OpenPOWER things for err... way too long and really should have merged this in by now. The patch no longer cleanly applies just because things have moved, but it looks like it should be easy to rebase, and with the acked-by's and some testing, we should be good to bring it in. Are you able to rebase and post?
On 07/23/2018 02:52 PM, Stewart Smith wrote: > Vasant Hegde <hegdevasant@linux.vnet.ibm.com> writes: >> On 04/03/2017 07:27 AM, Pridhiviraj Paidipeddi wrote: >>> Currently fsp-rtc reads/writes the cached RTC TOD on an fsp >>> reset. Use latest fsp_in_rr() function to properly read the cached rtc >>> value when fsp reset initiated by the hir. >>> >> >> .../... >> >> Looks like we are not triggering fsp_start_rr() for host initiated FSP R/R. >> While this patch works for >> RTC we may hit issues in some other place. >> >> Can you try with below patch? >> >> diff --git a/hw/fsp/fsp.c b/hw/fsp/fsp.c >> index a0c5a78..7321e63 100644 >> --- a/hw/fsp/fsp.c >> +++ b/hw/fsp/fsp.c >> @@ -1571,6 +1571,7 @@ static void __fsp_poll(bool interrupt) >> /* Handle host initiated resets */ >> if (fsp_in_hir(fsp)) { >> fsp_hir_poll(fsp, iop->psi); >> + fsp_start_rr(fsp); >> return; >> } > > So, it looks like I pretty successfully ignored non-POWER9 OpenPOWER > things for err... way too long and really should have merged this in by > now. > > The patch no longer cleanly applies just because things have moved, but > it looks like it should be easy to rebase, and with the acked-by's and > some testing, we should be good to bring it in. > > Are you able to rebase and post? Stewart, We did merge original Pridhiviraj patch to master (commit : 447ccc4d). Also we added subsequent cleanup patch (commit : a3436963 ). Am I missing something? -Vasant
Vasant Hegde <hegdevasant@linux.vnet.ibm.com> writes: > On 07/23/2018 02:52 PM, Stewart Smith wrote: >> Vasant Hegde <hegdevasant@linux.vnet.ibm.com> writes: >>> On 04/03/2017 07:27 AM, Pridhiviraj Paidipeddi wrote: >>>> Currently fsp-rtc reads/writes the cached RTC TOD on an fsp >>>> reset. Use latest fsp_in_rr() function to properly read the cached rtc >>>> value when fsp reset initiated by the hir. >>>> >>> >>> .../... >>> >>> Looks like we are not triggering fsp_start_rr() for host initiated FSP R/R. >>> While this patch works for >>> RTC we may hit issues in some other place. >>> >>> Can you try with below patch? >>> >>> diff --git a/hw/fsp/fsp.c b/hw/fsp/fsp.c >>> index a0c5a78..7321e63 100644 >>> --- a/hw/fsp/fsp.c >>> +++ b/hw/fsp/fsp.c >>> @@ -1571,6 +1571,7 @@ static void __fsp_poll(bool interrupt) >>> /* Handle host initiated resets */ >>> if (fsp_in_hir(fsp)) { >>> fsp_hir_poll(fsp, iop->psi); >>> + fsp_start_rr(fsp); >>> return; >>> } >> >> So, it looks like I pretty successfully ignored non-POWER9 OpenPOWER >> things for err... way too long and really should have merged this in by >> now. >> >> The patch no longer cleanly applies just because things have moved, but >> it looks like it should be easy to rebase, and with the acked-by's and >> some testing, we should be good to bring it in. >> >> Are you able to rebase and post? > > Stewart, > > We did merge original Pridhiviraj patch to master (commit : 447ccc4d). > Also we added subsequent cleanup patch (commit : a3436963 ). > > Am I missing something? I was wondering about the fsp_start_rr() call in that if(fsp_in_hir) block... I didn't see it when I looked briefly at current skiboot and was wondering if we still need it.
On 07/30/2018 08:44 AM, Stewart Smith wrote: > Vasant Hegde <hegdevasant@linux.vnet.ibm.com> writes: >> On 07/23/2018 02:52 PM, Stewart Smith wrote: >>> Vasant Hegde <hegdevasant@linux.vnet.ibm.com> writes: >>>> On 04/03/2017 07:27 AM, Pridhiviraj Paidipeddi wrote: >>>>> Currently fsp-rtc reads/writes the cached RTC TOD on an fsp >>>>> reset. Use latest fsp_in_rr() function to properly read the cached rtc >>>>> value when fsp reset initiated by the hir. >>>>> >>>> >>>> .../... >>>> >>>> Looks like we are not triggering fsp_start_rr() for host initiated FSP R/R. >>>> While this patch works for >>>> RTC we may hit issues in some other place. >>>> >>>> Can you try with below patch? >>>> >>>> diff --git a/hw/fsp/fsp.c b/hw/fsp/fsp.c >>>> index a0c5a78..7321e63 100644 >>>> --- a/hw/fsp/fsp.c >>>> +++ b/hw/fsp/fsp.c >>>> @@ -1571,6 +1571,7 @@ static void __fsp_poll(bool interrupt) >>>> /* Handle host initiated resets */ >>>> if (fsp_in_hir(fsp)) { >>>> fsp_hir_poll(fsp, iop->psi); >>>> + fsp_start_rr(fsp); >>>> return; >>>> } >>> >>> So, it looks like I pretty successfully ignored non-POWER9 OpenPOWER >>> things for err... way too long and really should have merged this in by >>> now. >>> >>> The patch no longer cleanly applies just because things have moved, but >>> it looks like it should be easy to rebase, and with the acked-by's and >>> some testing, we should be good to bring it in. >>> >>> Are you able to rebase and post? >> >> Stewart, >> >> We did merge original Pridhiviraj patch to master (commit : 447ccc4d). >> Also we added subsequent cleanup patch (commit : a3436963 ). >> >> Am I missing something? > > I was wondering about the fsp_start_rr() call in that if(fsp_in_hir) > block... I didn't see it when I looked briefly at current skiboot and > was wondering if we still need it. > Ah! sorry. I overlooked your previous questions. I did spent sometime today to track down Host initiated reset path. We don't need above fix. It gets called in below path __fsp_poll() -> fsp_handle_errors() -> fsp_start_rr() [ 437.497488637,3] SURV: Injected HIR, initiating FSP R/R [ 437.497501247,3] FSP: fsp_trigger_reset() entry [ 437.497809028,5] PSI: SEMR set to 10000100000 [ 437.497810789,5] PSI[0x000]: Disabling link! [ 437.497811715,5] PSI: PSIHB_CR (error bits) set to 48f0d10040000000 [ 437.497812821,5] PSI: starting link polling [ 437.497813554,5] FSP #0: FSP in reset. Giving up PSI link [ 437.497814370,3] FSP R/R called in HIR path [ 437.497816550,7] FSPCON: Closed consoles due to FSP reset/reload [ 437.497817898,5] SURV: Disabling surveillance [ 437.497819182,5] FSP: Closing NVRAM on account of FSP Reset [ 437.497820137,7] FSP #0: HDES stat change = 0xffffffff [ 440.026507384,5] PSI[0x008]: Poll CR=0x00f0110000000000 [ 440.026508681,5] PSI[0x000]: Poll CR=0x48f0d10040000000 -Vasant
diff --git a/hw/fsp/fsp.c b/hw/fsp/fsp.c index a0c5a78..7321e63 100644 --- a/hw/fsp/fsp.c +++ b/hw/fsp/fsp.c @@ -1571,6 +1571,7 @@ static void __fsp_poll(bool interrupt) /* Handle host initiated resets */ if (fsp_in_hir(fsp)) { fsp_hir_poll(fsp, iop->psi); + fsp_start_rr(fsp); return; }