RE: [PATCH v2] can: rcar_canfd: Add Renesas R-Car CAN FD driver

[Ramesh Shanmugasundaram]

Hi Oliver,

Thanks for the review comments.

> On 03/03/2016 04:38 PM, Ramesh Shanmugasundaram wrote:
> 
> > Changes since v1:
> > * Removed testmodes & debugfs code (suggested by Oliver H)
> > * Fixed tx path race issue by introducing lock (suggested by Marc K)
> > * Removed __maybe_unused attribute of rcar_canfd_of_table
> >
> 
> 
> >  obj-$(CONFIG_CAN_M_CAN)+= m_can/
> >  obj-$(CONFIG_CAN_RCAR) += rcar_can.o
> > +obj-$(CONFIG_CANFD_RCAR)   += rcar_canfd.o
> 
> Just for the sake of an ordered directory structure:
> 
> Would it make sense to create a rcar directory where rcar_can.c and
> rcar_canfd.c are placed?
> 
Yes. I'll place them in rcar dir.

> Additionally (besides of one accident with the obsolete PCH_CAN) all CAN
> drivers start with CONFIG_CAN_...
> 
> Following that scheme the config option should be named
> 
>   CONFIG_CAN_RCAR_CANFD
> 
> as we had in CONFIG_CAN_IFI_CANFD.

Agreed.

> 
> >  obj-$(CONFIG_CAN_SJA1000)  += sja1000/
> >  obj-$(CONFIG_CAN_SUN4I)+= sun4i_can.o
> >  obj-$(CONFIG_CAN_TI_HECC)  += ti_hecc.o
> > diff --git a/drivers/net/can/rcar_canfd.c
> > b/drivers/net/can/rcar_canfd.c
> 
> (..)
> 
> > +/* Channel priv data */
> > +struct rcar_canfd_channel {
> > +   struct can_priv can;/* Must be the first member */
> > +   struct net_device *ndev;
> > +   struct rcar_canfd_global *gpriv;/* Controller reference */
> > +   void __iomem *base; /* Register base address */
> > +#ifdef CONFIG_DEBUG_FS
> > +   struct dentry *dev_root;
> > +#endif /* CONFIG_DEBUG_FS */
> 
> Some missed stuff from debugfs removal?

:-(. Sorry. Will clean up.

> 
> > +   struct napi_struct napi;
> > +   u8  tx_len[RCANFD_FIFO_DEPTH];  /* For net stats */
> > +   u32 tx_head;/* Incremented on xmit */
> > +   u32 tx_tail;/* Incremented on xmit done */
> > +   u32 channel;/* Channel number */
> > +   spinlock_t tx_lock; /* To protect tx path */
> > +};
> 
> (..)
> 
> > +static int rcar_canfd_start(struct net_device *ndev) {
> > +   struct rcar_canfd_channel *priv = netdev_priv(ndev);
> > +   int err = -EOPNOTSUPP;
> > +   u32 sts, ch = priv->channel;
> > +   u32 ridx = ch + RCANFD_RFFIFO_IDX;
> > +
> > +   /* Ensure channel starts in FD mode */
> > +   if (!(priv->can.ctrlmode & CAN_CTRLMODE_FD)) {
> > +   netdev_err(ndev, "enable can fd mode for channel %d\n", ch);
> > +   goto fail_mode;
> > +   }
> 
> What's the reason behind this check?
> 
> A CAN FD capable CAN controller can be either configured to run as CAN 2.0
> (Classic CAN) or as CAN FD controller.
> 
> So why are to throwing an error here and produce an initialization
> failure?

When this controller is configured in FD mode and used only with CAN 2.0 nodes, 
it still expects a DTSEG (data bitrate) configuration same as NTSEG (nominal 
bitrate). This check, specifically in ndo_open, ensures both are configured and 
will work fine with CAN 2.0 nodes (e.g.)

"ip link set can0 up type can bitrate 100 dbitrate 100 fd on"

If I don't have this check, a configuration like this

"ip link set can0 up type can bitrate 100"

will bring up the controller without DTSEG configured. 

Thanks,
Ramesh
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] can: rcar_canfd: Add Renesas R-Car CAN FD driver

[Marc Kleine-Budde]

On 03/07/2016 09:02 AM, Ramesh Shanmugasundaram wrote:
> Hi Oliver,
> 
> Thanks for the review comments.
> 
>> On 03/03/2016 04:38 PM, Ramesh Shanmugasundaram wrote:
>>
>>> Changes since v1:
>>> * Removed testmodes & debugfs code (suggested by Oliver H)
>>> * Fixed tx path race issue by introducing lock (suggested by Marc K)
>>> * Removed __maybe_unused attribute of rcar_canfd_of_table
>>>
>>
>>
>>>  obj-$(CONFIG_CAN_M_CAN)+= m_can/
>>>  obj-$(CONFIG_CAN_RCAR) += rcar_can.o
>>> +obj-$(CONFIG_CANFD_RCAR)   += rcar_canfd.o
>>
>> Just for the sake of an ordered directory structure:
>>
>> Would it make sense to create a rcar directory where rcar_can.c and
>> rcar_canfd.c are placed?
>>
> Yes. I'll place them in rcar dir.
> 
>> Additionally (besides of one accident with the obsolete PCH_CAN) all CAN
>> drivers start with CONFIG_CAN_...
>>
>> Following that scheme the config option should be named
>>
>>  CONFIG_CAN_RCAR_CANFD
>>
>> as we had in CONFIG_CAN_IFI_CANFD.
> 
> Agreed.
> 
>>
>>>  obj-$(CONFIG_CAN_SJA1000)  += sja1000/
>>>  obj-$(CONFIG_CAN_SUN4I)+= sun4i_can.o
>>>  obj-$(CONFIG_CAN_TI_HECC)  += ti_hecc.o
>>> diff --git a/drivers/net/can/rcar_canfd.c
>>> b/drivers/net/can/rcar_canfd.c
>>
>> (..)
>>
>>> +/* Channel priv data */
>>> +struct rcar_canfd_channel {
>>> +   struct can_priv can;/* Must be the first member */
>>> +   struct net_device *ndev;
>>> +   struct rcar_canfd_global *gpriv;/* Controller reference */
>>> +   void __iomem *base; /* Register base address */
>>> +#ifdef CONFIG_DEBUG_FS
>>> +   struct dentry *dev_root;
>>> +#endif /* CONFIG_DEBUG_FS */
>>
>> Some missed stuff from debugfs removal?
> 
> :-(. Sorry. Will clean up.
> 
>>
>>> +   struct napi_struct napi;
>>> +   u8  tx_len[RCANFD_FIFO_DEPTH];  /* For net stats */
>>> +   u32 tx_head;/* Incremented on xmit */
>>> +   u32 tx_tail;/* Incremented on xmit done */
>>> +   u32 channel;/* Channel number */
>>> +   spinlock_t tx_lock; /* To protect tx path */
>>> +};
>>
>> (..)
>>
>>> +static int rcar_canfd_start(struct net_device *ndev) {
>>> +   struct rcar_canfd_channel *priv = netdev_priv(ndev);
>>> +   int err = -EOPNOTSUPP;
>>> +   u32 sts, ch = priv->channel;
>>> +   u32 ridx = ch + RCANFD_RFFIFO_IDX;
>>> +
>>> +   /* Ensure channel starts in FD mode */
>>> +   if (!(priv->can.ctrlmode & CAN_CTRLMODE_FD)) {
>>> +   netdev_err(ndev, "enable can fd mode for channel %d\n", ch);
>>> +   goto fail_mode;
>>> +   }
>>
>> What's the reason behind this check?
>>
>> A CAN FD capable CAN controller can be either configured to run as CAN 2.0
>> (Classic CAN) or as CAN FD controller.
>>
>> So why are to throwing an error here and produce an initialization
>> failure?
> 
> When this controller is configured in FD mode and used only with CAN 2.0 
> nodes, it still expects a DTSEG (data bitrate) configuration same as NTSEG 
> (nominal bitrate). This check, specifically in ndo_open, ensures both are 
> configured and will work fine with CAN 2.0 nodes (e.g.)
> 
> "ip link set can0 up type can bitrate 100 dbitrate 100 fd on"
> 
> If I don't have this check, a configuration like this
> 
> "ip link set can0 up type can bitrate 100"
> 
> will bring up the controller without DTSEG configured. 

That should bring up the controller in CAN 2.0 mode.

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

[PATCH] documentation: Make sample code and documentation consistent

[Yao Dongdong]

In the chapter 'analogy with reader-writer locking', the sample
code uses spinlock_t in reader-writer case. Just correct it so
that we can read the document easily.

Signed-off-by: Yao Dongdong 
---
 Documentation/RCU/whatisRCU.txt |   22 +++---
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/Documentation/RCU/whatisRCU.txt b/Documentation/RCU/whatisRCU.txt
index dc49c67..e33304e 100644
--- a/Documentation/RCU/whatisRCU.txt
+++ b/Documentation/RCU/whatisRCU.txt
@@ -681,22 +681,30 @@ Although RCU can be used in many different ways, a very 
common use of
 RCU is analogous to reader-writer locking.  The following unified
 diff shows how closely related RCU and reader-writer locking can be.

+   @@ -5,5 +5,5 @@ struct el {
+   int data;
+   /* Other data fields */
+   };
+   -rwlock_t listmutex;
+   +spinlock_t listmutex;
+   struct el head;
+
@@ -13,15 +14,15 @@
struct list_head *lp;
struct el *p;

-   -   read_lock();
+   -   read_lock(&listmutex);
-   list_for_each_entry(p, head, lp) {
+   rcu_read_lock();
+   list_for_each_entry_rcu(p, head, lp) {
if (p->key == key) {
*result = p->data;
-   -   read_unlock();
+   -   read_unlock(&listmutex);
+   rcu_read_unlock();
return 1;
}
}
-   -   read_unlock();
+   -   read_unlock(&listmutex);
+   rcu_read_unlock();
return 0;
 }
@@ -732,7 +740,7 @@ Or, for those who prefer a side-by-side listing:
  5   int data;  5   int data;
  6   /* Other data fields */6   /* Other data fields */
  7 };   7 };
- 8 spinlock_t listmutex;8 spinlock_t listmutex;
+ 8 rwlock_t listmutex;  8 spinlock_t listmutex;
  9 struct el head;  9 struct el head;

  1 int search(long key, int *result)1 int search(long key, int *result)
@@ -740,15 +748,15 @@ Or, for those who prefer a side-by-side listing:
  3   struct list_head *lp;  3   struct list_head *lp;
  4   struct el *p;  4   struct el *p;
  5  5
- 6   read_lock();   6   rcu_read_lock();
+ 6   read_lock(&listmutex); 6   rcu_read_lock();
  7   list_for_each_entry(p, head, lp) { 7   list_for_each_entry_rcu(p, head, 
lp) {
  8 if (p->key == key) { 8 if (p->key == key) {
  9   *result = p->data; 9   *result = p->data;
-10   read_unlock();10   rcu_read_unlock();
+10   read_unlock(&listmutex);  10   rcu_read_unlock();
 11   return 1; 11   return 1;
 12 }   12 }
 13   } 13   }
-14   read_unlock();14   rcu_read_unlock();
+14   read_unlock(&listmutex);  14   rcu_read_unlock();
 15   return 0; 15   return 0;
 16 }   16 }

--
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v2] can: rcar_canfd: Add Renesas R-Car CAN FD driver

[Ramesh Shanmugasundaram]

Hi Marc,

> On 03/07/2016 09:02 AM, Ramesh Shanmugasundaram wrote:
> > Hi Oliver,
> >
> > Thanks for the review comments.
> >
> >> On 03/03/2016 04:38 PM, Ramesh Shanmugasundaram wrote:
> >>
> >>> Changes since v1:
> >>>   * Removed testmodes & debugfs code (suggested by Oliver H)
> >>>   * Fixed tx path race issue by introducing lock (suggested by Marc K)
> >>>   * Removed __maybe_unused attribute of rcar_canfd_of_table
> >>>
> >>
> >>
> >>>  obj-$(CONFIG_CAN_M_CAN)  += m_can/
> >>>  obj-$(CONFIG_CAN_RCAR)   += rcar_can.o
> >>> +obj-$(CONFIG_CANFD_RCAR) += rcar_canfd.o
> >>
> >> Just for the sake of an ordered directory structure:
> >>
> >> Would it make sense to create a rcar directory where rcar_can.c and
> >> rcar_canfd.c are placed?
> >>
> > Yes. I'll place them in rcar dir.
> >
> >> Additionally (besides of one accident with the obsolete PCH_CAN) all
> >> CAN drivers start with CONFIG_CAN_...
> >>
> >> Following that scheme the config option should be named
> >>
> >>CONFIG_CAN_RCAR_CANFD
> >>
> >> as we had in CONFIG_CAN_IFI_CANFD.
> >
> > Agreed.
> >
> >>
> >>>  obj-$(CONFIG_CAN_SJA1000)+= sja1000/
> >>>  obj-$(CONFIG_CAN_SUN4I)  += sun4i_can.o
> >>>  obj-$(CONFIG_CAN_TI_HECC)+= ti_hecc.o
> >>> diff --git a/drivers/net/can/rcar_canfd.c
> >>> b/drivers/net/can/rcar_canfd.c
> >>
> >> (..)
> >>
> >>> +/* Channel priv data */
> >>> +struct rcar_canfd_channel {
> >>> + struct can_priv can;/* Must be the first member */
> >>> + struct net_device *ndev;
> >>> + struct rcar_canfd_global *gpriv;/* Controller reference */
> >>> + void __iomem *base; /* Register base address */
> >>> +#ifdef CONFIG_DEBUG_FS
> >>> + struct dentry *dev_root;
> >>> +#endif /* CONFIG_DEBUG_FS */
> >>
> >> Some missed stuff from debugfs removal?
> >
> > :-(. Sorry. Will clean up.
> >
> >>
> >>> + struct napi_struct napi;
> >>> + u8  tx_len[RCANFD_FIFO_DEPTH];  /* For net stats */
> >>> + u32 tx_head;/* Incremented on xmit */
> >>> + u32 tx_tail;/* Incremented on xmit done */
> >>> + u32 channel;/* Channel number */
> >>> + spinlock_t tx_lock; /* To protect tx path */
> >>> +};
> >>
> >> (..)
> >>
> >>> +static int rcar_canfd_start(struct net_device *ndev) {
> >>> + struct rcar_canfd_channel *priv = netdev_priv(ndev);
> >>> + int err = -EOPNOTSUPP;
> >>> + u32 sts, ch = priv->channel;
> >>> + u32 ridx = ch + RCANFD_RFFIFO_IDX;
> >>> +
> >>> + /* Ensure channel starts in FD mode */
> >>> + if (!(priv->can.ctrlmode & CAN_CTRLMODE_FD)) {
> >>> + netdev_err(ndev, "enable can fd mode for channel %d\n", ch);
> >>> + goto fail_mode;
> >>> + }
> >>
> >> What's the reason behind this check?
> >>
> >> A CAN FD capable CAN controller can be either configured to run as
> >> CAN 2.0 (Classic CAN) or as CAN FD controller.
> >>
> >> So why are to throwing an error here and produce an initialization
> >> failure?
> >
> > When this controller is configured in FD mode and used only with CAN
> > 2.0 nodes, it still expects a DTSEG (data bitrate) configuration same
> > as NTSEG (nominal bitrate). This check, specifically in ndo_open,
> > ensures both are configured and will work fine with CAN 2.0 nodes
> > (e.g.)
> >
> > "ip link set can0 up type can bitrate 100 dbitrate 100 fd on"
> >
> > If I don't have this check, a configuration like this
> >
> > "ip link set can0 up type can bitrate 100"
> >
> > will bring up the controller without DTSEG configured.
> 
> That should bring up the controller in CAN 2.0 mode.

Yes, that's the user's intention but the manual states DTSEG still need to be 
configured. In the above configuration, it will not be.
Besides, this will not be a "pure" CAN 2.0 node (i.e.) if a frame with length > 
8 bytes is received the controller will "ACK" because in FD mode it can receive 
up to 64 bytes frame.

The controller does support a "pure" classical CAN mode with a different set of 
register map itself. Do you think a pure CAN 2.0 mode support would be 
beneficial? I can submit this in coming days on top of current submission.

The current submission status is:
 - Controller operates in CAN FD mode only.
 - If needed to interoperate with CAN 2.0 nodes, data bitrate still need to be 
configured and it will work perfectly. However, it is not a "pure" CAN 2.0 node 
as mentioned above.

Thanks,
Ramesh

Re: [PATCH 2/2] isdn: i4l: move active-isdn drivers to staging

[Paul Bolle]

On vr, 2016-03-04 at 17:32 +0100, Arnd Bergmann wrote:
> A third patch moves the capidrv source from drivers/isdn/capi/
> into the i4l directory.

I see. Why exactly?

Thanks,


Paul Bolle
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] isdn: i4l: move active-isdn drivers to staging

[Paul Bolle]

On za, 2016-03-05 at 14:08 +0100, Tilman Schmidt wrote:
> As a consequence, owners of HiSAX type adapters are in fact stuck with
> the old hisax driver if they want to continue using i4l userspace
> tools.

Do you know whether or not mISDN tools offer functionality comparable to
i4l tools?


Paul Bolle
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Kernel docs: muddying the waters a bit

[Johannes Stezenbach]

On Mon, Mar 07, 2016 at 12:29:08AM +0100, Johannes Stezenbach wrote:
> On Sat, Mar 05, 2016 at 11:29:37PM -0300, Mauro Carvalho Chehab wrote:
> > 
> > I converted one of the big tables to CSV. At least now it recognized
> > it as a table. Yet, the table was very badly formated:
> > 
> > https://mchehab.fedorapeople.org/media-kabi-docs-test/rst_tests/packed-rgb.html
> > 
> > This is how this table should look like:
> > https://linuxtv.org/downloads/v4l-dvb-apis/packed-rgb.html
> > 
> > Also, as this table has merged cells at the legend. I've no idea how
> > to tell sphinx to do that on csv format.
> > 
> > The RST files are on this git tree:
> > https://git.linuxtv.org/mchehab/v4l2-docs-poc.git/
> 
> Yeah, seems it can't do merged cells in csv.  Attached patch converts it
> back to grid table format and fixes the table definition.
> The html output looks usable, but clearly it is no fun to
> work with tables in Sphinx.
> 
> Sphinx' latex writer can't handle nested tables, though.
> Python's docutils rst2latex can, but that doesn't help here.
> rst2pdf also supports it.  But I have doubts such a large
> table would render OK in pdf without using landscape orientation.
> I have not tried because I used python3-sphinx but rst2pdf
> is only availble for Python2 in Debian so it does not integrate
> with Sphinx.

Just a quick idea:
Perhaps one alternative would be to use Graphviz to render
the problematic tables, it supports a HTML-like syntax
and can be embedded in Spinx documents:

http://www.sphinx-doc.org/en/stable/ext/graphviz.html
http://www.graphviz.org/content/node-shapes#html
http://stackoverflow.com/questions/13890568/graphviz-html-nested-tables


Johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v2] can: rcar_canfd: Add Renesas R-Car CAN FD driver

[Ramesh Shanmugasundaram]

Hi Rob,

Thanks for the review comments.

> On Thu, Mar 03, 2016 at 03:38:35PM +, Ramesh Shanmugasundaram wrote:
> > This patch adds support for the CAN FD controller found in Renesas
> > R-Car SoCs. The controller operates in CAN FD mode by default.
> >
> > CAN FD mode supports both Classical CAN & CAN FD frame formats. The
> > controller supports ISO 11898-1:2015 CAN FD format only.
> >
> > This controller supports two channels and the driver can enable either
> > or both of the channels.
> >
> > Driver uses Rx FIFOs (one per channel) for reception & Common FIFOs
> > (one per channel) for transmission. Rx filter rules are configured to
> > the minimum (one per channel) and it accepts Standard, Extended, Data
> > & Remote Frame combinations.
> >
> > Note: There are few documentation errors in R-Car Gen3 Hardware User
> > Manual v0.5E with respect to CAN FD controller. They are listed below:
> >
> > 1. CAN FD interrupt numbers 29 & 30 are listed as per channel
> > interrupts. However, they are common to both channels (i.e.) they are
> > global and channel interrupts respectively.
> >
> > 2. CANFD clock is derived from PLL1. This is not documented.
> >
> > 3. CANFD clock is further divided by (1/2) within the CAN FD controller.
> > This is not documented.
> >
> > 4. The minimum value of NTSEG1 in RSCFDnCFDCmNCFG register is 2 Tq. It
> > is mentioned 4 Tq in the manual.
> >
> > 5. The maximum number of message RAM area the controller can use is
> > 3584 bytes. It is specified 10752 bytes in the manual.
> >
> > Signed-off-by: Ramesh Shanmugasundaram
> > 
> > ---
> > Changes since v1:
> > * Removed testmodes & debugfs code (suggested by Oliver H)
> > * Fixed tx path race issue by introducing lock (suggested by Marc K)
> > * Removed __maybe_unused attribute of rcar_canfd_of_table
> >
> > Thanks,
> > Ramesh
> > ---
> >  .../devicetree/bindings/net/can/rcar_canfd.txt |   86 ++
> >  drivers/net/can/Kconfig|   10 +
> >  drivers/net/can/Makefile   |1 +
> >  drivers/net/can/rcar_canfd.c   | 1624
> 
> >  4 files changed, 1721 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/net/can/rcar_canfd.txt
> >  create mode 100644 drivers/net/can/rcar_canfd.c
> >
> > diff --git a/Documentation/devicetree/bindings/net/can/rcar_canfd.txt
> > b/Documentation/devicetree/bindings/net/can/rcar_canfd.txt
> > new file mode 100644
> > index 000..4299bd8
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/can/rcar_canfd.txt
> > @@ -0,0 +1,86 @@
> > +Renesas R-Car CAN FD controller Device Tree Bindings
> > +
> > +
> > +Required properties:
> > +- compatible: Must contain one or more of the following:
> > +  - "renesas,rcar-gen3-canfd" for R-Car Gen3 compatible controller.
> > +  - "renesas,r8a7795-canfd" for R8A7795 (R-Car H3) compatible
> controller.
> > +
> > +  When compatible with the generic version, nodes must list the
> > + SoC-specific version corresponding to the platform first, followed
> > + by the  family-specific and/or generic versions.
> > +
> > +- reg: physical base address and size of the R-Car CAN FD register map.
> > +- interrupts: interrupt specifier for the Global & Channel interrupts
> > +- clocks: phandles and clock specifiers for 3 clock inputs.
> > +- clock-names: 3 clock input name strings: "fck", "canfd", "can_clk".
> > +- pinctrl-0: pin control group to be used for this controller.
> > +- pinctrl-names: must be "default".
> > +
> > +Required properties for "renesas,r8a7795-canfd" compatible:
> > +In R8A7795 SoC, canfd clock is a div6 clock and can be used by both
> > +CAN and CAN FD controller at the same time. It needs to be scaled to
> > +maximum frequency if any of these controllers use it. This is done
> > +using the below properties.
> > +
> > +- assigned-clocks: phandle of canfd clock.
> > +- assigned-clock-rates: maximum frequency of this clock.
> > +
> > +Each channel is represented as a child node. They can be
> > +enabled/disabled using "status" property.
> 
> How many channels? Required or optional?

Two channels and it is a required property. I will update as below in the next 
revision.

Required child node:
The controller supports two channels and each is represented as a child
node. The names of the child nodes are "channel0" and "channel1" respectively.
Each child node supports the "status" property only, which is used to
enable/disable the respective channel.

> 
> > +
> > +Example
> > +---
> > +
> > +SoC common .dtsi file:
> > +
> > +   canfd: canfd@e66c {
> 
> can@e66c

Thanks. I will update.

Thanks,
Ramesh
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Kernel docs: muddying the waters a bit

[Mauro Carvalho Chehab]

Em Mon, 7 Mar 2016 09:48:26 +0100
Johannes Stezenbach  escreveu:

> On Mon, Mar 07, 2016 at 12:29:08AM +0100, Johannes Stezenbach wrote:
> > On Sat, Mar 05, 2016 at 11:29:37PM -0300, Mauro Carvalho Chehab wrote:  
> > > 
> > > I converted one of the big tables to CSV. At least now it recognized
> > > it as a table. Yet, the table was very badly formated:
> > >   
> > > https://mchehab.fedorapeople.org/media-kabi-docs-test/rst_tests/packed-rgb.html
> > > 
> > > This is how this table should look like:
> > >   https://linuxtv.org/downloads/v4l-dvb-apis/packed-rgb.html
> > > 
> > > Also, as this table has merged cells at the legend. I've no idea how
> > > to tell sphinx to do that on csv format.
> > > 
> > > The RST files are on this git tree:
> > >   https://git.linuxtv.org/mchehab/v4l2-docs-poc.git/  
> > 
> > Yeah, seems it can't do merged cells in csv.  Attached patch converts it
> > back to grid table format and fixes the table definition.
> > The html output looks usable, but clearly it is no fun to
> > work with tables in Sphinx.
> > 
> > Sphinx' latex writer can't handle nested tables, though.
> > Python's docutils rst2latex can, but that doesn't help here.
> > rst2pdf also supports it.  But I have doubts such a large
> > table would render OK in pdf without using landscape orientation.
> > I have not tried because I used python3-sphinx but rst2pdf
> > is only availble for Python2 in Debian so it does not integrate
> > with Sphinx.  
> 
> Just a quick idea:
> Perhaps one alternative would be to use Graphviz to render
> the problematic tables, it supports a HTML-like syntax
> and can be embedded in Spinx documents:
> 
> http://www.sphinx-doc.org/en/stable/ext/graphviz.html
> http://www.graphviz.org/content/node-shapes#html
> http://stackoverflow.com/questions/13890568/graphviz-html-nested-tables

That could work, but it is scary... Graphviz is great to generate
diagrams, but it really sucks when one wants to put a graph element
on a specific place, as it loves to reorder elements putting them on
unexpected places.

Btw, 

I converted all docs from our uAPI docbook to rst using pandoc.
It was a brainless conversion, except for a few fixes.

The output is at:
https://mchehab.fedorapeople.org/media-kabi-docs-test/rst_tests/

I added it on the top of my PoC tree at:
https://git.linuxtv.org/mchehab/v4l2-docs-poc.git/  

Besides tables, I noticed some other bad things that needs to be
corrected somehow:

1) Document divisions are not numbered. We need that. It should be
broken into:
- Document divisions - one per documented API:
- V4L2
- Remote Controllers
- DVB
- Media Controller

- Chapters
- Sessions

Everything should be numbered, as, when discussing API improvements,
it is usual the need of pinpoint to an specific chapter and section.

Tables and images should also be numbered, and we need a way to
use references for table/image numbers.

2) Images

Most images didn't popup. We have images on different file formats:
- SVG
- GIF
- PDF
- PNG

3) References

It could be a conversion issue, but there are lots of missing 
references at the documentation.

4) We need to have some way to tell sphinx to not put some things
at the lateral ToC bar. For example, at the V4L2 "Changes" section,
we don't want to have one entry per version at the ToC bar.

Giving that, I suspect that we'll have huge headaches to address
if we use sphinx, as it seems too limited to handle complex
documents. We should try to use some other tool that would give
us better results.


Regards,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Kernel docs: muddying the waters a bit

[Mauro Carvalho Chehab]

Em Mon, 7 Mar 2016 00:29:08 +0100
Johannes Stezenbach  escreveu:

> On Sat, Mar 05, 2016 at 11:29:37PM -0300, Mauro Carvalho Chehab wrote:
> > 
> > I converted one of the big tables to CSV. At least now it recognized
> > it as a table. Yet, the table was very badly formated:
> > 
> > https://mchehab.fedorapeople.org/media-kabi-docs-test/rst_tests/packed-rgb.html
> > 
> > This is how this table should look like:
> > https://linuxtv.org/downloads/v4l-dvb-apis/packed-rgb.html
> > 
> > Also, as this table has merged cells at the legend. I've no idea how
> > to tell sphinx to do that on csv format.
> > 
> > The RST files are on this git tree:
> > https://git.linuxtv.org/mchehab/v4l2-docs-poc.git/  
> 
> Yeah, seems it can't do merged cells in csv.  Attached patch converts it
> back to grid table format and fixes the table definition.
> The html output looks usable, but clearly it is no fun to
> work with tables in Sphinx.

Yes, the output is OK, but, as you said, working with tables in
Sphinx is hard, and using asciiart for the kind of tables we have
is not nice.

> 
> Sphinx' latex writer can't handle nested tables, though.

Yeah, this is a big trouble that need to be solved if you're
willing to use Sphinx.

Btw, it crashes when trying to generate man pages:

Exception occurred:
  File "/usr/lib/python2.7/site-packages/docutils/writers/manpage.py", 
line 627, in depart_entry
self._active_table.append_cell(self.body[start:])
AttributeError: 'NoneType' object has no attribute 'append_cell'
The full traceback has been saved in /tmp/sphinx-err-04qRMz.log, if you 
want to report the issue to the developers.

So, if we're willing to use sphinx, someone should either fix
it to produce latex nexted table and fix it to generate manpages,
or we'll need to stick with just html output.

> Python's docutils rst2latex can, but that doesn't help here.
> rst2pdf also supports it.

At least here, rst2* scripts were unable to identify that the
index.rst had links to other rst documents. 

In the specific case of rst2latex, I got several errors like:

index.rst:21: (ERROR/3) Unknown interpreted text role "ref".


> But I have doubts such a large
> table would render OK in pdf without using landscape orientation.

Yeah, in the past, when we had pdf enabled for DocBook (e. g. when
media development was using a separate mercurial tree), I guess
we had tags changing the text orientation on a few tables that
would otherwise won't diplay fine, but I can't remember the dirty
details anymore.

> I have not tried because I used python3-sphinx but rst2pdf
> is only availble for Python2 in Debian so it does not integrate
> with Sphinx.
> 
> 
> Johannes


-- 
Thanks,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 10/11] tpm: Add documentation for the tpm_vtpm device driver

[Stefan Berger]

Add documentation for the tpm_vtpm device driver that implements
support for providing TPM functionality to Linux containers.

Parts of this documentation were recycled from the Xen vTPM
device driver documentation.

Signed-off-by: Stefan Berger 
CC: linux-ker...@vger.kernel.org
CC: linux-doc@vger.kernel.org
CC: linux-...@vger.kernel.org
---
 Documentation/tpm/tpm_vtpm.txt | 53 ++
 1 file changed, 53 insertions(+)
 create mode 100644 Documentation/tpm/tpm_vtpm.txt

diff --git a/Documentation/tpm/tpm_vtpm.txt b/Documentation/tpm/tpm_vtpm.txt
new file mode 100644
index 000..7746c0d
--- /dev/null
+++ b/Documentation/tpm/tpm_vtpm.txt
@@ -0,0 +1,53 @@
+Virtual TPM Device Driver for Linux Containers
+
+Authors: Stefan Berger (IBM)
+
+This document describes the virtual Trusted Platform Module (vTPM) device
+driver for Linux containers.
+
+INTRODUCTION
+
+
+The goal of this work is to provide TPM functionality to each Linux
+container. This allows programs to interact with a TPM in a container
+the same way they interact with a TPM on the physical system. Each
+container gets its own unique, emulated, software TPM.
+
+
+DESIGN
+--
+
+To make an emulated software TPM available to each container, the container
+management stack needs to create a device pair consisting of a client TPM
+character device /dev/tpmX (with X=0,1,2...) and a 'server side' file
+descriptor. The former is moved into the container by creating a character
+device with the appropriate major and minor numbers while the file descriptor
+is passed to the TPM emulator. Software inside the container can then send
+TPM commands using the character device and the emulator will receive the
+commands via the file descriptor and use it for sending back responses.
+
+To support this, the virtual TPM device driver provides a device /dev/vtpmx
+that is used to create device pairs using an ioctl. The ioctl takes as
+an input flags for configuring the device. The flags  for example indicate
+whether TPM 1.2 or TPM 2 functionality is supported by the TPM emulator.
+The result of the ioctl are the file descriptor for the 'server side'
+as well as the major and minor numbers of the character device that was 
created.
+Besides that the number of the TPM character device is return. If for
+example /dev/tpm10 was created, the number (dev_num) 10 is returned.
+
+The following is the data structure of the VTPM_NEW_DEV ioctl:
+
+struct vtpm_new_dev {
+   __u32 flags; /* input */
+   __u32 dev_num;   /* output */
+   __u32 fd;/* output */
+   __u32 major; /* output */
+   __u32 minor; /* output */
+};
+
+Note that if unsupported flags are passed to the device driver, the ioctl will
+fail and errno will be set to ENOSYS. Similarly, if an unsupported ioctl is
+called on the device driver, the ioctl will fail and errno will be set to 
ENOSYS.
+
+See /usr/include/linux/vtpm.h for definitions related to the public interface
+of this vTPM device driver.
-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 09/11] tpm: Initialize TPM and get durations and timeouts

[Stefan Berger]

Add the retrieval of TPM 1.2 durations and timeouts. Since this requires
the startup of the TPM, do this for TPM 1.2 and TPM 2.

Signed-off-by: Stefan Berger 
CC: linux-ker...@vger.kernel.org
CC: linux-doc@vger.kernel.org
CC: linux-...@vger.kernel.org
---
 drivers/char/tpm/tpm_vtpm.c | 94 -
 1 file changed, 85 insertions(+), 9 deletions(-)

diff --git a/drivers/char/tpm/tpm_vtpm.c b/drivers/char/tpm/tpm_vtpm.c
index 245ce91..b048898 100644
--- a/drivers/char/tpm/tpm_vtpm.c
+++ b/drivers/char/tpm/tpm_vtpm.c
@@ -45,8 +45,11 @@ struct vtpm_dev {
size_t req_len;  /* length of queued TPM request */
size_t resp_len; /* length of queued TPM response */
u8 buffer[TPM_BUFSIZE];  /* request/response buffer */
+
+   struct work_struct work; /* task that retrieves TPM timeouts */
 };
 
+static struct workqueue_struct *workqueue;
 
 static void vtpm_delete_device(struct vtpm_dev *vtpm_dev);
 
