Hi, On 13/09/2017 22:02, Christoffer Dall wrote: > Hi Eric, > > On Wed, Sep 06, 2017 at 05:18:35PM +0200, Auger Eric wrote: >> Hi Wanghaibin, >> >> On 06/09/2017 15:05, wanghaibin wrote: >>> This patch fix the migrate restore tables failure. >>> >>> The same scene, at the destination, the restore tables interface traversal >>> guest >>> memory, and check the dte/ite is valid or not. >>> If all dtes/ites are invalid, we will do try next one, and the last it will >>> take >>> the 1 return value, but currently, it be treated as error. That's not >>> correct. >> There's indeed a bug here! In case all entries are invalid we shouldn't >> return an error. >> >> One solution could be to relax the error checking in scan_its_table() >> and do not return 1 when the whole length has been scanned. This would >> fix your issue. If you do not return 1 in scan_its_table if the whole size has been scanned, you achieve the same thing as in this patch and you simplify the error handling. >> >> drawback of that change: >> at the moment we check the consistency of the entry data (next offset >> field). At the moment if the next_offset points to an entry outside of >> the table scope we are able to return an error (for top level tables). >> >> Otherwise, if we want to keep that check, I think we would need to add a >> bool *valid parameter to entry_fn_t. in scan_its_table() we would return >> 1 only if last fn() call returns valid and len <= 0. >> > > I don't really understand what you're proposing. The above method or Wanghaibin's patch removes a consistency check on the entry next offset field. But maybe this is better to drop it and have a code that gains in readability.
Thanks Eric > > I think Wanghaibin's patch actually looks correct. Is there any problem > with it that you see? > > Thanks, > -Christoffer > >>> >>> This patch try to fix this problem. >>> >>> Signed-off-by: wanghaibin <[email protected]> >>> --- >>> virt/kvm/arm/vgic/vgic-its.c | 5 +---- >>> 1 file changed, 1 insertion(+), 4 deletions(-) >>> >>> diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c >>> index 5c20352..2c69aeb 100644 >>> --- a/virt/kvm/arm/vgic/vgic-its.c >>> +++ b/virt/kvm/arm/vgic/vgic-its.c >>> @@ -2025,7 +2025,7 @@ static int vgic_its_restore_dte(struct vgic_its *its, >>> u32 id, >>> return PTR_ERR(dev); >>> >>> ret = vgic_its_restore_itt(its, dev); >>> - if (ret) { >>> + if (ret < 0) { >>> vgic_its_free_device(its->dev->kvm, dev); >>> return ret; >>> } >>> @@ -2147,9 +2147,6 @@ static int vgic_its_restore_device_tables(struct >>> vgic_its *its) >>> vgic_its_restore_dte, NULL); >>> } >>> >>> - if (ret > 0) >>> - ret = -EINVAL; >>> - >>> return ret; >>> } >>> >>> _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
