[PATCH 0/3] hw/isa/piix4: Remove MIPS Malta specific bits

[PATCH 0/3] hw/isa/piix4: Remove MIPS Malta specific bits

[Philippe Mathieu-Daudé]

Bernhard posted a series merging both PIIX3/PIIX4 models
in one [1]. Due to Malta-specific board code forced into
the PIIX4 reset values, Bernhard had to include an array
of "register values at reset" as a class property. This
is not wrong, but to model properly the model, we should
simply use the hardware real reset values, not try to
bend the model to please the Malta board.

This series fix this issue by having the Malta bootloader
code setting the board-specific PIIX4 IRQ routing values.

Note patch 2 still misses an equivalent nanoMIPS code.

Regards,

Phil.

[1] https://lore.kernel.org/qemu-devel/20221022150508.26830-1-shentey@gmail.com/

Philippe Mathieu-Daudé (3):
  hw/mips/malta: Introduce PIIX4_PCI_DEVFN definition
  hw/mips/malta: Set PIIX4 IRQ routes in embedded bootloader
  hw/isa/piix4: Correct IRQRC[A:D] reset values

 hw/isa/piix4.c  |  8 ++++----
 hw/mips/malta.c | 23 ++++++++++++++++++++++-
 2 files changed, 26 insertions(+), 5 deletions(-)

-- 
2.37.3

[PATCH 1/3] hw/mips/malta: Introduce PIIX4_PCI_DEVFN definition

[Philippe Mathieu-Daudé]

The PIIX4 PCI-ISA bridge function is always located at 10:0.
Since we want to re-use its address, add the PIIX4_PCI_DEVFN
definition.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/mips/malta.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/mips/malta.c b/hw/mips/malta.c
index 272d93eea7..df0f448b67 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -72,6 +72,8 @@
 
 #define MAX_IDE_BUS         2
 
