| Message ID | 20191120114334.2287-15-frankja@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | s390x: Protected Virtualization support | expand |
On 20/11/2019 12.43, Janosch Frank wrote: > IO instruction data is routed through SIDAD for protected guests, so > adresses do not need to be checked, as this is kernel memory. > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > --- > target/s390x/ioinst.c | 46 +++++++++++++++++++++++++++---------------- > 1 file changed, 29 insertions(+), 17 deletions(-) > > diff --git a/target/s390x/ioinst.c b/target/s390x/ioinst.c > index c437a1d8c6..d3bd422ddd 100644 > --- a/target/s390x/ioinst.c > +++ b/target/s390x/ioinst.c > @@ -110,11 +110,13 @@ void ioinst_handle_msch(S390CPU *cpu, uint64_t reg1, uint32_t ipb, uintptr_t ra) > int cssid, ssid, schid, m; > SubchDev *sch; > SCHIB schib; > - uint64_t addr; > + uint64_t addr = 0; > CPUS390XState *env = &cpu->env; > - uint8_t ar; > + uint8_t ar = 0; > > - addr = decode_basedisp_s(env, ipb, &ar); > + if (!env->pv) { > + addr = decode_basedisp_s(env, ipb, &ar); > + } > if (addr & 3) { > s390_program_interrupt(env, PGM_SPECIFICATION, ra); > return; > @@ -167,11 +169,13 @@ void ioinst_handle_ssch(S390CPU *cpu, uint64_t reg1, uint32_t ipb, uintptr_t ra) > int cssid, ssid, schid, m; > SubchDev *sch; > ORB orig_orb, orb; > - uint64_t addr; > + uint64_t addr = 0; > CPUS390XState *env = &cpu->env; > - uint8_t ar; > + uint8_t ar = 0; > > - addr = decode_basedisp_s(env, ipb, &ar); > + if (!env->pv) { > + addr = decode_basedisp_s(env, ipb, &ar); > + } > if (addr & 3) { > s390_program_interrupt(env, PGM_SPECIFICATION, ra); > return; > @@ -198,12 +202,14 @@ void ioinst_handle_ssch(S390CPU *cpu, uint64_t reg1, uint32_t ipb, uintptr_t ra) > void ioinst_handle_stcrw(S390CPU *cpu, uint32_t ipb, uintptr_t ra) > { > CRW crw; > - uint64_t addr; > + uint64_t addr = 0; > int cc; > CPUS390XState *env = &cpu->env; > - uint8_t ar; > + uint8_t ar = 0; > > - addr = decode_basedisp_s(env, ipb, &ar); > + if (!env->pv) { > + addr = decode_basedisp_s(env, ipb, &ar); > + } > if (addr & 3) { > s390_program_interrupt(env, PGM_SPECIFICATION, ra); > return; > @@ -228,13 +234,15 @@ void ioinst_handle_stsch(S390CPU *cpu, uint64_t reg1, uint32_t ipb, > { > int cssid, ssid, schid, m; > SubchDev *sch; > - uint64_t addr; > + uint64_t addr = 0; > int cc; > SCHIB schib; > CPUS390XState *env = &cpu->env; > - uint8_t ar; > + uint8_t ar = 0; > > - addr = decode_basedisp_s(env, ipb, &ar); > + if (!env->pv) { > + addr = decode_basedisp_s(env, ipb, &ar); > + } > if (addr & 3) { > s390_program_interrupt(env, PGM_SPECIFICATION, ra); > return; > @@ -294,16 +302,18 @@ int ioinst_handle_tsch(S390CPU *cpu, uint64_t reg1, uint32_t ipb, uintptr_t ra) > int cssid, ssid, schid, m; > SubchDev *sch; > IRB irb; > - uint64_t addr; > + uint64_t addr = 0; > int cc, irb_len; > - uint8_t ar; > + uint8_t ar = 0; > > if (ioinst_disassemble_sch_ident(reg1, &m, &cssid, &ssid, &schid)) { > s390_program_interrupt(env, PGM_OPERAND, ra); > return -EIO; > } > trace_ioinst_sch_id("tsch", cssid, ssid, schid); > - addr = decode_basedisp_s(env, ipb, &ar); > + if (!env->pv) { > + addr = decode_basedisp_s(env, ipb, &ar); > + } > if (addr & 3) { > s390_program_interrupt(env, PGM_SPECIFICATION, ra); > return -EIO; Would it make sense to hide all these changes in decode_basedisp_s() instead? ... so that decode_basedisp_s() returns 0 if env->pv == true ? ... or are there still cases where we need real values from decode_basedisp_s() in case of env->pv==true? Anyway, Reviewed-by: Thomas Huth <thuth@redhat.com>
On 11/28/19 4:28 PM, Thomas Huth wrote: > On 20/11/2019 12.43, Janosch Frank wrote: >> IO instruction data is routed through SIDAD for protected guests, so >> adresses do not need to be checked, as this is kernel memory. >> >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >> --- >> target/s390x/ioinst.c | 46 +++++++++++++++++++++++++++---------------- >> 1 file changed, 29 insertions(+), 17 deletions(-) >> >> diff --git a/target/s390x/ioinst.c b/target/s390x/ioinst.c >> index c437a1d8c6..d3bd422ddd 100644 >> --- a/target/s390x/ioinst.c >> +++ b/target/s390x/ioinst.c >> @@ -110,11 +110,13 @@ void ioinst_handle_msch(S390CPU *cpu, uint64_t reg1, uint32_t ipb, uintptr_t ra) >> int cssid, ssid, schid, m; >> SubchDev *sch; >> SCHIB schib; >> - uint64_t addr; >> + uint64_t addr = 0; >> CPUS390XState *env = &cpu->env; >> - uint8_t ar; >> + uint8_t ar = 0; >> >> - addr = decode_basedisp_s(env, ipb, &ar); >> + if (!env->pv) { >> + addr = decode_basedisp_s(env, ipb, &ar); >> + } >> if (addr & 3) { >> s390_program_interrupt(env, PGM_SPECIFICATION, ra); >> return; >> @@ -167,11 +169,13 @@ void ioinst_handle_ssch(S390CPU *cpu, uint64_t reg1, uint32_t ipb, uintptr_t ra) >> int cssid, ssid, schid, m; >> SubchDev *sch; >> ORB orig_orb, orb; >> - uint64_t addr; >> + uint64_t addr = 0; >> CPUS390XState *env = &cpu->env; >> - uint8_t ar; >> + uint8_t ar = 0; >> >> - addr = decode_basedisp_s(env, ipb, &ar); >> + if (!env->pv) { >> + addr = decode_basedisp_s(env, ipb, &ar); >> + } >> if (addr & 3) { >> s390_program_interrupt(env, PGM_SPECIFICATION, ra); >> return; >> @@ -198,12 +202,14 @@ void ioinst_handle_ssch(S390CPU *cpu, uint64_t reg1, uint32_t ipb, uintptr_t ra) >> void ioinst_handle_stcrw(S390CPU *cpu, uint32_t ipb, uintptr_t ra) >> { >> CRW crw; >> - uint64_t addr; >> + uint64_t addr = 0; >> int cc; >> CPUS390XState *env = &cpu->env; >> - uint8_t ar; >> + uint8_t ar = 0; >> >> - addr = decode_basedisp_s(env, ipb, &ar); >> + if (!env->pv) { >> + addr = decode_basedisp_s(env, ipb, &ar); >> + } >> if (addr & 3) { >> s390_program_interrupt(env, PGM_SPECIFICATION, ra); >> return; >> @@ -228,13 +234,15 @@ void ioinst_handle_stsch(S390CPU *cpu, uint64_t reg1, uint32_t ipb, >> { >> int cssid, ssid, schid, m; >> SubchDev *sch; >> - uint64_t addr; >> + uint64_t addr = 0; >> int cc; >> SCHIB schib; >> CPUS390XState *env = &cpu->env; >> - uint8_t ar; >> + uint8_t ar = 0; >> >> - addr = decode_basedisp_s(env, ipb, &ar); >> + if (!env->pv) { >> + addr = decode_basedisp_s(env, ipb, &ar); >> + } >> if (addr & 3) { >> s390_program_interrupt(env, PGM_SPECIFICATION, ra); >> return; >> @@ -294,16 +302,18 @@ int ioinst_handle_tsch(S390CPU *cpu, uint64_t reg1, uint32_t ipb, uintptr_t ra) >> int cssid, ssid, schid, m; >> SubchDev *sch; >> IRB irb; >> - uint64_t addr; >> + uint64_t addr = 0; >> int cc, irb_len; >> - uint8_t ar; >> + uint8_t ar = 0; >> >> if (ioinst_disassemble_sch_ident(reg1, &m, &cssid, &ssid, &schid)) { >> s390_program_interrupt(env, PGM_OPERAND, ra); >> return -EIO; >> } >> trace_ioinst_sch_id("tsch", cssid, ssid, schid); >> - addr = decode_basedisp_s(env, ipb, &ar); >> + if (!env->pv) { >> + addr = decode_basedisp_s(env, ipb, &ar); >> + } >> if (addr & 3) { >> s390_program_interrupt(env, PGM_SPECIFICATION, ra); >> return -EIO; > > Would it make sense to hide all these changes in decode_basedisp_s() > instead? ... so that decode_basedisp_s() returns 0 if env->pv == true ? > ... or are there still cases where we need real values from > decode_basedisp_s() in case of env->pv==true? Pierre already suggested that, I'll look into it. Hopefully we have no addr != 0 or addr > 2 * PAGE_SIZE checks. Because of that it might make more sense to just rip out the checks. > > Anyway, > Reviewed-by: Thomas Huth <thuth@redhat.com> >
On 11/28/19 4:28 PM, Thomas Huth wrote: > On 20/11/2019 12.43, Janosch Frank wrote: >> IO instruction data is routed through SIDAD for protected guests, so >> adresses do not need to be checked, as this is kernel memory. >> >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >> --- >> target/s390x/ioinst.c | 46 +++++++++++++++++++++++++++---------------- >> 1 file changed, 29 insertions(+), 17 deletions(-) >> >> diff --git a/target/s390x/ioinst.c b/target/s390x/ioinst.c >> index c437a1d8c6..d3bd422ddd 100644 >> --- a/target/s390x/ioinst.c >> +++ b/target/s390x/ioinst.c >> @@ -110,11 +110,13 @@ void ioinst_handle_msch(S390CPU *cpu, uint64_t reg1, uint32_t ipb, uintptr_t ra) >> int cssid, ssid, schid, m; >> SubchDev *sch; >> SCHIB schib; >> - uint64_t addr; >> + uint64_t addr = 0; >> CPUS390XState *env = &cpu->env; >> - uint8_t ar; >> + uint8_t ar = 0; >> >> - addr = decode_basedisp_s(env, ipb, &ar); >> + if (!env->pv) { >> + addr = decode_basedisp_s(env, ipb, &ar); >> + } >> if (addr & 3) { >> s390_program_interrupt(env, PGM_SPECIFICATION, ra); >> return; >> @@ -167,11 +169,13 @@ void ioinst_handle_ssch(S390CPU *cpu, uint64_t reg1, uint32_t ipb, uintptr_t ra) >> int cssid, ssid, schid, m; >> SubchDev *sch; >> ORB orig_orb, orb; >> - uint64_t addr; >> + uint64_t addr = 0; >> CPUS390XState *env = &cpu->env; >> - uint8_t ar; >> + uint8_t ar = 0; >> >> - addr = decode_basedisp_s(env, ipb, &ar); >> + if (!env->pv) { >> + addr = decode_basedisp_s(env, ipb, &ar); >> + } >> if (addr & 3) { >> s390_program_interrupt(env, PGM_SPECIFICATION, ra); >> return; >> @@ -198,12 +202,14 @@ void ioinst_handle_ssch(S390CPU *cpu, uint64_t reg1, uint32_t ipb, uintptr_t ra) >> void ioinst_handle_stcrw(S390CPU *cpu, uint32_t ipb, uintptr_t ra) >> { >> CRW crw; >> - uint64_t addr; >> + uint64_t addr = 0; >> int cc; >> CPUS390XState *env = &cpu->env; >> - uint8_t ar; >> + uint8_t ar = 0; >> >> - addr = decode_basedisp_s(env, ipb, &ar); >> + if (!env->pv) { >> + addr = decode_basedisp_s(env, ipb, &ar); >> + } >> if (addr & 3) { >> s390_program_interrupt(env, PGM_SPECIFICATION, ra); >> return; >> @@ -228,13 +234,15 @@ void ioinst_handle_stsch(S390CPU *cpu, uint64_t reg1, uint32_t ipb, >> { >> int cssid, ssid, schid, m; >> SubchDev *sch; >> - uint64_t addr; >> + uint64_t addr = 0; >> int cc; >> SCHIB schib; >> CPUS390XState *env = &cpu->env; >> - uint8_t ar; >> + uint8_t ar = 0; >> >> - addr = decode_basedisp_s(env, ipb, &ar); >> + if (!env->pv) { >> + addr = decode_basedisp_s(env, ipb, &ar); >> + } >> if (addr & 3) { >> s390_program_interrupt(env, PGM_SPECIFICATION, ra); >> return; >> @@ -294,16 +302,18 @@ int ioinst_handle_tsch(S390CPU *cpu, uint64_t reg1, uint32_t ipb, uintptr_t ra) >> int cssid, ssid, schid, m; >> SubchDev *sch; >> IRB irb; >> - uint64_t addr; >> + uint64_t addr = 0; >> int cc, irb_len; >> - uint8_t ar; >> + uint8_t ar = 0; >> >> if (ioinst_disassemble_sch_ident(reg1, &m, &cssid, &ssid, &schid)) { >> s390_program_interrupt(env, PGM_OPERAND, ra); >> return -EIO; >> } >> trace_ioinst_sch_id("tsch", cssid, ssid, schid); >> - addr = decode_basedisp_s(env, ipb, &ar); >> + if (!env->pv) { >> + addr = decode_basedisp_s(env, ipb, &ar); >> + } >> if (addr & 3) { >> s390_program_interrupt(env, PGM_SPECIFICATION, ra); >> return -EIO; > > Would it make sense to hide all these changes in decode_basedisp_s() > instead? ... so that decode_basedisp_s() returns 0 if env->pv == true ? > ... or are there still cases where we need real values from > decode_basedisp_s() in case of env->pv==true? I'd like to keep decode_basedisp_s() as is, but how about a static function in ioinst.c called something like get_address_from_regs()? It'll call decode_basedisp_s() or return 0. > > Anyway, > Reviewed-by: Thomas Huth <thuth@redhat.com> > Thanks!
On Thu, 28 Nov 2019 17:10:38 +0100 Janosch Frank <frankja@linux.ibm.com> wrote: > On 11/28/19 4:28 PM, Thomas Huth wrote: > > Would it make sense to hide all these changes in decode_basedisp_s() > > instead? ... so that decode_basedisp_s() returns 0 if env->pv == true ? > > ... or are there still cases where we need real values from > > decode_basedisp_s() in case of env->pv==true? > > I'd like to keep decode_basedisp_s() as is, but how about a static > function in ioinst.c called something like get_address_from_regs()? > > It'll call decode_basedisp_s() or return 0. We could do something like that; but do we ever get there for other instruction formats as well? It feels a bit odd to single out this one.
On 11/28/19 5:18 PM, Cornelia Huck wrote: > On Thu, 28 Nov 2019 17:10:38 +0100 > Janosch Frank <frankja@linux.ibm.com> wrote: > >> On 11/28/19 4:28 PM, Thomas Huth wrote: > >>> Would it make sense to hide all these changes in decode_basedisp_s() >>> instead? ... so that decode_basedisp_s() returns 0 if env->pv == true ? >>> ... or are there still cases where we need real values from >>> decode_basedisp_s() in case of env->pv==true? >> >> I'd like to keep decode_basedisp_s() as is, but how about a static >> function in ioinst.c called something like get_address_from_regs()? >> >> It'll call decode_basedisp_s() or return 0. > > We could do something like that; but do we ever get there for other > instruction formats as well? It feels a bit odd to single out this one. > sclp is rre (register address) and diag308 is rs-a (r1 is the address).
On 28/11/2019 17.10, Janosch Frank wrote: > On 11/28/19 4:28 PM, Thomas Huth wrote: >> On 20/11/2019 12.43, Janosch Frank wrote: >>> IO instruction data is routed through SIDAD for protected guests, so >>> adresses do not need to be checked, as this is kernel memory. >>> >>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >>> --- >>> target/s390x/ioinst.c | 46 +++++++++++++++++++++++++++---------------- >>> 1 file changed, 29 insertions(+), 17 deletions(-) >>> >>> diff --git a/target/s390x/ioinst.c b/target/s390x/ioinst.c >>> index c437a1d8c6..d3bd422ddd 100644 >>> --- a/target/s390x/ioinst.c >>> +++ b/target/s390x/ioinst.c >>> @@ -110,11 +110,13 @@ void ioinst_handle_msch(S390CPU *cpu, uint64_t reg1, uint32_t ipb, uintptr_t ra) >>> int cssid, ssid, schid, m; >>> SubchDev *sch; >>> SCHIB schib; >>> - uint64_t addr; >>> + uint64_t addr = 0; >>> CPUS390XState *env = &cpu->env; >>> - uint8_t ar; >>> + uint8_t ar = 0; >>> >>> - addr = decode_basedisp_s(env, ipb, &ar); >>> + if (!env->pv) { >>> + addr = decode_basedisp_s(env, ipb, &ar); >>> + } >>> if (addr & 3) { >>> s390_program_interrupt(env, PGM_SPECIFICATION, ra); >>> return; >>> @@ -167,11 +169,13 @@ void ioinst_handle_ssch(S390CPU *cpu, uint64_t reg1, uint32_t ipb, uintptr_t ra) >>> int cssid, ssid, schid, m; >>> SubchDev *sch; >>> ORB orig_orb, orb; >>> - uint64_t addr; >>> + uint64_t addr = 0; >>> CPUS390XState *env = &cpu->env; >>> - uint8_t ar; >>> + uint8_t ar = 0; >>> >>> - addr = decode_basedisp_s(env, ipb, &ar); >>> + if (!env->pv) { >>> + addr = decode_basedisp_s(env, ipb, &ar); >>> + } >>> if (addr & 3) { >>> s390_program_interrupt(env, PGM_SPECIFICATION, ra); >>> return; >>> @@ -198,12 +202,14 @@ void ioinst_handle_ssch(S390CPU *cpu, uint64_t reg1, uint32_t ipb, uintptr_t ra) >>> void ioinst_handle_stcrw(S390CPU *cpu, uint32_t ipb, uintptr_t ra) >>> { >>> CRW crw; >>> - uint64_t addr; >>> + uint64_t addr = 0; >>> int cc; >>> CPUS390XState *env = &cpu->env; >>> - uint8_t ar; >>> + uint8_t ar = 0; >>> >>> - addr = decode_basedisp_s(env, ipb, &ar); >>> + if (!env->pv) { >>> + addr = decode_basedisp_s(env, ipb, &ar); >>> + } >>> if (addr & 3) { >>> s390_program_interrupt(env, PGM_SPECIFICATION, ra); >>> return; >>> @@ -228,13 +234,15 @@ void ioinst_handle_stsch(S390CPU *cpu, uint64_t reg1, uint32_t ipb, >>> { >>> int cssid, ssid, schid, m; >>> SubchDev *sch; >>> - uint64_t addr; >>> + uint64_t addr = 0; >>> int cc; >>> SCHIB schib; >>> CPUS390XState *env = &cpu->env; >>> - uint8_t ar; >>> + uint8_t ar = 0; >>> >>> - addr = decode_basedisp_s(env, ipb, &ar); >>> + if (!env->pv) { >>> + addr = decode_basedisp_s(env, ipb, &ar); >>> + } >>> if (addr & 3) { >>> s390_program_interrupt(env, PGM_SPECIFICATION, ra); >>> return; >>> @@ -294,16 +302,18 @@ int ioinst_handle_tsch(S390CPU *cpu, uint64_t reg1, uint32_t ipb, uintptr_t ra) >>> int cssid, ssid, schid, m; >>> SubchDev *sch; >>> IRB irb; >>> - uint64_t addr; >>> + uint64_t addr = 0; >>> int cc, irb_len; >>> - uint8_t ar; >>> + uint8_t ar = 0; >>> >>> if (ioinst_disassemble_sch_ident(reg1, &m, &cssid, &ssid, &schid)) { >>> s390_program_interrupt(env, PGM_OPERAND, ra); >>> return -EIO; >>> } >>> trace_ioinst_sch_id("tsch", cssid, ssid, schid); >>> - addr = decode_basedisp_s(env, ipb, &ar); >>> + if (!env->pv) { >>> + addr = decode_basedisp_s(env, ipb, &ar); >>> + } >>> if (addr & 3) { >>> s390_program_interrupt(env, PGM_SPECIFICATION, ra); >>> return -EIO; >> >> Would it make sense to hide all these changes in decode_basedisp_s() >> instead? ... so that decode_basedisp_s() returns 0 if env->pv == true ? >> ... or are there still cases where we need real values from >> decode_basedisp_s() in case of env->pv==true? > > I'd like to keep decode_basedisp_s() as is, but how about a static > function in ioinst.c called something like get_address_from_regs()? > > It'll call decode_basedisp_s() or return 0. Sounds fine for me, too! Thomas
diff --git a/target/s390x/ioinst.c b/target/s390x/ioinst.c index c437a1d8c6..d3bd422ddd 100644 --- a/target/s390x/ioinst.c +++ b/target/s390x/ioinst.c @@ -110,11 +110,13 @@ void ioinst_handle_msch(S390CPU *cpu, uint64_t reg1, uint32_t ipb, uintptr_t ra) int cssid, ssid, schid, m; SubchDev *sch; SCHIB schib; - uint64_t addr; + uint64_t addr = 0; CPUS390XState *env = &cpu->env; - uint8_t ar; + uint8_t ar = 0; - addr = decode_basedisp_s(env, ipb, &ar); + if (!env->pv) { + addr = decode_basedisp_s(env, ipb, &ar); + } if (addr & 3) { s390_program_interrupt(env, PGM_SPECIFICATION, ra); return; @@ -167,11 +169,13 @@ void ioinst_handle_ssch(S390CPU *cpu, uint64_t reg1, uint32_t ipb, uintptr_t ra) int cssid, ssid, schid, m; SubchDev *sch; ORB orig_orb, orb; - uint64_t addr; + uint64_t addr = 0; CPUS390XState *env = &cpu->env; - uint8_t ar; + uint8_t ar = 0; - addr = decode_basedisp_s(env, ipb, &ar); + if (!env->pv) { + addr = decode_basedisp_s(env, ipb, &ar); + } if (addr & 3) { s390_program_interrupt(env, PGM_SPECIFICATION, ra); return; @@ -198,12 +202,14 @@ void ioinst_handle_ssch(S390CPU *cpu, uint64_t reg1, uint32_t ipb, uintptr_t ra) void ioinst_handle_stcrw(S390CPU *cpu, uint32_t ipb, uintptr_t ra) { CRW crw; - uint64_t addr; + uint64_t addr = 0; int cc; CPUS390XState *env = &cpu->env; - uint8_t ar; + uint8_t ar = 0; - addr = decode_basedisp_s(env, ipb, &ar); + if (!env->pv) { + addr = decode_basedisp_s(env, ipb, &ar); + } if (addr & 3) { s390_program_interrupt(env, PGM_SPECIFICATION, ra); return; @@ -228,13 +234,15 @@ void ioinst_handle_stsch(S390CPU *cpu, uint64_t reg1, uint32_t ipb, { int cssid, ssid, schid, m; SubchDev *sch; - uint64_t addr; + uint64_t addr = 0; int cc; SCHIB schib; CPUS390XState *env = &cpu->env; - uint8_t ar; + uint8_t ar = 0; - addr = decode_basedisp_s(env, ipb, &ar); + if (!env->pv) { + addr = decode_basedisp_s(env, ipb, &ar); + } if (addr & 3) { s390_program_interrupt(env, PGM_SPECIFICATION, ra); return; @@ -294,16 +302,18 @@ int ioinst_handle_tsch(S390CPU *cpu, uint64_t reg1, uint32_t ipb, uintptr_t ra) int cssid, ssid, schid, m; SubchDev *sch; IRB irb; - uint64_t addr; + uint64_t addr = 0; int cc, irb_len; - uint8_t ar; + uint8_t ar = 0; if (ioinst_disassemble_sch_ident(reg1, &m, &cssid, &ssid, &schid)) { s390_program_interrupt(env, PGM_OPERAND, ra); return -EIO; } trace_ioinst_sch_id("tsch", cssid, ssid, schid); - addr = decode_basedisp_s(env, ipb, &ar); + if (!env->pv) { + addr = decode_basedisp_s(env, ipb, &ar); + } if (addr & 3) { s390_program_interrupt(env, PGM_SPECIFICATION, ra); return -EIO; @@ -601,7 +611,7 @@ void ioinst_handle_chsc(S390CPU *cpu, uint32_t ipb, uintptr_t ra) { ChscReq *req; ChscResp *res; - uint64_t addr; + uint64_t addr = 0; int reg; uint16_t len; uint16_t command; @@ -610,7 +620,9 @@ void ioinst_handle_chsc(S390CPU *cpu, uint32_t ipb, uintptr_t ra) trace_ioinst("chsc"); reg = (ipb >> 20) & 0x00f; - addr = env->regs[reg]; + if (!env->pv) { + addr = env->regs[reg]; + } /* Page boundary? */ if (addr & 0xfff) { s390_program_interrupt(env, PGM_SPECIFICATION, ra);
IO instruction data is routed through SIDAD for protected guests, so adresses do not need to be checked, as this is kernel memory. Signed-off-by: Janosch Frank <frankja@linux.ibm.com> --- target/s390x/ioinst.c | 46 +++++++++++++++++++++++++++---------------- 1 file changed, 29 insertions(+), 17 deletions(-)