Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] PCI: imx6: fix spelling mistake: "contol" -> "control"
From: Bjorn Helgaas @ 2017-04-25 19:04 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170421070230.16582-1-colin.king@canonical.com>

On Fri, Apr 21, 2017 at 08:02:30AM +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> trivial fix to spelling mistake in dev_err message
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

Applied with Richard's ack to pci/host-imx6 for v4.12, thanks.

FWIW, there are about 25 other occurrences of "contol" in other parts
of the tree.

> ---
>  drivers/pci/dwc/pci-imx6.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/dwc/pci-imx6.c b/drivers/pci/dwc/pci-imx6.c
> index 102edcf1e261..129717ae5022 100644
> --- a/drivers/pci/dwc/pci-imx6.c
> +++ b/drivers/pci/dwc/pci-imx6.c
> @@ -726,13 +726,13 @@ static int imx6_pcie_probe(struct platform_device *pdev)
>  		imx6_pcie->pciephy_reset = devm_reset_control_get(dev,
>  								  "pciephy");
>  		if (IS_ERR(imx6_pcie->pciephy_reset)) {
> -			dev_err(dev, "Failed to get PCIEPHY reset contol\n");
> +			dev_err(dev, "Failed to get PCIEPHY reset control\n");
>  			return PTR_ERR(imx6_pcie->pciephy_reset);
>  		}
>  
>  		imx6_pcie->apps_reset = devm_reset_control_get(dev, "apps");
>  		if (IS_ERR(imx6_pcie->apps_reset)) {
> -			dev_err(dev, "Failed to get PCIE APPS reset contol\n");
> +			dev_err(dev, "Failed to get PCIE APPS reset control\n");
>  			return PTR_ERR(imx6_pcie->apps_reset);
>  		}
>  		break;
> -- 
> 2.11.0
> 
> 
> _______________________________________________
> 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 1/2] kvm: Fix mmu_notifier release race
From: Radim Krčmář @ 2017-04-25 18:49 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1493028624-29837-2-git-send-email-suzuki.poulose@arm.com>

2017-04-24 11:10+0100, Suzuki K Poulose:
> The KVM uses mmu_notifier (wherever available) to keep track
> of the changes to the mm of the guest. The guest shadow page
> tables are released when the VM exits via mmu_notifier->ops.release().
> There is a rare chance that the mmu_notifier->release could be
> called more than once via two different paths, which could end
> up in use-after-free of kvm instance (such as [0]).
> 
> e.g:
> 
> thread A                                        thread B
> -------                                         --------------
> 
>  get_signal->                                   kvm_destroy_vm()->
>  do_exit->                                        mmu_notifier_unregister->
>  exit_mm->                                        kvm_arch_flush_shadow_all()->
>  exit_mmap->                                      spin_lock(&kvm->mmu_lock)
>  mmu_notifier_release->                           ....
>   kvm_arch_flush_shadow_all()->                   .....
>   ... spin_lock(&kvm->mmu_lock)                   .....
>                                                   spin_unlock(&kvm->mmu_lock)
>                                                 kvm_arch_free_kvm()
>    *** use after free of kvm ***

I don't understand this race ...
a piece of code in mmu_notifier_unregister() says:

  	/*
  	 * Wait for any running method to finish, of course including
  	 * ->release if it was run by mmu_notifier_release instead of us.
  	 */
  	synchronize_srcu(&srcu);

and code before that removes the notifier from the list, so it cannot be
called after we pass this point.  mmu_notifier_release() does roughly
the same and explains it as:

  	/*
  	 * synchronize_srcu here prevents mmu_notifier_release from returning to
  	 * exit_mmap (which would proceed with freeing all pages in the mm)
  	 * until the ->release method returns, if it was invoked by
  	 * mmu_notifier_unregister.
  	 *
  	 * The mmu_notifier_mm can't go away from under us because one mm_count
  	 * is held by exit_mmap.
  	 */
  	synchronize_srcu(&srcu);

The call of mmu_notifier->release is protected by srcu in both cases and
while it seems possible that mmu_notifier->release would be called
twice, I don't see a combination that could result in use-after-free
from mmu_notifier_release after mmu_notifier_unregister() has returned.

Doesn't [2/2] solve the exact same issue (that the release method cannot
be called twice in parallel)?

Thanks.

^ permalink raw reply

* [PATCH V8 4/5] PCI/ASPM: save power on values during bridge init
From: Bjorn Helgaas @ 2017-04-25 18:45 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <92EBB4272BF81E4089A7126EC1E7B2846676C7EF@IRSMSX101.ger.corp.intel.com>

On Fri, Apr 21, 2017 at 2:46 AM, Patel, Mayurkumar
<mayurkumar.patel@intel.com> wrote:
> Hi Bjorn/Kaya,
>
>
>>
>>On 4/17/2017 12:38 PM, Bjorn Helgaas wrote:
>>>> Like you said, what do we do by default is the question. Should we opt
>>>> for safe like we are doing, or try to save some power.
>>> I think safety is paramount.  Every user should be able to boot safely
>>> without any kernel parameters.  We don't want users to have a problem
>>> booting and then have to search for a workaround like booting with
>>> "pcie_aspm=off".  Most users will never do that.
>>>
>>
>>OK, no problem with leaving the behavior as it is.
>>
>>My initial approach was #2. We knew this way that user had full control
>>over the ASPM policy by changing the BIOS option. Then, Mayurkumar
>>complained that ASPM is not enabled following a hotplug insertion to an
>>empty slot. That's when I switched to #3 as it sounded like a good thing
>>to have for us.
>>
>>> Here's a long-term strawman proposal, see what you think:
>>>
>>>   - Deprecate CONFIG_PCIEASPM_DEFAULT, CONFIG_PCIEASPM_POWERSAVE, etc.
>>>   - Default aspm_policy is POLICY_DEFAULT always.
>>>   - POLICY_DEFAULT means Linux doesn't touch anything: if BIOS enabled
>>> ASPM, we leave it that way; we leave ASPM disabled on hot-added
>>> devices.
>>
> I am also ok with leaving the same behavior as now.
> But still following is something open I feel besides, Which may be there in your comments redundantly.
> The current problem is, pcie_aspm_exit_link_state() disables the ASPM configuration even
> if POLICY_DEFAULT was set.

We call pcie_aspm_exit_link_state() when removing an endpoint.  When
we remove an endpoint, I think disabling ASPM is the right thing to
do.  The spec (PCIe r3.1, sec 5.4.1.3) says "Software must not enable
L0s in either direction on a given Link unless components on both
sides of the Link each support L0s; otherwise, the result is
undefined."

> I am seeing already following problem(or may be influence) with it. The Endpoint I have does not have
> does not have "Presence detect change" mechanism. Hot plug is working with Link status events.
> When link is in L1 or L1SS and if EP is powered off, no Link status change event are triggered (It might be
> the expected behavior in L1 or L1SS).  When next time EP is powered on there are link down and
> link up events coming one after other. BIOS enables ASPM on Root port and Endpoint, but while
> processing link status down, pcie_aspm_exit_link_state() clears the ASPM already which were enabled by BIOS.
> If we want to follow above approach then shall we consider having something similar as following?

The proposal was to leave ASPM disabled on hot-added devices.  If the
endpoint was powered off and powered back on again, I think that
device looks like a hot-added device, doesn't it?

Bjorn

^ permalink raw reply

* [RFC PATCH] drivers: dma-mapping: Do not attempt to create a scatterlist for from_coherent buffers
From: Catalin Marinas @ 2017-04-25 18:35 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170425180525.GT17774@n2100.armlinux.org.uk>

On Tue, Apr 25, 2017 at 07:05:26PM +0100, Russell King - ARM Linux wrote:
> On Tue, Apr 25, 2017 at 06:57:39PM +0100, Catalin Marinas wrote:
> > Memory returned by dma_alloc_from_coherent() is not backed by struct
> > page and creating a scatterlist would use invalid page pointers. The
> > patch introduces the dma_vaddr_from_coherent() function and the
> > corresponding check in dma_get_sgtable_attrs().
> > 
> > Fixes: d2b7428eb0ca ("common: dma-mapping: introduce dma_get_sgtable() function")
> > Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: Russell King - ARM Linux <linux@arm.linux.org.uk>
> > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> > ---
> > 
> > In a recent discussion around the iommu DMA ops on arm64, Russell
> > pointed out that dma_get_sgtable is not safe since the coherent DMA
> > memory is not always backed by struct page. Russell has queued an
> > arm-specific patch checking for pfn_valid() but I thought I'd make a
> > more generic fix. This patch aims to bring the dma_get_sgtable() API in
> > line with the dma_alloc/mmap/free with respect to the from_coherent
> > memory.
> 
> Sorry, I don't think this is the correct approach.
> 
> You're assuming that 'vaddr' will always be a valid lowmem address.
> That isn't always the case - some dma coherent allocations provide
> remapped memory.

I'm not assuming lowmem. The only thing this patch does is that if the
memory came from the dma_declare_coherent range, it returns -ENXIO. This
is in line with the dma_free_attrs() for example, which calls
dma_release_from_coherent(), but for sgtable this wouldn't make sense,
hence -ENXIO.

I agree with you that vaddr can be remapped (which is the case on arm64
as well) but it's the responsibility of the arch-specific
dma_ops->get_sgtable() to handle it properly (vmalloc_to_page etc.). The
fall-back dma_common_get_sgtable() does assume lowmem but it doesn't
mean that the arch DMA ops need to use it.

> The reason for dma_get_sgtable() existing is to coerce the DMA
> coherent memory into a scatterlist so that it can be passed through
> the dma_buf API - that's where the problem lies.  The dma_buf API
> needs fixing so that coherent memory can be sanely passed, and
> dma_get_sgtable() needs to be put out of its misery.

I agree but didn't dare to dig into this ;).

-- 
Catalin

^ permalink raw reply

* [PATCH v4 00/21] PCI: fix config space memory mappings
From: Khuong Dinh @ 2017-04-25 18:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <000101d2bddf$cd442e80$67cc8b80$@gmail.com>

Ack.
Tested-by: Khuong Dinh <kdinh@apm.com>


On Tue, Apr 25, 2017 at 9:20 AM, Jingoo Han <jingoohan1@gmail.com> wrote:
> On Tuesday, April 25, 2017 2:41 AM, Jon Masters wrote:
>>
>> On 04/19/2017 12:48 PM, Lorenzo Pieralisi wrote:
>>
>> > On some platforms (ie ARM/ARM64) ioremap fails to comply with the PCI
>> > configuration non-posted write transactions requirement, because it
>> > provides a memory mapping that issues "bufferable" or, in PCI terms
>> > "posted" write transactions. Likewise, the current pci_remap_iospace()
>> > implementation maps the physical address range that the PCI translates
>> > to I/O space cycles to virtual address space through pgprot_device()
>> > attributes that on eg ARM64 provides a memory mapping issuing
>> > posted writes transactions, which is not PCI specifications compliant.
>>
>> Side note that I've pinged all of the ARM server vendors and asked them
>> to verify this patch series on their platforms.
>
> Good! I really want to know the result of these patches on ARM serves.
> Please share it with us. Good luck.
>
> Best regards,
> Jingoo Han
>
>>
>> Jon.
>
>

-- 
CONFIDENTIALITY NOTICE: This e-mail message, including any attachments, is 
for the sole use of the intended recipient(s) and contains information that 
is confidential and proprietary to Applied Micro Circuits Corporation or 
its subsidiaries. It is to be used solely for the purpose of furthering the 
parties' business relationship. All unauthorized review, use, disclosure or 
distribution is prohibited. If you are not the intended recipient, please 
contact the sender by reply e-mail and destroy all copies of the original 
message.

^ permalink raw reply

* [PATCH] drivers: dma-mapping: Do not leave an invalid area->pages pointer in dma_common_contiguous_remap()
From: Catalin Marinas @ 2017-04-25 18:25 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1493144543-2497-1-git-send-email-catalin.marinas@arm.com>

On Tue, Apr 25, 2017 at 07:22:23PM +0100, Catalin Marinas wrote:
> The dma_common_pages_remap() function allocates a vm_struct object and
> initialises the pages pointer to value passed as argument. However, when
> this function is called dma_common_contiguous_remap(), the pages array
> is only temporarily allocated, being freed shortly after
> dma_common_contiguous_remap() returns. Architecture code checking the
> validity of an area->pages pointer would incorrectly dereference already
> freed pointers. This has been exposed by the arm64 commit 44176bb38fa4
> ("arm64: Add support for DMA_ATTR_FORCE_CONTIGUOUS to IOMMU").
> 
> Fixes: 513510ddba96 ("common: dma-mapping: introduce common remapping functions")
> Cc: Laura Abbott <labbott@redhat.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Reported-by: Andrzej Hajda <a.hajda@samsung.com>
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>

Small correction on the subject, the prefix should be something like:

drivers: dma-mapping:

It's not an arm64 patch.

-- 
Catalin

^ permalink raw reply

* [PATCH] arm64: Do not leave an invalid area->pages pointer in dma_common_contiguous_remap()
From: Catalin Marinas @ 2017-04-25 18:22 UTC (permalink / raw)
  To: linux-arm-kernel

The dma_common_pages_remap() function allocates a vm_struct object and
initialises the pages pointer to value passed as argument. However, when
this function is called dma_common_contiguous_remap(), the pages array
is only temporarily allocated, being freed shortly after
dma_common_contiguous_remap() returns. Architecture code checking the
validity of an area->pages pointer would incorrectly dereference already
freed pointers. This has been exposed by the arm64 commit 44176bb38fa4
("arm64: Add support for DMA_ATTR_FORCE_CONTIGUOUS to IOMMU").

Fixes: 513510ddba96 ("common: dma-mapping: introduce common remapping functions")
Cc: Laura Abbott <labbott@redhat.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reported-by: Andrzej Hajda <a.hajda@samsung.com>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---

This is for correctness since once the arm64's mmap and get_sgtable ops
are fixed for DMA_ATTR_FORCE_CONTIGUOUS, we would no longer see the
issue. Anyway, it's better to get this fixed in case others trip over a
similar issue. I added a "Fixes" tag for completeness but I'm not sure
it's worth back-porting.

 drivers/base/dma-mapping.c | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c
index efd71cf4fdea..ab7071041141 100644
--- a/drivers/base/dma-mapping.c
+++ b/drivers/base/dma-mapping.c
@@ -277,8 +277,8 @@ EXPORT_SYMBOL(dma_common_mmap);
  * remaps an array of PAGE_SIZE pages into another vm_area
  * Cannot be used in non-sleeping contexts
  */
-void *dma_common_pages_remap(struct page **pages, size_t size,
-			unsigned long vm_flags, pgprot_t prot,
+static struct vm_struct *__dma_common_pages_remap(struct page **pages,
+			size_t size, unsigned long vm_flags, pgprot_t prot,
 			const void *caller)
 {
 	struct vm_struct *area;
@@ -287,13 +287,26 @@ void *dma_common_pages_remap(struct page **pages, size_t size,
 	if (!area)
 		return NULL;
 
-	area->pages = pages;
-
 	if (map_vm_area(area, prot, pages)) {
 		vunmap(area->addr);
 		return NULL;
 	}
 
+	return area;
+}
+
+void *dma_common_pages_remap(struct page **pages, size_t size,
+			unsigned long vm_flags, pgprot_t prot,
+			const void *caller)
+{
+	struct vm_struct *area;
+
+	area = __dma_common_pages_remap(pages, size, vm_flags, prot, caller);
+	if (!area)
+		return NULL;
+
+	area->pages = pages;
+
 	return area->addr;
 }
 
@@ -308,7 +321,7 @@ void *dma_common_contiguous_remap(struct page *page, size_t size,
 {
 	int i;
 	struct page **pages;
-	void *ptr;
+	struct vm_struct *area;
 	unsigned long pfn;
 
 	pages = kmalloc(sizeof(struct page *) << get_order(size), GFP_KERNEL);
@@ -318,11 +331,13 @@ void *dma_common_contiguous_remap(struct page *page, size_t size,
 	for (i = 0, pfn = page_to_pfn(page); i < (size >> PAGE_SHIFT); i++)
 		pages[i] = pfn_to_page(pfn + i);
 
-	ptr = dma_common_pages_remap(pages, size, vm_flags, prot, caller);
+	area = __dma_common_pages_remap(pages, size, vm_flags, prot, caller);
 
 	kfree(pages);
 
-	return ptr;
+	if (!area)
+		return NULL;
+	return area->addr;
 }
 
 /*

^ permalink raw reply related

* usb: dwc2: NMI watchdog: BUG: soft lockup - CPU#0 stuck for 146s
From: Stefan Wahren @ 2017-04-25 18:11 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1998517910.54108.1492894253010@email.1und1.de>

> Stefan Wahren <stefan.wahren@i2se.com> hat am 22. April 2017 um 22:50 geschrieben:
> 
> 
> Hi,
> 
> > Eric Anholt <eric@anholt.net> hat am 20. April 2017 um 20:54 geschrieben:
> > 
> > 
> > Stefan Wahren <stefan.wahren@i2se.com> writes:
> > 
> > > Hi,
> > >
> > >> Doug Anderson <dianders@chromium.org> hat am 18. April 2017 um 22:41 geschrieben:
> > >> 
> > >> 
> > >> It's hard to know for sure that all of this time is really in
> > >> urb_enqueue().  Possible we could have task switched out and been
> > >> blocked elsewhere.  Using ftrace to get more fine-grained timings
> > >> would be useful.  ktime_get(), ktime_sub(), and ktime_to_us() are your
> > >> friends here if you want to use trace_printk.
> > >
> > > i'm a newbie to ftrace, so i hope this would be helpful.
> > >
> > > # connect PL2303 to the onboard hub
> > > # echo 0 > options/sleep-time
> > > # echo 0 > function_profile_enabled
> > > # echo 1 > function_profile_enabled
> > > # ./usb_test
> > > # Waiting for at least 20 seconds and then disconnect PL2303
> > > # echo 0 > function_profile_enabled
> > > # cat trace_stat/function0
> > >
> > >   Function                               Hit    Time            Avg             s^2
> > >   --------                               ---    ----            ---             ---
> > >   bcm2835_handle_irq                  361347    219567633 us     607.636 us      1485199 us  
> > >   __handle_domain_irq                1082482    212639551 us     196.437 us      3642030 us  
> > >   generic_handle_irq                 1082482    100592051 us     92.927 us       50511334 us 
> > >   irq_exit                           1082482    98197771 us     90.715 us       29649040 us 
> > >   handle_level_irq                   1082482    95812379 us     88.511 us       51910093 us 
> > 
> > If I'm reading this output right, we're spending half of our interrupt
> > processing time in irq_exit(), so even if dwc2's interrupt was free (the
> > generic_handle_irq() chain), we'd be eating about half the CPU getting
> > back out of the interrupt handler, right?
> > 
> > I don't really know anything about DWC2 or USB, but is there any way we
> > could mitigate the interrupt frequency with this hardware?  If nothing
> > else, could we loop reading gintsts until it reads back 0?
> 
> first of all i updated my kernel to 4.11rc7 and the issue still occures. Today i had some time to investigate this issue further and i made some interesting observations:
> 
> 1. The lockup doesn't occure always after starting usb_test. In rare cases i was able to run the program without lockup.
> 2. In case the lockup occured we are always able to "rescue" the BCM2835 from this state by sending some serial data to the PL2303.

Based on this scenario i patched the interrupt routine to detect the interrupt storm and normal condition. So i can dump the global and host registers in both situations (bad and goodcase).

Here is the diff between both register dumps, maybe someone see something interesting:

--- badcase.txt	2017-04-25 18:02:59.000000000 +0000
+++ goodcase.txt	2017-04-25 18:02:59.000000000 +0000
@@ -4,12 +4,12 @@
  dwc2_dump_global_registers: GAHBCFG	 @0xCC850008 : 0x00000031
  dwc2_dump_global_registers: GUSBCFG	 @0xCC85000C : 0x20001700
  dwc2_dump_global_registers: GRSTCTL	 @0xCC850010 : 0x80000000
- dwc2_dump_global_registers: GINTSTS	 @0xCC850014 : 0x06000021
+ dwc2_dump_global_registers: GINTSTS	 @0xCC850014 : 0x06000029
  dwc2_dump_global_registers: GINTMSK	 @0xCC850018 : 0xF300080E
- dwc2_dump_global_registers: GRXSTSR	 @0xCC85001C : 0x00070044
+ dwc2_dump_global_registers: GRXSTSR	 @0xCC85001C : 0x000F0001
  dwc2_dump_global_registers: GRXFSIZ	 @0xCC850024 : 0x00000306
  dwc2_dump_global_registers: GNPTXFSIZ	 @0xCC850028 : 0x01000306
- dwc2_dump_global_registers: GNPTXSTS	 @0xCC85002C : 0x1D080100
+ dwc2_dump_global_registers: GNPTXSTS	 @0xCC85002C : 0x18080100
  dwc2_dump_global_registers: GI2CCTL	 @0xCC850030 : 0x00000000
  dwc2_dump_global_registers: GPVNDCTL	 @0xCC850034 : 0x00000000
  dwc2_dump_global_registers: GGPIO	 @0xCC850038 : 0x00000000
@@ -27,64 +27,64 @@
  dwc2_dump_host_registers: Host Global Registers
  dwc2_dump_host_registers: HCFG	 @0xCC850400 : 0x00000000
  dwc2_dump_host_registers: HFIR	 @0xCC850404 : 0x00001D4B
- dwc2_dump_host_registers: HFNUM	 @0xCC850408 : 0x0BF03CA1
- dwc2_dump_host_registers: HPTXSTS	 @0xCC850410 : 0x29080200
- dwc2_dump_host_registers: HAINT	 @0xCC850414 : 0x00000002
- dwc2_dump_host_registers: HAINTMSK	 @0xCC850418 : 0x00000002
+ dwc2_dump_host_registers: HFNUM	 @0xCC850408 : 0x029F3421
+ dwc2_dump_host_registers: HPTXSTS	 @0xCC850410 : 0x25080200
+ dwc2_dump_host_registers: HAINT	 @0xCC850414 : 0x00000001
+ dwc2_dump_host_registers: HAINTMSK	 @0xCC850418 : 0x00000001
  dwc2_dump_host_registers: HPRT0	 @0xCC850440 : 0x00001005
  dwc2_dump_host_registers: Host Channel 0 Specific Registers
- dwc2_dump_host_registers: HCCHAR	 @0xCC850500 : 0x01189840
- dwc2_dump_host_registers: HCSPLT	 @0xCC850504 : 0x8001C102
- dwc2_dump_host_registers: HCINT	 @0xCC850508 : 0x00000000
- dwc2_dump_host_registers: HCINTMSK	 @0xCC85050C : 0x00000000
- dwc2_dump_host_registers: HCTSIZ	 @0xCC850510 : 0x00080040
- dwc2_dump_host_registers: HCDMA	 @0xCC850514 : 0x4A07D300
+ dwc2_dump_host_registers: HCCHAR	 @0xCC850500 : 0x013C880A
+ dwc2_dump_host_registers: HCSPLT	 @0xCC850504 : 0x8000C102
+ dwc2_dump_host_registers: HCINT	 @0xCC850508 : 0x00000022
+ dwc2_dump_host_registers: HCINTMSK	 @0xCC85050C : 0x00000006
+ dwc2_dump_host_registers: HCTSIZ	 @0xCC850510 : 0x4008000A
+ dwc2_dump_host_registers: HCDMA	 @0xCC850514 : 0x4A093840
  dwc2_dump_host_registers: Host Channel 1 Specific Registers
  dwc2_dump_host_registers: HCCHAR	 @0xCC850520 : 0x01189840
  dwc2_dump_host_registers: HCSPLT	 @0xCC850524 : 0x8000C102
- dwc2_dump_host_registers: HCINT	 @0xCC850528 : 0x00000022
- dwc2_dump_host_registers: HCINTMSK	 @0xCC85052C : 0x00000006
+ dwc2_dump_host_registers: HCINT	 @0xCC850528 : 0x00000000
+ dwc2_dump_host_registers: HCINTMSK	 @0xCC85052C : 0x00000000
  dwc2_dump_host_registers: HCTSIZ	 @0xCC850530 : 0x00080040
- dwc2_dump_host_registers: HCDMA	 @0xCC850534 : 0x4A07D300
+ dwc2_dump_host_registers: HCDMA	 @0xCC850534 : 0x4A07D500
  dwc2_dump_host_registers: Host Channel 2 Specific Registers
  dwc2_dump_host_registers: HCCHAR	 @0xCC850540 : 0x01189840
  dwc2_dump_host_registers: HCSPLT	 @0xCC850544 : 0x8001C102
  dwc2_dump_host_registers: HCINT	 @0xCC850548 : 0x00000000
  dwc2_dump_host_registers: HCINTMSK	 @0xCC85054C : 0x00000000
  dwc2_dump_host_registers: HCTSIZ	 @0xCC850550 : 0x00080040
- dwc2_dump_host_registers: HCDMA	 @0xCC850554 : 0x4A07D300
+ dwc2_dump_host_registers: HCDMA	 @0xCC850554 : 0x4A07D500
  dwc2_dump_host_registers: Host Channel 3 Specific Registers
- dwc2_dump_host_registers: HCCHAR	 @0xCC850560 : 0x01189840
- dwc2_dump_host_registers: HCSPLT	 @0xCC850564 : 0x8001C102
+ dwc2_dump_host_registers: HCCHAR	 @0xCC850560 : 0x00D08040
+ dwc2_dump_host_registers: HCSPLT	 @0xCC850564 : 0x00000000
  dwc2_dump_host_registers: HCINT	 @0xCC850568 : 0x00000000
  dwc2_dump_host_registers: HCINTMSK	 @0xCC85056C : 0x00000000
- dwc2_dump_host_registers: HCTSIZ	 @0xCC850570 : 0x00080040
- dwc2_dump_host_registers: HCDMA	 @0xCC850574 : 0x4A07D300
+ dwc2_dump_host_registers: HCTSIZ	 @0xCC850570 : 0x8000003C
+ dwc2_dump_host_registers: HCDMA	 @0xCC850574 : 0x4A3B7544
  dwc2_dump_host_registers: Host Channel 4 Specific Registers
- dwc2_dump_host_registers: HCCHAR	 @0xCC850580 : 0x01189840
- dwc2_dump_host_registers: HCSPLT	 @0xCC850584 : 0x8001C102
+ dwc2_dump_host_registers: HCCHAR	 @0xCC850580 : 0x00D00040
+ dwc2_dump_host_registers: HCSPLT	 @0xCC850584 : 0x00000000
  dwc2_dump_host_registers: HCINT	 @0xCC850588 : 0x00000000
  dwc2_dump_host_registers: HCINTMSK	 @0xCC85058C : 0x00000000
- dwc2_dump_host_registers: HCTSIZ	 @0xCC850590 : 0x00080040
- dwc2_dump_host_registers: HCDMA	 @0xCC850594 : 0x4A07D300
+ dwc2_dump_host_registers: HCTSIZ	 @0xCC850590 : 0x20000008
+ dwc2_dump_host_registers: HCDMA	 @0xCC850594 : 0x4A3B74C8
  dwc2_dump_host_registers: Host Channel 5 Specific Registers
  dwc2_dump_host_registers: HCCHAR	 @0xCC8505A0 : 0x01189840
  dwc2_dump_host_registers: HCSPLT	 @0xCC8505A4 : 0x8000C102
  dwc2_dump_host_registers: HCINT	 @0xCC8505A8 : 0x00000000
  dwc2_dump_host_registers: HCINTMSK	 @0xCC8505AC : 0x00000000
  dwc2_dump_host_registers: HCTSIZ	 @0xCC8505B0 : 0x00080040
- dwc2_dump_host_registers: HCDMA	 @0xCC8505B4 : 0x4A07D300
+ dwc2_dump_host_registers: HCDMA	 @0xCC8505B4 : 0x4A07D500
  dwc2_dump_host_registers: Host Channel 6 Specific Registers
- dwc2_dump_host_registers: HCCHAR	 @0xCC8505C0 : 0x01189840
- dwc2_dump_host_registers: HCSPLT	 @0xCC8505C4 : 0x8001C102
+ dwc2_dump_host_registers: HCCHAR	 @0xCC8505C0 : 0x00D00040
+ dwc2_dump_host_registers: HCSPLT	 @0xCC8505C4 : 0x00000000
  dwc2_dump_host_registers: HCINT	 @0xCC8505C8 : 0x00000000
  dwc2_dump_host_registers: HCINTMSK	 @0xCC8505CC : 0x00000000
- dwc2_dump_host_registers: HCTSIZ	 @0xCC8505D0 : 0x00080040
- dwc2_dump_host_registers: HCDMA	 @0xCC8505D4 : 0x4A07D300
+ dwc2_dump_host_registers: HCTSIZ	 @0xCC8505D0 : 0x00000000
+ dwc2_dump_host_registers: HCDMA	 @0xCC8505D4 : 0x4B080000
  dwc2_dump_host_registers: Host Channel 7 Specific Registers
- dwc2_dump_host_registers: HCCHAR	 @0xCC8505E0 : 0x01189840
+ dwc2_dump_host_registers: HCCHAR	 @0xCC8505E0 : 0x013C880A
  dwc2_dump_host_registers: HCSPLT	 @0xCC8505E4 : 0x8001C102
  dwc2_dump_host_registers: HCINT	 @0xCC8505E8 : 0x00000000
  dwc2_dump_host_registers: HCINTMSK	 @0xCC8505EC : 0x00000000
- dwc2_dump_host_registers: HCTSIZ	 @0xCC8505F0 : 0x00080040
- dwc2_dump_host_registers: HCDMA	 @0xCC8505F4 : 0x4A07D300
+ dwc2_dump_host_registers: HCTSIZ	 @0xCC8505F0 : 0x4008000A
+ dwc2_dump_host_registers: HCDMA	 @0xCC8505F4 : 0x4A093840

^ permalink raw reply

* [PATCH v2] crypto: arm64/sha: Add constant operand modifier to ASM_EXPORT
From: Ard Biesheuvel @ 2017-04-25 18:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170425173937.GN128305@google.com>

On 25 April 2017 at 18:39, Matthias Kaehlcke <mka@chromium.org> wrote:
> Hi,
>
> El Tue, Apr 18, 2017 at 04:35:02PM +0100 Ard Biesheuvel ha dit:
>
>> On 18 April 2017 at 15:47, Paul Gortmaker <paul.gortmaker@windriver.com> wrote:
>> > On Wed, Apr 5, 2017 at 2:34 PM, Matthias Kaehlcke <mka@chromium.org> wrote:
>> >> The operand is an integer constant, make the constness explicit by
>> >> adding the modifier. This is needed for clang to generate valid code
>> >> and also works with gcc.
>> >
>> > Actually it doesn't work with all gcc.  I've got an older arm64 toolchain that I
>> > only use for syntax checking (and hence I don't care if it is the latest and
>> > greatest) and this commit breaks it:
>> >
>> > arch/arm64/crypto/sha1-ce-glue.c:21:2: error: invalid 'asm': invalid
>> > operand prefix '%c'
>> >   asm(".globl " #sym "; .set " #sym ", %c0" :: "i"(val));
>> >
>> > I'm currently reverting this change locally so I can continue to use the old
>> > toolchain:
>> >
>> > $ aarch64-linux-gnu-gcc --version
>> > aarch64-linux-gnu-gcc (crosstool-NG linaro-1.13.1-4.8-2013.12 - Linaro
>> > GCC 2013.11) 4.8.3 20131202 (prerelease)
>> > Copyright (C) 2013 Free Software Foundation, Inc.
>> >
>> > $ aarch64-linux-gnu-as --version
>> > GNU assembler (crosstool-NG linaro-1.13.1-4.8-2013.12 - Linaro GCC
>> > 2013.11) 2.24.0.20131220
>> > Copyright 2013 Free Software Foundation, Inc.
>> >
>> > Maybe it is finally too old and nobody cares, but I thought it worth a mention.
>> >
>>
>> Thanks for the report. I think we care more about GCC 4.8 than about
>> Clang, which argues for reverting this patch.
>>
>> I understand these issues must be frustrating if you are working on
>> this stuff, but to me, it is not entirely obvious why we want to
>> support Clang in the first place (i.e., what does it buy you if your
>> distro/environment is not already using Clang for userland), and why
>> the burden is on Linux to make modifications to support Clang,
>> especially when it comes to GCC extensions such as inline assembly
>> syntax.
>>
>> It is ultimately up to the maintainers to decide what to do with this
>> patch, but my vote would be to revert it, especially given that the %c
>> placeholder prefix is not documented anywhere, and appears to simply
>> trigger some GCC internals that happen to do the right thing in this
>> case.
>>
>> However, the I -> i change is arguably an improvement, and considering
>> that the following
>>
>> asm("foo: .long %0" :: "i"(some value))
>>
>> doesn't compile with clang either, I suggest you (Matthias) file a bug
>> against Clang to get this fixed, and we can propose another patch just
>> for the I->i change.
>
> I consulted with folks with more expertise in this area than myself.
> This is their analysis of the situation:
>
> "The ARM ARM specifies that the correct AArch64 instruction assembly
> syntax is to have a hash sign (#) before an immediate.
>

It does not specify that at all:

"""
The A64 assembly language does not require the # character to
introduce constant immediate operands, but an assembler must allow
immediate values introduced with or without the # character. ARM
recommends that an A64 disassembler outputs a # before an immediate
operand.
"""
(ARM DDI 0487A.g page C1-121)

IOW, it only /recommends/ the # sign for *dis*assemblers. Big difference.

> Therefore, every time an inline assembly constraint is used that
> specifies to print an immediate (like 'i' or 'I'), the immediate
> (e.g. 42) should be printed with the hash (e.g. #42).
>
> Therefore, if you're using an immediate constraint where the hash sign
> must not be printed, you have to use the "c" operand modifier. The "c"
> operand modifier apparently got introduced to gcc after the 4.8
> release.
>

My problem with the %c modifier is that it is completely undocumented,
and appears in an internal GCC code generation code path. IOW, the GCC
developers could also remove it at any time (although this is highly
unlikely, of course)

> The binutils assembler and the clang integrated assembler accept
> immediates without the hash sign as a non-official extension.

Nope. *That* is mandated by the ARM ARM, see above.

> Some of
> the immediate constraints on gcc seem to not print out the hash sign
> either; which is why the variant in the linux kernel works with gcc.
>

Yes, and since it is perfectly legal for the "i" constraint not to
have a #, I don't understand what the big deal is tbh.

> In summary, it seems to me that the inline assembly with the %c0
> operand is the correct one and gcc 4.8 is simply too old to support
> this."
>

OK, so we're back to having to choose between GCC 4.8 and Clang.

> If the above is correct it seems that the solution is not to "fix"
> clang, but to use different instructions for gcc<=4.8 and newer
> compilers. I am aware that this is not a popular option.
>
> What do you think?
>

Perhaps I should just rework the code not to rely on inline asm at all.

I will take a look,

^ permalink raw reply

* [RFC PATCH] drivers: dma-mapping: Do not attempt to create a scatterlist for from_coherent buffers
From: Russell King - ARM Linux @ 2017-04-25 18:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1493143059-2113-1-git-send-email-catalin.marinas@arm.com>

On Tue, Apr 25, 2017 at 06:57:39PM +0100, Catalin Marinas wrote:
> Memory returned by dma_alloc_from_coherent() is not backed by struct
> page and creating a scatterlist would use invalid page pointers. The
> patch introduces the dma_vaddr_from_coherent() function and the
> corresponding check in dma_get_sgtable_attrs().
> 
> Fixes: d2b7428eb0ca ("common: dma-mapping: introduce dma_get_sgtable() function")
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Russell King - ARM Linux <linux@arm.linux.org.uk>
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> ---
> 
> In a recent discussion around the iommu DMA ops on arm64, Russell
> pointed out that dma_get_sgtable is not safe since the coherent DMA
> memory is not always backed by struct page. Russell has queued an
> arm-specific patch checking for pfn_valid() but I thought I'd make a
> more generic fix. This patch aims to bring the dma_get_sgtable() API in
> line with the dma_alloc/mmap/free with respect to the from_coherent
> memory.

Sorry, I don't think this is the correct approach.

You're assuming that 'vaddr' will always be a valid lowmem address.
That isn't always the case - some dma coherent allocations provide
remapped memory.

The reason for dma_get_sgtable() existing is to coerce the DMA
coherent memory into a scatterlist so that it can be passed through
the dma_buf API - that's where the problem lies.  The dma_buf API
needs fixing so that coherent memory can be sanely passed, and
dma_get_sgtable() needs to be put out of its misery.

-- 
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] arm64: Fix the DMA mmap and get_sgtable API with DMA_ATTR_FORCE_CONTIGUOUS
From: Catalin Marinas @ 2017-04-25 18:00 UTC (permalink / raw)
  To: linux-arm-kernel

While honouring the DMA_ATTR_FORCE_CONTIGUOUS on arm64 (commit
44176bb38fa4: "arm64: Add support for DMA_ATTR_FORCE_CONTIGUOUS to
IOMMU"), the existing uses of dma_mmap_attrs() and dma_get_sgtable()
have been broken by passing a physically contiguous vm_struct with an
invalid pages pointer through the common iommu API.

Since the coherent allocation with DMA_ATTR_FORCE_CONTIGUOUS uses CMA,
this patch simply reuses the existing swiotlb logic for mmap and
get_sgtable.

Note that the current implementation of get_sgtable (both swiotlb and
iommu) is broken if dma_declare_coherent_memory() is used since such
memory does not have a corresponding struct page. To be addressed in a
separate patch.

Fixes: 44176bb38fa4 ("arm64: Add support for DMA_ATTR_FORCE_CONTIGUOUS to IOMMU")
Reported-by: Andrzej Hajda <a.hajda@samsung.com>
Cc: Andrzej Hajda <a.hajda@samsung.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---

Here's my attempt of fixing this, though I'd like to wait for Robin's review
(who's currently on holiday until next week).

Not tested.

 arch/arm64/mm/dma-mapping.c | 65 ++++++++++++++++++++++++++++++++++-----------
 1 file changed, 49 insertions(+), 16 deletions(-)

diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index f7b54019ef55..c9e53dec3695 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -308,24 +308,15 @@ static void __swiotlb_sync_sg_for_device(struct device *dev,
 				       sg->length, dir);
 }
 
-static int __swiotlb_mmap(struct device *dev,
-			  struct vm_area_struct *vma,
-			  void *cpu_addr, dma_addr_t dma_addr, size_t size,
-			  unsigned long attrs)
+static int __swiotlb_mmap_pfn(struct vm_area_struct *vma,
+			      unsigned long pfn, size_t size)
 {
 	int ret = -ENXIO;
 	unsigned long nr_vma_pages = (vma->vm_end - vma->vm_start) >>
 					PAGE_SHIFT;
 	unsigned long nr_pages = PAGE_ALIGN(size) >> PAGE_SHIFT;
-	unsigned long pfn = dma_to_phys(dev, dma_addr) >> PAGE_SHIFT;
 	unsigned long off = vma->vm_pgoff;
 
-	vma->vm_page_prot = __get_dma_pgprot(attrs, vma->vm_page_prot,
-					     is_device_dma_coherent(dev));
-
-	if (dma_mmap_from_coherent(dev, vma, cpu_addr, size, &ret))
-		return ret;
-
 	if (off < nr_pages && nr_vma_pages <= (nr_pages - off)) {
 		ret = remap_pfn_range(vma, vma->vm_start,
 				      pfn + off,
@@ -336,19 +327,43 @@ static int __swiotlb_mmap(struct device *dev,
 	return ret;
 }
 
-static int __swiotlb_get_sgtable(struct device *dev, struct sg_table *sgt,
-				 void *cpu_addr, dma_addr_t handle, size_t size,
-				 unsigned long attrs)
+static int __swiotlb_mmap(struct device *dev,
+			  struct vm_area_struct *vma,
+			  void *cpu_addr, dma_addr_t dma_addr, size_t size,
+			  unsigned long attrs)
+{
+	int ret;
+	unsigned long pfn = dma_to_phys(dev, dma_addr) >> PAGE_SHIFT;
+
+	vma->vm_page_prot = __get_dma_pgprot(attrs, vma->vm_page_prot,
+					     is_device_dma_coherent(dev));
+
+	if (dma_mmap_from_coherent(dev, vma, cpu_addr, size, &ret))
+		return ret;
+
+	return __swiotlb_mmap_pfn(vma, pfn, size);
+}
+
+static int __swiotlb_get_sgtable_page(struct sg_table *sgt,
+				      struct page *page, size_t size)
 {
 	int ret = sg_alloc_table(sgt, 1, GFP_KERNEL);
 
 	if (!ret)
-		sg_set_page(sgt->sgl, phys_to_page(dma_to_phys(dev, handle)),
-			    PAGE_ALIGN(size), 0);
+		sg_set_page(sgt->sgl, page, PAGE_ALIGN(size), 0);
 
 	return ret;
 }
 
+static int __swiotlb_get_sgtable(struct device *dev, struct sg_table *sgt,
+				 void *cpu_addr, dma_addr_t handle, size_t size,
+				 unsigned long attrs)
+{
+	struct page *page = phys_to_page(dma_to_phys(dev, handle));
+
+	return __swiotlb_get_sgtable_page(sgt, page, size);
+}
+
 static int __swiotlb_dma_supported(struct device *hwdev, u64 mask)
 {
 	if (swiotlb)
@@ -703,6 +718,15 @@ static int __iommu_mmap_attrs(struct device *dev, struct vm_area_struct *vma,
 	if (dma_mmap_from_coherent(dev, vma, cpu_addr, size, &ret))
 		return ret;
 
+	if (attrs & DMA_ATTR_FORCE_CONTIGUOUS) {
+		/*
+		 * DMA_ATTR_FORCE_CONTIGUOUS allocations are always remapped,
+		 * hence in the vmalloc space.
+		 */
+		unsigned long pfn = vmalloc_to_pfn(cpu_addr);
+		return __swiotlb_mmap_pfn(vma, pfn, size);
+	}
+
 	area = find_vm_area(cpu_addr);
 	if (WARN_ON(!area || !area->pages))
 		return -ENXIO;
@@ -717,6 +741,15 @@ static int __iommu_get_sgtable(struct device *dev, struct sg_table *sgt,
 	unsigned int count = PAGE_ALIGN(size) >> PAGE_SHIFT;
 	struct vm_struct *area = find_vm_area(cpu_addr);
 
+	if (attrs & DMA_ATTR_FORCE_CONTIGUOUS) {
+		/*
+		 * DMA_ATTR_FORCE_CONTIGUOUS allocations are always remapped,
+		 * hence in the vmalloc space.
+		 */
+		struct page *page = vmalloc_to_page(cpu_addr);
+		return __swiotlb_get_sgtable_page(sgt, page, size);
+	}
+
 	if (WARN_ON(!area || !area->pages))
 		return -ENXIO;
 

^ permalink raw reply related

* [RFC PATCH] drivers: dma-mapping: Do not attempt to create a scatterlist for from_coherent buffers
From: Catalin Marinas @ 2017-04-25 17:57 UTC (permalink / raw)
  To: linux-arm-kernel

Memory returned by dma_alloc_from_coherent() is not backed by struct
page and creating a scatterlist would use invalid page pointers. The
patch introduces the dma_vaddr_from_coherent() function and the
corresponding check in dma_get_sgtable_attrs().

Fixes: d2b7428eb0ca ("common: dma-mapping: introduce dma_get_sgtable() function")
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Russell King - ARM Linux <linux@arm.linux.org.uk>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---

In a recent discussion around the iommu DMA ops on arm64, Russell
pointed out that dma_get_sgtable is not safe since the coherent DMA
memory is not always backed by struct page. Russell has queued an
arm-specific patch checking for pfn_valid() but I thought I'd make a
more generic fix. This patch aims to bring the dma_get_sgtable() API in
line with the dma_alloc/mmap/free with respect to the from_coherent
memory.

 drivers/base/dma-coherent.c | 9 +++++++++
 include/linux/dma-mapping.h | 5 +++++
 2 files changed, 14 insertions(+)

diff --git a/drivers/base/dma-coherent.c b/drivers/base/dma-coherent.c
index 640a7e63c453..1d2cdbefb850 100644
--- a/drivers/base/dma-coherent.c
+++ b/drivers/base/dma-coherent.c
@@ -279,6 +279,15 @@ int dma_mmap_from_coherent(struct device *dev, struct vm_area_struct *vma,
 }
 EXPORT_SYMBOL(dma_mmap_from_coherent);
 
+int dma_vaddr_from_coherent(struct device *dev, void *vaddr, size_t size)
+{
+	struct dma_coherent_mem *mem = dev ? dev->dma_mem : NULL;
+
+	return mem && vaddr >= mem->virt_base &&
+		vaddr + size <= (mem->virt_base + (mem->size << PAGE_SHIFT));
+}
+EXPORT_SYMBOL(dma_vaddr_from_coherent);
+
 /*
  * Support for reserved memory regions defined in device tree
  */
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 0977317c6835..4dc99c6db184 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -164,10 +164,12 @@ int dma_release_from_coherent(struct device *dev, int order, void *vaddr);
 
 int dma_mmap_from_coherent(struct device *dev, struct vm_area_struct *vma,
 			    void *cpu_addr, size_t size, int *ret);
+int dma_vaddr_from_coherent(struct device *dev, void *vaddr, size_t size);
 #else
 #define dma_alloc_from_coherent(dev, size, handle, ret) (0)
 #define dma_release_from_coherent(dev, order, vaddr) (0)
 #define dma_mmap_from_coherent(dev, vma, vaddr, order, ret) (0)
+#define dma_vaddr_from_coherent(dev, vaddr, size) (0)
 #endif /* CONFIG_HAVE_GENERIC_DMA_COHERENT */
 
 #ifdef CONFIG_HAS_DMA
@@ -461,6 +463,9 @@ dma_get_sgtable_attrs(struct device *dev, struct sg_table *sgt, void *cpu_addr,
 {
 	const struct dma_map_ops *ops = get_dma_ops(dev);
 	BUG_ON(!ops);
+	/* dma_alloc_from_coherent() memory is not backed by struct page */
+	if (dma_vaddr_from_coherent(dev, cpu_addr, size))
+		return -ENXIO;
 	if (ops->get_sgtable)
 		return ops->get_sgtable(dev, sgt, cpu_addr, dma_addr, size,
 					attrs);

^ permalink raw reply related

* [PATCH] ARM/shmem: Drop page coloring align for non-VIPT CPUs
From: Dmitry Safonov @ 2017-04-25 17:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170425173546.GS17774@n2100.armlinux.org.uk>

On 04/25/2017 08:35 PM, Russell King - ARM Linux wrote:
> On Tue, Apr 25, 2017 at 08:19:21PM +0300, Dmitry Safonov wrote:
>> On 04/14/2017 01:09 PM, Dmitry Safonov wrote:
>>> On ARMv6 CPUs with VIPT caches there are aliasing issues: if two
>>> different cache line indexes correspond to the same physical
>>> address, then changes made to one of the alias might be lost
>>> or they can overwrite each other. To overcome aliasing issues,
>>> the align for shared mappings was introduced with:
>>>
>>> commit 4197692eef113eeb8e3e413cc70993a5e667e5b8
>>> Author: Russell King <rmk@flint.arm.linux.org.uk>
>>> Date:   Wed Apr 28 22:22:33 2004 +0100
>>>
>>>      [ARM] Fix shared mmap()ings for ARM VIPT caches.
>>>
>>>      This allows us to appropriately align shared mappings on VIPT caches
>>>      with aliasing issues.
>>>
>>> Which introduced 4 pages align with SHMLBA, which resulted in
>>> unique physical address after any tag in cache (because two upper bits
>>> corresponding to page address get unused in tags).
>>>
>>> As this workaround is not needed by non-VIPT caches (like most armv7
>>> CPUs which have PIPT caches), ARM mmap() code checks if cache is VIPT
>>> aliasing for MAP_SHARED.
>>>
>>> The problem here is in shmat() syscall:
>>> 1. if shmaddr is NULL then do_shmat() uses arch_get_unmapped_area()
>>>     to allocate shared mapping.
>>> 2. if shmaddr is specified then do_shmat() checks that address has
>>>     SHMLBA alignment regardless to CPU cache aliasing.
>>>
>>> Which results on ARMv7 CPUs that shmat() with NULL shmaddr may return
>>> non-SHMLBA aligned address (page-aligned), but shmat() with the same
>>> address will fail.
>>>
>>> That is not critical issue for CRIU as after shmat() with NULL address,
> 
> CRIU?  Please try to keep use of acronyms to a minimum.

Ok.

> 
>>> we can mremap() resulted shmem to restore shared memory mappings on the
>>> same address where they were on checkpointing.
>>> But it's still worth fixing because we can't reliably tell from
>>> userspace if the platform has VIPT cache, and so this mremap()
>>> workaround is done with HUGE warning that restoring application, that
>>> uses SHMBLA-unaligned shmem on ARMv6 CPU with VIPT cache may result
>>> in data corruptions.
>>>
>>> I also changed SHMLBA build-time check to init-time WARN_ON(), as
>>> it's not constant afterward.
> 
> I'm not happy with this.  SHMLBA is defined by POSIX to be a constant,
> which means that if we want to have any kind of binary compatibility
> between different architecture versions, SHMLBA must be constant across
> all variants of the architecture.
> 
> Making it dependent on the cache architecture means that userspace's
> assumptions can be broken.  Increasing it is not an issue (since SHMLBA
> is defined to be the address multiple - an address that is aligned to
> 4-page is also by definition aligned to 1-page.)  So what I did back in
> 2004 wasn't a problem.
> 
> However, reducing it (as you're now suggesting) is - newly built programs
> are built today with:
> 
> #define SHMLBA              (__getpagesize () << 2)
> 
> so we must not allow the kernel to return addresses that violate that.
> As I say, we can't reduce SHMLBA now.

Thanks for the reply!
Hmm, so what do you think if we align then shmat(smid, NULL, shmflg)
allocations also? (with 0 == shmaddr)

Something like below:

--- >8 ---
diff --git a/arch/arm/mm/mmap.c b/arch/arm/mm/mmap.c
index 2239fde10b80..ac52f066f47f 100644
--- a/arch/arm/mm/mmap.c
+++ b/arch/arm/mm/mmap.c
@@ -59,21 +59,15 @@ arch_get_unmapped_area(struct file *filp, unsigned 
long addr,
  	struct mm_struct *mm = current->mm;
  	struct vm_area_struct *vma;
  	int do_align = 0;
-	int aliasing = cache_is_vipt_aliasing();
  	struct vm_unmapped_area_info info;

-	/*
-	 * We only need to do colour alignment if either the I or D
-	 * caches alias.
-	 */
-	if (aliasing)
-		do_align = filp || (flags & MAP_SHARED);
+	do_align = filp || (flags & MAP_SHARED);

  	/*
  	 * We enforce the MAP_FIXED case.
  	 */
  	if (flags & MAP_FIXED) {
-		if (aliasing && flags & MAP_SHARED &&
+		if (flags & MAP_SHARED &&
  		    (addr - (pgoff << PAGE_SHIFT)) & (SHMLBA - 1))
  			return -EINVAL;
  		return addr;
@@ -112,22 +106,16 @@ arch_get_unmapped_area_topdown(struct file *filp, 
const unsigned long addr0,
  	struct mm_struct *mm = current->mm;
  	unsigned long addr = addr0;
  	int do_align = 0;
-	int aliasing = cache_is_vipt_aliasing();
  	struct vm_unmapped_area_info info;

-	/*
-	 * We only need to do colour alignment if either the I or D
-	 * caches alias.
-	 */
-	if (aliasing)
-		do_align = filp || (flags & MAP_SHARED);
+	do_align = filp || (flags & MAP_SHARED);

  	/* requested length too big for entire address space */
  	if (len > TASK_SIZE)
  		return -ENOMEM;

  	if (flags & MAP_FIXED) {
-		if (aliasing && flags & MAP_SHARED &&
+		if (flags & MAP_SHARED &&
  		    (addr - (pgoff << PAGE_SHIFT)) & (SHMLBA - 1))
  			return -EINVAL;
  		return addr;

^ permalink raw reply related

* [PATCH V15 06/11] acpi: apei: handle SEA notification type for ARMv8
From: Borislav Petkov @ 2017-04-25 17:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <6d7249d3-e0b2-afa7-b5fa-8e85c40d8a13@codeaurora.org>

On Tue, Apr 25, 2017 at 11:41:39AM -0600, Baicar, Tyler wrote:
> I originally had this as a notifier, but Will requested to remove the
> notifier. That conversation is here: https://lkml.org/lkml/2017/1/18/1018

Yeah, he mentioned on IRC. I just think notifiers would be the cleaner
thing but whatever you guys say.

Just we had a nasty hack on x86 which I got rid of recently:

https://lkml.kernel.org/r/20170406090634.30950-1-bp at alien8.de

and I wouldn't want you guys to have the same "fun". :)

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

^ permalink raw reply

* [PATCH V15 06/11] acpi: apei: handle SEA notification type for ARMv8
From: Baicar, Tyler @ 2017-04-25 17:41 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170425172154.ggg56j52inf3522e@pd.tnic>

On 4/25/2017 11:21 AM, Borislav Petkov wrote:
> On Tue, Apr 18, 2017 at 05:05:18PM -0600, Tyler Baicar wrote:
>> ARM APEI extension proposal added SEA (Synchronous External Abort)
>> notification type for ARMv8.
>> Add a new GHES error source handling function for SEA. If an error
>> source's notification type is SEA, then this function can be registered
>> into the SEA exception handler. That way GHES will parse and report
>> SEA exceptions when they occur.
>> An SEA can interrupt code that had interrupts masked and is treated as
>> an NMI. To aid this the page of address space for mapping APEI buffers
>> while in_nmi() is always reserved, and ghes_ioremap_pfn_nmi() is
>> changed to use the helper methods to find the prot_t to map with in
>> the same way as ghes_ioremap_pfn_irq().
> ...
>
>> @@ -518,6 +520,17 @@ static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs)
>>   	pr_err("Synchronous External Abort: %s (0x%08x) at 0x%016lx\n",
>>   		inf->name, esr, addr);
>>   
>> +	/*
>> +	 * Synchronous aborts may interrupt code which had interrupts masked.
>> +	 * Before calling out into the wider kernel tell the interested
>> +	 * subsystems.
>> +	 */
>> +	if (IS_ENABLED(CONFIG_ACPI_APEI_SEA)) {
>> +		nmi_enter();
>> +		ghes_notify_sea();
>> +		nmi_exit();
>> +	}
> Well, the other GHES notification methods use a notifier:
> ghes_notify_sci, ghes_notify_nmi. You probably should do that too
> instead of calling straight into a driver from arch code.
I originally had this as a notifier, but Will requested to remove the 
notifier. That conversation is here: https://lkml.org/lkml/2017/1/18/1018

Thanks,
Tyler

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

^ permalink raw reply

* [PATCH v2] crypto: arm64/sha: Add constant operand modifier to ASM_EXPORT
From: Matthias Kaehlcke @ 2017-04-25 17:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAKv+Gu_DHBT=2YA9h+ao3pZXgEV5KsO9ia+J90wtLuDX=a4+eg@mail.gmail.com>

Hi,

El Tue, Apr 18, 2017 at 04:35:02PM +0100 Ard Biesheuvel ha dit:

> On 18 April 2017 at 15:47, Paul Gortmaker <paul.gortmaker@windriver.com> wrote:
> > On Wed, Apr 5, 2017 at 2:34 PM, Matthias Kaehlcke <mka@chromium.org> wrote:
> >> The operand is an integer constant, make the constness explicit by
> >> adding the modifier. This is needed for clang to generate valid code
> >> and also works with gcc.
> >
> > Actually it doesn't work with all gcc.  I've got an older arm64 toolchain that I
> > only use for syntax checking (and hence I don't care if it is the latest and
> > greatest) and this commit breaks it:
> >
> > arch/arm64/crypto/sha1-ce-glue.c:21:2: error: invalid 'asm': invalid
> > operand prefix '%c'
> >   asm(".globl " #sym "; .set " #sym ", %c0" :: "i"(val));
> >
> > I'm currently reverting this change locally so I can continue to use the old
> > toolchain:
> >
> > $ aarch64-linux-gnu-gcc --version
> > aarch64-linux-gnu-gcc (crosstool-NG linaro-1.13.1-4.8-2013.12 - Linaro
> > GCC 2013.11) 4.8.3 20131202 (prerelease)
> > Copyright (C) 2013 Free Software Foundation, Inc.
> >
> > $ aarch64-linux-gnu-as --version
> > GNU assembler (crosstool-NG linaro-1.13.1-4.8-2013.12 - Linaro GCC
> > 2013.11) 2.24.0.20131220
> > Copyright 2013 Free Software Foundation, Inc.
> >
> > Maybe it is finally too old and nobody cares, but I thought it worth a mention.
> >
> 
> Thanks for the report. I think we care more about GCC 4.8 than about
> Clang, which argues for reverting this patch.
> 
> I understand these issues must be frustrating if you are working on
> this stuff, but to me, it is not entirely obvious why we want to
> support Clang in the first place (i.e., what does it buy you if your
> distro/environment is not already using Clang for userland), and why
> the burden is on Linux to make modifications to support Clang,
> especially when it comes to GCC extensions such as inline assembly
> syntax.
> 
> It is ultimately up to the maintainers to decide what to do with this
> patch, but my vote would be to revert it, especially given that the %c
> placeholder prefix is not documented anywhere, and appears to simply
> trigger some GCC internals that happen to do the right thing in this
> case.
> 
> However, the I -> i change is arguably an improvement, and considering
> that the following
> 
> asm("foo: .long %0" :: "i"(some value))
> 
> doesn't compile with clang either, I suggest you (Matthias) file a bug
> against Clang to get this fixed, and we can propose another patch just
> for the I->i change.

I consulted with folks with more expertise in this area than myself.
This is their analysis of the situation:

"The ARM ARM specifies that the correct AArch64 instruction assembly
syntax is to have a hash sign (#) before an immediate.

Therefore, every time an inline assembly constraint is used that
specifies to print an immediate (like 'i' or 'I'), the immediate
(e.g. 42) should be printed with the hash (e.g. #42).

Therefore, if you're using an immediate constraint where the hash sign
must not be printed, you have to use the "c" operand modifier. The "c"
operand modifier apparently got introduced to gcc after the 4.8
release.

The binutils assembler and the clang integrated assembler accept
immediates without the hash sign as a non-official extension. Some of
the immediate constraints on gcc seem to not print out the hash sign
either; which is why the variant in the linux kernel works with gcc.

In summary, it seems to me that the inline assembly with the %c0
operand is the correct one and gcc 4.8 is simply too old to support
this."

If the above is correct it seems that the solution is not to "fix"
clang, but to use different instructions for gcc<=4.8 and newer
compilers. I am aware that this is not a popular option.

What do you think?

Thanks

Matthias

^ permalink raw reply

* [PATCH] ARM/shmem: Drop page coloring align for non-VIPT CPUs
From: Russell King - ARM Linux @ 2017-04-25 17:35 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <571024bf-892d-4d08-dd9e-654b1e28c23e@virtuozzo.com>

On Tue, Apr 25, 2017 at 08:19:21PM +0300, Dmitry Safonov wrote:
> On 04/14/2017 01:09 PM, Dmitry Safonov wrote:
> >On ARMv6 CPUs with VIPT caches there are aliasing issues: if two
> >different cache line indexes correspond to the same physical
> >address, then changes made to one of the alias might be lost
> >or they can overwrite each other. To overcome aliasing issues,
> >the align for shared mappings was introduced with:
> >
> >commit 4197692eef113eeb8e3e413cc70993a5e667e5b8
> >Author: Russell King <rmk@flint.arm.linux.org.uk>
> >Date:   Wed Apr 28 22:22:33 2004 +0100
> >
> >     [ARM] Fix shared mmap()ings for ARM VIPT caches.
> >
> >     This allows us to appropriately align shared mappings on VIPT caches
> >     with aliasing issues.
> >
> >Which introduced 4 pages align with SHMLBA, which resulted in
> >unique physical address after any tag in cache (because two upper bits
> >corresponding to page address get unused in tags).
> >
> >As this workaround is not needed by non-VIPT caches (like most armv7
> >CPUs which have PIPT caches), ARM mmap() code checks if cache is VIPT
> >aliasing for MAP_SHARED.
> >
> >The problem here is in shmat() syscall:
> >1. if shmaddr is NULL then do_shmat() uses arch_get_unmapped_area()
> >    to allocate shared mapping.
> >2. if shmaddr is specified then do_shmat() checks that address has
> >    SHMLBA alignment regardless to CPU cache aliasing.
> >
> >Which results on ARMv7 CPUs that shmat() with NULL shmaddr may return
> >non-SHMLBA aligned address (page-aligned), but shmat() with the same
> >address will fail.
> >
> >That is not critical issue for CRIU as after shmat() with NULL address,

CRIU?  Please try to keep use of acronyms to a minimum.

> >we can mremap() resulted shmem to restore shared memory mappings on the
> >same address where they were on checkpointing.
> >But it's still worth fixing because we can't reliably tell from
> >userspace if the platform has VIPT cache, and so this mremap()
> >workaround is done with HUGE warning that restoring application, that
> >uses SHMBLA-unaligned shmem on ARMv6 CPU with VIPT cache may result
> >in data corruptions.
> >
> >I also changed SHMLBA build-time check to init-time WARN_ON(), as
> >it's not constant afterward.

I'm not happy with this.  SHMLBA is defined by POSIX to be a constant,
which means that if we want to have any kind of binary compatibility
between different architecture versions, SHMLBA must be constant across
all variants of the architecture.

Making it dependent on the cache architecture means that userspace's
assumptions can be broken.  Increasing it is not an issue (since SHMLBA
is defined to be the address multiple - an address that is aligned to
4-page is also by definition aligned to 1-page.)  So what I did back in
2004 wasn't a problem.

However, reducing it (as you're now suggesting) is - newly built programs
are built today with:

#define SHMLBA              (__getpagesize () << 2)

so we must not allow the kernel to return addresses that violate that.
As I say, we can't reduce SHMLBA now.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently@9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply

* [patch V2 00/24] cpu/hotplug: Convert get_online_cpus() to a percpu_rwsem
From: Sebastian Siewior @ 2017-04-25 17:28 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170425161037.GA27156@leverpostej>

On 2017-04-25 17:10:37 [+0100], Mark Rutland wrote:
> Hi,
Hi,

> When we bring the secondary CPU online, we detect an erratum that wasn't
> present on the boot CPU, and try to enable a static branch we use to
> track the erratum. The call to static_branch_enable() blows up as above.

this (cpus_set_cap()) seems only to be used used in CPU up part.

> I see that we now have static_branch_disable_cpuslocked(), but we don't
> have an equivalent for enable. I'm not sure what we should be doing
> here.

We should introduce static_branch_enable_cpuslocked(). Does this work
for you (after s/static_branch_enable/static_branch_enable_cpuslocked/
in cpus_set_cap()) ?:

>From fe004351e4649da037d5165247e89ab976fe4a26 Mon Sep 17 00:00:00 2001
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Date: Tue, 25 Apr 2017 18:43:00 +0200
Subject: [PATCH] jump_label: Provide
 static_key_[enable|/slow_inc]_cpuslocked()

Provide static_key_[enable|slow_inc]_cpuslocked() variant that
don't take cpu_hotplug_lock().

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 include/linux/jump_label.h |  7 +++++++
 kernel/jump_label.c        | 10 ++++++++++
 2 files changed, 17 insertions(+)

diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
index d7b17d1ab875..c80d8b1279b5 100644
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -164,6 +164,7 @@ extern void static_key_slow_dec_cpuslocked(struct static_key *key);
 extern void jump_label_apply_nops(struct module *mod);
 extern int static_key_count(struct static_key *key);
 extern void static_key_enable(struct static_key *key);
+extern void static_key_enable_cpuslocked(struct static_key *key);
 extern void static_key_disable(struct static_key *key);
 extern void static_key_disable_cpuslocked(struct static_key *key);
 
@@ -252,6 +253,11 @@ static inline void static_key_enable(struct static_key *key)
 		static_key_slow_inc(key);
 }
 
+static inline void static_key_enable_cpuslocked(struct static_key *key)
+{
+	static_key_enable(key);
+}
+
 static inline void static_key_disable(struct static_key *key)
 {
 	int count = static_key_count(key);
@@ -429,6 +435,7 @@ extern bool ____wrong_branch_error(void);
  */
 
 #define static_branch_enable(x)			static_key_enable(&(x)->key)
+#define static_branch_enable_cpuslocked(x)	static_key_enable_cpuslocked(&(x)->key)
 #define static_branch_disable(x)		static_key_disable(&(x)->key)
 #define static_branch_disable_cpuslocked(x)	static_key_disable_cpuslocked(&(x)->key)
 
diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index d71124ee3b14..6343f4c7e27f 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -90,6 +90,16 @@ void static_key_enable(struct static_key *key)
 }
 EXPORT_SYMBOL_GPL(static_key_enable);
 
+void static_key_enable_cpuslocked(struct static_key *key)
+{
+	int count = static_key_count(key);
+
+	WARN_ON_ONCE(count < 0 || count > 1);
+
+	if (!count)
+		static_key_slow_inc_cpuslocked(key);
+}
+
 void static_key_disable(struct static_key *key)
 {
 	int count = static_key_count(key);
-- 
2.11.0

> Thanks,
> Mark.

Sebastian

^ permalink raw reply related

* [PATCH] ARM: dts: imx6sx-sdb: Remove cpufreq OPP override
From: Marek Vasut @ 2017-04-25 17:28 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1493141004.3557.8.camel@nxp.com>

On 04/25/2017 07:23 PM, Leonard Crestez wrote:
> On Tue, 2017-04-25 at 14:02 -0300, Fabio Estevam wrote:
>> On Tue, Apr 25, 2017 at 2:02 PM, Fabio Estevam <festevam@gmail.com> wrote:
>>>
>>> Hi Leonard,
>>>
>>> On Tue, Apr 25, 2017 at 1:57 PM, Leonard Crestez
>>> <leonard.crestez@nxp.com> wrote:
>>>>
>>>> The board file for imx6sx-dbg overrides cpufreq operating points to use
>>>> higher voltages. This is done because the board has a shared rail for
>>>> VDD_ARM_IN and VDD_SOC_IN and when using LDO bypass the shared voltage
>>>> needs to be a value suitable for both ARM and SOC.
>>>>
>>>> This was introduced in:
>>>>
>>>> commit 54183bd7f766 ("ARM: imx6sx-sdb: add revb board and make it default")
>>>>
>>>> This only only applies to LDO bypass mode, a feature not present in
>>>> upstream. When LDOs are enabled the effect is to use higher voltages than
>>>> necesarry for no good reason.
>>>>
>>>> Setting these higher voltages can make some boards fail to boot with ugly
>>>> semi-random crashes, reminiscent of memory corruption. These failures
>>>> happen the first time the lowest idle state is used. Remove the OPP
>>>> override in order to fix those crashes.
>>>>
>>>> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
>>>>
>>>> ---
>>>> It's not clear exactly why the crashes happen. Perhaps waking up from idle
>>>> draws more power than is available? Removing this override is a correct
>>>> change anyway so maybe there is no need to investigate deeper.
> 
>>> Marek just sent a similar one a few minutes ago:
>>> http://lists.infradead.org/pipermail/linux-arm-kernel/2017-April/503230.html
> 
>> Forgot to add Marek.
> 
> Wow, that was literally 15 minutes before my patch. In my defense I did
> search the archives before starting to format the patch but it had not
> arrived yet.

Hehehe :-)

> Anyway, that version also sets the supply for reg_arm and reg_soc. It
> is not necessary for fixing the crash I'm seeing but is good because it
> will result in the minimum voltage on VDD_ARM_SOC_IN rather than a fix
> 1375mv. I tested Marek's patch and it works fine on my rev B board
> (which otherwise fails to boot upstream).

Oh that's nice , thanks ! I don't have SDB and I hacked it up after a
brief discussion with Fabio without even compile-testing it, thus RFC.
Glad to hear it works and thanks for testing it ! Can you add a formal
Tested-by please ?

-- 
Best regards,
Marek Vasut

^ permalink raw reply

* [PATCH] ARM: dts: imx6sx-sdb: Remove cpufreq OPP override
From: Fabio Estevam @ 2017-04-25 17:26 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1493141004.3557.8.camel@nxp.com>

On Tue, Apr 25, 2017 at 2:23 PM, Leonard Crestez
<leonard.crestez@nxp.com> wrote:

> Wow, that was literally 15 minutes before my patch. In my defense I did
> search the archives before starting to format the patch but it had not
> arrived yet.
>
> Anyway, that version also sets the supply for reg_arm and reg_soc. It
> is not necessary for fixing the crash I'm seeing but is good because it
> will result in the minimum voltage on VDD_ARM_SOC_IN rather than a fix
> 1375mv. I tested Marek's patch and it works fine on my rev B board
> (which otherwise fails to boot upstream).

Excellent! I only have revA board and do not get the crash with this revision.

If you can reply to Marek's patch with your Tested-by that would be
nice, thanks.

^ permalink raw reply

* [PATCH] ARM: dts: imx6sx-sdb: Remove cpufreq OPP override
From: Leonard Crestez @ 2017-04-25 17:23 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAOMZO5CKUsEr1yf5RrsPLipNtLC_6sK0dPjT2M-eN-uo8hEbrQ@mail.gmail.com>

On Tue, 2017-04-25 at 14:02 -0300, Fabio Estevam wrote:
> On Tue, Apr 25, 2017 at 2:02 PM, Fabio Estevam <festevam@gmail.com> wrote:
> > 
> > Hi Leonard,
> > 
> > On Tue, Apr 25, 2017 at 1:57 PM, Leonard Crestez
> > <leonard.crestez@nxp.com> wrote:
> > > 
> > > The board file for imx6sx-dbg overrides cpufreq operating points to use
> > > higher voltages. This is done because the board has a shared rail for
> > > VDD_ARM_IN and VDD_SOC_IN and when using LDO bypass the shared voltage
> > > needs to be a value suitable for both ARM and SOC.
> > > 
> > > This was introduced in:
> > > 
> > > commit 54183bd7f766 ("ARM: imx6sx-sdb: add revb board and make it default")
> > > 
> > > This only only applies to LDO bypass mode, a feature not present in
> > > upstream. When LDOs are enabled the effect is to use higher voltages than
> > > necesarry for no good reason.
> > > 
> > > Setting these higher voltages can make some boards fail to boot with ugly
> > > semi-random crashes, reminiscent of memory corruption. These failures
> > > happen the first time the lowest idle state is used. Remove the OPP
> > > override in order to fix those crashes.
> > > 
> > > Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> > > 
> > > ---
> > > It's not clear exactly why the crashes happen. Perhaps waking up from idle
> > > draws more power than is available? Removing this override is a correct
> > > change anyway so maybe there is no need to investigate deeper.

> > Marek just sent a similar one a few minutes ago:
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2017-April/503230.html

> Forgot to add Marek.

Wow, that was literally 15 minutes before my patch. In my defense I did
search the archives before starting to format the patch but it had not
arrived yet.

Anyway, that version also sets the supply for reg_arm and reg_soc. It
is not necessary for fixing the crash I'm seeing but is good because it
will result in the minimum voltage on VDD_ARM_SOC_IN rather than a fix
1375mv. I tested Marek's patch and it works fine on my rev B board
(which otherwise fails to boot upstream).

--?
Regards,
Leonard

^ permalink raw reply

* [PATCH 1/2] xen/arm, arm64: fix xen_dma_ops after 815dd18 "Consolidate get_dma_ops..."
From: Stefano Stabellini @ 2017-04-25 17:22 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <f0667d8c-1c17-8d5f-a5d7-1e0d014e86ec@arm.com>

On Tue, 25 Apr 2017, Julien Grall wrote:
> Hi Stefano,
> 
> On 24/04/17 20:16, Stefano Stabellini wrote:
> > Given the outstanding regression we need to fix as soon as possible,
> > I'll queue these patches on the xentip tree for 4.12.
> 
> It looks like there is another rc for 4.11. I am wondering whether you could
> try to send a pull request to Linus so it can be fixed in 4.11?

No, especially without input from Russell.

^ permalink raw reply

* [PATCH V15 06/11] acpi: apei: handle SEA notification type for ARMv8
From: Borislav Petkov @ 2017-04-25 17:21 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1492556723-9189-7-git-send-email-tbaicar@codeaurora.org>

On Tue, Apr 18, 2017 at 05:05:18PM -0600, Tyler Baicar wrote:
> ARM APEI extension proposal added SEA (Synchronous External Abort)
> notification type for ARMv8.
> Add a new GHES error source handling function for SEA. If an error
> source's notification type is SEA, then this function can be registered
> into the SEA exception handler. That way GHES will parse and report
> SEA exceptions when they occur.
> An SEA can interrupt code that had interrupts masked and is treated as
> an NMI. To aid this the page of address space for mapping APEI buffers
> while in_nmi() is always reserved, and ghes_ioremap_pfn_nmi() is
> changed to use the helper methods to find the prot_t to map with in
> the same way as ghes_ioremap_pfn_irq().

...

> @@ -518,6 +520,17 @@ static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs)
>  	pr_err("Synchronous External Abort: %s (0x%08x) at 0x%016lx\n",
>  		inf->name, esr, addr);
>  
> +	/*
> +	 * Synchronous aborts may interrupt code which had interrupts masked.
> +	 * Before calling out into the wider kernel tell the interested
> +	 * subsystems.
> +	 */
> +	if (IS_ENABLED(CONFIG_ACPI_APEI_SEA)) {
> +		nmi_enter();
> +		ghes_notify_sea();
> +		nmi_exit();
> +	}

Well, the other GHES notification methods use a notifier:
ghes_notify_sci, ghes_notify_nmi. You probably should do that too
instead of calling straight into a driver from arch code.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

^ permalink raw reply

* [PATCH] ARM/shmem: Drop page coloring align for non-VIPT CPUs
From: Dmitry Safonov @ 2017-04-25 17:19 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170414100953.4703-1-dsafonov@virtuozzo.com>

On 04/14/2017 01:09 PM, Dmitry Safonov wrote:
> On ARMv6 CPUs with VIPT caches there are aliasing issues: if two
> different cache line indexes correspond to the same physical
> address, then changes made to one of the alias might be lost
> or they can overwrite each other. To overcome aliasing issues,
> the align for shared mappings was introduced with:
> 
> commit 4197692eef113eeb8e3e413cc70993a5e667e5b8
> Author: Russell King <rmk@flint.arm.linux.org.uk>
> Date:   Wed Apr 28 22:22:33 2004 +0100
> 
>      [ARM] Fix shared mmap()ings for ARM VIPT caches.
> 
>      This allows us to appropriately align shared mappings on VIPT caches
>      with aliasing issues.
> 
> Which introduced 4 pages align with SHMLBA, which resulted in
> unique physical address after any tag in cache (because two upper bits
> corresponding to page address get unused in tags).
> 
> As this workaround is not needed by non-VIPT caches (like most armv7
> CPUs which have PIPT caches), ARM mmap() code checks if cache is VIPT
> aliasing for MAP_SHARED.
> 
> The problem here is in shmat() syscall:
> 1. if shmaddr is NULL then do_shmat() uses arch_get_unmapped_area()
>     to allocate shared mapping.
> 2. if shmaddr is specified then do_shmat() checks that address has
>     SHMLBA alignment regardless to CPU cache aliasing.
> 
> Which results on ARMv7 CPUs that shmat() with NULL shmaddr may return
> non-SHMLBA aligned address (page-aligned), but shmat() with the same
> address will fail.
> 
> That is not critical issue for CRIU as after shmat() with NULL address,
> we can mremap() resulted shmem to restore shared memory mappings on the
> same address where they were on checkpointing.
> But it's still worth fixing because we can't reliably tell from
> userspace if the platform has VIPT cache, and so this mremap()
> workaround is done with HUGE warning that restoring application, that
> uses SHMBLA-unaligned shmem on ARMv6 CPU with VIPT cache may result
> in data corruptions.
> 
> I also changed SHMLBA build-time check to init-time WARN_ON(), as
> it's not constant afterward.
> 
> This is resend of the patch from discussion:
> http://www.spinics.net/lists/arm-kernel/msg258870.html
> 
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Russell King <rmk+kernel@armlinux.org.uk>
> Cc: Christopher Covington <cov@codeaurora.org>
> Cc: Cyrill Gorcunov <gorcunov@openvz.org>
> Cc: Pavel Emelyanov <xemul@virtuozzo.com>
> Suggested-by: Russell King <rmk+kernel@armlinux.org.uk>
> Signed-off-by: Dmitry Safonov <dsafonov@virtuozzo.com>
> ---
>   arch/arm/include/asm/shmparam.h | 8 ++++----
>   arch/arm/mm/copypage-v6.c       | 5 +----
>   2 files changed, 5 insertions(+), 8 deletions(-)

Ping?

> 
> diff --git a/arch/arm/include/asm/shmparam.h b/arch/arm/include/asm/shmparam.h
> index a5223b3a9bf9..6e1a72cbdf78 100644
> --- a/arch/arm/include/asm/shmparam.h
> +++ b/arch/arm/include/asm/shmparam.h
> @@ -1,16 +1,16 @@
>   #ifndef _ASMARM_SHMPARAM_H
>   #define _ASMARM_SHMPARAM_H
>   
> +#include <asm/cachetype.h>
> +
>   /*
>    * This should be the size of the virtually indexed cache/ways,
>    * or page size, whichever is greater since the cache aliases
>    * every size/ways bytes.
>    */
> -#define	SHMLBA	(4 * PAGE_SIZE)		 /* attach addr a multiple of this */
> +#define	SHMLBA (cache_is_vipt_aliasing() ? 4 * PAGE_SIZE : PAGE_SIZE)
>   
> -/*
> - * Enforce SHMLBA in shmat
> - */
> +/* Enforce SHMLBA in shmat */
>   #define __ARCH_FORCE_SHMLBA
>   
>   #endif /* _ASMARM_SHMPARAM_H */
> diff --git a/arch/arm/mm/copypage-v6.c b/arch/arm/mm/copypage-v6.c
> index 70423345da26..9b22c530e8b6 100644
> --- a/arch/arm/mm/copypage-v6.c
> +++ b/arch/arm/mm/copypage-v6.c
> @@ -20,10 +20,6 @@
>   
>   #include "mm.h"
>   
> -#if SHMLBA > 16384
> -#error FIX ME
> -#endif
> -
>   static DEFINE_RAW_SPINLOCK(v6_lock);
>   
>   /*
> @@ -129,6 +125,7 @@ struct cpu_user_fns v6_user_fns __initdata = {
>   
>   static int __init v6_userpage_init(void)
>   {
> +	WARN_ON(SHMLBA > 16384);
>   	if (cache_is_vipt_aliasing()) {
>   		cpu_user.cpu_clear_user_highpage = v6_clear_user_highpage_aliasing;
>   		cpu_user.cpu_copy_user_highpage = v6_copy_user_highpage_aliasing;
> 


-- 
              Dmitry

^ permalink raw reply

* [PATCHv2] ARM32: Support mremap() for sigpage/vDSO
From: Dmitry Safonov @ 2017-04-25 17:18 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170414132529.8337-1-dsafonov@virtuozzo.com>

On 04/14/2017 04:25 PM, Dmitry Safonov wrote:
> CRIU restores application mappings on the same place where they
> were before Checkpoint. That means, that we need to move vDSO
> and sigpage during restore on exactly the same place where
> they were before C/R.
> 
> Make mremap() code update mm->context.{sigpage,vdso} pointers
> during VMA move. Sigpage is used for landing after handling
> a signal - if the pointer is not updated during moving, the
> application might crash on any signal after mremap().
> 
> vDSO pointer on ARM32 is used only for setting auxv at this moment,
> update it during mremap() in case of future usage.
> 
> Without those updates, current work of CRIU on ARM32 is not reliable.
> Historically, we error Checkpointing if we find vDSO page on ARM32
> and suggest user to disable CONFIG_VDSO.
> But that's not correct - it goes from x86 where signal processing
> is ended in vDSO blob. For arm32 it's sigpage, which is not disabled
> with `CONFIG_VDSO=n'.
> 
> Looks like C/R was working by luck - because userspace on ARM32 at
> this moment always sets SA_RESTORER.
> 
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Andy Lutomirski <luto@amacapital.net>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Cyrill Gorcunov <gorcunov@openvz.org>
> Cc: Pavel Emelyanov <xemul@virtuozzo.com>
> Cc: Christopher Covington <cov@codeaurora.org>
> Signed-off-by: Dmitry Safonov <dsafonov@virtuozzo.com>
> ---
> v2: (buildbot) Fix (unsinged long) to (void*) cast warning.
> 
>   arch/arm/kernel/process.c |  8 ++++++++
>   arch/arm/kernel/vdso.c    | 18 ++++++++++++++++++
>   arch/x86/entry/vdso/vma.c |  3 ---
>   mm/mmap.c                 |  4 ++++
>   4 files changed, 30 insertions(+), 3 deletions(-)

Ping?

> 
> diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
> index 939e8b58c59d..1e6039cac68d 100644
> --- a/arch/arm/kernel/process.c
> +++ b/arch/arm/kernel/process.c
> @@ -404,9 +404,17 @@ static unsigned long sigpage_addr(const struct mm_struct *mm,
>   static struct page *signal_page;
>   extern struct page *get_signal_page(void);
>   
> +static int sigpage_mremap(const struct vm_special_mapping *sm,
> +		struct vm_area_struct *new_vma)
> +{
> +	current->mm->context.sigpage = new_vma->vm_start;
> +	return 0;
> +}
> +
>   static const struct vm_special_mapping sigpage_mapping = {
>   	.name = "[sigpage]",
>   	.pages = &signal_page,
> +	.mremap = sigpage_mremap,
>   };
>   
>   int arch_setup_additional_pages(struct linux_binprm *bprm, int uses_interp)
> diff --git a/arch/arm/kernel/vdso.c b/arch/arm/kernel/vdso.c
> index 53cf86cf2d1a..a4d6dc0f2427 100644
> --- a/arch/arm/kernel/vdso.c
> +++ b/arch/arm/kernel/vdso.c
> @@ -54,8 +54,26 @@ static const struct vm_special_mapping vdso_data_mapping = {
>   	.pages = &vdso_data_page,
>   };
>   
> +static int vdso_mremap(const struct vm_special_mapping *sm,
> +		struct vm_area_struct *new_vma)
> +{
> +	unsigned long new_size = new_vma->vm_end - new_vma->vm_start;
> +	unsigned long vdso_size;
> +
> +	/* without VVAR page */
> +	vdso_size = (vdso_total_pages - 1) << PAGE_SHIFT;
> +
> +	if (vdso_size != new_size)
> +		return -EINVAL;
> +
> +	current->mm->context.vdso = new_vma->vm_start;
> +
> +	return 0;
> +}
> +
>   static struct vm_special_mapping vdso_text_mapping __ro_after_init = {
>   	.name = "[vdso]",
> +	.mremap = vdso_mremap,
>   };
>   
>   struct elfinfo {
> diff --git a/arch/x86/entry/vdso/vma.c b/arch/x86/entry/vdso/vma.c
> index 226ca70dc6bd..363730caa60e 100644
> --- a/arch/x86/entry/vdso/vma.c
> +++ b/arch/x86/entry/vdso/vma.c
> @@ -77,9 +77,6 @@ static int vdso_mremap(const struct vm_special_mapping *sm,
>   	if (image->size != new_size)
>   		return -EINVAL;
>   
> -	if (WARN_ON_ONCE(current->mm != new_vma->vm_mm))
> -		return -EFAULT;
> -
>   	vdso_fix_landing(image, new_vma);
>   	current->mm->context.vdso = (void __user *)new_vma->vm_start;
>   
> diff --git a/mm/mmap.c b/mm/mmap.c
> index bfbe8856d134..534aef99cfe9 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -3152,8 +3152,12 @@ static int special_mapping_mremap(struct vm_area_struct *new_vma)
>   {
>   	struct vm_special_mapping *sm = new_vma->vm_private_data;
>   
> +	if (WARN_ON_ONCE(current->mm != new_vma->vm_mm))
> +		return -EFAULT;
> +
>   	if (sm->mremap)
>   		return sm->mremap(sm, new_vma);
> +
>   	return 0;
>   }
>   
> 


-- 
              Dmitry

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox