Small pile of bug fixes for rc1. I've included my patches to get

our docs building with Sphinx 3, just for convenience...

The following changes since commit b149dea55cce97cb226683d06af61984a1c11e96:

Merge remote-tracking branch 'remotes/cschoenebeck/tags/pull-9p-20201102' into staging (2020-11-02 10:57:48 +0000)

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

for you to fetch changes up to ffb4fbf90a2f63c9cb33e4bb9f854c79bf04ca4a:

tests/qtest/npcm7xx_rng-test: Disable randomness tests (2020-11-02 16:52:18 +0000)

* target/arm: Fix Neon emulation bugs on big-endian hosts

* target/arm: fix handling of HCR.FB

* target/arm: fix LORID_EL1 access check

* disas/capstone: Fix monitor disassembly of >32 bytes

* hw/arm/smmuv3: Fix potential integer overflow (CID 1432363)

* hw/arm/boot: fix SVE for EL3 direct kernel boot

* hw/display/omap_lcdc: Fix potential NULL pointer dereference

* hw/display/exynos4210_fimd: Fix potential NULL pointer dereference

* target/arm: Get correct MMU index for other-security-state

* configure: Test that gio libs from pkg-config work

* hw/intc/arm_gicv3_cpuif: Make GIC maintenance interrupts work

* docs: Fix building with Sphinx 3

* tests/qtest/npcm7xx_rng-test: Disable randomness tests

AlexChen (2):

hw/display/omap_lcdc: Fix potential NULL pointer dereference

hw/display/exynos4210_fimd: Fix potential NULL pointer dereference

Peter Maydell (9):

target/arm: Fix float16 pairwise Neon ops on big-endian hosts

target/arm: Fix VUDOT/VSDOT (scalar) on big-endian hosts

disas/capstone: Fix monitor disassembly of >32 bytes

target/arm: Get correct MMU index for other-security-state

configure: Test that gio libs from pkg-config work

hw/intc/arm_gicv3_cpuif: Make GIC maintenance interrupts work

scripts/kerneldoc: For Sphinx 3 use c:macro for macros with arguments

qemu-option-trace.rst.inc: Don't use option:: markup

tests/qtest/npcm7xx_rng-test: Disable randomness tests

Philippe Mathieu-Daudé (1):

hw/arm/smmuv3: Fix potential integer overflow (CID 1432363)

From: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>

Secure mode is not exempted from checking SCR_EL3.TLOR, and in the

future HCR_EL2.TLOR when S-EL2 is enabled.

Signed-off-by: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>

target/arm/helper.c | 19 +++++--------------

1 file changed, 5 insertions(+), 14 deletions(-)

@@ -XXX,XX +XXX,XX @@ static uint64_t id_aa64pfr0_read(CPUARMState *env, const ARMCPRegInfo *ri)

#endif

/* Shared logic between LORID and the rest of the LOR* registers.

- * Secure state has already been delt with.

+ * Secure state exclusion has already been dealt with.

*/

-static CPAccessResult access_lor_ns(CPUARMState *env)

+static CPAccessResult access_lor_ns(CPUARMState *env,

+ const ARMCPRegInfo *ri, bool isread)

{

int el = arm_current_el(env);

@@ -XXX,XX +XXX,XX @@ static CPAccessResult access_lor_ns(CPUARMState *env)

return CP_ACCESS_OK;

}

-static CPAccessResult access_lorid(CPUARMState *env, const ARMCPRegInfo *ri,

- bool isread)

-{

- if (arm_is_secure_below_el3(env)) {

- /* Access ok in secure mode. */

- return CP_ACCESS_OK;

- }

- return access_lor_ns(env);

-}

-

static CPAccessResult access_lor_other(CPUARMState *env,

const ARMCPRegInfo *ri, bool isread)

{

@@ -XXX,XX +XXX,XX @@ static CPAccessResult access_lor_other(CPUARMState *env,

/* Access denied in secure mode. */

return CP_ACCESS_TRAP;

}

- return access_lor_ns(env);

+ return access_lor_ns(env, ri, isread);

}

