Re: [PULL 1/1] vfio-ccw: Don't assume there are more ccws after a TIC

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 21 Feb 2019 20:09:16 +0100
Halil Pasic <pasic@xxxxxxxxxxxxx> wrote:

> On Thu, 21 Feb 2019 12:10:32 -0500
> Eric Farman <farman@xxxxxxxxxxxxx> wrote:

> > Well, taking the TIC-boundary logic from tic_target_chain_exists() and 
> > stuffing it into ccwchain_calc_length() seems to do the trick for seeing 
> > if we're sending a TIC back into our current chain, and makes the dasd 
> > boot work again (the following is built on top of this patch, and is 
> > surely whitespace damaged):
> > 
> > diff --git a/drivers/s390/cio/vfio_ccw_cp.c b/drivers/s390/cio/vfio_ccw_cp.c
> > index ba08fe137c2e..a4abe7519b7e 100644
> > --- a/drivers/s390/cio/vfio_ccw_cp.c
> > +++ b/drivers/s390/cio/vfio_ccw_cp.c
> > @@ -362,6 +362,7 @@ static int ccwchain_calc_length(u64 iova, struct 
> > channel_program *cp)
> >   {
> >          struct ccw1 *ccw, *p;
> >          int cnt;
> > +       u64 tail;
> > 
> >          /*
> >           * Copy current chain from guest to host kernel.
> > @@ -392,7 +393,9 @@ static int ccwchain_calc_length(u64 iova, struct 
> > channel_program *cp)
> >                          return -EOPNOTSUPP;
> >                  }
> > 
> > -               if (!ccw_is_chain(ccw))
> > +               tail = iova + (cnt - 1) * sizeof(struct ccw1);
> > +
> > +               if (!(ccw_is_chain(ccw) || (ccw_is_tic(ccw) && iova <= 
> > ccw->cda && ccw->cda <= tail)))  
> 
> After De Morgan this looks like
> 
> 
> !ccw_is_chain(ccw)) && !(ccw_is_tic(ccw) && tic_points_in_this_chain())
> 
> which only differs form the state without the Farhan fix by not carrying
> on for tic's that point outside of this chain.
> 
> Would certainly catch the problem you discovered.
> 
> Of course there is no guarantee that this covers all the theoretically
> possible cases where we could skip over a ccw that would terminate a chain
> (tic or not chaining) if there was no status modifier. If you like I can
> construct an example where this heuristic fails (at least I think so).
> 
> If this is the direction we are going to take I think "vfio-ccw: Don't
> assume there are more ccws after a TIC" should be superseded by this. And
> we need a comment that explains the second part of the expression.

As that patch is already queued, I think we should do that change with
'fixes' on top.

There are probably more corner cases that we're missing, but this fixes
a problem that can be hit today (well, with some posted patches...), so
I'd go ahead and queue it and go through the code and maybe fix/rework
on top.

Did any of you do any stress testing of that change yet?



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux