On Wed, 2022-11-09 at 17:20 -0500, Matthew Rosato wrote:
> On 11/9/22 3:21 PM, Eric Farman wrote:
> > From: Alexander Gordeev <agordeev@xxxxxxxxxxxxx>
> >
> > The ORB is a construct that is sent to the real hardware,
> > so should contain a physical address in its interrupt
> > parameter field. Let's clarify that.
> >
> > Note: this currently doesn't fix a real bug, since virtual
> > addresses are identical to physical ones.
> >
> > Signed-off-by: Alexander Gordeev <agordeev@xxxxxxxxxxxxx>
> > [EF: Updated commit message]
> > Signed-off-by: Eric Farman <farman@xxxxxxxxxxxxx>
> > ---
> > drivers/s390/cio/vfio_ccw_fsm.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/s390/cio/vfio_ccw_fsm.c
> > b/drivers/s390/cio/vfio_ccw_fsm.c
> > index a59c758869f8..0a5e8b4a6743 100644
> > --- a/drivers/s390/cio/vfio_ccw_fsm.c
> > +++ b/drivers/s390/cio/vfio_ccw_fsm.c
> > @@ -29,7 +29,7 @@ static int fsm_io_helper(struct vfio_ccw_private
> > *private)
> >
> > spin_lock_irqsave(sch->lock, flags);
> >
> > - orb = cp_get_orb(&private->cp, (u32)(addr_t)sch, sch->lpm);
> > + orb = cp_get_orb(&private->cp, (u32)virt_to_phys(sch), sch-
> > >lpm);
>
> Nit: I think it would make more sense to do the virt_to_phys inside
> cp_get_orb at the time we place the address in the orb (since that's
> what gets sent to hardware), rather than requiring all callers of
> cp_get_orb to pass a physical address. I realize there is only 1
> caller today.
Eh, maybe so. But that takes me into the 'what are we passing to this
routine and how can we simplify it' rabbit hole, and it stops becoming
a nit pretty quickly. I'd rather keep this patch as the simple change
described here. I have some more involved rework in the broader cp code
in the pipe, and can include your suggestion with that.
>
> Nit++: Can we make the patch subjects match? vfio/ccw or vfio-ccw
Heh, fair. "vfio/ccw" has been the style du jour of late.
>
> Either way:
>
> Reviewed-by: Matthew Rosato <mjrosato@xxxxxxxxxxxxx>
Thanks!
>
> > if (!orb) {
> > ret = -EIO;
> > goto out;
>
