* [Qemu-devel] [PATCH 00/10] hw/pci-host: Clean the GT64120 north bridge
@ 2019-06-24 22:28 Philippe Mathieu-Daudé
2019-06-24 22:28 ` [Qemu-devel] [PATCH 01/10] hw/mips/gt64xxx_pci: Fix multiline comment syntax Philippe Mathieu-Daudé
` (10 more replies)
0 siblings, 11 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-06-24 22:28 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Thomas Huth, Aleksandar Rikalo,
Philippe Mathieu-Daudé,
Hervé Poussineau, Artyom Tarasenko, Aleksandar Markovic,
Paolo Bonzini, Aurelien Jarno
Hi,
This series clean the gt64120 device.
It is no more target-dependent, and tracing is improved.
Regards,
Phil.
Based-on: 20190624220056.25861-1-f4bug@amsat.org
https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg05304.html
Philippe Mathieu-Daudé (10):
hw/mips/gt64xxx_pci: Fix multiline comment syntax
hw/mips/gt64xxx_pci: Fix 'tabs' coding style issues
hw/mips/gt64xxx_pci: Fix 'braces' coding style issues
hw/mips/gt64xxx_pci: Fix 'spaces' coding style issues
hw/mips/gt64xxx_pci: Use qemu_log_mask() instead of debug printf()
hw/mips/gt64xxx_pci: Convert debug printf()s to trace events
hw/mips/gt64xxx_pci: Align the pci0-mem size
hw/mips/gt64xxx_pci: Add a 'cpu_big_endian' qdev property
hw/mips/gt64xxx_pci: Move it to hw/pci-host/
hw/pci-host/gt64120: Clean the decoded address space
Makefile.objs | 1 +
include/hw/mips/mips.h | 2 +-
hw/mips/mips_malta.c | 8 +-
hw/{mips/gt64xxx_pci.c => pci-host/gt64120.c} | 542 ++++++++++--------
MAINTAINERS | 2 +-
hw/mips/Makefile.objs | 2 +-
hw/mips/trace-events | 0
hw/pci-host/Makefile.objs | 2 +-
hw/pci-host/trace-events | 5 +
9 files changed, 307 insertions(+), 257 deletions(-)
rename hw/{mips/gt64xxx_pci.c => pci-host/gt64120.c} (68%)
create mode 100644 hw/mips/trace-events
--
2.19.1
^ permalink raw reply [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH 01/10] hw/mips/gt64xxx_pci: Fix multiline comment syntax
2019-06-24 22:28 [Qemu-devel] [PATCH 00/10] hw/pci-host: Clean the GT64120 north bridge Philippe Mathieu-Daudé
@ 2019-06-24 22:28 ` Philippe Mathieu-Daudé
2019-06-25 0:20 ` Aleksandar Markovic
2019-06-24 22:28 ` [Qemu-devel] [PATCH 02/10] hw/mips/gt64xxx_pci: Fix 'tabs' coding style issues Philippe Mathieu-Daudé
` (9 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-06-24 22:28 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Thomas Huth, Aleksandar Rikalo,
Philippe Mathieu-Daudé,
Hervé Poussineau, Artyom Tarasenko, Aleksandar Markovic,
Paolo Bonzini, Aurelien Jarno
Since commit 8c06fbdf36b checkpatch.pl enforce a new multiline
comment syntax. Since we'll move this code around, fix its style
first.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/mips/gt64xxx_pci.c | 64 +++++++++++++++++++++++--------------------
1 file changed, 35 insertions(+), 29 deletions(-)
diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
index f707e59c7a..c0924646b5 100644
--- a/hw/mips/gt64xxx_pci.c
+++ b/hw/mips/gt64xxx_pci.c
@@ -248,10 +248,11 @@ typedef struct GT64120State {
} GT64120State;
/* Adjust range to avoid touching space which isn't mappable via PCI */
-/* XXX: Hardcoded values for Malta: 0x1e000000 - 0x1f100000
- 0x1fc00000 - 0x1fd00000 */
-static void check_reserved_space (hwaddr *start,
- hwaddr *length)
+/*
+ * XXX: Hardcoded values for Malta: 0x1e000000 - 0x1f100000
+ * 0x1fc00000 - 0x1fd00000
+ */
+static void check_reserved_space(hwaddr *start, hwaddr *length)
{
hwaddr begin = *start;
hwaddr end = *start + *length;
@@ -650,8 +651,10 @@ static void gt64120_writel (void *opaque, hwaddr addr,
case GT_SDRAM_B1:
case GT_SDRAM_B2:
case GT_SDRAM_B3:
- /* We don't simulate electrical parameters of the SDRAM.
- Accept, but ignore the values. */
+ /*
+ * We don't simulate electrical parameters of the SDRAM.
+ * Accept, but ignore the values.
+ */
s->regs[saddr] = val;
break;
@@ -674,8 +677,10 @@ static uint64_t gt64120_readl (void *opaque,
/* CPU Configuration */
case GT_MULTI:
- /* Only one GT64xxx is present on the CPU bus, return
- the initial value */
+ /*
+ * Only one GT64xxx is present on the CPU bus, return
+ * the initial value.
+ */
val = s->regs[saddr];
break;
@@ -685,17 +690,18 @@ static uint64_t gt64120_readl (void *opaque,
case GT_CPUERR_DATALO:
case GT_CPUERR_DATAHI:
case GT_CPUERR_PARITY:
- /* Emulated memory has no error, always return the initial
- values */
+ /* Emulated memory has no error, always return the initial values. */
val = s->regs[saddr];
break;
/* CPU Sync Barrier */
case GT_PCI0SYNC:
case GT_PCI1SYNC:
- /* Reading those register should empty all FIFO on the PCI
- bus, which are not emulated. The return value should be
- a random value that should be ignored. */
+ /*
+ * Reading those register should empty all FIFO on the PCI
+ * bus, which are not emulated. The return value should be
+ * a random value that should be ignored.
+ */
val = 0xc000ffee;
break;
@@ -705,8 +711,7 @@ static uint64_t gt64120_readl (void *opaque,
case GT_ECC_MEM:
case GT_ECC_CALC:
case GT_ECC_ERRADDR:
- /* Emulated memory has no error, always return the initial
- values */
+ /* Emulated memory has no error, always return the initial values. */
val = s->regs[saddr];
break;
@@ -785,8 +790,10 @@ static uint64_t gt64120_readl (void *opaque,
case GT_SDRAM_B1:
case GT_SDRAM_B2:
case GT_SDRAM_B3:
- /* We don't simulate electrical parameters of the SDRAM.
- Just return the last written value. */
+ /*
+ * We don't simulate electrical parameters of the SDRAM.
+ * Just return the last written value.
+ */
val = s->regs[saddr];
break;
@@ -949,20 +956,20 @@ static int gt64120_pci_map_irq(PCIDevice *pci_dev, int irq_num)
slot = (pci_dev->devfn >> 3);
switch (slot) {
- /* PIIX4 USB */
- case 10:
+ /* PIIX4 USB */
+ case 10:
return 3;
- /* AMD 79C973 Ethernet */
- case 11:
+ /* AMD 79C973 Ethernet */
+ case 11:
return 1;
- /* Crystal 4281 Sound */
- case 12:
+ /* Crystal 4281 Sound */
+ case 12:
return 2;
- /* PCI slot 1 to 4 */
- case 18 ... 21:
+ /* PCI slot 1 to 4 */
+ case 18 ... 21:
return ((slot - 18) + irq_num) & 0x03;
- /* Unknown device, don't do any translation */
- default:
+ /* Unknown device, don't do any translation */
+ default:
return irq_num;
}
}
@@ -980,8 +987,7 @@ static void gt64120_pci_set_irq(void *opaque, int irq_num, int level)
/* XXX: optimize */
pic_irq = piix4_dev->config[0x60 + irq_num];
if (pic_irq < 16) {
- /* The pic level is the logical OR of all the PCI irqs mapped
- to it */
+ /* The pic level is the logical OR of all the PCI irqs mapped to it. */
pic_level = 0;
for (i = 0; i < 4; i++) {
if (pic_irq == piix4_dev->config[0x60 + i])
--
2.19.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH 02/10] hw/mips/gt64xxx_pci: Fix 'tabs' coding style issues
2019-06-24 22:28 [Qemu-devel] [PATCH 00/10] hw/pci-host: Clean the GT64120 north bridge Philippe Mathieu-Daudé
2019-06-24 22:28 ` [Qemu-devel] [PATCH 01/10] hw/mips/gt64xxx_pci: Fix multiline comment syntax Philippe Mathieu-Daudé
@ 2019-06-24 22:28 ` Philippe Mathieu-Daudé
2019-06-25 0:21 ` Aleksandar Markovic
2019-06-24 22:28 ` [Qemu-devel] [PATCH 03/10] hw/mips/gt64xxx_pci: Fix 'braces' " Philippe Mathieu-Daudé
` (8 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-06-24 22:28 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Thomas Huth, Aleksandar Rikalo,
Philippe Mathieu-Daudé,
Hervé Poussineau, Artyom Tarasenko, Aleksandar Markovic,
Paolo Bonzini, Aurelien Jarno
Since we'll move this code around, fix its style first:
ERROR: code indent should never use tabs
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/mips/gt64xxx_pci.c | 312 +++++++++++++++++++++---------------------
1 file changed, 156 insertions(+), 156 deletions(-)
diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
index c0924646b5..bbd719f091 100644
--- a/hw/mips/gt64xxx_pci.c
+++ b/hw/mips/gt64xxx_pci.c
@@ -38,192 +38,192 @@
#define DPRINTF(fmt, ...)
#endif
-#define GT_REGS (0x1000 >> 2)
+#define GT_REGS (0x1000 >> 2)
/* CPU Configuration */
-#define GT_CPU (0x000 >> 2)
-#define GT_MULTI (0x120 >> 2)
+#define GT_CPU (0x000 >> 2)
+#define GT_MULTI (0x120 >> 2)
/* CPU Address Decode */
-#define GT_SCS10LD (0x008 >> 2)
-#define GT_SCS10HD (0x010 >> 2)
-#define GT_SCS32LD (0x018 >> 2)
-#define GT_SCS32HD (0x020 >> 2)
-#define GT_CS20LD (0x028 >> 2)
-#define GT_CS20HD (0x030 >> 2)
-#define GT_CS3BOOTLD (0x038 >> 2)
-#define GT_CS3BOOTHD (0x040 >> 2)
-#define GT_PCI0IOLD (0x048 >> 2)
-#define GT_PCI0IOHD (0x050 >> 2)
-#define GT_PCI0M0LD (0x058 >> 2)
-#define GT_PCI0M0HD (0x060 >> 2)
-#define GT_PCI0M1LD (0x080 >> 2)
-#define GT_PCI0M1HD (0x088 >> 2)
-#define GT_PCI1IOLD (0x090 >> 2)
-#define GT_PCI1IOHD (0x098 >> 2)
-#define GT_PCI1M0LD (0x0a0 >> 2)
-#define GT_PCI1M0HD (0x0a8 >> 2)
-#define GT_PCI1M1LD (0x0b0 >> 2)
-#define GT_PCI1M1HD (0x0b8 >> 2)
-#define GT_ISD (0x068 >> 2)
-
-#define GT_SCS10AR (0x0d0 >> 2)
-#define GT_SCS32AR (0x0d8 >> 2)
-#define GT_CS20R (0x0e0 >> 2)
-#define GT_CS3BOOTR (0x0e8 >> 2)
-
-#define GT_PCI0IOREMAP (0x0f0 >> 2)
-#define GT_PCI0M0REMAP (0x0f8 >> 2)
-#define GT_PCI0M1REMAP (0x100 >> 2)
-#define GT_PCI1IOREMAP (0x108 >> 2)
-#define GT_PCI1M0REMAP (0x110 >> 2)
-#define GT_PCI1M1REMAP (0x118 >> 2)
+#define GT_SCS10LD (0x008 >> 2)
+#define GT_SCS10HD (0x010 >> 2)
+#define GT_SCS32LD (0x018 >> 2)
+#define GT_SCS32HD (0x020 >> 2)
+#define GT_CS20LD (0x028 >> 2)
+#define GT_CS20HD (0x030 >> 2)
+#define GT_CS3BOOTLD (0x038 >> 2)
+#define GT_CS3BOOTHD (0x040 >> 2)
+#define GT_PCI0IOLD (0x048 >> 2)
+#define GT_PCI0IOHD (0x050 >> 2)
+#define GT_PCI0M0LD (0x058 >> 2)
+#define GT_PCI0M0HD (0x060 >> 2)
+#define GT_PCI0M1LD (0x080 >> 2)
+#define GT_PCI0M1HD (0x088 >> 2)
+#define GT_PCI1IOLD (0x090 >> 2)
+#define GT_PCI1IOHD (0x098 >> 2)
+#define GT_PCI1M0LD (0x0a0 >> 2)
+#define GT_PCI1M0HD (0x0a8 >> 2)
+#define GT_PCI1M1LD (0x0b0 >> 2)
+#define GT_PCI1M1HD (0x0b8 >> 2)
+#define GT_ISD (0x068 >> 2)
+
+#define GT_SCS10AR (0x0d0 >> 2)
+#define GT_SCS32AR (0x0d8 >> 2)
+#define GT_CS20R (0x0e0 >> 2)
+#define GT_CS3BOOTR (0x0e8 >> 2)
+
+#define GT_PCI0IOREMAP (0x0f0 >> 2)
+#define GT_PCI0M0REMAP (0x0f8 >> 2)
+#define GT_PCI0M1REMAP (0x100 >> 2)
+#define GT_PCI1IOREMAP (0x108 >> 2)
+#define GT_PCI1M0REMAP (0x110 >> 2)
+#define GT_PCI1M1REMAP (0x118 >> 2)
/* CPU Error Report */
-#define GT_CPUERR_ADDRLO (0x070 >> 2)
-#define GT_CPUERR_ADDRHI (0x078 >> 2)
-#define GT_CPUERR_DATALO (0x128 >> 2) /* GT-64120A only */
-#define GT_CPUERR_DATAHI (0x130 >> 2) /* GT-64120A only */
-#define GT_CPUERR_PARITY (0x138 >> 2) /* GT-64120A only */
+#define GT_CPUERR_ADDRLO (0x070 >> 2)
+#define GT_CPUERR_ADDRHI (0x078 >> 2)
+#define GT_CPUERR_DATALO (0x128 >> 2) /* GT-64120A only */
+#define GT_CPUERR_DATAHI (0x130 >> 2) /* GT-64120A only */
+#define GT_CPUERR_PARITY (0x138 >> 2) /* GT-64120A only */
/* CPU Sync Barrier */
-#define GT_PCI0SYNC (0x0c0 >> 2)
-#define GT_PCI1SYNC (0x0c8 >> 2)
+#define GT_PCI0SYNC (0x0c0 >> 2)
+#define GT_PCI1SYNC (0x0c8 >> 2)
/* SDRAM and Device Address Decode */
-#define GT_SCS0LD (0x400 >> 2)
-#define GT_SCS0HD (0x404 >> 2)
-#define GT_SCS1LD (0x408 >> 2)
-#define GT_SCS1HD (0x40c >> 2)
-#define GT_SCS2LD (0x410 >> 2)
-#define GT_SCS2HD (0x414 >> 2)
-#define GT_SCS3LD (0x418 >> 2)
-#define GT_SCS3HD (0x41c >> 2)
-#define GT_CS0LD (0x420 >> 2)
-#define GT_CS0HD (0x424 >> 2)
-#define GT_CS1LD (0x428 >> 2)
-#define GT_CS1HD (0x42c >> 2)
-#define GT_CS2LD (0x430 >> 2)
-#define GT_CS2HD (0x434 >> 2)
-#define GT_CS3LD (0x438 >> 2)
-#define GT_CS3HD (0x43c >> 2)
-#define GT_BOOTLD (0x440 >> 2)
-#define GT_BOOTHD (0x444 >> 2)
-#define GT_ADERR (0x470 >> 2)
+#define GT_SCS0LD (0x400 >> 2)
+#define GT_SCS0HD (0x404 >> 2)
+#define GT_SCS1LD (0x408 >> 2)
+#define GT_SCS1HD (0x40c >> 2)
+#define GT_SCS2LD (0x410 >> 2)
+#define GT_SCS2HD (0x414 >> 2)
+#define GT_SCS3LD (0x418 >> 2)
+#define GT_SCS3HD (0x41c >> 2)
+#define GT_CS0LD (0x420 >> 2)
+#define GT_CS0HD (0x424 >> 2)
+#define GT_CS1LD (0x428 >> 2)
+#define GT_CS1HD (0x42c >> 2)
+#define GT_CS2LD (0x430 >> 2)
+#define GT_CS2HD (0x434 >> 2)
+#define GT_CS3LD (0x438 >> 2)
+#define GT_CS3HD (0x43c >> 2)
+#define GT_BOOTLD (0x440 >> 2)
+#define GT_BOOTHD (0x444 >> 2)
+#define GT_ADERR (0x470 >> 2)
/* SDRAM Configuration */
-#define GT_SDRAM_CFG (0x448 >> 2)
-#define GT_SDRAM_OPMODE (0x474 >> 2)
-#define GT_SDRAM_BM (0x478 >> 2)
-#define GT_SDRAM_ADDRDECODE (0x47c >> 2)
+#define GT_SDRAM_CFG (0x448 >> 2)
+#define GT_SDRAM_OPMODE (0x474 >> 2)
+#define GT_SDRAM_BM (0x478 >> 2)
+#define GT_SDRAM_ADDRDECODE (0x47c >> 2)
/* SDRAM Parameters */
-#define GT_SDRAM_B0 (0x44c >> 2)
-#define GT_SDRAM_B1 (0x450 >> 2)
-#define GT_SDRAM_B2 (0x454 >> 2)
-#define GT_SDRAM_B3 (0x458 >> 2)
+#define GT_SDRAM_B0 (0x44c >> 2)
+#define GT_SDRAM_B1 (0x450 >> 2)
+#define GT_SDRAM_B2 (0x454 >> 2)
+#define GT_SDRAM_B3 (0x458 >> 2)
/* Device Parameters */
-#define GT_DEV_B0 (0x45c >> 2)
-#define GT_DEV_B1 (0x460 >> 2)
-#define GT_DEV_B2 (0x464 >> 2)
-#define GT_DEV_B3 (0x468 >> 2)
-#define GT_DEV_BOOT (0x46c >> 2)
+#define GT_DEV_B0 (0x45c >> 2)
+#define GT_DEV_B1 (0x460 >> 2)
+#define GT_DEV_B2 (0x464 >> 2)
+#define GT_DEV_B3 (0x468 >> 2)
+#define GT_DEV_BOOT (0x46c >> 2)
/* ECC */
-#define GT_ECC_ERRDATALO (0x480 >> 2) /* GT-64120A only */
-#define GT_ECC_ERRDATAHI (0x484 >> 2) /* GT-64120A only */
-#define GT_ECC_MEM (0x488 >> 2) /* GT-64120A only */
-#define GT_ECC_CALC (0x48c >> 2) /* GT-64120A only */
-#define GT_ECC_ERRADDR (0x490 >> 2) /* GT-64120A only */
+#define GT_ECC_ERRDATALO (0x480 >> 2) /* GT-64120A only */
+#define GT_ECC_ERRDATAHI (0x484 >> 2) /* GT-64120A only */
+#define GT_ECC_MEM (0x488 >> 2) /* GT-64120A only */
+#define GT_ECC_CALC (0x48c >> 2) /* GT-64120A only */
+#define GT_ECC_ERRADDR (0x490 >> 2) /* GT-64120A only */
/* DMA Record */
-#define GT_DMA0_CNT (0x800 >> 2)
-#define GT_DMA1_CNT (0x804 >> 2)
-#define GT_DMA2_CNT (0x808 >> 2)
-#define GT_DMA3_CNT (0x80c >> 2)
-#define GT_DMA0_SA (0x810 >> 2)
-#define GT_DMA1_SA (0x814 >> 2)
-#define GT_DMA2_SA (0x818 >> 2)
-#define GT_DMA3_SA (0x81c >> 2)
-#define GT_DMA0_DA (0x820 >> 2)
-#define GT_DMA1_DA (0x824 >> 2)
-#define GT_DMA2_DA (0x828 >> 2)
-#define GT_DMA3_DA (0x82c >> 2)
-#define GT_DMA0_NEXT (0x830 >> 2)
-#define GT_DMA1_NEXT (0x834 >> 2)
-#define GT_DMA2_NEXT (0x838 >> 2)
-#define GT_DMA3_NEXT (0x83c >> 2)
-#define GT_DMA0_CUR (0x870 >> 2)
-#define GT_DMA1_CUR (0x874 >> 2)
-#define GT_DMA2_CUR (0x878 >> 2)
-#define GT_DMA3_CUR (0x87c >> 2)
+#define GT_DMA0_CNT (0x800 >> 2)
+#define GT_DMA1_CNT (0x804 >> 2)
+#define GT_DMA2_CNT (0x808 >> 2)
+#define GT_DMA3_CNT (0x80c >> 2)
+#define GT_DMA0_SA (0x810 >> 2)
+#define GT_DMA1_SA (0x814 >> 2)
+#define GT_DMA2_SA (0x818 >> 2)
+#define GT_DMA3_SA (0x81c >> 2)
+#define GT_DMA0_DA (0x820 >> 2)
+#define GT_DMA1_DA (0x824 >> 2)
+#define GT_DMA2_DA (0x828 >> 2)
+#define GT_DMA3_DA (0x82c >> 2)
+#define GT_DMA0_NEXT (0x830 >> 2)
+#define GT_DMA1_NEXT (0x834 >> 2)
+#define GT_DMA2_NEXT (0x838 >> 2)
+#define GT_DMA3_NEXT (0x83c >> 2)
+#define GT_DMA0_CUR (0x870 >> 2)
+#define GT_DMA1_CUR (0x874 >> 2)
+#define GT_DMA2_CUR (0x878 >> 2)
+#define GT_DMA3_CUR (0x87c >> 2)
/* DMA Channel Control */
-#define GT_DMA0_CTRL (0x840 >> 2)
-#define GT_DMA1_CTRL (0x844 >> 2)
-#define GT_DMA2_CTRL (0x848 >> 2)
-#define GT_DMA3_CTRL (0x84c >> 2)
+#define GT_DMA0_CTRL (0x840 >> 2)
+#define GT_DMA1_CTRL (0x844 >> 2)
+#define GT_DMA2_CTRL (0x848 >> 2)
+#define GT_DMA3_CTRL (0x84c >> 2)
/* DMA Arbiter */
-#define GT_DMA_ARB (0x860 >> 2)
+#define GT_DMA_ARB (0x860 >> 2)
/* Timer/Counter */
-#define GT_TC0 (0x850 >> 2)
-#define GT_TC1 (0x854 >> 2)
-#define GT_TC2 (0x858 >> 2)
-#define GT_TC3 (0x85c >> 2)
-#define GT_TC_CONTROL (0x864 >> 2)
+#define GT_TC0 (0x850 >> 2)
+#define GT_TC1 (0x854 >> 2)
+#define GT_TC2 (0x858 >> 2)
+#define GT_TC3 (0x85c >> 2)
+#define GT_TC_CONTROL (0x864 >> 2)
/* PCI Internal */
-#define GT_PCI0_CMD (0xc00 >> 2)
-#define GT_PCI0_TOR (0xc04 >> 2)
-#define GT_PCI0_BS_SCS10 (0xc08 >> 2)
-#define GT_PCI0_BS_SCS32 (0xc0c >> 2)
-#define GT_PCI0_BS_CS20 (0xc10 >> 2)
-#define GT_PCI0_BS_CS3BT (0xc14 >> 2)
-#define GT_PCI1_IACK (0xc30 >> 2)
-#define GT_PCI0_IACK (0xc34 >> 2)
-#define GT_PCI0_BARE (0xc3c >> 2)
-#define GT_PCI0_PREFMBR (0xc40 >> 2)
-#define GT_PCI0_SCS10_BAR (0xc48 >> 2)
-#define GT_PCI0_SCS32_BAR (0xc4c >> 2)
-#define GT_PCI0_CS20_BAR (0xc50 >> 2)
-#define GT_PCI0_CS3BT_BAR (0xc54 >> 2)
-#define GT_PCI0_SSCS10_BAR (0xc58 >> 2)
-#define GT_PCI0_SSCS32_BAR (0xc5c >> 2)
-#define GT_PCI0_SCS3BT_BAR (0xc64 >> 2)
-#define GT_PCI1_CMD (0xc80 >> 2)
-#define GT_PCI1_TOR (0xc84 >> 2)
-#define GT_PCI1_BS_SCS10 (0xc88 >> 2)
-#define GT_PCI1_BS_SCS32 (0xc8c >> 2)
-#define GT_PCI1_BS_CS20 (0xc90 >> 2)
-#define GT_PCI1_BS_CS3BT (0xc94 >> 2)
-#define GT_PCI1_BARE (0xcbc >> 2)
-#define GT_PCI1_PREFMBR (0xcc0 >> 2)
-#define GT_PCI1_SCS10_BAR (0xcc8 >> 2)
-#define GT_PCI1_SCS32_BAR (0xccc >> 2)
-#define GT_PCI1_CS20_BAR (0xcd0 >> 2)
-#define GT_PCI1_CS3BT_BAR (0xcd4 >> 2)
-#define GT_PCI1_SSCS10_BAR (0xcd8 >> 2)
-#define GT_PCI1_SSCS32_BAR (0xcdc >> 2)
-#define GT_PCI1_SCS3BT_BAR (0xce4 >> 2)
-#define GT_PCI1_CFGADDR (0xcf0 >> 2)
-#define GT_PCI1_CFGDATA (0xcf4 >> 2)
-#define GT_PCI0_CFGADDR (0xcf8 >> 2)
-#define GT_PCI0_CFGDATA (0xcfc >> 2)
+#define GT_PCI0_CMD (0xc00 >> 2)
+#define GT_PCI0_TOR (0xc04 >> 2)
+#define GT_PCI0_BS_SCS10 (0xc08 >> 2)
+#define GT_PCI0_BS_SCS32 (0xc0c >> 2)
+#define GT_PCI0_BS_CS20 (0xc10 >> 2)
+#define GT_PCI0_BS_CS3BT (0xc14 >> 2)
+#define GT_PCI1_IACK (0xc30 >> 2)
+#define GT_PCI0_IACK (0xc34 >> 2)
+#define GT_PCI0_BARE (0xc3c >> 2)
+#define GT_PCI0_PREFMBR (0xc40 >> 2)
+#define GT_PCI0_SCS10_BAR (0xc48 >> 2)
+#define GT_PCI0_SCS32_BAR (0xc4c >> 2)
+#define GT_PCI0_CS20_BAR (0xc50 >> 2)
+#define GT_PCI0_CS3BT_BAR (0xc54 >> 2)
+#define GT_PCI0_SSCS10_BAR (0xc58 >> 2)
+#define GT_PCI0_SSCS32_BAR (0xc5c >> 2)
+#define GT_PCI0_SCS3BT_BAR (0xc64 >> 2)
+#define GT_PCI1_CMD (0xc80 >> 2)
+#define GT_PCI1_TOR (0xc84 >> 2)
+#define GT_PCI1_BS_SCS10 (0xc88 >> 2)
+#define GT_PCI1_BS_SCS32 (0xc8c >> 2)
+#define GT_PCI1_BS_CS20 (0xc90 >> 2)
+#define GT_PCI1_BS_CS3BT (0xc94 >> 2)
+#define GT_PCI1_BARE (0xcbc >> 2)
+#define GT_PCI1_PREFMBR (0xcc0 >> 2)
+#define GT_PCI1_SCS10_BAR (0xcc8 >> 2)
+#define GT_PCI1_SCS32_BAR (0xccc >> 2)
+#define GT_PCI1_CS20_BAR (0xcd0 >> 2)
+#define GT_PCI1_CS3BT_BAR (0xcd4 >> 2)
+#define GT_PCI1_SSCS10_BAR (0xcd8 >> 2)
+#define GT_PCI1_SSCS32_BAR (0xcdc >> 2)
+#define GT_PCI1_SCS3BT_BAR (0xce4 >> 2)
+#define GT_PCI1_CFGADDR (0xcf0 >> 2)
+#define GT_PCI1_CFGDATA (0xcf4 >> 2)
+#define GT_PCI0_CFGADDR (0xcf8 >> 2)
+#define GT_PCI0_CFGDATA (0xcfc >> 2)
/* Interrupts */
-#define GT_INTRCAUSE (0xc18 >> 2)
-#define GT_INTRMASK (0xc1c >> 2)
-#define GT_PCI0_ICMASK (0xc24 >> 2)
-#define GT_PCI0_SERR0MASK (0xc28 >> 2)
-#define GT_CPU_INTSEL (0xc70 >> 2)
-#define GT_PCI0_INTSEL (0xc74 >> 2)
-#define GT_HINTRCAUSE (0xc98 >> 2)
-#define GT_HINTRMASK (0xc9c >> 2)
-#define GT_PCI0_HICMASK (0xca4 >> 2)
-#define GT_PCI1_SERR1MASK (0xca8 >> 2)
+#define GT_INTRCAUSE (0xc18 >> 2)
+#define GT_INTRMASK (0xc1c >> 2)
+#define GT_PCI0_ICMASK (0xc24 >> 2)
+#define GT_PCI0_SERR0MASK (0xc28 >> 2)
+#define GT_CPU_INTSEL (0xc70 >> 2)
+#define GT_PCI0_INTSEL (0xc74 >> 2)
+#define GT_HINTRCAUSE (0xc98 >> 2)
+#define GT_HINTRMASK (0xc9c >> 2)
+#define GT_PCI0_HICMASK (0xca4 >> 2)
+#define GT_PCI1_SERR1MASK (0xca8 >> 2)
#define PCI_MAPPING_ENTRY(regname) \
hwaddr regname ##_start; \
--
2.19.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH 03/10] hw/mips/gt64xxx_pci: Fix 'braces' coding style issues
2019-06-24 22:28 [Qemu-devel] [PATCH 00/10] hw/pci-host: Clean the GT64120 north bridge Philippe Mathieu-Daudé
2019-06-24 22:28 ` [Qemu-devel] [PATCH 01/10] hw/mips/gt64xxx_pci: Fix multiline comment syntax Philippe Mathieu-Daudé
2019-06-24 22:28 ` [Qemu-devel] [PATCH 02/10] hw/mips/gt64xxx_pci: Fix 'tabs' coding style issues Philippe Mathieu-Daudé
@ 2019-06-24 22:28 ` Philippe Mathieu-Daudé
2019-06-25 0:22 ` Aleksandar Markovic
2019-06-24 22:28 ` [Qemu-devel] [PATCH 04/10] hw/mips/gt64xxx_pci: Fix 'spaces' " Philippe Mathieu-Daudé
` (7 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-06-24 22:28 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Thomas Huth, Aleksandar Rikalo,
Philippe Mathieu-Daudé,
Hervé Poussineau, Artyom Tarasenko, Aleksandar Markovic,
Paolo Bonzini, Aurelien Jarno
Since we'll move this code around, fix its style first:
ERROR: braces {} are necessary for all arms of this statement
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/mips/gt64xxx_pci.c | 27 ++++++++++++++++++---------
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
index bbd719f091..cfd497960c 100644
--- a/hw/mips/gt64xxx_pci.c
+++ b/hw/mips/gt64xxx_pci.c
@@ -257,19 +257,25 @@ static void check_reserved_space(hwaddr *start, hwaddr *length)
hwaddr begin = *start;
hwaddr end = *start + *length;
- if (end >= 0x1e000000LL && end < 0x1f100000LL)
+ if (end >= 0x1e000000LL && end < 0x1f100000LL) {
end = 0x1e000000LL;
- if (begin >= 0x1e000000LL && begin < 0x1f100000LL)
+ }
+ if (begin >= 0x1e000000LL && begin < 0x1f100000LL) {
begin = 0x1f100000LL;
- if (end >= 0x1fc00000LL && end < 0x1fd00000LL)
+ }
+ if (end >= 0x1fc00000LL && end < 0x1fd00000LL) {
end = 0x1fc00000LL;
- if (begin >= 0x1fc00000LL && begin < 0x1fd00000LL)
+ }
+ if (begin >= 0x1fc00000LL && begin < 0x1fd00000LL) {
begin = 0x1fd00000LL;
+ }
/* XXX: This is broken when a reserved range splits the requested range */
- if (end >= 0x1f100000LL && begin < 0x1e000000LL)
+ if (end >= 0x1f100000LL && begin < 0x1e000000LL) {
end = 0x1e000000LL;
- if (end >= 0x1fd00000LL && begin < 0x1fc00000LL)
+ }
+ if (end >= 0x1fd00000LL && begin < 0x1fc00000LL) {
end = 0x1fc00000LL;
+ }
*start = begin;
*length = end - begin;
@@ -385,8 +391,9 @@ static void gt64120_writel (void *opaque, hwaddr addr,
PCIHostState *phb = PCI_HOST_BRIDGE(s);
uint32_t saddr;
- if (!(s->regs[GT_CPU] & 0x00001000))
+ if (!(s->regs[GT_CPU] & 0x00001000)) {
val = bswap32(val);
+ }
saddr = (addr & 0xfff) >> 2;
switch (saddr) {
@@ -937,8 +944,9 @@ static uint64_t gt64120_readl (void *opaque,
break;
}
- if (!(s->regs[GT_CPU] & 0x00001000))
+ if (!(s->regs[GT_CPU] & 0x00001000)) {
val = bswap32(val);
+ }
return val;
}
@@ -990,8 +998,9 @@ static void gt64120_pci_set_irq(void *opaque, int irq_num, int level)
/* The pic level is the logical OR of all the PCI irqs mapped to it. */
pic_level = 0;
for (i = 0; i < 4; i++) {
- if (pic_irq == piix4_dev->config[0x60 + i])
+ if (pic_irq == piix4_dev->config[0x60 + i]) {
pic_level |= pci_irq_levels[i];
+ }
}
qemu_set_irq(pic[pic_irq], pic_level);
}
--
2.19.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH 04/10] hw/mips/gt64xxx_pci: Fix 'spaces' coding style issues
2019-06-24 22:28 [Qemu-devel] [PATCH 00/10] hw/pci-host: Clean the GT64120 north bridge Philippe Mathieu-Daudé
` (2 preceding siblings ...)
2019-06-24 22:28 ` [Qemu-devel] [PATCH 03/10] hw/mips/gt64xxx_pci: Fix 'braces' " Philippe Mathieu-Daudé
@ 2019-06-24 22:28 ` Philippe Mathieu-Daudé
2019-06-25 0:23 ` Aleksandar Markovic
2019-06-24 22:28 ` [Qemu-devel] [PATCH 05/10] hw/mips/gt64xxx_pci: Use qemu_log_mask() instead of debug printf() Philippe Mathieu-Daudé
` (6 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-06-24 22:28 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Thomas Huth, Aleksandar Rikalo,
Philippe Mathieu-Daudé,
Hervé Poussineau, Artyom Tarasenko, Aleksandar Markovic,
Paolo Bonzini, Aurelien Jarno
Since we'll move this code around, fix its style first:
ERROR: space prohibited between function name and open parenthesis
ERROR: line over 90 characters
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/mips/gt64xxx_pci.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
index cfd497960c..0b9fb02475 100644
--- a/hw/mips/gt64xxx_pci.c
+++ b/hw/mips/gt64xxx_pci.c
@@ -384,8 +384,8 @@ static const VMStateDescription vmstate_gt64120 = {
}
};
-static void gt64120_writel (void *opaque, hwaddr addr,
- uint64_t val, unsigned size)
+static void gt64120_writel(void *opaque, hwaddr addr,
+ uint64_t val, unsigned size)
{
GT64120State *s = opaque;
PCIHostState *phb = PCI_HOST_BRIDGE(s);
@@ -671,8 +671,8 @@ static void gt64120_writel (void *opaque, hwaddr addr,
}
}
-static uint64_t gt64120_readl (void *opaque,
- hwaddr addr, unsigned size)
+static uint64_t gt64120_readl(void *opaque,
+ hwaddr addr, unsigned size)
{
GT64120State *s = opaque;
PCIHostState *phb = PCI_HOST_BRIDGE(s);
@@ -1193,7 +1193,8 @@ PCIBus *gt64120_register(qemu_irq *pic)
get_system_io(),
PCI_DEVFN(18, 0), 4, TYPE_PCI_BUS);
qdev_init_nofail(dev);
- memory_region_init_io(&d->ISD_mem, OBJECT(dev), &isd_mem_ops, d, "isd-mem", 0x1000);
+ memory_region_init_io(&d->ISD_mem, OBJECT(dev), &isd_mem_ops, d,
+ "isd-mem", 0x1000);
pci_create_simple(phb->bus, PCI_DEVFN(0, 0), "gt64120_pci");
return phb->bus;
--
2.19.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH 05/10] hw/mips/gt64xxx_pci: Use qemu_log_mask() instead of debug printf()
2019-06-24 22:28 [Qemu-devel] [PATCH 00/10] hw/pci-host: Clean the GT64120 north bridge Philippe Mathieu-Daudé
` (3 preceding siblings ...)
2019-06-24 22:28 ` [Qemu-devel] [PATCH 04/10] hw/mips/gt64xxx_pci: Fix 'spaces' " Philippe Mathieu-Daudé
@ 2019-06-24 22:28 ` Philippe Mathieu-Daudé
2019-06-25 0:37 ` Aleksandar Markovic
2019-06-25 18:16 ` Aleksandar Markovic
2019-06-24 22:28 ` [Qemu-devel] [PATCH 06/10] hw/mips/gt64xxx_pci: Convert debug printf()s to trace events Philippe Mathieu-Daudé
` (5 subsequent siblings)
10 siblings, 2 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-06-24 22:28 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Thomas Huth, Aleksandar Rikalo,
Philippe Mathieu-Daudé,
Hervé Poussineau, Artyom Tarasenko, Aleksandar Markovic,
Paolo Bonzini, Aurelien Jarno
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/mips/gt64xxx_pci.c | 48 +++++++++++++++++++++++++++++++++----------
1 file changed, 37 insertions(+), 11 deletions(-)
diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
index 0b9fb02475..f44326f14f 100644
--- a/hw/mips/gt64xxx_pci.c
+++ b/hw/mips/gt64xxx_pci.c
@@ -23,6 +23,7 @@
*/
#include "qemu/osdep.h"
+#include "qemu/log.h"
#include "hw/hw.h"
#include "hw/mips/mips.h"
#include "hw/pci/pci.h"
@@ -466,12 +467,20 @@ static void gt64120_writel(void *opaque, hwaddr addr,
case GT_CPUERR_DATAHI:
case GT_CPUERR_PARITY:
/* Read-only registers, do nothing */
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "gt64120: Read-only register write "
+ "reg:0x03%x size:%u value:0x%0*" PRIx64 "\n",
+ saddr << 2, size, size << 1, val);
break;
/* CPU Sync Barrier */
case GT_PCI0SYNC:
case GT_PCI1SYNC:
/* Read-only registers, do nothing */
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "gt64120: Read-only register write "
+ "reg:0x03%x size:%u value:0x%0*" PRIx64 "\n",
+ saddr << 2, size, size << 1, val);
break;
/* SDRAM and Device Address Decode */
@@ -510,7 +519,10 @@ static void gt64120_writel(void *opaque, hwaddr addr,
case GT_DEV_B3:
case GT_DEV_BOOT:
/* Not implemented */
- DPRINTF ("Unimplemented device register offset 0x%x\n", saddr << 2);
+ qemu_log_mask(LOG_UNIMP,
+ "gt64120: Unimplemented device register write "
+ "reg:0x03%x size:%u value:0x%0*" PRIx64 "\n",
+ saddr << 2, size, size << 1, val);
break;
/* ECC */
@@ -520,6 +532,10 @@ static void gt64120_writel(void *opaque, hwaddr addr,
case GT_ECC_CALC:
case GT_ECC_ERRADDR:
/* Read-only registers, do nothing */
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "gt64120: Read-only register write "
+ "reg:0x03%x size:%u value:0x%0*" PRIx64 "\n",
+ saddr << 2, size, size << 1, val);
break;
/* DMA Record */
@@ -543,23 +559,20 @@ static void gt64120_writel(void *opaque, hwaddr addr,
case GT_DMA1_CUR:
case GT_DMA2_CUR:
case GT_DMA3_CUR:
- /* Not implemented */
- DPRINTF ("Unimplemented DMA register offset 0x%x\n", saddr << 2);
- break;
/* DMA Channel Control */
case GT_DMA0_CTRL:
case GT_DMA1_CTRL:
case GT_DMA2_CTRL:
case GT_DMA3_CTRL:
- /* Not implemented */
- DPRINTF ("Unimplemented DMA register offset 0x%x\n", saddr << 2);
- break;
/* DMA Arbiter */
case GT_DMA_ARB:
/* Not implemented */
- DPRINTF ("Unimplemented DMA register offset 0x%x\n", saddr << 2);
+ qemu_log_mask(LOG_UNIMP,
+ "gt64120: Unimplemented DMA register write "
+ "reg:0x03%x size:%u value:0x%0*" PRIx64 "\n",
+ saddr << 2, size, size << 1, val);
break;
/* Timer/Counter */
@@ -569,7 +582,10 @@ static void gt64120_writel(void *opaque, hwaddr addr,
case GT_TC3:
case GT_TC_CONTROL:
/* Not implemented */
- DPRINTF ("Unimplemented timer register offset 0x%x\n", saddr << 2);
+ qemu_log_mask(LOG_UNIMP,
+ "gt64120: Unimplemented timer register write "
+ "reg:0x03%x size:%u value:0x%0*" PRIx64 "\n",
+ saddr << 2, size, size << 1, val);
break;
/* PCI Internal */
@@ -610,6 +626,10 @@ static void gt64120_writel(void *opaque, hwaddr addr,
case GT_PCI1_CFGADDR:
case GT_PCI1_CFGDATA:
/* not implemented */
+ qemu_log_mask(LOG_UNIMP,
+ "gt64120: Unimplemented timer register write "
+ "reg:0x03%x size:%u value:0x%0*" PRIx64 "\n",
+ saddr << 2, size, size << 1, val);
break;
case GT_PCI0_CFGADDR:
phb->config_reg = val & 0x80fffffc;
@@ -666,7 +686,10 @@ static void gt64120_writel(void *opaque, hwaddr addr,
break;
default:
- DPRINTF ("Bad register offset 0x%x\n", (int)addr);
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "gt64120: Illegal register write "
+ "reg:0x03%x size:%u value:0x%0*" PRIx64 "\n",
+ saddr << 2, size, size << 1, val);
break;
}
}
@@ -940,7 +963,10 @@ static uint64_t gt64120_readl(void *opaque,
default:
val = s->regs[saddr];
- DPRINTF ("Bad register offset 0x%x\n", (int)addr);
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "gt64120: Illegal register read "
+ "reg:0x03%x size:%u value:0x%0*x\n",
+ saddr << 2, size, size << 1, val);
break;
}
--
2.19.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH 06/10] hw/mips/gt64xxx_pci: Convert debug printf()s to trace events
2019-06-24 22:28 [Qemu-devel] [PATCH 00/10] hw/pci-host: Clean the GT64120 north bridge Philippe Mathieu-Daudé
` (4 preceding siblings ...)
2019-06-24 22:28 ` [Qemu-devel] [PATCH 05/10] hw/mips/gt64xxx_pci: Use qemu_log_mask() instead of debug printf() Philippe Mathieu-Daudé
@ 2019-06-24 22:28 ` Philippe Mathieu-Daudé
2019-06-25 0:40 ` Aleksandar Markovic
2019-06-25 18:17 ` Aleksandar Markovic
2019-06-24 22:28 ` [Qemu-devel] [PATCH 07/10] hw/mips/gt64xxx_pci: Align the pci0-mem size Philippe Mathieu-Daudé
` (4 subsequent siblings)
10 siblings, 2 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-06-24 22:28 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Thomas Huth, Aleksandar Rikalo,
Philippe Mathieu-Daudé,
Hervé Poussineau, Artyom Tarasenko, Aleksandar Markovic,
Paolo Bonzini, Aurelien Jarno
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
Makefile.objs | 1 +
hw/mips/gt64xxx_pci.c | 29 ++++++++++-------------------
hw/mips/trace-events | 4 ++++
3 files changed, 15 insertions(+), 19 deletions(-)
create mode 100644 hw/mips/trace-events
diff --git a/Makefile.objs b/Makefile.objs
index 658cfc9d9f..3b83621f32 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -163,6 +163,7 @@ trace-events-subdirs += hw/input
trace-events-subdirs += hw/intc
trace-events-subdirs += hw/isa
trace-events-subdirs += hw/mem
+trace-events-subdirs += hw/mips
trace-events-subdirs += hw/misc
trace-events-subdirs += hw/misc/macio
trace-events-subdirs += hw/net
diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
index f44326f14f..815ef0711d 100644
--- a/hw/mips/gt64xxx_pci.c
+++ b/hw/mips/gt64xxx_pci.c
@@ -30,14 +30,7 @@
#include "hw/pci/pci_host.h"
#include "hw/i386/pc.h"
#include "exec/address-spaces.h"
-
-//#define DEBUG
-
-#ifdef DEBUG
-#define DPRINTF(fmt, ...) fprintf(stderr, "%s: " fmt, __func__, ##__VA_ARGS__)
-#else
-#define DPRINTF(fmt, ...)
-#endif
+#include "trace.h"
#define GT_REGS (0x1000 >> 2)
@@ -294,9 +287,7 @@ static void gt64120_isd_mapping(GT64120State *s)
check_reserved_space(&start, &length);
length = 0x1000;
/* Map new address */
- DPRINTF("ISD: "TARGET_FMT_plx"@"TARGET_FMT_plx
- " -> "TARGET_FMT_plx"@"TARGET_FMT_plx"\n",
- s->ISD_length, s->ISD_start, length, start);
+ trace_gt64120_isd_remap(s->ISD_length, s->ISD_start, length, start);
s->ISD_start = start;
s->ISD_length = length;
memory_region_add_subregion(get_system_memory(), s->ISD_start, &s->ISD_mem);
@@ -648,19 +639,19 @@ static void gt64120_writel(void *opaque, hwaddr addr,
/* not really implemented */
s->regs[saddr] = ~(~(s->regs[saddr]) | ~(val & 0xfffffffe));
s->regs[saddr] |= !!(s->regs[saddr] & 0xfffffffe);
- DPRINTF("INTRCAUSE %" PRIx64 "\n", val);
+ trace_gt64120_write("INTRCAUSE", size << 1, val);
break;
case GT_INTRMASK:
s->regs[saddr] = val & 0x3c3ffffe;
- DPRINTF("INTRMASK %" PRIx64 "\n", val);
+ trace_gt64120_write("INTRMASK", size << 1, val);
break;
case GT_PCI0_ICMASK:
s->regs[saddr] = val & 0x03fffffe;
- DPRINTF("ICMASK %" PRIx64 "\n", val);
+ trace_gt64120_write("ICMASK", size << 1, val);
break;
case GT_PCI0_SERR0MASK:
s->regs[saddr] = val & 0x0000003f;
- DPRINTF("SERR0MASK %" PRIx64 "\n", val);
+ trace_gt64120_write("SERR0MASK", size << 1, val);
break;
/* Reserved when only PCI_0 is configured. */
@@ -936,19 +927,19 @@ static uint64_t gt64120_readl(void *opaque,
/* Interrupts */
case GT_INTRCAUSE:
val = s->regs[saddr];
- DPRINTF("INTRCAUSE %x\n", val);
+ trace_gt64120_read("INTRCAUSE", size << 1, val);
break;
case GT_INTRMASK:
val = s->regs[saddr];
- DPRINTF("INTRMASK %x\n", val);
+ trace_gt64120_read("INTRMASK", size << 1, val);
break;
case GT_PCI0_ICMASK:
val = s->regs[saddr];
- DPRINTF("ICMASK %x\n", val);
+ trace_gt64120_read("ICMASK", size << 1, val);
break;
case GT_PCI0_SERR0MASK:
val = s->regs[saddr];
- DPRINTF("SERR0MASK %x\n", val);
+ trace_gt64120_read("SERR0MASK", size << 1, val);
break;
/* Reserved when only PCI_0 is configured. */
diff --git a/hw/mips/trace-events b/hw/mips/trace-events
new file mode 100644
index 0000000000..75d4c73f2e
--- /dev/null
+++ b/hw/mips/trace-events
@@ -0,0 +1,4 @@
+# gt64xxx.c
+gt64120_read(const char *regname, int width, uint64_t value) "gt64120 read %s value:0x%0*" PRIx64
+gt64120_write(const char *regname, int width, uint64_t value) "gt64120 write %s value:0x%0*" PRIx64
+gt64120_isd_remap(uint64_t from_length, uint64_t from_addr, uint64_t to_length, uint64_t to_addr) "ISD: 0x%08" PRIx64 "@0x%08" PRIx64 " -> 0x%08" PRIx64 "@0x%08" PRIx64
--
2.19.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH 07/10] hw/mips/gt64xxx_pci: Align the pci0-mem size
2019-06-24 22:28 [Qemu-devel] [PATCH 00/10] hw/pci-host: Clean the GT64120 north bridge Philippe Mathieu-Daudé
` (5 preceding siblings ...)
2019-06-24 22:28 ` [Qemu-devel] [PATCH 06/10] hw/mips/gt64xxx_pci: Convert debug printf()s to trace events Philippe Mathieu-Daudé
@ 2019-06-24 22:28 ` Philippe Mathieu-Daudé
2019-06-25 0:43 ` Aleksandar Markovic
2019-06-24 22:28 ` [Qemu-devel] [PATCH 08/10] hw/mips/gt64xxx_pci: Add a 'cpu_big_endian' qdev property Philippe Mathieu-Daudé
` (3 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-06-24 22:28 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Thomas Huth, Aleksandar Rikalo,
Philippe Mathieu-Daudé,
Hervé Poussineau, Artyom Tarasenko, Aleksandar Markovic,
Paolo Bonzini, Aurelien Jarno
One byte is missing, use an aligned size.
(qemu) info mtree
memory-region: pci0-mem
0000000000000000-00000000fffffffe (prio 0, i/o): pci0-mem
^
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/mips/gt64xxx_pci.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
index 815ef0711d..2fa313f498 100644
--- a/hw/mips/gt64xxx_pci.c
+++ b/hw/mips/gt64xxx_pci.c
@@ -23,6 +23,7 @@
*/
#include "qemu/osdep.h"
+#include "qemu/units.h"
#include "qemu/log.h"
#include "hw/hw.h"
#include "hw/mips/mips.h"
@@ -1201,7 +1202,7 @@ PCIBus *gt64120_register(qemu_irq *pic)
dev = qdev_create(NULL, TYPE_GT64120_PCI_HOST_BRIDGE);
d = GT64120_PCI_HOST_BRIDGE(dev);
phb = PCI_HOST_BRIDGE(dev);
- memory_region_init(&d->pci0_mem, OBJECT(dev), "pci0-mem", UINT32_MAX);
+ memory_region_init(&d->pci0_mem, OBJECT(dev), "pci0-mem", 4 * GiB);
address_space_init(&d->pci0_mem_as, &d->pci0_mem, "pci0-mem");
phb->bus = pci_register_root_bus(dev, "pci",
gt64120_pci_set_irq, gt64120_pci_map_irq,
--
2.19.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH 08/10] hw/mips/gt64xxx_pci: Add a 'cpu_big_endian' qdev property
2019-06-24 22:28 [Qemu-devel] [PATCH 00/10] hw/pci-host: Clean the GT64120 north bridge Philippe Mathieu-Daudé
` (6 preceding siblings ...)
2019-06-24 22:28 ` [Qemu-devel] [PATCH 07/10] hw/mips/gt64xxx_pci: Align the pci0-mem size Philippe Mathieu-Daudé
@ 2019-06-24 22:28 ` Philippe Mathieu-Daudé
2019-06-24 22:28 ` [Qemu-devel] [PATCH 09/10] hw/mips/gt64xxx_pci: Move it to hw/pci-host/ Philippe Mathieu-Daudé
` (2 subsequent siblings)
10 siblings, 0 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-06-24 22:28 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Thomas Huth, Aleksandar Rikalo,
Philippe Mathieu-Daudé,
Hervé Poussineau, Artyom Tarasenko, Aleksandar Markovic,
Paolo Bonzini, Aurelien Jarno
This device does not have to be TARGET-dependent.
Add a 'cpu_big_endian' property which sets the byte-swapping
options if required.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
I might change my mind and name it 'little_endian' to be closer
to the datasheet.
---
include/hw/mips/mips.h | 2 +-
hw/mips/gt64xxx_pci.c | 29 +++++++++++++----------------
hw/mips/mips_malta.c | 2 +-
3 files changed, 15 insertions(+), 18 deletions(-)
diff --git a/include/hw/mips/mips.h b/include/hw/mips/mips.h
index 2f6774d540..6ec41d33f1 100644
--- a/include/hw/mips/mips.h
+++ b/include/hw/mips/mips.h
@@ -9,7 +9,7 @@
#include "hw/irq.h"
/* gt64xxx.c */
-PCIBus *gt64120_register(qemu_irq *pic);
+PCIBus *gt64120_create(qemu_irq *pic, bool target_is_bigendian);
/* bonito.c */
PCIBus *bonito_init(qemu_irq *pic);
diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
index 2fa313f498..5209038ee5 100644
--- a/hw/mips/gt64xxx_pci.c
+++ b/hw/mips/gt64xxx_pci.c
@@ -240,6 +240,7 @@ typedef struct GT64120State {
PCI_MAPPING_ENTRY(ISD);
MemoryRegion pci0_mem;
AddressSpace pci0_mem_as;
+ bool cpu_big_endian;
} GT64120State;
/* Adjust range to avoid touching space which isn't mappable via PCI */
@@ -1028,15 +1029,12 @@ static void gt64120_pci_set_irq(void *opaque, int irq_num, int level)
static void gt64120_reset(DeviceState *dev)
{
GT64120State *s = GT64120_PCI_HOST_BRIDGE(dev);
+ const uint32_t pci_cmd = s->cpu_big_endian ? 0x00000000 : 0x00010001;
/* FIXME: Malta specific hw assumptions ahead */
/* CPU Configuration */
-#ifdef TARGET_WORDS_BIGENDIAN
- s->regs[GT_CPU] = 0x00000000;
-#else
- s->regs[GT_CPU] = 0x00001000;
-#endif
+ s->regs[GT_CPU] = !s->cpu_big_endian << 12;
s->regs[GT_MULTI] = 0x00000003;
/* CPU Address decode */
@@ -1143,11 +1141,7 @@ static void gt64120_reset(DeviceState *dev)
s->regs[GT_TC_CONTROL] = 0x00000000;
/* PCI Internal */
-#ifdef TARGET_WORDS_BIGENDIAN
- s->regs[GT_PCI0_CMD] = 0x00000000;
-#else
- s->regs[GT_PCI0_CMD] = 0x00010001;
-#endif
+ s->regs[GT_PCI0_CMD] = pci_cmd;
s->regs[GT_PCI0_TOR] = 0x0000070f;
s->regs[GT_PCI0_BS_SCS10] = 0x00fff000;
s->regs[GT_PCI0_BS_SCS32] = 0x00fff000;
@@ -1164,11 +1158,7 @@ static void gt64120_reset(DeviceState *dev)
s->regs[GT_PCI0_SSCS10_BAR] = 0x00000000;
s->regs[GT_PCI0_SSCS32_BAR] = 0x01000000;
s->regs[GT_PCI0_SCS3BT_BAR] = 0x1f000000;
-#ifdef TARGET_WORDS_BIGENDIAN
- s->regs[GT_PCI1_CMD] = 0x00000000;
-#else
- s->regs[GT_PCI1_CMD] = 0x00010001;
-#endif
+ s->regs[GT_PCI1_CMD] = pci_cmd;
s->regs[GT_PCI1_TOR] = 0x0000070f;
s->regs[GT_PCI1_BS_SCS10] = 0x00fff000;
s->regs[GT_PCI1_BS_SCS32] = 0x00fff000;
@@ -1193,13 +1183,14 @@ static void gt64120_reset(DeviceState *dev)
gt64120_pci_mapping(s);
}
-PCIBus *gt64120_register(qemu_irq *pic)
+PCIBus *gt64120_create(qemu_irq *pic, bool target_is_bigendian)
{
GT64120State *d;
PCIHostState *phb;
DeviceState *dev;
dev = qdev_create(NULL, TYPE_GT64120_PCI_HOST_BRIDGE);
+ qdev_prop_set_bit(dev, "cpu_big_endian", target_is_bigendian);
d = GT64120_PCI_HOST_BRIDGE(dev);
phb = PCI_HOST_BRIDGE(dev);
memory_region_init(&d->pci0_mem, OBJECT(dev), "pci0-mem", 4 * GiB);
@@ -1262,6 +1253,11 @@ static const TypeInfo gt64120_pci_info = {
},
};
+static Property gt64120_properties[] = {
+ DEFINE_PROP_BOOL("cpu_big_endian", GT64120State, cpu_big_endian, true),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
static void gt64120_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
@@ -1269,6 +1265,7 @@ static void gt64120_class_init(ObjectClass *klass, void *data)
set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
dc->reset = gt64120_reset;
dc->vmsd = &vmstate_gt64120;
+ dc->props = gt64120_properties;
}
static const TypeInfo gt64120_info = {
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index 51db5212be..97f8ffbf1b 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -1375,7 +1375,7 @@ void mips_malta_init(MachineState *machine)
isa_irq = qemu_irq_proxy(&s->i8259, 16);
/* Northbridge */
- pci_bus = gt64120_register(isa_irq);
+ pci_bus = gt64120_create(isa_irq, be);
/* Southbridge */
ide_drive_get(hd, ARRAY_SIZE(hd));
--
2.19.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH 09/10] hw/mips/gt64xxx_pci: Move it to hw/pci-host/
2019-06-24 22:28 [Qemu-devel] [PATCH 00/10] hw/pci-host: Clean the GT64120 north bridge Philippe Mathieu-Daudé
` (7 preceding siblings ...)
2019-06-24 22:28 ` [Qemu-devel] [PATCH 08/10] hw/mips/gt64xxx_pci: Add a 'cpu_big_endian' qdev property Philippe Mathieu-Daudé
@ 2019-06-24 22:28 ` Philippe Mathieu-Daudé
2019-06-24 22:28 ` [Qemu-devel] [RFC PATCH 10/10] hw/pci-host/gt64120: Clean the decoded address space Philippe Mathieu-Daudé
2019-07-01 17:16 ` [Qemu-devel] [PATCH 00/10] hw/pci-host: Clean the GT64120 north bridge Aleksandar Markovic
10 siblings, 0 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-06-24 22:28 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Thomas Huth, Aleksandar Rikalo,
Philippe Mathieu-Daudé,
Hervé Poussineau, Artyom Tarasenko, Aleksandar Markovic,
Paolo Bonzini, Aurelien Jarno
The GT-64120 is a north-bridge, and it is not MIPS specific.
Move it with the other north-bridge devices.
We move this device in the common-obj, and compile it once for
the 4 different MIPS targets.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/{mips/gt64xxx_pci.c => pci-host/gt64120.c} | 0
MAINTAINERS | 2 +-
hw/mips/Makefile.objs | 2 +-
hw/mips/trace-events | 4 ----
hw/pci-host/Makefile.objs | 2 +-
hw/pci-host/trace-events | 5 +++++
6 files changed, 8 insertions(+), 7 deletions(-)
rename hw/{mips/gt64xxx_pci.c => pci-host/gt64120.c} (100%)
diff --git a/hw/mips/gt64xxx_pci.c b/hw/pci-host/gt64120.c
similarity index 100%
rename from hw/mips/gt64xxx_pci.c
rename to hw/pci-host/gt64120.c
diff --git a/MAINTAINERS b/MAINTAINERS
index abef4a1cfc..da348e1af1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -928,7 +928,7 @@ M: Aurelien Jarno <aurelien@aurel32.net>
R: Aleksandar Rikalo <arikalo@wavecomp.com>
S: Maintained
F: hw/mips/mips_malta.c
-F: hw/mips/gt64xxx_pci.c
+F: hw/pci-host/gt64120.c
F: tests/acceptance/linux_ssh_mips_malta.py
Mipssim
diff --git a/hw/mips/Makefile.objs b/hw/mips/Makefile.objs
index 525809af07..da65e19c20 100644
--- a/hw/mips/Makefile.objs
+++ b/hw/mips/Makefile.objs
@@ -1,6 +1,6 @@
obj-y += addr.o mips_int.o
obj-$(CONFIG_R4K) += mips_r4k.o
-obj-$(CONFIG_MALTA) += gt64xxx_pci.o mips_malta.o
+obj-$(CONFIG_MALTA) += mips_malta.o
obj-$(CONFIG_MIPSSIM) += mips_mipssim.o
obj-$(CONFIG_JAZZ) += mips_jazz.o
obj-$(CONFIG_FULONG) += mips_fulong2e.o
diff --git a/hw/mips/trace-events b/hw/mips/trace-events
index 75d4c73f2e..e69de29bb2 100644
--- a/hw/mips/trace-events
+++ b/hw/mips/trace-events
@@ -1,4 +0,0 @@
-# gt64xxx.c
-gt64120_read(const char *regname, int width, uint64_t value) "gt64120 read %s value:0x%0*" PRIx64
-gt64120_write(const char *regname, int width, uint64_t value) "gt64120 write %s value:0x%0*" PRIx64
-gt64120_isd_remap(uint64_t from_length, uint64_t from_addr, uint64_t to_length, uint64_t to_addr) "ISD: 0x%08" PRIx64 "@0x%08" PRIx64 " -> 0x%08" PRIx64 "@0x%08" PRIx64
diff --git a/hw/pci-host/Makefile.objs b/hw/pci-host/Makefile.objs
index a9cd3e022d..3e1657774d 100644
--- a/hw/pci-host/Makefile.objs
+++ b/hw/pci-host/Makefile.objs
@@ -17,5 +17,5 @@ common-obj-$(CONFIG_PCI_PIIX) += piix.o
common-obj-$(CONFIG_PCI_EXPRESS_Q35) += q35.o
common-obj-$(CONFIG_PCI_EXPRESS_GENERIC_BRIDGE) += gpex.o
common-obj-$(CONFIG_PCI_EXPRESS_XILINX) += xilinx-pcie.o
-
+common-obj-$(CONFIG_MALTA) += gt64120.o
common-obj-$(CONFIG_PCI_EXPRESS_DESIGNWARE) += designware.o
diff --git a/hw/pci-host/trace-events b/hw/pci-host/trace-events
index d19ca9aef6..eecc233670 100644
--- a/hw/pci-host/trace-events
+++ b/hw/pci-host/trace-events
@@ -20,3 +20,8 @@ unin_data_write(uint64_t addr, unsigned len, uint64_t val) "write addr 0x%"PRIx6
unin_data_read(uint64_t addr, unsigned len, uint64_t val) "read addr 0x%"PRIx64 " len %d val 0x%"PRIx64
unin_write(uint64_t addr, uint64_t value) "addr=0x%" PRIx64 " val=0x%"PRIx64
unin_read(uint64_t addr, uint64_t value) "addr=0x%" PRIx64 " val=0x%"PRIx64
+
+# gt64120.c
+gt64120_read(const char *regname, int width, uint64_t value) "gt64120 read %s value:0x%0*" PRIx64
+gt64120_write(const char *regname, int width, uint64_t value) "gt64120 write %s value:0x%0*" PRIx64
+gt64120_isd_remap(uint64_t from_length, uint64_t from_addr, uint64_t to_length, uint64_t to_addr) "ISD: 0x%08" PRIx64 "@0x%08" PRIx64 " -> 0x%08" PRIx64 "@0x%08" PRIx64
--
2.19.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [RFC PATCH 10/10] hw/pci-host/gt64120: Clean the decoded address space
2019-06-24 22:28 [Qemu-devel] [PATCH 00/10] hw/pci-host: Clean the GT64120 north bridge Philippe Mathieu-Daudé
` (8 preceding siblings ...)
2019-06-24 22:28 ` [Qemu-devel] [PATCH 09/10] hw/mips/gt64xxx_pci: Move it to hw/pci-host/ Philippe Mathieu-Daudé
@ 2019-06-24 22:28 ` Philippe Mathieu-Daudé
2019-07-01 18:06 ` [Qemu-devel] " Philippe Mathieu-Daudé
2019-07-01 17:16 ` [Qemu-devel] [PATCH 00/10] hw/pci-host: Clean the GT64120 north bridge Aleksandar Markovic
10 siblings, 1 reply; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-06-24 22:28 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Thomas Huth, Aleksandar Rikalo,
Philippe Mathieu-Daudé,
Hervé Poussineau, Artyom Tarasenko, Aleksandar Markovic,
Paolo Bonzini, Aurelien Jarno
The SysAd bus is split in various address spaces.
Declare the different regions separately, this helps a lot
while tracing different access while debugging.
We also add the PCI1 ranges.
See 'GT-64120A System Controller' datasheet Rev, 1.1,
"Table 15: CPU and Device Decoder Default Address Mapping"
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
While this device is modelled toward the Malta board, it is generic.
---
hw/mips/mips_malta.c | 6 ------
hw/pci-host/gt64120.c | 19 +++++++++++++++++++
2 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index 97f8ffbf1b..d6e4a0dad9 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -53,7 +53,6 @@
#include "sysemu/qtest.h"
#include "qapi/error.h"
#include "qemu/error-report.h"
-#include "hw/misc/empty_slot.h"
#include "sysemu/kvm.h"
#include "hw/semihosting/semihost.h"
#include "hw/mips/cps.h"
@@ -1209,11 +1208,6 @@ void mips_malta_init(MachineState *machine)
DeviceState *dev = qdev_create(NULL, TYPE_MIPS_MALTA);
MaltaState *s = MIPS_MALTA(dev);
- /* The whole address space decoded by the GT-64120A doesn't generate
- exception when accessing invalid memory. Create an empty slot to
- emulate this feature. */
- empty_slot_init("gt64120-ad", 0x00000000, 0x20000000);
-
qdev_init_nofail(dev);
/* create CPU */
diff --git a/hw/pci-host/gt64120.c b/hw/pci-host/gt64120.c
index 5209038ee5..6eaa571994 100644
--- a/hw/pci-host/gt64120.c
+++ b/hw/pci-host/gt64120.c
@@ -31,6 +31,8 @@
#include "hw/pci/pci_host.h"
#include "hw/i386/pc.h"
#include "exec/address-spaces.h"
+#include "hw/misc/empty_slot.h"
+#include "hw/misc/unimp.h"
#include "trace.h"
#define GT_REGS (0x1000 >> 2)
@@ -1206,6 +1208,23 @@ PCIBus *gt64120_create(qemu_irq *pic, bool target_is_bigendian)
"isd-mem", 0x1000);
pci_create_simple(phb->bus, PCI_DEVFN(0, 0), "gt64120_pci");
+
+ create_unimplemented_device("gt64120_i2o", 0x14000000, 256);
+
+ empty_slot_init("SCS0", 0x00000000, 8 * MiB);
+ empty_slot_init("SCS1", 0x00800000, 8 * MiB);
+ empty_slot_init("SCS2", 0x01000000, 8 * MiB);
+ empty_slot_init("SCS3", 0x01800000, 8 * MiB);
+ empty_slot_init("CS0", 0x1c000000, 8 * MiB);
+ empty_slot_init("CS1", 0x1c800000, 8 * MiB);
+ empty_slot_init("CS2", 0x1d000000, 32 * MiB);
+ empty_slot_init("CS3", 0x1f000000, 12 * MiB);
+ empty_slot_init("BootCS", 0x1fc00000, 4 * MiB);
+
+ create_unimplemented_device("pci1-io", 0x20000000, 32 * MiB);
+ empty_slot_init("pci1-mem0", 0x22000000, 32 * MiB);
+ empty_slot_init("pci1-mem1", 0x24000000, 32 * MiB);
+
return phb->bus;
}
--
2.19.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 01/10] hw/mips/gt64xxx_pci: Fix multiline comment syntax
2019-06-24 22:28 ` [Qemu-devel] [PATCH 01/10] hw/mips/gt64xxx_pci: Fix multiline comment syntax Philippe Mathieu-Daudé
@ 2019-06-25 0:20 ` Aleksandar Markovic
0 siblings, 0 replies; 31+ messages in thread
From: Aleksandar Markovic @ 2019-06-25 0:20 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Peter Maydell, Thomas Huth, Aleksandar Rikalo, qemu-devel,
Hervé Poussineau, Artyom Tarasenko, Aleksandar Markovic,
Paolo Bonzini, Aurelien Jarno
On Jun 25, 2019 12:36 AM, "Philippe Mathieu-Daudé" <f4bug@amsat.org> wrote:
>
> Since commit 8c06fbdf36b checkpatch.pl enforce a new multiline
> comment syntax. Since we'll move this code around, fix its style
> first.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
Yes, I find that this a very good practice (this makes make the file moving
in one of subsequent patches pure moving, which is important for future
“git blames” and similar commands).
Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> hw/mips/gt64xxx_pci.c | 64 +++++++++++++++++++++++--------------------
> 1 file changed, 35 insertions(+), 29 deletions(-)
>
> diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
> index f707e59c7a..c0924646b5 100644
> --- a/hw/mips/gt64xxx_pci.c
> +++ b/hw/mips/gt64xxx_pci.c
> @@ -248,10 +248,11 @@ typedef struct GT64120State {
> } GT64120State;
>
> /* Adjust range to avoid touching space which isn't mappable via PCI */
> -/* XXX: Hardcoded values for Malta: 0x1e000000 - 0x1f100000
> - 0x1fc00000 - 0x1fd00000 */
> -static void check_reserved_space (hwaddr *start,
> - hwaddr *length)
> +/*
> + * XXX: Hardcoded values for Malta: 0x1e000000 - 0x1f100000
> + * 0x1fc00000 - 0x1fd00000
> + */
> +static void check_reserved_space(hwaddr *start, hwaddr *length)
> {
> hwaddr begin = *start;
> hwaddr end = *start + *length;
> @@ -650,8 +651,10 @@ static void gt64120_writel (void *opaque, hwaddr
addr,
> case GT_SDRAM_B1:
> case GT_SDRAM_B2:
> case GT_SDRAM_B3:
> - /* We don't simulate electrical parameters of the SDRAM.
> - Accept, but ignore the values. */
> + /*
> + * We don't simulate electrical parameters of the SDRAM.
> + * Accept, but ignore the values.
> + */
> s->regs[saddr] = val;
> break;
>
> @@ -674,8 +677,10 @@ static uint64_t gt64120_readl (void *opaque,
>
> /* CPU Configuration */
> case GT_MULTI:
> - /* Only one GT64xxx is present on the CPU bus, return
> - the initial value */
> + /*
> + * Only one GT64xxx is present on the CPU bus, return
> + * the initial value.
> + */
> val = s->regs[saddr];
> break;
>
> @@ -685,17 +690,18 @@ static uint64_t gt64120_readl (void *opaque,
> case GT_CPUERR_DATALO:
> case GT_CPUERR_DATAHI:
> case GT_CPUERR_PARITY:
> - /* Emulated memory has no error, always return the initial
> - values */
> + /* Emulated memory has no error, always return the initial
values. */
> val = s->regs[saddr];
> break;
>
> /* CPU Sync Barrier */
> case GT_PCI0SYNC:
> case GT_PCI1SYNC:
> - /* Reading those register should empty all FIFO on the PCI
> - bus, which are not emulated. The return value should be
> - a random value that should be ignored. */
> + /*
> + * Reading those register should empty all FIFO on the PCI
> + * bus, which are not emulated. The return value should be
> + * a random value that should be ignored.
> + */
> val = 0xc000ffee;
> break;
>
> @@ -705,8 +711,7 @@ static uint64_t gt64120_readl (void *opaque,
> case GT_ECC_MEM:
> case GT_ECC_CALC:
> case GT_ECC_ERRADDR:
> - /* Emulated memory has no error, always return the initial
> - values */
> + /* Emulated memory has no error, always return the initial
values. */
> val = s->regs[saddr];
> break;
>
> @@ -785,8 +790,10 @@ static uint64_t gt64120_readl (void *opaque,
> case GT_SDRAM_B1:
> case GT_SDRAM_B2:
> case GT_SDRAM_B3:
> - /* We don't simulate electrical parameters of the SDRAM.
> - Just return the last written value. */
> + /*
> + * We don't simulate electrical parameters of the SDRAM.
> + * Just return the last written value.
> + */
> val = s->regs[saddr];
> break;
>
> @@ -949,20 +956,20 @@ static int gt64120_pci_map_irq(PCIDevice *pci_dev,
int irq_num)
> slot = (pci_dev->devfn >> 3);
>
> switch (slot) {
> - /* PIIX4 USB */
> - case 10:
> + /* PIIX4 USB */
> + case 10:
> return 3;
> - /* AMD 79C973 Ethernet */
> - case 11:
> + /* AMD 79C973 Ethernet */
> + case 11:
> return 1;
> - /* Crystal 4281 Sound */
> - case 12:
> + /* Crystal 4281 Sound */
> + case 12:
> return 2;
> - /* PCI slot 1 to 4 */
> - case 18 ... 21:
> + /* PCI slot 1 to 4 */
> + case 18 ... 21:
> return ((slot - 18) + irq_num) & 0x03;
> - /* Unknown device, don't do any translation */
> - default:
> + /* Unknown device, don't do any translation */
> + default:
> return irq_num;
> }
> }
> @@ -980,8 +987,7 @@ static void gt64120_pci_set_irq(void *opaque, int
irq_num, int level)
> /* XXX: optimize */
> pic_irq = piix4_dev->config[0x60 + irq_num];
> if (pic_irq < 16) {
> - /* The pic level is the logical OR of all the PCI irqs mapped
> - to it */
> + /* The pic level is the logical OR of all the PCI irqs mapped to
it. */
> pic_level = 0;
> for (i = 0; i < 4; i++) {
> if (pic_irq == piix4_dev->config[0x60 + i])
> --
> 2.19.1
>
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 02/10] hw/mips/gt64xxx_pci: Fix 'tabs' coding style issues
2019-06-24 22:28 ` [Qemu-devel] [PATCH 02/10] hw/mips/gt64xxx_pci: Fix 'tabs' coding style issues Philippe Mathieu-Daudé
@ 2019-06-25 0:21 ` Aleksandar Markovic
0 siblings, 0 replies; 31+ messages in thread
From: Aleksandar Markovic @ 2019-06-25 0:21 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Peter Maydell, Thomas Huth, Aleksandar Rikalo, qemu-devel,
Hervé Poussineau, Artyom Tarasenko, Aleksandar Markovic,
Paolo Bonzini, Aurelien Jarno
On Jun 25, 2019 12:30 AM, "Philippe Mathieu-Daudé" <f4bug@amsat.org> wrote:
>
> Since we'll move this code around, fix its style first:
>
> ERROR: code indent should never use tabs
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> hw/mips/gt64xxx_pci.c | 312 +++++++++++++++++++++---------------------
> 1 file changed, 156 insertions(+), 156 deletions(-)
>
> diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
> index c0924646b5..bbd719f091 100644
> --- a/hw/mips/gt64xxx_pci.c
> +++ b/hw/mips/gt64xxx_pci.c
> @@ -38,192 +38,192 @@
> #define DPRINTF(fmt, ...)
> #endif
>
> -#define GT_REGS (0x1000 >> 2)
> +#define GT_REGS (0x1000 >> 2)
>
> /* CPU Configuration */
> -#define GT_CPU (0x000 >> 2)
> -#define GT_MULTI (0x120 >> 2)
> +#define GT_CPU (0x000 >> 2)
> +#define GT_MULTI (0x120 >> 2)
>
> /* CPU Address Decode */
> -#define GT_SCS10LD (0x008 >> 2)
> -#define GT_SCS10HD (0x010 >> 2)
> -#define GT_SCS32LD (0x018 >> 2)
> -#define GT_SCS32HD (0x020 >> 2)
> -#define GT_CS20LD (0x028 >> 2)
> -#define GT_CS20HD (0x030 >> 2)
> -#define GT_CS3BOOTLD (0x038 >> 2)
> -#define GT_CS3BOOTHD (0x040 >> 2)
> -#define GT_PCI0IOLD (0x048 >> 2)
> -#define GT_PCI0IOHD (0x050 >> 2)
> -#define GT_PCI0M0LD (0x058 >> 2)
> -#define GT_PCI0M0HD (0x060 >> 2)
> -#define GT_PCI0M1LD (0x080 >> 2)
> -#define GT_PCI0M1HD (0x088 >> 2)
> -#define GT_PCI1IOLD (0x090 >> 2)
> -#define GT_PCI1IOHD (0x098 >> 2)
> -#define GT_PCI1M0LD (0x0a0 >> 2)
> -#define GT_PCI1M0HD (0x0a8 >> 2)
> -#define GT_PCI1M1LD (0x0b0 >> 2)
> -#define GT_PCI1M1HD (0x0b8 >> 2)
> -#define GT_ISD (0x068 >> 2)
> -
> -#define GT_SCS10AR (0x0d0 >> 2)
> -#define GT_SCS32AR (0x0d8 >> 2)
> -#define GT_CS20R (0x0e0 >> 2)
> -#define GT_CS3BOOTR (0x0e8 >> 2)
> -
> -#define GT_PCI0IOREMAP (0x0f0 >> 2)
> -#define GT_PCI0M0REMAP (0x0f8 >> 2)
> -#define GT_PCI0M1REMAP (0x100 >> 2)
> -#define GT_PCI1IOREMAP (0x108 >> 2)
> -#define GT_PCI1M0REMAP (0x110 >> 2)
> -#define GT_PCI1M1REMAP (0x118 >> 2)
> +#define GT_SCS10LD (0x008 >> 2)
> +#define GT_SCS10HD (0x010 >> 2)
> +#define GT_SCS32LD (0x018 >> 2)
> +#define GT_SCS32HD (0x020 >> 2)
> +#define GT_CS20LD (0x028 >> 2)
> +#define GT_CS20HD (0x030 >> 2)
> +#define GT_CS3BOOTLD (0x038 >> 2)
> +#define GT_CS3BOOTHD (0x040 >> 2)
> +#define GT_PCI0IOLD (0x048 >> 2)
> +#define GT_PCI0IOHD (0x050 >> 2)
> +#define GT_PCI0M0LD (0x058 >> 2)
> +#define GT_PCI0M0HD (0x060 >> 2)
> +#define GT_PCI0M1LD (0x080 >> 2)
> +#define GT_PCI0M1HD (0x088 >> 2)
> +#define GT_PCI1IOLD (0x090 >> 2)
> +#define GT_PCI1IOHD (0x098 >> 2)
> +#define GT_PCI1M0LD (0x0a0 >> 2)
> +#define GT_PCI1M0HD (0x0a8 >> 2)
> +#define GT_PCI1M1LD (0x0b0 >> 2)
> +#define GT_PCI1M1HD (0x0b8 >> 2)
> +#define GT_ISD (0x068 >> 2)
> +
> +#define GT_SCS10AR (0x0d0 >> 2)
> +#define GT_SCS32AR (0x0d8 >> 2)
> +#define GT_CS20R (0x0e0 >> 2)
> +#define GT_CS3BOOTR (0x0e8 >> 2)
> +
> +#define GT_PCI0IOREMAP (0x0f0 >> 2)
> +#define GT_PCI0M0REMAP (0x0f8 >> 2)
> +#define GT_PCI0M1REMAP (0x100 >> 2)
> +#define GT_PCI1IOREMAP (0x108 >> 2)
> +#define GT_PCI1M0REMAP (0x110 >> 2)
> +#define GT_PCI1M1REMAP (0x118 >> 2)
>
> /* CPU Error Report */
> -#define GT_CPUERR_ADDRLO (0x070 >> 2)
> -#define GT_CPUERR_ADDRHI (0x078 >> 2)
> -#define GT_CPUERR_DATALO (0x128 >> 2) /* GT-64120A
only */
> -#define GT_CPUERR_DATAHI (0x130 >> 2) /* GT-64120A
only */
> -#define GT_CPUERR_PARITY (0x138 >> 2) /* GT-64120A
only */
> +#define GT_CPUERR_ADDRLO (0x070 >> 2)
> +#define GT_CPUERR_ADDRHI (0x078 >> 2)
> +#define GT_CPUERR_DATALO (0x128 >> 2) /* GT-64120A only */
> +#define GT_CPUERR_DATAHI (0x130 >> 2) /* GT-64120A only */
> +#define GT_CPUERR_PARITY (0x138 >> 2) /* GT-64120A only */
>
> /* CPU Sync Barrier */
> -#define GT_PCI0SYNC (0x0c0 >> 2)
> -#define GT_PCI1SYNC (0x0c8 >> 2)
> +#define GT_PCI0SYNC (0x0c0 >> 2)
> +#define GT_PCI1SYNC (0x0c8 >> 2)
>
> /* SDRAM and Device Address Decode */
> -#define GT_SCS0LD (0x400 >> 2)
> -#define GT_SCS0HD (0x404 >> 2)
> -#define GT_SCS1LD (0x408 >> 2)
> -#define GT_SCS1HD (0x40c >> 2)
> -#define GT_SCS2LD (0x410 >> 2)
> -#define GT_SCS2HD (0x414 >> 2)
> -#define GT_SCS3LD (0x418 >> 2)
> -#define GT_SCS3HD (0x41c >> 2)
> -#define GT_CS0LD (0x420 >> 2)
> -#define GT_CS0HD (0x424 >> 2)
> -#define GT_CS1LD (0x428 >> 2)
> -#define GT_CS1HD (0x42c >> 2)
> -#define GT_CS2LD (0x430 >> 2)
> -#define GT_CS2HD (0x434 >> 2)
> -#define GT_CS3LD (0x438 >> 2)
> -#define GT_CS3HD (0x43c >> 2)
> -#define GT_BOOTLD (0x440 >> 2)
> -#define GT_BOOTHD (0x444 >> 2)
> -#define GT_ADERR (0x470 >> 2)
> +#define GT_SCS0LD (0x400 >> 2)
> +#define GT_SCS0HD (0x404 >> 2)
> +#define GT_SCS1LD (0x408 >> 2)
> +#define GT_SCS1HD (0x40c >> 2)
> +#define GT_SCS2LD (0x410 >> 2)
> +#define GT_SCS2HD (0x414 >> 2)
> +#define GT_SCS3LD (0x418 >> 2)
> +#define GT_SCS3HD (0x41c >> 2)
> +#define GT_CS0LD (0x420 >> 2)
> +#define GT_CS0HD (0x424 >> 2)
> +#define GT_CS1LD (0x428 >> 2)
> +#define GT_CS1HD (0x42c >> 2)
> +#define GT_CS2LD (0x430 >> 2)
> +#define GT_CS2HD (0x434 >> 2)
> +#define GT_CS3LD (0x438 >> 2)
> +#define GT_CS3HD (0x43c >> 2)
> +#define GT_BOOTLD (0x440 >> 2)
> +#define GT_BOOTHD (0x444 >> 2)
> +#define GT_ADERR (0x470 >> 2)
>
> /* SDRAM Configuration */
> -#define GT_SDRAM_CFG (0x448 >> 2)
> -#define GT_SDRAM_OPMODE (0x474 >> 2)
> -#define GT_SDRAM_BM (0x478 >> 2)
> -#define GT_SDRAM_ADDRDECODE (0x47c >> 2)
> +#define GT_SDRAM_CFG (0x448 >> 2)
> +#define GT_SDRAM_OPMODE (0x474 >> 2)
> +#define GT_SDRAM_BM (0x478 >> 2)
> +#define GT_SDRAM_ADDRDECODE (0x47c >> 2)
>
> /* SDRAM Parameters */
> -#define GT_SDRAM_B0 (0x44c >> 2)
> -#define GT_SDRAM_B1 (0x450 >> 2)
> -#define GT_SDRAM_B2 (0x454 >> 2)
> -#define GT_SDRAM_B3 (0x458 >> 2)
> +#define GT_SDRAM_B0 (0x44c >> 2)
> +#define GT_SDRAM_B1 (0x450 >> 2)
> +#define GT_SDRAM_B2 (0x454 >> 2)
> +#define GT_SDRAM_B3 (0x458 >> 2)
>
> /* Device Parameters */
> -#define GT_DEV_B0 (0x45c >> 2)
> -#define GT_DEV_B1 (0x460 >> 2)
> -#define GT_DEV_B2 (0x464 >> 2)
> -#define GT_DEV_B3 (0x468 >> 2)
> -#define GT_DEV_BOOT (0x46c >> 2)
> +#define GT_DEV_B0 (0x45c >> 2)
> +#define GT_DEV_B1 (0x460 >> 2)
> +#define GT_DEV_B2 (0x464 >> 2)
> +#define GT_DEV_B3 (0x468 >> 2)
> +#define GT_DEV_BOOT (0x46c >> 2)
>
> /* ECC */
> -#define GT_ECC_ERRDATALO (0x480 >> 2) /* GT-64120A
only */
> -#define GT_ECC_ERRDATAHI (0x484 >> 2) /* GT-64120A
only */
> -#define GT_ECC_MEM (0x488 >> 2) /* GT-64120A
only */
> -#define GT_ECC_CALC (0x48c >> 2) /* GT-64120A
only */
> -#define GT_ECC_ERRADDR (0x490 >> 2) /* GT-64120A
only */
> +#define GT_ECC_ERRDATALO (0x480 >> 2) /* GT-64120A only */
> +#define GT_ECC_ERRDATAHI (0x484 >> 2) /* GT-64120A only */
> +#define GT_ECC_MEM (0x488 >> 2) /* GT-64120A only */
> +#define GT_ECC_CALC (0x48c >> 2) /* GT-64120A only */
> +#define GT_ECC_ERRADDR (0x490 >> 2) /* GT-64120A only */
>
> /* DMA Record */
> -#define GT_DMA0_CNT (0x800 >> 2)
> -#define GT_DMA1_CNT (0x804 >> 2)
> -#define GT_DMA2_CNT (0x808 >> 2)
> -#define GT_DMA3_CNT (0x80c >> 2)
> -#define GT_DMA0_SA (0x810 >> 2)
> -#define GT_DMA1_SA (0x814 >> 2)
> -#define GT_DMA2_SA (0x818 >> 2)
> -#define GT_DMA3_SA (0x81c >> 2)
> -#define GT_DMA0_DA (0x820 >> 2)
> -#define GT_DMA1_DA (0x824 >> 2)
> -#define GT_DMA2_DA (0x828 >> 2)
> -#define GT_DMA3_DA (0x82c >> 2)
> -#define GT_DMA0_NEXT (0x830 >> 2)
> -#define GT_DMA1_NEXT (0x834 >> 2)
> -#define GT_DMA2_NEXT (0x838 >> 2)
> -#define GT_DMA3_NEXT (0x83c >> 2)
> -#define GT_DMA0_CUR (0x870 >> 2)
> -#define GT_DMA1_CUR (0x874 >> 2)
> -#define GT_DMA2_CUR (0x878 >> 2)
> -#define GT_DMA3_CUR (0x87c >> 2)
> +#define GT_DMA0_CNT (0x800 >> 2)
> +#define GT_DMA1_CNT (0x804 >> 2)
> +#define GT_DMA2_CNT (0x808 >> 2)
> +#define GT_DMA3_CNT (0x80c >> 2)
> +#define GT_DMA0_SA (0x810 >> 2)
> +#define GT_DMA1_SA (0x814 >> 2)
> +#define GT_DMA2_SA (0x818 >> 2)
> +#define GT_DMA3_SA (0x81c >> 2)
> +#define GT_DMA0_DA (0x820 >> 2)
> +#define GT_DMA1_DA (0x824 >> 2)
> +#define GT_DMA2_DA (0x828 >> 2)
> +#define GT_DMA3_DA (0x82c >> 2)
> +#define GT_DMA0_NEXT (0x830 >> 2)
> +#define GT_DMA1_NEXT (0x834 >> 2)
> +#define GT_DMA2_NEXT (0x838 >> 2)
> +#define GT_DMA3_NEXT (0x83c >> 2)
> +#define GT_DMA0_CUR (0x870 >> 2)
> +#define GT_DMA1_CUR (0x874 >> 2)
> +#define GT_DMA2_CUR (0x878 >> 2)
> +#define GT_DMA3_CUR (0x87c >> 2)
>
> /* DMA Channel Control */
> -#define GT_DMA0_CTRL (0x840 >> 2)
> -#define GT_DMA1_CTRL (0x844 >> 2)
> -#define GT_DMA2_CTRL (0x848 >> 2)
> -#define GT_DMA3_CTRL (0x84c >> 2)
> +#define GT_DMA0_CTRL (0x840 >> 2)
> +#define GT_DMA1_CTRL (0x844 >> 2)
> +#define GT_DMA2_CTRL (0x848 >> 2)
> +#define GT_DMA3_CTRL (0x84c >> 2)
>
> /* DMA Arbiter */
> -#define GT_DMA_ARB (0x860 >> 2)
> +#define GT_DMA_ARB (0x860 >> 2)
>
> /* Timer/Counter */
> -#define GT_TC0 (0x850 >> 2)
> -#define GT_TC1 (0x854 >> 2)
> -#define GT_TC2 (0x858 >> 2)
> -#define GT_TC3 (0x85c >> 2)
> -#define GT_TC_CONTROL (0x864 >> 2)
> +#define GT_TC0 (0x850 >> 2)
> +#define GT_TC1 (0x854 >> 2)
> +#define GT_TC2 (0x858 >> 2)
> +#define GT_TC3 (0x85c >> 2)
> +#define GT_TC_CONTROL (0x864 >> 2)
>
> /* PCI Internal */
> -#define GT_PCI0_CMD (0xc00 >> 2)
> -#define GT_PCI0_TOR (0xc04 >> 2)
> -#define GT_PCI0_BS_SCS10 (0xc08 >> 2)
> -#define GT_PCI0_BS_SCS32 (0xc0c >> 2)
> -#define GT_PCI0_BS_CS20 (0xc10 >> 2)
> -#define GT_PCI0_BS_CS3BT (0xc14 >> 2)
> -#define GT_PCI1_IACK (0xc30 >> 2)
> -#define GT_PCI0_IACK (0xc34 >> 2)
> -#define GT_PCI0_BARE (0xc3c >> 2)
> -#define GT_PCI0_PREFMBR (0xc40 >> 2)
> -#define GT_PCI0_SCS10_BAR (0xc48 >> 2)
> -#define GT_PCI0_SCS32_BAR (0xc4c >> 2)
> -#define GT_PCI0_CS20_BAR (0xc50 >> 2)
> -#define GT_PCI0_CS3BT_BAR (0xc54 >> 2)
> -#define GT_PCI0_SSCS10_BAR (0xc58 >> 2)
> -#define GT_PCI0_SSCS32_BAR (0xc5c >> 2)
> -#define GT_PCI0_SCS3BT_BAR (0xc64 >> 2)
> -#define GT_PCI1_CMD (0xc80 >> 2)
> -#define GT_PCI1_TOR (0xc84 >> 2)
> -#define GT_PCI1_BS_SCS10 (0xc88 >> 2)
> -#define GT_PCI1_BS_SCS32 (0xc8c >> 2)
> -#define GT_PCI1_BS_CS20 (0xc90 >> 2)
> -#define GT_PCI1_BS_CS3BT (0xc94 >> 2)
> -#define GT_PCI1_BARE (0xcbc >> 2)
> -#define GT_PCI1_PREFMBR (0xcc0 >> 2)
> -#define GT_PCI1_SCS10_BAR (0xcc8 >> 2)
> -#define GT_PCI1_SCS32_BAR (0xccc >> 2)
> -#define GT_PCI1_CS20_BAR (0xcd0 >> 2)
> -#define GT_PCI1_CS3BT_BAR (0xcd4 >> 2)
> -#define GT_PCI1_SSCS10_BAR (0xcd8 >> 2)
> -#define GT_PCI1_SSCS32_BAR (0xcdc >> 2)
> -#define GT_PCI1_SCS3BT_BAR (0xce4 >> 2)
> -#define GT_PCI1_CFGADDR (0xcf0 >> 2)
> -#define GT_PCI1_CFGDATA (0xcf4 >> 2)
> -#define GT_PCI0_CFGADDR (0xcf8 >> 2)
> -#define GT_PCI0_CFGDATA (0xcfc >> 2)
> +#define GT_PCI0_CMD (0xc00 >> 2)
> +#define GT_PCI0_TOR (0xc04 >> 2)
> +#define GT_PCI0_BS_SCS10 (0xc08 >> 2)
> +#define GT_PCI0_BS_SCS32 (0xc0c >> 2)
> +#define GT_PCI0_BS_CS20 (0xc10 >> 2)
> +#define GT_PCI0_BS_CS3BT (0xc14 >> 2)
> +#define GT_PCI1_IACK (0xc30 >> 2)
> +#define GT_PCI0_IACK (0xc34 >> 2)
> +#define GT_PCI0_BARE (0xc3c >> 2)
> +#define GT_PCI0_PREFMBR (0xc40 >> 2)
> +#define GT_PCI0_SCS10_BAR (0xc48 >> 2)
> +#define GT_PCI0_SCS32_BAR (0xc4c >> 2)
> +#define GT_PCI0_CS20_BAR (0xc50 >> 2)
> +#define GT_PCI0_CS3BT_BAR (0xc54 >> 2)
> +#define GT_PCI0_SSCS10_BAR (0xc58 >> 2)
> +#define GT_PCI0_SSCS32_BAR (0xc5c >> 2)
> +#define GT_PCI0_SCS3BT_BAR (0xc64 >> 2)
> +#define GT_PCI1_CMD (0xc80 >> 2)
> +#define GT_PCI1_TOR (0xc84 >> 2)
> +#define GT_PCI1_BS_SCS10 (0xc88 >> 2)
> +#define GT_PCI1_BS_SCS32 (0xc8c >> 2)
> +#define GT_PCI1_BS_CS20 (0xc90 >> 2)
> +#define GT_PCI1_BS_CS3BT (0xc94 >> 2)
> +#define GT_PCI1_BARE (0xcbc >> 2)
> +#define GT_PCI1_PREFMBR (0xcc0 >> 2)
> +#define GT_PCI1_SCS10_BAR (0xcc8 >> 2)
> +#define GT_PCI1_SCS32_BAR (0xccc >> 2)
> +#define GT_PCI1_CS20_BAR (0xcd0 >> 2)
> +#define GT_PCI1_CS3BT_BAR (0xcd4 >> 2)
> +#define GT_PCI1_SSCS10_BAR (0xcd8 >> 2)
> +#define GT_PCI1_SSCS32_BAR (0xcdc >> 2)
> +#define GT_PCI1_SCS3BT_BAR (0xce4 >> 2)
> +#define GT_PCI1_CFGADDR (0xcf0 >> 2)
> +#define GT_PCI1_CFGDATA (0xcf4 >> 2)
> +#define GT_PCI0_CFGADDR (0xcf8 >> 2)
> +#define GT_PCI0_CFGDATA (0xcfc >> 2)
>
> /* Interrupts */
> -#define GT_INTRCAUSE (0xc18 >> 2)
> -#define GT_INTRMASK (0xc1c >> 2)
> -#define GT_PCI0_ICMASK (0xc24 >> 2)
> -#define GT_PCI0_SERR0MASK (0xc28 >> 2)
> -#define GT_CPU_INTSEL (0xc70 >> 2)
> -#define GT_PCI0_INTSEL (0xc74 >> 2)
> -#define GT_HINTRCAUSE (0xc98 >> 2)
> -#define GT_HINTRMASK (0xc9c >> 2)
> -#define GT_PCI0_HICMASK (0xca4 >> 2)
> -#define GT_PCI1_SERR1MASK (0xca8 >> 2)
> +#define GT_INTRCAUSE (0xc18 >> 2)
> +#define GT_INTRMASK (0xc1c >> 2)
> +#define GT_PCI0_ICMASK (0xc24 >> 2)
> +#define GT_PCI0_SERR0MASK (0xc28 >> 2)
> +#define GT_CPU_INTSEL (0xc70 >> 2)
> +#define GT_PCI0_INTSEL (0xc74 >> 2)
> +#define GT_HINTRCAUSE (0xc98 >> 2)
> +#define GT_HINTRMASK (0xc9c >> 2)
> +#define GT_PCI0_HICMASK (0xca4 >> 2)
> +#define GT_PCI1_SERR1MASK (0xca8 >> 2)
>
> #define PCI_MAPPING_ENTRY(regname) \
> hwaddr regname ##_start; \
> --
> 2.19.1
>
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 03/10] hw/mips/gt64xxx_pci: Fix 'braces' coding style issues
2019-06-24 22:28 ` [Qemu-devel] [PATCH 03/10] hw/mips/gt64xxx_pci: Fix 'braces' " Philippe Mathieu-Daudé
@ 2019-06-25 0:22 ` Aleksandar Markovic
0 siblings, 0 replies; 31+ messages in thread
From: Aleksandar Markovic @ 2019-06-25 0:22 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Peter Maydell, Thomas Huth, Aleksandar Rikalo, qemu-devel,
Hervé Poussineau, Artyom Tarasenko, Aleksandar Markovic,
Paolo Bonzini, Aurelien Jarno
On Jun 25, 2019 12:38 AM, "Philippe Mathieu-Daudé" <f4bug@amsat.org> wrote:
>
> Since we'll move this code around, fix its style first:
>
> ERROR: braces {} are necessary for all arms of this statement
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> hw/mips/gt64xxx_pci.c | 27 ++++++++++++++++++---------
> 1 file changed, 18 insertions(+), 9 deletions(-)
>
> diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
> index bbd719f091..cfd497960c 100644
> --- a/hw/mips/gt64xxx_pci.c
> +++ b/hw/mips/gt64xxx_pci.c
> @@ -257,19 +257,25 @@ static void check_reserved_space(hwaddr *start,
hwaddr *length)
> hwaddr begin = *start;
> hwaddr end = *start + *length;
>
> - if (end >= 0x1e000000LL && end < 0x1f100000LL)
> + if (end >= 0x1e000000LL && end < 0x1f100000LL) {
> end = 0x1e000000LL;
> - if (begin >= 0x1e000000LL && begin < 0x1f100000LL)
> + }
> + if (begin >= 0x1e000000LL && begin < 0x1f100000LL) {
> begin = 0x1f100000LL;
> - if (end >= 0x1fc00000LL && end < 0x1fd00000LL)
> + }
> + if (end >= 0x1fc00000LL && end < 0x1fd00000LL) {
> end = 0x1fc00000LL;
> - if (begin >= 0x1fc00000LL && begin < 0x1fd00000LL)
> + }
> + if (begin >= 0x1fc00000LL && begin < 0x1fd00000LL) {
> begin = 0x1fd00000LL;
> + }
> /* XXX: This is broken when a reserved range splits the requested
range */
> - if (end >= 0x1f100000LL && begin < 0x1e000000LL)
> + if (end >= 0x1f100000LL && begin < 0x1e000000LL) {
> end = 0x1e000000LL;
> - if (end >= 0x1fd00000LL && begin < 0x1fc00000LL)
> + }
> + if (end >= 0x1fd00000LL && begin < 0x1fc00000LL) {
> end = 0x1fc00000LL;
> + }
>
> *start = begin;
> *length = end - begin;
> @@ -385,8 +391,9 @@ static void gt64120_writel (void *opaque, hwaddr addr,
> PCIHostState *phb = PCI_HOST_BRIDGE(s);
> uint32_t saddr;
>
> - if (!(s->regs[GT_CPU] & 0x00001000))
> + if (!(s->regs[GT_CPU] & 0x00001000)) {
> val = bswap32(val);
> + }
>
> saddr = (addr & 0xfff) >> 2;
> switch (saddr) {
> @@ -937,8 +944,9 @@ static uint64_t gt64120_readl (void *opaque,
> break;
> }
>
> - if (!(s->regs[GT_CPU] & 0x00001000))
> + if (!(s->regs[GT_CPU] & 0x00001000)) {
> val = bswap32(val);
> + }
>
> return val;
> }
> @@ -990,8 +998,9 @@ static void gt64120_pci_set_irq(void *opaque, int
irq_num, int level)
> /* The pic level is the logical OR of all the PCI irqs mapped to
it. */
> pic_level = 0;
> for (i = 0; i < 4; i++) {
> - if (pic_irq == piix4_dev->config[0x60 + i])
> + if (pic_irq == piix4_dev->config[0x60 + i]) {
> pic_level |= pci_irq_levels[i];
> + }
> }
> qemu_set_irq(pic[pic_irq], pic_level);
> }
> --
> 2.19.1
>
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 04/10] hw/mips/gt64xxx_pci: Fix 'spaces' coding style issues
2019-06-24 22:28 ` [Qemu-devel] [PATCH 04/10] hw/mips/gt64xxx_pci: Fix 'spaces' " Philippe Mathieu-Daudé
@ 2019-06-25 0:23 ` Aleksandar Markovic
0 siblings, 0 replies; 31+ messages in thread
From: Aleksandar Markovic @ 2019-06-25 0:23 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Peter Maydell, Thomas Huth, Aleksandar Rikalo, qemu-devel,
Hervé Poussineau, Artyom Tarasenko, Aleksandar Markovic,
Paolo Bonzini, Aurelien Jarno
On Jun 25, 2019 12:29 AM, "Philippe Mathieu-Daudé" <f4bug@amsat.org> wrote:
>
> Since we'll move this code around, fix its style first:
>
> ERROR: space prohibited between function name and open parenthesis
> ERROR: line over 90 characters
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> hw/mips/gt64xxx_pci.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
> index cfd497960c..0b9fb02475 100644
> --- a/hw/mips/gt64xxx_pci.c
> +++ b/hw/mips/gt64xxx_pci.c
> @@ -384,8 +384,8 @@ static const VMStateDescription vmstate_gt64120 = {
> }
> };
>
> -static void gt64120_writel (void *opaque, hwaddr addr,
> - uint64_t val, unsigned size)
> +static void gt64120_writel(void *opaque, hwaddr addr,
> + uint64_t val, unsigned size)
> {
> GT64120State *s = opaque;
> PCIHostState *phb = PCI_HOST_BRIDGE(s);
> @@ -671,8 +671,8 @@ static void gt64120_writel (void *opaque, hwaddr addr,
> }
> }
>
> -static uint64_t gt64120_readl (void *opaque,
> - hwaddr addr, unsigned size)
> +static uint64_t gt64120_readl(void *opaque,
> + hwaddr addr, unsigned size)
> {
> GT64120State *s = opaque;
> PCIHostState *phb = PCI_HOST_BRIDGE(s);
> @@ -1193,7 +1193,8 @@ PCIBus *gt64120_register(qemu_irq *pic)
> get_system_io(),
> PCI_DEVFN(18, 0), 4, TYPE_PCI_BUS);
> qdev_init_nofail(dev);
> - memory_region_init_io(&d->ISD_mem, OBJECT(dev), &isd_mem_ops, d,
"isd-mem", 0x1000);
> + memory_region_init_io(&d->ISD_mem, OBJECT(dev), &isd_mem_ops, d,
> + "isd-mem", 0x1000);
>
> pci_create_simple(phb->bus, PCI_DEVFN(0, 0), "gt64120_pci");
> return phb->bus;
> --
> 2.19.1
>
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 05/10] hw/mips/gt64xxx_pci: Use qemu_log_mask() instead of debug printf()
2019-06-24 22:28 ` [Qemu-devel] [PATCH 05/10] hw/mips/gt64xxx_pci: Use qemu_log_mask() instead of debug printf() Philippe Mathieu-Daudé
@ 2019-06-25 0:37 ` Aleksandar Markovic
2019-06-25 7:14 ` Philippe Mathieu-Daudé
2019-06-25 18:16 ` Aleksandar Markovic
1 sibling, 1 reply; 31+ messages in thread
From: Aleksandar Markovic @ 2019-06-25 0:37 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Peter Maydell, Thomas Huth, Aleksandar Rikalo, qemu-devel,
Hervé Poussineau, Artyom Tarasenko, Aleksandar Markovic,
Paolo Bonzini, Aurelien Jarno
On Jun 25, 2019 12:42 AM, "Philippe Mathieu-Daudé" <f4bug@amsat.org> wrote:
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
This patch is not only mechanical replacement of printf(), but it also
improves existing log messages, and adds some new ones as well. Reflect
that in both commit message title and body. Perhaps there are more spots
that deserve logging. But, also, please, Philippe, doublecheck in real
scenarios if we don't flood the log with too many messages.
Thank you,
Aleksandar
> hw/mips/gt64xxx_pci.c | 48 +++++++++++++++++++++++++++++++++----------
> 1 file changed, 37 insertions(+), 11 deletions(-)
>
> diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
> index 0b9fb02475..f44326f14f 100644
> --- a/hw/mips/gt64xxx_pci.c
> +++ b/hw/mips/gt64xxx_pci.c
> @@ -23,6 +23,7 @@
> */
>
> #include "qemu/osdep.h"
> +#include "qemu/log.h"
> #include "hw/hw.h"
> #include "hw/mips/mips.h"
> #include "hw/pci/pci.h"
> @@ -466,12 +467,20 @@ static void gt64120_writel(void *opaque, hwaddr
addr,
> case GT_CPUERR_DATAHI:
> case GT_CPUERR_PARITY:
> /* Read-only registers, do nothing */
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "gt64120: Read-only register write "
> + "reg:0x03%x size:%u value:0x%0*" PRIx64 "\n",
> + saddr << 2, size, size << 1, val);
> break;
>
> /* CPU Sync Barrier */
> case GT_PCI0SYNC:
> case GT_PCI1SYNC:
> /* Read-only registers, do nothing */
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "gt64120: Read-only register write "
> + "reg:0x03%x size:%u value:0x%0*" PRIx64 "\n",
> + saddr << 2, size, size << 1, val);
> break;
>
> /* SDRAM and Device Address Decode */
> @@ -510,7 +519,10 @@ static void gt64120_writel(void *opaque, hwaddr addr,
> case GT_DEV_B3:
> case GT_DEV_BOOT:
> /* Not implemented */
> - DPRINTF ("Unimplemented device register offset 0x%x\n", saddr <<
2);
> + qemu_log_mask(LOG_UNIMP,
> + "gt64120: Unimplemented device register write "
> + "reg:0x03%x size:%u value:0x%0*" PRIx64 "\n",
> + saddr << 2, size, size << 1, val);
> break;
>
> /* ECC */
> @@ -520,6 +532,10 @@ static void gt64120_writel(void *opaque, hwaddr addr,
> case GT_ECC_CALC:
> case GT_ECC_ERRADDR:
> /* Read-only registers, do nothing */
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "gt64120: Read-only register write "
> + "reg:0x03%x size:%u value:0x%0*" PRIx64 "\n",
> + saddr << 2, size, size << 1, val);
> break;
>
> /* DMA Record */
> @@ -543,23 +559,20 @@ static void gt64120_writel(void *opaque, hwaddr
addr,
> case GT_DMA1_CUR:
> case GT_DMA2_CUR:
> case GT_DMA3_CUR:
> - /* Not implemented */
> - DPRINTF ("Unimplemented DMA register offset 0x%x\n", saddr << 2);
> - break;
>
> /* DMA Channel Control */
> case GT_DMA0_CTRL:
> case GT_DMA1_CTRL:
> case GT_DMA2_CTRL:
> case GT_DMA3_CTRL:
> - /* Not implemented */
> - DPRINTF ("Unimplemented DMA register offset 0x%x\n", saddr << 2);
> - break;
>
> /* DMA Arbiter */
> case GT_DMA_ARB:
> /* Not implemented */
> - DPRINTF ("Unimplemented DMA register offset 0x%x\n", saddr << 2);
> + qemu_log_mask(LOG_UNIMP,
> + "gt64120: Unimplemented DMA register write "
> + "reg:0x03%x size:%u value:0x%0*" PRIx64 "\n",
> + saddr << 2, size, size << 1, val);
> break;
>
> /* Timer/Counter */
> @@ -569,7 +582,10 @@ static void gt64120_writel(void *opaque, hwaddr addr,
> case GT_TC3:
> case GT_TC_CONTROL:
> /* Not implemented */
> - DPRINTF ("Unimplemented timer register offset 0x%x\n", saddr <<
2);
> + qemu_log_mask(LOG_UNIMP,
> + "gt64120: Unimplemented timer register write "
> + "reg:0x03%x size:%u value:0x%0*" PRIx64 "\n",
> + saddr << 2, size, size << 1, val);
> break;
>
> /* PCI Internal */
> @@ -610,6 +626,10 @@ static void gt64120_writel(void *opaque, hwaddr addr,
> case GT_PCI1_CFGADDR:
> case GT_PCI1_CFGDATA:
> /* not implemented */
> + qemu_log_mask(LOG_UNIMP,
> + "gt64120: Unimplemented timer register write "
> + "reg:0x03%x size:%u value:0x%0*" PRIx64 "\n",
> + saddr << 2, size, size << 1, val);
> break;
> case GT_PCI0_CFGADDR:
> phb->config_reg = val & 0x80fffffc;
> @@ -666,7 +686,10 @@ static void gt64120_writel(void *opaque, hwaddr addr,
> break;
>
> default:
> - DPRINTF ("Bad register offset 0x%x\n", (int)addr);
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "gt64120: Illegal register write "
> + "reg:0x03%x size:%u value:0x%0*" PRIx64 "\n",
> + saddr << 2, size, size << 1, val);
> break;
> }
> }
> @@ -940,7 +963,10 @@ static uint64_t gt64120_readl(void *opaque,
>
> default:
> val = s->regs[saddr];
> - DPRINTF ("Bad register offset 0x%x\n", (int)addr);
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "gt64120: Illegal register read "
> + "reg:0x03%x size:%u value:0x%0*x\n",
> + saddr << 2, size, size << 1, val);
> break;
> }
>
> --
> 2.19.1
>
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 06/10] hw/mips/gt64xxx_pci: Convert debug printf()s to trace events
2019-06-24 22:28 ` [Qemu-devel] [PATCH 06/10] hw/mips/gt64xxx_pci: Convert debug printf()s to trace events Philippe Mathieu-Daudé
@ 2019-06-25 0:40 ` Aleksandar Markovic
2019-06-25 10:00 ` Philippe Mathieu-Daudé
2019-06-25 18:17 ` Aleksandar Markovic
1 sibling, 1 reply; 31+ messages in thread
From: Aleksandar Markovic @ 2019-06-25 0:40 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Peter Maydell, Thomas Huth, Aleksandar Rikalo, qemu-devel,
Hervé Poussineau, Artyom Tarasenko, Aleksandar Markovic,
Paolo Bonzini, Aurelien Jarno
On Jun 25, 2019 12:46 AM, "Philippe Mathieu-Daudé" <f4bug@amsat.org> wrote:
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
Philipoe, can you hust clarify (explain) what is the criterium when to use
log message, and when to use trace event, which are bith present in gt64xxx
implementation.
> Makefile.objs | 1 +
> hw/mips/gt64xxx_pci.c | 29 ++++++++++-------------------
> hw/mips/trace-events | 4 ++++
> 3 files changed, 15 insertions(+), 19 deletions(-)
> create mode 100644 hw/mips/trace-events
>
> diff --git a/Makefile.objs b/Makefile.objs
> index 658cfc9d9f..3b83621f32 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -163,6 +163,7 @@ trace-events-subdirs += hw/input
> trace-events-subdirs += hw/intc
> trace-events-subdirs += hw/isa
> trace-events-subdirs += hw/mem
> +trace-events-subdirs += hw/mips
> trace-events-subdirs += hw/misc
> trace-events-subdirs += hw/misc/macio
> trace-events-subdirs += hw/net
> diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
> index f44326f14f..815ef0711d 100644
> --- a/hw/mips/gt64xxx_pci.c
> +++ b/hw/mips/gt64xxx_pci.c
> @@ -30,14 +30,7 @@
> #include "hw/pci/pci_host.h"
> #include "hw/i386/pc.h"
> #include "exec/address-spaces.h"
> -
> -//#define DEBUG
> -
> -#ifdef DEBUG
> -#define DPRINTF(fmt, ...) fprintf(stderr, "%s: " fmt, __func__,
##__VA_ARGS__)
> -#else
> -#define DPRINTF(fmt, ...)
> -#endif
> +#include "trace.h"
>
> #define GT_REGS (0x1000 >> 2)
>
> @@ -294,9 +287,7 @@ static void gt64120_isd_mapping(GT64120State *s)
> check_reserved_space(&start, &length);
> length = 0x1000;
> /* Map new address */
> - DPRINTF("ISD: "TARGET_FMT_plx"@"TARGET_FMT_plx
> - " -> "TARGET_FMT_plx"@"TARGET_FMT_plx"\n",
> - s->ISD_length, s->ISD_start, length, start);
> + trace_gt64120_isd_remap(s->ISD_length, s->ISD_start, length, start);
> s->ISD_start = start;
> s->ISD_length = length;
> memory_region_add_subregion(get_system_memory(), s->ISD_start,
&s->ISD_mem);
> @@ -648,19 +639,19 @@ static void gt64120_writel(void *opaque, hwaddr
addr,
> /* not really implemented */
> s->regs[saddr] = ~(~(s->regs[saddr]) | ~(val & 0xfffffffe));
> s->regs[saddr] |= !!(s->regs[saddr] & 0xfffffffe);
> - DPRINTF("INTRCAUSE %" PRIx64 "\n", val);
> + trace_gt64120_write("INTRCAUSE", size << 1, val);
> break;
> case GT_INTRMASK:
> s->regs[saddr] = val & 0x3c3ffffe;
> - DPRINTF("INTRMASK %" PRIx64 "\n", val);
> + trace_gt64120_write("INTRMASK", size << 1, val);
> break;
> case GT_PCI0_ICMASK:
> s->regs[saddr] = val & 0x03fffffe;
> - DPRINTF("ICMASK %" PRIx64 "\n", val);
> + trace_gt64120_write("ICMASK", size << 1, val);
> break;
> case GT_PCI0_SERR0MASK:
> s->regs[saddr] = val & 0x0000003f;
> - DPRINTF("SERR0MASK %" PRIx64 "\n", val);
> + trace_gt64120_write("SERR0MASK", size << 1, val);
> break;
>
> /* Reserved when only PCI_0 is configured. */
> @@ -936,19 +927,19 @@ static uint64_t gt64120_readl(void *opaque,
> /* Interrupts */
> case GT_INTRCAUSE:
> val = s->regs[saddr];
> - DPRINTF("INTRCAUSE %x\n", val);
> + trace_gt64120_read("INTRCAUSE", size << 1, val);
> break;
> case GT_INTRMASK:
> val = s->regs[saddr];
> - DPRINTF("INTRMASK %x\n", val);
> + trace_gt64120_read("INTRMASK", size << 1, val);
> break;
> case GT_PCI0_ICMASK:
> val = s->regs[saddr];
> - DPRINTF("ICMASK %x\n", val);
> + trace_gt64120_read("ICMASK", size << 1, val);
> break;
> case GT_PCI0_SERR0MASK:
> val = s->regs[saddr];
> - DPRINTF("SERR0MASK %x\n", val);
> + trace_gt64120_read("SERR0MASK", size << 1, val);
> break;
>
> /* Reserved when only PCI_0 is configured. */
> diff --git a/hw/mips/trace-events b/hw/mips/trace-events
> new file mode 100644
> index 0000000000..75d4c73f2e
> --- /dev/null
> +++ b/hw/mips/trace-events
> @@ -0,0 +1,4 @@
> +# gt64xxx.c
> +gt64120_read(const char *regname, int width, uint64_t value) "gt64120
read %s value:0x%0*" PRIx64
> +gt64120_write(const char *regname, int width, uint64_t value) "gt64120
write %s value:0x%0*" PRIx64
> +gt64120_isd_remap(uint64_t from_length, uint64_t from_addr, uint64_t
to_length, uint64_t to_addr) "ISD: 0x%08" PRIx64 "@0x%08" PRIx64 " ->
0x%08" PRIx64 "@0x%08" PRIx64
> --
> 2.19.1
>
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 07/10] hw/mips/gt64xxx_pci: Align the pci0-mem size
2019-06-24 22:28 ` [Qemu-devel] [PATCH 07/10] hw/mips/gt64xxx_pci: Align the pci0-mem size Philippe Mathieu-Daudé
@ 2019-06-25 0:43 ` Aleksandar Markovic
2019-06-25 7:41 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 31+ messages in thread
From: Aleksandar Markovic @ 2019-06-25 0:43 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Peter Maydell, Thomas Huth, Aleksandar Rikalo, qemu-devel,
Hervé Poussineau, Artyom Tarasenko, Aleksandar Markovic,
Paolo Bonzini, Aurelien Jarno
On Jun 25, 2019 12:44 AM, "Philippe Mathieu-Daudé" <f4bug@amsat.org> wrote:
>
> One byte is missing, use an aligned size.
>
> (qemu) info mtree
> memory-region: pci0-mem
> 0000000000000000-00000000fffffffe (prio 0, i/o): pci0-mem
> ^
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>
I agree with this change, but do we have similar situations in QEMU code
elsewhere?
> hw/mips/gt64xxx_pci.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
> index 815ef0711d..2fa313f498 100644
> --- a/hw/mips/gt64xxx_pci.c
> +++ b/hw/mips/gt64xxx_pci.c
> @@ -23,6 +23,7 @@
> */
>
> #include "qemu/osdep.h"
> +#include "qemu/units.h"
> #include "qemu/log.h"
> #include "hw/hw.h"
> #include "hw/mips/mips.h"
> @@ -1201,7 +1202,7 @@ PCIBus *gt64120_register(qemu_irq *pic)
> dev = qdev_create(NULL, TYPE_GT64120_PCI_HOST_BRIDGE);
> d = GT64120_PCI_HOST_BRIDGE(dev);
> phb = PCI_HOST_BRIDGE(dev);
> - memory_region_init(&d->pci0_mem, OBJECT(dev), "pci0-mem",
UINT32_MAX);
> + memory_region_init(&d->pci0_mem, OBJECT(dev), "pci0-mem", 4 * GiB);
> address_space_init(&d->pci0_mem_as, &d->pci0_mem, "pci0-mem");
> phb->bus = pci_register_root_bus(dev, "pci",
> gt64120_pci_set_irq,
gt64120_pci_map_irq,
> --
> 2.19.1
>
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 05/10] hw/mips/gt64xxx_pci: Use qemu_log_mask() instead of debug printf()
2019-06-25 0:37 ` Aleksandar Markovic
@ 2019-06-25 7:14 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-06-25 7:14 UTC (permalink / raw)
To: Aleksandar Markovic
Cc: Peter Maydell, Thomas Huth, Aleksandar Rikalo, qemu-devel,
Hervé Poussineau, Artyom Tarasenko, Aleksandar Markovic,
Paolo Bonzini, Aurelien Jarno
Hi Aleksandar,
On 6/25/19 2:37 AM, Aleksandar Markovic wrote:
>
> This patch is not only mechanical replacement of printf(), but it also
> improves existing log messages, and adds some new ones as well. Reflect
> that in both commit message title and body. Perhaps there are more spots
> that deserve logging. But, also, please, Philippe, doublecheck in real
> scenarios if we don't flood the log with too many messages.
While qemu_log(...) might flood the user, qemu_log_mask(mask, ...) do
not. By default the mask is empty, and you have to enable the specific
bits you want the relevant information to be logged.
The mask comes from:
$ qemu-system-mips -d help
Log items (comma separated):
out_asm show generated host assembly code for each compiled TB
in_asm show target assembly code for each compiled TB
op show micro ops for each compiled TB
op_opt show micro ops after optimization
op_ind show micro ops before indirect lowering
int show interrupts/exceptions in short format
exec show trace before each executed TB (lots of logs)
cpu show CPU registers before entering a TB (lots of logs)
fpu include FPU registers in the 'cpu' logging
mmu log MMU-related activities
pcall x86 only: show protected mode far calls/returns/exceptions
cpu_reset show CPU state before CPU resets
unimp log unimplemented functionality
guest_errors log when the guest OS does something invalid (eg accessing a
non-existent register)
page dump pages at beginning of user mode emulation
nochain do not chain compiled TBs so that "exec" and "cpu" show
complete traces
trace:PATTERN enable trace events
>> hw/mips/gt64xxx_pci.c | 48 +++++++++++++++++++++++++++++++++----------
>> 1 file changed, 37 insertions(+), 11 deletions(-)
>>
>> diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
>> index 0b9fb02475..f44326f14f 100644
>> --- a/hw/mips/gt64xxx_pci.c
>> +++ b/hw/mips/gt64xxx_pci.c
>> @@ -23,6 +23,7 @@
>> */
>>
>> #include "qemu/osdep.h"
>> +#include "qemu/log.h"
>> #include "hw/hw.h"
>> #include "hw/mips/mips.h"
>> #include "hw/pci/pci.h"
>> @@ -466,12 +467,20 @@ static void gt64120_writel(void *opaque, hwaddr
> addr,
>> case GT_CPUERR_DATAHI:
>> case GT_CPUERR_PARITY:
>> /* Read-only registers, do nothing */
>> + qemu_log_mask(LOG_GUEST_ERROR,
>> + "gt64120: Read-only register write "
>> + "reg:0x03%x size:%u value:0x%0*" PRIx64 "\n",
>> + saddr << 2, size, size << 1, val);
>> break;
[...]
So here if you do not run with '-d guest_errors', invalid accesses won't
be logged.
Note that there is no equivalent of error_report_once() with qemu_log(),
but IMO in case of I/O access I am not sure it would make sense.
Regards,
Phil.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 07/10] hw/mips/gt64xxx_pci: Align the pci0-mem size
2019-06-25 0:43 ` Aleksandar Markovic
@ 2019-06-25 7:41 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-06-25 7:41 UTC (permalink / raw)
To: Aleksandar Markovic
Cc: Peter Maydell, Thomas Huth, Aleksandar Rikalo, qemu-devel,
Hervé Poussineau, Artyom Tarasenko, Aleksandar Markovic,
Paolo Bonzini, Aurelien Jarno
On 6/25/19 2:43 AM, Aleksandar Markovic wrote:
>
> On Jun 25, 2019 12:44 AM, "Philippe Mathieu-Daudé" <f4bug@amsat.org
> <mailto:f4bug@amsat.org>> wrote:
>>
>> One byte is missing, use an aligned size.
>>
>> (qemu) info mtree
>> memory-region: pci0-mem
>> 0000000000000000-00000000fffffffe (prio 0, i/o): pci0-mem
>> ^
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org
> <mailto:f4bug@amsat.org>>
>> ---
>
> Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com
> <mailto:amarkovic@wavecomp.com>>
Thanks!
> I agree with this change, but do we have similar situations in QEMU code
> elsewhere?
Good reflex :)
We have the Jazz boards:
address-space: rc4030-dma
0000000000000000-00000000fffffffe (prio 0, i/o): rc4030.dma
address-space: dp8393x
0000000000000000-00000000fffffffe (prio 0, i/o): rc4030.dma
>> hw/mips/gt64xxx_pci.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
>> index 815ef0711d..2fa313f498 100644
>> --- a/hw/mips/gt64xxx_pci.c
>> +++ b/hw/mips/gt64xxx_pci.c
>> @@ -23,6 +23,7 @@
>> */
>>
>> #include "qemu/osdep.h"
>> +#include "qemu/units.h"
>> #include "qemu/log.h"
>> #include "hw/hw.h"
>> #include "hw/mips/mips.h"
>> @@ -1201,7 +1202,7 @@ PCIBus *gt64120_register(qemu_irq *pic)
>> dev = qdev_create(NULL, TYPE_GT64120_PCI_HOST_BRIDGE);
>> d = GT64120_PCI_HOST_BRIDGE(dev);
>> phb = PCI_HOST_BRIDGE(dev);
>> - memory_region_init(&d->pci0_mem, OBJECT(dev), "pci0-mem",
> UINT32_MAX);
>> + memory_region_init(&d->pci0_mem, OBJECT(dev), "pci0-mem", 4 * GiB);
>> address_space_init(&d->pci0_mem_as, &d->pci0_mem, "pci0-mem");
>> phb->bus = pci_register_root_bus(dev, "pci",
>> gt64120_pci_set_irq,
> gt64120_pci_map_irq,
>> --
>> 2.19.1
>>
>>
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 06/10] hw/mips/gt64xxx_pci: Convert debug printf()s to trace events
2019-06-25 0:40 ` Aleksandar Markovic
@ 2019-06-25 10:00 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-06-25 10:00 UTC (permalink / raw)
To: Aleksandar Markovic, Stefan Hajnoczi
Cc: Peter Maydell, Thomas Huth, Aleksandar Rikalo, qemu-devel,
Hervé Poussineau, Artyom Tarasenko, Aleksandar Markovic,
Paolo Bonzini, Aurelien Jarno
Hi Aleksandar,
On 6/25/19 2:40 AM, Aleksandar Markovic wrote:
>
> Philippe, can you hust clarify (explain) what is the criterium when to
> use log message, and when to use trace event, which are bith present in
> gt64xxx implementation.
The criterium is rather generic.
All those log/events are meant for developpers, and are disabled by default.
- DPRINTF()
This is the older printf() method, flooding the stdout (confuse when
the serial console is displayed on stdio).
It is deprecated because you have to edit the source and recompile to
be able to use it, and once enabled you can not disable it.
Since it is compile-time disabled, it tends to bitrot (string formats
are not updated).
Not recommended for new design.
- qemu_log_mask(loglevel_bits, ...)
These calls are filtered with the global qemu_loglevel.
The log is output to the 'log_file' file if set, or to stderr.
You can set the global loglevel from command line with '-d bits,...'
and the global logfile with the '-D file.log' command line option.
You can also set those globals at runtime using the HMP 'log' and
'logfile' commands:
(qemu) log help
Log items (comma separated):
none remove all logs
out_asm show generated host assembly code for each compiled TB
in_asm show target assembly code for each compiled TB
op show micro ops for each compiled TB
op_opt show micro ops after optimization
op_ind show micro ops before indirect lowering
[...]
(qemu) log in_asm
[...]
(qemu) log none
Note that this logging doesn't have precise timing information.
- qemu_log_mask(LOG_GUEST_ERROR, ...)
You select this level with '-d guest_errors' or via HMP.
This reports invalid hardware accesses from the guest (buggy firmware,
code running on an incorrect machine).
This is useful for developpers of bootloader, who might want to fix
their incorrect accesses before trying the fw on real hardware.
Hardware not always generate exception for these incorrect accesses.
Error reported come from the guest, and QEMU is not responsible, nor
need modification in its models.
- qemu_log_mask(LOG_UNIMP, ...)
You select this one with '-d unimp' or via HMP.
This reports accesses to devices QEMU is not modeling.
Having the missing device correctly modeled is likely to change the
guest code flow (device/driver initialization).
This also log accesses to not-yet-implemented registers within a
partially implemented device.
The common case is an OS driver added new functionalities. The model
was developped with a limited driver, newer drivers expect to set
more features up.
I don't use it with guests image I know are already working, but I
often use it when trying an image with a newer/older kernel for
example.
- trace events
Tracing is more powerful than the previous items, but usually requires
post-processing effort. It is usually targetting live/post-mortem
debugging. You can use various trace backends. It has precise timing,
is not invasite, thus is suitable for enterprise grade products.
You tipically want to use the 'nop' backend which totally disable
tracing when building a production release binary.
Trace events are useful to debug the guest but also the QEMU code.
They are better to debug asynchrone issues than log_mask.
They are also very useful to measure timings.
<Many more features to add here...>
Some backends allow easy parsing of events for further (graphical)
analysis.
You can enable/disable traces at runtime with the HMP
'trace-event NAME on|off' command:
(qemu) info trace-events
[...]
memory_region_ops_write : state 0
memory_region_ops_read : state 0
ram_block_discard_range : state 0
find_ram_offset_loop : state 0
find_ram_offset : state 0
dma_map_wait : state 0
dma_blk_cb : state 0
[...]
(qemu) trace-event find* on
(qemu) info trace-events
[...]
memory_region_ops_write : state 0
memory_region_ops_read : state 0
ram_block_discard_range : state 0
find_ram_offset_loop : state 1
find_ram_offset : state 1
dma_map_wait : state 0
dma_blk_cb : state 0
[...]
You can also use a file with filtered events.
Example tracing how YAMON access the flash and setup the
PCI bars:
$ cat yamon.trace
# all flash i/o, no data
pflash*
-pflash_data
pci*
gt64120*
$ qemu-system-mipsel -M malta \
-serial vc \
-pflash dump.bin \
-trace events=yamon.trace
11281@1561456612.571933:gt64120_isd_remap ISD: 0x00000000@0x00000000
-> 0x00001000@0x14000000
11284@1561456612.576392:gt64120_isd_remap ISD: 0x00001000@0x14000000
-> 0x00001000@0x1be00000
11284@1561456612.627574:pci_cfg_write gt64120_pci 00:0 @0x20 <- 0x1be00000
11284@1561456614.825315:gt64120_write gt64120 write INTRCAUSE
value:0xfff3ffff
...
11284@1561456615.176261:pci_cfg_write cirrus-vga 18:0 @0x4 <- 0x2
11284@1561456615.176275:pci_update_mappings_add d=0x564efb01dd50
00:12.0 0,0x10000000+0x2000000
11284@1561456615.176672:pci_update_mappings_add d=0x564efb01dd50
00:12.0 1,0x12050000+0x1000
...
Hope that help!
Regards,
Phil.
>> Makefile.objs | 1 +
>> hw/mips/gt64xxx_pci.c | 29 ++++++++++-------------------
>> hw/mips/trace-events | 4 ++++
>> 3 files changed, 15 insertions(+), 19 deletions(-)
>> create mode 100644 hw/mips/trace-events
>>
>> diff --git a/Makefile.objs b/Makefile.objs
>> index 658cfc9d9f..3b83621f32 100644
>> --- a/Makefile.objs
>> +++ b/Makefile.objs
>> @@ -163,6 +163,7 @@ trace-events-subdirs += hw/input
>> trace-events-subdirs += hw/intc
>> trace-events-subdirs += hw/isa
>> trace-events-subdirs += hw/mem
>> +trace-events-subdirs += hw/mips
>> trace-events-subdirs += hw/misc
>> trace-events-subdirs += hw/misc/macio
>> trace-events-subdirs += hw/net
>> diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
>> index f44326f14f..815ef0711d 100644
>> --- a/hw/mips/gt64xxx_pci.c
>> +++ b/hw/mips/gt64xxx_pci.c
>> @@ -30,14 +30,7 @@
>> #include "hw/pci/pci_host.h"
>> #include "hw/i386/pc.h"
>> #include "exec/address-spaces.h"
>> -
>> -//#define DEBUG
>> -
>> -#ifdef DEBUG
>> -#define DPRINTF(fmt, ...) fprintf(stderr, "%s: " fmt, __func__,
> ##__VA_ARGS__)
>> -#else
>> -#define DPRINTF(fmt, ...)
>> -#endif
>> +#include "trace.h"
>>
>> #define GT_REGS (0x1000 >> 2)
>>
>> @@ -294,9 +287,7 @@ static void gt64120_isd_mapping(GT64120State *s)
>> check_reserved_space(&start, &length);
>> length = 0x1000;
>> /* Map new address */
>> - DPRINTF("ISD: "TARGET_FMT_plx"@"TARGET_FMT_plx
>> - " -> "TARGET_FMT_plx"@"TARGET_FMT_plx"\n",
>> - s->ISD_length, s->ISD_start, length, start);
>> + trace_gt64120_isd_remap(s->ISD_length, s->ISD_start, length, start);
[...]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 05/10] hw/mips/gt64xxx_pci: Use qemu_log_mask() instead of debug printf()
2019-06-24 22:28 ` [Qemu-devel] [PATCH 05/10] hw/mips/gt64xxx_pci: Use qemu_log_mask() instead of debug printf() Philippe Mathieu-Daudé
2019-06-25 0:37 ` Aleksandar Markovic
@ 2019-06-25 18:16 ` Aleksandar Markovic
1 sibling, 0 replies; 31+ messages in thread
From: Aleksandar Markovic @ 2019-06-25 18:16 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Peter Maydell, Thomas Huth, Aleksandar Rikalo, qemu-devel,
Hervé Poussineau, Artyom Tarasenko, Aleksandar Markovic,
Paolo Bonzini, Aurelien Jarno
On Jun 25, 2019 12:42 AM, "Philippe Mathieu-Daudé" <f4bug@amsat.org> wrote:
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> hw/mips/gt64xxx_pci.c | 48 +++++++++++++++++++++++++++++++++----------
> 1 file changed, 37 insertions(+), 11 deletions(-)
>
> diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
> index 0b9fb02475..f44326f14f 100644
> --- a/hw/mips/gt64xxx_pci.c
> +++ b/hw/mips/gt64xxx_pci.c
> @@ -23,6 +23,7 @@
> */
>
> #include "qemu/osdep.h"
> +#include "qemu/log.h"
> #include "hw/hw.h"
> #include "hw/mips/mips.h"
> #include "hw/pci/pci.h"
> @@ -466,12 +467,20 @@ static void gt64120_writel(void *opaque, hwaddr
addr,
> case GT_CPUERR_DATAHI:
> case GT_CPUERR_PARITY:
> /* Read-only registers, do nothing */
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "gt64120: Read-only register write "
> + "reg:0x03%x size:%u value:0x%0*" PRIx64 "\n",
> + saddr << 2, size, size << 1, val);
> break;
>
> /* CPU Sync Barrier */
> case GT_PCI0SYNC:
> case GT_PCI1SYNC:
> /* Read-only registers, do nothing */
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "gt64120: Read-only register write "
> + "reg:0x03%x size:%u value:0x%0*" PRIx64 "\n",
> + saddr << 2, size, size << 1, val);
> break;
>
> /* SDRAM and Device Address Decode */
> @@ -510,7 +519,10 @@ static void gt64120_writel(void *opaque, hwaddr addr,
> case GT_DEV_B3:
> case GT_DEV_BOOT:
> /* Not implemented */
> - DPRINTF ("Unimplemented device register offset 0x%x\n", saddr <<
2);
> + qemu_log_mask(LOG_UNIMP,
> + "gt64120: Unimplemented device register write "
> + "reg:0x03%x size:%u value:0x%0*" PRIx64 "\n",
> + saddr << 2, size, size << 1, val);
> break;
>
> /* ECC */
> @@ -520,6 +532,10 @@ static void gt64120_writel(void *opaque, hwaddr addr,
> case GT_ECC_CALC:
> case GT_ECC_ERRADDR:
> /* Read-only registers, do nothing */
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "gt64120: Read-only register write "
> + "reg:0x03%x size:%u value:0x%0*" PRIx64 "\n",
> + saddr << 2, size, size << 1, val);
> break;
>
> /* DMA Record */
> @@ -543,23 +559,20 @@ static void gt64120_writel(void *opaque, hwaddr
addr,
> case GT_DMA1_CUR:
> case GT_DMA2_CUR:
> case GT_DMA3_CUR:
> - /* Not implemented */
> - DPRINTF ("Unimplemented DMA register offset 0x%x\n", saddr << 2);
> - break;
>
> /* DMA Channel Control */
> case GT_DMA0_CTRL:
> case GT_DMA1_CTRL:
> case GT_DMA2_CTRL:
> case GT_DMA3_CTRL:
> - /* Not implemented */
> - DPRINTF ("Unimplemented DMA register offset 0x%x\n", saddr << 2);
> - break;
>
> /* DMA Arbiter */
> case GT_DMA_ARB:
> /* Not implemented */
> - DPRINTF ("Unimplemented DMA register offset 0x%x\n", saddr << 2);
> + qemu_log_mask(LOG_UNIMP,
> + "gt64120: Unimplemented DMA register write "
> + "reg:0x03%x size:%u value:0x%0*" PRIx64 "\n",
> + saddr << 2, size, size << 1, val);
> break;
>
> /* Timer/Counter */
> @@ -569,7 +582,10 @@ static void gt64120_writel(void *opaque, hwaddr addr,
> case GT_TC3:
> case GT_TC_CONTROL:
> /* Not implemented */
> - DPRINTF ("Unimplemented timer register offset 0x%x\n", saddr <<
2);
> + qemu_log_mask(LOG_UNIMP,
> + "gt64120: Unimplemented timer register write "
> + "reg:0x03%x size:%u value:0x%0*" PRIx64 "\n",
> + saddr << 2, size, size << 1, val);
> break;
>
> /* PCI Internal */
> @@ -610,6 +626,10 @@ static void gt64120_writel(void *opaque, hwaddr addr,
> case GT_PCI1_CFGADDR:
> case GT_PCI1_CFGDATA:
> /* not implemented */
> + qemu_log_mask(LOG_UNIMP,
> + "gt64120: Unimplemented timer register write "
> + "reg:0x03%x size:%u value:0x%0*" PRIx64 "\n",
> + saddr << 2, size, size << 1, val);
> break;
> case GT_PCI0_CFGADDR:
> phb->config_reg = val & 0x80fffffc;
> @@ -666,7 +686,10 @@ static void gt64120_writel(void *opaque, hwaddr addr,
> break;
>
> default:
> - DPRINTF ("Bad register offset 0x%x\n", (int)addr);
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "gt64120: Illegal register write "
> + "reg:0x03%x size:%u value:0x%0*" PRIx64 "\n",
> + saddr << 2, size, size << 1, val);
> break;
> }
> }
> @@ -940,7 +963,10 @@ static uint64_t gt64120_readl(void *opaque,
>
> default:
> val = s->regs[saddr];
> - DPRINTF ("Bad register offset 0x%x\n", (int)addr);
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "gt64120: Illegal register read "
> + "reg:0x03%x size:%u value:0x%0*x\n",
> + saddr << 2, size, size << 1, val);
> break;
> }
>
> --
> 2.19.1
>
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 06/10] hw/mips/gt64xxx_pci: Convert debug printf()s to trace events
2019-06-24 22:28 ` [Qemu-devel] [PATCH 06/10] hw/mips/gt64xxx_pci: Convert debug printf()s to trace events Philippe Mathieu-Daudé
2019-06-25 0:40 ` Aleksandar Markovic
@ 2019-06-25 18:17 ` Aleksandar Markovic
1 sibling, 0 replies; 31+ messages in thread
From: Aleksandar Markovic @ 2019-06-25 18:17 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Peter Maydell, Thomas Huth, Aleksandar Rikalo, qemu-devel,
Hervé Poussineau, Artyom Tarasenko, Aleksandar Markovic,
Paolo Bonzini, Aurelien Jarno
On Jun 25, 2019 12:46 AM, "Philippe Mathieu-Daudé" <f4bug@amsat.org> wrote:
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> Makefile.objs | 1 +
> hw/mips/gt64xxx_pci.c | 29 ++++++++++-------------------
> hw/mips/trace-events | 4 ++++
> 3 files changed, 15 insertions(+), 19 deletions(-)
> create mode 100644 hw/mips/trace-events
>
> diff --git a/Makefile.objs b/Makefile.objs
> index 658cfc9d9f..3b83621f32 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -163,6 +163,7 @@ trace-events-subdirs += hw/input
> trace-events-subdirs += hw/intc
> trace-events-subdirs += hw/isa
> trace-events-subdirs += hw/mem
> +trace-events-subdirs += hw/mips
> trace-events-subdirs += hw/misc
> trace-events-subdirs += hw/misc/macio
> trace-events-subdirs += hw/net
> diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
> index f44326f14f..815ef0711d 100644
> --- a/hw/mips/gt64xxx_pci.c
> +++ b/hw/mips/gt64xxx_pci.c
> @@ -30,14 +30,7 @@
> #include "hw/pci/pci_host.h"
> #include "hw/i386/pc.h"
> #include "exec/address-spaces.h"
> -
> -//#define DEBUG
> -
> -#ifdef DEBUG
> -#define DPRINTF(fmt, ...) fprintf(stderr, "%s: " fmt, __func__,
##__VA_ARGS__)
> -#else
> -#define DPRINTF(fmt, ...)
> -#endif
> +#include "trace.h"
>
> #define GT_REGS (0x1000 >> 2)
>
> @@ -294,9 +287,7 @@ static void gt64120_isd_mapping(GT64120State *s)
> check_reserved_space(&start, &length);
> length = 0x1000;
> /* Map new address */
> - DPRINTF("ISD: "TARGET_FMT_plx"@"TARGET_FMT_plx
> - " -> "TARGET_FMT_plx"@"TARGET_FMT_plx"\n",
> - s->ISD_length, s->ISD_start, length, start);
> + trace_gt64120_isd_remap(s->ISD_length, s->ISD_start, length, start);
> s->ISD_start = start;
> s->ISD_length = length;
> memory_region_add_subregion(get_system_memory(), s->ISD_start,
&s->ISD_mem);
> @@ -648,19 +639,19 @@ static void gt64120_writel(void *opaque, hwaddr
addr,
> /* not really implemented */
> s->regs[saddr] = ~(~(s->regs[saddr]) | ~(val & 0xfffffffe));
> s->regs[saddr] |= !!(s->regs[saddr] & 0xfffffffe);
> - DPRINTF("INTRCAUSE %" PRIx64 "\n", val);
> + trace_gt64120_write("INTRCAUSE", size << 1, val);
> break;
> case GT_INTRMASK:
> s->regs[saddr] = val & 0x3c3ffffe;
> - DPRINTF("INTRMASK %" PRIx64 "\n", val);
> + trace_gt64120_write("INTRMASK", size << 1, val);
> break;
> case GT_PCI0_ICMASK:
> s->regs[saddr] = val & 0x03fffffe;
> - DPRINTF("ICMASK %" PRIx64 "\n", val);
> + trace_gt64120_write("ICMASK", size << 1, val);
> break;
> case GT_PCI0_SERR0MASK:
> s->regs[saddr] = val & 0x0000003f;
> - DPRINTF("SERR0MASK %" PRIx64 "\n", val);
> + trace_gt64120_write("SERR0MASK", size << 1, val);
> break;
>
> /* Reserved when only PCI_0 is configured. */
> @@ -936,19 +927,19 @@ static uint64_t gt64120_readl(void *opaque,
> /* Interrupts */
> case GT_INTRCAUSE:
> val = s->regs[saddr];
> - DPRINTF("INTRCAUSE %x\n", val);
> + trace_gt64120_read("INTRCAUSE", size << 1, val);
> break;
> case GT_INTRMASK:
> val = s->regs[saddr];
> - DPRINTF("INTRMASK %x\n", val);
> + trace_gt64120_read("INTRMASK", size << 1, val);
> break;
> case GT_PCI0_ICMASK:
> val = s->regs[saddr];
> - DPRINTF("ICMASK %x\n", val);
> + trace_gt64120_read("ICMASK", size << 1, val);
> break;
> case GT_PCI0_SERR0MASK:
> val = s->regs[saddr];
> - DPRINTF("SERR0MASK %x\n", val);
> + trace_gt64120_read("SERR0MASK", size << 1, val);
> break;
>
> /* Reserved when only PCI_0 is configured. */
> diff --git a/hw/mips/trace-events b/hw/mips/trace-events
> new file mode 100644
> index 0000000000..75d4c73f2e
> --- /dev/null
> +++ b/hw/mips/trace-events
> @@ -0,0 +1,4 @@
> +# gt64xxx.c
> +gt64120_read(const char *regname, int width, uint64_t value) "gt64120
read %s value:0x%0*" PRIx64
> +gt64120_write(const char *regname, int width, uint64_t value) "gt64120
write %s value:0x%0*" PRIx64
> +gt64120_isd_remap(uint64_t from_length, uint64_t from_addr, uint64_t
to_length, uint64_t to_addr) "ISD: 0x%08" PRIx64 "@0x%08" PRIx64 " ->
0x%08" PRIx64 "@0x%08" PRIx64
> --
> 2.19.1
>
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 00/10] hw/pci-host: Clean the GT64120 north bridge
2019-06-24 22:28 [Qemu-devel] [PATCH 00/10] hw/pci-host: Clean the GT64120 north bridge Philippe Mathieu-Daudé
` (9 preceding siblings ...)
2019-06-24 22:28 ` [Qemu-devel] [RFC PATCH 10/10] hw/pci-host/gt64120: Clean the decoded address space Philippe Mathieu-Daudé
@ 2019-07-01 17:16 ` Aleksandar Markovic
2019-07-01 17:45 ` Philippe Mathieu-Daudé
10 siblings, 1 reply; 31+ messages in thread
From: Aleksandar Markovic @ 2019-07-01 17:16 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Peter Maydell, Thomas Huth, Aleksandar Rikalo, qemu-devel,
Hervé Poussineau, Artyom Tarasenko, Aleksandar Markovic,
Paolo Bonzini, Aurelien Jarno
On Jun 25, 2019 12:31 AM, "Philippe Mathieu-Daudé" <f4bug@amsat.org> wrote:
>
> Hi,
>
> This series clean the gt64120 device.
> It is no more target-dependent, and tracing is improved.
>
If nobody objects, I am going to select majority of the patches for mips
queue scheduled tomorrow. Those that remain will be those that Philippe
still didn't make his mind.
Thanks thousand times, Philippe!
Aleksandar
> Regards,
>
> Phil.
>
> Based-on: 20190624220056.25861-1-f4bug@amsat.org
> https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg05304.html
>
> Philippe Mathieu-Daudé (10):
> hw/mips/gt64xxx_pci: Fix multiline comment syntax
> hw/mips/gt64xxx_pci: Fix 'tabs' coding style issues
> hw/mips/gt64xxx_pci: Fix 'braces' coding style issues
> hw/mips/gt64xxx_pci: Fix 'spaces' coding style issues
> hw/mips/gt64xxx_pci: Use qemu_log_mask() instead of debug printf()
> hw/mips/gt64xxx_pci: Convert debug printf()s to trace events
> hw/mips/gt64xxx_pci: Align the pci0-mem size
> hw/mips/gt64xxx_pci: Add a 'cpu_big_endian' qdev property
> hw/mips/gt64xxx_pci: Move it to hw/pci-host/
> hw/pci-host/gt64120: Clean the decoded address space
>
> Makefile.objs | 1 +
> include/hw/mips/mips.h | 2 +-
> hw/mips/mips_malta.c | 8 +-
> hw/{mips/gt64xxx_pci.c => pci-host/gt64120.c} | 542 ++++++++++--------
> MAINTAINERS | 2 +-
> hw/mips/Makefile.objs | 2 +-
> hw/mips/trace-events | 0
> hw/pci-host/Makefile.objs | 2 +-
> hw/pci-host/trace-events | 5 +
> 9 files changed, 307 insertions(+), 257 deletions(-)
> rename hw/{mips/gt64xxx_pci.c => pci-host/gt64120.c} (68%)
> create mode 100644 hw/mips/trace-events
>
> --
> 2.19.1
>
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 00/10] hw/pci-host: Clean the GT64120 north bridge
2019-07-01 17:16 ` [Qemu-devel] [PATCH 00/10] hw/pci-host: Clean the GT64120 north bridge Aleksandar Markovic
@ 2019-07-01 17:45 ` Philippe Mathieu-Daudé
2019-07-01 18:39 ` Aleksandar Markovic
0 siblings, 1 reply; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-07-01 17:45 UTC (permalink / raw)
To: Aleksandar Markovic
Cc: Peter Maydell, Thomas Huth, Aleksandar Rikalo, qemu-devel,
Hervé Poussineau, Artyom Tarasenko, Aleksandar Markovic,
Paolo Bonzini, Aurelien Jarno
Hi Aleksandar,
On 7/1/19 7:16 PM, Aleksandar Markovic wrote:
>
> On Jun 25, 2019 12:31 AM, "Philippe Mathieu-Daudé" <f4bug@amsat.org
> <mailto:f4bug@amsat.org>> wrote:
>>
>> Hi,
>>
>> This series clean the gt64120 device.
>> It is no more target-dependent, and tracing is improved.
>>
>
> If nobody objects, I am going to select majority of the patches for mips
> queue scheduled tomorrow. Those that remain will be those that Philippe
> still didn't make his mind.
Which ones remain?
For "pci-host/gt64120: Clean the decoded address space", I'd like a
review from someone comfortable with MEMTXATTRS and address spaces.
I'll ping on the patch.
> Thanks thousand times, Philippe!
>
> Aleksandar
>
>> Regards,
>>
>> Phil.
>>
>> Based-on: 20190624220056.25861-1-f4bug@amsat.org
> <mailto:20190624220056.25861-1-f4bug@amsat.org>
>> https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg05304.html
>>
>> Philippe Mathieu-Daudé (10):
>> hw/mips/gt64xxx_pci: Fix multiline comment syntax
>> hw/mips/gt64xxx_pci: Fix 'tabs' coding style issues
>> hw/mips/gt64xxx_pci: Fix 'braces' coding style issues
>> hw/mips/gt64xxx_pci: Fix 'spaces' coding style issues
>> hw/mips/gt64xxx_pci: Use qemu_log_mask() instead of debug printf()
>> hw/mips/gt64xxx_pci: Convert debug printf()s to trace events
>> hw/mips/gt64xxx_pci: Align the pci0-mem size
>> hw/mips/gt64xxx_pci: Add a 'cpu_big_endian' qdev property
>> hw/mips/gt64xxx_pci: Move it to hw/pci-host/
>> hw/pci-host/gt64120: Clean the decoded address space
>>
>> Makefile.objs | 1 +
>> include/hw/mips/mips.h | 2 +-
>> hw/mips/mips_malta.c | 8 +-
>> hw/{mips/gt64xxx_pci.c => pci-host/gt64120.c} | 542 ++++++++++--------
>> MAINTAINERS | 2 +-
>> hw/mips/Makefile.objs | 2 +-
>> hw/mips/trace-events | 0
>> hw/pci-host/Makefile.objs | 2 +-
>> hw/pci-host/trace-events | 5 +
>> 9 files changed, 307 insertions(+), 257 deletions(-)
>> rename hw/{mips/gt64xxx_pci.c => pci-host/gt64120.c} (68%)
>> create mode 100644 hw/mips/trace-events
>>
>> --
>> 2.19.1
>>
>>
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [RFC PATCH 10/10] hw/pci-host/gt64120: Clean the decoded address space
2019-06-24 22:28 ` [Qemu-devel] [RFC PATCH 10/10] hw/pci-host/gt64120: Clean the decoded address space Philippe Mathieu-Daudé
@ 2019-07-01 18:06 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-07-01 18:06 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Hervé Poussineau, Aleksandar Markovic,
Aurelien Jarno, Aleksandar Rikalo, Thomas Huth, Peter Maydell,
Artyom Tarasenko, Ralf Baechle, Paul Burton, James Hogan,
linux-mips
Cc'ing the kernel folks.
On 6/25/19 12:28 AM, Philippe Mathieu-Daudé wrote:
> The SysAd bus is split in various address spaces.
> Declare the different regions separately, this helps a lot
> while tracing different access while debugging.
>
> We also add the PCI1 ranges.
>
> See 'GT-64120A System Controller' datasheet Rev, 1.1,
> "Table 15: CPU and Device Decoder Default Address Mapping"
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> While this device is modelled toward the Malta board, it is generic.
> ---
> hw/mips/mips_malta.c | 6 ------
> hw/pci-host/gt64120.c | 19 +++++++++++++++++++
> 2 files changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
> index 97f8ffbf1b..d6e4a0dad9 100644
> --- a/hw/mips/mips_malta.c
> +++ b/hw/mips/mips_malta.c
> @@ -53,7 +53,6 @@
> #include "sysemu/qtest.h"
> #include "qapi/error.h"
> #include "qemu/error-report.h"
> -#include "hw/misc/empty_slot.h"
> #include "sysemu/kvm.h"
> #include "hw/semihosting/semihost.h"
> #include "hw/mips/cps.h"
> @@ -1209,11 +1208,6 @@ void mips_malta_init(MachineState *machine)
> DeviceState *dev = qdev_create(NULL, TYPE_MIPS_MALTA);
> MaltaState *s = MIPS_MALTA(dev);
>
> - /* The whole address space decoded by the GT-64120A doesn't generate
> - exception when accessing invalid memory. Create an empty slot to
> - emulate this feature. */
> - empty_slot_init("gt64120-ad", 0x00000000, 0x20000000);
> -
> qdev_init_nofail(dev);
>
> /* create CPU */
> diff --git a/hw/pci-host/gt64120.c b/hw/pci-host/gt64120.c
> index 5209038ee5..6eaa571994 100644
> --- a/hw/pci-host/gt64120.c
> +++ b/hw/pci-host/gt64120.c
> @@ -31,6 +31,8 @@
> #include "hw/pci/pci_host.h"
> #include "hw/i386/pc.h"
> #include "exec/address-spaces.h"
> +#include "hw/misc/empty_slot.h"
> +#include "hw/misc/unimp.h"
> #include "trace.h"
>
> #define GT_REGS (0x1000 >> 2)
> @@ -1206,6 +1208,23 @@ PCIBus *gt64120_create(qemu_irq *pic, bool target_is_bigendian)
> "isd-mem", 0x1000);
>
> pci_create_simple(phb->bus, PCI_DEVFN(0, 0), "gt64120_pci");
> +
> + create_unimplemented_device("gt64120_i2o", 0x14000000, 256);
> +
> + empty_slot_init("SCS0", 0x00000000, 8 * MiB);
> + empty_slot_init("SCS1", 0x00800000, 8 * MiB);
> + empty_slot_init("SCS2", 0x01000000, 8 * MiB);
> + empty_slot_init("SCS3", 0x01800000, 8 * MiB);
Since it is a bit pointless to alloc 4 regions, I could
simplify those 4 as:
empty_slot_init("SCS[4]", 0x00000000, 4 * 8 * MiB);
The difference with the previous content is now we have
two new holes:
- 0x02000000-0x10000000
- 0x14001000-0x1c000000
Ralf/Paul/James, what should happen when a guest access these
holes (hardware PoV, no QEMU)?
The address space with this patch is:
(qemu) info mtree
address-space: memory
0000000000000000-0000000007ffffff (prio 0, i/o): alias low_ram
@mips_malta.ram 0000000000000000-0000000007ffffff
0000000000000000-00000000007fffff (prio -10000, i/o): SCS0
0000000000800000-0000000000ffffff (prio -10000, i/o): SCS1
0000000001000000-00000000017fffff (prio -10000, i/o): SCS2
0000000001800000-0000000001ffffff (prio -10000, i/o): SCS3
0000000002000000-000000000fffffff [hole]
0000000010000000-0000000011ffffff (prio 0, i/o): alias pci0-io @io
0000000000000000-0000000001ffffff
0000000012000000-0000000013ffffff (prio 0, i/o): alias pci0-mem0
@pci0-mem 0000000012000000-0000000013ffffff
0000000014000000-0000000014000fff (prio 0, i/o): isd-mem
0000000014000000-00000000140000ff (prio -1000, i/o): gt64120_i2o
0000000014001000-000000001bffffff [hole]
000000001c000000-000000001c7fffff (prio -10000, i/o): CS0
000000001c800000-000000001cffffff (prio -10000, i/o): CS1
000000001d000000-000000001effffff (prio -10000, i/o): CS2
000000001e000000-000000001e3fffff (prio 0, romd): mips_malta.bios
000000001f000000-000000001f0008ff (prio 0, i/o): alias malta-fpga
@malta-fpga 0000000000000000-00000000000008ff
000000001f000000-000000001fbfffff (prio -10000, i/o): CS3
000000001f000900-000000001f00093f (prio 0, i/o): serial
000000001f000a00-000000001f00ffff (prio 0, i/o): alias malta-fpga
@malta-fpga 0000000000000a00-000000000000ffff
000000001fc00000-000000001fffffff (prio 0, rom): bios.1fc
000000001fc00000-000000001fffffff (prio -10000, i/o): BootCS
0000000020000000-0000000021ffffff (prio -1000, i/o): pci1-io
0000000022000000-0000000023ffffff (prio -10000, i/o): pci1-mem0
0000000024000000-0000000025ffffff (prio -10000, i/o): pci1-mem1
0000000080000000-0000000087ffffff (prio 0, ram): mips_malta.ram
00000000f2000000-00000000f3ffffff (prio 0, i/o): alias pci0-mem1
@pci0-mem 00000000f2000000-00000000f3ffffff
> + empty_slot_init("CS0", 0x1c000000, 8 * MiB);
> + empty_slot_init("CS1", 0x1c800000, 8 * MiB);
> + empty_slot_init("CS2", 0x1d000000, 32 * MiB);
> + empty_slot_init("CS3", 0x1f000000, 12 * MiB);
I'm not very happy to add a non-pow2 range, but this is how
it appears on the datasheet. I suppose the correct range is
16MB with lower priority than the BootCS.
> + empty_slot_init("BootCS", 0x1fc00000, 4 * MiB);
> + create_unimplemented_device("pci1-io", 0x20000000, 32 * MiB);
> + empty_slot_init("pci1-mem0", 0x22000000, 32 * MiB);
> + empty_slot_init("pci1-mem1", 0x24000000, 32 * MiB);
This part is new, and could go in a separate patch:
Currently, no guest ever accessed this space.
Regards,
Phil.
> +
> return phb->bus;
> }
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 10/10] hw/pci-host/gt64120: Clean the decoded address space
@ 2019-07-01 18:06 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-07-01 18:06 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Thomas Huth, Aleksandar Rikalo, James Hogan,
linux-mips, Paul Burton, Hervé Poussineau, Artyom Tarasenko,
Aleksandar Markovic, Paolo Bonzini, Aurelien Jarno
Cc'ing the kernel folks.
On 6/25/19 12:28 AM, Philippe Mathieu-Daudé wrote:
> The SysAd bus is split in various address spaces.
> Declare the different regions separately, this helps a lot
> while tracing different access while debugging.
>
> We also add the PCI1 ranges.
>
> See 'GT-64120A System Controller' datasheet Rev, 1.1,
> "Table 15: CPU and Device Decoder Default Address Mapping"
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> While this device is modelled toward the Malta board, it is generic.
> ---
> hw/mips/mips_malta.c | 6 ------
> hw/pci-host/gt64120.c | 19 +++++++++++++++++++
> 2 files changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
> index 97f8ffbf1b..d6e4a0dad9 100644
> --- a/hw/mips/mips_malta.c
> +++ b/hw/mips/mips_malta.c
> @@ -53,7 +53,6 @@
> #include "sysemu/qtest.h"
> #include "qapi/error.h"
> #include "qemu/error-report.h"
> -#include "hw/misc/empty_slot.h"
> #include "sysemu/kvm.h"
> #include "hw/semihosting/semihost.h"
> #include "hw/mips/cps.h"
> @@ -1209,11 +1208,6 @@ void mips_malta_init(MachineState *machine)
> DeviceState *dev = qdev_create(NULL, TYPE_MIPS_MALTA);
> MaltaState *s = MIPS_MALTA(dev);
>
> - /* The whole address space decoded by the GT-64120A doesn't generate
> - exception when accessing invalid memory. Create an empty slot to
> - emulate this feature. */
> - empty_slot_init("gt64120-ad", 0x00000000, 0x20000000);
> -
> qdev_init_nofail(dev);
>
> /* create CPU */
> diff --git a/hw/pci-host/gt64120.c b/hw/pci-host/gt64120.c
> index 5209038ee5..6eaa571994 100644
> --- a/hw/pci-host/gt64120.c
> +++ b/hw/pci-host/gt64120.c
> @@ -31,6 +31,8 @@
> #include "hw/pci/pci_host.h"
> #include "hw/i386/pc.h"
> #include "exec/address-spaces.h"
> +#include "hw/misc/empty_slot.h"
> +#include "hw/misc/unimp.h"
> #include "trace.h"
>
> #define GT_REGS (0x1000 >> 2)
> @@ -1206,6 +1208,23 @@ PCIBus *gt64120_create(qemu_irq *pic, bool target_is_bigendian)
> "isd-mem", 0x1000);
>
> pci_create_simple(phb->bus, PCI_DEVFN(0, 0), "gt64120_pci");
> +
> + create_unimplemented_device("gt64120_i2o", 0x14000000, 256);
> +
> + empty_slot_init("SCS0", 0x00000000, 8 * MiB);
> + empty_slot_init("SCS1", 0x00800000, 8 * MiB);
> + empty_slot_init("SCS2", 0x01000000, 8 * MiB);
> + empty_slot_init("SCS3", 0x01800000, 8 * MiB);
Since it is a bit pointless to alloc 4 regions, I could
simplify those 4 as:
empty_slot_init("SCS[4]", 0x00000000, 4 * 8 * MiB);
The difference with the previous content is now we have
two new holes:
- 0x02000000-0x10000000
- 0x14001000-0x1c000000
Ralf/Paul/James, what should happen when a guest access these
holes (hardware PoV, no QEMU)?
The address space with this patch is:
(qemu) info mtree
address-space: memory
0000000000000000-0000000007ffffff (prio 0, i/o): alias low_ram
@mips_malta.ram 0000000000000000-0000000007ffffff
0000000000000000-00000000007fffff (prio -10000, i/o): SCS0
0000000000800000-0000000000ffffff (prio -10000, i/o): SCS1
0000000001000000-00000000017fffff (prio -10000, i/o): SCS2
0000000001800000-0000000001ffffff (prio -10000, i/o): SCS3
0000000002000000-000000000fffffff [hole]
0000000010000000-0000000011ffffff (prio 0, i/o): alias pci0-io @io
0000000000000000-0000000001ffffff
0000000012000000-0000000013ffffff (prio 0, i/o): alias pci0-mem0
@pci0-mem 0000000012000000-0000000013ffffff
0000000014000000-0000000014000fff (prio 0, i/o): isd-mem
0000000014000000-00000000140000ff (prio -1000, i/o): gt64120_i2o
0000000014001000-000000001bffffff [hole]
000000001c000000-000000001c7fffff (prio -10000, i/o): CS0
000000001c800000-000000001cffffff (prio -10000, i/o): CS1
000000001d000000-000000001effffff (prio -10000, i/o): CS2
000000001e000000-000000001e3fffff (prio 0, romd): mips_malta.bios
000000001f000000-000000001f0008ff (prio 0, i/o): alias malta-fpga
@malta-fpga 0000000000000000-00000000000008ff
000000001f000000-000000001fbfffff (prio -10000, i/o): CS3
000000001f000900-000000001f00093f (prio 0, i/o): serial
000000001f000a00-000000001f00ffff (prio 0, i/o): alias malta-fpga
@malta-fpga 0000000000000a00-000000000000ffff
000000001fc00000-000000001fffffff (prio 0, rom): bios.1fc
000000001fc00000-000000001fffffff (prio -10000, i/o): BootCS
0000000020000000-0000000021ffffff (prio -1000, i/o): pci1-io
0000000022000000-0000000023ffffff (prio -10000, i/o): pci1-mem0
0000000024000000-0000000025ffffff (prio -10000, i/o): pci1-mem1
0000000080000000-0000000087ffffff (prio 0, ram): mips_malta.ram
00000000f2000000-00000000f3ffffff (prio 0, i/o): alias pci0-mem1
@pci0-mem 00000000f2000000-00000000f3ffffff
> + empty_slot_init("CS0", 0x1c000000, 8 * MiB);
> + empty_slot_init("CS1", 0x1c800000, 8 * MiB);
> + empty_slot_init("CS2", 0x1d000000, 32 * MiB);
> + empty_slot_init("CS3", 0x1f000000, 12 * MiB);
I'm not very happy to add a non-pow2 range, but this is how
it appears on the datasheet. I suppose the correct range is
16MB with lower priority than the BootCS.
> + empty_slot_init("BootCS", 0x1fc00000, 4 * MiB);
> + create_unimplemented_device("pci1-io", 0x20000000, 32 * MiB);
> + empty_slot_init("pci1-mem0", 0x22000000, 32 * MiB);
> + empty_slot_init("pci1-mem1", 0x24000000, 32 * MiB);
This part is new, and could go in a separate patch:
Currently, no guest ever accessed this space.
Regards,
Phil.
> +
> return phb->bus;
> }
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 00/10] hw/pci-host: Clean the GT64120 north bridge
2019-07-01 17:45 ` Philippe Mathieu-Daudé
@ 2019-07-01 18:39 ` Aleksandar Markovic
2019-07-02 8:50 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 31+ messages in thread
From: Aleksandar Markovic @ 2019-07-01 18:39 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Peter Maydell, Thomas Huth, Aleksandar Rikalo, qemu-devel,
Hervé Poussineau, Artyom Tarasenko, Aleksandar Markovic,
Paolo Bonzini, Aurelien Jarno
On Jul 1, 2019 7:46 PM, "Philippe Mathieu-Daudé" <f4bug@amsat.org> wrote:
>
> Hi Aleksandar,
>
> On 7/1/19 7:16 PM, Aleksandar Markovic wrote:
> >
> > On Jun 25, 2019 12:31 AM, "Philippe Mathieu-Daudé" <f4bug@amsat.org
> > <mailto:f4bug@amsat.org>> wrote:
> >>
> >> Hi,
> >>
> >> This series clean the gt64120 device.
> >> It is no more target-dependent, and tracing is improved.
> >>
> >
> > If nobody objects, I am going to select majority of the patches for mips
> > queue scheduled tomorrow. Those that remain will be those that Philippe
> > still didn't make his mind.
>
> Which ones remain?
>
> For "pci-host/gt64120: Clean the decoded address space", I'd like a
> review from someone comfortable with MEMTXATTRS and address spaces.
> I'll ping on the patch.
>
Don't worry, the addres space one was left for later.
Sorry for confusion, but these patches are already in main tree (this is
from today):
Philippe Mathieu-Daudé (7):
hw/mips/gt64xxx_pci: Fix multiline comment syntax
hw/mips/gt64xxx_pci: Fix 'tabs' coding style issues
hw/mips/gt64xxx_pci: Fix 'braces' coding style issues
hw/mips/gt64xxx_pci: Fix 'spaces' coding style issues
hw/mips/gt64xxx_pci: Use qemu_log_mask() instead of debug printf()
hw/mips/gt64xxx_pci: Convert debug printf()s to trace events
hw/mips/gt64xxx_pci: Align the pci0-mem size
Let me know if you want more for tomorrow, otherwise I won't do anything.
Amicalement,
Aleksandar
> > Thanks thousand times, Philippe!
> >
> > Aleksandar
> >
> >> Regards,
> >>
> >> Phil.
> >>
> >> Based-on: 20190624220056.25861-1-f4bug@amsat.org
> > <mailto:20190624220056.25861-1-f4bug@amsat.org>
> >> https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg05304.html
> >>
> >> Philippe Mathieu-Daudé (10):
> >> hw/mips/gt64xxx_pci: Fix multiline comment syntax
> >> hw/mips/gt64xxx_pci: Fix 'tabs' coding style issues
> >> hw/mips/gt64xxx_pci: Fix 'braces' coding style issues
> >> hw/mips/gt64xxx_pci: Fix 'spaces' coding style issues
> >> hw/mips/gt64xxx_pci: Use qemu_log_mask() instead of debug printf()
> >> hw/mips/gt64xxx_pci: Convert debug printf()s to trace events
> >> hw/mips/gt64xxx_pci: Align the pci0-mem size
> >> hw/mips/gt64xxx_pci: Add a 'cpu_big_endian' qdev property
> >> hw/mips/gt64xxx_pci: Move it to hw/pci-host/
> >> hw/pci-host/gt64120: Clean the decoded address space
> >>
> >> Makefile.objs | 1 +
> >> include/hw/mips/mips.h | 2 +-
> >> hw/mips/mips_malta.c | 8 +-
> >> hw/{mips/gt64xxx_pci.c => pci-host/gt64120.c} | 542 ++++++++++--------
> >> MAINTAINERS | 2 +-
> >> hw/mips/Makefile.objs | 2 +-
> >> hw/mips/trace-events | 0
> >> hw/pci-host/Makefile.objs | 2 +-
> >> hw/pci-host/trace-events | 5 +
> >> 9 files changed, 307 insertions(+), 257 deletions(-)
> >> rename hw/{mips/gt64xxx_pci.c => pci-host/gt64120.c} (68%)
> >> create mode 100644 hw/mips/trace-events
> >>
> >> --
> >> 2.19.1
> >>
> >>
> >
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH 00/10] hw/pci-host: Clean the GT64120 north bridge
2019-07-01 18:39 ` Aleksandar Markovic
@ 2019-07-02 8:50 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-07-02 8:50 UTC (permalink / raw)
To: Aleksandar Markovic, Philippe Mathieu-Daudé
Cc: Peter Maydell, Thomas Huth, Aleksandar Rikalo, qemu-devel,
Hervé Poussineau, Aurelien Jarno, Aleksandar Markovic,
Paolo Bonzini, Artyom Tarasenko
On 7/1/19 8:39 PM, Aleksandar Markovic wrote:
> On Jul 1, 2019 7:46 PM, "Philippe Mathieu-Daudé" <f4bug@amsat.org> wrote:
>>
>> Hi Aleksandar,
>>
>> On 7/1/19 7:16 PM, Aleksandar Markovic wrote:
>>>
>>> On Jun 25, 2019 12:31 AM, "Philippe Mathieu-Daudé" <f4bug@amsat.org
>>> <mailto:f4bug@amsat.org>> wrote:
>>>>
>>>> Hi,
>>>>
>>>> This series clean the gt64120 device.
>>>> It is no more target-dependent, and tracing is improved.
>>>>
>>>
>>> If nobody objects, I am going to select majority of the patches for mips
>>> queue scheduled tomorrow. Those that remain will be those that Philippe
>>> still didn't make his mind.
>>
>> Which ones remain?
>>
>> For "pci-host/gt64120: Clean the decoded address space", I'd like a
>> review from someone comfortable with MEMTXATTRS and address spaces.
>> I'll ping on the patch.
>>
>
> Don't worry, the addres space one was left for later.
>
> Sorry for confusion, but these patches are already in main tree (this is
> from today):
>
> Philippe Mathieu-Daudé (7):
>
> hw/mips/gt64xxx_pci: Fix multiline comment syntax
>
> hw/mips/gt64xxx_pci: Fix 'tabs' coding style issues
>
> hw/mips/gt64xxx_pci: Fix 'braces' coding style issues
>
> hw/mips/gt64xxx_pci: Fix 'spaces' coding style issues
>
> hw/mips/gt64xxx_pci: Use qemu_log_mask() instead of debug printf()
>
> hw/mips/gt64xxx_pci: Convert debug printf()s to trace events
>
> hw/mips/gt64xxx_pci: Align the pci0-mem size
>
> Let me know if you want more for tomorrow, otherwise I won't do anything.
Excellent, thank you!
patch #8 "hw/mips/gt64xxx_pci: Add a 'cpu_big_endian' qdev property"
needs R-b (Thomas/Paolo eventually).
patch #9 depends of #8
I pinged Linux kernel people for patch #10.
Regards,
Phil.
> Amicalement,
> Aleksandar
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [RFC PATCH 10/10] hw/pci-host/gt64120: Clean the decoded address space
2019-07-01 18:06 ` [Qemu-devel] " Philippe Mathieu-Daudé
@ 2019-07-17 20:58 ` Paul Burton
-1 siblings, 0 replies; 31+ messages in thread
From: Paul Burton @ 2019-07-17 20:58 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, Paolo Bonzini, Hervé Poussineau,
Aleksandar Markovic, Aurelien Jarno, Aleksandar Rikalo,
Thomas Huth, Peter Maydell, Artyom Tarasenko, Ralf Baechle,
James Hogan, linux-mips
Hi Philippe,
Sorry for the delay; I was away for a few weeks visiting family.
On Mon, Jul 01, 2019 at 08:06:21PM +0200, Philippe Mathieu-Daudé wrote:
> The difference with the previous content is now we have
> two new holes:
>
> - 0x02000000-0x10000000
> - 0x14001000-0x1c000000
>
> Ralf/Paul/James, what should happen when a guest access these
> holes (hardware PoV, no QEMU)?
I don't have a Malta handy to check (they're mostly "retired" these
days & the vast majority of surviving ones use newer FPGA daughtercards
with the MSC01-style system controller anyway), but I believe writes
just get dropped & reads return zero. At least that's the way I recall
most accesses to unused address ranges working on Malta.
(Which is really annoying sometimes, and the newer Boston system gives
you a bus error in the equivalent scenario.)
Thanks,
Paul
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 10/10] hw/pci-host/gt64120: Clean the decoded address space
@ 2019-07-17 20:58 ` Paul Burton
0 siblings, 0 replies; 31+ messages in thread
From: Paul Burton @ 2019-07-17 20:58 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Peter Maydell, Thomas Huth, Aleksandar Rikalo, James Hogan,
qemu-devel, linux-mips, Hervé Poussineau, Artyom Tarasenko,
Aleksandar Markovic, Paolo Bonzini, Aurelien Jarno
Hi Philippe,
Sorry for the delay; I was away for a few weeks visiting family.
On Mon, Jul 01, 2019 at 08:06:21PM +0200, Philippe Mathieu-Daudé wrote:
> The difference with the previous content is now we have
> two new holes:
>
> - 0x02000000-0x10000000
> - 0x14001000-0x1c000000
>
> Ralf/Paul/James, what should happen when a guest access these
> holes (hardware PoV, no QEMU)?
I don't have a Malta handy to check (they're mostly "retired" these
days & the vast majority of surviving ones use newer FPGA daughtercards
with the MSC01-style system controller anyway), but I believe writes
just get dropped & reads return zero. At least that's the way I recall
most accesses to unused address ranges working on Malta.
(Which is really annoying sometimes, and the newer Boston system gives
you a bus error in the equivalent scenario.)
Thanks,
Paul
^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2019-07-17 20:58 UTC | newest]
Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-24 22:28 [Qemu-devel] [PATCH 00/10] hw/pci-host: Clean the GT64120 north bridge Philippe Mathieu-Daudé
2019-06-24 22:28 ` [Qemu-devel] [PATCH 01/10] hw/mips/gt64xxx_pci: Fix multiline comment syntax Philippe Mathieu-Daudé
2019-06-25 0:20 ` Aleksandar Markovic
2019-06-24 22:28 ` [Qemu-devel] [PATCH 02/10] hw/mips/gt64xxx_pci: Fix 'tabs' coding style issues Philippe Mathieu-Daudé
2019-06-25 0:21 ` Aleksandar Markovic
2019-06-24 22:28 ` [Qemu-devel] [PATCH 03/10] hw/mips/gt64xxx_pci: Fix 'braces' " Philippe Mathieu-Daudé
2019-06-25 0:22 ` Aleksandar Markovic
2019-06-24 22:28 ` [Qemu-devel] [PATCH 04/10] hw/mips/gt64xxx_pci: Fix 'spaces' " Philippe Mathieu-Daudé
2019-06-25 0:23 ` Aleksandar Markovic
2019-06-24 22:28 ` [Qemu-devel] [PATCH 05/10] hw/mips/gt64xxx_pci: Use qemu_log_mask() instead of debug printf() Philippe Mathieu-Daudé
2019-06-25 0:37 ` Aleksandar Markovic
2019-06-25 7:14 ` Philippe Mathieu-Daudé
2019-06-25 18:16 ` Aleksandar Markovic
2019-06-24 22:28 ` [Qemu-devel] [PATCH 06/10] hw/mips/gt64xxx_pci: Convert debug printf()s to trace events Philippe Mathieu-Daudé
2019-06-25 0:40 ` Aleksandar Markovic
2019-06-25 10:00 ` Philippe Mathieu-Daudé
2019-06-25 18:17 ` Aleksandar Markovic
2019-06-24 22:28 ` [Qemu-devel] [PATCH 07/10] hw/mips/gt64xxx_pci: Align the pci0-mem size Philippe Mathieu-Daudé
2019-06-25 0:43 ` Aleksandar Markovic
2019-06-25 7:41 ` Philippe Mathieu-Daudé
2019-06-24 22:28 ` [Qemu-devel] [PATCH 08/10] hw/mips/gt64xxx_pci: Add a 'cpu_big_endian' qdev property Philippe Mathieu-Daudé
2019-06-24 22:28 ` [Qemu-devel] [PATCH 09/10] hw/mips/gt64xxx_pci: Move it to hw/pci-host/ Philippe Mathieu-Daudé
2019-06-24 22:28 ` [Qemu-devel] [RFC PATCH 10/10] hw/pci-host/gt64120: Clean the decoded address space Philippe Mathieu-Daudé
2019-07-01 18:06 ` Philippe Mathieu-Daudé
2019-07-01 18:06 ` [Qemu-devel] " Philippe Mathieu-Daudé
2019-07-17 20:58 ` Paul Burton
2019-07-17 20:58 ` [Qemu-devel] " Paul Burton
2019-07-01 17:16 ` [Qemu-devel] [PATCH 00/10] hw/pci-host: Clean the GT64120 north bridge Aleksandar Markovic
2019-07-01 17:45 ` Philippe Mathieu-Daudé
2019-07-01 18:39 ` Aleksandar Markovic
2019-07-02 8:50 ` Philippe Mathieu-Daudé
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.