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

Shannon Zhao posted 1 patch 6 years, 9 months ago
Test docker-clang@ubuntu passed
Test docker-mingw@fedora passed
Test asan passed
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190117030917.18844-1-shannon.zhaosl@gmail.com
Maintainers: Peter Maydell <peter.maydell@linaro.org>
hw/intc/arm_gic_kvm.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
[Qemu-devel] [PATCH] ARM: GIC: error out early when creating VGICv2 failed
Posted by Shannon Zhao 6 years, 9 months ago
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
Posted by Andrew Jones 6 years, 9 months ago
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
Posted by Peter Maydell 6 years, 9 months ago
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