bhyve e1000: Sanitize transmit ring indices.

[jhb]

Summary

When preparing to transmit pending packets, ensure that the head (TDH)
and tail (TDT) indices are in bounds. Note that validating values
when they are written is not sufficient along as the transmit length
(TDLEN) could be changed turning a value that was valid when written
into an out of bounds value.

While here, add further restrictions to the head register (TDH). The
manual states that writing to this value while transmit is enabled can
cause unexpected behavior and that it should only be written after a
reset. As such, ignore attempts to write while transmit is active,
and also ignore writes of non-zero values. Later e1000 chipsets have
this register as read-only.

PR: 264567
Reported by: Robert Morris <[e-mail]>
Sponsored by: The FreeBSD Foundation

[emaste]

usr.sbin/bhyve/pci_e82545.c

1470

Is it possible esc_TDLEN could be zero?

[jhb]

usr.sbin/bhyve/pci_e82545.c

1470

Mmm, yes.

[jhb]

• Ignore transmit attempts with an empty ring.

[emaste]

usr.sbin/bhyve/pci_e82545.c

1744–1747

This seems strange to me, why must tdh only be zero? Even if it seems quite unlikely there's a guest that would do so. Maybe commit in two parts so that the rest of this can be MFCd in days, while this could wait longer to catch anything unusual.

[jhb]

usr.sbin/bhyve/pci_e82545.c

1744–1747

The earlier comment already alluded to this. There is no valid reason to really write to this register except to clear it on reset. Future e1000 chipsets make this register read-only and clear it to 0 on reset implicitly, so drivers that support a range of e1000 chipsets like if_em or the like shouldn't be writing non-zero values. Also, the documented way to add new credits to the transmit ring to indicate there are packets to send is to only write to the tail (TDT). TDH is really owned by the controller as the producer index of the queue.

The description from the manual is:

<quote>
This register contains the head pointer for the transmit descriptor ring. It holds a value that is an offset from the base, and indicates the in–progress descriptor. It points to a 16-byte datum. Hardware controls this pointer. The only time that software should write to this register is after a reset (TCTL.RST or CTRL.RST) and before enabling the transmit function (TCTL.EN). If software were to write to this register while the transmit function was enabled, the on-chip descriptor buffers can be invalidated and indeterminate operation can result. Reading the transmit descriptor head to determine which buffers have been used (and can be returned to the memory pool) is not reliable.
</quote>

While it doesn't explicitly say zero, no other value makes sense after reset.

[emaste]

usr.sbin/bhyve/pci_e82545.c

1744–1747

Sounds good