[Qemu-devel] [PATCH] ARM: GIC: error out early when creating VGICv2 failed

[Qemu-devel] [PATCH] ARM: GIC: error out early when creating VGICv2 failed

[Shannon Zhao]

Currently if creating a gicv2 VM on the platform without
VGICv2 support, it failed in kvm_arm_pmu_set_irq while
trying to set the interrupt number for PMU with below
error message.
KVM_SET_DEVICE_ATTR failed: Invalid argument

This error log is a little confused. Also it should error
out in kvm_arm_gic_realize early.

Signed-off-by: Shannon Zhao <shannon.zhaosl@gmail.com>
---
 hw/intc/arm_gic_kvm.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c
index a611e8e..b4f2133 100644
--- a/hw/intc/arm_gic_kvm.c
+++ b/hw/intc/arm_gic_kvm.c
@@ -554,8 +554,9 @@ static void kvm_arm_gic_realize(DeviceState *dev, Error **errp)
                               KVM_DEV_ARM_VGIC_CTRL_INIT, NULL, true,
                               &error_abort);
         }
-    } else if (ret != -ENODEV && ret != -ENOTSUP) {
-        error_setg_errno(errp, -ret, "error creating in-kernel VGIC");
+    } else if (ret != -ENOTSUP) {
+        error_setg_errno(errp, -ret, "error creating in-kernel VGIC, maybe "
+                                     "vGICv2 not support on this platform");
         return;
     }
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH] ARM: GIC: error out early when creating VGICv2 failed

[Andrew Jones]

On Thu, Jan 17, 2019 at 11:09:17AM +0800, Shannon Zhao wrote:
> Currently if creating a gicv2 VM on the platform without
> VGICv2 support, it failed in kvm_arm_pmu_set_irq while
> trying to set the interrupt number for PMU with below
> error message.
> KVM_SET_DEVICE_ATTR failed: Invalid argument
> 
> This error log is a little confused. Also it should error
> out in kvm_arm_gic_realize early.
> 
> Signed-off-by: Shannon Zhao <shannon.zhaosl@gmail.com>
> ---
>  hw/intc/arm_gic_kvm.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c
> index a611e8e..b4f2133 100644
> --- a/hw/intc/arm_gic_kvm.c
> +++ b/hw/intc/arm_gic_kvm.c
> @@ -554,8 +554,9 @@ static void kvm_arm_gic_realize(DeviceState *dev, Error **errp)
>                                KVM_DEV_ARM_VGIC_CTRL_INIT, NULL, true,
>                                &error_abort);
>          }
> -    } else if (ret != -ENODEV && ret != -ENOTSUP) {
> -        error_setg_errno(errp, -ret, "error creating in-kernel VGIC");
> +    } else if (ret != -ENOTSUP) {
> +        error_setg_errno(errp, -ret, "error creating in-kernel VGIC, maybe "
> +                                     "vGICv2 not support on this platform");

vGICv2 is not supported on this platform

>          return;
>      }
>  
> -- 
> 1.8.3.1
> 
> 

With the error message change

Reviewed-by: Andrew Jones <drjones@redhat.com>

Re: [Qemu-devel] [PATCH] ARM: GIC: error out early when creating VGICv2 failed

[Peter Maydell]

On Thu, 17 Jan 2019 at 03:09, Shannon Zhao <shannon.zhaosl@gmail.com> wrote:
>
> Currently if creating a gicv2 VM on the platform without
> VGICv2 support, it failed in kvm_arm_pmu_set_irq while
> trying to set the interrupt number for PMU with below
> error message.
> KVM_SET_DEVICE_ATTR failed: Invalid argument
>
> This error log is a little confused. Also it should error
> out in kvm_arm_gic_realize early.
>
> Signed-off-by: Shannon Zhao <shannon.zhaosl@gmail.com>
> ---
>  hw/intc/arm_gic_kvm.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c
> index a611e8e..b4f2133 100644
> --- a/hw/intc/arm_gic_kvm.c
> +++ b/hw/intc/arm_gic_kvm.c
> @@ -554,8 +554,9 @@ static void kvm_arm_gic_realize(DeviceState *dev, Error **errp)
>                                KVM_DEV_ARM_VGIC_CTRL_INIT, NULL, true,
>                                &error_abort);
>          }
> -    } else if (ret != -ENODEV && ret != -ENOTSUP) {
> -        error_setg_errno(errp, -ret, "error creating in-kernel VGIC");
> +    } else if (ret != -ENOTSUP) {
> +        error_setg_errno(errp, -ret, "error creating in-kernel VGIC, maybe "
> +                                     "vGICv2 not support on this platform");
>          return;
>      }

Shouldn't we be able to tell specifically whether the
vGICv2 is not supported on this platform, without
having to emit an error message that says "we got
some error, maybe it was that there's no vGICv2
support but maybe it was something else" ?

It's not clear to me what the various
possible kvm_create_device returns are. We have
I guess several cases:
 * new device control API supported, vGICv2 supported:
   return >= 0
 * new device control API not supported at all (ie
   old host kernel version): return some error, which one?
 * new device control API supported, but vGICv2 not
   supported: return some error, which one?
 * new device control API supported, vGICv2 supported,
   but something unlikely went wrong in the kernel:
   return any other error

What are the errnos for the middle two cases, and can
we end up with userspace code that can distinguish
these four cases rather than merging some of them
together?

thanks
-- PMM