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 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.) thanks -- PMM
