lkml.org 
[lkml]   [2021]   [Oct]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] firmware: arm_sdei: pass sdei_api_event_register right parameters
From
Date
Hi James,

在 2021/10/9 1:39, James Morse 写道:
> Hello!
>
> (sorry for the delayed response)
>
> On 10/09/2021 05:01, Liguang Zhang wrote:
>> Function _local_event_enable is used for private sdei event
>> registeration called by sdei_event_register. We should pass
> (registration)
>
>> sdei_api_event_register right flag and mpidr parameters, otherwise atf
>> may trigger assert errors.
>> diff --git a/drivers/firmware/arm_sdei.c b/drivers/firmware/arm_sdei.c
>> index a7e762c352f9..0736752dadde 100644
>> --- a/drivers/firmware/arm_sdei.c
>> +++ b/drivers/firmware/arm_sdei.c
>> @@ -558,14 +558,16 @@ static int sdei_api_event_register(u32 event_num, unsigned long entry_point,
>> static void _local_event_register(void *data)
>> {
>> int err;
>> + u64 mpidr;
>> struct sdei_registered_event *reg;
>> struct sdei_crosscall_args *arg = data;
>>
>> WARN_ON(preemptible());
>>
>> + mpidr = read_cpuid_mpidr();
>> reg = per_cpu_ptr(arg->event->private_registered, smp_processor_id());
>> err = sdei_api_event_register(arg->event->event_num, sdei_entry_point,
>> - reg, 0, 0);
>> + reg, SDEI_EVENT_REGISTER_RM_PE, mpidr);
> Hmmm, this looks like a bug in TFA.
>
> 5.1.2.2 "Parameters" of DEN 0054B has:
> | Routing mode is valid only for a shared event. For a private event, the routing mode is
> | ignored.
>
> Worse, the mpidr field has:
> | Currently the format is defined only when the selected routing mode is RM_PE.

or a private event, we route SDEI_EVENT_REGISTER_RM_PE and mpidr
parameters may be more rationable.


>
>
> Over in trusted firmware land:
>
> https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/tree/services/std_svc/sdei/sdei_main.c?h=v2.5#n361
>
> | static int64_t sdei_event_register(int ev_num,
> | uint64_t ep,
> | uint64_t arg,
> | uint64_t flags,
> | uint64_t mpidr)
> | {
>
> | /* Private events always target the PE */
> | if (is_event_private(map))
> | flags = SDEI_REGF_RM_PE;
>
> It looks like this re-uses the 'caller specified the routing' code, but didn't update the
> mpidr.
>
>
> You mention TFA takes an assert failure, I assume that brings the machine down.
> (Presumably you don't have a CPU with an affinity of zero.)

Yes, that brings the machine down. In opensource ATF, CPU with an
affinity of zero.

The problem backaround:

we use local secure arch timer as sdei watchdog timer for hardlockup
detection, in  os panic flow, we mask sdei event, then trigger the assert


if (se->reg_flags == SDEI_REGF_RM_PE)

    assert(se->affinity == my_mpidr);


Thanks,

Liguang

>
> Does this mean no-one relies on this, and we can fix the firmware?
> (I'll go report this to the relevant folk)
>
>
> Thanks!
>
> James
>
>
>>
>> sdei_cross_call_return(arg, err);
>> }
>>

\
 
 \ /
  Last update: 2021-10-11 12:39    [W:0.071 / U:0.800 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site