| From 2f9a174f918e29608564c7a4e8329893ab604fb4 Mon Sep 17 00:00:00 2001 |
| From: Halil Pasic <pasic@linux.ibm.com> |
| Date: Mon, 11 Oct 2021 07:39:21 +0200 |
| Subject: virtio: write back F_VERSION_1 before validate |
| |
| From: Halil Pasic <pasic@linux.ibm.com> |
| |
| commit 2f9a174f918e29608564c7a4e8329893ab604fb4 upstream. |
| |
| The virtio specification virtio-v1.1-cs01 states: "Transitional devices |
| MUST detect Legacy drivers by detecting that VIRTIO_F_VERSION_1 has not |
| been acknowledged by the driver." This is exactly what QEMU as of 6.1 |
| has done relying solely on VIRTIO_F_VERSION_1 for detecting that. |
| |
| However, the specification also says: "... the driver MAY read (but MUST |
| NOT write) the device-specific configuration fields to check that it can |
| support the device ..." before setting FEATURES_OK. |
| |
| In that case, any transitional device relying solely on |
| VIRTIO_F_VERSION_1 for detecting legacy drivers will return data in |
| legacy format. In particular, this implies that it is in big endian |
| format for big endian guests. This naturally confuses the driver which |
| expects little endian in the modern mode. |
| |
| It is probably a good idea to amend the spec to clarify that |
| VIRTIO_F_VERSION_1 can only be relied on after the feature negotiation |
| is complete. Before validate callback existed, config space was only |
| read after FEATURES_OK. However, we already have two regressions, so |
| let's address this here as well. |
| |
| The regressions affect the VIRTIO_NET_F_MTU feature of virtio-net and |
| the VIRTIO_BLK_F_BLK_SIZE feature of virtio-blk for BE guests when |
| virtio 1.0 is used on both sides. The latter renders virtio-blk unusable |
| with DASD backing, because things simply don't work with the default. |
| See Fixes tags for relevant commits. |
| |
| For QEMU, we can work around the issue by writing out the feature bits |
| with VIRTIO_F_VERSION_1 bit set. We (ab)use the finalize_features |
| config op for this. This isn't enough to address all vhost devices since |
| these do not get the features until FEATURES_OK, however it looks like |
| the affected devices actually never handled the endianness for legacy |
| mode correctly, so at least that's not a regression. |
| |
| No devices except virtio net and virtio blk seem to be affected. |
| |
| Long term the right thing to do is to fix the hypervisors. |
| |
| Cc: <stable@vger.kernel.org> #v4.11 |
| Signed-off-by: Halil Pasic <pasic@linux.ibm.com> |
| Fixes: 82e89ea077b9 ("virtio-blk: Add validation for block size in config space") |
| Fixes: fe36cbe0671e ("virtio_net: clear MTU when out of range") |
| Reported-by: markver@us.ibm.com |
| Reviewed-by: Cornelia Huck <cohuck@redhat.com> |
| Link: https://lore.kernel.org/r/20211011053921.1198936-1-pasic@linux.ibm.com |
| Signed-off-by: Michael S. Tsirkin <mst@redhat.com> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| drivers/virtio/virtio.c | 11 +++++++++++ |
| 1 file changed, 11 insertions(+) |
| |
| --- a/drivers/virtio/virtio.c |
| +++ b/drivers/virtio/virtio.c |
| @@ -240,6 +240,17 @@ static int virtio_dev_probe(struct devic |
| driver_features_legacy = driver_features; |
| } |
| |
| + /* |
| + * Some devices detect legacy solely via F_VERSION_1. Write |
| + * F_VERSION_1 to force LE config space accesses before FEATURES_OK for |
| + * these when needed. |
| + */ |
| + if (drv->validate && !virtio_legacy_is_little_endian() |
| + && device_features & BIT_ULL(VIRTIO_F_VERSION_1)) { |
| + dev->features = BIT_ULL(VIRTIO_F_VERSION_1); |
| + dev->config->finalize_features(dev); |
| + } |
| + |
| if (device_features & (1ULL << VIRTIO_F_VERSION_1)) |
| dev->features = driver_features & device_features; |
| else |