* [PATCH RFC 5/5] pinctrl: mvebu: add simple regmap based pinctrl implementation
From: Russell King @ 2017-01-13 11:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170113110240.GA29164@n2100.armlinux.org.uk>
Add a simple regmap based pinctrl implementation for mvebu, for syscon
based regmap drivers.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/pinctrl/mvebu/pinctrl-mvebu.c | 60 +++++++++++++++++++++++++++++++++++
drivers/pinctrl/mvebu/pinctrl-mvebu.h | 16 +++++++++-
2 files changed, 75 insertions(+), 1 deletion(-)
diff --git a/drivers/pinctrl/mvebu/pinctrl-mvebu.c b/drivers/pinctrl/mvebu/pinctrl-mvebu.c
index 9c6736d0595d..2f9a4e3e1ff1 100644
--- a/drivers/pinctrl/mvebu/pinctrl-mvebu.c
+++ b/drivers/pinctrl/mvebu/pinctrl-mvebu.c
@@ -23,6 +23,8 @@
#include <linux/pinctrl/pinconf.h>
#include <linux/pinctrl/pinctrl.h>
#include <linux/pinctrl/pinmux.h>
+#include <linux/mfd/syscon.h>
+#include <linux/regmap.h>
#include "pinctrl-mvebu.h"
@@ -788,3 +790,61 @@ int mvebu_pinctrl_simple_mmio_probe(struct platform_device *pdev)
return mvebu_pinctrl_probe(pdev);
}
+
+int mvebu_regmap_mpp_ctrl_get(struct mvebu_mpp_ctrl_data *data,
+ unsigned int pid, unsigned long *config)
+{
+ unsigned off = (pid / MVEBU_MPPS_PER_REG) * MVEBU_MPP_BITS;
+ unsigned shift = (pid % MVEBU_MPPS_PER_REG) * MVEBU_MPP_BITS;
+ unsigned int val;
+ int err;
+
+ err = regmap_read(data->regmap.map, data->regmap.offset + off, &val);
+ if (err)
+ return err;
+
+ *config = (val >> shift) & MVEBU_MPP_MASK;
+
+ return 0;
+}
+
+int mvebu_regmap_mpp_ctrl_set(struct mvebu_mpp_ctrl_data *data,
+ unsigned int pid, unsigned long config)
+{
+ unsigned off = (pid / MVEBU_MPPS_PER_REG) * MVEBU_MPP_BITS;
+ unsigned shift = (pid % MVEBU_MPPS_PER_REG) * MVEBU_MPP_BITS;
+
+ return regmap_update_bits(data->regmap.map, data->regmap.offset + off,
+ MVEBU_MPP_MASK << shift, config << shift);
+}
+
+int mvebu_pinctrl_simple_regmap_probe(struct platform_device *pdev,
+ struct device *syscon_dev)
+{
+ struct mvebu_pinctrl_soc_info *soc = dev_get_platdata(&pdev->dev);
+ struct mvebu_mpp_ctrl_data *mpp_data;
+ struct regmap *regmap;
+ u32 offset;
+ int i;
+
+ regmap = syscon_node_to_regmap(syscon_dev->of_node);
+ if (IS_ERR(regmap))
+ return PTR_ERR(regmap);
+
+ if (of_property_read_u32(pdev->dev.of_node, "offset", &offset))
+ return -EINVAL;
+
+ mpp_data = devm_kcalloc(&pdev->dev, soc->ncontrols, sizeof(*mpp_data),
+ GFP_KERNEL);
+ if (!mpp_data)
+ return -ENOMEM;
+
+ for (i = 0; i < soc->ncontrols; i++) {
+ mpp_data[i].regmap.map = regmap;
+ mpp_data[i].regmap.offset = offset;
+ }
+
+ soc->control_data = mpp_data;
+
+ return mvebu_pinctrl_probe(pdev);
+}
diff --git a/drivers/pinctrl/mvebu/pinctrl-mvebu.h b/drivers/pinctrl/mvebu/pinctrl-mvebu.h
index a9304cdc23e2..c90704e74884 100644
--- a/drivers/pinctrl/mvebu/pinctrl-mvebu.h
+++ b/drivers/pinctrl/mvebu/pinctrl-mvebu.h
@@ -16,9 +16,17 @@
/**
* struct mvebu_mpp_ctrl_data - private data for the mpp ctrl operations
* @base: base address of pinctrl hardware
+ * @regmap.map: regmap structure
+ * @regmap.offset: regmap offset
*/
struct mvebu_mpp_ctrl_data {
- void __iomem *base;
+ union {
+ void __iomem *base;
+ struct {
+ struct regmap *map;
+ u32 offset;
+ } regmap;
+ };
};
/**
@@ -194,8 +202,14 @@ int mvebu_mmio_mpp_ctrl_get(struct mvebu_mpp_ctrl_data *data, unsigned pid,
unsigned long *config);
int mvebu_mmio_mpp_ctrl_set(struct mvebu_mpp_ctrl_data *data, unsigned pid,
unsigned long config);
+int mvebu_regmap_mpp_ctrl_get(struct mvebu_mpp_ctrl_data *data, unsigned pid,
+ unsigned long *config);
+int mvebu_regmap_mpp_ctrl_set(struct mvebu_mpp_ctrl_data *data, unsigned pid,
+ unsigned long config);
int mvebu_pinctrl_probe(struct platform_device *pdev);
int mvebu_pinctrl_simple_mmio_probe(struct platform_device *pdev);
+int mvebu_pinctrl_simple_regmap_probe(struct platform_device *pdev,
+ struct device *syscon_dev);
#endif
--
2.7.4
^ permalink raw reply related
* [PATCH] iommu/dma: Add support for DMA_ATTR_FORCE_CONTIGUOUS
From: Geert Uytterhoeven @ 2017-01-13 11:07 UTC (permalink / raw)
To: linux-arm-kernel
Add support for DMA_ATTR_FORCE_CONTIGUOUS to the generic IOMMU DMA code.
This allows to allocate physically contiguous DMA buffers on arm64
systems with an IOMMU.
Note that as this uses the CMA allocator, setting this attribute has a
runtime-dependency on CONFIG_DMA_CMA, just like on arm32.
For arm64 systems using swiotlb, no changes are needed to support the
allocation of physically contiguous DMA buffers:
- swiotlb always uses physically contiguous buffers (up to
IO_TLB_SEGSIZE = 128 pages),
- arm64's __dma_alloc_coherent() already calls
dma_alloc_from_contiguous() when CMA is available.
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
arch/arm64/mm/dma-mapping.c | 4 ++--
drivers/iommu/dma-iommu.c | 44 ++++++++++++++++++++++++++++++++++----------
include/linux/dma-iommu.h | 2 +-
3 files changed, 37 insertions(+), 13 deletions(-)
diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index 1d7d5d2881db7c19..5fba14a21163e41f 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -589,7 +589,7 @@ static void *__iommu_alloc_attrs(struct device *dev, size_t size,
addr = dma_common_pages_remap(pages, size, VM_USERMAP, prot,
__builtin_return_address(0));
if (!addr)
- iommu_dma_free(dev, pages, iosize, handle);
+ iommu_dma_free(dev, pages, iosize, handle, attrs);
} else {
struct page *page;
/*
@@ -642,7 +642,7 @@ static void __iommu_free_attrs(struct device *dev, size_t size, void *cpu_addr,
if (WARN_ON(!area || !area->pages))
return;
- iommu_dma_free(dev, area->pages, iosize, &handle);
+ iommu_dma_free(dev, area->pages, iosize, &handle, attrs);
dma_common_free_remap(cpu_addr, size, VM_USERMAP);
} else {
iommu_dma_unmap_page(dev, handle, iosize, 0, 0);
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 2db0d641cf4505b5..b991e8dcbbbb35c5 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -30,6 +30,7 @@
#include <linux/pci.h>
#include <linux/scatterlist.h>
#include <linux/vmalloc.h>
+#include <linux/dma-contiguous.h>
struct iommu_dma_msi_page {
struct list_head list;
@@ -238,15 +239,21 @@ static void __iommu_dma_unmap(struct iommu_domain *domain, dma_addr_t dma_addr)
__free_iova(iovad, iova);
}
-static void __iommu_dma_free_pages(struct page **pages, int count)
+static void __iommu_dma_free_pages(struct device *dev, struct page **pages,
+ int count, unsigned long attrs)
{
- while (count--)
- __free_page(pages[count]);
+ if (attrs & DMA_ATTR_FORCE_CONTIGUOUS) {
+ dma_release_from_contiguous(dev, pages[0], count);
+ } else {
+ while (count--)
+ __free_page(pages[count]);
+ }
kvfree(pages);
}
-static struct page **__iommu_dma_alloc_pages(unsigned int count,
- unsigned long order_mask, gfp_t gfp)
+static struct page **__iommu_dma_alloc_pages(struct device *dev,
+ unsigned int count, unsigned long order_mask, gfp_t gfp,
+ unsigned long attrs)
{
struct page **pages;
unsigned int i = 0, array_size = count * sizeof(*pages);
@@ -265,6 +272,20 @@ static struct page **__iommu_dma_alloc_pages(unsigned int count,
/* IOMMU can map any pages, so himem can also be used here */
gfp |= __GFP_NOWARN | __GFP_HIGHMEM;
+ if (attrs & DMA_ATTR_FORCE_CONTIGUOUS) {
+ int order = get_order(count << PAGE_SHIFT);
+ struct page *page;
+
+ page = dma_alloc_from_contiguous(dev, count, order);
+ if (!page)
+ return NULL;
+
+ while (count--)
+ pages[i++] = page++;
+
+ return pages;
+ }
+
while (count) {
struct page *page = NULL;
unsigned int order_size;
@@ -294,7 +315,7 @@ static struct page **__iommu_dma_alloc_pages(unsigned int count,
__free_pages(page, order);
}
if (!page) {
- __iommu_dma_free_pages(pages, i);
+ __iommu_dma_free_pages(dev, pages, i, attrs);
return NULL;
}
count -= order_size;
@@ -310,15 +331,17 @@ static struct page **__iommu_dma_alloc_pages(unsigned int count,
* @pages: Array of buffer pages as returned by iommu_dma_alloc()
* @size: Size of buffer in bytes
* @handle: DMA address of buffer
+ * @attrs: DMA attributes used for allocation of this buffer
*
* Frees both the pages associated with the buffer, and the array
* describing them
*/
void iommu_dma_free(struct device *dev, struct page **pages, size_t size,
- dma_addr_t *handle)
+ dma_addr_t *handle, unsigned long attrs)
{
__iommu_dma_unmap(iommu_get_domain_for_dev(dev), *handle);
- __iommu_dma_free_pages(pages, PAGE_ALIGN(size) >> PAGE_SHIFT);
+ __iommu_dma_free_pages(dev, pages, PAGE_ALIGN(size) >> PAGE_SHIFT,
+ attrs);
*handle = DMA_ERROR_CODE;
}
@@ -365,7 +388,8 @@ struct page **iommu_dma_alloc(struct device *dev, size_t size, gfp_t gfp,
alloc_sizes = min_size;
count = PAGE_ALIGN(size) >> PAGE_SHIFT;
- pages = __iommu_dma_alloc_pages(count, alloc_sizes >> PAGE_SHIFT, gfp);
+ pages = __iommu_dma_alloc_pages(dev, count, alloc_sizes >> PAGE_SHIFT,
+ gfp, attrs);
if (!pages)
return NULL;
@@ -403,7 +427,7 @@ struct page **iommu_dma_alloc(struct device *dev, size_t size, gfp_t gfp,
out_free_iova:
__free_iova(iovad, iova);
out_free_pages:
- __iommu_dma_free_pages(pages, count);
+ __iommu_dma_free_pages(dev, pages, count, attrs);
return NULL;
}
diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
index 7f7e9a7e3839966c..35fa6b7f9bac9b35 100644
--- a/include/linux/dma-iommu.h
+++ b/include/linux/dma-iommu.h
@@ -44,7 +44,7 @@ struct page **iommu_dma_alloc(struct device *dev, size_t size, gfp_t gfp,
unsigned long attrs, int prot, dma_addr_t *handle,
void (*flush_page)(struct device *, const void *, phys_addr_t));
void iommu_dma_free(struct device *dev, struct page **pages, size_t size,
- dma_addr_t *handle);
+ dma_addr_t *handle, unsigned long attrs);
int iommu_dma_mmap(struct page **pages, size_t size, struct vm_area_struct *vma);
--
1.9.1
^ permalink raw reply related
* [PATCH 0/4] video: ARM CLCD: add support of an optional GPIO to enable panel
From: Bartlomiej Zolnierkiewicz @ 2017-01-13 11:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <d1534f4b-e564-d87e-b408-9b7215a90de2@mleia.com>
Hi,
On Thursday, January 12, 2017 11:26:14 PM Vladimir Zapolskiy wrote:
> On 01/12/2017 10:27 AM, Linus Walleij wrote:
> > On Thu, Jan 12, 2017 at 1:05 AM, Vladimir Zapolskiy <vz@mleia.com> wrote:
> >> On 01/11/2017 05:16 PM, Linus Walleij wrote:
> >>> On Tue, Jan 10, 2017 at 2:47 PM, Vladimir Murzin
> >>> <vladimir.murzin@arm.com> wrote:
> >>>
> >>>> In another thread Benjamin pointed at patch [1] in drm/kms part for noMMU.
> >>>>
> >>>> [1] https://cgit.freedesktop.org/drm/drm-misc/commit/?id=62a0d98a188cc4ebd8ea54b37d274ec20465e464
> >>>
> >>> Problem solved?
> >>>
> >>> Vladimir: I do not require in any way that you create a CLCD driver for DRM,
> >>> I just think it would be very very nice...
> >>>
> >>
> >> I have no other option, this series is unreviewed and thus unlikely it will
> >> be applied, still a panel PCB on my board needs power management support.
> >
> > Hm I can ACK it I guess, but mergeing it into an unmaintained subsystem
> > is another issue, just not very optimal.
>
> + Bartlomiej
>
> Linus, your ack is always more than appreciated.
+1
> To all appearance Bartlomiej is a new honoured framebuffer layer maintainer.
>
> Bartlomiej, do you have the changes under discussion in your mailbox or
> should I resend them to you directly for review?
No need for resend, I picked them from the list and they are on TODO.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
> > If you get it working on your system I can look into migrating all the old
> > users to DRM as well.
> >
>
> Sure, I started development of a simple CLCD DRM driver as an own attractive
> exercise, I'll keep you informed of the progress.
>
> --
> With best wishes,
> Vladimir
^ permalink raw reply
* [PATCH v2] KVM: arm/arm64: Fix occasional warning from the timer work function
From: Marc Zyngier @ 2017-01-13 11:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170109111856.8439-1-christoffer.dall@linaro.org>
On 09/01/17 11:18, Christoffer Dall wrote:
> When a VCPU blocks (WFI) and has programmed the vtimer, we program a
> soft timer to expire in the future to wake up the vcpu thread when
> appropriate. Because such as wake up involves a vcpu kick, and the
> timer expire function can get called from interrupt context, and the
> kick may sleep, we have to schedule the kick in the work function.
>
> The work function currently has a warning that gets raised if it turns
> out that the timer shouldn't fire when it's run, which was added because
> the idea was that in that case the work should never have been cancelled.
>
> However, it turns out that this whole thing is racy and we can get
> spurious warnings. The problem is that we clear the armed flag in the
> work function, which may run in parallel with the
> kvm_timer_unschedule->timer_disarm() call. This results in a possible
> situation where the timer_disarm() call does not call
> cancel_work_sync(), which effectively synchronizes the completion of the
> work function with running the VCPU. As a result, the VCPU thread
> proceeds before the work function completees, causing changes to the
> timer state such that kvm_timer_should_fire(vcpu) returns false in the
> work function.
>
> All we do in the work function is to kick the VCPU, and an occasional
> rare extra kick never harmed anyone. Since the race above is extremely
> rare, we don't bother checking if the race happens but simply remove the
> check and the clearing of the armed flag from the work function.
>
> Reported-by: Matthias Brugger <mbrugger@suse.com>
> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* [PATCH v3 0/3] arm64: dts: juno: CoreSight support updates
From: Sudeep Holla @ 2017-01-13 11:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAJ9a7Vha=GiyEcVkzh-N9s7XN8N94KAiM=iY2W4MyLqLWRe-2w@mail.gmail.com>
On 13/01/17 11:09, Mike Leach wrote:
> Hi Sudeep,
>
>
> Patchset tested successfully on juno-r1
>
Thanks and I did a quick test on Juno R2. I have pushed the branch [1]
with minor comment change as pointed out my Mathieu.
--
Regards,
Sudeep
[1]
git://git.kernel.org/pub/scm/linux/kernel/git/sudeep.holla/linux.git
for-next/updates/juno
^ permalink raw reply
* [PATCH v29 9/9] Documentation: dt: chosen properties for arm64 kdump
From: Mark Rutland @ 2017-01-13 11:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170113091339.GK20972@linaro.org>
On Fri, Jan 13, 2017 at 06:13:49PM +0900, AKASHI Takahiro wrote:
> On Thu, Jan 12, 2017 at 03:39:45PM +0000, Mark Rutland wrote:
> > On Wed, Dec 28, 2016 at 01:37:34PM +0900, AKASHI Takahiro wrote:
> > > +linux,crashkernel-base
> > > +linux,crashkernel-size
> > > +----------------------
> > > +
> > > +These properties (currently used on PowerPC and arm64) indicates
> > > +the base address and the size, respectively, of the reserved memory
> > > +range for crash dump kernel.
> >
> > From this description, it's not clear to me what the (expected)
> > consumers of this property are, nor what is expected to provide it.
> >
> > In previous rounds of review, I had assumed that this was used to
> > describe a preference to the first kernel as to what region of memory
> > should be used for a subsequent kdump kernel. Looking around, I'm not
> > sure if I was correct in that assessment.
> >
> > I see that arch/powerpc seems to consume this property to configure
> > crashk_res, but it also rewrites it based on crashk_res, presumably for
> > the benefit of userspace. It's not clear to me how on powerpc the kdump
> > kernel knows its memory range -- is more DT modification done in the
> > kernel and/or userspace?
>
> I don't believe that powerpc will rewrite the property any way.
> As far as I know from *the source code*, powerpc kernel retrieves
> the memory range for crash dump kernel from a kernel command line, i.e.
> crashkernel=, and then exposes it through DT to userspace (assuming
> kexec-tools).
The rewriting I describe is in export_crashk_values() in
arch/powerpc/kernel/machine_kexec.c, where the code deletes existing the
properties, and adds new ones, to the DT exposed to userspace.
So I think we're just quibbling over the definition of "rewrite".
> > arm64 we should either ensure that /proc/iomem is consistently usable
> > (and have userspace consistently use it), or we should expose a new file
> > specifically to expose this information.
>
> The thing that I had in my mind when adding this property is that
> /proc/iomem would be obsolete in the future, then we should have
> an alternative in hand.
Ok.
My disagreement is with using the DT as a channel to convey information
from the kernel to userspace.
I'm more than happy for a new file or other mechanism to express this
information. For example, we could add
/sys/kernel/kexec_crash_{base,size} or similar.
> > Further, I do not think we need this property. It makes more sense to me
> > for the preference of a a region to be described to the *first* kernel
> > using the command line consistently.
> >
> > So I think we should drop this property, and not use it on arm64. Please
> > document this as powerpc only.
>
> OK, but if we drop the property from arm64 code, we have no reason
> to leave its description in this patch.
> (In fact, there are a few more (undocumented) properties that only ppc
> uses for kdump.)
I'm happy to drop it, then.
> > > +linux,usable-memory-range
> > > +-------------------------
> > > +
> > > +This property (currently used only on arm64) holds the memory range,
> > > +the base address and the size, which can be used as system ram on
> > > +the *current* kernel. Note that, if this property is present, any memory
> > > +regions under "memory" nodes in DT blob or ones marked as "conventional
> > > +memory" in EFI memory map should be ignored.
> >
> > Could you please replace this with:
> >
> > This property (arm64 only) holds a base address and size, describing a
> > limited region in which memory may be considered available for use by
> > the kernel. Memory outside of this range is not available for use.
> >
> > This property describes a limitation: memory within this range is only
> > valid when also described through another mechanism that the kernel
> > would otherwise use to determine available memory (e.g. memory nodes
> > or the EFI memory map). Valid memory may be sparse within the range.
>
> Sure.
Cheers!
Thanks,
Mark.
^ permalink raw reply
* [PATCH] arm64/mm: use phys_addr_t
From: Catalin Marinas @ 2017-01-13 11:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484287175-14342-1-git-send-email-miles.chen@mediatek.com>
On Fri, Jan 13, 2017 at 01:59:35PM +0800, miles.chen at mediatek.com wrote:
> From: Miles Chen <miles.chen@mediatek.com>
>
> Use phys_addr_t instead of unsigned long for the
> return value of __pa(), make code easy to understand.
>
> Signed-off-by: Miles Chen <miles.chen@mediatek.com>
> ---
> arch/arm64/mm/mmu.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 17243e4..7eb7c21 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -359,8 +359,8 @@ static void create_mapping_late(phys_addr_t phys, unsigned long virt,
>
> static void __init __map_memblock(pgd_t *pgd, phys_addr_t start, phys_addr_t end)
> {
> - unsigned long kernel_start = __pa(_text);
> - unsigned long kernel_end = __pa(__init_begin);
> + phys_addr_t kernel_start = __pa(_text);
> + phys_addr_t kernel_end = __pa(__init_begin);
Looks fine.
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
^ permalink raw reply
* [PATCH] arm64/mm: use phys_addr_t
From: Mark Rutland @ 2017-01-13 11:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484287175-14342-1-git-send-email-miles.chen@mediatek.com>
On Fri, Jan 13, 2017 at 01:59:35PM +0800, miles.chen at mediatek.com wrote:
> From: Miles Chen <miles.chen@mediatek.com>
>
> Use phys_addr_t instead of unsigned long for the
> return value of __pa(), make code easy to understand.
>
> Signed-off-by: Miles Chen <miles.chen@mediatek.com>
This looks sensible to me. It's consistent with the types these
variables are compared against, and with the types of function
parameters these are passed as.
Acked-by: Mark Rutland <mark.rutland@arm.com>
Thanks,
Mark.
> ---
> arch/arm64/mm/mmu.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 17243e4..7eb7c21 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -359,8 +359,8 @@ static void create_mapping_late(phys_addr_t phys, unsigned long virt,
>
> static void __init __map_memblock(pgd_t *pgd, phys_addr_t start, phys_addr_t end)
> {
> - unsigned long kernel_start = __pa(_text);
> - unsigned long kernel_end = __pa(__init_begin);
> + phys_addr_t kernel_start = __pa(_text);
> + phys_addr_t kernel_end = __pa(__init_begin);
>
> /*
> * Take care not to create a writable alias for the
> --
> 1.9.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [RFC] Kernel panic down to swiotlb when doing insmod a simple driver
From: Ard Biesheuvel @ 2017-01-13 11:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <99472068-069a-7759-8d6e-019fd875264d@arm.com>
On 13 January 2017 at 11:03, Robin Murphy <robin.murphy@arm.com> wrote:
> On 13/01/17 10:00, Shawn Lin wrote:
>> Hi,
>>
>> Sorry for sending this RFC for help as I couldn't find some useful hint
>> to slove my issue by git-log the swiotlb commit from kernel v4.4 to
>> v4.9 and I'm also not familar with these stuff. So could you kindly
>> point me to the right direction to debug it? Thanks. :)
>>
>> --------------------------------------
>> We just have a very simple wifi driver *built as ko module* which only
>> have a probe function to do the basic init work and call SDIO API to
>> transfer some bytes.
>>
>> Env: kernel 4.4 stable tree, ARM64(rk3399)
>>
>> Two cases are included:
>
> And they are both wrong :)
>
>> The crash case:
>>
>> u8 __aligned(32) buf[PAGE_SIZE]; //global here in ko driver file
>
> It is only valid to do DMA from linear map addresses - I'm not sure if
> the modules area was in the linear map before, but either way it
> probably isn't now (Ard, Mark?). Either way, I don't believe static data
> honours ARCH_DMA_MINALIGN in general, so it's still highly inadvisable.
>
The __aligned() modifier should work fine: the alignment is propagated
to the ELF section alignment, which in turn is honoured by the module
loader. The problem is that '32' is too low for non-coherent DMA to be
safe. In general, alignments up to 4 KB should work everywhere.
I am surprised though that this ever worked as a module, given that
modules are (and have always been) loaded in the vmalloc area, which
means VA to PA translations performed in the DMA layer on the
addresses of statically allocated buffers are unlikely to return
correct values (as your panic log proves)
>> static int wifi_probe(struct sdio_func *func, const struct
>> sdio_device_id *id)
>> {
>> // prepare some SDIO work before
>> printk("wifi_probe: buf = 0x%x\n", buf);
>> sdio_memcpy_toio(func, 0, buf, 200);
>> }
>>
>> The workable case:
>>
>> static int wifi_probe(struct sdio_func *func, const struct
>> sdio_device_id *id)
>> {
>>
>> u8 __aligned(32) buf[PAGE_SIZE]; //move inside the probe function
>
> No. DMA from the stack is right out, both for the aforementioned
> alignment reasons, and the fact that we now have (or will have)
> virtually-mapped stacks. One of the benefits of the latter is that it
> catches bugs like this ;)
>
Actually, aligned stack variables also work fine. But DMA involving
the stack is not, so that is not really relevant.
> Get your buffer from kmalloc() or a page allocation, and everything
> should be correct.
>
Agreed.
^ permalink raw reply
* [PATCH] arm64/mm: use phys_addr_t
From: Ard Biesheuvel @ 2017-01-13 11:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170113112205.GA26804@leverpostej>
On 13 January 2017 at 11:22, Mark Rutland <mark.rutland@arm.com> wrote:
> On Fri, Jan 13, 2017 at 01:59:35PM +0800, miles.chen at mediatek.com wrote:
>> From: Miles Chen <miles.chen@mediatek.com>
>>
>> Use phys_addr_t instead of unsigned long for the
>> return value of __pa(), make code easy to understand.
>>
>> Signed-off-by: Miles Chen <miles.chen@mediatek.com>
>
> This looks sensible to me. It's consistent with the types these
> variables are compared against, and with the types of function
> parameters these are passed as.
>
Indeed. But doesn't it clash with Laura's series?
> Acked-by: Mark Rutland <mark.rutland@arm.com>
>
> Thanks,
> Mark.
>
>> ---
>> arch/arm64/mm/mmu.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
>> index 17243e4..7eb7c21 100644
>> --- a/arch/arm64/mm/mmu.c
>> +++ b/arch/arm64/mm/mmu.c
>> @@ -359,8 +359,8 @@ static void create_mapping_late(phys_addr_t phys, unsigned long virt,
>>
>> static void __init __map_memblock(pgd_t *pgd, phys_addr_t start, phys_addr_t end)
>> {
>> - unsigned long kernel_start = __pa(_text);
>> - unsigned long kernel_end = __pa(__init_begin);
>> + phys_addr_t kernel_start = __pa(_text);
>> + phys_addr_t kernel_end = __pa(__init_begin);
>>
>> /*
>> * Take care not to create a writable alias for the
>> --
>> 1.9.1
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH RFC 0/5] mvebu cleanups and preparation for Armada 7k/8k
From: Russell King - ARM Linux @ 2017-01-13 11:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170113110240.GA29164@n2100.armlinux.org.uk>
On Fri, Jan 13, 2017 at 11:02:40AM +0000, Russell King - ARM Linux wrote:
> (Resend including linux-arm-kernel)
BTW, would be useful if MAINTAINERS could list maintainers for both
this code and the mvebu gpio code too. At the moment, it seems Linus
is the person responsible for both of those - but I suspect the mvebu
people want to be copied on patches...
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* [PATCH 0/3] KVM/ARM updates for 4.10-rc4
From: Marc Zyngier @ 2017-01-13 11:31 UTC (permalink / raw)
To: linux-arm-kernel
Radim, Paolo,
Here's the KVM/ARM updates for 4.10-rc4. Two timer fixes, and one vgic
fix for a deadlock that's been reported this week (which should land
into stable).
Please pull.
Thanks,
M.
The following changes since commit a121103c922847ba5010819a3f250f1f7fc84ab8:
Linux 4.10-rc3 (2017-01-08 14:18:17 -0800)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git tags/kvm-arm-for-4.10-rc4
for you to fetch changes up to 1193e6aeecb36c74c48c7cd0f641acbbed9ddeef:
KVM: arm/arm64: vgic: Fix deadlock on error handling (2017-01-13 11:19:35 +0000)
----------------------------------------------------------------
KVM/ARM updates for 4.10-rc4
- Fix for timer setup on VHE machines
- Drop spurious warning when the timer races against
the vcpu running again
- Prevent a vgic deadlock when the initialization fails
----------------------------------------------------------------
Christoffer Dall (1):
KVM: arm/arm64: Fix occasional warning from the timer work function
Jintack Lim (1):
KVM: arm64: Access CNTHCTL_EL2 bit fields correctly on VHE systems
Marc Zyngier (1):
KVM: arm/arm64: vgic: Fix deadlock on error handling
arch/arm/include/asm/virt.h | 5 +++++
arch/arm/kvm/arm.c | 3 +++
arch/arm64/include/asm/virt.h | 9 +++++++++
include/kvm/arm_arch_timer.h | 1 +
virt/kvm/arm/arch_timer.c | 26 +++++++++++++++++++++++---
virt/kvm/arm/hyp/timer-sr.c | 33 +++++++++++++++++++++------------
virt/kvm/arm/vgic/vgic-init.c | 18 +++++++++++++-----
virt/kvm/arm/vgic/vgic-v2.c | 2 --
virt/kvm/arm/vgic/vgic-v3.c | 2 --
9 files changed, 75 insertions(+), 24 deletions(-)
^ permalink raw reply
* [PATCH 1/3] KVM: arm/arm64: Fix occasional warning from the timer work function
From: Marc Zyngier @ 2017-01-13 11:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484307093-29153-1-git-send-email-marc.zyngier@arm.com>
From: Christoffer Dall <christoffer.dall@linaro.org>
When a VCPU blocks (WFI) and has programmed the vtimer, we program a
soft timer to expire in the future to wake up the vcpu thread when
appropriate. Because such as wake up involves a vcpu kick, and the
timer expire function can get called from interrupt context, and the
kick may sleep, we have to schedule the kick in the work function.
The work function currently has a warning that gets raised if it turns
out that the timer shouldn't fire when it's run, which was added because
the idea was that in that case the work should never have been cancelled.
However, it turns out that this whole thing is racy and we can get
spurious warnings. The problem is that we clear the armed flag in the
work function, which may run in parallel with the
kvm_timer_unschedule->timer_disarm() call. This results in a possible
situation where the timer_disarm() call does not call
cancel_work_sync(), which effectively synchronizes the completion of the
work function with running the VCPU. As a result, the VCPU thread
proceeds before the work function completees, causing changes to the
timer state such that kvm_timer_should_fire(vcpu) returns false in the
work function.
All we do in the work function is to kick the VCPU, and an occasional
rare extra kick never harmed anyone. Since the race above is extremely
rare, we don't bother checking if the race happens but simply remove the
check and the clearing of the armed flag from the work function.
Reported-by: Matthias Brugger <mbrugger@suse.com>
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
virt/kvm/arm/arch_timer.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
index a2dbbcc..a7fe606 100644
--- a/virt/kvm/arm/arch_timer.c
+++ b/virt/kvm/arm/arch_timer.c
@@ -89,9 +89,6 @@ static void kvm_timer_inject_irq_work(struct work_struct *work)
struct kvm_vcpu *vcpu;
vcpu = container_of(work, struct kvm_vcpu, arch.timer_cpu.expired);
- vcpu->arch.timer_cpu.armed = false;
-
- WARN_ON(!kvm_timer_should_fire(vcpu));
/*
* If the vcpu is blocked we want to wake it up so that it will see
--
2.1.4
^ permalink raw reply related
* [PATCH 2/3] KVM: arm64: Access CNTHCTL_EL2 bit fields correctly on VHE systems
From: Marc Zyngier @ 2017-01-13 11:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484307093-29153-1-git-send-email-marc.zyngier@arm.com>
From: Jintack Lim <jintack@cs.columbia.edu>
Current KVM world switch code is unintentionally setting wrong bits to
CNTHCTL_EL2 when E2H == 1, which may allow guest OS to access physical
timer. Bit positions of CNTHCTL_EL2 are changing depending on
HCR_EL2.E2H bit. EL1PCEN and EL1PCTEN are 1st and 0th bits when E2H is
not set, but they are 11th and 10th bits respectively when E2H is set.
In fact, on VHE we only need to set those bits once, not for every world
switch. This is because the host kernel runs in EL2 with HCR_EL2.TGE ==
1, which makes those bits have no effect for the host kernel execution.
So we just set those bits once for guests, and that's it.
Signed-off-by: Jintack Lim <jintack@cs.columbia.edu>
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
arch/arm/include/asm/virt.h | 5 +++++
arch/arm/kvm/arm.c | 3 +++
arch/arm64/include/asm/virt.h | 9 +++++++++
include/kvm/arm_arch_timer.h | 1 +
virt/kvm/arm/arch_timer.c | 23 +++++++++++++++++++++++
virt/kvm/arm/hyp/timer-sr.c | 33 +++++++++++++++++++++------------
6 files changed, 62 insertions(+), 12 deletions(-)
diff --git a/arch/arm/include/asm/virt.h b/arch/arm/include/asm/virt.h
index a2e75b8..6dae195 100644
--- a/arch/arm/include/asm/virt.h
+++ b/arch/arm/include/asm/virt.h
@@ -80,6 +80,11 @@ static inline bool is_kernel_in_hyp_mode(void)
return false;
}
+static inline bool has_vhe(void)
+{
+ return false;
+}
+
/* The section containing the hypervisor idmap text */
extern char __hyp_idmap_text_start[];
extern char __hyp_idmap_text_end[];
diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index 1167678..9d74464 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -1099,6 +1099,9 @@ static void cpu_init_hyp_mode(void *dummy)
__cpu_init_hyp_mode(pgd_ptr, hyp_stack_ptr, vector_ptr);
__cpu_init_stage2();
+ if (is_kernel_in_hyp_mode())
+ kvm_timer_init_vhe();
+
kvm_arm_init_debug();
}
diff --git a/arch/arm64/include/asm/virt.h b/arch/arm64/include/asm/virt.h
index fea1073..439f6b5 100644
--- a/arch/arm64/include/asm/virt.h
+++ b/arch/arm64/include/asm/virt.h
@@ -47,6 +47,7 @@
#include <asm/ptrace.h>
#include <asm/sections.h>
#include <asm/sysreg.h>
+#include <asm/cpufeature.h>
/*
* __boot_cpu_mode records what mode CPUs were booted in.
@@ -80,6 +81,14 @@ static inline bool is_kernel_in_hyp_mode(void)
return read_sysreg(CurrentEL) == CurrentEL_EL2;
}
+static inline bool has_vhe(void)
+{
+ if (cpus_have_const_cap(ARM64_HAS_VIRT_HOST_EXTN))
+ return true;
+
+ return false;
+}
+
#ifdef CONFIG_ARM64_VHE
extern void verify_cpu_run_el(void);
#else
diff --git a/include/kvm/arm_arch_timer.h b/include/kvm/arm_arch_timer.h
index b717ed9..5c970ce 100644
--- a/include/kvm/arm_arch_timer.h
+++ b/include/kvm/arm_arch_timer.h
@@ -76,4 +76,5 @@ void kvm_timer_unschedule(struct kvm_vcpu *vcpu);
void kvm_timer_vcpu_put(struct kvm_vcpu *vcpu);
+void kvm_timer_init_vhe(void);
#endif
diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
index a7fe606..6a084cd 100644
--- a/virt/kvm/arm/arch_timer.c
+++ b/virt/kvm/arm/arch_timer.c
@@ -24,6 +24,7 @@
#include <clocksource/arm_arch_timer.h>
#include <asm/arch_timer.h>
+#include <asm/kvm_hyp.h>
#include <kvm/arm_vgic.h>
#include <kvm/arm_arch_timer.h>
@@ -509,3 +510,25 @@ void kvm_timer_init(struct kvm *kvm)
{
kvm->arch.timer.cntvoff = kvm_phys_timer_read();
}
+
+/*
+ * On VHE system, we only need to configure trap on physical timer and counter
+ * accesses in EL0 and EL1 once, not for every world switch.
+ * The host kernel runs at EL2 with HCR_EL2.TGE == 1,
+ * and this makes those bits have no effect for the host kernel execution.
+ */
+void kvm_timer_init_vhe(void)
+{
+ /* When HCR_EL2.E2H ==1, EL1PCEN and EL1PCTEN are shifted by 10 */
+ u32 cnthctl_shift = 10;
+ u64 val;
+
+ /*
+ * Disallow physical timer access for the guest.
+ * Physical counter access is allowed.
+ */
+ val = read_sysreg(cnthctl_el2);
+ val &= ~(CNTHCTL_EL1PCEN << cnthctl_shift);
+ val |= (CNTHCTL_EL1PCTEN << cnthctl_shift);
+ write_sysreg(val, cnthctl_el2);
+}
diff --git a/virt/kvm/arm/hyp/timer-sr.c b/virt/kvm/arm/hyp/timer-sr.c
index 798866a..63e28dd 100644
--- a/virt/kvm/arm/hyp/timer-sr.c
+++ b/virt/kvm/arm/hyp/timer-sr.c
@@ -35,10 +35,16 @@ void __hyp_text __timer_save_state(struct kvm_vcpu *vcpu)
/* Disable the virtual timer */
write_sysreg_el0(0, cntv_ctl);
- /* Allow physical timer/counter access for the host */
- val = read_sysreg(cnthctl_el2);
- val |= CNTHCTL_EL1PCTEN | CNTHCTL_EL1PCEN;
- write_sysreg(val, cnthctl_el2);
+ /*
+ * We don't need to do this for VHE since the host kernel runs in EL2
+ * with HCR_EL2.TGE ==1, which makes those bits have no impact.
+ */
+ if (!has_vhe()) {
+ /* Allow physical timer/counter access for the host */
+ val = read_sysreg(cnthctl_el2);
+ val |= CNTHCTL_EL1PCTEN | CNTHCTL_EL1PCEN;
+ write_sysreg(val, cnthctl_el2);
+ }
/* Clear cntvoff for the host */
write_sysreg(0, cntvoff_el2);
@@ -50,14 +56,17 @@ void __hyp_text __timer_restore_state(struct kvm_vcpu *vcpu)
struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
u64 val;
- /*
- * Disallow physical timer access for the guest
- * Physical counter access is allowed
- */
- val = read_sysreg(cnthctl_el2);
- val &= ~CNTHCTL_EL1PCEN;
- val |= CNTHCTL_EL1PCTEN;
- write_sysreg(val, cnthctl_el2);
+ /* Those bits are already configured at boot on VHE-system */
+ if (!has_vhe()) {
+ /*
+ * Disallow physical timer access for the guest
+ * Physical counter access is allowed
+ */
+ val = read_sysreg(cnthctl_el2);
+ val &= ~CNTHCTL_EL1PCEN;
+ val |= CNTHCTL_EL1PCTEN;
+ write_sysreg(val, cnthctl_el2);
+ }
if (timer->enabled) {
write_sysreg(kvm->arch.timer.cntvoff, cntvoff_el2);
--
2.1.4
^ permalink raw reply related
* [PATCH 3/3] KVM: arm/arm64: vgic: Fix deadlock on error handling
From: Marc Zyngier @ 2017-01-13 11:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484307093-29153-1-git-send-email-marc.zyngier@arm.com>
Dmitry Vyukov reported that the syzkaller fuzzer triggered a
deadlock in the vgic setup code when an error was detected, as
the cleanup code tries to take a lock that is already held by
the setup code.
The fix is to avoid retaking the lock when cleaning up, by
telling the cleanup function that we already hold it.
Cc: stable at vger.kernel.org
Reported-by: Dmitry Vyukov <dvyukov@google.com>
Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
virt/kvm/arm/vgic/vgic-init.c | 18 +++++++++++++-----
virt/kvm/arm/vgic/vgic-v2.c | 2 --
virt/kvm/arm/vgic/vgic-v3.c | 2 --
3 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/virt/kvm/arm/vgic/vgic-init.c b/virt/kvm/arm/vgic/vgic-init.c
index 5114391..c737ea0 100644
--- a/virt/kvm/arm/vgic/vgic-init.c
+++ b/virt/kvm/arm/vgic/vgic-init.c
@@ -268,15 +268,11 @@ static void kvm_vgic_dist_destroy(struct kvm *kvm)
{
struct vgic_dist *dist = &kvm->arch.vgic;
- mutex_lock(&kvm->lock);
-
dist->ready = false;
dist->initialized = false;
kfree(dist->spis);
dist->nr_spis = 0;
-
- mutex_unlock(&kvm->lock);
}
void kvm_vgic_vcpu_destroy(struct kvm_vcpu *vcpu)
@@ -286,7 +282,8 @@ void kvm_vgic_vcpu_destroy(struct kvm_vcpu *vcpu)
INIT_LIST_HEAD(&vgic_cpu->ap_list_head);
}
-void kvm_vgic_destroy(struct kvm *kvm)
+/* To be called with kvm->lock held */
+static void __kvm_vgic_destroy(struct kvm *kvm)
{
struct kvm_vcpu *vcpu;
int i;
@@ -297,6 +294,13 @@ void kvm_vgic_destroy(struct kvm *kvm)
kvm_vgic_vcpu_destroy(vcpu);
}
+void kvm_vgic_destroy(struct kvm *kvm)
+{
+ mutex_lock(&kvm->lock);
+ __kvm_vgic_destroy(kvm);
+ mutex_unlock(&kvm->lock);
+}
+
/**
* vgic_lazy_init: Lazy init is only allowed if the GIC exposed to the guest
* is a GICv2. A GICv3 must be explicitly initialized by the guest using the
@@ -348,6 +352,10 @@ int kvm_vgic_map_resources(struct kvm *kvm)
ret = vgic_v2_map_resources(kvm);
else
ret = vgic_v3_map_resources(kvm);
+
+ if (ret)
+ __kvm_vgic_destroy(kvm);
+
out:
mutex_unlock(&kvm->lock);
return ret;
diff --git a/virt/kvm/arm/vgic/vgic-v2.c b/virt/kvm/arm/vgic/vgic-v2.c
index 9bab867..834137e 100644
--- a/virt/kvm/arm/vgic/vgic-v2.c
+++ b/virt/kvm/arm/vgic/vgic-v2.c
@@ -293,8 +293,6 @@ int vgic_v2_map_resources(struct kvm *kvm)
dist->ready = true;
out:
- if (ret)
- kvm_vgic_destroy(kvm);
return ret;
}
diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c
index 5c9f974..e6b03fd 100644
--- a/virt/kvm/arm/vgic/vgic-v3.c
+++ b/virt/kvm/arm/vgic/vgic-v3.c
@@ -302,8 +302,6 @@ int vgic_v3_map_resources(struct kvm *kvm)
dist->ready = true;
out:
- if (ret)
- kvm_vgic_destroy(kvm);
return ret;
}
--
2.1.4
^ permalink raw reply related
* [PATCH] iommu/dma: Add support for DMA_ATTR_FORCE_CONTIGUOUS
From: Robin Murphy @ 2017-01-13 11:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484305674-15018-1-git-send-email-geert+renesas@glider.be>
On 13/01/17 11:07, Geert Uytterhoeven wrote:
> Add support for DMA_ATTR_FORCE_CONTIGUOUS to the generic IOMMU DMA code.
> This allows to allocate physically contiguous DMA buffers on arm64
> systems with an IOMMU.
Can anyone explain what this attribute is actually used for? I've never
quite figured it out.
> Note that as this uses the CMA allocator, setting this attribute has a
> runtime-dependency on CONFIG_DMA_CMA, just like on arm32.
>
> For arm64 systems using swiotlb, no changes are needed to support the
> allocation of physically contiguous DMA buffers:
> - swiotlb always uses physically contiguous buffers (up to
> IO_TLB_SEGSIZE = 128 pages),
> - arm64's __dma_alloc_coherent() already calls
> dma_alloc_from_contiguous() when CMA is available.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> arch/arm64/mm/dma-mapping.c | 4 ++--
> drivers/iommu/dma-iommu.c | 44 ++++++++++++++++++++++++++++++++++----------
> include/linux/dma-iommu.h | 2 +-
> 3 files changed, 37 insertions(+), 13 deletions(-)
>
> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
> index 1d7d5d2881db7c19..5fba14a21163e41f 100644
> --- a/arch/arm64/mm/dma-mapping.c
> +++ b/arch/arm64/mm/dma-mapping.c
> @@ -589,7 +589,7 @@ static void *__iommu_alloc_attrs(struct device *dev, size_t size,
> addr = dma_common_pages_remap(pages, size, VM_USERMAP, prot,
> __builtin_return_address(0));
> if (!addr)
> - iommu_dma_free(dev, pages, iosize, handle);
> + iommu_dma_free(dev, pages, iosize, handle, attrs);
> } else {
> struct page *page;
> /*
> @@ -642,7 +642,7 @@ static void __iommu_free_attrs(struct device *dev, size_t size, void *cpu_addr,
>
> if (WARN_ON(!area || !area->pages))
> return;
> - iommu_dma_free(dev, area->pages, iosize, &handle);
> + iommu_dma_free(dev, area->pages, iosize, &handle, attrs);
> dma_common_free_remap(cpu_addr, size, VM_USERMAP);
> } else {
> iommu_dma_unmap_page(dev, handle, iosize, 0, 0);
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index 2db0d641cf4505b5..b991e8dcbbbb35c5 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -30,6 +30,7 @@
> #include <linux/pci.h>
> #include <linux/scatterlist.h>
> #include <linux/vmalloc.h>
> +#include <linux/dma-contiguous.h>
>
> struct iommu_dma_msi_page {
> struct list_head list;
> @@ -238,15 +239,21 @@ static void __iommu_dma_unmap(struct iommu_domain *domain, dma_addr_t dma_addr)
> __free_iova(iovad, iova);
> }
>
> -static void __iommu_dma_free_pages(struct page **pages, int count)
> +static void __iommu_dma_free_pages(struct device *dev, struct page **pages,
> + int count, unsigned long attrs)
> {
> - while (count--)
> - __free_page(pages[count]);
> + if (attrs & DMA_ATTR_FORCE_CONTIGUOUS) {
> + dma_release_from_contiguous(dev, pages[0], count);
> + } else {
> + while (count--)
> + __free_page(pages[count]);
> + }
> kvfree(pages);
> }
>
> -static struct page **__iommu_dma_alloc_pages(unsigned int count,
> - unsigned long order_mask, gfp_t gfp)
> +static struct page **__iommu_dma_alloc_pages(struct device *dev,
> + unsigned int count, unsigned long order_mask, gfp_t gfp,
> + unsigned long attrs)
> {
> struct page **pages;
> unsigned int i = 0, array_size = count * sizeof(*pages);
> @@ -265,6 +272,20 @@ static struct page **__iommu_dma_alloc_pages(unsigned int count,
> /* IOMMU can map any pages, so himem can also be used here */
> gfp |= __GFP_NOWARN | __GFP_HIGHMEM;
>
> + if (attrs & DMA_ATTR_FORCE_CONTIGUOUS) {
> + int order = get_order(count << PAGE_SHIFT);
> + struct page *page;
> +
> + page = dma_alloc_from_contiguous(dev, count, order);
> + if (!page)
> + return NULL;
> +
> + while (count--)
> + pages[i++] = page++;
> +
> + return pages;
> + }
> +
This is really yuck. Plus it's entirely pointless to go through the
whole page array/scatterlist dance when we know the buffer is going to
be physically contiguous - it should just be allocate, map, done. I'd
much rather see standalone iommu_dma_{alloc,free}_contiguous()
functions, and let the arch code handle dispatching appropriately.
Robin.
> while (count) {
> struct page *page = NULL;
> unsigned int order_size;
> @@ -294,7 +315,7 @@ static struct page **__iommu_dma_alloc_pages(unsigned int count,
> __free_pages(page, order);
> }
> if (!page) {
> - __iommu_dma_free_pages(pages, i);
> + __iommu_dma_free_pages(dev, pages, i, attrs);
> return NULL;
> }
> count -= order_size;
> @@ -310,15 +331,17 @@ static struct page **__iommu_dma_alloc_pages(unsigned int count,
> * @pages: Array of buffer pages as returned by iommu_dma_alloc()
> * @size: Size of buffer in bytes
> * @handle: DMA address of buffer
> + * @attrs: DMA attributes used for allocation of this buffer
> *
> * Frees both the pages associated with the buffer, and the array
> * describing them
> */
> void iommu_dma_free(struct device *dev, struct page **pages, size_t size,
> - dma_addr_t *handle)
> + dma_addr_t *handle, unsigned long attrs)
> {
> __iommu_dma_unmap(iommu_get_domain_for_dev(dev), *handle);
> - __iommu_dma_free_pages(pages, PAGE_ALIGN(size) >> PAGE_SHIFT);
> + __iommu_dma_free_pages(dev, pages, PAGE_ALIGN(size) >> PAGE_SHIFT,
> + attrs);
> *handle = DMA_ERROR_CODE;
> }
>
> @@ -365,7 +388,8 @@ struct page **iommu_dma_alloc(struct device *dev, size_t size, gfp_t gfp,
> alloc_sizes = min_size;
>
> count = PAGE_ALIGN(size) >> PAGE_SHIFT;
> - pages = __iommu_dma_alloc_pages(count, alloc_sizes >> PAGE_SHIFT, gfp);
> + pages = __iommu_dma_alloc_pages(dev, count, alloc_sizes >> PAGE_SHIFT,
> + gfp, attrs);
> if (!pages)
> return NULL;
>
> @@ -403,7 +427,7 @@ struct page **iommu_dma_alloc(struct device *dev, size_t size, gfp_t gfp,
> out_free_iova:
> __free_iova(iovad, iova);
> out_free_pages:
> - __iommu_dma_free_pages(pages, count);
> + __iommu_dma_free_pages(dev, pages, count, attrs);
> return NULL;
> }
>
> diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
> index 7f7e9a7e3839966c..35fa6b7f9bac9b35 100644
> --- a/include/linux/dma-iommu.h
> +++ b/include/linux/dma-iommu.h
> @@ -44,7 +44,7 @@ struct page **iommu_dma_alloc(struct device *dev, size_t size, gfp_t gfp,
> unsigned long attrs, int prot, dma_addr_t *handle,
> void (*flush_page)(struct device *, const void *, phys_addr_t));
> void iommu_dma_free(struct device *dev, struct page **pages, size_t size,
> - dma_addr_t *handle);
> + dma_addr_t *handle, unsigned long attrs);
>
> int iommu_dma_mmap(struct page **pages, size_t size, struct vm_area_struct *vma);
>
>
^ permalink raw reply
* [PATCH] arm64: defconfig: disable CONFIG_DEVMEM
From: Leif Lindholm @ 2017-01-13 11:37 UTC (permalink / raw)
To: linux-arm-kernel
/dev/mem is the opposite of what an operating system is for.
Additionally, on arm* it opens up for denial-of-service attacks from
userspace. So leave it disabled by default, requiring people who need
it to enable it explicitly.
Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
---
arch/arm64/configs/defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 33b744d..55ba73a 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -210,6 +210,7 @@ CONFIG_INPUT_HISI_POWERKEY=y
# CONFIG_SERIO_SERPORT is not set
CONFIG_SERIO_AMBAKMI=y
CONFIG_LEGACY_PTY_COUNT=16
+# CONFIG_DEVMEM is not set
CONFIG_SERIAL_8250=y
CONFIG_SERIAL_8250_CONSOLE=y
CONFIG_SERIAL_8250_EXTENDED=y
--
2.10.2
^ permalink raw reply related
* [PATCH] arm64: defconfig: disable CONFIG_DEVMEM
From: Ard Biesheuvel @ 2017-01-13 11:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170113113734.16524-1-leif.lindholm@linaro.org>
On 13 January 2017 at 11:37, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> /dev/mem is the opposite of what an operating system is for.
> Additionally, on arm* it opens up for denial-of-service attacks from
> userspace. So leave it disabled by default, requiring people who need
> it to enable it explicitly.
>
> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> arch/arm64/configs/defconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> index 33b744d..55ba73a 100644
> --- a/arch/arm64/configs/defconfig
> +++ b/arch/arm64/configs/defconfig
> @@ -210,6 +210,7 @@ CONFIG_INPUT_HISI_POWERKEY=y
> # CONFIG_SERIO_SERPORT is not set
> CONFIG_SERIO_AMBAKMI=y
> CONFIG_LEGACY_PTY_COUNT=16
> +# CONFIG_DEVMEM is not set
> CONFIG_SERIAL_8250=y
> CONFIG_SERIAL_8250_CONSOLE=y
> CONFIG_SERIAL_8250_EXTENDED=y
> --
> 2.10.2
>
^ permalink raw reply
* [PATCH v29 3/9] arm64: kdump: reserve memory for crash dump kernel
From: Mark Rutland @ 2017-01-13 11:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170113081617.GI20972@linaro.org>
On Fri, Jan 13, 2017 at 05:16:18PM +0900, AKASHI Takahiro wrote:
> On Thu, Jan 12, 2017 at 03:09:26PM +0000, Mark Rutland wrote:
> > > +static int __init export_crashkernel(void)
> > > + /* Add /chosen/linux,crashkernel-* properties */
> > > + of_remove_property(node, of_find_property(node,
> > > + "linux,crashkernel-base", NULL));
> > > + of_remove_property(node, of_find_property(node,
> > > + "linux,crashkernel-size", NULL));
> > > +
> > > + ret = of_add_property(node, &crash_base_prop);
> > > + if (ret)
> > > + goto ret_err;
> > > +
> > > + ret = of_add_property(node, &crash_size_prop);
> > > + if (ret)
> > > + goto ret_err;
> > I very much do not like this.
> >
> > I don't think we should be modifying the DT exposed to userspace in this
> > manner, in the usual boot path, especially given that the kernel itself
> > does not appear to be a consumer of this property. I do not think that
> > it is right to use the DT exposed to userspace as a communication
> > channel solely between the kernel and userspace.
>
> As you mentioned in your comments against my patch#9, this property
> originates from PPC implementation.
> I added it solely from the sympathy for dt-based architectures.
>
> > So I think we should drop the above, and for arm64 have userspace
> > consistently use /proc/iomem (or perhaps a new kexec-specific file) to
> > determine the region reserved for the crash kernel, if it needs to know
> > this.
>
> As a matter of fact, my port of kexec-tools doesn't check this property
> and dropping it won't cause any problem.
Ok. It sounds like we're both happy for this to go, then.
While it's unfortunate that architectures differ, I think we have
legitimate reasons to differ, and it's preferable to do so. We have a
different set of constraints (e.g. supporting EFI memory maps), and
following the PPC approach creates longer term issues for us, making it
harder to do the right thing consistently.
> > > +/*
> > > + * reserve_crashkernel() - reserves memory for crash kernel
> > > + *
> > > + * This function reserves memory area given in "crashkernel=" kernel command
> > > + * line parameter. The memory reserved is used by dump capture kernel when
> > > + * primary kernel is crashing.
> > > + */
> > > +static void __init reserve_crashkernel(void)
> > > + memblock_reserve(crash_base, crash_size);
> >
> > This will mean that the crash kernel will have a permanent alias in the linear
> > map which is vulnerable to being clobbered. There could also be issues
> > with mismatched attributes in future.
>
> Good point, I've never thought of that except making the memblock
> region "reserved."
>
> > We're probably ok for now, but in future we'll likely want to fix this
> > up to remove the region (or mark it nomap), and only map it temporarily
> > when loading things into the region.
>
> Well, I found that the following commit is already in:
> commit 9b492cf58077
> Author: Xunlei Pang <xlpang@redhat.com>
> Date: Mon May 23 16:24:10 2016 -0700
>
> kexec: introduce a protection mechanism for the crashkernel
> reserved memory
>
> To make best use of this framework, I'd like to re-use set_memory_ro/rx()
> instead of removing the region from linear mapping. But to do so,
> we need to
> * make memblock_isolate_range() global,
> * allow set_memory_ro/rx() to be applied to regions in linear mapping
> since set_memory_ro/rx() works only on page-level mappings.
>
> What do you think?
> (See my tentative solution below.)
Great! I think it would be better to follow the approach of
mark_rodata_ro(), rather than opening up set_memory_*(), but otherwise,
it looks like it should work.
Either way, this still leaves us with an RO alias on crashed cores (and
potential cache attribute mismatches in future). Do we need to read from
the region later, or could we unmap it entirely?
Thanks,
Mark.
> ===8<===
> diff --git a/arch/arm64/kernel/machine_kexec.c b/arch/arm64/kernel/machine_kexec.c
> index c0fc3d458195..bb21c0473b8e 100644
> --- a/arch/arm64/kernel/machine_kexec.c
> +++ b/arch/arm64/kernel/machine_kexec.c
> @@ -211,6 +211,44 @@ void machine_kexec(struct kimage *kimage)
> BUG(); /* Should never get here. */
> }
>
> +static int kexec_mark_range(unsigned long start, unsigned long end,
> + bool protect)
> +{
> + unsigned int nr_pages;
> +
> + if (!end || start >= end)
> + return 0;
> +
> + nr_pages = (end >> PAGE_SHIFT) - (start >> PAGE_SHIFT) + 1;
> +
> + if (protect)
> + return set_memory_ro(__phys_to_virt(start), nr_pages);
> + else
> + return set_memory_rw(__phys_to_virt(start), nr_pages);
> +}
> +
> +static void kexec_mark_crashkres(bool protect)
> +{
> + unsigned long control;
> +
> + /* Don't touch the control code page used in crash_kexec().*/
> + control = page_to_phys(kexec_crash_image->control_code_page);
> + kexec_mark_range(crashk_res.start, control - 1, protect);
> +
> + control += KEXEC_CONTROL_PAGE_SIZE;
> + kexec_mark_range(control, crashk_res.end, protect);
> +}
> +
> +void arch_kexec_protect_crashkres(void)
> +{
> + kexec_mark_crashkres(true);
> +}
> +
> +void arch_kexec_unprotect_crashkres(void)
> +{
> + kexec_mark_crashkres(false);
> +}
> +
> static void machine_kexec_mask_interrupts(void)
> {
> unsigned int i;
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index 569ec3325bc8..764ec89c4f76 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -90,6 +90,7 @@ early_param("initrd", early_initrd);
> static void __init reserve_crashkernel(void)
> {
> unsigned long long crash_size, crash_base;
> + int start_rgn, end_rgn;
> int ret;
>
> ret = parse_crashkernel(boot_command_line, memblock_phys_mem_size(),
> @@ -121,6 +122,9 @@ static void __init reserve_crashkernel(void)
> }
> }
> memblock_reserve(crash_base, crash_size);
> + memblock_isolate_range(&memblock.memory, crash_base, crash_size,
> + &start_rgn, &end_rgn);
> +
>
> pr_info("Reserving %lldMB of memory at %lldMB for crashkernel\n",
> crash_size >> 20, crash_base >> 20);
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 17243e43184e..0f60f19c287b 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -22,6 +22,7 @@
> #include <linux/kernel.h>
> #include <linux/errno.h>
> #include <linux/init.h>
> +#include <linux/kexec.h>
> #include <linux/libfdt.h>
> #include <linux/mman.h>
> #include <linux/nodemask.h>
> @@ -362,6 +363,17 @@ static void __init __map_memblock(pgd_t *pgd, phys_addr_t start, phys_addr_t end
> unsigned long kernel_start = __pa(_text);
> unsigned long kernel_end = __pa(__init_begin);
>
> +#ifdef CONFIG_KEXEC_CORE
> + if (crashk_res.end && start >= crashk_res.start &&
> + end <= (crashk_res.end + 1)) {
> + __create_pgd_mapping(pgd, start, __phys_to_virt(start),
> + end - start, PAGE_KERNEL,
> + early_pgtable_alloc,
> + true);
> + return;
> + }
> +#endif
> +
> /*
> * Take care not to create a writable alias for the
> * read-only text and rodata sections of the kernel image.
> ===>8===
^ permalink raw reply
* [PATCH] arm64: defconfig: disable CONFIG_DEVMEM
From: Mark Rutland @ 2017-01-13 11:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170113113734.16524-1-leif.lindholm@linaro.org>
On Fri, Jan 13, 2017 at 11:37:34AM +0000, Leif Lindholm wrote:
> /dev/mem is the opposite of what an operating system is for.
> Additionally, on arm* it opens up for denial-of-service attacks from
> userspace. So leave it disabled by default, requiring people who need
> it to enable it explicitly.
>
> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
Generally I'd agree that the use of /dev/mem is a bad idea (TM), so I'm
surprised we have it in our defconfig.
FWIW:
Acked-by: Mark Rutland <mark.rutland@arm.com>
Mark.
> ---
> arch/arm64/configs/defconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> index 33b744d..55ba73a 100644
> --- a/arch/arm64/configs/defconfig
> +++ b/arch/arm64/configs/defconfig
> @@ -210,6 +210,7 @@ CONFIG_INPUT_HISI_POWERKEY=y
> # CONFIG_SERIO_SERPORT is not set
> CONFIG_SERIO_AMBAKMI=y
> CONFIG_LEGACY_PTY_COUNT=16
> +# CONFIG_DEVMEM is not set
> CONFIG_SERIAL_8250=y
> CONFIG_SERIAL_8250_CONSOLE=y
> CONFIG_SERIAL_8250_EXTENDED=y
> --
> 2.10.2
>
^ permalink raw reply
* [PATCH] arm64/mm: use phys_addr_t
From: Mark Rutland @ 2017-01-13 11:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKv+Gu__4S+0B39SXn46sqxsH0OXeusotZOBmvzZmw-yRRzdjA@mail.gmail.com>
On Fri, Jan 13, 2017 at 11:27:48AM +0000, Ard Biesheuvel wrote:
> On 13 January 2017 at 11:22, Mark Rutland <mark.rutland@arm.com> wrote:
> > On Fri, Jan 13, 2017 at 01:59:35PM +0800, miles.chen at mediatek.com wrote:
> >> From: Miles Chen <miles.chen@mediatek.com>
> >>
> >> Use phys_addr_t instead of unsigned long for the
> >> return value of __pa(), make code easy to understand.
> >>
> >> Signed-off-by: Miles Chen <miles.chen@mediatek.com>
> >
> > This looks sensible to me. It's consistent with the types these
> > variables are compared against, and with the types of function
> > parameters these are passed as.
> >
>
> Indeed. But doesn't it clash with Laura's series?
Good point.
Yes, but only for the RHS of the assignment changing. This'll need to be
rebased atop of the arm64 for-next/core branch, or Catalin/Will might
fix it up when applying, perhaps?
Thanks,
Mark.
> > Acked-by: Mark Rutland <mark.rutland@arm.com>
> >
> > Thanks,
> > Mark.
> >
> >> ---
> >> arch/arm64/mm/mmu.c | 4 ++--
> >> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> >> index 17243e4..7eb7c21 100644
> >> --- a/arch/arm64/mm/mmu.c
> >> +++ b/arch/arm64/mm/mmu.c
> >> @@ -359,8 +359,8 @@ static void create_mapping_late(phys_addr_t phys, unsigned long virt,
> >>
> >> static void __init __map_memblock(pgd_t *pgd, phys_addr_t start, phys_addr_t end)
> >> {
> >> - unsigned long kernel_start = __pa(_text);
> >> - unsigned long kernel_end = __pa(__init_begin);
> >> + phys_addr_t kernel_start = __pa(_text);
> >> + phys_addr_t kernel_end = __pa(__init_begin);
> >>
> >> /*
> >> * Take care not to create a writable alias for the
> >> --
> >> 1.9.1
> >>
> >>
> >> _______________________________________________
> >> linux-arm-kernel mailing list
> >> linux-arm-kernel at lists.infradead.org
> >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel at lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v7 08/15] ACPI: IORT: rename iort_node_map_rid() to make it generic
From: Lorenzo Pieralisi @ 2017-01-13 11:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484147199-4267-9-git-send-email-hanjun.guo@linaro.org>
On Wed, Jan 11, 2017 at 11:06:32PM +0800, Hanjun Guo wrote:
> iort_node_map_rid() was designed for both PCI and platform
> device, but the rid means requester id is for ITS mappings,
I do not understand what this means sorry.
> rename iort_node_map_rid() to iort_node_map_id() and update
> its argument names to make it more generic.
>
"iort_node_map_rid() was designed to take an input id (that is not
necessarily a PCI requester id) and map it to an output id (eg an SMMU
streamid or an ITS deviceid) according to the mappings provided by an
IORT node mapping entries. This means that the iort_node_map_rid() input
id is not always a PCI requester id as its name, parameters and local
variables suggest, which is misleading.
Apply the s/rid/id substitution to the iort_node_map_rid() mapping
function and its users to make sure its intended usage is clearer."
Lorenzo
> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Tomasz Nowicki <tn@semihalf.com>
> ---
> drivers/acpi/arm64/iort.c | 30 +++++++++++++++---------------
> 1 file changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index 208eac9..069a690 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -355,11 +355,11 @@ struct acpi_iort_node *iort_node_get_id(struct acpi_iort_node *node,
> return NULL;
> }
>
> -static struct acpi_iort_node *iort_node_map_rid(struct acpi_iort_node *node,
> - u32 rid_in, u32 *rid_out,
> - u8 type_mask)
> +static struct acpi_iort_node *iort_node_map_id(struct acpi_iort_node *node,
> + u32 id_in, u32 *id_out,
> + u8 type_mask)
> {
> - u32 rid = rid_in;
> + u32 id = id_in;
>
> /* Parse the ID mapping tree to find specified node type */
> while (node) {
> @@ -367,8 +367,8 @@ static struct acpi_iort_node *iort_node_map_rid(struct acpi_iort_node *node,
> int i;
>
> if (IORT_TYPE_MASK(node->type) & type_mask) {
> - if (rid_out)
> - *rid_out = rid;
> + if (id_out)
> + *id_out = id;
> return node;
> }
>
> @@ -385,9 +385,9 @@ static struct acpi_iort_node *iort_node_map_rid(struct acpi_iort_node *node,
> goto fail_map;
> }
>
> - /* Do the RID translation */
> + /* Do the ID translation */
> for (i = 0; i < node->mapping_count; i++, map++) {
> - if (!iort_id_map(map, node->type, rid, &rid))
> + if (!iort_id_map(map, node->type, id, &id))
> break;
> }
>
> @@ -399,9 +399,9 @@ static struct acpi_iort_node *iort_node_map_rid(struct acpi_iort_node *node,
> }
>
> fail_map:
> - /* Map input RID to output RID unchanged on mapping failure*/
> - if (rid_out)
> - *rid_out = rid_in;
> + /* Map input ID to output ID unchanged on mapping failure */
> + if (id_out)
> + *id_out = id_in;
>
> return NULL;
> }
> @@ -439,7 +439,7 @@ u32 iort_msi_map_rid(struct device *dev, u32 req_id)
> if (!node)
> return req_id;
>
> - iort_node_map_rid(node, req_id, &dev_id, IORT_MSI_TYPE);
> + iort_node_map_id(node, req_id, &dev_id, IORT_MSI_TYPE);
> return dev_id;
> }
>
> @@ -462,7 +462,7 @@ static int iort_dev_find_its_id(struct device *dev, u32 req_id,
> if (!node)
> return -ENXIO;
>
> - node = iort_node_map_rid(node, req_id, NULL, IORT_MSI_TYPE);
> + node = iort_node_map_id(node, req_id, NULL, IORT_MSI_TYPE);
> if (!node)
> return -ENXIO;
>
> @@ -591,8 +591,8 @@ const struct iommu_ops *iort_iommu_configure(struct device *dev)
> if (!node)
> return NULL;
>
> - parent = iort_node_map_rid(node, rid, &streamid,
> - IORT_IOMMU_TYPE);
> + parent = iort_node_map_id(node, rid, &streamid,
> + IORT_IOMMU_TYPE);
>
> ops = iort_iommu_xlate(dev, parent, streamid);
>
> --
> 1.9.1
>
^ permalink raw reply
* [RFC] Kernel panic down to swiotlb when doing insmod a simple driver
From: Robin Murphy @ 2017-01-13 11:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKv+Gu-aaGuHmshi=aiexUD=dxuia7tHKm3LQt3RG-7D_3v9Mw@mail.gmail.com>
On 13/01/17 11:25, Ard Biesheuvel wrote:
> On 13 January 2017 at 11:03, Robin Murphy <robin.murphy@arm.com> wrote:
>> On 13/01/17 10:00, Shawn Lin wrote:
>>> Hi,
>>>
>>> Sorry for sending this RFC for help as I couldn't find some useful hint
>>> to slove my issue by git-log the swiotlb commit from kernel v4.4 to
>>> v4.9 and I'm also not familar with these stuff. So could you kindly
>>> point me to the right direction to debug it? Thanks. :)
>>>
>>> --------------------------------------
>>> We just have a very simple wifi driver *built as ko module* which only
>>> have a probe function to do the basic init work and call SDIO API to
>>> transfer some bytes.
>>>
>>> Env: kernel 4.4 stable tree, ARM64(rk3399)
>>>
>>> Two cases are included:
>>
>> And they are both wrong :)
>>
>>> The crash case:
>>>
>>> u8 __aligned(32) buf[PAGE_SIZE]; //global here in ko driver file
>>
>> It is only valid to do DMA from linear map addresses - I'm not sure if
>> the modules area was in the linear map before, but either way it
>> probably isn't now (Ard, Mark?). Either way, I don't believe static data
>> honours ARCH_DMA_MINALIGN in general, so it's still highly inadvisable.
>>
>
> The __aligned() modifier should work fine: the alignment is propagated
> to the ELF section alignment, which in turn is honoured by the module
> loader. The problem is that '32' is too low for non-coherent DMA to be
> safe. In general, alignments up to 4 KB should work everywhere.
Does that alignment also implicitly apply to the size, though? In other
words, given:
static int X
static int __aligned(32) Y;
static int Z;
is it guaranteed that if, say, X gets placed at .data + 0, so Y goes to
.data + 32, then Z *cannot* be placed at .data + 36?
Robin.
> I am surprised though that this ever worked as a module, given that
> modules are (and have always been) loaded in the vmalloc area, which
> means VA to PA translations performed in the DMA layer on the
> addresses of statically allocated buffers are unlikely to return
> correct values (as your panic log proves)
>
>>> static int wifi_probe(struct sdio_func *func, const struct
>>> sdio_device_id *id)
>>> {
>>> // prepare some SDIO work before
>>> printk("wifi_probe: buf = 0x%x\n", buf);
>>> sdio_memcpy_toio(func, 0, buf, 200);
>>> }
>>>
>>> The workable case:
>>>
>>> static int wifi_probe(struct sdio_func *func, const struct
>>> sdio_device_id *id)
>>> {
>>>
>>> u8 __aligned(32) buf[PAGE_SIZE]; //move inside the probe function
>>
>> No. DMA from the stack is right out, both for the aforementioned
>> alignment reasons, and the fact that we now have (or will have)
>> virtually-mapped stacks. One of the benefits of the latter is that it
>> catches bugs like this ;)
>>
>
> Actually, aligned stack variables also work fine. But DMA involving
> the stack is not, so that is not really relevant.
>
>> Get your buffer from kmalloc() or a page allocation, and everything
>> should be correct.
>>
>
> Agreed.
>
^ permalink raw reply
* [PATCH] arm64: defconfig: disable CONFIG_DEVMEM
From: Will Deacon @ 2017-01-13 11:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170113113734.16524-1-leif.lindholm@linaro.org>
On Fri, Jan 13, 2017 at 11:37:34AM +0000, Leif Lindholm wrote:
> /dev/mem is the opposite of what an operating system is for.
> Additionally, on arm* it opens up for denial-of-service attacks from
> userspace. So leave it disabled by default, requiring people who need
> it to enable it explicitly.
I really like the idea, but are we sure that nothing common breaks without
this? For example, does Debian still boot nicely with this patch applied?
Just trying to get a feel for how much userspace this has seen (particularly
on ACPI-based systems, which I seem to remember like poking about in here).
Will
^ permalink raw reply
* [RFC] Kernel panic down to swiotlb when doing insmod a simple driver
From: Ard Biesheuvel @ 2017-01-13 11:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <bf4b1861-7190-c888-b9be-646e2a2297bb@arm.com>
On 13 January 2017 at 11:47, Robin Murphy <robin.murphy@arm.com> wrote:
> On 13/01/17 11:25, Ard Biesheuvel wrote:
>> On 13 January 2017 at 11:03, Robin Murphy <robin.murphy@arm.com> wrote:
>>> On 13/01/17 10:00, Shawn Lin wrote:
>>>> Hi,
>>>>
>>>> Sorry for sending this RFC for help as I couldn't find some useful hint
>>>> to slove my issue by git-log the swiotlb commit from kernel v4.4 to
>>>> v4.9 and I'm also not familar with these stuff. So could you kindly
>>>> point me to the right direction to debug it? Thanks. :)
>>>>
>>>> --------------------------------------
>>>> We just have a very simple wifi driver *built as ko module* which only
>>>> have a probe function to do the basic init work and call SDIO API to
>>>> transfer some bytes.
>>>>
>>>> Env: kernel 4.4 stable tree, ARM64(rk3399)
>>>>
>>>> Two cases are included:
>>>
>>> And they are both wrong :)
>>>
>>>> The crash case:
>>>>
>>>> u8 __aligned(32) buf[PAGE_SIZE]; //global here in ko driver file
>>>
>>> It is only valid to do DMA from linear map addresses - I'm not sure if
>>> the modules area was in the linear map before, but either way it
>>> probably isn't now (Ard, Mark?). Either way, I don't believe static data
>>> honours ARCH_DMA_MINALIGN in general, so it's still highly inadvisable.
>>>
>>
>> The __aligned() modifier should work fine: the alignment is propagated
>> to the ELF section alignment, which in turn is honoured by the module
>> loader. The problem is that '32' is too low for non-coherent DMA to be
>> safe. In general, alignments up to 4 KB should work everywhere.
>
> Does that alignment also implicitly apply to the size, though? In other
> words, given:
>
> static int X
> static int __aligned(32) Y;
> static int Z;
>
> is it guaranteed that if, say, X gets placed at .data + 0, so Y goes to
> .data + 32, then Z *cannot* be placed at .data + 36?
>
I'm not sure if I understand the question: why would it be incorrect
for Z to be placed at .data + 36?
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox