* [PULL 01/19] Add dummy Aspeed AST2600 Display Port MCU (DPMCU)
2022-01-07 17:21 [PULL 00/19] target-arm queue Peter Maydell
@ 2022-01-07 17:21 ` Peter Maydell
2022-01-07 17:21 ` [PULL 02/19] target/arm: Add missing FEAT_TLBIOS instructions Peter Maydell
` (18 subsequent siblings)
19 siblings, 0 replies; 23+ messages in thread
From: Peter Maydell @ 2022-01-07 17:21 UTC (permalink / raw)
To: qemu-devel
From: Troy Lee <troy_lee@aspeedtech.com>
AST2600 Display Port MCU introduces 0x18000000~0x1803FFFF as it's memory
and io address. If guest machine try to access DPMCU memory, it will
cause a fatal error.
Signed-off-by: Troy Lee <troy_lee@aspeedtech.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Message-id: 20211210083034.726610-1-troy_lee@aspeedtech.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
include/hw/arm/aspeed_soc.h | 2 ++
hw/arm/aspeed_ast2600.c | 8 ++++++++
2 files changed, 10 insertions(+)
diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index 8139358549d..18fb7eed461 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -139,6 +139,8 @@ enum {
ASPEED_DEV_EMMC,
ASPEED_DEV_KCS,
ASPEED_DEV_HACE,
+ ASPEED_DEV_DPMCU,
+ ASPEED_DEV_DP,
};
#endif /* ASPEED_SOC_H */
diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index 0384357a951..e33483fb5dd 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -19,9 +19,11 @@
#include "sysemu/sysemu.h"
#define ASPEED_SOC_IOMEM_SIZE 0x00200000
+#define ASPEED_SOC_DPMCU_SIZE 0x00040000
static const hwaddr aspeed_soc_ast2600_memmap[] = {
[ASPEED_DEV_SRAM] = 0x10000000,
+ [ASPEED_DEV_DPMCU] = 0x18000000,
/* 0x16000000 0x17FFFFFF : AHB BUS do LPC Bus bridge */
[ASPEED_DEV_IOMEM] = 0x1E600000,
[ASPEED_DEV_PWM] = 0x1E610000,
@@ -44,6 +46,7 @@ static const hwaddr aspeed_soc_ast2600_memmap[] = {
[ASPEED_DEV_SCU] = 0x1E6E2000,
[ASPEED_DEV_XDMA] = 0x1E6E7000,
[ASPEED_DEV_ADC] = 0x1E6E9000,
+ [ASPEED_DEV_DP] = 0x1E6EB000,
[ASPEED_DEV_VIDEO] = 0x1E700000,
[ASPEED_DEV_SDHCI] = 0x1E740000,
[ASPEED_DEV_EMMC] = 0x1E750000,
@@ -104,6 +107,7 @@ static const int aspeed_soc_ast2600_irqmap[] = {
[ASPEED_DEV_ETH3] = 32,
[ASPEED_DEV_ETH4] = 33,
[ASPEED_DEV_KCS] = 138, /* 138 -> 142 */
+ [ASPEED_DEV_DP] = 62,
};
static qemu_irq aspeed_soc_get_irq(AspeedSoCState *s, int ctrl)
@@ -298,6 +302,10 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
memory_region_add_subregion(get_system_memory(),
sc->memmap[ASPEED_DEV_SRAM], &s->sram);
+ /* DPMCU */
+ create_unimplemented_device("aspeed.dpmcu", sc->memmap[ASPEED_DEV_DPMCU],
+ ASPEED_SOC_DPMCU_SIZE);
+
/* SCU */
if (!sysbus_realize(SYS_BUS_DEVICE(&s->scu), errp)) {
return;
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 02/19] target/arm: Add missing FEAT_TLBIOS instructions
2022-01-07 17:21 [PULL 00/19] target-arm queue Peter Maydell
2022-01-07 17:21 ` [PULL 01/19] Add dummy Aspeed AST2600 Display Port MCU (DPMCU) Peter Maydell
@ 2022-01-07 17:21 ` Peter Maydell
2022-01-07 17:21 ` [PULL 03/19] hw/intc/arm_gicv3_its: Correct off-by-one bounds check on rdbase Peter Maydell
` (17 subsequent siblings)
19 siblings, 0 replies; 23+ messages in thread
From: Peter Maydell @ 2022-01-07 17:21 UTC (permalink / raw)
To: qemu-devel
From: Idan Horowitz <idan.horowitz@gmail.com>
Some of the instructions added by the FEAT_TLBIOS extension were forgotten
when the extension was originally added to QEMU.
Fixes: 7113d618505b ("target/arm: Add support for FEAT_TLBIOS")
Signed-off-by: Idan Horowitz <idan.horowitz@gmail.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20211231103928.1455657-1-idan.horowitz@gmail.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target/arm/helper.c | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index db837d53bd9..cfca0f5ba6d 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6964,18 +6964,42 @@ static const ARMCPRegInfo tlbios_reginfo[] = {
.opc0 = 1, .opc1 = 0, .crn = 8, .crm = 1, .opc2 = 0,
.access = PL1_W, .type = ARM_CP_NO_RAW,
.writefn = tlbi_aa64_vmalle1is_write },
+ { .name = "TLBI_VAE1OS", .state = ARM_CP_STATE_AA64,
+ .opc0 = 1, .opc1 = 0, .crn = 8, .crm = 1, .opc2 = 1,
+ .access = PL1_W, .type = ARM_CP_NO_RAW,
+ .writefn = tlbi_aa64_vae1is_write },
{ .name = "TLBI_ASIDE1OS", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 0, .crn = 8, .crm = 1, .opc2 = 2,
.access = PL1_W, .type = ARM_CP_NO_RAW,
.writefn = tlbi_aa64_vmalle1is_write },
+ { .name = "TLBI_VAAE1OS", .state = ARM_CP_STATE_AA64,
+ .opc0 = 1, .opc1 = 0, .crn = 8, .crm = 1, .opc2 = 3,
+ .access = PL1_W, .type = ARM_CP_NO_RAW,
+ .writefn = tlbi_aa64_vae1is_write },
+ { .name = "TLBI_VALE1OS", .state = ARM_CP_STATE_AA64,
+ .opc0 = 1, .opc1 = 0, .crn = 8, .crm = 1, .opc2 = 5,
+ .access = PL1_W, .type = ARM_CP_NO_RAW,
+ .writefn = tlbi_aa64_vae1is_write },
+ { .name = "TLBI_VAALE1OS", .state = ARM_CP_STATE_AA64,
+ .opc0 = 1, .opc1 = 0, .crn = 8, .crm = 1, .opc2 = 7,
+ .access = PL1_W, .type = ARM_CP_NO_RAW,
+ .writefn = tlbi_aa64_vae1is_write },
{ .name = "TLBI_ALLE2OS", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 4, .crn = 8, .crm = 1, .opc2 = 0,
.access = PL2_W, .type = ARM_CP_NO_RAW,
.writefn = tlbi_aa64_alle2is_write },
+ { .name = "TLBI_VAE2OS", .state = ARM_CP_STATE_AA64,
+ .opc0 = 1, .opc1 = 4, .crn = 8, .crm = 1, .opc2 = 1,
+ .access = PL2_W, .type = ARM_CP_NO_RAW,
+ .writefn = tlbi_aa64_vae2is_write },
{ .name = "TLBI_ALLE1OS", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 4, .crn = 8, .crm = 1, .opc2 = 4,
.access = PL2_W, .type = ARM_CP_NO_RAW,
.writefn = tlbi_aa64_alle1is_write },
+ { .name = "TLBI_VALE2OS", .state = ARM_CP_STATE_AA64,
+ .opc0 = 1, .opc1 = 4, .crn = 8, .crm = 1, .opc2 = 5,
+ .access = PL2_W, .type = ARM_CP_NO_RAW,
+ .writefn = tlbi_aa64_vae2is_write },
{ .name = "TLBI_VMALLS12E1OS", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 4, .crn = 8, .crm = 1, .opc2 = 6,
.access = PL2_W, .type = ARM_CP_NO_RAW,
@@ -6996,6 +7020,14 @@ static const ARMCPRegInfo tlbios_reginfo[] = {
.opc0 = 1, .opc1 = 6, .crn = 8, .crm = 1, .opc2 = 0,
.access = PL3_W, .type = ARM_CP_NO_RAW,
.writefn = tlbi_aa64_alle3is_write },
+ { .name = "TLBI_VAE3OS", .state = ARM_CP_STATE_AA64,
+ .opc0 = 1, .opc1 = 6, .crn = 8, .crm = 1, .opc2 = 1,
+ .access = PL3_W, .type = ARM_CP_NO_RAW,
+ .writefn = tlbi_aa64_vae3is_write },
+ { .name = "TLBI_VALE3OS", .state = ARM_CP_STATE_AA64,
+ .opc0 = 1, .opc1 = 6, .crn = 8, .crm = 1, .opc2 = 5,
+ .access = PL3_W, .type = ARM_CP_NO_RAW,
+ .writefn = tlbi_aa64_vae3is_write },
REGINFO_SENTINEL
};
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 03/19] hw/intc/arm_gicv3_its: Correct off-by-one bounds check on rdbase
2022-01-07 17:21 [PULL 00/19] target-arm queue Peter Maydell
2022-01-07 17:21 ` [PULL 01/19] Add dummy Aspeed AST2600 Display Port MCU (DPMCU) Peter Maydell
2022-01-07 17:21 ` [PULL 02/19] target/arm: Add missing FEAT_TLBIOS instructions Peter Maydell
@ 2022-01-07 17:21 ` Peter Maydell
2022-01-07 17:21 ` [PULL 04/19] hw/intc/arm_gicv3_its: Remove redundant ITS_CTLR_ENABLED define Peter Maydell
` (16 subsequent siblings)
19 siblings, 0 replies; 23+ messages in thread
From: Peter Maydell @ 2022-01-07 17:21 UTC (permalink / raw)
To: qemu-devel
The checks in the ITS on the rdbase values in guest commands are
off-by-one: they permit the guest to pass us a value equal to
s->gicv3->num_cpu, but the valid values are 0...num_cpu-1. This
meant the guest could cause us to index off the end of the
s->gicv3->cpu[] array when calling gicv3_redist_process_lpi(), and we
would probably crash.
(This is not a security bug, because this code is only usable
with emulation, not with KVM.)
Cc: qemu-stable@nongnu.org
Fixes: 17fb5e36aabd4b ("hw/intc: GICv3 redistributor ITS processing")
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
hw/intc/arm_gicv3_its.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
index b99e63d58f7..677b96dfe23 100644
--- a/hw/intc/arm_gicv3_its.c
+++ b/hw/intc/arm_gicv3_its.c
@@ -311,7 +311,7 @@ static bool process_its_cmd(GICv3ITSState *s, uint64_t value, uint32_t offset,
*/
rdbase = (cte & GITS_CTE_RDBASE_PROCNUM_MASK) >> 1U;
- if (rdbase > s->gicv3->num_cpu) {
+ if (rdbase >= s->gicv3->num_cpu) {
return result;
}
@@ -505,7 +505,7 @@ static bool process_mapc(GICv3ITSState *s, uint32_t offset)
valid = (value & CMD_FIELD_VALID_MASK);
- if ((icid > s->ct.maxids.max_collids) || (rdbase > s->gicv3->num_cpu)) {
+ if ((icid > s->ct.maxids.max_collids) || (rdbase >= s->gicv3->num_cpu)) {
qemu_log_mask(LOG_GUEST_ERROR,
"ITS MAPC: invalid collection table attributes "
"icid %d rdbase %" PRIu64 "\n", icid, rdbase);
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 04/19] hw/intc/arm_gicv3_its: Remove redundant ITS_CTLR_ENABLED define
2022-01-07 17:21 [PULL 00/19] target-arm queue Peter Maydell
` (2 preceding siblings ...)
2022-01-07 17:21 ` [PULL 03/19] hw/intc/arm_gicv3_its: Correct off-by-one bounds check on rdbase Peter Maydell
@ 2022-01-07 17:21 ` Peter Maydell
2022-01-07 17:21 ` [PULL 05/19] hw/intc/arm_gicv3_its: Remove maxids union from TableDesc Peter Maydell
` (15 subsequent siblings)
19 siblings, 0 replies; 23+ messages in thread
From: Peter Maydell @ 2022-01-07 17:21 UTC (permalink / raw)
To: qemu-devel
We currently define a bitmask for the GITS_CTLR ENABLED bit in
two ways: as ITS_CTLR_ENABLED, and via the FIELD() macro as
R_GITS_CTLR_ENABLED_MASK. Consistently use the FIELD macro version
everywhere and remove the redundant ITS_CTLR_ENABLED define.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/intc/gicv3_internal.h | 2 --
hw/intc/arm_gicv3_its.c | 20 ++++++++++----------
2 files changed, 10 insertions(+), 12 deletions(-)
diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
index b9c37453b04..63de8667c61 100644
--- a/hw/intc/gicv3_internal.h
+++ b/hw/intc/gicv3_internal.h
@@ -289,8 +289,6 @@ FIELD(GITS_TYPER, CIL, 36, 1)
#define GITS_IDREGS 0xFFD0
-#define ITS_CTLR_ENABLED (1U) /* ITS Enabled */
-
#define GITS_BASER_RO_MASK (R_GITS_BASER_ENTRYSIZE_MASK | \
R_GITS_BASER_TYPE_MASK)
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
index 677b96dfe23..985ae03f5fc 100644
--- a/hw/intc/arm_gicv3_its.c
+++ b/hw/intc/arm_gicv3_its.c
@@ -651,7 +651,7 @@ static void process_cmdq(GICv3ITSState *s)
uint8_t cmd;
int i;
- if (!(s->ctlr & ITS_CTLR_ENABLED)) {
+ if (!(s->ctlr & R_GITS_CTLR_ENABLED_MASK)) {
return;
}
@@ -887,7 +887,7 @@ static MemTxResult gicv3_its_translation_write(void *opaque, hwaddr offset,
switch (offset) {
case GITS_TRANSLATER:
- if (s->ctlr & ITS_CTLR_ENABLED) {
+ if (s->ctlr & R_GITS_CTLR_ENABLED_MASK) {
devid = attrs.requester_id;
result = process_its_cmd(s, data, devid, NONE);
}
@@ -912,13 +912,13 @@ static bool its_writel(GICv3ITSState *s, hwaddr offset,
switch (offset) {
case GITS_CTLR:
if (value & R_GITS_CTLR_ENABLED_MASK) {
- s->ctlr |= ITS_CTLR_ENABLED;
+ s->ctlr |= R_GITS_CTLR_ENABLED_MASK;
extract_table_params(s);
extract_cmdq_params(s);
s->creadr = 0;
process_cmdq(s);
} else {
- s->ctlr &= ~ITS_CTLR_ENABLED;
+ s->ctlr &= ~R_GITS_CTLR_ENABLED_MASK;
}
break;
case GITS_CBASER:
@@ -926,7 +926,7 @@ static bool its_writel(GICv3ITSState *s, hwaddr offset,
* IMPDEF choice:- GITS_CBASER register becomes RO if ITS is
* already enabled
*/
- if (!(s->ctlr & ITS_CTLR_ENABLED)) {
+ if (!(s->ctlr & R_GITS_CTLR_ENABLED_MASK)) {
s->cbaser = deposit64(s->cbaser, 0, 32, value);
s->creadr = 0;
s->cwriter = s->creadr;
@@ -937,7 +937,7 @@ static bool its_writel(GICv3ITSState *s, hwaddr offset,
* IMPDEF choice:- GITS_CBASER register becomes RO if ITS is
* already enabled
*/
- if (!(s->ctlr & ITS_CTLR_ENABLED)) {
+ if (!(s->ctlr & R_GITS_CTLR_ENABLED_MASK)) {
s->cbaser = deposit64(s->cbaser, 32, 32, value);
s->creadr = 0;
s->cwriter = s->creadr;
@@ -979,7 +979,7 @@ static bool its_writel(GICv3ITSState *s, hwaddr offset,
* IMPDEF choice:- GITS_BASERn register becomes RO if ITS is
* already enabled
*/
- if (!(s->ctlr & ITS_CTLR_ENABLED)) {
+ if (!(s->ctlr & R_GITS_CTLR_ENABLED_MASK)) {
index = (offset - GITS_BASER) / 8;
if (offset & 7) {
@@ -1076,7 +1076,7 @@ static bool its_writell(GICv3ITSState *s, hwaddr offset,
* IMPDEF choice:- GITS_BASERn register becomes RO if ITS is
* already enabled
*/
- if (!(s->ctlr & ITS_CTLR_ENABLED)) {
+ if (!(s->ctlr & R_GITS_CTLR_ENABLED_MASK)) {
index = (offset - GITS_BASER) / 8;
s->baser[index] &= GITS_BASER_RO_MASK;
s->baser[index] |= (value & ~GITS_BASER_RO_MASK);
@@ -1087,7 +1087,7 @@ static bool its_writell(GICv3ITSState *s, hwaddr offset,
* IMPDEF choice:- GITS_CBASER register becomes RO if ITS is
* already enabled
*/
- if (!(s->ctlr & ITS_CTLR_ENABLED)) {
+ if (!(s->ctlr & R_GITS_CTLR_ENABLED_MASK)) {
s->cbaser = value;
s->creadr = 0;
s->cwriter = s->creadr;
@@ -1298,7 +1298,7 @@ static void gicv3_its_reset(DeviceState *dev)
static void gicv3_its_post_load(GICv3ITSState *s)
{
- if (s->ctlr & ITS_CTLR_ENABLED) {
+ if (s->ctlr & R_GITS_CTLR_ENABLED_MASK) {
extract_table_params(s);
extract_cmdq_params(s);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 05/19] hw/intc/arm_gicv3_its: Remove maxids union from TableDesc
2022-01-07 17:21 [PULL 00/19] target-arm queue Peter Maydell
` (3 preceding siblings ...)
2022-01-07 17:21 ` [PULL 04/19] hw/intc/arm_gicv3_its: Remove redundant ITS_CTLR_ENABLED define Peter Maydell
@ 2022-01-07 17:21 ` Peter Maydell
2022-01-07 17:21 ` [PULL 06/19] hw/intc/arm_gicv3_its: Don't return early in extract_table_params() loop Peter Maydell
` (14 subsequent siblings)
19 siblings, 0 replies; 23+ messages in thread
From: Peter Maydell @ 2022-01-07 17:21 UTC (permalink / raw)
To: qemu-devel
The TableDesc struct defines properties of the in-guest-memory tables
which the guest tells us about by writing to the GITS_BASER<n>
registers. This struct currently has a union 'maxids', but all the
fields of the union have the same type (uint32_t) and do the same
thing (record one-greater-than the maximum ID value that can be used
as an index into the table).
We're about to add another table type (the GICv4 vPE table); rather
than adding another specifically-named union field for that table
type with the same type as the other union fields, remove the union
entirely and just have a 'uint32_t max_ids' struct field.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
include/hw/intc/arm_gicv3_its_common.h | 5 +----
hw/intc/arm_gicv3_its.c | 20 ++++++++++----------
2 files changed, 11 insertions(+), 14 deletions(-)
diff --git a/include/hw/intc/arm_gicv3_its_common.h b/include/hw/intc/arm_gicv3_its_common.h
index 4e79145dde3..85a144b0e49 100644
--- a/include/hw/intc/arm_gicv3_its_common.h
+++ b/include/hw/intc/arm_gicv3_its_common.h
@@ -47,10 +47,7 @@ typedef struct {
uint16_t entry_sz;
uint32_t page_sz;
uint32_t max_entries;
- union {
- uint32_t max_devids;
- uint32_t max_collids;
- } maxids;
+ uint32_t max_ids;
uint64_t base_addr;
} TableDesc;
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
index 985ae03f5fc..f321f10189e 100644
--- a/hw/intc/arm_gicv3_its.c
+++ b/hw/intc/arm_gicv3_its.c
@@ -287,10 +287,10 @@ static bool process_its_cmd(GICv3ITSState *s, uint64_t value, uint32_t offset,
* In this implementation, in case of guest errors we ignore the
* command and move onto the next command in the queue.
*/
- if (devid > s->dt.maxids.max_devids) {
+ if (devid > s->dt.max_ids) {
qemu_log_mask(LOG_GUEST_ERROR,
"%s: invalid command attributes: devid %d>%d",
- __func__, devid, s->dt.maxids.max_devids);
+ __func__, devid, s->dt.max_ids);
} else if (!dte_valid || !ite_valid || !cte_valid) {
qemu_log_mask(LOG_GUEST_ERROR,
@@ -384,7 +384,7 @@ static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset,
max_Intid = (1ULL << (GICD_TYPER_IDBITS + 1)) - 1;
}
- if ((devid > s->dt.maxids.max_devids) || (icid > s->ct.maxids.max_collids)
+ if ((devid > s->dt.max_ids) || (icid > s->ct.max_ids)
|| !dte_valid || (eventid > max_eventid) ||
(!ignore_pInt && (((pIntid < GICV3_LPI_INTID_START) ||
(pIntid > max_Intid)) && (pIntid != INTID_SPURIOUS)))) {
@@ -505,7 +505,7 @@ static bool process_mapc(GICv3ITSState *s, uint32_t offset)
valid = (value & CMD_FIELD_VALID_MASK);
- if ((icid > s->ct.maxids.max_collids) || (rdbase >= s->gicv3->num_cpu)) {
+ if ((icid > s->ct.max_ids) || (rdbase >= s->gicv3->num_cpu)) {
qemu_log_mask(LOG_GUEST_ERROR,
"ITS MAPC: invalid collection table attributes "
"icid %d rdbase %" PRIu64 "\n", icid, rdbase);
@@ -618,7 +618,7 @@ static bool process_mapd(GICv3ITSState *s, uint64_t value, uint32_t offset)
valid = (value & CMD_FIELD_VALID_MASK);
- if ((devid > s->dt.maxids.max_devids) ||
+ if ((devid > s->dt.max_ids) ||
(size > FIELD_EX64(s->typer, GITS_TYPER, IDBITS))) {
qemu_log_mask(LOG_GUEST_ERROR,
"ITS MAPD: invalid device table attributes "
@@ -810,8 +810,8 @@ static void extract_table_params(GICv3ITSState *s)
(page_sz / s->dt.entry_sz));
}
- s->dt.maxids.max_devids = (1UL << (FIELD_EX64(s->typer, GITS_TYPER,
- DEVBITS) + 1));
+ s->dt.max_ids = (1UL << (FIELD_EX64(s->typer, GITS_TYPER,
+ DEVBITS) + 1));
s->dt.base_addr = baser_base_addr(value, page_sz);
@@ -842,11 +842,11 @@ static void extract_table_params(GICv3ITSState *s)
}
if (FIELD_EX64(s->typer, GITS_TYPER, CIL)) {
- s->ct.maxids.max_collids = (1UL << (FIELD_EX64(s->typer,
- GITS_TYPER, CIDBITS) + 1));
+ s->ct.max_ids = (1UL << (FIELD_EX64(s->typer,
+ GITS_TYPER, CIDBITS) + 1));
} else {
/* 16-bit CollectionId supported when CIL == 0 */
- s->ct.maxids.max_collids = (1UL << 16);
+ s->ct.max_ids = (1UL << 16);
}
s->ct.base_addr = baser_base_addr(value, page_sz);
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 06/19] hw/intc/arm_gicv3_its: Don't return early in extract_table_params() loop
2022-01-07 17:21 [PULL 00/19] target-arm queue Peter Maydell
` (4 preceding siblings ...)
2022-01-07 17:21 ` [PULL 05/19] hw/intc/arm_gicv3_its: Remove maxids union from TableDesc Peter Maydell
@ 2022-01-07 17:21 ` Peter Maydell
2022-01-07 17:21 ` [PULL 07/19] hw/intc/arm_gicv3_its: Reduce code duplication in extract_table_params() Peter Maydell
` (13 subsequent siblings)
19 siblings, 0 replies; 23+ messages in thread
From: Peter Maydell @ 2022-01-07 17:21 UTC (permalink / raw)
To: qemu-devel
In extract_table_params() we process each GITS_BASER<n> register. If
the register's Valid bit is not set, this means there is no
in-guest-memory table and so we should not try to interpret the other
fields in the register. This was incorrectly coded as a 'return'
rather than a 'break', so instead of looping round to process the
next GITS_BASER<n> we would stop entirely, treating any later tables
as being not valid also.
This has no real guest-visible effects because (since we don't have
GITS_TYPER.HCC != 0) the guest must in any case set up all the
GITS_BASER<n> to point to valid tables, so this only happens in an
odd misbehaving-guest corner case.
Fix the check to 'break', so that we leave the case statement and
loop back around to the next GITS_BASER<n>.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
hw/intc/arm_gicv3_its.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
index f321f10189e..c97b9982ae1 100644
--- a/hw/intc/arm_gicv3_its.c
+++ b/hw/intc/arm_gicv3_its.c
@@ -795,7 +795,7 @@ static void extract_table_params(GICv3ITSState *s)
s->dt.valid = FIELD_EX64(value, GITS_BASER, VALID);
if (!s->dt.valid) {
- return;
+ break;
}
s->dt.page_sz = page_sz;
@@ -826,7 +826,7 @@ static void extract_table_params(GICv3ITSState *s)
* hence writes are discarded if ct.valid is 0
*/
if (!s->ct.valid) {
- return;
+ break;
}
s->ct.page_sz = page_sz;
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 07/19] hw/intc/arm_gicv3_its: Reduce code duplication in extract_table_params()
2022-01-07 17:21 [PULL 00/19] target-arm queue Peter Maydell
` (5 preceding siblings ...)
2022-01-07 17:21 ` [PULL 06/19] hw/intc/arm_gicv3_its: Don't return early in extract_table_params() loop Peter Maydell
@ 2022-01-07 17:21 ` Peter Maydell
2022-01-07 17:21 ` [PULL 08/19] hw/intc/arm_gicv3_its: Correct setting of TableDesc entry_sz Peter Maydell
` (12 subsequent siblings)
19 siblings, 0 replies; 23+ messages in thread
From: Peter Maydell @ 2022-01-07 17:21 UTC (permalink / raw)
To: qemu-devel
The extract_table_params() decodes the fields in the GITS_BASER<n>
registers into TableDesc structs. Since the fields are the same for
all the GITS_BASER<n> registers, there is currently a lot of code
duplication within the switch (type) statement. Refactor so that the
cases include only what is genuinely different for each type:
the calculation of the number of bits in the ID value that indexes
into the table.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/intc/arm_gicv3_its.c | 97 +++++++++++++++++------------------------
1 file changed, 40 insertions(+), 57 deletions(-)
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
index c97b9982ae1..84808b1e298 100644
--- a/hw/intc/arm_gicv3_its.c
+++ b/hw/intc/arm_gicv3_its.c
@@ -758,6 +758,9 @@ static void extract_table_params(GICv3ITSState *s)
uint64_t value;
for (int i = 0; i < 8; i++) {
+ TableDesc *td;
+ int idbits;
+
value = s->baser[i];
if (!value) {
@@ -789,73 +792,53 @@ static void extract_table_params(GICv3ITSState *s)
type = FIELD_EX64(value, GITS_BASER, TYPE);
switch (type) {
-
case GITS_BASER_TYPE_DEVICE:
- memset(&s->dt, 0 , sizeof(s->dt));
- s->dt.valid = FIELD_EX64(value, GITS_BASER, VALID);
-
- if (!s->dt.valid) {
- break;
- }
-
- s->dt.page_sz = page_sz;
- s->dt.indirect = FIELD_EX64(value, GITS_BASER, INDIRECT);
- s->dt.entry_sz = FIELD_EX64(value, GITS_BASER, ENTRYSIZE);
-
- if (!s->dt.indirect) {
- s->dt.max_entries = (num_pages * page_sz) / s->dt.entry_sz;
- } else {
- s->dt.max_entries = (((num_pages * page_sz) /
- L1TABLE_ENTRY_SIZE) *
- (page_sz / s->dt.entry_sz));
- }
-
- s->dt.max_ids = (1UL << (FIELD_EX64(s->typer, GITS_TYPER,
- DEVBITS) + 1));
-
- s->dt.base_addr = baser_base_addr(value, page_sz);
-
+ td = &s->dt;
+ idbits = FIELD_EX64(s->typer, GITS_TYPER, DEVBITS) + 1;
break;
-
case GITS_BASER_TYPE_COLLECTION:
- memset(&s->ct, 0 , sizeof(s->ct));
- s->ct.valid = FIELD_EX64(value, GITS_BASER, VALID);
-
- /*
- * GITS_TYPER.HCC is 0 for this implementation
- * hence writes are discarded if ct.valid is 0
- */
- if (!s->ct.valid) {
- break;
- }
-
- s->ct.page_sz = page_sz;
- s->ct.indirect = FIELD_EX64(value, GITS_BASER, INDIRECT);
- s->ct.entry_sz = FIELD_EX64(value, GITS_BASER, ENTRYSIZE);
-
- if (!s->ct.indirect) {
- s->ct.max_entries = (num_pages * page_sz) / s->ct.entry_sz;
- } else {
- s->ct.max_entries = (((num_pages * page_sz) /
- L1TABLE_ENTRY_SIZE) *
- (page_sz / s->ct.entry_sz));
- }
-
+ td = &s->ct;
if (FIELD_EX64(s->typer, GITS_TYPER, CIL)) {
- s->ct.max_ids = (1UL << (FIELD_EX64(s->typer,
- GITS_TYPER, CIDBITS) + 1));
+ idbits = FIELD_EX64(s->typer, GITS_TYPER, CIDBITS) + 1;
} else {
/* 16-bit CollectionId supported when CIL == 0 */
- s->ct.max_ids = (1UL << 16);
+ idbits = 16;
}
-
- s->ct.base_addr = baser_base_addr(value, page_sz);
-
break;
-
default:
- break;
+ /*
+ * GITS_BASER<n>.TYPE is read-only, so GITS_BASER_RO_MASK
+ * ensures we will only see type values corresponding to
+ * the values set up in gicv3_its_reset().
+ */
+ g_assert_not_reached();
}
+
+ memset(td, 0, sizeof(*td));
+ td->valid = FIELD_EX64(value, GITS_BASER, VALID);
+ /*
+ * If GITS_BASER<n>.Valid is 0 for any <n> then we will not process
+ * interrupts. (GITS_TYPER.HCC is 0 for this implementation, so we
+ * do not have a special case where the GITS_BASER<n>.Valid bit is 0
+ * for the register corresponding to the Collection table but we
+ * still have to process interrupts using non-memory-backed
+ * Collection table entries.)
+ */
+ if (!td->valid) {
+ continue;
+ }
+ td->page_sz = page_sz;
+ td->indirect = FIELD_EX64(value, GITS_BASER, INDIRECT);
+ td->entry_sz = FIELD_EX64(value, GITS_BASER, ENTRYSIZE);
+ td->base_addr = baser_base_addr(value, page_sz);
+ if (!td->indirect) {
+ td->max_entries = (num_pages * page_sz) / td->entry_sz;
+ } else {
+ td->max_entries = (((num_pages * page_sz) /
+ L1TABLE_ENTRY_SIZE) *
+ (page_sz / td->entry_sz));
+ }
+ td->max_ids = 1ULL << idbits;
}
}
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 08/19] hw/intc/arm_gicv3_its: Correct setting of TableDesc entry_sz
2022-01-07 17:21 [PULL 00/19] target-arm queue Peter Maydell
` (6 preceding siblings ...)
2022-01-07 17:21 ` [PULL 07/19] hw/intc/arm_gicv3_its: Reduce code duplication in extract_table_params() Peter Maydell
@ 2022-01-07 17:21 ` Peter Maydell
2022-01-07 17:21 ` [PULL 09/19] hw/intc/arm_gicv3_its: Don't misuse GITS_TYPE_PHYSICAL define Peter Maydell
` (11 subsequent siblings)
19 siblings, 0 replies; 23+ messages in thread
From: Peter Maydell @ 2022-01-07 17:21 UTC (permalink / raw)
To: qemu-devel
We set the TableDesc entry_sz field from the appropriate
GITS_BASER.ENTRYSIZE field. That ID register field specifies the
number of bytes per table entry minus one. However when we use
td->entry_sz we assume it to be the number of bytes per table entry
(for instance we calculate the number of entries in a page by
dividing the page size by the entry size).
The effects of this bug are:
* we miscalculate the maximum number of entries in the table,
so our checks on guest index values are wrong (too lax)
* when looking up an entry in the second level of an indirect
table, we calculate an incorrect index into the L2 table.
Because we make the same incorrect calculation on both
reads and writes of the L2 table, the guest won't notice
unless it's unlucky enough to use an index value that
causes us to index off the end of the L2 table page and
cause guest memory corruption in whatever follows
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
hw/intc/arm_gicv3_its.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
index 84808b1e298..88f4d730999 100644
--- a/hw/intc/arm_gicv3_its.c
+++ b/hw/intc/arm_gicv3_its.c
@@ -829,7 +829,7 @@ static void extract_table_params(GICv3ITSState *s)
}
td->page_sz = page_sz;
td->indirect = FIELD_EX64(value, GITS_BASER, INDIRECT);
- td->entry_sz = FIELD_EX64(value, GITS_BASER, ENTRYSIZE);
+ td->entry_sz = FIELD_EX64(value, GITS_BASER, ENTRYSIZE) + 1;
td->base_addr = baser_base_addr(value, page_sz);
if (!td->indirect) {
td->max_entries = (num_pages * page_sz) / td->entry_sz;
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 09/19] hw/intc/arm_gicv3_its: Don't misuse GITS_TYPE_PHYSICAL define
2022-01-07 17:21 [PULL 00/19] target-arm queue Peter Maydell
` (7 preceding siblings ...)
2022-01-07 17:21 ` [PULL 08/19] hw/intc/arm_gicv3_its: Correct setting of TableDesc entry_sz Peter Maydell
@ 2022-01-07 17:21 ` Peter Maydell
2022-01-07 17:21 ` [PULL 10/19] hw/intc/arm_gicv3_its: Correct handling of MAPI Peter Maydell
` (10 subsequent siblings)
19 siblings, 0 replies; 23+ messages in thread
From: Peter Maydell @ 2022-01-07 17:21 UTC (permalink / raw)
To: qemu-devel
The GITS_TYPE_PHYSICAL define is the value we set the
GITS_TYPER.Physical field to -- this is 1 to indicate that we support
physical LPIs. (Support for virtual LPIs is the GITS_TYPER.Virtual
field.) We also use this define as the *value* that we write into an
interrupt translation table entry's INTTYPE field, which should be 1
for a physical interrupt and 0 for a virtual interrupt. Finally, we
use it as a *mask* when we read the interrupt translation table entry
INTTYPE field.
Untangle this confusion: define an ITE_INTTYPE_VIRTUAL and
ITE_INTTYPE_PHYSICAL to be the valid values of the ITE INTTYPE
field, and replace the ad-hoc collection of ITE_ENTRY_* defines with
use of the FIELD() macro to define the fields of an ITE and the
FIELD_EX64() and FIELD_DP64() macros to read and write them.
We use ITE in the new setup, rather than ITE_ENTRY, because
ITE stands for "Interrupt translation entry" and so the extra
"entry" would be redundant.
We take the opportunity to correct the name of the field that holds
the GICv4 'doorbell' interrupt ID (this is always the value 1023 in a
GICv3, which is why we were calling it the 'spurious' field).
The GITS_TYPE_PHYSICAL define is then used in only one place, where
we set the initial GITS_TYPER value. Since GITS_TYPER.Physical is
essentially a boolean, hiding the '1' value behind a macro is more
confusing than helpful, so expand out the macro there and remove the
define entirely.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
hw/intc/gicv3_internal.h | 26 ++++++++++++++------------
hw/intc/arm_gicv3_its.c | 30 +++++++++++++-----------------
2 files changed, 27 insertions(+), 29 deletions(-)
diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
index 63de8667c61..5a63e9ed5ce 100644
--- a/hw/intc/gicv3_internal.h
+++ b/hw/intc/gicv3_internal.h
@@ -354,28 +354,30 @@ FIELD(MAPC, RDBASE, 16, 32)
#define L2_TABLE_VALID_MASK CMD_FIELD_VALID_MASK
#define TABLE_ENTRY_VALID_MASK (1ULL << 0)
-/**
- * Default features advertised by this version of ITS
- */
-/* Physical LPIs supported */
-#define GITS_TYPE_PHYSICAL (1U << 0)
-
/*
* 12 bytes Interrupt translation Table Entry size
* as per Table 5.3 in GICv3 spec
* ITE Lower 8 Bytes
* Bits: | 49 ... 26 | 25 ... 2 | 1 | 0 |
- * Values: | 1023 | IntNum | IntType | Valid |
+ * Values: | Doorbell | IntNum | IntType | Valid |
* ITE Higher 4 Bytes
* Bits: | 31 ... 16 | 15 ...0 |
* Values: | vPEID | ICID |
+ * (When Doorbell is unused, as it always is in GICv3, it is 1023)
*/
#define ITS_ITT_ENTRY_SIZE 0xC
-#define ITE_ENTRY_INTTYPE_SHIFT 1
-#define ITE_ENTRY_INTID_SHIFT 2
-#define ITE_ENTRY_INTID_MASK MAKE_64BIT_MASK(2, 24)
-#define ITE_ENTRY_INTSP_SHIFT 26
-#define ITE_ENTRY_ICID_MASK MAKE_64BIT_MASK(0, 16)
+
+FIELD(ITE_L, VALID, 0, 1)
+FIELD(ITE_L, INTTYPE, 1, 1)
+FIELD(ITE_L, INTID, 2, 24)
+FIELD(ITE_L, DOORBELL, 26, 24)
+
+FIELD(ITE_H, ICID, 0, 16)
+FIELD(ITE_H, VPEID, 16, 16)
+
+/* Possible values for ITE_L INTTYPE */
+#define ITE_INTTYPE_VIRTUAL 0
+#define ITE_INTTYPE_PHYSICAL 1
/* 16 bits EventId */
#define ITS_IDBITS GICD_TYPER_IDBITS
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
index 88f4d730999..15eb72a0a15 100644
--- a/hw/intc/arm_gicv3_its.c
+++ b/hw/intc/arm_gicv3_its.c
@@ -156,12 +156,11 @@ static bool get_ite(GICv3ITSState *s, uint32_t eventid, uint64_t dte,
MEMTXATTRS_UNSPECIFIED, res);
if (*res == MEMTX_OK) {
- if (ite.itel & TABLE_ENTRY_VALID_MASK) {
- if ((ite.itel >> ITE_ENTRY_INTTYPE_SHIFT) &
- GITS_TYPE_PHYSICAL) {
- *pIntid = (ite.itel & ITE_ENTRY_INTID_MASK) >>
- ITE_ENTRY_INTID_SHIFT;
- *icid = ite.iteh & ITE_ENTRY_ICID_MASK;
+ if (FIELD_EX64(ite.itel, ITE_L, VALID)) {
+ int inttype = FIELD_EX64(ite.itel, ITE_L, INTTYPE);
+ if (inttype == ITE_INTTYPE_PHYSICAL) {
+ *pIntid = FIELD_EX64(ite.itel, ITE_L, INTID);
+ *icid = FIELD_EX32(ite.iteh, ITE_H, ICID);
status = true;
}
}
@@ -342,8 +341,6 @@ static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset,
MemTxResult res = MEMTX_OK;
uint16_t icid = 0;
uint64_t dte = 0;
- IteEntry ite;
- uint32_t int_spurious = INTID_SPURIOUS;
bool result = false;
devid = ((value & DEVID_MASK) >> DEVID_SHIFT);
@@ -400,16 +397,16 @@ static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset,
*/
} else {
/* add ite entry to interrupt translation table */
- ite.itel = (dte_valid & TABLE_ENTRY_VALID_MASK) |
- (GITS_TYPE_PHYSICAL << ITE_ENTRY_INTTYPE_SHIFT);
-
+ IteEntry ite = {};
+ ite.itel = FIELD_DP64(ite.itel, ITE_L, VALID, dte_valid);
+ ite.itel = FIELD_DP64(ite.itel, ITE_L, INTTYPE, ITE_INTTYPE_PHYSICAL);
if (ignore_pInt) {
- ite.itel |= (eventid << ITE_ENTRY_INTID_SHIFT);
+ ite.itel = FIELD_DP64(ite.itel, ITE_L, INTID, eventid);
} else {
- ite.itel |= (pIntid << ITE_ENTRY_INTID_SHIFT);
+ ite.itel = FIELD_DP64(ite.itel, ITE_L, INTID, pIntid);
}
- ite.itel |= (int_spurious << ITE_ENTRY_INTSP_SHIFT);
- ite.iteh = icid;
+ ite.itel = FIELD_DP64(ite.itel, ITE_L, DOORBELL, INTID_SPURIOUS);
+ ite.iteh = FIELD_DP32(ite.iteh, ITE_H, ICID, icid);
result = update_ite(s, eventid, dte, ite);
}
@@ -1237,8 +1234,7 @@ static void gicv3_arm_its_realize(DeviceState *dev, Error **errp)
"gicv3-its-sysmem");
/* set the ITS default features supported */
- s->typer = FIELD_DP64(s->typer, GITS_TYPER, PHYSICAL,
- GITS_TYPE_PHYSICAL);
+ s->typer = FIELD_DP64(s->typer, GITS_TYPER, PHYSICAL, 1);
s->typer = FIELD_DP64(s->typer, GITS_TYPER, ITT_ENTRY_SIZE,
ITS_ITT_ENTRY_SIZE - 1);
s->typer = FIELD_DP64(s->typer, GITS_TYPER, IDBITS, ITS_IDBITS);
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 10/19] hw/intc/arm_gicv3_its: Correct handling of MAPI
2022-01-07 17:21 [PULL 00/19] target-arm queue Peter Maydell
` (8 preceding siblings ...)
2022-01-07 17:21 ` [PULL 09/19] hw/intc/arm_gicv3_its: Don't misuse GITS_TYPE_PHYSICAL define Peter Maydell
@ 2022-01-07 17:21 ` Peter Maydell
2022-01-07 17:21 ` [PULL 11/19] hw/intc/arm_gicv3_its: Use FIELD macros for DTEs Peter Maydell
` (9 subsequent siblings)
19 siblings, 0 replies; 23+ messages in thread
From: Peter Maydell @ 2022-01-07 17:21 UTC (permalink / raw)
To: qemu-devel
The MAPI command takes arguments DeviceID, EventID, ICID, and is
defined to be equivalent to MAPTI DeviceID, EventID, EventID, ICID.
(That is, where MAPTI takes an explicit pINTID, MAPI uses the EventID
as the pINTID.)
We didn't quite get this right. In particular the error checks for
MAPI include "EventID does not specify a valid LPI identifier", which
is the same as MAPTI's error check for the pINTID field. QEMU's code
skips the pINTID error check entirely in the MAPI case.
We can fix this bug and in the process simplify the code by switching
to the obvious implementation of setting pIntid = eventid early
if ignore_pInt is true.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
hw/intc/arm_gicv3_its.c | 18 +++++++-----------
1 file changed, 7 insertions(+), 11 deletions(-)
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
index 15eb72a0a15..6f21c56fba2 100644
--- a/hw/intc/arm_gicv3_its.c
+++ b/hw/intc/arm_gicv3_its.c
@@ -354,7 +354,9 @@ static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset,
eventid = (value & EVENTID_MASK);
- if (!ignore_pInt) {
+ if (ignore_pInt) {
+ pIntid = eventid;
+ } else {
pIntid = ((value & pINTID_MASK) >> pINTID_SHIFT);
}
@@ -377,14 +379,12 @@ static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset,
max_eventid = (1UL << (((dte >> 1U) & SIZE_MASK) + 1));
- if (!ignore_pInt) {
- max_Intid = (1ULL << (GICD_TYPER_IDBITS + 1)) - 1;
- }
+ max_Intid = (1ULL << (GICD_TYPER_IDBITS + 1)) - 1;
if ((devid > s->dt.max_ids) || (icid > s->ct.max_ids)
|| !dte_valid || (eventid > max_eventid) ||
- (!ignore_pInt && (((pIntid < GICV3_LPI_INTID_START) ||
- (pIntid > max_Intid)) && (pIntid != INTID_SPURIOUS)))) {
+ (((pIntid < GICV3_LPI_INTID_START) || (pIntid > max_Intid)) &&
+ (pIntid != INTID_SPURIOUS))) {
qemu_log_mask(LOG_GUEST_ERROR,
"%s: invalid command attributes "
"devid %d or icid %d or eventid %d or pIntid %d or"
@@ -400,11 +400,7 @@ static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset,
IteEntry ite = {};
ite.itel = FIELD_DP64(ite.itel, ITE_L, VALID, dte_valid);
ite.itel = FIELD_DP64(ite.itel, ITE_L, INTTYPE, ITE_INTTYPE_PHYSICAL);
- if (ignore_pInt) {
- ite.itel = FIELD_DP64(ite.itel, ITE_L, INTID, eventid);
- } else {
- ite.itel = FIELD_DP64(ite.itel, ITE_L, INTID, pIntid);
- }
+ ite.itel = FIELD_DP64(ite.itel, ITE_L, INTID, pIntid);
ite.itel = FIELD_DP64(ite.itel, ITE_L, DOORBELL, INTID_SPURIOUS);
ite.iteh = FIELD_DP32(ite.iteh, ITE_H, ICID, icid);
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 11/19] hw/intc/arm_gicv3_its: Use FIELD macros for DTEs
2022-01-07 17:21 [PULL 00/19] target-arm queue Peter Maydell
` (9 preceding siblings ...)
2022-01-07 17:21 ` [PULL 10/19] hw/intc/arm_gicv3_its: Correct handling of MAPI Peter Maydell
@ 2022-01-07 17:21 ` Peter Maydell
2022-01-07 17:21 ` [PULL 12/19] hw/intc/arm_gicv3_its: Correct comment about CTE RDBase field size Peter Maydell
` (8 subsequent siblings)
19 siblings, 0 replies; 23+ messages in thread
From: Peter Maydell @ 2022-01-07 17:21 UTC (permalink / raw)
To: qemu-devel
Currently the ITS code that reads and writes DTEs uses open-coded
shift-and-mask to assemble the various fields into the 64-bit DTE
word. The names of the macros used for mask and shift values are
also somewhat inconsistent, and don't follow our usual convention
that a MASK macro should specify the bits in their place in the word.
Replace all these with use of the FIELD macro.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
hw/intc/gicv3_internal.h | 7 ++++---
hw/intc/arm_gicv3_its.c | 20 +++++++++-----------
2 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
index 5a63e9ed5ce..6a3b145f377 100644
--- a/hw/intc/gicv3_internal.h
+++ b/hw/intc/gicv3_internal.h
@@ -393,9 +393,10 @@ FIELD(ITE_H, VPEID, 16, 16)
* Valid = 1 bit,ITTAddr = 44 bits,Size = 5 bits
*/
#define GITS_DTE_SIZE (0x8ULL)
-#define GITS_DTE_ITTADDR_SHIFT 6
-#define GITS_DTE_ITTADDR_MASK MAKE_64BIT_MASK(GITS_DTE_ITTADDR_SHIFT, \
- ITTADDR_LENGTH)
+
+FIELD(DTE, VALID, 0, 1)
+FIELD(DTE, SIZE, 1, 5)
+FIELD(DTE, ITTADDR, 6, 44)
/*
* 8 bytes Collection Table Entry size
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
index 6f21c56fba2..7a217b00f89 100644
--- a/hw/intc/arm_gicv3_its.c
+++ b/hw/intc/arm_gicv3_its.c
@@ -114,7 +114,7 @@ static bool update_ite(GICv3ITSState *s, uint32_t eventid, uint64_t dte,
uint64_t itt_addr;
MemTxResult res = MEMTX_OK;
- itt_addr = (dte & GITS_DTE_ITTADDR_MASK) >> GITS_DTE_ITTADDR_SHIFT;
+ itt_addr = FIELD_EX64(dte, DTE, ITTADDR);
itt_addr <<= ITTADDR_SHIFT; /* 256 byte aligned */
address_space_stq_le(as, itt_addr + (eventid * (sizeof(uint64_t) +
@@ -141,7 +141,7 @@ static bool get_ite(GICv3ITSState *s, uint32_t eventid, uint64_t dte,
bool status = false;
IteEntry ite = {};
- itt_addr = (dte & GITS_DTE_ITTADDR_MASK) >> GITS_DTE_ITTADDR_SHIFT;
+ itt_addr = FIELD_EX64(dte, DTE, ITTADDR);
itt_addr <<= ITTADDR_SHIFT; /* 256 byte aligned */
ite.itel = address_space_ldq_le(as, itt_addr +
@@ -255,10 +255,10 @@ static bool process_its_cmd(GICv3ITSState *s, uint64_t value, uint32_t offset,
if (res != MEMTX_OK) {
return result;
}
- dte_valid = dte & TABLE_ENTRY_VALID_MASK;
+ dte_valid = FIELD_EX64(dte, DTE, VALID);
if (dte_valid) {
- max_eventid = (1UL << (((dte >> 1U) & SIZE_MASK) + 1));
+ max_eventid = 1UL << (FIELD_EX64(dte, DTE, SIZE) + 1);
ite_valid = get_ite(s, eventid, dte, &icid, &pIntid, &res);
@@ -375,10 +375,8 @@ static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset,
if (res != MEMTX_OK) {
return result;
}
- dte_valid = dte & TABLE_ENTRY_VALID_MASK;
-
- max_eventid = (1UL << (((dte >> 1U) & SIZE_MASK) + 1));
-
+ dte_valid = FIELD_EX64(dte, DTE, VALID);
+ max_eventid = 1UL << (FIELD_EX64(dte, DTE, SIZE) + 1);
max_Intid = (1ULL << (GICD_TYPER_IDBITS + 1)) - 1;
if ((devid > s->dt.max_ids) || (icid > s->ct.max_ids)
@@ -529,9 +527,9 @@ static bool update_dte(GICv3ITSState *s, uint32_t devid, bool valid,
if (s->dt.valid) {
if (valid) {
/* add mapping entry to device table */
- dte = (valid & TABLE_ENTRY_VALID_MASK) |
- ((size & SIZE_MASK) << 1U) |
- (itt_addr << GITS_DTE_ITTADDR_SHIFT);
+ dte = FIELD_DP64(dte, DTE, VALID, 1);
+ dte = FIELD_DP64(dte, DTE, SIZE, size);
+ dte = FIELD_DP64(dte, DTE, ITTADDR, itt_addr);
}
} else {
return true;
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 12/19] hw/intc/arm_gicv3_its: Correct comment about CTE RDBase field size
2022-01-07 17:21 [PULL 00/19] target-arm queue Peter Maydell
` (10 preceding siblings ...)
2022-01-07 17:21 ` [PULL 11/19] hw/intc/arm_gicv3_its: Use FIELD macros for DTEs Peter Maydell
@ 2022-01-07 17:21 ` Peter Maydell
2022-01-07 17:21 ` [PULL 13/19] hw/intc/arm_gicv3_its: Use FIELD macros for CTEs Peter Maydell
` (7 subsequent siblings)
19 siblings, 0 replies; 23+ messages in thread
From: Peter Maydell @ 2022-01-07 17:21 UTC (permalink / raw)
To: qemu-devel
The comment says that in our CTE format the RDBase field is 36 bits;
in fact for us it is only 16 bits, because we use the RDBase format
where it specifies a 16-bit CPU number. The code already uses
RDBASE_PROCNUM_LENGTH (16) as the field width, so fix the comment
to match it.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
hw/intc/gicv3_internal.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
index 6a3b145f377..14e8ef68e02 100644
--- a/hw/intc/gicv3_internal.h
+++ b/hw/intc/gicv3_internal.h
@@ -400,7 +400,7 @@ FIELD(DTE, ITTADDR, 6, 44)
/*
* 8 bytes Collection Table Entry size
- * Valid = 1 bit,RDBase = 36 bits(considering max RDBASE)
+ * Valid = 1 bit, RDBase = 16 bits
*/
#define GITS_CTE_SIZE (0x8ULL)
#define GITS_CTE_RDBASE_PROCNUM_MASK MAKE_64BIT_MASK(1, RDBASE_PROCNUM_LENGTH)
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 13/19] hw/intc/arm_gicv3_its: Use FIELD macros for CTEs
2022-01-07 17:21 [PULL 00/19] target-arm queue Peter Maydell
` (11 preceding siblings ...)
2022-01-07 17:21 ` [PULL 12/19] hw/intc/arm_gicv3_its: Correct comment about CTE RDBase field size Peter Maydell
@ 2022-01-07 17:21 ` Peter Maydell
2022-01-07 17:21 ` [PULL 14/19] hw/intc/arm_gicv3_its: Fix various off-by-one errors Peter Maydell
` (6 subsequent siblings)
19 siblings, 0 replies; 23+ messages in thread
From: Peter Maydell @ 2022-01-07 17:21 UTC (permalink / raw)
To: qemu-devel
Use FIELD macros to handle CTEs, rather than ad-hoc mask-and-shift.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
hw/intc/gicv3_internal.h | 3 ++-
hw/intc/arm_gicv3_its.c | 7 ++++---
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
index 14e8ef68e02..1eeb99035da 100644
--- a/hw/intc/gicv3_internal.h
+++ b/hw/intc/gicv3_internal.h
@@ -403,7 +403,8 @@ FIELD(DTE, ITTADDR, 6, 44)
* Valid = 1 bit, RDBase = 16 bits
*/
#define GITS_CTE_SIZE (0x8ULL)
-#define GITS_CTE_RDBASE_PROCNUM_MASK MAKE_64BIT_MASK(1, RDBASE_PROCNUM_LENGTH)
+FIELD(CTE, VALID, 0, 1)
+FIELD(CTE, RDBASE, 1, RDBASE_PROCNUM_LENGTH)
/* Special interrupt IDs */
#define INTID_SECURE 1020
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
index 7a217b00f89..2949157df34 100644
--- a/hw/intc/arm_gicv3_its.c
+++ b/hw/intc/arm_gicv3_its.c
@@ -104,7 +104,7 @@ static bool get_cte(GICv3ITSState *s, uint16_t icid, uint64_t *cte,
MEMTXATTRS_UNSPECIFIED, res);
}
- return (*cte & TABLE_ENTRY_VALID_MASK) != 0;
+ return FIELD_EX64(*cte, CTE, VALID);
}
static bool update_ite(GICv3ITSState *s, uint32_t eventid, uint64_t dte,
@@ -308,7 +308,7 @@ static bool process_its_cmd(GICv3ITSState *s, uint64_t value, uint32_t offset,
* Current implementation only supports rdbase == procnum
* Hence rdbase physical address is ignored
*/
- rdbase = (cte & GITS_CTE_RDBASE_PROCNUM_MASK) >> 1U;
+ rdbase = FIELD_EX64(cte, CTE, RDBASE);
if (rdbase >= s->gicv3->num_cpu) {
return result;
@@ -426,7 +426,8 @@ static bool update_cte(GICv3ITSState *s, uint16_t icid, bool valid,
if (valid) {
/* add mapping entry to collection table */
- cte = (valid & TABLE_ENTRY_VALID_MASK) | (rdbase << 1ULL);
+ cte = FIELD_DP64(cte, CTE, VALID, 1);
+ cte = FIELD_DP64(cte, CTE, RDBASE, rdbase);
}
/*
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 14/19] hw/intc/arm_gicv3_its: Fix various off-by-one errors
2022-01-07 17:21 [PULL 00/19] target-arm queue Peter Maydell
` (12 preceding siblings ...)
2022-01-07 17:21 ` [PULL 13/19] hw/intc/arm_gicv3_its: Use FIELD macros for CTEs Peter Maydell
@ 2022-01-07 17:21 ` Peter Maydell
2022-01-07 17:21 ` [PULL 15/19] hw/intc/arm_gicv3_its: Rename max_l2_entries to num_l2_entries Peter Maydell
` (5 subsequent siblings)
19 siblings, 0 replies; 23+ messages in thread
From: Peter Maydell @ 2022-01-07 17:21 UTC (permalink / raw)
To: qemu-devel
The ITS code has to check whether various parameters passed in
commands are in-bounds, where the limit is defined in terms of the
number of bits that are available for the parameter. (For example,
the GITS_TYPER.Devbits ID register field specifies the number of
DeviceID bits minus 1, and device IDs passed in the MAPTI and MAPD
command packets must fit in that many bits.)
Currently we have off-by-one bugs in many of these bounds checks.
The typical problem is that we define a max_foo as 1 << n. In
the Devbits example, we set
s->dt.max_ids = 1UL << (GITS_TYPER.Devbits + 1).
However later when we do the bounds check we write
if (devid > s->dt.max_ids) { /* command error */ }
which incorrectly permits a devid of 1 << n.
These bugs will not cause QEMU crashes because the ID values being
checked are only used for accesses into tables held in guest memory
which we access with address_space_*() functions, but they are
incorrect behaviour of our emulation.
Fix them by standardizing on this pattern:
* bounds limits are named num_foos and are the 2^n value
(equal to the number of valid foo values)
* bounds checks are either
if (fooid < num_foos) { good }
or
if (fooid >= num_foos) { bad }
In this commit we fix the handling of the number of IDs
in the device table and the collection table, and the number
of commands that will fit in the command queue.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---
include/hw/intc/arm_gicv3_its_common.h | 6 +++---
hw/intc/arm_gicv3_its.c | 26 +++++++++++++-------------
2 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/include/hw/intc/arm_gicv3_its_common.h b/include/hw/intc/arm_gicv3_its_common.h
index 85a144b0e49..b32c697207f 100644
--- a/include/hw/intc/arm_gicv3_its_common.h
+++ b/include/hw/intc/arm_gicv3_its_common.h
@@ -46,14 +46,14 @@ typedef struct {
bool indirect;
uint16_t entry_sz;
uint32_t page_sz;
- uint32_t max_entries;
- uint32_t max_ids;
+ uint32_t num_entries;
+ uint32_t num_ids;
uint64_t base_addr;
} TableDesc;
typedef struct {
bool valid;
- uint32_t max_entries;
+ uint32_t num_entries;
uint64_t base_addr;
} CmdQDesc;
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
index 2949157df34..95c1914eb32 100644
--- a/hw/intc/arm_gicv3_its.c
+++ b/hw/intc/arm_gicv3_its.c
@@ -286,10 +286,10 @@ static bool process_its_cmd(GICv3ITSState *s, uint64_t value, uint32_t offset,
* In this implementation, in case of guest errors we ignore the
* command and move onto the next command in the queue.
*/
- if (devid > s->dt.max_ids) {
+ if (devid >= s->dt.num_ids) {
qemu_log_mask(LOG_GUEST_ERROR,
- "%s: invalid command attributes: devid %d>%d",
- __func__, devid, s->dt.max_ids);
+ "%s: invalid command attributes: devid %d>=%d",
+ __func__, devid, s->dt.num_ids);
} else if (!dte_valid || !ite_valid || !cte_valid) {
qemu_log_mask(LOG_GUEST_ERROR,
@@ -379,7 +379,7 @@ static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset,
max_eventid = 1UL << (FIELD_EX64(dte, DTE, SIZE) + 1);
max_Intid = (1ULL << (GICD_TYPER_IDBITS + 1)) - 1;
- if ((devid > s->dt.max_ids) || (icid > s->ct.max_ids)
+ if ((devid >= s->dt.num_ids) || (icid >= s->ct.num_ids)
|| !dte_valid || (eventid > max_eventid) ||
(((pIntid < GICV3_LPI_INTID_START) || (pIntid > max_Intid)) &&
(pIntid != INTID_SPURIOUS))) {
@@ -497,7 +497,7 @@ static bool process_mapc(GICv3ITSState *s, uint32_t offset)
valid = (value & CMD_FIELD_VALID_MASK);
- if ((icid > s->ct.max_ids) || (rdbase >= s->gicv3->num_cpu)) {
+ if ((icid >= s->ct.num_ids) || (rdbase >= s->gicv3->num_cpu)) {
qemu_log_mask(LOG_GUEST_ERROR,
"ITS MAPC: invalid collection table attributes "
"icid %d rdbase %" PRIu64 "\n", icid, rdbase);
@@ -610,7 +610,7 @@ static bool process_mapd(GICv3ITSState *s, uint64_t value, uint32_t offset)
valid = (value & CMD_FIELD_VALID_MASK);
- if ((devid > s->dt.max_ids) ||
+ if ((devid >= s->dt.num_ids) ||
(size > FIELD_EX64(s->typer, GITS_TYPER, IDBITS))) {
qemu_log_mask(LOG_GUEST_ERROR,
"ITS MAPD: invalid device table attributes "
@@ -649,7 +649,7 @@ static void process_cmdq(GICv3ITSState *s)
wr_offset = FIELD_EX64(s->cwriter, GITS_CWRITER, OFFSET);
- if (wr_offset > s->cq.max_entries) {
+ if (wr_offset >= s->cq.num_entries) {
qemu_log_mask(LOG_GUEST_ERROR,
"%s: invalid write offset "
"%d\n", __func__, wr_offset);
@@ -658,7 +658,7 @@ static void process_cmdq(GICv3ITSState *s)
rd_offset = FIELD_EX64(s->creadr, GITS_CREADR, OFFSET);
- if (rd_offset > s->cq.max_entries) {
+ if (rd_offset >= s->cq.num_entries) {
qemu_log_mask(LOG_GUEST_ERROR,
"%s: invalid read offset "
"%d\n", __func__, rd_offset);
@@ -721,7 +721,7 @@ static void process_cmdq(GICv3ITSState *s)
}
if (result) {
rd_offset++;
- rd_offset %= s->cq.max_entries;
+ rd_offset %= s->cq.num_entries;
s->creadr = FIELD_DP64(s->creadr, GITS_CREADR, OFFSET, rd_offset);
} else {
/*
@@ -824,13 +824,13 @@ static void extract_table_params(GICv3ITSState *s)
td->entry_sz = FIELD_EX64(value, GITS_BASER, ENTRYSIZE) + 1;
td->base_addr = baser_base_addr(value, page_sz);
if (!td->indirect) {
- td->max_entries = (num_pages * page_sz) / td->entry_sz;
+ td->num_entries = (num_pages * page_sz) / td->entry_sz;
} else {
- td->max_entries = (((num_pages * page_sz) /
+ td->num_entries = (((num_pages * page_sz) /
L1TABLE_ENTRY_SIZE) *
(page_sz / td->entry_sz));
}
- td->max_ids = 1ULL << idbits;
+ td->num_ids = 1ULL << idbits;
}
}
@@ -845,7 +845,7 @@ static void extract_cmdq_params(GICv3ITSState *s)
s->cq.valid = FIELD_EX64(value, GITS_CBASER, VALID);
if (s->cq.valid) {
- s->cq.max_entries = (num_pages * GITS_PAGE_SIZE_4K) /
+ s->cq.num_entries = (num_pages * GITS_PAGE_SIZE_4K) /
GITS_CMDQ_ENTRY_SIZE;
s->cq.base_addr = FIELD_EX64(value, GITS_CBASER, PHYADDR);
s->cq.base_addr <<= R_GITS_CBASER_PHYADDR_SHIFT;
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 15/19] hw/intc/arm_gicv3_its: Rename max_l2_entries to num_l2_entries
2022-01-07 17:21 [PULL 00/19] target-arm queue Peter Maydell
` (13 preceding siblings ...)
2022-01-07 17:21 ` [PULL 14/19] hw/intc/arm_gicv3_its: Fix various off-by-one errors Peter Maydell
@ 2022-01-07 17:21 ` Peter Maydell
2022-01-07 17:21 ` [PULL 16/19] hw/arm: Add kudo i2c eeproms Peter Maydell
` (4 subsequent siblings)
19 siblings, 0 replies; 23+ messages in thread
From: Peter Maydell @ 2022-01-07 17:21 UTC (permalink / raw)
To: qemu-devel
In several places we have a local variable max_l2_entries which is
the number of entries which will fit in a level 2 table. The
calculations done on this value are correct; rename it to
num_l2_entries to fit the convention we're using in this code.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/intc/arm_gicv3_its.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
index 95c1914eb32..fa3cdb57554 100644
--- a/hw/intc/arm_gicv3_its.c
+++ b/hw/intc/arm_gicv3_its.c
@@ -74,7 +74,7 @@ static bool get_cte(GICv3ITSState *s, uint16_t icid, uint64_t *cte,
uint64_t value;
bool valid_l2t;
uint32_t l2t_id;
- uint32_t max_l2_entries;
+ uint32_t num_l2_entries;
if (s->ct.indirect) {
l2t_id = icid / (s->ct.page_sz / L1TABLE_ENTRY_SIZE);
@@ -88,12 +88,12 @@ static bool get_cte(GICv3ITSState *s, uint16_t icid, uint64_t *cte,
valid_l2t = (value & L2_TABLE_VALID_MASK) != 0;
if (valid_l2t) {
- max_l2_entries = s->ct.page_sz / s->ct.entry_sz;
+ num_l2_entries = s->ct.page_sz / s->ct.entry_sz;
l2t_addr = value & ((1ULL << 51) - 1);
*cte = address_space_ldq_le(as, l2t_addr +
- ((icid % max_l2_entries) * GITS_CTE_SIZE),
+ ((icid % num_l2_entries) * GITS_CTE_SIZE),
MEMTXATTRS_UNSPECIFIED, res);
}
}
@@ -176,7 +176,7 @@ static uint64_t get_dte(GICv3ITSState *s, uint32_t devid, MemTxResult *res)
uint64_t value;
bool valid_l2t;
uint32_t l2t_id;
- uint32_t max_l2_entries;
+ uint32_t num_l2_entries;
if (s->dt.indirect) {
l2t_id = devid / (s->dt.page_sz / L1TABLE_ENTRY_SIZE);
@@ -190,12 +190,12 @@ static uint64_t get_dte(GICv3ITSState *s, uint32_t devid, MemTxResult *res)
valid_l2t = (value & L2_TABLE_VALID_MASK) != 0;
if (valid_l2t) {
- max_l2_entries = s->dt.page_sz / s->dt.entry_sz;
+ num_l2_entries = s->dt.page_sz / s->dt.entry_sz;
l2t_addr = value & ((1ULL << 51) - 1);
value = address_space_ldq_le(as, l2t_addr +
- ((devid % max_l2_entries) * GITS_DTE_SIZE),
+ ((devid % num_l2_entries) * GITS_DTE_SIZE),
MEMTXATTRS_UNSPECIFIED, res);
}
}
@@ -416,7 +416,7 @@ static bool update_cte(GICv3ITSState *s, uint16_t icid, bool valid,
uint64_t l2t_addr;
bool valid_l2t;
uint32_t l2t_id;
- uint32_t max_l2_entries;
+ uint32_t num_l2_entries;
uint64_t cte = 0;
MemTxResult res = MEMTX_OK;
@@ -450,12 +450,12 @@ static bool update_cte(GICv3ITSState *s, uint16_t icid, bool valid,
valid_l2t = (value & L2_TABLE_VALID_MASK) != 0;
if (valid_l2t) {
- max_l2_entries = s->ct.page_sz / s->ct.entry_sz;
+ num_l2_entries = s->ct.page_sz / s->ct.entry_sz;
l2t_addr = value & ((1ULL << 51) - 1);
address_space_stq_le(as, l2t_addr +
- ((icid % max_l2_entries) * GITS_CTE_SIZE),
+ ((icid % num_l2_entries) * GITS_CTE_SIZE),
cte, MEMTXATTRS_UNSPECIFIED, &res);
}
} else {
@@ -521,7 +521,7 @@ static bool update_dte(GICv3ITSState *s, uint32_t devid, bool valid,
uint64_t l2t_addr;
bool valid_l2t;
uint32_t l2t_id;
- uint32_t max_l2_entries;
+ uint32_t num_l2_entries;
uint64_t dte = 0;
MemTxResult res = MEMTX_OK;
@@ -556,12 +556,12 @@ static bool update_dte(GICv3ITSState *s, uint32_t devid, bool valid,
valid_l2t = (value & L2_TABLE_VALID_MASK) != 0;
if (valid_l2t) {
- max_l2_entries = s->dt.page_sz / s->dt.entry_sz;
+ num_l2_entries = s->dt.page_sz / s->dt.entry_sz;
l2t_addr = value & ((1ULL << 51) - 1);
address_space_stq_le(as, l2t_addr +
- ((devid % max_l2_entries) * GITS_DTE_SIZE),
+ ((devid % num_l2_entries) * GITS_DTE_SIZE),
dte, MEMTXATTRS_UNSPECIFIED, &res);
}
} else {
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 16/19] hw/arm: Add kudo i2c eeproms.
2022-01-07 17:21 [PULL 00/19] target-arm queue Peter Maydell
` (14 preceding siblings ...)
2022-01-07 17:21 ` [PULL 15/19] hw/intc/arm_gicv3_its: Rename max_l2_entries to num_l2_entries Peter Maydell
@ 2022-01-07 17:21 ` Peter Maydell
2022-01-07 17:21 ` [PULL 17/19] hw/arm: attach MMC to kudo-bmc Peter Maydell
` (3 subsequent siblings)
19 siblings, 0 replies; 23+ messages in thread
From: Peter Maydell @ 2022-01-07 17:21 UTC (permalink / raw)
To: qemu-devel
From: Chris Rauer <crauer@google.com>
Signed-off-by: Chris Rauer <crauer@google.com>
Reviewed-by: Hao Wu <wuhaotsh@google.com>
Reviewed-by: Patrick Venture <venture@google.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-id: 20220102215844.2888833-2-venture@google.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/arm/npcm7xx_boards.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/hw/arm/npcm7xx_boards.c b/hw/arm/npcm7xx_boards.c
index 0866d2f4f05..37de9fef434 100644
--- a/hw/arm/npcm7xx_boards.c
+++ b/hw/arm/npcm7xx_boards.c
@@ -328,6 +328,13 @@ static void quanta_gbs_i2c_init(NPCM7xxState *soc)
*/
}
+static void kudo_bmc_i2c_init(NPCM7xxState *soc)
+{
+ at24c_eeprom_init(soc, 4, 0x50, 8192); /* mbfru */
+ at24c_eeprom_init(soc, 14, 0x55, 8192); /* bmcfru */
+ /* TODO: Add remaining i2c devices. */
+}
+
static void npcm750_evb_init(MachineState *machine)
{
NPCM7xxState *soc;
@@ -391,6 +398,7 @@ static void kudo_bmc_init(MachineState *machine)
npcm7xx_connect_flash(&soc->fiu[1], 0, "mx66u51235f",
drive_get(IF_MTD, 3, 0));
+ kudo_bmc_i2c_init(soc);
npcm7xx_load_kernel(machine, soc);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 17/19] hw/arm: attach MMC to kudo-bmc
2022-01-07 17:21 [PULL 00/19] target-arm queue Peter Maydell
` (15 preceding siblings ...)
2022-01-07 17:21 ` [PULL 16/19] hw/arm: Add kudo i2c eeproms Peter Maydell
@ 2022-01-07 17:21 ` Peter Maydell
2022-01-07 17:21 ` [PULL 18/19] hw/arm: add i2c muxes " Peter Maydell
` (2 subsequent siblings)
19 siblings, 0 replies; 23+ messages in thread
From: Peter Maydell @ 2022-01-07 17:21 UTC (permalink / raw)
To: qemu-devel
From: Shengtan Mao <stmao@google.com>
Signed-off-by: Shengtan Mao <stmao@google.com>
Reviewed-by: Hao Wu <wuhaotsh@google.com>
Reviewed-by: Chris Rauer <crauer@google.com>
Message-id: 20220102215844.2888833-3-venture@google.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/arm/npcm7xx_boards.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/arm/npcm7xx_boards.c b/hw/arm/npcm7xx_boards.c
index 37de9fef434..257bf638fd5 100644
--- a/hw/arm/npcm7xx_boards.c
+++ b/hw/arm/npcm7xx_boards.c
@@ -399,6 +399,7 @@ static void kudo_bmc_init(MachineState *machine)
drive_get(IF_MTD, 3, 0));
kudo_bmc_i2c_init(soc);
+ sdhci_attach_drive(&soc->mmc.sdhci, 0);
npcm7xx_load_kernel(machine, soc);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 18/19] hw/arm: add i2c muxes to kudo-bmc
2022-01-07 17:21 [PULL 00/19] target-arm queue Peter Maydell
` (16 preceding siblings ...)
2022-01-07 17:21 ` [PULL 17/19] hw/arm: attach MMC to kudo-bmc Peter Maydell
@ 2022-01-07 17:21 ` Peter Maydell
2022-01-07 17:21 ` [PULL 19/19] hw/arm: kudo add lm75s on bus 13 Peter Maydell
2022-01-07 21:17 ` [PULL 00/19] target-arm queue Richard Henderson
19 siblings, 0 replies; 23+ messages in thread
From: Peter Maydell @ 2022-01-07 17:21 UTC (permalink / raw)
To: qemu-devel
From: Patrick Venture <venture@google.com>
Signed-off-by: Patrick Venture <venture@google.com>
Reviewed-by: Hao Wu <wuhaotsh@google.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-id: 20220102215844.2888833-4-venture@google.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/arm/npcm7xx_boards.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/hw/arm/npcm7xx_boards.c b/hw/arm/npcm7xx_boards.c
index 257bf638fd5..4cd58972c56 100644
--- a/hw/arm/npcm7xx_boards.c
+++ b/hw/arm/npcm7xx_boards.c
@@ -330,8 +330,17 @@ static void quanta_gbs_i2c_init(NPCM7xxState *soc)
static void kudo_bmc_i2c_init(NPCM7xxState *soc)
{
+ i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 1), TYPE_PCA9548, 0x75);
+ i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 1), TYPE_PCA9548, 0x77);
+
+ i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 4), TYPE_PCA9548, 0x77);
+
at24c_eeprom_init(soc, 4, 0x50, 8192); /* mbfru */
+
+ i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 13), TYPE_PCA9548, 0x77);
+
at24c_eeprom_init(soc, 14, 0x55, 8192); /* bmcfru */
+
/* TODO: Add remaining i2c devices. */
}
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 19/19] hw/arm: kudo add lm75s on bus 13
2022-01-07 17:21 [PULL 00/19] target-arm queue Peter Maydell
` (17 preceding siblings ...)
2022-01-07 17:21 ` [PULL 18/19] hw/arm: add i2c muxes " Peter Maydell
@ 2022-01-07 17:21 ` Peter Maydell
2022-01-07 21:17 ` [PULL 00/19] target-arm queue Richard Henderson
19 siblings, 0 replies; 23+ messages in thread
From: Peter Maydell @ 2022-01-07 17:21 UTC (permalink / raw)
To: qemu-devel
From: Patrick Venture <venture@google.com>
Add the four lm75s behind the mux on bus 13.
Tested by booting the firmware:
lm75 42-0048: hwmon0: sensor 'lm75'
lm75 43-0049: supply vs not found, using dummy regulator
lm75 43-0049: hwmon1: sensor 'lm75'
lm75 44-0048: supply vs not found, using dummy regulator
lm75 44-0048: hwmon2: sensor 'lm75'
lm75 45-0049: supply vs not found, using dummy regulator
lm75 45-0049: hwmon3: sensor 'lm75'
Signed-off-by: Patrick Venture <venture@google.com>
Reviewed-by: Titus Rwantare <titusr@google.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-id: 20220102215844.2888833-5-venture@google.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/arm/npcm7xx_boards.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/hw/arm/npcm7xx_boards.c b/hw/arm/npcm7xx_boards.c
index 4cd58972c56..7d0f3148be0 100644
--- a/hw/arm/npcm7xx_boards.c
+++ b/hw/arm/npcm7xx_boards.c
@@ -330,6 +330,8 @@ static void quanta_gbs_i2c_init(NPCM7xxState *soc)
static void kudo_bmc_i2c_init(NPCM7xxState *soc)
{
+ I2CSlave *i2c_mux;
+
i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 1), TYPE_PCA9548, 0x75);
i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 1), TYPE_PCA9548, 0x77);
@@ -337,7 +339,14 @@ static void kudo_bmc_i2c_init(NPCM7xxState *soc)
at24c_eeprom_init(soc, 4, 0x50, 8192); /* mbfru */
- i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 13), TYPE_PCA9548, 0x77);
+ i2c_mux = i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 13),
+ TYPE_PCA9548, 0x77);
+
+ /* tmp105 is compatible with the lm75 */
+ i2c_slave_create_simple(pca954x_i2c_get_bus(i2c_mux, 2), "tmp105", 0x48);
+ i2c_slave_create_simple(pca954x_i2c_get_bus(i2c_mux, 3), "tmp105", 0x49);
+ i2c_slave_create_simple(pca954x_i2c_get_bus(i2c_mux, 4), "tmp105", 0x48);
+ i2c_slave_create_simple(pca954x_i2c_get_bus(i2c_mux, 5), "tmp105", 0x49);
at24c_eeprom_init(soc, 14, 0x55, 8192); /* bmcfru */
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PULL 00/19] target-arm queue
2022-01-07 17:21 [PULL 00/19] target-arm queue Peter Maydell
` (18 preceding siblings ...)
2022-01-07 17:21 ` [PULL 19/19] hw/arm: kudo add lm75s on bus 13 Peter Maydell
@ 2022-01-07 21:17 ` Richard Henderson
19 siblings, 0 replies; 23+ messages in thread
From: Richard Henderson @ 2022-01-07 21:17 UTC (permalink / raw)
To: Peter Maydell, qemu-devel
On 1/7/22 9:21 AM, Peter Maydell wrote:
>
> The following changes since commit 41fb4c14ee500125dc0ce6fb573cf84b8db29ed0:
>
> Merge tag 'linux-user-for-7.0-pull-request' of https://gitlab.com/laurent_vivier/qemu into staging (2022-01-06 11:22:42 -0800)
>
> are available in the Git repository at:
>
> https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20220107
>
> for you to fetch changes up to b8905cc2dde95ca6be5e56d77053b1ca0b8fc182:
>
> hw/arm: kudo add lm75s on bus 13 (2022-01-07 17:08:01 +0000)
>
> ----------------------------------------------------------------
> target-arm queue:
> * Add dummy Aspeed AST2600 Display Port MCU (DPMCU)
> * Add missing FEAT_TLBIOS instructions
> * arm_gicv3_its: Various bug fixes and cleanups
> * kudo-bmc: Add more devices
>
> ----------------------------------------------------------------
> Chris Rauer (1):
> hw/arm: Add kudo i2c eeproms.
>
> Idan Horowitz (1):
> target/arm: Add missing FEAT_TLBIOS instructions
>
> Patrick Venture (2):
> hw/arm: add i2c muxes to kudo-bmc
> hw/arm: kudo add lm75s on bus 13
>
> Peter Maydell (13):
> hw/intc/arm_gicv3_its: Correct off-by-one bounds check on rdbase
> hw/intc/arm_gicv3_its: Remove redundant ITS_CTLR_ENABLED define
> hw/intc/arm_gicv3_its: Remove maxids union from TableDesc
> hw/intc/arm_gicv3_its: Don't return early in extract_table_params() loop
> hw/intc/arm_gicv3_its: Reduce code duplication in extract_table_params()
> hw/intc/arm_gicv3_its: Correct setting of TableDesc entry_sz
> hw/intc/arm_gicv3_its: Don't misuse GITS_TYPE_PHYSICAL define
> hw/intc/arm_gicv3_its: Correct handling of MAPI
> hw/intc/arm_gicv3_its: Use FIELD macros for DTEs
> hw/intc/arm_gicv3_its: Correct comment about CTE RDBase field size
> hw/intc/arm_gicv3_its: Use FIELD macros for CTEs
> hw/intc/arm_gicv3_its: Fix various off-by-one errors
> hw/intc/arm_gicv3_its: Rename max_l2_entries to num_l2_entries
>
> Shengtan Mao (1):
> hw/arm: attach MMC to kudo-bmc
>
> Troy Lee (1):
> Add dummy Aspeed AST2600 Display Port MCU (DPMCU)
>
> hw/intc/gicv3_internal.h | 40 +++---
> include/hw/arm/aspeed_soc.h | 2 +
> include/hw/intc/arm_gicv3_its_common.h | 9 +-
> hw/arm/aspeed_ast2600.c | 8 ++
> hw/arm/npcm7xx_boards.c | 27 ++++
> hw/intc/arm_gicv3_its.c | 234 +++++++++++++++------------------
> target/arm/helper.c | 32 +++++
> 7 files changed, 197 insertions(+), 155 deletions(-)
Applied, thanks.
r~
^ permalink raw reply [flat|nested] 23+ messages in thread