> From: Max Gurtovoy <mgurtovoy@xxxxxxxxxx> > Sent: Wednesday, October 23, 2024 4:55 AM > > This field is only valid when the VIRTIO_BLK_F_BLK_SIZE feature bit is offered > by the device. > > The blk_size field actually represents the logical block size of the device. It is > always a power of two and typically ranges from 512 bytes to larger values > such as 4 KB. > > Add description for this field to provide clarity on its constraints. > > Fixes: https://github.com/oasis-tcs/virtio-spec/issues/208 > Reviewed-by: Daniel Verkamp <dverkamp@xxxxxxxxxxxx> > Reviewed-by: Stefan Hajnoczi <stefanha@xxxxxxxxxx> > Signed-off-by: Max Gurtovoy <mgurtovoy@xxxxxxxxxx> > --- > > changes from v4: > - reorder paragraphs (MST, Matias) > > --- > device-types/blk/description.tex | 33 ++++++++++++++++++++++++++++++++ > 1 file changed, 33 insertions(+) > > diff --git a/device-types/blk/description.tex b/device- > types/blk/description.tex > index 2712ada..093a139 100644 > --- a/device-types/blk/description.tex > +++ b/device-types/blk/description.tex > @@ -135,6 +135,10 @@ \subsection{Device configuration > layout}\label{sec:Device Types / Block Device / present. The availability of > the others all depend on various feature bits as indicated above. > > +The field \field{blk_size} reports the block size of the device, > +expressed in bytes. This field exists only if VIRTIO_BLK_F_BLK_SIZE is > +offered by the device. > + > The field \field{num_queues} only exists if VIRTIO_BLK_F_MQ is set. This > field specifies the number of queues. > > @@ -282,6 +286,13 @@ \subsection{Device Initialization}\label{sec:Device > Types / Block Device / Devic > > \drivernormative{\subsubsection}{Device Initialization}{Device Types / Block > Device / Device Initialization} > > +Drivers SHOULD negotiate VIRTIO_BLK_F_BLK_SIZE if the feature is > +offered by the device. When negotiated, drivers SHOULD interpret the > +\field{blk_size} as the logical block size. > + s/Drivers/The driver No need for plural. This will be consistent to rest of the spec requirement sections. Some existing requirements are using plural in blk and other places, but should not be used as base line. > +If the VIRTIO_BLK_F_BLK_SIZE feature is not offered by the device, then > +drivers MAY assume that the logical block size is 512 bytes. s/then drivers/then the driver/ > + > Drivers SHOULD NOT negotiate VIRTIO_BLK_F_FLUSH if they are incapable of > sending VIRTIO_BLK_T_FLUSH commands. > > @@ -319,6 +330,10 @@ \subsection{Device Initialization}\label{sec:Device > Types / Block Device / Devic > > \devicenormative{\subsubsection}{Device Initialization}{Device Types / Block > Device / Device Initialization} > > +Devices SHOULD offer VIRTIO_BLK_F_BLK_SIZE. When this feature is > +offered, devices MUST initialize \field{blk_size} to a power of two > +greater than or equal to 512. > + s/Devices/The device/ > Devices SHOULD always offer VIRTIO_BLK_F_FLUSH, and MUST offer it if > they offer VIRTIO_BLK_F_CONFIG_WCE. > Fixing above will be separate editorial patch unrelated to current in review. > @@ -879,6 +894,14 @@ \subsection{Device Operation}\label{sec:Device > Types / Block Device / Device Ope The length of \field{data} MUST be a > multiple of 512 bytes for VIRTIO_BLK_T_IN and VIRTIO_BLK_T_OUT > requests. > > +When the VIRTIO_BLK_F_BLK_SIZE feature is offered by the device, the > +length of \field{data} SHOULD be a multiple of \field{blk_size} for > +VIRTIO_BLK_T_IN and VIRTIO_BLK_T_OUT requests. > + > +When the VIRTIO_BLK_F_BLK_SIZE feature is offered by the device, the > +value of \field{sector} (multiplied by 512) SHOULD be a multiple of > +\field{blk_size} for VIRTIO_BLK_T_IN and VIRTIO_BLK_T_OUT requests. > + > The length of \field{data} MUST be a multiple of the size of struct > virtio_blk_discard_write_zeroes for VIRTIO_BLK_T_DISCARD, > VIRTIO_BLK_T_SECURE_ERASE and VIRTIO_BLK_T_WRITE_ZEROES requests. > @@ -966,6 +989,16 @@ \subsection{Device Operation}\label{sec:Device > Types / Block Device / Device Ope for a write request if the > VIRTIO_BLK_F_RO feature if offered, and MUST NOT write any data. > > +When the VIRTIO_BLK_F_BLK_SIZE feature is offered by the device, and > +the requested \field{sector} (multiplied by 512) is not an integer > +multiple of the device's \field{blk_size}, the device MAY set the > +\field{status} to VIRTIO_BLK_S_IOERR for both VIRTIO_BLK_T_IN and > VIRTIO_BLK_T_OUT requests. > + > +When the VIRTIO_BLK_F_BLK_SIZE feature is offered by the device, and > +the length of the requested \field{data} is not an integer multiple of > +the device's \field{blk_size}, the device MAY set the \field{status} to > +VIRTIO_BLK_S_IOERR for both VIRTIO_BLK_T_IN and VIRTIO_BLK_T_OUT > requests. > + > The device MUST set the \field{status} byte to VIRTIO_BLK_S_UNSUPP for > discard, secure erase and write zeroes commands if any unknown flag is set. > Furthermore, the device MUST set the \field{status} byte to > -- > 2.18.1 Apart from other small edits: Reviewed-by: Parav Pandit <parav@xxxxxxxxxx>