[PATCH] pci: switch from subsystem id to device id

[Michael S. Tsirkin]

Switch virtio pci to use standard device id instead of using the
subsystem id.

As a nice side effect, this allows us to make non-transitional
devices use IDs 0x40 and up, this excludes legacy windows drivers
(which ignored the revision ID even though we told them not to)
since these match the specific subsystem ID.

And, it's probably a good idea to allow them to do this if they
want to, so there's no requirement for drivers to match all
subsystem/vendor ID configurations, but they may do so.

To avoid confusion, say "PCI Device ID" and
"PCI Subsystem ID" everywhere, prefix "PCI"
for other standard registers, for consistency.

VIRTIO-102

Note: issue reporter suggested 0x10XX where XX is the virtio
device ID. This would break all legacy drivers, historically
PCI Device ID is off by one: 0xFFF + Virtio Device ID.

Signed-off-by: Michael S. Tsirkin <***@redhat.com>
---
content.tex | 45 +++++++++++++++++++++++++++++++--------------
1 file changed, 31 insertions(+), 14 deletions(-)

diff --git a/content.tex b/content.tex
index 6d6a756..da64e68 100644
--- a/content.tex
+++ b/content.tex
@@ -963,31 +963,48 @@ respectively.

\subsection{PCI Device Discovery}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Discovery}

-Any PCI device with Vendor ID 0x1AF4, and Device ID 0x1000 through
-0x103F inclusive is a virtio device\footnote{The actual value within this range is ignored
-}.
-
-The Subsystem Device ID indicates which virtio device is
-supported by the device, as indicated in section \ref{sec:Device Types}.
+Any PCI device with PCI Vendor ID 0x1AF4, and PCI Device ID 0x1000 through
+0x103F inclusive is a virtio device. The actual value within this range
+indicates which virtio device is supported by the device.
+The PCI Device ID is calculated by adding 0xFFF to the Virtio Device ID,
+as indicated in section \ref{sec:Device Types}.

\devicenormative{\subsubsection}{PCI Device Discovery}{Virtio Transport Options / Virtio Over PCI Bus / PCI Device Discovery}
-The Subsystem Vendor ID SHOULD reflect
-the PCI Vendor ID of the environment (it's currently only used
-for informational purposes by the driver).

-Non-transitional devices SHOULD have a Revision ID of 1 or higher.
+Devices MUST have the PCI Vendor ID 0x1AF4.
+Devices MUST have the PCI Device ID calculated by adding 0xFFF
+to the Virtio Device ID,
+as indicated in section \ref{sec:Device Types}.
+For example, the network card device with the Virtio Device ID 1
+has the PCI Device ID 0x1000.
+
+The PCI Subsystem Vendor ID and the PCI Subsystem Device ID MAY reflect
+the PCI Vendor and Device ID of the environment (for informational purposes by the driver).
+
+Non-transitional devices SHOULD have a PCI Revision ID of 1 or higher.
+Non-transitional devices SHOULD have a PCI Subsystem Device ID of 0x40 or higher.

This is to reduce the chance of a legacy driver attempting
to drive the device.

\drivernormative{\subsubsection}{PCI Device Discovery}{Virtio Transport Options / Virtio Over PCI Bus / PCI Device Discovery}
-All drivers MUST match devices with any Revision ID, this
-is to allow devices to be versioned without breaking drivers.
+Drivers MUST match devices with the PCI Vendor ID 0x1AF4 and
+the PCI Device ID calculated by adding 0xFFF to the Virtio Device ID,
+as indicated in section \ref{sec:Device Types}.
+
+Drivers MUST match devices with any Revision ID, and
+MAY match devices with any Subsystem Vendor ID and
+Subsystem Device ID, this is to allow devices to be
+versioned without breaking drivers.

-Drivers MUST match any Revision ID value.
+Drivers MUST match any PCI Revision ID value.
+Drivers MAY match any PCI Subsystem Vendor ID and any
+PCI Subsystem Device ID value.

\subsubsection{Legacy Interfaces: A Note on PCI Device Discovery}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Discovery / Legacy Interfaces: A Note on PCI Device Discovery}
-Transitional devices MUST have a Revision ID of 0.
+Transitional devices MUST have a PCI Revision ID of 0.
+Transitional devices MUST have the PCI Subsystem Device ID
+matching the Virtio Device ID, as indicated in section \ref{sec:Device Types}.

This is to match legacy drivers.

[Gerd Hoffmann]

Hi,

Post by Michael S. Tsirkin
Note: issue reporter suggested 0x10XX where XX is the virtio
device ID. This would break all legacy drivers, historically
PCI Device ID is off by one: 0xFFF + Virtio Device ID.

Adding 0xfff works for for net+block only.
There seems to be no fixed scheme:

