Re: [Qemu-arm] [Qemu-devel] [RFC 0/3] vITS Reset

[Peter Maydell]

On 11 October 2017 at 17:06, Auger Eric <address@hidden> wrote:
> On 09/10/2017 20:17, Peter Maydell wrote:
>> It's not clear to me why we need a new KVM device attribute
>> for doing ITS reset. The usual approach for this is:
>>  * system reset causes QEMU's device model reset code
>>    to reset state structure values to whatever their
>>    reset value is
>>  * we write those values up into the kernel, which is
>>    sufficient to reset it
>>
>> What goes wrong with that in the case of the ITS?
>> In particular, if GITS_CTLR.Enabled is 0 then I think the
>> kernel should not be trying to read guest memory tables.
>
> We discussed that on the kernel thread and Christoffer seemed to prefer
> an IOTCL instead of individual register writes
> (https://www.spinics.net/lists/kvm-arm/msg27211.html)
>
> The IOCTL empties the ITS cached data (list of collection, list of
> devices and list of LPIs) while it is not obvious the reset of BASER<n>
> would mandate that. Eventually in my kernel series I also voided the
> list on BASER<n>.valid toggling from 1 to 0.

OK, I guess that makes sense -- in hardware, the device
has extra internal state (the cached stuff) which isn't
visible in its register interface. So we want to provide
an API which is morally equivalent to the hardware reset line
to tell the ITS to drop that internal state.

> Spec also says:
> If a write to GITS.CTLR changes the Enabled field from 1 to 0, the
> Interrupt Translation Space must ensure
> that both:
> • Any caches containing mapping data are made consistent with external
> memory.
> • GITS_CTLR.Quiescent == 0 until all caches are consistent with external
> memory.
>
> I aknowledge I don't really get at which moment we are supposed to void
> the caches.

This is the interface for a software controlled clean powerdown:
 * OS writes 0 to Enabled
 * ITS finishes anything it is currently in the middle of
 * ITS updates any info that it currently has only in its internal
   cache so that it is also reflected in the external memory
   data structures
 * ITS sets Quiescent to 1 to tell the OS it has finished this process
   (and may no longer access guest RAM after this point)
 * OS can now unmap the RAM that the ITS tables are in,
   tell the h/w to power off the GIC/ITS, etc

It's a multistep process where the ITS gets the opportunity to
write memory in the middle. I think in theory there's no obligation
to actually drop the cached data when you set Quiescent to 1,
provided that you validate that when the OS sets Enabled back to
1 all your cached data is still valid, because that's
indistinguishable from dropping the cache and then repopulating it.

For the unclean powerdown (yanking the cord out of the back of the
machine), which is what qemu's reset is, we don't give the ITS
the option to write memory, it just has to effectively drop
any internal data structures it was using. So it needs a KVM
API that's different from the register-access API.

thanks
-- PMM