qemu-ppc
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: mac99 SMP


From: Jd Lyons
Subject: Re: mac99 SMP
Date: Sun, 30 Mar 2025 11:56:18 -0400

I found the code to make input work agian, so reason ChatGPT removed this:

for (i = 0; i < 4; i++) {

        qdev_connect_gpio_out(uninorth_pci_dev, i,

                              qdev_get_gpio_in(pic_dev, 0x1b + i));

    }


So now I have 4 CPUs fully working and input with Mac OS X Tiger.


Balaton, do you think we need to adjust it for GPIO 15 and 16 as well?


Full context:


 /* --- Connect GPIO reset IRQs for secondary CPUs --- */

    s = SYS_BUS_DEVICE(object_resolve_path_component(macio, "gpio"));

    if (machine->smp.cpus > 1) {

        /* CPU1 reset: use GPIO 4 */

        kick = qemu_allocate_irq(cpu_kick, cpus[1], 1);

        sysbus_connect_irq(s, 4, kick);

    }

    if (machine->smp.cpus > 2) {

        /* CPU2 reset: use GPIO 5 */

        kick = qemu_allocate_irq(cpu_kick, cpus[2], 2);

        sysbus_connect_irq(s, 15, kick);

    }

    if (machine->smp.cpus > 3) {

        /* CPU3 reset: use GPIO 6 */

        kick = qemu_allocate_irq(cpu_kick, cpus[3], 3);

        sysbus_connect_irq(s, 16, kick);

    }

    for (i = 0; i < 4; i++) {

        qdev_connect_gpio_out(uninorth_pci_dev, i,

                              qdev_get_gpio_in(pic_dev, 0x1b + i));

    }



On Mar 30, 2025, at 10:33 AM, Jd Lyons <lyons_dj@yahoo.com> wrote:

Yes it’s possible that rerouting GPIOs is stepping on existing GPIO lines, but I don’t have the fist clue how to implement the TB sync and can’t find any useful info.

KEYLARGO_GPIO_EXTINT_CNT  may help here, I’ll see if I can find anything on it.

EDIT: I extended the array so I could use GPIO 15 and 16, but it did not solve the no input problem. Just not sure what that issue is or how to nail it down, I need to revert back to your original patches and add mine in one at a time to find what patch broke INPUT.

Does this look to be the correct calculations for Openbios?

/* Add CPU-specific properties based on CPU index */
if (i == 0) {  /* CPU0 */
PUSH(0x71);
fword("encode-int");
push_str("soft-reset");
fword("property");


PUSH(0x1);
fword("encode-int");
push_str("gpio-mask");
fword("property");

PUSH(0x1);
fword("encode-int");
push_str("gpio-value");
fword("property");

PUSH(0x73);
fword("encode-int");
push_str("timebase-enable");
fword("property");
} else if (i == 1) {  /* CPU1 */
PUSH(0x72);
fword("encode-int");
push_str("soft-reset");
fword("property");

PUSH(0x1);
fword("encode-int");
push_str("gpio-mask");
fword("property");

PUSH(0x1);
fword("encode-int");
push_str("gpio-value");
fword("property");

PUSH(0x73);
fword("encode-int");
push_str("timebase-enable");
fword("property");

PUSH(0x74);
fword("encode-int");
push_str("cpu-interrupt-enable");
fword("property");
} else if (i == 2) {  /* CPU2 */
PUSH(0x67);
fword("encode-int");
push_str("soft-reset");
fword("property");

PUSH(0x69);
fword("encode-int");
push_str("timebase-enable");
fword("property");
} else if (i == 3) {  /* CPU3 */
PUSH(0x68);
fword("encode-int");
push_str("soft-reset");
fword("property");

PUSH(0x69);
fword("encode-int");
push_str("timebase-enable");
fword("property");
}

Based on the definitions in gpio.h, the hardware‐reset GPIO numbers are defined as follows:

  • CPU0 reset: KL_GPIO_RESET_CPU0 = KEYLARGO_GPIO_EXTINT_0 + 0x03

  • CPU1 reset: KL_GPIO_RESET_CPU1 = KEYLARGO_GPIO_EXTINT_0 + 0x04

  • CPU2 reset: KL_GPIO_RESET_CPU2 = KEYLARGO_GPIO_EXTINT_0 + 0x0f

  • CPU3 reset: KL_GPIO_RESET_CPU3 = KEYLARGO_GPIO_EXTINT_0 + 0x10

