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 <wanghaibin.wang@xxxxxxxxxx>
>>> ---
>>> 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
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
