| 1 | Handful of bugfix patches for arm for rc0; also | 1 | This bug seemed worth fixing for 8.0 since we need an rc4 anyway: |
|---|---|---|---|
| 2 | one milkymist patch, thrown in since I was putting | 2 | we were using uninitialized data for the guarded bit when |
| 3 | the pullreq together anyway. | 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 03c1ca1c51783603d42eb0f91d35961f0f4b4947: | 8 | The following changes since commit 08dede07030973c1053868bc64de7e10bfa02ad6: |
| 9 | 9 | ||
| 10 | Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20181105' into staging (2018-11-06 09:10:46 +0000) | 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 | https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20181106 | 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 23463e0e4aeb2f0a9c60549a2c163f4adc0b8512: | 16 | for you to fetch changes up to 8539dc00552e8ea60420856fc1262c8299bc6308: |
| 17 | 17 | ||
| 18 | target/arm: Fix ATS1Hx instructions (2018-11-06 11:32:14 +0000) | 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 | * Remove can't-happen if() from handle_vec_simd_shli() | 22 | guarded bit state when combining S1/S2 attrs |
| 23 | * hw/arm/exynos4210: Zero memory allocated for Exynos4210State | ||
| 24 | * Set S and PTW in 64-bit PAR format | ||
| 25 | * Fix ATS1Hx instructions | ||
| 26 | * milkymist: Check for failure trying to load BIOS image | ||
| 27 | 23 | ||
| 28 | ---------------------------------------------------------------- | 24 | ---------------------------------------------------------------- |
| 29 | Peter Maydell (5): | 25 | Richard Henderson (2): |
| 30 | target/arm: Remove can't-happen if() from handle_vec_simd_shli() | 26 | target/arm: PTE bit GP only applies to stage1 |
| 31 | milkymist: Check for failure trying to load BIOS image | 27 | target/arm: Copy guarded bit in combine_cacheattrs |
| 32 | hw/arm/exynos4210: Zero memory allocated for Exynos4210State | ||
| 33 | target/arm: Set S and PTW in 64-bit PAR format | ||
| 34 | target/arm: Fix ATS1Hx instructions | ||
| 35 | 28 | ||
| 36 | hw/arm/exynos4210.c | 2 +- | 29 | target/arm/ptw.c | 11 ++++++----- |
| 37 | hw/lm32/milkymist.c | 5 ++++- | 30 | 1 file changed, 6 insertions(+), 5 deletions(-) |
| 38 | target/arm/helper.c | 14 ++++++++------ | ||
| 39 | target/arm/translate-a64.c | 8 +++----- | ||
| 40 | 4 files changed, 16 insertions(+), 13 deletions(-) | ||
| 41 | diff view generated by jsdifflib |
Deleted patch | |||
|---|---|---|---|
| 1 | In handle_vec_simd_shli() we have a check: | ||
| 2 | if (size > 3 && !is_q) { | ||
| 3 | unallocated_encoding(s); | ||
| 4 | return; | ||
| 5 | } | ||
| 6 | However this can never be true, because we calculate | ||
| 7 | int size = 32 - clz32(immh) - 1; | ||
| 8 | where immh is a 4 bit field which we know cannot be all-zeroes. | ||
| 9 | So the clz32() return must be in {28,29,30,31} and the resulting | ||
| 10 | size is in {0,1,2,3}, and "size > 3" is never true. | ||
| 11 | 1 | ||
| 12 | This unnecessary code confuses Coverity's analysis: | ||
| 13 | in CID 1396476 it thinks we might later index off the | ||
| 14 | end of an array because the condition implies that we | ||
| 15 | might have a size > 3. | ||
| 16 | |||
| 17 | Remove the code, and instead assert that the size is in [0..3], | ||
| 18 | since the decode that enforces that is somewhat distant from | ||
| 19 | this function. | ||
| 20 | |||
| 21 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | ||
| 22 | Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> | ||
| 23 | Reviewed-by: Alex Bennée <alex.bennee@linaro.org> | ||
| 24 | Tested-by: Alex Bennée <alex.bennee@linaro.org> | ||
| 25 | Message-id: 20181030162517.21816-1-peter.maydell@linaro.org | ||
| 26 | --- | ||
| 27 | target/arm/translate-a64.c | 8 +++----- | ||
| 28 | 1 file changed, 3 insertions(+), 5 deletions(-) | ||
| 29 | |||
| 30 | diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c | ||
| 31 | index XXXXXXX..XXXXXXX 100644 | ||
| 32 | --- a/target/arm/translate-a64.c | ||
| 33 | +++ b/target/arm/translate-a64.c | ||
| 34 | @@ -XXX,XX +XXX,XX @@ static void handle_vec_simd_shli(DisasContext *s, bool is_q, bool insert, | ||
| 35 | int immhb = immh << 3 | immb; | ||
| 36 | int shift = immhb - (8 << size); | ||
| 37 | |||
| 38 | - if (extract32(immh, 3, 1) && !is_q) { | ||
| 39 | - unallocated_encoding(s); | ||
| 40 | - return; | ||
| 41 | - } | ||
| 42 | + /* Range of size is limited by decode: immh is a non-zero 4 bit field */ | ||
| 43 | + assert(size >= 0 && size <= 3); | ||
| 44 | |||
| 45 | - if (size > 3 && !is_q) { | ||
| 46 | + if (extract32(immh, 3, 1) && !is_q) { | ||
| 47 | unallocated_encoding(s); | ||
| 48 | return; | ||
| 49 | } | ||
| 50 | -- | ||
| 51 | 2.19.1 | ||
| 52 | |||
| 53 | diff view generated by jsdifflib |
| 1 | Check the return value from load_image_targphys(), which tells us | 1 | From: Richard Henderson <richard.henderson@linaro.org> |
|---|---|---|---|
| 2 | whether our attempt to load the BIOS image into RAM failed. | ||
| 3 | (Spotted by Coverity, CID 1190305.) | ||
| 4 | 2 | ||
| 3 | Only perform the extract of GP during the stage1 walk. | ||
| 4 | |||
| 5 | Reported-by: Peter Maydell <peter.maydell@linaro.org> | ||
| 6 | Signed-off-by: Richard Henderson <richard.henderson@linaro.org> | ||
| 7 | Reviewed-by: Peter Maydell <peter.maydell@linaro.org> | ||
| 8 | Message-id: 20230407185149.3253946-2-richard.henderson@linaro.org | ||
| 5 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | 9 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> |
| 6 | Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> | ||
| 7 | Acked-by: Michael Walle <michael@walle.cc> | ||
| 8 | Message-id: 20181030170032.1844-1-peter.maydell@linaro.org | ||
| 9 | --- | 10 | --- |
| 10 | hw/lm32/milkymist.c | 5 ++++- | 11 | target/arm/ptw.c | 10 +++++----- |
| 11 | 1 file changed, 4 insertions(+), 1 deletion(-) | 12 | 1 file changed, 5 insertions(+), 5 deletions(-) |
| 12 | 13 | ||
| 13 | diff --git a/hw/lm32/milkymist.c b/hw/lm32/milkymist.c | 14 | diff --git a/target/arm/ptw.c b/target/arm/ptw.c |
| 14 | index XXXXXXX..XXXXXXX 100644 | 15 | index XXXXXXX..XXXXXXX 100644 |
| 15 | --- a/hw/lm32/milkymist.c | 16 | --- a/target/arm/ptw.c |
| 16 | +++ b/hw/lm32/milkymist.c | 17 | +++ b/target/arm/ptw.c |
| 17 | @@ -XXX,XX +XXX,XX @@ milkymist_init(MachineState *machine) | 18 | @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw, |
| 18 | bios_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name); | 19 | result->f.attrs.secure = false; |
| 19 | 20 | } | |
| 20 | if (bios_filename) { | 21 | |
| 21 | - load_image_targphys(bios_filename, BIOS_OFFSET, BIOS_SIZE); | 22 | - /* When in aarch64 mode, and BTI is enabled, remember GP in the TLB. */ |
| 22 | + if (load_image_targphys(bios_filename, BIOS_OFFSET, BIOS_SIZE) < 0) { | 23 | - if (aarch64 && cpu_isar_feature(aa64_bti, cpu)) { |
| 23 | + error_report("could not load bios '%s'", bios_filename); | 24 | - result->f.guarded = extract64(attrs, 50, 1); /* GP */ |
| 24 | + exit(1); | 25 | - } |
| 26 | - | ||
| 27 | if (regime_is_stage2(mmu_idx)) { | ||
| 28 | result->cacheattrs.is_s2_format = true; | ||
| 29 | result->cacheattrs.attrs = extract32(attrs, 2, 4); | ||
| 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 */ | ||
| 25 | + } | 38 | + } |
| 26 | } | 39 | } |
| 27 | 40 | ||
| 28 | reset_info->bootstrap_pc = BIOS_OFFSET; | 41 | /* |
| 29 | -- | 42 | -- |
| 30 | 2.19.1 | 43 | 2.34.1 |
| 31 | |||
| 32 | diff view generated by jsdifflib |
Deleted patch | |||
|---|---|---|---|
| 1 | In exynos4210_init() we allocate memory for an Exynos4210State | ||
| 2 | struct. Generally devices can assume that the memory allocated | ||
| 3 | for their state struct is zero-initialized; we broke that | ||
| 4 | assumption here by using g_new(). Use g_new0() instead. | ||
| 5 | (In particular, some code assumes that the various irq arrays | ||
| 6 | in the Exynos4210Irq sub-struct are zero-initialized.) | ||
| 7 | 1 | ||
| 8 | In the longer term, this code should be QOMified, and then | ||
| 9 | the struct memory will be allocated elsewhere and by functions | ||
| 10 | which always zero-initalize it; but for 3.1 this is a | ||
| 11 | simple fix. | ||
| 12 | |||
| 13 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | ||
| 14 | Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> | ||
| 15 | Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com> | ||
| 16 | Message-id: 20181105151132.13884-1-peter.maydell@linaro.org | ||
| 17 | --- | ||
| 18 | hw/arm/exynos4210.c | 2 +- | ||
| 19 | 1 file changed, 1 insertion(+), 1 deletion(-) | ||
| 20 | |||
| 21 | diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c | ||
| 22 | index XXXXXXX..XXXXXXX 100644 | ||
| 23 | --- a/hw/arm/exynos4210.c | ||
| 24 | +++ b/hw/arm/exynos4210.c | ||
| 25 | @@ -XXX,XX +XXX,XX @@ static uint64_t exynos4210_calc_affinity(int cpu) | ||
| 26 | |||
| 27 | Exynos4210State *exynos4210_init(MemoryRegion *system_mem) | ||
| 28 | { | ||
| 29 | - Exynos4210State *s = g_new(Exynos4210State, 1); | ||
| 30 | + Exynos4210State *s = g_new0(Exynos4210State, 1); | ||
| 31 | qemu_irq gate_irq[EXYNOS4210_NCPUS][EXYNOS4210_IRQ_GATE_NINPUTS]; | ||
| 32 | SysBusDevice *busdev; | ||
| 33 | DeviceState *dev; | ||
| 34 | -- | ||
| 35 | 2.19.1 | ||
| 36 | |||
| 37 | diff view generated by jsdifflib |
Deleted patch | |||
|---|---|---|---|
| 1 | In do_ats_write() we construct a PAR value based on the result | ||
| 2 | of the translation. A comment says "S2WLK and FSTAGE are always | ||
| 3 | zero, because we don't implement virtualization". | ||
| 4 | Since we do in fact now implement virtualization, add the missing | ||
| 5 | code that sets these bits based on the reported ARMMMUFaultInfo. | ||
| 6 | 1 | ||
| 7 | (These bits are named PTW and S in ARMv8, so we follow that | ||
| 8 | convention in the new comments in this patch.) | ||
| 9 | |||
| 10 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | ||
| 11 | Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> | ||
| 12 | Reviewed-by: Alex Bennée <alex.bennee@linaro.org> | ||
| 13 | Message-id: 20181016093703.10637-2-peter.maydell@linaro.org | ||
| 14 | --- | ||
| 15 | target/arm/helper.c | 10 ++++++---- | ||
| 16 | 1 file changed, 6 insertions(+), 4 deletions(-) | ||
| 17 | |||
| 18 | diff --git a/target/arm/helper.c b/target/arm/helper.c | ||
| 19 | index XXXXXXX..XXXXXXX 100644 | ||
| 20 | --- a/target/arm/helper.c | ||
| 21 | +++ b/target/arm/helper.c | ||
| 22 | @@ -XXX,XX +XXX,XX @@ static uint64_t do_ats_write(CPUARMState *env, uint64_t value, | ||
| 23 | |||
| 24 | par64 |= 1; /* F */ | ||
| 25 | par64 |= (fsr & 0x3f) << 1; /* FS */ | ||
| 26 | - /* Note that S2WLK and FSTAGE are always zero, because we don't | ||
| 27 | - * implement virtualization and therefore there can't be a stage 2 | ||
| 28 | - * fault. | ||
| 29 | - */ | ||
| 30 | + if (fi.stage2) { | ||
| 31 | + par64 |= (1 << 9); /* S */ | ||
| 32 | + } | ||
| 33 | + if (fi.s1ptw) { | ||
| 34 | + par64 |= (1 << 8); /* PTW */ | ||
| 35 | + } | ||
| 36 | } | ||
| 37 | } else { | ||
| 38 | /* fsr is a DFSR/IFSR value for the short descriptor | ||
| 39 | -- | ||
| 40 | 2.19.1 | ||
| 41 | |||
| 42 | diff view generated by jsdifflib |
| 1 | ATS1HR and ATS1HW (which allow AArch32 EL2 to do address translations | 1 | From: Richard Henderson <richard.henderson@linaro.org> |
|---|---|---|---|
| 2 | on the EL2 translation regime) were implemented in commit 14db7fe09a2c8. | ||
| 3 | However, we got them wrong: these should do stage 1 address translations | ||
| 4 | as defined for NS-EL2, which is ARMMMUIdx_S1E2. We were incorrectly | ||
| 5 | making them perform stage 2 translations. | ||
| 6 | 2 | ||
| 7 | A few years later in commit 1313e2d7e2cd we forgot entirely that | 3 | The guarded bit comes from the stage1 walk. |
| 8 | we'd implemented ATS1Hx, and added a comment that ATS1Hx were | ||
| 9 | "not supported yet". Remove the comment; there is no extra code | ||
| 10 | needed to handle these operations in do_ats_write(), because | ||
| 11 | arm_s1_regime_using_lpae_format() returns true for ARMMMUIdx_S1E2, | ||
| 12 | which forces 64-bit PAR format. | ||
| 13 | 4 | ||
| 5 | Fixes: Coverity CID 1507929 | ||
| 6 | Signed-off-by: Richard Henderson <richard.henderson@linaro.org> | ||
| 7 | Reviewed-by: Peter Maydell <peter.maydell@linaro.org> | ||
| 8 | Message-id: 20230407185149.3253946-3-richard.henderson@linaro.org | ||
| 14 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | 9 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> |
| 15 | Reviewed-by: Alex Bennée <alex.bennee@linaro.org> | ||
| 16 | Message-id: 20181016093703.10637-3-peter.maydell@linaro.org | ||
| 17 | Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> | ||
| 18 | --- | 10 | --- |
| 19 | target/arm/helper.c | 4 ++-- | 11 | target/arm/ptw.c | 1 + |
| 20 | 1 file changed, 2 insertions(+), 2 deletions(-) | 12 | 1 file changed, 1 insertion(+) |
| 21 | 13 | ||
| 22 | 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 |
| 23 | index XXXXXXX..XXXXXXX 100644 | 15 | index XXXXXXX..XXXXXXX 100644 |
| 24 | --- a/target/arm/helper.c | 16 | --- a/target/arm/ptw.c |
| 25 | +++ b/target/arm/helper.c | 17 | +++ b/target/arm/ptw.c |
| 26 | @@ -XXX,XX +XXX,XX @@ static uint64_t do_ats_write(CPUARMState *env, uint64_t value, | 18 | @@ -XXX,XX +XXX,XX @@ static ARMCacheAttrs combine_cacheattrs(uint64_t hcr, |
| 27 | * | 19 | |
| 28 | * (Note that HCR.DC makes HCR.VM behave as if it is 1.) | 20 | assert(!s1.is_s2_format); |
| 29 | * | 21 | ret.is_s2_format = false; |
| 30 | - * ATS1Hx always uses the 64bit format (not supported yet). | 22 | + ret.guarded = s1.guarded; |
| 31 | + * ATS1Hx always uses the 64bit format. | 23 | |
| 32 | */ | 24 | if (s1.attrs == 0xf0) { |
| 33 | format64 = arm_s1_regime_using_lpae_format(env, mmu_idx); | 25 | tagged = true; |
| 34 | |||
| 35 | @@ -XXX,XX +XXX,XX @@ static void ats1h_write(CPUARMState *env, const ARMCPRegInfo *ri, | ||
| 36 | MMUAccessType access_type = ri->opc2 & 1 ? MMU_DATA_STORE : MMU_DATA_LOAD; | ||
| 37 | uint64_t par64; | ||
| 38 | |||
| 39 | - par64 = do_ats_write(env, value, access_type, ARMMMUIdx_S2NS); | ||
| 40 | + par64 = do_ats_write(env, value, access_type, ARMMMUIdx_S1E2); | ||
| 41 | |||
| 42 | A32_BANKED_CURRENT_REG_SET(env, par, par64); | ||
| 43 | } | ||
| 44 | -- | 26 | -- |
| 45 | 2.19.1 | 27 | 2.34.1 |
| 46 | |||
| 47 | diff view generated by jsdifflib |