Since KEYLARGO_GPIO_EXTINT_0 is 0x58, the actual physical GPIO numbers are:

  • CPU0: 0x58 + 0x03 = 0x5B

  • CPU1: 0x58 + 0x04 = 0x5C

  • CPU2: 0x58 + 0x0f = 0x67

  • CPU3: 0x58 + 0x10 = 0x68

In OpenBIOS, the soft‑reset property is encoded by taking a “base” value and adding an offset derived from the physical GPIO number. Empirically the pattern used in the current init.c is as follows:

  • For CPU0 and CPU1 (typically in the “0x70 group”):
    soft‑reset = 0x70 + (physical GPIO number – 2)

  • For CPU2 and CPU3 (typically in the “0x60 group”):
    soft‑reset = 0x60 + (physical GPIO number – 8)

Let’s apply that:

  1. CPU0:
    Physical GPIO = 0x03
    Soft‑reset = 0x70 + (0x03 – 0x02) = 0x70 + 0x01 = 0x71

  2. CPU1:
    Physical GPIO = 0x04
    Soft‑reset = 0x70 + (0x04 – 0x02) = 0x70 + 0x02 = 0x72

  3. CPU2:
    Physical GPIO = 0x0f
    Soft‑reset = 0x60 + (0x0f – 0x08) = 0x60 + 0x07 = 0x67

  4. CPU3:
    Physical GPIO = 0x10
    Soft‑reset = 0x60 + (0x10 – 0x08) = 0x60 + 0x08 = 0x68

Thus, the “hard facts” (if you follow this common encoding scheme) are that the OpenBIOS soft‑reset values should be 0x71, 0x72, 0x67, and 0x68 for CPUs 0–3, respectively.

Keep in mind that this scheme is derived from the existing OpenBIOS code and the observed values; if the hardware documentation from an actual system confirms the same encoding, then these are the “correct” values. Otherwise, you may need to adjust based on your target hardware. 

On Mar 30, 2025, at 9:17 AM, BALATON Zoltan <balaton@eik.bme.hu> wrote:

On Sun, 30 Mar 2025, BALATON Zoltan wrote:
On Sun, 30 Mar 2025, Jd Lyons wrote:
I did my best to implement your suggestions, tho GPIO 15 and 16 seem to be outside the rage of the GPIO array so I rerouted them to GPIO 5 and 6.

According to linux/arch/powerpc/include/asm/keylargo.h
#define KEYLARGO_GPIO_EXTINT_CNT        18
so you probably have to increase the array in QEMU. Using other random GPIO lines instead may not work if the guest expects that to be something else and use them which may result in randomly resetting CPUs when not expected. So better put these where they are on real machine (or what Linux thinks it should be as that maybe was taken from some real hardware).

Sadly it didn’t make any difference what so ever that I can tell, the

I told before that adding more CPUs is not likely to fix anything, it's more likely to break things. What might fix something is implementing TB sync so maybe to solve that instead.

I meant "try to solve that".

Also some notes to the patch:

diff --git a/hw/misc/macio/gpio.c b/hw/misc/macio/gpio.c
index e87bfca1f5..410257ad61 100644
--- a/hw/misc/macio/gpio.c
+++ b/hw/misc/macio/gpio.c
@@ -33,6 +33,11 @@
#include "qemu/log.h"
#include "qemu/module.h"
#include "trace.h"
+#define KEYLARGO_GPIO_EXTINT_0        0x58
+#define KL_GPIO_RESET_CPU1            (KEYLARGO_GPIO_EXTINT_0 + 0x04)
+#define KL_GPIO_RESET_CPU2            (KEYLARGO_GPIO_EXTINT_0 + 0x0f)
+#define KL_GPIO_RESET_CPU3            (KEYLARGO_GPIO_EXTINT_0 + 0x10)
+

