On Thu, 21 Feb 2019 10:30:01 +0100 Cornelia Huck <cohuck@xxxxxxxxxx> wrote: > On Wed, 20 Feb 2019 22:02:10 -0500 > Eric Farman <farman@xxxxxxxxxxxxx> wrote: > > > On 02/20/2019 05:35 PM, Eric Farman wrote: > > > > > > > > > On 02/20/2019 11:44 AM, Cornelia Huck wrote: > > >> On Wed, 20 Feb 2019 11:25:46 -0500 > > >> Eric Farman <farman@xxxxxxxxxxxxx> wrote: > > >> > > >>> On 02/20/2019 07:44 AM, Cornelia Huck wrote: > > >>>> On Wed, 20 Feb 2019 06:29:38 -0500 > > >>>> Eric Farman <farman@xxxxxxxxxxxxx> wrote: > > >>>>> On 02/20/2019 04:48 AM, Cornelia Huck wrote: > > >>>>>> On Tue, 19 Feb 2019 21:49:07 -0500 > > >>>>>> Eric Farman <farman@xxxxxxxxxxxxx> wrote: > > >>>>>>> Hi Connie, Farhan, > > >>>>>>> > > >>>>>>> On 02/04/2019 12:06 PM, Cornelia Huck wrote: > > >>>>>>>> From: Farhan Ali <alifm@xxxxxxxxxxxxx> > > >>>>>>>> > > >>>>>>>> When trying to calculate the length of a ccw chain, we assume > > >>>>>>>> there are ccws after a TIC. This can lead to overcounting and > > >>>>>>>> copying garbage data from guest memory. > > >>>>>>>> > > >>>>>>>> Signed-off-by: Farhan Ali <alifm@xxxxxxxxxxxxx> > > >>>>>>>> Message-Id: > > >>>>>>>> <d63748c1f1b03147bcbf401596638627a5e35ef7.1548082107.git.alifm@xxxxxxxxxxxxx> > > >>>>>>>> > > >>>>>>>> Reviewed-by: Halil Pasic <pasic@xxxxxxxxxxxxx> > > >>>>>>>> Signed-off-by: Cornelia Huck <cohuck@xxxxxxxxxx> > > >>>>>>>> --- > > >>>>>>>> drivers/s390/cio/vfio_ccw_cp.c | 2 +- > > >>>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) > > >>>>>>>> > > >>>>>>>> diff --git a/drivers/s390/cio/vfio_ccw_cp.c > > >>>>>>>> b/drivers/s390/cio/vfio_ccw_cp.c > > >>>>>>>> index 70a006ba4d05..ba08fe137c2e 100644 > > >>>>>>>> --- a/drivers/s390/cio/vfio_ccw_cp.c > > >>>>>>>> +++ b/drivers/s390/cio/vfio_ccw_cp.c > > >>>>>>>> @@ -392,7 +392,7 @@ static int ccwchain_calc_length(u64 iova, > > >>>>>>>> struct channel_program *cp) > > >>>>>>>> return -EOPNOTSUPP; > > >>>>>>>> } > > >>>>>>>> - if ((!ccw_is_chain(ccw)) && (!ccw_is_tic(ccw))) > > >>>>>>>> + if (!ccw_is_chain(ccw)) > > >>>> > > >>>> OK, this function now returns the length of the chain excluding the > > >>>> last tic. > > >> > > >> Hm, not so sure about that anymore. I'm a bit tired, please apply salt > > >> where needed to what I'm seeing. > > > > > > I didn't get as far as I had hoped today, so I'm going to need to reset > > > to tomorrow, have coffee, and try again. But it does seem that with > > > this patch, we calculate the length of the chain up to and including the > > > TIC, and nothing beyond it. > > > > > > That is, during the boot process we calculate a chain length of "3" for > > > a SEEK + SIDE + TIC that QEMU built. > > Hm... so it looks like that code does what it says on the tin. But why > are we missing the second round, that should give us a chain with the > forth ccw? Are we missing a loop somewhere? Have you seen this email of mine: https://www.spinics.net/lists/kvm/msg182535.html (MID: Message-Id: <20190220143729.6dda5665@oc2783563651>)? Eric told me the TIC points to the SIDE. I.e the channel program says loop until SIDE presents a status modifier the jump over the TIC and continue with the READ. The point is the fourth ccw is only reachable if we jump over the TIC because of status modifier (from device). Currently we do not seem to care about this 'jump over a ccw' possibility. IMHO that is where this problem comes from. Regards, Halil
