| Message ID | 20191010113356.5017-20-david@redhat.com |
|---|---|
| State | Accepted |
| Commit | 31b59419069eb844348b55bee4694f5685cfd8c0 |
| Headers | show |
| Series | None | expand |
On Thu, 10 Oct 2019 at 12:35, David Hildenbrand <david@redhat.com> wrote: > > From: Richard Henderson <richard.henderson@linaro.org> > > Do not raise the exception directly within mmu_translate_real, > but pass it back so that caller may do so. > > Reviewed-by: David Hildenbrand <david@redhat.com> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > Message-Id: <20191001171614.8405-8-richard.henderson@linaro.org> > Signed-off-by: David Hildenbrand <david@redhat.com> Hi; Coverity complains about dead code in this patch: > --- a/target/s390x/mmu_helper.c > +++ b/target/s390x/mmu_helper.c > @@ -554,14 +554,11 @@ void s390_cpu_virt_mem_handle_exc(S390CPU *cpu, uintptr_t ra) > * @param rw 0 = read, 1 = write, 2 = code fetch > * @param addr the translated address is stored to this pointer > * @param flags the PAGE_READ/WRITE/EXEC flags are stored to this pointer > - * @return 0 if the translation was successful, < 0 if a fault occurred > + * @return 0 = success, != 0, the exception to raise > */ > int mmu_translate_real(CPUS390XState *env, target_ulong raddr, int rw, > - target_ulong *addr, int *flags) > + target_ulong *addr, int *flags, uint64_t *tec) > { > - /* Code accesses have an undefined ilc, let's use 2 bytes. */ > - uint64_t tec = (raddr & TARGET_PAGE_MASK) | > - (rw == MMU_DATA_STORE ? FS_WRITE : FS_READ); > const bool lowprot_enabled = env->cregs[0] & CR0_LOWPROT; > > *flags = PAGE_READ | PAGE_WRITE | PAGE_EXEC; > @@ -570,9 +567,10 @@ int mmu_translate_real(CPUS390XState *env, target_ulong raddr, int rw, > *flags |= PAGE_WRITE_INV; > if (is_low_address(raddr) && rw == MMU_DATA_STORE) { > /* LAP sets bit 56 */ > - tec |= 0x80; > - trigger_access_exception(env, PGM_PROTECTION, ILEN_AUTO, tec); > - return -EACCES; > + *tec = (raddr & TARGET_PAGE_MASK) > + | (rw == MMU_DATA_STORE ? FS_WRITE : FS_READ) We're inside a condition which includes 'rw == MMU_DATA_STORE', so checking it again here is unnecessary, and the 'false' part of this ?: conditional is dead-code. > + | 0x80; > + return PGM_PROTECTION; > } > } thanks -- PMM
On Thu, 17 Oct 2019 at 12:57, Peter Maydell <peter.maydell@linaro.org> wrote: > > On Thu, 10 Oct 2019 at 12:35, David Hildenbrand <david@redhat.com> wrote: > > > > From: Richard Henderson <richard.henderson@linaro.org> > > > > Do not raise the exception directly within mmu_translate_real, > > but pass it back so that caller may do so. > > > > Reviewed-by: David Hildenbrand <david@redhat.com> > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > > Message-Id: <20191001171614.8405-8-richard.henderson@linaro.org> > > Signed-off-by: David Hildenbrand <david@redhat.com> > > Hi; Coverity complains about dead code in this patch Forgot to mention the issue number, which is CID 1406404. thanks -- PMM
On 17.10.19 14:05, Peter Maydell wrote: > On Thu, 17 Oct 2019 at 12:57, Peter Maydell <peter.maydell@linaro.org> wrote: >> >> On Thu, 10 Oct 2019 at 12:35, David Hildenbrand <david@redhat.com> wrote: >>> >>> From: Richard Henderson <richard.henderson@linaro.org> >>> >>> Do not raise the exception directly within mmu_translate_real, >>> but pass it back so that caller may do so. >>> >>> Reviewed-by: David Hildenbrand <david@redhat.com> >>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >>> Message-Id: <20191001171614.8405-8-richard.henderson@linaro.org> >>> Signed-off-by: David Hildenbrand <david@redhat.com> >> >> Hi; Coverity complains about dead code in this patch > > Forgot to mention the issue number, which is CID 1406404. > > thanks > -- PMM > Will have a look thanks! -- Thanks, David / dhildenb
diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c index ab2ed47fef..906b87c071 100644 --- a/target/s390x/excp_helper.c +++ b/target/s390x/excp_helper.c @@ -147,8 +147,8 @@ bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int size, if (!(env->psw.mask & PSW_MASK_64)) { vaddr &= 0x7fffffff; } - fail = mmu_translate_real(env, vaddr, access_type, &raddr, &prot); - excp = 0; /* exception already raised */ + excp = mmu_translate_real(env, vaddr, access_type, &raddr, &prot, &tec); + fail = excp; } else { g_assert_not_reached(); } diff --git a/target/s390x/internal.h b/target/s390x/internal.h index c243fa725b..c4388aaf23 100644 --- a/target/s390x/internal.h +++ b/target/s390x/internal.h @@ -362,7 +362,7 @@ void probe_write_access(CPUS390XState *env, uint64_t addr, uint64_t len, int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc, target_ulong *raddr, int *flags, bool exc); int mmu_translate_real(CPUS390XState *env, target_ulong raddr, int rw, - target_ulong *addr, int *flags); + target_ulong *addr, int *flags, uint64_t *tec); /* misc_helper.c */ diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c index 4a794dadcf..e8281d4413 100644 --- a/target/s390x/mmu_helper.c +++ b/target/s390x/mmu_helper.c @@ -554,14 +554,11 @@ void s390_cpu_virt_mem_handle_exc(S390CPU *cpu, uintptr_t ra) * @param rw 0 = read, 1 = write, 2 = code fetch * @param addr the translated address is stored to this pointer * @param flags the PAGE_READ/WRITE/EXEC flags are stored to this pointer - * @return 0 if the translation was successful, < 0 if a fault occurred + * @return 0 = success, != 0, the exception to raise */ int mmu_translate_real(CPUS390XState *env, target_ulong raddr, int rw, - target_ulong *addr, int *flags) + target_ulong *addr, int *flags, uint64_t *tec) { - /* Code accesses have an undefined ilc, let's use 2 bytes. */ - uint64_t tec = (raddr & TARGET_PAGE_MASK) | - (rw == MMU_DATA_STORE ? FS_WRITE : FS_READ); const bool lowprot_enabled = env->cregs[0] & CR0_LOWPROT; *flags = PAGE_READ | PAGE_WRITE | PAGE_EXEC; @@ -570,9 +567,10 @@ int mmu_translate_real(CPUS390XState *env, target_ulong raddr, int rw, *flags |= PAGE_WRITE_INV; if (is_low_address(raddr) && rw == MMU_DATA_STORE) { /* LAP sets bit 56 */ - tec |= 0x80; - trigger_access_exception(env, PGM_PROTECTION, ILEN_AUTO, tec); - return -EACCES; + *tec = (raddr & TARGET_PAGE_MASK) + | (rw == MMU_DATA_STORE ? FS_WRITE : FS_READ) + | 0x80; + return PGM_PROTECTION; } }