[v2,00/16] vfio/ccw: channel program cleanup

Message

Eric Farman Dec. 20, 2022, 5:09 p.m. UTC

Hi Matt, et al,

(Apologies for sending this out right before the holiday, again;
but I have this in a state where it's unlikely to change further.)

Here is a new version of the first batch of the vfio-ccw channel
program handler rework, which I'm referring to as "the IDA code."
Most of it is rearrangement to make it more readable, and to remove
restrictions in place since commit 0a19e61e6d4c ("vfio: ccw: introduce
channel program interfaces"). My hope is that with this off the plate,
it will be easier to extend this logic for newer, more modern, features.

Some background:

A Format-1 Channel Command Word (CCW) contains a 31-bit data address,
meaning any I/O transfer is limited to the first 2GB of memory.
The concept of an Indirect Data Address (IDA) was introduced long ago
to allow for non-contiguous memory to be used for data transfers,
while still using 31-bit data addresses.

The initial z/Architecture extended the definition of ESA/390's IDA
concept to include a new IDA format that allows for 64-bit data
addresses [1]. The result is three distinct IDA flavors:

 - Format-1 IDA (31-bit addresses), each IDAW moves up to 2K of data
 - Format-2 IDA (64-bit addresses), each IDAW moves up to 2K of data
 - Format-2 IDA (64-bit addresses), each IDAW moves up to 4K of data

The selection of these three possibilities is done by bits within the
Operation-Request Block (ORB), though it should not be a surprise
that the last one is far-and-away the most common these days.

While newer features can be masked off (by a CPU model or equivalent),
and guarded by a defensive check in a driver (such as our check for
a Transport Mode ORB in fsm_io_request()), all three of these
possibilities are available by the base z/Architecture, and cannot
be "hidden." So while it might be unlikely for a guest to issue
such an I/O, it's not impossible.

vfio-ccw was written to only support the third of these options,
while the first two are rejected by a check buried in
ccwchain_calc_length(). While a Transport Mode ORB gets a
distinct message logged, no such announcement as to the cause
of the problem is done here, except for a generic -EOPNOTSUPP
return code in the prefetch codepath.

The goal of this series is to rework the channel program handler
such that any of the above IDA formats can be processed and sent
to the hardware. Any Format-1 IDA issued by a guest will be
converted to the 2K Format-2 variety, such that it is able to
use the full 64-bit addressing. One change from today is that any
direct-addressed CCW can only converted to a 4K Format-2 IDA if
the ORB settings permit this. Otherwise, those CCWs would need to
be converted to a 2K Format-2 IDA to maintain compatibility with
potential IDA CCWs elsewhere in the chain.

The first few patches at the start of this series are improvements
that could stand alone, regardless of the rework that follows.
The remainder of the series is intended to do the code movement
that would enable these older IDA formats, but the restriction
itself isn't removed until the end.

Thanks in advance, I look forward to the feedback...

Eric

[1] See most recent Principles of Operation, page 1-6

v1->v2:
 - [EF] Rebased to current upstream master (with s390/vfio patches merged)
 - [MR, JG] Collected r-b's (Thank you!)
 - Patch 1:
   [MR] Drop Fixes tag
 - Patch 6:
   [MR] Fix typo in commit message
 - Patch 7:
   [MR] Convert forgotten "ch_pa + i" to "&ch_pa[i]" notation for consistency
 - Patch 8:
   [MR] Change "!len" to "len == 0" since it's not a pointer
 - Patch 11:
   [MR] Only read 4 bytes for a Format-1 IDA, instead of 8 bytes
 - Patch 12:
   [MR] Remove unnecessary else in ccw_count_idals
   [EF (checkpatch.pl --strict)] Whitespace changes
 - Patch 13:
   [MR] Return ERR_PTR for errors in get_guest_idal(), instead of NULL, and check
   appropriately in caller
 - Patch 14:
   [EF (checkpatch.pl --strict)] Wrap "_cp" in parentheses in definition of idal_is_2k
 - Patch 15:
   [MR] Add comments for "unaligned" parameter on page_array_{alloc|unpin}
 - Patch 16:
   [MR] Drop unnecessary comma from Documentation/s390/vfio-ccw.rst
v1: https://lore.kernel.org/kvm/[e-mail]/

Eric Farman (16):
  vfio/ccw: cleanup some of the mdev commentary
  vfio/ccw: simplify the cp_get_orb interface
  vfio/ccw: allow non-zero storage keys
  vfio/ccw: move where IDA flag is set in ORB
  vfio/ccw: replace copy_from_iova with vfio_dma_rw
  vfio/ccw: simplify CCW chain fetch routines
  vfio/ccw: remove unnecessary malloc alignment
  vfio/ccw: pass page count to page_array struct
  vfio/ccw: populate page_array struct inline
  vfio/ccw: refactor the idaw counter
  vfio/ccw: read only one Format-1 IDAW
  vfio/ccw: calculate number of IDAWs regardless of format
  vfio/ccw: allocate/populate the guest idal
  vfio/ccw: handle a guest Format-1 IDAL
  vfio/ccw: don't group contiguous pages on 2K IDAWs
  vfio/ccw: remove old IDA format restrictions

 Documentation/s390/vfio-ccw.rst |   4 +-
 arch/s390/include/asm/idals.h   |  12 ++
 drivers/s390/cio/vfio_ccw_cp.c  | 362 +++++++++++++++++---------------
 drivers/s390/cio/vfio_ccw_cp.h  |   3 +-
 drivers/s390/cio/vfio_ccw_fsm.c |   2 +-
 5 files changed, 210 insertions(+), 173 deletions(-)