Re: [Qemu-devel] commit virtio: recalculate vq->inuse after migration might cause last_avail_idx vs. used_idx failure

[Halil Pasic <pasic@linux.vnet.ibm.com>]

On 12/15/2016 02:37 PM, Paolo Bonzini wrote:
> 
> 
> On 15/12/2016 12:32, Halil Pasic wrote:
>> static inline uint16_t vring_avail_idx(VirtQueue *vq)
>> {
>>     hwaddr pa;
>>     pa = vq->vring.avail + offsetof(VRingAvail, idx);
>>     vq->shadow_avail_idx = virtio_lduw_phys(vq->vdev, pa);
>>
>> we should have an endiannes handling here before assigning shadow_avail_idx I guess
>>
>>     return vq->shadow_avail_idx;
>> } 
> 
> Endianness is already handled:
> 
> static inline uint16_t virtio_lduw_phys(VirtIODevice *vdev, hwaddr pa)
> {
>     if (virtio_access_is_big_endian(vdev)) {
>         return lduw_be_phys(&address_space_memory, pa);
>     }
>     return lduw_le_phys(&address_space_memory, pa);
> }

Thanks Paolo, you are obviously right. Sorry for the noise.

> 
>> I will meditate a bit more on this and probably create a patch to fix it.
>>
>> What make me wonder is that according to the reports live migration usually
>> works (ca 1% fails)...


Seems I will have to get a dump and/or reproduce the problem myself
before I can tell what is going on there  -- the guru saved me some
meditation.

> 
> What is the backtrace of the vring_avail_idx call?  If your device is

As far as I can see from the code the guest features should be already
loaded from the migration stream.

Thanks again!

Halil

> virtio 1.0, and vdev->guest_features has not been initialized correctly,
> you might incorrectly treat LE virtio 1.0 data as BE virtio 0.9 data:
> 
>     if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
>         /* Devices conforming to VIRTIO 1.0 or later are always LE. */
>         return false;
>     }
>     return true;
> 
> Thanks,
> 
> Paolo
>