Hi Eric, On 04/07/16 16:00, Auger Eric wrote: > Hi Peter, > > On 04/07/2016 16:32, Peter Maydell wrote: >> On 4 July 2016 at 15:27, Auger Eric <eric.auger@xxxxxxxxxx> wrote: >>> Andre, >>> >>> On 04/07/2016 16:05, Andre Przywara wrote: >>>> Hi, >>>> >>>> On 04/07/16 10:00, Auger Eric wrote: >>>>> From a QEMU integration point of view this means the init sequence used >>>>> for KVM GIC interrupt controllers cannot be reused for ITS and more >>>>> importantly this is not straightforward to have the proper sequence >>>>> ordering (hence the previously reported case). >>>> >>>> I am confused, can you please elaborate what the problem is? >>>> Or alternatively sketch what you ideally would the ITS init sequence to >>>> look like? I am totally open to any changes, just need to know what >>>> you/QEMU needs. >>> >>> in QEMU the address setting is done on a so-called qemu >>> "machine_init_done_notifier", ie. a callback that is registered at ITS >>> device init, to be called once the virt machine code has executed. This >>> callback calls kvm_device_ioctl(kd->dev_fd, KVM_SET_DEVICE_ATTR, attr); >>> >>> In case the userspace needs to explicitly "init" the ITS (actually ~ >>> map_resources) this must happen after the KVM_SET_DEVICE_ATTR. So you >>> also must register a callback in the same way. However there is a >>> framework existing to register kvm device addresses but this does not >>> exist to set other attributes than device addresses. >>> >>> This is feasible I think but this does not fit qemu nicely. So can't the >>> map_resources happen implicitly on the first VCPU run? >> >> I'm not clear what you think the problem here for QEMU is. >> We definitely want the API for the kernel to be: >> create device >> set attributes >> explicitly complete init of the device >> [attribute setting after this is illegal] >> run CPUs >> >> so I'm not sure why QEMU would care if the kernel does things at >> "final init" rather than "run CPUs". >> >> This is how the GICv3 init works and how the ITS should work too; > The GICv3 explicit does not do the same as the ITS init. > GICv3 init does not map the resources (KVM iodevice registration). This > is done at 1st VCPU run. > ITS init does map the resources. If we call the ITS init at the same > place as we call the GICv3 init, in the realization function, the region > mapping is not yet done so you will map resources at undefined location. What do you mean with "region mapping"? QEMU's internal mapping? But you set the GICv3 redist/dist addresses (or the ITS address, for that matter) before calling CTRL_INIT, right? So are you concerned that the kernel "maps" the region before QEMU connects the memory region? Is that really a problem? This "map_resources" equivalent for the ITS just creates a kvm_io_bus mapping, which would never fire without either a guest running (which we clearly don't at this point) or userland explicitly requesting access (which would require QEMU to have done the mapping?). Is that about right or do I miss something again? Sorry for my ignorance on the QEMU internals in that matter ;-) > I am definitively not opposed to call the ITS init function explicitly > from user side but this must happen after the KVM_SET_DEVICE_ATTR. So > another machine_init_done function must be registered and the notifier > must be called AFTER the notifier that calls the KVM_SET_DEVICE_ATTR > ioctl. However you cannot easily master the machine init done notifier > registration order because in target-arm/kvm.c there is a single > notifier that calls all the KVM_SET_DEVICE_ATTR for all the KVM devices > (kvm_arm_machine_init_done). So it is not possible to register the ITS > init notifier before the "kvm_arm_set_device_addr" notifier. > > So my understanding is one must do things outside of the existing framework? While I am certainly not interested in making QEMU's (or the QEMU patch author's) life harder than needed, I am wondering if we should really model the userland/kernel interface according to QEMU's current framework design. Is the current approach a leftover of the initial vGICv2 code, that was just slightly adjusted to support GICv3? Cheers, Andre > >> we don't want to extend the GICv2 mistake of "no explicit complete >> init" to anything else, because then you end up with ad-hoc >> "do this when we first run the vCPU; oh, but also do it if >> userspace tries to write a register content; and also if...". >> >> thanks >> -- PMM >> > -- 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
