Bug 392146

I will provide a patch

Created attachment 111541 [details]
Iplements "mrs <reg>, mdir_el1" for VEX aarch64

Hi; your patch consistently typos the register name -- it is MIDR_EL1 (for Main ID Register).

This register is only accessible at EL1 in hardware, though -- I guess your host kernel is emulating accesses to it? Are there other ID register accesses that also now need to be emulated in valgrind because the kernel passes them through to userspace?

(In reply to Peter Maydell from comment #3)
> Hi; your patch consistently typos the register name -- it is MIDR_EL1 (for
> Main ID Register).
> 

Uups, thanks for noting. 

> This register is only accessible at EL1 in hardware, though -- I guess your
> host kernel is emulating accesses to it? Are there other ID register
> accesses that also now need to be emulated in valgrind because the kernel
> passes them through to userspace?

I'll have to investigate further on this.

I'll update the after I can confirm that we only need MIDR access, or I'll add the necessary parts as well.

the following system registers are emulated by the kernel:
MIDR_EL1
MPIDR_EL1
REVIDR_EL1
ID_AA64PFR0_EL1
ID_AA64PFR1_EL1
ID_AA64ZFR0_EL1
ID_AA64DFR0_EL1
ID_AA64DFR1_EL1
ID_AA64AFR0_EL1
ID_AA64AFR1_EL1
ID_AA64ISAR0_EL1
ID_AA64ISAR1_EL1
ID_AA64MMFR0_EL1
ID_AA64MMFR1_EL1
ID_AA64MMFR2_EL1

I have a patch which just puts faked values into the registers in guest_arm64_toIR.c

But I wonder if we would need to actually provide the real values, as this may have influence on the program flow of the code under inspection.

The registers give information about the underlying HW, like manufacturer, but also about which HW enhancements are present.

Is my assumption correct, that we will need to provide the host values of this registers? Can anyone give guidance in which file/data structure this should be done.

I think you were already told on IRC when you asked that it should provide results which reflect the capabilities of the emulated environment, which in at least some cases is likely to mean not passing on host values.

Created attachment 112966 [details]
implementation of system register emulation

Emulate all system register accesses. Return only the bits that are supported by the emulation environment.

Created attachment 113113 [details]
Implement emulated system registers. Fixes #392146.

Updated version of the patch. I flip the registers of the bits which are not supported in the emulation environment in the dirty helper. Following the example of amd's cpuid.

Please provide some feedback about the patch.

any comments on that?

Julian, Peter, any comments on this patch?

*** Bug 482013 has been marked as a duplicate of this bug. ***

Do you have any tests for this?

I used to have a test for this, but I lost that over the last 6 years.

But under "Implementation" https://bugs.kde.org/show_bug.cgi?id=482013 gives a code snipped that can be easily used to create a test.

Well it's time for me to look at this again. I just tried to run Firefox on FreeBSD under Valgrind and I got

ARM64 front end: branch_etc
disInstr(arm64): unhandled instruction 0xD5380609
disInstr(arm64): 1101'0101 0011'1000 0000'0110 0000'1001
==2219== valgrind: Unrecognised instruction at address 0xdfa7880.
==2219==    at 0xDFA7880: SkCpu::CacheRuntimeFeatures() (in /usr/local/lib/firefox/libxul.so)

And the patch seems to work, with the patch FF proceeds until it hits a SIGBUS (which is what I was looking for).

I'll look at it in more detail.

The Linux kernel documents which ID registers and which fields in those registers it exposes here: https://www.kernel.org/doc/Documentation/arm64/cpu-feature-registers.txt . Does FreeBSD expose the same set, or a different set?

(In reply to Peter Maydell from comment #17)
> The Linux kernel documents which ID registers and which fields in those
> registers it exposes here:
> https://www.kernel.org/doc/Documentation/arm64/cpu-feature-registers.txt .
> Does FreeBSD expose the same set, or a different set?

I don't know of any such document. I expect that they are similar since FreeBSD has a linux compatibility layer and it sets possibly different HWCAPs for FreeBSD apps and for Linux apps running under the compat layer.

*** Bug 450203 has been marked as a duplicate of this bug. ***

Created attachment 168951 [details]
Attachment from 466732

*** Bug 466732 has been marked as a duplicate of this bug. ***

(In reply to Paul Floyd from comment #13)
> Do you have any tests for this?

Unfortunately after such a long time I'm not able to find the reproducer. At that time I remeber that is was some code in glibc that invoked reads of MDIR_EL1.

Never mind. I'm going to have to go through everything thoroughly.

I asked the FreeBSD dev that worked on aarch64 and he pointed me to this github repo he used during development

https://github.com/zxombie/arm64id
(with macros that make my eyes bleed and some abomination use of __attribute__section to pretend that a bunch of individual structs are a single array)

I'll attach the results for comparison with Linux.

Created attachment 169058 [details]
arm64id output on FreeBSD standalone

Created attachment 169059 [details]
arm64id output on FreeBSD under Valgrind

Created attachment 169061 [details]
arm64id output on linux standalone

Created attachment 169062 [details]
arm64id output on linux under Valgrind

(In reply to Peter Maydell from comment #17)
> The Linux kernel documents which ID registers and which fields in those
> registers it exposes here:
> https://www.kernel.org/doc/Documentation/arm64/cpu-feature-registers.txt .
> Does FreeBSD expose the same set, or a different set?

A different set.

There are 16 handled on linux but not FreeBSD
1 handled on FreeBSD but not linux
and 39 handled by both

Created attachment 169365 [details]
arm64id output on FreeBSD under Valgrind with patch

Not looking too good. Several bits of the patch just write constants whilst FreeBSD does seem to implement the ARM spec.

Going through the patch register by register.

MIDR_EL1 I think that this is OK and can be applied as-is. I might try to write a test that generates the expected on the fly.

MPIDR_EL1 FreeBSD sets the affinity masks and MT. I don't think that is of any concern in Valgrind so I'll ignore it for now.

REVDIR_EL1 same as standalone on both platforms

ID_AA64PFR0_EL1 looks OK. Also turns off all the exception level settings but from what I see that only affects kernel code and userland is all EL0

ID_AA64PFR1_EL1 looks OK, this is turning off a flag that says the CPU can mark regions as speculation safe, meaningless under Valgrind.

ID_AA64ZFR0_EL1 looks OK, FreeBSD sets BRP and WRP - hardware debug registers. Might affect gdb or lldb under Valgrind, or are they only for kernel debuggers?

ID_AA64DFR1_EL1 looks OK manual says this returns 64bits RES0

ID_AA64AFR0_EL1 looks OK manual says this is RES0 / implementation defined

ID_AA64AFR1_EL1 looks OK manual says this returns 64bits RES0

ID_AA64ISAR0_EL1 ISA feature register. Fairly important!

In the patch
+   /* Clear all but AES, SHA1 and SHA2 parts*/
+   w &= ~0xFFFF;

That looks wrong to me. AES and SHA are in the lowest 16 bits, I think that negation shouldn't be there.
We need to set DP bits 47-44  (if available on the underlying hardware) since f42b9a434e12bc14ec821183a69b86e91da0577c added those instructions.
We need to set RDM bits 31-28 since a8c274d0682b7265af7a5a9c71251d48169c9dc9
We need to set Atomic bits 23-20 since f1cf734554320e92f0dfc80125dfc2a7e5356ff2
Finally we need to set CRC32 since 6c99d37dd036d562b6a56cce576d125d0d239a3a
The lowest 16 bits are already covered above.

ID_AA64ISAR1_EL1 more ISA features
OK for the moment, will need to be updated when https://bugs.kde.org/show_bug.cgi?id=440095 gets applied

ID_AA64MMFR0_EL1
ID_AA64MMFR1_EL1
ID_AA64MMFR2_EL1
First two just pass values via dirty helpers. Last one uses constant zero. Looks OK.

So, in summary, I'll look at updating ID_AA64ISAR0_EL1, think about writing some tests.  And look at the other registers later.

I might add a test or two later. Not easy to write something that is really hardware independent.

commit 9ecbb9037b596babf69745f9e212c80b2cef4174 (HEAD -> master, origin/master, origin/HEAD)
Author: Paul Floyd <pjfloyd@wanadoo.fr>
Date:   Sat May 11 13:53:42 2024 +0200

    Bug 392146 - aarch64: unhandled instruction 0xD5380001 (MRS rT, midr_el1)