qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Halil Pasic <pasic@linux.ibm.com>
To: Pierre Morel <pmorel@linux.ibm.com>
Cc: thuth@redhat.com, frankja@linux.ibm.com, mst@redhat.com,
	Cornelia Huck <cohuck@redhat.com>,
	richard.henderson@linaro.org, qemu-devel@nongnu.org,
	borntraeger@de.ibm.com, qemu-s390x@nongnu.org,
	marcandre.lureau@redhat.com, pbonzini@redhat.com,
	david@redhat.com, imbrenda@linux.ibm.com
Subject: Re: [PATCH v1 1/1] s390x: css: report errors from ccw_dstream_read/write
Date: Wed, 7 Apr 2021 18:54:26 +0200	[thread overview]
Message-ID: <20210407185426.257ed03d.pasic@linux.ibm.com> (raw)
In-Reply-To: <c541aacd-34cb-3ed5-0016-cca1064c67e6@linux.ibm.com>

On Wed, 7 Apr 2021 13:41:57 +0200
Pierre Morel <pmorel@linux.ibm.com> wrote:

> > Here, however, I'm not sure. Returning a negative error here is fine,
> > but handle_payload_3270_write (not changed in this patch) seems to
> > match everything to -EIO. Shouldn't it just be propagated, and maybe 0
> > mapped to -EIO only? If I'm not confused, we'll end up mapping every
> > error to intervention required.  
> I know very little about the 3270 but in my opinion accessing memory is 
> not the problem of the device but of the subchannel.
> So we should certainly propagate the error instead of returning -EIO for 
> any error.

I agree, what condition needs to be indicated when we encounter an
invalid CCW, IDAW or MIDAW address is clear. In the PoP this is
described under Chapter 16. I/O Interruptions > Subchannel-Status Word >
Subchannel-Status Field > Program Check in the paragraphs on "Invalid
IDAW or MIDAW Address and "Invalid CCW Address".

My guess about handle_payload_3270_write() is that IMHO the initial 3270
emulation code was, let's say of mixed quality in the first place, so
wouldn't seek some hidden meaning/sense, behind the -EIO. IMHO first
mapping architectural conditions to "errno-style" error codes, passing
these around, and then mapping them back is a litle non-intuitive. If one
looks at the purpose of handle_payload_3270_write(), it is basically
emulating an IO data transfer from the device into the main storage. If
that sort of operation. So with -EIO the original author probably wanted
to say: hey there was an input/output error, which ain't that wrong. The
problem is that somewhere up the call stack EIO gets interpreted in a
very peculiar way, and along with other errnos mapped to CIO
architecture stuff like SCSW bits.


  reply	other threads:[~2021-04-07 16:56 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-06  7:44 [PATCH v1 0/1] " Pierre Morel
2021-04-06  7:44 ` [PATCH v1 1/1] " Pierre Morel
2021-04-06 15:03   ` Cornelia Huck
2021-04-07 11:41     ` Pierre Morel
2021-04-07 16:54       ` Halil Pasic [this message]
2021-04-08  8:53         ` Cornelia Huck
2021-04-07 17:47   ` Halil Pasic
2021-04-08  9:02     ` Cornelia Huck
2021-04-08 12:32       ` Pierre Morel
2021-04-08 13:23         ` Cornelia Huck
2021-04-08 16:18           ` Pierre Morel
2021-04-08 12:39       ` Halil Pasic
2021-04-08 13:26         ` Cornelia Huck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210407185426.257ed03d.pasic@linux.ibm.com \
    --to=pasic@linux.ibm.com \
    --cc=borntraeger@de.ibm.com \
    --cc=cohuck@redhat.com \
    --cc=david@redhat.com \
    --cc=frankja@linux.ibm.com \
    --cc=imbrenda@linux.ibm.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=pmorel@linux.ibm.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-s390x@nongnu.org \
    --cc=richard.henderson@linaro.org \
    --cc=thuth@redhat.com \
    --subject='Re: [PATCH v1 1/1] s390x: css: report errors from ccw_dstream_read/write' \


* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).