1
Ten arm-related bug fixes for 2.12...
1
The following changes since commit 41fb4c14ee500125dc0ce6fb573cf84b8db29ed0:
2
2
3
thanks
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)
4
-- PMM
5
6
The following changes since commit 4c2c1015905fa1d616750dfe024b4c0b35875950:
7
8
Merge remote-tracking branch 'remotes/borntraeger/tags/s390x-20180323' into staging (2018-03-23 10:20:54 +0000)
9
4
10
are available in the Git repository at:
5
are available in the Git repository at:
11
6
12
git://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20180323
7
https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20220107
13
8
14
for you to fetch changes up to 548f514cf89dd9ab39c0cb4c063097bccf141fdd:
9
for you to fetch changes up to b8905cc2dde95ca6be5e56d77053b1ca0b8fc182:
15
10
16
target/arm: Always set FAR to a known unknown value for debug exceptions (2018-03-23 18:26:46 +0000)
11
hw/arm: kudo add lm75s on bus 13 (2022-01-07 17:08:01 +0000)
17
12
18
----------------------------------------------------------------
13
----------------------------------------------------------------
19
target-arm queue:
14
target-arm queue:
20
* arm/translate-a64: don't lose interrupts after unmasking via write to DAIF
15
* Add dummy Aspeed AST2600 Display Port MCU (DPMCU)
21
* sdhci: fix incorrect use of Error *
16
* Add missing FEAT_TLBIOS instructions
22
* hw/intc/arm_gicv3: Fix secure-GIC NS ICC_PMR and ICC_RPR accesses
17
* arm_gicv3_its: Various bug fixes and cleanups
23
* hw/arm/bcm2836: Use the Cortex-A7 instead of Cortex-A15
18
* kudo-bmc: Add more devices
24
* i.MX: Support serial RS-232 break properly
25
* mach-virt: Set VM's SMBIOS system version to mc->name
26
* target/arm: Honour MDCR_EL2.TDE when routing exceptions due to BKPT/BRK
27
* target/arm: Factor out code to calculate FSR for debug exceptions
28
* target/arm: Set FSR for BKPT, BRK when raising exception
29
* target/arm: Always set FAR to a known unknown value for debug exceptions
30
19
31
----------------------------------------------------------------
20
----------------------------------------------------------------
32
Paolo Bonzini (1):
21
Chris Rauer (1):
33
sdhci: fix incorrect use of Error *
22
hw/arm: Add kudo i2c eeproms.
34
23
35
Peter Maydell (6):
24
Idan Horowitz (1):
36
hw/intc/arm_gicv3: Fix secure-GIC NS ICC_PMR and ICC_RPR accesses
25
target/arm: Add missing FEAT_TLBIOS instructions
37
hw/arm/bcm2836: Use the Cortex-A7 instead of Cortex-A15
38
target/arm: Honour MDCR_EL2.TDE when routing exceptions due to BKPT/BRK
39
target/arm: Factor out code to calculate FSR for debug exceptions
40
target/arm: Set FSR for BKPT, BRK when raising exception
41
target/arm: Always set FAR to a known unknown value for debug exceptions
42
26
43
Trent Piepho (1):
27
Patrick Venture (2):
44
i.MX: Support serial RS-232 break properly
28
hw/arm: add i2c muxes to kudo-bmc
29
hw/arm: kudo add lm75s on bus 13
45
30
46
Victor Kamensky (1):
31
Peter Maydell (13):
47
arm/translate-a64: treat DISAS_UPDATE as variant of DISAS_EXIT
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
48
45
49
Wei Huang (1):
46
Shengtan Mao (1):
50
mach-virt: Set VM's SMBIOS system version to mc->name
47
hw/arm: attach MMC to kudo-bmc
51
48
52
include/hw/arm/virt.h | 1 +
49
Troy Lee (1):
53
include/hw/char/imx_serial.h | 1 +
50
Add dummy Aspeed AST2600 Display Port MCU (DPMCU)
54
target/arm/helper.h | 1 +
55
target/arm/internals.h | 25 +++++++++++++++++++++++++
56
hw/arm/bcm2836.c | 2 +-
57
hw/arm/raspi.c | 2 +-
58
hw/arm/virt.c | 8 +++++++-
59
hw/char/imx_serial.c | 5 ++++-
60
hw/intc/arm_gicv3_cpuif.c | 6 +++---
61
hw/sd/sdhci.c | 4 ++--
62
target/arm/helper.c | 1 -
63
target/arm/op_helper.c | 33 ++++++++++++++++++++++-----------
64
target/arm/translate-a64.c | 21 ++++++++++++++++-----
65
target/arm/translate.c | 19 ++++++++++++++-----
66
14 files changed, 98 insertions(+), 31 deletions(-)
67
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
1
From: Paolo Bonzini <pbonzini@redhat.com>
1
From: Troy Lee <troy_lee@aspeedtech.com>
2
2
3
Detected by Coverity (CID 1386072, 1386073, 1386076, 1386077). local_err
3
AST2600 Display Port MCU introduces 0x18000000~0x1803FFFF as it's memory
4
was unused, and this made the static analyzer unhappy.
4
and io address. If guest machine try to access DPMCU memory, it will
5
cause a fatal error.
5
6
6
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
7
Signed-off-by: Troy Lee <troy_lee@aspeedtech.com>
7
Message-id: 20180320151355.25854-1-pbonzini@redhat.com
8
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
8
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
9
Reviewed-by: Cédric Le Goater <clg@kaod.org>
10
Message-id: 20211210083034.726610-1-troy_lee@aspeedtech.com
9
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
11
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
10
---
12
---
11
hw/sd/sdhci.c | 4 ++--
13
include/hw/arm/aspeed_soc.h | 2 ++
12
1 file changed, 2 insertions(+), 2 deletions(-)
14
hw/arm/aspeed_ast2600.c | 8 ++++++++
15
2 files changed, 10 insertions(+)
13
16
14
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
17
diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
15
index XXXXXXX..XXXXXXX 100644
18
index XXXXXXX..XXXXXXX 100644
16
--- a/hw/sd/sdhci.c
19
--- a/include/hw/arm/aspeed_soc.h
17
+++ b/hw/sd/sdhci.c
20
+++ b/include/hw/arm/aspeed_soc.h
18
@@ -XXX,XX +XXX,XX @@ static void sdhci_pci_realize(PCIDevice *dev, Error **errp)
21
@@ -XXX,XX +XXX,XX @@ enum {
19
Error *local_err = NULL;
22
ASPEED_DEV_EMMC,
20
23
ASPEED_DEV_KCS,
21
sdhci_initfn(s);
24
ASPEED_DEV_HACE,
22
- sdhci_common_realize(s, errp);
25
+ ASPEED_DEV_DPMCU,
23
+ sdhci_common_realize(s, &local_err);
26
+ ASPEED_DEV_DP,
24
if (local_err) {
27
};
25
error_propagate(errp, local_err);
28
26
return;
29
#endif /* ASPEED_SOC_H */
27
@@ -XXX,XX +XXX,XX @@ static void sdhci_sysbus_realize(DeviceState *dev, Error ** errp)
30
diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
28
SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
31
index XXXXXXX..XXXXXXX 100644
29
Error *local_err = NULL;
32
--- a/hw/arm/aspeed_ast2600.c
30
33
+++ b/hw/arm/aspeed_ast2600.c
31
- sdhci_common_realize(s, errp);
34
@@ -XXX,XX +XXX,XX @@
32
+ sdhci_common_realize(s, &local_err);
35
#include "sysemu/sysemu.h"
33
if (local_err) {
36
34
error_propagate(errp, local_err);
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)) {
35
return;
72
return;
36
--
73
--
37
2.16.2
74
2.25.1
38
75
39
76
diff view generated by jsdifflib
1
Now that we have a helper function specifically for the BRK and
1
From: Idan Horowitz <idan.horowitz@gmail.com>
2
BKPT instructions, we can set the exception.fsr there rather
3
than in arm_cpu_do_interrupt_aarch32(). This allows us to
4
use our new arm_debug_exception_fsr() helper.
5
2
6
In particular this fixes a bug where we were hardcoding the
3
Some of the instructions added by the FEAT_TLBIOS extension were forgotten
7
short-form IFSR value, which is wrong if the target exception
4
when the extension was originally added to QEMU.
8
level has LPAE enabled.
9
5
10
Fixes: https://bugs.launchpad.net/qemu/+bug/1756927
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
11
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
10
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
12
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
13
Message-id: 20180320134114.30418-4-peter.maydell@linaro.org
14
---
11
---
15
target/arm/helper.c | 1 -
12
target/arm/helper.c | 32 ++++++++++++++++++++++++++++++++
16
target/arm/op_helper.c | 2 ++
13
1 file changed, 32 insertions(+)
17
2 files changed, 2 insertions(+), 1 deletion(-)
18
14
19
diff --git a/target/arm/helper.c b/target/arm/helper.c
15
diff --git a/target/arm/helper.c b/target/arm/helper.c
20
index XXXXXXX..XXXXXXX 100644
16
index XXXXXXX..XXXXXXX 100644
21
--- a/target/arm/helper.c
17
--- a/target/arm/helper.c
22
+++ b/target/arm/helper.c
18
+++ b/target/arm/helper.c
23
@@ -XXX,XX +XXX,XX @@ static void arm_cpu_do_interrupt_aarch32(CPUState *cs)
19
@@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo tlbios_reginfo[] = {
24
offset = 0;
20
.opc0 = 1, .opc1 = 0, .crn = 8, .crm = 1, .opc2 = 0,
25
break;
21
.access = PL1_W, .type = ARM_CP_NO_RAW,
26
case EXCP_BKPT:
22
.writefn = tlbi_aa64_vmalle1is_write },
27
- env->exception.fsr = 2;
23
+ { .name = "TLBI_VAE1OS", .state = ARM_CP_STATE_AA64,
28
/* Fall through to prefetch abort. */
24
+ .opc0 = 1, .opc1 = 0, .crn = 8, .crm = 1, .opc2 = 1,
29
case EXCP_PREFETCH_ABORT:
25
+ .access = PL1_W, .type = ARM_CP_NO_RAW,
30
A32_BANKED_CURRENT_REG_SET(env, ifsr, env->exception.fsr);
26
+ .writefn = tlbi_aa64_vae1is_write },
31
diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
27
{ .name = "TLBI_ASIDE1OS", .state = ARM_CP_STATE_AA64,
32
index XXXXXXX..XXXXXXX 100644
28
.opc0 = 1, .opc1 = 0, .crn = 8, .crm = 1, .opc2 = 2,
33
--- a/target/arm/op_helper.c
29
.access = PL1_W, .type = ARM_CP_NO_RAW,
34
+++ b/target/arm/op_helper.c
30
.writefn = tlbi_aa64_vmalle1is_write },
35
@@ -XXX,XX +XXX,XX @@ void HELPER(exception_with_syndrome)(CPUARMState *env, uint32_t excp,
31
+ { .name = "TLBI_VAAE1OS", .state = ARM_CP_STATE_AA64,
36
*/
32
+ .opc0 = 1, .opc1 = 0, .crn = 8, .crm = 1, .opc2 = 3,
37
void HELPER(exception_bkpt_insn)(CPUARMState *env, uint32_t syndrome)
33
+ .access = PL1_W, .type = ARM_CP_NO_RAW,
38
{
34
+ .writefn = tlbi_aa64_vae1is_write },
39
+ /* FSR will only be used if the debug target EL is AArch32. */
35
+ { .name = "TLBI_VALE1OS", .state = ARM_CP_STATE_AA64,
40
+ env->exception.fsr = arm_debug_exception_fsr(env);
36
+ .opc0 = 1, .opc1 = 0, .crn = 8, .crm = 1, .opc2 = 5,
41
raise_exception(env, EXCP_BKPT, syndrome, arm_debug_target_el(env));
37
+ .access = PL1_W, .type = ARM_CP_NO_RAW,
42
}
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
};
43
76
44
--
77
--
45
2.16.2
78
2.25.1
46
79
47
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
1
The MDCR_EL2.TDE bit allows the exception level targeted by debug
1
The MAPI command takes arguments DeviceID, EventID, ICID, and is
2
exceptions to be set to EL2 for code executing at EL0. We handle
2
defined to be equivalent to MAPTI DeviceID, EventID, EventID, ICID.
3
this in the arm_debug_target_el() function, but this is only used for
3
(That is, where MAPTI takes an explicit pINTID, MAPI uses the EventID
4
hardware breakpoint and watchpoint exceptions, not for the exception
4
as the pINTID.)
5
generated when the guest executes an AArch32 BKPT or AArch64 BRK
5
6
instruction. We don't have enough information for a translate-time
6
We didn't quite get this right. In particular the error checks for
7
equivalent of arm_debug_target_el(), so instead make BKPT and BRK
7
MAPI include "EventID does not specify a valid LPI identifier", which
8
call a special purpose helper which can do the routing, rather than
8
is the same as MAPTI's error check for the pINTID field. QEMU's code
9
the generic exception_with_syndrome helper.
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.
10
14
11
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
15
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
12
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
16
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
13
Message-id: 20180320134114.30418-2-peter.maydell@linaro.org
17
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
14
---
18
---
15
target/arm/helper.h | 1 +
19
hw/intc/arm_gicv3_its.c | 18 +++++++-----------
16
target/arm/op_helper.c | 8 ++++++++
20
1 file changed, 7 insertions(+), 11 deletions(-)
17
target/arm/translate-a64.c | 15 +++++++++++++--
18
target/arm/translate.c | 19 ++++++++++++++-----
19
4 files changed, 36 insertions(+), 7 deletions(-)
20
21
21
diff --git a/target/arm/helper.h b/target/arm/helper.h
22
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
22
index XXXXXXX..XXXXXXX 100644
23
index XXXXXXX..XXXXXXX 100644
23
--- a/target/arm/helper.h
24
--- a/hw/intc/arm_gicv3_its.c
24
+++ b/target/arm/helper.h
25
+++ b/hw/intc/arm_gicv3_its.c
25
@@ -XXX,XX +XXX,XX @@ DEF_HELPER_FLAGS_3(sel_flags, TCG_CALL_NO_RWG_SE,
26
@@ -XXX,XX +XXX,XX @@ static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset,
26
i32, i32, i32, i32)
27
27
DEF_HELPER_2(exception_internal, void, env, i32)
28
eventid = (value & EVENTID_MASK);
28
DEF_HELPER_4(exception_with_syndrome, void, env, i32, i32, i32)
29
29
+DEF_HELPER_2(exception_bkpt_insn, void, env, i32)
30
- if (!ignore_pInt) {
30
DEF_HELPER_1(setend, void, env)
31
+ if (ignore_pInt) {
31
DEF_HELPER_2(wfi, void, env, i32)
32
+ pIntid = eventid;
32
DEF_HELPER_1(wfe, void, env)
33
+ } else {
33
diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
34
pIntid = ((value & pINTID_MASK) >> pINTID_SHIFT);
34
index XXXXXXX..XXXXXXX 100644
35
}
35
--- a/target/arm/op_helper.c
36
36
+++ b/target/arm/op_helper.c
37
@@ -XXX,XX +XXX,XX @@ static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset,
37
@@ -XXX,XX +XXX,XX @@ void HELPER(exception_with_syndrome)(CPUARMState *env, uint32_t excp,
38
38
raise_exception(env, excp, syndrome, target_el);
39
max_eventid = (1UL << (((dte >> 1U) & SIZE_MASK) + 1));
39
}
40
40
41
- if (!ignore_pInt) {
41
+/* Raise an EXCP_BKPT with the specified syndrome register value,
42
- max_Intid = (1ULL << (GICD_TYPER_IDBITS + 1)) - 1;
42
+ * targeting the correct exception level for debug exceptions.
43
- }
43
+ */
44
+ max_Intid = (1ULL << (GICD_TYPER_IDBITS + 1)) - 1;
44
+void HELPER(exception_bkpt_insn)(CPUARMState *env, uint32_t syndrome)
45
45
+{
46
if ((devid > s->dt.max_ids) || (icid > s->ct.max_ids)
46
+ raise_exception(env, EXCP_BKPT, syndrome, arm_debug_target_el(env));
47
|| !dte_valid || (eventid > max_eventid) ||
47
+}
48
- (!ignore_pInt && (((pIntid < GICV3_LPI_INTID_START) ||
48
+
49
- (pIntid > max_Intid)) && (pIntid != INTID_SPURIOUS)))) {
49
uint32_t HELPER(cpsr_read)(CPUARMState *env)
50
+ (((pIntid < GICV3_LPI_INTID_START) || (pIntid > max_Intid)) &&
50
{
51
+ (pIntid != INTID_SPURIOUS))) {
51
return cpsr_read(env) & ~(CPSR_EXEC | CPSR_RESERVED);
52
qemu_log_mask(LOG_GUEST_ERROR,
52
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
53
"%s: invalid command attributes "
53
index XXXXXXX..XXXXXXX 100644
54
"devid %d or icid %d or eventid %d or pIntid %d or"
54
--- a/target/arm/translate-a64.c
55
@@ -XXX,XX +XXX,XX @@ static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset,
55
+++ b/target/arm/translate-a64.c
56
IteEntry ite = {};
56
@@ -XXX,XX +XXX,XX @@ static void gen_exception_insn(DisasContext *s, int offset, int excp,
57
ite.itel = FIELD_DP64(ite.itel, ITE_L, VALID, dte_valid);
57
s->base.is_jmp = DISAS_NORETURN;
58
ite.itel = FIELD_DP64(ite.itel, ITE_L, INTTYPE, ITE_INTTYPE_PHYSICAL);
58
}
59
- if (ignore_pInt) {
59
60
- ite.itel = FIELD_DP64(ite.itel, ITE_L, INTID, eventid);
60
+static void gen_exception_bkpt_insn(DisasContext *s, int offset,
61
- } else {
61
+ uint32_t syndrome)
62
- ite.itel = FIELD_DP64(ite.itel, ITE_L, INTID, pIntid);
62
+{
63
- }
63
+ TCGv_i32 tcg_syn;
64
+ ite.itel = FIELD_DP64(ite.itel, ITE_L, INTID, pIntid);
64
+
65
ite.itel = FIELD_DP64(ite.itel, ITE_L, DOORBELL, INTID_SPURIOUS);
65
+ gen_a64_set_pc_im(s->pc - offset);
66
ite.iteh = FIELD_DP32(ite.iteh, ITE_H, ICID, icid);
66
+ tcg_syn = tcg_const_i32(syndrome);
67
+ gen_helper_exception_bkpt_insn(cpu_env, tcg_syn);
68
+ tcg_temp_free_i32(tcg_syn);
69
+ s->base.is_jmp = DISAS_NORETURN;
70
+}
71
+
72
static void gen_ss_advance(DisasContext *s)
73
{
74
/* If the singlestep state is Active-not-pending, advance to
75
@@ -XXX,XX +XXX,XX @@ static void disas_exc(DisasContext *s, uint32_t insn)
76
break;
77
}
78
/* BRK */
79
- gen_exception_insn(s, 4, EXCP_BKPT, syn_aa64_bkpt(imm16),
80
- default_exception_el(s));
81
+ gen_exception_bkpt_insn(s, 4, syn_aa64_bkpt(imm16));
82
break;
83
case 2:
84
if (op2_ll != 0) {
85
diff --git a/target/arm/translate.c b/target/arm/translate.c
86
index XXXXXXX..XXXXXXX 100644
87
--- a/target/arm/translate.c
88
+++ b/target/arm/translate.c
89
@@ -XXX,XX +XXX,XX @@ static void gen_exception_insn(DisasContext *s, int offset, int excp,
90
s->base.is_jmp = DISAS_NORETURN;
91
}
92
93
+static void gen_exception_bkpt_insn(DisasContext *s, int offset, uint32_t syn)
94
+{
95
+ TCGv_i32 tcg_syn;
96
+
97
+ gen_set_condexec(s);
98
+ gen_set_pc_im(s, s->pc - offset);
99
+ tcg_syn = tcg_const_i32(syn);
100
+ gen_helper_exception_bkpt_insn(cpu_env, tcg_syn);
101
+ tcg_temp_free_i32(tcg_syn);
102
+ s->base.is_jmp = DISAS_NORETURN;
103
+}
104
+
105
/* Force a TB lookup after an instruction that changes the CPU state. */
106
static inline void gen_lookup_tb(DisasContext *s)
107
{
108
@@ -XXX,XX +XXX,XX @@ static void disas_arm_insn(DisasContext *s, unsigned int insn)
109
case 1:
110
/* bkpt */
111
ARCH(5);
112
- gen_exception_insn(s, 4, EXCP_BKPT,
113
- syn_aa32_bkpt(imm16, false),
114
- default_exception_el(s));
115
+ gen_exception_bkpt_insn(s, 4, syn_aa32_bkpt(imm16, false));
116
break;
117
case 2:
118
/* Hypervisor call (v7) */
119
@@ -XXX,XX +XXX,XX @@ static void disas_thumb_insn(DisasContext *s, uint32_t insn)
120
{
121
int imm8 = extract32(insn, 0, 8);
122
ARCH(5);
123
- gen_exception_insn(s, 2, EXCP_BKPT, syn_aa32_bkpt(imm8, true),
124
- default_exception_el(s));
125
+ gen_exception_bkpt_insn(s, 2, syn_aa32_bkpt(imm8, true));
126
break;
127
}
128
67
129
--
68
--
130
2.16.2
69
2.25.1
131
70
132
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
1
For debug exceptions due to breakpoints or the BKPT instruction which
1
Use FIELD macros to handle CTEs, rather than ad-hoc mask-and-shift.
2
are taken to AArch32, the Fault Address Register is architecturally
3
UNKNOWN. We were using that as license to simply not set
4
env->exception.vaddress, but this isn't correct, because it will
5
expose to the guest whatever old value was in that field when
6
arm_cpu_do_interrupt_aarch32() writes it to the guest IFSR. That old
7
value might be a FAR for a previous guest EL2 or secure exception, in
8
which case we shouldn't show it to an EL1 or non-secure exception
9
handler. It might also be a non-deterministic value, which is bad
10
for record-and-replay.
11
12
Clear env->exception.vaddress before taking breakpoint debug
13
exceptions, to avoid this minor information leak.
14
2
15
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
3
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
16
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
4
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
17
Message-id: 20180320134114.30418-5-peter.maydell@linaro.org
5
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
18
---
6
---
19
target/arm/op_helper.c | 11 ++++++++++-
7
hw/intc/gicv3_internal.h | 3 ++-
20
1 file changed, 10 insertions(+), 1 deletion(-)
8
hw/intc/arm_gicv3_its.c | 7 ++++---
9
2 files changed, 6 insertions(+), 4 deletions(-)
21
10
22
diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
11
diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
23
index XXXXXXX..XXXXXXX 100644
12
index XXXXXXX..XXXXXXX 100644
24
--- a/target/arm/op_helper.c
13
--- a/hw/intc/gicv3_internal.h
25
+++ b/target/arm/op_helper.c
14
+++ b/hw/intc/gicv3_internal.h
26
@@ -XXX,XX +XXX,XX @@ void HELPER(exception_bkpt_insn)(CPUARMState *env, uint32_t syndrome)
15
@@ -XXX,XX +XXX,XX @@ FIELD(DTE, ITTADDR, 6, 44)
27
{
16
* Valid = 1 bit, RDBase = 16 bits
28
/* FSR will only be used if the debug target EL is AArch32. */
17
*/
29
env->exception.fsr = arm_debug_exception_fsr(env);
18
#define GITS_CTE_SIZE (0x8ULL)
30
+ /* FAR is UNKNOWN: clear vaddress to avoid potentially exposing
19
-#define GITS_CTE_RDBASE_PROCNUM_MASK MAKE_64BIT_MASK(1, RDBASE_PROCNUM_LENGTH)
31
+ * values to the guest that it shouldn't be able to see at its
20
+FIELD(CTE, VALID, 0, 1)
32
+ * exception/security level.
21
+FIELD(CTE, RDBASE, 1, RDBASE_PROCNUM_LENGTH)
33
+ */
22
34
+ env->exception.vaddress = 0;
23
/* Special interrupt IDs */
35
raise_exception(env, EXCP_BKPT, syndrome, arm_debug_target_el(env));
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);
36
}
35
}
37
36
38
@@ -XXX,XX +XXX,XX @@ void arm_debug_excp_handler(CPUState *cs)
37
static bool update_ite(GICv3ITSState *s, uint32_t eventid, uint64_t dte,
39
}
38
@@ -XXX,XX +XXX,XX @@ static bool process_its_cmd(GICv3ITSState *s, uint64_t value, uint32_t offset,
40
39
* Current implementation only supports rdbase == procnum
41
env->exception.fsr = arm_debug_exception_fsr(env);
40
* Hence rdbase physical address is ignored
42
- /* FAR is UNKNOWN, so doesn't need setting */
41
*/
43
+ /* FAR is UNKNOWN: clear vaddress to avoid potentially exposing
42
- rdbase = (cte & GITS_CTE_RDBASE_PROCNUM_MASK) >> 1U;
44
+ * values to the guest that it shouldn't be able to see at its
43
+ rdbase = FIELD_EX64(cte, CTE, RDBASE);
45
+ * exception/security level.
44
46
+ */
45
if (rdbase >= s->gicv3->num_cpu) {
47
+ env->exception.vaddress = 0;
46
return result;
48
raise_exception(env, EXCP_PREFETCH_ABORT,
47
@@ -XXX,XX +XXX,XX @@ static bool update_cte(GICv3ITSState *s, uint16_t icid, bool valid,
49
syn_breakpoint(same_el),
48
50
arm_debug_target_el(env));
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
/*
51
--
57
--
52
2.16.2
58
2.25.1
53
59
54
60
diff view generated by jsdifflib
1
When a debug exception is taken to AArch32, it appears as a Prefetch
1
The ITS code has to check whether various parameters passed in
2
Abort, and the Instruction Fault Status Register (IFSR) must be set.
2
commands are in-bounds, where the limit is defined in terms of the
3
The IFSR has two possible formats, depending on whether LPAE is in
3
number of bits that are available for the parameter. (For example,
4
use. Factor out the code in arm_debug_excp_handler() which picks
4
the GITS_TYPER.Devbits ID register field specifies the number of
5
an FSR value into its own utility function, update it to use
5
DeviceID bits minus 1, and device IDs passed in the MAPTI and MAPD
6
arm_fi_to_lfsc() and arm_fi_to_sfsc() rather than hard-coded constants,
6
command packets must fit in that many bits.)
7
and use the correct condition to select long or short format.
8
7
9
In particular this fixes a bug where we could select the short
8
Currently we have off-by-one bugs in many of these bounds checks.
10
format because we're at EL0 and the EL1 translation regime is
9
The typical problem is that we define a max_foo as 1 << n. In
11
not using LPAE, but then route the debug exception to EL2 because
10
the Devbits example, we set
12
of MDCR_EL2.TDE and hand EL2 the wrong format FSR.
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.
13
32
14
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
33
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
15
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
34
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
16
Message-id: 20180320134114.30418-3-peter.maydell@linaro.org
17
---
35
---
18
target/arm/internals.h | 25 +++++++++++++++++++++++++
36
include/hw/intc/arm_gicv3_its_common.h | 6 +++---
19
target/arm/op_helper.c | 12 ++----------
37
hw/intc/arm_gicv3_its.c | 26 +++++++++++++-------------
20
2 files changed, 27 insertions(+), 10 deletions(-)
38
2 files changed, 16 insertions(+), 16 deletions(-)
21
39
22
diff --git a/target/arm/internals.h b/target/arm/internals.h
40
diff --git a/include/hw/intc/arm_gicv3_its_common.h b/include/hw/intc/arm_gicv3_its_common.h
23
index XXXXXXX..XXXXXXX 100644
41
index XXXXXXX..XXXXXXX 100644
24
--- a/target/arm/internals.h
42
--- a/include/hw/intc/arm_gicv3_its_common.h
25
+++ b/target/arm/internals.h
43
+++ b/include/hw/intc/arm_gicv3_its_common.h
26
@@ -XXX,XX +XXX,XX @@ static inline bool regime_is_secure(CPUARMState *env, ARMMMUIdx mmu_idx)
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;
27
}
148
}
28
}
149
}
29
150
30
+/* Return the FSR value for a debug exception (watchpoint, hardware
151
@@ -XXX,XX +XXX,XX @@ static void extract_cmdq_params(GICv3ITSState *s)
31
+ * breakpoint or BKPT insn) targeting the specified exception level.
152
s->cq.valid = FIELD_EX64(value, GITS_CBASER, VALID);
32
+ */
153
33
+static inline uint32_t arm_debug_exception_fsr(CPUARMState *env)
154
if (s->cq.valid) {
34
+{
155
- s->cq.max_entries = (num_pages * GITS_PAGE_SIZE_4K) /
35
+ ARMMMUFaultInfo fi = { .type = ARMFault_Debug };
156
+ s->cq.num_entries = (num_pages * GITS_PAGE_SIZE_4K) /
36
+ int target_el = arm_debug_target_el(env);
157
GITS_CMDQ_ENTRY_SIZE;
37
+ bool using_lpae = false;
158
s->cq.base_addr = FIELD_EX64(value, GITS_CBASER, PHYADDR);
38
+
159
s->cq.base_addr <<= R_GITS_CBASER_PHYADDR_SHIFT;
39
+ if (target_el == 2 || arm_el_is_aa64(env, target_el)) {
40
+ using_lpae = true;
41
+ } else {
42
+ if (arm_feature(env, ARM_FEATURE_LPAE) &&
43
+ (env->cp15.tcr_el[target_el].raw_tcr & TTBCR_EAE)) {
44
+ using_lpae = true;
45
+ }
46
+ }
47
+
48
+ if (using_lpae) {
49
+ return arm_fi_to_lfsc(&fi);
50
+ } else {
51
+ return arm_fi_to_sfsc(&fi);
52
+ }
53
+}
54
+
55
#endif
56
diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
57
index XXXXXXX..XXXXXXX 100644
58
--- a/target/arm/op_helper.c
59
+++ b/target/arm/op_helper.c
60
@@ -XXX,XX +XXX,XX @@ void arm_debug_excp_handler(CPUState *cs)
61
62
cs->watchpoint_hit = NULL;
63
64
- if (extended_addresses_enabled(env)) {
65
- env->exception.fsr = (1 << 9) | 0x22;
66
- } else {
67
- env->exception.fsr = 0x2;
68
- }
69
+ env->exception.fsr = arm_debug_exception_fsr(env);
70
env->exception.vaddress = wp_hit->hitaddr;
71
raise_exception(env, EXCP_DATA_ABORT,
72
syn_watchpoint(same_el, 0, wnr),
73
@@ -XXX,XX +XXX,XX @@ void arm_debug_excp_handler(CPUState *cs)
74
return;
75
}
76
77
- if (extended_addresses_enabled(env)) {
78
- env->exception.fsr = (1 << 9) | 0x22;
79
- } else {
80
- env->exception.fsr = 0x2;
81
- }
82
+ env->exception.fsr = arm_debug_exception_fsr(env);
83
/* FAR is UNKNOWN, so doesn't need setting */
84
raise_exception(env, EXCP_PREFETCH_ABORT,
85
syn_breakpoint(same_el),
86
--
160
--
87
2.16.2
161
2.25.1
88
162
89
163
diff view generated by jsdifflib
1
The BCM2836 uses a Cortex-A7, not a Cortex-A15. Update the device to
1
In several places we have a local variable max_l2_entries which is
2
use the correct CPU.
2
the number of entries which will fit in a level 2 table. The
3
https://www.raspberrypi.org/documentation/hardware/raspberrypi/bcm2836/QA7_rev3.4.pdf
3
calculations done on this value are correct; rename it to
4
4
num_l2_entries to fit the convention we're using in this code.
5
When the BCM2836 was introduced (bad5623690b) the Cortex-A7 was not
6
available, so the very similar Cortex-A15 was used. Since dcf578ed8ce
7
we can model the correct core.
8
5
9
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
6
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
10
Reviewed-by: Alistair Francis <alistair@alistair23.me>
7
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
8
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
11
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
9
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
12
Message-id: 20180319110215.16755-1-peter.maydell@linaro.org
13
---
10
---
14
hw/arm/bcm2836.c | 2 +-
11
hw/intc/arm_gicv3_its.c | 24 ++++++++++++------------
15
hw/arm/raspi.c | 2 +-
12
1 file changed, 12 insertions(+), 12 deletions(-)
16
2 files changed, 2 insertions(+), 2 deletions(-)
17
13
18
diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
14
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
19
index XXXXXXX..XXXXXXX 100644
15
index XXXXXXX..XXXXXXX 100644
20
--- a/hw/arm/bcm2836.c
16
--- a/hw/intc/arm_gicv3_its.c
21
+++ b/hw/arm/bcm2836.c
17
+++ b/hw/intc/arm_gicv3_its.c
22
@@ -XXX,XX +XXX,XX @@ struct BCM283XInfo {
18
@@ -XXX,XX +XXX,XX @@ static bool get_cte(GICv3ITSState *s, uint16_t icid, uint64_t *cte,
23
static const BCM283XInfo bcm283x_socs[] = {
19
uint64_t value;
24
{
20
bool valid_l2t;
25
.name = TYPE_BCM2836,
21
uint32_t l2t_id;
26
- .cpu_type = ARM_CPU_TYPE_NAME("cortex-a15"),
22
- uint32_t max_l2_entries;
27
+ .cpu_type = ARM_CPU_TYPE_NAME("cortex-a7"),
23
+ uint32_t num_l2_entries;
28
.clusterid = 0xf,
24
29
},
25
if (s->ct.indirect) {
30
#ifdef TARGET_AARCH64
26
l2t_id = icid / (s->ct.page_sz / L1TABLE_ENTRY_SIZE);
31
diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
27
@@ -XXX,XX +XXX,XX @@ static bool get_cte(GICv3ITSState *s, uint16_t icid, uint64_t *cte,
32
index XXXXXXX..XXXXXXX 100644
28
valid_l2t = (value & L2_TABLE_VALID_MASK) != 0;
33
--- a/hw/arm/raspi.c
29
34
+++ b/hw/arm/raspi.c
30
if (valid_l2t) {
35
@@ -XXX,XX +XXX,XX @@ static void raspi2_machine_init(MachineClass *mc)
31
- max_l2_entries = s->ct.page_sz / s->ct.entry_sz;
36
mc->no_parallel = 1;
32
+ num_l2_entries = s->ct.page_sz / s->ct.entry_sz;
37
mc->no_floppy = 1;
33
38
mc->no_cdrom = 1;
34
l2t_addr = value & ((1ULL << 51) - 1);
39
- mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a15");
35
40
+ mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a7");
36
*cte = address_space_ldq_le(as, l2t_addr +
41
mc->max_cpus = BCM283X_NCPUS;
37
- ((icid % max_l2_entries) * GITS_CTE_SIZE),
42
mc->min_cpus = BCM283X_NCPUS;
38
+ ((icid % num_l2_entries) * GITS_CTE_SIZE),
43
mc->default_cpus = BCM283X_NCPUS;
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 {
44
--
114
--
45
2.16.2
115
2.25.1
46
116
47
117
diff view generated by jsdifflib
1
From: Trent Piepho <tpiepho@impinj.com>
1
From: Chris Rauer <crauer@google.com>
2
2
3
Linux does not detect a break from this IMX serial driver as a magic
3
Signed-off-by: Chris Rauer <crauer@google.com>
4
sysrq. Nor does it note a break in the port error counts.
4
Reviewed-by: Hao Wu <wuhaotsh@google.com>
5
5
Reviewed-by: Patrick Venture <venture@google.com>
6
The former is because the Linux driver uses the BRCD bit in the USR2
6
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
7
register to trigger the RS-232 break handler in the kernel, which is
7
Message-id: 20220102215844.2888833-2-venture@google.com
8
where sysrq hooks in. The emulated UART was not setting this status
9
bit.
10
11
The latter is because the Linux driver expects, in addition to the BRK
12
bit, that the ERR bit is set when a break is read in the FIFO. A break
13
should also count as a frame error, so add that bit too.
14
15
Cc: Andrey Smirnov <andrew.smirnov@gmail.com>
16
Signed-off-by: Trent Piepho <tpiepho@impinj.com>
17
Message-id: 20180320013657.25038-1-tpiepho@impinj.com
18
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
19
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
8
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
20
---
9
---
21
include/hw/char/imx_serial.h | 1 +
10
hw/arm/npcm7xx_boards.c | 8 ++++++++
22
hw/char/imx_serial.c | 5 ++++-
11
1 file changed, 8 insertions(+)
23
2 files changed, 5 insertions(+), 1 deletion(-)
24
12
25
diff --git a/include/hw/char/imx_serial.h b/include/hw/char/imx_serial.h
13
diff --git a/hw/arm/npcm7xx_boards.c b/hw/arm/npcm7xx_boards.c
26
index XXXXXXX..XXXXXXX 100644
14
index XXXXXXX..XXXXXXX 100644
27
--- a/include/hw/char/imx_serial.h
15
--- a/hw/arm/npcm7xx_boards.c
28
+++ b/include/hw/char/imx_serial.h
16
+++ b/hw/arm/npcm7xx_boards.c
29
@@ -XXX,XX +XXX,XX @@
17
@@ -XXX,XX +XXX,XX @@ static void quanta_gbs_i2c_init(NPCM7xxState *soc)
30
18
*/
31
#define URXD_CHARRDY (1<<15) /* character read is valid */
32
#define URXD_ERR (1<<14) /* Character has error */
33
+#define URXD_FRMERR (1<<12) /* Character has frame error */
34
#define URXD_BRK (1<<11) /* Break received */
35
36
#define USR1_PARTYER (1<<15) /* Parity Error */
37
diff --git a/hw/char/imx_serial.c b/hw/char/imx_serial.c
38
index XXXXXXX..XXXXXXX 100644
39
--- a/hw/char/imx_serial.c
40
+++ b/hw/char/imx_serial.c
41
@@ -XXX,XX +XXX,XX @@ static void imx_put_data(void *opaque, uint32_t value)
42
s->usr2 |= USR2_RDR;
43
s->uts1 &= ~UTS1_RXEMPTY;
44
s->readbuff = value;
45
+ if (value & URXD_BRK) {
46
+ s->usr2 |= USR2_BRCD;
47
+ }
48
imx_update(s);
49
}
19
}
50
20
51
@@ -XXX,XX +XXX,XX @@ static void imx_receive(void *opaque, const uint8_t *buf, int size)
21
+static void kudo_bmc_i2c_init(NPCM7xxState *soc)
52
static void imx_event(void *opaque, int event)
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)
53
{
29
{
54
if (event == CHR_EVENT_BREAK) {
30
NPCM7xxState *soc;
55
- imx_put_data(opaque, URXD_BRK);
31
@@ -XXX,XX +XXX,XX @@ static void kudo_bmc_init(MachineState *machine)
56
+ imx_put_data(opaque, URXD_BRK | URXD_FRMERR | URXD_ERR);
32
npcm7xx_connect_flash(&soc->fiu[1], 0, "mx66u51235f",
57
}
33
drive_get(IF_MTD, 3, 0));
34
35
+ kudo_bmc_i2c_init(soc);
36
npcm7xx_load_kernel(machine, soc);
58
}
37
}
59
38
60
--
39
--
61
2.16.2
40
2.25.1
62
41
63
42
diff view generated by jsdifflib
1
If the GIC has the security extension support enabled, then a
1
From: Shengtan Mao <stmao@google.com>
2
non-secure access to ICC_PMR must take account of the non-secure
3
view of interrupt priorities, where real priorities 0x00..0x7f
4
are secure-only and not visible to the non-secure guest, and
5
priorities 0x80..0xff are shown to the guest as if they were
6
0x00..0xff. We had the logic here wrong:
7
* on reads, the priority is in the secure range if bit 7
8
is clear, not if it is set
9
* on writes, we want to set bit 7, not mask everything else
10
2
11
Our ICC_RPR read code had the same error as ICC_PMR.
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(+)
12
11
13
(Compare the GICv3 spec pseudocode functions ICC_RPR_EL1
12
diff --git a/hw/arm/npcm7xx_boards.c b/hw/arm/npcm7xx_boards.c
14
and ICC_PMR_EL1.)
15
16
Fixes: https://bugs.launchpad.net/qemu/+bug/1748434
17
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
18
Reviewed-by: Andrew Jones <drjones@redhat.com>
19
Message-id: 20180315133441.24149-1-peter.maydell@linaro.org
20
---
21
hw/intc/arm_gicv3_cpuif.c | 6 +++---
22
1 file changed, 3 insertions(+), 3 deletions(-)
23
24
diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
25
index XXXXXXX..XXXXXXX 100644
13
index XXXXXXX..XXXXXXX 100644
26
--- a/hw/intc/arm_gicv3_cpuif.c
14
--- a/hw/arm/npcm7xx_boards.c
27
+++ b/hw/intc/arm_gicv3_cpuif.c
15
+++ b/hw/arm/npcm7xx_boards.c
28
@@ -XXX,XX +XXX,XX @@ static uint64_t icc_pmr_read(CPUARMState *env, const ARMCPRegInfo *ri)
16
@@ -XXX,XX +XXX,XX @@ static void kudo_bmc_init(MachineState *machine)
29
/* NS access and Group 0 is inaccessible to NS: return the
17
drive_get(IF_MTD, 3, 0));
30
* NS view of the current priority
18
31
*/
19
kudo_bmc_i2c_init(soc);
32
- if (value & 0x80) {
20
+ sdhci_attach_drive(&soc->mmc.sdhci, 0);
33
+ if ((value & 0x80) == 0) {
21
npcm7xx_load_kernel(machine, soc);
34
/* Secure priorities not visible to NS */
22
}
35
value = 0;
23
36
} else if (value != 0xff) {
37
@@ -XXX,XX +XXX,XX @@ static void icc_pmr_write(CPUARMState *env, const ARMCPRegInfo *ri,
38
/* Current PMR in the secure range, don't allow NS to change it */
39
return;
40
}
41
- value = (value >> 1) & 0x80;
42
+ value = (value >> 1) | 0x80;
43
}
44
cs->icc_pmr_el1 = value;
45
gicv3_cpuif_update(cs);
46
@@ -XXX,XX +XXX,XX @@ static uint64_t icc_rpr_read(CPUARMState *env, const ARMCPRegInfo *ri)
47
if (arm_feature(env, ARM_FEATURE_EL3) &&
48
!arm_is_secure(env) && (env->cp15.scr_el3 & SCR_FIQ)) {
49
/* NS GIC access and Group 0 is inaccessible to NS */
50
- if (prio & 0x80) {
51
+ if ((prio & 0x80) == 0) {
52
/* NS mustn't see priorities in the Secure half of the range */
53
prio = 0;
54
} else if (prio != 0xff) {
55
--
24
--
56
2.16.2
25
2.25.1
57
26
58
27
diff view generated by jsdifflib
1
From: Wei Huang <wei@redhat.com>
1
From: Patrick Venture <venture@google.com>
2
2
3
Instead of using "1.0" as the system version of SMBIOS, we should use
3
Signed-off-by: Patrick Venture <venture@google.com>
4
mc->name for mach-virt machine type to be consistent other architectures.
4
Reviewed-by: Hao Wu <wuhaotsh@google.com>
5
With this patch, "dmidecode -t 1" (e.g., "-M virt-2.12,accel=kvm") will
5
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
6
show:
6
Message-id: 20220102215844.2888833-4-venture@google.com
7
8
Handle 0x0100, DMI type 1, 27 bytes
9
System Information
10
Manufacturer: QEMU
11
Product Name: KVM Virtual Machine
12
Version: virt-2.12
13
Serial Number: Not Specified
14
...
15
16
instead of:
17
18
Handle 0x0100, DMI type 1, 27 bytes
19
System Information
20
Manufacturer: QEMU
21
Product Name: KVM Virtual Machine
22
Version: 1.0
23
Serial Number: Not Specified
24
...
25
26
For backward compatibility, we allow older machine types to keep "1.0"
27
as the default system version.
28
29
Signed-off-by: Wei Huang <wei@redhat.com>
30
Reviewed-by: Andrew Jones <drjones@redhat.com>
31
Message-id: 20180322212318.7182-1-wei@redhat.com
32
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
7
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
33
---
8
---
34
include/hw/arm/virt.h | 1 +
9
hw/arm/npcm7xx_boards.c | 9 +++++++++
35
hw/arm/virt.c | 8 +++++++-
10
1 file changed, 9 insertions(+)
36
2 files changed, 8 insertions(+), 1 deletion(-)
37
11
38
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
12
diff --git a/hw/arm/npcm7xx_boards.c b/hw/arm/npcm7xx_boards.c
39
index XXXXXXX..XXXXXXX 100644
13
index XXXXXXX..XXXXXXX 100644
40
--- a/include/hw/arm/virt.h
14
--- a/hw/arm/npcm7xx_boards.c
41
+++ b/include/hw/arm/virt.h
15
+++ b/hw/arm/npcm7xx_boards.c
42
@@ -XXX,XX +XXX,XX @@ typedef struct {
16
@@ -XXX,XX +XXX,XX @@ static void quanta_gbs_i2c_init(NPCM7xxState *soc)
43
bool no_its;
17
44
bool no_pmu;
18
static void kudo_bmc_i2c_init(NPCM7xxState *soc)
45
bool claim_edge_triggered_timers;
46
+ bool smbios_old_sys_ver;
47
} VirtMachineClass;
48
49
typedef struct {
50
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
51
index XXXXXXX..XXXXXXX 100644
52
--- a/hw/arm/virt.c
53
+++ b/hw/arm/virt.c
54
@@ -XXX,XX +XXX,XX @@ static void *machvirt_dtb(const struct arm_boot_info *binfo, int *fdt_size)
55
56
static void virt_build_smbios(VirtMachineState *vms)
57
{
19
{
58
+ MachineClass *mc = MACHINE_GET_CLASS(vms);
20
+ i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 1), TYPE_PCA9548, 0x75);
59
+ VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
21
+ i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 1), TYPE_PCA9548, 0x77);
60
uint8_t *smbios_tables, *smbios_anchor;
61
size_t smbios_tables_len, smbios_anchor_len;
62
const char *product = "QEMU Virtual Machine";
63
@@ -XXX,XX +XXX,XX @@ static void virt_build_smbios(VirtMachineState *vms)
64
}
65
66
smbios_set_defaults("QEMU", product,
67
- "1.0", false, true, SMBIOS_ENTRY_POINT_30);
68
+ vmc->smbios_old_sys_ver ? "1.0" : mc->name, false,
69
+ true, SMBIOS_ENTRY_POINT_30);
70
71
smbios_get_tables(NULL, 0, &smbios_tables, &smbios_tables_len,
72
&smbios_anchor, &smbios_anchor_len);
73
@@ -XXX,XX +XXX,XX @@ static void virt_2_11_instance_init(Object *obj)
74
75
static void virt_machine_2_11_options(MachineClass *mc)
76
{
77
+ VirtMachineClass *vmc = VIRT_MACHINE_CLASS(OBJECT_CLASS(mc));
78
+
22
+
79
virt_machine_2_12_options(mc);
23
+ i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 4), TYPE_PCA9548, 0x77);
80
SET_MACHINE_COMPAT(mc, VIRT_COMPAT_2_11);
24
+
81
+ vmc->smbios_old_sys_ver = true;
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. */
82
}
32
}
83
DEFINE_VIRT_MACHINE(2, 11)
84
33
85
--
34
--
86
2.16.2
35
2.25.1
87
36
88
37
diff view generated by jsdifflib
1
From: Victor Kamensky <kamensky@cisco.com>
1
From: Patrick Venture <venture@google.com>
2
2
3
In OE project 4.15 linux kernel boot hang was observed under
3
Add the four lm75s behind the mux on bus 13.
4
single cpu aarch64 qemu. Kernel code was in a loop waiting for
5
vtimer arrival, spinning in TC generated blocks, while interrupt
6
was pending unprocessed. This happened because when qemu tried to
7
handle vtimer interrupt target had interrupts disabled, as
8
result flag indicating TCG exit, cpu->icount_decr.u16.high,
9
was cleared but arm_cpu_exec_interrupt function did not call
10
arm_cpu_do_interrupt to process interrupt. Later when target
11
reenabled interrupts, it happened without exit into main loop, so
12
following code that waited for result of interrupt execution
13
run in infinite loop.
14
4
15
To solve the problem instructions that operate on CPU sys state
5
Tested by booting the firmware:
16
(i.e enable/disable interrupt), and marked as DISAS_UPDATE,
6
lm75 42-0048: hwmon0: sensor 'lm75'
17
should be considered as DISAS_EXIT variant, and should be
7
lm75 43-0049: supply vs not found, using dummy regulator
18
forced to exit back to main loop so qemu will have a chance
8
lm75 43-0049: hwmon1: sensor 'lm75'
19
processing pending CPU state updates, including pending
9
lm75 44-0048: supply vs not found, using dummy regulator
20
interrupts.
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'
21
13
22
This change brings consistency with how DISAS_UPDATE is treated
14
Signed-off-by: Patrick Venture <venture@google.com>
23
in aarch32 case.
15
Reviewed-by: Titus Rwantare <titusr@google.com>
24
16
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
25
CC: Peter Maydell <peter.maydell@linaro.org>
17
Message-id: 20220102215844.2888833-5-venture@google.com
26
CC: Alex Bennée <alex.bennee@linaro.org>
27
CC: qemu-stable@nongnu.org
28
Suggested-by: Peter Maydell <peter.maydell@linaro.org>
29
Signed-off-by: Victor Kamensky <kamensky@cisco.com>
30
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
31
Message-id: 1521526368-1996-1-git-send-email-kamensky@cisco.com
32
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
18
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
33
---
19
---
34
target/arm/translate-a64.c | 6 +++---
20
hw/arm/npcm7xx_boards.c | 11 ++++++++++-
35
1 file changed, 3 insertions(+), 3 deletions(-)
21
1 file changed, 10 insertions(+), 1 deletion(-)
36
22
37
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
23
diff --git a/hw/arm/npcm7xx_boards.c b/hw/arm/npcm7xx_boards.c
38
index XXXXXXX..XXXXXXX 100644
24
index XXXXXXX..XXXXXXX 100644
39
--- a/target/arm/translate-a64.c
25
--- a/hw/arm/npcm7xx_boards.c
40
+++ b/target/arm/translate-a64.c
26
+++ b/hw/arm/npcm7xx_boards.c
41
@@ -XXX,XX +XXX,XX @@ static void aarch64_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
27
@@ -XXX,XX +XXX,XX @@ static void quanta_gbs_i2c_init(NPCM7xxState *soc)
42
case DISAS_UPDATE:
28
43
gen_a64_set_pc_im(dc->pc);
29
static void kudo_bmc_i2c_init(NPCM7xxState *soc)
44
/* fall through */
30
{
45
- case DISAS_JUMP:
31
+ I2CSlave *i2c_mux;
46
- tcg_gen_lookup_and_goto_ptr();
32
+
47
- break;
33
i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 1), TYPE_PCA9548, 0x75);
48
case DISAS_EXIT:
34
i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 1), TYPE_PCA9548, 0x77);
49
tcg_gen_exit_tb(0);
35
50
break;
36
@@ -XXX,XX +XXX,XX @@ static void kudo_bmc_i2c_init(NPCM7xxState *soc)
51
+ case DISAS_JUMP:
37
52
+ tcg_gen_lookup_and_goto_ptr();
38
at24c_eeprom_init(soc, 4, 0x50, 8192); /* mbfru */
53
+ break;
39
54
case DISAS_NORETURN:
40
- i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 13), TYPE_PCA9548, 0x77);
55
case DISAS_SWI:
41
+ i2c_mux = i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 13),
56
break;
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 */
51
57
--
52
--
58
2.16.2
53
2.25.1
59
54
60
55
diff view generated by jsdifflib