nilsson kraxel ~/projects/qemu/include # grep VIRTIO_ID hw/virtio/*.h
hw/virtio/virtio-balloon.h:#define VIRTIO_ID_BALLOON 5
hw/virtio/virtio-blk.h:#define VIRTIO_ID_BLOCK 2
hw/virtio/virtio-net.h:#define VIRTIO_ID_NET 1
hw/virtio/virtio-rng.h:#define VIRTIO_ID_RNG 4
hw/virtio/virtio-scsi.h:#define VIRTIO_ID_SCSI 8
hw/virtio/virtio-serial.h:#define VIRTIO_ID_CONSOLE 3

nilsson kraxel ~/projects/qemu/include # grep VIRTIO hw/pci/*.h
hw/pci/pci.h:#define PCI_DEVICE_ID_VIRTIO_NET 0x1000
hw/pci/pci.h:#define PCI_DEVICE_ID_VIRTIO_BLOCK 0x1001
hw/pci/pci.h:#define PCI_DEVICE_ID_VIRTIO_BALLOON 0x1002
hw/pci/pci.h:#define PCI_DEVICE_ID_VIRTIO_CONSOLE 0x1003
hw/pci/pci.h:#define PCI_DEVICE_ID_VIRTIO_SCSI 0x1004
hw/pci/pci.h:#define PCI_DEVICE_ID_VIRTIO_RNG 0x1005
hw/pci/pci.h:#define PCI_DEVICE_ID_VIRTIO_9P 0x1009

cheers,
Gerd

[Michael S. Tsirkin]

Post by Gerd Hoffmann
Hi,

Post by Michael S. Tsirkin
Note: issue reporter suggested 0x10XX where XX is the virtio
device ID. This would break all legacy drivers, historically
PCI Device ID is off by one: 0xFFF + Virtio Device ID.

Adding 0xfff works for for net+block only.
nilsson kraxel ~/projects/qemu/include # grep VIRTIO_ID hw/virtio/*.h
hw/virtio/virtio-balloon.h:#define VIRTIO_ID_BALLOON 5
hw/virtio/virtio-blk.h:#define VIRTIO_ID_BLOCK 2
hw/virtio/virtio-net.h:#define VIRTIO_ID_NET 1
hw/virtio/virtio-rng.h:#define VIRTIO_ID_RNG 4
hw/virtio/virtio-scsi.h:#define VIRTIO_ID_SCSI 8
hw/virtio/virtio-serial.h:#define VIRTIO_ID_CONSOLE 3
nilsson kraxel ~/projects/qemu/include # grep VIRTIO hw/pci/*.h
hw/pci/pci.h:#define PCI_DEVICE_ID_VIRTIO_NET 0x1000
hw/pci/pci.h:#define PCI_DEVICE_ID_VIRTIO_BLOCK 0x1001
hw/pci/pci.h:#define PCI_DEVICE_ID_VIRTIO_BALLOON 0x1002
hw/pci/pci.h:#define PCI_DEVICE_ID_VIRTIO_CONSOLE 0x1003
hw/pci/pci.h:#define PCI_DEVICE_ID_VIRTIO_SCSI 0x1004
hw/pci/pci.h:#define PCI_DEVICE_ID_VIRTIO_RNG 0x1005
hw/pci/pci.h:#define PCI_DEVICE_ID_VIRTIO_9P 0x1009
cheers,
Gerd

You are right of course.
So we'll have to have a table for transitional devices, and something
like 0x1040 + device id or 0x1009 + device id for non transitional ones.

[Michael S. Tsirkin]

Switch virtio pci to use standard device id instead of using the
subsystem id.

Unfortunately, there's no system to the way KVM allocated
device IDs to virtio devices, we'll just have to
specify these using a table, and use a new range for
future devices. For existing devices this results in
two possible IDs that all drivers will need to match.
Unfortunate, but the cost is small.

As a nice side effect, this allows us to make non-transitional
devices use IDs 0x40 and up, this reduces even further the
chance that a non transitional device will match legacy drivers.

And, it's probably a good idea to allow drivers to match
specific subsystem IDs if they
want to, so relax requirement for drivers to match all
subsystem/vendor ID configurations, but allow them to do so.

To avoid confusion, say "PCI Device ID" and
"PCI Subsystem ID" everywhere, prefix "PCI"
for other standard registers, for consistency.

VIRTIO-102

Note: issue reporter suggested 0x10XX where XX is the virtio
device ID. This would conflict with legacy devices, which seem
to have used 7 IDs in the range 0x1000 to 0x103f without any
system. Let's use a new range 0x1040 to 0x107f for
non-transitional devices, and add a table documenting the
transitional IDs used by in practice.

Cc: Andrew Thornton <***@google.com>
Signed-off-by: Michael S. Tsirkin <***@redhat.com>
---

Changes from v1:
rule suggested for compatibility didn't work
in practice. Looks like we'll just have to use two IDs
for existing devices.

content.tex | 45 +++++++++++++++++++++++++++++++--------------
1 file changed, 31 insertions(+), 14 deletions(-)

diff --git a/content.tex b/content.tex
index b82748a..5936154 100644
--- a/content.tex
+++ b/content.tex
@@ -963,31 +963,48 @@ respectively.

\subsection{PCI Device Discovery}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Discovery}

-Any PCI device with Vendor ID 0x1AF4, and Device ID 0x1000 through
-0x103F inclusive is a virtio device\footnote{The actual value within this range is ignored
-}.
-
-The Subsystem Device ID indicates which virtio device is
-supported by the device, as indicated in section \ref{sec:Device Types}.
+Any PCI device with PCI Vendor ID 0x1AF4, and PCI Device ID 0x1000 through
+0x103F inclusive is a virtio device. The actual value within this range
+indicates which virtio device is supported by the device.
+The PCI Device ID is calculated by adding 0xFFF to the Virtio Device ID,
+as indicated in section \ref{sec:Device Types}.

\devicenormative{\subsubsection}{PCI Device Discovery}{Virtio Transport Options / Virtio Over PCI Bus / PCI Device Discovery}
-The Subsystem Vendor ID SHOULD reflect
-the PCI Vendor ID of the environment (it's currently only used
-for informational purposes by the driver).

-Non-transitional devices SHOULD have a Revision ID of 1 or higher.
+Devices MUST have the PCI Vendor ID 0x1AF4.
+Devices MUST have the PCI Device ID calculated by adding 0xFFF
+to the Virtio Device ID,
+as indicated in section \ref{sec:Device Types}.
+For example, the network card device with the Virtio Device ID 1
+has the PCI Device ID 0x1000.
+
+The PCI Subsystem Vendor ID and the PCI Subsystem Device ID MAY reflect
+the PCI Vendor and Device ID of the environment (for informational purposes by the driver).
+
+Non-transitional devices SHOULD have a PCI Revision ID of 1 or higher.
+Non-transitional devices SHOULD have a PCI Subsystem Device ID of 0x40 or higher.

This is to reduce the chance of a legacy driver attempting
to drive the device.

\drivernormative{\subsubsection}{PCI Device Discovery}{Virtio Transport Options / Virtio Over PCI Bus / PCI Device Discovery}
-All drivers MUST match devices with any Revision ID, this
-is to allow devices to be versioned without breaking drivers.
+Drivers MUST match devices with the PCI Vendor ID 0x1AF4 and
+the PCI Device ID calculated by adding 0xFFF to the Virtio Device ID,
+as indicated in section \ref{sec:Device Types}.
+
+Drivers MUST match devices with any Revision ID, and
+MAY match devices with any Subsystem Vendor ID and
+Subsystem Device ID, this is to allow devices to be
+versioned without breaking drivers.

-Drivers MUST match any Revision ID value.
+Drivers MUST match any PCI Revision ID value.
+Drivers MAY match any PCI Subsystem Vendor ID and any
+PCI Subsystem Device ID value.

\subsubsection{Legacy Interfaces: A Note on PCI Device Discovery}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Discovery / Legacy Interfaces: A Note on PCI Device Discovery}
-Transitional devices MUST have a Revision ID of 0.
+Transitional devices MUST have a PCI Revision ID of 0.
+Transitional devices MUST have the PCI Subsystem Device ID
+matching the Virtio Device ID, as indicated in section \ref{sec:Device Types}.

This is to match legacy drivers.

[Rusty Russell]

Post by Michael S. Tsirkin
Switch virtio pci to use standard device id instead of using the
subsystem id.
+Drivers MUST match devices with the PCI Vendor ID 0x1AF4 and
+the PCI Device ID calculated by adding 0xFFF to the Virtio Device ID,
+as indicated in section \ref{sec:Device Types}.
+
+Drivers MUST match devices with any Revision ID, and
+MAY match devices with any Subsystem Vendor ID and
+Subsystem Device ID, this is to allow devices to be
+versioned without breaking drivers.
-Drivers MUST match any Revision ID value.
+Drivers MUST match any PCI Revision ID value.
+Drivers MAY match any PCI Subsystem Vendor ID and any
+PCI Subsystem Device ID value.

Looks like those last two are a repeated paragraph?

Otherwise it looks reasonable.

Here's a diagram of my current understanding:

+----------------+----------------+
| Device ID | Vendor ID |
+----------------+----------------+
| Status | Command |
+----------------+----------------+
| Class code | Rev ID |
+----------------+----------------+
| BIST |Header T|Lat tim|Cache LS|
+----------------+----------------+
| Base Address registers |
| |
| |
| |
| |
| |
+----------------+----------------+
| Cardbus CIS Pointer |
+----------------+----------------+
|Subsys Device ID|Subsys Vendor ID|
+----------------+----------------+
...

Old spec:
Device ID: 0x1000 - 0x103F (val ignored)
Vendor ID: 0x1AF4
Subsys Device ID: virtio type (eg. 1 - 13)
Subsys Vendor ID: device author
Revision ID: 0 (transitional) 1 (non-trans)

Proposed spec:
Device ID: 0x1000 - 0x103F (== type + 0x0FFF)
Vendor ID: 0x1AF4
Subsys Device ID: virtio type (trans) 0x40+ (non-trans)
Subsys Vendor ID: device author
Revision ID: 0 (transitional) any (non-trans)

Thanks!
Rusty.