Re: [PATCH v2 06/10] KVM: arm/arm64: vgic-its: Always attempt to save/restore device and collection tables

Andre Przywara
2017-10-06T14:38:49Z

Hi,

On 27/09/17 14:28, Eric Auger wrote:
> In case the device table save fails, we currently do not
> attempt to save the collection table. However it may
> happen that the device table fails because the structures
> in memory are inconsistent with device GITS_BASER however
> this does not mean collection backup can't be performed and
> wouldn't succeed. Same on restore path. Without this patch,
> after a reset and in case the device table fails in case of
> L1 entry not valid, the guest gets stuck on restore.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>
> ---
>
> candidate to be CC'ed stable
> ---
> virt/kvm/arm/vgic/vgic-its.c | 11 +++--------
> 1 file changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
> index 720552c..9e6b556 100644
> --- a/virt/kvm/arm/vgic/vgic-its.c
> +++ b/virt/kvm/arm/vgic/vgic-its.c
> @@ -2304,12 +2304,9 @@ static int vgic_its_save_tables_v0(struct vgic_its *its)
> }
>
> ret = vgic_its_save_device_tables(its);
> - if (ret)
> - goto out;
>
> - ret = vgic_its_save_collection_table(its);
> + ret |= vgic_its_save_collection_table(its);
>
> -out:
> unlock_all_vcpus(kvm);
> mutex_unlock(&its->its_lock);
> mutex_unlock(&kvm->lock);
> @@ -2336,11 +2333,9 @@ static int vgic_its_restore_tables_v0(struct vgic_its *its)
> }
>
> ret = vgic_its_restore_collection_table(its);

While the save functions above and this _v0 function here all use the
standard C return semantics (==0 on success, failure otherwise),
vgic_its_restore_collection_table() and the function call below can
return 1 if successful, AFAICS. I don't think this handled correctly here?

Cheers,
Andre.

> - if (ret)
> - goto out;
>
> - ret = vgic_its_restore_device_tables(its);
> -out:
> + ret |= vgic_its_restore_device_tables(its);
> +
> unlock_all_vcpus(kvm);
> mutex_unlock(&its->its_lock);
> mutex_unlock(&kvm->lock);
>