[Qemu-devel] [PATCH] allow setting static devfn values for pci devices from the command line

[Qemu-devel] [PATCH] allow setting static devfn values for pci devices from the command line

[Izik Eidus]

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

hi,
this patch make it possible to define from the command line a static 
devfn value for each pci
device.
it was wrote for addressing a problem that right now qemu devices get 
their devfn in random way
(almost random)
the problem with this is that with adding and removing devices some 
devfn values can be changed
for each device.
this make (at least) windows unable to understand what happned to your 
device and mark it
in yellow color. (and will want you to reinstall it)

in this patch i simply use the device name that was registred with the 
pci device registration
function.
in case you have few devices from the same type (same name), it will 
simply increase each by one
so in this case all you have to do is give long enough  offset for the 
devfns from each other.

thanks

[-- Attachment #2: 0001-qemu-allow-setting-static-devfn-for-pci-devices-fro.patch --]
[-- Type: text/x-patch, Size: 7959 bytes --]

>From 3b26c9a10ddc01bf68c8d8e488559e3a082fa36b Mon Sep 17 00:00:00 2001
From: Izik Eidus <izike@qumranet.com>
Date: Mon, 19 Nov 2007 17:41:49 +0200
Subject: [PATCH] qemu: allow setting static devfn for pci devices from the command line

Signed-off-by: Izik Eidus <izike@qumranet.com>
---
 hw/pci.c |   78 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
 vl.c     |   66 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 139 insertions(+), 5 deletions(-)

diff --git a/hw/pci.c b/hw/pci.c
index 0b5e7bf..d12184c 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -52,6 +52,9 @@ target_phys_addr_t pci_mem_base;
 static int pci_irq_index;
 static PCIBus *first_bus;
 
+char static_devfns[PCI_DEVICES_MAX][128];
+int static_devfns_index = 0;
+
 PCIBus *pci_register_bus(pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
                          qemu_irq *pic, int devfn_min, int nirq)
 {
@@ -98,6 +101,67 @@ int pci_device_load(PCIDevice *s, QEMUFile *f)
     return 0;
 }
 
+int pci_read_static_devfn(PCIBus *bus, const char *name)
+{
+    int i;
+    int y;
+    int count;
+
+    for (i = 0; i < static_devfns_index; i++) {
+        count = 0;
+        for (y = 0; y < sizeof(static_devfns[0]) && name[y]; y++) {
+            if (tolower(static_devfns[i][y]) != tolower(name[y]))
+                continue;
+            count++;
+        }
+        /*if count is y we found devfn value for this device name */
+        if (count == y && y < sizeof(static_devfns[0]) &&
+            static_devfns[i][y] == '=') {
+            int devfn;
+            char *value_buffer;
+
+            static_devfns[i][sizeof(static_devfns[0]) - 1] = '\0';
+            value_buffer = strstr(static_devfns[i], "=");
+            if (!value_buffer)
+                return -1;
+            value_buffer++;
+            devfn = atoi(value_buffer);
+            if (devfn < 0)
+                return -1;
+            /*
+             * check if the devfn is already registered in case this pci device
+             * is registering now not for the first time.
+             * in this case we increase the value of the devfn by one untill
+             * we find a free devfn
+             * note: you should provide devices we big enougth spaces beteween
+             * them if you want to register the same devices more than once
+             */
+            for (; devfn < 256; devfn++)
+                if (!bus->devices[devfn])
+                    return devfn;
+            return -1;
+        }
+    }
+    return -1;
+}
+
+int pci_is_static_devfn(int devfn)
+{
+    int i;
+    char *value_buffer;
+
+    for (i = 0; i < static_devfns_index; i++) {
+        static_devfns[i][sizeof(static_devfns[0]) - 1] = '\0';
+        value_buffer = strstr(static_devfns[i], "=");
+        value_buffer++;
+        if (!value_buffer)
+            continue;
+        if (devfn == atoi(value_buffer))
+            return 1;
+    }
+    return 0;
+}
+
 /* -1 for devfn means auto assign */
 PCIDevice *pci_register_device(PCIBus *bus, const char *name,
                                int instance_size, int devfn,
@@ -110,13 +174,17 @@ PCIDevice *pci_register_device(PCIBus *bus, const char *name,
         return NULL;
 
     if (devfn < 0) {
-        for(devfn = bus->devfn_min ; devfn < 256; devfn += 8) {
-            if (!bus->devices[devfn])
-                goto found;
+        devfn = pci_read_static_devfn(bus, name);
+        if (devfn < 0) {
+            for(devfn = bus->devfn_min ; devfn < 256; devfn += 8) {
+                if (!bus->devices[devfn] && !pci_is_static_devfn(devfn))
+                    goto found;
+            }
+            return NULL;
+        found: ;
         }
-        return NULL;
-    found: ;
     }
+
     pci_dev = qemu_mallocz(instance_size);
     if (!pci_dev)
         return NULL;
diff --git a/vl.c b/vl.c
index 8b2b5bd..7ae7275 100644
--- a/vl.c
+++ b/vl.c
@@ -29,6 +29,7 @@
 #include "hw/fdc.h"
 #include "hw/audiodev.h"
 #include "hw/isa.h"
+#include "hw/pci.h"
 #include "net.h"
 #include "console.h"
 #include "sysemu.h"
@@ -871,6 +872,64 @@ static void rtc_stop_timer(struct qemu_alarm_timer *t);
 
 #endif /* _WIN32 */
 
+static void set_static_devfn(const char *optarg)
+{
+    int i;
+
+    if (*optarg == '?') {
+        printf("usage: -static-devfn dev_name=static_devfn_num\n"
+               "list of device names:\n"
+               "rtl8139 - realteak8139 network device\n"
+               "ne2000 - ne2000 network device\n"
+               "openpic\n"
+               "pcnet\n"
+               "i440fx\n"
+               "piix\n"
+               "vga - vga video card device\n"
+               "qemuware_svga - vmware svga video card device\n"
+               "piix3\n"
+               "piix4\n"
+               "macio\n"
+               "pci_brgd - pci bridge\n"
+               "cmd646_ide - ide device\n"
+               "advanced_pci_bus - acpi device\n"
+               "piix3_ide - ide device\n"
+               "prep_host_bridge_-motorola_raven\n"
+               "uni-north_main\n"
+               "uni-north_bridge\n"
+               "uni-north_agp\n"
+               "uni-north_internal\n"
+               "ochi_usb\n"
+               "usb_uchi\n"
+               "piix4_ide - ide device\n"
+               "lsi53c895a_scsi_hba\n"
+               "realview_eb_pci_controller\n"
+               "versatile/pb_pci_controller\n"
+               "cirrus_vga - cirrus vga video card device\n"
+               "es1370\n"
+               "grackle_host_bridge\n"
+               "gt64120_pci_bus\n"
+               "std-vga - standart vga video card device\n");
+               exit(0);
+        } else {
+            extern int static_devfns_index;
+            extern char static_devfns[PCI_DEVICES_MAX][64];
+            
+            if (static_devfns_index >= PCI_DEVICES_MAX) {
+                fprintf(stderr, "Too many static-devfns registred\n");
+                exit(1);
+            }
+            pstrcpy(static_devfns[static_devfns_index],
+                    sizeof(static_devfns[static_devfns_index]),
+                    optarg);
+            for (i = 0; static_devfns[static_devfns_index][i] &&
+                 i < sizeof(static_devfns[0]); i++)
+                if (static_devfns[static_devfns_index][i] == '_')
+                    static_devfns[static_devfns_index][i] = ' ';
+            static_devfns_index++;
+        }
+}
+
 static struct qemu_alarm_timer alarm_timers[] = {
 #ifndef _WIN32
 #ifdef __linux__
@@ -7076,6 +7135,8 @@ static void help(int exitcode)
 #endif
            "-usb            enable the USB driver (will be the default soon)\n"
            "-usbdevice name add the host or guest USB device 'name'\n"
+           "-static-devfn   set a static devfn for device\n"
+           "                use -static-devfn ? to get help\n"
 #if defined(TARGET_PPC) || defined(TARGET_SPARC)
            "-g WxH[xDEPTH]  Set the initial graphical resolution and depth\n"
 #endif
@@ -7243,6 +7304,7 @@ enum {
     QEMU_OPTION_win2k_hack,
     QEMU_OPTION_usb,
     QEMU_OPTION_usbdevice,
+    QEMU_OPTION_static_devfn,
     QEMU_OPTION_smp,
     QEMU_OPTION_vnc,
     QEMU_OPTION_no_acpi,
@@ -7339,6 +7401,7 @@ const QEMUOption qemu_options[] = {
     { "pidfile", HAS_ARG, QEMU_OPTION_pidfile },
     { "win2k-hack", 0, QEMU_OPTION_win2k_hack },
     { "usbdevice", HAS_ARG, QEMU_OPTION_usbdevice },
+    { "static-devfn", HAS_ARG, QEMU_OPTION_static_devfn },
     { "smp", HAS_ARG, QEMU_OPTION_smp },
     { "vnc", HAS_ARG, QEMU_OPTION_vnc },
 
@@ -8111,6 +8174,9 @@ int main(int argc, char **argv)
                         optarg);
                 usb_devices_index++;
                 break;
+            case QEMU_OPTION_static_devfn:
+                set_static_devfn(optarg);
+                break;
             case QEMU_OPTION_smp:
                 smp_cpus = atoi(optarg);
                 if (smp_cpus < 1 || smp_cpus > MAX_CPUS) {
-- 
1.5.3.4

Re: [Qemu-devel] [PATCH] allow setting static devfn values for pci devices from the command line

[Izik Eidus]

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

Izik Eidus wrote:
> hi,
> this patch make it possible to define from the command line a static 
> devfn value for each pci
> device.
> it was wrote for addressing a problem that right now qemu devices get 
> their devfn in random way
> (almost random)
> the problem with this is that with adding and removing devices some 
> devfn values can be changed
> for each device.
> this make (at least) windows unable to understand what happned to your 
> device and mark it
> in yellow color. (and will want you to reinstall it)
>
> in this patch i simply use the device name that was registred with the 
> pci device registration
> function.
> in case you have few devices from the same type (same name), it will 
> simply increase each by one
> so in this case all you have to do is give long enough  offset for the 
> devfns from each other.
>
> thanks
ok here is a fix to two issues that i noticed:
1.in one place i declared static_devfns[MAX_PCI_DEVICS][64] and in other 
place: static_devfns[MAX_PCI_DEVICS][128]

2.in one place i did a calculation on a pointer line before i checked if 
it is vaild pointer...

anyway here it is again and fixed.

[-- Attachment #2: 0003-qemu-allow-setting-static-devfn-for-pci-devices-fro.patch --]
[-- Type: text/x-patch, Size: 7958 bytes --]

>From 6570e2859a43a1bc5178dc2130b1e91672fec6ae Mon Sep 17 00:00:00 2001
From: Izik Eidus <izike@qumranet.com>
Date: Mon, 19 Nov 2007 18:49:48 +0200
Subject: [PATCH] qemu: allow setting static devfn for pci devices from the command line

Signed-off-by: Izik Eidus <izike@qumranet.com>
---
 hw/pci.c |   78 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
 vl.c     |   66 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 139 insertions(+), 5 deletions(-)

diff --git a/hw/pci.c b/hw/pci.c
index 0b5e7bf..3914704 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -52,6 +52,9 @@ target_phys_addr_t pci_mem_base;
 static int pci_irq_index;
 static PCIBus *first_bus;
 
+char static_devfns[PCI_DEVICES_MAX][64];
+int static_devfns_index = 0;
+
 PCIBus *pci_register_bus(pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
                          qemu_irq *pic, int devfn_min, int nirq)
 {
@@ -98,6 +101,67 @@ int pci_device_load(PCIDevice *s, QEMUFile *f)
     return 0;
 }
 
+int pci_read_static_devfn(PCIBus *bus, const char *name)
+{
+    int i;
+    int y;
+    int count;
+
+    for (i = 0; i < static_devfns_index; i++) {
+        count = 0;
+        for (y = 0; y < sizeof(static_devfns[0]) && name[y]; y++) {
+            if (tolower(static_devfns[i][y]) != tolower(name[y]))
+                continue;
+            count++;
+        }
+        /*if count is y we found devfn value for this device name */
+        if (count == y && y < sizeof(static_devfns[0]) &&
+            static_devfns[i][y] == '=') {
+            int devfn;
+            char *value_buffer;
+
+            static_devfns[i][sizeof(static_devfns[0]) - 1] = '\0';
+            value_buffer = strstr(static_devfns[i], "=");
+            if (!value_buffer)
+                return -1;
+            value_buffer++;
+            devfn = atoi(value_buffer);
+            if (devfn < 0)
+                return -1;
+            /*
+             * check if the devfn is already registered in case this pci device
+             * is registering now not for the first time.
+             * in this case we increase the value of the devfn by one untill
+             * we find a free devfn
+             * note: you should provide devices we big enougth spaces beteween
+             * them if you want to register the same devices more than once
+             */
+            for (; devfn < 256; devfn++)
+                if (!bus->devices[devfn])
+                    return devfn;
+            return -1;
+        }
+    }
+    return -1;
+}
+
+int pci_is_static_devfn(int devfn)
+{
+    int i;
+    char *value_buffer;
+
+    for (i = 0; i < static_devfns_index; i++) {
+        static_devfns[i][sizeof(static_devfns[0]) - 1] = '\0';
+        value_buffer = strstr(static_devfns[i], "=");
+        if (!value_buffer)
+            continue;
+        value_buffer++;
+        if (devfn == atoi(value_buffer))
+            return 1;
+    }
+    return 0;
+}
+
 /* -1 for devfn means auto assign */
 PCIDevice *pci_register_device(PCIBus *bus, const char *name,
                                int instance_size, int devfn,
@@ -110,13 +174,17 @@ PCIDevice *pci_register_device(PCIBus *bus, const char *name,
         return NULL;
 
     if (devfn < 0) {
-        for(devfn = bus->devfn_min ; devfn < 256; devfn += 8) {
-            if (!bus->devices[devfn])
-                goto found;
+        devfn = pci_read_static_devfn(bus, name);
+        if (devfn < 0) {
+            for(devfn = bus->devfn_min ; devfn < 256; devfn += 8) {
+                if (!bus->devices[devfn] && !pci_is_static_devfn(devfn))
+                    goto found;
+            }
+            return NULL;
+        found: ;
         }
-        return NULL;
-    found: ;
     }
+
     pci_dev = qemu_mallocz(instance_size);
     if (!pci_dev)
         return NULL;
diff --git a/vl.c b/vl.c
index 8b2b5bd..7ae7275 100644
--- a/vl.c
+++ b/vl.c
@@ -29,6 +29,7 @@
 #include "hw/fdc.h"
 #include "hw/audiodev.h"
 #include "hw/isa.h"
+#include "hw/pci.h"
 #include "net.h"
 #include "console.h"
 #include "sysemu.h"
@@ -871,6 +872,64 @@ static void rtc_stop_timer(struct qemu_alarm_timer *t);
 
 #endif /* _WIN32 */
 
+static void set_static_devfn(const char *optarg)
+{
+    int i;
+
+    if (*optarg == '?') {
+        printf("usage: -static-devfn dev_name=static_devfn_num\n"
+               "list of device names:\n"
+               "rtl8139 - realteak8139 network device\n"
+               "ne2000 - ne2000 network device\n"
+               "openpic\n"
+               "pcnet\n"
+               "i440fx\n"
+               "piix\n"
+               "vga - vga video card device\n"
+               "qemuware_svga - vmware svga video card device\n"
+               "piix3\n"
+               "piix4\n"
+               "macio\n"
+               "pci_brgd - pci bridge\n"
+               "cmd646_ide - ide device\n"
+               "advanced_pci_bus - acpi device\n"
+               "piix3_ide - ide device\n"
+               "prep_host_bridge_-motorola_raven\n"
+               "uni-north_main\n"
+               "uni-north_bridge\n"
+               "uni-north_agp\n"
+               "uni-north_internal\n"
+               "ochi_usb\n"
+               "usb_uchi\n"
+               "piix4_ide - ide device\n"
+               "lsi53c895a_scsi_hba\n"
+               "realview_eb_pci_controller\n"
+               "versatile/pb_pci_controller\n"
+               "cirrus_vga - cirrus vga video card device\n"
+               "es1370\n"
+               "grackle_host_bridge\n"
+               "gt64120_pci_bus\n"
+               "std-vga - standart vga video card device\n");
+               exit(0);
+        } else {
+            extern int static_devfns_index;
+            extern char static_devfns[PCI_DEVICES_MAX][64];
+            
+            if (static_devfns_index >= PCI_DEVICES_MAX) {
+                fprintf(stderr, "Too many static-devfns registred\n");
+                exit(1);
+            }
+            pstrcpy(static_devfns[static_devfns_index],
+                    sizeof(static_devfns[static_devfns_index]),
+                    optarg);
+            for (i = 0; static_devfns[static_devfns_index][i] &&
+                 i < sizeof(static_devfns[0]); i++)
+                if (static_devfns[static_devfns_index][i] == '_')
+                    static_devfns[static_devfns_index][i] = ' ';
+            static_devfns_index++;
+        }
+}
+
 static struct qemu_alarm_timer alarm_timers[] = {
 #ifndef _WIN32
 #ifdef __linux__
@@ -7076,6 +7135,8 @@ static void help(int exitcode)
 #endif
            "-usb            enable the USB driver (will be the default soon)\n"
            "-usbdevice name add the host or guest USB device 'name'\n"
+           "-static-devfn   set a static devfn for device\n"
+           "                use -static-devfn ? to get help\n"
 #if defined(TARGET_PPC) || defined(TARGET_SPARC)
            "-g WxH[xDEPTH]  Set the initial graphical resolution and depth\n"
 #endif
@@ -7243,6 +7304,7 @@ enum {
     QEMU_OPTION_win2k_hack,
     QEMU_OPTION_usb,
     QEMU_OPTION_usbdevice,
+    QEMU_OPTION_static_devfn,
     QEMU_OPTION_smp,
     QEMU_OPTION_vnc,
     QEMU_OPTION_no_acpi,
@@ -7339,6 +7401,7 @@ const QEMUOption qemu_options[] = {
     { "pidfile", HAS_ARG, QEMU_OPTION_pidfile },
     { "win2k-hack", 0, QEMU_OPTION_win2k_hack },
     { "usbdevice", HAS_ARG, QEMU_OPTION_usbdevice },
+    { "static-devfn", HAS_ARG, QEMU_OPTION_static_devfn },
     { "smp", HAS_ARG, QEMU_OPTION_smp },
     { "vnc", HAS_ARG, QEMU_OPTION_vnc },
 
@@ -8111,6 +8174,9 @@ int main(int argc, char **argv)
                         optarg);
                 usb_devices_index++;
                 break;
+            case QEMU_OPTION_static_devfn:
+                set_static_devfn(optarg);
+                break;
             case QEMU_OPTION_smp:
                 smp_cpus = atoi(optarg);
                 if (smp_cpus < 1 || smp_cpus > MAX_CPUS) {
-- 
1.5.3.4

Re: [Qemu-devel] [PATCH] allow setting static devfn values for pci devices from the command line

[Jocelyn Mayer]

On Mon, 2007-11-19 at 18:53 +0200, Izik Eidus wrote:
> Izik Eidus wrote:
> > hi,
> > this patch make it possible to define from the command line a static 
> > devfn value for each pci
> > device.
> > it was wrote for addressing a problem that right now qemu devices get 
> > their devfn in random way
> > (almost random)
> > the problem with this is that with adding and removing devices some 
> > devfn values can be changed
> > for each device.
> > this make (at least) windows unable to understand what happned to your 
> > device and mark it
> > in yellow color. (and will want you to reinstall it)
> >
> > in this patch i simply use the device name that was registred with the 
> > pci device registration
> > function.
> > in case you have few devices from the same type (same name), it will 
> > simply increase each by one
> > so in this case all you have to do is give long enough  offset for the 
> > devfns from each other.
> >
> > thanks
> ok here is a fix to two issues that i noticed:
> 1.in one place i declared static_devfns[MAX_PCI_DEVICS][64] and in other 
> place: static_devfns[MAX_PCI_DEVICS][128]
> 
> 2.in one place i did a calculation on a pointer line before i checked if 
> it is vaild pointer...
> 
> anyway here it is again and fixed.

It seems that you cannot impose the PCI device numbers mapping, which is
likely to be architecture dependant. What you could however change is
the PCI bus & slot the device is inserted into, the same way you can
choose the PCI slot you put a PCI card into on a real machine. The
architecture could then determine what is the corresponding PCI device
number, given this PCI bus & slot numbers.
For other PCI devices, like PCI bridges or other internal devices, the
PCI slots / devfn are fixed by the architecture. You cannot change them
in any way, then it seems pointless to have an option that would change
the behavior for any of those devices.
The last problem I see is that it seems very ugly to hardcode the device
names the way you did in vl.c. This is even false in most cases, as most
of those devices are only available for a few machines and are not
tunable at all (for example macio or uninorth are only available in some
Apple Mac machines, not even all). 

-- 
Jocelyn Mayer <l_indien@magic.fr>

Re: [Qemu-devel] [PATCH] allow setting static devfn values for pci devices from the command line

[Izik Eidus]

Jocelyn Mayer wrote:
> On Mon, 2007-11-19 at 18:53 +0200, Izik Eidus wrote:
>   
>> Izik Eidus wrote:
>>     
>>> hi,
>>> this patch make it possible to define from the command line a static 
>>> devfn value for each pci
>>> device.
>>> it was wrote for addressing a problem that right now qemu devices get 
>>> their devfn in random way
>>> (almost random)
>>> the problem with this is that with adding and removing devices some 
>>> devfn values can be changed
>>> for each device.
>>> this make (at least) windows unable to understand what happned to your 
>>> device and mark it
>>> in yellow color. (and will want you to reinstall it)
>>>
>>> in this patch i simply use the device name that was registred with the 
>>> pci device registration
>>> function.
>>> in case you have few devices from the same type (same name), it will 
>>> simply increase each by one
>>> so in this case all you have to do is give long enough  offset for the 
>>> devfns from each other.
>>>
>>> thanks
>>>       
>> ok here is a fix to two issues that i noticed:
>> 1.in one place i declared static_devfns[MAX_PCI_DEVICS][64] and in other 
>> place: static_devfns[MAX_PCI_DEVICS][128]
>>
>> 2.in one place i did a calculation on a pointer line before i checked if 
>> it is vaild pointer...
>>
>> anyway here it is again and fixed.
>>     
>
> It seems that you cannot impose the PCI device numbers mapping, which is
> likely to be architecture dependant. What you could however change is
> the PCI bus & slot the device is inserted into, the same way you can
> choose the PCI slot you put a PCI card into on a real machine. The
> architecture could then determine what is the corresponding PCI device
> number, given this PCI bus & slot numbers.
i will check this.
> For other PCI devices, like PCI bridges or other internal devices, the
> PCI slots / devfn are fixed by the architecture. You cannot change them
> in any way, then it seems pointless to have an option that would change
> the behavior for any of those devices.
> The last problem I see is that it seems very ugly to hardcode the device
> names the way you did in vl.c. This is even false in most cases, as most
> of those devices are only available for a few machines and are not
> tunable at all (for example macio or uninorth are only available in some
> Apple Mac machines, not even all). 
>   
so how will i inform the user with the devices name?