Re: [PATCH 2/2] Detect vGIC presence at runtime

[Christoffer Dall <christoffer.dall@linaro.org>]

On Thu, Jul 09, 2015 at 03:50:49PM +0300, Pavel Fedin wrote:
>  Hello!
> 
> > why not report ENXIO as an error?  If probing the vgic fails due to
> > being unable to request the irq or something similar, then surely your
> > system has and error and this should be reported.
> 
>  It is reported by probe function itself.
>  -ENODEV here means there's no GIC at all. -ENXIO happens when, for example, there is GIC node in
> the device tree, but it does not specify vGIC resources. Normally this means that vGIC is defunct on
> the machine.

I'd like to distinguish between the 'missing vgic' and 'something bad
happened when trying to initialize the vgic' cases, which I don't think
we do currently, because the ENXIO code is used in various situations.

> 
> > This may be more nicely implemented by letting the vgic init/probe
> > functions set the vgic_present, or maybe better yet, just export a
> > function from vgic.c:
> > 
> > bool kvm_vgic_present(void)
> > {
> > 	return vgic_ops != NULL;
> > }
> 
>  Is it necessary? Actually this flag is not needed anywhere else except arch/arm/kvm/arm.c, only at
> init time. Runtime should, i believe, use irqchip_in_kernel(), because userland can choose just not
> to use vGIC for some reason (testing for example).
> 
I feel the init flow is relatively difficult to follow and adding a
bunch of flags here and there doesn't seem to help.  By adding a
function with a proper comment, it should be more clear, and I don't
like the switch statement on the error return values.

-Christoffer