diff mbox series

[v2,07/10] s390x: smp: Use full PSW to bringup new cpu

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

Commit Message

Janosch Frank April 23, 2020, 9:10 a.m. UTC
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(-)

Comments

David Hildenbrand April 24, 2020, 10:09 a.m. UTC | #1
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] ?
Janosch Frank April 24, 2020, 11:16 a.m. UTC | #2
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.
David Hildenbrand April 24, 2020, 11:23 a.m. UTC | #3
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>
Janosch Frank April 24, 2020, 11:31 a.m. UTC | #4
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 mbox series

Patch

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