All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 06/14] irqchip: gicv3-its: platform-msi: refactor its_pmsi_init() to prepare for ACPI
From: Tomasz Nowicki @ 2017-01-04  7:29 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <601cbdf2-823d-8bde-bbd9-fcc6a1c67f2c@linaro.org>

On 04.01.2017 08:02, Hanjun Guo wrote:
> Hi Tomasz,
>
> On 2017/1/3 15:41, Tomasz Nowicki wrote:
>> Hi,
>>
>> Can we merge patch 4 & 6 into one patch so that we keep refactoring part
>> as one piece ? I do not see a reason to keep them separate or have patch
>> 5 in between. You can refactor what needs to be refactored, add
>> necessary functions to iort.c and then support ACPI for
>> irq-gic-v3-its-platform-msi.c
>
> There are two functions here,
>  - retrieve the dev id from IORT which was DT based only;
>
>  - init the platform msi domain from MADT;
>
> For each of them split it into two steps,
>  - refactor the code for ACPI later and it's easy for review
>    because wen can easily to figure out it has functional
>    change or not
>
>  - add ACPI functionality
>
> Does it make sense?

It is up to Marc, but personally I prefer:
1. Refactor dev id retrieving and init function in one patch and 
highlight no functional changes in changelog
2. Crate necessary infrastructure in iort.c
3. Then add ACPI support to irq-gic-v3-its-platform-msi.c

Thanks,
Tomasz

^ permalink raw reply

* Re: [PATCH 2/7] mm, vmscan: add active list aging tracepoint
From: Vlastimil Babka @ 2017-01-04  7:28 UTC (permalink / raw)
  To: Minchan Kim, Michal Hocko
  Cc: Hillf Danton, linux-mm, Andrew Morton, Mel Gorman,
	Johannes Weiner, Rik van Riel, LKML
In-Reply-To: <20170104050722.GA17166@bbox>

On 01/04/2017 06:07 AM, Minchan Kim wrote:
> With this,
> ./scripts/bloat-o-meter vmlinux.old vmlinux.new.new
> add/remove: 1/1 grow/shrink: 0/9 up/down: 1394/-1636 (-242)
> function                                     old     new   delta
> isolate_lru_pages                              -    1394   +1394
> print_fmt_mm_vmscan_lru_shrink_inactive      359     355      -4
> vermagic                                      64      58      -6
> perf_trace_mm_vmscan_lru_shrink_active       264     256      -8
> trace_raw_output_mm_vmscan_lru_shrink_active     203     193     -10
> trace_event_raw_event_mm_vmscan_lru_shrink_active     241     225     -16
> print_fmt_mm_vmscan_lru_shrink_active        458     426     -32
> trace_event_define_fields_mm_vmscan_lru_shrink_active     384     336     -48
> shrink_inactive_list                        1430    1271    -159
> shrink_active_list                          1265    1082    -183
> isolate_lru_pages.isra                      1170       -   -1170
> Total: Before=26268743, After=26268501, chg -0.00%
> 
> We can save 242 bytes.
> 
> If we consider binary size, 424 bytes save.
> 
> #> ls -l vmlinux.old vmlinux.new.new
> 194092840  vmlinux.old
> 194092416  vmlinux.new.new

Which is roughly 0.0002%. Not that I'm against fighting bloat, but let's
not forget that it's not the only factor. For example the following part
from above:

> isolate_lru_pages                              -    1394   +1394
> isolate_lru_pages.isra                      1170       -   -1170

shows that your change has prevented a -fipa-src gcc optimisation, which
is "interprocedural scalar replacement of aggregates, removal of unused
parameters and replacement of parameters passed by reference by
parameters passed by value." Well, I'm no gcc expert :) but it might be
that the change is not a simple win-win.

^ permalink raw reply

* Re: [PATCH 2/7] mm, vmscan: add active list aging tracepoint
From: Vlastimil Babka @ 2017-01-04  7:28 UTC (permalink / raw)
  To: Minchan Kim, Michal Hocko
  Cc: Hillf Danton, linux-mm, Andrew Morton, Mel Gorman,
	Johannes Weiner, Rik van Riel, LKML
In-Reply-To: <20170104050722.GA17166@bbox>

On 01/04/2017 06:07 AM, Minchan Kim wrote:
> With this,
> ./scripts/bloat-o-meter vmlinux.old vmlinux.new.new
> add/remove: 1/1 grow/shrink: 0/9 up/down: 1394/-1636 (-242)
> function                                     old     new   delta
> isolate_lru_pages                              -    1394   +1394
> print_fmt_mm_vmscan_lru_shrink_inactive      359     355      -4
> vermagic                                      64      58      -6
> perf_trace_mm_vmscan_lru_shrink_active       264     256      -8
> trace_raw_output_mm_vmscan_lru_shrink_active     203     193     -10
> trace_event_raw_event_mm_vmscan_lru_shrink_active     241     225     -16
> print_fmt_mm_vmscan_lru_shrink_active        458     426     -32
> trace_event_define_fields_mm_vmscan_lru_shrink_active     384     336     -48
> shrink_inactive_list                        1430    1271    -159
> shrink_active_list                          1265    1082    -183
> isolate_lru_pages.isra                      1170       -   -1170
> Total: Before=26268743, After=26268501, chg -0.00%
> 
> We can save 242 bytes.
> 
> If we consider binary size, 424 bytes save.
> 
> #> ls -l vmlinux.old vmlinux.new.new
> 194092840  vmlinux.old
> 194092416  vmlinux.new.new

Which is roughly 0.0002%. Not that I'm against fighting bloat, but let's
not forget that it's not the only factor. For example the following part
from above:

> isolate_lru_pages                              -    1394   +1394
> isolate_lru_pages.isra                      1170       -   -1170

shows that your change has prevented a -fipa-src gcc optimisation, which
is "interprocedural scalar replacement of aggregates, removal of unused
parameters and replacement of parameters passed by reference by
parameters passed by value." Well, I'm no gcc expert :) but it might be
that the change is not a simple win-win.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* [PATCH v5 8/8] doc: add MACsec offload into NIC feature list
From: Tiwei Bie @ 2017-01-04  7:21 UTC (permalink / raw)
  To: dev
  Cc: adrien.mazarguil, wenzhuo.lu, john.mcnamara, olivier.matz,
	thomas.monjalon, konstantin.ananyev, helin.zhang, wei.dai,
	xiao.w.wang
In-Reply-To: <1483514502-32841-1-git-send-email-tiwei.bie@intel.com>

Add MACsec offload into NIC feature list. And also update the
feature list for the supported drivers.

Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
---
 doc/guides/nics/features/default.ini | 1 +
 doc/guides/nics/features/ixgbe.ini   | 1 +
 2 files changed, 2 insertions(+)

diff --git a/doc/guides/nics/features/default.ini b/doc/guides/nics/features/default.ini
index f1bf9bf..c412d6f 100644
--- a/doc/guides/nics/features/default.ini
+++ b/doc/guides/nics/features/default.ini
@@ -43,6 +43,7 @@ VLAN offload         =
 QinQ offload         =
 L3 checksum offload  =
 L4 checksum offload  =
+MACsec offload       =
 Inner L3 checksum    =
 Inner L4 checksum    =
 Packet type parsing  =
diff --git a/doc/guides/nics/features/ixgbe.ini b/doc/guides/nics/features/ixgbe.ini
index 4a5667f..24c02fb 100644
--- a/doc/guides/nics/features/ixgbe.ini
+++ b/doc/guides/nics/features/ixgbe.ini
@@ -36,6 +36,7 @@ VLAN offload         = Y
 QinQ offload         = Y
 L3 checksum offload  = Y
 L4 checksum offload  = Y
+MACsec offload       = Y
 Inner L3 checksum    = Y
 Inner L4 checksum    = Y
 Packet type parsing  = Y
-- 
2.7.4

^ permalink raw reply related

* [PATCH v5 7/8] doc: update the release notes for the reserved flags
From: Tiwei Bie @ 2017-01-04  7:21 UTC (permalink / raw)
  To: dev
  Cc: adrien.mazarguil, wenzhuo.lu, john.mcnamara, olivier.matz,
	thomas.monjalon, konstantin.ananyev, helin.zhang, wei.dai,
	xiao.w.wang
In-Reply-To: <1483514502-32841-1-git-send-email-tiwei.bie@intel.com>

Add information about the flags reserved for PMD-specific API in
the release notes.

Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
 doc/guides/rel_notes/release_17_02.rst | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/doc/guides/rel_notes/release_17_02.rst b/doc/guides/rel_notes/release_17_02.rst
index 34dba45..af71f39 100644
--- a/doc/guides/rel_notes/release_17_02.rst
+++ b/doc/guides/rel_notes/release_17_02.rst
@@ -58,6 +58,18 @@ New Features
   MACsec offload support. The declarations of the APIs and the definitions
   of the flags can be found in ``rte_pmd_ixgbe.h``.
 
+* **Reserved flags for PMD to implement the PMD-specific API.**
+
+  Following flags in mbuf and ethdev are reserved for PMD to define its
+  own flags when implementing the PMD-specific API.
+
+  * DEV_RX_OFFLOAD_RESERVED_0
+  * DEV_TX_OFFLOAD_RESERVED_0
+  * RTE_ETH_EVENT_RESERVED_0
+  * PKT_TX_RESERVED_0
+
+  You can refer to ``rte_pmd_ixgbe.h`` for an example.
+
 
 Resolved Issues
 ---------------
-- 
2.7.4

^ permalink raw reply related

* [PATCH v5 6/8] doc: add ixgbe specific APIs
From: Tiwei Bie @ 2017-01-04  7:21 UTC (permalink / raw)
  To: dev
  Cc: adrien.mazarguil, wenzhuo.lu, john.mcnamara, olivier.matz,
	thomas.monjalon, konstantin.ananyev, helin.zhang, wei.dai,
	xiao.w.wang
In-Reply-To: <1483514502-32841-1-git-send-email-tiwei.bie@intel.com>

Add information about the new ixgbe PMD APIs in the release notes.

Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
 doc/guides/rel_notes/release_17_02.rst | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/doc/guides/rel_notes/release_17_02.rst b/doc/guides/rel_notes/release_17_02.rst
index 180af82..34dba45 100644
--- a/doc/guides/rel_notes/release_17_02.rst
+++ b/doc/guides/rel_notes/release_17_02.rst
@@ -52,6 +52,12 @@ New Features
   See the :ref:`Generic flow API <Generic_flow_API>` documentation for more
   information.
 
+* **Added APIs for MACsec offload support to the ixgbe PMD.**
+
+  Six new APIs and some related flags have been added to the ixgbe PMD for
+  MACsec offload support. The declarations of the APIs and the definitions
+  of the flags can be found in ``rte_pmd_ixgbe.h``.
+
 
 Resolved Issues
 ---------------
-- 
2.7.4

^ permalink raw reply related

* [PATCH v5 5/8] app/testpmd: add MACsec offload commands
From: Tiwei Bie @ 2017-01-04  7:21 UTC (permalink / raw)
  To: dev
  Cc: adrien.mazarguil, wenzhuo.lu, john.mcnamara, olivier.matz,
	thomas.monjalon, konstantin.ananyev, helin.zhang, wei.dai,
	xiao.w.wang
In-Reply-To: <1483514502-32841-1-git-send-email-tiwei.bie@intel.com>

Below MACsec offload commands are added:

- set macsec offload <port_id> on encrypt on|off replay-protect on|off
- set macsec offload <port_id> off
- set macsec sc tx|rx <port_id> <mac> <pi>
- set macsec sa tx|rx <port_id> <idx> <an> <pn> <key>

Also update the testpmd user guide.

Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
 app/test-pmd/cmdline.c                      | 389 ++++++++++++++++++++++++++++
 app/test-pmd/macfwd.c                       |   7 +
 app/test-pmd/macswap.c                      |   7 +
 app/test-pmd/testpmd.h                      |   2 +
 app/test-pmd/txonly.c                       |   7 +
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  32 +++
 6 files changed, 444 insertions(+)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index f768b6b..4a67894 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -275,6 +275,18 @@ static void cmd_help_long_parsed(void *parsed_result,
 
 			"set vf mac antispoof (port_id) (vf_id) (on|off).\n"
 			"    Set MAC antispoof for a VF from the PF.\n\n"
+
+			"set macsec offload (port_id) on encrypt (on|off) replay-protect (on|off)\n"
+			"    Enable MACsec offload.\n\n"
+
+			"set macsec offload (port_id) off\n"
+			"    Disable MACsec offload.\n\n"
+
+			"set macsec sc (tx|rx) (port_id) (mac) (pi)\n"
+			"    Configure MACsec secure connection (SC).\n\n"
+
+			"set macsec sa (tx|rx) (port_id) (idx) (an) (pn) (key)\n"
+			"    Configure MACsec secure association (SA).\n\n"
 #endif
 
 			"vlan set strip (on|off) (port_id)\n"
@@ -11488,6 +11500,379 @@ cmdline_parse_inst_t cmd_set_vf_mac_addr = {
 		NULL,
 	},
 };
+
+/* MACsec configuration */
+
+/* Common result structure for MACsec offload enable */
+struct cmd_macsec_offload_on_result {
+	cmdline_fixed_string_t set;
+	cmdline_fixed_string_t macsec;
+	cmdline_fixed_string_t offload;
+	uint8_t port_id;
+	cmdline_fixed_string_t on;
+	cmdline_fixed_string_t encrypt;
+	cmdline_fixed_string_t en_on_off;
+	cmdline_fixed_string_t replay_protect;
+	cmdline_fixed_string_t rp_on_off;
+};
+
+/* Common CLI fields for MACsec offload disable */
+cmdline_parse_token_string_t cmd_macsec_offload_on_set =
+	TOKEN_STRING_INITIALIZER
+		(struct cmd_macsec_offload_on_result,
+		 set, "set");
+cmdline_parse_token_string_t cmd_macsec_offload_on_macsec =
+	TOKEN_STRING_INITIALIZER
+		(struct cmd_macsec_offload_on_result,
+		 macsec, "macsec");
+cmdline_parse_token_string_t cmd_macsec_offload_on_offload =
+	TOKEN_STRING_INITIALIZER
+		(struct cmd_macsec_offload_on_result,
+		 offload, "offload");
+cmdline_parse_token_string_t cmd_macsec_offload_on_port_id =
+	TOKEN_NUM_INITIALIZER
+		(struct cmd_macsec_offload_on_result,
+		 port_id, UINT8);
+cmdline_parse_token_string_t cmd_macsec_offload_on_on =
+	TOKEN_STRING_INITIALIZER
+		(struct cmd_macsec_offload_on_result,
+		 on, "on");
+cmdline_parse_token_string_t cmd_macsec_offload_on_encrypt =
+	TOKEN_STRING_INITIALIZER
+		(struct cmd_macsec_offload_on_result,
+		 encrypt, "encrypt");
+cmdline_parse_token_string_t cmd_macsec_offload_on_en_on_off =
+	TOKEN_STRING_INITIALIZER
+		(struct cmd_macsec_offload_on_result,
+		 en_on_off, "on#off");
+cmdline_parse_token_string_t cmd_macsec_offload_on_replay_protect =
+	TOKEN_STRING_INITIALIZER
+		(struct cmd_macsec_offload_on_result,
+		 replay_protect, "replay-protect");
+cmdline_parse_token_string_t cmd_macsec_offload_on_rp_on_off =
+	TOKEN_STRING_INITIALIZER
+		(struct cmd_macsec_offload_on_result,
+		 rp_on_off, "on#off");
+
+static void
+cmd_set_macsec_offload_on_parsed(
+	void *parsed_result,
+	__attribute__((unused)) struct cmdline *cl,
+	__attribute__((unused)) void *data)
+{
+	struct cmd_macsec_offload_on_result *res = parsed_result;
+	int ret;
+	portid_t port_id = res->port_id;
+	int en = (strcmp(res->en_on_off, "on") == 0) ? 1 : 0;
+	int rp = (strcmp(res->rp_on_off, "on") == 0) ? 1 : 0;
+
+	if (port_id_is_invalid(port_id, ENABLED_WARN))
+		return;
+
+	ports[port_id].tx_ol_flags |= TESTPMD_TX_OFFLOAD_MACSEC;
+	ret = rte_pmd_ixgbe_macsec_enable(port_id, en, rp);
+
+	switch (ret) {
+	case 0:
+		break;
+	case -ENODEV:
+		printf("invalid port_id %d\n", port_id);
+		break;
+	default:
+		printf("programming error: (%s)\n", strerror(-ret));
+	}
+}
+
+cmdline_parse_inst_t cmd_set_macsec_offload_on = {
+	.f = cmd_set_macsec_offload_on_parsed,
+	.data = NULL,
+	.help_str = "set macsec offload <port_id> on "
+		"encrypt on|off replay-protect on|off",
+	.tokens = {
+		(void *)&cmd_macsec_offload_on_set,
+		(void *)&cmd_macsec_offload_on_macsec,
+		(void *)&cmd_macsec_offload_on_offload,
+		(void *)&cmd_macsec_offload_on_port_id,
+		(void *)&cmd_macsec_offload_on_on,
+		(void *)&cmd_macsec_offload_on_encrypt,
+		(void *)&cmd_macsec_offload_on_en_on_off,
+		(void *)&cmd_macsec_offload_on_replay_protect,
+		(void *)&cmd_macsec_offload_on_rp_on_off,
+		NULL,
+	},
+};
+
+/* Common result structure for MACsec offload disable */
+struct cmd_macsec_offload_off_result {
+	cmdline_fixed_string_t set;
+	cmdline_fixed_string_t macsec;
+	cmdline_fixed_string_t offload;
+	uint8_t port_id;
+	cmdline_fixed_string_t off;
+};
+
+/* Common CLI fields for MACsec offload disable */
+cmdline_parse_token_string_t cmd_macsec_offload_off_set =
+	TOKEN_STRING_INITIALIZER
+		(struct cmd_macsec_offload_off_result,
+		 set, "set");
+cmdline_parse_token_string_t cmd_macsec_offload_off_macsec =
+	TOKEN_STRING_INITIALIZER
+		(struct cmd_macsec_offload_off_result,
+		 macsec, "macsec");
+cmdline_parse_token_string_t cmd_macsec_offload_off_offload =
+	TOKEN_STRING_INITIALIZER
+		(struct cmd_macsec_offload_off_result,
+		 offload, "offload");
+cmdline_parse_token_string_t cmd_macsec_offload_off_port_id =
+	TOKEN_NUM_INITIALIZER
+		(struct cmd_macsec_offload_off_result,
+		 port_id, UINT8);
+cmdline_parse_token_string_t cmd_macsec_offload_off_off =
+	TOKEN_STRING_INITIALIZER
+		(struct cmd_macsec_offload_off_result,
+		 off, "off");
+
+static void
+cmd_set_macsec_offload_off_parsed(
+	void *parsed_result,
+	__attribute__((unused)) struct cmdline *cl,
+	__attribute__((unused)) void *data)
+{
+	struct cmd_macsec_offload_off_result *res = parsed_result;
+	int ret;
+	portid_t port_id = res->port_id;
+
+	if (port_id_is_invalid(port_id, ENABLED_WARN))
+		return;
+
+	ports[port_id].tx_ol_flags &= ~TESTPMD_TX_OFFLOAD_MACSEC;
+	ret = rte_pmd_ixgbe_macsec_disable(port_id);
+
+	switch (ret) {
+	case 0:
+		break;
+	case -ENODEV:
+		printf("invalid port_id %d\n", port_id);
+		break;
+	default:
+		printf("programming error: (%s)\n", strerror(-ret));
+	}
+}
+
+cmdline_parse_inst_t cmd_set_macsec_offload_off = {
+	.f = cmd_set_macsec_offload_off_parsed,
+	.data = NULL,
+	.help_str = "set macsec offload <port_id> off",
+	.tokens = {
+		(void *)&cmd_macsec_offload_off_set,
+		(void *)&cmd_macsec_offload_off_macsec,
+		(void *)&cmd_macsec_offload_off_offload,
+		(void *)&cmd_macsec_offload_off_port_id,
+		(void *)&cmd_macsec_offload_off_off,
+		NULL,
+	},
+};
+
+/* Common result structure for MACsec secure connection configure */
+struct cmd_macsec_sc_result {
+	cmdline_fixed_string_t set;
+	cmdline_fixed_string_t macsec;
+	cmdline_fixed_string_t sc;
+	cmdline_fixed_string_t tx_rx;
+	uint8_t port_id;
+	struct ether_addr mac;
+	uint16_t pi;
+};
+
+/* Common CLI fields for MACsec secure connection configure */
+cmdline_parse_token_string_t cmd_macsec_sc_set =
+	TOKEN_STRING_INITIALIZER
+		(struct cmd_macsec_sc_result,
+		 set, "set");
+cmdline_parse_token_string_t cmd_macsec_sc_macsec =
+	TOKEN_STRING_INITIALIZER
+		(struct cmd_macsec_sc_result,
+		 macsec, "macsec");
+cmdline_parse_token_string_t cmd_macsec_sc_sc =
+	TOKEN_STRING_INITIALIZER
+		(struct cmd_macsec_sc_result,
+		 sc, "sc");
+cmdline_parse_token_string_t cmd_macsec_sc_tx_rx =
+	TOKEN_STRING_INITIALIZER
+		(struct cmd_macsec_sc_result,
+		 tx_rx, "tx#rx");
+cmdline_parse_token_num_t cmd_macsec_sc_port_id =
+	TOKEN_NUM_INITIALIZER
+		(struct cmd_macsec_sc_result,
+		 port_id, UINT8);
+cmdline_parse_token_etheraddr_t cmd_macsec_sc_mac =
+	TOKEN_ETHERADDR_INITIALIZER
+		(struct cmd_macsec_sc_result,
+		 mac);
+cmdline_parse_token_num_t cmd_macsec_sc_pi =
+	TOKEN_NUM_INITIALIZER
+		(struct cmd_macsec_sc_result,
+		 pi, UINT16);
+
+static void
+cmd_set_macsec_sc_parsed(
+	void *parsed_result,
+	__attribute__((unused)) struct cmdline *cl,
+	__attribute__((unused)) void *data)
+{
+	struct cmd_macsec_sc_result *res = parsed_result;
+	int ret;
+	int is_tx = (strcmp(res->tx_rx, "tx") == 0) ? 1 : 0;
+
+	ret = is_tx ?
+		rte_pmd_ixgbe_macsec_config_txsc(res->port_id,
+				res->mac.addr_bytes) :
+		rte_pmd_ixgbe_macsec_config_rxsc(res->port_id,
+				res->mac.addr_bytes, res->pi);
+	switch (ret) {
+	case 0:
+		break;
+	case -ENODEV:
+		printf("invalid port_id %d\n", res->port_id);
+		break;
+	default:
+		printf("programming error: (%s)\n", strerror(-ret));
+	}
+}
+
+cmdline_parse_inst_t cmd_set_macsec_sc = {
+	.f = cmd_set_macsec_sc_parsed,
+	.data = NULL,
+	.help_str = "set macsec sc tx|rx <port_id> <mac> <pi>",
+	.tokens = {
+		(void *)&cmd_macsec_sc_set,
+		(void *)&cmd_macsec_sc_macsec,
+		(void *)&cmd_macsec_sc_sc,
+		(void *)&cmd_macsec_sc_tx_rx,
+		(void *)&cmd_macsec_sc_port_id,
+		(void *)&cmd_macsec_sc_mac,
+		(void *)&cmd_macsec_sc_pi,
+		NULL,
+	},
+};
+
+/* Common result structure for MACsec secure connection configure */
+struct cmd_macsec_sa_result {
+	cmdline_fixed_string_t set;
+	cmdline_fixed_string_t macsec;
+	cmdline_fixed_string_t sa;
+	cmdline_fixed_string_t tx_rx;
+	uint8_t port_id;
+	uint8_t idx;
+	uint8_t an;
+	uint32_t pn;
+	cmdline_fixed_string_t key;
+};
+
+/* Common CLI fields for MACsec secure connection configure */
+cmdline_parse_token_string_t cmd_macsec_sa_set =
+	TOKEN_STRING_INITIALIZER
+		(struct cmd_macsec_sa_result,
+		 set, "set");
+cmdline_parse_token_string_t cmd_macsec_sa_macsec =
+	TOKEN_STRING_INITIALIZER
+		(struct cmd_macsec_sa_result,
+		 macsec, "macsec");
+cmdline_parse_token_string_t cmd_macsec_sa_sa =
+	TOKEN_STRING_INITIALIZER
+		(struct cmd_macsec_sa_result,
+		 sa, "sa");
+cmdline_parse_token_string_t cmd_macsec_sa_tx_rx =
+	TOKEN_STRING_INITIALIZER
+		(struct cmd_macsec_sa_result,
+		 tx_rx, "tx#rx");
+cmdline_parse_token_num_t cmd_macsec_sa_port_id =
+	TOKEN_NUM_INITIALIZER
+		(struct cmd_macsec_sa_result,
+		 port_id, UINT8);
+cmdline_parse_token_num_t cmd_macsec_sa_idx =
+	TOKEN_NUM_INITIALIZER
+		(struct cmd_macsec_sa_result,
+		 idx, UINT8);
+cmdline_parse_token_num_t cmd_macsec_sa_an =
+	TOKEN_NUM_INITIALIZER
+		(struct cmd_macsec_sa_result,
+		 an, UINT8);
+cmdline_parse_token_num_t cmd_macsec_sa_pn =
+	TOKEN_NUM_INITIALIZER
+		(struct cmd_macsec_sa_result,
+		 pn, UINT32);
+cmdline_parse_token_string_t cmd_macsec_sa_key =
+	TOKEN_STRING_INITIALIZER
+		(struct cmd_macsec_sa_result,
+		 key, NULL);
+
+static void
+cmd_set_macsec_sa_parsed(
+	void *parsed_result,
+	__attribute__((unused)) struct cmdline *cl,
+	__attribute__((unused)) void *data)
+{
+	struct cmd_macsec_sa_result *res = parsed_result;
+	int ret;
+	int is_tx = (strcmp(res->tx_rx, "tx") == 0) ? 1 : 0;
+	uint8_t key[16] = { 0 };
+	uint8_t xdgt0;
+	uint8_t xdgt1;
+	int key_len;
+	int i;
+
+	key_len = strlen(res->key) / 2;
+	if (key_len > 16)
+		key_len = 16;
+
+	for (i = 0; i < key_len; i++) {
+		xdgt0 = parse_and_check_key_hexa_digit(res->key, (i * 2));
+		if (xdgt0 == 0xFF)
+			return;
+		xdgt1 = parse_and_check_key_hexa_digit(res->key, (i * 2) + 1);
+		if (xdgt1 == 0xFF)
+			return;
+		key[i] = (uint8_t) ((xdgt0 * 16) + xdgt1);
+	}
+
+	ret = is_tx ?
+		rte_pmd_ixgbe_macsec_select_txsa(res->port_id,
+			res->idx, res->an, res->pn, key) :
+		rte_pmd_ixgbe_macsec_select_rxsa(res->port_id,
+			res->idx, res->an, res->pn, key);
+	switch (ret) {
+	case 0:
+		break;
+	case -EINVAL:
+		printf("invalid idx %d or an %d\n", res->idx, res->an);
+		break;
+	case -ENODEV:
+		printf("invalid port_id %d\n", res->port_id);
+		break;
+	default:
+		printf("programming error: (%s)\n", strerror(-ret));
+	}
+}
+
+cmdline_parse_inst_t cmd_set_macsec_sa = {
+	.f = cmd_set_macsec_sa_parsed,
+	.data = NULL,
+	.help_str = "set macsec sa tx|rx <port_id> <idx> <an> <pn> <key>",
+	.tokens = {
+		(void *)&cmd_macsec_sa_set,
+		(void *)&cmd_macsec_sa_macsec,
+		(void *)&cmd_macsec_sa_sa,
+		(void *)&cmd_macsec_sa_tx_rx,
+		(void *)&cmd_macsec_sa_port_id,
+		(void *)&cmd_macsec_sa_idx,
+		(void *)&cmd_macsec_sa_an,
+		(void *)&cmd_macsec_sa_pn,
+		(void *)&cmd_macsec_sa_key,
+		NULL,
+	},
+};
 #endif
 
 /* ******************************************************************************** */
@@ -11656,6 +12041,10 @@ cmdline_parse_ctx_t main_ctx[] = {
 	(cmdline_parse_inst_t *)&cmd_set_all_queues_drop_en,
 	(cmdline_parse_inst_t *)&cmd_set_vf_split_drop_en,
 	(cmdline_parse_inst_t *)&cmd_set_vf_mac_addr,
+	(cmdline_parse_inst_t *)&cmd_set_macsec_offload_on,
+	(cmdline_parse_inst_t *)&cmd_set_macsec_offload_off,
+	(cmdline_parse_inst_t *)&cmd_set_macsec_sc,
+	(cmdline_parse_inst_t *)&cmd_set_macsec_sa,
 #endif
 	NULL,
 };
diff --git a/app/test-pmd/macfwd.c b/app/test-pmd/macfwd.c
index d361db1..38e955f 100644
--- a/app/test-pmd/macfwd.c
+++ b/app/test-pmd/macfwd.c
@@ -66,6 +66,9 @@
 #include <rte_ip.h>
 #include <rte_string_fns.h>
 #include <rte_flow.h>
+#ifdef RTE_LIBRTE_IXGBE_PMD
+#include <rte_pmd_ixgbe.h>
+#endif
 
 #include "testpmd.h"
 
@@ -113,6 +116,10 @@ pkt_burst_mac_forward(struct fwd_stream *fs)
 		ol_flags = PKT_TX_VLAN_PKT;
 	if (txp->tx_ol_flags & TESTPMD_TX_OFFLOAD_INSERT_QINQ)
 		ol_flags |= PKT_TX_QINQ_PKT;
+#ifdef RTE_LIBRTE_IXGBE_PMD
+	if (txp->tx_ol_flags & TESTPMD_TX_OFFLOAD_MACSEC)
+		ol_flags |= PKT_TX_IXGBE_MACSEC;
+#endif
 	for (i = 0; i < nb_rx; i++) {
 		if (likely(i < nb_rx - 1))
 			rte_prefetch0(rte_pktmbuf_mtod(pkts_burst[i + 1],
diff --git a/app/test-pmd/macswap.c b/app/test-pmd/macswap.c
index f996039..7f5dcd7 100644
--- a/app/test-pmd/macswap.c
+++ b/app/test-pmd/macswap.c
@@ -66,6 +66,9 @@
 #include <rte_ip.h>
 #include <rte_string_fns.h>
 #include <rte_flow.h>
+#ifdef RTE_LIBRTE_IXGBE_PMD
+#include <rte_pmd_ixgbe.h>
+#endif
 
 #include "testpmd.h"
 
@@ -113,6 +116,10 @@ pkt_burst_mac_swap(struct fwd_stream *fs)
 		ol_flags = PKT_TX_VLAN_PKT;
 	if (txp->tx_ol_flags & TESTPMD_TX_OFFLOAD_INSERT_QINQ)
 		ol_flags |= PKT_TX_QINQ_PKT;
+#ifdef RTE_LIBRTE_IXGBE_PMD
+	if (txp->tx_ol_flags & TESTPMD_TX_OFFLOAD_MACSEC)
+		ol_flags |= PKT_TX_IXGBE_MACSEC;
+#endif
 	for (i = 0; i < nb_rx; i++) {
 		if (likely(i < nb_rx - 1))
 			rte_prefetch0(rte_pktmbuf_mtod(pkts_burst[i + 1],
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 22ce2d6..0a9a1af 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -143,6 +143,8 @@ struct fwd_stream {
 #define TESTPMD_TX_OFFLOAD_INSERT_VLAN       0x0040
 /** Insert double VLAN header in forward engine */
 #define TESTPMD_TX_OFFLOAD_INSERT_QINQ       0x0080
+/** Offload MACsec in forward engine */
+#define TESTPMD_TX_OFFLOAD_MACSEC            0x0100
 
 /** Descriptor for a single flow. */
 struct port_flow {
diff --git a/app/test-pmd/txonly.c b/app/test-pmd/txonly.c
index e996f35..3aea8b3 100644
--- a/app/test-pmd/txonly.c
+++ b/app/test-pmd/txonly.c
@@ -69,6 +69,9 @@
 #include <rte_udp.h>
 #include <rte_string_fns.h>
 #include <rte_flow.h>
+#ifdef RTE_LIBRTE_IXGBE_PMD
+#include <rte_pmd_ixgbe.h>
+#endif
 
 #include "testpmd.h"
 
@@ -215,6 +218,10 @@ pkt_burst_transmit(struct fwd_stream *fs)
 		ol_flags = PKT_TX_VLAN_PKT;
 	if (txp->tx_ol_flags & TESTPMD_TX_OFFLOAD_INSERT_QINQ)
 		ol_flags |= PKT_TX_QINQ_PKT;
+#ifdef RTE_LIBRTE_IXGBE_PMD
+	if (txp->tx_ol_flags & TESTPMD_TX_OFFLOAD_MACSEC)
+		ol_flags |= PKT_TX_IXGBE_MACSEC;
+#endif
 	for (nb_pkt = 0; nb_pkt < nb_pkt_per_burst; nb_pkt++) {
 		pkt = rte_mbuf_raw_alloc(mbp);
 		if (pkt == NULL) {
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index cacdef1..f4068d3 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -507,6 +507,38 @@ Set mac antispoof for a VF from the PF::
 
    testpmd> set vf mac antispoof  (port_id) (vf_id) (on|off)
 
+set macsec offload
+~~~~~~~~~~~~~~~~~~
+
+Enable/disable MACsec offload::
+
+   testpmd> set macsec offload (port_id) on encrypt (on|off) replay-protect (on|off)
+   testpmd> set macsec offload (port_id) off
+
+set macsec sc
+~~~~~~~~~~~~~
+
+Configure MACsec secure connection (SC)::
+
+   testpmd> set macsec sc (tx|rx) (port_id) (mac) (pi)
+
+.. note::
+
+   The pi argument is ignored for tx.
+   Check the NIC Datasheet for hardware limits.
+
+set macsec sa
+~~~~~~~~~~~~~
+
+Configure MACsec secure association (SA)::
+
+   testpmd> set macsec sa (tx|rx) (port_id) (idx) (an) (pn) (key)
+
+.. note::
+
+   The IDX value must be 0 or 1.
+   Check the NIC Datasheet for hardware limits.
+
 vlan set strip
 ~~~~~~~~~~~~~~
 
-- 
2.7.4

^ permalink raw reply related

* [PATCH v5 4/8] net/ixgbe: add MACsec offload support
From: Tiwei Bie @ 2017-01-04  7:21 UTC (permalink / raw)
  To: dev
  Cc: adrien.mazarguil, wenzhuo.lu, john.mcnamara, olivier.matz,
	thomas.monjalon, konstantin.ananyev, helin.zhang, wei.dai,
	xiao.w.wang
In-Reply-To: <1483514502-32841-1-git-send-email-tiwei.bie@intel.com>

MACsec (or LinkSec, 802.1AE) is a MAC level encryption/authentication
scheme defined in IEEE 802.1AE that uses symmetric cryptography.
This commit adds the MACsec offload support for ixgbe.

Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c            | 481 +++++++++++++++++++++++++++-
 drivers/net/ixgbe/ixgbe_ethdev.h            |  45 +++
 drivers/net/ixgbe/ixgbe_rxtx.c              |   5 +
 drivers/net/ixgbe/rte_pmd_ixgbe.h           | 122 +++++++
 drivers/net/ixgbe/rte_pmd_ixgbe_version.map |  11 +
 5 files changed, 659 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index ec2edad..653ddd2 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -231,6 +231,7 @@ static int ixgbe_dev_rss_reta_query(struct rte_eth_dev *dev,
 			uint16_t reta_size);
 static void ixgbe_dev_link_status_print(struct rte_eth_dev *dev);
 static int ixgbe_dev_lsc_interrupt_setup(struct rte_eth_dev *dev);
+static int ixgbe_dev_macsec_interrupt_setup(struct rte_eth_dev *dev);
 static int ixgbe_dev_rxq_interrupt_setup(struct rte_eth_dev *dev);
 static int ixgbe_dev_interrupt_get_status(struct rte_eth_dev *dev);
 static int ixgbe_dev_interrupt_action(struct rte_eth_dev *dev,
@@ -745,6 +746,51 @@ static const struct rte_ixgbe_xstats_name_off rte_ixgbe_stats_strings[] = {
 #define IXGBE_NB_HW_STATS (sizeof(rte_ixgbe_stats_strings) / \
 			   sizeof(rte_ixgbe_stats_strings[0]))
 
+/* MACsec statistics */
+static const struct rte_ixgbe_xstats_name_off rte_ixgbe_macsec_strings[] = {
+	{"out_pkts_untagged", offsetof(struct ixgbe_macsec_stats,
+		out_pkts_untagged)},
+	{"out_pkts_encrypted", offsetof(struct ixgbe_macsec_stats,
+		out_pkts_encrypted)},
+	{"out_pkts_protected", offsetof(struct ixgbe_macsec_stats,
+		out_pkts_protected)},
+	{"out_octets_encrypted", offsetof(struct ixgbe_macsec_stats,
+		out_octets_encrypted)},
+	{"out_octets_protected", offsetof(struct ixgbe_macsec_stats,
+		out_octets_protected)},
+	{"in_pkts_untagged", offsetof(struct ixgbe_macsec_stats,
+		in_pkts_untagged)},
+	{"in_pkts_badtag", offsetof(struct ixgbe_macsec_stats,
+		in_pkts_badtag)},
+	{"in_pkts_nosci", offsetof(struct ixgbe_macsec_stats,
+		in_pkts_nosci)},
+	{"in_pkts_unknownsci", offsetof(struct ixgbe_macsec_stats,
+		in_pkts_unknownsci)},
+	{"in_octets_decrypted", offsetof(struct ixgbe_macsec_stats,
+		in_octets_decrypted)},
+	{"in_octets_validated", offsetof(struct ixgbe_macsec_stats,
+		in_octets_validated)},
+	{"in_pkts_unchecked", offsetof(struct ixgbe_macsec_stats,
+		in_pkts_unchecked)},
+	{"in_pkts_delayed", offsetof(struct ixgbe_macsec_stats,
+		in_pkts_delayed)},
+	{"in_pkts_late", offsetof(struct ixgbe_macsec_stats,
+		in_pkts_late)},
+	{"in_pkts_ok", offsetof(struct ixgbe_macsec_stats,
+		in_pkts_ok)},
+	{"in_pkts_invalid", offsetof(struct ixgbe_macsec_stats,
+		in_pkts_invalid)},
+	{"in_pkts_notvalid", offsetof(struct ixgbe_macsec_stats,
+		in_pkts_notvalid)},
+	{"in_pkts_unusedsa", offsetof(struct ixgbe_macsec_stats,
+		in_pkts_unusedsa)},
+	{"in_pkts_notusingsa", offsetof(struct ixgbe_macsec_stats,
+		in_pkts_notusingsa)},
+};
+
+#define IXGBE_NB_MACSEC_STATS (sizeof(rte_ixgbe_macsec_strings) / \
+			   sizeof(rte_ixgbe_macsec_strings[0]))
+
 /* Per-queue statistics */
 static const struct rte_ixgbe_xstats_name_off rte_ixgbe_rxq_strings[] = {
 	{"mbuf_allocation_errors", offsetof(struct ixgbe_hw_stats, rnbc)},
@@ -2367,6 +2413,7 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
 		/* check if lsc interrupt is enabled */
 		if (dev->data->dev_conf.intr_conf.lsc != 0)
 			ixgbe_dev_lsc_interrupt_setup(dev);
+		ixgbe_dev_macsec_interrupt_setup(dev);
 	} else {
 		rte_intr_callback_unregister(intr_handle,
 					     ixgbe_dev_interrupt_handler, dev);
@@ -2557,6 +2604,7 @@ ixgbe_dev_close(struct rte_eth_dev *dev)
 static void
 ixgbe_read_stats_registers(struct ixgbe_hw *hw,
 			   struct ixgbe_hw_stats *hw_stats,
+			   struct ixgbe_macsec_stats *macsec_stats,
 			   uint64_t *total_missed_rx, uint64_t *total_qbrc,
 			   uint64_t *total_qprc, uint64_t *total_qprdc)
 {
@@ -2726,6 +2774,40 @@ ixgbe_read_stats_registers(struct ixgbe_hw *hw,
 	/* Flow Director Stats registers */
 	hw_stats->fdirmatch += IXGBE_READ_REG(hw, IXGBE_FDIRMATCH);
 	hw_stats->fdirmiss += IXGBE_READ_REG(hw, IXGBE_FDIRMISS);
+
+	/* MACsec Stats registers */
+	macsec_stats->out_pkts_untagged += IXGBE_READ_REG(hw, IXGBE_LSECTXUT);
+	macsec_stats->out_pkts_encrypted +=
+		IXGBE_READ_REG(hw, IXGBE_LSECTXPKTE);
+	macsec_stats->out_pkts_protected +=
+		IXGBE_READ_REG(hw, IXGBE_LSECTXPKTP);
+	macsec_stats->out_octets_encrypted +=
+		IXGBE_READ_REG(hw, IXGBE_LSECTXOCTE);
+	macsec_stats->out_octets_protected +=
+		IXGBE_READ_REG(hw, IXGBE_LSECTXOCTP);
+	macsec_stats->in_pkts_untagged += IXGBE_READ_REG(hw, IXGBE_LSECRXUT);
+	macsec_stats->in_pkts_badtag += IXGBE_READ_REG(hw, IXGBE_LSECRXBAD);
+	macsec_stats->in_pkts_nosci += IXGBE_READ_REG(hw, IXGBE_LSECRXNOSCI);
+	macsec_stats->in_pkts_unknownsci +=
+		IXGBE_READ_REG(hw, IXGBE_LSECRXUNSCI);
+	macsec_stats->in_octets_decrypted +=
+		IXGBE_READ_REG(hw, IXGBE_LSECRXOCTD);
+	macsec_stats->in_octets_validated +=
+		IXGBE_READ_REG(hw, IXGBE_LSECRXOCTV);
+	macsec_stats->in_pkts_unchecked += IXGBE_READ_REG(hw, IXGBE_LSECRXUNCH);
+	macsec_stats->in_pkts_delayed += IXGBE_READ_REG(hw, IXGBE_LSECRXDELAY);
+	macsec_stats->in_pkts_late += IXGBE_READ_REG(hw, IXGBE_LSECRXLATE);
+	for (i = 0; i < 2; i++) {
+		macsec_stats->in_pkts_ok +=
+			IXGBE_READ_REG(hw, IXGBE_LSECRXOK(i));
+		macsec_stats->in_pkts_invalid +=
+			IXGBE_READ_REG(hw, IXGBE_LSECRXINV(i));
+		macsec_stats->in_pkts_notvalid +=
+			IXGBE_READ_REG(hw, IXGBE_LSECRXNV(i));
+	}
+	macsec_stats->in_pkts_unusedsa += IXGBE_READ_REG(hw, IXGBE_LSECRXUNSA);
+	macsec_stats->in_pkts_notusingsa +=
+		IXGBE_READ_REG(hw, IXGBE_LSECRXNUSA);
 }
 
 /*
@@ -2738,6 +2820,9 @@ ixgbe_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 			IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct ixgbe_hw_stats *hw_stats =
 			IXGBE_DEV_PRIVATE_TO_STATS(dev->data->dev_private);
+	struct ixgbe_macsec_stats *macsec_stats =
+			IXGBE_DEV_PRIVATE_TO_MACSEC_STATS(
+				dev->data->dev_private);
 	uint64_t total_missed_rx, total_qbrc, total_qprc, total_qprdc;
 	unsigned i;
 
@@ -2746,8 +2831,8 @@ ixgbe_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 	total_qprc = 0;
 	total_qprdc = 0;
 
-	ixgbe_read_stats_registers(hw, hw_stats, &total_missed_rx, &total_qbrc,
-			&total_qprc, &total_qprdc);
+	ixgbe_read_stats_registers(hw, hw_stats, macsec_stats, &total_missed_rx,
+			&total_qbrc, &total_qprc, &total_qprdc);
 
 	if (stats == NULL)
 		return;
@@ -2799,7 +2884,7 @@ ixgbe_dev_stats_reset(struct rte_eth_dev *dev)
 /* This function calculates the number of xstats based on the current config */
 static unsigned
 ixgbe_xstats_calc_num(void) {
-	return IXGBE_NB_HW_STATS +
+	return IXGBE_NB_HW_STATS + IXGBE_NB_MACSEC_STATS +
 		(IXGBE_NB_RXQ_PRIO_STATS * IXGBE_NB_RXQ_PRIO_VALUES) +
 		(IXGBE_NB_TXQ_PRIO_STATS * IXGBE_NB_TXQ_PRIO_VALUES);
 }
@@ -2826,6 +2911,15 @@ static int ixgbe_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 			count++;
 		}
 
+		/* MACsec Stats */
+		for (i = 0; i < IXGBE_NB_MACSEC_STATS; i++) {
+			snprintf(xstats_names[count].name,
+				sizeof(xstats_names[count].name),
+				"%s",
+				rte_ixgbe_macsec_strings[i].name);
+			count++;
+		}
+
 		/* RX Priority Stats */
 		for (stat = 0; stat < IXGBE_NB_RXQ_PRIO_STATS; stat++) {
 			for (i = 0; i < IXGBE_NB_RXQ_PRIO_VALUES; i++) {
@@ -2875,6 +2969,9 @@ ixgbe_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 			IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct ixgbe_hw_stats *hw_stats =
 			IXGBE_DEV_PRIVATE_TO_STATS(dev->data->dev_private);
+	struct ixgbe_macsec_stats *macsec_stats =
+			IXGBE_DEV_PRIVATE_TO_MACSEC_STATS(
+				dev->data->dev_private);
 	uint64_t total_missed_rx, total_qbrc, total_qprc, total_qprdc;
 	unsigned i, stat, count = 0;
 
@@ -2888,8 +2985,8 @@ ixgbe_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 	total_qprc = 0;
 	total_qprdc = 0;
 
-	ixgbe_read_stats_registers(hw, hw_stats, &total_missed_rx, &total_qbrc,
-				   &total_qprc, &total_qprdc);
+	ixgbe_read_stats_registers(hw, hw_stats, macsec_stats, &total_missed_rx,
+			&total_qbrc, &total_qprc, &total_qprdc);
 
 	/* If this is a reset xstats is NULL, and we have cleared the
 	 * registers by reading them.
@@ -2905,6 +3002,13 @@ ixgbe_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 		count++;
 	}
 
+	/* MACsec Stats */
+	for (i = 0; i < IXGBE_NB_MACSEC_STATS; i++) {
+		xstats[count].value = *(uint64_t *)(((char *)macsec_stats) +
+				rte_ixgbe_macsec_strings[i].offset);
+		count++;
+	}
+
 	/* RX Priority Stats */
 	for (stat = 0; stat < IXGBE_NB_RXQ_PRIO_STATS; stat++) {
 		for (i = 0; i < IXGBE_NB_RXQ_PRIO_VALUES; i++) {
@@ -2932,6 +3036,9 @@ ixgbe_dev_xstats_reset(struct rte_eth_dev *dev)
 {
 	struct ixgbe_hw_stats *stats =
 			IXGBE_DEV_PRIVATE_TO_STATS(dev->data->dev_private);
+	struct ixgbe_macsec_stats *macsec_stats =
+			IXGBE_DEV_PRIVATE_TO_MACSEC_STATS(
+				dev->data->dev_private);
 
 	unsigned count = ixgbe_xstats_calc_num();
 
@@ -2940,6 +3047,7 @@ ixgbe_dev_xstats_reset(struct rte_eth_dev *dev)
 
 	/* Reset software totals */
 	memset(stats, 0, sizeof(*stats));
+	memset(macsec_stats, 0, sizeof(*macsec_stats));
 }
 
 static void
@@ -3072,6 +3180,10 @@ ixgbe_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	    !RTE_ETH_DEV_SRIOV(dev).active)
 		dev_info->rx_offload_capa |= DEV_RX_OFFLOAD_TCP_LRO;
 
+	if (hw->mac.type == ixgbe_mac_82599EB ||
+	    hw->mac.type == ixgbe_mac_X540)
+		dev_info->rx_offload_capa |= DEV_RX_OFFLOAD_IXGBE_MACSEC_STRIP;
+
 	if (hw->mac.type == ixgbe_mac_X550 ||
 	    hw->mac.type == ixgbe_mac_X550EM_x ||
 	    hw->mac.type == ixgbe_mac_X550EM_a)
@@ -3085,6 +3197,10 @@ ixgbe_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 		DEV_TX_OFFLOAD_SCTP_CKSUM  |
 		DEV_TX_OFFLOAD_TCP_TSO;
 
+	if (hw->mac.type == ixgbe_mac_82599EB ||
+	    hw->mac.type == ixgbe_mac_X540)
+		dev_info->tx_offload_capa |= DEV_TX_OFFLOAD_IXGBE_MACSEC_INSERT;
+
 	if (hw->mac.type == ixgbe_mac_X550 ||
 	    hw->mac.type == ixgbe_mac_X550EM_x ||
 	    hw->mac.type == ixgbe_mac_X550EM_a)
@@ -3382,6 +3498,28 @@ ixgbe_dev_rxq_interrupt_setup(struct rte_eth_dev *dev)
 	return 0;
 }
 
+/**
+ * It clears the interrupt causes and enables the interrupt.
+ * It will be called once only during nic initialized.
+ *
+ * @param dev
+ *  Pointer to struct rte_eth_dev.
+ *
+ * @return
+ *  - On success, zero.
+ *  - On failure, a negative value.
+ */
+static int
+ixgbe_dev_macsec_interrupt_setup(struct rte_eth_dev *dev)
+{
+	struct ixgbe_interrupt *intr =
+		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
+
+	intr->mask |= IXGBE_EICR_LINKSEC;
+
+	return 0;
+}
+
 /*
  * It reads ICR and sets flag (IXGBE_EICR_LSC) for the link_update.
  *
@@ -3416,6 +3554,9 @@ ixgbe_dev_interrupt_get_status(struct rte_eth_dev *dev)
 	if (eicr & IXGBE_EICR_MAILBOX)
 		intr->flags |= IXGBE_FLAG_MAILBOX;
 
+	if (eicr & IXGBE_EICR_LINKSEC)
+		intr->flags |= IXGBE_FLAG_MACSEC;
+
 	if (hw->mac.type ==  ixgbe_mac_X550EM_x &&
 	    hw->phy.type == ixgbe_phy_x550em_ext_t &&
 	    (eicr & IXGBE_EICR_GPI_SDP0_X550EM_x))
@@ -3570,6 +3711,12 @@ ixgbe_dev_interrupt_delayed_handler(void *param)
 		_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC, NULL);
 	}
 
+	if (intr->flags & IXGBE_FLAG_MACSEC) {
+		_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_IXGBE_MACSEC,
+					      NULL);
+		intr->flags &= ~IXGBE_FLAG_MACSEC;
+	}
+
 	PMD_DRV_LOG(DEBUG, "enable intr in delayed handler S[%08x]", eicr);
 	ixgbe_enable_intr(dev);
 	rte_intr_enable(intr_handle);
@@ -7610,6 +7757,330 @@ ixgbevf_dev_interrupt_handler(__rte_unused struct rte_intr_handle *handle,
 	ixgbevf_dev_interrupt_action(dev);
 }
 
+/**
+ *  ixgbe_disable_sec_tx_path_generic - Stops the transmit data path
+ *  @hw: pointer to hardware structure
+ *
+ *  Stops the transmit data path and waits for the HW to internally empty
+ *  the Tx security block
+ **/
+int ixgbe_disable_sec_tx_path_generic(struct ixgbe_hw *hw)
+{
+#define IXGBE_MAX_SECTX_POLL 40
+
+	int i;
+	int sectxreg;
+
+	sectxreg = IXGBE_READ_REG(hw, IXGBE_SECTXCTRL);
+	sectxreg |= IXGBE_SECTXCTRL_TX_DIS;
+	IXGBE_WRITE_REG(hw, IXGBE_SECTXCTRL, sectxreg);
+	for (i = 0; i < IXGBE_MAX_SECTX_POLL; i++) {
+		sectxreg = IXGBE_READ_REG(hw, IXGBE_SECTXSTAT);
+		if (sectxreg & IXGBE_SECTXSTAT_SECTX_RDY)
+			break;
+		/* Use interrupt-safe sleep just in case */
+		usec_delay(1000);
+	}
+
+	/* For informational purposes only */
+	if (i >= IXGBE_MAX_SECTX_POLL)
+		PMD_DRV_LOG(DEBUG, "Tx unit being enabled before security "
+			 "path fully disabled.  Continuing with init.\n");
+
+	return IXGBE_SUCCESS;
+}
+
+/**
+ *  ixgbe_enable_sec_tx_path_generic - Enables the transmit data path
+ *  @hw: pointer to hardware structure
+ *
+ *  Enables the transmit data path.
+ **/
+int ixgbe_enable_sec_tx_path_generic(struct ixgbe_hw *hw)
+{
+	uint32_t sectxreg;
+
+	sectxreg = IXGBE_READ_REG(hw, IXGBE_SECTXCTRL);
+	sectxreg &= ~IXGBE_SECTXCTRL_TX_DIS;
+	IXGBE_WRITE_REG(hw, IXGBE_SECTXCTRL, sectxreg);
+	IXGBE_WRITE_FLUSH(hw);
+
+	return IXGBE_SUCCESS;
+}
+
+int
+rte_pmd_ixgbe_macsec_enable(uint8_t port, uint8_t en, uint8_t rp)
+{
+	struct ixgbe_hw *hw;
+	struct rte_eth_dev *dev;
+	uint32_t ctrl;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
+
+	dev = &rte_eth_devices[port];
+	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	/* Stop the data paths */
+	if (ixgbe_disable_sec_rx_path(hw) != IXGBE_SUCCESS)
+		return -ENOTSUP;
+	/*
+	 * Workaround:
+	 * As no ixgbe_disable_sec_rx_path equivalent is
+	 * implemented for tx in the base code, and we are
+	 * not allowed to modify the base code in DPDK, so
+	 * just call the hand-written one directly for now.
+	 * The hardware support has been checked by
+	 * ixgbe_disable_sec_rx_path().
+	 */
+	ixgbe_disable_sec_tx_path_generic(hw);
+
+	/* Enable Ethernet CRC (required by MACsec offload) */
+	ctrl = IXGBE_READ_REG(hw, IXGBE_HLREG0);
+	ctrl |= IXGBE_HLREG0_TXCRCEN | IXGBE_HLREG0_RXCRCSTRP;
+	IXGBE_WRITE_REG(hw, IXGBE_HLREG0, ctrl);
+
+	/* Enable the TX and RX crypto engines */
+	ctrl = IXGBE_READ_REG(hw, IXGBE_SECTXCTRL);
+	ctrl &= ~IXGBE_SECTXCTRL_SECTX_DIS;
+	IXGBE_WRITE_REG(hw, IXGBE_SECTXCTRL, ctrl);
+
+	ctrl = IXGBE_READ_REG(hw, IXGBE_SECRXCTRL);
+	ctrl &= ~IXGBE_SECRXCTRL_SECRX_DIS;
+	IXGBE_WRITE_REG(hw, IXGBE_SECRXCTRL, ctrl);
+
+	ctrl = IXGBE_READ_REG(hw, IXGBE_SECTXMINIFG);
+	ctrl &= ~IXGBE_SECTX_MINSECIFG_MASK;
+	ctrl |= 0x3;
+	IXGBE_WRITE_REG(hw, IXGBE_SECTXMINIFG, ctrl);
+
+	/* Enable SA lookup */
+	ctrl = IXGBE_READ_REG(hw, IXGBE_LSECTXCTRL);
+	ctrl &= ~IXGBE_LSECTXCTRL_EN_MASK;
+	ctrl |= en ? IXGBE_LSECTXCTRL_AUTH_ENCRYPT :
+		     IXGBE_LSECTXCTRL_AUTH;
+	ctrl |= IXGBE_LSECTXCTRL_AISCI;
+	ctrl &= ~IXGBE_LSECTXCTRL_PNTHRSH_MASK;
+	ctrl |= IXGBE_MACSEC_PNTHRSH & IXGBE_LSECTXCTRL_PNTHRSH_MASK;
+	IXGBE_WRITE_REG(hw, IXGBE_LSECTXCTRL, ctrl);
+
+	ctrl = IXGBE_READ_REG(hw, IXGBE_LSECRXCTRL);
+	ctrl &= ~IXGBE_LSECRXCTRL_EN_MASK;
+	ctrl |= IXGBE_LSECRXCTRL_STRICT << IXGBE_LSECRXCTRL_EN_SHIFT;
+	ctrl &= ~IXGBE_LSECRXCTRL_PLSH;
+	if (rp)
+		ctrl |= IXGBE_LSECRXCTRL_RP;
+	else
+		ctrl &= ~IXGBE_LSECRXCTRL_RP;
+	IXGBE_WRITE_REG(hw, IXGBE_LSECRXCTRL, ctrl);
+
+	/* Start the data paths */
+	ixgbe_enable_sec_rx_path(hw);
+	/*
+	 * Workaround:
+	 * As no ixgbe_enable_sec_rx_path equivalent is
+	 * implemented for tx in the base code, and we are
+	 * not allowed to modify the base code in DPDK, so
+	 * just call the hand-written one directly for now.
+	 */
+	ixgbe_enable_sec_tx_path_generic(hw);
+
+	return 0;
+}
+
+int
+rte_pmd_ixgbe_macsec_disable(uint8_t port)
+{
+	struct ixgbe_hw *hw;
+	struct rte_eth_dev *dev;
+	uint32_t ctrl;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
+
+	dev = &rte_eth_devices[port];
+	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	/* Stop the data paths */
+	if (ixgbe_disable_sec_rx_path(hw) != IXGBE_SUCCESS)
+		return -ENOTSUP;
+	/*
+	 * Workaround:
+	 * As no ixgbe_disable_sec_rx_path equivalent is
+	 * implemented for tx in the base code, and we are
+	 * not allowed to modify the base code in DPDK, so
+	 * just call the hand-written one directly for now.
+	 * The hardware support has been checked by
+	 * ixgbe_disable_sec_rx_path().
+	 */
+	ixgbe_disable_sec_tx_path_generic(hw);
+
+	/* Disable the TX and RX crypto engines */
+	ctrl = IXGBE_READ_REG(hw, IXGBE_SECTXCTRL);
+	ctrl |= IXGBE_SECTXCTRL_SECTX_DIS;
+	IXGBE_WRITE_REG(hw, IXGBE_SECTXCTRL, ctrl);
+
+	ctrl = IXGBE_READ_REG(hw, IXGBE_SECRXCTRL);
+	ctrl |= IXGBE_SECRXCTRL_SECRX_DIS;
+	IXGBE_WRITE_REG(hw, IXGBE_SECRXCTRL, ctrl);
+
+	/* Disable SA lookup */
+	ctrl = IXGBE_READ_REG(hw, IXGBE_LSECTXCTRL);
+	ctrl &= ~IXGBE_LSECTXCTRL_EN_MASK;
+	ctrl |= IXGBE_LSECTXCTRL_DISABLE;
+	IXGBE_WRITE_REG(hw, IXGBE_LSECTXCTRL, ctrl);
+
+	ctrl = IXGBE_READ_REG(hw, IXGBE_LSECRXCTRL);
+	ctrl &= ~IXGBE_LSECRXCTRL_EN_MASK;
+	ctrl |= IXGBE_LSECRXCTRL_DISABLE << IXGBE_LSECRXCTRL_EN_SHIFT;
+	IXGBE_WRITE_REG(hw, IXGBE_LSECRXCTRL, ctrl);
+
+	/* Start the data paths */
+	ixgbe_enable_sec_rx_path(hw);
+	/*
+	 * Workaround:
+	 * As no ixgbe_enable_sec_rx_path equivalent is
+	 * implemented for tx in the base code, and we are
+	 * not allowed to modify the base code in DPDK, so
+	 * just call the hand-written one directly for now.
+	 */
+	ixgbe_enable_sec_tx_path_generic(hw);
+
+	return 0;
+}
+
+int
+rte_pmd_ixgbe_macsec_config_txsc(uint8_t port, uint8_t *mac)
+{
+	struct ixgbe_hw *hw;
+	struct rte_eth_dev *dev;
+	uint32_t ctrl;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
+
+	dev = &rte_eth_devices[port];
+	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	ctrl = mac[0] | (mac[1] << 8) | (mac[2] << 16) | (mac[3] << 24);
+	IXGBE_WRITE_REG(hw, IXGBE_LSECTXSCL, ctrl);
+
+	ctrl = mac[4] | (mac[5] << 8);
+	IXGBE_WRITE_REG(hw, IXGBE_LSECTXSCH, ctrl);
+
+	return 0;
+}
+
+int
+rte_pmd_ixgbe_macsec_config_rxsc(uint8_t port, uint8_t *mac, uint16_t pi)
+{
+	struct ixgbe_hw *hw;
+	struct rte_eth_dev *dev;
+	uint32_t ctrl;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
+
+	dev = &rte_eth_devices[port];
+	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	ctrl = mac[0] | (mac[1] << 8) | (mac[2] << 16) | (mac[3] << 24);
+	IXGBE_WRITE_REG(hw, IXGBE_LSECRXSCL, ctrl);
+
+	pi = rte_cpu_to_be_16(pi);
+	ctrl = mac[4] | (mac[5] << 8) | (pi << 16);
+	IXGBE_WRITE_REG(hw, IXGBE_LSECRXSCH, ctrl);
+
+	return 0;
+}
+
+int
+rte_pmd_ixgbe_macsec_select_txsa(uint8_t port, uint8_t idx, uint8_t an,
+				 uint32_t pn, uint8_t *key)
+{
+	struct ixgbe_hw *hw;
+	struct rte_eth_dev *dev;
+	uint32_t ctrl, i;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
+
+	dev = &rte_eth_devices[port];
+	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	if (idx != 0 && idx != 1)
+		return -EINVAL;
+
+	if (an >= 4)
+		return -EINVAL;
+
+	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	/* Set the PN and key */
+	pn = rte_cpu_to_be_32(pn);
+	if (idx == 0) {
+		IXGBE_WRITE_REG(hw, IXGBE_LSECTXPN0, pn);
+
+		for (i = 0; i < 4; i++) {
+			ctrl = (key[i * 4 + 0] <<  0) |
+			       (key[i * 4 + 1] <<  8) |
+			       (key[i * 4 + 2] << 16) |
+			       (key[i * 4 + 3] << 24);
+			IXGBE_WRITE_REG(hw, IXGBE_LSECTXKEY0(i), ctrl);
+		}
+	} else {
+		IXGBE_WRITE_REG(hw, IXGBE_LSECTXPN1, pn);
+
+		for (i = 0; i < 4; i++) {
+			ctrl = (key[i * 4 + 0] <<  0) |
+			       (key[i * 4 + 1] <<  8) |
+			       (key[i * 4 + 2] << 16) |
+			       (key[i * 4 + 3] << 24);
+			IXGBE_WRITE_REG(hw, IXGBE_LSECTXKEY1(i), ctrl);
+		}
+	}
+
+	/* Set AN and select the SA */
+	ctrl = (an << idx * 2) | (idx << 4);
+	IXGBE_WRITE_REG(hw, IXGBE_LSECTXSA, ctrl);
+
+	return 0;
+}
+
+int
+rte_pmd_ixgbe_macsec_select_rxsa(uint8_t port, uint8_t idx, uint8_t an,
+				 uint32_t pn, uint8_t *key)
+{
+	struct ixgbe_hw *hw;
+	struct rte_eth_dev *dev;
+	uint32_t ctrl, i;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
+
+	dev = &rte_eth_devices[port];
+	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	if (idx != 0 && idx != 1)
+		return -EINVAL;
+
+	if (an >= 4)
+		return -EINVAL;
+
+	/* Set the PN */
+	pn = rte_cpu_to_be_32(pn);
+	IXGBE_WRITE_REG(hw, IXGBE_LSECRXPN(idx), pn);
+
+	/* Set the key */
+	for (i = 0; i < 4; i++) {
+		ctrl = (key[i * 4 + 0] <<  0) |
+		       (key[i * 4 + 1] <<  8) |
+		       (key[i * 4 + 2] << 16) |
+		       (key[i * 4 + 3] << 24);
+		IXGBE_WRITE_REG(hw, IXGBE_LSECRXKEY(idx, i), ctrl);
+	}
+
+	/* Set the AN and validate the SA */
+	ctrl = an | (1 << 2);
+	IXGBE_WRITE_REG(hw, IXGBE_LSECRXSA(idx), ctrl);
+
+	return 0;
+}
+
 RTE_PMD_REGISTER_PCI(net_ixgbe, rte_ixgbe_pmd.pci_drv);
 RTE_PMD_REGISTER_PCI_TABLE(net_ixgbe, pci_id_ixgbe_map);
 RTE_PMD_REGISTER_KMOD_DEP(net_ixgbe, "* igb_uio | uio_pci_generic | vfio");
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
index 80350c2..ffced1c 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.h
+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
@@ -43,6 +43,7 @@
 #define IXGBE_FLAG_NEED_LINK_UPDATE (uint32_t)(1 << 0)
 #define IXGBE_FLAG_MAILBOX          (uint32_t)(1 << 1)
 #define IXGBE_FLAG_PHY_INTERRUPT    (uint32_t)(1 << 2)
+#define IXGBE_FLAG_MACSEC           (uint32_t)(1 << 3)
 
 /*
  * Defines that were not part of ixgbe_type.h as they are not used by the
@@ -130,6 +131,10 @@
 #define IXGBE_MISC_VEC_ID               RTE_INTR_VEC_ZERO_OFFSET
 #define IXGBE_RX_VEC_START              RTE_INTR_VEC_RXTX_OFFSET
 
+#define IXGBE_SECTX_MINSECIFG_MASK      0x0000000F
+
+#define IXGBE_MACSEC_PNTHRSH            0xFFFFFE00
+
 /*
  * Information about the fdir mode.
  */
@@ -265,11 +270,44 @@ struct ixgbe_filter_info {
 };
 
 /*
+ * Statistics counters collected by the MACsec
+ */
+struct ixgbe_macsec_stats {
+	/* TX port statistics */
+	uint64_t out_pkts_untagged;
+	uint64_t out_pkts_encrypted;
+	uint64_t out_pkts_protected;
+	uint64_t out_octets_encrypted;
+	uint64_t out_octets_protected;
+
+	/* RX port statistics */
+	uint64_t in_pkts_untagged;
+	uint64_t in_pkts_badtag;
+	uint64_t in_pkts_nosci;
+	uint64_t in_pkts_unknownsci;
+	uint64_t in_octets_decrypted;
+	uint64_t in_octets_validated;
+
+	/* RX SC statistics */
+	uint64_t in_pkts_unchecked;
+	uint64_t in_pkts_delayed;
+	uint64_t in_pkts_late;
+
+	/* RX SA statistics */
+	uint64_t in_pkts_ok;
+	uint64_t in_pkts_invalid;
+	uint64_t in_pkts_notvalid;
+	uint64_t in_pkts_unusedsa;
+	uint64_t in_pkts_notusingsa;
+};
+
+/*
  * Structure to store private data for each driver instance (for each port).
  */
 struct ixgbe_adapter {
 	struct ixgbe_hw             hw;
 	struct ixgbe_hw_stats       stats;
+	struct ixgbe_macsec_stats   macsec_stats;
 	struct ixgbe_hw_fdir_info   fdir;
 	struct ixgbe_interrupt      intr;
 	struct ixgbe_stat_mapping_registers stat_mappings;
@@ -300,6 +338,9 @@ struct ixgbe_adapter {
 #define IXGBE_DEV_PRIVATE_TO_STATS(adapter) \
 	(&((struct ixgbe_adapter *)adapter)->stats)
 
+#define IXGBE_DEV_PRIVATE_TO_MACSEC_STATS(adapter) \
+	(&((struct ixgbe_adapter *)adapter)->macsec_stats)
+
 #define IXGBE_DEV_PRIVATE_TO_INTR(adapter) \
 	(&((struct ixgbe_adapter *)adapter)->intr)
 
@@ -445,4 +486,8 @@ uint32_t ixgbe_convert_vm_rx_mask_to_val(uint16_t rx_mask, uint32_t orig_val);
 
 int ixgbe_fdir_ctrl_func(struct rte_eth_dev *dev,
 			enum rte_filter_op filter_op, void *arg);
+
+int ixgbe_disable_sec_tx_path_generic(struct ixgbe_hw *hw);
+
+int ixgbe_enable_sec_tx_path_generic(struct ixgbe_hw *hw);
 #endif /* _IXGBE_ETHDEV_H_ */
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index b2d9f45..db9fc18 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -79,12 +79,15 @@
 #include "base/ixgbe_common.h"
 #include "ixgbe_rxtx.h"
 
+#include "rte_pmd_ixgbe.h"
+
 /* Bit Mask to indicate what bits required for building TX context */
 #define IXGBE_TX_OFFLOAD_MASK (			 \
 		PKT_TX_VLAN_PKT |		 \
 		PKT_TX_IP_CKSUM |		 \
 		PKT_TX_L4_MASK |		 \
 		PKT_TX_TCP_SEG |		 \
+		PKT_TX_IXGBE_MACSEC |		 \
 		PKT_TX_OUTER_IP_CKSUM)
 
 #if 1
@@ -519,6 +522,8 @@ tx_desc_ol_flags_to_cmdtype(uint64_t ol_flags)
 		cmdtype |= IXGBE_ADVTXD_DCMD_TSE;
 	if (ol_flags & PKT_TX_OUTER_IP_CKSUM)
 		cmdtype |= (1 << IXGBE_ADVTXD_OUTERIPCS_SHIFT);
+	if (ol_flags & PKT_TX_IXGBE_MACSEC)
+		cmdtype |= IXGBE_ADVTXD_MAC_LINKSEC;
 	return cmdtype;
 }
 
diff --git a/drivers/net/ixgbe/rte_pmd_ixgbe.h b/drivers/net/ixgbe/rte_pmd_ixgbe.h
index c2fb826..0c28ea2 100644
--- a/drivers/net/ixgbe/rte_pmd_ixgbe.h
+++ b/drivers/net/ixgbe/rte_pmd_ixgbe.h
@@ -42,6 +42,28 @@
 #include <rte_ethdev.h>
 
 /**
+ * If these flags are advertised by the PMD, the NIC supports the MACsec
+ * offload. The incoming MACsec traffics can be offloaded transparently
+ * after the MACsec offload is configured correctly by the application.
+ * And the application can set the PKT_TX_IXGBE_MACSEC flag in mbufs to
+ * enable the MACsec offload for the packets to be transmitted.
+ */
+#define DEV_RX_OFFLOAD_IXGBE_MACSEC_STRIP	DEV_RX_OFFLOAD_RESERVED_0
+#define DEV_TX_OFFLOAD_IXGBE_MACSEC_INSERT	DEV_TX_OFFLOAD_RESERVED_0
+
+/**
+ * This event will occur when the PN counter in a MACsec connection
+ * reach the exhaustion threshold.
+ */
+#define RTE_ETH_EVENT_IXGBE_MACSEC		RTE_ETH_EVENT_RESERVED_0
+
+/**
+ * Offload the MACsec. This flag must be set by the application in mbuf
+ * to enable this offload feature for a packet to be transmitted.
+ */
+#define PKT_TX_IXGBE_MACSEC			PKT_TX_RESERVED_0
+
+/**
  * Set the VF MAC address.
  *
  * @param port
@@ -183,6 +205,106 @@ int
 rte_pmd_ixgbe_set_vf_vlan_stripq(uint8_t port, uint16_t vf, uint8_t on);
 
 /**
+ * Enable MACsec offload.
+ *
+ * @param port
+ *   The port identifier of the Ethernet device.
+ * @param en
+ *    1 - Enable encryption (encrypt and add integrity signature).
+ *    0 - Disable encryption (only add integrity signature).
+ * @param rp
+ *    1 - Enable replay protection.
+ *    0 - Disable replay protection.
+ * @return
+ *   - (0) if successful.
+ *   - (-ENODEV) if *port* invalid.
+ *   - (-ENOTSUP) if hardware doesn't support this feature.
+ */
+int rte_pmd_ixgbe_macsec_enable(uint8_t port, uint8_t en, uint8_t rp);
+
+/**
+ * Disable MACsec offload.
+ *
+ * @param port
+ *   The port identifier of the Ethernet device.
+ * @return
+ *   - (0) if successful.
+ *   - (-ENODEV) if *port* invalid.
+ *   - (-ENOTSUP) if hardware doesn't support this feature.
+ */
+int rte_pmd_ixgbe_macsec_disable(uint8_t port);
+
+/**
+ * Configure Tx SC (Secure Connection).
+ *
+ * @param port
+ *   The port identifier of the Ethernet device.
+ * @param mac
+ *   The MAC address on the local side.
+ * @return
+ *   - (0) if successful.
+ *   - (-ENODEV) if *port* invalid.
+ */
+int rte_pmd_ixgbe_macsec_config_txsc(uint8_t port, uint8_t *mac);
+
+/**
+ * Configure Rx SC (Secure Connection).
+ *
+ * @param port
+ *   The port identifier of the Ethernet device.
+ * @param mac
+ *   The MAC address on the remote side.
+ * @param pi
+ *   The PI (port identifier) on the remote side.
+ * @return
+ *   - (0) if successful.
+ *   - (-ENODEV) if *port* invalid.
+ */
+int rte_pmd_ixgbe_macsec_config_rxsc(uint8_t port, uint8_t *mac, uint16_t pi);
+
+/**
+ * Enable Tx SA (Secure Association).
+ *
+ * @param port
+ *   The port identifier of the Ethernet device.
+ * @param idx
+ *   The SA to be enabled (0 or 1).
+ * @param an
+ *   The association number on the local side.
+ * @param pn
+ *   The packet number on the local side.
+ * @param key
+ *   The key on the local side.
+ * @return
+ *   - (0) if successful.
+ *   - (-ENODEV) if *port* invalid.
+ *   - (-EINVAL) if bad parameter.
+ */
+int rte_pmd_ixgbe_macsec_select_txsa(uint8_t port, uint8_t idx, uint8_t an,
+		uint32_t pn, uint8_t *key);
+
+/**
+ * Enable Rx SA (Secure Association).
+ *
+ * @param port
+ *   The port identifier of the Ethernet device.
+ * @param idx
+ *   The SA to be enabled (0 or 1)
+ * @param an
+ *   The association number on the remote side.
+ * @param pn
+ *   The packet number on the remote side.
+ * @param key
+ *   The key on the remote side.
+ * @return
+ *   - (0) if successful.
+ *   - (-ENODEV) if *port* invalid.
+ *   - (-EINVAL) if bad parameter.
+ */
+int rte_pmd_ixgbe_macsec_select_rxsa(uint8_t port, uint8_t idx, uint8_t an,
+		uint32_t pn, uint8_t *key);
+
+/**
  * Response sent back to ixgbe driver from user app after callback
  */
 enum rte_pmd_ixgbe_mb_event_rsp {
diff --git a/drivers/net/ixgbe/rte_pmd_ixgbe_version.map b/drivers/net/ixgbe/rte_pmd_ixgbe_version.map
index 92434f3..6d68934 100644
--- a/drivers/net/ixgbe/rte_pmd_ixgbe_version.map
+++ b/drivers/net/ixgbe/rte_pmd_ixgbe_version.map
@@ -15,3 +15,14 @@ DPDK_16.11 {
 	rte_pmd_ixgbe_set_vf_vlan_insert;
 	rte_pmd_ixgbe_set_vf_vlan_stripq;
 } DPDK_2.0;
+
+DPDK_17.02 {
+	global:
+
+	rte_pmd_ixgbe_macsec_enable;
+	rte_pmd_ixgbe_macsec_disable;
+	rte_pmd_ixgbe_macsec_config_txsc;
+	rte_pmd_ixgbe_macsec_config_rxsc;
+	rte_pmd_ixgbe_macsec_select_txsa;
+	rte_pmd_ixgbe_macsec_select_rxsa;
+} DPDK_16.11;
-- 
2.7.4

^ permalink raw reply related

* [PATCH v5 3/8] ethdev: reserve capability flags for PMD-specific API
From: Tiwei Bie @ 2017-01-04  7:21 UTC (permalink / raw)
  To: dev
  Cc: adrien.mazarguil, wenzhuo.lu, john.mcnamara, olivier.matz,
	thomas.monjalon, konstantin.ananyev, helin.zhang, wei.dai,
	xiao.w.wang
In-Reply-To: <1483514502-32841-1-git-send-email-tiwei.bie@intel.com>

Reserve a Tx capability flag and a Rx capability flag, that can be
used by PMD to define its own capability flags when implementing the
PMD-specific API.

Suggested-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
 lib/librte_ether/rte_ethdev.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index d465825..8800b39 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -857,6 +857,7 @@ struct rte_eth_conf {
 #define DEV_RX_OFFLOAD_TCP_LRO     0x00000010
 #define DEV_RX_OFFLOAD_QINQ_STRIP  0x00000020
 #define DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM 0x00000040
+#define DEV_RX_OFFLOAD_RESERVED_0  0x00000080 /**< Used for PMD-specific API. */
 
 /**
  * TX offload capabilities of a device.
@@ -874,6 +875,7 @@ struct rte_eth_conf {
 #define DEV_TX_OFFLOAD_GRE_TNL_TSO      0x00000400    /**< Used for tunneling packet. */
 #define DEV_TX_OFFLOAD_IPIP_TNL_TSO     0x00000800    /**< Used for tunneling packet. */
 #define DEV_TX_OFFLOAD_GENEVE_TNL_TSO   0x00001000    /**< Used for tunneling packet. */
+#define DEV_TX_OFFLOAD_RESERVED_0  0x00002000 /**< Used for PMD-specific API. */
 
 /**
  * Ethernet device information
-- 
2.7.4

^ permalink raw reply related

* [PATCH v5 2/8] ethdev: reserve an event type for PMD-specific API
From: Tiwei Bie @ 2017-01-04  7:21 UTC (permalink / raw)
  To: dev
  Cc: adrien.mazarguil, wenzhuo.lu, john.mcnamara, olivier.matz,
	thomas.monjalon, konstantin.ananyev, helin.zhang, wei.dai,
	xiao.w.wang
In-Reply-To: <1483514502-32841-1-git-send-email-tiwei.bie@intel.com>

Reserve an event type, that can be used by PMD to define its own
event type when implementing the PMD-specific API.

Suggested-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
 lib/librte_ether/rte_ethdev.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index fb51754..d465825 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -3044,6 +3044,8 @@ enum rte_eth_event_type {
 	RTE_ETH_EVENT_INTR_RESET,
 			/**< reset interrupt event, sent to VF on PF reset */
 	RTE_ETH_EVENT_VF_MBOX,  /**< message from the VF received by PF */
+	RTE_ETH_EVENT_RESERVED_0,
+			/**< reserved event type for PMD-specific API */
 	RTE_ETH_EVENT_MAX       /**< max value of this enum */
 };
 
-- 
2.7.4

^ permalink raw reply related

* [PATCH v5 1/8] mbuf: reserve a Tx offload flag for PMD-specific API
From: Tiwei Bie @ 2017-01-04  7:21 UTC (permalink / raw)
  To: dev
  Cc: adrien.mazarguil, wenzhuo.lu, john.mcnamara, olivier.matz,
	thomas.monjalon, konstantin.ananyev, helin.zhang, wei.dai,
	xiao.w.wang
In-Reply-To: <1483514502-32841-1-git-send-email-tiwei.bie@intel.com>

Reserve a Tx offload flag in mbuf, that can be used by PMD to define
its own Tx offload flag when implementing the PMD-specific API.

Suggested-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
 lib/librte_mbuf/rte_mbuf.c | 2 ++
 lib/librte_mbuf/rte_mbuf.h | 5 +++++
 2 files changed, 7 insertions(+)

diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
index 63f43c8..15c4f68 100644
--- a/lib/librte_mbuf/rte_mbuf.c
+++ b/lib/librte_mbuf/rte_mbuf.c
@@ -404,6 +404,7 @@ const char *rte_get_tx_ol_flag_name(uint64_t mask)
 	case PKT_TX_TUNNEL_GRE: return "PKT_TX_TUNNEL_GRE";
 	case PKT_TX_TUNNEL_IPIP: return "PKT_TX_TUNNEL_IPIP";
 	case PKT_TX_TUNNEL_GENEVE: return "PKT_TX_TUNNEL_GENEVE";
+	case PKT_TX_RESERVED_0: return "PKT_TX_RESERVED_0";
 	default: return NULL;
 	}
 }
@@ -434,6 +435,7 @@ rte_get_tx_ol_flag_list(uint64_t mask, char *buf, size_t buflen)
 		  "PKT_TX_TUNNEL_NONE" },
 		{ PKT_TX_TUNNEL_GENEVE, PKT_TX_TUNNEL_MASK,
 		  "PKT_TX_TUNNEL_NONE" },
+		{ PKT_TX_RESERVED_0, PKT_TX_RESERVED_0, NULL },
 	};
 	const char *name;
 	unsigned int i;
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index ead7c6e..6168a6d 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -182,6 +182,11 @@ extern "C" {
 /* add new TX flags here */
 
 /**
+ * Reserved Tx offload flag for PMD-specific API.
+ */
+#define PKT_TX_RESERVED_0     (0x1ULL << 44)
+
+/**
  * Bits 45:48 used for the tunnel type.
  * When doing Tx offload like TSO or checksum, the HW needs to configure the
  * tunnel type into the HW descriptors.
-- 
2.7.4

^ permalink raw reply related

* [PATCH v5 0/8] Add MACsec offload support for ixgbe
From: Tiwei Bie @ 2017-01-04  7:21 UTC (permalink / raw)
  To: dev
  Cc: adrien.mazarguil, wenzhuo.lu, john.mcnamara, olivier.matz,
	thomas.monjalon, konstantin.ananyev, helin.zhang, wei.dai,
	xiao.w.wang
In-Reply-To: <1482939691-34855-1-git-send-email-tiwei.bie@intel.com>

This patch set adds the MACsec offload support for ixgbe.
The testpmd is also updated to support MACsec cmds.

v2:
- Update the documents for testpmd;
- Update the release notes;
- Reuse the functions provided by base code;

v3:
- Add the missing parts of MACsec mbuf flag and reorganize the patch set;
- Add an ethdev event type for MACsec;
- Advertise the MACsec offload capabilities based on the mac type;
- Minor fixes and improvements;

v4:
- Reserve bits in mbuf and ethdev for PMD specific API;
- Use the reserved bits in PMD specific API;

v5:
- Add MACsec offload in the NIC feature list;
- Minor improvements on comments;

Tiwei Bie (8):
  mbuf: reserve a Tx offload flag for PMD-specific API
  ethdev: reserve an event type for PMD-specific API
  ethdev: reserve capability flags for PMD-specific API
  net/ixgbe: add MACsec offload support
  app/testpmd: add MACsec offload commands
  doc: add ixgbe specific APIs
  doc: update the release notes for the reserved flags
  doc: add MACsec offload into NIC feature list

 app/test-pmd/cmdline.c                      | 389 ++++++++++++++++++++++
 app/test-pmd/macfwd.c                       |   7 +
 app/test-pmd/macswap.c                      |   7 +
 app/test-pmd/testpmd.h                      |   2 +
 app/test-pmd/txonly.c                       |   7 +
 doc/guides/nics/features/default.ini        |   1 +
 doc/guides/nics/features/ixgbe.ini          |   1 +
 doc/guides/rel_notes/release_17_02.rst      |  18 ++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  32 ++
 drivers/net/ixgbe/ixgbe_ethdev.c            | 481 +++++++++++++++++++++++++++-
 drivers/net/ixgbe/ixgbe_ethdev.h            |  45 +++
 drivers/net/ixgbe/ixgbe_rxtx.c              |   5 +
 drivers/net/ixgbe/rte_pmd_ixgbe.h           | 122 +++++++
 drivers/net/ixgbe/rte_pmd_ixgbe_version.map |  11 +
 lib/librte_ether/rte_ethdev.h               |   4 +
 lib/librte_mbuf/rte_mbuf.c                  |   2 +
 lib/librte_mbuf/rte_mbuf.h                  |   5 +
 17 files changed, 1134 insertions(+), 5 deletions(-)

-- 
2.7.4

^ permalink raw reply

* [PATCH v3 1/3] dmaengine: xilinx_dma: Check for channel idle state before submitting dma descriptor
From: Kedareswara rao Appana @ 2017-01-04  6:54 UTC (permalink / raw)
  To: robh+dt, mark.rutland, dan.j.williams, vinod.koul, michal.simek,
	soren.brinkmann, appanad, moritz.fischer, laurent.pinchart, luis,
	Jose.Abreu
  Cc: dmaengine, linux-arm-kernel, linux-kernel, devicetree
In-Reply-To: <1483512847-25710-1-git-send-email-appanad@xilinx.com>

Add channel idle state to ensure that dma descriptor is not
submitted when VDMA engine is in progress.

Reviewed-by: Jose Abreu <joabreu@synopsys.com>
Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com>
---
Changes for v3:
---> None.
Changes for v2:
---> Add idle check in the reset as suggested by Jose Abreu
---> Removed xilinx_dma_is_running/xilinx_dma_is_idle checks
    in the driver and used common idle checks across the driver
    as suggested by Laurent Pinchart.

 drivers/dma/xilinx/xilinx_dma.c | 56 +++++++++++++----------------------------
 1 file changed, 17 insertions(+), 39 deletions(-)

diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index 8288fe4..be7eb41 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -321,6 +321,7 @@ struct xilinx_dma_tx_descriptor {
  * @cyclic: Check for cyclic transfers.
  * @genlock: Support genlock mode
  * @err: Channel has errors
+ * @idle: Check for channel idle
  * @tasklet: Cleanup work after irq
  * @config: Device configuration info
  * @flush_on_fsync: Flush on Frame sync
@@ -351,6 +352,7 @@ struct xilinx_dma_chan {
 	bool cyclic;
 	bool genlock;
 	bool err;
+	bool idle;
 	struct tasklet_struct tasklet;
 	struct xilinx_vdma_config config;
 	bool flush_on_fsync;
@@ -920,32 +922,6 @@ static enum dma_status xilinx_dma_tx_status(struct dma_chan *dchan,
 }
 
 /**
- * xilinx_dma_is_running - Check if DMA channel is running
- * @chan: Driver specific DMA channel
- *
- * Return: '1' if running, '0' if not.
- */
-static bool xilinx_dma_is_running(struct xilinx_dma_chan *chan)
-{
-	return !(dma_ctrl_read(chan, XILINX_DMA_REG_DMASR) &
-		 XILINX_DMA_DMASR_HALTED) &&
-		(dma_ctrl_read(chan, XILINX_DMA_REG_DMACR) &
-		 XILINX_DMA_DMACR_RUNSTOP);
-}
-
-/**
- * xilinx_dma_is_idle - Check if DMA channel is idle
- * @chan: Driver specific DMA channel
- *
- * Return: '1' if idle, '0' if not.
- */
-static bool xilinx_dma_is_idle(struct xilinx_dma_chan *chan)
-{
-	return dma_ctrl_read(chan, XILINX_DMA_REG_DMASR) &
-		XILINX_DMA_DMASR_IDLE;
-}
-
-/**
  * xilinx_dma_halt - Halt DMA channel
  * @chan: Driver specific DMA channel
  */
@@ -966,6 +942,7 @@ static void xilinx_dma_halt(struct xilinx_dma_chan *chan)
 			chan, dma_ctrl_read(chan, XILINX_DMA_REG_DMASR));
 		chan->err = true;
 	}
+	chan->idle = true;
 }
 
 /**
@@ -1007,6 +984,9 @@ static void xilinx_vdma_start_transfer(struct xilinx_dma_chan *chan)
 	if (chan->err)
 		return;
 
+	if (!chan->idle)
+		return;
+
 	if (list_empty(&chan->pending_list))
 		return;
 
@@ -1018,13 +998,6 @@ static void xilinx_vdma_start_transfer(struct xilinx_dma_chan *chan)
 	tail_segment = list_last_entry(&tail_desc->segments,
 				       struct xilinx_vdma_tx_segment, node);
 
-	/* If it is SG mode and hardware is busy, cannot submit */
-	if (chan->has_sg && xilinx_dma_is_running(chan) &&
-	    !xilinx_dma_is_idle(chan)) {
-		dev_dbg(chan->dev, "DMA controller still busy\n");
-		return;
-	}
-
 	/*
 	 * If hardware is idle, then all descriptors on the running lists are
 	 * done, start new transfers
@@ -1110,6 +1083,7 @@ static void xilinx_vdma_start_transfer(struct xilinx_dma_chan *chan)
 		vdma_desc_write(chan, XILINX_DMA_REG_VSIZE, last->hw.vsize);
 	}
 
+	chan->idle = false;
 	if (!chan->has_sg) {
 		list_del(&desc->node);
 		list_add_tail(&desc->node, &chan->active_list);
@@ -1136,6 +1110,9 @@ static void xilinx_cdma_start_transfer(struct xilinx_dma_chan *chan)
 	if (chan->err)
 		return;
 
+	if (!chan->idle)
+		return;
+
 	if (list_empty(&chan->pending_list))
 		return;
 
@@ -1181,6 +1158,7 @@ static void xilinx_cdma_start_transfer(struct xilinx_dma_chan *chan)
 
 	list_splice_tail_init(&chan->pending_list, &chan->active_list);
 	chan->desc_pendingcount = 0;
+	chan->idle = false;
 }
 
 /**
@@ -1196,15 +1174,11 @@ static void xilinx_dma_start_transfer(struct xilinx_dma_chan *chan)
 	if (chan->err)
 		return;
 
-	if (list_empty(&chan->pending_list))
+	if (!chan->idle)
 		return;
 
-	/* If it is SG mode and hardware is busy, cannot submit */
-	if (chan->has_sg && xilinx_dma_is_running(chan) &&
-	    !xilinx_dma_is_idle(chan)) {
-		dev_dbg(chan->dev, "DMA controller still busy\n");
+	if (list_empty(&chan->pending_list))
 		return;
-	}
 
 	head_desc = list_first_entry(&chan->pending_list,
 				     struct xilinx_dma_tx_descriptor, node);
@@ -1302,6 +1276,7 @@ static void xilinx_dma_start_transfer(struct xilinx_dma_chan *chan)
 
 	list_splice_tail_init(&chan->pending_list, &chan->active_list);
 	chan->desc_pendingcount = 0;
+	chan->idle = false;
 }
 
 /**
@@ -1366,6 +1341,7 @@ static int xilinx_dma_reset(struct xilinx_dma_chan *chan)
 	}
 
 	chan->err = false;
+	chan->idle = true;
 
 	return err;
 }
@@ -1447,6 +1423,7 @@ static irqreturn_t xilinx_dma_irq_handler(int irq, void *data)
 	if (status & XILINX_DMA_DMASR_FRM_CNT_IRQ) {
 		spin_lock(&chan->lock);
 		xilinx_dma_complete_descriptor(chan);
+		chan->idle = true;
 		chan->start_transfer(chan);
 		spin_unlock(&chan->lock);
 	}
@@ -2327,6 +2304,7 @@ static int xilinx_dma_chan_probe(struct xilinx_dma_device *xdev,
 	chan->has_sg = xdev->has_sg;
 	chan->desc_pendingcount = 0x0;
 	chan->ext_addr = xdev->ext_addr;
+	chan->idle = true;
 
 	spin_lock_init(&chan->lock);
 	INIT_LIST_HEAD(&chan->pending_list);
-- 
2.1.2

^ permalink raw reply related

* adding 32 bit compatibility layer in custom kernel module
From: anish singh @ 2017-01-04  7:26 UTC (permalink / raw)
  To: kernelnewbies
In-Reply-To: <CAHt1LmZhgoo-RC6dE6hynx_SZQ-iMNB2NuSxJPjL-ca5pPWnXA@mail.gmail.com>

On Tue, Jan 3, 2017 at 10:59 PM, Pradeepa Kumar <cdpradeepa@gmail.com>
wrote:

> my app is crashing as it is trying iterate cmsghdrs
> it got after call to recvmsg();
> To give some context
> below is the flow
>
> 32 bit app  <--> my kernel module <--> 64 bit app
> my kernel module implements new protocol and
> provides 'struct proto_ops'
> it never calls __sys_socket* calls.
> once 32 bit app calls recvmsg(),
> kernel module gets required data from another 64 bit app
> via netlink msg.
> issue here is the struct msghdr and strcu cmsgdr sizes
> and members are of different lenghts.
> so kernel module needs to give data to 32 bit app
> in proper struct which are of correct sizes in 32 bit mode.
> so kernel module needs a way to figure out
> it is 32 bit app that made recvmsg() call and process accordingly.
> i dont see compat_recv in struct proto_ops
>

Don't top post.
I suggest sending mail to netdev kernel mailing list
with all the relevant information as I doubt if networking
layer in kernel doesn't handle 32/64 bit properly.

>
>
>
> On Wed, Jan 4, 2017 at 12:18 PM, Anish Kumar <anish198519851985@gmail.com>
> wrote:
>
>>
>>
>> On Jan 3, 2017, at 10:03 PM, Pradeepa Kumar <cdpradeepa@gmail.com> wrote:
>>
>> Please see inline below
>>
>> On Wed, Jan 4, 2017 at 11:07 AM, Anish Kumar <anish198519851985@gmail.com
>> > wrote:
>>
>>>
>>>
>>> On Jan 3, 2017, at 8:04 PM, Pradeepa Kumar <cdpradeepa@gmail.com> wrote:
>>>
>>> Hi experts
>>>
>>> down votefavorite
>>> <http://stackoverflow.com/questions/41455943/adding-32-bit-compatibility-layer-in-custom-kernel-module#>
>>>
>>> in my 64 bit kernel, I have a custom kernel module providing new
>>> protocol and providing socket system calls.
>>>
>>> it works fine when 64 bit app runs.
>>>
>>> i am seeing issues when 32 bit app runs (for exp recvmsg() call does not
>>> work if msg has cmsghdrs as struct size of cmsghdr is different in 32 bit
>>> and 64 bit).
>>>
>>> This is because my custom kernel module does not have 32 bit
>>> compatibility layer ( but linux kernel has this in compat.c etc).
>>>
>>>    -
>>>
>>>    is it simple to add compatibility layer to my custom kernel module
>>>
>>>
>>>    All you have to do is add compat_ioctl call to your driver.
>>>
>>> my kernel module provides recvmsg() but i dont see any 'compat_recvmsg'
>> in struct proto_ops.
>>
>>
>> Try to wrap your text in 80 characters.
>>
>> Your application is crashing? Right?
>> Have you figured where it is crashing exactly?
>>
>> I understood that your application is crashing as
>> It not able to function with 64 bit kernel.
>>
>> This happens because of the data being passed
>> to kernel is not in the right format. You need to
>> look at what you are passing to the driver and
>> if needed write the compat_ioctl callback in your
>> driver fops.
>>
>> had there been  'compat_recvmsg' it would have been possible for me to
>> define recvmsg() for 32 bit apps.
>> please let me know if i am missing anything
>>
>>
>>>    -
>>>
>>>    how do i do this (any links ?)
>>>
>>>
>>>    You can see many drivers supports compat call.
>>>
>>> Thanks
>>>
>>> _______________________________________________
>>> Kernelnewbies mailing list
>>> Kernelnewbies at kernelnewbies.org
>>> https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20170103/b94db475/attachment-0001.html 

^ permalink raw reply

* [PATCH v4 07/12] mmc: sdhci-xenon: Add Marvell Xenon SDHC core functionality
From: Adrian Hunter @ 2017-01-04  7:26 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <f48940626343ca9cf68b32b7324b243e76a11960.1481651244.git-series.gregory.clement@free-electrons.com>

On 13/12/16 19:48, Gregory CLEMENT wrote:
> From: Hu Ziji <huziji@marvell.com>
> 
> Add Xenon eMMC/SD/SDIO host controller core functionality.
> Add Xenon specific intialization process.
> Add Xenon specific mmc_host_ops APIs.
> Add Xenon specific register definitions.
> 
> Add CONFIG_MMC_SDHCI_XENON support in drivers/mmc/host/Kconfig.
> 
> Marvell Xenon SDHC conforms to SD Physical Layer Specification
> Version 3.01 and is designed according to the guidelines provided
> in the SD Host Controller Standard Specification Version 3.00.
> 
> Signed-off-by: Hu Ziji <huziji@marvell.com>
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> ---
>  MAINTAINERS                    |   1 +-
>  drivers/mmc/host/Kconfig       |   9 +-
>  drivers/mmc/host/Makefile      |   3 +-
>  drivers/mmc/host/sdhci-xenon.c | 612 ++++++++++++++++++++++++++++++++++-
>  drivers/mmc/host/sdhci-xenon.h |  70 ++++-
>  5 files changed, 695 insertions(+)
>  create mode 100644 drivers/mmc/host/sdhci-xenon.c
>  create mode 100644 drivers/mmc/host/sdhci-xenon.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 850a0afb0c8d..bb33286aeb48 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7608,6 +7608,7 @@ MARVELL XENON MMC/SD/SDIO HOST CONTROLLER DRIVER
>  M:	Ziji Hu <huziji@marvell.com>
>  L:	linux-mmc at vger.kernel.org
>  S:	Supported
> +F:	drivers/mmc/host/sdhci-xenon*
>  F:	Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
>  
>  MATROX FRAMEBUFFER DRIVER
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 5274f503a39a..85a53623526a 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -798,3 +798,12 @@ config MMC_SDHCI_BRCMSTB
>  	  Broadcom STB SoCs.
>  
>  	  If unsure, say Y.
> +
> +config MMC_SDHCI_XENON
> +	tristate "Marvell Xenon eMMC/SD/SDIO SDHCI driver"
> +	depends on MMC_SDHCI && MMC_SDHCI_PLTFM
> +	help
> +	  This selects Marvell Xenon eMMC/SD/SDIO SDHCI.
> +	  If you have a machine with integrated Marvell Xenon SDHC IP,
> +	  say Y or M here.
> +	  If unsure, say N.
> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
> index e2bdaaf43184..75eaf743486c 100644
> --- a/drivers/mmc/host/Makefile
> +++ b/drivers/mmc/host/Makefile
> @@ -80,3 +80,6 @@ obj-$(CONFIG_MMC_SDHCI_BRCMSTB)		+= sdhci-brcmstb.o
>  ifeq ($(CONFIG_CB710_DEBUG),y)
>  	CFLAGS-cb710-mmc	+= -DDEBUG
>  endif
> +
> +obj-$(CONFIG_MMC_SDHCI_XENON)	+= sdhci-xenon-driver.o
> +sdhci-xenon-driver-y		+= sdhci-xenon.o
> diff --git a/drivers/mmc/host/sdhci-xenon.c b/drivers/mmc/host/sdhci-xenon.c
> new file mode 100644
> index 000000000000..c71439fbc308
> --- /dev/null
> +++ b/drivers/mmc/host/sdhci-xenon.c
> @@ -0,0 +1,612 @@
> +/*
> + * Driver for Marvell Xenon SDHC as a platform device
> + *
> + * Copyright (C) 2016 Marvell, All Rights Reserved.
> + *
> + * Author:	Hu Ziji <huziji@marvell.com>
> + * Date:	2016-8-24
> + *
> + * 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.
> + *
> + * Inspired by Jisheng Zhang <jszhang@marvell.com>
> + * Special thanks to Video BG4 project team.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +
> +#include "sdhci-pltfm.h"
> +#include "sdhci-xenon.h"
> +
> +static int enable_xenon_internal_clk(struct sdhci_host *host)
> +{
> +	u32 reg;
> +	u8 timeout;
> +
> +	reg = sdhci_readl(host, SDHCI_CLOCK_CONTROL);
> +	reg |= SDHCI_CLOCK_INT_EN;
> +	sdhci_writel(host, reg, SDHCI_CLOCK_CONTROL);
> +	/* Wait max 20 ms */
> +	timeout = 20;
> +	while (!((reg = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
> +			& SDHCI_CLOCK_INT_STABLE)) {
> +		if (timeout == 0) {
> +			pr_err("%s: Internal clock never stabilised.\n",
> +			       mmc_hostname(host->mmc));
> +			return -ETIMEDOUT;
> +		}
> +		timeout--;
> +		mdelay(1);
> +	}
> +
> +	return 0;
> +}
> +
> +/* Set SDCLK-off-while-idle */
> +static void xenon_set_sdclk_off_idle(struct sdhci_host *host,
> +				     unsigned char sdhc_id, bool enable)
> +{
> +	u32 reg;
> +	u32 mask;
> +
> +	reg = sdhci_readl(host, SDHCI_SYS_OP_CTRL);
> +	/* Get the bit shift basing on the SDHC index */
> +	mask = (0x1 << (SDHCI_SDCLK_IDLEOFF_ENABLE_SHIFT + sdhc_id));
> +	if (enable)
> +		reg |= mask;
> +	else
> +		reg &= ~mask;
> +
> +	sdhci_writel(host, reg, SDHCI_SYS_OP_CTRL);
> +}
> +
> +/* Enable/Disable the Auto Clock Gating function */
> +static void xenon_set_acg(struct sdhci_host *host, bool enable)
> +{
> +	u32 reg;
> +
> +	reg = sdhci_readl(host, SDHCI_SYS_OP_CTRL);
> +	if (enable)
> +		reg &= ~SDHCI_AUTO_CLKGATE_DISABLE_MASK;
> +	else
> +		reg |= SDHCI_AUTO_CLKGATE_DISABLE_MASK;
> +	sdhci_writel(host, reg, SDHCI_SYS_OP_CTRL);
> +}
> +
> +/* Enable this SDHC */
> +static void xenon_enable_sdhc(struct sdhci_host *host,
> +			      unsigned char sdhc_id)
> +{
> +	u32 reg;
> +
> +	reg = sdhci_readl(host, SDHCI_SYS_OP_CTRL);
> +	reg |= (BIT(sdhc_id) << SDHCI_SLOT_ENABLE_SHIFT);
> +	sdhci_writel(host, reg, SDHCI_SYS_OP_CTRL);
> +
> +	/*
> +	 * Manually set the flag which all the card types require,
> +	 * including SD, eMMC, SDIO
> +	 */
> +	host->mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY;
> +}
> +
> +/* Disable this SDHC */
> +static void xenon_disable_sdhc(struct sdhci_host *host,
> +			       unsigned char sdhc_id)
> +{
> +	u32 reg;
> +
> +	reg = sdhci_readl(host, SDHCI_SYS_OP_CTRL);
> +	reg &= ~(BIT(sdhc_id) << SDHCI_SLOT_ENABLE_SHIFT);
> +	sdhci_writel(host, reg, SDHCI_SYS_OP_CTRL);
> +}
> +
> +/* Enable Parallel Transfer Mode */
> +static void xenon_enable_sdhc_parallel_tran(struct sdhci_host *host,
> +					    unsigned char sdhc_id)
> +{
> +	u32 reg;
> +
> +	reg = sdhci_readl(host, SDHCI_SYS_EXT_OP_CTRL);
> +	reg |= BIT(sdhc_id);
> +	sdhci_writel(host, reg, SDHCI_SYS_EXT_OP_CTRL);
> +}
> +
> +static void xenon_sdhc_tuning_setup(struct sdhci_host *host)
> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> +	u32 reg;
> +
> +	/* Disable the Re-Tuning Request functionality */
> +	reg = sdhci_readl(host, SDHCI_SLOT_RETUNING_REQ_CTRL);
> +	reg &= ~SDHCI_RETUNING_COMPATIBLE;
> +	sdhci_writel(host, reg, SDHCI_SLOT_RETUNING_REQ_CTRL);
> +
> +	/* Disable the Re-tuning Event Signal Enable */
> +	reg = sdhci_readl(host, SDHCI_SIGNAL_ENABLE);
> +	reg &= ~SDHCI_INT_RETUNE;
> +	sdhci_writel(host, reg, SDHCI_SIGNAL_ENABLE);
> +
> +	/* Force to use Tuning Mode 1 */
> +	host->tuning_mode = SDHCI_TUNING_MODE_1;
> +	/* Set re-tuning period */
> +	host->tuning_count = 1 << (priv->tuning_count - 1);

host->tuning_mode and host->tuning_count get overwritten in
sdhci_setup_host() called by sdhci_add_host()

> +}
> +
> +/*
> + * Operations inside struct sdhci_ops
> + */
> +/* Recover the Register Setting cleared during SOFTWARE_RESET_ALL */
> +static void sdhci_xenon_reset_exit(struct sdhci_host *host,
> +				   unsigned char sdhc_id, u8 mask)
> +{
> +	/* Only SOFTWARE RESET ALL will clear the register setting */
> +	if (!(mask & SDHCI_RESET_ALL))
> +		return;
> +
> +	/* Disable tuning request and auto-retuning again */
> +	xenon_sdhc_tuning_setup(host);
> +
> +	xenon_set_acg(host, true);
> +
> +	xenon_set_sdclk_off_idle(host, sdhc_id, false);
> +}
> +
> +static void sdhci_xenon_reset(struct sdhci_host *host, u8 mask)
> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> +
> +	sdhci_reset(host, mask);
> +	sdhci_xenon_reset_exit(host, priv->sdhc_id, mask);
> +}
> +
> +/*
> + * Xenon defines different values for HS200 and HS400
> + * in Host_Control_2
> + */
> +static void xenon_set_uhs_signaling(struct sdhci_host *host,
> +				    unsigned int timing)
> +{
> +	u16 ctrl_2;
> +
> +	ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> +	/* Select Bus Speed Mode for host */
> +	ctrl_2 &= ~SDHCI_CTRL_UHS_MASK;
> +	if (timing == MMC_TIMING_MMC_HS200)
> +		ctrl_2 |= SDHCI_XENON_CTRL_HS200;
> +	else if (timing == MMC_TIMING_UHS_SDR104)
> +		ctrl_2 |= SDHCI_CTRL_UHS_SDR104;
> +	else if (timing == MMC_TIMING_UHS_SDR12)
> +		ctrl_2 |= SDHCI_CTRL_UHS_SDR12;
> +	else if (timing == MMC_TIMING_UHS_SDR25)
> +		ctrl_2 |= SDHCI_CTRL_UHS_SDR25;
> +	else if (timing == MMC_TIMING_UHS_SDR50)
> +		ctrl_2 |= SDHCI_CTRL_UHS_SDR50;
> +	else if ((timing == MMC_TIMING_UHS_DDR50) ||
> +		 (timing == MMC_TIMING_MMC_DDR52))
> +		ctrl_2 |= SDHCI_CTRL_UHS_DDR50;
> +	else if (timing == MMC_TIMING_MMC_HS400)
> +		ctrl_2 |= SDHCI_XENON_CTRL_HS400;
> +	sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
> +}
> +
> +static const struct sdhci_ops sdhci_xenon_ops = {
> +	.set_clock		= sdhci_set_clock,
> +	.set_bus_width		= sdhci_set_bus_width,
> +	.reset			= sdhci_xenon_reset,
> +	.set_uhs_signaling	= xenon_set_uhs_signaling,
> +	.get_max_clock		= sdhci_pltfm_clk_get_max_clock,
> +};
> +
> +static const struct sdhci_pltfm_data sdhci_xenon_pdata = {
> +	.ops = &sdhci_xenon_ops,
> +	.quirks = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC |
> +		  SDHCI_QUIRK_NO_SIMULT_VDD_AND_POWER |
> +		  SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
> +};
> +
> +/*
> + * Xenon Specific Operations in mmc_host_ops
> + */
> +static void xenon_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
> +{
> +	struct sdhci_host *host = mmc_priv(mmc);
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> +	unsigned long flags;
> +	u32 reg;
> +
> +	/*
> +	 * HS400/HS200/eMMC HS doesn't have Preset Value register.
> +	 * However, sdhci_set_ios will read HS400/HS200 Preset register.
> +	 * Disable Preset Value register for HS400/HS200.
> +	 * eMMC HS with preset_enabled set will trigger a bug in
> +	 * get_preset_value().
> +	 */
> +	spin_lock_irqsave(&host->lock, flags);
> +	if ((ios->timing == MMC_TIMING_MMC_HS400) ||
> +	    (ios->timing == MMC_TIMING_MMC_HS200) ||
> +	    (ios->timing == MMC_TIMING_MMC_HS)) {
> +		host->preset_enabled = false;
> +		host->quirks2 |= SDHCI_QUIRK2_PRESET_VALUE_BROKEN;
> +
> +		reg = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> +		reg &= ~SDHCI_CTRL_PRESET_VAL_ENABLE;
> +		sdhci_writew(host, reg, SDHCI_HOST_CONTROL2);
> +	} else {
> +		host->quirks2 &= ~SDHCI_QUIRK2_PRESET_VALUE_BROKEN;
> +	}
> +	spin_unlock_irqrestore(&host->lock, flags);

At some point we will have to get rid of SDHCI_QUIRK2_PRESET_VALUE_BROKEN
and add a callback instead.

> +
> +	sdhci_set_ios(mmc, ios);
> +
> +	if (host->clock > SDHCI_DEFAULT_SDCLK_FREQ) {
> +		spin_lock_irqsave(&host->lock, flags);
> +		xenon_set_sdclk_off_idle(host, priv->sdhc_id, true);
> +		spin_unlock_irqrestore(&host->lock, flags);
> +	}
> +}
> +
> +static int xenon_emmc_signal_voltage_switch(struct mmc_host *mmc,
> +					    struct mmc_ios *ios)
> +{
> +	unsigned char voltage = ios->signal_voltage;
> +	struct sdhci_host *host = mmc_priv(mmc);
> +	unsigned char voltage_code;
> +	u32 ctrl;
> +
> +	if ((voltage == MMC_SIGNAL_VOLTAGE_330) ||
> +	    (voltage == MMC_SIGNAL_VOLTAGE_180)) {
> +		if (voltage == MMC_SIGNAL_VOLTAGE_330)
> +			voltage_code = SDHCI_EMMC_VCCQ_3_3V;
> +		else if (voltage == MMC_SIGNAL_VOLTAGE_180)
> +			voltage_code = SDHCI_EMMC_VCCQ_1_8V;
> +
> +		/*
> +		 * This host is for eMMC, XENON self-defined
> +		 * eMMC control register should be accessed
> +		 * instead of Host Control 2
> +		 */
> +		ctrl = sdhci_readl(host, SDHCI_SLOT_EMMC_CTRL);
> +		ctrl &= ~SDHCI_EMMC_VCCQ_MASK;
> +		ctrl |= voltage_code;
> +		sdhci_writel(host, ctrl, SDHCI_SLOT_EMMC_CTRL);
> +
> +		/* There is no standard to determine this waiting period */
> +		usleep_range(1000, 2000);
> +
> +		/* Check whether io voltage switch is done */
> +		ctrl = sdhci_readl(host, SDHCI_SLOT_EMMC_CTRL);
> +		ctrl &= SDHCI_EMMC_VCCQ_MASK;
> +		/*
> +		 * This bit is set only when regulator feeds back
> +		 * the voltage switch results to Xenon SDHC.
> +		 * However, in actaul implementation, regulator might not
> +		 * provide this feedback.
> +		 * Thus we shall not rely on this bit to determine
> +		 * if switch failed.
> +		 * If the bit is not set, just throw a message.
> +		 * Besides, error code should not be returned.
> +		 */
> +		if (ctrl != voltage_code)
> +			dev_info(mmc_dev(mmc), "fail to detect eMMC signal voltage stable\n");
> +		return 0;
> +	}
> +
> +	dev_err(mmc_dev(mmc), "Unsupported signal voltage: %d\n", voltage);
> +	return -EINVAL;
> +}
> +
> +static int xenon_start_signal_voltage_switch(struct mmc_host *mmc,
> +					     struct mmc_ios *ios)
> +{
> +	struct sdhci_host *host = mmc_priv(mmc);
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> +
> +	/*
> +	 * Before SD/SDIO set signal voltage, SD bus clock should be
> +	 * disabled. However, sdhci_set_clock will also disable the Internal
> +	 * clock in mmc_set_signal_voltage().
> +	 * If Internal clock is disabled, the 3.3V/1.8V bit can not be updated.
> +	 * Thus here manually enable internal clock.
> +	 *
> +	 * After switch completes, it is unnecessary to disable internal clock,
> +	 * since keeping internal clock active obeys SD spec.
> +	 */
> +	enable_xenon_internal_clk(host);

We could try the attached patch.

> +
> +	if (priv->init_card_type == MMC_TYPE_MMC)
> +		return xenon_emmc_signal_voltage_switch(mmc, ios);
> +
> +	return sdhci_start_signal_voltage_switch(mmc, ios);
> +}
> +
> +/*
> + * Update card type.
> + * priv->init_card_type will be used in PHY timing adjustment.
> + */
> +static void xenon_init_card(struct mmc_host *mmc, struct mmc_card *card)
> +{
> +	struct sdhci_host *host = mmc_priv(mmc);
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> +
> +	/* Update card type*/
> +	priv->init_card_type = card->type;
> +}
> +
> +static int xenon_execute_tuning(struct mmc_host *mmc, u32 opcode)
> +{
> +	struct sdhci_host *host = mmc_priv(mmc);
> +
> +	if (host->timing == MMC_TIMING_UHS_DDR50)
> +		return 0;
> +
> +	return sdhci_execute_tuning(mmc, opcode);
> +}
> +
> +static void xenon_enable_sdio_irq(struct mmc_host *mmc, int enable)
> +{
> +	struct sdhci_host *host = mmc_priv(mmc);
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> +	u32 reg;
> +	u8 sdhc_id = priv->sdhc_id;
> +
> +	sdhci_enable_sdio_irq(mmc, enable);
> +
> +	if (enable) {
> +		/*
> +		 * Set SDIO Card Inserted indication
> +		 * to enable detecting SDIO async irq.
> +		 */
> +		reg = sdhci_readl(host, SDHCI_SYS_CFG_INFO);
> +		reg |= (1 << (sdhc_id + SDHCI_SLOT_TYPE_SDIO_SHIFT));
> +		sdhci_writel(host, reg, SDHCI_SYS_CFG_INFO);
> +	} else {
> +		/* Clear SDIO Card Inserted indication */
> +		reg = sdhci_readl(host, SDHCI_SYS_CFG_INFO);
> +		reg &= ~(1 << (sdhc_id + SDHCI_SLOT_TYPE_SDIO_SHIFT));
> +		sdhci_writel(host, reg, SDHCI_SYS_CFG_INFO);
> +	}
> +}
> +
> +static void xenon_replace_mmc_host_ops(struct sdhci_host *host)
> +{
> +	host->mmc_host_ops.set_ios = xenon_set_ios;
> +	host->mmc_host_ops.start_signal_voltage_switch =
> +			xenon_start_signal_voltage_switch;
> +	host->mmc_host_ops.init_card = xenon_init_card;
> +	host->mmc_host_ops.execute_tuning = xenon_execute_tuning;
> +	host->mmc_host_ops.enable_sdio_irq = xenon_enable_sdio_irq;
> +}
> +
> +/*
> + * Parse child node in Xenon DT.
> + * Search for the following item(s):
> + * - eMMC card type
> + */
> +static int xenon_child_node_of_parse(struct platform_device *pdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	struct sdhci_host *host = platform_get_drvdata(pdev);
> +	struct mmc_host *mmc = host->mmc;
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> +	struct device_node *child;
> +	int nr_child;
> +
> +	priv->init_card_type = SDHCI_CARD_TYPE_UNKNOWN;
> +
> +	nr_child = of_get_child_count(np);
> +	if (!nr_child)
> +		return 0;
> +
> +	for_each_child_of_node(np, child) {
> +		if (of_device_is_compatible(child, "mmc-card"))	{
> +			priv->init_card_type = MMC_TYPE_MMC;
> +			mmc->caps |= MMC_CAP_NONREMOVABLE;
> +
> +			/*
> +			 * Force to clear BUS_TEST to
> +			 * skip bus_test_pre and bus_test_post
> +			 */
> +			mmc->caps &= ~MMC_CAP_BUS_WIDTH_TEST;
> +			mmc->caps2 |= MMC_CAP2_HC_ERASE_SZ |
> +				      MMC_CAP2_PACKED_CMD |
> +				      MMC_CAP2_NO_SD |
> +				      MMC_CAP2_NO_SDIO;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int xenon_probe_dt(struct platform_device *pdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	struct sdhci_host *host = platform_get_drvdata(pdev);
> +	struct mmc_host *mmc = host->mmc;
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> +	int err;
> +	u32 sdhc_id, nr_sdhc;
> +	u32 tuning_count;
> +
> +	/* Standard MMC property */
> +	err = mmc_of_parse(mmc);
> +	if (err)
> +		return err;
> +
> +	/* Standard SDHCI property */
> +	sdhci_get_of_property(pdev);
> +
> +	/*
> +	 * Xenon Specific property:
> +	 * init_card_type: check whether this SDHC is for eMMC
> +	 * sdhc-id: the index of current SDHC.
> +	 *	    Refer to SDHCI_SYS_CFG_INFO register
> +	 * tun-count: the interval between re-tuning
> +	 */
> +	/* Parse child node, including checking emmc type */
> +	err = xenon_child_node_of_parse(pdev);
> +	if (err)
> +		return err;
> +
> +	priv->sdhc_id = 0x0;
> +	if (!of_property_read_u32(np, "marvell,xenon-sdhc-id", &sdhc_id)) {
> +		nr_sdhc = sdhci_readl(host, SDHCI_SYS_CFG_INFO);
> +		nr_sdhc &= SDHCI_NR_SUPPORTED_SLOT_MASK;
> +		if (unlikely(sdhc_id > nr_sdhc)) {
> +			dev_err(mmc_dev(mmc), "SDHC Index %d exceeds Number of SDHCs %d\n",
> +				sdhc_id, nr_sdhc);
> +			return -EINVAL;
> +		}
> +	}
> +
> +	tuning_count = SDHCI_DEF_TUNING_COUNT;
> +	if (!of_property_read_u32(np, "marvell,xenon-tun-count",
> +				  &tuning_count)) {
> +		if (unlikely(tuning_count >= SDHCI_TMR_RETUN_NO_PRESENT)) {
> +			dev_err(mmc_dev(mmc), "Wrong Re-tuning Count. Set default value %d\n",
> +				SDHCI_DEF_TUNING_COUNT);
> +			tuning_count = SDHCI_DEF_TUNING_COUNT;
> +		}
> +	}
> +	priv->tuning_count = tuning_count;
> +
> +	return err;
> +}
> +
> +static int xenon_sdhc_probe(struct sdhci_host *host)
> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> +	u8 sdhc_id = priv->sdhc_id;
> +
> +	/* Enable SDHC */
> +	xenon_enable_sdhc(host, sdhc_id);
> +
> +	/* Enable ACG */
> +	xenon_set_acg(host, true);
> +
> +	/* Enable Parallel Transfer Mode */
> +	xenon_enable_sdhc_parallel_tran(host, sdhc_id);
> +
> +	/* Set tuning functionality of this SDHC */
> +	xenon_sdhc_tuning_setup(host);
> +
> +	return 0;
> +}
> +
> +static void xenon_sdhc_remove(struct sdhci_host *host)
> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> +	u8 sdhc_id = priv->sdhc_id;
> +
> +	/* disable SDHC */
> +	xenon_disable_sdhc(host, sdhc_id);
> +}
> +
> +static int sdhci_xenon_probe(struct platform_device *pdev)
> +{
> +	struct sdhci_pltfm_host *pltfm_host;
> +	struct sdhci_host *host;
> +	struct sdhci_xenon_priv *priv;
> +	int err;
> +
> +	host = sdhci_pltfm_init(pdev, &sdhci_xenon_pdata,
> +				sizeof(struct sdhci_xenon_priv));
> +	if (IS_ERR(host))
> +		return PTR_ERR(host);
> +
> +	pltfm_host = sdhci_priv(host);
> +	priv = sdhci_pltfm_priv(pltfm_host);
> +
> +	xenon_set_acg(host, false);
> +
> +	/*
> +	 * Link Xenon specific mmc_host_ops function,
> +	 * to replace standard ones in sdhci_ops.
> +	 */
> +	xenon_replace_mmc_host_ops(host);
> +
> +	pltfm_host->clk = devm_clk_get(&pdev->dev, "core");
> +	if (IS_ERR(pltfm_host->clk)) {
> +		err = PTR_ERR(pltfm_host->clk);
> +		dev_err(&pdev->dev, "Failed to setup input clk: %d\n", err);
> +		goto free_pltfm;
> +	}
> +	err = clk_prepare_enable(pltfm_host->clk);
> +	if (err)
> +		goto free_pltfm;
> +
> +	err = xenon_probe_dt(pdev);
> +	if (err)
> +		goto err_clk;
> +
> +	err = xenon_sdhc_probe(host);
> +	if (err)
> +		goto err_clk;
> +
> +	err = sdhci_add_host(host);
> +	if (err)
> +		goto remove_sdhc;
> +
> +	return 0;
> +
> +remove_sdhc:
> +	xenon_sdhc_remove(host);
> +err_clk:
> +	clk_disable_unprepare(pltfm_host->clk);
> +free_pltfm:
> +	sdhci_pltfm_free(pdev);
> +	return err;
> +}
> +
> +static int sdhci_xenon_remove(struct platform_device *pdev)
> +{
> +	struct sdhci_host *host = platform_get_drvdata(pdev);
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	int dead = (readl(host->ioaddr + SDHCI_INT_STATUS) == 0xFFFFFFFF);

This 'dead' check was originally for PCI I think.  Unless you know it makes
sense for your device, I would leave it out. i.e. just do
sdhci_remove_host(host, 0);

> +
> +	xenon_sdhc_remove(host);
> +
> +	sdhci_remove_host(host, dead);
> +
> +	clk_disable_unprepare(pltfm_host->clk);
> +
> +	sdhci_pltfm_free(pdev);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id sdhci_xenon_dt_ids[] = {
> +	{ .compatible = "marvell,armada-7000-sdhci",},
> +	{ .compatible = "marvell,armada-3700-sdhci",},
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, sdhci_xenon_dt_ids);
> +
> +static struct platform_driver sdhci_xenon_driver = {
> +	.driver	= {
> +		.name	= "xenon-sdhci",
> +		.of_match_table = sdhci_xenon_dt_ids,
> +		.pm = &sdhci_pltfm_pmops,
> +	},
> +	.probe	= sdhci_xenon_probe,
> +	.remove	= sdhci_xenon_remove,
> +};
> +
> +module_platform_driver(sdhci_xenon_driver);
> +
> +MODULE_DESCRIPTION("SDHCI platform driver for Marvell Xenon SDHC");
> +MODULE_AUTHOR("Hu Ziji <huziji@marvell.com>");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/mmc/host/sdhci-xenon.h b/drivers/mmc/host/sdhci-xenon.h
> new file mode 100644
> index 000000000000..d50cd663a265
> --- /dev/null
> +++ b/drivers/mmc/host/sdhci-xenon.h
> @@ -0,0 +1,70 @@
> +/*
> + * Copyright (C) 2016 Marvell, All Rights Reserved.
> + *
> + * Author:	Hu Ziji <huziji@marvell.com>
> + * Date:	2016-8-24
> + *
> + * 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.
> + */
> +#ifndef SDHCI_XENON_H_
> +#define SDHCI_XENON_H_
> +
> +

Double blank line

> +/* Register Offset of Xenon SDHC self-defined register */
> +#define SDHCI_SYS_CFG_INFO			0x0104

A lot of these defines look like they could be just in sdhci-xenon.c or
sdhci-xenon-phy.c.  It is also a little odd that they are prefixed by
"SDHCI" because they are not standard.  "XENON" would be better.

> +#define SDHCI_SLOT_TYPE_SDIO_SHIFT		24
> +#define SDHCI_NR_SUPPORTED_SLOT_MASK		0x7
> +
> +#define SDHCI_SYS_OP_CTRL			0x0108
> +#define SDHCI_AUTO_CLKGATE_DISABLE_MASK		BIT(20)
> +#define SDHCI_SDCLK_IDLEOFF_ENABLE_SHIFT	8
> +#define SDHCI_SLOT_ENABLE_SHIFT			0
> +
> +#define SDHCI_SYS_EXT_OP_CTRL			0x010C
> +
> +#define SDHCI_SLOT_EMMC_CTRL			0x0130
> +#define SDHCI_EMMC_VCCQ_MASK			0x3
> +#define SDHCI_EMMC_VCCQ_1_8V			0x1
> +#define SDHCI_EMMC_VCCQ_3_3V			0x3
> +
> +#define SDHCI_SLOT_RETUNING_REQ_CTRL		0x0144
> +/* retuning compatible */
> +#define SDHCI_RETUNING_COMPATIBLE		0x1
> +
> +/* Tuning Parameter */
> +#define SDHCI_TMR_RETUN_NO_PRESENT		0xF
> +#define SDHCI_DEF_TUNING_COUNT			0x9
> +
> +#define SDHCI_DEFAULT_SDCLK_FREQ		(400000)

Unnecessary ()

> +
> +/* Xenon specific Mode Select value */
> +#define SDHCI_XENON_CTRL_HS200			0x5
> +#define SDHCI_XENON_CTRL_HS400			0x6
> +
> +/* Indicate Card Type is not clear yet */
> +#define SDHCI_CARD_TYPE_UNKNOWN			0xF
> +
> +struct sdhci_xenon_priv {
> +	unsigned char	tuning_count;
> +	/* idx of SDHC */
> +	u8		sdhc_id;
> +
> +	/*
> +	 * eMMC/SD/SDIO require different PHY settings or
> +	 * voltage control. It's necessary for Xenon driver to
> +	 * recognize card type during, or even before initialization.
> +	 * However, mmc_host->card is not available yet at that time.
> +	 * This field records the card type during init.
> +	 * For eMMC, it is updated in dt parse. For SD/SDIO, it is
> +	 * updated in xenon_init_card().
> +	 *
> +	 * It is only valid during initialization after it is updated.
> +	 * Do not access this variable in normal transfers after
> +	 * initialization completes.
> +	 */
> +	unsigned int	init_card_type;
> +};
> +
> +#endif
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-mmc-sdhci-Leave-internal-clock-on-when-bus-power-is-.patch
Type: text/x-patch
Size: 1423 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170104/f4ab8f52/attachment-0001.bin>

^ permalink raw reply

* [PATCH] cfg80211: sysfs: use wiphy_name()
From: Johannes Berg @ 2017-01-04  7:25 UTC (permalink / raw)
  To: linux-wireless; +Cc: Johannes Berg

From: Johannes Berg <johannes.berg@intel.com>

Instead of open-coding dev_name(), use the wiphy_name() inline
to make the code easier to understand. While at it, clean up
some coding style.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/wireless/sysfs.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/wireless/sysfs.c b/net/wireless/sysfs.c
index 14b3f007826d..16b6b5988be9 100644
--- a/net/wireless/sysfs.c
+++ b/net/wireless/sysfs.c
@@ -39,9 +39,11 @@ SHOW_FMT(address_mask, "%pM", wiphy.addr_mask);
 
 static ssize_t name_show(struct device *dev,
 			 struct device_attribute *attr,
-			 char *buf) {
+			 char *buf)
+{
 	struct wiphy *wiphy = &dev_to_rdev(dev)->wiphy;
-	return sprintf(buf, "%s\n", dev_name(&wiphy->dev));
+
+	return sprintf(buf, "%s\n", wiphy_name(wiphy));
 }
 static DEVICE_ATTR_RO(name);
 
-- 
2.9.3

^ permalink raw reply related

* Re: [LSF/MM TOPIC][LSF/MM ATTEND] OCSSDs - SMR, Hierarchical Interface, and Vector I/Os
From: Damien Le Moal @ 2017-01-04  7:24 UTC (permalink / raw)
  To: Slava Dubeyko, Matias Bjørling, Viacheslav Dubeyko,
	lsf-pc@lists.linux-foundation.org
  Cc: Linux FS Devel, linux-block@vger.kernel.org,
	linux-nvme@lists.infradead.org
In-Reply-To: <SN2PR04MB219184580108939442EBDA7E88610@SN2PR04MB2191.namprd04.prod.outlook.com>


Slava,

On 1/4/17 11:59, Slava Dubeyko wrote:
> What's the goal of SMR compatibility? Any unification or interface
> abstraction has the goal to hide the peculiarities of underlying
> hardware. But we have block device abstraction that hides all 
> hardware's peculiarities perfectly. Also FTL (or any other
> Translation Layer) is able to represent the device as sequence of
> physical sectors without real knowledge on software side about 
> sophisticated management activity on the device side. And, finally,
> guys will be completely happy to use the regular file systems (ext4,
> xfs) without necessity to modify software stack. But I believe that 
> the goal of Open-channel SSD approach is completely opposite. Namely,
> provide the opportunity for software side (file system, for example)
> to manage the Open-channel SSD device with smarter policy.

The Zoned Block Device API is part of the block layer. So as such, it
does abstract many aspects of the device characteristics, as so many
other API of the block layer do (look at blkdev_issue_discard or zeroout
implementations to see how far this can be pushed).

Regarding the use of open channel SSDs, I think you are absolutely
correct: (1) some users may be very happy to use a regular, unmodified
ext4 or xfs on top of an open channel SSD, as long as the FTL
implementation does a complete abstraction of the device special
features and presents a regular block device to upper layers. And
conversely, (2) some file system implementations may prefer to directly
use those special features and characteristics of open channel SSDs. No
arguing with this.

But you are missing the parallel with SMR. For SMR, or more correctly
zoned block devices since the ZBC or ZAC standards can equally apply to
HDDs and SSDs, 3 models exists: drive-managed, host-aware and host-managed.

Case (1) above corresponds *exactly* to the drive managed model, with
the difference that the abstraction of the device characteristics (SMR
here) is in the drive FW and not in a host-level FTL implementation as
it would be for open channel SSDs. Case (2) above corresponds to the
host-managed model, that is, the device user has to deal with the device
characteristics itself and use it correctly. The host-aware model lies
in between these 2 extremes: it offers the possibility of complete
abstraction by default, but also allows a user to optimize its operation
for the device by allowing access to the device characteristics. So this
would correspond to a possible third way of implementing an FTL for open
channel SSDs.

> So, my key worry that the trying to hide under the same interface the
> two different technologies (SMR and NAND flash) will be resulted in
> the loss of opportunity to manage the device in more smarter way.
> Because any unification has the goal to create a simple interface.
> But SMR and NAND flash are significantly different technologies. And
> if somebody creates technology-oriented file system, for example,
> then it needs to have access to really special features of the
> technology. Otherwise, interface will be overloaded by features of
> both technologies and it will looks like as a mess.

I do not think so, as long as the device "model" is exposed to the user
as the zoned block device interface does. This allows a user to adjust
its operation depending on the device. This is true of course as long as
each "model" has a clearly defined set of features associated. Again,
that is the case for zoned block devices and an example of how this can
be used is now in f2fs (which allows different operation modes for
host-aware devices, but only one for host-managed devices). Again, I can
see a clear parallel with open channel SSDs here.

> SMR zone and NAND flash erase block look comparable but, finally, it
> significantly different stuff. Usually, SMR zone has 265 MB in size
> but NAND flash erase block can vary from 512 KB to 8 MB (it will be
> slightly larger in the future but not more than 32 MB, I suppose). It
> is possible to group several erase blocks into aggregated entity but
> it could be not very good policy from file system point of view.

Why not? For f2fs, the 2MB segments are grouped together into sections
with a size matching the device zone size. That works well and can
actually even reduce the garbage collection overhead in some cases.
Nothing in the kernel zoned block device support limits the zone size to
a particular minimum or maximum. The only direct implication of the zone
size on the block I/O stack is that BIOs and requests cannot cross zone
boundaries. In an extreme setup, a zone size of 4KB would work too and
result in read/write commands of 4KB at most to the device.

> Another point that QLC device could have more tricky features of
> erase blocks management. Also we should apply erase operation on NAND
> flash erase block but it is not mandatory for the case of SMR zone.

Incorrect: host-managed devices require a zone "reset" (equivalent to
discard/trim) to be reused after being written once. So again, the
"tricky features" you mention will depend on the device "model",
whatever this ends up to be for an open channel SSD.

> Because SMR zone could be simply re-written in sequential order if
> all zone's data is invalid, for example. Also conventional zone could
> be really tricky point. Because it is one zone only for the whole
> device that could be updated in-place. Raw NAND flash, usually,
> hasn't likewise conventional zone.

Conventional zones are optional in zoned block devices. There may be
none at all and an implementation may well decide to not support a
device without any conventional zones if some are required.
In the case of open channel SSDs, the FTL implementation may well decide
to expose a particular range of LBAs as "conventional zones" and have a
lower level exposure for the remaining capacity whcih can then be
optimally used by the file system based on the features available for
that remaining LBA range. Again, a parallel is possible with SMR.

> Finally, if I really like to develop SMR- or NAND flash oriented file
> system then I would like to play with peculiarities of concrete
> technologies. And any unified interface will destroy the opportunity 
> to create the really efficient solution. Finally, if my software
> solution is unable to provide some fancy and efficient features then
> guys will prefer to use the regular stack (ext4, xfs + block layer).

Not necessarily. Again think in terms of device "model" and associated
feature set. An FS implementation may decide to support all possible
models, with likely a resulting incredible complexity. More likely,
similarly with what is happening with SMR, only models that make sense
will be supported by FS implementation that can be easily modified.
Example again here of f2fs: changes to support SMR were rather simple,
whereas the initial effort to support SMR with ext4 was pretty much
abandoned as it was too complex to integrate in the existing code while
keeping the existing on-disk format.

Your argument above is actually making the same point: you want your
implementation to use the device features directly. That is, your
implementation wants a "host-managed" like device model. Using ext4 will
require a "host-aware" or "drive-managed" model, which could be provided
through a different FTL or device-mapper implementation in the case of
open channel SSDs.

I am not trying to argue that open channel SSDs and zoned block devices
should be supported under the exact same API. But I can definitely see
clear parallels worth a discussion. As a first step, I would suggest
trying to try defining open channel SSDs "models" and their feature set
and see how these fit with the existing ZBC/ZAC defined models and at
least estimate the implications on the block I/O stack. If adding the
new models only results in the addition of a few top level functions or
ioctls, it may be entirely feasible to integrate the two together.

Best regards.

-- 
Damien Le Moal, Ph.D.
Sr Manager, System Software Research Group,
Western Digital
Damien.LeMoal@hgst.com
Tel: (+81) 0466-98-3593 (Ext. 51-3593)
1 kirihara-cho, Fujisawa, Kanagawa, 252-0888 Japan
www.wdc.com, www.hgst.com

^ permalink raw reply

* [LSF/MM TOPIC][LSF/MM ATTEND] OCSSDs - SMR, Hierarchical Interface, and Vector I/Os
From: Damien Le Moal @ 2017-01-04  7:24 UTC (permalink / raw)

In-Reply-To: <SN2PR04MB219184580108939442EBDA7E88610@SN2PR04MB2191.namprd04.prod.outlook.com>


Slava,

On 1/4/17 11:59, Slava Dubeyko wrote:
> What's the goal of SMR compatibility? Any unification or interface
> abstraction has the goal to hide the peculiarities of underlying
> hardware. But we have block device abstraction that hides all 
> hardware's peculiarities perfectly. Also FTL (or any other
> Translation Layer) is able to represent the device as sequence of
> physical sectors without real knowledge on software side about 
> sophisticated management activity on the device side. And, finally,
> guys will be completely happy to use the regular file systems (ext4,
> xfs) without necessity to modify software stack. But I believe that 
> the goal of Open-channel SSD approach is completely opposite. Namely,
> provide the opportunity for software side (file system, for example)
> to manage the Open-channel SSD device with smarter policy.

The Zoned Block Device API is part of the block layer. So as such, it
does abstract many aspects of the device characteristics, as so many
other API of the block layer do (look at blkdev_issue_discard or zeroout
implementations to see how far this can be pushed).

Regarding the use of open channel SSDs, I think you are absolutely
correct: (1) some users may be very happy to use a regular, unmodified
ext4 or xfs on top of an open channel SSD, as long as the FTL
implementation does a complete abstraction of the device special
features and presents a regular block device to upper layers. And
conversely, (2) some file system implementations may prefer to directly
use those special features and characteristics of open channel SSDs. No
arguing with this.

But you are missing the parallel with SMR. For SMR, or more correctly
zoned block devices since the ZBC or ZAC standards can equally apply to
HDDs and SSDs, 3 models exists: drive-managed, host-aware and host-managed.

Case (1) above corresponds *exactly* to the drive managed model, with
the difference that the abstraction of the device characteristics (SMR
here) is in the drive FW and not in a host-level FTL implementation as
it would be for open channel SSDs. Case (2) above corresponds to the
host-managed model, that is, the device user has to deal with the device
characteristics itself and use it correctly. The host-aware model lies
in between these 2 extremes: it offers the possibility of complete
abstraction by default, but also allows a user to optimize its operation
for the device by allowing access to the device characteristics. So this
would correspond to a possible third way of implementing an FTL for open
channel SSDs.

> So, my key worry that the trying to hide under the same interface the
> two different technologies (SMR and NAND flash) will be resulted in
> the loss of opportunity to manage the device in more smarter way.
> Because any unification has the goal to create a simple interface.
> But SMR and NAND flash are significantly different technologies. And
> if somebody creates technology-oriented file system, for example,
> then it needs to have access to really special features of the
> technology. Otherwise, interface will be overloaded by features of
> both technologies and it will looks like as a mess.

I do not think so, as long as the device "model" is exposed to the user
as the zoned block device interface does. This allows a user to adjust
its operation depending on the device. This is true of course as long as
each "model" has a clearly defined set of features associated. Again,
that is the case for zoned block devices and an example of how this can
be used is now in f2fs (which allows different operation modes for
host-aware devices, but only one for host-managed devices). Again, I can
see a clear parallel with open channel SSDs here.

> SMR zone and NAND flash erase block look comparable but, finally, it
> significantly different stuff. Usually, SMR zone has 265 MB in size
> but NAND flash erase block can vary from 512 KB to 8 MB (it will be
> slightly larger in the future but not more than 32 MB, I suppose). It
> is possible to group several erase blocks into aggregated entity but
> it could be not very good policy from file system point of view.

Why not? For f2fs, the 2MB segments are grouped together into sections
with a size matching the device zone size. That works well and can
actually even reduce the garbage collection overhead in some cases.
Nothing in the kernel zoned block device support limits the zone size to
a particular minimum or maximum. The only direct implication of the zone
size on the block I/O stack is that BIOs and requests cannot cross zone
boundaries. In an extreme setup, a zone size of 4KB would work too and
result in read/write commands of 4KB at most to the device.

> Another point that QLC device could have more tricky features of
> erase blocks management. Also we should apply erase operation on NAND
> flash erase block but it is not mandatory for the case of SMR zone.

Incorrect: host-managed devices require a zone "reset" (equivalent to
discard/trim) to be reused after being written once. So again, the
"tricky features" you mention will depend on the device "model",
whatever this ends up to be for an open channel SSD.

> Because SMR zone could be simply re-written in sequential order if
> all zone's data is invalid, for example. Also conventional zone could
> be really tricky point. Because it is one zone only for the whole
> device that could be updated in-place. Raw NAND flash, usually,
> hasn't likewise conventional zone.

Conventional zones are optional in zoned block devices. There may be
none at all and an implementation may well decide to not support a
device without any conventional zones if some are required.
In the case of open channel SSDs, the FTL implementation may well decide
to expose a particular range of LBAs as "conventional zones" and have a
lower level exposure for the remaining capacity whcih can then be
optimally used by the file system based on the features available for
that remaining LBA range. Again, a parallel is possible with SMR.

> Finally, if I really like to develop SMR- or NAND flash oriented file
> system then I would like to play with peculiarities of concrete
> technologies. And any unified interface will destroy the opportunity 
> to create the really efficient solution. Finally, if my software
> solution is unable to provide some fancy and efficient features then
> guys will prefer to use the regular stack (ext4, xfs + block layer).

Not necessarily. Again think in terms of device "model" and associated
feature set. An FS implementation may decide to support all possible
models, with likely a resulting incredible complexity. More likely,
similarly with what is happening with SMR, only models that make sense
will be supported by FS implementation that can be easily modified.
Example again here of f2fs: changes to support SMR were rather simple,
whereas the initial effort to support SMR with ext4 was pretty much
abandoned as it was too complex to integrate in the existing code while
keeping the existing on-disk format.

Your argument above is actually making the same point: you want your
implementation to use the device features directly. That is, your
implementation wants a "host-managed" like device model. Using ext4 will
require a "host-aware" or "drive-managed" model, which could be provided
through a different FTL or device-mapper implementation in the case of
open channel SSDs.

I am not trying to argue that open channel SSDs and zoned block devices
should be supported under the exact same API. But I can definitely see
clear parallels worth a discussion. As a first step, I would suggest
trying to try defining open channel SSDs "models" and their feature set
and see how these fit with the existing ZBC/ZAC defined models and at
least estimate the implications on the block I/O stack. If adding the
new models only results in the addition of a few top level functions or
ioctls, it may be entirely feasible to integrate the two together.

Best regards.

-- 
Damien Le Moal, Ph.D.
Sr Manager, System Software Research Group,
Western Digital
Damien.LeMoal at hgst.com
Tel: (+81) 0466-98-3593 (Ext. 51-3593)
1 kirihara-cho, Fujisawa, Kanagawa, 252-0888 Japan
www.wdc.com, www.hgst.com

^ permalink raw reply

* [RESEND,PATCH v7 2/2] Documentation/ABI: Add ABI information for QCOM socinfo driver
From: Imran Khan @ 2017-01-04  7:14 UTC (permalink / raw)
  To: andy.gross; +Cc: lee.jones, Imran Khan, open list

The socinfo ABI document describes the information provided
by socinfo driver and the corresponding attributes to access
that information.

Signed-off-by: Imran Khan <kimran@codeaurora.org>
---
 Documentation/ABI/stable/sysfs-driver-qcom_socinfo | 147 +++++++++++++++++++++
 1 file changed, 147 insertions(+)
 create mode 100644 Documentation/ABI/stable/sysfs-driver-qcom_socinfo

diff --git a/Documentation/ABI/stable/sysfs-driver-qcom_socinfo b/Documentation/ABI/stable/sysfs-driver-qcom_socinfo
new file mode 100644
index 0000000..f90c142
--- /dev/null
+++ b/Documentation/ABI/stable/sysfs-driver-qcom_socinfo
@@ -0,0 +1,147 @@
+What:		/sys/devices/soc0/accessory_chip
+Date:		January 2017
+Description:
+		This file shows the id of the accessory chip.
+
+What:		/sys/devices/soc0/adsp_image_crm
+What:		/sys/devices/soc0/adsp_image_variant
+What:		/sys/devices/soc0/adsp_image_version
+Date:		January 2017
+Description:
+		These files respectively show the crm version, variant and
+		version of the ADSP image.
+
+What:		/sys/devices/soc0/apps_image_crm
+What:		/sys/devices/soc0/apps_image_variant
+What:		/sys/devices/soc0/apps_image_version
+Date:		January 2017
+Description:
+		These files respectively show the crm version, variant and
+		version of the APPS(Linux kernel, rootfs) image.
+
+What:		/sys/devices/soc0/boot_image_crm
+What:		/sys/devices/soc0/boot_image_variant
+What:		/sys/devices/soc0/boot_image_version
+Date:		January 2017
+Description:
+		These files respectively show the crm version, variant and
+		version of the Boot(bootloader) image.
+
+What:		/sys/devices/soc0/build_id
+Date:		January 2017
+Description:
+		This file shows the unique id of current build being used on
+		the system.
+
+What:		/sys/devices/soc0/cnss_image_crm
+What:		/sys/devices/soc0/cnss_image_variant
+What:		/sys/devices/soc0/cnss_image_version
+Date:		January 2017
+Description:
+		These files respectively show the crm version, variant and
+		version of the CNSS image.
+
+What:		/sys/devices/soc0/family
+Date:		January 2017
+Description:
+		This file shows the family(e.g Snapdragon) of the SoC.
+
+What:		/sys/devices/soc0/foundry_id
+Date:		January 2017
+Description:
+		This file shows the id of the foundry, where soc was
+		manufactured.
+
+What:		/sys/devices/soc0/hw_platform
+Date:		January 2017
+Description:
+		This file shows the type of hardware platform
+		(e.g MTP, QRD etc) where SoC is being used.
+
+What:		/sys/devices/soc0/machine
+Date:		January 2017
+Description:
+		This file shows the machine name as given in the DT.
+
+What:		/sys/devices/soc0/mpss_image_crm
+What:		/sys/devices/soc0/mpss_image_variant
+What:		/sys/devices/soc0/mpss_image_version
+Date:		January 2017
+Description:
+		These files respectively show the crm version, variant and
+		version of the MPSS image.
+
+What:		/sys/devices/soc0/platform_subtype
+Date:		January 2017
+Description:
+		This file shows the sub-type of hardware platform
+		(SKUAA, SKUF etc.) where SoC is being used.
+
+What:		/sys/devices/soc0/platform_version
+Date:		January 2017
+Description:
+		This file show the version of the hardware platform.
+
+What:		/sys/devices/soc0/pmic_die_revision
+Date:		January 2017
+Description:
+		This file shows revision of PMIC die.
+
+What:		/sys/devices/soc0/pmic_model
+Date:		January 2017
+Description:
+		This file shows name of PMIC model.
+
+What:		/sys/devices/soc0/qcom_odm
+Date:		January 2017
+Description:
+		This file shows the ODM using the SoC.
+
+What:		/sys/devices/soc0/raw_version
+Date:		January 2017
+Description:
+		This file shows raw version of the SoC.
+
+What:		/sys/devices/soc0/revision
+Date:		January 2017
+Description:
+		This file shows revision of the SoC.
+
+What:		/sys/devices/soc0/rpm_image_crm
+What:		/sys/devices/soc0/rpm_image_variant
+What:		/sys/devices/soc0/rpm_image_version
+Date:		January 2017
+Description:
+		These files respectively show the crm version, variant and
+		version of the RPM image.
+
+What:		/sys/devices/soc0/serial_number
+Date:		January 2017
+Description:
+		This file shows serial number of the SoC.
+
+What:		/sys/devices/soc0/soc_id
+Date:		January 2017
+Description:
+		This file shows the unique numeric id of a Qualcomm SoC.
+
+What:		/sys/devices/soc0/tz_image_crm
+What:		/sys/devices/soc0/tz_image_variant
+What:		/sys/devices/soc0/tz_image_version
+Date:		January 2017
+Description:
+		These files respectively show the crm version, variant and
+		version of the TZ image.
+
+What:		/sys/devices/soc0/vendor
+Date:		January 2017
+Description:
+		This file shows the vendor of the SoC.
+
+What:		/sys/devices/soc0/video_image_crm
+What:		/sys/devices/soc0/video_image_variant
+What:		/sys/devices/soc0/video_image_version
+Date:		January 2017
+Description:
+		These files respectively show the crm version, variant and
+		version of the video image.
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply related

* Re: [RFC] kbdif: add multi-touch support
From: Oleksandr Andrushchenko @ 2017-01-04  7:24 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: lars.kurth, vlad.babchuk, dario.faggioli, julien.grall,
	andrii.anisov, olekstysh, al1img, JBeulich, xen-devel, joculator
In-Reply-To: <alpine.DEB.2.10.1701031653080.17742@sstabellini-ThinkPad-X260>

First of all, thank you for comments

On 01/04/2017 03:03 AM, Stefano Stabellini wrote:
> On Tue, 3 Jan 2017, Oleksandr Andrushchenko wrote:
>> From: Oleksandr Andrushchenko <Oleksandr_Andrushchenko@epam.com>
>>
>> Signed-off-by: Oleksandr Andrushchenko <Oleksandr_Andrushchenko@epam.com>
>> ---
>>   xen/include/public/io/kbdif.h | 64 +++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 64 insertions(+)
>>
>> diff --git a/xen/include/public/io/kbdif.h b/xen/include/public/io/kbdif.h
>> index 2d2aebd..ad94b53 100644
>> --- a/xen/include/public/io/kbdif.h
>> +++ b/xen/include/public/io/kbdif.h
>> @@ -45,6 +45,19 @@
>>    */
>>   #define XENKBD_TYPE_POS     4
>>   
>> +/*
>> + * Multi-touch event
>> + * Capable backend sets feature-multi-touch in xenstore.
>> + * Frontend requests feature by setting request-multi-touch in xenstore.
>> + * Frontend supports up to XENKBD_MT_NUM_DEV virtual multi-touch input devices,
>> + * configured by the backend in xenstore under mt-%d folder, %d being
>> + * a sequential number of the virtual input device:
>> + *   o num-contacts - number of simultaneous touches supported
>> + *   o width - width of the touch area in pixels
>> + *   o height - height of the touch area in pixels
> Please write down the max width and height supported by the protocol,
> keeping in mind that motion events below use int32_t for coordinates.
I will put "width(height) of the... in pixels, 32-bit signed integer"
>
> Are there any benefits of this compared to just setting up multiple kbd
> connections, one per multi-touch device? The only benefit I can think of
> is saving few pages.
Well, not only saving a few pages, but somewhat
simplifying handling of the protocol on both back and
front ends. But, you are probably right as current
protocol is capable of holding
(XENKBD_IN_RING_SIZE - XENKBD_IN_RING_OFFS) / XENKBD_IN_EVENT_SIZE ==
(2048 - 1024) / 40 = 25 incoming events which may not be
enough for multiple mt devices delivering hundreds of
events per second.
Will set-up dedicated rings per mt device then.
Will also remove
   uint8_t dev_idx;        /* index of the multi-touch device */
as every device will have its own ring.
Will extend XenStore configuration for mt devices with:
  o page-ref (?)
  o page-gref
  o event-channel
as it is done by the Linux xen-kbdfront driver.

BTW, is there any reason we need "page-ref"? My understanding is
that the pair of page-gref + event-channel is enough to establish
and uniquely identify the connection.

>> + */
>> +#define XENKBD_TYPE_MTOUCH  5
>>   struct xenkbd_motion
>>   {
>>       uint8_t type;        /* XENKBD_TYPE_MOTION */
>> @@ -68,6 +81,56 @@ struct xenkbd_position
>>       int32_t rel_z;       /* relative Z motion (wheel) */
>>   };
>>   
>> +/* number of simultaneously supported multi-touch virtual input devices */
>> +#define XENKBD_MT_NUM_DEV   4
> If it turns out that supporting multiple devices per connection is a
> good idea, then I suggest that the max number of devices is a backend
> property on xenstore.
Instead of setting the number of mt devices front can easily
find this value by reading "mt-%d" XenStore entries.
The requirement in the protocol is that "...%d being
a sequential number of the virtual input device". Thus,
first entry which we fail to sequentially read will
indicate end of configured devices. Of course,
xenbus_scanf return value needs to be checked if it
has failed because there is no such value in XenStore
or for any other reason.

>
>> +/* Sent when a new touch is made: touch is assigned a unique contact
>> + * ID, sent with this and consequent events related to this touch.
>> + * Contact ID will be reused after XENKBD_MT_EV_UP event.
> Will be reused or can be reused?
I would probably say "may be reused" as it depends on how
and if new touches/contacts are made
>   Please provide an example of a Contact
> ID lifecycle.
Do you want it to be described in the protocol or just here?
If the latter then, for example, as Wayland documentation
describes it [1]:
"Touch interactions can consist of one or more contacts.
  For each contact, a series of events is generated, starting
  with a down event, followed by zero or more motion events,
  and ending with an up event. Events relating to the same
  contact point can be identified by the ID of the sequence."
So, if there is contact/touch a free Contact ID is assigned to
this contact(sequence) and it is "released" when contact is
done, e.g. after up event. From this point contact ID may be
reused.

I was basing the mt additions to the protocol on Wayland [1],
Linux [2] and Windows [3] multi-touch support, so those may
better explain the idea
> What is the max Contact ID?
>
/* contact ID, [0; num-contacts - 1] */
num-contacts - number of simultaneous touches supported
>> + */
>> +#define XENKBD_MT_EV_DOWN   0
>> +/* Touch point has been released */
>> +#define XENKBD_MT_EV_UP     1
>> +/* Touch point has changed its coordinate(s) */
>> +#define XENKBD_MT_EV_MOTION 2
>> +/* Input synchronization event: shows end of a set of events
>> + * which logically belong together.
>> + */
>> +#define XENKBD_MT_EV_SYN    3
>> +/* Touch point has changed its shape. Shape is approximated by an ellipse
>> + * through the major and minor axis lengths: major is the longer diameter
>> + * of the ellipse and minor is the shorter one. Center of the ellipse is
>> + * reported via XENKBD_MT_EV_DOWN/XENKBD_MT_EV_MOTION events.
>> + */
>> +#define XENKBD_MT_EV_SHAPE  4
>> +/* Touch point's shape has changed its orientation: calculated as a clockwise
>> + * angle between the major axis of the ellipse and positive Y axis in degrees,
>> + * [-180; +180].
>> + */
>> +#define XENKBD_MT_EV_ORIENT 5
>> +
>> +struct xenkbd_mtouch {
>> +    uint8_t type;             /* XENKBD_TYPE_MTOUCH */
>> +    uint8_t dev_idx;          /* index of the multi-touch device */
>> +    uint8_t event_type;       /* XENKBD_MT_EV_??? */
>> +    uint8_t reserved;         /* reserved for the future use */
>> +    int32_t contact_id;       /* contact ID, [0; num-contacts - 1] */
>> +    union {
>> +        /* XENKBD_MT_EV_DOWN/XENKBD_MT_EV_MOTION */
>> +        struct {
>> +            int32_t abs_x;    /* absolute X position, pixels */
>> +            int32_t abs_y;    /* absolute Y position, pixels */
>> +        } pos;
>> +        /* XENKBD_MT_EV_SHAPE */
>> +        struct {
>> +            uint32_t major;   /* length of the major axis, pixels */
>> +            uint32_t minor;   /* length of the minor axis, pixels */
>> +        } shape;
>> +        /* XENKBD_MT_EV_ORIENT */
>> +        uint16_t orientation; /* clockwise angle of the major axis */
>> +    } u;
>> +};
> Need a binary representation.
>
Could you please elaborate more on this, this is not
clear to me.
>>   #define XENKBD_IN_EVENT_SIZE 40
>>   
>>   union xenkbd_in_event
>> @@ -76,6 +139,7 @@ union xenkbd_in_event
>>       struct xenkbd_motion motion;
>>       struct xenkbd_key key;
>>       struct xenkbd_position pos;
>> +    struct xenkbd_mtouch mtouch;
>>       char pad[XENKBD_IN_EVENT_SIZE];
>>   };
>>   
>> -- 
>> 2.7.4

Related question: what if I also extend existing
xenkbd_motion/key/position by adding uint32(8?)_t dev_idx
at the bottom of the structures, e.g.

struct xenkbd_motion {
     uint8_t type;        /* XENKBD_TYPE_MOTION */
     int32_t rel_x;        /* relative X motion */
     int32_t rel_y;        /* relative Y motion */
     int32_t rel_z;        /* relative Z motion (wheel) */
     uint32_t dev_idx; or uint8_t dev_idx;
};

This way:
1. Existing fronts/backs will not be affected
2. New fronts/backs may use this field to serve
multiple keyboards and/or pointer devices via single ring

Should I make this change as a dedicated patch or do you think
it can be in the same one for mt?

Thank you,
Oleksandr
[1] 
https://cgit.freedesktop.org/wayland/wayland/tree/protocol/wayland.xml#n2196
[2] https://www.kernel.org/doc/Documentation/input/multi-touch-protocol.txt
[3] 
https://msdn.microsoft.com/en-us/library/jj151564(v=vs.85).aspx#optional_hid_usages

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply

* [PATCH] nvme: use __iowrite64_copy for SQs in the CMB
From: Saar Gross @ 2017-01-04  7:22 UTC (permalink / raw)

In-Reply-To: <20161221102237.GA24532@infradead.org>

As already mentioned: the current implementation uses memcpy() which
is generic in nature, whereas the new implementation uses explicit
64-bit writes when possible.
What else are you looking for?

Thanks,
Saar


On Wed, Dec 21, 2016@12:22 PM, Christoph Hellwig <hch@infradead.org> wrote:
> On Mon, Dec 19, 2016@02:18:30PM +0200, Saar Gross wrote:
>> Hi again,
>>
>> Any other comments?
>
> I still can't see a good explanation on why we'd want/need the patch.

^ permalink raw reply

* Re: [PATCH v2 5/7] net/virtio_user: add vhost kernel support
From: Tan, Jianfeng @ 2017-01-04  7:22 UTC (permalink / raw)
  To: Yuanhan Liu; +Cc: dev, ferruh.yigit, cunming.liang
In-Reply-To: <20161226074437.GD19288@yliu-dev.sh.intel.com>


Sorry, I forget to reply this comment.

On 12/26/2016 3:44 PM, Yuanhan Liu wrote:
> [...]
>> +
>> +	/* Does not work when VIRTIO_F_IOMMU_PLATFORM now, why? */
> Because this feature need the vhost IOTLB support from the device
> emulation. Patches for QEMU hasn't been merged yet, but it has been
> there for a while.

Yes. And it's in need of help from QEMU.

>
> Since we don't have the support yet, for sure it won't work. But
> I'm wondering why you have to disable it explicitly?

Here we do not have QEMU. Frontend driver talks with vhost-net through 
virtio_user_dev. And both ends claim to support VIRTIO_F_IOMMU_PLATFORM. 
So this feature bit will be negotiated if we don't explicitly disable 
it. In my previous test, it fails in vhost_init_device_iotlb() of vhost 
kernel module. My guess is that, for this feature, without the help of 
QEMU, vhost kernel module cannot work independently.

Thanks,
Jianfeng

^ permalink raw reply

* Re: [PATCH v2 5/9] net/virtio: setup rxq interrupts
From: Yuanhan Liu @ 2017-01-04  7:22 UTC (permalink / raw)
  To: Tan, Jianfeng; +Cc: dev, stephen
In-Reply-To: <88093c9a-5bd8-5acb-aa87-4aa92c03a2df@intel.com>

On Wed, Jan 04, 2017 at 02:56:50PM +0800, Tan, Jianfeng wrote:
> 
> 
> On 12/30/2016 2:27 PM, Yuanhan Liu wrote:
> >On Thu, Dec 29, 2016 at 07:30:39AM +0000, Jianfeng Tan wrote:
> >>This patch mainly allocates structure to store queue/irq mapping,
> >>and configure queue/irq mapping down through PCI ops. It also creates
> >>eventfds for each Rx queue and tell the kernel about the eventfd/intr
> >>binding.
> >>
> >>Mostly importantly, different from previous NICs (usually implements
> >>these logic in dev_start()), virtio's interrupt settings should be
> >>configured down to QEMU before sending DRIVER_OK notification.
> >Isn't it obvious we have to have all driver stuff (including interrupt
> >settings) configured properly before setting DRIVER_OK? :) That said,
> >it's meanless to state the fact that virtio acts differently than other
> >nics here on dev_start/stop.
> >
> >>Note: We only support 1:1 queue/irq mapping so far, which means, each
> >>rx queue has one exclusive interrupt (corresponding to irqfd in the
> >>qemu/kvm) to get notified when packets are available on that queue.
> >That means you have to setup the "vectors=N" option has to set correctly
> >in QEMU, otherwise it won't work?
> 
> Yes, actually, the correct value should be "vectors>=N+1", with N standing

Yeah, and it's a typo.

> for the number of queue pairs. It's due to the hard coded mapping logic:
> 0 -> config irq
> 1 -> rxq0
> 2 -> rxq1
> ...
> 
> >  If so, you also have to doc it somewhere.
> 
> Agreed.
> 
> [...]
> >>+
> >>+	if (virtio_queues_bind_intr(dev) < 0) {
> >>+		PMD_INIT_LOG(ERR, "Failed to bind queue/interrupt");
> >>+		return -1;
> >You have to free intr_handle->intr_vec, otherwise, memory leak occurs.
> 
> It's freed at dev_close(). Do you mean freeing and reallocating here? As

The typical way is free the resources have been allocated when errors
happens.

> nr_rx_queues is not a changeable value, I don't see the necessity here. I
> miss something?

No. nb_rx_queues does change, when people reconfigure the queue number.
However, the MAX queues the virito supports does not change. You could
use that number for allocation.

	--yliu

^ permalink raw reply

* [PATCH v2 1/1] extcon: usb-gpio: add pinctrl operation during system PM
From: Peter Chen @ 2017-01-04  7:19 UTC (permalink / raw)
  To: myungjoo.ham, cw00.choi; +Cc: rogerq, linux-kernel, Peter Chen

At some systems, the pinctrl setting will be lost or needs to
set as "sleep" state to save power consumption. So, we need to
configure pinctrl as "sleep" state when system enters suspend,
and as "default" state after system resumes. In this way, the
pinctrl value can be recovered as "default" state after resuming.

Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
Changes for v2:
- Add header file for pinctrl to avoid build error
- Only set pin as "sleep" state when the wakeup is not requested

 drivers/extcon/extcon-usb-gpio.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/extcon/extcon-usb-gpio.c b/drivers/extcon/extcon-usb-gpio.c
index d589c5f..a5e1882 100644
--- a/drivers/extcon/extcon-usb-gpio.c
+++ b/drivers/extcon/extcon-usb-gpio.c
@@ -27,6 +27,7 @@
 #include <linux/slab.h>
 #include <linux/workqueue.h>
 #include <linux/acpi.h>
+#include <linux/pinctrl/consumer.h>
 
 #define USB_GPIO_DEBOUNCE_MS	20	/* ms */
 
@@ -245,6 +246,9 @@ static int usb_extcon_suspend(struct device *dev)
 	if (info->vbus_gpiod)
 		disable_irq(info->vbus_irq);
 
+	if (!device_may_wakeup(dev))
+		pinctrl_pm_select_sleep_state(dev);
+
 	return ret;
 }
 
@@ -253,6 +257,9 @@ static int usb_extcon_resume(struct device *dev)
 	struct usb_extcon_info *info = dev_get_drvdata(dev);
 	int ret = 0;
 
+	if (!device_may_wakeup(dev))
+		pinctrl_pm_select_default_state(dev);
+
 	if (device_may_wakeup(dev)) {
 		if (info->id_gpiod) {
 			ret = disable_irq_wake(info->id_irq);
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH] dax: fix deadlock with DAX 4k holes
From: Jan Kara @ 2017-01-04  7:18 UTC (permalink / raw)
  To: Ross Zwisler
  Cc: Xiong Zhou, stable, linux-kernel, Andrew Morton,
	Christoph Hellwig, Dan Williams, Dave Chinner, Dave Hansen,
	Jan Kara, linux-mm, linux-nvdimm
In-Reply-To: <1483479365-13607-1-git-send-email-ross.zwisler@linux.intel.com>

On Tue 03-01-17 14:36:05, Ross Zwisler wrote:
> Currently in DAX if we have three read faults on the same hole address we
> can end up with the following:
> 
> Thread 0		Thread 1		Thread 2
> --------		--------		--------
> dax_iomap_fault
>  grab_mapping_entry
>   lock_slot
>    <locks empty DAX entry>
> 
>   			dax_iomap_fault
> 			 grab_mapping_entry
> 			  get_unlocked_mapping_entry
> 			   <sleeps on empty DAX entry>
> 
> 						dax_iomap_fault
> 						 grab_mapping_entry
> 						  get_unlocked_mapping_entry
> 						   <sleeps on empty DAX entry>
>   dax_load_hole
>    find_or_create_page
>    ...
>     page_cache_tree_insert
>      dax_wake_mapping_entry_waiter
>       <wakes one sleeper>
>      __radix_tree_replace
>       <swaps empty DAX entry with 4k zero page>
> 
> 			<wakes>
> 			get_page
> 			lock_page
> 			...
> 			put_locked_mapping_entry
> 			unlock_page
> 			put_page
> 
> 						<sleeps forever on the DAX
> 						 wait queue>
> 
> The crux of the problem is that once we insert a 4k zero page, all locking
> from then on is done in terms of that 4k zero page and any additional
> threads sleeping on the empty DAX entry will never be woken.  Fix this by
> waking all sleepers when we replace the DAX radix tree entry with a 4k zero
> page.  This will allow all sleeping threads to successfully transition from
> locking based on the DAX empty entry to locking on the 4k zero page.
> 
> With the test case reported by Xiong this happens very regularly in my test
> setup, with some runs resulting in 9+ threads in this deadlocked state.
> With this fix I've been able to run that same test dozens of times in a
> loop without issue.
> 
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> Reported-by: Xiong Zhou <xzhou@redhat.com>
> Fixes: commit ac401cc78242 ("dax: New fault locking")
> Cc: Jan Kara <jack@suse.cz>
> Cc: stable@vger.kernel.org # 4.7+

Ah, very good catch. You can add:

Reviewed-by: Jan Kara <jack@suse.cz>

I wonder why I was not able to reproduce this... Probably the timing didn't
work out right on my test machine.

								Honza

> ---
> 
> This issue exists as far back as v4.7, and I was easly able to reproduce it
> with v4.7 using the same test.
> 
> Unfortunately this patch won't apply cleanly to the stable trees, but the
> change is very simple and should be easy to replicate by hand.  Please ping
> me if you'd like patches that apply cleanly to the v4.9 and v4.8.15 trees.
> 
> ---
>  mm/filemap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/filemap.c b/mm/filemap.c
> index d0e4d10..b772a33 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -138,7 +138,7 @@ static int page_cache_tree_insert(struct address_space *mapping,
>  				dax_radix_locked_entry(0, RADIX_DAX_EMPTY));
>  			/* Wakeup waiters for exceptional entry lock */
>  			dax_wake_mapping_entry_waiter(mapping, page->index, p,
> -						      false);
> +						      true);
>  		}
>  	}
>  	__radix_tree_replace(&mapping->page_tree, node, slot, page,
> -- 
> 2.7.4
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply


This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.