Re: [RFC PATCH 4/7] KVM: arm/arm64: Check that system supports split eoi/deactivate

Christoffer Dall
2017-01-06T10:53:22Z

On Fri, Jan 06, 2017 at 10:24:04AM +0000, Marc Zyngier wrote:
> On 06/01/17 10:02, Christoffer Dall wrote:
> > On Thu, Jan 05, 2017 at 05:40:58PM +0000, Marc Zyngier wrote:
> >> On 10/12/16 20:47, Christoffer Dall wrote:
> >>> Some systems without proper firmware and/or hardware description data
> >>> don't support the split EOI and deactivate operation and therefore
> >>> don't provide an irq_set_vcpu_affinity implementation. On such
> >>> systems, we cannot leave the physical interrupt active after the timer
> >>> handler on the host has run, so we cannot support KVM with the timer
> >>> changes we about to introduce.
> >>>
> >>> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
> >>> ---
> >>> virt/kvm/arm/arch_timer.c | 30 ++++++++++++++++++++++++++++++
> >>> 1 file changed, 30 insertions(+)
> >>>
> >>> diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
> >>> index c7c3bfd..f27a086 100644
> >>> --- a/virt/kvm/arm/arch_timer.c
> >>> +++ b/virt/kvm/arm/arch_timer.c
> >>> @@ -418,6 +418,31 @@ static int kvm_timer_dying_cpu(unsigned int cpu)
> >>> return 0;
> >>> }
> >>>
> >>> +static bool has_split_eoi_deactivate_support(void)
> >>> +{
> >>> + struct irq_desc *desc;
> >>> + struct irq_data *data;
> >>> + struct irq_chip *chip;
> >>> +
> >>> + /*
> >>> + * Check if split EOI and deactivate is supported on this machine.
> >>> + */
> >>> + desc = irq_to_desc(host_vtimer_irq);
> >>> + if (!desc) {
> >>> + kvm_err("kvm_arch_timer: no host_vtimer_irq descriptor\n");
> >>> + return false;
> >>> + }
> >>> +
> >>> + data = irq_desc_get_irq_data(desc);
> >>> + chip = irq_data_get_irq_chip(data);
> >>> + if (!chip || !chip->irq_set_vcpu_affinity) {
> >>> + kvm_err("kvm_arch_timer: no split EOI/deactivate; abort\n");
> >>> + return false;
> >>> + }
> >>> +
> >>> + return true;
> >>> +}
> >>
> >> That feels really involved. How about reporting that we don't have a
> >> usable VGIC altogether from the GIC driver?
> >>
> >
> > You mean not booting the kernel at all, or establish some communication
> > from the GIC driver to KVM, telling KVM if it's usable for
> > virtualization or not?
>
> We already query the vgic configuration by calling gic_get_kvm_info() at
> boot time. My suggestion is to make it return NULL if the GIC can't
> support split EOI/deactivate, and fallback to the "no-vgic" mode, if
> ever implemented.
>

That sounds good to me.

Thanks,
-Christoffer