[Qemu-devel] Re: [PATCH] qemu PPC Linux 2.6 IDE

["J. Mayer" <l_indien@magic.fr>]

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

On Wed, 2005-02-23 at 23:28, Thayne Harbaugh wrote:
> I have qemu PPC booting Linux 2.6.  Here's a patch for IDE.
> 
> IDE detection of macio has changed significantly.  2.4
> drivers/ide/ppc/pmac.c:pmac_ide_probe() simply looked for the IDE/ATA
> controllers in OF.  2.6 pmac.c:pmace_ide_probe() registers macio device
> filters for IDE/ATA and relies on macio device enumeration to connect to
> the IDE driver.  Where things are broken is that
> drivers/macintosh/macio_asic.c:macio_pci_probe() fails to match the
> macio device due to the bus:devfn not being set as a "reg" property in
> OHW.

OK, your patch is as false as what I've done for many reasons (see
below)

> 
> There are two questions about this patch:
> 
> * What is the complete information expected in the "reg" property of a
> PCI device?

"reg" is supposed to contain information about the configuration space,
I/O regions and memory regions for the PCI device.

> * Where should the current macio "reg" information really be stored, if
> at all?

Current MacIO "reg" informations are to be stored in "reg" property, but
there are false, as you've seen. Those "reg" properties where taken from
a real mac...
I did not notice that I should have changed the preg.hi cell to match
the Qemu PCI device informations, but the real fix is to generate the
whole information...

I made a quick fix, following OF bindings for PCI specifications.
Note that I already had a fix for OF_property_set with no value, less
intrusive than yours.
I also did add a dprintf (as debug printf) function to make early debug
easier and there's also a needed fix in CHRP boot file loader.

Please consider this patch against 0.4-pre1d, it should fix the macIO
problem the right way.
There's still a lack of informations about configuration space in reg
properties for PCI devices, but this should not hurt.

-- 
J. Mayer <l_indien@magic.fr>
Never organized

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

Index: src/bios.h
===================================================================
--- src/bios.h	(revision 105)
+++ src/bios.h	(working copy)
@@ -401,7 +401,8 @@ void *OF_register_pci_device (void *pare
                               uint8_t devfn, uint8_t rev, uint32_t ccode,
                               uint16_t min_grant, uint16_t max_latency);
 void OF_finalize_pci_host (void *dev, int first_bus, int nb_busses);
