Skip to content

Commit

Permalink
hw: aspeed_gpio: Fix GPIO array indexing
Browse files Browse the repository at this point in the history 
The gpio array is declared as a dense array:

  qemu_irq gpios[ASPEED_GPIO_NR_PINS];

(AST2500 has 228, AST2400 has 216, AST2600 has 208)

However, this array is used like a matrix of GPIO sets
(e.g. gpio[NR_SETS][NR_PINS_PER_SET] = gpio[8][32])

  size_t offset = set * GPIOS_PER_SET + gpio;
  qemu_set_irq(s->gpios[offset], !!(new & mask));

This can result in an out-of-bounds access to "s->gpios" because the
gpio sets do _not_ have the same length. Some of the groups (e.g.
GPIOAB) only have 4 pins. 228 != 8 * 32 == 256.

To fix this, I converted the gpio array from dense to sparse, to that
match both the hardware layout and this existing indexing code.

Fixes: 4b7f956 ("hw/gpio: Add basic Aspeed GPIO model for AST2400 and AST2500")
Signed-off-by: Peter Delevoryas <pdel@fb.com>
Message-Id: <20211008033501.934729-2-pdel@fb.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
  • Loading branch information
@peterdelevoryas @legoater
peterdelevoryas authored and legoater committed Oct 12, 2021
1 parent 9fffe14 commit 87bd33e
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 50 deletions.
80 changes: 33 additions & 47 deletions hw/gpio/aspeed_gpio.c
View file
Expand Up @@ -16,11 +16,7 @@
#include "hw/irq.h"
#include "migration/vmstate.h"

#define GPIOS_PER_REG 32
#define GPIOS_PER_SET GPIOS_PER_REG
#define GPIO_PIN_GAP_SIZE 4
#define GPIOS_PER_GROUP 8
#define GPIO_GROUP_SHIFT 3

