Re: [PATCH] s390/vfio_ccw: Fix target addresses of TIC CCWs

[Eric Farman <farman@linux.ibm.com>]

On Fri, 2024-06-28 at 15:40 +0200, Heiko Carstens wrote:
> On Fri, Jun 28, 2024 at 09:31:56AM -0400, Eric Farman wrote:
> > > > dma32_to_u32(ccw->cda) - ccw_head;
> > > > -			ccw->cda = u32_to_dma32(cda);
> > > > +			/* Calculate offset of TIC target */
> > > > +			cda = dma32_to_u32(ccw->cda) - ccw_head;
> > > > +			ccw->cda = virt_to_dma32(iter->ch_ccw) +
> > > > cda;
> > > 
> > > I would suggest to rename cda to "offset", since that reflects what
> > > it is
> > > now. Also this code needs to take care of type checking, which will
> > > fail now
> > > due to dma32_t type (try "make C=1 drivers/s390/cio/vfio_ccw_cp.o).
> 
> ...
> 
> > I was poking at that code yesterday because it seemed suspect, but as I
> > wasn't getting an explicit failure (versus the CPC generated by hw), I
> > opted to leave it for now. I agree they should both be fixed up.
> 
> ...
> 
> > > I guess
> > > you could add this hunk to your patch:
> > > 
> > > @@ -915,7 +915,7 @@ void cp_update_scsw(struct channel_program *cp,
> > > union scsw *scsw)
> > >  	 * in the ioctl directly. Path status changes etc.
> > >  	 */
> > >  	list_for_each_entry(chain, &cp->ccwchain_list, next) {
> > > -		ccw_head = (u32)(u64)chain->ch_ccw;
> > > +		ccw_head = (__force u32)virt_to_dma32(chain-
> > > > ch_ccw);
> > >  		/*
> > >  		 * On successful execution, cpa points just beyond
> > > the end
> > >  		 * of the chain.
> 
> ...
> 
> > > Furthermore it looks to me like the ch_iova member of struct ccwchain
> > > should
> > > get a dma32_t type instead of u64. The same applies to quite a few
> > > variables
> > > to the code. 
> > 
> > Agreed. I started this some time back after the IDAW code got reworked,
> > but have been sidetracked. The problem with ch_iova is more apparent
> > after the dma32 stuff.
> > 
> > > I could give this a try, but I think it would be better if
> > > somebody who knows what he is doing would address this :)
> > 
> > I'll finish them up. But v2 will have to wait until after my holiday.
> > Thanks for reminding me of the typechecking!
> 
> I hope you didn't get me wrong: from my point of view we want one or
> two small patches (the above hunks), which fix the bugs, if you
> agree.
> 
> And then address the type checking stuff at a later point in time.

-ENOCOFFEE. Sorry, I did misunderstand. I'll send the small patches
later this morning.

> 
> (btw: your mailer adds lot's of extra line wraps)
> 

Ugh. Thanks for the heads up.