Thread (14 messages) 14 messages, 2 authors, 2025-10-12

Re: [PATCH 1/3] virtio: dwords->qwords

From: "Michael S. Tsirkin" <mst@redhat.com>
Date: 2025-10-12 19:17:59
Also in: kvm, linux-doc, lkml, virtualization

On Sun, Oct 12, 2025 at 04:39:06PM +0200, Andrew Lunn wrote:
quoted
quoted
DeviceFeaturesSel 0x014

Device (host) features word selection.
Writing to this register selects a set of 32 device feature bits accessible by reading from DeviceFeatures.

and

DriverFeaturesSel 0x024

Activated (guest) features word selection
Writing to this register selects a set of 32 activated feature bits accessible by writing to DriverFeatures.

I would interpret this as meaning a feature word is a u32. Hence a
DWORD is a u64, as the current code uses.

	Andrew

Hmm indeed.
At the same time, pci transport has:

         u8 padding[2];  /* Pad to full dword. */

and i2c has:

The \field{padding} is used to pad to full dword.

both of which use dword to mean 32 bit.

This comes from PCI which also does not define word but uses it
to mean 16 bit.
Yes, reading the spec, you need to consider the context 'word' is used
in. Maybe this is something which can be cleaned up, made uniform
across the whole document?
Yes and thanks for bringing this to my attention.
So MMIO can be "features set selection"

And pci can be "pad to 32 bit".


Less work than defining "word".
quoted
I don't have the problem changing everything to some other
wording completely but "chunk" is uninformative, and
more importantly does not give a clean way to refer to
2 chunks and 4 chunks.
Similarly, if we use "word" to mean 32 bit there is n clean
way to refer to 16 bits which we use a lot.


using word as 16 bit has the advantage that you
can say byte/word/dword/qword and these do not
cause too much confusion.
quoted
So I am still inclined to align everything on pci terminology
but interested to hear what alternative you suggest.
How about something simple:

#define VIRTIO_FEATURES_DU32WORDS	2
#define VIRTIO_FEATURES_U32WORDS	(VIRTIO_FEATURES_D32WORDS * 2)

or, if the spec moves away from using 'word':

#define VIRTIO_FEATURES_U64S	2
#define VIRTIO_FEATURES_U32S	(VIRTIO_FEATURES_U32S * 2)

The coding style says not to use Hungarian notation, but here it
actually make sense, and avoids the ambiguity in the spec.

	Andrew
I mean we can just define number of bits. Open-code / 32 or / 64
as needed. Hmm?


-- 
MST
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help