Re: [PATCH v2 09/10] KVM: arm/arm64: Document KVM_DEV_ARM_ITS_CTRL_RESET

From: Christoffer Dall
Date: Fri Oct 13 2017 - 11:26:57 EST


Hi Eric,

On Wed, Sep 27, 2017 at 03:28:39PM +0200, Eric Auger wrote:
> At the moment, the in-kernel emulated ITS is not properly reset.
> On guest restart/reset some registers keep their old values and
> internal structures like device, ITE, collection lists are not freed.

, and collection ...
>
> This may lead to various bugs. Among them, we can have incorrect state
> backup or failure when saving the ITS state at early guest boot stage.
>
> This patch documents a new attribute, KVM_DEV_ARM_ITS_CTRL_RESET in
> the KVM_DEV_ARM_VGIC_GRP_CTRL group.
>
> Upon this action, we can reset registers and especially those
> pointing to tables previously allocated by the guest and free
> the internal data structures storing the list of devices, collections
> and lpis.
>
> Signed-off-by: Eric Auger <eric.auger@xxxxxxxxxx>
> Reported-by: wanghaibin <wanghaibin.wang@xxxxxxxxxx>
>
> ---
>
> v1 -> v2:
> - Describe architecturally-defined reset values
> ---
> Documentation/virtual/kvm/devices/arm-vgic-its.txt | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/Documentation/virtual/kvm/devices/arm-vgic-its.txt b/Documentation/virtual/kvm/devices/arm-vgic-its.txt
> index eb06beb..047358c 100644
> --- a/Documentation/virtual/kvm/devices/arm-vgic-its.txt
> +++ b/Documentation/virtual/kvm/devices/arm-vgic-its.txt
> @@ -33,6 +33,10 @@ Groups:
> request the initialization of the ITS, no additional parameter in
> kvm_device_attr.addr.
>
> + KVM_DEV_ARM_ITS_CTRL_RESET
> + reset the ITS, no additional parameter in kvm_device_attr.addr.
> + See "ITS Reset State" section.
> +
> KVM_DEV_ARM_ITS_SAVE_TABLES
> save the ITS table data into guest RAM, at the location provisioned
> by the guest in corresponding registers/table entries.
> @@ -157,3 +161,15 @@ Then vcpus can be started.
> - pINTID is the physical LPI ID; if zero, it means the entry is not valid
> and other fields are not meaningful.
> - ICID is the collection ID
> +
> + ITS Reset State:
> + ----------------

Are the bullet points below the state after the device has been reset
(after KVM_DEV_ARM_ITS_CTRL_RESET returns) or ? I think this should be
clarified.

> +
> +- the ITS is not enabled and quiescent:

The

> + GITS_CTLR.Enabled = 0 .Quiescent=1
> +- caches are empty
Caches (would it make more sense to say that there is no
internally cached state?)
> +- No collection or device table is provisionned

are used:

> + GITS_BASER<n>.Valid = 0
> +- the command queue is not allocated:

The

> + GITS_CBASER = 0, GITS_CREADR = 0, GITS_CWRITER = 0
> +- The ABI version corresponds to the one set before reset

Do you mean that resetting the ITS cannot change the ABI version used
for save/restore, and therefore remains the same as it the version
configured when the device was first created ?

> --
> 2.5.5
>

Thanks, (and sorry for being pedantic about ABI wordings)
-Christoffer