* [RFC PATCH 1/6] drm: Add Content Protection property
From: Pavel Machek @ 2017-12-05 18:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CADnq5_PcbBeGL+3CxOpNgRiih4czU9SnmjNbRyyRHOn-OZvy5Q@mail.gmail.com>
Hi!
> >> > Why would user of the machine want this to be something else than
> >> > 'OFF'?
> >> >
> >> > If kernel implements this, will it mean hardware vendors will have to
> >> > prevent user from updating kernel on machines they own?
> >> >
> >> > If this is merged, does it open kernel developers to DMCA threats if
> >> > they try to change it?
> >>
> >> Because this just implements one part of the content protection scheme.
> >> This only gives you an option to enable HDCP (aka encryption, it's really
> >> nothing else) on the cable. Just because it has Content Protection in the
> >> name does _not_ mean it is (stand-alone) an effective nor complete content
> >> protection scheme. It's simply encrypting data, that's all.
> >
> > Yep. So my first question was: why would user of the machine ever want
> > encryption "ENABLED" or "DESIRED"? Could you answer it?
>
> How about for sensitive video streams in government offices where you
> want to avoid a spy potentially tapping the cable to see the video
> stream?
Except that spies already have the keys, as every monitor
manufacturer has them?
> >> kernels and be able to exercise their software freedoms already know to
> >> avoid such locked down systems.
> >>
> >> So yeah it would be better to call this the "HDMI/DP cable encryption
> >> support", but well, it's not what it's called really.
> >
> > Well, it does not belong in kernel, no matter what is the name.
>
> Should we remove support for encrypted file systems and encrypted
> virtual machines? Just like them the option is there is you want to
> use it. If you don't want to, you don't have to.
Encrypted file systems benefit users. Encrypted video is designed to
work against users. In particular, users don't have encryption keys
for video they generate. I'd have nothing against feature that would
let users encrypt video with keys they control.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20171205/179f88b1/attachment.sig>
^ permalink raw reply
* [PATCH v2 11/19] arm64: assembler: add macro to conditionally yield the NEON under PREEMPT
From: Ard Biesheuvel @ 2017-12-05 18:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <DD92A22D-E5EF-4C97-8C76-4797892518A4@linaro.org>
On 5 December 2017 at 12:45, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
>
>> On 5 Dec 2017, at 12:28, Dave Martin <Dave.Martin@arm.com> wrote:
>>
>>> On Mon, Dec 04, 2017 at 12:26:37PM +0000, Ard Biesheuvel wrote:
>>> Add a support macro to conditionally yield the NEON (and thus the CPU)
>>> that may be called from the assembler code. Given that especially the
>>> instruction based accelerated crypto code may use very tight loops, add
>>> some parametrization so that the TIF_NEED_RESCHED flag test is only
>>> executed every so many loop iterations.
>>>
>>> In some cases, yielding the NEON involves saving and restoring a non
>>> trivial amount of context (especially in the CRC folding algorithms),
>>> and so the macro is split into two, and the code in between is only
>>> executed when the yield path is taken, allowing the contex to be preserved.
>>> The second macro takes a label argument that marks the resume-from-yield
>>> path, which should restore the preserved context again.
>>>
>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> ---
>>> arch/arm64/include/asm/assembler.h | 50 ++++++++++++++++++++
>>> 1 file changed, 50 insertions(+)
>>>
>>> diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
>>> index aef72d886677..917b026d3e00 100644
>>> --- a/arch/arm64/include/asm/assembler.h
>>> +++ b/arch/arm64/include/asm/assembler.h
>>> @@ -512,4 +512,54 @@ alternative_else_nop_endif
>>> #endif
>>> .endm
>>>
>>> +/*
>>> + * yield_neon - check whether to yield to another runnable task from
>>> + * kernel mode NEON code (running with preemption disabled)
>>> + *
>>> + * - Check whether the preempt count is exactly 1, in which case disabling
>>> + * preemption once will make the task preemptible. If this is not the case,
>>> + * yielding is pointless.
>>> + * - Check whether TIF_NEED_RESCHED is set, and if so, disable and re-enable
>>> + * kernel mode NEON (which will trigger a reschedule), and branch to the
>>> + * yield fixup code at @lbl.
>>> + */
>>> + .macro yield_neon, lbl:req, ctr, order, stride, loop
>>> + yield_neon_pre \ctr, \order, \stride, \loop
>>> + yield_neon_post \lbl
>>> + .endm
>>> +
>>> + .macro yield_neon_pre, ctr, order=0, stride, loop=4444f
>>> +#ifdef CONFIG_PREEMPT
>>> + /*
>>> + * With some algorithms, it makes little sense to poll the
>>> + * TIF_NEED_RESCHED flag after every iteration, so only perform
>>> + * the check every 2^order strides.
>>> + */
>>> + .if \order > 1
>>> + .if (\stride & (\stride - 1)) != 0
>>> + .error "stride should be a power of 2"
>>> + .endif
>>> + tst \ctr, #((1 << \order) * \stride - 1) & ~(\stride - 1)
>>> + b.ne \loop
>>> + .endif
>>
>> I'm not sure what baking in this check gives us, and this seems to
>> conflate two rather different things: yielding and defining a
>> "heartbeat" frequency for the calling code.
>>
>> Can we separate out the crypto-loop-helper semantics from the yield-
>> neon stuff?
>>
>
> Fair enough. I incorporated the check here so it disappears from the code entirely when !CONFIG_PREEMPT, because otherwise, you end up with a sequence that is mispredicted every # iterations without any benefit.
> I guess i could macroise that separately though.
>
>> If we had
>>
>> if_cond_yield_neon
>> // patchup code
>> endif_yield_neon
>>
I like this, but ...
>> then the caller is free to conditionally branch over that as appropriate
>> like
>>
>> loop:
>> // crypto stuff
>> tst x0, #0xf
>> b.ne loop
>>
>> if_cond_yield_neon
>> // patchup code
>> endif_cond_yield_neon
>>
I need to put the post patchup code somewhere too. Please refer to the
CRC patches for the best examples of this.
>> b loop
>>
>> I think this is clearer than burying checks and branches in a macro that
>> is trying to be generic.
>>
>
> Agreed.
>
>> Label arguments can be added to elide some branches of course, at a
>> corresponding cost to clarity... in the common case the cache will
>> be hot and the branches won't be mispredicted though. Is it really
>> worth it?
>>
>
> Perhaps not. And i have not made any attempts yet to benchmark at great detail, given that i need some feedback from the rt crowd first whether this is likely to work as desired.
>
>>> +
>>> + get_thread_info x0
>>> + ldr w1, [x0, #TSK_TI_PREEMPT]
>>> + ldr x0, [x0, #TSK_TI_FLAGS]
>>> + cmp w1, #1 // == PREEMPT_OFFSET
>>
>> asm-offsets?
>>
>
> This is not an offset in that regard, but the header that defines it is not asm safe
>
>> [...]
>>
>> Cheers
>> ---Dave
^ permalink raw reply
* [v2] mfd: stm32: Adopt SPDX identifier
From: Guenter Roeck @ 2017-12-05 18:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171205172811.GA5534@kroah.com>
On Tue, Dec 05, 2017 at 06:28:11PM +0100, Greg KH wrote:
> On Tue, Dec 05, 2017 at 09:23:37AM -0800, Guenter Roeck wrote:
> > On Tue, Dec 05, 2017 at 04:24:18PM +0100, benjamin.gaignard at linaro.org wrote:
> > > Add SPDX identifier
> > >
> > > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> > > ---
> > > drivers/mfd/stm32-lptimer.c | 6 +-----
> > > drivers/mfd/stm32-timers.c | 4 +---
> > > include/linux/mfd/stm32-lptimer.h | 6 +-----
> > > include/linux/mfd/stm32-timers.h | 4 +---
> > > 4 files changed, 4 insertions(+), 16 deletions(-)
> > >
> > > diff --git a/drivers/mfd/stm32-lptimer.c b/drivers/mfd/stm32-lptimer.c
> > > index 075330a25f61..a00f99f36559 100644
> > > --- a/drivers/mfd/stm32-lptimer.c
> > > +++ b/drivers/mfd/stm32-lptimer.c
> > > @@ -1,13 +1,9 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > /*
> >
> > I have been wondering - why is the SPDX identifier for the most part added
> > as separate C++ comment at the beginning of the file, and not inline with
> > the rest of the top comments ? SPDX doesn't mandate this - the examples
> > I can find on the SPDX web site all show it inline. Is there a special
> > Linux kernel convention ?
>
> See the patch series from Thomas that answers this question for you by
> adding documentation to the kernel explaining it all :)
>
So it is a special Linux kernel convention. Interesting (and brrr ... :-)
Thanks for the clarification.
Guenter
^ permalink raw reply
* [PATCH v4 0/5] ARM: ep93xx: ts72xx: Add support for BK3 board
From: Hartley Sweeten @ 2017-12-05 18:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171130235140.12243-1-lukma@denx.de>
On Thursday, November 30, 2017 4:52 PM, Lukasz Majewski wrote:
>
> This patch series adds support for Liebherr's BK3 board, being a derivative of TS72XX design.
>
> This patchset consists of following patches:
>
> - ts72xx.[c|h] cosmetic cleanup/improvement
> - Rewrite ts72xx.c to be reusable by bk3
> - The Liebherr's BK3 board has been added with re-using code of
> ts72xx.c (detalied list of changes can be found in patch 6/6)
>
> This series applies on top of linux-next/master (next-20171130)
>
> Lukasz Majewski (5):
> ARM: ep93xx: ts72xx: Provide include guards for ts72xx.h file
> ARM: ep93xx: ts72xx: Rewrite ts72xx_register_flash() to accept
> parameters
> ARM: ep93xx: ts72xx: Rewrite map IO code to be reusable
> ARM: ep93xx: ts72xx: cosmetic: Add some description to ts72xx code
> ARM: ep93xx: ts72xx: Add support for BK3 board - ts72xx derivative
>
> MAINTAINERS | 6 ++
> arch/arm/mach-ep93xx/Kconfig | 7 ++
> arch/arm/mach-ep93xx/ts72xx.c | 198 ++++++++++++++++++++++++++++++++++++++----
> arch/arm/mach-ep93xx/ts72xx.h | 9 ++
> arch/arm/tools/mach-types | 1 +
> 5 files changed, 202 insertions(+), 19 deletions(-)
Looks good.
Acked-by: H Hartley Sweeten <hsweeten@visionengravers.com>
Linus,
Would you mind picking this series up?
Thanks,
Hartley
^ permalink raw reply
* [PATCH v9 3/5] perf utils: use pmu->is_uncore to detect PMU UNCORE devices
From: Arnaldo Carvalho de Melo @ 2017-12-05 18:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1f79aa3c-a061-7c55-5d3e-f13638be01ad@linux.intel.com>
Em Tue, Dec 05, 2017 at 08:35:22PM +0800, Jin, Yao escreveu:
> A quick test with the new patch 'fix_json_v9_2.patch' shows it working.
I'll take this as a Tested-by: you, ok?
> See the log:
>
> root at skl:/tmp# perf stat --per-thread -p 10322 -M CPI,IPC
> ^C
> Performance counter stats for process id '10322':
>
> vmstat-10322 1,879,654 inst_retired.any #
> 0.8 CPI
> vmstat-10322 1,565,807 cycles
> vmstat-10322 1,879,654 inst_retired.any #
> 1.2 IPC
> vmstat-10322 1,565,807 cpu_clk_unhalted.thread
>
> 2.850291804 seconds time elapsed
>
> Thanks for fixing it quickly.
>
> Thanks
> Jin Yao
>
> On 12/5/2017 3:23 PM, Jin, Yao wrote:
> > Hi,
> >
> > I applied the diff but it's failed.
> >
> > jinyao at skl:~/skl-ws/perf-dev/lck-4594/src$ patch -p1 < 1.pat
> > patching file tools/perf/util/pmu.c
> > patch: **** malformed patch at line 41: *head, struct perf_pmu *pmu)
> >
> > Could you send the patch as attachment to me in another mail thread?
> >
> > to yao.jin at linux.intel.com
> > cc yao.jin at intel.com
> >
> > Thanks
> > Jin Yao
> >
> > On 12/5/2017 3:12 PM, Ganapatrao Kulkarni wrote:
> > > diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> > > index 5ad8a18..57e38fd 100644
> > > --- a/tools/perf/util/pmu.c
> > > +++ b/tools/perf/util/pmu.c
> > > @@ -538,6 +538,34 @@ static bool pmu_is_uncore(const char *name)
> > > ? }
> > >
> > > ? /*
> > > + *? PMU CORE devices have different name other than cpu in sysfs on some
> > > + *? platforms. looking for possible sysfs files to identify as core
> > > device.
> > > + */
> > > +static int is_pmu_core(const char *name)
> > > +{
> > > + struct stat st;
> > > + char path[PATH_MAX];
> > > + const char *sysfs = sysfs__mountpoint();
> > > +
> > > + if (!sysfs)
> > > + return 0;
> > > +
> > > + /* Look for cpu sysfs (x86 and others) */
> > > + scnprintf(path, PATH_MAX, "%s/bus/event_source/devices/cpu", sysfs);
> > > + if ((stat(path, &st) == 0) &&
> > > + (strncmp(name, "cpu", strlen("cpu")) == 0))
> > > + return 1;
> > > +
> > > + /* Look for cpu sysfs (specific to arm) */
> > > + scnprintf(path, PATH_MAX, "%s/bus/event_source/devices/%s/cpus",
> > > + sysfs, name);
> > > + if (stat(path, &st) == 0)
> > > + return 1;
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +/*
> > > ?? * Return the CPU id as a raw string.
> > > ?? *
> > > ?? * Each architecture should provide a more precise id string that
> > > @@ -641,7 +669,7 @@ static void pmu_add_cpu_aliases(struct list_head
> > > *head, struct perf_pmu *pmu)
> > > ?? break;
> > > ?? }
> > >
> > > - if (pmu->is_uncore) {
> > > + if (!is_pmu_core(name)) {
> > > ?? /* check for uncore devices */
> > > ?? if (pe->pmu == NULL)
> > > ?? continue;
^ permalink raw reply
* [RFC PATCH 1/6] drm: Add Content Protection property
From: Sean Paul @ 2017-12-05 19:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171205173408.GA18425@amd>
On Tue, Dec 5, 2017 at 12:34 PM, Pavel Machek <pavel@ucw.cz> wrote:
> On Tue 2017-12-05 11:45:38, Daniel Vetter wrote:
>> On Tue, Dec 05, 2017 at 11:28:40AM +0100, Pavel Machek wrote:
>> > On Wed 2017-11-29 22:08:56, Sean Paul wrote:
>> > > This patch adds a new optional connector property to allow userspace to enable
>> > > protection over the content it is displaying. This will typically be implemented
>> > > by the driver using HDCP.
>> > >
>> > > The property is a tri-state with the following values:
>> > > - OFF: Self explanatory, no content protection
>> > > - DESIRED: Userspace requests that the driver enable protection
>> > > - ENABLED: Once the driver has authenticated the link, it sets this value
>> > >
>> > > The driver is responsible for downgrading ENABLED to DESIRED if the link becomes
>> > > unprotected. The driver should also maintain the desiredness of protection
>> > > across hotplug/dpms/suspend.
>> >
>> > Why would user of the machine want this to be something else than
>> > 'OFF'?
>> >
>> > If kernel implements this, will it mean hardware vendors will have to
>> > prevent user from updating kernel on machines they own?
>> >
>> > If this is merged, does it open kernel developers to DMCA threats if
>> > they try to change it?
>>
>> Because this just implements one part of the content protection scheme.
>> This only gives you an option to enable HDCP (aka encryption, it's really
>> nothing else) on the cable. Just because it has Content Protection in the
>> name does _not_ mean it is (stand-alone) an effective nor complete content
>> protection scheme. It's simply encrypting data, that's all.
>
> Yep. So my first question was: why would user of the machine ever want
> encryption "ENABLED" or "DESIRED"? Could you answer it?
>
Sure. We have a lot of Chrome OS users who would really like to enjoy
premium hd content on their tvs.
>> If you want to actually lock down a machine to implement content
>> protection, then you need secure boot without unlockable boot-loader and a
>> pile more bits in userspace. If you do all that, only then do you have
>> full content protection. And yes, then you don't really own the machine
>> fully, and I think users who are concerned with being able to update
>> their
>
> Yes, so... This patch makes it more likely to see machines with locked
> down kernels, preventing developers from working with systems their
> own, running hardware. That is evil, and direct threat to Free
> software movement.
>
> Users compiling their own kernels get no benefit from it. Actually it
> looks like this only benefits Intel and Disney. We don't want that.
>
Major citation needed here. Did you actually read the code? If you
did, you would realize that the feature is already latent in your
computer. This patchset merely exposes how that hardware can be
enabled to encrypt your video link. Would you have the same problems
with a new whizzbang video encoding format, or is it just the "Content
Protection" name? Perhaps you'd prefer this feature was implemented in
Intel's firmware, or a userspace blob? It wouldn't change the fact
that those registers exist and _can_ be used for HDCP, it's just that
now you know about it.
Having all of the code in the open allows users to see what is
happening with their hardware, how is this a bad thing?
Sean
>> kernels and be able to exercise their software freedoms already know to
>> avoid such locked down systems.
>>
>> So yeah it would be better to call this the "HDMI/DP cable encryption
>> support", but well, it's not what it's called really.
>
> Well, it does not belong in kernel, no matter what is the name.
>
> Pavel
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply
* [PATCH v2] irqchip/gic-v3: Fix the driver probe() fail due to disabled GICC entry
From: Shanker Donthineni @ 2017-12-05 19:03 UTC (permalink / raw)
To: linux-arm-kernel
The ACPI specification says OS shouldn't attempt to use GICC configuration
parameters if the flag ACPI_MADT_ENABLED is cleared. The ARM64-SMP code
skips the disabled GICC entries but not causing any issue. However the
current GICv3 driver probe bails out causing kernel panic() instead of
skipping the disabled GICC interfaces. This issues on happens on systems
where redistributor regions are not in the always-on power domain and
one of GICC interface marked with ACPI_MADT_ENABLED=0.
This patch does the two things to fix the panic.
- Don't return an error in gic_acpi_match_gicc() for disabled GICC entry.
- No need to keep GICR region information for disabled GICC entry.
Observed kernel crash on QDF2400 platform GICC entry is disabled.
Kernel crash traces:
Kernel panic - not syncing: No interrupt controller found.
CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.13.5 #26
[<ffff000008087770>] dump_backtrace+0x0/0x218
[<ffff0000080879dc>] show_stack+0x14/0x20
[<ffff00000883b078>] dump_stack+0x98/0xb8
[<ffff0000080c5c14>] panic+0x118/0x26c
[<ffff000008b62348>] init_IRQ+0x24/0x2c
[<ffff000008b609fc>] start_kernel+0x230/0x394
[<ffff000008b601e4>] __primary_switched+0x64/0x6c
---[ end Kernel panic - not syncing: No interrupt controller found.
Signed-off-by: Shanker Donthineni <shankerd@codeaurora.org>
---
changes since v1:
Added a new "if condition" to skip disabled GICC entry and edited commit.
drivers/irqchip/irq-gic-v3.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index b56c3e2..606d4d2 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -1331,6 +1331,10 @@ static int __init gic_of_init(struct device_node *node, struct device_node *pare
u32 size = reg == GIC_PIDR2_ARCH_GICv4 ? SZ_64K * 4 : SZ_64K * 2;
void __iomem *redist_base;
+ /* GICC entry which has !ACPI_MADT_ENABLED is not unusable so skip */
+ if (!(gicc->flags & ACPI_MADT_ENABLED))
+ return 0;
+
redist_base = ioremap(gicc->gicr_base_address, size);
if (!redist_base)
return -ENOMEM;
@@ -1380,6 +1384,13 @@ static int __init gic_acpi_match_gicc(struct acpi_subtable_header *header,
if ((gicc->flags & ACPI_MADT_ENABLED) && gicc->gicr_base_address)
return 0;
+ /*
+ * It's perfectly valid firmware can pass disabled GICC entry, driver
+ * should treat an errors, skip the entry.
+ */
+ if (!(gicc->flags & ACPI_MADT_ENABLED))
+ return 0;
+
return -ENODEV;
}
--
Qualcomm Datacenter Technologies, Inc. on behalf of the Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply related
* [PATCH net-next 2/2 v6] net: ethernet: Add a driver for Gemini gigabit ethernet
From: Michał Mirosław @ 2017-12-05 19:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171202110640.5284-2-linus.walleij@linaro.org>
On Sat, Dec 02, 2017 at 12:06:40PM +0100, Linus Walleij wrote:
[...]
> The latest v6 incarnation of this driver was written by Micha?
> Miros?aw and submitted for inclusion in 2011. This was the
> last post:
> https://lwn.net/Articles/437889/
>
> DaveM ACKed it at the time:
> https://marc.info/?l=linux-netdev&m=130255434310315&w=2
>
> The controversial pieces under ARM (board files) and other
> subsystems are now gone and replaced by DeviceTree.
>
> Micha?: I hope you don't mind me picking it up and hope
> you can still test it on your ICYbox, I have device tree
> patches in my tree:
> https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-nomadik.git/log/?h=gemini-ethernet
I'm happy to see that my work didn't go to /dev/null after all.
I haven't finished it at the time because the box I had broke down
beyond repair.
I skimmed through the patch - please find my comments below.
Best Regards,
Micha? Miros?aw
[...]
> --- /dev/null
> +++ b/drivers/net/ethernet/cortina/gemini.c
> @@ -0,0 +1,2461 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Ethernet device driver for Cortina Systems Gemini SoC
> + * Also known as the StorLink SL3512 and SL3516 (SL351x) GMAC
> + *
> + * Authors:
> + * Linus Walleij <linus.walleij@linaro.org>
> + * Tobias Waldvogel <tobias.waldvogel@gmail.com> (OpenWRT)
> + * Micha?? Miros??aw <mirq-linux@rere.qmqm.pl>
Doubly UTF-8 encoded?
[...]
> +static int gmac_setup_txqs(struct net_device *netdev)
> +{
[...]
> + desc_ring = dma_alloc_coherent(geth->dev, len * sizeof(*desc_ring),
> + &port->txq_dma_base, GFP_KERNEL);
> +
> + if (!desc_ring) {
Should check with dma_mapping_error().
> + kfree(skb_tab);
> + return -ENOMEM;
> + }
> +
> + BUG_ON(port->txq_dma_base & ~DMA_Q_BASE_MASK);
BUG is too hard here. return -EINVAL or other error? Shouldn't happen
if dma_alloc_coherent() guarantees 16B alignment.
[...]
> +static int gmac_setup_rxq(struct net_device *netdev)
> +{
[...]
> + BUG_ON(port->rxq_dma_base & ~NONTOE_QHDR0_BASE_MASK);
Like above.
[...]
> +static struct page *geth_freeq_alloc_map_page(struct gemini_ethernet *geth,
> + int pn)
> +{
> + unsigned int fpp_order = PAGE_SHIFT - geth->freeq_frag_order;
> + unsigned int frag_len = 1 << geth->freeq_frag_order;
> + GMAC_RXDESC_T *freeq_entry;
> + dma_addr_t mapping;
> + struct page *page;
> + int i;
> +
> + page = alloc_page(GFP_ATOMIC);
> + if (!page)
> + return NULL;
> +
> + mapping = dma_map_single(geth->dev, page_address(page),
> + PAGE_SIZE, DMA_FROM_DEVICE);
> +
> + if (unlikely(dma_mapping_error(geth->dev, mapping) || !mapping)) {
This should test only dma_mapping_error() since mapping == 0 is valid,
but unlikely.
> +static int geth_setup_freeq(struct gemini_ethernet *geth)
> +{
> + void __iomem *dma_reg = geth->base + GLOBAL_SW_FREEQ_BASE_SIZE_REG;
> + QUEUE_THRESHOLD_T qt;
> + DMA_SKB_SIZE_T skbsz;
> + unsigned int filled;
> + unsigned int frag_len = 1 << geth->freeq_frag_order;
> + unsigned int len = 1 << geth->freeq_order;
> + unsigned int fpp_order = PAGE_SHIFT - geth->freeq_frag_order;
> + unsigned int pages = len >> fpp_order;
> + dma_addr_t mapping;
> + unsigned int pn;
> +
> + geth->freeq_ring = dma_alloc_coherent(geth->dev,
> + sizeof(*geth->freeq_ring) << geth->freeq_order,
> + &geth->freeq_dma_base, GFP_KERNEL);
> + if (!geth->freeq_ring)
> + return -ENOMEM;
> +
> + BUG_ON(geth->freeq_dma_base & ~DMA_Q_BASE_MASK);
Another BUG_ON:
if (WARN_ON(...)) goto err_freeq;
[...]
> +static int gmac_map_tx_bufs(struct net_device *netdev, struct sk_buff *skb,
> + struct gmac_txq *txq, unsigned short *desc)
> +{
[...]
> + if (word1 > mtu) {
> + word1 |= TSS_MTU_ENABLE_BIT;
> + word3 += mtu;
word3 |= mtu; would be more natural.
[...]
> + mapping = dma_map_single(geth->dev, buffer, buflen,
> + DMA_TO_DEVICE);
> + if (dma_mapping_error(geth->dev, mapping) ||
> + !(mapping & PAGE_MASK))
> + goto map_error;
Is page at phys 0 special to the HW?
[...]
> +map_error:
> + while (w != *desc) {
> + w--;
> + w &= m;
> +
> + dma_unmap_page(geth->dev, txq->ring[w].word2.buf_adr,
> + txq->ring[w].word0.bits.buffer_size, DMA_TO_DEVICE);
> + }
> + return ENOMEM;
return -ENOMEM; for consistency, though not used in the caller.
[...]
> +static int gmac_start_xmit(struct sk_buff *skb, struct net_device *netdev)
> +{
[...]
> + if (unlikely(gmac_map_tx_bufs(netdev, skb, txq, &w))) {
> + if (skb_linearize(skb))
> + goto out_drop;
> +
> + if (unlikely(gmac_map_tx_bufs(netdev, skb, txq, &w)))
> + goto out_drop_free;
> +
> + u64_stats_update_begin(&port->tx_stats_syncp);
> + port->tx_frags_linearized++;
> + u64_stats_update_end(&port->tx_stats_syncp);
This misses stats update when mapping after skb_linearize() fails.
[...]
> +static struct sk_buff *gmac_skb_if_good_frame(struct gemini_ethernet_port *port,
> + GMAC_RXDESC_0_T word0, unsigned frame_len)
> +{
> + struct sk_buff *skb = NULL;
> + unsigned rx_status = word0.bits.status;
> + unsigned rx_csum = word0.bits.chksum_status;
> + port->rx_stats[rx_status]++;
> + port->rx_csum_stats[rx_csum]++;
> +
> + if (word0.bits.derr || word0.bits.perr ||
> + rx_status || frame_len < ETH_ZLEN ||
> + rx_csum >= RX_CHKSUM_IP_ERR_UNKNOWN) {
> + port->stats.rx_errors++;
> +
> + if (frame_len < ETH_ZLEN || RX_ERROR_LENGTH(rx_status))
> + port->stats.rx_length_errors++;
> + if (RX_ERROR_OVER(rx_status))
> + port->stats.rx_over_errors++;
> + if (RX_ERROR_CRC(rx_status))
> + port->stats.rx_crc_errors++;
> + if (RX_ERROR_FRAME(rx_status))
> + port->stats.rx_frame_errors++;
> +
> + return NULL;
Could support RXALL feature here.
> + skb = napi_get_frags(&port->napi);
> + if (!skb)
> + return NULL;
This case could get a stats update, too.
> +static unsigned int gmac_rx(struct net_device *netdev, unsigned budget)
> +{
[...]
> + if (unlikely(!mapping)) {
> + netdev_err(netdev,
> + "rxq[%u]: HW BUG: zero DMA desc\n", r);
> + goto err_drop;
> + }
I wonder if this was a bug in the driver or in HW. Does it trigger on
your boxes?
[...]
> +static void gmac_set_rx_mode(struct net_device *netdev)
> +{
> + struct gemini_ethernet_port *port = netdev_priv(netdev);
> + struct netdev_hw_addr *ha;
> + __u32 mc_filter[2];
> + unsigned bit_nr;
> + GMAC_RX_FLTR_T filter = { .bits = {
> + .broadcast = 1,
> + .multicast = 1,
> + .unicast = 1,
> + } };
> +
> + mc_filter[1] = mc_filter[0] = 0;
Looks like this should be = ~0u (IFF_ALLMULTI case).
> + if (netdev->flags & IFF_PROMISC) {
> + filter.bits.error = 1;
> + filter.bits.promiscuous = 1;
> + } else if (!(netdev->flags & IFF_ALLMULTI)) {
> + mc_filter[1] = mc_filter[0] = 0;
> + netdev_for_each_mc_addr(ha, netdev) {
> + bit_nr = ~crc32_le(~0, ha->addr, ETH_ALEN) & 0x3f;
> + mc_filter[bit_nr >> 5] |= 1 << (bit_nr & 0x1f);
> + }
> + }
[...]
^ permalink raw reply
* [PATCH v3] irqchip/gic-v3: Fix the driver probe() fail due to disabled GICC entry
From: Shanker Donthineni @ 2017-12-05 19:07 UTC (permalink / raw)
To: linux-arm-kernel
The ACPI specification says OS shouldn't attempt to use GICC configuration
parameters if the flag ACPI_MADT_ENABLED is cleared. The ARM64-SMP code
skips the disabled GICC entries but not causing any issue. However the
current GICv3 driver probe bails out causing kernel panic() instead of
skipping the disabled GICC interfaces. This issue happens on systems
where redistributor regions are not in the always-on power domain and
one of GICC interface is marked with ACPI_MADT_ENABLED=0.
This patch does the two things to fix the panic.
- Don't return an error in gic_acpi_match_gicc() for disabled GICC entry.
- No need to keep GICR region information for disabled GICC entry.
Observed kernel crash on QDF2400 platform GICC entry is disabled.
Kernel crash traces:
Kernel panic - not syncing: No interrupt controller found.
CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.13.5 #26
[<ffff000008087770>] dump_backtrace+0x0/0x218
[<ffff0000080879dc>] show_stack+0x14/0x20
[<ffff00000883b078>] dump_stack+0x98/0xb8
[<ffff0000080c5c14>] panic+0x118/0x26c
[<ffff000008b62348>] init_IRQ+0x24/0x2c
[<ffff000008b609fc>] start_kernel+0x230/0x394
[<ffff000008b601e4>] __primary_switched+0x64/0x6c
---[ end Kernel panic - not syncing: No interrupt controller found.
Disabled GICC subtable example:
Subtable Type : 0B [Generic Interrupt Controller]
Length : 50
Reserved : 0000
CPU Interface Number : 0000003D
Processor UID : 0000003D
Flags (decoded below) : 00000000
Processor Enabled : 0
Performance Interrupt Trig Mode : 0
Virtual GIC Interrupt Trig Mode : 0
Parking Protocol Version : 00000000
Performance Interrupt : 00000017
Parked Address : 0000000000000000
Base Address : 0000000000000000
Virtual GIC Base Address : 0000000000000000
Hypervisor GIC Base Address : 0000000000000000
Virtual GIC Interrupt : 00000019
Redistributor Base Address : 0000FFFF88F40000
ARM MPIDR : 000000000000000D
Efficiency Class : 00
Reserved : 000000
Signed-off-by: Shanker Donthineni <shankerd@codeaurora.org>
---
changes since v1:
Edited commit text.
changes since v1:
Added a new "if condition" to skip disabled GICC entry and edited commit.
drivers/irqchip/irq-gic-v3.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index b56c3e2..606d4d2 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -1331,6 +1331,10 @@ static int __init gic_of_init(struct device_node *node, struct device_node *pare
u32 size = reg == GIC_PIDR2_ARCH_GICv4 ? SZ_64K * 4 : SZ_64K * 2;
void __iomem *redist_base;
+ /* GICC entry which has !ACPI_MADT_ENABLED is not unusable so skip */
+ if (!(gicc->flags & ACPI_MADT_ENABLED))
+ return 0;
+
redist_base = ioremap(gicc->gicr_base_address, size);
if (!redist_base)
return -ENOMEM;
@@ -1380,6 +1384,13 @@ static int __init gic_acpi_match_gicc(struct acpi_subtable_header *header,
if ((gicc->flags & ACPI_MADT_ENABLED) && gicc->gicr_base_address)
return 0;
+ /*
+ * It's perfectly valid firmware can pass disabled GICC entry, driver
+ * should treat an errors, skip the entry.
+ */
+ if (!(gicc->flags & ACPI_MADT_ENABLED))
+ return 0;
+
return -ENODEV;
}
--
Qualcomm Datacenter Technologies, Inc. on behalf of the Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply related
* [PATCH v4] irqchip/gic-v3: Fix the driver probe() fail due to disabled GICC entry
From: Shanker Donthineni @ 2017-12-05 19:16 UTC (permalink / raw)
To: linux-arm-kernel
The ACPI specification says OS shouldn't attempt to use GICC configuration
parameters if the flag ACPI_MADT_ENABLED is cleared. The ARM64-SMP code
skips the disabled GICC entries but not causing any issue. However the
current GICv3 driver probe bails out causing kernel panic() instead of
skipping the disabled GICC interfaces. This issue happens on systems
where redistributor regions are not in the always-on power domain and
one of GICC interface marked with ACPI_MADT_ENABLED=0.
This patch does the two things to fix the panic.
- Don't return an error in gic_acpi_match_gicc() for disabled GICC entry.
- No need to keep GICR region information for disabled GICC entry.
Observed kernel crash on QDF2400 platform GICC entry is disabled.
Kernel crash traces:
Kernel panic - not syncing: No interrupt controller found.
CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.13.5 #26
[<ffff000008087770>] dump_backtrace+0x0/0x218
[<ffff0000080879dc>] show_stack+0x14/0x20
[<ffff00000883b078>] dump_stack+0x98/0xb8
[<ffff0000080c5c14>] panic+0x118/0x26c
[<ffff000008b62348>] init_IRQ+0x24/0x2c
[<ffff000008b609fc>] start_kernel+0x230/0x394
[<ffff000008b601e4>] __primary_switched+0x64/0x6c
---[ end Kernel panic - not syncing: No interrupt controller found.
Disabled GICC subtable example:
Subtable Type : 0B [Generic Interrupt Controller]
Length : 50
Reserved : 0000
CPU Interface Number : 0000003D
Processor UID : 0000003D
Flags (decoded below) : 00000000
Processor Enabled : 0
Performance Interrupt Trig Mode : 0
Virtual GIC Interrupt Trig Mode : 0
Parking Protocol Version : 00000000
Performance Interrupt : 00000017
Parked Address : 0000000000000000
Base Address : 0000000000000000
Virtual GIC Base Address : 0000000000000000
Hypervisor GIC Base Address : 0000000000000000
Virtual GIC Interrupt : 00000019
Redistributor Base Address : 0000FFFF88F40000
ARM MPIDR : 000000000000000D
Efficiency Class : 00
Reserved : 000000
Signed-off-by: Shanker Donthineni <shankerd@codeaurora.org>
---
changes since v3:
Fix typo.
changes since v2:
Edited commit text.
changes since v1:
Added a new "if condition" to skip disabled GICC entry and edited commit.
drivers/irqchip/irq-gic-v3.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index b56c3e2..a874777 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -1331,6 +1331,10 @@ static int __init gic_of_init(struct device_node *node, struct device_node *pare
u32 size = reg == GIC_PIDR2_ARCH_GICv4 ? SZ_64K * 4 : SZ_64K * 2;
void __iomem *redist_base;
+ /* GICC entry which has !ACPI_MADT_ENABLED is not unusable so skip */
+ if (!(gicc->flags & ACPI_MADT_ENABLED))
+ return 0;
+
redist_base = ioremap(gicc->gicr_base_address, size);
if (!redist_base)
return -ENOMEM;
@@ -1380,6 +1384,13 @@ static int __init gic_acpi_match_gicc(struct acpi_subtable_header *header,
if ((gicc->flags & ACPI_MADT_ENABLED) && gicc->gicr_base_address)
return 0;
+ /*
+ * It's perfectly valid firmware can pass disabled GICC entry, driver
+ * should not treat as errors, skip the entry instead of probe fail.
+ */
+ if (!(gicc->flags & ACPI_MADT_ENABLED))
+ return 0;
+
return -ENODEV;
}
--
Qualcomm Datacenter Technologies, Inc. on behalf of the Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply related
* [PATCH v2] irqchip/gic-v3: Fix the driver probe() fail due to disabled GICC entry
From: Shanker Donthineni @ 2017-12-05 19:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1512500607-20160-1-git-send-email-shankerd@codeaurora.org>
Hi Marc,
I messed-up patch contents please disregard this one and review v4 patch.
[v4] irqchip/gic-v3: Fix the driver probe() fail due to disabled GICC entry
https://patchwork.kernel.org/patch/10093653/
--
Shanker Donthineni
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply
* [PATCH v2 00/27] Improve DE2 support
From: Maxime Ripard @ 2017-12-05 19:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <23633465.uxSxGdCvtg@jernej-laptop>
Hi,
On Tue, Dec 05, 2017 at 04:52:57PM +0100, Jernej ?krabec wrote:
> Dne torek, 05. december 2017 ob 11:36:18 CET je Maxime Ripard napisal(a):
> > Hi,
> >
> > On Fri, Dec 01, 2017 at 07:05:23AM +0100, Jernej Skrabec wrote:
> > > Current DE2 driver is very basic and uses a lot of magic constants since
> > > there is no documentation and knowledge about it was limited at the time.
> > >
> > > With studying BSP source code, deeper knowledge was gained which allows
> > > to improve mainline driver considerably.
> > >
> > > At the beginning of this series, some code refactoring is done as well
> > > as adding some checks (patches 1-15).
> > >
> > > Further patches add multi-plane support with HW scaling and all possible
> > > RGB formats (patches 16-21).
> > >
> > > At last, support for YUV formats is added (patches 22-26).
> > >
> > > At the end, I included patch which puts lowest plane before second lowest.
> > > This should help testing VI planes when mixer has configuration 1 VI plane
> > > and 1 or more UI planes (most SoCs except V3s).
> > >
> > > This code was developed on H3, but it should work on every SoC if correct
> > > configuration structure is provided.
> > >
> > > H3 code can be found here:
> > > https://github.com/jernejsk/linux-1/commits/de2_impr_for_next
> >
> > Thanks a lot for that huge rework.
> >
> > I've applied the patches 1 to 26, and will push them to drm-misc once
> > the compilations are done.
> >
> > In the future, if you happen to do such a huge rework again (which
> > hopefully won't be needed :)), please use the -M option of
> > format-patch. It will reduce a lot the verbosity of files renaming and
> > will help the review.
>
> Noted.
It turned out that there was also a change queued for drm-misc that
was renaming (and changing the prototype of)
drm_plane_helper_check_state into
drm_atomic_helper_check_plane_state. I fixed that up in tree, and
tested on the A83t, but you probably want to double check.
> I think I missed initialization of min_scaler and max_scaler in
> sun8i_vi_layer_atomic_check() in sun8i_vi_layer.c when I was reworking
> patches.
>
> Will you fix patch with those two lines
> min_scale = DRM_PLANE_HELPER_NO_SCALING;
> max_scale = DRM_PLANE_HELPER_NO_SCALING;
>
> or should I send new patch which fixes that or should I send new version of
> original patch?
We don't rebase in drm-misc, so please send an additional patch.
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20171205/40b15ffc/attachment.sig>
^ permalink raw reply
* [PATCH v3] irqchip/gic-v3: Fix the driver probe() fail due to disabled GICC entry
From: Shanker Donthineni @ 2017-12-05 19:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1512500849-20304-1-git-send-email-shankerd@codeaurora.org>
Hi Marc,
I messed-up patch contents please disregard this one and review v4 patch.
[v4] irqchip/gic-v3: Fix the driver probe() fail due to disabled GICC entry
https://patchwork.kernel.org/patch/10093653/
--
Shanker Donthineni
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply
* [linux-sunxi] [PATCH 1/2] clk: sunxi-ng: Support fixed post-dividers on MP style clocks
From: Maxime Ripard @ 2017-12-05 19:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAGb2v64w7EEYbJug+PQQwM8zXcyoOo8B+zk9MzU8cAW=K=gvEw@mail.gmail.com>
1;5002;0c
On Tue, Dec 05, 2017 at 11:01:11AM +0800, Chen-Yu Tsai wrote:
> On Tue, Dec 5, 2017 at 7:18 AM, Andr? Przywara <andre.przywara@arm.com> wrote:
> > Hi Chen-Yu,
> >
> > On 04/12/17 05:19, Chen-Yu Tsai wrote:
> >> On the A64, the MMC module clocks are fixed in the new timing mode,
> >> i.e. they do not have a bit to select the mode. These clocks have
> >> a 2x divider somewhere between the clock and the MMC module.
> >>
> >> To be consistent with other SoCs supporting the new timing mode,
> >> we model the 2x divider as a fixed post-divider on the MMC module
> >> clocks.
> >>
> >> To do this, we first add fixed post-divider to the MP style clocks,
> >> which the MMC module clocks are.
> >>
> >> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> >> ---
> >> drivers/clk/sunxi-ng/ccu_mp.c | 20 ++++++++++++++++++--
> >> drivers/clk/sunxi-ng/ccu_mp.h | 24 ++++++++++++++++++++++++
> >> 2 files changed, 42 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/clk/sunxi-ng/ccu_mp.c b/drivers/clk/sunxi-ng/ccu_mp.c
> >> index 688855e7dc8c..5d0af4051737 100644
> >> --- a/drivers/clk/sunxi-ng/ccu_mp.c
> >> +++ b/drivers/clk/sunxi-ng/ccu_mp.c
> >> @@ -50,12 +50,19 @@ static unsigned long ccu_mp_round_rate(struct ccu_mux_internal *mux,
> >> unsigned int max_m, max_p;
> >> unsigned int m, p;
> >>
> >> + if (cmp->common.features & CCU_FEATURE_FIXED_POSTDIV)
> >> + rate *= cmp->fixed_post_div;
> >
> > Can't you just initialise fixed_post_div to 1 normally and save the
> > CCU_FEATURE_FIXED_POSTDIV?
>
> I'll refer to Maxime about this. The feature flag was there from day
> one. We only started to implement support for it later on. I'm not
> sure if there was a reason to add them as feature flags, instead of
> a field that defaults to something (0 even).
>
> Otherwise it's a reasonable change. And we probably don't have to
> do a wholesale change for the other clocks in one go. Incidentally
> I have a A83T audio series that also adds post-dividers for another
> clock type. I'll wait for a conclusion on this end before posting
> it.
We can definitely remove that feature flag. However, that would also
mean going over all the clocks we define everywhere to set it to 1,
which is going to be cumbersome and likely to introduce some bugs. I
don't really want to tie those two patches to that effort.
I applied both, thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20171205/1874050b/attachment.sig>
^ permalink raw reply
* [PATCH] arm64/mm: Do not write ASID generation to ttbr0
From: James Morse @ 2017-12-05 19:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1512495507-23259-1-git-send-email-julien.thierry@arm.com>
Hi Julien,
On 05/12/17 17:38, Julien Thierry wrote:
> When writing the user ASID to ttbr0, 16bit get copied to ttbr0, potentially
> including part of the ASID generation in the ttbr0.ASID. If the kernel is
> using less than 16bits ASIDs and the other ttbr0 bits aren't RES0, two
> tasks using the same mm context might end up running with different
> ttbr0.ASID values.
> This would be triggered by one of the threads being scheduled before a
> roll-over and the second one scheduled after a roll-over.
>
> Pad the generation out of the 16bits of the mm id that are written to
> ttbr0. Thus, what the hardware sees is what the kernel considers ASID.
Is this to fix a system with a mix of 8/16 asid bits? Or someone being clever
and reducing asid_bits to stress rollover?
(I think we should do something like this so we can test rollover.)
> diff --git a/arch/arm64/mm/context.c b/arch/arm64/mm/context.c
> index 6f40170..a7c72d4 100644
> --- a/arch/arm64/mm/context.c
> +++ b/arch/arm64/mm/context.c
> @@ -180,6 +190,13 @@ static u64 new_context(struct mm_struct *mm, unsigned int cpu)
> /* We're out of ASIDs, so increment the global generation count */
> generation = atomic64_add_return_relaxed(ASID_FIRST_VERSION,
> &asid_generation);
> +
> + /*
> + * It is unlikely the generation will ever overflow, but if this
> + * happens, let it be known strange things can occur.
> + */
> + WARN_ON(generation == ASID_FIRST_VERSION);
Won't generation wrap to zero, not '1<<16'?
asid_generation-is-never-zero is one of the nasty things in this code.
In check_and_switch_context() we switch to the 'fast path' where the current
asid is usable if: the generation matches and the active_asid isn't 0 (which
indicates a rollover has occurred).
from mm/context.c::check_and_switch_context():
> if (!((asid ^ atomic64_read(&asid_generation)) >> asid_bits)
> && atomic64_xchg_relaxed(&per_cpu(active_asids, cpu), asid))
> goto switch_mm_fastpath;
If asid_generation is ever zero, (even briefly) multiple new tasks with
different pages-tables will pass the generation test, and run with asid=0.
Short of xchg(ASID_MAX_VERSION, ASID_FIRST_VERSION), every time, just in case, I
don't think this is something we can handle.
But, this thing is layers on layers of subtle behaviour, so I may have missed
something...
Instead, do you think we can duplicate just the asid bits (asid & ASID_MASK)
into a new 16bit field in mm_context_t, which we then use instead of the ASID()
and mmid macros? (We only set a new asid in one place as returned by new_context()).
This would let context.c's asid_bits be arbitrary, as the hardware only uses the
masked value it exposes in the new field.
This doesn't solve the mixed 8/16 bit case. I think we should force those
systems to use 8bit asids via cpufeature to preserve as much of the generation
counter as possible.
Thanks,
James
^ permalink raw reply
* [PATCH] Arm: mm: ftrace: Only set text back to ro after kernel has been marked ro
From: Kees Cook @ 2017-12-05 19:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171205133601.GX10595@n2100.armlinux.org.uk>
On Tue, Dec 5, 2017 at 5:36 AM, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
> On Tue, Dec 05, 2017 at 01:30:11PM +0000, Phil Elwell wrote:
>> This was my initial explanation:
>>
>> 1. Data which is marked __ro_after_init is initially writeable.
>>
>> 2. The ro_perms data covers kernel text, read-only data and __ro_after_init data.
>>
>> 3. set_kernel_text_rw marks everything in ro_perms as writeable.
>>
>> 4. set_kernel_text_ro marks everything in ro_perms as read-only, including the __ro_after_init data.
>>
>> 5. Using the function tracing code involves code modification, resulting in calls to
>> __ftrace_modify_code and set_kernel_text_ro.
>>
>> 6. Therefore if function tracing is enabled before kernel_init has completed then the __ro_after_init
>> data is made read-only prematurely.
>
> My question still stands, but let me rephrase. Do we need
> set_kernel_text_*() to touch the read-only data?
We don't _need_ to, but they're all contiguous, so the ro_perms array
used by set_kernel_text_*() is actually only a single entry:
static struct section_perm ro_perms[] = {
/* Make kernel code and rodata RX (set RO). */
{
.name = "text/rodata RO",
.start = (unsigned long)_stext,
.end = (unsigned long)__init_begin,
...
-Kees
--
Kees Cook
Pixel Security
^ permalink raw reply
* [kernel-hardening][PATCH v3 1/3] arm: mm: dump: make page table dumping reusable
From: Kees Cook @ 2017-12-05 19:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171204142553.GA3344@pjb1027-Latitude-E5410>
On Mon, Dec 4, 2017 at 6:25 AM, Jinbum Park <jinb.park7@gmail.com> wrote:
> This patch refactors the arm page table dumping code,
> so multiple tables may be registered with the framework.
>
> This patch refers below commits of arm64.
> (4674fdb9f149 ("arm64: mm: dump: make page table dumping reusable"))
> (4ddb9bf83349 ("arm64: dump: Make ptdump debugfs a separate option"))
>
> Signed-off-by: Jinbum Park <jinb.park7@gmail.com>
> ---
> v3: No changes
>
> arch/arm/Kconfig.debug | 6 +++-
> arch/arm/include/asm/ptdump.h | 48 ++++++++++++++++++++++++++++++++
> arch/arm/mm/Makefile | 3 +-
> arch/arm/mm/dump.c | 65 +++++++++++++++++++------------------------
> arch/arm/mm/ptdump_debugfs.c | 34 ++++++++++++++++++++++
> 5 files changed, 117 insertions(+), 39 deletions(-)
> create mode 100644 arch/arm/include/asm/ptdump.h
> create mode 100644 arch/arm/mm/ptdump_debugfs.c
>
> diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
> index 17685e1..e7b94db 100644
> --- a/arch/arm/Kconfig.debug
> +++ b/arch/arm/Kconfig.debug
> @@ -3,10 +3,14 @@ menu "Kernel hacking"
>
> source "lib/Kconfig.debug"
>
> -config ARM_PTDUMP
> +config ARM_PTDUMP_CORE
> + def_bool n
> +
> +config ARM_PTDUMP_DEBUGFS
> bool "Export kernel pagetable layout to userspace via debugfs"
> depends on DEBUG_KERNEL
> depends on MMU
> + select ARM_PTDUMP_CORE
> select DEBUG_FS
> ---help---
> Say Y here if you want to show the kernel pagetable layout in a
> diff --git a/arch/arm/include/asm/ptdump.h b/arch/arm/include/asm/ptdump.h
> new file mode 100644
> index 0000000..3a6c0b7
> --- /dev/null
> +++ b/arch/arm/include/asm/ptdump.h
> @@ -0,0 +1,48 @@
> +/*
> + * Copyright (C) 2014 ARM Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +#ifndef __ASM_PTDUMP_H
> +#define __ASM_PTDUMP_H
> +
> +#ifdef CONFIG_ARM_PTDUMP_CORE
Is this #ifdef needed? I think this file is only included in dump.c
and ptdump_debugfs.c, both of which are only built when
CONFIG_ARM_PTDUMP_CORE is defined.
> +
> +#include <linux/mm_types.h>
> +#include <linux/seq_file.h>
> +
> +struct addr_marker {
> + unsigned long start_address;
> + char *name;
> +};
> +
> +struct ptdump_info {
> + struct mm_struct *mm;
> + const struct addr_marker *markers;
> + unsigned long base_addr;
> +};
> +
> +void ptdump_walk_pgd(struct seq_file *s, struct ptdump_info *info);
> +#ifdef CONFIG_ARM_PTDUMP_DEBUGFS
> +int ptdump_debugfs_register(struct ptdump_info *info, const char *name);
> +#else
> +static inline int ptdump_debugfs_register(struct ptdump_info *info,
> + const char *name)
> +{
> + return 0;
> +}
> +#endif /* CONFIG_ARM_PTDUMP_DEBUGFS */
> +
> +#endif /* CONFIG_ARM_PTDUMP_CORE */
> +
> +#endif /* __ASM_PTDUMP_H */
> diff --git a/arch/arm/mm/Makefile b/arch/arm/mm/Makefile
> index 01bcc33..28be5f4 100644
> --- a/arch/arm/mm/Makefile
> +++ b/arch/arm/mm/Makefile
> @@ -13,7 +13,8 @@ obj-y += nommu.o
> obj-$(CONFIG_ARM_MPU) += pmsa-v7.o
> endif
>
> -obj-$(CONFIG_ARM_PTDUMP) += dump.o
> +obj-$(CONFIG_ARM_PTDUMP_CORE) += dump.o
> +obj-$(CONFIG_ARM_PTDUMP_DEBUGFS) += ptdump_debugfs.o
> obj-$(CONFIG_MODULES) += proc-syms.o
> obj-$(CONFIG_DEBUG_VIRTUAL) += physaddr.o
>
> diff --git a/arch/arm/mm/dump.c b/arch/arm/mm/dump.c
> index fc3b440..8dfe7c3 100644
> --- a/arch/arm/mm/dump.c
> +++ b/arch/arm/mm/dump.c
> @@ -21,11 +21,7 @@
> #include <asm/fixmap.h>
> #include <asm/memory.h>
> #include <asm/pgtable.h>
> -
> -struct addr_marker {
> - unsigned long start_address;
> - const char *name;
> -};
> +#include <asm/ptdump.h>
>
> static struct addr_marker address_markers[] = {
> { MODULES_VADDR, "Modules" },
> @@ -335,50 +331,36 @@ static void walk_pud(struct pg_state *st, pgd_t *pgd, unsigned long start)
> }
> }
>
> -static void walk_pgd(struct seq_file *m)
> +static void walk_pgd(struct pg_state *st, struct mm_struct *mm,
> + unsigned long start)
> {
> - pgd_t *pgd = swapper_pg_dir;
> - struct pg_state st;
> - unsigned long addr;
> + pgd_t *pgd = pgd_offset(mm, 0UL);
> unsigned i;
> -
> - memset(&st, 0, sizeof(st));
> - st.seq = m;
> - st.marker = address_markers;
> + unsigned long addr;
>
> for (i = 0; i < PTRS_PER_PGD; i++, pgd++) {
> - addr = i * PGDIR_SIZE;
> + addr = start + i * PGDIR_SIZE;
> if (!pgd_none(*pgd)) {
> - walk_pud(&st, pgd, addr);
> + walk_pud(st, pgd, addr);
> } else {
> - note_page(&st, addr, 1, pgd_val(*pgd), NULL);
> + note_page(st, addr, 1, pgd_val(*pgd), NULL);
> }
> }
> -
> - note_page(&st, 0, 0, 0, NULL);
> }
>
> -static int ptdump_show(struct seq_file *m, void *v)
> +void ptdump_walk_pgd(struct seq_file *m, struct ptdump_info *info)
> {
> - walk_pgd(m);
> - return 0;
> -}
> + struct pg_state st = {
> + .seq = m,
> + .marker = info->markers,
> + };
>
> -static int ptdump_open(struct inode *inode, struct file *file)
> -{
> - return single_open(file, ptdump_show, NULL);
> + walk_pgd(&st, info->mm, info->base_addr);
> + note_page(&st, 0, 0, 0, NULL);
> }
>
> -static const struct file_operations ptdump_fops = {
> - .open = ptdump_open,
> - .read = seq_read,
> - .llseek = seq_lseek,
> - .release = single_release,
> -};
> -
> -static int ptdump_init(void)
> +static void ptdump_initialize(void)
> {
> - struct dentry *pe;
> unsigned i, j;
>
> for (i = 0; i < ARRAY_SIZE(pg_level); i++)
> @@ -387,9 +369,18 @@ static int ptdump_init(void)
> pg_level[i].mask |= pg_level[i].bits[j].mask;
>
> address_markers[2].start_address = VMALLOC_START;
> +}
>
> - pe = debugfs_create_file("kernel_page_tables", 0400, NULL, NULL,
> - &ptdump_fops);
> - return pe ? 0 : -ENOMEM;
> +static struct ptdump_info kernel_ptdump_info = {
> + .mm = &init_mm,
> + .markers = address_markers,
> + .base_addr = 0,
> +};
> +
> +static int ptdump_init(void)
> +{
> + ptdump_initialize();
> + return ptdump_debugfs_register(&kernel_ptdump_info,
> + "kernel_page_tables");
This changes the return value of ptdump_init. This should do similar
to what was done before:
return ptdump_debugfs_register(&kernel_ptdump_info,
"kernel_page_tables") ? 0 : -ENOMEM;
> }
> __initcall(ptdump_init);
> diff --git a/arch/arm/mm/ptdump_debugfs.c b/arch/arm/mm/ptdump_debugfs.c
> new file mode 100644
> index 0000000..be8d87b
> --- /dev/null
> +++ b/arch/arm/mm/ptdump_debugfs.c
> @@ -0,0 +1,34 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <linux/debugfs.h>
> +#include <linux/seq_file.h>
> +
> +#include <asm/ptdump.h>
> +
> +static int ptdump_show(struct seq_file *m, void *v)
> +{
> + struct ptdump_info *info = m->private;
> +
> + ptdump_walk_pgd(m, info);
> + return 0;
> +}
> +
> +static int ptdump_open(struct inode *inode, struct file *file)
> +{
> + return single_open(file, ptdump_show, inode->i_private);
> +}
> +
> +static const struct file_operations ptdump_fops = {
> + .open = ptdump_open,
> + .read = seq_read,
> + .llseek = seq_lseek,
> + .release = single_release,
> +};
> +
> +int ptdump_debugfs_register(struct ptdump_info *info, const char *name)
> +{
> + struct dentry *pe;
> +
> + pe = debugfs_create_file(name, 0400, NULL, info, &ptdump_fops);
> + return pe ? 0 : -ENOMEM;
> +
> +}
> --
> 1.9.1
>
-Kees
--
Kees Cook
Pixel Security
^ permalink raw reply
* [kernel-hardening][PATCH v3 2/3] arm: mm: dump: make the page table dumping seq_file optional
From: Kees Cook @ 2017-12-05 19:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171204142631.GA3359@pjb1027-Latitude-E5410>
On Mon, Dec 4, 2017 at 6:26 AM, Jinbum Park <jinb.park7@gmail.com> wrote:
> This patch makes the page table dumping seq_file optional.
> It makes the page table dumping code usable for other cases.
>
> This patch refers below commit of arm64.
> (ae5d1cf358a5
> ("arm64: dump: Make the page table dumping seq_file optional"))
>
> Signed-off-by: Jinbum Park <jinb.park7@gmail.com>
Looks good to me. :)
Acked-by: Kees Cook <keescook@chromium.org>
-Kees
> ---
> v3: No changes
>
> arch/arm/mm/dump.c | 28 +++++++++++++++++++++-------
> 1 file changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm/mm/dump.c b/arch/arm/mm/dump.c
> index 8dfe7c3..43a2bee 100644
> --- a/arch/arm/mm/dump.c
> +++ b/arch/arm/mm/dump.c
> @@ -34,6 +34,18 @@
> { -1, NULL },
> };
>
> +#define pt_dump_seq_printf(m, fmt, args...) \
> +({ \
> + if (m) \
> + seq_printf(m, fmt, ##args); \
> +})
> +
> +#define pt_dump_seq_puts(m, fmt) \
> +({ \
> + if (m) \
> + seq_printf(m, fmt); \
> +})
> +
> struct pg_state {
> struct seq_file *seq;
> const struct addr_marker *marker;
> @@ -210,7 +222,7 @@ static void dump_prot(struct pg_state *st, const struct prot_bits *bits, size_t
> s = bits->clear;
>
> if (s)
> - seq_printf(st->seq, " %s", s);
> + pt_dump_seq_printf(st->seq, " %s", s);
> }
> }
>
> @@ -224,7 +236,7 @@ static void note_page(struct pg_state *st, unsigned long addr,
> st->level = level;
> st->current_prot = prot;
> st->current_domain = domain;
> - seq_printf(st->seq, "---[ %s ]---\n", st->marker->name);
> + pt_dump_seq_printf(st->seq, "---[ %s ]---\n", st->marker->name);
> } else if (prot != st->current_prot || level != st->level ||
> domain != st->current_domain ||
> addr >= st->marker[1].start_address) {
> @@ -232,7 +244,7 @@ static void note_page(struct pg_state *st, unsigned long addr,
> unsigned long delta;
>
> if (st->current_prot) {
> - seq_printf(st->seq, "0x%08lx-0x%08lx ",
> + pt_dump_seq_printf(st->seq, "0x%08lx-0x%08lx ",
> st->start_address, addr);
>
> delta = (addr - st->start_address) >> 10;
> @@ -240,17 +252,19 @@ static void note_page(struct pg_state *st, unsigned long addr,
> delta >>= 10;
> unit++;
> }
> - seq_printf(st->seq, "%9lu%c", delta, *unit);
> + pt_dump_seq_printf(st->seq, "%9lu%c", delta, *unit);
> if (st->current_domain)
> - seq_printf(st->seq, " %s", st->current_domain);
> + pt_dump_seq_printf(st->seq, " %s",
> + st->current_domain);
> if (pg_level[st->level].bits)
> dump_prot(st, pg_level[st->level].bits, pg_level[st->level].num);
> - seq_printf(st->seq, "\n");
> + pt_dump_seq_printf(st->seq, "\n");
> }
>
> if (addr >= st->marker[1].start_address) {
> st->marker++;
> - seq_printf(st->seq, "---[ %s ]---\n", st->marker->name);
> + pt_dump_seq_printf(st->seq, "---[ %s ]---\n",
> + st->marker->name);
> }
> st->start_address = addr;
> st->current_prot = prot;
> --
> 1.9.1
>
--
Kees Cook
Pixel Security
^ permalink raw reply
* [kernel-hardening][PATCH v3 0/3] arm: Makes ptdump resuable and add WX page checking
From: Kees Cook @ 2017-12-05 19:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171204142458.GA3325@pjb1027-Latitude-E5410>
On Mon, Dec 4, 2017 at 6:24 AM, Jinbum Park <jinb.park7@gmail.com> wrote:
> Hi,
>
> Page table dumping code for arm64-x86 is reusable,
> and they have function for WX page checking.
> But arm doesn't have that.
>
> This path series are to makes ptdump reusable,
> and add WX page checking for arm.
> This is heavily based on arm64 version.
Thanks for working on this! I sent along a few nits.
-Kees
>
> v2 :
> Fix a sender name of mail header, there was an mistake.
> (from "jinb.park" to Jinbum Park)
> Contents of patch-set are perfectly same.
>
> v3 :
> Take advantage of the existing pg_level and bits arrays
> to check ro, nx prot.
>
> jinb.park (3):
> arm: mm: dump: make page table dumping reusable
> arm: mm: dump: make the page table dumping seq_file optional
> arm: mm: dump: add checking for writable and executable pages
>
> arch/arm/Kconfig.debug | 33 +++++++++-
> arch/arm/include/asm/ptdump.h | 56 ++++++++++++++++
> arch/arm/mm/Makefile | 3 +-
> arch/arm/mm/dump.c | 144 +++++++++++++++++++++++++++++-------------
> arch/arm/mm/init.c | 2 +
> arch/arm/mm/ptdump_debugfs.c | 34 ++++++++++
> 6 files changed, 226 insertions(+), 46 deletions(-)
> create mode 100644 arch/arm/include/asm/ptdump.h
> create mode 100644 arch/arm/mm/ptdump_debugfs.c
>
> --
> 1.9.1
>
--
Kees Cook
Pixel Security
^ permalink raw reply
* [PATCH 2/2] clk: sunxi-ng: sun50i: a64: Add 2x fixed post-divider to MMC module clocks
From: Maxime Ripard @ 2017-12-05 19:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171204051912.7485-3-wens@csie.org>
Hi,
On Mon, Dec 04, 2017 at 01:19:12PM +0800, Chen-Yu Tsai wrote:
> On the A64, the MMC module clocks are fixed in the new timing mode,
> i.e. they do not have a bit to select the mode. These clocks have
> a 2x divider somewhere between the clock and the MMC module.
>
> To be consistent with other SoCs supporting the new timing mode,
> we model the 2x divider as a fixed post-divider on the MMC module
> clocks.
>
> This patch adds the post-dividers to the MMC clocks.
>
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
I had a doubt applying that one... sorry.
> ---
> drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 57 +++++++++++++++++++++++------------
> 1 file changed, 37 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> index 2bb4cabf802f..ee9c12cf3f08 100644
> --- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> +++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> @@ -400,28 +400,45 @@ static SUNXI_CCU_MP_WITH_MUX_GATE(nand_clk, "nand", mod0_default_parents, 0x080,
> BIT(31), /* gate */
> 0);
>
> +/*
> + * MMC clocks are the new timing mode (see A83T & H3) variety, but without
> + * the mode switch. This means they have a 2x post divider between the clock
> + * and the MMC module. This is not documented in the manual, but is taken
> + * into consideration when setting the mmc module clocks in the BSP kernel.
> + * Without it, MMC performance is degraded.
> + *
> + * We model it here to be consistent with other SoCs supporting this mode.
> + * The alternative would be to add the 2x multiplier when setting the MMC
> + * module clock in the MMC driver, just for the A64.
> + */
> static const char * const mmc_default_parents[] = { "osc24M", "pll-periph0-2x",
> "pll-periph1-2x" };
> -static SUNXI_CCU_MP_WITH_MUX_GATE(mmc0_clk, "mmc0", mmc_default_parents, 0x088,
> - 0, 4, /* M */
> - 16, 2, /* P */
> - 24, 2, /* mux */
> - BIT(31), /* gate */
> - 0);
> -
> -static SUNXI_CCU_MP_WITH_MUX_GATE(mmc1_clk, "mmc1", mmc_default_parents, 0x08c,
> - 0, 4, /* M */
> - 16, 2, /* P */
> - 24, 2, /* mux */
> - BIT(31), /* gate */
> - 0);
> -
> -static SUNXI_CCU_MP_WITH_MUX_GATE(mmc2_clk, "mmc2", mmc_default_parents, 0x090,
> - 0, 4, /* M */
> - 16, 2, /* P */
> - 24, 2, /* mux */
> - BIT(31), /* gate */
> - 0);
> +static SUNXI_CCU_MP_WITH_MUX_GATE_POSTDIV(mmc0_clk, "mmc0",
> + mmc_default_parents, 0x088,
> + 0, 4, /* M */
> + 16, 2, /* P */
> + 24, 2, /* mux */
> + BIT(31), /* gate */
> + 2, /* post-div */
> + 0);
> +
> +static SUNXI_CCU_MP_WITH_MUX_GATE_POSTDIV(mmc1_clk, "mmc1",
> + mmc_default_parents, 0x08c,
> + 0, 4, /* M */
> + 16, 2, /* P */
> + 24, 2, /* mux */
> + BIT(31), /* gate */
> + 2, /* post-div */
> + 0);
> +
Are you sure that the divider there for the non-eMMC clocks? Usually,
the new mode is only here for the eMMC, so we would divide the rate by
two in the non-eMMC case.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20171205/db1f3f9f/attachment.sig>
^ permalink raw reply
* [PATCH] Arm: mm: ftrace: Only set text back to ro after kernel has been marked ro
From: Russell King - ARM Linux @ 2017-12-05 20:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAGXu5jJb0PCSuC4DuOkh5-901iVLcmmPpn1G7UBq3-r9DaeMvw@mail.gmail.com>
On Tue, Dec 05, 2017 at 11:35:59AM -0800, Kees Cook wrote:
> We don't _need_ to, but they're all contiguous, so the ro_perms array
> used by set_kernel_text_*() is actually only a single entry:
>
> static struct section_perm ro_perms[] = {
> /* Make kernel code and rodata RX (set RO). */
> {
> .name = "text/rodata RO",
> .start = (unsigned long)_stext,
> .end = (unsigned long)__init_begin,
> ...
Well, they may not be contiguous - it depends on DEBUG_ALIGN_RODATA.
Either way, we have __start_rodata_section_aligned, which is either
the start of the read-only data section, or the start of the first
section beyond __start_rodata if DEBUG_ALIGN_RODATA is not set.
Given that __start_rodata_section_aligned will always be less than
__init_begin, is there any reason not to make the above end at
__start_rodata_section_aligned, thereby allowing more of the read-only
data (in the case of DEBUG_ALIGN_RODATA=n) or all of the read-only
data (in the case of DEBUG_ALIGN_RODATA=y) to remain write-protected?
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
^ permalink raw reply
* [RFC PATCH 1/6] drm: Add Content Protection property
From: Daniel Stone @ 2017-12-05 20:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171205173408.GA18425@amd>
Hi Pavel,
On 5 December 2017 at 17:34, Pavel Machek <pavel@ucw.cz> wrote:
> Yes, so... This patch makes it more likely to see machines with locked
> down kernels, preventing developers from working with systems their
> own, running hardware. That is evil, and direct threat to Free
> software movement.
>
> Users compiling their own kernels get no benefit from it. Actually it
> looks like this only benefits Intel and Disney. We don't want that.
With all due respect, you can't claim to speak for the entire kernel
and FLOSS community of users and developers.
The feature is optional: it does not enforce additional constraints on
users, but exposes additional functionality already present in
hardware, for those who wish to opt in to it. Those who wish to avoid
it can do so, by simply not making active use of it.
Cheers,
Daniel
^ permalink raw reply
* [PATCH] Arm: mm: ftrace: Only set text back to ro after kernel has been marked ro
From: Kees Cook @ 2017-12-05 20:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171205200935.GY10595@n2100.armlinux.org.uk>
On Tue, Dec 5, 2017 at 12:09 PM, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
> On Tue, Dec 05, 2017 at 11:35:59AM -0800, Kees Cook wrote:
>> We don't _need_ to, but they're all contiguous, so the ro_perms array
>> used by set_kernel_text_*() is actually only a single entry:
>>
>> static struct section_perm ro_perms[] = {
>> /* Make kernel code and rodata RX (set RO). */
>> {
>> .name = "text/rodata RO",
>> .start = (unsigned long)_stext,
>> .end = (unsigned long)__init_begin,
>> ...
>
> Well, they may not be contiguous - it depends on DEBUG_ALIGN_RODATA.
Maybe I'm picking a slightly wrong word. I guess I meant adjacent. The
range _stext to __init_begin is all read-only, though there may be
padding (controlled by DEBUG_ALIGN_RODATA), to allow a split for NX
markings on rodata.
> Either way, we have __start_rodata_section_aligned, which is either
> the start of the read-only data section, or the start of the first
> section beyond __start_rodata if DEBUG_ALIGN_RODATA is not set.
>
> Given that __start_rodata_section_aligned will always be less than
> __init_begin, is there any reason not to make the above end at
> __start_rodata_section_aligned, thereby allowing more of the read-only
> data (in the case of DEBUG_ALIGN_RODATA=n) or all of the read-only
> data (in the case of DEBUG_ALIGN_RODATA=y) to remain write-protected?
Sure, there's no reason not to split this into two entries. It'll
require some reworking of the function calls to get it right,
obviously.
-Kees
--
Kees Cook
Pixel Security
^ permalink raw reply
* [PATCH 8/8] PCIe: imx6: imx7d: add support for phy refclk source
From: Tyler Baker @ 2017-12-05 20:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAOMZO5Cxavb+iXdxH=JiWzP4VVo-ue0PU-LjBE6kD1VBPgW_0g@mail.gmail.com>
On Fri, Dec 1, 2017 at 12:21 PM, Fabio Estevam <festevam@gmail.com> wrote:
> On Thu, Nov 30, 2017 at 6:14 PM, <tyler@opensourcefoundries.com> wrote:
>> From: Tyler Baker <tyler@opensourcefoundries.com>
>>
>> In the i.MX7D the PCIe PHY can use either externel oscillator or
>> internal PLL as a reference clock source.
>> Add support for the PHY Reference Clock source including
>> device tree property phy-ref-clk.
>> External oscillator is used as a default reference clock source.
>>
>> Signed-off-by: Tyler Baker <tyler@opensourcefoundries.com>
>> Signed-off-by: Ilya Ledvich <ilya@compulab.co.il>
>
> Please submit this one to the PCI list and PCI maintainers.
Ack. Will do.
>
> As you are adding a new property you should update
> Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt.
I'll add this when I resubmit to the PCI list.
>
>> ---
>> drivers/pci/dwc/pci-imx6.c | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git adrivers/pci/dwc/pci-imx6.c b/drivers/pci/dwc/pci-imx6.c
>> index b734835..e935db4 100644
>> --- a/drivers/pci/dwc/pci-imx6.c
>> +++ b/drivers/pci/dwc/pci-imx6.c
>> @@ -45,6 +45,7 @@ enum imx6_pcie_variants {
>> struct imx6_pcie {
>> struct dw_pcie *pci;
>> int reset_gpio;
>> + u32 phy_refclk;
>
> Could this be bool instead?
>
>> bool gpio_active_high;
>> struct clk *pcie_bus;
>> struct clk *pcie_phy;
>> @@ -474,7 +475,7 @@ static void imx6_pcie_init_phy(struct imx6_pcie *imx6_pcie)
>> switch (imx6_pcie->variant) {
>> case IMX7D:
>> regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>> - IMX7D_GPR12_PCIE_PHY_REFCLK_SEL, 0);
>> + BIT(5), imx6_pcie->phy_refclk ? BIT(5) : 0);
>> break;
>> case IMX6SX:
>> regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>> @@ -733,6 +734,11 @@ static int imx6_pcie_probe(struct platform_device *pdev)
>> if (IS_ERR(pci->dbi_base))
>> return PTR_ERR(pci->dbi_base);
>>
>> + /* Fetch PHY Reference Clock */
>> + if (of_property_read_u32(node, "phy-ref-clk", &imx6_pcie->phy_refclk))
>
> You could use of_property_read_bool instead.
Thanks for the review, I'll switch to to use a bool instead.
^ permalink raw reply
* [PATCH 1/8] ARM: dts: imx7d-sbc-iot: add initial iot gateway dts
From: Tyler Baker @ 2017-12-05 20:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAOMZO5DAEMCnEoD_k8d787wk392zYMoKkiMM-TOcRco9UJL2Lw@mail.gmail.com>
On Fri, Dec 1, 2017 at 12:10 PM, Fabio Estevam <festevam@gmail.com> wrote:
> On Thu, Nov 30, 2017 at 6:14 PM, <tyler@opensourcefoundries.com> wrote:
>
>> +&ecspi3 {
>> + fsl,spi-num-chipselects = <1>;
>
> Please remove this property. It is no longer used.
Ack.
>
>
>> + dvicape at 39 {
>> + compatible = "sil164_simple";
>
> This compatible string does not exist.
I'm going to drop this, and address the display hardware it in a later series.
>
>
>
>> +&lcdif {
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_lcdif_dat
>> + &pinctrl_lcdif_ctrl>;
>> + display = <&display0>;
>> + status = "okay";
>> +
>> + display0: display {
>> + bits-per-pixel = <24>;
>> + bus-width = <24>;
>> +
>> + display-timings {
>> + native-mode = <&timing0>;
>> + timing0: dvi {
>> + /* 1024x768p60 */
>> + clock-frequency = <65000000>;
>> + hactive = <1024>;
>> + hfront-porch = <40>;
>> + hback-porch = <220>;
>> + hsync-len = <60>;
>> + vactive = <768>;
>> + vfront-porch = <7>;
>> + vback-porch = <21>;
>> + vsync-len = <10>;
>> +
>> + hsync-active = <0>;
>> + vsync-active = <0>;
>> + de-active = <1>;
>> + pixelclk-active = <0>;
>
> Which panel is this? Could you use a simple panel driver compatible
> string instead?
I'll switch to a simple panel driver and do some more testing. Will
submit a follow up series to address the LCD panel and dvi cape.
>
>
>> + };
>> + };
>> + };
>> +};
>> +
>> +&uart2 {
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_uart2>;
>> + assigned-clocks = <&clks IMX7D_UART2_ROOT_SRC>;
>> + assigned-clock-parents = <&clks IMX7D_OSC_24M_CLK>;
>> + fsl,uart-has-rtscts;
>
> Please use 'uart-has-rtscts' instead.
Ack.
>
>> + status = "okay";
>> +};
>> +
>> +&uart5 {
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_uart5>;
>> + assigned-clocks = <&clks IMX7D_UART5_ROOT_SRC>;
>> + assigned-clock-parents = <&clks IMX7D_PLL_SYS_MAIN_240M_CLK>;
>> + fsl,uart-has-rtscts;
>
> Ditto.
Ack.
>
>> + status = "okay";
>> +};
>> +
>> +&uart7 {
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_uart7>;
>> + assigned-clocks = <&clks IMX7D_UART7_ROOT_SRC>;
>> + assigned-clock-parents = <&clks IMX7D_PLL_SYS_MAIN_240M_CLK>;
>> + fsl,uart-has-rtscts;
>
> Ditto
Ack.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox