On Mon, 2022-12-19 at 15:29 -0500, Matthew Rosato wrote: > 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. This is a fair point. I have some follow-on work to this series for after the holidays, so I'm going to put this suggestion on that todo list. > > 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) >