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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6ca20538-d2ab-4b73-8b1a-028f83828f3e@lunn.ch>
Date: Sat, 11 Oct 2025 19:25:55 +0200
From: Andrew Lunn <andrew@...n.ch>
To: "Michael S. Tsirkin" <mst@...hat.com>
Cc: linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
	Paolo Abeni <pabeni@...hat.com>, Jason Wang <jasowang@...hat.com>,
	Eugenio Pérez <eperezma@...hat.com>,
	Xuan Zhuo <xuanzhuo@...ux.alibaba.com>,
	Jonathan Corbet <corbet@....net>, kvm@...r.kernel.org,
	virtualization@...ts.linux.dev, linux-doc@...r.kernel.org
Subject: Re: [PATCH 1/3] virtio: dwords->qwords

On Thu, Oct 09, 2025 at 09:37:20AM -0400, Michael S. Tsirkin wrote:
> On Thu, Oct 09, 2025 at 02:31:04PM +0200, Andrew Lunn wrote:
> > On Thu, Oct 09, 2025 at 07:24:08AM -0400, Michael S. Tsirkin wrote:
> > > A "word" is 16 bit. 64 bit integers like virtio uses are not dwords,
> > > they are actually qwords.
> > 
> > I'm having trouble with this....
> > 
> > This bit makes sense. 4x 16bits = 64 bits.
> > 
> > > -static const u64 vhost_net_features[VIRTIO_FEATURES_DWORDS] = {
> > > +static const u64 vhost_net_features[VIRTIO_FEATURES_QWORDS] = {
> > 
> > If this was u16, and VIRTIO_FEATURES_QWORDS was 4, which the Q would
> > imply, than i would agree with what you are saying. But this is a u64
> > type.  It is already a QWORD, and this is an array of two of them.
> 
> I don't get what you are saying here.
> It's an array of qwords and VIRTIO_FEATURES_QWORDS tells you
> how many QWORDS are needed to fit all of them.
> 
> This is how C arrays are declared.
> 
> 
> > I think the real issue here is not D vs Q, but WORD. We have a default
> > meaning of a u16 for a word, especially in C. But that is not the
> > actual definition of a word a computer scientist would use. Wikipedia
> > has:
> > 
> >   In computing, a word is any processor design's natural unit of
> >   data. A word is a fixed-sized datum handled as a unit by the
> >   instruction set or the hardware of the processor.
> > 
> > A word can be any size. In this context, virtio is not referring to
> > the instruction set, but a protocol. Are all fields in this protocol
> > u64? Hence word is u64? And this is an array of two words? That would
> > make DWORD correct, it is two words.
> > 
> > If you want to change anything here, i would actually change WORD to
> > something else, maybe FIELD?
> > 
> > And i could be wrong here, i've not looked at the actual protocol, so
> > i've no idea if all fields in the protocol are u64. There are
> > protocols like this, IPv6 uses u32, not octets, and the length field
> > in the headers refer to the number of u32s in the header.
> > 
> > 	Andrew
> 
> 
> Virtio uses "dword" to mean "32 bits" in several places:

It also uses WORD to represent 32 bits:

void
vp_modern_get_driver_extended_features(struct virtio_pci_modern_device *mdev,
				       u64 *features)
{
	struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
	int i;

	virtio_features_zero(features);
	for (i = 0; i < VIRTIO_FEATURES_WORDS; i++) {
		u64 cur;

		vp_iowrite32(i, &cfg->guest_feature_select);
		cur = vp_ioread32(&cfg->guest_feature);
		features[i >> 1] |= cur << (32 * (i & 1));
	}
}

And this is a function dealing features. So this seems to suggest a
WORD is a u32, when dealing with features. A DWORD would thus be a
u64, making the current code correct.

As i said, the problem here is WORD. It means different things to
different people. And it even has different means to different parts
of the virtio code, as you pointed out.

If we want to change anything here, i suggest we change WORD to
something else, to try to avoid the problem that word could be a u16,
u32, or even a u42, depending on where it is used.

	Andrew