All of lore.kernel.org
 help / color / mirror / Atom feed
From: thomas.petazzoni@free-electrons.com (Thomas Petazzoni)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v4] ARM: l2c: add options to overwrite prefetching behavior
Date: Thu, 11 Jun 2015 11:24:59 +0200	[thread overview]
Message-ID: <20150611112459.6590ab05@free-electrons.com> (raw)
In-Reply-To: <55788F6B.7060406@hauke-m.de>

Dear Hauke Mehrtens,

On Wed, 10 Jun 2015 21:26:35 +0200, Hauke Mehrtens wrote:

> Thanks for asking, I did not know Russell's patch process, but Florian
> guided me and now it is submitted, see
> http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=8391/1

I tested your patch, and I'm not sure how it can work for you, because
the changes made to the PREFETCH_CTRL register on the data/instruction
prefetch bits are later overridden by changes made to the AUX_CTRL
register.

Here is what I've found so far:

static void l2c_configure(void __iomem *base)
{
	if (outer_cache.configure) {
		outer_cache.configure(&l2x0_saved_regs);
		return;
	}

	if (l2x0_data->configure)
		l2x0_data->configure(base);

	l2c_write_sec(l2x0_saved_regs.aux_ctrl, base, L2X0_AUX_CTRL);
}

l2x0_data->configure() is what writes the PREFETCH_CTRL register with
the value determined in l2c310_of_parse() :

	if (revision >= L310_CACHE_ID_RTL_R2P0) {
		l2c_write_sec(l2x0_saved_regs.prefetch_ctrl, base,
			      L310_PREFETCH_CTRL);
	}

The value written in the L310_PREFETCH_CTRL register is correct: it
properly has bits 28/29 sets depending on prefetch-data/prefetch-instr.

However, when l2c_configure() does:

	l2c_write_sec(l2x0_saved_regs.aux_ctrl, base, L2X0_AUX_CTRL);

It writes l2x0_saved_regs.aux_ctrl to the AUX_CTRL register, which has
a "clone" of the prefetch data and prefetch instruction bits. And it
resets them to zero. I've added debug messages before/after this line,
and here is what I see:

[    0.000000]  ==> (1) prefetch is now 0x58800000
[    0.000000]  ==> (2) prefetch is now 0x48800000

I had enabled only the prefetch-data, so in step (1) (before aux_ctrl
is written to AUX_CTRL), bit 28 is correctly set to 1. However, after
AUX_CTRL is written, it's restored to 0.

How does your patch handles the fact that the prefetch data and
prefetch instr are cloned between PREFETCH_CTRL and AUX_CTRL ?

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

WARNING: multiple messages have this Message-ID (diff)
From: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
To: Hauke Mehrtens <hauke-5/S+JYg5SzeELgA04lAiVw@public.gmane.org>
Cc: linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org,
	arnd-r2nGTMty4D4@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org,
	catalin.marinas-5wv7dgnIgG8@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	"Gregory Clément"
	<gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Subject: Re: [PATCH v4] ARM: l2c: add options to overwrite prefetching behavior
Date: Thu, 11 Jun 2015 11:24:59 +0200	[thread overview]
Message-ID: <20150611112459.6590ab05@free-electrons.com> (raw)
In-Reply-To: <55788F6B.7060406-5/S+JYg5SzeELgA04lAiVw@public.gmane.org>

Dear Hauke Mehrtens,

On Wed, 10 Jun 2015 21:26:35 +0200, Hauke Mehrtens wrote:

> Thanks for asking, I did not know Russell's patch process, but Florian
> guided me and now it is submitted, see
> http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=8391/1

I tested your patch, and I'm not sure how it can work for you, because
the changes made to the PREFETCH_CTRL register on the data/instruction
prefetch bits are later overridden by changes made to the AUX_CTRL
register.

Here is what I've found so far:

static void l2c_configure(void __iomem *base)
{
	if (outer_cache.configure) {
		outer_cache.configure(&l2x0_saved_regs);
		return;
	}

	if (l2x0_data->configure)
		l2x0_data->configure(base);

	l2c_write_sec(l2x0_saved_regs.aux_ctrl, base, L2X0_AUX_CTRL);
}

l2x0_data->configure() is what writes the PREFETCH_CTRL register with
the value determined in l2c310_of_parse() :

	if (revision >= L310_CACHE_ID_RTL_R2P0) {
		l2c_write_sec(l2x0_saved_regs.prefetch_ctrl, base,
			      L310_PREFETCH_CTRL);
	}

The value written in the L310_PREFETCH_CTRL register is correct: it
properly has bits 28/29 sets depending on prefetch-data/prefetch-instr.

However, when l2c_configure() does:

	l2c_write_sec(l2x0_saved_regs.aux_ctrl, base, L2X0_AUX_CTRL);

It writes l2x0_saved_regs.aux_ctrl to the AUX_CTRL register, which has
a "clone" of the prefetch data and prefetch instruction bits. And it
resets them to zero. I've added debug messages before/after this line,
and here is what I see:

[    0.000000]  ==> (1) prefetch is now 0x58800000
[    0.000000]  ==> (2) prefetch is now 0x48800000

I had enabled only the prefetch-data, so in step (1) (before aux_ctrl
is written to AUX_CTRL), bit 28 is correctly set to 1. However, after
AUX_CTRL is written, it's restored to 0.

How does your patch handles the fact that the prefetch data and
prefetch instr are cloned between PREFETCH_CTRL and AUX_CTRL ?

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  parent reply	other threads:[~2015-06-11  9:24 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-29 23:19 [PATCH v4] ARM: l2c: add options to overwrite prefetching behavior Hauke Mehrtens
2015-05-29 23:19 ` Hauke Mehrtens
2015-06-10 15:21 ` Thomas Petazzoni
2015-06-10 15:21   ` Thomas Petazzoni
2015-06-10 19:26   ` Hauke Mehrtens
2015-06-10 19:26     ` Hauke Mehrtens
2015-06-11  7:26     ` Thomas Petazzoni
2015-06-11  7:26       ` Thomas Petazzoni
2015-06-11  9:24     ` Thomas Petazzoni [this message]
2015-06-11  9:24       ` Thomas Petazzoni
2015-06-11  9:34       ` Russell King - ARM Linux
2015-06-11  9:34         ` Russell King - ARM Linux
2015-06-11  9:39         ` Russell King - ARM Linux
2015-06-11  9:39           ` Russell King - ARM Linux
2015-06-11  9:47           ` Thomas Petazzoni
2015-06-11  9:47             ` Thomas Petazzoni
2015-06-10 16:57 ` Florian Fainelli
2015-06-10 16:57   ` Florian Fainelli

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=20150611112459.6590ab05@free-electrons.com \
    --to=thomas.petazzoni@free-electrons.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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.