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

[Peter Maydell]

On 27 September 2017 at 15:56, Eric Auger <address@hidden> wrote:
> At the moment the ITS is not properly reset. On System reset or
> reboot, previous ITS register values and caches are left
> unchanged. Some of the registers might point to some guest RAM
> tables which are not provisionned. This leads to state
> inconsistencies that are detected by the kernel save/restore
> code. And eventually this may cause qemu abort on source or
> destination.
>
> The 1st patch, suggested to be cc'ed stable proposes to remove
> the abort in case of table save/restore failure. This is
> definitively not ideal but looks the most reasonable until we
> get a proper way to reset the ITS. Still a message is emitted
> to report the save/restore did not happen correctly.
>
> Subsequent patches add the support of explicit reset using
> a new kvm device group/attribute combo. The associated kernel
> series is not upstream [1], hence the RFC.
>
> ITS specification is not very clear about reset. There is no
> reset wire. Some register fields are documented to have
> architecturally defined reset values and we use those here:
> Most importantly the Valid bit of GITS_CBASER and GITS_BASER
> are cleared and the GITS_CTLR.Enabled bit is cleared as well.

The ITS is a device, not part of the CPU, so its reset is
implementation-defined but typically via some signal that's
controlled by the SoC (this is no different to the other
parts of the GICv3 which aren't denoted as being in the
reset domain of the CPU). For instance if you look at the TRM
for the GIC-500 it has a single 'resetn' reset signal which
resets all of the GIC/ITS apart from the CPU interface parts:
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.100336_0002_01_en/index.html

The GICv3 spec 6.6 indicates what is supposed to happen to
the ITS on powerup. For QEMU's purposes reset is generally
considered to be a cold reset and we should always reset our
state to the same thing it was when QEMU first started.

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.

thanks
-- PMM