[PATCH] scsi: storvsc: remove return at end of void function

2017-03-16 Thread Miguel Bernal Marin
storvsc_on_channel_callback is a void function and the return
statement at the end is not useful.

Found with checkpatch.

Signed-off-by: Miguel Bernal Marin 
---
 drivers/scsi/storvsc_drv.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 3d70d1cf49a3..538f3e131275 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -1191,8 +1191,6 @@ static void storvsc_on_channel_callback(void *context)
break;
}
} while (1);
-
-   return;
 }
 
 static int storvsc_connect_to_vsp(struct hv_device *device, u32 ring_size,
-- 
2.12.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [patch 4/4] staging: speakup: move those functions which do outgoing serial comms, into serialio.c

2017-03-16 Thread Greg Kroah-Hartman
On Thu, Mar 16, 2017 at 06:51:54AM +, Okash Khawaja wrote:
> On Thu, Mar 16, 2017 at 11:14:09AM +0900, Greg Kroah-Hartman wrote:
> > On Tue, Mar 14, 2017 at 01:41:55PM +, Okash Khawaja wrote:
> > > This moves spk_synth_immediate and spk_serial_synth_probe functions into
> > > serialio.c. These functions do outgoing serial comms. The move is a step
> > > towards collecting all serial comms in serialio.c. This also renames
> > > spk_synth_immediate to spk_serial_synth_immediate.
> > > 
> > > Code inside those functions has not been changed. Along the way, this 
> > > patch
> > > also fixes a couple of spots which were calling spk_synth_immediate 
> > > directly,
> > > so that the calls now happen via the spk_syth struct.
> > > 
> > > Signed-off-by: Okash Khawaja 
> > > Reviewed-by: Samuel Thibault 
> > > 
> > > Index: linux-4.11-rc2/drivers/staging/speakup/serialio.c
> > 
> > This patch doesn't apply to my tree at all.  The first 3 applied with
> > some fuzz, so I think you are not working against linux-next. Please
> > rebase your patch against linux-next, or my staging-testing branch, and
> > resend so I can apply it.
> 
> Sure, will rebase against staging-testing. Just this patch or the
> patch set?

I applied the first 3, so if you rebase, they should just "go away" :)

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v3 0/2] staging: checkpatch cleanups

2017-03-16 Thread Greg KH
On Thu, Mar 16, 2017 at 12:02:25PM +0530, Arushi Singhal wrote:
> 
> 
> On Tue, Mar 14, 2017 at 3:35 AM, Greg KH  wrote:
> 
> On Tue, Mar 14, 2017 at 01:49:52AM +0530, Arushi Singhal wrote:
> > Improve readability by fixing multiple checkpatch.pl
> > issues in drivers.
> >
> > Arushi Singhal (2):
> >   staging: speakup: identation should use tabs
> >   staging: dvb-frontends: removed code in comments.
> 
> What changed from v2?
> 
>  In this patch nothing is changed from v2 but it is a patch series so have to
> write v2 everywhere.

And everywhere you also need to say why it is v2, otherwise we have no
idea :(

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 1/2] staging: dgnc: make error codes uniform

2017-03-16 Thread Tobin C. Harding
On Thu, Mar 16, 2017 at 03:32:50PM +0900, Greg Kroah-Hartman wrote:
> On Thu, Mar 16, 2017 at 03:46:58PM +1100, Tobin C. Harding wrote:
> > On Thu, Mar 16, 2017 at 11:45:17AM +0900, Greg Kroah-Hartman wrote:
> > > On Wed, Mar 15, 2017 at 10:44:28AM +1100, Tobin C. Harding wrote:
> > > > Driver code is non-uniform in its use of error return codes, identical
> > > > failures are returning different error codes. Return is on failure
> > > > when checking struct magic numbers. Error codes used include -ENODEV,
> > > > -ENXIO, -EIO, and -EFAULT.
> > > > 
> > > > Use -ENXIO. Justification is that usual call includes a check that the
> > > > struct is non-NULL OR'd with check on the magic number.
> > > > 
> > > > #define ENXIO 6 /* No such device or address */
> > > > 
> > > > Seems like a good fit. Also this choice results in the minimum number
> > > > of files touched.
> > > > 
> > > > Pick one error code, ENXIO. Change all functions to use the same error
> > > > return code for the cases of failed magic number checks.
> > > > 
> > > > Signed-off-by: Tobin C. Harding 
> > > > ---
> > > >  drivers/staging/dgnc/dgnc_mgmt.c |  2 +-
> > > >  drivers/staging/dgnc/dgnc_tty.c  | 44 
> > > > 
> > > >  2 files changed, 23 insertions(+), 23 deletions(-)
> > > > 
> > > > diff --git a/drivers/staging/dgnc/dgnc_mgmt.c 
> > > > b/drivers/staging/dgnc/dgnc_mgmt.c
> > > > index ee8a626..f134222 100644
> > > > --- a/drivers/staging/dgnc/dgnc_mgmt.c
> > > > +++ b/drivers/staging/dgnc/dgnc_mgmt.c
> > > > @@ -187,7 +187,7 @@ long dgnc_mgmt_ioctl(struct file *file, unsigned 
> > > > int cmd, unsigned long arg)
> > > > ch = dgnc_board[board]->channels[channel];
> > > >  
> > > > if (!ch || ch->magic != DGNC_CHANNEL_MAGIC)
> > > > -   return -ENODEV;
> > > > +   return -ENXIO;
> > > 
> > > No, this really means the device is gone.  And the whole ->magic stuff
> > > needs to just be deleted, it's not needed at all.
> > 
> > Can I please confirm, all these magic number checks are useless?
> > 
> >  Module: drivers/staging/dgnc
> > 
> > if (!tty || tty->magic != TTY_MAGIC)
> > 
> 
> Checking for tty is fine, right?
> 
> > un = tty->driver_data;
> > if (!un || un->magic != DGNC_UNIT_MAGIC)
> 
> Any "magic" check should be removed, it's a _very_ old pattern (before
> Linux 1.0) that somehow was done because the developer was worried about
> memory corruption.  It's not an issue anymore.

Awesome, thanks for clarifying.

Tobin.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: Michael MIC

2017-03-16 Thread Wolfram Sang
Hi,

> Is there some technical reason why module ks7010 includes a
> implementation of Michael MIC instead of using the implementation in
> crypto/?
> 
> Or is this covered by the statement from the TODO
> 
> - check what other upstream wireless mechanisms can be used instead of the
>   custom ones here

Exactly. Furthermore, for this driver to ever go upstream, it needs to
be converted from WEXT to cfg80211. I didn't look super much into it,
but from what I grasped, the Michael conversion comes with the above
conversion "for free".

Regards,

   Wolfram



signature.asc
Description: PGP signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[patch 4/4] staging: speakup: move those functions which do outgoing serial comms, into serialio.c

2017-03-16 Thread Okash Khawaja
This moves spk_synth_immediate and spk_serial_synth_probe functions into
serialio.c. These functions do outgoing serial comms. The move is a step
towards collecting all serial comms in serialio.c. This also renames
spk_synth_immediate to spk_serial_synth_immediate.

Code inside those functions has not been changed. Along the way, this patch
also fixes a couple of spots which were calling spk_synth_immediate directly,
so that the calls now happen via the spk_syth struct.

Signed-off-by: Okash Khawaja 

Reviewed-by: Samuel Thibault 

Index: linux-staging/drivers/staging/speakup/serialio.c
===
--- linux-staging.orig/drivers/staging/speakup/serialio.c
+++ linux-staging/drivers/staging/speakup/serialio.c
@@ -136,6 +136,35 @@
outb(1, speakup_info.port_tts + UART_FCR);  /* Turn FIFO On */
 }
 
+int spk_serial_synth_probe(struct spk_synth *synth)
+{
+   const struct old_serial_port *ser;
+   int failed = 0;
+
+   if ((synth->ser >= SPK_LO_TTY) && (synth->ser <= SPK_HI_TTY)) {
+   ser = spk_serial_init(synth->ser);
+   if (!ser) {
+   failed = -1;
+   } else {
+   outb_p(0, ser->port);
+   mdelay(1);
+   outb_p('\r', ser->port);
+   }
+   } else {
+   failed = -1;
+   pr_warn("ttyS%i is an invalid port\n", synth->ser);
+   }
+   if (failed) {
+   pr_info("%s: not found\n", synth->long_name);
+   return -ENODEV;
+   }
+   pr_info("%s: ttyS%i, Driver Version %s\n",
+   synth->long_name, synth->ser, synth->version);
+   synth->alive = 1;
+   return 0;
+}
+EXPORT_SYMBOL_GPL(spk_serial_synth_probe);
+
 void spk_stop_serial_interrupt(void)
 {
if (speakup_info.port_tts == 0)
@@ -223,6 +252,23 @@
return 0;
 }
 
+const char *spk_serial_synth_immediate(struct spk_synth *synth, const char 
*buff)
+{
+   u_char ch;
+
+   while ((ch = *buff)) {
+   if (ch == '\n')
+   ch = synth->procspeech;
+   if (spk_wait_for_xmitr(synth))
+   outb(ch, speakup_info.port_tts);
+   else
+   return buff;
+   buff++;
+   }
+   return NULL;
+}
+EXPORT_SYMBOL_GPL(spk_serial_synth_immediate);
+
 void spk_serial_release(void)
 {
spk_stop_serial_interrupt();
Index: linux-staging/drivers/staging/speakup/speakup_acntsa.c
===
--- linux-staging.orig/drivers/staging/speakup/speakup_acntsa.c
+++ linux-staging/drivers/staging/speakup/speakup_acntsa.c
@@ -102,7 +102,7 @@
.io_ops = &spk_serial_io_ops,
.probe = synth_probe,
.release = spk_serial_release,
-   .synth_immediate = spk_synth_immediate,
+   .synth_immediate = spk_serial_synth_immediate,
.catch_up = spk_do_catch_up,
.flush = spk_synth_flush,
.is_alive = spk_synth_is_alive_restart,
@@ -127,7 +127,7 @@
 
failed = spk_serial_synth_probe(synth);
if (failed == 0) {
-   spk_synth_immediate(synth, "\033=R\r");
+   synth->synth_immediate(synth, "\033=R\r");
mdelay(100);
}
synth->alive = !failed;
Index: linux-staging/drivers/staging/speakup/speakup_apollo.c
===
--- linux-staging.orig/drivers/staging/speakup/speakup_apollo.c
+++ linux-staging/drivers/staging/speakup/speakup_apollo.c
@@ -111,7 +111,7 @@
.io_ops = &spk_serial_io_ops,
.probe = spk_serial_synth_probe,
.release = spk_serial_release,
-   .synth_immediate = spk_synth_immediate,
+   .synth_immediate = spk_serial_synth_immediate,
.catch_up = do_catch_up,
.flush = spk_synth_flush,
.is_alive = spk_synth_is_alive_restart,
Index: linux-staging/drivers/staging/speakup/speakup_audptr.c
===
--- linux-staging.orig/drivers/staging/speakup/speakup_audptr.c
+++ linux-staging/drivers/staging/speakup/speakup_audptr.c
@@ -107,7 +107,7 @@
.io_ops = &spk_serial_io_ops,
.probe = synth_probe,
.release = spk_serial_release,
-   .synth_immediate = spk_synth_immediate,
+   .synth_immediate = spk_serial_synth_immediate,
.catch_up = spk_do_catch_up,
.flush = synth_flush,
.is_alive = spk_synth_is_alive_restart,
@@ -144,7 +144,7 @@
unsigned char test = 0;
char synth_id[40] = "";
 
-   spk_synth_immediate(synth, "\x05[Q]");
+   synth->synth_immediate(synth, "\x05[Q]");
synth_id[test] = spk_serial_in();
if (synth_id[test] == 'A') {
do {
Index: linux-staging/drivers/staging/speakup/speakup_bns.c
===

Re: [PATCH] staging: android: ion: reduce lock contention latency

2017-03-16 Thread junil0814....@lge.com

On 03/15/2017 2:10 AM, Laura Abbott wrote:
> On 03/14/2017 12:51 AM, Junil Lee wrote:
>> Replace list into lock-less list of ion page pool.
>>
>> Measure how mutex lock contention latency on android.
>>
>> 1. the test is done under android 7.0
>> 2. startup many applications circularly
>> 3. find sample in trace log as below
>>
>> cameraserver-625 [004] ...1 1891.952958: mutex_lock_enter: id=0
>> Binder:384_2-417 [005] ...1 1891.952958: mutex_lock_enter: id=0
>> Binder:384_2-417 [005] ...1 1891.952966: mutex_lock_enter: id=1
>> Binder:384_2-417 [005] ...1 1891.952970: mutex_lock_enter: id=0
>> Binder:384_2-417 [005] ...1 1891.952971: mutex_lock_enter: id=1
>> Binder:384_2-417 [005] ...1 1891.952982: mutex_lock_enter: id=0
>> Binder:384_2-417 [005] ...1 1891.952983: mutex_lock_enter: id=1
>> Binder:384_2-417 [005] ...1 1891.952989: mutex_lock_enter: id=0
>> Binder:384_2-417 [005] ...1 1891.952989: mutex_lock_enter: id=1
>> Binder:384_2-417 [005] ...1 1891.952995: mutex_lock_enter: id=0
>> cameraserver-625 [004] ...1 1891.952995: mutex_lock_enter: id=1
>>
>> - id 0 is try to lock, id 1 is locked
>>
>> Figure out how many latency reduction by this patch as below.
>>
>> The test is startup 60 applications circularly (repeat 10cycles)
>> - lock contention count : 3717 -> 93
>>
> 
> We really need to finish up the work to move Ion out of staging before
> looking at performance improvements so please help with that discussion.
> Once that is finished up, we can look at performance improvements again.
> 
> That said, this is removing the lock from the free path. Is the
> contention happening on the free path? Is the system heap deferring
> frees? Does this have a notable affect on anything besides the count
> of contention? None of this is necessarily a deal breaker but I'd
> like a few more details.
> 
> Thanks,
> Laura

Appreciate your comments Laura,

At first, my target uses system heap deferred free.

The background of making this patch is that I tried to shrink ion pool
much at specific time. (e.g. shrink 40MB ion pool instead of lmk kill)
In above test, the problem ion allocation took very long times was
happened and I found it occurred by lock contention.
So, I have tried to reduce contention and use lock-less list as my patch.

As my first email, contention cost is too small about 27usec at once
(the cameraserver waiting time between tries to lock and get lock).
It's too hard to verify meaningful improvement using waiting time.

Therefore, I just add some code as below to check how many contentions
are happened.

static int ion_page_pool_add(struct ion_page_pool *pool, struct page *page)
{
+   if (mutex_is_locked(&pool->mutex))
+   atomic_inc(&mutex_add_lock);
+
mutex_lock(&pool->mutex);
if (PageHighMem(page)) {
list_add_tail(&page->lru, &pool->high_items);
@@ -101,6 +106,9 @@ struct page *ion_page_pool_alloc(struct
ion_page_pool *pool)

BUG_ON(!pool);

+   if (mutex_is_locked(&pool->mutex))
+   atomic_inc(&mutex_remove_lock);
+
trace_mutex_lock_enter(0);
mutex_lock(&pool->mutex);
trace_mutex_lock_enter(1);
@@ -177,6 +185,9 @@ int ion_page_pool_shrink(struct ion_page_pool *pool,
gfp_t gfp_mask,
for (freed = 0; freed < nr_to_scan; freed++) {
struct page *page;

+   if (mutex_is_locked(&pool->mutex))
+   atomic_inc(&mutex_remove_lock);
+

Then, I found the count of contention.
Before test, add_lock_cnt : 2519, remove_lock: 1202
After, add_lock_cnt: N/A, remove_lock: 93
(The patched ion don't try lock in free path, I ignore add_lock_cnt.)

Thanks,
Junil Lee

> 
>> Signed-off-by: Bongkyu Kim 
>> Signed-off-by: Junil Lee 
>> ---
>> drivers/staging/android/ion/ion_page_pool.c | 52
> ++-
>> drivers/staging/android/ion/ion_priv.h | 8 ++---
>> drivers/staging/android/ion/ion_system_heap.c | 16 -
>> 3 files changed, 40 insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/staging/android/ion/ion_page_pool.c
> b/drivers/staging/android/ion/ion_page_pool.c
>> index aea89c1..1beb2c8 100644
>> --- a/drivers/staging/android/ion/ion_page_pool.c
>> +++ b/drivers/staging/android/ion/ion_page_pool.c
>> @@ -22,6 +22,7 @@
>> #include 
>> #include 
>> #include 
>> +#include 
>> #include "ion_priv.h"
>>
>> static void *ion_page_pool_alloc_pages(struct ion_page_pool *pool)
>> @@ -44,33 +45,36 @@ static void ion_page_pool_free_pages(struct
> ion_page_pool *pool,
>>
>> static int ion_page_pool_add(struct ion_page_pool *pool, struct page
> *page)
>> {
>> - mutex_lock(&pool->mutex);
>> if (PageHighMem(page)) {
>> - list_add_tail(&page->lru, &pool->high_items);
>> - pool->high_count++;
>> + llist_add((struct llist_node *)&page->lru, &pool->high_items);
>> + atomic_inc(&pool->high_count);
>> } else {
>> - list_add_tail(&page->lru, &pool->low_items);
>> - pool->low_count++;
>> + llist_add((struct llist_node *)&page->lru, &pool->low_items);
>> + atomi

Re: [patch 0/7] staging: speakup: introduce tty-based comms

2017-03-16 Thread Samuel Thibault
Hello,

Also, could serdev also provide an send_xchar function similar to the
send_xchar tty driver function?  We need this to send high-priority
characters.

Samuel
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: media / v4l2-mc: wishlist for complex cameras (was Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline)

2017-03-16 Thread Philipp Zabel
On Wed, 2017-03-15 at 14:55 -0400, Nicolas Dufresne wrote:
> Le mercredi 15 mars 2017 à 11:50 +0100, Philippe De Muyter a écrit :
> > > I would say: camorama, xawtv3, zbar, google talk, skype. If it runs
> > > with those, it will likely run with any other application.
> > > 
> > 
> > I would like to add the 'v4l2src' plugin of gstreamer, and on the
> > imx6 its
> 
> While it would be nice if somehow you would get v4l2src to work (in
> some legacy/emulation mode through libv4l2),

v4l2src works just fine, provided the pipeline is configured manually in
advance via media-ctl.

>  the longer plan is to
> implement smart bin that handle several v4l2src, that can do the
> required interactions so we can expose similar level of controls as
> found in Android Camera HAL3, and maybe even further assuming userspace
> can change the media tree at run-time. We might be a long way from
> there, specially that some of the features depends on how much the
> hardware can do. Just being able to figure-out how to build the MC tree
> dynamically seems really hard when thinking of generic mechanism. Also,
> Request API will be needed.
> 
> I think for this one, we'll need some userspace driver that enable the
> features (not hide them), and that's what I'd be looking for from
> libv4l2 in this regard.

regards
Philipp

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 0/6] staging: BCM2835 MMAL V4L2 camera driver

2017-03-16 Thread Mauro Carvalho Chehab
Em Wed, 15 Mar 2017 18:46:24 -0700
Michael Zoran  escreveu:

> On Wed, 2017-03-15 at 22:08 -0300, Mauro Carvalho Chehab wrote:
> 
> > No, I didn't. Thanks! Applied it but, unfortunately, didn't work.
> > Perhaps I'm missing some other patch. I'm compiling it from
> > the Greg's staging tree (branch staging-next):
> > https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.
> > git/log/?h=staging-next
> > 
> > Btw, as I'm running Raspbian, and didn't want to use compat32 bits, 
> > I'm compiling the Kernel as an arm32 bits Kernel.
> > 
> > I did a small trick to build the DTB on arm32:
> > 
> > ln -sf ../../../arm64/boot/dts/broadcom/bcm2837-rpi-3-b.dts
> > arch/arm/boot/dts/bcm2837-rpi-3-b.dts
> > ln -sf ../../../arm64/boot/dts/broadcom/bcm2837.dtsi
> > arch/arm/boot/dts/bcm2837.dtsi
> > git checkout arch/arm/boot/dts/Makefile
> > sed "s,bcm2835-rpi-zero.dtb,bcm2835-rpi-zero.dtb bcm2837-rpi-3-
> > b.dtb," a && mv a arch/arm/boot/dts/Makefile
> >   
> 
> Two other hacks are currently needed to get the camera to work:
> 
> 1. Add this to config.txt(This required to get the firmware to detect
> the camera)
> 
> start_x=1
> gpu_mem=128

I had this already.

> 
> 2. VC4 is incompatible with the firmware at this time, so you need 
> to presently munge the build configuration. What you do is leave
> simplefb in the build config(I'm assuming you already have that), but
> you will need to remove VC4 from the config.
> 
> The firmware currently adds a node for a simplefb for debugging
> purposes to show the boot log.  Surprisingly, this is still good enough
> for basic usage and testing.  

That solved the issue. Thanks! It would be good to add a notice
about that at the TODO, not let it build if DRM_VC4.

Please consider applying the enclosed path.

> The only remaining issue is that since simplefb is intented for
> debugging, you wan't be able to use many of the RPI specific
> applications.  
> 
> I've been using cheese and ffmpeg to test the camera which are not RPI
> specific.

I did a quick test with camorama and qv4l2. it worked with both.

Thanks,
Mauro


[PATCH] staging: bcm2835-camera: make it dependent of !DRM_VC4

Currently, if DRM_VC4 is enabled, this driver doesn't work,
as the firmware doesn't support having both enabled at the
same time.

Document that and prevent it to be built if !DRM_VC4.

Signed-off-by: Mauro Carvalho Chehab 

diff --git a/drivers/staging/vc04_services/bcm2835-camera/Kconfig 
b/drivers/staging/vc04_services/bcm2835-camera/Kconfig
index b8b01aa4e426..678dc2efb91a 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/Kconfig
+++ b/drivers/staging/vc04_services/bcm2835-camera/Kconfig
@@ -2,6 +2,8 @@ config VIDEO_BCM2835
tristate "BCM2835 Camera"
depends on MEDIA_SUPPORT
depends on VIDEO_V4L2 && (ARCH_BCM2835 || COMPILE_TEST)
+   # Currently, firmware is incompatible with VC4 DRM driver
+   depends on !DRM_VC4
select BCM2835_VCHIQ
select VIDEOBUF2_VMALLOC
select BTREE
diff --git a/drivers/staging/vc04_services/bcm2835-camera/TODO 
b/drivers/staging/vc04_services/bcm2835-camera/TODO
index 61a509992b9a..fec70a1cc4a3 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/TODO
+++ b/drivers/staging/vc04_services/bcm2835-camera/TODO
@@ -37,3 +37,4 @@ v4l2 module after VCHI loads.
 This was a temporary workaround for a bug that was fixed mid-2014, and
 we should remove it before stabilizing the driver.
 
+6) Make firmware compatible with both DRM_VC4 and VIDEO_BCM2835.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: media / v4l2-mc: wishlist for complex cameras (was Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline)

2017-03-16 Thread Philippe De Muyter
On Thu, Mar 16, 2017 at 10:26:00AM +0100, Philipp Zabel wrote:
> On Wed, 2017-03-15 at 14:55 -0400, Nicolas Dufresne wrote:
> > Le mercredi 15 mars 2017 à 11:50 +0100, Philippe De Muyter a écrit :
> > > > I would say: camorama, xawtv3, zbar, google talk, skype. If it runs
> > > > with those, it will likely run with any other application.
> > > > 
> > > 
> > > I would like to add the 'v4l2src' plugin of gstreamer, and on the
> > > imx6 its
> > 
> > While it would be nice if somehow you would get v4l2src to work (in
> > some legacy/emulation mode through libv4l2),
> 
> v4l2src works just fine, provided the pipeline is configured manually in
> advance via media-ctl.

Including choosing the framerate ?  Sorry, I have no time these days
to test it myself.

And I cited imxv4l2videosrc for its ability to provide the physical address
of the image buffers for further processing by other (not necessarily next
in gstreamer pipeline, or for all frames) hardware-accelerated plugins likes
the h.264 video encoder.  As I am stuck with fsl/nxp kernel and driver on that
matter, I don't know how the interfaces have evolved in current linux kernels.

BR

Philippe

-- 
Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: media / v4l2-mc: wishlist for complex cameras (was Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline)

2017-03-16 Thread Philipp Zabel
On Thu, 2017-03-16 at 10:47 +0100, Philippe De Muyter wrote:
> On Thu, Mar 16, 2017 at 10:26:00AM +0100, Philipp Zabel wrote:
> > On Wed, 2017-03-15 at 14:55 -0400, Nicolas Dufresne wrote:
> > > Le mercredi 15 mars 2017 à 11:50 +0100, Philippe De Muyter a écrit :
> > > > > I would say: camorama, xawtv3, zbar, google talk, skype. If it runs
> > > > > with those, it will likely run with any other application.
> > > > > 
> > > > 
> > > > I would like to add the 'v4l2src' plugin of gstreamer, and on the
> > > > imx6 its
> > > 
> > > While it would be nice if somehow you would get v4l2src to work (in
> > > some legacy/emulation mode through libv4l2),
> > 
> > v4l2src works just fine, provided the pipeline is configured manually in
> > advance via media-ctl.
> 
> Including choosing the framerate ?  Sorry, I have no time these days
> to test it myself.

No, the framerate is set with media-ctl on the CSI output pad. To really
choose the framerate, the element would indeed need a deeper
understanding of the pipeline, as the resulting framerate depends on at
least the source v4l2_subdevice (sensor) framerate and the CSI frame
skipping.

> And I cited imxv4l2videosrc for its ability to provide the physical address
> of the image buffers for further processing by other (not necessarily next
> in gstreamer pipeline, or for all frames) hardware-accelerated plugins likes
> the h.264 video encoder.  As I am stuck with fsl/nxp kernel and driver on that
> matter, I don't know how the interfaces have evolved in current linux kernels.

The physical address of the image buffers is hidden from userspace by
dma-buf objects, but those can be passed around to the next driver
without copying the image data.

regards
Philipp

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [RFC PATCH v2 12/32] x86: Add early boot support when running with SEV active

2017-03-16 Thread Borislav Petkov
On Fri, Mar 10, 2017 at 10:35:30AM -0600, Brijesh Singh wrote:
> We could update this patch to use the below logic:
> 
>  * CPUID(0) - Check for AuthenticAMD
>  * CPID(1) - Check if under hypervisor
>  * CPUID(0x8000) - Check for highest supported leaf
>  * CPUID(0x801F).EAX - Check for SME and SEV support
>  * rdmsr (MSR_K8_SYSCFG)[MemEncryptionModeEnc] - Check if SMEE is set

Actually, it is still not clear to me *why* we need to do anything
special wrt SEV in the guest.

Lemme clarify: why can't the guest boot just like a normal Linux on
baremetal and use the SME(!) detection code to set sme_enable and so
on? IOW, I'd like to avoid all those checks whether we're running under
hypervisor and handle all that like we're running on baremetal.

-- 
Regards/Gruss,
Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 
(AG Nürnberg)
-- 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: media / v4l2-mc: wishlist for complex cameras (was Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline)

2017-03-16 Thread Philippe De Muyter
On Thu, Mar 16, 2017 at 11:01:56AM +0100, Philipp Zabel wrote:
> On Thu, 2017-03-16 at 10:47 +0100, Philippe De Muyter wrote:
> > On Thu, Mar 16, 2017 at 10:26:00AM +0100, Philipp Zabel wrote:
> > > On Wed, 2017-03-15 at 14:55 -0400, Nicolas Dufresne wrote:
> > > > Le mercredi 15 mars 2017 à 11:50 +0100, Philippe De Muyter a écrit :
> > > > > > I would say: camorama, xawtv3, zbar, google talk, skype. If it runs
> > > > > > with those, it will likely run with any other application.
> > > > > > 
> > > > > 
> > > > > I would like to add the 'v4l2src' plugin of gstreamer, and on the
> > > > > imx6 its
> > > > 
> > > > While it would be nice if somehow you would get v4l2src to work (in
> > > > some legacy/emulation mode through libv4l2),
> > > 
> > > v4l2src works just fine, provided the pipeline is configured manually in
> > > advance via media-ctl.
> > 
> > Including choosing the framerate ?  Sorry, I have no time these days
> > to test it myself.
> 
> No, the framerate is set with media-ctl on the CSI output pad. To really
> choose the framerate, the element would indeed need a deeper
> understanding of the pipeline, as the resulting framerate depends on at
> least the source v4l2_subdevice (sensor) framerate and the CSI frame
> skipping.

Count me in than as a supporter of Steve's "v4l2-mc: add a function to
inherit controls from a pipeline" patch

> > of the image buffers for further processing by other (not necessarily next
> > in gstreamer pipeline, or for all frames) hardware-accelerated plugins likes
> > the h.264 video encoder.  As I am stuck with fsl/nxp kernel and driver on 
> > that
> > matter, I don't know how the interfaces have evolved in current linux 
> > kernels.
> 
> The physical address of the image buffers is hidden from userspace by
> dma-buf objects, but those can be passed around to the next driver
> without copying the image data.

OK

thanks

Philippe
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [RFC PATCH v2 23/32] kvm: introduce KVM_MEMORY_ENCRYPT_OP ioctl

2017-03-16 Thread Paolo Bonzini


On 02/03/2017 16:17, Brijesh Singh wrote:
> If hardware supports encrypting then KVM_MEMORY_ENCRYPT_OP ioctl can
> be used by qemu to issue platform specific memory encryption commands.
> 
> Signed-off-by: Brijesh Singh 
> ---
>  arch/x86/include/asm/kvm_host.h |2 ++
>  arch/x86/kvm/x86.c  |   12 
>  include/uapi/linux/kvm.h|2 ++
>  3 files changed, 16 insertions(+)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index bff1f15..62651ad 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1033,6 +1033,8 @@ struct kvm_x86_ops {
>   void (*cancel_hv_timer)(struct kvm_vcpu *vcpu);
>  
>   void (*setup_mce)(struct kvm_vcpu *vcpu);
> +
> + int (*memory_encryption_op)(struct kvm *kvm, void __user *argp);
>  };
>  
>  struct kvm_arch_async_pf {
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 2099df8..6a737e9 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -3926,6 +3926,14 @@ static int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
>   return r;
>  }
>  
> +static int kvm_vm_ioctl_memory_encryption_op(struct kvm *kvm, void __user 
> *argp)
> +{
> + if (kvm_x86_ops->memory_encryption_op)
> + return kvm_x86_ops->memory_encryption_op(kvm, argp);
> +
> + return -ENOTTY;
> +}
> +
>  long kvm_arch_vm_ioctl(struct file *filp,
>  unsigned int ioctl, unsigned long arg)
>  {
> @@ -4189,6 +4197,10 @@ long kvm_arch_vm_ioctl(struct file *filp,
>   r = kvm_vm_ioctl_enable_cap(kvm, &cap);
>   break;
>   }
> + case KVM_MEMORY_ENCRYPT_OP: {
> + r = kvm_vm_ioctl_memory_encryption_op(kvm, argp);
> + break;
> + }
>   default:
>   r = kvm_vm_ioctl_assigned_device(kvm, ioctl, arg);
>   }
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index cac48ed..fef7d83 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -1281,6 +1281,8 @@ struct kvm_s390_ucas_mapping {
>  #define KVM_S390_GET_IRQ_STATE _IOW(KVMIO, 0xb6, struct 
> kvm_s390_irq_state)
>  /* Available with KVM_CAP_X86_SMM */
>  #define KVM_SMI   _IO(KVMIO,   0xb7)
> +/* Memory Encryption Commands */
> +#define KVM_MEMORY_ENCRYPT_OP  _IOWR(KVMIO, 0xb8, unsigned long)
>  
>  #define KVM_DEV_ASSIGN_ENABLE_IOMMU  (1 << 0)
>  #define KVM_DEV_ASSIGN_PCI_2_3   (1 << 1)
> 

Reviewed-by: Paolo Bonzini 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [RFC PATCH v2 24/32] kvm: x86: prepare for SEV guest management API support

2017-03-16 Thread Paolo Bonzini


On 02/03/2017 16:17, Brijesh Singh wrote:
> ASID management:
>  - Reserve asid range for SEV guest, SEV asid range is obtained through
>CPUID Fn8000_001f[ECX]. A non-SEV guest can use any asid outside the SEV
>asid range.

How is backwards compatibility handled?

>  - SEV guest must have asid value within asid range obtained through CPUID.
>  - SEV guest must have the same asid for all vcpu's. A TLB flush is required
>if different vcpu for the same ASID is to be run on the same host CPU.

[...]

> +
> + /* which host cpu was used for running this vcpu */
> + bool last_cpuid;

Should be unsigned int.

> 
> + /* Assign the asid allocated for this SEV guest */
> + svm->vmcb->control.asid = asid;
> +
> + /* Flush guest TLB:
> +  * - when different VMCB for the same ASID is to be run on the
> +  *   same host CPU
> +  *   or
> +  * - this VMCB was executed on different host cpu in previous VMRUNs.
> +  */
> + if (sd->sev_vmcbs[asid] != (void *)svm->vmcb ||

Why the cast?

> + svm->last_cpuid != cpu)
> + svm->vmcb->control.tlb_ctl = TLB_CONTROL_FLUSH_ALL_ASID;

If there is a match, you don't need to do anything else (neither reset
the asid, nor mark it as dirty, nor update the fields), so:

if (sd->sev_vmcbs[asid] == svm->vmcb &&
svm->last_cpuid == cpu)
return;

svm->last_cpuid = cpu;
sd->sev_vmcbs[asid] = svm->vmcb;
svm->vmcb->control.tlb_ctl = TLB_CONTROL_FLUSH_ALL_ASID;
svm->vmcb->control.asid = asid;
mark_dirty(svm->vmcb, VMCB_ASID);

(plus comments ;)).

Also, why not TLB_CONTROL_FLUSH_ASID if possible?

> + svm->last_cpuid = cpu;
> + sd->sev_vmcbs[asid] = (void *)svm->vmcb;
> +
> + mark_dirty(svm->vmcb, VMCB_ASID);

[...]

> 
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index fef7d83..9df37a2 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -1284,6 +1284,104 @@ struct kvm_s390_ucas_mapping {
>  /* Memory Encryption Commands */
>  #define KVM_MEMORY_ENCRYPT_OP  _IOWR(KVMIO, 0xb8, unsigned long)
>  
> +/* Secure Encrypted Virtualization mode */
> +enum sev_cmd_id {

Please add documentation in Documentation/virtual/kvm/memory_encrypt.txt.

Paolo
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [RFC PATCH v2 32/32] x86: kvm: Pin the guest memory when SEV is active

2017-03-16 Thread Paolo Bonzini


On 02/03/2017 16:18, Brijesh Singh wrote:
> The SEV memory encryption engine uses a tweak such that two identical
> plaintexts at different location will have a different ciphertexts.
> So swapping or moving ciphertexts of two pages will not result in
> plaintexts being swapped. Relocating (or migrating) a physical backing pages
> for SEV guest will require some additional steps. The current SEV key
> management spec [1] does not provide commands to swap or migrate (move)
> ciphertexts. For now we pin the memory allocated for the SEV guest. In
> future when SEV key management spec provides the commands to support the
> page migration we can update the KVM code to remove the pinning logical
> without making any changes into userspace (qemu).
> 
> The patch pins userspace memory when a new slot is created and unpin the
> memory when slot is removed.
> 
> [1] http://support.amd.com/TechDocs/55766_SEV-KM%20API_Spec.pdf

This is not enough, because memory can be hidden temporarily from the
guest and remapped later.  Think of a PCI BAR that is backed by RAM, or
also SMRAM.  The pinning must be kept even in that case.

You need to add a pair of KVM_MEMORY_ENCRYPT_OPs (one that doesn't map
to a PSP operation), such as KVM_REGISTER/UNREGISTER_ENCRYPTED_RAM.  In
QEMU you can use a RAMBlockNotifier to invoke the ioctls.

Paolo

> Signed-off-by: Brijesh Singh 
> ---
>  arch/x86/include/asm/kvm_host.h |6 +++
>  arch/x86/kvm/svm.c  |   93 
> +++
>  arch/x86/kvm/x86.c  |3 +
>  3 files changed, 102 insertions(+)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index fcc4710..9dc59f0 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -723,6 +723,7 @@ struct kvm_sev_info {
>   unsigned int handle;/* firmware handle */
>   unsigned int asid;  /* asid for this guest */
>   int sev_fd; /* SEV device fd */
> + struct list_head pinned_memory_slot;
>  };
>  
>  struct kvm_arch {
> @@ -1043,6 +1044,11 @@ struct kvm_x86_ops {
>   void (*setup_mce)(struct kvm_vcpu *vcpu);
>  
>   int (*memory_encryption_op)(struct kvm *kvm, void __user *argp);
> +
> + void (*prepare_memory_region)(struct kvm *kvm,
> + struct kvm_memory_slot *memslot,
> + const struct kvm_userspace_memory_region *mem,
> + enum kvm_mr_change change);
>  };
>  
>  struct kvm_arch_async_pf {
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 13996d6..ab973f9 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -498,12 +498,21 @@ static inline bool gif_set(struct vcpu_svm *svm)
>  }
>  
>  /* Secure Encrypted Virtualization */
> +struct kvm_sev_pinned_memory_slot {
> + struct list_head list;
> + unsigned long npages;
> + struct page **pages;
> + unsigned long userspace_addr;
> + short id;
> +};
> +
>  static unsigned int max_sev_asid;
>  static unsigned long *sev_asid_bitmap;
>  static void sev_deactivate_handle(struct kvm *kvm);
>  static void sev_decommission_handle(struct kvm *kvm);
>  static int sev_asid_new(void);
>  static void sev_asid_free(int asid);
> +static void sev_unpin_memory(struct page **pages, unsigned long npages);
>  #define __sev_page_pa(x) ((page_to_pfn(x) << PAGE_SHIFT) | sme_me_mask)
>  
>  static bool kvm_sev_enabled(void)
> @@ -1544,9 +1553,25 @@ static inline int avic_free_vm_id(int id)
>  
>  static void sev_vm_destroy(struct kvm *kvm)
>  {
> + struct list_head *pos, *q;
> + struct kvm_sev_pinned_memory_slot *pinned_slot;
> + struct list_head *head = &kvm->arch.sev_info.pinned_memory_slot;
> +
>   if (!sev_guest(kvm))
>   return;
>  
> + /* if guest memory is pinned then unpin it now */
> + if (!list_empty(head)) {
> + list_for_each_safe(pos, q, head) {
> + pinned_slot = list_entry(pos,
> + struct kvm_sev_pinned_memory_slot, list);
> + sev_unpin_memory(pinned_slot->pages,
> + pinned_slot->npages);
> + list_del(pos);
> + kfree(pinned_slot);
> + }
> + }
> +
>   /* release the firmware resources */
>   sev_deactivate_handle(kvm);
>   sev_decommission_handle(kvm);
> @@ -5663,6 +5688,8 @@ static int sev_pre_start(struct kvm *kvm, int *asid)
>   }
>   *asid = ret;
>   ret = 0;
> +
> + INIT_LIST_HEAD(&kvm->arch.sev_info.pinned_memory_slot);
>   }
>  
>   return ret;
> @@ -6189,6 +6216,71 @@ static int sev_launch_measure(struct kvm *kvm, struct 
> kvm_sev_cmd *argp)
>   return ret;
>  }
>  
> +static struct kvm_sev_pinned_memory_slot *sev_find_pinned_memory_slot(
> + struct kvm *kvm, struct kvm_memory_slot *slot)
> +{
> + struct kvm_sev_pinned_memory_slot *i;
> +

Re: [RFC PATCH v2 26/32] kvm: svm: Add support for SEV LAUNCH_UPDATE_DATA command

2017-03-16 Thread Paolo Bonzini


On 02/03/2017 16:17, Brijesh Singh wrote:
> +static struct page **sev_pin_memory(unsigned long uaddr, unsigned long ulen,
> + unsigned long *n)
> +{
> + struct page **pages;
> + int first, last;
> + unsigned long npages, pinned;
> +
> + /* Get number of pages */
> + first = (uaddr & PAGE_MASK) >> PAGE_SHIFT;
> + last = ((uaddr + ulen - 1) & PAGE_MASK) >> PAGE_SHIFT;
> + npages = (last - first + 1);
> +
> + pages = kzalloc(npages * sizeof(struct page *), GFP_KERNEL);
> + if (!pages)
> + return NULL;
> +
> + /* pin the user virtual address */
> + down_read(¤t->mm->mmap_sem);
> + pinned = get_user_pages_fast(uaddr, npages, 1, pages);
> + up_read(¤t->mm->mmap_sem);

get_user_pages_fast, like get_user_pages_unlocked, must be called
without mmap_sem held.

> + if (pinned != npages) {
> + printk(KERN_ERR "SEV: failed to pin  %ld pages (got %ld)\n",
> + npages, pinned);
> + goto err;
> + }
> +
> + *n = npages;
> + return pages;
> +err:
> + if (pinned > 0)
> + release_pages(pages, pinned, 0);
> + kfree(pages);
> +
> + return NULL;
> +}
>
> + /* the array of pages returned by get_user_pages() is a page-aligned
> +  * memory. Since the user buffer is probably not page-aligned, we need
> +  * to calculate the offset within a page for first update entry.
> +  */
> + offset = uaddr & (PAGE_SIZE - 1);
> + len = min_t(size_t, (PAGE_SIZE - offset), ulen);
> + ulen -= len;
> +
> + /* update first page -
> +  * special care need to be taken for the first page because we might
> +  * be dealing with offset within the page
> +  */

No need to special case the first page; just set "offset = 0" inside the
loop after the first iteration.

Paolo

> + data->handle = sev_get_handle(kvm);
> + data->length = len;
> + data->address = __sev_page_pa(inpages[0]) + offset;
> + ret = sev_issue_cmd(kvm, SEV_CMD_LAUNCH_UPDATE_DATA,
> + data, &argp->error);
> + if (ret)
> + goto err_3;
> +
> + /* update remaining pages */
> + for (i = 1; i < nr_pages; i++) {
> +
> + len = min_t(size_t, PAGE_SIZE, ulen);
> + ulen -= len;
> + data->length = len;
> + data->address = __sev_page_pa(inpages[i]);
> + ret = sev_issue_cmd(kvm, SEV_CMD_LAUNCH_UPDATE_DATA,
> + data, &argp->error);
> + if (ret)
> + goto err_3;
> + }

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [RFC PATCH v2 29/32] kvm: svm: Add support for SEV DEBUG_DECRYPT command

2017-03-16 Thread Paolo Bonzini


On 02/03/2017 16:18, Brijesh Singh wrote:
> +static int __sev_dbg_decrypt_page(struct kvm *kvm, unsigned long src,
> + void *dst, int *error)
> +{
> + inpages = sev_pin_memory(src, PAGE_SIZE, &npages);
> + if (!inpages) {
> + ret = -ENOMEM;
> + goto err_1;
> + }
> +
> + data->handle = sev_get_handle(kvm);
> + data->dst_addr = __psp_pa(dst);
> + data->src_addr = __sev_page_pa(inpages[0]);
> + data->length = PAGE_SIZE;
> +
> + ret = sev_issue_cmd(kvm, SEV_CMD_DBG_DECRYPT, data, error);
> + if (ret)
> + printk(KERN_ERR "SEV: DEBUG_DECRYPT %d (%#010x)\n",
> + ret, *error);
> + sev_unpin_memory(inpages, npages);
> +err_1:
> + kfree(data);
> + return ret;
> +}
> +
> +static int sev_dbg_decrypt(struct kvm *kvm, struct kvm_sev_cmd *argp)
> +{
> + void *data;
> + int ret, offset, len;
> + struct kvm_sev_dbg debug;
> +
> + if (!sev_guest(kvm))
> + return -ENOTTY;
> +
> + if (copy_from_user(&debug, (void *)argp->data,
> + sizeof(struct kvm_sev_dbg)))
> + return -EFAULT;
> + /*
> +  * TODO: add support for decrypting length which crosses the
> +  * page boundary.
> +  */
> + offset = debug.src_addr & (PAGE_SIZE - 1);
> + if (offset + debug.length > PAGE_SIZE)
> + return -EINVAL;
> +

Please do add it, it doesn't seem very different from what you're doing
in LAUNCH_UPDATE_DATA.  There's no need for a separate
__sev_dbg_decrypt_page function, you can just pin/unpin here and do a
per-page loop as in LAUNCH_UPDATE_DATA.

Paolo
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [RFC PATCH v2 30/32] kvm: svm: Add support for SEV DEBUG_ENCRYPT command

2017-03-16 Thread Paolo Bonzini


On 02/03/2017 16:18, Brijesh Singh wrote:
> + data = (void *) get_zeroed_page(GFP_KERNEL);

The page does not need to be zeroed, does it?

> +
> + if ((len & 15) || (dst_addr & 15)) {
> + /* if destination address and length are not 16-byte
> +  * aligned then:
> +  * a) decrypt destination page into temporary buffer
> +  * b) copy source data into temporary buffer at correct offset
> +  * c) encrypt temporary buffer
> +  */
> + ret = __sev_dbg_decrypt_page(kvm, dst_addr, data, &argp->error);

Ah, I see now you're using this function here for read-modify-write.
data is already pinned here, so even if you keep the function it makes
sense to push pinning out of __sev_dbg_decrypt_page and into
sev_dbg_decrypt.

> + if (ret)
> + goto err_3;
> + d_off = dst_addr & (PAGE_SIZE - 1);
> +
> + if (copy_from_user(data + d_off,
> + (uint8_t *)debug.src_addr, len)) {
> + ret = -EFAULT;
> + goto err_3;
> + }
> +
> + encrypt->length = PAGE_SIZE;

Why decrypt/re-encrypt all the page instead of just the 16 byte area
around the [dst_addr, dst_addr+len) range?

> + encrypt->src_addr = __psp_pa(data);
> + encrypt->dst_addr =  __sev_page_pa(inpages[0]);
> + } else {
> + if (copy_from_user(data, (uint8_t *)debug.src_addr, len)) {
> + ret = -EFAULT;
> + goto err_3;
> + }

Do you need copy_from_user, or can you just pin/unpin memory as for
DEBUG_DECRYPT?

Paolo

> + d_off = dst_addr & (PAGE_SIZE - 1);
> + encrypt->length = len;
> + encrypt->src_addr = __psp_pa(data);
> + encrypt->dst_addr = __sev_page_pa(inpages[0]);
> + encrypt->dst_addr += d_off;
> + }
> +
> + encrypt->handle = sev_get_handle(kvm);
> + ret = sev_issue_cmd(kvm, SEV_CMD_DBG_ENCRYPT, encrypt, &argp->error);
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [RFC PATCH v2 16/32] x86: kvm: Provide support to create Guest and HV shared per-CPU variables

2017-03-16 Thread Paolo Bonzini


On 02/03/2017 16:15, Brijesh Singh wrote:
> Some KVM specific MSR's (steal-time, asyncpf, avic_eio) allocates per-CPU
> variable at compile time and share its physical address with hypervisor.
> It presents a challege when SEV is active in guest OS. When SEV is active,
> guest memory is encrypted with guest key and hypervisor will no longer able
> to modify the guest memory. When SEV is active, we need to clear the
> encryption attribute of shared physical addresses so that both guest and
> hypervisor can access the data.
> 
> To solve this problem, I have tried these three options:
> 
> 1) Convert the static per-CPU to dynamic per-CPU allocation. When SEV is
> detected then clear the encryption attribute. But while doing so I found
> that per-CPU dynamic allocator was not ready when kvm_guest_cpu_init was
> called.
> 
> 2) Since the encryption attributes works on PAGE_SIZE hence add some extra
> padding to 'struct kvm-steal-time' to make it PAGE_SIZE and then at runtime
> clear the encryption attribute of the full PAGE. The downside of this was
> now we need to modify structure which may break the compatibility.
> 
> 3) Define a new per-CPU section (.data..percpu.hv_shared) which will be
> used to hold the compile time shared per-CPU variables. When SEV is
> detected we map this section with encryption attribute cleared.
> 
> This patch implements #3. It introduces a new DEFINE_PER_CPU_HV_SHAHRED
> macro to create a compile time per-CPU variable. When SEV is detected we
> map the per-CPU variable as decrypted (i.e with encryption attribute cleared).
> 
> Signed-off-by: Brijesh Singh 

Looks good to me.

Paolo

> ---
>  arch/x86/kernel/kvm.c |   43 
> +++--
>  include/asm-generic/vmlinux.lds.h |3 +++
>  include/linux/percpu-defs.h   |9 
>  3 files changed, 48 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index 099fcba..706a08e 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
> @@ -75,8 +75,8 @@ static int parse_no_kvmclock_vsyscall(char *arg)
>  
>  early_param("no-kvmclock-vsyscall", parse_no_kvmclock_vsyscall);
>  
> -static DEFINE_PER_CPU(struct kvm_vcpu_pv_apf_data, apf_reason) __aligned(64);
> -static DEFINE_PER_CPU(struct kvm_steal_time, steal_time) __aligned(64);
> +static DEFINE_PER_CPU_HV_SHARED(struct kvm_vcpu_pv_apf_data, apf_reason) 
> __aligned(64);
> +static DEFINE_PER_CPU_HV_SHARED(struct kvm_steal_time, steal_time) 
> __aligned(64);
>  static int has_steal_clock = 0;
>  
>  /*
> @@ -290,6 +290,22 @@ static void __init paravirt_ops_setup(void)
>  #endif
>  }
>  
> +static int kvm_map_percpu_hv_shared(void *addr, unsigned long size)
> +{
> + /* When SEV is active, the percpu static variables initialized
> +  * in data section will contain the encrypted data so we first
> +  * need to decrypt it and then map it as decrypted.
> +  */
> + if (sev_active()) {
> + unsigned long pa = slow_virt_to_phys(addr);
> +
> + sme_early_decrypt(pa, size);
> + return early_set_memory_decrypted(addr, size);
> + }
> +
> + return 0;
> +}
> +
>  static void kvm_register_steal_time(void)
>  {
>   int cpu = smp_processor_id();
> @@ -298,12 +314,17 @@ static void kvm_register_steal_time(void)
>   if (!has_steal_clock)
>   return;
>  
> + if (kvm_map_percpu_hv_shared(st, sizeof(*st))) {
> + pr_err("kvm-stealtime: failed to map hv_shared percpu\n");
> + return;
> + }
> +
>   wrmsrl(MSR_KVM_STEAL_TIME, (slow_virt_to_phys(st) | KVM_MSR_ENABLED));
>   pr_info("kvm-stealtime: cpu %d, msr %llx\n",
>   cpu, (unsigned long long) slow_virt_to_phys(st));
>  }
>  
> -static DEFINE_PER_CPU(unsigned long, kvm_apic_eoi) = KVM_PV_EOI_DISABLED;
> +static DEFINE_PER_CPU_HV_SHARED(unsigned long, kvm_apic_eoi) = 
> KVM_PV_EOI_DISABLED;
>  
>  static notrace void kvm_guest_apic_eoi_write(u32 reg, u32 val)
>  {
> @@ -327,25 +348,33 @@ static void kvm_guest_cpu_init(void)
>   if (kvm_para_has_feature(KVM_FEATURE_ASYNC_PF) && kvmapf) {
>   u64 pa = slow_virt_to_phys(this_cpu_ptr(&apf_reason));
>  
> + if (kvm_map_percpu_hv_shared(this_cpu_ptr(&apf_reason),
> + sizeof(struct kvm_vcpu_pv_apf_data)))
> + goto skip_asyncpf;
>  #ifdef CONFIG_PREEMPT
>   pa |= KVM_ASYNC_PF_SEND_ALWAYS;
>  #endif
>   wrmsrl(MSR_KVM_ASYNC_PF_EN, pa | KVM_ASYNC_PF_ENABLED);
>   __this_cpu_write(apf_reason.enabled, 1);
> - printk(KERN_INFO"KVM setup async PF for cpu %d\n",
> -smp_processor_id());
> + printk(KERN_INFO"KVM setup async PF for cpu %d msr %llx\n",
> +smp_processor_id(), pa);
>   }
> -
> +skip_asyncpf:
>   if (kvm_para_has_feature(KVM_FEATURE_PV_EOI)) {
>   unsigned long pa;
>   

Re: [PATCH] Staging: goldfish: use __func__ instead of embedded function names

2017-03-16 Thread Dan Carpenter
You should have put a v3 in the subject and a changelog under the
--- cut off but otherwise it looks good.  Thanks!

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] Staging: goldfish: use __func__ instead of embedded function names v3

2017-03-16 Thread Mohsin Shan
Embedded function names are less appropriate to use when
refactoring, can cause function renaming.  Prefer the use
of "%s", __func__ to embedded function names

Signed-off-by: Mohsin Shan 
---
 drivers/staging/goldfish/goldfish_nand.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/goldfish/goldfish_nand.c 
b/drivers/staging/goldfish/goldfish_nand.c
index 76d60ee..8f92ff4 100644
--- a/drivers/staging/goldfish/goldfish_nand.c
+++ b/drivers/staging/goldfish/goldfish_nand.c
@@ -114,8 +114,8 @@ static int goldfish_nand_erase(struct mtd_info *mtd, struct 
erase_info *instr)
len = len / mtd->writesize * (mtd->writesize + mtd->oobsize);
 
if (goldfish_nand_cmd(mtd, NAND_CMD_ERASE, ofs, len, NULL) != len) {
-   pr_err("goldfish_nand_erase: erase failed, start %llx, len %x, 
dev_size %llx, erase_size %x\n",
-  ofs, len, mtd->size, mtd->erasesize);
+   pr_err("%s: erase failed, start %llx, len %x, dev_size %llx, 
erase_size %x\n",
+  __func__, ofs, len, mtd->size, mtd->erasesize);
return -EIO;
}
 
@@ -125,8 +125,8 @@ static int goldfish_nand_erase(struct mtd_info *mtd, struct 
erase_info *instr)
return 0;
 
 invalid_arg:
-   pr_err("goldfish_nand_erase: invalid erase, start %llx, len %x, 
dev_size %llx, erase_size %x\n",
-  ofs, len, mtd->size, mtd->erasesize);
+   pr_err("%s: invalid erase, start %llx, len %x, dev_size %llx, 
erase_size %x\n",
+  __func__, ofs, len, mtd->size, mtd->erasesize);
return -EINVAL;
 }
 
@@ -254,8 +254,8 @@ static int goldfish_nand_block_isbad(struct mtd_info *mtd, 
loff_t ofs)
return goldfish_nand_cmd(mtd, NAND_CMD_BLOCK_BAD_GET, ofs, 0, NULL);
 
 invalid_arg:
-   pr_err("goldfish_nand_block_isbad: invalid arg, ofs %llx, dev_size 
%llx, write_size %x\n",
-  ofs, mtd->size, mtd->writesize);
+   pr_err("%s: invalid arg, ofs %llx, dev_size %llx, write_size %x\n",
+  __func__, ofs, mtd->size, mtd->writesize);
return -EINVAL;
 }
 
@@ -277,8 +277,8 @@ static int goldfish_nand_block_markbad(struct mtd_info 
*mtd, loff_t ofs)
return 0;
 
 invalid_arg:
-   pr_err("goldfish_nand_block_markbad: invalid arg, ofs %llx, dev_size 
%llx, write_size %x\n",
-  ofs, mtd->size, mtd->writesize);
+   pr_err("%s: invalid arg, ofs %llx, dev_size %llx, write_size %x\n",
+  __func__, ofs, mtd->size, mtd->writesize);
return -EINVAL;
 }
 
-- 
2.7.4

# Change Log.
## [v1]
- Changed the name of the patch. Previous name 
"drivers/staging/goldfish/goldfish_nand.c: 
  Fixed Coding style Warnings" was ambiguous.
- Used formal name in the patch.

## [v2]
- Changed name in the email header to formal name.

## [v3]
- Added Changelog
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 2/6] staging: ks7010: replace identifier retval with rc

2017-03-16 Thread Dan Carpenter
On Thu, Mar 16, 2017 at 11:40:47AM +0900, Greg Kroah-Hartman wrote:
> On Tue, Mar 14, 2017 at 09:20:13PM +1100, Tobin C. Harding wrote:
> > Code uses identifiers retval, ret, and rc all for function return
> > values. It would be more readable if the whole driver used a single
> > identifier for this task. Lets use 'rc' since it is the shortest.
> > 
> > Change retval -> rc
> 
> Ick, retval is much more readable, please use that.
> 

Or "ret" is probably the most common.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] bcm2835-v4l2: Fix buffer overflow problem

2017-03-16 Thread Dave Stevenson
On 16 March 2017 at 05:11, Eric Anholt  wrote:
> Greg KH  writes:
>
>> On Wed, Mar 15, 2017 at 03:32:56PM +, Dave Stevenson wrote:
>>> You've got a reason. It's GPLv2 licenced code so I have no control
>>> over what happens to it.
>>> Everywhere I have worked, when a patch has issues it is better to "-1"
>>> to stop/delay the merge even (or particularly) on your own patches,
>>> but this is your playground so your rules.
>>
>> It's fine to say "don't apply, for _THIS REASON_".  You didn't specify a
>> reason, which is why I complained.
>>
>>> Anyway, I'll go back to working with the downstream tree (pull request
>>> for the full fix there) and stop bothering you.
>>
>> Ah, fun, we will diverge even more in the future, not good.  Any reason
>> why I shouldn't just delete this whole in-kernel tree if you are going
>> to spend time maintaining a different version?
>>
>> If you want to maintain this driver, in the kernel tree, by all means I
>> would love the help as I don't have hardware or the ability to test
>> anything.  Having two different trees for the source guarantees that
>> there are going to be constant problems here, and that's not good for
>> anyone.
>
> If Dave doesn't do the upstreaming work for his future work, I'll be
> forced to.  The rpi tree still hasn't diverged, yet (other than this
> patch and the related one being discussed).

Firstly, sorry. Bad day and frustrations vented inappropriately.
I'll consider this my baptism of fire in the ways of the mainline
kernel. Lessons learnt: Justify everything, provide defined timelines
for fixes, and develop a thick skin.

Upstreaming this driver isn't my main task at the moment, but I do
intend to come back to it, in particular to address the issues raised
on linux-media. Timescales of 4-6 weeks all being well (defined
timeline!).
There are no current developments on this driver that I am aware of,
so that delay shouldn't be critical for divergence. My next set of
changes to it are dependent on the current tasks.
I will also be undertaking backporting the changes made in staging to
the out-of-tree driver, so divergence will be minimised. Out-of-tree
is currently 4.4 but about to adopt 4.9, so is not in a position to
take the current staging as-is.

Michael's request was an interrupt serviced badly. Having no existing
tree to test staging with it was taking me a little while after
getting the code reviewed internally to get a tree working and confirm
all was good (at least a basic compile test). To suddenly find that
wasted was annoying and I reacted :-(


Take this patch as is, and I'll liase with Michael so that one of us
creates and sends the follow up patch to complete the fix.

I'm learning :-)

  Dave
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] vc04_services: Fixing coding guideline error

2017-03-16 Thread Dan Carpenter
On Thu, Mar 16, 2017 at 11:01:38AM +0530, Pushkar Jambhlekar wrote:
> Any comment?
> 

You're too impatient.  Wait for 2 weeks before asking for responses.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 4/6] staging: ks7010: use le16_to_cpu for arithmetic

2017-03-16 Thread Tobin C. Harding
Code performs arithmetic using a little endian type mixed with
non-endian types. This won't work on big endian architectures.

Wrap little endian type in call to le16_to_cpu before doing arithmetic.

Signed-off-by: Tobin C. Harding 
---
 drivers/staging/ks7010/ks_hostif.c | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/ks7010/ks_hostif.c 
b/drivers/staging/ks7010/ks_hostif.c
index d70472e..8dfe508 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -1944,6 +1944,7 @@ void hostif_sme_set_rsn(struct ks_wlan_private *priv, int 
type)
struct wpa_suite_t wpa_suite;
struct rsn_mode_t rsn_mode;
__le32 val;
+   u16 size;
 
memset(&wpa_suite, 0, sizeof(wpa_suite));
 
@@ -1992,11 +1993,10 @@ void hostif_sme_set_rsn(struct ks_wlan_private *priv, 
int type)
   CIPHER_ID_WPA_WEP104, CIPHER_ID_LEN);
break;
}
-
+   size = sizeof(wpa_suite.size) + CIPHER_ID_LEN * 
le16_to_cpu(wpa_suite.size);
hostif_mib_set_request(priv, DOT11_RSN_CONFIG_UNICAST_CIPHER,
-  sizeof(wpa_suite.size) +
-  CIPHER_ID_LEN * wpa_suite.size,
-  MIB_VALUE_TYPE_OSTRING, &wpa_suite);
+  size, MIB_VALUE_TYPE_OSTRING,
+  &wpa_suite);
break;
case SME_RSN_MCAST_REQUEST:
switch (priv->wpa.group_suite) {
@@ -2085,9 +2085,8 @@ void hostif_sme_set_rsn(struct ks_wlan_private *priv, int 
type)
break;
}
 
-   hostif_mib_set_request(priv, DOT11_RSN_CONFIG_AUTH_SUITE,
-  sizeof(wpa_suite.size) +
-  KEY_MGMT_ID_LEN * wpa_suite.size,
+   size =  sizeof(wpa_suite.size) + KEY_MGMT_ID_LEN * 
le16_to_cpu(wpa_suite.size);
+   hostif_mib_set_request(priv, DOT11_RSN_CONFIG_AUTH_SUITE, size,
   MIB_VALUE_TYPE_OSTRING, &wpa_suite);
break;
case SME_RSN_ENABLED_REQUEST:
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 3/6] staging: ks7010: set lval type of cpu_to_leXX()

2017-03-16 Thread Tobin C. Harding
Sparse emits numerous warning: incorrect type in assignment.

These are all caused by calls to cpu_to_le16() and cpu_to_le32(). Any
identifier that is used as an lval in a call to one of these functions
should be a little endian type. The size of the type is deterministic
because of the size returned by cpu_to_leXX()

Change unsigned types to little endian types.

Signed-off-by: Tobin C. Harding 
---
 drivers/staging/ks7010/ks_hostif.c | 18 
 drivers/staging/ks7010/ks_hostif.h | 86 +++---
 2 files changed, 52 insertions(+), 52 deletions(-)

diff --git a/drivers/staging/ks7010/ks_hostif.c 
b/drivers/staging/ks7010/ks_hostif.c
index 57b3e7c..d70472e 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -1880,7 +1880,7 @@ void hostif_receive(struct ks_wlan_private *priv, 
unsigned char *p,
 static
 void hostif_sme_set_wep(struct ks_wlan_private *priv, int type)
 {
-   u32 val;
+   __le32 val;
 
switch (type) {
case SME_WEP_INDEX_REQUEST:
@@ -1929,13 +1929,13 @@ void hostif_sme_set_wep(struct ks_wlan_private *priv, 
int type)
 }
 
 struct wpa_suite_t {
-   unsigned short size;
+   __le16 size;
unsigned char suite[4][CIPHER_ID_LEN];
 } __packed;
 
 struct rsn_mode_t {
-   u32 rsn_mode;
-   u16 rsn_capability;
+   __le32 rsn_mode;
+   __le16 rsn_capability;
 } __packed;
 
 static
@@ -1943,7 +1943,7 @@ void hostif_sme_set_rsn(struct ks_wlan_private *priv, int 
type)
 {
struct wpa_suite_t wpa_suite;
struct rsn_mode_t rsn_mode;
-   u32 val;
+   __le32 val;
 
memset(&wpa_suite, 0, sizeof(wpa_suite));
 
@@ -2230,7 +2230,7 @@ void hostif_sme_multicast_set(struct ks_wlan_private 
*priv)
int mc_count;
struct netdev_hw_addr *ha;
char set_address[NIC_MAX_MCAST_LIST * ETH_ALEN];
-   unsigned long filter_type;
+   __le32 filter_type;
int i = 0;
 
DPRINTK(3, "\n");
@@ -2339,7 +2339,7 @@ void hostif_sme_sleep_set(struct ks_wlan_private *priv)
 static
 void hostif_sme_set_key(struct ks_wlan_private *priv, int type)
 {
-   u32 val;
+   __le32 val;
 
switch (type) {
case SME_SET_FLAG:
@@ -2398,7 +2398,7 @@ static
 void hostif_sme_set_pmksa(struct ks_wlan_private *priv)
 {
struct pmk_cache_t {
-   u16 size;
+   __le16 size;
struct {
u8 bssid[ETH_ALEN];
u8 pmkid[IW_PMKID_LEN];
@@ -2429,7 +2429,7 @@ void hostif_sme_set_pmksa(struct ks_wlan_private *priv)
 static
 void hostif_sme_execute(struct ks_wlan_private *priv, int event)
 {
-   u32 val;
+   __le32 val;
 
DPRINTK(3, "event=%d\n", event);
switch (event) {
diff --git a/drivers/staging/ks7010/ks_hostif.h 
b/drivers/staging/ks7010/ks_hostif.h
index aa11d43..4c04048 100644
--- a/drivers/staging/ks7010/ks_hostif.h
+++ b/drivers/staging/ks7010/ks_hostif.h
@@ -62,13 +62,13 @@
  */
 
 struct hostif_hdr {
-   u16 size;
-   u16 event;
+   __le16 size;
+   __le16 event;
 } __packed;
 
 struct hostif_data_request_t {
struct hostif_hdr header;
-   u16 auth_type;
+   __le16 auth_type;
 #define TYPE_DATA 0x
 #define TYPE_AUTH 0x0001
u16 reserved;
@@ -143,12 +143,12 @@ struct channel_list_t {
 
 struct hostif_mib_get_request_t {
struct hostif_hdr header;
-   u32 mib_attribute;
+   __le32 mib_attribute;
 } __packed;
 
 struct hostif_mib_value_t {
-   u16 size;
-   u16 type;
+   __le16 size;
+   __le16 type;
 #define MIB_VALUE_TYPE_NULL 0
 #define MIB_VALUE_TYPE_INT  1
 #define MIB_VALUE_TYPE_BOOL 2
@@ -170,7 +170,7 @@ struct hostif_mib_get_confirm_t {
 
 struct hostif_mib_set_request_t {
struct hostif_hdr header;
-   u32 mib_attribute;
+   __le32 mib_attribute;
struct hostif_mib_value_t mib_value;
 } __packed;
 
@@ -182,13 +182,13 @@ struct hostif_mib_set_confirm_t {
 
 struct hostif_power_mngmt_request_t {
struct hostif_hdr header;
-   u32 mode;
+   __le32 mode;
 #define POWER_ACTIVE  1
 #define POWER_SAVE2
-   u32 wake_up;
+   __le32 wake_up;
 #define SLEEP_FALSE 0
 #define SLEEP_TRUE  1  /* not used */
-   u32 receiveDTIMs;
+   __le32 receiveDTIMs;
 #define DTIM_FALSE 0
 #define DTIM_TRUE  1
 } __packed;
@@ -213,7 +213,7 @@ struct hostif_power_mngmt_confirm_t {
 
 struct hostif_start_request_t {
struct hostif_hdr header;
-   u16 mode;
+   __le16 mode;
 #define MODE_PSEUDO_ADHOC   0
 #define MODE_INFRASTRUCTURE 1
 #define MODE_AP 2  /* not used */
@@ -357,18 +357,18 @@ struct hostif_stop_confirm_t {
  */
 struct hostif_ps_adhoc_set_request_t {
struct hostif_hdr header;
-   u16 phy_type;
+   __le16 phy_type;
 #define D_11B_ONLY_MODE0
 #define D_11G_ONLY_MODE1
 #define D_11BG_COMPATIBLE_MODE 2
 #define D_11A_ONLY_MODE  

[PATCH 2/6] staging: ks7010: change unsigned short to __be16

2017-03-16 Thread Tobin C. Harding
Sparse emits warning: cast to restricted __be16. This warning is
caused by passing an unsigned short as argument to ntohs(). ntohs() is
defined in linux/byteorder/generic.h (via __ntohs()) to
__be16_to_cpu(). The argument should therefore be big endian.

Change data type unsigned short -> __be16.

Signed-off-by: Tobin C. Harding 
---
 drivers/staging/ks7010/eap_packet.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/ks7010/eap_packet.h 
b/drivers/staging/ks7010/eap_packet.h
index 7a3decf..124167f 100644
--- a/drivers/staging/ks7010/eap_packet.h
+++ b/drivers/staging/ks7010/eap_packet.h
@@ -16,7 +16,7 @@ struct ether_hdr {
unsigned char h_source_snap;
unsigned char h_command;
unsigned char h_vendor_id[3];
-   unsigned short h_proto; /* packet type ID field */
+   __be16 h_proto; /* packet type ID field */
 #define ETHER_PROTOCOL_TYPE_EAP0x888e
 #define ETHER_PROTOCOL_TYPE_IP 0x0800
 #define ETHER_PROTOCOL_TYPE_ARP0x0806
@@ -89,7 +89,7 @@ struct ieee802_1x_eapol_key {
 
 struct wpa_eapol_key {
unsigned char type;
-   unsigned short key_info;
+   __be16 key_info;
unsigned short key_length;
unsigned char replay_counter[WPA_REPLAY_COUNTER_LEN];
unsigned char key_nonce[WPA_NONCE_LEN];
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 0/6] staging: ks7010: fix endian issues

2017-03-16 Thread Tobin C. Harding
Sparse emits numerous warnings highlighting endian issues. The
endianess is deterministic because the function calls that are causing
the issues either accept as parameters, or return as results, types of
specific size and endianess. We can use these types as a guide in
changing variable declarations and struct member definitions to suit
the required endianess.

Patch 01 renames a common kernel identifier.

Patch 02 fixes variable passed into ntohs().

Patch 03 fixes declarations for variables that are used as lval's in calls
to cpu_to_leXX().

Patch 04 fixes endian bug, making arithmetic work on any architecture.

Patch 05 fixes endian variable degrading to integer.

Patch 06 uses le16_to_cpu() for assigning endian value to cpu type.

Code has not been tested. Patch set builds on x86_64 and PowerPC.

Tobin C. Harding (6):
  staging: ks7010: rename sk_buf ptr to skb
  staging: ks7010: change unsigned short to __be16
  staging: ks7010: set lval type of cpu_to_leXX()
  staging: ks7010: use le16_to_cpu for arithmetic
  staging: ks7010: add endian non-specific variable
  staging: ks7010: use le16_to_cpu() to queue event

 drivers/staging/ks7010/eap_packet.h  |  4 +-
 drivers/staging/ks7010/ks7010_sdio.c | 14 --
 drivers/staging/ks7010/ks_hostif.c   | 71 ++---
 drivers/staging/ks7010/ks_hostif.h   | 88 ++--
 4 files changed, 90 insertions(+), 87 deletions(-)

-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 6/6] staging: ks7010: use le16_to_cpu() to queue event

2017-03-16 Thread Tobin C. Harding
Events are stored as a circular buffer of integers. An event is of type
__le16, to store this in the buffer we need to convert it to the
endianess of the cpu.

Call le16_to_cpu() before adding event to events buffer.

Signed-off-by: Tobin C. Harding 
---
 drivers/staging/ks7010/ks7010_sdio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c 
b/drivers/staging/ks7010/ks7010_sdio.c
index 6083b7e..c47583d 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -357,7 +357,7 @@ int ks_wlan_hw_tx(struct ks_wlan_private *priv, void *p, 
unsigned long size,
}
 
/* add event to hostt buffer */
-   priv->hostt.buff[priv->hostt.qtail] = hdr->event;
+   priv->hostt.buff[priv->hostt.qtail] = le16_to_cpu(hdr->event);
priv->hostt.qtail = (priv->hostt.qtail + 1) % SME_EVENT_BUFF_SIZE;
 
DPRINTK(4, "event=%04X\n", hdr->event);
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 5/6] staging: ks7010: add endian non-specific variable

2017-03-16 Thread Tobin C. Harding
Sparse emits warning: restricted __le16 degrades to integer. This is
caused by a comparison between an endian type and a constant. Before
comparison can be safely done we need to convert the endian type to
the endianess of the cpu.

Declare kernel standard type and convert endian type with call to
le16_to_cpu().

Signed-off-by: Tobin C. Harding 
---
 drivers/staging/ks7010/ks7010_sdio.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c 
b/drivers/staging/ks7010/ks7010_sdio.c
index 0fa13cd..6083b7e 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -280,12 +280,14 @@ static int write_to_device(struct ks_wlan_private *priv, 
unsigned char *buffer,
unsigned char rw_data;
struct hostif_hdr *hdr;
int rc;
+   u16 event;
 
hdr = (struct hostif_hdr *)buffer;
+   event = le16_to_cpu(hdr->event);
 
DPRINTK(4, "size=%d\n", hdr->size);
-   if (hdr->event < HIF_DATA_REQ || HIF_REQ_MAX < hdr->event) {
-   DPRINTK(1, "unknown event=%04X\n", hdr->event);
+   if (event < HIF_DATA_REQ || HIF_REQ_MAX < event) {
+   DPRINTK(1, "unknown event=%04X\n", event);
return 0;
}
 
@@ -344,11 +346,13 @@ int ks_wlan_hw_tx(struct ks_wlan_private *priv, void *p, 
unsigned long size,
 {
int result = 0;
struct hostif_hdr *hdr;
+   u16 event;
 
hdr = (struct hostif_hdr *)p;
+   event = le16_to_cpu(hdr->event);
 
-   if (hdr->event < HIF_DATA_REQ || HIF_REQ_MAX < hdr->event) {
-   DPRINTK(1, "unknown event=%04X\n", hdr->event);
+   if (event < HIF_DATA_REQ || HIF_REQ_MAX < event) {
+   DPRINTK(1, "unknown event=%04X\n", event);
return 0;
}
 
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 1/6] staging: ks7010: rename sk_buf ptr to skb

2017-03-16 Thread Tobin C. Harding
There are 8088 struct sk_buf pointers declared in the kernel under
net/ of these 6670 are call skb. This is an indication that
'skb' a better identifier than 'packet' for an sk_buf pointer.

Rename packet -> skb

Signed-off-by: Tobin C. Harding 
---
 drivers/staging/ks7010/ks_hostif.c | 40 +++---
 drivers/staging/ks7010/ks_hostif.h |  2 +-
 2 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/drivers/staging/ks7010/ks_hostif.c 
b/drivers/staging/ks7010/ks_hostif.c
index db10e16..57b3e7c 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -1108,9 +1108,9 @@ void hostif_event_check(struct ks_wlan_private *priv)
 
 #define CHECK_ALINE(size) (size % 4 ? (size + (4 - (size % 4))) : size)
 
-int hostif_data_request(struct ks_wlan_private *priv, struct sk_buff *packet)
+int hostif_data_request(struct ks_wlan_private *priv, struct sk_buff *skb)
 {
-   unsigned int packet_len = 0;
+   unsigned int skb_len = 0;
 
unsigned char *buffer = NULL;
unsigned int length = 0;
@@ -1126,9 +1126,9 @@ int hostif_data_request(struct ks_wlan_private *priv, 
struct sk_buff *packet)
struct ethhdr *eth;
int rc;
 
-   packet_len = packet->len;
-   if (packet_len > ETH_FRAME_LEN) {
-   DPRINTK(1, "bad length packet_len=%d\n", packet_len);
+   skb_len = skb->len;
+   if (skb_len > ETH_FRAME_LEN) {
+   DPRINTK(1, "bad length skb_len=%d\n", skb_len);
rc = -EOVERFLOW;
goto err_kfree_skb;
}
@@ -1139,8 +1139,8 @@ int hostif_data_request(struct ks_wlan_private *priv, 
struct sk_buff *packet)
DPRINTK(3, " DISCONNECT\n");
if (netif_queue_stopped(priv->net_dev))
netif_wake_queue(priv->net_dev);
-   if (packet)
-   dev_kfree_skb(packet);
+   if (skb)
+   dev_kfree_skb(skb);
 
return 0;
}
@@ -1151,8 +1151,8 @@ int hostif_data_request(struct ks_wlan_private *priv, 
struct sk_buff *packet)
netif_stop_queue(priv->net_dev);
}
 
-   DPRINTK(4, "skb_buff length=%d\n", packet_len);
-   pp = kmalloc(hif_align_size(sizeof(*pp) + 6 + packet_len + 8),
+   DPRINTK(4, "skb_buff length=%d\n", skb_len);
+   pp = kmalloc(hif_align_size(sizeof(*pp) + 6 + skb_len + 8),
 KS_WLAN_MEM_FLAG);
 
if (!pp) {
@@ -1163,11 +1163,11 @@ int hostif_data_request(struct ks_wlan_private *priv, 
struct sk_buff *packet)
 
p = (unsigned char *)pp->data;
 
-   buffer = packet->data;
-   length = packet->len;
+   buffer = skb->data;
+   length = skb->len;
 
/* packet check */
-   eth = (struct ethhdr *)packet->data;
+   eth = (struct ethhdr *)skb->data;
if (memcmp(&priv->eth_addr[0], eth->h_source, ETH_ALEN)) {
DPRINTK(1, "invalid mac address !!\n");
DPRINTK(1, "ethernet->h_source=%pM\n", eth->h_source);
@@ -1191,13 +1191,13 @@ int hostif_data_request(struct ks_wlan_private *priv, 
struct sk_buff *packet)
*p++ = 0x00;/* OUI ("00") */
*p++ = 0x00;/* OUI ("00") */
*p++ = 0x00;/* OUI ("00") */
-   packet_len += 6;
+   skb_len += 6;
} else {
DPRINTK(4, "DIX\n");
/* Length(2 byte) delete */
buffer += 2;
length -= 2;
-   packet_len -= 2;
+   skb_len -= 2;
}
 
/* pp->data copy */
@@ -1227,12 +1227,12 @@ int hostif_data_request(struct ks_wlan_private *priv, 
struct sk_buff *packet)
pp->auth_type = cpu_to_le16((uint16_t)TYPE_AUTH);   
/* no encryption */
} else {
if (priv->wpa.pairwise_suite == IW_AUTH_CIPHER_TKIP) {
-   MichaelMICFunction(&michael_mic, (uint8_t 
*)priv->wpa.key[0].tx_mic_key, (uint8_t *)&pp->data[0], (int)packet_len, 
(uint8_t)0,  /* priority */
+   MichaelMICFunction(&michael_mic, (uint8_t 
*)priv->wpa.key[0].tx_mic_key, (uint8_t *)&pp->data[0], (int)skb_len, 
(uint8_t)0, /* priority */
   (uint8_t *)michael_mic.
   Result);
memcpy(p, michael_mic.Result, 8);
length += 8;
-   packet_len += 8;
+   skb_len += 8;
p += 8;
pp->auth_type =
cpu_to_le16((uint16_t)TYPE_DATA);
@@ -1253,14 +1253,14 @@ int hostif_data_request(struct ks_wlan_private *priv, 
struct sk_buff *packet)
/* header value set */
pp->header.size =
cpu_to_

Re: [RFC PATCH v2 14/32] x86: mm: Provide support to use memblock when spliting large pages

2017-03-16 Thread Paolo Bonzini


On 02/03/2017 16:15, Brijesh Singh wrote:
> 
>  __split_large_page(struct cpa_data *cpa, pte_t *kpte, unsigned long address,
> -struct page *base)
> +   pte_t *pbase, unsigned long new_pfn)
>  {
> - pte_t *pbase = (pte_t *)page_address(base);

Just one comment and I'll reply to Boris, I think you can compute pbase 
with pfn_to_kaddr, and avoid adding a new argument.

>*/
> - __set_pmd_pte(kpte, address, mk_pte(base, __pgprot(_KERNPG_TABLE)));
> + __set_pmd_pte(kpte, address,
> + native_make_pte((new_pfn << PAGE_SHIFT) + _KERNPG_TABLE));

And this probably is better written as:

__set_pmd_pte(kpte, address, pfn_pte(new_pfn, __pgprot(_KERNPG_TABLE));

Paolo
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 2/6] staging: ks7010: replace identifier retval with rc

2017-03-16 Thread Tobin C. Harding
On Thu, Mar 16, 2017 at 02:29:04PM +0300, Dan Carpenter wrote:
> On Thu, Mar 16, 2017 at 11:40:47AM +0900, Greg Kroah-Hartman wrote:
> > On Tue, Mar 14, 2017 at 09:20:13PM +1100, Tobin C. Harding wrote:
> > > Code uses identifiers retval, ret, and rc all for function return
> > > values. It would be more readable if the whole driver used a single
> > > identifier for this task. Lets use 'rc' since it is the shortest.
> > > 
> > > Change retval -> rc
> > 
> > Ick, retval is much more readable, please use that.
> > 
> 
> Or "ret" is probably the most common.

fight!

$ git grep -e 'int ret;' --or -e 'int ret =' | wc -l
36610

$ git grep -e 'int retval;' --or -e 'int retval =' | wc -l
2420

$ git grep -e 'int rc;' --or -e 'int rc =' | wc -l
8307

*ouch*! 'retval' is more readable, 'rc' is shorter but perfectly
descriptive and 'ret' is just plain common.


thanks,
Tobin.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [RFC PATCH v2 14/32] x86: mm: Provide support to use memblock when spliting large pages

2017-03-16 Thread Paolo Bonzini


On 10/03/2017 23:41, Brijesh Singh wrote:
>> Maybe there's a reason this fires:
>>
>> WARNING: modpost: Found 2 section mismatch(es).
>> To see full details build your kernel with:
>> 'make CONFIG_DEBUG_SECTION_MISMATCH=y'
>>
>> WARNING: vmlinux.o(.text+0x48edc): Section mismatch in reference from
>> the function __change_page_attr() to the function
>> .init.text:memblock_alloc()
>> The function __change_page_attr() references
>> the function __init memblock_alloc().
>> This is often because __change_page_attr lacks a __init
>> annotation or the annotation of memblock_alloc is wrong.
>>
>> WARNING: vmlinux.o(.text+0x491d1): Section mismatch in reference from
>> the function __change_page_attr() to the function
>> .meminit.text:memblock_free()
>> The function __change_page_attr() references
>> the function __meminit memblock_free().
>> This is often because __change_page_attr lacks a __meminit
>> annotation or the annotation of memblock_free is wrong.
>> 
>> But maybe Paolo might have an even better idea...
> 
> I am sure he will have better idea :)

Not sure if it's better or worse, but an alternative idea is to turn
__change_page_attr and __change_page_attr_set_clr inside out, so that:
1) the alloc_pages/__free_page happens in __change_page_attr_set_clr;
2) __change_page_attr_set_clr overall does not beocome more complex.

Then you can introduce __early_change_page_attr_set_clr and/or
early_kernel_map_pages_in_pgd, for use in your next patches.  They use
the memblock allocator instead of alloc/free_page

The attached patch is compile-tested only and almost certainly has some
thinko in it.  But it even skims a few lines from the code so the idea
might have some merit.

Paolo
diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c
index 28d42130243c..953c8e697562 100644
--- a/arch/x86/mm/pageattr.c
+++ b/arch/x86/mm/pageattr.c
@@ -490,11 +490,12 @@ static void __set_pmd_pte(pte_t *kpte, unsigned long address, pte_t pte)
 }
 
 static int
-try_preserve_large_page(pte_t *kpte, unsigned long address,
+try_preserve_large_page(pte_t **p_kpte, unsigned long address,
 			struct cpa_data *cpa)
 {
 	unsigned long nextpage_addr, numpages, pmask, psize, addr, pfn, old_pfn;
-	pte_t new_pte, old_pte, *tmp;
+	pte_t *kpte = *p_kpte;
+	pte_t new_pte, old_pte;
 	pgprot_t old_prot, new_prot, req_prot;
 	int i, do_split = 1;
 	enum pg_level level;
@@ -507,8 +508,8 @@ try_preserve_large_page(pte_t *kpte, unsigned long address,
 	 * Check for races, another CPU might have split this page
 	 * up already:
 	 */
-	tmp = _lookup_address_cpa(cpa, address, &level);
-	if (tmp != kpte)
+	*p_kpte = _lookup_address_cpa(cpa, address, &level);
+	if (*p_kpte != kpte)
 		goto out_unlock;
 
 	switch (level) {
@@ -634,17 +635,18 @@ __split_large_page(struct cpa_data *cpa, pte_t *kpte, unsigned long address,
 	unsigned int i, level;
 	pte_t *tmp;
 	pgprot_t ref_prot;
+	int retry = 1;
 
+	if (!debug_pagealloc_enabled())
+		spin_lock(&cpa_lock);
 	spin_lock(&pgd_lock);
 	/*
 	 * Check for races, another CPU might have split this page
 	 * up for us already:
 	 */
 	tmp = _lookup_address_cpa(cpa, address, &level);
-	if (tmp != kpte) {
-		spin_unlock(&pgd_lock);
-		return 1;
-	}
+	if (tmp != kpte)
+		goto out;
 
 	paravirt_alloc_pte(&init_mm, page_to_pfn(base));
 
@@ -671,10 +673,11 @@ __split_large_page(struct cpa_data *cpa, pte_t *kpte, unsigned long address,
 		break;
 
 	default:
-		spin_unlock(&pgd_lock);
-		return 1;
+		goto out;
 	}
 
+	retry = 0;
+
 	/*
 	 * Set the GLOBAL flags only if the PRESENT flag is set
 	 * otherwise pmd/pte_present will return true even on a non
@@ -718,28 +721,34 @@ __split_large_page(struct cpa_data *cpa, pte_t *kpte, unsigned long address,
 	 * going on.
 	 */
 	__flush_tlb_all();
-	spin_unlock(&pgd_lock);
 
-	return 0;
-}
-
-static int split_large_page(struct cpa_data *cpa, pte_t *kpte,
-			unsigned long address)
-{
-	struct page *base;
+out:
+	spin_unlock(&pgd_lock);
 
+	/*
+	 * Do a global flush tlb after splitting the large page
+ 	 * and before we do the actual change page attribute in the PTE.
+ 	 *
+ 	 * With out this, we violate the TLB application note, that says
+ 	 * "The TLBs may contain both ordinary and large-page
+	 *  translations for a 4-KByte range of linear addresses. This
+	 *  may occur if software modifies the paging structures so that
+	 *  the page size used for the address range changes. If the two
+	 *  translations differ with respect to page frame or attributes
+	 *  (e.g., permissions), processor behavior is undefined and may
+	 *  be implementation-specific."
+ 	 *
+ 	 * We do this global tlb flush inside the cpa_lock, so that we
+	 * don't allow any other cpu, with stale tlb entries change the
+	 * page attribute in parallel, that also falls into the
+	 * just split large page entry.
+ 	 */
+	if (!retry)
+		flush_tlb_all();
 	if (!debug_pagealloc_enabled())
 		spin_unlock(&cpa_lock);
-	base = alloc_pages(GFP_KERNEL | __GFP_NOTRACK, 0);
-	if (!debug_pagealloc_enabled())
-

Re: [PATCH] Staging: goldfish: use __func__ instead of embedded function names v3

2017-03-16 Thread Greg KH
On Thu, Mar 16, 2017 at 04:25:54AM -0700, Mohsin Shan wrote:
> Embedded function names are less appropriate to use when
> refactoring, can cause function renaming.  Prefer the use
> of "%s", __func__ to embedded function names
> 
> Signed-off-by: Mohsin Shan 
> ---
>  drivers/staging/goldfish/goldfish_nand.c | 16 
>  1 file changed, 8 insertions(+), 8 deletions(-)

Please read Documenation/SubmittingPatches in the section that describes
how to version your patch properly.

v4?  :)

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [RFC PATCH v2 12/32] x86: Add early boot support when running with SEV active

2017-03-16 Thread Tom Lendacky

On 3/16/2017 5:16 AM, Borislav Petkov wrote:

On Fri, Mar 10, 2017 at 10:35:30AM -0600, Brijesh Singh wrote:

We could update this patch to use the below logic:

 * CPUID(0) - Check for AuthenticAMD
 * CPID(1) - Check if under hypervisor
 * CPUID(0x8000) - Check for highest supported leaf
 * CPUID(0x801F).EAX - Check for SME and SEV support
 * rdmsr (MSR_K8_SYSCFG)[MemEncryptionModeEnc] - Check if SMEE is set


Actually, it is still not clear to me *why* we need to do anything
special wrt SEV in the guest.

Lemme clarify: why can't the guest boot just like a normal Linux on
baremetal and use the SME(!) detection code to set sme_enable and so
on? IOW, I'd like to avoid all those checks whether we're running under
hypervisor and handle all that like we're running on baremetal.


Because there are differences between how SME and SEV behave
(instruction fetches are always decrypted under SEV, DMA to an
encrypted location is not supported under SEV, etc.) we need to
determine which mode we are in so that things can be setup properly
during boot. For example, if SEV is active the kernel will already
be encrypted and so we don't perform that step or the trampoline area
for bringing up an AP must be decrypted for SME but encrypted for SEV.
The hypervisor check will provide that ability to determine how we
handle things.

Thanks,
Tom




___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [RFC PATCH v2 12/32] x86: Add early boot support when running with SEV active

2017-03-16 Thread Borislav Petkov
On Thu, Mar 16, 2017 at 09:28:58AM -0500, Tom Lendacky wrote:
> Because there are differences between how SME and SEV behave
> (instruction fetches are always decrypted under SEV, DMA to an
> encrypted location is not supported under SEV, etc.) we need to
> determine which mode we are in so that things can be setup properly
> during boot. For example, if SEV is active the kernel will already
> be encrypted and so we don't perform that step or the trampoline area
> for bringing up an AP must be decrypted for SME but encrypted for SEV.

So with SEV enabled, it seems to me a guest doesn't know anything about
encryption and can run as if SME is disabled. So sme_active() will be
false. And then the kernel can bypass all that code dealing with SME.

So a guest should simply run like on baremetal with no SME, IMHO.

But then there's that part: "instruction fetches are always decrypted
under SEV". What does that mean exactly? And how much of that code can
be reused so that

* SME on baremetal
* SEV on guest

use the same logic?

Having the larger SEV preparation part on the kvm host side is perfectly
fine. But I'd like to keep kernel initialization paths clean.

Thanks.

-- 
Regards/Gruss,
Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 
(AG Nürnberg)
-- 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] Staging: iio: resolver: Fix warning, statements should start on a tabstop

2017-03-16 Thread Miguel Robles
Fix checkpatch warning:
Statements should start on a tabstop.

Signed-off-by: Miguel Robles 
---
 drivers/staging/iio/resolver/ad2s1210.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/iio/resolver/ad2s1210.c 
b/drivers/staging/iio/resolver/ad2s1210.c
index 90b57c0..a6a8393 100644
--- a/drivers/staging/iio/resolver/ad2s1210.c
+++ b/drivers/staging/iio/resolver/ad2s1210.c
@@ -490,8 +490,8 @@ static int ad2s1210_read_raw(struct iio_dev *indio_dev,
ad2s1210_set_mode(MOD_VEL, st);
break;
default:
-  ret = -EINVAL;
-  break;
+   ret = -EINVAL;
+   break;
}
if (ret < 0)
goto error_ret;
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [RFC PATCH v2 12/32] x86: Add early boot support when running with SEV active

2017-03-16 Thread Tom Lendacky

On 3/16/2017 10:09 AM, Borislav Petkov wrote:

On Thu, Mar 16, 2017 at 09:28:58AM -0500, Tom Lendacky wrote:

Because there are differences between how SME and SEV behave
(instruction fetches are always decrypted under SEV, DMA to an
encrypted location is not supported under SEV, etc.) we need to
determine which mode we are in so that things can be setup properly
during boot. For example, if SEV is active the kernel will already
be encrypted and so we don't perform that step or the trampoline area
for bringing up an AP must be decrypted for SME but encrypted for SEV.


So with SEV enabled, it seems to me a guest doesn't know anything about
encryption and can run as if SME is disabled. So sme_active() will be
false. And then the kernel can bypass all that code dealing with SME.

So a guest should simply run like on baremetal with no SME, IMHO.



Not quite. The guest still needs to understand about the encryption mask
so that it can protect memory by setting the encryption mask in the
pagetable entries.  It can also decide when to share memory with the
hypervisor by not setting the encryption mask in the pagetable entries.


But then there's that part: "instruction fetches are always decrypted
under SEV". What does that mean exactly? And how much of that code can


"Instruction fetches are always decrypted under SEV" means that,
regardless of how a virtual address is mapped, encrypted or decrypted,
if an instruction fetch is performed by the CPU from that address it
will always be decrypted. This is to prevent the hypervisor from
injecting executable code into the guest since it would have to be
valid encrypted instructions.


be reused so that

* SME on baremetal
* SEV on guest

use the same logic?


There are many areas that use the same logic, but there are certain
situations where we need to check between SME vs SEV (e.g. DMA operation
setup or decrypting the trampoline area) and act accordingly.

Thanks,
Tom



Having the larger SEV preparation part on the kvm host side is perfectly
fine. But I'd like to keep kernel initialization paths clean.

Thanks.


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [RFC PATCH v2 12/32] x86: Add early boot support when running with SEV active

2017-03-16 Thread Borislav Petkov
On Thu, Mar 16, 2017 at 11:11:26AM -0500, Tom Lendacky wrote:
> Not quite. The guest still needs to understand about the encryption mask
> so that it can protect memory by setting the encryption mask in the
> pagetable entries.  It can also decide when to share memory with the
> hypervisor by not setting the encryption mask in the pagetable entries.

Ok, so the kernel - by that I mean both the baremetal and guest kernel -
needs to know whether we're encrypting stuff. So it needs to know about
SME.

> "Instruction fetches are always decrypted under SEV" means that,
> regardless of how a virtual address is mapped, encrypted or decrypted,
> if an instruction fetch is performed by the CPU from that address it
> will always be decrypted. This is to prevent the hypervisor from
> injecting executable code into the guest since it would have to be
> valid encrypted instructions.

Ok, so the guest needs to map its pages encrypted.

Which reminds me, KSM might be a PITA to enable with SEV but that's a
different story. :)

> There are many areas that use the same logic, but there are certain
> situations where we need to check between SME vs SEV (e.g. DMA operation
> setup or decrypting the trampoline area) and act accordingly.

Right, and I'd like to keep those areas where it differs at minimum and
nicely cordoned off from the main paths.

So looking back at the current patch in this subthread:

we do check

* CPUID 0x4000
* 8000_001F[EAX] for SME
* 8000_001F[EBX][5:0] for the encryption bits.

So how about we generate the following CPUID picture for the guest:

CPUID_Fn801F_EAX = ...10b

That is, SME bit is cleared, SEV is set. This will mean for the guest
kernel that SEV is enabled and you can avoid yourself the 0x4000
leaf check and the additional KVM feature bit glue.

10b configuration will be invalid for baremetal as - I'm assuming - you
can't have SEV=1b with SME=0b. It will be a virt-only configuration and
this way you can even avoid the hypervisor-specific detection but do
that for all.

Hmmm?

-- 
Regards/Gruss,
Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 
(AG Nürnberg)
-- 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 4/6] staging: media: atomisp: fix build when PM is disabled

2017-03-16 Thread Alan Cox
> + if (IS_ENABLED(CONFI_PM)) {

I think not.

Please actually test build both ways at the very least.

Alan

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 5/6] staging: media: atomisp: fill properly hmm_bo_type_strings when ION is disabled

2017-03-16 Thread Alan Cox
On Wed, 2017-03-15 at 14:09 -0400, Jérémy Lefaure wrote:
> When CONFIG_ION is disabled, HMM_BO_LAST is 3. In this case, "i"
> should
> not be added in hmm_bo_type_strings.

No the goal is to remove ifdefs - that's not the way to go with driver
clean up. Instead the array size should always cover the four strings.

Alan

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [bug report] staging/atomisp: Add support for the Intel IPU v2

2017-03-16 Thread Alan Cox
>    457  if (ret)
>    458  return ret;
>   ^^
> We're returning directly with the lock held.  We should either unlock
> before returning or do a goto out but I'm not sure which.


unlock and return (although clearly this path never happens in the real
world since that bug is in lots of product 8)).

Added to my queue.

Alan

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


RE: [PATCH 08/29] drivers, md: convert mddev.active from atomic_t to refcount_t

2017-03-16 Thread Reshetova, Elena
> On Tue, 2017-03-14 at 12:29 +, Reshetova, Elena wrote:
> > > Elena Reshetova  writes:
> > >
> > > > refcount_t type and corresponding API should be
> > > > used instead of atomic_t when the variable is used as
> > > > a reference counter. This allows to avoid accidental
> > > > refcounter overflows that might lead to use-after-free
> > > > situations.
> > > >
> > > > Signed-off-by: Elena Reshetova 
> > > > Signed-off-by: Hans Liljestrand 
> > > > Signed-off-by: Kees Cook 
> > > > Signed-off-by: David Windsor 
> > > > ---
> > > >  drivers/md/md.c | 6 +++---
> > > >  drivers/md/md.h | 3 ++-
> > > >  2 files changed, 5 insertions(+), 4 deletions(-)
> > >
> > > When booting linux-next (specifically 5be4921c9958ec) I'm seeing
> > > the
> > > backtrace below. I suspect this patch is just exposing an existing
> > > issue?
> >
> > Yes, we have actually been following this issue in the another
> > thread.
> > It looks like the object is re-used somehow, but I can't quite
> > understand how just by reading the code.
> > This was what I put into the previous thread:
> >
> > "The log below indicates that you are using your refcounter in a bit
> > weird way in mddev_find().
> > However, I can't find the place (just by reading the code) where you
> > would increment refcounter from zero (vs. setting it to one).
> > It looks like you either iterate over existing nodes (and increment
> > their counters, which should be >= 1 at the time of increment) or
> > create a new node, but then mddev_init() sets the counter to 1. "
> >
> > If you can help to understand what is going on with the object
> > creation/destruction, would be appreciated!
> >
> > Also Shaohua Li stopped this patch coming from his tree since the
> > issue was caught at that time, so we are not going to merge this
> > until we figure it out.
> 
> Asking on the correct list (dm-devel) would have got you the easy
> answer:  The refcount behind mddev->active is a genuine atomic.  It has
> refcount properties but only if the array fails to initialise (in that
> case, final put kills it).  Once it's added to the system as a gendisk,
> it cannot be freed until md_free().  Thus its ->active count can go to
> zero (when it becomes inactive; usually because of an unmount). On a
> simple allocation regardless of outcome, the last executed statement in
> md_alloc is mddev_put(): that destroys the device if we didn't manage
> to create it or returns 0 and adds an inactive device to the system
> which the user can get with mddev_find().

Thank you James for explaining this! I guess in this case, the conversion 
doesn't make sense. 
And sorry about not asking in a correct place: we are handling many similar 
patches now and while I try to reach the right audience using get_maintainer 
script, it doesn't always succeeds. 

Best Regards,
Elena.

> 
> James
> 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [RFC PATCH v2 32/32] x86: kvm: Pin the guest memory when SEV is active

2017-03-16 Thread Brijesh Singh



On 03/16/2017 05:38 AM, Paolo Bonzini wrote:



On 02/03/2017 16:18, Brijesh Singh wrote:

The SEV memory encryption engine uses a tweak such that two identical
plaintexts at different location will have a different ciphertexts.
So swapping or moving ciphertexts of two pages will not result in
plaintexts being swapped. Relocating (or migrating) a physical backing pages
for SEV guest will require some additional steps. The current SEV key
management spec [1] does not provide commands to swap or migrate (move)
ciphertexts. For now we pin the memory allocated for the SEV guest. In
future when SEV key management spec provides the commands to support the
page migration we can update the KVM code to remove the pinning logical
without making any changes into userspace (qemu).

The patch pins userspace memory when a new slot is created and unpin the
memory when slot is removed.

[1] http://support.amd.com/TechDocs/55766_SEV-KM%20API_Spec.pdf


This is not enough, because memory can be hidden temporarily from the
guest and remapped later.  Think of a PCI BAR that is backed by RAM, or
also SMRAM.  The pinning must be kept even in that case.

You need to add a pair of KVM_MEMORY_ENCRYPT_OPs (one that doesn't map
to a PSP operation), such as KVM_REGISTER/UNREGISTER_ENCRYPTED_RAM.  In
QEMU you can use a RAMBlockNotifier to invoke the ioctls.



I was hoping to avoid adding new ioctl, but I see your point. Will add a pair 
of ioctl's
and use RAMBlocNotifier to invoke those ioctls.

-Brijesh
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [RFC PATCH v2 26/32] kvm: svm: Add support for SEV LAUNCH_UPDATE_DATA command

2017-03-16 Thread Brijesh Singh


On 03/16/2017 05:48 AM, Paolo Bonzini wrote:



On 02/03/2017 16:17, Brijesh Singh wrote:

+static struct page **sev_pin_memory(unsigned long uaddr, unsigned long ulen,
+   unsigned long *n)
+{
+   struct page **pages;
+   int first, last;
+   unsigned long npages, pinned;
+
+   /* Get number of pages */
+   first = (uaddr & PAGE_MASK) >> PAGE_SHIFT;
+   last = ((uaddr + ulen - 1) & PAGE_MASK) >> PAGE_SHIFT;
+   npages = (last - first + 1);
+
+   pages = kzalloc(npages * sizeof(struct page *), GFP_KERNEL);
+   if (!pages)
+   return NULL;
+
+   /* pin the user virtual address */
+   down_read(¤t->mm->mmap_sem);
+   pinned = get_user_pages_fast(uaddr, npages, 1, pages);
+   up_read(¤t->mm->mmap_sem);


get_user_pages_fast, like get_user_pages_unlocked, must be called
without mmap_sem held.


Sure.




+   if (pinned != npages) {
+   printk(KERN_ERR "SEV: failed to pin  %ld pages (got %ld)\n",
+   npages, pinned);
+   goto err;
+   }
+
+   *n = npages;
+   return pages;
+err:
+   if (pinned > 0)
+   release_pages(pages, pinned, 0);
+   kfree(pages);
+
+   return NULL;
+}

+   /* the array of pages returned by get_user_pages() is a page-aligned
+* memory. Since the user buffer is probably not page-aligned, we need
+* to calculate the offset within a page for first update entry.
+*/
+   offset = uaddr & (PAGE_SIZE - 1);
+   len = min_t(size_t, (PAGE_SIZE - offset), ulen);
+   ulen -= len;
+
+   /* update first page -
+* special care need to be taken for the first page because we might
+* be dealing with offset within the page
+*/


No need to special case the first page; just set "offset = 0" inside the
loop after the first iteration.



Will do.

-Brijesh
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [RFC PATCH v2 14/32] x86: mm: Provide support to use memblock when spliting large pages

2017-03-16 Thread Borislav Petkov
On Fri, Mar 10, 2017 at 04:41:56PM -0600, Brijesh Singh wrote:
> I can take a look at fixing those warning. In my initial attempt was to create
> a new function to clear encryption bit but it ended up looking very similar to
> __change_page_attr_set_clr() hence decided to extend the exiting function to
> use memblock_alloc().

... except that having all that SEV-specific code in main code paths is
yucky and I'd like to avoid it, if possible.

> Early in boot process, guest kernel allocates some structure (its either
> statically allocated or dynamic allocated via memblock_alloc). And shares the 
> physical
> address of these structure with hypervisor. Since entire guest memory area is 
> mapped
> as encrypted hence those structure's are mapped as encrypted memory range. We 
> need
> a method to clear the encryption bit. Sometime these structure maybe part of 
> 2M pages
> and need to split into smaller pages.

So how hard would it be if the hypervisor allocated that memory for the
guest instead? It would allocate it decrypted and guest would need to
access it decrypted too. All in preparation for SEV-ES which will need a
block of unencrypted memory for the guest anyway...

> In most cases, guest and hypervisor communication starts as soon as guest 
> provides
> the physical address to hypervisor. So we must map the pages as decrypted 
> before
> sharing the physical address to hypervisor.

See above: so purely theoretically speaking, the hypervisor could prep
that decrypted range for the guest. I'd look in Paolo's direction,
though, for the feasibility of something like that.

Thanks.

-- 
Regards/Gruss,
Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 
(AG Nürnberg)
-- 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [RFC PATCH v2 30/32] kvm: svm: Add support for SEV DEBUG_ENCRYPT command

2017-03-16 Thread Brijesh Singh



On 03/16/2017 06:03 AM, Paolo Bonzini wrote:



On 02/03/2017 16:18, Brijesh Singh wrote:

+   data = (void *) get_zeroed_page(GFP_KERNEL);


The page does not need to be zeroed, does it?



No, we don't have to zero it. I will fix it.


+
+   if ((len & 15) || (dst_addr & 15)) {
+   /* if destination address and length are not 16-byte
+* aligned then:
+* a) decrypt destination page into temporary buffer
+* b) copy source data into temporary buffer at correct offset
+* c) encrypt temporary buffer
+*/
+   ret = __sev_dbg_decrypt_page(kvm, dst_addr, data, &argp->error);


Ah, I see now you're using this function here for read-modify-write.
data is already pinned here, so even if you keep the function it makes
sense to push pinning out of __sev_dbg_decrypt_page and into
sev_dbg_decrypt.


I can push out pinning part outside __sev_dbg_decrypt_page




+   if (ret)
+   goto err_3;
+   d_off = dst_addr & (PAGE_SIZE - 1);
+
+   if (copy_from_user(data + d_off,
+   (uint8_t *)debug.src_addr, len)) {
+   ret = -EFAULT;
+   goto err_3;
+   }
+
+   encrypt->length = PAGE_SIZE;


Why decrypt/re-encrypt all the page instead of just the 16 byte area
around the [dst_addr, dst_addr+len) range?



good catch, I should be fine just decrypting a 16 byte area. Will fix in next 
rev


+   encrypt->src_addr = __psp_pa(data);
+   encrypt->dst_addr =  __sev_page_pa(inpages[0]);
+   } else {
+   if (copy_from_user(data, (uint8_t *)debug.src_addr, len)) {
+   ret = -EFAULT;
+   goto err_3;
+   }


Do you need copy_from_user, or can you just pin/unpin memory as for
DEBUG_DECRYPT?



We can work either with pin/unpin or copy_from_user. I think I choose 
copy_from_user because
in most of time ENCRYPT path was used when I set breakpoint through gdb which 
basically
requires copying pretty small data into guest memory. It may be very much 
possible that
someone can try to copy lot more data and then pin/unpin can speedup the things.

-Brijesh
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [RFC PATCH v2 29/32] kvm: svm: Add support for SEV DEBUG_DECRYPT command

2017-03-16 Thread Brijesh Singh



On 03/16/2017 05:54 AM, Paolo Bonzini wrote:



On 02/03/2017 16:18, Brijesh Singh wrote:

+static int __sev_dbg_decrypt_page(struct kvm *kvm, unsigned long src,
+   void *dst, int *error)
+{
+   inpages = sev_pin_memory(src, PAGE_SIZE, &npages);
+   if (!inpages) {
+   ret = -ENOMEM;
+   goto err_1;
+   }
+
+   data->handle = sev_get_handle(kvm);
+   data->dst_addr = __psp_pa(dst);
+   data->src_addr = __sev_page_pa(inpages[0]);
+   data->length = PAGE_SIZE;
+
+   ret = sev_issue_cmd(kvm, SEV_CMD_DBG_DECRYPT, data, error);
+   if (ret)
+   printk(KERN_ERR "SEV: DEBUG_DECRYPT %d (%#010x)\n",
+   ret, *error);
+   sev_unpin_memory(inpages, npages);
+err_1:
+   kfree(data);
+   return ret;
+}
+
+static int sev_dbg_decrypt(struct kvm *kvm, struct kvm_sev_cmd *argp)
+{
+   void *data;
+   int ret, offset, len;
+   struct kvm_sev_dbg debug;
+
+   if (!sev_guest(kvm))
+   return -ENOTTY;
+
+   if (copy_from_user(&debug, (void *)argp->data,
+   sizeof(struct kvm_sev_dbg)))
+   return -EFAULT;
+   /*
+* TODO: add support for decrypting length which crosses the
+* page boundary.
+*/
+   offset = debug.src_addr & (PAGE_SIZE - 1);
+   if (offset + debug.length > PAGE_SIZE)
+   return -EINVAL;
+


Please do add it, it doesn't seem very different from what you're doing
in LAUNCH_UPDATE_DATA.  There's no need for a separate
__sev_dbg_decrypt_page function, you can just pin/unpin here and do a
per-page loop as in LAUNCH_UPDATE_DATA.



I can certainly add support to handle crossing the page boundary cases.
Should we limit the size to prevent user passing arbitrary long length
and we end up looping inside the kernel? I was thinking to limit to a PAGE_SIZE.

~ Brijesh
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: android: ion: reduce lock contention latency

2017-03-16 Thread kbuild test robot
Hi Junil,

[auto build test ERROR on staging/staging-testing]
[also build test ERROR on v4.11-rc2 next-20170310]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Junil-Lee/staging-android-ion-reduce-lock-contention-latency/20170317-021346
config: x86_64-randconfig-x004-201711 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/staging/android/ion/ion_page_pool.c: In function 
'ion_page_pool_remove':
>> drivers/staging/android/ion/ion_page_pool.c:68:9: error: assignment from 
>> incompatible pointer type [-Werror=incompatible-pointer-types]
   node = llist_entry((struct list_head *)node, struct page, lru);
^
   drivers/staging/android/ion/ion_page_pool.c:74:9: error: assignment from 
incompatible pointer type [-Werror=incompatible-pointer-types]
   node = llist_entry((struct list_head *)node, struct page, lru);
^
   cc1: some warnings being treated as errors

vim +68 drivers/staging/android/ion/ion_page_pool.c

62  struct llist_node *node;
63  
64  if (high) {
65  BUG_ON(!atomic_read(&pool->high_count));
66  node = llist_del_first(&pool->high_items);
67  if (node)
  > 68  node = llist_entry((struct list_head *)node, 
struct page, lru);
69  atomic_dec(&pool->high_count);
70  } else {
71  BUG_ON(!atomic_read(&pool->low_count));

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [RFC PATCH v2 05/32] x86: Use encrypted access of BOOT related data with SEV

2017-03-16 Thread Tom Lendacky

On 3/7/2017 5:09 AM, Borislav Petkov wrote:

On Thu, Mar 02, 2017 at 10:12:59AM -0500, Brijesh Singh wrote:

From: Tom Lendacky 

When Secure Encrypted Virtualization (SEV) is active, BOOT data (such as
EFI related data, setup data) is encrypted and needs to be accessed as
such when mapped. Update the architecture override in early_memremap to
keep the encryption attribute when mapping this data.


This could also explain why persistent memory needs to be accessed
decrypted with SEV.


I'll add some comments about why persistent memory needs to be accessed
decrypted (because the encryption key changes across reboots) for both
SME and SEV.



In general, what the difference in that aspect is in respect to SME. And
I'd write that in the comment over the function. And not say "E820 areas
are checked in making this determination." because that is visible but
say *why* we need to check those ranges and determine access depending
on their type.


Will do.

Thanks,
Tom




___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] HV: properly delay KVP packets when negotiation is in progress

2017-03-16 Thread Long Li
The host may send multiple KVP packets before the negotiation with daemon
is finished. We need to keep those packets in ring buffer until the daemon
is negotiated and connected.

This patch is based on the work of Nick Meier 

Signed-off-by: Long Li 
---
 drivers/hv/hv_kvp.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c
index de26371..b9f928d 100644
--- a/drivers/hv/hv_kvp.c
+++ b/drivers/hv/hv_kvp.c
@@ -628,16 +628,17 @@ void hv_kvp_onchannelcallback(void *context)
 NEGO_IN_PROGRESS,
 NEGO_FINISHED} host_negotiatied = NEGO_NOT_STARTED;
 
-   if (host_negotiatied == NEGO_NOT_STARTED &&
-   kvp_transaction.state < HVUTIL_READY) {
+   if (kvp_transaction.state < HVUTIL_READY) {
/*
 * If userspace daemon is not connected and host is asking
 * us to negotiate we need to delay to not lose messages.
 * This is important for Failover IP setting.
 */
-   host_negotiatied = NEGO_IN_PROGRESS;
-   schedule_delayed_work(&kvp_host_handshake_work,
+   if (host_negotiatied == NEGO_NOT_STARTED) {
+   host_negotiatied = NEGO_IN_PROGRESS;
+   schedule_delayed_work(&kvp_host_handshake_work,
  HV_UTIL_NEGO_TIMEOUT * HZ);
+   }
return;
}
if (kvp_transaction.state > HVUTIL_READY)
-- 
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [RFC PATCH v2 08/32] x86: Use PAGE_KERNEL protection for ioremap of memory page

2017-03-16 Thread Tom Lendacky

On 3/7/2017 8:59 AM, Borislav Petkov wrote:

On Thu, Mar 02, 2017 at 10:13:32AM -0500, Brijesh Singh wrote:

From: Tom Lendacky 

In order for memory pages to be properly mapped when SEV is active, we
need to use the PAGE_KERNEL protection attribute as the base protection.
This will insure that memory mapping of, e.g. ACPI tables, receives the
proper mapping attributes.

Signed-off-by: Tom Lendacky 
---



diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index c400ab5..481c999 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -151,7 +151,15 @@ static void __iomem *__ioremap_caller(resource_size_t 
phys_addr,
pcm = new_pcm;
}

+   /*
+* If the page being mapped is in memory and SEV is active then
+* make sure the memory encryption attribute is enabled in the
+* resulting mapping.
+*/
prot = PAGE_KERNEL_IO;
+   if (sev_active() && page_is_mem(pfn))


Hmm, a resource tree walk per ioremap call. This could get expensive for
ioremap-heavy workloads.

__ioremap_caller() gets called here during boot 55 times so not a whole
lot but I wouldn't be surprised if there were some nasty use cases which
ioremap a lot.

...


diff --git a/kernel/resource.c b/kernel/resource.c
index 9b5f044..db56ba3 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -518,6 +518,46 @@ int __weak page_is_ram(unsigned long pfn)
 }
 EXPORT_SYMBOL_GPL(page_is_ram);

+/*
+ * This function returns true if the target memory is marked as
+ * IORESOURCE_MEM and IORESOUCE_BUSY and described as other than
+ * IORES_DESC_NONE (e.g. IORES_DESC_ACPI_TABLES).
+ */
+static int walk_mem_range(unsigned long start_pfn, unsigned long nr_pages)
+{
+   struct resource res;
+   unsigned long pfn, end_pfn;
+   u64 orig_end;
+   int ret = -1;
+
+   res.start = (u64) start_pfn << PAGE_SHIFT;
+   res.end = ((u64)(start_pfn + nr_pages) << PAGE_SHIFT) - 1;
+   res.flags = IORESOURCE_MEM | IORESOURCE_BUSY;
+   orig_end = res.end;
+   while ((res.start < res.end) &&
+   (find_next_iomem_res(&res, IORES_DESC_NONE, true) >= 0)) {
+   pfn = (res.start + PAGE_SIZE - 1) >> PAGE_SHIFT;
+   end_pfn = (res.end + 1) >> PAGE_SHIFT;
+   if (end_pfn > pfn)
+   ret = (res.desc != IORES_DESC_NONE) ? 1 : 0;
+   if (ret)
+   break;
+   res.start = res.end + 1;
+   res.end = orig_end;
+   }
+   return ret;
+}


So the relevant difference between this one and walk_system_ram_range()
is this:

-   ret = (*func)(pfn, end_pfn - pfn, arg);
+   ret = (res.desc != IORES_DESC_NONE) ? 1 : 0;

so it seems to me you can have your own *func() pointer which does that
IORES_DESC_NONE comparison. And then you can define your own workhorse
__walk_memory_range() which gets called by both walk_mem_range() and
walk_system_ram_range() instead of almost duplicating them.

And looking at walk_system_ram_res(), that one looks similar too except
the pfn computation. But AFAICT the pfn/end_pfn things are computed from
res.start and res.end so it looks to me like all those three functions
are crying for unification...


I'll take a look at what it takes to consolidate these with a pre-patch. 
Then I'll add the new support.


Thanks,
Tom




___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 1/2 v4] pci-hyperv: properly handle pci bus remove

2017-03-16 Thread Bjorn Helgaas
On Tue, Feb 28, 2017 at 02:19:45AM +, Long Li wrote:
> hv_pci_devices_present is called in hv_pci_remove when we remove a PCI
> device from host (e.g. by disabling SRIOV on a device). In hv_pci_remove,
> the bus is already removed before the call, so we don't need to rescan 
> the bus in the workqueue scheduled from hv_pci_devices_present. By 
> introducing status hv_pcibus_removed, we can avoid this situation.
> 
> Signed-off-by: Long Li 
> Reported-by: Xiaofeng Wang 
> Acked-by: K. Y. Srinivasan 

This didn't apply for me because saving it to a file resulted in some
encoded file with "=3D" instead of "=".  I see that mutt is smart enough to
deal with that in this reply, so there's probably a way for it to decode it
when saving to a file, but I don't know it.

I tried to apply it by hand, but the hunk in hv_pci_remove() doesn't match
the context.  I think your patch is based on something previous to
17978524a636 ("PCI: hv: Fix hv_pci_remove() for hot-remove").  Please
refresh the patch so it applies to my "master" branch (currently
v4.11-rc1).

Also, the "default: break;" case below is redundant and can be removed.

So I'll wait for your updated versions of both these patches.

> ---
>  drivers/pci/host/pci-hyperv.c | 20 +---
>  1 file changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
> index a8deeca..4a37598 100644
> --- a/drivers/pci/host/pci-hyperv.c
> +++ b/drivers/pci/host/pci-hyperv.c
> @@ -348,6 +348,7 @@ enum hv_pcibus_state {
>   hv_pcibus_init = 0,
>   hv_pcibus_probed,
>   hv_pcibus_installed,
> + hv_pcibus_removed,
>   hv_pcibus_maximum
>  };
>  
> @@ -1481,13 +1482,24 @@ static void pci_devices_present_work(struct 
> work_struct *work)
>   put_pcichild(hpdev, hv_pcidev_ref_initial);
>   }
>  
> - /* Tell the core to rescan bus because there may have been changes. */
> - if (hbus->state == hv_pcibus_installed) {
> + switch (hbus->state) {
> + case hv_pcibus_installed:
> + /*
> +  * Tell the core to rescan bus
> +  * because there may have been changes.
> +  */
>   pci_lock_rescan_remove();
>   pci_scan_child_bus(hbus->pci_bus);
>   pci_unlock_rescan_remove();
> - } else {
> + break;
> +
> + case hv_pcibus_init:
> + case hv_pcibus_probed:
>   survey_child_resources(hbus);
> + break;
> +
> + default:
> + break;

^ This is redundant.

>   }
>  
>   up(&hbus->enum_sem);
> @@ -2163,6 +2175,7 @@ static int hv_pci_probe(struct hv_device *hdev,
>   hbus = kzalloc(sizeof(*hbus), GFP_KERNEL);
>   if (!hbus)
>   return -ENOMEM;
> + hbus->state = hv_pcibus_init;
>  
>   /*
>* The PCI bus "domain" is what is called "segment" in ACPI and
> @@ -2305,6 +2318,7 @@ static int hv_pci_remove(struct hv_device *hdev)
>   pci_stop_root_bus(hbus->pci_bus);
>   pci_remove_root_bus(hbus->pci_bus);
>   pci_unlock_rescan_remove();
> + hbus->state = hv_pcibus_removed;
>   }
>  
>   ret = hv_send_resources_released(hdev);
> -- 
> 1.8.5.6
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] HV: properly delay KVP packets when negotiation is in progress

2017-03-16 Thread Joshua R. Poulson
This should be submitted to stable, we're seeing KVP fail to start in
some tests of 4.10.

On Thu, Mar 16, 2017 at 12:51 PM, Long Li  wrote:
> The host may send multiple KVP packets before the negotiation with daemon
> is finished. We need to keep those packets in ring buffer until the daemon
> is negotiated and connected.
>
> This patch is based on the work of Nick Meier 
>
> Signed-off-by: Long Li 
> ---
>  drivers/hv/hv_kvp.c | 9 +
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c
> index de26371..b9f928d 100644
> --- a/drivers/hv/hv_kvp.c
> +++ b/drivers/hv/hv_kvp.c
> @@ -628,16 +628,17 @@ void hv_kvp_onchannelcallback(void *context)
>  NEGO_IN_PROGRESS,
>  NEGO_FINISHED} host_negotiatied = NEGO_NOT_STARTED;
>
> -   if (host_negotiatied == NEGO_NOT_STARTED &&
> -   kvp_transaction.state < HVUTIL_READY) {
> +   if (kvp_transaction.state < HVUTIL_READY) {
> /*
>  * If userspace daemon is not connected and host is asking
>  * us to negotiate we need to delay to not lose messages.
>  * This is important for Failover IP setting.
>  */
> -   host_negotiatied = NEGO_IN_PROGRESS;
> -   schedule_delayed_work(&kvp_host_handshake_work,
> +   if (host_negotiatied == NEGO_NOT_STARTED) {
> +   host_negotiatied = NEGO_IN_PROGRESS;
> +   schedule_delayed_work(&kvp_host_handshake_work,
>   HV_UTIL_NEGO_TIMEOUT * HZ);
> +   }
> return;
> }
> if (kvp_transaction.state > HVUTIL_READY)
> --
> 2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v5 01/39] [media] dt-bindings: Add bindings for video-multiplexer device

2017-03-16 Thread Rob Herring
On Thu, Mar 09, 2017 at 08:52:41PM -0800, Steve Longerbeam wrote:
> From: Philipp Zabel 
> 
> Add bindings documentation for the video multiplexer device.
> 
> Signed-off-by: Sascha Hauer 
> Signed-off-by: Philipp Zabel 
> Signed-off-by: Steve Longerbeam 
> ---
>  .../bindings/media/video-multiplexer.txt   | 59 
> ++
>  1 file changed, 59 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/media/video-multiplexer.txt

Acked-by: Rob Herring 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: android: ion: reduce lock contention latency

2017-03-16 Thread kbuild test robot
Hi Junil,

[auto build test WARNING on staging/staging-testing]
[also build test WARNING on v4.11-rc2 next-20170310]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Junil-Lee/staging-android-ion-reduce-lock-contention-latency/20170317-021346
config: i386-randconfig-i0-201711 (attached as .config)
compiler: gcc-4.8 (Debian 4.8.4-1) 4.8.4
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All warnings (new ones prefixed by >>):

   drivers/staging/android/ion/ion_page_pool.c: In function 
'ion_page_pool_remove':
>> drivers/staging/android/ion/ion_page_pool.c:68:9: warning: assignment from 
>> incompatible pointer type [enabled by default]
   node = llist_entry((struct list_head *)node, struct page, lru);
^
   drivers/staging/android/ion/ion_page_pool.c:74:9: warning: assignment from 
incompatible pointer type [enabled by default]
   node = llist_entry((struct list_head *)node, struct page, lru);
^

vim +68 drivers/staging/android/ion/ion_page_pool.c

52  llist_add((struct llist_node *)&page->lru, 
&pool->low_items);
53  atomic_inc(&pool->low_count);
54  }
55  
56  return 0;
57  }
58  
59  static struct page *ion_page_pool_remove(struct ion_page_pool *pool, 
bool high)
60  {
61  struct page *page = NULL;
62  struct llist_node *node;
63  
64  if (high) {
65  BUG_ON(!atomic_read(&pool->high_count));
66  node = llist_del_first(&pool->high_items);
67  if (node)
  > 68  node = llist_entry((struct list_head *)node, 
struct page, lru);
69  atomic_dec(&pool->high_count);
70  } else {
71  BUG_ON(!atomic_read(&pool->low_count));
72  node = llist_del_first(&pool->low_items);
73  if (node)
74  node = llist_entry((struct list_head *)node, 
struct page, lru);
75  atomic_dec(&pool->low_count);
76  }

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


PATCH for style warnings in drivers/staging/most/aim-v4l2

2017-03-16 Thread Chandra Annamaneni


Enclosed is a patch to the file video.c. It only fixes style warning 
flagged by checkpatch.pl.


Please let me know if anything else needs to be done.

Thanks.
Chandra


diff --git a/drivers/staging/most/aim-v4l2/video.c 
b/drivers/staging/most/aim-v4l2/video.c
index e074841..59e861e 100644
--- a/drivers/staging/most/aim-v4l2/video.c
+++ b/drivers/staging/most/aim-v4l2/video.c
@@ -79,7 +79,7 @@ static int aim_vdev_open(struct file *filp)
struct most_video_dev *mdev = video_drvdata(filp);
struct aim_fh *fh;

-   v4l2_info(&mdev->v4l2_dev, "aim_vdev_open()\n");
+   v4l2_info(&mdev->v4l2_dev, "%s()\n", __func__);

switch (vdev->vfl_type) {
case VFL_TYPE_GRABBER:
@@ -128,7 +128,7 @@ static int aim_vdev_close(struct file *filp)
struct most_video_dev *mdev = fh->mdev;
struct mbo *mbo, *tmp;

-   v4l2_info(&mdev->v4l2_dev, "aim_vdev_close()\n");
+   v4l2_info(&mdev->v4l2_dev, "%s()\n", __func__);

/*
 * We need to put MBOs back before we call most_stop_channel()
@@ -324,7 +324,7 @@ static int vidioc_g_std(struct file *file, void *priv, 
v4l2_std_id *norm)
struct aim_fh *fh = priv;
struct most_video_dev *mdev = fh->mdev;

-   v4l2_info(&mdev->v4l2_dev, "vidioc_g_std()\n");
+   v4l2_info(&mdev->v4l2_dev, "%s()\n", __func__);

*norm = V4L2_STD_UNKNOWN;
return 0;
@@ -361,7 +361,7 @@ static int vidioc_s_input(struct file *file, void *priv, 
unsigned int index)
struct aim_fh *fh = priv;
struct most_video_dev *mdev = fh->mdev;

-   v4l2_info(&mdev->v4l2_dev, "vidioc_s_input(%d)\n", index);
+   v4l2_info(&mdev->v4l2_dev, "%s(%d)\n", __func__, index);

if (index >= V4L2_AIM_MAX_INPUT)
return -EINVAL;
@@ -441,7 +441,7 @@ static int aim_register_videodev(struct most_video_dev 
*mdev)
 {
int ret;

-   v4l2_info(&mdev->v4l2_dev, "aim_register_videodev()\n");
+   v4l2_info(&mdev->v4l2_dev, "%s()\n", __func__);

init_waitqueue_head(&mdev->wait_data);

@@ -471,7 +471,7 @@ static int aim_register_videodev(struct most_video_dev 
*mdev)

 static void aim_unregister_videodev(struct most_video_dev *mdev)
 {
-   v4l2_info(&mdev->v4l2_dev, "aim_unregister_videodev()\n");
+   v4l2_info(&mdev->v4l2_dev, "%s()\n", __func__);

video_unregister_device(mdev->vdev);
 }
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


RE: [PATCH 1/2 v4] pci-hyperv: properly handle pci bus remove

2017-03-16 Thread Long Li
> -Original Message-
> From: Bjorn Helgaas [mailto:helg...@kernel.org]
> Sent: Thursday, March 16, 2017 1:07 PM
> To: Long Li 
> Cc: KY Srinivasan ; Haiyang Zhang
> ; Bjorn Helgaas ;
> de...@linuxdriverproject.org; linux-...@vger.kernel.org; linux-
> ker...@vger.kernel.org
> Subject: Re: [PATCH 1/2 v4] pci-hyperv: properly handle pci bus remove
> 
> On Tue, Feb 28, 2017 at 02:19:45AM +, Long Li wrote:
> > hv_pci_devices_present is called in hv_pci_remove when we remove a PCI
> > device from host (e.g. by disabling SRIOV on a device). In
> > hv_pci_remove, the bus is already removed before the call, so we don't
> > need to rescan the bus in the workqueue scheduled from
> > hv_pci_devices_present. By introducing status hv_pcibus_removed, we
> can avoid this situation.
> >
> > Signed-off-by: Long Li 
> > Reported-by: Xiaofeng Wang 
> > Acked-by: K. Y. Srinivasan 
> 
> This didn't apply for me because saving it to a file resulted in some encoded
> file with "=3D" instead of "=".  I see that mutt is smart enough to deal with
> that in this reply, so there's probably a way for it to decode it when saving 
> to
> a file, but I don't know it.
> 
> I tried to apply it by hand, but the hunk in hv_pci_remove() doesn't match
> the context.  I think your patch is based on something previous to
> 17978524a636 ("PCI: hv: Fix hv_pci_remove() for hot-remove").  Please
> refresh the patch so it applies to my "master" branch (currently v4.11-rc1).
> 
> Also, the "default: break;" case below is redundant and can be removed.
> 
> So I'll wait for your updated versions of both these patches.

Thanks, I'll address those issues and resend the patch.

> 
> > ---
> >  drivers/pci/host/pci-hyperv.c | 20 +---
> >  1 file changed, 17 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/pci/host/pci-hyperv.c
> > b/drivers/pci/host/pci-hyperv.c index a8deeca..4a37598 100644
> > --- a/drivers/pci/host/pci-hyperv.c
> > +++ b/drivers/pci/host/pci-hyperv.c
> > @@ -348,6 +348,7 @@ enum hv_pcibus_state {
> > hv_pcibus_init = 0,
> > hv_pcibus_probed,
> > hv_pcibus_installed,
> > +   hv_pcibus_removed,
> > hv_pcibus_maximum
> >  };
> >
> > @@ -1481,13 +1482,24 @@ static void pci_devices_present_work(struct
> work_struct *work)
> > put_pcichild(hpdev, hv_pcidev_ref_initial);
> > }
> >
> > -   /* Tell the core to rescan bus because there may have been changes.
> */
> > -   if (hbus->state == hv_pcibus_installed) {
> > +   switch (hbus->state) {
> > +   case hv_pcibus_installed:
> > +   /*
> > +* Tell the core to rescan bus
> > +* because there may have been changes.
> > +*/
> > pci_lock_rescan_remove();
> > pci_scan_child_bus(hbus->pci_bus);
> > pci_unlock_rescan_remove();
> > -   } else {
> > +   break;
> > +
> > +   case hv_pcibus_init:
> > +   case hv_pcibus_probed:
> > survey_child_resources(hbus);
> > +   break;
> > +
> > +   default:
> > +   break;
> 
> ^ This is redundant.
> 
> > }
> >
> > up(&hbus->enum_sem);
> > @@ -2163,6 +2175,7 @@ static int hv_pci_probe(struct hv_device *hdev,
> > hbus = kzalloc(sizeof(*hbus), GFP_KERNEL);
> > if (!hbus)
> > return -ENOMEM;
> > +   hbus->state = hv_pcibus_init;
> >
> > /*
> >  * The PCI bus "domain" is what is called "segment" in ACPI and @@
> > -2305,6 +2318,7 @@ static int hv_pci_remove(struct hv_device *hdev)
> > pci_stop_root_bus(hbus->pci_bus);
> > pci_remove_root_bus(hbus->pci_bus);
> > pci_unlock_rescan_remove();
> > +   hbus->state = hv_pcibus_removed;
> > }
> >
> > ret = hv_send_resources_released(hdev);
> > --
> > 1.8.5.6
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] Staging: wilc1000: fix two typos in #define's

2017-03-16 Thread Dylan Leggio
Signed-off-by: Dylan Leggio 
---
 drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c 
b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
index a37896fcd683..1bb63702c623 100644
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
@@ -29,8 +29,8 @@
 #define P2P_INV_REQ0x03
 #define P2P_INV_RSP0x04
 #define PUBLIC_ACT_VENDORSPEC  0x09
-#define GAS_INTIAL_REQ 0x0a
-#define GAS_INTIAL_RSP 0x0b
+#define GAS_INITIAL_REQ0x0a
+#define GAS_INITIAL_RSP0x0b
 
 #define INVALID_CHANNEL0
 
@@ -1477,10 +1477,10 @@ void WILC_WFI_p2p_rx(struct net_device *dev, u8 *buff, 
u32 size)
}
if (buff[ACTION_CAT_ID] == PUB_ACTION_ATTR_ID) {
switch (buff[ACTION_SUBTYPE_ID]) {
-   case GAS_INTIAL_REQ:
+   case GAS_INITIAL_REQ:
break;
 
-   case GAS_INTIAL_RSP:
+   case GAS_INITIAL_RSP:
break;
 
case PUBLIC_ACT_VENDORSPEC:
@@ -1666,10 +1666,10 @@ static int mgmt_tx(struct wiphy *wiphy,
curr_channel = chan->hw_value;
}
switch (buf[ACTION_SUBTYPE_ID]) {
-   case GAS_INTIAL_REQ:
+   case GAS_INITIAL_REQ:
break;
 
-   case GAS_INTIAL_RSP:
+   case GAS_INITIAL_RSP:
break;
 
case PUBLIC_ACT_VENDORSPEC:
-- 
2.12.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: media / v4l2-mc: wishlist for complex cameras (was Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline)

2017-03-16 Thread Pavel Machek
Hi!

> > > > mplayer is useful for testing... but that one already works (after you
> > > > setup the pipeline, and configure exposure/gain).
> > > > 
> > > > But thats useful for testing, not really for production. Image will be
> > > > out of focus and with wrong white balance.
> > > > 
> > > > What I would really like is an application to get still photos. For
> > > > taking pictures with manual settings we need
> > > > 
> > > > a) units for controls: user wants to focus on 1m, and take picture
> > > > with ISO200, 1/125 sec. We should also tell him that lens is f/5.6 and
> > > > focal length is 20mm with 5mm chip.
> > > > 
> > > > But... autofocus/autogain would really be good to have. Thus we need:
> > > > 
> > > > b) for each frame, we need exposure settings and focus position at
> > > > time frame was taken. Otherwise autofocus/autogain will be too
> > > > slow. At least focus position is going to be tricky -- either kernel
> > > > would have to compute focus position for us (not trivial) or we'd need
> > > > enough information to compute it in userspace.
> > > > 
> > > > There are more problems: hardware-accelerated preview is not trivial
> > > > to set up (and I'm unsure if it can be done in generic way). Still
> > > > photos application needs to switch resolutions between preview and
> > > > photo capture. Probably hardware-accelerated histograms are needed for
> > > > white balance, auto gain and auto focus, 
> > > > 
> > > > It seems like there's a _lot_ of stuff to be done before we have
> > > > useful support for complex cameras...  
> > > 
> > > Taking still pictures using a hardware-accelerated preview is
> > > a sophisticated use case. I don't know any userspace application
> > > that does that. Ok, several allow taking snapshots, by simply
> > > storing the image of the current frame.  
> > 
> > Well, there are applications that take still pictures. Android has
> > one. Maemo has another. Then there's fcam-dev. Its open source; with
> > modified kernel it is fully usable. I have version that runs on recent
> > nearly-mainline on N900. 
> 
> Hmm... it seems that FCam is specific for N900:
>   http://fcam.garage.maemo.org/
> 
> If so, then we have here just the opposite problem, if want it to be
> used as a generic application, as very likely it requires OMAP3-specific
> graph/subdevs.

Well... there's quick and great version on maemo.org. I do have local
version (still somehow N900-specific), but it no longer uses hardware
histogram/sharpness support. Should be almost generic.

> > So yes, I'd like solution for problems a) and b).

...but it has camera parameters hardcoded (problem a) and slow
(problem b). 

> > Question is if camera without autofocus is usable. I'd say "not
> > really".qv4l2
> 
> That actually depends on the sensor and how focus is adjusted.
> 
> I'm testing right now this camera module for RPi:
>https://www.raspberrypi.org/products/camera-module-v2/
> 
> I might be wrong, but this sensor doesn't seem to have auto-focus.
> Instead, it seems to use a wide-angle lens. So, except when the
> object is too close, the focus look OK.

Well, cameras without autofocus are somehow usable without
autofocus. But cameras with autofocus don't work too well without one.

> > If we really want to go that way (is not modifying library to access
> > the right files quite easy?), I believe non-confusing option would be
> > to have '/dev/video0 -- omap3 camera for legacy applications' which
> > would include all the controls.
> 
> Yeah, keeping /dev/video0 reserved for generic applications is something
> that could work. Not sure how easy would be to implement it.

Plus advanced applications would just ignore /dev/video0.. and not be confused.

> > > > > > You can get Nokia N900 on aliexpress. If not, they are still 
> > > > > > available
> > > > between people :-)  
> > > 
> > > I have one. Unfortunately, I never had a chance to use it, as the display
> > > stopped working one week after I get it.  
> > 
> > Well, I guess the easiest option is to just get another one :-).
> 
> :-)  Well, I guess very few units of N900 was sold in Brazil. Importing
> one is too expensive, due to taxes.

Try to ask at local mailing list. Those machines were quite common.

> > But otoh -- N900 is quite usable without the screen. 0x tool can
> > be used to boot the kernel, then you can use nfsroot and usb
> > networking. It also has serial port (over strange
> > connector). Connected over ssh over usb network is actually how I do
> > most of the v4l work.
> 
> If you pass me the pointers, I can try it when I have some time.

Ok, I guess I'll do that in private email.

> Anyway, I got myself an ISEE IGEPv2, with the expansion board:
>   https://www.isee.biz/products/igep-processor-boards/igepv2-dm3730
>   https://www.isee.biz/products/igep-expansion-boards/igepv2-expansion
> 
> The expansion board comes with a tvp5150 analog TV demod. So, with
> this device, I can simply connect it to a c

[PATCH v4 0/6] staging: rtl8192e: Fix coding style, warnings and checks

2017-03-16 Thread sunil . m
From: Suniel Mahesh 

Split earlier patches into multiple commits for easy review as
suggested by Dan Carpenter.
Modified subject, description and in few patches both for
better readability as suggested by Greg KH.
Dropped two patches from the earler series, as they were not adding 
significant value, suggested by Dan Carpenter.
Fixed the following issues reported by checkpatch.pl:
Block comments should align the * on each line, aligned.
Block comments use * on subsequent lines, other characters
are replaced by * .
Removed unnecessary 'out of memory' message.
Comparison's to NULL could be written '!foo' or 'foo', modified.
Replaced sizeof(struct foo) into sizeof(*ptr).
Spaces preferred around that 'operator', spacing provided.
Logical continuations should be on the previous line, modified accordingly.
Unnecessary parentheses around variables, removed.
Please use a blank line after function/struct/union/enum declarations, used.
Blank lines aren't necessary after an open brace '{' and before a
close brace '}', removed.
No space is necessary after a cast, removed.
Please don't use multiple blank lines, removed.

Rebased on top of next-20170310.
Patches were tested and built on next-20170310 and staging-testing
as suggested by Greg K-H, no errors reported.

Suniel Mahesh (6):
  staging: rtl8192e: Fix comments as per kernel coding style
  staging: rtl8192e: Remove unnecessary 'out of memory' message
  staging: rtl8192e: Rectify pointer comparisions with NULL
  staging: rtl8192e: Pass a pointer as an argument to sizeof() instead
of struct
  staging: rtl8192e: Fix issues reported by checkpatch.pl
  staging: rtl8192e: Fix blank lines and space after a cast

 drivers/staging/rtl8192e/rtl8192e/rtl_core.c | 141 ++-
 1 file changed, 50 insertions(+), 91 deletions(-)

-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v4 6/6] staging: rtl8192e: Fix blank lines and space after a cast

2017-03-16 Thread sunil . m
From: Suniel Mahesh 

Fixed the following checkpatch.pl checks:
Blank lines aren't necessary after an open brace '{'
and before a close brace '}', removed
No space is necessary after a cast, removed
Please don't use multiple blank lines, removed

Signed-off-by: Suniel Mahesh 
---
Changes for v4:

- Dropped two patches from the series, as they were not adding significant value
  suggested by Dan Carpenter.
  staging: rtl8192e: Fix coding style, this was fixing line over 80 characters.
  staging: rtl8192e: Fix unbalanced braces
- Resending the whole series as requested by Greg K-H
- Patches were tested and built on next-20170310 and staging-testing

Changes for v3:

- Split earlier patches into multiple commits for easy review
  as suggested by Greg K-H
- New patch addition to the series
- Rebased on top of next-20170310
- Patches were tested and built on next-20170310 and staging-testing
  as suggested by Greg K-H, no errors reported

Changes for v2:
---
 drivers/staging/rtl8192e/rtl8192e/rtl_core.c | 26 --
 1 file changed, 26 deletions(-)

diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_core.c 
b/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
index 1d22f18..bc2c732 100644
--- a/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
+++ b/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
@@ -37,7 +37,6 @@
 static int channels = 0x3fff;
 static char *ifname = "wlan%d";
 
-
 static const struct rtl819x_ops rtl819xp_ops = {
.nic_type   = NIC_8192E,
.get_eeprom_size= rtl92e_get_eeprom_size,
@@ -195,7 +194,6 @@ bool rtl92e_set_rf_state(struct net_device *dev,
priv->rtllib->RfOffReason = 0;
bActionAllowed = true;
 
-
if (rtState == eRfOff &&
ChangeSource >= RF_CHANGE_BY_HW)
bConnectBySSID = true;
@@ -242,7 +240,6 @@ bool rtl92e_set_rf_state(struct net_device *dev,
 StateToSet, priv->rtllib->RfOffReason);
PHY_SetRFPowerState(dev, StateToSet);
if (StateToSet == eRfOn) {
-
if (bConnectBySSID && priv->blinked_ingpio) {
schedule_delayed_work(
 &ieee->associate_procedure_wq, 0);
@@ -401,8 +398,6 @@ static void _rtl92e_qos_activate(void *data)
 
for (i = 0; i <  QOS_QUEUE_NUM; i++)
priv->rtllib->SetHwRegHandler(dev, HW_VAR_AC_PARAM, (u8 *)(&i));
-
-
 success:
mutex_unlock(&priv->mutex);
 }
@@ -462,7 +457,6 @@ static int _rtl92e_handle_beacon(struct net_device *dev,
 
schedule_delayed_work(&priv->update_beacon_wq, 0);
return 0;
-
 }
 
 static int _rtl92e_qos_assoc_resp(struct r8192_priv *priv,
@@ -888,7 +882,6 @@ static void _rtl92e_init_priv_constant(struct net_device 
*dev)
pPSC->RegMaxLPSAwakeIntvl = 5;
 }
 
-
 static void _rtl92e_init_priv_variable(struct net_device *dev)
 {
struct r8192_priv *priv = rtllib_priv(dev);
@@ -1211,7 +1204,6 @@ static enum reset_type _rtl92e_if_check_reset(struct 
net_device *dev)
} else {
return RESET_TYPE_NORESET;
}
-
 }
 
 static void _rtl92e_if_silent_reset(struct net_device *dev)
@@ -1223,7 +1215,6 @@ static void _rtl92e_if_silent_reset(struct net_device 
*dev)
unsigned long flag;
 
if (priv->ResetProgress == RESET_TYPE_NORESET) {
-
RT_TRACE(COMP_RESET, "=>Reset progress!!\n");
 
priv->ResetProgress = RESET_TYPE_SILENT;
@@ -1417,7 +1408,6 @@ static void _rtl92e_watchdog_wq_cb(void *data)
ieee->LinkDetectInfo.NumTxOkInPeriod > 100)
bBusyTraffic = true;
 
-
if (ieee->LinkDetectInfo.NumRxOkInPeriod > 4000 ||
ieee->LinkDetectInfo.NumTxOkInPeriod > 4000) {
bHigherBusyTraffic = true;
@@ -1466,7 +1456,6 @@ static void _rtl92e_watchdog_wq_cb(void *data)
else
priv->check_roaming_cnt = 0;
 
-
if (priv->check_roaming_cnt > 0) {
if (ieee->eRFPowerState == eRfOff)
netdev_info(dev, "%s(): RF is off\n", __func__);
@@ -1497,7 +1486,6 @@ static void _rtl92e_watchdog_wq_cb(void *data)
}
ieee->LinkDetectInfo.NumRecvBcnInPeriod = 0;
ieee->LinkDetectInfo.NumRecvDataInPeriod = 0;
-
}
 
spin_lock_irqsave(&priv->tx_lock, flags);
@@ -1549,7 +1537,6 @@ void rtl92e_tx_enable(struct net_device *dev)
rtllib_reset_queue(priv->rtllib);
 }
 
-
 static void _rtl92e_free_rx_ring(struct net_device *dev)
 {
struct r8192_priv *priv = rtllib_priv(dev);
@@ -1951,13 +1938,11 @@ long rtl92e_translate_to_dbm(struct r8192_priv *priv, 
u8 signal_strength_index)
return signal_power;
 }
 
-
 void rtl92e_update_rx_statistics(struct r8192_priv *pri

[PATCH v4 4/6] staging: rtl8192e: Pass a pointer as an argument to sizeof() instead of struct

2017-03-16 Thread sunil . m
From: Suniel Mahesh 

Replaced sizeof(struct foo) into sizeof(*ptr), found by checkpatch.pl

Signed-off-by: Suniel Mahesh 
---
Changes for v4:

- Dropped two patches from the series, as they were not adding significant value
  suggested by Dan Carpenter.
  staging: rtl8192e: Fix coding style, this was fixing line over 80 characters.
  staging: rtl8192e: Fix unbalanced braces
- Resending the whole series as requested by Greg K-H
- Patches were tested and built on next-20170310 and staging-testing

Changes for v3:

- Split earlier patches into multiple commits for easy review
  as suggested by Greg K-H
- Modified description for better readability
- Rebased on top of next-20170310
- Patches were tested and built on next-20170310 and staging-testing
  as suggested by Greg K-H, no errors reported

Changes for v2:

- new patch addition to the series
- Rebased on top of next-20170306
---
 drivers/staging/rtl8192e/rtl8192e/rtl_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_core.c 
b/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
index 4f4cd07..999af05 100644
--- a/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
+++ b/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
@@ -969,7 +969,7 @@ static void _rtl92e_init_priv_variable(struct net_device 
*dev)
 
priv->card_type = PCI;
 
-   priv->pFirmware = vzalloc(sizeof(struct rt_firmware));
+   priv->pFirmware = vzalloc(sizeof(*priv->pFirmware));
if (!priv->pFirmware)
return;
 
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event

2017-03-16 Thread Sakari Ailus
Hi Steve,

On Tue, Mar 14, 2017 at 09:43:09AM -0700, Steve Longerbeam wrote:
> 
> 
> On 03/14/2017 09:21 AM, Nicolas Dufresne wrote:
> >Le lundi 13 mars 2017 à 10:45 +, Russell King - ARM Linux a écrit :
> >>On Mon, Mar 13, 2017 at 11:02:34AM +0100, Hans Verkuil wrote:
> >>>On 03/11/2017 07:14 PM, Steve Longerbeam wrote:
> The event must be user visible, otherwise the user has no indication
> the error, and can't correct it by stream restart.
> >>>In that case the driver can detect this and call vb2_queue_error. It's
> >>>what it is there for.
> >>>
> >>>The event doesn't help you since only this driver has this issue. So nobody
> >>>will watch this event, unless it is sw specifically written for this SoC.
> >>>
> >>>Much better to call vb2_queue_error to signal a fatal error (which this
> >>>apparently is) since there are more drivers that do this, and vivid 
> >>>supports
> >>>triggering this condition as well.
> >>So today, I can fiddle around with the IMX219 registers to help gain
> >>an understanding of how this sensor works.  Several of the registers
> >>(such as the PLL setup [*]) require me to disable streaming on the
> >>sensor while changing them.
> >>
> >>This is something I've done many times while testing various ideas,
> >>and is my primary way of figuring out and testing such things.
> >>
> >>Whenever I resume streaming (provided I've let the sensor stop
> >>streaming at a frame boundary) it resumes as if nothing happened.  If I
> >>stop the sensor mid-frame, then I get the rolling issue that Steve
> >>reports, but once the top of the frame becomes aligned with the top of
> >>the capture, everything then becomes stable again as if nothing happened.
> >>
> >>The side effect of what you're proposing is that when I disable streaming
> >>at the sensor by poking at its registers, rather than the capture just
> >>stopping, an error is going to be delivered to gstreamer, and gstreamer
> >>is going to exit, taking the entire capture process down.
> >Indeed, there is no recovery attempt in GStreamer code, and it's hard
> >for an higher level programs to handle this. Nothing prevents from
> >adding something of course, but the errors are really un-specific, so
> >it would be something pretty blind. For what it has been tested, this
> >case was never met, usually the error is triggered by a USB camera
> >being un-plugged, a driver failure or even a firmware crash. Most of
> >the time, this is not recoverable.
> >
> >My main concern here based on what I'm reading, is that this driver is
> >not even able to notice immediately that a produced frame was corrupted
> >(because it's out of sync). From usability perspective, this is really
> >bad.
> 
> First, this is an isolated problem, specific to bt.656 and it only
> occurs when disrupting the analog video source signal in some
> way (by unplugging the RCA cable from the ADV718x connector
> for example).
> 
> Second, there is no DMA status support in i.MX6 to catch these
> shifted bt.656 codes, and the ADV718x does not provide any
> status indicators of this event either. So monitoring frame intervals
> is the only solution available, until FSL/NXP issues a new silicon rev.
> 
> 
> >  Can't the driver derive a clock from some irq and calculate for
> >each frame if the timing was correct ?
> 
> That's what is being done, essentially.
> 
> >  And if not mark the buffer with
> >V4L2_BUF_FLAG_ERROR ?
> 
> I prefer to keep the private event, V4L2_BUF_FLAG_ERROR is too
> unspecific.

Is the reason you prefer an event that you have multiple drivers involved,
or that the error flag is, well, only telling there was an error with a
particular frame?

Returning -EIO (by calling vb2_queue_error()) would be a better choice as it
is documented behaviour.

-- 
Regard,s

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v5 18/39] [media] v4l: subdev: Add function to validate frame interval

2017-03-16 Thread Sakari Ailus
On Sat, Mar 11, 2017 at 12:31:24PM -0800, Steve Longerbeam wrote:
> 
> 
> On 03/11/2017 05:41 AM, Sakari Ailus wrote:
> >Hi Steve,
> >
> >On Thu, Mar 09, 2017 at 08:52:58PM -0800, Steve Longerbeam wrote:
> >>If the pads on both sides of a link specify a frame interval, then
> >>those frame intervals should match. Create the exported function
> >>v4l2_subdev_link_validate_frame_interval() to verify this. This
> >>function can be called in a subdevice's media_entity_operations
> >>or v4l2_subdev_pad_ops link_validate callbacks.
> >>
> >>Signed-off-by: Steve Longerbeam 
> >
> >If your only goal is to configure frame dropping on a sub-device, I suggest
> >to implement s_frame_interval() on the pads of that sub-device only. The
> >frames are then dropped according to the configured frame rates between the
> >sink and source pads. Say, configuring sink for 1/30 s and source 1/15 would
> >drop half of the incoming frames.
> >
> >Considering that supporting specific frame interval on most sub-devices adds
> >no value or is not the interface through which it the frame rate configured,
> >I think it is overkill to change the link validation to expect otherwise.
> 
> 
> Well, while I think this function might still have validity in the future, I
> do agree with you that a subdev that has no control over
> frame rate has no business implementing the get|set ops.
> 
> In the imx-media subdevs, the only one that can affect frame rate (via
> frame skipping) is the CSI. So I'll go ahead and remove the
> [gs]_frame_interval ops from the others. I can remove this patch as
> a result.

Agreed.

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [RFC PATCH v2 14/32] x86: mm: Provide support to use memblock when spliting large pages

2017-03-16 Thread Paolo Bonzini


On 16/03/2017 19:28, Borislav Petkov wrote:
> So how hard would it be if the hypervisor allocated that memory for the
> guest instead? It would allocate it decrypted and guest would need to
> access it decrypted too. All in preparation for SEV-ES which will need a
> block of unencrypted memory for the guest anyway...

The kvmclock memory is initially zero so there is no need for the
hypervisor to allocate anything; the point of these patches is just to
access the data in a natural way from Linux source code.

I also don't really like the patch as is (plus it fails modpost), but
IMO reusing __change_page_attr and __split_large_page is the right thing
to do.

Paolo
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] staging: wilc1000: fix incorrect copy of pmkid data

2017-03-16 Thread Colin King
From: Colin Ian King 

The pmkid data is meant be be copied to the previous item in the
pmkidlist, however the code is just copying the data to itself because
the src index into pmkidlist is the same as the dst index into pmkidlist.
Fix this with i + 1 instead of i.

Detected by CoverityScan,CID#13339465 ("Overlapping buffer in memory copy")

Signed-off-by: Colin Ian King 
---
 drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c 
b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
index a37896f..4034f40 100644
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
@@ -1346,7 +1346,7 @@ static int del_pmksa(struct wiphy *wiphy, struct 
net_device *netdev,
   priv->pmkid_list.pmkidlist[i + 1].bssid,
   ETH_ALEN);
memcpy(priv->pmkid_list.pmkidlist[i].pmkid,
-  priv->pmkid_list.pmkidlist[i].pmkid,
+  priv->pmkid_list.pmkidlist[i + 1].pmkid,
   PMKID_LEN);
}
priv->pmkid_list.numpmkid--;
-- 
2.10.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 5/6] staging: media: atomisp: fill properly hmm_bo_type_strings when ION is disabled

2017-03-16 Thread Jérémy Lefaure
On Thu, 16 Mar 2017 17:51:06 +
Alan Cox  wrote:

> On Wed, 2017-03-15 at 14:09 -0400, Jérémy Lefaure wrote:
> > When CONFIG_ION is disabled, HMM_BO_LAST is 3. In this case, "i"
> > should
> > not be added in hmm_bo_type_strings.  
> 
> No the goal is to remove ifdefs - that's not the way to go with driver
> clean up. Instead the array size should always cover the four strings.
> 
Mmh, ok I understand. So I guess that the fix should have been to
remove the ifdef in hmm_bo.h. At first glance this should be ok even if
HMM_BO_LAST is used for the loop limit in bo_show.

Greg, this patch is already in staging-next. Should I send a patch that
reverts that change (maybe a patch that removes both ifdef if it is
really ok) or will you discard it ? Sorry, I don't know how you manage
this branch.

Thanks,
Jérémy
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] Staging: wilc1000: fix two typos in #define's

2017-03-16 Thread Greg KH
On Thu, Mar 16, 2017 at 05:51:12PM -0400, Dylan Leggio wrote:
> Signed-off-by: Dylan Leggio 

I can't take patches without any changelog text at all, sorry.

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: PATCH for style warnings in drivers/staging/most/aim-v4l2

2017-03-16 Thread Greg KH
On Thu, Mar 16, 2017 at 02:38:39PM -0700, Chandra Annamaneni wrote:
> 
> Enclosed is a patch to the file video.c. It only fixes style warning flagged
> by checkpatch.pl.
> 
> Please let me know if anything else needs to be done.
> 
> Thanks.
> Chandra
> 
> 
> diff --git a/drivers/staging/most/aim-v4l2/video.c 
> b/drivers/staging/most/aim-v4l2/video.c
> index e074841..59e861e 100644
> --- a/drivers/staging/most/aim-v4l2/video.c
> +++ b/drivers/staging/most/aim-v4l2/video.c
> @@ -79,7 +79,7 @@ static int aim_vdev_open(struct file *filp)
>   struct most_video_dev *mdev = video_drvdata(filp);
>   struct aim_fh *fh;
> 
> - v4l2_info(&mdev->v4l2_dev, "aim_vdev_open()\n");
> + v4l2_info(&mdev->v4l2_dev, "%s()\n", __func__);
> 
>   switch (vdev->vfl_type) {
>   case VFL_TYPE_GRABBER:
> @@ -128,7 +128,7 @@ static int aim_vdev_close(struct file *filp)
>   struct most_video_dev *mdev = fh->mdev;
>   struct mbo *mbo, *tmp;
> 
> - v4l2_info(&mdev->v4l2_dev, "aim_vdev_close()\n");
> + v4l2_info(&mdev->v4l2_dev, "%s()\n", __func__);
> 
>   /*
>* We need to put MBOs back before we call most_stop_channel()
> @@ -324,7 +324,7 @@ static int vidioc_g_std(struct file *file, void *priv, 
> v4l2_std_id *norm)
>   struct aim_fh *fh = priv;
>   struct most_video_dev *mdev = fh->mdev;
> 
> - v4l2_info(&mdev->v4l2_dev, "vidioc_g_std()\n");
> + v4l2_info(&mdev->v4l2_dev, "%s()\n", __func__);
> 
>   *norm = V4L2_STD_UNKNOWN;
>   return 0;
> @@ -361,7 +361,7 @@ static int vidioc_s_input(struct file *file, void *priv, 
> unsigned int index)
>   struct aim_fh *fh = priv;
>   struct most_video_dev *mdev = fh->mdev;
> 
> - v4l2_info(&mdev->v4l2_dev, "vidioc_s_input(%d)\n", index);
> + v4l2_info(&mdev->v4l2_dev, "%s(%d)\n", __func__, index);
> 
>   if (index >= V4L2_AIM_MAX_INPUT)
>   return -EINVAL;
> @@ -441,7 +441,7 @@ static int aim_register_videodev(struct most_video_dev 
> *mdev)
>  {
>   int ret;
> 
> - v4l2_info(&mdev->v4l2_dev, "aim_register_videodev()\n");
> + v4l2_info(&mdev->v4l2_dev, "%s()\n", __func__);
> 
>   init_waitqueue_head(&mdev->wait_data);
> 
> @@ -471,7 +471,7 @@ static int aim_register_videodev(struct most_video_dev 
> *mdev)
> 
>  static void aim_unregister_videodev(struct most_video_dev *mdev)
>  {
> - v4l2_info(&mdev->v4l2_dev, "aim_unregister_videodev()\n");
> + v4l2_info(&mdev->v4l2_dev, "%s()\n", __func__);
> 
>   video_unregister_device(mdev->vdev);
>  }


Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- Your patch does not have a Signed-off-by: line.  Please read the
  kernel file, Documentation/SubmittingPatches and resend it after
  adding that line.  Note, the line needs to be in the body of the
  email, before the patch, not at the bottom of the patch or in the
  email signature.

- You did not specify a description of why the patch is needed, or
  possibly, any description at all, in the email body.  Please read the
  section entitled "The canonical patch format" in the kernel file,
  Documentation/SubmittingPatches for what is needed in order to
  properly describe the change.

- You did not write a descriptive Subject: for the patch, allowing Greg,
  and everyone else, to know what this patch is all about.  Please read
  the section entitled "The canonical patch format" in the kernel file,
  Documentation/SubmittingPatches for what a proper Subject: line should
  look like.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] Staging: wilc1000: fix two typos in #define's

2017-03-16 Thread Dylan Leggio
GAS_INTIAL_REQ should be GAS_INITIAL_REQ.
GAS_INTIAL_RSP should be GAS_INITIAL_RSP.
Improves readability of code.

Signed-off-by: Dylan Leggio 
---
 drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c 
b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
index a37896fcd683..1bb63702c623 100644
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
@@ -29,8 +29,8 @@
 #define P2P_INV_REQ0x03
 #define P2P_INV_RSP0x04
 #define PUBLIC_ACT_VENDORSPEC  0x09
-#define GAS_INTIAL_REQ 0x0a
-#define GAS_INTIAL_RSP 0x0b
+#define GAS_INITIAL_REQ0x0a
+#define GAS_INITIAL_RSP0x0b
 
 #define INVALID_CHANNEL0
 
@@ -1477,10 +1477,10 @@ void WILC_WFI_p2p_rx(struct net_device *dev, u8 *buff, 
u32 size)
}
if (buff[ACTION_CAT_ID] == PUB_ACTION_ATTR_ID) {
switch (buff[ACTION_SUBTYPE_ID]) {
-   case GAS_INTIAL_REQ:
+   case GAS_INITIAL_REQ:
break;
 
-   case GAS_INTIAL_RSP:
+   case GAS_INITIAL_RSP:
break;
 
case PUBLIC_ACT_VENDORSPEC:
@@ -1666,10 +1666,10 @@ static int mgmt_tx(struct wiphy *wiphy,
curr_channel = chan->hw_value;
}
switch (buf[ACTION_SUBTYPE_ID]) {
-   case GAS_INTIAL_REQ:
+   case GAS_INITIAL_REQ:
break;
 
-   case GAS_INTIAL_RSP:
+   case GAS_INITIAL_RSP:
break;
 
case PUBLIC_ACT_VENDORSPEC:
-- 
2.12.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 5/6] staging: media: atomisp: fill properly hmm_bo_type_strings when ION is disabled

2017-03-16 Thread Greg Kroah-Hartman
On Thu, Mar 16, 2017 at 08:15:57PM -0400, Jérémy Lefaure wrote:
> On Thu, 16 Mar 2017 17:51:06 +
> Alan Cox  wrote:
> 
> > On Wed, 2017-03-15 at 14:09 -0400, Jérémy Lefaure wrote:
> > > When CONFIG_ION is disabled, HMM_BO_LAST is 3. In this case, "i"
> > > should
> > > not be added in hmm_bo_type_strings.  
> > 
> > No the goal is to remove ifdefs - that's not the way to go with driver
> > clean up. Instead the array size should always cover the four strings.
> > 
> Mmh, ok I understand. So I guess that the fix should have been to
> remove the ifdef in hmm_bo.h. At first glance this should be ok even if
> HMM_BO_LAST is used for the loop limit in bo_show.
> 
> Greg, this patch is already in staging-next. Should I send a patch that
> reverts that change (maybe a patch that removes both ifdef if it is
> really ok) or will you discard it ? Sorry, I don't know how you manage
> this branch.

Please send a revert.

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] Revert "staging: media: atomisp: fill properly hmm_bo_type_strings when ION is disabled"

2017-03-16 Thread Jérémy Lefaure
This reverts commit fde469701c7efabebf885e785edf367bfb1a8f3f.

Adding a preprocessor condition is not the best solution to fix this
issue. Let's revert this commit before fixing this problem in a more
appropriate way.

Signed-off-by: Jérémy Lefaure 
---
 drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm.c 
b/drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm.c
index e78f02f74d0f..a362b492ec8e 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm.c
@@ -49,9 +49,7 @@ const char *hmm_bo_type_strings[HMM_BO_LAST] = {
"p", /* private */
"s", /* shared */
"u", /* user */
-#ifdef CONFIG_ION
"i", /* ion */
-#endif
 };
 
 static ssize_t bo_show(struct device *dev, struct device_attribute *attr,
-- 
2.12.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v2 1/6] staging: media: atomisp: add missing include in vlv2_plat_clock.c

2017-03-16 Thread Jérémy Lefaure
To use IO functions like writel, readl, ioremap_nocache and iounmap,
linux/io.h should be included.

Signed-off-by: Jérémy Lefaure 
---
 drivers/staging/media/atomisp/platform/clock/vlv2_plat_clock.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/media/atomisp/platform/clock/vlv2_plat_clock.c 
b/drivers/staging/media/atomisp/platform/clock/vlv2_plat_clock.c
index a8ca93d..25e939c 100644
--- a/drivers/staging/media/atomisp/platform/clock/vlv2_plat_clock.c
+++ b/drivers/staging/media/atomisp/platform/clock/vlv2_plat_clock.c
@@ -20,6 +20,7 @@
  */
 
 #include 
+#include 
 #include 
 #include 
 #include "../../include/linux/vlv2_plat_clock.h"
-- 
2.1.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v4 0/6] staging: rtl8192e: Fix coding style, warnings and checks

2017-03-16 Thread Greg KH
On Fri, Mar 17, 2017 at 03:42:15AM +0530, suni...@techveda.org wrote:
> From: Suniel Mahesh 
> 
> Split earlier patches into multiple commits for easy review as
> suggested by Dan Carpenter.
> Modified subject, description and in few patches both for
> better readability as suggested by Greg KH.
> Dropped two patches from the earler series, as they were not adding 
> significant value, suggested by Dan Carpenter.
> Fixed the following issues reported by checkpatch.pl:
> Block comments should align the * on each line, aligned.
> Block comments use * on subsequent lines, other characters
> are replaced by * .
> Removed unnecessary 'out of memory' message.
> Comparison's to NULL could be written '!foo' or 'foo', modified.
> Replaced sizeof(struct foo) into sizeof(*ptr).
> Spaces preferred around that 'operator', spacing provided.
> Logical continuations should be on the previous line, modified accordingly.
> Unnecessary parentheses around variables, removed.
> Please use a blank line after function/struct/union/enum declarations, used.
> Blank lines aren't necessary after an open brace '{' and before a
> close brace '}', removed.
> No space is necessary after a cast, removed.
> Please don't use multiple blank lines, removed.

I have no idea what all of this means :(

Can you resend the full series, not just random patches in a series?  I
don't have any of the old ones anymore.

If I have applied other patches, just send this as a sequence that is
numbered properly, with all of the patches to be applied.

You need to make it very easy for a maintainer to know what to do here,
remember, we have no short-term memory of any previous submissions.

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCHv5 1/4] Staging: ks7010: ks_wlan_ioctl.h: Removed mixed spaces/tabs

2017-03-16 Thread Greg KH
On Fri, Mar 17, 2017 at 03:22:57PM +0900, Greg KH wrote:
> On Wed, Mar 15, 2017 at 08:18:08PM -0700, Matthew Giassa wrote:
> > Removing mixed spaces/hard-tabs used to create a "column alignment" of
> > macro names and macro values.
> > 
> > Signed-off-by: Matthew Giassa 
> > ---
> >  drivers/staging/ks7010/ks_wlan_ioctl.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> This patch does not apply to my tree at all.  Are you sure you are
> working against the staging-next branch?  Please rebase and resend.

Same thing for all of the patches in this series.

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCHv5 1/4] Staging: ks7010: ks_wlan_ioctl.h: Removed mixed spaces/tabs

2017-03-16 Thread Greg KH
On Wed, Mar 15, 2017 at 08:18:08PM -0700, Matthew Giassa wrote:
> Removing mixed spaces/hard-tabs used to create a "column alignment" of
> macro names and macro values.
> 
> Signed-off-by: Matthew Giassa 
> ---
>  drivers/staging/ks7010/ks_wlan_ioctl.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

This patch does not apply to my tree at all.  Are you sure you are
working against the staging-next branch?  Please rebase and resend.

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] Staging: media: Unmap and release region obtained by ioremap_nocache

2017-03-16 Thread Arvind Yadav
Free memory mapping, if vpfe_ipipe_init is not successful.

Signed-off-by: Arvind Yadav 
---
 drivers/staging/media/davinci_vpfe/dm365_ipipe.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c 
b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
index ff47a8f3..6a3434c 100644
--- a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
+++ b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
@@ -1803,14 +1803,14 @@ void vpfe_ipipe_unregister_entities(struct 
vpfe_ipipe_device *vpfe_ipipe)
return -EBUSY;
ipipe->base_addr = ioremap_nocache(res->start, res_len);
if (!ipipe->base_addr)
-   return -EBUSY;
+   goto error_release;
 
res = platform_get_resource(pdev, IORESOURCE_MEM, 6);
if (!res)
-   return -ENOENT;
+   goto error_unmap;
ipipe->isp5_base_addr = ioremap_nocache(res->start, res_len);
if (!ipipe->isp5_base_addr)
-   return -EBUSY;
+   goto error_unmap;
 
v4l2_subdev_init(sd, &ipipe_v4l2_ops);
sd->internal_ops = &ipipe_v4l2_internal_ops;
@@ -1839,6 +1839,12 @@ void vpfe_ipipe_unregister_entities(struct 
vpfe_ipipe_device *vpfe_ipipe)
sd->ctrl_handler = &ipipe->ctrls;
 
return media_entity_pads_init(me, IPIPE_PADS_NUM, pads);
+
+error_unmap:
+   iounmap(ipipe->base_addr);
+error_release:
+   release_mem_region(res->start, res_len);
+   return -ENOMEM;
 }
 
 /*
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel