Re: [Xen-devel] [RFC PATCH 25/49] ARM: new VGIC: Add GICv2 world switch backend

[Andre Przywara <andre.przywara@xxxxxxxxxx>]

Hi,

forgot to mention:

On 13/02/18 14:31, Julien Grall wrote:
> Hi,
> 
> On 09/02/18 14:39, Andre Przywara wrote:
>> Processing maintenance interrupts and accessing the list registers
>> are dependent on the host's GIC version.
>> Introduce vgic-v2.c to contain GICv2 specific functions.
>> Implement the GICv2 specific code for syncing the emulation state
>> into the VGIC registers.
>> This also adds the hook to let Xen setup the host GIC addresses.
>>
>> This is based on Linux commit 140b086dd197, written by Marc Zyngier.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxxxxx>
>> ---
>>   xen/arch/arm/vgic/vgic-v2.c | 261
>> ++++++++++++++++++++++++++++++++++++++++++++
>>   xen/arch/arm/vgic/vgic.c    |  20 ++++
>>   xen/arch/arm/vgic/vgic.h    |   8 ++
>>   3 files changed, 289 insertions(+)
>>   create mode 100644 xen/arch/arm/vgic/vgic-v2.c
>>
>> diff --git a/xen/arch/arm/vgic/vgic-v2.c b/xen/arch/arm/vgic/vgic-v2.c
>> new file mode 100644
>> index 0000000000..10fc467ffa
>> --- /dev/null
>> +++ b/xen/arch/arm/vgic/vgic-v2.c

....

>> +void vgic_v2_save_state(struct vcpu *vcpu)
>> +{
>> +    u64 used_lrs = vcpu->arch.vgic_cpu.used_lrs;
>> +
>> +    if ( used_lrs )
>> +    {
>> +        save_lrs(vcpu, gic_v2_hw_data.hbase);
>> +        writel_relaxed(0, gic_v2_hw_data.hbase + GICH_HCR);
>> +    }
>> +}
> 
> I am not entirely convinced that have a separate function to save the
> LRs is necessary. This could be done in fold_lr_state().
> 
>> +
>> +void vgic_v2_restore_state(struct vcpu *vcpu)
>> +{
>> +    struct vgic_v2_cpu_if *cpu_if = &vcpu->arch.vgic_cpu.vgic_v2;
>> +    u64 used_lrs = vcpu->arch.vgic_cpu.used_lrs;
>> +    int i;
>> +
>> +    if ( used_lrs )
>> +    {
>> +        writel_relaxed(cpu_if->vgic_hcr,
>> +                       gic_v2_hw_data.hbase + GICH_HCR);
>> +        for ( i = 0; i < used_lrs; i++ )
>> +            writel_relaxed(cpu_if->vgic_lr[i],
>> +                           gic_v2_hw_data.hbase + GICH_LR0 + (i * 4));
>> +    }
> 
> Same here but with populate_lr_state(). This would make the code easier
> to follow and also avoid a lot ifery in the vgic.c code.

This is mostly due to KVM's inability to directly access the GICv3 LRs
when running in EL1. I will take a look whether what it would take to
merge this. Sounds tempting, but there might be side effects.

Cheers,
Andre.

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