On Mon, 12 Aug 2019 09:52:56 +0200 David Hildenbrand <da...@redhat.com> wrote:
> On 12.08.19 09:12, Thomas Huth wrote: > > On 8/5/19 5:29 PM, David Hildenbrand wrote: > >> Let's select the ASC before calling the function and use MMU_DATA_LOAD. > >> This is a preparation to: > >> - Remove the ASC magic depending on the access mode from mmu_translate > >> - Implement IEP support, where we could run into access exceptions > >> trying to fetch instructions > >> > >> Signed-off-by: David Hildenbrand <da...@redhat.com> > >> --- > >> target/s390x/helper.c | 10 +++++++++- > >> 1 file changed, 9 insertions(+), 1 deletion(-) > >> > >> diff --git a/target/s390x/helper.c b/target/s390x/helper.c > >> index 13ae9909ad..08166558a0 100644 > >> --- a/target/s390x/helper.c > >> +++ b/target/s390x/helper.c > >> @@ -58,7 +58,15 @@ hwaddr s390_cpu_get_phys_page_debug(CPUState *cs, vaddr > >> vaddr) > >> vaddr &= 0x7fffffff; > >> } > >> > >> - if (mmu_translate(env, vaddr, MMU_INST_FETCH, asc, &raddr, &prot, > >> false)) { > >> + /* > >> + * We want to read the code, however, not run into access exceptions > > > > Is this really a safe assumption here that we always use this to > > translate code addresses and not data addresses? ... I don't think so. > > For example with the "gva2gpa" HMP command, I'd rather expect that it > > also works with the secondary space mode...? > > Well, it's what current code does. I am not changing that behavior. Agreed, that is not actively breaking something. > > While it is in general broken to have a single interface to debug > code+data (which is only a problem on s390x), it makes a lot of sense if > you think about single-stepping through disassembled code using the > gdbstub. Or dumping code where you crashed. What about the memsave interface? > > In Linux, code+data will luckily usually have the same virtual->physical > tables, so it's not a real issue. > > > > > So maybe we need a proper MemTxAttrs bit or something similar for > > distinguishing instruction accesses from data accesses here? > > There would first have to be a way to ask "get_phys_page_debug" to get > code or data for this to make sense. Right now we used it to get code. I'm wondering if we're able to do better; but if the code/data distinction is not considered in architecture independent code, probably not easily.