On Wed, Aug 12, 2015 at 2:59 PM, Peter Maydell <peter.mayd...@linaro.org> wrote: > On 12 August 2015 at 13:27, Pavel Fedin <p.fe...@samsung.com> wrote: >> Hello! >> >>> I still think this is the wrong approach -- see my remarks >>> in the previous round of patch review. >> >> You know... I thought a little bit... >> So far, test = true in KVM_CREATE_DEVICE means that we just want to know >> whether this type is supported. No actual actions is done by the kernel. Is >> it correct? If yes, we can just leave this test as it is, because if it says >> that GICv2 is supported by KVM_CREATE_DEVICE, then: >> 1. We use new API. No KVM_IRQCHIP_CREATE. >> 2. GICv3 may be supported. >> >> Therefore, if we do this check, and it succeeds, then we just >> proceed, and later actually try to create GICv3. If it fails for some >> reason, we will see error message anyway. So would it be OK just not >> to touch kvm_arch_irqchip_create() at all? > > No, because then if the kernel only supports GICv3 the > code in kvm_arch_irqchip_create() (as it is currently written) > will erroneously fall back to using the old API. > > Christoffer: the question was, why does kvm_arch_irqchip_create() > not only check the KVM_CAP_DEVICE_CTRL but also try to see > if it can KVM_CREATE_DEVICE the GICv2 in order to avoid > falling back to the old pre-KVM_CREATE_DEVICE API ? Are there > kernels which have the capability bit set but which can't > actually use KVM_CREATE_DEVICE to create the irqchip?
My thinking probably was that technically the KVM_CAP_DEVICE_CTRL is an orthogonal concept from how to create the vgic, and you could advertise this capability without also supporting the GICv2 device type. However, I don't believe such kernels exist and they cannot in the future as that would be because we would remove an actively supported API. > > My preference here would be for kvm_arch_irqchip_create() > to just use 'is the KVM_CAP_DEVICE_CTRL capability set' > for its "can we use the new API" test; that will then > work whether we have a GICv2 or GICv3 in the host. (The > actual GIC device creation later on might fail, of course, > but that can be handled at that point. The only thing > we need to do as early as kvm_arch_irqchip_create is > determine whether we must use the old API.) > I'm fine with this. -Christoffer