-void OF_finalize_pci_device (void *dev, uint32_t *regions, uint32_t *sizes);
+void OF_finalize_pci_device (void *dev, uint8_t bus, uint8_t devfn,
+                             uint32_t *regions, uint32_t *sizes);
 void OF_finalize_pci_macio (void *dev, uint32_t base_address, uint32_t size,
                             void *private_data);
 int OF_register_bus (const unsigned char *name, uint32_t address,

Index: src/of.c
===================================================================
--- src/of.c	(revision 99)
+++ src/of.c	(working copy)
@@ -886,7 +886,7 @@ static OF_prop_t *OF_property_set (OF_en
     if (prop != NULL) {
         OF_DPRINTF("change property [%s]\n", name);
         tmp = malloc(len);
-        if (tmp == NULL) {
+        if (tmp == NULL && len != 0) {
             ERROR("%s cannot set property '%s'\n", __func__, name);
             return NULL;
         }
@@ -2009,7 +2009,7 @@ static void *OF_pci_device_new (OF_env_t
 {
     OF_node_t *node;
 
-    printf("register '%s' '%s' '%s' '%s' 0x%08x in '%s' 0x%08x\n",
+    dprintf("register '%s' '%s' '%s' '%s' 0x%08x in '%s' 0x%08x\n",
            dev->name, dev->type, dev->compat, dev->model, address,
            parent->prop_name->value, *(uint32_t *)parent->prop_address->value);
     node = OF_node_new(OF_env, parent, dev->name, address);
@@ -2033,7 +2033,7 @@ static void *OF_pci_device_new (OF_env_t
         OF_prop_int_new(OF_env, node, "#interrupt-cells", dev->acells);
     if (dev->icells != 0)
         OF_prop_int_new(OF_env, node, "#size-cells", dev->acells);
-    printf("Done %p %p\n", parent, node);
+    dprintf("Done %p %p\n", parent, node);
     
     return node;
 }
@@ -2150,35 +2156,112 @@ void OF_finalize_pci_host (void *dev, in
     OF_property_new(OF_env, dev, "bus-range", regs, sizeof(OF_regprop_t));
 }
 
-void OF_finalize_pci_device (void *dev, uint32_t *regions, uint32_t *sizes)
+void OF_finalize_pci_device (void *dev, uint8_t bus, uint8_t devfn,
+                             uint32_t *regions, uint32_t *sizes)
 {
     OF_env_t *OF_env;
-    pci_reg_prop_t pregs[6];
-    int i, j;
+    pci_reg_prop_t pregs[6], rregs[6];
+    uint32_t mask;
+    int i, j, k;
 
     OF_env = OF_env_main;
     if (regions[0] != 0x00000000)
         OF_prop_int_set(OF_env, dev, "address", regions[0] & ~0x0000000F);
-    for (i = 0, j = 0; i < 6; i++) {
+    for (i = 0, j = 0, k = 0; i < 6; i++) {
         if (regions[i] != 0x00000000 && sizes[i] != 0x00000000) {
+            /* Generate "reg" property */
             if (regions[i] & 1) {
-                pregs[j].addr.hi = 0x01000000;
+                /* IO space */
+                rregs[j].addr.hi = 0x01000000;
+                mask = 0x00000001;
+            } else if (regions[i] & 4) {
+                /* 64 bits address space */
+                rregs[j].addr.hi = 0x83000000;
+                mask = 0x0000000F;
+#if 0
+            } else if ((regions[i] & 0xF) == 0x00) { /* ? */
+                /* Configuration space */
+                rregs[j].addr.hi = 0x00000000;
+                mask = 0x0000000F;
+#endif
             } else {
-                pregs[j].addr.hi = 0x02000000;
+                /* 32 bits address space */
+                rregs[j].addr.hi = 0x82000000;
+                mask = 0x0000000F;
+            }
+            /* Set bus number */
+            rregs[j].addr.hi |= bus << 16;
+            /* Set device/function */
+            rregs[j].addr.hi |= devfn << 8;
+            /* Set register */
+#if 1
+            rregs[j].addr.hi |= 0x10 + (i * sizeof(uint32_t));
+#endif
+            /* Set address */
+            rregs[j].addr.mid = 0x00000000;
+            rregs[j].addr.lo = regions[i] & ~mask;
+            /* Set size */
+            rregs[j].size_hi = 0x00000000;
+            rregs[j].size_lo = sizes[i];
+#if 0
+            if ((rregs[j].addr.hi & 0x03000000) != 0x00000000)
+#endif
+            {
+                /* No assigned address for configuration space */
+                pregs[k].addr.hi = rregs[j].addr.hi;
+                pregs[k].addr.mid = rregs[j].addr.mid;
+                pregs[k].addr.lo = rregs[j].addr.lo;
+                pregs[k].size_hi = rregs[j].size_hi;
+                pregs[k].size_lo = rregs[j].size_lo;
+                k++;
             }
-            pregs[j].addr.mid = 0x00000000;
-            pregs[j].addr.lo = regions[i] & ~0x0000000F;
-            pregs[j].size_hi = 0x00000000;
-            pregs[j].size_lo = sizes[i];
             j++;
         }
     }
     if (j > 0) {
+        OF_property_new(OF_env, dev, "reg",
+                        rregs, j * sizeof(pci_reg_prop_t));
+    } else {
+        OF_property_new(OF_env, dev, "reg", NULL, 0);
+    }
+    if (k > 0) {
         OF_property_new(OF_env, dev, "assigned-addresses",
-                        pregs, j * sizeof(pci_reg_prop_t));
+                        pregs, k * sizeof(pci_reg_prop_t));
     } else {
         OF_property_new(OF_env, dev, "assigned-addresses", NULL, 0);
     }
+#if 1
+    {
+        OF_prop_t *prop_name = ((OF_node_t *)dev)->prop_name;
+
+        if (j > 0) {
+            dprintf("PCI device '%s' %d %d %d reg properties:\n",
+                    prop_name->value, bus, devfn >> 3, devfn & 7);
+            for (i = 0; i < j; i++) {
+                dprintf("  addr: %08x %08x %08x size: %08x %08x\n",
+                        rregs[i].addr.hi, rregs[i].addr.mid, rregs[i].addr.lo,
+                        rregs[i].size_hi, rregs[i].size_lo);
+            }
+        } else {
+            dprintf("PCI device '%s' %d %d %d has no reg properties:\n",
+                    prop_name->value, bus, devfn >> 3, devfn & 7);
+        }
+        if (k > 0) {
+            dprintf("PCI device '%s' %d %d %d "
+                    "assigned addresses properties:\n",
+                    prop_name->value, bus, devfn >> 3, devfn & 7);
+            for (i = 0; i < j; i++) {
+                dprintf("  addr: %08x %08x %08x size: %08x %08x\n",
+                        pregs[i].addr.hi, pregs[i].addr.mid, pregs[i].addr.lo,
+                        pregs[i].size_hi, pregs[i].size_lo);
+            }
+        } else {
+            dprintf("PCI device '%s' %d %d %d has no "
+                    "assigned addresses properties:\n",
+                    prop_name->value, bus, devfn >> 3, devfn & 7);
+        }
+    }
+#endif
 }
 
 __attribute__ (( section (".OpenFirmware") ))
@@ -2340,7 +2423,7 @@ void OF_finalize_pci_macio (void *dev, u
     /* Mac-IO is mandatory for OSX to boot */
     mio = dev;
     mio->private_data = private_data;
-#if 1
+#if 0 // Done by generic PCI device registration routines
     pregs[0].addr.hi = 0x00013800;
     pregs[0].addr.mid = 0x00000000;
     pregs[0].addr.lo = 0x00000000;
@@ -3811,9 +3894,9 @@ static void OF_setprop (OF_env_t *OF_env
         return;
     }
     value = malloc(len);
-    if (value == NULL) {
+    if (value == NULL && len != 0) {
         pushd(OF_env, -1);
-        ERROR("%s: Cannot alloc property\n", __func__);
+        ERROR("%s: Cannot alloc property '%s' (%d)\n", __func__, name, len);
         return;
     }
     for (i = 0; i < len; i++)

Index: src/pci.c
===================================================================
--- src/pci.c	(revision 100)
+++ src/pci.c	(working copy)
@@ -2003,7 +2003,7 @@ static pci_u_t *pci_check_device (pci_ho
     }
  update_device:
     pci_update_device(bridge, newd, min_grant, max_latency, irq_line);
-    OF_finalize_pci_device(newd->common.OF_private,
+    OF_finalize_pci_device(newd->common.OF_private, bus, devfn,
                            newd->regions, newd->sizes);
     /* Call special inits if needed */
     if (dev->config_cb != NULL)
Index: src/char.c
===================================================================
--- src/char.c	(revision 101)
+++ src/char.c	(working copy)
@@ -300,6 +300,20 @@ static cops_t pc_serial_ops = {
     .close = &pc_serial_close,
 };
 
+/* XXX: debug stuff only ! (TOFIX with a generic debug console) */
+int serial_write (const void *buffer, int len)
+{
+    const char *p;
+
+    for (p = buffer; len > 0; len--) {
+        if (!(inb(0x3F8 + PC_SERIAL_LSR_OFFSET) & 0x20))
+            usleep(100);
+        outb(0x3F8, *p++);
+    }
+
+    return 0;
+}
+
 int pc_serial_register (uint16_t base)
 {
     pc_serial_t *serial;
Index: src/libc/include/stdio.h
===================================================================
--- src/libc/include/stdio.h	(revision 90)
+++ src/libc/include/stdio.h	(working copy)
@@ -35,9 +35,11 @@
 #define EOF ((int)-1)
 
 int printf (const char *format, ...);
+int dprintf (const char *format, ...);
 int sprintf (char *str, const char *format, ...);
 int snprintf (char *str, size_t size, const char *format, ...);
 int vprintf (const char *format, va_list ap);
+int vdprintf (const char *format, va_list ap);
 int vsprintf (char *str, const char *format, va_list ap);
 int vsnprintf (char *str, size_t size, const char *format, va_list ap);
 

Index: src/libc/src/format.c
===================================================================
--- src/libc/src/format.c	(revision 90)
+++ src/libc/src/format.c	(working copy)
@@ -41,6 +41,8 @@
 
 #define unused __attribute__ (( unused ))
 int console_write (const void *buffer, int len);
+int serial_write (const void *buffer, int len);
+#define debug_write serial_write
 
 /* Low level output fonctions */
 typedef size_t (*outf_t)(void *private, const unsigned char *buf, size_t len);
@@ -57,6 +59,16 @@ size_t outfd (void *private, const unsig
     return write(*fd, buf, len);
 }
 
+size_t outf_dbg (void *private, const unsigned char *buf, size_t len)
+{
+    int *fd = private;
+
+    if (*fd == 1 || *fd == 2)
+        return debug_write(buf, len);
+
+    return write(*fd, buf, len);
+}
+
 /* output to buffer */
 size_t outbuf (void *private, const unsigned char *buf, size_t len)
 {
@@ -362,6 +374,7 @@ int _vprintf(outf_t outf, void *private,
 }
 #else /* defined (__USE__vprintf__) */
 size_t outfd (void *private, const unsigned char *buf, size_t len);
+size_t outf_dbg (void *private, const unsigned char *buf, size_t len);
 size_t outbuf (void *private, const unsigned char *buf, size_t len);
 int _vprintf(outf_t outf, void *private, size_t maxlen,
              const unsigned char *format, va_list ap);
@@ -382,6 +395,21 @@ int printf (const char *format, ...)
 }
 #endif /* defined (__USE_printf__) */
 
+#if defined (__USE_dprintf__)
+int dprintf (const char *format, ...)
+{
+    va_list ap;
+    int fd = 1;
+    int ret;
+
+    va_start(ap, format);
+    ret = _vprintf(&outf_dbg, &fd, -1, format, ap);
+    va_end(ap);
+
+    return ret;
+}
+#endif /* defined (__USE_dprintf__) */
+
 #if defined (__USE_sprintf__)
 int sprintf (char *str, const char *format, ...)
 {
@@ -419,6 +447,15 @@ int vprintf (const char *format, va_list
 }
 #endif /* defined (__USE_vprintf__) */
 
+#if defined (__USE_vdprintf__)
+int vdprintf (const char *format, va_list ap)
+{
+    int fd = 1;
+
+    return _vprintf(&outf_dbg, &fd, -1, format, ap);
+}
+#endif /* defined (__USE_vdprintf__) */
+
 #if defined (__USE_vsprintf__)
 int vsprintf (char *str, const char *format, va_list ap)
 {

Index: src/libexec/chrp.c
===================================================================
--- src/libexec/chrp.c	(revision 90)
+++ src/libexec/chrp.c	(working copy)
@@ -168,7 +168,7 @@ int exec_load_chrp (inode_t *file, void 
                     if (*(char *)tag->data == 0x0d) {
                         pos++;
                     }
-                    DPRINTF("Boot file: '%s'\n", pos);
+                    DPRINTF("Description: '%s'\n", pos);
                     break;
                 case CHRP_TAG_BOOT_SCRIPT:
                     /* Here is the interresting part... */
@@ -255,7 +255,8 @@ int exec_load_chrp (inode_t *file, void 
                             /* Find file name end */
                         redo:
                             for (endc = pos;
-                                 *endc != ' ' && *endc != '"' && *endc != '\n';
+                                 *endc != ' ' && *endc != '"' &&
+                                 *endc != '\n' && *endc != '\r';
                                  endc++)
                                 continue;
                             if (memcmp(pos, "ofwboot", 7) == 0) {
@@ -274,8 +275,7 @@ int exec_load_chrp (inode_t *file, void 
                             *endc = '/';
                         }
                         /* check if it's a path or just a file */
-                        if (strchr(pos, '/') == NULL ||
-                            (pos[0] == '/' && pos[1] == '/')) {
+                        if (pos[0] == '/' && pos[1] == '/') {
                             unsigned char *bootdir;
                             bootdir = fs_get_boot_dirname(part_fs(part));
                             if (bootdir == NULL) {

Index: Makefile
===================================================================
--- Makefile	(revision 104)
+++ Makefile	(working copy)
@@ -72,6 +72,7 @@ main.out_OBJS:= main.o bootinfos.o bloc.
 main.out_OBJS:= $(addprefix $(OBJDIR)/, $(main.out_OBJS))
 # Pseudo-libc objects
 FORMAT_FUNCS:= _vprintf printf sprintf snprintf vprintf vsprintf vsnprintf
+FORMAT_FUNCS+= dprintf vdprintf
 MEM_FUNCS:= memcpy memccpy mempcpy memmove memcmove mempmove
 MEM_FUNCS+= memset memcmp memchr rawmemchr memrchr memmem
 STR_FUNCS:= strcpy strdup strndup stpcpy stpncpy strcat strncat