linux-kernel - Re: [PATCH v2 08/11] swiotlb-xen: introduce phys_to_dma/dma_to

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.21.2006082045430.2815@sstabellini-ThinkPad-T480s>
Date:   Mon, 8 Jun 2020 20:46:45 -0700 (PDT)
From:   Stefano Stabellini <sstabellini@...nel.org>
To:     Stefano Stabellini <sstabellini@...nel.org>
cc:     Christoph Hellwig <hch@...radead.org>, jgross@...e.com,
        boris.ostrovsky@...cle.com, konrad.wilk@...cle.com,
        xen-devel@...ts.xenproject.org, linux-kernel@...r.kernel.org,
        tamas@...engyel.com, roman@...eda.com,
        Stefano Stabellini <stefano.stabellini@...inx.com>
Subject: Re: [PATCH v2 08/11] swiotlb-xen: introduce phys_to_dma/dma_to_phys
 translations

On Mon, 8 Jun 2020, Stefano Stabellini wrote:
> On Mon, 8 Jun 2020, Christoph Hellwig wrote:
> > On Wed, Jun 03, 2020 at 03:22:44PM -0700, Stefano Stabellini wrote:
> > > From: Stefano Stabellini <stefano.stabellini@...inx.com>
> > > 
> > > With some devices physical addresses are different than dma addresses.
> > > To be able to deal with these cases, we need to call phys_to_dma on
> > > physical addresses (including machine addresses in Xen terminology)
> > > before returning them from xen_swiotlb_alloc_coherent and
> > > xen_swiotlb_map_page.
> > > 
> > > We also need to convert dma addresses back to physical addresses using
> > > dma_to_phys in xen_swiotlb_free_coherent and xen_swiotlb_unmap_page if
> > > we want to do any operations on them.
> > > 
> > > Call dma_to_phys in is_xen_swiotlb_buffer.
> > > Call phys_to_dma in xen_phys_to_bus.
> > > Call dma_to_phys in xen_bus_to_phys.
> > > 
> > > Everything is taken care of by these changes except for
> > > xen_swiotlb_alloc_coherent and xen_swiotlb_free_coherent, which need a
> > > few explicit phys_to_dma/dma_to_phys calls.
> > > 
> > > Signed-off-by: Stefano Stabellini <stefano.stabellini@...inx.com>
> > > Tested-by: Corey Minyard <cminyard@...sta.com>
> > > Tested-by: Roman Shaposhnik <roman@...eda.com>
> > > ---
> > > Changes in v2:
> > > - improve commit message
> > > ---
> > >  drivers/xen/swiotlb-xen.c | 22 ++++++++++++----------
> > >  1 file changed, 12 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> > > index 0a6cb67f0fc4..60ef07440905 100644
> > > --- a/drivers/xen/swiotlb-xen.c
> > > +++ b/drivers/xen/swiotlb-xen.c
> > > @@ -64,16 +64,16 @@ static inline dma_addr_t xen_phys_to_bus(struct device *dev, phys_addr_t paddr)
> > >  
> > >  	dma |= paddr & ~XEN_PAGE_MASK;
> > >  
> > > -	return dma;
> > > +	return phys_to_dma(dev, dma);
> > 
> > So looking at this function:
> > 
> > The dma name for something passed to phys_to_dma is really
> > weird.
> 
> Yeah, that is true, I am not sure why I chose that confusing name. I'll
> rename it.
> 
> 
> > The fact that the comments says don't use XEN_PFN_PHYS
> > beause of the type mismatch while nothing but swiotlb-xen is the only
> > user of XEN_PFN_PHYS is also weird.  I think XEN_PFN_PHYS needs to move
> > to swiotlb-xen first, then use a hardcoded u64 for the size, and the
> > split the function into a phys_to_xen_phys (or so) function where
> > the result gets passed to phys_to_dma.
> 
> I understand what you are suggesting about having something like:
> 
>     xen_phys_to_dma(...)
>     {
>         phys_addr_t phys = xen_phys_to_bus(dev, paddr)
>         return phys_to_dma(phys);
>     }
> 
> I thought about it myself. I'll do it.
> 
> But I don't think I understood the comment about XEN_PFN_PHYS.

You meant to move the #define from the header to swiotlb-xen.c, didn't
you, and to use a cast to u64 instead of phys_addr_t?