[Qemu-devel] [PULL 0/5] target-arm queue

[Qemu-devel] [PULL 0/5] target-arm queue

[Peter Maydell]

Handful of bugfix patches for arm for rc0; also
one milkymist patch, thrown in since I was putting
the pullreq together anyway.

thanks
-- PMM

The following changes since commit 03c1ca1c51783603d42eb0f91d35961f0f4b4947:

  Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20181105' into staging (2018-11-06 09:10:46 +0000)

are available in the Git repository at:

  https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20181106

for you to fetch changes up to 23463e0e4aeb2f0a9c60549a2c163f4adc0b8512:

  target/arm: Fix ATS1Hx instructions (2018-11-06 11:32:14 +0000)

----------------------------------------------------------------
target-arm queue:
 * Remove can't-happen if() from handle_vec_simd_shli()
 * hw/arm/exynos4210: Zero memory allocated for Exynos4210State
 * Set S and PTW in 64-bit PAR format
 * Fix ATS1Hx instructions
 * milkymist: Check for failure trying to load BIOS image

----------------------------------------------------------------
Peter Maydell (5):
      target/arm: Remove can't-happen if() from handle_vec_simd_shli()
      milkymist: Check for failure trying to load BIOS image
      hw/arm/exynos4210: Zero memory allocated for Exynos4210State
      target/arm: Set S and PTW in 64-bit PAR format
      target/arm: Fix ATS1Hx instructions

 hw/arm/exynos4210.c        |  2 +-
 hw/lm32/milkymist.c        |  5 ++++-
 target/arm/helper.c        | 14 ++++++++------
 target/arm/translate-a64.c |  8 +++-----
 4 files changed, 16 insertions(+), 13 deletions(-)

[Qemu-devel] [PULL 1/5] target/arm: Remove can't-happen if() from handle_vec_simd_shli()

[Peter Maydell]

In handle_vec_simd_shli() we have a check:
     if (size > 3 && !is_q) {
         unallocated_encoding(s);
         return;
     }
However this can never be true, because we calculate
    int size = 32 - clz32(immh) - 1;
where immh is a 4 bit field which we know cannot be all-zeroes.
So the clz32() return must be in {28,29,30,31} and the resulting
size is in {0,1,2,3}, and "size > 3" is never true.

This unnecessary code confuses Coverity's analysis:
in CID 1396476 it thinks we might later index off the
end of an array because the condition implies that we
might have a size > 3.

Remove the code, and instead assert that the size is in [0..3],
since the decode that enforces that is somewhat distant from
this function.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Tested-by: Alex Bennée <alex.bennee@linaro.org>
Message-id: 20181030162517.21816-1-peter.maydell@linaro.org
---
 target/arm/translate-a64.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 88195ab9490..fd36425f1ae 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -9483,12 +9483,10 @@ static void handle_vec_simd_shli(DisasContext *s, bool is_q, bool insert,
     int immhb = immh << 3 | immb;
     int shift = immhb - (8 << size);
 
-    if (extract32(immh, 3, 1) && !is_q) {
-        unallocated_encoding(s);
-        return;
-    }
+    /* Range of size is limited by decode: immh is a non-zero 4 bit field */
+    assert(size >= 0 && size <= 3);
 
-    if (size > 3 && !is_q) {
+    if (extract32(immh, 3, 1) && !is_q) {
         unallocated_encoding(s);
         return;
     }
-- 
2.19.1

[Qemu-devel] [PULL 2/5] milkymist: Check for failure trying to load BIOS image

[Peter Maydell]

Check the return value from load_image_targphys(), which tells us
whether our attempt to load the BIOS image into RAM failed.
(Spotted by Coverity, CID 1190305.)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Acked-by: Michael Walle <michael@walle.cc>
Message-id: 20181030170032.1844-1-peter.maydell@linaro.org
---
 hw/lm32/milkymist.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/lm32/milkymist.c b/hw/lm32/milkymist.c
index 321f184595e..63c6894c955 100644
--- a/hw/lm32/milkymist.c
+++ b/hw/lm32/milkymist.c
@@ -138,7 +138,10 @@ milkymist_init(MachineState *machine)
     bios_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
 
     if (bios_filename) {
-        load_image_targphys(bios_filename, BIOS_OFFSET, BIOS_SIZE);
+        if (load_image_targphys(bios_filename, BIOS_OFFSET, BIOS_SIZE) < 0) {
+            error_report("could not load bios '%s'", bios_filename);
+            exit(1);
+        }
     }
 
     reset_info->bootstrap_pc = BIOS_OFFSET;
-- 
2.19.1

[Qemu-devel] [PULL 3/5] hw/arm/exynos4210: Zero memory allocated for Exynos4210State

[Peter Maydell]

In exynos4210_init() we allocate memory for an Exynos4210State
struct. Generally devices can assume that the memory allocated
for their state struct is zero-initialized; we broke that
assumption here by using g_new(). Use g_new0() instead.
(In particular, some code assumes that the various irq arrays
in the Exynos4210Irq sub-struct are zero-initialized.)

In the longer term, this code should be QOMified, and then
the struct memory will be allocated elsewhere and by functions
which always zero-initalize it; but for 3.1 this is a
simple fix.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20181105151132.13884-1-peter.maydell@linaro.org
---
 hw/arm/exynos4210.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index 827318a0036..af82e955421 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -162,7 +162,7 @@ static uint64_t exynos4210_calc_affinity(int cpu)
 
 Exynos4210State *exynos4210_init(MemoryRegion *system_mem)
 {
-    Exynos4210State *s = g_new(Exynos4210State, 1);
+    Exynos4210State *s = g_new0(Exynos4210State, 1);
     qemu_irq gate_irq[EXYNOS4210_NCPUS][EXYNOS4210_IRQ_GATE_NINPUTS];
     SysBusDevice *busdev;
     DeviceState *dev;
-- 
2.19.1

[Qemu-devel] [PULL 4/5] target/arm: Set S and PTW in 64-bit PAR format

[Peter Maydell]

In do_ats_write() we construct a PAR value based on the result
of the translation.  A comment says "S2WLK and FSTAGE are always
zero, because we don't implement virtualization".
Since we do in fact now implement virtualization, add the missing
code that sets these bits based on the reported ARMMMUFaultInfo.

(These bits are named PTW and S in ARMv8, so we follow that
convention in the new comments in this patch.)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Message-id: 20181016093703.10637-2-peter.maydell@linaro.org
---
 target/arm/helper.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 0ea95b08151..69f684abd89 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -2347,10 +2347,12 @@ static uint64_t do_ats_write(CPUARMState *env, uint64_t value,
 
             par64 |= 1; /* F */
             par64 |= (fsr & 0x3f) << 1; /* FS */
-            /* Note that S2WLK and FSTAGE are always zero, because we don't
-             * implement virtualization and therefore there can't be a stage 2
-             * fault.
-             */
+            if (fi.stage2) {
+                par64 |= (1 << 9); /* S */
+            }
+            if (fi.s1ptw) {
+                par64 |= (1 << 8); /* PTW */
+            }
         }
     } else {
         /* fsr is a DFSR/IFSR value for the short descriptor
-- 
2.19.1

[Qemu-devel] [PULL 5/5] target/arm: Fix ATS1Hx instructions

[Peter Maydell]

ATS1HR and ATS1HW (which allow AArch32 EL2 to do address translations
on the EL2 translation regime) were implemented in commit 14db7fe09a2c8.
However, we got them wrong: these should do stage 1 address translations
as defined for NS-EL2, which is ARMMMUIdx_S1E2. We were incorrectly
making them perform stage 2 translations.

A few years later in commit 1313e2d7e2cd we forgot entirely that
we'd implemented ATS1Hx, and added a comment that ATS1Hx were
"not supported yet". Remove the comment; there is no extra code
needed to handle these operations in do_ats_write(), because
arm_s1_regime_using_lpae_format() returns true for ARMMMUIdx_S1E2,
which forces 64-bit PAR format.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Message-id: 20181016093703.10637-3-peter.maydell@linaro.org
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target/arm/helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 69f684abd89..96301930cc8 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -2319,7 +2319,7 @@ static uint64_t do_ats_write(CPUARMState *env, uint64_t value,
          *
          * (Note that HCR.DC makes HCR.VM behave as if it is 1.)
          *
-         * ATS1Hx always uses the 64bit format (not supported yet).
+         * ATS1Hx always uses the 64bit format.
          */
         format64 = arm_s1_regime_using_lpae_format(env, mmu_idx);
 
@@ -2444,7 +2444,7 @@ static void ats1h_write(CPUARMState *env, const ARMCPRegInfo *ri,
     MMUAccessType access_type = ri->opc2 & 1 ? MMU_DATA_STORE : MMU_DATA_LOAD;
     uint64_t par64;
 
-    par64 = do_ats_write(env, value, access_type, ARMMMUIdx_S2NS);
+    par64 = do_ats_write(env, value, access_type, ARMMMUIdx_S1E2);
 
     A32_BANKED_CURRENT_REG_SET(env, par, par64);
 }
-- 
2.19.1

Re: [Qemu-devel] [PULL 0/5] target-arm queue

[Peter Maydell]

On 6 November 2018 at 11:38, Peter Maydell <peter.maydell@linaro.org> wrote:
> Handful of bugfix patches for arm for rc0; also
> one milkymist patch, thrown in since I was putting
> the pullreq together anyway.
>
> thanks
> -- PMM
>
> The following changes since commit 03c1ca1c51783603d42eb0f91d35961f0f4b4947:
>
>   Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20181105' into staging (2018-11-06 09:10:46 +0000)
>
> are available in the Git repository at:
>
>   https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20181106
>
> for you to fetch changes up to 23463e0e4aeb2f0a9c60549a2c163f4adc0b8512:
>
>   target/arm: Fix ATS1Hx instructions (2018-11-06 11:32:14 +0000)
>
> ----------------------------------------------------------------
> target-arm queue:
>  * Remove can't-happen if() from handle_vec_simd_shli()
>  * hw/arm/exynos4210: Zero memory allocated for Exynos4210State
>  * Set S and PTW in 64-bit PAR format
>  * Fix ATS1Hx instructions
>  * milkymist: Check for failure trying to load BIOS image
>

Applied, thanks.

-- PMM