Hej Christoffer, On 11/01/15 15:22, Christoffer Dall wrote: > On Fri, Jan 09, 2015 at 01:42:42PM +0000, Andre Przywara wrote: >> Hi Christoffer, >> >> On 09/01/15 12:33, Christoffer Dall wrote: >>> On Fri, Jan 09, 2015 at 11:54:36AM +0000, Andre Przywara wrote: >>>> While we can easily register and unregister KVM devices, there is >>>> currently no easy way of checking whether a device has been >>>> registered. >>>> Introduce kvm_check_device_type() for that purpose and use it in two >>>> existing functions. Also change the return code for an invalid >>>> type number from ENOSPC to EINVAL. >>>> This function will be later used by another patch set to check >>>> whether a KVM_CREATE_IRQCHIP ioctl is valid. >>> >>> I feel like this is misguided and the vgic should be able to figure this >>> stuff out internally. Did you have code for this approach somewhere >>> that I can take a look at? >> >> I pushed my WIP patch on top of the kvm-gicv3/v6 tree. >> Given how that looks I reckoned the generic solution would be more >> preferable. >> Basically we internally decide in the _probe function whether we support >> GICv2 emulation or not, which is mostly driven by device tree >> properties. So at the moment I just register the GIC_V2 KVM device or >> not. Now with the "vgic internal" solution I misuse the GICV address >> base as a hint of the GICv2 emulation availability. Alternatively I have >> to introduce a new variable to mirror what the KVM device array already >> holds, which seems kind of exerted to me. >> Besides that I am not sure if the GICV address hint will always be a >> reliable indicator and what we will do if there will be another GIC >> model to be emulated in the future (maybe we need that for the ITS >> emulation already?) > > I don't think it looks that bad. > > Only your gicv3 and gicv2 code files know what they are capable of > emulating, how you choose to store this state internally in those files > is a somewhat orthogonal discussion from using the kvm device API. Well, the point is that the emulation capability is a hardware property and thus the knowledge is actually in the host part of the VGIC (so in vgic-v3.c and vgic-v2.c). From here we "communicate" the capability to userland by registering the respective VGIC KVM devices only. Since the emulation part of the VGIC lives in different files (vgic.c and vgic-vx-emul.c) we would need some kind of export to them, too. I found that it would be cleaner to just re-use what we already have with the KVM devices. > Using the KVM device api is just another way of storing and exposing the > information globally (you take registering the device types as an > indication of the state). > > Finally, I don't even think you ned the can_emulate function, I think > you should just return an error from init_vgic_model (which happens to > collide with my suggestion on making those functions a void function in > one of the previous patches) and you're done. I think I checked this before and since the init_vgic_model() implementations are in vgic-vx-emul.c we don't know the hardware capability anymore and would need some kind of variable holding that information (which lead me to re-using the KVM device knowledge). But I will re-check if there is an easy fix in here. >> >> So I prefer the more generic solution. >> Let me know what you think, I can as well drop 18/20 and merge the above >> mentioned patch. >> >>> I forget: Are we still requiring KVM_CREATE_IRQCHIP for VGICv3 or are we >>> just relying on users to use KVM_CREATE_DEVICE for anything in the >>> future? >> >> Since KVM_CREATE_IRQCHIP does not take an argument, we cannot use it for >> GICv3. So GICv3 mandates KVM_CREATE_DEVICE. We need userspace >> adjustments for GICv3 anyway, so that's not a problem. > > ok, so KVM_CREATE_IRQCHIP is a direct alias for KVM_CREATE_DEVICE(GIC_V2) > and is deprecated for GICv3? If so, we should probably update the > documentation to indicate the KVM_CREATE_IRQCHIP creates a GICv2 and > should not be used for any other in-kernel GIC versions. What about the following wording in api.txt: ----- On ARM/arm64, a GICv2 is created. Any other VGIC versions require the usage of KVM_CREATE_DEVICE (which can and should also be used to create a virtual GICv2). ----- In fact both QEMU and kvmtool currently try KVM_CREATE_DEVICE first even for a VGICv2 on a GICv2 and only fall back to KVM_CREATE_IRQCHIP if that fails (to support older kernels). Cheers, Andre. _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
