Re: [qemu-s390x] [kvm-unit-tests PATCH 2/5] s390x: Switch to z/Arch if needed

[Janosch Frank]

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 <address@hidden>
>> ---
>>  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 */
>>
>

signature.asc
Description: OpenPGP digital signature