Re: [PATCH v2] sh4: Fix PCI ISA IO memory subregion

[Peter Maydell]

On Tue, 18 Feb 2020 at 20:10, Guenter Roeck <address@hidden> wrote:
>
> Booting the r2d machine from flash fails because flash is not discovered.
> Looking at the flattened memory tree, we see the following.
>
> FlatView #1
>  AS "memory", root: system
>  AS "cpu-memory-0", root: system
>  AS "sh_pci_host", root: bus master container
>  Root memory region: system
>   0000000000000000-000000000000ffff (prio 0, i/o): io
>   0000000000010000-0000000000ffffff (prio 0, i/o): r2d.flash @0000000000010000
>
> The overlapping memory region is sh_pci.isa, ie the ISA I/O region bridge.
> This region is initially assigned to address 0xfe240000, but overwritten
> with a write into the PCIIOBR register. This write is expected to adjust
> the PCI memory window, but not to change the region's base adddress.
>
> Peter Maydell provided the following detailed explanation.
>
> "Section 22.3.7 and in particular figure 22.3 (of "SSH7751R user's manual:
> hardware") are clear about how this is supposed to work: there is a window
> at 0xfe240000 in the system register space for PCI I/O space. When the CPU
> makes an access into that area, the PCI controller calculates the PCI
> address to use by combining bits 0..17 of the system address with the
> bits 31..18 value that the guest has put into the PCIIOBR. That is, writing
> to the PCIIOBR changes which section of the IO address space is visible in
> the 0xfe240000 window. Instead what QEMU's implementation does is move the
> window to whatever value the guest writes to the PCIIOBR register -- so if
> the guest writes 0 we put the window at 0 in system address space."
>
> Fix the problem by calling memory_region_set_alias_offset() instead of
> removing and re-adding the PCI ISA subregion on writes into PCIIOBR.
> At the same time, in sh_pci_device_realize(), don't set iobr since
> it is overwritten later anyway. Instead, pass the base address to
> memory_region_add_subregion() directly.
>
> Many thanks to Peter Maydell for the detailed problem analysis, and for
> providing suggestions on how to fix the problem.
>
> Signed-off-by: Guenter Roeck <address@hidden>

Reviewed-by: Peter Maydell <address@hidden>

I'll put this in via target-arm.next, since we don't really
have a more active sh4-specific tree to send it via.

thanks
-- PMM