Re: [PATCH v1] s390/vfio-ap: GISA: sort out physical vs virtual pointers usage

[Claudio Imbrenda <imbrenda@linux.ibm.com>]

On Thu, 17 Nov 2022 09:50:14 +0100
Nico Boehr <nrb@linux.ibm.com> wrote:

> Quoting Janosch Frank (2022-11-15 09:56:52)
> > On 11/8/22 16:26, Nico Boehr wrote:  
> > > Fix virtual vs physical address confusion (which currently are the same)
> > > for the GISA when enabling the IRQ.
> > > 
> > > Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
> > > ---
> > >   drivers/s390/crypto/vfio_ap_ops.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
> > > index 0b4cc8c597ae..20859cabbced 100644
> > > --- a/drivers/s390/crypto/vfio_ap_ops.c
> > > +++ b/drivers/s390/crypto/vfio_ap_ops.c
> > > @@ -429,7 +429,7 @@ static struct ap_queue_status vfio_ap_irq_enable(struct vfio_ap_queue *q,
> > >   
> > >       aqic_gisa.isc = nisc;
> > >       aqic_gisa.ir = 1;
> > > -     aqic_gisa.gisa = (uint64_t)gisa >> 4;
> > > +     aqic_gisa.gisa = (uint64_t)virt_to_phys(gisa) >> 4;  
> > 
> > I'd suggest doing s/uint64_t/u64/ or s/uint64_t/unsigned long/ but I'm 
> > wondering if (u32)(u64) would be more appropriate anyway.  
> 
> The gisa origin is a unsigned int, hence you are right, uint64_t is odd. But since virt_to_phys() returns unsigned long, the cast to uint64_t is now useless.
> 
> My suggestion is to remove the cast alltogether.

I agree to remove it