1 | A mixed bag, all bug fixes or similar small stuff. | 1 | target-arm queue for rc1 -- these are all bug fixes. |
---|---|---|---|
2 | 2 | ||
3 | thanks | 3 | thanks |
4 | -- PMM | 4 | -- PMM |
5 | 5 | ||
6 | The following changes since commit b9404bf592e7ba74180e1a54ed7a266ec6ee67f2: | ||
6 | 7 | ||
7 | The following changes since commit 19eb2d4e736dc895f31fbd6b520e514f10cc08e0: | 8 | Merge remote-tracking branch 'remotes/dgilbert/tags/pull-hmp-20190715' into staging (2019-07-15 12:22:07 +0100) |
8 | |||
9 | Merge remote-tracking branch 'remotes/thibault/tags/samuel-thibault' into staging (2019-05-07 10:43:32 +0100) | ||
10 | 9 | ||
11 | are available in the Git repository at: | 10 | are available in the Git repository at: |
12 | 11 | ||
13 | https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20190507 | 12 | https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20190715 |
14 | 13 | ||
15 | for you to fetch changes up to 63159601fb3e396b28da14cbb71e50ed3f5a0331: | 14 | for you to fetch changes up to 51c9122e92b776a3f16af0b9282f1dc5012e2a19: |
16 | 15 | ||
17 | target/arm: Stop using variable length array in dc_zva (2019-05-07 12:55:04 +0100) | 16 | target/arm: NS BusFault on vector table fetch escalates to NS HardFault (2019-07-15 14:17:04 +0100) |
18 | 17 | ||
19 | ---------------------------------------------------------------- | 18 | ---------------------------------------------------------------- |
20 | target-arm queue: | 19 | target-arm queue: |
21 | * Stop using variable length array in dc_zva | 20 | * report ARMv8-A FP support for AArch32 -cpu max |
22 | * Implement M-profile XPSR GE bits | 21 | * hw/ssi/xilinx_spips: Avoid AXI writes to the LQSPI linear memory |
23 | * Don't enable ARMV7M_EXCP_DEBUG from reset | 22 | * hw/ssi/xilinx_spips: Avoid out-of-bound access to lqspi_buf[] |
24 | * armv7m_nvic: NS BFAR and BFSR are RAZ/WI if BFHFNMINS == 0 | 23 | * hw/ssi/mss-spi: Avoid crash when reading empty RX FIFO |
25 | * armv7m_nvic: Check subpriority in nvic_recompute_state_secure() | 24 | * hw/display/xlnx_dp: Avoid crash when reading empty RX FIFO |
26 | * fix various minor issues to allow building for Windows-on-ARM64 | 25 | * hw/arm/virt: Fix non-secure flash mode |
27 | * aspeed: Set SDRAM size | 26 | * pl031: Correctly migrate state when using -rtc clock=host |
28 | * Allow system registers for KVM guests to be changed by QEMU code | 27 | * fix regression that meant arm926 and arm1026 lost VFP |
29 | * raspi: Diagnose requests for too much RAM | 28 | double-precision support |
30 | * virt: Support firmware configuration with -blockdev | 29 | * v8M: NS BusFault on vector table fetch escalates to NS HardFault |
31 | 30 | ||
32 | ---------------------------------------------------------------- | 31 | ---------------------------------------------------------------- |
33 | Cao Jiaxi (4): | 32 | Alex Bennée (1): |
34 | QEMU_PACKED: Remove gcc_struct attribute in Windows non x86 targets | 33 | target/arm: report ARMv8-A FP support for AArch32 -cpu max |
35 | qga: Fix mingw compilation warnings on enum conversion | ||
36 | util/cacheinfo: Use uint64_t on LLP64 model to satisfy Windows ARM64 | ||
37 | osdep: Fix mingw compilation regarding stdio formats | ||
38 | 34 | ||
39 | Joel Stanley (1): | 35 | David Engraf (1): |
40 | arm: aspeed: Set SDRAM size | 36 | hw/arm/virt: Fix non-secure flash mode |
41 | 37 | ||
42 | Markus Armbruster (3): | 38 | Peter Maydell (3): |
43 | pc: Rearrange pc_system_firmware_init()'s legacy -drive loop | 39 | pl031: Correctly migrate state when using -rtc clock=host |
44 | pflash_cfi01: New pflash_cfi01_legacy_drive() | 40 | target/arm: Set VFP-related MVFR0 fields for arm926 and arm1026 |
45 | hw/arm/virt: Support firmware configuration with -blockdev | 41 | target/arm: NS BusFault on vector table fetch escalates to NS HardFault |
46 | 42 | ||
47 | Peter Maydell (7): | 43 | Philippe Mathieu-Daudé (5): |
48 | hw/arm/raspi: Diagnose requests for too much RAM | 44 | hw/ssi/xilinx_spips: Convert lqspi_read() to read_with_attrs |
49 | arm: Allow system registers for KVM guests to be changed by QEMU code | 45 | hw/ssi/xilinx_spips: Avoid AXI writes to the LQSPI linear memory |
50 | hw/arm/armv7m_nvic: Check subpriority in nvic_recompute_state_secure() | 46 | hw/ssi/xilinx_spips: Avoid out-of-bound access to lqspi_buf[] |
51 | hw/intc/armv7m_nvic: NS BFAR and BFSR are RAZ/WI if BFHFNMINS == 0 | 47 | hw/ssi/mss-spi: Avoid crash when reading empty RX FIFO |
52 | hw/intc/armv7m_nvic: Don't enable ARMV7M_EXCP_DEBUG from reset | 48 | hw/display/xlnx_dp: Avoid crash when reading empty RX FIFO |
53 | target/arm: Implement XPSR GE bits | ||
54 | target/arm: Stop using variable length array in dc_zva | ||
55 | 49 | ||
56 | contrib/libvhost-user/libvhost-user.h | 2 +- | 50 | include/hw/timer/pl031.h | 2 ++ |
57 | include/hw/arm/aspeed.h | 1 + | 51 | hw/arm/virt.c | 2 +- |
58 | include/hw/arm/virt.h | 2 + | 52 | hw/core/machine.c | 1 + |
59 | include/hw/block/flash.h | 1 + | 53 | hw/display/xlnx_dp.c | 15 +++++--- |
60 | include/qemu/compiler.h | 2 +- | 54 | hw/ssi/mss-spi.c | 8 ++++- |
61 | include/qemu/osdep.h | 10 +- | 55 | hw/ssi/xilinx_spips.c | 43 +++++++++++++++------- |
62 | scripts/cocci-macro-file.h | 7 +- | 56 | hw/timer/pl031.c | 92 +++++++++++++++++++++++++++++++++++++++++++++--- |
63 | target/arm/cpu.h | 13 ++- | 57 | target/arm/cpu.c | 16 +++++++++ |
64 | hw/arm/aspeed.c | 8 ++ | 58 | target/arm/m_helper.c | 21 ++++++++--- |
65 | hw/arm/raspi.c | 7 ++ | 59 | 9 files changed, 174 insertions(+), 26 deletions(-) |
66 | hw/arm/virt.c | 202 ++++++++++++++++++++++------------ | ||
67 | hw/block/pflash_cfi01.c | 28 +++++ | ||
68 | hw/i386/pc_sysfw.c | 18 +-- | ||
69 | hw/intc/armv7m_nvic.c | 40 ++++++- | ||
70 | qga/commands-win32.c | 2 +- | ||
71 | target/arm/helper.c | 47 +++++++- | ||
72 | target/arm/kvm.c | 8 ++ | ||
73 | target/arm/kvm32.c | 20 +--- | ||
74 | target/arm/kvm64.c | 2 + | ||
75 | target/arm/machine.c | 2 +- | ||
76 | util/cacheinfo.c | 2 +- | ||
77 | 21 files changed, 294 insertions(+), 130 deletions(-) | ||
78 | 60 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Markus Armbruster <armbru@redhat.com> | ||
2 | 1 | ||
3 | The loop does two things: map legacy -drive to properties, and collect | ||
4 | all the backends for use after the loop. The next patch will factor | ||
5 | out the former for reuse in hw/arm/virt.c. To make that easier, | ||
6 | rearrange the loop so it does the first thing first, and the second | ||
7 | thing second. | ||
8 | |||
9 | Signed-off-by: Markus Armbruster <armbru@redhat.com> | ||
10 | Reviewed-by: Laszlo Ersek <lersek@redhat.com> | ||
11 | Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> | ||
12 | Message-id: 20190416091348.26075-2-armbru@redhat.com | ||
13 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | ||
14 | --- | ||
15 | hw/i386/pc_sysfw.c | 24 +++++++++++------------- | ||
16 | 1 file changed, 11 insertions(+), 13 deletions(-) | ||
17 | |||
18 | diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c | ||
19 | index XXXXXXX..XXXXXXX 100644 | ||
20 | --- a/hw/i386/pc_sysfw.c | ||
21 | +++ b/hw/i386/pc_sysfw.c | ||
22 | @@ -XXX,XX +XXX,XX @@ void pc_system_firmware_init(PCMachineState *pcms, | ||
23 | |||
24 | /* Map legacy -drive if=pflash to machine properties */ | ||
25 | for (i = 0; i < ARRAY_SIZE(pcms->flash); i++) { | ||
26 | - pflash_blk[i] = pflash_cfi01_get_blk(pcms->flash[i]); | ||
27 | pflash_drv = drive_get(IF_PFLASH, 0, i); | ||
28 | - if (!pflash_drv) { | ||
29 | - continue; | ||
30 | + if (pflash_drv) { | ||
31 | + loc_push_none(&loc); | ||
32 | + qemu_opts_loc_restore(pflash_drv->opts); | ||
33 | + if (pflash_cfi01_get_blk(pcms->flash[i])) { | ||
34 | + error_report("clashes with -machine"); | ||
35 | + exit(1); | ||
36 | + } | ||
37 | + qdev_prop_set_drive(DEVICE(pcms->flash[i]), "drive", | ||
38 | + blk_by_legacy_dinfo(pflash_drv), &error_fatal); | ||
39 | + loc_pop(&loc); | ||
40 | } | ||
41 | - loc_push_none(&loc); | ||
42 | - qemu_opts_loc_restore(pflash_drv->opts); | ||
43 | - if (pflash_blk[i]) { | ||
44 | - error_report("clashes with -machine"); | ||
45 | - exit(1); | ||
46 | - } | ||
47 | - pflash_blk[i] = blk_by_legacy_dinfo(pflash_drv); | ||
48 | - qdev_prop_set_drive(DEVICE(pcms->flash[i]), | ||
49 | - "drive", pflash_blk[i], &error_fatal); | ||
50 | - loc_pop(&loc); | ||
51 | + pflash_blk[i] = pflash_cfi01_get_blk(pcms->flash[i]); | ||
52 | } | ||
53 | |||
54 | /* Reject gaps */ | ||
55 | -- | ||
56 | 2.20.1 | ||
57 | |||
58 | diff view generated by jsdifflib |
1 | From: Cao Jiaxi <driver1998@foxmail.com> | 1 | From: Alex Bennée <alex.bennee@linaro.org> |
---|---|---|---|
2 | 2 | ||
3 | I encountered the following compilation error on mingw: | 3 | When we converted to using feature bits in 602f6e42cfbf we missed out |
4 | the fact (dp && arm_dc_feature(s, ARM_FEATURE_V8)) was supported for | ||
5 | -cpu max configurations. This caused a regression in the GCC test | ||
6 | suite. Fix this by setting the appropriate bits in mvfr1.FPHP to | ||
7 | report ARMv8-A with FP support (but not ARMv8.2-FP16). | ||
4 | 8 | ||
5 | /mnt/d/qemu/include/qemu/osdep.h:97:9: error: '__USE_MINGW_ANSI_STDIO' macro redefined [-Werror,-Wmacro-redefined] | 9 | Fixes: https://bugs.launchpad.net/qemu/+bug/1836078 |
6 | #define __USE_MINGW_ANSI_STDIO 1 | 10 | Signed-off-by: Alex Bennée <alex.bennee@linaro.org> |
7 | ^ | 11 | Reviewed-by: Richard Henderson <richard.henderson@linaro.org> |
8 | /mnt/d/llvm-mingw/aarch64-w64-mingw32/include/_mingw.h:433:9: note: previous definition is here | 12 | Message-id: 20190711103737.10017-1-alex.bennee@linaro.org |
9 | #define __USE_MINGW_ANSI_STDIO 0 /* was not defined so it should be 0 */ | ||
10 | |||
11 | It turns out that __USE_MINGW_ANSI_STDIO must be set before any | ||
12 | system headers are included, not just before stdio.h. | ||
13 | |||
14 | Signed-off-by: Cao Jiaxi <driver1998@foxmail.com> | ||
15 | Reviewed-by: Thomas Huth <thuth@redhat.com> | ||
16 | Reviewed-by: Stefan Weil <sw@weilnetz.de> | ||
17 | Message-id: 20190503003719.10233-1-driver1998@foxmail.com | ||
18 | Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> | ||
19 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | 13 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> |
20 | --- | 14 | --- |
21 | include/qemu/osdep.h | 10 +++++----- | 15 | target/arm/cpu.c | 4 ++++ |
22 | 1 file changed, 5 insertions(+), 5 deletions(-) | 16 | 1 file changed, 4 insertions(+) |
23 | 17 | ||
24 | diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h | 18 | diff --git a/target/arm/cpu.c b/target/arm/cpu.c |
25 | index XXXXXXX..XXXXXXX 100644 | 19 | index XXXXXXX..XXXXXXX 100644 |
26 | --- a/include/qemu/osdep.h | 20 | --- a/target/arm/cpu.c |
27 | +++ b/include/qemu/osdep.h | 21 | +++ b/target/arm/cpu.c |
28 | @@ -XXX,XX +XXX,XX @@ extern int daemon(int, int); | 22 | @@ -XXX,XX +XXX,XX @@ static void arm_max_initfn(Object *obj) |
29 | #endif | 23 | t = FIELD_DP32(t, ID_ISAR6, SPECRES, 1); |
30 | #endif | 24 | cpu->isar.id_isar6 = t; |
31 | 25 | ||
32 | +/* enable C99/POSIX format strings (needs mingw32-runtime 3.15 or later) */ | 26 | + t = cpu->isar.mvfr1; |
33 | +#ifdef __MINGW32__ | 27 | + t = FIELD_DP32(t, MVFR1, FPHP, 2); /* v8.0 FP support */ |
34 | +#define __USE_MINGW_ANSI_STDIO 1 | 28 | + cpu->isar.mvfr1 = t; |
35 | +#endif | ||
36 | + | 29 | + |
37 | #include <stdarg.h> | 30 | t = cpu->isar.mvfr2; |
38 | #include <stddef.h> | 31 | t = FIELD_DP32(t, MVFR2, SIMDMISC, 3); /* SIMD MaxNum */ |
39 | #include <stdbool.h> | 32 | t = FIELD_DP32(t, MVFR2, FPMISC, 4); /* FP MaxNum */ |
40 | #include <stdint.h> | ||
41 | #include <sys/types.h> | ||
42 | #include <stdlib.h> | ||
43 | - | ||
44 | -/* enable C99/POSIX format strings (needs mingw32-runtime 3.15 or later) */ | ||
45 | -#ifdef __MINGW32__ | ||
46 | -#define __USE_MINGW_ANSI_STDIO 1 | ||
47 | -#endif | ||
48 | #include <stdio.h> | ||
49 | |||
50 | #include <string.h> | ||
51 | -- | 33 | -- |
52 | 2.20.1 | 34 | 2.20.1 |
53 | 35 | ||
54 | 36 | diff view generated by jsdifflib |
1 | In the M-profile architecture, if the CPU implements the DSP extension | 1 | From: Philippe Mathieu-Daudé <philmd@redhat.com> |
---|---|---|---|
2 | then the XPSR has GE bits, in the same way as the A-profile CPSR. When | ||
3 | we added DSP extension support we forgot to add support for reading | ||
4 | and writing the GE bits, which are stored in env->GE. We did put in | ||
5 | the code to add XPSR_GE to the mask of bits to update in the v7m_msr | ||
6 | helper, but forgot it in v7m_mrs. We also must not allow the XPSR we | ||
7 | pull off the stack on exception return to set the nonexistent GE bits. | ||
8 | Correct these errors: | ||
9 | * read and write env->GE in xpsr_read() and xpsr_write() | ||
10 | * only set GE bits on exception return if DSP present | ||
11 | * read GE bits for MRS if DSP present | ||
12 | 2 | ||
3 | In the next commit we will implement the write_with_attrs() | ||
4 | handler. To avoid using different APIs, convert the read() | ||
5 | handler first. | ||
6 | |||
7 | Reviewed-by: Francisco Iglesias <frasse.iglesias@gmail.com> | ||
8 | Tested-by: Francisco Iglesias <frasse.iglesias@gmail.com> | ||
9 | Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> | ||
13 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | 10 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> |
14 | Reviewed-by: Richard Henderson <richard.henderson@linaro.org> | ||
15 | Message-id: 20190430131439.25251-5-peter.maydell@linaro.org | ||
16 | --- | 11 | --- |
17 | target/arm/cpu.h | 4 ++++ | 12 | hw/ssi/xilinx_spips.c | 23 +++++++++++------------ |
18 | target/arm/helper.c | 12 ++++++++++-- | 13 | 1 file changed, 11 insertions(+), 12 deletions(-) |
19 | 2 files changed, 14 insertions(+), 2 deletions(-) | ||
20 | 14 | ||
21 | diff --git a/target/arm/cpu.h b/target/arm/cpu.h | 15 | diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c |
22 | index XXXXXXX..XXXXXXX 100644 | 16 | index XXXXXXX..XXXXXXX 100644 |
23 | --- a/target/arm/cpu.h | 17 | --- a/hw/ssi/xilinx_spips.c |
24 | +++ b/target/arm/cpu.h | 18 | +++ b/hw/ssi/xilinx_spips.c |
25 | @@ -XXX,XX +XXX,XX @@ static inline uint32_t xpsr_read(CPUARMState *env) | 19 | @@ -XXX,XX +XXX,XX @@ static void lqspi_load_cache(void *opaque, hwaddr addr) |
26 | | (env->CF << 29) | ((env->VF & 0x80000000) >> 3) | (env->QF << 27) | 20 | } |
27 | | (env->thumb << 24) | ((env->condexec_bits & 3) << 25) | ||
28 | | ((env->condexec_bits & 0xfc) << 8) | ||
29 | + | (env->GE << 16) | ||
30 | | env->v7m.exception; | ||
31 | } | 21 | } |
32 | 22 | ||
33 | @@ -XXX,XX +XXX,XX @@ static inline void xpsr_write(CPUARMState *env, uint32_t val, uint32_t mask) | 23 | -static uint64_t |
34 | if (mask & XPSR_Q) { | 24 | -lqspi_read(void *opaque, hwaddr addr, unsigned int size) |
35 | env->QF = ((val & XPSR_Q) != 0); | 25 | +static MemTxResult lqspi_read(void *opaque, hwaddr addr, uint64_t *value, |
36 | } | 26 | + unsigned size, MemTxAttrs attrs) |
37 | + if (mask & XPSR_GE) { | ||
38 | + env->GE = (val & XPSR_GE) >> 16; | ||
39 | + } | ||
40 | if (mask & XPSR_T) { | ||
41 | env->thumb = ((val & XPSR_T) != 0); | ||
42 | } | ||
43 | diff --git a/target/arm/helper.c b/target/arm/helper.c | ||
44 | index XXXXXXX..XXXXXXX 100644 | ||
45 | --- a/target/arm/helper.c | ||
46 | +++ b/target/arm/helper.c | ||
47 | @@ -XXX,XX +XXX,XX @@ static void do_v7m_exception_exit(ARMCPU *cpu) | ||
48 | { | 27 | { |
49 | CPUARMState *env = &cpu->env; | 28 | - XilinxQSPIPS *q = opaque; |
50 | uint32_t excret; | 29 | - uint32_t ret; |
51 | - uint32_t xpsr; | 30 | + XilinxQSPIPS *q = XILINX_QSPIPS(opaque); |
52 | + uint32_t xpsr, xpsr_mask; | 31 | |
53 | bool ufault = false; | 32 | if (addr >= q->lqspi_cached_addr && |
54 | bool sfault = false; | 33 | addr <= q->lqspi_cached_addr + LQSPI_CACHE_SIZE - 4) { |
55 | bool return_to_sp_process; | 34 | uint8_t *retp = &q->lqspi_buf[addr - q->lqspi_cached_addr]; |
56 | @@ -XXX,XX +XXX,XX @@ static void do_v7m_exception_exit(ARMCPU *cpu) | 35 | - ret = cpu_to_le32(*(uint32_t *)retp); |
57 | } | 36 | - DB_PRINT_L(1, "addr: %08x, data: %08x\n", (unsigned)addr, |
58 | *frame_sp_p = frameptr; | 37 | - (unsigned)ret); |
38 | - return ret; | ||
39 | - } else { | ||
40 | - lqspi_load_cache(opaque, addr); | ||
41 | - return lqspi_read(opaque, addr, size); | ||
42 | + *value = cpu_to_le32(*(uint32_t *)retp); | ||
43 | + DB_PRINT_L(1, "addr: %08" HWADDR_PRIx ", data: %08" PRIx64 "\n", | ||
44 | + addr, *value); | ||
45 | + return MEMTX_OK; | ||
59 | } | 46 | } |
60 | + | 47 | + |
61 | + xpsr_mask = ~(XPSR_SPREALIGN | XPSR_SFPA); | 48 | + lqspi_load_cache(opaque, addr); |
62 | + if (!arm_feature(env, ARM_FEATURE_THUMB_DSP)) { | 49 | + return lqspi_read(opaque, addr, value, size, attrs); |
63 | + xpsr_mask &= ~XPSR_GE; | 50 | } |
64 | + } | 51 | |
65 | /* This xpsr_write() will invalidate frame_sp_p as it may switch stack */ | 52 | static const MemoryRegionOps lqspi_ops = { |
66 | - xpsr_write(env, xpsr, ~(XPSR_SPREALIGN | XPSR_SFPA)); | 53 | - .read = lqspi_read, |
67 | + xpsr_write(env, xpsr, xpsr_mask); | 54 | + .read_with_attrs = lqspi_read, |
68 | 55 | .endianness = DEVICE_NATIVE_ENDIAN, | |
69 | if (env->v7m.secure) { | 56 | .valid = { |
70 | bool sfpa = xpsr & XPSR_SFPA; | 57 | .min_access_size = 1, |
71 | @@ -XXX,XX +XXX,XX @@ uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg) | ||
72 | } | ||
73 | if (!(reg & 4)) { | ||
74 | mask |= XPSR_NZCV | XPSR_Q; /* APSR */ | ||
75 | + if (arm_feature(env, ARM_FEATURE_THUMB_DSP)) { | ||
76 | + mask |= XPSR_GE; | ||
77 | + } | ||
78 | } | ||
79 | /* EPSR reads as zero */ | ||
80 | return xpsr_read(env) & mask; | ||
81 | -- | 58 | -- |
82 | 2.20.1 | 59 | 2.20.1 |
83 | 60 | ||
84 | 61 | diff view generated by jsdifflib |
1 | From: Markus Armbruster <armbru@redhat.com> | 1 | From: Philippe Mathieu-Daudé <philmd@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | Factored out of pc_system_firmware_init() so the next commit can reuse | 3 | Lei Sun found while auditing the code that a CPU write would |
4 | it in hw/arm/virt.c. | 4 | trigger a NULL pointer dereference. |
5 | 5 | ||
6 | Signed-off-by: Markus Armbruster <armbru@redhat.com> | 6 | >From UG1085 datasheet [*] AXI writes in this region are ignored |
7 | Reviewed-by: Laszlo Ersek <lersek@redhat.com> | 7 | and generates an AXI Slave Error (SLVERR). |
8 | Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> | 8 | |
9 | Message-id: 20190416091348.26075-3-armbru@redhat.com | 9 | Fix by implementing the write_with_attrs() handler. |
10 | Return MEMTX_ERROR when the region is accessed (this error maps | ||
11 | to an AXI slave error). | ||
12 | |||
13 | [*] https://www.xilinx.com/support/documentation/user_guides/ug1085-zynq-ultrascale-trm.pdf | ||
14 | |||
15 | Reported-by: Lei Sun <slei.casper@gmail.com> | ||
16 | Reviewed-by: Francisco Iglesias <frasse.iglesias@gmail.com> | ||
17 | Tested-by: Francisco Iglesias <frasse.iglesias@gmail.com> | ||
18 | Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> | ||
10 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | 19 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> |
11 | --- | 20 | --- |
12 | include/hw/block/flash.h | 1 + | 21 | hw/ssi/xilinx_spips.c | 16 ++++++++++++++++ |
13 | hw/block/pflash_cfi01.c | 28 ++++++++++++++++++++++++++++ | 22 | 1 file changed, 16 insertions(+) |
14 | hw/i386/pc_sysfw.c | 16 ++-------------- | ||
15 | 3 files changed, 31 insertions(+), 14 deletions(-) | ||
16 | 23 | ||
17 | diff --git a/include/hw/block/flash.h b/include/hw/block/flash.h | 24 | diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c |
18 | index XXXXXXX..XXXXXXX 100644 | 25 | index XXXXXXX..XXXXXXX 100644 |
19 | --- a/include/hw/block/flash.h | 26 | --- a/hw/ssi/xilinx_spips.c |
20 | +++ b/include/hw/block/flash.h | 27 | +++ b/hw/ssi/xilinx_spips.c |
21 | @@ -XXX,XX +XXX,XX @@ PFlashCFI01 *pflash_cfi01_register(hwaddr base, | 28 | @@ -XXX,XX +XXX,XX @@ static MemTxResult lqspi_read(void *opaque, hwaddr addr, uint64_t *value, |
22 | int be); | 29 | return lqspi_read(opaque, addr, value, size, attrs); |
23 | BlockBackend *pflash_cfi01_get_blk(PFlashCFI01 *fl); | ||
24 | MemoryRegion *pflash_cfi01_get_memory(PFlashCFI01 *fl); | ||
25 | +void pflash_cfi01_legacy_drive(PFlashCFI01 *dev, DriveInfo *dinfo); | ||
26 | |||
27 | /* pflash_cfi02.c */ | ||
28 | |||
29 | diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c | ||
30 | index XXXXXXX..XXXXXXX 100644 | ||
31 | --- a/hw/block/pflash_cfi01.c | ||
32 | +++ b/hw/block/pflash_cfi01.c | ||
33 | @@ -XXX,XX +XXX,XX @@ | ||
34 | #include "qapi/error.h" | ||
35 | #include "qemu/timer.h" | ||
36 | #include "qemu/bitops.h" | ||
37 | +#include "qemu/error-report.h" | ||
38 | #include "qemu/host-utils.h" | ||
39 | #include "qemu/log.h" | ||
40 | +#include "qemu/option.h" | ||
41 | #include "hw/sysbus.h" | ||
42 | +#include "sysemu/blockdev.h" | ||
43 | #include "sysemu/sysemu.h" | ||
44 | #include "trace.h" | ||
45 | |||
46 | @@ -XXX,XX +XXX,XX @@ MemoryRegion *pflash_cfi01_get_memory(PFlashCFI01 *fl) | ||
47 | return &fl->mem; | ||
48 | } | 30 | } |
49 | 31 | ||
50 | +/* | 32 | +static MemTxResult lqspi_write(void *opaque, hwaddr offset, uint64_t value, |
51 | + * Handle -drive if=pflash for machines that use properties. | 33 | + unsigned size, MemTxAttrs attrs) |
52 | + * If @dinfo is null, do nothing. | ||
53 | + * Else if @fl's property "drive" is already set, fatal error. | ||
54 | + * Else set it to the BlockBackend with @dinfo. | ||
55 | + */ | ||
56 | +void pflash_cfi01_legacy_drive(PFlashCFI01 *fl, DriveInfo *dinfo) | ||
57 | +{ | 34 | +{ |
58 | + Location loc; | 35 | + /* |
36 | + * From UG1085, Chapter 24 (Quad-SPI controllers): | ||
37 | + * - Writes are ignored | ||
38 | + * - AXI writes generate an external AXI slave error (SLVERR) | ||
39 | + */ | ||
40 | + qemu_log_mask(LOG_GUEST_ERROR, "%s Unexpected %u-bit access to 0x%" PRIx64 | ||
41 | + " (value: 0x%" PRIx64 "\n", | ||
42 | + __func__, size << 3, offset, value); | ||
59 | + | 43 | + |
60 | + if (!dinfo) { | 44 | + return MEMTX_ERROR; |
61 | + return; | ||
62 | + } | ||
63 | + | ||
64 | + loc_push_none(&loc); | ||
65 | + qemu_opts_loc_restore(dinfo->opts); | ||
66 | + if (fl->blk) { | ||
67 | + error_report("clashes with -machine"); | ||
68 | + exit(1); | ||
69 | + } | ||
70 | + qdev_prop_set_drive(DEVICE(fl), "drive", | ||
71 | + blk_by_legacy_dinfo(dinfo), &error_fatal); | ||
72 | + loc_pop(&loc); | ||
73 | +} | 45 | +} |
74 | + | 46 | + |
75 | static void postload_update_cb(void *opaque, int running, RunState state) | 47 | static const MemoryRegionOps lqspi_ops = { |
76 | { | 48 | .read_with_attrs = lqspi_read, |
77 | PFlashCFI01 *pfl = opaque; | 49 | + .write_with_attrs = lqspi_write, |
78 | diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c | 50 | .endianness = DEVICE_NATIVE_ENDIAN, |
79 | index XXXXXXX..XXXXXXX 100644 | 51 | .valid = { |
80 | --- a/hw/i386/pc_sysfw.c | 52 | .min_access_size = 1, |
81 | +++ b/hw/i386/pc_sysfw.c | ||
82 | @@ -XXX,XX +XXX,XX @@ void pc_system_firmware_init(PCMachineState *pcms, | ||
83 | { | ||
84 | PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms); | ||
85 | int i; | ||
86 | - DriveInfo *pflash_drv; | ||
87 | BlockBackend *pflash_blk[ARRAY_SIZE(pcms->flash)]; | ||
88 | - Location loc; | ||
89 | |||
90 | if (!pcmc->pci_enabled) { | ||
91 | old_pc_system_rom_init(rom_memory, true); | ||
92 | @@ -XXX,XX +XXX,XX @@ void pc_system_firmware_init(PCMachineState *pcms, | ||
93 | |||
94 | /* Map legacy -drive if=pflash to machine properties */ | ||
95 | for (i = 0; i < ARRAY_SIZE(pcms->flash); i++) { | ||
96 | - pflash_drv = drive_get(IF_PFLASH, 0, i); | ||
97 | - if (pflash_drv) { | ||
98 | - loc_push_none(&loc); | ||
99 | - qemu_opts_loc_restore(pflash_drv->opts); | ||
100 | - if (pflash_cfi01_get_blk(pcms->flash[i])) { | ||
101 | - error_report("clashes with -machine"); | ||
102 | - exit(1); | ||
103 | - } | ||
104 | - qdev_prop_set_drive(DEVICE(pcms->flash[i]), "drive", | ||
105 | - blk_by_legacy_dinfo(pflash_drv), &error_fatal); | ||
106 | - loc_pop(&loc); | ||
107 | - } | ||
108 | + pflash_cfi01_legacy_drive(pcms->flash[i], | ||
109 | + drive_get(IF_PFLASH, 0, i)); | ||
110 | pflash_blk[i] = pflash_cfi01_get_blk(pcms->flash[i]); | ||
111 | } | ||
112 | |||
113 | -- | 53 | -- |
114 | 2.20.1 | 54 | 2.20.1 |
115 | 55 | ||
116 | 56 | diff view generated by jsdifflib |
1 | From: Cao Jiaxi <driver1998@foxmail.com> | 1 | From: Philippe Mathieu-Daudé <philmd@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | Windows ARM64 uses LLP64 model, which breaks current assumptions. | 3 | Both lqspi_read() and lqspi_load_cache() expect a 32-bit |
4 | aligned address. | ||
4 | 5 | ||
5 | Signed-off-by: Cao Jiaxi <driver1998@foxmail.com> | 6 | >From UG1085 datasheet [*] chapter on 'Quad-SPI Controller': |
6 | Reviewed-by: Richard Henderson <richard.henderson@linaro.org> | 7 | |
7 | Reviewed-by: Thomas Huth <thuth@redhat.com> | 8 | Transfer Size Limitations |
8 | Message-id: 20190503003707.10185-1-driver1998@foxmail.com | 9 | |
9 | Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> | 10 | Because of the 32-bit wide TX, RX, and generic FIFO, all |
10 | Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com> | 11 | APB/AXI transfers must be an integer multiple of 4-bytes. |
12 | Shorter transfers are not possible. | ||
13 | |||
14 | Set MemoryRegionOps.impl values to force 32-bit accesses, | ||
15 | this way we are sure we do not access the lqspi_buf[] array | ||
16 | out of bound. | ||
17 | |||
18 | [*] https://www.xilinx.com/support/documentation/user_guides/ug1085-zynq-ultrascale-trm.pdf | ||
19 | |||
20 | Reviewed-by: Francisco Iglesias <frasse.iglesias@gmail.com> | ||
21 | Tested-by: Francisco Iglesias <frasse.iglesias@gmail.com> | ||
22 | Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> | ||
11 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | 23 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> |
12 | --- | 24 | --- |
13 | util/cacheinfo.c | 2 +- | 25 | hw/ssi/xilinx_spips.c | 4 ++++ |
14 | 1 file changed, 1 insertion(+), 1 deletion(-) | 26 | 1 file changed, 4 insertions(+) |
15 | 27 | ||
16 | diff --git a/util/cacheinfo.c b/util/cacheinfo.c | 28 | diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c |
17 | index XXXXXXX..XXXXXXX 100644 | 29 | index XXXXXXX..XXXXXXX 100644 |
18 | --- a/util/cacheinfo.c | 30 | --- a/hw/ssi/xilinx_spips.c |
19 | +++ b/util/cacheinfo.c | 31 | +++ b/hw/ssi/xilinx_spips.c |
20 | @@ -XXX,XX +XXX,XX @@ static void sys_cache_info(int *isize, int *dsize) | 32 | @@ -XXX,XX +XXX,XX @@ static const MemoryRegionOps lqspi_ops = { |
21 | static void arch_cache_info(int *isize, int *dsize) | 33 | .read_with_attrs = lqspi_read, |
22 | { | 34 | .write_with_attrs = lqspi_write, |
23 | if (*isize == 0 || *dsize == 0) { | 35 | .endianness = DEVICE_NATIVE_ENDIAN, |
24 | - unsigned long ctr; | 36 | + .impl = { |
25 | + uint64_t ctr; | 37 | + .min_access_size = 4, |
26 | 38 | + .max_access_size = 4, | |
27 | /* The real cache geometry is in CCSIDR_EL1/CLIDR_EL1/CSSELR_EL1, | 39 | + }, |
28 | but (at least under Linux) these are marked protected by the | 40 | .valid = { |
41 | .min_access_size = 1, | ||
42 | .max_access_size = 4 | ||
29 | -- | 43 | -- |
30 | 2.20.1 | 44 | 2.20.1 |
31 | 45 | ||
32 | 46 | diff view generated by jsdifflib |
1 | From: Cao Jiaxi <driver1998@foxmail.com> | 1 | From: Philippe Mathieu-Daudé <philmd@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | The win2qemu[] is supposed to be the conversion table to convert between | 3 | Reading the RX_DATA register when the RX_FIFO is empty triggers |
4 | STORAGE_BUS_TYPE in Windows SDK and GuestDiskBusType in qga. | 4 | an abort. This can be easily reproduced: |
5 | 5 | ||
6 | But it was incorrectly written that it forces to set a GuestDiskBusType | 6 | $ qemu-system-arm -M emcraft-sf2 -monitor stdio -S |
7 | value to STORAGE_BUS_TYPE, which generates an enum conversion warning in clang. | 7 | QEMU 4.0.50 monitor - type 'help' for more information |
8 | (qemu) x 0x40001010 | ||
9 | Aborted (core dumped) | ||
8 | 10 | ||
9 | Suggested-by: Eric Blake <eblake@redhat.com> | 11 | (gdb) bt |
10 | Signed-off-by: Cao Jiaxi <driver1998@foxmail.com> | 12 | #1 0x00007f035874f895 in abort () at /lib64/libc.so.6 |
11 | Reviewed-by: Richard Henderson <richard.henderson@linaro.org> | 13 | #2 0x00005628686591ff in fifo8_pop (fifo=0x56286a9a4c68) at util/fifo8.c:66 |
12 | Reviewed-by: Thomas Huth <thuth@redhat.com> | 14 | #3 0x00005628683e0b8e in fifo32_pop (fifo=0x56286a9a4c68) at include/qemu/fifo32.h:137 |
13 | Message-id: 20190503003650.10137-1-driver1998@foxmail.com | 15 | #4 0x00005628683e0efb in spi_read (opaque=0x56286a9a4850, addr=4, size=4) at hw/ssi/mss-spi.c:168 |
14 | Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> | 16 | #5 0x0000562867f96801 in memory_region_read_accessor (mr=0x56286a9a4b60, addr=16, value=0x7ffeecb0c5c8, size=4, shift=0, mask=4294967295, attrs=...) at memory.c:439 |
15 | Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com> | 17 | #6 0x0000562867f96cdb in access_with_adjusted_size (addr=16, value=0x7ffeecb0c5c8, size=4, access_size_min=1, access_size_max=4, access_fn=0x562867f967c3 <memory_region_read_accessor>, mr=0x56286a9a4b60, attrs=...) at memory.c:569 |
18 | #7 0x0000562867f99940 in memory_region_dispatch_read1 (mr=0x56286a9a4b60, addr=16, pval=0x7ffeecb0c5c8, size=4, attrs=...) at memory.c:1420 | ||
19 | #8 0x0000562867f99a08 in memory_region_dispatch_read (mr=0x56286a9a4b60, addr=16, pval=0x7ffeecb0c5c8, size=4, attrs=...) at memory.c:1447 | ||
20 | #9 0x0000562867f38721 in flatview_read_continue (fv=0x56286aec6360, addr=1073745936, attrs=..., buf=0x7ffeecb0c7c0 "\340ǰ\354\376\177", len=4, addr1=16, l=4, mr=0x56286a9a4b60) at exec.c:3385 | ||
21 | #10 0x0000562867f38874 in flatview_read (fv=0x56286aec6360, addr=1073745936, attrs=..., buf=0x7ffeecb0c7c0 "\340ǰ\354\376\177", len=4) at exec.c:3423 | ||
22 | #11 0x0000562867f388ea in address_space_read_full (as=0x56286aa3e890, addr=1073745936, attrs=..., buf=0x7ffeecb0c7c0 "\340ǰ\354\376\177", len=4) at exec.c:3436 | ||
23 | #12 0x0000562867f389c5 in address_space_rw (as=0x56286aa3e890, addr=1073745936, attrs=..., buf=0x7ffeecb0c7c0 "\340ǰ\354\376\177", len=4, is_write=false) at exec.c:3466 | ||
24 | #13 0x0000562867f3bdd7 in cpu_memory_rw_debug (cpu=0x56286aa19d00, addr=1073745936, buf=0x7ffeecb0c7c0 "\340ǰ\354\376\177", len=4, is_write=0) at exec.c:3976 | ||
25 | #14 0x000056286811ed51 in memory_dump (mon=0x56286a8c32d0, count=1, format=120, wsize=4, addr=1073745936, is_physical=0) at monitor/misc.c:730 | ||
26 | #15 0x000056286811eff1 in hmp_memory_dump (mon=0x56286a8c32d0, qdict=0x56286b15c400) at monitor/misc.c:785 | ||
27 | #16 0x00005628684740ee in handle_hmp_command (mon=0x56286a8c32d0, cmdline=0x56286a8caeb2 "0x40001010") at monitor/hmp.c:1082 | ||
28 | |||
29 | From the datasheet "Actel SmartFusion Microcontroller Subsystem | ||
30 | User's Guide" Rev.1, Table 13-3 "SPI Register Summary", this | ||
31 | register has a reset value of 0. | ||
32 | |||
33 | Check the FIFO is not empty before accessing it, else log an | ||
34 | error message. | ||
35 | |||
36 | Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> | ||
37 | Reviewed-by: Alistair Francis <alistair.francis@wdc.com> | ||
38 | Message-id: 20190709113715.7761-3-philmd@redhat.com | ||
16 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | 39 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> |
17 | --- | 40 | --- |
18 | qga/commands-win32.c | 2 +- | 41 | hw/ssi/mss-spi.c | 8 +++++++- |
19 | 1 file changed, 1 insertion(+), 1 deletion(-) | 42 | 1 file changed, 7 insertions(+), 1 deletion(-) |
20 | 43 | ||
21 | diff --git a/qga/commands-win32.c b/qga/commands-win32.c | 44 | diff --git a/hw/ssi/mss-spi.c b/hw/ssi/mss-spi.c |
22 | index XXXXXXX..XXXXXXX 100644 | 45 | index XXXXXXX..XXXXXXX 100644 |
23 | --- a/qga/commands-win32.c | 46 | --- a/hw/ssi/mss-spi.c |
24 | +++ b/qga/commands-win32.c | 47 | +++ b/hw/ssi/mss-spi.c |
25 | @@ -XXX,XX +XXX,XX @@ void qmp_guest_file_flush(int64_t handle, Error **errp) | 48 | @@ -XXX,XX +XXX,XX @@ spi_read(void *opaque, hwaddr addr, unsigned int size) |
26 | 49 | case R_SPI_RX: | |
27 | #ifdef CONFIG_QGA_NTDDSCSI | 50 | s->regs[R_SPI_STATUS] &= ~S_RXFIFOFUL; |
28 | 51 | s->regs[R_SPI_STATUS] &= ~S_RXCHOVRF; | |
29 | -static STORAGE_BUS_TYPE win2qemu[] = { | 52 | - ret = fifo32_pop(&s->rx_fifo); |
30 | +static GuestDiskBusType win2qemu[] = { | 53 | + if (fifo32_is_empty(&s->rx_fifo)) { |
31 | [BusTypeUnknown] = GUEST_DISK_BUS_TYPE_UNKNOWN, | 54 | + qemu_log_mask(LOG_GUEST_ERROR, |
32 | [BusTypeScsi] = GUEST_DISK_BUS_TYPE_SCSI, | 55 | + "%s: Reading empty RX_FIFO\n", |
33 | [BusTypeAtapi] = GUEST_DISK_BUS_TYPE_IDE, | 56 | + __func__); |
57 | + } else { | ||
58 | + ret = fifo32_pop(&s->rx_fifo); | ||
59 | + } | ||
60 | if (fifo32_is_empty(&s->rx_fifo)) { | ||
61 | s->regs[R_SPI_STATUS] |= S_RXFIFOEMP; | ||
62 | } | ||
34 | -- | 63 | -- |
35 | 2.20.1 | 64 | 2.20.1 |
36 | 65 | ||
37 | 66 | diff view generated by jsdifflib |
1 | From: Cao Jiaxi <driver1998@foxmail.com> | 1 | From: Philippe Mathieu-Daudé <philmd@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | gcc_struct is for x86 only, and it generates an warning on ARM64 Clang/MinGW targets. | 3 | In the previous commit we fixed a crash when the guest read a |
4 | register that pop from an empty FIFO. | ||
5 | By auditing the repository, we found another similar use with | ||
6 | an easy way to reproduce: | ||
4 | 7 | ||
5 | Signed-off-by: Cao Jiaxi <driver1998@foxmail.com> | 8 | $ qemu-system-aarch64 -M xlnx-zcu102 -monitor stdio -S |
6 | Reviewed-by: Thomas Huth <thuth@redhat.com> | 9 | QEMU 4.0.50 monitor - type 'help' for more information |
7 | Message-id: 20190503003618.10089-1-driver1998@foxmail.com | 10 | (qemu) xp/b 0xfd4a0134 |
8 | [PMM: dropped the slirp change as slirp is now a submodule] | 11 | Aborted (core dumped) |
12 | |||
13 | (gdb) bt | ||
14 | #0 0x00007f6936dea57f in raise () at /lib64/libc.so.6 | ||
15 | #1 0x00007f6936dd4895 in abort () at /lib64/libc.so.6 | ||
16 | #2 0x0000561ad32975ec in xlnx_dp_aux_pop_rx_fifo (s=0x7f692babee70) at hw/display/xlnx_dp.c:431 | ||
17 | #3 0x0000561ad3297dc0 in xlnx_dp_read (opaque=0x7f692babee70, offset=77, size=4) at hw/display/xlnx_dp.c:667 | ||
18 | #4 0x0000561ad321b896 in memory_region_read_accessor (mr=0x7f692babf620, addr=308, value=0x7ffe05c1db88, size=4, shift=0, mask=4294967295, attrs=...) at memory.c:439 | ||
19 | #5 0x0000561ad321bd70 in access_with_adjusted_size (addr=308, value=0x7ffe05c1db88, size=1, access_size_min=4, access_size_max=4, access_fn=0x561ad321b858 <memory_region_read_accessor>, mr=0x7f692babf620, attrs=...) at memory.c:569 | ||
20 | #6 0x0000561ad321e9d5 in memory_region_dispatch_read1 (mr=0x7f692babf620, addr=308, pval=0x7ffe05c1db88, size=1, attrs=...) at memory.c:1420 | ||
21 | #7 0x0000561ad321ea9d in memory_region_dispatch_read (mr=0x7f692babf620, addr=308, pval=0x7ffe05c1db88, size=1, attrs=...) at memory.c:1447 | ||
22 | #8 0x0000561ad31bd742 in flatview_read_continue (fv=0x561ad69c04f0, addr=4249485620, attrs=..., buf=0x7ffe05c1dcf0 "\020\335\301\005\376\177", len=1, addr1=308, l=1, mr=0x7f692babf620) at exec.c:3385 | ||
23 | #9 0x0000561ad31bd895 in flatview_read (fv=0x561ad69c04f0, addr=4249485620, attrs=..., buf=0x7ffe05c1dcf0 "\020\335\301\005\376\177", len=1) at exec.c:3423 | ||
24 | #10 0x0000561ad31bd90b in address_space_read_full (as=0x561ad5bb3020, addr=4249485620, attrs=..., buf=0x7ffe05c1dcf0 "\020\335\301\005\376\177", len=1) at exec.c:3436 | ||
25 | #11 0x0000561ad33b1c42 in address_space_read (len=1, buf=0x7ffe05c1dcf0 "\020\335\301\005\376\177", attrs=..., addr=4249485620, as=0x561ad5bb3020) at include/exec/memory.h:2131 | ||
26 | #12 0x0000561ad33b1c42 in memory_dump (mon=0x561ad59c4530, count=1, format=120, wsize=1, addr=4249485620, is_physical=1) at monitor/misc.c:723 | ||
27 | #13 0x0000561ad33b1fc1 in hmp_physical_memory_dump (mon=0x561ad59c4530, qdict=0x561ad6c6fd00) at monitor/misc.c:795 | ||
28 | #14 0x0000561ad37b4a9f in handle_hmp_command (mon=0x561ad59c4530, cmdline=0x561ad59d0f22 "/b 0x00000000fd4a0134") at monitor/hmp.c:1082 | ||
29 | |||
30 | Fix by checking the FIFO is not empty before popping from it. | ||
31 | |||
32 | The datasheet is not clear about the reset value of this register, | ||
33 | we choose to return '0'. | ||
34 | |||
35 | Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> | ||
36 | Reviewed-by: Alistair Francis <alistair.francis@wdc.com> | ||
37 | Message-id: 20190709113715.7761-4-philmd@redhat.com | ||
9 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | 38 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> |
10 | --- | 39 | --- |
11 | contrib/libvhost-user/libvhost-user.h | 2 +- | 40 | hw/display/xlnx_dp.c | 15 +++++++++++---- |
12 | include/qemu/compiler.h | 2 +- | 41 | 1 file changed, 11 insertions(+), 4 deletions(-) |
13 | scripts/cocci-macro-file.h | 7 ++++++- | ||
14 | 3 files changed, 8 insertions(+), 3 deletions(-) | ||
15 | 42 | ||
16 | diff --git a/contrib/libvhost-user/libvhost-user.h b/contrib/libvhost-user/libvhost-user.h | 43 | diff --git a/hw/display/xlnx_dp.c b/hw/display/xlnx_dp.c |
17 | index XXXXXXX..XXXXXXX 100644 | 44 | index XXXXXXX..XXXXXXX 100644 |
18 | --- a/contrib/libvhost-user/libvhost-user.h | 45 | --- a/hw/display/xlnx_dp.c |
19 | +++ b/contrib/libvhost-user/libvhost-user.h | 46 | +++ b/hw/display/xlnx_dp.c |
20 | @@ -XXX,XX +XXX,XX @@ typedef struct VhostUserInflight { | 47 | @@ -XXX,XX +XXX,XX @@ static uint8_t xlnx_dp_aux_pop_rx_fifo(XlnxDPState *s) |
21 | uint16_t queue_size; | 48 | uint8_t ret; |
22 | } VhostUserInflight; | 49 | |
23 | 50 | if (fifo8_is_empty(&s->rx_fifo)) { | |
24 | -#if defined(_WIN32) | 51 | - DPRINTF("rx_fifo underflow..\n"); |
25 | +#if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__)) | 52 | - abort(); |
26 | # define VU_PACKED __attribute__((gcc_struct, packed)) | 53 | + qemu_log_mask(LOG_GUEST_ERROR, |
27 | #else | 54 | + "%s: Reading empty RX_FIFO\n", |
28 | # define VU_PACKED __attribute__((packed)) | 55 | + __func__); |
29 | diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h | 56 | + /* |
30 | index XXXXXXX..XXXXXXX 100644 | 57 | + * The datasheet is not clear about the reset value, it seems |
31 | --- a/include/qemu/compiler.h | 58 | + * to be unspecified. We choose to return '0'. |
32 | +++ b/include/qemu/compiler.h | 59 | + */ |
33 | @@ -XXX,XX +XXX,XX @@ | 60 | + ret = 0; |
34 | 61 | + } else { | |
35 | #define QEMU_SENTINEL __attribute__((sentinel)) | 62 | + ret = fifo8_pop(&s->rx_fifo); |
36 | 63 | + DPRINTF("pop 0x%" PRIX8 " from rx_fifo.\n", ret); | |
37 | -#if defined(_WIN32) | 64 | } |
38 | +#if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__)) | 65 | - ret = fifo8_pop(&s->rx_fifo); |
39 | # define QEMU_PACKED __attribute__((gcc_struct, packed)) | 66 | - DPRINTF("pop 0x%" PRIX8 " from rx_fifo.\n", ret); |
40 | #else | 67 | return ret; |
41 | # define QEMU_PACKED __attribute__((packed)) | 68 | } |
42 | diff --git a/scripts/cocci-macro-file.h b/scripts/cocci-macro-file.h | 69 | |
43 | index XXXXXXX..XXXXXXX 100644 | ||
44 | --- a/scripts/cocci-macro-file.h | ||
45 | +++ b/scripts/cocci-macro-file.h | ||
46 | @@ -XXX,XX +XXX,XX @@ | ||
47 | #define QEMU_NORETURN __attribute__ ((__noreturn__)) | ||
48 | #define QEMU_WARN_UNUSED_RESULT __attribute__((warn_unused_result)) | ||
49 | #define QEMU_SENTINEL __attribute__((sentinel)) | ||
50 | -#define QEMU_PACKED __attribute__((gcc_struct, packed)) | ||
51 | + | ||
52 | +#if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__)) | ||
53 | +# define QEMU_PACKED __attribute__((gcc_struct, packed)) | ||
54 | +#else | ||
55 | +# define QEMU_PACKED __attribute__((packed)) | ||
56 | +#endif | ||
57 | |||
58 | #define cat(x,y) x ## y | ||
59 | #define cat2(x,y) cat(x,y) | ||
60 | -- | 70 | -- |
61 | 2.20.1 | 71 | 2.20.1 |
62 | 72 | ||
63 | 73 | diff view generated by jsdifflib |
1 | From: Markus Armbruster <armbru@redhat.com> | 1 | From: David Engraf <david.engraf@sysgo.com> |
---|---|---|---|
2 | 2 | ||
3 | The ARM virt machines put firmware in flash memory. To configure it, | 3 | Using the whole 128 MiB flash in non-secure mode is not working because |
4 | you use -drive if=pflash,unit=0,... and optionally -drive | 4 | virt_flash_fdt() expects the same address for secure_sysmem and sysmem. |
5 | if=pflash,unit=1,... | 5 | This is not correctly handled by caller because it forwards NULL for |
6 | secure_sysmem in non-secure flash mode. | ||
6 | 7 | ||
7 | Why two -drive? This permits setting up one part of the flash memory | 8 | Fixed by using sysmem when secure_sysmem is NULL. |
8 | read-only, and the other part read/write. It also makes upgrading | ||
9 | firmware on the host easier. Below the hood, we get two separate | ||
10 | flash devices, because we were too lazy to improve our flash device | ||
11 | models to support sector protection. | ||
12 | 9 | ||
13 | The problem at hand is to do the same with -blockdev somehow, as one | 10 | Signed-off-by: David Engraf <david.engraf@sysgo.com> |
14 | more step towards deprecating -drive. | 11 | Message-id: 20190712075002.14326-1-david.engraf@sysgo.com |
15 | |||
16 | We recently solved this problem for x86 PC machines, in commit | ||
17 | ebc29e1beab. See the commit message for design rationale. | ||
18 | |||
19 | This commit solves it for ARM virt basically the same way: new machine | ||
20 | properties pflash0, pflash1 forward to the onboard flash devices' | ||
21 | properties. Requires creating the onboard devices in the | ||
22 | .instance_init() method virt_instance_init(). The existing code to | ||
23 | pick up drives defined with -drive if=pflash is replaced by code to | ||
24 | desugar into the machine properties. | ||
25 | |||
26 | There are a few behavioral differences, though: | ||
27 | |||
28 | * The flash devices are always present (x86: only present if | ||
29 | configured) | ||
30 | |||
31 | * Flash base addresses and sizes are fixed (x86: sizes depend on | ||
32 | images, mapped back to back below a fixed address) | ||
33 | |||
34 | * -bios configures contents of first pflash (x86: -bios configures ROM | ||
35 | contents) | ||
36 | |||
37 | * -bios is rejected when first pflash is also configured with -machine | ||
38 | pflash0=... (x86: bios is silently ignored then) | ||
39 | |||
40 | * -machine pflash1=... does not require -machine pflash0=... (x86: it | ||
41 | does). | ||
42 | |||
43 | The actual code is a bit simpler than for x86 mostly due to the first | ||
44 | two differences. | ||
45 | |||
46 | Before the patch, all the action is in create_flash(), called from the | ||
47 | machine's .init() method machvirt_init(): | ||
48 | |||
49 | main() | ||
50 | machine_run_board_init() | ||
51 | machvirt_init() | ||
52 | create_flash() | ||
53 | create_one_flash() for flash[0] | ||
54 | create | ||
55 | configure | ||
56 | includes obeying -drive if=pflash,unit=0 | ||
57 | realize | ||
58 | map | ||
59 | fall back to -bios | ||
60 | create_one_flash() for flash[1] | ||
61 | create | ||
62 | configure | ||
63 | includes obeying -drive if=pflash,unit=1 | ||
64 | realize | ||
65 | map | ||
66 | update FDT | ||
67 | |||
68 | To make the machine properties work, we need to move device creation | ||
69 | to its .instance_init() method virt_instance_init(). | ||
70 | |||
71 | Another complication is machvirt_init()'s computation of | ||
72 | @firmware_loaded: it predicts what create_flash() will do. Instead of | ||
73 | predicting what create_flash()'s replacement virt_firmware_init() will | ||
74 | do, I decided to have virt_firmware_init() return what it did. | ||
75 | Requires calling it a bit earlier. | ||
76 | |||
77 | Resulting call tree: | ||
78 | |||
79 | main() | ||
80 | current_machine = object_new() | ||
81 | ... | ||
82 | virt_instance_init() | ||
83 | virt_flash_create() | ||
84 | virt_flash_create1() for flash[0] | ||
85 | create | ||
86 | configure: set defaults | ||
87 | become child of machine [NEW] | ||
88 | add machine prop pflash0 as alias for drive [NEW] | ||
89 | virt_flash_create1() for flash[1] | ||
90 | create | ||
91 | configure: set defaults | ||
92 | become child of machine [NEW] | ||
93 | add machine prop pflash1 as alias for drive [NEW] | ||
94 | for all machine props from the command line: machine_set_property() | ||
95 | ... | ||
96 | property_set_alias() for machine props pflash0, pflash1 | ||
97 | ... | ||
98 | set_drive() for cfi.pflash01 prop drive | ||
99 | this is how -machine pflash0=... etc set | ||
100 | machine_run_board_init(current_machine); | ||
101 | virt_firmware_init() | ||
102 | pflash_cfi01_legacy_drive() | ||
103 | legacy -drive if=pflash,unit=0 and =1 [NEW] | ||
104 | virt_flash_map() | ||
105 | virt_flash_map1() for flash[0] | ||
106 | configure: num-blocks | ||
107 | realize | ||
108 | map | ||
109 | virt_flash_map1() for flash[1] | ||
110 | configure: num-blocks | ||
111 | realize | ||
112 | map | ||
113 | fall back to -bios | ||
114 | virt_flash_fdt() | ||
115 | update FDT | ||
116 | |||
117 | You have László to thank for making me explain this in detail. | ||
118 | |||
119 | Signed-off-by: Markus Armbruster <armbru@redhat.com> | ||
120 | Acked-by: Laszlo Ersek <lersek@redhat.com> | ||
121 | Message-id: 20190416091348.26075-4-armbru@redhat.com | ||
122 | Reviewed-by: Peter Maydell <peter.maydell@linaro.org> | 12 | Reviewed-by: Peter Maydell <peter.maydell@linaro.org> |
123 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | 13 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> |
124 | --- | 14 | --- |
125 | include/hw/arm/virt.h | 2 + | 15 | hw/arm/virt.c | 2 +- |
126 | hw/arm/virt.c | 202 +++++++++++++++++++++++++++--------------- | 16 | 1 file changed, 1 insertion(+), 1 deletion(-) |
127 | 2 files changed, 132 insertions(+), 72 deletions(-) | ||
128 | 17 | ||
129 | diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h | ||
130 | index XXXXXXX..XXXXXXX 100644 | ||
131 | --- a/include/hw/arm/virt.h | ||
132 | +++ b/include/hw/arm/virt.h | ||
133 | @@ -XXX,XX +XXX,XX @@ | ||
134 | #include "qemu/notify.h" | ||
135 | #include "hw/boards.h" | ||
136 | #include "hw/arm/arm.h" | ||
137 | +#include "hw/block/flash.h" | ||
138 | #include "sysemu/kvm.h" | ||
139 | #include "hw/intc/arm_gicv3_common.h" | ||
140 | |||
141 | @@ -XXX,XX +XXX,XX @@ typedef struct { | ||
142 | Notifier machine_done; | ||
143 | DeviceState *platform_bus_dev; | ||
144 | FWCfgState *fw_cfg; | ||
145 | + PFlashCFI01 *flash[2]; | ||
146 | bool secure; | ||
147 | bool highmem; | ||
148 | bool highmem_ecam; | ||
149 | diff --git a/hw/arm/virt.c b/hw/arm/virt.c | 18 | diff --git a/hw/arm/virt.c b/hw/arm/virt.c |
150 | index XXXXXXX..XXXXXXX 100644 | 19 | index XXXXXXX..XXXXXXX 100644 |
151 | --- a/hw/arm/virt.c | 20 | --- a/hw/arm/virt.c |
152 | +++ b/hw/arm/virt.c | 21 | +++ b/hw/arm/virt.c |
153 | @@ -XXX,XX +XXX,XX @@ | ||
154 | |||
155 | #include "qemu/osdep.h" | ||
156 | #include "qemu/units.h" | ||
157 | +#include "qemu/option.h" | ||
158 | #include "qapi/error.h" | ||
159 | #include "hw/sysbus.h" | ||
160 | #include "hw/arm/arm.h" | ||
161 | @@ -XXX,XX +XXX,XX @@ static void create_virtio_devices(const VirtMachineState *vms, qemu_irq *pic) | ||
162 | } | ||
163 | } | ||
164 | |||
165 | -static void create_one_flash(const char *name, hwaddr flashbase, | ||
166 | - hwaddr flashsize, const char *file, | ||
167 | - MemoryRegion *sysmem) | ||
168 | +#define VIRT_FLASH_SECTOR_SIZE (256 * KiB) | ||
169 | + | ||
170 | +static PFlashCFI01 *virt_flash_create1(VirtMachineState *vms, | ||
171 | + const char *name, | ||
172 | + const char *alias_prop_name) | ||
173 | { | ||
174 | - /* Create and map a single flash device. We use the same | ||
175 | - * parameters as the flash devices on the Versatile Express board. | ||
176 | + /* | ||
177 | + * Create a single flash device. We use the same parameters as | ||
178 | + * the flash devices on the Versatile Express board. | ||
179 | */ | ||
180 | - DriveInfo *dinfo = drive_get_next(IF_PFLASH); | ||
181 | DeviceState *dev = qdev_create(NULL, TYPE_PFLASH_CFI01); | ||
182 | - SysBusDevice *sbd = SYS_BUS_DEVICE(dev); | ||
183 | - const uint64_t sectorlength = 256 * 1024; | ||
184 | |||
185 | - if (dinfo) { | ||
186 | - qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo), | ||
187 | - &error_abort); | ||
188 | - } | ||
189 | - | ||
190 | - qdev_prop_set_uint32(dev, "num-blocks", flashsize / sectorlength); | ||
191 | - qdev_prop_set_uint64(dev, "sector-length", sectorlength); | ||
192 | + qdev_prop_set_uint64(dev, "sector-length", VIRT_FLASH_SECTOR_SIZE); | ||
193 | qdev_prop_set_uint8(dev, "width", 4); | ||
194 | qdev_prop_set_uint8(dev, "device-width", 2); | ||
195 | qdev_prop_set_bit(dev, "big-endian", false); | ||
196 | @@ -XXX,XX +XXX,XX @@ static void create_one_flash(const char *name, hwaddr flashbase, | ||
197 | qdev_prop_set_uint16(dev, "id2", 0x00); | ||
198 | qdev_prop_set_uint16(dev, "id3", 0x00); | ||
199 | qdev_prop_set_string(dev, "name", name); | ||
200 | - qdev_init_nofail(dev); | ||
201 | - | ||
202 | - memory_region_add_subregion(sysmem, flashbase, | ||
203 | - sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0)); | ||
204 | - | ||
205 | - if (file) { | ||
206 | - char *fn; | ||
207 | - int image_size; | ||
208 | - | ||
209 | - if (drive_get(IF_PFLASH, 0, 0)) { | ||
210 | - error_report("The contents of the first flash device may be " | ||
211 | - "specified with -bios or with -drive if=pflash... " | ||
212 | - "but you cannot use both options at once"); | ||
213 | - exit(1); | ||
214 | - } | ||
215 | - fn = qemu_find_file(QEMU_FILE_TYPE_BIOS, file); | ||
216 | - if (!fn) { | ||
217 | - error_report("Could not find ROM image '%s'", file); | ||
218 | - exit(1); | ||
219 | - } | ||
220 | - image_size = load_image_mr(fn, sysbus_mmio_get_region(sbd, 0)); | ||
221 | - g_free(fn); | ||
222 | - if (image_size < 0) { | ||
223 | - error_report("Could not load ROM image '%s'", file); | ||
224 | - exit(1); | ||
225 | - } | ||
226 | - } | ||
227 | + object_property_add_child(OBJECT(vms), name, OBJECT(dev), | ||
228 | + &error_abort); | ||
229 | + object_property_add_alias(OBJECT(vms), alias_prop_name, | ||
230 | + OBJECT(dev), "drive", &error_abort); | ||
231 | + return PFLASH_CFI01(dev); | ||
232 | } | ||
233 | |||
234 | -static void create_flash(const VirtMachineState *vms, | ||
235 | - MemoryRegion *sysmem, | ||
236 | - MemoryRegion *secure_sysmem) | ||
237 | +static void virt_flash_create(VirtMachineState *vms) | ||
238 | { | ||
239 | - /* Create two flash devices to fill the VIRT_FLASH space in the memmap. | ||
240 | - * Any file passed via -bios goes in the first of these. | ||
241 | + vms->flash[0] = virt_flash_create1(vms, "virt.flash0", "pflash0"); | ||
242 | + vms->flash[1] = virt_flash_create1(vms, "virt.flash1", "pflash1"); | ||
243 | +} | ||
244 | + | ||
245 | +static void virt_flash_map1(PFlashCFI01 *flash, | ||
246 | + hwaddr base, hwaddr size, | ||
247 | + MemoryRegion *sysmem) | ||
248 | +{ | ||
249 | + DeviceState *dev = DEVICE(flash); | ||
250 | + | ||
251 | + assert(size % VIRT_FLASH_SECTOR_SIZE == 0); | ||
252 | + assert(size / VIRT_FLASH_SECTOR_SIZE <= UINT32_MAX); | ||
253 | + qdev_prop_set_uint32(dev, "num-blocks", size / VIRT_FLASH_SECTOR_SIZE); | ||
254 | + qdev_init_nofail(dev); | ||
255 | + | ||
256 | + memory_region_add_subregion(sysmem, base, | ||
257 | + sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), | ||
258 | + 0)); | ||
259 | +} | ||
260 | + | ||
261 | +static void virt_flash_map(VirtMachineState *vms, | ||
262 | + MemoryRegion *sysmem, | ||
263 | + MemoryRegion *secure_sysmem) | ||
264 | +{ | ||
265 | + /* | ||
266 | + * Map two flash devices to fill the VIRT_FLASH space in the memmap. | ||
267 | * sysmem is the system memory space. secure_sysmem is the secure view | ||
268 | * of the system, and the first flash device should be made visible only | ||
269 | * there. The second flash device is visible to both secure and nonsecure. | ||
270 | @@ -XXX,XX +XXX,XX @@ static void create_flash(const VirtMachineState *vms, | ||
271 | */ | ||
272 | hwaddr flashsize = vms->memmap[VIRT_FLASH].size / 2; | ||
273 | hwaddr flashbase = vms->memmap[VIRT_FLASH].base; | ||
274 | - char *nodename; | ||
275 | |||
276 | - create_one_flash("virt.flash0", flashbase, flashsize, | ||
277 | - bios_name, secure_sysmem); | ||
278 | - create_one_flash("virt.flash1", flashbase + flashsize, flashsize, | ||
279 | - NULL, sysmem); | ||
280 | + virt_flash_map1(vms->flash[0], flashbase, flashsize, | ||
281 | + secure_sysmem); | ||
282 | + virt_flash_map1(vms->flash[1], flashbase + flashsize, flashsize, | ||
283 | + sysmem); | ||
284 | +} | ||
285 | + | ||
286 | +static void virt_flash_fdt(VirtMachineState *vms, | ||
287 | + MemoryRegion *sysmem, | ||
288 | + MemoryRegion *secure_sysmem) | ||
289 | +{ | ||
290 | + hwaddr flashsize = vms->memmap[VIRT_FLASH].size / 2; | ||
291 | + hwaddr flashbase = vms->memmap[VIRT_FLASH].base; | ||
292 | + char *nodename; | ||
293 | |||
294 | if (sysmem == secure_sysmem) { | ||
295 | /* Report both flash devices as a single node in the DT */ | ||
296 | @@ -XXX,XX +XXX,XX @@ static void create_flash(const VirtMachineState *vms, | ||
297 | qemu_fdt_setprop_cell(vms->fdt, nodename, "bank-width", 4); | ||
298 | g_free(nodename); | ||
299 | } else { | ||
300 | - /* Report the devices as separate nodes so we can mark one as | ||
301 | + /* | ||
302 | + * Report the devices as separate nodes so we can mark one as | ||
303 | * only visible to the secure world. | ||
304 | */ | ||
305 | nodename = g_strdup_printf("/secflash@%" PRIx64, flashbase); | ||
306 | @@ -XXX,XX +XXX,XX @@ static void create_flash(const VirtMachineState *vms, | ||
307 | } | ||
308 | } | ||
309 | |||
310 | +static bool virt_firmware_init(VirtMachineState *vms, | ||
311 | + MemoryRegion *sysmem, | ||
312 | + MemoryRegion *secure_sysmem) | ||
313 | +{ | ||
314 | + int i; | ||
315 | + BlockBackend *pflash_blk0; | ||
316 | + | ||
317 | + /* Map legacy -drive if=pflash to machine properties */ | ||
318 | + for (i = 0; i < ARRAY_SIZE(vms->flash); i++) { | ||
319 | + pflash_cfi01_legacy_drive(vms->flash[i], | ||
320 | + drive_get(IF_PFLASH, 0, i)); | ||
321 | + } | ||
322 | + | ||
323 | + virt_flash_map(vms, sysmem, secure_sysmem); | ||
324 | + | ||
325 | + pflash_blk0 = pflash_cfi01_get_blk(vms->flash[0]); | ||
326 | + | ||
327 | + if (bios_name) { | ||
328 | + char *fname; | ||
329 | + MemoryRegion *mr; | ||
330 | + int image_size; | ||
331 | + | ||
332 | + if (pflash_blk0) { | ||
333 | + error_report("The contents of the first flash device may be " | ||
334 | + "specified with -bios or with -drive if=pflash... " | ||
335 | + "but you cannot use both options at once"); | ||
336 | + exit(1); | ||
337 | + } | ||
338 | + | ||
339 | + /* Fall back to -bios */ | ||
340 | + | ||
341 | + fname = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name); | ||
342 | + if (!fname) { | ||
343 | + error_report("Could not find ROM image '%s'", bios_name); | ||
344 | + exit(1); | ||
345 | + } | ||
346 | + mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(vms->flash[0]), 0); | ||
347 | + image_size = load_image_mr(fname, mr); | ||
348 | + g_free(fname); | ||
349 | + if (image_size < 0) { | ||
350 | + error_report("Could not load ROM image '%s'", bios_name); | ||
351 | + exit(1); | ||
352 | + } | ||
353 | + } | ||
354 | + | ||
355 | + return pflash_blk0 || bios_name; | ||
356 | +} | ||
357 | + | ||
358 | static FWCfgState *create_fw_cfg(const VirtMachineState *vms, AddressSpace *as) | ||
359 | { | ||
360 | hwaddr base = vms->memmap[VIRT_FW_CFG].base; | ||
361 | @@ -XXX,XX +XXX,XX @@ static void machvirt_init(MachineState *machine) | ||
362 | MemoryRegion *secure_sysmem = NULL; | ||
363 | int n, virt_max_cpus; | ||
364 | MemoryRegion *ram = g_new(MemoryRegion, 1); | ||
365 | - bool firmware_loaded = bios_name || drive_get(IF_PFLASH, 0, 0); | ||
366 | + bool firmware_loaded; | ||
367 | bool aarch64 = true; | ||
368 | |||
369 | /* | ||
370 | @@ -XXX,XX +XXX,XX @@ static void machvirt_init(MachineState *machine) | ||
371 | exit(1); | ||
372 | } | ||
373 | |||
374 | + if (vms->secure) { | ||
375 | + if (kvm_enabled()) { | ||
376 | + error_report("mach-virt: KVM does not support Security extensions"); | ||
377 | + exit(1); | ||
378 | + } | ||
379 | + | ||
380 | + /* | ||
381 | + * The Secure view of the world is the same as the NonSecure, | ||
382 | + * but with a few extra devices. Create it as a container region | ||
383 | + * containing the system memory at low priority; any secure-only | ||
384 | + * devices go in at higher priority and take precedence. | ||
385 | + */ | ||
386 | + secure_sysmem = g_new(MemoryRegion, 1); | ||
387 | + memory_region_init(secure_sysmem, OBJECT(machine), "secure-memory", | ||
388 | + UINT64_MAX); | ||
389 | + memory_region_add_subregion_overlap(secure_sysmem, 0, sysmem, -1); | ||
390 | + } | ||
391 | + | ||
392 | + firmware_loaded = virt_firmware_init(vms, sysmem, | ||
393 | + secure_sysmem ?: sysmem); | ||
394 | + | ||
395 | /* If we have an EL3 boot ROM then the assumption is that it will | ||
396 | * implement PSCI itself, so disable QEMU's internal implementation | ||
397 | * so it doesn't get in the way. Instead of starting secondary | ||
398 | @@ -XXX,XX +XXX,XX @@ static void machvirt_init(MachineState *machine) | ||
399 | exit(1); | ||
400 | } | ||
401 | |||
402 | - if (vms->secure) { | ||
403 | - if (kvm_enabled()) { | ||
404 | - error_report("mach-virt: KVM does not support Security extensions"); | ||
405 | - exit(1); | ||
406 | - } | ||
407 | - | ||
408 | - /* The Secure view of the world is the same as the NonSecure, | ||
409 | - * but with a few extra devices. Create it as a container region | ||
410 | - * containing the system memory at low priority; any secure-only | ||
411 | - * devices go in at higher priority and take precedence. | ||
412 | - */ | ||
413 | - secure_sysmem = g_new(MemoryRegion, 1); | ||
414 | - memory_region_init(secure_sysmem, OBJECT(machine), "secure-memory", | ||
415 | - UINT64_MAX); | ||
416 | - memory_region_add_subregion_overlap(secure_sysmem, 0, sysmem, -1); | ||
417 | - } | ||
418 | - | ||
419 | create_fdt(vms); | ||
420 | |||
421 | possible_cpus = mc->possible_cpu_arch_ids(machine); | ||
422 | @@ -XXX,XX +XXX,XX @@ static void machvirt_init(MachineState *machine) | 22 | @@ -XXX,XX +XXX,XX @@ static void machvirt_init(MachineState *machine) |
423 | &machine->device_memory->mr); | 23 | &machine->device_memory->mr); |
424 | } | 24 | } |
425 | 25 | ||
426 | - create_flash(vms, sysmem, secure_sysmem ? secure_sysmem : sysmem); | 26 | - virt_flash_fdt(vms, sysmem, secure_sysmem); |
427 | + virt_flash_fdt(vms, sysmem, secure_sysmem); | 27 | + virt_flash_fdt(vms, sysmem, secure_sysmem ?: sysmem); |
428 | 28 | ||
429 | create_gic(vms, pic); | 29 | create_gic(vms, pic); |
430 | 30 | ||
431 | @@ -XXX,XX +XXX,XX @@ static void virt_instance_init(Object *obj) | ||
432 | NULL); | ||
433 | |||
434 | vms->irqmap = a15irqmap; | ||
435 | + | ||
436 | + virt_flash_create(vms); | ||
437 | } | ||
438 | |||
439 | static const TypeInfo virt_machine_info = { | ||
440 | -- | 31 | -- |
441 | 2.20.1 | 32 | 2.20.1 |
442 | 33 | ||
443 | 34 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | The Raspberry Pi boards have a physical memory map which does | ||
2 | not allow for more than 1GB of RAM. Currently if the user tries | ||
3 | to ask for more then we fail in a confusing way: | ||
4 | 1 | ||
5 | $ qemu-system-aarch64 --machine raspi3 -m 8G | ||
6 | Unexpected error in visit_type_uintN() at qapi/qapi-visit-core.c:164: | ||
7 | qemu-system-aarch64: Parameter 'vcram-base' expects uint32_t | ||
8 | Aborted (core dumped) | ||
9 | |||
10 | Catch this earlier and diagnose it with a more friendly message: | ||
11 | $ qemu-system-aarch64 --machine raspi3 -m 8G | ||
12 | qemu-system-aarch64: Requested ram size is too large for this machine: maximum is 1GB | ||
13 | |||
14 | Fixes: https://bugs.launchpad.net/qemu/+bug/1794187 | ||
15 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | ||
16 | Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> | ||
17 | Reviewed-by: Richard Henderson <richard.henderson@linaro.org> | ||
18 | Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com> | ||
19 | --- | ||
20 | hw/arm/raspi.c | 7 +++++++ | ||
21 | 1 file changed, 7 insertions(+) | ||
22 | |||
23 | diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c | ||
24 | index XXXXXXX..XXXXXXX 100644 | ||
25 | --- a/hw/arm/raspi.c | ||
26 | +++ b/hw/arm/raspi.c | ||
27 | @@ -XXX,XX +XXX,XX @@ | ||
28 | */ | ||
29 | |||
30 | #include "qemu/osdep.h" | ||
31 | +#include "qemu/units.h" | ||
32 | #include "qapi/error.h" | ||
33 | #include "qemu-common.h" | ||
34 | #include "cpu.h" | ||
35 | @@ -XXX,XX +XXX,XX @@ static void raspi_init(MachineState *machine, int version) | ||
36 | BusState *bus; | ||
37 | DeviceState *carddev; | ||
38 | |||
39 | + if (machine->ram_size > 1 * GiB) { | ||
40 | + error_report("Requested ram size is too large for this machine: " | ||
41 | + "maximum is 1GB"); | ||
42 | + exit(1); | ||
43 | + } | ||
44 | + | ||
45 | object_initialize(&s->soc, sizeof(s->soc), | ||
46 | version == 3 ? TYPE_BCM2837 : TYPE_BCM2836); | ||
47 | object_property_add_child(OBJECT(machine), "soc", OBJECT(&s->soc), | ||
48 | -- | ||
49 | 2.20.1 | ||
50 | |||
51 | diff view generated by jsdifflib |
1 | From: Joel Stanley <joel@jms.id.au> | 1 | The PL031 RTC tracks the difference between the guest RTC |
---|---|---|---|
2 | 2 | and the host RTC using a tick_offset field. For migration, | |
3 | We currently use Qemu's default of 128MB. As we know how much ram each | 3 | however, we currently always migrate the offset between |
4 | machine ships with, make it easier on users by setting a default. | 4 | the guest and the vm_clock, even if the RTC clock is not |
5 | 5 | the same as the vm_clock; this was an attempt to retain | |
6 | It can still be overridden with -m on the command line. | 6 | migration backwards compatibility. |
7 | 7 | ||
8 | Signed-off-by: Joel Stanley <joel@jms.id.au> | 8 | Unfortunately this results in the RTC behaving oddly across |
9 | Reviewed-by: Andrew Jeffery <andrew@aj.id.au> | 9 | a VM state save and restore -- since the VM clock stands still |
10 | Reviewed-by: Richard Henderson <richard.henderson@linaro.org> | 10 | across save-then-restore, regardless of how much real world |
11 | Message-id: 20190503022958.1394-1-joel@jms.id.au | 11 | time has elapsed, the guest RTC ends up out of sync with the |
12 | Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> | 12 | host RTC in the restored VM. |
13 | |||
14 | Fix this by migrating the raw tick_offset. To retain migration | ||
15 | compatibility as far as possible, we have a new property | ||
16 | migrate-tick-offset; by default this is 'true' and we will | ||
17 | migrate the true tick offset in a new subsection; if the | ||
18 | incoming data has no subsection we fall back to the old | ||
19 | vm_clock-based offset information, so old->new migration | ||
20 | compatibility is preserved. For complete new->old migration | ||
21 | compatibility, the property is set to 'false' for 4.0 and | ||
22 | earlier machine types (this will only affect 'virt-4.0' | ||
23 | and below, as none of the other pl031-using machines are | ||
24 | versioned). | ||
25 | |||
26 | Reported-by: Russell King <rmk@armlinux.org.uk> | ||
13 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | 27 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> |
28 | Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> | ||
29 | Message-id: 20190709143912.28905-1-peter.maydell@linaro.org | ||
14 | --- | 30 | --- |
15 | include/hw/arm/aspeed.h | 1 + | 31 | include/hw/timer/pl031.h | 2 + |
16 | hw/arm/aspeed.c | 8 ++++++++ | 32 | hw/core/machine.c | 1 + |
17 | 2 files changed, 9 insertions(+) | 33 | hw/timer/pl031.c | 92 ++++++++++++++++++++++++++++++++++++++-- |
18 | 34 | 3 files changed, 91 insertions(+), 4 deletions(-) | |
19 | diff --git a/include/hw/arm/aspeed.h b/include/hw/arm/aspeed.h | 35 | |
36 | diff --git a/include/hw/timer/pl031.h b/include/hw/timer/pl031.h | ||
20 | index XXXXXXX..XXXXXXX 100644 | 37 | index XXXXXXX..XXXXXXX 100644 |
21 | --- a/include/hw/arm/aspeed.h | 38 | --- a/include/hw/timer/pl031.h |
22 | +++ b/include/hw/arm/aspeed.h | 39 | +++ b/include/hw/timer/pl031.h |
23 | @@ -XXX,XX +XXX,XX @@ typedef struct AspeedBoardConfig { | 40 | @@ -XXX,XX +XXX,XX @@ typedef struct PL031State { |
24 | const char *spi_model; | 41 | */ |
25 | uint32_t num_cs; | 42 | uint32_t tick_offset_vmstate; |
26 | void (*i2c_init)(AspeedBoardState *bmc); | 43 | uint32_t tick_offset; |
27 | + uint32_t ram; | 44 | + bool tick_offset_migrated; |
28 | } AspeedBoardConfig; | 45 | + bool migrate_tick_offset; |
29 | 46 | ||
30 | #define TYPE_ASPEED_MACHINE MACHINE_TYPE_NAME("aspeed") | 47 | uint32_t mr; |
31 | diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c | 48 | uint32_t lr; |
49 | diff --git a/hw/core/machine.c b/hw/core/machine.c | ||
32 | index XXXXXXX..XXXXXXX 100644 | 50 | index XXXXXXX..XXXXXXX 100644 |
33 | --- a/hw/arm/aspeed.c | 51 | --- a/hw/core/machine.c |
34 | +++ b/hw/arm/aspeed.c | 52 | +++ b/hw/core/machine.c |
35 | @@ -XXX,XX +XXX,XX @@ | 53 | @@ -XXX,XX +XXX,XX @@ GlobalProperty hw_compat_4_0[] = { |
36 | #include "sysemu/block-backend.h" | 54 | { "virtio-gpu-pci", "edid", "false" }, |
37 | #include "hw/loader.h" | 55 | { "virtio-device", "use-started", "false" }, |
38 | #include "qemu/error-report.h" | 56 | { "virtio-balloon-device", "qemu-4-0-config-size", "true" }, |
39 | +#include "qemu/units.h" | 57 | + { "pl031", "migrate-tick-offset", "false" }, |
40 | 58 | }; | |
41 | static struct arm_boot_info aspeed_board_binfo = { | 59 | const size_t hw_compat_4_0_len = G_N_ELEMENTS(hw_compat_4_0); |
42 | .board_id = -1, /* device-tree-only board */ | 60 | |
43 | @@ -XXX,XX +XXX,XX @@ static void aspeed_machine_class_init(ObjectClass *oc, void *data) | 61 | diff --git a/hw/timer/pl031.c b/hw/timer/pl031.c |
44 | mc->no_floppy = 1; | 62 | index XXXXXXX..XXXXXXX 100644 |
45 | mc->no_cdrom = 1; | 63 | --- a/hw/timer/pl031.c |
46 | mc->no_parallel = 1; | 64 | +++ b/hw/timer/pl031.c |
47 | + if (board->ram) { | 65 | @@ -XXX,XX +XXX,XX @@ static int pl031_pre_save(void *opaque) |
48 | + mc->default_ram_size = board->ram; | 66 | { |
67 | PL031State *s = opaque; | ||
68 | |||
69 | - /* tick_offset is base_time - rtc_clock base time. Instead, we want to | ||
70 | - * store the base time relative to the QEMU_CLOCK_VIRTUAL for backwards-compatibility. */ | ||
71 | + /* | ||
72 | + * The PL031 device model code uses the tick_offset field, which is | ||
73 | + * the offset between what the guest RTC should read and what the | ||
74 | + * QEMU rtc_clock reads: | ||
75 | + * guest_rtc = rtc_clock + tick_offset | ||
76 | + * and so | ||
77 | + * tick_offset = guest_rtc - rtc_clock | ||
78 | + * | ||
79 | + * We want to migrate this offset, which sounds straightforward. | ||
80 | + * Unfortunately older versions of QEMU migrated a conversion of this | ||
81 | + * offset into an offset from the vm_clock. (This was in turn an | ||
82 | + * attempt to be compatible with even older QEMU versions, but it | ||
83 | + * has incorrect behaviour if the rtc_clock is not the same as the | ||
84 | + * vm_clock.) So we put the actual tick_offset into a migration | ||
85 | + * subsection, and the backwards-compatible time-relative-to-vm_clock | ||
86 | + * in the main migration state. | ||
87 | + * | ||
88 | + * Calculate base time relative to QEMU_CLOCK_VIRTUAL: | ||
89 | + */ | ||
90 | int64_t delta = qemu_clock_get_ns(rtc_clock) - qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); | ||
91 | s->tick_offset_vmstate = s->tick_offset + delta / NANOSECONDS_PER_SECOND; | ||
92 | |||
93 | return 0; | ||
94 | } | ||
95 | |||
96 | +static int pl031_pre_load(void *opaque) | ||
97 | +{ | ||
98 | + PL031State *s = opaque; | ||
99 | + | ||
100 | + s->tick_offset_migrated = false; | ||
101 | + return 0; | ||
102 | +} | ||
103 | + | ||
104 | static int pl031_post_load(void *opaque, int version_id) | ||
105 | { | ||
106 | PL031State *s = opaque; | ||
107 | |||
108 | - int64_t delta = qemu_clock_get_ns(rtc_clock) - qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); | ||
109 | - s->tick_offset = s->tick_offset_vmstate - delta / NANOSECONDS_PER_SECOND; | ||
110 | + /* | ||
111 | + * If we got the tick_offset subsection, then we can just use | ||
112 | + * the value in that. Otherwise the source is an older QEMU and | ||
113 | + * has given us the offset from the vm_clock; convert it back to | ||
114 | + * an offset from the rtc_clock. This will cause time to incorrectly | ||
115 | + * go backwards compared to the host RTC, but this is unavoidable. | ||
116 | + */ | ||
117 | + | ||
118 | + if (!s->tick_offset_migrated) { | ||
119 | + int64_t delta = qemu_clock_get_ns(rtc_clock) - | ||
120 | + qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); | ||
121 | + s->tick_offset = s->tick_offset_vmstate - | ||
122 | + delta / NANOSECONDS_PER_SECOND; | ||
49 | + } | 123 | + } |
50 | amc->board = board; | 124 | pl031_set_alarm(s); |
125 | return 0; | ||
51 | } | 126 | } |
52 | 127 | ||
53 | @@ -XXX,XX +XXX,XX @@ static const AspeedBoardConfig aspeed_boards[] = { | 128 | +static int pl031_tick_offset_post_load(void *opaque, int version_id) |
54 | .spi_model = "mx25l25635e", | 129 | +{ |
55 | .num_cs = 1, | 130 | + PL031State *s = opaque; |
56 | .i2c_init = palmetto_bmc_i2c_init, | 131 | + |
57 | + .ram = 256 * MiB, | 132 | + s->tick_offset_migrated = true; |
58 | }, { | 133 | + return 0; |
59 | .name = MACHINE_TYPE_NAME("ast2500-evb"), | 134 | +} |
60 | .desc = "Aspeed AST2500 EVB (ARM1176)", | 135 | + |
61 | @@ -XXX,XX +XXX,XX @@ static const AspeedBoardConfig aspeed_boards[] = { | 136 | +static bool pl031_tick_offset_needed(void *opaque) |
62 | .spi_model = "mx25l25635e", | 137 | +{ |
63 | .num_cs = 1, | 138 | + PL031State *s = opaque; |
64 | .i2c_init = ast2500_evb_i2c_init, | 139 | + |
65 | + .ram = 512 * MiB, | 140 | + return s->migrate_tick_offset; |
66 | }, { | 141 | +} |
67 | .name = MACHINE_TYPE_NAME("romulus-bmc"), | 142 | + |
68 | .desc = "OpenPOWER Romulus BMC (ARM1176)", | 143 | +static const VMStateDescription vmstate_pl031_tick_offset = { |
69 | @@ -XXX,XX +XXX,XX @@ static const AspeedBoardConfig aspeed_boards[] = { | 144 | + .name = "pl031/tick-offset", |
70 | .spi_model = "mx66l1g45g", | 145 | + .version_id = 1, |
71 | .num_cs = 2, | 146 | + .minimum_version_id = 1, |
72 | .i2c_init = romulus_bmc_i2c_init, | 147 | + .needed = pl031_tick_offset_needed, |
73 | + .ram = 512 * MiB, | 148 | + .post_load = pl031_tick_offset_post_load, |
74 | }, { | 149 | + .fields = (VMStateField[]) { |
75 | .name = MACHINE_TYPE_NAME("witherspoon-bmc"), | 150 | + VMSTATE_UINT32(tick_offset, PL031State), |
76 | .desc = "OpenPOWER Witherspoon BMC (ARM1176)", | 151 | + VMSTATE_END_OF_LIST() |
77 | @@ -XXX,XX +XXX,XX @@ static const AspeedBoardConfig aspeed_boards[] = { | 152 | + } |
78 | .spi_model = "mx66l1g45g", | 153 | +}; |
79 | .num_cs = 2, | 154 | + |
80 | .i2c_init = witherspoon_bmc_i2c_init, | 155 | static const VMStateDescription vmstate_pl031 = { |
81 | + .ram = 512 * MiB, | 156 | .name = "pl031", |
82 | }, | 157 | .version_id = 1, |
158 | .minimum_version_id = 1, | ||
159 | .pre_save = pl031_pre_save, | ||
160 | + .pre_load = pl031_pre_load, | ||
161 | .post_load = pl031_post_load, | ||
162 | .fields = (VMStateField[]) { | ||
163 | VMSTATE_UINT32(tick_offset_vmstate, PL031State), | ||
164 | @@ -XXX,XX +XXX,XX @@ static const VMStateDescription vmstate_pl031 = { | ||
165 | VMSTATE_UINT32(im, PL031State), | ||
166 | VMSTATE_UINT32(is, PL031State), | ||
167 | VMSTATE_END_OF_LIST() | ||
168 | + }, | ||
169 | + .subsections = (const VMStateDescription*[]) { | ||
170 | + &vmstate_pl031_tick_offset, | ||
171 | + NULL | ||
172 | } | ||
83 | }; | 173 | }; |
84 | 174 | ||
175 | +static Property pl031_properties[] = { | ||
176 | + /* | ||
177 | + * True to correctly migrate the tick offset of the RTC. False to | ||
178 | + * obtain backward migration compatibility with older QEMU versions, | ||
179 | + * at the expense of the guest RTC going backwards compared with the | ||
180 | + * host RTC when the VM is saved/restored if using -rtc host. | ||
181 | + * (Even if set to 'true' older QEMU can migrate forward to newer QEMU; | ||
182 | + * 'false' also permits newer QEMU to migrate to older QEMU.) | ||
183 | + */ | ||
184 | + DEFINE_PROP_BOOL("migrate-tick-offset", | ||
185 | + PL031State, migrate_tick_offset, true), | ||
186 | + DEFINE_PROP_END_OF_LIST() | ||
187 | +}; | ||
188 | + | ||
189 | static void pl031_class_init(ObjectClass *klass, void *data) | ||
190 | { | ||
191 | DeviceClass *dc = DEVICE_CLASS(klass); | ||
192 | |||
193 | dc->vmsd = &vmstate_pl031; | ||
194 | + dc->props = pl031_properties; | ||
195 | } | ||
196 | |||
197 | static const TypeInfo pl031_info = { | ||
85 | -- | 198 | -- |
86 | 2.20.1 | 199 | 2.20.1 |
87 | 200 | ||
88 | 201 | diff view generated by jsdifflib |
1 | At the moment the Arm implementations of kvm_arch_{get,put}_registers() | 1 | The ARMv5 architecture didn't specify detailed per-feature ID |
---|---|---|---|
2 | don't support having QEMU change the values of system registers | 2 | registers. Now that we're using the MVFR0 register fields to |
3 | (aka coprocessor registers for AArch32). This is because although | 3 | gate the existence of VFP instructions, we need to set up |
4 | kvm_arch_get_registers() calls write_list_to_cpustate() to | 4 | the correct values in the cpu->isar structure so that we still |
5 | update the CPU state struct fields (so QEMU code can read the | 5 | provide an FPU to the guest. |
6 | values in the usual way), kvm_arch_put_registers() does not | ||
7 | call write_cpustate_to_list(), meaning that any changes to | ||
8 | the CPU state struct fields will not be passed back to KVM. | ||
9 | 6 | ||
10 | The rationale for this design is documented in a comment in the | 7 | This fixes a regression in the arm926 and arm1026 CPUs, which |
11 | AArch32 kvm_arch_put_registers() -- writing the values in the | 8 | are the only ones that both have VFP and are ARMv5 or earlier. |
12 | cpregs list into the CPU state struct is "lossy" because the | 9 | This regression was introduced by the VFP refactoring, and more |
13 | write of a register might not succeed, and so if we blindly | 10 | specifically by commits 1120827fa182f0e76 and 266bd25c485597c, |
14 | copy the CPU state values back again we will incorrectly | 11 | which accidentally disabled VFP short-vector support and |
15 | change register values for the guest. The assumption was that | 12 | double-precision support on these CPUs. |
16 | no QEMU code would need to write to the registers. | ||
17 | 13 | ||
18 | However, when we implemented debug support for KVM guests, we | 14 | Fixes: 1120827fa182f0e |
19 | broke that assumption: the code to handle "set the guest up | 15 | Fixes: 266bd25c485597c |
20 | to take a breakpoint exception" does so by updating various | 16 | Fixes: https://bugs.launchpad.net/qemu/+bug/1836192 |
21 | guest registers including ESR_EL1. | 17 | Reported-by: Christophe Lyon <christophe.lyon@linaro.org> |
22 | |||
23 | Support this by making kvm_arch_put_registers() synchronize | ||
24 | CPU state back into the list. We sync only those registers | ||
25 | where the initial write succeeds, which should be sufficient. | ||
26 | |||
27 | This commit is the same as commit 823e1b3818f9b10b824ddc which we | ||
28 | had to revert in commit 942f99c825fc94c8b1a4, except that the bug | ||
29 | which was preventing EDK2 guest firmware running has been fixed: | ||
30 | kvm_arm_reset_vcpu() now calls write_list_to_cpustate(). | ||
31 | |||
32 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | 18 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> |
33 | Reviewed-by: Richard Henderson <richard.henderson@linaro.org> | 19 | Reviewed-by: Richard Henderson <richard.henderson@linaro.org> |
34 | Tested-by: Eric Auger <eric.auger@redhat.com> | 20 | Reviewed-by: Alex Bennée <alex.bennee@linaro.org> |
21 | Tested-by: Christophe Lyon <christophe.lyon@linaro.org> | ||
22 | Message-id: 20190711131241.22231-1-peter.maydell@linaro.org | ||
35 | --- | 23 | --- |
36 | target/arm/cpu.h | 9 ++++++++- | 24 | target/arm/cpu.c | 12 ++++++++++++ |
37 | target/arm/helper.c | 27 +++++++++++++++++++++++++-- | 25 | 1 file changed, 12 insertions(+) |
38 | target/arm/kvm.c | 8 ++++++++ | ||
39 | target/arm/kvm32.c | 20 ++------------------ | ||
40 | target/arm/kvm64.c | 2 ++ | ||
41 | target/arm/machine.c | 2 +- | ||
42 | 6 files changed, 46 insertions(+), 22 deletions(-) | ||
43 | 26 | ||
44 | diff --git a/target/arm/cpu.h b/target/arm/cpu.h | 27 | diff --git a/target/arm/cpu.c b/target/arm/cpu.c |
45 | index XXXXXXX..XXXXXXX 100644 | 28 | index XXXXXXX..XXXXXXX 100644 |
46 | --- a/target/arm/cpu.h | 29 | --- a/target/arm/cpu.c |
47 | +++ b/target/arm/cpu.h | 30 | +++ b/target/arm/cpu.c |
48 | @@ -XXX,XX +XXX,XX @@ bool write_list_to_cpustate(ARMCPU *cpu); | 31 | @@ -XXX,XX +XXX,XX @@ static void arm926_initfn(Object *obj) |
49 | /** | 32 | * set the field to indicate Jazelle support within QEMU. |
50 | * write_cpustate_to_list: | 33 | */ |
51 | * @cpu: ARMCPU | 34 | cpu->isar.id_isar1 = FIELD_DP32(cpu->isar.id_isar1, ID_ISAR1, JAZELLE, 1); |
52 | + * @kvm_sync: true if this is for syncing back to KVM | 35 | + /* |
53 | * | 36 | + * Similarly, we need to set MVFR0 fields to enable double precision |
54 | * For each register listed in the ARMCPU cpreg_indexes list, write | 37 | + * and short vector support even though ARMv5 doesn't have this register. |
55 | * its value from the ARMCPUState structure into the cpreg_values list. | 38 | + */ |
56 | * This is used to copy info from TCG's working data structures into | 39 | + cpu->isar.mvfr0 = FIELD_DP32(cpu->isar.mvfr0, MVFR0, FPSHVEC, 1); |
57 | * KVM or for outbound migration. | 40 | + cpu->isar.mvfr0 = FIELD_DP32(cpu->isar.mvfr0, MVFR0, FPDP, 1); |
58 | * | ||
59 | + * @kvm_sync is true if we are doing this in order to sync the | ||
60 | + * register state back to KVM. In this case we will only update | ||
61 | + * values in the list if the previous list->cpustate sync actually | ||
62 | + * successfully wrote the CPU state. Otherwise we will keep the value | ||
63 | + * that is in the list. | ||
64 | + * | ||
65 | * Returns: true if all register values were read correctly, | ||
66 | * false if some register was unknown or could not be read. | ||
67 | * Note that we do not stop early on failure -- we will attempt | ||
68 | * reading all registers in the list. | ||
69 | */ | ||
70 | -bool write_cpustate_to_list(ARMCPU *cpu); | ||
71 | +bool write_cpustate_to_list(ARMCPU *cpu, bool kvm_sync); | ||
72 | |||
73 | #define ARM_CPUID_TI915T 0x54029152 | ||
74 | #define ARM_CPUID_TI925T 0x54029252 | ||
75 | diff --git a/target/arm/helper.c b/target/arm/helper.c | ||
76 | index XXXXXXX..XXXXXXX 100644 | ||
77 | --- a/target/arm/helper.c | ||
78 | +++ b/target/arm/helper.c | ||
79 | @@ -XXX,XX +XXX,XX @@ static bool raw_accessors_invalid(const ARMCPRegInfo *ri) | ||
80 | return true; | ||
81 | } | 41 | } |
82 | 42 | ||
83 | -bool write_cpustate_to_list(ARMCPU *cpu) | 43 | static void arm946_initfn(Object *obj) |
84 | +bool write_cpustate_to_list(ARMCPU *cpu, bool kvm_sync) | 44 | @@ -XXX,XX +XXX,XX @@ static void arm1026_initfn(Object *obj) |
85 | { | 45 | * set the field to indicate Jazelle support within QEMU. |
86 | /* Write the coprocessor state from cpu->env to the (index,value) list. */ | 46 | */ |
87 | int i; | 47 | cpu->isar.id_isar1 = FIELD_DP32(cpu->isar.id_isar1, ID_ISAR1, JAZELLE, 1); |
88 | @@ -XXX,XX +XXX,XX @@ bool write_cpustate_to_list(ARMCPU *cpu) | ||
89 | for (i = 0; i < cpu->cpreg_array_len; i++) { | ||
90 | uint32_t regidx = kvm_to_cpreg_id(cpu->cpreg_indexes[i]); | ||
91 | const ARMCPRegInfo *ri; | ||
92 | + uint64_t newval; | ||
93 | |||
94 | ri = get_arm_cp_reginfo(cpu->cp_regs, regidx); | ||
95 | if (!ri) { | ||
96 | @@ -XXX,XX +XXX,XX @@ bool write_cpustate_to_list(ARMCPU *cpu) | ||
97 | if (ri->type & ARM_CP_NO_RAW) { | ||
98 | continue; | ||
99 | } | ||
100 | - cpu->cpreg_values[i] = read_raw_cp_reg(&cpu->env, ri); | ||
101 | + | ||
102 | + newval = read_raw_cp_reg(&cpu->env, ri); | ||
103 | + if (kvm_sync) { | ||
104 | + /* | ||
105 | + * Only sync if the previous list->cpustate sync succeeded. | ||
106 | + * Rather than tracking the success/failure state for every | ||
107 | + * item in the list, we just recheck "does the raw write we must | ||
108 | + * have made in write_list_to_cpustate() read back OK" here. | ||
109 | + */ | ||
110 | + uint64_t oldval = cpu->cpreg_values[i]; | ||
111 | + | ||
112 | + if (oldval == newval) { | ||
113 | + continue; | ||
114 | + } | ||
115 | + | ||
116 | + write_raw_cp_reg(&cpu->env, ri, oldval); | ||
117 | + if (read_raw_cp_reg(&cpu->env, ri) != oldval) { | ||
118 | + continue; | ||
119 | + } | ||
120 | + | ||
121 | + write_raw_cp_reg(&cpu->env, ri, newval); | ||
122 | + } | ||
123 | + cpu->cpreg_values[i] = newval; | ||
124 | } | ||
125 | return ok; | ||
126 | } | ||
127 | diff --git a/target/arm/kvm.c b/target/arm/kvm.c | ||
128 | index XXXXXXX..XXXXXXX 100644 | ||
129 | --- a/target/arm/kvm.c | ||
130 | +++ b/target/arm/kvm.c | ||
131 | @@ -XXX,XX +XXX,XX @@ void kvm_arm_reset_vcpu(ARMCPU *cpu) | ||
132 | fprintf(stderr, "write_kvmstate_to_list failed\n"); | ||
133 | abort(); | ||
134 | } | ||
135 | + /* | 48 | + /* |
136 | + * Sync the reset values also into the CPUState. This is necessary | 49 | + * Similarly, we need to set MVFR0 fields to enable double precision |
137 | + * because the next thing we do will be a kvm_arch_put_registers() | 50 | + * and short vector support even though ARMv5 doesn't have this register. |
138 | + * which will update the list values from the CPUState before copying | ||
139 | + * the list values back to KVM. It's OK to ignore failure returns here | ||
140 | + * for the same reason we do so in kvm_arch_get_registers(). | ||
141 | + */ | 51 | + */ |
142 | + write_list_to_cpustate(cpu); | 52 | + cpu->isar.mvfr0 = FIELD_DP32(cpu->isar.mvfr0, MVFR0, FPSHVEC, 1); |
143 | } | 53 | + cpu->isar.mvfr0 = FIELD_DP32(cpu->isar.mvfr0, MVFR0, FPDP, 1); |
144 | 54 | ||
145 | /* | 55 | { |
146 | diff --git a/target/arm/kvm32.c b/target/arm/kvm32.c | 56 | /* The 1026 had an IFAR at c6,c0,0,1 rather than the ARMv6 c6,c0,0,2 */ |
147 | index XXXXXXX..XXXXXXX 100644 | ||
148 | --- a/target/arm/kvm32.c | ||
149 | +++ b/target/arm/kvm32.c | ||
150 | @@ -XXX,XX +XXX,XX @@ int kvm_arch_put_registers(CPUState *cs, int level) | ||
151 | return ret; | ||
152 | } | ||
153 | |||
154 | - /* Note that we do not call write_cpustate_to_list() | ||
155 | - * here, so we are only writing the tuple list back to | ||
156 | - * KVM. This is safe because nothing can change the | ||
157 | - * CPUARMState cp15 fields (in particular gdb accesses cannot) | ||
158 | - * and so there are no changes to sync. In fact syncing would | ||
159 | - * be wrong at this point: for a constant register where TCG and | ||
160 | - * KVM disagree about its value, the preceding write_list_to_cpustate() | ||
161 | - * would not have had any effect on the CPUARMState value (since the | ||
162 | - * register is read-only), and a write_cpustate_to_list() here would | ||
163 | - * then try to write the TCG value back into KVM -- this would either | ||
164 | - * fail or incorrectly change the value the guest sees. | ||
165 | - * | ||
166 | - * If we ever want to allow the user to modify cp15 registers via | ||
167 | - * the gdb stub, we would need to be more clever here (for instance | ||
168 | - * tracking the set of registers kvm_arch_get_registers() successfully | ||
169 | - * managed to update the CPUARMState with, and only allowing those | ||
170 | - * to be written back up into the kernel). | ||
171 | - */ | ||
172 | + write_cpustate_to_list(cpu, true); | ||
173 | + | ||
174 | if (!write_list_to_kvmstate(cpu, level)) { | ||
175 | return EINVAL; | ||
176 | } | ||
177 | diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c | ||
178 | index XXXXXXX..XXXXXXX 100644 | ||
179 | --- a/target/arm/kvm64.c | ||
180 | +++ b/target/arm/kvm64.c | ||
181 | @@ -XXX,XX +XXX,XX @@ int kvm_arch_put_registers(CPUState *cs, int level) | ||
182 | return ret; | ||
183 | } | ||
184 | |||
185 | + write_cpustate_to_list(cpu, true); | ||
186 | + | ||
187 | if (!write_list_to_kvmstate(cpu, level)) { | ||
188 | return EINVAL; | ||
189 | } | ||
190 | diff --git a/target/arm/machine.c b/target/arm/machine.c | ||
191 | index XXXXXXX..XXXXXXX 100644 | ||
192 | --- a/target/arm/machine.c | ||
193 | +++ b/target/arm/machine.c | ||
194 | @@ -XXX,XX +XXX,XX @@ static int cpu_pre_save(void *opaque) | ||
195 | abort(); | ||
196 | } | ||
197 | } else { | ||
198 | - if (!write_cpustate_to_list(cpu)) { | ||
199 | + if (!write_cpustate_to_list(cpu, false)) { | ||
200 | /* This should never fail. */ | ||
201 | abort(); | ||
202 | } | ||
203 | -- | 57 | -- |
204 | 2.20.1 | 58 | 2.20.1 |
205 | 59 | ||
206 | 60 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | Rule R_CQRV says that if two pending interrupts have the same | ||
2 | group priority then ties are broken by looking at the subpriority. | ||
3 | We had a comment describing this but had forgotten to actually | ||
4 | implement the subpriority comparison. Correct the omission. | ||
5 | 1 | ||
6 | (The further tie break rules of "lowest exception number" and | ||
7 | "secure before non-secure" are handled implicitly by the order | ||
8 | in which we iterate through the exceptions in the loops.) | ||
9 | |||
10 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | ||
11 | Reviewed-by: Richard Henderson <richard.henderson@linaro.org> | ||
12 | Message-id: 20190430131439.25251-2-peter.maydell@linaro.org | ||
13 | --- | ||
14 | hw/intc/armv7m_nvic.c | 9 +++++++-- | ||
15 | 1 file changed, 7 insertions(+), 2 deletions(-) | ||
16 | |||
17 | diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c | ||
18 | index XXXXXXX..XXXXXXX 100644 | ||
19 | --- a/hw/intc/armv7m_nvic.c | ||
20 | +++ b/hw/intc/armv7m_nvic.c | ||
21 | @@ -XXX,XX +XXX,XX @@ static void nvic_recompute_state_secure(NVICState *s) | ||
22 | int active_prio = NVIC_NOEXC_PRIO; | ||
23 | int pend_irq = 0; | ||
24 | bool pending_is_s_banked = false; | ||
25 | + int pend_subprio = 0; | ||
26 | |||
27 | /* R_CQRV: precedence is by: | ||
28 | * - lowest group priority; if both the same then | ||
29 | @@ -XXX,XX +XXX,XX @@ static void nvic_recompute_state_secure(NVICState *s) | ||
30 | for (i = 1; i < s->num_irq; i++) { | ||
31 | for (bank = M_REG_S; bank >= M_REG_NS; bank--) { | ||
32 | VecInfo *vec; | ||
33 | - int prio; | ||
34 | + int prio, subprio; | ||
35 | bool targets_secure; | ||
36 | |||
37 | if (bank == M_REG_S) { | ||
38 | @@ -XXX,XX +XXX,XX @@ static void nvic_recompute_state_secure(NVICState *s) | ||
39 | } | ||
40 | |||
41 | prio = exc_group_prio(s, vec->prio, targets_secure); | ||
42 | - if (vec->enabled && vec->pending && prio < pend_prio) { | ||
43 | + subprio = vec->prio & ~nvic_gprio_mask(s, targets_secure); | ||
44 | + if (vec->enabled && vec->pending && | ||
45 | + ((prio < pend_prio) || | ||
46 | + (prio == pend_prio && prio >= 0 && subprio < pend_subprio))) { | ||
47 | pend_prio = prio; | ||
48 | + pend_subprio = subprio; | ||
49 | pend_irq = i; | ||
50 | pending_is_s_banked = (bank == M_REG_S); | ||
51 | } | ||
52 | -- | ||
53 | 2.20.1 | ||
54 | |||
55 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | The non-secure versions of the BFAR and BFSR registers are | ||
2 | supposed to be RAZ/WI if AICR.BFHFNMINS == 0; we were | ||
3 | incorrectly allowing NS code to access the real values. | ||
4 | 1 | ||
5 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | ||
6 | Reviewed-by: Richard Henderson <richard.henderson@linaro.org> | ||
7 | Message-id: 20190430131439.25251-3-peter.maydell@linaro.org | ||
8 | --- | ||
9 | hw/intc/armv7m_nvic.c | 27 ++++++++++++++++++++++++--- | ||
10 | 1 file changed, 24 insertions(+), 3 deletions(-) | ||
11 | |||
12 | diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c | ||
13 | index XXXXXXX..XXXXXXX 100644 | ||
14 | --- a/hw/intc/armv7m_nvic.c | ||
15 | +++ b/hw/intc/armv7m_nvic.c | ||
16 | @@ -XXX,XX +XXX,XX @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset, MemTxAttrs attrs) | ||
17 | if (!arm_feature(&cpu->env, ARM_FEATURE_M_MAIN)) { | ||
18 | goto bad_offset; | ||
19 | } | ||
20 | + if (!attrs.secure && | ||
21 | + !(s->cpu->env.v7m.aircr & R_V7M_AIRCR_BFHFNMINS_MASK)) { | ||
22 | + return 0; | ||
23 | + } | ||
24 | return cpu->env.v7m.bfar; | ||
25 | case 0xd3c: /* Aux Fault Status. */ | ||
26 | /* TODO: Implement fault status registers. */ | ||
27 | @@ -XXX,XX +XXX,XX @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value, | ||
28 | if (!arm_feature(&cpu->env, ARM_FEATURE_M_MAIN)) { | ||
29 | goto bad_offset; | ||
30 | } | ||
31 | + if (!attrs.secure && | ||
32 | + !(s->cpu->env.v7m.aircr & R_V7M_AIRCR_BFHFNMINS_MASK)) { | ||
33 | + return; | ||
34 | + } | ||
35 | cpu->env.v7m.bfar = value; | ||
36 | return; | ||
37 | case 0xd3c: /* Aux Fault Status. */ | ||
38 | @@ -XXX,XX +XXX,XX @@ static MemTxResult nvic_sysreg_read(void *opaque, hwaddr addr, | ||
39 | val = 0; | ||
40 | break; | ||
41 | }; | ||
42 | - /* The BFSR bits [15:8] are shared between security states | ||
43 | - * and we store them in the NS copy | ||
44 | + /* | ||
45 | + * The BFSR bits [15:8] are shared between security states | ||
46 | + * and we store them in the NS copy. They are RAZ/WI for | ||
47 | + * NS code if AIRCR.BFHFNMINS is 0. | ||
48 | */ | ||
49 | val = s->cpu->env.v7m.cfsr[attrs.secure]; | ||
50 | - val |= s->cpu->env.v7m.cfsr[M_REG_NS] & R_V7M_CFSR_BFSR_MASK; | ||
51 | + if (!attrs.secure && | ||
52 | + !(s->cpu->env.v7m.aircr & R_V7M_AIRCR_BFHFNMINS_MASK)) { | ||
53 | + val &= ~R_V7M_CFSR_BFSR_MASK; | ||
54 | + } else { | ||
55 | + val |= s->cpu->env.v7m.cfsr[M_REG_NS] & R_V7M_CFSR_BFSR_MASK; | ||
56 | + } | ||
57 | val = extract32(val, (offset - 0xd28) * 8, size * 8); | ||
58 | break; | ||
59 | case 0xfe0 ... 0xfff: /* ID. */ | ||
60 | @@ -XXX,XX +XXX,XX @@ static MemTxResult nvic_sysreg_write(void *opaque, hwaddr addr, | ||
61 | */ | ||
62 | value <<= ((offset - 0xd28) * 8); | ||
63 | |||
64 | + if (!attrs.secure && | ||
65 | + !(s->cpu->env.v7m.aircr & R_V7M_AIRCR_BFHFNMINS_MASK)) { | ||
66 | + /* BFSR bits are RAZ/WI for NS if BFHFNMINS is set */ | ||
67 | + value &= ~R_V7M_CFSR_BFSR_MASK; | ||
68 | + } | ||
69 | + | ||
70 | s->cpu->env.v7m.cfsr[attrs.secure] &= ~value; | ||
71 | if (attrs.secure) { | ||
72 | /* The BFSR bits [15:8] are shared between security states | ||
73 | -- | ||
74 | 2.20.1 | ||
75 | |||
76 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | The M-profile architecture specifies that the DebugMonitor exception | ||
2 | should be initially disabled, not enabled. It should be controlled | ||
3 | by the DEMCR register's MON_EN bit, but we don't implement that | ||
4 | register yet (like most of the debug architecture for M-profile). | ||
5 | 1 | ||
6 | Note that BKPT instructions will still work, because they | ||
7 | will be escalated to HardFault. | ||
8 | |||
9 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | ||
10 | Reviewed-by: Richard Henderson <richard.henderson@linaro.org> | ||
11 | Message-id: 20190430131439.25251-4-peter.maydell@linaro.org | ||
12 | --- | ||
13 | hw/intc/armv7m_nvic.c | 4 +++- | ||
14 | 1 file changed, 3 insertions(+), 1 deletion(-) | ||
15 | |||
16 | diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c | ||
17 | index XXXXXXX..XXXXXXX 100644 | ||
18 | --- a/hw/intc/armv7m_nvic.c | ||
19 | +++ b/hw/intc/armv7m_nvic.c | ||
20 | @@ -XXX,XX +XXX,XX @@ static void armv7m_nvic_reset(DeviceState *dev) | ||
21 | * the System Handler Control register | ||
22 | */ | ||
23 | s->vectors[ARMV7M_EXCP_SVC].enabled = 1; | ||
24 | - s->vectors[ARMV7M_EXCP_DEBUG].enabled = 1; | ||
25 | s->vectors[ARMV7M_EXCP_PENDSV].enabled = 1; | ||
26 | s->vectors[ARMV7M_EXCP_SYSTICK].enabled = 1; | ||
27 | |||
28 | + /* DebugMonitor is enabled via DEMCR.MON_EN */ | ||
29 | + s->vectors[ARMV7M_EXCP_DEBUG].enabled = 0; | ||
30 | + | ||
31 | resetprio = arm_feature(&s->cpu->env, ARM_FEATURE_V8) ? -4 : -3; | ||
32 | s->vectors[ARMV7M_EXCP_RESET].prio = resetprio; | ||
33 | s->vectors[ARMV7M_EXCP_NMI].prio = -2; | ||
34 | -- | ||
35 | 2.20.1 | ||
36 | |||
37 | diff view generated by jsdifflib |
1 | Currently the dc_zva helper function uses a variable length | 1 | In the M-profile architecture, when we do a vector table fetch and it |
---|---|---|---|
2 | array. In fact we know (as the comment above remarks) that | 2 | fails, we need to report a HardFault. Whether this is a Secure HF or |
3 | the length of this array is bounded because the architecture | 3 | a NonSecure HF depends on several things. If AIRCR.BFHFNMINS is 0 |
4 | limits the block size and QEMU limits the target page size. | 4 | then HF is always Secure, because there is no NonSecure HardFault. |
5 | Use a fixed array size and assert that we don't run off it. | 5 | Otherwise, the answer depends on whether the 'underlying exception' |
6 | (MemManage, BusFault, SecureFault) targets Secure or NonSecure. (In | ||
7 | the pseudocode, this is handled in the Vector() function: the final | ||
8 | exc.isSecure is calculated by looking at the exc.isSecure from the | ||
9 | exception returned from the memory access, not the isSecure input | ||
10 | argument.) | ||
11 | |||
12 | We weren't doing this correctly, because we were looking at | ||
13 | the target security domain of the exception we were trying to | ||
14 | load the vector table entry for. This produces errors of two kinds: | ||
15 | * a load from the NS vector table which hits the "NS access | ||
16 | to S memory" SecureFault should end up as a Secure HardFault, | ||
17 | but we were raising an NS HardFault | ||
18 | * a load from the S vector table which causes a BusFault | ||
19 | should raise an NS HardFault if BFHFNMINS == 1 (because | ||
20 | in that case all BusFaults are NonSecure), but we were raising | ||
21 | a Secure HardFault | ||
22 | |||
23 | Correct the logic. | ||
24 | |||
25 | We also fix a comment error where we claimed that we might | ||
26 | be escalating MemManage to HardFault, and forgot about SecureFault. | ||
27 | (Vector loads can never hit MPU access faults, because they're | ||
28 | always aligned and always use the default address map.) | ||
6 | 29 | ||
7 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | 30 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> |
8 | Reviewed-by: Richard Henderson <richard.henderson@linaro.org> | 31 | Message-id: 20190705094823.28905-1-peter.maydell@linaro.org |
9 | Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> | ||
10 | Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com> | ||
11 | Message-id: 20190503120448.13385-1-peter.maydell@linaro.org | ||
12 | --- | 32 | --- |
13 | target/arm/helper.c | 8 ++++++-- | 33 | target/arm/m_helper.c | 21 +++++++++++++++++---- |
14 | 1 file changed, 6 insertions(+), 2 deletions(-) | 34 | 1 file changed, 17 insertions(+), 4 deletions(-) |
15 | 35 | ||
16 | diff --git a/target/arm/helper.c b/target/arm/helper.c | 36 | diff --git a/target/arm/m_helper.c b/target/arm/m_helper.c |
17 | index XXXXXXX..XXXXXXX 100644 | 37 | index XXXXXXX..XXXXXXX 100644 |
18 | --- a/target/arm/helper.c | 38 | --- a/target/arm/m_helper.c |
19 | +++ b/target/arm/helper.c | 39 | +++ b/target/arm/m_helper.c |
20 | @@ -XXX,XX +XXX,XX @@ | 40 | @@ -XXX,XX +XXX,XX @@ static bool arm_v7m_load_vector(ARMCPU *cpu, int exc, bool targets_secure, |
21 | #include "qemu/osdep.h" | 41 | if (sattrs.ns) { |
22 | +#include "qemu/units.h" | 42 | attrs.secure = false; |
23 | #include "target/arm/idau.h" | 43 | } else if (!targets_secure) { |
24 | #include "trace.h" | 44 | - /* NS access to S memory */ |
25 | #include "cpu.h" | 45 | + /* |
26 | @@ -XXX,XX +XXX,XX @@ void HELPER(dc_zva)(CPUARMState *env, uint64_t vaddr_in) | 46 | + * NS access to S memory: the underlying exception which we escalate |
27 | * We know that in fact for any v8 CPU the page size is at least 4K | 47 | + * to HardFault is SecureFault, which always targets Secure. |
28 | * and the block size must be 2K or less, but TARGET_PAGE_SIZE is only | 48 | + */ |
29 | * 1K as an artefact of legacy v5 subpage support being present in the | 49 | + exc_secure = true; |
30 | - * same QEMU executable. | 50 | goto load_fail; |
31 | + * same QEMU executable. So in practice the hostaddr[] array has | 51 | } |
32 | + * two entries, given the current setting of TARGET_PAGE_BITS_MIN. | 52 | } |
33 | */ | 53 | @@ -XXX,XX +XXX,XX @@ static bool arm_v7m_load_vector(ARMCPU *cpu, int exc, bool targets_secure, |
34 | int maxidx = DIV_ROUND_UP(blocklen, TARGET_PAGE_SIZE); | 54 | vector_entry = address_space_ldl(arm_addressspace(cs, attrs), addr, |
35 | - void *hostaddr[maxidx]; | 55 | attrs, &result); |
36 | + void *hostaddr[DIV_ROUND_UP(2 * KiB, 1 << TARGET_PAGE_BITS_MIN)]; | 56 | if (result != MEMTX_OK) { |
37 | int try, i; | 57 | + /* |
38 | unsigned mmu_idx = cpu_mmu_index(env, false); | 58 | + * Underlying exception is BusFault: its target security state |
39 | TCGMemOpIdx oi = make_memop_idx(MO_UB, mmu_idx); | 59 | + * depends on BFHFNMINS. |
40 | 60 | + */ | |
41 | + assert(maxidx <= ARRAY_SIZE(hostaddr)); | 61 | + exc_secure = !(cpu->env.v7m.aircr & R_V7M_AIRCR_BFHFNMINS_MASK); |
42 | + | 62 | goto load_fail; |
43 | for (try = 0; try < 2; try++) { | 63 | } |
44 | 64 | *pvec = vector_entry; | |
45 | for (i = 0; i < maxidx; i++) { | 65 | @@ -XXX,XX +XXX,XX @@ load_fail: |
66 | /* | ||
67 | * All vector table fetch fails are reported as HardFault, with | ||
68 | * HFSR.VECTTBL and .FORCED set. (FORCED is set because | ||
69 | - * technically the underlying exception is a MemManage or BusFault | ||
70 | + * technically the underlying exception is a SecureFault or BusFault | ||
71 | * that is escalated to HardFault.) This is a terminal exception, | ||
72 | * so we will either take the HardFault immediately or else enter | ||
73 | * lockup (the latter case is handled in armv7m_nvic_set_pending_derived()). | ||
74 | + * The HardFault is Secure if BFHFNMINS is 0 (meaning that all HFs are | ||
75 | + * secure); otherwise it targets the same security state as the | ||
76 | + * underlying exception. | ||
77 | */ | ||
78 | - exc_secure = targets_secure || | ||
79 | - !(cpu->env.v7m.aircr & R_V7M_AIRCR_BFHFNMINS_MASK); | ||
80 | + if (!(cpu->env.v7m.aircr & R_V7M_AIRCR_BFHFNMINS_MASK)) { | ||
81 | + exc_secure = true; | ||
82 | + } | ||
83 | env->v7m.hfsr |= R_V7M_HFSR_VECTTBL_MASK | R_V7M_HFSR_FORCED_MASK; | ||
84 | armv7m_nvic_set_pending_derived(env->nvic, ARMV7M_EXCP_HARD, exc_secure); | ||
85 | return false; | ||
46 | -- | 86 | -- |
47 | 2.20.1 | 87 | 2.20.1 |
48 | 88 | ||
49 | 89 | diff view generated by jsdifflib |