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

[Thomas Huth]

 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.

[...]
> 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.

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

> +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!)
> 

 Thomas