| Message ID | 20190906075750.14791-1-david@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | s390x/tcg: mem_helper: Fault-safe handling | expand |
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.
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