On 06.09.19 09:57, David Hildenbrand wrote:
> This is the successor of
> "[PATCH v1 0/4] s390x/tcg: MOVE (MVC): Fault-safe handling"
>
> ----
>
> This series fixes a bunch of issues related to some mem helpers and makes
> sure that they are fault-safe, meaning no system state is modified in case
> a fault is triggered.
>
> I can spot tons of other issues with other mem helpers that will have
> to be fixed later. Also, fault-safe handling for some instructions
> (especially TR) might be harder to implement (you don't know what will
> actually be accessed upfront - we might need a buffer and go over
> inputs twice). Focusing on the MOVE instructions for now.
>
> ----
>
> Newer versions of glibc use memcpy() in memmove() for forward moves. The
> implementation makese use of MVC. The TCG implementation of MVC is
> currently not able to handle faults reliably when crossing pages. MVC
> can cross with 256 bytes at most two pages.
>
> In case we get a fault on the second page, we already moved data. When
> continuing after the fault we might try to move already overwritten data,
> which is very bad in case we have overlapping data on a forward move.
>
> Triggered for now only by rpmbuild (crashes when checking the spec file)
> and rpm (database corruptions). This fixes installing Fedora rawhide (31)
> under TCG.
>
> This was horrible to debug as it barely triggers and we fail at completely
> different places.
>
> Cc: Stefano Brivio <sbrivio@redhat.com>
> Cc: Florian Weimer <fweimer@redhat.com>
>
> v1 -> v2:
> - Include many fixes
> - Fix more instructions
> - Use the new probe_access() function
> - Include "tests/tcg: target/s390x: Test MVO"
>
> David Hildenbrand (28):
> s390x/tcg: Reset exception_index to -1 instead of 0
> s390x/tcg: MVCL: Zero out unused bits of address
> s390x/tcg: MVCL: Detect destructive overlaps
> s390x/tcg: MVCL: Process max 2k bytes at a time
> s390x/tcg: MVC: Increment the length once
> s390x/tcg: MVC: Use is_destructive_overlap()
> s390x/tcg: MVPG: Check for specification exceptions
> s390x/tcg: MVPG: Properly wrap the addresses
> s390x/tcg: MVCLU/MVCLE: Process max 4k bytes at a time
> s390x/tcg: MVCS/MVCP: Check for special operation exceptions
> s390x/tcg: MVCS/MVCP: Properly wrap the length
> s390x/tcg: MVST: Check for specification exceptions
> s390x/tcg: MVST: Fix storing back the addresses to registers
> s390x/tcg: Always use MMU_USER_IDX for CONFIG_USER_ONLY
> s390x/tcg: Fault-safe memset
> s390x/tcg: Fault-safe memmove
> s390x/tcg: MVCS/MVCP: Use access_memmove_idx()
> s390x/tcg: MVC: Fault-safe handling on destructive overlaps
> s390x/tcg: MVCLU: Fault-safe handling
> s390x/tcg: OC: Fault-safe handling
> s390x/tcg: XC: Fault-safe handling
> s390x/tcg: NC: Fault-safe handling
> s390x/tcg: MVCIN: Fault-safe handling
> s390x/tcg: MVN: Fault-safe handling
> s390x/tcg: MVZ: Fault-safe handling
> s390x/tcg: MVST: Fault-safe handling
> s390x/tcg: MVO: Fault-safe handling
> tests/tcg: target/s390x: Test MVO
>
> target/s390x/cpu.h | 4 +
> target/s390x/helper.h | 2 +-
> target/s390x/insn-data.def | 2 +-
> target/s390x/mem_helper.c | 672 ++++++++++++++++++++++----------
> target/s390x/translate.c | 12 +-
> tests/tcg/s390x/Makefile.target | 1 +
> tests/tcg/s390x/mvo.c | 25 ++
> 7 files changed, 507 insertions(+), 211 deletions(-)
> create mode 100644 tests/tcg/s390x/mvo.c
>
I guess, for a minimal review besides the obvious fixes, looking at
"s390x/tcg: Fault-safe memset" and "s390x/tcg: Fault-safe memmove" and
"s390x/tcg: MVC: Fault-safe handling on destructive overlaps" makes
sense. They contain the real magic used within remaining patches.
--
Thanks,
David / dhildenb