/*

@@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo lor_reginfo[] = {

.type = ARM_CP_CONST, .resetvalue = 0 },

{ .name = "LORID_EL1", .state = ARM_CP_STATE_AA64,

.opc0 = 3, .opc1 = 0, .crn = 10, .crm = 4, .opc2 = 7,

- .access = PL1_R, .accessfn = access_lorid,

+ .access = PL1_R, .accessfn = access_lor_ns,

.type = ARM_CP_CONST, .resetvalue = 0 },

Sphinx 3.2 is pickier than earlier versions about the option:: markup,

and complains about our usage in qemu-option-trace.rst:

../../docs/qemu-option-trace.rst.inc:4:Malformed option description

'[enable=]PATTERN', should look like "opt", "-opt args", "--opt args",

"/opt args" or "+opt args"

In this file, we're really trying to document the different parts of

the top-level --trace option, which qemu-nbd.rst and qemu-img.rst

have already introduced with an option:: markup. So it's not right

to use option:: here anyway. Switch to a different markup

(definition lists) which gives about the same formatted output.

(Unlike option::, this markup doesn't produce index entries; but

at the moment we don't do anything much with indexes anyway, and

in any case I think it doesn't make much sense to have individual

index entries for the sub-parts of the --trace option.)

Tested-by: Stefan Hajnoczi <stefanha@redhat.com>

Message-id: 20201030174700.7204-3-peter.maydell@linaro.org

docs/qemu-option-trace.rst.inc | 6 +++---

1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/docs/qemu-option-trace.rst.inc b/docs/qemu-option-trace.rst.inc

--- a/docs/qemu-option-trace.rst.inc

+++ b/docs/qemu-option-trace.rst.inc

Specify tracing options.

-.. option:: [enable=]PATTERN

+``[enable=]PATTERN``

Immediately enable events matching *PATTERN*

(either event name or a globbing pattern). This option is only

@@ -XXX,XX +XXX,XX @@ Specify tracing options.

Use :option:`-trace help` to print a list of names of trace points.

-.. option:: events=FILE

+``events=FILE``

Immediately enable events listed in *FILE*.

The file must contain one event name (as listed in the ``trace-events-all``

@@ -XXX,XX +XXX,XX @@ Specify tracing options.

available if QEMU has been compiled with the ``simple``, ``log`` or

``ftrace`` tracing backend.

-.. option:: file=FILE

+``file=FILE``

Log output traces to *FILE*.

This option is only available if QEMU has been compiled with

From: Richard Henderson <richard.henderson@linaro.org>

This seems a bit more readable than using offsetof CPU_DoubleU.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Message-id: 20201030022618.785675-5-richard.henderson@linaro.org

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

---

target/arm/translate.c | 13 ++++---------

1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/target/arm/translate.c b/target/arm/translate.c

--- a/target/arm/translate.c

+++ b/target/arm/translate.c

@@ -XXX,XX +XXX,XX @@ static long neon_element_offset(int reg, int element, MemOp size)

return neon_full_reg_offset(reg) + ofs;

}

-static inline long vfp_reg_offset(bool dp, unsigned reg)

+/* Return the offset of a VFP Dreg (dp = true) or VFP Sreg (dp = false). */

+static long vfp_reg_offset(bool dp, unsigned reg)

{

if (dp) {

- return offsetof(CPUARMState, vfp.zregs[reg >> 1].d[reg & 1]);

+ return neon_element_offset(reg, 0, MO_64);

} else {

- long ofs = offsetof(CPUARMState, vfp.zregs[reg >> 2].d[(reg >> 1) & 1]);

- if (reg & 1) {

- ofs += offsetof(CPU_DoubleU, l.upper);

- } else {

- ofs += offsetof(CPU_DoubleU, l.lower);

- }

- return ofs;

+ return neon_element_offset(reg >> 1, reg & 1, MO_32);

}

}

From: AlexChen <alex.chen@huawei.com>

In exynos4210_fimd_update(), the pointer s is dereferinced before

being check if it is valid, which may lead to NULL pointer dereference.

So move the assignment to global_width after checking that the s is valid.

Reported-by: Euler Robot <euler.robot@huawei.com>

Signed-off-by: Alex Chen <alex.chen@huawei.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Message-id: 5F9F8D88.9030102@huawei.com

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

---

hw/display/exynos4210_fimd.c | 4 +++-

1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/display/exynos4210_fimd.c b/hw/display/exynos4210_fimd.c

--- a/hw/display/exynos4210_fimd.c

+++ b/hw/display/exynos4210_fimd.c

@@ -XXX,XX +XXX,XX @@ static void exynos4210_fimd_update(void *opaque)

bool blend = false;

uint8_t *host_fb_addr;

bool is_dirty = false;

- const int global_width = (s->vidtcon[2] & FIMD_VIDTCON2_SIZE_MASK) + 1;

+ int global_width;

if (!s || !s->console || !s->enabled ||

surface_bits_per_pixel(qemu_console_surface(s->console)) == 0) {

return;

}

+

+ global_width = (s->vidtcon[2] & FIMD_VIDTCON2_SIZE_MASK) + 1;

exynos4210_update_resolution(s);

surface = qemu_console_surface(s->console);

The randomness tests in the NPCM7xx RNG test fail intermittently

but fairly frequently. On my machine running the test in a loop:

while QTEST_QEMU_BINARY=./qemu-system-aarch64 ./tests/qtest/npcm7xx_rng-test; do true; done

will fail in less than a minute with an error like:

ERROR:../../tests/qtest/npcm7xx_rng-test.c:256:test_first_byte_runs:

assertion failed (calc_runs_p(buf.l, sizeof(buf) * BITS_PER_BYTE) > 0.01): (0.00286205989 > 0.01)

(Failures have been observed on all 4 of the randomness tests,

not just first_byte_runs.)

It's not clear why these tests are failing like this, but intermittent

failures make CI and merge testing awkward, so disable running them

unless a developer specifically sets QEMU_TEST_FLAKY_RNG_TESTS when

running the test suite, until we work out the cause.

Message-id: 20201102152454.8287-1-peter.maydell@linaro.org

Reviewed-by: Havard Skinnemoen <hskinnemoen@google.com>

tests/qtest/npcm7xx_rng-test.c | 14 ++++++++++----

1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/tests/qtest/npcm7xx_rng-test.c b/tests/qtest/npcm7xx_rng-test.c

--- a/tests/qtest/npcm7xx_rng-test.c

+++ b/tests/qtest/npcm7xx_rng-test.c

@@ -XXX,XX +XXX,XX @@ int main(int argc, char **argv)

qtest_add_func("npcm7xx_rng/enable_disable", test_enable_disable);

qtest_add_func("npcm7xx_rng/rosel", test_rosel);

- qtest_add_func("npcm7xx_rng/continuous/monobit", test_continuous_monobit);

- qtest_add_func("npcm7xx_rng/continuous/runs", test_continuous_runs);

- qtest_add_func("npcm7xx_rng/first_byte/monobit", test_first_byte_monobit);

- qtest_add_func("npcm7xx_rng/first_byte/runs", test_first_byte_runs);

+ /*

+ * These tests fail intermittently; only run them on explicit

+ * request until we figure out why.

+ */

+ if (getenv("QEMU_TEST_FLAKY_RNG_TESTS")) {

+ qtest_add_func("npcm7xx_rng/continuous/monobit", test_continuous_monobit);

+ qtest_add_func("npcm7xx_rng/continuous/runs", test_continuous_runs);

+ qtest_add_func("npcm7xx_rng/first_byte/monobit", test_first_byte_monobit);

+ qtest_add_func("npcm7xx_rng/first_byte/runs", test_first_byte_runs);

+ }

qtest_start("-machine npcm750-evb");

ret = g_test_run();

From: AlexChen <alex.chen@huawei.com>

In omap_lcd_interrupts(), the pointer omap_lcd is dereferinced before

being check if it is valid, which may lead to NULL pointer dereference.

So move the assignment to surface after checking that the omap_lcd is valid

and move surface_bits_per_pixel(surface) to after the surface assignment.

Reported-by: Euler Robot <euler.robot@huawei.com>

Signed-off-by: AlexChen <alex.chen@huawei.com>

Message-id: 5F9CDB8A.9000001@huawei.com

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

---

hw/display/omap_lcdc.c | 10 +++++++---

1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/hw/display/omap_lcdc.c b/hw/display/omap_lcdc.c

--- a/hw/display/omap_lcdc.c

+++ b/hw/display/omap_lcdc.c

@@ -XXX,XX +XXX,XX @@ static void omap_lcd_interrupts(struct omap_lcd_panel_s *s)

static void omap_update_display(void *opaque)

{

struct omap_lcd_panel_s *omap_lcd = (struct omap_lcd_panel_s *) opaque;

- DisplaySurface *surface = qemu_console_surface(omap_lcd->con);

+ DisplaySurface *surface;

draw_line_func draw_line;

int size, height, first, last;

int width, linesize, step, bpp, frame_offset;

hwaddr frame_base;

- if (!omap_lcd || omap_lcd->plm == 1 || !omap_lcd->enable ||

- !surface_bits_per_pixel(surface)) {

+ if (!omap_lcd || omap_lcd->plm == 1 || !omap_lcd->enable) {

+ return;

+ }

+ surface = qemu_console_surface(omap_lcd->con);

+ if (!surface_bits_per_pixel(surface)) {

return;

}

The kerneldoc script currently emits Sphinx markup for a macro with

arguments that uses the c:function directive. This is correct for

Sphinx versions earlier than Sphinx 3, where c:macro doesn't allow

documentation of macros with arguments and c:function is not picky

about the syntax of what it is passed. However, in Sphinx 3 the

c:macro directive was enhanced to support macros with arguments,

and c:function was made more picky about what syntax it accepted.

When kerneldoc is told that it needs to produce output for Sphinx

3 or later, make it emit c:function only for functions and c:macro

for macros with arguments. We assume that anything with a return

type is a function and anything without is a macro.

This fixes the Sphinx error:

/home/petmay01/linaro/qemu-from-laptop/qemu/docs/../include/qom/object.h:155:Error in declarator

If declarator-id with parameters (e.g., 'void f(int arg)'):

Invalid C declaration: Expected identifier in nested name. [error at 25]

DECLARE_INSTANCE_CHECKER ( InstanceType, OBJ_NAME, TYPENAME)

-------------------------^

If parenthesis in noptr-declarator (e.g., 'void (*f(int arg))(double)'):

Error in declarator or parameters

Invalid C declaration: Expecting "(" in parameters. [error at 39]

DECLARE_INSTANCE_CHECKER ( InstanceType, OBJ_NAME, TYPENAME)

---------------------------------------^

Tested-by: Stefan Hajnoczi <stefanha@redhat.com>

Message-id: 20201030174700.7204-2-peter.maydell@linaro.org

scripts/kernel-doc | 18 +++++++++++++++++-

1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/scripts/kernel-doc b/scripts/kernel-doc

index XXXXXXX..XXXXXXX 100755

--- a/scripts/kernel-doc

+++ b/scripts/kernel-doc

@@ -XXX,XX +XXX,XX @@ sub output_function_rst(%) {

    output_highlight_rst($args{'purpose'});

    $start = "\n\n**Syntax**\n\n ``";

} else {

-    print ".. c:function:: ";

+ if ((split(/\./, $sphinx_version))[0] >= 3) {

+ # Sphinx 3 and later distinguish macros and functions and

+ # complain if you use c:function with something that's not

+ # syntactically valid as a function declaration.

+ # We assume that anything with a return type is a function

+ # and anything without is a macro.

+ if ($args{'functiontype'} ne "") {

+ print ".. c:function:: ";

+ } else {

+ print ".. c:macro:: ";

+ }

+ } else {

+ # Older Sphinx don't support documenting macros that take

+ # arguments with c:macro, and don't complain about the use

+ # of c:function for this.

+ print ".. c:function:: ";

+ }

}

if ($args{'functiontype'} ne "") {

    $start .= $args{'functiontype'} . " " . $args{'function'} . " (";

On some hosts (eg Ubuntu Bionic) pkg-config returns a set of

libraries for gio-2.0 which don't actually work when compiling

statically. (Specifically, the returned library string includes

-lmount, but not -lblkid which -lmount depends upon, so linking

fails due to missing symbols.)

Check that the libraries work, and don't enable gio if they don't,

in the same way we do for gnutls.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Message-id: 20200928160402.7961-1-peter.maydell@linaro.org

configure | 10 +++++++++-

1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/configure b/configure

index XXXXXXX..XXXXXXX 100755

--- a/configure

+++ b/configure

@@ -XXX,XX +XXX,XX @@ if test "$static" = yes && test "$mingw32" = yes; then

fi

if $pkg_config --atleast-version=$glib_req_ver gio-2.0; then

- gio=yes

gio_cflags=$($pkg_config --cflags gio-2.0)

gio_libs=$($pkg_config --libs gio-2.0)

gdbus_codegen=$($pkg_config --variable=gdbus_codegen gio-2.0)

if [ ! -x "$gdbus_codegen" ]; then

gdbus_codegen=

fi

+ # Check that the libraries actually work -- Ubuntu 18.04 ships

+ # with pkg-config --static --libs data for gio-2.0 that is missing

+ # -lblkid and will give a link error.

+ write_c_skeleton

+ if compile_prog "" "gio_libs" ; then

+ gio=yes

+ else

+ gio=no

+ fi

else

gio=no

fi

In arm_v7m_mmu_idx_for_secstate() we get the 'priv' level to pass to

armv7m_mmu_idx_for_secstate_and_priv() by calling arm_current_el().

This is incorrect when the security state being queried is not the

current one, because arm_current_el() uses the current security state

to determine which of the banked CONTROL.nPRIV bits to look at.

The effect was that if (for instance) Secure state was in privileged

mode but Non-Secure was not then we would return the wrong MMU index.

The only places where we are using this function in a way that could

trigger this bug are for the stack loads during a v8M function-return

and for the instruction fetch of a v8M SG insn.

Fix the bug by expanding out the M-profile version of the

arm_current_el() logic inline so it can use the passed in secstate

rather than env->v7m.secure.

Message-id: 20201022164408.13214-1-peter.maydell@linaro.org

target/arm/m_helper.c | 3 ++-

1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target/arm/m_helper.c b/target/arm/m_helper.c

--- a/target/arm/m_helper.c

+++ b/target/arm/m_helper.c

@@ -XXX,XX +XXX,XX @@ ARMMMUIdx arm_v7m_mmu_idx_for_secstate_and_priv(CPUARMState *env,

/* Return the MMU index for a v7M CPU in the specified security state */

ARMMMUIdx arm_v7m_mmu_idx_for_secstate(CPUARMState *env, bool secstate)

{

- bool priv = arm_current_el(env) != 0;

+ bool priv = arm_v7m_is_handler_mode(env) ||

+ !(env->v7m.control[secstate] & 1);

return arm_v7m_mmu_idx_for_secstate_and_priv(env, secstate, priv);

}

The helper functions for performing the udot/sdot operations against

a scalar were not using an address-swizzling macro when converting

the index of the scalar element into a pointer into the vm array.

This had no effect on little-endian hosts but meant we generated

incorrect results on big-endian hosts.

For these insns, the index is indexing over group of 4 8-bit values,

so 32 bits per indexed entity, and H4() is therefore what we want.

(For Neon the only possible input indexes are 0 and 1.)

Message-id: 20201028191712.4910-3-peter.maydell@linaro.org

target/arm/vec_helper.c | 4 ++--

1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/arm/vec_helper.c b/target/arm/vec_helper.c

--- a/target/arm/vec_helper.c

+++ b/target/arm/vec_helper.c

@@ -XXX,XX +XXX,XX @@ void HELPER(gvec_sdot_idx_b)(void *vd, void *vn, void *vm, uint32_t desc)

intptr_t index = simd_data(desc);

uint32_t *d = vd;

int8_t *n = vn;

- int8_t *m_indexed = (int8_t *)vm + index * 4;

+ int8_t *m_indexed = (int8_t *)vm + H4(index) * 4;

/* Notice the special case of opr_sz == 8, from aa64/aa32 advsimd.

* Otherwise opr_sz is a multiple of 16.

@@ -XXX,XX +XXX,XX @@ void HELPER(gvec_udot_idx_b)(void *vd, void *vn, void *vm, uint32_t desc)

intptr_t index = simd_data(desc);

uint32_t *d = vd;

uint8_t *n = vn;

- uint8_t *m_indexed = (uint8_t *)vm + index * 4;

+ uint8_t *m_indexed = (uint8_t *)vm + H4(index) * 4;

/* Notice the special case of opr_sz == 8, from aa64/aa32 advsimd.

* Otherwise opr_sz is a multiple of 16.