On 11/21/22 4:40 PM, Eric Farman wrote:
> There are two scenarios that need to be addressed here.
>
> First, an ORB that does NOT have the Format-2 IDAL bit set could
> have both a direct-addressed CCW and an indirect-data-address CCW
> chained together. This means that the IDA CCW will contain a
> Format-1 IDAL, and can be easily converted to a 2K Format-2 IDAL.
> But it also means that the direct-addressed CCW needs to be
> converted to the same 2K Format-2 IDAL for consistency with the
> ORB settings.
>
> Secondly, a Format-1 IDAL is comprised of 31-bit addresses.
> Thus, we need to cast this IDAL to a pointer of ints while
> populating the list of addresses that are sent to vfio.
>
> Since the result of both of these is the use of the 2K IDAL
> variants, and the output of vfio-ccw is always a Format-2 IDAL
> (in order to use 64-bit addresses), make sure that the correct
> control bit gets set in the ORB when these scenarios occur.
>
> Signed-off-by: Eric Farman <farman@xxxxxxxxxxxxx>
> ---
> drivers/s390/cio/vfio_ccw_cp.c | 19 ++++++++++++++++---
> 1 file changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/s390/cio/vfio_ccw_cp.c b/drivers/s390/cio/vfio_ccw_cp.c
> index 90685cee85db..9527f3d8da77 100644
> --- a/drivers/s390/cio/vfio_ccw_cp.c
> +++ b/drivers/s390/cio/vfio_ccw_cp.c
> @@ -222,6 +222,8 @@ static void convert_ccw0_to_ccw1(struct ccw1 *source, unsigned long len)
> }
> }
>
> +#define idal_is_2k(_cp) (!_cp->orb.cmd.c64 || _cp->orb.cmd.i2k)
> +
> /*
> * Helpers to operate ccwchain.
> */
> @@ -504,8 +506,9 @@ static unsigned long *get_guest_idal(struct ccw1 *ccw,
> struct vfio_device *vdev =
> &container_of(cp, struct vfio_ccw_private, cp)->vdev;
> unsigned long *idaws;
> + unsigned int *idaws_f1;
I wonder if we should be using explicit u64/u32 here since we are dealing with hardware-architected data sizes and specifically want to index by 32- or 64-bits. Honestly, there's probably a number of other spots in vfio-ccw where that might make sense so it would also be OK to look into that as a follow-on.
Otherwise, LGTM
Reviewed-by: Matthew Rosato <mjrosato@xxxxxxxxxxxxx>
> int idal_len = idaw_nr * sizeof(*idaws);
> - int idaw_size = PAGE_SIZE;
> + int idaw_size = idal_is_2k(cp) ? PAGE_SIZE / 2 : PAGE_SIZE;
> int idaw_mask = ~(idaw_size - 1);
> int i, ret;
>
> @@ -527,8 +530,10 @@ static unsigned long *get_guest_idal(struct ccw1 *ccw,
> for (i = 1; i < idaw_nr; i++)
> idaws[i] = (idaws[i - 1] + idaw_size) & idaw_mask;
> } else {
> - kfree(idaws);
> - return NULL;
> + idaws_f1 = (unsigned int *)idaws;
> + idaws_f1[0] = ccw->cda;
> + for (i = 1; i < idaw_nr; i++)
> + idaws_f1[i] = (idaws_f1[i - 1] + idaw_size) & idaw_mask;
> }
> }
>
> @@ -593,6 +598,7 @@ static int ccwchain_fetch_ccw(struct ccw1 *ccw,
> struct vfio_device *vdev =
> &container_of(cp, struct vfio_ccw_private, cp)->vdev;
> unsigned long *idaws;
> + unsigned int *idaws_f1;
> int ret;
> int idaw_nr;
> int i;
> @@ -623,9 +629,12 @@ static int ccwchain_fetch_ccw(struct ccw1 *ccw,
> * Copy guest IDAWs into page_array, in case the memory they
> * occupy is not contiguous.
> */
> + idaws_f1 = (unsigned int *)idaws;
> for (i = 0; i < idaw_nr; i++) {
> if (cp->orb.cmd.c64)
> pa->pa_iova[i] = idaws[i];
> + else
> + pa->pa_iova[i] = idaws_f1[i];
> }
>
> if (ccw_does_data_transfer(ccw)) {
> @@ -846,7 +855,11 @@ union orb *cp_get_orb(struct channel_program *cp, struct subchannel *sch)
>
> /*
> * Everything built by vfio-ccw is a Format-2 IDAL.
> + * If the input was a Format-1 IDAL, indicate that
> + * 2K Format-2 IDAWs were created here.
> */
> + if (!orb->cmd.c64)
> + orb->cmd.i2k = 1;
> orb->cmd.c64 = 1;
>
> if (orb->cmd.lpm == 0)
