Re: [PATCH 11/12] swiotlb: merge swiotlb-xen initialization into swiotlb
On 3/9/22 1:18 AM, Christoph Hellwig wrote:
On Tue, Mar 08, 2022 at 04:38:21PM -0500, Boris Ostrovsky wrote:
On 3/1/22 5:53 AM, Christoph Hellwig wrote:
Allow to pass a remap argument to the swiotlb initialization functions
to handle the Xen/x86 remap case. ARM/ARM64 never did any remapping
from xen_swiotlb_fixup, so we don't even need that quirk.
Any chance this patch could be split? Lots of things are happening here and
it's somewhat hard to review. (Patch 7 too BTW but I think I managed to get
through it)
What would be your preferred split?
swiotlb_init() rework to be done separately?
diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
index e0def4b1c3181..2f2c468acb955 100644
--- a/arch/x86/kernel/pci-dma.c
+++ b/arch/x86/kernel/pci-dma.c
@@ -71,15 +71,12 @@ static inline void __init pci_swiotlb_detect(void)
#endif /* CONFIG_SWIOTLB */
#ifdef CONFIG_SWIOTLB_XEN
-static bool xen_swiotlb;
-
static void __init pci_xen_swiotlb_init(void)
{
if (!xen_initial_domain() && !x86_swiotlb_enable)
return;
Now that there is a single call site for this routine I think this check can be
dropped. We are only called here for xen_initial_domain()==true.
The callsite just checks xen_pv_domain() and itself is called
unconditionally during initialization.
Oh, right, nevermind. *pv* domain.
-boris
Re: [PATCH 11/12] swiotlb: merge swiotlb-xen initialization into swiotlb
On Tue, Mar 08, 2022 at 04:38:21PM -0500, Boris Ostrovsky wrote:
>
> On 3/1/22 5:53 AM, Christoph Hellwig wrote:
>> Allow to pass a remap argument to the swiotlb initialization functions
>> to handle the Xen/x86 remap case. ARM/ARM64 never did any remapping
>> from xen_swiotlb_fixup, so we don't even need that quirk.
>
>
> Any chance this patch could be split? Lots of things are happening here and
> it's somewhat hard to review. (Patch 7 too BTW but I think I managed to get
> through it)
What would be your preferred split?
>> diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
>> index e0def4b1c3181..2f2c468acb955 100644
>> --- a/arch/x86/kernel/pci-dma.c
>> +++ b/arch/x86/kernel/pci-dma.c
>> @@ -71,15 +71,12 @@ static inline void __init pci_swiotlb_detect(void)
>> #endif /* CONFIG_SWIOTLB */
>> #ifdef CONFIG_SWIOTLB_XEN
>> -static bool xen_swiotlb;
>> -
>> static void __init pci_xen_swiotlb_init(void)
>> {
>> if (!xen_initial_domain() && !x86_swiotlb_enable)
>> return;
>
>
> Now that there is a single call site for this routine I think this check can
> be dropped. We are only called here for xen_initial_domain()==true.
The callsite just checks xen_pv_domain() and itself is called
unconditionally during initialization.
Re: [PATCH 11/12] swiotlb: merge swiotlb-xen initialization into swiotlb
On 3/1/22 5:53 AM, Christoph Hellwig wrote:
Allow to pass a remap argument to the swiotlb initialization functions
to handle the Xen/x86 remap case. ARM/ARM64 never did any remapping
from xen_swiotlb_fixup, so we don't even need that quirk.
Any chance this patch could be split? Lots of things are happening here and
it's somewhat hard to review. (Patch 7 too BTW but I think I managed to get
through it)
diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
index e0def4b1c3181..2f2c468acb955 100644
--- a/arch/x86/kernel/pci-dma.c
+++ b/arch/x86/kernel/pci-dma.c
@@ -71,15 +71,12 @@ static inline void __init pci_swiotlb_detect(void)
#endif /* CONFIG_SWIOTLB */
#ifdef CONFIG_SWIOTLB_XEN
-static bool xen_swiotlb;
-
static void __init pci_xen_swiotlb_init(void)
{
if (!xen_initial_domain() && !x86_swiotlb_enable)
return;
Now that there is a single call site for this routine I think this check can be
dropped. We are only called here for xen_initial_domain()==true.
-boris
Re: [PATCH 11/12] swiotlb: merge swiotlb-xen initialization into swiotlb
On Fri, 4 Mar 2022, Christoph Hellwig wrote:
> On Thu, Mar 03, 2022 at 02:49:29PM -0800, Stefano Stabellini wrote:
> > On Thu, 3 Mar 2022, Christoph Hellwig wrote:
> > > On Wed, Mar 02, 2022 at 05:25:10PM -0800, Stefano Stabellini wrote:
> > > > Thinking more about it we actually need to drop the xen_initial_domain()
> > > > check otherwise some cases won't be functional (Dom0 not 1:1 mapped, or
> > > > DomU 1:1 mapped).
> > >
> > > Hmm, but that would be the case even before this series, right?
> >
> > Before this series we only have the xen_swiotlb_detect() check in
> > xen_mm_init, we don't have a second xen_initial_domain() check.
> >
> > The issue is that this series is adding one more xen_initial_domain()
> > check in xen_mm_init.
>
> In current mainline xen_mm_init calls xen_swiotlb_init unconditionally.
> But xen_swiotlb_init then calls xen_swiotlb_fixup after allocating
> the memory, which in turn calls xen_create_contiguous_region.
> xen_create_contiguous_region fails with -EINVAL for the
> !xen_initial_domain() and thus caues xen_swiotlb_fixup and
> xen_swiotlb_init to unwind and return -EINVAL.
>
> So as far as I can tell there is no change in behavior, but maybe I'm
> missing something subtle?
You are right.
The xen_initial_domain() check in xen_create_contiguous_region() is
wrong and we should get rid of it. It is a leftover from before the
xen_swiotlb_detect rework.
We could either remove it or change it into another xen_swiotlb_detect()
check.
Feel free to add the patch to your series or fold it with another patch
or rework it as you prefer. Thanks for spotting this!
---
arm/xen: don't check for xen_initial_domain() in xen_create_contiguous_region
It used to be that Linux enabled swiotlb-xen when running a dom0 on ARM.
Since f5079a9a2a31 "xen/arm: introduce XENFEAT_direct_mapped and
XENFEAT_not_direct_mapped", Linux detects whether to enable or disable
swiotlb-xen based on the new feature flags: XENFEAT_direct_mapped and
XENFEAT_not_direct_mapped.
However, there is still a leftover xen_initial_domain() check in
xen_create_contiguous_region. Remove the check as
xen_create_contiguous_region is only called by swiotlb-xen during
initialization. If xen_create_contiguous_region is called, we know Linux
is running 1:1 mapped so there is no need for additional checks.
Also update the in-code comment.
Signed-off-by: Stefano Stabellini
diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
index a7e54a087b80..28c207060253 100644
--- a/arch/arm/xen/mm.c
+++ b/arch/arm/xen/mm.c
@@ -122,10 +122,7 @@ int xen_create_contiguous_region(phys_addr_t pstart,
unsigned int order,
unsigned int address_bits,
dma_addr_t *dma_handle)
{
- if (!xen_initial_domain())
- return -EINVAL;
-
- /* we assume that dom0 is mapped 1:1 for now */
+ /* the domain is 1:1 mapped to use swiotlb-xen */
*dma_handle = pstart;
return 0;
}
Re: [PATCH 11/12] swiotlb: merge swiotlb-xen initialization into swiotlb
On Fri, Mar 04, 2022 at 03:18:23PM -0500, Boris Ostrovsky wrote: > This indeed allows dom0 to boot. Not sure I see where in the next patch this > would have been fixed? I thought it did, but it doesn't. In the meantime I've pushed out an updated branch with this folded in to: git://git.infradead.org/users/hch/misc.git swiotlb-init-cleanup > (BTW, just noticed in iommu_setup() you set this variable to 1. Should be > 'true') Thank, I'll fix this up.
Re: [PATCH 11/12] swiotlb: merge swiotlb-xen initialization into swiotlb
On 3/4/22 12:43 PM, Christoph Hellwig wrote: On Fri, Mar 04, 2022 at 12:36:17PM -0500, Boris Ostrovsky wrote: I bisected it to "x86: remove the IOMMU table infrastructure" but haven't actually looked at the code yet. That looks like the swiotlb buffer did not get initialized at all, but I can't really explain why. Can you stick in a printk and see if xen_swiotlb_init_early gets called at all? Actually, that's the only thing I did do so far and yes, it does get called. So, specifically for "x86: remove the IOMMU table infrastructure" I think we need the one-liner below so that swiotlb_exit doesn't get called for the Xen case. But that should have been fixed up by the next patch already. This indeed allows dom0 to boot. Not sure I see where in the next patch this would have been fixed? (BTW, just noticed in iommu_setup() you set this variable to 1. Should be 'true') -boris
Re: [PATCH 11/12] swiotlb: merge swiotlb-xen initialization into swiotlb
On Fri, Mar 04, 2022 at 12:36:17PM -0500, Boris Ostrovsky wrote: >>> I bisected it to "x86: remove the IOMMU table infrastructure" but haven't >>> actually looked at the code yet. >> That looks like the swiotlb buffer did not get initialized at all, but I >> can't really explain why. >> >> Can you stick in a printk and see if xen_swiotlb_init_early gets called >> at all? > > > > Actually, that's the only thing I did do so far and yes, it does get called. So, specifically for "x86: remove the IOMMU table infrastructure" I think we need the one-liner below so that swiotlb_exit doesn't get called for the Xen case. But that should have been fixed up by the next patch already. diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c index 2ac0ef9c2fb76..1173aa282ab27 100644 --- a/arch/x86/kernel/pci-dma.c +++ b/arch/x86/kernel/pci-dma.c @@ -70,7 +70,7 @@ static void __init pci_xen_swiotlb_init(void) if (!xen_initial_domain() && !x86_swiotlb_enable && swiotlb_force != SWIOTLB_FORCE) return; - x86_swiotlb_enable = false; + x86_swiotlb_enable = true; xen_swiotlb = true; xen_swiotlb_init_early(); dma_ops = _swiotlb_dma_ops;
Re: [PATCH 11/12] swiotlb: merge swiotlb-xen initialization into swiotlb
On 3/4/22 12:28 PM, Christoph Hellwig wrote: On Wed, Mar 02, 2022 at 08:15:03AM -0500, Boris Ostrovsky wrote: Not for me, I fail to boot with [ 52.202000] bnxt_en :31:00.0: swiotlb buffer is full (sz: 256 bytes), total 0 (slots), used 0 (slots) (this is iscsi root so I need the NIC). I bisected it to "x86: remove the IOMMU table infrastructure" but haven't actually looked at the code yet. That looks like the swiotlb buffer did not get initialized at all, but I can't really explain why. Can you stick in a printk and see if xen_swiotlb_init_early gets called at all? Actually, that's the only thing I did do so far and yes, it does get called. -boris
Re: [PATCH 11/12] swiotlb: merge swiotlb-xen initialization into swiotlb
On Wed, Mar 02, 2022 at 08:15:03AM -0500, Boris Ostrovsky wrote: > Not for me, I fail to boot with > > [ 52.202000] bnxt_en :31:00.0: swiotlb buffer is full (sz: 256 bytes), > total 0 (slots), used 0 (slots) > > (this is iscsi root so I need the NIC). > > > I bisected it to "x86: remove the IOMMU table infrastructure" but haven't > actually looked at the code yet. That looks like the swiotlb buffer did not get initialized at all, but I can't really explain why. Can you stick in a printk and see if xen_swiotlb_init_early gets called at all?
Re: [PATCH 11/12] swiotlb: merge swiotlb-xen initialization into swiotlb
On Thu, Mar 03, 2022 at 02:49:29PM -0800, Stefano Stabellini wrote: > On Thu, 3 Mar 2022, Christoph Hellwig wrote: > > On Wed, Mar 02, 2022 at 05:25:10PM -0800, Stefano Stabellini wrote: > > > Thinking more about it we actually need to drop the xen_initial_domain() > > > check otherwise some cases won't be functional (Dom0 not 1:1 mapped, or > > > DomU 1:1 mapped). > > > > Hmm, but that would be the case even before this series, right? > > Before this series we only have the xen_swiotlb_detect() check in > xen_mm_init, we don't have a second xen_initial_domain() check. > > The issue is that this series is adding one more xen_initial_domain() > check in xen_mm_init. In current mainline xen_mm_init calls xen_swiotlb_init unconditionally. But xen_swiotlb_init then calls xen_swiotlb_fixup after allocating the memory, which in turn calls xen_create_contiguous_region. xen_create_contiguous_region fails with -EINVAL for the !xen_initial_domain() and thus caues xen_swiotlb_fixup and xen_swiotlb_init to unwind and return -EINVAL. So as far as I can tell there is no change in behavior, but maybe I'm missing something subtle?
Re: [PATCH 11/12] swiotlb: merge swiotlb-xen initialization into swiotlb
On Thu, 3 Mar 2022, Christoph Hellwig wrote: > On Wed, Mar 02, 2022 at 05:25:10PM -0800, Stefano Stabellini wrote: > > Thinking more about it we actually need to drop the xen_initial_domain() > > check otherwise some cases won't be functional (Dom0 not 1:1 mapped, or > > DomU 1:1 mapped). > > Hmm, but that would be the case even before this series, right? Before this series we only have the xen_swiotlb_detect() check in xen_mm_init, we don't have a second xen_initial_domain() check. The issue is that this series is adding one more xen_initial_domain() check in xen_mm_init.
Re: [PATCH 11/12] swiotlb: merge swiotlb-xen initialization into swiotlb
On 3/3/22 5:57 AM, Christoph Hellwig wrote: On Wed, Mar 02, 2022 at 08:15:03AM -0500, Boris Ostrovsky wrote: Not for me, I fail to boot with [ 52.202000] bnxt_en :31:00.0: swiotlb buffer is full (sz: 256 bytes), total 0 (slots), used 0 (slots) (this is iscsi root so I need the NIC). I bisected it to "x86: remove the IOMMU table infrastructure" but haven't actually looked at the code yet. Thanks. Looks like the sizing is going wrong. Just to confirm, this is dom0 on x86 and no special command line options? Right. module2 /boot/vmlinuz-5.17.0-rc6swiotlb placeholder root=UUID=dbef1262-8c8a-43db-8055-7d9bec7bece0 ro crashkernel=auto LANG=en_US.UTF-8 rd.luks=0 rd.lvm=0 rd.md=0 rd.dm=0 netroot=iscsi:169.254.0.2:::1:iqn.2015-02.oracle.boot:uefi iscsi_param=node.session.timeo.replacement_timeout=6000 net.ifnames=1 nvme_core.shutdown_timeout=10 ipmi_si.tryacpi=0 ipmi_si.trydmi=0 ipmi_si.trydefaults=0 libiscsi.debug_libiscsi_eh=1 panic=20 nokaslr earlyprintk=xen console=hvc0 loglevel=8 4
Re: [PATCH 11/12] swiotlb: merge swiotlb-xen initialization into swiotlb
On Wed, Mar 02, 2022 at 05:25:10PM -0800, Stefano Stabellini wrote: > Thinking more about it we actually need to drop the xen_initial_domain() > check otherwise some cases won't be functional (Dom0 not 1:1 mapped, or > DomU 1:1 mapped). Hmm, but that would be the case even before this series, right?
Re: [PATCH 11/12] swiotlb: merge swiotlb-xen initialization into swiotlb
On Wed, Mar 02, 2022 at 08:15:03AM -0500, Boris Ostrovsky wrote: > Not for me, I fail to boot with > > [ 52.202000] bnxt_en :31:00.0: swiotlb buffer is full (sz: 256 bytes), > total 0 (slots), used 0 (slots) > > (this is iscsi root so I need the NIC). > > > I bisected it to "x86: remove the IOMMU table infrastructure" but haven't > actually looked at the code yet. Thanks. Looks like the sizing is going wrong. Just to confirm, this is dom0 on x86 and no special command line options?
Re: [PATCH 11/12] swiotlb: merge swiotlb-xen initialization into swiotlb
On Wed, 2 Mar 2022, Christoph Hellwig wrote:
> On Tue, Mar 01, 2022 at 06:55:47PM -0800, Stefano Stabellini wrote:
> > Unrelated to this specific patch series: now that I think about it, if
> > io_tlb_default_mem.nslabs is already allocated by the time xen_mm_init
> > is called, wouldn't we potentially have an issue with the GFP flags used
> > for the earlier allocation (e.g. GFP_DMA32 not used)? Maybe something
> > for another day.
>
> swiotlb_init allocates low memory from meblock, which is roughly
> equivalent to GFP_DMA allocations, so we'll be fine.
>
> > > @@ -143,10 +141,15 @@ static int __init xen_mm_init(void)
> > > if (!xen_swiotlb_detect())
> > > return 0;
> > >
> > > - rc = xen_swiotlb_init();
> > > /* we can work with the default swiotlb */
> > > - if (rc < 0 && rc != -EEXIST)
> > > - return rc;
> > > + if (!io_tlb_default_mem.nslabs) {
> > > + if (!xen_initial_domain())
> > > + return -EINVAL;
> >
> > I don't think we need this xen_initial_domain() check. It is all
> > already sorted out by the xen_swiotlb_detect() check above.
>
> Is it?
>
> static inline int xen_swiotlb_detect(void)
> {
> if (!xen_domain())
> return 0;
> if (xen_feature(XENFEAT_direct_mapped))
> return 1;
> /* legacy case */
> if (!xen_feature(XENFEAT_not_direct_mapped) && xen_initial_domain())
> return 1;
> return 0;
> }
It used to be that we had a
if (!xen_initial_domain())
return -EINVAL;
check in the initialization of swiotlb-xen on ARM. Then we replaced it
with the more sophisticated xen_swiotlb_detect().
The reason is that swiotlb-xen on ARM relies on Dom0 being 1:1 mapped
(guest physical addresses == physical addresses). Recent changes in Xen
allowed also DomUs to be 1:1 mapped. Changes still under discussion will
allow Dom0 not to be 1:1 mapped.
So, before all the Xen-side changes, knowing what was going to happen, I
introduced a clearer interface: XENFEAT_direct_mapped and
XENFEAT_not_direct_mapped tell us whether the guest (Linux) is 1:1
mapped or not. If it is 1:1 mapped then Linux can take advantage of
swiotlb-xen. Now xen_swiotlb_detect() returns true if Linux is 1:1
mapped.
Then of course there is the legacy case. That's taken care of by:
if (!xen_feature(XENFEAT_not_direct_mapped) && xen_initial_domain())
return 1;
The intention is to say that if
XENFEAT_direct_mapped/XENFEAT_not_direct_mapped are not present, then
use xen_initial_domain() like we did before.
So if xen_swiotlb_detect() returns true we know that Linux is either
dom0 (xen_initial_domain() == true) or we have very good reasons to
think we should initialize swiotlb-xen anyway
(xen_feature(XENFEAT_direct_mapped) == true).
> I think I'd keep it as-is for now, as my planned next step would be to
> fold xen-swiotlb into swiotlb entirely.
Thinking more about it we actually need to drop the xen_initial_domain()
check otherwise some cases won't be functional (Dom0 not 1:1 mapped, or
DomU 1:1 mapped).
Re: [PATCH 11/12] swiotlb: merge swiotlb-xen initialization into swiotlb
On 3/1/22 9:55 PM, Stefano Stabellini wrote: On Tue, 1 Mar 2022, Christoph Hellwig wrote: Allow to pass a remap argument to the swiotlb initialization functions to handle the Xen/x86 remap case. ARM/ARM64 never did any remapping from xen_swiotlb_fixup, so we don't even need that quirk. Aside from that the rest looks OK. Also, you can add my: Tested-by: Stefano Stabellini Not for me, I fail to boot with [ 52.202000] bnxt_en :31:00.0: swiotlb buffer is full (sz: 256 bytes), total 0 (slots), used 0 (slots) (this is iscsi root so I need the NIC). I bisected it to "x86: remove the IOMMU table infrastructure" but haven't actually looked at the code yet. -boris
Re: [PATCH 11/12] swiotlb: merge swiotlb-xen initialization into swiotlb
On 3/2/22 8:15 AM, Boris Ostrovsky wrote: On 3/1/22 9:55 PM, Stefano Stabellini wrote: On Tue, 1 Mar 2022, Christoph Hellwig wrote: Allow to pass a remap argument to the swiotlb initialization functions to handle the Xen/x86 remap case. ARM/ARM64 never did any remapping from xen_swiotlb_fixup, so we don't even need that quirk. Aside from that the rest looks OK. Also, you can add my: Tested-by: Stefano Stabellini Not for me, I fail to boot with [ 52.202000] bnxt_en :31:00.0: swiotlb buffer is full (sz: 256 bytes), total 0 (slots), used 0 (slots) (this is iscsi root so I need the NIC). I bisected it to "x86: remove the IOMMU table infrastructure" but haven't actually looked at the code yet. Again, this is as dom0. Baremetal is fine. -boris
Re: [PATCH 11/12] swiotlb: merge swiotlb-xen initialization into swiotlb
On Tue, Mar 01, 2022 at 06:55:47PM -0800, Stefano Stabellini wrote:
> Unrelated to this specific patch series: now that I think about it, if
> io_tlb_default_mem.nslabs is already allocated by the time xen_mm_init
> is called, wouldn't we potentially have an issue with the GFP flags used
> for the earlier allocation (e.g. GFP_DMA32 not used)? Maybe something
> for another day.
swiotlb_init allocates low memory from meblock, which is roughly
equivalent to GFP_DMA allocations, so we'll be fine.
> > @@ -143,10 +141,15 @@ static int __init xen_mm_init(void)
> > if (!xen_swiotlb_detect())
> > return 0;
> >
> > - rc = xen_swiotlb_init();
> > /* we can work with the default swiotlb */
> > - if (rc < 0 && rc != -EEXIST)
> > - return rc;
> > + if (!io_tlb_default_mem.nslabs) {
> > + if (!xen_initial_domain())
> > + return -EINVAL;
>
> I don't think we need this xen_initial_domain() check. It is all
> already sorted out by the xen_swiotlb_detect() check above.
Is it?
static inline int xen_swiotlb_detect(void)
{
if (!xen_domain())
return 0;
if (xen_feature(XENFEAT_direct_mapped))
return 1;
/* legacy case */
if (!xen_feature(XENFEAT_not_direct_mapped) && xen_initial_domain())
return 1;
return 0;
}
I think I'd keep it as-is for now, as my planned next step would be to
fold xen-swiotlb into swiotlb entirely.
Re: [PATCH 11/12] swiotlb: merge swiotlb-xen initialization into swiotlb
On Tue, 1 Mar 2022, Christoph Hellwig wrote:
> Allow to pass a remap argument to the swiotlb initialization functions
> to handle the Xen/x86 remap case. ARM/ARM64 never did any remapping
> from xen_swiotlb_fixup, so we don't even need that quirk.
>
> Signed-off-by: Christoph Hellwig
> ---
> arch/arm/xen/mm.c | 23 +++---
> arch/x86/include/asm/xen/page.h | 5 --
> arch/x86/kernel/pci-dma.c | 19 +++--
> arch/x86/pci/sta2x11-fixup.c| 2 +-
> drivers/xen/swiotlb-xen.c | 128 +---
> include/linux/swiotlb.h | 7 +-
> include/xen/arm/page.h | 1 -
> include/xen/swiotlb-xen.h | 8 +-
> kernel/dma/swiotlb.c| 120 +++---
> 9 files changed, 96 insertions(+), 217 deletions(-)
>
> diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
> index a7e54a087b802..58b40f87617d3 100644
> --- a/arch/arm/xen/mm.c
> +++ b/arch/arm/xen/mm.c
> @@ -23,22 +23,20 @@
> #include
> #include
>
> -unsigned long xen_get_swiotlb_free_pages(unsigned int order)
> +static gfp_t xen_swiotlb_gfp(void)
> {
> phys_addr_t base;
> - gfp_t flags = __GFP_NOWARN|__GFP_KSWAPD_RECLAIM;
> u64 i;
>
> for_each_mem_range(i, , NULL) {
> if (base < (phys_addr_t)0x) {
> if (IS_ENABLED(CONFIG_ZONE_DMA32))
> - flags |= __GFP_DMA32;
> - else
> - flags |= __GFP_DMA;
> - break;
> + return __GFP_DMA32;
> + return __GFP_DMA;
> }
> }
> - return __get_free_pages(flags, order);
> +
> + return GFP_KERNEL;
> }
Unrelated to this specific patch series: now that I think about it, if
io_tlb_default_mem.nslabs is already allocated by the time xen_mm_init
is called, wouldn't we potentially have an issue with the GFP flags used
for the earlier allocation (e.g. GFP_DMA32 not used)? Maybe something
for another day.
> static bool hypercall_cflush = false;
> @@ -143,10 +141,15 @@ static int __init xen_mm_init(void)
> if (!xen_swiotlb_detect())
> return 0;
>
> - rc = xen_swiotlb_init();
> /* we can work with the default swiotlb */
> - if (rc < 0 && rc != -EEXIST)
> - return rc;
> + if (!io_tlb_default_mem.nslabs) {
> + if (!xen_initial_domain())
> + return -EINVAL;
I don't think we need this xen_initial_domain() check. It is all
already sorted out by the xen_swiotlb_detect() check above.
Aside from that the rest looks OK. Also, you can add my:
Tested-by: Stefano Stabellini
> + rc = swiotlb_init_late(swiotlb_size_or_default(),
> +xen_swiotlb_gfp(), NULL);
> + if (rc < 0)
> + return rc;
> + }
>
> cflush.op = 0;
> cflush.a.dev_bus_addr = 0;
> diff --git a/arch/x86/include/asm/xen/page.h b/arch/x86/include/asm/xen/page.h
> index e989bc2269f54..1fc67df500145 100644
> --- a/arch/x86/include/asm/xen/page.h
> +++ b/arch/x86/include/asm/xen/page.h
> @@ -357,9 +357,4 @@ static inline bool xen_arch_need_swiotlb(struct device
> *dev,
> return false;
> }
>
> -static inline unsigned long xen_get_swiotlb_free_pages(unsigned int order)
> -{
> - return __get_free_pages(__GFP_NOWARN, order);
> -}
> -
> #endif /* _ASM_X86_XEN_PAGE_H */
> diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
> index e0def4b1c3181..2f2c468acb955 100644
> --- a/arch/x86/kernel/pci-dma.c
> +++ b/arch/x86/kernel/pci-dma.c
> @@ -71,15 +71,12 @@ static inline void __init pci_swiotlb_detect(void)
> #endif /* CONFIG_SWIOTLB */
>
> #ifdef CONFIG_SWIOTLB_XEN
> -static bool xen_swiotlb;
> -
> static void __init pci_xen_swiotlb_init(void)
> {
> if (!xen_initial_domain() && !x86_swiotlb_enable)
> return;
> x86_swiotlb_enable = false;
> - xen_swiotlb = true;
> - xen_swiotlb_init_early();
> + swiotlb_init_remap(true, x86_swiotlb_flags, xen_swiotlb_fixup);
> dma_ops = _swiotlb_dma_ops;
> if (IS_ENABLED(CONFIG_PCI))
> pci_request_acs();
> @@ -87,14 +84,16 @@ static void __init pci_xen_swiotlb_init(void)
>
> int pci_xen_swiotlb_init_late(void)
> {
> - int rc;
> -
> - if (xen_swiotlb)
> + if (dma_ops == _swiotlb_dma_ops)
> return 0;
>
> - rc = xen_swiotlb_init();
> - if (rc)
> - return rc;
> + /* we can work with the default swiotlb */
> + if (!io_tlb_default_mem.nslabs) {
> + int rc = swiotlb_init_late(swiotlb_size_or_default(),
> +GFP_KERNEL, xen_swiotlb_fixup);
> + if (rc < 0)
> + return rc;
> + }
>
> /* XXX: this switches the dma ops under live devices! */
> dma_ops = _swiotlb_dma_ops;
> diff --git
[PATCH 11/12] swiotlb: merge swiotlb-xen initialization into swiotlb
Allow to pass a remap argument to the swiotlb initialization functions
to handle the Xen/x86 remap case. ARM/ARM64 never did any remapping
from xen_swiotlb_fixup, so we don't even need that quirk.
Signed-off-by: Christoph Hellwig
---
arch/arm/xen/mm.c | 23 +++---
arch/x86/include/asm/xen/page.h | 5 --
arch/x86/kernel/pci-dma.c | 19 +++--
arch/x86/pci/sta2x11-fixup.c| 2 +-
drivers/xen/swiotlb-xen.c | 128 +---
include/linux/swiotlb.h | 7 +-
include/xen/arm/page.h | 1 -
include/xen/swiotlb-xen.h | 8 +-
kernel/dma/swiotlb.c| 120 +++---
9 files changed, 96 insertions(+), 217 deletions(-)
diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
index a7e54a087b802..58b40f87617d3 100644
--- a/arch/arm/xen/mm.c
+++ b/arch/arm/xen/mm.c
@@ -23,22 +23,20 @@
#include
#include
-unsigned long xen_get_swiotlb_free_pages(unsigned int order)
+static gfp_t xen_swiotlb_gfp(void)
{
phys_addr_t base;
- gfp_t flags = __GFP_NOWARN|__GFP_KSWAPD_RECLAIM;
u64 i;
for_each_mem_range(i, , NULL) {
if (base < (phys_addr_t)0x) {
if (IS_ENABLED(CONFIG_ZONE_DMA32))
- flags |= __GFP_DMA32;
- else
- flags |= __GFP_DMA;
- break;
+ return __GFP_DMA32;
+ return __GFP_DMA;
}
}
- return __get_free_pages(flags, order);
+
+ return GFP_KERNEL;
}
static bool hypercall_cflush = false;
@@ -143,10 +141,15 @@ static int __init xen_mm_init(void)
if (!xen_swiotlb_detect())
return 0;
- rc = xen_swiotlb_init();
/* we can work with the default swiotlb */
- if (rc < 0 && rc != -EEXIST)
- return rc;
+ if (!io_tlb_default_mem.nslabs) {
+ if (!xen_initial_domain())
+ return -EINVAL;
+ rc = swiotlb_init_late(swiotlb_size_or_default(),
+ xen_swiotlb_gfp(), NULL);
+ if (rc < 0)
+ return rc;
+ }
cflush.op = 0;
cflush.a.dev_bus_addr = 0;
diff --git a/arch/x86/include/asm/xen/page.h b/arch/x86/include/asm/xen/page.h
index e989bc2269f54..1fc67df500145 100644
--- a/arch/x86/include/asm/xen/page.h
+++ b/arch/x86/include/asm/xen/page.h
@@ -357,9 +357,4 @@ static inline bool xen_arch_need_swiotlb(struct device *dev,
return false;
}
-static inline unsigned long xen_get_swiotlb_free_pages(unsigned int order)
-{
- return __get_free_pages(__GFP_NOWARN, order);
-}
-
#endif /* _ASM_X86_XEN_PAGE_H */
diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
index e0def4b1c3181..2f2c468acb955 100644
--- a/arch/x86/kernel/pci-dma.c
+++ b/arch/x86/kernel/pci-dma.c
@@ -71,15 +71,12 @@ static inline void __init pci_swiotlb_detect(void)
#endif /* CONFIG_SWIOTLB */
#ifdef CONFIG_SWIOTLB_XEN
-static bool xen_swiotlb;
-
static void __init pci_xen_swiotlb_init(void)
{
if (!xen_initial_domain() && !x86_swiotlb_enable)
return;
x86_swiotlb_enable = false;
- xen_swiotlb = true;
- xen_swiotlb_init_early();
+ swiotlb_init_remap(true, x86_swiotlb_flags, xen_swiotlb_fixup);
dma_ops = _swiotlb_dma_ops;
if (IS_ENABLED(CONFIG_PCI))
pci_request_acs();
@@ -87,14 +84,16 @@ static void __init pci_xen_swiotlb_init(void)
int pci_xen_swiotlb_init_late(void)
{
- int rc;
-
- if (xen_swiotlb)
+ if (dma_ops == _swiotlb_dma_ops)
return 0;
- rc = xen_swiotlb_init();
- if (rc)
- return rc;
+ /* we can work with the default swiotlb */
+ if (!io_tlb_default_mem.nslabs) {
+ int rc = swiotlb_init_late(swiotlb_size_or_default(),
+ GFP_KERNEL, xen_swiotlb_fixup);
+ if (rc < 0)
+ return rc;
+ }
/* XXX: this switches the dma ops under live devices! */
dma_ops = _swiotlb_dma_ops;
diff --git a/arch/x86/pci/sta2x11-fixup.c b/arch/x86/pci/sta2x11-fixup.c
index c7e6faf59a861..7368afc039987 100644
--- a/arch/x86/pci/sta2x11-fixup.c
+++ b/arch/x86/pci/sta2x11-fixup.c
@@ -57,7 +57,7 @@ static void sta2x11_new_instance(struct pci_dev *pdev)
int size = STA2X11_SWIOTLB_SIZE;
/* First instance: register your own swiotlb area */
dev_info(>dev, "Using SWIOTLB (size %i)\n", size);
- if (swiotlb_init_late(size, GFP_DMA))
+ if (swiotlb_init_late(size, GFP_DMA, NULL))
dev_emerg(>dev, "init swiotlb failed\n");
}
list_add(>list, _instance_list);
diff --git a/drivers/xen/swiotlb-xen.c
