| Message ID | 20230807114921.438881-1-iii@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] target/s390x: Define TARGET_HAS_PRECISE_SMC | expand |
On 07.08.23 13:48, Ilya Leoshkevich wrote: > PoP (Sequence of Storage References -> Instruction Fetching) says: > > ... if a store that is conceptually earlier is > made by the same CPU using the same effective > address as that by which the instruction is subse- > quently fetched, the updated information is obtained ... > > QEMU already has support for this in the common code; enable it for > s390x. Figuring out what TARGET_HAS_PRECISE_SMC is all about, I only learned from git history commit d720b93d0bcfe1beb729245b9ed1e5f071a24bd5 Author: Fabrice Bellard <fabrice@bellard.org> Date: Sun Apr 25 17:57:43 2004 +0000 precise self modifying code support git-svn-id: svn://svn.savannah.nongnu.org/qemu/trunk@745 c046a42c-6fe2-441c-8c8c-71466251a162 AFAIU, precise SMC is stricter compared to what we have right now. So i suspect that this patch is actually fixing SMC behavior: for example, when a basic block ends up modifying itself. Were there any BUG reports? (does patch #2 test for that and can reproduce the original issue?) > > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> > --- > target/s390x/cpu.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h > index eb5b65b7d3a..304029e57cf 100644 > --- a/target/s390x/cpu.h > +++ b/target/s390x/cpu.h > @@ -36,6 +36,8 @@ > /* The z/Architecture has a strong memory model with some store-after-load re-ordering */ > #define TCG_GUEST_DEFAULT_MO (TCG_MO_ALL & ~TCG_MO_ST_LD) > > +#define TARGET_HAS_PRECISE_SMC > + > #define TARGET_INSN_START_EXTRA_WORDS 2 > > #define MMU_USER_IDX 0
On Mon, 2023-08-07 at 17:31 +0200, David Hildenbrand wrote: > On 07.08.23 13:48, Ilya Leoshkevich wrote: > > PoP (Sequence of Storage References -> Instruction Fetching) says: > > > > ... if a store that is conceptually earlier is > > made by the same CPU using the same effective > > address as that by which the instruction is subse- > > quently fetched, the updated information is obtained ... > > > > QEMU already has support for this in the common code; enable it for > > s390x. > > > Figuring out what TARGET_HAS_PRECISE_SMC is all about, I only learned > from git history > > commit d720b93d0bcfe1beb729245b9ed1e5f071a24bd5 > Author: Fabrice Bellard <fabrice@bellard.org> > Date: Sun Apr 25 17:57:43 2004 +0000 > > precise self modifying code support > > > git-svn-id: svn://svn.savannah.nongnu.org/qemu/trunk@745 > c046a42c-6fe2-441c-8c8c-71466251a162 > > > > AFAIU, precise SMC is stricter compared to what we have right now. So > i > suspect that this patch is actually fixing SMC behavior: for example, > when a basic block ends up modifying itself. > > Were there any BUG reports? (does patch #2 test for that and can > reproduce the original issue?) There were no bug reports, I found this issue with fuzzing. Patch #2 tests a TB modifying itself. Reverting this commit makes the test fail. [...]
On 07.08.23 18:13, Ilya Leoshkevich wrote: > On Mon, 2023-08-07 at 17:31 +0200, David Hildenbrand wrote: >> On 07.08.23 13:48, Ilya Leoshkevich wrote: >>> PoP (Sequence of Storage References -> Instruction Fetching) says: >>> >>> ... if a store that is conceptually earlier is >>> made by the same CPU using the same effective >>> address as that by which the instruction is subse- >>> quently fetched, the updated information is obtained ... >>> >>> QEMU already has support for this in the common code; enable it for >>> s390x. >> >> >> Figuring out what TARGET_HAS_PRECISE_SMC is all about, I only learned >> from git history >> >> commit d720b93d0bcfe1beb729245b9ed1e5f071a24bd5 >> Author: Fabrice Bellard <fabrice@bellard.org> >> Date: Sun Apr 25 17:57:43 2004 +0000 >> >> precise self modifying code support >> >> >> git-svn-id: svn://svn.savannah.nongnu.org/qemu/trunk@745 >> c046a42c-6fe2-441c-8c8c-71466251a162 >> >> >> >> AFAIU, precise SMC is stricter compared to what we have right now. So >> i >> suspect that this patch is actually fixing SMC behavior: for example, >> when a basic block ends up modifying itself. >> >> Were there any BUG reports? (does patch #2 test for that and can >> reproduce the original issue?) > > There were no bug reports, I found this issue with fuzzing. > > Patch #2 tests a TB modifying itself. > Reverting this commit makes the test fail. > > [...] > Cool, thanks. Worth adding to the patch description. Acked-by: David Hildenbrand <david@redhat.com>
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h index eb5b65b7d3a..304029e57cf 100644 --- a/target/s390x/cpu.h +++ b/target/s390x/cpu.h @@ -36,6 +36,8 @@ /* The z/Architecture has a strong memory model with some store-after-load re-ordering */ #define TCG_GUEST_DEFAULT_MO (TCG_MO_ALL & ~TCG_MO_ST_LD) +#define TARGET_HAS_PRECISE_SMC + #define TARGET_INSN_START_EXTRA_WORDS 2 #define MMU_USER_IDX 0
PoP (Sequence of Storage References -> Instruction Fetching) says: ... if a store that is conceptually earlier is made by the same CPU using the same effective address as that by which the instruction is subse- quently fetched, the updated information is obtained ... QEMU already has support for this in the common code; enable it for s390x. Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> --- target/s390x/cpu.h | 2 ++ 1 file changed, 2 insertions(+)