>>> + case S390_RESET_PV: /* Subcode 10 */ >>> + subsystem_reset(); >>> + s390_crypto_reset(); >>> + >>> + CPU_FOREACH(t) { >>> + if (t == cs) { >>> + continue; >>> + } >>> + run_on_cpu(t, s390_do_cpu_full_reset, RUN_ON_CPU_NULL); >>> + } >>> + run_on_cpu(cs, s390_do_cpu_reset, RUN_ON_CPU_NULL); >>> + >>> + if (s390_machine_protect(ms)) { >>> + s390_machine_inject_pv_error(cs); >>> + /* >>> + * Continue after the diag308 so the guest knows something >>> + * went wrong. >>> + */ >>> + s390_cpu_set_state(S390_CPU_STATE_OPERATING, cpu); >>> + return; >> >> Didn't you want to squash in that hunk from the other patch? (I remember >> seeing a goto) > > I chose to leave it this way so we don't jump around with the goto
Fine with me as long as the other patch won't touch this code anymore ;) >>> #if !defined(CONFIG_USER_ONLY) >>> uint32_t core_id; /* PoP "CPU address", same as cpu_index */ >>> diff --git a/target/s390x/cpu_features_def.inc.h >>> b/target/s390x/cpu_features_def.inc.h >>> index 31dff0d84e..60db28351d 100644 >>> --- a/target/s390x/cpu_features_def.inc.h >>> +++ b/target/s390x/cpu_features_def.inc.h >>> @@ -107,6 +107,7 @@ DEF_FEAT(DEFLATE_BASE, "deflate-base", STFL, 151, >>> "Deflate-conversion facility ( >>> DEF_FEAT(VECTOR_PACKED_DECIMAL_ENH, "vxpdeh", STFL, 152, >>> "Vector-Packed-Decimal-Enhancement Facility") >>> DEF_FEAT(MSA_EXT_9, "msa9-base", STFL, 155, >>> "Message-security-assist-extension-9 facility (excluding subfunctions)") >>> DEF_FEAT(ETOKEN, "etoken", STFL, 156, "Etoken facility") >>> +DEF_FEAT(UNPACK, "unpack", STFL, 161, "Unpack facility") >> >> Random thought: The naming of that facility could have been improved to >> something that gives users/readers an idea what it's actually doing. > > That's the name from our specifications and with those feature bits we > normally use the facility names from those docs, no? > Something like "protected guest boot facility" would certainly have been > better to understand. Yeah, I was not talking about bad naming from the HW/FW folks for that facility. Not your fault. >> [...] >> >>> @@ -128,17 +142,31 @@ 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_pv(); >>> + } 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; >>> } else { >>> env->regs[r1 + 1] = DIAG_308_RC_NO_CONF; >>> } >>> - return; >>> + break; >> >> return->break is unrelated, but we do have a wild mixture already. > > I removed it here and in the move diag structures over SIDA patch. > After this has been merged I can do a cleanup patch if wanted. Whatever you prefer. -- Thanks, David / dhildenb