Re: [PATCH v5 1/6] hw/hppa/dino.c: Improve emulation of Dino PCI chip
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 >> >> 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 >> Signed-off-by: Sven Schnelle >> Reviewed-by: Philippe Mathieu-Daudé >> --- >> 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 >> R: Helge Deller >> 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 >> + * (C) 2017-2019 by Helge Deller >> * >> * 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
Re: [PATCH v5 1/6] hw/hppa/dino.c: Improve emulation of Dino PCI chip
Hi Sven, Helge. On 12/20/19 10:15 PM, Sven Schnelle wrote: From: Helge Deller 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 Signed-off-by: Sven Schnelle Reviewed-by: Philippe Mathieu-Daudé --- 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 R: Helge Deller 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 + * (C) 2017-2019 by Helge Deller * * 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 +
[PATCH v5 1/6] hw/hppa/dino.c: Improve emulation of Dino PCI chip
From: Helge Deller 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 Signed-off-by: Sven Schnelle Reviewed-by: Philippe Mathieu-Daudé --- 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 R: Helge Deller 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 + * (C) 2017-2019 by Helge Deller * * 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 ==