Hi, On 08/12/2015 03:23 PM, Christoffer Dall wrote: > 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? yes 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 the capability was advertised for arm/arm64 with GICv2 7330672 KVM: arm-vgic: Support KVM_CREATE_DEVICE for VGIC (1 year, 8 months ago) <Christoffer Dall> so effectively I don't think we have any arm kernel advertising the CAP without GICv3.
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.) another way was proposed in the past was consisting in calling first ret = kvm_create_device(s, KVM_DEV_TYPE_ARM_VGIC_V3, true); if this succeeds this means we have the new API (the only one used vy v3) and hence we have it as well for VGIC_V2, we can return ... if this fails we just can say VGICv3 KVM device hasn't registered, try KVM_DEV_TYPE_ARM_VGIC_V2 if we have it return else fall back to older API I think it worked as well? Besides I think Peter's suggestion is simpler. For info we also use KVM VFIO device now. Best Regards Eric >> > I'm fine with this. > > -Christoffer >