From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christoffer Dall Subject: Re: [PATCH v4 07/11] KVM: arm/arm64: vgic-its: Always attempt to save/restore device and collection tables Date: Wed, 18 Oct 2017 00:15:02 +0200 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-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org, marc.zyngier@arm.com, andre.przywara@arm.com, linux-kernel@vger.kernel.org, kvmarm@lists.cs.columbia.edu, wu.wubin@huawei.com, eric.auger.pro@gmail.com To: Eric Auger Return-path: Content-Disposition: inline In-Reply-To: <1508224209-15366-8-git-send-email-eric.auger@redhat.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kvmarm-bounces@lists.cs.columbia.edu Sender: kvmarm-bounces@lists.cs.columbia.edu List-Id: kvm.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