On 2017-09-05 13:49, Ralf Ramsauer wrote:
> 
> 
> On 09/04/2017 08:54 PM, Jan Kiszka wrote:
>> From: Jan Kiszka <[email protected]>
>>
>> In case of mode 2, the targets field won't be evaluated. So we can safe
>> one statement and rather invest in explaining when we need to adjust
>> targets.>
>> Signed-off-by: Jan Kiszka <[email protected]>
>> ---
>>  hypervisor/arch/arm-common/irqchip.c | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/hypervisor/arch/arm-common/irqchip.c 
>> b/hypervisor/arch/arm-common/irqchip.c
>> index 4103fed4..89097ef2 100644
>> --- a/hypervisor/arch/arm-common/irqchip.c
>> +++ b/hypervisor/arch/arm-common/irqchip.c
>> @@ -136,7 +136,6 @@ void gic_handle_sgir_write(struct sgi *sgi, bool 
>> virt_input)
>>      if (sgi->routing_mode == 2) {
>>              /* Route to the caller itself */
>>              irqchip_set_pending(cpu_data, sgi->id);
>> -            sgi->targets = (1 << cpu_data->cpu_id);
>>      } else {
>>              sgi->targets = 0;
> Just for simplicity: can't we execute this statement unconditionally?
> 
> In case of mode 2, sgi->targets doesn't matter, and we would make sure
> that the GIC is not filled with any confusing value, in case of mode 0
> and 1 we need it to be zeroed before filling it. So we can zero it in
> any case.
> 

Yes, seems reasonable. I can send a version 2 of this patch as well.

> 
> One further question as I just read the irqchip code.
> 
> If we receive an SGI in routing mode 0 and the caller tries to route the
> SGI outside of the cell, we're simply ignoring the request, as we're
> only iterating over the cell's cpu_set.
> 
> Shouldn't we panic the cell in this case, as we do if someone tries to
> route an IRQ outside of the cell?

I think we can follow the GIC spec here:

"If a bit is 1 and the bit does not correspond to a valid target PE, the
bit must be ignored by the Distributor. It is IMPLEMENTATION DEFINED
whether, in such cases, a Distributor can signal a system error."

Let's define we don't signal an error, at least not a fatal one. I'm not
even sure how such error could be signaled without falling back to the
hypervisor console (which is not accessible by non-root cells).

Jan

-- 
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux

-- 
You received this message because you are subscribed to the Google Groups 
"Jailhouse" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to