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 <eric.auger@xxxxxxxxxx> > > --- > > 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