+#define PIIX4_PCI_DEVFN     PCI_DEVFN(10, 0)
+
 typedef struct {
     MemoryRegion iomem;
     MemoryRegion iomem_lo; /* 0 - 0x900 */
@@ -1377,7 +1379,7 @@ void mips_malta_init(MachineState *machine)
     empty_slot_init("GT64120", 0, 0x20000000);
 
     /* Southbridge */
-    piix4 = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(10, 0), true,
+    piix4 = pci_create_simple_multifunction(pci_bus, PIIX4_PCI_DEVFN, true,
                                             TYPE_PIIX4_PCI_DEVICE);
     dev = DEVICE(piix4);
     isa_bus = ISA_BUS(qdev_get_child_bus(dev, "isa.0"));
-- 
2.37.3

[PATCH 2/3] hw/mips/malta: Set PIIX4 IRQ routes in embedded bootloader

[Philippe Mathieu-Daudé]

Linux kernel expects the northbridge & southbridge chipsets
configured by the BIOS firmware. We emulate that by writing
a tiny bootloader code in write_bootloader().

Upon introduction in commit 5c2b87e34d ("PIIX4 support"),
the PIIX4 configuration space included values specific to
the Malta board.

Set the Malta-specific IRQ routing values in the embedded
bootloader, so the next commit can remove the Malta specific
bits from the PIIX4 PCI-ISA bridge and make it generic
(matching the real hardware).

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
FIXME: Missing the nanoMIPS counter-part!
---
 hw/mips/malta.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/hw/mips/malta.c b/hw/mips/malta.c
index df0f448b67..f0b98922f2 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -804,6 +804,8 @@ static void write_bootloader_nanomips(uint8_t *base, uint64_t run_addr,
     stw_p(p++, 0x8422); stw_p(p++, 0x9088);
                                 /* sw t0, 0x88(t1)              */
 
+    /* TODO set PIIX IRQC[A:D] routing values! */
+
     stw_p(p++, 0xe320 | NM_HI1(kernel_entry));
 
     stw_p(p++, NM_HI2(kernel_entry));
@@ -841,6 +843,9 @@ static void write_bootloader_nanomips(uint8_t *base, uint64_t run_addr,
 static void write_bootloader(uint8_t *base, uint64_t run_addr,
                              uint64_t kernel_entry)
 {
+    const char pci_pins_cfg[PCI_NUM_PINS] = {
+        10, 10, 11, 11 /* PIIX IRQRC[A:D] */
+    };
     uint32_t *p;
 
     /* Small bootloader */
@@ -915,6 +920,20 @@ static void write_bootloader(uint8_t *base, uint64_t run_addr,
 
 #undef cpu_to_gt32
 
+    /*
+     * The PIIX ISA bridge is on PCI bus 0 dev 10 func 0.
+     * Load the PIIX IRQC[A:D] routing config address, then
+     * write routing configuration to the config data register.
+     */
+    bl_gen_write_u32(&p, /* GT_PCI0_CFGADDR */
+                     cpu_mips_phys_to_kseg1(NULL, 0x1be00000 + 0xcf8),
+                     bswap32((1 << 31) /* ConfigEn */
+                             | PCI_BUILD_BDF(0, PIIX4_PCI_DEVFN) << 8
+                             | PIIX_PIRQCA));
+    bl_gen_write_u32(&p, /* GT_PCI0_CFGDATA */
+                     cpu_mips_phys_to_kseg1(NULL, 0x1be00000 + 0xcfc),
+                     bswap32(ldl_le_p(pci_pins_cfg)));
+
     bl_gen_jump_kernel(&p,
                        true, ENVP_VADDR - 64,
                        /*
-- 
2.37.3

[PATCH 3/3] hw/isa/piix4: Correct IRQRC[A:D] reset values

[Philippe Mathieu-Daudé]

IRQRC[A:D] registers reset value is 0x80. We were forcing
the MIPS Malta machine routing to be able to boot a Linux
kernel without any bootloader.
We now have these registers initialized in the Malta machine
write_bootloader(), so we can use the correct reset values.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/isa/piix4.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 15f344dbb7..a2165c6a49 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -115,10 +115,10 @@ static void piix4_isa_reset(DeviceState *dev)
     pci_conf[0x4c] = 0x4d;
     pci_conf[0x4e] = 0x03;
     pci_conf[0x4f] = 0x00;
-    pci_conf[0x60] = 0x0a; // PCI A -> IRQ 10
-    pci_conf[0x61] = 0x0a; // PCI B -> IRQ 10
-    pci_conf[0x62] = 0x0b; // PCI C -> IRQ 11
-    pci_conf[0x63] = 0x0b; // PCI D -> IRQ 11
+    pci_conf[0x60] = 0x80;
+    pci_conf[0x61] = 0x80;
+    pci_conf[0x62] = 0x80;
+    pci_conf[0x63] = 0x80;
     pci_conf[0x69] = 0x02;
     pci_conf[0x70] = 0x80;
     pci_conf[0x76] = 0x0c;
-- 
2.37.3

Re: [PATCH 0/3] hw/isa/piix4: Remove MIPS Malta specific bits

[Philippe Mathieu-Daudé]

On 26/10/22 21:46, Philippe Mathieu-Daudé wrote:
> Bernhard posted a series merging both PIIX3/PIIX4 models
> in one [1]. Due to Malta-specific board code forced into
> the PIIX4 reset values, Bernhard had to include an array
> of "register values at reset" as a class property. This
> is not wrong, but to model properly the model, we should
> simply use the hardware real reset values, not try to
> bend the model to please the Malta board.
> 
> This series fix this issue by having the Malta bootloader
> code setting the board-specific PIIX4 IRQ routing values.
> 
> Note patch 2 still misses an equivalent nanoMIPS code.
> 
> Regards,
> 
> Phil.
> 
> [1] https://lore.kernel.org/qemu-devel/20221022150508.26830-1-shentey@gmail.com/
> 
> Philippe Mathieu-Daudé (3):
>    hw/mips/malta: Introduce PIIX4_PCI_DEVFN definition
>    hw/mips/malta: Set PIIX4 IRQ routes in embedded bootloader
>    hw/isa/piix4: Correct IRQRC[A:D] reset values

Based-on: <20221026191821.28167-1-philmd@linaro.org>
https://lore.kernel.org/qemu-devel/20221026191821.28167-1-philmd@linaro.org/

Re: [PATCH 3/3] hw/isa/piix4: Correct IRQRC[A:D] reset values

[Bernhard Beschow]

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

On Wed, Oct 26, 2022 at 9:46 PM Philippe Mathieu-Daudé <philmd@linaro.org>
wrote:

> IRQRC[A:D] registers reset value is 0x80. We were forcing
> the MIPS Malta machine routing to be able to boot a Linux
> kernel without any bootloader.
> We now have these registers initialized in the Malta machine
> write_bootloader(), so we can use the correct reset values.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/isa/piix4.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
> index 15f344dbb7..a2165c6a49 100644
> --- a/hw/isa/piix4.c
> +++ b/hw/isa/piix4.c
> @@ -115,10 +115,10 @@ static void piix4_isa_reset(DeviceState *dev)
>      pci_conf[0x4c] = 0x4d;
>      pci_conf[0x4e] = 0x03;
>      pci_conf[0x4f] = 0x00;
> -    pci_conf[0x60] = 0x0a; // PCI A -> IRQ 10
> -    pci_conf[0x61] = 0x0a; // PCI B -> IRQ 10
> -    pci_conf[0x62] = 0x0b; // PCI C -> IRQ 11
> -    pci_conf[0x63] = 0x0b; // PCI D -> IRQ 11
> +    pci_conf[0x60] = 0x80;
> +    pci_conf[0x61] = 0x80;
> +    pci_conf[0x62] = 0x80;
> +    pci_conf[0x63] = 0x80;
>

Running `qemu-system-mips64el -M malta -kernel vmlinux-3.2.0-4-5kc-malta
-hda debian_wheezy_mipsel_standard.qcow2 -append "root=/dev/sda1
console=ttyS0"` with this patch Linux outputs:

[    7.944000] uhci_hcd: USB Universal Host Controller Interface driver
[    7.944000] uhci_hcd 0000:00:0a.2: Found HC with no IRQ. Check BIOS/PCI
0000:00:0a.2 setup!
[    7.944000] uhci_hcd 0000:00:0a.2: init 0000:00:0a.2 fail, -19

Omitting this patch from the series the USB host is found.

Best regards,
Bernhard


>      pci_conf[0x69] = 0x02;
>      pci_conf[0x70] = 0x80;
>      pci_conf[0x76] = 0x0c;
> --
> 2.37.3
>
>

[-- Attachment #2: Type: text/html, Size: 2480 bytes --]

Re: [PATCH 3/3] hw/isa/piix4: Correct IRQRC[A:D] reset values

[Philippe Mathieu-Daudé]

On 26/10/22 23:32, Bernhard Beschow wrote:
> On Wed, Oct 26, 2022 at 9:46 PM Philippe Mathieu-Daudé 
> <philmd@linaro.org <mailto:philmd@linaro.org>> wrote:
> 
>     IRQRC[A:D] registers reset value is 0x80. We were forcing
>     the MIPS Malta machine routing to be able to boot a Linux
>     kernel without any bootloader.
>     We now have these registers initialized in the Malta machine
>     write_bootloader(), so we can use the correct reset values.
> 
>     Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org
>     <mailto:philmd@linaro.org>>
>     ---
>       hw/isa/piix4.c | 8 ++++----
>       1 file changed, 4 insertions(+), 4 deletions(-)
> 
>     diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
>     index 15f344dbb7..a2165c6a49 100644
>     --- a/hw/isa/piix4.c
>     +++ b/hw/isa/piix4.c
>     @@ -115,10 +115,10 @@ static void piix4_isa_reset(DeviceState *dev)
>           pci_conf[0x4c] = 0x4d;
>           pci_conf[0x4e] = 0x03;
>           pci_conf[0x4f] = 0x00;
>     -    pci_conf[0x60] = 0x0a; // PCI A -> IRQ 10
>     -    pci_conf[0x61] = 0x0a; // PCI B -> IRQ 10
>     -    pci_conf[0x62] = 0x0b; // PCI C -> IRQ 11
>     -    pci_conf[0x63] = 0x0b; // PCI D -> IRQ 11
>     +    pci_conf[0x60] = 0x80;
>     +    pci_conf[0x61] = 0x80;
>     +    pci_conf[0x62] = 0x80;
>     +    pci_conf[0x63] = 0x80;
> 
> 
> Running `qemu-system-mips64el -M malta -kernel vmlinux-3.2.0-4-5kc-malta 
> -hda debian_wheezy_mipsel_standard.qcow2 -append "root=/dev/sda1 
> console=ttyS0"` with this patch Linux outputs:
> 
> [    7.944000] uhci_hcd: USB Universal Host Controller Interface driver
> [    7.944000] uhci_hcd 0000:00:0a.2: Found HC with no IRQ. Check 
> BIOS/PCI 0000:00:0a.2 setup!
> [    7.944000] uhci_hcd 0000:00:0a.2: init 0000:00:0a.2 fail, -19
> 
> Omitting this patch from the series the USB host is found.

Oh, I should have used tswap() instead of bswap()! I have been moving
and my test suite is on an offline backup, I'm not yet set up. As you
see my testing is poor :/