MIDR_EL1 is a 32-bit register.
This fixes when compiling with -Werror=conversion:
target/arm/cpu64.c: In function ‘aarch64_max_initfn’:
target/arm/cpu64.c:628:21: error: conversion from ‘uint64_t’ {aka ‘long unsigned int’} to ‘uint32_t’ {aka ‘unsigned int’} may change value [-Werror=conversion]
628 | cpu->midr = t;
| ^
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
target/arm/cpu64.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 95d0c8c101..4eb0a9030e 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -620,12 +620,12 @@ static void aarch64_max_initfn(Object *obj)
* code needs to distinguish this QEMU CPU from other software
* implementations, though this shouldn't be needed.
*/
- t = FIELD_DP64(0, MIDR_EL1, IMPLEMENTER, 0);
- t = FIELD_DP64(t, MIDR_EL1, ARCHITECTURE, 0xf);
- t = FIELD_DP64(t, MIDR_EL1, PARTNUM, 'Q');
- t = FIELD_DP64(t, MIDR_EL1, VARIANT, 0);
- t = FIELD_DP64(t, MIDR_EL1, REVISION, 0);
- cpu->midr = t;
+ u = FIELD_DP32(0, MIDR_EL1, IMPLEMENTER, 0);
+ u = FIELD_DP32(u, MIDR_EL1, ARCHITECTURE, 0xf);
+ u = FIELD_DP32(u, MIDR_EL1, PARTNUM, 'Q');
+ u = FIELD_DP32(u, MIDR_EL1, VARIANT, 0);
+ u = FIELD_DP32(u, MIDR_EL1, REVISION, 0);
+ cpu->midr = u;
t = cpu->isar.id_aa64isar0;
t = FIELD_DP64(t, ID_AA64ISAR0, AES, 2); /* AES + PMULL */
--
2.21.1
On Thu, Apr 23, 2020 at 2:44 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> MIDR_EL1 is a 32-bit register.
In fact MIDR_EL1 a 64-bit system register with the top 32-bit being RES0.
So the right fix might be to change midr field size, just to be future proof :-)
But if we stick to a 32-bit midr then:
Reviewed-by: Laurent Desnogues <laurent.desnogues@gmail.com>
Thanks,
Laurent
> This fixes when compiling with -Werror=conversion:
>
> target/arm/cpu64.c: In function ‘aarch64_max_initfn’:
> target/arm/cpu64.c:628:21: error: conversion from ‘uint64_t’ {aka ‘long unsigned int’} to ‘uint32_t’ {aka ‘unsigned int’} may change value [-Werror=conversion]
> 628 | cpu->midr = t;
> | ^
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> target/arm/cpu64.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> index 95d0c8c101..4eb0a9030e 100644
> --- a/target/arm/cpu64.c
> +++ b/target/arm/cpu64.c
> @@ -620,12 +620,12 @@ static void aarch64_max_initfn(Object *obj)
> * code needs to distinguish this QEMU CPU from other software
> * implementations, though this shouldn't be needed.
> */
> - t = FIELD_DP64(0, MIDR_EL1, IMPLEMENTER, 0);
> - t = FIELD_DP64(t, MIDR_EL1, ARCHITECTURE, 0xf);
> - t = FIELD_DP64(t, MIDR_EL1, PARTNUM, 'Q');
> - t = FIELD_DP64(t, MIDR_EL1, VARIANT, 0);
> - t = FIELD_DP64(t, MIDR_EL1, REVISION, 0);
> - cpu->midr = t;
> + u = FIELD_DP32(0, MIDR_EL1, IMPLEMENTER, 0);
> + u = FIELD_DP32(u, MIDR_EL1, ARCHITECTURE, 0xf);
> + u = FIELD_DP32(u, MIDR_EL1, PARTNUM, 'Q');
> + u = FIELD_DP32(u, MIDR_EL1, VARIANT, 0);
> + u = FIELD_DP32(u, MIDR_EL1, REVISION, 0);
> + cpu->midr = u;
>
> t = cpu->isar.id_aa64isar0;
> t = FIELD_DP64(t, ID_AA64ISAR0, AES, 2); /* AES + PMULL */
> --
> 2.21.1
>
>
On Thu, 23 Apr 2020 at 14:08, Laurent Desnogues
<laurent.desnogues@gmail.com> wrote:
> On Thu, Apr 23, 2020 at 2:44 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> >
> > MIDR_EL1 is a 32-bit register.
>
> In fact MIDR_EL1 a 64-bit system register with the top 32-bit being RES0.
>
> So the right fix might be to change midr field size, just to be future proof :-)
Yes, I think I prefer changing the midr field size. Looking at the
code this should just be a matter of updating the 'uint32_t midr' in
the CPU struct to 'uint64_t midr' and changing the
DEFINE_PROP_UINT32("midr",...)
in cpu.c to UINT64. (The one user of the property in xlnx-zynqmp.c
doesn't need to change because object_property_set_int() works on
both 32-bit and 64-bit integer properties.)
Mostly we have been fixing up these ID register field size values as
we move them from being top-level ARMCPU fields to being in the
ARMISARegisters struct, but I think midr is unlikely to ever need
to move there because no CPU feature is gated on the MIDR value.
thanks
-- PMM
Patchew URL: https://patchew.org/QEMU/20200423124305.14718-1-f4bug@amsat.org/ Hi, This series failed the asan build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #!/bin/bash export ARCH=x86_64 make docker-image-fedora V=1 NETWORK=1 time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1 === TEST SCRIPT END === The full log is available at http://patchew.org/logs/20200423124305.14718-1-f4bug@amsat.org/testing.asan/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-devel@redhat.com
Patchew URL: https://patchew.org/QEMU/20200423124305.14718-1-f4bug@amsat.org/ Hi, This series failed build test on FreeBSD host. Please find the details below. === TEST SCRIPT BEGIN === #!/bin/bash # Testing script will be invoked under the git checkout with # HEAD pointing to a commit that has the patches applied on top of "base" # branch if qemu-system-x86_64 --help >/dev/null 2>&1; then QEMU=qemu-system-x86_64 elif /usr/libexec/qemu-kvm --help >/dev/null 2>&1; then QEMU=/usr/libexec/qemu-kvm else exit 1 fi make vm-build-freebsd J=21 QEMU=$QEMU exit 0 === TEST SCRIPT END === The full log is available at http://patchew.org/logs/20200423124305.14718-1-f4bug@amsat.org/testing.FreeBSD/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-devel@redhat.com
Patchew URL: https://patchew.org/QEMU/20200423124305.14718-1-f4bug@amsat.org/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Subject: [PATCH] target/arm: Use correct variable for setting 'max' cpu's MIDR_EL1
Message-id: 20200423124305.14718-1-f4bug@amsat.org
Type: series
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
fatal: unable to access 'https://github.com/patchew-project/qemu/': The requested URL returned error: 504
error: Could not fetch 3c8cf5a9c21ff8782164d1def7f44bd888713384
Traceback (most recent call last):
File "patchew-tester/src/patchew-cli", line 521, in test_one
git_clone_repo(clone, r["repo"], r["head"], logf, True)
File "patchew-tester/src/patchew-cli", line 48, in git_clone_repo
stdout=logf, stderr=logf)
File "/opt/rh/rh-python36/root/usr/lib64/python3.6/subprocess.py", line 291, in check_call
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['git', 'remote', 'add', '-f', '--mirror=fetch', '3c8cf5a9c21ff8782164d1def7f44bd888713384', 'https://github.com/patchew-project/qemu']' returned non-zero exit status 1.
The full log is available at
http://patchew.org/logs/20200423124305.14718-1-f4bug@amsat.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Patchew URL: https://patchew.org/QEMU/20200423124305.14718-1-f4bug@amsat.org/ Hi, This series failed the docker-mingw@fedora build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #! /bin/bash export ARCH=x86_64 make docker-image-fedora V=1 NETWORK=1 time make docker-test-mingw@fedora J=14 NETWORK=1 === TEST SCRIPT END === The full log is available at http://patchew.org/logs/20200423124305.14718-1-f4bug@amsat.org/testing.docker-mingw@fedora/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-devel@redhat.com
Patchew URL: https://patchew.org/QEMU/20200423124305.14718-1-f4bug@amsat.org/ Hi, This series failed the docker-quick@centos7 build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #!/bin/bash make docker-image-centos7 V=1 NETWORK=1 time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1 === TEST SCRIPT END === The full log is available at http://patchew.org/logs/20200423124305.14718-1-f4bug@amsat.org/testing.docker-quick@centos7/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-devel@redhat.com
© 2016 - 2024 Red Hat, Inc.