On Mon, Jun 06, 2022 at 10:51:23AM -0300, Daniel Henrique Barboza wrote: > Michael, > > > I'll queue this patch with the commit msg proposed by Zoltan as follows: > > > Author: Michael S. Tsirkin <m...@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
ACK > > 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 <bala...@eik.bme.hu> > > Tested-by: BALATON Zoltan <bala...@eik.bme.hu> > > Signed-off-by: Michael S. Tsirkin <m...@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: