Patchwork [kvm-unit-tests,2/5] s390x: Switch to z/Arch if needed

login
register
mail settings
Submitter Janosch Frank
Date Dec. 4, 2018, 1:48 p.m.
Message ID <20181204134838.5841-3-frankja@linux.ibm.com>
Download mbox | patch
Permalink /patch/671911/
State New
Headers show

Comments

Janosch Frank - Dec. 4, 2018, 1:48 p.m.
LPAR and z/VM start in esam mode, so we need to switch to z/Arch and
set 64 bit addressing.

Under Qemu/KVM we already start out with both when being run with the
Qemu --kernel argument or we lack only 64 bit when booting from disk.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 s390x/cstart64.S | 9 +++++++++
 1 file changed, 9 insertions(+)
David Hildenbrand - Dec. 5, 2018, 9:45 a.m.
On 04.12.18 14:48, Janosch Frank wrote:
> LPAR and z/VM start in esam mode, so we need to switch to z/Arch and
> set 64 bit addressing.
> 
> Under Qemu/KVM we already start out with both when being run with the
> Qemu --kernel argument or we lack only 64 bit when booting from disk.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>  s390x/cstart64.S | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/s390x/cstart64.S b/s390x/cstart64.S
> index abd6b58..80dbcba 100644
> --- a/s390x/cstart64.S
> +++ b/s390x/cstart64.S
> @@ -20,6 +20,15 @@
>   */
>  	.globl start
>  start:
> +	tam				# Test for 64 bit addressing (KVM IPL)
> +	brc	1, start64		# If cc = 3 (64 bit), we have z/Arch

Looking at the kernel, we don't have to test for 64bit but can simply do
that always unconditionally. SIGP might bail out but we ignore it. sam64
will also simply do nothing.

So tam+brc are not necessary.

> +	/* Switch to z/Architecture mode (64-bit) */
> +	slr     %r0, %r0		# set cpuid to zero
> +	lhi     %r1, 2			# mode 2 = esame
> +	sigp    %r1, %r0, 0x12		# sigp set arch
> +	bras	%r13,0f
> +	.fill	16,4,0x0
> +0:	lmh	%r0,%r15,0(%r13)	# clear high-order half of gprs

This is just what the kernel does. The ".fill" part is confusing at
first, but it is just the data loaded into the high-order halfs (all 0s)

(I was told mixing code and data is bad practice, but for some reason
people in Linux decided to do it like that (maybe because of some
relocation thingy? or because of instruction set restrictions?))

Should we introduce defines for SIGPs? We might have more once we
support multiple CPUs eiher way.

>  	sam64				# Set addressing mode to 64 bit
>  start64:
>  	/* setup stack */
>
Janosch Frank - Dec. 5, 2018, 10:15 a.m.
On 05.12.18 10:45, David Hildenbrand wrote:
> On 04.12.18 14:48, Janosch Frank wrote:
>> LPAR and z/VM start in esam mode, so we need to switch to z/Arch and
>> set 64 bit addressing.
>>
>> Under Qemu/KVM we already start out with both when being run with the
>> Qemu --kernel argument or we lack only 64 bit when booting from disk.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>>  s390x/cstart64.S | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/s390x/cstart64.S b/s390x/cstart64.S
>> index abd6b58..80dbcba 100644
>> --- a/s390x/cstart64.S
>> +++ b/s390x/cstart64.S
>> @@ -20,6 +20,15 @@
>>   */
>>  	.globl start
>>  start:
>> +	tam				# Test for 64 bit addressing (KVM IPL)
>> +	brc	1, start64		# If cc = 3 (64 bit), we have z/Arch
> 
> Looking at the kernel, we don't have to test for 64bit but can simply do
> that always unconditionally. SIGP might bail out but we ignore it. sam64
> will also simply do nothing.
> 
> So tam+brc are not necessary.

Right, sigp will be a nop with inv parm and inc state as a return.
I guess we don't need to save cycles here so I can get rid of the branch.

> 
>> +	/* Switch to z/Architecture mode (64-bit) */
>> +	slr     %r0, %r0		# set cpuid to zero
>> +	lhi     %r1, 2			# mode 2 = esame
>> +	sigp    %r1, %r0, 0x12		# sigp set arch
>> +	bras	%r13,0f
>> +	.fill	16,4,0x0
>> +0:	lmh	%r0,%r15,0(%r13)	# clear high-order half of gprs
> 
> This is just what the kernel does. The ".fill" part is confusing at
> first, but it is just the data loaded into the high-order halfs (all 0s)
> 
> (I was told mixing code and data is bad practice, but for some reason
> people in Linux decided to do it like that (maybe because of some
> relocation thingy? or because of instruction set restrictions?))

Honestly I wouldn't even have considered clearing if I haven't seen it
in the kernel. I could define a zero area at the end of the file or do
an lm targeting the cleared bss right before jumping into setup. Or we
could do 16 xc register to register :)

> 
> Should we introduce defines for SIGPs? We might have more once we
> support multiple CPUs eiher way.

We also have sigp stop so why not.
How about making the indent patch a cleanup patch and adding all
commands there?

> 
>>  	sam64				# Set addressing mode to 64 bit
>>  start64:
>>  	/* setup stack */
>>
>
David Hildenbrand - Dec. 5, 2018, 10:22 a.m.
>>
>>> +	/* Switch to z/Architecture mode (64-bit) */
>>> +	slr     %r0, %r0		# set cpuid to zero
>>> +	lhi     %r1, 2			# mode 2 = esame
>>> +	sigp    %r1, %r0, 0x12		# sigp set arch
>>> +	bras	%r13,0f
>>> +	.fill	16,4,0x0
>>> +0:	lmh	%r0,%r15,0(%r13)	# clear high-order half of gprs
>>
>> This is just what the kernel does. The ".fill" part is confusing at
>> first, but it is just the data loaded into the high-order halfs (all 0s)
>>
>> (I was told mixing code and data is bad practice, but for some reason
>> people in Linux decided to do it like that (maybe because of some
>> relocation thingy? or because of instruction set restrictions?))
> 
> Honestly I wouldn't even have considered clearing if I haven't seen it
> in the kernel. I could define a zero area at the end of the file or do
> an lm targeting the cleared bss right before jumping into setup. Or we
> could do 16 xc register to register :)

Or we'll just stick to what the kernel did here ... at least that way we
can be pretty sure we don't make mistakes. Whatever you prefer ;)

> 
>>
>> Should we introduce defines for SIGPs? We might have more once we
>> support multiple CPUs eiher way.
> 
> We also have sigp stop so why not.
> How about making the indent patch a cleanup patch and adding all
> commands there?

Sure, fine with me!

Patch

diff --git a/s390x/cstart64.S b/s390x/cstart64.S
index abd6b58..80dbcba 100644
--- a/s390x/cstart64.S
+++ b/s390x/cstart64.S
@@ -20,6 +20,15 @@ 
  */
 	.globl start
 start:
+	tam				# Test for 64 bit addressing (KVM IPL)
+	brc	1, start64		# If cc = 3 (64 bit), we have z/Arch
+	/* Switch to z/Architecture mode (64-bit) */
+	slr     %r0, %r0		# set cpuid to zero
+	lhi     %r1, 2			# mode 2 = esame
+	sigp    %r1, %r0, 0x12		# sigp set arch
+	bras	%r13,0f
+	.fill	16,4,0x0
+0:	lmh	%r0,%r15,0(%r13)	# clear high-order half of gprs
 	sam64				# Set addressing mode to 64 bit
 start64:
 	/* setup stack */