| Message ID | 20180213161240.19891-1-david@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v1] s390x/tcg: fix disabling/enabling DAT | expand |
On 02/13/2018 08:12 AM, David Hildenbrand wrote: > Currently, all memory accesses go via the MMU of the address space > (primary, secondary, ...). This is bad, because we don't flush the TLB > when disabling/enabling DAT. So we could add a tlb flush. However it > is easier to simply select the MMU we already have in place for real > memory access. > > All we have to do is point at the right MMU and allow to execute these > pages. > > Signed-off-by: David Hildenbrand <david@redhat.com> > --- > > This is necessary to make the upcomming kvm-unit-tests with vmalloc support > pass under TCG. > > target/s390x/cpu.h | 7 ++++++- > target/s390x/mmu_helper.c | 2 +- > target/s390x/translate.c | 10 +++++++--- > 3 files changed, 14 insertions(+), 5 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20180213161240.19891-1-david@redhat.com
Subject: [Qemu-devel] [PATCH v1] s390x/tcg: fix disabling/enabling DAT
=== TEST SCRIPT BEGIN ===
#!/bin/bash
BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0
git config --local diff.renamelimit 0
git config --local diff.renames True
commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done
exit $failed
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
t [tag update] patchew/20180109122252.17670-1-alex.bennee@linaro.org -> patchew/20180109122252.17670-1-alex.bennee@linaro.org
t [tag update] patchew/20180206164815.10084-1-alex.bennee@linaro.org -> patchew/20180206164815.10084-1-alex.bennee@linaro.org
t [tag update] patchew/20180213130356.8885-1-mreitz@redhat.com -> patchew/20180213130356.8885-1-mreitz@redhat.com
t [tag update] patchew/20180213132246.26844-1-peter.maydell@linaro.org -> patchew/20180213132246.26844-1-peter.maydell@linaro.org
t [tag update] patchew/20180213140029.8308-1-peter.maydell@linaro.org -> patchew/20180213140029.8308-1-peter.maydell@linaro.org
t [tag update] patchew/20180213142102.14450-1-stefanha@redhat.com -> patchew/20180213142102.14450-1-stefanha@redhat.com
* [new tag] patchew/20180213161240.19891-1-david@redhat.com -> patchew/20180213161240.19891-1-david@redhat.com
Auto packing the repository in background for optimum performance.
See "git help gc" for manual housekeeping.
Switched to a new branch 'test'
3931f2fdd7 s390x/tcg: fix disabling/enabling DAT
=== OUTPUT BEGIN ===
Checking PATCH 1/1: s390x/tcg: fix disabling/enabling DAT...
WARNING: line over 80 characters
#32: FILE: target/s390x/cpu.h:320:
+#define FLAG_MASK_PSW (FLAG_MASK_PER | FLAG_MASK_DAT | FLAG_MASK_PSTATE \
ERROR: code indent should never use tabs
#32: FILE: target/s390x/cpu.h:320:
+#define FLAG_MASK_PSW^I^I(FLAG_MASK_PER | FLAG_MASK_DAT | FLAG_MASK_PSTATE \$
total: 1 errors, 1 warnings, 51 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
On Tue, 13 Feb 2018 08:31:21 -0800 (PST) no-reply@patchew.org wrote: > Checking PATCH 1/1: s390x/tcg: fix disabling/enabling DAT... > WARNING: line over 80 characters > #32: FILE: target/s390x/cpu.h:320: > +#define FLAG_MASK_PSW (FLAG_MASK_PER | FLAG_MASK_DAT | FLAG_MASK_PSTATE \ I'll ignore this... > > ERROR: code indent should never use tabs > #32: FILE: target/s390x/cpu.h:320: > +#define FLAG_MASK_PSW^I^I(FLAG_MASK_PER | FLAG_MASK_DAT | FLAG_MASK_PSTATE \$ ...and just get rid of the pre-existing tabs here while merging, no need to resend. > > total: 1 errors, 1 warnings, 51 lines checked > > Your patch has style problems, please review. If any of these errors > are false positives report them to the maintainer, see > CHECKPATCH in MAINTAINERS. > > === OUTPUT END === > > Test command exited with code: 1 > > > --- > Email generated automatically by Patchew [http://patchew.org/]. > Please send your feedback to patchew-devel@freelists.org
On Tue, 13 Feb 2018 17:12:40 +0100 David Hildenbrand <david@redhat.com> wrote: > Currently, all memory accesses go via the MMU of the address space > (primary, secondary, ...). This is bad, because we don't flush the TLB > when disabling/enabling DAT. So we could add a tlb flush. However it > is easier to simply select the MMU we already have in place for real > memory access. > > All we have to do is point at the right MMU and allow to execute these > pages. > > Signed-off-by: David Hildenbrand <david@redhat.com> > --- > > This is necessary to make the upcomming kvm-unit-tests with vmalloc support > pass under TCG. > > target/s390x/cpu.h | 7 ++++++- > target/s390x/mmu_helper.c | 2 +- > target/s390x/translate.c | 10 +++++++--- > 3 files changed, 14 insertions(+), 5 deletions(-) Thanks, applied (with tabs removed).
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h index 21ce40d5b6..c8008df845 100644 --- a/target/s390x/cpu.h +++ b/target/s390x/cpu.h @@ -312,11 +312,12 @@ extern const struct VMStateDescription vmstate_s390_cpu; #define FLAG_MASK_PSW_SHIFT 31 #define FLAG_MASK_PER (PSW_MASK_PER >> FLAG_MASK_PSW_SHIFT) +#define FLAG_MASK_DAT (PSW_MASK_DAT >> FLAG_MASK_PSW_SHIFT) #define FLAG_MASK_PSTATE (PSW_MASK_PSTATE >> FLAG_MASK_PSW_SHIFT) #define FLAG_MASK_ASC (PSW_MASK_ASC >> FLAG_MASK_PSW_SHIFT) #define FLAG_MASK_64 (PSW_MASK_64 >> FLAG_MASK_PSW_SHIFT) #define FLAG_MASK_32 (PSW_MASK_32 >> FLAG_MASK_PSW_SHIFT) -#define FLAG_MASK_PSW (FLAG_MASK_PER | FLAG_MASK_PSTATE \ +#define FLAG_MASK_PSW (FLAG_MASK_PER | FLAG_MASK_DAT | FLAG_MASK_PSTATE \ | FLAG_MASK_ASC | FLAG_MASK_64 | FLAG_MASK_32) /* Control register 0 bits */ @@ -340,6 +341,10 @@ extern const struct VMStateDescription vmstate_s390_cpu; static inline int cpu_mmu_index(CPUS390XState *env, bool ifetch) { + if (!(env->psw.mask & PSW_MASK_DAT)) { + return MMU_REAL_IDX; + } + switch (env->psw.mask & PSW_MASK_ASC) { case PSW_ASC_PRIMARY: return MMU_PRIMARY_IDX; diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c index f477cc006a..23fb2e7501 100644 --- a/target/s390x/mmu_helper.c +++ b/target/s390x/mmu_helper.c @@ -544,7 +544,7 @@ int mmu_translate_real(CPUS390XState *env, target_ulong raddr, int rw, { const bool lowprot_enabled = env->cregs[0] & CR0_LOWPROT; - *flags = PAGE_READ | PAGE_WRITE; + *flags = PAGE_READ | PAGE_WRITE | PAGE_EXEC; if (is_low_address(raddr & TARGET_PAGE_MASK) && lowprot_enabled) { /* see comment in mmu_translate() how this works */ *flags |= PAGE_WRITE_INV; diff --git a/target/s390x/translate.c b/target/s390x/translate.c index b470d691d3..5aea3bbca6 100644 --- a/target/s390x/translate.c +++ b/target/s390x/translate.c @@ -252,13 +252,17 @@ static inline uint64_t ld_code4(CPUS390XState *env, uint64_t pc) static int get_mem_index(DisasContext *s) { + if (!(s->tb->flags & FLAG_MASK_DAT)) { + return MMU_REAL_IDX; + } + switch (s->tb->flags & FLAG_MASK_ASC) { case PSW_ASC_PRIMARY >> FLAG_MASK_PSW_SHIFT: - return 0; + return MMU_PRIMARY_IDX; case PSW_ASC_SECONDARY >> FLAG_MASK_PSW_SHIFT: - return 1; + return MMU_SECONDARY_IDX; case PSW_ASC_HOME >> FLAG_MASK_PSW_SHIFT: - return 2; + return MMU_HOME_IDX; default: tcg_abort(); break;
Currently, all memory accesses go via the MMU of the address space (primary, secondary, ...). This is bad, because we don't flush the TLB when disabling/enabling DAT. So we could add a tlb flush. However it is easier to simply select the MMU we already have in place for real memory access. All we have to do is point at the right MMU and allow to execute these pages. Signed-off-by: David Hildenbrand <david@redhat.com> --- This is necessary to make the upcomming kvm-unit-tests with vmalloc support pass under TCG. target/s390x/cpu.h | 7 ++++++- target/s390x/mmu_helper.c | 2 +- target/s390x/translate.c | 10 +++++++--- 3 files changed, 14 insertions(+), 5 deletions(-)