Hi Andre,

On 04/07/2016 19:40, Andre Przywara wrote:

Hi Eric,

On 04/07/16 16:00, Auger Eric wrote:

quoted

Hi Peter,

On 04/07/2016 16:32, Peter Maydell wrote:

quoted

On 4 July 2016 at 15:27, Auger Eric [off-list ref] wrote:

quoted

Andre,

On 04/07/2016 16:05, Andre Przywara wrote:

quoted

Hi,

On 04/07/16 10:00, Auger Eric wrote:

quoted

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 ;-)

quoted

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?

I have a solution to workaround the issue on qemu side and I can see the
guest ITS properly initialized now so proceed according to your will &
consensus.

Eric

Cheers,
Andre

quoted

quoted

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