Hi Peter,
On 3/9/20 2:28 PM, Peter Maydell wrote:
> On Mon, 2 Mar 2020 at 10:55, Eric Auger <eric.au...@redhat.com> wrote:
>>
>> Restructure the finalize_gic_version with switch cases and, in
>> KVM mode, explictly check whether the chosen version is supported
>> by the host.
>>
>> if the end-user explicitly sets v2/v3 and this is not supported by
>> the host, then the user gets an explicit error message.
>>
>> Signed-off-by: Eric Auger <eric.au...@redhat.com>
>> Reviewed-by: Richard Henderson <richard.hender...@linaro.org>
>> Reviewed-by: Andrew Jones <drjo...@redhat.com>
>>
>> ---
>>
>> v2 -> v3:
>> - explictly list V2 and V3 in the switch/case
>> - fix indent
>> ---
>> hw/arm/virt.c | 77 +++++++++++++++++++++++++++++++++++----------------
>> 1 file changed, 53 insertions(+), 24 deletions(-)
>>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index eb8c57c85e..aeb6c45e51 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -1542,33 +1542,62 @@ static void virt_set_memmap(VirtMachineState *vms)
>> */
>> static void finalize_gic_version(VirtMachineState *vms)
>> {
>> - if (vms->gic_version == VIRT_GIC_VERSION_HOST ||
>> - vms->gic_version == VIRT_GIC_VERSION_MAX) {
>> - if (!kvm_enabled()) {
>> - if (vms->gic_version == VIRT_GIC_VERSION_HOST) {
>> - error_report("gic-version=host requires KVM");
>> - exit(1);
>> - } else {
>> - /* "max": currently means 3 for TCG */
>> - vms->gic_version = VIRT_GIC_VERSION_3;
>> - }
>> - } else {
>> - int probe_bitmap = kvm_arm_vgic_probe();
>> + if (kvm_enabled()) {
>> + int probe_bitmap = kvm_arm_vgic_probe();
>
> Previously we would only do kvm_arm_vgic_probe() if the
> user asked for 'host' or 'max'. Now we do it always,
> which means that if the user is on a really old kernel
> where the CREATE_DEVICE ioctl doesn't exist then we
> will now fail if the user specifically asked for gicv2,
> where previously we (probably) would have succeeded.
> I don't think we should put too much weight on continuing
> to theoretically support ancient kernels which we're not
> actually testing against, but it does seem a bit odd to
> probe even if we don't need to know the answer.
>
> More relevant to actual plausible use cases, if
> kvm_irqchip_in_kernel() == false, we shouldn't be
> probing the kernel to ask what kind of GIC to use.
I think the existing code also does the same:
kvm_arm_vgic_probe() gets called as soon as vms->gic_version <= 0 &&
kvm_enabled() whatever the state of kvm_irqchip_in_kernel().
So in case the host only supports GICv2, in kvm mode with userspace
irqchip we would use GICV2 in host/max mode. If host supports GICv3 we
would select GICv3 which is not supported in !kvm_irqchip_in_kernel().
So do I understand correctly that you want me to change that behavior
and always set v2 in KVM/!kvm_irqchip_in_kernel() max/host mode?
Thanks
Eric
>
> thanks
> -- PMM
>
