On 3/12/20 4:41 PM, Christian Borntraeger wrote: > On 11.03.20 14:21, 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. > > Maybe also mention that the data structures for the I/O instrutions > start at offset 0 of the SIDA?
You mean in the comment for get_address_from_regs() or in the commit
message? It would certainly make sense for get_address_from_regs().
>>
>> Signed-off-by: Janosch Frank <fran...@linux.ibm.com>
>> Reviewed-by: Thomas Huth <th...@redhat.com>
>> Reviewed-by: David Hildenbrand <da...@redhat.com>
>
>
> Reviewed-by: Christian Borntraeger <borntrae...@de.ibm.com>
>
>
>
>> ---
>> target/s390x/ioinst.c | 33 ++++++++++++++++++++++++++-------
>> 1 file changed, 26 insertions(+), 7 deletions(-)
>>
>> diff --git a/target/s390x/ioinst.c b/target/s390x/ioinst.c
>> index c437a1d8c6afed80..481d789de9e09a04 100644
>> --- a/target/s390x/ioinst.c
>> +++ b/target/s390x/ioinst.c
>> @@ -16,6 +16,23 @@
>> #include "hw/s390x/ioinst.h"
>> #include "trace.h"
>> #include "hw/s390x/s390-pci-bus.h"
>> +#include "hw/s390x/pv.h"
>> +
>> +static uint64_t get_address_from_regs(CPUS390XState *env, uint32_t ipb,
>> + uint8_t *ar)
>> +{
>> + /*
>> + * Addresses for protected guests are all offsets into the
>> + * satellite block which holds the IO control structures. Those
>> + * control structures are always aligned and accessible, so we can
>> + * return 0 here which will pass the following address checks.
>> + */
>> + if (s390_is_pv()) {
>> + *ar = 0;
>> + return 0;
>> + }
>> + return decode_basedisp_s(env, ipb, ar);
>> +}
>>
>> int ioinst_disassemble_sch_ident(uint32_t value, int *m, int *cssid, int
>> *ssid,
>> int *schid)
>> @@ -114,7 +131,7 @@ void ioinst_handle_msch(S390CPU *cpu, uint64_t reg1,
>> uint32_t ipb, uintptr_t ra)
>> CPUS390XState *env = &cpu->env;
>> uint8_t ar;
>>
>> - addr = decode_basedisp_s(env, ipb, &ar);
>> + addr = get_address_from_regs(env, ipb, &ar);
>> if (addr & 3) {
>> s390_program_interrupt(env, PGM_SPECIFICATION, ra);
>> return;
>> @@ -171,7 +188,7 @@ void ioinst_handle_ssch(S390CPU *cpu, uint64_t reg1,
>> uint32_t ipb, uintptr_t ra)
>> CPUS390XState *env = &cpu->env;
>> uint8_t ar;
>>
>> - addr = decode_basedisp_s(env, ipb, &ar);
>> + addr = get_address_from_regs(env, ipb, &ar);
>> if (addr & 3) {
>> s390_program_interrupt(env, PGM_SPECIFICATION, ra);
>> return;
>> @@ -203,7 +220,7 @@ void ioinst_handle_stcrw(S390CPU *cpu, uint32_t ipb,
>> uintptr_t ra)
>> CPUS390XState *env = &cpu->env;
>> uint8_t ar;
>>
>> - addr = decode_basedisp_s(env, ipb, &ar);
>> + addr = get_address_from_regs(env, ipb, &ar);
>> if (addr & 3) {
>> s390_program_interrupt(env, PGM_SPECIFICATION, ra);
>> return;
>> @@ -234,7 +251,7 @@ void ioinst_handle_stsch(S390CPU *cpu, uint64_t reg1,
>> uint32_t ipb,
>> CPUS390XState *env = &cpu->env;
>> uint8_t ar;
>>
>> - addr = decode_basedisp_s(env, ipb, &ar);
>> + addr = get_address_from_regs(env, ipb, &ar);
>> if (addr & 3) {
>> s390_program_interrupt(env, PGM_SPECIFICATION, ra);
>> return;
>> @@ -303,7 +320,7 @@ int ioinst_handle_tsch(S390CPU *cpu, uint64_t reg1,
>> uint32_t ipb, uintptr_t ra)
>> return -EIO;
>> }
>> trace_ioinst_sch_id("tsch", cssid, ssid, schid);
>> - addr = decode_basedisp_s(env, ipb, &ar);
>> + addr = get_address_from_regs(env, ipb, &ar);
>> if (addr & 3) {
>> s390_program_interrupt(env, PGM_SPECIFICATION, ra);
>> return -EIO;
>> @@ -601,7 +618,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 +627,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 (!s390_is_pv()) {
>> + addr = env->regs[reg];
>> + }
>> /* Page boundary? */
>> if (addr & 0xfff) {
>> s390_program_interrupt(env, PGM_SPECIFICATION, ra);
>>
signature.asc
Description: OpenPGP digital signature
