[SLOF] [PATCH slof] rtas: Reserve space for FWNMI log

[Aravinda Prasad]

On Tuesday 27 August 2019 09:47 AM, David Gibson wrote:
> On Tue, Aug 27, 2019 at 11:39:21AM +1000, Alexey Kardashevskiy wrote:
>>
>>
>> On 26/08/2019 22:29, Thomas Huth wrote:
>>> On 26/08/2019 05.44, Alexey Kardashevskiy wrote:
>>>> The Firmware Assisted Non-Maskable Interrupts Option (FWNMI) feature
>>>> requires some space for RTAS log which is in the RTAS blob area.
>>>>
>>>> This expands the RTAS blob size to 2k.
>>>>
>>>> More details here: https://patchwork.ozlabs.org/patch/1146765/
>>>
>>> Ouch, that sounds like an very fragile interface. Is this mandated by
>>> sPAPR that we have to put the log into the RTAS region?
>>
>> This is my understanding, yes. Aravinda can correct me if I am wrong. QEMU
>> uses a fixed offset of 0x30 into this area to decide where the log starts.
>> Not perfect but the frequency of RTAS changes (which a naught) assumes we
>> are quite safe here.

Yes. QEMU uses a fixed offset 0x30 into the RTAS region, because the
first few bytes of the RTAS region is used by
pc-bios/spapr-rtas/spapr-rtas.S.

> 
> This is also my understanding from Aravinda, in the grand tradition of
> PAPR imposing all the performance penalty of paravirtualization, with
> none of the abstraction of paravirtualization.
> 
> I just had a look at PAPR and it's kind of complicated, but amounts to
> requiring this in practice.  The wording implies that the RTAS space
> should be used.  The mechanics don't exactly require this, since the
> guest does receive an address containing (amongst other things) the
> error log when it gets an NMI notification.  But, AFAICT, there's not
> really any other option: the page at 0x7000 is reserved for FWNMI
> stuff, but use of that is deprecated, leaving the RTAS space as
> basically the only other firmware-owned piece of memory to use for
> this.  There's no way for the guest to supply a buffer, which would be
> the obvious sensible way of doing this.

PAPR (section 7.3.14 FWNMI) mentions that the RTAS space should be used
to save FWNMI error logs. The one that allows FWNMI error log to be
placed at 0x7000 is deprecated. So RTAS space is the only option if we
want to adhere to PAPR.

Also the kernel checks if the error log is in the RTAS area before
reading it.

Regards,
Aravinda


> 
>>>> Signed-off-by: Alexey Kardashevskiy <aik at ozlabs.ru>
>>>> ---
>>>>   lib/libhvcall/hvcall.S | 2 ++
>>>>   1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/lib/libhvcall/hvcall.S b/lib/libhvcall/hvcall.S
>>>> index b19f6dbeff2c..adcda0dd905d 100644
>>>> --- a/lib/libhvcall/hvcall.S
>>>> +++ b/lib/libhvcall/hvcall.S
>>>> @@ -134,6 +134,7 @@ ENTRY(hv_rtas)
>>>>   	ori	r3,r3,KVMPPC_H_RTAS at l
>>>>   	HVCALL
>>>>   	blr
>>>> +	.space 2048 - (. - hv_rtas)
>>>>   	.globl hv_rtas_size
>>>>   hv_rtas_size:
>>>>   	.long . - hv_rtas;
>>>> @@ -144,6 +145,7 @@ ENTRY(hv_rtas_broken_sc1)
>>>>   	ori	r3,r3,KVMPPC_H_RTAS at l
>>>>   	.long	0x7c000268
>>>>   	blr
>>>> +	.space 2048 - (. - hv_rtas_broken_sc1)
>>>>   	.globl hv_rtas_broken_sc1_size
>>>>   hv_rtas_broken_sc1_size:
>>>>   	.long . - hv_rtas_broken_sc1;
>>>>
>>>
>>> Can you please at least add some comments to the code which explain the
>>> padding - otherwise it's pretty impossible to understand when you look
>>> at the code later.
>>
>> Sure can do that, how about:
>>
>> /*
>>  * A space reserved for a RTAS log from Firmware Assisted
>>  * Non-Maskable Interrupts Option (FWNMI) feature.
>>  *
>>  * The QEMU implementation uses 0x30..0x800 for the log.
>>  */
>>
>> ?
>>
>>
> 

-- 
Regards,
Aravinda