Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
On Tue, Sep 08, 2020 at 11:43:45AM +0200, Christoph Hellwig wrote: > And because I like replying to myself so much, here is a link to the > version with the arm cleanup patch applied. Unlike the previous two > attempts this has at least survived very basic sanity testing: > > http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/dma-ranges.2 Tested-by: Nathan Chancellor Thank you for fixing this up quickly! ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
On Tue, Sep 8, 2020 at 5:43 AM Christoph Hellwig wrote: > > And because I like replying to myself so much, here is a link to the > version with the arm cleanup patch applied. Unlike the previous two > attempts this has at least survived very basic sanity testing: > > http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/dma-ranges.2 > Tested-by: Jim Quinlan > Note that we'll still need to sort out the arm/keystone warnings from > the original patch. Do we have anyone on the CC list who knows that > platform a little better to figure out if the ifdef solution would work? ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
On Tue, Sep 08, 2020 at 01:20:56PM +0200, Nicolas Saenz Julienne wrote: > On Tue, 2020-09-08 at 11:43 +0200, Christoph Hellwig wrote: > > And because I like replying to myself so much, here is a link to the > > version with the arm cleanup patch applied. Unlike the previous two > > attempts this has at least survived very basic sanity testing: > > > > http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/dma-ranges.2 > > > > Note that we'll still need to sort out the arm/keystone warnings from > > the original patch. Do we have anyone on the CC list who knows that > > platform a little better to figure out if the ifdef solution would work? > > Had to do the following to boot without errors: I've folded in. That being said the whole RPi4 setup confuses the heck out of me. I wonder what thing was smoked to come up with it.. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
On Tue, 2020-09-08 at 11:43 +0200, Christoph Hellwig wrote:
> And because I like replying to myself so much, here is a link to the
> version with the arm cleanup patch applied. Unlike the previous two
> attempts this has at least survived very basic sanity testing:
>
> http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/dma-ranges.2
>
> Note that we'll still need to sort out the arm/keystone warnings from
> the original patch. Do we have anyone on the CC list who knows that
> platform a little better to figure out if the ifdef solution would work?
Had to do the following to boot without errors:
diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
index ef61a33c47bc..7dd88a0b6d0b 100644
--- a/include/linux/dma-direct.h
+++ b/include/linux/dma-direct.h
@@ -97,6 +97,9 @@ static inline bool dma_capable(struct device *dev, dma_addr_t
addr, size_t size,
{
dma_addr_t end = addr + size - 1;
+ if (addr == DMA_MAPPING_ERROR)
+ return false;
+
if (!dev->dma_mask)
return false;
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 90f1ecb6baaf..25809703a5bf 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -71,7 +71,12 @@ static gfp_t dma_direct_optimal_gfp_mask(struct device *dev,
u64 dma_mask,
static bool dma_coherent_ok(struct device *dev, phys_addr_t phys, size_t size)
{
- return phys_to_dma_direct(dev, phys) + size - 1 <=
+ dma_addr_t dma_addr = phys_to_dma_direct(dev, phys);
+
+ if (dma_addr == DMA_MAPPING_ERROR)
+ return false;
+
+ return dma_addr + size - 1 <=
min_not_zero(dev->coherent_dma_mask,
dev->bus_dma_limit);
}
Regards,
Nicolas
signature.asc
Description: This is a digitally signed message part
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
And because I like replying to myself so much, here is a link to the version with the arm cleanup patch applied. Unlike the previous two attempts this has at least survived very basic sanity testing: http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/dma-ranges.2 Note that we'll still need to sort out the arm/keystone warnings from the original patch. Do we have anyone on the CC list who knows that platform a little better to figure out if the ifdef solution would work? ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
On Tue, Sep 08, 2020 at 09:29:35AM +0200, Christoph Hellwig wrote:
> FYI, this is what I'd do relative to the patch on the dma-ranges
> branch. In fact realizing this makes me want to refactor things a bit
> so that the new code can entirely live in the dma-direct code, but please
> test this first:
And of course this isn't going to work for arm devices without any
range, so let's try this instead:
diff --git a/arch/arm/include/asm/dma-mapping.h
b/arch/arm/include/asm/dma-mapping.h
index c21893f683b585..e913e04d2be8b9 100644
--- a/arch/arm/include/asm/dma-mapping.h
+++ b/arch/arm/include/asm/dma-mapping.h
@@ -35,21 +35,20 @@ static inline const struct dma_map_ops
*get_arch_dma_ops(struct bus_type *bus)
#ifndef __arch_pfn_to_dma
static inline dma_addr_t pfn_to_dma(struct device *dev, unsigned long pfn)
{
- if (dev) {
- phys_addr_t paddr = PFN_PHYS(pfn);
-
- pfn -= PFN_DOWN(dma_offset_from_phys_addr(dev, paddr));
- }
- return (dma_addr_t)__pfn_to_bus(pfn);
+ if (!dev)
+ return (dma_addr_t)__pfn_to_bus(pfn);
+ if (dev->dma_range_map)
+ return translate_phys_to_dma(dev, PFN_PHYS(pfn));
+ return (dma_addr_t)PFN_PHYS(pfn);
}
static inline unsigned long dma_to_pfn(struct device *dev, dma_addr_t addr)
{
- unsigned long pfn = __bus_to_pfn(addr);
-
- if (dev)
- pfn += PFN_DOWN(dma_offset_from_dma_addr(dev, addr));
- return pfn;
+ if (!dev)
+ return __bus_to_pfn(addr);
+ if (dev->dma_range_map)
+ return PFN_DOWN(translate_dma_to_phys(dev, addr));
+ return PFN_DOWN(addr);
}
static inline void *dma_to_virt(struct device *dev, dma_addr_t addr)
diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
index 7831ca5b1b5dd6..e624171c4962ad 100644
--- a/include/linux/dma-direct.h
+++ b/include/linux/dma-direct.h
@@ -19,12 +19,16 @@ extern unsigned int zone_dma_bits;
#else
static inline dma_addr_t __phys_to_dma(struct device *dev, phys_addr_t paddr)
{
- return (dma_addr_t)paddr - dma_offset_from_phys_addr(dev, paddr);
+ if (dev->dma_range_map)
+ return (dma_addr_t)paddr - translate_phys_to_dma(dev, paddr);
+ return (dma_addr_t)paddr;
}
-static inline phys_addr_t __dma_to_phys(struct device *dev, dma_addr_t
dev_addr)
+static inline phys_addr_t __dma_to_phys(struct device *dev, dma_addr_t
dma_addr)
{
- return (phys_addr_t)dev_addr + dma_offset_from_dma_addr(dev, dev_addr);
+ if (dev->dma_range_map)
+ return translate_dma_to_phys(dev, dma_addr);
+ return (phys_addr_t)dma_addr;
}
#endif /* !CONFIG_ARCH_HAS_PHYS_TO_DMA */
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 4c4646761afee4..3b1ceebb6f2ad5 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -199,29 +199,28 @@ struct bus_dma_region {
};
#ifdef CONFIG_HAS_DMA
-static inline u64 dma_offset_from_dma_addr(struct device *dev,
- dma_addr_t dma_addr)
+static inline dma_addr_t translate_phys_to_dma(struct device *dev,
+ phys_addr_t paddr)
{
- const struct bus_dma_region *m = dev->dma_range_map;
+ const struct bus_dma_region *m;
- if (m)
- for (; m->size; m++)
- if (dma_addr >= m->dma_start &&
- dma_addr - m->dma_start < m->size)
- return m->offset;
- return 0;
+ for (m = dev->dma_range_map; m->size; m++)
+ if (paddr >= m->cpu_start && paddr - m->cpu_start < m->size)
+ return (dma_addr_t)paddr - m->offset;
+
+ /* make sure dma_capable fails when no translation is available */
+ return DMA_MAPPING_ERROR;
}
-static inline u64 dma_offset_from_phys_addr(struct device *dev,
- phys_addr_t paddr)
+static inline phys_addr_t translate_dma_to_phys(struct device *dev,
+ dma_addr_t dma_addr)
{
- const struct bus_dma_region *m = dev->dma_range_map;
+ const struct bus_dma_region *m;
+
+ for (m = dev->dma_range_map; m->size; m++)
+ if (dma_addr >= m->dma_start && dma_addr - m->dma_start <
m->size)
+ return (phys_addr_t)dma_addr + m->offset;
- if (m)
- for (; m->size; m++)
- if (paddr >= m->cpu_start &&
- paddr - m->cpu_start < m->size)
- return m->offset;
return 0;
}
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
FYI, this is what I'd do relative to the patch on the dma-ranges
branch. In fact realizing this makes me want to refactor things a bit
so that the new code can entirely live in the dma-direct code, but please
test this first:
diff --git a/arch/arm/include/asm/dma-mapping.h
b/arch/arm/include/asm/dma-mapping.h
index c21893f683b585..072fc42349874d 100644
--- a/arch/arm/include/asm/dma-mapping.h
+++ b/arch/arm/include/asm/dma-mapping.h
@@ -35,21 +35,16 @@ static inline const struct dma_map_ops
*get_arch_dma_ops(struct bus_type *bus)
#ifndef __arch_pfn_to_dma
static inline dma_addr_t pfn_to_dma(struct device *dev, unsigned long pfn)
{
- if (dev) {
- phys_addr_t paddr = PFN_PHYS(pfn);
-
- pfn -= PFN_DOWN(dma_offset_from_phys_addr(dev, paddr));
- }
- return (dma_addr_t)__pfn_to_bus(pfn);
+ if (!dev)
+ return (dma_addr_t)__pfn_to_bus(pfn);
+ return translate_phys_to_dma(dev, PFN_PHYS(pfn));
}
static inline unsigned long dma_to_pfn(struct device *dev, dma_addr_t addr)
{
- unsigned long pfn = __bus_to_pfn(addr);
-
- if (dev)
- pfn += PFN_DOWN(dma_offset_from_dma_addr(dev, addr));
- return pfn;
+ if (!dev)
+ return __bus_to_pfn(addr);
+ return PFN_DOWN(translate_dma_to_phys(dev, addr));
}
static inline void *dma_to_virt(struct device *dev, dma_addr_t addr)
diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
index 7831ca5b1b5dd6..e624171c4962ad 100644
--- a/include/linux/dma-direct.h
+++ b/include/linux/dma-direct.h
@@ -19,12 +19,16 @@ extern unsigned int zone_dma_bits;
#else
static inline dma_addr_t __phys_to_dma(struct device *dev, phys_addr_t paddr)
{
- return (dma_addr_t)paddr - dma_offset_from_phys_addr(dev, paddr);
+ if (dev->dma_range_map)
+ return (dma_addr_t)paddr - translate_phys_to_dma(dev, paddr);
+ return (dma_addr_t)paddr;
}
-static inline phys_addr_t __dma_to_phys(struct device *dev, dma_addr_t
dev_addr)
+static inline phys_addr_t __dma_to_phys(struct device *dev, dma_addr_t
dma_addr)
{
- return (phys_addr_t)dev_addr + dma_offset_from_dma_addr(dev, dev_addr);
+ if (dev->dma_range_map)
+ return translate_dma_to_phys(dev, dma_addr);
+ return (phys_addr_t)dma_addr;
}
#endif /* !CONFIG_ARCH_HAS_PHYS_TO_DMA */
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 4c4646761afee4..3b1ceebb6f2ad5 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -199,29 +199,28 @@ struct bus_dma_region {
};
#ifdef CONFIG_HAS_DMA
-static inline u64 dma_offset_from_dma_addr(struct device *dev,
- dma_addr_t dma_addr)
+static inline dma_addr_t translate_phys_to_dma(struct device *dev,
+ phys_addr_t paddr)
{
- const struct bus_dma_region *m = dev->dma_range_map;
+ const struct bus_dma_region *m;
- if (m)
- for (; m->size; m++)
- if (dma_addr >= m->dma_start &&
- dma_addr - m->dma_start < m->size)
- return m->offset;
- return 0;
+ for (m = dev->dma_range_map; m->size; m++)
+ if (paddr >= m->cpu_start && paddr - m->cpu_start < m->size)
+ return (dma_addr_t)paddr - m->offset;
+
+ /* make sure dma_capable fails when no translation is available */
+ return DMA_MAPPING_ERROR;
}
-static inline u64 dma_offset_from_phys_addr(struct device *dev,
- phys_addr_t paddr)
+static inline phys_addr_t translate_dma_to_phys(struct device *dev,
+ dma_addr_t dma_addr)
{
- const struct bus_dma_region *m = dev->dma_range_map;
+ const struct bus_dma_region *m;
+
+ for (m = dev->dma_range_map; m->size; m++)
+ if (dma_addr >= m->dma_start && dma_addr - m->dma_start <
m->size)
+ return (phys_addr_t)dma_addr + m->offset;
- if (m)
- for (; m->size; m++)
- if (paddr >= m->cpu_start &&
- paddr - m->cpu_start < m->size)
- return m->offset;
return 0;
}
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
On Mon, Sep 07, 2020 at 08:19:43PM +0200, Nicolas Saenz Julienne wrote: > Indeed, that's why I wasn't all that happy with my solution. > > As an alternative, how about returning '-dev->bus_dma_limit' instead of 0? > It's > always 0 for the devices without bus_dma_regions, and, I think, an always > unattainable offset for devices that do (I tested it on RPi4 with the 30bit > limitd mmc controller and it seems to work alright). No, bus_dma_limit can be set independent of offsets. We use it e.g. to limit old x86 VIA PCI bridges to 32-bit addressing. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
On Mon, Sep 07, 2020 at 01:40:46PM -0400, Jim Quinlan wrote: > Thanks for looking into this. The concern I have with your solution > is that returning an arbitrarily large offset might overlap with an > improbable but valid usage. AFAIK there is nothing that disallows > mapping a device to anywhere within the 64bit range of PCIe space, > including up to say 0x. > > As an alternative perhaps changing dma_capable() so that if the > dma_range_map is present then it validates that both ends of the > prospective DMA region get "hits" on one of the offset regions in the > map. Christoph, if you are okay with this I can quickly post a patch. We use a dma_addr of all-Fs as an error code, see the definition of DMA_MAPPING_ERROR. The rationale of why we think it is safe: We don't do single byte mappings, so the last address of the address space can't effectively be used for anything. So I think it would be a good fit here. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
On Mon, Sep 07, 2020 at 05:18:59PM +0200, Nicolas Saenz Julienne wrote: > Hi Christoph, a small fix to your fixes: Thanks, folded into the patch on the dma-ranges branch. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
On Mon, 2020-09-07 at 13:40 -0400, Jim Quinlan wrote:
> On Mon, Sep 7, 2020 at 11:01 AM Nicolas Saenz Julienne
> wrote:
> > >
> > > Hi Nicolas,
> > >
> > > Can you please help us out here? It appears that your commit
> >
> > It's dma_offset_from_dma_addr() that's causing trouble. It goes over all the
> > dma regions and, if no region matches the phys address range, it returns 0.
> > This isn't acceptable as is. In the past, we used to pass the offset
> > nonetheless, which would make the phys address grow out of the dma memory
> > area
> > and fail the dma_capable() test.
> >
> > For example, RPi4 devices attached to the old interconnect see phys [0x0
> > 0x3fff] at [0xc000 0x]. So say you're trying to convert
> > physical address 0xfa00, you'll get 0 from dma_offset_from_phys_addr()
> > (since your dma range only covers the first GB) to then test if 0xfa00
> > is
> > dma_capable(), which it is, but for the wrong reasons. Causing DMA issues
> > further down the line.
> >
> > I don't have a proper suggestion on how to solve this but here's the
> > solution I
> > used:
> >
> > diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> > index 4c4646761afe..40fe3c97f2be 100644
> > --- a/include/linux/dma-mapping.h
> > +++ b/include/linux/dma-mapping.h
> > @@ -217,11 +217,19 @@ static inline u64 dma_offset_from_phys_addr(struct
> > device *dev,
> > {
> > const struct bus_dma_region *m = dev->dma_range_map;
> >
> > - if (m)
> > + if (m) {
> > for (; m->size; m++)
> > if (paddr >= m->cpu_start &&
> > paddr - m->cpu_start < m->size)
> > return m->offset;
> > +
> > + /*
> > +* The physical address doesn't fit any of the DMA regions,
> > +* return an impossible to fulfill offset.
> > +*/
> > + return -(1ULL << 46);
> > + }
> > +
> > return 0;
> > }
> Hi Nicolas,
>
> Thanks for looking into this. The concern I have with your solution
> is that returning an arbitrarily large offset might overlap with an
> improbable but valid usage. AFAIK there is nothing that disallows
> mapping a device to anywhere within the 64bit range of PCIe space,
> including up to say 0x.
Indeed, that's why I wasn't all that happy with my solution.
As an alternative, how about returning '-dev->bus_dma_limit' instead of 0? It's
always 0 for the devices without bus_dma_regions, and, I think, an always
unattainable offset for devices that do (I tested it on RPi4 with the 30bit
limitd mmc controller and it seems to work alright).
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -222,7 +222,8 @@ static inline u64 dma_offset_from_phys_addr(struct device
*dev,
if (paddr >= m->cpu_start &&
paddr - m->cpu_start < m->size)
return m->offset;
- return 0;
+
+ return -dev->bus_dma_limit;
}
> As an alternative perhaps changing dma_capable() so that if the
> dma_range_map is present then it validates that both ends of the
> prospective DMA region get "hits" on one of the offset regions in the
> map. Christoph, if you are okay with this I can quickly post a patch.
IIUC, by the time you enter dma_capable() you already converted the physical
address to DMA address and the damage is done.
Regards,
Nicolas
signature.asc
Description: This is a digitally signed message part
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
On Mon, Sep 7, 2020 at 11:01 AM Nicolas Saenz Julienne
wrote:
>
> Hi Jim, sorry I'm a little late to the party, but was on vacation.
>
> On Thu, 2020-09-03 at 13:32 -0400, Jim Quinlan wrote:
> > On Wed, Sep 2, 2020 at 8:52 PM Nathan Chancellor
> > wrote:
> > > On Wed, Sep 02, 2020 at 05:36:29PM -0700, Florian Fainelli wrote:
> > > >
> > > > On 9/2/2020 3:38 PM, Nathan Chancellor wrote:
> > > > [snip]
> > > > > > Hello Nathan,
> > > > > >
> > > > > > Can you tell me how much memory your RPI has and if all of it is
> > > > >
> > > > > This is the 4GB version.
> > > > >
> > > > > > accessible by the PCIe device? Could you also please include the
> > > > > > DTS
> > > > > > of the PCIe node? IIRC, the RPI firmware does some mangling of the
> > > > > > PCIe DT before Linux boots -- could you describe what is going on
> > > > > > there?
> > > > >
> > > > > Unfortunately, I am not familiar with how to get this information. If
> > > > > you could provide some instructions for how to do so, I am more than
> > > > > happy to. I am not very knowleagable about the inner working of the
> > > > > Pi,
> > > > > I mainly use it as a test platform for making sure that LLVM does not
> > > > > cause problems on real devices.
> > > >
> > > > Can you bring the dtc application to your Pi root filesystem, and if
> > > > so, can
> > > > you run the following:
> > > >
> > > > dtc -I fs -O dtb /proc/device-tree -f > /tmp/device.dtb
> > >
> > > Sure, the result is attached.
> > >
> > > > or cat /sys/firmware/fdt > device.dtb
> > > >
> > > > and attach the resulting file?
> > > >
> > > > > > Finally, can you attach the text of the full boot log?
> > > > >
> > > > > I have attached a working and broken boot log. Thank you for the quick
> > > > > response!
> > > >
> > > > Is it possible for you to rebuild your kernel with CONFIG_MMC_DEBUG by
> > > > any
> > > > chance?
> > >
> > > Of course. A new log is attached with the debug output from that config.
> >
> > Hi Nicolas,
> >
> > Can you please help us out here? It appears that your commit
>
> It's dma_offset_from_dma_addr() that's causing trouble. It goes over all the
> dma regions and, if no region matches the phys address range, it returns 0.
> This isn't acceptable as is. In the past, we used to pass the offset
> nonetheless, which would make the phys address grow out of the dma memory area
> and fail the dma_capable() test.
>
> For example, RPi4 devices attached to the old interconnect see phys [0x0
> 0x3fff] at [0xc000 0x]. So say you're trying to convert
> physical address 0xfa00, you'll get 0 from dma_offset_from_phys_addr()
> (since your dma range only covers the first GB) to then test if 0xfa00 is
> dma_capable(), which it is, but for the wrong reasons. Causing DMA issues
> further down the line.
>
> I don't have a proper suggestion on how to solve this but here's the solution
> I
> used:
>
> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index 4c4646761afe..40fe3c97f2be 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -217,11 +217,19 @@ static inline u64 dma_offset_from_phys_addr(struct
> device *dev,
> {
> const struct bus_dma_region *m = dev->dma_range_map;
>
> - if (m)
> + if (m) {
> for (; m->size; m++)
> if (paddr >= m->cpu_start &&
> paddr - m->cpu_start < m->size)
> return m->offset;
> +
> + /*
> +* The physical address doesn't fit any of the DMA regions,
> +* return an impossible to fulfill offset.
> +*/
> + return -(1ULL << 46);
> + }
> +
> return 0;
> }
Hi Nicolas,
Thanks for looking into this. The concern I have with your solution
is that returning an arbitrarily large offset might overlap with an
improbable but valid usage. AFAIK there is nothing that disallows
mapping a device to anywhere within the 64bit range of PCIe space,
including up to say 0x.
As an alternative perhaps changing dma_capable() so that if the
dma_range_map is present then it validates that both ends of the
prospective DMA region get "hits" on one of the offset regions in the
map. Christoph, if you are okay with this I can quickly post a patch.
Regards,
Jim Quinlan
Broadcom STB
>
> I didn't catch this on my previous tests as I was using a newer bcm2711 SoC
> revision which expanded emmc2's DMA capabilities (can do 32 bit DMA as opposed
> to 30 bit).
>
> Regards,
> Nicolas
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
Hi Christoph, a small fix to your fixes: On Tue, 2020-09-01 at 10:24 +0200, Christoph Hellwig wrote: > I've applied this to the dma-mapping tree. > > I had to resolve a conflict in drivers/of/address.c with a recent > mainline commit. I also applied the minor tweaks Andy pointed out > plus a few more style changes. A real change is that I changed the > prototype for dma_copy_dma_range_map to require less boilerplate code. diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c index 0b5f8d62f251..4cd012817af6 100644 --- a/drivers/usb/core/usb.c +++ b/drivers/usb/core/usb.c @@ -610,7 +610,7 @@ struct usb_device *usb_alloc_dev(struct usb_device *parent, * mask for the entire HCD, so don't do that. */ dev->dev.dma_mask = bus->sysdev->dma_mask; - if (!dma_copy_dma_range_map(>dev, bus->sysdev)) + if (dma_copy_dma_range_map(>dev, bus->sysdev)) dev_err(>dev, "failed to copy DMA map\n"); set_dev_node(>dev, dev_to_node(bus->sysdev)); dev->state = USB_STATE_ATTACHED; Regards, Nicolas signature.asc Description: This is a digitally signed message part ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
Hi Jim, sorry I'm a little late to the party, but was on vacation.
On Thu, 2020-09-03 at 13:32 -0400, Jim Quinlan wrote:
> On Wed, Sep 2, 2020 at 8:52 PM Nathan Chancellor
> wrote:
> > On Wed, Sep 02, 2020 at 05:36:29PM -0700, Florian Fainelli wrote:
> > >
> > > On 9/2/2020 3:38 PM, Nathan Chancellor wrote:
> > > [snip]
> > > > > Hello Nathan,
> > > > >
> > > > > Can you tell me how much memory your RPI has and if all of it is
> > > >
> > > > This is the 4GB version.
> > > >
> > > > > accessible by the PCIe device? Could you also please include the DTS
> > > > > of the PCIe node? IIRC, the RPI firmware does some mangling of the
> > > > > PCIe DT before Linux boots -- could you describe what is going on
> > > > > there?
> > > >
> > > > Unfortunately, I am not familiar with how to get this information. If
> > > > you could provide some instructions for how to do so, I am more than
> > > > happy to. I am not very knowleagable about the inner working of the Pi,
> > > > I mainly use it as a test platform for making sure that LLVM does not
> > > > cause problems on real devices.
> > >
> > > Can you bring the dtc application to your Pi root filesystem, and if so,
> > > can
> > > you run the following:
> > >
> > > dtc -I fs -O dtb /proc/device-tree -f > /tmp/device.dtb
> >
> > Sure, the result is attached.
> >
> > > or cat /sys/firmware/fdt > device.dtb
> > >
> > > and attach the resulting file?
> > >
> > > > > Finally, can you attach the text of the full boot log?
> > > >
> > > > I have attached a working and broken boot log. Thank you for the quick
> > > > response!
> > >
> > > Is it possible for you to rebuild your kernel with CONFIG_MMC_DEBUG by any
> > > chance?
> >
> > Of course. A new log is attached with the debug output from that config.
>
> Hi Nicolas,
>
> Can you please help us out here? It appears that your commit
It's dma_offset_from_dma_addr() that's causing trouble. It goes over all the
dma regions and, if no region matches the phys address range, it returns 0.
This isn't acceptable as is. In the past, we used to pass the offset
nonetheless, which would make the phys address grow out of the dma memory area
and fail the dma_capable() test.
For example, RPi4 devices attached to the old interconnect see phys [0x0
0x3fff] at [0xc000 0x]. So say you're trying to convert
physical address 0xfa00, you'll get 0 from dma_offset_from_phys_addr()
(since your dma range only covers the first GB) to then test if 0xfa00 is
dma_capable(), which it is, but for the wrong reasons. Causing DMA issues
further down the line.
I don't have a proper suggestion on how to solve this but here's the solution I
used:
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 4c4646761afe..40fe3c97f2be 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -217,11 +217,19 @@ static inline u64 dma_offset_from_phys_addr(struct device
*dev,
{
const struct bus_dma_region *m = dev->dma_range_map;
- if (m)
+ if (m) {
for (; m->size; m++)
if (paddr >= m->cpu_start &&
paddr - m->cpu_start < m->size)
return m->offset;
+
+ /*
+* The physical address doesn't fit any of the DMA regions,
+* return an impossible to fulfill offset.
+*/
+ return -(1ULL << 46);
+ }
+
return 0;
}
I didn't catch this on my previous tests as I was using a newer bcm2711 SoC
revision which expanded emmc2's DMA capabilities (can do 32 bit DMA as opposed
to 30 bit).
Regards,
Nicolas
signature.asc
Description: This is a digitally signed message part
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
On Wed, Sep 2, 2020 at 8:52 PM Nathan Chancellor wrote: > > On Wed, Sep 02, 2020 at 05:36:29PM -0700, Florian Fainelli wrote: > > > > > > On 9/2/2020 3:38 PM, Nathan Chancellor wrote: > > [snip] > > > > Hello Nathan, > > > > > > > > Can you tell me how much memory your RPI has and if all of it is > > > > > > This is the 4GB version. > > > > > > > accessible by the PCIe device? Could you also please include the DTS > > > > of the PCIe node? IIRC, the RPI firmware does some mangling of the > > > > PCIe DT before Linux boots -- could you describe what is going on > > > > there? > > > > > > Unfortunately, I am not familiar with how to get this information. If > > > you could provide some instructions for how to do so, I am more than > > > happy to. I am not very knowleagable about the inner working of the Pi, > > > I mainly use it as a test platform for making sure that LLVM does not > > > cause problems on real devices. > > > > Can you bring the dtc application to your Pi root filesystem, and if so, can > > you run the following: > > > > dtc -I fs -O dtb /proc/device-tree -f > /tmp/device.dtb > > Sure, the result is attached. > > > or cat /sys/firmware/fdt > device.dtb > > > > and attach the resulting file? > > > > > > > > > Finally, can you attach the text of the full boot log? > > > > > > I have attached a working and broken boot log. Thank you for the quick > > > response! > > > > Is it possible for you to rebuild your kernel with CONFIG_MMC_DEBUG by any > > chance? > > Of course. A new log is attached with the debug output from that config. Hi Nicolas, Can you please help us out here? It appears that your commit 3d2cbb644836 "ARM: dts: bcm2711: Move emmc2 into its own bus" added the following line to the bcm2711.dtsi file: + dma-ranges = <0x0 0xc000 0x0 0x 0x4000>; and for some reason the eMMC is not operating properly w/ my commit.. Regardless, the only difference that my commit should make is to enforce the bounds of the dma_window (whereas the previous code adds/subs the pfn_offset everywhere). Thanks, Jim Quinlan PS If you would like to talk, let me know and we can make arrangements. > > > > I have a suspicion that this part of the DTS for the bcm2711.dtsi platform > > is at fault: > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/bcm2711.dtsi#n264 > > > > and the resulting dma-ranges parsing is just not working for reasons to be > > determined. > > -- > > Florian > > Let me know if you need anything else out of me. > > Cheers, > Nathan ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
FYI, I've moved it off for-next and into its own dma-ranges branch for now until we sort out the regressions. I still hope to bring it back soon. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
On Wed, Sep 02, 2020 at 05:36:29PM -0700, Florian Fainelli wrote: > > > On 9/2/2020 3:38 PM, Nathan Chancellor wrote: > [snip] > > > Hello Nathan, > > > > > > Can you tell me how much memory your RPI has and if all of it is > > > > This is the 4GB version. > > > > > accessible by the PCIe device? Could you also please include the DTS > > > of the PCIe node? IIRC, the RPI firmware does some mangling of the > > > PCIe DT before Linux boots -- could you describe what is going on > > > there? > > > > Unfortunately, I am not familiar with how to get this information. If > > you could provide some instructions for how to do so, I am more than > > happy to. I am not very knowleagable about the inner working of the Pi, > > I mainly use it as a test platform for making sure that LLVM does not > > cause problems on real devices. > > Can you bring the dtc application to your Pi root filesystem, and if so, can > you run the following: > > dtc -I fs -O dtb /proc/device-tree -f > /tmp/device.dtb Sure, the result is attached. > or cat /sys/firmware/fdt > device.dtb > > and attach the resulting file? > > > > > > Finally, can you attach the text of the full boot log? > > > > I have attached a working and broken boot log. Thank you for the quick > > response! > > Is it possible for you to rebuild your kernel with CONFIG_MMC_DEBUG by any > chance? Of course. A new log is attached with the debug output from that config. > I have a suspicion that this part of the DTS for the bcm2711.dtsi platform > is at fault: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/bcm2711.dtsi#n264 > > and the resulting dma-ranges parsing is just not working for reasons to be > determined. > -- > Florian Let me know if you need anything else out of me. Cheers, Nathan device.dtb Description: Binary data [0.00] Booting Linux on physical CPU 0x00 [0x410fd083] [0.00] Linux version 5.9.0-rc3-next-20200902-dirty (nathan@ubuntu-n2-xlarge-x86) (ClangBuiltLinux clang version 12.0.0 (https://github.com/llvm/llvm-project.git b21ddded8f04fee925bbf9e6458347104b5b99eb), LLD 12.0.0 (https://github.com/llvm/llvm-project.git b21ddded8f04fee925bbf9e6458347104b5b99eb)) #1 SMP PREEMPT Wed Sep 2 17:41:42 MST 2020 [0.00] Machine model: Raspberry Pi 4 Model B Rev 1.2 [0.00] efi: UEFI not found. [0.00] Reserved memory: created CMA memory pool at 0x3740, size 64 MiB [0.00] OF: reserved mem: initialized node linux,cma, compatible id shared-dma-pool [0.00] NUMA: No NUMA configuration found [0.00] NUMA: Faking a node at [mem 0x-0xfbff] [0.00] NUMA: NODE_DATA [mem 0xfb81f100-0xfb820fff] [0.00] Zone ranges: [0.00] DMA [mem 0x-0x3fff] [0.00] DMA32[mem 0x4000-0xfbff] [0.00] Normal empty [0.00] Movable zone start for each node [0.00] Early memory node ranges [0.00] node 0: [mem 0x-0x3b3f] [0.00] node 0: [mem 0x4000-0xfbff] [0.00] Initmem setup node 0 [mem 0x-0xfbff] [0.00] percpu: Embedded 23 pages/cpu s54168 r8192 d31848 u94208 [0.00] Detected PIPT I-cache on CPU0 [0.00] CPU features: detected: EL2 vector hardening [0.00] CPU features: kernel page table isolation forced ON by KASLR [0.00] CPU features: detected: Kernel page table isolation (KPTI) [0.00] ARM_SMCCC_ARCH_WORKAROUND_1 missing from firmware [0.00] CPU features: detected: ARM errata 1165522, 1319367, or 1530923 [0.00] Built 1 zonelists, mobility grouping on. Total pages: 996912 [0.00] Policy zone: DMA32 [0.00] Kernel command line: dma.dmachans=0x71f5 bcm2709.boardrev=0xc03112 bcm2709.serial=0xb78d398 bcm2709.uart_clock=4800 bcm2709.disk_led_gpio=42 bcm2709.disk_led_active_low=0 smsc95xx.macaddr=DC:A6:32:60:6C:87 vc_mem.mem_base=0x3ec0 vc_mem.mem_size=0x4000 console=ttyS1,115200 console=tty1 root=PARTUUID=45a8dd8a-02 rootfstype=ext4 elevator=deadline fsck.repair=yes rootwait plymouth.ignore-serial-consoles [0.00] Kernel parameter elevator= does not have any effect anymore. [0.00] Please use sysfs to set IO scheduler for individual devices. [0.00] Dentry cache hash table entries: 524288 (order: 10, 4194304 bytes, linear) [0.00] Inode-cache hash table entries: 262144 (order: 9, 2097152 bytes, linear) [0.00] mem auto-init: stack:off, heap alloc:off, heap free:off [0.00] software IO TLB: mapped [mem 0x3340-0x3740] (64MB) [0.00] Memory: 3812260K/4050944K available (14524K kernel code, 2636K rwdata, 7540K rodata, 1600K init, 615K bss, 173148K reserved, 65536K cma-reserved) [0.00] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=4,
Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
On 9/2/2020 3:38 PM, Nathan Chancellor wrote: [snip] Hello Nathan, Can you tell me how much memory your RPI has and if all of it is This is the 4GB version. accessible by the PCIe device? Could you also please include the DTS of the PCIe node? IIRC, the RPI firmware does some mangling of the PCIe DT before Linux boots -- could you describe what is going on there? Unfortunately, I am not familiar with how to get this information. If you could provide some instructions for how to do so, I am more than happy to. I am not very knowleagable about the inner working of the Pi, I mainly use it as a test platform for making sure that LLVM does not cause problems on real devices. Can you bring the dtc application to your Pi root filesystem, and if so, can you run the following: dtc -I fs -O dtb /proc/device-tree -f > /tmp/device.dtb or cat /sys/firmware/fdt > device.dtb and attach the resulting file? Finally, can you attach the text of the full boot log? I have attached a working and broken boot log. Thank you for the quick response! Is it possible for you to rebuild your kernel with CONFIG_MMC_DEBUG by any chance? I have a suspicion that this part of the DTS for the bcm2711.dtsi platform is at fault: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/bcm2711.dtsi#n264 and the resulting dma-ranges parsing is just not working for reasons to be determined. -- Florian ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
On Wed, Sep 02, 2020 at 06:11:08PM -0400, Jim Quinlan wrote: > On Wed, Sep 2, 2020 at 5:53 PM Nathan Chancellor > wrote: > > > > On Mon, Aug 24, 2020 at 03:30:20PM -0400, Jim Quinlan wrote: > > > The new field 'dma_range_map' in struct device is used to facilitate the > > > use of single or multiple offsets between mapping regions of cpu addrs and > > > dma addrs. It subsumes the role of "dev->dma_pfn_offset" which was only > > > capable of holding a single uniform offset and had no region bounds > > > checking. > > > > > > The function of_dma_get_range() has been modified so that it takes a > > > single > > > argument -- the device node -- and returns a map, NULL, or an error code. > > > The map is an array that holds the information regarding the DMA regions. > > > Each range entry contains the address offset, the cpu_start address, the > > > dma_start address, and the size of the region. > > > > > > of_dma_configure() is the typical manner to set range offsets but there > > > are > > > a number of ad hoc assignments to "dev->dma_pfn_offset" in the kernel > > > driver code. These cases now invoke the function > > > dma_attach_offset_range(dev, cpu_addr, dma_addr, size). > > > > > > Signed-off-by: Jim Quinlan > > > --- > > > arch/arm/include/asm/dma-mapping.h| 10 +-- > > > arch/arm/mach-keystone/keystone.c | 17 +++-- > > > arch/sh/drivers/pci/pcie-sh7786.c | 9 +-- > > > arch/x86/pci/sta2x11-fixup.c | 7 +- > > > drivers/acpi/arm64/iort.c | 5 +- > > > drivers/base/core.c | 2 + > > > drivers/gpu/drm/sun4i/sun4i_backend.c | 5 +- > > > drivers/iommu/io-pgtable-arm.c| 2 +- > > > .../platform/sunxi/sun4i-csi/sun4i_csi.c | 5 +- > > > .../platform/sunxi/sun6i-csi/sun6i_csi.c | 4 +- > > > drivers/of/address.c | 72 +-- > > > drivers/of/device.c | 43 ++- > > > drivers/of/of_private.h | 10 +-- > > > drivers/of/unittest.c | 34 ++--- > > > drivers/remoteproc/remoteproc_core.c | 8 ++- > > > .../staging/media/sunxi/cedrus/cedrus_hw.c| 7 +- > > > drivers/usb/core/message.c| 9 ++- > > > drivers/usb/core/usb.c| 7 +- > > > include/linux/device.h| 4 +- > > > include/linux/dma-direct.h| 8 +-- > > > include/linux/dma-mapping.h | 36 ++ > > > kernel/dma/coherent.c | 10 +-- > > > kernel/dma/mapping.c | 66 + > > > 23 files changed, 265 insertions(+), 115 deletions(-) > > > > Apologies if this has already been reported or is known but this commit > > is now in next-20200902 and it causes my Raspberry Pi 4 to no longer > > make it to userspace, instead spewing mmc errors: > > > > That commit causes my Raspberry Pi 4 to no longer make it to userspace, > > instead spewing mmc errors: > > > > [0.00] Booting Linux on physical CPU 0x00 [0x410fd083] > > [0.00] Linux version 5.9.0-rc3-4-geef520b232c6-dirty > > (nathan@ubuntu-n2-xlarge-x86) (ClangBuiltLinux clang version 12.0.0 > > (https://github.com/llvm/llvm-project.git > > b21ddded8f04fee925bbf9e6458347104b5b99eb), LLD 12.0.0 > > (https://github.com/llvm/llvm-project.git > > b21ddded8f04fee925bbf9e6458347104b5b99eb)) #1 SMP PREEMPT Wed Sep 2 > > 13:48:49 MST 2020 > > [0.00] Machine model: Raspberry Pi 4 Model B Rev 1.2 > > ... > > [1.459752] raspberrypi-firmware soc:firmware: Attached to firmware from > > 2020-08-24T18:50:56 > > [1.57] dwc2 fe98.usb: supply vusb_d not found, using dummy > > regulator > > [1.507454] dwc2 fe98.usb: supply vusb_a not found, using dummy > > regulator > > [1.615547] dwc2 fe98.usb: EPs: 8, dedicated fifos, 4080 entries in > > SPRAM > > [1.627537] sdhci-iproc fe30.sdhci: allocated mmc-pwrseq > > [1.665497] mmc0: SDHCI controller on fe30.sdhci [fe30.sdhci] > > using PIO > > [1.690601] mmc0: queuing unknown CIS tuple 0x80 (2 bytes) > > [1.697892] mmc0: queuing unknown CIS tuple 0x80 (3 bytes) > > [1.705173] mmc0: queuing unknown CIS tuple 0x80 (3 bytes) > > [1.713788] mmc0: queuing unknown CIS tuple 0x80 (7 bytes) > > [1.721228] mmc0: queuing unknown CIS tuple 0x80 (3 bytes) > > [1.732062] mmc1: SDHCI controller on fe34.emmc2 [fe34.emmc2] > > using ADMA > > [1.741828] ALSA device list: > > [1.744885] No soundcards found. > > [1.748540] Waiting for root device PARTUUID=45a8dd8a-02... > > [1.788865] random: fast init done > > [1.793489] mmc1: unrecognised SCR structure version 4 > > [1.798814] mmc1: error -22 whilst initialising SD card > > [1.813969] mmc0: new high speed SDIO card at address 0001 > > [1.883178] mmc1:
Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
On Wed, Sep 2, 2020 at 5:53 PM Nathan Chancellor wrote: > > On Mon, Aug 24, 2020 at 03:30:20PM -0400, Jim Quinlan wrote: > > The new field 'dma_range_map' in struct device is used to facilitate the > > use of single or multiple offsets between mapping regions of cpu addrs and > > dma addrs. It subsumes the role of "dev->dma_pfn_offset" which was only > > capable of holding a single uniform offset and had no region bounds > > checking. > > > > The function of_dma_get_range() has been modified so that it takes a single > > argument -- the device node -- and returns a map, NULL, or an error code. > > The map is an array that holds the information regarding the DMA regions. > > Each range entry contains the address offset, the cpu_start address, the > > dma_start address, and the size of the region. > > > > of_dma_configure() is the typical manner to set range offsets but there are > > a number of ad hoc assignments to "dev->dma_pfn_offset" in the kernel > > driver code. These cases now invoke the function > > dma_attach_offset_range(dev, cpu_addr, dma_addr, size). > > > > Signed-off-by: Jim Quinlan > > --- > > arch/arm/include/asm/dma-mapping.h| 10 +-- > > arch/arm/mach-keystone/keystone.c | 17 +++-- > > arch/sh/drivers/pci/pcie-sh7786.c | 9 +-- > > arch/x86/pci/sta2x11-fixup.c | 7 +- > > drivers/acpi/arm64/iort.c | 5 +- > > drivers/base/core.c | 2 + > > drivers/gpu/drm/sun4i/sun4i_backend.c | 5 +- > > drivers/iommu/io-pgtable-arm.c| 2 +- > > .../platform/sunxi/sun4i-csi/sun4i_csi.c | 5 +- > > .../platform/sunxi/sun6i-csi/sun6i_csi.c | 4 +- > > drivers/of/address.c | 72 +-- > > drivers/of/device.c | 43 ++- > > drivers/of/of_private.h | 10 +-- > > drivers/of/unittest.c | 34 ++--- > > drivers/remoteproc/remoteproc_core.c | 8 ++- > > .../staging/media/sunxi/cedrus/cedrus_hw.c| 7 +- > > drivers/usb/core/message.c| 9 ++- > > drivers/usb/core/usb.c| 7 +- > > include/linux/device.h| 4 +- > > include/linux/dma-direct.h| 8 +-- > > include/linux/dma-mapping.h | 36 ++ > > kernel/dma/coherent.c | 10 +-- > > kernel/dma/mapping.c | 66 + > > 23 files changed, 265 insertions(+), 115 deletions(-) > > Apologies if this has already been reported or is known but this commit > is now in next-20200902 and it causes my Raspberry Pi 4 to no longer > make it to userspace, instead spewing mmc errors: > > That commit causes my Raspberry Pi 4 to no longer make it to userspace, > instead spewing mmc errors: > > [0.00] Booting Linux on physical CPU 0x00 [0x410fd083] > [0.00] Linux version 5.9.0-rc3-4-geef520b232c6-dirty > (nathan@ubuntu-n2-xlarge-x86) (ClangBuiltLinux clang version 12.0.0 > (https://github.com/llvm/llvm-project.git > b21ddded8f04fee925bbf9e6458347104b5b99eb), LLD 12.0.0 > (https://github.com/llvm/llvm-project.git > b21ddded8f04fee925bbf9e6458347104b5b99eb)) #1 SMP PREEMPT Wed Sep 2 13:48:49 > MST 2020 > [0.00] Machine model: Raspberry Pi 4 Model B Rev 1.2 > ... > [1.459752] raspberrypi-firmware soc:firmware: Attached to firmware from > 2020-08-24T18:50:56 > [1.57] dwc2 fe98.usb: supply vusb_d not found, using dummy > regulator > [1.507454] dwc2 fe98.usb: supply vusb_a not found, using dummy > regulator > [1.615547] dwc2 fe98.usb: EPs: 8, dedicated fifos, 4080 entries in > SPRAM > [1.627537] sdhci-iproc fe30.sdhci: allocated mmc-pwrseq > [1.665497] mmc0: SDHCI controller on fe30.sdhci [fe30.sdhci] > using PIO > [1.690601] mmc0: queuing unknown CIS tuple 0x80 (2 bytes) > [1.697892] mmc0: queuing unknown CIS tuple 0x80 (3 bytes) > [1.705173] mmc0: queuing unknown CIS tuple 0x80 (3 bytes) > [1.713788] mmc0: queuing unknown CIS tuple 0x80 (7 bytes) > [1.721228] mmc0: queuing unknown CIS tuple 0x80 (3 bytes) > [1.732062] mmc1: SDHCI controller on fe34.emmc2 [fe34.emmc2] > using ADMA > [1.741828] ALSA device list: > [1.744885] No soundcards found. > [1.748540] Waiting for root device PARTUUID=45a8dd8a-02... > [1.788865] random: fast init done > [1.793489] mmc1: unrecognised SCR structure version 4 > [1.798814] mmc1: error -22 whilst initialising SD card > [1.813969] mmc0: new high speed SDIO card at address 0001 > [1.883178] mmc1: unrecognised SCR structure version 2 > [1.888423] mmc1: error -22 whilst initialising SD card > [1.964069] mmc1: unrecognised SCR structure version 4 > [1.969314] mmc1: error -22 whilst initialising SD card > [2.061225] mmc1: unrecognised
Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
On Mon, Aug 24, 2020 at 03:30:20PM -0400, Jim Quinlan wrote: > The new field 'dma_range_map' in struct device is used to facilitate the > use of single or multiple offsets between mapping regions of cpu addrs and > dma addrs. It subsumes the role of "dev->dma_pfn_offset" which was only > capable of holding a single uniform offset and had no region bounds > checking. > > The function of_dma_get_range() has been modified so that it takes a single > argument -- the device node -- and returns a map, NULL, or an error code. > The map is an array that holds the information regarding the DMA regions. > Each range entry contains the address offset, the cpu_start address, the > dma_start address, and the size of the region. > > of_dma_configure() is the typical manner to set range offsets but there are > a number of ad hoc assignments to "dev->dma_pfn_offset" in the kernel > driver code. These cases now invoke the function > dma_attach_offset_range(dev, cpu_addr, dma_addr, size). > > Signed-off-by: Jim Quinlan > --- > arch/arm/include/asm/dma-mapping.h| 10 +-- > arch/arm/mach-keystone/keystone.c | 17 +++-- > arch/sh/drivers/pci/pcie-sh7786.c | 9 +-- > arch/x86/pci/sta2x11-fixup.c | 7 +- > drivers/acpi/arm64/iort.c | 5 +- > drivers/base/core.c | 2 + > drivers/gpu/drm/sun4i/sun4i_backend.c | 5 +- > drivers/iommu/io-pgtable-arm.c| 2 +- > .../platform/sunxi/sun4i-csi/sun4i_csi.c | 5 +- > .../platform/sunxi/sun6i-csi/sun6i_csi.c | 4 +- > drivers/of/address.c | 72 +-- > drivers/of/device.c | 43 ++- > drivers/of/of_private.h | 10 +-- > drivers/of/unittest.c | 34 ++--- > drivers/remoteproc/remoteproc_core.c | 8 ++- > .../staging/media/sunxi/cedrus/cedrus_hw.c| 7 +- > drivers/usb/core/message.c| 9 ++- > drivers/usb/core/usb.c| 7 +- > include/linux/device.h| 4 +- > include/linux/dma-direct.h| 8 +-- > include/linux/dma-mapping.h | 36 ++ > kernel/dma/coherent.c | 10 +-- > kernel/dma/mapping.c | 66 + > 23 files changed, 265 insertions(+), 115 deletions(-) Apologies if this has already been reported or is known but this commit is now in next-20200902 and it causes my Raspberry Pi 4 to no longer make it to userspace, instead spewing mmc errors: That commit causes my Raspberry Pi 4 to no longer make it to userspace, instead spewing mmc errors: [0.00] Booting Linux on physical CPU 0x00 [0x410fd083] [0.00] Linux version 5.9.0-rc3-4-geef520b232c6-dirty (nathan@ubuntu-n2-xlarge-x86) (ClangBuiltLinux clang version 12.0.0 (https://github.com/llvm/llvm-project.git b21ddded8f04fee925bbf9e6458347104b5b99eb), LLD 12.0.0 (https://github.com/llvm/llvm-project.git b21ddded8f04fee925bbf9e6458347104b5b99eb)) #1 SMP PREEMPT Wed Sep 2 13:48:49 MST 2020 [0.00] Machine model: Raspberry Pi 4 Model B Rev 1.2 ... [1.459752] raspberrypi-firmware soc:firmware: Attached to firmware from 2020-08-24T18:50:56 [1.57] dwc2 fe98.usb: supply vusb_d not found, using dummy regulator [1.507454] dwc2 fe98.usb: supply vusb_a not found, using dummy regulator [1.615547] dwc2 fe98.usb: EPs: 8, dedicated fifos, 4080 entries in SPRAM [1.627537] sdhci-iproc fe30.sdhci: allocated mmc-pwrseq [1.665497] mmc0: SDHCI controller on fe30.sdhci [fe30.sdhci] using PIO [1.690601] mmc0: queuing unknown CIS tuple 0x80 (2 bytes) [1.697892] mmc0: queuing unknown CIS tuple 0x80 (3 bytes) [1.705173] mmc0: queuing unknown CIS tuple 0x80 (3 bytes) [1.713788] mmc0: queuing unknown CIS tuple 0x80 (7 bytes) [1.721228] mmc0: queuing unknown CIS tuple 0x80 (3 bytes) [1.732062] mmc1: SDHCI controller on fe34.emmc2 [fe34.emmc2] using ADMA [1.741828] ALSA device list: [1.744885] No soundcards found. [1.748540] Waiting for root device PARTUUID=45a8dd8a-02... [1.788865] random: fast init done [1.793489] mmc1: unrecognised SCR structure version 4 [1.798814] mmc1: error -22 whilst initialising SD card [1.813969] mmc0: new high speed SDIO card at address 0001 [1.883178] mmc1: unrecognised SCR structure version 2 [1.888423] mmc1: error -22 whilst initialising SD card [1.964069] mmc1: unrecognised SCR structure version 4 [1.969314] mmc1: error -22 whilst initialising SD card [2.061225] mmc1: unrecognised SCR structure version 4 [2.066470] mmc1: error -22 whilst initialising SD card [3.160476] mmc1: unrecognised SCR structure version 4 [3.165718] mmc1: error -22 whilst initialising SD card This is what it looks like before that
Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
On Tue, Sep 1, 2020 at 4:24 AM Christoph Hellwig wrote: > > I've applied this to the dma-mapping tree. > > I had to resolve a conflict in drivers/of/address.c with a recent > mainline commit. I also applied the minor tweaks Andy pointed out > plus a few more style changes. A real change is that I changed the > prototype for dma_copy_dma_range_map to require less boilerplate code. > > The result is here: > > > http://git.infradead.org/users/hch/dma-mapping.git/commitdiff/eef520b232c60e74eb8b33a5a7863ad8f2b4a5c7 > > please double check that everyting works as expected. Tested-by: Jim Quinlan Thanks Christoph Jim > > I can cut a stable branch with this if you need it for other trees, but > I'd like to wait a few days to see if there is any fallout first. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
I've applied this to the dma-mapping tree. I had to resolve a conflict in drivers/of/address.c with a recent mainline commit. I also applied the minor tweaks Andy pointed out plus a few more style changes. A real change is that I changed the prototype for dma_copy_dma_range_map to require less boilerplate code. The result is here: http://git.infradead.org/users/hch/dma-mapping.git/commitdiff/eef520b232c60e74eb8b33a5a7863ad8f2b4a5c7 please double check that everyting works as expected. I can cut a stable branch with this if you need it for other trees, but I'd like to wait a few days to see if there is any fallout first. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
Hi Andy,
On Tue, Aug 25, 2020 at 5:54 AM Andy Shevchenko
wrote:
>
> On Mon, Aug 24, 2020 at 03:30:20PM -0400, Jim Quinlan wrote:
> > The new field 'dma_range_map' in struct device is used to facilitate the
> > use of single or multiple offsets between mapping regions of cpu addrs and
> > dma addrs. It subsumes the role of "dev->dma_pfn_offset" which was only
> > capable of holding a single uniform offset and had no region bounds
> > checking.
> >
> > The function of_dma_get_range() has been modified so that it takes a single
> > argument -- the device node -- and returns a map, NULL, or an error code.
> > The map is an array that holds the information regarding the DMA regions.
> > Each range entry contains the address offset, the cpu_start address, the
> > dma_start address, and the size of the region.
> >
> > of_dma_configure() is the typical manner to set range offsets but there are
> > a number of ad hoc assignments to "dev->dma_pfn_offset" in the kernel
> > driver code. These cases now invoke the function
> > dma_attach_offset_range(dev, cpu_addr, dma_addr, size).
>
> ...
>
> > + /*
> > + * Record all info in the generic DMA ranges array for struct device.
> > + */
> > + *map = r;
> > + for_each_of_range(, ) {
> > + pr_debug("dma_addr(%llx) cpu_addr(%llx) size(%llx)\n",
> > + range.bus_addr, range.cpu_addr, range.size);
> > + r->cpu_start = range.cpu_addr;
> > + r->dma_start = range.bus_addr;
> > + r->size = range.size;
>
> > + r->offset = (u64)range.cpu_addr - (u64)range.bus_addr;
>
> What's the point in explicit castings to the same type?
No point. If I have to send out another version I will fix this.
>
> > + r++;
> > + }
>
> ...
>
> > + phys_addr_t paddr;
> > + dma_addr_t dma_addr;
> > + struct device dev_bogus;
>
> > unittest(paddr == expect_paddr,
> > - "of_dma_get_range wrong phys addr (%llx) on node
> > %pOF", paddr, np);
> > + "of_dma_get_range: wrong phys addr %llx (expecting
> > %llx) on node %pOF\n",
> > + (u64)paddr, expect_paddr, np);
>
> %llx -> %pap
This was intentional -- I'm aware of %pap and %pad. The problem is
that %pa[pd] print out a zero-filled 16 character number regardless
of what the number is. For example, 1 is "0x0001",
whereas using %llx yields "1".
>
> > unittest(dma_addr == expect_dma_addr,
> > - "of_dma_get_range wrong DMA addr (%llx) on node
> > %pOF", dma_addr, np);
> > + "of_dma_get_range: wrong DMA addr %llx (expecting
> > %llx) on node %pOF\n",
> > + (u64)dma_addr, expect_dma_addr, np);
>
> %llx -> %pad
>
> ...
>
> > + if (mem->use_dev_dma_pfn_offset) {
> > + u64 base_addr = PFN_PHYS((u64)mem->pfn_base);
>
> Do we need explicit casting here?
I don't think it is needed. However, the "(u64)" is useless though
since the macro recasts it to a phys_addr_t.
If there is another version of this submission I will change this.
>
> > +
> > + return base_addr - dma_offset_from_phys_addr(dev, base_addr);
> > + }
>
> ...
>
> > +int dma_set_offset_range(struct device *dev, phys_addr_t cpu_start,
> > + dma_addr_t dma_start, u64 size)
> > +{
> > + struct bus_dma_region *map;
> > + u64 offset = (u64)cpu_start - (u64)dma_start;
> > +
> > + if (dev->dma_range_map) {
> > + dev_err(dev, "attempt to add DMA range to existing map\n");
> > + return -EINVAL;
> > + }
>
> Wouldn't be better to do an assignment of offset here?
IIRC this was what Christoph requested. It has actually gone back and
forth over the versions of this submission.
>
> > + if (!offset)
> > + return 0;
> > +
> > + map = kcalloc(2, sizeof(*map), GFP_KERNEL);
> > + if (!map)
> > + return -ENOMEM;
> > + map[0].cpu_start = cpu_start;
> > + map[0].dma_start = dma_start;
> > + map[0].offset = offset;
> > + map[0].size = size;
> > + dev->dma_range_map = map;
> > +
> > + return 0;
> > +}
>
> --
> With Best Regards,
> Andy Shevchenko
Thanks again,
Jim
>
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
On Mon, Aug 24, 2020 at 03:30:20PM -0400, Jim Quinlan wrote:
> The new field 'dma_range_map' in struct device is used to facilitate the
> use of single or multiple offsets between mapping regions of cpu addrs and
> dma addrs. It subsumes the role of "dev->dma_pfn_offset" which was only
> capable of holding a single uniform offset and had no region bounds
> checking.
>
> The function of_dma_get_range() has been modified so that it takes a single
> argument -- the device node -- and returns a map, NULL, or an error code.
> The map is an array that holds the information regarding the DMA regions.
> Each range entry contains the address offset, the cpu_start address, the
> dma_start address, and the size of the region.
>
> of_dma_configure() is the typical manner to set range offsets but there are
> a number of ad hoc assignments to "dev->dma_pfn_offset" in the kernel
> driver code. These cases now invoke the function
> dma_attach_offset_range(dev, cpu_addr, dma_addr, size).
...
> + /*
> + * Record all info in the generic DMA ranges array for struct device.
> + */
> + *map = r;
> + for_each_of_range(, ) {
> + pr_debug("dma_addr(%llx) cpu_addr(%llx) size(%llx)\n",
> + range.bus_addr, range.cpu_addr, range.size);
> + r->cpu_start = range.cpu_addr;
> + r->dma_start = range.bus_addr;
> + r->size = range.size;
> + r->offset = (u64)range.cpu_addr - (u64)range.bus_addr;
What's the point in explicit castings to the same type?
> + r++;
> + }
...
> + phys_addr_t paddr;
> + dma_addr_t dma_addr;
> + struct device dev_bogus;
> unittest(paddr == expect_paddr,
> - "of_dma_get_range wrong phys addr (%llx) on node
> %pOF", paddr, np);
> + "of_dma_get_range: wrong phys addr %llx (expecting
> %llx) on node %pOF\n",
> + (u64)paddr, expect_paddr, np);
%llx -> %pap
> unittest(dma_addr == expect_dma_addr,
> - "of_dma_get_range wrong DMA addr (%llx) on node %pOF",
> dma_addr, np);
> + "of_dma_get_range: wrong DMA addr %llx (expecting
> %llx) on node %pOF\n",
> + (u64)dma_addr, expect_dma_addr, np);
%llx -> %pad
...
> + if (mem->use_dev_dma_pfn_offset) {
> + u64 base_addr = PFN_PHYS((u64)mem->pfn_base);
Do we need explicit casting here?
> +
> + return base_addr - dma_offset_from_phys_addr(dev, base_addr);
> + }
...
> +int dma_set_offset_range(struct device *dev, phys_addr_t cpu_start,
> + dma_addr_t dma_start, u64 size)
> +{
> + struct bus_dma_region *map;
> + u64 offset = (u64)cpu_start - (u64)dma_start;
> +
> + if (dev->dma_range_map) {
> + dev_err(dev, "attempt to add DMA range to existing map\n");
> + return -EINVAL;
> + }
Wouldn't be better to do an assignment of offset here?
> + if (!offset)
> + return 0;
> +
> + map = kcalloc(2, sizeof(*map), GFP_KERNEL);
> + if (!map)
> + return -ENOMEM;
> + map[0].cpu_start = cpu_start;
> + map[0].dma_start = dma_start;
> + map[0].offset = offset;
> + map[0].size = size;
> + dev->dma_range_map = map;
> +
> + return 0;
> +}
--
With Best Regards,
Andy Shevchenko
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
