RE: [PATCH] hw/misc/bcm2835_thermal: Handle invalid address accesses gracefully

[Xingtao Yao (Fujitsu)]

Hi, zheyu

> -----Original Message-----
> From: qemu-devel-bounces+yaoxt.fnst=fujitsu.com@nongnu.org
> <qemu-devel-bounces+yaoxt.fnst=fujitsu.com@nongnu.org> On Behalf Of Zheyu
> Ma
> Sent: Sunday, June 30, 2024 11:14 PM
> To: Peter Maydell <peter.maydell@linaro.org>; Philippe Mathieu-Daudé
> <philmd@linaro.org>
> Cc: Zheyu Ma <zheyuma97@gmail.com>; qemu-arm@nongnu.org;
> qemu-devel@nongnu.org
> Subject: [PATCH] hw/misc/bcm2835_thermal: Handle invalid address accesses
> gracefully
> 
> This commit handles invalid address accesses gracefully in both read and write
> functions. Instead of asserting and aborting, it logs an error message and 
> returns
> a neutral value for read operations and does nothing for write operations.
> 
> Error log:
> ERROR:hw/misc/bcm2835_thermal.c:55:bcm2835_thermal_read: code should not
> be reached
> Bail out! ERROR:hw/misc/bcm2835_thermal.c:55:bcm2835_thermal_read: code
> should not be reached
> Aborted
> 
> Reproducer:
> cat << EOF | qemu-system-aarch64 -display \
> none -machine accel=qtest, -m 512M -machine raspi3b -m 1G -qtest stdio
> readw 0x3f212003
> EOF
> 
> Signed-off-by: Zheyu Ma <zheyuma97@gmail.com>
> ---
>  hw/misc/bcm2835_thermal.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/misc/bcm2835_thermal.c b/hw/misc/bcm2835_thermal.c
> index ee7816b8a5..5c2a429d58 100644
> --- a/hw/misc/bcm2835_thermal.c
> +++ b/hw/misc/bcm2835_thermal.c
> @@ -51,8 +51,10 @@ static uint64_t bcm2835_thermal_read(void *opaque,
> hwaddr addr, unsigned size)
>          val = FIELD_DP32(bcm2835_thermal_temp2adc(25), STAT, VALID, true);
>          break;
>      default:
> -        /* MemoryRegionOps are aligned, so this can not happen. */
> -        g_assert_not_reached();
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "bcm2835_thermal_read: invalid address 0x%"
> +                      HWADDR_PRIx "\n", addr);
> +        val = 0;
>      }
>      return val;
>  }
> @@ -72,8 +74,10 @@ static void bcm2835_thermal_write(void *opaque, hwaddr
> addr,
>                         __func__, value, addr);
>          break;
>      default:
> -        /* MemoryRegionOps are aligned, so this can not happen. */
> -        g_assert_not_reached();
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "bcm2835_thermal_write: invalid address 0x%"
> +                      HWADDR_PRIx "\n", addr);
> +        break;
>      }
>  }

the default branch will never be reached in normal access, so I think this 
modification is not needed.

> 
> --
> 2.34.1
>