Re: [Qemu-devel] [PATCH] target/arm/kvm: gic: Prevent creating userspace GICv3 with KVM

[Peter Maydell]

On 1 February 2018 at 20:53, Christoffer Dall
 wrote:
> KVM doesn't support emulating a GICv3 in userspace, only GICv2.  We
> currently attempt this anyway, and as a result a KVM guest doesn't
> receive interrupts and the user is left wondering why.  Report an error
> to the user if this particular combination is requested.
>
> Signed-off-by: Christoffer Dall 
> ---
>  target/arm/kvm_arm.h | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
> index ff53e9fafb..cfb7e5af72 100644
> --- a/target/arm/kvm_arm.h
> +++ b/target/arm/kvm_arm.h
> @@ -234,6 +234,10 @@ static inline const char *gicv3_class_name(void)
>  exit(1);
>  #endif
>  } else {
> +if (kvm_enabled()) {
> +error_report("Userspace GICv3 is not supported with KVM");
> +exit(1);
> +}
>  return "arm-gicv3";
>  }



Applied to target-arm.next, thanks.

-- PMM

Re: [Qemu-devel] [PATCH] target/arm/kvm: gic: Prevent creating userspace GICv3 with KVM

[Christoffer Dall]

Hi Philippe,

On Mon, Feb 05, 2018 at 09:10:48PM -0300, Philippe Mathieu-Daudé wrote:
> 
> On 02/01/2018 05:53 PM, Christoffer Dall wrote:
> > KVM doesn't support emulating a GICv3 in userspace, only GICv2.  We
> > currently attempt this anyway, and as a result a KVM guest doesn't
> > receive interrupts and the user is left wondering why.  Report an error
> > to the user if this particular combination is requested.
> > 
> > Signed-off-by: Christoffer Dall 
> > ---
> >  target/arm/kvm_arm.h | 4 
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
> > index ff53e9fafb..cfb7e5af72 100644
> > --- a/target/arm/kvm_arm.h
> > +++ b/target/arm/kvm_arm.h
> > @@ -234,6 +234,10 @@ static inline const char *gicv3_class_name(void)
> 
> Any reason why these functions are inlined in this include?

Not really sure what you mean?  What do you think would be a better
approach?

> 
> Also - not related to your patch - while gicv3_class_name() is
> self-explicit, gic_class_name() isn't (to me at least).

I think I tend to see 'gic' as GICv2 and 'gicv3' as GICv3 in QEMU, but I
don't remember the details of how you make the GIC be a proper GICv2 vs.
the 11mpcore version thingy.

> 
> There are many check for gic_version 1/2/3 in virt.c, which we could
> clean using more generic functions such gic_class_name(int gic_version)
> and Co.

Possibly.  Do note, that checking "which version of the GIC does this
board contain" is a separate thing from "which particular mechanism do I
use to emulate the GIC".

> 
> >  exit(1);
> >  #endif
> >  } else {
> > +if (kvm_enabled()) {
> > +error_report("Userspace GICv3 is not supported with KVM");
> > +exit(1);
> 
> Reviewed-by: Philippe Mathieu-Daudé 
> 
> > +}
> >  return "arm-gicv3";
> >  }
> >  }
> > 
> 
Thanks!
-Christoffer


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH] target/arm/kvm: gic: Prevent creating userspace GICv3 with KVM

[Alistair Francis]

On Mon, Feb 5, 2018 at 4:10 PM, Philippe Mathieu-Daudé  wrote:
> Hi Christoffer,
>
> On 02/01/2018 05:53 PM, Christoffer Dall wrote:
>> KVM doesn't support emulating a GICv3 in userspace, only GICv2.  We
>> currently attempt this anyway, and as a result a KVM guest doesn't
>> receive interrupts and the user is left wondering why.  Report an error
>> to the user if this particular combination is requested.
>>
>> Signed-off-by: Christoffer Dall 
>> ---
>>  target/arm/kvm_arm.h | 4 
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
>> index ff53e9fafb..cfb7e5af72 100644
>> --- a/target/arm/kvm_arm.h
>> +++ b/target/arm/kvm_arm.h
>> @@ -234,6 +234,10 @@ static inline const char *gicv3_class_name(void)
>
> Any reason why these functions are inlined in this include?
>
> Also - not related to your patch - while gicv3_class_name() is
> self-explicit, gic_class_name() isn't (to me at least).
>
> There are many check for gic_version 1/2/3 in virt.c, which we could
> clean using more generic functions such gic_class_name(int gic_version)
> and Co.
>
> Alistair/Edgar:
> Why the ZynqMP is not using the GICv3 for the APU? I remember hearing
> something about not fully implemented so not working correctly.

The ZynqMP only has a GICv2.

Maybe you are thinking of the virtualisation features? Mainline
doesn't have those fully implemented so they don't work correctly.

Alistair

>
>>  exit(1);
>>  #endif
>>  } else {
>> +if (kvm_enabled()) {
>> +error_report("Userspace GICv3 is not supported with KVM");
>> +exit(1);
>
> Reviewed-by: Philippe Mathieu-Daudé 
>
>> +}
>>  return "arm-gicv3";
>>  }
>>  }
>>
>
> Regards,
>
> Phil.
>

Re: [Qemu-devel] [PATCH] target/arm/kvm: gic: Prevent creating userspace GICv3 with KVM

[Philippe Mathieu-Daudé]

Hi Christoffer,

On 02/01/2018 05:53 PM, Christoffer Dall wrote:
> KVM doesn't support emulating a GICv3 in userspace, only GICv2.  We
> currently attempt this anyway, and as a result a KVM guest doesn't
> receive interrupts and the user is left wondering why.  Report an error
> to the user if this particular combination is requested.
> 
> Signed-off-by: Christoffer Dall 
> ---
>  target/arm/kvm_arm.h | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
> index ff53e9fafb..cfb7e5af72 100644
> --- a/target/arm/kvm_arm.h
> +++ b/target/arm/kvm_arm.h
> @@ -234,6 +234,10 @@ static inline const char *gicv3_class_name(void)

Any reason why these functions are inlined in this include?

Also - not related to your patch - while gicv3_class_name() is
self-explicit, gic_class_name() isn't (to me at least).

There are many check for gic_version 1/2/3 in virt.c, which we could
clean using more generic functions such gic_class_name(int gic_version)
and Co.

Alistair/Edgar:
Why the ZynqMP is not using the GICv3 for the APU? I remember hearing
something about not fully implemented so not working correctly.

>  exit(1);
>  #endif
>  } else {
> +if (kvm_enabled()) {
> +error_report("Userspace GICv3 is not supported with KVM");
> +exit(1);

Reviewed-by: Philippe Mathieu-Daudé 

> +}
>  return "arm-gicv3";
>  }
>  }
> 

Regards,

Phil.



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH] target/arm/kvm: gic: Prevent creating userspace GICv3 with KVM

[Christoffer Dall]

KVM doesn't support emulating a GICv3 in userspace, only GICv2.  We
currently attempt this anyway, and as a result a KVM guest doesn't
receive interrupts and the user is left wondering why.  Report an error
to the user if this particular combination is requested.

Signed-off-by: Christoffer Dall 
---
 target/arm/kvm_arm.h | 4 
 1 file changed, 4 insertions(+)

diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index ff53e9fafb..cfb7e5af72 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -234,6 +234,10 @@ static inline const char *gicv3_class_name(void)
 exit(1);
 #endif
 } else {
+if (kvm_enabled()) {
+error_report("Userspace GICv3 is not supported with KVM");
+exit(1);
+}
 return "arm-gicv3";
 }
 }
-- 
2.14.2