On 11/29/19 1:40 PM, Thomas Huth wrote: > On 29/11/2019 10.47, Janosch Frank wrote: > [...] >> Subcodes 8-10 are not valid in protected mode, we have to do a subcode >> 3 and then the 8 and 10 combination for a protected reboot. > > So if 8-10 are not valid in protected mode... > >> @@ -59,6 +61,9 @@ int handle_diag_288(CPUS390XState *env, uint64_t r1, >> uint64_t r3) >> #define DIAG308_LOAD_NORMAL_DUMP 4 >> #define DIAG308_SET 5 >> #define DIAG308_STORE 6 >> +#define DIAG308_PV_SET 8 >> +#define DIAG308_PV_STORE 9 >> +#define DIAG308_PV_START 10 >> >> static int diag308_parm_check(CPUS390XState *env, uint64_t r1, uint64_t >> addr, >> uintptr_t ra, bool write) >> @@ -105,6 +110,7 @@ void handle_diag_308(CPUS390XState *env, uint64_t r1, >> uint64_t r3, uintptr_t ra) >> s390_ipl_reset_request(cs, S390_RESET_REIPL); >> break; >> case DIAG308_SET: >> + case DIAG308_PV_SET: > > ... should you maybe add a check here (and the other cases) to make sure > that the guest is currently not running in PV mode? Or is this taken > care of by the Ultravisor already?
The Ultravisor takes care of that.
>
>> if (diag308_parm_check(env, r1, addr, ra, false)) {
>> return;
>> }
>> @@ -117,7 +123,8 @@ void handle_diag_308(CPUS390XState *env, uint64_t r1,
>> uint64_t r3, uintptr_t ra)
>>
>> cpu_physical_memory_read(addr, iplb, be32_to_cpu(iplb->len));
>>
>> - if (!iplb_valid_ccw(iplb) && !iplb_valid_fcp(iplb)) {
>> + if (!iplb_valid_ccw(iplb) && !iplb_valid_fcp(iplb) &&
>> + !(iplb_valid_se(iplb) && s390_ipl_pv_check_comp(iplb) >= 0)) {
>> env->regs[r1 + 1] = DIAG_308_RC_INVALID;
>> goto out;
>> }
>> @@ -128,10 +135,15 @@ out:
>> g_free(iplb);
>> return;
>> case DIAG308_STORE:
>> + case DIAG308_PV_STORE:
>> if (diag308_parm_check(env, r1, addr, ra, true)) {
>> return;
>> }
>> - iplb = s390_ipl_get_iplb();
>> + if (subcode == DIAG308_PV_STORE) {
>> + iplb = s390_ipl_get_iplb_secure();
>> + } else {
>> + iplb = s390_ipl_get_iplb();
>> + }
>> if (iplb) {
>> cpu_physical_memory_write(addr, iplb, be32_to_cpu(iplb->len));
>> env->regs[r1 + 1] = DIAG_308_RC_OK;
>> @@ -139,6 +151,16 @@ out:
>> env->regs[r1 + 1] = DIAG_308_RC_NO_CONF;
>> }
>> return;
>> + break;
>
> Please remove the break. Or the return. But let's not do both.
Right, I forgot to remove that...
>
>> + case DIAG308_PV_START:
>> + iplb = s390_ipl_get_iplb_secure();
>> + if (!iplb_valid_se(iplb)) {
>> + env->regs[r1 + 1] = DIAG_308_RC_NO_PV_CONF;
>> + return;
>> + }
>> +
>> + s390_ipl_reset_request(cs, S390_RESET_PV);
>> + break;
>> default:
>> s390_program_interrupt(env, PGM_SPECIFICATION, ra);
>> break;
>>
>
> Thomas
>
signature.asc
Description: OpenPGP digital signature
