Re: [RFC PATCH] hw/s390x/sclp: Do not ignore address_space_read/write() errors

["Jason J. Herne" <jjherne@linux.ibm.com>]

On 10/6/25 9:58 PM, Philippe Mathieu-Daudé wrote:
> If address_space_read() fails, return PGM_ADDRESSING. In the
> unlikely case address_space_write() fails (we already checked
> the address is readable), return PGM_PROTECTION.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> Based-on: <20251007014958.19086-1-philmd@linaro.org>
> ---
>   hw/s390x/sclp.c | 22 ++++++++++++++++------
>   1 file changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> index d7cb99482b2..8604cd305e5 100644
> --- a/hw/s390x/sclp.c
> +++ b/hw/s390x/sclp.c
> @@ -305,6 +305,7 @@ int sclp_service_call(S390CPU *cpu, uint64_t sccb, uint32_t code)
>       SCCBHeader header;
>       g_autofree SCCB *work_sccb = NULL;
>       AddressSpace *as = CPU(cpu)->as;
> +    MemTxResult ret;
>   
>       /* first some basic checks on program checks */
>       if (env->psw.mask & PSW_MASK_PSTATE) {
> @@ -319,8 +320,11 @@ int sclp_service_call(S390CPU *cpu, uint64_t sccb, uint32_t code)
>       }
>   
>       /* the header contains the actual length of the sccb */
> -    address_space_read(as, sccb, MEMTXATTRS_UNSPECIFIED,
> -                       &header, sizeof(SCCBHeader));
> +    ret = address_space_read(as, sccb, MEMTXATTRS_UNSPECIFIED,
> +                             &header, sizeof(SCCBHeader));
> +    if (ret != MEMTX_OK) {
> +        return -PGM_ADDRESSING;
> +    }
>   
>       /* Valid sccb sizes */
>       if (be16_to_cpu(header.length) < sizeof(SCCBHeader)) {
> @@ -333,8 +337,11 @@ int sclp_service_call(S390CPU *cpu, uint64_t sccb, uint32_t code)
>        * the host has checked the values
>        */
>       work_sccb = g_malloc0(be16_to_cpu(header.length));
> -    address_space_read(as, sccb, MEMTXATTRS_UNSPECIFIED,
> -                       work_sccb, be16_to_cpu(header.length));
> +    ret = address_space_read(as, sccb, MEMTXATTRS_UNSPECIFIED,
> +                             work_sccb, be16_to_cpu(header.length));
> +    if (ret != MEMTX_OK) {
> +        return -PGM_ADDRESSING;
> +    }
>   
>       if (!sclp_command_code_valid(code)) {
>           work_sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND);
> @@ -348,8 +355,11 @@ int sclp_service_call(S390CPU *cpu, uint64_t sccb, uint32_t code)
>   
>       sclp_c->execute(sclp, work_sccb, code);
>   out_write:
> -    address_space_write(as, sccb, MEMTXATTRS_UNSPECIFIED,
> -                        work_sccb, be16_to_cpu(header.length));
> +    ret = address_space_write(as, sccb, MEMTXATTRS_UNSPECIFIED,
> +                              work_sccb, be16_to_cpu(header.length));
> +    if (ret != MEMTX_OK) {
> +        return -PGM_PROTECTION;
> +    }
>   
>       sclp_c->service_interrupt(sclp, sccb);
>   

Reviewed-by: Jason J. Herne <jjherne@linux.ibm.com>