From: Christoph Hellwig <hch@lst•de>
To: Claire Chang <tientzu@chromium•org>
Cc: heikki.krogerus@linux•intel.com, peterz@infradead•org,
grant.likely@arm•com, paulus@samba•org, frowand.list@gmail•com,
mingo@kernel•org, m.szyprowski@samsung•com,
sstabellini@kernel•org, saravanak@google•com, joro@8bytes•org,
rafael.j.wysocki@intel•com, hch@lst•de,
bgolaszewski@baylibre•com, xen-devel@lists•xenproject.org,
treding@nvidia•com, devicetree@vger•kernel.org, will@kernel•org,
konrad.wilk@oracle•com, dan.j.williams@intel•com,
linuxppc-dev@lists•ozlabs.org, robh+dt@kernel•org,
boris.ostrovsky@oracle•com, andriy.shevchenko@linux•intel.com,
jgross@suse•com, drinkcat@chromium•org,
gregkh@linuxfoundation•org, rdunlap@infradead•org,
linux-kernel@vger•kernel.org, tfiga@chromium•org,
iommu@lists•linux-foundation.org, xypron.glpk@gmx•de,
robin.murphy@arm•com, bauerman@linux•ibm.com
Subject: Re: [RFC PATCH v3 1/6] swiotlb: Add io_tlb_mem struct
Date: Wed, 13 Jan 2021 12:50:31 +0100 [thread overview]
Message-ID: <20210113115031.GA29376@lst.de> (raw)
In-Reply-To: <20210106034124.30560-2-tientzu@chromium.org>
On Wed, Jan 06, 2021 at 11:41:19AM +0800, Claire Chang wrote:
> Added a new struct, io_tlb_mem, as the IO TLB memory pool descriptor and
> moved relevant global variables into that struct.
> This will be useful later to allow for restricted DMA pool.
I like where this is going, but a few comments.
Mostly I'd love to be able to entirely hide io_tlb_default_mem
and struct io_tlb_mem inside of swiotlb.c.
> --- a/arch/powerpc/platforms/pseries/svm.c
> +++ b/arch/powerpc/platforms/pseries/svm.c
> @@ -55,8 +55,8 @@ void __init svm_swiotlb_init(void)
> if (vstart && !swiotlb_init_with_tbl(vstart, io_tlb_nslabs, false))
> return;
>
> - if (io_tlb_start)
> - memblock_free_early(io_tlb_start,
> + if (io_tlb_default_mem.start)
> + memblock_free_early(io_tlb_default_mem.start,
> PAGE_ALIGN(io_tlb_nslabs << IO_TLB_SHIFT));
I think this should switch to use the local vstart variable in
prep patch.
> panic("SVM: Cannot allocate SWIOTLB buffer");
> }
> diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> index 2b385c1b4a99..4d17dff7ffd2 100644
> --- a/drivers/xen/swiotlb-xen.c
> +++ b/drivers/xen/swiotlb-xen.c
> @@ -192,8 +192,8 @@ int __ref xen_swiotlb_init(int verbose, bool early)
> /*
> * IO TLB memory already allocated. Just use it.
> */
> - if (io_tlb_start != 0) {
> - xen_io_tlb_start = phys_to_virt(io_tlb_start);
> + if (io_tlb_default_mem.start != 0) {
> + xen_io_tlb_start = phys_to_virt(io_tlb_default_mem.start);
> goto end;
xen_io_tlb_start is interesting. It is used only in two functions:
1) is_xen_swiotlb_buffer, where I think we should be able to just use
is_swiotlb_buffer instead of open coding it with the extra
phys_to_virt/virt_to_phys cycle.
2) xen_swiotlb_init, where except for the assignment it only is used
locally for the case not touched above and could this be replaced
with a local variable.
Konrad, does this make sense to you?
> static inline bool is_swiotlb_buffer(phys_addr_t paddr)
> {
> - return paddr >= io_tlb_start && paddr < io_tlb_end;
> + struct io_tlb_mem *mem = &io_tlb_default_mem;
> +
> + return paddr >= mem->start && paddr < mem->end;
We'd then have to move this out of line as well.
next prev parent reply other threads:[~2021-01-13 11:56 UTC|newest]
Thread overview: 59+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-06 3:41 [RFC PATCH v3 0/6] Restricted DMA Claire Chang
2021-01-06 3:41 ` [RFC PATCH v3 1/6] swiotlb: Add io_tlb_mem struct Claire Chang
2021-01-13 11:50 ` Christoph Hellwig [this message]
2021-01-06 3:41 ` [RFC PATCH v3 2/6] swiotlb: Add restricted DMA pool Claire Chang
2021-01-06 7:50 ` Greg KH
2021-01-13 11:51 ` Christoph Hellwig
2021-01-13 12:29 ` Greg KH
2021-01-13 12:37 ` Christoph Hellwig
2021-01-06 18:52 ` Konrad Rzeszutek Wilk
2021-01-07 17:39 ` Claire Chang
2021-01-07 17:57 ` Konrad Rzeszutek Wilk
2021-01-07 18:09 ` Florian Fainelli
2021-01-07 21:19 ` Konrad Rzeszutek Wilk
2021-01-12 23:52 ` Florian Fainelli
2021-01-25 5:26 ` Jon Masters
2021-01-13 1:53 ` Robin Murphy
2021-01-13 0:03 ` Florian Fainelli
2021-01-13 13:59 ` Nicolas Saenz Julienne
2021-01-13 15:27 ` Robin Murphy
2021-01-13 17:43 ` Florian Fainelli
2021-01-13 18:03 ` Robin Murphy
2021-01-13 12:42 ` Christoph Hellwig
2021-01-14 9:06 ` Claire Chang
2021-01-06 3:41 ` [RFC PATCH v3 3/6] swiotlb: Use restricted DMA pool if available Claire Chang
2021-01-12 23:39 ` Florian Fainelli
2021-01-13 12:44 ` Christoph Hellwig
2021-01-06 3:41 ` [RFC PATCH v3 4/6] swiotlb: Add restricted DMA alloc/free support Claire Chang
2021-01-12 23:41 ` Florian Fainelli
2021-01-13 12:48 ` Christoph Hellwig
2021-01-13 18:27 ` Robin Murphy
2021-01-13 18:32 ` Christoph Hellwig
2021-01-06 3:41 ` [RFC PATCH v3 5/6] dt-bindings: of: Add restricted DMA pool Claire Chang
2021-01-06 18:57 ` Konrad Rzeszutek Wilk
2021-01-07 17:39 ` Claire Chang
2021-01-07 18:00 ` Konrad Rzeszutek Wilk
2021-01-07 18:14 ` Florian Fainelli
2021-01-12 7:47 ` Claire Chang
2021-01-20 16:53 ` Rob Herring
2021-01-20 17:30 ` Robin Murphy
2021-01-20 21:31 ` Rob Herring
2021-01-21 1:09 ` Robin Murphy
2021-01-21 15:48 ` Rob Herring
2021-01-21 17:29 ` Robin Murphy
2021-01-06 3:41 ` [RFC PATCH v3 6/6] of: Add plumbing for " Claire Chang
2021-01-12 23:48 ` Florian Fainelli
2021-01-14 9:08 ` Claire Chang
2021-01-14 18:52 ` Florian Fainelli
2021-01-15 3:46 ` Claire Chang
2021-01-06 18:48 ` [RFC PATCH v3 0/6] Restricted DMA Florian Fainelli
2021-01-07 17:38 ` Claire Chang
2021-01-07 17:42 ` Claire Chang
2021-01-07 17:59 ` Florian Fainelli
2021-01-12 7:48 ` Claire Chang
2021-01-12 18:01 ` Florian Fainelli
2021-01-13 2:29 ` Tomasz Figa
2021-01-13 3:56 ` Florian Fainelli
2021-01-13 4:25 ` Tomasz Figa
2021-01-13 4:41 ` Florian Fainelli
2021-02-09 6:27 ` Claire Chang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210113115031.GA29376@lst.de \
--to=hch@lst$(echo .)de \
--cc=andriy.shevchenko@linux$(echo .)intel.com \
--cc=bauerman@linux$(echo .)ibm.com \
--cc=bgolaszewski@baylibre$(echo .)com \
--cc=boris.ostrovsky@oracle$(echo .)com \
--cc=dan.j.williams@intel$(echo .)com \
--cc=devicetree@vger$(echo .)kernel.org \
--cc=drinkcat@chromium$(echo .)org \
--cc=frowand.list@gmail$(echo .)com \
--cc=grant.likely@arm$(echo .)com \
--cc=gregkh@linuxfoundation$(echo .)org \
--cc=heikki.krogerus@linux$(echo .)intel.com \
--cc=iommu@lists$(echo .)linux-foundation.org \
--cc=jgross@suse$(echo .)com \
--cc=joro@8bytes$(echo .)org \
--cc=konrad.wilk@oracle$(echo .)com \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=linuxppc-dev@lists$(echo .)ozlabs.org \
--cc=m.szyprowski@samsung$(echo .)com \
--cc=mingo@kernel$(echo .)org \
--cc=paulus@samba$(echo .)org \
--cc=peterz@infradead$(echo .)org \
--cc=rafael.j.wysocki@intel$(echo .)com \
--cc=rdunlap@infradead$(echo .)org \
--cc=robh+dt@kernel$(echo .)org \
--cc=robin.murphy@arm$(echo .)com \
--cc=saravanak@google$(echo .)com \
--cc=sstabellini@kernel$(echo .)org \
--cc=tfiga@chromium$(echo .)org \
--cc=tientzu@chromium$(echo .)org \
--cc=treding@nvidia$(echo .)com \
--cc=will@kernel$(echo .)org \
--cc=xen-devel@lists$(echo .)xenproject.org \
--cc=xypron.glpk@gmx$(echo .)de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox