Hi Scott, On 07/01/15 18:16, Scott Wood wrote: > On Wed, 2015-01-07 at 18:11 +0000, Andre Przywara wrote: >> On 07/01/15 17:45, Scott Wood wrote: >>> On Wed, 2015-01-07 at 10:55 +0000, Andre Przywara wrote: >>>> Hi Scott, >>>> >>>> thanks for looking at the patch. >>>> >>>> On 06/01/15 20:52, Scott Wood wrote: >>>>> Out of curiosity, why do you need to test it from inside the kernel but >>>>> outside kvm_main.c? >>>> >>>> I need it from arch/arm/kvm/arm.c or alternatively virt/kvm/arm/vgic.c. >>>> The problem is that while KVM_CREATE_DEVICE works fine with checking the >>>> availability of the requested device, KVM_CREATE_IRQCHIP does not - and >>>> the latter is handled in the arch specific parts of the code. At the >>>> moment the GIC_V2 is the only IRQ chip, so it's all or nothing right >>>> now. But very soon there will be a GIC_V3, with GIC_V2 compatibility not >>>> always being available, so KVM_CREATE_IRQCHIP may fail although there is >>>> an in-kernel IRQ chip available. >>>> Instead of hacking something up I thought it would be cleaner to use the >>>> existing framework and just map KVM_CREATE_IRQCHIP to >>>> KVM_CREATE_DEVICE(KVM_DEV_TYPE_ARM_VGIC_V2). >>> >>> In that case you'd need the full create_device functionality from >>> arch/arm/kvm, not just testing whether a device type is present, right? >> >> Well, not really. On KVM_CREATE_IRQCHIP kvm_vgic_create is called, which >> is also what the KVM device .create function for GIC_V2 does. >> So yes, we don't fully use the KVM device framework, but just use the >> same functionality. >> Not sure if it would make sense to use more of the KVM device framework, >> as currently there is no issue with the current approach and I just need >> to know whether the GIC_V2 has been registered. > > If you're communicating privately with the vgic code to create the > irqchip, can't you also communicate privately to see if that > functionality is available? I'm not opposed to this patch, but it seems > odd to use the device framework just for testing whether some other > interface is available. Mainly because I wanted to avoid a solution that is private to the VGIC and ARM. Also hacking this into the VGIC code looks much uglier than simply using kvm_check_device_type(). Currently I just register GIC_V2 or GIC_V3 according to the hardware's capabilities. Now I have to duplicate this decision making in the VGIC code again. I now worked around the issue without this patch now, but it looks quite ugly. I will send out a new version of the patch series tomorrow and will put you on CC: for the respective patches. So if in doubt we can continue this discussion on those patches later. Cheers, Andre. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
