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 ==
