RE: [PATCH rdma-rc v1] RDMA/core: fix sg_to_page mapping for boundary condition

[Kashyap Desai <kashyap.desai@broadcom.com>]

[-- Attachment #1: Type: text/plain, Size: 2441 bytes --]

>
> On Thu, Sep 01, 2022 at 05:36:57PM +0530, Kashyap Desai wrote:
> > > -----Original Message-----
> > > From: Jason Gunthorpe [mailto:jgg@nvidia.com]
> > > Sent: Friday, August 26, 2022 6:45 PM
> > > To: Kashyap Desai <kashyap.desai@broadcom.com>
> > > Cc: linux-rdma@vger.kernel.org; leonro@nvidia.com; Selvin Xavier
> > > <selvin.xavier@broadcom.com>; Andrew Gospodarek
> > > <andrew.gospodarek@broadcom.com>
> > > Subject: Re: [PATCH rdma-rc v1] RDMA/core: fix sg_to_page mapping
> > > for boundary condition
> > >
> > > On Mon, Aug 22, 2022 at 07:51:22PM +0530, Kashyap Desai wrote:
> > >
> > > > Now, we will enter into below loop with dma_addr = page_addr =
> > > > 0xffffffffffffe000 and "end_dma_addr = dma_addr + dma_len" is
ZERO.
> > > > eval 0xffffffffffffe000 + 8192
> > > > hexadecimal: 0
> > >
> > > This is called overflow.
> >
> > Is this not DMAable for 64bit DMA mask device ? It is DMAable. So not
> > sure why you call it as overflow. ?
>
> Beacuse the normal math overflowed.
>
> Should it work? Yes. Is it a special edge case that might have bugs?
> Certainly.
>
> So the IOMMU layer shouldn't be stressing this edge case at all. It is
crazy, there
> is no reason to do this.
>
> > I agree that such mapping is obviously dangerous, but it is not
> > illegal as well.
> > Same sgl mapping works if it is direct attached Storage, so there will
> > be a logical question why IB stack is not handling this.
>
> Oh that is probably very driver dependent.
>
> > > You need to write the code so you never create the situation where
> > > A+B=0 - don't try to fix things up after that happens.
> >
> > In proposed patch, A + B = 0 is possible, but it will be considered as
> > end of the loop.
>
> Like I said, don't do that. End of the loop is -1 which requires a
different loop
> logic design, so send a patch like that.
>
> But I would still send a patch for iommu to not create this in the first
place.


Jason -
I noted your response.  Issue is possible to any RDMA h/w and issue is not
completely Rejected. It is just that you need another way to fix it
(preferred to handle it in iommu)
We can reopen discussion if we see another instance from other vendors.
Quick workaround is - use  63 bit DMA mask in rdma low level driver if
someone really wants to work around this issue until something more robust
fix committed in upstream kernel (either ib stack or iommu stack).

We can close this thread.

Kashyap
>
> Jason

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]