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> I have a general question here. Do we need this field before feature negotiation? If not, I would actually say here that it exists only if feature has been negotiated. This opens the door for devices to detect legacy drivers that do not read block size and work around, somehow. > --- > 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. I would reorder here, too. 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 +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 I don't get what does always mean here. also the feature? > 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}. > + > +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}. > + > 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
