Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* iommu/arm-smmu: Regression (sleeping function called from invalid context)
From: Will Deacon @ 2014-02-03 16:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140131095809.GO13543@alberich>

Hi Andreas,

On Fri, Jan 31, 2014 at 09:58:09AM +0000, Andreas Herrmann wrote:
> On Fri, Jan 31, 2014 at 09:46:23AM +0100, Andreas Herrmann wrote:
> > On Fri, Jan 31, 2014 at 12:55:52AM +0100, Andreas Herrmann wrote:
> > > Hi Will,
> > > 
> > > Seems that commit a44a9791e778d9ccda50d5534028ed4057a9a45b
> > > (iommu/arm-smmu: use mutex instead of spinlock for locking page tables)
> > > introduced a regression.
> > > 
> > > At least I've hit
> > > 
> > >   BUG: scheduling while atomic: ksoftirqd/0/3/0x00000100
> > >...
> > 
> > >   BUG: sleeping function called from invalid context at mm/page_alloc.c:2679
> > >   in_atomic(): 1, irqs_disabled(): 128, pid: 0, name: swapper/0
> > >   CPU: 0 PID: 0 Comm: swapper/0 Not tainted 3.13.0-00016-g6e90346 #413
> > >   [<c0014740>] (unwind_backtrace+0x0/0xf8) from [<c00115b0>] (show_stack+0x10/0x14)
> > >   [<c00115b0>] (show_stack+0x10/0x14) from [<c057ea24>] (dump_stack+0x74/0xa8)
> > >   [<c057ea24>] (dump_stack+0x74/0xa8) from [<c00acc1c>] (__alloc_pages_nodemask+0x174/0x930)
> > >   [<c00acc1c>] (__alloc_pages_nodemask+0x174/0x930) from [<c042a250>] (arm_smmu_handle_mapping+0x470/0x66c)
> > >   [<c042a250>] (arm_smmu_handle_mapping+0x470/0x66c) from [<c0428e74>] (iommu_map+0xf0/0x148)
> > >   [<c0428e74>] (iommu_map+0xf0/0x148) from [<c001935c>] (__map_sg_chunk+0x198/0x2d4)
> > >...
> > 
> > > Maybe that was the reason why the offending commit was introduced(?).

Right, there are two issues here:

  (1) If we use a spinlock to protect our page tables, we can perform a
      blocking allocation whilst holding the lock (during a ->map()
      callback)

  (2) Fixing this to use a mutex means that we can't ->map() in atomic
      context. I hadn't thought that was something we would be doing...

> > > I think with the current code "atomic allocations" should be used when
> > > IO page tables are created. With below patch I've not triggered above
> > > errors.
> > 
> > I think allocating memory with GFP_KERNEL in this dma-mapping path
> > doesn't seem to be a good idea. What if the DMA operation for which we
> > modify IO page tables was triggered to free pages (page cache, swap)?
> 
> I mean in case we run out of memory wouldn't we worsen the situation
> by triggering additional IO (and thus DMA)? Whereas when we let the
> mapping fail, the OS "just might have to wait a little bit" until
> other DMA activities are completed, pages unmapped and iova freed. The
> freed resources instantly can be used for further DMA activities.
> 
> Hmm, but maybe I need to rethink this (and look more closely into
> page_alloc.c).

The problem I see is that we don't want to use atomic allocations for
potentially large allocations, especially where there are cases where we're
not called in atomic context.

How do other IOMMU drivers deal with this? amd_iommu.c uses GFP_KERNEL for
its pte allocation in iommu_ops, but GFP_ATOMIC for its dma_ops.

Will

^ permalink raw reply

* a LLC sched domain bug for panda board?
From: Vincent Guittot @ 2014-02-03 16:28 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAKfTPtDD9g6PPx=cDkbH2VDO0k1HDAjcxwxeW2YLXwL5T4d6NA@mail.gmail.com>

On 3 February 2014 17:27, Vincent Guittot <vincent.guittot@linaro.org> wrote:
> Have you checked that CONFIG_SCHED_LC is set ?

sorry it's CONFIG_SCHED_MC

>
>
> On 3 February 2014 17:17, Alex Shi <alex.shi@linaro.org> wrote:
>> I just run the 3.14-rc1 kernel on panda board. The only domain for it is
>> 'CPU' domain, but this domain has no SD_SHARE_PKG_RESOURCES setting, it
>> has no sd_llc.
>>
>> Guess the right domain for this board should be MC. So is it a bug?
>>
>> ..
>> /proc/sys/kernel/sched_domain/cpu0/domain0/name:CPU
>> ..
>> /proc/sys/kernel/sched_domain/cpu1/domain0/name:CPU
>>
>> --
>> Thanks
>>     Alex
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply

* a LLC sched domain bug for panda board?
From: Vincent Guittot @ 2014-02-03 16:27 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <52EFC11A.8010709@linaro.org>

Have you checked that CONFIG_SCHED_LC is set ?


On 3 February 2014 17:17, Alex Shi <alex.shi@linaro.org> wrote:
> I just run the 3.14-rc1 kernel on panda board. The only domain for it is
> 'CPU' domain, but this domain has no SD_SHARE_PKG_RESOURCES setting, it
> has no sd_llc.
>
> Guess the right domain for this board should be MC. So is it a bug?
>
> ..
> /proc/sys/kernel/sched_domain/cpu0/domain0/name:CPU
> ..
> /proc/sys/kernel/sched_domain/cpu1/domain0/name:CPU
>
> --
> Thanks
>     Alex
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply

* [PATCH v2 2/3] ARM/ARM64: KVM: Add support for PSCI v0.2 emulation
From: Christoffer Dall @ 2014-02-03 16:26 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140203105409.GF30209@e106331-lin.cambridge.arm.com>

On Mon, Feb 03, 2014 at 10:54:09AM +0000, Mark Rutland wrote:
> On Thu, Jan 30, 2014 at 10:41:18AM +0000, Anup Patel wrote:
> > Currently, the in-kernel PSCI emulation provides PSCI v0.1 interface to
> > VCPUs. This patch extends current in-kernel PSCI emulation to provide
> > PSCI v0.2 interface to VCPUs.
> > 
> > By default, ARM/ARM64 KVM will always provide PSCI v0.1 interface for
> > keeping the ABI backward-compatible.
> > 
> > To select PSCI v0.2 interface for VCPUs, the user space (i.e. QEMU or
> > KVMTOOL) will have to set KVM_ARM_VCPU_PSCI_0_2 feature when doing VCPU
> > init using KVM_ARM_VCPU_INIT ioctl.
> 
> I have an issue with this. PSCI 0.2 makes all but two functions (MIGRATE
> and MIGRATE_INFO_CPU_UP) mandatory, and hence not allowed to return
> NOT_SUPPORTED.
> 
> Additionally, for correct behaviour across a kexec in future, we'll
> require AFFINITY_INFO for PSCI 0.2+ systems to determint when a CPU is
> actually dead (and cannot affect the cache hierarchy). I'd very much
> like to make that a hard requirement to ensure correctness.
> 
> I would very much like to see at least trivial implementations of those
> mandatory functions, so that we don't need a
> KVM_ARM_VCPU_PSCI_REALLY_0_2 or similar in future. As it stands this
> series does not implement PSCI 0.2.
> 
I didn't realize that PSCI 0.2 mandates more functions, that should
clearly be implemented first, and the patch series should also be
ordered with first supporting the implementation and then finally
exposing the functionality to user space.

Thanks,
-Christoffer

^ permalink raw reply

* [PATCH v4 2/5] arm: add new asm macro update_sctlr
From: Rob Herring @ 2014-02-03 16:20 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140203160051.GG14112@mudshark.cambridge.arm.com>

On Mon, Feb 3, 2014 at 10:00 AM, Will Deacon <will.deacon@arm.com> wrote:
> On Mon, Feb 03, 2014 at 03:55:42PM +0000, Leif Lindholm wrote:
>> On Mon, Feb 03, 2014 at 10:34:15AM +0000, Will Deacon wrote:
>> > On Thu, Jan 30, 2014 at 01:12:47PM +0000, Leif Lindholm wrote:
>> > > Oh, that's neat - thanks!
>> > >
>> > > Well, given that, I can think of two less horrible options:
>> > > 1)
>> > >   .macro  update_sctlr, tmp:req, set=, clear=
>> > >         mrc       p15, 0, \tmp, c1, c0, 0
>> > >   .ifnc   \set,
>> > >         orr       \tmp, \set
>> > >   .endif
>> > >   .ifnc   \clear,
>> > >   mvn     \clear, \clear
>> > >   and     \tmp, \tmp, \clear
>> >
>> > Can't you use bic here?
>>
>> Yeah.
>>
>> > >   .endif
>> > >         mcr       p15, 0, \tmp, c1, c0, 0
>> > >   .endm
>> > >
>> > > With the two call sites in uefi_phys.S as:
>> > >
>> > >   ldr     r5, =(CR_M)
>> > >   update_sctlr    r12, , r5
>> > > and
>> > >   ldr     r4, =(CR_I | CR_C | CR_M)
>> > >   update_sctlr    r12, r4
>> >
>> > These ldr= could be movs, right?
>>
>> The first one could.
>> The second one could be movw on armv7+.
>>
>> > If so, I definitely prefer this to putting an ldr = into the macro itself
>> > (option 2).
>>
>> And your preference between 1) and 2) is?
>
> (1), using bic and mov[tw] where possible.

Using mov[tw] will break on V6 enabled builds.

Rob

^ permalink raw reply

* [PATCH 5/5] arm64: add Crypto Extensions based synchronous core AES cipher
From: Will Deacon @ 2014-02-03 16:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAKv+Gu_NdVHawdVz3KUeZPHUL4ZPNSpeq7EWAgsVy+DfS6Kaqw@mail.gmail.com>

On Thu, Jan 30, 2014 at 07:20:38PM +0000, Ard Biesheuvel wrote:
> On 30 January 2014 19:56, Will Deacon <will.deacon@arm.com> wrote:
> > On Wed, Jan 29, 2014 at 04:50:46PM +0000, Ard Biesheuvel wrote:
> >> +static void aes_cipher_encrypt(struct crypto_tfm *tfm, u8 dst[], u8 const src[])
> >> +{
> >> +     struct crypto_aes_ctx *ctx = crypto_tfm_ctx(tfm);
> >> +     u32 rounds = 6 + ctx->key_length / 4;
> >
> > Can you document these constants please?
> >
> 
> Sure.

Thanks.

> >> +
> >> +     kernel_neon_begin();
> >> +
> >> +     __asm__("       ld1             {v0.16b}, [%[in]]               ;"
> >> +             "       ld1             {v1.16b}, [%[key]], #16         ;"
> >> +             "0:     aese            v0.16b, v1.16b                  ;"
> >> +             "       subs            %[rounds], %[rounds], #1        ;"
> >> +             "       ld1             {v1.16b}, [%[key]], #16         ;"
> >> +             "       beq             1f                              ;"
> >> +             "       aesmc           v0.16b, v0.16b                  ;"
> >> +             "       b               0b                              ;"
> >> +             "1:     eor             v0.16b, v0.16b, v1.16b          ;"
> >> +             "       st1             {v0.16b}, [%[out]]              ;"
> >> +     : :
> >> +             [out]           "r"(dst),
> >> +             [in]            "r"(src),
> >> +             [rounds]        "r"(rounds),
> >> +             [key]           "r"(ctx->key_enc)
> >> +     :                       "cc");
> >
> > You probably need a memory output to stop this being re-ordered by the
> > compiler. Can GCC not generate the addressing modes you need directly,
> > allowing you to avoid moving everything into registers?
> >
> 
> Would a memory clobber work as well?

It would, but it could lead to suboptimal code generation by GCC (although
neon_{begin,end} may well stop GCC in its tracks anyway, so worth looking at
the disassembly).

> Re addressing modes: I would prefer to explicitly use v0 and v1, I
> have another patch pending that allows partial saves/restores of the
> NEON register file when called from interrupt context. I suppose I
> could use 'register asm("v0")' or something like that, but that won't
> make it any prettier.

It's not the use of v0/v1 that I was objecting to. I was hoping that we
could describe [in] and [out] as memory operands, so that GCC could
potentially reduce register usage for base + offset style addressing modes.
Unfortunately, I don't think we have such a constraint for AArch64 :(

If the disassembly looks ok, the "memory" clobber is probably our best bet.

Will

^ permalink raw reply

* a LLC sched domain bug for panda board?
From: Alex Shi @ 2014-02-03 16:17 UTC (permalink / raw)
  To: linux-arm-kernel

I just run the 3.14-rc1 kernel on panda board. The only domain for it is
'CPU' domain, but this domain has no SD_SHARE_PKG_RESOURCES setting, it
has no sd_llc.

Guess the right domain for this board should be MC. So is it a bug?

..
/proc/sys/kernel/sched_domain/cpu0/domain0/name:CPU
..
/proc/sys/kernel/sched_domain/cpu1/domain0/name:CPU

-- 
Thanks
    Alex

^ permalink raw reply

* [PATCH v5 00/14] ahci: library-ise ahci_platform, add sunxi driver and cleanup imx driver
From: Tejun Heo @ 2014-02-03 16:09 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1390417489-5354-1-git-send-email-hdegoede@redhat.com>

Hello,

On Wed, Jan 22, 2014 at 08:04:35PM +0100, Hans de Goede wrote:
> Here is v4 of my patchset for adding ahci-sunxi support. It has grown quite
> a bit since I've been going for a more generic approach this time and I've
> also cleaned up the ahci-imx driver to use the same generic approach.

On top of which tree were these patches generated?  They don't apply
to 3.14-rc1.

Thanks.

-- 
tejun

^ permalink raw reply

* [PATCH v2 1/6] audit: Enable arm64 support
From: Richard Guy Briggs @ 2014-02-03 16:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <52EF304F.80202@linaro.org>

On 14/02/03, AKASHI Takahiro wrote:
> Richard,

Takahiro,

> On 01/30/2014 07:36 AM, Richard Guy Briggs wrote:
> >On 14/01/29, Richard Guy Briggs wrote:
> >>On 14/01/27, AKASHI Takahiro wrote:
> >>>[To audit maintainers]
> >>>
> >>>On 01/23/2014 11:18 PM, Catalin Marinas wrote:
> >>>>On Fri, Jan 17, 2014 at 08:13:14AM +0000, AKASHI Takahiro wrote:
> >>>>>--- a/include/uapi/linux/audit.h
> >>>>>+++ b/include/uapi/linux/audit.h
> >>>>>@@ -327,6 +327,8 @@ enum {
> >>>>>  /* distinguish syscall tables */
> >>>>>  #define __AUDIT_ARCH_64BIT 0x80000000
> >>>>>  #define __AUDIT_ARCH_LE	   0x40000000
> >>>>>+#define AUDIT_ARCH_AARCH64	(EM_AARCH64|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
> >>>>>+#define AUDIT_ARCH_AARCH64EB	(EM_AARCH64|__AUDIT_ARCH_64BIT)
> >>>>>  #define AUDIT_ARCH_ALPHA	(EM_ALPHA|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
> >>>>>  #define AUDIT_ARCH_ARM		(EM_ARM|__AUDIT_ARCH_LE)
> >>>>>  #define AUDIT_ARCH_ARMEB	(EM_ARM)
> >>>>>diff --git a/init/Kconfig b/init/Kconfig
> >>>>>index 79383d3..3aae602 100644
> >>>>>--- a/init/Kconfig
> >>>>>+++ b/init/Kconfig
> >>>>>@@ -284,7 +284,7 @@ config AUDIT
> >>>>>
> >>>>>  config AUDITSYSCALL
> >>>>>  	bool "Enable system-call auditing support"
> >>>>>-	depends on AUDIT && (X86 || PARISC || PPC || S390 || IA64 || UML || SPARC64 || SUPERH || (ARM && AEABI && !OABI_COMPAT))
> >>>>>+	depends on AUDIT && (X86 || PARISC || PPC || S390 || IA64 || UML || SPARC64 || SUPERH || (ARM && AEABI && !OABI_COMPAT) || ARM64)
> >>>>
> >>>>The usual comment for such changes: could you please clean this up and
> >>>>just use something like "depends on HAVE_ARCH_AUDITSYSCALL"?
> >>>
> >>>Do you agree to this change?
> >>>
> >>>If so, I can create a patch, but have some concerns:
> >>>1) I can't verify it on other architectures than (arm &) arm64.
> >>>2) Some architectures (microblaze, mips, openrisc) are not listed here, but
> >>>    their ptrace.c have a call to audit_syscall_entry/exit().
> >>>    (audit_syscall_entry/exit are null if !AUDITSYSCALL, though)
> >>
> >>I can try: ppc s390 x86_64 ppc64 i686 s390x
> >
> >These arches above all pass compile and basic tests with the following patches applied:
> >
> >	audit: correct a type mismatch in audit_syscall_exit() pending (already upstream)
> >
> >	audit: Modify a set of system calls in audit class definitions (already upstream)
> >
> >	[PATCH v3] audit: Add generic compat syscall support
> >
> >	[PATCH v2] audit: Enable arm64 support
> >	[PATCH v2] arm64: Add regs_return_value() in syscall.h
> >	[PATCH v2] arm64: Add audit support
> >	[PATCH v2] arm64: audit: Add 32-bit (compat) syscall support
> >	[PATCH v2] arm64: audit: Add makefile rule to create unistd_32.h for compat syscalls
> >	[PATCH v2] arm64: audit: Add audit hook in ptrace/syscall_trace
> 
> I think that you missed Catalin's suggestion.

I didn't miss his suggestions.  I think they are a good way to go, but I
wanted to make a test at referrable point in time to validate the work
to that point and to avoid introducing errors by mis-interpreting ideas
that were not yet fully-formed patches.

> Please use the patch I will post after this message and try it again, please?

I was certainly intending to do so.

> Thanks,
> -Takahiro AKASHI
> 
> >>>So I'm afraid that the change might break someone's assumption.
> >>>
> >>>Thanks,
> >>>-Takahiro AKASHI
> >>
> >>- RGB
> >
> >- RGB
> >
> >--
> >Richard Guy Briggs <rbriggs@redhat.com>
> >Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
> >Remote, Ottawa, Canada
> >Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545
> >

- RGB

--
Richard Guy Briggs <rbriggs@redhat.com>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545

^ permalink raw reply

* [PATCH v2 1/7] cpufreq: cpufreq-cpu0: allow optional safe voltage during frequency transitions
From: Thomas Abraham @ 2014-02-03 16:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140201041051.9977.46568@quantum>

On Sat, Feb 1, 2014 at 9:40 AM, Mike Turquette <mturquette@linaro.org> wrote:
> Quoting Heiko St?bner (2014-01-30 07:09:04)
>> On Thursday, 30. January 2014 18:23:44 Thomas Abraham wrote:
>> > Hi Mike,
>> >
>> > On Wed, Jan 29, 2014 at 12:17 AM, Mike Turquette <mturquette@linaro.org>
>> wrote:
>> > > On Mon, Jan 27, 2014 at 9:30 PM, Thomas Abraham <ta.omasab@gmail.com>
>> wrote:
>> > >> Hi Mike,
>> > >>
>> > >> On Tue, Jan 28, 2014 at 1:55 AM, Mike Turquette <mturquette@linaro.org>
>> wrote:
>> > >>> Quoting Thomas Abraham (2014-01-18 04:10:51)
>> > >>>
>> > > As far as I can tell
>> > > the remux does not happen because it is necessary to generate the
>> > > required clock rate, but because we don't want to run the ARM core out
>> > > of spec for a short time while the PLL relocks. Assuming I have that
>> > > part of it right, I prefer for the parent mux operation to be a part
>> > > of the CPUfreq driver's .target callback instead of hidden away in the
>> > > clock driver.
>> >
>> > The re-parenting is mostly done to keep the ARM CPU clocked while the
>> > PLL is stopped, reprogrammed and restarted. One of the side effects of
>> > that is, the clock speed of the temporary parent could be higher then
>> > what is allowed due to the ARM voltage at the time of re-parenting.
>> > That is the reason to use the safe voltage.
>>
>> The Rockchip-SoCs use something similar, so I'm following quite closely what
>> Thomas is trying to do here, as similar solution would also solve this issue
>> for me.
>>
>> On some Rockchip-SoCs even stuff like pclk and hclk seems to be sourced from
>> the divided armclk, creating additional constraints.
>>
>> But on the RKs (at least in the upstream sources) the armclk is simply equal
>> to the pll output. A divider exists, but is only used to make sure that the
>> armclk stays below the original rate when sourced from the temp-parent, like
>>
>>         if (clk_get_rate(temp_parent) > clk_get_rate(main_parent)
>>                 set_divider(something so that rate(temp) <= rate(main)
>>         clk_set_parent(...)
>>
>> Isn't there a similar possiblity on your platform, as it would remove the need
>> for the safe-voltage?
>>
>>
>> In general I also like the approach of hiding the rate-change logic inside
>> this composite clock, as the depending clocks can be easily kept in sync. As
>> with the Rockchips the depending clocks are different for each of the three
>> Cortex-A9 SoCs I looked at, it would be "interesting" if all of this would
>> need to be done in a cpufreq driver.
>
> I wonder if hiding these details inside of the composite clock
> implementation indicates the lack of some needed feature in the clk
> core? I've discussed the idea of "coordinated rate changes" before. E.g:
>
> _________________________________________________________
> |  clk  |  opp-low      |  opp-mid      |  opp-fast     |
> |       |               |               |               |
> |pll    | 300000        |  600000       |  600000       |
> |       |               |               |               |
> |div    | 150000        |  300000       |  600000       |
> |       |               |               |               |
> |mpu_clk| 150000        |  300000       |  600000       |
> |       |               |               |               |
> |periph | 150000        |  150000       |  300000       |
> ---------------------------------------------------------
>
> A call to clk_set_rate() against any of those clocks will result in all
> of their dividers being updated. At the implementation level this might
> look something like this extremely simplified pseudocode:
>
> int clk_set_rate(struct clk* clk, unsigned long rate)
> {
>         /* trap clks that support coordinated rate changes */
>         if (clk->ops->coordinate_rate)
>                 return clk->ops->coordinate_rate(clk->hw, rate);
>         ...
> }
>
> and,
>
> struct coord_rates {
>         struct clk_hw *hw;
>         struct clk *parent;
>         struct clk *rate;
> };
>
> and in the clock driver,
>
> #define PLL 0
> #define DIV 1
> #define MPU 2
> #define PER 3
>
> #define NR_OPP 4
> #define NR_CLK 4
>
> struct coord_rates my_opps[NR_OPP][NR_CLK]; // populated from DT data
>
> int my_coordinate_rate_callback(struct clk_hw *hw, unsigned long rate)
> {
>         struct coord_rate **selected_opp;
>
>         for(i = 0; i < NR_OPP; i++) {
>                 for(j = 0; j < NR_CLK; j++) {
>                         if (my_opps[i][j]->hw == hw &&
>                                 my_opps[i][j]->rate == rate)
>                                 selected_opp = my_opps[i];
>                                 break;
>                 }
>         }
>
>         /*
>          * order of operations is specific to my hardware and should be
>          * managed by my clock driver, not generic code
>          */
>
>         __clk_set_parent(selected_opp[DIV]->hw, temp_parent);
>         __clk_set_rate(selected_opp[PLL]->hw, selected_opp[PLL]->rate);
>         __clk_set_parent(selected_opp[DIV]->hw,
>                                 selected_opp[PLL]->hw->clk);
>         ...
>
>         /*
>          * note that the above could be handled by a switch-case or
>          * something else
>          */
> }
>
> Thoughts? Please forgive any gaps in my logic or abuse of C.
>
> I have long thought that something like the above would someday go into
> a generic dvfs layer instead of the clock framework, but maybe starting
> with the clk framework makes more sense?

Hi Mike,

Yes, this will be very helpful for atomically controlling the rates of
a group of clocks. This coordinated rate change method can be used
during the armclk rate changes on Samsung platforms.

Thanks,
Thomas.

>
> Regards,
> Mike
>
>>
>>
>> Heiko
>>

^ permalink raw reply

* [PATCH v2 1/7] cpufreq: cpufreq-cpu0: allow optional safe voltage during frequency transitions
From: Thomas Abraham @ 2014-02-03 16:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <2009408.gnUEcoqA3p@phil>

On Thu, Jan 30, 2014 at 8:39 PM, Heiko St?bner <heiko@sntech.de> wrote:
> On Thursday, 30. January 2014 18:23:44 Thomas Abraham wrote:
>> Hi Mike,
>>
>> On Wed, Jan 29, 2014 at 12:17 AM, Mike Turquette <mturquette@linaro.org>
> wrote:
>> > On Mon, Jan 27, 2014 at 9:30 PM, Thomas Abraham <ta.omasab@gmail.com>
> wrote:
>> >> Hi Mike,
>> >>
>> >> On Tue, Jan 28, 2014 at 1:55 AM, Mike Turquette <mturquette@linaro.org>
> wrote:
>> >>> Quoting Thomas Abraham (2014-01-18 04:10:51)
>> >>>
>> > As far as I can tell
>> > the remux does not happen because it is necessary to generate the
>> > required clock rate, but because we don't want to run the ARM core out
>> > of spec for a short time while the PLL relocks. Assuming I have that
>> > part of it right, I prefer for the parent mux operation to be a part
>> > of the CPUfreq driver's .target callback instead of hidden away in the
>> > clock driver.
>>
>> The re-parenting is mostly done to keep the ARM CPU clocked while the
>> PLL is stopped, reprogrammed and restarted. One of the side effects of
>> that is, the clock speed of the temporary parent could be higher then
>> what is allowed due to the ARM voltage at the time of re-parenting.
>> That is the reason to use the safe voltage.
>
> The Rockchip-SoCs use something similar, so I'm following quite closely what
> Thomas is trying to do here, as similar solution would also solve this issue
> for me.
>
> On some Rockchip-SoCs even stuff like pclk and hclk seems to be sourced from
> the divided armclk, creating additional constraints.
>
> But on the RKs (at least in the upstream sources) the armclk is simply equal
> to the pll output. A divider exists, but is only used to make sure that the
> armclk stays below the original rate when sourced from the temp-parent, like
>
>         if (clk_get_rate(temp_parent) > clk_get_rate(main_parent)
>                 set_divider(something so that rate(temp) <= rate(main)
>         clk_set_parent(...)
>
> Isn't there a similar possiblity on your platform, as it would remove the need
> for the safe-voltage?

Hi Heiko,

Yes, this works too! I have tested this method on Exynos4210,
Exynos4412 and Exynos5250 and it works fine without any need for safe
voltage. This is much better than using safe voltage. Thank you for
suggesting this.

Regards,
Thomas.

>
>
> In general I also like the approach of hiding the rate-change logic inside
> this composite clock, as the depending clocks can be easily kept in sync. As
> with the Rockchips the depending clocks are different for each of the three
> Cortex-A9 SoCs I looked at, it would be "interesting" if all of this would
> need to be done in a cpufreq driver.
>
>
> Heiko
>

^ permalink raw reply

* [PATCH v4 2/5] arm: add new asm macro update_sctlr
From: Will Deacon @ 2014-02-03 16:00 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140203155542.GI11329@bivouac.eciton.net>

On Mon, Feb 03, 2014 at 03:55:42PM +0000, Leif Lindholm wrote:
> On Mon, Feb 03, 2014 at 10:34:15AM +0000, Will Deacon wrote:
> > On Thu, Jan 30, 2014 at 01:12:47PM +0000, Leif Lindholm wrote:
> > > Oh, that's neat - thanks!
> > > 
> > > Well, given that, I can think of two less horrible options:
> > > 1)
> > > 	.macro  update_sctlr, tmp:req, set=, clear=
> > >         mrc	p15, 0, \tmp, c1, c0, 0
> > > 	.ifnc	\set,
> > >         orr	\tmp, \set
> > > 	.endif
> > > 	.ifnc	\clear,
> > > 	mvn	\clear, \clear
> > > 	and	\tmp, \tmp, \clear
> > 
> > Can't you use bic here?
> 
> Yeah.
> 
> > > 	.endif
> > >         mcr	p15, 0, \tmp, c1, c0, 0
> > > 	.endm
> > > 
> > > With the two call sites in uefi_phys.S as:
> > > 
> > > 	ldr	r5, =(CR_M)
> > > 	update_sctlr	r12, , r5
> > > and
> > > 	ldr	r4, =(CR_I | CR_C | CR_M)
> > > 	update_sctlr	r12, r4
> > 
> > These ldr= could be movs, right?
> 
> The first one could.
> The second one could be movw on armv7+.
> 
> > If so, I definitely prefer this to putting an ldr = into the macro itself
> > (option 2).
> 
> And your preference between 1) and 2) is?

(1), using bic and mov[tw] where possible.

Will

^ permalink raw reply

* about last level cache on big little cpu
From: Sudeep Holla @ 2014-02-03 15:56 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <52EFACDC.7090103@linaro.org>

On 03/02/14 14:51, Alex Shi wrote:
> Does all the big.LITTLE cpu share the same llc? or it depends on
> different vendor's products?
> 

It depends on implementation. e.g. on Cortex-A7 L2 is optional.
So there can be big.LITTLE system with CA15(with L2) and CA7(without L2).
In general depends are the choice of the processors in bL and if L2
is mandatory or optional on those processors.

Regards,
Sudeep

^ permalink raw reply

* [PATCH v4 2/5] arm: add new asm macro update_sctlr
From: Leif Lindholm @ 2014-02-03 15:55 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140203103415.GA12187@mudshark.cambridge.arm.com>

On Mon, Feb 03, 2014 at 10:34:15AM +0000, Will Deacon wrote:
> On Thu, Jan 30, 2014 at 01:12:47PM +0000, Leif Lindholm wrote:
> > Oh, that's neat - thanks!
> > 
> > Well, given that, I can think of two less horrible options:
> > 1)
> > 	.macro  update_sctlr, tmp:req, set=, clear=
> >         mrc	p15, 0, \tmp, c1, c0, 0
> > 	.ifnc	\set,
> >         orr	\tmp, \set
> > 	.endif
> > 	.ifnc	\clear,
> > 	mvn	\clear, \clear
> > 	and	\tmp, \tmp, \clear
> 
> Can't you use bic here?

Yeah.

> > 	.endif
> >         mcr	p15, 0, \tmp, c1, c0, 0
> > 	.endm
> > 
> > With the two call sites in uefi_phys.S as:
> > 
> > 	ldr	r5, =(CR_M)
> > 	update_sctlr	r12, , r5
> > and
> > 	ldr	r4, =(CR_I | CR_C | CR_M)
> > 	update_sctlr	r12, r4
> 
> These ldr= could be movs, right?

The first one could.
The second one could be movw on armv7+.

> If so, I definitely prefer this to putting an ldr = into the macro itself
> (option 2).

And your preference between 1) and 2) is?

/
    Leif

^ permalink raw reply

* [PATCH 09/10] watchdog: xilinx: Add missing binding
From: Michal Simek @ 2014-02-03 15:47 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <3121595.GbLMuGPlvA@wuerfel>

On 02/03/2014 04:32 PM, Arnd Bergmann wrote:
> On Monday 03 February 2014 16:13:47 Michal Simek wrote:
>> Intention wasn't to fix binding but document current one
>> which is in mainline for a long time.
> 
> Ok, I see.
> 
>> Apart of this - yes, wdt-enable-once is nowayout and wdt-interval should be timeout
>> is seconds, and clock-frequency should go out and use CCF for getting clock.
> 
> Could we make a common binding then, and document that the xilinx
> watchdog can optionally provide either one?

Do you mean to have 2 DT bindings?

This binding is used from 2011-07.
It means it was generated for all hw designs at least from this time.
I would say from DT usage on Microblaze because it is not special case
in our dt generator.

xlnx,XXX are XXX parameters which you have to setup in tools
and get synthesized. This is valid for all xilinx IPs. We have full
IP description by generating xlnx,XXX parameters directly from tools
because we know all variants which can happen.

Just back to your previous post:
"I'm not sure about the enable-once flag, which seems to just map to the
"nowayout" watchdog option that is not a hardware feature at all"
this is hw feature which you can select in tools because this is fpga. :-)

Thanks,
Michal

^ permalink raw reply

* [PATCH v5 16/16] ARM: Remove uprobes dependency on kprobes
From: Jon Medhurst (Tixy) @ 2014-02-03 15:45 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1390507559-4697-17-git-send-email-dave.long@linaro.org>

On Thu, 2014-01-23 at 15:05 -0500, David Long wrote:
> From: "David A. Long" <dave.long@linaro.org>
> 
> Now that arm uprobes support has been made separate from the arm kprobes code
> the Kconfig can be changed to reflect that.
> 
> Signed-off-by: David A. Long <dave.long@linaro.org>
> ---
>  arch/arm/Kconfig | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index fec5a6b..9ddc4ae 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -204,7 +204,6 @@ config NEED_DMA_MAP_STATE
>         def_bool y
>  
>  config ARCH_SUPPORTS_UPROBES
> -	depends on KPROBES
>  	def_bool y
>  
>  config ARCH_HAS_DMA_SET_COHERENT_MASK


Was this patch meant to have other contents? If not, it seems a bit
pointless as all it does is remove a line added in the previous patch,
so should just be folded into that one.

-- 
Tixy

^ permalink raw reply

* NFS client broken in Linus' tip
From: Trond Myklebust @ 2014-02-03 15:45 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140203145759.GA30263@infradead.org>


On Feb 3, 2014, at 9:57, Christoph Hellwig <hch@infradead.org> wrote:

> On Mon, Feb 03, 2014 at 09:17:30AM -0500, Trond Myklebust wrote:
>> As I said above, that causes posix_acl_xattr_get() to return the wrong answer (ENODATA instead of EOPNOTSUPP).
> 
> Is it really the wrong answer?  How does userspace care wether this
> server doesn't support ACLs at all or none is set?  The resulting
> behavior is the same.

It will certainly cause acl_get_file() to behave differently than previously. I?ve no idea how that will affect applications, though.

> If there's a good reason to care we might have to go with your patch,
> but if we can avoid it I'd prefer to keep things simple.

One alternative is to simply wrap posix_acl_xattr_get() in fs/nfs/nfs3acl.c, and have it check the value of nfs_server_capable(inode, NFS_CAP_ACLS) before returning ENODATA. That?s rather ugly too...

--
Trond Myklebust
Linux NFS client maintainer

^ permalink raw reply

* [PATCH] asm-generic: add sched_setattr/sched_getattr syscalls
From: James Hogan @ 2014-02-03 15:33 UTC (permalink / raw)
  To: linux-arm-kernel

Add the sched_setattr and sched_getattr syscalls to the generic syscall
list, which is used by the following architectures: arc, arm64, c6x,
hexagon, metag, openrisc, score, tile, unicore32.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: linux-arch at vger.kernel.org
Cc: Vineet Gupta <vgupta@synopsys.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: linux-arm-kernel at lists.infradead.org
Cc: Mark Salter <msalter@redhat.com>
Cc: Aurelien Jacquiot <a-jacquiot@ti.com>
Cc: linux-c6x-dev at linux-c6x.org
Cc: Richard Kuo <rkuo@codeaurora.org>
Cc: linux-hexagon at vger.kernel.org
Cc: linux-metag at vger.kernel.org
Cc: Jonas Bonn <jonas@southpole.se>
Cc: linux at lists.openrisc.net
Cc: Chen Liqin <liqin.linux@gmail.com>
Cc: Lennox Wu <lennox.wu@gmail.com>
Cc: Chris Metcalf <cmetcalf@tilera.com>
Cc: Guan Xuetao <gxt@mprc.pku.edu.cn>
---
 include/uapi/asm-generic/unistd.h | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
index a20a9b4d3871..dde8041f40d2 100644
--- a/include/uapi/asm-generic/unistd.h
+++ b/include/uapi/asm-generic/unistd.h
@@ -692,9 +692,13 @@ __SC_COMP(__NR_process_vm_writev, sys_process_vm_writev, \
 __SYSCALL(__NR_kcmp, sys_kcmp)
 #define __NR_finit_module 273
 __SYSCALL(__NR_finit_module, sys_finit_module)
+#define __NR_sched_setattr 274
+__SYSCALL(__NR_sched_setattr, sys_sched_setattr)
+#define __NR_sched_getattr 275
+__SYSCALL(__NR_sched_getattr, sys_sched_getattr)
 
 #undef __NR_syscalls
-#define __NR_syscalls 274
+#define __NR_syscalls 276
 
 /*
  * All syscalls below here should go away really,
-- 
1.8.1.2

^ permalink raw reply related

* [PATCH 09/10] watchdog: xilinx: Add missing binding
From: Arnd Bergmann @ 2014-02-03 15:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <24ecacca-2714-4834-921c-b827c0e592a3@VA3EHSMHS003.ehs.local>

On Monday 03 February 2014 16:13:47 Michal Simek wrote:
> Intention wasn't to fix binding but document current one
> which is in mainline for a long time.

Ok, I see.

> Apart of this - yes, wdt-enable-once is nowayout and wdt-interval should be timeout
> is seconds, and clock-frequency should go out and use CCF for getting clock.

Could we make a common binding then, and document that the xilinx
watchdog can optionally provide either one?

	Arnd

^ permalink raw reply

* [PATCH 09/10] watchdog: xilinx: Add missing binding
From: Michal Simek @ 2014-02-03 15:13 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <201402031606.11753.arnd@arndb.de>

On 02/03/2014 04:06 PM, Arnd Bergmann wrote:
> On Friday 31 January 2014, Michal Simek wrote:
>> +Optional properties:
>> +- clock-frequency      : Frequency of clock in Hz
>> +- xlnx,wdt-enable-once : 0 - Watchdog can be restarted
>> +                         1 - Watchdog can be enabled just once
>> +- xlnx,wdt-interval    : Watchdog timeout interval in 2^<val> clock cycles,
>> +                         <val> is integer from 8 to 31.
>> +
> 
> The latter two don't really seem to be xilinx specific, it would be
> reasonable to have a standard watchdog binding that mandates a common
> format for them.
> 
> I'm not sure about the enable-once flag, which seems to just map to the
> "nowayout" watchdog option that is not a hardware feature at all
> and should probably be kept as a software setting only, rather than
> settable through DT. If it is kept, it should have a standard name and
> get turned into a boolean (present/absent) property rather than a
> 0/1 integer property.
> 
> The interval should really be specified in terms of seconds or miliseconds,
> not in clock cycles.

Intention wasn't to fix binding but document current one
which is in mainline for a long time.

Apart of this - yes, wdt-enable-once is nowayout and wdt-interval should be timeout
is seconds, and clock-frequency should go out and use CCF for getting clock.

Thanks,
Michal

^ permalink raw reply

* [PATCH 09/10] watchdog: xilinx: Add missing binding
From: Arnd Bergmann @ 2014-02-03 15:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <9c721b89ed2ceb6997809bb3363f852277e67dc2.1391177880.git.michal.simek@xilinx.com>

On Friday 31 January 2014, Michal Simek wrote:
> +Optional properties:
> +- clock-frequency      : Frequency of clock in Hz
> +- xlnx,wdt-enable-once : 0 - Watchdog can be restarted
> +                         1 - Watchdog can be enabled just once
> +- xlnx,wdt-interval    : Watchdog timeout interval in 2^<val> clock cycles,
> +                         <val> is integer from 8 to 31.
> +

The latter two don't really seem to be xilinx specific, it would be
reasonable to have a standard watchdog binding that mandates a common
format for them.

I'm not sure about the enable-once flag, which seems to just map to the
"nowayout" watchdog option that is not a hardware feature at all
and should probably be kept as a software setting only, rather than
settable through DT. If it is kept, it should have a standard name and
get turned into a boolean (present/absent) property rather than a
0/1 integer property.

The interval should really be specified in terms of seconds or miliseconds,
not in clock cycles.

	Arnd

^ permalink raw reply

* about last level cache on big little cpu
From: Morten Rasmussen @ 2014-02-03 15:03 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <52EFACDC.7090103@linaro.org>

On Mon, Feb 03, 2014 at 02:51:08PM +0000, Alex Shi wrote:
> Does all the big.LITTLE cpu share the same llc? or it depends on
> different vendor's products?

No. I don't know of any big.LITTLE implementation that does have a
shared last level cache. TC2 has per cluster L2 caches as last level
caches, so no cache is shared between the big and little clusters.

Morten

^ permalink raw reply

* NFS client broken in Linus' tip
From: Christoph Hellwig @ 2014-02-03 14:57 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <85AAFCF5-60EE-42E5-B103-37A4613C5947@primarydata.com>

On Mon, Feb 03, 2014 at 09:17:30AM -0500, Trond Myklebust wrote:
> As I said above, that causes posix_acl_xattr_get() to return the wrong answer (ENODATA instead of EOPNOTSUPP).

Is it really the wrong answer?  How does userspace care wether this
server doesn't support ACLs at all or none is set?  The resulting
behavior is the same.

If there's a good reason to care we might have to go with your patch,
but if we can avoid it I'd prefer to keep things simple.

^ permalink raw reply

* [PATCH v5 07/16] ARM: Remove use of struct kprobe from generic probes code
From: Jon Medhurst (Tixy) @ 2014-02-03 14:57 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1390507559-4697-8-git-send-email-dave.long@linaro.org>

On Thu, 2014-01-23 at 15:05 -0500, David Long wrote:
> From: "David A. Long" <dave.long@linaro.org>
> 
> Change the generic ARM probes code to pass in the opcode and architecture-specific
> structure separately instead of using struct kprobe, so we do not pollute
> code being used only for uprobes or other non-kprobes instruction
> interpretation.
> 
> Signed-off-by: David A. Long <dave.long@linaro.org>
> ---

One minor nit-pick...

[...]
> diff --git a/arch/arm/kernel/kprobes-thumb.c b/arch/arm/kernel/kprobes-thumb.c
> index c7ee290..cea707a 100644
> --- a/arch/arm/kernel/kprobes-thumb.c
> +++ b/arch/arm/kernel/kprobes-thumb.c
[...]
> @@ -593,7 +590,7 @@ t16_emulate_pop_pc(struct kprobe *p, struct pt_regs *regs)
>  	bx_write_pc(pc, regs);
>  }
>  
> -static enum kprobe_insn __kprobes
> +enum kprobe_insn __kprobes
>  t16_decode_pop(kprobe_opcode_t insn, struct arch_specific_insn *asi,
>  		struct decode_header *d)
>  {

The above removal of 'static' appears to be an unneeded accidental
change?

-- 
Tixy

^ permalink raw reply

* [PATCH v5 08/14] ahci-platform: "Library-ise" suspend / resume functionality
From: Arnd Bergmann @ 2014-02-03 14:53 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1390417489-5354-9-git-send-email-hdegoede@redhat.com>

On Wednesday 22 January 2014, Hans de Goede wrote:
> --- a/include/linux/ahci_platform.h
> +++ b/include/linux/ahci_platform.h
> @@ -50,4 +50,11 @@ int ahci_platform_init_host(struct platform_device *pdev,
>                             unsigned int force_port_map,
>                             unsigned int mask_port_map);
>  
> +#ifdef CONFIG_PM_SLEEP
> +int ahci_platform_suspend_host(struct device *dev);
> +int ahci_platform_resume_host(struct device *dev);
> +int ahci_platform_suspend(struct device *dev);
> +int ahci_platform_resume(struct device *dev);
> +#endif
> +

Not sure if the #ifdef does any good here. Normally, we don't hide declarations
so we can do stuff like

	if (IS_ENABLED(CONFIG_PM_SLEEP))
		ret = ahci_platform_suspend_host(dev);

and expect the code to compile and link just fine.

	Arnd

^ 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