| Message ID | 20210417103028.601124-7-f4bug@amsat.org |
|---|---|
| State | New |
| Headers | show |
| Series | memory: Forbid mapping AddressSpace root MemoryRegion | expand |
On Sat, Apr 17, 2021 at 12:30:23PM +0200, Philippe Mathieu-Daudé wrote: > Commit 1ae1dc5ba24 ("raven: Set a correct PCI I/O memory region") > abused an AddressSpace API weakness which allows set non-zero base > address to AddressSpace root region. We will fix that in the next > commit. First add an assertion to ensure no regression is introduced. > As raven_io_address() is called by both MemoryRegionOps handlers, it > is a good place for such assert call. > > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Acked-by: David Gibson <david@gibson.dropbear.id.au> > --- > hw/pci-host/raven.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/hw/pci-host/raven.c b/hw/pci-host/raven.c > index 730f31a8931..36652122424 100644 > --- a/hw/pci-host/raven.c > +++ b/hw/pci-host/raven.c > @@ -141,6 +141,17 @@ static const MemoryRegionOps raven_intack_ops = { > static inline hwaddr raven_io_address(PREPPCIState *s, > hwaddr addr) > { > + /* > + * We shouldn't access AddressSpace internals. However this assert > + * is temporarily introduced to prove a subtle inconsistency from > + * commit 1ae1dc5ba24 ("raven: Set a correct PCI I/O memory region"): > + * AddressSpace root region must be zero-based, but the Raven use is not. > + * > + * Assert the root region is based on physical address 0x80000000 > + * until the issue is fixed. > + */ > + assert(s->pci_io_as.root->addr == PCI_IO_BASE_ADDR); > + > if (s->contiguous_map == 0) { > /* 64 KB contiguous space for IOs */ > addr &= 0xFFFF;
diff --git a/hw/pci-host/raven.c b/hw/pci-host/raven.c index 730f31a8931..36652122424 100644 --- a/hw/pci-host/raven.c +++ b/hw/pci-host/raven.c @@ -141,6 +141,17 @@ static const MemoryRegionOps raven_intack_ops = { static inline hwaddr raven_io_address(PREPPCIState *s, hwaddr addr) { + /* + * We shouldn't access AddressSpace internals. However this assert + * is temporarily introduced to prove a subtle inconsistency from + * commit 1ae1dc5ba24 ("raven: Set a correct PCI I/O memory region"): + * AddressSpace root region must be zero-based, but the Raven use is not. + * + * Assert the root region is based on physical address 0x80000000 + * until the issue is fixed. + */ + assert(s->pci_io_as.root->addr == PCI_IO_BASE_ADDR); + if (s->contiguous_map == 0) { /* 64 KB contiguous space for IOs */ addr &= 0xFFFF;
Commit 1ae1dc5ba24 ("raven: Set a correct PCI I/O memory region") abused an AddressSpace API weakness which allows set non-zero base address to AddressSpace root region. We will fix that in the next commit. First add an assertion to ensure no regression is introduced. As raven_io_address() is called by both MemoryRegionOps handlers, it is a good place for such assert call. Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> --- hw/pci-host/raven.c | 11 +++++++++++ 1 file changed, 11 insertions(+)