@@ -67,6 +70,15 @@ static ssize_t vtpm_fops_read(struct file *filp, char __user 
*buf,
size_t len;
int sig, rc;
 
+   mutex_lock(&vtpm_dev->buf_lock);
+
+   if (!(vtpm_dev->state & STATE_OPENED_FLAG)) {
+   mutex_unlock(&vtpm_dev->buf_lock);
+   return -EPIPE;
+   }
+
+   mutex_unlock(&vtpm_dev->buf_lock);
+
sig = wait_event_interruptible(vtpm_dev->wq, vtpm_dev->req_len != 0);
if (sig)
return -EINTR;
@@ -110,6 +122,11 @@ static ssize_t vtpm_fops_write(struct file *filp, const 
char __user *buf,
 
mutex_lock(&vtpm_dev->buf_lock);
 
+   if (!(vtpm_dev->state & STATE_OPENED_FLAG)) {
+   mutex_unlock(&vtpm_dev->buf_lock);
+   return -EPIPE;
+   }
+
if (count > sizeof(vtpm_dev->buffer) ||
!(vtpm_dev->state & STATE_WAIT_RESPONSE_FLAG)) {
mutex_unlock(&vtpm_dev->buf_lock);
@@ -154,6 +171,9 @@ static unsigned int vtpm_fops_poll(struct file *filp, 
poll_table *wait)
if (vtpm_dev->req_len)
ret |= POLLIN | POLLRDNORM;
 
+   if (!(vtpm_dev->state & STATE_OPENED_FLAG))
+   ret |= POLLHUP;
+
mutex_unlock(&vtpm_dev->buf_lock);
 
return ret;
@@ -341,6 +361,54 @@ static const struct tpm_class_ops vtpm_tpm_ops = {
 };
 
 /*
+ * Code related to the startup of the TPM 2 and startup of TPM 1.2 +
+ * retrieval of timeouts and durations.
+ */
+
+static void vtpm_dev_work(struct work_struct *work)
+{
+   struct vtpm_dev *vtpm_dev = container_of(work, struct vtpm_dev, work);
+   int rc;
+
+   if (vtpm_dev->flags & VTPM_FLAG_TPM2)
+   rc = tpm2_startup(vtpm_dev->chip, TPM2_SU_CLEAR);
+   else
+   rc = tpm_get_timeouts(vtpm_dev->chip);
+
+   if (rc)
+   goto err;
+
+   rc = tpm_chip_register(vtpm_dev->chip);
+   if (rc)
+   goto err;
+
+   return;
+
+err:
+   vtpm_fops_undo_open(vtpm_dev);
+}
+
+/*
+ * vtpm_dev_work_stop: make sure the work has finished
+ *
+ * This function is useful when user space closed the fd
+ * while the driver still determines timeouts.
+ */
+static void vtpm_dev_work_stop(struct vtpm_dev *vtpm_dev)
+{
+   vtpm_fops_undo_open(vtpm_dev);
+   flush_work(&vtpm_dev->work);
+}
+
+/*
+ * vtpm_dev_work_start: Schedule the work for TPM 1.2 & 2 initialization
+ */
+static inline void vtpm_dev_work_start(struct vtpm_dev *vtpm_dev)
+{
+   queue_work(workqueue, &vtpm_dev->work);
+}
+
+/*
  * Code related to creation and deletion of device pairs
  */
 static struct vtpm_dev *vtpm_create_vtpm_dev(void)
@@ -355,6 +423,7 @@ static struct vtpm_dev *vtpm_create_vtpm_dev(void)
 
init_waitqueue_head(&vtpm_dev->wq);
mutex_init(&vtpm_dev->buf_lock);
+   INIT_WORK(&vtpm_dev->work, vtpm_dev_work);
 
chip = tpm_chip_alloc(NULL, &vtpm_tpm_ops);
if (IS_ERR(chip)) {
@@ -424,9 +493,7 @@ static struct file *vtpm_create_device(
if (vtpm_dev->flags & VTPM_FLAG_TPM2)
vtpm_dev->chip->flags |= TPM_CHIP_FLAG_TPM2;
 
-   rc = tpm_chip_register(vtpm_dev->chip);
-   if (rc)
-   goto err_vtpm_fput;
+   vtpm_dev_work_start(vtpm_dev);
 
vtpm_new_dev->fd = fd;
vtpm_new_dev->major = MAJOR(vtpm_dev->chip->dev.devt);
@@ -435,12 +502,6 @@ static struct file *vtpm_create_device(
 
return file;
 
-err_vtpm_fput:
-   put_unused_fd(fd);
-   fput(file);
-
-   return ERR_PTR(rc);
-
 err_put_unused_fd:
put_unused_fd(fd);
 
@@ -455,6 +516,8 @@ err_delete_vtpm_dev:
  */
 static void vtpm_delete_device(struct vtpm_dev *vtpm_dev)
 {
+   vtpm_dev_work_stop(vtpm_dev);
+
tpm_chip_unregister(vtpm_dev->chip);
 
vtpm_fops_undo_open(vtpm_dev);
@@ -549,11 +612,24 @@ static int __init vtpm_module_init(void)
return rc;
}
 
+   workqueue = create_workqueue("tpm-vtpm");
+   if (!workqueue) {
+   pr_er

[PATCH v5 08/11] tpm: Driver for supporting multiple emulated TPMs

[Stefan Berger]

This patch implements a driver for supporting multiple emulated TPMs in a
system.

The driver implements a device /dev/vtpmx that is used to created
a client device pair /dev/tpmX (e.g., /dev/tpm10) and a server side that
is accessed using a file descriptor returned by an ioctl.
The device /dev/tpmX is the usual TPM device created by the core TPM
driver. Applications or kernel subsystems can send TPM commands to it
and the corresponding server-side file descriptor receives these
commands and delivers them to an emulated TPM.

Signed-off-by: Stefan Berger 
CC: linux-ker...@vger.kernel.org
CC: linux-doc@vger.kernel.org
CC: linux-...@vger.kernel.org
---
 drivers/char/tpm/Kconfig|  10 +
 drivers/char/tpm/Makefile   |   1 +
 drivers/char/tpm/tpm_vtpm.c | 566 
 include/uapi/linux/Kbuild   |   1 +
 include/uapi/linux/vtpm.h   |  41 
 5 files changed, 619 insertions(+)
 create mode 100644 drivers/char/tpm/tpm_vtpm.c
 create mode 100644 include/uapi/linux/vtpm.h

diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
index 3b84a8b..4c4e843 100644
--- a/drivers/char/tpm/Kconfig
+++ b/drivers/char/tpm/Kconfig
@@ -122,5 +122,15 @@ config TCG_CRB
  from within Linux.  To compile this driver as a module, choose
  M here; the module will be called tpm_crb.
 
+config TCG_VTPM
+   tristate "VTPM Interface"
+   depends on TCG_TPM
+   ---help---
+ This driver supports an emulated TPM (vTPM) running in userspace.
+ A device /dev/vtpmx is provided that creates a device pair
+ /dev/vtpmX and a server-side file descriptor on which the vTPM
+ can receive commands.
+
+
 source "drivers/char/tpm/st33zp24/Kconfig"
 endif # TCG_TPM
diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index 56e8f1f..1a409c57 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -23,3 +23,4 @@ obj-$(CONFIG_TCG_IBMVTPM) += tpm_ibmvtpm.o
 obj-$(CONFIG_TCG_TIS_ST33ZP24) += st33zp24/
 obj-$(CONFIG_TCG_XEN) += xen-tpmfront.o
 obj-$(CONFIG_TCG_CRB) += tpm_crb.o
+obj-$(CONFIG_TCG_VTPM) += tpm_vtpm.o
diff --git a/drivers/char/tpm/tpm_vtpm.c b/drivers/char/tpm/tpm_vtpm.c
new file mode 100644
index 000..245ce91
--- /dev/null
+++ b/drivers/char/tpm/tpm_vtpm.c
@@ -0,0 +1,566 @@
+/*
+ * Copyright (C) 2015, 2016 IBM Corporation
+ *
+ * Author: Stefan Berger 
+ *
+ * Maintained by: 
+ *
+ * Device driver for vTPM.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, version 2 of the
+ * License.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "tpm.h"
+
+#define VTPM_REQ_COMPLETE_FLAG  BIT(0)
+
+struct vtpm_dev {
+   struct tpm_chip *chip;
+
+   u32 flags;   /* public API flags */
+
+   wait_queue_head_t wq;
+
+   struct mutex buf_lock;   /* protect buffer and flags */
+
+   long state;  /* internal state */
+#define STATE_OPENED_FLAGBIT(0)
+#define STATE_WAIT_RESPONSE_FLAG BIT(1)  /* waiting for emulator response */
+
+   size_t req_len;  /* length of queued TPM request */
+   size_t resp_len; /* length of queued TPM response */
+   u8 buffer[TPM_BUFSIZE];  /* request/response buffer */
+};
+
+
+static void vtpm_delete_device(struct vtpm_dev *vtpm_dev);
+
+/*
+ * Functions related to 'server side'
+ */
+
+/**
+ * vtpm_fops_read - Read TPM commands on 'server side'
+ *
+ * Return value:
+ * Number of bytes read or negative error code
+ */
+static ssize_t vtpm_fops_read(struct file *filp, char __user *buf,
+ size_t count, loff_t *off)
+{
+   struct vtpm_dev *vtpm_dev = filp->private_data;
+   size_t len;
+   int sig, rc;
+
+   sig = wait_event_interruptible(vtpm_dev->wq, vtpm_dev->req_len != 0);
+   if (sig)
+   return -EINTR;
+
+   mutex_lock(&vtpm_dev->buf_lock);
+
+   len = vtpm_dev->req_len;
+
+   if (count < len) {
+   mutex_unlock(&vtpm_dev->buf_lock);
+   pr_debug("Invalid size in recv: count=%zd, req_len=%zd\n",
+count, len);
+   return -EIO;
+   }
+
+   rc = copy_to_user(buf, vtpm_dev->buffer, len);
+   memset(vtpm_dev->buffer, 0, len);
+   vtpm_dev->req_len = 0;
+
+   if (!rc)
+   vtpm_dev->state |= STATE_WAIT_RESPONSE_FLAG;
+
+   mutex_unlock(&vtpm_dev->buf_lock);
+
+   if (rc)
+   return -EFAULT;
+
+   return len;
+}
+
+/**
+ * vtpm_fops_write - Write TPM responses on 'server side'
+ *
+ * Return value:
+ * Number of bytes read or negative error value
+ */
+static ssize_t vtpm_fops_write(struct file *filp, const char __user *buf,
+  size_t count, loff_t *off)
+{
+   

Re: [PATCH] modsign: Fix documentation on module signing enforcement parameter.

[David Howells]

James Johnston  wrote:

> -If CONFIG_MODULE_SIG_FORCE is enabled or enforcemodulesig=1 is supplied on
> +If CONFIG_MODULE_SIG_FORCE is enabled or module.sig_enforce=1 is supplied

You're definitely right about the change from enforcemodulesig to sig_enforce,
but how does the "module." come about?

David
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[Khalid Aziz]

On 03/05/2016 09:07 PM, David Miller wrote:

From: Khalid Aziz 
Date: Wed,  2 Mar 2016 13:39:37 -0700


In this
first implementation I am enabling ADI for hugepages only
since these pages are locked in memory and hence avoid the
issue of saving and restoring tags.


This makes the feature almost entire useless.

Non-hugepages must be in the initial implementation.


Hi David,

Thanks for the feedback. I will get this working for non-hugepages as 
well. ADI state of each VMA region is already stored in the VMA itself 
in my first implementation, so I do not lose it when the page is swapped 
out. The trouble is ADI version tags for each VMA region have to be 
stored on the swapped out pages since the ADI version tags are flushed 
when TLB entry for a page is flushed. When that page is brought back in, 
its version tags have to be set up again. Version tags are set on 
cacheline boundary and hence there can be multiple version tags for a 
single page. Version tags have to be stored in the swap space somehow 
along with the page. I can start out with allowing ADI to be enabled 
only on pages locked in memory.





+   PR_ENABLE_SPARC_ADI - Enable ADI checking in all pages in the address
+   range specified. The pages in the range must be already
+   locked. This operation enables the TTE.mcd bit for the
+   pages specified. arg2 is the starting address for address
+   range and must be page aligned. arg3 is the length of
+   memory address range and must be a multiple of page size.


I strongly dislike this interface, and it makes the prtctl cases look
extremely ugly and hide to the casual reader what the code is actually
doing.

This is an mprotect() operation, so add a new flag bit and implement
this via mprotect please.


That is an interesting idea. Adding a PROT_ADI protection to mprotect() 
sounds cleaner. There are three steps to enabling ADI - (1) set 
PSTATE.mcde bit which is not tied to any VMA, (2) set TTE.mcd for each 
VMA, and (3) set the version tag on cacheline using MCD ASI. I can 
combine steps 1 and 2 in one mprotect() call. That will leave 
PR_GET_SPARC_ADICAPS and PR_GET_SPARC_ADI_STATUS prctl commands still to 
be implemented. PR_SET_SPARC_ADI is also used to check if the process 
has PSTATE.mcde bit set. I could use PR_GET_SPARC_ADI_STATUS to do that 
where return values of 0 and 1 mean the same as before and possibly add 
return value of 2 to mean PSTATE.mcde is not set?




Then since you are guarenteed to have a consistent ADI setting for
every single VMA region, you never "lose" the ADI state when you swap
out.  It's implicit in the VMA itself, because you'll store in the VMA
that this is an ADI region.

I also want this enabled unconditionally, without any Kconfig knobs.



I can remove CONFIG_SPARC_ADI. It does mean this code will be built into 
32-bit kernels as well but it will be inactive code.


Thanks,
Khalid



--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[Rob Gardner]

On 03/07/2016 07:07 AM, Khalid Aziz wrote:

On 03/05/2016 09:07 PM, David Miller wrote:

From: Khalid Aziz 
Date: Wed,  2 Mar 2016 13:39:37 -0700


In this
first implementation I am enabling ADI for hugepages only
since these pages are locked in memory and hence avoid the
issue of saving and restoring tags.


This makes the feature almost entire useless.

Non-hugepages must be in the initial implementation.


Hi David,

Thanks for the feedback. I will get this working for non-hugepages as 
well. ADI state of each VMA region is already stored in the VMA itself 
in my first implementation, so I do not lose it when the page is 
swapped out. The trouble is ADI version tags for each VMA region have 
to be stored on the swapped out pages since the ADI version tags are 
flushed when TLB entry for a page is flushed. 



Khalid,

Are you sure about that last statement? My understanding is that the 
tags are stored in physical memory, and remain there until explicitly 
changed or removed, and so flushing a TLB entry has no effect on the ADI 
tags. If it worked the way you think, then somebody would have to 
potentially reload a long list of ADI tags on every TLB miss.


Rob



When that page is brought back in, its version tags have to be set up 
again. Version tags are set on cacheline boundary and hence there can 
be multiple version tags for a single page. Version tags have to be 
stored in the swap space somehow along with the page. I can start out 
with allowing ADI to be enabled only on pages locked in memory.




+PR_ENABLE_SPARC_ADI - Enable ADI checking in all pages in the 
address

+range specified. The pages in the range must be already
+locked. This operation enables the TTE.mcd bit for the
+pages specified. arg2 is the starting address for address
+range and must be page aligned. arg3 is the length of
+memory address range and must be a multiple of page size.


I strongly dislike this interface, and it makes the prtctl cases look
extremely ugly and hide to the casual reader what the code is actually
doing.

This is an mprotect() operation, so add a new flag bit and implement
this via mprotect please.


That is an interesting idea. Adding a PROT_ADI protection to 
mprotect() sounds cleaner. There are three steps to enabling ADI - (1) 
set PSTATE.mcde bit which is not tied to any VMA, (2) set TTE.mcd for 
each VMA, and (3) set the version tag on cacheline using MCD ASI. I 
can combine steps 1 and 2 in one mprotect() call. That will leave 
PR_GET_SPARC_ADICAPS and PR_GET_SPARC_ADI_STATUS prctl commands still 
to be implemented. PR_SET_SPARC_ADI is also used to check if the 
process has PSTATE.mcde bit set. I could use PR_GET_SPARC_ADI_STATUS 
to do that where return values of 0 and 1 mean the same as before and 
possibly add return value of 2 to mean PSTATE.mcde is not set?




Then since you are guarenteed to have a consistent ADI setting for
every single VMA region, you never "lose" the ADI state when you swap
out.  It's implicit in the VMA itself, because you'll store in the VMA
that this is an ADI region.

I also want this enabled unconditionally, without any Kconfig knobs.



I can remove CONFIG_SPARC_ADI. It does mean this code will be built 
into 32-bit kernels as well but it will be inactive code.


Thanks,
Khalid





--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[Andy Lutomirski]

On Mon, Mar 7, 2016 at 7:30 AM, Rob Gardner  wrote:
> On 03/07/2016 07:07 AM, Khalid Aziz wrote:
>>
>> On 03/05/2016 09:07 PM, David Miller wrote:
>>>
>>> From: Khalid Aziz 
>>> Date: Wed,  2 Mar 2016 13:39:37 -0700
>>>
 In this
 first implementation I am enabling ADI for hugepages only
 since these pages are locked in memory and hence avoid the
 issue of saving and restoring tags.
>>>
>>>
>>> This makes the feature almost entire useless.
>>>
>>> Non-hugepages must be in the initial implementation.
>>
>>
>> Hi David,
>>
>> Thanks for the feedback. I will get this working for non-hugepages as
>> well. ADI state of each VMA region is already stored in the VMA itself in my
>> first implementation, so I do not lose it when the page is swapped out. The
>> trouble is ADI version tags for each VMA region have to be stored on the
>> swapped out pages since the ADI version tags are flushed when TLB entry for
>> a page is flushed.
>
>
>
> Khalid,
>
> Are you sure about that last statement? My understanding is that the tags
> are stored in physical memory, and remain there until explicitly changed or
> removed, and so flushing a TLB entry has no effect on the ADI tags. If it
> worked the way you think, then somebody would have to potentially reload a
> long list of ADI tags on every TLB miss.
>

I'll bite, since this was sent to linux-api:

Can someone explain what this feature does for the benefit of people
who haven't read the manual (and who don't even know where to find the
manual)?

Are the top few bits of a sparc64 virtual address currently
must-be-zero?  Does this feature change the semantics so that those
bits are ignored for address resolution and instead must match
whatever the ADI tag is determined to be during address resolution?

Is this enforced for both user and kernel accesses?

Is the actual ADI tag associated with a "page" associated with the
page of physical memory or is it associated with a mapping?  That is,
if there are two virtual aliases of the same physical page (in the
same process or otherwise), does the hardware require them to have the
same ADI tag?  If the answer is no, then IMO this is definitely
something that should use mprotect and you should seriously consider
using something like mprotect_key (new syscall, not in Linus' tree
yet) for it.  In fact, you might consider a possible extra parameter
to that syscall for this purpose.

Cc: Dave Hansen.  It seems to be the zeitgeist to throw tag bits at
PTEs these days.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[Khalid Aziz]

On 03/07/2016 08:30 AM, Rob Gardner wrote:

On 03/07/2016 07:07 AM, Khalid Aziz wrote:

On 03/05/2016 09:07 PM, David Miller wrote:

From: Khalid Aziz 
Date: Wed,  2 Mar 2016 13:39:37 -0700


In this
first implementation I am enabling ADI for hugepages only
since these pages are locked in memory and hence avoid the
issue of saving and restoring tags.


This makes the feature almost entire useless.

Non-hugepages must be in the initial implementation.


Hi David,

Thanks for the feedback. I will get this working for non-hugepages as
well. ADI state of each VMA region is already stored in the VMA itself
in my first implementation, so I do not lose it when the page is
swapped out. The trouble is ADI version tags for each VMA region have
to be stored on the swapped out pages since the ADI version tags are
flushed when TLB entry for a page is flushed.



Khalid,

Are you sure about that last statement? My understanding is that the
tags are stored in physical memory, and remain there until explicitly
changed or removed, and so flushing a TLB entry has no effect on the ADI
tags. If it worked the way you think, then somebody would have to
potentially reload a long list of ADI tags on every TLB miss.

Rob



Hi Rob,

I am fairly sure that is the case. This is what I found from the 
processor guys and others working on ADI. I tested it out by setting up 
ADI on normal malloc'd pages that got swapped out and I got MCD 
exceptions when those pages were swapped back in on access.


I mis-spoke when I said "ADI version tags are flushed when TLB entry 
for a page is flushed". I meant ADI version tags are flushed when 
mapping for a virtual address is removed from TSB, not when TLB entry is 
flushed. Yes, ADI tags are stored in physical memory and removed when 
mapping is removed.


Thanks,
Khalid

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[Khalid Aziz]

On 03/07/2016 08:43 AM, Andy Lutomirski wrote:

On Mon, Mar 7, 2016 at 7:30 AM, Rob Gardner  wrote:

On 03/07/2016 07:07 AM, Khalid Aziz wrote:


On 03/05/2016 09:07 PM, David Miller wrote:


From: Khalid Aziz 
Date: Wed,  2 Mar 2016 13:39:37 -0700


 In this
 first implementation I am enabling ADI for hugepages only
 since these pages are locked in memory and hence avoid the
 issue of saving and restoring tags.



This makes the feature almost entire useless.

Non-hugepages must be in the initial implementation.



Hi David,

Thanks for the feedback. I will get this working for non-hugepages as
well. ADI state of each VMA region is already stored in the VMA itself in my
first implementation, so I do not lose it when the page is swapped out. The
trouble is ADI version tags for each VMA region have to be stored on the
swapped out pages since the ADI version tags are flushed when TLB entry for
a page is flushed.




Khalid,

Are you sure about that last statement? My understanding is that the tags
are stored in physical memory, and remain there until explicitly changed or
removed, and so flushing a TLB entry has no effect on the ADI tags. If it
worked the way you think, then somebody would have to potentially reload a
long list of ADI tags on every TLB miss.



I'll bite, since this was sent to linux-api:

Can someone explain what this feature does for the benefit of people
who haven't read the manual (and who don't even know where to find the
manual)?

Are the top few bits of a sparc64 virtual address currently
must-be-zero?  Does this feature change the semantics so that those
bits are ignored for address resolution and instead must match
whatever the ADI tag is determined to be during address resolution?

Is this enforced for both user and kernel accesses?

Is the actual ADI tag associated with a "page" associated with the
page of physical memory or is it associated with a mapping?  That is,
if there are two virtual aliases of the same physical page (in the
same process or otherwise), does the hardware require them to have the
same ADI tag?  If the answer is no, then IMO this is definitely
something that should use mprotect and you should seriously consider
using something like mprotect_key (new syscall, not in Linus' tree
yet) for it.  In fact, you might consider a possible extra parameter
to that syscall for this purpose.

Cc: Dave Hansen.  It seems to be the zeitgeist to throw tag bits at
PTEs these days.



Hi Andy,

The primary purpose of this feature is to prevent rogue accesses to 
memory regions. If a database were to allocate memory pages to cache 
database, it can enable ADI on those pages and set version tags. Version 
tag for a memory address is encoded in bits 63-60 in the virtual 
address. When accessing an ADI enabled memory region, top 4 bits of the 
virtual address presented to the MMU must match the version tag set 
earlier. When these bits do not match a tag, an MCD (Memory Corruption 
Detected) exception is raised. Kernel sends a SIGBUS to the offending 
process in response. There is some more info on ADI at 
.


Top 4-bits of sparc64 virtual address are used for version tag only when 
a process has its PSTATE.mcde bit set and it is accessing a memory 
region that has ADI enabled on it (TTE.mcd set) and a version tag was 
set on the virtual address being accessed. These 4-bits retain their 
original semantics in all other cases.


ADI version tags are checked for data fetches only. My implementation 
enforces this for userspace addresses only. Expanding this to include 
kernel data addresses as well will be a good thing to do to protect 
kernel data but I want to try to do this incrementally - (1) ADI for 
userspace addresses only for mlock'd pages, (2) expand support to 
swappable pages, (3) ADI for kernel data pages, (4)..whatever else 
makes sense...


ADI version tag applies to virtual addresses only. If two processes have 
virtual addresses mapping to the same physical page, they must use the 
same tag. Hardware will send MCD exception if the tags do not match. 
This was done to ensure a hack does not bypass ADI protection by simply 
inserting another VA-to-PA mapping. I do like the idea of mprotect() as 
David suggested and it can be done with existing mprotect() call. I will 
have to add a new key PROT_ADI to support this.


Thanks,
Khalid
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v17 2/6] ARM: socfpga: add bindings document for fpga bridge drivers

[atull]

On Sat, 5 Mar 2016, Rob Herring wrote:

> On Thu, Feb 25, 2016 at 05:25:07PM -0600, Alan Tull wrote:
> > Add bindings documentation for Altera SOCFPGA bridges:
> >  * fpga2sdram
> >  * fpga2hps
> >  * hps2fpga
> >  * lwhps2fpga
> > 
> > Signed-off-by: Alan Tull 
> > Signed-off-by: Matthew Gerlach 
> > Signed-off-by: Dinh Nguyen 
> > ---
> > v2:  separate into 2 documents for the 2 drivers
> > v12: bump version to line up with simple-fpga-bus version
> >  remove Linux specific notes such as references to sysfs
> >  move non-DT specific documentation elsewhere
> >  remove bindings that would have been used to pass configuration
> >  clean up formatting
> > v13: Remove 'label' property
> >  Change property from init-val to bridge-enable
> >  Fix email address
> > v14: Add resets
> >  Change order of bridges to put lw bridge (controlling bridge) first
> > v15: No change in this patch for v15 of this patch set
> > v16: Added regs property, cleaned up unit addresses
> > v17: No change to this patch in v17 of patch set
> > ---
> >  .../bindings/fpga/altera-fpga2sdram-bridge.txt |   15 +++
> >  .../bindings/fpga/altera-hps2fpga-bridge.txt   |   47 
> > 
> >  2 files changed, 62 insertions(+)
> >  create mode 100644 
> > Documentation/devicetree/bindings/fpga/altera-fpga2sdram-bridge.txt
> >  create mode 100644 
> > Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt
> 
> Just a few minor things.
> 
> > diff --git 
> > a/Documentation/devicetree/bindings/fpga/altera-fpga2sdram-bridge.txt 
> > b/Documentation/devicetree/bindings/fpga/altera-fpga2sdram-bridge.txt
> > new file mode 100644
> > index 000..4479a79
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/fpga/altera-fpga2sdram-bridge.txt
> > @@ -0,0 +1,15 @@
> > +Altera FPGA To SDRAM Bridge Driver
> > +
> > +Required properties:
> > +- compatible   : Should contain 
> > "altr,socfpga-fpga2sdram-bridge"
> > +
> > +Optional properties:
> > +- bridge-enable: 0 if driver should disable bridge at startup
> > + 1 if driver should enable bridge at startup
> > + Default is to leave bridge in current state.
> > +
> > +Example:
> > +   fpga2sdram_br {
> 
> fpga-bridge@??

The hardware is messy here as the control of this bridge is
lumped into the sdram controller.  I could give the address
of the one register that enables/disable the bridge here.

> 
> > +   compatible = "altr,socfpga-fpga2sdram-bridge";
> > +   bridge-enable = <0>;
> > +   };
> > diff --git 
> > a/Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt 
> > b/Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt
> > new file mode 100644
> > index 000..e6b7474
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt
> > @@ -0,0 +1,47 @@
> > +Altera FPGA/HPS Bridge Driver
> > +
> > +Required properties:
> > +- regs : base address and size for AXI bridge module
> > +- compatible   : Should contain one of:
> > + "altr,socfpga-lwhps2fpga-bridge",
> > + "altr,socfpga-hps2fpga-bridge", or
> > + "altr,socfpga-fpga2hps-bridge"
> > +- reset-names  : Should contain one of:
> > + "lwhps2fpga",
> > + "hps2fpga", or
> > + "fpga2hps"
> 
> Names should be the input signal names. Do you need names with only one?

Right, I will use
of_reset_control_get_by_index(dev->of_node, 0) and eliminate
the reset-names here.

> 
> > +- resets   : Phandle and reset specifier for the reset listed in
> > + reset-names
> > +- clocks   : Clocks used by this module.
> > +
> > +Optional properties:
> > +- bridge-enable: 0 if driver should disable bridge at startup.
> > + 1 if driver should enable bridge at startup.
> > + Default is to leave bridge in its current state.
> > +
> > +Example:
> > +   hps_fpgabridge0: fpgabridge@ff40 {
> 
> No underscores.
> 
> fpga-bridge@...

OK, will add these fixes in v18.

> 
> > +   compatible = "altr,socfpga-lwhps2fpga-bridge";
> > +   reg = <0xff40 0x10>;
> > +   resets = <&rst LWHPS2FPGA_RESET>;
> > +   reset-names = "lwhps2fpga";
> > +   clocks = <&l4_main_clk>;
> > +   bridge-enable = <0>;
> > +   };
> > +
> > +   hps_fpgabridge1: fpgabridge@ff50 {
> > +   compatible = "altr,socfpga-hps2fpga-bridge";
> > +   reg = <0xff50 0x1>;
> > +   resets = <&rst HPS2FPGA_RESET>;
> > +   reset-names = "hps2fpga";
> > +   clocks = <&l4_main_clk>;
> > +   bridge-enable = <1>;
> > +   };
> > +
> > +   hps_fpgabridge2: fpgabridge@ff60 {
> > +   compatible = "altr,socfpga-fpga2hps-bridge";
> > +   reg = <0xff60 0x10>;
> > +   resets = <&rst FPGA2HPS_RESET>;
> > +   reset-names = "fpga2hps";
> > +

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[David Miller]

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 08:07:53 -0700

> I can remove CONFIG_SPARC_ADI. It does mean this code will be built
> into 32-bit kernels as well but it will be inactive code.

The code should be built only into obj-$(CONFIG_SPARC64) just like the
rest of the 64-bit specific code.  I don't know why in the world you
would build it into the 32-bit kernel.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[David Miller]

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 08:07:53 -0700

> PR_GET_SPARC_ADICAPS

Put this into a new ELF auxiliary vector entry via ARCH_DLINFO.

So now all that's left is supposedly the TAG stuff, please explain
that to me so I can direct you to the correct existing interface to
provide that as well.

Really, try to avoid prtctl, it's poorly typed and almost worse than
ioctl().
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[Dave Hansen]

On 03/02/2016 12:39 PM, Khalid Aziz wrote:
> --- a/include/uapi/asm-generic/siginfo.h
> +++ b/include/uapi/asm-generic/siginfo.h
> @@ -206,7 +206,10 @@ typedef struct siginfo {
>  #define SEGV_MAPERR  (__SI_FAULT|1)  /* address not mapped to object */
>  #define SEGV_ACCERR  (__SI_FAULT|2)  /* invalid permissions for mapped 
> object */
>  #define SEGV_BNDERR  (__SI_FAULT|3)  /* failed address bound checks */
> -#define NSIGSEGV 3
> +#define SEGV_ACCADI  (__SI_FAULT|4)  /* ADI not enabled for mapped object */
> +#define SEGV_ADIDERR (__SI_FAULT|5)  /* Disrupting MCD error */
> +#define SEGV_ADIPERR (__SI_FAULT|6)  /* Precise MCD exception */
> +#define NSIGSEGV 6

FYI, this will conflict with code in -tip right now:

> http://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?h=mm/pkeys&id=cd0ea35ff5511cde299a61c21a95889b4a71464e

It's not a big deal to resolve, of course.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[Dave Hansen]

On 03/02/2016 12:39 PM, Khalid Aziz wrote:
> +long enable_sparc_adi(unsigned long addr, unsigned long len)
> +{
> + unsigned long end, pagemask;
> + int error;
> + struct vm_area_struct *vma, *vma2;
> + struct mm_struct *mm;
> +
> + if (!ADI_CAPABLE())
> + return -EINVAL;
...

This whole thing with the VMA splitting and so forth looks pretty darn
arch-independent.  Are you sure you need that much arch-specific code
for it, or can you share more of the generic VMA management code?
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 0/10] watchdog: Add support for keepalives triggered by infrastructure

[Guenter Roeck]

Hi Wim,

On Sun, Mar 06, 2016 at 11:49:56AM +0100, Wim Van Sebroeck wrote:
> Hi Guenter,
> 
> > The watchdog infrastructure is currently purely passive, meaning
> > it only passes information from user space to drivers and vice versa.
> > 
[ ... ]
> 
> Patches 1 till 7 of this series has been added to linux-watchdog-next.
> 
Thanks a lot!

Guenter
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[Dave Hansen]

On 03/07/2016 08:06 AM, Khalid Aziz wrote:
> Top 4-bits of sparc64 virtual address are used for version tag only when
> a process has its PSTATE.mcde bit set and it is accessing a memory
> region that has ADI enabled on it (TTE.mcd set) and a version tag was
> set on the virtual address being accessed. These 4-bits retain their
> original semantics in all other cases.

OK, so this effectively reduces the address space of a process using the
feature.  Do we need to do anything explicit to keep an app from using
that address space?  Do we make sure the kernel doesn't place VMAs
there?  Do we respect mmap() hints that try to place memory there?
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[Khalid Aziz]

On 03/07/2016 09:45 AM, David Miller wrote:

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 08:07:53 -0700


I can remove CONFIG_SPARC_ADI. It does mean this code will be built
into 32-bit kernels as well but it will be inactive code.


The code should be built only into obj-$(CONFIG_SPARC64) just like the
rest of the 64-bit specific code.  I don't know why in the world you
would build it into the 32-bit kernel.



You are right. I did not understand you correctly the first time.

Thanks,
Khalid
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[Andy Lutomirski]

On Mon, Mar 7, 2016 at 9:46 AM, Dave Hansen  wrote:
> On 03/07/2016 08:06 AM, Khalid Aziz wrote:
>> Top 4-bits of sparc64 virtual address are used for version tag only when
>> a process has its PSTATE.mcde bit set and it is accessing a memory
>> region that has ADI enabled on it (TTE.mcd set) and a version tag was
>> set on the virtual address being accessed. These 4-bits retain their
>> original semantics in all other cases.
>
> OK, so this effectively reduces the address space of a process using the
> feature.  Do we need to do anything explicit to keep an app from using
> that address space?  Do we make sure the kernel doesn't place VMAs
> there?  Do we respect mmap() hints that try to place memory there?

Also, what happens when someone does this to an aliased page?  This
could be a MAP_SHARED mapping or a not-yet-COWed MAP_ANONYMOUS
mapping.

Also, what am I missing?  Tying these tags to the physical page seems
like a poor design to me.  This seems really awkward to use.

-- 
Andy Lutomirski
AMA Capital Management, LLC
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[Khalid Aziz]

On 03/07/2016 09:56 AM, David Miller wrote:

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 08:07:53 -0700


PR_GET_SPARC_ADICAPS


Put this into a new ELF auxiliary vector entry via ARCH_DLINFO.

So now all that's left is supposedly the TAG stuff, please explain
that to me so I can direct you to the correct existing interface to
provide that as well.

Really, try to avoid prtctl, it's poorly typed and almost worse than
ioctl().



The two remaining operations I am looking at are:

1. Is PSTATE.mcde bit set for the process? PR_SET_SPARC_ADI provides 
this in its return value in the patch I sent.


2. Is TTE.mcd set for a given virtual address? PR_GET_SPARC_ADI_STATUS 
provides this function in the patch I sent.


Setting and clearing version tags can be done entirely from userspace:

while (addr < end) {
asm volatile(
"stxa %1, [%0]ASI_MCD_PRIMARY\n\t"
:
: "r" (addr), "r" (version));
addr += adicap.blksz;
}
so I do not have to add any kernel code for tags.

Thanks,
Khalid
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[Andy Lutomirski]

On Mon, Mar 7, 2016 at 10:04 AM, Khalid Aziz  wrote:
> On 03/07/2016 09:56 AM, David Miller wrote:
>>
>> From: Khalid Aziz 
>> Date: Mon, 7 Mar 2016 08:07:53 -0700
>>
>>> PR_GET_SPARC_ADICAPS
>>
>>
>> Put this into a new ELF auxiliary vector entry via ARCH_DLINFO.
>>
>> So now all that's left is supposedly the TAG stuff, please explain
>> that to me so I can direct you to the correct existing interface to
>> provide that as well.
>>
>> Really, try to avoid prtctl, it's poorly typed and almost worse than
>> ioctl().
>>
>
> The two remaining operations I am looking at are:
>
> 1. Is PSTATE.mcde bit set for the process? PR_SET_SPARC_ADI provides this in
> its return value in the patch I sent.
>
> 2. Is TTE.mcd set for a given virtual address? PR_GET_SPARC_ADI_STATUS
> provides this function in the patch I sent.
>
> Setting and clearing version tags can be done entirely from userspace:
>
> while (addr < end) {
> asm volatile(
> "stxa %1, [%0]ASI_MCD_PRIMARY\n\t"
> :
> : "r" (addr), "r" (version));
> addr += adicap.blksz;
> }
> so I do not have to add any kernel code for tags.

Is the effect of that to change the tag associated with a page to
which the caller has write access?

I sense DoS issues in your future.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[Dave Hansen]

On 03/07/2016 09:53 AM, Andy Lutomirski wrote:
> Also, what am I missing?  Tying these tags to the physical page seems
> like a poor design to me.  This seems really awkward to use.

Yeah, can you describe the structures that store these things?  Surely
the hardware has some kind of lookup tables for them and stores them in
memory _somewhere_.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[Rob Gardner]

On 03/07/2016 10:04 AM, Khalid Aziz wrote:

On 03/07/2016 09:56 AM, David Miller wrote:

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 08:07:53 -0700


PR_GET_SPARC_ADICAPS


Put this into a new ELF auxiliary vector entry via ARCH_DLINFO.

So now all that's left is supposedly the TAG stuff, please explain
that to me so I can direct you to the correct existing interface to
provide that as well.

Really, try to avoid prtctl, it's poorly typed and almost worse than
ioctl().



The two remaining operations I am looking at are:

1. Is PSTATE.mcde bit set for the process? PR_SET_SPARC_ADI provides 
this in its return value in the patch I sent.


2. Is TTE.mcd set for a given virtual address? PR_GET_SPARC_ADI_STATUS 
provides this function in the patch I sent.


Setting and clearing version tags can be done entirely from userspace:

while (addr < end) {
asm volatile(
"stxa %1, [%0]ASI_MCD_PRIMARY\n\t"
:
: "r" (addr), "r" (version));
addr += adicap.blksz;
}
so I do not have to add any kernel code for tags.



What about clearing the tags when the user is done with the memory? You 
can't count on the user to do that, so doesn't the kernel have to do it 
someplace?


Rob


--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[Khalid Aziz]

On 03/07/2016 10:35 AM, Dave Hansen wrote:

On 03/02/2016 12:39 PM, Khalid Aziz wrote:

+long enable_sparc_adi(unsigned long addr, unsigned long len)
+{
+   unsigned long end, pagemask;
+   int error;
+   struct vm_area_struct *vma, *vma2;
+   struct mm_struct *mm;
+
+   if (!ADI_CAPABLE())
+   return -EINVAL;

...

This whole thing with the VMA splitting and so forth looks pretty darn
arch-independent.  Are you sure you need that much arch-specific code
for it, or can you share more of the generic VMA management code?



All of the VMA splitting/merging code is rather generic and is very 
similar to the code for mbind, mlock, madavise and mprotect. Currently 
there is no code sharing across all of these implementations. Maybe that 
should change. In any case, I am looking at changing the interface to go 
through mprotect instead as Dave suggested. I can share the code in 
mprotect in that case. The only arch dependent part will be to set the 
VM_SPARC_ADI flag on the VMA.


Thanks,
Khalid
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[Khalid Aziz]

On 03/07/2016 11:08 AM, Andy Lutomirski wrote:

On Mon, Mar 7, 2016 at 10:04 AM, Khalid Aziz  wrote:

On 03/07/2016 09:56 AM, David Miller wrote:


From: Khalid Aziz 
Date: Mon, 7 Mar 2016 08:07:53 -0700


PR_GET_SPARC_ADICAPS



Put this into a new ELF auxiliary vector entry via ARCH_DLINFO.

So now all that's left is supposedly the TAG stuff, please explain
that to me so I can direct you to the correct existing interface to
provide that as well.

Really, try to avoid prtctl, it's poorly typed and almost worse than
ioctl().



The two remaining operations I am looking at are:

1. Is PSTATE.mcde bit set for the process? PR_SET_SPARC_ADI provides this in
its return value in the patch I sent.

2. Is TTE.mcd set for a given virtual address? PR_GET_SPARC_ADI_STATUS
provides this function in the patch I sent.

Setting and clearing version tags can be done entirely from userspace:

 while (addr < end) {
 asm volatile(
 "stxa %1, [%0]ASI_MCD_PRIMARY\n\t"
 :
 : "r" (addr), "r" (version));
 addr += adicap.blksz;
 }
so I do not have to add any kernel code for tags.


Is the effect of that to change the tag associated with a page to
which the caller has write access?


No, it changes the tag associated with the virtual address for the 
caller. Physical page backing this virtual address is unaffected. Tag 
checking is done for virtual addresses. The one restriction where 
physical address is relevant is when two processes map the same physical 
page, they both have to use the same tag for the virtual addresses that 
map on to the shared physical pages.




I sense DoS issues in your future.



Are you concerned about DoS even if the tag is associated with virtual 
address, not physical address?


Thanks,
Khalid

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[Khalid Aziz]

On 03/07/2016 11:09 AM, Rob Gardner wrote:

On 03/07/2016 10:04 AM, Khalid Aziz wrote:

On 03/07/2016 09:56 AM, David Miller wrote:

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 08:07:53 -0700


PR_GET_SPARC_ADICAPS


Put this into a new ELF auxiliary vector entry via ARCH_DLINFO.

So now all that's left is supposedly the TAG stuff, please explain
that to me so I can direct you to the correct existing interface to
provide that as well.

Really, try to avoid prtctl, it's poorly typed and almost worse than
ioctl().



The two remaining operations I am looking at are:

1. Is PSTATE.mcde bit set for the process? PR_SET_SPARC_ADI provides
this in its return value in the patch I sent.

2. Is TTE.mcd set for a given virtual address? PR_GET_SPARC_ADI_STATUS
provides this function in the patch I sent.

Setting and clearing version tags can be done entirely from userspace:

while (addr < end) {
asm volatile(
"stxa %1, [%0]ASI_MCD_PRIMARY\n\t"
:
: "r" (addr), "r" (version));
addr += adicap.blksz;
}
so I do not have to add any kernel code for tags.



What about clearing the tags when the user is done with the memory? You
can't count on the user to do that, so doesn't the kernel have to do it
someplace?



Tags can be cleared by user by setting tag to 0. Tags are automatically 
cleared by the hardware when the mapping for a virtual address is 
removed from TSB (which is why swappable pages are a problem), so kernel 
does not have to do it as part of clean up.


Thanks,
Khalid

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[Khalid Aziz]

On 03/07/2016 11:12 AM, Dave Hansen wrote:

On 03/07/2016 09:53 AM, Andy Lutomirski wrote:

Also, what am I missing?  Tying these tags to the physical page seems
like a poor design to me.  This seems really awkward to use.


Yeah, can you describe the structures that store these things?  Surely
the hardware has some kind of lookup tables for them and stores them in
memory _somewhere_.



Version tags are tied to virtual addresses, not physical pages.

Where exactly are the tags stored is part of processor architecture and 
I am not privy to that. MMU stores these lookup tables somewhere and 
uses it to authenticate access to virtual addresses. It really is 
irrelevant to kernel how MMU implements access controls as long as we 
have access to the knowledge of how to use it.


Thanks,
Khalid

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[Andy Lutomirski]

On Mon, Mar 7, 2016 at 10:22 AM, Khalid Aziz  wrote:
> On 03/07/2016 11:08 AM, Andy Lutomirski wrote:
>>
>> On Mon, Mar 7, 2016 at 10:04 AM, Khalid Aziz 
>> wrote:
>>>
>>> On 03/07/2016 09:56 AM, David Miller wrote:


 From: Khalid Aziz 
 Date: Mon, 7 Mar 2016 08:07:53 -0700

> PR_GET_SPARC_ADICAPS



 Put this into a new ELF auxiliary vector entry via ARCH_DLINFO.

 So now all that's left is supposedly the TAG stuff, please explain
 that to me so I can direct you to the correct existing interface to
 provide that as well.

 Really, try to avoid prtctl, it's poorly typed and almost worse than
 ioctl().

>>>
>>> The two remaining operations I am looking at are:
>>>
>>> 1. Is PSTATE.mcde bit set for the process? PR_SET_SPARC_ADI provides this
>>> in
>>> its return value in the patch I sent.
>>>
>>> 2. Is TTE.mcd set for a given virtual address? PR_GET_SPARC_ADI_STATUS
>>> provides this function in the patch I sent.
>>>
>>> Setting and clearing version tags can be done entirely from userspace:
>>>
>>>  while (addr < end) {
>>>  asm volatile(
>>>  "stxa %1, [%0]ASI_MCD_PRIMARY\n\t"
>>>  :
>>>  : "r" (addr), "r" (version));
>>>  addr += adicap.blksz;
>>>  }
>>> so I do not have to add any kernel code for tags.
>>
>>
>> Is the effect of that to change the tag associated with a page to
>> which the caller has write access?
>
>
> No, it changes the tag associated with the virtual address for the caller.
> Physical page backing this virtual address is unaffected. Tag checking is
> done for virtual addresses. The one restriction where physical address is
> relevant is when two processes map the same physical page, they both have to
> use the same tag for the virtual addresses that map on to the shared
> physical pages.

Slow down, please.  *Why* do the tags for two different VAs that map
to the same PA have to match?  What goes wrong if they don't, and why
is requiring them to be the same a good idea?

>
>>
>> I sense DoS issues in your future.
>>
>
> Are you concerned about DoS even if the tag is associated with virtual
> address, not physical address?

Yes, absolutely.

fd = open("/lib/ld.so");
mmap(fd)
stxa to write the tag

*boom*, presumably, because the tags apparently have to match for all mappings.

What data structure or structures changes when this stxa instruction happens?

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 0/10] watchdog: Add support for keepalives triggered by infrastructure

[Wim Van Sebroeck]

Hi Guenter,

> Hi Wim,
> 
> On Sun, Mar 06, 2016 at 11:49:56AM +0100, Wim Van Sebroeck wrote:
> > Hi Guenter,
> > 
> > > The watchdog infrastructure is currently purely passive, meaning
> > > it only passes information from user space to drivers and vice versa.
> > > 
> [ ... ]
> > 
> > Patches 1 till 7 of this series has been added to linux-watchdog-next.
> > 
> Thanks a lot!

Did you receive any feedback/test-results on patches 8 till 10 ?

Kind regards,
Wim.

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[Andy Lutomirski]

On Mon, Mar 7, 2016 at 10:39 AM, Khalid Aziz  wrote:
> On 03/07/2016 11:12 AM, Dave Hansen wrote:
>>
>> On 03/07/2016 09:53 AM, Andy Lutomirski wrote:
>>>
>>> Also, what am I missing?  Tying these tags to the physical page seems
>>> like a poor design to me.  This seems really awkward to use.
>>
>>
>> Yeah, can you describe the structures that store these things?  Surely
>> the hardware has some kind of lookup tables for them and stores them in
>> memory _somewhere_.
>>
>
> Version tags are tied to virtual addresses, not physical pages.
>
> Where exactly are the tags stored is part of processor architecture and I am
> not privy to that. MMU stores these lookup tables somewhere and uses it to
> authenticate access to virtual addresses. It really is irrelevant to kernel
> how MMU implements access controls as long as we have access to the
> knowledge of how to use it.
>

Can you translate this for people who don't know all the SPARC acronyms?

x86 has an upcoming feature called protection keys.  A page of virtual
memory has a protection key, which is a number from 0 through 16.  The
master copy is in the PTE, i.e. page table entry, which is a
software-managed data structure in memory and is exactly the thing
that Linux calls "pte".  The processor can cache that value in the TLB
(translation lookaside buffer), which is a hardware cache that caches
PTEs.  On access to a page of virtual memory, the processor does a
certain calculation involving a new register called PKRU and the
protection key and may deny access.

Hopefully that description makes sense even to people completely
unfamiliar with x86.

Can you try something similar for SPARC?  So far I'm lost, because
you've said that the ADI tag is associated with a VA, but it has to
match for aliases, and you've mentioned a bunch of acronyms, and I
have no clue what's going on.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[David Miller]

From: Dave Hansen 
Date: Mon, 7 Mar 2016 09:35:57 -0800

> On 03/02/2016 12:39 PM, Khalid Aziz wrote:
>> +long enable_sparc_adi(unsigned long addr, unsigned long len)
>> +{
>> +unsigned long end, pagemask;
>> +int error;
>> +struct vm_area_struct *vma, *vma2;
>> +struct mm_struct *mm;
>> +
>> +if (!ADI_CAPABLE())
>> +return -EINVAL;
> ...
> 
> This whole thing with the VMA splitting and so forth looks pretty darn
> arch-independent.  Are you sure you need that much arch-specific code
> for it, or can you share more of the generic VMA management code?

This is exactly what I have suggested to him, and he has agreed to pursue.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[David Miller]

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 11:04:38 -0700

> On 03/07/2016 09:56 AM, David Miller wrote:
>> From: Khalid Aziz 
>> Date: Mon, 7 Mar 2016 08:07:53 -0700
>>
>>> PR_GET_SPARC_ADICAPS
>>
>> Put this into a new ELF auxiliary vector entry via ARCH_DLINFO.
>>
>> So now all that's left is supposedly the TAG stuff, please explain
>> that to me so I can direct you to the correct existing interface to
>> provide that as well.
>>
>> Really, try to avoid prtctl, it's poorly typed and almost worse than
>> ioctl().
>>
> 
> The two remaining operations I am looking at are:
> 
> 1. Is PSTATE.mcde bit set for the process? PR_SET_SPARC_ADI provides
> this in its return value in the patch I sent.

Unnecessary.  If any ADI mappings exist then mcde is set, otherwise it is
clear.  This is internal state and the application has no need to every
set nor query it.

It is implicit from the mprotect() calls the user makes to enable ADI
regions.

> 2. Is TTE.mcd set for a given virtual address? PR_GET_SPARC_ADI_STATUS
> provides this function in the patch I sent.

Again, implied by the mprotect() calls.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[David Miller]

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 11:24:54 -0700

> Tags can be cleared by user by setting tag to 0. Tags are
> automatically cleared by the hardware when the mapping for a virtual
> address is removed from TSB (which is why swappable pages are a
> problem), so kernel does not have to do it as part of clean up.

You might be able to crib some bits for the Tag in the swp_entry_t, it's
64-bit and you can therefore steal bits from the offset field.

That way you'll have the ADI tag in the page tables, ready to re-install
at swapin time.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[David Miller]

From: Andy Lutomirski 
Date: Mon, 7 Mar 2016 10:49:57 -0800

> What data structure or structures changes when this stxa instruction happens?

An internal table, maintained by the CPU and/or hypervisor, and if in physical
addresses then in a region which is only accessible by the hypervisor.

The table is not accessible by the kernel at all via loads or stores.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[David Miller]

From: Andy Lutomirski 
Date: Mon, 7 Mar 2016 10:53:23 -0800

> x86 has an upcoming feature called protection keys.  A page of virtual
> memory has a protection key, which is a number from 0 through 16.  The
> master copy is in the PTE, i.e. page table entry, which is a
> software-managed data structure in memory and is exactly the thing
> that Linux calls "pte".  The processor can cache that value in the TLB
> (translation lookaside buffer), which is a hardware cache that caches
> PTEs.  On access to a page of virtual memory, the processor does a
> certain calculation involving a new register called PKRU and the
> protection key and may deny access.

ADI is similar, except the "keys" (or "tags") are stored externally
rather than in the PTEs.  A bit in the PTE is used to enable tag match
checking.

The tags live in an external table, which is populated by ASI store
instructions.  The location of the table is implementation specific,
it could be hypervisor or CPU managed, but if stored in memory it is
to a region of memory accessible only to the hypervisor at best.

Khalid, maybe you should share notes with the folks working on x86
protection keys.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 01/11] EDAC: Altera L2 Kconfig change from select to depends upon.

[tthayer]

From: Thor Thayer 

Force L2 cache dependency instead of forcing selection of
L2 cache.

Signed-off-by: Thor Thayer 
---
v2 No change
---
 drivers/edac/Kconfig |5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
index 37755e6..6ca7474 100644
--- a/drivers/edac/Kconfig
+++ b/drivers/edac/Kconfig
@@ -378,12 +378,11 @@ config EDAC_ALTERA
 
 config EDAC_ALTERA_L2C
bool "Altera L2 Cache ECC"
-   depends on EDAC_ALTERA=y
-   select CACHE_L2X0
+   depends on EDAC_ALTERA=y && CACHE_L2X0
help
  Support for error detection and correction on the
  Altera L2 cache Memory for Altera SoCs. This option
- requires L2 cache so it will force that selection.
+ requires L2 cache.
 
 config EDAC_ALTERA_OCRAM
bool "Altera On-Chip RAM ECC"
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 0/11] Series adding Altera Arria10 L2 Cache EDAC

[tthayer]

This version splits the larger patch in V1 into smaller,
patches.

 [PATCHv2 01/11] EDAC: Altera L2 Kconfig change from select to
 [PATCHv2 02/11] EDAC, altera: Move Device structs and defines to
 [PATCHv2 03/11] EDAC, altera: Add register offset for ECC Enable
 [PATCHv2 04/11] EDAC, altera: Add register offset for ECC Error
 [PATCHv2 05/11] EDAC, altera: Add register offset for ECC Error
 [PATCHv2 06/11] EDAC, altera: Add IRQ flags to private data struct
 [PATCHv2 07/11] EDAC, altera: Add status offset & masks
 [PATCHv2 08/11] Documentation: dt: socfpga: Add Altera Arria10 L2
 [PATCHv2 09/11] EDAC, altera: Addition of Arria10 L2 Cache ECC
 [PATCHv2 10/11] ARM: socfpga: Enable Arria10 L2 cache ECC on startup
 [PATCHv2 11/11] ARM: dts: Add Altera Arria10 L2 Cache EDAC
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 05/11] EDAC, altera: Add register offset for ECC Error Clear

[tthayer]

From: Thor Thayer 

In preparation for the Arria10 peripheral ECCs, a register
offset from the ECC base was added to the private data
structure to index to the error clear register. Since
the Arria10 L2 cache ECC registers are not contiguous,
a status base address was added.

Signed-off-by: Thor Thayer 
---
v2: Split large patch into smaller patches. Add an ECC
error clear offset to support the different register
layout of Arria10 peripheral ECCs.
---
 drivers/edac/altera_edac.c |   10 --
 drivers/edac/altera_edac.h |2 ++
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
index 9e62a49..c28cd78 100644
--- a/drivers/edac/altera_edac.c
+++ b/drivers/edac/altera_edac.c
@@ -556,15 +556,16 @@ static irqreturn_t altr_edac_device_handler(int irq, void 
*dev_id)
struct edac_device_ctl_info *dci = dev_id;
struct altr_edac_device_dev *drvdata = dci->pvt_info;
const struct edac_device_prv_data *priv = drvdata->data;
+   void __iomem *clear_addr = drvdata->status + priv->clear_err_ofst;
 
if (irq == drvdata->sb_irq) {
if (priv->ce_clear_mask)
-   writel(priv->ce_clear_mask, drvdata->base);
+   writel(priv->ce_clear_mask, clear_addr);
edac_device_handle_ce(dci, 0, 0, drvdata->edac_dev_name);
ret_value = IRQ_HANDLED;
} else if (irq == drvdata->db_irq) {
if (priv->ue_clear_mask)
-   writel(priv->ue_clear_mask, drvdata->base);
+   writel(priv->ue_clear_mask, clear_addr);
edac_device_handle_ue(dci, 0, 0, drvdata->edac_dev_name);
panic("\nEDAC:ECC_DEVICE[Uncorrectable errors]\n");
ret_value = IRQ_HANDLED;
@@ -742,6 +743,9 @@ static int altr_edac_device_probe(struct platform_device 
*pdev)
if (!drvdata->base)
goto fail1;
 
+   /* Except for A10 L2 cache, status reg is within alloced base mem */
+   drvdata->status = drvdata->base;
+
/* Get driver specific data for this EDAC device */
drvdata->data = of_match_node(altr_edac_device_of_match, np)->data;
 
@@ -875,6 +879,7 @@ const struct edac_device_prv_data ocramecc_data = {
.setup = altr_ocram_check_deps,
.ce_clear_mask = (ALTR_OCR_ECC_EN | ALTR_OCR_ECC_SERR),
.ue_clear_mask = (ALTR_OCR_ECC_EN | ALTR_OCR_ECC_DERR),
+   .clear_err_ofst = ALTR_OCR_ECC_REG_OFFSET,
.dbgfs_name = "altr_ocram_trigger",
.alloc_mem = ocram_alloc_mem,
.free_mem = ocram_free_mem,
@@ -949,6 +954,7 @@ const struct edac_device_prv_data l2ecc_data = {
.setup = altr_l2_check_deps,
.ce_clear_mask = 0,
.ue_clear_mask = 0,
+   .clear_err_ofst = ALTR_L2_ECC_REG_OFFSET,
.dbgfs_name = "altr_l2_trigger",
.alloc_mem = l2_alloc_mem,
.free_mem = l2_free_mem,
diff --git a/drivers/edac/altera_edac.h b/drivers/edac/altera_edac.h
index d4105b0..f15b4ad 100644
--- a/drivers/edac/altera_edac.h
+++ b/drivers/edac/altera_edac.h
@@ -225,6 +225,7 @@ struct edac_device_prv_data {
 struct altr_edac_device_dev *drvdata);
int ce_clear_mask;
int ue_clear_mask;
+   int clear_err_ofst;
char dbgfs_name[20];
void * (*alloc_mem)(size_t size, void **other);
void (*free_mem)(void *p, size_t size, void *other);
@@ -238,6 +239,7 @@ struct edac_device_prv_data {
 
 struct altr_edac_device_dev {
void __iomem *base;
+   void __iomem *status;
int sb_irq;
int db_irq;
const struct edac_device_prv_data *data;
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 08/11] Documentation: dt: socfpga: Add Altera Arria10 L2 cache binding

[tthayer]

From: Thor Thayer 

Add the device tree binding string needed to support the Altera L2
cache on the Arria10 chip.

Signed-off-by: Thor Thayer 
Acked-by: Rob Herring 
---
v2 Correct spelling of Arria10 in patch title.
---
 .../bindings/arm/altera/socfpga-eccmgr.txt |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt 
b/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt
index 885f93d..4cea386 100644
--- a/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt
+++ b/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt
@@ -13,7 +13,8 @@ Subcomponents:
 
 L2 Cache ECC
 Required Properties:
-- compatible : Should be "altr,socfpga-l2-ecc"
+- compatible : Should be "altr,socfpga-l2-ecc" or
+  "altr,socfpga-a10-l2-ecc"
 - reg : Address and size for ECC error interrupt clear registers.
 - interrupts : Should be single bit error interrupt, then double bit error
interrupt. Note the rising edge type.
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 02/11] EDAC, altera: Move Device structs and defines to header file

[tthayer]

From: Thor Thayer 

Move the device structs and defines to altera_edac.h in preparation
for adding the Arria10 L2 cache ECC.

Signed-off-by: Thor Thayer 
---
v2: Split original patch into smaller patches. Move private data
and defines into header file.
---
 drivers/edac/altera_edac.c |   43 ---
 drivers/edac/altera_edac.h |   43 +++
 2 files changed, 43 insertions(+), 43 deletions(-)

diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
index 63e4209..eee7a39 100644
--- a/drivers/edac/altera_edac.c
+++ b/drivers/edac/altera_edac.c
@@ -78,27 +78,6 @@ static const struct altr_sdram_prv_data a10_data = {
.ue_set_mask= A10_DIAGINT_TDERRA_MASK,
 };
 
-/** EDAC Device Defines **/
-
-/* OCRAM ECC Management Group Defines */
-#define ALTR_MAN_GRP_OCRAM_ECC_OFFSET   0x04
-#define ALTR_OCR_ECC_EN BIT(0)
-#define ALTR_OCR_ECC_INJS   BIT(1)
-#define ALTR_OCR_ECC_INJD   BIT(2)
-#define ALTR_OCR_ECC_SERR   BIT(3)
-#define ALTR_OCR_ECC_DERR   BIT(4)
-
-/* L2 ECC Management Group Defines */
-#define ALTR_MAN_GRP_L2_ECC_OFFSET  0x00
-#define ALTR_L2_ECC_EN  BIT(0)
-#define ALTR_L2_ECC_INJSBIT(1)
-#define ALTR_L2_ECC_INJDBIT(2)
-
-#define ALTR_UE_TRIGGER_CHAR'U'   /* Trigger for UE */
-#define ALTR_TRIGGER_READ_WRD_CNT   32/* Line size x 4 */
-#define ALTR_TRIG_OCRAM_BYTE_SIZE   128   /* Line size x 4 */
-#define ALTR_TRIG_L2C_BYTE_SIZE 4096  /* Full Page */
-
 /*** EDAC Memory Controller Functions /
 
 /* The SDRAM controller uses the EDAC Memory Controller framework.   */
@@ -571,28 +550,6 @@ module_platform_driver(altr_edac_driver);
 const struct edac_device_prv_data ocramecc_data;
 const struct edac_device_prv_data l2ecc_data;
 
-struct edac_device_prv_data {
-   int (*setup)(struct platform_device *pdev, void __iomem *base);
-   int ce_clear_mask;
-   int ue_clear_mask;
-   char dbgfs_name[20];
-   void * (*alloc_mem)(size_t size, void **other);
-   void (*free_mem)(void *p, size_t size, void *other);
-   int ecc_enable_mask;
-   int ce_set_mask;
-   int ue_set_mask;
-   int trig_alloc_sz;
-};
-
-struct altr_edac_device_dev {
-   void __iomem *base;
-   int sb_irq;
-   int db_irq;
-   const struct edac_device_prv_data *data;
-   struct dentry *debugfs_dir;
-   char *edac_dev_name;
-};
-
 static irqreturn_t altr_edac_device_handler(int irq, void *dev_id)
 {
irqreturn_t ret_value = IRQ_NONE;
diff --git a/drivers/edac/altera_edac.h b/drivers/edac/altera_edac.h
index 953077d..e531da4 100644
--- a/drivers/edac/altera_edac.h
+++ b/drivers/edac/altera_edac.h
@@ -195,4 +195,47 @@ struct altr_sdram_mc_data {
const struct altr_sdram_prv_data *data;
 };
 
+/** EDAC Device Defines **/
+
+#define ALTR_UE_TRIGGER_CHAR'U'   /* Trigger for UE */
+#define ALTR_TRIGGER_READ_WRD_CNT   32/* Line size x 4 */
+#define ALTR_TRIG_OCRAM_BYTE_SIZE   128   /* Line size x 4 */
+#define ALTR_TRIG_L2C_BYTE_SIZE 4096  /* Full Page */
+
+/* OCRAM ECC Management Group Defines */
+#define ALTR_MAN_GRP_OCRAM_ECC_OFFSET   0x04
+#define ALTR_OCR_ECC_EN BIT(0)
+#define ALTR_OCR_ECC_INJS   BIT(1)
+#define ALTR_OCR_ECC_INJD   BIT(2)
+#define ALTR_OCR_ECC_SERR   BIT(3)
+#define ALTR_OCR_ECC_DERR   BIT(4)
+
+/* L2 ECC Management Group Defines */
+#define ALTR_MAN_GRP_L2_ECC_OFFSET  0x00
+#define ALTR_L2_ECC_EN  BIT(0)
+#define ALTR_L2_ECC_INJSBIT(1)
+#define ALTR_L2_ECC_INJDBIT(2)
+
+struct edac_device_prv_data {
+   int (*setup)(struct platform_device *pdev, void __iomem *base);
+   int ce_clear_mask;
+   int ue_clear_mask;
+   char dbgfs_name[20];
+   void * (*alloc_mem)(size_t size, void **other);
+   void (*free_mem)(void *p, size_t size, void *other);
+   int ecc_enable_mask;
+   int ce_set_mask;
+   int ue_set_mask;
+   int trig_alloc_sz;
+};
+
+struct altr_edac_device_dev {
+   void __iomem *base;
+   int sb_irq;
+   int db_irq;
+   const struct edac_device_prv_data *data;
+   struct dentry *debugfs_dir;
+   char *edac_dev_name;
+};
+
 #endif /* #ifndef _ALTERA_EDAC_H */
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 03/11] EDAC, altera: Add register offset for ECC Enable

[tthayer]

From: Thor Thayer 

In preparation for the Arria10 peripheral ECCs, a register
offset from the ECC base was added to the private data
structure to index into the ECC enable register.

Signed-off-by: Thor Thayer 
---
v2: Split large patch into smaller patches. Add an ECC
control offset to support the different register layout
of Arria10 peripheral ECCs.
---
 drivers/edac/altera_edac.c |   20 +++-
 drivers/edac/altera_edac.h |8 +++-
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
index eee7a39..138446c 100644
--- a/drivers/edac/altera_edac.c
+++ b/drivers/edac/altera_edac.c
@@ -746,7 +746,7 @@ static int altr_edac_device_probe(struct platform_device 
*pdev)
 
/* Check specific dependencies for the module */
if (drvdata->data->setup) {
-   res = drvdata->data->setup(pdev, drvdata->base);
+   res = drvdata->data->setup(pdev, drvdata);
if (res)
goto fail1;
}
@@ -857,9 +857,12 @@ static void ocram_free_mem(void *p, size_t size, void 
*other)
  * memory will cause CE/UE errors possibly causing an ABORT.
  */
 static int altr_ocram_check_deps(struct platform_device *pdev,
-void __iomem *base)
+struct altr_edac_device_dev *drvdata)
 {
-   if (readl(base) & ALTR_OCR_ECC_EN)
+   void __iomem  *base = drvdata->base;
+   const struct edac_device_prv_data *prv = drvdata->data;
+
+   if (readl(base + prv->ecc_en_ofst) & prv->ecc_enable_mask)
return 0;
 
edac_printk(KERN_ERR, EDAC_DEVICE,
@@ -875,6 +878,7 @@ const struct edac_device_prv_data ocramecc_data = {
.alloc_mem = ocram_alloc_mem,
.free_mem = ocram_free_mem,
.ecc_enable_mask = ALTR_OCR_ECC_EN,
+   .ecc_en_ofst = ALTR_OCR_ECC_REG_OFFSET,
.ce_set_mask = (ALTR_OCR_ECC_EN | ALTR_OCR_ECC_INJS),
.ue_set_mask = (ALTR_OCR_ECC_EN | ALTR_OCR_ECC_INJD),
.trig_alloc_sz = ALTR_TRIG_OCRAM_BYTE_SIZE,
@@ -924,10 +928,15 @@ static void l2_free_mem(void *p, size_t size, void *other)
  * Note that L2 Cache Enable is forced at build time.
  */
 static int altr_l2_check_deps(struct platform_device *pdev,
- void __iomem *base)
+ struct altr_edac_device_dev *drvdata)
 {
-   if (readl(base) & ALTR_L2_ECC_EN)
+   void __iomem  *base = drvdata->base;
+   const struct edac_device_prv_data *prv = drvdata->data;
+
+   if ((readl(base + prv->ecc_en_ofst) & prv->ecc_enable_mask) ==
+prv->ecc_enable_mask) {
return 0;
+   }
 
edac_printk(KERN_ERR, EDAC_DEVICE,
"L2: No ECC present, or ECC disabled\n");
@@ -942,6 +951,7 @@ const struct edac_device_prv_data l2ecc_data = {
.alloc_mem = l2_alloc_mem,
.free_mem = l2_free_mem,
.ecc_enable_mask = ALTR_L2_ECC_EN,
+   .ecc_en_ofst = ALTR_L2_ECC_REG_OFFSET,
.ce_set_mask = (ALTR_L2_ECC_EN | ALTR_L2_ECC_INJS),
.ue_set_mask = (ALTR_L2_ECC_EN | ALTR_L2_ECC_INJD),
.trig_alloc_sz = ALTR_TRIG_L2C_BYTE_SIZE,
diff --git a/drivers/edac/altera_edac.h b/drivers/edac/altera_edac.h
index e531da4..54e2742 100644
--- a/drivers/edac/altera_edac.h
+++ b/drivers/edac/altera_edac.h
@@ -204,6 +204,7 @@ struct altr_sdram_mc_data {
 
 /* OCRAM ECC Management Group Defines */
 #define ALTR_MAN_GRP_OCRAM_ECC_OFFSET   0x04
+#define ALTR_OCR_ECC_REG_OFFSET 0x00
 #define ALTR_OCR_ECC_EN BIT(0)
 #define ALTR_OCR_ECC_INJS   BIT(1)
 #define ALTR_OCR_ECC_INJD   BIT(2)
@@ -212,18 +213,23 @@ struct altr_sdram_mc_data {
 
 /* L2 ECC Management Group Defines */
 #define ALTR_MAN_GRP_L2_ECC_OFFSET  0x00
+#define ALTR_L2_ECC_REG_OFFSET  0x00
 #define ALTR_L2_ECC_EN  BIT(0)
 #define ALTR_L2_ECC_INJSBIT(1)
 #define ALTR_L2_ECC_INJDBIT(2)
 
+struct altr_edac_device_dev;
+
 struct edac_device_prv_data {
-   int (*setup)(struct platform_device *pdev, void __iomem *base);
+   int (*setup)(struct platform_device *pdev,
+struct altr_edac_device_dev *drvdata);
int ce_clear_mask;
int ue_clear_mask;
char dbgfs_name[20];
void * (*alloc_mem)(size_t size, void **other);
void (*free_mem)(void *p, size_t size, void *other);
int ecc_enable_mask;
+   int ecc_en_ofst;
int ce_set_mask;
int ue_set_mask;
int trig_alloc_sz;
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 07/11] EDAC, altera: Add status offset & masks

[tthayer]

From: Thor Thayer 

In preparation for the Arria10 peripheral ECCs, the IRQ
status needs to be determined because the IRQs are shared.
The IRQ status register is read to determine if the IRQ
was for this ECC peripheral. Cyclone5 and Arria5 have
dedicated IRQs so the confirmation mechanism is not
required and the mask is set to 0.

Signed-off-by: Thor Thayer 
---
v2: Split large patch into smaller patches. Determine
if IRQ matches this ECC peripheral before handling it.
---
 drivers/edac/altera_edac.c |   41 +++--
 drivers/edac/altera_edac.h |3 +++
 2 files changed, 34 insertions(+), 10 deletions(-)

diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
index fd73a77..11b7291 100644
--- a/drivers/edac/altera_edac.c
+++ b/drivers/edac/altera_edac.c
@@ -556,19 +556,32 @@ static irqreturn_t altr_edac_device_handler(int irq, void 
*dev_id)
struct edac_device_ctl_info *dci = dev_id;
struct altr_edac_device_dev *drvdata = dci->pvt_info;
const struct edac_device_prv_data *priv = drvdata->data;
+   void __iomem *status_addr = drvdata->status + priv->err_status_ofst;
void __iomem *clear_addr = drvdata->status + priv->clear_err_ofst;
 
+   /*
+* CycloneV is directly mapped to a specific IRQ. Arria10
+* shares the IRQ with other ECCs so we must match first.
+*/
if (irq == drvdata->sb_irq) {
-   if (priv->ce_clear_mask)
-   writel(priv->ce_clear_mask, clear_addr);
-   edac_device_handle_ce(dci, 0, 0, drvdata->edac_dev_name);
-   ret_value = IRQ_HANDLED;
+   if (!priv->ce_status_mask ||
+   (priv->ce_status_mask & readl(status_addr))) {
+   if (priv->ce_clear_mask)
+   writel(priv->ce_clear_mask, clear_addr);
+   edac_device_handle_ce(dci, 0, 0,
+ drvdata->edac_dev_name);
+   ret_value = IRQ_HANDLED;
+   }
} else if (irq == drvdata->db_irq) {
-   if (priv->ue_clear_mask)
-   writel(priv->ue_clear_mask, clear_addr);
-   edac_device_handle_ue(dci, 0, 0, drvdata->edac_dev_name);
-   panic("\nEDAC:ECC_DEVICE[Uncorrectable errors]\n");
-   ret_value = IRQ_HANDLED;
+   if (!priv->ue_status_mask ||
+   (priv->ue_status_mask & readl(status_addr))) {
+   if (priv->ue_clear_mask)
+   writel(priv->ue_clear_mask, clear_addr);
+   edac_device_handle_ue(dci, 0, 0,
+ drvdata->edac_dev_name);
+   panic("\nEDAC:ECC_DEVICE[Uncorrectable errors]\n");
+   ret_value = IRQ_HANDLED;
+   }
} else {
WARN_ON(1);
}
@@ -882,6 +895,10 @@ const struct edac_device_prv_data ocramecc_data = {
.ce_clear_mask = (ALTR_OCR_ECC_EN | ALTR_OCR_ECC_SERR),
.ue_clear_mask = (ALTR_OCR_ECC_EN | ALTR_OCR_ECC_DERR),
.clear_err_ofst = ALTR_OCR_ECC_REG_OFFSET,
+   /* Cyclone5 & Arria5 have separate IRQs so status = 0 */
+   .ce_status_mask = 0,
+   .ue_status_mask = 0,
+   .err_status_ofst = 0,
.dbgfs_name = "altr_ocram_trigger",
.alloc_mem = ocram_alloc_mem,
.free_mem = ocram_free_mem,
@@ -957,7 +974,11 @@ const struct edac_device_prv_data l2ecc_data = {
.setup = altr_l2_check_deps,
.ce_clear_mask = 0,
.ue_clear_mask = 0,
-   .clear_err_ofst = ALTR_L2_ECC_REG_OFFSET,
+   .clear_err_ofst = ALTR_MAN_GRP_L2_ECC_OFFSET,
+   /* Cyclone5 & Arria5 have separate IRQs so status = 0 */
+   .ce_status_mask = 0,
+   .ue_status_mask = 0,
+   .err_status_ofst = 0,
.dbgfs_name = "altr_l2_trigger",
.alloc_mem = l2_alloc_mem,
.free_mem = l2_free_mem,
diff --git a/drivers/edac/altera_edac.h b/drivers/edac/altera_edac.h
index b262f74..43e0dae 100644
--- a/drivers/edac/altera_edac.h
+++ b/drivers/edac/altera_edac.h
@@ -226,6 +226,9 @@ struct edac_device_prv_data {
int ce_clear_mask;
int ue_clear_mask;
int clear_err_ofst;
+   int ce_status_mask;
+   int ue_status_mask;
+   int err_status_ofst;
char dbgfs_name[20];
void * (*alloc_mem)(size_t size, void **other);
void (*free_mem)(void *p, size_t size, void *other);
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 06/11] EDAC, altera: Add IRQ flags to private data struct

[tthayer]

From: Thor Thayer 

In preparation for the Arria10 peripheral ECCs, irq_flags
was added to the private data structure because Arria10
uses shared IRQs while Cyclone5/Arria5 have exclusive IRQs.

Signed-off-by: Thor Thayer 
---
v2: Split large patch into smaller patches. Add irq_flags
to the private data structure.
---
 drivers/edac/altera_edac.c |8 ++--
 drivers/edac/altera_edac.h |1 +
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
index c28cd78..fd73a77 100644
--- a/drivers/edac/altera_edac.c
+++ b/drivers/edac/altera_edac.c
@@ -759,14 +759,16 @@ static int altr_edac_device_probe(struct platform_device 
*pdev)
drvdata->sb_irq = platform_get_irq(pdev, 0);
res = devm_request_irq(&pdev->dev, drvdata->sb_irq,
   altr_edac_device_handler,
-  0, dev_name(&pdev->dev), dci);
+  drvdata->data->irq_flags,
+  dev_name(&pdev->dev), dci);
if (res)
goto fail1;
 
drvdata->db_irq = platform_get_irq(pdev, 1);
res = devm_request_irq(&pdev->dev, drvdata->db_irq,
   altr_edac_device_handler,
-  0, dev_name(&pdev->dev), dci);
+  drvdata->data->irq_flags,
+  dev_name(&pdev->dev), dci);
if (res)
goto fail1;
 
@@ -889,6 +891,7 @@ const struct edac_device_prv_data ocramecc_data = {
.ue_set_mask = (ALTR_OCR_ECC_EN | ALTR_OCR_ECC_INJD),
.set_err_ofst = ALTR_OCR_ECC_REG_OFFSET,
.trig_alloc_sz = ALTR_TRIG_OCRAM_BYTE_SIZE,
+   .irq_flags = 0,
 };
 
 #endif /* CONFIG_EDAC_ALTERA_OCRAM */
@@ -964,6 +967,7 @@ const struct edac_device_prv_data l2ecc_data = {
.ue_set_mask = (ALTR_L2_ECC_EN | ALTR_L2_ECC_INJD),
.set_err_ofst = ALTR_L2_ECC_REG_OFFSET,
.trig_alloc_sz = ALTR_TRIG_L2C_BYTE_SIZE,
+   .irq_flags = 0,
 };
 
 #endif /* CONFIG_EDAC_ALTERA_L2C */
diff --git a/drivers/edac/altera_edac.h b/drivers/edac/altera_edac.h
index f15b4ad..b262f74 100644
--- a/drivers/edac/altera_edac.h
+++ b/drivers/edac/altera_edac.h
@@ -235,6 +235,7 @@ struct edac_device_prv_data {
int ue_set_mask;
int set_err_ofst;
int trig_alloc_sz;
+   int irq_flags;
 };
 
 struct altr_edac_device_dev {
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 10/11] ARM: socfpga: Enable Arria10 L2 cache ECC on startup

[tthayer]

From: Thor Thayer 

Enable ECC for Arria10 L2 cache on machine startup. The ECC has to be
enabled before data is stored in memory otherwise the ECC will fail
on reads.
Use DT_MACHINE to select Arria10 L2 cache function.

Signed-off-by: Thor Thayer 
---
v2: Split into 2 separate functions selected with DT_MACHINE.
---
 arch/arm/mach-socfpga/core.h |1 +
 arch/arm/mach-socfpga/l2_cache.c |   49 ++
 arch/arm/mach-socfpga/socfpga.c  |   10 +++-
 3 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-socfpga/core.h b/arch/arm/mach-socfpga/core.h
index 575195b..bfbc78d 100644
--- a/arch/arm/mach-socfpga/core.h
+++ b/arch/arm/mach-socfpga/core.h
@@ -38,6 +38,7 @@ extern void socfpga_init_clocks(void);
 extern void socfpga_sysmgr_init(void);
 void socfpga_init_l2_ecc(void);
 void socfpga_init_ocram_ecc(void);
+void socfpga_init_arria10_l2_ecc(void);
 
 extern void __iomem *sys_manager_base_addr;
 extern void __iomem *rst_manager_base_addr;
diff --git a/arch/arm/mach-socfpga/l2_cache.c b/arch/arm/mach-socfpga/l2_cache.c
index e3907ab..4267c95f 100644
--- a/arch/arm/mach-socfpga/l2_cache.c
+++ b/arch/arm/mach-socfpga/l2_cache.c
@@ -17,6 +17,20 @@
 #include 
 #include 
 
+#include "core.h"
+
+/* A10 System Manager L2 ECC Control register */
+#define A10_MPU_CTRL_L2_ECC_OFST  0x0
+#define A10_MPU_CTRL_L2_ECC_ENBIT(0)
+
+/* A10 System Manager Global IRQ Mask register */
+#define A10_SYSMGR_ECC_INTMASK_CLR_OFST   0x98
+#define A10_SYSMGR_ECC_INTMASK_CLR_L2 BIT(0)
+
+/* A10 System Manager L2 ECC IRQ Clear register */
+#define A10_SYSMGR_MPU_CLEAR_L2_ECC_OFST  0xA8
+#define A10_SYSMGR_MPU_CLEAR_L2_ECC   (BIT(31) | BIT(15))
+
 void socfpga_init_l2_ecc(void)
 {
struct device_node *np;
@@ -39,3 +53,38 @@ void socfpga_init_l2_ecc(void)
writel(0x01, mapped_l2_edac_addr);
iounmap(mapped_l2_edac_addr);
 }
+
+void socfpga_init_arria10_l2_ecc(void)
+{
+   struct device_node *np;
+   void __iomem *mapped_l2_edac_addr;
+
+   /* Find the L2 EDAC device tree node */
+   np = of_find_compatible_node(NULL, NULL, "altr,socfpga-a10-l2-ecc");
+   if (!np) {
+   pr_err("Unable to find socfpga-a10-l2-ecc in dtb\n");
+   return;
+   }
+
+   mapped_l2_edac_addr = of_iomap(np, 0);
+   of_node_put(np);
+   if (!mapped_l2_edac_addr) {
+   pr_err("Unable to find L2 ECC mapping in dtb\n");
+   return;
+   }
+
+   if (!sys_manager_base_addr) {
+   pr_err("System Mananger not mapped for L2 ECC\n");
+   goto exit;
+   }
+   /* Clear any pending IRQs */
+   writel(A10_SYSMGR_MPU_CLEAR_L2_ECC, (sys_manager_base_addr +
+  A10_SYSMGR_MPU_CLEAR_L2_ECC_OFST));
+   /* Enable ECC */
+   writel(A10_SYSMGR_ECC_INTMASK_CLR_L2, sys_manager_base_addr +
+  A10_SYSMGR_ECC_INTMASK_CLR_OFST);
+   writel(A10_MPU_CTRL_L2_ECC_EN, mapped_l2_edac_addr +
+  A10_MPU_CTRL_L2_ECC_OFST);
+exit:
+   iounmap(mapped_l2_edac_addr);
+}
diff --git a/arch/arm/mach-socfpga/socfpga.c b/arch/arm/mach-socfpga/socfpga.c
index 7e0aad2..e9b5b60 100644
--- a/arch/arm/mach-socfpga/socfpga.c
+++ b/arch/arm/mach-socfpga/socfpga.c
@@ -66,6 +66,14 @@ static void __init socfpga_init_irq(void)
socfpga_init_ocram_ecc();
 }
 
+static void __init socfpga_arria10_init_irq(void)
+{
+   irqchip_init();
+   socfpga_sysmgr_init();
+   if (IS_ENABLED(CONFIG_EDAC_ALTERA_L2C))
+   socfpga_init_arria10_l2_ecc();
+}
+
 static void socfpga_cyclone5_restart(enum reboot_mode mode, const char *cmd)
 {
u32 temp;
@@ -113,7 +121,7 @@ static const char *altera_a10_dt_match[] = {
 DT_MACHINE_START(SOCFPGA_A10, "Altera SOCFPGA Arria10")
.l2c_aux_val= 0,
.l2c_aux_mask   = ~0,
-   .init_irq   = socfpga_init_irq,
+   .init_irq   = socfpga_arria10_init_irq,
.restart= socfpga_arria10_restart,
.dt_compat  = altera_a10_dt_match,
 MACHINE_END
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 04/11] EDAC, altera: Add register offset for ECC Error Inject

[tthayer]

From: Thor Thayer 

In preparation for the Arria10 peripheral ECCs, a register
offset from the ECC base was added to the private data
structure to index to the error injection register.

Signed-off-by: Thor Thayer 
---
v2: Split large patch into smaller patches. Add an ECC
error inject offset to support the different register
layout of Arria10 peripheral ECCs.
---
 drivers/edac/altera_edac.c |7 +--
 drivers/edac/altera_edac.h |1 +
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
index 138446c..9e62a49 100644
--- a/drivers/edac/altera_edac.c
+++ b/drivers/edac/altera_edac.c
@@ -622,8 +622,9 @@ static ssize_t altr_edac_device_trig(struct file *file,
if (ACCESS_ONCE(ptemp[i]))
result = -1;
/* Toggle Error bit (it is latched), leave ECC enabled */
-   writel(error_mask, drvdata->base);
-   writel(priv->ecc_enable_mask, drvdata->base);
+   writel(error_mask, (drvdata->base + priv->set_err_ofst));
+   writel(priv->ecc_enable_mask, (drvdata->base +
+  priv->set_err_ofst));
ptemp[i] = i;
}
/* Ensure it has been written out */
@@ -881,6 +882,7 @@ const struct edac_device_prv_data ocramecc_data = {
.ecc_en_ofst = ALTR_OCR_ECC_REG_OFFSET,
.ce_set_mask = (ALTR_OCR_ECC_EN | ALTR_OCR_ECC_INJS),
.ue_set_mask = (ALTR_OCR_ECC_EN | ALTR_OCR_ECC_INJD),
+   .set_err_ofst = ALTR_OCR_ECC_REG_OFFSET,
.trig_alloc_sz = ALTR_TRIG_OCRAM_BYTE_SIZE,
 };
 
@@ -954,6 +956,7 @@ const struct edac_device_prv_data l2ecc_data = {
.ecc_en_ofst = ALTR_L2_ECC_REG_OFFSET,
.ce_set_mask = (ALTR_L2_ECC_EN | ALTR_L2_ECC_INJS),
.ue_set_mask = (ALTR_L2_ECC_EN | ALTR_L2_ECC_INJD),
+   .set_err_ofst = ALTR_L2_ECC_REG_OFFSET,
.trig_alloc_sz = ALTR_TRIG_L2C_BYTE_SIZE,
 };
 
diff --git a/drivers/edac/altera_edac.h b/drivers/edac/altera_edac.h
index 54e2742..d4105b0 100644
--- a/drivers/edac/altera_edac.h
+++ b/drivers/edac/altera_edac.h
@@ -232,6 +232,7 @@ struct edac_device_prv_data {
int ecc_en_ofst;
int ce_set_mask;
int ue_set_mask;
+   int set_err_ofst;
int trig_alloc_sz;
 };
 
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[Khalid Aziz]

On 03/07/2016 12:22 PM, David Miller wrote:

Khalid, maybe you should share notes with the folks working on x86
protection keys.



Good idea. Sparc ADI feature is indeed similar to x86 protection keys 
sounds like.


Thanks,
Khalid
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[Khalid Aziz]

On 03/07/2016 11:49 AM, Andy Lutomirski wrote:

On Mon, Mar 7, 2016 at 10:22 AM, Khalid Aziz  wrote:

No, it changes the tag associated with the virtual address for the caller.
Physical page backing this virtual address is unaffected. Tag checking is
done for virtual addresses. The one restriction where physical address is
relevant is when two processes map the same physical page, they both have to
use the same tag for the virtual addresses that map on to the shared
physical pages.


Slow down, please.  *Why* do the tags for two different VAs that map
to the same PA have to match?  What goes wrong if they don't, and why
is requiring them to be the same a good idea?



Consider this scenario:

1. Process A creates a shm and attaches to it.
2. Process A fills shm with data it wants to share with only known 
processes. It enables ADI and sets tags on the shm.
3. Hacker triggers something like stack overflow on process A, exec's a 
new rogue binary and manages to attach to this shm. MMU knows tags were 
set on the virtual address mapping to the physical pages hosting the 
shm. If MMU does not require the rogue process to set the exact same 
tags on its mapping of the same shm, rogue process has defeated the ADI 
protection easily.


Does this make sense?





I sense DoS issues in your future.



Are you concerned about DoS even if the tag is associated with virtual
address, not physical address?


Yes, absolutely.

fd = open("/lib/ld.so");
mmap(fd)
stxa to write the tag

*boom*, presumably, because the tags apparently have to match for all mappings.



A process can not just write version tags and make the file inaccessible 
to others. It takes three steps to enable ADI:


1. Set PSTATE.mcde for the process.
2. Set TTE.mcd on all PTEs for the virtual addresses ADI is being 
enabled on.

3. Set version tags.

Unless all three steps are taken, tag checking will not be done. stxa 
will fail unless step 2 is completed. In your example, the step of 
setting TTE.mcd will force sharing to stop for the process through 
change_protection(), right?


Thanks for asking these tough questions. These are very helpful in 
refining my implementation and avoiding silly bugs.


--
Khalid


--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[Andy Lutomirski]

On Mon, Mar 7, 2016 at 11:44 AM, Khalid Aziz  wrote:
> On 03/07/2016 11:49 AM, Andy Lutomirski wrote:
>>
>> On Mon, Mar 7, 2016 at 10:22 AM, Khalid Aziz 
>> wrote:
>>>
>>> No, it changes the tag associated with the virtual address for the
>>> caller.
>>> Physical page backing this virtual address is unaffected. Tag checking is
>>> done for virtual addresses. The one restriction where physical address is
>>> relevant is when two processes map the same physical page, they both have
>>> to
>>> use the same tag for the virtual addresses that map on to the shared
>>> physical pages.
>>
>>
>> Slow down, please.  *Why* do the tags for two different VAs that map
>> to the same PA have to match?  What goes wrong if they don't, and why
>> is requiring them to be the same a good idea?
>>
>
> Consider this scenario:
>
> 1. Process A creates a shm and attaches to it.
> 2. Process A fills shm with data it wants to share with only known
> processes. It enables ADI and sets tags on the shm.
> 3. Hacker triggers something like stack overflow on process A, exec's a new
> rogue binary and manages to attach to this shm. MMU knows tags were set on
> the virtual address mapping to the physical pages hosting the shm. If MMU
> does not require the rogue process to set the exact same tags on its mapping
> of the same shm, rogue process has defeated the ADI protection easily.
>
> Does this make sense?

This makes sense, but I still think the design is poor.  If the hacker
gets code execution, then they can trivially brute force the ADI bits.

Also, if this is the use case in mind, shouldn't the ADI bits bet set
on the file, not the mapping?  E.g. have an ioctl on the shmfs file
that sets its ADI bits?

>
>>>

 I sense DoS issues in your future.

>>>
>>> Are you concerned about DoS even if the tag is associated with virtual
>>> address, not physical address?
>>
>>
>> Yes, absolutely.
>>
>> fd = open("/lib/ld.so");
>> mmap(fd)
>> stxa to write the tag
>>
>> *boom*, presumably, because the tags apparently have to match for all
>> mappings.
>>
>
> A process can not just write version tags and make the file inaccessible to
> others. It takes three steps to enable ADI:
>
> 1. Set PSTATE.mcde for the process.
> 2. Set TTE.mcd on all PTEs for the virtual addresses ADI is being enabled
> on.
> 3. Set version tags.
>
> Unless all three steps are taken, tag checking will not be done. stxa will
> fail unless step 2 is completed. In your example, the step of setting
> TTE.mcd will force sharing to stop for the process through
> change_protection(), right?

OK, that makes some sense.

Can a shared page ever have TTE.mcd set?  How does one share a page,
even deliberately, between two processes with cmd set?

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 09/11] EDAC, altera: Addition of Arria10 L2 Cache ECC

[tthayer]

From: Thor Thayer 

Addition of the Arria10 L2 Cache ECC handling. Addition
of private data structure for Arria10 L2 cache ECC and
the initialization function for it.

Signed-off-by: Thor Thayer 
---
v2: Split large patch into smaller patches. Addition of
Arria10 L2 cache dependency check and private data.
---
 drivers/edac/altera_edac.c |   57 
 drivers/edac/altera_edac.h |   21 +++-
 2 files changed, 77 insertions(+), 1 deletion(-)

diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
index 11b7291..8afdf8b 100644
--- a/drivers/edac/altera_edac.c
+++ b/drivers/edac/altera_edac.c
@@ -549,6 +549,7 @@ module_platform_driver(altr_edac_driver);
 
 const struct edac_device_prv_data ocramecc_data;
 const struct edac_device_prv_data l2ecc_data;
+const struct edac_device_prv_data a10_l2ecc_data;
 
 static irqreturn_t altr_edac_device_handler(int irq, void *dev_id)
 {
@@ -687,6 +688,8 @@ static void altr_create_edacdev_dbgfs(struct 
edac_device_ctl_info *edac_dci,
 static const struct of_device_id altr_edac_device_of_match[] = {
 #ifdef CONFIG_EDAC_ALTERA_L2C
{ .compatible = "altr,socfpga-l2-ecc", .data = (void *)&l2ecc_data },
+   { .compatible = "altr,socfpga-a10-l2-ecc",
+ .data = (void *)&a10_l2ecc_data },
 #endif
 #ifdef CONFIG_EDAC_ALTERA_OCRAM
{ .compatible = "altr,socfpga-ocram-ecc",
@@ -970,6 +973,40 @@ static int altr_l2_check_deps(struct platform_device *pdev,
return -ENODEV;
 }
 
+static int altr_a10_l2_check_deps(struct platform_device *pdev,
+ struct altr_edac_device_dev *drvdata)
+{
+   void __iomem  *status_base, *base = drvdata->base;
+   const struct edac_device_prv_data *prv = drvdata->data;
+
+   if ((readl(base + prv->ecc_en_ofst) & prv->ecc_enable_mask) !=
+prv->ecc_enable_mask) {
+   edac_printk(KERN_ERR, EDAC_DEVICE,
+   "L2: No ECC present, or ECC disabled\n");
+   return -ENODEV;
+   }
+
+   /* A10 L2 cache status registers are not contiguous with base */
+   if (!devm_request_mem_region(&pdev->dev, ALTR_A10_L2_ECC_STATUS,
+2 * sizeof(u32), dev_name(&pdev->dev))) {
+   edac_printk(KERN_ERR, EDAC_DEVICE,
+   "Unable to request mem region\n");
+   return -EBUSY;
+   }
+
+   status_base = devm_ioremap(&pdev->dev, ALTR_A10_L2_ECC_STATUS,
+  2 * sizeof(u32));
+   if (!status_base) {
+   edac_printk(KERN_ERR, EDAC_DEVICE,
+   "Unable to ioremap L2 status\n");
+   return -ENOMEM;
+   }
+
+   drvdata->status = status_base;
+
+   return 0;
+}
+
 const struct edac_device_prv_data l2ecc_data = {
.setup = altr_l2_check_deps,
.ce_clear_mask = 0,
@@ -991,6 +1028,26 @@ const struct edac_device_prv_data l2ecc_data = {
.irq_flags = 0,
 };
 
+const struct edac_device_prv_data a10_l2ecc_data = {
+   .setup = altr_a10_l2_check_deps,
+   .ce_clear_mask = ALTR_A10_L2_ECC_SERR_CLR,
+   .ue_clear_mask = ALTR_A10_L2_ECC_MERR_CLR,
+   .clear_err_ofst = ALTR_A10_L2_ECC_CLR_OFST,
+   .ce_status_mask = ALTR_A10_L2_ECC_SERR_PEND,
+   .ue_status_mask = ALTR_A10_L2_ECC_MERR_PEND,
+   .err_status_ofst = ALTR_A10_L2_ECC_STAT_OFST,
+   .dbgfs_name = "altr_l2_trigger",
+   .alloc_mem = l2_alloc_mem,
+   .free_mem = l2_free_mem,
+   .ecc_enable_mask = ALTR_A10_L2_ECC_EN_CTL,
+   .ecc_en_ofst = ALTR_A10_L2_ECC_CTL_OFST,
+   .ce_set_mask = ALTR_A10_L2_ECC_CE_INJ_MASK,
+   .ue_set_mask = ALTR_A10_L2_ECC_UE_INJ_MASK,
+   .set_err_ofst = ALTR_A10_L2_ECC_INJ_OFST,
+   .trig_alloc_sz = ALTR_TRIG_L2C_BYTE_SIZE,
+   .irq_flags = IRQF_SHARED,
+};
+
 #endif /* CONFIG_EDAC_ALTERA_L2C */
 
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/edac/altera_edac.h b/drivers/edac/altera_edac.h
index 43e0dae..a9177c8 100644
--- a/drivers/edac/altera_edac.h
+++ b/drivers/edac/altera_edac.h
@@ -196,12 +196,13 @@ struct altr_sdram_mc_data {
 };
 
 /** EDAC Device Defines **/
-
+/* General Device Trigger Defines */
 #define ALTR_UE_TRIGGER_CHAR'U'   /* Trigger for UE */
 #define ALTR_TRIGGER_READ_WRD_CNT   32/* Line size x 4 */
 #define ALTR_TRIG_OCRAM_BYTE_SIZE   128   /* Line size x 4 */
 #define ALTR_TRIG_L2C_BYTE_SIZE 4096  /* Full Page */
 
+/*** Cyclone5 and Arria5 Defines ***/
 /* OCRAM ECC Management Group Defines */
 #define ALTR_MAN_GRP_OCRAM_ECC_OFFSET   0x04
 #define ALTR_OCR_ECC_REG_OFFSET 0x00
@@ -218,6 +219,24 @@ struct altr_sdram_mc_data {
 #define ALTR_L2_ECC_INJSBIT(1)
 #define ALTR_L2_ECC_INJDBIT(2)
 
+/* Arria10 Defines */
+/* Arria 10 L2 ECC Management Group Defines */
+#defin

[PATCHv2 11/11] ARM: dts: Add Altera Arria10 L2 Cache EDAC devicetree entry

[tthayer]

From: Thor Thayer 

Add the device tree entries needed to support the Altera L2
cache EDAC on the Arria10 chip.

Signed-off-by: Thor Thayer 
---
v2 Match register value (l2-ecc@ffd06010)
---
 arch/arm/boot/dts/socfpga_arria10.dtsi |   14 ++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm/boot/dts/socfpga_arria10.dtsi 
b/arch/arm/boot/dts/socfpga_arria10.dtsi
index cce9e50..44aeb3f 100644
--- a/arch/arm/boot/dts/socfpga_arria10.dtsi
+++ b/arch/arm/boot/dts/socfpga_arria10.dtsi
@@ -599,6 +599,20 @@
reg = <0xffe0 0x4>;
};
 
+   eccmgr: eccmgr@ffd06090 {
+   compatible = "altr,socfpga-ecc-manager";
+   #address-cells = <1>;
+   #size-cells = <1>;
+   ranges;
+
+   l2-ecc@ffd06010 {
+   compatible = "altr,socfpga-a10-l2-ecc";
+   reg = <0xffd06010 0x4>;
+   interrupts = <0 2 IRQ_TYPE_LEVEL_HIGH>,
+<0 0 IRQ_TYPE_LEVEL_HIGH>;
+   };
+   };
+
rst: rstmgr@ffd05000 {
#reset-cells = <1>;
compatible = "altr,rst-mgr";
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] documentation: Make sample code and documentation consistent

[Paul E. McKenney]

On Mon, Mar 07, 2016 at 04:02:14PM +0800, Yao Dongdong wrote:
> In the chapter 'analogy with reader-writer locking', the sample
> code uses spinlock_t in reader-writer case. Just correct it so
> that we can read the document easily.
> 
> Signed-off-by: Yao Dongdong 

Good catch, queued for review.

Thanx, Paul

> ---
>  Documentation/RCU/whatisRCU.txt |   22 +++---
>  1 file changed, 15 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/RCU/whatisRCU.txt b/Documentation/RCU/whatisRCU.txt
> index dc49c67..e33304e 100644
> --- a/Documentation/RCU/whatisRCU.txt
> +++ b/Documentation/RCU/whatisRCU.txt
> @@ -681,22 +681,30 @@ Although RCU can be used in many different ways, a very 
> common use of
>  RCU is analogous to reader-writer locking.  The following unified
>  diff shows how closely related RCU and reader-writer locking can be.
> 
> + @@ -5,5 +5,5 @@ struct el {
> + int data;
> + /* Other data fields */
> + };
> + -rwlock_t listmutex;
> + +spinlock_t listmutex;
> + struct el head;
> +
>   @@ -13,15 +14,15 @@
>   struct list_head *lp;
>   struct el *p;
> 
> - -   read_lock();
> + -   read_lock(&listmutex);
>   -   list_for_each_entry(p, head, lp) {
>   +   rcu_read_lock();
>   +   list_for_each_entry_rcu(p, head, lp) {
>   if (p->key == key) {
>   *result = p->data;
> - -   read_unlock();
> + -   read_unlock(&listmutex);
>   +   rcu_read_unlock();
>   return 1;
>   }
>   }
> - -   read_unlock();
> + -   read_unlock(&listmutex);
>   +   rcu_read_unlock();
>   return 0;
>}
> @@ -732,7 +740,7 @@ Or, for those who prefer a side-by-side listing:
>   5   int data;  5   int data;
>   6   /* Other data fields */6   /* Other data fields */
>   7 };   7 };
> - 8 spinlock_t listmutex;8 spinlock_t listmutex;
> + 8 rwlock_t listmutex;  8 spinlock_t listmutex;
>   9 struct el head;  9 struct el head;
> 
>   1 int search(long key, int *result)1 int search(long key, int *result)
> @@ -740,15 +748,15 @@ Or, for those who prefer a side-by-side listing:
>   3   struct list_head *lp;  3   struct list_head *lp;
>   4   struct el *p;  4   struct el *p;
>   5  5
> - 6   read_lock();   6   rcu_read_lock();
> + 6   read_lock(&listmutex); 6   rcu_read_lock();
>   7   list_for_each_entry(p, head, lp) { 7   list_for_each_entry_rcu(p, head, 
> lp) {
>   8 if (p->key == key) { 8 if (p->key == key) {
>   9   *result = p->data; 9   *result = p->data;
> -10   read_unlock();10   rcu_read_unlock();
> +10   read_unlock(&listmutex);  10   rcu_read_unlock();
>  11   return 1; 11   return 1;
>  12 }   12 }
>  13   } 13   }
> -14   read_unlock();14   rcu_read_unlock();
> +14   read_unlock(&listmutex);  14   rcu_read_unlock();
>  15   return 0; 15   return 0;
>  16 }   16 }
> 
> --
> 1.7.9.5
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[Khalid Aziz]

On 03/07/2016 12:54 PM, Andy Lutomirski wrote:

On Mon, Mar 7, 2016 at 11:44 AM, Khalid Aziz  wrote:


Consider this scenario:

1. Process A creates a shm and attaches to it.
2. Process A fills shm with data it wants to share with only known
processes. It enables ADI and sets tags on the shm.
3. Hacker triggers something like stack overflow on process A, exec's a new
rogue binary and manages to attach to this shm. MMU knows tags were set on
the virtual address mapping to the physical pages hosting the shm. If MMU
does not require the rogue process to set the exact same tags on its mapping
of the same shm, rogue process has defeated the ADI protection easily.

Does this make sense?


This makes sense, but I still think the design is poor.  If the hacker
gets code execution, then they can trivially brute force the ADI bits.


True, with only 16 possible tag values (actually only 14 since 0 and 15 
are reserved values), it is entirely possible to brute force the ADI 
tag. ADI is just another tool one can use to mitigate attacks. A process 
that accesses an ADI enabled memory with invalid tag gets a SIGBUS and 
is terminated. This can trigger alerts on the system and system policies 
could block the next attack. If a daemon is compromised and is forced to 
hand out data from memory it should not be reading (similar to 
heartbleed bug). the daemon itself is terminated with SIGBUS which 
should be enough to alert system admins. A rotating set of tags would 
reduce the risk from brute force attacks. Tags are set on cacheline 
(which is 64 bytes on M7). A single regular sized page can have 128 sets 
of tags. Allowing for 14 possible values for each set, that is a lot of 
possible combinations of tags making it very hard to brute force tags 
for more than a cacheline at a time. There are probably other better 
ways to make the tags harder to crack.




Also, if this is the use case in mind, shouldn't the ADI bits bet set
on the file, not the mapping?  E.g. have an ioctl on the shmfs file
that sets its ADI bits?


Shared data may not always be backed by a file. My understanding is one 
of the use cases is for in-memory databases. This shared space could 
also be used to hand off transactions in flight to other processes. 
These transactions in flight would not be backed by a file. Some of 
these use cases might not use shmfs even. Setting ADI bits at virtual 
address level catches all these cases since what backs the tagged 
virtual address can be anything - a mapped file, mmio space, just plain 
chunk of memory.





A process can not just write version tags and make the file inaccessible to
others. It takes three steps to enable ADI:

1. Set PSTATE.mcde for the process.
2. Set TTE.mcd on all PTEs for the virtual addresses ADI is being enabled
on.
3. Set version tags.

Unless all three steps are taken, tag checking will not be done. stxa will
fail unless step 2 is completed. In your example, the step of setting
TTE.mcd will force sharing to stop for the process through
change_protection(), right?


OK, that makes some sense.

Can a shared page ever have TTE.mcd set?  How does one share a page,
even deliberately, between two processes with cmd set?


For two processes to share a page, their VMAs have to be identical as I 
understand it. If one process has TTE.mcd set (which means vma->vm_flags 
is different) while the other does not, they do not share a page.


Thanks,
Khalid

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[David Miller]

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 13:41:39 -0700

> Shared data may not always be backed by a file. My understanding is
> one of the use cases is for in-memory databases. This shared space
> could also be used to hand off transactions in flight to other
> processes. These transactions in flight would not be backed by a
> file. Some of these use cases might not use shmfs even. Setting ADI
> bits at virtual address level catches all these cases since what backs
> the tagged virtual address can be anything - a mapped file, mmio
> space, just plain chunk of memory.

Frankly the most interesting use case to me is simply finding bugs
and memory scribbles, and for that we're want to be able to ADI
arbitrary memory returned from malloc() and friends.

I personally see ADI more as a debugging than a security feature,
but that's just my view.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[Andy Lutomirski]

On Mon, Mar 7, 2016 at 12:58 PM, David Miller  wrote:
> From: Khalid Aziz 
> Date: Mon, 7 Mar 2016 13:41:39 -0700
>
>> Shared data may not always be backed by a file. My understanding is
>> one of the use cases is for in-memory databases. This shared space
>> could also be used to hand off transactions in flight to other
>> processes. These transactions in flight would not be backed by a
>> file. Some of these use cases might not use shmfs even. Setting ADI
>> bits at virtual address level catches all these cases since what backs
>> the tagged virtual address can be anything - a mapped file, mmio
>> space, just plain chunk of memory.
>
> Frankly the most interesting use case to me is simply finding bugs
> and memory scribbles, and for that we're want to be able to ADI
> arbitrary memory returned from malloc() and friends.
>
> I personally see ADI more as a debugging than a security feature,
> but that's just my view.

The thing that seems awkward to me is that setting, say, ADI=1 seems
almost equivalent to remapping the memory up to 0x10...whatever, and
the latter is a heck of a lot simpler to think about.

-- 
Andy Lutomirski
AMA Capital Management, LLC
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[Khalid Aziz]

On 03/07/2016 10:46 AM, Dave Hansen wrote:

On 03/07/2016 08:06 AM, Khalid Aziz wrote:

Top 4-bits of sparc64 virtual address are used for version tag only when
a process has its PSTATE.mcde bit set and it is accessing a memory
region that has ADI enabled on it (TTE.mcd set) and a version tag was
set on the virtual address being accessed. These 4-bits retain their
original semantics in all other cases.


OK, so this effectively reduces the address space of a process using the
feature.  Do we need to do anything explicit to keep an app from using
that address space?  Do we make sure the kernel doesn't place VMAs
there?  Do we respect mmap() hints that try to place memory there?



Good questions. Isn't set of valid VAs already constrained by VA_BITS 
(set to 44 in arch/sparc/include/asm/processor_64.h)? As I see it we are 
already not using the top 4 bits. Please correct me if I am wrong.


Thanks,
Khalid
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[Khalid Aziz]

On 03/07/2016 01:58 PM, David Miller wrote:

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 13:41:39 -0700


Shared data may not always be backed by a file. My understanding is
one of the use cases is for in-memory databases. This shared space
could also be used to hand off transactions in flight to other
processes. These transactions in flight would not be backed by a
file. Some of these use cases might not use shmfs even. Setting ADI
bits at virtual address level catches all these cases since what backs
the tagged virtual address can be anything - a mapped file, mmio
space, just plain chunk of memory.


Frankly the most interesting use case to me is simply finding bugs
and memory scribbles, and for that we're want to be able to ADI
arbitrary memory returned from malloc() and friends.

I personally see ADI more as a debugging than a security feature,
but that's just my view.



I think that is a very strong use case. It can be a very effective tool 
for debugging especially when it comes to catching wild writes.


--
Khalid
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[Khalid Aziz]

On 03/07/2016 12:09 PM, David Miller wrote:

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 11:04:38 -0700


On 03/07/2016 09:56 AM, David Miller wrote:

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 08:07:53 -0700


PR_GET_SPARC_ADICAPS


Put this into a new ELF auxiliary vector entry via ARCH_DLINFO.

So now all that's left is supposedly the TAG stuff, please explain
that to me so I can direct you to the correct existing interface to
provide that as well.

Really, try to avoid prtctl, it's poorly typed and almost worse than
ioctl().



The two remaining operations I am looking at are:

1. Is PSTATE.mcde bit set for the process? PR_SET_SPARC_ADI provides
this in its return value in the patch I sent.


Unnecessary.  If any ADI mappings exist then mcde is set, otherwise it is
clear.  This is internal state and the application has no need to every
set nor query it.

It is implicit from the mprotect() calls the user makes to enable ADI
regions.


2. Is TTE.mcd set for a given virtual address? PR_GET_SPARC_ADI_STATUS
provides this function in the patch I sent.


Again, implied by the mprotect() calls.



Hi Dave,

I agree with your point of view. PSTATE.mcde and TTE.mcd are set in 
response to request from userspace. If userspace asked for them to be 
set, they already know but it was the database guys that asked for these 
two functions and they are the primary customers for the ADI feature. I 
am not crazy about this idea since this extends the mprotect API even 
further but would you consider using the return value from mprotect to 
indicate if PSTATE.mcde or TTE.mcd were already set on the given address?


Thanks,
Khalid
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[Khalid Aziz]

On 03/07/2016 12:16 PM, David Miller wrote:

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 11:24:54 -0700


Tags can be cleared by user by setting tag to 0. Tags are
automatically cleared by the hardware when the mapping for a virtual
address is removed from TSB (which is why swappable pages are a
problem), so kernel does not have to do it as part of clean up.


You might be able to crib some bits for the Tag in the swp_entry_t, it's
64-bit and you can therefore steal bits from the offset field.

That way you'll have the ADI tag in the page tables, ready to re-install
at swapin time.



That is a possibility but limited in scope. An address range covered by 
a single TTE can have large number of tags. Version tags are set on 
cacheline. In extreme case, one could set a tag for each set of 64-bytes 
in a page. Also tags are set completely in userspace and no transition 
occurs to kernel space, so kernel has no idea of what tags have been 
set. I have not found a way to query the MMU on tags.


I will think some more about it.

Thanks,
Khalid
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[David Miller]

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 14:27:09 -0700

> I agree with your point of view. PSTATE.mcde and TTE.mcd are set in
> response to request from userspace. If userspace asked for them to be
> set, they already know but it was the database guys that asked for
> these two functions and they are the primary customers for the ADI
> feature. I am not crazy about this idea since this extends the
> mprotect API even further but would you consider using the return
> value from mprotect to indicate if PSTATE.mcde or TTE.mcd were already
> set on the given address?

Well, that's the idea.

If the mprotect using MAP_ADI or whatever succeeds, then ADI is
enabled.

Users can thus also pass MAP_ADI as a flag to mmap() to get ADI
protection from the very beginning.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[David Miller]

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 14:33:56 -0700

> On 03/07/2016 12:16 PM, David Miller wrote:
>> From: Khalid Aziz 
>> Date: Mon, 7 Mar 2016 11:24:54 -0700
>>
>>> Tags can be cleared by user by setting tag to 0. Tags are
>>> automatically cleared by the hardware when the mapping for a virtual
>>> address is removed from TSB (which is why swappable pages are a
>>> problem), so kernel does not have to do it as part of clean up.
>>
>> You might be able to crib some bits for the Tag in the swp_entry_t,
>> it's
>> 64-bit and you can therefore steal bits from the offset field.
>>
>> That way you'll have the ADI tag in the page tables, ready to
>> re-install
>> at swapin time.
>>
> 
> That is a possibility but limited in scope. An address range covered
> by a single TTE can have large number of tags. Version tags are set on
> cacheline. In extreme case, one could set a tag for each set of
> 64-bytes in a page. Also tags are set completely in userspace and no
> transition occurs to kernel space, so kernel has no idea of what tags
> have been set. I have not found a way to query the MMU on tags.
> 
> I will think some more about it.

That would mean that ADI is impossible to use for swappable memory.

...

If that's true I'm extremely disappointed that they devoted so much
silicon and engineering to this feature yet didn't take that one
critical step to make it generally useful. :(

We could have a way to do this via the kernel, wherein the user has a
contract with us.  Basically we have a call to pass the Tags (what
granularity to use for this is a design point, pages, cache lines,
etc.)  into the kernel and the user agrees not to change them behind
the kernel's back.

In return the kernel agrees to restore the tags upon swapin.

So we could support something for swappable pages, it would just be
more work.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[Khalid Aziz]

On 03/07/2016 02:34 PM, David Miller wrote:

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 14:27:09 -0700


I agree with your point of view. PSTATE.mcde and TTE.mcd are set in
response to request from userspace. If userspace asked for them to be
set, they already know but it was the database guys that asked for
these two functions and they are the primary customers for the ADI
feature. I am not crazy about this idea since this extends the
mprotect API even further but would you consider using the return
value from mprotect to indicate if PSTATE.mcde or TTE.mcd were already
set on the given address?


Well, that's the idea.

If the mprotect using MAP_ADI or whatever succeeds, then ADI is
enabled.

Users can thus also pass MAP_ADI as a flag to mmap() to get ADI
protection from the very beginning.



MAP_ADI has been sitting in my backlog for some time. Looks like you 
just raised its priority ;)


--
Khalid
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[Dave Hansen]

On 03/07/2016 11:46 AM, Khalid Aziz wrote:
> On 03/07/2016 12:22 PM, David Miller wrote:
>> Khalid, maybe you should share notes with the folks working on x86
>> protection keys.
> 
> Good idea. Sparc ADI feature is indeed similar to x86 protection keys
> sounds like.

There are definitely some similarities.  But protection keys doesn't
have any additional tables in which to keep metadata.  It keeps all of
its data in the page tables.  It also doesn't have an impact on the
virtual address layout.

But, it does have metadata to store in the VMA, has a special
siginfo->si_code, and it uses mprotect() (although a new pkey_mprotect()
variant that takes an extra argument).

Protection Keys are described a bit more here:

> http://git.kernel.org/cgit/linux/kernel/git/daveh/x86-pkeys.git/tree/Documentation/x86/protection-keys.txt?h=pkeys-v025&id=1b5b8a8836de8eb667027178b4820665dea5a038

MPX is another Intel feature separate from protection keys, but *it* has
some tables that it keep its metadata memory and special special
instructions to move metadata in and out of it.  It also has a prctl()
to enable/disable kernel assistance for the feature.  Unlike ADI, the
tables are exposed (and accessible) to user applications in normal
application memory.

MPX's documentation is here:

> http://git.kernel.org/cgit/linux/kernel/git/daveh/x86-pkeys.git/tree/Documentation/x86/intel_mpx.txt

Overall, I'm not seeing much overlap at all between the features, honestly.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[Rob Gardner]

On 03/07/2016 01:33 PM, Khalid Aziz wrote:


That is a possibility but limited in scope. An address range covered 
by a single TTE can have large number of tags. Version tags are set on 
cacheline. In extreme case, one could set a tag for each set of 
64-bytes in a page. Also tags are set completely in userspace and no 
transition occurs to kernel space, so kernel has no idea of what tags 
have been set.


  ...

I have not found a way to query the MMU on tags.



To query the tag for a cache line, you just read it back with ldxa and 
ASI_MCD_PRIMARY (ie, asi 0x90), basically the same way you stored the 
tag in the first place.


Rob



--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[Rob Gardner]

On 03/07/2016 01:38 PM, David Miller wrote:

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 14:33:56 -0700


On 03/07/2016 12:16 PM, David Miller wrote:

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 11:24:54 -0700


Tags can be cleared by user by setting tag to 0. Tags are
automatically cleared by the hardware when the mapping for a virtual
address is removed from TSB (which is why swappable pages are a
problem), so kernel does not have to do it as part of clean up.

You might be able to crib some bits for the Tag in the swp_entry_t,
it's
64-bit and you can therefore steal bits from the offset field.

That way you'll have the ADI tag in the page tables, ready to
re-install
at swapin time.


That is a possibility but limited in scope. An address range covered
by a single TTE can have large number of tags. Version tags are set on
cacheline. In extreme case, one could set a tag for each set of
64-bytes in a page. Also tags are set completely in userspace and no
transition occurs to kernel space, so kernel has no idea of what tags
have been set. I have not found a way to query the MMU on tags.

I will think some more about it.

That would mean that ADI is impossible to use for swappable memory.

...

If that's true I'm extremely disappointed that they devoted so much
silicon and engineering to this feature yet didn't take that one
critical step to make it generally useful. :(


You can easily read ADI tags with a simple ldxa #ASI_MCD_PRIMARY 
instruction.


Rob


--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[Khalid Aziz]

On 03/07/2016 04:12 PM, Rob Gardner wrote:

On 03/07/2016 01:33 PM, Khalid Aziz wrote:


That is a possibility but limited in scope. An address range covered
by a single TTE can have large number of tags. Version tags are set on
cacheline. In extreme case, one could set a tag for each set of
64-bytes in a page. Also tags are set completely in userspace and no
transition occurs to kernel space, so kernel has no idea of what tags
have been set.


   ...

I have not found a way to query the MMU on tags.



To query the tag for a cache line, you just read it back with ldxa and
ASI_MCD_PRIMARY (ie, asi 0x90), basically the same way you stored the
tag in the first place.



Thanks, Rob. I just saw it while reading through the manual.

--
Khalid

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[Rob Gardner]

On 03/07/2016 10:24 AM, Khalid Aziz wrote:


Tags can be cleared by user by setting tag to 0. Tags are 
automatically cleared by the hardware when the mapping for a virtual 
address is removed from TSB (which is why swappable pages are a 
problem), so kernel does not have to do it as part of clean up.




I don't understand this. The hardware isn't involved  when a mapping for 
a virtual address is removed from the TSB, so how could it automatically 
clear tags?


Rob

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[James Morris]

On 03/08/2016 07:58 AM, David Miller wrote:

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 13:41:39 -0700


Shared data may not always be backed by a file. My understanding is
one of the use cases is for in-memory databases. This shared space
could also be used to hand off transactions in flight to other
processes. These transactions in flight would not be backed by a
file. Some of these use cases might not use shmfs even. Setting ADI
bits at virtual address level catches all these cases since what backs
the tagged virtual address can be anything - a mapped file, mmio
space, just plain chunk of memory.


Frankly the most interesting use case to me is simply finding bugs
and memory scribbles, and for that we're want to be able to ADI
arbitrary memory returned from malloc() and friends.

I personally see ADI more as a debugging than a security feature,
but that's just my view.


This is certainly a major use of the feature. The Solaris folks have 
made some interesting use of it here:


https://docs.oracle.com/cd/E37069_01/html/E37085/gphwb.html

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[James Morris]

On 03/08/2016 06:54 AM, Andy Lutomirski wrote:


This makes sense, but I still think the design is poor.  If the hacker
gets code execution, then they can trivially brute force the ADI bits.



ADI in this scenario is intended to prevent the attacker from gaining 
code execution in the first place.




--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[Khalid Aziz]

On 03/07/2016 12:16 PM, David Miller wrote:

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 11:24:54 -0700


Tags can be cleared by user by setting tag to 0. Tags are
automatically cleared by the hardware when the mapping for a virtual
address is removed from TSB (which is why swappable pages are a
problem), so kernel does not have to do it as part of clean up.


You might be able to crib some bits for the Tag in the swp_entry_t, it's
64-bit and you can therefore steal bits from the offset field.

That way you'll have the ADI tag in the page tables, ready to re-install
at swapin time.



Hi Dave,

Can we enable ADI support for swappable pages in a subsequent update 
after the core functionality is stable on mlock'd pages?


Thanks,
Khalid
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 0/10] watchdog: Add support for keepalives triggered by infrastructure

[Guenter Roeck]

On Mon, Mar 07, 2016 at 07:50:23PM +0100, Wim Van Sebroeck wrote:
> Hi Guenter,
> 
> > Hi Wim,
> > 
> > On Sun, Mar 06, 2016 at 11:49:56AM +0100, Wim Van Sebroeck wrote:
> > > Hi Guenter,
> > > 
> > > > The watchdog infrastructure is currently purely passive, meaning
> > > > it only passes information from user space to drivers and vice versa.
> > > > 
> > [ ... ]
> > > 
> > > Patches 1 till 7 of this series has been added to linux-watchdog-next.
> > > 
> > Thanks a lot!
> 
> Did you receive any feedback/test-results on patches 8 till 10 ?
> 
No, unfortunately not. I think I'll ping some of those involved in the driver
development, but I'll do it after the -rc is out. No need to rush with those
now.

Thanks,
Guenter
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[Rob Gardner]

On 03/07/2016 10:39 AM, Khalid Aziz wrote:

On 03/07/2016 11:12 AM, Dave Hansen wrote:

On 03/07/2016 09:53 AM, Andy Lutomirski wrote:

Also, what am I missing?  Tying these tags to the physical page seems
like a poor design to me.  This seems really awkward to use.


Yeah, can you describe the structures that store these things? Surely
the hardware has some kind of lookup tables for them and stores them in
memory _somewhere_.



Version tags are tied to virtual addresses, not physical pages.

Where exactly are the tags stored is part of processor architecture 
and I am not privy to that. MMU stores these lookup tables somewhere 
and uses it to authenticate access to virtual addresses. It really is 
irrelevant to kernel how MMU implements access controls as long as we 
have access to the knowledge of how to use it.


The tags are stored in physical memory, and you can write a tag directly 
to that memory via stxa with ASI_MCD_REAL and completely bypass the MMU. 
When you do that, the tag will still be seen by any virtual address that 
maps to that physical address.


Rob



--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] modsign: Fix documentation on module signing enforcement parameter.

[James Johnston]

David Howells  wrote:

> James Johnston  wrote:
> 
> > -If CONFIG_MODULE_SIG_FORCE is enabled or enforcemodulesig=1 is
> > supplied on
> > +If CONFIG_MODULE_SIG_FORCE is enabled or module.sig_enforce=1 is
> > +supplied
> 
> You're definitely right about the change from enforcemodulesig to
> sig_enforce, but how does the "module." come about?

It was reviewed by you 3.5 years ago. :)

https://github.com/torvalds/linux/commit/106a4ee258d14818467829bf0e12aeae14c
16cd7#diff-04da0379417c7dc6cf47548c26e69236

In Documentation/kernel-parameters.txt:

module.sig_enforce
[KNL] When CONFIG_MODULE_SIG is set, this means that
modules without (valid) signatures will fail to
load.
Note that if CONFIG_MODULE_SIG_FORCE is set, that
is always true, so this option does nothing.

The top of kernel-parameters.txt is also explicit that module parameters
must be prefixed with a module name prefix, in this case "module."  It
sounds weird that the code loading modules is itself a module, but I guess
that's how it works from a parameters standpoint?  (The code in
kernel/module.c certainly makes use of the normal module parameter macros; I
guess that's how "module." prefix comes about.)

In practice, I'm running an Ubuntu 4.2.0-30-generic kernel and this
parameter can be seen in /sys/module and is controlled by the kernel
cmdline:

$ cat /sys/module/module/parameters/sig_enforce
Y
$ cat /proc/cmdline
 module.sig_enforce=1
$ grep CONFIG_MODULE_SIG_FORCE /boot/config-4.2.0-30-generic
# CONFIG_MODULE_SIG_FORCE is not set

And I tested to verify that some unsigned drivers I had did not load.  If I
reboot and change my cmdline to just "sig_enforce=1" then the above
sig_enforce /sys parameters file reverts to "N", indicating that the
"module." prefix is required.

James


--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[David Miller]

From: Rob Gardner 
Date: Mon, 7 Mar 2016 15:13:31 -0800

> You can easily read ADI tags with a simple ldxa #ASI_MCD_PRIMARY
> instruction.

Awesome!
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] sparc64: Add support for Application Data Integrity (ADI)

[David Miller]

From: Khalid Aziz 
Date: Mon, 7 Mar 2016 17:21:05 -0700

> Can we enable ADI support for swappable pages in a subsequent update
> after the core functionality is stable on mlock'd pages?

I already said no.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Kernel docs: muddying the waters a bit

[Russel Winder]

On Fri, 2016-03-04 at 09:46 +0200, Jani Nikula wrote:
> […]
> If we're talking about the same asciidoctor (http://asciidoctor.org/)
> it's written in ruby but you can apparently run it in JVM using
> JRuby. Calling it Java-based is misleading.

Indeed, I was somewhat imprecise. Thanks to the work mostly of Charles
Nutter, JRuby is invariably a faster platform for Ruby code than Ruby
is. So yes ASCIIDoctor is JVM-based via JRuby, not Java-based.

The real point here is that in a move from DocBook/XML as a
documentation source, ASCIIDoctor is an excellent choice.

-- 
Russel.=Dr
 Russel Winder  t: +44 20 7585 2200   voip: sip:russel.winder@ekiga.net41 
Buckmaster Roadm: +44 7770 465 077   xmpp: rus...@winder.org.ukLondon SW11 
1EN, UK   w: www.russel.org.uk  skype: russel_winder


signature.asc
Description: This is a digitally signed message part

[RESEND PATCH V4 0/4] Introduce CoreSight STM support

[Chunyan Zhang]

This patchset adds support for the CoreSight STM IP block.

In the fourth version, comments from various people have been
addressed.  Representing configurations where channels are shared
between multiple masterIDs has been kept unchanged from the previous
version because a viable alternative hasn't been suggested.

This RESEND PATCH 1/4 depends on the patch [3] which has been
merged into linux-next.

Changes from V3:
 - Removed ioctl get_options interface from the generic STM code and CoreSight 
STM driver.
 - Removed 'write_max' from the structure 'stm_drvdata', and changed 
'write_64bit' to 'write_bytes'.
 - Revised stm_fundamental_data_size() to return the fundamental data size 
instead of 0/1.
 - Removed stm_remove() from the driver.
 - Revised the return value of ::packet() callback function according to [2].
 - Modified stm_send() to send one STP packet at a time.
 - Added comments to invariant/guaranteed CoreSight STM transaction mode.

Changes from V2:
 - Changed to return -EFAULT if failed on the command STP_GET_OPTIONS.
 - Used Alex's patch [1] instead of the last 2/6.
 - Removed the while loop from stm_send(), since the packet size passed
   to it isn't larger than 4 bytes on 32-bit system and 8 bytes on
   64-bit system.
 - Removed stm_send_64bit(), since the process of packets on 64-bit
   CS-STM should be basically the same with on 32-bit system, except the 
   maximum length of writing STM at a time.
 - Removed the support of writing 64-bit to CoreSight STM buffer at a time
   on 32-bit ARM architecture according to an ARM engineer suggestion.  As
   he said that the STM might receive a 64-bit write, or might receive a
   pair of 32-bit writes to the two addressed words in either order.
   So 64-bit write isn't guaranteed to work on the ARM 32-bit architecture.

Changes from v1:
 - Added a definition of coresight_simple_func() in CS-STM driver to
   avoid the kbuild test robot error for the time being.  This
   modification will be removed when merging the code in which the
   coresight_simple_func() has been moved to the header file.
 - Calculate the channel number according to the channel memory space size.


Thanks,
Chunyan

[1] https://lkml.org/lkml/2016/2/4/652
[2] https://lkml.org/lkml/2016/2/12/397
[3] https://lkml.org/lkml/2015/12/22/348

Chunyan Zhang (1):
  Documentations: Add explanations of the case for non-configurable
masters

Mathieu Poirier (2):
  stm class: provision for statically assigned masterIDs
  coresight-stm: Bindings for System Trace Macrocell

Pratik Patel (1):
  coresight-stm: adding driver for CoreSight STM component

 .../ABI/testing/sysfs-bus-coresight-devices-stm|  53 ++
 .../devicetree/bindings/arm/coresight.txt  |  28 +
 Documentation/trace/coresight.txt  |  37 +-
 Documentation/trace/stm.txt|   6 +
 drivers/hwtracing/coresight/Kconfig|  11 +
 drivers/hwtracing/coresight/Makefile   |   1 +
 drivers/hwtracing/coresight/coresight-stm.c| 890 +
 drivers/hwtracing/stm/core.c   |  17 +-
 drivers/hwtracing/stm/policy.c |  18 +-
 include/linux/coresight-stm.h  |   6 +
 include/linux/stm.h|   8 +
 include/uapi/linux/coresight-stm.h |  21 +
 12 files changed, 1090 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-coresight-devices-stm
 create mode 100644 drivers/hwtracing/coresight/coresight-stm.c
 create mode 100644 include/linux/coresight-stm.h
 create mode 100644 include/uapi/linux/coresight-stm.h

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RESEND PATCH V4 1/4] stm class: provision for statically assigned masterIDs

[Chunyan Zhang]

From: Mathieu Poirier 

Some architecture like ARM assign masterIDs at the HW design
phase.  Those are therefore unreachable to users, making masterID
management in the generic STM core irrelevant.

In this kind of configuration channels are shared between masters
rather than being allocated on a per master basis.

This patch adds a new 'mshared' flag to struct stm_data that tells the
core that this specific STM device doesn't need explicit masterID
management.  In the core sw_start/end of masterID are set to '1',
i.e there is only one masterID to deal with.

Also this patch depends on [1], so that the number of masterID
is '1' too.

Finally the lower and upper bound for masterIDs as presented
in ($SYSFS)/class/stm/XYZ.stm/masters and
($SYSFS)/../stp-policy/XYZ.stm.my_policy/some_device/masters
are set to '-1'.  That way users can't confuse them with
architecture where masterID management is required (where any
other value would be valid).

[1] https://lkml.org/lkml/2015/12/22/348

Signed-off-by: Mathieu Poirier 
Signed-off-by: Chunyan Zhang 
---
 drivers/hwtracing/stm/core.c   | 17 +++--
 drivers/hwtracing/stm/policy.c | 18 --
 include/linux/stm.h|  8 
 3 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c
index 66cec56..0d7f1c5 100644
--- a/drivers/hwtracing/stm/core.c
+++ b/drivers/hwtracing/stm/core.c
@@ -44,9 +44,15 @@ static ssize_t masters_show(struct device *dev,
char *buf)
 {
struct stm_device *stm = to_stm_device(dev);
-   int ret;
+   int ret, sw_start, sw_end;
+
+   sw_start = stm->data->sw_start;
+   sw_end = stm->data->sw_end;
+
+   if (stm->data->mshared)
+   sw_start = sw_end = STM_SHARED_MASTERID;
 
-   ret = sprintf(buf, "%u %u\n", stm->data->sw_start, stm->data->sw_end);
+   ret = sprintf(buf, "%d %d\n", sw_start, sw_end);
 
return ret;
 }
@@ -618,6 +624,13 @@ int stm_register_device(struct device *parent, struct 
stm_data *stm_data,
if (!stm_data->packet || !stm_data->sw_nchannels)
return -EINVAL;
 
+   /*
+* MasterIDs are assigned at HW design phase. As such the core is
+* using a single master for interaction with this device.
+*/
+   if (stm_data->mshared)
+   stm_data->sw_start = stm_data->sw_end = 1;
+
nmasters = stm_data->sw_end - stm_data->sw_start + 1;
stm = kzalloc(sizeof(*stm) + nmasters * sizeof(void *), GFP_KERNEL);
if (!stm)
diff --git a/drivers/hwtracing/stm/policy.c b/drivers/hwtracing/stm/policy.c
index 17a1416..19455db 100644
--- a/drivers/hwtracing/stm/policy.c
+++ b/drivers/hwtracing/stm/policy.c
@@ -80,10 +80,17 @@ static ssize_t
 stp_policy_node_masters_show(struct config_item *item, char *page)
 {
struct stp_policy_node *policy_node = to_stp_policy_node(item);
+   struct stm_device *stm = policy_node->policy->stm;
+   int first_master, last_master;
ssize_t count;
 
-   count = sprintf(page, "%u %u\n", policy_node->first_master,
-   policy_node->last_master);
+   first_master = policy_node->first_master;
+   last_master = policy_node->last_master;
+
+   if (stm && stm->data->mshared)
+   first_master = last_master = STM_SHARED_MASTERID;
+
+   count = sprintf(page, "%d %d\n", first_master, last_master);
 
return count;
 }
@@ -106,6 +113,13 @@ stp_policy_node_masters_store(struct config_item *item, 
const char *page,
if (!stm)
goto unlock;
 
+   /*
+* masterIDs are allocated in HW, no point in trying to
+* change their values.
+*/
+   if (stm->data->mshared)
+   goto unlock;
+
/* must be within [sw_start..sw_end], which is an inclusive range */
if (first > INT_MAX || last > INT_MAX || first > last ||
first < stm->data->sw_start ||
diff --git a/include/linux/stm.h b/include/linux/stm.h
index 9d0083d..6fac8b1 100644
--- a/include/linux/stm.h
+++ b/include/linux/stm.h
@@ -18,6 +18,11 @@
 #include 
 
 /**
+ * The masterIDs are set in hardware and can't be queried
+ */
+#define STM_SHARED_MASTERID -1
+
+/**
  * enum stp_packet_type - STP packets that an STM driver sends
  */
 enum stp_packet_type {
@@ -46,6 +51,8 @@ struct stm_device;
  * struct stm_data - STM device description and callbacks
  * @name:  device name
  * @stm:   internal structure, only used by stm class code
+ * @mshared:   true if masterIDs are assigned in HW.  If so @sw_start
+ * and @sw_end are set to '1' by the core.
  * @sw_start:  first STP master available to software
  * @sw_end:last STP master available to software
  * @sw_nchannels:  number of STP channels per master
@@ -71,6 +78,7 @@ struct stm_device;
 struct stm_data {
const char  *na

[RESEND PATCH V4 2/4] Documentations: Add explanations of the case for non-configurable masters

[Chunyan Zhang]

For some STM hardware (e.g. ARM CoreSight STM), the masterID associated
to a source is set at the hardware level and not user configurable.
Since the masterID information isn't available to SW, introducing
a new value of -1 to reflect this reality.

Signed-off-by: Chunyan Zhang 
---
 Documentation/trace/stm.txt | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/trace/stm.txt b/Documentation/trace/stm.txt
index ea035f9..f03bc2b 100644
--- a/Documentation/trace/stm.txt
+++ b/Documentation/trace/stm.txt
@@ -47,6 +47,12 @@ through 127 in it. Now, any producer (trace source) 
identifying itself
 with "user" identification string will be allocated a master and
 channel from within these ranges.
 
+$ cat /config/stp-policy/dummy_stm.my-policy/user/masters
+-1 -1
+
+Would indicate the masters for this rule are set in hardware and
+not configurable in user space.
+
 These rules can be nested, for example, one can define a rule "dummy"
 under "user" directory from the example above and this new rule will
 be used for trace sources with the id string of "user/dummy".
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RESEND PATCH V4 3/4] coresight-stm: Bindings for System Trace Macrocell

[Chunyan Zhang]

From: Mathieu Poirier 

The System Trace Macrocell (STM) is an IP block falling under the
CoreSight umbrella.  It's main purpose it so expose stimulus channels
to any system component for the purpose of information logging.

Bindings for this IP block adds a couple of items to the current
mandatory definition for CoreSight components.

Signed-off-by: Mathieu Poirier 
Acked-by: Rob Herring 
Signed-off-by: Chunyan Zhang 
---
 .../devicetree/bindings/arm/coresight.txt  | 28 ++
 1 file changed, 28 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/coresight.txt 
b/Documentation/devicetree/bindings/arm/coresight.txt
index 62938eb..93147c0c 100644
--- a/Documentation/devicetree/bindings/arm/coresight.txt
+++ b/Documentation/devicetree/bindings/arm/coresight.txt
@@ -19,6 +19,7 @@ its hardware characteristcs.
- "arm,coresight-etm3x", "arm,primecell";
- "arm,coresight-etm4x", "arm,primecell";
- "qcom,coresight-replicator1x", "arm,primecell";
+   - "arm,coresight-stm", "arm,primecell"; [1]
 
* reg: physical base address and length of the register
  set(s) of the component.
@@ -36,6 +37,14 @@ its hardware characteristcs.
  layout using the generic DT graph presentation found in
  "bindings/graph.txt".
 
+* Additional required properties for System Trace Macrocells (STM):
+   * reg: along with the physical base address and length of the register
+ set as described above, another entry is required to describe the
+ mapping of the extended stimulus port area.
+
+   * reg-names: the only acceptable values are "stm-base" and
+ "stm-stimulus-base", each corresponding to the areas defined in "reg".
+
 * Required properties for devices that don't show up on the AMBA bus, such as
   non-configurable replicators:
 
@@ -202,3 +211,22 @@ Example:
};
};
};
+
+4. STM
+   stm@2010 {
+   compatible = "arm,coresight-stm", "arm,primecell";
+   reg = <0 0x2010 0 0x1000>,
+ <0 0x2800 0 0x18>;
+   reg-names = "stm-base", "stm-stimulus-base";
+
+   clocks = <&soc_smc50mhz>;
+   clock-names = "apb_pclk";
+   port {
+   stm_out_port: endpoint {
+   remote-endpoint = <&main_funnel_in_port2>;
+   };
+   };
+   };
+
+[1]. There is currently two version of STM: STM32 and STM500.  Both
+have the same HW interface and as such don't need an explicit binding name.
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RESEND PATCH V4 4/4] coresight-stm: adding driver for CoreSight STM component

[Chunyan Zhang]

From: Pratik Patel 

This driver adds support for the STM CoreSight IP block, allowing any
system compoment (HW or SW) to log and aggregate messages via a
single entity.

The CoreSight STM exposes an application defined number of channels
called stimulus port.  Configuration is done using entries in sysfs
and channels made available to userspace via configfs.

Signed-off-by: Pratik Patel 
Signed-off-by: Mathieu Poirier 
Signed-off-by: Chunyan Zhang 
---
 .../ABI/testing/sysfs-bus-coresight-devices-stm|  53 ++
 Documentation/trace/coresight.txt  |  37 +-
 drivers/hwtracing/coresight/Kconfig|  11 +
 drivers/hwtracing/coresight/Makefile   |   1 +
 drivers/hwtracing/coresight/coresight-stm.c| 890 +
 include/linux/coresight-stm.h  |   6 +
 include/uapi/linux/coresight-stm.h |  21 +
 7 files changed, 1017 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-coresight-devices-stm
 create mode 100644 drivers/hwtracing/coresight/coresight-stm.c
 create mode 100644 include/linux/coresight-stm.h
 create mode 100644 include/uapi/linux/coresight-stm.h

diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-stm 
b/Documentation/ABI/testing/sysfs-bus-coresight-devices-stm
new file mode 100644
index 000..c519056
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-stm
@@ -0,0 +1,53 @@
+What:  /sys/bus/coresight/devices/.stm/enable_source
+Date:  February 2016
+KernelVersion: 4.5
+Contact:   Mathieu Poirier 
+Description:   (RW) Enable/disable tracing on this specific trace macrocell.
+   Enabling the trace macrocell implies it has been configured
+   properly and a sink has been identified for it.  The path
+   of coresight components linking the source to the sink is
+   configured and managed automatically by the coresight framework.
+
+What:  /sys/bus/coresight/devices/.stm/hwevent_enable
+Date:  February 2016
+KernelVersion: 4.5
+Contact:   Mathieu Poirier 
+Description:   (RW) Provides access to the HW event enable register, used in
+   conjunction with HW event bank select register.
+
+What:  /sys/bus/coresight/devices/.stm/hwevent_select
+Date:  February 2016
+KernelVersion: 4.5
+Contact:   Mathieu Poirier 
+Description:   (RW) Gives access to the HW event block select register
+   (STMHEBSR) in order to configure up to 256 channels.  Used in
+   conjunction with "hwevent_enable" register as described above.
+
+What:  /sys/bus/coresight/devices/.stm/port_enable
+Date:  February 2016
+KernelVersion: 4.5
+Contact:   Mathieu Poirier 
+Description:   (RW) Provides access to the stimulus port enable register
+   (STMSPER).  Used in conjunction with "port_select" described
+   below.
+
+What:  /sys/bus/coresight/devices/.stm/port_select
+Date:  February 2016
+KernelVersion: 4.5
+Contact:   Mathieu Poirier 
+Description:   (RW) Used to determine which bank of stimulus port bit in
+   register STMSPER (see above) apply to.
+
+What:  /sys/bus/coresight/devices/.stm/status
+Date:  February 2016
+KernelVersion: 4.5
+Contact:   Mathieu Poirier 
+Description:   (R) List various control and status registers.  The specific
+   layout and content is driver specific.
+
+What:  /sys/bus/coresight/devices/.stm/traceid
+Date:  February 2016
+KernelVersion: 4.5
+Contact:   Mathieu Poirier 
+Description:   (RW) Holds the trace ID that will appear in the trace stream
+   coming from this trace entity.
diff --git a/Documentation/trace/coresight.txt 
b/Documentation/trace/coresight.txt
index 0a5c329..a33c88c 100644
--- a/Documentation/trace/coresight.txt
+++ b/Documentation/trace/coresight.txt
@@ -190,8 +190,8 @@ expected to be accessed and controlled using those entries.
 Last but not least, "struct module *owner" is expected to be set to reflect
 the information carried in "THIS_MODULE".
 
-How to use
---
+How to use the tracer modules
+-
 
 Before trace collection can start, a coresight sink needs to be identify.
 There is no limit on the amount of sinks (nor sources) that can be enabled at
@@ -297,3 +297,36 @@ InfoTracing enabled
 Instruction 135708310x8026B584  E28DD00Cfalse   ADD
  sp,sp,#0xc
 Instruction 0   0x8026B588  E8BD8000trueLDM  
sp!,{pc}
 Timestamp   Timestamp: 17107041535
+
+How to use the STM module
+-
+
+Using the System Trace Macrocell module is the same as the tracers - the only
+difference is that clients are driving the trace capture rather
+than the program flow through the 

Re: [PATCH] fat: add config option to set UTF-8 mount option by default

[OGAWA Hirofumi]

"Maciej S. Szmigiero"  writes:

> +#ifdef CONFIG_FAT_DEFAULT_UTF8
> + opts->utf8 = is_vfat;
> +#else
> + opts->utf8 = 0;
> +#endif
> +

Maybe, better to use IS_ENABLED(CONFIG_FAT_DEFAULT_UTF8)?

I.e.,

opts->utf8 = IS_ENABLED(CONFIG_FAT_DEFAULT_UTF8) && is_vfat;

Thanks.
-- 
OGAWA Hirofumi 
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] can: rcar_canfd: Add Renesas R-Car CAN FD driver

[Oliver Hartkopp]

On 03/07/2016 09:32 AM, Ramesh Shanmugasundaram wrote:

> + /* Ensure channel starts in FD mode */
> + if (!(priv->can.ctrlmode & CAN_CTRLMODE_FD)) {
> + netdev_err(ndev, "enable can fd mode for channel %d\n", ch);
> + goto fail_mode;
> + }

 What's the reason behind this check?

 A CAN FD capable CAN controller can be either configured to run as
 CAN 2.0 (Classic CAN) or as CAN FD controller.

 So why are to throwing an error here and produce an initialization
 failure?
>>>
>>> When this controller is configured in FD mode and used only with CAN
>>> 2.0 nodes, it still expects a DTSEG (data bitrate) configuration same
>>> as NTSEG (nominal bitrate). This check, specifically in ndo_open,
>>> ensures both are configured and will work fine with CAN 2.0 nodes
>>> (e.g.)
>>>
>>> "ip link set can0 up type can bitrate 100 dbitrate 100 fd on"
>>>
>>> If I don't have this check, a configuration like this
>>>
>>> "ip link set can0 up type can bitrate 100"
>>>
>>> will bring up the controller without DTSEG configured.

What about spending some status flag or setting the data bitrate equal to the
nominal bitrate unless a data bitrate is provided?

>>
>> That should bring up the controller in CAN 2.0 mode.
> 
> Yes, that's the user's intention but the manual states DTSEG still need to be 
> configured. In the above configuration, it will not be.
> Besides, this will not be a "pure" CAN 2.0 node (i.e.) if a frame with length 
> > 8 bytes is received the controller will "ACK" because in FD mode it can 
> receive up to 64 bytes frame.

Oh. We probably mix something up here (CAN frame formats & bitrates).

A CAN2.0 frame and a CAN FD frame have very different representations on the
wire! So if you see a FDF (former EDL) bit this is a CAN FD frame, which
requires two bitrates (nominal/data bitrate) where the data bitrate has to be
greater or equal the nominal bitrate.

The fact that the data bitrate is equal the nominal/arbitration bitrate has
nothing to do with CAN2.0 then. Regarding your answer this is not even "a pure
CAN2.0" node - it still looks like a CAN FD node with equal data/nominal 
bitrates.

The fact that a CAN FD frame has a size of 8 bytes doesn't make it a CAN2.0
frame :-)

> 
> The controller does support a "pure" classical CAN mode with a different set 
> of register map itself.

Is this a can_rcar controller register mapping then?

> Do you think a pure CAN 2.0 mode support would be beneficial? I can submit 
> this in coming days on top of current submission.
> 
> The current submission status is:
>  - Controller operates in CAN FD mode only.
>  - If needed to interoperate with CAN 2.0 nodes, data bitrate still need to 
> be configured and it will work perfectly. However, it is not a "pure" CAN 2.0 
> node as mentioned above.

When you have a CAN FD /capable/ controller the idea is:

"ip link set can0 up type can bitrate 100"

The controller is in CAN2.0 mode:

1. It can send and receive CAN2.0 frames @1MBit/s.
2. The MTU is set to 16 (sizeof(struct can_frame)) ; CAN_CTRLMODE_FD is unset.
3. The CAN controller is not CAN FD tolerant (will produce error frames)

"ip link set can0 up type can bitrate 100 dbitrate 100 fd on"

1. It can send and receive CAN2.0 frames @1MBit/s.
2. It can send and receive CAN FD frames @1MBit/s (arbitration&data bitrate).
3. The MTU is set to 72 (sizeof(struct canfd_frame)) ; CAN_CTRLMODE_FD is set.

For CAN FD frames the data bitrate can be increased like:
"ip link set can0 up type can bitrate 100 dbitrate 400 fd on"

So when CAN_CTRLMODE_FD is unset the controller should act like a "pure
CAN2.0" node. When people configure a CAN FD controller with "fd on" and use
CAN2.0 frames all the time this is ok either - but the controller is able to
process CAN FD frames with the correct bitrate too.

Regards,
Oliver

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html