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

Marc Zyngier
2017-01-06T10:24:04Z

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.

Thoughts?

M.
--
Jazz is not dead. It just smells funny...