/* GPIO Source Types */
#define ASPEED_CMD_SRC_MASK 0x01010101
Expand Down Expand Up @@ -259,7 +255,7 @@ static void aspeed_gpio_update(AspeedGPIOState *s, GPIOSets *regs,

diff = old ^ new;
if (diff) {
for (gpio = 0; gpio < GPIOS_PER_REG; gpio++) {
for (gpio = 0; gpio < ASPEED_GPIOS_PER_SET; gpio++) {
uint32_t mask = 1 << gpio;

/* If the gpio needs to be updated... */
Expand All @@ -283,8 +279,7 @@ static void aspeed_gpio_update(AspeedGPIOState *s, GPIOSets *regs,
if (direction & mask) {
/* ...trigger the line-state IRQ */
ptrdiff_t set = aspeed_gpio_set_idx(s, regs);
size_t offset = set * GPIOS_PER_SET + gpio;
qemu_set_irq(s->gpios[offset], !!(new & mask));
qemu_set_irq(s->gpios[set][gpio], !!(new & mask));
} else {
/* ...otherwise if we meet the line's current IRQ policy... */
if (aspeed_evaluate_irq(regs, old & mask, gpio)) {
Expand All @@ -297,21 +292,6 @@ static void aspeed_gpio_update(AspeedGPIOState *s, GPIOSets *regs,
qemu_set_irq(s->irq, !!(s->pending));
}

static uint32_t aspeed_adjust_pin(AspeedGPIOState *s, uint32_t pin)
{
AspeedGPIOClass *agc = ASPEED_GPIO_GET_CLASS(s);
/*
* The 2500 has a 4 pin gap in group AB and the 2400 has a 4 pin
* gap in group Y (and only four pins in AB but this is the last group so
* it doesn't matter).
*/
if (agc->gap && pin >= agc->gap) {
pin += GPIO_PIN_GAP_SIZE;
}

return pin;
}

static bool aspeed_gpio_get_pin_level(AspeedGPIOState *s, uint32_t set_idx,
uint32_t pin)
{
Expand Down Expand Up @@ -367,7 +347,7 @@ static uint32_t update_value_control_source(GPIOSets *regs, uint32_t old_value,
uint32_t new_value = 0;

/* for each group in set */
for (i = 0; i < GPIOS_PER_REG; i += GPIOS_PER_GROUP) {
for (i = 0; i < ASPEED_GPIOS_PER_SET; i += GPIOS_PER_GROUP) {
cmd_source = extract32(regs->cmd_source_0, i, 1)
| (extract32(regs->cmd_source_1, i, 1) << 1);

Expand Down Expand Up @@ -637,7 +617,7 @@ static void aspeed_gpio_write(void *opaque, hwaddr offset, uint64_t data,
* bidirectional | 1 | 1 | data
* input only | 1 | 0 | 0
* output only | 0 | 1 | 1
* no pin / gap | 0 | 0 | 0
* no pin | 0 | 0 | 0
*
* which is captured by:
* data = ( data | ~input) & output;
Expand Down Expand Up @@ -779,7 +759,7 @@ static void aspeed_gpio_set_pin(Object *obj, Visitor *v, const char *name,
}

/****************** Setup functions ******************/
static const GPIOSetProperties ast2400_set_props[] = {
static const GPIOSetProperties ast2400_set_props[ASPEED_GPIO_MAX_NR_SETS] = {
[0] = {0xffffffff, 0xffffffff, {"A", "B", "C", "D"} },
[1] = {0xffffffff, 0xffffffff, {"E", "F", "G", "H"} },
[2] = {0xffffffff, 0xffffffff, {"I", "J", "K", "L"} },
Expand All @@ -789,7 +769,7 @@ static const GPIOSetProperties ast2400_set_props[] = {
[6] = {0x0000000f, 0x0fffff0f, {"Y", "Z", "AA", "AB"} },
};

static const GPIOSetProperties ast2500_set_props[] = {
static const GPIOSetProperties ast2500_set_props[ASPEED_GPIO_MAX_NR_SETS] = {
[0] = {0xffffffff, 0xffffffff, {"A", "B", "C", "D"} },
[1] = {0xffffffff, 0xffffffff, {"E", "F", "G", "H"} },
[2] = {0xffffffff, 0xffffffff, {"I", "J", "K", "L"} },
Expand All @@ -800,7 +780,7 @@ static const GPIOSetProperties ast2500_set_props[] = {
[7] = {0x000000ff, 0x000000ff, {"AC"} },
};

static GPIOSetProperties ast2600_3_3v_set_props[] = {
static GPIOSetProperties ast2600_3_3v_set_props[ASPEED_GPIO_MAX_NR_SETS] = {
[0] = {0xffffffff, 0xffffffff, {"A", "B", "C", "D"} },
[1] = {0xffffffff, 0xffffffff, {"E", "F", "G", "H"} },
[2] = {0xffffffff, 0xffffffff, {"I", "J", "K", "L"} },
Expand All @@ -810,7 +790,7 @@ static GPIOSetProperties ast2600_3_3v_set_props[] = {
[6] = {0x0000ffff, 0x0000ffff, {"Y", "Z"} },
};

static GPIOSetProperties ast2600_1_8v_set_props[] = {
static GPIOSetProperties ast2600_1_8v_set_props[ASPEED_GPIO_MAX_NR_SETS] = {
[0] = {0xffffffff, 0xffffffff, {"18A", "18B", "18C", "18D"} },
[1] = {0x0000000f, 0x0000000f, {"18E"} },
};
Expand All @@ -836,14 +816,20 @@ static void aspeed_gpio_realize(DeviceState *dev, Error **errp)
AspeedGPIOState *s = ASPEED_GPIO(dev);
SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
AspeedGPIOClass *agc = ASPEED_GPIO_GET_CLASS(s);
int pin;

/* Interrupt parent line */
sysbus_init_irq(sbd, &s->irq);

/* Individual GPIOs */
for (pin = 0; pin < agc->nr_gpio_pins; pin++) {
sysbus_init_irq(sbd, &s->gpios[pin]);
for (int i = 0; i < ASPEED_GPIO_MAX_NR_SETS; i++) {
const GPIOSetProperties *props = &agc->props[i];
uint32_t skip = ~(props->input | props->output);
for (int j = 0; j < ASPEED_GPIOS_PER_SET; j++) {
if (skip >> j & 1) {
continue;
}
sysbus_init_irq(sbd, &s->gpios[i][j]);
}
}

memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_gpio_ops, s,
Expand All @@ -856,20 +842,22 @@ static void aspeed_gpio_init(Object *obj)
{
AspeedGPIOState *s = ASPEED_GPIO(obj);
AspeedGPIOClass *agc = ASPEED_GPIO_GET_CLASS(s);
int pin;

for (pin = 0; pin < agc->nr_gpio_pins; pin++) {
char *name;
int set_idx = pin / GPIOS_PER_SET;
int pin_idx = aspeed_adjust_pin(s, pin) - (set_idx * GPIOS_PER_SET);
int group_idx = pin_idx >> GPIO_GROUP_SHIFT;
const GPIOSetProperties *props = &agc->props[set_idx];

name = g_strdup_printf("gpio%s%d", props->group_label[group_idx],
pin_idx % GPIOS_PER_GROUP);
object_property_add(obj, name, "bool", aspeed_gpio_get_pin,
aspeed_gpio_set_pin, NULL, NULL);
g_free(name);

for (int i = 0; i < ASPEED_GPIO_MAX_NR_SETS; i++) {
const GPIOSetProperties *props = &agc->props[i];
uint32_t skip = ~(props->input | props->output);
for (int j = 0; j < ASPEED_GPIOS_PER_SET; j++) {
if (skip >> j & 1) {
continue;
}
int group_idx = j / GPIOS_PER_GROUP;
int pin_idx = j % GPIOS_PER_GROUP;
const char *group = &props->group_label[group_idx][0];
char *name = g_strdup_printf("gpio%s%d", group, pin_idx);
object_property_add(obj, name, "bool", aspeed_gpio_get_pin,
aspeed_gpio_set_pin, NULL, NULL);
g_free(name);
}
}
}

Expand Down Expand Up @@ -926,7 +914,6 @@ static void aspeed_gpio_ast2400_class_init(ObjectClass *klass, void *data)
agc->props = ast2400_set_props;
agc->nr_gpio_pins = 216;
agc->nr_gpio_sets = 7;
agc->gap = 196;
agc->reg_table = aspeed_3_3v_gpios;
}

Expand All @@ -937,7 +924,6 @@ static void aspeed_gpio_2500_class_init(ObjectClass *klass, void *data)
agc->props = ast2500_set_props;
agc->nr_gpio_pins = 228;
agc->nr_gpio_sets = 8;
agc->gap = 220;
agc->reg_table = aspeed_3_3v_gpios;
}

Expand Down
5 changes: 2 additions & 3 deletions include/hw/gpio/aspeed_gpio.h
View file
Expand Up @@ -17,9 +17,9 @@
OBJECT_DECLARE_TYPE(AspeedGPIOState, AspeedGPIOClass, ASPEED_GPIO)

#define ASPEED_GPIO_MAX_NR_SETS 8
#define ASPEED_GPIOS_PER_SET 32
#define ASPEED_REGS_PER_BANK 14
#define ASPEED_GPIO_MAX_NR_REGS (ASPEED_REGS_PER_BANK * ASPEED_GPIO_MAX_NR_SETS)
#define ASPEED_GPIO_NR_PINS 228
#define ASPEED_GROUPS_PER_SET 4
#define ASPEED_GPIO_NR_DEBOUNCE_REGS 3
#define ASPEED_CHARS_PER_GROUP_LABEL 4
Expand Down Expand Up @@ -60,7 +60,6 @@ struct AspeedGPIOClass {
const GPIOSetProperties *props;
uint32_t nr_gpio_pins;
uint32_t nr_gpio_sets;
uint32_t gap;
const AspeedGPIOReg *reg_table;
};

Expand All @@ -72,7 +71,7 @@ struct AspeedGPIOState {
MemoryRegion iomem;
int pending;
qemu_irq irq;
qemu_irq gpios[ASPEED_GPIO_NR_PINS];
qemu_irq gpios[ASPEED_GPIO_MAX_NR_SETS][ASPEED_GPIOS_PER_SET];

/* Parallel GPIO Registers */
uint32_t debounce_regs[ASPEED_GPIO_NR_DEBOUNCE_REGS];
Expand Down

0 comments on commit 87bd33e

Please sign in to comment.