[PATCH v2 1/1] virtio: blk: support block sizes exceeding 512 bytes

[Heinrich Schuchardt]

On 10/8/25 17:39, Andre Przywara wrote:
> On Sat, 30 Aug 2025 22:39:54 +0200
> Heinrich Schuchardt <heinrich.schuchardt at canonical.com> wrote:
> 
> Hi Heinrich,
> 
>> QEMU allows to specify the logical block size via parameter
>> logical_block_size of a virtio-blk-device.
>>
>> The communication channel via virtqueues remains based on 512 byte blocks
>> even if the logical_block_size is larger.
>>
>> Consider the logical block size in the block device driver.
>>
>> Reported-by: Emil Renner Berthing <emil.renner.berthing at canonical.com>
>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
>> ---
>> v2:
>> 	remove superfluous include linux/err.h
>> ---
>>   drivers/virtio/virtio_blk.c | 25 ++++++++++++++++++++++---
>>   1 file changed, 22 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/virtio/virtio_blk.c b/drivers/virtio/virtio_blk.c
>> index 2f999fc8bbe..4224e3c17f4 100644
>> --- a/drivers/virtio/virtio_blk.c
>> +++ b/drivers/virtio/virtio_blk.c
>> @@ -12,10 +12,17 @@
>>   #include <virtio_types.h>
>>   #include <virtio.h>
>>   #include <virtio_ring.h>
>> +#include <linux/log2.h>
>>   #include "virtio_blk.h"
>>   
>> +/**
>> + * struct virtio_blk_priv - private date for virtio block device
>> + */
>>   struct virtio_blk_priv {
>> +	/** @virtqueue - virtqueue to process */
>>   	struct virtqueue *vq;
>> +	/** @blksz_shift - log2 of block size divided by 512 */
>> +	u32 blksz_shift;
>>   };
>>   
>>   static const u32 feature[] = {
>> @@ -71,6 +78,8 @@ static ulong virtio_blk_do_req(struct udevice *dev, u64 sector,
>>   	u8 status;
>>   	int ret;
>>   
>> +	sector <<= priv->blksz_shift;
>> +	blkcnt <<= priv->blksz_shift;
>>   	virtio_blk_init_header_sg(dev, sector, type, &out_hdr, &hdr_sg);
>>   	sgs[num_out++] = &hdr_sg;
>>   
>> @@ -109,7 +118,7 @@ static ulong virtio_blk_do_req(struct udevice *dev, u64 sector,
>>   		;
>>   	log_debug("done\n");
>>   
>> -	return status == VIRTIO_BLK_S_OK ? blkcnt : -EIO;
>> +	return status == VIRTIO_BLK_S_OK ? blkcnt >> priv->blksz_shift : -EIO;
>>   }
>>   
>>   static ulong virtio_blk_read(struct udevice *dev, lbaint_t start,
>> @@ -177,15 +186,25 @@ static int virtio_blk_probe(struct udevice *dev)
>>   	struct blk_desc *desc = dev_get_uclass_plat(dev);
>>   	u64 cap;
>>   	int ret;
>> +	u32 blk_size;
>>   
>>   	ret = virtio_find_vqs(dev, 1, &priv->vq);
>>   	if (ret)
>>   		return ret;
>>   
>> -	desc->blksz = 512;
>> -	desc->log2blksz = 9;
>>   	virtio_cread(dev, struct virtio_blk_config, capacity, &cap);
>>   	desc->lba = cap;
>> +	if (!virtio_has_feature(dev, VIRTIO_BLK_F_BLK_SIZE)) {
> 
> Is that actually the right condition check? I would assume that we only
> query the blk_size field if the feature bit is *set*, not when it's
> cleared?
> We see a failure (virtio block device not registering), because blk_size is
> 0 on the Arm FVP fast model, which is fine I think because the feature bit
> is clear as well.
> 
> Cheers,
> Andre

Hello Andre,

Thanks for sharing your test results.

Looking at these QEMU lines, I think the evaluated feature flag is correct:

pc-bios/s390-ccw/virtio.h:163:
uint32_t blk_size; /* block size of device (if VIRTIO_BLK_F_BLK_SIZE) */

include/standard-headers/linux/virtio_blk.h:72-73:
/* block size of device (if VIRTIO_BLK_F_BLK_SIZE) */
__virtio32 blk_size;

virtio_has_feature() cannot detect a feature if is not in the feature 
set of the U-Boot driver. See function virtio_uclass_child_pre_probe():

     uc_priv->features = driver_features_legacy & device_features;

VIRTIO_BLK_F_BLK_SIZE is missing in the feature set of virtio_blk.
The not in front of virtio_has_feature() is wrong.

Could you, please, retest with this diff:

diff --git a/drivers/virtio/virtio_blk.c b/drivers/virtio/virtio_blk.c
index 4224e3c17f4..ec2e28b54f7 100644
--- a/drivers/virtio/virtio_blk.c
+++ b/drivers/virtio/virtio_blk.c
@@ -26,6 +26,7 @@ struct virtio_blk_priv {
  };

  static const u32 feature[] = {
+       VIRTIO_BLK_F_BLK_SIZE,
         VIRTIO_BLK_F_WRITE_ZEROES
  };

@@ -194,7 +195,7 @@ static int virtio_blk_probe(struct udevice *dev)

         virtio_cread(dev, struct virtio_blk_config, capacity, &cap);
         desc->lba = cap;
-       if (!virtio_has_feature(dev, VIRTIO_BLK_F_BLK_SIZE)) {
+       if (virtio_has_feature(dev, VIRTIO_BLK_F_BLK_SIZE)) {
                 virtio_cread(dev, struct virtio_blk_config, blk_size, 
&blk_size);
                 desc->blksz = blk_size;
                 if (!is_power_of_2(blk_size) || desc->blksz < 512)

Best regards

Heinrich

> 
>> +		virtio_cread(dev, struct virtio_blk_config, blk_size, &blk_size);
>> +		desc->blksz = blk_size;
>> +		if (!is_power_of_2(blk_size) || desc->blksz < 512)
>> +			return -EIO;
>> +	} else {
>> +		desc->blksz = 512;
>> +	}
>> +	desc->log2blksz = LOG2(desc->blksz);
>> +	priv->blksz_shift = desc->log2blksz - 9;
>> +	desc->lba >>= priv->blksz_shift;
>>   
>>   	return 0;
>>   }
>