diff mbox series

pci-ids: sync docs + header

Message ID 20220930073553.1626190-1-kraxel@redhat.com (mailing list archive)
State New, archived
Headers show
Series pci-ids: sync docs + header | expand

Commit Message

Gerd Hoffmann Sept. 30, 2022, 7:35 a.m. UTC 
docs/specs/pci-ids.txt and include/hw/pci/pci.h are out of sync,
fix that.  Try improve the comment which points to pci-ids.txt.

Also drop the list of modern virtio devices and explain how they
are calculated instead.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 docs/specs/pci-ids.txt | 19 ++++++++++---------
 include/hw/pci/pci.h   |  7 ++++++-
 2 files changed, 16 insertions(+), 10 deletions(-)

Comments

Eric Auger Sept. 30, 2022, 8:07 a.m. UTC | #1 
Hi Gerd,

On 9/30/22 09:35, Gerd Hoffmann wrote:
> docs/specs/pci-ids.txt and include/hw/pci/pci.h are out of sync,
> fix that.  Try improve the comment which points to pci-ids.txt.
> 
> Also drop the list of modern virtio devices and explain how they
> are calculated instead.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  docs/specs/pci-ids.txt | 19 ++++++++++---------
>  include/hw/pci/pci.h   |  7 ++++++-
>  2 files changed, 16 insertions(+), 10 deletions(-)
> 
> diff --git a/docs/specs/pci-ids.txt b/docs/specs/pci-ids.txt
> index dd6859d039d0..6be7bc108d66 100644
> --- a/docs/specs/pci-ids.txt
> +++ b/docs/specs/pci-ids.txt
> @@ -22,16 +22,17 @@ maintained as part of the virtio specification.
>  1af4:1004  SCSI host bus adapter device (legacy)
>  1af4:1005  entropy generator device (legacy)
>  1af4:1009  9p filesystem device (legacy)
> +1af4:1012  vsock device (legacy)
> +1af4:1013  pmem device (legacy)
> +1af4:1014  iommu device (legacy)
> +1af4:1015  mem device (legacy)
While I understand the 1af4:1040 range, I do not get where the above ids
come from. Could we add an explanation in the intro. Also there, we may
fix s/Note that this allocation separate/Note that this allocation is
separate. Also why do we have a hole inbetween 1009 and 1012?
>  
> -1af4:1041  network device (modern)
> -1af4:1042  block device (modern)
> -1af4:1043  console device (modern)
> -1af4:1044  entropy generator device (modern)
> -1af4:1045  balloon device (modern)
> -1af4:1048  SCSI host bus adapter device (modern)
> -1af4:1049  9p filesystem device (modern)
> -1af4:1050  virtio gpu device (modern)
> -1af4:1052  virtio input device (modern)
> +1af4:1040  Start of id range for modern virtio devices.  The pci device
> +           id is is calculated from the virtio device id by adding the
s/is is/is
> +           0x1040 offset.  The virtio ids are defined in the virtio
> +           specification.  The linux kernel has a header file with
> +           defines for all virtio ids (linux/virtio_ids.h), qemu has a
> +           copy in include/standard-headers/.
>  
>  1af4:10f0  Available for experimental usage without registration.  Must get
>     to      official ID when the code leaves the test lab (i.e. when seeking
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index b54b6ef88fc3..3b852199660c 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -71,7 +71,12 @@ extern bool pci_available;
>  #define PCI_DEVICE_ID_INTEL_82557        0x1229
>  #define PCI_DEVICE_ID_INTEL_82801IR      0x2922
>  
> -/* Red Hat / Qumranet (for QEMU) -- see pci-ids.txt */
> +/*
> + * Red Hat / Qumranet (for QEMU)
> + *
> + * These are documented in docs/specs/pci-ids.txt
> + * PLEASE KEEP HEADER + DOCS IN SYNC
> + */
>  #define PCI_VENDOR_ID_REDHAT_QUMRANET    0x1af4
>  #define PCI_SUBVENDOR_ID_REDHAT_QUMRANET 0x1af4
>  #define PCI_SUBDEVICE_ID_QEMU            0x1100

Adding Jean in copy as we discussed that in
https://lore.kernel.org/qemu-devel/5641321a-4bec-2ca9-bb14-d5ed883a9ded@redhat.com/

Thanks

Eric
Michael S. Tsirkin Sept. 30, 2022, 9:22 a.m. UTC | #2 
On Fri, Sep 30, 2022 at 09:35:53AM +0200, Gerd Hoffmann wrote:
> docs/specs/pci-ids.txt and include/hw/pci/pci.h are out of sync,
> fix that.  Try improve the comment which points to pci-ids.txt.
> 
> Also drop the list of modern virtio devices and explain how they
> are calculated instead.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  docs/specs/pci-ids.txt | 19 ++++++++++---------
>  include/hw/pci/pci.h   |  7 ++++++-
>  2 files changed, 16 insertions(+), 10 deletions(-)
> 
> diff --git a/docs/specs/pci-ids.txt b/docs/specs/pci-ids.txt
> index dd6859d039d0..6be7bc108d66 100644
> --- a/docs/specs/pci-ids.txt
> +++ b/docs/specs/pci-ids.txt
> @@ -22,16 +22,17 @@ maintained as part of the virtio specification.
>  1af4:1004  SCSI host bus adapter device (legacy)
>  1af4:1005  entropy generator device (legacy)
>  1af4:1009  9p filesystem device (legacy)
> +1af4:1012  vsock device (legacy)
> +1af4:1013  pmem device (legacy)
> +1af4:1014  iommu device (legacy)
> +1af4:1015  mem device (legacy)


Wait, how come we have legacy vsock/pmem/iommu/mem?
They were only introduced after 1.0.


>  
> -1af4:1041  network device (modern)
> -1af4:1042  block device (modern)
> -1af4:1043  console device (modern)
> -1af4:1044  entropy generator device (modern)
> -1af4:1045  balloon device (modern)
> -1af4:1048  SCSI host bus adapter device (modern)
> -1af4:1049  9p filesystem device (modern)
> -1af4:1050  virtio gpu device (modern)
> -1af4:1052  virtio input device (modern)
> +1af4:1040  Start of id range for modern virtio devices.  The pci device
> +           id is is calculated from the virtio device id by adding the
> +           0x1040 offset.  The virtio ids are defined in the virtio
> +           specification.  The linux kernel has a header file with
> +           defines for all virtio ids (linux/virtio_ids.h), qemu has a
> +           copy in include/standard-headers/.
>  
>  1af4:10f0  Available for experimental usage without registration.  Must get
>     to      official ID when the code leaves the test lab (i.e. when seeking
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index b54b6ef88fc3..3b852199660c 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -71,7 +71,12 @@ extern bool pci_available;
>  #define PCI_DEVICE_ID_INTEL_82557        0x1229
>  #define PCI_DEVICE_ID_INTEL_82801IR      0x2922
>  
> -/* Red Hat / Qumranet (for QEMU) -- see pci-ids.txt */
> +/*
> + * Red Hat / Qumranet (for QEMU)
> + *
> + * These are documented in docs/specs/pci-ids.txt
> + * PLEASE KEEP HEADER + DOCS IN SYNC
> + */
>  #define PCI_VENDOR_ID_REDHAT_QUMRANET    0x1af4
>  #define PCI_SUBVENDOR_ID_REDHAT_QUMRANET 0x1af4
>  #define PCI_SUBDEVICE_ID_QEMU            0x1100
> -- 
> 2.37.3
Peter Maydell Sept. 30, 2022, 9:29 a.m. UTC | #3 
On Fri, 30 Sept 2022 at 08:35, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> docs/specs/pci-ids.txt and include/hw/pci/pci.h are out of sync,
> fix that.  Try improve the comment which points to pci-ids.txt.
>
> Also drop the list of modern virtio devices and explain how they
> are calculated instead.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  docs/specs/pci-ids.txt | 19 ++++++++++---------
>  include/hw/pci/pci.h   |  7 ++++++-
>  2 files changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/docs/specs/pci-ids.txt b/docs/specs/pci-ids.txt
> index dd6859d039d0..6be7bc108d66 100644
> --- a/docs/specs/pci-ids.txt
> +++ b/docs/specs/pci-ids.txt
> @@ -22,16 +22,17 @@ maintained as part of the virtio specification.
>  1af4:1004  SCSI host bus adapter device (legacy)
>  1af4:1005  entropy generator device (legacy)
>  1af4:1009  9p filesystem device (legacy)
> +1af4:1012  vsock device (legacy)
> +1af4:1013  pmem device (legacy)
> +1af4:1014  iommu device (legacy)
> +1af4:1015  mem device (legacy)
>
> -1af4:1041  network device (modern)
> -1af4:1042  block device (modern)
> -1af4:1043  console device (modern)
> -1af4:1044  entropy generator device (modern)
> -1af4:1045  balloon device (modern)
> -1af4:1048  SCSI host bus adapter device (modern)
> -1af4:1049  9p filesystem device (modern)
> -1af4:1050  virtio gpu device (modern)
> -1af4:1052  virtio input device (modern)
> +1af4:1040  Start of id range for modern virtio devices.  The pci device
> +           id is is calculated from the virtio device id by adding the
> +           0x1040 offset.  The virtio ids are defined in the virtio
> +           specification.  The linux kernel has a header file with
> +           defines for all virtio ids (linux/virtio_ids.h), qemu has a
> +           copy in include/standard-headers/.

I think you should also specify the end point of this range, in the
same way that the "experimental usage" range has both a defined
start and end point. Otherwise we have no way to know how much
(if any) of the space before 10f0 is available for other uses.

Nits: capitalization: ID, PCI, Linux.

>  1af4:10f0  Available for experimental usage without registration.  Must get
>     to      official ID when the code leaves the test lab (i.e. when seeking

thanks
-- PMM
Gerd Hoffmann Sept. 30, 2022, 9:54 a.m. UTC | #4 
On Fri, Sep 30, 2022 at 05:22:33AM -0400, Michael S. Tsirkin wrote:
> On Fri, Sep 30, 2022 at 09:35:53AM +0200, Gerd Hoffmann wrote:
> > docs/specs/pci-ids.txt and include/hw/pci/pci.h are out of sync,
> > fix that.  Try improve the comment which points to pci-ids.txt.
> > 
> > Also drop the list of modern virtio devices and explain how they
> > are calculated instead.
> > 
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > ---
> >  docs/specs/pci-ids.txt | 19 ++++++++++---------
> >  include/hw/pci/pci.h   |  7 ++++++-
> >  2 files changed, 16 insertions(+), 10 deletions(-)
> > 
> > diff --git a/docs/specs/pci-ids.txt b/docs/specs/pci-ids.txt
> > index dd6859d039d0..6be7bc108d66 100644
> > --- a/docs/specs/pci-ids.txt
> > +++ b/docs/specs/pci-ids.txt
> > @@ -22,16 +22,17 @@ maintained as part of the virtio specification.
> >  1af4:1004  SCSI host bus adapter device (legacy)
> >  1af4:1005  entropy generator device (legacy)
> >  1af4:1009  9p filesystem device (legacy)
> > +1af4:1012  vsock device (legacy)
> > +1af4:1013  pmem device (legacy)
> > +1af4:1014  iommu device (legacy)
> > +1af4:1015  mem device (legacy)
> 
> Wait, how come we have legacy vsock/pmem/iommu/mem?
> They were only introduced after 1.0.

I've just synced with the header file, and the #defines there
seem to be actually used:

kraxel@sirius ~/projects/qemu (pci-ids)# git grep PCI_DEVICE_ID_VIRTIO_IOMMU
hw/virtio/virtio-iommu-pci.c:    pcidev_k->device_id = PCI_DEVICE_ID_VIRTIO_IOMMU;
include/hw/pci/pci.h:#define PCI_DEVICE_ID_VIRTIO_IOMMU       0x1014

But, yes, the question is valid.  1.0-only devices should not need
a legacy id.  Dunno how that happened ...

take care,
  Gerd
diff mbox series

Patch

diff --git a/docs/specs/pci-ids.txt b/docs/specs/pci-ids.txt
index dd6859d039d0..6be7bc108d66 100644
--- a/docs/specs/pci-ids.txt
+++ b/docs/specs/pci-ids.txt
@@ -22,16 +22,17 @@  maintained as part of the virtio specification.
 1af4:1004  SCSI host bus adapter device (legacy)
 1af4:1005  entropy generator device (legacy)
 1af4:1009  9p filesystem device (legacy)
+1af4:1012  vsock device (legacy)
+1af4:1013  pmem device (legacy)
+1af4:1014  iommu device (legacy)
+1af4:1015  mem device (legacy)
 
-1af4:1041  network device (modern)
-1af4:1042  block device (modern)
-1af4:1043  console device (modern)
-1af4:1044  entropy generator device (modern)
-1af4:1045  balloon device (modern)
-1af4:1048  SCSI host bus adapter device (modern)
-1af4:1049  9p filesystem device (modern)
-1af4:1050  virtio gpu device (modern)
-1af4:1052  virtio input device (modern)
+1af4:1040  Start of id range for modern virtio devices.  The pci device
+           id is is calculated from the virtio device id by adding the
+           0x1040 offset.  The virtio ids are defined in the virtio
+           specification.  The linux kernel has a header file with
+           defines for all virtio ids (linux/virtio_ids.h), qemu has a
+           copy in include/standard-headers/.
 
 1af4:10f0  Available for experimental usage without registration.  Must get
    to      official ID when the code leaves the test lab (i.e. when seeking
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index b54b6ef88fc3..3b852199660c 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -71,7 +71,12 @@  extern bool pci_available;
 #define PCI_DEVICE_ID_INTEL_82557        0x1229
 #define PCI_DEVICE_ID_INTEL_82801IR      0x2922
 
-/* Red Hat / Qumranet (for QEMU) -- see pci-ids.txt */
+/*
+ * Red Hat / Qumranet (for QEMU)
+ *
+ * These are documented in docs/specs/pci-ids.txt
+ * PLEASE KEEP HEADER + DOCS IN SYNC
+ */
 #define PCI_VENDOR_ID_REDHAT_QUMRANET    0x1af4
 #define PCI_SUBVENDOR_ID_REDHAT_QUMRANET 0x1af4
 #define PCI_SUBDEVICE_ID_QEMU            0x1100