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 |