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

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

[Philippe Mathieu-Daudé]

Since v1:
- bswap -> tswap (Bernhard)

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/

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

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 v2 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 v2 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..4403028778 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),
+                     tswap32((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),
+                     tswap32(ldl_be_p(pci_pins_cfg)));
+
     bl_gen_jump_kernel(&p,
                        true, ENVP_VADDR - 64,
                        /*
-- 
2.37.3

[PATCH v2 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 v2 0/3] hw/isa/piix4: Remove MIPS Malta specific bits

[Philippe Mathieu-Daudé]

On 27/10/22 22:47, Philippe Mathieu-Daudé wrote:
> Since v1:
> - bswap -> tswap (Bernhard)
> 
> 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.

So this series won't be merged until this is added, but
it should be enough to let Bernhard keep working on the
"Consolidate PIIX series".

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

[Bernhard Beschow]

Am 27. Oktober 2022 20:47:19 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>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!

Who will be taking care of this? I have absolutely no clue how the write_bootloader functions work, so I don't see how to fix it.

Couldn't we just do it like in pegasos2_init() where the registers are initialized by QEMU directly if there is no bootloader binary configured? I could do that.

Best regards,
Bernhard
>---
> hw/mips/malta.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
>diff --git a/hw/mips/malta.c b/hw/mips/malta.c
>index df0f448b67..4403028778 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),
>+                     tswap32((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),
>+                     tswap32(ldl_be_p(pci_pins_cfg)));
>+
>     bl_gen_jump_kernel(&p,
>                        true, ENVP_VADDR - 64,
>                        /*

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

[Philippe Mathieu-Daudé]

On 21/11/22 16:34, Bernhard Beschow wrote:
> 
> 
> Am 27. Oktober 2022 20:47:19 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>> 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!
> 
> Who will be taking care of this? I have absolutely no clue how the write_bootloader functions work, so I don't see how to fix it.

Oh actually I wrote that and tested it but context switched and forgot
about it... I'll look back when I get some time, probably around the
release.

> Couldn't we just do it like in pegasos2_init() where the registers are initialized by QEMU directly if there is no bootloader binary configured? I could do that.
I rather mimic bootloaders... maybe a matter of taste?

Regards,

Phil.

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

[Bernhard Beschow]

Am 21. November 2022 22:43:50 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>On 21/11/22 16:34, Bernhard Beschow wrote:
>> 
>> 
>> Am 27. Oktober 2022 20:47:19 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>>> 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!
>> 
>> Who will be taking care of this? I have absolutely no clue how the write_bootloader functions work, so I don't see how to fix it.
>
>Oh actually I wrote that and tested it but context switched and forgot
>about it... I'll look back when I get some time, probably around the
>release.
>
>> Couldn't we just do it like in pegasos2_init() where the registers are initialized by QEMU directly if there is no bootloader binary configured? I could do that.
>I rather mimic bootloaders... maybe a matter of taste?

I don't mind either way. I meant that I could help with the second approach but not with the current one since I have no clue whatsoever how it works. There are just too many magic constants that don't make any sense to me, and too many layers of indirection, for example.

Anyway, I'm asking for the current state because I'm pretty much ready for posting a v3 of my PIIX consolidation series which now depends on this series.

Best regards,
Bernhard

>
>Regards,
>
>Phil.

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

[BALATON Zoltan]

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

Hello,

On Mon, 21 Nov 2022, Bernhard Beschow wrote:
> Am 21. November 2022 22:43:50 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>> On 21/11/22 16:34, Bernhard Beschow wrote:
>>> Am 27. Oktober 2022 20:47:19 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>>>> 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!
>>>
>>> Who will be taking care of this? I have absolutely no clue how the write_bootloader functions work, so I don't see how to fix it.
>>
>> Oh actually I wrote that and tested it but context switched and forgot
>> about it... I'll look back when I get some time, probably around the
>> release.

Unrelated to this but found it while looking at malta.c now: another 
possible clean up is to replace the local generate_eeprom_spd() func with 
spd_data_generate() from hw/i2c/smbus_eeprom.c that other boards use 
already but I did not change malta because I could not test it. If you can 
test malta then it should be an easy change and simplify malta.c a bit.

>>> Couldn't we just do it like in pegasos2_init() where the registers are initialized by QEMU directly if there is no bootloader binary configured? I could do that.
>> I rather mimic bootloaders... maybe a matter of taste?

Is that a bootloader or a replacement firmware? To me bootloader is some 
OS specific binary that is loaded by firware to boot an OS. But there are 
OS independent bootloaders like grub so maybe you could emulate something 
like that, I don't know what malta does.

If there's no firmware binary QEMU should provide something to replace it 
to give the expected environment for the binary loaded by -kernel. In case 
of pegasos2 the init method sets up regs to init devices as done by the 
firmware and the rest is implemented by VOF (loaded from pc-bios) that 
provices the OpenFirmware client interface. The device setup in init is 
needed because VOF does not do that.

> I don't mind either way. I meant that I could help with the second 
> approach but not with the current one since I have no clue whatsoever 
> how it works. There are just too many magic constants that don't make 
> any sense to me, and too many layers of indirection, for example.

If malta has a replacement firmware for this case maybe it could be stored 
in a binary in pc-bios and loaded from there instead of writing it in hex 
to guest memory. That binary could even be assembled from source which 
should make it simpler to write and change. Or is YAMON open source? 
According to this page it is: 
https://www.mips.com/develop/tools/boot-loaders/ so maybe it could be 
included as a firmware binary instead of being emulated?

Regards,
BALATON Zoltan

> Anyway, I'm asking for the current state because I'm pretty much ready for posting a v3 of my PIIX consolidation series which now depends on this series.
>
> Best regards,
> Bernhard
>
>>
>> Regards,
>>
>> Phil.
>
>

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

[Jiaxun Yang]

> 2022年11月22日 12:37，BALATON Zoltan <balaton@eik.bme.hu> 写道：
> 
> Hello,
> 
> On Mon, 21 Nov 2022, Bernhard Beschow wrote:
>> Am 21. November 2022 22:43:50 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>>> On 21/11/22 16:34, Bernhard Beschow wrote:
>>>> Am 27. Oktober 2022 20:47:19 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>>>>> 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!
>>>> 
>>>> Who will be taking care of this? I have absolutely no clue how the write_bootloader functions work, so I don't see how to fix it.
>>> 
>>> Oh actually I wrote that and tested it but context switched and forgot
>>> about it... I'll look back when I get some time, probably around the
>>> release.

I can try to adopt existing boot loader helper functions, just a matter of opcodes I think.

> 
> Unrelated to this but found it while looking at malta.c now: another possible clean up is to replace the local generate_eeprom_spd() func with spd_data_generate() from hw/i2c/smbus_eeprom.c that other boards use already but I did not change malta because I could not test it. If you can test malta then it should be an easy change and simplify malta.c a bit.
> 
>>>> Couldn't we just do it like in pegasos2_init() where the registers are initialized by QEMU directly if there is no bootloader binary configured? I could do that.
>>> I rather mimic bootloaders... maybe a matter of taste?
> 
> Is that a bootloader or a replacement firmware? To me bootloader is some OS specific binary that is loaded by firware to boot an OS. But there are OS independent bootloaders like grub so maybe you could emulate something like that, I don't know what malta does.

YAMON is a OS-dependent and HW-dependent firmware like u-boot.

> 
> If there's no firmware binary QEMU should provide something to replace it to give the expected environment for the binary loaded by -kernel. In case of pegasos2 the init method sets up regs to init devices as done by the firmware and the rest is implemented by VOF (loaded from pc-bios) that provices the OpenFirmware client interface. The device setup in init is needed because VOF does not do that.
> 
>> I don't mind either way. I meant that I could help with the second approach but not with the current one since I have no clue whatsoever how it works. There are just too many magic constants that don't make any sense to me, and too many layers of indirection, for example.
> 
> If malta has a replacement firmware for this case maybe it could be stored in a binary in pc-bios and loaded from there instead of writing it in hex to guest memory. That binary could even be assembled from source which should make it simpler to write and change. Or is YAMON open source? According to this page it is: https://www.mips.com/develop/tools/boot-loaders/ so maybe it could be included as a firmware binary instead of being emulated?

Hmm, YAMON was a open source software but I’m unable to find a copy of source for Malta board comes with GT chipset that QEMU emulated.
So nowadays we mainly use -kernel feature to do direct kernel boot.

Direct kernel boot is really a brilliant function that I don’t want to lose :-)

Thanks
- Jiaxun


> 
> Regards,
> BALATON Zoltan
> 
>> Anyway, I'm asking for the current state because I'm pretty much ready for posting a v3 of my PIIX consolidation series which now depends on this series.
>> 
>> Best regards,
>> Bernhard
>> 
>>> 
>>> Regards,
>>> 
>>> Phil.
>> 

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

[Bernhard Beschow]

Am 27. Oktober 2022 20:47:18 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>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>
>---

Reviewed-by: Bernhard Beschow <shentey@gmail.com>

> 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"));

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

[Bernhard Beschow]

Am 27. Oktober 2022 20:47:20 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>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>
>---

Reviewed-by: Bernhard Beschow <shentey@gmail.com>

> 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;

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

[Bernhard Beschow]

Am 27. Oktober 2022 20:47:20 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>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>

https://lists.nongnu.org/archive/html/qemu-devel/2022-12/msg03048.html :

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

>---
> 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;

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

[Bernhard Beschow]

Am 21. November 2022 15:34:05 UTC schrieb Bernhard Beschow <shentey@gmail.com>:
>
>
>Am 27. Oktober 2022 20:47:19 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>>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!
>
>Who will be taking care of this? I have absolutely no clue how the write_bootloader functions work, so I don't see how to fix it.

Ping

>Couldn't we just do it like in pegasos2_init() where the registers are initialized by QEMU directly if there is no bootloader binary configured? I could do that.
>
>Best regards,
>Bernhard
>>---
>> hw/mips/malta.c | 19 +++++++++++++++++++
>> 1 file changed, 19 insertions(+)
>>
>>diff --git a/hw/mips/malta.c b/hw/mips/malta.c
>>index df0f448b67..4403028778 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),
>>+                     tswap32((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),
>>+                     tswap32(ldl_be_p(pci_pins_cfg)));
>>+
>>     bl_gen_jump_kernel(&p,
>>                        true, ENVP_VADDR - 64,
>>                        /*

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

[Philippe Mathieu-Daudé]

On 31/12/22 10:53, Bernhard Beschow wrote:
> 
> 
> Am 21. November 2022 15:34:05 UTC schrieb Bernhard Beschow <shentey@gmail.com>:
>>
>>
>> Am 27. Oktober 2022 20:47:19 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>>> 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!
>>
>> Who will be taking care of this? I have absolutely no clue how the write_bootloader functions work, so I don't see how to fix it.
> 
> Ping

This comment has been taken care of:
https://lore.kernel.org/qemu-devel/a3c3f639-dbb1-88a7-43fe-547a234c5890@linaro.org/

However while testing the MIPS pull request I prepared I
found a bug in the GT64120 which I'm trying to fix since
various days... Unfortunately your series depends on it,
so this is a blocking issue. Sorry for this long delay...

Phil.

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

[Bernhard Beschow]

Am 31. Dezember 2022 13:44:00 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>On 31/12/22 10:53, Bernhard Beschow wrote:
>> 
>> 
>> Am 21. November 2022 15:34:05 UTC schrieb Bernhard Beschow <shentey@gmail.com>:
>>> 
>>> 
>>> Am 27. Oktober 2022 20:47:19 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>>>> 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!
>>> 
>>> Who will be taking care of this? I have absolutely no clue how the write_bootloader functions work, so I don't see how to fix it.
>> 
>> Ping
>
>This comment has been taken care of:
>https://lore.kernel.org/qemu-devel/a3c3f639-dbb1-88a7-43fe-547a234c5890@linaro.org/

Ah, now I see where this is going.

>However while testing the MIPS pull request I prepared I
>found a bug in the GT64120 which I'm trying to fix since
>various days... Unfortunately your series depends on it,
>so this is a blocking issue. Sorry for this long delay...

Don't worry.

How can the bug be reproduced? Is there a test run in the CI? Note that I get a 404 when trying to access https://gitlab.com/philmd/qemu/-/commits/mips-testing/ .

Best regards,
Bernhard

>
>Phil.

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

[Philippe Mathieu-Daudé]

On 2/1/23 01:03, Bernhard Beschow wrote:
> 
> 
> Am 31. Dezember 2022 13:44:00 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>> On 31/12/22 10:53, Bernhard Beschow wrote:
>>>
>>>
>>> Am 21. November 2022 15:34:05 UTC schrieb Bernhard Beschow <shentey@gmail.com>:
>>>>
>>>>
>>>> Am 27. Oktober 2022 20:47:19 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>>>>> 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!
>>>>
>>>> Who will be taking care of this? I have absolutely no clue how the write_bootloader functions work, so I don't see how to fix it.
>>>
>>> Ping
>>
>> This comment has been taken care of:
>> https://lore.kernel.org/qemu-devel/a3c3f639-dbb1-88a7-43fe-547a234c5890@linaro.org/
> 
> Ah, now I see where this is going.
> 
>> However while testing the MIPS pull request I prepared I
>> found a bug in the GT64120 which I'm trying to fix since
>> various days... Unfortunately your series depends on it,
>> so this is a blocking issue. Sorry for this long delay...
> 
> Don't worry.
> 
> How can the bug be reproduced? Is there a test run in the CI?

My problem was on big-endian hosts, now fixed by:
https://lore.kernel.org/qemu-devel/20230104133935.4639-1-philmd@linaro.org/

> Note that I get a 404 when trying to access https://gitlab.com/philmd/qemu/-/commits/mips-testing/ .

Oh for some reason my repository was set for 'project members', I now 
changed that to 'everyone'.

I'll rebuild the mips-testing queue later today or tomorrow and restart 
my testing.

Regards,

Phil.