Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1] watchdog: imx2: fix hang-up on boot for i.MX21, i.MX27 and i.MX31 SoCs
From: Uwe Kleine-König @ 2016-12-11 10:26 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <d6d987f1-3461-214a-0ca1-16d310ddd370@mleia.com>

Hello Vladimir,

On Sun, Dec 11, 2016 at 12:01:08PM +0200, Vladimir Zapolskiy wrote:
> On 12/11/2016 11:35 AM, Uwe Kleine-K?nig wrote:
> > On Sun, Dec 11, 2016 at 03:06:48AM +0200, Vladimir Zapolskiy wrote:
> >> Power down counter enable/disable bit switch is located in WMCR
> >> register, but watchdog controllers found on legacy i.MX21, i.MX27 and
> >> i.MX31 SoCs don't have this register. As a result of writing data to
> >> the non-existing register on driver probe the SoC hangs up, to fix the
> >> problem add more OF compatible strings and on this basis get
> >> information about availability of the WMCR register.
> >>
> >> Fixes: 5fe65ce7ccbb ("watchdog: imx2_wdt: Disable power down counter on boot")
> >> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
> >> ---
> 
> [snip]
> 
> >>  
> >>  static const struct of_device_id imx2_wdt_dt_ids[] = {
> >> -	{ .compatible = "fsl,imx21-wdt", },
> >> +	{ .compatible = "fsl,imx21-wdt", .data = IMX2_WDT_NO_WMCR },
> >> +	{ .compatible = "fsl,imx25-wdt",  },
> >> +	{ .compatible = "fsl,imx27-wdt", .data = IMX2_WDT_NO_WMCR },
> >> +	{ .compatible = "fsl,imx31-wdt", .data = IMX2_WDT_NO_WMCR },
> >> +	{ .compatible = "fsl,imx35-wdt",  },
> >> +	{ .compatible = "fsl,imx50-wdt",  },
> >> +	{ .compatible = "fsl,imx51-wdt",  },
> >> +	{ .compatible = "fsl,imx53-wdt",  },
> >> +	{ .compatible = "fsl,imx6q-wdt",  },
> >> +	{ .compatible = "fsl,imx6sl-wdt", },
> >> +	{ .compatible = "fsl,imx6sx-wdt", },
> >> +	{ .compatible = "fsl,imx6ul-wdt", },
> >> +	{ .compatible = "fsl,imx7d-wdt",  },
> >> +	{ .compatible = "fsl,vf610-wdt",  },
> > 
> > Up to now we only had fsl,imx21-wdt, now it seems that iMX31, i.MX27 and
> > i.MX21 form a group of equal watchdog IPs and the others form another
> > group, right?
> > 
> > So conceptually we should differenciate between
> > 
> > 	fsl,imx21-wdt (which is used on i.MX21, i.MX27 and i.MX31)
> > 	fsl,imx35-wdt (which is used on all others)
> > 
> 
> The problem is that "fsl,imx35-wdt" is not used on all others in the
> existing DTS, i.e. in the old firmware, which shall be compatible with
> new changes.

So old i.MX53 has

	compatible = "fsl,imx53-wdt", "fsl,imx21-wdt";

. With the change to the driver it's like using the watchdog driver in
it's pre-5fe65ce7ccbb4-state on it. If this is bad, we must indeed make
fsl,imx53-wdt known to the driver. If not, just adding a single new
compatible to the driver is just fine.

If you want to call it imx25 or imx35 doesn't matter much. The reason I
picked imx35 is that this one was already picked before for cspi. This
suggests that i.MX35 is older than i.MX25 and so this is the canonical
choice.

> $ grep -H 'fsl,imx.*-wdt' arch/arm/boot/dts/*.dtsi
> arch/arm/boot/dts/imx25.dtsi:				compatible = "fsl,imx25-wdt", "fsl,imx21-wdt";
> arch/arm/boot/dts/imx27.dtsi:				compatible = "fsl,imx27-wdt", "fsl,imx21-wdt";
> arch/arm/boot/dts/imx35.dtsi:				compatible = "fsl,imx35-wdt", "fsl,imx21-wdt";
> arch/arm/boot/dts/imx50.dtsi:				compatible = "fsl,imx50-wdt", "fsl,imx21-wdt";
> arch/arm/boot/dts/imx51.dtsi:				compatible = "fsl,imx51-wdt", "fsl,imx21-wdt";
> arch/arm/boot/dts/imx51.dtsi:				compatible = "fsl,imx51-wdt", "fsl,imx21-wdt";
> arch/arm/boot/dts/imx53.dtsi:				compatible = "fsl,imx53-wdt", "fsl,imx21-wdt";
> arch/arm/boot/dts/imx53.dtsi:				compatible = "fsl,imx53-wdt", "fsl,imx21-wdt";
> arch/arm/boot/dts/imx6qdl.dtsi:				compatible = "fsl,imx6q-wdt", "fsl,imx21-wdt";
> arch/arm/boot/dts/imx6qdl.dtsi:				compatible = "fsl,imx6q-wdt", "fsl,imx21-wdt";
> arch/arm/boot/dts/imx6sl.dtsi:				compatible = "fsl,imx6sl-wdt", "fsl,imx21-wdt";
> arch/arm/boot/dts/imx6sl.dtsi:				compatible = "fsl,imx6sl-wdt", "fsl,imx21-wdt";
> arch/arm/boot/dts/imx6sx.dtsi:				compatible = "fsl,imx6sx-wdt", "fsl,imx21-wdt";
> arch/arm/boot/dts/imx6sx.dtsi:				compatible = "fsl,imx6sx-wdt", "fsl,imx21-wdt";
> arch/arm/boot/dts/imx6sx.dtsi:				compatible = "fsl,imx6sx-wdt", "fsl,imx21-wdt";
> arch/arm/boot/dts/imx6ul.dtsi:				compatible = "fsl,imx6ul-wdt", "fsl,imx21-wdt";
> arch/arm/boot/dts/imx6ul.dtsi:				compatible = "fsl,imx6ul-wdt", "fsl,imx21-wdt";
> arch/arm/boot/dts/imx7s.dtsi:				compatible = "fsl,imx7d-wdt", "fsl,imx21-wdt";
> arch/arm/boot/dts/imx7s.dtsi:				compatible = "fsl,imx7d-wdt", "fsl,imx21-wdt";
> arch/arm/boot/dts/imx7s.dtsi:				compatible = "fsl,imx7d-wdt", "fsl,imx21-wdt";
> arch/arm/boot/dts/imx7s.dtsi:				compatible = "fsl,imx7d-wdt", "fsl,imx21-wdt";
> arch/arm/boot/dts/ls1021a.dtsi:			compatible = "fsl,imx21-wdt";
> arch/arm/boot/dts/vfxxx.dtsi:				compatible = "fsl,vf610-wdt", "fsl,imx21-wdt";
> 
> I don't see a confirmation of the fact that "fsl,imx35-wdt" is the best
> selection, hence definitely "fsl,imx25-wdt" overscores it.

I stated my reason to pick imx35 over imx25 above. Why do yo prefer
imx25?

> > A reason to add e.g. fsl,imx6sl-wdt is that dtbs in the wild use
> > 
> > 	"fsl,imx6sl-wdt", "fsl,imx21-wdt"
> > 
> > . But then I wonder if 
> > 
> > 	5fe65ce7ccbb ("watchdog: imx2_wdt: Disable power down counter on boot")
> > 
> > is that important to justify to add these all.
> 
> About this comment I don't know, you may have better insight about the original
> change. By the way, in barebox you may want to fix the same hang-up, because
> you touch WMCR unconditionally.

Sure.
 
> > Independant of this I'd like to have all dtsi for the newer SoCs changed
> > to get
> > 
> > 	"fsl,imx6sl-wdt", "fsl,imx35-wdt"
> > 
> > and if you really want to add all these compatibles, add a comment that
> > these are really fsl,imx35-wdt and were added to work around broken
> > dtbs.
> 
> No objections, but
> 
> 1) I'd like to see "fsl,imx25-wdt" as a new common compatible, and that's
> what have been proposed and shared in RFC 2/2.

Yeah, I missed that other patch (which was not part of this series).

> 2) Please remind me, why do you ask to drop "fsl,imx21-wdt" from the list?
> 
>      compatible = "fsl,imx6sx-wdt", "fsl,imx25-wdt", "fsl,imx21-wdt";
> 
> The list of compatibles above is valid (from the most specific on the left
> to the most generic on the right), and that compatible property is rightly
> handled in the driver with this change applied. I don't see a need to
> drop "fsl,imx21-wdt".

If the wdt in the i.MX6SX can be operated by the watchdog driver in it's
imx21 mode, you should keep the imx21 entry. If not, you shouldn't.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply

* [PATCH v1] watchdog: imx2: fix hang-up on boot for i.MX21, i.MX27 and i.MX31 SoCs
From: Vladimir Zapolskiy @ 2016-12-11 10:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161211093519.xrxxhrqpc7fmadfr@pengutronix.de>

Hello Uwe,

On 12/11/2016 11:35 AM, Uwe Kleine-K?nig wrote:
> On Sun, Dec 11, 2016 at 03:06:48AM +0200, Vladimir Zapolskiy wrote:
>> Power down counter enable/disable bit switch is located in WMCR
>> register, but watchdog controllers found on legacy i.MX21, i.MX27 and
>> i.MX31 SoCs don't have this register. As a result of writing data to
>> the non-existing register on driver probe the SoC hangs up, to fix the
>> problem add more OF compatible strings and on this basis get
>> information about availability of the WMCR register.
>>
>> Fixes: 5fe65ce7ccbb ("watchdog: imx2_wdt: Disable power down counter on boot")
>> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
>> ---

[snip]

>>  
>>  static const struct of_device_id imx2_wdt_dt_ids[] = {
>> -	{ .compatible = "fsl,imx21-wdt", },
>> +	{ .compatible = "fsl,imx21-wdt", .data = IMX2_WDT_NO_WMCR },
>> +	{ .compatible = "fsl,imx25-wdt",  },
>> +	{ .compatible = "fsl,imx27-wdt", .data = IMX2_WDT_NO_WMCR },
>> +	{ .compatible = "fsl,imx31-wdt", .data = IMX2_WDT_NO_WMCR },
>> +	{ .compatible = "fsl,imx35-wdt",  },
>> +	{ .compatible = "fsl,imx50-wdt",  },
>> +	{ .compatible = "fsl,imx51-wdt",  },
>> +	{ .compatible = "fsl,imx53-wdt",  },
>> +	{ .compatible = "fsl,imx6q-wdt",  },
>> +	{ .compatible = "fsl,imx6sl-wdt", },
>> +	{ .compatible = "fsl,imx6sx-wdt", },
>> +	{ .compatible = "fsl,imx6ul-wdt", },
>> +	{ .compatible = "fsl,imx7d-wdt",  },
>> +	{ .compatible = "fsl,vf610-wdt",  },
> 
> Up to now we only had fsl,imx21-wdt, now it seems that iMX31, i.MX27 and
> i.MX21 form a group of equal watchdog IPs and the others form another
> group, right?
> 
> So conceptually we should differenciate between
> 
> 	fsl,imx21-wdt (which is used on i.MX21, i.MX27 and i.MX31)
> 	fsl,imx35-wdt (which is used on all others)
> 

The problem is that "fsl,imx35-wdt" is not used on all others in the
existing DTS, i.e. in the old firmware, which shall be compatible with
new changes.

$ grep -H 'fsl,imx.*-wdt' arch/arm/boot/dts/*.dtsi
arch/arm/boot/dts/imx25.dtsi:				compatible = "fsl,imx25-wdt", "fsl,imx21-wdt";
arch/arm/boot/dts/imx27.dtsi:				compatible = "fsl,imx27-wdt", "fsl,imx21-wdt";
arch/arm/boot/dts/imx35.dtsi:				compatible = "fsl,imx35-wdt", "fsl,imx21-wdt";
arch/arm/boot/dts/imx50.dtsi:				compatible = "fsl,imx50-wdt", "fsl,imx21-wdt";
arch/arm/boot/dts/imx51.dtsi:				compatible = "fsl,imx51-wdt", "fsl,imx21-wdt";
arch/arm/boot/dts/imx51.dtsi:				compatible = "fsl,imx51-wdt", "fsl,imx21-wdt";
arch/arm/boot/dts/imx53.dtsi:				compatible = "fsl,imx53-wdt", "fsl,imx21-wdt";
arch/arm/boot/dts/imx53.dtsi:				compatible = "fsl,imx53-wdt", "fsl,imx21-wdt";
arch/arm/boot/dts/imx6qdl.dtsi:				compatible = "fsl,imx6q-wdt", "fsl,imx21-wdt";
arch/arm/boot/dts/imx6qdl.dtsi:				compatible = "fsl,imx6q-wdt", "fsl,imx21-wdt";
arch/arm/boot/dts/imx6sl.dtsi:				compatible = "fsl,imx6sl-wdt", "fsl,imx21-wdt";
arch/arm/boot/dts/imx6sl.dtsi:				compatible = "fsl,imx6sl-wdt", "fsl,imx21-wdt";
arch/arm/boot/dts/imx6sx.dtsi:				compatible = "fsl,imx6sx-wdt", "fsl,imx21-wdt";
arch/arm/boot/dts/imx6sx.dtsi:				compatible = "fsl,imx6sx-wdt", "fsl,imx21-wdt";
arch/arm/boot/dts/imx6sx.dtsi:				compatible = "fsl,imx6sx-wdt", "fsl,imx21-wdt";
arch/arm/boot/dts/imx6ul.dtsi:				compatible = "fsl,imx6ul-wdt", "fsl,imx21-wdt";
arch/arm/boot/dts/imx6ul.dtsi:				compatible = "fsl,imx6ul-wdt", "fsl,imx21-wdt";
arch/arm/boot/dts/imx7s.dtsi:				compatible = "fsl,imx7d-wdt", "fsl,imx21-wdt";
arch/arm/boot/dts/imx7s.dtsi:				compatible = "fsl,imx7d-wdt", "fsl,imx21-wdt";
arch/arm/boot/dts/imx7s.dtsi:				compatible = "fsl,imx7d-wdt", "fsl,imx21-wdt";
arch/arm/boot/dts/imx7s.dtsi:				compatible = "fsl,imx7d-wdt", "fsl,imx21-wdt";
arch/arm/boot/dts/ls1021a.dtsi:			compatible = "fsl,imx21-wdt";
arch/arm/boot/dts/vfxxx.dtsi:				compatible = "fsl,vf610-wdt", "fsl,imx21-wdt";

I don't see a confirmation of the fact that "fsl,imx35-wdt" is the best
selection, hence definitely "fsl,imx25-wdt" overscores it.

> A reason to add e.g. fsl,imx6sl-wdt is that dtbs in the wild use
> 
> 	"fsl,imx6sl-wdt", "fsl,imx21-wdt"
> 
> . But then I wonder if 
> 
> 	5fe65ce7ccbb ("watchdog: imx2_wdt: Disable power down counter on boot")
> 
> is that important to justify to add these all.

About this comment I don't know, you may have better insight about the original
change. By the way, in barebox you may want to fix the same hang-up, because
you touch WMCR unconditionally.

> 
> Independant of this I'd like to have all dtsi for the newer SoCs changed
> to get
> 
> 	"fsl,imx6sl-wdt", "fsl,imx35-wdt"
> 
> and if you really want to add all these compatibles, add a comment that
> these are really fsl,imx35-wdt and were added to work around broken
> dtbs.

No objections, but

1) I'd like to see "fsl,imx25-wdt" as a new common compatible, and that's
what have been proposed and shared in RFC 2/2.

2) Please remind me, why do you ask to drop "fsl,imx21-wdt" from the list?

     compatible = "fsl,imx6sx-wdt", "fsl,imx25-wdt", "fsl,imx21-wdt";

The list of compatibles above is valid (from the most specific on the left
to the most generic on the right), and that compatible property is rightly
handled in the driver with this change applied. I don't see a need to
drop "fsl,imx21-wdt".

As a reminder while changing the source code new (updated) kernel must be
compatible with the old (not updated) DTB firmware, and not the other way
round.

--
With best wishes,
Vladimir

^ permalink raw reply

* [PATCH] media: platform: xilinx: xilinx-tpg: constify v4l2_subdev_* structures
From: Bhumika Goyal @ 2016-12-11  9:52 UTC (permalink / raw)
  To: linux-arm-kernel

v4l2_subdev_{core/pad/video}_ops structures are stored in the
fields of the v4l2_subdev_ops structure which are of type const. 
Also, v4l2_subdev_ops structure is passed to a function
having its argument of type const. As these structures are never
modified, so declare them as const.
Done using Coccinelle: (One of the scripts used)

@r1 disable optional_qualifier @
identifier i;
position p;
@@
static struct v4l2_subdev_video_ops i at p = {...};

@ok1@
identifier r1.i;
position p;
struct v4l2_subdev_ops obj;
@@
obj.video=&i at p;

@bad@
position p!={r1.p,ok1.p};
identifier r1.i;
@@
i at p

@depends on !bad disable optional_qualifier@
identifier r1.i;
@@
+const
struct v4l2_subdev_video_ops i; 

File size before:
   text	   data	    bss	    dec	    hex	filename
   6170	   2752	    144	   9066	   236a media/platform/xilinx/xilinx-tpg.o

File size after:
   text	   data	    bss	    dec	    hex	filename
   6666	   2384	      8	   9058	   2362 media/platform/xilinx/xilinx-tpg.o

Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
---
 drivers/media/platform/xilinx/xilinx-tpg.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/xilinx/xilinx-tpg.c b/drivers/media/platform/xilinx/xilinx-tpg.c
index 2ec1f6c..9c49d1d 100644
--- a/drivers/media/platform/xilinx/xilinx-tpg.c
+++ b/drivers/media/platform/xilinx/xilinx-tpg.c
@@ -460,21 +460,21 @@ static int xtpg_s_ctrl(struct v4l2_ctrl *ctrl)
 	.s_ctrl	= xtpg_s_ctrl,
 };
 
-static struct v4l2_subdev_core_ops xtpg_core_ops = {
+static const struct v4l2_subdev_core_ops xtpg_core_ops = {
 };
 
-static struct v4l2_subdev_video_ops xtpg_video_ops = {
+static const struct v4l2_subdev_video_ops xtpg_video_ops = {
 	.s_stream = xtpg_s_stream,
 };
 
-static struct v4l2_subdev_pad_ops xtpg_pad_ops = {
+static const struct v4l2_subdev_pad_ops xtpg_pad_ops = {
 	.enum_mbus_code		= xvip_enum_mbus_code,
 	.enum_frame_size	= xtpg_enum_frame_size,
 	.get_fmt		= xtpg_get_format,
 	.set_fmt		= xtpg_set_format,
 };
 
-static struct v4l2_subdev_ops xtpg_ops = {
+static const struct v4l2_subdev_ops xtpg_ops = {
 	.core   = &xtpg_core_ops,
 	.video  = &xtpg_video_ops,
 	.pad    = &xtpg_pad_ops,
-- 
1.9.1

^ permalink raw reply related

* [RFC PATCH 2/2] ARM: i.MX: dts: add fsl, imx25-wdt compatible to all relevant watchdog nodes
From: Uwe Kleine-König @ 2016-12-11  9:40 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160926062759.3eoezyezog6clz6x@pengutronix.de>

Hello,

On Mon, Sep 26, 2016 at 08:27:59AM +0200, Uwe Kleine-K?nig wrote:
> On Mon, Sep 26, 2016 at 03:39:21AM +0300, Vladimir Zapolskiy wrote:
> > Watchdog device controller found on all modern SoCs from i.MX series
> > and firstly introduced in i.MX25 is not one in one compatible with the
> > watchdog controllers on i.MX21, i.MX27 and i.MX31, the latter
> > controlles don't have WICR (and pretimeout notification support) and
> > WMCR registers. To get benefit from the more advanced watchdog device
> > and to avoid operations over non-existing registers on legacy SoCs add
> > fsl,imx25-wdt compatible to descriptions of all i.MX25 compatible
> > watchdog controllers.
> > 
> > Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
> > ---
> >  arch/arm/boot/dts/imx35.dtsi   |  3 ++-
> >  arch/arm/boot/dts/imx50.dtsi   |  3 ++-
> >  arch/arm/boot/dts/imx51.dtsi   |  6 ++++--
> >  arch/arm/boot/dts/imx53.dtsi   |  6 ++++--
> >  arch/arm/boot/dts/imx6qdl.dtsi |  6 ++++--
> >  arch/arm/boot/dts/imx6sl.dtsi  |  6 ++++--
> >  arch/arm/boot/dts/imx6sx.dtsi  |  9 ++++++---
> >  arch/arm/boot/dts/imx6ul.dtsi  |  6 ++++--
> >  arch/arm/boot/dts/imx7s.dtsi   | 12 ++++++++----
> >  arch/arm/boot/dts/ls1021a.dtsi |  2 +-
> >  arch/arm/boot/dts/vfxxx.dtsi   |  3 ++-
> >  11 files changed, 41 insertions(+), 21 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/imx35.dtsi b/arch/arm/boot/dts/imx35.dtsi
> > index 490b7b4..8fd4482 100644
> > --- a/arch/arm/boot/dts/imx35.dtsi
> > +++ b/arch/arm/boot/dts/imx35.dtsi
> > @@ -284,7 +284,8 @@
> >  			};
> >  
> >  			wdog: wdog at 53fdc000 {
> > -				compatible = "fsl,imx35-wdt", "fsl,imx21-wdt";
> > +				compatible = "fsl,imx35-wdt", "fsl,imx25-wdt",
> > +					     "fsl,imx21-wdt";
> 
> When this is used on an old kernel that doesn't know about fsl,imx25-wdt
> this picks up the imx21 driver logic. As this is wrong I think you
> should drop imx21-wdt here. Can one of the dt-people comfirm?

I forgot in the mail at the other end of this thread that the dti were
already addressed. I (implicitly) wrote there that fsl,imx35-wdt should
be the new compatible describing the wdt with misc register. Picking
imx25 (as you did) works, too, but e.g. the CSPI device has

	compatible = "fsl,imx25-cspi", "fsl,imx35-cspi";

on the i.MX25 and

	compatible = "fsl,imx35-cspi";

on the i.MX35. So I think we should pick i.MX35 here, too, as the one
giving its name.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply

* [PATCH v1] watchdog: imx2: fix hang-up on boot for i.MX21, i.MX27 and i.MX31 SoCs
From: Uwe Kleine-König @ 2016-12-11  9:35 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161211010648.6591-1-vz@mleia.com>

On Sun, Dec 11, 2016 at 03:06:48AM +0200, Vladimir Zapolskiy wrote:
> Power down counter enable/disable bit switch is located in WMCR
> register, but watchdog controllers found on legacy i.MX21, i.MX27 and
> i.MX31 SoCs don't have this register. As a result of writing data to
> the non-existing register on driver probe the SoC hangs up, to fix the
> problem add more OF compatible strings and on this basis get
> information about availability of the WMCR register.
> 
> Fixes: 5fe65ce7ccbb ("watchdog: imx2_wdt: Disable power down counter on boot")
> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
> ---
> Changes from RFC to v1:
> * replaced private data struct with a macro as suggested by Guenter,
> * updated the comment in the source code to reflect the change,
> * rearranged and simplified the logic of detecting WMCR presence,
> * pulled the fix out from the series with optional proposed DTS changes.
> 
>  drivers/watchdog/imx2_wdt.c | 30 +++++++++++++++++++++++++-----
>  1 file changed, 25 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/watchdog/imx2_wdt.c b/drivers/watchdog/imx2_wdt.c
> index 4874b0f..66bd454 100644
> --- a/drivers/watchdog/imx2_wdt.c
> +++ b/drivers/watchdog/imx2_wdt.c
> @@ -30,6 +30,7 @@
>  #include <linux/module.h>
>  #include <linux/moduleparam.h>
>  #include <linux/of_address.h>
> +#include <linux/of_device.h>
>  #include <linux/platform_device.h>
>  #include <linux/regmap.h>
>  #include <linux/watchdog.h>
> @@ -57,6 +58,7 @@
>  #define IMX2_WDT_WICR_WICT	0xFF		/* -> Interrupt Count Timeout */
>  
>  #define IMX2_WDT_WMCR		0x08		/* Misc Register */
> +#define IMX2_WDT_NO_WMCR	((void *)true)	/* Indicates unavailable WMCR */
>  
>  #define IMX2_WDT_MAX_TIME	128
>  #define IMX2_WDT_DEFAULT_TIME	60		/* in seconds */
> @@ -70,6 +72,8 @@ struct imx2_wdt_device {
>  	bool ext_reset;
>  };
>  
> +static const struct of_device_id imx2_wdt_dt_ids[];
> +
>  static bool nowayout = WATCHDOG_NOWAYOUT;
>  module_param(nowayout, bool, 0);
>  MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
> @@ -242,6 +246,7 @@ static const struct regmap_config imx2_wdt_regmap_config = {
>  
>  static int __init imx2_wdt_probe(struct platform_device *pdev)
>  {
> +	const struct of_device_id *of_id;
>  	struct imx2_wdt_device *wdev;
>  	struct watchdog_device *wdog;
>  	struct resource *res;
> @@ -310,11 +315,13 @@ static int __init imx2_wdt_probe(struct platform_device *pdev)
>  	}
>  
>  	/*
> -	 * Disable the watchdog power down counter at boot. Otherwise the power
> -	 * down counter will pull down the #WDOG interrupt line for one clock
> -	 * cycle.
> +	 * If watchdog controller has WMCR, disable the watchdog power
> +	 * down counter at boot. Otherwise the power down counter will
> +	 * pull down the #WDOG interrupt line for one clock cycle.
>  	 */
> -	regmap_write(wdev->regmap, IMX2_WDT_WMCR, 0);
> +	of_id = of_match_device(imx2_wdt_dt_ids, &pdev->dev);
> +	if (of_id && !of_id->data)
> +		regmap_write(wdev->regmap, IMX2_WDT_WMCR, 0);
>  
>  	ret = watchdog_register_device(wdog);
>  	if (ret) {
> @@ -412,7 +419,20 @@ static SIMPLE_DEV_PM_OPS(imx2_wdt_pm_ops, imx2_wdt_suspend,
>  			 imx2_wdt_resume);
>  
>  static const struct of_device_id imx2_wdt_dt_ids[] = {
> -	{ .compatible = "fsl,imx21-wdt", },
> +	{ .compatible = "fsl,imx21-wdt", .data = IMX2_WDT_NO_WMCR },
> +	{ .compatible = "fsl,imx25-wdt",  },
> +	{ .compatible = "fsl,imx27-wdt", .data = IMX2_WDT_NO_WMCR },
> +	{ .compatible = "fsl,imx31-wdt", .data = IMX2_WDT_NO_WMCR },
> +	{ .compatible = "fsl,imx35-wdt",  },
> +	{ .compatible = "fsl,imx50-wdt",  },
> +	{ .compatible = "fsl,imx51-wdt",  },
> +	{ .compatible = "fsl,imx53-wdt",  },
> +	{ .compatible = "fsl,imx6q-wdt",  },
> +	{ .compatible = "fsl,imx6sl-wdt", },
> +	{ .compatible = "fsl,imx6sx-wdt", },
> +	{ .compatible = "fsl,imx6ul-wdt", },
> +	{ .compatible = "fsl,imx7d-wdt",  },
> +	{ .compatible = "fsl,vf610-wdt",  },

Up to now we only had fsl,imx21-wdt, now it seems that iMX31, i.MX27 and
i.MX21 form a group of equal watchdog IPs and the others form another
group, right?

So conceptually we should differenciate between

	fsl,imx21-wdt (which is used on i.MX21, i.MX27 and i.MX31)
	fsl,imx35-wdt (which is used on all others)

A reason to add e.g. fsl,imx6sl-wdt is that dtbs in the wild use

	"fsl,imx6sl-wdt", "fsl,imx21-wdt"

. But then I wonder if 

	5fe65ce7ccbb ("watchdog: imx2_wdt: Disable power down counter on boot")

is that important to justify to add these all.

Independant of this I'd like to have all dtsi for the newer SoCs changed
to get

	"fsl,imx6sl-wdt", "fsl,imx35-wdt"

and if you really want to add all these compatibles, add a comment that
these are really fsl,imx35-wdt and were added to work around broken
dtbs.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply

* [PATCH 2/2] kcov: make kcov work properly with KASLR enabled
From: Dmitry Vyukov @ 2016-12-11  9:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1481417456-28826-3-git-send-email-alex.popov@linux.com>

On Sun, Dec 11, 2016 at 1:50 AM, Alexander Popov <alex.popov@linux.com> wrote:
> Subtract KASLR offset from the kernel addresses reported by kcov.
> Tested on x86_64 and AArch64 (Hikey LeMaker).
>
> Signed-off-by: Alexander Popov <alex.popov@linux.com>
> ---
>  kernel/kcov.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/kcov.c b/kernel/kcov.c
> index 3cbb0c8..f8f3f4c 100644
> --- a/kernel/kcov.c
> +++ b/kernel/kcov.c
> @@ -14,6 +14,7 @@
>  #include <linux/debugfs.h>
>  #include <linux/uaccess.h>
>  #include <linux/kcov.h>
> +#include <asm/setup.h>
>
>  /*
>   * kcov descriptor (one per opened debugfs file).
> @@ -68,6 +69,11 @@ void notrace __sanitizer_cov_trace_pc(void)
>         if (mode == KCOV_MODE_TRACE) {
>                 unsigned long *area;
>                 unsigned long pos;
> +               unsigned long ip = _RET_IP_;
> +
> +#ifdef CONFIG_RANDOMIZE_BASE
> +               ip -= kaslr_offset();
> +#endif
>
>                 /*
>                  * There is some code that runs in interrupts but for which
> @@ -81,7 +87,7 @@ void notrace __sanitizer_cov_trace_pc(void)
>                 /* The first word is number of subsequent PCs. */
>                 pos = READ_ONCE(area[0]) + 1;
>                 if (likely(pos < t->kcov_size)) {
> -                       area[pos] = _RET_IP_;
> +                       area[pos] = ip;
>                         WRITE_ONCE(area[0], pos);
>                 }
>         }
> --
> 2.7.4


Hi,

I think generally this is the right thing to do.

 There are 2 pending patches for kcov by +Quentin (hopefully in mm):
"kcov: add AFL-style tracing"
"kcov: size of arena is now given in bytes"
https://groups.google.com/forum/#!topic/syzkaller/gcqbIhKjGcY
https://groups.google.com/d/msg/syzkaller/gcqbIhKjGcY/KQFryjBKCAAJ

Your patch probably conflicts with them.
Should you base them on top of these patches, so that Andrew can merge
it without conflicts?

^ permalink raw reply

* [PATCH v1] watchdog: imx2: fix hang-up on boot for i.MX21, i.MX27 and i.MX31 SoCs
From: Magnus Lilja @ 2016-12-11  8:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161211010648.6591-1-vz@mleia.com>

Hi

Excellent!

On 11 December 2016 at 02:06, Vladimir Zapolskiy <vz@mleia.com> wrote:
> Power down counter enable/disable bit switch is located in WMCR
> register, but watchdog controllers found on legacy i.MX21, i.MX27 and
> i.MX31 SoCs don't have this register. As a result of writing data to
> the non-existing register on driver probe the SoC hangs up, to fix the
> problem add more OF compatible strings and on this basis get
> information about availability of the WMCR register.
>
> Fixes: 5fe65ce7ccbb ("watchdog: imx2_wdt: Disable power down counter on boot")
> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
> ---

Tested-by: Magnus Lilja <lilja.magnus@gmail.com>

(Tested on i.MX31 without device tree)

/Magnus

^ permalink raw reply

* [PATCH V1] pinctrl:pxa:pinctrl-pxa2xx:- No need of devm functions
From: arvind Yadav @ 2016-12-11  8:04 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <87eg1g16r8.fsf@belgarion.home>

Yes, It will not fixes any defect. But we are going to free allocate
memory then why we need devm api. In this case Devm will first add this
entry to list and immediately  it will remove from list.

-Arvind

On Saturday 10 December 2016 02:49 PM, Robert Jarzmik wrote:
> Arvind Yadav <arvind.yadav.cs@gmail.com> writes:
>
> Hi Arvind,
>
>> In functions pxa2xx_build_functions, the memory allocated for
>> 'functions' is live within the function only. After the
>> allocation it is immediately freed with devm_kfree. There is
>> no need to allocate memory for 'functions' with devm function
>> so replace devm_kcalloc  with kcalloc and devm_kfree with kfree.
> That's not very true : the "need" is to spare the "manual" kfree you're adding
> in your patch for one, and make it consistent with pxa2xx_build_groups() and
> pxa2xx_build_state() for two.
>
> Therefore I'm not very thrilled by this patch and unless it fixes a defect in
> the driver I'd rather not have it in.
>
> Cheers.
>
> --
> Robert

^ permalink raw reply

* [PATCH] Input: imx6ul_tsc - generalize the averaging property
From: Guy Shapiro @ 2016-12-11  7:06 UTC (permalink / raw)
  To: linux-arm-kernel

Make the avarage-samples property a general touchscreen property
rather than imx6ul device specific.

Signed-off-by: Guy Shapiro <guy.shapiro@mobi-wize.com>
---
 .../bindings/input/touchscreen/imx6ul_tsc.txt      | 11 ++----
 .../bindings/input/touchscreen/touchscreen.txt     |  3 ++
 drivers/input/touchscreen/imx6ul_tsc.c             | 46 ++++++++++++++++------
 3 files changed, 41 insertions(+), 19 deletions(-)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt b/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
index a66069f..d4927c2 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
@@ -17,13 +17,8 @@ Optional properties:
   This value depends on the touch screen.
 - pre-charge-time: the touch screen need some time to precharge.
   This value depends on the touch screen.
-- average-samples: Number of data samples which are averaged for each read.
-	Valid values 0-4
-	0 =  1 sample
-	1 =  4 samples
-	2 =  8 samples
-	3 = 16 samples
-	4 = 32 samples
+- touchscreen-average-samples: Number of data samples which are averaged for
+  each read. Valid values are 1, 4, 8, 16 and 32.
 
 Example:
 	tsc: tsc at 02040000 {
@@ -39,6 +34,6 @@ Example:
 		xnur-gpio = <&gpio1 3 GPIO_ACTIVE_LOW>;
 		measure-delay-time = <0xfff>;
 		pre-charge-time = <0xffff>;
-		average-samples = <4>;
+		touchscreen-average-samples = <32>;
 		status = "okay";
 	};
diff --git a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt
index bccaa4e..537643e 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt
@@ -14,6 +14,9 @@ Optional properties for Touchscreens:
  - touchscreen-fuzz-pressure	: pressure noise value of the absolute input
 				  device (arbitrary range dependent on the
 				  controller)
+ - touchscreen-average-samples : Number of data samples which are averaged
+				  for each read (valid values dependent on the
+				  controller)
  - touchscreen-inverted-x	: X axis is inverted (boolean)
  - touchscreen-inverted-y	: Y axis is inverted (boolean)
  - touchscreen-swapped-x-y	: X and Y axis are swapped (boolean)
diff --git a/drivers/input/touchscreen/imx6ul_tsc.c b/drivers/input/touchscreen/imx6ul_tsc.c
index d2a3912..58d1aa5 100644
--- a/drivers/input/touchscreen/imx6ul_tsc.c
+++ b/drivers/input/touchscreen/imx6ul_tsc.c
@@ -93,7 +93,8 @@ struct imx6ul_tsc {
 
 	u32 measure_delay_time;
 	u32 pre_charge_time;
-	u32 average_samples;
+	bool average_enable;
+	u32 average_select;
 
 	struct completion completion;
 };
@@ -117,9 +118,9 @@ static int imx6ul_adc_init(struct imx6ul_tsc *tsc)
 	adc_cfg |= ADC_12BIT_MODE | ADC_IPG_CLK;
 	adc_cfg &= ~(ADC_CLK_DIV_MASK | ADC_SAMPLE_MODE_MASK);
 	adc_cfg |= ADC_CLK_DIV_8 | ADC_SHORT_SAMPLE_MODE;
-	if (tsc->average_samples) {
+	if (tsc->average_enable) {
 		adc_cfg &= ~ADC_AVGS_MASK;
-		adc_cfg |= (tsc->average_samples - 1) << ADC_AVGS_SHIFT;
+		adc_cfg |= (tsc->average_select) << ADC_AVGS_SHIFT;
 	}
 	adc_cfg &= ~ADC_HARDWARE_TRIGGER;
 	writel(adc_cfg, tsc->adc_regs + REG_ADC_CFG);
@@ -132,7 +133,7 @@ static int imx6ul_adc_init(struct imx6ul_tsc *tsc)
 	/* start ADC calibration */
 	adc_gc = readl(tsc->adc_regs + REG_ADC_GC);
 	adc_gc |= ADC_CAL;
-	if (tsc->average_samples)
+	if (tsc->average_enable)
 		adc_gc |= ADC_AVGE;
 	writel(adc_gc, tsc->adc_regs + REG_ADC_GC);
 
@@ -362,6 +363,7 @@ static int imx6ul_tsc_probe(struct platform_device *pdev)
 	int err;
 	int tsc_irq;
 	int adc_irq;
+	u32 average_samples;
 
 	tsc = devm_kzalloc(&pdev->dev, sizeof(*tsc), GFP_KERNEL);
 	if (!tsc)
@@ -466,14 +468,36 @@ static int imx6ul_tsc_probe(struct platform_device *pdev)
 	if (err)
 		tsc->pre_charge_time = 0xfff;
 
-	err = of_property_read_u32(np, "average-samples",
-				   &tsc->average_samples);
+	err = of_property_read_u32(np, "touchscreen-average-samples",
+				   &average_samples);
 	if (err)
-		tsc->average_samples = 0;
-
-	if (tsc->average_samples > 4) {
-		dev_err(&pdev->dev, "average-samples (%u) must be [0-4]\n",
-			tsc->average_samples);
+		average_samples = 1;
+
+	switch (average_samples) {
+	case 1:
+		tsc->average_enable = false;
+		tsc->average_select = 0; /* value unused; initialize anyway */
+		break;
+	case 4:
+		tsc->average_enable = true;
+		tsc->average_select = 0;
+		break;
+	case 8:
+		tsc->average_enable = true;
+		tsc->average_select = 1;
+		break;
+	case 16:
+		tsc->average_enable = true;
+		tsc->average_select = 2;
+		break;
+	case 32:
+		tsc->average_enable = true;
+		tsc->average_select = 3;
+		break;
+	default:
+		dev_err(&pdev->dev,
+			"touchscreen-average-samples (%u) must be 1, 4, 8, 16 or 32\n",
+			average_samples);
 		return -EINVAL;
 	}
 
-- 
2.1.4

^ permalink raw reply related

* [RFC v3 PATCH 00/25] Allow NOMMU for MULTIPLATFORM
From: Afzal Mohammed @ 2016-12-11  7:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <878trntrig.fsf@dell.be.48ers.dk>

Hi,

On Sat, Dec 10, 2016 at 10:15:35PM +0100, Peter Korsgaard wrote:
> >>>>> "Afzal" == Afzal Mohammed <afzal.mohd.ma@gmail.com> writes:

>  > Seems no !MMU A-class suitable toolchains are available, the existing
>  > !MMU toolchains available are for either M or R class. Looks like
>  > FDPIC one's also are so (cc'ing Maxime & Mickeal in case they have
>  > some input here).
> 
>  > And compiling a suitable compiler failed.
> 
>  > Let know if any signposts to travel the remaining distance.

> You can build a toolchain and initramfs with Buildroot. Have a look at
> the stm32f429 nommu config:
> 
> https://git.buildroot.net/buildroot/tree/configs/stm32f429_disco_defconfig

iiuc, it builds one for Cortex-M. i already had a file system w/
busybox compiled using a Cortex-M toolchain (stolen from
Pengutronix's OSELAS.Toolchain), which works on Cortex M4 (Vybrid
VF610 M4 core). But it does not work here, i.e. on Cortex A, seems the
above mentioned also would have the same effect. And in buildroot,
couldn't see Cortex R option in menuconfig, and selecting Cortex-A's
excludes flat binary target & presents only with ELF.

Here compiler was attempted to be built using crosstool-NG, target
selected was "arm-unknown-linux-gnueabi", and in crosstool-NG
menuconfig, MMU was deselected, upon building target got changed to
"arm-unknown-linux-uclibcgnueabi". After building binutils & pass 1
compiler, it failed while building multilib. Probably would have to
dive into toolchain details now.

Regards
afzal

^ permalink raw reply

* [RFC v3 PATCH 00/25] Allow NOMMU for MULTIPLATFORM
From: Afzal Mohammed @ 2016-12-11  6:33 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <2683998.OKe6qmCH0L@wuerfel>

Hi,

On Sat, Dec 10, 2016 at 08:19:19PM +0100, Arnd Bergmann wrote:
> On Saturday, December 10, 2016 11:46:39 PM CET Afzal Mohammed wrote:

> > Seems no !MMU A-class suitable toolchains are available, the existing
> > !MMU toolchains available are for either M or R class. Looks like
> > FDPIC one's also are so (cc'ing Maxime & Mickeal in case they have
> > some input here).
> > 
> > And compiling a suitable compiler failed.
> > 
> > Let know if any signposts to travel the remaining distance.

> What's wrong with the R class toolchain?

Yes, hoping that R class toochain for Linux would help here. After a
sleep, the same thing came to my mind.

The search for R class toolchains turned up baremetal ones only
(including Linaro's), but seems there are people using R class with
Linux, so the journey now is to get/create R class Linux toolchain.

Regards
afzal

^ permalink raw reply

* [PATCH v5 3/3] fpga manager: Add cyclone-ps-spi driver for Altera FPGAs
From: kbuild test robot @ 2016-12-11  6:13 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1778a82d1989d13919b24e47fa09eeb56b2cb8e5.1481139171.git.stillcompiling@gmail.com>

Hi Joshua,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.9-rc8]
[cannot apply to next-20161209]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Joshua-Clayton/lib-add-bitrev8x4/20161208-070638
config: arm64-allmodconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm64 

All errors (new ones prefixed by >>):

   In file included from drivers/fpga/cyclone-ps-spi.c:13:0:
   drivers/fpga/cyclone-ps-spi.c: In function 'rev_buf':
>> include/linux/bitrev.h:12:21: error: implicit declaration of function '__arch_bitrev8x4' [-Werror=implicit-function-declaration]
    #define __bitrev8x4 __arch_bitrev8x4
                        ^
   include/linux/bitrev.h:101:2: note: in expansion of macro '__bitrev8x4'
     __bitrev8x4(__x);    \
     ^~~~~~~~~~~
   drivers/fpga/cyclone-ps-spi.c:84:11: note: in expansion of macro 'bitrev8x4'
      *fw32 = bitrev8x4(*fw32);
              ^~~~~~~~~
   In file included from include/linux/delay.h:10:0,
                    from drivers/fpga/cyclone-ps-spi.c:14:
   drivers/fpga/cyclone-ps-spi.c: In function 'cyclonespi_write':
   include/linux/kernel.h:739:16: warning: comparison of distinct pointer types lacks a cast
     (void) (&min1 == &min2);   \
                   ^
   include/linux/kernel.h:742:2: note: in expansion of macro '__min'
     __min(typeof(x), typeof(y),   \
     ^~~~~
   drivers/fpga/cyclone-ps-spi.c:98:19: note: in expansion of macro 'min'
      size_t stride = min(fw_data_end - fw_data, SZ_4K);
                      ^~~
   cc1: some warnings being treated as errors

vim +/__arch_bitrev8x4 +12 include/linux/bitrev.h

556d2f05 Yalin Wang     2014-11-03   6  #ifdef CONFIG_HAVE_ARCH_BITREVERSE
556d2f05 Yalin Wang     2014-11-03   7  #include <asm/bitrev.h>
556d2f05 Yalin Wang     2014-11-03   8  
556d2f05 Yalin Wang     2014-11-03   9  #define __bitrev32 __arch_bitrev32
556d2f05 Yalin Wang     2014-11-03  10  #define __bitrev16 __arch_bitrev16
556d2f05 Yalin Wang     2014-11-03  11  #define __bitrev8 __arch_bitrev8
533d0eab Joshua Clayton 2016-12-07 @12  #define __bitrev8x4 __arch_bitrev8x4
a5cfc1ec Akinobu Mita   2006-12-08  13  
556d2f05 Yalin Wang     2014-11-03  14  #else
556d2f05 Yalin Wang     2014-11-03  15  extern u8 const byte_rev_table[256];

:::::: The code at line 12 was first introduced by commit
:::::: 533d0eabff8f37edfdec7fdf6e705fdecb297daa lib: add bitrev8x4()

:::::: TO: Joshua Clayton <stillcompiling@gmail.com>
:::::: CC: 0day robot <fengguang.wu@intel.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 52456 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161211/4a13661a/attachment-0001.gz>

^ permalink raw reply

* [PATCH v1] watchdog: imx2: fix hang-up on boot for i.MX21, i.MX27 and i.MX31 SoCs
From: Guenter Roeck @ 2016-12-11  2:18 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161211010648.6591-1-vz@mleia.com>

On 12/10/2016 05:06 PM, Vladimir Zapolskiy wrote:
> Power down counter enable/disable bit switch is located in WMCR
> register, but watchdog controllers found on legacy i.MX21, i.MX27 and
> i.MX31 SoCs don't have this register. As a result of writing data to
> the non-existing register on driver probe the SoC hangs up, to fix the
> problem add more OF compatible strings and on this basis get
> information about availability of the WMCR register.
>
> Fixes: 5fe65ce7ccbb ("watchdog: imx2_wdt: Disable power down counter on boot")
> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> Changes from RFC to v1:
> * replaced private data struct with a macro as suggested by Guenter,
> * updated the comment in the source code to reflect the change,
> * rearranged and simplified the logic of detecting WMCR presence,
> * pulled the fix out from the series with optional proposed DTS changes.
>
>  drivers/watchdog/imx2_wdt.c | 30 +++++++++++++++++++++++++-----
>  1 file changed, 25 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/watchdog/imx2_wdt.c b/drivers/watchdog/imx2_wdt.c
> index 4874b0f..66bd454 100644
> --- a/drivers/watchdog/imx2_wdt.c
> +++ b/drivers/watchdog/imx2_wdt.c
> @@ -30,6 +30,7 @@
>  #include <linux/module.h>
>  #include <linux/moduleparam.h>
>  #include <linux/of_address.h>
> +#include <linux/of_device.h>
>  #include <linux/platform_device.h>
>  #include <linux/regmap.h>
>  #include <linux/watchdog.h>
> @@ -57,6 +58,7 @@
>  #define IMX2_WDT_WICR_WICT	0xFF		/* -> Interrupt Count Timeout */
>
>  #define IMX2_WDT_WMCR		0x08		/* Misc Register */
> +#define IMX2_WDT_NO_WMCR	((void *)true)	/* Indicates unavailable WMCR */
>
>  #define IMX2_WDT_MAX_TIME	128
>  #define IMX2_WDT_DEFAULT_TIME	60		/* in seconds */
> @@ -70,6 +72,8 @@ struct imx2_wdt_device {
>  	bool ext_reset;
>  };
>
> +static const struct of_device_id imx2_wdt_dt_ids[];
> +
>  static bool nowayout = WATCHDOG_NOWAYOUT;
>  module_param(nowayout, bool, 0);
>  MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
> @@ -242,6 +246,7 @@ static const struct regmap_config imx2_wdt_regmap_config = {
>
>  static int __init imx2_wdt_probe(struct platform_device *pdev)
>  {
> +	const struct of_device_id *of_id;
>  	struct imx2_wdt_device *wdev;
>  	struct watchdog_device *wdog;
>  	struct resource *res;
> @@ -310,11 +315,13 @@ static int __init imx2_wdt_probe(struct platform_device *pdev)
>  	}
>
>  	/*
> -	 * Disable the watchdog power down counter at boot. Otherwise the power
> -	 * down counter will pull down the #WDOG interrupt line for one clock
> -	 * cycle.
> +	 * If watchdog controller has WMCR, disable the watchdog power
> +	 * down counter at boot. Otherwise the power down counter will
> +	 * pull down the #WDOG interrupt line for one clock cycle.
>  	 */
> -	regmap_write(wdev->regmap, IMX2_WDT_WMCR, 0);
> +	of_id = of_match_device(imx2_wdt_dt_ids, &pdev->dev);
> +	if (of_id && !of_id->data)
> +		regmap_write(wdev->regmap, IMX2_WDT_WMCR, 0);
>
>  	ret = watchdog_register_device(wdog);
>  	if (ret) {
> @@ -412,7 +419,20 @@ static SIMPLE_DEV_PM_OPS(imx2_wdt_pm_ops, imx2_wdt_suspend,
>  			 imx2_wdt_resume);
>
>  static const struct of_device_id imx2_wdt_dt_ids[] = {
> -	{ .compatible = "fsl,imx21-wdt", },
> +	{ .compatible = "fsl,imx21-wdt", .data = IMX2_WDT_NO_WMCR },
> +	{ .compatible = "fsl,imx25-wdt",  },
> +	{ .compatible = "fsl,imx27-wdt", .data = IMX2_WDT_NO_WMCR },
> +	{ .compatible = "fsl,imx31-wdt", .data = IMX2_WDT_NO_WMCR },
> +	{ .compatible = "fsl,imx35-wdt",  },
> +	{ .compatible = "fsl,imx50-wdt",  },
> +	{ .compatible = "fsl,imx51-wdt",  },
> +	{ .compatible = "fsl,imx53-wdt",  },
> +	{ .compatible = "fsl,imx6q-wdt",  },
> +	{ .compatible = "fsl,imx6sl-wdt", },
> +	{ .compatible = "fsl,imx6sx-wdt", },
> +	{ .compatible = "fsl,imx6ul-wdt", },
> +	{ .compatible = "fsl,imx7d-wdt",  },
> +	{ .compatible = "fsl,vf610-wdt",  },
>  	{ /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, imx2_wdt_dt_ids);
>

^ permalink raw reply

* [RFC v3 00/10] KVM PCIe/MSI passthrough on ARM/ARM64 and IOVA reserved regions
From: Don Dutile @ 2016-12-11  2:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <c5d4efa7-699d-4aa3-44cc-4ce03d0ce185@redhat.com>

On 12/08/2016 04:36 AM, Auger Eric wrote:
> Hi,
>
> On 15/11/2016 14:09, Eric Auger wrote:
>> Following LPC discussions, we now report reserved regions through
>> iommu-group sysfs reserved_regions attribute file.
>
>
> While I am respinning this series into v4, here is a tentative summary
> of technical topics for which no consensus was reached at this point.
>
> 1) Shall we report the usable IOVA range instead of reserved IOVA
>     ranges. Not discussed at/after LPC.
>     x I currently report reserved regions. Alex expressed the need to
>       report the full usable IOVA range instead (x86 min-max range
>       minus MSI APIC window). I think this is meaningful for ARM
>       too where arm-smmu might not support the full 64b range.
>     x Any objection we report the usable IOVA regions instead?
>
> 2) Shall the kernel check collision with MSI window* when userspace
>     calls VFIO_IOMMU_MAP_DMA?
>     Joerg/Will No; Alex yes
>     *for IOVA regions consumed downstream to the IOMMU: everyone says NO
>
> 3) RMRR reporting in the iommu group sysfs? Joerg: yes; Don: no
Um, I'm missing context, but the only thing I recall saying no to wrt RMRR
is that _any_ device that has an RMRR cannot be assigned to a guest.
Or, are you saying, RMRR's should be exposed in the guest os?  if so, then
you have my 'no' there.

>     My current series does not expose them in iommu group sysfs.
>     I understand we can expose the RMRR regions in the iomm group sysfs
>     without necessarily supporting RMRR requiring device assignment.
This sentence doesn't make sense to me.
Can you try re-wording it?
I can't tell what RMRR has to do w/device assignment, other than what I said above.
Exposing RMRR's in sysfs is not an issue in general.

>     We can also add this support later.
>
> Thanks
>
> Eric
>
>
>>
>> Reserved regions are populated through the IOMMU get_resv_region callback
>> (former get_dm_regions), now implemented by amd-iommu, intel-iommu and
>> arm-smmu.
>>
>> The intel-iommu reports the [FEE0_0000h - FEF0_000h] MSI window as an
>> IOMMU_RESV_NOMAP reserved region.
>>
>> arm-smmu reports the MSI window (arbitrarily located at 0x8000000 and
>> 1MB large) and the PCI host bridge windows.
>>
>> The series integrates a not officially posted patch from Robin:
>> "iommu/dma: Allow MSI-only cookies".
>>
>> This series currently does not address IRQ safety assessment.
>>
>> Best Regards
>>
>> Eric
>>
>> Git: complete series available at
>> https://github.com/eauger/linux/tree/v4.9-rc5-reserved-rfc-v3
>>
>> History:
>> RFC v2 -> v3:
>> - switch to an iommu-group sysfs API
>> - use new dummy allocator provided by Robin
>> - dummy allocator initialized by vfio-iommu-type1 after enumerating
>>    the reserved regions
>> - at the moment ARM MSI base address/size is left unchanged compared
>>    to v2
>> - we currently report reserved regions and not usable IOVA regions as
>>    requested by Alex
>>
>> RFC v1 -> v2:
>> - fix intel_add_reserved_regions
>> - add mutex lock/unlock in vfio_iommu_type1
>>
>>
>> Eric Auger (10):
>>    iommu/dma: Allow MSI-only cookies
>>    iommu: Rename iommu_dm_regions into iommu_resv_regions
>>    iommu: Add new reserved IOMMU attributes
>>    iommu: iommu_alloc_resv_region
>>    iommu: Do not map reserved regions
>>    iommu: iommu_get_group_resv_regions
>>    iommu: Implement reserved_regions iommu-group sysfs file
>>    iommu/vt-d: Implement reserved region get/put callbacks
>>    iommu/arm-smmu: Implement reserved region get/put callbacks
>>    vfio/type1: Get MSI cookie
>>
>>   drivers/iommu/amd_iommu.c       |  20 +++---
>>   drivers/iommu/arm-smmu.c        |  52 +++++++++++++++
>>   drivers/iommu/dma-iommu.c       | 116 ++++++++++++++++++++++++++-------
>>   drivers/iommu/intel-iommu.c     |  50 ++++++++++----
>>   drivers/iommu/iommu.c           | 141 ++++++++++++++++++++++++++++++++++++----
>>   drivers/vfio/vfio_iommu_type1.c |  26 ++++++++
>>   include/linux/dma-iommu.h       |   7 ++
>>   include/linux/iommu.h           |  49 ++++++++++----
>>   8 files changed, 391 insertions(+), 70 deletions(-)
>>

^ permalink raw reply

* [PATCH v1] watchdog: imx2: fix hang-up on boot for i.MX21, i.MX27 and i.MX31 SoCs
From: Vladimir Zapolskiy @ 2016-12-11  1:06 UTC (permalink / raw)
  To: linux-arm-kernel

Power down counter enable/disable bit switch is located in WMCR
register, but watchdog controllers found on legacy i.MX21, i.MX27 and
i.MX31 SoCs don't have this register. As a result of writing data to
the non-existing register on driver probe the SoC hangs up, to fix the
problem add more OF compatible strings and on this basis get
information about availability of the WMCR register.

Fixes: 5fe65ce7ccbb ("watchdog: imx2_wdt: Disable power down counter on boot")
Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
---
Changes from RFC to v1:
* replaced private data struct with a macro as suggested by Guenter,
* updated the comment in the source code to reflect the change,
* rearranged and simplified the logic of detecting WMCR presence,
* pulled the fix out from the series with optional proposed DTS changes.

 drivers/watchdog/imx2_wdt.c | 30 +++++++++++++++++++++++++-----
 1 file changed, 25 insertions(+), 5 deletions(-)

diff --git a/drivers/watchdog/imx2_wdt.c b/drivers/watchdog/imx2_wdt.c
index 4874b0f..66bd454 100644
--- a/drivers/watchdog/imx2_wdt.c
+++ b/drivers/watchdog/imx2_wdt.c
@@ -30,6 +30,7 @@
 #include <linux/module.h>
 #include <linux/moduleparam.h>
 #include <linux/of_address.h>
+#include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
 #include <linux/watchdog.h>
@@ -57,6 +58,7 @@
 #define IMX2_WDT_WICR_WICT	0xFF		/* -> Interrupt Count Timeout */
 
 #define IMX2_WDT_WMCR		0x08		/* Misc Register */
+#define IMX2_WDT_NO_WMCR	((void *)true)	/* Indicates unavailable WMCR */
 
 #define IMX2_WDT_MAX_TIME	128
 #define IMX2_WDT_DEFAULT_TIME	60		/* in seconds */
@@ -70,6 +72,8 @@ struct imx2_wdt_device {
 	bool ext_reset;
 };
 
+static const struct of_device_id imx2_wdt_dt_ids[];
+
 static bool nowayout = WATCHDOG_NOWAYOUT;
 module_param(nowayout, bool, 0);
 MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
@@ -242,6 +246,7 @@ static const struct regmap_config imx2_wdt_regmap_config = {
 
 static int __init imx2_wdt_probe(struct platform_device *pdev)
 {
+	const struct of_device_id *of_id;
 	struct imx2_wdt_device *wdev;
 	struct watchdog_device *wdog;
 	struct resource *res;
@@ -310,11 +315,13 @@ static int __init imx2_wdt_probe(struct platform_device *pdev)
 	}
 
 	/*
-	 * Disable the watchdog power down counter at boot. Otherwise the power
-	 * down counter will pull down the #WDOG interrupt line for one clock
-	 * cycle.
+	 * If watchdog controller has WMCR, disable the watchdog power
+	 * down counter at boot. Otherwise the power down counter will
+	 * pull down the #WDOG interrupt line for one clock cycle.
 	 */
-	regmap_write(wdev->regmap, IMX2_WDT_WMCR, 0);
+	of_id = of_match_device(imx2_wdt_dt_ids, &pdev->dev);
+	if (of_id && !of_id->data)
+		regmap_write(wdev->regmap, IMX2_WDT_WMCR, 0);
 
 	ret = watchdog_register_device(wdog);
 	if (ret) {
@@ -412,7 +419,20 @@ static SIMPLE_DEV_PM_OPS(imx2_wdt_pm_ops, imx2_wdt_suspend,
 			 imx2_wdt_resume);
 
 static const struct of_device_id imx2_wdt_dt_ids[] = {
-	{ .compatible = "fsl,imx21-wdt", },
+	{ .compatible = "fsl,imx21-wdt", .data = IMX2_WDT_NO_WMCR },
+	{ .compatible = "fsl,imx25-wdt",  },
+	{ .compatible = "fsl,imx27-wdt", .data = IMX2_WDT_NO_WMCR },
+	{ .compatible = "fsl,imx31-wdt", .data = IMX2_WDT_NO_WMCR },
+	{ .compatible = "fsl,imx35-wdt",  },
+	{ .compatible = "fsl,imx50-wdt",  },
+	{ .compatible = "fsl,imx51-wdt",  },
+	{ .compatible = "fsl,imx53-wdt",  },
+	{ .compatible = "fsl,imx6q-wdt",  },
+	{ .compatible = "fsl,imx6sl-wdt", },
+	{ .compatible = "fsl,imx6sx-wdt", },
+	{ .compatible = "fsl,imx6ul-wdt", },
+	{ .compatible = "fsl,imx7d-wdt",  },
+	{ .compatible = "fsl,vf610-wdt",  },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, imx2_wdt_dt_ids);
-- 
2.10.2

^ permalink raw reply related

* [PATCH 2/2] kcov: make kcov work properly with KASLR enabled
From: Alexander Popov @ 2016-12-11  0:50 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1481417456-28826-1-git-send-email-alex.popov@linux.com>

Subtract KASLR offset from the kernel addresses reported by kcov.
Tested on x86_64 and AArch64 (Hikey LeMaker).

Signed-off-by: Alexander Popov <alex.popov@linux.com>
---
 kernel/kcov.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/kernel/kcov.c b/kernel/kcov.c
index 3cbb0c8..f8f3f4c 100644
--- a/kernel/kcov.c
+++ b/kernel/kcov.c
@@ -14,6 +14,7 @@
 #include <linux/debugfs.h>
 #include <linux/uaccess.h>
 #include <linux/kcov.h>
+#include <asm/setup.h>
 
 /*
  * kcov descriptor (one per opened debugfs file).
@@ -68,6 +69,11 @@ void notrace __sanitizer_cov_trace_pc(void)
 	if (mode == KCOV_MODE_TRACE) {
 		unsigned long *area;
 		unsigned long pos;
+		unsigned long ip = _RET_IP_;
+
+#ifdef CONFIG_RANDOMIZE_BASE
+		ip -= kaslr_offset();
+#endif
 
 		/*
 		 * There is some code that runs in interrupts but for which
@@ -81,7 +87,7 @@ void notrace __sanitizer_cov_trace_pc(void)
 		/* The first word is number of subsequent PCs. */
 		pos = READ_ONCE(area[0]) + 1;
 		if (likely(pos < t->kcov_size)) {
-			area[pos] = _RET_IP_;
+			area[pos] = ip;
 			WRITE_ONCE(area[0], pos);
 		}
 	}
-- 
2.7.4

^ permalink raw reply related

* [PATCH 1/2] arm64: setup: introduce kaslr_offset()
From: Alexander Popov @ 2016-12-11  0:50 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1481417456-28826-1-git-send-email-alex.popov@linux.com>

Introduce kaslr_offset() similarly to x86_64 for fixing kcov.

Signed-off-by: Alexander Popov <alex.popov@linux.com>
---
 arch/arm64/include/asm/setup.h      | 19 +++++++++++++++++++
 arch/arm64/include/uapi/asm/setup.h |  4 ++--
 arch/arm64/kernel/setup.c           |  8 ++++----
 3 files changed, 25 insertions(+), 6 deletions(-)
 create mode 100644 arch/arm64/include/asm/setup.h

diff --git a/arch/arm64/include/asm/setup.h b/arch/arm64/include/asm/setup.h
new file mode 100644
index 0000000..e7b59b9
--- /dev/null
+++ b/arch/arm64/include/asm/setup.h
@@ -0,0 +1,19 @@
+/*
+ * arch/arm64/include/asm/setup.h
+ *
+ * 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.
+ */
+
+#ifndef __ASM_SETUP_H
+#define __ASM_SETUP_H
+
+#include <uapi/asm/setup.h>
+
+static inline unsigned long kaslr_offset(void)
+{
+	return kimage_vaddr - KIMAGE_VADDR;
+}
+
+#endif
diff --git a/arch/arm64/include/uapi/asm/setup.h b/arch/arm64/include/uapi/asm/setup.h
index 9cf2e46..26631c8 100644
--- a/arch/arm64/include/uapi/asm/setup.h
+++ b/arch/arm64/include/uapi/asm/setup.h
@@ -16,8 +16,8 @@
  * You should have received a copy of the GNU General Public License
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
-#ifndef __ASM_SETUP_H
-#define __ASM_SETUP_H
+#ifndef _UAPI__ASM_SETUP_H
+#define _UAPI__ASM_SETUP_H
 
 #include <linux/types.h>
 
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index f534f49..11eefda5 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -329,11 +329,11 @@ subsys_initcall(topology_init);
 static int dump_kernel_offset(struct notifier_block *self, unsigned long v,
 			      void *p)
 {
-	u64 const kaslr_offset = kimage_vaddr - KIMAGE_VADDR;
+	const unsigned long offset = kaslr_offset();
 
-	if (IS_ENABLED(CONFIG_RANDOMIZE_BASE) && kaslr_offset > 0) {
-		pr_emerg("Kernel Offset: 0x%llx from 0x%lx\n",
-			 kaslr_offset, KIMAGE_VADDR);
+	if (IS_ENABLED(CONFIG_RANDOMIZE_BASE) && offset > 0) {
+		pr_emerg("Kernel Offset: 0x%lx from 0x%lx\n",
+			 offset, KIMAGE_VADDR);
 	} else {
 		pr_emerg("Kernel Offset: disabled\n");
 	}
-- 
2.7.4

^ permalink raw reply related

* [PATCH 0/2] Make kcov work properly with KASLR enabled
From: Alexander Popov @ 2016-12-11  0:50 UTC (permalink / raw)
  To: linux-arm-kernel

If CONFIG_RANDOMIZE_BASE is enabled, kcov currently reports kernel addresses
including the random offset which breaks the coverage-guided fuzzing on x86_64 and
AArch64. Fix that by subtracting kaslr_offset() return value.

Alexander Popov (2):
  arm64: setup: introduce kaslr_offset()
  kcov: make kcov work properly with KASLR enabled

 arch/arm64/include/asm/setup.h      | 19 +++++++++++++++++++
 arch/arm64/include/uapi/asm/setup.h |  4 ++--
 arch/arm64/kernel/setup.c           |  8 ++++----
 kernel/kcov.c                       |  8 +++++++-
 4 files changed, 32 insertions(+), 7 deletions(-)
 create mode 100644 arch/arm64/include/asm/setup.h

-- 
2.7.4

^ permalink raw reply

* [PATCH 3/3] crypto: brcm: Add Broadcom SPU driver DT entry.
From: kbuild test robot @ 2016-12-11  0:14 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1480536453-24781-4-git-send-email-rob.rice@broadcom.com>

Hi Rob,

[auto build test ERROR on cryptodev/master]
[also build test ERROR on v4.9-rc8]
[cannot apply to next-20161209]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Rob-Rice/crypto-brcm-DT-documentation-for-Broadcom-SPU-driver/20161202-010038
base:   https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm64 

All errors (new ones prefixed by >>):

>> ERROR: Input tree has errors, aborting (use -f to force output)

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 31957 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161211/bead5da1/attachment-0001.gz>

^ permalink raw reply

* [RFC PATCH 1/2] watchdog: imx2: fix hang-up on boot for i.MX21, i.MX27 and i.MX31 SoCs
From: Vladimir Zapolskiy @ 2016-12-10 22:37 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAM=E1R6hzob5ZzyTLmrLL5s-6=vbZQbCig1sLHMSLChJEz9YgA@mail.gmail.com>

Hi Magnus,

On 12/10/2016 09:28 PM, Magnus Lilja wrote:
> Hi,
>
> On 26 September 2016 at 15:02, Guenter Roeck <linux@roeck-us.net> wrote:
>> On 09/25/2016 05:39 PM, Vladimir Zapolskiy wrote:
>>>
>>> Power down counter enable/disable bit switch is located in WMCR
>>> register, but watchdog controllers found on legacy i.MX21, i.MX27 and
>>> i.MX31 SoCs don't have this register. As a result of writing data to
>>> the non-existing register on driver probe the SoC hangs up, to fix the
>>> problem add more OF compatible strings and on this basis get
>>> information about availability of the WMCR register.
>>>
>>> Fixes: 5fe65ce7ccbb ("watchdog: imx2_wdt: Disable power down counter on
>>> boot")
>>> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
>>> ---

[snip]

>
> Has this patch (or an updated one) been merged in any tree? I've
> tested it on a i.MX31 board with positive result.
>

no, it's not in the linux-watchdog repository at the moment, I'm going
to fix Guenter's review comments and send the change today or tomorrow
in hope that it enters v4.10.

Thank you for testing, for your information I'm on the way to send more
i.MX31 changes to get better i.MX31 device tree support.

--
With best wishes,
Vladimir

^ permalink raw reply

* [RFC v3 PATCH 00/25] Allow NOMMU for MULTIPLATFORM
From: Peter Korsgaard @ 2016-12-10 21:15 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161210181639.GA5660@afzalpc>

>>>>> "Afzal" == Afzal Mohammed <afzal.mohd.ma@gmail.com> writes:

Hi,

 > Creating user space executables suitable for Cortex-A !MMU is the
 > distance to reach prompt.

 > Seems no !MMU A-class suitable toolchains are available, the existing
 > !MMU toolchains available are for either M or R class. Looks like
 > FDPIC one's also are so (cc'ing Maxime & Mickeal in case they have
 > some input here).

 > And compiling a suitable compiler failed.

 > Let know if any signposts to travel the remaining distance.

You can build a toolchain and initramfs with Buildroot. Have a look at
the stm32f429 nommu config:

https://git.buildroot.net/buildroot/tree/configs/stm32f429_disco_defconfig

-- 
Bye, Peter Korsgaard

^ permalink raw reply

* [RFC PATCH 7/7] KVM: arm/arm64: Guard kvm_vgic_map_is_active against !vgic_initialized
From: Christoffer Dall @ 2016-12-10 20:47 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161210204712.21830-1-christoffer.dall@linaro.org>

If the vgic is not initialized, don't try to grab its spinlocks or
traverse its data structures.

Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
---
 virt/kvm/arm/vgic/vgic.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
index 67d231d..a53e215 100644
--- a/virt/kvm/arm/vgic/vgic.c
+++ b/virt/kvm/arm/vgic/vgic.c
@@ -733,6 +733,9 @@ bool kvm_vgic_map_is_active(struct kvm_vcpu *vcpu, unsigned int virt_irq)
 	struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, virt_irq);
 	bool map_is_active;
 
+	if (!vgic_initialized(vcpu->kvm))
+		return false;
+
 	DEBUG_SPINLOCK_BUG_ON(!irqs_disabled());
 
 	spin_lock(&irq->irq_lock);
-- 
2.9.0

^ permalink raw reply related

* [RFC PATCH 6/7] KVM: arm/arm64: Remove unnecessary timer BUG_ON operations
From: Christoffer Dall @ 2016-12-10 20:47 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161210204712.21830-1-christoffer.dall@linaro.org>

We don't need BUG_ON operations for the timer code here anymore as this
is not a likely case to get wrong and they are in the critical path so
may potentially add overhead.

Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
---
 virt/kvm/arm/arch_timer.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
index beede1b..242728a 100644
--- a/virt/kvm/arm/arch_timer.c
+++ b/virt/kvm/arm/arch_timer.c
@@ -367,8 +367,6 @@ void kvm_timer_sync_hwstate(struct kvm_vcpu *vcpu)
 {
 	struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
 
-	BUG_ON(timer_is_armed(timer));
-
 	/*
 	 * If we entered the guest with the timer output asserted we have to
 	 * check if the guest has modified the timer so that we should lower
-- 
2.9.0

^ permalink raw reply related

* [RFC PATCH 5/7] KVM: arm/arm64: Move timer save/restore out of hyp code where possible
From: Christoffer Dall @ 2016-12-10 20:47 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161210204712.21830-1-christoffer.dall@linaro.org>

We can access the virtual timer registers from EL1 so there's no need to
do this from the hyp path.  This gives us some advantages in being able to
optimize our handling of timer registers.

We don't adjust the cntvoff during vcpu execution so we add a hyp hook
to be able to call into hyp mode for setting the cntvoff whenever we
load/put the timer, but we don't have to touch this register on every
entry/exit to the VM.

On VHE systems we also don't need to enable and disable physical counter
access traps on every trip to the VM, we can just do that when we load a
VCPU and put a VCPU.

Therefore we factor out the trap configuration from the save/restore of
the virtual timer state, which is moved to the main arch timer file and
use static keys to avoid touching the trap configuration registers on
VHE systems.

We can now leave the timer running on exiting the guest, and handle
interrupts from the vtimer directly and inject the interrupt as a
virtual interrupt to the GIC directly from the ISR.  An added benefit is
that we no longer have to actively write the active state to the
physical distributor, because we set the affinity of the vtimer
interrupt when loading the timer state, so that the interrupt
automatically stays active after firing.

Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
---
 arch/arm/include/asm/kvm_asm.h   |   2 +
 arch/arm/include/asm/kvm_hyp.h   |   4 +-
 arch/arm/kvm/arm.c               |  12 ++-
 arch/arm/kvm/hyp/switch.c        |   5 +-
 arch/arm64/include/asm/kvm_asm.h |   2 +
 arch/arm64/include/asm/kvm_hyp.h |   4 +-
 arch/arm64/kvm/hyp/switch.c      |   4 +-
 include/kvm/arm_arch_timer.h     |   7 +-
 virt/kvm/arm/arch_timer.c        | 190 +++++++++++++++++++++++++--------------
 virt/kvm/arm/hyp/timer-sr.c      |  32 ++-----
 10 files changed, 156 insertions(+), 106 deletions(-)

diff --git a/arch/arm/include/asm/kvm_asm.h b/arch/arm/include/asm/kvm_asm.h
index 8ef0538..1eabfe2 100644
--- a/arch/arm/include/asm/kvm_asm.h
+++ b/arch/arm/include/asm/kvm_asm.h
@@ -68,6 +68,8 @@ extern void __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa);
 extern void __kvm_tlb_flush_vmid(struct kvm *kvm);
 extern void __kvm_tlb_flush_local_vmid(struct kvm_vcpu *vcpu);
 
+extern void __kvm_timer_set_cntvoff(u32 cntvoff_low, u32 cntvoff_high);
+
 extern int __kvm_vcpu_run(struct kvm_vcpu *vcpu);
 
 extern void __init_stage2_translation(void);
diff --git a/arch/arm/include/asm/kvm_hyp.h b/arch/arm/include/asm/kvm_hyp.h
index 5850890..92db213 100644
--- a/arch/arm/include/asm/kvm_hyp.h
+++ b/arch/arm/include/asm/kvm_hyp.h
@@ -98,8 +98,8 @@
 #define cntvoff_el2			CNTVOFF
 #define cnthctl_el2			CNTHCTL
 
-void __timer_save_state(struct kvm_vcpu *vcpu);
-void __timer_restore_state(struct kvm_vcpu *vcpu);
+void __timer_enable_traps(struct kvm_vcpu *vcpu);
+void __timer_disable_traps(struct kvm_vcpu *vcpu);
 
 void __vgic_v2_save_state(struct kvm_vcpu *vcpu);
 void __vgic_v2_restore_state(struct kvm_vcpu *vcpu);
diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index 6591af7..582e2f7 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -347,10 +347,13 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 	vcpu->arch.host_cpu_context = this_cpu_ptr(kvm_host_cpu_state);
 
 	kvm_arm_set_running_vcpu(vcpu);
+	kvm_timer_vcpu_load(vcpu);
 }
 
 void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
 {
+	kvm_timer_vcpu_put(vcpu);
+
 	/*
 	 * The arch-generic KVM code expects the cpu field of a vcpu to be -1
 	 * if the vcpu is no longer assigned to a cpu.  This is used for the
@@ -359,7 +362,6 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
 	vcpu->cpu = -1;
 
 	kvm_arm_set_running_vcpu(NULL);
-	kvm_timer_vcpu_put(vcpu);
 }
 
 int kvm_arch_vcpu_ioctl_get_mpstate(struct kvm_vcpu *vcpu,
@@ -645,7 +647,6 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
 			vcpu->arch.power_off || vcpu->arch.pause) {
 			local_irq_enable();
 			kvm_pmu_sync_hwstate(vcpu);
-			kvm_timer_sync_hwstate(vcpu);
 			kvm_vgic_sync_hwstate(vcpu);
 			preempt_enable();
 			continue;
@@ -671,6 +672,12 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
 		kvm_arm_clear_debug(vcpu);
 
 		/*
+		 * Sync timer state before enabling interrupts as the sync
+		 * must not collide with a timer interrupt.
+		 */
+		kvm_timer_sync_hwstate(vcpu);
+
+		/*
 		 * We may have taken a host interrupt in HYP mode (ie
 		 * while executing the guest). This interrupt is still
 		 * pending, as we haven't serviced it yet!
@@ -699,7 +706,6 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
 		 * interrupt line.
 		 */
 		kvm_pmu_sync_hwstate(vcpu);
-		kvm_timer_sync_hwstate(vcpu);
 
 		kvm_vgic_sync_hwstate(vcpu);
 
diff --git a/arch/arm/kvm/hyp/switch.c b/arch/arm/kvm/hyp/switch.c
index 92678b7..f4f3ebf 100644
--- a/arch/arm/kvm/hyp/switch.c
+++ b/arch/arm/kvm/hyp/switch.c
@@ -172,7 +172,7 @@ int __hyp_text __kvm_vcpu_run(struct kvm_vcpu *vcpu)
 	__activate_vm(vcpu);
 
 	__vgic_restore_state(vcpu);
-	__timer_restore_state(vcpu);
+	__timer_enable_traps(vcpu);
 
 	__sysreg_restore_state(guest_ctxt);
 	__banked_restore_state(guest_ctxt);
@@ -189,7 +189,8 @@ int __hyp_text __kvm_vcpu_run(struct kvm_vcpu *vcpu)
 
 	__banked_save_state(guest_ctxt);
 	__sysreg_save_state(guest_ctxt);
-	__timer_save_state(vcpu);
+	__timer_disable_traps(vcpu);
+
 	__vgic_save_state(vcpu);
 
 	__deactivate_traps(vcpu);
diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
index ec3553eb..5daf476 100644
--- a/arch/arm64/include/asm/kvm_asm.h
+++ b/arch/arm64/include/asm/kvm_asm.h
@@ -56,6 +56,8 @@ extern void __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa);
 extern void __kvm_tlb_flush_vmid(struct kvm *kvm);
 extern void __kvm_tlb_flush_local_vmid(struct kvm_vcpu *vcpu);
 
+extern void __kvm_timer_set_cntvoff(u32 cntvoff_low, u32 cntvoff_high);
+
 extern int __kvm_vcpu_run(struct kvm_vcpu *vcpu);
 
 extern u64 __vgic_v3_get_ich_vtr_el2(void);
diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
index b18e852..7bbc717 100644
--- a/arch/arm64/include/asm/kvm_hyp.h
+++ b/arch/arm64/include/asm/kvm_hyp.h
@@ -128,8 +128,8 @@ int __vgic_v2_perform_cpuif_access(struct kvm_vcpu *vcpu);
 void __vgic_v3_save_state(struct kvm_vcpu *vcpu);
 void __vgic_v3_restore_state(struct kvm_vcpu *vcpu);
 
-void __timer_save_state(struct kvm_vcpu *vcpu);
-void __timer_restore_state(struct kvm_vcpu *vcpu);
+void __timer_enable_traps(struct kvm_vcpu *vcpu);
+void __timer_disable_traps(struct kvm_vcpu *vcpu);
 
 void __sysreg_save_host_state(struct kvm_cpu_context *ctxt);
 void __sysreg_restore_host_state(struct kvm_cpu_context *ctxt);
diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index 8bcae7b..db920a34 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -281,7 +281,7 @@ int __hyp_text __kvm_vcpu_run(struct kvm_vcpu *vcpu)
 	__activate_vm(vcpu);
 
 	__vgic_restore_state(vcpu);
-	__timer_restore_state(vcpu);
+	__timer_enable_traps(vcpu);
 
 	/*
 	 * We must restore the 32-bit state before the sysregs, thanks
@@ -337,7 +337,7 @@ int __hyp_text __kvm_vcpu_run(struct kvm_vcpu *vcpu)
 
 	__sysreg_save_guest_state(guest_ctxt);
 	__sysreg32_save_state(vcpu);
-	__timer_save_state(vcpu);
+	__timer_disable_traps(vcpu);
 	__vgic_save_state(vcpu);
 
 	__deactivate_traps(vcpu);
diff --git a/include/kvm/arm_arch_timer.h b/include/kvm/arm_arch_timer.h
index 2d54903..3e518a6 100644
--- a/include/kvm/arm_arch_timer.h
+++ b/include/kvm/arm_arch_timer.h
@@ -50,11 +50,12 @@ struct arch_timer_cpu {
 	/* Timer IRQ */
 	struct kvm_irq_level		irq;
 
-	/* Active IRQ state caching */
-	bool				active_cleared_last;
 
 	/* Is the timer enabled */
 	bool			enabled;
+
+	/* Is the timer state loaded on the hardware timer */
+	bool			loaded;
 };
 
 int kvm_timer_hyp_init(void);
@@ -74,7 +75,9 @@ bool kvm_timer_should_fire(struct kvm_vcpu *vcpu);
 void kvm_timer_schedule(struct kvm_vcpu *vcpu);
 void kvm_timer_unschedule(struct kvm_vcpu *vcpu);
 
+void kvm_timer_vcpu_load(struct kvm_vcpu *vcpu);
 void kvm_timer_vcpu_put(struct kvm_vcpu *vcpu);
 
 void kvm_timer_init_vhe(void);
+
 #endif
diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
index f27a086..beede1b 100644
--- a/virt/kvm/arm/arch_timer.c
+++ b/virt/kvm/arm/arch_timer.c
@@ -35,10 +35,8 @@ static struct timecounter *timecounter;
 static unsigned int host_vtimer_irq;
 static u32 host_vtimer_irq_flags;
 
-void kvm_timer_vcpu_put(struct kvm_vcpu *vcpu)
-{
-	vcpu->arch.timer_cpu.active_cleared_last = false;
-}
+static bool kvm_timer_irq_can_fire(struct kvm_vcpu *vcpu);
+static void kvm_timer_update_irq(struct kvm_vcpu *vcpu, bool new_level);
 
 static cycle_t kvm_phys_timer_read(void)
 {
@@ -70,14 +68,14 @@ static void timer_disarm(struct arch_timer_cpu *timer)
 static irqreturn_t kvm_arch_timer_handler(int irq, void *dev_id)
 {
 	struct kvm_vcpu *vcpu = *(struct kvm_vcpu **)dev_id;
+	struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
+
+	if (!timer->irq.level) {
+		timer->cntv_ctl = read_sysreg_el0(cntv_ctl);
+		if (kvm_timer_irq_can_fire(vcpu))
+			kvm_timer_update_irq(vcpu, true);
+	}
 
-	/*
-	 * We disable the timer in the world switch and let it be
-	 * handled by kvm_timer_sync_hwstate(). Getting a timer
-	 * interrupt at this point is a sure sign of some major
-	 * breakage.
-	 */
-	pr_warn("Unexpected interrupt %d on vcpu %p\n", irq, vcpu);
 	return IRQ_HANDLED;
 }
 
@@ -158,6 +156,16 @@ bool kvm_timer_should_fire(struct kvm_vcpu *vcpu)
 	struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
 	cycle_t cval, now;
 
+	/*
+	 * If somebody is asking if the timer should fire, but the timer state
+	 * is maintained in hardware, we need to take a snapshot of the
+	 * current hardware state first.
+	 */
+	if (timer->loaded) {
+		timer->cntv_ctl = read_sysreg_el0(cntv_ctl);
+		timer->cntv_cval = read_sysreg_el0(cntv_cval);
+	}
+
 	if (!kvm_timer_irq_can_fire(vcpu))
 		return false;
 
@@ -174,7 +182,6 @@ static void kvm_timer_update_irq(struct kvm_vcpu *vcpu, bool new_level)
 
 	BUG_ON(!vgic_initialized(vcpu->kvm));
 
-	timer->active_cleared_last = false;
 	timer->irq.level = new_level;
 	trace_kvm_timer_update_irq(vcpu->vcpu_id, timer->irq.irq,
 				   timer->irq.level);
@@ -207,6 +214,29 @@ static int kvm_timer_update_state(struct kvm_vcpu *vcpu)
 	return 0;
 }
 
+static void timer_save_state(struct kvm_vcpu *vcpu)
+{
+	struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
+	unsigned long flags;
+
+	local_irq_save(flags);
+
+	if (!timer->loaded)
+		goto out;
+
+	if (timer->enabled) {
+		timer->cntv_ctl = read_sysreg_el0(cntv_ctl);
+		timer->cntv_cval = read_sysreg_el0(cntv_cval);
+	}
+
+	/* Disable the virtual timer */
+	write_sysreg_el0(0, cntv_ctl);
+
+	timer->loaded = false;
+out:
+	local_irq_restore(flags);
+}
+
 /*
  * Schedule the background timer before calling kvm_vcpu_block, so that this
  * thread is removed from its waitqueue and made runnable when there's a timer
@@ -218,6 +248,8 @@ void kvm_timer_schedule(struct kvm_vcpu *vcpu)
 
 	BUG_ON(timer_is_armed(timer));
 
+	timer_save_state(vcpu);
+
 	/*
 	 * No need to schedule a background timer if the guest timer has
 	 * already expired, because kvm_vcpu_block will return before putting
@@ -237,77 +269,91 @@ void kvm_timer_schedule(struct kvm_vcpu *vcpu)
 	timer_arm(timer, kvm_timer_compute_delta(vcpu));
 }
 
+static void timer_restore_state(struct kvm_vcpu *vcpu)
+{
+	struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
+	unsigned long flags;
+
+	local_irq_save(flags);
+
+	if (timer->loaded)
+		goto out;
+
+	if (timer->enabled) {
+		write_sysreg_el0(timer->cntv_cval, cntv_cval);
+		isb();
+		write_sysreg_el0(timer->cntv_ctl, cntv_ctl);
+	}
+
+	timer->loaded = true;
+out:
+	local_irq_restore(flags);
+}
+
 void kvm_timer_unschedule(struct kvm_vcpu *vcpu)
 {
 	struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
 	timer_disarm(timer);
+
+	timer_restore_state(vcpu);
 }
 
-/**
- * kvm_timer_flush_hwstate - prepare to move the virt timer to the cpu
- * @vcpu: The vcpu pointer
- *
- * Check if the virtual timer has expired while we were running in the host,
- * and inject an interrupt if that was the case.
- */
-void kvm_timer_flush_hwstate(struct kvm_vcpu *vcpu)
+static void set_cntvoff(u64 cntvoff)
+{
+	u32 low = cntvoff & GENMASK(31, 0);
+	u32 high = (cntvoff >> 32) & GENMASK(31, 0);
+	kvm_call_hyp(__kvm_timer_set_cntvoff, low, high);
+}
+
+void kvm_timer_vcpu_load(struct kvm_vcpu *vcpu)
 {
 	struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
 	bool phys_active;
 	int ret;
 
-	if (kvm_timer_update_state(vcpu))
-		return;
-
-	/*
-	* If we enter the guest with the virtual input level to the VGIC
-	* asserted, then we have already told the VGIC what we need to, and
-	* we don't need to exit from the guest until the guest deactivates
-	* the already injected interrupt, so therefore we should set the
-	* hardware active state to prevent unnecessary exits from the guest.
-	*
-	* Also, if we enter the guest with the virtual timer interrupt active,
-	* then it must be active on the physical distributor, because we set
-	* the HW bit and the guest must be able to deactivate the virtual and
-	* physical interrupt at the same time.
-	*
-	* Conversely, if the virtual input level is deasserted and the virtual
-	* interrupt is not active, then always clear the hardware active state
-	* to ensure that hardware interrupts from the timer triggers a guest
-	* exit.
-	*/
-	phys_active = timer->irq.level ||
-			kvm_vgic_map_is_active(vcpu, timer->irq.irq);
-
-	/*
-	 * We want to avoid hitting the (re)distributor as much as
-	 * possible, as this is a potentially expensive MMIO access
-	 * (not to mention locks in the irq layer), and a solution for
-	 * this is to cache the "active" state in memory.
-	 *
-	 * Things to consider: we cannot cache an "active set" state,
-	 * because the HW can change this behind our back (it becomes
-	 * "clear" in the HW). We must then restrict the caching to
-	 * the "clear" state.
-	 *
-	 * The cache is invalidated on:
-	 * - vcpu put, indicating that the HW cannot be trusted to be
-	 *   in a sane state on the next vcpu load,
-	 * - any change in the interrupt state
-	 *
-	 * Usage conditions:
-	 * - cached value is "active clear"
-	 * - value to be programmed is "active clear"
-	 */
-	if (timer->active_cleared_last && !phys_active)
-		return;
+	ret = irq_set_vcpu_affinity(host_vtimer_irq, vcpu);
+	WARN(ret, "irq_set_vcpu_affinity ret %d\n", ret);
 
+	if (timer->irq.level || kvm_vgic_map_is_active(vcpu, timer->irq.irq))
+		phys_active = true;
+	else
+		phys_active = false;
 	ret = irq_set_irqchip_state(host_vtimer_irq,
 				    IRQCHIP_STATE_ACTIVE,
 				    phys_active);
 	WARN_ON(ret);
 
-	timer->active_cleared_last = !phys_active;
+	set_cntvoff(vcpu->kvm->arch.timer.cntvoff);
+	timer_restore_state(vcpu);
+
+	if (is_kernel_in_hyp_mode())
+		__timer_enable_traps(vcpu);
+}
+
+void kvm_timer_vcpu_put(struct kvm_vcpu *vcpu)
+{
+	int ret;
+
+	if (is_kernel_in_hyp_mode())
+		__timer_disable_traps(vcpu);
+
+	timer_save_state(vcpu);
+
+	set_cntvoff(0);
+
+	ret = irq_set_vcpu_affinity(host_vtimer_irq, NULL);
+	WARN(ret, "irq_set_vcpu_affinity ret %d\n", ret);
+}
+
+/**
+ * kvm_timer_flush_hwstate - prepare to move the virt timer to the cpu
+ * @vcpu: The vcpu pointer
+ *
+ * Check if the virtual timer has expired while we were running in the host,
+ * and inject an interrupt if that was the case.
+ */
+void kvm_timer_flush_hwstate(struct kvm_vcpu *vcpu)
+{
 }
 
 /**
@@ -324,10 +370,16 @@ void kvm_timer_sync_hwstate(struct kvm_vcpu *vcpu)
 	BUG_ON(timer_is_armed(timer));
 
 	/*
-	 * The guest could have modified the timer registers or the timer
-	 * could have expired, update the timer state.
+	 * If we entered the guest with the timer output asserted we have to
+	 * check if the guest has modified the timer so that we should lower
+	 * the line at this point.
 	 */
-	kvm_timer_update_state(vcpu);
+	if (timer->irq.level) {
+		timer->cntv_ctl = read_sysreg_el0(cntv_ctl);
+		timer->cntv_cval = read_sysreg_el0(cntv_cval);
+		if (!kvm_timer_should_fire(vcpu))
+			kvm_timer_update_irq(vcpu, false);
+	}
 }
 
 int kvm_timer_vcpu_reset(struct kvm_vcpu *vcpu,
diff --git a/virt/kvm/arm/hyp/timer-sr.c b/virt/kvm/arm/hyp/timer-sr.c
index 63e28dd..3786ac65 100644
--- a/virt/kvm/arm/hyp/timer-sr.c
+++ b/virt/kvm/arm/hyp/timer-sr.c
@@ -21,19 +21,15 @@
 
 #include <asm/kvm_hyp.h>
 
-/* vcpu is already in the HYP VA space */
-void __hyp_text __timer_save_state(struct kvm_vcpu *vcpu)
+void __hyp_text __kvm_timer_set_cntvoff(u32 cntvoff_low, u32 cntvoff_high)
 {
-	struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
-	u64 val;
-
-	if (timer->enabled) {
-		timer->cntv_ctl = read_sysreg_el0(cntv_ctl);
-		timer->cntv_cval = read_sysreg_el0(cntv_cval);
-	}
+	u64 cntvoff = (u64)cntvoff_high << 32 | cntvoff_low;
+	write_sysreg(cntvoff, cntvoff_el2);
+}
 
-	/* Disable the virtual timer */
-	write_sysreg_el0(0, cntv_ctl);
+void __hyp_text __timer_disable_traps(struct kvm_vcpu *vcpu)
+{
+	u64 val;
 
 	/*
 	 * We don't need to do this for VHE since the host kernel runs in EL2
@@ -45,15 +41,10 @@ void __hyp_text __timer_save_state(struct kvm_vcpu *vcpu)
 		val |= CNTHCTL_EL1PCTEN | CNTHCTL_EL1PCEN;
 		write_sysreg(val, cnthctl_el2);
 	}
-
-	/* Clear cntvoff for the host */
-	write_sysreg(0, cntvoff_el2);
 }
 
-void __hyp_text __timer_restore_state(struct kvm_vcpu *vcpu)
+void __hyp_text __timer_enable_traps(struct kvm_vcpu *vcpu)
 {
-	struct kvm *kvm = kern_hyp_va(vcpu->kvm);
-	struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
 	u64 val;
 
 	/* Those bits are already configured at boot on VHE-system */
@@ -67,11 +58,4 @@ void __hyp_text __timer_restore_state(struct kvm_vcpu *vcpu)
 		val |= CNTHCTL_EL1PCTEN;
 		write_sysreg(val, cnthctl_el2);
 	}
-
-	if (timer->enabled) {
-		write_sysreg(kvm->arch.timer.cntvoff, cntvoff_el2);
-		write_sysreg_el0(timer->cntv_cval, cntv_cval);
-		isb();
-		write_sysreg_el0(timer->cntv_ctl, cntv_ctl);
-	}
 }
-- 
2.9.0

^ permalink raw reply related

* [RFC PATCH 4/7] KVM: arm/arm64: Check that system supports split eoi/deactivate
From: Christoffer Dall @ 2016-12-10 20:47 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161210204712.21830-1-christoffer.dall@linaro.org>

Some systems without proper firmware and/or hardware description data
don't support the split EOI and deactivate operation and therefore
don't provide an irq_set_vcpu_affinity implementation.  On such
systems, we cannot leave the physical interrupt active after the timer
handler on the host has run, so we cannot support KVM with the timer
changes we about to introduce.

Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
---
 virt/kvm/arm/arch_timer.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
index c7c3bfd..f27a086 100644
--- a/virt/kvm/arm/arch_timer.c
+++ b/virt/kvm/arm/arch_timer.c
@@ -418,6 +418,31 @@ static int kvm_timer_dying_cpu(unsigned int cpu)
 	return 0;
 }
 
+static bool has_split_eoi_deactivate_support(void)
+{
+	struct irq_desc *desc;
+	struct irq_data *data;
+	struct irq_chip *chip;
+
+	/*
+	 * Check if split EOI and deactivate is supported on this machine.
+	 */
+	desc = irq_to_desc(host_vtimer_irq);
+	if (!desc) {
+		kvm_err("kvm_arch_timer: no host_vtimer_irq descriptor\n");
+		return false;
+	}
+
+	data = irq_desc_get_irq_data(desc);
+	chip = irq_data_get_irq_chip(data);
+	if (!chip || !chip->irq_set_vcpu_affinity) {
+		kvm_err("kvm_arch_timer: no split EOI/deactivate; abort\n");
+		return false;
+	}
+
+	return true;
+}
+
 int kvm_timer_hyp_init(void)
 {
 	struct arch_timer_kvm_info *info;
@@ -449,6 +474,11 @@ int kvm_timer_hyp_init(void)
 		return err;
 	}
 
+	if (!has_split_eoi_deactivate_support()) {
+		disable_percpu_irq(host_vtimer_irq);
+		return -ENODEV;
+	}
+
 	kvm_info("virtual timer IRQ%d\n", host_vtimer_irq);
 
 	cpuhp_setup_state(CPUHP_AP_KVM_ARM_TIMER_STARTING,
-- 
2.9.0

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox