| Message ID | 1353320494-15033-5-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Nov 19, 2012 at 11:21:33AM +0100, Paolo Bonzini wrote: > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > docs/specs/pci-ids.txt | 2 ++ > hw/ivshmem.c | 2 +- > hw/pci.h | 1 + > 3 file modificati, 4 inserzioni(+). 1 rimozione(-) > > diff --git a/docs/specs/pci-ids.txt b/docs/specs/pci-ids.txt > index 28dcf90..6b5cf42 100644 > --- a/docs/specs/pci-ids.txt > +++ b/docs/specs/pci-ids.txt > @@ -29,4 +29,6 @@ maintained as part of the virtio specification. > 1af4:1100 Used as PCI Subsystem ID for existing hardware devices emulated > by qemu. > > +1af4:1110 ivshmem device (shared memory, docs/specs/ivshmem_device_spec.txt) > + > All other device IDs are reserved. > diff --git a/hw/ivshmem.c b/hw/ivshmem.c > index f6dbb21..8adeb2c 100644 > --- a/hw/ivshmem.c > +++ b/hw/ivshmem.c > @@ -800,7 +800,7 @@ static void ivshmem_class_init(ObjectClass *klass, void *data) > k->init = pci_ivshmem_init; > k->exit = pci_ivshmem_uninit; > k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET; > - k->device_id = 0x1110; > + k->device_id = PCI_DEVICE_ID_QUMRANET_IVSHMEM; > k->class_id = PCI_CLASS_MEMORY_RAM; > dc->reset = ivshmem_reset; > dc->props = ivshmem_properties; > diff --git a/hw/pci.h b/hw/pci.h > index 0719521..3704d5f 100644 > --- a/hw/pci.h > +++ b/hw/pci.h > @@ -78,6 +78,7 @@ > #define PCI_DEVICE_ID_VIRTIO_SCSI 0x1004 > #define PCI_DEVICE_ID_VIRTIO_RNG 0x1005 > #define PCI_DEVICE_ID_VIRTIO_9P 0x1009 > +#define PCI_DEVICE_ID_QUMRANET_IVSHMEM 0x1110 > > #define FMT_PCIBUS PRIx64 Why _QUMRANET I wonder? > -- > 1.7.12.1 >
Il 12/12/2012 15:22, Michael S. Tsirkin ha scritto: >> > @@ -29,4 +29,6 @@ maintained as part of the virtio specification. >> > 1af4:1100 Used as PCI Subsystem ID for existing hardware devices emulated >> > by qemu. >> > >> > +1af4:1110 ivshmem device (shared memory, docs/specs/ivshmem_device_spec.txt) >> > + >> > All other device IDs are reserved. >> > diff --git a/hw/ivshmem.c b/hw/ivshmem.c >> > index f6dbb21..8adeb2c 100644 >> > --- a/hw/ivshmem.c >> > +++ b/hw/ivshmem.c >> > @@ -800,7 +800,7 @@ static void ivshmem_class_init(ObjectClass *klass, void *data) >> > k->init = pci_ivshmem_init; >> > k->exit = pci_ivshmem_uninit; >> > k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET; >> > - k->device_id = 0x1110; >> > + k->device_id = PCI_DEVICE_ID_QUMRANET_IVSHMEM; >> > k->class_id = PCI_CLASS_MEMORY_RAM; >> > dc->reset = ivshmem_reset; >> > dc->props = ivshmem_properties; >> > diff --git a/hw/pci.h b/hw/pci.h >> > index 0719521..3704d5f 100644 >> > --- a/hw/pci.h >> > +++ b/hw/pci.h >> > @@ -78,6 +78,7 @@ >> > #define PCI_DEVICE_ID_VIRTIO_SCSI 0x1004 >> > #define PCI_DEVICE_ID_VIRTIO_RNG 0x1005 >> > #define PCI_DEVICE_ID_VIRTIO_9P 0x1009 >> > +#define PCI_DEVICE_ID_QUMRANET_IVSHMEM 0x1110 >> > >> > #define FMT_PCIBUS PRIx64 > Why _QUMRANET I wonder? > Because it's under 0x1af4 (PCI_VENDOR_ID_REDHAT_QUMRANET), not 0x1b36 (PCI_VENDOR_ID_REDHAT). Paolo
On Wed, Dec 12, 2012 at 03:29:31PM +0100, Paolo Bonzini wrote: > Il 12/12/2012 15:22, Michael S. Tsirkin ha scritto: > >> > @@ -29,4 +29,6 @@ maintained as part of the virtio specification. > >> > 1af4:1100 Used as PCI Subsystem ID for existing hardware devices emulated > >> > by qemu. > >> > > >> > +1af4:1110 ivshmem device (shared memory, docs/specs/ivshmem_device_spec.txt) > >> > + > >> > All other device IDs are reserved. > >> > diff --git a/hw/ivshmem.c b/hw/ivshmem.c > >> > index f6dbb21..8adeb2c 100644 > >> > --- a/hw/ivshmem.c > >> > +++ b/hw/ivshmem.c > >> > @@ -800,7 +800,7 @@ static void ivshmem_class_init(ObjectClass *klass, void *data) > >> > k->init = pci_ivshmem_init; > >> > k->exit = pci_ivshmem_uninit; > >> > k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET; > >> > - k->device_id = 0x1110; > >> > + k->device_id = PCI_DEVICE_ID_QUMRANET_IVSHMEM; > >> > k->class_id = PCI_CLASS_MEMORY_RAM; > >> > dc->reset = ivshmem_reset; > >> > dc->props = ivshmem_properties; > >> > diff --git a/hw/pci.h b/hw/pci.h > >> > index 0719521..3704d5f 100644 > >> > --- a/hw/pci.h > >> > +++ b/hw/pci.h > >> > @@ -78,6 +78,7 @@ > >> > #define PCI_DEVICE_ID_VIRTIO_SCSI 0x1004 > >> > #define PCI_DEVICE_ID_VIRTIO_RNG 0x1005 > >> > #define PCI_DEVICE_ID_VIRTIO_9P 0x1009 > >> > +#define PCI_DEVICE_ID_QUMRANET_IVSHMEM 0x1110 > >> > > >> > #define FMT_PCIBUS PRIx64 > > Why _QUMRANET I wonder? > > > > Because it's under 0x1af4 (PCI_VENDOR_ID_REDHAT_QUMRANET), not 0x1b36 > (PCI_VENDOR_ID_REDHAT). > > Paolo Yes but so are all virtio devices are they not?
Il 12/12/2012 15:48, Michael S. Tsirkin ha scritto: >>>>> > >> > diff --git a/hw/pci.h b/hw/pci.h >>>>> > >> > index 0719521..3704d5f 100644 >>>>> > >> > --- a/hw/pci.h >>>>> > >> > +++ b/hw/pci.h >>>>> > >> > @@ -78,6 +78,7 @@ >>>>> > >> > #define PCI_DEVICE_ID_VIRTIO_SCSI 0x1004 >>>>> > >> > #define PCI_DEVICE_ID_VIRTIO_RNG 0x1005 >>>>> > >> > #define PCI_DEVICE_ID_VIRTIO_9P 0x1009 >>>>> > >> > +#define PCI_DEVICE_ID_QUMRANET_IVSHMEM 0x1110 >>>>> > >> > >>>>> > >> > #define FMT_PCIBUS PRIx64 >>> > > Why _QUMRANET I wonder? >>> > > >> > >> > Because it's under 0x1af4 (PCI_VENDOR_ID_REDHAT_QUMRANET), not 0x1b36 >> > (PCI_VENDOR_ID_REDHAT). > > Yes but so are all virtio devices are they not? All device IDs start with PCI_DEVICE_ID_<vendor>. virtio devices are the exception for some historical reason I don't know. PCI_DEVICE_ID_VIRTIO_IVSHMEM is wrong, and PCI_DEVICE_ID_IVSHMEM is unspecific. PCI_DEVICE_ID_QEMU_IVSHMEM is wrong because the QEMU vendor id is 0x1234. So either it stays Qumranet, or the 0x1234 vendor id is renamed to PCI_VENDOR_ID_BOCHS and 0x1af4 becomes PCI_VENDOR_ID_REDHAT_QEMU. Either is fine for me, but I wanted to minimize the churn. Paolo
On Wed, Dec 12, 2012 at 03:51:16PM +0100, Paolo Bonzini wrote: > Il 12/12/2012 15:48, Michael S. Tsirkin ha scritto: > >>>>> > >> > diff --git a/hw/pci.h b/hw/pci.h > >>>>> > >> > index 0719521..3704d5f 100644 > >>>>> > >> > --- a/hw/pci.h > >>>>> > >> > +++ b/hw/pci.h > >>>>> > >> > @@ -78,6 +78,7 @@ > >>>>> > >> > #define PCI_DEVICE_ID_VIRTIO_SCSI 0x1004 > >>>>> > >> > #define PCI_DEVICE_ID_VIRTIO_RNG 0x1005 > >>>>> > >> > #define PCI_DEVICE_ID_VIRTIO_9P 0x1009 > >>>>> > >> > +#define PCI_DEVICE_ID_QUMRANET_IVSHMEM 0x1110 > >>>>> > >> > > >>>>> > >> > #define FMT_PCIBUS PRIx64 > >>> > > Why _QUMRANET I wonder? > >>> > > > >> > > >> > Because it's under 0x1af4 (PCI_VENDOR_ID_REDHAT_QUMRANET), not 0x1b36 > >> > (PCI_VENDOR_ID_REDHAT). > > > > Yes but so are all virtio devices are they not? > > All device IDs start with PCI_DEVICE_ID_<vendor>. virtio devices are > the exception for some historical reason I don't know. > PCI_DEVICE_ID_VIRTIO_IVSHMEM is wrong, and PCI_DEVICE_ID_IVSHMEM is > unspecific. PCI_DEVICE_ID_QEMU_IVSHMEM is wrong because the QEMU vendor > id is 0x1234. > > So either it stays Qumranet, or the 0x1234 vendor id is renamed to > PCI_VENDOR_ID_BOCHS and 0x1af4 becomes PCI_VENDOR_ID_REDHAT_QEMU. > Either is fine for me, but I wanted to minimize the churn. > > Paolo Thinking about it some more, I'd rather move this stuff out of pci.h For virtio, there's actually linux/virtio_ids.h which defines VIRTIO_ID_NET etc. We should add that, and have virtio just use it. Doesn't ivshmem have a linux driver? When it lands upstream we'll be able to add it to pci_ids.h meanwhile keeping a number in device .c seems fine as well. I will do the virtio change I think - meanwhile could you limit this patch to just the .txt change please?
Il 12/12/2012 16:21, Michael S. Tsirkin ha scritto: > Thinking about it some more, I'd rather move this stuff > out of pci.h > > For virtio, there's actually linux/virtio_ids.h > which defines VIRTIO_ID_NET etc. We should add that, > and have virtio just use it. > > Doesn't ivshmem have a linux driver? No, you use it with mmap of sysfs files. The doorbell feature requires uio. > When it lands upstream we'll be able to add it to pci_ids.h > meanwhile keeping a number in device .c seems fine as well. I think the point was to avoid conflicts, but as long as docs/specs/ is kept in sync that's fine. > I will do the virtio change I think - meanwhile > could you limit this patch to just the .txt change please? Ok, will resend. Paolo
On Wed, Dec 12, 2012 at 04:24:26PM +0100, Paolo Bonzini wrote: > Il 12/12/2012 16:21, Michael S. Tsirkin ha scritto: > > Thinking about it some more, I'd rather move this stuff > > out of pci.h > > > > For virtio, there's actually linux/virtio_ids.h > > which defines VIRTIO_ID_NET etc. We should add that, > > and have virtio just use it. > > > > Doesn't ivshmem have a linux driver? > > No, you use it with mmap of sysfs files. The doorbell feature requires uio. > > > When it lands upstream we'll be able to add it to pci_ids.h > > meanwhile keeping a number in device .c seems fine as well. > > I think the point was to avoid conflicts, but as long as docs/specs/ is > kept in sync that's fine. Right. We are not the final authority on device/vendor IDs anyway - I am more worried about using the correct IDs for emulated devices. > > I will do the virtio change I think - meanwhile > > could you limit this patch to just the .txt change please? > > Ok, will resend. > > Paolo
diff --git a/docs/specs/pci-ids.txt b/docs/specs/pci-ids.txt index 28dcf90..6b5cf42 100644 --- a/docs/specs/pci-ids.txt +++ b/docs/specs/pci-ids.txt @@ -29,4 +29,6 @@ maintained as part of the virtio specification. 1af4:1100 Used as PCI Subsystem ID for existing hardware devices emulated by qemu. +1af4:1110 ivshmem device (shared memory, docs/specs/ivshmem_device_spec.txt) + All other device IDs are reserved. diff --git a/hw/ivshmem.c b/hw/ivshmem.c index f6dbb21..8adeb2c 100644 --- a/hw/ivshmem.c +++ b/hw/ivshmem.c @@ -800,7 +800,7 @@ static void ivshmem_class_init(ObjectClass *klass, void *data) k->init = pci_ivshmem_init; k->exit = pci_ivshmem_uninit; k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET; - k->device_id = 0x1110; + k->device_id = PCI_DEVICE_ID_QUMRANET_IVSHMEM; k->class_id = PCI_CLASS_MEMORY_RAM; dc->reset = ivshmem_reset; dc->props = ivshmem_properties; diff --git a/hw/pci.h b/hw/pci.h index 0719521..3704d5f 100644 --- a/hw/pci.h +++ b/hw/pci.h @@ -78,6 +78,7 @@ #define PCI_DEVICE_ID_VIRTIO_SCSI 0x1004 #define PCI_DEVICE_ID_VIRTIO_RNG 0x1005 #define PCI_DEVICE_ID_VIRTIO_9P 0x1009 +#define PCI_DEVICE_ID_QUMRANET_IVSHMEM 0x1110 #define FMT_PCIBUS PRIx64
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- docs/specs/pci-ids.txt | 2 ++ hw/ivshmem.c | 2 +- hw/pci.h | 1 + 3 file modificati, 4 inserzioni(+). 1 rimozione(-)