| Message ID | 20200423091013.11587-8-frankja@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390x: smp: Improve smp code part 2 | expand |
On 23.04.20 11:10, Janosch Frank wrote: > Up to now we ignored the psw mask and only used the psw address when > bringing up a new cpu. For DAT we need to also load the mask, so let's > do that. > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > Reviewed-by: Cornelia Huck <cohuck@redhat.com> > --- > lib/s390x/smp.c | 2 ++ > s390x/cstart64.S | 3 ++- > 2 files changed, 4 insertions(+), 1 deletion(-) > > diff --git a/lib/s390x/smp.c b/lib/s390x/smp.c > index 3f86243..6ef0335 100644 > --- a/lib/s390x/smp.c > +++ b/lib/s390x/smp.c > @@ -202,6 +202,8 @@ int smp_cpu_setup(uint16_t addr, struct psw psw) > cpu->stack = (uint64_t *)alloc_pages(2); > > /* Start without DAT and any other mask bits. */ > + cpu->lowcore->sw_int_psw.mask = psw.mask; > + cpu->lowcore->sw_int_psw.addr = psw.addr; > cpu->lowcore->sw_int_grs[14] = psw.addr; Do we still have to set sw_int_grs[14] ?
On 4/24/20 12:09 PM, David Hildenbrand wrote: > On 23.04.20 11:10, Janosch Frank wrote: >> Up to now we ignored the psw mask and only used the psw address when >> bringing up a new cpu. For DAT we need to also load the mask, so let's >> do that. >> >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >> Reviewed-by: Cornelia Huck <cohuck@redhat.com> >> --- >> lib/s390x/smp.c | 2 ++ >> s390x/cstart64.S | 3 ++- >> 2 files changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/lib/s390x/smp.c b/lib/s390x/smp.c >> index 3f86243..6ef0335 100644 >> --- a/lib/s390x/smp.c >> +++ b/lib/s390x/smp.c >> @@ -202,6 +202,8 @@ int smp_cpu_setup(uint16_t addr, struct psw psw) >> cpu->stack = (uint64_t *)alloc_pages(2); >> >> /* Start without DAT and any other mask bits. */ >> + cpu->lowcore->sw_int_psw.mask = psw.mask; >> + cpu->lowcore->sw_int_psw.addr = psw.addr; >> cpu->lowcore->sw_int_grs[14] = psw.addr; > > Do we still have to set sw_int_grs[14] ? > r14 is saved to restart_new so we don't go through setup twice. We could instead copy cpu->lowcore->sw_int_psw.addr to restart new, but that means more changes than the removal of one line. Also, what about backtraces or plain old debug? Having r14 is a good backup to have IMHO.
On 24.04.20 13:16, Janosch Frank wrote: > On 4/24/20 12:09 PM, David Hildenbrand wrote: >> On 23.04.20 11:10, Janosch Frank wrote: >>> Up to now we ignored the psw mask and only used the psw address when >>> bringing up a new cpu. For DAT we need to also load the mask, so let's >>> do that. >>> >>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >>> Reviewed-by: Cornelia Huck <cohuck@redhat.com> >>> --- >>> lib/s390x/smp.c | 2 ++ >>> s390x/cstart64.S | 3 ++- >>> 2 files changed, 4 insertions(+), 1 deletion(-) >>> >>> diff --git a/lib/s390x/smp.c b/lib/s390x/smp.c >>> index 3f86243..6ef0335 100644 >>> --- a/lib/s390x/smp.c >>> +++ b/lib/s390x/smp.c >>> @@ -202,6 +202,8 @@ int smp_cpu_setup(uint16_t addr, struct psw psw) >>> cpu->stack = (uint64_t *)alloc_pages(2); >>> >>> /* Start without DAT and any other mask bits. */ >>> + cpu->lowcore->sw_int_psw.mask = psw.mask; >>> + cpu->lowcore->sw_int_psw.addr = psw.addr; >>> cpu->lowcore->sw_int_grs[14] = psw.addr; >> >> Do we still have to set sw_int_grs[14] ? >> > r14 is saved to restart_new so we don't go through setup twice. > We could instead copy cpu->lowcore->sw_int_psw.addr to restart new, but > that means more changes than the removal of one line. > > Also, what about backtraces or plain old debug? > Having r14 is a good backup to have IMHO. > Fine with me Reviewed-by: David Hildenbrand <david@redhat.com>
On 4/24/20 1:23 PM, David Hildenbrand wrote: > On 24.04.20 13:16, Janosch Frank wrote: >> On 4/24/20 12:09 PM, David Hildenbrand wrote: >>> On 23.04.20 11:10, Janosch Frank wrote: >>>> Up to now we ignored the psw mask and only used the psw address when >>>> bringing up a new cpu. For DAT we need to also load the mask, so let's >>>> do that. >>>> >>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >>>> Reviewed-by: Cornelia Huck <cohuck@redhat.com> >>>> --- >>>> lib/s390x/smp.c | 2 ++ >>>> s390x/cstart64.S | 3 ++- >>>> 2 files changed, 4 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/lib/s390x/smp.c b/lib/s390x/smp.c >>>> index 3f86243..6ef0335 100644 >>>> --- a/lib/s390x/smp.c >>>> +++ b/lib/s390x/smp.c >>>> @@ -202,6 +202,8 @@ int smp_cpu_setup(uint16_t addr, struct psw psw) >>>> cpu->stack = (uint64_t *)alloc_pages(2); >>>> >>>> /* Start without DAT and any other mask bits. */ >>>> + cpu->lowcore->sw_int_psw.mask = psw.mask; >>>> + cpu->lowcore->sw_int_psw.addr = psw.addr; >>>> cpu->lowcore->sw_int_grs[14] = psw.addr; >>> >>> Do we still have to set sw_int_grs[14] ? >>> >> r14 is saved to restart_new so we don't go through setup twice. >> We could instead copy cpu->lowcore->sw_int_psw.addr to restart new, but >> that means more changes than the removal of one line. >> >> Also, what about backtraces or plain old debug? >> Having r14 is a good backup to have IMHO. >> > > Fine with me > > Reviewed-by: David Hildenbrand <david@redhat.com> > Thanks! Also we need that if the cpu returns from its assigned function, so it drops into the infinite loop.
diff --git a/lib/s390x/smp.c b/lib/s390x/smp.c index 3f86243..6ef0335 100644 --- a/lib/s390x/smp.c +++ b/lib/s390x/smp.c @@ -202,6 +202,8 @@ int smp_cpu_setup(uint16_t addr, struct psw psw) cpu->stack = (uint64_t *)alloc_pages(2); /* Start without DAT and any other mask bits. */ + cpu->lowcore->sw_int_psw.mask = psw.mask; + cpu->lowcore->sw_int_psw.addr = psw.addr; cpu->lowcore->sw_int_grs[14] = psw.addr; cpu->lowcore->sw_int_grs[15] = (uint64_t)cpu->stack + (PAGE_SIZE * 4); lc->restart_new_psw.mask = 0x0000000180000000UL; diff --git a/s390x/cstart64.S b/s390x/cstart64.S index ecffbe0..e084f13 100644 --- a/s390x/cstart64.S +++ b/s390x/cstart64.S @@ -161,7 +161,8 @@ smp_cpu_setup_state: lctlg %c0, %c0, GEN_LC_SW_INT_CRS /* We should only go once through cpu setup and not for every restart */ stg %r14, GEN_LC_RESTART_NEW_PSW + 8 - brasl %r14, %r14 + larl %r14, 0f + lpswe GEN_LC_SW_INT_PSW /* If the function returns, just loop here */ 0: j 0