From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S935198AbdJQWPI (ORCPT ); Tue, 17 Oct 2017 18:15:08 -0400 Received: from mail-wm0-f42.google.com ([74.125.82.42]:49020 "EHLO mail-wm0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1760844AbdJQWPF (ORCPT ); Tue, 17 Oct 2017 18:15:05 -0400 X-Google-Smtp-Source: AOwi7QDC9tyXLrxYDD2VuDwl68GUAqLxdNagUJcf6JFNzl+VM0u2FayH2vL5id+plWoDKW6erbrxYQ== Date: Wed, 18 Oct 2017 00:15:02 +0200 From: Christoffer Dall To: Eric Auger Cc: eric.auger.pro@gmail.com, linux-kernel@vger.kernel.org, kvm@vger.kernel.org, kvmarm@lists.cs.columbia.edu, marc.zyngier@arm.com, peter.maydell@linaro.org, andre.przywara@arm.com, wanghaibin.wang@huawei.com, wu.wubin@huawei.com, drjones@redhat.com, wei@redhat.com Subject: Re: [PATCH v4 07/11] KVM: arm/arm64: vgic-its: Always attempt to save/restore device and collection tables Message-ID: <20171017221502.GG8326@lvm> References: <1508224209-15366-1-git-send-email-eric.auger@redhat.com> <1508224209-15366-8-git-send-email-eric.auger@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1508224209-15366-8-git-send-email-eric.auger@redhat.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Oct 17, 2017 at 09:10:05AM +0200, 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 and shouldn't > be performed. Same must happen on restore path. > > Without this patch, after a reset and early state backup, > the device table restore may fail due to L1 entry not valid. > If we don't restore the collection table the guest does > not reboot properly. I don't really understand. After the previous patches, why would a properly configured ITS return an error in its_save_device_tables? If that's not possible, are we not trying to support partially migrating half-way broken state, and is that something we care about? > > Signed-off-by: Eric Auger > > --- > > 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 e18f1e4..1c3e83f 100644 > --- a/virt/kvm/arm/vgic/vgic-its.c > +++ b/virt/kvm/arm/vgic/vgic-its.c > @@ -2381,12 +2381,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); What if the two functions return two different error codes, is the binary OR of these error codes going to tell userspace anything meaningful? > > -out: > unlock_all_vcpus(kvm); > mutex_unlock(&its->its_lock); > mutex_unlock(&kvm->lock); > @@ -2413,11 +2410,9 @@ static int vgic_its_restore_tables_v0(struct vgic_its *its) > } > > ret = vgic_its_restore_collection_table(its); > - 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); > -- > 2.5.5 > Thanks, -Christoffer