| Message ID | 20191220211512.3289-2-svens@stackframe.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v5,1/6] hw/hppa/dino.c: Improve emulation of Dino PCI chip | expand |
Hi Sven, Helge. On 12/20/19 10:15 PM, Sven Schnelle wrote: > From: Helge Deller <deller@gmx.de> > > The tests of the dino chip with the Online-diagnostics CD > ("ODE DINOTEST") now succeeds. > Additionally add some qemu trace events. > > Signed-off-by: Helge Deller <deller@gmx.de> > Signed-off-by: Sven Schnelle <svens@stackframe.org> > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > MAINTAINERS | 2 +- > hw/hppa/dino.c | 97 +++++++++++++++++++++++++++++++++++++------- > hw/hppa/trace-events | 5 +++ > 3 files changed, 89 insertions(+), 15 deletions(-) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 387879aebc..e333bc67a4 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -876,7 +876,7 @@ F: hw/*/etraxfs_*.c > > HP-PARISC Machines > ------------------ > -Dino > +HP B160L > M: Richard Henderson <rth@twiddle.net> > R: Helge Deller <deller@gmx.de> > S: Odd Fixes > diff --git a/hw/hppa/dino.c b/hw/hppa/dino.c > index ab6969b45f..9797a7f0d9 100644 > --- a/hw/hppa/dino.c > +++ b/hw/hppa/dino.c > @@ -1,7 +1,7 @@ > /* > - * HP-PARISC Dino PCI chipset emulation. > + * HP-PARISC Dino PCI chipset emulation, as in B160L and similiar machines > * > - * (C) 2017 by Helge Deller <deller@gmx.de> > + * (C) 2017-2019 by Helge Deller <deller@gmx.de> > * > * This work is licensed under the GNU GPL license version 2 or later. > * > @@ -21,6 +21,7 @@ > #include "migration/vmstate.h" > #include "hppa_sys.h" > #include "exec/address-spaces.h" > +#include "trace.h" > > > #define TYPE_DINO_PCI_HOST_BRIDGE "dino-pcihost" > @@ -82,11 +83,28 @@ > #define DINO_PCI_HOST_BRIDGE(obj) \ > OBJECT_CHECK(DinoState, (obj), TYPE_DINO_PCI_HOST_BRIDGE) > > +#define DINO800_REGS ((DINO_TLTIM - DINO_GMASK) / 4) Coverity noticed (CID 1419394) this should be '(1 + (DINO_TLTIM - DINO_GMASK) / 4)' (13 registers). > +static const uint32_t reg800_keep_bits[DINO800_REGS] = { > + MAKE_64BIT_MASK(0, 1), > + MAKE_64BIT_MASK(0, 7), > + MAKE_64BIT_MASK(0, 7), > + MAKE_64BIT_MASK(0, 8), > + MAKE_64BIT_MASK(0, 7), > + MAKE_64BIT_MASK(0, 9), > + MAKE_64BIT_MASK(0, 32), Looking at the datasheet pp. 30, this register is 'Undefined'. We should report GUEST_ERROR if it is accessed. > + MAKE_64BIT_MASK(0, 8), > + MAKE_64BIT_MASK(0, 30), > + MAKE_64BIT_MASK(0, 25), Still looking at the datasheet (pp. 37) PCIROR is 24-bit (not 25). > + MAKE_64BIT_MASK(0, 22), Here you are missing register 0x82c... > + MAKE_64BIT_MASK(0, 9), > +}; > + Altogether: static const uint32_t reg800_keep_bits[DINO800_REGS] = { MAKE_64BIT_MASK(0, 1), /* GMASK */ MAKE_64BIT_MASK(0, 7), /* PAMR */ MAKE_64BIT_MASK(0, 7), /* PAPR */ MAKE_64BIT_MASK(0, 8), /* DAMODE */ MAKE_64BIT_MASK(0, 7), /* PCICMD */ MAKE_64BIT_MASK(0, 9), /* PCISTS */ MAKE_64BIT_MASK(0, 0), /* Undefined */ MAKE_64BIT_MASK(0, 8), /* MLTIM */ MAKE_64BIT_MASK(0, 30), /* BRDG_FEAT */ MAKE_64BIT_MASK(0, 24), /* PCIROR */ MAKE_64BIT_MASK(0, 22), /* PCIWOR */ MAKE_64BIT_MASK(0, 0), /* Undocumented */ MAKE_64BIT_MASK(0, 9), /* TLTIM */ }; > typedef struct DinoState { > PCIHostState parent_obj; > > /* PCI_CONFIG_ADDR is parent_obj.config_reg, via pci_host_conf_be_ops, > so that we can map PCI_CONFIG_DATA to pci_host_data_be_ops. */ > + uint32_t config_reg_dino; /* keep original copy, including 2 lowest bits */ > > uint32_t iar0; > uint32_t iar1; > @@ -94,8 +112,12 @@ typedef struct DinoState { > uint32_t ipr; > uint32_t icr; > uint32_t ilr; > + uint32_t io_fbb_en; > uint32_t io_addr_en; > uint32_t io_control; > + uint32_t toc_addr; > + > + uint32_t reg800[DINO800_REGS]; > > MemoryRegion this_mem; > MemoryRegion pci_mem; > @@ -106,8 +128,6 @@ typedef struct DinoState { > MemoryRegion bm_ram_alias; > MemoryRegion bm_pci_alias; > MemoryRegion bm_cpu_alias; > - > - MemoryRegion cpu0_eir_mem; > } DinoState; > > /* > @@ -122,6 +142,8 @@ static void gsc_to_pci_forwarding(DinoState *s) > tmp = extract32(s->io_control, 7, 2); > enabled = (tmp == 0x01); > io_addr_en = s->io_addr_en; > + /* Mask out first (=firmware) and last (=Dino) areas. */ > + io_addr_en &= ~(BIT(31) | BIT(0)); > > memory_region_transaction_begin(); > for (i = 1; i < 31; i++) { > @@ -142,6 +164,8 @@ static bool dino_chip_mem_valid(void *opaque, hwaddr addr, > unsigned size, bool is_write, > MemTxAttrs attrs) > { > + bool ret = false; > + > switch (addr) { > case DINO_IAR0: > case DINO_IAR1: > @@ -152,16 +176,22 @@ static bool dino_chip_mem_valid(void *opaque, hwaddr addr, > case DINO_ICR: > case DINO_ILR: > case DINO_IO_CONTROL: > + case DINO_IO_FBB_EN: > case DINO_IO_ADDR_EN: > case DINO_PCI_IO_DATA: > - return true; > + case DINO_TOC_ADDR: > + case DINO_GMASK ... DINO_TLTIM: > + ret = true; > + break; > case DINO_PCI_IO_DATA + 2: > - return size <= 2; > + ret = (size <= 2); > + break; > case DINO_PCI_IO_DATA + 1: > case DINO_PCI_IO_DATA + 3: > - return size == 1; > + ret = (size == 1); > } > - return false; > + trace_dino_chip_mem_valid(addr, ret); > + return ret; > } > > static MemTxResult dino_chip_read_with_attrs(void *opaque, hwaddr addr, > @@ -194,6 +224,9 @@ static MemTxResult dino_chip_read_with_attrs(void *opaque, hwaddr addr, > } > break; > > + case DINO_IO_FBB_EN: > + val = s->io_fbb_en; > + break; > case DINO_IO_ADDR_EN: > val = s->io_addr_en; > break; > @@ -227,12 +260,28 @@ static MemTxResult dino_chip_read_with_attrs(void *opaque, hwaddr addr, > case DINO_IRR1: > val = s->ilr & s->imr & s->icr; > break; > + case DINO_TOC_ADDR: > + val = s->toc_addr; > + break; > + case DINO_GMASK ... DINO_TLTIM: > + val = s->reg800[(addr - DINO_GMASK) / 4]; > + if (addr == DINO_PAMR) { > + val &= ~0x01; /* LSB is hardwired to 0 */ > + } > + if (addr == DINO_MLTIM) { > + val &= ~0x07; /* 3 LSB are hardwired to 0 */ > + } > + if (addr == DINO_BRDG_FEAT) { > + val &= ~(0x10710E0ul | 8); /* bits 5-7, 24 & 15 reserved */ > + } > + break; > > default: > /* Controlled by dino_chip_mem_valid above. */ > g_assert_not_reached(); > } > > + trace_dino_chip_read(addr, val); > *data = val; > return ret; > } > @@ -245,6 +294,9 @@ static MemTxResult dino_chip_write_with_attrs(void *opaque, hwaddr addr, > AddressSpace *io; > MemTxResult ret; > uint16_t ioaddr; > + int i; > + > + trace_dino_chip_write(addr, val); > > switch (addr) { > case DINO_IO_DATA ... DINO_PCI_IO_DATA + 3: > @@ -266,9 +318,11 @@ static MemTxResult dino_chip_write_with_attrs(void *opaque, hwaddr addr, > } > return ret; > > + case DINO_IO_FBB_EN: > + s->io_fbb_en = val & 0x03; > + break; > case DINO_IO_ADDR_EN: > - /* Never allow first (=firmware) and last (=Dino) areas. */ > - s->io_addr_en = val & 0x7ffffffe; > + s->io_addr_en = val; > gsc_to_pci_forwarding(s); > break; > case DINO_IO_CONTROL: > @@ -292,6 +346,10 @@ static MemTxResult dino_chip_write_with_attrs(void *opaque, hwaddr addr, > /* Any write to IPR clears the register. */ > s->ipr = 0; > break; > + case DINO_TOC_ADDR: > + /* IO_COMMAND of CPU with client_id bits */ > + s->toc_addr = 0xFFFA0030 | (val & 0x1e000); > + break; > > case DINO_ILR: > case DINO_IRR0: > @@ -299,6 +357,12 @@ static MemTxResult dino_chip_write_with_attrs(void *opaque, hwaddr addr, > /* These registers are read-only. */ > break; > > + case DINO_GMASK ... DINO_TLTIM: > + i = (addr - DINO_GMASK) / 4; > + val &= reg800_keep_bits[i]; Due to the missing register, Coverity reports: >>> CID 1419394: Memory - illegal accesses (OVERRUN) >>> Overrunning array "reg800_keep_bits" of 12 4-byte elements at element index 12 (byte offset 48) using index "i" (which evaluates to 12). > + s->reg800[i] = val; > + break; > + > default: > /* Controlled by dino_chip_mem_valid above. */ > g_assert_not_reached(); > @@ -323,7 +387,7 @@ static const MemoryRegionOps dino_chip_ops = { > > static const VMStateDescription vmstate_dino = { > .name = "Dino", > - .version_id = 1, > + .version_id = 2, > .minimum_version_id = 1, > .fields = (VMStateField[]) { > VMSTATE_UINT32(iar0, DinoState), > @@ -332,13 +396,14 @@ static const VMStateDescription vmstate_dino = { > VMSTATE_UINT32(ipr, DinoState), > VMSTATE_UINT32(icr, DinoState), > VMSTATE_UINT32(ilr, DinoState), > + VMSTATE_UINT32(io_fbb_en, DinoState), > VMSTATE_UINT32(io_addr_en, DinoState), > VMSTATE_UINT32(io_control, DinoState), > + VMSTATE_UINT32(toc_addr, DinoState), > VMSTATE_END_OF_LIST() > } > }; > > - > /* Unlike pci_config_data_le_ops, no check of high bit set in config_reg. */ > > static uint64_t dino_config_data_read(void *opaque, hwaddr addr, unsigned len) > @@ -362,14 +427,16 @@ static const MemoryRegionOps dino_config_data_ops = { > > static uint64_t dino_config_addr_read(void *opaque, hwaddr addr, unsigned len) > { > - PCIHostState *s = opaque; > - return s->config_reg; > + DinoState *s = opaque; > + return s->config_reg_dino; > } > > static void dino_config_addr_write(void *opaque, hwaddr addr, > uint64_t val, unsigned len) > { > PCIHostState *s = opaque; > + DinoState *ds = opaque; > + ds->config_reg_dino = val; /* keep a copy of original value */ > s->config_reg = val & ~3U; > } > > @@ -453,6 +520,8 @@ PCIBus *dino_init(MemoryRegion *addr_space, > > dev = qdev_create(NULL, TYPE_DINO_PCI_HOST_BRIDGE); > s = DINO_PCI_HOST_BRIDGE(dev); > + s->iar0 = s->iar1 = CPU_HPA + 3; > + s->toc_addr = 0xFFFA0030; /* IO_COMMAND of CPU */ > > /* Dino PCI access from main memory. */ > memory_region_init_io(&s->this_mem, OBJECT(s), &dino_chip_ops, > diff --git a/hw/hppa/trace-events b/hw/hppa/trace-events > index 4e2acb6176..f943b16c4e 100644 > --- a/hw/hppa/trace-events > +++ b/hw/hppa/trace-events > @@ -2,3 +2,8 @@ > > # pci.c > hppa_pci_iack_write(void) "" > + > +# dino.c > +dino_chip_mem_valid(uint64_t addr, uint32_t val) "access to addr 0x%"PRIx64" is %d" > +dino_chip_read(uint64_t addr, uint32_t val) "addr 0x%"PRIx64" val 0x%08x" > +dino_chip_write(uint64_t addr, uint32_t val) "addr 0x%"PRIx64" val 0x%08x" >
On 2/13/20 12:37 AM, Philippe Mathieu-Daudé wrote: > Hi Sven, Helge. > > On 12/20/19 10:15 PM, Sven Schnelle wrote: >> From: Helge Deller <deller@gmx.de> >> >> The tests of the dino chip with the Online-diagnostics CD >> ("ODE DINOTEST") now succeeds. >> Additionally add some qemu trace events. >> >> Signed-off-by: Helge Deller <deller@gmx.de> >> Signed-off-by: Sven Schnelle <svens@stackframe.org> >> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> >> --- >> MAINTAINERS | 2 +- >> hw/hppa/dino.c | 97 +++++++++++++++++++++++++++++++++++++------- >> hw/hppa/trace-events | 5 +++ >> 3 files changed, 89 insertions(+), 15 deletions(-) >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index 387879aebc..e333bc67a4 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -876,7 +876,7 @@ F: hw/*/etraxfs_*.c >> HP-PARISC Machines >> ------------------ >> -Dino >> +HP B160L >> M: Richard Henderson <rth@twiddle.net> >> R: Helge Deller <deller@gmx.de> >> S: Odd Fixes >> diff --git a/hw/hppa/dino.c b/hw/hppa/dino.c >> index ab6969b45f..9797a7f0d9 100644 >> --- a/hw/hppa/dino.c >> +++ b/hw/hppa/dino.c >> @@ -1,7 +1,7 @@ >> /* >> - * HP-PARISC Dino PCI chipset emulation. >> + * HP-PARISC Dino PCI chipset emulation, as in B160L and similiar >> machines >> * >> - * (C) 2017 by Helge Deller <deller@gmx.de> >> + * (C) 2017-2019 by Helge Deller <deller@gmx.de> >> * >> * This work is licensed under the GNU GPL license version 2 or later. >> * >> @@ -21,6 +21,7 @@ >> #include "migration/vmstate.h" >> #include "hppa_sys.h" >> #include "exec/address-spaces.h" >> +#include "trace.h" >> #define TYPE_DINO_PCI_HOST_BRIDGE "dino-pcihost" >> @@ -82,11 +83,28 @@ >> #define DINO_PCI_HOST_BRIDGE(obj) \ >> OBJECT_CHECK(DinoState, (obj), TYPE_DINO_PCI_HOST_BRIDGE) >> +#define DINO800_REGS ((DINO_TLTIM - DINO_GMASK) / 4) > > Coverity noticed (CID 1419394) this should be '(1 + (DINO_TLTIM - > DINO_GMASK) / 4)' (13 registers). > >> +static const uint32_t reg800_keep_bits[DINO800_REGS] = { >> + MAKE_64BIT_MASK(0, 1), >> + MAKE_64BIT_MASK(0, 7), >> + MAKE_64BIT_MASK(0, 7), >> + MAKE_64BIT_MASK(0, 8), >> + MAKE_64BIT_MASK(0, 7), >> + MAKE_64BIT_MASK(0, 9), >> + MAKE_64BIT_MASK(0, 32), > > Looking at the datasheet pp. 30, this register is 'Undefined'. > We should report GUEST_ERROR if it is accessed. > >> + MAKE_64BIT_MASK(0, 8), >> + MAKE_64BIT_MASK(0, 30), >> + MAKE_64BIT_MASK(0, 25), > > Still looking at the datasheet (pp. 37) PCIROR is 24-bit (not 25). > >> + MAKE_64BIT_MASK(0, 22), > > Here you are missing register 0x82c... > >> + MAKE_64BIT_MASK(0, 9), >> +}; >> + > > Altogether: > > static const uint32_t reg800_keep_bits[DINO800_REGS] = { > MAKE_64BIT_MASK(0, 1), /* GMASK */ > MAKE_64BIT_MASK(0, 7), /* PAMR */ > MAKE_64BIT_MASK(0, 7), /* PAPR */ > MAKE_64BIT_MASK(0, 8), /* DAMODE */ > MAKE_64BIT_MASK(0, 7), /* PCICMD */ > MAKE_64BIT_MASK(0, 9), /* PCISTS */ > MAKE_64BIT_MASK(0, 0), /* Undefined */ > MAKE_64BIT_MASK(0, 8), /* MLTIM */ > MAKE_64BIT_MASK(0, 30), /* BRDG_FEAT */ > MAKE_64BIT_MASK(0, 24), /* PCIROR */ > MAKE_64BIT_MASK(0, 22), /* PCIWOR */ > MAKE_64BIT_MASK(0, 0), /* Undocumented */ > MAKE_64BIT_MASK(0, 9), /* TLTIM */ > }; > >> typedef struct DinoState { >> PCIHostState parent_obj; >> /* PCI_CONFIG_ADDR is parent_obj.config_reg, via >> pci_host_conf_be_ops, >> so that we can map PCI_CONFIG_DATA to pci_host_data_be_ops. */ >> + uint32_t config_reg_dino; /* keep original copy, including 2 >> lowest bits */ >> uint32_t iar0; >> uint32_t iar1; >> @@ -94,8 +112,12 @@ typedef struct DinoState { >> uint32_t ipr; >> uint32_t icr; >> uint32_t ilr; >> + uint32_t io_fbb_en; >> uint32_t io_addr_en; >> uint32_t io_control; >> + uint32_t toc_addr; >> + >> + uint32_t reg800[DINO800_REGS]; >> MemoryRegion this_mem; >> MemoryRegion pci_mem; >> @@ -106,8 +128,6 @@ typedef struct DinoState { >> MemoryRegion bm_ram_alias; >> MemoryRegion bm_pci_alias; >> MemoryRegion bm_cpu_alias; >> - >> - MemoryRegion cpu0_eir_mem; >> } DinoState; >> /* >> @@ -122,6 +142,8 @@ static void gsc_to_pci_forwarding(DinoState *s) >> tmp = extract32(s->io_control, 7, 2); >> enabled = (tmp == 0x01); >> io_addr_en = s->io_addr_en; >> + /* Mask out first (=firmware) and last (=Dino) areas. */ >> + io_addr_en &= ~(BIT(31) | BIT(0)); >> memory_region_transaction_begin(); >> for (i = 1; i < 31; i++) { >> @@ -142,6 +164,8 @@ static bool dino_chip_mem_valid(void *opaque, >> hwaddr addr, >> unsigned size, bool is_write, >> MemTxAttrs attrs) >> { >> + bool ret = false; >> + >> switch (addr) { >> case DINO_IAR0: >> case DINO_IAR1: >> @@ -152,16 +176,22 @@ static bool dino_chip_mem_valid(void *opaque, >> hwaddr addr, >> case DINO_ICR: >> case DINO_ILR: >> case DINO_IO_CONTROL: >> + case DINO_IO_FBB_EN: >> case DINO_IO_ADDR_EN: >> case DINO_PCI_IO_DATA: >> - return true; >> + case DINO_TOC_ADDR: >> + case DINO_GMASK ... DINO_TLTIM: >> + ret = true; >> + break; >> case DINO_PCI_IO_DATA + 2: >> - return size <= 2; >> + ret = (size <= 2); >> + break; >> case DINO_PCI_IO_DATA + 1: >> case DINO_PCI_IO_DATA + 3: >> - return size == 1; >> + ret = (size == 1); >> } >> - return false; >> + trace_dino_chip_mem_valid(addr, ret); >> + return ret; >> } >> static MemTxResult dino_chip_read_with_attrs(void *opaque, hwaddr >> addr, >> @@ -194,6 +224,9 @@ static MemTxResult dino_chip_read_with_attrs(void >> *opaque, hwaddr addr, >> } >> break; >> + case DINO_IO_FBB_EN: >> + val = s->io_fbb_en; >> + break; >> case DINO_IO_ADDR_EN: >> val = s->io_addr_en; >> break; >> @@ -227,12 +260,28 @@ static MemTxResult >> dino_chip_read_with_attrs(void *opaque, hwaddr addr, >> case DINO_IRR1: >> val = s->ilr & s->imr & s->icr; >> break; >> + case DINO_TOC_ADDR: >> + val = s->toc_addr; >> + break; >> + case DINO_GMASK ... DINO_TLTIM: >> + val = s->reg800[(addr - DINO_GMASK) / 4]; >> + if (addr == DINO_PAMR) { >> + val &= ~0x01; /* LSB is hardwired to 0 */ >> + } >> + if (addr == DINO_MLTIM) { >> + val &= ~0x07; /* 3 LSB are hardwired to 0 */ >> + } >> + if (addr == DINO_BRDG_FEAT) { >> + val &= ~(0x10710E0ul | 8); /* bits 5-7, 24 & 15 reserved */ >> + } >> + break; >> default: >> /* Controlled by dino_chip_mem_valid above. */ >> g_assert_not_reached(); >> } >> + trace_dino_chip_read(addr, val); >> *data = val; >> return ret; >> } >> @@ -245,6 +294,9 @@ static MemTxResult dino_chip_write_with_attrs(void >> *opaque, hwaddr addr, >> AddressSpace *io; >> MemTxResult ret; >> uint16_t ioaddr; >> + int i; >> + >> + trace_dino_chip_write(addr, val); >> switch (addr) { >> case DINO_IO_DATA ... DINO_PCI_IO_DATA + 3: >> @@ -266,9 +318,11 @@ static MemTxResult >> dino_chip_write_with_attrs(void *opaque, hwaddr addr, >> } >> return ret; >> + case DINO_IO_FBB_EN: >> + s->io_fbb_en = val & 0x03; >> + break; >> case DINO_IO_ADDR_EN: >> - /* Never allow first (=firmware) and last (=Dino) areas. */ >> - s->io_addr_en = val & 0x7ffffffe; >> + s->io_addr_en = val; >> gsc_to_pci_forwarding(s); >> break; >> case DINO_IO_CONTROL: >> @@ -292,6 +346,10 @@ static MemTxResult >> dino_chip_write_with_attrs(void *opaque, hwaddr addr, >> /* Any write to IPR clears the register. */ >> s->ipr = 0; >> break; >> + case DINO_TOC_ADDR: >> + /* IO_COMMAND of CPU with client_id bits */ >> + s->toc_addr = 0xFFFA0030 | (val & 0x1e000); >> + break; >> case DINO_ILR: >> case DINO_IRR0: >> @@ -299,6 +357,12 @@ static MemTxResult >> dino_chip_write_with_attrs(void *opaque, hwaddr addr, >> /* These registers are read-only. */ >> break; >> + case DINO_GMASK ... DINO_TLTIM: >> + i = (addr - DINO_GMASK) / 4; >> + val &= reg800_keep_bits[i]; > > Due to the missing register, Coverity reports: > >>>> CID 1419394: Memory - illegal accesses (OVERRUN) >>>> Overrunning array "reg800_keep_bits" of 12 4-byte elements at > element index 12 (byte offset 48) using index "i" (which evaluates to 12). > >> + s->reg800[i] = val; >> + break; > + >> default: >> /* Controlled by dino_chip_mem_valid above. */ >> g_assert_not_reached(); Easy reproducer: $ echo writeb 0xfff80830 0x69 \ | hppa-softmmu/qemu-system-hppa -S -accel qtest -qtest stdio -display none [I 1581634452.654113] OPENED [R +4.105415] writeb 0xfff80830 0x69 qemu/hw/hppa/dino.c:362:16: runtime error: index 12 out of bounds for type 'const uint32_t [12]' SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior qemu/hw/hppa/dino.c:362:16 in ================================================================= ==29607==ERROR: AddressSanitizer: global-buffer-overflow on address 0x5577dae32f30 at pc 0x5577d93f2463 bp 0x7ffd97ea11b0 sp 0x7ffd97ea11a8 READ of size 4 at 0x5577dae32f30 thread T0 #0 0x5577d93f2462 in dino_chip_write_with_attrs qemu/hw/hppa/dino.c:362:16 #1 0x5577d9025664 in memory_region_write_with_attrs_accessor qemu/memory.c:503:12 #2 0x5577d9024920 in access_with_adjusted_size qemu/memory.c:539:18 #3 0x5577d9023608 in memory_region_dispatch_write qemu/memory.c:1482:13 #4 0x5577d8e3177a in flatview_write_continue qemu/exec.c:3166:23 #5 0x5577d8e20357 in flatview_write qemu/exec.c:3206:14 #6 0x5577d8e1fef4 in address_space_write qemu/exec.c:3296:18 #7 0x5577d8e20693 in address_space_rw qemu/exec.c:3306:16 #8 0x5577d9011595 in qtest_process_command qemu/qtest.c:432:13 #9 0x5577d900d19f in qtest_process_inbuf qemu/qtest.c:705:9 #10 0x5577d900ca22 in qtest_read qemu/qtest.c:717:5 #11 0x5577da8c4254 in qemu_chr_be_write_impl qemu/chardev/char.c:183:9 #12 0x5577da8c430c in qemu_chr_be_write qemu/chardev/char.c:195:9 #13 0x5577da8cf587 in fd_chr_read qemu/chardev/char-fd.c:68:9 #14 0x5577da9836cd in qio_channel_fd_source_dispatch qemu/io/channel-watch.c:84:12 #15 0x7faf44509ecc in g_main_context_dispatch (/lib64/libglib-2.0.so.0+0x4fecc) #16 0x5577dab75f96 in glib_pollfds_poll qemu/util/main-loop.c:219:9 #17 0x5577dab74797 in os_host_main_loop_wait qemu/util/main-loop.c:242:5 #18 0x5577dab7435a in main_loop_wait qemu/util/main-loop.c:518:11 #19 0x5577d9514eb3 in main_loop qemu/vl.c:1682:9 #20 0x5577d950699d in main qemu/vl.c:4450:5 #21 0x7faf41a87f42 in __libc_start_main (/lib64/libc.so.6+0x23f42) #22 0x5577d8cd4d4d in _start (qemu/build/sanitizer/hppa-softmmu/qemu-system-hppa+0x1256d4d) 0x5577dae32f30 is located 0 bytes to the right of global variable 'reg800_keep_bits' defined in 'qemu/hw/hppa/dino.c:87:23' (0x5577dae32f00) of size 48 SUMMARY: AddressSanitizer: global-buffer-overflow qemu/hw/hppa/dino.c:362:16 in dino_chip_write_with_attrs Shadow bytes around the buggy address: 0x0aaf7b5be590: 00 f9 f9 f9 f9 f9 f9 f9 00 02 f9 f9 f9 f9 f9 f9 0x0aaf7b5be5a0: 07 f9 f9 f9 f9 f9 f9 f9 07 f9 f9 f9 f9 f9 f9 f9 0x0aaf7b5be5b0: 07 f9 f9 f9 f9 f9 f9 f9 00 00 00 00 00 00 00 00 0x0aaf7b5be5c0: 00 00 00 02 f9 f9 f9 f9 00 00 00 00 00 00 00 00 0x0aaf7b5be5d0: 00 00 00 00 00 00 00 00 00 00 00 03 f9 f9 f9 f9 =>0x0aaf7b5be5e0: 00 00 00 00 00 00[f9]f9 f9 f9 f9 f9 00 00 00 00 0x0aaf7b5be5f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0aaf7b5be600: 00 00 01 f9 f9 f9 f9 f9 00 00 00 00 07 f9 f9 f9 0x0aaf7b5be610: f9 f9 f9 f9 00 00 00 00 00 00 00 00 00 00 00 00 0x0aaf7b5be620: 00 00 00 05 f9 f9 f9 f9 00 00 00 00 07 f9 f9 f9 0x0aaf7b5be630: f9 f9 f9 f9 00 00 f9 f9 f9 f9 f9 f9 07 f9 f9 f9 Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb Shadow gap: cc ==29607==ABORTING
diff --git a/MAINTAINERS b/MAINTAINERS index 387879aebc..e333bc67a4 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -876,7 +876,7 @@ F: hw/*/etraxfs_*.c HP-PARISC Machines ------------------ -Dino +HP B160L M: Richard Henderson <rth@twiddle.net> R: Helge Deller <deller@gmx.de> S: Odd Fixes diff --git a/hw/hppa/dino.c b/hw/hppa/dino.c index ab6969b45f..9797a7f0d9 100644 --- a/hw/hppa/dino.c +++ b/hw/hppa/dino.c @@ -1,7 +1,7 @@ /* - * HP-PARISC Dino PCI chipset emulation. + * HP-PARISC Dino PCI chipset emulation, as in B160L and similiar machines * - * (C) 2017 by Helge Deller <deller@gmx.de> + * (C) 2017-2019 by Helge Deller <deller@gmx.de> * * This work is licensed under the GNU GPL license version 2 or later. * @@ -21,6 +21,7 @@ #include "migration/vmstate.h" #include "hppa_sys.h" #include "exec/address-spaces.h" +#include "trace.h" #define TYPE_DINO_PCI_HOST_BRIDGE "dino-pcihost" @@ -82,11 +83,28 @@ #define DINO_PCI_HOST_BRIDGE(obj) \ OBJECT_CHECK(DinoState, (obj), TYPE_DINO_PCI_HOST_BRIDGE) +#define DINO800_REGS ((DINO_TLTIM - DINO_GMASK) / 4) +static const uint32_t reg800_keep_bits[DINO800_REGS] = { + MAKE_64BIT_MASK(0, 1), + MAKE_64BIT_MASK(0, 7), + MAKE_64BIT_MASK(0, 7), + MAKE_64BIT_MASK(0, 8), + MAKE_64BIT_MASK(0, 7), + MAKE_64BIT_MASK(0, 9), + MAKE_64BIT_MASK(0, 32), + MAKE_64BIT_MASK(0, 8), + MAKE_64BIT_MASK(0, 30), + MAKE_64BIT_MASK(0, 25), + MAKE_64BIT_MASK(0, 22), + MAKE_64BIT_MASK(0, 9), +}; + typedef struct DinoState { PCIHostState parent_obj; /* PCI_CONFIG_ADDR is parent_obj.config_reg, via pci_host_conf_be_ops, so that we can map PCI_CONFIG_DATA to pci_host_data_be_ops. */ + uint32_t config_reg_dino; /* keep original copy, including 2 lowest bits */ uint32_t iar0; uint32_t iar1; @@ -94,8 +112,12 @@ typedef struct DinoState { uint32_t ipr; uint32_t icr; uint32_t ilr; + uint32_t io_fbb_en; uint32_t io_addr_en; uint32_t io_control; + uint32_t toc_addr; + + uint32_t reg800[DINO800_REGS]; MemoryRegion this_mem; MemoryRegion pci_mem; @@ -106,8 +128,6 @@ typedef struct DinoState { MemoryRegion bm_ram_alias; MemoryRegion bm_pci_alias; MemoryRegion bm_cpu_alias; - - MemoryRegion cpu0_eir_mem; } DinoState; /* @@ -122,6 +142,8 @@ static void gsc_to_pci_forwarding(DinoState *s) tmp = extract32(s->io_control, 7, 2); enabled = (tmp == 0x01); io_addr_en = s->io_addr_en; + /* Mask out first (=firmware) and last (=Dino) areas. */ + io_addr_en &= ~(BIT(31) | BIT(0)); memory_region_transaction_begin(); for (i = 1; i < 31; i++) { @@ -142,6 +164,8 @@ static bool dino_chip_mem_valid(void *opaque, hwaddr addr, unsigned size, bool is_write, MemTxAttrs attrs) { + bool ret = false; + switch (addr) { case DINO_IAR0: case DINO_IAR1: @@ -152,16 +176,22 @@ static bool dino_chip_mem_valid(void *opaque, hwaddr addr, case DINO_ICR: case DINO_ILR: case DINO_IO_CONTROL: + case DINO_IO_FBB_EN: case DINO_IO_ADDR_EN: case DINO_PCI_IO_DATA: - return true; + case DINO_TOC_ADDR: + case DINO_GMASK ... DINO_TLTIM: + ret = true; + break; case DINO_PCI_IO_DATA + 2: - return size <= 2; + ret = (size <= 2); + break; case DINO_PCI_IO_DATA + 1: case DINO_PCI_IO_DATA + 3: - return size == 1; + ret = (size == 1); } - return false; + trace_dino_chip_mem_valid(addr, ret); + return ret; } static MemTxResult dino_chip_read_with_attrs(void *opaque, hwaddr addr, @@ -194,6 +224,9 @@ static MemTxResult dino_chip_read_with_attrs(void *opaque, hwaddr addr, } break; + case DINO_IO_FBB_EN: + val = s->io_fbb_en; + break; case DINO_IO_ADDR_EN: val = s->io_addr_en; break; @@ -227,12 +260,28 @@ static MemTxResult dino_chip_read_with_attrs(void *opaque, hwaddr addr, case DINO_IRR1: val = s->ilr & s->imr & s->icr; break; + case DINO_TOC_ADDR: + val = s->toc_addr; + break; + case DINO_GMASK ... DINO_TLTIM: + val = s->reg800[(addr - DINO_GMASK) / 4]; + if (addr == DINO_PAMR) { + val &= ~0x01; /* LSB is hardwired to 0 */ + } + if (addr == DINO_MLTIM) { + val &= ~0x07; /* 3 LSB are hardwired to 0 */ + } + if (addr == DINO_BRDG_FEAT) { + val &= ~(0x10710E0ul | 8); /* bits 5-7, 24 & 15 reserved */ + } + break; default: /* Controlled by dino_chip_mem_valid above. */ g_assert_not_reached(); } + trace_dino_chip_read(addr, val); *data = val; return ret; } @@ -245,6 +294,9 @@ static MemTxResult dino_chip_write_with_attrs(void *opaque, hwaddr addr, AddressSpace *io; MemTxResult ret; uint16_t ioaddr; + int i; + + trace_dino_chip_write(addr, val); switch (addr) { case DINO_IO_DATA ... DINO_PCI_IO_DATA + 3: @@ -266,9 +318,11 @@ static MemTxResult dino_chip_write_with_attrs(void *opaque, hwaddr addr, } return ret; + case DINO_IO_FBB_EN: + s->io_fbb_en = val & 0x03; + break; case DINO_IO_ADDR_EN: - /* Never allow first (=firmware) and last (=Dino) areas. */ - s->io_addr_en = val & 0x7ffffffe; + s->io_addr_en = val; gsc_to_pci_forwarding(s); break; case DINO_IO_CONTROL: @@ -292,6 +346,10 @@ static MemTxResult dino_chip_write_with_attrs(void *opaque, hwaddr addr, /* Any write to IPR clears the register. */ s->ipr = 0; break; + case DINO_TOC_ADDR: + /* IO_COMMAND of CPU with client_id bits */ + s->toc_addr = 0xFFFA0030 | (val & 0x1e000); + break; case DINO_ILR: case DINO_IRR0: @@ -299,6 +357,12 @@ static MemTxResult dino_chip_write_with_attrs(void *opaque, hwaddr addr, /* These registers are read-only. */ break; + case DINO_GMASK ... DINO_TLTIM: + i = (addr - DINO_GMASK) / 4; + val &= reg800_keep_bits[i]; + s->reg800[i] = val; + break; + default: /* Controlled by dino_chip_mem_valid above. */ g_assert_not_reached(); @@ -323,7 +387,7 @@ static const MemoryRegionOps dino_chip_ops = { static const VMStateDescription vmstate_dino = { .name = "Dino", - .version_id = 1, + .version_id = 2, .minimum_version_id = 1, .fields = (VMStateField[]) { VMSTATE_UINT32(iar0, DinoState), @@ -332,13 +396,14 @@ static const VMStateDescription vmstate_dino = { VMSTATE_UINT32(ipr, DinoState), VMSTATE_UINT32(icr, DinoState), VMSTATE_UINT32(ilr, DinoState), + VMSTATE_UINT32(io_fbb_en, DinoState), VMSTATE_UINT32(io_addr_en, DinoState), VMSTATE_UINT32(io_control, DinoState), + VMSTATE_UINT32(toc_addr, DinoState), VMSTATE_END_OF_LIST() } }; - /* Unlike pci_config_data_le_ops, no check of high bit set in config_reg. */ static uint64_t dino_config_data_read(void *opaque, hwaddr addr, unsigned len) @@ -362,14 +427,16 @@ static const MemoryRegionOps dino_config_data_ops = { static uint64_t dino_config_addr_read(void *opaque, hwaddr addr, unsigned len) { - PCIHostState *s = opaque; - return s->config_reg; + DinoState *s = opaque; + return s->config_reg_dino; } static void dino_config_addr_write(void *opaque, hwaddr addr, uint64_t val, unsigned len) { PCIHostState *s = opaque; + DinoState *ds = opaque; + ds->config_reg_dino = val; /* keep a copy of original value */ s->config_reg = val & ~3U; } @@ -453,6 +520,8 @@ PCIBus *dino_init(MemoryRegion *addr_space, dev = qdev_create(NULL, TYPE_DINO_PCI_HOST_BRIDGE); s = DINO_PCI_HOST_BRIDGE(dev); + s->iar0 = s->iar1 = CPU_HPA + 3; + s->toc_addr = 0xFFFA0030; /* IO_COMMAND of CPU */ /* Dino PCI access from main memory. */ memory_region_init_io(&s->this_mem, OBJECT(s), &dino_chip_ops, diff --git a/hw/hppa/trace-events b/hw/hppa/trace-events index 4e2acb6176..f943b16c4e 100644 --- a/hw/hppa/trace-events +++ b/hw/hppa/trace-events @@ -2,3 +2,8 @@ # pci.c hppa_pci_iack_write(void) "" + +# dino.c +dino_chip_mem_valid(uint64_t addr, uint32_t val) "access to addr 0x%"PRIx64" is %d" +dino_chip_read(uint64_t addr, uint32_t val) "addr 0x%"PRIx64" val 0x%08x" +dino_chip_write(uint64_t addr, uint32_t val) "addr 0x%"PRIx64" val 0x%08x"