[Qemu-devel] [PATCH v2 0/3] slirp: NC-SI enhancements

[Qemu-devel] [PATCH v2 0/3] slirp: NC-SI enhancements

[Cédric Le Goater]

Hello,

Here is a couple of enhancements and fixes for the NC-SI backend used
on the Aspeed SoC when soldered on OpenPOWER boards. 

Thanks,

C.

Cédric Le Goater (3):
  slirp/ncsi: fix "Get Version ID" payload length
  slirp/ncsi: add a "Get Parameters" response
  slirp/ncsi: add checksum support

 slirp/ncsi.c | 51 ++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 44 insertions(+), 7 deletions(-)

-- 
2.13.6

[Qemu-devel] [PATCH v2 1/3] slirp/ncsi: fix "Get Version ID" payload length

[Cédric Le Goater]

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 slirp/ncsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/slirp/ncsi.c b/slirp/ncsi.c
index d12ba3e494b0..02d0e9def3e8 100644
--- a/slirp/ncsi.c
+++ b/slirp/ncsi.c
@@ -60,7 +60,7 @@ static const struct ncsi_rsp_handler {
         { NCSI_PKT_RSP_EGMF,    4, NULL },
         { NCSI_PKT_RSP_DGMF,    4, NULL },
         { NCSI_PKT_RSP_SNFC,    4, NULL },
-        { NCSI_PKT_RSP_GVI,    36, NULL },
+        { NCSI_PKT_RSP_GVI,    40, NULL },
         { NCSI_PKT_RSP_GC,     32, ncsi_rsp_handler_gc },
         { NCSI_PKT_RSP_GP,     -1, NULL },
         { NCSI_PKT_RSP_GCPS,  172, NULL },
-- 
2.13.6

[Qemu-devel] [PATCH v2 2/3] slirp/ncsi: add a "Get Parameters" response

[Cédric Le Goater]

Command 0x17 'Get Parameters' is used to get configuration parameter
values currently in effect on the controller and it is mandatory in
the NS-CI specification.

Provide a minimum response to exercise the kernel.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---

 Changes since v1:

 - removed bogus MAC addr and initialized VLAN filters to zero.

 slirp/ncsi.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/slirp/ncsi.c b/slirp/ncsi.c
index 02d0e9def3e8..7b3fff207ab3 100644
--- a/slirp/ncsi.c
+++ b/slirp/ncsi.c
@@ -35,6 +35,20 @@ static int ncsi_rsp_handler_gls(struct ncsi_rsp_pkt_hdr *rnh)
     return 0;
 }
 
+/* Get Parameters */
+static int ncsi_rsp_handler_gp(struct ncsi_rsp_pkt_hdr *rnh)
+{
+    struct ncsi_rsp_gp_pkt *rsp = (struct ncsi_rsp_gp_pkt *) rnh;
+
+    /* no MAC address filters or VLAN filters on the channel */
+    rsp->mac_cnt = 0;
+    rsp->mac_enable = 0;
+    rsp->vlan_cnt = 0;
+    rsp->vlan_enable = 0;
+
+    return 0;
+}
+
 static const struct ncsi_rsp_handler {
         unsigned char   type;
         int             payload;
@@ -62,7 +76,7 @@ static const struct ncsi_rsp_handler {
         { NCSI_PKT_RSP_SNFC,    4, NULL },
         { NCSI_PKT_RSP_GVI,    40, NULL },
         { NCSI_PKT_RSP_GC,     32, ncsi_rsp_handler_gc },
-        { NCSI_PKT_RSP_GP,     -1, NULL },
+        { NCSI_PKT_RSP_GP,     40, ncsi_rsp_handler_gp },
         { NCSI_PKT_RSP_GCPS,  172, NULL },
         { NCSI_PKT_RSP_GNS,   172, NULL },
         { NCSI_PKT_RSP_GNPTS, 172, NULL },
-- 
2.13.6

[Qemu-devel] [PATCH v2 3/3] slirp/ncsi: add checksum support

[Cédric Le Goater]

The checksum field of a NC-SI packet contains a value that may be
included in each command and response. The verification is optional
but the Linux driver does so when a non-zero value is provided. Let's
extend the model to compute the checksum value and exercise a little
more the Linux driver.

See section "8.2.2.3 - 2's Complement Checksum Compensation" in the
Network Controller Sideband Interface (NC-SI) Specification for more
details.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---

 Changes since v1:

 - updated Copyright
 - used uint16_t instead of chars to compute the checksum
 
 slirp/ncsi.c | 33 ++++++++++++++++++++++++++++-----
 1 file changed, 28 insertions(+), 5 deletions(-)

diff --git a/slirp/ncsi.c b/slirp/ncsi.c
index 7b3fff207ab3..7116034afc20 100644
--- a/slirp/ncsi.c
+++ b/slirp/ncsi.c
@@ -1,7 +1,7 @@
 /*
  * NC-SI (Network Controller Sideband Interface) "echo" model
  *
- * Copyright (C) 2016 IBM Corp.
+ * Copyright (C) 2016-2018 IBM Corp.
  *
  * This code is licensed under the GPL version 2 or later. See the
  * COPYING file in the top-level directory.
@@ -11,6 +11,23 @@
 
 #include "ncsi-pkt.h"
 
+static uint32_t ncsi_calculate_checksum(uint16_t *data, int len)
+{
+    uint32_t checksum = 0;
+    int i;
+
+    /*
+     * 32-bit unsigned sum of the NC-SI packet header and NC-SI packet
+     * payload interpreted as a series of 16-bit unsigned integer values.
+     */
+    for (i = 0; i < len; i++) {
+        checksum += htons(data[i]);
+    }
+
+    checksum = (~checksum + 1);
+    return checksum;
+}
+
 /* Get Capabilities */
 static int ncsi_rsp_handler_gc(struct ncsi_rsp_pkt_hdr *rnh)
 {
@@ -101,6 +118,9 @@ void ncsi_input(Slirp *slirp, const uint8_t *pkt, int pkt_len)
         (ncsi_reply + ETH_HLEN);
     const struct ncsi_rsp_handler *handler = NULL;
     int i;
+    int ncsi_rsp_len = sizeof(*nh);
+    uint32_t checksum;
+    uint32_t *pchecksum;
 
     memset(ncsi_reply, 0, sizeof(ncsi_reply));
 
@@ -130,15 +150,18 @@ void ncsi_input(Slirp *slirp, const uint8_t *pkt, int pkt_len)
             /* TODO: handle errors */
             handler->handler(rnh);
         }
+        ncsi_rsp_len += handler->payload;
     } else {
         rnh->common.length = 0;
         rnh->code          = htons(NCSI_PKT_RSP_C_UNAVAILABLE);
         rnh->reason        = htons(NCSI_PKT_RSP_R_UNKNOWN);
     }
 
-    /* TODO: add a checksum at the end of the frame but the specs
-     * allows it to be zero */
+    /* Add the optional checksum at the end of the frame. */
+    checksum = ncsi_calculate_checksum((uint16_t *) rnh, ncsi_rsp_len);
+    pchecksum = (uint32_t *)((void *) rnh + ncsi_rsp_len);
+    *pchecksum = htonl(checksum);
+    ncsi_rsp_len += 4;
 
-    slirp_output(slirp->opaque, ncsi_reply, ETH_HLEN + sizeof(*nh) +
-                 (handler ? handler->payload : 0) + 4);
+    slirp_output(slirp->opaque, ncsi_reply, ETH_HLEN + ncsi_rsp_len);
 }
-- 
2.13.6

Re: [Qemu-devel] [PATCH v2 2/3] slirp/ncsi: add a "Get Parameters" response

[Philippe Mathieu-Daudé]

On 05/30/2018 03:10 AM, Cédric Le Goater wrote:
> Command 0x17 'Get Parameters' is used to get configuration parameter
> values currently in effect on the controller and it is mandatory in
> the NS-CI specification.
> 
> Provide a minimum response to exercise the kernel.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
> 
>  Changes since v1:
> 
>  - removed bogus MAC addr and initialized VLAN filters to zero.
> 
>  slirp/ncsi.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/slirp/ncsi.c b/slirp/ncsi.c
> index 02d0e9def3e8..7b3fff207ab3 100644
> --- a/slirp/ncsi.c
> +++ b/slirp/ncsi.c
> @@ -35,6 +35,20 @@ static int ncsi_rsp_handler_gls(struct ncsi_rsp_pkt_hdr *rnh)
>      return 0;
>  }
>  
> +/* Get Parameters */
> +static int ncsi_rsp_handler_gp(struct ncsi_rsp_pkt_hdr *rnh)
> +{
> +    struct ncsi_rsp_gp_pkt *rsp = (struct ncsi_rsp_gp_pkt *) rnh;
> +
> +    /* no MAC address filters or VLAN filters on the channel */
> +    rsp->mac_cnt = 0;
> +    rsp->mac_enable = 0;
> +    rsp->vlan_cnt = 0;
> +    rsp->vlan_enable = 0;
> +
> +    return 0;
> +}
> +
>  static const struct ncsi_rsp_handler {
>          unsigned char   type;
>          int             payload;
> @@ -62,7 +76,7 @@ static const struct ncsi_rsp_handler {
>          { NCSI_PKT_RSP_SNFC,    4, NULL },
>          { NCSI_PKT_RSP_GVI,    40, NULL },
>          { NCSI_PKT_RSP_GC,     32, ncsi_rsp_handler_gc },
> -        { NCSI_PKT_RSP_GP,     -1, NULL },
> +        { NCSI_PKT_RSP_GP,     40, ncsi_rsp_handler_gp },
>          { NCSI_PKT_RSP_GCPS,  172, NULL },
>          { NCSI_PKT_RSP_GNS,   172, NULL },
>          { NCSI_PKT_RSP_GNPTS, 172, NULL },
> 

Re: [Qemu-devel] [PATCH v2 0/3] slirp: NC-SI enhancements

[Samuel Thibault]

Hello,

Cédric Le Goater, le mer. 30 mai 2018 08:10:32 +0200, a ecrit:
> Here is a couple of enhancements and fixes for the NC-SI backend used
> on the Aspeed SoC when soldered on OpenPOWER boards. 

Applied to my tree, thanks!

(my train is 2h late, so I'll probably have the time to pull-request it
into master).

Samuel