[Qemu-devel] [PATCH] [STABLE] Clean some PCI defines

[Qemu-devel] [PATCH] [STABLE] Clean some PCI defines

[Stefan Weil]

[-- Attachment #1: Type: text/plain, Size: 793 bytes --]

Hello,

this patch adds some more defines from linux/pci_regs.h to
hw/pci.h. There is now no longer a need to define them in
eepro100.c, so they were removed there.

Some defines from linux/pci_regs.h had similar, but not
the same defines in hw/pci.h (PCI_REVISION_ID / PCI_REVISION,
PCI_SUBSYSTEM_VENDOR_ID / PCI_SUBVENDOR_ID,
PCI_SUBSYSTEM_ID / PCI_SUBDEVICE_ID).

I suggest to use the "standard" from linux/pci_regs.h and
replace the "old" Qemu ones. To facilitate the migration,
my patch does not remove the old defines but marks them
as obsolete. After a migration to the "standard" defines,
pci.h could use linux/pci_regs.h which is far more complete.

The patch is needed for an updated maintainer version of
hw/eepro100.c which I'd like to see in Qemu stable.

Regards

Stefan Weil




[-- Attachment #2: pci.patch --]
[-- Type: text/x-diff, Size: 2720 bytes --]

* Remove declarations already declared in header file from eepro100.c
* Add missing declarations from pci_regs.h to pci.h
* Mark "non-standard" declarations in pci.h as obsolete

Signed-off-by: Stefan Weil <weil@mail.berlios.de>

Index: trunk/hw/eepro100.c
===================================================================
--- trunk.orig/hw/eepro100.c	2009-03-22 11:19:10.000000000 +0100
+++ trunk/hw/eepro100.c	2009-03-22 11:58:06.000000000 +0100
@@ -47,12 +47,6 @@
 
 /* Common declarations for all PCI devices. */
 
-#define PCI_DEVICE_ID           0x02    /* 16 bits */
-#define PCI_COMMAND             0x04    /* 16 bits */
-#define PCI_STATUS              0x06    /* 16 bits */
-
-#define PCI_REVISION_ID         0x08    /* 8 bits  */
-
 #define PCI_CONFIG_8(offset, value) \
     (pci_conf[offset] = (value))
 #define PCI_CONFIG_16(offset, value) \
Index: trunk/hw/pci.h
===================================================================
--- trunk.orig/hw/pci.h	2009-03-22 11:22:26.000000000 +0100
+++ trunk/hw/pci.h	2009-03-22 11:57:22.000000000 +0100
@@ -55,7 +55,7 @@
 #define PCI_DEVICE_ID_VMWARE_SCSI        0x0730
 #define PCI_DEVICE_ID_VMWARE_IDE         0x1729
 
-#define PCI_VENDOR_ID_INTEL              0x8086
+/* Intel (0x8086) */
 #define PCI_DEVICE_ID_INTEL_82551IT      0x1209
 
 /* Red Hat / Qumranet (for QEMU) -- see pci-ids.txt */
@@ -92,20 +92,27 @@
 
 #define PCI_DEVICES_MAX 64
 
+/* Declarations from linux/pci_regs.h */
 #define PCI_VENDOR_ID		0x00	/* 16 bits */
 #define PCI_DEVICE_ID		0x02	/* 16 bits */
 #define PCI_COMMAND		0x04	/* 16 bits */
 #define  PCI_COMMAND_IO		0x1	/* Enable response in I/O space */
 #define  PCI_COMMAND_MEMORY	0x2	/* Enable response in Memory space */
-#define PCI_REVISION            0x08
+#define PCI_STATUS              0x06    /* 16 bits */
+#define PCI_REVISION_ID         0x08    /* 8 bits  */
 #define PCI_CLASS_DEVICE        0x0a    /* Device class */
-#define PCI_SUBVENDOR_ID	0x2c	/* 16 bits */
-#define PCI_SUBDEVICE_ID	0x2e	/* 16 bits */
+#define PCI_HEADER_TYPE         0x0e    /* 8 bits */
+#define PCI_SUBSYSTEM_VENDOR_ID 0x2c    /* 16 bits */
+#define PCI_SUBSYSTEM_ID        0x2e    /* 16 bits */
 #define PCI_INTERRUPT_LINE	0x3c	/* 8 bits */
 #define PCI_INTERRUPT_PIN	0x3d	/* 8 bits */
 #define PCI_MIN_GNT		0x3e	/* 8 bits */
 #define PCI_MAX_LAT		0x3f	/* 8 bits */
 
+#define PCI_REVISION            0x08    /* obsolete, use PCI_REVISION_ID */
+#define PCI_SUBVENDOR_ID        0x2c    /* obsolete, use PCI_SUBSYSTEM_VENDOR_ID */
+#define PCI_SUBDEVICE_ID        0x2e    /* obsolete, use PCI_SUBSYSTEM_ID */
+
 /* Bits in the PCI Status Register (PCI 2.3 spec) */
 #define PCI_STATUS_RESERVED1	0x007
 #define PCI_STATUS_INT_STATUS	0x008

Re: [Qemu-devel] [PATCH] [STABLE] Clean some PCI defines

[Andreas Färber]

Hello,

Am 22.03.2009 um 12:46 schrieb Stefan Weil:

> Some defines from linux/pci_regs.h had similar, but not
> the same defines in hw/pci.h (PCI_REVISION_ID / PCI_REVISION,
> PCI_SUBSYSTEM_VENDOR_ID / PCI_SUBVENDOR_ID,
> PCI_SUBSYSTEM_ID / PCI_SUBDEVICE_ID).
>
> I suggest to use the "standard" from linux/pci_regs.h and
> replace the "old" Qemu ones. [...]
> After a migration to the "standard" defines,
> pci.h could use linux/pci_regs.h which is far more complete.

Please keep in mind that linux/pci_regs.h is most likely not suitable  
for cross-platform use.

You could of course do

#ifdef __linux__
#include <linux/pci_regs.h>
#else
#define PCI_... 0x1234
...
#endif

but then you would still need to sync the two sections.

Andreas

Re: [Qemu-devel] [PATCH] [STABLE] Clean some PCI defines

[Stefan Weil]

Andreas Färber schrieb:
> Hello,
>
> Am 22.03.2009 um 12:46 schrieb Stefan Weil:
>
>> Some defines from linux/pci_regs.h had similar, but not
>> the same defines in hw/pci.h (PCI_REVISION_ID / PCI_REVISION,
>> PCI_SUBSYSTEM_VENDOR_ID / PCI_SUBVENDOR_ID,
>> PCI_SUBSYSTEM_ID / PCI_SUBDEVICE_ID).
>>
>> I suggest to use the "standard" from linux/pci_regs.h and
>> replace the "old" Qemu ones. [...]
>> After a migration to the "standard" defines,
>> pci.h could use linux/pci_regs.h which is far more complete.
>
> Please keep in mind that linux/pci_regs.h is most likely not suitable
> for cross-platform use.
>
> You could of course do
>
> #ifdef __linux__
> #include <linux/pci_regs.h>
> #else
> #define PCI_... 0x1234
> ...
> #endif
>
> but then you would still need to sync the two sections.
>
> Andreas
>
>
>


There is nothing platform-specific in linux/pci_regs.h, only
general PCI stuff. No need for conditional compilation.

Stefan

Re: [Qemu-devel] [PATCH] [STABLE] Clean some PCI defines

[Andreas Färber]

Am 22.03.2009 um 15:20 schrieb Stefan Weil:

> Andreas Färber schrieb:
>> Am 22.03.2009 um 12:46 schrieb Stefan Weil:
>>
>>> Some defines from linux/pci_regs.h had similar, but not
>>> the same defines in hw/pci.h (PCI_REVISION_ID / PCI_REVISION,
>>> PCI_SUBSYSTEM_VENDOR_ID / PCI_SUBVENDOR_ID,
>>> PCI_SUBSYSTEM_ID / PCI_SUBDEVICE_ID).
>>>
>>> I suggest to use the "standard" from linux/pci_regs.h and
>>> replace the "old" Qemu ones. [...]
>>> After a migration to the "standard" defines,
>>> pci.h could use linux/pci_regs.h which is far more complete.
>>
>> Please keep in mind that linux/pci_regs.h is most likely not suitable
>> for cross-platform use.
>>
>> You could of course do
>>
>> #ifdef __linux__
>> #include <linux/pci_regs.h>
>> #else
>> #define PCI_... 0x1234
>> ...
>> #endif
>>
>> but then you would still need to sync the two sections.
>
> There is nothing platform-specific in linux/pci_regs.h, only
> general PCI stuff. No need for conditional compilation.

The *linux*/pci_regs.h file doesn't seem present on Mac OS X, Solaris,  
Haiku, not to mention Windows. So either you need to #include it  
conditionally where available, as outlined above, or ship a copy from  
Linux/NetBSD with QEMU, as suggested by Warner, and #include that  
local file only.

Andreas

Re: [Qemu-devel] [PATCH] [STABLE] Clean some PCI defines

[Stefan Weil]

Andreas Färber schrieb:
>
> Am 22.03.2009 um 15:20 schrieb Stefan Weil:
>
>> Andreas Färber schrieb:
>>> Am 22.03.2009 um 12:46 schrieb Stefan Weil:
>>>
>>>> Some defines from linux/pci_regs.h had similar, but not
>>>> the same defines in hw/pci.h (PCI_REVISION_ID / PCI_REVISION,
>>>> PCI_SUBSYSTEM_VENDOR_ID / PCI_SUBVENDOR_ID,
>>>> PCI_SUBSYSTEM_ID / PCI_SUBDEVICE_ID).
>>>>
>>>> I suggest to use the "standard" from linux/pci_regs.h and
>>>> replace the "old" Qemu ones. [...]
>>>> After a migration to the "standard" defines,
>>>> pci.h could use linux/pci_regs.h which is far more complete.
>>>
>>> Please keep in mind that linux/pci_regs.h is most likely not suitable
>>> for cross-platform use.
>>>
>>> You could of course do
>>>
>>> #ifdef __linux__
>>> #include <linux/pci_regs.h>
>>> #else
>>> #define PCI_... 0x1234
>>> ...
>>> #endif
>>>
>>> but then you would still need to sync the two sections.
>>
>> There is nothing platform-specific in linux/pci_regs.h, only
>> general PCI stuff. No need for conditional compilation.
>
> The *linux*/pci_regs.h file doesn't seem present on Mac OS X, Solaris,
> Haiku, not to mention Windows. So either you need to #include it
> conditionally where available, as outlined above, or ship a copy from
> Linux/NetBSD with QEMU, as suggested by Warner, and #include that
> local file only.
>
> Andreas
>

Oh, sorry, this was a misunderstanding.

Of course I meant to include a copy (linux/pci_regs.h -> hw/pci_regs.h),
in Qemu, like it was done with linux/pci_ids.h.

Stefan

Re: [Qemu-devel] [PATCH] [STABLE] Clean some PCI defines

[M. Warner Losh]

In message: <49C62504.8070208@mail.berlios.de>
            Stefan Weil <weil@mail.berlios.de> writes:
: Hello,
: 
: this patch adds some more defines from linux/pci_regs.h to
: hw/pci.h. There is now no longer a need to define them in
: eepro100.c, so they were removed there.
: 
: Some defines from linux/pci_regs.h had similar, but not
: the same defines in hw/pci.h (PCI_REVISION_ID / PCI_REVISION,
: PCI_SUBSYSTEM_VENDOR_ID / PCI_SUBVENDOR_ID,
: PCI_SUBSYSTEM_ID / PCI_SUBDEVICE_ID).
: 
: I suggest to use the "standard" from linux/pci_regs.h and
: replace the "old" Qemu ones. To facilitate the migration,
: my patch does not remove the old defines but marks them
: as obsolete. After a migration to the "standard" defines,
: pci.h could use linux/pci_regs.h which is far more complete.
: 
: The patch is needed for an updated maintainer version of
: hw/eepro100.c which I'd like to see in Qemu stable.

Maybe the right answer here is to snag the pcidevs file from NetBSD
and use it.  After all, it doesn't gratuitously introduce GPL
contamination everywhere?

Warner

Re: [Qemu-devel] [PATCH] [STABLE] Clean some PCI defines

[Anthony Liguori]

Stefan Weil wrote:
> Hello,
>
> this patch adds some more defines from linux/pci_regs.h to
> hw/pci.h. There is now no longer a need to define them in
> eepro100.c, so they were removed there.
>
> Some defines from linux/pci_regs.h had similar, but not
> the same defines in hw/pci.h (PCI_REVISION_ID / PCI_REVISION,
> PCI_SUBSYSTEM_VENDOR_ID / PCI_SUBVENDOR_ID,
> PCI_SUBSYSTEM_ID / PCI_SUBDEVICE_ID).
>
> I suggest to use the "standard" from linux/pci_regs.h and
> replace the "old" Qemu ones. To facilitate the migration,
> my patch does not remove the old defines but marks them
> as obsolete. After a migration to the "standard" defines,
> pci.h could use linux/pci_regs.h which is far more complete.
>
> The patch is needed for an updated maintainer version of
> hw/eepro100.c which I'd like to see in Qemu stable.
>   

Applied to trunk.  Since this isn't a bug fix, I don't think it's 
appropriate for stable.  If you have another bug fix in your queue that 
depends on this patch, I'd rather that a simpler change be used.

Regards,

Anthony Liguori

> Regards
>
> Stefan Weil
>
>
>
>