Re: [Xen-devel] [RFC PATCH 43/49] ARM: new VGIC: Add preliminary stub implementations

[Andre Przywara <andre.przywara@xxxxxxxxxx>]

Hi,

On 19/02/18 12:34, Julien Grall wrote:
> Hi,
> 
> On 09/02/18 14:39, Andre Przywara wrote:
>> The Xen core code requires an interrupt controller emulation to implement
>> arch_move_irqs(), to move the affinity of an hardware mapped virtual IRQ
>> to another core. In the moment we don't implement this
>> physical-follow-virtual regime in our new VGIC, so just provide an empty
>> stub implementation to make the linker happy.
> 
> physical-follow-virtual is a must feature for the new vGIC. This has
> shown better interrupt latency.
> 
>> Similarily vgic_clear_pending_irqs() is required by the ARM code,
>> although it is suspected that it is actually not necessary. Go with a
>> stub for now.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxxxxx>
>> ---
>>   xen/arch/arm/vgic/vgic.c | 13 +++++++++++++
>>   1 file changed, 13 insertions(+)
>>
>> diff --git a/xen/arch/arm/vgic/vgic.c b/xen/arch/arm/vgic/vgic.c
>> index d91028bd43..77fa756329 100644
>> --- a/xen/arch/arm/vgic/vgic.c
>> +++ b/xen/arch/arm/vgic/vgic.c
>> @@ -770,6 +770,19 @@ void gic_dump_vgic_info(struct vcpu *v)
>>                  irq->active ? "" : "not ", irq->enabled ? "" : "not ");
>>   }
>>   +void vgic_clear_pending_irqs(struct vcpu *v)
>> +{
>> +    /*
>> +     * TODO: It is unclear whether we really need this, so we might
>> instead
>> +     * remove it on the caller site.
>> +     */
> 
> I remember some issue with the current vGIC when not removing pending
> interrupts on PSCI CPU ON. But that was a while ago. I will have another
> try and see if we can drop it.

So that's what I came up with:
CPU_ON can be called from two states:
- The initial state of a core before it's being brought up for the first
time.
- After the OS has called CPU_OFF.
 The PSCI spec says that before calling CPU_OFF an OS all IRQs must have
been migrated away from that core. I take this as no IRQs are allowed,
hence we don't have to clear anything on CPU_ON.

In both cases the CPU is expected to enter in a defined state, which
includes all interrupts masked on the CPU level (SPSR.ELx.[DAIF] = 1).
The GIC defaults to ISPENDR being 0.

So I take we should not be held responsible for clearing the pending
state upon CPU_ON.

What is your opinion?

>> +}
>> +
>> +void arch_move_irqs(struct vcpu *v)
>> +{
>> +    /* TODO: implement this (?) */
> 
> Here you would need to go through the interrupt and modify the affinity
> of the physical IRQs routed to that vCPU.

Since this is apparently the next best thing after sliced bread, I
implemented this now. Coming to a theatre near you any time soon.

> 
>> +}
>> +
>>   struct irq_desc *vgic_get_hw_irq_desc(struct domain *d, struct vcpu *v,
>>                                         unsigned int virq)
>>   {
>>
> 
> Cheers,
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel