Re: [RFC PATCH 3/3] kvm: arm/arm64: vgic-its: fix return value for restore

[Christoffer Dall <cdall@linaro.org>]

On Wed, Sep 13, 2017 at 11:25:01PM +0200, Auger Eric wrote:
> 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.

Yes, but then you have to rework the scan function otherwise to work
with indirect tables, and I'm not sure that becomes more pretty.

> >>
> >> 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.
> 

Can you show me an alternative better patch and we can compare?

Thanks,
-Christoffer