Re: [PATCH 11/11] kvmtool: add command line parameter to instantiate a vGICv3

[Andre Przywara <andre.przywara@arm.com>]

Hi Will,

On 26/01/15 11:30, Will Deacon wrote:
> On Fri, Jan 23, 2015 at 04:35:10PM +0000, Andre Przywara wrote:
>> Add the command line parameter "--gicv3" to request GICv3 emulation
>> in the kernel. Connect that to the already existing GICv3 code.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>> ---
>>  tools/kvm/arm/aarch64/arm-cpu.c                    |    5 ++++-
>>  .../kvm/arm/aarch64/include/kvm/kvm-config-arch.h  |    4 +++-
>>  tools/kvm/arm/gic.c                                |   14 ++++++++++++++
>>  tools/kvm/arm/include/arm-common/kvm-config-arch.h |    1 +
>>  tools/kvm/arm/kvm-cpu.c                            |    2 +-
>>  tools/kvm/arm/kvm.c                                |    3 ++-
>>  6 files changed, 25 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/kvm/arm/aarch64/arm-cpu.c b/tools/kvm/arm/aarch64/arm-cpu.c
>> index a70d6bb..46d6d6a 100644
>> --- a/tools/kvm/arm/aarch64/arm-cpu.c
>> +++ b/tools/kvm/arm/aarch64/arm-cpu.c
>> @@ -12,7 +12,10 @@
>>  static void generate_fdt_nodes(void *fdt, struct kvm *kvm, u32 gic_phandle)
>>  {
>>  	int timer_interrupts[4] = {13, 14, 11, 10};
>> -	gic__generate_fdt_nodes(fdt, gic_phandle, KVM_DEV_TYPE_ARM_VGIC_V2);
>> +	gic__generate_fdt_nodes(fdt, gic_phandle,
>> +				kvm->cfg.arch.gicv3 ?
>> +					KVM_DEV_TYPE_ARM_VGIC_V3 :
>> +					KVM_DEV_TYPE_ARM_VGIC_V2);
>>  	timer__generate_fdt_nodes(fdt, kvm, timer_interrupts);
>>  }
>>  
>> diff --git a/tools/kvm/arm/aarch64/include/kvm/kvm-config-arch.h b/tools/kvm/arm/aarch64/include/kvm/kvm-config-arch.h
>> index 89860ae..106e52f 100644
>> --- a/tools/kvm/arm/aarch64/include/kvm/kvm-config-arch.h
>> +++ b/tools/kvm/arm/aarch64/include/kvm/kvm-config-arch.h
>> @@ -3,7 +3,9 @@
>>  
>>  #define ARM_OPT_ARCH_RUN(cfg)						\
>>  	OPT_BOOLEAN('\0', "aarch32", &(cfg)->aarch32_guest,		\
>> -			"Run AArch32 guest"),
>> +			"Run AArch32 guest"),				\
>> +	OPT_BOOLEAN('\0', "gicv3", &(cfg)->gicv3,			\
>> +			"Use a GICv3 interrupt controller in the guest"),
> 
> On a GICv3-capable system, why would I *not* want to enable this option?
> In other words, could we make this the default behaviour on systems that
> support it, and if you need an override then it should be something like
> --force-gicv2.

Well, you could have a guest kernel < 3.17, which does not have GICv3
support. In general I consider GICv2 better tested, so I reckon that
people will only want to use GICv3 emulation if there is a need for it
(non-compat GICv3 host or more than 8 VCPUs in the guest). That probably
changes over time, but for the time being I'd better keep the default at
GICv2 emulation.

Having said that, there could be a fallback in case GICv2 emulation is
not available. Let me take a look at that.
Also thinking about the future (ITS emulation) I found that I'd like to
replace this option with something more generic like --irqchip=.

Cheers,
Andre.