* [PATCH v4] arm64: fpsimd: improve stacking logic in non-interruptible context
From: Catalin Marinas @ 2016-12-09 18:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481301992-2344-1-git-send-email-ard.biesheuvel@linaro.org>
On Fri, Dec 09, 2016 at 04:46:32PM +0000, Ard Biesheuvel wrote:
> void kernel_neon_begin_partial(u32 num_regs)
> {
> - if (in_interrupt()) {
> - struct fpsimd_partial_state *s = this_cpu_ptr(
> - in_irq() ? &hardirq_fpsimdstate : &softirq_fpsimdstate);
> + struct fpsimd_partial_state *s;
> + int level;
> +
> + preempt_disable();
> +
> + level = this_cpu_inc_return(kernel_neon_nesting_level);
> + BUG_ON(level > 3);
> +
> + if (level > 1) {
> + s = this_cpu_ptr(nested_fpsimdstate);
>
> - BUG_ON(num_regs > 32);
> - fpsimd_save_partial_state(s, roundup(num_regs, 2));
> + WARN_ON_ONCE(num_regs > 32);
> + num_regs = min(roundup(num_regs, 2), 32U);
> +
> + fpsimd_save_partial_state(&s[level - 2], num_regs);
> } else {
> /*
> * Save the userland FPSIMD state if we have one and if we
> @@ -241,7 +256,6 @@ void kernel_neon_begin_partial(u32 num_regs)
> * that there is no longer userland FPSIMD state in the
> * registers.
> */
> - preempt_disable();
> if (current->mm &&
> !test_and_set_thread_flag(TIF_FOREIGN_FPSTATE))
> fpsimd_save_state(¤t->thread.fpsimd_state);
I wonder whether we could actually do this saving and flag/level setting
in reverse to simplify the races. Something like your previous patch but
only set TIF_FOREIGN_FPSTATE after saving:
level = this_cpu_read(kernel_neon_nesting_level);
if (level > 0) {
...
fpsimd_save_partial_state();
} else {
if (!test_thread_flag(TIF_FOREIGN_FPSTATE))
fpsimd_save_state();
set_thread_flag(TIF_FOREIGN_FPSTATE);
}
this_cpu_inc(kernel_neon_nesting_level);
There is a risk of extra saving if we get an interrupt after
test_thread_flag() and before set_thread_flag() but I don't think this
would corrupt any state, just writing things twice.
(disclaimer: I haven't thought of all the possible races and I'm not
entirely sure about the kernel_neon_end() part)
--
Catalin
^ permalink raw reply
* Tearing down DMA transfer setup after DMA client has finished
From: Vinod Koul @ 2016-12-09 18:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161209175606.GM6408@localhost>
On Fri, Dec 09, 2016 at 11:26:06PM +0530, Vinod Koul wrote:
> On Fri, Dec 09, 2016 at 06:34:15PM +0100, Mason wrote:
> > On 09/12/2016 18:17, Vinod Koul wrote:
> >
> > > On Fri, Dec 09, 2016 at 11:25:57AM +0100, Sebastian Frias wrote:
> > >>
> > >> What concrete solution do you propose?
> > >
> > > I have already proposed two solutions.
> > >
> > > A) Request a channel only when you need it. Obviously we can't do virtual
> > > channels with this (though we should still use virt-channels framework).
> > > The sbox setup and teardown can be done as part of channel request and
> > > freeup. PL08x already does this.
> > >
> > > Downside is that we can only have as many consumers at a time as channels.
> > >
> > > I have not heard any technical reason for not doing this apart from drivers
> > > grab the channel at probe, which is incorrect and needs to be fixed
> > > irrespective of the problem at hand.
> > >
> > > This is my preferred option.
> >
> > There is one important drawback with this solution. If a driver calls
> > dma_request_chan() when no channels are currently available, it will
> > get -EBUSY. If there were a flag in dma_request_chan to be put to
> > sleep (with timeout) until a channel is available, then it would
> > work. But busy waiting in the client driver is a waste of power.
>
> Right, but in that case the fallback would be PIO mode, and if that is
> not availble (IIRC some f your devices don't) then reject the usage with
> EAGAIN.
Alternatively I can think of one more way.
If there is fixed delay or maximum delay predicted between ISR being
fired and transaction being completed from client, then we can use that
magic value and degrade the performance a bit but make a simpler system
than other two suggestions.
The idea here is that typically the subsequent transaction should be
issued as soon as possible, best case being in the ISR. But we can
degrade that performance a bit and issue in the tasklet. But that can be
done after introducing a delay, that too only in the case where new sbox
configuration is different from previous one (so performance degrade is
only on the switch and not for txn for same setup). You can possible
optimize even further by issuing in ISR for same sbox setup and issuing
in tasklet if configuration is different.
Yes this is bit iffy and adds more burden on driver, but lets us get
away with decent performance and being able to handle the hardware
condition.
Would that work for your case...?
--
~Vinod
^ permalink raw reply
* [PATCH 10/11] Document: dt: binding: imx: update doc for imx6sll
From: Rob Herring @ 2016-12-09 18:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480660774-25055-11-git-send-email-ping.bai@nxp.com>
On Fri, Dec 02, 2016 at 02:39:33PM +0800, Bai Ping wrote:
> Add necessary document update for i.MX6SLL support.
>
> Signed-off-by: Bai Ping <ping.bai@nxp.com>
> ---
> .../devicetree/bindings/clock/imx6sll-clock.txt | 13 ++++++++
> .../bindings/pinctrl/fsl,imx6sll-pinctrl.txt | 37 ++++++++++++++++++++++
> 2 files changed, 50 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/imx6sll-clock.txt
> create mode 100644 Documentation/devicetree/bindings/pinctrl/fsl,imx6sll-pinctrl.txt
>
> diff --git a/Documentation/devicetree/bindings/clock/imx6sll-clock.txt b/Documentation/devicetree/bindings/clock/imx6sll-clock.txt
> new file mode 100644
> index 0000000..4f52efa
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/imx6sll-clock.txt
> @@ -0,0 +1,13 @@
> +* Clock bindings for Freescale i.MX6 UltraLite
I thought UltraLite was MX6UL?
> +
> +Required properties:
> +- compatible: Should be "fsl,imx6sll-ccm"
> +- reg: Address and length of the register set
> +- #clock-cells: Should be <1>
> +- clocks: list of clock specifiers, must contain an entry for each required
> + entry in clock-names
> +- clock-names: should include entries "ckil", "osc", "ipp_di0" and "ipp_di1"
> +
> +The clock consumer should specify the desired clock by having the clock
> +ID in its "clocks" phandle cell. See include/dt-bindings/clock/imx6sll-clock.h
> +for the full list of i.MX6 SLL clock IDs.
> diff --git a/Documentation/devicetree/bindings/pinctrl/fsl,imx6sll-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/fsl,imx6sll-pinctrl.txt
> new file mode 100644
> index 0000000..096e471
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/fsl,imx6sll-pinctrl.txt
> @@ -0,0 +1,37 @@
> +* Freescale i.MX6 UltraLite IOMUX Controller
ditto
> +
> +Please refer to fsl,imx-pinctrl.txt in this directory for common binding part
> +and usage.
> +
> +Required properties:
> +- compatible: "fsl,imx6sll-iomuxc"
> +- fsl,pins: each entry consists of 6 integers and represents the mux and config
> + setting for one pin. The first 5 integers <mux_reg conf_reg input_reg mux_val
> + input_val> are specified using a PIN_FUNC_ID macro, which can be found in
> + imx6ul-pinfunc.h under device tree source folder. The last integer CONFIG is
> + the pad setting value like pull-up on this pin. Please refer to i.MX6SLL
> + Reference Manual for detailed CONFIG settings.
> +
> +CONFIG bits definition:
> +PAD_CTL_LVE (1 << 22)
> +PAD_CTL_HYS (1 << 16)
> +PAD_CTL_PUS_100K_DOWN (0 << 14)
> +PAD_CTL_PUS_47K_UP (1 << 14)
> +PAD_CTL_PUS_100K_UP (2 << 14)
> +PAD_CTL_PUS_22K_UP (3 << 14)
> +PAD_CTL_PUE (1 << 13)
> +PAD_CTL_PKE (1 << 12)
> +PAD_CTL_ODE (1 << 11)
> +PAD_CTL_SPEED_LOW (0 << 6)
> +PAD_CTL_SPEED_MED (1 << 6)
> +PAD_CTL_SPEED_HIGH (3 << 6)
> +PAD_CTL_DSE_DISABLE (0 << 3)
> +PAD_CTL_DSE_260ohm (1 << 3)
> +PAD_CTL_DSE_130ohm (2 << 3)
> +PAD_CTL_DSE_87ohm (3 << 3)
> +PAD_CTL_DSE_65ohm (4 << 3)
> +PAD_CTL_DSE_52ohm (5 << 3)
> +PAD_CTL_DSE_43ohm (6 << 3)
> +PAD_CTL_DSE_37ohm (7 << 3)
> +PAD_CTL_SRE_FAST (1 << 0)
> +PAD_CTL_SRE_SLOW (0 << 0)
> --
> 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
* [PATCH] arm64: mm: Fix NOMAP page initialization
From: Robert Richter @ 2016-12-09 18:10 UTC (permalink / raw)
To: linux-arm-kernel
On ThunderX systems with certain memory configurations we see the
following BUG_ON():
kernel BUG at mm/page_alloc.c:1848!
This happens for some configs with 64k page size enabled. The BUG_ON()
checks if start and end page of a memmap range belongs to the same
zone.
The BUG_ON() check fails if a memory zone contains NOMAP regions. In
this case the node information of those pages is not initialized. This
causes an inconsistency of the page links with wrong zone and node
information for that pages. NOMAP pages from node 1 still point to the
mem zone from node 0 and have the wrong nid assigned.
The reason for the mis-configuration is a change in pfn_valid() which
reports pages marked NOMAP as invalid:
68709f45385a arm64: only consider memblocks with NOMAP cleared for linear mapping
This causes pages marked as nomap being no longer reassigned to the
new zone in memmap_init_zone() by calling __init_single_pfn().
Fixing this by implementing an arm64 specific early_pfn_valid(). This
causes the whole mem range including NOMAP memory to be initialized by
__init_single_page() and ensures consistency of page links to zone,
node and section.
The HAVE_ARCH_PFN_VALID config option now requires an explicit
definiton of early_pfn_valid() in the same way as pfn_valid(). This
allows a customized implementation of early_pfn_valid() which
redirects to memblock_is_memory() for arm64.
Signed-off-by: Robert Richter <rrichter@cavium.com>
---
arch/arm/include/asm/page.h | 1 +
arch/arm64/include/asm/page.h | 2 ++
arch/arm64/mm/init.c | 12 ++++++++++++
include/linux/mmzone.h | 5 ++++-
4 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/arch/arm/include/asm/page.h b/arch/arm/include/asm/page.h
index 4355f0ec44d6..79761bd55f94 100644
--- a/arch/arm/include/asm/page.h
+++ b/arch/arm/include/asm/page.h
@@ -158,6 +158,7 @@ typedef struct page *pgtable_t;
#ifdef CONFIG_HAVE_ARCH_PFN_VALID
extern int pfn_valid(unsigned long);
+#define early_pfn_valid(pfn) pfn_valid(pfn)
#endif
#include <asm/memory.h>
diff --git a/arch/arm64/include/asm/page.h b/arch/arm64/include/asm/page.h
index 8472c6def5ef..17ceb7435ded 100644
--- a/arch/arm64/include/asm/page.h
+++ b/arch/arm64/include/asm/page.h
@@ -49,6 +49,8 @@ typedef struct page *pgtable_t;
#ifdef CONFIG_HAVE_ARCH_PFN_VALID
extern int pfn_valid(unsigned long);
+extern int early_pfn_valid(unsigned long);
+#define early_pfn_valid early_pfn_valid
#endif
#include <asm/memory.h>
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 212c4d1e2f26..fbc136533472 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -145,11 +145,23 @@ static void __init zone_sizes_init(unsigned long min, unsigned long max)
#endif /* CONFIG_NUMA */
#ifdef CONFIG_HAVE_ARCH_PFN_VALID
+
int pfn_valid(unsigned long pfn)
{
return memblock_is_map_memory(pfn << PAGE_SHIFT);
}
EXPORT_SYMBOL(pfn_valid);
+
+/*
+ * We use memblock_is_memory() here to make sure all pages including
+ * NOMAP ranges are initialized with __init_single_page().
+ */
+int early_pfn_valid(unsigned long pfn)
+{
+ return memblock_is_memory(pfn << PAGE_SHIFT);
+}
+EXPORT_SYMBOL(early_pfn_valid);
+
#endif
#ifndef CONFIG_SPARSEMEM
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 0f088f3a2fed..bedcf8a95881 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -1170,12 +1170,16 @@ static inline struct mem_section *__pfn_to_section(unsigned long pfn)
}
#ifndef CONFIG_HAVE_ARCH_PFN_VALID
+
static inline int pfn_valid(unsigned long pfn)
{
if (pfn_to_section_nr(pfn) >= NR_MEM_SECTIONS)
return 0;
return valid_section(__nr_to_section(pfn_to_section_nr(pfn)));
}
+
+#define early_pfn_valid(pfn) pfn_valid(pfn)
+
#endif
static inline int pfn_present(unsigned long pfn)
@@ -1200,7 +1204,6 @@ static inline int pfn_present(unsigned long pfn)
#define pfn_to_nid(pfn) (0)
#endif
-#define early_pfn_valid(pfn) pfn_valid(pfn)
void sparse_init(void);
#else
#define sparse_init() do {} while (0)
--
2.1.4
^ permalink raw reply related
* [PATCH v4] arm64: fpsimd: improve stacking logic in non-interruptible context
From: Catalin Marinas @ 2016-12-09 18:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161209172407.GC1574@e103592.cambridge.arm.com>
On Fri, Dec 09, 2016 at 05:24:08PM +0000, Dave P Martin wrote:
> On Fri, Dec 09, 2016 at 04:46:32PM +0000, Ard Biesheuvel wrote:
> > Currently, we allow kernel mode NEON in softirq or hardirq context by
> > stacking and unstacking a slice of the NEON register file for each call
> > to kernel_neon_begin() and kernel_neon_end(), respectively.
> >
> > Given that
> > a) a CPU typically spends most of its time in userland, during which time
> > no kernel mode NEON in process context is in progress,
> > b) a CPU spends most of its time in the kernel doing other things than
> > kernel mode NEON when it gets interrupted to perform kernel mode NEON
> > in softirq context
> >
> > the stacking and subsequent unstacking is only necessary if we are
> > interrupting a thread while it is performing kernel mode NEON in process
> > context, which means that in all other cases, we can simply preserve the
> > userland FPSIMD state once, and only restore it upon return to userland,
> > even if we are being invoked from softirq or hardirq context.
> >
> > So instead of checking whether we are running in interrupt context, keep
> > track of the level of nested kernel mode NEON calls in progress, and only
> > perform the eager stack/unstack if the level exceeds 1.
> >
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> This looks good to me.
>
> This should also make the SVE case trivial now: there can only be live
> SVE state when in process context with !TIF_FOREIGN_FPSTATE, and the
> SVE save/restore is then handled by fpsimd_{save,load}_state()
> directly. For deeper nesting levels, there is already no live SVE
> state, so kernel_neon_{save,load}_partial_state() are enough in that
> case.
That's still tricky for SVE. If you get an interrupt in
kernel_neon_begin_partial() after the level has been incremented to 1
but before fpsimd_save_state() has been called, SVE-unaware
kernel_neon_{save,load}_partial_state() would corrupt the SVE state.
--
Catalin
^ permalink raw reply
* [GIT PULL] ARM: SoC fixes
From: Olof Johansson @ 2016-12-09 18:01 UTC (permalink / raw)
To: linux-arm-kernel
Hi Linus,
This should be our last set of fixes for 4.9. Please merge!
Thanks,
-Olof
The following changes since commit 909e481e2467f202b97d42beef246e8829416a85:
arm64: dts: juno: fix cluster sleep state entry latency on all SoC versions (2016-12-02 17:28:17 +0100)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc.git tags/armsoc-fixes
for you to fetch changes up to 038ccb3e8cee52e07dc118ff99f47eaebc1d0746:
ARM: dts: orion5x: fix number of sata port for linkstation ls-gl (2016-12-08 10:19:24 -0800)
----------------------------------------------------------------
ARM: Final batch of SoC fixes
A few fixes that have trickled in over the last week, all fixing minor
errors in devicetrees -- UART pin assignment on Allwinner H3, correcting
number of SATA ports on a Marvell-based Linkstation platform and a display
clock fix for Freescale/NXP i.MX7D that fixes a freeze when starting up X.
----------------------------------------------------------------
Jorik Jonker (1):
dts: sun8i-h3: correct UART3 pin definitions
Roger Shimizu (1):
ARM: dts: orion5x: fix number of sata port for linkstation ls-gl
Stefan Agner (1):
ARM: dts: imx7d: fix LCDIF clock assignment
arch/arm/boot/dts/imx7s.dtsi | 5 ++---
arch/arm/boot/dts/orion5x-linkstation-lsgl.dts | 4 ++++
arch/arm/boot/dts/sun8i-h3.dtsi | 2 +-
3 files changed, 7 insertions(+), 4 deletions(-)
^ permalink raw reply
* [PATCH v2 2/2] dt-bindings: Add DT bindings info for FlexRM ring manager
From: Rob Herring @ 2016-12-09 18:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480653536-5551-3-git-send-email-anup.patel@broadcom.com>
On Fri, Dec 02, 2016 at 10:08:56AM +0530, Anup Patel wrote:
> This patch adds device tree bindings document for the FlexRM
> ring manager found on Broadcom iProc SoCs.
>
> Reviewed-by: Ray Jui <ray.jui@broadcom.com>
> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
> Signed-off-by: Anup Patel <anup.patel@broadcom.com>
> ---
> .../bindings/mailbox/brcm,iproc-flexrm-mbox.txt | 60 ++++++++++++++++++++++
> 1 file changed, 60 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mailbox/brcm,iproc-flexrm-mbox.txt
>
> diff --git a/Documentation/devicetree/bindings/mailbox/brcm,iproc-flexrm-mbox.txt b/Documentation/devicetree/bindings/mailbox/brcm,iproc-flexrm-mbox.txt
> new file mode 100644
> index 0000000..e81f116
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mailbox/brcm,iproc-flexrm-mbox.txt
> @@ -0,0 +1,60 @@
[...]
> +Example:
> +--------
> +crypto_mbox: mbox at 67000000 {
> + compatible = "brcm,flexrm-mbox";
> + reg = <0x67000000 0x200000>;
> + msi-parent = <&gic_its 0x7f00>;
> + #mbox-cells = <3>;
> +};
> +
> +crypto_client {
> + ...
> + mboxes = <&crypto_mbox 0 0x1 0xffff>,
> + <&crypto_mbox 1 0x1 0xffff>,
> + <&crypto_mbox 16 0x1 0xffff>,
> + <&crypto_mbox 17 0x1 0xffff>,
> + <&crypto_mbox 30 0x1 0xffff>,
> + <&crypto_mbox 31 0x1 0xffff>;
The FlexRM part looks fine. I still don't understand what this node is.
Is this a h/w block or just a list of mailboxes? What determines the
mailbox channel numbers here? I need to see what the complete node looks
like.
Rob
^ permalink raw reply
* Tearing down DMA transfer setup after DMA client has finished
From: Vinod Koul @ 2016-12-09 17:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <ec33d3f0-6474-2b13-8a26-077ce3785d31@free.fr>
On Fri, Dec 09, 2016 at 06:34:15PM +0100, Mason wrote:
> On 09/12/2016 18:17, Vinod Koul wrote:
>
> > On Fri, Dec 09, 2016 at 11:25:57AM +0100, Sebastian Frias wrote:
> >>
> >> What concrete solution do you propose?
> >
> > I have already proposed two solutions.
> >
> > A) Request a channel only when you need it. Obviously we can't do virtual
> > channels with this (though we should still use virt-channels framework).
> > The sbox setup and teardown can be done as part of channel request and
> > freeup. PL08x already does this.
> >
> > Downside is that we can only have as many consumers at a time as channels.
> >
> > I have not heard any technical reason for not doing this apart from drivers
> > grab the channel at probe, which is incorrect and needs to be fixed
> > irrespective of the problem at hand.
> >
> > This is my preferred option.
>
> There is one important drawback with this solution. If a driver calls
> dma_request_chan() when no channels are currently available, it will
> get -EBUSY. If there were a flag in dma_request_chan to be put to
> sleep (with timeout) until a channel is available, then it would
> work. But busy waiting in the client driver is a waste of power.
Right, but in that case the fallback would be PIO mode, and if that is
not availble (IIRC some f your devices don't) then reject the usage with
EAGAIN.
--
~Vinod
^ permalink raw reply
* [PATCH 6/9] dt-bindings: Document rk3399 Gru/Kevin
From: Rob Herring @ 2016-12-09 17:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480645653-36943-7-git-send-email-briannorris@chromium.org>
On Thu, Dec 01, 2016 at 06:27:30PM -0800, Brian Norris wrote:
> Gru is a base dev board for a family of devices, including Kevin. Both
> utilize Rockchip RK3399, and they share much of their design.
>
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
> Documentation/devicetree/bindings/arm/rockchip.txt | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/arm/rockchip.txt b/Documentation/devicetree/bindings/arm/rockchip.txt
> index cc4ace6397ab..830e13f5890c 100644
> --- a/Documentation/devicetree/bindings/arm/rockchip.txt
> +++ b/Documentation/devicetree/bindings/arm/rockchip.txt
> @@ -99,6 +99,26 @@ Rockchip platforms device tree bindings
> "google,veyron-speedy-rev3", "google,veyron-speedy-rev2",
> "google,veyron-speedy", "google,veyron", "rockchip,rk3288";
>
> +- Google Gru (dev-board):
> + Required root node properties:
> + - compatible = "google,gru-rev15", "google,gru-rev14",
> + "google,gru-rev13", "google,gru-rev12",
> + "google,gru-rev11", "google,gru-rev10",
> + "google,gru-rev9", "google,gru-rev8",
> + "google,gru-rev7", "google,gru-rev6",
> + "google,gru-rev5", "google,gru-rev4",
> + "google,gru-rev3", "google,gru-rev2",
> + "google,gru", "rockchip,rk3399";
All of these are supposed to be specified or just one rev at a time?
> +
> +- Google Kevin:
> + Required root node properties:
> + - compatible = "google,kevin-rev15", "google,kevin-rev14",
> + "google,kevin-rev13", "google,kevin-rev12",
> + "google,kevin-rev11", "google,kevin-rev10",
> + "google,kevin-rev9", "google,kevin-rev8",
> + "google,kevin-rev7", "google,kevin-rev6",
> + "google,kevin", "google,gru", "rockchip,rk3399";
> +
> - mqmaker MiQi:
> Required root node properties:
> - compatible = "mqmaker,miqi", "rockchip,rk3288";
> --
> 2.8.0.rc3.226.g39d4020
>
^ permalink raw reply
* Tearing down DMA transfer setup after DMA client has finished
From: Vinod Koul @ 2016-12-09 17:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <yw1xbmwlxba6.fsf@unicorn.mansr.com>
On Fri, Dec 09, 2016 at 05:28:01PM +0000, M?ns Rullg?rd wrote:
> Vinod Koul <vinod.koul@intel.com> writes:
>
> > On Fri, Dec 09, 2016 at 11:25:57AM +0100, Sebastian Frias wrote:
> >>
> >> What concrete solution do you propose?
> >
> > I have already proposed two solutions.
> >
> > A) Request a channel only when you need it. Obviously we can't do virtual
> > channels with this (though we should still use virt-channels framework).
> > The sbox setup and teardown can be done as part of channel request and
> > freeup. PL08x already does this.
> >
> > Downside is that we can only have as many consumers at a time as channels.
> >
> > I have not heard any technical reason for not doing this apart from drivers
> > grab the channel at probe, which is incorrect and needs to be fixed
> > irrespective of the problem at hand.
> >
> > This is my preferred option.
>
> Sorry, but this is not acceptable.
without outlining why..
>
> > B) Create a custom driver specific API. This API for example:
> > sbox_setup(bool enable, ...)
> > can be called by client to explicitly setup and clear up the sbox setting.
> >
> > This way we can have transactions muxed.
> >
> > I have not heard any arguments on why we shouldn't do this except Russell's
> > comment that A) solves this.
>
> Driver-specific interfaces are not the solution. That way lies chaos
> and madness.
Yes fair enough. So would API change which 99% world doesnt need.
> This would all be so much easier if you all would just shut up for a
> moment and let me fix it properly.
Oh please go away, noone is asking you to reply!
--
~Vinod
^ permalink raw reply
* [BUG/RFC] efi/libstub: Preserve the memory map pointed to by the FDT
From: Ard Biesheuvel @ 2016-12-09 17:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161209171501.21269-1-james.morse@arm.com>
Hi James,
On 9 December 2016 at 17:15, James Morse <james.morse@arm.com> wrote:
> allocate_new_fdt_and_exit_boot() bakes a pointer to the UEFI memory map
> into the FDT for later use. The corresponding memory is then free()d
> and (hopefully) allocated again via efi_exit_boot_services() calling
> efi_get_memory_map() as part of the exit_boot_services() loop. The
> final copy is annotated with virtual mappings and used to create the
> runtime map.
>
> Linux expects the FDT to contain a pointer to the UEFI memory map
> with the virtual mapping annotations, which will only happen if
> AllocatePool() returns memory to the stub that was just FreePool()d
> by the stub. This behaviour doesn't appear to guaranteed by the spec.
>
> Platforms that don't do this will use the stale memory map (assuming
> no later owner of the freed() memory changed it) and fail to initialise
> runtime services.
>
> Instead, keep the memory map we baked info the FDT, and create a new
> 'exit_map' pointer for use in the exit_boot_services() loop. (This was
> already different, it just had the same name and we hoped it would be
> in the same place).
>
> Annotate the memory map pointed to by the FDT with virtual mappings.
> This assumes the final memory map (which may be different), has
> not moved any of the regions with the runtime attribute.
>
> (Take the opportunity to remove some comments that are stale since
> commit ed9cc156c42f ("efi/libstub: Use efi_exit_boot_services() in FDT"))
>
> Signed-off-by: James Morse <james.morse@arm.com>
> Fixes: ed9cc156c42f ("efi/libstub: Use efi_exit_boot_services() in FDT")
> Cc: Jeffrey Hugo <jhugo@codeaurora.org>
---
> I haven't seen this causing problems on any real platforms, only kvmtool
> which I'm busily hacking up to have primitive UEFI support. Needless to
> say my AllocatePool() doesn't re-use memory.
>
Thanks for the report. This is obviously a serious bug, and should be
fixed asap.
However, the approach is not correct. The whole point of the memory
map dance is that *only* the final version is correct, and the code
Jeffrey added is to ensure that even if ExitBootServices() fails the
first time (which can occur when a timer event fires between
GetMemoryMap() and ExitBootServices()), the memory map is retrieved
again.
The only correct approach is to annotate the final version with
virtual addresses, and pass /that/ address to the kernel. So the bug
is arguably that we pass the wrong version of the memory map to the
OS.
I think we need to fix this by using fdt_setprop_inplace() to poke the
correct value into the FDT after ExitBootServices() returns.
--
Ard.
> drivers/firmware/efi/libstub/fdt.c | 23 +++++++++++------------
> 1 file changed, 11 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c
> index a6a93116a8f0..f623894c633c 100644
> --- a/drivers/firmware/efi/libstub/fdt.c
> +++ b/drivers/firmware/efi/libstub/fdt.c
> @@ -181,10 +181,6 @@ static efi_status_t exit_boot_func(efi_system_table_t *sys_table_arg,
> * which are fixed at 4K bytes, so in most cases the first
> * allocation should succeed.
> * EFI boot services are exited at the end of this function.
> - * There must be no allocations between the get_memory_map()
> - * call and the exit_boot_services() call, so the exiting of
> - * boot services is very tightly tied to the creation of the FDT
> - * with the final memory map in it.
> */
>
> efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
> @@ -199,7 +195,7 @@ efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
> unsigned long map_size, desc_size, buff_size;
> u32 desc_ver;
> unsigned long mmap_key;
> - efi_memory_desc_t *memory_map, *runtime_map;
> + efi_memory_desc_t *memory_map, *runtime_map, *exit_map;
> unsigned long new_fdt_size;
> efi_status_t status;
> int runtime_entry_count = 0;
> @@ -243,11 +239,6 @@ efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
> goto fail;
> }
>
> - /*
> - * Now that we have done our final memory allocation (and free)
> - * we can get the memory map key needed for
> - * exit_boot_services().
> - */
> status = efi_get_memory_map(sys_table, &map);
> if (status != EFI_SUCCESS)
> goto fail_free_new_fdt;
> @@ -259,8 +250,16 @@ efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
> memory_map, map_size, desc_size, desc_ver);
>
> /* Succeeding the first time is the expected case. */
> - if (status == EFI_SUCCESS)
> + if (status == EFI_SUCCESS) {
> + /*
> + * Annotate the memory_map in the FDT with virtual
> + * addresses. These shouldn't change as the placement
> + * of EFI_MEMORY_RUNTIME regions should be fixed.
> + */
> + efi_get_virtmap(memory_map, map_size, desc_size,
> + runtime_map, &runtime_entry_count);
> break;
> + }
>
> if (status == EFI_BUFFER_TOO_SMALL) {
> /*
> @@ -279,7 +278,7 @@ efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
> }
> }
>
> - sys_table->boottime->free_pool(memory_map);
> + map.map = &exit_map;
> priv.runtime_map = runtime_map;
> priv.runtime_entry_count = &runtime_entry_count;
> status = efi_exit_boot_services(sys_table, handle, &map, &priv,
> --
> 2.10.1
>
^ permalink raw reply
* Tearing down DMA transfer setup after DMA client has finished
From: Mason @ 2016-12-09 17:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161209171727.GK6408@localhost>
On 09/12/2016 18:17, Vinod Koul wrote:
> On Fri, Dec 09, 2016 at 11:25:57AM +0100, Sebastian Frias wrote:
>>
>> What concrete solution do you propose?
>
> I have already proposed two solutions.
>
> A) Request a channel only when you need it. Obviously we can't do virtual
> channels with this (though we should still use virt-channels framework).
> The sbox setup and teardown can be done as part of channel request and
> freeup. PL08x already does this.
>
> Downside is that we can only have as many consumers at a time as channels.
>
> I have not heard any technical reason for not doing this apart from drivers
> grab the channel at probe, which is incorrect and needs to be fixed
> irrespective of the problem at hand.
>
> This is my preferred option.
There is one important drawback with this solution. If a driver calls
dma_request_chan() when no channels are currently available, it will
get -EBUSY. If there were a flag in dma_request_chan to be put to
sleep (with timeout) until a channel is available, then it would
work. But busy waiting in the client driver is a waste of power.
Regards.
^ permalink raw reply
* Tearing down DMA transfer setup after DMA client has finished
From: Måns Rullgård @ 2016-12-09 17:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161209171727.GK6408@localhost>
Vinod Koul <vinod.koul@intel.com> writes:
> On Fri, Dec 09, 2016 at 11:25:57AM +0100, Sebastian Frias wrote:
>>
>> What concrete solution do you propose?
>
> I have already proposed two solutions.
>
> A) Request a channel only when you need it. Obviously we can't do virtual
> channels with this (though we should still use virt-channels framework).
> The sbox setup and teardown can be done as part of channel request and
> freeup. PL08x already does this.
>
> Downside is that we can only have as many consumers at a time as channels.
>
> I have not heard any technical reason for not doing this apart from drivers
> grab the channel at probe, which is incorrect and needs to be fixed
> irrespective of the problem at hand.
>
> This is my preferred option.
Sorry, but this is not acceptable.
> B) Create a custom driver specific API. This API for example:
> sbox_setup(bool enable, ...)
> can be called by client to explicitly setup and clear up the sbox setting.
>
> This way we can have transactions muxed.
>
> I have not heard any arguments on why we shouldn't do this except Russell's
> comment that A) solves this.
Driver-specific interfaces are not the solution. That way lies chaos
and madness.
This would all be so much easier if you all would just shut up for a
moment and let me fix it properly.
--
M?ns Rullg?rd
^ permalink raw reply
* [PATCH v4] arm64: fpsimd: improve stacking logic in non-interruptible context
From: Dave Martin @ 2016-12-09 17:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481301992-2344-1-git-send-email-ard.biesheuvel@linaro.org>
On Fri, Dec 09, 2016 at 04:46:32PM +0000, Ard Biesheuvel wrote:
> Currently, we allow kernel mode NEON in softirq or hardirq context by
> stacking and unstacking a slice of the NEON register file for each call
> to kernel_neon_begin() and kernel_neon_end(), respectively.
>
> Given that
> a) a CPU typically spends most of its time in userland, during which time
> no kernel mode NEON in process context is in progress,
> b) a CPU spends most of its time in the kernel doing other things than
> kernel mode NEON when it gets interrupted to perform kernel mode NEON
> in softirq context
>
> the stacking and subsequent unstacking is only necessary if we are
> interrupting a thread while it is performing kernel mode NEON in process
> context, which means that in all other cases, we can simply preserve the
> userland FPSIMD state once, and only restore it upon return to userland,
> even if we are being invoked from softirq or hardirq context.
>
> So instead of checking whether we are running in interrupt context, keep
> track of the level of nested kernel mode NEON calls in progress, and only
> perform the eager stack/unstack if the level exceeds 1.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
This looks good to me.
This should also make the SVE case trivial now: there can only be live
SVE state when in process context with !TIF_FOREIGN_FPSTATE, and the
SVE save/restore is then handled by fpsimd_{save,load}_state()
directly. For deeper nesting levels, there is already no live SVE
state, so kernel_neon_{save,load}_partial_state() are enough in that
case.
As and when KERNEL_MODE_SVE comes along this will need another look,
but this patch looks like a step forward for now.
Reviewed-by: Dave Martin <Dave.Martin@arm.com>
> ---
> v4:
> - use this_cpu_inc/dec, which give sufficient guarantees regarding
> concurrency, but do not imply SMP barriers, which are not needed here
>
> v3:
> - avoid corruption by concurrent invocations of kernel_neon_begin()/_end()
>
> v2:
> - BUG() on unexpected values of the nesting level
> - relax the BUG() on num_regs>32 to a WARN, given that nothing actually
> breaks in that case
>
> arch/arm64/kernel/fpsimd.c | 47 ++++++++++++++------
> 1 file changed, 33 insertions(+), 14 deletions(-)
>
> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
> index 394c61db5566..37d6dfc9059b 100644
> --- a/arch/arm64/kernel/fpsimd.c
> +++ b/arch/arm64/kernel/fpsimd.c
> @@ -220,20 +220,35 @@ void fpsimd_flush_task_state(struct task_struct *t)
>
> #ifdef CONFIG_KERNEL_MODE_NEON
>
> -static DEFINE_PER_CPU(struct fpsimd_partial_state, hardirq_fpsimdstate);
> -static DEFINE_PER_CPU(struct fpsimd_partial_state, softirq_fpsimdstate);
> +/*
> + * Although unlikely, it is possible for three kernel mode NEON contexts to
> + * be live at the same time: process context, softirq context and hardirq
> + * context. So while the userland context is stashed in the thread's fpsimd
> + * state structure, we need two additional levels of storage.
> + */
> +static DEFINE_PER_CPU(struct fpsimd_partial_state, nested_fpsimdstate[2]);
> +static DEFINE_PER_CPU(int, kernel_neon_nesting_level);
>
> /*
> * Kernel-side NEON support functions
> */
> void kernel_neon_begin_partial(u32 num_regs)
> {
> - if (in_interrupt()) {
> - struct fpsimd_partial_state *s = this_cpu_ptr(
> - in_irq() ? &hardirq_fpsimdstate : &softirq_fpsimdstate);
> + struct fpsimd_partial_state *s;
> + int level;
> +
> + preempt_disable();
> +
> + level = this_cpu_inc_return(kernel_neon_nesting_level);
> + BUG_ON(level > 3);
> +
> + if (level > 1) {
> + s = this_cpu_ptr(nested_fpsimdstate);
>
> - BUG_ON(num_regs > 32);
> - fpsimd_save_partial_state(s, roundup(num_regs, 2));
> + WARN_ON_ONCE(num_regs > 32);
> + num_regs = min(roundup(num_regs, 2), 32U);
> +
> + fpsimd_save_partial_state(&s[level - 2], num_regs);
> } else {
> /*
> * Save the userland FPSIMD state if we have one and if we
> @@ -241,7 +256,6 @@ void kernel_neon_begin_partial(u32 num_regs)
> * that there is no longer userland FPSIMD state in the
> * registers.
> */
> - preempt_disable();
> if (current->mm &&
> !test_and_set_thread_flag(TIF_FOREIGN_FPSTATE))
> fpsimd_save_state(¤t->thread.fpsimd_state);
> @@ -252,13 +266,18 @@ EXPORT_SYMBOL(kernel_neon_begin_partial);
>
> void kernel_neon_end(void)
> {
> - if (in_interrupt()) {
> - struct fpsimd_partial_state *s = this_cpu_ptr(
> - in_irq() ? &hardirq_fpsimdstate : &softirq_fpsimdstate);
> - fpsimd_load_partial_state(s);
> - } else {
> - preempt_enable();
> + struct fpsimd_partial_state *s;
> + int level;
> +
> + level = this_cpu_read(kernel_neon_nesting_level);
> + BUG_ON(level < 1);
> +
> + if (level > 1) {
> + s = this_cpu_ptr(nested_fpsimdstate);
> + fpsimd_load_partial_state(&s[level - 2]);
> }
> + this_cpu_dec(kernel_neon_nesting_level);
> + preempt_enable();
> }
> EXPORT_SYMBOL(kernel_neon_end);
>
> --
> 2.7.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Tearing down DMA transfer setup after DMA client has finished
From: Vinod Koul @ 2016-12-09 17:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <6ce1ea97-1d68-2203-c7b4-73315e801655@laposte.net>
On Fri, Dec 09, 2016 at 11:25:57AM +0100, Sebastian Frias wrote:
>
> What concrete solution do you propose?
I have already proposed two solutions.
A) Request a channel only when you need it. Obviously we can't do virtual
channels with this (though we should still use virt-channels framework).
The sbox setup and teardown can be done as part of channel request and
freeup. PL08x already does this.
Downside is that we can only have as many consumers at a time as channels.
I have not heard any technical reason for not doing this apart from drivers
grab the channel at probe, which is incorrect and needs to be fixed
irrespective of the problem at hand.
This is my preferred option.
B) Create a custom driver specific API. This API for example:
sbox_setup(bool enable, ...)
can be called by client to explicitly setup and clear up the sbox setting.
This way we can have transactions muxed.
I have not heard any arguments on why we shouldn't do this except Russell's
comment that A) solves this.
> Alternatively, one can think of the current issue (i.e.: the fact that the IRQ
> arrives "too soon") in a different way.
> Instead of thinking the IRQ indicates "transfer complete", it is indicating "ready
> to accept another command", which in practice (and with proper API support) can
> translate into efficient queuing of DMA operations.
That IMO is a better understanding of this issue. But based on discussion, I
think the issue is that submitting next transaction cannot be done until
sbox is setup (as a consequence torn down for previous). Tear down can be
down only when client knows that transfer is done, from dma controller data
has been pushed and in-flight.
--
~Vinod
^ permalink raw reply
* [BUG/RFC] efi/libstub: Preserve the memory map pointed to by the FDT
From: James Morse @ 2016-12-09 17:15 UTC (permalink / raw)
To: linux-arm-kernel
allocate_new_fdt_and_exit_boot() bakes a pointer to the UEFI memory map
into the FDT for later use. The corresponding memory is then free()d
and (hopefully) allocated again via efi_exit_boot_services() calling
efi_get_memory_map() as part of the exit_boot_services() loop. The
final copy is annotated with virtual mappings and used to create the
runtime map.
Linux expects the FDT to contain a pointer to the UEFI memory map
with the virtual mapping annotations, which will only happen if
AllocatePool() returns memory to the stub that was just FreePool()d
by the stub. This behaviour doesn't appear to guaranteed by the spec.
Platforms that don't do this will use the stale memory map (assuming
no later owner of the freed() memory changed it) and fail to initialise
runtime services.
Instead, keep the memory map we baked info the FDT, and create a new
'exit_map' pointer for use in the exit_boot_services() loop. (This was
already different, it just had the same name and we hoped it would be
in the same place).
Annotate the memory map pointed to by the FDT with virtual mappings.
This assumes the final memory map (which may be different), has
not moved any of the regions with the runtime attribute.
(Take the opportunity to remove some comments that are stale since
commit ed9cc156c42f ("efi/libstub: Use efi_exit_boot_services() in FDT"))
Signed-off-by: James Morse <james.morse@arm.com>
Fixes: ed9cc156c42f ("efi/libstub: Use efi_exit_boot_services() in FDT")
Cc: Jeffrey Hugo <jhugo@codeaurora.org>
---
I haven't seen this causing problems on any real platforms, only kvmtool
which I'm busily hacking up to have primitive UEFI support. Needless to
say my AllocatePool() doesn't re-use memory.
drivers/firmware/efi/libstub/fdt.c | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c
index a6a93116a8f0..f623894c633c 100644
--- a/drivers/firmware/efi/libstub/fdt.c
+++ b/drivers/firmware/efi/libstub/fdt.c
@@ -181,10 +181,6 @@ static efi_status_t exit_boot_func(efi_system_table_t *sys_table_arg,
* which are fixed at 4K bytes, so in most cases the first
* allocation should succeed.
* EFI boot services are exited at the end of this function.
- * There must be no allocations between the get_memory_map()
- * call and the exit_boot_services() call, so the exiting of
- * boot services is very tightly tied to the creation of the FDT
- * with the final memory map in it.
*/
efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
@@ -199,7 +195,7 @@ efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
unsigned long map_size, desc_size, buff_size;
u32 desc_ver;
unsigned long mmap_key;
- efi_memory_desc_t *memory_map, *runtime_map;
+ efi_memory_desc_t *memory_map, *runtime_map, *exit_map;
unsigned long new_fdt_size;
efi_status_t status;
int runtime_entry_count = 0;
@@ -243,11 +239,6 @@ efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
goto fail;
}
- /*
- * Now that we have done our final memory allocation (and free)
- * we can get the memory map key needed for
- * exit_boot_services().
- */
status = efi_get_memory_map(sys_table, &map);
if (status != EFI_SUCCESS)
goto fail_free_new_fdt;
@@ -259,8 +250,16 @@ efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
memory_map, map_size, desc_size, desc_ver);
/* Succeeding the first time is the expected case. */
- if (status == EFI_SUCCESS)
+ if (status == EFI_SUCCESS) {
+ /*
+ * Annotate the memory_map in the FDT with virtual
+ * addresses. These shouldn't change as the placement
+ * of EFI_MEMORY_RUNTIME regions should be fixed.
+ */
+ efi_get_virtmap(memory_map, map_size, desc_size,
+ runtime_map, &runtime_entry_count);
break;
+ }
if (status == EFI_BUFFER_TOO_SMALL) {
/*
@@ -279,7 +278,7 @@ efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
}
}
- sys_table->boottime->free_pool(memory_map);
+ map.map = &exit_map;
priv.runtime_map = runtime_map;
priv.runtime_entry_count = &runtime_entry_count;
status = efi_exit_boot_services(sys_table, handle, &map, &priv,
--
2.10.1
^ permalink raw reply related
* [PATCH v8 04/16] MSI-X: update GSI routing after changed MSI-X configuration
From: Marc Zyngier @ 2016-12-09 17:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161104173203.21168-5-andre.przywara@arm.com>
On 04/11/16 17:31, Andre Przywara wrote:
> When we set up GSI routing to map MSIs to KVM's GSI numbers, we
> write the current device's MSI setup into the kernel routing table.
> However the device driver in the guest can use PCI configuration space
> accesses to change the MSI configuration (address and/or payload data).
> Whenever this happens after we have setup the routing table already,
> we must amend the previously sent data.
> So when MSI-X PCI config space accesses write address or payload,
> find the associated GSI number and the matching routing table entry
> and update the kernel routing table (only if the data has changed).
>
> This fixes vhost-net, where the queue's IRQFD was setup before the
> MSI vectors.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
> include/kvm/irq.h | 1 +
> irq.c | 31 +++++++++++++++++++++++++++++++
> virtio/pci.c | 36 +++++++++++++++++++++++++++++++++---
> 3 files changed, 65 insertions(+), 3 deletions(-)
>
> diff --git a/include/kvm/irq.h b/include/kvm/irq.h
> index bb71521..f35eb7e 100644
> --- a/include/kvm/irq.h
> +++ b/include/kvm/irq.h
> @@ -21,5 +21,6 @@ int irq__exit(struct kvm *kvm);
>
> int irq__allocate_routing_entry(void);
> int irq__add_msix_route(struct kvm *kvm, struct msi_msg *msg);
> +void irq__update_msix_route(struct kvm *kvm, u32 gsi, struct msi_msg *msg);
>
> #endif
> diff --git a/irq.c b/irq.c
> index a742aa2..895e5eb 100644
> --- a/irq.c
> +++ b/irq.c
> @@ -93,6 +93,37 @@ int irq__add_msix_route(struct kvm *kvm, struct msi_msg *msg)
> return next_gsi++;
> }
>
> +static bool update_data(u32 *ptr, u32 newdata)
> +{
> + if (*ptr == newdata)
> + return false;
> +
> + *ptr = newdata;
> + return true;
> +}
> +
> +void irq__update_msix_route(struct kvm *kvm, u32 gsi, struct msi_msg *msg)
> +{
> + struct kvm_irq_routing_msi *entry;
> + unsigned int i;
> + bool changed;
> +
> + for (i = 0; i < irq_routing->nr; i++)
> + if (gsi == irq_routing->entries[i].gsi)
> + break;
> + if (i == irq_routing->nr)
> + return;
> +
> + entry = &irq_routing->entries[i].u.msi;
> +
> + changed = update_data(&entry->address_hi, msg->address_hi);
> + changed |= update_data(&entry->address_lo, msg->address_lo);
> + changed |= update_data(&entry->data, msg->data);
> +
> + if (changed)
> + ioctl(kvm->vm_fd, KVM_SET_GSI_ROUTING, irq_routing);
Check the return value and let the caller know if something has failed?
> +}
> +
> int __attribute__((weak)) irq__exit(struct kvm *kvm)
> {
> free(irq_routing);
> diff --git a/virtio/pci.c b/virtio/pci.c
> index 072e5b7..b3b4aac 100644
> --- a/virtio/pci.c
> +++ b/virtio/pci.c
> @@ -152,6 +152,30 @@ static bool virtio_pci__io_in(struct ioport *ioport, struct kvm_cpu *vcpu, u16 p
> return ret;
> }
>
> +static void update_msix_map(struct virtio_pci *vpci,
> + struct msix_table *msix_entry, u32 vecnum)
> +{
> + u32 gsi, i;
> +
> + /* Find the GSI number used for that vector */
> + if (vecnum == vpci->config_vector) {
> + gsi = vpci->config_gsi;
> + } else {
> + for (i = 0; i < VIRTIO_PCI_MAX_VQ; i++)
> + if (vpci->vq_vector[i] == vecnum)
> + break;
> + if (i == VIRTIO_PCI_MAX_VQ)
> + return;
> + gsi = vpci->gsis[i];
> + }
> +
> + if (gsi == 0)
> + return;
> +
> + msix_entry = &msix_entry[vecnum];
> + irq__update_msix_route(vpci->kvm, gsi, &msix_entry->msg);
> +}
> +
> static bool virtio_pci__specific_io_out(struct kvm *kvm, struct virtio_device *vdev, u16 port,
> void *data, int size, int offset)
> {
> @@ -270,10 +294,16 @@ static void virtio_pci__msix_mmio_callback(struct kvm_cpu *vcpu,
> offset = vpci->msix_io_block;
> }
>
> - if (is_write)
> - memcpy(table + addr - offset, data, len);
> - else
> + if (!is_write) {
> memcpy(data, table + addr - offset, len);
> + return;
> + }
> +
> + memcpy(table + addr - offset, data, len);
> +
> + /* Did we just update the address or payload? */
> + if (addr % 0x10 < 0xc)
> + update_msix_map(vpci, table, (addr - offset) / 16);
Where are these constants coming from? Please stick to either decimal or
hex...
> }
>
> static void virtio_pci__signal_msi(struct kvm *kvm, struct virtio_pci *vpci, int vec)
>
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* [PATCH v7 2/2] ARM: davinci: da8xx: Fix sleeping function called from invalid context
From: Alexandre Bailon @ 2016-12-09 16:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481302773-14181-1-git-send-email-abailon@baylibre.com>
Everytime the usb20 phy is enabled, there is a
"sleeping function called from invalid context" BUG.
In addition, there is a recursive locking happening
because of the recurse call to clk_enable().
clk_enable() from arch/arm/mach-davinci/clock.c uses spin_lock_irqsave()
before to invoke the callback usb20_phy_clk_enable().
usb20_phy_clk_enable() uses clk_get() and clk_enable_prepapre()
which may sleep.
replace clk_prepare_enable() by davinci_clk_enable().
Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
Suggested-by: David Lechner <david@lechnology.com>
---
arch/arm/mach-davinci/usb-da8xx.c | 34 +++++++++++++++-------------------
1 file changed, 15 insertions(+), 19 deletions(-)
diff --git a/arch/arm/mach-davinci/usb-da8xx.c b/arch/arm/mach-davinci/usb-da8xx.c
index b010e5f..71f1e81 100644
--- a/arch/arm/mach-davinci/usb-da8xx.c
+++ b/arch/arm/mach-davinci/usb-da8xx.c
@@ -22,6 +22,8 @@
#define DA8XX_USB0_BASE 0x01e00000
#define DA8XX_USB1_BASE 0x01e25000
+static struct clk *usb20_clk;
+
static struct platform_device da8xx_usb_phy = {
.name = "da8xx-usb-phy",
.id = -1,
@@ -158,26 +160,13 @@ int __init da8xx_register_usb_refclkin(int rate)
static void usb20_phy_clk_enable(struct clk *clk)
{
- struct clk *usb20_clk;
- int err;
u32 val;
u32 timeout = 500000; /* 500 msec */
val = readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
- usb20_clk = clk_get(&da8xx_usb20_dev.dev, "usb20");
- if (IS_ERR(usb20_clk)) {
- pr_err("could not get usb20 clk: %ld\n", PTR_ERR(usb20_clk));
- return;
- }
-
/* The USB 2.O PLL requires that the USB 2.O PSC is enabled as well. */
- err = clk_prepare_enable(usb20_clk);
- if (err) {
- pr_err("failed to enable usb20 clk: %d\n", err);
- clk_put(usb20_clk);
- return;
- }
+ davinci_clk_enable(usb20_clk);
/*
* Turn on the USB 2.0 PHY, but just the PLL, and not OTG. The USB 1.1
@@ -197,8 +186,7 @@ static void usb20_phy_clk_enable(struct clk *clk)
pr_err("Timeout waiting for USB 2.0 PHY clock good\n");
done:
- clk_disable_unprepare(usb20_clk);
- clk_put(usb20_clk);
+ davinci_clk_disable(usb20_clk);
}
static void usb20_phy_clk_disable(struct clk *clk)
@@ -285,11 +273,19 @@ static struct clk_lookup usb20_phy_clk_lookup =
int __init da8xx_register_usb20_phy_clk(bool use_usb_refclkin)
{
struct clk *parent;
- int ret = 0;
+ int ret;
+
+ usb20_clk = clk_get(&da8xx_usb20_dev.dev, "usb20");
+ ret = PTR_ERR_OR_ZERO(usb20_clk);
+ if (ret)
+ return ret;
parent = clk_get(NULL, use_usb_refclkin ? "usb_refclkin" : "pll0_aux");
- if (IS_ERR(parent))
- return PTR_ERR(parent);
+ ret = PTR_ERR_OR_ZERO(parent);
+ if (ret) {
+ clk_put(usb20_clk);
+ return ret;
+ }
usb20_phy_clk.parent = parent;
ret = clk_register(&usb20_phy_clk);
--
2.7.3
^ permalink raw reply related
* [PATCH v7 1/2] provide lock-less versions of clk_{enable|disable}
From: Alexandre Bailon @ 2016-12-09 16:59 UTC (permalink / raw)
To: linux-arm-kernel
Rename __clk_{enable|disble} in davinci_clk_{enable|disable}.
davinci_clk_{enable|disable} is a lock-less version of
clk_{enable|disable}.
This is useful to recursively enable clock without doing recursive call
to clk_enable(), which would cause a recursive locking issue.
The lock-less version could be used by example by the usb20 phy clock,
that need to enable the usb20 clock before to start.
Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
Suggested-by: David Lechner <david@lechnology.com>
---
arch/arm/mach-davinci/clock.c | 12 ++++++------
arch/arm/mach-davinci/clock.h | 2 ++
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/arch/arm/mach-davinci/clock.c b/arch/arm/mach-davinci/clock.c
index df42c93..f5dce9b 100644
--- a/arch/arm/mach-davinci/clock.c
+++ b/arch/arm/mach-davinci/clock.c
@@ -31,10 +31,10 @@ static LIST_HEAD(clocks);
static DEFINE_MUTEX(clocks_mutex);
static DEFINE_SPINLOCK(clockfw_lock);
-static void __clk_enable(struct clk *clk)
+void davinci_clk_enable(struct clk *clk)
{
if (clk->parent)
- __clk_enable(clk->parent);
+ davinci_clk_enable(clk->parent);
if (clk->usecount++ == 0) {
if (clk->flags & CLK_PSC)
davinci_psc_config(clk->domain, clk->gpsc, clk->lpsc,
@@ -44,7 +44,7 @@ static void __clk_enable(struct clk *clk)
}
}
-static void __clk_disable(struct clk *clk)
+void davinci_clk_disable(struct clk *clk)
{
if (WARN_ON(clk->usecount == 0))
return;
@@ -56,7 +56,7 @@ static void __clk_disable(struct clk *clk)
clk->clk_disable(clk);
}
if (clk->parent)
- __clk_disable(clk->parent);
+ davinci_clk_disable(clk->parent);
}
int davinci_clk_reset(struct clk *clk, bool reset)
@@ -103,7 +103,7 @@ int clk_enable(struct clk *clk)
return -EINVAL;
spin_lock_irqsave(&clockfw_lock, flags);
- __clk_enable(clk);
+ davinci_clk_enable(clk);
spin_unlock_irqrestore(&clockfw_lock, flags);
return 0;
@@ -118,7 +118,7 @@ void clk_disable(struct clk *clk)
return;
spin_lock_irqsave(&clockfw_lock, flags);
- __clk_disable(clk);
+ davinci_clk_disable(clk);
spin_unlock_irqrestore(&clockfw_lock, flags);
}
EXPORT_SYMBOL(clk_disable);
diff --git a/arch/arm/mach-davinci/clock.h b/arch/arm/mach-davinci/clock.h
index e2a5437..fa2b837 100644
--- a/arch/arm/mach-davinci/clock.h
+++ b/arch/arm/mach-davinci/clock.h
@@ -132,6 +132,8 @@ int davinci_set_sysclk_rate(struct clk *clk, unsigned long rate);
int davinci_set_refclk_rate(unsigned long rate);
int davinci_simple_set_rate(struct clk *clk, unsigned long rate);
int davinci_clk_reset(struct clk *clk, bool reset);
+void davinci_clk_enable(struct clk *clk);
+void davinci_clk_disable(struct clk *clk);
extern struct platform_device davinci_wdt_device;
extern void davinci_watchdog_reset(struct platform_device *);
--
2.7.3
^ permalink raw reply related
* [PATCH v4] arm64: fpsimd: improve stacking logic in non-interruptible context
From: Ard Biesheuvel @ 2016-12-09 16:46 UTC (permalink / raw)
To: linux-arm-kernel
Currently, we allow kernel mode NEON in softirq or hardirq context by
stacking and unstacking a slice of the NEON register file for each call
to kernel_neon_begin() and kernel_neon_end(), respectively.
Given that
a) a CPU typically spends most of its time in userland, during which time
no kernel mode NEON in process context is in progress,
b) a CPU spends most of its time in the kernel doing other things than
kernel mode NEON when it gets interrupted to perform kernel mode NEON
in softirq context
the stacking and subsequent unstacking is only necessary if we are
interrupting a thread while it is performing kernel mode NEON in process
context, which means that in all other cases, we can simply preserve the
userland FPSIMD state once, and only restore it upon return to userland,
even if we are being invoked from softirq or hardirq context.
So instead of checking whether we are running in interrupt context, keep
track of the level of nested kernel mode NEON calls in progress, and only
perform the eager stack/unstack if the level exceeds 1.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
v4:
- use this_cpu_inc/dec, which give sufficient guarantees regarding
concurrency, but do not imply SMP barriers, which are not needed here
v3:
- avoid corruption by concurrent invocations of kernel_neon_begin()/_end()
v2:
- BUG() on unexpected values of the nesting level
- relax the BUG() on num_regs>32 to a WARN, given that nothing actually
breaks in that case
arch/arm64/kernel/fpsimd.c | 47 ++++++++++++++------
1 file changed, 33 insertions(+), 14 deletions(-)
diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index 394c61db5566..37d6dfc9059b 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -220,20 +220,35 @@ void fpsimd_flush_task_state(struct task_struct *t)
#ifdef CONFIG_KERNEL_MODE_NEON
-static DEFINE_PER_CPU(struct fpsimd_partial_state, hardirq_fpsimdstate);
-static DEFINE_PER_CPU(struct fpsimd_partial_state, softirq_fpsimdstate);
+/*
+ * Although unlikely, it is possible for three kernel mode NEON contexts to
+ * be live at the same time: process context, softirq context and hardirq
+ * context. So while the userland context is stashed in the thread's fpsimd
+ * state structure, we need two additional levels of storage.
+ */
+static DEFINE_PER_CPU(struct fpsimd_partial_state, nested_fpsimdstate[2]);
+static DEFINE_PER_CPU(int, kernel_neon_nesting_level);
/*
* Kernel-side NEON support functions
*/
void kernel_neon_begin_partial(u32 num_regs)
{
- if (in_interrupt()) {
- struct fpsimd_partial_state *s = this_cpu_ptr(
- in_irq() ? &hardirq_fpsimdstate : &softirq_fpsimdstate);
+ struct fpsimd_partial_state *s;
+ int level;
+
+ preempt_disable();
+
+ level = this_cpu_inc_return(kernel_neon_nesting_level);
+ BUG_ON(level > 3);
+
+ if (level > 1) {
+ s = this_cpu_ptr(nested_fpsimdstate);
- BUG_ON(num_regs > 32);
- fpsimd_save_partial_state(s, roundup(num_regs, 2));
+ WARN_ON_ONCE(num_regs > 32);
+ num_regs = min(roundup(num_regs, 2), 32U);
+
+ fpsimd_save_partial_state(&s[level - 2], num_regs);
} else {
/*
* Save the userland FPSIMD state if we have one and if we
@@ -241,7 +256,6 @@ void kernel_neon_begin_partial(u32 num_regs)
* that there is no longer userland FPSIMD state in the
* registers.
*/
- preempt_disable();
if (current->mm &&
!test_and_set_thread_flag(TIF_FOREIGN_FPSTATE))
fpsimd_save_state(¤t->thread.fpsimd_state);
@@ -252,13 +266,18 @@ EXPORT_SYMBOL(kernel_neon_begin_partial);
void kernel_neon_end(void)
{
- if (in_interrupt()) {
- struct fpsimd_partial_state *s = this_cpu_ptr(
- in_irq() ? &hardirq_fpsimdstate : &softirq_fpsimdstate);
- fpsimd_load_partial_state(s);
- } else {
- preempt_enable();
+ struct fpsimd_partial_state *s;
+ int level;
+
+ level = this_cpu_read(kernel_neon_nesting_level);
+ BUG_ON(level < 1);
+
+ if (level > 1) {
+ s = this_cpu_ptr(nested_fpsimdstate);
+ fpsimd_load_partial_state(&s[level - 2]);
}
+ this_cpu_dec(kernel_neon_nesting_level);
+ preempt_enable();
}
EXPORT_SYMBOL(kernel_neon_end);
--
2.7.4
^ permalink raw reply related
* [PATCH 1/3] rtc: armada38x: improve RTC errata implementation
From: Gregory CLEMENT @ 2016-12-09 16:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161208173717.GJ14217@n2100.armlinux.org.uk>
Hi Russell King,
On jeu., d?c. 08 2016, Russell King - ARM Linux <linux@armlinux.org.uk> wrote:
> On Thu, Dec 08, 2016 at 06:10:08PM +0100, Gregory CLEMENT wrote:
>> From: Shaker Daibes <shaker@marvell.com>
>>
>> According to FE-3124064:
>> The device supports CPU write and read access to the RTC time register.
>> However, due to this errata, read from RTC TIME register may fail.
>>
>> Workaround:
>> General configuration:
>> 1. Configure the RTC Mbus Bridge Timing Control register (offset 0x184A0)
>> to value 0xFD4D4FFF
>> Write RTC WRCLK Period to its maximum value (0x3FF)
>> Write RTC WRCLK setup to 0x53 (default value )
>> Write RTC WRCLK High Time to 0x53 (default value )
>> Write RTC Read Output Delay to its maximum value (0x1F)
>> Mbus - Read All Byte Enable to 0x1 (default value )
>> 2. Configure the RTC Test Configuration Register (offset 0xA381C) bit3
>> to '1' (Reserved, Marvell internal)
>>
>> For any RTC register read operation:
>> 1. Read the requested register 100 times.
>> 2. Find the result that appears most frequently and use this result
>> as the correct value.
>>
>> For any RTC register write operation:
>> 1. Issue two dummy writes of 0x0 to the RTC Status register (offset
>> 0xA3800).
>> 2. Write the time to the RTC Time register (offset 0xA380C).
>>
>> [gregory.clement at free-electrons.com: cosmetic changes and fix issues for
>> interrupt in original patch]
>
> A particularly interesting question here is whether the above description
> came first or whether the code below came first, and which was derived
> from which.
Well, it is difficult to say. the above description comes from the
errata datasheet (since rev B). But the errata sheet itself mentioned
the software implementation in one of the Marvell release.
>
> Given that both readl() writel() involves a barrier operation, which is
> not mentioned in the above workaround text, is it appropriate to use the
> barriered operations?
Do you suggest to use the the relaxed version of readl() and writel()?
>
>>
>> Reviewed-by: Lior Amsalem <alior@marvell.com>
>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>> ---
>> drivers/rtc/rtc-armada38x.c | 109 ++++++++++++++++++++++++++++++++++----------
>> 1 file changed, 85 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/rtc/rtc-armada38x.c b/drivers/rtc/rtc-armada38x.c
>> index 9a3f2a6f512e..a0859286a4c4 100644
>> --- a/drivers/rtc/rtc-armada38x.c
>> +++ b/drivers/rtc/rtc-armada38x.c
>> @@ -29,12 +29,21 @@
>> #define RTC_TIME 0xC
>> #define RTC_ALARM1 0x10
>>
>> +#define SOC_RTC_BRIDGE_TIMING_CTL 0x0
>> +#define SOC_RTC_PERIOD_OFFS 0
>> +#define SOC_RTC_PERIOD_MASK (0x3FF << SOC_RTC_PERIOD_OFFS)
>> +#define SOC_RTC_READ_DELAY_OFFS 26
>> +#define SOC_RTC_READ_DELAY_MASK (0x1F << SOC_RTC_READ_DELAY_OFFS)
>> +
>> #define SOC_RTC_INTERRUPT 0x8
>> #define SOC_RTC_ALARM1 BIT(0)
>> #define SOC_RTC_ALARM2 BIT(1)
>> #define SOC_RTC_ALARM1_MASK BIT(2)
>> #define SOC_RTC_ALARM2_MASK BIT(3)
>>
>> +
>> +#define SAMPLE_NR 100
>> +
>> struct armada38x_rtc {
>> struct rtc_device *rtc_dev;
>> void __iomem *regs;
>> @@ -47,32 +56,85 @@ struct armada38x_rtc {
>> * According to the datasheet, the OS should wait 5us after every
>> * register write to the RTC hard macro so that the required update
>> * can occur without holding off the system bus
>> + * According to errata FE-3124064, Write to any RTC register
>> + * may fail. As a workaround, before writing to RTC
>> + * register, issue a dummy write of 0x0 twice to RTC Status
>> + * register.
>> */
>> +
>> static void rtc_delayed_write(u32 val, struct armada38x_rtc *rtc, int offset)
>> {
>> + writel(0, rtc->regs + RTC_STATUS);
>> + writel(0, rtc->regs + RTC_STATUS);
>> writel(val, rtc->regs + offset);
>> udelay(5);
>> }
>>
>> +/* Update RTC-MBUS bridge timing parameters */
>> +static void rtc_update_mbus_timing_params(struct armada38x_rtc *rtc)
>> +{
>> + uint32_t reg;
>> +
>> + reg = readl(rtc->regs_soc + SOC_RTC_BRIDGE_TIMING_CTL);
>> + reg &= ~SOC_RTC_PERIOD_MASK;
>> + reg |= 0x3FF << SOC_RTC_PERIOD_OFFS; /* Maximum value */
>> + reg &= ~SOC_RTC_READ_DELAY_MASK;
>> + reg |= 0x1F << SOC_RTC_READ_DELAY_OFFS; /* Maximum value */
>> + writel(reg, rtc->regs_soc + SOC_RTC_BRIDGE_TIMING_CTL);
>> +}
>> +
>> +struct str_value_to_freq {
>> + unsigned long value;
>> + u8 freq;
>> +} __packed;
>
> Why packed? This makes accesses to this structure very inefficient -
> the compiler for some CPUs will load "value" by bytes.
As pointed by Andrew, I think the intent was to reduce the amount of
memory used from the stack.
Thanks,
Gregory
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply
* [PATCH 1/3] rtc: armada38x: improve RTC errata implementation
From: Andrew Lunn @ 2016-12-09 16:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <8760mtf538.fsf@free-electrons.com>
On Fri, Dec 09, 2016 at 05:19:07PM +0100, Gregory CLEMENT wrote:
> Hi Andrew,
>
> On jeu., d?c. 08 2016, Andrew Lunn <andrew@lunn.ch> wrote:
>
> >> +struct str_value_to_freq {
> >> + unsigned long value;
> >> + u8 freq;
> >> +} __packed;
> >> +
> >> +static unsigned long read_rtc_register_wa(struct armada38x_rtc *rtc, u8 rtc_reg)
> >> +{
> >> + unsigned long value_array[SAMPLE_NR], i, j, value;
> >> + unsigned long max = 0, index_max = SAMPLE_NR - 1;
> >> + struct str_value_to_freq value_to_freq[SAMPLE_NR];
> >
> > Hi Gregory
> >
> > This appears to be putting over 900 bytes on the stack. Is there any
>
> Actually the structure being packed it is 500 bytes.
Did you verify this? I never remember where the __packed needs to go.
You clearly have a packed structure, but is the array of structures
packed?
And the long value_array[SAMPLE_NR] is another 400 bytes, totalling
900. And as Russell pointed out, this is on 32 bit systems. Until your
third patch, 64 bit systems probably have double that. I would also
suggest squashing patch #3 into #1.
> > danger of overflowing the stack? Would it be safer to make these
> > arrays part of armada38x_rtc?
>
> We could do this if you fear a stack overflow.
It is generally consider not a good idea to put > $BIG structures on
the stack, but the value of $BIG is not clearly defined. Stack
overflow seems to be an issue with lots of layering going on, swap on
NFS etc. But it seems unlikely to me reading the RTC will happen with
an already deep stack. So this is probably O.K.
Andrew
^ permalink raw reply
* [PATCH] staging: vc04_services: Fix NULL ptr sparse warnings
From: Mike Kofron @ 2016-12-09 16:21 UTC (permalink / raw)
To: linux-arm-kernel
In calls to queue_message() in vchiq_core.c, the "void *context"
parameter is set as 0 rather than NULL. This patch amends each call to
use the proper NULL pointer. The following sparse warnings are fixed:
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c:1623:23: warning: Using plain integer as NULL pointer
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c:1976:47: warning: Using plain integer as NULL pointer
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c:2075:47: warning: Using plain integer as NULL pointer
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c:2095:47: warning: Using plain integer as NULL pointer
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c:2907:39: warning: Using plain integer as NULL pointer
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c:2929:39: warning: Using plain integer as NULL pointer
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c:3059:72: warning: Using plain integer as NULL pointer
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c:3860:31: warning: Using plain integer as NULL pointer
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c:3870:31: warning: Using plain integer as NULL pointer
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c:3880:31: warning: Using plain integer as NULL pointer
Signed-off-by: Mike Kofron <mpkofron@gmail.com>
---
.../vc04_services/interface/vchiq_arm/vchiq_core.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index 028e90b..32b00f8 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -1620,7 +1620,7 @@ parse_open(VCHIQ_STATE_T *state, VCHIQ_HEADER_T *header)
/* No available service, or an invalid request - send a CLOSE */
if (queue_message(state, NULL,
VCHIQ_MAKE_MSG(VCHIQ_MSG_CLOSE, 0, VCHIQ_MSG_SRCPORT(msgid)),
- NULL, 0, 0, 0) == VCHIQ_RETRY)
+ NULL, NULL, 0, 0) == VCHIQ_RETRY)
goto bail_not_ready;
return 1;
@@ -1973,7 +1973,7 @@ parse_rx_slots(VCHIQ_STATE_T *state)
/* Send a PAUSE in response */
if (queue_message(state, NULL,
VCHIQ_MAKE_MSG(VCHIQ_MSG_PAUSE, 0, 0),
- NULL, 0, 0, QMFLAGS_NO_MUTEX_UNLOCK)
+ NULL, NULL, 0, QMFLAGS_NO_MUTEX_UNLOCK)
== VCHIQ_RETRY)
goto bail_not_ready;
if (state->is_master)
@@ -2072,7 +2072,7 @@ slot_handler_func(void *v)
pause_bulks(state);
if (queue_message(state, NULL,
VCHIQ_MAKE_MSG(VCHIQ_MSG_PAUSE, 0, 0),
- NULL, 0, 0,
+ NULL, NULL, 0,
QMFLAGS_NO_MUTEX_UNLOCK)
!= VCHIQ_RETRY) {
vchiq_set_conn_state(state,
@@ -2092,7 +2092,7 @@ slot_handler_func(void *v)
case VCHIQ_CONNSTATE_RESUMING:
if (queue_message(state, NULL,
VCHIQ_MAKE_MSG(VCHIQ_MSG_RESUME, 0, 0),
- NULL, 0, 0, QMFLAGS_NO_MUTEX_LOCK)
+ NULL, NULL, 0, QMFLAGS_NO_MUTEX_LOCK)
!= VCHIQ_RETRY) {
if (state->is_master)
resume_bulks(state);
@@ -2904,7 +2904,7 @@ vchiq_close_service_internal(VCHIQ_SERVICE_T *service, int close_recvd)
(VCHIQ_MSG_CLOSE,
service->localport,
VCHIQ_MSG_DSTPORT(service->remoteport)),
- NULL, 0, 0, 0);
+ NULL, NULL, 0, 0);
}
break;
@@ -2926,7 +2926,7 @@ vchiq_close_service_internal(VCHIQ_SERVICE_T *service, int close_recvd)
(VCHIQ_MSG_CLOSE,
service->localport,
VCHIQ_MSG_DSTPORT(service->remoteport)),
- NULL, 0, 0, QMFLAGS_NO_MUTEX_UNLOCK);
+ NULL, NULL, 0, QMFLAGS_NO_MUTEX_UNLOCK);
if (status == VCHIQ_SUCCESS) {
if (!close_recvd) {
@@ -3056,7 +3056,7 @@ vchiq_connect_internal(VCHIQ_STATE_T *state, VCHIQ_INSTANCE_T instance)
if (state->conn_state == VCHIQ_CONNSTATE_DISCONNECTED) {
if (queue_message(state, NULL,
- VCHIQ_MAKE_MSG(VCHIQ_MSG_CONNECT, 0, 0), NULL, 0,
+ VCHIQ_MAKE_MSG(VCHIQ_MSG_CONNECT, 0, 0), NULL, NULL,
0, QMFLAGS_IS_BLOCKING) == VCHIQ_RETRY)
return VCHIQ_RETRY;
@@ -3857,7 +3857,7 @@ VCHIQ_STATUS_T vchiq_send_remote_use(VCHIQ_STATE_T *state)
if (state->conn_state != VCHIQ_CONNSTATE_DISCONNECTED)
status = queue_message(state, NULL,
VCHIQ_MAKE_MSG(VCHIQ_MSG_REMOTE_USE, 0, 0),
- NULL, 0, 0, 0);
+ NULL, NULL, 0, 0);
return status;
}
@@ -3867,7 +3867,7 @@ VCHIQ_STATUS_T vchiq_send_remote_release(VCHIQ_STATE_T *state)
if (state->conn_state != VCHIQ_CONNSTATE_DISCONNECTED)
status = queue_message(state, NULL,
VCHIQ_MAKE_MSG(VCHIQ_MSG_REMOTE_RELEASE, 0, 0),
- NULL, 0, 0, 0);
+ NULL, NULL, 0, 0);
return status;
}
@@ -3877,7 +3877,7 @@ VCHIQ_STATUS_T vchiq_send_remote_use_active(VCHIQ_STATE_T *state)
if (state->conn_state != VCHIQ_CONNSTATE_DISCONNECTED)
status = queue_message(state, NULL,
VCHIQ_MAKE_MSG(VCHIQ_MSG_REMOTE_USE_ACTIVE, 0, 0),
- NULL, 0, 0, 0);
+ NULL, NULL, 0, 0);
return status;
}
--
2.9.3
^ permalink raw reply related
* [PATCH 1/3] rtc: armada38x: improve RTC errata implementation
From: Gregory CLEMENT @ 2016-12-09 16:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161208172923.GQ26852@lunn.ch>
Hi Andrew,
On jeu., d?c. 08 2016, Andrew Lunn <andrew@lunn.ch> wrote:
>> +struct str_value_to_freq {
>> + unsigned long value;
>> + u8 freq;
>> +} __packed;
>> +
>> +static unsigned long read_rtc_register_wa(struct armada38x_rtc *rtc, u8 rtc_reg)
>> +{
>> + unsigned long value_array[SAMPLE_NR], i, j, value;
>> + unsigned long max = 0, index_max = SAMPLE_NR - 1;
>> + struct str_value_to_freq value_to_freq[SAMPLE_NR];
>
> Hi Gregory
>
> This appears to be putting over 900 bytes on the stack. Is there any
Actually the structure being packed it is 500 bytes.
> danger of overflowing the stack? Would it be safer to make these
> arrays part of armada38x_rtc?
We could do this if you fear a stack overflow.
Gregory
>
> Andrew
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply
* [PATCH 6/6] arm64: KVM: pmu: Reset PMSELR_EL0.SEL to a sane value before entering the guest
From: Marc Zyngier @ 2016-12-09 15:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481298811-30798-1-git-send-email-marc.zyngier@arm.com>
The ARMv8 architecture allows the cycle counter to be configured
by setting PMSELR_EL0.SEL==0x1f and then accessing PMXEVTYPER_EL0,
hence accessing PMCCFILTR_EL0. But it disallows the use of
PMSELR_EL0.SEL==0x1f to access the cycle counter itself through
PMXEVCNTR_EL0.
Linux itself doesn't violate this rule, but we may end up with
PMSELR_EL0.SEL being set to 0x1f when we enter a guest. If that
guest accesses PMXEVCNTR_EL0, the access may UNDEF at EL1,
despite the guest not having done anything wrong.
In order to avoid this unfortunate course of events (haha!), let's
sanitize PMSELR_EL0 on guest entry. This ensures that the guest
won't explode unexpectedly.
Cc: stable at vger.kernel.org #4.6+
Acked-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
arch/arm64/kvm/hyp/switch.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index 83037cd..0c848c1 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -85,7 +85,13 @@ static void __hyp_text __activate_traps(struct kvm_vcpu *vcpu)
write_sysreg(val, hcr_el2);
/* Trap on AArch32 cp15 c15 accesses (EL1 or EL0) */
write_sysreg(1 << 15, hstr_el2);
- /* Make sure we trap PMU access from EL0 to EL2 */
+ /*
+ * Make sure we trap PMU access from EL0 to EL2. Also sanitize
+ * PMSELR_EL0 to make sure it never contains the cycle
+ * counter, which could make a PMXEVCNTR_EL0 access UNDEF at
+ * EL1 instead of being trapped to EL2.
+ */
+ write_sysreg(0, pmselr_el0);
write_sysreg(ARMV8_PMU_USERENR_MASK, pmuserenr_el0);
write_sysreg(vcpu->arch.mdcr_el2, mdcr_el2);
__activate_traps_arch()();
--
2.1.4
^ permalink raw reply related
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