qemu-devel
[Top][All Lists]
Advanced

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

Re: [PATCH] ppc: fix boot with sam460ex


From: Daniel Henrique Barboza
Subject: Re: [PATCH] ppc: fix boot with sam460ex
Date: Mon, 6 Jun 2022 10:51:23 -0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.10.0

Michael,


I'll queue this patch with the commit msg proposed by Zoltan as follows:


Author: Michael S. Tsirkin <mst@redhat.com>
Date:   Thu May 26 18:43:43 2022 -0400

    ppc: fix boot with sam460ex
Recent changes to pcie_host corrected size of its internal region to
    match what it expects: only the low 28 bits are ever decoded. Previous
    code just ignored bit 29 (if size was 1 << 29) in the address which does
    not make much sense.  We are now asserting on size > 1 << 28 instead,
    but PPC 4xx actually allows guest to configure different sizes, and some
    firmwares seem to set it to 1 << 29.
This caused e.g. qemu-system-ppc -M sam460ex to exit with an assert when
    the guest writes a value to CFGMSK register when trying to map config
    space. This is done in the board firmware in ppc4xx_init_pcie_port() in
    roms/u-boot-sam460ex/arch/powerpc/cpu/ppc4xx/4xx_pcie.c
It's not clear what the proper fix should be but for now let's force the
    size to 256MB, so anything outside the expected address range is
    ignored.

Is that ok with you?


Thanks,


Daniel


On 5/26/22 19:43, Michael S. Tsirkin wrote:
Recent changes to pcie_host corrected size of its internal region to
match what it expects - only the low 28 bits are ever decoded. Previous
code just ignored bit 29 (if size was 1 << 29) in the address which does
not make much sense.  We are now asserting on size > 1 << 28 instead,
but it so happened that ppc actually allows guest to configure as large
a size as it wants to, and current firmware set it to 1 << 29.

With just qemu-system-ppc -M sam460ex this triggers an assert which
seems to happen when the guest (board firmware?) writes a value to
CFGMSK reg:

(gdb) bt

This is done in the board firmware here:

https://git.qemu.org/?p=u-boot-sam460ex.git;a=blob;f=arch/powerpc/cpu/ppc4xx/4xx_pcie.c;h=13348be93dccc74c13ea043d6635a7f8ece4b5f0;hb=HEAD

when trying to map config space.

Note that what firmware does matches
https://www.hardware.com.br/comunidade/switch-cisco/1128380/

So it's not clear what the proper fix should be.

However, allowing guest to trigger an assert in qemu is not good practice 
anyway.

For now let's just force the mask to 256MB on guest write, this way
anything outside the expected address range is ignored.

Fixes: commit 1f1a7b2269 ("include/hw/pci/pcie_host: Correct 
PCIE_MMCFG_SIZE_MAX")
Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
Tested-by: BALATON Zoltan <balaton@eik.bme.hu>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---

Affected system is orphan so I guess I will merge the patch unless
someone objects.

  hw/ppc/ppc440_uc.c | 8 ++++++++
  1 file changed, 8 insertions(+)

diff --git a/hw/ppc/ppc440_uc.c b/hw/ppc/ppc440_uc.c
index 993e3ba955..a1ecf6dd1c 100644
--- a/hw/ppc/ppc440_uc.c
+++ b/hw/ppc/ppc440_uc.c
@@ -1180,6 +1180,14 @@ static void dcr_write_pcie(void *opaque, int dcrn, 
uint32_t val)
      case PEGPL_CFGMSK:
          s->cfg_mask = val;
          size = ~(val & 0xfffffffe) + 1;
+        /*
+         * Firmware sets this register to E0000001. Why we are not sure,
+         * but the current guess is anything above PCIE_MMCFG_SIZE_MAX is
+         * ignored.
+         */
+        if (size > PCIE_MMCFG_SIZE_MAX) {
+            size = PCIE_MMCFG_SIZE_MAX;
+        }
          pcie_host_mmcfg_update(PCIE_HOST_BRIDGE(s), val & 1, s->cfg_base, 
size);
          break;
      case PEGPL_MSGBAH:



reply via email to

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