1
Last minute pullreq with one patch, fixing the GICv3 ICH_MISR_EL2.LRENP
1
The following changes since commit 41fb4c14ee500125dc0ce6fb573cf84b8db29ed0:
2
calculation. I went back-and-forth on whether to put this in, but:
3
* it's an effective regression from 6.1 (the bug itself has been
4
present since before then, but it was previously masked by the
5
other bug which we fixed in 9cee1efe92)
6
* I just realized it could cause a screaming maintenance interrupt
7
even for hypervisors like KVM that don't set LRENPIE
8
2
9
On the other hand this is very late and we haven't seen it be a
3
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)
10
problem with any guest except Qualcomm's hypervisor. So if you want
11
to decide it's better not going in that's OK too.
12
13
Tested on the gitlab CI and with a local test of nested KVM.
14
15
-- PMM
16
17
The following changes since commit 7635eff97104242d618400e4b6746d0a5c97af82:
18
19
Merge tag 'block-pull-request' of https://gitlab.com/stefanha/qemu into staging (2021-12-06 11:18:06 -0800)
20
4
21
are available in the Git repository at:
5
are available in the Git repository at:
22
6
23
https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20211207
7
https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20220107
24
8
25
for you to fetch changes up to 2958e5150dfa297dd5a51fe57a29156b8744f07f:
9
for you to fetch changes up to b8905cc2dde95ca6be5e56d77053b1ca0b8fc182:
26
10
27
gicv3: fix ICH_MISR's LRENP computation (2021-12-07 15:30:08 +0000)
11
hw/arm: kudo add lm75s on bus 13 (2022-01-07 17:08:01 +0000)
28
12
29
----------------------------------------------------------------
13
----------------------------------------------------------------
30
target-arm queue:
14
target-arm queue:
31
* Fix calculation of ICH_MISR_EL2.LRENP to avoid incorrect generation
15
* Add dummy Aspeed AST2600 Display Port MCU (DPMCU)
32
of maintenance interrupts
16
* Add missing FEAT_TLBIOS instructions
17
* arm_gicv3_its: Various bug fixes and cleanups
18
* kudo-bmc: Add more devices
33
19
34
----------------------------------------------------------------
20
----------------------------------------------------------------
35
Damien Hedde (1):
21
Chris Rauer (1):
36
gicv3: fix ICH_MISR's LRENP computation
22
hw/arm: Add kudo i2c eeproms.
37
23
38
hw/intc/arm_gicv3_cpuif.c | 3 ++-
24
Idan Horowitz (1):
39
1 file changed, 2 insertions(+), 1 deletion(-)
25
target/arm: Add missing FEAT_TLBIOS instructions
40
26
27
Patrick Venture (2):
28
hw/arm: add i2c muxes to kudo-bmc
29
hw/arm: kudo add lm75s on bus 13
30
31
Peter Maydell (13):
32
hw/intc/arm_gicv3_its: Correct off-by-one bounds check on rdbase
33
hw/intc/arm_gicv3_its: Remove redundant ITS_CTLR_ENABLED define
34
hw/intc/arm_gicv3_its: Remove maxids union from TableDesc
35
hw/intc/arm_gicv3_its: Don't return early in extract_table_params() loop
36
hw/intc/arm_gicv3_its: Reduce code duplication in extract_table_params()
37
hw/intc/arm_gicv3_its: Correct setting of TableDesc entry_sz
38
hw/intc/arm_gicv3_its: Don't misuse GITS_TYPE_PHYSICAL define
39
hw/intc/arm_gicv3_its: Correct handling of MAPI
40
hw/intc/arm_gicv3_its: Use FIELD macros for DTEs
41
hw/intc/arm_gicv3_its: Correct comment about CTE RDBase field size
42
hw/intc/arm_gicv3_its: Use FIELD macros for CTEs
43
hw/intc/arm_gicv3_its: Fix various off-by-one errors
44
hw/intc/arm_gicv3_its: Rename max_l2_entries to num_l2_entries
45
46
Shengtan Mao (1):
47
hw/arm: attach MMC to kudo-bmc
48
49
Troy Lee (1):
50
Add dummy Aspeed AST2600 Display Port MCU (DPMCU)
51
52
hw/intc/gicv3_internal.h | 40 +++---
53
include/hw/arm/aspeed_soc.h | 2 +
54
include/hw/intc/arm_gicv3_its_common.h | 9 +-
55
hw/arm/aspeed_ast2600.c | 8 ++
56
hw/arm/npcm7xx_boards.c | 27 ++++
57
hw/intc/arm_gicv3_its.c | 234 +++++++++++++++------------------
58
target/arm/helper.c | 32 +++++
59
7 files changed, 197 insertions(+), 155 deletions(-)
60
diff view generated by jsdifflib
New patch
1
From: Troy Lee <troy_lee@aspeedtech.com>
1
2
3
AST2600 Display Port MCU introduces 0x18000000~0x1803FFFF as it's memory
4
and io address. If guest machine try to access DPMCU memory, it will
5
cause a fatal error.
6
7
Signed-off-by: Troy Lee <troy_lee@aspeedtech.com>
8
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
9
Reviewed-by: Cédric Le Goater <clg@kaod.org>
10
Message-id: 20211210083034.726610-1-troy_lee@aspeedtech.com
11
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
12
---
13
include/hw/arm/aspeed_soc.h | 2 ++
14
hw/arm/aspeed_ast2600.c | 8 ++++++++
15
2 files changed, 10 insertions(+)
16
17
diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
18
index XXXXXXX..XXXXXXX 100644
19
--- a/include/hw/arm/aspeed_soc.h
20
+++ b/include/hw/arm/aspeed_soc.h
21
@@ -XXX,XX +XXX,XX @@ enum {
22
ASPEED_DEV_EMMC,
23
ASPEED_DEV_KCS,
24
ASPEED_DEV_HACE,
25
+ ASPEED_DEV_DPMCU,
26
+ ASPEED_DEV_DP,
27
};
28
29
#endif /* ASPEED_SOC_H */
30
diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
31
index XXXXXXX..XXXXXXX 100644
32
--- a/hw/arm/aspeed_ast2600.c
33
+++ b/hw/arm/aspeed_ast2600.c
34
@@ -XXX,XX +XXX,XX @@
35
#include "sysemu/sysemu.h"
36
37
#define ASPEED_SOC_IOMEM_SIZE 0x00200000
38
+#define ASPEED_SOC_DPMCU_SIZE 0x00040000
39
40
static const hwaddr aspeed_soc_ast2600_memmap[] = {
41
[ASPEED_DEV_SRAM] = 0x10000000,
42
+ [ASPEED_DEV_DPMCU] = 0x18000000,
43
/* 0x16000000 0x17FFFFFF : AHB BUS do LPC Bus bridge */
44
[ASPEED_DEV_IOMEM] = 0x1E600000,
45
[ASPEED_DEV_PWM] = 0x1E610000,
46
@@ -XXX,XX +XXX,XX @@ static const hwaddr aspeed_soc_ast2600_memmap[] = {
47
[ASPEED_DEV_SCU] = 0x1E6E2000,
48
[ASPEED_DEV_XDMA] = 0x1E6E7000,
49
[ASPEED_DEV_ADC] = 0x1E6E9000,
50
+ [ASPEED_DEV_DP] = 0x1E6EB000,
51
[ASPEED_DEV_VIDEO] = 0x1E700000,
52
[ASPEED_DEV_SDHCI] = 0x1E740000,
53
[ASPEED_DEV_EMMC] = 0x1E750000,
54
@@ -XXX,XX +XXX,XX @@ static const int aspeed_soc_ast2600_irqmap[] = {
55
[ASPEED_DEV_ETH3] = 32,
56
[ASPEED_DEV_ETH4] = 33,
57
[ASPEED_DEV_KCS] = 138, /* 138 -> 142 */
58
+ [ASPEED_DEV_DP] = 62,
59
};
60
61
static qemu_irq aspeed_soc_get_irq(AspeedSoCState *s, int ctrl)
62
@@ -XXX,XX +XXX,XX @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
63
memory_region_add_subregion(get_system_memory(),
64
sc->memmap[ASPEED_DEV_SRAM], &s->sram);
65
66
+ /* DPMCU */
67
+ create_unimplemented_device("aspeed.dpmcu", sc->memmap[ASPEED_DEV_DPMCU],
68
+ ASPEED_SOC_DPMCU_SIZE);
69
+
70
/* SCU */
71
if (!sysbus_realize(SYS_BUS_DEVICE(&s->scu), errp)) {
72
return;
73
--
74
2.25.1
75
76
diff view generated by jsdifflib
New patch
1
From: Idan Horowitz <idan.horowitz@gmail.com>
1
2
3
Some of the instructions added by the FEAT_TLBIOS extension were forgotten
4
when the extension was originally added to QEMU.
5
6
Fixes: 7113d618505b ("target/arm: Add support for FEAT_TLBIOS")
7
Signed-off-by: Idan Horowitz <idan.horowitz@gmail.com>
8
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
9
Message-id: 20211231103928.1455657-1-idan.horowitz@gmail.com
10
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
11
---
12
target/arm/helper.c | 32 ++++++++++++++++++++++++++++++++
13
1 file changed, 32 insertions(+)
14
15
diff --git a/target/arm/helper.c b/target/arm/helper.c
16
index XXXXXXX..XXXXXXX 100644
17
--- a/target/arm/helper.c
18
+++ b/target/arm/helper.c
19
@@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo tlbios_reginfo[] = {
20
.opc0 = 1, .opc1 = 0, .crn = 8, .crm = 1, .opc2 = 0,
21
.access = PL1_W, .type = ARM_CP_NO_RAW,
22
.writefn = tlbi_aa64_vmalle1is_write },
23
+ { .name = "TLBI_VAE1OS", .state = ARM_CP_STATE_AA64,
24
+ .opc0 = 1, .opc1 = 0, .crn = 8, .crm = 1, .opc2 = 1,
25
+ .access = PL1_W, .type = ARM_CP_NO_RAW,
26
+ .writefn = tlbi_aa64_vae1is_write },
27
{ .name = "TLBI_ASIDE1OS", .state = ARM_CP_STATE_AA64,
28
.opc0 = 1, .opc1 = 0, .crn = 8, .crm = 1, .opc2 = 2,
29
.access = PL1_W, .type = ARM_CP_NO_RAW,
30
.writefn = tlbi_aa64_vmalle1is_write },
31
+ { .name = "TLBI_VAAE1OS", .state = ARM_CP_STATE_AA64,
32
+ .opc0 = 1, .opc1 = 0, .crn = 8, .crm = 1, .opc2 = 3,
33
+ .access = PL1_W, .type = ARM_CP_NO_RAW,
34
+ .writefn = tlbi_aa64_vae1is_write },
35
+ { .name = "TLBI_VALE1OS", .state = ARM_CP_STATE_AA64,
36
+ .opc0 = 1, .opc1 = 0, .crn = 8, .crm = 1, .opc2 = 5,
37
+ .access = PL1_W, .type = ARM_CP_NO_RAW,
38
+ .writefn = tlbi_aa64_vae1is_write },
39
+ { .name = "TLBI_VAALE1OS", .state = ARM_CP_STATE_AA64,
40
+ .opc0 = 1, .opc1 = 0, .crn = 8, .crm = 1, .opc2 = 7,
41
+ .access = PL1_W, .type = ARM_CP_NO_RAW,
42
+ .writefn = tlbi_aa64_vae1is_write },
43
{ .name = "TLBI_ALLE2OS", .state = ARM_CP_STATE_AA64,
44
.opc0 = 1, .opc1 = 4, .crn = 8, .crm = 1, .opc2 = 0,
45
.access = PL2_W, .type = ARM_CP_NO_RAW,
46
.writefn = tlbi_aa64_alle2is_write },
47
+ { .name = "TLBI_VAE2OS", .state = ARM_CP_STATE_AA64,
48
+ .opc0 = 1, .opc1 = 4, .crn = 8, .crm = 1, .opc2 = 1,
49
+ .access = PL2_W, .type = ARM_CP_NO_RAW,
50
+ .writefn = tlbi_aa64_vae2is_write },
51
{ .name = "TLBI_ALLE1OS", .state = ARM_CP_STATE_AA64,
52
.opc0 = 1, .opc1 = 4, .crn = 8, .crm = 1, .opc2 = 4,
53
.access = PL2_W, .type = ARM_CP_NO_RAW,
54
.writefn = tlbi_aa64_alle1is_write },
55
+ { .name = "TLBI_VALE2OS", .state = ARM_CP_STATE_AA64,
56
+ .opc0 = 1, .opc1 = 4, .crn = 8, .crm = 1, .opc2 = 5,
57
+ .access = PL2_W, .type = ARM_CP_NO_RAW,
58
+ .writefn = tlbi_aa64_vae2is_write },
59
{ .name = "TLBI_VMALLS12E1OS", .state = ARM_CP_STATE_AA64,
60
.opc0 = 1, .opc1 = 4, .crn = 8, .crm = 1, .opc2 = 6,
61
.access = PL2_W, .type = ARM_CP_NO_RAW,
62
@@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo tlbios_reginfo[] = {
63
.opc0 = 1, .opc1 = 6, .crn = 8, .crm = 1, .opc2 = 0,
64
.access = PL3_W, .type = ARM_CP_NO_RAW,
65
.writefn = tlbi_aa64_alle3is_write },
66
+ { .name = "TLBI_VAE3OS", .state = ARM_CP_STATE_AA64,
67
+ .opc0 = 1, .opc1 = 6, .crn = 8, .crm = 1, .opc2 = 1,
68
+ .access = PL3_W, .type = ARM_CP_NO_RAW,
69
+ .writefn = tlbi_aa64_vae3is_write },
70
+ { .name = "TLBI_VALE3OS", .state = ARM_CP_STATE_AA64,
71
+ .opc0 = 1, .opc1 = 6, .crn = 8, .crm = 1, .opc2 = 5,
72
+ .access = PL3_W, .type = ARM_CP_NO_RAW,
73
+ .writefn = tlbi_aa64_vae3is_write },
74
REGINFO_SENTINEL
75
};
76
77
--
78
2.25.1
79
80
diff view generated by jsdifflib
New patch
1
The checks in the ITS on the rdbase values in guest commands are
2
off-by-one: they permit the guest to pass us a value equal to
3
s->gicv3->num_cpu, but the valid values are 0...num_cpu-1. This
4
meant the guest could cause us to index off the end of the
5
s->gicv3->cpu[] array when calling gicv3_redist_process_lpi(), and we
6
would probably crash.
1
7
8
(This is not a security bug, because this code is only usable
9
with emulation, not with KVM.)
10
11
Cc: qemu-stable@nongnu.org
12
Fixes: 17fb5e36aabd4b ("hw/intc: GICv3 redistributor ITS processing")
13
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
14
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
15
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
16
---
17
hw/intc/arm_gicv3_its.c | 4 ++--
18
1 file changed, 2 insertions(+), 2 deletions(-)
19
20
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
21
index XXXXXXX..XXXXXXX 100644
22
--- a/hw/intc/arm_gicv3_its.c
23
+++ b/hw/intc/arm_gicv3_its.c
24
@@ -XXX,XX +XXX,XX @@ static bool process_its_cmd(GICv3ITSState *s, uint64_t value, uint32_t offset,
25
*/
26
rdbase = (cte & GITS_CTE_RDBASE_PROCNUM_MASK) >> 1U;
27
28
- if (rdbase > s->gicv3->num_cpu) {
29
+ if (rdbase >= s->gicv3->num_cpu) {
30
return result;
31
}
32
33
@@ -XXX,XX +XXX,XX @@ static bool process_mapc(GICv3ITSState *s, uint32_t offset)
34
35
valid = (value & CMD_FIELD_VALID_MASK);
36
37
- if ((icid > s->ct.maxids.max_collids) || (rdbase > s->gicv3->num_cpu)) {
38
+ if ((icid > s->ct.maxids.max_collids) || (rdbase >= s->gicv3->num_cpu)) {
39
qemu_log_mask(LOG_GUEST_ERROR,
40
"ITS MAPC: invalid collection table attributes "
41
"icid %d rdbase %" PRIu64 "\n", icid, rdbase);
42
--
43
2.25.1
44
45
diff view generated by jsdifflib
New patch
1
We currently define a bitmask for the GITS_CTLR ENABLED bit in
2
two ways: as ITS_CTLR_ENABLED, and via the FIELD() macro as
3
R_GITS_CTLR_ENABLED_MASK. Consistently use the FIELD macro version
4
everywhere and remove the redundant ITS_CTLR_ENABLED define.
1
5
6
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
7
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
8
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
9
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
10
---
11
hw/intc/gicv3_internal.h | 2 --
12
hw/intc/arm_gicv3_its.c | 20 ++++++++++----------
13
2 files changed, 10 insertions(+), 12 deletions(-)
14
15
diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
16
index XXXXXXX..XXXXXXX 100644
17
--- a/hw/intc/gicv3_internal.h
18
+++ b/hw/intc/gicv3_internal.h
19
@@ -XXX,XX +XXX,XX @@ FIELD(GITS_TYPER, CIL, 36, 1)
20
21
#define GITS_IDREGS 0xFFD0
22
23
-#define ITS_CTLR_ENABLED (1U) /* ITS Enabled */
24
-
25
#define GITS_BASER_RO_MASK (R_GITS_BASER_ENTRYSIZE_MASK | \
26
R_GITS_BASER_TYPE_MASK)
27
28
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
29
index XXXXXXX..XXXXXXX 100644
30
--- a/hw/intc/arm_gicv3_its.c
31
+++ b/hw/intc/arm_gicv3_its.c
32
@@ -XXX,XX +XXX,XX @@ static void process_cmdq(GICv3ITSState *s)
33
uint8_t cmd;
34
int i;
35
36
- if (!(s->ctlr & ITS_CTLR_ENABLED)) {
37
+ if (!(s->ctlr & R_GITS_CTLR_ENABLED_MASK)) {
38
return;
39
}
40
41
@@ -XXX,XX +XXX,XX @@ static MemTxResult gicv3_its_translation_write(void *opaque, hwaddr offset,
42
43
switch (offset) {
44
case GITS_TRANSLATER:
45
- if (s->ctlr & ITS_CTLR_ENABLED) {
46
+ if (s->ctlr & R_GITS_CTLR_ENABLED_MASK) {
47
devid = attrs.requester_id;
48
result = process_its_cmd(s, data, devid, NONE);
49
}
50
@@ -XXX,XX +XXX,XX @@ static bool its_writel(GICv3ITSState *s, hwaddr offset,
51
switch (offset) {
52
case GITS_CTLR:
53
if (value & R_GITS_CTLR_ENABLED_MASK) {
54
- s->ctlr |= ITS_CTLR_ENABLED;
55
+ s->ctlr |= R_GITS_CTLR_ENABLED_MASK;
56
extract_table_params(s);
57
extract_cmdq_params(s);
58
s->creadr = 0;
59
process_cmdq(s);
60
} else {
61
- s->ctlr &= ~ITS_CTLR_ENABLED;
62
+ s->ctlr &= ~R_GITS_CTLR_ENABLED_MASK;
63
}
64
break;
65
case GITS_CBASER:
66
@@ -XXX,XX +XXX,XX @@ static bool its_writel(GICv3ITSState *s, hwaddr offset,
67
* IMPDEF choice:- GITS_CBASER register becomes RO if ITS is
68
* already enabled
69
*/
70
- if (!(s->ctlr & ITS_CTLR_ENABLED)) {
71
+ if (!(s->ctlr & R_GITS_CTLR_ENABLED_MASK)) {
72
s->cbaser = deposit64(s->cbaser, 0, 32, value);
73
s->creadr = 0;
74
s->cwriter = s->creadr;
75
@@ -XXX,XX +XXX,XX @@ static bool its_writel(GICv3ITSState *s, hwaddr offset,
76
* IMPDEF choice:- GITS_CBASER register becomes RO if ITS is
77
* already enabled
78
*/
79
- if (!(s->ctlr & ITS_CTLR_ENABLED)) {
80
+ if (!(s->ctlr & R_GITS_CTLR_ENABLED_MASK)) {
81
s->cbaser = deposit64(s->cbaser, 32, 32, value);
82
s->creadr = 0;
83
s->cwriter = s->creadr;
84
@@ -XXX,XX +XXX,XX @@ static bool its_writel(GICv3ITSState *s, hwaddr offset,
85
* IMPDEF choice:- GITS_BASERn register becomes RO if ITS is
86
* already enabled
87
*/
88
- if (!(s->ctlr & ITS_CTLR_ENABLED)) {
89
+ if (!(s->ctlr & R_GITS_CTLR_ENABLED_MASK)) {
90
index = (offset - GITS_BASER) / 8;
91
92
if (offset & 7) {
93
@@ -XXX,XX +XXX,XX @@ static bool its_writell(GICv3ITSState *s, hwaddr offset,
94
* IMPDEF choice:- GITS_BASERn register becomes RO if ITS is
95
* already enabled
96
*/
97
- if (!(s->ctlr & ITS_CTLR_ENABLED)) {
98
+ if (!(s->ctlr & R_GITS_CTLR_ENABLED_MASK)) {
99
index = (offset - GITS_BASER) / 8;
100
s->baser[index] &= GITS_BASER_RO_MASK;
101
s->baser[index] |= (value & ~GITS_BASER_RO_MASK);
102
@@ -XXX,XX +XXX,XX @@ static bool its_writell(GICv3ITSState *s, hwaddr offset,
103
* IMPDEF choice:- GITS_CBASER register becomes RO if ITS is
104
* already enabled
105
*/
106
- if (!(s->ctlr & ITS_CTLR_ENABLED)) {
107
+ if (!(s->ctlr & R_GITS_CTLR_ENABLED_MASK)) {
108
s->cbaser = value;
109
s->creadr = 0;
110
s->cwriter = s->creadr;
111
@@ -XXX,XX +XXX,XX @@ static void gicv3_its_reset(DeviceState *dev)
112
113
static void gicv3_its_post_load(GICv3ITSState *s)
114
{
115
- if (s->ctlr & ITS_CTLR_ENABLED) {
116
+ if (s->ctlr & R_GITS_CTLR_ENABLED_MASK) {
117
extract_table_params(s);
118
extract_cmdq_params(s);
119
}
120
--
121
2.25.1
122
123
diff view generated by jsdifflib
New patch
1
The TableDesc struct defines properties of the in-guest-memory tables
2
which the guest tells us about by writing to the GITS_BASER<n>
3
registers. This struct currently has a union 'maxids', but all the
4
fields of the union have the same type (uint32_t) and do the same
5
thing (record one-greater-than the maximum ID value that can be used
6
as an index into the table).
1
7
8
We're about to add another table type (the GICv4 vPE table); rather
9
than adding another specifically-named union field for that table
10
type with the same type as the other union fields, remove the union
11
entirely and just have a 'uint32_t max_ids' struct field.
12
13
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
14
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
15
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
16
---
17
include/hw/intc/arm_gicv3_its_common.h | 5 +----
18
hw/intc/arm_gicv3_its.c | 20 ++++++++++----------
19
2 files changed, 11 insertions(+), 14 deletions(-)
20
21
diff --git a/include/hw/intc/arm_gicv3_its_common.h b/include/hw/intc/arm_gicv3_its_common.h
22
index XXXXXXX..XXXXXXX 100644
23
--- a/include/hw/intc/arm_gicv3_its_common.h
24
+++ b/include/hw/intc/arm_gicv3_its_common.h
25
@@ -XXX,XX +XXX,XX @@ typedef struct {
26
uint16_t entry_sz;
27
uint32_t page_sz;
28
uint32_t max_entries;
29
- union {
30
- uint32_t max_devids;
31
- uint32_t max_collids;
32
- } maxids;
33
+ uint32_t max_ids;
34
uint64_t base_addr;
35
} TableDesc;
36
37
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
38
index XXXXXXX..XXXXXXX 100644
39
--- a/hw/intc/arm_gicv3_its.c
40
+++ b/hw/intc/arm_gicv3_its.c
41
@@ -XXX,XX +XXX,XX @@ static bool process_its_cmd(GICv3ITSState *s, uint64_t value, uint32_t offset,
42
* In this implementation, in case of guest errors we ignore the
43
* command and move onto the next command in the queue.
44
*/
45
- if (devid > s->dt.maxids.max_devids) {
46
+ if (devid > s->dt.max_ids) {
47
qemu_log_mask(LOG_GUEST_ERROR,
48
"%s: invalid command attributes: devid %d>%d",
49
- __func__, devid, s->dt.maxids.max_devids);
50
+ __func__, devid, s->dt.max_ids);
51
52
} else if (!dte_valid || !ite_valid || !cte_valid) {
53
qemu_log_mask(LOG_GUEST_ERROR,
54
@@ -XXX,XX +XXX,XX @@ static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset,
55
max_Intid = (1ULL << (GICD_TYPER_IDBITS + 1)) - 1;
56
}
57
58
- if ((devid > s->dt.maxids.max_devids) || (icid > s->ct.maxids.max_collids)
59
+ if ((devid > s->dt.max_ids) || (icid > s->ct.max_ids)
60
|| !dte_valid || (eventid > max_eventid) ||
61
(!ignore_pInt && (((pIntid < GICV3_LPI_INTID_START) ||
62
(pIntid > max_Intid)) && (pIntid != INTID_SPURIOUS)))) {
63
@@ -XXX,XX +XXX,XX @@ static bool process_mapc(GICv3ITSState *s, uint32_t offset)
64
65
valid = (value & CMD_FIELD_VALID_MASK);
66
67
- if ((icid > s->ct.maxids.max_collids) || (rdbase >= s->gicv3->num_cpu)) {
68
+ if ((icid > s->ct.max_ids) || (rdbase >= s->gicv3->num_cpu)) {
69
qemu_log_mask(LOG_GUEST_ERROR,
70
"ITS MAPC: invalid collection table attributes "
71
"icid %d rdbase %" PRIu64 "\n", icid, rdbase);
72
@@ -XXX,XX +XXX,XX @@ static bool process_mapd(GICv3ITSState *s, uint64_t value, uint32_t offset)
73
74
valid = (value & CMD_FIELD_VALID_MASK);
75
76
- if ((devid > s->dt.maxids.max_devids) ||
77
+ if ((devid > s->dt.max_ids) ||
78
(size > FIELD_EX64(s->typer, GITS_TYPER, IDBITS))) {
79
qemu_log_mask(LOG_GUEST_ERROR,
80
"ITS MAPD: invalid device table attributes "
81
@@ -XXX,XX +XXX,XX @@ static void extract_table_params(GICv3ITSState *s)
82
(page_sz / s->dt.entry_sz));
83
}
84
85
- s->dt.maxids.max_devids = (1UL << (FIELD_EX64(s->typer, GITS_TYPER,
86
- DEVBITS) + 1));
87
+ s->dt.max_ids = (1UL << (FIELD_EX64(s->typer, GITS_TYPER,
88
+ DEVBITS) + 1));
89
90
s->dt.base_addr = baser_base_addr(value, page_sz);
91
92
@@ -XXX,XX +XXX,XX @@ static void extract_table_params(GICv3ITSState *s)
93
}
94
95
if (FIELD_EX64(s->typer, GITS_TYPER, CIL)) {
96
- s->ct.maxids.max_collids = (1UL << (FIELD_EX64(s->typer,
97
- GITS_TYPER, CIDBITS) + 1));
98
+ s->ct.max_ids = (1UL << (FIELD_EX64(s->typer,
99
+ GITS_TYPER, CIDBITS) + 1));
100
} else {
101
/* 16-bit CollectionId supported when CIL == 0 */
102
- s->ct.maxids.max_collids = (1UL << 16);
103
+ s->ct.max_ids = (1UL << 16);
104
}
105
106
s->ct.base_addr = baser_base_addr(value, page_sz);
107
--
108
2.25.1
109
110
diff view generated by jsdifflib
New patch
1
In extract_table_params() we process each GITS_BASER<n> register. If
2
the register's Valid bit is not set, this means there is no
3
in-guest-memory table and so we should not try to interpret the other
4
fields in the register. This was incorrectly coded as a 'return'
5
rather than a 'break', so instead of looping round to process the
6
next GITS_BASER<n> we would stop entirely, treating any later tables
7
as being not valid also.
1
8
9
This has no real guest-visible effects because (since we don't have
10
GITS_TYPER.HCC != 0) the guest must in any case set up all the
11
GITS_BASER<n> to point to valid tables, so this only happens in an
12
odd misbehaving-guest corner case.
13
14
Fix the check to 'break', so that we leave the case statement and
15
loop back around to the next GITS_BASER<n>.
16
17
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
18
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
19
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
20
---
21
hw/intc/arm_gicv3_its.c | 4 ++--
22
1 file changed, 2 insertions(+), 2 deletions(-)
23
24
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
25
index XXXXXXX..XXXXXXX 100644
26
--- a/hw/intc/arm_gicv3_its.c
27
+++ b/hw/intc/arm_gicv3_its.c
28
@@ -XXX,XX +XXX,XX @@ static void extract_table_params(GICv3ITSState *s)
29
s->dt.valid = FIELD_EX64(value, GITS_BASER, VALID);
30
31
if (!s->dt.valid) {
32
- return;
33
+ break;
34
}
35
36
s->dt.page_sz = page_sz;
37
@@ -XXX,XX +XXX,XX @@ static void extract_table_params(GICv3ITSState *s)
38
* hence writes are discarded if ct.valid is 0
39
*/
40
if (!s->ct.valid) {
41
- return;
42
+ break;
43
}
44
45
s->ct.page_sz = page_sz;
46
--
47
2.25.1
48
49
diff view generated by jsdifflib
New patch
1
The extract_table_params() decodes the fields in the GITS_BASER<n>
2
registers into TableDesc structs. Since the fields are the same for
3
all the GITS_BASER<n> registers, there is currently a lot of code
4
duplication within the switch (type) statement. Refactor so that the
5
cases include only what is genuinely different for each type:
6
the calculation of the number of bits in the ID value that indexes
7
into the table.
1
8
9
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
10
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
11
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
12
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
13
---
14
hw/intc/arm_gicv3_its.c | 97 +++++++++++++++++------------------------
15
1 file changed, 40 insertions(+), 57 deletions(-)
16
17
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
18
index XXXXXXX..XXXXXXX 100644
19
--- a/hw/intc/arm_gicv3_its.c
20
+++ b/hw/intc/arm_gicv3_its.c
21
@@ -XXX,XX +XXX,XX @@ static void extract_table_params(GICv3ITSState *s)
22
uint64_t value;
23
24
for (int i = 0; i < 8; i++) {
25
+ TableDesc *td;
26
+ int idbits;
27
+
28
value = s->baser[i];
29
30
if (!value) {
31
@@ -XXX,XX +XXX,XX @@ static void extract_table_params(GICv3ITSState *s)
32
type = FIELD_EX64(value, GITS_BASER, TYPE);
33
34
switch (type) {
35
-
36
case GITS_BASER_TYPE_DEVICE:
37
- memset(&s->dt, 0 , sizeof(s->dt));
38
- s->dt.valid = FIELD_EX64(value, GITS_BASER, VALID);
39
-
40
- if (!s->dt.valid) {
41
- break;
42
- }
43
-
44
- s->dt.page_sz = page_sz;
45
- s->dt.indirect = FIELD_EX64(value, GITS_BASER, INDIRECT);
46
- s->dt.entry_sz = FIELD_EX64(value, GITS_BASER, ENTRYSIZE);
47
-
48
- if (!s->dt.indirect) {
49
- s->dt.max_entries = (num_pages * page_sz) / s->dt.entry_sz;
50
- } else {
51
- s->dt.max_entries = (((num_pages * page_sz) /
52
- L1TABLE_ENTRY_SIZE) *
53
- (page_sz / s->dt.entry_sz));
54
- }
55
-
56
- s->dt.max_ids = (1UL << (FIELD_EX64(s->typer, GITS_TYPER,
57
- DEVBITS) + 1));
58
-
59
- s->dt.base_addr = baser_base_addr(value, page_sz);
60
-
61
+ td = &s->dt;
62
+ idbits = FIELD_EX64(s->typer, GITS_TYPER, DEVBITS) + 1;
63
break;
64
-
65
case GITS_BASER_TYPE_COLLECTION:
66
- memset(&s->ct, 0 , sizeof(s->ct));
67
- s->ct.valid = FIELD_EX64(value, GITS_BASER, VALID);
68
-
69
- /*
70
- * GITS_TYPER.HCC is 0 for this implementation
71
- * hence writes are discarded if ct.valid is 0
72
- */
73
- if (!s->ct.valid) {
74
- break;
75
- }
76
-
77
- s->ct.page_sz = page_sz;
78
- s->ct.indirect = FIELD_EX64(value, GITS_BASER, INDIRECT);
79
- s->ct.entry_sz = FIELD_EX64(value, GITS_BASER, ENTRYSIZE);
80
-
81
- if (!s->ct.indirect) {
82
- s->ct.max_entries = (num_pages * page_sz) / s->ct.entry_sz;
83
- } else {
84
- s->ct.max_entries = (((num_pages * page_sz) /
85
- L1TABLE_ENTRY_SIZE) *
86
- (page_sz / s->ct.entry_sz));
87
- }
88
-
89
+ td = &s->ct;
90
if (FIELD_EX64(s->typer, GITS_TYPER, CIL)) {
91
- s->ct.max_ids = (1UL << (FIELD_EX64(s->typer,
92
- GITS_TYPER, CIDBITS) + 1));
93
+ idbits = FIELD_EX64(s->typer, GITS_TYPER, CIDBITS) + 1;
94
} else {
95
/* 16-bit CollectionId supported when CIL == 0 */
96
- s->ct.max_ids = (1UL << 16);
97
+ idbits = 16;
98
}
99
-
100
- s->ct.base_addr = baser_base_addr(value, page_sz);
101
-
102
break;
103
-
104
default:
105
- break;
106
+ /*
107
+ * GITS_BASER<n>.TYPE is read-only, so GITS_BASER_RO_MASK
108
+ * ensures we will only see type values corresponding to
109
+ * the values set up in gicv3_its_reset().
110
+ */
111
+ g_assert_not_reached();
112
}
113
+
114
+ memset(td, 0, sizeof(*td));
115
+ td->valid = FIELD_EX64(value, GITS_BASER, VALID);
116
+ /*
117
+ * If GITS_BASER<n>.Valid is 0 for any <n> then we will not process
118
+ * interrupts. (GITS_TYPER.HCC is 0 for this implementation, so we
119
+ * do not have a special case where the GITS_BASER<n>.Valid bit is 0
120
+ * for the register corresponding to the Collection table but we
121
+ * still have to process interrupts using non-memory-backed
122
+ * Collection table entries.)
123
+ */
124
+ if (!td->valid) {
125
+ continue;
126
+ }
127
+ td->page_sz = page_sz;
128
+ td->indirect = FIELD_EX64(value, GITS_BASER, INDIRECT);
129
+ td->entry_sz = FIELD_EX64(value, GITS_BASER, ENTRYSIZE);
130
+ td->base_addr = baser_base_addr(value, page_sz);
131
+ if (!td->indirect) {
132
+ td->max_entries = (num_pages * page_sz) / td->entry_sz;
133
+ } else {
134
+ td->max_entries = (((num_pages * page_sz) /
135
+ L1TABLE_ENTRY_SIZE) *
136
+ (page_sz / td->entry_sz));
137
+ }
138
+ td->max_ids = 1ULL << idbits;
139
}
140
}
141
142
--
143
2.25.1
144
145
diff view generated by jsdifflib
New patch
1
We set the TableDesc entry_sz field from the appropriate
2
GITS_BASER.ENTRYSIZE field. That ID register field specifies the
3
number of bytes per table entry minus one. However when we use
4
td->entry_sz we assume it to be the number of bytes per table entry
5
(for instance we calculate the number of entries in a page by
6
dividing the page size by the entry size).
1
7
8
The effects of this bug are:
9
* we miscalculate the maximum number of entries in the table,
10
so our checks on guest index values are wrong (too lax)
11
* when looking up an entry in the second level of an indirect
12
table, we calculate an incorrect index into the L2 table.
13
Because we make the same incorrect calculation on both
14
reads and writes of the L2 table, the guest won't notice
15
unless it's unlucky enough to use an index value that
16
causes us to index off the end of the L2 table page and
17
cause guest memory corruption in whatever follows
18
19
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
20
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
21
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
22
---
23
hw/intc/arm_gicv3_its.c | 2 +-
24
1 file changed, 1 insertion(+), 1 deletion(-)
25
26
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
27
index XXXXXXX..XXXXXXX 100644
28
--- a/hw/intc/arm_gicv3_its.c
29
+++ b/hw/intc/arm_gicv3_its.c
30
@@ -XXX,XX +XXX,XX @@ static void extract_table_params(GICv3ITSState *s)
31
}
32
td->page_sz = page_sz;
33
td->indirect = FIELD_EX64(value, GITS_BASER, INDIRECT);
34
- td->entry_sz = FIELD_EX64(value, GITS_BASER, ENTRYSIZE);
35
+ td->entry_sz = FIELD_EX64(value, GITS_BASER, ENTRYSIZE) + 1;
36
td->base_addr = baser_base_addr(value, page_sz);
37
if (!td->indirect) {
38
td->max_entries = (num_pages * page_sz) / td->entry_sz;
39
--
40
2.25.1
41
42
diff view generated by jsdifflib
New patch
1
The GITS_TYPE_PHYSICAL define is the value we set the
2
GITS_TYPER.Physical field to -- this is 1 to indicate that we support
3
physical LPIs. (Support for virtual LPIs is the GITS_TYPER.Virtual
4
field.) We also use this define as the *value* that we write into an
5
interrupt translation table entry's INTTYPE field, which should be 1
6
for a physical interrupt and 0 for a virtual interrupt. Finally, we
7
use it as a *mask* when we read the interrupt translation table entry
8
INTTYPE field.
1
9
10
Untangle this confusion: define an ITE_INTTYPE_VIRTUAL and
11
ITE_INTTYPE_PHYSICAL to be the valid values of the ITE INTTYPE
12
field, and replace the ad-hoc collection of ITE_ENTRY_* defines with
13
use of the FIELD() macro to define the fields of an ITE and the
14
FIELD_EX64() and FIELD_DP64() macros to read and write them.
15
We use ITE in the new setup, rather than ITE_ENTRY, because
16
ITE stands for "Interrupt translation entry" and so the extra
17
"entry" would be redundant.
18
19
We take the opportunity to correct the name of the field that holds
20
the GICv4 'doorbell' interrupt ID (this is always the value 1023 in a
21
GICv3, which is why we were calling it the 'spurious' field).
22
23
The GITS_TYPE_PHYSICAL define is then used in only one place, where
24
we set the initial GITS_TYPER value. Since GITS_TYPER.Physical is
25
essentially a boolean, hiding the '1' value behind a macro is more
26
confusing than helpful, so expand out the macro there and remove the
27
define entirely.
28
29
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
30
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
31
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
32
---
33
hw/intc/gicv3_internal.h | 26 ++++++++++++++------------
34
hw/intc/arm_gicv3_its.c | 30 +++++++++++++-----------------
35
2 files changed, 27 insertions(+), 29 deletions(-)
36
37
diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
38
index XXXXXXX..XXXXXXX 100644
39
--- a/hw/intc/gicv3_internal.h
40
+++ b/hw/intc/gicv3_internal.h
41
@@ -XXX,XX +XXX,XX @@ FIELD(MAPC, RDBASE, 16, 32)
42
#define L2_TABLE_VALID_MASK CMD_FIELD_VALID_MASK
43
#define TABLE_ENTRY_VALID_MASK (1ULL << 0)
44
45
-/**
46
- * Default features advertised by this version of ITS
47
- */
48
-/* Physical LPIs supported */
49
-#define GITS_TYPE_PHYSICAL (1U << 0)
50
-
51
/*
52
* 12 bytes Interrupt translation Table Entry size
53
* as per Table 5.3 in GICv3 spec
54
* ITE Lower 8 Bytes
55
* Bits: | 49 ... 26 | 25 ... 2 | 1 | 0 |
56
- * Values: | 1023 | IntNum | IntType | Valid |
57
+ * Values: | Doorbell | IntNum | IntType | Valid |
58
* ITE Higher 4 Bytes
59
* Bits: | 31 ... 16 | 15 ...0 |
60
* Values: | vPEID | ICID |
61
+ * (When Doorbell is unused, as it always is in GICv3, it is 1023)
62
*/
63
#define ITS_ITT_ENTRY_SIZE 0xC
64
-#define ITE_ENTRY_INTTYPE_SHIFT 1
65
-#define ITE_ENTRY_INTID_SHIFT 2
66
-#define ITE_ENTRY_INTID_MASK MAKE_64BIT_MASK(2, 24)
67
-#define ITE_ENTRY_INTSP_SHIFT 26
68
-#define ITE_ENTRY_ICID_MASK MAKE_64BIT_MASK(0, 16)
69
+
70
+FIELD(ITE_L, VALID, 0, 1)
71
+FIELD(ITE_L, INTTYPE, 1, 1)
72
+FIELD(ITE_L, INTID, 2, 24)
73
+FIELD(ITE_L, DOORBELL, 26, 24)
74
+
75
+FIELD(ITE_H, ICID, 0, 16)
76
+FIELD(ITE_H, VPEID, 16, 16)
77
+
78
+/* Possible values for ITE_L INTTYPE */
79
+#define ITE_INTTYPE_VIRTUAL 0
80
+#define ITE_INTTYPE_PHYSICAL 1
81
82
/* 16 bits EventId */
83
#define ITS_IDBITS GICD_TYPER_IDBITS
84
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
85
index XXXXXXX..XXXXXXX 100644
86
--- a/hw/intc/arm_gicv3_its.c
87
+++ b/hw/intc/arm_gicv3_its.c
88
@@ -XXX,XX +XXX,XX @@ static bool get_ite(GICv3ITSState *s, uint32_t eventid, uint64_t dte,
89
MEMTXATTRS_UNSPECIFIED, res);
90
91
if (*res == MEMTX_OK) {
92
- if (ite.itel & TABLE_ENTRY_VALID_MASK) {
93
- if ((ite.itel >> ITE_ENTRY_INTTYPE_SHIFT) &
94
- GITS_TYPE_PHYSICAL) {
95
- *pIntid = (ite.itel & ITE_ENTRY_INTID_MASK) >>
96
- ITE_ENTRY_INTID_SHIFT;
97
- *icid = ite.iteh & ITE_ENTRY_ICID_MASK;
98
+ if (FIELD_EX64(ite.itel, ITE_L, VALID)) {
99
+ int inttype = FIELD_EX64(ite.itel, ITE_L, INTTYPE);
100
+ if (inttype == ITE_INTTYPE_PHYSICAL) {
101
+ *pIntid = FIELD_EX64(ite.itel, ITE_L, INTID);
102
+ *icid = FIELD_EX32(ite.iteh, ITE_H, ICID);
103
status = true;
104
}
105
}
106
@@ -XXX,XX +XXX,XX @@ static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset,
107
MemTxResult res = MEMTX_OK;
108
uint16_t icid = 0;
109
uint64_t dte = 0;
110
- IteEntry ite;
111
- uint32_t int_spurious = INTID_SPURIOUS;
112
bool result = false;
113
114
devid = ((value & DEVID_MASK) >> DEVID_SHIFT);
115
@@ -XXX,XX +XXX,XX @@ static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset,
116
*/
117
} else {
118
/* add ite entry to interrupt translation table */
119
- ite.itel = (dte_valid & TABLE_ENTRY_VALID_MASK) |
120
- (GITS_TYPE_PHYSICAL << ITE_ENTRY_INTTYPE_SHIFT);
121
-
122
+ IteEntry ite = {};
123
+ ite.itel = FIELD_DP64(ite.itel, ITE_L, VALID, dte_valid);
124
+ ite.itel = FIELD_DP64(ite.itel, ITE_L, INTTYPE, ITE_INTTYPE_PHYSICAL);
125
if (ignore_pInt) {
126
- ite.itel |= (eventid << ITE_ENTRY_INTID_SHIFT);
127
+ ite.itel = FIELD_DP64(ite.itel, ITE_L, INTID, eventid);
128
} else {
129
- ite.itel |= (pIntid << ITE_ENTRY_INTID_SHIFT);
130
+ ite.itel = FIELD_DP64(ite.itel, ITE_L, INTID, pIntid);
131
}
132
- ite.itel |= (int_spurious << ITE_ENTRY_INTSP_SHIFT);
133
- ite.iteh = icid;
134
+ ite.itel = FIELD_DP64(ite.itel, ITE_L, DOORBELL, INTID_SPURIOUS);
135
+ ite.iteh = FIELD_DP32(ite.iteh, ITE_H, ICID, icid);
136
137
result = update_ite(s, eventid, dte, ite);
138
}
139
@@ -XXX,XX +XXX,XX @@ static void gicv3_arm_its_realize(DeviceState *dev, Error **errp)
140
"gicv3-its-sysmem");
141
142
/* set the ITS default features supported */
143
- s->typer = FIELD_DP64(s->typer, GITS_TYPER, PHYSICAL,
144
- GITS_TYPE_PHYSICAL);
145
+ s->typer = FIELD_DP64(s->typer, GITS_TYPER, PHYSICAL, 1);
146
s->typer = FIELD_DP64(s->typer, GITS_TYPER, ITT_ENTRY_SIZE,
147
ITS_ITT_ENTRY_SIZE - 1);
148
s->typer = FIELD_DP64(s->typer, GITS_TYPER, IDBITS, ITS_IDBITS);
149
--
150
2.25.1
151
152
diff view generated by jsdifflib
New patch
1
The MAPI command takes arguments DeviceID, EventID, ICID, and is
2
defined to be equivalent to MAPTI DeviceID, EventID, EventID, ICID.
3
(That is, where MAPTI takes an explicit pINTID, MAPI uses the EventID
4
as the pINTID.)
1
5
6
We didn't quite get this right. In particular the error checks for
7
MAPI include "EventID does not specify a valid LPI identifier", which
8
is the same as MAPTI's error check for the pINTID field. QEMU's code
9
skips the pINTID error check entirely in the MAPI case.
10
11
We can fix this bug and in the process simplify the code by switching
12
to the obvious implementation of setting pIntid = eventid early
13
if ignore_pInt is true.
14
15
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
16
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
17
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
18
---
19
hw/intc/arm_gicv3_its.c | 18 +++++++-----------
20
1 file changed, 7 insertions(+), 11 deletions(-)
21
22
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
23
index XXXXXXX..XXXXXXX 100644
24
--- a/hw/intc/arm_gicv3_its.c
25
+++ b/hw/intc/arm_gicv3_its.c
26
@@ -XXX,XX +XXX,XX @@ static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset,
27
28
eventid = (value & EVENTID_MASK);
29
30
- if (!ignore_pInt) {
31
+ if (ignore_pInt) {
32
+ pIntid = eventid;
33
+ } else {
34
pIntid = ((value & pINTID_MASK) >> pINTID_SHIFT);
35
}
36
37
@@ -XXX,XX +XXX,XX @@ static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset,
38
39
max_eventid = (1UL << (((dte >> 1U) & SIZE_MASK) + 1));
40
41
- if (!ignore_pInt) {
42
- max_Intid = (1ULL << (GICD_TYPER_IDBITS + 1)) - 1;
43
- }
44
+ max_Intid = (1ULL << (GICD_TYPER_IDBITS + 1)) - 1;
45
46
if ((devid > s->dt.max_ids) || (icid > s->ct.max_ids)
47
|| !dte_valid || (eventid > max_eventid) ||
48
- (!ignore_pInt && (((pIntid < GICV3_LPI_INTID_START) ||
49
- (pIntid > max_Intid)) && (pIntid != INTID_SPURIOUS)))) {
50
+ (((pIntid < GICV3_LPI_INTID_START) || (pIntid > max_Intid)) &&
51
+ (pIntid != INTID_SPURIOUS))) {
52
qemu_log_mask(LOG_GUEST_ERROR,
53
"%s: invalid command attributes "
54
"devid %d or icid %d or eventid %d or pIntid %d or"
55
@@ -XXX,XX +XXX,XX @@ static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset,
56
IteEntry ite = {};
57
ite.itel = FIELD_DP64(ite.itel, ITE_L, VALID, dte_valid);
58
ite.itel = FIELD_DP64(ite.itel, ITE_L, INTTYPE, ITE_INTTYPE_PHYSICAL);
59
- if (ignore_pInt) {
60
- ite.itel = FIELD_DP64(ite.itel, ITE_L, INTID, eventid);
61
- } else {
62
- ite.itel = FIELD_DP64(ite.itel, ITE_L, INTID, pIntid);
63
- }
64
+ ite.itel = FIELD_DP64(ite.itel, ITE_L, INTID, pIntid);
65
ite.itel = FIELD_DP64(ite.itel, ITE_L, DOORBELL, INTID_SPURIOUS);
66
ite.iteh = FIELD_DP32(ite.iteh, ITE_H, ICID, icid);
67
68
--
69
2.25.1
70
71
diff view generated by jsdifflib
New patch
1
Currently the ITS code that reads and writes DTEs uses open-coded
2
shift-and-mask to assemble the various fields into the 64-bit DTE
3
word. The names of the macros used for mask and shift values are
4
also somewhat inconsistent, and don't follow our usual convention
5
that a MASK macro should specify the bits in their place in the word.
6
Replace all these with use of the FIELD macro.
1
7
8
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
9
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
10
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
11
---
12
hw/intc/gicv3_internal.h | 7 ++++---
13
hw/intc/arm_gicv3_its.c | 20 +++++++++-----------
14
2 files changed, 13 insertions(+), 14 deletions(-)
15
16
diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
17
index XXXXXXX..XXXXXXX 100644
18
--- a/hw/intc/gicv3_internal.h
19
+++ b/hw/intc/gicv3_internal.h
20
@@ -XXX,XX +XXX,XX @@ FIELD(ITE_H, VPEID, 16, 16)
21
* Valid = 1 bit,ITTAddr = 44 bits,Size = 5 bits
22
*/
23
#define GITS_DTE_SIZE (0x8ULL)
24
-#define GITS_DTE_ITTADDR_SHIFT 6
25
-#define GITS_DTE_ITTADDR_MASK MAKE_64BIT_MASK(GITS_DTE_ITTADDR_SHIFT, \
26
- ITTADDR_LENGTH)
27
+
28
+FIELD(DTE, VALID, 0, 1)
29
+FIELD(DTE, SIZE, 1, 5)
30
+FIELD(DTE, ITTADDR, 6, 44)
31
32
/*
33
* 8 bytes Collection Table Entry size
34
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
35
index XXXXXXX..XXXXXXX 100644
36
--- a/hw/intc/arm_gicv3_its.c
37
+++ b/hw/intc/arm_gicv3_its.c
38
@@ -XXX,XX +XXX,XX @@ static bool update_ite(GICv3ITSState *s, uint32_t eventid, uint64_t dte,
39
uint64_t itt_addr;
40
MemTxResult res = MEMTX_OK;
41
42
- itt_addr = (dte & GITS_DTE_ITTADDR_MASK) >> GITS_DTE_ITTADDR_SHIFT;
43
+ itt_addr = FIELD_EX64(dte, DTE, ITTADDR);
44
itt_addr <<= ITTADDR_SHIFT; /* 256 byte aligned */
45
46
address_space_stq_le(as, itt_addr + (eventid * (sizeof(uint64_t) +
47
@@ -XXX,XX +XXX,XX @@ static bool get_ite(GICv3ITSState *s, uint32_t eventid, uint64_t dte,
48
bool status = false;
49
IteEntry ite = {};
50
51
- itt_addr = (dte & GITS_DTE_ITTADDR_MASK) >> GITS_DTE_ITTADDR_SHIFT;
52
+ itt_addr = FIELD_EX64(dte, DTE, ITTADDR);
53
itt_addr <<= ITTADDR_SHIFT; /* 256 byte aligned */
54
55
ite.itel = address_space_ldq_le(as, itt_addr +
56
@@ -XXX,XX +XXX,XX @@ static bool process_its_cmd(GICv3ITSState *s, uint64_t value, uint32_t offset,
57
if (res != MEMTX_OK) {
58
return result;
59
}
60
- dte_valid = dte & TABLE_ENTRY_VALID_MASK;
61
+ dte_valid = FIELD_EX64(dte, DTE, VALID);
62
63
if (dte_valid) {
64
- max_eventid = (1UL << (((dte >> 1U) & SIZE_MASK) + 1));
65
+ max_eventid = 1UL << (FIELD_EX64(dte, DTE, SIZE) + 1);
66
67
ite_valid = get_ite(s, eventid, dte, &icid, &pIntid, &res);
68
69
@@ -XXX,XX +XXX,XX @@ static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset,
70
if (res != MEMTX_OK) {
71
return result;
72
}
73
- dte_valid = dte & TABLE_ENTRY_VALID_MASK;
74
-
75
- max_eventid = (1UL << (((dte >> 1U) & SIZE_MASK) + 1));
76
-
77
+ dte_valid = FIELD_EX64(dte, DTE, VALID);
78
+ max_eventid = 1UL << (FIELD_EX64(dte, DTE, SIZE) + 1);
79
max_Intid = (1ULL << (GICD_TYPER_IDBITS + 1)) - 1;
80
81
if ((devid > s->dt.max_ids) || (icid > s->ct.max_ids)
82
@@ -XXX,XX +XXX,XX @@ static bool update_dte(GICv3ITSState *s, uint32_t devid, bool valid,
83
if (s->dt.valid) {
84
if (valid) {
85
/* add mapping entry to device table */
86
- dte = (valid & TABLE_ENTRY_VALID_MASK) |
87
- ((size & SIZE_MASK) << 1U) |
88
- (itt_addr << GITS_DTE_ITTADDR_SHIFT);
89
+ dte = FIELD_DP64(dte, DTE, VALID, 1);
90
+ dte = FIELD_DP64(dte, DTE, SIZE, size);
91
+ dte = FIELD_DP64(dte, DTE, ITTADDR, itt_addr);
92
}
93
} else {
94
return true;
95
--
96
2.25.1
97
98
diff view generated by jsdifflib
New patch
1
The comment says that in our CTE format the RDBase field is 36 bits;
2
in fact for us it is only 16 bits, because we use the RDBase format
3
where it specifies a 16-bit CPU number. The code already uses
4
RDBASE_PROCNUM_LENGTH (16) as the field width, so fix the comment
5
to match it.
1
6
7
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
8
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
9
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
10
---
11
hw/intc/gicv3_internal.h | 2 +-
12
1 file changed, 1 insertion(+), 1 deletion(-)
13
14
diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
15
index XXXXXXX..XXXXXXX 100644
16
--- a/hw/intc/gicv3_internal.h
17
+++ b/hw/intc/gicv3_internal.h
18
@@ -XXX,XX +XXX,XX @@ FIELD(DTE, ITTADDR, 6, 44)
19
20
/*
21
* 8 bytes Collection Table Entry size
22
- * Valid = 1 bit,RDBase = 36 bits(considering max RDBASE)
23
+ * Valid = 1 bit, RDBase = 16 bits
24
*/
25
#define GITS_CTE_SIZE (0x8ULL)
26
#define GITS_CTE_RDBASE_PROCNUM_MASK MAKE_64BIT_MASK(1, RDBASE_PROCNUM_LENGTH)
27
--
28
2.25.1
29
30
diff view generated by jsdifflib
New patch
1
Use FIELD macros to handle CTEs, rather than ad-hoc mask-and-shift.
1
2
3
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
4
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
5
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
6
---
7
hw/intc/gicv3_internal.h | 3 ++-
8
hw/intc/arm_gicv3_its.c | 7 ++++---
9
2 files changed, 6 insertions(+), 4 deletions(-)
10
11
diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
12
index XXXXXXX..XXXXXXX 100644
13
--- a/hw/intc/gicv3_internal.h
14
+++ b/hw/intc/gicv3_internal.h
15
@@ -XXX,XX +XXX,XX @@ FIELD(DTE, ITTADDR, 6, 44)
16
* Valid = 1 bit, RDBase = 16 bits
17
*/
18
#define GITS_CTE_SIZE (0x8ULL)
19
-#define GITS_CTE_RDBASE_PROCNUM_MASK MAKE_64BIT_MASK(1, RDBASE_PROCNUM_LENGTH)
20
+FIELD(CTE, VALID, 0, 1)
21
+FIELD(CTE, RDBASE, 1, RDBASE_PROCNUM_LENGTH)
22
23
/* Special interrupt IDs */
24
#define INTID_SECURE 1020
25
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
26
index XXXXXXX..XXXXXXX 100644
27
--- a/hw/intc/arm_gicv3_its.c
28
+++ b/hw/intc/arm_gicv3_its.c
29
@@ -XXX,XX +XXX,XX @@ static bool get_cte(GICv3ITSState *s, uint16_t icid, uint64_t *cte,
30
MEMTXATTRS_UNSPECIFIED, res);
31
}
32
33
- return (*cte & TABLE_ENTRY_VALID_MASK) != 0;
34
+ return FIELD_EX64(*cte, CTE, VALID);
35
}
36
37
static bool update_ite(GICv3ITSState *s, uint32_t eventid, uint64_t dte,
38
@@ -XXX,XX +XXX,XX @@ static bool process_its_cmd(GICv3ITSState *s, uint64_t value, uint32_t offset,
39
* Current implementation only supports rdbase == procnum
40
* Hence rdbase physical address is ignored
41
*/
42
- rdbase = (cte & GITS_CTE_RDBASE_PROCNUM_MASK) >> 1U;
43
+ rdbase = FIELD_EX64(cte, CTE, RDBASE);
44
45
if (rdbase >= s->gicv3->num_cpu) {
46
return result;
47
@@ -XXX,XX +XXX,XX @@ static bool update_cte(GICv3ITSState *s, uint16_t icid, bool valid,
48
49
if (valid) {
50
/* add mapping entry to collection table */
51
- cte = (valid & TABLE_ENTRY_VALID_MASK) | (rdbase << 1ULL);
52
+ cte = FIELD_DP64(cte, CTE, VALID, 1);
53
+ cte = FIELD_DP64(cte, CTE, RDBASE, rdbase);
54
}
55
56
/*
57
--
58
2.25.1
59
60
diff view generated by jsdifflib
New patch
1
The ITS code has to check whether various parameters passed in
2
commands are in-bounds, where the limit is defined in terms of the
3
number of bits that are available for the parameter. (For example,
4
the GITS_TYPER.Devbits ID register field specifies the number of
5
DeviceID bits minus 1, and device IDs passed in the MAPTI and MAPD
6
command packets must fit in that many bits.)
1
7
8
Currently we have off-by-one bugs in many of these bounds checks.
9
The typical problem is that we define a max_foo as 1 << n. In
10
the Devbits example, we set
11
s->dt.max_ids = 1UL << (GITS_TYPER.Devbits + 1).
12
However later when we do the bounds check we write
13
if (devid > s->dt.max_ids) { /* command error */ }
14
which incorrectly permits a devid of 1 << n.
15
16
These bugs will not cause QEMU crashes because the ID values being
17
checked are only used for accesses into tables held in guest memory
18
which we access with address_space_*() functions, but they are
19
incorrect behaviour of our emulation.
20
21
Fix them by standardizing on this pattern:
22
* bounds limits are named num_foos and are the 2^n value
23
(equal to the number of valid foo values)
24
* bounds checks are either
25
if (fooid < num_foos) { good }
26
or
27
if (fooid >= num_foos) { bad }
28
29
In this commit we fix the handling of the number of IDs
30
in the device table and the collection table, and the number
31
of commands that will fit in the command queue.
32
33
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
34
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
35
---
36
include/hw/intc/arm_gicv3_its_common.h | 6 +++---
37
hw/intc/arm_gicv3_its.c | 26 +++++++++++++-------------
38
2 files changed, 16 insertions(+), 16 deletions(-)
39
40
diff --git a/include/hw/intc/arm_gicv3_its_common.h b/include/hw/intc/arm_gicv3_its_common.h
41
index XXXXXXX..XXXXXXX 100644
42
--- a/include/hw/intc/arm_gicv3_its_common.h
43
+++ b/include/hw/intc/arm_gicv3_its_common.h
44
@@ -XXX,XX +XXX,XX @@ typedef struct {
45
bool indirect;
46
uint16_t entry_sz;
47
uint32_t page_sz;
48
- uint32_t max_entries;
49
- uint32_t max_ids;
50
+ uint32_t num_entries;
51
+ uint32_t num_ids;
52
uint64_t base_addr;
53
} TableDesc;
54
55
typedef struct {
56
bool valid;
57
- uint32_t max_entries;
58
+ uint32_t num_entries;
59
uint64_t base_addr;
60
} CmdQDesc;
61
62
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
63
index XXXXXXX..XXXXXXX 100644
64
--- a/hw/intc/arm_gicv3_its.c
65
+++ b/hw/intc/arm_gicv3_its.c
66
@@ -XXX,XX +XXX,XX @@ static bool process_its_cmd(GICv3ITSState *s, uint64_t value, uint32_t offset,
67
* In this implementation, in case of guest errors we ignore the
68
* command and move onto the next command in the queue.
69
*/
70
- if (devid > s->dt.max_ids) {
71
+ if (devid >= s->dt.num_ids) {
72
qemu_log_mask(LOG_GUEST_ERROR,
73
- "%s: invalid command attributes: devid %d>%d",
74
- __func__, devid, s->dt.max_ids);
75
+ "%s: invalid command attributes: devid %d>=%d",
76
+ __func__, devid, s->dt.num_ids);
77
78
} else if (!dte_valid || !ite_valid || !cte_valid) {
79
qemu_log_mask(LOG_GUEST_ERROR,
80
@@ -XXX,XX +XXX,XX @@ static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset,
81
max_eventid = 1UL << (FIELD_EX64(dte, DTE, SIZE) + 1);
82
max_Intid = (1ULL << (GICD_TYPER_IDBITS + 1)) - 1;
83
84
- if ((devid > s->dt.max_ids) || (icid > s->ct.max_ids)
85
+ if ((devid >= s->dt.num_ids) || (icid >= s->ct.num_ids)
86
|| !dte_valid || (eventid > max_eventid) ||
87
(((pIntid < GICV3_LPI_INTID_START) || (pIntid > max_Intid)) &&
88
(pIntid != INTID_SPURIOUS))) {
89
@@ -XXX,XX +XXX,XX @@ static bool process_mapc(GICv3ITSState *s, uint32_t offset)
90
91
valid = (value & CMD_FIELD_VALID_MASK);
92
93
- if ((icid > s->ct.max_ids) || (rdbase >= s->gicv3->num_cpu)) {
94
+ if ((icid >= s->ct.num_ids) || (rdbase >= s->gicv3->num_cpu)) {
95
qemu_log_mask(LOG_GUEST_ERROR,
96
"ITS MAPC: invalid collection table attributes "
97
"icid %d rdbase %" PRIu64 "\n", icid, rdbase);
98
@@ -XXX,XX +XXX,XX @@ static bool process_mapd(GICv3ITSState *s, uint64_t value, uint32_t offset)
99
100
valid = (value & CMD_FIELD_VALID_MASK);
101
102
- if ((devid > s->dt.max_ids) ||
103
+ if ((devid >= s->dt.num_ids) ||
104
(size > FIELD_EX64(s->typer, GITS_TYPER, IDBITS))) {
105
qemu_log_mask(LOG_GUEST_ERROR,
106
"ITS MAPD: invalid device table attributes "
107
@@ -XXX,XX +XXX,XX @@ static void process_cmdq(GICv3ITSState *s)
108
109
wr_offset = FIELD_EX64(s->cwriter, GITS_CWRITER, OFFSET);
110
111
- if (wr_offset > s->cq.max_entries) {
112
+ if (wr_offset >= s->cq.num_entries) {
113
qemu_log_mask(LOG_GUEST_ERROR,
114
"%s: invalid write offset "
115
"%d\n", __func__, wr_offset);
116
@@ -XXX,XX +XXX,XX @@ static void process_cmdq(GICv3ITSState *s)
117
118
rd_offset = FIELD_EX64(s->creadr, GITS_CREADR, OFFSET);
119
120
- if (rd_offset > s->cq.max_entries) {
121
+ if (rd_offset >= s->cq.num_entries) {
122
qemu_log_mask(LOG_GUEST_ERROR,
123
"%s: invalid read offset "
124
"%d\n", __func__, rd_offset);
125
@@ -XXX,XX +XXX,XX @@ static void process_cmdq(GICv3ITSState *s)
126
}
127
if (result) {
128
rd_offset++;
129
- rd_offset %= s->cq.max_entries;
130
+ rd_offset %= s->cq.num_entries;
131
s->creadr = FIELD_DP64(s->creadr, GITS_CREADR, OFFSET, rd_offset);
132
} else {
133
/*
134
@@ -XXX,XX +XXX,XX @@ static void extract_table_params(GICv3ITSState *s)
135
td->entry_sz = FIELD_EX64(value, GITS_BASER, ENTRYSIZE) + 1;
136
td->base_addr = baser_base_addr(value, page_sz);
137
if (!td->indirect) {
138
- td->max_entries = (num_pages * page_sz) / td->entry_sz;
139
+ td->num_entries = (num_pages * page_sz) / td->entry_sz;
140
} else {
141
- td->max_entries = (((num_pages * page_sz) /
142
+ td->num_entries = (((num_pages * page_sz) /
143
L1TABLE_ENTRY_SIZE) *
144
(page_sz / td->entry_sz));
145
}
146
- td->max_ids = 1ULL << idbits;
147
+ td->num_ids = 1ULL << idbits;
148
}
149
}
150
151
@@ -XXX,XX +XXX,XX @@ static void extract_cmdq_params(GICv3ITSState *s)
152
s->cq.valid = FIELD_EX64(value, GITS_CBASER, VALID);
153
154
if (s->cq.valid) {
155
- s->cq.max_entries = (num_pages * GITS_PAGE_SIZE_4K) /
156
+ s->cq.num_entries = (num_pages * GITS_PAGE_SIZE_4K) /
157
GITS_CMDQ_ENTRY_SIZE;
158
s->cq.base_addr = FIELD_EX64(value, GITS_CBASER, PHYADDR);
159
s->cq.base_addr <<= R_GITS_CBASER_PHYADDR_SHIFT;
160
--
161
2.25.1
162
163
diff view generated by jsdifflib
New patch
1
In several places we have a local variable max_l2_entries which is
2
the number of entries which will fit in a level 2 table. The
3
calculations done on this value are correct; rename it to
4
num_l2_entries to fit the convention we're using in this code.
1
5
6
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
7
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
8
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
9
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
10
---
11
hw/intc/arm_gicv3_its.c | 24 ++++++++++++------------
12
1 file changed, 12 insertions(+), 12 deletions(-)
13
14
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
15
index XXXXXXX..XXXXXXX 100644
16
--- a/hw/intc/arm_gicv3_its.c
17
+++ b/hw/intc/arm_gicv3_its.c
18
@@ -XXX,XX +XXX,XX @@ static bool get_cte(GICv3ITSState *s, uint16_t icid, uint64_t *cte,
19
uint64_t value;
20
bool valid_l2t;
21
uint32_t l2t_id;
22
- uint32_t max_l2_entries;
23
+ uint32_t num_l2_entries;
24
25
if (s->ct.indirect) {
26
l2t_id = icid / (s->ct.page_sz / L1TABLE_ENTRY_SIZE);
27
@@ -XXX,XX +XXX,XX @@ static bool get_cte(GICv3ITSState *s, uint16_t icid, uint64_t *cte,
28
valid_l2t = (value & L2_TABLE_VALID_MASK) != 0;
29
30
if (valid_l2t) {
31
- max_l2_entries = s->ct.page_sz / s->ct.entry_sz;
32
+ num_l2_entries = s->ct.page_sz / s->ct.entry_sz;
33
34
l2t_addr = value & ((1ULL << 51) - 1);
35
36
*cte = address_space_ldq_le(as, l2t_addr +
37
- ((icid % max_l2_entries) * GITS_CTE_SIZE),
38
+ ((icid % num_l2_entries) * GITS_CTE_SIZE),
39
MEMTXATTRS_UNSPECIFIED, res);
40
}
41
}
42
@@ -XXX,XX +XXX,XX @@ static uint64_t get_dte(GICv3ITSState *s, uint32_t devid, MemTxResult *res)
43
uint64_t value;
44
bool valid_l2t;
45
uint32_t l2t_id;
46
- uint32_t max_l2_entries;
47
+ uint32_t num_l2_entries;
48
49
if (s->dt.indirect) {
50
l2t_id = devid / (s->dt.page_sz / L1TABLE_ENTRY_SIZE);
51
@@ -XXX,XX +XXX,XX @@ static uint64_t get_dte(GICv3ITSState *s, uint32_t devid, MemTxResult *res)
52
valid_l2t = (value & L2_TABLE_VALID_MASK) != 0;
53
54
if (valid_l2t) {
55
- max_l2_entries = s->dt.page_sz / s->dt.entry_sz;
56
+ num_l2_entries = s->dt.page_sz / s->dt.entry_sz;
57
58
l2t_addr = value & ((1ULL << 51) - 1);
59
60
value = address_space_ldq_le(as, l2t_addr +
61
- ((devid % max_l2_entries) * GITS_DTE_SIZE),
62
+ ((devid % num_l2_entries) * GITS_DTE_SIZE),
63
MEMTXATTRS_UNSPECIFIED, res);
64
}
65
}
66
@@ -XXX,XX +XXX,XX @@ static bool update_cte(GICv3ITSState *s, uint16_t icid, bool valid,
67
uint64_t l2t_addr;
68
bool valid_l2t;
69
uint32_t l2t_id;
70
- uint32_t max_l2_entries;
71
+ uint32_t num_l2_entries;
72
uint64_t cte = 0;
73
MemTxResult res = MEMTX_OK;
74
75
@@ -XXX,XX +XXX,XX @@ static bool update_cte(GICv3ITSState *s, uint16_t icid, bool valid,
76
valid_l2t = (value & L2_TABLE_VALID_MASK) != 0;
77
78
if (valid_l2t) {
79
- max_l2_entries = s->ct.page_sz / s->ct.entry_sz;
80
+ num_l2_entries = s->ct.page_sz / s->ct.entry_sz;
81
82
l2t_addr = value & ((1ULL << 51) - 1);
83
84
address_space_stq_le(as, l2t_addr +
85
- ((icid % max_l2_entries) * GITS_CTE_SIZE),
86
+ ((icid % num_l2_entries) * GITS_CTE_SIZE),
87
cte, MEMTXATTRS_UNSPECIFIED, &res);
88
}
89
} else {
90
@@ -XXX,XX +XXX,XX @@ static bool update_dte(GICv3ITSState *s, uint32_t devid, bool valid,
91
uint64_t l2t_addr;
92
bool valid_l2t;
93
uint32_t l2t_id;
94
- uint32_t max_l2_entries;
95
+ uint32_t num_l2_entries;
96
uint64_t dte = 0;
97
MemTxResult res = MEMTX_OK;
98
99
@@ -XXX,XX +XXX,XX @@ static bool update_dte(GICv3ITSState *s, uint32_t devid, bool valid,
100
valid_l2t = (value & L2_TABLE_VALID_MASK) != 0;
101
102
if (valid_l2t) {
103
- max_l2_entries = s->dt.page_sz / s->dt.entry_sz;
104
+ num_l2_entries = s->dt.page_sz / s->dt.entry_sz;
105
106
l2t_addr = value & ((1ULL << 51) - 1);
107
108
address_space_stq_le(as, l2t_addr +
109
- ((devid % max_l2_entries) * GITS_DTE_SIZE),
110
+ ((devid % num_l2_entries) * GITS_DTE_SIZE),
111
dte, MEMTXATTRS_UNSPECIFIED, &res);
112
}
113
} else {
114
--
115
2.25.1
116
117
diff view generated by jsdifflib
New patch
1
From: Chris Rauer <crauer@google.com>
1
2
3
Signed-off-by: Chris Rauer <crauer@google.com>
4
Reviewed-by: Hao Wu <wuhaotsh@google.com>
5
Reviewed-by: Patrick Venture <venture@google.com>
6
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
7
Message-id: 20220102215844.2888833-2-venture@google.com
8
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
9
---
10
hw/arm/npcm7xx_boards.c | 8 ++++++++
11
1 file changed, 8 insertions(+)
12
13
diff --git a/hw/arm/npcm7xx_boards.c b/hw/arm/npcm7xx_boards.c
14
index XXXXXXX..XXXXXXX 100644
15
--- a/hw/arm/npcm7xx_boards.c
16
+++ b/hw/arm/npcm7xx_boards.c
17
@@ -XXX,XX +XXX,XX @@ static void quanta_gbs_i2c_init(NPCM7xxState *soc)
18
*/
19
}
20
21
+static void kudo_bmc_i2c_init(NPCM7xxState *soc)
22
+{
23
+ at24c_eeprom_init(soc, 4, 0x50, 8192); /* mbfru */
24
+ at24c_eeprom_init(soc, 14, 0x55, 8192); /* bmcfru */
25
+ /* TODO: Add remaining i2c devices. */
26
+}
27
+
28
static void npcm750_evb_init(MachineState *machine)
29
{
30
NPCM7xxState *soc;
31
@@ -XXX,XX +XXX,XX @@ static void kudo_bmc_init(MachineState *machine)
32
npcm7xx_connect_flash(&soc->fiu[1], 0, "mx66u51235f",
33
drive_get(IF_MTD, 3, 0));
34
35
+ kudo_bmc_i2c_init(soc);
36
npcm7xx_load_kernel(machine, soc);
37
}
38
39
--
40
2.25.1
41
42
diff view generated by jsdifflib
New patch
1
From: Shengtan Mao <stmao@google.com>
1
2
3
Signed-off-by: Shengtan Mao <stmao@google.com>
4
Reviewed-by: Hao Wu <wuhaotsh@google.com>
5
Reviewed-by: Chris Rauer <crauer@google.com>
6
Message-id: 20220102215844.2888833-3-venture@google.com
7
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
8
---
9
hw/arm/npcm7xx_boards.c | 1 +
10
1 file changed, 1 insertion(+)
11
12
diff --git a/hw/arm/npcm7xx_boards.c b/hw/arm/npcm7xx_boards.c
13
index XXXXXXX..XXXXXXX 100644
14
--- a/hw/arm/npcm7xx_boards.c
15
+++ b/hw/arm/npcm7xx_boards.c
16
@@ -XXX,XX +XXX,XX @@ static void kudo_bmc_init(MachineState *machine)
17
drive_get(IF_MTD, 3, 0));
18
19
kudo_bmc_i2c_init(soc);
20
+ sdhci_attach_drive(&soc->mmc.sdhci, 0);
21
npcm7xx_load_kernel(machine, soc);
22
}
23
24
--
25
2.25.1
26
27
diff view generated by jsdifflib
New patch
1
From: Patrick Venture <venture@google.com>
1
2
3
Signed-off-by: Patrick Venture <venture@google.com>
4
Reviewed-by: Hao Wu <wuhaotsh@google.com>
5
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
6
Message-id: 20220102215844.2888833-4-venture@google.com
7
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
8
---
9
hw/arm/npcm7xx_boards.c | 9 +++++++++
10
1 file changed, 9 insertions(+)
11
12
diff --git a/hw/arm/npcm7xx_boards.c b/hw/arm/npcm7xx_boards.c
13
index XXXXXXX..XXXXXXX 100644
14
--- a/hw/arm/npcm7xx_boards.c
15
+++ b/hw/arm/npcm7xx_boards.c
16
@@ -XXX,XX +XXX,XX @@ static void quanta_gbs_i2c_init(NPCM7xxState *soc)
17
18
static void kudo_bmc_i2c_init(NPCM7xxState *soc)
19
{
20
+ i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 1), TYPE_PCA9548, 0x75);
21
+ i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 1), TYPE_PCA9548, 0x77);
22
+
23
+ i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 4), TYPE_PCA9548, 0x77);
24
+
25
at24c_eeprom_init(soc, 4, 0x50, 8192); /* mbfru */
26
+
27
+ i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 13), TYPE_PCA9548, 0x77);
28
+
29
at24c_eeprom_init(soc, 14, 0x55, 8192); /* bmcfru */
30
+
31
/* TODO: Add remaining i2c devices. */
32
}
33
34
--
35
2.25.1
36
37
diff view generated by jsdifflib
1
From: Damien Hedde <damien.hedde@greensocs.com>
1
From: Patrick Venture <venture@google.com>
2
2
3
According to the "Arm Generic Interrupt Controller Architecture
3
Add the four lm75s behind the mux on bus 13.
4
Specification GIC architecture version 3 and 4" (version G: page 345
5
for aarch64 or 509 for aarch32):
6
LRENP bit of ICH_MISR is set when ICH_HCR.LRENPIE==1 and
7
ICH_HCR.EOIcount is non-zero.
8
4
9
When only LRENPIE was set (and EOI count was zero), the LRENP bit was
5
Tested by booting the firmware:
10
wrongly set and MISR value was wrong.
6
lm75 42-0048: hwmon0: sensor 'lm75'
7
lm75 43-0049: supply vs not found, using dummy regulator
8
lm75 43-0049: hwmon1: sensor 'lm75'
9
lm75 44-0048: supply vs not found, using dummy regulator
10
lm75 44-0048: hwmon2: sensor 'lm75'
11
lm75 45-0049: supply vs not found, using dummy regulator
12
lm75 45-0049: hwmon3: sensor 'lm75'
11
13
12
As an additional consequence, if an hypervisor set ICH_HCR.LRENPIE,
14
Signed-off-by: Patrick Venture <venture@google.com>
13
the maintenance interrupt was constantly fired. It happens since patch
15
Reviewed-by: Titus Rwantare <titusr@google.com>
14
9cee1efe92 ("hw/intc: Set GIC maintenance interrupt level to only 0 or 1")
16
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
15
which fixed another bug about maintenance interrupt (most significant
17
Message-id: 20220102215844.2888833-5-venture@google.com
16
bits of misr, including this one, were ignored in the interrupt trigger).
17
18
Fixes: 83f036fe3d ("hw/intc/arm_gicv3: Add accessors for ICH_ system registers")
19
Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
20
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
21
Message-id: 20211207094427.3473-1-damien.hedde@greensocs.com
22
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
18
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
23
---
19
---
24
hw/intc/arm_gicv3_cpuif.c | 3 ++-
20
hw/arm/npcm7xx_boards.c | 11 ++++++++++-
25
1 file changed, 2 insertions(+), 1 deletion(-)
21
1 file changed, 10 insertions(+), 1 deletion(-)
26
22
27
diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
23
diff --git a/hw/arm/npcm7xx_boards.c b/hw/arm/npcm7xx_boards.c
28
index XXXXXXX..XXXXXXX 100644
24
index XXXXXXX..XXXXXXX 100644
29
--- a/hw/intc/arm_gicv3_cpuif.c
25
--- a/hw/arm/npcm7xx_boards.c
30
+++ b/hw/intc/arm_gicv3_cpuif.c
26
+++ b/hw/arm/npcm7xx_boards.c
31
@@ -XXX,XX +XXX,XX @@ static uint32_t maintenance_interrupt_state(GICv3CPUState *cs)
27
@@ -XXX,XX +XXX,XX @@ static void quanta_gbs_i2c_init(NPCM7xxState *soc)
32
/* Scan list registers and fill in the U, NP and EOI bits */
28
33
eoi_maintenance_interrupt_state(cs, &value);
29
static void kudo_bmc_i2c_init(NPCM7xxState *soc)
34
30
{
35
- if (cs->ich_hcr_el2 & (ICH_HCR_EL2_LRENPIE | ICH_HCR_EL2_EOICOUNT_MASK)) {
31
+ I2CSlave *i2c_mux;
36
+ if ((cs->ich_hcr_el2 & ICH_HCR_EL2_LRENPIE) &&
32
+
37
+ (cs->ich_hcr_el2 & ICH_HCR_EL2_EOICOUNT_MASK)) {
33
i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 1), TYPE_PCA9548, 0x75);
38
value |= ICH_MISR_EL2_LRENP;
34
i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 1), TYPE_PCA9548, 0x77);
39
}
35
36
@@ -XXX,XX +XXX,XX @@ static void kudo_bmc_i2c_init(NPCM7xxState *soc)
37
38
at24c_eeprom_init(soc, 4, 0x50, 8192); /* mbfru */
39
40
- i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 13), TYPE_PCA9548, 0x77);
41
+ i2c_mux = i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 13),
42
+ TYPE_PCA9548, 0x77);
43
+
44
+ /* tmp105 is compatible with the lm75 */
45
+ i2c_slave_create_simple(pca954x_i2c_get_bus(i2c_mux, 2), "tmp105", 0x48);
46
+ i2c_slave_create_simple(pca954x_i2c_get_bus(i2c_mux, 3), "tmp105", 0x49);
47
+ i2c_slave_create_simple(pca954x_i2c_get_bus(i2c_mux, 4), "tmp105", 0x48);
48
+ i2c_slave_create_simple(pca954x_i2c_get_bus(i2c_mux, 5), "tmp105", 0x49);
49
50
at24c_eeprom_init(soc, 14, 0x55, 8192); /* bmcfru */
40
51
41
--
52
--
42
2.25.1
53
2.25.1
43
54
44
55
diff view generated by jsdifflib