Re: [PATCH 1/2] vfio-ccw: sort out physical vs virtual pointers usage

[Matthew Rosato <mjrosato@linux.ibm.com>]

On 11/9/22 3:21 PM, Eric Farman wrote:
> From: Alexander Gordeev <agordeev@linux.ibm.com>
> 
> 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@linux.ibm.com>
> [EF: Updated commit message]
> Signed-off-by: Eric Farman <farman@linux.ibm.com>
> ---
>  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.

Nit++: Can we make the patch subjects match?  vfio/ccw or vfio-ccw

Either way:

Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>

>  	if (!orb) {
>  		ret = -EIO;
>  		goto out;