* [PATCH net-next v5] net/ncsi: Extend NC-SI Netlink interface to allow user space to send NC-SI command
From: Samuel Mendoza-Jonas @ 2018-10-11 3:55 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <245b32d9a3dd463f9334b9704b639ec4@AUSX13MPS302.AMER.DELL.COM>
On Wed, 2018-10-10 at 18:11 +0000, Justin.Lee1 at Dell.com wrote:
<snip>
> +
> + len = nla_len(info->attrs[NCSI_ATTR_DATA]);
> + if (len < sizeof(struct ncsi_pkt_hdr)) {
> + netdev_info(ndp->ndev.dev, "NCSI: no command to send %u\n",
> + package_id);
> + ret = -EINVAL;
> + goto out_netlink;
> + } else {
> + data = (unsigned char *)nla_data(info->attrs[NCSI_ATTR_DATA]);
> + }
I only just noticed this, the call to nla_len() can cause a null-dereference if
the NCSI_ATTR_DATA attribute isn't present; we need to make sure it exists
before accessing it in info->attrs.
eg:
root at ozrom2-bmc:~# ./ncsi-netlink -l 2 -p 0 -c 0 --cmd
[ 81.399837] Unable to handle kernel NULL pointer dereference at virtual address 00000000
[ 81.409092] pgd = ddaa9fa6
[ 81.413084] [00000000] *pgd=9702c831, *pte=00000000, *ppte=00000000
[ 81.420729] Internal error: Oops: 17 [#1] ARM
[ 81.426447] CPU: 0 PID: 1028 Comm: ncsi-netlink Not tainted 4.18.8-sammj-00144-gbc129f31bfa5 #12
...
[ 81.874434] Kernel panic - not syncing: Fatal exception
Cheers,
Sam
^ permalink raw reply
* [PATCH net-next v5] net/ncsi: Extend NC-SI Netlink interface to allow user space to send NC-SI command
From: Justin.Lee1 @ 2018-10-11 5:37 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <e26c7aa664a2dad4f5bcf4efaa1b3eb655548b01.camel@mendozajonas.com>
> On Wed, 2018-10-10 at 18:11 +0000, Justin.Lee1 at Dell.com wrote:
> <snip>
> > +
> > + len = nla_len(info->attrs[NCSI_ATTR_DATA]);
> > + if (len < sizeof(struct ncsi_pkt_hdr)) {
> > + netdev_info(ndp->ndev.dev, "NCSI: no command to send %u\n",
> > + package_id);
> > + ret = -EINVAL;
> > + goto out_netlink;
> > + } else {
> > + data = (unsigned char *)nla_data(info->attrs[NCSI_ATTR_DATA]);
> > + }
>
> I only just noticed this, the call to nla_len() can cause a null-dereference if
> the NCSI_ATTR_DATA attribute isn't present; we need to make sure it exists
> before accessing it in info->attrs.
>
> eg:
>
> root at ozrom2-bmc:~# ./ncsi-netlink -l 2 -p 0 -c 0 --cmd
> [ 81.399837] Unable to handle kernel NULL pointer dereference at virtual address 00000000
> [ 81.409092] pgd = ddaa9fa6
> [ 81.413084] [00000000] *pgd=9702c831, *pte=00000000, *ppte=00000000
> [ 81.420729] Internal error: Oops: 17 [#1] ARM
> [ 81.426447] CPU: 0 PID: 1028 Comm: ncsi-netlink Not tainted 4.18.8-sammj-00144-gbc129f31bfa5 #12
> ...
> [ 81.874434] Kernel panic - not syncing: Fatal exception
>
> Cheers,
> Sam
Good catch! I will address this and generate the new patch.
Thanks,
Justin
^ permalink raw reply
* [PATCH net-next v6] net/ncsi: Extend NC-SI Netlink interface to allow user space to send NC-SI command
From: Justin.Lee1 @ 2018-10-11 6:21 UTC (permalink / raw)
To: linux-aspeed
The new command (NCSI_CMD_SEND_CMD) is added to allow user space application
to send NC-SI command to the network card.
Also, add a new attribute (NCSI_ATTR_DATA) for transferring request and response.
The work flow is as below.
Request:
User space application
-> Netlink interface (msg)
-> new Netlink handler - ncsi_send_cmd_nl()
-> ncsi_xmit_cmd()
Response:
Response received - ncsi_rcv_rsp()
-> internal response handler - ncsi_rsp_handler_xxx()
-> ncsi_rsp_handler_netlink()
-> ncsi_send_netlink_rsp ()
-> Netlink interface (msg)
-> user space application
Command timeout - ncsi_request_timeout()
-> ncsi_send_netlink_timeout ()
-> Netlink interface (msg with zero data length)
-> user space application
Error:
Error detected
-> ncsi_send_netlink_err ()
-> Netlink interface (err msg)
-> user space application
Signed-off-by: Justin Lee <justin.lee1@dell.com>
---
V6: Add checking before accessing NCSI_ATTR_DATA attribute to avoid null-dereference.
V5: Update comments and debug message.
V4: Update comments and remove some debug message.
V3: Based on http://patchwork.ozlabs.org/patch/979688/ to remove the duplicated code.
V2: Remove non-related debug message and clean up the code.
include/uapi/linux/ncsi.h | 6 ++
net/ncsi/internal.h | 7 ++
net/ncsi/ncsi-cmd.c | 8 ++
net/ncsi/ncsi-manage.c | 16 ++++
net/ncsi/ncsi-netlink.c | 205 ++++++++++++++++++++++++++++++++++++++++++++++
net/ncsi/ncsi-netlink.h | 12 +++
net/ncsi/ncsi-rsp.c | 67 +++++++++++++--
7 files changed, 316 insertions(+), 5 deletions(-)
diff --git a/include/uapi/linux/ncsi.h b/include/uapi/linux/ncsi.h
index 4c292ec..0a26a55 100644
--- a/include/uapi/linux/ncsi.h
+++ b/include/uapi/linux/ncsi.h
@@ -23,6 +23,9 @@
* optionally the preferred NCSI_ATTR_CHANNEL_ID.
* @NCSI_CMD_CLEAR_INTERFACE: clear any preferred package/channel combination.
* Requires NCSI_ATTR_IFINDEX.
+ * @NCSI_CMD_SEND_CMD: send NC-SI command to network card.
+ * Requires NCSI_ATTR_IFINDEX, NCSI_ATTR_PACKAGE_ID
+ * and NCSI_ATTR_CHANNEL_ID.
* @NCSI_CMD_MAX: highest command number
*/
enum ncsi_nl_commands {
@@ -30,6 +33,7 @@ enum ncsi_nl_commands {
NCSI_CMD_PKG_INFO,
NCSI_CMD_SET_INTERFACE,
NCSI_CMD_CLEAR_INTERFACE,
+ NCSI_CMD_SEND_CMD,
__NCSI_CMD_AFTER_LAST,
NCSI_CMD_MAX = __NCSI_CMD_AFTER_LAST - 1
@@ -43,6 +47,7 @@ enum ncsi_nl_commands {
* @NCSI_ATTR_PACKAGE_LIST: nested array of NCSI_PKG_ATTR attributes
* @NCSI_ATTR_PACKAGE_ID: package ID
* @NCSI_ATTR_CHANNEL_ID: channel ID
+ * @NCSI_ATTR_DATA: command payload
* @NCSI_ATTR_MAX: highest attribute number
*/
enum ncsi_nl_attrs {
@@ -51,6 +56,7 @@ enum ncsi_nl_attrs {
NCSI_ATTR_PACKAGE_LIST,
NCSI_ATTR_PACKAGE_ID,
NCSI_ATTR_CHANNEL_ID,
+ NCSI_ATTR_DATA,
__NCSI_ATTR_AFTER_LAST,
NCSI_ATTR_MAX = __NCSI_ATTR_AFTER_LAST - 1
diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index 3d0a33b..13c9b5e 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -175,6 +175,8 @@ struct ncsi_package;
#define NCSI_RESERVED_CHANNEL 0x1f
#define NCSI_CHANNEL_INDEX(c) ((c) & ((1 << NCSI_PACKAGE_SHIFT) - 1))
#define NCSI_TO_CHANNEL(p, c) (((p) << NCSI_PACKAGE_SHIFT) | (c))
+#define NCSI_MAX_PACKAGE 8
+#define NCSI_MAX_CHANNEL 32
struct ncsi_channel {
unsigned char id;
@@ -220,11 +222,15 @@ struct ncsi_request {
bool used; /* Request that has been assigned */
unsigned int flags; /* NCSI request property */
#define NCSI_REQ_FLAG_EVENT_DRIVEN 1
+#define NCSI_REQ_FLAG_NETLINK_DRIVEN 2
struct ncsi_dev_priv *ndp; /* Associated NCSI device */
struct sk_buff *cmd; /* Associated NCSI command packet */
struct sk_buff *rsp; /* Associated NCSI response packet */
struct timer_list timer; /* Timer on waiting for response */
bool enabled; /* Time has been enabled or not */
+ u32 snd_seq; /* netlink sending sequence number */
+ u32 snd_portid; /* netlink portid of sender */
+ struct nlmsghdr nlhdr; /* netlink message header */
};
enum {
@@ -310,6 +316,7 @@ struct ncsi_cmd_arg {
unsigned int dwords[4];
};
unsigned char *data; /* NCSI OEM data */
+ struct genl_info *info; /* Netlink information */
};
extern struct list_head ncsi_dev_list;
diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c
index 82b7d92..356af47 100644
--- a/net/ncsi/ncsi-cmd.c
+++ b/net/ncsi/ncsi-cmd.c
@@ -17,6 +17,7 @@
#include <net/ncsi.h>
#include <net/net_namespace.h>
#include <net/sock.h>
+#include <net/genetlink.h>
#include "internal.h"
#include "ncsi-pkt.h"
@@ -346,6 +347,13 @@ int ncsi_xmit_cmd(struct ncsi_cmd_arg *nca)
if (!nr)
return -ENOMEM;
+ /* track netlink information */
+ if (nca->req_flags == NCSI_REQ_FLAG_NETLINK_DRIVEN) {
+ nr->snd_seq = nca->info->snd_seq;
+ nr->snd_portid = nca->info->snd_portid;
+ nr->nlhdr = *nca->info->nlhdr;
+ }
+
/* Prepare the packet */
nca->id = nr->id;
ret = nch->handler(nr->cmd, nca);
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 0912847..76a4bcb 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -19,6 +19,7 @@
#include <net/addrconf.h>
#include <net/ipv6.h>
#include <net/if_inet6.h>
+#include <net/genetlink.h>
#include "internal.h"
#include "ncsi-pkt.h"
@@ -406,6 +407,9 @@ static void ncsi_request_timeout(struct timer_list *t)
{
struct ncsi_request *nr = from_timer(nr, t, timer);
struct ncsi_dev_priv *ndp = nr->ndp;
+ struct ncsi_package *np;
+ struct ncsi_channel *nc;
+ struct ncsi_cmd_pkt *cmd;
unsigned long flags;
/* If the request already had associated response,
@@ -419,6 +423,18 @@ static void ncsi_request_timeout(struct timer_list *t)
}
spin_unlock_irqrestore(&ndp->lock, flags);
+ if (nr->flags == NCSI_REQ_FLAG_NETLINK_DRIVEN) {
+ if (nr->cmd) {
+ /* Find the package */
+ cmd = (struct ncsi_cmd_pkt *)
+ skb_network_header(nr->cmd);
+ ncsi_find_package_and_channel(ndp,
+ cmd->cmd.common.channel,
+ &np, &nc);
+ ncsi_send_netlink_timeout(nr, np, nc);
+ }
+ }
+
/* Release the request */
ncsi_free_request(nr);
}
diff --git a/net/ncsi/ncsi-netlink.c b/net/ncsi/ncsi-netlink.c
index 45f33d6..80ba212 100644
--- a/net/ncsi/ncsi-netlink.c
+++ b/net/ncsi/ncsi-netlink.c
@@ -20,6 +20,7 @@
#include <uapi/linux/ncsi.h>
#include "internal.h"
+#include "ncsi-pkt.h"
#include "ncsi-netlink.h"
static struct genl_family ncsi_genl_family;
@@ -29,6 +30,7 @@ static const struct nla_policy ncsi_genl_policy[NCSI_ATTR_MAX + 1] = {
[NCSI_ATTR_PACKAGE_LIST] = { .type = NLA_NESTED },
[NCSI_ATTR_PACKAGE_ID] = { .type = NLA_U32 },
[NCSI_ATTR_CHANNEL_ID] = { .type = NLA_U32 },
+ [NCSI_ATTR_DATA] = { .type = NLA_BINARY, .len = 2048 },
};
static struct ncsi_dev_priv *ndp_from_ifindex(struct net *net, u32 ifindex)
@@ -366,6 +368,203 @@ static int ncsi_clear_interface_nl(struct sk_buff *msg, struct genl_info *info)
return 0;
}
+static int ncsi_send_cmd_nl(struct sk_buff *msg, struct genl_info *info)
+{
+ struct ncsi_dev_priv *ndp;
+
+ struct ncsi_cmd_arg nca;
+ struct ncsi_pkt_hdr *hdr;
+
+ u32 package_id, channel_id;
+ unsigned char *data;
+ int len, ret;
+
+ if (!info || !info->attrs) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ if (!info->attrs[NCSI_ATTR_IFINDEX]) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ if (!info->attrs[NCSI_ATTR_PACKAGE_ID]) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ if (!info->attrs[NCSI_ATTR_CHANNEL_ID]) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ if (!info->attrs[NCSI_ATTR_DATA]) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ ndp = ndp_from_ifindex(get_net(sock_net(msg->sk)),
+ nla_get_u32(info->attrs[NCSI_ATTR_IFINDEX]));
+ if (!ndp) {
+ ret = -ENODEV;
+ goto out;
+ }
+
+ package_id = nla_get_u32(info->attrs[NCSI_ATTR_PACKAGE_ID]);
+ channel_id = nla_get_u32(info->attrs[NCSI_ATTR_CHANNEL_ID]);
+
+ if (package_id >= NCSI_MAX_PACKAGE || channel_id >= NCSI_MAX_CHANNEL) {
+ ret = -ERANGE;
+ goto out_netlink;
+ }
+
+ len = nla_len(info->attrs[NCSI_ATTR_DATA]);
+ if (len < sizeof(struct ncsi_pkt_hdr)) {
+ netdev_info(ndp->ndev.dev, "NCSI: no command to send %u\n",
+ package_id);
+ ret = -EINVAL;
+ goto out_netlink;
+ } else {
+ data = (unsigned char *)nla_data(info->attrs[NCSI_ATTR_DATA]);
+ }
+
+ hdr = (struct ncsi_pkt_hdr *)data;
+
+ nca.ndp = ndp;
+ nca.package = (unsigned char)package_id;
+ nca.channel = (unsigned char)channel_id;
+ nca.type = hdr->type;
+ nca.req_flags = NCSI_REQ_FLAG_NETLINK_DRIVEN;
+ nca.info = info;
+ nca.payload = ntohs(hdr->length);
+ nca.data = data + sizeof(*hdr);
+
+ ret = ncsi_xmit_cmd(&nca);
+out_netlink:
+ if (ret != 0) {
+ netdev_err(ndp->ndev.dev,
+ "NCSI: Error %d sending command\n",
+ ret);
+ ncsi_send_netlink_err(ndp->ndev.dev,
+ info->snd_seq,
+ info->snd_portid,
+ info->nlhdr,
+ ret);
+ }
+out:
+ return ret;
+}
+
+int ncsi_send_netlink_rsp(struct ncsi_request *nr,
+ struct ncsi_package *np,
+ struct ncsi_channel *nc)
+{
+ struct sk_buff *skb;
+ struct net *net;
+ void *hdr;
+ int rc;
+
+ net = dev_net(nr->rsp->dev);
+
+ skb = genlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
+ if (!skb)
+ return -ENOMEM;
+
+ hdr = genlmsg_put(skb, nr->snd_portid, nr->snd_seq,
+ &ncsi_genl_family, 0, NCSI_CMD_SEND_CMD);
+ if (!hdr) {
+ kfree_skb(skb);
+ return -EMSGSIZE;
+ }
+
+ nla_put_u32(skb, NCSI_ATTR_IFINDEX, nr->rsp->dev->ifindex);
+ if (np)
+ nla_put_u32(skb, NCSI_ATTR_PACKAGE_ID, np->id);
+ if (nc)
+ nla_put_u32(skb, NCSI_ATTR_CHANNEL_ID, nc->id);
+ else
+ nla_put_u32(skb, NCSI_ATTR_CHANNEL_ID, NCSI_RESERVED_CHANNEL);
+
+ rc = nla_put(skb, NCSI_ATTR_DATA, nr->rsp->len, (void *)nr->rsp->data);
+ if (rc)
+ goto err;
+
+ genlmsg_end(skb, hdr);
+ return genlmsg_unicast(net, skb, nr->snd_portid);
+
+err:
+ kfree_skb(skb);
+ return rc;
+}
+
+int ncsi_send_netlink_timeout(struct ncsi_request *nr,
+ struct ncsi_package *np,
+ struct ncsi_channel *nc)
+{
+ struct sk_buff *skb;
+ struct net *net;
+ void *hdr;
+
+ skb = genlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
+ if (!skb)
+ return -ENOMEM;
+
+ hdr = genlmsg_put(skb, nr->snd_portid, nr->snd_seq,
+ &ncsi_genl_family, 0, NCSI_CMD_SEND_CMD);
+ if (!hdr) {
+ kfree_skb(skb);
+ return -EMSGSIZE;
+ }
+
+ net = dev_net(nr->cmd->dev);
+
+ nla_put_u32(skb, NCSI_ATTR_IFINDEX, nr->cmd->dev->ifindex);
+
+ if (np)
+ nla_put_u32(skb, NCSI_ATTR_PACKAGE_ID, np->id);
+ else
+ nla_put_u32(skb, NCSI_ATTR_PACKAGE_ID,
+ NCSI_PACKAGE_INDEX((((struct ncsi_pkt_hdr *)
+ nr->cmd->data)->channel)));
+
+ if (nc)
+ nla_put_u32(skb, NCSI_ATTR_CHANNEL_ID, nc->id);
+ else
+ nla_put_u32(skb, NCSI_ATTR_CHANNEL_ID, NCSI_RESERVED_CHANNEL);
+
+ genlmsg_end(skb, hdr);
+ return genlmsg_unicast(net, skb, nr->snd_portid);
+}
+
+int ncsi_send_netlink_err(struct net_device *dev,
+ u32 snd_seq,
+ u32 snd_portid,
+ struct nlmsghdr *nlhdr,
+ int err)
+{
+ struct sk_buff *skb;
+ struct nlmsghdr *nlh;
+ struct nlmsgerr *nle;
+ struct net *net;
+
+ skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
+ if (!skb)
+ return -ENOMEM;
+
+ net = dev_net(dev);
+
+ nlh = nlmsg_put(skb, snd_portid, snd_seq,
+ NLMSG_ERROR, sizeof(*nle), 0);
+ nle = (struct nlmsgerr *)nlmsg_data(nlh);
+ nle->error = err;
+ memcpy(&nle->msg, nlhdr, sizeof(*nlh));
+
+ nlmsg_end(skb, nlh);
+
+ return nlmsg_unicast(net->genl_sock, skb, snd_portid);
+}
+
static const struct genl_ops ncsi_ops[] = {
{
.cmd = NCSI_CMD_PKG_INFO,
@@ -386,6 +585,12 @@ static const struct genl_ops ncsi_ops[] = {
.doit = ncsi_clear_interface_nl,
.flags = GENL_ADMIN_PERM,
},
+ {
+ .cmd = NCSI_CMD_SEND_CMD,
+ .policy = ncsi_genl_policy,
+ .doit = ncsi_send_cmd_nl,
+ .flags = GENL_ADMIN_PERM,
+ },
};
static struct genl_family ncsi_genl_family __ro_after_init = {
diff --git a/net/ncsi/ncsi-netlink.h b/net/ncsi/ncsi-netlink.h
index 91a5c25..c4a4688 100644
--- a/net/ncsi/ncsi-netlink.h
+++ b/net/ncsi/ncsi-netlink.h
@@ -14,6 +14,18 @@
#include "internal.h"
+int ncsi_send_netlink_rsp(struct ncsi_request *nr,
+ struct ncsi_package *np,
+ struct ncsi_channel *nc);
+int ncsi_send_netlink_timeout(struct ncsi_request *nr,
+ struct ncsi_package *np,
+ struct ncsi_channel *nc);
+int ncsi_send_netlink_err(struct net_device *dev,
+ u32 snd_seq,
+ u32 snd_portid,
+ struct nlmsghdr *nlhdr,
+ int err);
+
int ncsi_init_netlink(struct net_device *dev);
int ncsi_unregister_netlink(struct net_device *dev);
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index d66b347..dd931d2 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -16,9 +16,11 @@
#include <net/ncsi.h>
#include <net/net_namespace.h>
#include <net/sock.h>
+#include <net/genetlink.h>
#include "internal.h"
#include "ncsi-pkt.h"
+#include "ncsi-netlink.h"
static int ncsi_validate_rsp_pkt(struct ncsi_request *nr,
unsigned short payload)
@@ -32,15 +34,25 @@ static int ncsi_validate_rsp_pkt(struct ncsi_request *nr,
* before calling this function.
*/
h = (struct ncsi_rsp_pkt_hdr *)skb_network_header(nr->rsp);
- if (h->common.revision != NCSI_PKT_REVISION)
+
+ if (h->common.revision != NCSI_PKT_REVISION) {
+ netdev_dbg(nr->ndp->ndev.dev,
+ "NCSI: unsupported header revision\n");
return -EINVAL;
- if (ntohs(h->common.length) != payload)
+ }
+ if (ntohs(h->common.length) != payload) {
+ netdev_dbg(nr->ndp->ndev.dev,
+ "NCSI: payload length mismatched\n");
return -EINVAL;
+ }
/* Check on code and reason */
if (ntohs(h->code) != NCSI_PKT_RSP_C_COMPLETED ||
- ntohs(h->reason) != NCSI_PKT_RSP_R_NO_ERROR)
- return -EINVAL;
+ ntohs(h->reason) != NCSI_PKT_RSP_R_NO_ERROR) {
+ netdev_dbg(nr->ndp->ndev.dev,
+ "NCSI: non zero response/reason code\n");
+ return -EPERM;
+ }
/* Validate checksum, which might be zeroes if the
* sender doesn't support checksum according to NCSI
@@ -52,8 +64,11 @@ static int ncsi_validate_rsp_pkt(struct ncsi_request *nr,
checksum = ncsi_calculate_checksum((unsigned char *)h,
sizeof(*h) + payload - 4);
- if (*pchecksum != htonl(checksum))
+
+ if (*pchecksum != htonl(checksum)) {
+ netdev_dbg(nr->ndp->ndev.dev, "NCSI: checksum mismatched\n");
return -EINVAL;
+ }
return 0;
}
@@ -941,6 +956,26 @@ static int ncsi_rsp_handler_gpuuid(struct ncsi_request *nr)
return 0;
}
+static int ncsi_rsp_handler_netlink(struct ncsi_request *nr)
+{
+ struct ncsi_rsp_pkt *rsp;
+ struct ncsi_dev_priv *ndp = nr->ndp;
+ struct ncsi_package *np;
+ struct ncsi_channel *nc;
+ int ret;
+
+ /* Find the package */
+ rsp = (struct ncsi_rsp_pkt *)skb_network_header(nr->rsp);
+ ncsi_find_package_and_channel(ndp, rsp->rsp.common.channel,
+ &np, &nc);
+ if (!np)
+ return -ENODEV;
+
+ ret = ncsi_send_netlink_rsp(nr, np, nc);
+
+ return ret;
+}
+
static struct ncsi_rsp_handler {
unsigned char type;
int payload;
@@ -1043,6 +1078,17 @@ int ncsi_rcv_rsp(struct sk_buff *skb, struct net_device *dev,
netdev_warn(ndp->ndev.dev,
"NCSI: 'bad' packet ignored for type 0x%x\n",
hdr->type);
+
+ if (nr->flags == NCSI_REQ_FLAG_NETLINK_DRIVEN) {
+ if (ret == -EPERM)
+ goto out_netlink;
+ else
+ ncsi_send_netlink_err(ndp->ndev.dev,
+ nr->snd_seq,
+ nr->snd_portid,
+ &nr->nlhdr,
+ ret);
+ }
goto out;
}
@@ -1052,6 +1098,17 @@ int ncsi_rcv_rsp(struct sk_buff *skb, struct net_device *dev,
netdev_err(ndp->ndev.dev,
"NCSI: Handler for packet type 0x%x returned %d\n",
hdr->type, ret);
+
+out_netlink:
+ if (nr->flags == NCSI_REQ_FLAG_NETLINK_DRIVEN) {
+ ret = ncsi_rsp_handler_netlink(nr);
+ if (ret) {
+ netdev_err(ndp->ndev.dev,
+ "NCSI: Netlink handler for packet type 0x%x returned %d\n",
+ hdr->type, ret);
+ }
+ }
+
out:
ncsi_free_request(nr);
return ret;
--
2.9.3
^ permalink raw reply related
* [PATCH net-next v6] net/ncsi: Extend NC-SI Netlink interface to allow user space to send NC-SI command
From: David Miller @ 2018-10-11 16:59 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <70bcb509b3f94eb4af186e604363770d@AUSX13MPS302.AMER.DELL.COM>
From: <Justin.Lee1@Dell.com>
Date: Thu, 11 Oct 2018 06:21:10 +0000
> @@ -366,6 +368,203 @@ static int ncsi_clear_interface_nl(struct sk_buff *msg, struct genl_info *info)
> return 0;
> }
>
> +static int ncsi_send_cmd_nl(struct sk_buff *msg, struct genl_info *info)
> +{
> + struct ncsi_dev_priv *ndp;
> +
> + struct ncsi_cmd_arg nca;
> + struct ncsi_pkt_hdr *hdr;
> +
> + u32 package_id, channel_id;
> + unsigned char *data;
> + int len, ret;
Please order all local variable declarations from longest to shortest
line. Please do not insert empty lines in the middle of a set of
variable declarations.
> +int ncsi_send_netlink_err(struct net_device *dev,
> + u32 snd_seq,
> + u32 snd_portid,
> + struct nlmsghdr *nlhdr,
> + int err)
> +{
> + struct sk_buff *skb;
> + struct nlmsghdr *nlh;
> + struct nlmsgerr *nle;
> + struct net *net;
Please order local variables from longest to shortest line.
> @@ -941,6 +956,26 @@ static int ncsi_rsp_handler_gpuuid(struct ncsi_request *nr)
> return 0;
> }
>
> +static int ncsi_rsp_handler_netlink(struct ncsi_request *nr)
> +{
> + struct ncsi_rsp_pkt *rsp;
> + struct ncsi_dev_priv *ndp = nr->ndp;
> + struct ncsi_package *np;
> + struct ncsi_channel *nc;
> + int ret;
Likewise.
^ permalink raw reply
* [PATCH net-next v7] net/ncsi: Extend NC-SI Netlink interface to allow user space to send NC-SI command
From: Justin.Lee1 @ 2018-10-11 18:07 UTC (permalink / raw)
To: linux-aspeed
The new command (NCSI_CMD_SEND_CMD) is added to allow user space application
to send NC-SI command to the network card.
Also, add a new attribute (NCSI_ATTR_DATA) for transferring request and response.
The work flow is as below.
Request:
User space application
-> Netlink interface (msg)
-> new Netlink handler - ncsi_send_cmd_nl()
-> ncsi_xmit_cmd()
Response:
Response received - ncsi_rcv_rsp()
-> internal response handler - ncsi_rsp_handler_xxx()
-> ncsi_rsp_handler_netlink()
-> ncsi_send_netlink_rsp ()
-> Netlink interface (msg)
-> user space application
Command timeout - ncsi_request_timeout()
-> ncsi_send_netlink_timeout ()
-> Netlink interface (msg with zero data length)
-> user space application
Error:
Error detected
-> ncsi_send_netlink_err ()
-> Netlink interface (err msg)
-> user space application
Signed-off-by: Justin Lee <justin.lee1@dell.com>
---
V7: Adjust the order of local variables from longest to shortest line.
V6: Add checking before accessing NCSI_ATTR_DATA attribute to avoid null-dereference.
V5: Update comments and debug message.
V4: Update comments and remove some debug message.
V3: Based on http://patchwork.ozlabs.org/patch/979688/ to remove the duplicated code.
V2: Remove non-related debug message and clean up the code.
include/uapi/linux/ncsi.h | 6 ++
net/ncsi/internal.h | 7 ++
net/ncsi/ncsi-cmd.c | 8 ++
net/ncsi/ncsi-manage.c | 16 ++++
net/ncsi/ncsi-netlink.c | 204 ++++++++++++++++++++++++++++++++++++++++++++++
net/ncsi/ncsi-netlink.h | 12 +++
net/ncsi/ncsi-rsp.c | 67 +++++++++++++--
7 files changed, 315 insertions(+), 5 deletions(-)
diff --git a/include/uapi/linux/ncsi.h b/include/uapi/linux/ncsi.h
index 4c292ec..0a26a55 100644
--- a/include/uapi/linux/ncsi.h
+++ b/include/uapi/linux/ncsi.h
@@ -23,6 +23,9 @@
* optionally the preferred NCSI_ATTR_CHANNEL_ID.
* @NCSI_CMD_CLEAR_INTERFACE: clear any preferred package/channel combination.
* Requires NCSI_ATTR_IFINDEX.
+ * @NCSI_CMD_SEND_CMD: send NC-SI command to network card.
+ * Requires NCSI_ATTR_IFINDEX, NCSI_ATTR_PACKAGE_ID
+ * and NCSI_ATTR_CHANNEL_ID.
* @NCSI_CMD_MAX: highest command number
*/
enum ncsi_nl_commands {
@@ -30,6 +33,7 @@ enum ncsi_nl_commands {
NCSI_CMD_PKG_INFO,
NCSI_CMD_SET_INTERFACE,
NCSI_CMD_CLEAR_INTERFACE,
+ NCSI_CMD_SEND_CMD,
__NCSI_CMD_AFTER_LAST,
NCSI_CMD_MAX = __NCSI_CMD_AFTER_LAST - 1
@@ -43,6 +47,7 @@ enum ncsi_nl_commands {
* @NCSI_ATTR_PACKAGE_LIST: nested array of NCSI_PKG_ATTR attributes
* @NCSI_ATTR_PACKAGE_ID: package ID
* @NCSI_ATTR_CHANNEL_ID: channel ID
+ * @NCSI_ATTR_DATA: command payload
* @NCSI_ATTR_MAX: highest attribute number
*/
enum ncsi_nl_attrs {
@@ -51,6 +56,7 @@ enum ncsi_nl_attrs {
NCSI_ATTR_PACKAGE_LIST,
NCSI_ATTR_PACKAGE_ID,
NCSI_ATTR_CHANNEL_ID,
+ NCSI_ATTR_DATA,
__NCSI_ATTR_AFTER_LAST,
NCSI_ATTR_MAX = __NCSI_ATTR_AFTER_LAST - 1
diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index 3d0a33b..13c9b5e 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -175,6 +175,8 @@ struct ncsi_package;
#define NCSI_RESERVED_CHANNEL 0x1f
#define NCSI_CHANNEL_INDEX(c) ((c) & ((1 << NCSI_PACKAGE_SHIFT) - 1))
#define NCSI_TO_CHANNEL(p, c) (((p) << NCSI_PACKAGE_SHIFT) | (c))
+#define NCSI_MAX_PACKAGE 8
+#define NCSI_MAX_CHANNEL 32
struct ncsi_channel {
unsigned char id;
@@ -220,11 +222,15 @@ struct ncsi_request {
bool used; /* Request that has been assigned */
unsigned int flags; /* NCSI request property */
#define NCSI_REQ_FLAG_EVENT_DRIVEN 1
+#define NCSI_REQ_FLAG_NETLINK_DRIVEN 2
struct ncsi_dev_priv *ndp; /* Associated NCSI device */
struct sk_buff *cmd; /* Associated NCSI command packet */
struct sk_buff *rsp; /* Associated NCSI response packet */
struct timer_list timer; /* Timer on waiting for response */
bool enabled; /* Time has been enabled or not */
+ u32 snd_seq; /* netlink sending sequence number */
+ u32 snd_portid; /* netlink portid of sender */
+ struct nlmsghdr nlhdr; /* netlink message header */
};
enum {
@@ -310,6 +316,7 @@ struct ncsi_cmd_arg {
unsigned int dwords[4];
};
unsigned char *data; /* NCSI OEM data */
+ struct genl_info *info; /* Netlink information */
};
extern struct list_head ncsi_dev_list;
diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c
index 82b7d92..356af47 100644
--- a/net/ncsi/ncsi-cmd.c
+++ b/net/ncsi/ncsi-cmd.c
@@ -17,6 +17,7 @@
#include <net/ncsi.h>
#include <net/net_namespace.h>
#include <net/sock.h>
+#include <net/genetlink.h>
#include "internal.h"
#include "ncsi-pkt.h"
@@ -346,6 +347,13 @@ int ncsi_xmit_cmd(struct ncsi_cmd_arg *nca)
if (!nr)
return -ENOMEM;
+ /* track netlink information */
+ if (nca->req_flags == NCSI_REQ_FLAG_NETLINK_DRIVEN) {
+ nr->snd_seq = nca->info->snd_seq;
+ nr->snd_portid = nca->info->snd_portid;
+ nr->nlhdr = *nca->info->nlhdr;
+ }
+
/* Prepare the packet */
nca->id = nr->id;
ret = nch->handler(nr->cmd, nca);
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 0912847..6aa0614 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -19,6 +19,7 @@
#include <net/addrconf.h>
#include <net/ipv6.h>
#include <net/if_inet6.h>
+#include <net/genetlink.h>
#include "internal.h"
#include "ncsi-pkt.h"
@@ -406,6 +407,9 @@ static void ncsi_request_timeout(struct timer_list *t)
{
struct ncsi_request *nr = from_timer(nr, t, timer);
struct ncsi_dev_priv *ndp = nr->ndp;
+ struct ncsi_cmd_pkt *cmd;
+ struct ncsi_package *np;
+ struct ncsi_channel *nc;
unsigned long flags;
/* If the request already had associated response,
@@ -419,6 +423,18 @@ static void ncsi_request_timeout(struct timer_list *t)
}
spin_unlock_irqrestore(&ndp->lock, flags);
+ if (nr->flags == NCSI_REQ_FLAG_NETLINK_DRIVEN) {
+ if (nr->cmd) {
+ /* Find the package */
+ cmd = (struct ncsi_cmd_pkt *)
+ skb_network_header(nr->cmd);
+ ncsi_find_package_and_channel(ndp,
+ cmd->cmd.common.channel,
+ &np, &nc);
+ ncsi_send_netlink_timeout(nr, np, nc);
+ }
+ }
+
/* Release the request */
ncsi_free_request(nr);
}
diff --git a/net/ncsi/ncsi-netlink.c b/net/ncsi/ncsi-netlink.c
index 45f33d6..9b48b9f 100644
--- a/net/ncsi/ncsi-netlink.c
+++ b/net/ncsi/ncsi-netlink.c
@@ -20,6 +20,7 @@
#include <uapi/linux/ncsi.h>
#include "internal.h"
+#include "ncsi-pkt.h"
#include "ncsi-netlink.h"
static struct genl_family ncsi_genl_family;
@@ -29,6 +30,7 @@ static const struct nla_policy ncsi_genl_policy[NCSI_ATTR_MAX + 1] = {
[NCSI_ATTR_PACKAGE_LIST] = { .type = NLA_NESTED },
[NCSI_ATTR_PACKAGE_ID] = { .type = NLA_U32 },
[NCSI_ATTR_CHANNEL_ID] = { .type = NLA_U32 },
+ [NCSI_ATTR_DATA] = { .type = NLA_BINARY, .len = 2048 },
};
static struct ncsi_dev_priv *ndp_from_ifindex(struct net *net, u32 ifindex)
@@ -366,6 +368,202 @@ static int ncsi_clear_interface_nl(struct sk_buff *msg, struct genl_info *info)
return 0;
}
+static int ncsi_send_cmd_nl(struct sk_buff *msg, struct genl_info *info)
+{
+ struct ncsi_dev_priv *ndp;
+ struct ncsi_pkt_hdr *hdr;
+ struct ncsi_cmd_arg nca;
+ unsigned char *data;
+ u32 package_id;
+ u32 channel_id;
+ int len, ret;
+
+ if (!info || !info->attrs) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ if (!info->attrs[NCSI_ATTR_IFINDEX]) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ if (!info->attrs[NCSI_ATTR_PACKAGE_ID]) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ if (!info->attrs[NCSI_ATTR_CHANNEL_ID]) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ if (!info->attrs[NCSI_ATTR_DATA]) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ ndp = ndp_from_ifindex(get_net(sock_net(msg->sk)),
+ nla_get_u32(info->attrs[NCSI_ATTR_IFINDEX]));
+ if (!ndp) {
+ ret = -ENODEV;
+ goto out;
+ }
+
+ package_id = nla_get_u32(info->attrs[NCSI_ATTR_PACKAGE_ID]);
+ channel_id = nla_get_u32(info->attrs[NCSI_ATTR_CHANNEL_ID]);
+
+ if (package_id >= NCSI_MAX_PACKAGE || channel_id >= NCSI_MAX_CHANNEL) {
+ ret = -ERANGE;
+ goto out_netlink;
+ }
+
+ len = nla_len(info->attrs[NCSI_ATTR_DATA]);
+ if (len < sizeof(struct ncsi_pkt_hdr)) {
+ netdev_info(ndp->ndev.dev, "NCSI: no command to send %u\n",
+ package_id);
+ ret = -EINVAL;
+ goto out_netlink;
+ } else {
+ data = (unsigned char *)nla_data(info->attrs[NCSI_ATTR_DATA]);
+ }
+
+ hdr = (struct ncsi_pkt_hdr *)data;
+
+ nca.ndp = ndp;
+ nca.package = (unsigned char)package_id;
+ nca.channel = (unsigned char)channel_id;
+ nca.type = hdr->type;
+ nca.req_flags = NCSI_REQ_FLAG_NETLINK_DRIVEN;
+ nca.info = info;
+ nca.payload = ntohs(hdr->length);
+ nca.data = data + sizeof(*hdr);
+
+ ret = ncsi_xmit_cmd(&nca);
+out_netlink:
+ if (ret != 0) {
+ netdev_err(ndp->ndev.dev,
+ "NCSI: Error %d sending command\n",
+ ret);
+ ncsi_send_netlink_err(ndp->ndev.dev,
+ info->snd_seq,
+ info->snd_portid,
+ info->nlhdr,
+ ret);
+ }
+out:
+ return ret;
+}
+
+int ncsi_send_netlink_rsp(struct ncsi_request *nr,
+ struct ncsi_package *np,
+ struct ncsi_channel *nc)
+{
+ struct sk_buff *skb;
+ struct net *net;
+ void *hdr;
+ int rc;
+
+ net = dev_net(nr->rsp->dev);
+
+ skb = genlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
+ if (!skb)
+ return -ENOMEM;
+
+ hdr = genlmsg_put(skb, nr->snd_portid, nr->snd_seq,
+ &ncsi_genl_family, 0, NCSI_CMD_SEND_CMD);
+ if (!hdr) {
+ kfree_skb(skb);
+ return -EMSGSIZE;
+ }
+
+ nla_put_u32(skb, NCSI_ATTR_IFINDEX, nr->rsp->dev->ifindex);
+ if (np)
+ nla_put_u32(skb, NCSI_ATTR_PACKAGE_ID, np->id);
+ if (nc)
+ nla_put_u32(skb, NCSI_ATTR_CHANNEL_ID, nc->id);
+ else
+ nla_put_u32(skb, NCSI_ATTR_CHANNEL_ID, NCSI_RESERVED_CHANNEL);
+
+ rc = nla_put(skb, NCSI_ATTR_DATA, nr->rsp->len, (void *)nr->rsp->data);
+ if (rc)
+ goto err;
+
+ genlmsg_end(skb, hdr);
+ return genlmsg_unicast(net, skb, nr->snd_portid);
+
+err:
+ kfree_skb(skb);
+ return rc;
+}
+
+int ncsi_send_netlink_timeout(struct ncsi_request *nr,
+ struct ncsi_package *np,
+ struct ncsi_channel *nc)
+{
+ struct sk_buff *skb;
+ struct net *net;
+ void *hdr;
+
+ skb = genlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
+ if (!skb)
+ return -ENOMEM;
+
+ hdr = genlmsg_put(skb, nr->snd_portid, nr->snd_seq,
+ &ncsi_genl_family, 0, NCSI_CMD_SEND_CMD);
+ if (!hdr) {
+ kfree_skb(skb);
+ return -EMSGSIZE;
+ }
+
+ net = dev_net(nr->cmd->dev);
+
+ nla_put_u32(skb, NCSI_ATTR_IFINDEX, nr->cmd->dev->ifindex);
+
+ if (np)
+ nla_put_u32(skb, NCSI_ATTR_PACKAGE_ID, np->id);
+ else
+ nla_put_u32(skb, NCSI_ATTR_PACKAGE_ID,
+ NCSI_PACKAGE_INDEX((((struct ncsi_pkt_hdr *)
+ nr->cmd->data)->channel)));
+
+ if (nc)
+ nla_put_u32(skb, NCSI_ATTR_CHANNEL_ID, nc->id);
+ else
+ nla_put_u32(skb, NCSI_ATTR_CHANNEL_ID, NCSI_RESERVED_CHANNEL);
+
+ genlmsg_end(skb, hdr);
+ return genlmsg_unicast(net, skb, nr->snd_portid);
+}
+
+int ncsi_send_netlink_err(struct net_device *dev,
+ u32 snd_seq,
+ u32 snd_portid,
+ struct nlmsghdr *nlhdr,
+ int err)
+{
+ struct nlmsghdr *nlh;
+ struct nlmsgerr *nle;
+ struct sk_buff *skb;
+ struct net *net;
+
+ skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
+ if (!skb)
+ return -ENOMEM;
+
+ net = dev_net(dev);
+
+ nlh = nlmsg_put(skb, snd_portid, snd_seq,
+ NLMSG_ERROR, sizeof(*nle), 0);
+ nle = (struct nlmsgerr *)nlmsg_data(nlh);
+ nle->error = err;
+ memcpy(&nle->msg, nlhdr, sizeof(*nlh));
+
+ nlmsg_end(skb, nlh);
+
+ return nlmsg_unicast(net->genl_sock, skb, snd_portid);
+}
+
static const struct genl_ops ncsi_ops[] = {
{
.cmd = NCSI_CMD_PKG_INFO,
@@ -386,6 +584,12 @@ static const struct genl_ops ncsi_ops[] = {
.doit = ncsi_clear_interface_nl,
.flags = GENL_ADMIN_PERM,
},
+ {
+ .cmd = NCSI_CMD_SEND_CMD,
+ .policy = ncsi_genl_policy,
+ .doit = ncsi_send_cmd_nl,
+ .flags = GENL_ADMIN_PERM,
+ },
};
static struct genl_family ncsi_genl_family __ro_after_init = {
diff --git a/net/ncsi/ncsi-netlink.h b/net/ncsi/ncsi-netlink.h
index 91a5c25..c4a4688 100644
--- a/net/ncsi/ncsi-netlink.h
+++ b/net/ncsi/ncsi-netlink.h
@@ -14,6 +14,18 @@
#include "internal.h"
+int ncsi_send_netlink_rsp(struct ncsi_request *nr,
+ struct ncsi_package *np,
+ struct ncsi_channel *nc);
+int ncsi_send_netlink_timeout(struct ncsi_request *nr,
+ struct ncsi_package *np,
+ struct ncsi_channel *nc);
+int ncsi_send_netlink_err(struct net_device *dev,
+ u32 snd_seq,
+ u32 snd_portid,
+ struct nlmsghdr *nlhdr,
+ int err);
+
int ncsi_init_netlink(struct net_device *dev);
int ncsi_unregister_netlink(struct net_device *dev);
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index d66b347..85fa59a 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -16,9 +16,11 @@
#include <net/ncsi.h>
#include <net/net_namespace.h>
#include <net/sock.h>
+#include <net/genetlink.h>
#include "internal.h"
#include "ncsi-pkt.h"
+#include "ncsi-netlink.h"
static int ncsi_validate_rsp_pkt(struct ncsi_request *nr,
unsigned short payload)
@@ -32,15 +34,25 @@ static int ncsi_validate_rsp_pkt(struct ncsi_request *nr,
* before calling this function.
*/
h = (struct ncsi_rsp_pkt_hdr *)skb_network_header(nr->rsp);
- if (h->common.revision != NCSI_PKT_REVISION)
+
+ if (h->common.revision != NCSI_PKT_REVISION) {
+ netdev_dbg(nr->ndp->ndev.dev,
+ "NCSI: unsupported header revision\n");
return -EINVAL;
- if (ntohs(h->common.length) != payload)
+ }
+ if (ntohs(h->common.length) != payload) {
+ netdev_dbg(nr->ndp->ndev.dev,
+ "NCSI: payload length mismatched\n");
return -EINVAL;
+ }
/* Check on code and reason */
if (ntohs(h->code) != NCSI_PKT_RSP_C_COMPLETED ||
- ntohs(h->reason) != NCSI_PKT_RSP_R_NO_ERROR)
- return -EINVAL;
+ ntohs(h->reason) != NCSI_PKT_RSP_R_NO_ERROR) {
+ netdev_dbg(nr->ndp->ndev.dev,
+ "NCSI: non zero response/reason code\n");
+ return -EPERM;
+ }
/* Validate checksum, which might be zeroes if the
* sender doesn't support checksum according to NCSI
@@ -52,8 +64,11 @@ static int ncsi_validate_rsp_pkt(struct ncsi_request *nr,
checksum = ncsi_calculate_checksum((unsigned char *)h,
sizeof(*h) + payload - 4);
- if (*pchecksum != htonl(checksum))
+
+ if (*pchecksum != htonl(checksum)) {
+ netdev_dbg(nr->ndp->ndev.dev, "NCSI: checksum mismatched\n");
return -EINVAL;
+ }
return 0;
}
@@ -941,6 +956,26 @@ static int ncsi_rsp_handler_gpuuid(struct ncsi_request *nr)
return 0;
}
+static int ncsi_rsp_handler_netlink(struct ncsi_request *nr)
+{
+ struct ncsi_dev_priv *ndp = nr->ndp;
+ struct ncsi_rsp_pkt *rsp;
+ struct ncsi_package *np;
+ struct ncsi_channel *nc;
+ int ret;
+
+ /* Find the package */
+ rsp = (struct ncsi_rsp_pkt *)skb_network_header(nr->rsp);
+ ncsi_find_package_and_channel(ndp, rsp->rsp.common.channel,
+ &np, &nc);
+ if (!np)
+ return -ENODEV;
+
+ ret = ncsi_send_netlink_rsp(nr, np, nc);
+
+ return ret;
+}
+
static struct ncsi_rsp_handler {
unsigned char type;
int payload;
@@ -1043,6 +1078,17 @@ int ncsi_rcv_rsp(struct sk_buff *skb, struct net_device *dev,
netdev_warn(ndp->ndev.dev,
"NCSI: 'bad' packet ignored for type 0x%x\n",
hdr->type);
+
+ if (nr->flags == NCSI_REQ_FLAG_NETLINK_DRIVEN) {
+ if (ret == -EPERM)
+ goto out_netlink;
+ else
+ ncsi_send_netlink_err(ndp->ndev.dev,
+ nr->snd_seq,
+ nr->snd_portid,
+ &nr->nlhdr,
+ ret);
+ }
goto out;
}
@@ -1052,6 +1098,17 @@ int ncsi_rcv_rsp(struct sk_buff *skb, struct net_device *dev,
netdev_err(ndp->ndev.dev,
"NCSI: Handler for packet type 0x%x returned %d\n",
hdr->type, ret);
+
+out_netlink:
+ if (nr->flags == NCSI_REQ_FLAG_NETLINK_DRIVEN) {
+ ret = ncsi_rsp_handler_netlink(nr);
+ if (ret) {
+ netdev_err(ndp->ndev.dev,
+ "NCSI: Netlink handler for packet type 0x%x returned %d\n",
+ hdr->type, ret);
+ }
+ }
+
out:
ncsi_free_request(nr);
return ret;
--
2.9.3
^ permalink raw reply related
* [PATCH i2c-next v7 1/5] dt-bindings: i2c: Add 'bus-timeout-ms' and '#retries' properties as common optional
From: Rob Herring @ 2018-10-11 22:11 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20181005214507.26315-2-jae.hyun.yoo@linux.intel.com>
On Fri, 5 Oct 2018 14:45:03 -0700, Jae Hyun Yoo wrote:
> This commit adds 'bus-timeout-ms' and '#retries' properties as
> common optional properties that can be used for setting 'timeout'
> and 'retries' values of 'struct i2c_adapter'. With this patch, the
> bus timeout value and the master transfer retries count can be set
> through these properties at the registration time of an adapter.
> Still the values can be set by I2C_TIMEOUT and I2C_RETRIES ioctls
> on cdev at runtime too.
>
> These properties may not be supported by all drivers. However, if
> a driver wants to support one of them, it should adapt the
> bindings in this document.
>
> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
> ---
> Documentation/devicetree/bindings/i2c/i2c.txt | 6 ++++++
> 1 file changed, 6 insertions(+)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* [PATCH i2c-next v7 3/5] dt-bindings: i2c: aspeed: Add 'bus-timeout-ms' property as an optional property
From: Rob Herring @ 2018-10-11 22:12 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20181005214507.26315-4-jae.hyun.yoo@linux.intel.com>
On Fri, 5 Oct 2018 14:45:05 -0700, Jae Hyun Yoo wrote:
> This commit adds 'bus-timeout-ms' property as an optional property
> which can be used for setting the bus timeout value of an adapter.
> With this patch, the bus timeout value can be set through this
> property at the probing time of this module. Still the bus timeout
> value can be set by an I2C_TIMEOUT ioctl on cdev at runtime too.
>
> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
> ---
> Documentation/devicetree/bindings/i2c/i2c-aspeed.txt | 3 +++
> 1 file changed, 3 insertions(+)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* [PATCH v2 1/2] net/ncsi: Add NCSI Broadcom OEM command
From: Vijay Khemka @ 2018-10-11 22:12 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <7893ec3aa673b6009d00df1646b2820be2fc33a6.camel@mendozajonas.com>
Thanks Sam,
I will take care of this and generate new patch.
Regards
-Vijay
?On 10/9/18, 9:40 PM, "Samuel Mendoza-Jonas" <sam@mendozajonas.com> wrote:
On Tue, 2018-10-09 at 10:48 -0700, Vijay Khemka wrote:
> This patch adds OEM Broadcom commands and response handling. It also
> defines OEM Get MAC Address handler to get and configure the device.
>
> ncsi_oem_gma_handler_bcm: This handler send NCSI broadcom command for
> getting mac address.
> ncsi_rsp_handler_oem_bcm: This handles response received for all
> broadcom OEM commands.
> ncsi_rsp_handler_oem_bcm_gma: This handles get mac address response and
> set it to device.
>
> Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
> ---
> net/ncsi/Kconfig | 6 ++++
> net/ncsi/internal.h | 8 +++++
> net/ncsi/ncsi-manage.c | 70 +++++++++++++++++++++++++++++++++++++++++-
> net/ncsi/ncsi-pkt.h | 8 +++++
> net/ncsi/ncsi-rsp.c | 40 +++++++++++++++++++++++-
> 5 files changed, 130 insertions(+), 2 deletions(-)
>
<snip>
> static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
> {
> struct ncsi_dev *nd = &ndp->ndev;
> @@ -643,9 +676,10 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
> struct ncsi_channel *nc = ndp->active_channel;
> struct ncsi_channel *hot_nc = NULL;
> struct ncsi_cmd_arg nca;
> + struct ncsi_oem_gma_handler *nch = NULL;
> unsigned char index;
> unsigned long flags;
> - int ret;
> + int ret, i;
>
I just noticed that if CONFIG_NCSI_OEM_CMD_GET_MAC is not set then this
generates unused variable warnings for i and nch. Otherwise all looking good!
Regards,
Sam
^ permalink raw reply
* [PATCH i2c-next v7 1/5] dt-bindings: i2c: Add 'bus-timeout-ms' and '#retries' properties as common optional
From: Jae Hyun Yoo @ 2018-10-11 22:46 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20181011221148.GA31195@bogus>
On 10/11/2018 3:11 PM, Rob Herring wrote:
>
> Reviewed-by: Rob Herring <robh@kernel.org>
>
Thanks for the review, Rob!
^ permalink raw reply
* [PATCH i2c-next v7 3/5] dt-bindings: i2c: aspeed: Add 'bus-timeout-ms' property as an optional property
From: Jae Hyun Yoo @ 2018-10-11 22:47 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20181011221219.GA31991@bogus>
On 10/11/2018 3:12 PM, Rob Herring wrote:
>
> Reviewed-by: Rob Herring <robh@kernel.org>
>
Thanks much for your review, Rob!
-Jae
^ permalink raw reply
* [PATCH net-next v3 1/2] net/ncsi: Add NCSI Broadcom OEM command
From: Vijay Khemka @ 2018-10-11 23:05 UTC (permalink / raw)
To: linux-aspeed
This patch adds OEM Broadcom commands and response handling. It also
defines OEM Get MAC Address handler to get and configure the device.
ncsi_oem_gma_handler_bcm: This handler send NCSI broadcom command for
getting mac address.
ncsi_rsp_handler_oem_bcm: This handles response received for all
broadcom OEM commands.
ncsi_rsp_handler_oem_bcm_gma: This handles get mac address response and
set it to device.
Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
---
net/ncsi/Kconfig | 6 ++++
net/ncsi/internal.h | 8 +++++
net/ncsi/ncsi-manage.c | 70 ++++++++++++++++++++++++++++++++++++++++++
net/ncsi/ncsi-pkt.h | 8 +++++
net/ncsi/ncsi-rsp.c | 40 +++++++++++++++++++++++-
5 files changed, 131 insertions(+), 1 deletion(-)
diff --git a/net/ncsi/Kconfig b/net/ncsi/Kconfig
index 08a8a6031fd7..7f2b46108a24 100644
--- a/net/ncsi/Kconfig
+++ b/net/ncsi/Kconfig
@@ -10,3 +10,9 @@ config NET_NCSI
support. Enable this only if your system connects to a network
device via NCSI and the ethernet driver you're using supports
the protocol explicitly.
+config NCSI_OEM_CMD_GET_MAC
+ bool "Get NCSI OEM MAC Address"
+ depends on NET_NCSI
+ ---help---
+ This allows to get MAC address from NCSI firmware and set them back to
+ controller.
diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index 3d0a33b874f5..45883b32790e 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -71,6 +71,13 @@ enum {
/* OEM Vendor Manufacture ID */
#define NCSI_OEM_MFR_MLX_ID 0x8119
#define NCSI_OEM_MFR_BCM_ID 0x113d
+/* Broadcom specific OEM Command */
+#define NCSI_OEM_BCM_CMD_GMA 0x01 /* CMD ID for Get MAC */
+/* OEM Command payload lengths*/
+#define NCSI_OEM_BCM_CMD_GMA_LEN 12
+/* Mac address offset in OEM response */
+#define BCM_MAC_ADDR_OFFSET 28
+
struct ncsi_channel_version {
u32 version; /* Supported BCD encoded NCSI version */
@@ -240,6 +247,7 @@ enum {
ncsi_dev_state_probe_dp,
ncsi_dev_state_config_sp = 0x0301,
ncsi_dev_state_config_cis,
+ ncsi_dev_state_config_oem_gma,
ncsi_dev_state_config_clear_vids,
ncsi_dev_state_config_svf,
ncsi_dev_state_config_ev,
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 091284760d21..75504ccd1b95 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -635,6 +635,39 @@ static int set_one_vid(struct ncsi_dev_priv *ndp, struct ncsi_channel *nc,
return 0;
}
+#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC)
+
+/* NCSI OEM Command APIs */
+static void ncsi_oem_gma_handler_bcm(struct ncsi_cmd_arg *nca)
+{
+ int ret = 0;
+ unsigned char data[NCSI_OEM_BCM_CMD_GMA_LEN];
+
+ nca->payload = NCSI_OEM_BCM_CMD_GMA_LEN;
+
+ memset(data, 0, NCSI_OEM_BCM_CMD_GMA_LEN);
+ *(unsigned int *)data = ntohl(NCSI_OEM_MFR_BCM_ID);
+ data[5] = NCSI_OEM_BCM_CMD_GMA;
+
+ nca->data = data;
+
+ ret = ncsi_xmit_cmd(nca);
+ if (ret)
+ netdev_err(nca->ndp->ndev.dev,
+ "NCSI: Failed to transmit cmd 0x%x during configure\n",
+ nca->type);
+}
+
+/* OEM Command handlers initialization */
+static struct ncsi_oem_gma_handler {
+ unsigned int mfr_id;
+ void (*handler)(struct ncsi_cmd_arg *nca);
+} ncsi_oem_gma_handlers[] = {
+ { NCSI_OEM_MFR_BCM_ID, ncsi_oem_gma_handler_bcm }
+};
+
+#endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */
+
static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
{
struct ncsi_dev *nd = &ndp->ndev;
@@ -685,6 +718,43 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
goto error;
}
+#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC)
+ nd->state = ncsi_dev_state_config_oem_gma;
+ break;
+ case ncsi_dev_state_config_oem_gma:
+ nca.type = NCSI_PKT_CMD_OEM;
+ nca.package = np->id;
+ nca.channel = nc->id;
+ ndp->pending_req_num = 1;
+
+ /* Check for manufacturer id and Find the handler */
+ struct ncsi_oem_gma_handler *nch = NULL;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(ncsi_oem_gma_handlers); i++) {
+ if (ncsi_oem_gma_handlers[i].mfr_id ==
+ nc->version.mf_id) {
+ if (ncsi_oem_gma_handlers[i].handler)
+ nch = &ncsi_oem_gma_handlers[i];
+ else
+ nch = NULL;
+
+ break;
+ }
+ }
+
+ if (!nch) {
+ netdev_err(ndp->ndev.dev, "No handler available for GMA with MFR-ID (0x%x)\n",
+ nc->version.mf_id);
+ nd->state = ncsi_dev_state_config_clear_vids;
+ schedule_work(&ndp->work);
+ break;
+ }
+
+ /* Get Mac address from NCSI device */
+ nch->handler(&nca);
+#endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */
+
nd->state = ncsi_dev_state_config_clear_vids;
break;
case ncsi_dev_state_config_clear_vids:
diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h
index 0f2087c8d42a..4d3f06be38bd 100644
--- a/net/ncsi/ncsi-pkt.h
+++ b/net/ncsi/ncsi-pkt.h
@@ -165,6 +165,14 @@ struct ncsi_rsp_oem_pkt {
unsigned char data[]; /* Payload data */
};
+/* Broadcom Response Data */
+struct ncsi_rsp_oem_bcm_pkt {
+ unsigned char ver; /* Payload Version */
+ unsigned char type; /* OEM Command type */
+ __be16 len; /* Payload Length */
+ unsigned char data[]; /* Cmd specific Data */
+};
+
/* Get Link Status */
struct ncsi_rsp_gls_pkt {
struct ncsi_rsp_pkt_hdr rsp; /* Response header */
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index d66b34749027..31672e967db2 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -596,12 +596,50 @@ static int ncsi_rsp_handler_snfc(struct ncsi_request *nr)
return 0;
}
+/* Response handler for Broadcom command Get Mac Address */
+static int ncsi_rsp_handler_oem_bcm_gma(struct ncsi_request *nr)
+{
+ struct ncsi_rsp_oem_pkt *rsp;
+ struct ncsi_dev_priv *ndp = nr->ndp;
+ struct net_device *ndev = ndp->ndev.dev;
+ int ret = 0;
+ const struct net_device_ops *ops = ndev->netdev_ops;
+ struct sockaddr saddr;
+
+ /* Get the response header */
+ rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
+
+ saddr.sa_family = ndev->type;
+ ndev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
+ memcpy(saddr.sa_data, &rsp->data[BCM_MAC_ADDR_OFFSET], ETH_ALEN);
+ ret = ops->ndo_set_mac_address(ndev, &saddr);
+ if (ret < 0)
+ netdev_warn(ndev, "NCSI: Writing mac address to device failed\n");
+
+ return ret;
+}
+
+/* Response handler for Broadcom card */
+static int ncsi_rsp_handler_oem_bcm(struct ncsi_request *nr)
+{
+ struct ncsi_rsp_oem_pkt *rsp;
+ struct ncsi_rsp_oem_bcm_pkt *bcm;
+
+ /* Get the response header */
+ rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
+ bcm = (struct ncsi_rsp_oem_bcm_pkt *)(rsp->data);
+
+ if (bcm->type == NCSI_OEM_BCM_CMD_GMA)
+ return ncsi_rsp_handler_oem_bcm_gma(nr);
+ return 0;
+}
+
static struct ncsi_rsp_oem_handler {
unsigned int mfr_id;
int (*handler)(struct ncsi_request *nr);
} ncsi_rsp_oem_handlers[] = {
{ NCSI_OEM_MFR_MLX_ID, NULL },
- { NCSI_OEM_MFR_BCM_ID, NULL }
+ { NCSI_OEM_MFR_BCM_ID, ncsi_rsp_handler_oem_bcm }
};
/* Response handler for OEM command */
--
2.17.1
^ permalink raw reply related
* [PATCH net-next v3 2/2] net/ncsi: Add NCSI Mellanox OEM command
From: Vijay Khemka @ 2018-10-11 23:05 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20181011230518.3204700-1-vijaykhemka@fb.com>
This patch adds OEM Mellanox commands and response handling. It also
defines OEM Get MAC Address handler to get and configure the device.
ncsi_oem_gma_handler_mlx: This handler send NCSI mellanox command for
getting mac address.
ncsi_rsp_handler_oem_mlx: This handles response received for all
mellanox OEM commands.
ncsi_rsp_handler_oem_mlx_gma: This handles get mac address response and
set it to device.
Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
---
net/ncsi/internal.h | 5 +++++
net/ncsi/ncsi-manage.c | 24 +++++++++++++++++++++++-
net/ncsi/ncsi-pkt.h | 9 +++++++++
net/ncsi/ncsi-rsp.c | 41 ++++++++++++++++++++++++++++++++++++++++-
4 files changed, 77 insertions(+), 2 deletions(-)
diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index 45883b32790e..d4558628a991 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -73,10 +73,15 @@ enum {
#define NCSI_OEM_MFR_BCM_ID 0x113d
/* Broadcom specific OEM Command */
#define NCSI_OEM_BCM_CMD_GMA 0x01 /* CMD ID for Get MAC */
+/* Mellanox specific OEM Command */
+#define NCSI_OEM_MLX_CMD_GMA 0x00 /* CMD ID for Get MAC */
+#define NCSI_OEM_MLX_CMD_GMA_PARAM 0x1b /* Parameter for GMA */
/* OEM Command payload lengths*/
#define NCSI_OEM_BCM_CMD_GMA_LEN 12
+#define NCSI_OEM_MLX_CMD_GMA_LEN 8
/* Mac address offset in OEM response */
#define BCM_MAC_ADDR_OFFSET 28
+#define MLX_MAC_ADDR_OFFSET 8
struct ncsi_channel_version {
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 75504ccd1b95..9306fa464190 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -658,12 +658,34 @@ static void ncsi_oem_gma_handler_bcm(struct ncsi_cmd_arg *nca)
nca->type);
}
+static void ncsi_oem_gma_handler_mlx(struct ncsi_cmd_arg *nca)
+{
+ int ret = 0;
+ unsigned char data[NCSI_OEM_MLX_CMD_GMA_LEN];
+
+ nca->payload = NCSI_OEM_MLX_CMD_GMA_LEN;
+
+ memset(data, 0, NCSI_OEM_MLX_CMD_GMA_LEN);
+ *(unsigned int *)data = ntohl(NCSI_OEM_MFR_MLX_ID);
+ data[5] = NCSI_OEM_MLX_CMD_GMA;
+ data[6] = NCSI_OEM_MLX_CMD_GMA_PARAM;
+
+ nca->data = data;
+
+ ret = ncsi_xmit_cmd(nca);
+ if (ret)
+ netdev_err(nca->ndp->ndev.dev,
+ "NCSI: Failed to transmit cmd 0x%x during configure\n",
+ nca->type);
+}
+
/* OEM Command handlers initialization */
static struct ncsi_oem_gma_handler {
unsigned int mfr_id;
void (*handler)(struct ncsi_cmd_arg *nca);
} ncsi_oem_gma_handlers[] = {
- { NCSI_OEM_MFR_BCM_ID, ncsi_oem_gma_handler_bcm }
+ { NCSI_OEM_MFR_BCM_ID, ncsi_oem_gma_handler_bcm },
+ { NCSI_OEM_MFR_MLX_ID, ncsi_oem_gma_handler_mlx }
};
#endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */
diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h
index 4d3f06be38bd..2a6d83a596c9 100644
--- a/net/ncsi/ncsi-pkt.h
+++ b/net/ncsi/ncsi-pkt.h
@@ -165,6 +165,15 @@ struct ncsi_rsp_oem_pkt {
unsigned char data[]; /* Payload data */
};
+/* Mellanox Response Data */
+struct ncsi_rsp_oem_mlx_pkt {
+ unsigned char cmd_rev; /* Command Revision */
+ unsigned char cmd; /* Command ID */
+ unsigned char param; /* Parameter */
+ unsigned char optional; /* Optional data */
+ unsigned char data[]; /* Data */
+};
+
/* Broadcom Response Data */
struct ncsi_rsp_oem_bcm_pkt {
unsigned char ver; /* Payload Version */
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index 31672e967db2..5158836796ce 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -596,6 +596,45 @@ static int ncsi_rsp_handler_snfc(struct ncsi_request *nr)
return 0;
}
+/* Response handler for Mellanox command Get Mac Address */
+static int ncsi_rsp_handler_oem_mlx_gma(struct ncsi_request *nr)
+{
+ struct ncsi_rsp_oem_pkt *rsp;
+ struct ncsi_dev_priv *ndp = nr->ndp;
+ struct net_device *ndev = ndp->ndev.dev;
+ int ret = 0;
+ const struct net_device_ops *ops = ndev->netdev_ops;
+ struct sockaddr saddr;
+
+ /* Get the response header */
+ rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
+
+ saddr.sa_family = ndev->type;
+ ndev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
+ memcpy(saddr.sa_data, &rsp->data[MLX_MAC_ADDR_OFFSET], ETH_ALEN);
+ ret = ops->ndo_set_mac_address(ndev, &saddr);
+ if (ret < 0)
+ netdev_warn(ndev, "NCSI: Writing mac address to device failed\n");
+
+ return ret;
+}
+
+/* Response handler for Mellanox card */
+static int ncsi_rsp_handler_oem_mlx(struct ncsi_request *nr)
+{
+ struct ncsi_rsp_oem_pkt *rsp;
+ struct ncsi_rsp_oem_mlx_pkt *mlx;
+
+ /* Get the response header */
+ rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
+ mlx = (struct ncsi_rsp_oem_mlx_pkt *)(rsp->data);
+
+ if (mlx->cmd == NCSI_OEM_MLX_CMD_GMA &&
+ mlx->param == NCSI_OEM_MLX_CMD_GMA_PARAM)
+ return ncsi_rsp_handler_oem_mlx_gma(nr);
+ return 0;
+}
+
/* Response handler for Broadcom command Get Mac Address */
static int ncsi_rsp_handler_oem_bcm_gma(struct ncsi_request *nr)
{
@@ -638,7 +677,7 @@ static struct ncsi_rsp_oem_handler {
unsigned int mfr_id;
int (*handler)(struct ncsi_request *nr);
} ncsi_rsp_oem_handlers[] = {
- { NCSI_OEM_MFR_MLX_ID, NULL },
+ { NCSI_OEM_MFR_MLX_ID, ncsi_rsp_handler_oem_mlx },
{ NCSI_OEM_MFR_BCM_ID, ncsi_rsp_handler_oem_bcm }
};
--
2.17.1
^ permalink raw reply related
* [PATCH net-next v3 1/2] net/ncsi: Add NCSI Broadcom OEM command
From: Justin.Lee1 @ 2018-10-11 23:21 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20181011230518.3204700-1-vijaykhemka@fb.com>
Hi Vijay,
Please order all local variable declarations from longest to shortest
line as instructed by Dave.
Thanks,
Justin
> This patch adds OEM Broadcom commands and response handling. It also
> defines OEM Get MAC Address handler to get and configure the device.
>
> ncsi_oem_gma_handler_bcm: This handler send NCSI broadcom command for
> getting mac address.
> ncsi_rsp_handler_oem_bcm: This handles response received for all
> broadcom OEM commands.
> ncsi_rsp_handler_oem_bcm_gma: This handles get mac address response and
> set it to device.
>
> Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
> ---
> net/ncsi/Kconfig | 6 ++++
> net/ncsi/internal.h | 8 +++++
> net/ncsi/ncsi-manage.c | 70 ++++++++++++++++++++++++++++++++++++++++++
> net/ncsi/ncsi-pkt.h | 8 +++++
> net/ncsi/ncsi-rsp.c | 40 +++++++++++++++++++++++-
> 5 files changed, 131 insertions(+), 1 deletion(-)
>
> diff --git a/net/ncsi/Kconfig b/net/ncsi/Kconfig
> index 08a8a6031fd7..7f2b46108a24 100644
> --- a/net/ncsi/Kconfig
> +++ b/net/ncsi/Kconfig
> @@ -10,3 +10,9 @@ config NET_NCSI
> support. Enable this only if your system connects to a network
> device via NCSI and the ethernet driver you're using supports
> the protocol explicitly.
> +config NCSI_OEM_CMD_GET_MAC
> + bool "Get NCSI OEM MAC Address"
> + depends on NET_NCSI
> + ---help---
> + This allows to get MAC address from NCSI firmware and set them back to
> + controller.
> diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
> index 3d0a33b874f5..45883b32790e 100644
> --- a/net/ncsi/internal.h
> +++ b/net/ncsi/internal.h
> @@ -71,6 +71,13 @@ enum {
> /* OEM Vendor Manufacture ID */
> #define NCSI_OEM_MFR_MLX_ID 0x8119
> #define NCSI_OEM_MFR_BCM_ID 0x113d
> +/* Broadcom specific OEM Command */
> +#define NCSI_OEM_BCM_CMD_GMA 0x01 /* CMD ID for Get MAC */
> +/* OEM Command payload lengths*/
> +#define NCSI_OEM_BCM_CMD_GMA_LEN 12
> +/* Mac address offset in OEM response */
> +#define BCM_MAC_ADDR_OFFSET 28
> +
>
> struct ncsi_channel_version {
> u32 version; /* Supported BCD encoded NCSI version */
> @@ -240,6 +247,7 @@ enum {
> ncsi_dev_state_probe_dp,
> ncsi_dev_state_config_sp = 0x0301,
> ncsi_dev_state_config_cis,
> + ncsi_dev_state_config_oem_gma,
> ncsi_dev_state_config_clear_vids,
> ncsi_dev_state_config_svf,
> ncsi_dev_state_config_ev,
> diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
> index 091284760d21..75504ccd1b95 100644
> --- a/net/ncsi/ncsi-manage.c
> +++ b/net/ncsi/ncsi-manage.c
> @@ -635,6 +635,39 @@ static int set_one_vid(struct ncsi_dev_priv *ndp, struct ncsi_channel *nc,
> return 0;
> }
>
> +#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC)
> +
> +/* NCSI OEM Command APIs */
> +static void ncsi_oem_gma_handler_bcm(struct ncsi_cmd_arg *nca)
> +{
> + int ret = 0;
> + unsigned char data[NCSI_OEM_BCM_CMD_GMA_LEN];
> +
> + nca->payload = NCSI_OEM_BCM_CMD_GMA_LEN;
> +
> + memset(data, 0, NCSI_OEM_BCM_CMD_GMA_LEN);
> + *(unsigned int *)data = ntohl(NCSI_OEM_MFR_BCM_ID);
> + data[5] = NCSI_OEM_BCM_CMD_GMA;
> +
> + nca->data = data;
> +
> + ret = ncsi_xmit_cmd(nca);
> + if (ret)
> + netdev_err(nca->ndp->ndev.dev,
> + "NCSI: Failed to transmit cmd 0x%x during configure\n",
> + nca->type);
> +}
> +
> +/* OEM Command handlers initialization */
> +static struct ncsi_oem_gma_handler {
> + unsigned int mfr_id;
> + void (*handler)(struct ncsi_cmd_arg *nca);
> +} ncsi_oem_gma_handlers[] = {
> + { NCSI_OEM_MFR_BCM_ID, ncsi_oem_gma_handler_bcm }
> +};
> +
> +#endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */
> +
> static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
> {
> struct ncsi_dev *nd = &ndp->ndev;
> @@ -685,6 +718,43 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
> goto error;
> }
>
> +#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC)
> + nd->state = ncsi_dev_state_config_oem_gma;
> + break;
> + case ncsi_dev_state_config_oem_gma:
> + nca.type = NCSI_PKT_CMD_OEM;
> + nca.package = np->id;
> + nca.channel = nc->id;
> + ndp->pending_req_num = 1;
> +
> + /* Check for manufacturer id and Find the handler */
> + struct ncsi_oem_gma_handler *nch = NULL;
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(ncsi_oem_gma_handlers); i++) {
> + if (ncsi_oem_gma_handlers[i].mfr_id ==
> + nc->version.mf_id) {
> + if (ncsi_oem_gma_handlers[i].handler)
> + nch = &ncsi_oem_gma_handlers[i];
> + else
> + nch = NULL;
> +
> + break;
> + }
> + }
> +
> + if (!nch) {
> + netdev_err(ndp->ndev.dev, "No handler available for GMA with MFR-ID (0x%x)\n",
> + nc->version.mf_id);
> + nd->state = ncsi_dev_state_config_clear_vids;
> + schedule_work(&ndp->work);
> + break;
> + }
> +
> + /* Get Mac address from NCSI device */
> + nch->handler(&nca);
> +#endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */
> +
> nd->state = ncsi_dev_state_config_clear_vids;
> break;
> case ncsi_dev_state_config_clear_vids:
> diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h
> index 0f2087c8d42a..4d3f06be38bd 100644
> --- a/net/ncsi/ncsi-pkt.h
> +++ b/net/ncsi/ncsi-pkt.h
> @@ -165,6 +165,14 @@ struct ncsi_rsp_oem_pkt {
> unsigned char data[]; /* Payload data */
> };
>
> +/* Broadcom Response Data */
> +struct ncsi_rsp_oem_bcm_pkt {
> + unsigned char ver; /* Payload Version */
> + unsigned char type; /* OEM Command type */
> + __be16 len; /* Payload Length */
> + unsigned char data[]; /* Cmd specific Data */
> +};
> +
> /* Get Link Status */
> struct ncsi_rsp_gls_pkt {
> struct ncsi_rsp_pkt_hdr rsp; /* Response header */
> diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
> index d66b34749027..31672e967db2 100644
> --- a/net/ncsi/ncsi-rsp.c
> +++ b/net/ncsi/ncsi-rsp.c
> @@ -596,12 +596,50 @@ static int ncsi_rsp_handler_snfc(struct ncsi_request *nr)
> return 0;
> }
>
> +/* Response handler for Broadcom command Get Mac Address */
> +static int ncsi_rsp_handler_oem_bcm_gma(struct ncsi_request *nr)
> +{
> + struct ncsi_rsp_oem_pkt *rsp;
> + struct ncsi_dev_priv *ndp = nr->ndp;
> + struct net_device *ndev = ndp->ndev.dev;
> + int ret = 0;
> + const struct net_device_ops *ops = ndev->netdev_ops;
> + struct sockaddr saddr;
> +
> + /* Get the response header */
> + rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
> +
> + saddr.sa_family = ndev->type;
> + ndev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
> + memcpy(saddr.sa_data, &rsp->data[BCM_MAC_ADDR_OFFSET], ETH_ALEN);
> + ret = ops->ndo_set_mac_address(ndev, &saddr);
> + if (ret < 0)
> + netdev_warn(ndev, "NCSI: Writing mac address to device failed\n");
> +
> + return ret;
> +}
> +
> +/* Response handler for Broadcom card */
> +static int ncsi_rsp_handler_oem_bcm(struct ncsi_request *nr)
> +{
> + struct ncsi_rsp_oem_pkt *rsp;
> + struct ncsi_rsp_oem_bcm_pkt *bcm;
> +
> + /* Get the response header */
> + rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
> + bcm = (struct ncsi_rsp_oem_bcm_pkt *)(rsp->data);
> +
> + if (bcm->type == NCSI_OEM_BCM_CMD_GMA)
> + return ncsi_rsp_handler_oem_bcm_gma(nr);
> + return 0;
> +}
> +
> static struct ncsi_rsp_oem_handler {
> unsigned int mfr_id;
> int (*handler)(struct ncsi_request *nr);
> } ncsi_rsp_oem_handlers[] = {
> { NCSI_OEM_MFR_MLX_ID, NULL },
> - { NCSI_OEM_MFR_BCM_ID, NULL }
> + { NCSI_OEM_MFR_BCM_ID, ncsi_rsp_handler_oem_bcm }
> };
>
> /* Response handler for OEM command */
> --
> 2.17.1
>
^ permalink raw reply
* [PATCH net-next v3 1/2] net/ncsi: Add NCSI Broadcom OEM command
From: Vijay Khemka @ 2018-10-11 23:24 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <fcd53ec5b915431087b7100432bece2f@AUSX13MPS302.AMER.DELL.COM>
?On 10/11/18, 4:21 PM, "Justin.Lee1 at Dell.com" <Justin.Lee1@Dell.com> wrote:
Hi Vijay,
Please order all local variable declarations from longest to shortest
line as instructed by Dave.
Thanks,
Justin
Thanks Justin,
I will take care of this.
Regards
-Vijay
^ permalink raw reply
* [PATCH net-next v3 1/2] net/ncsi: Add NCSI Broadcom OEM command
From: Samuel Mendoza-Jonas @ 2018-10-12 0:41 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20181011230518.3204700-1-vijaykhemka@fb.com>
On Thu, 2018-10-11 at 16:05 -0700, Vijay Khemka wrote:
> This patch adds OEM Broadcom commands and response handling. It also
> defines OEM Get MAC Address handler to get and configure the device.
>
> ncsi_oem_gma_handler_bcm: This handler send NCSI broadcom command for
> getting mac address.
> ncsi_rsp_handler_oem_bcm: This handles response received for all
> broadcom OEM commands.
> ncsi_rsp_handler_oem_bcm_gma: This handles get mac address response and
> set it to device.
>
> Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
> ---
> net/ncsi/Kconfig | 6 ++++
> net/ncsi/internal.h | 8 +++++
> net/ncsi/ncsi-manage.c | 70 ++++++++++++++++++++++++++++++++++++++++++
> net/ncsi/ncsi-pkt.h | 8 +++++
> net/ncsi/ncsi-rsp.c | 40 +++++++++++++++++++++++-
> 5 files changed, 131 insertions(+), 1 deletion(-)
>
> diff --git a/net/ncsi/Kconfig b/net/ncsi/Kconfig
> index 08a8a6031fd7..7f2b46108a24 100644
> --- a/net/ncsi/Kconfig
> +++ b/net/ncsi/Kconfig
> @@ -10,3 +10,9 @@ config NET_NCSI
> support. Enable this only if your system connects to a network
> device via NCSI and the ethernet driver you're using supports
> the protocol explicitly.
> +config NCSI_OEM_CMD_GET_MAC
> + bool "Get NCSI OEM MAC Address"
> + depends on NET_NCSI
> + ---help---
> + This allows to get MAC address from NCSI firmware and set them back to
> + controller.
> diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
> index 3d0a33b874f5..45883b32790e 100644
> --- a/net/ncsi/internal.h
> +++ b/net/ncsi/internal.h
> @@ -71,6 +71,13 @@ enum {
> /* OEM Vendor Manufacture ID */
> #define NCSI_OEM_MFR_MLX_ID 0x8119
> #define NCSI_OEM_MFR_BCM_ID 0x113d
> +/* Broadcom specific OEM Command */
> +#define NCSI_OEM_BCM_CMD_GMA 0x01 /* CMD ID for Get MAC */
> +/* OEM Command payload lengths*/
> +#define NCSI_OEM_BCM_CMD_GMA_LEN 12
> +/* Mac address offset in OEM response */
> +#define BCM_MAC_ADDR_OFFSET 28
> +
>
> struct ncsi_channel_version {
> u32 version; /* Supported BCD encoded NCSI version */
> @@ -240,6 +247,7 @@ enum {
> ncsi_dev_state_probe_dp,
> ncsi_dev_state_config_sp = 0x0301,
> ncsi_dev_state_config_cis,
> + ncsi_dev_state_config_oem_gma,
> ncsi_dev_state_config_clear_vids,
> ncsi_dev_state_config_svf,
> ncsi_dev_state_config_ev,
> diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
> index 091284760d21..75504ccd1b95 100644
> --- a/net/ncsi/ncsi-manage.c
> +++ b/net/ncsi/ncsi-manage.c
> @@ -635,6 +635,39 @@ static int set_one_vid(struct ncsi_dev_priv *ndp, struct ncsi_channel *nc,
> return 0;
> }
>
> +#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC)
> +
> +/* NCSI OEM Command APIs */
> +static void ncsi_oem_gma_handler_bcm(struct ncsi_cmd_arg *nca)
> +{
> + int ret = 0;
> + unsigned char data[NCSI_OEM_BCM_CMD_GMA_LEN];
> +
> + nca->payload = NCSI_OEM_BCM_CMD_GMA_LEN;
> +
> + memset(data, 0, NCSI_OEM_BCM_CMD_GMA_LEN);
> + *(unsigned int *)data = ntohl(NCSI_OEM_MFR_BCM_ID);
> + data[5] = NCSI_OEM_BCM_CMD_GMA;
> +
> + nca->data = data;
> +
> + ret = ncsi_xmit_cmd(nca);
> + if (ret)
> + netdev_err(nca->ndp->ndev.dev,
> + "NCSI: Failed to transmit cmd 0x%x during configure\n",
> + nca->type);
> +}
> +
> +/* OEM Command handlers initialization */
> +static struct ncsi_oem_gma_handler {
> + unsigned int mfr_id;
> + void (*handler)(struct ncsi_cmd_arg *nca);
> +} ncsi_oem_gma_handlers[] = {
> + { NCSI_OEM_MFR_BCM_ID, ncsi_oem_gma_handler_bcm }
> +};
> +
> +#endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */
> +
> static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
> {
> struct ncsi_dev *nd = &ndp->ndev;
> @@ -685,6 +718,43 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
> goto error;
> }
>
> +#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC)
> + nd->state = ncsi_dev_state_config_oem_gma;
> + break;
> + case ncsi_dev_state_config_oem_gma:
> + nca.type = NCSI_PKT_CMD_OEM;
> + nca.package = np->id;
> + nca.channel = nc->id;
> + ndp->pending_req_num = 1;
> +
> + /* Check for manufacturer id and Find the handler */
> + struct ncsi_oem_gma_handler *nch = NULL;
> + int i;
> +
This has the opposite affect, now if we do compile with
CONFIG_NCSI_OEM_CMD_GET_MAC we get:
../net/ncsi/ncsi-manage.c: In function ?ncsi_configure_channel?:
../net/ncsi/ncsi-manage.c:769:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
struct ncsi_oem_gma_handler *nch = NULL;
^~~~~~
Perhaps we should lay this out slightly differently. For example we could go
through the ncsi_dev_state_config_oem_gma state regardless and call some other
function that finds and calls the handler, or does nothing if the config option
isn't set.
Regards,
Sam
> + for (i = 0; i < ARRAY_SIZE(ncsi_oem_gma_handlers); i++) {
> + if (ncsi_oem_gma_handlers[i].mfr_id ==
> + nc->version.mf_id) {
> + if (ncsi_oem_gma_handlers[i].handler)
> + nch = &ncsi_oem_gma_handlers[i];
> + else
> + nch = NULL;
> +
> + break;
> + }
> + }
> +
> + if (!nch) {
> + netdev_err(ndp->ndev.dev, "No handler available for GMA with MFR-ID (0x%x)\n",
> + nc->version.mf_id);
> + nd->state = ncsi_dev_state_config_clear_vids;
> + schedule_work(&ndp->work);
> + break;
> + }
> +
> + /* Get Mac address from NCSI device */
> + nch->handler(&nca);
> +#endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */
> +
> nd->state = ncsi_dev_state_config_clear_vids;
> break;
> case ncsi_dev_state_config_clear_vids:
> diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h
> index 0f2087c8d42a..4d3f06be38bd 100644
> --- a/net/ncsi/ncsi-pkt.h
> +++ b/net/ncsi/ncsi-pkt.h
> @@ -165,6 +165,14 @@ struct ncsi_rsp_oem_pkt {
> unsigned char data[]; /* Payload data */
> };
>
> +/* Broadcom Response Data */
> +struct ncsi_rsp_oem_bcm_pkt {
> + unsigned char ver; /* Payload Version */
> + unsigned char type; /* OEM Command type */
> + __be16 len; /* Payload Length */
> + unsigned char data[]; /* Cmd specific Data */
> +};
> +
> /* Get Link Status */
> struct ncsi_rsp_gls_pkt {
> struct ncsi_rsp_pkt_hdr rsp; /* Response header */
> diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
> index d66b34749027..31672e967db2 100644
> --- a/net/ncsi/ncsi-rsp.c
> +++ b/net/ncsi/ncsi-rsp.c
> @@ -596,12 +596,50 @@ static int ncsi_rsp_handler_snfc(struct ncsi_request *nr)
> return 0;
> }
>
> +/* Response handler for Broadcom command Get Mac Address */
> +static int ncsi_rsp_handler_oem_bcm_gma(struct ncsi_request *nr)
> +{
> + struct ncsi_rsp_oem_pkt *rsp;
> + struct ncsi_dev_priv *ndp = nr->ndp;
> + struct net_device *ndev = ndp->ndev.dev;
> + int ret = 0;
> + const struct net_device_ops *ops = ndev->netdev_ops;
> + struct sockaddr saddr;
> +
> + /* Get the response header */
> + rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
> +
> + saddr.sa_family = ndev->type;
> + ndev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
> + memcpy(saddr.sa_data, &rsp->data[BCM_MAC_ADDR_OFFSET], ETH_ALEN);
> + ret = ops->ndo_set_mac_address(ndev, &saddr);
> + if (ret < 0)
> + netdev_warn(ndev, "NCSI: Writing mac address to device failed\n");
> +
> + return ret;
> +}
> +
> +/* Response handler for Broadcom card */
> +static int ncsi_rsp_handler_oem_bcm(struct ncsi_request *nr)
> +{
> + struct ncsi_rsp_oem_pkt *rsp;
> + struct ncsi_rsp_oem_bcm_pkt *bcm;
> +
> + /* Get the response header */
> + rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
> + bcm = (struct ncsi_rsp_oem_bcm_pkt *)(rsp->data);
> +
> + if (bcm->type == NCSI_OEM_BCM_CMD_GMA)
> + return ncsi_rsp_handler_oem_bcm_gma(nr);
> + return 0;
> +}
> +
> static struct ncsi_rsp_oem_handler {
> unsigned int mfr_id;
> int (*handler)(struct ncsi_request *nr);
> } ncsi_rsp_oem_handlers[] = {
> { NCSI_OEM_MFR_MLX_ID, NULL },
> - { NCSI_OEM_MFR_BCM_ID, NULL }
> + { NCSI_OEM_MFR_BCM_ID, ncsi_rsp_handler_oem_bcm }
> };
>
> /* Response handler for OEM command */
^ permalink raw reply
* [PATCH net-next v7] net/ncsi: Extend NC-SI Netlink interface to allow user space to send NC-SI command
From: Samuel Mendoza-Jonas @ 2018-10-12 2:39 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <cd371b0265b74889a10241a6af7e4843@AUSX13MPS302.AMER.DELL.COM>
On Thu, 2018-10-11 at 18:07 +0000, Justin.Lee1 at Dell.com wrote:
> The new command (NCSI_CMD_SEND_CMD) is added to allow user space application
> to send NC-SI command to the network card.
> Also, add a new attribute (NCSI_ATTR_DATA) for transferring request and response.
>
> The work flow is as below.
>
> Request:
> User space application
> -> Netlink interface (msg)
> -> new Netlink handler - ncsi_send_cmd_nl()
> -> ncsi_xmit_cmd()
>
> Response:
> Response received - ncsi_rcv_rsp()
> -> internal response handler - ncsi_rsp_handler_xxx()
> -> ncsi_rsp_handler_netlink()
> -> ncsi_send_netlink_rsp ()
> -> Netlink interface (msg)
> -> user space application
>
> Command timeout - ncsi_request_timeout()
> -> ncsi_send_netlink_timeout ()
> -> Netlink interface (msg with zero data length)
> -> user space application
>
> Error:
> Error detected
> -> ncsi_send_netlink_err ()
> -> Netlink interface (err msg)
> -> user space application
>
>
> Signed-off-by: Justin Lee <justin.lee1@dell.com>
>
Reviewed-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
^ permalink raw reply
* [PATCH v4 2/2] media: platform: Add Aspeed Video Engine driver
From: Hans Verkuil @ 2018-10-12 12:22 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <1538769466-14860-3-git-send-email-eajames@linux.ibm.com>
On 10/05/2018 09:57 PM, Eddie James wrote:
> The Video Engine (VE) embedded in the Aspeed AST2400 and AST2500 SOCs
> can capture and compress video data from digital or analog sources. With
> the Aspeed chip acting a service processor, the Video Engine can capture
> the host processor graphics output.
>
> Add a V4L2 driver to capture video data and compress it to JPEG images.
> Make the video frames available through the V4L2 streaming interface.
>
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> ---
> MAINTAINERS | 8 +
> drivers/media/platform/Kconfig | 8 +
> drivers/media/platform/Makefile | 1 +
> drivers/media/platform/aspeed-video.c | 1674 +++++++++++++++++++++++++++++++++
> 4 files changed, 1691 insertions(+)
> create mode 100644 drivers/media/platform/aspeed-video.c
>
<snip>
> +static int aspeed_video_enum_input(struct file *file, void *fh,
> + struct v4l2_input *inp)
> +{
> + if (inp->index)
> + return -EINVAL;
> +
> + strscpy(inp->name, "Host VGA capture", sizeof(inp->name));
> + inp->type = V4L2_INPUT_TYPE_CAMERA;
> + inp->capabilities = V4L2_IN_CAP_DV_TIMINGS;
> + inp->status = V4L2_IN_ST_NO_SIGNAL | V4L2_IN_ST_NO_SYNC;
This can't be right. If there is a valid signal, then status should be 0.
And ideally you can tell the difference between no signal and no sync
as well.
> +
> + return 0;
> +}
> +
> +static int aspeed_video_get_input(struct file *file, void *fh, unsigned int *i)
> +{
> + *i = 0;
> +
> + return 0;
> +}
> +
> +static int aspeed_video_set_input(struct file *file, void *fh, unsigned int i)
> +{
> + if (i)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static int aspeed_video_get_parm(struct file *file, void *fh,
> + struct v4l2_streamparm *a)
> +{
> + struct aspeed_video *video = video_drvdata(file);
> +
> + a->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
> + a->parm.capture.readbuffers = 3;
> + a->parm.capture.timeperframe.numerator = 1;
> + if (!video->frame_rate)
> + a->parm.capture.timeperframe.denominator = MAX_FRAME_RATE + 1;
> + else
> + a->parm.capture.timeperframe.denominator = video->frame_rate;
> +
> + return 0;
> +}
> +
> +static int aspeed_video_set_parm(struct file *file, void *fh,
> + struct v4l2_streamparm *a)
> +{
> + unsigned int frame_rate = 0;
> + struct aspeed_video *video = video_drvdata(file);
> +
> + a->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
> + a->parm.capture.readbuffers = 3;
> +
> + if (a->parm.capture.timeperframe.numerator)
> + frame_rate = a->parm.capture.timeperframe.denominator /
> + a->parm.capture.timeperframe.numerator;
> +
> + if (!frame_rate || frame_rate > MAX_FRAME_RATE) {
> + frame_rate = 0;
> +
> + /*
> + * Set to max + 1 to differentiate between max and 0, which
> + * means "don't care".
But what does "don't care" mean in practice? It's still not clear to me how this
is supposed to work.
> + */
> + a->parm.capture.timeperframe.denominator = MAX_FRAME_RATE + 1;
And regardless of anything else this timeperframe is out of the range that
aspeed_video_enum_frameintervals() returns.
> + a->parm.capture.timeperframe.numerator = 1;
> + }
> +
> + if (video->frame_rate != frame_rate) {
> + video->frame_rate = frame_rate;
> + aspeed_video_update(video, VE_CTRL, VE_CTRL_FRC,
> + FIELD_PREP(VE_CTRL_FRC, frame_rate));
> + }
> +
> + return 0;
> +}
> +
> +static int aspeed_video_enum_framesizes(struct file *file, void *fh,
> + struct v4l2_frmsizeenum *fsize)
> +{
> + struct aspeed_video *video = video_drvdata(file);
> +
> + if (fsize->index)
> + return -EINVAL;
> +
> + if (fsize->pixel_format != V4L2_PIX_FMT_JPEG)
> + return -EINVAL;
> +
> + fsize->discrete.width = video->pix_fmt.width;
> + fsize->discrete.height = video->pix_fmt.height;
> + fsize->type = V4L2_FRMSIZE_TYPE_DISCRETE;
> +
> + return 0;
> +}
> +
> +static int aspeed_video_enum_frameintervals(struct file *file, void *fh,
> + struct v4l2_frmivalenum *fival)
> +{
> + struct aspeed_video *video = video_drvdata(file);
> +
> + if (fival->index)
> + return -EINVAL;
> +
> + if (fival->width != video->width || fival->height != video->height)
> + return -EINVAL;
> +
> + if (fival->pixel_format != V4L2_PIX_FMT_JPEG)
> + return -EINVAL;
> +
> + fival->type = V4L2_FRMIVAL_TYPE_CONTINUOUS;
> +
> + fival->stepwise.min.denominator = MAX_FRAME_RATE;
> + fival->stepwise.min.numerator = 1;
> + fival->stepwise.max.denominator = 1;
> + fival->stepwise.max.numerator = 1;
> + fival->stepwise.step = fival->stepwise.max;
> +
> + return 0;
> +}
> +
> +static int aspeed_video_set_dv_timings(struct file *file, void *fh,
> + struct v4l2_dv_timings *timings)
> +{
> + struct aspeed_video *video = video_drvdata(file);
> +
If vb2_is_busy() returns true, then return -EBUSY here. It is not allowed to
set the timings while vb2 is busy.
> + if (video->width != timings->bt.width ||
> + video->height != timings->bt.height)
> + return -EINVAL;
> +
> + video->pix_fmt.width = timings->bt.width;
> + video->pix_fmt.height = timings->bt.height;
> + video->pix_fmt.sizeimage = video->max_compressed_size;
> + video->timings.width = timings->bt.width;
> + video->timings.height = timings->bt.height;
> +
> + timings->type = V4L2_DV_BT_656_1120;
> +
> + return 0;
> +}
> +
> +static int aspeed_video_get_dv_timings(struct file *file, void *fh,
> + struct v4l2_dv_timings *timings)
> +{
> + struct aspeed_video *video = video_drvdata(file);
> +
> + timings->type = V4L2_DV_BT_656_1120;
> + timings->bt = video->timings;
> +
> + return 0;
> +}
> +
> +static int aspeed_video_query_dv_timings(struct file *file, void *fh,
> + struct v4l2_dv_timings *timings)
> +{
> + int rc;
> + struct aspeed_video *video = video_drvdata(file);
> +
> + if (file->f_flags & O_NONBLOCK) {
> + if (test_bit(VIDEO_RES_CHANGE, &video->flags))
> + return -EAGAIN;
> + } else {
> + rc = wait_event_interruptible(video->wait,
> + !test_bit(VIDEO_RES_CHANGE,
> + &video->flags));
> + if (rc)
> + return -EINTR;
> + }
> +
> + timings->type = V4L2_DV_BT_656_1120;
> + timings->bt = video->timings;
> + timings->bt.width = video->width;
> + timings->bt.height = video->height;
> +
> + return 0;
> +}
> +
> +static int aspeed_video_enum_dv_timings(struct file *file, void *fh,
> + struct v4l2_enum_dv_timings *timings)
> +{
> + if (timings->index)
> + return -EINVAL;
> +
> + return aspeed_video_get_dv_timings(file, fh, &timings->timings);
> +}
> +
> +static int aspeed_video_dv_timings_cap(struct file *file, void *fh,
> + struct v4l2_dv_timings_cap *cap)
> +{
> + struct aspeed_video *video = video_drvdata(file);
> +
> + cap->type = V4L2_DV_BT_656_1120;
> + cap->bt.capabilities = V4L2_DV_BT_CAP_PROGRESSIVE;
> + cap->bt.min_width = video->width;
> + cap->bt.max_width = video->width;
> + cap->bt.min_height = video->height;
> + cap->bt.max_height = video->height;
This should return the capabilities of the aspeed. In this case I'd
guess that the max width/height is 1920x1080 (or perhaps 1200).
The minimum is probably the VGA resolution.
> +
> + return 0;
> +}
> +
> +static int aspeed_video_sub_event(struct v4l2_fh *fh,
> + const struct v4l2_event_subscription *sub)
> +{
> + switch (sub->type) {
> + case V4L2_EVENT_SOURCE_CHANGE:
> + return v4l2_src_change_event_subscribe(fh, sub);
> + }
> +
> + return v4l2_ctrl_subscribe_event(fh, sub);
> +}
> +
> +static const struct v4l2_ioctl_ops aspeed_video_ioctl_ops = {
> + .vidioc_querycap = aspeed_video_querycap,
> +
> + .vidioc_enum_fmt_vid_cap = aspeed_video_enum_format,
> + .vidioc_g_fmt_vid_cap = aspeed_video_get_format,
> + .vidioc_s_fmt_vid_cap = aspeed_video_get_format,
> + .vidioc_try_fmt_vid_cap = aspeed_video_get_format,
> +
> + .vidioc_reqbufs = vb2_ioctl_reqbufs,
> + .vidioc_querybuf = vb2_ioctl_querybuf,
> + .vidioc_qbuf = vb2_ioctl_qbuf,
> + .vidioc_dqbuf = vb2_ioctl_dqbuf,
> + .vidioc_create_bufs = vb2_ioctl_create_bufs,
> + .vidioc_prepare_buf = vb2_ioctl_prepare_buf,
> + .vidioc_streamon = vb2_ioctl_streamon,
> + .vidioc_streamoff = vb2_ioctl_streamoff,
> +
> + .vidioc_enum_input = aspeed_video_enum_input,
> + .vidioc_g_input = aspeed_video_get_input,
> + .vidioc_s_input = aspeed_video_set_input,
> +
> + .vidioc_g_parm = aspeed_video_get_parm,
> + .vidioc_s_parm = aspeed_video_set_parm,
> + .vidioc_enum_framesizes = aspeed_video_enum_framesizes,
> + .vidioc_enum_frameintervals = aspeed_video_enum_frameintervals,
> +
> + .vidioc_s_dv_timings = aspeed_video_set_dv_timings,
> + .vidioc_g_dv_timings = aspeed_video_get_dv_timings,
> + .vidioc_query_dv_timings = aspeed_video_query_dv_timings,
> + .vidioc_enum_dv_timings = aspeed_video_enum_dv_timings,
> + .vidioc_dv_timings_cap = aspeed_video_dv_timings_cap,
> +
> + .vidioc_subscribe_event = aspeed_video_sub_event,
> + .vidioc_unsubscribe_event = v4l2_event_unsubscribe,
> +};
> +
> +static void aspeed_video_update_jpeg_quality(struct aspeed_video *video)
> +{
> + u32 comp_ctrl = FIELD_PREP(VE_COMP_CTRL_DCT_LUM, video->jpeg_quality) |
> + FIELD_PREP(VE_COMP_CTRL_DCT_CHR, video->jpeg_quality | 0x10);
> +
> + aspeed_video_update(video, VE_COMP_CTRL,
> + VE_COMP_CTRL_DCT_LUM | VE_COMP_CTRL_DCT_CHR,
> + comp_ctrl);
> +}
> +
> +static void aspeed_video_update_subsampling(struct aspeed_video *video)
> +{
> + if (video->jpeg.virt)
> + aspeed_video_init_jpeg_table(video->jpeg.virt, video->yuv420);
> +
> + if (video->yuv420)
> + aspeed_video_update(video, VE_SEQ_CTRL, 0, VE_SEQ_CTRL_YUV420);
> + else
> + aspeed_video_update(video, VE_SEQ_CTRL, VE_SEQ_CTRL_YUV420, 0);
> +}
> +
> +static int aspeed_video_set_ctrl(struct v4l2_ctrl *ctrl)
> +{
> + struct aspeed_video *video = container_of(ctrl->handler,
> + struct aspeed_video,
> + ctrl_handler);
> +
> + switch (ctrl->id) {
> + case V4L2_CID_JPEG_COMPRESSION_QUALITY:
> + video->jpeg_quality = ctrl->val;
> + aspeed_video_update_jpeg_quality(video);
> + break;
> + case V4L2_CID_JPEG_CHROMA_SUBSAMPLING:
> + if (ctrl->val == V4L2_JPEG_CHROMA_SUBSAMPLING_420) {
> + video->yuv420 = true;
> + aspeed_video_update_subsampling(video);
> + } else {
> + video->yuv420 = false;
> + aspeed_video_update_subsampling(video);
> + }
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static const struct v4l2_ctrl_ops aspeed_video_ctrl_ops = {
> + .s_ctrl = aspeed_video_set_ctrl,
> +};
> +
> +static void aspeed_video_resolution_work(struct work_struct *work)
> +{
> + int rc;
> + struct delayed_work *dwork = to_delayed_work(work);
> + struct aspeed_video *video = container_of(dwork, struct aspeed_video,
> + res_work);
> +
> + /* No clients remaining after delay */
> + if (atomic_read(&video->clients) == 0)
> + goto done;
> +
> + aspeed_video_on(video);
> +
> + aspeed_video_init_regs(video);
> +
> + rc = aspeed_video_get_resolution(video);
> + if (rc)
> + dev_err(video->dev,
> + "resolution changed; couldn't get new resolution\n");
> + else if (test_bit(VIDEO_STREAMING, &video->flags))
> + aspeed_video_start_frame(video);
> +
> + if (video->width != video->pix_fmt.width ||
> + video->height != video->pix_fmt.height) {
> + static const struct v4l2_event ev = {
> + .type = V4L2_EVENT_SOURCE_CHANGE,
> + .u.src_change.changes = V4L2_EVENT_SRC_CH_RESOLUTION,
> + };
> +
> + v4l2_event_queue(&video->vdev, &ev);
> + }
> +
> +done:
> + clear_bit(VIDEO_RES_CHANGE, &video->flags);
> + wake_up_interruptible_all(&video->wait);
> +}
> +
> +static int aspeed_video_open(struct file *file)
> +{
> + int rc;
> + struct aspeed_video *video = video_drvdata(file);
> +
> + mutex_lock(&video->video_lock);
> +
> + if (atomic_inc_return(&video->clients) == 1) {
> + rc = aspeed_video_start(video);
> + if (rc) {
> + dev_err(video->dev, "Failed to start video engine\n");
> + atomic_dec(&video->clients);
> + mutex_unlock(&video->video_lock);
> + return rc;
> + }
> + }
> +
> + mutex_unlock(&video->video_lock);
> +
> + return v4l2_fh_open(file);
> +}
> +
> +static int aspeed_video_release(struct file *file)
> +{
> + int rc;
> + struct aspeed_video *video = video_drvdata(file);
> +
> + rc = vb2_fop_release(file);
> +
> + mutex_lock(&video->video_lock);
> +
> + if (atomic_dec_return(&video->clients) == 0)
> + aspeed_video_stop(video);
> +
> + mutex_unlock(&video->video_lock);
> +
> + return rc;
> +}
> +
> +static const struct v4l2_file_operations aspeed_video_v4l2_fops = {
> + .owner = THIS_MODULE,
> + .read = vb2_fop_read,
> + .poll = vb2_fop_poll,
> + .unlocked_ioctl = video_ioctl2,
> + .mmap = vb2_fop_mmap,
> + .open = aspeed_video_open,
> + .release = aspeed_video_release,
> +};
> +
> +static int aspeed_video_queue_setup(struct vb2_queue *q,
> + unsigned int *num_buffers,
> + unsigned int *num_planes,
> + unsigned int sizes[],
> + struct device *alloc_devs[])
> +{
> + struct aspeed_video *video = vb2_get_drv_priv(q);
> +
> + if (*num_planes) {
> + if (sizes[0] < video->max_compressed_size)
> + return -EINVAL;
> +
> + return 0;
> + }
> +
> + *num_planes = 1;
> + sizes[0] = video->max_compressed_size;
> +
> + return 0;
> +}
> +
> +static int aspeed_video_buf_prepare(struct vb2_buffer *vb)
> +{
> + struct aspeed_video *video = vb2_get_drv_priv(vb->vb2_queue);
> +
> + if (vb2_plane_size(vb, 0) < video->max_compressed_size)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static int aspeed_video_start_streaming(struct vb2_queue *q,
> + unsigned int count)
> +{
> + int rc;
> + struct aspeed_video *video = vb2_get_drv_priv(q);
> +
> + rc = aspeed_video_start_frame(video);
> + if (rc) {
> + aspeed_video_bufs_done(video, VB2_BUF_STATE_QUEUED);
> + return rc;
> + }
> +
> + video->sequence = 0;
> + set_bit(VIDEO_STREAMING, &video->flags);
> + return 0;
> +}
> +
> +static void aspeed_video_stop_streaming(struct vb2_queue *q)
> +{
> + int rc;
> + struct aspeed_video *video = vb2_get_drv_priv(q);
> +
> + clear_bit(VIDEO_STREAMING, &video->flags);
> +
> + rc = wait_event_timeout(video->wait,
> + !test_bit(VIDEO_FRAME_INPRG, &video->flags),
> + STOP_TIMEOUT);
> + if (!rc) {
> + dev_err(video->dev, "Timed out when stopping streaming\n");
> + aspeed_video_stop(video);
> + }
> +
> + aspeed_video_bufs_done(video, VB2_BUF_STATE_ERROR);
> +}
> +
> +static void aspeed_video_buf_queue(struct vb2_buffer *vb)
> +{
> + struct aspeed_video *video = vb2_get_drv_priv(vb->vb2_queue);
> + struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> + struct aspeed_video_buffer *avb = to_aspeed_video_buffer(vbuf);
> + unsigned long flags;
> +
> + spin_lock_irqsave(&video->lock, flags);
> + list_add_tail(&avb->link, &video->buffers);
> + spin_unlock_irqrestore(&video->lock, flags);
> +}
> +
> +static const struct vb2_ops aspeed_video_vb2_ops = {
> + .queue_setup = aspeed_video_queue_setup,
> + .wait_prepare = vb2_ops_wait_prepare,
> + .wait_finish = vb2_ops_wait_finish,
> + .buf_prepare = aspeed_video_buf_prepare,
> + .start_streaming = aspeed_video_start_streaming,
> + .stop_streaming = aspeed_video_stop_streaming,
> + .buf_queue = aspeed_video_buf_queue,
> +};
> +
> +static int aspeed_video_setup_video(struct aspeed_video *video)
> +{
> + const u64 mask = ~(BIT(V4L2_JPEG_CHROMA_SUBSAMPLING_444) |
> + BIT(V4L2_JPEG_CHROMA_SUBSAMPLING_420));
> + struct v4l2_device *v4l2_dev = &video->v4l2_dev;
> + struct vb2_queue *vbq = &video->queue;
> + struct video_device *vdev = &video->vdev;
> + int rc;
> +
> + video->pix_fmt.pixelformat = V4L2_PIX_FMT_JPEG;
> + video->pix_fmt.field = V4L2_FIELD_NONE;
> + video->pix_fmt.colorspace = V4L2_COLORSPACE_SRGB;
> + video->pix_fmt.quantization = V4L2_QUANTIZATION_FULL_RANGE;
> +
> + rc = v4l2_device_register(video->dev, v4l2_dev);
> + if (rc) {
> + dev_err(video->dev, "Failed to register v4l2 device\n");
> + return rc;
> + }
> +
> + v4l2_ctrl_handler_init(&video->ctrl_handler, 2);
> + v4l2_ctrl_new_std(&video->ctrl_handler, &aspeed_video_ctrl_ops,
> + V4L2_CID_JPEG_COMPRESSION_QUALITY, 0,
> + ASPEED_VIDEO_JPEG_NUM_QUALITIES - 1, 1, 0);
> + v4l2_ctrl_new_std_menu(&video->ctrl_handler, &aspeed_video_ctrl_ops,
> + V4L2_CID_JPEG_CHROMA_SUBSAMPLING,
> + V4L2_JPEG_CHROMA_SUBSAMPLING_420, mask,
> + V4L2_JPEG_CHROMA_SUBSAMPLING_444);
> +
> + if (video->ctrl_handler.error) {
> + v4l2_ctrl_handler_free(&video->ctrl_handler);
> + v4l2_device_unregister(v4l2_dev);
> +
> + dev_err(video->dev, "Failed to init controls: %d\n",
> + video->ctrl_handler.error);
> + return rc;
> + }
> +
> + v4l2_dev->ctrl_handler = &video->ctrl_handler;
> +
> + vbq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> + vbq->io_modes = VB2_MMAP | VB2_READ | VB2_DMABUF;
> + vbq->dev = v4l2_dev->dev;
> + vbq->lock = &video->video_lock;
> + vbq->ops = &aspeed_video_vb2_ops;
> + vbq->mem_ops = &vb2_dma_contig_memops;
> + vbq->drv_priv = video;
> + vbq->buf_struct_size = sizeof(struct aspeed_video_buffer);
> + vbq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> + vbq->min_buffers_needed = 3;
> +
> + rc = vb2_queue_init(vbq);
> + if (rc) {
> + v4l2_ctrl_handler_free(&video->ctrl_handler);
> + v4l2_device_unregister(v4l2_dev);
> +
> + dev_err(video->dev, "Failed to init vb2 queue\n");
> + return rc;
> + }
> +
> + vdev->queue = vbq;
> + vdev->fops = &aspeed_video_v4l2_fops;
> + vdev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_READWRITE |
READWRITE doesn't work for JPEG since there is no clear end of a frame. Just drop
this and the read op in aspeed_video_v4l2_fops.
> + V4L2_CAP_STREAMING;
> + vdev->v4l2_dev = v4l2_dev;
> + strscpy(vdev->name, DEVICE_NAME, sizeof(vdev->name));
> + vdev->vfl_type = VFL_TYPE_GRABBER;
> + vdev->vfl_dir = VFL_DIR_RX;
> + vdev->release = video_device_release_empty;
> + vdev->ioctl_ops = &aspeed_video_ioctl_ops;
> + vdev->lock = &video->video_lock;
> +
> + video_set_drvdata(vdev, video);
> + rc = video_register_device(vdev, VFL_TYPE_GRABBER, 0);
> + if (rc) {
> + vb2_queue_release(vbq);
> + v4l2_ctrl_handler_free(&video->ctrl_handler);
> + v4l2_device_unregister(v4l2_dev);
> +
> + dev_err(video->dev, "Failed to register video device\n");
> + return rc;
> + }
> +
> + return 0;
> +}
> +
> +static int aspeed_video_init(struct aspeed_video *video)
> +{
> + int irq;
> + int rc;
> + struct device *dev = video->dev;
> +
> + irq = irq_of_parse_and_map(dev->of_node, 0);
> + if (!irq) {
> + dev_err(dev, "Unable to find IRQ\n");
> + return -ENODEV;
> + }
> +
> + rc = devm_request_irq(dev, irq, aspeed_video_irq, IRQF_SHARED,
> + DEVICE_NAME, video);
> + if (rc < 0) {
> + dev_err(dev, "Unable to request IRQ %d\n", irq);
> + return rc;
> + }
> +
> + video->eclk = devm_clk_get(dev, "eclk");
> + if (IS_ERR(video->eclk)) {
> + dev_err(dev, "Unable to get ECLK\n");
> + return PTR_ERR(video->eclk);
> + }
> +
> + video->vclk = devm_clk_get(dev, "vclk");
> + if (IS_ERR(video->vclk)) {
> + dev_err(dev, "Unable to get VCLK\n");
> + return PTR_ERR(video->vclk);
> + }
> +
> + video->rst = devm_reset_control_get_exclusive(dev, NULL);
> + if (IS_ERR(video->rst)) {
> + dev_err(dev, "Unable to get VE reset\n");
> + return PTR_ERR(video->rst);
> + }
> +
> + rc = of_reserved_mem_device_init(dev);
> + if (rc) {
> + dev_err(dev, "Unable to reserve memory\n");
> + return rc;
> + }
> +
> + rc = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
> + if (rc) {
> + dev_err(dev, "Failed to set DMA mask\n");
> + of_reserved_mem_device_release(dev);
> + return rc;
> + }
> +
> + if (!aspeed_video_alloc_buf(video, &video->jpeg,
> + VE_JPEG_HEADER_SIZE)) {
> + dev_err(dev, "Failed to allocate DMA for JPEG header\n");
> + of_reserved_mem_device_release(dev);
> + return rc;
> + }
> +
> + aspeed_video_init_jpeg_table(video->jpeg.virt, video->yuv420);
> +
> + return 0;
> +}
> +
> +static int aspeed_video_probe(struct platform_device *pdev)
> +{
> + int rc;
> + struct resource *res;
> + struct aspeed_video *video = kzalloc(sizeof(*video), GFP_KERNEL);
> +
> + if (!video)
> + return -ENOMEM;
> +
> + video->frame_rate = 30;
> + video->dev = &pdev->dev;
> + mutex_init(&video->video_lock);
> + init_waitqueue_head(&video->wait);
> + INIT_DELAYED_WORK(&video->res_work, aspeed_video_resolution_work);
> + INIT_LIST_HEAD(&video->buffers);
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +
> + video->base = devm_ioremap_resource(video->dev, res);
> +
> + if (IS_ERR(video->base))
> + return PTR_ERR(video->base);
> +
> + rc = aspeed_video_init(video);
> + if (rc)
> + return rc;
> +
> + rc = aspeed_video_setup_video(video);
> + if (rc)
> + return rc;
> +
> + return 0;
> +}
> +
> +static int aspeed_video_remove(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct v4l2_device *v4l2_dev = dev_get_drvdata(dev);
> + struct aspeed_video *video = to_aspeed_video(v4l2_dev);
> +
> + video_unregister_device(&video->vdev);
> +
> + vb2_queue_release(&video->queue);
> +
> + v4l2_ctrl_handler_free(&video->ctrl_handler);
> +
> + v4l2_device_unregister(v4l2_dev);
> +
> + dma_free_coherent(video->dev, VE_JPEG_HEADER_SIZE, video->jpeg.virt,
> + video->jpeg.dma);
> +
> + of_reserved_mem_device_release(dev);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id aspeed_video_of_match[] = {
> + { .compatible = "aspeed,ast2400-video-engine" },
> + { .compatible = "aspeed,ast2500-video-engine" },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, aspeed_video_of_match);
> +
> +static struct platform_driver aspeed_video_driver = {
> + .driver = {
> + .name = DEVICE_NAME,
> + .of_match_table = aspeed_video_of_match,
> + },
> + .probe = aspeed_video_probe,
> + .remove = aspeed_video_remove,
> +};
> +
> +module_platform_driver(aspeed_video_driver);
> +
> +MODULE_DESCRIPTION("ASPEED Video Engine Driver");
> +MODULE_AUTHOR("Eddie James");
> +MODULE_LICENSE("GPL v2");
>
Regards,
Hans
^ permalink raw reply
* [PATCH net-next v4] net/ncsi: Add NCSI Broadcom OEM command
From: Vijay Khemka @ 2018-10-12 18:20 UTC (permalink / raw)
To: linux-aspeed
This patch adds OEM Broadcom commands and response handling. It also
defines OEM Get MAC Address handler to get and configure the device.
ncsi_oem_gma_handler_bcm: This handler send NCSI broadcom command for
getting mac address.
ncsi_rsp_handler_oem_bcm: This handles response received for all
broadcom OEM commands.
ncsi_rsp_handler_oem_bcm_gma: This handles get mac address response and
set it to device.
Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
---
v4: updated as per comment from Sam, I was just wondering if I can remove
NCSI_OEM_CMD_GET_MAC config option and let this code be valid always and
it will configure mac address if there is get mac address handler for given
manufacture id.
net/ncsi/Kconfig | 6 ++++
net/ncsi/internal.h | 8 +++++
net/ncsi/ncsi-manage.c | 75 ++++++++++++++++++++++++++++++++++++++++++
net/ncsi/ncsi-pkt.h | 8 +++++
net/ncsi/ncsi-rsp.c | 44 +++++++++++++++++++++++--
5 files changed, 139 insertions(+), 2 deletions(-)
diff --git a/net/ncsi/Kconfig b/net/ncsi/Kconfig
index 08a8a6031fd7..7f2b46108a24 100644
--- a/net/ncsi/Kconfig
+++ b/net/ncsi/Kconfig
@@ -10,3 +10,9 @@ config NET_NCSI
support. Enable this only if your system connects to a network
device via NCSI and the ethernet driver you're using supports
the protocol explicitly.
+config NCSI_OEM_CMD_GET_MAC
+ bool "Get NCSI OEM MAC Address"
+ depends on NET_NCSI
+ ---help---
+ This allows to get MAC address from NCSI firmware and set them back to
+ controller.
diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index 3d0a33b874f5..45883b32790e 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -71,6 +71,13 @@ enum {
/* OEM Vendor Manufacture ID */
#define NCSI_OEM_MFR_MLX_ID 0x8119
#define NCSI_OEM_MFR_BCM_ID 0x113d
+/* Broadcom specific OEM Command */
+#define NCSI_OEM_BCM_CMD_GMA 0x01 /* CMD ID for Get MAC */
+/* OEM Command payload lengths*/
+#define NCSI_OEM_BCM_CMD_GMA_LEN 12
+/* Mac address offset in OEM response */
+#define BCM_MAC_ADDR_OFFSET 28
+
struct ncsi_channel_version {
u32 version; /* Supported BCD encoded NCSI version */
@@ -240,6 +247,7 @@ enum {
ncsi_dev_state_probe_dp,
ncsi_dev_state_config_sp = 0x0301,
ncsi_dev_state_config_cis,
+ ncsi_dev_state_config_oem_gma,
ncsi_dev_state_config_clear_vids,
ncsi_dev_state_config_svf,
ncsi_dev_state_config_ev,
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 091284760d21..e58bf51ff685 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -635,6 +635,65 @@ static int set_one_vid(struct ncsi_dev_priv *ndp, struct ncsi_channel *nc,
return 0;
}
+#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC)
+
+/* NCSI OEM Command APIs */
+static void ncsi_oem_gma_handler_bcm(struct ncsi_cmd_arg *nca)
+{
+ unsigned char data[NCSI_OEM_BCM_CMD_GMA_LEN];
+ int ret = 0;
+
+ nca->payload = NCSI_OEM_BCM_CMD_GMA_LEN;
+
+ memset(data, 0, NCSI_OEM_BCM_CMD_GMA_LEN);
+ *(unsigned int *)data = ntohl(NCSI_OEM_MFR_BCM_ID);
+ data[5] = NCSI_OEM_BCM_CMD_GMA;
+
+ nca->data = data;
+
+ ret = ncsi_xmit_cmd(nca);
+ if (ret)
+ netdev_err(nca->ndp->ndev.dev,
+ "NCSI: Failed to transmit cmd 0x%x during configure\n",
+ nca->type);
+}
+
+/* OEM Command handlers initialization */
+static struct ncsi_oem_gma_handler {
+ unsigned int mfr_id;
+ void (*handler)(struct ncsi_cmd_arg *nca);
+} ncsi_oem_gma_handlers[] = {
+ { NCSI_OEM_MFR_BCM_ID, ncsi_oem_gma_handler_bcm }
+};
+
+static int ncsi_oem_handler(struct ncsi_cmd_arg *nca, unsigned int mf_id)
+{
+ struct ncsi_oem_gma_handler *nch = NULL;
+ int i;
+
+ /* Find gma handler for given manufacturer id */
+ for (i = 0; i < ARRAY_SIZE(ncsi_oem_gma_handlers); i++) {
+ if (ncsi_oem_gma_handlers[i].mfr_id == mf_id) {
+ if (ncsi_oem_gma_handlers[i].handler)
+ nch = &ncsi_oem_gma_handlers[i];
+ break;
+ }
+ }
+
+ if (!nch) {
+ netdev_err(nca->ndp->ndev.dev,
+ "NCSI: No GMA handler available for MFR-ID (0x%x)\n",
+ mf_id);
+ return -1;
+ }
+
+ /* Get Mac address from NCSI device */
+ nch->handler(nca);
+ return 0;
+}
+
+#endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */
+
static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
{
struct ncsi_dev *nd = &ndp->ndev;
@@ -685,7 +744,23 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
goto error;
}
+ nd->state = ncsi_dev_state_config_oem_gma;
+ break;
+ case ncsi_dev_state_config_oem_gma:
nd->state = ncsi_dev_state_config_clear_vids;
+ ret = -1;
+
+#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC)
+ nca.type = NCSI_PKT_CMD_OEM;
+ nca.package = np->id;
+ nca.channel = nc->id;
+ ndp->pending_req_num = 1;
+ ret = ncsi_oem_handler(&nca, nc->version.mf_id);
+#endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */
+
+ if (ret < 0)
+ schedule_work(&ndp->work);
+
break;
case ncsi_dev_state_config_clear_vids:
case ncsi_dev_state_config_svf:
diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h
index 0f2087c8d42a..4d3f06be38bd 100644
--- a/net/ncsi/ncsi-pkt.h
+++ b/net/ncsi/ncsi-pkt.h
@@ -165,6 +165,14 @@ struct ncsi_rsp_oem_pkt {
unsigned char data[]; /* Payload data */
};
+/* Broadcom Response Data */
+struct ncsi_rsp_oem_bcm_pkt {
+ unsigned char ver; /* Payload Version */
+ unsigned char type; /* OEM Command type */
+ __be16 len; /* Payload Length */
+ unsigned char data[]; /* Cmd specific Data */
+};
+
/* Get Link Status */
struct ncsi_rsp_gls_pkt {
struct ncsi_rsp_pkt_hdr rsp; /* Response header */
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index d66b34749027..d052a3cafed4 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -596,19 +596,59 @@ static int ncsi_rsp_handler_snfc(struct ncsi_request *nr)
return 0;
}
+/* Response handler for Broadcom command Get Mac Address */
+static int ncsi_rsp_handler_oem_bcm_gma(struct ncsi_request *nr)
+{
+ struct ncsi_dev_priv *ndp = nr->ndp;
+ struct net_device *ndev = ndp->ndev.dev;
+ const struct net_device_ops *ops = ndev->netdev_ops;
+ struct ncsi_rsp_oem_pkt *rsp;
+ struct sockaddr saddr;
+ int ret = 0;
+
+ /* Get the response header */
+ rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
+
+ saddr.sa_family = ndev->type;
+ ndev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
+ memcpy(saddr.sa_data, &rsp->data[BCM_MAC_ADDR_OFFSET], ETH_ALEN);
+ /* Increase mac address by 1 for BMC's address */
+ saddr.sa_data[ETH_ALEN - 1]++;
+ ret = ops->ndo_set_mac_address(ndev, &saddr);
+ if (ret < 0)
+ netdev_warn(ndev, "NCSI: 'Writing mac address to device failed\n");
+
+ return ret;
+}
+
+/* Response handler for Broadcom card */
+static int ncsi_rsp_handler_oem_bcm(struct ncsi_request *nr)
+{
+ struct ncsi_rsp_oem_bcm_pkt *bcm;
+ struct ncsi_rsp_oem_pkt *rsp;
+
+ /* Get the response header */
+ rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
+ bcm = (struct ncsi_rsp_oem_bcm_pkt *)(rsp->data);
+
+ if (bcm->type == NCSI_OEM_BCM_CMD_GMA)
+ return ncsi_rsp_handler_oem_bcm_gma(nr);
+ return 0;
+}
+
static struct ncsi_rsp_oem_handler {
unsigned int mfr_id;
int (*handler)(struct ncsi_request *nr);
} ncsi_rsp_oem_handlers[] = {
{ NCSI_OEM_MFR_MLX_ID, NULL },
- { NCSI_OEM_MFR_BCM_ID, NULL }
+ { NCSI_OEM_MFR_BCM_ID, ncsi_rsp_handler_oem_bcm }
};
/* Response handler for OEM command */
static int ncsi_rsp_handler_oem(struct ncsi_request *nr)
{
- struct ncsi_rsp_oem_pkt *rsp;
struct ncsi_rsp_oem_handler *nrh = NULL;
+ struct ncsi_rsp_oem_pkt *rsp;
unsigned int mfr_id, i;
/* Get the response header */
--
2.17.1
^ permalink raw reply related
* [PATCH net-next v3 1/2] net/ncsi: Add NCSI Broadcom OEM command
From: Vijay Khemka @ 2018-10-12 18:23 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <0291dc2447720438a25c9c922252ab71e92985dd.camel@mendozajonas.com>
?On 10/11/18, 5:42 PM, "Samuel Mendoza-Jonas" <sam@mendozajonas.com> wrote:
> diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
> index 091284760d21..75504ccd1b95 100644
> --- a/net/ncsi/ncsi-manage.c
> +++ b/net/ncsi/ncsi-manage.c
> @@ -635,6 +635,39 @@ static int set_one_vid(struct ncsi_dev_priv *ndp, struct ncsi_channel *nc,
> return 0;
> }
>
> +#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC)
> +
> +/* NCSI OEM Command APIs */
> +static void ncsi_oem_gma_handler_bcm(struct ncsi_cmd_arg *nca)
> +{
> + int ret = 0;
> + unsigned char data[NCSI_OEM_BCM_CMD_GMA_LEN];
> +
> + nca->payload = NCSI_OEM_BCM_CMD_GMA_LEN;
> +
> + memset(data, 0, NCSI_OEM_BCM_CMD_GMA_LEN);
> + *(unsigned int *)data = ntohl(NCSI_OEM_MFR_BCM_ID);
> + data[5] = NCSI_OEM_BCM_CMD_GMA;
> +
> + nca->data = data;
> +
> + ret = ncsi_xmit_cmd(nca);
> + if (ret)
> + netdev_err(nca->ndp->ndev.dev,
> + "NCSI: Failed to transmit cmd 0x%x during configure\n",
> + nca->type);
> +}
> +
> +/* OEM Command handlers initialization */
> +static struct ncsi_oem_gma_handler {
> + unsigned int mfr_id;
> + void (*handler)(struct ncsi_cmd_arg *nca);
> +} ncsi_oem_gma_handlers[] = {
> + { NCSI_OEM_MFR_BCM_ID, ncsi_oem_gma_handler_bcm }
> +};
> +
> +#endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */
> +
> static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
> {
> struct ncsi_dev *nd = &ndp->ndev;
> @@ -685,6 +718,43 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
> goto error;
> }
>
> +#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC)
> + nd->state = ncsi_dev_state_config_oem_gma;
> + break;
> + case ncsi_dev_state_config_oem_gma:
> + nca.type = NCSI_PKT_CMD_OEM;
> + nca.package = np->id;
> + nca.channel = nc->id;
> + ndp->pending_req_num = 1;
> +
> + /* Check for manufacturer id and Find the handler */
> + struct ncsi_oem_gma_handler *nch = NULL;
> + int i;
> +
This has the opposite affect, now if we do compile with
CONFIG_NCSI_OEM_CMD_GET_MAC we get:
../net/ncsi/ncsi-manage.c: In function ?ncsi_configure_channel?:
../net/ncsi/ncsi-manage.c:769:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
struct ncsi_oem_gma_handler *nch = NULL;
^~~~~~
Perhaps we should lay this out slightly differently. For example we could go
through the ncsi_dev_state_config_oem_gma state regardless and call some other
function that finds and calls the handler, or does nothing if the config option
isn't set.
Regards,
Sam
Sam,
I have created a new patch v4 and introduced new function. I was just wondering if I can remove
CONFIG_NCSI_OEM_CMD_GET_MAC config all together. And it always get and set mac address if
Handler is available. Looking for your thought here.
-Vijay
^ permalink raw reply
* [PATCH net-next v4] net/ncsi: Add NCSI Broadcom OEM command
From: Samuel Mendoza-Jonas @ 2018-10-15 2:08 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20181012182008.1665690-1-vijaykhemka@fb.com>
On Fri, 2018-10-12 at 11:20 -0700, Vijay Khemka wrote:
> This patch adds OEM Broadcom commands and response handling. It also
> defines OEM Get MAC Address handler to get and configure the device.
>
> ncsi_oem_gma_handler_bcm: This handler send NCSI broadcom command for
> getting mac address.
> ncsi_rsp_handler_oem_bcm: This handles response received for all
> broadcom OEM commands.
> ncsi_rsp_handler_oem_bcm_gma: This handles get mac address response and
> set it to device.
>
> Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
> ---
> v4: updated as per comment from Sam, I was just wondering if I can remove
> NCSI_OEM_CMD_GET_MAC config option and let this code be valid always and
> it will configure mac address if there is get mac address handler for given
> manufacture id.
Hi Vijay,
We can look at handling this a different way, but I don't think we want
to unconditionally set the system's MAC address based on the OEM GMA
command. If the user wants to set a custom MAC address, or in the case of
OpenBMC for example who have their MAC address saved in flash, this will
override that value with whatever the Network Controller has saved. In
particular as it is set up it will override any MAC address every time a
channel is configured, such as during a failover event.
We *could* always send the GMA command if it is available and move the
decision whether to use the resulting address or not into the response
handler. That would simplify the ncsi_configure_channel() logic a bit.
Another idea may be to have a Netlink command to tell NCSI to ignore the
GMA result; then we could drop the config option and the system can
safely change the address if desired.
Any thoughts? I'll also ping some of the OpenBMC people and see what
their expectations are.
> +#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC)
> +
> +/* NCSI OEM Command APIs */
> +static void ncsi_oem_gma_handler_bcm(struct ncsi_cmd_arg *nca)
> +{
> + unsigned char data[NCSI_OEM_BCM_CMD_GMA_LEN];
> + int ret = 0;
> +
> + nca->payload = NCSI_OEM_BCM_CMD_GMA_LEN;
> +
> + memset(data, 0, NCSI_OEM_BCM_CMD_GMA_LEN);
> + *(unsigned int *)data = ntohl(NCSI_OEM_MFR_BCM_ID);
> + data[5] = NCSI_OEM_BCM_CMD_GMA;
> +
> + nca->data = data;
> +
> + ret = ncsi_xmit_cmd(nca);
> + if (ret)
> + netdev_err(nca->ndp->ndev.dev,
> + "NCSI: Failed to transmit cmd 0x%x during configure\n",
> + nca->type);
> +}
As a side note while unlikely we probably want to propagate the return
value of ncsi_xmit_cmd() from here; otherwise we'll miss a failure and
the configure process will stall.
Regards,
Sam
^ permalink raw reply
* [PATCH net-next v4] net/ncsi: Add NCSI Broadcom OEM command
From: Samuel Mendoza-Jonas @ 2018-10-15 3:51 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <69cae9d44cbebb2cd4f468dc710d6a97210af835.camel@mendozajonas.com>
On Mon, 2018-10-15 at 13:08 +1100, Samuel Mendoza-Jonas wrote:
> On Fri, 2018-10-12 at 11:20 -0700, Vijay Khemka wrote:
> > This patch adds OEM Broadcom commands and response handling. It also
> > defines OEM Get MAC Address handler to get and configure the device.
> >
> > ncsi_oem_gma_handler_bcm: This handler send NCSI broadcom command for
> > getting mac address.
> > ncsi_rsp_handler_oem_bcm: This handles response received for all
> > broadcom OEM commands.
> > ncsi_rsp_handler_oem_bcm_gma: This handles get mac address response and
> > set it to device.
> >
> > Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
> > ---
> > v4: updated as per comment from Sam, I was just wondering if I can remove
> > NCSI_OEM_CMD_GET_MAC config option and let this code be valid always and
> > it will configure mac address if there is get mac address handler for given
> > manufacture id.
>
> Hi Vijay,
>
> We can look at handling this a different way, but I don't think we want
> to unconditionally set the system's MAC address based on the OEM GMA
> command. If the user wants to set a custom MAC address, or in the case of
> OpenBMC for example who have their MAC address saved in flash, this will
> override that value with whatever the Network Controller has saved. In
> particular as it is set up it will override any MAC address every time a
> channel is configured, such as during a failover event.
>
> We *could* always send the GMA command if it is available and move the
> decision whether to use the resulting address or not into the response
> handler. That would simplify the ncsi_configure_channel() logic a bit.
> Another idea may be to have a Netlink command to tell NCSI to ignore the
> GMA result; then we could drop the config option and the system can
> safely change the address if desired.
>
> Any thoughts? I'll also ping some of the OpenBMC people and see what
> their expectations are.
After a bit of a think and an ask around, to quote a colleague:
> I think we'd want it handled (overall) like any other net device; the MAC
> address in the device's ROM provides a default, and is overridden by anything
> specified by userspace
Which describes what I was thinking pretty well.
So if we can have it such that the NCSI driver only sets the MAC address
_once_, and then after then does not update it again, we should be able to call
the OEM GMA command without hiding it behind a config option. So the first time
a channel was configured we store and set the MAC address given, but then on
later configure events we don't continue to update it. What do you think?
Cheers,
Sam
>
> > +#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC)
> > +
> > +/* NCSI OEM Command APIs */
> > +static void ncsi_oem_gma_handler_bcm(struct ncsi_cmd_arg *nca)
> > +{
> > + unsigned char data[NCSI_OEM_BCM_CMD_GMA_LEN];
> > + int ret = 0;
> > +
> > + nca->payload = NCSI_OEM_BCM_CMD_GMA_LEN;
> > +
> > + memset(data, 0, NCSI_OEM_BCM_CMD_GMA_LEN);
> > + *(unsigned int *)data = ntohl(NCSI_OEM_MFR_BCM_ID);
> > + data[5] = NCSI_OEM_BCM_CMD_GMA;
> > +
> > + nca->data = data;
> > +
> > + ret = ncsi_xmit_cmd(nca);
> > + if (ret)
> > + netdev_err(nca->ndp->ndev.dev,
> > + "NCSI: Failed to transmit cmd 0x%x during configure\n",
> > + nca->type);
> > +}
>
> As a side note while unlikely we probably want to propagate the return
> value of ncsi_xmit_cmd() from here; otherwise we'll miss a failure and
> the configure process will stall.
>
> Regards,
> Sam
>
^ permalink raw reply
* [PATCH net-next v4] net/ncsi: Add NCSI Broadcom OEM command
From: Vijay Khemka @ 2018-10-15 17:27 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <9c82f52a7ce3fb9052600709a10783b23ef88457.camel@mendozajonas.com>
?On 10/14/18, 8:51 PM, "Samuel Mendoza-Jonas" <sam@mendozajonas.com> wrote:
On Mon, 2018-10-15 at 13:08 +1100, Samuel Mendoza-Jonas wrote:
> On Fri, 2018-10-12 at 11:20 -0700, Vijay Khemka wrote:
> > This patch adds OEM Broadcom commands and response handling. It also
> > defines OEM Get MAC Address handler to get and configure the device.
> >
> > ncsi_oem_gma_handler_bcm: This handler send NCSI broadcom command for
> > getting mac address.
> > ncsi_rsp_handler_oem_bcm: This handles response received for all
> > broadcom OEM commands.
> > ncsi_rsp_handler_oem_bcm_gma: This handles get mac address response and
> > set it to device.
> >
> > Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
> > ---
> > v4: updated as per comment from Sam, I was just wondering if I can remove
> > NCSI_OEM_CMD_GET_MAC config option and let this code be valid always and
> > it will configure mac address if there is get mac address handler for given
> > manufacture id.
>
> Hi Vijay,
>
> We can look at handling this a different way, but I don't think we want
> to unconditionally set the system's MAC address based on the OEM GMA
> command. If the user wants to set a custom MAC address, or in the case of
> OpenBMC for example who have their MAC address saved in flash, this will
> override that value with whatever the Network Controller has saved. In
> particular as it is set up it will override any MAC address every time a
> channel is configured, such as during a failover event.
>
> We *could* always send the GMA command if it is available and move the
> decision whether to use the resulting address or not into the response
> handler. That would simplify the ncsi_configure_channel() logic a bit.
> Another idea may be to have a Netlink command to tell NCSI to ignore the
> GMA result; then we could drop the config option and the system can
> safely change the address if desired.
>
> Any thoughts? I'll also ping some of the OpenBMC people and see what
> their expectations are.
After a bit of a think and an ask around, to quote a colleague:
> I think we'd want it handled (overall) like any other net device; the MAC
> address in the device's ROM provides a default, and is overridden by anything
> specified by userspace
Which describes what I was thinking pretty well.
So if we can have it such that the NCSI driver only sets the MAC address
_once_, and then after then does not update it again, we should be able to call
the OEM GMA command without hiding it behind a config option. So the first time
a channel was configured we store and set the MAC address given, but then on
later configure events we don't continue to update it. What do you think?
Cheers,
Sam
I agree with you setting it only once. I gave a thought about config option and realize that
we should allow user to configure it. If user wants to set mac address through device tree
and not through ROM then we must not override mac set by device tree. So my proposal is
setting of mac address in response should be hidden under config option. Getting mac address
can still go without config option. Your thought?
>
> > +#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC)
> > +
> > +/* NCSI OEM Command APIs */
> > +static void ncsi_oem_gma_handler_bcm(struct ncsi_cmd_arg *nca)
> > +{
> > + unsigned char data[NCSI_OEM_BCM_CMD_GMA_LEN];
> > + int ret = 0;
> > +
> > + nca->payload = NCSI_OEM_BCM_CMD_GMA_LEN;
> > +
> > + memset(data, 0, NCSI_OEM_BCM_CMD_GMA_LEN);
> > + *(unsigned int *)data = ntohl(NCSI_OEM_MFR_BCM_ID);
> > + data[5] = NCSI_OEM_BCM_CMD_GMA;
> > +
> > + nca->data = data;
> > +
> > + ret = ncsi_xmit_cmd(nca);
> > + if (ret)
> > + netdev_err(nca->ndp->ndev.dev,
> > + "NCSI: Failed to transmit cmd 0x%x during configure\n",
> > + nca->type);
> > +}
>
> As a side note while unlikely we probably want to propagate the return
> value of ncsi_xmit_cmd() from here; otherwise we'll miss a failure and
> the configure process will stall.
>
> Regards,
> Sam
>
I will take care of this.
^ permalink raw reply
* [Potential Spoof] Re: [PATCH net-next v4] net/ncsi: Add NCSI Broadcom OEM command
From: Vijay Khemka @ 2018-10-15 17:38 UTC (permalink / raw)
To: linux-aspeed
?On 10/15/18, 10:27 AM, "Linux-aspeed on behalf of Vijay Khemka" <linux-aspeed-bounces+vijaykhemka=fb.com at lists.ozlabs.org on behalf of vijaykhemka@fb.com> wrote:
On 10/14/18, 8:51 PM, "Samuel Mendoza-Jonas" <sam@mendozajonas.com> wrote:
On Mon, 2018-10-15 at 13:08 +1100, Samuel Mendoza-Jonas wrote:
> On Fri, 2018-10-12 at 11:20 -0700, Vijay Khemka wrote:
> > This patch adds OEM Broadcom commands and response handling. It also
> > defines OEM Get MAC Address handler to get and configure the device.
> >
> > ncsi_oem_gma_handler_bcm: This handler send NCSI broadcom command for
> > getting mac address.
> > ncsi_rsp_handler_oem_bcm: This handles response received for all
> > broadcom OEM commands.
> > ncsi_rsp_handler_oem_bcm_gma: This handles get mac address response and
> > set it to device.
> >
> > Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
> > ---
> > v4: updated as per comment from Sam, I was just wondering if I can remove
> > NCSI_OEM_CMD_GET_MAC config option and let this code be valid always and
> > it will configure mac address if there is get mac address handler for given
> > manufacture id.
>
> Hi Vijay,
>
> We can look at handling this a different way, but I don't think we want
> to unconditionally set the system's MAC address based on the OEM GMA
> command. If the user wants to set a custom MAC address, or in the case of
> OpenBMC for example who have their MAC address saved in flash, this will
> override that value with whatever the Network Controller has saved. In
> particular as it is set up it will override any MAC address every time a
> channel is configured, such as during a failover event.
>
> We *could* always send the GMA command if it is available and move the
> decision whether to use the resulting address or not into the response
> handler. That would simplify the ncsi_configure_channel() logic a bit.
> Another idea may be to have a Netlink command to tell NCSI to ignore the
> GMA result; then we could drop the config option and the system can
> safely change the address if desired.
>
> Any thoughts? I'll also ping some of the OpenBMC people and see what
> their expectations are.
After a bit of a think and an ask around, to quote a colleague:
> I think we'd want it handled (overall) like any other net device; the MAC
> address in the device's ROM provides a default, and is overridden by anything
> specified by userspace
Which describes what I was thinking pretty well.
So if we can have it such that the NCSI driver only sets the MAC address
_once_, and then after then does not update it again, we should be able to call
the OEM GMA command without hiding it behind a config option. So the first time
a channel was configured we store and set the MAC address given, but then on
later configure events we don't continue to update it. What do you think?
Cheers,
Sam
I agree with you setting it only once. I gave a thought about config option and realize that
we should allow user to configure it. If user wants to set mac address through device tree
and not through ROM then we must not override mac set by device tree. So my proposal is
setting of mac address in response should be hidden under config option. Getting mac address
can still go without config option. Your thought?
or simply guard following block under config and no other function declaration guard required.
And set static variable flag in function " ncsi_oem_handler" for calling this only once.
#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC)
nca.type = NCSI_PKT_CMD_OEM;
nca.package = np->id;
nca.channel = nc->id;
ndp->pending_req_num = 1;
ret = ncsi_oem_handler(&nca, nc->version.mf_id);
#endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */
^ permalink raw reply
* [PATCH net-next v7] net/ncsi: Extend NC-SI Netlink interface to allow user space to send NC-SI command
From: David Miller @ 2018-10-16 5:01 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <cd371b0265b74889a10241a6af7e4843@AUSX13MPS302.AMER.DELL.COM>
From: <Justin.Lee1@Dell.com>
Date: Thu, 11 Oct 2018 18:07:37 +0000
> The new command (NCSI_CMD_SEND_CMD) is added to allow user space application
> to send NC-SI command to the network card.
> Also, add a new attribute (NCSI_ATTR_DATA) for transferring request and response.
>
> The work flow is as below.
>
> Request:
> User space application
> -> Netlink interface (msg)
> -> new Netlink handler - ncsi_send_cmd_nl()
> -> ncsi_xmit_cmd()
>
> Response:
> Response received - ncsi_rcv_rsp()
> -> internal response handler - ncsi_rsp_handler_xxx()
> -> ncsi_rsp_handler_netlink()
> -> ncsi_send_netlink_rsp ()
> -> Netlink interface (msg)
> -> user space application
>
> Command timeout - ncsi_request_timeout()
> -> ncsi_send_netlink_timeout ()
> -> Netlink interface (msg with zero data length)
> -> user space application
>
> Error:
> Error detected
> -> ncsi_send_netlink_err ()
> -> Netlink interface (err msg)
> -> user space application
>
>
> Signed-off-by: Justin Lee <justin.lee1@dell.com>
Applied.
^ permalink raw reply
* [Potential Spoof] Re: [PATCH net-next v4] net/ncsi: Add NCSI Broadcom OEM command
From: Samuel Mendoza-Jonas @ 2018-10-16 5:46 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <E82A7C2E-6DBF-437D-B571-5376F0122176@fb.com>
On Mon, 2018-10-15 at 17:38 +0000, Vijay Khemka wrote:
>
> ?On 10/15/18, 10:27 AM, "Linux-aspeed on behalf of Vijay Khemka" <linux-aspeed-bounces+vijaykhemka=fb.com at lists.ozlabs.org on behalf of vijaykhemka@fb.com> wrote:
>
>
>
> On 10/14/18, 8:51 PM, "Samuel Mendoza-Jonas" <sam@mendozajonas.com> wrote:
>
> On Mon, 2018-10-15 at 13:08 +1100, Samuel Mendoza-Jonas wrote:
> > On Fri, 2018-10-12 at 11:20 -0700, Vijay Khemka wrote:
> > > This patch adds OEM Broadcom commands and response handling. It also
> > > defines OEM Get MAC Address handler to get and configure the device.
> > >
> > > ncsi_oem_gma_handler_bcm: This handler send NCSI broadcom command for
> > > getting mac address.
> > > ncsi_rsp_handler_oem_bcm: This handles response received for all
> > > broadcom OEM commands.
> > > ncsi_rsp_handler_oem_bcm_gma: This handles get mac address response and
> > > set it to device.
> > >
> > > Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
> > > ---
> > > v4: updated as per comment from Sam, I was just wondering if I can remove
> > > NCSI_OEM_CMD_GET_MAC config option and let this code be valid always and
> > > it will configure mac address if there is get mac address handler for given
> > > manufacture id.
> >
> > Hi Vijay,
> >
> > We can look at handling this a different way, but I don't think we want
> > to unconditionally set the system's MAC address based on the OEM GMA
> > command. If the user wants to set a custom MAC address, or in the case of
> > OpenBMC for example who have their MAC address saved in flash, this will
> > override that value with whatever the Network Controller has saved. In
> > particular as it is set up it will override any MAC address every time a
> > channel is configured, such as during a failover event.
> >
> > We *could* always send the GMA command if it is available and move the
> > decision whether to use the resulting address or not into the response
> > handler. That would simplify the ncsi_configure_channel() logic a bit.
> > Another idea may be to have a Netlink command to tell NCSI to ignore the
> > GMA result; then we could drop the config option and the system can
> > safely change the address if desired.
> >
> > Any thoughts? I'll also ping some of the OpenBMC people and see what
> > their expectations are.
>
> After a bit of a think and an ask around, to quote a colleague:
> > I think we'd want it handled (overall) like any other net device; the MAC
> > address in the device's ROM provides a default, and is overridden by anything
> > specified by userspace
>
> Which describes what I was thinking pretty well.
> So if we can have it such that the NCSI driver only sets the MAC address
> _once_, and then after then does not update it again, we should be able to call
> the OEM GMA command without hiding it behind a config option. So the first time
> a channel was configured we store and set the MAC address given, but then on
> later configure events we don't continue to update it. What do you think?
>
> Cheers,
> Sam
>
> I agree with you setting it only once. I gave a thought about config option and realize that
> we should allow user to configure it. If user wants to set mac address through device tree
> and not through ROM then we must not override mac set by device tree. So my proposal is
> setting of mac address in response should be hidden under config option. Getting mac address
> can still go without config option. Your thought?
>
> or simply guard following block under config and no other function declaration guard required.
> And set static variable flag in function " ncsi_oem_handler" for calling this only once.
>
> #if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC)
> nca.type = NCSI_PKT_CMD_OEM;
> nca.package = np->id;
> nca.channel = nc->id;
> ndp->pending_req_num = 1;
> ret = ncsi_oem_handler(&nca, nc->version.mf_id);
> #endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */
>
Hi Vijay,
Either way is likely fine; although you might need to take care you don't
get an unused function warning depending on how you guard
ncsi_oem_handler(). Also a flag in the ncsi_dev_priv struct could be
easier to keep track of than having a static variable in the handler
function.
Sam
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox