kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [kvm-unit-tests PATCH 0/2] s390x: smp: Improve setup of additional cpus
@ 2019-12-11 11:59 Janosch Frank
  2019-12-11 11:59 ` [kvm-unit-tests PATCH 1/2] s390x: smp: Use full PSW to bringup new cpu Janosch Frank
  2019-12-11 11:59 ` [kvm-unit-tests PATCH 2/2] s390x: smp: Setup CRs from cpu 0 Janosch Frank
  0 siblings, 2 replies; 12+ messages in thread
From: Janosch Frank @ 2019-12-11 11:59 UTC (permalink / raw)
  To: kvm; +Cc: thuth, david, linux-s390

Let's not use constants for the PSW mask and CRs, but set them from
input data or cpu 0.

Later on we could add functions that take a ptr for setting up CRs of
additional cpus, but for now using the CRs 0,1,7,13 from cpu 0 will at
least result in a working DAT environment.

Janosch Frank (2):
  s390x: smp: Use full PSW to bringup new cpu
  s390x: smp: Setup CRs from cpu 0

 lib/s390x/smp.c  | 7 ++++++-
 s390x/cstart64.S | 4 ++--
 2 files changed, 8 insertions(+), 3 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [kvm-unit-tests PATCH 1/2] s390x: smp: Use full PSW to bringup new cpu
  2019-12-11 11:59 [kvm-unit-tests PATCH 0/2] s390x: smp: Improve setup of additional cpus Janosch Frank
@ 2019-12-11 11:59 ` Janosch Frank
  2019-12-11 12:31   ` David Hildenbrand
  2019-12-12 14:16   ` David Hildenbrand
  2019-12-11 11:59 ` [kvm-unit-tests PATCH 2/2] s390x: smp: Setup CRs from cpu 0 Janosch Frank
  1 sibling, 2 replies; 12+ messages in thread
From: Janosch Frank @ 2019-12-11 11:59 UTC (permalink / raw)
  To: kvm; +Cc: thuth, david, linux-s390

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
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [kvm-unit-tests PATCH 2/2] s390x: smp: Setup CRs from cpu 0
  2019-12-11 11:59 [kvm-unit-tests PATCH 0/2] s390x: smp: Improve setup of additional cpus Janosch Frank
  2019-12-11 11:59 ` [kvm-unit-tests PATCH 1/2] s390x: smp: Use full PSW to bringup new cpu Janosch Frank
@ 2019-12-11 11:59 ` Janosch Frank
  2019-12-11 12:32   ` David Hildenbrand
  1 sibling, 1 reply; 12+ messages in thread
From: Janosch Frank @ 2019-12-11 11:59 UTC (permalink / raw)
  To: kvm; +Cc: thuth, david, linux-s390

Grab the CRs (currently only 0, 1, 7, 13) from cpu 0, so we can
bringup the new cpu in DAT mode or set other control options.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 lib/s390x/smp.c  | 5 ++++-
 s390x/cstart64.S | 2 +-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/lib/s390x/smp.c b/lib/s390x/smp.c
index e17751a..4dfe7c6 100644
--- a/lib/s390x/smp.c
+++ b/lib/s390x/smp.c
@@ -191,7 +191,10 @@ int smp_cpu_setup(uint16_t addr, struct psw psw)
 	cpu->lowcore->sw_int_grs[15] = (uint64_t)cpu->stack + (PAGE_SIZE * 4);
 	lc->restart_new_psw.mask = 0x0000000180000000UL;
 	lc->restart_new_psw.addr = (uint64_t)smp_cpu_setup_state;
-	lc->sw_int_crs[0] = 0x0000000000040000UL;
+	lc->sw_int_crs[0] = stctg(0);
+	lc->sw_int_crs[1] = stctg(1);
+	lc->sw_int_crs[7] = stctg(7);
+	lc->sw_int_crs[13] = stctg(13);
 
 	/* Start processing */
 	rc = sigp_retry(cpu->addr, SIGP_RESTART, 0, NULL);
diff --git a/s390x/cstart64.S b/s390x/cstart64.S
index e6a6bdb..399ae9b 100644
--- a/s390x/cstart64.S
+++ b/s390x/cstart64.S
@@ -158,7 +158,7 @@ diag308_load_reset:
 smp_cpu_setup_state:
 	xgr	%r1, %r1
 	lmg     %r0, %r15, GEN_LC_SW_INT_GRS
-	lctlg   %c0, %c0, GEN_LC_SW_INT_CRS
+	lctlg   %c0, %c15, GEN_LC_SW_INT_CRS
 	lpswe	GEN_LC_SW_INT_PSW
 
 pgm_int:
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [kvm-unit-tests PATCH 1/2] s390x: smp: Use full PSW to bringup new cpu
  2019-12-11 11:59 ` [kvm-unit-tests PATCH 1/2] s390x: smp: Use full PSW to bringup new cpu Janosch Frank
@ 2019-12-11 12:31   ` David Hildenbrand
  2019-12-11 12:34     ` Janosch Frank
  2019-12-12 14:16   ` David Hildenbrand
  1 sibling, 1 reply; 12+ messages in thread
From: David Hildenbrand @ 2019-12-11 12:31 UTC (permalink / raw)
  To: Janosch Frank, kvm; +Cc: thuth, linux-s390

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
> 


-- 
Thanks,

David / dhildenb


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [kvm-unit-tests PATCH 2/2] s390x: smp: Setup CRs from cpu 0
  2019-12-11 11:59 ` [kvm-unit-tests PATCH 2/2] s390x: smp: Setup CRs from cpu 0 Janosch Frank
@ 2019-12-11 12:32   ` David Hildenbrand
  2019-12-11 12:37     ` Janosch Frank
  0 siblings, 1 reply; 12+ messages in thread
From: David Hildenbrand @ 2019-12-11 12:32 UTC (permalink / raw)
  To: Janosch Frank, kvm; +Cc: thuth, linux-s390

On 11.12.19 12:59, Janosch Frank wrote:
> Grab the CRs (currently only 0, 1, 7, 13) from cpu 0, so we can
> bringup the new cpu in DAT mode or set other control options.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>  lib/s390x/smp.c  | 5 ++++-
>  s390x/cstart64.S | 2 +-
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/s390x/smp.c b/lib/s390x/smp.c
> index e17751a..4dfe7c6 100644
> --- a/lib/s390x/smp.c
> +++ b/lib/s390x/smp.c
> @@ -191,7 +191,10 @@ int smp_cpu_setup(uint16_t addr, struct psw psw)
>  	cpu->lowcore->sw_int_grs[15] = (uint64_t)cpu->stack + (PAGE_SIZE * 4);
>  	lc->restart_new_psw.mask = 0x0000000180000000UL;
>  	lc->restart_new_psw.addr = (uint64_t)smp_cpu_setup_state;
> -	lc->sw_int_crs[0] = 0x0000000000040000UL;
> +	lc->sw_int_crs[0] = stctg(0);
> +	lc->sw_int_crs[1] = stctg(1);
> +	lc->sw_int_crs[7] = stctg(7);
> +	lc->sw_int_crs[13] = stctg(13);

Wouldn't it be better to also be able to specify the CRs explicitly here?

-- 
Thanks,

David / dhildenb


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [kvm-unit-tests PATCH 1/2] s390x: smp: Use full PSW to bringup new cpu
  2019-12-11 12:31   ` David Hildenbrand
@ 2019-12-11 12:34     ` Janosch Frank
  2019-12-11 12:54       ` David Hildenbrand
  0 siblings, 1 reply; 12+ messages in thread
From: Janosch Frank @ 2019-12-11 12:34 UTC (permalink / raw)
  To: David Hildenbrand, kvm; +Cc: thuth, linux-s390


[-- Attachment #1.1: Type: text/plain, Size: 1671 bytes --]

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
>>
> 
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [kvm-unit-tests PATCH 2/2] s390x: smp: Setup CRs from cpu 0
  2019-12-11 12:32   ` David Hildenbrand
@ 2019-12-11 12:37     ` Janosch Frank
  2019-12-11 12:54       ` David Hildenbrand
  0 siblings, 1 reply; 12+ messages in thread
From: Janosch Frank @ 2019-12-11 12:37 UTC (permalink / raw)
  To: David Hildenbrand, kvm; +Cc: thuth, linux-s390


[-- Attachment #1.1: Type: text/plain, Size: 1431 bytes --]

On 12/11/19 1:32 PM, David Hildenbrand wrote:
> On 11.12.19 12:59, Janosch Frank wrote:
>> Grab the CRs (currently only 0, 1, 7, 13) from cpu 0, so we can
>> bringup the new cpu in DAT mode or set other control options.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>>  lib/s390x/smp.c  | 5 ++++-
>>  s390x/cstart64.S | 2 +-
>>  2 files changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/s390x/smp.c b/lib/s390x/smp.c
>> index e17751a..4dfe7c6 100644
>> --- a/lib/s390x/smp.c
>> +++ b/lib/s390x/smp.c
>> @@ -191,7 +191,10 @@ int smp_cpu_setup(uint16_t addr, struct psw psw)
>>  	cpu->lowcore->sw_int_grs[15] = (uint64_t)cpu->stack + (PAGE_SIZE * 4);
>>  	lc->restart_new_psw.mask = 0x0000000180000000UL;
>>  	lc->restart_new_psw.addr = (uint64_t)smp_cpu_setup_state;
>> -	lc->sw_int_crs[0] = 0x0000000000040000UL;
>> +	lc->sw_int_crs[0] = stctg(0);
>> +	lc->sw_int_crs[1] = stctg(1);
>> +	lc->sw_int_crs[7] = stctg(7);
>> +	lc->sw_int_crs[13] = stctg(13);
> 
> Wouldn't it be better to also be able to specify the CRs explicitly here?
> 

Yes, but currently there are no users for something like that and it
would mean that we might need to add more code to support it.

As I said in the cover letter, this is a good first step to allow DAT on
additional cpus without any real setup needed in a test. Later we could
add a function to specify the CRs explicitly.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [kvm-unit-tests PATCH 2/2] s390x: smp: Setup CRs from cpu 0
  2019-12-11 12:37     ` Janosch Frank
@ 2019-12-11 12:54       ` David Hildenbrand
  2019-12-11 13:08         ` Janosch Frank
  0 siblings, 1 reply; 12+ messages in thread
From: David Hildenbrand @ 2019-12-11 12:54 UTC (permalink / raw)
  To: Janosch Frank, kvm; +Cc: thuth, linux-s390

On 11.12.19 13:37, Janosch Frank wrote:
> On 12/11/19 1:32 PM, David Hildenbrand wrote:
>> On 11.12.19 12:59, Janosch Frank wrote:
>>> Grab the CRs (currently only 0, 1, 7, 13) from cpu 0, so we can
>>> bringup the new cpu in DAT mode or set other control options.
>>>
>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>>> ---
>>>  lib/s390x/smp.c  | 5 ++++-
>>>  s390x/cstart64.S | 2 +-
>>>  2 files changed, 5 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/lib/s390x/smp.c b/lib/s390x/smp.c
>>> index e17751a..4dfe7c6 100644
>>> --- a/lib/s390x/smp.c
>>> +++ b/lib/s390x/smp.c
>>> @@ -191,7 +191,10 @@ int smp_cpu_setup(uint16_t addr, struct psw psw)
>>>  	cpu->lowcore->sw_int_grs[15] = (uint64_t)cpu->stack + (PAGE_SIZE * 4);
>>>  	lc->restart_new_psw.mask = 0x0000000180000000UL;
>>>  	lc->restart_new_psw.addr = (uint64_t)smp_cpu_setup_state;
>>> -	lc->sw_int_crs[0] = 0x0000000000040000UL;
>>> +	lc->sw_int_crs[0] = stctg(0);
>>> +	lc->sw_int_crs[1] = stctg(1);
>>> +	lc->sw_int_crs[7] = stctg(7);
>>> +	lc->sw_int_crs[13] = stctg(13);
>>
>> Wouldn't it be better to also be able to specify the CRs explicitly here?
>>
> 
> Yes, but currently there are no users for something like that and it
> would mean that we might need to add more code to support it.
> 
> As I said in the cover letter, this is a good first step to allow DAT on
> additional cpus without any real setup needed in a test. Later we could
> add a function to specify the CRs explicitly.
> 

Can you clarify why we need this patch now (e.g., DAT)? This patch
sounds like it would make sense in the future only (it is easier to
review with future changes IMHO).

-- 
Thanks,

David / dhildenb


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [kvm-unit-tests PATCH 1/2] s390x: smp: Use full PSW to bringup new cpu
  2019-12-11 12:34     ` Janosch Frank
@ 2019-12-11 12:54       ` David Hildenbrand
  0 siblings, 0 replies; 12+ messages in thread
From: David Hildenbrand @ 2019-12-11 12:54 UTC (permalink / raw)
  To: Janosch Frank, kvm; +Cc: thuth, linux-s390

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>


-- 
Thanks,

David / dhildenb


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [kvm-unit-tests PATCH 2/2] s390x: smp: Setup CRs from cpu 0
  2019-12-11 12:54       ` David Hildenbrand
@ 2019-12-11 13:08         ` Janosch Frank
  2019-12-11 13:14           ` David Hildenbrand
  0 siblings, 1 reply; 12+ messages in thread
From: Janosch Frank @ 2019-12-11 13:08 UTC (permalink / raw)
  To: David Hildenbrand, kvm; +Cc: thuth, linux-s390


[-- Attachment #1.1: Type: text/plain, Size: 2026 bytes --]

On 12/11/19 1:54 PM, David Hildenbrand wrote:
> On 11.12.19 13:37, Janosch Frank wrote:
>> On 12/11/19 1:32 PM, David Hildenbrand wrote:
>>> On 11.12.19 12:59, Janosch Frank wrote:
>>>> Grab the CRs (currently only 0, 1, 7, 13) from cpu 0, so we can
>>>> bringup the new cpu in DAT mode or set other control options.
>>>>
>>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>>>> ---
>>>>  lib/s390x/smp.c  | 5 ++++-
>>>>  s390x/cstart64.S | 2 +-
>>>>  2 files changed, 5 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/lib/s390x/smp.c b/lib/s390x/smp.c
>>>> index e17751a..4dfe7c6 100644
>>>> --- a/lib/s390x/smp.c
>>>> +++ b/lib/s390x/smp.c
>>>> @@ -191,7 +191,10 @@ int smp_cpu_setup(uint16_t addr, struct psw psw)
>>>>  	cpu->lowcore->sw_int_grs[15] = (uint64_t)cpu->stack + (PAGE_SIZE * 4);
>>>>  	lc->restart_new_psw.mask = 0x0000000180000000UL;
>>>>  	lc->restart_new_psw.addr = (uint64_t)smp_cpu_setup_state;
>>>> -	lc->sw_int_crs[0] = 0x0000000000040000UL;
>>>> +	lc->sw_int_crs[0] = stctg(0);
>>>> +	lc->sw_int_crs[1] = stctg(1);
>>>> +	lc->sw_int_crs[7] = stctg(7);
>>>> +	lc->sw_int_crs[13] = stctg(13);
>>>
>>> Wouldn't it be better to also be able to specify the CRs explicitly here?
>>>
>>
>> Yes, but currently there are no users for something like that and it
>> would mean that we might need to add more code to support it.
>>
>> As I said in the cover letter, this is a good first step to allow DAT on
>> additional cpus without any real setup needed in a test. Later we could
>> add a function to specify the CRs explicitly.
>>
> 
> Can you clarify why we need this patch now (e.g., DAT)? This patch
> sounds like it would make sense in the future only (it is easier to
> review with future changes IMHO).
> 

Some G1 UV calls need the home space and therefore I added this patch to
my concurrency tests which are still in my queue. I thought these fixes
might make sense anyway and could be flushed from my queue before the UV
patches are ready.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [kvm-unit-tests PATCH 2/2] s390x: smp: Setup CRs from cpu 0
  2019-12-11 13:08         ` Janosch Frank
@ 2019-12-11 13:14           ` David Hildenbrand
  0 siblings, 0 replies; 12+ messages in thread
From: David Hildenbrand @ 2019-12-11 13:14 UTC (permalink / raw)
  To: Janosch Frank, kvm; +Cc: thuth, linux-s390

On 11.12.19 14:08, Janosch Frank wrote:
> On 12/11/19 1:54 PM, David Hildenbrand wrote:
>> On 11.12.19 13:37, Janosch Frank wrote:
>>> On 12/11/19 1:32 PM, David Hildenbrand wrote:
>>>> On 11.12.19 12:59, Janosch Frank wrote:
>>>>> Grab the CRs (currently only 0, 1, 7, 13) from cpu 0, so we can
>>>>> bringup the new cpu in DAT mode or set other control options.
>>>>>
>>>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>>>>> ---
>>>>>  lib/s390x/smp.c  | 5 ++++-
>>>>>  s390x/cstart64.S | 2 +-
>>>>>  2 files changed, 5 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/lib/s390x/smp.c b/lib/s390x/smp.c
>>>>> index e17751a..4dfe7c6 100644
>>>>> --- a/lib/s390x/smp.c
>>>>> +++ b/lib/s390x/smp.c
>>>>> @@ -191,7 +191,10 @@ int smp_cpu_setup(uint16_t addr, struct psw psw)
>>>>>  	cpu->lowcore->sw_int_grs[15] = (uint64_t)cpu->stack + (PAGE_SIZE * 4);
>>>>>  	lc->restart_new_psw.mask = 0x0000000180000000UL;
>>>>>  	lc->restart_new_psw.addr = (uint64_t)smp_cpu_setup_state;
>>>>> -	lc->sw_int_crs[0] = 0x0000000000040000UL;
>>>>> +	lc->sw_int_crs[0] = stctg(0);
>>>>> +	lc->sw_int_crs[1] = stctg(1);
>>>>> +	lc->sw_int_crs[7] = stctg(7);
>>>>> +	lc->sw_int_crs[13] = stctg(13);
>>>>
>>>> Wouldn't it be better to also be able to specify the CRs explicitly here?
>>>>
>>>
>>> Yes, but currently there are no users for something like that and it
>>> would mean that we might need to add more code to support it.
>>>
>>> As I said in the cover letter, this is a good first step to allow DAT on
>>> additional cpus without any real setup needed in a test. Later we could
>>> add a function to specify the CRs explicitly.
>>>
>>
>> Can you clarify why we need this patch now (e.g., DAT)? This patch
>> sounds like it would make sense in the future only (it is easier to
>> review with future changes IMHO).
>>
> 
> Some G1 UV calls need the home space and therefore I added this patch to
> my concurrency tests which are still in my queue. I thought these fixes
> might make sense anyway and could be flushed from my queue before the UV
> patches are ready.

Okay, so it needs e.g., CR13, but we only populate CR0 and CR1 right now
if I'm not wrong. IOW, let's send this with the other patches (or only
populate cr0/cr1 in this patch and add the other stuff on top later on).

-- 
Thanks,

David / dhildenb


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [kvm-unit-tests PATCH 1/2] s390x: smp: Use full PSW to bringup new cpu
  2019-12-11 11:59 ` [kvm-unit-tests PATCH 1/2] s390x: smp: Use full PSW to bringup new cpu Janosch Frank
  2019-12-11 12:31   ` David Hildenbrand
@ 2019-12-12 14:16   ` David Hildenbrand
  1 sibling, 0 replies; 12+ messages in thread
From: David Hildenbrand @ 2019-12-12 14:16 UTC (permalink / raw)
  To: Janosch Frank, kvm; +Cc: thuth, linux-s390

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!

-- 
Thanks,

David / dhildenb


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2019-12-12 14:16 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-11 11:59 [kvm-unit-tests PATCH 0/2] s390x: smp: Improve setup of additional cpus Janosch Frank
2019-12-11 11:59 ` [kvm-unit-tests PATCH 1/2] s390x: smp: Use full PSW to bringup new cpu Janosch Frank
2019-12-11 12:31   ` David Hildenbrand
2019-12-11 12:34     ` Janosch Frank
2019-12-11 12:54       ` David Hildenbrand
2019-12-12 14:16   ` David Hildenbrand
2019-12-11 11:59 ` [kvm-unit-tests PATCH 2/2] s390x: smp: Setup CRs from cpu 0 Janosch Frank
2019-12-11 12:32   ` David Hildenbrand
2019-12-11 12:37     ` Janosch Frank
2019-12-11 12:54       ` David Hildenbrand
2019-12-11 13:08         ` Janosch Frank
2019-12-11 13:14           ` David Hildenbrand

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).