[PATCH v2 0/2] Separate memory access logs from guest_errors

BALATON Zoltan posted 2 patches 1 year, 3 months ago
Failed in applying to current master (apply log)
docs/devel/secure-coding-practices.rst | 2 +-
include/qemu/log.h                     | 1 +
system/memory.c                        | 6 +++---
system/physmem.c                       | 2 +-
tests/avocado/smmu.py                  | 2 +-
tests/qtest/pnv-host-i2c-test.c        | 2 +-
util/log.c                             | 8 +++++++-
7 files changed, 15 insertions(+), 8 deletions(-)
[PATCH v2 0/2] Separate memory access logs from guest_errors
Posted by BALATON Zoltan 1 year, 3 months ago
Originally memory access logs were a debug define that then were
converted to log messages but were classified as guest_errors which
already logs misc errors. As invalid memory access logs can come from
accessing not emulated peripherals or memory areas, these often
generate a lot of messages that are better be controlled separately
from other errors to avoid obscuring those. As an example try
'qemu-system-ppc -d guest_errors' to see the problem. After this
series the actual guest error logs are easier to spot. I've tried to
submit this before but there were some people who liked the current
behaviour so now this series has another patch that preserves the old
option printing a warning to allow time to get used to the new
behaviour (which actually brings back the old behaviour when mem
access logs were a debug define). This second patch is optional if
changing the behaviour without notice is acceptable. As these are
debug switches no deprecation period is needed so the second patch
could be omitted. I leave that decision to the maintainers.

v2: Rename the option from memaccess to invalid_mem as suggested by
Peter Maydell

Regards,
BALATON Zoltan

BALATON Zoltan (2):
  log: Add separate debug option for logging invalid memory accesses
  log: Suggest using -d guest_error,invalid_mem instead of guest_errors

 docs/devel/secure-coding-practices.rst | 2 +-
 include/qemu/log.h                     | 1 +
 system/memory.c                        | 6 +++---
 system/physmem.c                       | 2 +-
 tests/avocado/smmu.py                  | 2 +-
 tests/qtest/pnv-host-i2c-test.c        | 2 +-
 util/log.c                             | 8 +++++++-
 7 files changed, 15 insertions(+), 8 deletions(-)

-- 
2.30.9
Re: [PATCH v2 0/2] Separate memory access logs from guest_errors
Posted by BALATON Zoltan 1 year, 1 month ago
On Sat, 2 Nov 2024, BALATON Zoltan wrote:
> Originally memory access logs were a debug define that then were
> converted to log messages but were classified as guest_errors which
> already logs misc errors. As invalid memory access logs can come from
> accessing not emulated peripherals or memory areas, these often
> generate a lot of messages that are better be controlled separately
> from other errors to avoid obscuring those. As an example try
> 'qemu-system-ppc -d guest_errors' to see the problem. After this
> series the actual guest error logs are easier to spot. I've tried to
> submit this before but there were some people who liked the current
> behaviour so now this series has another patch that preserves the old
> option printing a warning to allow time to get used to the new
> behaviour (which actually brings back the old behaviour when mem
> access logs were a debug define). This second patch is optional if
> changing the behaviour without notice is acceptable. As these are
> debug switches no deprecation period is needed so the second patch
> could be omitted. I leave that decision to the maintainers.
>
> v2: Rename the option from memaccess to invalid_mem as suggested by
> Peter Maydell

Ping? <https://patchew.org/QEMU/cover.1730549443.git.balaton@eik.bme.hu/>

Regards,
BALATON Zoltan

> BALATON Zoltan (2):
>  log: Add separate debug option for logging invalid memory accesses
>  log: Suggest using -d guest_error,invalid_mem instead of guest_errors
>
> docs/devel/secure-coding-practices.rst | 2 +-
> include/qemu/log.h                     | 1 +
> system/memory.c                        | 6 +++---
> system/physmem.c                       | 2 +-
> tests/avocado/smmu.py                  | 2 +-
> tests/qtest/pnv-host-i2c-test.c        | 2 +-
> util/log.c                             | 8 +++++++-
> 7 files changed, 15 insertions(+), 8 deletions(-)
>
>
Re: [PATCH v2 0/2] Separate memory access logs from guest_errors
Posted by Peter Maydell 1 year, 1 month ago
On Fri, 13 Dec 2024 at 12:41, BALATON Zoltan <balaton@eik.bme.hu> wrote:
>
> On Sat, 2 Nov 2024, BALATON Zoltan wrote:
> > Originally memory access logs were a debug define that then were
> > converted to log messages but were classified as guest_errors which
> > already logs misc errors. As invalid memory access logs can come from
> > accessing not emulated peripherals or memory areas, these often
> > generate a lot of messages that are better be controlled separately
> > from other errors to avoid obscuring those. As an example try
> > 'qemu-system-ppc -d guest_errors' to see the problem. After this
> > series the actual guest error logs are easier to spot. I've tried to
> > submit this before but there were some people who liked the current
> > behaviour so now this series has another patch that preserves the old
> > option printing a warning to allow time to get used to the new
> > behaviour (which actually brings back the old behaviour when mem
> > access logs were a debug define). This second patch is optional if
> > changing the behaviour without notice is acceptable. As these are
> > debug switches no deprecation period is needed so the second patch
> > could be omitted. I leave that decision to the maintainers.
> >
> > v2: Rename the option from memaccess to invalid_mem as suggested by
> > Peter Maydell
>
> Ping? <https://patchew.org/QEMU/cover.1730549443.git.balaton@eik.bme.hu/>

Personally I do not think we need the back-compat handling so
I would be happy with just patch 1.

(I think the way I envisaged this working was that if there
were cases where some missing device generates a ton of
errors that we would stick in a TYPE_UNIMPLEMENTED_DEVICE in
that region, so that the logging in the invalid access
paths in memory.c would not be hit except where the guest
was doing something wrong. But obviously we haven't actually
done that, and I don't think there's a strong reason not
to split out the invalid-access logging into its own
category.)

thanks
-- PMM