On Thu, 17 Nov 2022 11:01:43 +0100 Claudio Imbrenda <imbrenda@xxxxxxxxxxxxx> wrote: > On Thu, 17 Nov 2022 09:50:14 +0100 > Nico Boehr <nrb@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx> > > > > --- > > > > 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. The reason for the cast was that gisa is a pointer, but we needed to do integer arithmetic on the address of the object pointed to by the pointer. It happens so that the pointer must point to a piece of memory that is 31 bit addressable in host real address space, but for getting the address from a pointer, casting to the unsigned integral type with-wise corresponds to the pointer is IMHO sensible regardless of that information. >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 Right: that cast makes no sense any more. And with that change: Reviewed-by: Halil Pasic <pasic@xxxxxxxxxxxxx>
