diff mbox series

[kvm-unit-tests,1/2] s390x: smp: Use full PSW to bringup new cpu

Message ID 20191211115923.9191-2-frankja@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series s390x: smp: Improve setup of additional cpus | expand

Commit Message

Janosch Frank Dec. 11, 2019, 11:59 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>
---
 lib/s390x/smp.c  | 2 ++
 s390x/cstart64.S | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

Comments

David Hildenbrand Dec. 11, 2019, 12:31 p.m. UTC | #1 
On 11.12.19 12:59, 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>
> ---
>  lib/s390x/smp.c  | 2 ++
>  s390x/cstart64.S | 2 +-
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/s390x/smp.c b/lib/s390x/smp.c
> index f57f420..e17751a 100644
> --- a/lib/s390x/smp.c
> +++ b/lib/s390x/smp.c
> @@ -185,6 +185,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;

Not looking at the code (sorry :D ), do we still need this then? (you
drop the br bewlo)

>  	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 86dd4c4..e6a6bdb 100644
> --- a/s390x/cstart64.S
> +++ b/s390x/cstart64.S
> @@ -159,7 +159,7 @@ smp_cpu_setup_state:
>  	xgr	%r1, %r1
>  	lmg     %r0, %r15, GEN_LC_SW_INT_GRS
>  	lctlg   %c0, %c0, GEN_LC_SW_INT_CRS
> -	br	%r14
> +	lpswe	GEN_LC_SW_INT_PSW
>  
>  pgm_int:
>  	SAVE_REGS
>
Janosch Frank Dec. 11, 2019, 12:34 p.m. UTC | #2 
On 12/11/19 1:31 PM, David Hildenbrand wrote:
> On 11.12.19 12:59, 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>
>> ---
>>  lib/s390x/smp.c  | 2 ++
>>  s390x/cstart64.S | 2 +-
>>  2 files changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/s390x/smp.c b/lib/s390x/smp.c
>> index f57f420..e17751a 100644
>> --- a/lib/s390x/smp.c
>> +++ b/lib/s390x/smp.c
>> @@ -185,6 +185,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;
> 
> Not looking at the code (sorry :D ), do we still need this then? (you
> drop the br bewlo)

r14 is the return address, saving/initialising it doesn't sound like a
bad idea to me. If we ever have stack traces, it might show up, or won't it?

> 
>>  	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 86dd4c4..e6a6bdb 100644
>> --- a/s390x/cstart64.S
>> +++ b/s390x/cstart64.S
>> @@ -159,7 +159,7 @@ smp_cpu_setup_state:
>>  	xgr	%r1, %r1
>>  	lmg     %r0, %r15, GEN_LC_SW_INT_GRS
>>  	lctlg   %c0, %c0, GEN_LC_SW_INT_CRS
>> -	br	%r14
>> +	lpswe	GEN_LC_SW_INT_PSW
>>  
>>  pgm_int:
>>  	SAVE_REGS
>>
> 
>
David Hildenbrand Dec. 11, 2019, 12:54 p.m. UTC | #3 
On 11.12.19 13:34, Janosch Frank wrote:
> On 12/11/19 1:31 PM, David Hildenbrand wrote:
>> On 11.12.19 12:59, 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>
>>> ---
>>>  lib/s390x/smp.c  | 2 ++
>>>  s390x/cstart64.S | 2 +-
>>>  2 files changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/lib/s390x/smp.c b/lib/s390x/smp.c
>>> index f57f420..e17751a 100644
>>> --- a/lib/s390x/smp.c
>>> +++ b/lib/s390x/smp.c
>>> @@ -185,6 +185,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;
>>
>> Not looking at the code (sorry :D ), do we still need this then? (you
>> drop the br bewlo)
> 
> r14 is the return address, saving/initialising it doesn't sound like a
> bad idea to me. If we ever have stack traces, it might show up, or won't it?
> 
>>
>>>  	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 86dd4c4..e6a6bdb 100644
>>> --- a/s390x/cstart64.S
>>> +++ b/s390x/cstart64.S
>>> @@ -159,7 +159,7 @@ smp_cpu_setup_state:
>>>  	xgr	%r1, %r1
>>>  	lmg     %r0, %r15, GEN_LC_SW_INT_GRS
>>>  	lctlg   %c0, %c0, GEN_LC_SW_INT_CRS
>>> -	br	%r14
>>> +	lpswe	GEN_LC_SW_INT_PSW
>>>  

Makes sense

Reviewed-by: David Hildenbrand <david@redhat.com>
David Hildenbrand Dec. 12, 2019, 2:16 p.m. UTC | #4 
On 11.12.19 12:59, 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>
> ---
>  lib/s390x/smp.c  | 2 ++
>  s390x/cstart64.S | 2 +-
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/s390x/smp.c b/lib/s390x/smp.c
> index f57f420..e17751a 100644
> --- a/lib/s390x/smp.c
> +++ b/lib/s390x/smp.c
> @@ -185,6 +185,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 86dd4c4..e6a6bdb 100644
> --- a/s390x/cstart64.S
> +++ b/s390x/cstart64.S
> @@ -159,7 +159,7 @@ smp_cpu_setup_state:
>  	xgr	%r1, %r1
>  	lmg     %r0, %r15, GEN_LC_SW_INT_GRS
>  	lctlg   %c0, %c0, GEN_LC_SW_INT_CRS
> -	br	%r14
> +	lpswe	GEN_LC_SW_INT_PSW
>  
>  pgm_int:
>  	SAVE_REGS
> 

Queued to

https://github.com/davidhildenbrand/kvm-unit-tests.git s390x-next

Thanks!
diff mbox series

Patch

diff --git a/lib/s390x/smp.c b/lib/s390x/smp.c
index f57f420..e17751a 100644
--- a/lib/s390x/smp.c
+++ b/lib/s390x/smp.c
@@ -185,6 +185,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 86dd4c4..e6a6bdb 100644
--- a/s390x/cstart64.S
+++ b/s390x/cstart64.S
@@ -159,7 +159,7 @@  smp_cpu_setup_state:
 	xgr	%r1, %r1
 	lmg     %r0, %r15, GEN_LC_SW_INT_GRS
 	lctlg   %c0, %c0, GEN_LC_SW_INT_CRS
-	br	%r14
+	lpswe	GEN_LC_SW_INT_PSW
 
 pgm_int:
 	SAVE_REGS