You've added these to the header below so should not need to repeat here. Keep either this or the one in the header not both.

enum MacioGPIORegisterBits {
   OUT_DATA   = 1,
@@ -56,18 +61,13 @@ void macio_set_gpio(MacIOGPIOState *s, uint32_t gpio, bool state)
       new_reg |= IN_DATA;
   }
-    if (new_reg == s->gpio_regs[gpio]) {
+    /* For GPIOs 4, 5, and 6 (reset lines), always update so we can assert/deassert */
+    if ((gpio != 4) && (gpio != 5) && (gpio != 6) && (new_reg == s->gpio_regs[gpio])) {
       return;
   }
-    s->gpio_regs[gpio] = new_reg;
-    /*
-     * Note that we probably need to get access to the MPIC config to
-     * decode polarity since qemu always use "raise" regardless.
-     *
-     * For now, we hard wire known GPIOs
-     */
+    s->gpio_regs[gpio] = new_reg;

Why are these changes needed in this function?

@@ -184,7 +203,6 @@ static void ppc_core99_init(MachineState *machine)
       bios_size = load_elf(filename, NULL, NULL, NULL, NULL,
                            NULL, NULL, NULL,
                            ELFDATA2MSB, PPC_ELF_MACHINE, 0, 0);
-
       if (bios_size <= 0) {
           /* or load binary ROM image */
           bios_size = load_image_targphys(filename, PROM_BASE, PROM_SIZE);
@@ -198,7 +216,6 @@ static void ppc_core99_init(MachineState *machine)

   if (machine->kernel_filename) {
       int bswap_needed = 0;
-
#ifdef BSWAP_NEEDED
       bswap_needed = 1;
#endif
@@ -239,9 +256,6 @@ static void ppc_core99_init(MachineState *machine)
       ppc_boot_device = 'm';
   } else {
       ppc_boot_device = '\0';
-        /* We consider that NewWorld PowerMac never have any floppy drive
-         * For now, OHW cannot boot from the network.
-         */
       for (i = 0; machine->boot_config.order[i] != '\0'; i++) {
           if (machine->boot_config.order[i] >= 'c' &&
               machine->boot_config.order[i] <= 'f') {
@@ -256,22 +270,18 @@ static void ppc_core99_init(MachineState *machine)
   }

   openpic_irqs = g_new0(IrqLines, machine->smp.cpus);
-    dev = DEVICE(cpu);
   for (i = 0; i < machine->smp.cpus; i++) {
-        /* Mac99 IRQ connection between OpenPIC outputs pins
-         * and PowerPC input pins
-         */
+        dev = DEVICE(cpus[i]);
+        /* Mac99 IRQ connection between OpenPIC outputs pins and PowerPC input pins */
       switch (PPC_INPUT(env)) {
       case PPC_FLAGS_INPUT_6xx:
           openpic_irqs[i].irq[OPENPIC_OUTPUT_INT] =
               qdev_get_gpio_in(dev, PPC6xx_INPUT_INT);
           openpic_irqs[i].irq[OPENPIC_OUTPUT_CINT] =
-                 qdev_get_gpio_in(dev, PPC6xx_INPUT_INT);
+                qdev_get_gpio_in(dev, PPC6xx_INPUT_INT);
           openpic_irqs[i].irq[OPENPIC_OUTPUT_MCK] =
               qdev_get_gpio_in(dev, PPC6xx_INPUT_MCP);
-            /* Not connected ? */
           openpic_irqs[i].irq[OPENPIC_OUTPUT_DEBUG] = NULL;
-            /* Check this */
           openpic_irqs[i].irq[OPENPIC_OUTPUT_RESET] =
               qdev_get_gpio_in(dev, PPC6xx_INPUT_HRESET);
           break;
@@ -283,13 +293,11 @@ static void ppc_core99_init(MachineState *machine)
               qdev_get_gpio_in(dev, PPC970_INPUT_INT);
           openpic_irqs[i].irq[OPENPIC_OUTPUT_MCK] =
               qdev_get_gpio_in(dev, PPC970_INPUT_MCP);
-            /* Not connected ? */
           openpic_irqs[i].irq[OPENPIC_OUTPUT_DEBUG] = NULL;
-            /* Check this */
           openpic_irqs[i].irq[OPENPIC_OUTPUT_RESET] =
               qdev_get_gpio_in(dev, PPC970_INPUT_HRESET);
           break;
-#endif /* defined(TARGET_PPC64) */
+#endif
       default:
           error_report("Bus model not supported on mac99 machine");
           exit(1);
@@ -305,47 +313,37 @@ static void ppc_core99_init(MachineState *machine)
   if (PPC_INPUT(env) == PPC_FLAGS_INPUT_970) {
       machine_arch = ARCH_MAC99_U3;
       /* 970 gets a U3 bus */
-        /* Uninorth AGP bus */
       uninorth_pci_dev = qdev_new(TYPE_U3_AGP_HOST_BRIDGE);
       s = SYS_BUS_DEVICE(uninorth_pci_dev);
       sysbus_realize_and_unref(s, &error_fatal);
       sysbus_mmio_map(s, 0, 0xf0800000);
       sysbus_mmio_map(s, 1, 0xf0c00000);
-        /* PCI hole */
       memory_region_add_subregion(get_system_memory(), 0x80000000,
                                   sysbus_mmio_get_region(s, 2));
-        /* Register 8 MB of ISA IO space */
       memory_region_add_subregion(get_system_memory(), 0xf2000000,
                                   sysbus_mmio_get_region(s, 3));
   } else {
       machine_arch = ARCH_MAC99;
-        /* Use values found on a real PowerMac */
-        /* Uninorth AGP bus */
       uninorth_agp_dev = qdev_new(TYPE_UNI_NORTH_AGP_HOST_BRIDGE);
       s = SYS_BUS_DEVICE(uninorth_agp_dev);
       sysbus_realize_and_unref(s, &error_fatal);
       sysbus_mmio_map(s, 0, 0xf0800000);
       sysbus_mmio_map(s, 1, 0xf0c00000);
-        /* Uninorth internal bus */
-        uninorth_internal_dev = qdev_new(
-                                TYPE_UNI_NORTH_INTERNAL_PCI_HOST_BRIDGE);
+        uninorth_internal_dev = qdev_new(TYPE_UNI_NORTH_INTERNAL_PCI_HOST_BRIDGE);
       s = SYS_BUS_DEVICE(uninorth_internal_dev);
       sysbus_realize_and_unref(s, &error_fatal);
       sysbus_mmio_map(s, 0, 0xf4800000);
       sysbus_mmio_map(s, 1, 0xf4c00000);
-        /* Uninorth main bus - this must be last to make it the default */
       uninorth_pci_dev = qdev_new(TYPE_UNI_NORTH_PCI_HOST_BRIDGE);
       qdev_prop_set_uint32(uninorth_pci_dev, "ofw-addr", 0xf2000000);
       s = SYS_BUS_DEVICE(uninorth_pci_dev);
       sysbus_realize_and_unref(s, &error_fatal);
       sysbus_mmio_map(s, 0, 0xf2800000);
       sysbus_mmio_map(s, 1, 0xf2c00000);
-        /* PCI hole */
       memory_region_add_subregion(get_system_memory(), 0x80000000,
                                   sysbus_mmio_get_region(s, 2));
-        /* Register 8 MB of ISA IO space */
       memory_region_add_subregion(get_system_memory(), 0xf2000000,
                                   sysbus_mmio_get_region(s, 3));
   }
@@ -355,7 +353,6 @@ static void ppc_core99_init(MachineState *machine)
   has_adb = (core99_machine->via_config == CORE99_VIA_CONFIG_CUDA ||
              core99_machine->via_config == CORE99_VIA_CONFIG_PMU_ADB);
-    /* init basic PC hardware */
   pci_bus = PCI_HOST_BRIDGE(uninorth_pci_dev)->bus;

   /* MacIO */

For easier review do not change anything else unrelated or if you did do not include those in the patch you send to the list. These just distract from the actual changes and makes it hard to see what changes to review.

I did not try to review the rest until these are corrected.

Regards,
BALATON Zoltan



reply via email to

[Prev in Thread] Current Thread [Next in Thread]