[PATCH 04/13] hw/pci-host/astro: Avoid aborting on access failure

deller@kernel.org posted 13 patches 9 months, 3 weeks ago
Maintainers: Richard Henderson <richard.henderson@linaro.org>, Helge Deller <deller@gmx.de>
There is a newer version of this series
[PATCH 04/13] hw/pci-host/astro: Avoid aborting on access failure
Posted by deller@kernel.org 9 months, 3 weeks ago
From: Helge Deller <deller@gmx.de>

Instead of stopping the emulation, report a MEMTX_DECODE_ERROR if the OS
tries to access non-existent registers.

Signed-off-by: Helge Deller <deller@gmx.de>
---
 hw/pci-host/astro.c | 30 +++++++++++++-----------------
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/hw/pci-host/astro.c b/hw/pci-host/astro.c
index 37d271118c..42bd65de53 100644
--- a/hw/pci-host/astro.c
+++ b/hw/pci-host/astro.c
@@ -122,10 +122,6 @@ static MemTxResult elroy_chip_read_with_attrs(void *opaque, hwaddr addr,
     case 0x0800:                /* IOSAPIC_REG_SELECT */
         val = s->iosapic_reg_select;
         break;
-    case 0x0808:
-        val = UINT64_MAX;            /* XXX: tbc. */
-        g_assert_not_reached();
-        break;
     case 0x0810:                /* IOSAPIC_REG_WINDOW */
         switch (s->iosapic_reg_select) {
         case 0x01:              /* IOSAPIC_REG_VERSION */
@@ -135,15 +131,15 @@ static MemTxResult elroy_chip_read_with_attrs(void *opaque, hwaddr addr,
             if (s->iosapic_reg_select < ARRAY_SIZE(s->iosapic_reg)) {
                 val = s->iosapic_reg[s->iosapic_reg_select];
             } else {
-                trace_iosapic_reg_read(s->iosapic_reg_select, size, val);
-                g_assert_not_reached();
+                val = 0;
+                ret = MEMTX_DECODE_ERROR;
             }
         }
         trace_iosapic_reg_read(s->iosapic_reg_select, size, val);
         break;
     default:
-        trace_elroy_read(addr, size, val);
-        g_assert_not_reached();
+        val = 0;
+        ret = MEMTX_DECODE_ERROR;
     }
     trace_elroy_read(addr, size, val);
 
@@ -160,6 +156,7 @@ static MemTxResult elroy_chip_write_with_attrs(void *opaque, hwaddr addr,
                                               uint64_t val, unsigned size,
                                               MemTxAttrs attrs)
 {
+    MemTxResult ret = MEMTX_OK;
     ElroyState *s = opaque;
     int i;
 
@@ -191,7 +188,7 @@ static MemTxResult elroy_chip_write_with_attrs(void *opaque, hwaddr addr,
         if (s->iosapic_reg_select < ARRAY_SIZE(s->iosapic_reg)) {
             s->iosapic_reg[s->iosapic_reg_select] = val;
         } else {
-            g_assert_not_reached();
+            ret = MEMTX_DECODE_ERROR;
         }
         break;
     case 0x0840:                /* IOSAPIC_REG_EOI */
@@ -204,9 +201,9 @@ static MemTxResult elroy_chip_write_with_attrs(void *opaque, hwaddr addr,
         }
         break;
     default:
-        g_assert_not_reached();
+        ret = MEMTX_DECODE_ERROR;
     }
-    return MEMTX_OK;
+    return ret;
 }
 
 static const MemoryRegionOps elroy_chip_ops = {
@@ -594,8 +591,8 @@ static MemTxResult astro_chip_read_with_attrs(void *opaque, hwaddr addr,
 #undef EMPTY_PORT
 
     default:
-        trace_astro_chip_read(addr, size, val);
-        g_assert_not_reached();
+        val = 0;
+        ret = MEMTX_DECODE_ERROR;
     }
 
     /* for 32-bit accesses mask return value */
@@ -610,6 +607,7 @@ static MemTxResult astro_chip_write_with_attrs(void *opaque, hwaddr addr,
                                               uint64_t val, unsigned size,
                                               MemTxAttrs attrs)
 {
+    MemTxResult ret = MEMTX_OK;
     AstroState *s = opaque;
 
     trace_astro_chip_write(addr, size, val);
@@ -686,11 +684,9 @@ static MemTxResult astro_chip_write_with_attrs(void *opaque, hwaddr addr,
 #undef EMPTY_PORT
 
     default:
-        /* Controlled by astro_chip_mem_valid above.  */
-        trace_astro_chip_write(addr, size, val);
-        g_assert_not_reached();
+        ret = MEMTX_DECODE_ERROR;
     }
-    return MEMTX_OK;
+    return ret;
 }
 
 static const MemoryRegionOps astro_chip_ops = {
-- 
2.43.0
Re: [PATCH 04/13] hw/pci-host/astro: Avoid aborting on access failure
Posted by Richard Henderson 9 months, 3 weeks ago
On 2/7/24 08:20, deller@kernel.org wrote:
> @@ -610,6 +607,7 @@ static MemTxResult astro_chip_write_with_attrs(void *opaque, hwaddr addr,
>                                                 uint64_t val, unsigned size,
>                                                 MemTxAttrs attrs)
>   {
> +    MemTxResult ret = MEMTX_OK;
>       AstroState *s = opaque;
>   
>       trace_astro_chip_write(addr, size, val);
> @@ -686,11 +684,9 @@ static MemTxResult astro_chip_write_with_attrs(void *opaque, hwaddr addr,
>   #undef EMPTY_PORT
>   
>       default:
> -        /* Controlled by astro_chip_mem_valid above.  */
> -        trace_astro_chip_write(addr, size, val);
> -        g_assert_not_reached();
> +        ret = MEMTX_DECODE_ERROR;
>       }
> -    return MEMTX_OK;
> +    return ret;
>   }

For writes it's cleaner to just return these values directly than introduce a local.
Unlike reads, were you still want to go through the tracepoint on the way out.

Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~