| 1 | A set of small bugfixes for arm for 3.0; the "migration was | 1 | This bug seemed worth fixing for 8.0 since we need an rc4 anyway: |
|---|---|---|---|
| 2 | broken" fixes for SMMUv3 and v7M NVIC with security extensions | 2 | we were using uninitialized data for the guarded bit when |
| 3 | are the most significant. | 3 | combining stage 1 and stage 2 attrs. |
| 4 | 4 | ||
| 5 | thanks | 5 | thanks |
| 6 | -- PMM | 6 | -- PMM |
| 7 | 7 | ||
| 8 | The following changes since commit 6d9dd5fb9d0e9f4a174f53a0e20a39fbe809c71e: | 8 | The following changes since commit 08dede07030973c1053868bc64de7e10bfa02ad6: |
| 9 | 9 | ||
| 10 | Merge remote-tracking branch 'remotes/armbru/tags/pull-qobject-2018-07-27-v2' into staging (2018-07-30 09:55:47 +0100) | 10 | Merge tag 'pull-ppc-20230409' of https://github.com/legoater/qemu into staging (2023-04-10 11:47:52 +0100) |
| 11 | 11 | ||
| 12 | are available in the Git repository at: | 12 | are available in the Git repository at: |
| 13 | 13 | ||
| 14 | git://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20180730 | 14 | https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20230410 |
| 15 | 15 | ||
| 16 | for you to fetch changes up to 0261fb805c00a6f97d143235e7b06b0906bdf898: | 16 | for you to fetch changes up to 8539dc00552e8ea60420856fc1262c8299bc6308: |
| 17 | 17 | ||
| 18 | target/arm: Remove duplicate 'host' entry in '-cpu ?' output (2018-07-30 15:07:08 +0100) | 18 | target/arm: Copy guarded bit in combine_cacheattrs (2023-04-10 14:31:40 +0100) |
| 19 | 19 | ||
| 20 | ---------------------------------------------------------------- | 20 | ---------------------------------------------------------------- |
| 21 | target-arm queue: | 21 | target-arm: Fix bug where we weren't initializing |
| 22 | * arm/smmuv3: Fix broken VM state migration | 22 | guarded bit state when combining S1/S2 attrs |
| 23 | * armv7m_nvic: Fix broken VM state migration | ||
| 24 | * hw/arm/sysbus-fdt: Fix assertion in copy_properties_from_host() | ||
| 25 | * hw/arm/iotkit: Fix IRQ number for timer1 | ||
| 26 | * hw/misc/tz-mpc: Zero the LUT on initialization, not just reset | ||
| 27 | * target/arm: Remove duplicate 'host' entry in '-cpu ?' output | ||
| 28 | 23 | ||
| 29 | ---------------------------------------------------------------- | 24 | ---------------------------------------------------------------- |
| 30 | Dr. David Alan Gilbert (1): | 25 | Richard Henderson (2): |
| 31 | arm/smmuv3: Fix missing VMSD terminator | 26 | target/arm: PTE bit GP only applies to stage1 |
| 27 | target/arm: Copy guarded bit in combine_cacheattrs | ||
| 32 | 28 | ||
| 33 | Geert Uytterhoeven (1): | 29 | target/arm/ptw.c | 11 ++++++----- |
| 34 | hw/arm/sysbus-fdt: Fix assertion in copy_properties_from_host() | 30 | 1 file changed, 6 insertions(+), 5 deletions(-) |
| 35 | |||
| 36 | Peter Maydell (3): | ||
| 37 | armv7m_nvic: Fix m-security subsection name | ||
| 38 | hw/arm/iotkit: Fix IRQ number for timer1 | ||
| 39 | hw/misc/tz-mpc: Zero the LUT on initialization, not just reset | ||
| 40 | |||
| 41 | Philippe Mathieu-Daudé (1): | ||
| 42 | target/arm: Remove duplicate 'host' entry in '-cpu ?' output | ||
| 43 | |||
| 44 | hw/arm/iotkit.c | 2 +- | ||
| 45 | hw/arm/smmuv3.c | 1 + | ||
| 46 | hw/arm/sysbus-fdt.c | 1 + | ||
| 47 | hw/intc/armv7m_nvic.c | 2 +- | ||
| 48 | hw/misc/tz-mpc.c | 2 +- | ||
| 49 | target/arm/helper.c | 6 ------ | ||
| 50 | 6 files changed, 5 insertions(+), 9 deletions(-) | ||
| 51 | diff view generated by jsdifflib |
| 1 | From: Philippe Mathieu-Daudé <f4bug@amsat.org> | 1 | From: Richard Henderson <richard.henderson@linaro.org> |
|---|---|---|---|
| 2 | 2 | ||
| 3 | Since 86f0a186d6f the TYPE_ARM_HOST_CPU is only compiled when CONFIG_KVM | 3 | Only perform the extract of GP during the stage1 walk. |
| 4 | is enabled. | ||
| 5 | 4 | ||
| 6 | Remove the now redundant special-case introduced in a96c0514ab7, to avoid: | 5 | Reported-by: Peter Maydell <peter.maydell@linaro.org> |
| 7 | 6 | Signed-off-by: Richard Henderson <richard.henderson@linaro.org> | |
| 8 | $ qemu-system-aarch64 -machine virt -cpu \? | fgrep host | ||
| 9 | host | ||
| 10 | host (only available in KVM mode) | ||
| 11 | |||
| 12 | Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> | ||
| 13 | Message-id: 20180727132311.2777-1-f4bug@amsat.org | ||
| 14 | Reviewed-by: Peter Maydell <peter.maydell@linaro.org> | 7 | Reviewed-by: Peter Maydell <peter.maydell@linaro.org> |
| 8 | Message-id: 20230407185149.3253946-2-richard.henderson@linaro.org | ||
| 15 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | 9 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> |
| 16 | --- | 10 | --- |
| 17 | target/arm/helper.c | 6 ------ | 11 | target/arm/ptw.c | 10 +++++----- |
| 18 | 1 file changed, 6 deletions(-) | 12 | 1 file changed, 5 insertions(+), 5 deletions(-) |
| 19 | 13 | ||
| 20 | diff --git a/target/arm/helper.c b/target/arm/helper.c | 14 | diff --git a/target/arm/ptw.c b/target/arm/ptw.c |
| 21 | index XXXXXXX..XXXXXXX 100644 | 15 | index XXXXXXX..XXXXXXX 100644 |
| 22 | --- a/target/arm/helper.c | 16 | --- a/target/arm/ptw.c |
| 23 | +++ b/target/arm/helper.c | 17 | +++ b/target/arm/ptw.c |
| 24 | @@ -XXX,XX +XXX,XX @@ void arm_cpu_list(FILE *f, fprintf_function cpu_fprintf) | 18 | @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw, |
| 25 | (*cpu_fprintf)(f, "Available CPUs:\n"); | 19 | result->f.attrs.secure = false; |
| 26 | g_slist_foreach(list, arm_cpu_list_entry, &s); | 20 | } |
| 27 | g_slist_free(list); | 21 | |
| 28 | -#ifdef CONFIG_KVM | 22 | - /* When in aarch64 mode, and BTI is enabled, remember GP in the TLB. */ |
| 29 | - /* The 'host' CPU type is dynamically registered only if KVM is | 23 | - if (aarch64 && cpu_isar_feature(aa64_bti, cpu)) { |
| 30 | - * enabled, so we have to special-case it here: | 24 | - result->f.guarded = extract64(attrs, 50, 1); /* GP */ |
| 31 | - */ | 25 | - } |
| 32 | - (*cpu_fprintf)(f, " host (only available in KVM mode)\n"); | 26 | - |
| 33 | -#endif | 27 | if (regime_is_stage2(mmu_idx)) { |
| 34 | } | 28 | result->cacheattrs.is_s2_format = true; |
| 35 | 29 | result->cacheattrs.attrs = extract32(attrs, 2, 4); | |
| 36 | static void arm_cpu_add_definition(gpointer data, gpointer user_data) | 30 | @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw, |
| 31 | assert(attrindx <= 7); | ||
| 32 | result->cacheattrs.is_s2_format = false; | ||
| 33 | result->cacheattrs.attrs = extract64(mair, attrindx * 8, 8); | ||
| 34 | + | ||
| 35 | + /* When in aarch64 mode, and BTI is enabled, remember GP in the TLB. */ | ||
| 36 | + if (aarch64 && cpu_isar_feature(aa64_bti, cpu)) { | ||
| 37 | + result->f.guarded = extract64(attrs, 50, 1); /* GP */ | ||
| 38 | + } | ||
| 39 | } | ||
| 40 | |||
| 41 | /* | ||
| 37 | -- | 42 | -- |
| 38 | 2.17.1 | 43 | 2.34.1 |
| 39 | |||
| 40 | diff view generated by jsdifflib |
| 1 | From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> | 1 | From: Richard Henderson <richard.henderson@linaro.org> |
|---|---|---|---|
| 2 | 2 | ||
| 3 | The 'vmstate_smmuv3_queue' is missing the end-of-list marker. | 3 | The guarded bit comes from the stage1 walk. |
| 4 | 4 | ||
| 5 | Fixes: 10a83cb9887 | 5 | Fixes: Coverity CID 1507929 |
| 6 | Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> | 6 | Signed-off-by: Richard Henderson <richard.henderson@linaro.org> |
| 7 | Message-id: 20180727135406.15132-1-dgilbert@redhat.com | ||
| 8 | Reviewed-by: Peter Maydell <peter.maydell@linaro.org> | 7 | Reviewed-by: Peter Maydell <peter.maydell@linaro.org> |
| 9 | [PMM: dropped stray blank line] | 8 | Message-id: 20230407185149.3253946-3-richard.henderson@linaro.org |
| 10 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | 9 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> |
| 11 | --- | 10 | --- |
| 12 | hw/arm/smmuv3.c | 1 + | 11 | target/arm/ptw.c | 1 + |
| 13 | 1 file changed, 1 insertion(+) | 12 | 1 file changed, 1 insertion(+) |
| 14 | 13 | ||
| 15 | diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c | 14 | diff --git a/target/arm/ptw.c b/target/arm/ptw.c |
| 16 | index XXXXXXX..XXXXXXX 100644 | 15 | index XXXXXXX..XXXXXXX 100644 |
| 17 | --- a/hw/arm/smmuv3.c | 16 | --- a/target/arm/ptw.c |
| 18 | +++ b/hw/arm/smmuv3.c | 17 | +++ b/target/arm/ptw.c |
| 19 | @@ -XXX,XX +XXX,XX @@ static const VMStateDescription vmstate_smmuv3_queue = { | 18 | @@ -XXX,XX +XXX,XX @@ static ARMCacheAttrs combine_cacheattrs(uint64_t hcr, |
| 20 | VMSTATE_UINT32(prod, SMMUQueue), | 19 | |
| 21 | VMSTATE_UINT32(cons, SMMUQueue), | 20 | assert(!s1.is_s2_format); |
| 22 | VMSTATE_UINT8(log2size, SMMUQueue), | 21 | ret.is_s2_format = false; |
| 23 | + VMSTATE_END_OF_LIST(), | 22 | + ret.guarded = s1.guarded; |
| 24 | }, | 23 | |
| 25 | }; | 24 | if (s1.attrs == 0xf0) { |
| 26 | 25 | tagged = true; | |
| 27 | -- | 26 | -- |
| 28 | 2.17.1 | 27 | 2.34.1 |
| 29 | |||
| 30 | diff view generated by jsdifflib |
Deleted patch | |||
|---|---|---|---|
| 1 | From: Geert Uytterhoeven <geert+renesas@glider.be> | ||
| 2 | 1 | ||
| 3 | When copy_properties_from_host() ignores the error for an optional | ||
| 4 | property, it frees the error, but fails to reset it. | ||
| 5 | |||
| 6 | Hence if two or more optional properties are missing, an assertion is | ||
| 7 | triggered: | ||
| 8 | |||
| 9 | util/error.c:57: error_setv: Assertion `*errp == NULL' failed. | ||
| 10 | |||
| 11 | Fis this by resetting err to NULL after ignoring the error. | ||
| 12 | |||
| 13 | Fixes: 9481cf2e5f2f2bb6 ("hw/arm/sysbus-fdt: helpers for clock node generation") | ||
| 14 | Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> | ||
| 15 | Message-id: 20180725113000.11014-1-geert+renesas@glider.be | ||
| 16 | Reviewed-by: Peter Maydell <peter.maydell@linaro.org> | ||
| 17 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | ||
| 18 | --- | ||
| 19 | hw/arm/sysbus-fdt.c | 1 + | ||
| 20 | 1 file changed, 1 insertion(+) | ||
| 21 | |||
| 22 | diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c | ||
| 23 | index XXXXXXX..XXXXXXX 100644 | ||
| 24 | --- a/hw/arm/sysbus-fdt.c | ||
| 25 | +++ b/hw/arm/sysbus-fdt.c | ||
| 26 | @@ -XXX,XX +XXX,XX @@ static void copy_properties_from_host(HostProperty *props, int nb_props, | ||
| 27 | /* mandatory property not found: bail out */ | ||
| 28 | exit(1); | ||
| 29 | } | ||
| 30 | + err = NULL; | ||
| 31 | } | ||
| 32 | } | ||
| 33 | } | ||
| 34 | -- | ||
| 35 | 2.17.1 | ||
| 36 | |||
| 37 | diff view generated by jsdifflib |
Deleted patch | |||
|---|---|---|---|
| 1 | The vmstate save/load code insists that subsections of a VMState must | ||
| 2 | have names which include their parent VMState's name as a leading | ||
| 3 | substring. Unfortunately it neither documents this nor checks it on | ||
| 4 | device init or state save, but instead fails state load with a | ||
| 5 | confusing error message ("Missing section footer for armv7m_nvic"). | ||
| 6 | 1 | ||
| 7 | Fix the name of the m-security subsection of the NVIC, so that | ||
| 8 | state save/load works correctly for the security-enabled NVIC. | ||
| 9 | |||
| 10 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | ||
| 11 | Reviewed-by: Richard Henderson <richard.henderson@linaro.org> | ||
| 12 | Message-id: 20180727113854.20283-2-peter.maydell@linaro.org | ||
| 13 | --- | ||
| 14 | hw/intc/armv7m_nvic.c | 2 +- | ||
| 15 | 1 file changed, 1 insertion(+), 1 deletion(-) | ||
| 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 int nvic_security_post_load(void *opaque, int version_id) | ||
| 22 | } | ||
| 23 | |||
| 24 | static const VMStateDescription vmstate_nvic_security = { | ||
| 25 | - .name = "nvic/m-security", | ||
| 26 | + .name = "armv7m_nvic/m-security", | ||
| 27 | .version_id = 1, | ||
| 28 | .minimum_version_id = 1, | ||
| 29 | .needed = nvic_security_needed, | ||
| 30 | -- | ||
| 31 | 2.17.1 | ||
| 32 | |||
| 33 | diff view generated by jsdifflib |
Deleted patch | |||
|---|---|---|---|
| 1 | A cut-and-paste error meant we were incorrectly wiring up the timer1 | ||
| 2 | IRQ to IRQ3. IRQ3 is the interrupt for timer0 -- move timer0 to | ||
| 3 | IRQ4 where it belongs. | ||
| 4 | 1 | ||
| 5 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | ||
| 6 | Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> | ||
| 7 | Message-id: 20180727113854.20283-3-peter.maydell@linaro.org | ||
| 8 | --- | ||
| 9 | hw/arm/iotkit.c | 2 +- | ||
| 10 | 1 file changed, 1 insertion(+), 1 deletion(-) | ||
| 11 | |||
| 12 | diff --git a/hw/arm/iotkit.c b/hw/arm/iotkit.c | ||
| 13 | index XXXXXXX..XXXXXXX 100644 | ||
| 14 | --- a/hw/arm/iotkit.c | ||
| 15 | +++ b/hw/arm/iotkit.c | ||
| 16 | @@ -XXX,XX +XXX,XX @@ static void iotkit_realize(DeviceState *dev, Error **errp) | ||
| 17 | return; | ||
| 18 | } | ||
| 19 | sysbus_connect_irq(SYS_BUS_DEVICE(&s->timer1), 0, | ||
| 20 | - qdev_get_gpio_in(DEVICE(&s->armv7m), 3)); | ||
| 21 | + qdev_get_gpio_in(DEVICE(&s->armv7m), 4)); | ||
| 22 | mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->timer1), 0); | ||
| 23 | object_property_set_link(OBJECT(&s->apb_ppc0), OBJECT(mr), "port[1]", &err); | ||
| 24 | if (err) { | ||
| 25 | -- | ||
| 26 | 2.17.1 | ||
| 27 | |||
| 28 | diff view generated by jsdifflib |
Deleted patch | |||
|---|---|---|---|
| 1 | In the tz-mpc device we allocate a data block for the LUT, | ||
| 2 | which we then clear to zero in the device's reset method. | ||
| 3 | This is conceptually fine, but unfortunately results in a | ||
| 4 | valgrind complaint about use of uninitialized data on startup: | ||
| 5 | 1 | ||
| 6 | ==30906== Conditional jump or move depends on uninitialised value(s) | ||
| 7 | ==30906== at 0x503609: tz_mpc_translate (tz-mpc.c:439) | ||
| 8 | ==30906== by 0x3F3D90: address_space_translate_iommu (exec.c:511) | ||
| 9 | ==30906== by 0x3F3FF8: flatview_do_translate (exec.c:584) | ||
| 10 | ==30906== by 0x3F4292: flatview_translate (exec.c:644) | ||
| 11 | ==30906== by 0x3F2120: address_space_translate (memory.h:1962) | ||
| 12 | ==30906== by 0x3FB753: address_space_ldl_internal (memory_ldst.inc.c:36) | ||
| 13 | ==30906== by 0x3FB8A6: address_space_ldl (memory_ldst.inc.c:80) | ||
| 14 | ==30906== by 0x619037: ldl_phys (memory_ldst_phys.inc.h:25) | ||
| 15 | ==30906== by 0x61985D: arm_cpu_reset (cpu.c:255) | ||
| 16 | ==30906== by 0x98791B: cpu_reset (cpu.c:249) | ||
| 17 | ==30906== by 0x57FFDB: armv7m_reset (armv7m.c:265) | ||
| 18 | ==30906== by 0x7B1775: qemu_devices_reset (reset.c:69) | ||
| 19 | |||
| 20 | This is because of a reset ordering problem -- the TZ MPC | ||
| 21 | resets after the CPU, but an M-profile CPU's reset function | ||
| 22 | includes memory loads to get the initial PC and SP, which | ||
| 23 | then go through an MPC that hasn't yet been reset. | ||
| 24 | |||
| 25 | The simplest fix for this is to zero the LUT when we | ||
| 26 | initialize the data, which will result in the MPC's | ||
| 27 | translate function giving the right answers for these | ||
| 28 | early memory accesses. | ||
| 29 | |||
| 30 | Reported-by: Thomas Huth <thuth@redhat.com> | ||
| 31 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | ||
| 32 | Tested-by: Thomas Huth <thuth@redhat.com> | ||
| 33 | Message-id: 20180724153616.32352-1-peter.maydell@linaro.org | ||
| 34 | --- | ||
| 35 | hw/misc/tz-mpc.c | 2 +- | ||
| 36 | 1 file changed, 1 insertion(+), 1 deletion(-) | ||
| 37 | |||
| 38 | diff --git a/hw/misc/tz-mpc.c b/hw/misc/tz-mpc.c | ||
| 39 | index XXXXXXX..XXXXXXX 100644 | ||
| 40 | --- a/hw/misc/tz-mpc.c | ||
| 41 | +++ b/hw/misc/tz-mpc.c | ||
| 42 | @@ -XXX,XX +XXX,XX @@ static void tz_mpc_realize(DeviceState *dev, Error **errp) | ||
| 43 | address_space_init(&s->blocked_io_as, &s->blocked_io, | ||
| 44 | "tz-mpc-blocked-io"); | ||
| 45 | |||
| 46 | - s->blk_lut = g_new(uint32_t, s->blk_max); | ||
| 47 | + s->blk_lut = g_new0(uint32_t, s->blk_max); | ||
| 48 | } | ||
| 49 | |||
| 50 | static int tz_mpc_post_load(void *opaque, int version_id) | ||
| 51 | -- | ||
| 52 | 2.17.1 | ||
| 53 | |||
| 54 | diff view generated by jsdifflib |