[PATCH 1/2] hw/gpio/aspeed: Don't let guests modify input pins

[Peter Delevoryas]

It seems that aspeed_gpio_update is allowing the value for input pins to be
modified through register writes and QOM property modification.

The QOM property modification is fine, but modifying the value through
register writes from the guest OS seems wrong if the pin's direction is set
to input.

The datasheet specifies that "0" bits in the direction register select input
mode, and "1" selects output mode.

OpenBMC userspace code is accidentally writing 0's to the GPIO data
registers somewhere (or perhaps the driver is doing it through a reset or
something), and this is overwriting GPIO FRU information (board ID, slot
presence pins) that is initialized in Aspeed machine reset code (see
fby35_reset() in hw/arm/aspeed.c).

Signed-off-by: Peter Delevoryas <peter@pjd.dev>
Fixes: 4b7f956862dc ("hw/gpio: Add basic Aspeed GPIO model for AST2400 and 
AST2500")
---
 hw/gpio/aspeed_gpio.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/hw/gpio/aspeed_gpio.c b/hw/gpio/aspeed_gpio.c
index a62a673857..2eae427201 100644
--- a/hw/gpio/aspeed_gpio.c
+++ b/hw/gpio/aspeed_gpio.c
@@ -268,7 +268,7 @@ static ptrdiff_t aspeed_gpio_set_idx(AspeedGPIOState *s, 
GPIOSets *regs)
 }
 
 static void aspeed_gpio_update(AspeedGPIOState *s, GPIOSets *regs,
-                               uint32_t value)
+                               uint32_t value, bool force)
 {
     uint32_t input_mask = regs->input_mask;
     uint32_t direction = regs->direction;
@@ -293,10 +293,12 @@ static void aspeed_gpio_update(AspeedGPIOState *s, 
GPIOSets *regs,
             }
 
             /* ...then update the state. */
-            if (mask & new) {
-                regs->data_value |= mask;
-            } else {
-                regs->data_value &= ~mask;
+            if (direction & mask || force) {
+                if (mask & new) {
+                    regs->data_value |= mask;
+                } else {
+                    regs->data_value &= ~mask;
+                }
             }
 
             /* If the gpio is set to output... */
@@ -339,7 +341,7 @@ static void aspeed_gpio_set_pin_level(AspeedGPIOState *s, 
uint32_t set_idx,
         value &= ~pin_mask;
     }
 
-    aspeed_gpio_update(s, &s->sets[set_idx], value);
+    aspeed_gpio_update(s, &s->sets[set_idx], value, true);
 }
 
 /*
@@ -653,7 +655,7 @@ static void aspeed_gpio_write_index_mode(void *opaque, 
hwaddr offset,
         reg_value = update_value_control_source(set, set->data_value,
                                                 reg_value);
         set->data_read = reg_value;
-        aspeed_gpio_update(s, set, reg_value);
+        aspeed_gpio_update(s, set, reg_value, false);
         return;
     case gpio_reg_idx_direction:
         reg_value = set->direction;
@@ -753,7 +755,7 @@ static void aspeed_gpio_write_index_mode(void *opaque, 
hwaddr offset,
             __func__, offset, data, reg_idx_type);
         return;
     }
-    aspeed_gpio_update(s, set, set->data_value);
+    aspeed_gpio_update(s, set, set->data_value, false);
     return;
 }
 
@@ -799,7 +801,7 @@ static void aspeed_gpio_write(void *opaque, hwaddr offset, 
uint64_t data,
         data &= props->output;
         data = update_value_control_source(set, set->data_value, data);
         set->data_read = data;
-        aspeed_gpio_update(s, set, data);
+        aspeed_gpio_update(s, set, data, false);
         return;
     case gpio_reg_direction:
         /*
@@ -875,7 +877,7 @@ static void aspeed_gpio_write(void *opaque, hwaddr offset, 
uint64_t data,
                       PRIx64"\n", __func__, offset);
         return;
     }
-    aspeed_gpio_update(s, set, set->data_value);
+    aspeed_gpio_update(s, set, set->data_value, false);
     return;
 }
 
-- 
2.36.1