On Thu, Oct 10, 2024 at 07:07:25PM +0300, Max Gurtovoy wrote: > 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> > --- > device-types/blk/description.tex | 32 ++++++++++++++++++++++++++++++++ > 1 file changed, 32 insertions(+) > > diff --git a/device-types/blk/description.tex b/device-types/blk/description.tex > index 2712ada..0ddc223 100644 > --- a/device-types/blk/description.tex > +++ b/device-types/blk/description.tex > @@ -135,6 +135,9 @@ \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} exists only if VIRTIO_BLK_F_BLK_SIZE is offered by the device. > +This field reports the block size of the device, expressed in bytes. > + > The field \field{num_queues} only exists if VIRTIO_BLK_F_MQ is set. This field specifies > the number of queues. > > @@ -282,6 +285,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. > + > +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. > + > Drivers SHOULD NOT negotiate VIRTIO_BLK_F_FLUSH if they are incapable of > sending VIRTIO_BLK_T_FLUSH commands. > > @@ -319,6 +329,10 @@ \subsection{Device Initialization}\label{sec:Device Types / Block Device / Devic > > \devicenormative{\subsubsection}{Device Initialization}{Device Types / Block Device / Device Initialization} > > +Devices SHOULD always offer VIRTIO_BLK_F_BLK_SIZE feature. When this feature is > +offered, devices MUST initialize \field{blk_size} to a power of two greater > +than or equal to 512. > + > Devices SHOULD always offer VIRTIO_BLK_F_FLUSH, and MUST offer it > if they offer VIRTIO_BLK_F_CONFIG_WCE. > > @@ -879,6 +893,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 +988,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, the device MAY > +set the \field{status} to VIRTIO_BLK_S_IOERR for VIRTIO_BLK_T_IN or > +VIRTIO_BLK_T_OUT requests if the requested \field{sector} (multiplied by 512) > +is not an integer multiple of the device's \field{blk_size}. > + I think the whole paragraph could be rewritten as: `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, the device MAY > +set the \field{status} to VIRTIO_BLK_S_IOERR for VIRTIO_BLK_T_IN or > +VIRTIO_BLK_T_OUT requests if the length of the requested \field{data} is not an > +integer multiple of the device's \field{blk_size}. > + I think this paragraph also could be rewritten as I proposed above. Matias
