* [PULL 01/17] hw/gpio/aspeed: Fix coding style
2024-10-24 6:34 [PULL 00/17] aspeed queue Cédric Le Goater
@ 2024-10-24 6:34 ` Cédric Le Goater
2024-10-24 6:34 ` [PULL 02/17] hw/gpio/aspeed: Support to set the different memory size Cédric Le Goater
` (16 subsequent siblings)
17 siblings, 0 replies; 34+ messages in thread
From: Cédric Le Goater @ 2024-10-24 6:34 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Jamin Lin, Cédric Le Goater
From: Jamin Lin <jamin_lin@aspeedtech.com>
Fix coding style issues from checkpatch.pl
Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
---
include/hw/gpio/aspeed_gpio.h | 2 +-
hw/gpio/aspeed_gpio.c | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/hw/gpio/aspeed_gpio.h b/include/hw/gpio/aspeed_gpio.h
index 90a12ae3182a..39febda9eaea 100644
--- a/include/hw/gpio/aspeed_gpio.h
+++ b/include/hw/gpio/aspeed_gpio.h
@@ -88,7 +88,7 @@ struct AspeedGPIOState {
qemu_irq irq;
qemu_irq gpios[ASPEED_GPIO_MAX_NR_SETS][ASPEED_GPIOS_PER_SET];
-/* Parallel GPIO Registers */
+ /* Parallel GPIO Registers */
uint32_t debounce_regs[ASPEED_GPIO_NR_DEBOUNCE_REGS];
struct GPIOSets {
uint32_t data_value; /* Reflects pin values */
diff --git a/hw/gpio/aspeed_gpio.c b/hw/gpio/aspeed_gpio.c
index 71756664dd69..00fb72a509ef 100644
--- a/hw/gpio/aspeed_gpio.c
+++ b/hw/gpio/aspeed_gpio.c
@@ -340,7 +340,8 @@ 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, ~s->sets[set_idx].direction);
+ aspeed_gpio_update(s, &s->sets[set_idx], value,
+ ~s->sets[set_idx].direction);
}
/*
@@ -629,7 +630,6 @@ static uint64_t aspeed_gpio_read(void *opaque, hwaddr offset, uint32_t size)
static void aspeed_gpio_write_index_mode(void *opaque, hwaddr offset,
uint64_t data, uint32_t size)
{
-
AspeedGPIOState *s = ASPEED_GPIO(opaque);
AspeedGPIOClass *agc = ASPEED_GPIO_GET_CLASS(s);
const GPIOSetProperties *props;
@@ -963,7 +963,7 @@ static void aspeed_gpio_set_pin(Object *obj, Visitor *v, const char *name,
aspeed_gpio_set_pin_level(s, set_idx, pin, level);
}
-/****************** Setup functions ******************/
+/* Setup functions */
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.47.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PULL 02/17] hw/gpio/aspeed: Support to set the different memory size
2024-10-24 6:34 [PULL 00/17] aspeed queue Cédric Le Goater
2024-10-24 6:34 ` [PULL 01/17] hw/gpio/aspeed: Fix coding style Cédric Le Goater
@ 2024-10-24 6:34 ` Cédric Le Goater
2024-10-24 6:34 ` [PULL 03/17] hw/gpio/aspeed: Support different memory region ops Cédric Le Goater
` (15 subsequent siblings)
17 siblings, 0 replies; 34+ messages in thread
From: Cédric Le Goater @ 2024-10-24 6:34 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Jamin Lin, Cédric Le Goater
From: Jamin Lin <jamin_lin@aspeedtech.com>
According to the datasheet of ASPEED SOCs, a GPIO controller owns 4KB of
register space for AST2700, AST2500, AST2400 and AST1030; owns 2KB of
register space for AST2600 1.8v and owns 2KB of register space for
AST2600 3.3v.
It set the memory region size 2KB by default and it does not compatible
register space for AST2700.
Introduce a new class attribute to set the GPIO controller memory size
for different ASPEED SOCs.
Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
---
include/hw/gpio/aspeed_gpio.h | 1 +
hw/gpio/aspeed_gpio.c | 7 ++++++-
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/include/hw/gpio/aspeed_gpio.h b/include/hw/gpio/aspeed_gpio.h
index 39febda9eaea..8cd2ff54968c 100644
--- a/include/hw/gpio/aspeed_gpio.h
+++ b/include/hw/gpio/aspeed_gpio.h
@@ -76,6 +76,7 @@ struct AspeedGPIOClass {
uint32_t nr_gpio_sets;
const AspeedGPIOReg *reg_table;
unsigned reg_table_count;
+ uint64_t mem_size;
};
struct AspeedGPIOState {
diff --git a/hw/gpio/aspeed_gpio.c b/hw/gpio/aspeed_gpio.c
index 00fb72a509ef..564459ad4f07 100644
--- a/hw/gpio/aspeed_gpio.c
+++ b/hw/gpio/aspeed_gpio.c
@@ -1047,7 +1047,7 @@ static void aspeed_gpio_realize(DeviceState *dev, Error **errp)
}
memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_gpio_ops, s,
- TYPE_ASPEED_GPIO, 0x800);
+ TYPE_ASPEED_GPIO, agc->mem_size);
sysbus_init_mmio(sbd, &s->iomem);
}
@@ -1130,6 +1130,7 @@ static void aspeed_gpio_ast2400_class_init(ObjectClass *klass, void *data)
agc->nr_gpio_sets = 7;
agc->reg_table = aspeed_3_3v_gpios;
agc->reg_table_count = GPIO_3_3V_REG_ARRAY_SIZE;
+ agc->mem_size = 0x1000;
}
static void aspeed_gpio_2500_class_init(ObjectClass *klass, void *data)
@@ -1141,6 +1142,7 @@ static void aspeed_gpio_2500_class_init(ObjectClass *klass, void *data)
agc->nr_gpio_sets = 8;
agc->reg_table = aspeed_3_3v_gpios;
agc->reg_table_count = GPIO_3_3V_REG_ARRAY_SIZE;
+ agc->mem_size = 0x1000;
}
static void aspeed_gpio_ast2600_3_3v_class_init(ObjectClass *klass, void *data)
@@ -1152,6 +1154,7 @@ static void aspeed_gpio_ast2600_3_3v_class_init(ObjectClass *klass, void *data)
agc->nr_gpio_sets = 7;
agc->reg_table = aspeed_3_3v_gpios;
agc->reg_table_count = GPIO_3_3V_REG_ARRAY_SIZE;
+ agc->mem_size = 0x800;
}
static void aspeed_gpio_ast2600_1_8v_class_init(ObjectClass *klass, void *data)
@@ -1163,6 +1166,7 @@ static void aspeed_gpio_ast2600_1_8v_class_init(ObjectClass *klass, void *data)
agc->nr_gpio_sets = 2;
agc->reg_table = aspeed_1_8v_gpios;
agc->reg_table_count = GPIO_1_8V_REG_ARRAY_SIZE;
+ agc->mem_size = 0x800;
}
static void aspeed_gpio_1030_class_init(ObjectClass *klass, void *data)
@@ -1174,6 +1178,7 @@ static void aspeed_gpio_1030_class_init(ObjectClass *klass, void *data)
agc->nr_gpio_sets = 6;
agc->reg_table = aspeed_3_3v_gpios;
agc->reg_table_count = GPIO_3_3V_REG_ARRAY_SIZE;
+ agc->mem_size = 0x1000;
}
static const TypeInfo aspeed_gpio_info = {
--
2.47.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PULL 03/17] hw/gpio/aspeed: Support different memory region ops
2024-10-24 6:34 [PULL 00/17] aspeed queue Cédric Le Goater
2024-10-24 6:34 ` [PULL 01/17] hw/gpio/aspeed: Fix coding style Cédric Le Goater
2024-10-24 6:34 ` [PULL 02/17] hw/gpio/aspeed: Support to set the different memory size Cédric Le Goater
@ 2024-10-24 6:34 ` Cédric Le Goater
2024-10-24 6:34 ` [PULL 04/17] hw/gpio/aspeed: Fix clear incorrect interrupt status for GPIO index mode Cédric Le Goater
` (14 subsequent siblings)
17 siblings, 0 replies; 34+ messages in thread
From: Cédric Le Goater @ 2024-10-24 6:34 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Jamin Lin, Cédric Le Goater
From: Jamin Lin <jamin_lin@aspeedtech.com>
It set "aspeed_gpio_ops" struct which containing read and write callbacks
to be used when I/O is performed on the GPIO region.
Besides, in the previous design of ASPEED SOCs, one register is used for
setting one function for 32 GPIO pins.
ex: GPIO000 is used for setting data value for GPIO A, B, C and D in AST2600.
ex: GPIO004 is used for setting direction for GPIO A, B, C and D in AST2600.
However, the register set have a significant change in AST2700.
Each GPIO pin has their own control register. In other words, users are able to
set one GPIO pin’s direction, interrupt enable, input mask and so on
in one register. The aspeed_gpio_read/aspeed_gpio_write callback functions
are not compatible AST2700.
Introduce a new "const MemoryRegionOps *" attribute in AspeedGPIOClass and
use it in aspeed_gpio_realize function.
Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
---
include/hw/gpio/aspeed_gpio.h | 1 +
hw/gpio/aspeed_gpio.c | 7 ++++++-
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/include/hw/gpio/aspeed_gpio.h b/include/hw/gpio/aspeed_gpio.h
index 8cd2ff54968c..e1e6c543339e 100644
--- a/include/hw/gpio/aspeed_gpio.h
+++ b/include/hw/gpio/aspeed_gpio.h
@@ -77,6 +77,7 @@ struct AspeedGPIOClass {
const AspeedGPIOReg *reg_table;
unsigned reg_table_count;
uint64_t mem_size;
+ const MemoryRegionOps *reg_ops;
};
struct AspeedGPIOState {
diff --git a/hw/gpio/aspeed_gpio.c b/hw/gpio/aspeed_gpio.c
index 564459ad4f07..8725606aecae 100644
--- a/hw/gpio/aspeed_gpio.c
+++ b/hw/gpio/aspeed_gpio.c
@@ -1046,7 +1046,7 @@ static void aspeed_gpio_realize(DeviceState *dev, Error **errp)
}
}
- memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_gpio_ops, s,
+ memory_region_init_io(&s->iomem, OBJECT(s), agc->reg_ops, s,
TYPE_ASPEED_GPIO, agc->mem_size);
sysbus_init_mmio(sbd, &s->iomem);
@@ -1131,6 +1131,7 @@ static void aspeed_gpio_ast2400_class_init(ObjectClass *klass, void *data)
agc->reg_table = aspeed_3_3v_gpios;
agc->reg_table_count = GPIO_3_3V_REG_ARRAY_SIZE;
agc->mem_size = 0x1000;
+ agc->reg_ops = &aspeed_gpio_ops;
}
static void aspeed_gpio_2500_class_init(ObjectClass *klass, void *data)
@@ -1143,6 +1144,7 @@ static void aspeed_gpio_2500_class_init(ObjectClass *klass, void *data)
agc->reg_table = aspeed_3_3v_gpios;
agc->reg_table_count = GPIO_3_3V_REG_ARRAY_SIZE;
agc->mem_size = 0x1000;
+ agc->reg_ops = &aspeed_gpio_ops;
}
static void aspeed_gpio_ast2600_3_3v_class_init(ObjectClass *klass, void *data)
@@ -1155,6 +1157,7 @@ static void aspeed_gpio_ast2600_3_3v_class_init(ObjectClass *klass, void *data)
agc->reg_table = aspeed_3_3v_gpios;
agc->reg_table_count = GPIO_3_3V_REG_ARRAY_SIZE;
agc->mem_size = 0x800;
+ agc->reg_ops = &aspeed_gpio_ops;
}
static void aspeed_gpio_ast2600_1_8v_class_init(ObjectClass *klass, void *data)
@@ -1167,6 +1170,7 @@ static void aspeed_gpio_ast2600_1_8v_class_init(ObjectClass *klass, void *data)
agc->reg_table = aspeed_1_8v_gpios;
agc->reg_table_count = GPIO_1_8V_REG_ARRAY_SIZE;
agc->mem_size = 0x800;
+ agc->reg_ops = &aspeed_gpio_ops;
}
static void aspeed_gpio_1030_class_init(ObjectClass *klass, void *data)
@@ -1179,6 +1183,7 @@ static void aspeed_gpio_1030_class_init(ObjectClass *klass, void *data)
agc->reg_table = aspeed_3_3v_gpios;
agc->reg_table_count = GPIO_3_3V_REG_ARRAY_SIZE;
agc->mem_size = 0x1000;
+ agc->reg_ops = &aspeed_gpio_ops;
}
static const TypeInfo aspeed_gpio_info = {
--
2.47.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PULL 04/17] hw/gpio/aspeed: Fix clear incorrect interrupt status for GPIO index mode
2024-10-24 6:34 [PULL 00/17] aspeed queue Cédric Le Goater
` (2 preceding siblings ...)
2024-10-24 6:34 ` [PULL 03/17] hw/gpio/aspeed: Support different memory region ops Cédric Le Goater
@ 2024-10-24 6:34 ` Cédric Le Goater
2024-10-24 6:34 ` [PULL 05/17] hw/gpio/aspeed: Add AST2700 support Cédric Le Goater
` (13 subsequent siblings)
17 siblings, 0 replies; 34+ messages in thread
From: Cédric Le Goater @ 2024-10-24 6:34 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Jamin Lin, Andrew Jeffery
From: Jamin Lin <jamin_lin@aspeedtech.com>
The interrupt status field is W1C, where a set bit on read indicates an
interrupt is pending. If the bit extracted from data is set it should
clear the corresponding bit in reg_value. However, if the extracted
bit is clear then the value of the corresponding bit in reg_value
should be unchanged.
SHARED_FIELD_EX32() extracts the interrupt status bit from the write
(data). reg_value is set to the set's interrupt status, which means
that for any pin with an interrupt pending, the corresponding bit is
set. The deposit32() call updates the bit at pin_idx in the
reg_value, using the value extracted from the write (data).
The result is that if multiple interrupt status bits
were pending and the write was acknowledging specific one bit,
then the all interrupt status bits will be cleared.
However, it is index mode and should only clear the corresponding bit.
For example, say we have an interrupt pending for GPIOA0, where the
following statements are true:
set->int_status == 0b01
s->pending == 1
Before it is acknowledged, an interrupt becomes pending for GPIOA1:
set->int_status == 0b11
s->pending == 2
A write is issued to acknowledge the interrupt for GPIOA0. This causes
the following sequence:
reg_value == 0b11
pending == 2
s->pending == 0
set->int_status == 0b00
It should only clear bit 0 in index mode and the correct result
should be as following.
set->int_status == 0b11
s->pending == 2
pending == 1
s->pending == 1
set->int_status == 0b10
Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
Suggested-by: Andrew Jeffery <andrew@codeconstruct.com.au>
Reviewed-by: Andrew Jeffery <andrew@codeconstruct.com.au>
---
hw/gpio/aspeed_gpio.c | 27 +++++++++++++++++----------
1 file changed, 17 insertions(+), 10 deletions(-)
diff --git a/hw/gpio/aspeed_gpio.c b/hw/gpio/aspeed_gpio.c
index 8725606aecae..16c18ea2f743 100644
--- a/hw/gpio/aspeed_gpio.c
+++ b/hw/gpio/aspeed_gpio.c
@@ -641,7 +641,7 @@ static void aspeed_gpio_write_index_mode(void *opaque, hwaddr offset,
uint32_t pin_idx = reg_idx_number % ASPEED_GPIOS_PER_SET;
uint32_t group_idx = pin_idx / GPIOS_PER_GROUP;
uint32_t reg_value = 0;
- uint32_t cleared;
+ uint32_t pending = 0;
set = &s->sets[set_idx];
props = &agc->props[set_idx];
@@ -703,16 +703,23 @@ static void aspeed_gpio_write_index_mode(void *opaque, hwaddr offset,
FIELD_EX32(data, GPIO_INDEX_REG, INT_SENS_2));
set->int_sens_2 = update_value_control_source(set, set->int_sens_2,
reg_value);
- /* set interrupt status */
- reg_value = set->int_status;
- reg_value = deposit32(reg_value, pin_idx, 1,
- FIELD_EX32(data, GPIO_INDEX_REG, INT_STATUS));
- cleared = ctpop32(reg_value & set->int_status);
- if (s->pending && cleared) {
- assert(s->pending >= cleared);
- s->pending -= cleared;
+ /* interrupt status */
+ if (FIELD_EX32(data, GPIO_INDEX_REG, INT_STATUS)) {
+ /* pending is either 1 or 0 for a 1-bit field */
+ pending = extract32(set->int_status, pin_idx, 1);
+
+ assert(s->pending >= pending);
+
+ /* No change to s->pending if pending is 0 */
+ s->pending -= pending;
+
+ /*
+ * The write acknowledged the interrupt regardless of whether it
+ * was pending or not. The post-condition is that it mustn't be
+ * pending. Unconditionally clear the status bit.
+ */
+ set->int_status = deposit32(set->int_status, pin_idx, 1, 0);
}
- set->int_status &= ~reg_value;
break;
case gpio_reg_idx_debounce:
reg_value = set->debounce_1;
--
2.47.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PULL 05/17] hw/gpio/aspeed: Add AST2700 support
2024-10-24 6:34 [PULL 00/17] aspeed queue Cédric Le Goater
` (3 preceding siblings ...)
2024-10-24 6:34 ` [PULL 04/17] hw/gpio/aspeed: Fix clear incorrect interrupt status for GPIO index mode Cédric Le Goater
@ 2024-10-24 6:34 ` Cédric Le Goater
2024-10-24 6:34 ` [PULL 06/17] aspeed/soc: Correct GPIO irq 130 for AST2700 Cédric Le Goater
` (12 subsequent siblings)
17 siblings, 0 replies; 34+ messages in thread
From: Cédric Le Goater @ 2024-10-24 6:34 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Jamin Lin, Andrew Jeffery
From: Jamin Lin <jamin_lin@aspeedtech.com>
AST2700 integrates two set of Parallel GPIO Controller with maximum 212
control pins, which are 27 groups. (H, exclude pin: H7 H6 H5 H4)
In the previous design of ASPEED SOCs, one register is used for setting
one function for one set which are 32 pins and 4 groups.
ex: GPIO000 is used for setting data value for GPIO A, B, C and D in AST2600.
ex: GPIO004 is used for setting direction for GPIO A, B, C and D in AST2600.
However, the register set have a significant change since AST2700.
Each GPIO pin has their own individual control register.
In other words, users are able to set one GPIO pin’s direction,
interrupt enable, input mask and so on in the same one register.
Currently, aspeed_gpio_read and aspeed_gpio_write callback functions
are not compatible AST2700.
Introduce new aspeed_gpio_2700_read and aspeed_gpio_2700_write callback
functions and aspeed_gpio_2700_ops memory region operation for AST2700.
Introduce a new ast2700 class to support AST2700.
Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
Reviewed-by: Andrew Jeffery <andrew@codeconstruct.com.au>
---
hw/gpio/aspeed_gpio.c | 380 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 380 insertions(+)
diff --git a/hw/gpio/aspeed_gpio.c b/hw/gpio/aspeed_gpio.c
index 16c18ea2f743..a5b3f454e800 100644
--- a/hw/gpio/aspeed_gpio.c
+++ b/hw/gpio/aspeed_gpio.c
@@ -227,6 +227,38 @@ REG32(GPIO_INDEX_REG, 0x2AC)
FIELD(GPIO_INDEX_REG, COMMAND_SRC_1, 21, 1)
FIELD(GPIO_INDEX_REG, INPUT_MASK, 20, 1)
+/* AST2700 GPIO Register Address Offsets */
+REG32(GPIO_2700_DEBOUNCE_TIME_1, 0x000)
+REG32(GPIO_2700_DEBOUNCE_TIME_2, 0x004)
+REG32(GPIO_2700_DEBOUNCE_TIME_3, 0x008)
+REG32(GPIO_2700_INT_STATUS_1, 0x100)
+REG32(GPIO_2700_INT_STATUS_2, 0x104)
+REG32(GPIO_2700_INT_STATUS_3, 0x108)
+REG32(GPIO_2700_INT_STATUS_4, 0x10C)
+REG32(GPIO_2700_INT_STATUS_5, 0x110)
+REG32(GPIO_2700_INT_STATUS_6, 0x114)
+REG32(GPIO_2700_INT_STATUS_7, 0x118)
+/* GPIOA0 - GPIOAA7 Control Register */
+REG32(GPIO_A0_CONTROL, 0x180)
+ SHARED_FIELD(GPIO_CONTROL_OUT_DATA, 0, 1)
+ SHARED_FIELD(GPIO_CONTROL_DIRECTION, 1, 1)
+ SHARED_FIELD(GPIO_CONTROL_INT_ENABLE, 2, 1)
+ SHARED_FIELD(GPIO_CONTROL_INT_SENS_0, 3, 1)
+ SHARED_FIELD(GPIO_CONTROL_INT_SENS_1, 4, 1)
+ SHARED_FIELD(GPIO_CONTROL_INT_SENS_2, 5, 1)
+ SHARED_FIELD(GPIO_CONTROL_RESET_TOLERANCE, 6, 1)
+ SHARED_FIELD(GPIO_CONTROL_DEBOUNCE_1, 7, 1)
+ SHARED_FIELD(GPIO_CONTROL_DEBOUNCE_2, 8, 1)
+ SHARED_FIELD(GPIO_CONTROL_INPUT_MASK, 9, 1)
+ SHARED_FIELD(GPIO_CONTROL_BLINK_COUNTER_1, 10, 1)
+ SHARED_FIELD(GPIO_CONTROL_BLINK_COUNTER_2, 11, 1)
+ SHARED_FIELD(GPIO_CONTROL_INT_STATUS, 12, 1)
+ SHARED_FIELD(GPIO_CONTROL_IN_DATA, 13, 1)
+ SHARED_FIELD(GPIO_CONTROL_RESERVED, 14, 18)
+REG32(GPIO_AA7_CONTROL, 0x4DC)
+#define GPIO_2700_MEM_SIZE 0x4E0
+#define GPIO_2700_REG_ARRAY_SIZE (GPIO_2700_MEM_SIZE >> 2)
+
static int aspeed_evaluate_irq(GPIOSets *regs, int gpio_prev_high, int gpio)
{
uint32_t falling_edge = 0, rising_edge = 0;
@@ -970,6 +1002,316 @@ static void aspeed_gpio_set_pin(Object *obj, Visitor *v, const char *name,
aspeed_gpio_set_pin_level(s, set_idx, pin, level);
}
+static uint64_t aspeed_gpio_2700_read_control_reg(AspeedGPIOState *s,
+ uint32_t pin)
+{
+ AspeedGPIOClass *agc = ASPEED_GPIO_GET_CLASS(s);
+ GPIOSets *set;
+ uint64_t value = 0;
+ uint32_t set_idx;
+ uint32_t pin_idx;
+
+ set_idx = pin / ASPEED_GPIOS_PER_SET;
+ pin_idx = pin % ASPEED_GPIOS_PER_SET;
+
+ if (set_idx >= agc->nr_gpio_sets) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: set index: %d, out of bounds\n",
+ __func__, set_idx);
+ return 0;
+ }
+
+ set = &s->sets[set_idx];
+ value = SHARED_FIELD_DP32(value, GPIO_CONTROL_OUT_DATA,
+ extract32(set->data_read, pin_idx, 1));
+ value = SHARED_FIELD_DP32(value, GPIO_CONTROL_DIRECTION,
+ extract32(set->direction, pin_idx, 1));
+ value = SHARED_FIELD_DP32(value, GPIO_CONTROL_INT_ENABLE,
+ extract32(set->int_enable, pin_idx, 1));
+ value = SHARED_FIELD_DP32(value, GPIO_CONTROL_INT_SENS_0,
+ extract32(set->int_sens_0, pin_idx, 1));
+ value = SHARED_FIELD_DP32(value, GPIO_CONTROL_INT_SENS_1,
+ extract32(set->int_sens_1, pin_idx, 1));
+ value = SHARED_FIELD_DP32(value, GPIO_CONTROL_INT_SENS_2,
+ extract32(set->int_sens_2, pin_idx, 1));
+ value = SHARED_FIELD_DP32(value, GPIO_CONTROL_RESET_TOLERANCE,
+ extract32(set->reset_tol, pin_idx, 1));
+ value = SHARED_FIELD_DP32(value, GPIO_CONTROL_DEBOUNCE_1,
+ extract32(set->debounce_1, pin_idx, 1));
+ value = SHARED_FIELD_DP32(value, GPIO_CONTROL_DEBOUNCE_2,
+ extract32(set->debounce_2, pin_idx, 1));
+ value = SHARED_FIELD_DP32(value, GPIO_CONTROL_INPUT_MASK,
+ extract32(set->input_mask, pin_idx, 1));
+ value = SHARED_FIELD_DP32(value, GPIO_CONTROL_INT_STATUS,
+ extract32(set->int_status, pin_idx, 1));
+ value = SHARED_FIELD_DP32(value, GPIO_CONTROL_IN_DATA,
+ extract32(set->data_value, pin_idx, 1));
+ return value;
+}
+
+static void aspeed_gpio_2700_write_control_reg(AspeedGPIOState *s,
+ uint32_t pin, uint64_t data)
+{
+ AspeedGPIOClass *agc = ASPEED_GPIO_GET_CLASS(s);
+ const GPIOSetProperties *props;
+ GPIOSets *set;
+ uint32_t set_idx;
+ uint32_t pin_idx;
+ uint32_t group_value = 0;
+ uint32_t pending = 0;
+
+ set_idx = pin / ASPEED_GPIOS_PER_SET;
+ pin_idx = pin % ASPEED_GPIOS_PER_SET;
+
+ if (set_idx >= agc->nr_gpio_sets) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: set index: %d, out of bounds\n",
+ __func__, set_idx);
+ return;
+ }
+
+ set = &s->sets[set_idx];
+ props = &agc->props[set_idx];
+
+ /* direction */
+ group_value = set->direction;
+ group_value = deposit32(group_value, pin_idx, 1,
+ SHARED_FIELD_EX32(data, GPIO_CONTROL_DIRECTION));
+ /*
+ * where data is the value attempted to be written to the pin:
+ * pin type | input mask | output mask | expected value
+ * ------------------------------------------------------------
+ * bidirectional | 1 | 1 | data
+ * input only | 1 | 0 | 0
+ * output only | 0 | 1 | 1
+ * no pin | 0 | 0 | 0
+ *
+ * which is captured by:
+ * data = ( data | ~input) & output;
+ */
+ group_value = (group_value | ~props->input) & props->output;
+ set->direction = update_value_control_source(set, set->direction,
+ group_value);
+
+ /* out data */
+ group_value = set->data_read;
+ group_value = deposit32(group_value, pin_idx, 1,
+ SHARED_FIELD_EX32(data, GPIO_CONTROL_OUT_DATA));
+ group_value &= props->output;
+ group_value = update_value_control_source(set, set->data_read,
+ group_value);
+ set->data_read = group_value;
+
+ /* interrupt enable */
+ group_value = set->int_enable;
+ group_value = deposit32(group_value, pin_idx, 1,
+ SHARED_FIELD_EX32(data, GPIO_CONTROL_INT_ENABLE));
+ set->int_enable = update_value_control_source(set, set->int_enable,
+ group_value);
+
+ /* interrupt sensitivity type 0 */
+ group_value = set->int_sens_0;
+ group_value = deposit32(group_value, pin_idx, 1,
+ SHARED_FIELD_EX32(data, GPIO_CONTROL_INT_SENS_0));
+ set->int_sens_0 = update_value_control_source(set, set->int_sens_0,
+ group_value);
+
+ /* interrupt sensitivity type 1 */
+ group_value = set->int_sens_1;
+ group_value = deposit32(group_value, pin_idx, 1,
+ SHARED_FIELD_EX32(data, GPIO_CONTROL_INT_SENS_1));
+ set->int_sens_1 = update_value_control_source(set, set->int_sens_1,
+ group_value);
+
+ /* interrupt sensitivity type 2 */
+ group_value = set->int_sens_2;
+ group_value = deposit32(group_value, pin_idx, 1,
+ SHARED_FIELD_EX32(data, GPIO_CONTROL_INT_SENS_2));
+ set->int_sens_2 = update_value_control_source(set, set->int_sens_2,
+ group_value);
+
+ /* reset tolerance enable */
+ group_value = set->reset_tol;
+ group_value = deposit32(group_value, pin_idx, 1,
+ SHARED_FIELD_EX32(data, GPIO_CONTROL_RESET_TOLERANCE));
+ set->reset_tol = update_value_control_source(set, set->reset_tol,
+ group_value);
+
+ /* debounce 1 */
+ group_value = set->debounce_1;
+ group_value = deposit32(group_value, pin_idx, 1,
+ SHARED_FIELD_EX32(data, GPIO_CONTROL_DEBOUNCE_1));
+ set->debounce_1 = update_value_control_source(set, set->debounce_1,
+ group_value);
+
+ /* debounce 2 */
+ group_value = set->debounce_2;
+ group_value = deposit32(group_value, pin_idx, 1,
+ SHARED_FIELD_EX32(data, GPIO_CONTROL_DEBOUNCE_2));
+ set->debounce_2 = update_value_control_source(set, set->debounce_2,
+ group_value);
+
+ /* input mask */
+ group_value = set->input_mask;
+ group_value = deposit32(group_value, pin_idx, 1,
+ SHARED_FIELD_EX32(data, GPIO_CONTROL_INPUT_MASK));
+ /*
+ * feeds into interrupt generation
+ * 0: read from data value reg will be updated
+ * 1: read from data value reg will not be updated
+ */
+ set->input_mask = group_value & props->input;
+
+ /* blink counter 1 */
+ /* blink counter 2 */
+ /* unimplement */
+
+ /* interrupt status */
+ if (SHARED_FIELD_EX32(data, GPIO_CONTROL_INT_STATUS)) {
+ /* pending is either 1 or 0 for a 1-bit field */
+ pending = extract32(set->int_status, pin_idx, 1);
+
+ assert(s->pending >= pending);
+
+ /* No change to s->pending if pending is 0 */
+ s->pending -= pending;
+
+ /*
+ * The write acknowledged the interrupt regardless of whether it
+ * was pending or not. The post-condition is that it mustn't be
+ * pending. Unconditionally clear the status bit.
+ */
+ set->int_status = deposit32(set->int_status, pin_idx, 1, 0);
+ }
+
+ aspeed_gpio_update(s, set, set->data_value, UINT32_MAX);
+ return;
+}
+
+static uint64_t aspeed_gpio_2700_read(void *opaque, hwaddr offset,
+ uint32_t size)
+{
+ AspeedGPIOState *s = ASPEED_GPIO(opaque);
+ AspeedGPIOClass *agc = ASPEED_GPIO_GET_CLASS(s);
+ GPIOSets *set;
+ uint64_t value;
+ uint64_t reg;
+ uint32_t pin;
+ uint32_t idx;
+
+ reg = offset >> 2;
+
+ if (reg >= agc->reg_table_count) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: offset 0x%" PRIx64 " out of bounds\n",
+ __func__, offset);
+ return 0;
+ }
+
+ switch (reg) {
+ case R_GPIO_2700_DEBOUNCE_TIME_1 ... R_GPIO_2700_DEBOUNCE_TIME_3:
+ idx = reg - R_GPIO_2700_DEBOUNCE_TIME_1;
+
+ if (idx >= ASPEED_GPIO_NR_DEBOUNCE_REGS) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: debounce index: %d, out of bounds\n",
+ __func__, idx);
+ return 0;
+ }
+
+ value = (uint64_t) s->debounce_regs[idx];
+ break;
+ case R_GPIO_2700_INT_STATUS_1 ... R_GPIO_2700_INT_STATUS_7:
+ idx = reg - R_GPIO_2700_INT_STATUS_1;
+
+ if (idx >= agc->nr_gpio_sets) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: interrupt status index: %d, out of bounds\n",
+ __func__, idx);
+ return 0;
+ }
+
+ set = &s->sets[idx];
+ value = (uint64_t) set->int_status;
+ break;
+ case R_GPIO_A0_CONTROL ... R_GPIO_AA7_CONTROL:
+ pin = reg - R_GPIO_A0_CONTROL;
+
+ if (pin >= agc->nr_gpio_pins) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid pin number: %d\n",
+ __func__, pin);
+ return 0;
+ }
+
+ value = aspeed_gpio_2700_read_control_reg(s, pin);
+ break;
+ default:
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: no getter for offset 0x%"
+ PRIx64"\n", __func__, offset);
+ return 0;
+ }
+
+ trace_aspeed_gpio_read(offset, value);
+ return value;
+}
+
+static void aspeed_gpio_2700_write(void *opaque, hwaddr offset,
+ uint64_t data, uint32_t size)
+{
+ AspeedGPIOState *s = ASPEED_GPIO(opaque);
+ AspeedGPIOClass *agc = ASPEED_GPIO_GET_CLASS(s);
+ uint64_t reg;
+ uint32_t pin;
+ uint32_t idx;
+
+ trace_aspeed_gpio_write(offset, data);
+
+ reg = offset >> 2;
+
+ if (reg >= agc->reg_table_count) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: offset 0x%" PRIx64 " out of bounds\n",
+ __func__, offset);
+ return;
+ }
+
+ switch (reg) {
+ case R_GPIO_2700_DEBOUNCE_TIME_1 ... R_GPIO_2700_DEBOUNCE_TIME_3:
+ idx = reg - R_GPIO_2700_DEBOUNCE_TIME_1;
+
+ if (idx >= ASPEED_GPIO_NR_DEBOUNCE_REGS) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: debounce index: %d out of bounds\n",
+ __func__, idx);
+ return;
+ }
+
+ s->debounce_regs[idx] = (uint32_t) data;
+ break;
+ case R_GPIO_A0_CONTROL ... R_GPIO_AA7_CONTROL:
+ pin = reg - R_GPIO_A0_CONTROL;
+
+ if (pin >= agc->nr_gpio_pins) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid pin number: %d\n",
+ __func__, pin);
+ return;
+ }
+
+ if (SHARED_FIELD_EX32(data, GPIO_CONTROL_RESERVED)) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid reserved data: 0x%"
+ PRIx64"\n", __func__, data);
+ return;
+ }
+
+ aspeed_gpio_2700_write_control_reg(s, pin, data);
+ break;
+ default:
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: no setter for offset 0x%"
+ PRIx64"\n", __func__, offset);
+ break;
+ }
+
+ return;
+}
+
/* Setup functions */
static const GPIOSetProperties ast2400_set_props[ASPEED_GPIO_MAX_NR_SETS] = {
[0] = {0xffffffff, 0xffffffff, {"A", "B", "C", "D"} },
@@ -1016,6 +1358,16 @@ static GPIOSetProperties ast1030_set_props[ASPEED_GPIO_MAX_NR_SETS] = {
[5] = {0x000000ff, 0x00000000, {"U"} },
};
+static GPIOSetProperties ast2700_set_props[ASPEED_GPIO_MAX_NR_SETS] = {
+ [0] = {0xffffffff, 0xffffffff, {"A", "B", "C", "D"} },
+ [1] = {0x0fffffff, 0x0fffffff, {"E", "F", "G", "H"} },
+ [2] = {0xffffffff, 0xffffffff, {"I", "J", "K", "L"} },
+ [3] = {0xffffffff, 0xffffffff, {"M", "N", "O", "P"} },
+ [4] = {0xffffffff, 0xffffffff, {"Q", "R", "S", "T"} },
+ [5] = {0xffffffff, 0xffffffff, {"U", "V", "W", "X"} },
+ [6] = {0x00ffffff, 0x00ffffff, {"Y", "Z", "AA"} },
+};
+
static const MemoryRegionOps aspeed_gpio_ops = {
.read = aspeed_gpio_read,
.write = aspeed_gpio_write,
@@ -1024,6 +1376,14 @@ static const MemoryRegionOps aspeed_gpio_ops = {
.valid.max_access_size = 4,
};
+static const MemoryRegionOps aspeed_gpio_2700_ops = {
+ .read = aspeed_gpio_2700_read,
+ .write = aspeed_gpio_2700_write,
+ .endianness = DEVICE_LITTLE_ENDIAN,
+ .valid.min_access_size = 4,
+ .valid.max_access_size = 4,
+};
+
static void aspeed_gpio_reset(DeviceState *dev)
{
AspeedGPIOState *s = ASPEED_GPIO(dev);
@@ -1193,6 +1553,18 @@ static void aspeed_gpio_1030_class_init(ObjectClass *klass, void *data)
agc->reg_ops = &aspeed_gpio_ops;
}
+static void aspeed_gpio_2700_class_init(ObjectClass *klass, void *data)
+{
+ AspeedGPIOClass *agc = ASPEED_GPIO_CLASS(klass);
+
+ agc->props = ast2700_set_props;
+ agc->nr_gpio_pins = 216;
+ agc->nr_gpio_sets = 7;
+ agc->reg_table_count = GPIO_2700_REG_ARRAY_SIZE;
+ agc->mem_size = 0x1000;
+ agc->reg_ops = &aspeed_gpio_2700_ops;
+}
+
static const TypeInfo aspeed_gpio_info = {
.name = TYPE_ASPEED_GPIO,
.parent = TYPE_SYS_BUS_DEVICE,
@@ -1237,6 +1609,13 @@ static const TypeInfo aspeed_gpio_ast1030_info = {
.instance_init = aspeed_gpio_init,
};
+static const TypeInfo aspeed_gpio_ast2700_info = {
+ .name = TYPE_ASPEED_GPIO "-ast2700",
+ .parent = TYPE_ASPEED_GPIO,
+ .class_init = aspeed_gpio_2700_class_init,
+ .instance_init = aspeed_gpio_init,
+};
+
static void aspeed_gpio_register_types(void)
{
type_register_static(&aspeed_gpio_info);
@@ -1245,6 +1624,7 @@ static void aspeed_gpio_register_types(void)
type_register_static(&aspeed_gpio_ast2600_3_3v_info);
type_register_static(&aspeed_gpio_ast2600_1_8v_info);
type_register_static(&aspeed_gpio_ast1030_info);
+ type_register_static(&aspeed_gpio_ast2700_info);
}
type_init(aspeed_gpio_register_types);
--
2.47.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PULL 06/17] aspeed/soc: Correct GPIO irq 130 for AST2700
2024-10-24 6:34 [PULL 00/17] aspeed queue Cédric Le Goater
` (4 preceding siblings ...)
2024-10-24 6:34 ` [PULL 05/17] hw/gpio/aspeed: Add AST2700 support Cédric Le Goater
@ 2024-10-24 6:34 ` Cédric Le Goater
2024-10-24 6:34 ` [PULL 07/17] aspeed/soc: Support GPIO " Cédric Le Goater
` (11 subsequent siblings)
17 siblings, 0 replies; 34+ messages in thread
From: Cédric Le Goater @ 2024-10-24 6:34 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Jamin Lin, Cédric Le Goater
From: Jamin Lin <jamin_lin@aspeedtech.com>
The register set of GPIO have a significant change since AST2700.
Each GPIO pin has their own individual control register and users are able to
set one GPIO pin’s direction, interrupt enable, input mask and so on in the
same one control register.
AST2700 does not have GPIO18_XXX registers for GPIO 1.8v, removes
ASPEED_DEV_GPIO_1_8V. It is enough to only have ASPEED_DEV_GPIO
device in AST2700.
The AST2700 GPIO controller interrupt is connected to GICINT130_INTC at
bit 18. Therefore, correct GPIO irq 130.
Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
---
hw/arm/aspeed_ast27x0.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/hw/arm/aspeed_ast27x0.c b/hw/arm/aspeed_ast27x0.c
index 761ee116579c..99135edc1ed2 100644
--- a/hw/arm/aspeed_ast27x0.c
+++ b/hw/arm/aspeed_ast27x0.c
@@ -87,8 +87,7 @@ static const int aspeed_soc_ast2700_irqmap[] = {
[ASPEED_DEV_ADC] = 130,
[ASPEED_DEV_XDMA] = 5,
[ASPEED_DEV_EMMC] = 15,
- [ASPEED_DEV_GPIO] = 11,
- [ASPEED_DEV_GPIO_1_8V] = 130,
+ [ASPEED_DEV_GPIO] = 130,
[ASPEED_DEV_RTC] = 13,
[ASPEED_DEV_TIMER1] = 16,
[ASPEED_DEV_TIMER2] = 17,
@@ -124,7 +123,7 @@ static const int aspeed_soc_ast2700_gic128_intcmap[] = {
static const int aspeed_soc_ast2700_gic130_intcmap[] = {
[ASPEED_DEV_I2C] = 0,
[ASPEED_DEV_ADC] = 16,
- [ASPEED_DEV_GPIO_1_8V] = 18,
+ [ASPEED_DEV_GPIO] = 18,
};
/* GICINT 131 */
--
2.47.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PULL 07/17] aspeed/soc: Support GPIO for AST2700
2024-10-24 6:34 [PULL 00/17] aspeed queue Cédric Le Goater
` (5 preceding siblings ...)
2024-10-24 6:34 ` [PULL 06/17] aspeed/soc: Correct GPIO irq 130 for AST2700 Cédric Le Goater
@ 2024-10-24 6:34 ` Cédric Le Goater
2024-10-24 6:34 ` [PULL 08/17] tests/qtest:ast2700-gpio-test: Add GPIO test case " Cédric Le Goater
` (10 subsequent siblings)
17 siblings, 0 replies; 34+ messages in thread
From: Cédric Le Goater @ 2024-10-24 6:34 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Jamin Lin, Cédric Le Goater
From: Jamin Lin <jamin_lin@aspeedtech.com>
Add GPIO model for AST2700 GPIO support. The GPIO controller registers base
address is start at 0x14C0_B000 and its address space is 0x1000.
Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
---
hw/arm/aspeed_ast27x0.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/hw/arm/aspeed_ast27x0.c b/hw/arm/aspeed_ast27x0.c
index 99135edc1ed2..dca660eb6be2 100644
--- a/hw/arm/aspeed_ast27x0.c
+++ b/hw/arm/aspeed_ast27x0.c
@@ -62,6 +62,7 @@ static const hwaddr aspeed_soc_ast2700_memmap[] = {
[ASPEED_GIC_REDIST] = 0x12280000,
[ASPEED_DEV_ADC] = 0x14C00000,
[ASPEED_DEV_I2C] = 0x14C0F000,
+ [ASPEED_DEV_GPIO] = 0x14C0B000,
};
#define AST2700_MAX_IRQ 288
@@ -372,6 +373,9 @@ static void aspeed_soc_ast2700_init(Object *obj)
snprintf(typename, sizeof(typename), "aspeed.i2c-%s", socname);
object_initialize_child(obj, "i2c", &s->i2c, typename);
+
+ snprintf(typename, sizeof(typename), "aspeed.gpio-%s", socname);
+ object_initialize_child(obj, "gpio", &s->gpio, typename);
}
/*
@@ -657,6 +661,15 @@ static void aspeed_soc_ast2700_realize(DeviceState *dev, Error **errp)
sysbus_connect_irq(SYS_BUS_DEVICE(&s->i2c.busses[i]), 0, irq);
}
+ /* GPIO */
+ if (!sysbus_realize(SYS_BUS_DEVICE(&s->gpio), errp)) {
+ return;
+ }
+ aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->gpio), 0,
+ sc->memmap[ASPEED_DEV_GPIO]);
+ sysbus_connect_irq(SYS_BUS_DEVICE(&s->gpio), 0,
+ aspeed_soc_get_irq(s, ASPEED_DEV_GPIO));
+
create_unimplemented_device("ast2700.dpmcu", 0x11000000, 0x40000);
create_unimplemented_device("ast2700.iomem0", 0x12000000, 0x01000000);
create_unimplemented_device("ast2700.iomem1", 0x14000000, 0x01000000);
--
2.47.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PULL 08/17] tests/qtest:ast2700-gpio-test: Add GPIO test case for AST2700
2024-10-24 6:34 [PULL 00/17] aspeed queue Cédric Le Goater
` (6 preceding siblings ...)
2024-10-24 6:34 ` [PULL 07/17] aspeed/soc: Support GPIO " Cédric Le Goater
@ 2024-10-24 6:34 ` Cédric Le Goater
2024-10-24 6:34 ` [PULL 09/17] hw/misc/aspeed_hace: Fix SG Accumulative hashing Cédric Le Goater
` (9 subsequent siblings)
17 siblings, 0 replies; 34+ messages in thread
From: Cédric Le Goater @ 2024-10-24 6:34 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Jamin Lin, Thomas Huth, Cédric Le Goater
From: Jamin Lin <jamin_lin@aspeedtech.com>
Add GPIO test cases to test output and input pins from A0 to D7 for AST2700.
Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
[ clg: - Updated MAINTAINERS ]
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
MAINTAINERS | 1 +
tests/qtest/ast2700-gpio-test.c | 95 +++++++++++++++++++++++++++++++++
tests/qtest/meson.build | 3 ++
3 files changed, 99 insertions(+)
create mode 100644 tests/qtest/ast2700-gpio-test.c
diff --git a/MAINTAINERS b/MAINTAINERS
index c3bfa132fd6e..bcaf36e525df 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1123,6 +1123,7 @@ F: hw/net/ftgmac100.c
F: include/hw/net/ftgmac100.h
F: docs/system/arm/aspeed.rst
F: tests/*/*aspeed*
+F: tests/*/*ast2700*
F: hw/arm/fby35.c
NRF51
diff --git a/tests/qtest/ast2700-gpio-test.c b/tests/qtest/ast2700-gpio-test.c
new file mode 100644
index 000000000000..92758455641f
--- /dev/null
+++ b/tests/qtest/ast2700-gpio-test.c
@@ -0,0 +1,95 @@
+/*
+ * QTest testcase for the ASPEED AST2700 GPIO Controller.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ * Copyright (C) 2024 ASPEED Technology Inc.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/bitops.h"
+#include "qemu/timer.h"
+#include "qapi/qmp/qdict.h"
+#include "libqtest-single.h"
+
+#define AST2700_GPIO_BASE 0x14C0B000
+#define GPIOA0_CONTROL 0x180
+
+static void test_output_pins(const char *machine, const uint32_t base)
+{
+ QTestState *s = qtest_init(machine);
+ uint32_t offset = 0;
+ uint32_t value = 0;
+ uint32_t pin = 0;
+
+ for (char c = 'A'; c <= 'D'; c++) {
+ for (int i = 0; i < 8; i++) {
+ offset = base + (pin * 4);
+
+ /* output direction and output hi */
+ qtest_writel(s, offset, 0x00000003);
+ value = qtest_readl(s, offset);
+ g_assert_cmphex(value, ==, 0x00000003);
+
+ /* output direction and output low */
+ qtest_writel(s, offset, 0x00000002);
+ value = qtest_readl(s, offset);
+ g_assert_cmphex(value, ==, 0x00000002);
+ pin++;
+ }
+ }
+
+ qtest_quit(s);
+}
+
+static void test_input_pins(const char *machine, const uint32_t base)
+{
+ QTestState *s = qtest_init(machine);
+ char name[16];
+ uint32_t offset = 0;
+ uint32_t value = 0;
+ uint32_t pin = 0;
+
+ for (char c = 'A'; c <= 'D'; c++) {
+ for (int i = 0; i < 8; i++) {
+ sprintf(name, "gpio%c%d", c, i);
+ offset = base + (pin * 4);
+ /* input direction */
+ qtest_writel(s, offset, 0);
+
+ /* set input */
+ qtest_qom_set_bool(s, "/machine/soc/gpio", name, true);
+ value = qtest_readl(s, offset);
+ g_assert_cmphex(value, ==, 0x00002000);
+
+ /* clear input */
+ qtest_qom_set_bool(s, "/machine/soc/gpio", name, false);
+ value = qtest_readl(s, offset);
+ g_assert_cmphex(value, ==, 0);
+ pin++;
+ }
+ }
+
+ qtest_quit(s);
+}
+
+static void test_2700_input_pins(void)
+{
+ test_input_pins("-machine ast2700-evb",
+ AST2700_GPIO_BASE + GPIOA0_CONTROL);
+}
+
+static void test_2700_output_pins(void)
+{
+ test_output_pins("-machine ast2700-evb",
+ AST2700_GPIO_BASE + GPIOA0_CONTROL);
+}
+
+int main(int argc, char **argv)
+{
+ g_test_init(&argc, &argv, NULL);
+
+ qtest_add_func("/ast2700/gpio/input_pins", test_2700_input_pins);
+ qtest_add_func("/ast2700/gpio/output_pins", test_2700_output_pins);
+
+ return g_test_run();
+}
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index e8be8b3942d7..f7a19032f7d8 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -210,6 +210,8 @@ qtests_aspeed = \
['aspeed_hace-test',
'aspeed_smc-test',
'aspeed_gpio-test']
+qtests_aspeed64 = \
+ ['ast2700-gpio-test']
qtests_stm32l4x5 = \
['stm32l4x5_exti-test',
@@ -248,6 +250,7 @@ qtests_aarch64 = \
(config_all_devices.has_key('CONFIG_RASPI') ? ['bcm2835-dma-test', 'bcm2835-i2c-test'] : []) + \
(config_all_accel.has_key('CONFIG_TCG') and \
config_all_devices.has_key('CONFIG_TPM_TIS_I2C') ? ['tpm-tis-i2c-test'] : []) + \
+ (config_all_devices.has_key('CONFIG_ASPEED_SOC') ? qtests_aspeed64 : []) + \
['arm-cpu-features',
'numa-test',
'boot-serial-test',
--
2.47.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PULL 09/17] hw/misc/aspeed_hace: Fix SG Accumulative hashing
2024-10-24 6:34 [PULL 00/17] aspeed queue Cédric Le Goater
` (7 preceding siblings ...)
2024-10-24 6:34 ` [PULL 08/17] tests/qtest:ast2700-gpio-test: Add GPIO test case " Cédric Le Goater
@ 2024-10-24 6:34 ` Cédric Le Goater
2024-10-24 6:35 ` [PULL 10/17] tests/functional: Convert most Aspeed machine tests Cédric Le Goater
` (8 subsequent siblings)
17 siblings, 0 replies; 34+ messages in thread
From: Cédric Le Goater @ 2024-10-24 6:34 UTC (permalink / raw)
To: qemu-arm, qemu-devel
Cc: Alejandro Zeise, Cédric Le Goater, Andrew Jeffery, Jamin Lin,
Joel Stanley
From: Alejandro Zeise <alejandro.zeise@seagate.com>
Make the Aspeed HACE module use the new qcrypto accumulative hashing functions
when in scatter-gather accumulative mode. A hash context will maintain a
"running-hash" as each scatter-gather chunk is received.
Previously each scatter-gather "chunk" was cached
so the hash could be computed once the final chunk was received.
However, the cache was a shallow copy, so once the guest overwrote the
memory provided to HACE the final hash would not be correct.
Possibly related to: https://gitlab.com/qemu-project/qemu/-/issues/1121
Buglink: https://github.com/openbmc/qemu/issues/36
Signed-off-by: Alejandro Zeise <alejandro.zeise@seagate.com>
[ clg: - Checkpatch fixes
- Reworked qcrypto_hash*() error reports in do_hash_operation() ]
Signed-off-by: Cédric Le Goater <clg@redhat.com>
Acked-by: Andrew Jeffery <andrew@codeconstruct.com.au>
Reviewed-by: Jamin Lin <jamin_lin@aspeedtech.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
---
include/hw/misc/aspeed_hace.h | 4 ++
hw/misc/aspeed_hace.c | 104 +++++++++++++++++++---------------
2 files changed, 63 insertions(+), 45 deletions(-)
diff --git a/include/hw/misc/aspeed_hace.h b/include/hw/misc/aspeed_hace.h
index ecb1b67de816..4af99191955a 100644
--- a/include/hw/misc/aspeed_hace.h
+++ b/include/hw/misc/aspeed_hace.h
@@ -1,6 +1,7 @@
/*
* ASPEED Hash and Crypto Engine
*
+ * Copyright (c) 2024 Seagate Technology LLC and/or its Affiliates
* Copyright (C) 2021 IBM Corp.
*
* SPDX-License-Identifier: GPL-2.0-or-later
@@ -10,6 +11,7 @@
#define ASPEED_HACE_H
#include "hw/sysbus.h"
+#include "crypto/hash.h"
#define TYPE_ASPEED_HACE "aspeed.hace"
#define TYPE_ASPEED_AST2400_HACE TYPE_ASPEED_HACE "-ast2400"
@@ -35,6 +37,8 @@ struct AspeedHACEState {
MemoryRegion *dram_mr;
AddressSpace dram_as;
+
+ QCryptoHash *hash_ctx;
};
diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
index b6f43f65b29a..bc1d66ad8064 100644
--- a/hw/misc/aspeed_hace.c
+++ b/hw/misc/aspeed_hace.c
@@ -1,6 +1,7 @@
/*
* ASPEED Hash and Crypto Engine
*
+ * Copyright (c) 2024 Seagate Technology LLC and/or its Affiliates
* Copyright (C) 2021 IBM Corp.
*
* Joel Stanley <joel@jms.id.au>
@@ -151,49 +152,28 @@ static int reconstruct_iov(AspeedHACEState *s, struct iovec *iov, int id,
return iov_count;
}
-/**
- * Generate iov for accumulative mode.
- *
- * @param s aspeed hace state object
- * @param iov iov of the current request
- * @param id index of the current iov
- * @param req_len length of the current request
- *
- * @return count of iov
- */
-static int gen_acc_mode_iov(AspeedHACEState *s, struct iovec *iov, int id,
- hwaddr *req_len)
-{
- uint32_t pad_offset;
- uint32_t total_msg_len;
- s->total_req_len += *req_len;
-
- if (has_padding(s, &iov[id], *req_len, &total_msg_len, &pad_offset)) {
- if (s->iov_count) {
- return reconstruct_iov(s, iov, id, &pad_offset);
- }
-
- *req_len -= s->total_req_len - total_msg_len;
- s->total_req_len = 0;
- iov[id].iov_len = *req_len;
- } else {
- s->iov_cache[s->iov_count].iov_base = iov->iov_base;
- s->iov_cache[s->iov_count].iov_len = *req_len;
- ++s->iov_count;
- }
-
- return id + 1;
-}
-
static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
bool acc_mode)
{
struct iovec iov[ASPEED_HACE_MAX_SG];
+ uint32_t total_msg_len;
+ uint32_t pad_offset;
g_autofree uint8_t *digest_buf = NULL;
size_t digest_len = 0;
- int niov = 0;
+ bool sg_acc_mode_final_request = false;
int i;
void *haddr;
+ Error *local_err = NULL;
+
+ if (acc_mode && s->hash_ctx == NULL) {
+ s->hash_ctx = qcrypto_hash_new(algo, &local_err);
+ if (s->hash_ctx == NULL) {
+ qemu_log_mask(LOG_GUEST_ERROR, "qcrypto hash failed : %s",
+ error_get_pretty(local_err));
+ error_free(local_err);
+ return;
+ }
+ }
if (sg_mode) {
uint32_t len = 0;
@@ -226,8 +206,16 @@ static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
}
iov[i].iov_base = haddr;
if (acc_mode) {
- niov = gen_acc_mode_iov(s, iov, i, &plen);
-
+ s->total_req_len += plen;
+
+ if (has_padding(s, &iov[i], plen, &total_msg_len,
+ &pad_offset)) {
+ /* Padding being present indicates the final request */
+ sg_acc_mode_final_request = true;
+ iov[i].iov_len = pad_offset;
+ } else {
+ iov[i].iov_len = plen;
+ }
} else {
iov[i].iov_len = plen;
}
@@ -252,21 +240,42 @@ static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
* required to check whether cache is empty. If no, we should
* combine cached iov and the current iov.
*/
- uint32_t total_msg_len;
- uint32_t pad_offset;
s->total_req_len += len;
if (has_padding(s, iov, len, &total_msg_len, &pad_offset)) {
- niov = reconstruct_iov(s, iov, 0, &pad_offset);
+ i = reconstruct_iov(s, iov, 0, &pad_offset);
}
}
}
- if (niov) {
- i = niov;
- }
+ if (acc_mode) {
+ if (qcrypto_hash_updatev(s->hash_ctx, iov, i, &local_err) < 0) {
+ qemu_log_mask(LOG_GUEST_ERROR, "qcrypto hash update failed : %s",
+ error_get_pretty(local_err));
+ error_free(local_err);
+ return;
+ }
+
+ if (sg_acc_mode_final_request) {
+ if (qcrypto_hash_finalize_bytes(s->hash_ctx, &digest_buf,
+ &digest_len, &local_err)) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "qcrypto hash finalize failed : %s",
+ error_get_pretty(local_err));
+ error_free(local_err);
+ local_err = NULL;
+ }
- if (qcrypto_hash_bytesv(algo, iov, i, &digest_buf, &digest_len, NULL) < 0) {
- qemu_log_mask(LOG_GUEST_ERROR, "%s: qcrypto failed\n", __func__);
+ qcrypto_hash_free(s->hash_ctx);
+
+ s->hash_ctx = NULL;
+ s->iov_count = 0;
+ s->total_req_len = 0;
+ }
+ } else if (qcrypto_hash_bytesv(algo, iov, i, &digest_buf,
+ &digest_len, &local_err) < 0) {
+ qemu_log_mask(LOG_GUEST_ERROR, "qcrypto hash bytesv failed : %s",
+ error_get_pretty(local_err));
+ error_free(local_err);
return;
}
@@ -397,6 +406,11 @@ static void aspeed_hace_reset(DeviceState *dev)
{
struct AspeedHACEState *s = ASPEED_HACE(dev);
+ if (s->hash_ctx != NULL) {
+ qcrypto_hash_free(s->hash_ctx);
+ s->hash_ctx = NULL;
+ }
+
memset(s->regs, 0, sizeof(s->regs));
s->iov_count = 0;
s->total_req_len = 0;
--
2.47.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PULL 10/17] tests/functional: Convert most Aspeed machine tests
2024-10-24 6:34 [PULL 00/17] aspeed queue Cédric Le Goater
` (8 preceding siblings ...)
2024-10-24 6:34 ` [PULL 09/17] hw/misc/aspeed_hace: Fix SG Accumulative hashing Cédric Le Goater
@ 2024-10-24 6:35 ` Cédric Le Goater
2024-11-05 16:14 ` Peter Maydell
2024-10-24 6:35 ` [PULL 11/17] aspeed/smc: Fix write incorrect data into flash in user mode Cédric Le Goater
` (7 subsequent siblings)
17 siblings, 1 reply; 34+ messages in thread
From: Cédric Le Goater @ 2024-10-24 6:35 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Cédric Le Goater, Thomas Huth
This is a simple conversion of the tests with some cleanups and
adjustments to match the new test framework. Replace the zephyr image
MD5 hashes with SHA256 hashes while at it.
The SDK tests depend on a ssh class from avocado.utils which is
difficult to replace. To be addressed separately.
Signed-off-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Tested-by: Thomas Huth <thuth@redhat.com>
---
tests/avocado/machine_aspeed.py | 292 ----------------------------
tests/functional/meson.build | 2 +
tests/functional/test_arm_aspeed.py | 282 +++++++++++++++++++++++++++
3 files changed, 284 insertions(+), 292 deletions(-)
create mode 100644 tests/functional/test_arm_aspeed.py
diff --git a/tests/avocado/machine_aspeed.py b/tests/avocado/machine_aspeed.py
index 4e144bde9131..241ef180affc 100644
--- a/tests/avocado/machine_aspeed.py
+++ b/tests/avocado/machine_aspeed.py
@@ -19,258 +19,6 @@
from avocado_qemu import has_cmd
from avocado.utils import archive
from avocado import skipUnless
-from avocado import skipUnless
-
-
-class AST1030Machine(QemuSystemTest):
- """Boots the zephyr os and checks that the console is operational"""
-
- timeout = 10
-
- def test_ast1030_zephyros_1_04(self):
- """
- :avocado: tags=arch:arm
- :avocado: tags=machine:ast1030-evb
- :avocado: tags=os:zephyr
- """
- tar_url = ('https://github.com/AspeedTech-BMC'
- '/zephyr/releases/download/v00.01.04/ast1030-evb-demo.zip')
- tar_hash = '4c6a8ce3a8ba76ef1a65dae419ae3409343c4b20'
- tar_path = self.fetch_asset(tar_url, asset_hash=tar_hash)
- archive.extract(tar_path, self.workdir)
- kernel_file = self.workdir + "/ast1030-evb-demo/zephyr.elf"
- self.vm.set_console()
- self.vm.add_args('-kernel', kernel_file,
- '-nographic')
- self.vm.launch()
- wait_for_console_pattern(self, "Booting Zephyr OS")
- exec_command_and_wait_for_pattern(self, "help",
- "Available commands")
-
- def test_ast1030_zephyros_1_07(self):
- """
- :avocado: tags=arch:arm
- :avocado: tags=machine:ast1030-evb
- :avocado: tags=os:zephyr
- """
- tar_url = ('https://github.com/AspeedTech-BMC'
- '/zephyr/releases/download/v00.01.07/ast1030-evb-demo.zip')
- tar_hash = '40ac87eabdcd3b3454ce5aad11fedc72a33ecda2'
- tar_path = self.fetch_asset(tar_url, asset_hash=tar_hash)
- archive.extract(tar_path, self.workdir)
- kernel_file = self.workdir + "/ast1030-evb-demo/zephyr.bin"
- self.vm.set_console()
- self.vm.add_args('-kernel', kernel_file,
- '-nographic')
- self.vm.launch()
- wait_for_console_pattern(self, "Booting Zephyr OS")
- for shell_cmd in [
- 'kernel stacks',
- 'otp info conf',
- 'otp info scu',
- 'hwinfo devid',
- 'crypto aes256_cbc_vault',
- 'random get',
- 'jtag JTAG1 sw_xfer high TMS',
- 'adc ADC0 resolution 12',
- 'adc ADC0 read 42',
- 'adc ADC1 read 69',
- 'i2c scan I2C_0',
- 'i3c attach I3C_0',
- 'hash test',
- 'kernel uptime',
- 'kernel reboot warm',
- 'kernel uptime',
- 'kernel reboot cold',
- 'kernel uptime',
- ]: exec_command_and_wait_for_pattern(self, shell_cmd, "uart:~$")
-
-class AST2x00Machine(QemuSystemTest):
-
- timeout = 180
-
- def wait_for_console_pattern(self, success_message, vm=None):
- wait_for_console_pattern(self, success_message,
- failure_message='Kernel panic - not syncing',
- vm=vm)
-
- def do_test_arm_aspeed(self, image):
- self.vm.set_console()
- self.vm.add_args('-drive', 'file=' + image + ',if=mtd,format=raw',
- '-net', 'nic')
- self.vm.launch()
-
- self.wait_for_console_pattern("U-Boot 2016.07")
- self.wait_for_console_pattern("## Loading kernel from FIT Image at 20080000")
- self.wait_for_console_pattern("Starting kernel ...")
- self.wait_for_console_pattern("Booting Linux on physical CPU 0x0")
- wait_for_console_pattern(self,
- "aspeed-smc 1e620000.spi: read control register: 203b0641")
- self.wait_for_console_pattern("ftgmac100 1e660000.ethernet eth0: irq ")
- self.wait_for_console_pattern("systemd[1]: Set hostname to")
-
- def test_arm_ast2400_palmetto_openbmc_v2_9_0(self):
- """
- :avocado: tags=arch:arm
- :avocado: tags=machine:palmetto-bmc
- """
-
- image_url = ('https://github.com/openbmc/openbmc/releases/download/2.9.0/'
- 'obmc-phosphor-image-palmetto.static.mtd')
- image_hash = ('3e13bbbc28e424865dc42f35ad672b10f2e82cdb11846bb28fa625b48beafd0d')
- image_path = self.fetch_asset(image_url, asset_hash=image_hash,
- algorithm='sha256')
-
- self.do_test_arm_aspeed(image_path)
-
- def test_arm_ast2500_romulus_openbmc_v2_9_0(self):
- """
- :avocado: tags=arch:arm
- :avocado: tags=machine:romulus-bmc
- """
-
- image_url = ('https://github.com/openbmc/openbmc/releases/download/2.9.0/'
- 'obmc-phosphor-image-romulus.static.mtd')
- image_hash = ('820341076803f1955bc31e647a512c79f9add4f5233d0697678bab4604c7bb25')
- image_path = self.fetch_asset(image_url, asset_hash=image_hash,
- algorithm='sha256')
-
- self.do_test_arm_aspeed(image_path)
-
- def do_test_arm_aspeed_buildroot_start(self, image, cpu_id, pattern='Aspeed EVB'):
- self.require_netdev('user')
-
- self.vm.set_console()
- self.vm.add_args('-drive', 'file=' + image + ',if=mtd,format=raw',
- '-net', 'nic', '-net', 'user')
- self.vm.launch()
-
- self.wait_for_console_pattern('U-Boot 2019.04')
- self.wait_for_console_pattern('## Loading kernel from FIT Image')
- self.wait_for_console_pattern('Starting kernel ...')
- self.wait_for_console_pattern('Booting Linux on physical CPU ' + cpu_id)
- self.wait_for_console_pattern('lease of 10.0.2.15')
- # the line before login:
- self.wait_for_console_pattern(pattern)
- time.sleep(0.1)
- exec_command(self, 'root')
- time.sleep(0.1)
- exec_command(self, "passw0rd")
-
- def do_test_arm_aspeed_buildroot_poweroff(self):
- exec_command_and_wait_for_pattern(self, 'poweroff',
- 'reboot: System halted');
-
- def test_arm_ast2500_evb_buildroot(self):
- """
- :avocado: tags=arch:arm
- :avocado: tags=machine:ast2500-evb
- """
-
- image_url = ('https://github.com/legoater/qemu-aspeed-boot/raw/master/'
- 'images/ast2500-evb/buildroot-2023.11/flash.img')
- image_hash = ('c23db6160cf77d0258397eb2051162c8473a56c441417c52a91ba217186e715f')
- image_path = self.fetch_asset(image_url, asset_hash=image_hash,
- algorithm='sha256')
-
- self.vm.add_args('-device',
- 'tmp105,bus=aspeed.i2c.bus.3,address=0x4d,id=tmp-test');
- self.do_test_arm_aspeed_buildroot_start(image_path, '0x0', 'Aspeed AST2500 EVB')
-
- exec_command_and_wait_for_pattern(self,
- 'echo lm75 0x4d > /sys/class/i2c-dev/i2c-3/device/new_device',
- 'i2c i2c-3: new_device: Instantiated device lm75 at 0x4d');
- exec_command_and_wait_for_pattern(self,
- 'cat /sys/class/hwmon/hwmon1/temp1_input', '0')
- self.vm.cmd('qom-set', path='/machine/peripheral/tmp-test',
- property='temperature', value=18000);
- exec_command_and_wait_for_pattern(self,
- 'cat /sys/class/hwmon/hwmon1/temp1_input', '18000')
-
- self.do_test_arm_aspeed_buildroot_poweroff()
-
- def test_arm_ast2600_evb_buildroot(self):
- """
- :avocado: tags=arch:arm
- :avocado: tags=machine:ast2600-evb
- """
-
- image_url = ('https://github.com/legoater/qemu-aspeed-boot/raw/master/'
- 'images/ast2600-evb/buildroot-2023.11/flash.img')
- image_hash = ('b62808daef48b438d0728ee07662290490ecfa65987bb91294cafb1bb7ad1a68')
- image_path = self.fetch_asset(image_url, asset_hash=image_hash,
- algorithm='sha256')
-
- self.vm.add_args('-device',
- 'tmp105,bus=aspeed.i2c.bus.3,address=0x4d,id=tmp-test');
- self.vm.add_args('-device',
- 'ds1338,bus=aspeed.i2c.bus.3,address=0x32');
- self.vm.add_args('-device',
- 'i2c-echo,bus=aspeed.i2c.bus.3,address=0x42');
- self.do_test_arm_aspeed_buildroot_start(image_path, '0xf00', 'Aspeed AST2600 EVB')
-
- exec_command_and_wait_for_pattern(self,
- 'echo lm75 0x4d > /sys/class/i2c-dev/i2c-3/device/new_device',
- 'i2c i2c-3: new_device: Instantiated device lm75 at 0x4d');
- exec_command_and_wait_for_pattern(self,
- 'cat /sys/class/hwmon/hwmon1/temp1_input', '0')
- self.vm.cmd('qom-set', path='/machine/peripheral/tmp-test',
- property='temperature', value=18000);
- exec_command_and_wait_for_pattern(self,
- 'cat /sys/class/hwmon/hwmon1/temp1_input', '18000')
-
- exec_command_and_wait_for_pattern(self,
- 'echo ds1307 0x32 > /sys/class/i2c-dev/i2c-3/device/new_device',
- 'i2c i2c-3: new_device: Instantiated device ds1307 at 0x32');
- year = time.strftime("%Y")
- exec_command_and_wait_for_pattern(self, 'hwclock -f /dev/rtc1', year);
-
- exec_command_and_wait_for_pattern(self,
- 'echo slave-24c02 0x1064 > /sys/bus/i2c/devices/i2c-3/new_device',
- 'i2c i2c-3: new_device: Instantiated device slave-24c02 at 0x64');
- exec_command(self, 'i2cset -y 3 0x42 0x64 0x00 0xaa i');
- time.sleep(0.1)
- exec_command_and_wait_for_pattern(self,
- 'hexdump /sys/bus/i2c/devices/3-1064/slave-eeprom',
- '0000000 ffaa ffff ffff ffff ffff ffff ffff ffff');
- self.do_test_arm_aspeed_buildroot_poweroff()
-
- @skipUnless(*has_cmd('swtpm'))
- def test_arm_ast2600_evb_buildroot_tpm(self):
- """
- :avocado: tags=arch:arm
- :avocado: tags=machine:ast2600-evb
- """
-
- image_url = ('https://github.com/legoater/qemu-aspeed-boot/raw/master/'
- 'images/ast2600-evb/buildroot-2023.02-tpm/flash.img')
- image_hash = ('a46009ae8a5403a0826d607215e731a8c68d27c14c41e55331706b8f9c7bd997')
- image_path = self.fetch_asset(image_url, asset_hash=image_hash,
- algorithm='sha256')
-
- # force creation of VM object, which also defines self._sd
- vm = self.vm
-
- socket = os.path.join(self._sd.name, 'swtpm-socket')
-
- subprocess.run(['swtpm', 'socket', '-d', '--tpm2',
- '--tpmstate', f'dir={self.vm.temp_dir}',
- '--ctrl', f'type=unixio,path={socket}'])
-
- self.vm.add_args('-chardev', f'socket,id=chrtpm,path={socket}')
- self.vm.add_args('-tpmdev', 'emulator,id=tpm0,chardev=chrtpm')
- self.vm.add_args('-device',
- 'tpm-tis-i2c,tpmdev=tpm0,bus=aspeed.i2c.bus.12,address=0x2e')
- self.do_test_arm_aspeed_buildroot_start(image_path, '0xf00', 'Aspeed AST2600 EVB')
-
- exec_command_and_wait_for_pattern(self,
- 'echo tpm_tis_i2c 0x2e > /sys/bus/i2c/devices/i2c-12/new_device',
- 'tpm_tis_i2c 12-002e: 2.0 TPM (device-id 0x1, rev-id 1)');
- exec_command_and_wait_for_pattern(self,
- 'cat /sys/class/tpm/tpm0/pcr-sha256/0',
- 'B804724EA13F52A9072BA87FE8FDCC497DFC9DF9AA15B9088694639C431688E0');
-
- self.do_test_arm_aspeed_buildroot_poweroff()
class AST2x00MachineSDK(QemuSystemTest, LinuxSSHMixIn):
@@ -452,43 +200,3 @@ def test_aarch64_ast2700_evb_sdk_v09_02(self):
property='temperature', value=18000)
self.ssh_command_output_contains(
'cat /sys/class/hwmon/hwmon20/temp1_input', '18000')
-
-class AST2x00MachineMMC(QemuSystemTest):
-
- timeout = 240
-
- def wait_for_console_pattern(self, success_message, vm=None):
- wait_for_console_pattern(self, success_message,
- failure_message='Kernel panic - not syncing',
- vm=vm)
-
- def test_arm_aspeed_emmc_boot(self):
- """
- :avocado: tags=arch:arm
- :avocado: tags=machine:rainier-bmc
- :avocado: tags=device:emmc
- """
-
- image_url = ('https://fileserver.linaro.org/s/B6pJTwWEkzSDi36/download/'
- 'mmc-p10bmc-20240617.qcow2')
- image_hash = ('d523fb478d2b84d5adc5658d08502bc64b1486955683814f89c6137518acd90b')
- image_path = self.fetch_asset(image_url, asset_hash=image_hash,
- algorithm='sha256')
-
- self.require_netdev('user')
-
- self.vm.set_console()
- self.vm.add_args('-drive',
- 'file=' + image_path + ',if=sd,id=sd2,index=2',
- '-net', 'nic', '-net', 'user')
- self.vm.launch()
-
- self.wait_for_console_pattern('U-Boot SPL 2019.04')
- self.wait_for_console_pattern('Trying to boot from MMC1')
- self.wait_for_console_pattern('U-Boot 2019.04')
- self.wait_for_console_pattern('eMMC 2nd Boot')
- self.wait_for_console_pattern('## Loading kernel from FIT Image')
- self.wait_for_console_pattern('Starting kernel ...')
- self.wait_for_console_pattern('Booting Linux on physical CPU 0xf00')
- self.wait_for_console_pattern('mmcblk0: p1 p2 p3 p4 p5 p6 p7')
- self.wait_for_console_pattern('IBM eBMC (OpenBMC for IBM Enterprise')
diff --git a/tests/functional/meson.build b/tests/functional/meson.build
index 5ccc1aa66ddc..97c1c597e861 100644
--- a/tests/functional/meson.build
+++ b/tests/functional/meson.build
@@ -15,6 +15,7 @@ test_timeouts = {
'aarch64_sbsaref' : 600,
'aarch64_virt' : 360,
'acpi_bits' : 240,
+ 'arm_aspeed' : 600,
'arm_raspi2' : 120,
'arm_tuxrun' : 120,
'mips_malta' : 120,
@@ -51,6 +52,7 @@ tests_alpha_system_thorough = [
]
tests_arm_system_thorough = [
+ 'arm_aspeed',
'arm_canona1100',
'arm_integratorcp',
'arm_raspi2',
diff --git a/tests/functional/test_arm_aspeed.py b/tests/functional/test_arm_aspeed.py
new file mode 100644
index 000000000000..9761fc06a454
--- /dev/null
+++ b/tests/functional/test_arm_aspeed.py
@@ -0,0 +1,282 @@
+#!/usr/bin/env python3
+#
+# Functional test that boots the ASPEED SoCs with firmware
+#
+# Copyright (C) 2022 ASPEED Technology Inc
+#
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+import os
+import time
+import subprocess
+import tempfile
+
+from qemu_test import LinuxKernelTest, Asset
+from qemu_test import exec_command_and_wait_for_pattern
+from qemu_test import interrupt_interactive_console_until_pattern
+from qemu_test import exec_command
+from qemu_test import has_cmd
+from qemu_test.utils import archive_extract
+from zipfile import ZipFile
+from unittest import skipUnless
+
+class AST1030Machine(LinuxKernelTest):
+
+ ASSET_ZEPHYR_1_04 = Asset(
+ ('https://github.com/AspeedTech-BMC'
+ '/zephyr/releases/download/v00.01.04/ast1030-evb-demo.zip'),
+ '4ac6210adcbc61294927918707c6762483fd844dde5e07f3ba834ad1f91434d3')
+
+ def test_ast1030_zephyros_1_04(self):
+ self.set_machine('ast1030-evb')
+
+ zip_file = self.ASSET_ZEPHYR_1_04.fetch()
+
+ kernel_name = "ast1030-evb-demo/zephyr.elf"
+ with ZipFile(zip_file, 'r') as zf:
+ zf.extract(kernel_name, path=self.workdir)
+ kernel_file = os.path.join(self.workdir, kernel_name)
+
+ self.vm.set_console()
+ self.vm.add_args('-kernel', kernel_file, '-nographic')
+ self.vm.launch()
+ self.wait_for_console_pattern("Booting Zephyr OS")
+ exec_command_and_wait_for_pattern(self, "help",
+ "Available commands")
+
+ ASSET_ZEPHYR_1_07 = Asset(
+ ('https://github.com/AspeedTech-BMC'
+ '/zephyr/releases/download/v00.01.07/ast1030-evb-demo.zip'),
+ 'ad52e27959746988afaed8429bf4e12ab988c05c4d07c9d90e13ec6f7be4574c')
+
+ def test_ast1030_zephyros_1_07(self):
+ self.set_machine('ast1030-evb')
+
+ zip_file = self.ASSET_ZEPHYR_1_07.fetch()
+
+ kernel_name = "ast1030-evb-demo/zephyr.bin"
+ with ZipFile(zip_file, 'r') as zf:
+ zf.extract(kernel_name, path=self.workdir)
+ kernel_file = os.path.join(self.workdir, kernel_name)
+
+ self.vm.set_console()
+ self.vm.add_args('-kernel', kernel_file, '-nographic')
+ self.vm.launch()
+ self.wait_for_console_pattern("Booting Zephyr OS")
+ for shell_cmd in [
+ 'kernel stacks',
+ 'otp info conf',
+ 'otp info scu',
+ 'hwinfo devid',
+ 'crypto aes256_cbc_vault',
+ 'random get',
+ 'jtag JTAG1 sw_xfer high TMS',
+ 'adc ADC0 resolution 12',
+ 'adc ADC0 read 42',
+ 'adc ADC1 read 69',
+ 'i2c scan I2C_0',
+ 'i3c attach I3C_0',
+ 'hash test',
+ 'kernel uptime',
+ 'kernel reboot warm',
+ 'kernel uptime',
+ 'kernel reboot cold',
+ 'kernel uptime',
+ ]: exec_command_and_wait_for_pattern(self, shell_cmd, "uart:~$")
+
+class AST2x00Machine(LinuxKernelTest):
+
+ def do_test_arm_aspeed(self, machine, image):
+ self.set_machine(machine)
+ self.vm.set_console()
+ self.vm.add_args('-drive', 'file=' + image + ',if=mtd,format=raw',
+ '-net', 'nic', '-snapshot')
+ self.vm.launch()
+
+ self.wait_for_console_pattern("U-Boot 2016.07")
+ self.wait_for_console_pattern("## Loading kernel from FIT Image at 20080000")
+ self.wait_for_console_pattern("Starting kernel ...")
+ self.wait_for_console_pattern("Booting Linux on physical CPU 0x0")
+ self.wait_for_console_pattern(
+ "aspeed-smc 1e620000.spi: read control register: 203b0641")
+ self.wait_for_console_pattern("ftgmac100 1e660000.ethernet eth0: irq ")
+ self.wait_for_console_pattern("systemd[1]: Set hostname to")
+
+ ASSET_PALMETTO_FLASH = Asset(
+ ('https://github.com/openbmc/openbmc/releases/download/2.9.0/'
+ 'obmc-phosphor-image-palmetto.static.mtd'),
+ '3e13bbbc28e424865dc42f35ad672b10f2e82cdb11846bb28fa625b48beafd0d');
+
+ def test_arm_ast2400_palmetto_openbmc_v2_9_0(self):
+ image_path = self.ASSET_PALMETTO_FLASH.fetch()
+
+ self.do_test_arm_aspeed('palmetto-bmc', image_path)
+
+ ASSET_ROMULUS_FLASH = Asset(
+ ('https://github.com/openbmc/openbmc/releases/download/2.9.0/'
+ 'obmc-phosphor-image-romulus.static.mtd'),
+ '820341076803f1955bc31e647a512c79f9add4f5233d0697678bab4604c7bb25')
+
+ def test_arm_ast2500_romulus_openbmc_v2_9_0(self):
+ image_path = self.ASSET_ROMULUS_FLASH.fetch()
+
+ self.do_test_arm_aspeed('romulus-bmc', image_path)
+
+ def do_test_arm_aspeed_buildroot_start(self, image, cpu_id, pattern='Aspeed EVB'):
+ self.require_netdev('user')
+ self.vm.set_console()
+ self.vm.add_args('-drive', 'file=' + image + ',if=mtd,format=raw',
+ '-net', 'nic', '-net', 'user')
+ self.vm.launch()
+
+ self.wait_for_console_pattern('U-Boot 2019.04')
+ self.wait_for_console_pattern('## Loading kernel from FIT Image')
+ self.wait_for_console_pattern('Starting kernel ...')
+ self.wait_for_console_pattern('Booting Linux on physical CPU ' + cpu_id)
+ self.wait_for_console_pattern('lease of 10.0.2.15')
+ # the line before login:
+ self.wait_for_console_pattern(pattern)
+ time.sleep(0.1)
+ exec_command(self, 'root')
+ time.sleep(0.1)
+ exec_command(self, "passw0rd")
+
+ def do_test_arm_aspeed_buildroot_poweroff(self):
+ exec_command_and_wait_for_pattern(self, 'poweroff',
+ 'reboot: System halted');
+
+ ASSET_BR2_202311_AST2500_FLASH = Asset(
+ ('https://github.com/legoater/qemu-aspeed-boot/raw/master/'
+ 'images/ast2500-evb/buildroot-2023.11/flash.img'),
+ 'c23db6160cf77d0258397eb2051162c8473a56c441417c52a91ba217186e715f')
+
+ def test_arm_ast2500_evb_buildroot(self):
+ self.set_machine('ast2500-evb')
+
+ image_path = self.ASSET_BR2_202311_AST2500_FLASH.fetch()
+
+ self.vm.add_args('-device',
+ 'tmp105,bus=aspeed.i2c.bus.3,address=0x4d,id=tmp-test');
+ self.do_test_arm_aspeed_buildroot_start(image_path, '0x0',
+ 'Aspeed AST2500 EVB')
+
+ exec_command_and_wait_for_pattern(self,
+ 'echo lm75 0x4d > /sys/class/i2c-dev/i2c-3/device/new_device',
+ 'i2c i2c-3: new_device: Instantiated device lm75 at 0x4d');
+ exec_command_and_wait_for_pattern(self,
+ 'cat /sys/class/hwmon/hwmon1/temp1_input', '0')
+ self.vm.cmd('qom-set', path='/machine/peripheral/tmp-test',
+ property='temperature', value=18000);
+ exec_command_and_wait_for_pattern(self,
+ 'cat /sys/class/hwmon/hwmon1/temp1_input', '18000')
+
+ self.do_test_arm_aspeed_buildroot_poweroff()
+
+ ASSET_BR2_202311_AST2600_FLASH = Asset(
+ ('https://github.com/legoater/qemu-aspeed-boot/raw/master/'
+ 'images/ast2600-evb/buildroot-2023.11/flash.img'),
+ 'b62808daef48b438d0728ee07662290490ecfa65987bb91294cafb1bb7ad1a68')
+
+ def test_arm_ast2600_evb_buildroot(self):
+ self.set_machine('ast2600-evb')
+
+ image_path = self.ASSET_BR2_202311_AST2600_FLASH.fetch()
+
+ self.vm.add_args('-device',
+ 'tmp105,bus=aspeed.i2c.bus.3,address=0x4d,id=tmp-test');
+ self.vm.add_args('-device',
+ 'ds1338,bus=aspeed.i2c.bus.3,address=0x32');
+ self.vm.add_args('-device',
+ 'i2c-echo,bus=aspeed.i2c.bus.3,address=0x42');
+ self.do_test_arm_aspeed_buildroot_start(image_path, '0xf00', 'Aspeed AST2600 EVB')
+
+ exec_command_and_wait_for_pattern(self,
+ 'echo lm75 0x4d > /sys/class/i2c-dev/i2c-3/device/new_device',
+ 'i2c i2c-3: new_device: Instantiated device lm75 at 0x4d');
+ exec_command_and_wait_for_pattern(self,
+ 'cat /sys/class/hwmon/hwmon1/temp1_input', '0')
+ self.vm.cmd('qom-set', path='/machine/peripheral/tmp-test',
+ property='temperature', value=18000);
+ exec_command_and_wait_for_pattern(self,
+ 'cat /sys/class/hwmon/hwmon1/temp1_input', '18000')
+
+ exec_command_and_wait_for_pattern(self,
+ 'echo ds1307 0x32 > /sys/class/i2c-dev/i2c-3/device/new_device',
+ 'i2c i2c-3: new_device: Instantiated device ds1307 at 0x32');
+ year = time.strftime("%Y")
+ exec_command_and_wait_for_pattern(self, 'hwclock -f /dev/rtc1', year);
+
+ exec_command_and_wait_for_pattern(self,
+ 'echo slave-24c02 0x1064 > /sys/bus/i2c/devices/i2c-3/new_device',
+ 'i2c i2c-3: new_device: Instantiated device slave-24c02 at 0x64');
+ exec_command(self, 'i2cset -y 3 0x42 0x64 0x00 0xaa i');
+ time.sleep(0.1)
+ exec_command_and_wait_for_pattern(self,
+ 'hexdump /sys/bus/i2c/devices/3-1064/slave-eeprom',
+ '0000000 ffaa ffff ffff ffff ffff ffff ffff ffff');
+ self.do_test_arm_aspeed_buildroot_poweroff()
+
+ ASSET_BR2_202302_AST2600_TPM_FLASH = Asset(
+ ('https://github.com/legoater/qemu-aspeed-boot/raw/master/'
+ 'images/ast2600-evb/buildroot-2023.02-tpm/flash.img'),
+ 'a46009ae8a5403a0826d607215e731a8c68d27c14c41e55331706b8f9c7bd997')
+
+ @skipUnless(*has_cmd('swtpm'))
+ def test_arm_ast2600_evb_buildroot_tpm(self):
+ self.set_machine('ast2600-evb')
+
+ image_path = self.ASSET_BR2_202302_AST2600_TPM_FLASH.fetch()
+
+ socket_dir = tempfile.TemporaryDirectory(prefix="qemu_")
+ socket = os.path.join(socket_dir.name, 'swtpm-socket')
+
+ subprocess.run(['swtpm', 'socket', '-d', '--tpm2',
+ '--tpmstate', f'dir={self.vm.temp_dir}',
+ '--ctrl', f'type=unixio,path={socket}'])
+
+ self.vm.add_args('-chardev', f'socket,id=chrtpm,path={socket}')
+ self.vm.add_args('-tpmdev', 'emulator,id=tpm0,chardev=chrtpm')
+ self.vm.add_args('-device',
+ 'tpm-tis-i2c,tpmdev=tpm0,bus=aspeed.i2c.bus.12,address=0x2e')
+ self.do_test_arm_aspeed_buildroot_start(image_path, '0xf00', 'Aspeed AST2600 EVB')
+
+ exec_command_and_wait_for_pattern(self,
+ 'echo tpm_tis_i2c 0x2e > /sys/bus/i2c/devices/i2c-12/new_device',
+ 'tpm_tis_i2c 12-002e: 2.0 TPM (device-id 0x1, rev-id 1)');
+ exec_command_and_wait_for_pattern(self,
+ 'cat /sys/class/tpm/tpm0/pcr-sha256/0',
+ 'B804724EA13F52A9072BA87FE8FDCC497DFC9DF9AA15B9088694639C431688E0');
+
+ self.do_test_arm_aspeed_buildroot_poweroff()
+
+class AST2x00MachineMMC(LinuxKernelTest):
+
+ ASSET_RAINIER_EMMC = Asset(
+ ('https://fileserver.linaro.org/s/B6pJTwWEkzSDi36/download/'
+ 'mmc-p10bmc-20240617.qcow2'),
+ 'd523fb478d2b84d5adc5658d08502bc64b1486955683814f89c6137518acd90b')
+
+ def test_arm_aspeed_emmc_boot(self):
+ self.set_machine('rainier-bmc')
+ self.require_netdev('user')
+
+ image_path = self.ASSET_RAINIER_EMMC.fetch()
+
+ self.vm.set_console()
+ self.vm.add_args('-drive',
+ 'file=' + image_path + ',if=sd,id=sd2,index=2',
+ '-net', 'nic', '-net', 'user', '-snapshot')
+ self.vm.launch()
+
+ self.wait_for_console_pattern('U-Boot SPL 2019.04')
+ self.wait_for_console_pattern('Trying to boot from MMC1')
+ self.wait_for_console_pattern('U-Boot 2019.04')
+ self.wait_for_console_pattern('eMMC 2nd Boot')
+ self.wait_for_console_pattern('## Loading kernel from FIT Image')
+ self.wait_for_console_pattern('Starting kernel ...')
+ self.wait_for_console_pattern('Booting Linux on physical CPU 0xf00')
+ self.wait_for_console_pattern('mmcblk0: p1 p2 p3 p4 p5 p6 p7')
+ self.wait_for_console_pattern('IBM eBMC (OpenBMC for IBM Enterprise')
+
+if __name__ == '__main__':
+ LinuxKernelTest.main()
--
2.47.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PULL 10/17] tests/functional: Convert most Aspeed machine tests
2024-10-24 6:35 ` [PULL 10/17] tests/functional: Convert most Aspeed machine tests Cédric Le Goater
@ 2024-11-05 16:14 ` Peter Maydell
2024-11-05 16:35 ` Stefan Berger
0 siblings, 1 reply; 34+ messages in thread
From: Peter Maydell @ 2024-11-05 16:14 UTC (permalink / raw)
To: Cédric Le Goater
Cc: qemu-arm, qemu-devel, Thomas Huth, Stefan Berger,
Daniel P. Berrange
On Thu, 24 Oct 2024 at 07:39, Cédric Le Goater <clg@redhat.com> wrote:
>
> This is a simple conversion of the tests with some cleanups and
> adjustments to match the new test framework. Replace the zephyr image
> MD5 hashes with SHA256 hashes while at it.
(ccing Stefan Berger for possible insight into swtpm)
Hi; I find that this swtpm-using test fails for me on my
local system due to an apparmor/swtpm problem...
> + @skipUnless(*has_cmd('swtpm'))
> + def test_arm_ast2600_evb_buildroot_tpm(self):
> + self.set_machine('ast2600-evb')
> +
> + image_path = self.ASSET_BR2_202302_AST2600_TPM_FLASH.fetch()
> +
> + socket_dir = tempfile.TemporaryDirectory(prefix="qemu_")
> + socket = os.path.join(socket_dir.name, 'swtpm-socket')
> +
> + subprocess.run(['swtpm', 'socket', '-d', '--tpm2',
> + '--tpmstate', f'dir={self.vm.temp_dir}',
> + '--ctrl', f'type=unixio,path={socket}'])
> +
> + self.vm.add_args('-chardev', f'socket,id=chrtpm,path={socket}')
> + self.vm.add_args('-tpmdev', 'emulator,id=tpm0,chardev=chrtpm')
> + self.vm.add_args('-device',
> + 'tpm-tis-i2c,tpmdev=tpm0,bus=aspeed.i2c.bus.12,address=0x2e')
> + self.do_test_arm_aspeed_buildroot_start(image_path, '0xf00', 'Aspeed AST2600 EVB')
> +
> + exec_command_and_wait_for_pattern(self,
> + 'echo tpm_tis_i2c 0x2e > /sys/bus/i2c/devices/i2c-12/new_device',
> + 'tpm_tis_i2c 12-002e: 2.0 TPM (device-id 0x1, rev-id 1)');
> + exec_command_and_wait_for_pattern(self,
> + 'cat /sys/class/tpm/tpm0/pcr-sha256/0',
> + 'B804724EA13F52A9072BA87FE8FDCC497DFC9DF9AA15B9088694639C431688E0');
> +
> + self.do_test_arm_aspeed_buildroot_poweroff()
The test fails like this:
qemu-system-arm: tpm-emulator: TPM result for CMD_INIT: 0x9 operation failed
Adding extra logging to swtpm (--log file=/tmp/swtpm.log,level=20)
reveals:
SWTPM_NVRAM_Lock_Lockfile: Could not open lockfile: Permission denied
Error: Could not initialize libtpms.
Error: Could not initialize the TPM
Checking the system logs, this is because apparmor has denied it:
Nov 5 16:01:14 e104462 kernel: [946406.489088] audit: type=1400
audit(1730822474.384:446): apparmor="DENIED" operation="mknod"
profile="swtpm"
name="/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/tests/functional/arm/test_arm_aspeed.AST2x00Machine.test_arm_ast2600_evb_buildroot_tpm/qemu-machine-hhuvwytc/.lock"
pid=2820156 comm="swtpm" requested_mask="c" denied_mask="c" fsuid=1000
ouid=1000
Q1: why is apparmor forbidding swtpm from doing something that
it needs to do to work?
Q2: is there a way to run swtpm such that it is not
confined by apparmor, for purposes of running it in a test case?
Q3: if not, is there a way to at least detect that swtpm is
broken on this system so we can skip the test case?
(I note that there is a thing in the apparmor config
"owner @{HOME}/** rwk" which I think means you only run into
this if you happen to be building/testing QEMU somewhere other
than your own home directory -- but that's hardly an
unreasonable configuration...)
thanks
-- PMM
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PULL 10/17] tests/functional: Convert most Aspeed machine tests
2024-11-05 16:14 ` Peter Maydell
@ 2024-11-05 16:35 ` Stefan Berger
2024-11-05 17:13 ` Peter Maydell
0 siblings, 1 reply; 34+ messages in thread
From: Stefan Berger @ 2024-11-05 16:35 UTC (permalink / raw)
To: Peter Maydell, Cédric Le Goater
Cc: qemu-arm, qemu-devel, Thomas Huth, Daniel P. Berrange
On 11/5/24 11:14 AM, Peter Maydell wrote:
> On Thu, 24 Oct 2024 at 07:39, Cédric Le Goater <clg@redhat.com> wrote:
>>
>> This is a simple conversion of the tests with some cleanups and
>> adjustments to match the new test framework. Replace the zephyr image
>> MD5 hashes with SHA256 hashes while at it.
>
> (ccing Stefan Berger for possible insight into swtpm)
>
> Hi; I find that this swtpm-using test fails for me on my
> local system due to an apparmor/swtpm problem...
>
>> + @skipUnless(*has_cmd('swtpm'))
>> + def test_arm_ast2600_evb_buildroot_tpm(self):
>> + self.set_machine('ast2600-evb')
>> +
>> + image_path = self.ASSET_BR2_202302_AST2600_TPM_FLASH.fetch()
>> +
>> + socket_dir = tempfile.TemporaryDirectory(prefix="qemu_")
>> + socket = os.path.join(socket_dir.name, 'swtpm-socket')
>> +
>> + subprocess.run(['swtpm', 'socket', '-d', '--tpm2',
>> + '--tpmstate', f'dir={self.vm.temp_dir}',
>> + '--ctrl', f'type=unixio,path={socket}'])
>> +
>> + self.vm.add_args('-chardev', f'socket,id=chrtpm,path={socket}')
>> + self.vm.add_args('-tpmdev', 'emulator,id=tpm0,chardev=chrtpm')
>> + self.vm.add_args('-device',
>> + 'tpm-tis-i2c,tpmdev=tpm0,bus=aspeed.i2c.bus.12,address=0x2e')
>> + self.do_test_arm_aspeed_buildroot_start(image_path, '0xf00', 'Aspeed AST2600 EVB')
>> +
>> + exec_command_and_wait_for_pattern(self,
>> + 'echo tpm_tis_i2c 0x2e > /sys/bus/i2c/devices/i2c-12/new_device',
>> + 'tpm_tis_i2c 12-002e: 2.0 TPM (device-id 0x1, rev-id 1)');
>> + exec_command_and_wait_for_pattern(self,
>> + 'cat /sys/class/tpm/tpm0/pcr-sha256/0',
>> + 'B804724EA13F52A9072BA87FE8FDCC497DFC9DF9AA15B9088694639C431688E0');
>> +
>> + self.do_test_arm_aspeed_buildroot_poweroff()
>
> The test fails like this:
>
> qemu-system-arm: tpm-emulator: TPM result for CMD_INIT: 0x9 operation failed
>
> Adding extra logging to swtpm (--log file=/tmp/swtpm.log,level=20)
> reveals:
>
> SWTPM_NVRAM_Lock_Lockfile: Could not open lockfile: Permission denied
> Error: Could not initialize libtpms.
> Error: Could not initialize the TPM
>
> Checking the system logs, this is because apparmor has denied it:
>
> Nov 5 16:01:14 e104462 kernel: [946406.489088] audit: type=1400
> audit(1730822474.384:446): apparmor="DENIED" operation="mknod"
> profile="swtpm"
> name="/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/tests/functional/arm/test_arm_aspeed.AST2x00Machine.test_arm_ast2600_evb_buildroot_tpm/qemu-machine-hhuvwytc/.lock"
> pid=2820156 comm="swtpm" requested_mask="c" denied_mask="c" fsuid=1000
> ouid=1000
>
>
>
> Q1: why is apparmor forbidding swtpm from doing something that
> it needs to do to work?
What distro and version is this?
The profile may be too strict and not reflecting all the paths needed
for running the test cases. Ubuntu for example would have to update
their profile in such a case.
>
> Q2: is there a way to run swtpm such that it is not
> confined by apparmor, for purposes of running it in a test case?
Try either one:
- sudo aa-complain /usr/bin/swtpm
- sudo aa-disable /usr/bin/swtpm
>
> Q3: if not, is there a way to at least detect that swtpm is
> broken on this system so we can skip the test case?
It's not swtpm that is broken but the AppArmor profile is too strict.
Above command lines should work.
>
> (I note that there is a thing in the apparmor config
> "owner @{HOME}/** rwk" which I think means you only run into
> this if you happen to be building/testing QEMU somewhere other
> than your own home directory -- but that's hardly an
> unreasonable configuration...)
>
> thanks
> -- PMM
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PULL 10/17] tests/functional: Convert most Aspeed machine tests
2024-11-05 16:35 ` Stefan Berger
@ 2024-11-05 17:13 ` Peter Maydell
2024-11-05 18:02 ` Stefan Berger
0 siblings, 1 reply; 34+ messages in thread
From: Peter Maydell @ 2024-11-05 17:13 UTC (permalink / raw)
To: Stefan Berger
Cc: Cédric Le Goater, qemu-arm, qemu-devel, Thomas Huth,
Daniel P. Berrange
On Tue, 5 Nov 2024 at 17:02, Stefan Berger <stefanb@linux.ibm.com> wrote:
> On 11/5/24 11:14 AM, Peter Maydell wrote:
> > Q1: why is apparmor forbidding swtpm from doing something that
> > it needs to do to work?
>
> What distro and version is this?
>
> The profile may be too strict and not reflecting all the paths needed
> for running the test cases. Ubuntu for example would have to update
> their profile in such a case.
This is Ubuntu 22.04 "jammy" (with swtpm 0.6.3-0ubuntu3.3).
> > Q2: is there a way to run swtpm such that it is not
> > confined by apparmor, for purposes of running it in a test case?
>
> Try either one:
> - sudo aa-complain /usr/bin/swtpm
> - sudo aa-disable /usr/bin/swtpm
We don't have root access from QEMU's 'make check',
though (and shouldn't be globally disabling apparmor
even if we could). I had in mind more a way that an
individual user can say "run this swtpm process but don't
apply the apparmor profile to it".
> > Q3: if not, is there a way to at least detect that swtpm is
> > broken on this system so we can skip the test case?
>
> It's not swtpm that is broken but the AppArmor profile is too strict.
> Above command lines should work.
But this is a widely deployed distro in its default
configuration. We have to either work with it or detect
that it's broken so we can skip the test.
thanks
-- PMM
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PULL 10/17] tests/functional: Convert most Aspeed machine tests
2024-11-05 17:13 ` Peter Maydell
@ 2024-11-05 18:02 ` Stefan Berger
2024-11-05 18:12 ` Peter Maydell
0 siblings, 1 reply; 34+ messages in thread
From: Stefan Berger @ 2024-11-05 18:02 UTC (permalink / raw)
To: Peter Maydell
Cc: Cédric Le Goater, qemu-arm, qemu-devel, Thomas Huth,
Daniel P. Berrange, lena.voytek
On 11/5/24 12:13 PM, Peter Maydell wrote:
> On Tue, 5 Nov 2024 at 17:02, Stefan Berger <stefanb@linux.ibm.com> wrote:
>> On 11/5/24 11:14 AM, Peter Maydell wrote:
>>> Q1: why is apparmor forbidding swtpm from doing something that
>>> it needs to do to work?
>>
>> What distro and version is this?
>>
>> The profile may be too strict and not reflecting all the paths needed
>> for running the test cases. Ubuntu for example would have to update
>> their profile in such a case.
>
> This is Ubuntu 22.04 "jammy" (with swtpm 0.6.3-0ubuntu3.3).
>
>>> Q2: is there a way to run swtpm such that it is not
>>> confined by apparmor, for purposes of running it in a test case?
>>
>> Try either one:
>> - sudo aa-complain /usr/bin/swtpm
>> - sudo aa-disable /usr/bin/swtpm
>
> We don't have root access from QEMU's 'make check',
> though (and shouldn't be globally disabling apparmor
> even if we could). I had in mind more a way that an
> individual user can say "run this swtpm process but don't
> apply the apparmor profile to it".
So the problem is that the avocado tests are using /var/tmp but we only
have AppArmor rules for /tmp/
The following solutions should work:
- do not install swtpm at all
- sudo cp /usr/bin/swtpm /usr/local/bin/swtpm
- as root: echo "include <abstractions/user-tmp>" >>
/etc/apparmor.d/local/usr.bin.swtpm && apparmor_parser -r
/etc/apparmor.d/usr.bin.swtpm
Lena, it looks like we would need the following additional line in the
profile:
include <abstractions/user-tmp>
>
>>> Q3: if not, is there a way to at least detect that swtpm is
>>> broken on this system so we can skip the test case?
>>
>> It's not swtpm that is broken but the AppArmor profile is too strict.
>> Above command lines should work.
>
> But this is a widely deployed distro in its default
> configuration. We have to either work with it or detect
> that it's broken so we can skip the test.
Cc'in Lena from Ubuntu.
>
> thanks
> -- PMM
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PULL 10/17] tests/functional: Convert most Aspeed machine tests
2024-11-05 18:02 ` Stefan Berger
@ 2024-11-05 18:12 ` Peter Maydell
2024-11-05 18:35 ` Stefan Berger
0 siblings, 1 reply; 34+ messages in thread
From: Peter Maydell @ 2024-11-05 18:12 UTC (permalink / raw)
To: Stefan Berger
Cc: Cédric Le Goater, qemu-arm, qemu-devel, Thomas Huth,
Daniel P. Berrange, lena.voytek
On Tue, 5 Nov 2024 at 18:02, Stefan Berger <stefanb@linux.ibm.com> wrote:
>
>
>
> On 11/5/24 12:13 PM, Peter Maydell wrote:
> > On Tue, 5 Nov 2024 at 17:02, Stefan Berger <stefanb@linux.ibm.com> wrote:
> >> On 11/5/24 11:14 AM, Peter Maydell wrote:
> >>> Q1: why is apparmor forbidding swtpm from doing something that
> >>> it needs to do to work?
> >>
> >> What distro and version is this?
> >>
> >> The profile may be too strict and not reflecting all the paths needed
> >> for running the test cases. Ubuntu for example would have to update
> >> their profile in such a case.
> >
> > This is Ubuntu 22.04 "jammy" (with swtpm 0.6.3-0ubuntu3.3).
> >
> >>> Q2: is there a way to run swtpm such that it is not
> >>> confined by apparmor, for purposes of running it in a test case?
> >>
> >> Try either one:
> >> - sudo aa-complain /usr/bin/swtpm
> >> - sudo aa-disable /usr/bin/swtpm
> >
> > We don't have root access from QEMU's 'make check',
> > though (and shouldn't be globally disabling apparmor
> > even if we could). I had in mind more a way that an
> > individual user can say "run this swtpm process but don't
> > apply the apparmor profile to it".
>
> So the problem is that the avocado tests are using /var/tmp but we only
> have AppArmor rules for /tmp/
The file AppArmor gives the error for is not in /var/tmp:
it's in a local directory inside QEMU's build dir:
Nov 5 16:01:14 e104462 kernel: [946406.489088] audit: type=1400
audit(1730822474.384:446): apparmor="DENIED" operation="mknod"
profile="swtpm"
name="/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/tests/functional/arm/test_arm_aspeed.AST2x00Machine.test_arm_ast2600_evb_buildroot_tpm/qemu-machine-hhuvwytc/.lock"
pid=2820156 comm="swtpm" requested_mask="c" denied_mask="c" fsuid=1000
ouid=1000
> The following solutions should work:
> - do not install swtpm at all
> - sudo cp /usr/bin/swtpm /usr/local/bin/swtpm
> - as root: echo "include <abstractions/user-tmp>" >>
> /etc/apparmor.d/local/usr.bin.swtpm && apparmor_parser -r
> /etc/apparmor.d/usr.bin.swtpm
Is there no way to just have apparmor not apply at all
here? I can see why you might want it to apply for the
case of "I'm using it as part of a sandboxed VM setup",
but in this scenario I am a local user running this binary
which is not setuid root and it is accessing a file in a
directory which my user owns and has permissions for.
This should not be being rejected: there is no security
boundary involved and swtpm is not doing anything
that I could not directly do myself anyway (as you
can tell from the fact that copying the swtpm binary
to a different location and running it works).
thanks
-- PMM
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PULL 10/17] tests/functional: Convert most Aspeed machine tests
2024-11-05 18:12 ` Peter Maydell
@ 2024-11-05 18:35 ` Stefan Berger
2024-11-05 19:54 ` Peter Maydell
0 siblings, 1 reply; 34+ messages in thread
From: Stefan Berger @ 2024-11-05 18:35 UTC (permalink / raw)
To: Peter Maydell
Cc: Cédric Le Goater, qemu-arm, qemu-devel, Thomas Huth,
Daniel P. Berrange, lena.voytek
On 11/5/24 1:12 PM, Peter Maydell wrote:
> On Tue, 5 Nov 2024 at 18:02, Stefan Berger <stefanb@linux.ibm.com> wrote:
>>
>>
>>
>> On 11/5/24 12:13 PM, Peter Maydell wrote:
>>> On Tue, 5 Nov 2024 at 17:02, Stefan Berger <stefanb@linux.ibm.com> wrote:
>>>> On 11/5/24 11:14 AM, Peter Maydell wrote:
>>>>> Q1: why is apparmor forbidding swtpm from doing something that
>>>>> it needs to do to work?
>>>>
>>>> What distro and version is this?
>>>>
>>>> The profile may be too strict and not reflecting all the paths needed
>>>> for running the test cases. Ubuntu for example would have to update
>>>> their profile in such a case.
>>>
>>> This is Ubuntu 22.04 "jammy" (with swtpm 0.6.3-0ubuntu3.3).
>>>
>>>>> Q2: is there a way to run swtpm such that it is not
>>>>> confined by apparmor, for purposes of running it in a test case?
>>>>
>>>> Try either one:
>>>> - sudo aa-complain /usr/bin/swtpm
>>>> - sudo aa-disable /usr/bin/swtpm
>>>
>>> We don't have root access from QEMU's 'make check',
>>> though (and shouldn't be globally disabling apparmor
>>> even if we could). I had in mind more a way that an
>>> individual user can say "run this swtpm process but don't
>>> apply the apparmor profile to it".
>>
>> So the problem is that the avocado tests are using /var/tmp but we only
>> have AppArmor rules for /tmp/
>
> The file AppArmor gives the error for is not in /var/tmp:
> it's in a local directory inside QEMU's build dir:
>
> Nov 5 16:01:14 e104462 kernel: [946406.489088] audit: type=1400
> audit(1730822474.384:446): apparmor="DENIED" operation="mknod"
> profile="swtpm"
> name="/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/tests/functional/arm/test_arm_aspeed.AST2x00Machine.test_arm_ast2600_evb_buildroot_tpm/qemu-machine-hhuvwytc/.lock"
> pid=2820156 comm="swtpm" requested_mask="c" denied_mask="c" fsuid=1000
> ouid=1000
>> The following solutions should work:
>> - do not install swtpm at all
>> - sudo cp /usr/bin/swtpm /usr/local/bin/swtpm
>> - as root: echo "include <abstractions/user-tmp>" >>
>> /etc/apparmor.d/local/usr.bin.swtpm && apparmor_parser -r
>> /etc/apparmor.d/usr.bin.swtpm
>
> Is there no way to just have apparmor not apply at all
> here? I can see why you might want it to apply for the
If you are root you can change things. I have shown the options using
aa-complain and aa-disable that you can revert once the test has
finished: sudo aa-enforce /usr/bin/swtpm
You could also copy swtpm into a user-owned directory but you will have
to adapt the user's PATH. That's an easy option.
The most compatible option is the 3rd option since I would expect that
we will have this rule in a future version of the usr.bin.swtpm Ubuntu
profile provided by the swtpm package:
echo "include <abstractions/user-tmp>" >>
/etc/apparmor.d/local/usr.bin.swtpm
apparmor_parser -r /etc/apparmor.d/usr.bin.swtpm
> case of "I'm using it as part of a sandboxed VM setup",
> but in this scenario I am a local user running this binary
> which is not setuid root and it is accessing a file in a
> directory which my user owns and has permissions for.
> This should not be being rejected: there is no security
> boundary involved and swtpm is not doing anything
> that I could not directly do myself anyway (as you
> can tell from the fact that copying the swtpm binary
> to a different location and running it works).
I am not aware of how user/non-root-started programs can be generally
made exempt from AppArmor.
There may still be a security boundary if a user runs QEMU and swtpm was
able to manipulate (with malicious input) the user's files in some
undesirable way or copy the user's data elsewhere. In this case it may
be desirable for the user that the profile be applied and the PATH he is
using points to the standard swtpm.
>
> thanks
> -- PMM
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PULL 10/17] tests/functional: Convert most Aspeed machine tests
2024-11-05 18:35 ` Stefan Berger
@ 2024-11-05 19:54 ` Peter Maydell
2024-11-05 20:12 ` Stefan Berger
0 siblings, 1 reply; 34+ messages in thread
From: Peter Maydell @ 2024-11-05 19:54 UTC (permalink / raw)
To: Stefan Berger
Cc: Cédric Le Goater, qemu-arm, qemu-devel, Thomas Huth,
Daniel P. Berrange, lena.voytek
On Tue, 5 Nov 2024 at 18:36, Stefan Berger <stefanb@linux.ibm.com> wrote:
> On 11/5/24 1:12 PM, Peter Maydell wrote:
> > On Tue, 5 Nov 2024 at 18:02, Stefan Berger <stefanb@linux.ibm.com> wrote:
> >> On 11/5/24 12:13 PM, Peter Maydell wrote:
> > Is there no way to just have apparmor not apply at all
> > here? I can see why you might want it to apply for the
>
> If you are root you can change things. I have shown the options using
> aa-complain and aa-disable that you can revert once the test has
> finished: sudo aa-enforce /usr/bin/swtpm
>
> You could also copy swtpm into a user-owned directory but you will have
> to adapt the user's PATH. That's an easy option.
>
> The most compatible option is the 3rd option since I would expect that
> we will have this rule in a future version of the usr.bin.swtpm Ubuntu
> profile provided by the swtpm package:
>
> echo "include <abstractions/user-tmp>" >>
> /etc/apparmor.d/local/usr.bin.swtpm
> apparmor_parser -r /etc/apparmor.d/usr.bin.swtpm
>
> > case of "I'm using it as part of a sandboxed VM setup",
> > but in this scenario I am a local user running this binary
> > which is not setuid root and it is accessing a file in a
> > directory which my user owns and has permissions for.
> > This should not be being rejected: there is no security
> > boundary involved and swtpm is not doing anything
> > that I could not directly do myself anyway (as you
> > can tell from the fact that copying the swtpm binary
> > to a different location and running it works).
>
> I am not aware of how user/non-root-started programs can be generally
> made exempt from AppArmor.
>
> There may still be a security boundary if a user runs QEMU and swtpm was
> able to manipulate (with malicious input) the user's files in some
> undesirable way or copy the user's data elsewhere. In this case it may
> be desirable for the user that the profile be applied and the PATH he is
> using points to the standard swtpm.
But our test makefiles could equally well just run "cp" !
swtpm has no privilege here that we do not already have.
Anyway, the thing here is that we run swtpm like this:
swtpm socket -d --tpm2 --tpmstate dir=/path/to/somewhere --ctrl
type=unixio,path=/path/to/socket
where we use command line arguments to tell it where to
put the tpmstate and the socket.
Either:
(1) there are places where it's not valid for us to tell swtpm to
put the tpmstate or to put the control socket
(2) it's valid to put those anywhere we like
If (1), then swtpm should give a clear error message that we've
given it an invalid argument (and its manpage should say what
the restrictions are)
If (2), then apparmor should not be rejecting this usage
One of swtpm or apparmor must be wrong here and I think it should
be fixed. In particular, having the failure mode be "something
goes wrong after swtpm has successfully started and only once
it gets sent the TPM_INIT command, and the information about it
only winds up in the syslog" is pretty awkward -- it would
be much nicer if it failed fast, as soon as you ran it, and
printed the error to stderr.
In the interim, since we'd like to be able to run the test suite
on Ubuntu, it sounds like we can work around this by putting
the tpmstate and socket in either /tmp/ or somewhere under
the user's home directory.
thanks
-- PMM
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PULL 10/17] tests/functional: Convert most Aspeed machine tests
2024-11-05 19:54 ` Peter Maydell
@ 2024-11-05 20:12 ` Stefan Berger
2024-11-05 21:34 ` Peter Maydell
0 siblings, 1 reply; 34+ messages in thread
From: Stefan Berger @ 2024-11-05 20:12 UTC (permalink / raw)
To: Peter Maydell
Cc: Cédric Le Goater, qemu-arm, qemu-devel, Thomas Huth,
Daniel P. Berrange, lena.voytek
On 11/5/24 2:54 PM, Peter Maydell wrote:
> On Tue, 5 Nov 2024 at 18:36, Stefan Berger <stefanb@linux.ibm.com> wrote:
>> On 11/5/24 1:12 PM, Peter Maydell wrote:
>>> On Tue, 5 Nov 2024 at 18:02, Stefan Berger <stefanb@linux.ibm.com> wrote:
>>>> On 11/5/24 12:13 PM, Peter Maydell wrote:
>>> Is there no way to just have apparmor not apply at all
>>> here? I can see why you might want it to apply for the
>>
>> If you are root you can change things. I have shown the options using
>> aa-complain and aa-disable that you can revert once the test has
>> finished: sudo aa-enforce /usr/bin/swtpm
>>
>> You could also copy swtpm into a user-owned directory but you will have
>> to adapt the user's PATH. That's an easy option.
>>
>> The most compatible option is the 3rd option since I would expect that
>> we will have this rule in a future version of the usr.bin.swtpm Ubuntu
>> profile provided by the swtpm package:
>>
>> echo "include <abstractions/user-tmp>" >>
>> /etc/apparmor.d/local/usr.bin.swtpm
>> apparmor_parser -r /etc/apparmor.d/usr.bin.swtpm
>>
>>> case of "I'm using it as part of a sandboxed VM setup",
>>> but in this scenario I am a local user running this binary
>>> which is not setuid root and it is accessing a file in a
>>> directory which my user owns and has permissions for.
>>> This should not be being rejected: there is no security
>>> boundary involved and swtpm is not doing anything
>>> that I could not directly do myself anyway (as you
>>> can tell from the fact that copying the swtpm binary
>>> to a different location and running it works).
>>
>> I am not aware of how user/non-root-started programs can be generally
>> made exempt from AppArmor.
>>
>> There may still be a security boundary if a user runs QEMU and swtpm was
>> able to manipulate (with malicious input) the user's files in some
>> undesirable way or copy the user's data elsewhere. In this case it may
>> be desirable for the user that the profile be applied and the PATH he is
>> using points to the standard swtpm.
>
> But our test makefiles could equally well just run "cp" !
> swtpm has no privilege here that we do not already have.
>
> Anyway, the thing here is that we run swtpm like this:
>
> swtpm socket -d --tpm2 --tpmstate dir=/path/to/somewhere --ctrl
> type=unixio,path=/path/to/socket
>
> where we use command line arguments to tell it where to
> put the tpmstate and the socket.
>
> Either:
> (1) there are places where it's not valid for us to tell swtpm to
> put the tpmstate or to put the control socket
> (2) it's valid to put those anywhere we like
>
> If (1), then swtpm should give a clear error message that we've
> given it an invalid argument (and its manpage should say what
> the restrictions are)
There are no restrictions on the swtpm level when it comes to paths.
> If (2), then apparmor should not be rejecting this usage
AppArmor file restrictions are all path based. We have support for home
directory and /tmp, but were missing /var/tmp. So, please.
> > One of swtpm or apparmor must be wrong here and I think it should
> be fixed. In particular, having the failure mode be "something
As stated, we were going to fix the AppArmor path in the swtpm Ubuntu
package.
> goes wrong after swtpm has successfully started and only once
> it gets sent the TPM_INIT command, and the information about it
> only winds up in the syslog" is pretty awkward -- it would
> be much nicer if it failed fast, as soon as you ran it, and
> printed the error to stderr.
>
> In the interim, since we'd like to be able to run the test suite
> on Ubuntu, it sounds like we can work around this by putting
> the tpmstate and socket in either /tmp/ or somewhere under
> the user's home directory.
Right, I gave several options.
>
> thanks
> -- PMM
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PULL 10/17] tests/functional: Convert most Aspeed machine tests
2024-11-05 20:12 ` Stefan Berger
@ 2024-11-05 21:34 ` Peter Maydell
2024-11-05 21:50 ` Stefan Berger
0 siblings, 1 reply; 34+ messages in thread
From: Peter Maydell @ 2024-11-05 21:34 UTC (permalink / raw)
To: Stefan Berger
Cc: Cédric Le Goater, qemu-arm, qemu-devel, Thomas Huth,
Daniel P. Berrange, lena.voytek
On Tue, 5 Nov 2024 at 20:12, Stefan Berger <stefanb@linux.ibm.com> wrote:
> On 11/5/24 2:54 PM, Peter Maydell wrote:
> > On Tue, 5 Nov 2024 at 18:36, Stefan Berger <stefanb@linux.ibm.com> wrote:
> > Anyway, the thing here is that we run swtpm like this:
> >
> > swtpm socket -d --tpm2 --tpmstate dir=/path/to/somewhere --ctrl
> > type=unixio,path=/path/to/socket
> >
> > where we use command line arguments to tell it where to
> > put the tpmstate and the socket.
> >
> > Either:
> > (1) there are places where it's not valid for us to tell swtpm to
> > put the tpmstate or to put the control socket
> > (2) it's valid to put those anywhere we like
> >
> > If (1), then swtpm should give a clear error message that we've
> > given it an invalid argument (and its manpage should say what
> > the restrictions are)
>
> There are no restrictions on the swtpm level when it comes to paths.
> > If (2), then apparmor should not be rejecting this usage
>
> AppArmor file restrictions are all path based. We have support for home
> directory and /tmp, but were missing /var/tmp. So, please.
>
> > > One of swtpm or apparmor must be wrong here and I think it should
> > be fixed. In particular, having the failure mode be "something
>
> As stated, we were going to fix the AppArmor path in the swtpm Ubuntu
> package.
But AIUI the solution you've proposed is to add the user
temp directory -- abstractions/user-tmp looks like it
adds permissions for $HOME/tmp, /var/tmp and /tmp/. None
of those will fix the failure we ran into, because we're not
using any of those tmp directories. We use a directory
that's a subdirectory of wherever the user put the build
directory, which can be anywhere the user has permissions for.
That's why I'm confused -- as far as I can see the only
way to make swtpm work the way its documentation says it
should work is to for apparmor to permit anything
(or at least to permit anything that matches the file paths
the user handed swtmp, if it can do that).
Or if you want to say "this has to be in one of these
handful of authorised /tmp/ directories", then it should
say that in the manpage and check that at init time, not fail
near-silently much later. At the moment the docs and the
distro-integration of swtmp disagree, and the effect for
somebody trying to use it is very confusing.
thanks
-- PMM
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PULL 10/17] tests/functional: Convert most Aspeed machine tests
2024-11-05 21:34 ` Peter Maydell
@ 2024-11-05 21:50 ` Stefan Berger
2024-11-06 15:21 ` Stefan Berger
0 siblings, 1 reply; 34+ messages in thread
From: Stefan Berger @ 2024-11-05 21:50 UTC (permalink / raw)
To: Peter Maydell
Cc: Cédric Le Goater, qemu-arm, qemu-devel, Thomas Huth,
Daniel P. Berrange, lena.voytek
On 11/5/24 4:34 PM, Peter Maydell wrote:
> On Tue, 5 Nov 2024 at 20:12, Stefan Berger <stefanb@linux.ibm.com> wrote:
>> On 11/5/24 2:54 PM, Peter Maydell wrote:
>>> On Tue, 5 Nov 2024 at 18:36, Stefan Berger <stefanb@linux.ibm.com> wrote:
>>> Anyway, the thing here is that we run swtpm like this:
>>>
>>> swtpm socket -d --tpm2 --tpmstate dir=/path/to/somewhere --ctrl
>>> type=unixio,path=/path/to/socket
>>>
>>> where we use command line arguments to tell it where to
>>> put the tpmstate and the socket.
>>>
>>> Either:
>>> (1) there are places where it's not valid for us to tell swtpm to
>>> put the tpmstate or to put the control socket
>>> (2) it's valid to put those anywhere we like
>>>
>>> If (1), then swtpm should give a clear error message that we've
>>> given it an invalid argument (and its manpage should say what
>>> the restrictions are)
>>
>> There are no restrictions on the swtpm level when it comes to paths.
>
>>> If (2), then apparmor should not be rejecting this usage
>>
>> AppArmor file restrictions are all path based. We have support for home
>> directory and /tmp, but were missing /var/tmp. So, please.
>>
>> > > One of swtpm or apparmor must be wrong here and I think it should
>>> be fixed. In particular, having the failure mode be "something
>>
>> As stated, we were going to fix the AppArmor path in the swtpm Ubuntu
>> package.
>
> But AIUI the solution you've proposed is to add the user
> temp directory -- abstractions/user-tmp looks like it
> adds permissions for $HOME/tmp, /var/tmp and /tmp/. None
> of those will fix the failure we ran into, because we're not
> using any of those tmp directories. We use a directory
> that's a subdirectory of wherever the user put the build
> directory, which can be anywhere the user has permissions for.
Yes, you are right. The same test failed for me locally due to the usage
of /var/tmp/ path but that's not what was originally reported.
I am not aware that user-started programs can have an exception from
having their profiles applied, nor do I know whether rules exist that
allow a user to circumvent any rule. So my guess is we need rules like
either one of the following:
owner /mnt/** rwkl
or worse:
owner /** rwkl
I don't see another choice than adding one of these rules, maybe even
the 2nd. Lena?
>
> That's why I'm confused -- as far as I can see the only
> way to make swtpm work the way its documentation says it
> should work is to for apparmor to permit anything
> (or at least to permit anything that matches the file paths
> the user handed swtmp, if it can do that).
and from what I know we need explicit rules for allowing paths.
>
> Or if you want to say "this has to be in one of these
> handful of authorised /tmp/ directories", then it should
> say that in the manpage and check that at init time, not fail
> near-silently much later. At the moment the docs and the
> distro-integration of swtmp disagree, and the effect for
> somebody trying to use it is very confusing.
We haven't run into this type of a problem with paths in a while. The
applications return 'permission denied' but to find the exact reason
(LSM) for it one may have to dig into the audit log.
>
> thanks
> -- PMM
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PULL 10/17] tests/functional: Convert most Aspeed machine tests
2024-11-05 21:50 ` Stefan Berger
@ 2024-11-06 15:21 ` Stefan Berger
0 siblings, 0 replies; 34+ messages in thread
From: Stefan Berger @ 2024-11-06 15:21 UTC (permalink / raw)
To: Peter Maydell
Cc: Cédric Le Goater, qemu-arm, qemu-devel, Thomas Huth,
Daniel P. Berrange, lena.voytek
On 11/5/24 4:50 PM, Stefan Berger wrote:
>
>
>>>
>>> > > One of swtpm or apparmor must be wrong here and I think it should
>>>> be fixed. In particular, having the failure mode be "something
>>>
>>> As stated, we were going to fix the AppArmor path in the swtpm Ubuntu
>>> package.
>>
>> But AIUI the solution you've proposed is to add the user
>> temp directory -- abstractions/user-tmp looks like it
>> adds permissions for $HOME/tmp, /var/tmp and /tmp/. None
>> of those will fix the failure we ran into, because we're not
>> using any of those tmp directories. We use a directory
>> that's a subdirectory of wherever the user put the build
>> directory, which can be anywhere the user has permissions for.
>
> Yes, you are right. The same test failed for me locally due to the usage
> of /var/tmp/ path but that's not what was originally reported.
>
> I am not aware that user-started programs can have an exception from
> having their profiles applied, nor do I know whether rules exist that
> allow a user to circumvent any rule. So my guess is we need rules like
> either one of the following:
>
> owner /mnt/** rwkl
>
> or worse:
>
> owner /** rwkl
>
> I don't see another choice than adding one of these rules, maybe even
> the 2nd. Lena?
If there was value in the path-confinement of swtpm (for a few years) do
we really want to loose it now because of a test case? We could either
- adjust the test case to have swtpm use a directory under one of the
accepted paths, e.g., /tmp or /var/tmp
- or add /mnt as a newly supported path to the AppArmor profile
The latter works for the setup that Peter has but a new user creating
paths under /mymnt would cause the same discussion again.
Stefan
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PULL 11/17] aspeed/smc: Fix write incorrect data into flash in user mode
2024-10-24 6:34 [PULL 00/17] aspeed queue Cédric Le Goater
` (9 preceding siblings ...)
2024-10-24 6:35 ` [PULL 10/17] tests/functional: Convert most Aspeed machine tests Cédric Le Goater
@ 2024-10-24 6:35 ` Cédric Le Goater
2024-10-24 6:35 ` [PULL 12/17] hw/block:m25p80: Fix coding style Cédric Le Goater
` (6 subsequent siblings)
17 siblings, 0 replies; 34+ messages in thread
From: Cédric Le Goater @ 2024-10-24 6:35 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Jamin Lin, Cédric Le Goater
From: Jamin Lin <jamin_lin@aspeedtech.com>
According to the design of ASPEED SPI controllers user mode, users write the
data to flash, the SPI drivers set the Control Register(0x10) bit 0 and 1
enter user mode. Then, SPI drivers send flash commands for writing data.
Finally, SPI drivers set the Control Register (0x10) bit 2 to stop
active control and restore bit 0 and 1.
According to the design of ASPEED SMC model, firmware writes the
Control Register and the "aspeed_smc_flash_update_ctrl" function is called.
Then, this function verify Control Register(0x10) bit 0 and 1. If it set user
mode, the value of s->snoop_index is SNOOP_START else SNOOP_OFF.
If s->snoop_index is SNOOP_START, the "aspeed_smc_do_snoop" function verify
the first incomming data is a new flash command and writes the corresponding
dummy bytes if need.
However, it did not check the current unselect status. If current unselect
status is "false" and firmware set the IO MODE by Control Register bit 31:28,
the value of s->snoop_index will be changed to SNOOP_START again and
"aspeed_smc_do_snoop" misunderstand that the incomming data is the new flash
command and it causes writing unexpected data into flash.
Example:
1. Firmware set user mode by Control Register bit 0 and 1(0x03)
2. SMC model set s->snoop SNOOP_START
3. Firmware set Quad Page Program with 4-Byte Address command (0x34)
4. SMC model verify this flash command and it needs 4 dummy bytes.
5. Firmware send 4 bytes address.
6. SMC model receives 4 bytes address
7. Firmware set QPI IO MODE by Control Register bit 31. (0x80000003)
8. SMC model verify new user mode by Control Register bit 0 and 1.
Then, set s->snoop SNOOP_START again. (It is the wrong behavior.)
9. Firmware send 0xebd8c134 data and it should be written into flash.
However, SMC model misunderstand that the first incoming data, 0x34,
is the new command because the value of s->snoop is changed to SNOOP_START.
Finally, SMC sned the incorrect data to flash model.
Introduce a new unselect attribute in AspeedSMCState to save the current
unselect status for user mode and set it "true" by default.
Update "aspeed_smc_flash_update_ctrl" function to check the previous unselect
status. If both new unselect status and previous unselect status is different,
update s->snoop_index value and call "aspeed_smc_flash_do_select".
Increase VMStateDescription version.
Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
[ clg: - Replaced VMSTATE_BOOL -> VMSTATE_BOOL_V ]
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
include/hw/ssi/aspeed_smc.h | 1 +
hw/ssi/aspeed_smc.c | 40 ++++++++++++++++++++++++++-----------
2 files changed, 29 insertions(+), 12 deletions(-)
diff --git a/include/hw/ssi/aspeed_smc.h b/include/hw/ssi/aspeed_smc.h
index 234dca32b017..25b95e740608 100644
--- a/include/hw/ssi/aspeed_smc.h
+++ b/include/hw/ssi/aspeed_smc.h
@@ -82,6 +82,7 @@ struct AspeedSMCState {
uint8_t snoop_index;
uint8_t snoop_dummies;
+ bool unselect;
};
typedef struct AspeedSegments {
diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
index e3fdc66cb2b7..033cbbb59b06 100644
--- a/hw/ssi/aspeed_smc.c
+++ b/hw/ssi/aspeed_smc.c
@@ -417,7 +417,7 @@ static void aspeed_smc_flash_do_select(AspeedSMCFlash *fl, bool unselect)
AspeedSMCState *s = fl->controller;
trace_aspeed_smc_flash_select(fl->cs, unselect ? "un" : "");
-
+ s->unselect = unselect;
qemu_set_irq(s->cs_lines[fl->cs], unselect);
}
@@ -677,22 +677,35 @@ static const MemoryRegionOps aspeed_smc_flash_ops = {
static void aspeed_smc_flash_update_ctrl(AspeedSMCFlash *fl, uint32_t value)
{
AspeedSMCState *s = fl->controller;
- bool unselect;
+ bool unselect = false;
+ uint32_t old_mode;
+ uint32_t new_mode;
+
+ old_mode = s->regs[s->r_ctrl0 + fl->cs] & CTRL_CMD_MODE_MASK;
+ new_mode = value & CTRL_CMD_MODE_MASK;
- /* User mode selects the CS, other modes unselect */
- unselect = (value & CTRL_CMD_MODE_MASK) != CTRL_USERMODE;
+ if (old_mode == CTRL_USERMODE) {
+ if (new_mode != CTRL_USERMODE) {
+ unselect = true;
+ }
- /* A change of CTRL_CE_STOP_ACTIVE from 0 to 1, unselects the CS */
- if (!(s->regs[s->r_ctrl0 + fl->cs] & CTRL_CE_STOP_ACTIVE) &&
- value & CTRL_CE_STOP_ACTIVE) {
- unselect = true;
+ /* A change of CTRL_CE_STOP_ACTIVE from 0 to 1, unselects the CS */
+ if (!(s->regs[s->r_ctrl0 + fl->cs] & CTRL_CE_STOP_ACTIVE) &&
+ value & CTRL_CE_STOP_ACTIVE) {
+ unselect = true;
+ }
+ } else {
+ if (new_mode != CTRL_USERMODE) {
+ unselect = true;
+ }
}
s->regs[s->r_ctrl0 + fl->cs] = value;
- s->snoop_index = unselect ? SNOOP_OFF : SNOOP_START;
-
- aspeed_smc_flash_do_select(fl, unselect);
+ if (unselect != s->unselect) {
+ s->snoop_index = unselect ? SNOOP_OFF : SNOOP_START;
+ aspeed_smc_flash_do_select(fl, unselect);
+ }
}
static void aspeed_smc_reset(DeviceState *d)
@@ -729,6 +742,8 @@ static void aspeed_smc_reset(DeviceState *d)
qemu_set_irq(s->cs_lines[i], true);
}
+ s->unselect = true;
+
/* setup the default segment register values and regions for all */
for (i = 0; i < asc->cs_num_max; ++i) {
aspeed_smc_flash_set_segment_region(s, i,
@@ -1261,12 +1276,13 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp)
static const VMStateDescription vmstate_aspeed_smc = {
.name = "aspeed.smc",
- .version_id = 2,
+ .version_id = 3,
.minimum_version_id = 2,
.fields = (const VMStateField[]) {
VMSTATE_UINT32_ARRAY(regs, AspeedSMCState, ASPEED_SMC_R_MAX),
VMSTATE_UINT8(snoop_index, AspeedSMCState),
VMSTATE_UINT8(snoop_dummies, AspeedSMCState),
+ VMSTATE_BOOL_V(unselect, AspeedSMCState, 3),
VMSTATE_END_OF_LIST()
}
};
--
2.47.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PULL 12/17] hw/block:m25p80: Fix coding style
2024-10-24 6:34 [PULL 00/17] aspeed queue Cédric Le Goater
` (10 preceding siblings ...)
2024-10-24 6:35 ` [PULL 11/17] aspeed/smc: Fix write incorrect data into flash in user mode Cédric Le Goater
@ 2024-10-24 6:35 ` Cédric Le Goater
2024-10-24 6:35 ` [PULL 13/17] hw/block:m25p80: Support write status register 2 command (0x31) for w25q01jvq Cédric Le Goater
` (5 subsequent siblings)
17 siblings, 0 replies; 34+ messages in thread
From: Cédric Le Goater @ 2024-10-24 6:35 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Jamin Lin, Cédric Le Goater
From: Jamin Lin <jamin_lin@aspeedtech.com>
Fix coding style issues from checkpatch.pl
Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
---
hw/block/m25p80.c | 22 ++++++++++++++--------
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index f7123f9e6878..3f55b8f38561 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -61,7 +61,8 @@ typedef struct FlashPartInfo {
*/
uint8_t id[SPI_NOR_MAX_ID_LEN];
uint8_t id_len;
- /* there is confusion between manufacturers as to what a sector is. In this
+ /*
+ * there is confusion between manufacturers as to what a sector is. In this
* device model, a "sector" is the size that is erased by the ERASE_SECTOR
* command (opcode 0xd8).
*/
@@ -168,7 +169,7 @@ typedef struct FlashPartInfo {
/*
* Spansion read mode command length in bytes,
* the mode is currently not supported.
-*/
+ */
#define SPANSION_CONTINUOUS_READ_MODE_CMD_LEN 1
#define WINBOND_CONTINUOUS_READ_MODE_CMD_LEN 1
@@ -189,7 +190,8 @@ static const FlashPartInfo known_devices[] = {
{ INFO("at45db081d", 0x1f2500, 0, 64 << 10, 16, ER_4K) },
- /* Atmel EEPROMS - it is assumed, that don't care bit in command
+ /*
+ * Atmel EEPROMS - it is assumed, that don't care bit in command
* is set to 0. Block protection is not supported.
*/
{ INFO("at25128a-nonjedec", 0x0, 0, 1, 131072, EEPROM) },
@@ -275,10 +277,13 @@ static const FlashPartInfo known_devices[] = {
{ INFO_STACKED("n25q00a", 0x20bb21, 0x1000, 64 << 10, 2048, ER_4K, 4) },
{ INFO_STACKED("mt25ql01g", 0x20ba21, 0x1040, 64 << 10, 2048, ER_4K, 2) },
{ INFO_STACKED("mt25qu01g", 0x20bb21, 0x1040, 64 << 10, 2048, ER_4K, 2) },
- { INFO_STACKED("mt25ql02g", 0x20ba22, 0x1040, 64 << 10, 4096, ER_4K | ER_32K, 2) },
- { INFO_STACKED("mt25qu02g", 0x20bb22, 0x1040, 64 << 10, 4096, ER_4K | ER_32K, 2) },
+ { INFO_STACKED("mt25ql02g", 0x20ba22, 0x1040, 64 << 10, 4096,
+ ER_4K | ER_32K, 2) },
+ { INFO_STACKED("mt25qu02g", 0x20bb22, 0x1040, 64 << 10, 4096,
+ ER_4K | ER_32K, 2) },
- /* Spansion -- single (large) sector size only, at least
+ /*
+ * Spansion -- single (large) sector size only, at least
* for the chips listed here (without boot sectors).
*/
{ INFO("s25sl032p", 0x010215, 0x4d00, 64 << 10, 64, ER_4K) },
@@ -549,7 +554,8 @@ static void blk_sync_complete(void *opaque, int ret)
qemu_iovec_destroy(iov);
g_free(iov);
- /* do nothing. Masters do not directly interact with the backing store,
+ /*
+ * do nothing. Masters do not directly interact with the backing store,
* only the working copy so no mutexing required.
*/
}
@@ -1843,7 +1849,7 @@ static void m25p80_register_types(void)
type_register_static(&m25p80_info);
for (i = 0; i < ARRAY_SIZE(known_devices); ++i) {
- TypeInfo ti = {
+ const TypeInfo ti = {
.name = known_devices[i].part_name,
.parent = TYPE_M25P80,
.class_init = m25p80_class_init,
--
2.47.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PULL 13/17] hw/block:m25p80: Support write status register 2 command (0x31) for w25q01jvq
2024-10-24 6:34 [PULL 00/17] aspeed queue Cédric Le Goater
` (11 preceding siblings ...)
2024-10-24 6:35 ` [PULL 12/17] hw/block:m25p80: Fix coding style Cédric Le Goater
@ 2024-10-24 6:35 ` Cédric Le Goater
2024-10-24 6:35 ` [PULL 14/17] hw/block/m25p80: Add SFDP table for w25q80bl flash Cédric Le Goater
` (4 subsequent siblings)
17 siblings, 0 replies; 34+ messages in thread
From: Cédric Le Goater @ 2024-10-24 6:35 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Jamin Lin, Cédric Le Goater
From: Jamin Lin <jamin_lin@aspeedtech.com>
According to the w25q01jv datasheet at page 16, it is required to set QE bit
in "Status Register 2" to enable quad mode.
Currently, m25p80 support users utilize "Write Status Register 1(0x01)" command
to set QE bit in "Status Register 2" and utilize "Read Status Register 2(0x35)"
command to get the QE bit status.
However, some firmware directly utilize "Status Register 2(0x31)" command to
set QE bit. To fully support quad mode for w25q01jvq, adds WRSR2 command.
Update collecting data needed 1 byte for WRSR2 command in decode_new_cmd
function and verify QE bit at the first byte of collecting data bit 2 in
complete_collecting_data.
Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
---
hw/block/m25p80.c | 38 ++++++++++++++++++++++++++++++++++++++
1 file changed, 38 insertions(+)
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 3f55b8f38561..411d810d3b1b 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -430,6 +430,11 @@ typedef enum {
RDCR_EQIO = 0x35,
RSTQIO = 0xf5,
+ /*
+ * Winbond: 0x31 - write status register 2
+ */
+ WRSR2 = 0x31,
+
RNVCR = 0xB5,
WNVCR = 0xB1,
@@ -821,6 +826,15 @@ static void complete_collecting_data(Flash *s)
s->write_enable = false;
}
break;
+ case WRSR2:
+ switch (get_man(s)) {
+ case MAN_WINBOND:
+ s->quad_enable = !!(s->data[0] & 0x02);
+ break;
+ default:
+ break;
+ }
+ break;
case BRWR:
case EXTEND_ADDR_WRITE:
s->ear = s->data[0];
@@ -1280,7 +1294,31 @@ static void decode_new_cmd(Flash *s, uint32_t value)
}
s->pos = 0;
break;
+ case WRSR2:
+ /*
+ * If WP# is low and status_register_write_disabled is high,
+ * status register writes are disabled.
+ * This is also called "hardware protected mode" (HPM). All other
+ * combinations of the two states are called "software protected mode"
+ * (SPM), and status register writes are permitted.
+ */
+ if ((s->wp_level == 0 && s->status_register_write_disabled)
+ || !s->write_enable) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "M25P80: Status register 2 write is disabled!\n");
+ break;
+ }
+ switch (get_man(s)) {
+ case MAN_WINBOND:
+ s->needed_bytes = 1;
+ s->state = STATE_COLLECTING_DATA;
+ s->pos = 0;
+ break;
+ default:
+ break;
+ }
+ break;
case WRDI:
s->write_enable = false;
if (get_man(s) == MAN_SST) {
--
2.47.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PULL 14/17] hw/block/m25p80: Add SFDP table for w25q80bl flash
2024-10-24 6:34 [PULL 00/17] aspeed queue Cédric Le Goater
` (12 preceding siblings ...)
2024-10-24 6:35 ` [PULL 13/17] hw/block:m25p80: Support write status register 2 command (0x31) for w25q01jvq Cédric Le Goater
@ 2024-10-24 6:35 ` Cédric Le Goater
2024-10-24 6:35 ` [PULL 15/17] hw/arm/aspeed: Correct spi_model w25q256 for ast1030-a1 EVB Cédric Le Goater
` (3 subsequent siblings)
17 siblings, 0 replies; 34+ messages in thread
From: Cédric Le Goater @ 2024-10-24 6:35 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Jamin Lin, Cédric Le Goater
From: Jamin Lin <jamin_lin@aspeedtech.com>
Add the SFDP table for the Windbond w25q80bl flash.
Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
---
hw/block/m25p80_sfdp.h | 2 +-
hw/block/m25p80.c | 3 ++-
hw/block/m25p80_sfdp.c | 36 ++++++++++++++++++++++++++++++++++++
3 files changed, 39 insertions(+), 2 deletions(-)
diff --git a/hw/block/m25p80_sfdp.h b/hw/block/m25p80_sfdp.h
index 89c2d8f72d5c..35785686a0ec 100644
--- a/hw/block/m25p80_sfdp.h
+++ b/hw/block/m25p80_sfdp.h
@@ -25,7 +25,7 @@ uint8_t m25p80_sfdp_mx66l1g45g(uint32_t addr);
uint8_t m25p80_sfdp_w25q256(uint32_t addr);
uint8_t m25p80_sfdp_w25q512jv(uint32_t addr);
-
+uint8_t m25p80_sfdp_w25q80bl(uint32_t addr);
uint8_t m25p80_sfdp_w25q01jvq(uint32_t addr);
uint8_t m25p80_sfdp_is25wp256(uint32_t addr);
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 411d810d3b1b..e2e84f8b5f8d 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -356,7 +356,8 @@ static const FlashPartInfo known_devices[] = {
{ INFO("w25x64", 0xef3017, 0, 64 << 10, 128, ER_4K) },
{ INFO("w25q64", 0xef4017, 0, 64 << 10, 128, ER_4K) },
{ INFO("w25q80", 0xef5014, 0, 64 << 10, 16, ER_4K) },
- { INFO("w25q80bl", 0xef4014, 0, 64 << 10, 16, ER_4K) },
+ { INFO("w25q80bl", 0xef4014, 0, 64 << 10, 16, ER_4K),
+ .sfdp_read = m25p80_sfdp_w25q80bl },
{ INFO("w25q256", 0xef4019, 0, 64 << 10, 512, ER_4K),
.sfdp_read = m25p80_sfdp_w25q256 },
{ INFO("w25q512jv", 0xef4020, 0, 64 << 10, 1024, ER_4K),
diff --git a/hw/block/m25p80_sfdp.c b/hw/block/m25p80_sfdp.c
index 82d84cc21fe6..a03a291a09b5 100644
--- a/hw/block/m25p80_sfdp.c
+++ b/hw/block/m25p80_sfdp.c
@@ -404,6 +404,42 @@ static const uint8_t sfdp_w25q01jvq[] = {
};
define_sfdp_read(w25q01jvq);
+static const uint8_t sfdp_w25q80bl[] = {
+ 0x53, 0x46, 0x44, 0x50, 0x05, 0x01, 0x00, 0xff,
+ 0x00, 0x05, 0x01, 0x10, 0x80, 0x00, 0x00, 0xff,
+ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+ 0xe5, 0x20, 0xf1, 0xff, 0xff, 0xff, 0x7f, 0x00,
+ 0x44, 0xeb, 0x08, 0x6b, 0x08, 0x3b, 0x42, 0xbb,
+ 0xee, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00, 0x00,
+ 0xff, 0xff, 0x00, 0x00, 0x0c, 0x20, 0x0f, 0x52,
+ 0x10, 0xd8, 0x00, 0x00, 0x23, 0x02, 0xa6, 0x00,
+ 0x81, 0x6c, 0x14, 0xa7, 0xed, 0x61, 0x76, 0x33,
+ 0x7a, 0x75, 0x7a, 0x75, 0xf7, 0xa2, 0xd5, 0x5c,
+ 0x00, 0xf7, 0x1d, 0xff, 0xe9, 0x30, 0xc0, 0x80,
+ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+};
+define_sfdp_read(w25q80bl);
+
/*
* Integrated Silicon Solution (ISSI)
*/
--
2.47.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PULL 15/17] hw/arm/aspeed: Correct spi_model w25q256 for ast1030-a1 EVB.
2024-10-24 6:34 [PULL 00/17] aspeed queue Cédric Le Goater
` (13 preceding siblings ...)
2024-10-24 6:35 ` [PULL 14/17] hw/block/m25p80: Add SFDP table for w25q80bl flash Cédric Le Goater
@ 2024-10-24 6:35 ` Cédric Le Goater
2024-10-24 6:35 ` [PULL 16/17] hw/arm/aspeed: Correct fmc_model w25q80bl " Cédric Le Goater
` (2 subsequent siblings)
17 siblings, 0 replies; 34+ messages in thread
From: Cédric Le Goater @ 2024-10-24 6:35 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Jamin Lin, Cédric Le Goater
From: Jamin Lin <jamin_lin@aspeedtech.com>
Currently, the default spi_model was "sst25vf032b" whose size was 4MB for
ast1030-a1 EVB. However, according to the schematic of ast1030-a1 EVB,
ASPEED shipped default flash of spi1 and spi2 were w25q256 whose size
was 32MB.
Correct spi_model default flash to w25q256 for ast1030-a1 EVB.
Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
---
hw/arm/aspeed.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index cf0c6c580b2a..bf68224295f8 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -1643,7 +1643,7 @@ static void aspeed_minibmc_machine_ast1030_evb_class_init(ObjectClass *oc,
amc->i2c_init = ast1030_evb_i2c_init;
mc->default_ram_size = 0;
amc->fmc_model = "sst25vf032b";
- amc->spi_model = "sst25vf032b";
+ amc->spi_model = "w25q256";
amc->num_cs = 2;
amc->macs_mask = 0;
aspeed_machine_class_init_cpus_defaults(mc);
--
2.47.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PULL 16/17] hw/arm/aspeed: Correct fmc_model w25q80bl for ast1030-a1 EVB
2024-10-24 6:34 [PULL 00/17] aspeed queue Cédric Le Goater
` (14 preceding siblings ...)
2024-10-24 6:35 ` [PULL 15/17] hw/arm/aspeed: Correct spi_model w25q256 for ast1030-a1 EVB Cédric Le Goater
@ 2024-10-24 6:35 ` Cédric Le Goater
2024-10-24 6:35 ` [PULL 17/17] test/qtest/aspeed_smc-test: Fix coding style Cédric Le Goater
2024-10-25 14:23 ` [PULL 00/17] aspeed queue Peter Maydell
17 siblings, 0 replies; 34+ messages in thread
From: Cédric Le Goater @ 2024-10-24 6:35 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Jamin Lin, Cédric Le Goater
From: Jamin Lin <jamin_lin@aspeedtech.com>
Currently, the default fmc_model was "sst25vf032b" whose size was 4MB for
ast1030-a1 EVB. However, according to the schematic of ast1030-a1 EVB,
ASPEED shipped default flash of fmc_cs0 and fmc_cs1 were "w25q80bl" and
"w25q256", respectively. The size of w25q80bl is 1MB and the size of w25q256
is 32MB.
The fmc_cs0 was connected to AST1030 A1 internal flash and the fmc_cs1 was
connected to external flash. The internal flash could not be changed because
it was placed into AST1030 A1 chip. Users only can change fmc_cs1 external
flash.
So far, only supports to set the default fmc_model for all chip select pins.
In other words, users cannot set the different default flash model for
fmc_cs0 and fmc_cs1, respectively.
Correct fmc_model default flash to w25q80bl the same as AST1030 A1
internal flash for ast1030-a1 EVB.
Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
---
hw/arm/aspeed.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index bf68224295f8..b4b1ce9efb2b 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -1642,7 +1642,7 @@ static void aspeed_minibmc_machine_ast1030_evb_class_init(ObjectClass *oc,
mc->init = aspeed_minibmc_machine_init;
amc->i2c_init = ast1030_evb_i2c_init;
mc->default_ram_size = 0;
- amc->fmc_model = "sst25vf032b";
+ amc->fmc_model = "w25q80bl";
amc->spi_model = "w25q256";
amc->num_cs = 2;
amc->macs_mask = 0;
--
2.47.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PULL 17/17] test/qtest/aspeed_smc-test: Fix coding style
2024-10-24 6:34 [PULL 00/17] aspeed queue Cédric Le Goater
` (15 preceding siblings ...)
2024-10-24 6:35 ` [PULL 16/17] hw/arm/aspeed: Correct fmc_model w25q80bl " Cédric Le Goater
@ 2024-10-24 6:35 ` Cédric Le Goater
2024-10-25 14:23 ` [PULL 00/17] aspeed queue Peter Maydell
17 siblings, 0 replies; 34+ messages in thread
From: Cédric Le Goater @ 2024-10-24 6:35 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Jamin Lin, Thomas Huth, Cédric Le Goater
From: Jamin Lin <jamin_lin@aspeedtech.com>
Fix coding style issues from checkpatch.pl
Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
---
tests/qtest/aspeed_smc-test.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/tests/qtest/aspeed_smc-test.c b/tests/qtest/aspeed_smc-test.c
index c713a3700b6d..4673371d9539 100644
--- a/tests/qtest/aspeed_smc-test.c
+++ b/tests/qtest/aspeed_smc-test.c
@@ -353,7 +353,8 @@ static void test_read_page_mem(void)
uint32_t page[FLASH_PAGE_SIZE / 4];
int i;
- /* Enable 4BYTE mode for controller. This is should be strapped by
+ /*
+ * Enable 4BYTE mode for controller. This is should be strapped by
* HW for CE0 anyhow.
*/
spi_ce_ctrl(1 << CRTL_EXTENDED0);
@@ -394,7 +395,8 @@ static void test_write_page_mem(void)
uint32_t page[FLASH_PAGE_SIZE / 4];
int i;
- /* Enable 4BYTE mode for controller. This is should be strapped by
+ /*
+ * Enable 4BYTE mode for controller. This is should be strapped by
* HW for CE0 anyhow.
*/
spi_ce_ctrl(1 << CRTL_EXTENDED0);
--
2.47.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PULL 00/17] aspeed queue
2024-10-24 6:34 [PULL 00/17] aspeed queue Cédric Le Goater
` (16 preceding siblings ...)
2024-10-24 6:35 ` [PULL 17/17] test/qtest/aspeed_smc-test: Fix coding style Cédric Le Goater
@ 2024-10-25 14:23 ` Peter Maydell
17 siblings, 0 replies; 34+ messages in thread
From: Peter Maydell @ 2024-10-25 14:23 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: qemu-arm, qemu-devel
On Thu, 24 Oct 2024 at 07:38, Cédric Le Goater <clg@redhat.com> wrote:
>
> The following changes since commit 6f625ce2f21d6a1243065d236298277c56f972d5:
>
> Merge tag 'pull-request-2024-10-21' of https://gitlab.com/thuth/qemu into staging (2024-10-21 17:12:59 +0100)
>
> are available in the Git repository at:
>
> https://github.com/legoater/qemu/ tags/pull-aspeed-20241024
>
> for you to fetch changes up to 1df52a9ac0897687cff7c38705007b2b58065042:
>
> test/qtest/aspeed_smc-test: Fix coding style (2024-10-24 07:57:47 +0200)
>
> ----------------------------------------------------------------
> aspeed queue:
>
> * Fixed GPIO interrupt status when in index mode
> * Added GPIO support for the AST2700 SoC and specific test cases
> * Fixed crypto controller (HACE) Accumulative hash function
> * Converted Aspeed machine avocado tests to the new functional
> framework. SDK tests still to be addressed.
> * Fixed issue in the SSI controller when doing writes in user mode
> * Added support for the WRSR2 register of Winbond flash devices
> * Added SFDP table for the Windbond w25q80bl flash device
> * Changed flash device models for the ast1030-a1 EVB
>
Applied, thanks.
Please update the changelog at https://wiki.qemu.org/ChangeLog/9.2
for any user-visible changes.
-- PMM
^ permalink raw reply [flat|nested] 34+ messages in thread