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
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
>
>
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.
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
>
>
© 2016 - 2026 Red Hat, Inc.