All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tomasz Figa <tomasz.figa@gmail.com>
To: Russell King - ARM Linux <linux@arm.linux.org.uk>,
	Tomasz Figa <t.figa@samsung.com>
Cc: linux-samsung-soc@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Kukjin Kim <kgene.kim@samsung.com>,
	lauraa@codeaurora.org, linux-omap@vger.kernel.org,
	linus.walleij@linaro.org, santosh.shilimkar@ti.com,
	tony@atomide.com, drake@endlessm.com,
	Marek Szyprowski <m.szyprowski@samsung.com>,
	loeliger@gmail.com
Subject: Re: [PATCH v3 1/7] ARM: l2c: Refactor the driver to use commit-like interface
Date: Sun, 03 Aug 2014 00:16:13 +0200	[thread overview]
Message-ID: <53DD632D.6070308@gmail.com> (raw)
In-Reply-To: <20140802220940.GW3705@n2100.arm.linux.org.uk>

On 03.08.2014 00:09, Russell King - ARM Linux wrote:
> On Thu, Jul 17, 2014 at 06:38:56PM +0200, Tomasz Figa wrote:
>> Certain implementations of secure hypervisors (namely the one found on
>> Samsung Exynos-based boards) do not provide access to individual L2C
>> registers. This makes the .write_sec()-based interface insufficient and
>> provoking ugly hacks.
>>
>> This patch is first step to make the driver not rely on availability of
>> writes to individual registers. This is achieved by refactoring the
>> driver to use a commit-like operation scheme: all register values are
>> prepared first and stored in an instance of l2x0_regs struct and then a
>> single callback is responsible to flush those values to the hardware.
> 
> This isn't going to work very well...
> 
>> +static const struct l2c_init_data *l2x0_data;
> 
> So you keep a pointer to the init data...
> 
>> +static void l2c_resume(void)
>> +{
>> +	l2x0_data->enable(l2x0_base, l2x0_saved_regs.aux_ctrl,
>> +				l2x0_data->num_lock);
> 
> which you dereference at resume time...
> 
>>  static const struct l2c_init_data l2c210_data __initconst = {
> 
> but the structures which get assigned to the pointer are marked __initconst.

Good catch. The code was tested on Exynos which requires the cache to be
resumed from early assembly code and so I did not hit issues caused by this.

Proposed solution: kmemdup() in init, so that only used data remain in
memory and the structures can be kept __initconst.

Best regards,
Tomasz

WARNING: multiple messages have this Message-ID (diff)
From: tomasz.figa@gmail.com (Tomasz Figa)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v3 1/7] ARM: l2c: Refactor the driver to use commit-like interface
Date: Sun, 03 Aug 2014 00:16:13 +0200	[thread overview]
Message-ID: <53DD632D.6070308@gmail.com> (raw)
In-Reply-To: <20140802220940.GW3705@n2100.arm.linux.org.uk>

On 03.08.2014 00:09, Russell King - ARM Linux wrote:
> On Thu, Jul 17, 2014 at 06:38:56PM +0200, Tomasz Figa wrote:
>> Certain implementations of secure hypervisors (namely the one found on
>> Samsung Exynos-based boards) do not provide access to individual L2C
>> registers. This makes the .write_sec()-based interface insufficient and
>> provoking ugly hacks.
>>
>> This patch is first step to make the driver not rely on availability of
>> writes to individual registers. This is achieved by refactoring the
>> driver to use a commit-like operation scheme: all register values are
>> prepared first and stored in an instance of l2x0_regs struct and then a
>> single callback is responsible to flush those values to the hardware.
> 
> This isn't going to work very well...
> 
>> +static const struct l2c_init_data *l2x0_data;
> 
> So you keep a pointer to the init data...
> 
>> +static void l2c_resume(void)
>> +{
>> +	l2x0_data->enable(l2x0_base, l2x0_saved_regs.aux_ctrl,
>> +				l2x0_data->num_lock);
> 
> which you dereference at resume time...
> 
>>  static const struct l2c_init_data l2c210_data __initconst = {
> 
> but the structures which get assigned to the pointer are marked __initconst.

Good catch. The code was tested on Exynos which requires the cache to be
resumed from early assembly code and so I did not hit issues caused by this.

Proposed solution: kmemdup() in init, so that only used data remain in
memory and the structures can be kept __initconst.

Best regards,
Tomasz

  reply	other threads:[~2014-08-02 22:16 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-17 16:38 [PATCH v3 0/7] Enable L2 cache support on Exynos4210/4x12 SoCs Tomasz Figa
2014-07-17 16:38 ` Tomasz Figa
2014-07-17 16:38 ` Tomasz Figa
2014-07-17 16:38 ` [PATCH v3 1/7] ARM: l2c: Refactor the driver to use commit-like interface Tomasz Figa
2014-07-17 16:38   ` Tomasz Figa
2014-08-02 22:09   ` Russell King - ARM Linux
2014-08-02 22:09     ` Russell King - ARM Linux
2014-08-02 22:16     ` Tomasz Figa [this message]
2014-08-02 22:16       ` Tomasz Figa
2014-07-17 16:38 ` [PATCH v3 2/7] ARM: l2c: Add interface to ask hypervisor to configure L2C Tomasz Figa
2014-07-17 16:38   ` Tomasz Figa
2014-07-17 16:38   ` Tomasz Figa
2014-07-17 16:38 ` [PATCH v3 3/7] ARM: l2c: Get outer cache .write_sec callback from mach_desc only if not NULL Tomasz Figa
2014-07-17 16:38   ` Tomasz Figa
2014-07-17 16:38   ` Tomasz Figa
2014-07-17 16:38 ` [PATCH v3 4/7] ARM: l2c: Add support for overriding prefetch settings Tomasz Figa
2014-07-17 16:38   ` Tomasz Figa
2014-07-17 16:38   ` Tomasz Figa
2014-07-17 16:39 ` [PATCH v3 5/7] ARM: EXYNOS: Add .write_sec outer cache callback for L2C-310 Tomasz Figa
2014-07-17 16:39   ` Tomasz Figa
2014-07-17 16:39 ` [PATCH v3 6/7] ARM: EXYNOS: Add support for non-secure L2X0 resume Tomasz Figa
2014-07-17 16:39   ` Tomasz Figa
2014-07-17 16:39 ` [PATCH v3 7/7] ARM: dts: exynos4: Add nodes for L2 cache controller Tomasz Figa
2014-07-17 16:39   ` Tomasz Figa
2014-07-31 19:08 ` [PATCH v3 0/7] Enable L2 cache support on Exynos4210/4x12 SoCs Tomasz Figa
2014-07-31 19:08   ` Tomasz Figa

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=53DD632D.6070308@gmail.com \
    --to=tomasz.figa@gmail.com \
    --cc=drake@endlessm.com \
    --cc=kgene.kim@samsung.com \
    --cc=lauraa@codeaurora.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=loeliger@gmail.com \
    --cc=m.szyprowski@samsung.com \
    --cc=santosh.shilimkar@ti.com \
    --cc=t.figa@samsung.com \
    --cc=tony@atomide.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.