All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PULL 4/7] iotests: remove LUKS support from test 226
From: Kevin Wolf @ 2018-07-23 16:45 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel, peter.maydell
In-Reply-To: <20180723164530.26599-1-kwolf@redhat.com>

From: John Snow <jsnow@redhat.com>

This test doesn't actually care about the format anyway, it just
supports "all formats" as a convenience. LUKS however does not use a
simple image filename which confuses this iotest.

We can simply skip the test for formats that use IMGOPTSSYNTAX for
their filenames without missing much coverage.

Signed-off-by: John Snow <jsnow@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/226 | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/qemu-iotests/226 b/tests/qemu-iotests/226
index 460aea2fc9..34987d43f9 100755
--- a/tests/qemu-iotests/226
+++ b/tests/qemu-iotests/226
@@ -41,6 +41,9 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 # Generic format, but tests file-protocol specific error handling
 _supported_fmt generic
+if [ "$IMGOPTSSYNTAX" = "true" ]; then
+    _unsupported_fmt $IMGFMT
+fi
 _supported_proto file
 _supported_os Linux
 
-- 
2.13.6

^ permalink raw reply related

* [Qemu-devel] [PULL 3/7] qemu-img: avoid overflow of min_sparse parameter
From: Kevin Wolf @ 2018-07-23 16:45 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel, peter.maydell
In-Reply-To: <20180723164530.26599-1-kwolf@redhat.com>

From: Peter Lieven <pl@kamp.de>

the min_sparse convert parameter can overflow (e.g. -S 1024G)
in the conversion from int64_t to int resulting in a negative
min_sparse parameter. Avoid this by limiting the valid parameters
to sane values. In fact anything exceeding the convert buffer size
is also pointless. While at it also forbid values that are non
multiple of 512 to avoid undesired behaviour. For instance, values
between 1 and 511 were legal, but resulted in full allocation.

Cc: qemu-stable@nongnu.org
Signed-off-by: Peter Lieven <pl@kamp.de>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qemu-img.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 4a7ce43dc9..9b7506b8ae 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -2005,6 +2005,8 @@ static int convert_do_copy(ImgConvertState *s)
     return s->ret;
 }
 
+#define MAX_BUF_SECTORS 32768
+
 static int img_convert(int argc, char **argv)
 {
     int c, bs_i, flags, src_flags = 0;
@@ -2100,8 +2102,12 @@ static int img_convert(int argc, char **argv)
             int64_t sval;
 
             sval = cvtnum(optarg);
-            if (sval < 0) {
-                error_report("Invalid minimum zero buffer size for sparse output specified");
+            if (sval < 0 || sval & (BDRV_SECTOR_SIZE - 1) ||
+                sval / BDRV_SECTOR_SIZE > MAX_BUF_SECTORS) {
+                error_report("Invalid buffer size for sparse output specified. "
+                    "Valid sizes are multiples of %llu up to %llu. Select "
+                    "0 to disable sparse detection (fully allocates output).",
+                    BDRV_SECTOR_SIZE, MAX_BUF_SECTORS * BDRV_SECTOR_SIZE);
                 goto fail_getopt;
             }
 
@@ -2385,9 +2391,9 @@ static int img_convert(int argc, char **argv)
     }
 
     /* increase bufsectors from the default 4096 (2M) if opt_transfer
-     * or discard_alignment of the out_bs is greater. Limit to 32768 (16MB)
-     * as maximum. */
-    s.buf_sectors = MIN(32768,
+     * or discard_alignment of the out_bs is greater. Limit to
+     * MAX_BUF_SECTORS as maximum which is currently 32768 (16MB). */
+    s.buf_sectors = MIN(MAX_BUF_SECTORS,
                         MAX(s.buf_sectors,
                             MAX(out_bs->bl.opt_transfer >> BDRV_SECTOR_BITS,
                                 out_bs->bl.pdiscard_alignment >>
-- 
2.13.6

^ permalink raw reply related

* [Qemu-devel] [PULL 2/7] block: Fix typos in comments (found by codespell)
From: Kevin Wolf @ 2018-07-23 16:45 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel, peter.maydell
In-Reply-To: <20180723164530.26599-1-kwolf@redhat.com>

From: Stefan Weil <sw@weilnetz.de>

Signed-off-by: Stefan Weil <sw@weilnetz.de>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c         | 2 +-
 block/backup.c  | 4 ++--
 block/curl.c    | 2 +-
 block/gluster.c | 2 +-
 block/vhdx.c    | 2 +-
 5 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/block.c b/block.c
index a2fe05ea96..39f373e035 100644
--- a/block.c
+++ b/block.c
@@ -3002,7 +3002,7 @@ BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue,
  *
  * Reopens all BDS specified in the queue, with the appropriate
  * flags.  All devices are prepared for reopen, and failure of any
- * device will cause all device changes to be abandonded, and intermediate
+ * device will cause all device changes to be abandoned, and intermediate
  * data cleaned up.
  *
  * If all devices prepare successfully, then the changes are committed
diff --git a/block/backup.c b/block/backup.c
index 319fc922e8..8630d32926 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -91,7 +91,7 @@ static void cow_request_end(CowRequest *req)
 }
 
 /* Copy range to target with a bounce buffer and return the bytes copied. If
- * error occured, return a negative error number */
+ * error occurred, return a negative error number */
 static int coroutine_fn backup_cow_with_bounce_buffer(BackupBlockJob *job,
                                                       int64_t start,
                                                       int64_t end,
@@ -148,7 +148,7 @@ fail:
 
 }
 
-/* Copy range to target and return the bytes copied. If error occured, return a
+/* Copy range to target and return the bytes copied. If error occurred, return a
  * negative error number. */
 static int coroutine_fn backup_cow_with_offload(BackupBlockJob *job,
                                                 int64_t start,
diff --git a/block/curl.c b/block/curl.c
index aa42535783..229bb84a27 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -804,7 +804,7 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags,
     }
     /* Prior CURL 7.19.4 return value of 0 could mean that the file size is not
      * know or the size is zero. From 7.19.4 CURL returns -1 if size is not
-     * known and zero if it is realy zero-length file. */
+     * known and zero if it is really zero-length file. */
 #if LIBCURL_VERSION_NUM >= 0x071304
     if (d < 0) {
         pstrcpy(state->errmsg, CURL_ERROR_SIZE,
diff --git a/block/gluster.c b/block/gluster.c
index a4e1c8ecd8..4fd55a9cc5 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -1326,7 +1326,7 @@ static int qemu_gluster_has_zero_init(BlockDriverState *bs)
  * If @start is in a trailing hole or beyond EOF, return -ENXIO.
  * If we can't find out, return a negative errno other than -ENXIO.
  *
- * (Shamefully copied from file-posix.c, only miniscule adaptions.)
+ * (Shamefully copied from file-posix.c, only minuscule adaptions.)
  */
 static int find_allocation(BlockDriverState *bs, off_t start,
                            off_t *data, off_t *hole)
diff --git a/block/vhdx.c b/block/vhdx.c
index 4d0819750f..0795ca1985 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -185,7 +185,7 @@ uint32_t vhdx_checksum_calc(uint32_t crc, uint8_t *buf, size_t size,
 /* Validates the checksum of the buffer, with an in-place CRC.
  *
  * Zero is substituted during crc calculation for the original crc field,
- * and the crc field is restored afterwards.  But the buffer will be modifed
+ * and the crc field is restored afterwards.  But the buffer will be modified
  * during the calculation, so this may not be not suitable for multi-threaded
  * use.
  *
-- 
2.13.6

^ permalink raw reply related

* [Qemu-devel] [PULL 0/7] Block layer patches
From: Kevin Wolf @ 2018-07-23 16:45 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel, peter.maydell

The following changes since commit 55b1f14cefcb19ce6d5e28c4c83404230888aa7e:

  Merge remote-tracking branch 'remotes/vivier2/tags/linux-user-for-3.0-pull-request' into staging (2018-07-23 14:03:14 +0100)

are available in the git repository at:

  git://repo.or.cz/qemu/kevin.git tags/for-upstream

for you to fetch changes up to 3e31b4e17064d22e533071aaa57d3f01499ef910:

  block/vvfat: Disable debug message by default (2018-07-23 16:50:43 +0200)

----------------------------------------------------------------
Block layer patches:

- vvfat: Disable debug message by default
- qemu-iotests fixes
- Fix typos in comments

----------------------------------------------------------------
John Snow (1):
      iotests: remove LUKS support from test 226

Kevin Wolf (1):
      qemu-iotests: Use host_device instead of file in 149

Max Reitz (2):
      iotest: Fix filtering order in 226
      iotests: Disallow compat=0.10 in 223

Peter Lieven (1):
      qemu-img: avoid overflow of min_sparse parameter

Stefan Weil (1):
      block: Fix typos in comments (found by codespell)

Thomas Huth (1):
      block/vvfat: Disable debug message by default

 block.c                    |   2 +-
 block/backup.c             |   4 +-
 block/curl.c               |   2 +-
 block/gluster.c            |   2 +-
 block/vhdx.c               |   2 +-
 block/vvfat.c              |   4 +-
 qemu-img.c                 |  16 ++-
 tests/qemu-iotests/149     |   2 +-
 tests/qemu-iotests/149.out | 344 ++++++++++++++++++++++-----------------------
 tests/qemu-iotests/223     |   2 +
 tests/qemu-iotests/226     |   7 +-
 11 files changed, 199 insertions(+), 188 deletions(-)

^ permalink raw reply

* option: Do not try to bind to ADB interfaces
From: Romain Izard @ 2018-07-23 16:45 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Johan Hovold, linux-usb, LKML, stable

2018-07-23 16:08 GMT+02:00 Greg Kroah-Hartman <gregkh@linuxfoundation.org>:
> On Mon, Jul 23, 2018 at 04:02:20PM +0200, Romain Izard wrote:
>> Some modems now use the Android Debug Bridge to provide a debugging
>> interface, and some phones can also export serial ports managed by the
>> "option" driver.
>>
>> The ADB daemon running in userspace tries to use USB interfaces with
>> bDeviceClass=0xFF, bDeviceSubClass=0x42, bDeviceProtocol=1
>>
>> Prevent the option driver from binding to those interfaces, as they
>> will not be serial ports.
>>
>> This can fix issues like:
>> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=781256
>>
>> Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
>> Cc: stable <stable@vger.kernel.org>
>> ---
>>  drivers/usb/serial/option.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
>> index 664e61f16b6a..f98943a57ff0 100644
>> --- a/drivers/usb/serial/option.c
>> +++ b/drivers/usb/serial/option.c
>> @@ -1987,6 +1987,12 @@ static int option_probe(struct usb_serial *serial,
>>       if (iface_desc->bInterfaceClass == USB_CLASS_MASS_STORAGE)
>>               return -ENODEV;
>>
>> +     /* Do not bind Android Debug Bridge interfaces */
>> +     if (iface_desc->bInterfaceClass == USB_CLASS_VENDOR_SPEC &&
>> +             iface_desc->bInterfaceSubClass == 0x42 &&
>> +             iface_desc->bInterfaceProtocol == 1)
>> +             return -ENODEV;
>
> Shouldn't you also check the vendor/product id as well?  Otherwise this
> has the potential to match random devices that are not really adb
> devices.

The only random devices are those that already match with the option driver,
either with the whole device or the whole reserved class. It reduces the
amount of potentially affected devices.

Among those, I do not expect any of them to use 0xff,0x42,0x01 for a
serial port. But if it occurred, it would be necessary to revert this change as
no userspace hack would allow to rebind the interface.

As this is only an intuition, please discard this patch if you have any doubt
about this.

Best regards,

^ permalink raw reply

* [PATCH xf86-video-amdgpu] Use strcpy for RandR output property names
From: Michel Dänzer @ 2018-07-23 16:45 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

From: Michel Dänzer <michel.daenzer@amd.com>

Instead of strncpy with the string length. Avoids new warnings with GCC
8:

../../src/drmmode_display.c: In function ‘drmmode_output_create_resources’:
../../src/drmmode_display.c:2240:2: warning: ‘strncpy’ output truncated before terminating nul copying 8 bytes from a string of the same length [-Wstringop-truncation]
  strncpy(tearfree_prop->name, "TearFree", 8);
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../src/drmmode_display.c:2244:2: warning: ‘strncpy’ output truncated before terminating nul copying 3 bytes from a string of the same length [-Wstringop-truncation]
  strncpy(tearfree_prop->enums[0].name, "off", 3);
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../src/drmmode_display.c:2245:2: warning: ‘strncpy’ output truncated before terminating nul copying 2 bytes from a string of the same length [-Wstringop-truncation]
  strncpy(tearfree_prop->enums[1].name, "on", 2);
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../src/drmmode_display.c:2247:2: warning: ‘strncpy’ output truncated before terminating nul copying 4 bytes from a string of the same length [-Wstringop-truncation]
  strncpy(tearfree_prop->enums[2].name, "auto", 4);
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
---
 src/drmmode_display.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/drmmode_display.c b/src/drmmode_display.c
index b3e754005..92f58c157 100644
--- a/src/drmmode_display.c
+++ b/src/drmmode_display.c
@@ -2211,14 +2211,14 @@ static void drmmode_output_create_resources(xf86OutputPtr output)
 	/* Userspace-only property for TearFree */
 	tearfree_prop = calloc(1, sizeof(*tearfree_prop));
 	tearfree_prop->flags = DRM_MODE_PROP_ENUM;
-	strncpy(tearfree_prop->name, "TearFree", 8);
+	strcpy(tearfree_prop->name, "TearFree");
 	tearfree_prop->count_enums = 3;
 	tearfree_prop->enums = calloc(tearfree_prop->count_enums,
 				      sizeof(*tearfree_prop->enums));
-	strncpy(tearfree_prop->enums[0].name, "off", 3);
-	strncpy(tearfree_prop->enums[1].name, "on", 2);
+	strcpy(tearfree_prop->enums[0].name, "off");
+	strcpy(tearfree_prop->enums[1].name, "on");
 	tearfree_prop->enums[1].value = 1;
-	strncpy(tearfree_prop->enums[2].name, "auto", 4);
+	strcpy(tearfree_prop->enums[2].name, "auto");
 	tearfree_prop->enums[2].value = 2;
 	drmmode_output->props[j].mode_prop = tearfree_prop;
 	drmmode_output->props[j].value = info->tear_free;
-- 
2.18.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related

* RE: P.O
From: Simon Cook  @ 2018-07-22 21:46 UTC (permalink / raw)
  To: Recipients


[-- Attachment #1.1: Mail message body --]
[-- Type: text/plain, Size: 125 bytes --]

FYI
  
 Reference to the subject above, Please kindly find the attached P.O and do the needful.
  
 Sent from my Iphone

[-- Attachment #1.2: Mail message body --]
[-- Type: text/html, Size: 1737 bytes --]

[-- Attachment #2: P.O.jar --]
[-- Type: application/octet-stream, Size: 562439 bytes --]

^ permalink raw reply

* Re: [PATCH v2] Documentation: proc.txt: Adding 'HardwareCorrupted' field and description.
From: Prashant Dhamdhere @ 2018-07-23 16:44 UTC (permalink / raw)
  To: Jonathan Corbet; +Cc: linux-doc, linux-kernel
In-Reply-To: <20180723092539.177749b8@lwn.net>

On Mon, Jul 23, 2018 at 8:55 PM, Jonathan Corbet <corbet@lwn.net> wrote:
> On Fri, 13 Jul 2018 22:58:06 +0530
> Prashant Dhamdhere <pdhamdhe@redhat.com> wrote:
>
>> This patch talks about the 'HardwareCorrupted' field currently missing from
>> the 'meminfo' section of 'proc.txt' file. It also includes short description
>> of 'HardwareCorrupted' field.
>
> I've gone ahead and applied this.  Please note, though, that the "this
> patch..." style of changelogging strongly irritates some developers;
> please use an imperative style in the future.  I rewrote the changelog to:
>
>         Documentation: proc.txt: Adding 'HardwareCorrupted' field and
>         description.
>
>         Fill in missing documentation for the HardwareCorrupted field in
>         proc.txt.
>
> Thanks,
>
> jon


Sure. Thank you for correcting it.

--
Prashant

^ permalink raw reply

* Re: [PATCH v2] Documentation: proc.txt: Adding 'HardwareCorrupted' field and description.
From: Prashant Dhamdhere @ 2018-07-23 16:44 UTC (permalink / raw)
  To: Jonathan Corbet; +Cc: linux-doc, linux-kernel
In-Reply-To: <20180723092539.177749b8@lwn.net>

On Mon, Jul 23, 2018 at 8:55 PM, Jonathan Corbet <corbet@lwn.net> wrote:
> On Fri, 13 Jul 2018 22:58:06 +0530
> Prashant Dhamdhere <pdhamdhe@redhat.com> wrote:
>
>> This patch talks about the 'HardwareCorrupted' field currently missing from
>> the 'meminfo' section of 'proc.txt' file. It also includes short description
>> of 'HardwareCorrupted' field.
>
> I've gone ahead and applied this.  Please note, though, that the "this
> patch..." style of changelogging strongly irritates some developers;
> please use an imperative style in the future.  I rewrote the changelog to:
>
>         Documentation: proc.txt: Adding 'HardwareCorrupted' field and
>         description.
>
>         Fill in missing documentation for the HardwareCorrupted field in
>         proc.txt.
>
> Thanks,
>
> jon


Sure. Thank you for correcting it.

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

^ permalink raw reply

* ✓ Fi.CI.IGT: success for drm/i915: Skip repeated calls to i915_gem_set_wedged()
From: Patchwork @ 2018-07-23 16:44 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx
In-Reply-To: <20180723145335.24579-1-chris@chris-wilson.co.uk>

== Series Details ==

Series: drm/i915: Skip repeated calls to i915_gem_set_wedged()
URL   : https://patchwork.freedesktop.org/series/47067/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4528_full -> Patchwork_9747_full =

== Summary - SUCCESS ==

  No regressions found.

  

== Known issues ==

  Here are the changes found in Patchwork_9747_full that come from known issues:

  === IGT changes ===

    ==== Possible fixes ====

    igt@kms_flip@flip-vs-expired-vblank-interruptible:
      shard-glk:          FAIL (fdo#105363) -> PASS

    
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4528 -> Patchwork_9747

  CI_DRM_4528: 9e2763473a5f24fa8848e5508cae60c3b5af3cf3 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4571: 65fccc149b85968cdce4737266b056059c1510f3 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9747: 8c8dac2e80e2deaa195e17732c979a8949325c01 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9747/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply

* Re: [PATCH] net: axienet: Fix double deregister of mdio
From: David Miller @ 2018-07-23 16:43 UTC (permalink / raw)
  To: shubhrajyoti.datta; +Cc: netdev, michal.simek, linux-kernel
In-Reply-To: <1532335127-10683-1-git-send-email-shubhrajyoti.datta@xilinx.com>

From: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
Date: Mon, 23 Jul 2018 14:08:47 +0530

> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c b/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c
> index 16c3bfb..757a3b3 100644
> --- a/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c
> +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c
> @@ -218,6 +218,7 @@ int axienet_mdio_setup(struct axienet_local *lp, struct device_node *np)
>         ret = of_mdiobus_register(bus, np1);
>         if (ret) {
>                 mdiobus_free(bus);
> +               lp->mii_bus = NULL;
>                 return ret;
>         }

Your patch was corrupted by your email client, in particular it has transformed TAB
characters into SPACEs.

Please fix this, email the patch to yourself as a test, and please do
not send the patch here to the list again until you can successfully
apply the patch you receive in that test email.

> This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

This is inappropriate for postings to this mailing list, please disable this footer
if you would like to post here.

Thank you.

^ permalink raw reply

* Re: python-btrfs & btrfs-heatmap ebuilds now available for Gentoo
From: Hans van Kranenburg @ 2018-07-23 15:41 UTC (permalink / raw)
  To: Holger Hoffstätte, linux-btrfs
In-Reply-To: <2356e8d0-3dcd-34c5-bfc7-b01340fc9cef@applied-asynchrony.com>

On 07/23/2018 04:25 PM, Holger Hoffstätte wrote:
> 
> I wanted to migrate my collection of questionable shell scripts for btrfs
> maintenance/inspection to a more stable foundation and therefore created
> Gentoo ebuilds for Hans van Kranenburg's excellent python-btrfs [1] and
> btrfs-heatmap [2] packages.
> 
> They can be found in my overlay at:
> 
> https://github.com/hhoffstaette/portage/tree/master/sys-fs/python-btrfs
> https://github.com/hhoffstaette/portage/tree/master/sys-fs/btrfs-heatmap

Thanks for doing this!

And now you're making us curious about those
not-so-questionable-any-more scripts. :D

> Both are curently only available for python-3.6 since a change in 3.7
> broke the existing python API [3]. As soon as that is fixed I'll add 3.7
> to the PYTHON_COMPAT entries.

Yes, thanks for testing, I'll have a look at it and will probably also
ask you to help testing the fix.

> btrfs-heatmap properly uses the python-exec wrapper and therefore works
> regardless of currently selected default python version.  :)
> 
> I hope this is useful to someone.

I bet it will. ;-]

-- 
Hans van Kranenburg

^ permalink raw reply

* Re: [Qemu-devel] [PATCH] block/file-posix: add bdrv_attach_aio_context callback for host dev and cdrom
From: Farhan Ali @ 2018-07-23 16:42 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: Nishanth Aravamudan, Eric Blake, Kevin Wolf, John Snow, Max Reitz,
	Fam Zheng, Paolo Bonzini, qemu-block, qemu-devel
In-Reply-To: <20180723163011.GH20596@stefanha-x1.localdomain>



On 07/23/2018 12:30 PM, Stefan Hajnoczi wrote:
> On Fri, Jul 20, 2018 at 03:11:14PM -0400, Farhan Ali wrote:
>> I am seeing another issue pop up, in a different test. Even though it's a
>> different assertion, it might be related based on the call trace.
> 
> Which test case?

This test case involved one guest with 2 disks, with an iothread for 
each disk. The guest was running a memory workload.

> 
>> Stack trace of thread 276199:
>> #0  0x000003ff8473e274 raise (libc.so.6)
>> #1  0x000003ff847239a8 abort (libc.so.6)
>> #2  0x000003ff847362ce __assert_fail_base (libc.so.6)
>> #3  0x000003ff8473634c __assert_fail (libc.so.6)
>> #4  0x000002aa30aba0c4 iov_memset (qemu-system-s390x)
>> #5  0x000002aa30aba9a6 qemu_iovec_memset (qemu-system-s390x)
>> #6  0x000002aa30a23e88 qemu_laio_process_completion (qemu-system-s390x)
> 
> What are the values of laiocb->qiov->size and laiocb->ret?

The laiocb->qiov->size was 4096 and laiocb->ret was 8192

> 
>> #7  0x000002aa30a23f68 qemu_laio_process_completions (qemu-system-s390x)
>> #8  0x000002aa30a2418e qemu_laio_process_completions_and_submit
>> (qemu-system-s390x)
>> #9  0x000002aa30a24220 qemu_laio_poll_cb (qemu-system-s390x)
>> #10 0x000002aa30ab22c4 run_poll_handlers_once (qemu-system-s390x)
>> #11 0x000002aa30ab2e78 aio_poll (qemu-system-s390x)
>> #12 0x000002aa30a29f4e bdrv_do_drained_begin (qemu-system-s390x)
>> #13 0x000002aa30a2a276 bdrv_drain (qemu-system-s390x)
>> #14 0x000002aa309d45aa bdrv_set_aio_context (qemu-system-s390x)
>> #15 0x000002aa3085acfe virtio_blk_data_plane_stop (qemu-system-s390x)
>> #16 0x000002aa3096994c virtio_bus_stop_ioeventfd.part.1 (qemu-system-s390x)
>> #17 0x000002aa3087d1d6 virtio_vmstate_change (qemu-system-s390x)
>> #18 0x000002aa308e8a12 vm_state_notify (qemu-system-s390x)
>> #19 0x000002aa3080ed54 do_vm_stop (qemu-system-s390x)
>> #20 0x000002aa307bea04 main (qemu-system-s390x)
>> #21 0x000003ff84723dd2 __libc_start_main (libc.so.6)
>> #22 0x000002aa307c0414 _start (qemu-system-s390x)
>>
>>
>> The failing assertion is:
>>
>> qemu-kvm: util/iov.c:78: iov_memset: Assertion `offset == 0' failed.
> 
> I wonder if the offset is beyond the end of the iovecs.
> 
> Thanks,
> Stefan
> 

Thanks
Farhan

^ permalink raw reply

* Re: [PATCH RESEND] kthread, tracing: Don't expose half-written comm when creating kthreads
From: Steven Rostedt @ 2018-07-23 16:41 UTC (permalink / raw)
  To: Snild Dolkow
  Cc: linux-kernel, Ingo Molnar, Jens Axboe, Tejun Heo,
	Greg Kroah-Hartman, Linus Torvalds, Peter Enderborg,
	Yoshitaka Seto, Oleksiy Avramchenko, KOSAKI Motohiro, John Stultz
In-Reply-To: <7749ed7d-50d5-4b95-fbd2-b056a1fff957@sony.com>

On Mon, 23 Jul 2018 17:49:36 +0200
Snild Dolkow <snild@sony.com> wrote:

> On 07/23/2018 05:37 PM, Steven Rostedt wrote:

> Will add:
> 
> 		/*
> 		 * task is already visible to other tasks, so updating
> 		 * COMM must be protected.
> 		 */

Thanks.

> 
> Any issues with the commit message? Reading it back again now, it doesn't
> seem quite as clear as when I wrote it.

Yeah, I think it does need some updates:

> There was a window for racing when task->comm was being written. The

It would be nice to explain this race window in more detail.

> vsnprintf function writes 16 bytes, then counts the rest, then null
> terminates. In the meantime, other threads could see the non-terminated
> comm value. In our case, it got into the trace system's saved cmdlines
> and could cause stack corruption when strcpy'd out of there.

Perhaps add in the change log something about the fact that the
vsprintf() is performed on the COMM when the task is visible to other
tasks, and that could cause problems if other tasks read the COMM (like
in tracing) without updating it properly with set_task_comm().

-- Steve


> 
> The workaround in e09e28671 (use strlcpy in __trace_find_cmdline) was
> likely needed because of this bug.
> 
> Solved by vsnprintf:ing to a local buffer, then using set_task_comm().

^ permalink raw reply

* automation: Creating a patchwork instance to improve pre-commit build testing
From: Lars Kurth @ 2018-07-23 16:40 UTC (permalink / raw)
  To: xen-devel
  Cc: Iurii Artemenko, Wei Liu, Doug Goldstein,
	minios-devel@lists.xenproject.org, Committers, Matt Spencer

Hi all,

Wei and Doug have recently been improving [xen.git] / automation / build, implementing some of the functionality outlined in [1] (see "CI / CD" heading). 

We are intending to take every patch series that is sent to xen-devel and create a branch at https://gitlab.com/xen-project/xen.git scoped to something like "ml/<msg-id>". ml stands for mailing list. The goal is to run whatever basic CI tests we can to help reviewers and contributors. Wei and Doug have gotten things to the point where we can compile against a number of distros and they continue to expand that functionality. 

What is missing, is the capability to take patch series from the mailing list and use the posted patches to trigger the CI/CD functionality. We are planning to use Patchwork because it provides functionality to allow us to do this via the Patchwork API (https://patchwork.readthedocs.io/en/latest/api/rest/) Originally Doug suggested he could just try this via one of his own VMs, but the recent introduction of the GDPR creates risks and obligations for everyone (both individuals and companies) who processes Xen Project related mailing list data (including git meta-data). To avoid this, we were planning to run patchwork on a new Xen Project hosted VM, for the sole purpose of using the Patchwork API. 

At this stage, we are not intending to expose the patchwork UI.

# How does this impact me?
The contribution workflow is *not* impacted by this change, but once up and running the following will happen once you post a patch or patch series to xen-devel:
* Patchwork will take patch series from the mailing list and applies it
* CI/DC testing is triggered
* A test report will be sent as a mail to the patch or the series (aka the 00 patch of the series)

This does mean though that series which do not build or show other issues, will likely not be reviewed until the tests pass. This would lessen the burden on reviewers, as they will know whether the code submitted builds on a wide array of environments. 

# Supported build environments (for now)
For now, we are only supporting this on x86. The exact list of distros we build against can be found in [xen.git] / automation / build 
Arm support will be added later, once the missing pieces are in place (see [1] section "What OSs (architectures) should we support?") 

# What about checkpatch.pl?
Once clang-format changes have been completed, Wei and Doug are going to look at automating code style checks as well. Iurii Artemenko is working on clang-format improvements that will allow us to work around the issues outlined in [2]. There are still 3 missing pieces (see [3]). 

I don't think this is a controversial proposal, but I wanted to ask whether there are any objections or suggestions. I included the minios-devel list, as the Unikraft project is planning to introduce patchwork into their workflow.

Best Regards
Lars


# References
[1] https://lists.xenproject.org/archives/html/xen-devel/2018-07/msg00126.html 

[2] https://xen.markmail.org/thread/pc2dhjgbgrdcz5sb
[3] Coding style patterns used in Xen that we cannot yet support

Braces should be omitted for blocks with a single statement. e.g.,
 if ( condition )
     single_statement();
 
Comments
--------
 
 Only C style /* ... */ comments are to be used.  C++ style // comments
 should not be used.  Multi-word comments should begin with a capital
 letter.  Comments containing a single sentence may end with a full
 stop; comments containing several sentences must have a full stop
 after each sentence.
 
 Multi-line comment blocks should start and end with comment markers on
 separate lines and each line should begin with a leading '*'.
 
 /*
  * Example, multi-line comment block.
  *
  * Note beginning and end markers on separate lines and leading '*'.
  */
 
Emacs local variables
---------------------
A comment block containing local variables for emacs is permitted at
the end of files.  It should be:
 
 /*
  * Local variables:
  * mode: C
  * c-file-style: "BSD"
  * c-basic-offset: 4
  * indent-tabs-mode: nil
  * End:
  */


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

^ permalink raw reply

* Re: [RFC PATCH ghak59 V1 3/6] audit: exclude user records from syscall context
From: Richard Guy Briggs @ 2018-07-23 16:40 UTC (permalink / raw)
  To: Paul Moore; +Cc: Eric Paris, linux-audit, linux-kernel
In-Reply-To: <20180712214608.ed4chli7mdc7ymin@madcap2.tricolour.ca>

On 2018-07-12 17:46, Richard Guy Briggs wrote:
> On 2018-06-28 18:11, Paul Moore wrote:
> > On Thu, Jun 14, 2018 at 4:23 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> > > Since the function audit_log_common_recv_msg() is shared by a number of
> > > AUDIT_CONFIG_CHANGE and the entire range of AUDIT_USER_* record types,
> > > and since the AUDIT_CONFIG_CHANGE message type has been converted to a
> > > syscall accompanied record type, special-case the AUDIT_USER_* range of
> > > messages so they remain standalone records.
> > >
> > > See: https://github.com/linux-audit/audit-kernel/issues/59
> > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > > ---
> > >  kernel/audit.c | 12 +++++++++---
> > >  1 file changed, 9 insertions(+), 3 deletions(-)
> > 
> > I think this is fine, but see my previous comment about combining 2/6
> > and 3/6 as a safety measure.
> 
> This one I left as a seperate patch for discussion.  We'd previously
> talked about connecting all possible records with syscall records if
> they exist, but this one I'm unsure about, since we don't really care
> what userspace process is issuing this message.  It is just the message
> content itself that is important.  Or is it?  Are we concerned about
> CAP_AUDIT_WRITE holders/abusers and want as much info about them as we
> can get in case they go rogue or pear-shaped?

I'm waiting on re-spinning this patchset because of this open question.

Is connecting AUDIT_USER* records desirable or a liability?

> > > diff --git a/kernel/audit.c b/kernel/audit.c
> > > index e469234..c8c2efc 100644
> > > --- a/kernel/audit.c
> > > +++ b/kernel/audit.c
> > > @@ -1057,7 +1057,8 @@ static int audit_netlink_ok(struct sk_buff *skb, u16 msg_type)
> > >         return err;
> > >  }
> > >
> > > -static void audit_log_common_recv_msg(struct audit_buffer **ab, u16 msg_type)
> > > +static void __audit_log_common_recv_msg(struct audit_context *context,
> > > +                                       struct audit_buffer **ab, u16 msg_type)
> > >  {
> > >         uid_t uid = from_kuid(&init_user_ns, current_uid());
> > >         pid_t pid = task_tgid_nr(current);
> > > @@ -1067,7 +1068,7 @@ static void audit_log_common_recv_msg(struct audit_buffer **ab, u16 msg_type)
> > >                 return;
> > >         }
> > >
> > > -       *ab = audit_log_start(audit_context(), GFP_KERNEL, msg_type);
> > > +       *ab = audit_log_start(context, GFP_KERNEL, msg_type);
> > >         if (unlikely(!*ab))
> > >                 return;
> > >         audit_log_format(*ab, "pid=%d uid=%u", pid, uid);
> > > @@ -1075,6 +1076,11 @@ static void audit_log_common_recv_msg(struct audit_buffer **ab, u16 msg_type)
> > >         audit_log_task_context(*ab);
> > >  }
> > >
> > > +static inline void audit_log_common_recv_msg(struct audit_buffer **ab, u16 msg_type)
> > > +{
> > > +       __audit_log_common_recv_msg(audit_context(), ab, msg_type);
> > > +}
> > > +
> > >  int is_audit_feature_set(int i)
> > >  {
> > >         return af.features & AUDIT_FEATURE_TO_MASK(i);
> > > @@ -1341,7 +1347,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
> > >                                 if (err)
> > >                                         break;
> > >                         }
> > > -                       audit_log_common_recv_msg(&ab, msg_type);
> > > +                       __audit_log_common_recv_msg(NULL, &ab, msg_type);
> > >                         if (msg_type != AUDIT_USER_TTY)
> > >                                 audit_log_format(ab, " msg='%.*s'",
> > >                                                  AUDIT_MESSAGE_TEXT_MAX,
> > > --
> > > 1.8.3.1
> > >
> > 
> > 
> > -- 
> > paul moore
> > www.paul-moore.com
> > 
> > --
> > Linux-audit mailing list
> > Linux-audit@redhat.com
> > https://www.redhat.com/mailman/listinfo/linux-audit
> 
> - RGB
> 
> --
> Richard Guy Briggs <rgb@redhat.com>
> Sr. S/W Engineer, Kernel Security, Base Operating Systems
> Remote, Ottawa, Red Hat Canada
> IRC: rgb, SunRaycer
> Voice: +1.647.777.2635, Internal: (81) 32635
> 
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635

^ permalink raw reply

* Re: [PATCH v2 07/12] sched/core: uclamp: enforce last task UCLAMP_MAX
From: Suren Baghdasaryan @ 2018-07-23 16:40 UTC (permalink / raw)
  To: Patrick Bellasi
  Cc: linux-kernel, linux-pm, Ingo Molnar, Peter Zijlstra, Tejun Heo,
	Rafael J . Wysocki, Viresh Kumar, Vincent Guittot, Paul Turner,
	Dietmar Eggemann, Morten Rasmussen, Juri Lelli, Todd Kjos,
	Joel Fernandes, Steve Muckle
In-Reply-To: <20180723150249.GD2683@e110439-lin>

On Mon, Jul 23, 2018 at 8:02 AM, Patrick Bellasi
<patrick.bellasi@arm.com> wrote:
> On 20-Jul 18:23, Suren Baghdasaryan wrote:
>> Hi Patrick,
>
> Hi Sure,
> thank!
>
>> On Mon, Jul 16, 2018 at 1:29 AM, Patrick Bellasi
>> <patrick.bellasi@arm.com> wrote:
>
> [...]
>
>> > @@ -977,13 +991,21 @@ static inline void uclamp_cpu_get_id(struct task_struct *p,
>> >         uc_grp = &rq->uclamp.group[clamp_id][0];
>> >         uc_grp[group_id].tasks += 1;
>> >
>> > +       /* Force clamp update on idle exit */
>> > +       uc_cpu = &rq->uclamp;
>> > +       clamp_value = p->uclamp[clamp_id].value;
>> > +       if (unlikely(uc_cpu->flags & UCLAMP_FLAG_IDLE)) {
>>
>> The condition below is not needed because UCLAMP_FLAG_IDLE is set only
>> for UCLAMP_MAX clamp_id, therefore the above condition already covers
>> the one below.
>
> Not really, this function is called two times, the first time to
> update UCLAMP_MIN and a second time to update UCLAMP_MAX.
>
> For both clamp_id we want to force update uc_cpu->value[clamp_id],
> thus the UCLAMP_FLAG_IDLE flag has to be cleared only the second time.
>
> Maybe I can had the following comment to better explain the reason of
> the check:
>
>                 /*
>                  * This function is called for both UCLAMP_MIN (before) and
>                  * UCLAMP_MAX (after). Let's reset the flag only the when
>                  * we know that UCLAMP_MIN has been already updated.
>                  */
>

Ah, my bad. I missed the fact that uc_cpu->flags is shared for both
UCLAMP_MIN and UCLAMP_MAX. It's fine the way it originally was. Thanks
for explanation!

>> > +               if (clamp_id == UCLAMP_MAX)
>> > +                       uc_cpu->flags &= ~UCLAMP_FLAG_IDLE;
>> > +               uc_cpu->value[clamp_id] = clamp_value;
>> > +               return;
>> > +       }
>
> [...]
>
> --
> #include <best/regards.h>
>
> Patrick Bellasi

^ permalink raw reply

* Re: [PATCH net-next] ipv6: sr: Use kmemdup instead of duplicating it in parse_nla_srh
From: David Miller @ 2018-07-23 16:39 UTC (permalink / raw)
  To: yuehaibing; +Cc: kuznet, yoshfuji, linux-kernel, netdev
In-Reply-To: <20180723083319.19144-1-yuehaibing@huawei.com>

From: YueHaibing <yuehaibing@huawei.com>
Date: Mon, 23 Jul 2018 16:33:19 +0800

> Replace calls to kmalloc followed by a memcpy with a direct call to
> kmemdup.
> 
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>

Applied, thank you.

^ permalink raw reply

* Re: [PATCH 0/4][RFC v2] Introduce the in-kernel hibernation encryption
From: Yu Chen @ 2018-07-23 16:38 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Oliver Neukum, Rafael J . Wysocki, Eric Biggers, Lee, Chun-Yi,
	Theodore Ts o, Stephan Mueller, Denis Kenzior, linux-pm,
	linux-crypto, linux-kernel, Gu, Kookoo, Zhang, Rui
In-Reply-To: <20180723122227.GA30092@amd>

Hello,
On Mon, Jul 23, 2018 at 02:22:27PM +0200, Pavel Machek wrote:
> Hi!
> 
> > > > 2. Ideally kernel memory should be encrypted by the
> > > >    kernel itself. We have uswsusp to support user
> > > >    space hibernation, however doing the encryption
> > > >    in kernel space has more advantages:
> > > >    2.1 Not having to transfer plain text kernel memory to
> > > >        user space. Per Lee, Chun-Yi, uswsusp is disabled
> > > >        when the kernel is locked down:
> > > >        https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/
> > > >        linux-fs.git/commit/?h=lockdown-20180410&
> > > >        id=8732c1663d7c0305ae01ba5a1ee4d2299b7b4612
> > > >        due to:
> > > >        "There have some functions be locked-down because
> > > >        there have no appropriate mechanisms to check the
> > > >        integrity of writing data."
> > > >        https://patchwork.kernel.org/patch/10476751/
> > > 
> > > So your goal is to make hibernation compatible with kernel
> > > lockdown? Do your patches provide sufficient security that hibernation
> > > can be enabled with kernel lockdown?
> > 
> > OK, maybe I am dense, but if the key comes from user space, will that
> > be enough?
> 
> Yes, that seems to be one of problems of Yu Chen's patchset.
> 
It is a trade off to derive the key in user space, we once
tried to derive the key in user space, and people suggested
a better way is to do it in user space. And there is a similar
user case of kernel using key from user space is derived from ecryptfs
for ext4.
> > > > Joey Lee and I had a discussion on his previous work at
> > > > https://patchwork.kernel.org/patch/10476751
> > > > We collaborate on this task and his snapshot signature
> > > > feature can be based on this patch set.
> > > 
> > > Well, his work can also work without your patchset, right?
> > 
> > Yes. But you are objecting to encryption in kernel space at all,
> > aren't you?
> 
> I don't particulary love the idea of doing hibernation encryption in
> the kernel, correct.
> 
> But we have this weird thing called secure boot, some people seem to
> want. So we may need some crypto in the kernel -- but I'd like
> something that works with uswsusp, too. Plus, it is mandatory that
> patch explains what security guarantees they want to provide against
> what kinds of attacks...
> 
> Lee, Chun-Yi's patch seemed more promising.				Pavel
> 
The only difference between Chun-Yi's hibernation encrytion solution
and our solution is that his strategy encrypts the snapshot from sratch, 
and ours encryts each page before them going to block device. The benefit
of his solution is that the snapshot can be encrypt in kernel first
thus the uswsusp is allowed  to read it to user space even kernel
is lock down. And I had a discussion with Chun-Yi that we can use
his snapshot solution to make uswsusp happy, and we share the crypto
help code and he can also use our user provided key for his signature.
>From this point of view, our code are actually the same, except that
we can help clean up the code and also enhance some encrytion process
for his solution. I don't know why you don't like encryption in kernel, 
because from my point of view, without encryption hibernation in kernel,
uswsusp could not be enabled if kernel is lock down : -) Or do I miss something?
Best,
Yu
> -- 
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply

* [PATCHv7 6/9] fpga: add intel stratix10 soc fpga manager driver
From: Moritz Fischer @ 2018-07-23 16:38 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1531930724-22061-7-git-send-email-richard.gong@linux.intel.com>

Hi Alan, Richard,

On Wed, Jul 18, 2018 at 11:18:41AM -0500, richard.gong at linux.intel.com wrote:
> From: Alan Tull <atull@kernel.org>
> 
> Add driver for reconfiguring Intel Stratix10 SoC FPGA devices.
> This driver communicates through the Intel Service Driver which
> does communication with privileged hardware (that does the
> FPGA programming) through a secure mailbox.
> 
> Signed-off-by: Alan Tull <atull@kernel.org>
> Signed-off-by: Richard Gong <richard.gong@intel.com>
Acked-by: Moritz Fischer <mdf@kernel.org>
> ---
> v2: this patch is added in patch set version 2
> v3: change to align to the update of service client APIs, and the
>     update of fpga_mgr device node
> v4: changes to align with stratix10-svc-client API updates
>     add Richard's signed-off-by
> v5: update to align changes at service layer to minimize service
>     layer thread usages
> v6: add S10_RECONFIG_TIMEOUT
>     rename s/S10_BUF_TIMEOUT/S10_BUFFER_TIMEOUT/
>     fix klocwork errors
> v7: s/pr_debug/dev_dbg, s/dev_info/dev_dbg
>     add unlock buffer if s10_svc_send_msg() fails
>     add release channel if fpga_mgr_create() fails
> ---
>  drivers/fpga/Kconfig         |   6 +
>  drivers/fpga/Makefile        |   1 +
>  drivers/fpga/stratix10-soc.c | 558 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 565 insertions(+)
>  create mode 100644 drivers/fpga/stratix10-soc.c
> 
> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> index ee9c542..7d20743 100644
> --- a/drivers/fpga/Kconfig
> +++ b/drivers/fpga/Kconfig
> @@ -56,6 +56,12 @@ config FPGA_MGR_ZYNQ_FPGA
>  	help
>  	  FPGA manager driver support for Xilinx Zynq FPGAs.
>  
> +config FPGA_MGR_STRATIX10_SOC
> +	tristate "Intel Stratix10 SoC FPGA Manager"
> +	depends on (ARCH_STRATIX10 && STRATIX10_SERVICE)
> +	help
> +	  FPGA manager driver support for the Intel Stratix10 SoC.
> +
>  config FPGA_MGR_XILINX_SPI
>  	tristate "Xilinx Configuration over Slave Serial (SPI)"
>  	depends on SPI
> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> index f9803da..9f17b7f 100644
> --- a/drivers/fpga/Makefile
> +++ b/drivers/fpga/Makefile
> @@ -13,6 +13,7 @@ obj-$(CONFIG_FPGA_MGR_ICE40_SPI)	+= ice40-spi.o
>  obj-$(CONFIG_FPGA_MGR_MACHXO2_SPI)	+= machxo2-spi.o
>  obj-$(CONFIG_FPGA_MGR_SOCFPGA)		+= socfpga.o
>  obj-$(CONFIG_FPGA_MGR_SOCFPGA_A10)	+= socfpga-a10.o
> +obj-$(CONFIG_FPGA_MGR_STRATIX10_SOC)	+= stratix10-soc.o
>  obj-$(CONFIG_FPGA_MGR_TS73XX)		+= ts73xx-fpga.o
>  obj-$(CONFIG_FPGA_MGR_XILINX_SPI)	+= xilinx-spi.o
>  obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA)	+= zynq-fpga.o
> diff --git a/drivers/fpga/stratix10-soc.c b/drivers/fpga/stratix10-soc.c
> new file mode 100644
> index 0000000..f34830d
> --- /dev/null
> +++ b/drivers/fpga/stratix10-soc.c
> @@ -0,0 +1,558 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * FPGA Manager Driver for Intel Stratix10 SoC
> + *
> + *  Copyright (C) 2018 Intel Corporation
> + */
> +#include <linux/completion.h>
> +#include <linux/fpga/fpga-mgr.h>
> +#include <linux/stratix10-svc-client.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +
> +/*
> + * FPGA programming requires a higher level of privilege (EL3), per the SoC
> + * design.
> + */
> +#define NUM_SVC_BUFS	4
> +#define SVC_BUF_SIZE	SZ_512K
> +
> +/* Indicates buffer is in use if set */
> +#define SVC_BUF_LOCK	0
> +
> +#define S10_BUFFER_TIMEOUT (msecs_to_jiffies(SVC_RECONFIG_BUFFER_TIMEOUT_MS))
> +#define S10_RECONFIG_TIMEOUT (msecs_to_jiffies(SVC_RECONFIG_REQUEST_TIMEOUT_MS))
> +
> +/**
> + * struct s10_svc_buf
> + * @buf: virtual address of buf provided by service layer
> + * @lock: locked if buffer is in use
> + */
> +struct s10_svc_buf {
> +	char *buf;
> +	unsigned long lock;
> +};
> +
> +struct s10_priv {
> +	struct stratix10_svc_chan *chan;
> +	struct stratix10_svc_client client;
> +	struct completion status_return_completion;
> +	struct s10_svc_buf svc_bufs[NUM_SVC_BUFS];
> +	unsigned long status;
> +};
> +
> +static int s10_svc_send_msg(struct s10_priv *priv,
> +			    enum stratix10_svc_command_code command,
> +			    void *payload, u32 payload_length)
> +{
> +	struct stratix10_svc_chan *chan = priv->chan;
> +	struct device *dev = priv->client.dev;
> +	struct stratix10_svc_client_msg msg;
> +	int ret;
> +
> +	dev_dbg(dev, "%s cmd=%d payload=%p length=%d\n",
> +		__func__, command, payload, payload_length);
> +
> +	msg.command = command;
> +	msg.payload = payload;
> +	msg.payload_length = payload_length;
> +
> +	ret = stratix10_svc_send(chan, &msg);
> +	dev_dbg(dev, "stratix10_svc_send returned status %d\n", ret);
> +
> +	return ret;
> +}
> +
> +/**
> + * s10_free_buffers
> + * Free buffers allocated from the service layer's pool that are not in use.
> + * @mgr: fpga manager struct
> + * Free all buffers that are not in use.
> + * Return true when all buffers are freed.
> + */
> +static bool s10_free_buffers(struct fpga_manager *mgr)
> +{
> +	struct s10_priv *priv = mgr->priv;
> +	uint num_free = 0;
> +	uint i;
> +
> +	for (i = 0; i < NUM_SVC_BUFS; i++) {
> +		if (!priv->svc_bufs[i].buf) {
> +			num_free++;
> +			continue;
> +		}
> +
> +		if (!test_and_set_bit_lock(SVC_BUF_LOCK,
> +					   &priv->svc_bufs[i].lock)) {
> +			stratix10_svc_free_memory(priv->chan,
> +					      priv->svc_bufs[i].buf);
> +			priv->svc_bufs[i].buf = NULL;
> +			num_free++;
> +		}
> +	}
> +
> +	return num_free == NUM_SVC_BUFS;
> +}
> +
> +/**
> + * s10_free_buffer_count
> + * Count how many buffers are not in use.
> + * @mgr: fpga manager struct
> + * Return # of buffers that are not in use.
> + */
> +static uint s10_free_buffer_count(struct fpga_manager *mgr)
> +{
> +	struct s10_priv *priv = mgr->priv;
> +	uint num_free = 0;
> +	uint i;
> +
> +	for (i = 0; i < NUM_SVC_BUFS; i++)
> +		if (!priv->svc_bufs[i].buf)
> +			num_free++;
> +
> +	return num_free;
> +}
> +
> +/**
> + * s10_unlock_bufs
> + * Given the returned buffer address, match that address to our buffer struct
> + * and unlock that buffer.  This marks it as available to be refilled and sent
> + * (or freed).
> + * @priv: private data
> + * @kaddr: kernel address of buffer that was returned from service layer
> + */
> +static void s10_unlock_bufs(struct s10_priv *priv, void *kaddr)
> +{
> +	uint i;
> +
> +	if (!kaddr)
> +		return;
> +
> +	for (i = 0; i < NUM_SVC_BUFS; i++)
> +		if (priv->svc_bufs[i].buf == kaddr) {
> +			clear_bit_unlock(SVC_BUF_LOCK,
> +					 &priv->svc_bufs[i].lock);
> +			return;
> +		}
> +
> +	WARN(1, "Unknown buffer returned from service layer %p\n", kaddr);
> +}
> +
> +/**
> + * s10_receive_callback
> + * Callback for service layer to use to provide client (this driver) messages
> + * received through the mailbox.
> + * @client: service layer client struct
> + * @data: message
> + */
> +static void s10_receive_callback(struct stratix10_svc_client *client,
> +				 struct stratix10_svc_cb_data *data)
> +{
> +	struct s10_priv *priv = client->priv;
> +	u32 status;
> +	int i;
> +
> +	WARN_ONCE(!data, "%s: stratix10_svc_rc_data = NULL", __func__);
> +
> +	status = data->status;
> +
> +	/*
> +	 * Here we set status bits as we receive them.  Elsewhere, we always use
> +	 * test_and_clear_bit() to check status in priv->status
> +	 */
> +	for (i = 0; i <= SVC_STATUS_RECONFIG_ERROR; i++)
> +		if (status & (1 << i))
> +			set_bit(i, &priv->status);
> +
> +	if (status & BIT(SVC_STATUS_RECONFIG_BUFFER_DONE)) {
> +		s10_unlock_bufs(priv, data->kaddr1);
> +		s10_unlock_bufs(priv, data->kaddr2);
> +		s10_unlock_bufs(priv, data->kaddr3);
> +	}
> +
> +	complete(&priv->status_return_completion);
> +}
> +
> +/**
> + * s10_ops_write_init
> + * Prepare for FPGA reconfiguration by requesting partial reconfig and
> + * allocating buffers from the service layer.
> + * @mgr: fpga manager
> + * @info: fpga image info
> + * @buf: fpga image buffer
> + * @count: size of buf in bytes
> + */
> +static int s10_ops_write_init(struct fpga_manager *mgr,
> +			      struct fpga_image_info *info,
> +			      const char *buf, size_t count)
> +{
> +	struct s10_priv *priv = mgr->priv;
> +	struct device *dev = priv->client.dev;
> +	struct stratix10_svc_command_reconfig_payload payload;
> +	char *kbuf;
> +	uint i;
> +	int ret;
> +
> +	payload.flags = 0;
> +	if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
> +		dev_dbg(dev, "Requesting partial reconfiguration.\n");
> +		payload.flags |= BIT(COMMAND_RECONFIG_FLAG_PARTIAL);
> +	} else {
> +		dev_dbg(dev, "Requesting full reconfiguration.\n");
> +	}
> +
> +	reinit_completion(&priv->status_return_completion);
> +	ret = s10_svc_send_msg(priv, COMMAND_RECONFIG,
> +			       &payload, sizeof(payload));
> +	if (ret < 0)
> +		goto init_done;
> +
> +	ret = wait_for_completion_interruptible_timeout(
> +		&priv->status_return_completion, S10_RECONFIG_TIMEOUT);
> +	if (!ret) {
> +		dev_err(dev, "timeout waiting for RECONFIG_REQUEST\n");
> +		ret = -ETIMEDOUT;
> +		goto init_done;
> +	}
> +	if (ret < 0) {
> +		dev_err(dev, "error (%d) waiting for RECONFIG_REQUEST\n", ret);
> +		goto init_done;
> +	}
> +
> +	ret = 0;
> +	if (!test_and_clear_bit(SVC_STATUS_RECONFIG_REQUEST_OK,
> +				&priv->status)) {
> +		ret = -ETIMEDOUT;
> +		goto init_done;
> +	}
> +
> +	/* Allocate buffers from the service layer's pool. */
> +	for (i = 0; i < NUM_SVC_BUFS; i++) {
> +		kbuf = stratix10_svc_allocate_memory(priv->chan, SVC_BUF_SIZE);
> +		if (!kbuf) {
> +			s10_free_buffers(mgr);
> +			ret = -ENOMEM;
> +			goto init_done;
> +		}
> +
> +		priv->svc_bufs[i].buf = kbuf;
> +		priv->svc_bufs[i].lock = 0;
> +	}
> +
> +init_done:
> +	stratix10_svc_done(priv->chan);
> +	return ret;
> +}
> +
> +/**
> + * s10_send_buf
> + * Send a buffer to the service layer queue
> + * @mgr: fpga manager struct
> + * @buf: fpga image buffer
> + * @count: size of buf in bytes
> + * Returns # of bytes transferred or -ENOBUFS if the all the buffers are in use
> + * or if the service queue is full. Never returns 0.
> + */
> +static int s10_send_buf(struct fpga_manager *mgr, const char *buf, size_t count)
> +{
> +	struct s10_priv *priv = mgr->priv;
> +	struct device *dev = priv->client.dev;
> +	void *svc_buf;
> +	size_t xfer_sz;
> +	int ret;
> +	uint i;
> +
> +	/* get/lock a buffer that that's not being used */
> +	for (i = 0; i < NUM_SVC_BUFS; i++)
> +		if (!test_and_set_bit_lock(SVC_BUF_LOCK,
> +					   &priv->svc_bufs[i].lock))
> +			break;
> +
> +	if (i == NUM_SVC_BUFS)
> +		return -ENOBUFS;
> +
> +	xfer_sz = count < SVC_BUF_SIZE ? count : SVC_BUF_SIZE;
> +
> +	svc_buf = priv->svc_bufs[i].buf;
> +	memcpy(svc_buf, buf, xfer_sz);
> +	ret = s10_svc_send_msg(priv, COMMAND_RECONFIG_DATA_SUBMIT,
> +			       svc_buf, xfer_sz);
> +	if (ret < 0) {
> +		dev_err(dev,
> +			"Error while sending data to service layer (%d)", ret);
> +		clear_bit_unlock(SVC_BUF_LOCK, &priv->svc_bufs[i].lock);
> +		return ret;
> +	}
> +
> +	return xfer_sz;
> +}
> +
> +/**
> + * s10_ops_write
> + * Send a FPGA image to privileged layers to write to the FPGA.  When done
> + * sending, free all service layer buffers we allocated in write_init.
> + * @mgr: fpga manager
> + * @buf: fpga image buffer
> + * @count: size of buf in bytes
> + * Returns 0 for success or negative errno.
> + */
> +static int s10_ops_write(struct fpga_manager *mgr, const char *buf,
> +			 size_t count)
> +{
> +	struct s10_priv *priv = mgr->priv;
> +	struct device *dev = priv->client.dev;
> +	long wait_status;
> +	int sent = 0;
> +	int ret = 0;
> +
> +	/*
> +	 * Loop waiting for buffers to be returned.  When a buffer is returned,
> +	 * reuse it to send more data or free if if all data has been sent.
> +	 */
> +	while (count > 0 || s10_free_buffer_count(mgr) != NUM_SVC_BUFS) {
> +		reinit_completion(&priv->status_return_completion);
> +
> +		if (count > 0) {
> +			sent = s10_send_buf(mgr, buf, count);
> +			if (sent < 0)
> +				continue;
> +
> +			count -= sent;
> +			buf += sent;
> +		} else {
> +			if (s10_free_buffers(mgr))
> +				return 0;
> +
> +			ret = s10_svc_send_msg(
> +				priv, COMMAND_RECONFIG_DATA_CLAIM,
> +				NULL, 0);
> +			if (ret < 0)
> +				break;
> +		}
> +
> +		/*
> +		 * If callback hasn't already happened, wait for buffers to be
> +		 * returned from service layer
> +		 */
> +		wait_status = 1; /* not timed out */
> +		if (!priv->status)
> +			wait_status = wait_for_completion_interruptible_timeout(
> +				&priv->status_return_completion,
> +				S10_BUFFER_TIMEOUT);
> +
> +		if (test_and_clear_bit(SVC_STATUS_RECONFIG_BUFFER_DONE,
> +				       &priv->status) ||
> +		    test_and_clear_bit(SVC_STATUS_RECONFIG_BUFFER_SUBMITTED,
> +				       &priv->status)) {
> +			ret = 0;
> +			continue;
> +		}
> +
> +		if (test_and_clear_bit(SVC_STATUS_RECONFIG_ERROR,
> +				       &priv->status)) {
> +			dev_err(dev, "ERROR - giving up - SVC_STATUS_RECONFIG_ERROR\n");
> +			ret = -EFAULT;
> +			break;
> +		}
> +
> +		if (!wait_status) {
> +			dev_err(dev, "timeout waiting for svc layer buffers\n");
> +			ret = -ETIMEDOUT;
> +			break;
> +		}
> +		if (wait_status < 0) {
> +			ret = wait_status;
> +			dev_err(dev,
> +				"error (%d) waiting for svc layer buffers\n",
> +				ret);
> +			break;
> +		}
> +	}
> +
> +	if (!s10_free_buffers(mgr))
> +		dev_err(dev, "%s not all buffers were freed\n", __func__);
> +
> +	return ret;
> +}
> +
> +/**
> + * s10_ops_write_complete
> + * Wait for FPGA configuration to be done
> + * @mgr: fpga manager
> + * @info: fpga image info
> + * Returns 0 for success negative errno.
> + */
> +static int s10_ops_write_complete(struct fpga_manager *mgr,
> +				  struct fpga_image_info *info)
> +{
> +	struct s10_priv *priv = mgr->priv;
> +	struct device *dev = priv->client.dev;
> +	unsigned long timeout;
> +	int ret;
> +
> +	timeout = usecs_to_jiffies(info->config_complete_timeout_us);
> +
> +	do {
> +		reinit_completion(&priv->status_return_completion);
> +
> +		ret = s10_svc_send_msg(priv, COMMAND_RECONFIG_STATUS, NULL, 0);
> +		if (ret < 0)
> +			break;
> +
> +		ret = wait_for_completion_interruptible_timeout(
> +			&priv->status_return_completion, timeout);
> +		if (!ret) {
> +			dev_err(dev,
> +				"timeout waiting for RECONFIG_COMPLETED\n");
> +			ret = -ETIMEDOUT;
> +			break;
> +		}
> +		if (ret < 0) {
> +			dev_err(dev,
> +				"error (%d) waiting for RECONFIG_COMPLETED\n",
> +				ret);
> +			break;
> +		}
> +		/* Not error or timeout, so ret is # of jiffies until timeout */
> +		timeout = ret;
> +		ret = 0;
> +
> +		if (test_and_clear_bit(SVC_STATUS_RECONFIG_COMPLETED,
> +				       &priv->status))
> +			break;
> +
> +		if (test_and_clear_bit(SVC_STATUS_RECONFIG_ERROR,
> +				       &priv->status)) {
> +			dev_err(dev, "ERROR - giving up - SVC_STATUS_RECONFIG_ERROR\n");
> +			ret = -EFAULT;
> +			break;
> +		}
> +	} while (1);
> +
> +	stratix10_svc_done(priv->chan);
> +
> +	return ret;
> +}
> +
> +static enum fpga_mgr_states s10_ops_state(struct fpga_manager *mgr)
> +{
> +	return FPGA_MGR_STATE_UNKNOWN;
> +}
> +
> +static const struct fpga_manager_ops s10_ops = {
> +	.state = s10_ops_state,
> +	.write_init = s10_ops_write_init,
> +	.write = s10_ops_write,
> +	.write_complete = s10_ops_write_complete,
> +};
> +
> +static int s10_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct s10_priv *priv;
> +	struct fpga_manager *mgr;
> +	int ret;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->client.dev = dev;
> +	priv->client.receive_cb = s10_receive_callback;
> +	priv->client.priv = priv;
> +
> +	priv->chan = stratix10_svc_request_channel_byname(&priv->client,
> +						SVC_CLIENT_FPGA);
> +	if (IS_ERR(priv->chan)) {
> +		dev_err(dev, "couldn't get service channel (%s)\n",
> +			SVC_CLIENT_FPGA);
> +		return PTR_ERR(priv->chan);
> +	}
> +
> +	init_completion(&priv->status_return_completion);
> +
> +	mgr = fpga_mgr_create(dev, "Stratix10 SOC FPGA Manager",
> +			      &s10_ops, priv);
> +	if (!mgr) {
> +		dev_err(dev, "unable to create FPGA manager\n");
> +		stratix10_svc_free_channel(priv->chan);
> +		return -ENOMEM;
Would this be shorter with a goto and common error handling at
the end of the function?
> +	}
> +
> +	ret = fpga_mgr_register(mgr);
> +	if (ret) {
> +		dev_err(dev, "unable to register FPGA manager\n");
> +		stratix10_svc_free_channel(priv->chan);
> +		fpga_mgr_free(mgr);
> +		return ret;
> +	}
> +
> +	platform_set_drvdata(pdev, mgr);
> +
> +	return ret;
> +}
> +
> +static int s10_remove(struct platform_device *pdev)
> +{
> +	struct fpga_manager *mgr = platform_get_drvdata(pdev);
> +	struct s10_priv *priv = mgr->priv;
> +
> +	fpga_mgr_unregister(mgr);
> +	stratix10_svc_free_channel(priv->chan);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id s10_of_match[] = {
> +	{ .compatible = "intel,stratix10-soc-fpga-mgr", },
> +	{},
> +};
> +
> +MODULE_DEVICE_TABLE(of, s10_of_match);
> +
> +static struct platform_driver s10_driver = {
> +	.probe = s10_probe,
> +	.remove = s10_remove,
> +	.driver = {
> +		.name	= "Stratix10 SoC FPGA manager",
> +		.of_match_table = of_match_ptr(s10_of_match),
> +	},
> +};
> +
> +static int __init s10_init(void)
> +{
> +	struct device_node *fw_np;
> +	struct device_node *np;
> +	int ret;
> +
> +	fw_np = of_find_node_by_name(NULL, "svc");
> +	if (!fw_np)
> +		return -ENODEV;
> +
> +	np = of_find_matching_node(fw_np, s10_of_match);
> +	if (!np) {
> +		of_node_put(fw_np);
> +		return -ENODEV;
> +	}
> +
> +	of_node_put(np);
> +	ret = of_platform_populate(fw_np, s10_of_match, NULL, NULL);
> +	of_node_put(fw_np);
> +	if (ret)
> +		return ret;
> +
> +	return platform_driver_register(&s10_driver);
> +}
> +
> +static void __exit s10_exit(void)
> +{
> +	return platform_driver_unregister(&s10_driver);
> +}
> +
> +module_init(s10_init);
> +module_exit(s10_exit);
> +
> +MODULE_AUTHOR("Alan Tull <atull@kernel.org>");
> +MODULE_DESCRIPTION("Intel Stratix 10 SOC FPGA Manager");
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.7.4
> 

Thanks,

Moritz

^ permalink raw reply

* Re: [PATCHv7 6/9] fpga: add intel stratix10 soc fpga manager driver
From: Moritz Fischer @ 2018-07-23 16:38 UTC (permalink / raw)
  To: richard.gong
  Cc: gregkh, catalin.marinas, will.deacon, dinguyen, robh+dt,
	mark.rutland, atull, mdf, arnd, corbet, linux-arm-kernel,
	linux-kernel, devicetree, linux-fpga, linux-doc,
	yves.vandervennet, richard.gong
In-Reply-To: <1531930724-22061-7-git-send-email-richard.gong@linux.intel.com>

Hi Alan, Richard,

On Wed, Jul 18, 2018 at 11:18:41AM -0500, richard.gong@linux.intel.com wrote:
> From: Alan Tull <atull@kernel.org>
> 
> Add driver for reconfiguring Intel Stratix10 SoC FPGA devices.
> This driver communicates through the Intel Service Driver which
> does communication with privileged hardware (that does the
> FPGA programming) through a secure mailbox.
> 
> Signed-off-by: Alan Tull <atull@kernel.org>
> Signed-off-by: Richard Gong <richard.gong@intel.com>
Acked-by: Moritz Fischer <mdf@kernel.org>
> ---
> v2: this patch is added in patch set version 2
> v3: change to align to the update of service client APIs, and the
>     update of fpga_mgr device node
> v4: changes to align with stratix10-svc-client API updates
>     add Richard's signed-off-by
> v5: update to align changes at service layer to minimize service
>     layer thread usages
> v6: add S10_RECONFIG_TIMEOUT
>     rename s/S10_BUF_TIMEOUT/S10_BUFFER_TIMEOUT/
>     fix klocwork errors
> v7: s/pr_debug/dev_dbg, s/dev_info/dev_dbg
>     add unlock buffer if s10_svc_send_msg() fails
>     add release channel if fpga_mgr_create() fails
> ---
>  drivers/fpga/Kconfig         |   6 +
>  drivers/fpga/Makefile        |   1 +
>  drivers/fpga/stratix10-soc.c | 558 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 565 insertions(+)
>  create mode 100644 drivers/fpga/stratix10-soc.c
> 
> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> index ee9c542..7d20743 100644
> --- a/drivers/fpga/Kconfig
> +++ b/drivers/fpga/Kconfig
> @@ -56,6 +56,12 @@ config FPGA_MGR_ZYNQ_FPGA
>  	help
>  	  FPGA manager driver support for Xilinx Zynq FPGAs.
>  
> +config FPGA_MGR_STRATIX10_SOC
> +	tristate "Intel Stratix10 SoC FPGA Manager"
> +	depends on (ARCH_STRATIX10 && STRATIX10_SERVICE)
> +	help
> +	  FPGA manager driver support for the Intel Stratix10 SoC.
> +
>  config FPGA_MGR_XILINX_SPI
>  	tristate "Xilinx Configuration over Slave Serial (SPI)"
>  	depends on SPI
> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> index f9803da..9f17b7f 100644
> --- a/drivers/fpga/Makefile
> +++ b/drivers/fpga/Makefile
> @@ -13,6 +13,7 @@ obj-$(CONFIG_FPGA_MGR_ICE40_SPI)	+= ice40-spi.o
>  obj-$(CONFIG_FPGA_MGR_MACHXO2_SPI)	+= machxo2-spi.o
>  obj-$(CONFIG_FPGA_MGR_SOCFPGA)		+= socfpga.o
>  obj-$(CONFIG_FPGA_MGR_SOCFPGA_A10)	+= socfpga-a10.o
> +obj-$(CONFIG_FPGA_MGR_STRATIX10_SOC)	+= stratix10-soc.o
>  obj-$(CONFIG_FPGA_MGR_TS73XX)		+= ts73xx-fpga.o
>  obj-$(CONFIG_FPGA_MGR_XILINX_SPI)	+= xilinx-spi.o
>  obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA)	+= zynq-fpga.o
> diff --git a/drivers/fpga/stratix10-soc.c b/drivers/fpga/stratix10-soc.c
> new file mode 100644
> index 0000000..f34830d
> --- /dev/null
> +++ b/drivers/fpga/stratix10-soc.c
> @@ -0,0 +1,558 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * FPGA Manager Driver for Intel Stratix10 SoC
> + *
> + *  Copyright (C) 2018 Intel Corporation
> + */
> +#include <linux/completion.h>
> +#include <linux/fpga/fpga-mgr.h>
> +#include <linux/stratix10-svc-client.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +
> +/*
> + * FPGA programming requires a higher level of privilege (EL3), per the SoC
> + * design.
> + */
> +#define NUM_SVC_BUFS	4
> +#define SVC_BUF_SIZE	SZ_512K
> +
> +/* Indicates buffer is in use if set */
> +#define SVC_BUF_LOCK	0
> +
> +#define S10_BUFFER_TIMEOUT (msecs_to_jiffies(SVC_RECONFIG_BUFFER_TIMEOUT_MS))
> +#define S10_RECONFIG_TIMEOUT (msecs_to_jiffies(SVC_RECONFIG_REQUEST_TIMEOUT_MS))
> +
> +/**
> + * struct s10_svc_buf
> + * @buf: virtual address of buf provided by service layer
> + * @lock: locked if buffer is in use
> + */
> +struct s10_svc_buf {
> +	char *buf;
> +	unsigned long lock;
> +};
> +
> +struct s10_priv {
> +	struct stratix10_svc_chan *chan;
> +	struct stratix10_svc_client client;
> +	struct completion status_return_completion;
> +	struct s10_svc_buf svc_bufs[NUM_SVC_BUFS];
> +	unsigned long status;
> +};
> +
> +static int s10_svc_send_msg(struct s10_priv *priv,
> +			    enum stratix10_svc_command_code command,
> +			    void *payload, u32 payload_length)
> +{
> +	struct stratix10_svc_chan *chan = priv->chan;
> +	struct device *dev = priv->client.dev;
> +	struct stratix10_svc_client_msg msg;
> +	int ret;
> +
> +	dev_dbg(dev, "%s cmd=%d payload=%p length=%d\n",
> +		__func__, command, payload, payload_length);
> +
> +	msg.command = command;
> +	msg.payload = payload;
> +	msg.payload_length = payload_length;
> +
> +	ret = stratix10_svc_send(chan, &msg);
> +	dev_dbg(dev, "stratix10_svc_send returned status %d\n", ret);
> +
> +	return ret;
> +}
> +
> +/**
> + * s10_free_buffers
> + * Free buffers allocated from the service layer's pool that are not in use.
> + * @mgr: fpga manager struct
> + * Free all buffers that are not in use.
> + * Return true when all buffers are freed.
> + */
> +static bool s10_free_buffers(struct fpga_manager *mgr)
> +{
> +	struct s10_priv *priv = mgr->priv;
> +	uint num_free = 0;
> +	uint i;
> +
> +	for (i = 0; i < NUM_SVC_BUFS; i++) {
> +		if (!priv->svc_bufs[i].buf) {
> +			num_free++;
> +			continue;
> +		}
> +
> +		if (!test_and_set_bit_lock(SVC_BUF_LOCK,
> +					   &priv->svc_bufs[i].lock)) {
> +			stratix10_svc_free_memory(priv->chan,
> +					      priv->svc_bufs[i].buf);
> +			priv->svc_bufs[i].buf = NULL;
> +			num_free++;
> +		}
> +	}
> +
> +	return num_free == NUM_SVC_BUFS;
> +}
> +
> +/**
> + * s10_free_buffer_count
> + * Count how many buffers are not in use.
> + * @mgr: fpga manager struct
> + * Return # of buffers that are not in use.
> + */
> +static uint s10_free_buffer_count(struct fpga_manager *mgr)
> +{
> +	struct s10_priv *priv = mgr->priv;
> +	uint num_free = 0;
> +	uint i;
> +
> +	for (i = 0; i < NUM_SVC_BUFS; i++)
> +		if (!priv->svc_bufs[i].buf)
> +			num_free++;
> +
> +	return num_free;
> +}
> +
> +/**
> + * s10_unlock_bufs
> + * Given the returned buffer address, match that address to our buffer struct
> + * and unlock that buffer.  This marks it as available to be refilled and sent
> + * (or freed).
> + * @priv: private data
> + * @kaddr: kernel address of buffer that was returned from service layer
> + */
> +static void s10_unlock_bufs(struct s10_priv *priv, void *kaddr)
> +{
> +	uint i;
> +
> +	if (!kaddr)
> +		return;
> +
> +	for (i = 0; i < NUM_SVC_BUFS; i++)
> +		if (priv->svc_bufs[i].buf == kaddr) {
> +			clear_bit_unlock(SVC_BUF_LOCK,
> +					 &priv->svc_bufs[i].lock);
> +			return;
> +		}
> +
> +	WARN(1, "Unknown buffer returned from service layer %p\n", kaddr);
> +}
> +
> +/**
> + * s10_receive_callback
> + * Callback for service layer to use to provide client (this driver) messages
> + * received through the mailbox.
> + * @client: service layer client struct
> + * @data: message
> + */
> +static void s10_receive_callback(struct stratix10_svc_client *client,
> +				 struct stratix10_svc_cb_data *data)
> +{
> +	struct s10_priv *priv = client->priv;
> +	u32 status;
> +	int i;
> +
> +	WARN_ONCE(!data, "%s: stratix10_svc_rc_data = NULL", __func__);
> +
> +	status = data->status;
> +
> +	/*
> +	 * Here we set status bits as we receive them.  Elsewhere, we always use
> +	 * test_and_clear_bit() to check status in priv->status
> +	 */
> +	for (i = 0; i <= SVC_STATUS_RECONFIG_ERROR; i++)
> +		if (status & (1 << i))
> +			set_bit(i, &priv->status);
> +
> +	if (status & BIT(SVC_STATUS_RECONFIG_BUFFER_DONE)) {
> +		s10_unlock_bufs(priv, data->kaddr1);
> +		s10_unlock_bufs(priv, data->kaddr2);
> +		s10_unlock_bufs(priv, data->kaddr3);
> +	}
> +
> +	complete(&priv->status_return_completion);
> +}
> +
> +/**
> + * s10_ops_write_init
> + * Prepare for FPGA reconfiguration by requesting partial reconfig and
> + * allocating buffers from the service layer.
> + * @mgr: fpga manager
> + * @info: fpga image info
> + * @buf: fpga image buffer
> + * @count: size of buf in bytes
> + */
> +static int s10_ops_write_init(struct fpga_manager *mgr,
> +			      struct fpga_image_info *info,
> +			      const char *buf, size_t count)
> +{
> +	struct s10_priv *priv = mgr->priv;
> +	struct device *dev = priv->client.dev;
> +	struct stratix10_svc_command_reconfig_payload payload;
> +	char *kbuf;
> +	uint i;
> +	int ret;
> +
> +	payload.flags = 0;
> +	if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
> +		dev_dbg(dev, "Requesting partial reconfiguration.\n");
> +		payload.flags |= BIT(COMMAND_RECONFIG_FLAG_PARTIAL);
> +	} else {
> +		dev_dbg(dev, "Requesting full reconfiguration.\n");
> +	}
> +
> +	reinit_completion(&priv->status_return_completion);
> +	ret = s10_svc_send_msg(priv, COMMAND_RECONFIG,
> +			       &payload, sizeof(payload));
> +	if (ret < 0)
> +		goto init_done;
> +
> +	ret = wait_for_completion_interruptible_timeout(
> +		&priv->status_return_completion, S10_RECONFIG_TIMEOUT);
> +	if (!ret) {
> +		dev_err(dev, "timeout waiting for RECONFIG_REQUEST\n");
> +		ret = -ETIMEDOUT;
> +		goto init_done;
> +	}
> +	if (ret < 0) {
> +		dev_err(dev, "error (%d) waiting for RECONFIG_REQUEST\n", ret);
> +		goto init_done;
> +	}
> +
> +	ret = 0;
> +	if (!test_and_clear_bit(SVC_STATUS_RECONFIG_REQUEST_OK,
> +				&priv->status)) {
> +		ret = -ETIMEDOUT;
> +		goto init_done;
> +	}
> +
> +	/* Allocate buffers from the service layer's pool. */
> +	for (i = 0; i < NUM_SVC_BUFS; i++) {
> +		kbuf = stratix10_svc_allocate_memory(priv->chan, SVC_BUF_SIZE);
> +		if (!kbuf) {
> +			s10_free_buffers(mgr);
> +			ret = -ENOMEM;
> +			goto init_done;
> +		}
> +
> +		priv->svc_bufs[i].buf = kbuf;
> +		priv->svc_bufs[i].lock = 0;
> +	}
> +
> +init_done:
> +	stratix10_svc_done(priv->chan);
> +	return ret;
> +}
> +
> +/**
> + * s10_send_buf
> + * Send a buffer to the service layer queue
> + * @mgr: fpga manager struct
> + * @buf: fpga image buffer
> + * @count: size of buf in bytes
> + * Returns # of bytes transferred or -ENOBUFS if the all the buffers are in use
> + * or if the service queue is full. Never returns 0.
> + */
> +static int s10_send_buf(struct fpga_manager *mgr, const char *buf, size_t count)
> +{
> +	struct s10_priv *priv = mgr->priv;
> +	struct device *dev = priv->client.dev;
> +	void *svc_buf;
> +	size_t xfer_sz;
> +	int ret;
> +	uint i;
> +
> +	/* get/lock a buffer that that's not being used */
> +	for (i = 0; i < NUM_SVC_BUFS; i++)
> +		if (!test_and_set_bit_lock(SVC_BUF_LOCK,
> +					   &priv->svc_bufs[i].lock))
> +			break;
> +
> +	if (i == NUM_SVC_BUFS)
> +		return -ENOBUFS;
> +
> +	xfer_sz = count < SVC_BUF_SIZE ? count : SVC_BUF_SIZE;
> +
> +	svc_buf = priv->svc_bufs[i].buf;
> +	memcpy(svc_buf, buf, xfer_sz);
> +	ret = s10_svc_send_msg(priv, COMMAND_RECONFIG_DATA_SUBMIT,
> +			       svc_buf, xfer_sz);
> +	if (ret < 0) {
> +		dev_err(dev,
> +			"Error while sending data to service layer (%d)", ret);
> +		clear_bit_unlock(SVC_BUF_LOCK, &priv->svc_bufs[i].lock);
> +		return ret;
> +	}
> +
> +	return xfer_sz;
> +}
> +
> +/**
> + * s10_ops_write
> + * Send a FPGA image to privileged layers to write to the FPGA.  When done
> + * sending, free all service layer buffers we allocated in write_init.
> + * @mgr: fpga manager
> + * @buf: fpga image buffer
> + * @count: size of buf in bytes
> + * Returns 0 for success or negative errno.
> + */
> +static int s10_ops_write(struct fpga_manager *mgr, const char *buf,
> +			 size_t count)
> +{
> +	struct s10_priv *priv = mgr->priv;
> +	struct device *dev = priv->client.dev;
> +	long wait_status;
> +	int sent = 0;
> +	int ret = 0;
> +
> +	/*
> +	 * Loop waiting for buffers to be returned.  When a buffer is returned,
> +	 * reuse it to send more data or free if if all data has been sent.
> +	 */
> +	while (count > 0 || s10_free_buffer_count(mgr) != NUM_SVC_BUFS) {
> +		reinit_completion(&priv->status_return_completion);
> +
> +		if (count > 0) {
> +			sent = s10_send_buf(mgr, buf, count);
> +			if (sent < 0)
> +				continue;
> +
> +			count -= sent;
> +			buf += sent;
> +		} else {
> +			if (s10_free_buffers(mgr))
> +				return 0;
> +
> +			ret = s10_svc_send_msg(
> +				priv, COMMAND_RECONFIG_DATA_CLAIM,
> +				NULL, 0);
> +			if (ret < 0)
> +				break;
> +		}
> +
> +		/*
> +		 * If callback hasn't already happened, wait for buffers to be
> +		 * returned from service layer
> +		 */
> +		wait_status = 1; /* not timed out */
> +		if (!priv->status)
> +			wait_status = wait_for_completion_interruptible_timeout(
> +				&priv->status_return_completion,
> +				S10_BUFFER_TIMEOUT);
> +
> +		if (test_and_clear_bit(SVC_STATUS_RECONFIG_BUFFER_DONE,
> +				       &priv->status) ||
> +		    test_and_clear_bit(SVC_STATUS_RECONFIG_BUFFER_SUBMITTED,
> +				       &priv->status)) {
> +			ret = 0;
> +			continue;
> +		}
> +
> +		if (test_and_clear_bit(SVC_STATUS_RECONFIG_ERROR,
> +				       &priv->status)) {
> +			dev_err(dev, "ERROR - giving up - SVC_STATUS_RECONFIG_ERROR\n");
> +			ret = -EFAULT;
> +			break;
> +		}
> +
> +		if (!wait_status) {
> +			dev_err(dev, "timeout waiting for svc layer buffers\n");
> +			ret = -ETIMEDOUT;
> +			break;
> +		}
> +		if (wait_status < 0) {
> +			ret = wait_status;
> +			dev_err(dev,
> +				"error (%d) waiting for svc layer buffers\n",
> +				ret);
> +			break;
> +		}
> +	}
> +
> +	if (!s10_free_buffers(mgr))
> +		dev_err(dev, "%s not all buffers were freed\n", __func__);
> +
> +	return ret;
> +}
> +
> +/**
> + * s10_ops_write_complete
> + * Wait for FPGA configuration to be done
> + * @mgr: fpga manager
> + * @info: fpga image info
> + * Returns 0 for success negative errno.
> + */
> +static int s10_ops_write_complete(struct fpga_manager *mgr,
> +				  struct fpga_image_info *info)
> +{
> +	struct s10_priv *priv = mgr->priv;
> +	struct device *dev = priv->client.dev;
> +	unsigned long timeout;
> +	int ret;
> +
> +	timeout = usecs_to_jiffies(info->config_complete_timeout_us);
> +
> +	do {
> +		reinit_completion(&priv->status_return_completion);
> +
> +		ret = s10_svc_send_msg(priv, COMMAND_RECONFIG_STATUS, NULL, 0);
> +		if (ret < 0)
> +			break;
> +
> +		ret = wait_for_completion_interruptible_timeout(
> +			&priv->status_return_completion, timeout);
> +		if (!ret) {
> +			dev_err(dev,
> +				"timeout waiting for RECONFIG_COMPLETED\n");
> +			ret = -ETIMEDOUT;
> +			break;
> +		}
> +		if (ret < 0) {
> +			dev_err(dev,
> +				"error (%d) waiting for RECONFIG_COMPLETED\n",
> +				ret);
> +			break;
> +		}
> +		/* Not error or timeout, so ret is # of jiffies until timeout */
> +		timeout = ret;
> +		ret = 0;
> +
> +		if (test_and_clear_bit(SVC_STATUS_RECONFIG_COMPLETED,
> +				       &priv->status))
> +			break;
> +
> +		if (test_and_clear_bit(SVC_STATUS_RECONFIG_ERROR,
> +				       &priv->status)) {
> +			dev_err(dev, "ERROR - giving up - SVC_STATUS_RECONFIG_ERROR\n");
> +			ret = -EFAULT;
> +			break;
> +		}
> +	} while (1);
> +
> +	stratix10_svc_done(priv->chan);
> +
> +	return ret;
> +}
> +
> +static enum fpga_mgr_states s10_ops_state(struct fpga_manager *mgr)
> +{
> +	return FPGA_MGR_STATE_UNKNOWN;
> +}
> +
> +static const struct fpga_manager_ops s10_ops = {
> +	.state = s10_ops_state,
> +	.write_init = s10_ops_write_init,
> +	.write = s10_ops_write,
> +	.write_complete = s10_ops_write_complete,
> +};
> +
> +static int s10_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct s10_priv *priv;
> +	struct fpga_manager *mgr;
> +	int ret;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->client.dev = dev;
> +	priv->client.receive_cb = s10_receive_callback;
> +	priv->client.priv = priv;
> +
> +	priv->chan = stratix10_svc_request_channel_byname(&priv->client,
> +						SVC_CLIENT_FPGA);
> +	if (IS_ERR(priv->chan)) {
> +		dev_err(dev, "couldn't get service channel (%s)\n",
> +			SVC_CLIENT_FPGA);
> +		return PTR_ERR(priv->chan);
> +	}
> +
> +	init_completion(&priv->status_return_completion);
> +
> +	mgr = fpga_mgr_create(dev, "Stratix10 SOC FPGA Manager",
> +			      &s10_ops, priv);
> +	if (!mgr) {
> +		dev_err(dev, "unable to create FPGA manager\n");
> +		stratix10_svc_free_channel(priv->chan);
> +		return -ENOMEM;
Would this be shorter with a goto and common error handling at
the end of the function?
> +	}
> +
> +	ret = fpga_mgr_register(mgr);
> +	if (ret) {
> +		dev_err(dev, "unable to register FPGA manager\n");
> +		stratix10_svc_free_channel(priv->chan);
> +		fpga_mgr_free(mgr);
> +		return ret;
> +	}
> +
> +	platform_set_drvdata(pdev, mgr);
> +
> +	return ret;
> +}
> +
> +static int s10_remove(struct platform_device *pdev)
> +{
> +	struct fpga_manager *mgr = platform_get_drvdata(pdev);
> +	struct s10_priv *priv = mgr->priv;
> +
> +	fpga_mgr_unregister(mgr);
> +	stratix10_svc_free_channel(priv->chan);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id s10_of_match[] = {
> +	{ .compatible = "intel,stratix10-soc-fpga-mgr", },
> +	{},
> +};
> +
> +MODULE_DEVICE_TABLE(of, s10_of_match);
> +
> +static struct platform_driver s10_driver = {
> +	.probe = s10_probe,
> +	.remove = s10_remove,
> +	.driver = {
> +		.name	= "Stratix10 SoC FPGA manager",
> +		.of_match_table = of_match_ptr(s10_of_match),
> +	},
> +};
> +
> +static int __init s10_init(void)
> +{
> +	struct device_node *fw_np;
> +	struct device_node *np;
> +	int ret;
> +
> +	fw_np = of_find_node_by_name(NULL, "svc");
> +	if (!fw_np)
> +		return -ENODEV;
> +
> +	np = of_find_matching_node(fw_np, s10_of_match);
> +	if (!np) {
> +		of_node_put(fw_np);
> +		return -ENODEV;
> +	}
> +
> +	of_node_put(np);
> +	ret = of_platform_populate(fw_np, s10_of_match, NULL, NULL);
> +	of_node_put(fw_np);
> +	if (ret)
> +		return ret;
> +
> +	return platform_driver_register(&s10_driver);
> +}
> +
> +static void __exit s10_exit(void)
> +{
> +	return platform_driver_unregister(&s10_driver);
> +}
> +
> +module_init(s10_init);
> +module_exit(s10_exit);
> +
> +MODULE_AUTHOR("Alan Tull <atull@kernel.org>");
> +MODULE_DESCRIPTION("Intel Stratix 10 SOC FPGA Manager");
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.7.4
> 

Thanks,

Moritz

^ permalink raw reply

* Re: [PATCHv7 6/9] fpga: add intel stratix10 soc fpga manager driver
From: Moritz Fischer @ 2018-07-23 16:38 UTC (permalink / raw)
  To: richard.gong
  Cc: gregkh, catalin.marinas, will.deacon, dinguyen, robh+dt,
	mark.rutland, atull, mdf, arnd, corbet, linux-arm-kernel,
	linux-kernel, devicetree, linux-fpga, linux-doc,
	yves.vandervennet, richard.gong
In-Reply-To: <1531930724-22061-7-git-send-email-richard.gong@linux.intel.com>

Hi Alan, Richard,

On Wed, Jul 18, 2018 at 11:18:41AM -0500, richard.gong@linux.intel.com wrote:
> From: Alan Tull <atull@kernel.org>
> 
> Add driver for reconfiguring Intel Stratix10 SoC FPGA devices.
> This driver communicates through the Intel Service Driver which
> does communication with privileged hardware (that does the
> FPGA programming) through a secure mailbox.
> 
> Signed-off-by: Alan Tull <atull@kernel.org>
> Signed-off-by: Richard Gong <richard.gong@intel.com>
Acked-by: Moritz Fischer <mdf@kernel.org>
> ---
> v2: this patch is added in patch set version 2
> v3: change to align to the update of service client APIs, and the
>     update of fpga_mgr device node
> v4: changes to align with stratix10-svc-client API updates
>     add Richard's signed-off-by
> v5: update to align changes at service layer to minimize service
>     layer thread usages
> v6: add S10_RECONFIG_TIMEOUT
>     rename s/S10_BUF_TIMEOUT/S10_BUFFER_TIMEOUT/
>     fix klocwork errors
> v7: s/pr_debug/dev_dbg, s/dev_info/dev_dbg
>     add unlock buffer if s10_svc_send_msg() fails
>     add release channel if fpga_mgr_create() fails
> ---
>  drivers/fpga/Kconfig         |   6 +
>  drivers/fpga/Makefile        |   1 +
>  drivers/fpga/stratix10-soc.c | 558 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 565 insertions(+)
>  create mode 100644 drivers/fpga/stratix10-soc.c
> 
> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> index ee9c542..7d20743 100644
> --- a/drivers/fpga/Kconfig
> +++ b/drivers/fpga/Kconfig
> @@ -56,6 +56,12 @@ config FPGA_MGR_ZYNQ_FPGA
>  	help
>  	  FPGA manager driver support for Xilinx Zynq FPGAs.
>  
> +config FPGA_MGR_STRATIX10_SOC
> +	tristate "Intel Stratix10 SoC FPGA Manager"
> +	depends on (ARCH_STRATIX10 && STRATIX10_SERVICE)
> +	help
> +	  FPGA manager driver support for the Intel Stratix10 SoC.
> +
>  config FPGA_MGR_XILINX_SPI
>  	tristate "Xilinx Configuration over Slave Serial (SPI)"
>  	depends on SPI
> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> index f9803da..9f17b7f 100644
> --- a/drivers/fpga/Makefile
> +++ b/drivers/fpga/Makefile
> @@ -13,6 +13,7 @@ obj-$(CONFIG_FPGA_MGR_ICE40_SPI)	+= ice40-spi.o
>  obj-$(CONFIG_FPGA_MGR_MACHXO2_SPI)	+= machxo2-spi.o
>  obj-$(CONFIG_FPGA_MGR_SOCFPGA)		+= socfpga.o
>  obj-$(CONFIG_FPGA_MGR_SOCFPGA_A10)	+= socfpga-a10.o
> +obj-$(CONFIG_FPGA_MGR_STRATIX10_SOC)	+= stratix10-soc.o
>  obj-$(CONFIG_FPGA_MGR_TS73XX)		+= ts73xx-fpga.o
>  obj-$(CONFIG_FPGA_MGR_XILINX_SPI)	+= xilinx-spi.o
>  obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA)	+= zynq-fpga.o
> diff --git a/drivers/fpga/stratix10-soc.c b/drivers/fpga/stratix10-soc.c
> new file mode 100644
> index 0000000..f34830d
> --- /dev/null
> +++ b/drivers/fpga/stratix10-soc.c
> @@ -0,0 +1,558 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * FPGA Manager Driver for Intel Stratix10 SoC
> + *
> + *  Copyright (C) 2018 Intel Corporation
> + */
> +#include <linux/completion.h>
> +#include <linux/fpga/fpga-mgr.h>
> +#include <linux/stratix10-svc-client.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +
> +/*
> + * FPGA programming requires a higher level of privilege (EL3), per the SoC
> + * design.
> + */
> +#define NUM_SVC_BUFS	4
> +#define SVC_BUF_SIZE	SZ_512K
> +
> +/* Indicates buffer is in use if set */
> +#define SVC_BUF_LOCK	0
> +
> +#define S10_BUFFER_TIMEOUT (msecs_to_jiffies(SVC_RECONFIG_BUFFER_TIMEOUT_MS))
> +#define S10_RECONFIG_TIMEOUT (msecs_to_jiffies(SVC_RECONFIG_REQUEST_TIMEOUT_MS))
> +
> +/**
> + * struct s10_svc_buf
> + * @buf: virtual address of buf provided by service layer
> + * @lock: locked if buffer is in use
> + */
> +struct s10_svc_buf {
> +	char *buf;
> +	unsigned long lock;
> +};
> +
> +struct s10_priv {
> +	struct stratix10_svc_chan *chan;
> +	struct stratix10_svc_client client;
> +	struct completion status_return_completion;
> +	struct s10_svc_buf svc_bufs[NUM_SVC_BUFS];
> +	unsigned long status;
> +};
> +
> +static int s10_svc_send_msg(struct s10_priv *priv,
> +			    enum stratix10_svc_command_code command,
> +			    void *payload, u32 payload_length)
> +{
> +	struct stratix10_svc_chan *chan = priv->chan;
> +	struct device *dev = priv->client.dev;
> +	struct stratix10_svc_client_msg msg;
> +	int ret;
> +
> +	dev_dbg(dev, "%s cmd=%d payload=%p length=%d\n",
> +		__func__, command, payload, payload_length);
> +
> +	msg.command = command;
> +	msg.payload = payload;
> +	msg.payload_length = payload_length;
> +
> +	ret = stratix10_svc_send(chan, &msg);
> +	dev_dbg(dev, "stratix10_svc_send returned status %d\n", ret);
> +
> +	return ret;
> +}
> +
> +/**
> + * s10_free_buffers
> + * Free buffers allocated from the service layer's pool that are not in use.
> + * @mgr: fpga manager struct
> + * Free all buffers that are not in use.
> + * Return true when all buffers are freed.
> + */
> +static bool s10_free_buffers(struct fpga_manager *mgr)
> +{
> +	struct s10_priv *priv = mgr->priv;
> +	uint num_free = 0;
> +	uint i;
> +
> +	for (i = 0; i < NUM_SVC_BUFS; i++) {
> +		if (!priv->svc_bufs[i].buf) {
> +			num_free++;
> +			continue;
> +		}
> +
> +		if (!test_and_set_bit_lock(SVC_BUF_LOCK,
> +					   &priv->svc_bufs[i].lock)) {
> +			stratix10_svc_free_memory(priv->chan,
> +					      priv->svc_bufs[i].buf);
> +			priv->svc_bufs[i].buf = NULL;
> +			num_free++;
> +		}
> +	}
> +
> +	return num_free == NUM_SVC_BUFS;
> +}
> +
> +/**
> + * s10_free_buffer_count
> + * Count how many buffers are not in use.
> + * @mgr: fpga manager struct
> + * Return # of buffers that are not in use.
> + */
> +static uint s10_free_buffer_count(struct fpga_manager *mgr)
> +{
> +	struct s10_priv *priv = mgr->priv;
> +	uint num_free = 0;
> +	uint i;
> +
> +	for (i = 0; i < NUM_SVC_BUFS; i++)
> +		if (!priv->svc_bufs[i].buf)
> +			num_free++;
> +
> +	return num_free;
> +}
> +
> +/**
> + * s10_unlock_bufs
> + * Given the returned buffer address, match that address to our buffer struct
> + * and unlock that buffer.  This marks it as available to be refilled and sent
> + * (or freed).
> + * @priv: private data
> + * @kaddr: kernel address of buffer that was returned from service layer
> + */
> +static void s10_unlock_bufs(struct s10_priv *priv, void *kaddr)
> +{
> +	uint i;
> +
> +	if (!kaddr)
> +		return;
> +
> +	for (i = 0; i < NUM_SVC_BUFS; i++)
> +		if (priv->svc_bufs[i].buf == kaddr) {
> +			clear_bit_unlock(SVC_BUF_LOCK,
> +					 &priv->svc_bufs[i].lock);
> +			return;
> +		}
> +
> +	WARN(1, "Unknown buffer returned from service layer %p\n", kaddr);
> +}
> +
> +/**
> + * s10_receive_callback
> + * Callback for service layer to use to provide client (this driver) messages
> + * received through the mailbox.
> + * @client: service layer client struct
> + * @data: message
> + */
> +static void s10_receive_callback(struct stratix10_svc_client *client,
> +				 struct stratix10_svc_cb_data *data)
> +{
> +	struct s10_priv *priv = client->priv;
> +	u32 status;
> +	int i;
> +
> +	WARN_ONCE(!data, "%s: stratix10_svc_rc_data = NULL", __func__);
> +
> +	status = data->status;
> +
> +	/*
> +	 * Here we set status bits as we receive them.  Elsewhere, we always use
> +	 * test_and_clear_bit() to check status in priv->status
> +	 */
> +	for (i = 0; i <= SVC_STATUS_RECONFIG_ERROR; i++)
> +		if (status & (1 << i))
> +			set_bit(i, &priv->status);
> +
> +	if (status & BIT(SVC_STATUS_RECONFIG_BUFFER_DONE)) {
> +		s10_unlock_bufs(priv, data->kaddr1);
> +		s10_unlock_bufs(priv, data->kaddr2);
> +		s10_unlock_bufs(priv, data->kaddr3);
> +	}
> +
> +	complete(&priv->status_return_completion);
> +}
> +
> +/**
> + * s10_ops_write_init
> + * Prepare for FPGA reconfiguration by requesting partial reconfig and
> + * allocating buffers from the service layer.
> + * @mgr: fpga manager
> + * @info: fpga image info
> + * @buf: fpga image buffer
> + * @count: size of buf in bytes
> + */
> +static int s10_ops_write_init(struct fpga_manager *mgr,
> +			      struct fpga_image_info *info,
> +			      const char *buf, size_t count)
> +{
> +	struct s10_priv *priv = mgr->priv;
> +	struct device *dev = priv->client.dev;
> +	struct stratix10_svc_command_reconfig_payload payload;
> +	char *kbuf;
> +	uint i;
> +	int ret;
> +
> +	payload.flags = 0;
> +	if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
> +		dev_dbg(dev, "Requesting partial reconfiguration.\n");
> +		payload.flags |= BIT(COMMAND_RECONFIG_FLAG_PARTIAL);
> +	} else {
> +		dev_dbg(dev, "Requesting full reconfiguration.\n");
> +	}
> +
> +	reinit_completion(&priv->status_return_completion);
> +	ret = s10_svc_send_msg(priv, COMMAND_RECONFIG,
> +			       &payload, sizeof(payload));
> +	if (ret < 0)
> +		goto init_done;
> +
> +	ret = wait_for_completion_interruptible_timeout(
> +		&priv->status_return_completion, S10_RECONFIG_TIMEOUT);
> +	if (!ret) {
> +		dev_err(dev, "timeout waiting for RECONFIG_REQUEST\n");
> +		ret = -ETIMEDOUT;
> +		goto init_done;
> +	}
> +	if (ret < 0) {
> +		dev_err(dev, "error (%d) waiting for RECONFIG_REQUEST\n", ret);
> +		goto init_done;
> +	}
> +
> +	ret = 0;
> +	if (!test_and_clear_bit(SVC_STATUS_RECONFIG_REQUEST_OK,
> +				&priv->status)) {
> +		ret = -ETIMEDOUT;
> +		goto init_done;
> +	}
> +
> +	/* Allocate buffers from the service layer's pool. */
> +	for (i = 0; i < NUM_SVC_BUFS; i++) {
> +		kbuf = stratix10_svc_allocate_memory(priv->chan, SVC_BUF_SIZE);
> +		if (!kbuf) {
> +			s10_free_buffers(mgr);
> +			ret = -ENOMEM;
> +			goto init_done;
> +		}
> +
> +		priv->svc_bufs[i].buf = kbuf;
> +		priv->svc_bufs[i].lock = 0;
> +	}
> +
> +init_done:
> +	stratix10_svc_done(priv->chan);
> +	return ret;
> +}
> +
> +/**
> + * s10_send_buf
> + * Send a buffer to the service layer queue
> + * @mgr: fpga manager struct
> + * @buf: fpga image buffer
> + * @count: size of buf in bytes
> + * Returns # of bytes transferred or -ENOBUFS if the all the buffers are in use
> + * or if the service queue is full. Never returns 0.
> + */
> +static int s10_send_buf(struct fpga_manager *mgr, const char *buf, size_t count)
> +{
> +	struct s10_priv *priv = mgr->priv;
> +	struct device *dev = priv->client.dev;
> +	void *svc_buf;
> +	size_t xfer_sz;
> +	int ret;
> +	uint i;
> +
> +	/* get/lock a buffer that that's not being used */
> +	for (i = 0; i < NUM_SVC_BUFS; i++)
> +		if (!test_and_set_bit_lock(SVC_BUF_LOCK,
> +					   &priv->svc_bufs[i].lock))
> +			break;
> +
> +	if (i == NUM_SVC_BUFS)
> +		return -ENOBUFS;
> +
> +	xfer_sz = count < SVC_BUF_SIZE ? count : SVC_BUF_SIZE;
> +
> +	svc_buf = priv->svc_bufs[i].buf;
> +	memcpy(svc_buf, buf, xfer_sz);
> +	ret = s10_svc_send_msg(priv, COMMAND_RECONFIG_DATA_SUBMIT,
> +			       svc_buf, xfer_sz);
> +	if (ret < 0) {
> +		dev_err(dev,
> +			"Error while sending data to service layer (%d)", ret);
> +		clear_bit_unlock(SVC_BUF_LOCK, &priv->svc_bufs[i].lock);
> +		return ret;
> +	}
> +
> +	return xfer_sz;
> +}
> +
> +/**
> + * s10_ops_write
> + * Send a FPGA image to privileged layers to write to the FPGA.  When done
> + * sending, free all service layer buffers we allocated in write_init.
> + * @mgr: fpga manager
> + * @buf: fpga image buffer
> + * @count: size of buf in bytes
> + * Returns 0 for success or negative errno.
> + */
> +static int s10_ops_write(struct fpga_manager *mgr, const char *buf,
> +			 size_t count)
> +{
> +	struct s10_priv *priv = mgr->priv;
> +	struct device *dev = priv->client.dev;
> +	long wait_status;
> +	int sent = 0;
> +	int ret = 0;
> +
> +	/*
> +	 * Loop waiting for buffers to be returned.  When a buffer is returned,
> +	 * reuse it to send more data or free if if all data has been sent.
> +	 */
> +	while (count > 0 || s10_free_buffer_count(mgr) != NUM_SVC_BUFS) {
> +		reinit_completion(&priv->status_return_completion);
> +
> +		if (count > 0) {
> +			sent = s10_send_buf(mgr, buf, count);
> +			if (sent < 0)
> +				continue;
> +
> +			count -= sent;
> +			buf += sent;
> +		} else {
> +			if (s10_free_buffers(mgr))
> +				return 0;
> +
> +			ret = s10_svc_send_msg(
> +				priv, COMMAND_RECONFIG_DATA_CLAIM,
> +				NULL, 0);
> +			if (ret < 0)
> +				break;
> +		}
> +
> +		/*
> +		 * If callback hasn't already happened, wait for buffers to be
> +		 * returned from service layer
> +		 */
> +		wait_status = 1; /* not timed out */
> +		if (!priv->status)
> +			wait_status = wait_for_completion_interruptible_timeout(
> +				&priv->status_return_completion,
> +				S10_BUFFER_TIMEOUT);
> +
> +		if (test_and_clear_bit(SVC_STATUS_RECONFIG_BUFFER_DONE,
> +				       &priv->status) ||
> +		    test_and_clear_bit(SVC_STATUS_RECONFIG_BUFFER_SUBMITTED,
> +				       &priv->status)) {
> +			ret = 0;
> +			continue;
> +		}
> +
> +		if (test_and_clear_bit(SVC_STATUS_RECONFIG_ERROR,
> +				       &priv->status)) {
> +			dev_err(dev, "ERROR - giving up - SVC_STATUS_RECONFIG_ERROR\n");
> +			ret = -EFAULT;
> +			break;
> +		}
> +
> +		if (!wait_status) {
> +			dev_err(dev, "timeout waiting for svc layer buffers\n");
> +			ret = -ETIMEDOUT;
> +			break;
> +		}
> +		if (wait_status < 0) {
> +			ret = wait_status;
> +			dev_err(dev,
> +				"error (%d) waiting for svc layer buffers\n",
> +				ret);
> +			break;
> +		}
> +	}
> +
> +	if (!s10_free_buffers(mgr))
> +		dev_err(dev, "%s not all buffers were freed\n", __func__);
> +
> +	return ret;
> +}
> +
> +/**
> + * s10_ops_write_complete
> + * Wait for FPGA configuration to be done
> + * @mgr: fpga manager
> + * @info: fpga image info
> + * Returns 0 for success negative errno.
> + */
> +static int s10_ops_write_complete(struct fpga_manager *mgr,
> +				  struct fpga_image_info *info)
> +{
> +	struct s10_priv *priv = mgr->priv;
> +	struct device *dev = priv->client.dev;
> +	unsigned long timeout;
> +	int ret;
> +
> +	timeout = usecs_to_jiffies(info->config_complete_timeout_us);
> +
> +	do {
> +		reinit_completion(&priv->status_return_completion);
> +
> +		ret = s10_svc_send_msg(priv, COMMAND_RECONFIG_STATUS, NULL, 0);
> +		if (ret < 0)
> +			break;
> +
> +		ret = wait_for_completion_interruptible_timeout(
> +			&priv->status_return_completion, timeout);
> +		if (!ret) {
> +			dev_err(dev,
> +				"timeout waiting for RECONFIG_COMPLETED\n");
> +			ret = -ETIMEDOUT;
> +			break;
> +		}
> +		if (ret < 0) {
> +			dev_err(dev,
> +				"error (%d) waiting for RECONFIG_COMPLETED\n",
> +				ret);
> +			break;
> +		}
> +		/* Not error or timeout, so ret is # of jiffies until timeout */
> +		timeout = ret;
> +		ret = 0;
> +
> +		if (test_and_clear_bit(SVC_STATUS_RECONFIG_COMPLETED,
> +				       &priv->status))
> +			break;
> +
> +		if (test_and_clear_bit(SVC_STATUS_RECONFIG_ERROR,
> +				       &priv->status)) {
> +			dev_err(dev, "ERROR - giving up - SVC_STATUS_RECONFIG_ERROR\n");
> +			ret = -EFAULT;
> +			break;
> +		}
> +	} while (1);
> +
> +	stratix10_svc_done(priv->chan);
> +
> +	return ret;
> +}
> +
> +static enum fpga_mgr_states s10_ops_state(struct fpga_manager *mgr)
> +{
> +	return FPGA_MGR_STATE_UNKNOWN;
> +}
> +
> +static const struct fpga_manager_ops s10_ops = {
> +	.state = s10_ops_state,
> +	.write_init = s10_ops_write_init,
> +	.write = s10_ops_write,
> +	.write_complete = s10_ops_write_complete,
> +};
> +
> +static int s10_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct s10_priv *priv;
> +	struct fpga_manager *mgr;
> +	int ret;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->client.dev = dev;
> +	priv->client.receive_cb = s10_receive_callback;
> +	priv->client.priv = priv;
> +
> +	priv->chan = stratix10_svc_request_channel_byname(&priv->client,
> +						SVC_CLIENT_FPGA);
> +	if (IS_ERR(priv->chan)) {
> +		dev_err(dev, "couldn't get service channel (%s)\n",
> +			SVC_CLIENT_FPGA);
> +		return PTR_ERR(priv->chan);
> +	}
> +
> +	init_completion(&priv->status_return_completion);
> +
> +	mgr = fpga_mgr_create(dev, "Stratix10 SOC FPGA Manager",
> +			      &s10_ops, priv);
> +	if (!mgr) {
> +		dev_err(dev, "unable to create FPGA manager\n");
> +		stratix10_svc_free_channel(priv->chan);
> +		return -ENOMEM;
Would this be shorter with a goto and common error handling at
the end of the function?
> +	}
> +
> +	ret = fpga_mgr_register(mgr);
> +	if (ret) {
> +		dev_err(dev, "unable to register FPGA manager\n");
> +		stratix10_svc_free_channel(priv->chan);
> +		fpga_mgr_free(mgr);
> +		return ret;
> +	}
> +
> +	platform_set_drvdata(pdev, mgr);
> +
> +	return ret;
> +}
> +
> +static int s10_remove(struct platform_device *pdev)
> +{
> +	struct fpga_manager *mgr = platform_get_drvdata(pdev);
> +	struct s10_priv *priv = mgr->priv;
> +
> +	fpga_mgr_unregister(mgr);
> +	stratix10_svc_free_channel(priv->chan);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id s10_of_match[] = {
> +	{ .compatible = "intel,stratix10-soc-fpga-mgr", },
> +	{},
> +};
> +
> +MODULE_DEVICE_TABLE(of, s10_of_match);
> +
> +static struct platform_driver s10_driver = {
> +	.probe = s10_probe,
> +	.remove = s10_remove,
> +	.driver = {
> +		.name	= "Stratix10 SoC FPGA manager",
> +		.of_match_table = of_match_ptr(s10_of_match),
> +	},
> +};
> +
> +static int __init s10_init(void)
> +{
> +	struct device_node *fw_np;
> +	struct device_node *np;
> +	int ret;
> +
> +	fw_np = of_find_node_by_name(NULL, "svc");
> +	if (!fw_np)
> +		return -ENODEV;
> +
> +	np = of_find_matching_node(fw_np, s10_of_match);
> +	if (!np) {
> +		of_node_put(fw_np);
> +		return -ENODEV;
> +	}
> +
> +	of_node_put(np);
> +	ret = of_platform_populate(fw_np, s10_of_match, NULL, NULL);
> +	of_node_put(fw_np);
> +	if (ret)
> +		return ret;
> +
> +	return platform_driver_register(&s10_driver);
> +}
> +
> +static void __exit s10_exit(void)
> +{
> +	return platform_driver_unregister(&s10_driver);
> +}
> +
> +module_init(s10_init);
> +module_exit(s10_exit);
> +
> +MODULE_AUTHOR("Alan Tull <atull@kernel.org>");
> +MODULE_DESCRIPTION("Intel Stratix 10 SOC FPGA Manager");
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.7.4
> 

Thanks,

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

^ permalink raw reply

* Re: [PATCH] option: Do not try to bind to ADB interfaces
From: Romain Izard @ 2018-07-23 16:37 UTC (permalink / raw)
  To: Lars Melin; +Cc: Johan Hovold, Greg Kroah-Hartman, linux-usb, LKML, stable
In-Reply-To: <73f2233a-cafd-a477-0c0a-260be832777d@gmail.com>

2018-07-23 17:15 GMT+02:00 Lars Melin <larsm17@gmail.com>:
> On 7/23/2018 21:02, Romain Izard wrote:
>>
>> Some modems now use the Android Debug Bridge to provide a debugging
>> interface, and some phones can also export serial ports managed by the
>> "option" driver.
>>
>> The ADB daemon running in userspace tries to use USB interfaces with
>> bDeviceClass=0xFF, bDeviceSubClass=0x42, bDeviceProtocol=1
>>
>> Prevent the option driver from binding to those interfaces, as they
>> will not be serial ports.
>
>
> You are assuming that an interface with these attributes are always a
> ADB interface - that is wrong. Vendor specific class (0xff) is not
> standardized to be something specific.

Yes. And the option driver binds to all the vendor-specific interfaces for many
devices, assuming all the vendor-specific interfaces are serial ports, unless
they are blacklisted.

>
>> This can fix issues like:
>> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=781256
>>
>
> You are trying to solve a 4++ years old bug report where it was assumed that
> the option driver was the culprit. The device in question, a Qualcomm modem
> with vid/pid 05c6:9025 has never been included in option.

While I'm not going to investigate why the 3.16 kernel did it, in the bug
report the logs indicated that 05c6:9025 was bound to the option driver. But I
did not try to solve this issue directly, I reported it as it led me to the
solution for a similar problem that I encountered last week.

A vendor for an SDK based on a modem was using an old Ubuntu 16.04, and
everything was working correctly. But on my 18.04, and on an up-to-date 16.04
LTS, it was impossible to use ADB with the vendor's SDK.

As ADB has many other failure modes, it took a lot of time to see the source of
the problem. The option driver in the old Ubuntu did not know about the modem's
vid/pid, so the driver was not bound by default and the ADB interface was free.
But the modem was added in the stable branch of the kernel, so the driver was
greedily binding to all interfaces that were not blacklisted. As a result, it
was not possible to use ADB as long as the interface was bound to the option
driver.

One of the reasons for the problem, of course, was the reuse by the modem
manufacturer of the same vid/pid for the standard version of the modem and for
the SDK version.

I see my change as a useful heuristic. ADB is a quite common protocol, using
one of the 65536 vendor subclass/protocol combinations. As you can see in
https://github.com/apkudo/adbusbini more than 3000 vendors have been spotted
using it in the wild, including more than half of those mentioned in the option
driver.

And on the other hand, I don't expect any existing modem to use 0xff,0x42,0x01
for its serial port interface, and a USB facedancer would not gain anything by
doing so.

Best regards,
-- 
Romain Izard

^ permalink raw reply

* option: Do not try to bind to ADB interfaces
From: Romain Izard @ 2018-07-23 16:37 UTC (permalink / raw)
  To: Lars Melin; +Cc: Johan Hovold, Greg Kroah-Hartman, linux-usb, LKML, stable

2018-07-23 17:15 GMT+02:00 Lars Melin <larsm17@gmail.com>:
> On 7/23/2018 21:02, Romain Izard wrote:
>>
>> Some modems now use the Android Debug Bridge to provide a debugging
>> interface, and some phones can also export serial ports managed by the
>> "option" driver.
>>
>> The ADB daemon running in userspace tries to use USB interfaces with
>> bDeviceClass=0xFF, bDeviceSubClass=0x42, bDeviceProtocol=1
>>
>> Prevent the option driver from binding to those interfaces, as they
>> will not be serial ports.
>
>
> You are assuming that an interface with these attributes are always a
> ADB interface - that is wrong. Vendor specific class (0xff) is not
> standardized to be something specific.

Yes. And the option driver binds to all the vendor-specific interfaces for many
devices, assuming all the vendor-specific interfaces are serial ports, unless
they are blacklisted.

>
>> This can fix issues like:
>> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=781256
>>
>
> You are trying to solve a 4++ years old bug report where it was assumed that
> the option driver was the culprit. The device in question, a Qualcomm modem
> with vid/pid 05c6:9025 has never been included in option.

While I'm not going to investigate why the 3.16 kernel did it, in the bug
report the logs indicated that 05c6:9025 was bound to the option driver. But I
did not try to solve this issue directly, I reported it as it led me to the
solution for a similar problem that I encountered last week.

A vendor for an SDK based on a modem was using an old Ubuntu 16.04, and
everything was working correctly. But on my 18.04, and on an up-to-date 16.04
LTS, it was impossible to use ADB with the vendor's SDK.

As ADB has many other failure modes, it took a lot of time to see the source of
the problem. The option driver in the old Ubuntu did not know about the modem's
vid/pid, so the driver was not bound by default and the ADB interface was free.
But the modem was added in the stable branch of the kernel, so the driver was
greedily binding to all interfaces that were not blacklisted. As a result, it
was not possible to use ADB as long as the interface was bound to the option
driver.

One of the reasons for the problem, of course, was the reuse by the modem
manufacturer of the same vid/pid for the standard version of the modem and for
the SDK version.

I see my change as a useful heuristic. ADB is a quite common protocol, using
one of the 65536 vendor subclass/protocol combinations. As you can see in
https://github.com/apkudo/adbusbini more than 3000 vendors have been spotted
using it in the wild, including more than half of those mentioned in the option
driver.

And on the other hand, I don't expect any existing modem to use 0xff,0x42,0x01
for its serial port interface, and a USB facedancer would not gain anything by
doing so.

Best regards,

^ permalink raw reply

* Re: [PATCH v1] earlyprintk configuration for rk3399 boards
From: Ihor Matushchak @ 2018-07-23 16:37 UTC (permalink / raw)
  To: Julien Grall
  Cc: xen-devel, Stefano Stabellini,
	Ігор Матущак,
	xen-devel
In-Reply-To: <d6883cd0-1138-5df2-34ee-cf9a2774cfcb@arm.com>


[-- Attachment #1.1: Type: text/plain, Size: 3543 bytes --]

Julien,

Got it, thanks.

*BR,*
*Ihor Matushchak*

2018-07-23 17:59 GMT+03:00 Julien Grall <julien.grall@arm.com>:

>
>
> On 23/07/18 15:47, Ihor Matushchak wrote:
>
>> Hi Julien,
>>
>
> Hi Ihor,
>
>
>> Sure, I would be glad to do that.
>>
>> I suppose it should be placed somewhere on wiki.xenproject.org <
>> http://wiki.xenproject.org>?
>>
>
> We usually create a new platform under https://wiki.xenproject.org/wi
> ki/Xen_ARM_with_Virtualization_Extensions
>
> We can then add a link on the main page to the documentation of the rk3399.
>
> Cheers,
>
> /
>> /
>> /BR,/
>> /Ihor Matushchak/
>>
>> 2018-07-23 17:33 GMT+03:00 Julien Grall <julien.grall@arm.com <mailto:
>> julien.grall@arm.com>>:
>>
>>     On 06/07/18 12:04, Ігор Матущак wrote:
>>
>>         Hello Julien,
>>
>>
>>     Hi Ihor,
>>
>>     Sorry for the late reply.
>>
>>
>>         I suppose there is no such documentation, at least I didn't find
>>         any.
>>         Bringing up XEN 4.10.0 on RK3399 is my pet-project.
>>         My current target board is Ibox3399
>>         <https://www.aliexpress.com/item/RK3399-Development-Board-Ib
>> ox3399-2GB-DDR3-16GB-EMMC-Six-Core-A72-A53-Mali-T860-GPU-
>> Android6/32816273232.html
>>         <https://www.aliexpress.com/item/RK3399-Development-Board-Ib
>> ox3399-2GB-DDR3-16GB-EMMC-Six-Core-A72-A53-Mali-T860-GPU-
>> Android6/32816273232.html>>
>>
>>         At the moment I got dom0 running on this board and some test
>>         guest domain (both 4.4 kernels).
>>         Actually no changes made to XEN (except earlyprintk) at the
>>         moment, Xen works from the box.
>>
>>
>>     Glad to see Xen booting out of box on RK3399 :).
>>
>>     The alias is only for convience. It would be possible to do the same
>>     with 8250,0xff1a0000,2.
>>
>>     So instead of that patch, I would suggest to write down a
>>     documentation for booting Xen on the board and early debugging.
>>
>>     What do you think?
>>
>>     Cheers,
>>
>>
>>         /BR,/
>>         /Ihor Matushchak/
>>
>>         2018-07-06 13:36 GMT+03:00 Julien Grall <julien.grall@arm.com
>>         <mailto:julien.grall@arm.com> <mailto:julien.grall@arm.com
>>         <mailto:julien.grall@arm.com>>>:
>>
>>              Hello,
>>
>>              On 04/07/18 21:55, ihor.matushchak@foobox.net
>>         <mailto:ihor.matushchak@foobox.net>
>>              <mailto:ihor.matushchak@foobox.net
>>         <mailto:ihor.matushchak@foobox.net>> wrote:
>>
>>                  From: Ihor Matushchak <ihor.matushchak@foobox.net
>>         <mailto:ihor.matushchak@foobox.net>
>>                  <mailto:ihor.matushchak@foobox.net
>>         <mailto:ihor.matushchak@foobox.net>>>
>>
>>                  This patch enables earlyprintk for Rockchip rk3399
>>         based SoC.
>>
>>              Is there any missing pieces in Xen to boot on Rockchip? I
>>         would also
>>              quite like to see some documentation how to boot Xen on
>>         that platform.
>>
>>              Cheers,
>>
>>
>>
>>                  Ihor Matushchak (1):
>>                      xen:arm:earlyprintk configuration for rk3399 boards
>>
>>                     docs/misc/arm/early-printk.txt | 1 +
>>                     xen/arch/arm/Rules.mk          | 1 +
>>                     2 files changed, 2 insertions(+)
>>
>>
>>              --     Julien Grall
>>
>>
>>
>>     --     Julien Grall
>>
>>
>>
> --
> Julien Grall
>

[-- Attachment #1.2: Type: text/html, Size: 6497 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

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

^ permalink raw reply


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