| Message ID | 20180201205307.30343-1-christoffer.dall@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | target/arm/kvm: gic: Prevent creating userspace GICv3 with KVM | expand |
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 <christoffer.dall@linaro.org> > --- > 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é <f4bug@amsat.org> > + } > return "arm-gicv3"; > } > } > Regards, Phil.
On Mon, Feb 5, 2018 at 4:10 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> 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 <christoffer.dall@linaro.org> >> --- >> 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é <f4bug@amsat.org> > >> + } >> return "arm-gicv3"; >> } >> } >> > > Regards, > > Phil. >
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 <christoffer.dall@linaro.org> > > --- > > 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é <f4bug@amsat.org> > > > + } > > return "arm-gicv3"; > > } > > } > > > Thanks! -Christoffer
On 1 February 2018 at 20:53, Christoffer Dall <christoffer.dall@linaro.org> 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 <christoffer.dall@linaro.org> > --- > 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
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"; } }
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 <christoffer.dall@linaro.org> --- target/arm/kvm_arm.h | 4 ++++ 1 file changed, 4 insertions(+) -- 2.14.2