diff mbox series

[v2] s390/vfio_ccw: Fix target addresses of TIC CCWs

Message ID 20240628163738.3643513-1-farman@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series [v2] s390/vfio_ccw: Fix target addresses of TIC CCWs | expand

Commit Message

Eric Farman June 28, 2024, 4:37 p.m. UTC
The processing of a Transfer-In-Channel (TIC) CCW requires locating
the target of the CCW in the channel program, and updating the
address to reflect what will actually be sent to hardware.

An error exists where the 64-bit virtual address is truncated to
32-bits (variable "cda") when performing this math. Since s390
addresses of that size are 31-bits, this leaves that additional
bit enabled such that the resulting I/O triggers a channel
program check. This shows up occasionally when booting a KVM
guest from a passthrough DASD device:

  ..snip...
  Interrupt Response Block Data:
  : 0x0000000000003990
      Function Ctrl : [Start]
      Activity Ctrl :
      Status Ctrl : [Alert] [Primary] [Secondary] [Status-Pending]
      Device Status :
      Channel Status : [Program-Check]
      cpa=: 0x00000000008d0018
      prev_ccw=: 0x0000000000000000
      this_ccw=: 0x0000000000000000
  ...snip...
  dasd-ipl: Failed to run IPL1 channel program

The channel program address of "0x008d0018" in the IRB doesn't
look wrong, but tracing the CCWs shows the offending bit enabled:

  ccw=0x0000012e808d0000 cda=00a0b030
  ccw=0x0000012e808d0008 cda=00a0b038
  ccw=0x0000012e808d0010 cda=808d0008
  ccw=0x0000012e808d0018 cda=00a0b040

Fix the calculation of the TIC CCW's data address such that it points
to a valid 31-bit address regardless of the input address.

Fixes: bd36cfbbb9e1 ("s390/vfio_ccw_cp: use new address translation helpers")
Signed-off-by: Eric Farman <farman@linux.ibm.com>
---

Notes:
    v2:
     - [HC] Fix dma32/int warning on make C=1
     - [HC] Rename cda variable to offset
     - [HC] Fix similar bug in cp_update_scsw()
    v1: https://lore.kernel.org/r/20240627200740.373192-1-farman@linux.ibm.com/

 drivers/s390/cio/vfio_ccw_cp.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

Comments

Heiko Carstens July 1, 2024, 8:33 a.m. UTC | #1
On Fri, Jun 28, 2024 at 06:37:38PM +0200, Eric Farman wrote:
> The processing of a Transfer-In-Channel (TIC) CCW requires locating
> the target of the CCW in the channel program, and updating the
> address to reflect what will actually be sent to hardware.
> 
> An error exists where the 64-bit virtual address is truncated to
> 32-bits (variable "cda") when performing this math. Since s390
> addresses of that size are 31-bits, this leaves that additional
> bit enabled such that the resulting I/O triggers a channel
> program check. This shows up occasionally when booting a KVM
> guest from a passthrough DASD device:
...
> Fixes: bd36cfbbb9e1 ("s390/vfio_ccw_cp: use new address translation helpers")
> Signed-off-by: Eric Farman <farman@linux.ibm.com>
> ---
> 
> Notes:
>     v2:
>      - [HC] Fix dma32/int warning on make C=1
>      - [HC] Rename cda variable to offset
>      - [HC] Fix similar bug in cp_update_scsw()
>     v1: https://lore.kernel.org/r/20240627200740.373192-1-farman@linux.ibm.com/
> 
>  drivers/s390/cio/vfio_ccw_cp.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)

Reviewed-by: Heiko Carstens <hca@linux.ibm.com>
Alexander Gordeev July 2, 2024, 6:44 a.m. UTC | #2
On Fri, Jun 28, 2024 at 06:37:38PM +0200, Eric Farman wrote:
> The processing of a Transfer-In-Channel (TIC) CCW requires locating
> the target of the CCW in the channel program, and updating the
> address to reflect what will actually be sent to hardware.
> 
> An error exists where the 64-bit virtual address is truncated to
> 32-bits (variable "cda") when performing this math. Since s390
> addresses of that size are 31-bits, this leaves that additional
> bit enabled such that the resulting I/O triggers a channel
> program check. This shows up occasionally when booting a KVM
> guest from a passthrough DASD device:
> 
>   ..snip...
...
>  drivers/s390/cio/vfio_ccw_cp.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)

Applied, thanks!
diff mbox series

Patch

diff --git a/drivers/s390/cio/vfio_ccw_cp.c b/drivers/s390/cio/vfio_ccw_cp.c
index 6e5c508b1e07..5f6e10225627 100644
--- a/drivers/s390/cio/vfio_ccw_cp.c
+++ b/drivers/s390/cio/vfio_ccw_cp.c
@@ -490,13 +490,14 @@  static int ccwchain_fetch_tic(struct ccw1 *ccw,
 			      struct channel_program *cp)
 {
 	struct ccwchain *iter;
-	u32 cda, ccw_head;
+	u32 offset, ccw_head;
 
 	list_for_each_entry(iter, &cp->ccwchain_list, next) {
 		ccw_head = iter->ch_iova;
 		if (is_cpa_within_range(ccw->cda, ccw_head, iter->ch_len)) {
-			cda = (u64)iter->ch_ccw + dma32_to_u32(ccw->cda) - ccw_head;
-			ccw->cda = u32_to_dma32(cda);
+			/* Calculate offset of TIC target */
+			offset = dma32_to_u32(ccw->cda) - ccw_head;
+			ccw->cda = virt_to_dma32((void *)iter->ch_ccw + offset);
 			return 0;
 		}
 	}
@@ -914,7 +915,7 @@  void cp_update_scsw(struct channel_program *cp, union scsw *scsw)
 	 * in the ioctl directly. Path status changes etc.
 	 */
 	list_for_each_entry(chain, &cp->ccwchain_list, next) {
-		ccw_head = (u32)(u64)chain->ch_ccw;
+		ccw_head = dma32_to_u32(virt_to_dma32(chain->ch_ccw));
 		/*
 		 * On successful execution, cpa points just beyond the end
 		 * of the chain.