On 21/11/2019 12.21, David Hildenbrand wrote:
> On 20.11.19 12:43, Janosch Frank wrote:
>> Let's improve readability by:
>> * Using constants for the subcodes
>> * Moving parameter checking into a function
>> * Removing subcode > 6 check as the default case catches that
>>
>> Signed-off-by: Janosch Frank <fran...@linux.ibm.com>
>> ---
>> target/s390x/diag.c | 54 +++++++++++++++++++++++++++------------------
>> 1 file changed, 32 insertions(+), 22 deletions(-)
>>
>> diff --git a/target/s390x/diag.c b/target/s390x/diag.c
>> index 53c2f81f2a..067c667ba7 100644
>> --- a/target/s390x/diag.c
>> +++ b/target/s390x/diag.c
>> @@ -53,6 +53,29 @@ int handle_diag_288(CPUS390XState *env, uint64_t
>> r1, uint64_t r3)
>> #define DIAG_308_RC_NO_CONF 0x0102
>> #define DIAG_308_RC_INVALID 0x0402
>> +#define DIAG308_RES_MOD_CLR 0
>> +#define DIAG308_RES_LOAD_NORM 1
>> +#define DIAG308_LOAD_CLEAR 3
>> +#define DIAG308_LOAD_NORMAL_DUMP 4
>> +#define DIAG308_SET 5
>> +#define DIAG308_STORE 6
>> +
>> +static int diag308_parm_check(CPUS390XState *env, uint64_t r1,
>> uint64_t addr,
>> + uintptr_t ra, bool write)
>> +{
>> + if ((r1 & 1) || (addr & ~TARGET_PAGE_MASK)) {
>> + s390_program_interrupt(env, PGM_SPECIFICATION, ra);
>> + return -EINVAL;
>> + }
>> + if (!address_space_access_valid(&address_space_memory, addr,
>> + sizeof(IplParameterBlock), write,
>> + MEMTXATTRS_UNSPECIFIED)) {
>> + s390_program_interrupt(env, PGM_ADDRESSING, ra);
>> + return -EINVAL;
>> + }
>> + return 0;
>> +}
>> +
>> void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3,
>> uintptr_t ra)
>> {
>> CPUState *cs = env_cpu(env);
>> @@ -65,30 +88,24 @@ void handle_diag_308(CPUS390XState *env, uint64_t
>> r1, uint64_t r3, uintptr_t ra)
>> return;
>> }
>> - if ((subcode & ~0x0ffffULL) || (subcode > 6)) {
>> + if (subcode & ~0x0ffffULL) {
>
> Strange, the default case in the switch was basically dead code.
Not really, case 2 and case 4 are not handled there. We even had a funny
bug some months ago, where a guest could terminate QEMU with one of
these codes, see commit 37dbd1f4d4805edcd18d94eb202bb3461b3cd52d.
Thomas
