Re: [PATCH kvmtool] virtio-net: Fix vq->use_event_idx flag check

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 28 Sep 2022 17:16:15 +0200
dinhngoc.tu@xxxxxxx wrote:

Hi,

> From: Tu Dinh Ngoc <dinhngoc.tu@xxxxxxx>
> 
> VIRTIO_RING_F_EVENT_IDX is a bit position value, but
> virtio_init_device_vq populates vq->use_event_idx by ANDing this value
> directly to vdev->features.

Indeed, it's used correctly in other parts of the code (virtio/net.c,
virtio/scsi.c), so we need to shift.

Just one thing below, but good catch!

> 
> Fix the check for this flag in virtio_init_device_vq.
> ---
>  virtio/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/virtio/core.c b/virtio/core.c
> index f432421..32e6a7a 100644
> --- a/virtio/core.c
> +++ b/virtio/core.c
> @@ -165,7 +165,7 @@ void virtio_init_device_vq(struct kvm *kvm, struct virtio_device *vdev,
>  	struct vring_addr *addr = &vq->vring_addr;
>  
>  	vq->endian		= vdev->endian;
> -	vq->use_event_idx	= (vdev->features & VIRTIO_RING_F_EVENT_IDX);
> +	vq->use_event_idx	= (vdev->features & (1 << VIRTIO_RING_F_EVENT_IDX));

Can you make this "1UL << ..." instead? Shifting signed values is quite
fragile.

Cheers,
Andre

>  	vq->enabled		= true;
>  
>  	if (addr->legacy) {




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux