[SLOF] [PATCH slof] rtas: Integrate RTAS blob

Alexey Kardashevskiy aik at ozlabs.ru
Tue Jul 16 20:21:03 AEST 2019



On 16/07/2019 18:40, Thomas Huth wrote:
>   Hi Alexey!
> 
> On 16/07/2019 07.11, Alexey Kardashevskiy wrote:
>> We implement RTAS as a simple binary blob which calls directly into QEMU
>> via a custom hcall. So far we were relying on QEMU putting the RTAS blob
>> to the guest memory with its location in linux,rtas-base/rtas-size.
> 
> Likely a left-over from the time when it was possible to start a Linux
> kernel directly, without SLOF. But since this is not possible anymore, I
> think your patch is a nice clean-up, indeed.


With a small patch to call rtas directly via H_RTAS from linux, it is 
possible :)


> [...]
>> diff --git a/board-qemu/slof/rtas.fs b/board-qemu/slof/rtas.fs
>> index 092f5b1131f0..1b791b538d92 100644
>> --- a/board-qemu/slof/rtas.fs
>> +++ b/board-qemu/slof/rtas.fs
>> @@ -40,38 +40,10 @@ rtas-cb /rtas-control-block erase
>>   0 VALUE rtas-entry
>>   0 VALUE rtas-node
>>   
>> -\ Locate qemu RTAS, remove the linux,... properties we really don't
>> -\ want them to stick around
>> -
>>   372 cp
>>   
>>   : find-qemu-rtas ( -- )
>>       " /rtas" find-device get-node to rtas-node
>> -
>> -    " linux,rtas-base" rtas-node get-package-property IF
>> -         device-end EXIT THEN
>> -    drop l@ to rtas-base
>> -    " linux,rtas-base" delete-property
>> -
>> -    " rtas-size" rtas-node get-package-property IF
>> -         device-end EXIT THEN
>> -    drop l@ to rtas-size
>> -
>> -    " linux,rtas-entry" rtas-node get-package-property IF
>> -        rtas-base to rtas-entry
>> -    ELSE
>> -        drop l@ to rtas-entry
>> -        " linux,rtas-entry" delete-property
>> -    THEN
>> -
>> -\    ." RTAS found, base=" rtas-base . ."  size=" rtas-size . cr
>> -
>> -    \ Patch the RTAS blob with our sc1 patcher if necessary
>> -    0
>> -    rtas-base
>> -    dup rtas-size +
>> -    check-and-patch-sc1
>> -
>>       device-end
>>   ;
>>   find-qemu-rtas
> 
> I think I'd rather simplify that whole function now to:
> 
> s" /rtas" find-node to rtas-node
> 
> (no need to put this code into a function, and no need for device-end
> anymore)
> 
>> @@ -176,6 +148,14 @@ rtas-node set-node
>>   : open true ;
>>   : close ;
>>   
>> +: store-hv-rtas-size (  )
>> +    hv-rtas-size
>> +    dup to rtas-size
>> +    encode-int s" rtas-size" s" /rtas" find-node set-property
>> +;
>> +
>> +store-hv-rtas-size
>> +
>>   : store-rtas-loc ( adr )
>>       s" /rtas" find-node >r
>>       encode-int s" slof,rtas-base" r@ set-property
>> @@ -184,8 +164,12 @@ rtas-node set-node
>>   
>>   : instantiate-rtas ( adr -- entry )
>>       dup store-rtas-loc
>> -    dup rtas-base swap rtas-size move
>> -    rtas-entry rtas-base - +
>> +    dup to rtas-base
>> +    dup to rtas-entry
>> +    s" /rtas" find-node >r
>> +    dup encode-int s" linux,rtas-base" r@ set-property
>> +    dup encode-int s" linux,rtas-entry" r> set-property
> 
> Why do you create the "linux,..." properties here? SLOF should normally
> not create anything with the "linux," prefix.


Well, I am removing those from qemu, that's why but I do not need those 
anyway as linux creates them anyway.


> 
>> +    dup hv-instantiate-rtas
>>   ;
>>   
>>   device-end
>> diff --git a/lib/libhvcall/hvcall.S b/lib/libhvcall/hvcall.S
>> index 92cf22e4c22c..19163f619be4 100644
>> --- a/lib/libhvcall/hvcall.S
>> +++ b/lib/libhvcall/hvcall.S
>> @@ -127,6 +127,25 @@ ENTRY(hv_cas)
>>   	HVCALL
>>   	blr
>>   
>> +/* This is the actual RTAS blob copied to the OS at instantiate-rtas */
>> +ENTRY(hv_rtas)
>> +	mr	r4,r3
>> +	lis	r3,KVMPPC_H_RTAS at h
>> +	ori	r3,r3,KVMPPC_H_RTAS at l
>> +	HVCALL
>> +	blr
>> +	.globl hv_rtas_end
>> +hv_rtas_end = .;
> 
> you could calculate the size here instead already, a la:
> 
> hv_rtas_code_size = . - hv_rtas;
> 
> ...
> 
>> +ENTRY(hv_rtas_broken_sc1)
>> +	mr	r4,r3
>> +	lis	r3,KVMPPC_H_RTAS at h
>> +	ori	r3,r3,KVMPPC_H_RTAS at l
>> +	.long	0x7c000268
>> +	blr
>> +	.globl hv_rtas_broken_sc1_end
>> +hv_rtas_broken_sc1_end = .;
> 
> ... and:
> 
> hv_rtas_broken_sc1_size = . - hv_rtas_broken_sc1;
> 
> ...
> 
>>   	.section ".bss"
>>   inbuf:	.space	16
>>   inlen:	.space	4
>> diff --git a/lib/libhvcall/hvcall.code b/lib/libhvcall/hvcall.code
>> index 5918c901252e..1e47e55a5cc2 100644
>> --- a/lib/libhvcall/hvcall.code
>> +++ b/lib/libhvcall/hvcall.code
>> @@ -128,3 +128,25 @@ PRIM(hv_X2d_update_X2d_dt)
>>   	unsigned long dt = TOS.u;
>>   	TOS.u = hv_generic(KVMPPC_H_UPDATE_DT, dt);
>>   MIRP
>> +
>> +PRIM(hv_X2d_instantiate_X2d_rtas)
>> +	unsigned long start = TOS.u; POP;
>> +    if (check_broken_sc1()) {
>> +        memcpy((void *) start, &hv_rtas_broken_sc1,
>> +                (unsigned long) &hv_rtas_broken_sc1_end -
>> +                (unsigned long) &hv_rtas_broken_sc1);
>> +    } else {
>> +        memcpy((void *) start, &hv_rtas,
>> +                (unsigned long) &hv_rtas_end - (unsigned long) &hv_rtas);
>> +    }
> 
> ... then you can use the size here directly and don't have to deal with
> the ugly casting to (unsigned long) all over the place.


If I do as you say, I'll have to take address of hv_rtas_size as 
hv_rtas_size is not a variable but address and for some reason it is 
0x6faf0014 instead of 0x14. Although it seems to be correct in objdump:

[fstn1-p1 slof]$ objdump -t ./board-qemu/slof/paflof | grep hv_rtas_size
0000000000000014 g       *ABS*  0000000000000000 hv_rtas_size

This produces nice "extern unsigned int hv_rtas_size" with no casts in 
those PRIM(xxx):

ENTRY(hv_rtas)
    mr. r4,r3
    lis r3,KVMPPC_H_RTAS at h
    ori r3,r3,KVMPPC_H_RTAS at l
    HVCALL
    blr
hv_rtas_end = .;
.data
   .globl hv_rtas_size
hv_rtas_size:
    .long hv_rtas_end - hv_rtas;

I am missing here something (did not touch assembly for years), what is 
it? Thanks,



> 
>> +MIRP
>> +
>> +PRIM(hv_X2d_rtas_X2d_size)
>> +    PUSH;
>> +    if (check_broken_sc1()) {
>> +        TOS.u = (unsigned long) &hv_rtas_broken_sc1_end -
>> +                (unsigned long) &hv_rtas_broken_sc1;
>> +    } else {
>> +        TOS.u = (unsigned long) &hv_rtas_end - (unsigned long) &hv_rtas;
>> +    }
>> +MIRP
> 
> Please indent C code with tabs, not with spaces.
> 
>> diff --git a/lib/libhvcall/hvcall.in b/lib/libhvcall/hvcall.in
>> index 9193162dd8ce..4165c6bcab58 100644
>> --- a/lib/libhvcall/hvcall.in
>> +++ b/lib/libhvcall/hvcall.in
>> @@ -18,6 +18,8 @@ cod(hv-free-crq)
>>   cod(hv-send-crq)
>>   cod(hv-put-tce)
>>   cod(check-and-patch-sc1)
>> +cod(hv-instantiate-rtas)
>> +cod(hv-rtas-size)
>>   
>>   cod(RB@)
>>   cod(RB!)


-- 
Alexey


More information about the SLOF mailing list