All of lore.kernel.org
 help / color / mirror / Atom feed
From: Cornelia Huck <cohuck@redhat.com>
To: Heiko Carstens <hca@linux.ibm.com>,
	Vasily Gorbik <gor@linux.ibm.com>,
	Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Halil Pasic <pasic@linux.ibm.com>,
	Pierre Morel <pmorel@linux.ibm.com>,
	linux-s390@vger.kernel.org,
	virtualization@lists.linux-foundation.org, kvm@vger.kernel.org,
	stable@vger.kernel.org
Subject: Re: [PATCH v2] virtio/s390: implement virtio-ccw revision 2 correctly
Date: Fri, 19 Feb 2021 17:38:28 +0100	[thread overview]
Message-ID: <20210219173828.6a2ab5d4.cohuck@redhat.com> (raw)
In-Reply-To: <20210216110645.1087321-1-cohuck@redhat.com>

I was thinking of queuing this, but maybe it is quicker to pick it into
the s390 tree directly and save us the extra pull request dance?
Especially as this is a stable-worthy bugfix.

On Tue, 16 Feb 2021 12:06:45 +0100
Cornelia Huck <cohuck@redhat.com> wrote:

> CCW_CMD_READ_STATUS was introduced with revision 2 of virtio-ccw,
> and drivers should only rely on it being implemented when they
> negotiated at least that revision with the device.
> 
> However, virtio_ccw_get_status() issued READ_STATUS for any
> device operating at least at revision 1. If the device accepts
> READ_STATUS regardless of the negotiated revision (which some
> implementations like QEMU do, even though the spec currently does
> not allow it), everything works as intended. While a device
> rejecting the command should also be handled gracefully, we will
> not be able to see any changes the device makes to the status,
> such as setting NEEDS_RESET or setting the status to zero after
> a completed reset.
> 
> We negotiated the revision to at most 1, as we never bumped the
> maximum revision; let's do that now and properly send READ_STATUS
> only if we are operating at least at revision 2.
> 
> Cc: stable@vger.kernel.org
> Fixes: 7d3ce5ab9430 ("virtio/s390: support READ_STATUS command for virtio-ccw")
> Reviewed-by: Halil Pasic <pasic@linux.ibm.com>
> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> ---
> 
> v1->v2:
>   tweak patch description and cc:stable
> 
> ---
>  drivers/s390/virtio/virtio_ccw.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c
> index 5730572b52cd..54e686dca6de 100644
> --- a/drivers/s390/virtio/virtio_ccw.c
> +++ b/drivers/s390/virtio/virtio_ccw.c
> @@ -117,7 +117,7 @@ struct virtio_rev_info {
>  };
>  
>  /* the highest virtio-ccw revision we support */
> -#define VIRTIO_CCW_REV_MAX 1
> +#define VIRTIO_CCW_REV_MAX 2
>  
>  struct virtio_ccw_vq_info {
>  	struct virtqueue *vq;
> @@ -952,7 +952,7 @@ static u8 virtio_ccw_get_status(struct virtio_device *vdev)
>  	u8 old_status = vcdev->dma_area->status;
>  	struct ccw1 *ccw;
>  
> -	if (vcdev->revision < 1)
> +	if (vcdev->revision < 2)
>  		return vcdev->dma_area->status;
>  
>  	ccw = ccw_device_dma_zalloc(vcdev->cdev, sizeof(*ccw));


WARNING: multiple messages have this Message-ID (diff)
From: Cornelia Huck <cohuck@redhat.com>
To: Heiko Carstens <hca@linux.ibm.com>,
	Vasily Gorbik <gor@linux.ibm.com>,
	Christian Borntraeger <borntraeger@de.ibm.com>
Cc: linux-s390@vger.kernel.org, Pierre Morel <pmorel@linux.ibm.com>,
	kvm@vger.kernel.org, stable@vger.kernel.org,
	virtualization@lists.linux-foundation.org,
	Halil Pasic <pasic@linux.ibm.com>
Subject: Re: [PATCH v2] virtio/s390: implement virtio-ccw revision 2 correctly
Date: Fri, 19 Feb 2021 17:38:28 +0100	[thread overview]
Message-ID: <20210219173828.6a2ab5d4.cohuck@redhat.com> (raw)
In-Reply-To: <20210216110645.1087321-1-cohuck@redhat.com>

I was thinking of queuing this, but maybe it is quicker to pick it into
the s390 tree directly and save us the extra pull request dance?
Especially as this is a stable-worthy bugfix.

On Tue, 16 Feb 2021 12:06:45 +0100
Cornelia Huck <cohuck@redhat.com> wrote:

> CCW_CMD_READ_STATUS was introduced with revision 2 of virtio-ccw,
> and drivers should only rely on it being implemented when they
> negotiated at least that revision with the device.
> 
> However, virtio_ccw_get_status() issued READ_STATUS for any
> device operating at least at revision 1. If the device accepts
> READ_STATUS regardless of the negotiated revision (which some
> implementations like QEMU do, even though the spec currently does
> not allow it), everything works as intended. While a device
> rejecting the command should also be handled gracefully, we will
> not be able to see any changes the device makes to the status,
> such as setting NEEDS_RESET or setting the status to zero after
> a completed reset.
> 
> We negotiated the revision to at most 1, as we never bumped the
> maximum revision; let's do that now and properly send READ_STATUS
> only if we are operating at least at revision 2.
> 
> Cc: stable@vger.kernel.org
> Fixes: 7d3ce5ab9430 ("virtio/s390: support READ_STATUS command for virtio-ccw")
> Reviewed-by: Halil Pasic <pasic@linux.ibm.com>
> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> ---
> 
> v1->v2:
>   tweak patch description and cc:stable
> 
> ---
>  drivers/s390/virtio/virtio_ccw.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c
> index 5730572b52cd..54e686dca6de 100644
> --- a/drivers/s390/virtio/virtio_ccw.c
> +++ b/drivers/s390/virtio/virtio_ccw.c
> @@ -117,7 +117,7 @@ struct virtio_rev_info {
>  };
>  
>  /* the highest virtio-ccw revision we support */
> -#define VIRTIO_CCW_REV_MAX 1
> +#define VIRTIO_CCW_REV_MAX 2
>  
>  struct virtio_ccw_vq_info {
>  	struct virtqueue *vq;
> @@ -952,7 +952,7 @@ static u8 virtio_ccw_get_status(struct virtio_device *vdev)
>  	u8 old_status = vcdev->dma_area->status;
>  	struct ccw1 *ccw;
>  
> -	if (vcdev->revision < 1)
> +	if (vcdev->revision < 2)
>  		return vcdev->dma_area->status;
>  
>  	ccw = ccw_device_dma_zalloc(vcdev->cdev, sizeof(*ccw));

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

  reply	other threads:[~2021-02-19 16:40 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-16 11:06 [PATCH v2] virtio/s390: implement virtio-ccw revision 2 correctly Cornelia Huck
2021-02-16 11:06 ` Cornelia Huck
2021-02-19 16:38 ` Cornelia Huck [this message]
2021-02-19 16:38   ` Cornelia Huck
2021-02-19 19:00   ` Vasily Gorbik

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210219173828.6a2ab5d4.cohuck@redhat.com \
    --to=cohuck@redhat.com \
    --cc=borntraeger@de.ibm.com \
    --cc=gor@linux.ibm.com \
    --cc=hca@linux.ibm.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=pasic@linux.ibm.com \
    --cc=pmorel@linux.ibm.com \
    --cc=stable@vger.kernel.org \
    --cc=virtualization@lists.linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.