On Fri, Mar 12, 2021 at 03:38:21PM -0500, Peter Xu wrote: > On Fri, Mar 12, 2021 at 07:28:49PM +0100, Philippe Mathieu-Daudé wrote: > > The pci_io_non_contiguous region is mapped on top of pci_io > > with higher priority, but simply dispatch into this region > > address space. Simplify by directly registering the former > > region in place, and adapt the address space dispatch offsets. > > > > Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org> > > --- > > hw/pci-host/prep.c | 11 ++++------- > > 1 file changed, 4 insertions(+), 7 deletions(-) > > > > diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c > > index 0a9162fba97..00a28c2d18c 100644 > > --- a/hw/pci-host/prep.c > > +++ b/hw/pci-host/prep.c > > @@ -159,8 +159,7 @@ static uint64_t raven_io_read(void *opaque, hwaddr addr, > > uint8_t buf[4]; > > > > addr = raven_io_address(s, addr); > > - address_space_read(&s->pci_io_as, addr + 0x80000000, > > - MEMTXATTRS_UNSPECIFIED, buf, size); > > + address_space_read(&s->pci_io_as, addr, MEMTXATTRS_UNSPECIFIED, buf, > > size); > > > > if (size == 1) { > > return buf[0]; > > @@ -191,8 +190,7 @@ static void raven_io_write(void *opaque, hwaddr addr, > > g_assert_not_reached(); > > } > > > > - address_space_write(&s->pci_io_as, addr + 0x80000000, > > - MEMTXATTRS_UNSPECIFIED, buf, size); > > + address_space_write(&s->pci_io_as, addr, MEMTXATTRS_UNSPECIFIED, buf, > > size); > > This changes access to s->pci_io_as, but below didn't change s->pci_io_as > layout at all (below is about address_space_mem). Is this intended? > > > } > > > > static const MemoryRegionOps raven_io_ops = { > > @@ -294,9 +292,8 @@ static void raven_pcihost_initfn(Object *obj) > > address_space_init(&s->pci_io_as, &s->pci_io, "raven-io"); > > > > /* CPU address space */ > > - memory_region_add_subregion(address_space_mem, 0x80000000, &s->pci_io); > > - memory_region_add_subregion_overlap(address_space_mem, 0x80000000, > > - &s->pci_io_non_contiguous, 1); > > + memory_region_add_subregion(address_space_mem, 0x80000000, > > + &s->pci_io_non_contiguous); > > I don't know any of this code at all... but it seems the two memory regions > are > not identical in size: > > memory_region_init(&s->pci_io, obj, "pci-io", 0x3f800000); > memory_region_init_io(&s->pci_io_non_contiguous, obj, &raven_io_ops, s, > "pci-io-non-contiguous", 0x00800000); > > Then it seems the memory access dispatching to (0x00800000, 0x3f800000) would > change too, from s->pci_io to nothing. Raise this up too since I don't know > either whether it's intended..
Right, it seems like this removes the mapping of s->pci_io entirely. > > > memory_region_add_subregion(address_space_mem, 0xc0000000, > > &s->pci_memory); > > pci_root_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), DEVICE(obj), > > NULL, > > &s->pci_memory, &s->pci_io, 0, TYPE_PCI_BUS); > -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature