* [PATCH 0/2] dts: arm64: db820c: adds support to pmic pins
From: Srinivas Kandagatla @ 2016-09-16 17:41 UTC (permalink / raw)
To: linux-arm-kernel
Hi Andy/Linus W,
This patchset adds support to pm8994 gpios/mpps on db820c board.
DB820c board Low Speed expansion GPIO_F is connected to PM8994_GPIO5
Its better if we take this patchset from single tree, as dts patch has
dependency on the dt-bindings include file.
Without this patchset GPIO_F on LS expansion would be at 2.7v
instead of 1.8v.
Thanks,
srini
Srinivas Kandagatla (2):
pinctrl: pm8994: add pad voltage regulator defines
dts: arm64: db820c: add pmic pins specific dts file
arch/arm64/boot/dts/qcom/apq8096-db820c-pmic-pins.dtsi | 15 +++++++++++++++
arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi | 2 ++
include/dt-bindings/pinctrl/qcom,pmic-gpio.h | 4 ++++
include/dt-bindings/pinctrl/qcom,pmic-mpp.h | 6 ++++++
4 files changed, 27 insertions(+)
create mode 100644 arch/arm64/boot/dts/qcom/apq8096-db820c-pmic-pins.dtsi
--
2.7.4
^ permalink raw reply
* [RFC PATCH 0/5] arm64: Signal context expansion
From: Dave Martin @ 2016-09-16 17:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <ad89f920-897c-47cb-d584-4b306bc24d1c@redhat.com>
On Fri, Sep 16, 2016 at 02:10:53PM +0200, Florian Weimer wrote:
> On 09/15/2016 06:45 PM, Dave Martin wrote:
> >On Tue, Sep 13, 2016 at 06:02:45PM +0200, Florian Weimer wrote:
> >>On 09/13/2016 05:52 PM, Dave Martin wrote:
> >>
> >>>Agreed. I'll need to think some more about how this should work in
> >>>general.
> >>
> >>Thanks.
> >>
> >>Depending on some SVE implementations details (which I know nothing about, I
> >>only saw some public overview slides), we may also need additional storage
> >>space to preserve SVE registers in the dynamic linker. Due to lazy binding,
> >>this code cn be called from a signal handler, so this needs to be factored
> >>into stack size requirements as well.
> >
> >Yes and no. The kernel SIGSTKSZ constants don't care about ld.so --
> >that's userspace overhead, not kernel overhead.
>
> Well yes, so is jmp_buf. But I think you still should consider the full
> picture. :)
>
> At least the lazy binding stack overhead can be avoided with LD_BIND_NOW=1.
I am considering the wider picture, but we're a bit out of scope for the
patch series, except that I will split out the ability to grow the
signal frame out as a separate patch.
>
> >>Problematic are register width extensions used for argument passing and
> >>callee-saved registers whose width has been extended. Both are particularly
> >>challenging to deal with if existing vector instructions clear the extension
> >>part (which may be desirable for other reasons).
> >>
> >>The size of the jmp_buf type is a concern as well.
> >
> >The default PCS for SVE will not be introducing any extra save/restore
> >requirements for SVE -- i.e., everything is caller-save at public
> >interfaces, except for the FPSIMD register bits that are already callee-
> >save under the existing PCS.
>
> Is it possible to pass arguments in the register extension parts? If
> existing non-SVE code can clobber these bits, we need some adjustments in
> libc to save and restore SVE state in some places (which may need further
> stack space).
Arguments can be passed the the extended vector registers in theory, but
this is expected only at private interfaces and within compilation units
to begin with. General-purpose libraries including libc would not be
using those bits at their interfaces, at least for now.
When calling any public interface using the default PCS, all the SVE
registers can be clobbered -- the caller must compensate for this if
it cares. So, the compensation is always in the _new_ binaries where
code gen in the compiler can take care of it automatically -- not
the current and non-SVE binaries. libc needs to update for this
specifically, so long as only the default PCS is supported for all
dynamic symbols (which I think will be the case to begin with).
A similar argument applies to all libc entry points, including
setjmp()/longjmp()/jmp_buf.
Notwithstanding this, a binary built for SVE _may_ use more stack
than a binary built without SVE support. This can always happen when
compiling with different options or even after a minor compiler or
library upgrade, so any software that doesn't already have some safety
buffer in its stack allocations is running a risk even without SVE. SVE
will probably tend to grow frames rather than shrink them, so there will
be increased pressure on stack allocations, but I don't have a good
answer about the scale of the impact at this point.
For now, I don't expect to fix everything in one go.
[...]
Cheers
---Dave
^ permalink raw reply
* Applied "ASoC: rockchip: Support headset button function on rk3399" to the asoc tree
From: Mark Brown @ 2016-09-16 17:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473981006-20542-1-git-send-email-cychiang@chromium.org>
The patch
ASoC: rockchip: Support headset button function on rk3399
has been applied to the asoc tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
>From 22b93eaf55edfd2f1fe9d1fc0d7338f582f12745 Mon Sep 17 00:00:00 2001
From: Cheng-Yi Chiang <cychiang@chromium.org>
Date: Fri, 16 Sep 2016 07:10:06 +0800
Subject: [PATCH] ASoC: rockchip: Support headset button function on rk3399
Set the mapping between button and media key event.
Signed-off-by: Cheng-Yi Chiang <cychiang@chromium.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
sound/soc/rockchip/rk3399_gru_sound.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/sound/soc/rockchip/rk3399_gru_sound.c b/sound/soc/rockchip/rk3399_gru_sound.c
index 164b6da4f2cf..ee0648927afc 100644
--- a/sound/soc/rockchip/rk3399_gru_sound.c
+++ b/sound/soc/rockchip/rk3399_gru_sound.c
@@ -23,6 +23,7 @@
#include <linux/of_gpio.h>
#include <linux/delay.h>
#include <linux/spi/spi.h>
+#include <linux/input.h>
#include <sound/core.h>
#include <sound/jack.h>
#include <sound/pcm.h>
@@ -209,6 +210,14 @@ static int rockchip_sound_da7219_init(struct snd_soc_pcm_runtime *rtd)
return ret;
}
+ snd_jack_set_key(rockchip_sound_jack.jack, SND_JACK_BTN_0, KEY_MEDIA);
+ snd_jack_set_key(
+ rockchip_sound_jack.jack, SND_JACK_BTN_1, KEY_VOLUMEUP);
+ snd_jack_set_key(
+ rockchip_sound_jack.jack, SND_JACK_BTN_2, KEY_VOLUMEDOWN);
+ snd_jack_set_key(
+ rockchip_sound_jack.jack, SND_JACK_BTN_3, KEY_VOICECOMMAND);
+
da7219_aad_jack_det(codec, &rockchip_sound_jack);
return 0;
--
2.8.1
^ permalink raw reply related
* [PATCH v5 02/16] dt/bindings: Update binding for PM domain idle states
From: Sudeep Holla @ 2016-09-16 17:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <7hpoo3ix80.fsf@baylibre.com>
Hi Kevin,
Thanks for looking at this and simplifying various discussions we had so
far. I was thinking of summarizing something very similar. I couldn't
due to lack of time.
On 16/09/16 18:13, Kevin Hilman wrote:
[...]
> I think we're having some terminology issues...
>
> FWIW, the kernel terminolgy is actually "PM domain", not power domain.
> This was intentional because the goal of the PM domain was to group
> devices that some PM features. To be very specific to the kernel, they
> us the same set of PM callbacks. Today, this is most commonly used to
> model power domains, where a group of devices share a power rail, but it
> does not need to be limited to that.
>
Agreed/Understood.
> That being said, I'm having a hard time understanding the root of the
> disagreement.
>
Yes. I tried to convey the same earlier, but have failed. The only
disagreement is about a small part of this DT bindings. We would like to
make it completely hierarchical up to CPU nodes. More comments on that
below.
> It seems that you and Sudeep would like to use domain-idle-states to
> replace/superceed cpu-idle-states with the primary goal (and benefit)
> being that it simplifies the DT bindings. Is that correct?
>
Correct, we want to deprecate cpu-idle-states with the introduction of
this hierarchical PM bindings. Yes IMO, it simplifies things and avoids
any ABI break we might trigger if we miss to consider some use-case now.
> The objections have come in because that means that implies that CPUs
> become their own domains, which may not be the case in hardware in the
> sense that they share a power rail.
>
Agreed.
> However, IMO, thinking of a CPU as it's own "PM domain" may make some
> sense based on the terminology above.
>
Thanks for that, we do understand that it may not be 100% correct when
we strictly considers hardware terminologies instead of above ones.
As along as we see no issues with the above terminologies it should be fine.
> I think the other objection may be that using a genpd to model domain
> with only a single device in it may be overkill, and I agree with that.
I too agree with that. Just because we represent that in DT in that way
doesn't mean we need to create a genpd to model domain. We can always
skip that if not required. That's pure implementation specifics and I
have tried to convey the same in my previous emails. I must say you have
summarized it very clearly in this email. Thanks again for that.
> But, I'm not sure if making CPUs use domain-idle-states implies that
> they necessarily have to use genpd is what you are proposing. Maybe
> someone could clarify that?
>
No, I have not proposing anything around implementation in the whole
discussion so far. I have constrained myself just to DT bindings so far.
That's the main reason why I was opposed to mentions of OS vs platform
co-ordinated modes of CPU suspend in this discussion. IMO that's
completely out of scope of this DT binding we are defining here.
Hope that helps/clarifies the misunderstanding/disagreement.
--
Regards,
Sudeep
^ permalink raw reply
* [PATCH] ARM: dts: msm8974: Add definitions for QCE & cryptobam
From: Iaroslav Gridin @ 2016-09-16 17:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160915211842.GB5431@hector.attlocal.net>
On Thu, Sep 15, 2016 at 04:18:42PM -0500, Andy Gross wrote:
> Actually, on thinking about this more, the bam block itself only requires the
> single clock. The peripheral it is attached to has to keep its sanity during
> the duration of the transfer (crypto). The crypto requires 3 clocks, one of
> which is the same clk the bam requires.
>
> You can access the BAM registers with the bam_clk only, correct?
Not preparing bam_clk degrades QCE performance about 3x, though.
^ permalink raw reply
* [PATCH v7 0/5] mfd: tps65218: Clean ups
From: Mark Brown @ 2016-09-16 17:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <0a30f210-0a23-0899-51b0-28f200ddd0a4@ti.com>
On Fri, Sep 16, 2016 at 09:51:38PM +0530, Keerthy wrote:
> On Friday 16 September 2016 07:01 PM, Mark Brown wrote:
> > On Fri, Sep 16, 2016 at 05:12:38PM +0530, Keerthy wrote:
> > > Should i repost this series? I do not see this series in linux-next yet.
> > Please don't send content free pings and please allow a reasonable time
> > for review. People get busy, go on holiday, attend conferences and so
> I should have been clearer.
> The last Lee Jones conveyed that:
> "I can't take this series yet, since it relies on a change which was
> taken into Mark's Regulator tree"
> https://lkml.org/lkml/2016/8/31/312
> So wanted to check if i needed to re-base/repost this series again.
> Sorry about the confusion i think that is because of $Subject goof up in the
> 0th patch of the series.
I'm afraid I've no real idea what this series is about or what patches
it might depend on, sorry. If you're waiting for me to do something
you'll need to tell me what it is but if it's a dependency that needs
cross merging then reposting this series isn't going to be needed.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160916/416ca661/attachment.sig>
^ permalink raw reply
* [PATCH v6 2/3] ARM: dts: add TOPEET itop elite based board
From: Krzysztof Kozlowski @ 2016-09-16 17:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <865b9f84-385f-fea8-2e3d-2b31e3bfe84c@soulik.info>
On Fri, Sep 16, 2016 at 10:04:29PM +0800, ayaka wrote:
> On 09/16/2016 03:23 PM, Krzysztof Kozlowski wrote:
> >On 09/07/2016 11:58 PM, Randy Li wrote:
> >>The TOPEET itop exynos 4412 have three versions base board. The
> >>Elite version is the cheap one without too much peripheral devices
> >>on it.
> >>
> >>Currently supported are serial console, wired networking(USB),
> >>USB OTG in peripheral mode, USB host, SD storage, GPIO buttons,
> >>PWM beeper, ADC and LEDs. The WM8960 analog audio codec is also
> >>enabled.
> >>
> >>The FIMC is not used for camera currently, I enabled it just for a
> >>colorspace convertor.
> >>
> >>Signed-off-by: Randy Li <ayaka@soulik.info>
> >>---
> >> .../bindings/arm/samsung/samsung-boards.txt | 3 +
> >> arch/arm/boot/dts/Makefile | 1 +
> >> arch/arm/boot/dts/exynos4412-itop-elite.dts | 239 +++++++++++++++++++++
> >> 3 files changed, 243 insertions(+)
> >> create mode 100644 arch/arm/boot/dts/exynos4412-itop-elite.dts
> >>
> >>diff --git a/Documentation/devicetree/bindings/arm/samsung/samsung-boards.txt b/Documentation/devicetree/bindings/arm/samsung/samsung-boards.txt
> >>index 0ea7f14..c7159ac 100644
> >>--- a/Documentation/devicetree/bindings/arm/samsung/samsung-boards.txt
> >>+++ b/Documentation/devicetree/bindings/arm/samsung/samsung-boards.txt
> >>@@ -22,6 +22,9 @@ Required root node properties:
> >> * FriendlyARM
> >> - "friendlyarm,tiny4412" - for Exynos4412-based FriendlyARM
> >> TINY4412 board.
> >>+ * TOPEET
> >>+ - "topeet,itop4412-elite" - for Exynos4412-based TOPEET
> >>+ Elite base board.
> >> * Google
> >> - "google,pi" - for Exynos5800-based Google Peach Pi
> >>diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> >>index 207f96f..0b39d00 100644
> >>--- a/arch/arm/boot/dts/Makefile
> >>+++ b/arch/arm/boot/dts/Makefile
> >>@@ -130,6 +130,7 @@ dtb-$(CONFIG_ARCH_EXYNOS4) += \
> >> exynos4210-smdkv310.dtb \
> >> exynos4210-trats.dtb \
> >> exynos4210-universal_c210.dtb \
> >>+ exynos4412-itop-elite.dtb \
> >> exynos4412-odroidu3.dtb \
> >> exynos4412-odroidx.dtb \
> >> exynos4412-odroidx2.dtb \
> >>diff --git a/arch/arm/boot/dts/exynos4412-itop-elite.dts b/arch/arm/boot/dts/exynos4412-itop-elite.dts
> >>new file mode 100644
> >>index 0000000..dd83689
> >>--- /dev/null
> >>+++ b/arch/arm/boot/dts/exynos4412-itop-elite.dts
> >>@@ -0,0 +1,239 @@
> >>+/*
> >>+ * TOPEET's Exynos4412 based itop board device tree source
> >>+ *
> >>+ * Copyright (c) 2016 SUMOMO Computer Association
> >>+ * https://www.sumomo.mobi
> >>+ * Randy Li <ayaka@soulik.info>
> >>+ *
> >>+ * Device tree source file for TOPEET iTop Exynos 4412 core board
> >>+ * which is based on Samsung's Exynos4412 SoC.
> >>+ *
> >>+ * 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.
> >>+*/
> >>+
> >>+/dts-v1/;
> >>+#include <dt-bindings/sound/samsung-i2s.h>
> >>+#include "exynos4412-itop-scp-core.dtsi"
> >>+
> >>+/ {
> >>+ model = "TOPEET iTop 4412 Elite board based on Exynos4412";
> >>+ compatible = "topeet,itop4412-elite", "samsung,exynos4412", "samsung,exynos4";
> >>+
> >>+ chosen {
> >>+ bootargs = "root=/dev/mmcblk0p2 rw rootfstype=ext4 rootwait";
> >>+ stdout-path = "serial2:115200n8";
> >>+ };
> >>+
> >>+ leds {
> >>+ compatible = "gpio-leds";
> >>+
> >>+ led2 {
> >>+ label = "red:system";
> >>+ gpios = <&gpx1 0 GPIO_ACTIVE_HIGH>;
> >>+ default-state = "off";
> >>+ linux,default-trigger = "heartbeat";
> >>+ };
> >>+
> >>+ led3 {
> >>+ label = "red:user";
> >>+ gpios = <&gpk1 1 GPIO_ACTIVE_HIGH>;
> >>+ default-state = "off";
> >>+ };
> >>+ };
> >>+
> >>+ gpio-keys {
> >>+ compatible = "gpio-keys";
> >>+
> >>+ home {
> >>+ label = "GPIO Key Home";
> >>+ linux,code = <KEY_HOME>;
> >>+ gpios = <&gpx1 1 GPIO_ACTIVE_LOW>;
> >>+ };
> >>+
> >>+ back {
> >>+ label = "GPIO Key Back";
> >>+ linux,code = <KEY_BACK>;
> >>+ gpios = <&gpx1 2 GPIO_ACTIVE_LOW>;
> >>+ };
> >>+
> >>+ sleep {
> >>+ label = "GPIO Key Sleep";
> >>+ linux,code = <KEY_POWER>;
> >>+ gpios = <&gpx3 3 GPIO_ACTIVE_LOW>;
> >>+ };
> >>+
> >>+ vol-up {
> >>+ label = "GPIO Key Vol+";
> >>+ linux,code = <KEY_UP>;
> >>+ gpios = <&gpx2 1 GPIO_ACTIVE_LOW>;
> >>+ };
> >>+
> >>+ vol-down {
> >>+ label = "GPIO Key Vol-";
> >>+ linux,code = <KEY_DOWN>;
> >>+ gpios = <&gpx2 0 GPIO_ACTIVE_LOW>;
> >>+ };
> >>+ };
> >>+
> >>+ sound {
> >>+ compatible = "simple-audio-card";
> >>+ simple-audio-card,name = "wm-sound";
> >>+
> >>+ assigned-clocks = <&clock_audss EXYNOS_MOUT_AUDSS>,
> >>+ <&clock_audss EXYNOS_MOUT_I2S>,
> >>+ <&clock_audss EXYNOS_DOUT_SRP>,
> >>+ <&clock_audss EXYNOS_DOUT_AUD_BUS>;
> >>+ assigned-clock-parents = <&clock CLK_FOUT_EPLL>,
> >>+ <&clock_audss EXYNOS_MOUT_AUDSS>;
> >>+ assigned-clock-rates = <0>,
> >>+ <0>,
> >>+ <112896000>,
> >>+ <11289600>;
> >>+
> >>+ simple-audio-card,format = "i2s";
> >>+ simple-audio-card,bitclock-master = <&link0_codec>;
> >>+ simple-audio-card,frame-master = <&link0_codec>;
> >>+
> >>+ simple-audio-card,widgets =
> >>+ "Microphone", "Mic Jack",
> >>+ "Line", "Line In",
> >>+ "Line", "Line Out",
> >>+ "Speaker", "Speaker",
> >>+ "Headphone", "Headphone Jack";
> >>+ simple-audio-card,routing =
> >>+ "Headphone Jack", "HP_L",
> >>+ "Headphone Jack", "HP_R",
> >>+ "Speaker", "SPK_LP",
> >>+ "Speaker", "SPK_LN",
> >>+ "Speaker", "SPK_RP",
> >>+ "Speaker", "SPK_RN",
> >>+ "LINPUT1", "Mic Jack",
> >>+ "LINPUT3", "Mic Jack",
> >>+ "RINPUT1", "Mic Jack",
> >>+ "RINPUT2", "Mic Jack";
> >>+
> >>+ simple-audio-card,cpu {
> >>+ sound-dai = <&i2s0 0>;
> >>+ };
> >>+
> >>+ link0_codec: simple-audio-card,codec {
> >>+ sound-dai = <&codec>;
> >>+ clocks = <&i2s0 CLK_I2S_CDCLK>;
> >>+ system-clock-frequency = <11289600>;
> >>+ };
> >>+ };
> >>+
> >>+ beep {
> >>+ compatible = "pwm-beeper";
> >>+ pwms = <&pwm 0 4000000 PWM_POLARITY_INVERTED>;
> >I have serious doubts that you run this code... if it does not even
> >compile. Sorry, I cannot accept code that does not compile and was not
> >tested.
> >
> >Please, test your DTS on top of current Linux tree, which would be one of:
> >1. Linus' master branch: v4.8-rc6,
> >2. one of my branches (for-next, next/dt etc),
> When I sent those patches, I tested in "Add linux-next specific files for
> 20160907"
OK, good, but please tell me why kbuild reported that build error?
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH v5 02/16] dt/bindings: Update binding for PM domain idle states
From: Kevin Hilman @ 2016-09-16 17:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <87h99i6b5d.fsf@arm.com>
Brendan Jackman <brendan.jackman@arm.com> writes:
> On Tue, Sep 13 2016 at 20:38, Lina Iyer wrote:
>> On Tue, Sep 13 2016 at 11:50 -0600, Brendan Jackman wrote:
>>>
>>>On Mon, Sep 12 2016 at 18:09, Sudeep Holla wrote:
>>>> On 12/09/16 17:16, Lina Iyer wrote:
>>>>> On Mon, Sep 12 2016 at 09:19 -0600, Brendan Jackman wrote:
>>>>>>
>>>>>> Hi Lina,
>>>>>>
>>>>>> Sorry for the delay here, Sudeep and I were both been on holiday last
>>>>>> week.
>>>>>>
>>>>>> On Fri, Sep 02 2016 at 21:16, Lina Iyer wrote:
>>>>>>> On Fri, Sep 02 2016 at 07:21 -0700, Sudeep Holla wrote:
>>>>>> [...]
>>>>>>>> This version is *not very descriptive*. Also the discussion we had
>>>>>>>> on v3
>>>>>>>> version has not yet concluded IMO. So can I take that we agreed on what
>>>>>>>> was proposed there or not ?
>>>>>>>>
>>>>>>> Sorry, this example is not very descriptive. Pls. check the 8916 dtsi
>>>>>>> for the new changes in the following patches. Let me know if that makes
>>>>>>> sense.
>>>>
>>>> Please add all possible use-cases in the bindings. Though one can refer
>>>> the usage examples, it might not cover all usage descriptions. It helps
>>>> preventing people from defining their own when they don't see examples.
>>>> Again DT bindings are like specifications, it should be descriptive
>>>> especially this kind of generic ones.
>>>>
>>>>>>
>>>>>> The not-yet-concluded discussion Sudeep is referring to is at [1].
>>>>>>
>>>>>> In that thread we initially proposed the idea of, instead of splitting
>>>>>> state phandles between cpu-idle-states and domain-idle-states, putting
>>>>>> CPUs in their own domains and using domain-idle-states for _all_
>>>>>> phandles, deprecating cpu-idle-states. I've brought this up in other
>>>>>> threads [2] but discussion keeps petering out, and neither this example
>>>>>> nor the 8916 dtsi in this patch series reflect the idea.
>>>>>>
>>>>> Brendan, while your idea is good and will work for CPUs, I do not expect
>>>>> other domains and possibly CPU domains on some architectures to follow
>>>>> this model. There is nothing that prevents you from doing this today,
>>>
>>>As I understand it your opposition to this approach is this:
>>>
>>>There may be devices/CPUs which have idle states which do not constitute
>>>"power off". If we put those devices in their own power domain for the
>>>purpose of putting their (non-power-off) idle state phandles in
>>>domain-idle-states, we are "lying" because no true power domain exists
>>>there.
>>>
>>>Am I correct that that's your opposition?
>>>
>>>If so, it seems we essentially disagree on the definition of a power
>>>domain, i.e. you define it as a set of devices that are powered on/off
>>>together while I define it as a set of devices whose power states
>>>(including idle states, not just on/off) are tied together. I said
>>>something similar on another thread [1] which died out.
>>>
>>>Do you agree that this is basically where we disagree, or am I missing
>>>something else?
>>>
>>>[2] http://www.spinics.net/lists/devicetree/msg141050.html
>>>
>> Yes, you are right, I disagree with the definition of a domain around a
>> device.
> OK, great.
>> However, as long as you don't force SoC's to define devices in
>> the CPU PM domain to have their own virtual domains, I have no problem.
>> You are welcome to define it the way you want for Juno or any other
>> platform.
> I don't think that's true; the bindings have to work the same way for
> all platforms. If for Juno we put CPU idle state phandles in a
> domain-idle-states property for per-CPU domains then, with the current
> implementation, the CPU-level idle states would be duplicated between
> cpuidle and the CPU PM domains.
>> I don't want that to be the forced and expected out of all
>> SoCs. All I am saying here is that the current implementation would
>> handle your case as well.
>
> The current implementation certainly does cover the work I want to
> do. The suggestion of per-device power domains for devices/CPUs with
> their own idle states is simply intended to minimise the binding design,
> since we'd no longer need cpu-idle-states or device-idle-states
> (the latter was proposed elsewhere).
>
> I am fine with the bindings as they are implemented currently so long
> as:
>
> - The binding doc makes clear how idle state phandles should be split
> between cpu-idle-states and domain-idle-states. It should make it
> obvious that no phandle should ever appear in both properties. It
> would even be worth briefly going over the backward-compatibility
> implications (e.g. what happens with old-kernel/new-DT and
> new-kernel/old-DT combos if a platform has OSI and PC support and we
> move cluster-level idle state phandles out of cpu-idle-states and into
> domai-idle-states).
>
> - We have a reason against the definition of power domains as "a set of
> devices bound by a common power (including idle) state", since that
> definition would simplify the bindings. In my view, "nobody thinks
> that's what a power domain is" _is_ a compelling reason, so if others
> on the list get involved I'm convinced. I think I speak for Sudeep
> here too.
I think we're having some terminology issues...
FWIW, the kernel terminolgy is actually "PM domain", not power domain.
This was intentional because the goal of the PM domain was to group
devices that some PM features. To be very specific to the kernel, they
us the same set of PM callbacks. Today, this is most commonly used to
model power domains, where a group of devices share a power rail, but it
does not need to be limited to that.
That being said, I'm having a hard time understanding the root of the
disagreement.
It seems that you and Sudeep would like to use domain-idle-states to
replace/superceed cpu-idle-states with the primary goal (and benefit)
being that it simplifies the DT bindings. Is that correct?
The objections have come in because that means that implies that CPUs
become their own domains, which may not be the case in hardware in the
sense that they share a power rail.
However, IMO, thinking of a CPU as it's own "PM domain" may make some
sense based on the terminology above.
I think the other objection may be that using a genpd to model domain
with only a single device in it may be overkill, and I agree with that.
But, I'm not sure if making CPUs use domain-idle-states implies that
they necessarily have to use genpd is what you are proposing. Maybe
someone could clarify that?
Kevin
^ permalink raw reply
* [PATCH v9 00/10] Enable PMUs in ACPI systems
From: Punit Agrawal @ 2016-09-16 17:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473892358-22574-1-git-send-email-jeremy.linton@arm.com>
Hi Jeremy,
Jeremy Linton <jeremy.linton@arm.com> writes:
> This patch expands and reworks the patches published by Mark Salter
> in order to clean up a few of the previous review comments, as well as
> add support for newer CPUs and big/little configurations.
>
> v9:
> Add/cleanup an additional hotplug patch I've had sitting around. This
> patch brings the ACPI PMU mostly on par with the DT functionality with
> respect to having CPUs offline during boot. This should help clarify
> some of the code structuring.
> Cleanup the list of PMU types early if we fail to allocate memory for an
> additional pmu type.
The earlier patches in the series look OK but there are still
unaddressed comments on Patch 8 from previous versions - coding style,
excessive indentation, complicated logic etc.
Additionally, I'd suggest moving hotplug/unsed MADT related changes out
of Patch 8 to later patches, i.e., add support for hotplug on top.
Thanks,
Punit
>
> v8:
> Rebase to 4.8rc4
> Assorted minor comment/hunk placement/etc tweaks per Punit Agrawal
>
> v7:
> Rebase to 4.8rc3
> Remove cpu affinity sysfs entry. While providing a CPU mask for
> ARMv8 PMU's is really helpful in big/little environments, reworking
> the PMU code to support the cpumask attribute for !arm64 PMUs is out
> of the scope of this patch set.
> Fix CPU miscount problem where an alloc failure followed by successfully
> allocating the structure can result in under counting the CPUs associated
> with the PMU. This bug was created in v6 with the conversion to a linked
> list.
> Remove initial platform device creation code by Mark Salter, and re-squash
> multiple platform device creation code together with helper routines.
> Other minor tweakage.
>
> v6:
> Added cpu affinity sysfs entry
> Converted pmu_types array, to linked list
> Restrict use of the armv8_pmu_probe_table to ACPI systems
> Rename MADT parsing routines in smp.c
> Convert sysfs PMU name to use index rather than partnum
> Remove pr_devel statements
> Other Minor cleanups
> Add Partial Ack-by Will Deacon
>
> v5:
> Remove list of CPU types for ACPI systems. We now match a generic
> event list, and use the PMCIED[01] to select events which exist on
> the given PMU. This avoids the need to update the kernel every time
> a new CPU is released.
> Update the maintainers list to include the new file.
>
> v4:
> Correct build issues with ARM (!ARM64) kernels.
> Add ThunderX to list of PMU types.
>
> v3:
> Enable ARM performance monitoring units on ACPI/arm64 machines.
>
> Jeremy Linton (8):
> arm64: pmu: Probe default hw/cache counters
> arm64: pmu: Hoist pmu platform device name
> arm64: Rename the common MADT parse routine
> arm: arm64: Add routine to determine cpuid of other cpus
> arm: arm64: pmu: Assign platform PMU CPU affinity
> arm64: pmu: Detect and enable multiple PMUs in an ACPI system
> arm: pmu: Add PMU definitions for hot-plugged CPUs
> MAINTAINERS: Tweak ARM PMU maintainers
>
> Mark Salter (2):
> arm64: pmu: add fallback probe table
> arm64: pmu: Cache PMU interrupt numbers from MADT parse
>
> MAINTAINERS | 3 +-
> arch/arm/include/asm/cputype.h | 2 +
> arch/arm64/include/asm/cputype.h | 3 +
> arch/arm64/kernel/perf_event.c | 58 ++++++++-
> arch/arm64/kernel/smp.c | 18 ++-
> drivers/perf/Kconfig | 4 +
> drivers/perf/Makefile | 1 +
> drivers/perf/arm_pmu.c | 110 ++++++++++++++--
> drivers/perf/arm_pmu_acpi.c | 264 +++++++++++++++++++++++++++++++++++++++
> include/linux/perf/arm_pmu.h | 13 ++
> 10 files changed, 452 insertions(+), 24 deletions(-)
> create mode 100644 drivers/perf/arm_pmu_acpi.c
^ permalink raw reply
* [PATCH v3 1/3] tty: amba-pl011: define flag register bits for ZTE device
From: Timur Tabi @ 2016-09-16 17:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160916163952.GH1041@n2100.armlinux.org.uk>
Russell King - ARM Linux wrote:
> Maybe what we should've done with ZTE is insisted that they implement
> a complete new driver, rather than trying to shoe-horn it into PL011
> even though it is in theory PL011.
When I suggested that a year ago, I was shot down:
http://www.spinics.net/lists/arm-kernel/msg455888.html
--
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 v9 08/10] arm64: pmu: Detect and enable multiple PMUs in an ACPI system
From: Punit Agrawal @ 2016-09-16 17:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <6e0b08db-2e13-be49-5208-c396bf51fe68@arm.com>
Jeremy Linton <jeremy.linton@arm.com> writes:
> Hi,
>
> On 09/16/2016 08:33 AM, Punit Agrawal wrote:
>> Jeremy Linton <jeremy.linton@arm.com> writes:
>>
>>> Its possible that an ACPI system has multiple CPU types in it
>>> with differing PMU counters. Iterate the CPU's and make a determination
>>> about how many of each type exist in the system. Then take and create
>>> a PMU platform device for each type, and assign it the interrupts parsed
>>> from the MADT. Creating a platform device is necessary because the PMUs
>>> are not described as devices in the DSDT table.
>>>
>>> This code is loosely based on earlier work by Mark Salter.
>
> (trimming)
>
>>> +
>>> + list_for_each_entry_safe(pmu, safe_temp, &pmus, list) {
>>> + if (unused_madt_entries)
>>> + pmu->cpu_count = num_possible_cpus();
>>
>> So if there is any unbooted cpu ...
>>
>>> +
>>> + res = kcalloc(pmu->cpu_count,
>>> + sizeof(struct resource), GFP_KERNEL);
>>
>> ... we allocate potentially large number (num_possible_cpus()) of
>> resources for each PMU.
>>
>> This is needlessly wasteful. Under what conditions have you found
>> reg_midr to be 0?
>
> Unused MADT entries, in place for potentially unbooted/hotplug
> CPUs.
Is linux able to deal with booting secondaries that have unused MADT
entries?
> In those cases you don't know for sure which PMU the CPU belongs
> to until it comes online and the MIDR can be read. I'm open to
> suggestions on how to deal with this, outside of pushing my luck and
> further breaking the platform device encapsulation by trying to
> reallocate the resource structure while its active. Besides its only
> wasteful for ACPI+big.little, which at the moment only applies to a
> development platform.
I don't have any ideas to solve this problem, but in the interest of
helping review, please move all the changes arising from hotplug/unused
MADT entries in this patch to the next one.
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH] coresight: tmc: Cleanup operation mode handling
From: Mathieu Poirier @ 2016-09-16 17:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473861230-5666-1-git-send-email-suzuki.poulose@arm.com>
On 14 September 2016 at 07:53, Suzuki K Poulose <suzuki.poulose@arm.com> wrote:
> The mode of operation of the TMC tracked in drvdata->mode is defined
> as a local_t type. This is always checked and modified under the
> drvdata->spinlock and hence we don't need local_t for it and the
> unnecessary synchronisation instructions that comes with it. This
> change makes the code a bit more cleaner.
>
> Also fixes the order in which we update the drvdata->mode to
> CS_MODE_DISABLED. i.e, in tmc_disable_etX_sink we change the
> mode to CS_MODE_DISABLED before invoking tmc_disable_etX_hw()
> which in turn depends on the mode to decide whether to dump the
> trace to a buffer.
Thank you for the patch - just a few comments below.
Regards,
Mathieu
>
> Applies on mathieu's coresight/next tree [1]
>
> https://git.linaro.org/kernel/coresight.git next
>
> Reported-by: Venkatesh Vivekanandan <venkatesh.vivekanandan@broadcom.com>
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
> drivers/hwtracing/coresight/coresight-tmc-etf.c | 32 +++++++++++--------------
> drivers/hwtracing/coresight/coresight-tmc-etr.c | 30 ++++++++++-------------
> drivers/hwtracing/coresight/coresight-tmc.h | 2 +-
> 3 files changed, 28 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c
> index d6941ea..c51ce45 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
> @@ -70,7 +70,7 @@ static void tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
> * When operating in sysFS mode the content of the buffer needs to be
> * read before the TMC is disabled.
> */
> - if (local_read(&drvdata->mode) == CS_MODE_SYSFS)
> + if (drvdata->mode == CS_MODE_SYSFS)
> tmc_etb_dump_hw(drvdata);
> tmc_disable_hw(drvdata);
>
> @@ -108,7 +108,6 @@ static int tmc_enable_etf_sink_sysfs(struct coresight_device *csdev, u32 mode)
> int ret = 0;
> bool used = false;
> char *buf = NULL;
> - long val;
> unsigned long flags;
> struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>
> @@ -138,13 +137,12 @@ static int tmc_enable_etf_sink_sysfs(struct coresight_device *csdev, u32 mode)
> goto out;
> }
>
> - val = local_xchg(&drvdata->mode, mode);
> /*
> * In sysFS mode we can have multiple writers per sink. Since this
> * sink is already enabled no memory is needed and the HW need not be
> * touched.
> */
> - if (val == CS_MODE_SYSFS)
> + if (drvdata->mode == CS_MODE_SYSFS)
> goto out;
>
> /*
> @@ -163,6 +161,7 @@ static int tmc_enable_etf_sink_sysfs(struct coresight_device *csdev, u32 mode)
> drvdata->buf = buf;
> }
>
> + drvdata->mode = CS_MODE_SYSFS;
> tmc_etb_enable_hw(drvdata);
> out:
> spin_unlock_irqrestore(&drvdata->spinlock, flags);
> @@ -180,7 +179,6 @@ out:
> static int tmc_enable_etf_sink_perf(struct coresight_device *csdev, u32 mode)
> {
> int ret = 0;
> - long val;
> unsigned long flags;
> struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>
> @@ -194,17 +192,17 @@ static int tmc_enable_etf_sink_perf(struct coresight_device *csdev, u32 mode)
> goto out;
> }
>
> - val = local_xchg(&drvdata->mode, mode);
> /*
> * In Perf mode there can be only one writer per sink. There
> * is also no need to continue if the ETB/ETR is already operated
> * from sysFS.
> */
> - if (val != CS_MODE_DISABLED) {
> + if (drvdata->mode != CS_MODE_DISABLED) {
> ret = -EINVAL;
> goto out;
> }
>
> + drvdata->mode = mode;
Given the way tmc_enable_etf_sink_perf() is called in
tmc_enable_etf_sink(), I think it is time to get rid of the 'mode'
parameter - it doesn't do anything nowadays. Same thing for
tmc_enable_etf_sink_sysfs() and ETR.
> tmc_etb_enable_hw(drvdata);
> out:
> spin_unlock_irqrestore(&drvdata->spinlock, flags);
> @@ -227,7 +225,6 @@ static int tmc_enable_etf_sink(struct coresight_device *csdev, u32 mode)
>
> static void tmc_disable_etf_sink(struct coresight_device *csdev)
> {
> - long val;
> unsigned long flags;
> struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>
> @@ -237,10 +234,11 @@ static void tmc_disable_etf_sink(struct coresight_device *csdev)
> return;
> }
>
> - val = local_xchg(&drvdata->mode, CS_MODE_DISABLED);
> /* Disable the TMC only if it needs to */
> - if (val != CS_MODE_DISABLED)
> + if (drvdata->mode != CS_MODE_DISABLED) {
> tmc_etb_disable_hw(drvdata);
> + drvdata->mode = CS_MODE_DISABLED;
> + }
>
> spin_unlock_irqrestore(&drvdata->spinlock, flags);
>
> @@ -260,7 +258,7 @@ static int tmc_enable_etf_link(struct coresight_device *csdev,
> }
>
> tmc_etf_enable_hw(drvdata);
> - local_set(&drvdata->mode, CS_MODE_SYSFS);
> + drvdata->mode = CS_MODE_SYSFS;
> spin_unlock_irqrestore(&drvdata->spinlock, flags);
>
> dev_info(drvdata->dev, "TMC-ETF enabled\n");
> @@ -279,8 +277,8 @@ static void tmc_disable_etf_link(struct coresight_device *csdev,
> return;
> }
>
> + drvdata->mode = CS_MODE_DISABLED;
> tmc_etf_disable_hw(drvdata);
> - local_set(&drvdata->mode, CS_MODE_DISABLED);
I think setting the mode should come after tmc_etf_disable_hw(), as it
was before.
> spin_unlock_irqrestore(&drvdata->spinlock, flags);
>
> dev_info(drvdata->dev, "TMC disabled\n");
> @@ -383,7 +381,7 @@ static void tmc_update_etf_buffer(struct coresight_device *csdev,
> return;
>
> /* This shouldn't happen */
> - if (WARN_ON_ONCE(local_read(&drvdata->mode) != CS_MODE_PERF))
> + if (WARN_ON_ONCE(drvdata->mode != CS_MODE_PERF))
> return;
>
> CS_UNLOCK(drvdata->base);
> @@ -504,7 +502,6 @@ const struct coresight_ops tmc_etf_cs_ops = {
>
> int tmc_read_prepare_etb(struct tmc_drvdata *drvdata)
> {
> - long val;
> enum tmc_mode mode;
> int ret = 0;
> unsigned long flags;
> @@ -528,9 +525,8 @@ int tmc_read_prepare_etb(struct tmc_drvdata *drvdata)
> goto out;
> }
>
> - val = local_read(&drvdata->mode);
> /* Don't interfere if operated from Perf */
> - if (val == CS_MODE_PERF) {
> + if (drvdata->mode == CS_MODE_PERF) {
> ret = -EINVAL;
> goto out;
> }
> @@ -542,7 +538,7 @@ int tmc_read_prepare_etb(struct tmc_drvdata *drvdata)
> }
>
> /* Disable the TMC if need be */
> - if (val == CS_MODE_SYSFS)
> + if (drvdata->mode == CS_MODE_SYSFS)
> tmc_etb_disable_hw(drvdata);
>
> drvdata->reading = true;
> @@ -573,7 +569,7 @@ int tmc_read_unprepare_etb(struct tmc_drvdata *drvdata)
> }
>
> /* Re-enable the TMC if need be */
> - if (local_read(&drvdata->mode) == CS_MODE_SYSFS) {
> + if (drvdata->mode == CS_MODE_SYSFS) {
> /*
> * The trace run will continue with the same allocated trace
> * buffer. As such zero-out the buffer so that we don't end
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> index 886ea83..cf2bf60 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> @@ -86,7 +86,7 @@ static void tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
> * When operating in sysFS mode the content of the buffer needs to be
> * read before the TMC is disabled.
> */
> - if (local_read(&drvdata->mode) == CS_MODE_SYSFS)
> + if (drvdata->mode == CS_MODE_SYSFS)
> tmc_etr_dump_hw(drvdata);
> tmc_disable_hw(drvdata);
>
> @@ -97,7 +97,6 @@ static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev, u32 mode)
> {
> int ret = 0;
> bool used = false;
> - long val;
> unsigned long flags;
> void __iomem *vaddr = NULL;
> dma_addr_t paddr;
> @@ -134,13 +133,12 @@ static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev, u32 mode)
> goto out;
> }
>
> - val = local_xchg(&drvdata->mode, mode);
> /*
> * In sysFS mode we can have multiple writers per sink. Since this
> * sink is already enabled no memory is needed and the HW need not be
> * touched.
> */
> - if (val == CS_MODE_SYSFS)
> + if (drvdata->mode == CS_MODE_SYSFS)
> goto out;
>
> /*
> @@ -157,6 +155,7 @@ static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev, u32 mode)
>
> memset(drvdata->vaddr, 0, drvdata->size);
>
> + drvdata->mode = CS_MODE_SYSFS;
> tmc_etr_enable_hw(drvdata);
> out:
> spin_unlock_irqrestore(&drvdata->spinlock, flags);
> @@ -174,7 +173,6 @@ out:
> static int tmc_enable_etr_sink_perf(struct coresight_device *csdev, u32 mode)
> {
> int ret = 0;
> - long val;
> unsigned long flags;
> struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>
> @@ -188,18 +186,18 @@ static int tmc_enable_etr_sink_perf(struct coresight_device *csdev, u32 mode)
> goto out;
> }
>
> - val = local_xchg(&drvdata->mode, mode);
> /*
> * In Perf mode there can be only one writer per sink. There
> * is also no need to continue if the ETR is already operated
> * from sysFS.
> */
> - if (val != CS_MODE_DISABLED) {
> + if (drvdata->mode == CS_MODE_DISABLED) {
> + drvdata->mode = CS_MODE_PERF;
> + tmc_etr_enable_hw(drvdata);
> + } else {
> ret = -EINVAL;
> - goto out;
> }
>
> - tmc_etr_enable_hw(drvdata);
Is there a reason for not proceeding the same way as in
tmc_enable_etr_sink_perf()? I thought that was a better approach.
> out:
> spin_unlock_irqrestore(&drvdata->spinlock, flags);
>
> @@ -221,7 +219,6 @@ static int tmc_enable_etr_sink(struct coresight_device *csdev, u32 mode)
>
> static void tmc_disable_etr_sink(struct coresight_device *csdev)
> {
> - long val;
> unsigned long flags;
> struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>
> @@ -231,10 +228,11 @@ static void tmc_disable_etr_sink(struct coresight_device *csdev)
> return;
> }
>
> - val = local_xchg(&drvdata->mode, CS_MODE_DISABLED);
> /* Disable the TMC only if it needs to */
> - if (val != CS_MODE_DISABLED)
> + if (drvdata->mode != CS_MODE_DISABLED) {
> tmc_etr_disable_hw(drvdata);
> + drvdata->mode = CS_MODE_DISABLED;
> + }
>
> spin_unlock_irqrestore(&drvdata->spinlock, flags);
>
> @@ -253,7 +251,6 @@ const struct coresight_ops tmc_etr_cs_ops = {
> int tmc_read_prepare_etr(struct tmc_drvdata *drvdata)
> {
> int ret = 0;
> - long val;
> unsigned long flags;
>
> /* config types are set a boot time and never change */
> @@ -266,9 +263,8 @@ int tmc_read_prepare_etr(struct tmc_drvdata *drvdata)
> goto out;
> }
>
> - val = local_read(&drvdata->mode);
> /* Don't interfere if operated from Perf */
> - if (val == CS_MODE_PERF) {
> + if (drvdata->mode == CS_MODE_PERF) {
> ret = -EINVAL;
> goto out;
> }
> @@ -280,7 +276,7 @@ int tmc_read_prepare_etr(struct tmc_drvdata *drvdata)
> }
>
> /* Disable the TMC if need be */
> - if (val == CS_MODE_SYSFS)
> + if (drvdata->mode == CS_MODE_SYSFS)
> tmc_etr_disable_hw(drvdata);
>
> drvdata->reading = true;
> @@ -303,7 +299,7 @@ int tmc_read_unprepare_etr(struct tmc_drvdata *drvdata)
> spin_lock_irqsave(&drvdata->spinlock, flags);
>
> /* RE-enable the TMC if need be */
> - if (local_read(&drvdata->mode) == CS_MODE_SYSFS) {
> + if (drvdata->mode == CS_MODE_SYSFS) {
> /*
> * The trace run will continue with the same allocated trace
> * buffer. The trace buffer is cleared in tmc_etr_enable_hw(),
> diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
> index 44b3ae3..51c0185 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc.h
> +++ b/drivers/hwtracing/coresight/coresight-tmc.h
> @@ -117,7 +117,7 @@ struct tmc_drvdata {
> void __iomem *vaddr;
> u32 size;
> u32 len;
> - local_t mode;
> + u32 mode;
> enum tmc_config_type config_type;
> enum tmc_mem_intf_width memwidth;
> u32 trigger_cntr;
> --
> 2.7.4
>
^ permalink raw reply
* [PATCH 5/6] arm/arm64: vgic-new: Implement VGICv3 CPU interface access
From: Marc Zyngier @ 2016-09-16 17:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CALicx6u-uYLY3DtDGMh3n6NbNH3kD9fn9aYHHJC=LVUQfFjTsQ@mail.gmail.com>
On 16/09/16 17:57, Vijay Kilari wrote:
> On Fri, Sep 16, 2016 at 8:06 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
>> On 16/09/16 13:20, vijay.kilari at gmail.com wrote:
>>> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>>>
>>> VGICv3 CPU interface registers are accessed using
>>> KVM_DEV_ARM_VGIC_CPU_SYSREGS ioctl. These registers are accessed
>>> as 64-bit. The cpu MPIDR value is passed along with register id.
>>> is used to identify the cpu for registers access.
>>>
>>> The version of VGIC v3 specification is define here
>>> http://lists.infradead.org/pipermail/linux-arm-kernel/2016-July/445611.html
>>>
>>> Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
>>> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>>> ---
>>> arch/arm64/include/uapi/asm/kvm.h | 3 +
>>> arch/arm64/kvm/Makefile | 1 +
>>> include/linux/irqchip/arm-gic-v3.h | 30 ++++
>>> virt/kvm/arm/vgic/vgic-kvm-device.c | 27 ++++
>>> virt/kvm/arm/vgic/vgic-mmio-v3.c | 18 +++
>>> virt/kvm/arm/vgic/vgic-sys-reg-v3.c | 296 ++++++++++++++++++++++++++++++++++++
>>> virt/kvm/arm/vgic/vgic.h | 10 ++
>>> 7 files changed, 385 insertions(+)
[...]
>>> diff --git a/virt/kvm/arm/vgic/vgic-sys-reg-v3.c b/virt/kvm/arm/vgic/vgic-sys-reg-v3.c
>>> new file mode 100644
>>> index 0000000..8e4f403
>>> --- /dev/null
>>> +++ b/virt/kvm/arm/vgic/vgic-sys-reg-v3.c
>>> @@ -0,0 +1,296 @@
>>> +#include <linux/irqchip/arm-gic-v3.h>
>>> +#include <linux/kvm.h>
>>> +#include <linux/kvm_host.h>
>>> +#include <kvm/iodev.h>
>>> +#include <kvm/arm_vgic.h>
>>> +#include <asm/kvm_emulate.h>
>>> +#include <asm/kvm_arm.h>
>>> +#include <asm/kvm_mmu.h>
>>> +
>>> +#include "vgic.h"
>>> +#include "vgic-mmio.h"
>>> +#include "sys_regs.h"
>>> +
>>> +static bool access_gic_ctlr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>>> + const struct sys_reg_desc *r)
>>> +{
>>> + struct vgic_vmcr vmcr;
>>> + u64 val;
>>> + u32 ich_vtr;
>>> +
>>> + vgic_get_vmcr(vcpu, &vmcr);
>>> + if (p->is_write) {
>>> + val = p->regval;
>>> + vmcr.ctlr &= ~(ICH_VMCR_CBPR_MASK | ICH_VMCR_EOIM_MASK);
>>> + vmcr.ctlr |= ((val & ICC_CTLR_EL1_CBPR_MASK) >>
>>> + ICC_CTLR_EL1_CBPR_SHIFT) << ICH_VMCR_CBPR_SHIFT;
>>> + vmcr.ctlr |= ((val & ICC_CTLR_EL1_EOImode_MASK) >>
>>> + ICC_CTLR_EL1_EOImode_SHIFT) << ICH_VMCR_EOIM_SHIFT;
>>> + vgic_set_vmcr(vcpu, &vmcr);
>>
>> You've ignored my comments again: "What if userspace writes something
>> that is incompatible with the current configuration? Wrong number of ID
>> bits, or number of priorities?"
>
> IMO, In case of incompatibility,
> If ID bits and PRI bits are less than HW supported, it is ok.
Yes. But you also need to track of what the guest has programmed in
order to be able to migrate it back to its original configuration.
> If ID bits and PRI bits are greater than HW supported, then warn would be good
> enough. Please suggest the behaviour that you think it should be.
No, it is an error, plain and simple. You cannot run in this condition.
>
>>
>>> + } else {
>>> + ich_vtr = kvm_call_hyp(__vgic_v3_get_ich_vtr_el2);
>>> +
>>> + val = 0;
>>> + val |= ((ich_vtr & ICH_VTR_PRI_BITS_MASK) >>
>>> + ICH_VTR_PRI_BITS_SHIFT) << ICC_CTLR_EL1_PRI_BITS_SHIFT;
>>> + val |= ((ich_vtr & ICH_VTR_ID_BITS_MASK) >>
>>> + ICH_VTR_ID_BITS_SHIFT) << ICC_CTLR_EL1_ID_BITS_SHIFT;
>>> + val |= ((ich_vtr & ICH_VTR_SEIS_MASK) >> ICH_VTR_SEIS_SHIFT)
>>> + << ICC_CTLR_EL1_SEIS_SHIFT;
>>> + val |= ((ich_vtr & ICH_VTR_A3V_MASK) >> ICH_VTR_A3V_SHIFT)
>>> + << ICC_CTLR_EL1_A3V_SHIFT;
>>> + val |= ((vmcr.ctlr & ICH_VMCR_CBPR_MASK) >>
>>> + ICH_VMCR_CBPR_SHIFT) << ICC_CTLR_EL1_CBPR_SHIFT;
>>> + val |= ((vmcr.ctlr & ICH_VMCR_EOIM_MASK) >>
>>> + ICH_VMCR_EOIM_SHIFT) << ICC_CTLR_EL1_EOImode_SHIFT;
>>> +
>>> + p->regval = val;
>>> + }
>>> +
>>> + return true;
>>> +}
>>> +
>>> +static bool access_gic_pmr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>>> + const struct sys_reg_desc *r)
>>> +{
>>> + struct vgic_vmcr vmcr;
>>> +
>>> + vgic_get_vmcr(vcpu, &vmcr);
>>> + if (p->is_write) {
>>> + vmcr.pmr = (p->regval & ICC_PMR_EL1_MASK) >> ICC_PMR_EL1_SHIFT;
>>> + vgic_set_vmcr(vcpu, &vmcr);
>>> + } else {
>>> + p->regval = (vmcr.pmr << ICC_PMR_EL1_SHIFT) & ICC_PMR_EL1_MASK;
>>> + }
>>> +
>>> + return true;
>>> +}
>>> +
>>> +static bool access_gic_bpr0(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>>> + const struct sys_reg_desc *r)
>>> +{
>>> + struct vgic_vmcr vmcr;
>>> +
>>> + vgic_get_vmcr(vcpu, &vmcr);
>>> + if (p->is_write) {
>>> + vmcr.bpr = (p->regval & ICC_BPR0_EL1_MASK) >>
>>> + ICC_BPR0_EL1_SHIFT;
>>> + vgic_set_vmcr(vcpu, &vmcr);
>>> + } else {
>>> + p->regval = (vmcr.bpr << ICC_BPR0_EL1_SHIFT) &
>>> + ICC_BPR0_EL1_MASK;
>>> + }
>>> +
>>> + return true;
>>> +}
>>> +
>>> +static bool access_gic_bpr1(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>>> + const struct sys_reg_desc *r)
>>> +{
>>> + struct vgic_vmcr vmcr;
>>> +
>>> + vgic_get_vmcr(vcpu, &vmcr);
>>> + if (p->is_write) {
>>> + vmcr.abpr = (p->regval & ICC_BPR1_EL1_MASK) >>
>>> + ICC_BPR1_EL1_SHIFT;
>>> + vgic_set_vmcr(vcpu, &vmcr);
>>> + } else {
>>> + p->regval = (vmcr.abpr << ICC_BPR1_EL1_SHIFT) &
>>> + ICC_BPR1_EL1_MASK;
>>> + }
>>> +
>>> + return true;
>>> +}
>>> +
>>> +static bool access_gic_grpen0(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>>> + const struct sys_reg_desc *r)
>>> +{
>>> + struct vgic_vmcr vmcr;
>>> +
>>> + vgic_get_vmcr(vcpu, &vmcr);
>>> + if (p->is_write) {
>>> + vmcr.grpen0 = (p->regval & ICC_IGRPEN0_EL1_MASK) >>
>>> + ICC_IGRPEN0_EL1_SHIFT;
>>> + vgic_set_vmcr(vcpu, &vmcr);
>>> + } else {
>>> + p->regval = (vmcr.grpen0 << ICC_IGRPEN0_EL1_SHIFT) &
>>> + ICC_IGRPEN0_EL1_MASK;
>>> + }
>>> +
>>> + return true;
>>> +}
>>> +
>>> +static bool access_gic_grpen1(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>>> + const struct sys_reg_desc *r)
>>> +{
>>> + struct vgic_vmcr vmcr;
>>> +
>>> + vgic_get_vmcr(vcpu, &vmcr);
>>> + if (p->is_write) {
>>> + vmcr.grpen1 = (p->regval & ICC_IGRPEN1_EL1_MASK) >>
>>> + ICC_IGRPEN1_EL1_SHIFT;
>>> + vgic_set_vmcr(vcpu, &vmcr);
>>> + } else {
>>> + p->regval = (vmcr.grpen1 << ICC_IGRPEN1_EL1_SHIFT) &
>>> + ICC_IGRPEN1_EL1_MASK;
>>
>> From the previous review comments: "Shouldn't this account for the
>> ICC_CTLR_EL1.CBPR setting?"
>
> Ok. I think this comment is for ICC_BPR1_EL1 access.
Yes, sorry about the misplaced comment.
> I will make a check on ICC_CTLR.EL1.CBPR for accessing ICC_BPR1_EL1.
The reverse is also true: you also need to account the value of
ICC_BPR1_EL1 when accessing ICC_CTLR_EL1.
>>
>>> + }
>>> +
>>> + return true;
>>> +}
>>> +
>>> +static void vgic_v3_access_apr_reg(struct kvm_vcpu *vcpu,
>>> + struct sys_reg_params *p, u8 apr, u8 idx)
>>> +{
>>> + struct vgic_v3_cpu_if *vgicv3 = &vcpu->arch.vgic_cpu.vgic_v3;
>>> + uint32_t *ap_reg;
>>> +
>>> + if (apr)
>>> + ap_reg = &vgicv3->vgic_ap1r[idx];
>>> + else
>>> + ap_reg = &vgicv3->vgic_ap0r[idx];
>>> +
>>> + if (p->is_write)
>>> + *ap_reg = p->regval;
>>> + else
>>> + p->regval = *ap_reg;
>>> +}
>>> +
>>> +static void access_gic_aprn(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>>> + const struct sys_reg_desc *r, u8 apr)
>>> +{
>>> + u8 num_pri_bits, idx;
>>> + u32 ich_vtr = kvm_call_hyp(__vgic_v3_get_ich_vtr_el2);
>>
>> Maybe you should cache this once and for all? It is fairly unlikely to
>> change over time...
>
> Is it ok to cache in vgic_global struct?
I can't see why not.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* [PATCH 5/6] arm/arm64: vgic-new: Implement VGICv3 CPU interface access
From: Vijay Kilari @ 2016-09-16 16:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <57DC0366.8060304@arm.com>
On Fri, Sep 16, 2016 at 8:06 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On 16/09/16 13:20, vijay.kilari at gmail.com wrote:
>> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>>
>> VGICv3 CPU interface registers are accessed using
>> KVM_DEV_ARM_VGIC_CPU_SYSREGS ioctl. These registers are accessed
>> as 64-bit. The cpu MPIDR value is passed along with register id.
>> is used to identify the cpu for registers access.
>>
>> The version of VGIC v3 specification is define here
>> http://lists.infradead.org/pipermail/linux-arm-kernel/2016-July/445611.html
>>
>> Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
>> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>> ---
>> arch/arm64/include/uapi/asm/kvm.h | 3 +
>> arch/arm64/kvm/Makefile | 1 +
>> include/linux/irqchip/arm-gic-v3.h | 30 ++++
>> virt/kvm/arm/vgic/vgic-kvm-device.c | 27 ++++
>> virt/kvm/arm/vgic/vgic-mmio-v3.c | 18 +++
>> virt/kvm/arm/vgic/vgic-sys-reg-v3.c | 296 ++++++++++++++++++++++++++++++++++++
>> virt/kvm/arm/vgic/vgic.h | 10 ++
>> 7 files changed, 385 insertions(+)
>>
>> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
>> index 56dc08d..91c7137 100644
>> --- a/arch/arm64/include/uapi/asm/kvm.h
>> +++ b/arch/arm64/include/uapi/asm/kvm.h
>> @@ -206,9 +206,12 @@ struct kvm_arch_memory_slot {
>> (0xffffffffULL << KVM_DEV_ARM_VGIC_V3_MPIDR_SHIFT)
>> #define KVM_DEV_ARM_VGIC_OFFSET_SHIFT 0
>> #define KVM_DEV_ARM_VGIC_OFFSET_MASK (0xffffffffULL << KVM_DEV_ARM_VGIC_OFFSET_SHIFT)
>> +#define KVM_DEV_ARM_VGIC_SYSREG_INSTR_MASK (0xffff)
>> #define KVM_DEV_ARM_VGIC_GRP_NR_IRQS 3
>> #define KVM_DEV_ARM_VGIC_GRP_CTRL 4
>> #define KVM_DEV_ARM_VGIC_GRP_REDIST_REGS 5
>> +#define KVM_DEV_ARM_VGIC_CPU_SYSREGS 6
>> +
>> #define KVM_DEV_ARM_VGIC_CTRL_INIT 0
>>
>> /* Device Control API on vcpu fd */
>> diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
>> index d50a82a..1a14e29 100644
>> --- a/arch/arm64/kvm/Makefile
>> +++ b/arch/arm64/kvm/Makefile
>> @@ -32,5 +32,6 @@ kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-mmio-v3.o
>> kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-kvm-device.o
>> kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-its.o
>> kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/irqchip.o
>> +kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-sys-reg-v3.o
>> kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/arch_timer.o
>> kvm-$(CONFIG_KVM_ARM_PMU) += $(KVM)/arm/pmu.o
>> diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h
>> index 88d83d3..d4e9c7d 100644
>> --- a/include/linux/irqchip/arm-gic-v3.h
>> +++ b/include/linux/irqchip/arm-gic-v3.h
>> @@ -355,6 +355,27 @@
>> #define ICC_CTLR_EL1_EOImode_SHIFT (1)
>> #define ICC_CTLR_EL1_EOImode_drop_dir (0U << ICC_CTLR_EL1_EOImode_SHIFT)
>> #define ICC_CTLR_EL1_EOImode_drop (1U << ICC_CTLR_EL1_EOImode_SHIFT)
>> +#define ICC_CTLR_EL1_EOImode_MASK (1 << ICC_CTLR_EL1_EOImode_SHIFT)
>> +#define ICC_CTLR_EL1_CBPR_SHIFT (0)
>> +#define ICC_CTLR_EL1_CBPR_MASK (1 << ICC_CTLR_EL1_CBPR_SHIFT)
>> +#define ICC_CTLR_EL1_PRI_BITS_SHIFT (8)
>> +#define ICC_CTLR_EL1_PRI_BITS_MASK (0x7 << ICC_CTLR_EL1_PRI_BITS_SHIFT)
>> +#define ICC_CTLR_EL1_ID_BITS_SHIFT (11)
>> +#define ICC_CTLR_EL1_ID_BITS_MASK (0x7 << ICC_CTLR_EL1_ID_BITS_SHIFT)
>> +#define ICC_CTLR_EL1_SEIS_SHIFT (14)
>> +#define ICC_CTLR_EL1_SEIS_MASK (0x1 << ICC_CTLR_EL1_SEIS_SHIFT)
>> +#define ICC_CTLR_EL1_A3V_SHIFT (15)
>> +#define ICC_CTLR_EL1_A3V_MASK (0x1 << ICC_CTLR_EL1_A3V_SHIFT)
>> +#define ICC_PMR_EL1_SHIFT (0)
>> +#define ICC_PMR_EL1_MASK (0xff << ICC_PMR_EL1_SHIFT)
>> +#define ICC_BPR0_EL1_SHIFT (0)
>> +#define ICC_BPR0_EL1_MASK (0x7 << ICC_BPR0_EL1_SHIFT)
>> +#define ICC_BPR1_EL1_SHIFT (0)
>> +#define ICC_BPR1_EL1_MASK (0x7 << ICC_BPR1_EL1_SHIFT)
>> +#define ICC_IGRPEN0_EL1_SHIFT (0)
>> +#define ICC_IGRPEN0_EL1_MASK (1 << ICC_IGRPEN0_EL1_SHIFT)
>> +#define ICC_IGRPEN1_EL1_SHIFT (0)
>> +#define ICC_IGRPEN1_EL1_MASK (1 << ICC_IGRPEN1_EL1_SHIFT)
>> #define ICC_SRE_EL1_SRE (1U << 0)
>>
>> /*
>> @@ -398,6 +419,15 @@
>> #define ICH_VMCR_ENG1_SHIFT 1
>> #define ICH_VMCR_ENG1_MASK (1 << ICH_VMCR_ENG1_SHIFT)
>>
>> +#define ICH_VTR_PRI_BITS_SHIFT 29
>> +#define ICH_VTR_PRI_BITS_MASK (7 << ICH_VTR_PRI_BITS_SHIFT)
>> +#define ICH_VTR_ID_BITS_SHIFT 23
>> +#define ICH_VTR_ID_BITS_MASK (7 << ICH_VTR_ID_BITS_SHIFT)
>> +#define ICH_VTR_SEIS_SHIFT 22
>> +#define ICH_VTR_SEIS_MASK (1 << ICH_VTR_SEIS_SHIFT)
>> +#define ICH_VTR_A3V_SHIFT 21
>> +#define ICH_VTR_A3V_MASK (1 << ICH_VTR_A3V_SHIFT)
>> +
>
> Same thing here. Please group all the additions to this file into a
> single, independent patch.
>
>> #define ICC_IAR1_EL1_SPURIOUS 0x3ff
>>
>> #define ICC_SRE_EL2_SRE (1 << 0)
>> diff --git a/virt/kvm/arm/vgic/vgic-kvm-device.c b/virt/kvm/arm/vgic/vgic-kvm-device.c
>> index a4656fc..a48101f 100644
>> --- a/virt/kvm/arm/vgic/vgic-kvm-device.c
>> +++ b/virt/kvm/arm/vgic/vgic-kvm-device.c
>> @@ -506,6 +506,14 @@ static int vgic_v3_attr_regs_access(struct kvm_device *dev,
>> if (!is_write)
>> *reg = tmp32;
>> break;
>> + case KVM_DEV_ARM_VGIC_CPU_SYSREGS: {
>> + u64 regid;
>> +
>> + regid = (attr->attr & KVM_DEV_ARM_VGIC_SYSREG_INSTR_MASK);
>> + ret = vgic_v3_cpu_sysregs_uaccess(vcpu, is_write,
>> + regid, reg);
>> + break;
>> + }
>> default:
>> ret = -EINVAL;
>> break;
>> @@ -539,6 +547,15 @@ static int vgic_v3_set_attr(struct kvm_device *dev,
>> reg = tmp32;
>> return vgic_v3_attr_regs_access(dev, attr, ®, true);
>> }
>> + case KVM_DEV_ARM_VGIC_CPU_SYSREGS: {
>> + u64 __user *uaddr = (u64 __user *)(long)attr->addr;
>> + u64 reg;
>> +
>> + if (get_user(reg, uaddr))
>> + return -EFAULT;
>> +
>> + return vgic_v3_attr_regs_access(dev, attr, ®, true);
>> + }
>> }
>> return -ENXIO;
>> }
>> @@ -565,6 +582,15 @@ static int vgic_v3_get_attr(struct kvm_device *dev,
>> tmp32 = reg;
>> return put_user(tmp32, uaddr);
>> }
>> + case KVM_DEV_ARM_VGIC_CPU_SYSREGS: {
>> + u64 __user *uaddr = (u64 __user *)(long)attr->addr;
>> + u64 reg;
>> +
>> + ret = vgic_v3_attr_regs_access(dev, attr, ®, false);
>> + if (ret)
>> + return ret;
>> + return put_user(reg, uaddr);
>> + }
>> }
>>
>> return -ENXIO;
>> @@ -583,6 +609,7 @@ static int vgic_v3_has_attr(struct kvm_device *dev,
>> break;
>> case KVM_DEV_ARM_VGIC_GRP_DIST_REGS:
>> case KVM_DEV_ARM_VGIC_GRP_REDIST_REGS:
>> + case KVM_DEV_ARM_VGIC_CPU_SYSREGS:
>> return vgic_v3_has_attr_regs(dev, attr);
>> case KVM_DEV_ARM_VGIC_GRP_NR_IRQS:
>> return 0;
>> diff --git a/virt/kvm/arm/vgic/vgic-mmio-v3.c b/virt/kvm/arm/vgic/vgic-mmio-v3.c
>> index 83dece8..af748d7 100644
>> --- a/virt/kvm/arm/vgic/vgic-mmio-v3.c
>> +++ b/virt/kvm/arm/vgic/vgic-mmio-v3.c
>> @@ -23,6 +23,7 @@
>>
>> #include "vgic.h"
>> #include "vgic-mmio.h"
>> +#include "sys_regs.h"
>>
>> /* extract @num bytes at @offset bytes offset in data */
>> unsigned long extract_bytes(u64 data, unsigned int offset,
>> @@ -639,6 +640,23 @@ int vgic_v3_has_attr_regs(struct kvm_device *dev, struct kvm_device_attr *attr)
>> nr_regions = ARRAY_SIZE(vgic_v3_rdbase_registers);
>> break;
>> }
>> + case KVM_DEV_ARM_VGIC_CPU_SYSREGS: {
>> + u64 reg, id;
>> + unsigned long mpidr;
>> + struct kvm_vcpu *vcpu;
>> +
>> + mpidr = (attr->attr & KVM_DEV_ARM_VGIC_V3_MPIDR_MASK) >>
>> + KVM_DEV_ARM_VGIC_V3_MPIDR_SHIFT;
>> +
>> + vcpu = kvm_mpidr_to_vcpu(dev->kvm, mpidr);
>
> No. Your "mpidr" variable is not an MPIDR. It represents the same
> thing, but not with the right format.
Ok. I will format in the form of MPIDR reg.
>
>> + if (!vcpu)
>> + return -EINVAL;
>> + if (vcpu->vcpu_id >= atomic_read(&dev->kvm->online_vcpus))
>
> How can that happen?
OK. This check can be dropped as kvm_for_each_vcpu() already checks
for only online_vcpus.
>
>> + return -EINVAL;
>> +
>> + id = (attr->attr & KVM_DEV_ARM_VGIC_SYSREG_INSTR_MASK);
>> + return vgic_v3_has_cpu_sysregs_attr(vcpu, 0, id, ®);
>> + }
>> default:
>> return -ENXIO;
>> }
>> diff --git a/virt/kvm/arm/vgic/vgic-sys-reg-v3.c b/virt/kvm/arm/vgic/vgic-sys-reg-v3.c
>> new file mode 100644
>> index 0000000..8e4f403
>> --- /dev/null
>> +++ b/virt/kvm/arm/vgic/vgic-sys-reg-v3.c
>> @@ -0,0 +1,296 @@
>> +#include <linux/irqchip/arm-gic-v3.h>
>> +#include <linux/kvm.h>
>> +#include <linux/kvm_host.h>
>> +#include <kvm/iodev.h>
>> +#include <kvm/arm_vgic.h>
>> +#include <asm/kvm_emulate.h>
>> +#include <asm/kvm_arm.h>
>> +#include <asm/kvm_mmu.h>
>> +
>> +#include "vgic.h"
>> +#include "vgic-mmio.h"
>> +#include "sys_regs.h"
>> +
>> +static bool access_gic_ctlr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>> + const struct sys_reg_desc *r)
>> +{
>> + struct vgic_vmcr vmcr;
>> + u64 val;
>> + u32 ich_vtr;
>> +
>> + vgic_get_vmcr(vcpu, &vmcr);
>> + if (p->is_write) {
>> + val = p->regval;
>> + vmcr.ctlr &= ~(ICH_VMCR_CBPR_MASK | ICH_VMCR_EOIM_MASK);
>> + vmcr.ctlr |= ((val & ICC_CTLR_EL1_CBPR_MASK) >>
>> + ICC_CTLR_EL1_CBPR_SHIFT) << ICH_VMCR_CBPR_SHIFT;
>> + vmcr.ctlr |= ((val & ICC_CTLR_EL1_EOImode_MASK) >>
>> + ICC_CTLR_EL1_EOImode_SHIFT) << ICH_VMCR_EOIM_SHIFT;
>> + vgic_set_vmcr(vcpu, &vmcr);
>
> You've ignored my comments again: "What if userspace writes something
> that is incompatible with the current configuration? Wrong number of ID
> bits, or number of priorities?"
IMO, In case of incompatibility,
If ID bits and PRI bits are less than HW supported, it is ok.
If ID bits and PRI bits are greater than HW supported, then warn would be good
enough. Please suggest the behaviour that you think it should be.
>
>> + } else {
>> + ich_vtr = kvm_call_hyp(__vgic_v3_get_ich_vtr_el2);
>> +
>> + val = 0;
>> + val |= ((ich_vtr & ICH_VTR_PRI_BITS_MASK) >>
>> + ICH_VTR_PRI_BITS_SHIFT) << ICC_CTLR_EL1_PRI_BITS_SHIFT;
>> + val |= ((ich_vtr & ICH_VTR_ID_BITS_MASK) >>
>> + ICH_VTR_ID_BITS_SHIFT) << ICC_CTLR_EL1_ID_BITS_SHIFT;
>> + val |= ((ich_vtr & ICH_VTR_SEIS_MASK) >> ICH_VTR_SEIS_SHIFT)
>> + << ICC_CTLR_EL1_SEIS_SHIFT;
>> + val |= ((ich_vtr & ICH_VTR_A3V_MASK) >> ICH_VTR_A3V_SHIFT)
>> + << ICC_CTLR_EL1_A3V_SHIFT;
>> + val |= ((vmcr.ctlr & ICH_VMCR_CBPR_MASK) >>
>> + ICH_VMCR_CBPR_SHIFT) << ICC_CTLR_EL1_CBPR_SHIFT;
>> + val |= ((vmcr.ctlr & ICH_VMCR_EOIM_MASK) >>
>> + ICH_VMCR_EOIM_SHIFT) << ICC_CTLR_EL1_EOImode_SHIFT;
>> +
>> + p->regval = val;
>> + }
>> +
>> + return true;
>> +}
>> +
>> +static bool access_gic_pmr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>> + const struct sys_reg_desc *r)
>> +{
>> + struct vgic_vmcr vmcr;
>> +
>> + vgic_get_vmcr(vcpu, &vmcr);
>> + if (p->is_write) {
>> + vmcr.pmr = (p->regval & ICC_PMR_EL1_MASK) >> ICC_PMR_EL1_SHIFT;
>> + vgic_set_vmcr(vcpu, &vmcr);
>> + } else {
>> + p->regval = (vmcr.pmr << ICC_PMR_EL1_SHIFT) & ICC_PMR_EL1_MASK;
>> + }
>> +
>> + return true;
>> +}
>> +
>> +static bool access_gic_bpr0(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>> + const struct sys_reg_desc *r)
>> +{
>> + struct vgic_vmcr vmcr;
>> +
>> + vgic_get_vmcr(vcpu, &vmcr);
>> + if (p->is_write) {
>> + vmcr.bpr = (p->regval & ICC_BPR0_EL1_MASK) >>
>> + ICC_BPR0_EL1_SHIFT;
>> + vgic_set_vmcr(vcpu, &vmcr);
>> + } else {
>> + p->regval = (vmcr.bpr << ICC_BPR0_EL1_SHIFT) &
>> + ICC_BPR0_EL1_MASK;
>> + }
>> +
>> + return true;
>> +}
>> +
>> +static bool access_gic_bpr1(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>> + const struct sys_reg_desc *r)
>> +{
>> + struct vgic_vmcr vmcr;
>> +
>> + vgic_get_vmcr(vcpu, &vmcr);
>> + if (p->is_write) {
>> + vmcr.abpr = (p->regval & ICC_BPR1_EL1_MASK) >>
>> + ICC_BPR1_EL1_SHIFT;
>> + vgic_set_vmcr(vcpu, &vmcr);
>> + } else {
>> + p->regval = (vmcr.abpr << ICC_BPR1_EL1_SHIFT) &
>> + ICC_BPR1_EL1_MASK;
>> + }
>> +
>> + return true;
>> +}
>> +
>> +static bool access_gic_grpen0(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>> + const struct sys_reg_desc *r)
>> +{
>> + struct vgic_vmcr vmcr;
>> +
>> + vgic_get_vmcr(vcpu, &vmcr);
>> + if (p->is_write) {
>> + vmcr.grpen0 = (p->regval & ICC_IGRPEN0_EL1_MASK) >>
>> + ICC_IGRPEN0_EL1_SHIFT;
>> + vgic_set_vmcr(vcpu, &vmcr);
>> + } else {
>> + p->regval = (vmcr.grpen0 << ICC_IGRPEN0_EL1_SHIFT) &
>> + ICC_IGRPEN0_EL1_MASK;
>> + }
>> +
>> + return true;
>> +}
>> +
>> +static bool access_gic_grpen1(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>> + const struct sys_reg_desc *r)
>> +{
>> + struct vgic_vmcr vmcr;
>> +
>> + vgic_get_vmcr(vcpu, &vmcr);
>> + if (p->is_write) {
>> + vmcr.grpen1 = (p->regval & ICC_IGRPEN1_EL1_MASK) >>
>> + ICC_IGRPEN1_EL1_SHIFT;
>> + vgic_set_vmcr(vcpu, &vmcr);
>> + } else {
>> + p->regval = (vmcr.grpen1 << ICC_IGRPEN1_EL1_SHIFT) &
>> + ICC_IGRPEN1_EL1_MASK;
>
> From the previous review comments: "Shouldn't this account for the
> ICC_CTLR_EL1.CBPR setting?"
Ok. I think this comment is for ICC_BPR1_EL1 access.
I will make a check on ICC_CTLR.EL1.CBPR for accessing ICC_BPR1_EL1.
>
>> + }
>> +
>> + return true;
>> +}
>> +
>> +static void vgic_v3_access_apr_reg(struct kvm_vcpu *vcpu,
>> + struct sys_reg_params *p, u8 apr, u8 idx)
>> +{
>> + struct vgic_v3_cpu_if *vgicv3 = &vcpu->arch.vgic_cpu.vgic_v3;
>> + uint32_t *ap_reg;
>> +
>> + if (apr)
>> + ap_reg = &vgicv3->vgic_ap1r[idx];
>> + else
>> + ap_reg = &vgicv3->vgic_ap0r[idx];
>> +
>> + if (p->is_write)
>> + *ap_reg = p->regval;
>> + else
>> + p->regval = *ap_reg;
>> +}
>> +
>> +static void access_gic_aprn(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>> + const struct sys_reg_desc *r, u8 apr)
>> +{
>> + u8 num_pri_bits, idx;
>> + u32 ich_vtr = kvm_call_hyp(__vgic_v3_get_ich_vtr_el2);
>
> Maybe you should cache this once and for all? It is fairly unlikely to
> change over time...
Is it ok to cache in vgic_global struct?
^ permalink raw reply
* [PATCH v3 6/6] ARM: dts: bcm283x: add pinctrl group to &sdhci, drop pins from &gpio
From: Stefan Wahren @ 2016-09-16 16:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473841415-937-7-git-send-email-kraxel@redhat.com>
Hi Gerd,
> Gerd Hoffmann <kraxel@redhat.com> hat am 14. September 2016 um 10:23
> geschrieben:
>
>
> As the alt3 group has no pins left after this
> update drop the whole alt3 group from &gpio.
sorry it won't be that simple. The references needs to removed too :-(
> grep alt3 *.dts*
bcm2835-rpi-a.dts: pinctrl-0 = <&gpioout &alt0 &i2s_alt2 &alt3>;
bcm2835-rpi-a-plus.dts: pinctrl-0 = <&gpioout &alt0 &i2s_alt0 &alt3>;
bcm2835-rpi-b.dts: pinctrl-0 = <&gpioout &alt0 &alt3>;
bcm2835-rpi-b-plus.dts: pinctrl-0 = <&gpioout &alt0 &i2s_alt0 &alt3>;
bcm2835-rpi-b-rev2.dts: pinctrl-0 = <&gpioout &alt0 &i2s_alt2 &alt3>;
bcm2835-rpi.dtsi: alt3: alt3 {
bcm2835-rpi-zero.dts: pinctrl-0 = <&gpioout &alt0 &i2s_alt0 &alt3>;
bcm2836-rpi-2-b.dts: pinctrl-0 = <&gpioout &alt0 &i2s_alt0 &alt3>;
Regards
Stefan
^ permalink raw reply
* [PATCH v4 2/2] soc: qcom: add l2 cache perf events driver
From: Mark Rutland @ 2016-09-16 16:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <27f91809-f968-e278-7887-d0f9af275502@codeaurora.org>
On Fri, Sep 16, 2016 at 11:33:39AM -0400, Neil Leeder wrote:
> Hi Mark,
> Thank you for the thorough review. I will post an updated patchset
> which addresses all of your comments. There is just one outstanding
> comment which I have a question about:
[...]
> On 9/1/2016 12:30 PM, Mark Rutland wrote:
> > On Tue, Aug 30, 2016 at 01:01:33PM -0400, Neil Leeder wrote:
> >> + /* Don't allow groups with mixed PMUs, except for s/w events */
> >> + if (event->group_leader->pmu != event->pmu &&
> >> + !is_software_event(event->group_leader)) {
> >> + dev_warn(&l2cache_pmu->pdev->dev,
> >> + "Can't create mixed PMU group\n");
> >> + return -EINVAL;
> >> + }
> >> +
> >> + list_for_each_entry(sibling, &event->group_leader->sibling_list,
> >> + group_entry)
> >> + if (sibling->pmu != event->pmu &&
> >> + !is_software_event(sibling)) {
> >> + dev_warn(&l2cache_pmu->pdev->dev,
> >> + "Can't create mixed PMU group\n");
> >> + return -EINVAL;
> >> + }
> >> +
> >> + hwc->idx = -1;
> >> + hwc->config_base = event->attr.config;
> >> +
> >> + /*
> >> + * Ensure all events are on the same cpu so all events are in the
> >> + * same cpu context, to avoid races on pmu_enable etc.
> >> + */
> >> + slice = get_hml2_pmu(event->cpu);
> >> + event->cpu = slice->on_cpu;
> >
> > This could put an event on a different CPU to its group siblings, which
> > is broken.
>
> This is the same logic as in arm-ccn.c:arm_ccn_pmu_event_init(), where there
> is a single CPU designated as the CPU to be used for all events.
>
> All events for this slice are forced to slice->on_cpu which is the CPU
> set in the cpumask for this slice.
The CCN is a little different. For the CCN, a single CPU is designated
to handle *all* events.
For this driver, a CPU is designated per-slice, judging by the existence
of hml2_pmu::on_cpu (unless that's superfluous). We've only verified
that the events are all for this PMU, not the same slice, and thus each
event->cpu may differ.
> I'm not sure how this can put an event on a different CPU to its group
> siblings?
In practice today, we'll try to schedule the event on it's group
leader's CPU, but accounting and subsequent manipulation could go wrong.
Thanks,
Mark.
^ permalink raw reply
* [PATCH v3 1/3] tty: amba-pl011: define flag register bits for ZTE device
From: Russell King - ARM Linux @ 2016-09-16 16:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <b549cd51-a81f-b693-e3c2-9e3c598d3d9e@arm.com>
On Fri, Sep 16, 2016 at 03:23:57PM +0100, Sudeep Holla wrote:
> Hi Shawn, Russell,
>
> I noticed this change is in linux-next and but I happen to hit an issue
> with this as I tested it with earlycon enabled today for the first time.
>
> On 08/07/16 10:00, Shawn Guo wrote:
> >For some reason we do not really understand, ZTE hardware designers
> >choose to define PL011 Flag Register bit positions differently from
> >standard ones as below.
> >
> > Bit Standard ZTE
> > -----------------------------------
> > CTS 0 1
> > DSR 1 3
> > BUSY 3 8
> > RI 8 0
> >
> >Let's define these bits into vendor data and get ZTE PL011 supported
> >properly.
> >
> >Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
>
> [...]
>
> >@@ -2303,13 +2325,16 @@ static struct console amba_console = {
> >
> > static void pl011_putc(struct uart_port *port, int c)
> > {
> >+ struct uart_amba_port *uap =
> >+ container_of(port, struct uart_amba_port, port);
> >+
> > while (readl(port->membase + UART01x_FR) & UART01x_FR_TXFF)
> > cpu_relax();
> > if (port->iotype == UPIO_MEM32)
> > writel(c, port->membase + UART01x_DR);
> > else
> > writeb(c, port->membase + UART01x_DR);
> >- while (readl(port->membase + UART01x_FR) & UART01x_FR_BUSY)
> >+ while (readl(port->membase + UART01x_FR) & uap->vendor->fr_busy)
> > cpu_relax();
> > }
>
> The above hunk won't work for early console devices. The earlycon_device
> just has uart_port and is not uart_amba_port. I don't know how to fix
> this properly but I thought we could reuse private_data in uart_port for
> early_con devices. Something like below(incomplete for other vendors,
> works only for ARM)
Unfortunate - I'm getting rather annoyed with the way vendors create
these derivatives of ARM hardware which then cause problems for the
kernel, springing up all these vendor specific hacks all over the
place.
Maybe what we should've done with ZTE is insisted that they implement
a complete new driver, rather than trying to shoe-horn it into PL011
even though it is in theory PL011.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* [PATCH v9 07/10] arm: arm64: pmu: Assign platform PMU CPU affinity
From: Punit Agrawal @ 2016-09-16 16:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473892358-22574-8-git-send-email-jeremy.linton@arm.com>
Hi Jeremy,
One comment below.
Jeremy Linton <jeremy.linton@arm.com> writes:
> On systems with multiple PMU types the PMU to CPU affinity
> needs to be detected and set. The CPU to interrupt affinity
> should also be set.
>
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> ---
> drivers/perf/arm_pmu.c | 63 ++++++++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 53 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
> index 58117d7..63f16a5 100644
> --- a/drivers/perf/arm_pmu.c
> +++ b/drivers/perf/arm_pmu.c
> @@ -11,6 +11,7 @@
> */
> #define pr_fmt(fmt) "hw perfevents: " fmt
>
> +#include <linux/acpi.h>
> #include <linux/bitmap.h>
> #include <linux/cpumask.h>
> #include <linux/cpu_pm.h>
> @@ -24,6 +25,7 @@
> #include <linux/irq.h>
> #include <linux/irqdesc.h>
>
> +#include <asm/cpu.h>
> #include <asm/cputype.h>
> #include <asm/irq_regs.h>
>
> @@ -876,25 +878,67 @@ static void cpu_pmu_destroy(struct arm_pmu *cpu_pmu)
> }
>
> /*
> - * CPU PMU identification and probing.
> + * CPU PMU identification and probing. Its possible to have
> + * multiple CPU types in an ARM machine. Assure that we are
> + * picking the right PMU types based on the CPU in question
> */
> -static int probe_current_pmu(struct arm_pmu *pmu,
> - const struct pmu_probe_info *info)
> +static int probe_plat_pmu(struct arm_pmu *pmu,
> + const struct pmu_probe_info *info,
> + unsigned int pmuid)
> {
> - int cpu = get_cpu();
> - unsigned int cpuid = read_cpuid_id();
> int ret = -ENODEV;
> + int cpu;
> + int aff_ctr = 0;
> + static int duplicate_pmus;
> + struct platform_device *pdev = pmu->plat_device;
> + int irq = platform_get_irq(pdev, 0);
>
> - pr_info("probing PMU on CPU %d\n", cpu);
> + if (irq >= 0 && !irq_is_percpu(irq)) {
Marc's got a patch[0] changing all instances of "irq >= 0" in this file to
"irq > 0" on the basis that irq 0 is an error.
If this version doesn't get merged, please drop the "=" for the next
version.
[0] http://marc.info/?l=linux-arm-kernel&m=147318284923863
[...]
^ permalink raw reply
* [PATCH v9 08/10] arm64: pmu: Detect and enable multiple PMUs in an ACPI system
From: Jeremy Linton @ 2016-09-16 16:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <87y42st1e3.fsf@e105922-lin.cambridge.arm.com>
Hi,
On 09/16/2016 08:33 AM, Punit Agrawal wrote:
> Jeremy Linton <jeremy.linton@arm.com> writes:
>
>> Its possible that an ACPI system has multiple CPU types in it
>> with differing PMU counters. Iterate the CPU's and make a determination
>> about how many of each type exist in the system. Then take and create
>> a PMU platform device for each type, and assign it the interrupts parsed
>> from the MADT. Creating a platform device is necessary because the PMUs
>> are not described as devices in the DSDT table.
>>
>> This code is loosely based on earlier work by Mark Salter.
(trimming)
>> +
>> + list_for_each_entry_safe(pmu, safe_temp, &pmus, list) {
>> + if (unused_madt_entries)
>> + pmu->cpu_count = num_possible_cpus();
>
> So if there is any unbooted cpu ...
>
>> +
>> + res = kcalloc(pmu->cpu_count,
>> + sizeof(struct resource), GFP_KERNEL);
>
> ... we allocate potentially large number (num_possible_cpus()) of
> resources for each PMU.
>
> This is needlessly wasteful. Under what conditions have you found
> reg_midr to be 0?
Unused MADT entries, in place for potentially unbooted/hotplug CPUs. In
those cases you don't know for sure which PMU the CPU belongs to until
it comes online and the MIDR can be read. I'm open to suggestions on how
to deal with this, outside of pushing my luck and further breaking the
platform device encapsulation by trying to reallocate the resource
structure while its active. Besides its only wasteful for
ACPI+big.little, which at the moment only applies to a development platform.
^ permalink raw reply
* [PATCH v5 1/6] arm/arm64: vgic-new: Implement support for userspace access
From: Marc Zyngier @ 2016-09-16 16:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1474028453-29132-2-git-send-email-vijay.kilari@gmail.com>
On 16/09/16 13:20, vijay.kilari at gmail.com wrote:
> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>
> Read and write of some registers like ISPENDR and ICPENDR
> from userspace requires special handling when compared to
> guest access for these registers.
>
> Refer to Documentation/virtual/kvm/devices/arm-vgic-v3.txt
> for handling of ISPENDR, ICPENDR registers handling.
>
> Add infrastructure to support guest and userspace read
> and write for the required registers
> Also moved vgic_uaccess from vgic-mmio-v2.c to vgic-mmio.c
>
> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
> ---
> virt/kvm/arm/vgic/vgic-mmio-v2.c | 25 ----------
> virt/kvm/arm/vgic/vgic-mmio-v3.c | 98 ++++++++++++++++++++++++++++++++--------
> virt/kvm/arm/vgic/vgic-mmio.c | 78 ++++++++++++++++++++++++++++----
> virt/kvm/arm/vgic/vgic-mmio.h | 19 ++++++++
> 4 files changed, 169 insertions(+), 51 deletions(-)
>
> diff --git a/virt/kvm/arm/vgic/vgic-mmio-v2.c b/virt/kvm/arm/vgic/vgic-mmio-v2.c
> index b44b359..0b32f40 100644
> --- a/virt/kvm/arm/vgic/vgic-mmio-v2.c
> +++ b/virt/kvm/arm/vgic/vgic-mmio-v2.c
> @@ -406,31 +406,6 @@ int vgic_v2_has_attr_regs(struct kvm_device *dev, struct kvm_device_attr *attr)
> return -ENXIO;
> }
>
> -/*
> - * When userland tries to access the VGIC register handlers, we need to
> - * create a usable struct vgic_io_device to be passed to the handlers and we
> - * have to set up a buffer similar to what would have happened if a guest MMIO
> - * access occurred, including doing endian conversions on BE systems.
> - */
> -static int vgic_uaccess(struct kvm_vcpu *vcpu, struct vgic_io_device *dev,
> - bool is_write, int offset, u32 *val)
> -{
> - unsigned int len = 4;
> - u8 buf[4];
> - int ret;
> -
> - if (is_write) {
> - vgic_data_host_to_mmio_bus(buf, len, *val);
> - ret = kvm_io_gic_ops.write(vcpu, &dev->dev, offset, len, buf);
> - } else {
> - ret = kvm_io_gic_ops.read(vcpu, &dev->dev, offset, len, buf);
> - if (!ret)
> - *val = vgic_data_mmio_bus_to_host(buf, len);
> - }
> -
> - return ret;
> -}
> -
> int vgic_v2_cpuif_uaccess(struct kvm_vcpu *vcpu, bool is_write,
> int offset, u32 *val)
> {
> diff --git a/virt/kvm/arm/vgic/vgic-mmio-v3.c b/virt/kvm/arm/vgic/vgic-mmio-v3.c
> index 0d3c76a..edd3d40 100644
> --- a/virt/kvm/arm/vgic/vgic-mmio-v3.c
> +++ b/virt/kvm/arm/vgic/vgic-mmio-v3.c
> @@ -209,6 +209,62 @@ static unsigned long vgic_mmio_read_v3_idregs(struct kvm_vcpu *vcpu,
> return 0;
> }
>
> +static unsigned long vgic_uaccess_read_v3_pending(struct kvm_vcpu *vcpu,
Maming is completely inconsistent. This should read "vgic_v3_uaccess_read_pending".
> + gpa_t addr, unsigned int len)
> +{
> + u32 intid = VGIC_ADDR_TO_INTID(addr, 1);
> + u32 value = 0;
> + int i;
> +
> + /*
> + * A level triggerred interrupt pending state is latched in both
> + * "soft_pending" and "line_level" variables. Userspace will save
> + * and restore soft_pending and line_level separately.
> + * Refer to Documentation/virtual/kvm/devices/arm-vgic-v3.txt
> + * handling of ISPENDR and ICPENDR.
> + */
> + for (i = 0; i < len * 8; i++) {
> + struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
> +
> + if (irq->config == VGIC_CONFIG_LEVEL && irq->soft_pending)
> + value |= (1U << i);
> + if (irq->config == VGIC_CONFIG_EDGE && irq->pending)
> + value |= (1U << i);
> +
> + vgic_put_irq(vcpu->kvm, irq);
> + }
> +
> + return value;
> +}
> +
> +static void vgic_uaccess_write_v3_pending(struct kvm_vcpu *vcpu,
Same here.
> + gpa_t addr, unsigned int len,
> + unsigned long val)
> +{
> + u32 intid = VGIC_ADDR_TO_INTID(addr, 1);
> + int i;
> +
> + for (i = 0; i < len * 8; i++) {
> + struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
> +
> + spin_lock(&irq->irq_lock);
> + if (test_bit(i, &val)) {
> + irq->pending = true;
> + irq->soft_pending = true;
> + vgic_queue_irq_unlock(vcpu->kvm, irq);
> + } else {
> + irq->soft_pending = false;
> + if (irq->config == VGIC_CONFIG_EDGE ||
> + (irq->config == VGIC_CONFIG_LEVEL &&
> + !irq->line_level))
> + irq->pending = false;
> + spin_unlock(&irq->irq_lock);
> + }
> +
> + vgic_put_irq(vcpu->kvm, irq);
> + }
> +}
> +
> /* We want to avoid outer shareable. */
> u64 vgic_sanitise_shareability(u64 field)
> {
> @@ -358,7 +414,7 @@ static void vgic_mmio_write_pendbase(struct kvm_vcpu *vcpu,
> * We take some special care here to fix the calculation of the register
> * offset.
> */
> -#define REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(off, rd, wr, bpi, acc) \
> +#define REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(off, rd, wr, ur, uw, bpi, acc) \
> { \
> .reg_offset = off, \
> .bits_per_irq = bpi, \
> @@ -373,6 +429,8 @@ static void vgic_mmio_write_pendbase(struct kvm_vcpu *vcpu,
> .access_flags = acc, \
> .read = rd, \
> .write = wr, \
> + .uaccess_read = ur, \
> + .uaccess_write = uw, \
> }
>
> static const struct vgic_register_region vgic_v3_dist_registers[] = {
> @@ -380,40 +438,42 @@ static const struct vgic_register_region vgic_v3_dist_registers[] = {
> vgic_mmio_read_v3_misc, vgic_mmio_write_v3_misc, 16,
> VGIC_ACCESS_32bit),
> REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_IGROUPR,
> - vgic_mmio_read_rao, vgic_mmio_write_wi, 1,
> + vgic_mmio_read_rao, vgic_mmio_write_wi, NULL, NULL, 1,
> VGIC_ACCESS_32bit),
> REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_ISENABLER,
> - vgic_mmio_read_enable, vgic_mmio_write_senable, 1,
> + vgic_mmio_read_enable, vgic_mmio_write_senable, NULL, NULL, 1,
> VGIC_ACCESS_32bit),
> REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_ICENABLER,
> - vgic_mmio_read_enable, vgic_mmio_write_cenable, 1,
> + vgic_mmio_read_enable, vgic_mmio_write_cenable, NULL, NULL, 1,
> VGIC_ACCESS_32bit),
> REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_ISPENDR,
> - vgic_mmio_read_pending, vgic_mmio_write_spending, 1,
> + vgic_mmio_read_pending, vgic_mmio_write_spending,
> + vgic_uaccess_read_v3_pending, vgic_uaccess_write_v3_pending, 1,
> VGIC_ACCESS_32bit),
> REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_ICPENDR,
> - vgic_mmio_read_pending, vgic_mmio_write_cpending, 1,
> + vgic_mmio_read_pending, vgic_mmio_write_cpending,
> + vgic_uaccess_read_v3_pending, vgic_mmio_write_wi, 1,
>From the spec you quoted in the commit message:
+ Accesses to GICD_ICPENDR register region and GICR_ICPENDR0 registers have
+ RAZ/WI semantics, meaning that reads always return 0 and writes are always
+ ignored.
> VGIC_ACCESS_32bit),
> REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_ISACTIVER,
> - vgic_mmio_read_active, vgic_mmio_write_sactive, 1,
> + vgic_mmio_read_active, vgic_mmio_write_sactive, NULL, NULL, 1,
> VGIC_ACCESS_32bit),
> REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_ICACTIVER,
> - vgic_mmio_read_active, vgic_mmio_write_cactive, 1,
> + vgic_mmio_read_active, vgic_mmio_write_cactive, NULL, NULL, 1,
> VGIC_ACCESS_32bit),
> REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_IPRIORITYR,
> - vgic_mmio_read_priority, vgic_mmio_write_priority, 8,
> - VGIC_ACCESS_32bit | VGIC_ACCESS_8bit),
> + vgic_mmio_read_priority, vgic_mmio_write_priority, NULL, NULL,
> + 8, VGIC_ACCESS_32bit | VGIC_ACCESS_8bit),
> REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_ITARGETSR,
> - vgic_mmio_read_raz, vgic_mmio_write_wi, 8,
> + vgic_mmio_read_raz, vgic_mmio_write_wi, NULL, NULL, 8,
> VGIC_ACCESS_32bit | VGIC_ACCESS_8bit),
> REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_ICFGR,
> - vgic_mmio_read_config, vgic_mmio_write_config, 2,
> + vgic_mmio_read_config, vgic_mmio_write_config, NULL, NULL, 2,
> VGIC_ACCESS_32bit),
> REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_IGRPMODR,
> - vgic_mmio_read_raz, vgic_mmio_write_wi, 1,
> + vgic_mmio_read_raz, vgic_mmio_write_wi, NULL, NULL, 1,
> VGIC_ACCESS_32bit),
> REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_IROUTER,
> - vgic_mmio_read_irouter, vgic_mmio_write_irouter, 64,
> + vgic_mmio_read_irouter, vgic_mmio_write_irouter, NULL, NULL, 64,
> VGIC_ACCESS_64bit | VGIC_ACCESS_32bit),
> REGISTER_DESC_WITH_LENGTH(GICD_IDREGS,
> vgic_mmio_read_v3_idregs, vgic_mmio_write_wi, 48,
> @@ -451,11 +511,13 @@ static const struct vgic_register_region vgic_v3_sgibase_registers[] = {
> REGISTER_DESC_WITH_LENGTH(GICR_ICENABLER0,
> vgic_mmio_read_enable, vgic_mmio_write_cenable, 4,
> VGIC_ACCESS_32bit),
> - REGISTER_DESC_WITH_LENGTH(GICR_ISPENDR0,
> - vgic_mmio_read_pending, vgic_mmio_write_spending, 4,
> + REGISTER_DESC_WITH_LENGTH_UACCESS(GICR_ISPENDR0,
> + vgic_mmio_read_pending, vgic_mmio_write_spending,
> + vgic_uaccess_read_v3_pending, vgic_uaccess_write_v3_pending, 4,
> VGIC_ACCESS_32bit),
> - REGISTER_DESC_WITH_LENGTH(GICR_ICPENDR0,
> - vgic_mmio_read_pending, vgic_mmio_write_cpending, 4,
> + REGISTER_DESC_WITH_LENGTH_UACCESS(GICR_ICPENDR0,
> + vgic_mmio_read_pending, vgic_mmio_write_cpending,
> + vgic_uaccess_read_v3_pending, vgic_mmio_write_wi, 4,
Same issue.
> VGIC_ACCESS_32bit),
> REGISTER_DESC_WITH_LENGTH(GICR_ISACTIVER0,
> vgic_mmio_read_active, vgic_mmio_write_sactive, 4,
> diff --git a/virt/kvm/arm/vgic/vgic-mmio.c b/virt/kvm/arm/vgic/vgic-mmio.c
> index e18b30d..31f85df 100644
> --- a/virt/kvm/arm/vgic/vgic-mmio.c
> +++ b/virt/kvm/arm/vgic/vgic-mmio.c
> @@ -468,6 +468,73 @@ static bool check_region(const struct vgic_register_region *region,
> return false;
> }
>
> +static const struct vgic_register_region *
> +vgic_get_mmio_region(struct vgic_io_device *iodev, gpa_t addr, int len)
> +{
> + const struct vgic_register_region *region;
> +
> + region = vgic_find_mmio_region(iodev->regions, iodev->nr_regions,
> + addr - iodev->base_addr);
> + if (!region || !check_region(region, addr, len))
> + return NULL;
> +
> + return region;
> +}
> +
> +static int vgic_uaccess_read(struct kvm_vcpu *vcpu, struct kvm_io_device *dev,
> + gpa_t addr, u32 *val)
> +{
> + struct vgic_io_device *iodev = kvm_to_vgic_iodev(dev);
> + const struct vgic_register_region *region;
> + struct kvm_vcpu *r_vcpu;
> +
> + region = vgic_get_mmio_region(iodev, addr, sizeof(u32));
> + if (!region) {
> + *val = 0;
> + return 0;
> + }
> +
> + r_vcpu = iodev->redist_vcpu ? iodev->redist_vcpu : vcpu;
> + if (region->uaccess_read)
> + *val = region->uaccess_read(r_vcpu, addr, sizeof(u32));
> + else
> + *val = region->read(r_vcpu, addr, sizeof(u32));
> +
> + return 0;
> +}
> +
> +static int vgic_uaccess_write(struct kvm_vcpu *vcpu, struct kvm_io_device *dev,
> + gpa_t addr, const u32 *val)
> +{
> + struct vgic_io_device *iodev = kvm_to_vgic_iodev(dev);
> + const struct vgic_register_region *region;
> + struct kvm_vcpu *r_vcpu;
> +
> + region = vgic_get_mmio_region(iodev, addr, sizeof(u32));
> + if (!region)
> + return 0;
> +
> + r_vcpu = iodev->redist_vcpu ? iodev->redist_vcpu : vcpu;
> + if (region->uaccess_write)
> + region->uaccess_write(r_vcpu, addr, sizeof(u32), *val);
> + else
> + region->write(r_vcpu, addr, sizeof(u32), *val);
> +
> + return 0;
> +}
> +
> +/*
> + * Userland access to VGIC registers.
> + */
> +int vgic_uaccess(struct kvm_vcpu *vcpu, struct vgic_io_device *dev,
> + bool is_write, int offset, u32 *val)
> +{
> + if (is_write)
> + return vgic_uaccess_write(vcpu, &dev->dev, offset, val);
> + else
> + return vgic_uaccess_read(vcpu, &dev->dev, offset, val);
> +}
> +
> static int dispatch_mmio_read(struct kvm_vcpu *vcpu, struct kvm_io_device *dev,
> gpa_t addr, int len, void *val)
> {
> @@ -475,9 +542,8 @@ static int dispatch_mmio_read(struct kvm_vcpu *vcpu, struct kvm_io_device *dev,
> const struct vgic_register_region *region;
> unsigned long data = 0;
>
> - region = vgic_find_mmio_region(iodev->regions, iodev->nr_regions,
> - addr - iodev->base_addr);
> - if (!region || !check_region(region, addr, len)) {
> + region = vgic_get_mmio_region(iodev, addr, len);
> + if (!region) {
> memset(val, 0, len);
> return 0;
> }
> @@ -508,14 +574,10 @@ static int dispatch_mmio_write(struct kvm_vcpu *vcpu, struct kvm_io_device *dev,
> const struct vgic_register_region *region;
> unsigned long data = vgic_data_mmio_bus_to_host(val, len);
>
> - region = vgic_find_mmio_region(iodev->regions, iodev->nr_regions,
> - addr - iodev->base_addr);
> + region = vgic_get_mmio_region(iodev, addr, len);
> if (!region)
> return 0;
>
> - if (!check_region(region, addr, len))
> - return 0;
> -
> switch (iodev->iodev_type) {
> case IODEV_CPUIF:
> region->write(vcpu, addr, len, data);
> diff --git a/virt/kvm/arm/vgic/vgic-mmio.h b/virt/kvm/arm/vgic/vgic-mmio.h
> index 4c34d39..97e6df7 100644
> --- a/virt/kvm/arm/vgic/vgic-mmio.h
> +++ b/virt/kvm/arm/vgic/vgic-mmio.h
> @@ -34,6 +34,10 @@ struct vgic_register_region {
> gpa_t addr, unsigned int len,
> unsigned long val);
> };
> + unsigned long (*uaccess_read)(struct kvm_vcpu *vcpu, gpa_t addr,
> + unsigned int len);
> + void (*uaccess_write)(struct kvm_vcpu *vcpu, gpa_t addr,
> + unsigned int len, unsigned long val);
> };
>
> extern struct kvm_io_device_ops kvm_io_gic_ops;
> @@ -86,6 +90,18 @@ extern struct kvm_io_device_ops kvm_io_gic_ops;
> .write = wr, \
> }
>
> +#define REGISTER_DESC_WITH_LENGTH_UACCESS(off, rd, wr, urd, uwr, length, acc) \
> + { \
> + .reg_offset = off, \
> + .bits_per_irq = 0, \
> + .len = length, \
> + .access_flags = acc, \
> + .read = rd, \
> + .write = wr, \
> + .uaccess_read = urd, \
> + .uaccess_write = uwr, \
> + }
> +
> int kvm_vgic_register_mmio_region(struct kvm *kvm, struct kvm_vcpu *vcpu,
> struct vgic_register_region *reg_desc,
> struct vgic_io_device *region,
> @@ -158,6 +174,9 @@ void vgic_mmio_write_config(struct kvm_vcpu *vcpu,
> gpa_t addr, unsigned int len,
> unsigned long val);
>
> +int vgic_uaccess(struct kvm_vcpu *vcpu, struct vgic_io_device *dev,
> + bool is_write, int offset, u32 *val);
> +
> unsigned int vgic_v2_init_dist_iodev(struct vgic_io_device *dev);
>
> unsigned int vgic_v3_init_dist_iodev(struct vgic_io_device *dev);
>
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* [PATCH v7 0/5] mfd: tps65218: Clean ups
From: Keerthy @ 2016-09-16 16:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160916133153.GB27974@sirena.org.uk>
On Friday 16 September 2016 07:01 PM, Mark Brown wrote:
> On Fri, Sep 16, 2016 at 05:12:38PM +0530, Keerthy wrote:
>
>> Should i repost this series? I do not see this series in linux-next yet.
>
> Please don't send content free pings and please allow a reasonable time
> for review. People get busy, go on holiday, attend conferences and so
> on so unless there is some reason for urgency (like critical bug fixes)
> please allow at least a couple of weeks for review. If there have been
> review comments then people may be waiting for those to be addressed.
I should have been clearer.
The last Lee Jones conveyed that:
"I can't take this series yet, since it relies on a change which was
taken into Mark's Regulator tree"
https://lkml.org/lkml/2016/8/31/312
So wanted to check if i needed to re-base/repost this series again.
Sorry about the confusion i think that is because of $Subject goof up in
the 0th patch of the series.
>
> Sending content free pings adds to the mail volume (if they are seen at
> all) which is often the problem and since they can't be reviewed
> directly if something has gone wrong you'll have to resend the patches
> anyway, though there are some other maintainers who like them - if in
> doubt look at how patches for the subsystem are normally handled.
>
^ permalink raw reply
* [PATCH v7 00/22] Generic DT bindings for PCI IOMMUs and ARM SMMU
From: Robin Murphy @ 2016-09-16 16:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1838c65d-5944-8946-781c-b420bea1acab@redhat.com>
Hi Eric,
On 15/09/16 17:46, Auger Eric wrote:
[...]
> Hum OK; thanks for the explanation. With that implementation however,
> don't we face back the issue we encountered in early stage of default
> domain implementation:
>
> With this sample config (AMD overdrive + I350-T2 + 2VFs per PF) I fill
> the 8 context banks. Whereas practically we didn't need them before?
>
> 00:00.0 0600: 1022:1a00
> Subsystem: 1022:1a00
> 00:02.0 0600: 1022:1a01
> 00:02.2 0604: 1022:1a02
> Kernel driver in use: pcieport
> 01:00.0 0200: 8086:1521 (rev 01)
> Subsystem: 8086:0002
> Kernel driver in use: igb
> 01:00.1 0200: 8086:1521 (rev 01)
> Subsystem: 8086:0002
> Kernel driver in use: igb
> 01:10.0 0200: 8086:1520 (rev 01) -> context 5
> Subsystem: 8086:0002
> Kernel driver in use: vfio-pci
> 01:10.1 0200: 8086:1520 (rev 01) -> context 7
> Subsystem: 8086:0002
> Kernel driver in use: igbvf
> 01:10.4 0200: 8086:1520 (rev 01) -> context 6
> Subsystem: 8086:0002
> Kernel driver in use: igbvf
> 01:10.5 0200: 8086:1520 (rev 01) -> shortage
> Subsystem: 8086:0002
> Kernel driver in use: igbvf
>
> So I can't even do passthrough anymore with that config. Is there
> anything wrong in my setup/understanding?
It's kind of hard to avoid, really - people want DMA ops support (try
plugging a card which can only do 32-bit DMA into that Seattle, for
instance); DMA ops need default domains; default domains are allocated
per group; each domain requires a context bank to back it; thus if you
have 9 groups and 8 context banks you're in a corner. It would relieve
the pressure to have a single default domain per SMMU, but we've no way
to do that due to the way the iommu_domain_alloc() API is intended to work.
Ultimately, it's a hardware limitation of that platform - plug in a card
with 16 VFs with ACS, and either way you're stuck. There are a number of
bodges I can think of that would make your specific situation work, but
none of them are really sufficiently general to consider upstreaming.
The most logical thing to do right now, if you were happy without DMA
ops using the old binding before, is to keep using the old binding (just
fixing your DT with #stream-id-cells=0 on the host controller so as not
to create the fake aliasing problem). Hopefully future platforms will be
in a position to couple their PCI host controllers to an IOMMU which is
actually designed to support a PCI host controller.
What I probably will do, though, since we have the functionality in
place for the sake of the old binding, and I think there are other folks
who want PCI iommu-map support but would prefer not to bother with DMA
ops on the host, is add a command-line option to disable DMA domains
even for the generic bindings.
Robin.
^ permalink raw reply
* [PATCH v26 6/7] arm64: kdump: update a kernel doc
From: James Morse @ 2016-09-16 16:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160907042908.6232-7-takahiro.akashi@linaro.org>
Hi Akashi,
On 07/09/16 05:29, AKASHI Takahiro wrote:
> This patch adds arch specific descriptions about kdump usage on arm64
> to kdump.txt.
> diff --git a/Documentation/kdump/kdump.txt b/Documentation/kdump/kdump.txt
> @@ -249,6 +249,13 @@ Dump-capture kernel config options (Arch Dependent, arm)
>
> AUTO_ZRELADDR=y
>
> +Dump-capture kernel config options (Arch Dependent, arm64)
> +----------------------------------------------------------
> +
> +- Please note that kvm of the dump-capture kernel will not be enabled
> + on non-VHE systems even if it is configured. This is because the CPU
> + cannot be reset to EL2 on panic.
Nit:
cannot be -> will not be
We could try to do this, but its more code that could prevent us reaching the
kdump kernel, so we choose not to.
> @@ -370,6 +381,9 @@ For s390x:
> For arm:
> "1 maxcpus=1 reset_devices"
>
> +For arm64:
> + "1 maxcpus=1 reset_devices"
> +
'maxcpus=1' is a bit fragile. Since 44dbcc93ab67145 ("arm64: Fix behavior of
maxcpus=N") udev on ubuntu vivid (running on Juno) has taken it upon itself to
bring the secondary cores online, even when booted with 'maxcpus=1'.
Can we change the recomendation to "1 nosmp reset_devices"?
Thanks,
James
^ permalink raw reply
* [PATCH v26 4/7] arm64: kdump: add VMCOREINFO's for user-space coredump tools
From: James Morse @ 2016-09-16 16:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160907042908.6232-5-takahiro.akashi@linaro.org>
On 07/09/16 05:29, AKASHI Takahiro wrote:
> For the current crash utility, we need to know, at least,
> - kimage_voffset
> - PHYS_OFFSET
> to handle the contents of core dump file (/proc/vmcore) correctly due to
> the introduction of KASLR (CONFIG_RANDOMIZE_BASE) in v4.6.
> This patch puts them as VMCOREINFO's into the file.
>
> - VA_BITS
> is also added for makedumpfile command.
> More VMCOREINFO's may be added later.
Reviewed-by: James Morse <james.morse@arm.com>
Thanks,
James
^ permalink raw reply
* [PATCH v26 0/7] arm64: add kdump support
From: James Morse @ 2016-09-16 16:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160907042908.6232-1-takahiro.akashi@linaro.org>
(Cc: Ard),
Mark, Ard, how does/will reserved-memory work on an APCI only system?
On 07/09/16 05:29, AKASHI Takahiro wrote:
> v26-specific note: After a comment from Rob[0], an idea of adding
> "linux,usable-memory-range" was dropped. Instead, an existing
> "reserved-memory" node will be used to limit usable memory ranges
> on crash dump kernel.
> This works not only on UEFI/ACPI systems but also on DT-only systems,
> but if he really insists on using DT-specific "usable-memory" property,
> I will post additional patches for kexec-tools. Those would be
> redundant, though.
> Even in that case, the kernel will not have to be changed.
Some narrative on how the old memory ranges get reserved, as there is no longer
any code in the series doing this, (which is pretty neat!):
kexec-tools parses the list of memory ranges in /proc/iomem, and adds a node to
the /reserved-memory for System RAM ranges that don't cover the crash kernel.
Decompiling the crash-kernel DT from Seattle, it looks roughly like this:
reserved-memory {
ranges;
#size-cells = <0x2>;
#address-cells = <0x2>;
crash_dump at 83ffe50000 {
no-map;
reg = <0x83 0xffe50000 0x0 0x1b0000>;
};
[ ... ]
};
'no-map' means its doing the same thing to memblock as
'linux,usable-memory-range' did in earlier versions,
early_init_dt_reserve_memory_arch() takes no-map to mean memblock_remove().
We trigger the removing via early_init_fdt_scan_reserved_mem() in
arch/arm64/mm/init.c. This happens later than before, but its before the
crashkernel and cma ranges get reserved.
One difference I can see is that before we avoided memblock_remove()ing ranges
that were also in memblock.nomap. This was to avoid the ACPI tables getting
mapped as device memory by mistake, this is fixed by [1]. Now these ranges are
published in /proc/iomem as 'reserved' and won't get covered by a
reserved-memory node, and so we don't need to check memblock.nomap when
memblock_remove()ing.
The only odd thing I can see is for a (mythical?) pure-ACPI system. The EFI stub
will create a DT with a chosen node containing pointers to the memory map and
the efi command line. Now such as system may also grow a /reserved-memory node
after kdump. I don't think this is a problem, but it may not match how an
acpi-only system reserves memory. (how does that work?)
> [1] "arm64: mark reserved memblock regions explicitly in iomem"
> http://lists.infradead.org/pipermail/linux-arm-kernel/2016-August/450433.html
This is queued in Will's arm64/for-next/core,
> [2] "efi: arm64: treat regions with WT/WC set but WB cleared as memory"
> http://lists.infradead.org/pipermail/linux-arm-kernel/2016-August/451491.html
This is queued in tip, but I can't see why kdump depends on it. It only has an
effect if the uefi memory map has !WB regions that linux needs to use.
Thanks,
James
^ 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