Re: [RFC GIT PULL, v2] RCU changes for v4.12

2017-05-10 Thread Ingo Molnar

* Linus Torvalds  wrote:

> On Wed, May 10, 2017 at 12:54 PM, Ingo Molnar  wrote:
> >
> > Yeah, you are right and sorry about that - I have removed the patch
> > generation from my pull request scripts, so it shouldn't happen in
> > the future.
> 
> I do have to say, that during the later -rc series in particular when
> people send me smaller fixes, I enjoy seeing the full patches.

I find them useful too, and to answer your original question:

> > Nobody is ever going to review a 300kB patch that is ~7500 lines.

I _did_ skim over that 300K patch, because I always try to do a final manual 
check 
on the raw diffs I'm sending to you, and also to make it very clear what was 
sent 
from a full disclosure and security log POV, independent of the Git pull space. 

When patches are way too long, for example as the perf pull request diffs often 
are, I trim them, so it's never an absolute, script-only thing.

Still it's not an excuse:

 - I doubt anyone else but me would skim over a 30K (let alone a 300K) patch,

 - I also missed the pain large patches cause in Gmail replies (with Mutt that
   pain is considerably less),

 - plus, most importantly, I didn't notice that the extra RCU mode bloat was 
one 
   too many in an already sizable line-up of RCU complexity ...

Thanks,

Ingo


Re: [PATCH] vmscan: scan pages until it founds eligible pages

2017-05-10 Thread Minchan Kim
On Wed, May 10, 2017 at 08:13:12AM +0200, Michal Hocko wrote:
> On Wed 10-05-17 10:46:54, Minchan Kim wrote:
> > On Wed, May 03, 2017 at 08:00:44AM +0200, Michal Hocko wrote:
> [...]
> > > @@ -1486,6 +1486,12 @@ static unsigned long isolate_lru_pages(unsigned 
> > > long nr_to_scan,
> > >   continue;
> > >   }
> > >  
> > > + /*
> > > +  * Do not count skipped pages because we do want to isolate
> > > +  * some pages even when the LRU mostly contains ineligible
> > > +  * pages
> > > +  */
> > 
> > How about adding comment about "why"?
> > 
> > /*
> >  * Do not count skipped pages because it makes the function to return with
> >  * none isolated pages if the LRU mostly contains inelgible pages so that
> >  * VM cannot reclaim any pages and trigger premature OOM.
> >  */
> 
> I am not sure this is necessarily any better. Mentioning a pre-mature
> OOM would require a much better explanation because a first immediate
> question would be "why don't we scan those pages at priority 0". Also
> decision about the OOM is at a different layer and it might change in
> future when this doesn't apply any more. But it is not like I would
> insist...
> 
> > > + scan++;
> > >   switch (__isolate_lru_page(page, mode)) {
> > >   case 0:
> > >   nr_pages = hpage_nr_pages(page);
> > 
> > Confirmed.
> 
> Hmm. I can clearly see how we could skip over too many pages and hit
> small reclaim priorities too quickly but I am still scratching my head
> about how we could hit the OOM killer as a result. The amount of pages
> on the active anonymous list suggests that we are not able to rotate
> pages quickly enough. I have to keep thinking about that.

I explained it but seems to be not enouggh. Let me try again.

The problem is that get_scan_count determines nr_to_scan with
eligible zones.

size = lruvec_lru_size(lruvec, lru, sc->reclaim_idx);
size = size >> sc->priority;

Assumes sc->priority is 0 and LRU list is as follows.

N-N-N-N-H-H-H-H-H-H-H-H-H-H-H-H-H-H-H-H

(Ie, small eligible pages are in the head of LRU but others are
almost ineligible pages)

In that case, size becomes 4 so VM want to scan 4 pages but 4 pages
from tail of the LRU are not eligible pages.
If get_scan_count counts skipped pages, it doesn't reclaim remained
pages after scanning 4 pages.

If it's more helpful to understand the problem, I will add it to
the description.



Re: RFC: i2c designware gpio recovery

2017-05-10 Thread Phil Reid

G'day Tim,

Sorry for the delay in looking at this.
My device is currently running a 4.9 kernel and I had to backport the cahnges 
to the driver
to get things running with your patch.

In general the code works and the bus recovers now.
I've been using the i2c gpio bus driver because the dw wouldn't do recovery.
But this looks alot nicer.


On 4/05/2017 03:04, Tim Sander wrote:

So i took a look into the device tree file socfpga.dtsi and found that
the
reset lines where not defined (although available in the corresponding
reset manager). Is there a reason for this? Other components are
connected.


There's a few thing like that where the bootloader has been expected to
setup the resets etc.


Yes, but if the resets are not connected in the device tree, the linux
drivers are not going to use them?


Yes, so they should be added. I don't think we should assume the bootloader
sets things up. But that doesn't seem to have been the assumption with the
Alter SOC's.

I will prepare a patch for this.


However with the patch below my previously sent patch works!

If there is interest in would cleanup the patch and send it in for
mainlining. I think the most unacceptable part would be this line:
+   ret = gpio_request_one(bri->scl_gpio, //GPIOF_OPEN_DRAIN |
My gpio drivers refuse to work as output as they have no open drain
mode.
So i wonder how to get this solved in a clean manner.


I thought the gpio system would emulate open drain by switching the pin
between an input and output driven low in this case. How are you
configuring the GPIO's in the FPGA?


I don't switch to GPIO mode. As the I2C logic is only pulling active low,
i only do a wired and with the gpio (implemented in the fpga) port output
on the output enable line for the SCL output.  SDA is only an additional
input for the second in fpga gpio port.

A picture should make it a clearer:

gpio scl--\
i2c   scl --&---i2c mode output pin (configured as fpga loan)

In my case the original i2c pins where occupied by some other logic
conflicting so the i2c pins had to be shifted to some other pins using
fpga logic. So it was just a matter of adding a two port gpio port.


I think I understand. What soft core gpio controller are you using?

I am using the standard altera fpga gpios.




I dug into things a little and found the following init function works without 
requiring modification to the core.
The GPIO config (open drain or not etc) can be put in the device tree.

static int i2c_dw_get_scl(struct i2c_adapter *adap)
{
struct platform_device *pdev = to_platform_device(&adap->dev);
struct dw_i2c_dev *dev = platform_get_drvdata(pdev);
return gpiod_get_value_cansleep(dev->gpio_scl);
}

static void i2c_dw_set_scl(struct i2c_adapter *adap, int val)
{
struct platform_device *pdev = to_platform_device(&adap->dev);
struct dw_i2c_dev *dev = platform_get_drvdata(pdev);
gpiod_set_value_cansleep(dev->gpio_scl, val);
}

static int i2c_dw_get_sda(struct i2c_adapter *adap)
{
struct platform_device *pdev = to_platform_device(&adap->dev);
struct dw_i2c_dev *dev = platform_get_drvdata(pdev);
return gpiod_get_value_cansleep(dev->gpio_sda);
}

static int i2c_dw_init_recovery_info(struct dw_i2c_dev *dev, struct i2c_adapter 
*adap)
{
struct i2c_bus_recovery_info *rinfo = &dev->rinfo;

dev->gpio_scl = devm_gpiod_get_optional(dev->dev, "scl", 
GPIOD_OUT_HIGH);
if (IS_ERR_OR_NULL(dev->gpio_scl))
return PTR_ERR(dev->gpio_scl);

dev->gpio_sda = devm_gpiod_get_optional(dev->dev, "sda", GPIOD_IN);
if (IS_ERR_OR_NULL(dev->gpio_sda))
return PTR_ERR(dev->gpio_sda);

rinfo->scl_gpio  = desc_to_gpio(dev->gpio_scl);
rinfo->sda_gpio = desc_to_gpio(dev->gpio_sda);
rinfo->set_scl  = i2c_dw_set_scl;
rinfo->get_scl  = i2c_dw_get_scl;
rinfo->get_sda  = i2c_dw_get_sda;

rinfo->recover_bus = i2c_generic_scl_recovery;
rinfo->prepare_recovery = i2c_dw_prepare_recovery;
rinfo->unprepare_recovery = i2c_dw_unprepare_recovery;
adap->bus_recovery_info = rinfo;

dev_info(dev->dev,
"adapter: %s running with gpio recovery mode! scl:%i sda:%i \n",
adap->name, rinfo->scl_gpio, rinfo->sda_gpio);

return 0;
};

A small modification to the i2c-core could be done in i2c_init_recovery to 
allow:
rinfo->recover_bus == i2c_generic_scl_recovery
when scl_gpio is also set and fallback to using the core set / get scl / sda 
calls
Which would remove the need for the above i2c_dw_* functions.
I wouldn't think that would cause any problems.



--
Regards
Phil Reid

ElectroMagnetic Imaging Technology Pty Ltd
Development of Geophysical Instrumentation & Software
www.electromag.com.au

3 The Avenue, Midland WA 6056, AUSTRALIA
Ph: +61 8 9250 8100
Fax: +61 8 9250 7100
Email: pr...@electromag.com.au


Re: [kernel-hardening] Re: [PATCH v9 1/4] syscalls: Verify address limit before returning to user-mode

2017-05-10 Thread Christoph Hellwig
On Tue, May 09, 2017 at 04:31:00PM -0700, Kees Cook wrote:
> > I don't like silent fixups.  If we want to do this, we should BUG or
> > at least WARN, not just change the addr limit.  But I'm also not
> > convinced it's indicative of an actual bug here.
> 
> Nothing should enter that function with KERNEL_DS set, right?

It might very well do.  Various drivers or the networking code mess
with the address limits for fairly broad sections of code.


Re: [PATCH] mm/vmscan: fix unsequenced modification and access warning

2017-05-10 Thread Michal Hocko
On Tue 09-05-17 23:53:28, Nick Desaulniers wrote:
> Clang flags this file with the -Wunsequenced error that GCC does not
> have.
> 
> unsequenced modification and access to 'gfp_mask'
> 
> It seems that gfp_mask is both read and written without a sequence point
> in between, which is undefined behavior.

Hmm. This is rather news to me. I thought that a = foo(a) is perfectly
valid. Same as a = b = c where c = foo(b) or is the problem in the
following .reclaim_idx = gfp_zone(gfp_mask) initialization? If that is
the case then the current code is OKish because gfp_zone doesn't depend
on the gfp_mask modification. It is messy, right, but works as expected.

Anyway, we have a similar construct __node_reclaim

If you really want to change this code, and I would agree it would be
slightly less tricky, then I would suggest doing something like the
following instead
---
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 5ebf468c5429..ba4b695e810e 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -2965,7 +2965,7 @@ unsigned long try_to_free_pages(struct zonelist 
*zonelist, int order,
unsigned long nr_reclaimed;
struct scan_control sc = {
.nr_to_reclaim = SWAP_CLUSTER_MAX,
-   .gfp_mask = (gfp_mask = current_gfp_context(gfp_mask)),
+   .gfp_mask = current_gfp_context(gfp_mask),
.reclaim_idx = gfp_zone(gfp_mask),
.order = order,
.nodemask = nodemask,
@@ -2980,12 +2980,12 @@ unsigned long try_to_free_pages(struct zonelist 
*zonelist, int order,
 * 1 is returned so that the page allocator does not OOM kill at this
 * point.
 */
-   if (throttle_direct_reclaim(gfp_mask, zonelist, nodemask))
+   if (throttle_direct_reclaim(sc.gfp_mask, zonelist, nodemask))
return 1;
 
trace_mm_vmscan_direct_reclaim_begin(order,
sc.may_writepage,
-   gfp_mask,
+   sc.gfp_mask,
sc.reclaim_idx);
 
nr_reclaimed = do_try_to_free_pages(zonelist, &sc);
@@ -3772,17 +3772,16 @@ static int __node_reclaim(struct pglist_data *pgdat, 
gfp_t gfp_mask, unsigned in
const unsigned long nr_pages = 1 << order;
struct task_struct *p = current;
struct reclaim_state reclaim_state;
-   int classzone_idx = gfp_zone(gfp_mask);
unsigned int noreclaim_flag;
struct scan_control sc = {
.nr_to_reclaim = max(nr_pages, SWAP_CLUSTER_MAX),
-   .gfp_mask = (gfp_mask = current_gfp_context(gfp_mask)),
+   .gfp_mask = current_gfp_context(gfp_mask),
.order = order,
.priority = NODE_RECLAIM_PRIORITY,
.may_writepage = !!(node_reclaim_mode & RECLAIM_WRITE),
.may_unmap = !!(node_reclaim_mode & RECLAIM_UNMAP),
.may_swap = 1,
-   .reclaim_idx = classzone_idx,
+   .reclaim_idx = gfp_znoe(gfp_mask),
};
 
cond_resched();
@@ -3793,7 +3792,7 @@ static int __node_reclaim(struct pglist_data *pgdat, 
gfp_t gfp_mask, unsigned in
 */
noreclaim_flag = memalloc_noreclaim_save();
p->flags |= PF_SWAPWRITE;
-   lockdep_set_current_reclaim_state(gfp_mask);
+   lockdep_set_current_reclaim_state(sc.gfp_mask);
reclaim_state.reclaimed_slab = 0;
p->reclaim_state = &reclaim_state;
 
-- 
Michal Hocko
SUSE Labs


Re: [PATCH 0/3] staging: ccree: resolve checkpatch issues.

2017-05-10 Thread Gilad Ben-Yossef
On Sun, May 7, 2017 at 1:56 AM, Matthew Giassa  wrote:
> * Matthew Giassa  [2017-05-06 15:46:53 -0700]:
>
>
>> Included is a set of small fixes to resolve all outstanding checkpatch
>> warnings issues for drivers/staging/ccree/cc_hal.h. Two are cosmetic
>> (training whitespace and 80+ character comment), and the other is
>> functional (macro argument previously not wrapped in parentheses).
>>
>
> Forgot to mention, applies cleanly against staging-next
> (3ef2bc099d1cce09e2844467e2ced98e1a44609d).
>
For 2-3:

Acked-by: Gilad Ben-Yossef 

I saw GregKH already carries them in the staging-testing

About 1 - I never saw it and there isn't one in staging-testing either
so I'm assuming
I'm not the only only one that missed it. Can you please resend?

Thanks,
Gilad



-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
 -- Jean-Baptiste Queru


Re: [PATCH] vmscan: scan pages until it founds eligible pages

2017-05-10 Thread Michal Hocko
On Wed 10-05-17 16:03:11, Minchan Kim wrote:
> On Wed, May 10, 2017 at 08:13:12AM +0200, Michal Hocko wrote:
> > On Wed 10-05-17 10:46:54, Minchan Kim wrote:
> > > On Wed, May 03, 2017 at 08:00:44AM +0200, Michal Hocko wrote:
[...]
> > > > +   scan++;
> > > > switch (__isolate_lru_page(page, mode)) {
> > > > case 0:
> > > > nr_pages = hpage_nr_pages(page);
> > > 
> > > Confirmed.
> > 
> > Hmm. I can clearly see how we could skip over too many pages and hit
> > small reclaim priorities too quickly but I am still scratching my head
> > about how we could hit the OOM killer as a result. The amount of pages
> > on the active anonymous list suggests that we are not able to rotate
> > pages quickly enough. I have to keep thinking about that.
> 
> I explained it but seems to be not enouggh. Let me try again.
> 
> The problem is that get_scan_count determines nr_to_scan with
> eligible zones.
> 
> size = lruvec_lru_size(lruvec, lru, sc->reclaim_idx);
> size = size >> sc->priority;

Ohh, right. Who has done that ;) Now it is much more clear. We simply
reclaimed all the pages on the inactive LRU list and only very slowly
progress over active list and hit the OOM before we can actually reach
anything. I completely forgot about the scan window not being the full
LRU list.

Thanks for bearing with me!
-- 
Michal Hocko
SUSE Labs


No mouse cursor since 36cc79bc9077319c04bd3b132edcacaa9a0d9f2b

2017-05-10 Thread m.t
Hi,
I don't have a mouse cursor in my virtual machine (vmware workstation 12.5.5 
build-5234757) with latest master from https://git.kernel.org/pub/scm/linux/
kernel/git/torvalds/linux.git

git bisect led me to 36cc79bc9077319c04bd3b132edcacaa9a0d9f2b as culprit.

Regards 
Mark


Re: [PATCH 3/9] VFS: Introduce a mount context

2017-05-10 Thread Miklos Szeredi
On Tue, May 9, 2017 at 8:51 PM, Jeff Layton  wrote:
> On Tue, 2017-05-09 at 14:02 +0200, Miklos Szeredi wrote:
>> On Tue, May 9, 2017 at 11:41 AM, David Howells  wrote:
>> > Miklos Szeredi  wrote:
>> >
>> > > I think that's crazy.  We don't return detailed errors for any other
>> > > syscall for path lookup, so why would path lookup for mount be
>> > > special.
>> >
>> > Firstly, we don't return detailed errors for mount() at the moment either.
>> >
>> > Secondly, path lookup might entail automounts, so perhaps we should do it 
>> > for
>> > path lookup too.  Particularly in light of the fact that NFS4 mount uses
>> > pathwalk to get from server:/ to server:/the/dir/I/actually/wanted/ so I'm
>> > currently losing that error:-/
>> >
>> > Thirdly, the security operation I'm talking about is separate to path 
>> > lookup -
>> > though perhaps we should pass LOOKUP_MOUNT as an intent flag into pathwalk 
>> > so
>> > that the security check can be done there; perhaps combined with another 
>> > one.
>> >
>> > Fourthly, why shouldn't we consider extending the facility to other system
>> > calls in future?  It would involve copying the string to task_struct and
>> > providing a way to retrieve it, but that's not that hard to achieve.
>>
>> Maybe we should.   In fact that sounds like a splendid idea.  IMO even
>> better, than having errors go via the fsfd descriptor.  Pretty cheap
>> on the kernel side, and completely optional on the userspace side.
>>
>
> A question here: What should happen if you go to set an error here, and
> one is already set? Should it just free the string and replace it with
> the new one? IOW, just keep the latest error? Or is it better to keep
> the earlier one?
>
> If you want to put this in the task_struct then I think you'll want to
> sort that out. You could easily end up in this situation if a lot of
> different kernel subsystems started using it to pass back detailed
> errors.

Possible rule of thumb: use it only at the place where the error
originates and not where errors are just passed on.  This would result
in at most one report per syscall, normally.

And the static string thing that David implemented is also a very good
idea, IMO.

So it would look something like this (possibly needs better naming:

   error_detail("description of error");

or

   return error_detail(-EINVAL, "description of error");

Compiler could automatically include source file/line information as
well, although it may be enough if the string is uniquely greppable
(we could check uniqueness at compile time).

Thanks,
Miklos


Re: [v3 0/9] parallelized "struct page" zeroing

2017-05-10 Thread Michal Hocko
On Tue 09-05-17 14:54:50, Pasha Tatashin wrote:
[...]
> >The implementation just looks too large to what I would expect. E.g. do
> >we really need to add zero argument to the large part of the memblock
> >API? Wouldn't it be easier to simply export memblock_virt_alloc_internal
> >(or its tiny wrapper memblock_virt_alloc_core) and move the zeroing
> >outside to its 2 callers? A completely untested scratched version at the
> >end of the email.
> 
> I am OK, with this change. But, I do not really see a difference between:
> 
> memblock_virt_alloc_raw()
> and
> memblock_virt_alloc_core()
> 
> In both cases we use memblock_virt_alloc_internal(), but the only difference
> is that in my case we tell memblock_virt_alloc_internal() to zero the pages
> if needed, and in your case the other two callers are zeroing it. I like
> moving memblock_dbg() inside memblock_virt_alloc_internal()

Well, I didn't object to this particular part. I was mostly concerned
about
http://lkml.kernel.org/r/1494003796-748672-4-git-send-email-pasha.tatas...@oracle.com
and the "zero" argument for other functions. I guess we can do without
that. I _think_ that we should simply _always_ initialize the page at the
__init_single_page time rather than during the allocation. That would
require dropping __GFP_ZERO for non-memblock allocations. Or do you
think we could regress for single threaded initialization?

> >Also it seems that this is not 100% correct either as it only cares
> >about VMEMMAP while DEFERRED_STRUCT_PAGE_INIT might be enabled also for
> >SPARSEMEM. This would suggest that we would zero out pages twice,
> >right?
> 
> Thank you, I will check this combination before sending out the next patch.
> 
> >
> >A similar concern would go to the memory hotplug patch which will
> >fall back to the slab/page allocator IIRC. On the other hand
> >__init_single_page is shared with the hotplug code so again we would
> >initialize 2 times.
> 
> Correct, when memory it hotplugged, to gain the benefit of this fix, and
> also not to regress by actually double zeroing "struct pages" we should not
> zero it out. However, I do not really have means to test it.

It should be pretty easy to test with kvm, but I can help with testing
on the real HW as well.

Thanks!
-- 
Michal Hocko
SUSE Labs


Re: [PATCH] wcn36xx: Close SMD channel on device removal

2017-05-10 Thread Arend van Spriel

On 5/10/2017 1:03 AM, Bjorn Andersson wrote:

On Mon 08 May 23:17 PDT 2017, Kalle Valo wrote:


Bjorn Andersson  writes:


The SMD channel is not the primary WCNSS channel and must explicitly be
closed as the device is removed, or the channel will already by open on
a subsequent probe call in e.g. the case of reloading the kernel module.

This issue was introduced because I simplified the underlying SMD
implementation while the SMD adaptions of the driver sat on the mailing
list, but missed to update these patches. The patch does however only
apply back to the transition to rpmsg, hence the limited Fixes.

Fixes: 5052de8deff5 ("soc: qcom: smd: Transition client drivers from smd to 
rpmsg")
Reported-by: Eyal Ilsar 
Signed-off-by: Bjorn Andersson 


As this is a regression I'll queue this to 4.12.



Thanks.


But if this is an older bug (didn't quite understand your description
though) should there be a separate patch for stable releases?



AFAICT this never worked, as it seems I did the rework in SMD while we
tried to figure out the dependency issues we had with moving to SMD. So
v4.9 through v4.11 has SMD support - with this bug.

How do I proceed, do you want me to write up a fix for stable@? Do I
send that out as an ordinary patch?


If the patch applies cleanly on branches linux-4.9.y through 
linux-4.11.y in the stable repository you can go for '--- Option 1 ---' 
as described in /Documentation/stable_kernel_rules.txt.


Regards,
Arend


Re: sparse on scripts/kconfig/*.c

2017-05-10 Thread Christoph Hellwig
On Tue, May 09, 2017 at 05:27:01PM -0700, Randy Dunlap wrote:
> On 05/09/17 13:17, Christoph Hellwig wrote:
> > On Tue, May 09, 2017 at 09:47:41AM -0700, Randy Dunlap wrote:
> >> Hi,
> >>
> >> I've been attempting to run sparse on the kconfig/ C files -- without 
> >> success.
> >>
> >> The kbuild files don't try to support CHECK in scripts/kconfig/ AFAICT,
> >> and just running sparse on the C files has issues with not being able to
> >> find header files.
> >>
> >> Has anyone done this?  Any clues about how to do it?
> > 
> > As a wild guess from using sparse on various userspace projects:
> > 
> > have you tried simply setting HOSTCC to cgcc?
> 
> I don't quite see what that has to do with running sparse ($CHECK, not 
> $HOSTCC).

cgcc is a gcc wrappr that calls sparse.  I just trie quickly to patch
Makefile to run cgcc instead of gcc as HOSTCC an it seems to work:

  HOSTCC  scripts/basic/fixdep
scripts/basic/fixdep.c:117:5: warning: symbol 'insert_extra_deps' was not 
declared. Should it be static?
scripts/basic/fixdep.c:118:6: warning: symbol 'target' was not declared. Should 
it be static?
scripts/basic/fixdep.c:119:6: warning: symbol 'depfile' was not declared. 
Should it be static?
scripts/basic/fixdep.c:120:6: warning: symbol 'cmdline' was not declared. 
Should it be static?

But then I run into the known cgcc bug that it also calls sparse when
called for linking.  Which reminds me that I need to go back and fix
that.


Re: [kernel-hardening] Re: [PATCH v9 1/4] syscalls: Verify address limit before returning to user-mode

2017-05-10 Thread Arnd Bergmann
On Tue, May 9, 2017 at 6:03 PM, Christoph Hellwig  wrote:
> On Tue, May 09, 2017 at 06:02:50AM -0700, Christoph Hellwig wrote:
>> On Tue, May 09, 2017 at 06:00:01AM -0700, Andy Lutomirski wrote:
>> > fs/splice.c has some, ahem, interesting uses that have been the source
>> > of nasty exploits in the past.  Converting them to use iov_iter
>> > properly would be really, really nice.  Christoph, I don't suppose
>> > you'd like to do that?
>>
>> I can take care of all the fs code including this one.
>
> I spent the afternoon hacking up where I'd like this to head.  It's
> completely untested as of now:
>
> 
> http://git.infradead.org/users/hch/vfs.git/shortlog/refs/heads/setfs-elimination

My older time64_t syscall series has the side-effect of doing something
like this to the time-related compat handlers in kernel/compat.c. If nobody
else has started looking at removing set_fs from those, I can extract
the relevant parts from my series.

  Arnd


Re: [kernel-hardening] Re: [PATCH v9 1/4] syscalls: Verify address limit before returning to user-mode

2017-05-10 Thread Al Viro
On Tue, May 09, 2017 at 11:53:01PM -0700, Christoph Hellwig wrote:
> On Wed, May 10, 2017 at 04:12:54AM +0100, Al Viro wrote:
> > What's the point?  What's wrong with having 
> > kernel_read()/kernel_readv()/etc.?
> > You still have set_fs() in there; doing that one level up in call chain 
> > would
> > be just fine...  IDGI.
> 
> The problem is that they modify the address limit, which the whole
> subthread here wants to get rid of.

And you *still* do the same.  Christoph, this is ridiculous - the worst
part of the area is not a couple of functions in fs/read_write.c, it's
a fucking lot of ->read() and ->write() instances in shitty driver code,
pardon the redundance.  And _that_ is still done under set_fs(KERNEL_DS).

Claiming that set_fs() done one function deeper in callchain (both in
fs/read_write.c) is somehow better because it reduces the amount of code
under that thing...  Get real, please - helpers that encapsulate those
set_fs() pairs (a-la kernel_read(), etc.) absolutely make sense and
converting their open-coded instances to calls of those helpers is clearly
a good thing.  However, we are not
* getting rid of low-quality code run under KERNEL_DS
* gettind rid of set_fs() itself
* getting a generic kernel_read() variant that would really take
an iov_iter.

That's what I'm objecting to.  Centralized kernel_readv() et.al. - sure,
and fs/read_write.c is the right place for those.  No arguments here.
Conversion to those - absolutely; drivers have no fucking business touching
set_fs() at all.  But your primitives are trouble waiting to happen.
Let them take kvec arrays.  And let them, in case when there's no
->read_iter()/->write_iter(), do set_fs().  Statically, without this
if (iter->type & ITER_KVEC) ... stuff.

> > Another delicate place: you can't assume that write() always advances
> > file position by its (positive) return value.  btrfs stuff is sensitive
> > to that.
> 
> If we don't want to assume that we need to pass pointer to pos to
> kernel_read/write.  Which might be a good idea in general.

Yes.

> > ashmem probably _is_ OK with demanding ->read_iter(), but I'm not sure
> > about blind asma->file->f_pos += ret.  That's begging for races.  Actually,
> > scratch that - it *is* racy.
> 
> I think the proper fix is to not even bother to maintain f_pos of the
> backing file, as we don't ever use it - all reads from it pass in
> an explicit position anyway.

vfs_llseek() used by ashmem_llseek()...


Re: [PATCH v6 19/23] drivers/fsi: Add GPIO based FSI master

2017-05-10 Thread Joel Stanley
Hi Chris,

On Tue, Apr 11, 2017 at 5:17 AM, Christopher Bostic
 wrote:

> From: Chris Bostic 
>
> Implement a FSI master using GPIO.  Will generate FSI protocol for
> read and write commands to particular addresses.  Sends master command
> and waits for and decodes a slave response.
>
> Includes changes from Edward A. James  and Jeremy
> Kerr .

I think the series is looking good. I've done a bunch of testing on
some machines, and it worked for me.

I've got a few comments and things to be clarified below.

>
> Signed-off-by: Edward A. James 
> Signed-off-by: Jeremy Kerr 
> Signed-off-by: Chris Bostic 
> Signed-off-by: Joel Stanley 
> ---
>  drivers/fsi/Kconfig   |  11 +
>  drivers/fsi/Makefile  |   1 +
>  drivers/fsi/fsi-master-gpio.c | 610 
> ++
>  3 files changed, 622 insertions(+)
>  create mode 100644 drivers/fsi/fsi-master-gpio.c
>
> diff --git a/drivers/fsi/Kconfig b/drivers/fsi/Kconfig
> index 04c1a0e..448bc3b 100644
> --- a/drivers/fsi/Kconfig
> +++ b/drivers/fsi/Kconfig
> @@ -9,4 +9,15 @@ config FSI
> ---help---
>   FSI - the FRU Support Interface - is a simple bus for low-level
>   access to POWER-based hardware.
> +
> +if FSI
> +
> +config FSI_MASTER_GPIO
> +   tristate "GPIO-based FSI master"
> +   depends on GPIOLIB
> +   ---help---
> +   This option enables a FSI master driver using GPIO lines.
> +
> +endif
> +
>  endmenu
> diff --git a/drivers/fsi/Makefile b/drivers/fsi/Makefile
> index db0e5e7..ed28ac0 100644
> --- a/drivers/fsi/Makefile
> +++ b/drivers/fsi/Makefile
> @@ -1,2 +1,3 @@
>
>  obj-$(CONFIG_FSI) += fsi-core.o
> +obj-$(CONFIG_FSI_MASTER_GPIO) += fsi-master-gpio.o
> diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
> new file mode 100644
> index 000..9fedfaf
> --- /dev/null
> +++ b/drivers/fsi/fsi-master-gpio.c
> @@ -0,0 +1,610 @@
> +/*
> + * A FSI master controller, using a simple GPIO bit-banging interface
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "fsi-master.h"
> +
> +#defineFSI_GPIO_STD_DLY1   /* Standard pin delay in nS */
> +#defineFSI_ECHO_DELAY_CLOCKS   16  /* Number clocks for echo 
> delay */
> +#defineFSI_PRE_BREAK_CLOCKS50  /* Number clocks to prep for 
> break */
> +#defineFSI_BREAK_CLOCKS256 /* Number of clocks to issue 
> break */
> +#defineFSI_POST_BREAK_CLOCKS   16000   /* Number clocks to set up 
> cfam */
> +#defineFSI_INIT_CLOCKS 5000/* Clock out any old data */
> +#defineFSI_GPIO_STD_DELAY  10  /* Standard GPIO delay in nS 
> */
> +   /* todo: adjust down as low as */
> +   /* possible or eliminate */
> +#defineFSI_GPIO_CMD_DPOLL  0x2
> +#defineFSI_GPIO_CMD_TERM   0x3f
> +#define FSI_GPIO_CMD_ABS_AR0x4
> +
> +#defineFSI_GPIO_DPOLL_CLOCKS   100  /* < 21 will cause slave to 
> hang */
> +
> +/* Bus errors */
> +#defineFSI_GPIO_ERR_BUSY   1   /* Slave stuck in busy state 
> */
> +#defineFSI_GPIO_RESP_ERRA  2   /* Any (misc) Error */
> +#defineFSI_GPIO_RESP_ERRC  3   /* Slave reports master CRC 
> error */
> +#defineFSI_GPIO_MTOE   4   /* Master time out error */
> +#defineFSI_GPIO_CRC_INVAL  5   /* Master reports slave CRC 
> error */
> +
> +/* Normal slave responses */
> +#defineFSI_GPIO_RESP_BUSY  1
> +#defineFSI_GPIO_RESP_ACK   0
> +#defineFSI_GPIO_RESP_ACKD  4
> +
> +#defineFSI_GPIO_MAX_BUSY   100
> +#defineFSI_GPIO_MTOE_COUNT 1000
> +#defineFSI_GPIO_DRAIN_BITS 20
> +#defineFSI_GPIO_CRC_SIZE   4
> +#defineFSI_GPIO_MSG_ID_SIZE2
> +#defineFSI_GPIO_MSG_RESPID_SIZE2
> +#defineFSI_GPIO_PRIME_SLAVE_CLOCKS 100
> +
> +static DEFINE_SPINLOCK(fsi_gpio_cmd_lock); /* lock around fsi commands */

Should this be per-master?

> +
> +struct fsi_master_gpio {
> +   struct fsi_master   master;
> +   struct device   *dev;
> +   struct gpio_desc*gpio_clk;
> +   struct gpio_desc*gpio_data;
> +   struct gpio_desc*gpio_trans;/* Voltage translator */
> +   struct gpio_desc*gpio_enable;   /* FSI enable */
> +   struct gpio_desc*gpio_mux;  /* Mux control */
> +};
> +
> +#define to_fsi_master_gpio(m) container_of(m, struct fsi_master_gpio, master)
> +
> +struct fsi_gpio_msg {
> +   uint64_tmsg;
> +   uint8_t bits;
> +};
> +
> +static void clock_toggle(struct fsi_master_gpio *master, int count)
> +{
> +   int i;
> +
> +   for (i = 0; i < count; i++) {
> +   ndelay(FSI_GPIO_STD_DLY);
> + 

Re: [PATCH v6 18/23] drivers/fsi: Document FSI master sysfs files in ABI

2017-05-10 Thread Joel Stanley
On Tue, Apr 11, 2017 at 5:17 AM, Christopher Bostic
 wrote:
> From: Chris Bostic 
>
> Add info for sysfs scan file in Documentaiton ABI/testing

You are missing documentation for the 'raw', 'term' and 'break' files.

> Signed-off-by: Chris Bostic 
> ---
>  Documentation/ABI/testing/sysfs-bus-fsi | 6 ++
>  1 file changed, 6 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-fsi
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-fsi 
> b/Documentation/ABI/testing/sysfs-bus-fsi
> new file mode 100644
> index 000..dfcbc1b
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-fsi
> @@ -0,0 +1,6 @@
> +What:   /sys/bus/platform/devices/fsi-master/scan

This has been renamed 'rescan' in this series. The path has changed too.

> +KernelVersion:  4.9

This should be 4.12 I guess, assuming we get it merged this cycle.

> +Contact:cbos...@us.ibm.com
> +Description:
> +Initiates a FSI master scan for all connected
> +slave devices on its links.
> --
> 1.8.2.2
>


Re: [PATCH v6 09/23] drivers/fsi: scan slaves & register devices

2017-05-10 Thread Joel Stanley
On Tue, Apr 11, 2017 at 5:16 AM, Christopher Bostic
 wrote:
> From: Jeremy Kerr 
>
> Now that we have fsi_slave devices, scan each for endpoints, and
> register them on the fsi bus.
>
> Includes contributions from Chris Bostic 
>
> Signed-off-by: Jeremy Kerr 
> Signed-off-by: Chris Bostic 
> Signed-off-by: Joel Stanley 
> ---
>  drivers/fsi/fsi-core.c | 127 
> +++--
>  include/linux/fsi.h|   4 ++
>  2 files changed, 128 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
> index b7b138b..a8faa89 100644
> --- a/drivers/fsi/fsi-core.c
> +++ b/drivers/fsi/fsi-core.c
> @@ -21,6 +21,19 @@
>
>  #include "fsi-master.h"
>
> +#define FSI_SLAVE_CONF_NEXT_MASK   0x8000
> +#define FSI_SLAVE_CONF_SLOTS_MASK  0x00ff
> +#define FSI_SLAVE_CONF_SLOTS_SHIFT 16
> +#define FSI_SLAVE_CONF_VERSION_MASK0xf000
> +#define FSI_SLAVE_CONF_VERSION_SHIFT   12
> +#define FSI_SLAVE_CONF_TYPE_MASK   0x0ff0
> +#define FSI_SLAVE_CONF_TYPE_SHIFT  4
> +#define FSI_SLAVE_CONF_CRC_SHIFT   4
> +#define FSI_SLAVE_CONF_CRC_MASK0x000f
> +#define FSI_SLAVE_CONF_DATA_BITS   28

You could use GENAMSK for these. It would make it easier to check eg.
that 0x00ff needs to be shifted by 16.


> +
> +static const int engine_page_size = 0x400;
> +
>  #define FSI_SLAVE_BASE 0x800
>
>  /*
> @@ -61,6 +74,30 @@ static int fsi_master_read(struct fsi_master *master, int 
> link,
>  static int fsi_master_write(struct fsi_master *master, int link,
> uint8_t slave_id, uint32_t addr, const void *val, size_t 
> size);
>
> +/* FSI endpoint-device support */
> +
> +static void fsi_device_release(struct device *_device)
> +{
> +   struct fsi_device *device = to_fsi_dev(_device);
> +
> +   kfree(device);
> +}
> +
> +static struct fsi_device *fsi_create_device(struct fsi_slave *slave)
> +{
> +   struct fsi_device *dev;
> +
> +   dev = kzalloc(sizeof(*dev), GFP_KERNEL);
> +   if (!dev)
> +   return NULL;
> +
> +   dev->dev.parent = &slave->dev;
> +   dev->dev.bus = &fsi_bus_type;
> +   dev->dev.release = fsi_device_release;
> +
> +   return dev;
> +}
> +
>  /* crc helpers */
>  static const uint8_t crc4_tab[] = {
> 0x0, 0x7, 0xe, 0x9, 0xb, 0xc, 0x5, 0x2,
> @@ -138,6 +175,91 @@ static int fsi_slave_write(struct fsi_slave *slave, 
> uint32_t addr,
> addr, val, size);
>  }
>
> +static int fsi_slave_scan(struct fsi_slave *slave)
> +{
> +   uint32_t engine_addr;
> +   uint32_t conf;
> +   int rc, i;
> +
> +   /*
> +* scan engines
> +*
> +* We keep the peek mode and slave engines for the core; so start
> +* at the third slot in the configuration table. We also need to
> +* skip the chip ID entry at the start of the address space.
> +*/
> +   engine_addr = engine_page_size * 3;
> +   for (i = 2; i < engine_page_size / sizeof(uint32_t); i++) {
> +   uint8_t slots, version, type, crc;
> +   struct fsi_device *dev;
> +
> +   rc = fsi_slave_read(slave, (i + 1) * sizeof(conf),
> +   &conf, sizeof(conf));
> +   if (rc) {
> +   dev_warn(&slave->dev,
> +   "error reading slave registers\n");
> +   return -1;
> +   }
> +   conf = be32_to_cpu(conf);
> +
> +   crc = fsi_crc4(0, conf, 32);
> +   if (crc) {
> +   dev_warn(&slave->dev,
> +   "crc error in slave register at 0x%04x\n",
> +   i);
> +   return -1;
> +   }
> +
> +   slots = (conf & FSI_SLAVE_CONF_SLOTS_MASK)
> +   >> FSI_SLAVE_CONF_SLOTS_SHIFT;
> +   version = (conf & FSI_SLAVE_CONF_VERSION_MASK)
> +   >> FSI_SLAVE_CONF_VERSION_SHIFT;
> +   type = (conf & FSI_SLAVE_CONF_TYPE_MASK)
> +   >> FSI_SLAVE_CONF_TYPE_SHIFT;
> +
> +   /*
> +* Unused address areas are marked by a zero type value; this
> +* skips the defined address areas
> +*/
> +   if (type != 0 && slots != 0) {
> +
> +   /* create device */
> +   dev = fsi_create_device(slave);
> +   if (!dev)
> +   return -ENOMEM;
> +
> +   dev->slave = slave;
> +   dev->engine_type = type;
> +   dev->version = version;
> +   dev->unit = i;
> +   dev->addr = engine_addr;
> +   dev->size = slots * engine_page_size;
> +
> +   dev_info(&slave->dev,
> +  

Re: [PATCH v6 11/23] drivers/fsi: Add master unscan

2017-05-10 Thread Joel Stanley
On Tue, Apr 11, 2017 at 5:16 AM, Christopher Bostic
 wrote:
> From: Chris Bostic 
>
> Allow a master to undo a previous scan.  Should a master scan a bus
> twice it will need to ensure it doesn't double register any
> previously detected device.
>
> Signed-off-by: Chris Bostic 
> Signed-off-by: Joel Stanley 
> ---
>  drivers/fsi/fsi-core.c | 40 
>  1 file changed, 40 insertions(+)
>
> diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
> index 4da0b030..75d2a88 100644
> --- a/drivers/fsi/fsi-core.c
> +++ b/drivers/fsi/fsi-core.c
> @@ -69,6 +69,7 @@ struct fsi_slave {
> uint32_tsize;   /* size of slave address space */
>  };
>
> +#define to_fsi_master(d) container_of(d, struct fsi_master, dev)
>  #define to_fsi_slave(d) container_of(d, struct fsi_slave, dev)
>
>  static int fsi_master_read(struct fsi_master *master, int link,
> @@ -491,6 +492,37 @@ static int fsi_master_scan(struct fsi_master *master)
> return 0;
>  }
>
> +static int __fsi_slave_remove_device(struct device *dev, void *arg)
> +{
> +   device_unregister(dev);
> +   return 0;
> +}
> +
> +static int __fsi_master_remove_slave(struct device *dev, void *arg)
> +{
> +   device_for_each_child(dev, NULL, __fsi_slave_remove_device);
> +   device_unregister(dev);
> +   return 0;
> +}

I can't see why the two above functions to have the __ prefix.

> +
> +static void fsi_master_unscan(struct fsi_master *master)
> +{
> +   device_for_each_child(&master->dev, NULL, __fsi_master_remove_slave);
> +}
> +
> +static ssize_t master_rescan_store(struct device *dev,
> +   struct device_attribute *attr, const char *buf, size_t count)
> +{
> +   struct fsi_master *master = to_fsi_master(dev);
> +
> +   fsi_master_unscan(master);
> +   fsi_master_scan(master);

These function can return errors. Do you want to return those errors
to userspace?

> +
> +   return count;
> +}
> +
> +static DEVICE_ATTR(rescan, 0200, NULL, master_rescan_store);
> +
>  int fsi_master_register(struct fsi_master *master)
>  {
> int rc;
> @@ -507,7 +539,15 @@ int fsi_master_register(struct fsi_master *master)
> return rc;
> }
>
> +   rc = device_create_file(&master->dev, &dev_attr_rescan);
> +   if (rc) {
> +   device_unregister(&master->dev);
> +   ida_simple_remove(&master_ida, master->idx);
> +   return rc;
> +   }
> +
> fsi_master_scan(master);
> +
> return 0;
>  }
>  EXPORT_SYMBOL_GPL(fsi_master_register);
> --
> 1.8.2.2
>


Re: [kernel-hardening] Re: [PATCH v9 1/4] syscalls: Verify address limit before returning to user-mode

2017-05-10 Thread Christoph Hellwig
On Wed, May 10, 2017 at 08:27:47AM +0100, Al Viro wrote:
> And you *still* do the same.  Christoph, this is ridiculous - the worst
> part of the area is not a couple of functions in fs/read_write.c, it's
> a fucking lot of ->read() and ->write() instances in shitty driver code,
> pardon the redundance.  And _that_ is still done under set_fs(KERNEL_DS).

For now yes.  But it provides a sane API on top, and then we can
start moving the drivers that matter to the iter variants and drop
support for the rest soon.  Most in-kernel I/O is done to files, and
the rest to a very limited set of devices. (not accounting for sockets
through their own APIs, thats another story)

> That's what I'm objecting to.  Centralized kernel_readv() et.al. - sure,
> and fs/read_write.c is the right place for those.  No arguments here.
> Conversion to those - absolutely; drivers have no fucking business touching
> set_fs() at all.  But your primitives are trouble waiting to happen.
> Let them take kvec arrays.  And let them, in case when there's no
> ->read_iter()/->write_iter(), do set_fs().  Statically, without this
> if (iter->type & ITER_KVEC) ... stuff.

Oh weĺl.  I can do that first, but I think eventually we'll have to
get back to what I've done now.


Re: [kernel-hardening] Re: [PATCH v9 1/4] syscalls: Verify address limit before returning to user-mode

2017-05-10 Thread Christoph Hellwig
On Wed, May 10, 2017 at 09:28:48AM +0200, Arnd Bergmann wrote:
> My older time64_t syscall series has the side-effect of doing something
> like this to the time-related compat handlers in kernel/compat.c. If nobody
> else has started looking at removing set_fs from those, I can extract
> the relevant parts from my series.

That would be great.


Re: [kernel-hardening] Re: [PATCH v9 1/4] syscalls: Verify address limit before returning to user-mode

2017-05-10 Thread Arnd Bergmann
On Tue, May 9, 2017 at 3:00 PM, Andy Lutomirski  wrote:
> On Tue, May 9, 2017 at 1:56 AM, Christoph Hellwig  wrote:
>> On Tue, May 09, 2017 at 08:45:22AM +0200, Ingo Molnar wrote:
>>> We only have ~115 code blocks in the kernel that set/restore KERNEL_DS, it 
>>> would
>>> be a pity to add a runtime check to every system call ...
>>
>> I think we should simply strive to remove all of them that aren't
>> in core scheduler / arch code.  Basically evetyytime we do the
>>
>> oldfs = get_fs();
>> set_fs(KERNEL_DS);
>> ..
>> set_fs(oldfs);
>>
>> trick we're doing something wrong, and there should always be better
>> ways to archive it.  E.g. using iov_iter with a ITER_KVEC type
>> consistently would already remove most of them.
>
> How about trying to remove all of them?  If we could actually get rid
> of all of them, we could drop the arch support, and we'd get faster,
> simpler, shorter uaccess code throughout the kernel.
>
> The ones in kernel/compat.c are generally garbage.  They should be
> using compat_alloc_user_space().  Ditto for kernel/power/user.c.

compat_alloc_user_space() has some problems too, it adds
complexity to a rarely-tested code path and can add some noticeable
overhead in cases where user space access is slow because of
extra checks.

It's clearly better than set_fs(), but the way I prefer to convert the
code is to avoid both and instead move compat handlers next to
the native code, and splitting out the common code between native
and compat mode into a helper that takes a regular kernel pointer.

I think that's what both Al has done in the past on compat_ioctl()
and select() and what Christoph does in his latest series, but
it seems worth pointing out for others that decide to help out here.

 Arnd


Re: [PATCH 7/7] DWARF: add the config option

2017-05-10 Thread Jiri Slaby
On 05/05/2017, 09:57 PM, Linus Torvalds wrote:
> On Fri, May 5, 2017 at 5:22 AM, Jiri Slaby  wrote:
>> The DWARF unwinder is in place and ready. So introduce the config option
>> to allow users to enable it. It is by default off due to missing
>> assembly annotations.
> 
> Who actually ends up using this?

Every SUSE user has been using this for almost a decade and we are not
about to switch to FP for performance reasons as noted by Jiri Kosina.
So SUSE users are going to be exposed to DWARF unwinder for another
decade or so at least.

Therefore, this is another attempt to make the unwinder (in some form)
upstream. Since this was first proposed many years ago, we have been
forced to forward-port it over and over and everyone knows what pain it
is. So it is nice, that this opened the discussion at least.

> Because from the last time we had fancy unwindoers, and all the
> problems it caused for oops handling with absolutely _zero_ upsides
> ever, I do not ever again want to see fancy unwinders with complex
> state machine handling used by the oopsing code.

Well, reliable stack-traces with minimal performance impact thanks to
out-of-band data is hell good reason in my opinion.

> The fact that it gets disabled for KASAN also makes me suspicious. It
> basically means that now all the accesses it does are not even
> validated.

OK, I inclined to disable KASAN when I started cleaning this up for
_performance_ reasons. The system was so slow, that the RCU stall or
soft-lockup detectors came up complaining. From that time, I measured
the bottlenecks and optimized the unwinder so that 1000 iterations of
unwinding takes:

Before:
real0m1.808s
user0m0.001s
sys 0m1.807s

After:
real0m0.018s
user0m0.001s
sys 0m0.017s

So let me check, whether KASAN still has to be disabled globally. I do
not think so.

OTOH, TBH, I am not sure KASAN can be enabled for dwarf.c, the same as
holds now for the rest of the current unwinding:
KASAN_SANITIZE_dumpstack.o  := n
KASAN_SANITIZE_dumpstack_$(BITS).o  := n
KASAN_SANITIZE_stacktrace.o := n

Still, I can let KASAN := y for dwarf.c for testing purposes locally and
smoke-test the unwinder.

> The fact that the most of the code seems to be disabled for the first
> six patches, and then just enabled in the last patch, also seems to
> mean that the series also gets no bisection coverage or testing that
> the individual patches make any sense. (ie there's a lot of code
> inside "CONFIG_DWARF_UNWIND" in the early patches but that config
> option cannot even be enabled until the last patch).

Correct. This was one big patch previously. I separated that patch into
several smaller commits touching different places of the kernel for
easier review.

It does not make sense to test any of the patches separately except the
first. Hence the config option which enables the rest of the series is
the last one. I deemed this as one of possible approaches to split
patches (I have seen this many times in the past.) I can of course
squash this back into a single patch (or two).

> We used to have nasty issues with not just missing dwarf info, but
> also actively *wrong* dwarf info. Compiler versions that generated
> subtly wrong info, because nobody actually really depended on it, and
> the people who had tested it seldom did the kinds of things we do in 
> the kernel (eg inline asms etc).

I must admit I am not aware of any issues in this manner during the
years. Again, this unwinder is the default in SUSE kernels since ever,
so we have been using gcc from at least 3.2 to 7. But see below.

> So I'm personally still very suspicious of these things.
> 
> Last time I had huge issues with people also always blaming *anything*
> else than that unwinder. It was always "oh, somebody wrote asm without
> getting it right". Or "oh, the compiler generated bad tables, it's not
> *my* fault that now the kernel oopsing code no longer works".

Now we have objtool. My objtool clone:
1) verifies the DWARF data (prepared by Josh)

2) generates DWARF data for assembly -- incomplete yet: see the thread
about x86 assembly cleanup which is a pre-requisite for this to work.
This is BTW the reason why the DWARF unwinder is default-off in this
series yet.

And we can add:
3) fix up the data, if they are wrong

That said, objtool could handle the data so they are correct and
as-expected for the unwinder. Without objtool, the data (and unwinder)
is hopeless (only vdso from all the assembly is annotated.)

> When I asked for more stricter debug table validation to avoid issues,
> it was always "oh, we fixed it, no worries", and then two months later
> somebody hit another issue.

Reasonable, indeed. I am all for strict checking. objtool is to do that.

> Put another way; the last time we did crazy stuff like this, it got
> reverted. For a damn good reason, despite some people being in denial
> about those reasons.

Speaking for myself, having it out-of-tree ca

[PATCH] scsi: sg: don't return bogus Sg_requests

2017-05-10 Thread Johannes Thumshirn
If the list search in sg_get_rq_mark() fails to find a valid request, we
return a bogus element. This then can later lead to a GPF in sg_remove_scat().

So don't return bogus Sg_requests in sg_get_rq_mark() but NULL in case the
list search doesn't find a valid request.

Signed-off-by: Johannes Thumshirn 
Reported-by: Andrey Konovalov 
Cc: Hannes Reinecke 
Cc: Christoph Hellwig 
Cc: Doug Gilbert 
---
 drivers/scsi/sg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 0a38ba01b7b4..abfde23fa186 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -2078,7 +2078,7 @@ sg_get_rq_mark(Sg_fd * sfp, int pack_id)
}
}
write_unlock_irqrestore(&sfp->rq_list_lock, iflags);
-   return resp;
+   return (resp->done == 2) ? resp : NULL;
 }
 
 /* always adds to end of list */
-- 
2.12.0



Re: [PATCH v2 2/2] mm: skip HWPoisoned pages when onlining pages

2017-05-10 Thread Michal Hocko
On Fri 28-04-17 08:51:31, Michal Hocko wrote:
> On Fri 28-04-17 08:50:50, Michal Hocko wrote:
> > [Drop Wen Congyang because his address bounces - we will have to find
> > out ourselves...]
> > On Fri 28-04-17 08:30:48, Michal Hocko wrote:
> > > On Wed 26-04-17 03:13:04, Naoya Horiguchi wrote:
> > > > On Wed, Apr 26, 2017 at 12:10:15PM +1000, Balbir Singh wrote:
> > > > > On Tue, 2017-04-25 at 16:27 +0200, Laurent Dufour wrote:
> > > > > > The commit b023f46813cd ("memory-hotplug: skip HWPoisoned page when
> > > > > > offlining pages") skip the HWPoisoned pages when offlining pages, 
> > > > > > but
> > > > > > this should be skipped when onlining the pages too.
> > > > > >
> > > > > > Signed-off-by: Laurent Dufour 
> > > > > > ---
> > > > > >  mm/memory_hotplug.c | 4 
> > > > > >  1 file changed, 4 insertions(+)
> > > > > >
> > > > > > diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> > > > > > index 6fa7208bcd56..741ddb50e7d2 100644
> > > > > > --- a/mm/memory_hotplug.c
> > > > > > +++ b/mm/memory_hotplug.c
> > > > > > @@ -942,6 +942,10 @@ static int online_pages_range(unsigned long 
> > > > > > start_pfn, unsigned long nr_pages,
> > > > > > if (PageReserved(pfn_to_page(start_pfn)))
> > > > > > for (i = 0; i < nr_pages; i++) {
> > > > > > page = pfn_to_page(start_pfn + i);
> > > > > > +   if (PageHWPoison(page)) {
> > > > > > +   ClearPageReserved(page);
> > > > >
> > > > > Why do we clear page reserved? Also if the page is marked 
> > > > > PageHWPoison, it
> > > > > was never offlined to begin with? Or do you expect this to be set on 
> > > > > newly
> > > > > hotplugged memory? Also don't we need to skip the entire pageblock?
> > > > 
> > > > If I read correctly, to "skip HWPoiosned page" in commit b023f46813cd 
> > > > means
> > > > that we skip the page status check for hwpoisoned pages *not* to prevent
> > > > memory offlining for memblocks with hwpoisoned pages. That means that
> > > > hwpoisoned pages can be offlined.
> > > 
> > > Is this patch actually correct? I am trying to wrap my head around it
> > > but it smells like it tries to avoid the problem rather than fix it
> > > properly. I might be wrong here of course but to me it sounds like
> > > poisoned page should simply be offlined and keep its poison state all
> > > the time. If the memory is hot-removed and added again we have lost the
> > > struct page along with the state which is the expected behavior. If it
> > > is still broken we will re-poison it.
> > > 
> > > Anyway a patch to skip over poisoned pages during online makes perfect
> > > sense to me. The PageReserved fiddling around much less so.
> > > 
> > > Or am I missing something. Let's CC Wen Congyang for the clarification
> > > here.

Can we revisit this please? The PageReserved() logic for poisoned pages
is completely unclear to me. I would rather not rely on the previous
changelogs and rather build the picture from what is the expected
behavior instead.
-- 
Michal Hocko
SUSE Labs


Re: [PATCH] irq_bcm2836: Send event when onlining sleeping cores

2017-05-10 Thread Marc Zyngier
On 09/05/17 20:02, Phil Elwell wrote:
> On 09/05/2017 19:53, Marc Zyngier wrote:
>> On 09/05/17 19:52, Phil Elwell wrote:
>>> On 09/05/2017 19:14, Marc Zyngier wrote:
 On 09/05/17 19:08, Eric Anholt wrote:
> Marc Zyngier  writes:
>
>> On 09/05/17 17:59, Eric Anholt wrote:
>>> Phil Elwell  writes:
>>>
 In order to reduce power consumption and bus traffic, it is sensible
 for secondary cores to enter a low-power idle state when waiting to
 be started. The wfe instruction causes a core to wait until an event
 or interrupt arrives before continuing to the next instruction.
 The sev instruction sends a wakeup event to the other cores, so call
 it from bcm2836_smp_boot_secondary, the function that wakes up the
 waiting cores during booting.

 It is harmless to use this patch without the corresponding change
 adding wfe to the ARMv7/ARMv8-32 stubs, but if the stubs are updated
 and this patch is not applied then the other cores will sleep forever.

 See: https://github.com/raspberrypi/linux/issues/1989

 Signed-off-by: Phil Elwell 
 ---
  drivers/irqchip/irq-bcm2836.c | 3 +++
  1 file changed, 3 insertions(+)

 diff --git a/drivers/irqchip/irq-bcm2836.c 
 b/drivers/irqchip/irq-bcm2836.c
 index e10597c..6dccdf9 100644
 --- a/drivers/irqchip/irq-bcm2836.c
 +++ b/drivers/irqchip/irq-bcm2836.c
 @@ -248,6 +248,9 @@ static int __init 
 bcm2836_smp_boot_secondary(unsigned int cpu,
writel(secondary_startup_phys,
   intc.base + LOCAL_MAILBOX3_SET0 + 16 * cpu);

 +  dsb(sy); /* Ensure write has completed before waking the other 
 CPUs */
 +  sev();
 +
return 0;
  }
>>>
>>> This is also the behavior that the standard arm64 spin-table method has,
>>> which we unfortunately can't quite use.
>>
>> And why is that so? Why do you have to reinvent the wheel (and hide the
>> cloned wheel in an interrupt controller driver)?
>>
>> That doesn't seem right to me.
>
> The armv8 stubs (firmware-supplied code in the low page that do the
> spinning) do actually implement arm64's spin-table method.  It's the
> armv7 stubs that use these registers in the irqchip instead of plain
> addresses in system memory.

 Let's put ARMv7 aside for the time being. If your firmware already
 implements spin-tables, why don't you simply use that at least on arm64?
>>>
>>> We do.
>>
>> Obviously not the way it is intended if you have to duplicate the core
>> architectural code in the interrupt controller driver, which couldn't
>> care less.
> 
> If we were using this method on arm64 then the other cores would not start up
> because armstub8.S has always included a wfe. Nothing in the commit mentions
> arm64 - this is an ARCH=arm fix.

Thanks for the clarification, which you could have added to the commit
message.

The question still remains: why do we have CPU bring-up code in an
interrupt controller, instead of having it in the architecture code?

The RPi-2 is the *only* platform to have its SMP bringup code outside of
arch/arm, so the first course of action would be to move that code where
it belongs.

Thanks,

M.
-- 
Jazz is not dead. It just smells funny...


Re: RFC v2: post-init-read-only protection for data allocated dynamically

2017-05-10 Thread Michal Hocko
On Fri 05-05-17 15:19:19, Igor Stoppa wrote:
> 
> 
> On 04/05/17 17:01, Michal Hocko wrote:
> > On Thu 04-05-17 16:37:55, Igor Stoppa wrote:
> 
> [...]
> 
> >> The disadvantage is that anything can happen, undetected, while the seal
> >> is lifted.
> > 
> > Yes and I think this makes it basically pointless
> 
> ok, this goes a bit beyond what I had in mind initially, but I see your
> point
> 
> [...]
> 
> > Just to make my proposal more clear. I suggest the following workflow
> > 
> > cache = kmem_cache_create(foo, object_size, ..., SLAB_SEAL);
> >
> > obj = kmem_cache_alloc(cache, gfp_mask);
> > init_obj(obj)
> > [more allocations]
> > kmem_cache_seal(cache);
> 
> In case one doesn't want the feature, at which point would it be disabled?
> 
> * not creating the slab
> * not sealing it
> * something else?

If the sealing would be disabled then sealing would be a noop.

-- 
Michal Hocko
SUSE Labs


Re: [PATCH 7/7] DWARF: add the config option

2017-05-10 Thread Jiri Slaby
On 05/06/2017, 09:19 AM, Ingo Molnar wrote:
> 
> * Linus Torvalds  wrote:
> 
>> On Fri, May 5, 2017 at 5:22 AM, Jiri Slaby  wrote:
>>> The DWARF unwinder is in place and ready. So introduce the config option
>>> to allow users to enable it. It is by default off due to missing
>>> assembly annotations.
>>
>> Who actually ends up using this?
> 
> Also, why wasn't Josh Poimboeuf Cc:-ed, who de-facto maintains the x86 
> unwinding 
> code?

I explicitly CCed Josh on 5/7 and 6/7 which touches the code. Besides
that, I assumed he is implicitly CCed via live-patch...@vger.kernel.org
which is carbon-copied on each of the patches.

> AFAICS this series is just repeating the old mistakes of the old Dwarf 
> unwinders 
> of trusting GCC's unwinder data. So NAK for the time being on the whole 
> approach:
> 
> NAcked-by: Ingo Molnar 

OK, as the series stands now, we indeed do. Noteworthy, we, in SUSE, had
no problems with this reliance for all the time we have been using the
unwinder.

Anyway, objtool is about to vaidate the DWARF data, generate it for
assembly and potentially fix it if problems occur. Could you elaborate
on what else would help you to change your stance?

thanks,
-- 
js
suse labs


Re: [PATCH] scsi: sg: don't return bogus Sg_requests

2017-05-10 Thread Hannes Reinecke
On 05/10/2017 09:41 AM, Johannes Thumshirn wrote:
> If the list search in sg_get_rq_mark() fails to find a valid request, we
> return a bogus element. This then can later lead to a GPF in sg_remove_scat().
> 
> So don't return bogus Sg_requests in sg_get_rq_mark() but NULL in case the
> list search doesn't find a valid request.
> 
> Signed-off-by: Johannes Thumshirn 
> Reported-by: Andrey Konovalov 
> Cc: Hannes Reinecke 
> Cc: Christoph Hellwig 
> Cc: Doug Gilbert 
> ---
>  drivers/scsi/sg.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
> index 0a38ba01b7b4..abfde23fa186 100644
> --- a/drivers/scsi/sg.c
> +++ b/drivers/scsi/sg.c
> @@ -2078,7 +2078,7 @@ sg_get_rq_mark(Sg_fd * sfp, int pack_id)
>   }
>   }
>   write_unlock_irqrestore(&sfp->rq_list_lock, iflags);
> - return resp;
> + return (resp->done == 2) ? resp : NULL;
>  }
>  
>  /* always adds to end of list */
> 
Not quite.
Please return 'resp' directly from within the loop.
With this fix we run into the risk that by chance the uninitialized
'resp' pointer contains a '2' at the 'done' location, triggering this
issue again.

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH] efi/libstub: Indicate clang the relocation mode for arm64

2017-05-10 Thread Ard Biesheuvel
On 9 May 2017 at 22:49, Matthias Kaehlcke  wrote:
> El Tue, May 09, 2017 at 01:50:36PM -0700 Greg Hackmann ha dit:
>
>> On 05/09/2017 12:36 PM, Matthias Kaehlcke wrote:
>> >From: Greg Hackmann 
>> >
>> >Without any extra guidance, clang will generate libstub with either
>> >absolute or relative ELF relocations. Use the right combination of
>> >-fpic and -fno-pic on different files to avoid this.
>> >
>> >Signed-off-by: Greg Hackmann 
>> >Signed-off-by: Bernhard Rosenkränzer 
>> >Signed-off-by: Matthias Kaehlcke 
>> >---
>> > drivers/firmware/efi/libstub/Makefile | 6 ++
>> > 1 file changed, 6 insertions(+)
>> >
>> >diff --git a/drivers/firmware/efi/libstub/Makefile 
>> >b/drivers/firmware/efi/libstub/Makefile
>> >index f7425960f6a5..ccbaaf4d8650 100644
>> >--- a/drivers/firmware/efi/libstub/Makefile
>> >+++ b/drivers/firmware/efi/libstub/Makefile
>> >@@ -11,6 +11,9 @@ cflags-$(CONFIG_X86)   += -m$(BITS) 
>> >-D__KERNEL__ -O2 \
>> >-mno-mmx -mno-sse
>> >
>> > cflags-$(CONFIG_ARM64)  := $(subst -pg,,$(KBUILD_CFLAGS))
>> >+ifeq ($(cc-name),clang)
>> >+cflags-$(CONFIG_ARM64)  += -fpic
>> >+endif
>> > cflags-$(CONFIG_ARM):= $(subst -pg,,$(KBUILD_CFLAGS)) \
>> >-fno-builtin -fpic -mno-single-pic-base
>> >
>> >@@ -38,6 +41,9 @@ $(obj)/lib-%.o: $(srctree)/lib/%.c FORCE
>> >
>> > lib-$(CONFIG_EFI_ARMSTUB)   += arm-stub.o fdt.o string.o random.o \
>> >$(patsubst %.c,lib-%.o,$(arm-deps))
>> >+ifeq ($(cc-name),clang)
>> >+CFLAGS_arm64-stub.o+= -fno-pic
>> >+endif
>> >
>> > lib-$(CONFIG_ARM)   += arm32-stub.o
>> > lib-$(CONFIG_ARM64) += arm64-stub.o
>> >
>>
>> NAK.
>>
>> This patch was labeled "HACK:" in our experimental tree.  There's no
>> rhyme or reason to why this combination of -f[no-]pic flags
>> generates code without problematic relocations.  It's inherently
>> fragile, and was only intended as a temporary workaround until I (or
>> someone more familiar with EFI) got a chance to revisit the problem.
>>
>> Unless the gcc CFLAGS are also an artifact of "mess with -f[no-]pic
>> until the compiler generates what you want", this doesn't belong
>> upstream.
>
> Sorry, I didn't realize it is that bad of a hack. Unfortunately I'm
> not very familiar with EFI either.
>
> I saw Ard did some work in this code related with relocation, maybe he
> can provide a pointer towards a better solution.
>

This is a known issue. The problem is that generic AArch64 small model
code is mostly position independent already, due to its use of
adrp/add pairs to generate symbol references with a +/- 4 GB range.
Building the same code with -fpic will result in GOT entries to be
generated, which carry absolute addresses, so this achieves the exact
opposite of what we want.

The reason for the GOT entries is that GCC (and Clang, apparently)
infer from the -fpic flag that you are building objects that will be
linked into a shared library, to which ELF symbol preemption rules
apply that stipulate that a symbol in the main executable supersedes a
symbol under the same name in the shared library, and that the shared
library should update all its internal references to the main
executable's version of the symbol. The easiest way (but certainly not
the only way) to achieve that is to indirect all internal symbol
references via GOT entries, which can be made to refer to another
symbol by updating a single value.

The workaround I used is to use hidden visibility, using a #pragma.
(There is a -fvisibility=hidden command line option as well, but this
is a weaker form that does not apply to extern declarations, only to
definitions). So if you add

#pragma GCC visibility push(hidden)

at the beginning of arm64-stub.c (and perhaps to one or two other
files that contain externally visible symbol declarations these days),
you should be able to compile the entire EFI stub with -fpic. Note
that making those externally visible symbols 'static' where possible
would solve the problem as well, but this triggers another issue in
the 32-bit ARM stub.

In my opinion, the correct fix would be to make -fpie (as opposed to
-fpic) imply hidden visibility, given that PIE executables don't
export symbols in the first place, and so the preemption rules do not
apply. It is worth a try whether -fpie works as expected in this case
on Clang, but the last time I tried it on GCC, it behaved exactly like
-fpic.


[PATCH v2] scsi: sg: don't return bogus Sg_requests

2017-05-10 Thread Johannes Thumshirn
If the list search in sg_get_rq_mark() fails to find a valid request, we
return a bogus element. This then can later lead to a GPF in sg_remove_scat().

So don't return bogus Sg_requests in sg_get_rq_mark() but NULL in case the
list search doesn't find a valid request.

Signed-off-by: Johannes Thumshirn 
Reported-by: Andrey Konovalov 
Cc: Hannes Reinecke 
Cc: Christoph Hellwig 
Cc: Doug Gilbert 
---
Changes to v1:
* Directly return found element within the loop (Hannes)

 drivers/scsi/sg.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 0a38ba01b7b4..82c33a6edbea 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -2074,11 +2074,12 @@ sg_get_rq_mark(Sg_fd * sfp, int pack_id)
if ((1 == resp->done) && (!resp->sg_io_owned) &&
((-1 == pack_id) || (resp->header.pack_id == pack_id))) {
resp->done = 2; /* guard against other readers */
-   break;
+   write_unlock_irqrestore(&sfp->rq_list_lock, iflags);
+   return resp;
}
}
write_unlock_irqrestore(&sfp->rq_list_lock, iflags);
-   return resp;
+   return NULL;
 }
 
 /* always adds to end of list */
-- 
2.12.0



Re: [PATCH] gpu: drm: gma500: remove dead code

2017-05-10 Thread Daniel Vetter
On Tue, May 09, 2017 at 10:22:21AM -0500, Gustavo A. R. Silva wrote:
> Local variable use_gct is assigned to a constant value and it is never
> updated again. Remove this variable and the dead code it guards.
> 
> Addresses-Coverity-ID: 145690
> Signed-off-by: Gustavo A. R. Silva 

Looks reasonable, applied to drm-misc for 4.13.

Thanks, Daniel

> ---
>  drivers/gpu/drm/gma500/mdfld_tpo_vid.c | 51 
> ++
>  1 file changed, 9 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/gpu/drm/gma500/mdfld_tpo_vid.c 
> b/drivers/gpu/drm/gma500/mdfld_tpo_vid.c
> index d8d4170..d40628e 100644
> --- a/drivers/gpu/drm/gma500/mdfld_tpo_vid.c
> +++ b/drivers/gpu/drm/gma500/mdfld_tpo_vid.c
> @@ -32,53 +32,20 @@ static struct drm_display_mode 
> *tpo_vid_get_config_mode(struct drm_device *dev)
>   struct drm_display_mode *mode;
>   struct drm_psb_private *dev_priv = dev->dev_private;
>   struct oaktrail_timing_info *ti = &dev_priv->gct_data.DTD;
> - bool use_gct = false;
>  
>   mode = kzalloc(sizeof(*mode), GFP_KERNEL);
>   if (!mode)
>   return NULL;
>  
> - if (use_gct) {
> - mode->hdisplay = (ti->hactive_hi << 8) | ti->hactive_lo;
> - mode->vdisplay = (ti->vactive_hi << 8) | ti->vactive_lo;
> - mode->hsync_start = mode->hdisplay +
> - ((ti->hsync_offset_hi << 8) |
> - ti->hsync_offset_lo);
> - mode->hsync_end = mode->hsync_start +
> - ((ti->hsync_pulse_width_hi << 8) |
> - ti->hsync_pulse_width_lo);
> - mode->htotal = mode->hdisplay + ((ti->hblank_hi << 8) |
> - ti->hblank_lo);
> - mode->vsync_start =
> - mode->vdisplay + ((ti->vsync_offset_hi << 8) |
> - ti->vsync_offset_lo);
> - mode->vsync_end =
> - mode->vsync_start + ((ti->vsync_pulse_width_hi << 8) |
> - ti->vsync_pulse_width_lo);
> - mode->vtotal = mode->vdisplay +
> - ((ti->vblank_hi << 8) | ti->vblank_lo);
> - mode->clock = ti->pixel_clock * 10;
> -
> - dev_dbg(dev->dev, "hdisplay is %d\n", mode->hdisplay);
> - dev_dbg(dev->dev, "vdisplay is %d\n", mode->vdisplay);
> - dev_dbg(dev->dev, "HSS is %d\n", mode->hsync_start);
> - dev_dbg(dev->dev, "HSE is %d\n", mode->hsync_end);
> - dev_dbg(dev->dev, "htotal is %d\n", mode->htotal);
> - dev_dbg(dev->dev, "VSS is %d\n", mode->vsync_start);
> - dev_dbg(dev->dev, "VSE is %d\n", mode->vsync_end);
> - dev_dbg(dev->dev, "vtotal is %d\n", mode->vtotal);
> - dev_dbg(dev->dev, "clock is %d\n", mode->clock);
> - } else {
> - mode->hdisplay = 864;
> - mode->vdisplay = 480;
> - mode->hsync_start = 873;
> - mode->hsync_end = 876;
> - mode->htotal = 887;
> - mode->vsync_start = 487;
> - mode->vsync_end = 490;
> - mode->vtotal = 499;
> - mode->clock = 33264;
> - }
> + mode->hdisplay = 864;
> + mode->vdisplay = 480;
> + mode->hsync_start = 873;
> + mode->hsync_end = 876;
> + mode->htotal = 887;
> + mode->vsync_start = 487;
> + mode->vsync_end = 490;
> + mode->vtotal = 499;
> + mode->clock = 33264;
>  
>   drm_mode_set_name(mode);
>   drm_mode_set_crtcinfo(mode, 0);
> -- 
> 2.5.0
> 
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


Re: [linux-next][bock] [bisected c20cfc27a] WARNING: CPU: 22 PID: 0 at block/blk-core.c:2655 .blk_update_request+0x4f8/0x500

2017-05-10 Thread Christoph Hellwig
On Tue, May 09, 2017 at 08:48:21PM +0530, Abdul Haleem wrote:
> A bisection for the above suspects resulted a bad commit;
> 
> c20cfc27a47307e811346f85959cf3cc07ae42f9 is the first bad commit
> commit c20cfc27a47307e811346f85959cf3cc07ae42f9
> Author: Christoph Hellwig 
> Date:   Wed Apr 5 19:21:07 2017 +0200
> 
> block: stop using blkdev_issue_write_same for zeroing

And this effectively switches us to use the write_zeroes for SCSI.



> 
> We'll always use the WRITE ZEROES code for zeroing now.
> 
> Signed-off-by: Christoph Hellwig 
> Reviewed-by: Martin K. Petersen 
> Reviewed-by: Hannes Reinecke 
> Signed-off-by: Jens Axboe 
> 
> 
> @Christoph FYI, the machine configured with 64K page size
> > 
> > WARNING: CPU: 12 PID: 0 at block/blk-core.c:2651 
> > .blk_update_request+0x4cc/0x4e0

Can you decode which warning this is?  Is it:

WARN_ON_ONCE(req->rq_flags & RQF_SPECIAL_PAYLOAD);

?  In which case your setup did a partial completion of a WRITE SAME
command, which is perfectly legal according to SCSI, but a bit unusual.


Re: [PATCH v3 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe

2017-05-10 Thread Matthias Brugger



On 10/05/17 04:07, Ryder Lee wrote:

Add documentation for PCIe host driver available in MT7623
series SoCs.

Signed-off-by: Ryder Lee 
Acked-by: Rob Herring 
---
  .../bindings/pci/mediatek,mt7623-pcie.txt  | 149 +
  1 file changed, 149 insertions(+)
  create mode 100644 
Documentation/devicetree/bindings/pci/mediatek,mt7623-pcie.txt

diff --git a/Documentation/devicetree/bindings/pci/mediatek,mt7623-pcie.txt 
b/Documentation/devicetree/bindings/pci/mediatek,mt7623-pcie.txt
new file mode 100644
index 000..a9393ce
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/mediatek,mt7623-pcie.txt
@@ -0,0 +1,149 @@
+Mediatek Gen2 PCIe controller which is available on MT7623 series SoCs
+
+PCIe subsys supports single root complex (RC) with 3 Root Ports. Each root
+ports supports a Gen2 1-lane Link and has PIPE interface to PHY.
+
+Required properties:
+- compatible: Should contain "mediatek,mt7623-pcie".
+- device_type: Must be "pci"
+- reg: Base addresses and lengths of the PCIe controller.
+- #address-cells: Address representation for root ports (must be 3)
+- #size-cells: Size representation for root ports (must be 2)
+- #interrupt-cells: Size representation for interrupts (must be 1)
+- interrupts: Three interrupt outputs of the controller. Must contain an
+  entry for each entry in the interrupt-names property.
+- interrupt-names: Must include the following names
+  - "pcie-int0"
+  - "pcie-int1"
+  - "pcie-int2"
+- interrupt-map-mask and interrupt-map: Standard PCI IRQ mapping properties
+  Please refer to the standard PCI bus binding document for a more detailed
+  explanation.
+- clocks: Must contain an entry for each entry in clock-names.
+  See ../clocks/clock-bindings.txt for details.
+- clock-names: Must include the following entries:
+  - free_ck :for reference clock of PCIe subsys
+  - sys_ck0 :for clock of Port0
+  - sys_ck1 :for clock of Port1
+  - sys_ck2 :for clock of Port2
+- resets: Must contain an entry for each entry in reset-names.
+  See ../reset/reset.txt for details.
+- reset-names: Must include the following entries:
+  - pcie-rst0 :port0 reset
+  - pcie-rst1 :port1 reset
+  - pcie-rst2 :port2 reset
+- phys: list of PHY specifiers (used by generic PHY framework).
+- phy-names : must be "pcie-phy0", "pcie-phy1", "pcie-phyN".. based on the
+  number of PHYs as specified in *phys* property.
+- power-domains: A phandle and power domain specifier pair to the power domain
+  which is responsible for collapsing and restoring power to the peripheral.
+- bus-range: Range of bus numbers associated with this controller.
+- ranges:
+  - The first three entries are expected to translate the addresses for the 
root
+port registers, which are referenced by the assigned-addresses property of
+the root port nodes (see below).
+  - The remaining entries setup the mapping for the standard I/O and memory
+   regions.
+  Please refer to the standard PCI bus binding document for a more detailed
+  explanation.
+
+In addition, the device tree node must have sub-nodes describing each
+PCIe port interface, having the following mandatory properties:
+
+Required properties:
+- device_type: Must be "pci"
+- assigned-addresses: Address and size of the port configuration registers
+- reg: Only the first four bytes are used to refer to the correct bus number
+  and device number.
+- #address-cells: Must be 3
+- #size-cells: Must be 2
+- #interrupt-cells: Must be 1
+- interrupt-map-mask and interrupt-map: Standard PCI IRQ mapping properties
+  Please refer to the standard PCI bus binding document for a more detailed
+  explanation.
+- ranges: Sub-ranges distributed from the PCIe controller node. An empty
+  property is sufficient.
+- num-lanes: Number of lanes to use for this port.
+
+Examples:
+
+   hifsys: syscon@1a00 {
+   compatible = "mediatek,mt7623-hifsys", "syscon";


If you want to put the clock controller subsystem node to the example, 
please use a valid compatible, for example:


compatible = "mediatek,mt7623-hifsys",
 "mediatek,mt2701-hifsys",
 "syscon";

Thanks,
Matthias


+   reg = <0 0x1a00 0 0x1000>;
+   #clock-cells = <1>;
+   #reset-cells = <1>;
+   };
+
+   pcie: pcie-controller@1a14 {
+   compatible = "mediatek,mt7623-pcie";
+   device_type = "pci";
+   reg = <0 0x1a14 0 0x1000>; /* PCIe shared registers */
+   #address-cells = <3>;
+   #size-cells = <2>;
+   #interrupt-cells = <1>;
+   interrupts = ,
+,
+;
+   interrupt-names = "pcie-int0", "pcie-int1", "pcie-int2";
+   interrupt-map-mask = <0xf800 0 0 0>;
+   interrupt-map = <0x 0 0 0 &gic GIC_SPI 193 IRQ_TYPE_NONE>,
+   <0x0800 0 0 0 &gic GIC_SP

Re: [PATCH] powerpc/modules: If mprofile-kernel is enabled add it to vermagic

2017-05-10 Thread Balbir Singh
On Wed, May 10, 2017 at 4:57 PM, Michael Ellerman  wrote:
> On powerpc we can build the kernel with two different ABIs for mcount(), which
> is used by ftrace. Kernels built with one ABI do not know how to load modules
> built with the other ABI. The new style ABI is called "mprofile-kernel", for
> want of a better name.
>
> Currently if we build a module using the old style ABI, and the kernel with
> mprofile-kernel, when we load the module we'll oops something like:
>
>   # insmod autofs4-no-mprofile-kernel.ko
>   ftrace-powerpc: Unexpected instruction f8810028 around bl _mcount
>   [ cut here ]
>   WARNING: CPU: 6 PID: 3759 at ../kernel/trace/ftrace.c:2024 
> ftrace_bug+0x2b8/0x3c0
>   CPU: 6 PID: 3759 Comm: insmod Not tainted 
> 4.11.0-rc3-gcc-5.4.1-00017-g5a61ef74f269 #11
>   ...
>   NIP [c01eaa48] ftrace_bug+0x2b8/0x3c0
>   LR [c01eaff8] ftrace_process_locs+0x4a8/0x590
>   Call Trace:
> alloc_pages_current+0xc4/0x1d0 (unreliable)
> ftrace_process_locs+0x4a8/0x590
> load_module+0x1c8c/0x28f0
> SyS_finit_module+0x110/0x140
> system_call+0x38/0xfc
>   ...
>   ftrace failed to modify
>   [] 0xd2a31024
>actual:   35:65:00:48
>
> We can avoid this by including in the vermagic whether the kernel/module was
> built with mprofile-kernel. Which results in:
>
>   # insmod autofs4-pg.ko
>   autofs4: version magic
>   '4.11.0-rc3-gcc-5.4.1-00017-g5a61ef74f269 SMP mod_unload modversions '
>   should be
>   '4.11.0-rc3-gcc-5.4.1-00017-g5a61ef74f269-dirty SMP mod_unload modversions 
> mprofile-kernel'
>   insmod: ERROR: could not insert module autofs4-pg.ko: Invalid module format
>
> Signed-off-by: Michael Ellerman 
> ---
>  arch/powerpc/include/asm/module.h | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/module.h 
> b/arch/powerpc/include/asm/module.h
> index 53885512b8d3..6c0132c7212f 100644
> --- a/arch/powerpc/include/asm/module.h
> +++ b/arch/powerpc/include/asm/module.h
> @@ -14,6 +14,10 @@
>  #include 
>
>
> +#ifdef CC_USING_MPROFILE_KERNEL
> +#define MODULE_ARCH_VERMAGIC   "mprofile-kernel"
> +#endif
> +
>  #ifndef __powerpc64__
>  /*
>   * Thanks to Paul M for explaining this.
> --

Makes sense

Acked-by: Balbir Singh 


Re: Updating kernel.org cross compilers?

2017-05-10 Thread Arnd Bergmann
On Wed, May 10, 2017 at 12:18 AM, Segher Boessenkool
 wrote:
> On Tue, May 09, 2017 at 03:59:27PM +0100, Andre Przywara wrote:
>> >> I was able to build (bare-metal) toolchains for
>> >> all architectures except arc, m68k, tilegx and tilepro.
>> >
>> > arc needs a more recent GCC; the other probably as well.  GCC 7 should
>> > be out very soon, you probably want to wait for that :-)
>>
>> Well, GCC 7 indeed builds better, but then again is a very new compiler.
>> For instance in the moment it spits a lot of warnings when compiling the
>> kernel (mostly due to some *printf analysis). It's not hard to fix, but
>> this will take a while to trickle in and it's questionable whether this
>> will be backported everywhere.
>> So in addition to GCC 7.1 I'd like to have at least GCC 6.3 around,
>> which builds kernels without warnings today.
>
> If you don't want warnings, turn off the warnings or just don't look at
> them...  or fix the problems?  Many of the new warnings point out actual
> problems.
>
> Many of those sprintf problems in the kernel have already been fixed.

I've been using gcc-7.0 for a long time and fixed a lot of bugs it found,
along with more harmless warnings, but I had disabled a couple of
warning options when I first installed gcc-7 and ended up ignoring
those.

The exact set of additional options I used is:

-Wimplicit-fallthrough=0 -Wno-duplicate-decl-specifier
-Wno-int-in-bool-context -Wno-bool-operation -Wno-format-truncation
-Wno-format-overflow

there were a couple of others that I sent kernel fixes for instead.
I should probably revisit that list and for each of them either
only enable it with "make W=1" or fix all known warnings.
In the long run, I'd actually hope to fix all W=1 warnings too
and enable them by default.

  Arnd


Re: [PATCH v2 2/8] drm: Add drm_crtc_mode_valid()

2017-05-10 Thread Daniel Vetter
On Tue, May 09, 2017 at 06:00:09PM +0100, Jose Abreu wrote:
> Add a new helper to call crtc->mode_valid callback.
> 
> Suggested-by: Ville Syrjälä 
> Signed-off-by: Jose Abreu 
> Cc: Carlos Palminha 
> Cc: Alexey Brodkin 
> Cc: Ville Syrjälä 
> Cc: Daniel Vetter 
> Cc: Dave Airlie 
> Cc: Andrzej Hajda 
> Cc: Archit Taneja 
> ---
>  drivers/gpu/drm/drm_crtc.c  | 22 ++
>  drivers/gpu/drm/drm_crtc_internal.h |  3 +++
>  2 files changed, 25 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 5af25ce..07ae705 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -38,6 +38,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -741,3 +742,24 @@ int drm_mode_crtc_set_obj_prop(struct drm_mode_object 
> *obj,
>  
>   return ret;
>  }
> +
> +/**
> + * drm_crtc_mode_valid - call crtc->mode_valid callback, if any.
> + * @crtc: crtc
> + * @mode: mode to be validated
> + *
> + * If no mode_valid callback is available this will return MODE_OK.
> + *
> + * Returns: drm_mode_status Enum
> + */
> +enum drm_mode_status drm_crtc_mode_valid(struct drm_crtc *crtc,
> +  const struct drm_display_mode *mode)
> +{
> + const struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private;

This is clearly a helper func, but you place it into the core and
EXPORT_SYMBOL it. Imo this should be entirely internal to the helpers,
perhaps just stuff them all into drm_probe_helpers.c? Header file would be
drm_crtc_helper_internal.h.

That also means no need for kernel-doc (only the driver api is formally
documented) and then these 3 patches are so tiny it's better to squash
them into the patch that adds their users.

Thanks, Daniel
> +
> + if (!crtc_funcs || !crtc_funcs->mode_valid)
> + return MODE_OK;
> +
> + return crtc_funcs->mode_valid(crtc, mode);
> +}
> +EXPORT_SYMBOL(drm_crtc_mode_valid);
> diff --git a/drivers/gpu/drm/drm_crtc_internal.h 
> b/drivers/gpu/drm/drm_crtc_internal.h
> index d077c54..3800abd 100644
> --- a/drivers/gpu/drm/drm_crtc_internal.h
> +++ b/drivers/gpu/drm/drm_crtc_internal.h
> @@ -45,6 +45,9 @@ int drm_crtc_check_viewport(const struct drm_crtc *crtc,
>  
>  struct dma_fence *drm_crtc_create_fence(struct drm_crtc *crtc);
>  
> +enum drm_mode_status drm_crtc_mode_valid(struct drm_crtc *crtc,
> +  const struct drm_display_mode *mode);
> +
>  /* IOCTLs */
>  int drm_mode_getcrtc(struct drm_device *dev,
>void *data, struct drm_file *file_priv);
> -- 
> 1.9.1
> 
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


Re: [PATCH] vhost/vsock: use static minor number

2017-05-10 Thread Arnd Bergmann
On Tue, May 9, 2017 at 9:28 PM, Stefan Hajnoczi  wrote:
>
> Note that the "reserved for local use" range in
> Documentation/admin-guide/devices.txt is incorrect.  The userio driver
> already occupies part of that range.  I've updated the documentation
> accordingly.
...
> diff --git a/Documentation/admin-guide/devices.txt 
> b/Documentation/admin-guide/devices.txt
> index c9cea2e..5fe3480 100644
> --- a/Documentation/admin-guide/devices.txt
> +++ b/Documentation/admin-guide/devices.txt
> @@ -369,8 +369,9 @@
> 237 = /dev/loop-control Loopback control device
> 238 = /dev/vhost-netHost kernel accelerator for virtio net
> 239 = /dev/uhid User-space I/O driver support for HID 
> subsystem
> +   241 = /dev/vhost-vsock  Host kernel driver for virtio vsock

It seems like you intended to add the 240 documentation here as well but
then sent the patch without it.

   Arnd


Re: [PATCH v2 5/8] drm: Use new mode_valid() helpers in connector probe helper

2017-05-10 Thread Daniel Vetter
On Tue, May 09, 2017 at 06:00:12PM +0100, Jose Abreu wrote:
> This changes the connector probe helper function to use the new
> encoder->mode_valid() and crtc->mode_valid() helper callbacks to
> validate the modes.
> 
> The new callbacks are optional so the behaviour remains the same
> if they are not implemented. If they are, then the code loops
> through all the connector's encodersXcrtcs and calls the
> callback.
> 
> If at least a valid encoderXcrtc combination is found which
> accepts the mode then the function returns MODE_OK.
> 
> Signed-off-by: Jose Abreu 
> Cc: Carlos Palminha 
> Cc: Alexey Brodkin 
> Cc: Ville Syrjälä 
> Cc: Daniel Vetter 
> Cc: Dave Airlie 
> Cc: Andrzej Hajda 
> Cc: Archit Taneja 
> ---
> 
> Changes v1->v2:
>   - Use new helpers suggested by Ville
>   - Change documentation (Daniel)
> 
>  drivers/gpu/drm/drm_probe_helper.c | 60 
> --
>  1 file changed, 57 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_probe_helper.c 
> b/drivers/gpu/drm/drm_probe_helper.c
> index 1b0c14a..de47413 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -39,6 +39,8 @@
>  #include 
>  #include 
>  
> +#include "drm_crtc_internal.h"
> +
>  /**
>   * DOC: output probing helper overview
>   *
> @@ -80,6 +82,54 @@
>   return MODE_OK;
>  }
>  
> +static enum drm_mode_status
> +drm_mode_validate_connector(struct drm_connector *connector,
> + struct drm_display_mode *mode)
> +{
> + struct drm_device *dev = connector->dev;
> + uint32_t *ids = connector->encoder_ids;
> + enum drm_mode_status ret = MODE_OK;
> + unsigned int i;
> +
> + /* Step 1: Validate against connector */
> + ret = drm_connector_mode_valid(connector, mode);
> + if (ret != MODE_OK)
> + return ret;
> +
> + /* Step 2: Validate against encoders and crtcs */
> + for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
> + struct drm_encoder *encoder = drm_encoder_find(dev, ids[i]);
> + struct drm_crtc *crtc;
> +
> + if (!encoder)
> + continue;
> +
> + ret = drm_encoder_mode_valid(encoder, mode);
> + if (ret != MODE_OK) {
> + /* No point in continuing for crtc check as this encoder
> +  * will not accept the mode anyway. If all encoders
> +  * reject the mode then, at exit, ret will not be
> +  * MODE_OK. */
> + continue;
> + }

One thing I've forgotten the last time around: Please also check
bridge->mode_valid here. The encoder->bridge mapping is fixed.

Otherwise I think this looks good.
-Daniel

> +
> + drm_for_each_crtc(crtc, dev) {
> + if (!drm_encoder_crtc_ok(encoder, crtc))
> + continue;
> +
> + ret = drm_crtc_mode_valid(crtc, mode);
> + if (ret == MODE_OK) {
> + /* If we get to this point there is at least
> +  * one combination of encoder+crtc that works
> +  * for this mode. Lets return now. */
> + return ret;
> + }
> + }
> + }
> +
> + return ret;
> +}
> +
>  static int drm_helper_probe_add_cmdline_mode(struct drm_connector *connector)
>  {
>   struct drm_cmdline_mode *cmdline_mode;
> @@ -284,7 +334,11 @@ void drm_kms_helper_poll_enable(struct drm_device *dev)
>   *- drm_mode_validate_flag() checks the modes against basic connector
>   *  capabilities (interlace_allowed,doublescan_allowed,stereo_allowed)
>   *- the optional &drm_connector_helper_funcs.mode_valid helper can 
> perform
> - *  driver and/or hardware specific checks
> + *  driver and/or sink specific checks
> + *- the optional &drm_crtc_helper_funcs.mode_valid and
> + *  &drm_encoder_helper_funcs.mode_valid helpers can perform driver 
> and/or
> + *  source specific checks which are also enforced by the modeset/atomic
> + *  helpers
>   *
>   * 5. Any mode whose status is not OK is pruned from the connector's modes 
> list,
>   *accompanied by a debug message indicating the reason for the mode's
> @@ -428,8 +482,8 @@ int drm_helper_probe_single_connector_modes(struct 
> drm_connector *connector,
>   if (mode->status == MODE_OK)
>   mode->status = drm_mode_validate_flag(mode, mode_flags);
>  
> - if (mode->status == MODE_OK && connector_funcs->mode_valid)
> - mode->status = connector_funcs->mode_valid(connector,
> + if (mode->status == MODE_OK)
> + mode->status = drm_mode_validate_connector(connector,
>  mode);
>   }
>  
> -- 
> 1.9.1
> 
> 

-- 
Daniel Vetter

Re: [Xen-devel] [PATCH] xen: adjust early dom0 p2m handling to xen hypervisor behavior

2017-05-10 Thread Jan Beulich
>>> On 10.05.17 at 06:08,  wrote:
> When booted as pv-guest the p2m list presented by the Xen is already
> mapped to virtual addresses. In dom0 case the hypervisor might make use
> of 2M- or 1G-pages for this mapping. Unfortunately while being properly
> aligned in virtual and machine address space, those pages might not be
> aligned properly in guest physical address space.
> 
> So when trying to obtain the guest physical address of such a page
> pud_pfn() and pmd_pfn() must be avoided as those will mask away guest
> physical address bits not being zero in this special case.
> 
> Signed-off-by: Juergen Gross 

Reviewed-by: Jan Beulich 

Perhaps worth Cc-ing stable@ ?

Jan



Re: [PATCH v2] scsi: sg: don't return bogus Sg_requests

2017-05-10 Thread Hannes Reinecke
On 05/10/2017 09:53 AM, Johannes Thumshirn wrote:
> If the list search in sg_get_rq_mark() fails to find a valid request, we
> return a bogus element. This then can later lead to a GPF in sg_remove_scat().
> 
> So don't return bogus Sg_requests in sg_get_rq_mark() but NULL in case the
> list search doesn't find a valid request.
> 
> Signed-off-by: Johannes Thumshirn 
> Reported-by: Andrey Konovalov 
> Cc: Hannes Reinecke 
> Cc: Christoph Hellwig 
> Cc: Doug Gilbert 
> ---
> Changes to v1:
> * Directly return found element within the loop (Hannes)
> 
>  drivers/scsi/sg.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
> index 0a38ba01b7b4..82c33a6edbea 100644
> --- a/drivers/scsi/sg.c
> +++ b/drivers/scsi/sg.c
> @@ -2074,11 +2074,12 @@ sg_get_rq_mark(Sg_fd * sfp, int pack_id)
>   if ((1 == resp->done) && (!resp->sg_io_owned) &&
>   ((-1 == pack_id) || (resp->header.pack_id == pack_id))) {
>   resp->done = 2; /* guard against other readers */
> - break;
> + write_unlock_irqrestore(&sfp->rq_list_lock, iflags);
> + return resp;
>   }
>   }
>   write_unlock_irqrestore(&sfp->rq_list_lock, iflags);
> - return resp;
> + return NULL;
>  }
>  
>  /* always adds to end of list */
> 
Better.

Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH v2 1/8] drm: Add crtc/encoder/bridge->mode_valid() callbacks

2017-05-10 Thread Daniel Vetter
On Tue, May 09, 2017 at 06:00:08PM +0100, Jose Abreu wrote:
> This adds a new callback to crtc, encoder and bridge helper functions
> called mode_valid(). This callback shall be implemented if the
> corresponding component has some sort of restriction in the modes
> that can be displayed. A NULL callback implicates that the component
> can display all the modes.
> 
> We also change the description of connector->mode_valid() callback
> so that it matches the existing behaviour: It is never called in
> atomic check phase.
> 
> Only the callbacks were implemented to simplify review process,
> following patches will make use of them.
> 
> Signed-off-by: Jose Abreu 
> Cc: Carlos Palminha 
> Cc: Alexey Brodkin 
> Cc: Ville Syrjälä 
> Cc: Daniel Vetter 
> Cc: Dave Airlie 
> Cc: Andrzej Hajda 
> Cc: Archit Taneja 
> ---
> 
> Changes v1->v2:
>   - Change description of connector->mode_valid() (Daniel)
> 
>  include/drm/drm_bridge.h | 20 ++
>  include/drm/drm_modeset_helper_vtables.h | 45 
> 
>  2 files changed, 65 insertions(+)
> 
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index fdd82fc..00c6c36 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -59,6 +59,26 @@ struct drm_bridge_funcs {
>   void (*detach)(struct drm_bridge *bridge);
>  
>   /**
> +  * @mode_valid:
> +  *
> +  * This callback is used to check if a specific mode is valid in this
> +  * bridge. This should be implemented if the bridge has some sort of
> +  * restriction in the modes it can display. For example, a given bridge
> +  * may be responsible to set a clock value. If the clock can not
> +  * produce all the values for the available modes then this callback
> +  * can be used to restrict the number of modes to only the ones that
> +  * can be displayed.
> +  *
> +  * This is called at mode probe and at atomic check phase.
> +  *
> +  * RETURNS:
> +  *
> +  * drm_mode_status Enum
> +  */
> + enum drm_mode_status (*mode_valid)(struct drm_bridge *crtc,
> +const struct drm_display_mode *mode);
> +
> + /**
>* @mode_fixup:
>*
>* This callback is used to validate and adjust a mode. The paramater
> diff --git a/include/drm/drm_modeset_helper_vtables.h 
> b/include/drm/drm_modeset_helper_vtables.h
> index c01c328..eec2c70 100644
> --- a/include/drm/drm_modeset_helper_vtables.h
> +++ b/include/drm/drm_modeset_helper_vtables.h
> @@ -106,6 +106,26 @@ struct drm_crtc_helper_funcs {
>   void (*commit)(struct drm_crtc *crtc);
>  
>   /**
> +  * @mode_valid:
> +  *
> +  * This callback is used to check if a specific mode is valid in this
> +  * crtc. This should be implemented if the crtc has some sort of
> +  * restriction in the modes it can display. For example, a given crtc
> +  * may be responsible to set a clock value. If the clock can not
> +  * produce all the values for the available modes then this callback
> +  * can be used to restrict the number of modes to only the ones that
> +  * can be displayed.
> +  *
> +  * This is called at mode probe and at atomic check phase.
> +  *
> +  * RETURNS:
> +  *
> +  * drm_mode_status Enum
> +  */
> + enum drm_mode_status (*mode_valid)(struct drm_crtc *crtc,
> +const struct drm_display_mode *mode);
> +
> + /**
>* @mode_fixup:
>*
>* This callback is used to validate a mode. The parameter mode is the
> @@ -457,6 +477,26 @@ struct drm_encoder_helper_funcs {
>   void (*dpms)(struct drm_encoder *encoder, int mode);
>  
>   /**
> +  * @mode_valid:
> +  *
> +  * This callback is used to check if a specific mode is valid in this
> +  * encoder. This should be implemented if the encoder has some sort
> +  * of restriction in the modes it can display. For example, a given
> +  * encoder may be responsible to set a clock value. If the clock can
> +  * not produce all the values for the available modes then this callback
> +  * can be used to restrict the number of modes to only the ones that
> +  * can be displayed.
> +  *
> +  * This is called at mode probe and at atomic check phase.
> +  *
> +  * RETURNS:
> +  *
> +  * drm_mode_status Enum
> +  */
> + enum drm_mode_status (*mode_valid)(struct drm_encoder *crtc,
> +const struct drm_display_mode *mode);
> +
> + /**
>* @mode_fixup:
>*
>* This callback is used to validate and adjust a mode. The parameter
> @@ -795,6 +835,11 @@ struct drm_connector_helper_funcs {
>* (which is usually derived from the EDID data block from the sink).
>* See e.g. drm_helper_probe_single_connector_modes().
>*
> +  * This callbac

Re: [PATCH 3/9] VFS: Introduce a mount context

2017-05-10 Thread David Howells
Miklos Szeredi  wrote:

> And the static string thing that David implemented is also a very good
> idea, IMO.

There is an issue with it: it's fine as long as you keep a ref on the module
that generated it or clear all strings as part of module removal (which the
mount context in this patchset does).  With the NFS mount context I did, I
have to keep a ref on the NFS protocol module as well as the NFS filesystem
module.

I'm tempted to make it conditionally copy the string using kvasprintf_const()
- which would also permit format substitution.

David


Re: RFC v2: post-init-read-only protection for data allocated dynamically

2017-05-10 Thread Michal Hocko
On Fri 05-05-17 13:42:27, Igor Stoppa wrote:
> On 04/05/17 19:49, Laura Abbott wrote:
> > [adding kernel-hardening since I think there would be interest]
> 
> thank you, I overlooked this
> 
> 
> > BPF takes the approach of calling set_memory_ro to mark regions as
> > read only. I'm certainly over simplifying but it sounds like this
> > is mostly a mechanism to have this happen mostly automatically.
> > Can you provide any more details about tradeoffs of the two approaches?
> 
> I am not sure I understand the question ...
> For what I can understand, the bpf is marking as read only something
> that spans across various pages, which is fine.
> The payload to be protected is already organized in such pages.
> 
> But in the case I have in mind, I have various, heterogeneous chunks of
> data, coming from various subsystems, not necessarily page aligned.
> And, even if they were page aligned, most likely they would be far
> smaller than a page, even a 4k page.

This aspect of various sizes makes the SLAB allocator not optimal
because it operates on caches (pools of pages) which manage objects of
the same size. You could use the maximum size of all objects and waste
some memory but you would have to know this max in advance which would
make this approach less practical. You could create more caches of
course but that still requires to know those sizes in advance.

So it smells like a dedicated allocator which operates on a pool of
pages might be a better option in the end. This depends on what you
expect from the allocator. NUMA awareness? Very effective hotpath? Very
good fragmentation avoidance? CPU cache awareness? Special alignment
requirements? Reasonable free()? Etc...

To me it seems that this being an initialization mostly thingy a simple
allocator which manages a pool of pages (one set of sealed and one for
allocations) and which only appends new objects as they fit to unsealed
pages would be sufficient for starter.
-- 
Michal Hocko
SUSE Labs


Re: [PATCH v3 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe

2017-05-10 Thread Arnd Bergmann
On Wed, May 10, 2017 at 4:07 AM, Ryder Lee  wrote:

> +- ranges:
> +  - The first three entries are expected to translate the addresses for the 
> root
> +port registers, which are referenced by the assigned-addresses property 
> of
> +the root port nodes (see below).

I don't understand this part. Why do you need a static translation for these?
Shouldn't they just be listed in the 'reg' property of the parent node now that
you have the clk/reset/phy properties in the parent as well?

> +Required properties:
> +- device_type: Must be "pci"
> +- assigned-addresses: Address and size of the port configuration registers
> +- reg: Only the first four bytes are used to refer to the correct bus number
> +  and device number.
> +- #address-cells: Must be 3
> +- #size-cells: Must be 2
> +- #interrupt-cells: Must be 1
> +- interrupt-map-mask and interrupt-map: Standard PCI IRQ mapping properties
> +  Please refer to the standard PCI bus binding document for a more detailed
> +  explanation.

Child nodes do not normally have interrupt-map properties. Isn't this
already covered by the interrupt-map in the parent?

  Arnd


Re: [Xen-devel] [PATCH] xen: adjust early dom0 p2m handling to xen hypervisor behavior

2017-05-10 Thread Juergen Gross
On 10/05/17 10:02, Jan Beulich wrote:
 On 10.05.17 at 06:08,  wrote:
>> When booted as pv-guest the p2m list presented by the Xen is already
>> mapped to virtual addresses. In dom0 case the hypervisor might make use
>> of 2M- or 1G-pages for this mapping. Unfortunately while being properly
>> aligned in virtual and machine address space, those pages might not be
>> aligned properly in guest physical address space.
>>
>> So when trying to obtain the guest physical address of such a page
>> pud_pfn() and pmd_pfn() must be avoided as those will mask away guest
>> physical address bits not being zero in this special case.
>>
>> Signed-off-by: Juergen Gross 
> 
> Reviewed-by: Jan Beulich 
> 
> Perhaps worth Cc-ing stable@ ?

Any backport needs to be done manually, as mmu_pv.c has been introduced
in 4.12 only.

As soon as the patch is upstream I'm planning to do that (trivial)
backport and send it to stable.


Juergen


Re: [kernel-hardening] Re: [PATCH v9 1/4] syscalls: Verify address limit before returning to user-mode

2017-05-10 Thread Al Viro
On Wed, May 10, 2017 at 09:37:04AM +0200, Arnd Bergmann wrote:

> > How about trying to remove all of them?  If we could actually get rid
> > of all of them, we could drop the arch support, and we'd get faster,
> > simpler, shorter uaccess code throughout the kernel.

BTW, not all get_user() under KERNEL_DS are plain loads.  There is an
exception - probe_kernel_read().

> > The ones in kernel/compat.c are generally garbage.  They should be
> > using compat_alloc_user_space().  Ditto for kernel/power/user.c.
> 
> compat_alloc_user_space() has some problems too, it adds
> complexity to a rarely-tested code path and can add some noticeable
> overhead in cases where user space access is slow because of
> extra checks.
> 
> It's clearly better than set_fs(), but the way I prefer to convert the
> code is to avoid both and instead move compat handlers next to
> the native code, and splitting out the common code between native
> and compat mode into a helper that takes a regular kernel pointer.
> 
> I think that's what both Al has done in the past on compat_ioctl()
> and select() and what Christoph does in his latest series, but
> it seems worth pointing out for others that decide to help out here.

Folks, reducing the amount of places where we play with set_fs() is certainly
a good thing.  Getting rid of them completely is something entirely different;
I have tried to plot out patch series in this direction many times during the
last 5 years or so, but it's not going to be easy.  Tomorrow I can start
posting my notes in that direction (and there are tons of those, unfortunately
mixed with git grep results, highly unprintable personal comments, etc.);
just let me grab some sleep first...

BTW, slow userland access is not just due to extra checks; access_ok(),
in particular, is pretty much noise.  The real PITA comes from the things
like STAC/CLAC on recent x86.  Or hardware overhead of cross-address-space
block copy insn (e.g. on s390, where it's optimized for multi-cacheline
blocks).  Or things like uml, where it's a matter of walking the page
tables for each sodding __get_user().  It's not always just a matter of
address space limit...


Re: [PATCH v6 10/23] drivers/fsi: Add device read/write/peek API

2017-05-10 Thread Joel Stanley
On Tue, Apr 11, 2017 at 5:16 AM, Christopher Bostic
 wrote:
> From: Jeremy Kerr 
>
> This change introduces the fsi device API: simple read, write and peek
> accessors for the devices' address spaces.
>
> Includes contributions from Chris Bostic 
> and Edward A. James .
>
> Signed-off-by: Edward A. James 
> Signed-off-by: Jeremy Kerr 
> Signed-off-by: Chris Bostic 
> Signed-off-by: Joel Stanley 
> ---
>  drivers/fsi/fsi-core.c | 33 +
>  include/linux/fsi.h|  7 ++-
>  2 files changed, 39 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
> index a8faa89..4da0b030 100644
> --- a/drivers/fsi/fsi-core.c
> +++ b/drivers/fsi/fsi-core.c
> @@ -32,6 +32,8 @@
>  #define FSI_SLAVE_CONF_CRC_MASK0x000f
>  #define FSI_SLAVE_CONF_DATA_BITS   28
>
> +#define FSI_PEEK_BASE  0x410
> +
>  static const int engine_page_size = 0x400;
>
>  #define FSI_SLAVE_BASE 0x800
> @@ -73,9 +75,40 @@ static int fsi_master_read(struct fsi_master *master, int 
> link,
> uint8_t slave_id, uint32_t addr, void *val, size_t size);
>  static int fsi_master_write(struct fsi_master *master, int link,
> uint8_t slave_id, uint32_t addr, const void *val, size_t 
> size);
> +static int fsi_slave_read(struct fsi_slave *slave, uint32_t addr,
> +   void *val, size_t size);
> +static int fsi_slave_write(struct fsi_slave *slave, uint32_t addr,
> +   const void *val, size_t size);

I don't think these
>
>  /* FSI endpoint-device support */
>
> +int fsi_device_read(struct fsi_device *dev, uint32_t addr, void *val,
> +   size_t size)

I suggest adding some comments about the use of fsi_device_read,
fsi_device_write and fsi_device_peek APIs for driver authors.

One important note is that the data in val must be FSI bus endian (big endian).

It would be nice to have the sparse notation (eg. __be32) as well.
That doesn't make sense for void *, but we could add some helper
functions like:

 int fsi_device_read32(struct fsi_device *dev, uint32_t addr, __be32 *val)

 int fsi_device_write32(struct fsi_device *dev, uint32_t addr, __be32 val)

We probably only need a 32 bit version, as all of the reads/writes
being done in users of this function are 32 bit.

What do you think?

> +{
> +   if (addr > dev->size || size > dev->size || addr > dev->size - size)
> +   return -EINVAL;
> +
> +   return fsi_slave_read(dev->slave, dev->addr + addr, val, size);
> +}
> +EXPORT_SYMBOL_GPL(fsi_device_read);
> +
> +int fsi_device_write(struct fsi_device *dev, uint32_t addr, const void *val,
> +   size_t size)
> +{
> +   if (addr > dev->size || size > dev->size || addr > dev->size - size)
> +   return -EINVAL;
> +
> +   return fsi_slave_write(dev->slave, dev->addr + addr, val, size);
> +}
> +EXPORT_SYMBOL_GPL(fsi_device_write);
> +
> +int fsi_device_peek(struct fsi_device *dev, void *val)
> +{
> +   uint32_t addr = FSI_PEEK_BASE + ((dev->unit - 2) * sizeof(uint32_t));
> +
> +   return fsi_slave_read(dev->slave, addr, val, sizeof(uint32_t));
> +}
> +
>  static void fsi_device_release(struct device *_device)
>  {
> struct fsi_device *device = to_fsi_dev(_device);
> diff --git a/include/linux/fsi.h b/include/linux/fsi.h
> index efa55ba..66bce48 100644
> --- a/include/linux/fsi.h
> +++ b/include/linux/fsi.h
> @@ -27,6 +27,12 @@ struct fsi_device {
> uint32_tsize;
>  };
>
> +extern int fsi_device_read(struct fsi_device *dev, uint32_t addr,
> +   void *val, size_t size);
> +extern int fsi_device_write(struct fsi_device *dev, uint32_t addr,
> +   const void *val, size_t size);
> +extern int fsi_device_peek(struct fsi_device *dev, void *val);
> +
>  struct fsi_device_id {
> u8  engine_type;
> u8  version;
> @@ -40,7 +46,6 @@ struct fsi_device_id {
>  #define FSI_DEVICE_VERSIONED(t, v) \
> .engine_type = (t), .version = (v),
>
> -
>  struct fsi_driver {
> struct device_driverdrv;
> const struct fsi_device_id  *id_table;
> --
> 1.8.2.2
>


Re: [PATCH 7/7] DWARF: add the config option

2017-05-10 Thread Jiri Slaby
On 05/09/2017, 12:00 PM, h...@zytor.com wrote:
> As far as I understand, the .eh_frame section is supposed to contain the 
> subset of the DWARF bytecode needed to do a stack unwind when an exception is 
> thrown, whereas the .debug* sections contain the full DWARF data a debugger 
> might want.  Thus .eh_frame is mapped into the runtime process while .debug* 
> [usually?] is not.  .debug* can easily be 10x larger than the executable text 
> segments.

As it currently stands, the (same) data is generated either to
.eh_frame, or to .debug_frame. Depending if DWARF_UNWINDER is turned on,
or off, respectively. vdso has the same data in both, always.

> Assembly language routines become problematic no matter what you do unless 
> you restrict the way the assembly can be written.  Experience has shown us 
> that hand-maintaining annotations in assembly code is doomed to failure, and 
> in many cases it isn't even clear to even experienced programmers how to do 
> it.

Sure, manual annotations of assembly will be avoided as much as
possible. We have to rely objtool to generate them in most cases.

>  [H.J.: is the .cfi_* operations set even documented anywhere in a way that 
> non-compiler-writers can comprehend it?]

Until now, I always looked into as manual:
$ pinfo --node='CFI directives' as

> I'm, ahem, highly skeptical to creating our own unwinding data format unless 
> there is *documented, supported, and tested* way to force the compiler to 
> *automatically* fall back to frame pointers any time there may be complexity 
> involved,

I second this. Inventing a new format like this mostly ends up with
using the standard one after several iterations. One cannot think of all
the consequences and needs while proposing a new one.

The memory footprint is ~2M for average vmlinux. And people who need to
access:
* either need it frequently -- those do not need performance (LOCKDEP,
KASAN, or other debug builds)
* or are in the middle of WARNING, BUG, crash, panic or such and this is
not that often...

And we would need *both*. The limited proprietary one in some sort of
.kernel_eh_frame, and DWARF cfis in .debug_frame for tools like crash,
gdb and so on.

And yes, the DWARF unwinder falls back to FP if they are available (see
function dw_fp_fallback).

thanks,
-- 
js
suse labs


Re: [kernel-hardening] Re: [PATCH v9 1/4] syscalls: Verify address limit before returning to user-mode

2017-05-10 Thread Christoph Hellwig
On Wed, May 10, 2017 at 09:08:41AM +0100, Al Viro wrote:
> On Wed, May 10, 2017 at 09:37:04AM +0200, Arnd Bergmann wrote:
> 
> > > How about trying to remove all of them?  If we could actually get rid
> > > of all of them, we could drop the arch support, and we'd get faster,
> > > simpler, shorter uaccess code throughout the kernel.
> 
> BTW, not all get_user() under KERNEL_DS are plain loads.  There is an
> exception - probe_kernel_read().

And various calls that looks like opencoded versions, e.g. drivers/dio
or the ELF loader.

But in the long run we'll just need a separate primitive for that,
but that can wait until the set_fs calls outside the core code are
gone.


[PATCH] ASoC: Intel: sst: fix spelling mistake: "allocationf" -> "allocation"

2017-05-10 Thread Colin King
From: Colin Ian King 

Trivial fix to spelling mistake in dev_err message.

Signed-off-by: Colin Ian King 
---
 sound/soc/intel/atom/sst-mfld-platform-pcm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/intel/atom/sst-mfld-platform-pcm.c 
b/sound/soc/intel/atom/sst-mfld-platform-pcm.c
index 21cac1c8dd4c..e74119113713 100644
--- a/sound/soc/intel/atom/sst-mfld-platform-pcm.c
+++ b/sound/soc/intel/atom/sst-mfld-platform-pcm.c
@@ -690,7 +690,7 @@ static int sst_pcm_new(struct snd_soc_pcm_runtime *rtd)
snd_dma_continuous_data(GFP_DMA),
SST_MIN_BUFFER, SST_MAX_BUFFER);
if (retval) {
-   dev_err(rtd->dev, "dma buffer allocationf fail\n");
+   dev_err(rtd->dev, "dma buffer allocation fail\n");
return retval;
}
}
-- 
2.11.0



Re: [RFC 09/10] x86/mm: Rework lazy TLB to track the actual loaded mm

2017-05-10 Thread Thomas Gleixner
On Wed, 10 May 2017, Ingo Molnar wrote:
> 
> * Thomas Gleixner  wrote:
> 
> > On Sun, 7 May 2017, Andy Lutomirski wrote:
> > >  /* context.lock is held for us, so we don't need any locking. */
> > >  static void flush_ldt(void *current_mm)
> > >  {
> > > + struct mm_struct *mm = current_mm;
> > >   mm_context_t *pc;
> > >  
> > > - if (current->active_mm != current_mm)
> > > + if (this_cpu_read(cpu_tlbstate.loaded_mm) != current_mm)
> > 
> > While functional correct, this really should compare against 'mm'.
> > 
> > >   return;
> > >  
> > > - pc = ¤t->active_mm->context;
> > > + pc = &mm->context;
> 
> So this appears to be the function:
> 
>  static void flush_ldt(void *current_mm)
>  {
> struct mm_struct *mm = current_mm;
> mm_context_t *pc;
> 
> if (this_cpu_read(cpu_tlbstate.loaded_mm) != current_mm)
> return;
> 
> pc = &mm->context;
> set_ldt(pc->ldt->entries, pc->ldt->size);
>  }
> 
> why not rename 'current_mm' to 'mm' and remove the 'mm' local variable?

Because you cannot dereference a void pointer, i.e. &mm->context 

Thanks,

tglx


Re: [RFC 09/10] x86/mm: Rework lazy TLB to track the actual loaded mm

2017-05-10 Thread Ingo Molnar

* Thomas Gleixner  wrote:

> On Wed, 10 May 2017, Ingo Molnar wrote:
> > 
> > * Thomas Gleixner  wrote:
> > 
> > > On Sun, 7 May 2017, Andy Lutomirski wrote:
> > > >  /* context.lock is held for us, so we don't need any locking. */
> > > >  static void flush_ldt(void *current_mm)
> > > >  {
> > > > +   struct mm_struct *mm = current_mm;
> > > > mm_context_t *pc;
> > > >  
> > > > -   if (current->active_mm != current_mm)
> > > > +   if (this_cpu_read(cpu_tlbstate.loaded_mm) != current_mm)
> > > 
> > > While functional correct, this really should compare against 'mm'.
> > > 
> > > > return;
> > > >  
> > > > -   pc = ¤t->active_mm->context;
> > > > +   pc = &mm->context;
> > 
> > So this appears to be the function:
> > 
> >  static void flush_ldt(void *current_mm)
> >  {
> > struct mm_struct *mm = current_mm;
> > mm_context_t *pc;
> > 
> > if (this_cpu_read(cpu_tlbstate.loaded_mm) != current_mm)
> > return;
> > 
> > pc = &mm->context;
> > set_ldt(pc->ldt->entries, pc->ldt->size);
> >  }
> > 
> > why not rename 'current_mm' to 'mm' and remove the 'mm' local variable?
> 
> Because you cannot dereference a void pointer, i.e. &mm->context 

Indeed, doh! The naming totally confused me. The way I'd write it is the 
canonical 
form for such callbacks:

static void flush_ldt(void *data)
{
struct mm_struct *mm = data;

... which beyond unconfusing me would probably also have prevented any 
accidental 
use of the 'current_mm' callback argument.

Thanks,

Ingo


nouveau "eDP-1: EDID is invalid" regression after 4.11 with HP ZBook 15 G3

2017-05-10 Thread Tommi Rantala
Hi,

The HP ZBook 15 G3 laptop builtin display (eDP-1) does not work
correctly with v4.11-11413-g2868b25.

When booting the laptop, the resolution seems to be limited to
1024x768, and gnome-session segfaults.

Up to 4.11 the display works just fine in 1920x1080 mode.

I'm seeing this in the kernel logs:

nouveau :01:00.0: eDP-1: EDID is invalid:
[00] BAD  00 ff ff ff ff ff ff 00 ff ff ff ff ff ff ff ff
[00] BAD  ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[00] BAD  ff ff ff ff ff ff ff ff ff ff ff ff ff 84 53 54
[00] BAD  66 69 50 55 57 66 74 49 48 ff ff ff ff ff ff ff
[00] BAD  ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[00] BAD  ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[00] BAD  ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[00] BAD  ff ff ff ff ff ff ff ff ff ff ff 00 00 ff 00 ff
nouveau :01:00.0: DRM: DDC responded, but no EDID for eDP-1
[drm] Cannot find any crtc or sizes - going 1024x768


$ lspci | grep NVIDIA
01:00.0 VGA compatible controller: NVIDIA Corporation GM107GLM [Quadro
M2000M] (rev a2)

Any ideas, or should I bisect?

4.11 dmesg & xrandr output:
https://pastebin.com/raw/P9LGP7e1

4.11-11413-g2868b25 dmesg:
https://pastebin.com/raw/QBT9mMua

-Tommi


Re: [PATCH] irq_bcm2836: Send event when onlining sleeping cores

2017-05-10 Thread Phil Elwell
On 10/05/2017 08:42, Marc Zyngier wrote:
> On 09/05/17 20:02, Phil Elwell wrote:
>> On 09/05/2017 19:53, Marc Zyngier wrote:
>>> On 09/05/17 19:52, Phil Elwell wrote:
 On 09/05/2017 19:14, Marc Zyngier wrote:
> On 09/05/17 19:08, Eric Anholt wrote:
>> Marc Zyngier  writes:
>>
>>> On 09/05/17 17:59, Eric Anholt wrote:
 Phil Elwell  writes:

> In order to reduce power consumption and bus traffic, it is sensible
> for secondary cores to enter a low-power idle state when waiting to
> be started. The wfe instruction causes a core to wait until an event
> or interrupt arrives before continuing to the next instruction.
> The sev instruction sends a wakeup event to the other cores, so call
> it from bcm2836_smp_boot_secondary, the function that wakes up the
> waiting cores during booting.
>
> It is harmless to use this patch without the corresponding change
> adding wfe to the ARMv7/ARMv8-32 stubs, but if the stubs are updated
> and this patch is not applied then the other cores will sleep forever.
>
> See: https://github.com/raspberrypi/linux/issues/1989
>
> Signed-off-by: Phil Elwell 
> ---
>  drivers/irqchip/irq-bcm2836.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/irqchip/irq-bcm2836.c 
> b/drivers/irqchip/irq-bcm2836.c
> index e10597c..6dccdf9 100644
> --- a/drivers/irqchip/irq-bcm2836.c
> +++ b/drivers/irqchip/irq-bcm2836.c
> @@ -248,6 +248,9 @@ static int __init 
> bcm2836_smp_boot_secondary(unsigned int cpu,
>   writel(secondary_startup_phys,
>  intc.base + LOCAL_MAILBOX3_SET0 + 16 * cpu);
>
> + dsb(sy); /* Ensure write has completed before waking the other 
> CPUs */
> + sev();
> +
>   return 0;
>  }

 This is also the behavior that the standard arm64 spin-table method 
 has,
 which we unfortunately can't quite use.
>>>
>>> And why is that so? Why do you have to reinvent the wheel (and hide the
>>> cloned wheel in an interrupt controller driver)?
>>>
>>> That doesn't seem right to me.
>>
>> The armv8 stubs (firmware-supplied code in the low page that do the
>> spinning) do actually implement arm64's spin-table method.  It's the
>> armv7 stubs that use these registers in the irqchip instead of plain
>> addresses in system memory.
>
> Let's put ARMv7 aside for the time being. If your firmware already
> implements spin-tables, why don't you simply use that at least on arm64?

 We do.
>>>
>>> Obviously not the way it is intended if you have to duplicate the core
>>> architectural code in the interrupt controller driver, which couldn't
>>> care less.
>>
>> If we were using this method on arm64 then the other cores would not start up
>> because armstub8.S has always included a wfe. Nothing in the commit mentions
>> arm64 - this is an ARCH=arm fix.
> 
> Thanks for the clarification, which you could have added to the commit
> message.
> 
> The question still remains: why do we have CPU bring-up code in an
> interrupt controller, instead of having it in the architecture code?
> 
> The RPi-2 is the *only* platform to have its SMP bringup code outside of
> arch/arm, so the first course of action would be to move that code where
> it belongs.

You were CC'd on the commit (41f4988cc287e5f836d3f6620c9f900bc9b560e9) that
introduced bcm2836_smp_boot_secondary - it seems strange to start objecting
now. Yes, I think it is odd that it didn't go into arch/arm/mach-bcm, but in
the interests of making changes in small, independent steps, do you have a
problem with this commit?

Phil



[Patch v2] mm/vmscan: fix unsequenced modification and access warning

2017-05-10 Thread Nick Desaulniers
Clang flags this file with the -Wunsequenced error that GCC does not
have.

unsequenced modification and access to 'gfp_mask'

It seems that gfp_mask is both read and written without a sequence point
in between, which is undefined behavior.

Signed-off-by: Nick Desaulniers 
---
Changes in v2:
- don't assign back to gfp_mask, reuse sc.gfp_mask
- initialize reclaim_idx directly, without classzone_idx

 mm/vmscan.c | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 4e7ed65842af..d32c42d17935 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -2958,7 +2958,7 @@ unsigned long try_to_free_pages(struct zonelist 
*zonelist, int order,
unsigned long nr_reclaimed;
struct scan_control sc = {
.nr_to_reclaim = SWAP_CLUSTER_MAX,
-   .gfp_mask = (gfp_mask = current_gfp_context(gfp_mask)),
+   .gfp_mask = current_gfp_context(gfp_mask),
.reclaim_idx = gfp_zone(gfp_mask),
.order = order,
.nodemask = nodemask,
@@ -2973,12 +2973,12 @@ unsigned long try_to_free_pages(struct zonelist 
*zonelist, int order,
 * 1 is returned so that the page allocator does not OOM kill at this
 * point.
 */
-   if (throttle_direct_reclaim(gfp_mask, zonelist, nodemask))
+   if (throttle_direct_reclaim(sc.gfp_mask, zonelist, nodemask))
return 1;
 
trace_mm_vmscan_direct_reclaim_begin(order,
sc.may_writepage,
-   gfp_mask,
+   sc.gfp_mask,
sc.reclaim_idx);
 
nr_reclaimed = do_try_to_free_pages(zonelist, &sc);
@@ -3763,16 +3763,15 @@ static int __node_reclaim(struct pglist_data *pgdat, 
gfp_t gfp_mask, unsigned in
const unsigned long nr_pages = 1 << order;
struct task_struct *p = current;
struct reclaim_state reclaim_state;
-   int classzone_idx = gfp_zone(gfp_mask);
struct scan_control sc = {
.nr_to_reclaim = max(nr_pages, SWAP_CLUSTER_MAX),
-   .gfp_mask = (gfp_mask = current_gfp_context(gfp_mask)),
+   .gfp_mask = current_gfp_context(gfp_mask),
.order = order,
.priority = NODE_RECLAIM_PRIORITY,
.may_writepage = !!(node_reclaim_mode & RECLAIM_WRITE),
.may_unmap = !!(node_reclaim_mode & RECLAIM_UNMAP),
.may_swap = 1,
-   .reclaim_idx = classzone_idx,
+   .reclaim_idx = gfp_zone(gfp_mask),
};
 
cond_resched();
@@ -3782,7 +3781,7 @@ static int __node_reclaim(struct pglist_data *pgdat, 
gfp_t gfp_mask, unsigned in
 * and RECLAIM_UNMAP.
 */
p->flags |= PF_MEMALLOC | PF_SWAPWRITE;
-   lockdep_set_current_reclaim_state(gfp_mask);
+   lockdep_set_current_reclaim_state(sc.gfp_mask);
reclaim_state.reclaimed_slab = 0;
p->reclaim_state = &reclaim_state;
 
-- 
2.11.0



[PATCH][RESEND] regulator: lp8755: fix spelling mistake "acceess" -> "access"

2017-05-10 Thread Colin King
From: Colin Ian King 

Trivial fix to spelling mistake in dev_err messages.

Signed-off-by: Colin Ian King 
---
 drivers/regulator/lp8755.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/regulator/lp8755.c b/drivers/regulator/lp8755.c
index db34e1da75ef..244822bb63cd 100644
--- a/drivers/regulator/lp8755.c
+++ b/drivers/regulator/lp8755.c
@@ -99,7 +99,7 @@ static int lp8755_buck_enable_time(struct regulator_dev *rdev)
 
ret = lp8755_read(pchip, 0x12 + id, ®val);
if (ret < 0) {
-   dev_err(pchip->dev, "i2c acceess error %s\n", __func__);
+   dev_err(pchip->dev, "i2c access error %s\n", __func__);
return ret;
}
return (regval & 0xff) * 100;
@@ -144,7 +144,7 @@ static int lp8755_buck_set_mode(struct regulator_dev *rdev, 
unsigned int mode)
goto err_i2c;
return ret;
 err_i2c:
-   dev_err(pchip->dev, "i2c acceess error %s\n", __func__);
+   dev_err(pchip->dev, "i2c access error %s\n", __func__);
return ret;
 }
 
@@ -175,7 +175,7 @@ static unsigned int lp8755_buck_get_mode(struct 
regulator_dev *rdev)
return REGULATOR_MODE_NORMAL;
 
 err_i2c:
-   dev_err(pchip->dev, "i2c acceess error %s\n", __func__);
+   dev_err(pchip->dev, "i2c access error %s\n", __func__);
return 0;
 }
 
@@ -223,7 +223,7 @@ static int lp8755_buck_set_ramp(struct regulator_dev *rdev, 
int ramp)
goto err_i2c;
return ret;
 err_i2c:
-   dev_err(pchip->dev, "i2c acceess error %s\n", __func__);
+   dev_err(pchip->dev, "i2c access error %s\n", __func__);
return ret;
 }
 
@@ -295,7 +295,7 @@ static int lp8755_init_data(struct lp8755_chip *pchip)
return ret;
 
 out_i2c_error:
-   dev_err(pchip->dev, "i2c acceess error %s\n", __func__);
+   dev_err(pchip->dev, "i2c access error %s\n", __func__);
return ret;
 }
 
@@ -404,7 +404,7 @@ static irqreturn_t lp8755_irq_handler(int irq, void *data)
return IRQ_HANDLED;
 
 err_i2c:
-   dev_err(pchip->dev, "i2c acceess error %s\n", __func__);
+   dev_err(pchip->dev, "i2c access error %s\n", __func__);
return IRQ_NONE;
 }
 
@@ -420,7 +420,7 @@ static int lp8755_int_config(struct lp8755_chip *pchip)
 
ret = lp8755_read(pchip, 0x0F, ®val);
if (ret < 0) {
-   dev_err(pchip->dev, "i2c acceess error %s\n", __func__);
+   dev_err(pchip->dev, "i2c access error %s\n", __func__);
return ret;
}
 
-- 
2.11.0



Re: [PATCH 7/7] DWARF: add the config option

2017-05-10 Thread Jiri Slaby
On 05/09/2017, 09:22 PM, Josh Poimboeuf wrote:
> On Tue, May 09, 2017 at 08:47:50PM +0200, Jiri Kosina wrote:
>> On Sun, 7 May 2017, Josh Poimboeuf wrote:
>>
>>> DWARF is great for debuggers.  It helps you find all the registers on 
>>> the stack, so you can see function arguments and local variables.  All 
>>> expressed in a nice compact format.
>>>
>>> But that's overkill for unwinders.  We don't need all those registers,
>>> and the state machine is too complicated.  
>>
>> OTOH if we make the failures in processing of those "auxiliary" 
>> information non-fatal (in a sense that it neither causes kernel bug nor 
>> does it actually corrupt the unwinding process, but the only effect is 
>> losing "optional" information), having this data available doesn't hurt. 
> 
> But it does hurt, in the sense that the complicated format of DWARF CFI
> means the unwinder has to jump through a lot more hoops to read it.

Why that matters, actually? Unwinder is nothing to be performance
oriented. And if somebody is doing a lot of unwinding during runtime,
they can switch to in-this-case-faster FP unwinder.

> And if we wanted it to be reasonably reliable, we'd also need to fix up
> the DWARF data somehow before converting it, presumably with objtool.

We have to do this anyway. Be it the DWARF info or whatever we end up with.

>>> Unwinders basically only need to know one thing: given an instruction 
>>> address and a stack pointer, where is the caller's stack frame?
>>
>> Again, DWARF should be able to give us all of this (including the 
>> FP-fallback etc). It feels a bit silly to purposedly ignore it and 
>> reinvent parts of it again, instead of fixing (read: "asking toolchain 
>> guys to fix")

And we can just do, if a totally broken compiler emerges:
#if defined(CONFIG_DWARF_UNWINDER) && GCC_VERSION == 59000
#error Sorry, choose a different compiler or disable DWARF unwinder
#endif

We haven't to do this during the past decade and I am sceptic if we
would have to do it in the next one.

>>  the cases where we actually are not getting the proper data 
>> in DWARF. That's a win-win at the end of the day.
> 
> Most of the kernel DWARF issues I've seen aren't caused by toolchain
> bugs.  They're caused by the kernel's quirks: asm, inline asm, special
> sections.

Right.

> And anyway, fixing the correctness of the DWARF data is only half the
> problem IMO.  The other half of the problem is unwinder complexity.

Complex, but generic and working. IMO, it would be rather though to come
up with some tool working on different compilers or even different
versions of gcc. I mean some tool to convert the DWARF data to something
proprietary. The conversion would be as complex as is the unwinder plus
conversion to the proprietary format and its dump into ELF. We would
still rely on a (now out-of-kernel-runtime-code) complex monolith to do
it right.

thanks,
-- 
js
suse labs


Re: [PATCH v3 3/3] arm64: Silence first allocation with CONFIG_ARM64_MODULE_PLTS=y

2017-05-10 Thread Catalin Marinas
On Mon, May 08, 2017 at 11:07:24AM +0100, Will Deacon wrote:
> On Fri, May 05, 2017 at 02:07:28PM -0700, Florian Fainelli wrote:
> > On 05/03/2017 04:18 AM, Will Deacon wrote:
> > > On Thu, Apr 27, 2017 at 11:19:02AM -0700, Florian Fainelli wrote:
> > >> When CONFIG_ARM64_MODULE_PLTS is enabled, the first allocation using the
> > >> module space fails, because the module is too big, and then the module
> > >> allocation is attempted from vmalloc space. Silence the first allocation
> > >> failure in that case by setting __GFP_NOWARN.
> > >>
> > >> Reviewed-by: Ard Biesheuvel 
> > >> Signed-off-by: Florian Fainelli 
> > >> ---
> > >>  arch/arm64/kernel/module.c | 7 ++-
> > >>  1 file changed, 6 insertions(+), 1 deletion(-)
> > > 
> > > I'm not sure what the merge plan is for these, but the arm64 bit here
> > > looks fine to me:
> > > 
> > > Acked-by: Will Deacon 
> > 
> > Thanks, not sure either, would you or Catalin want to pick this series?
> 
> We'd need an Ack from Russell on the arch/arm/ part before we could take
> this series.

The first patch touches mm/vmalloc.c, so we could also merge the series
via akpm's tree. Andrew, do you have any preference?

-- 
Catalin


Re: [Patch v2] mm/vmscan: fix unsequenced modification and access warning

2017-05-10 Thread Michal Hocko
On Wed 10-05-17 01:27:34, Nick Desaulniers wrote:
> Clang flags this file with the -Wunsequenced error that GCC does not
> have.
> 
> unsequenced modification and access to 'gfp_mask'
> 
> It seems that gfp_mask is both read and written without a sequence point
> in between, which is undefined behavior.
> 
> Signed-off-by: Nick Desaulniers 

I will definitely not object to the patch as the code is cleaner and less 
tricky.
You can add
Acked-by: Michal Hocko 

But I
still do not understand which part of the code is undefined and why. My
reading and understanding of the C specification is that
struct A {
int a;
int b;
};

struct A f = { .a = c = foo(c), .b = c};

as long as foo(c) doesn't have any side effects because because .a is
initialized before b and the assignment ordering will make sure that c
is initialized before a.

6.7.8 par 19 (ISO/IEC 9899)
19 The initialization shall occur in initializer list order, each
   initializer provided for a particular subobject overriding any
   previously listed initializer for the same subobject; all subobjects
   that are not initialized explicitly shall be initialized implicitly
   the same as objects that have static storage duration.

So is my understanding of the specification wrong or is this a bug in
-Wunsequenced in Clang?

> ---
> Changes in v2:
> - don't assign back to gfp_mask, reuse sc.gfp_mask
> - initialize reclaim_idx directly, without classzone_idx
> 
>  mm/vmscan.c | 13 ++---
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 4e7ed65842af..d32c42d17935 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -2958,7 +2958,7 @@ unsigned long try_to_free_pages(struct zonelist 
> *zonelist, int order,
>   unsigned long nr_reclaimed;
>   struct scan_control sc = {
>   .nr_to_reclaim = SWAP_CLUSTER_MAX,
> - .gfp_mask = (gfp_mask = current_gfp_context(gfp_mask)),
> + .gfp_mask = current_gfp_context(gfp_mask),
>   .reclaim_idx = gfp_zone(gfp_mask),
>   .order = order,
>   .nodemask = nodemask,
> @@ -2973,12 +2973,12 @@ unsigned long try_to_free_pages(struct zonelist 
> *zonelist, int order,
>* 1 is returned so that the page allocator does not OOM kill at this
>* point.
>*/
> - if (throttle_direct_reclaim(gfp_mask, zonelist, nodemask))
> + if (throttle_direct_reclaim(sc.gfp_mask, zonelist, nodemask))
>   return 1;
>  
>   trace_mm_vmscan_direct_reclaim_begin(order,
>   sc.may_writepage,
> - gfp_mask,
> + sc.gfp_mask,
>   sc.reclaim_idx);
>  
>   nr_reclaimed = do_try_to_free_pages(zonelist, &sc);
> @@ -3763,16 +3763,15 @@ static int __node_reclaim(struct pglist_data *pgdat, 
> gfp_t gfp_mask, unsigned in
>   const unsigned long nr_pages = 1 << order;
>   struct task_struct *p = current;
>   struct reclaim_state reclaim_state;
> - int classzone_idx = gfp_zone(gfp_mask);
>   struct scan_control sc = {
>   .nr_to_reclaim = max(nr_pages, SWAP_CLUSTER_MAX),
> - .gfp_mask = (gfp_mask = current_gfp_context(gfp_mask)),
> + .gfp_mask = current_gfp_context(gfp_mask),
>   .order = order,
>   .priority = NODE_RECLAIM_PRIORITY,
>   .may_writepage = !!(node_reclaim_mode & RECLAIM_WRITE),
>   .may_unmap = !!(node_reclaim_mode & RECLAIM_UNMAP),
>   .may_swap = 1,
> - .reclaim_idx = classzone_idx,
> + .reclaim_idx = gfp_zone(gfp_mask),
>   };
>  
>   cond_resched();
> @@ -3782,7 +3781,7 @@ static int __node_reclaim(struct pglist_data *pgdat, 
> gfp_t gfp_mask, unsigned in
>* and RECLAIM_UNMAP.
>*/
>   p->flags |= PF_MEMALLOC | PF_SWAPWRITE;
> - lockdep_set_current_reclaim_state(gfp_mask);
> + lockdep_set_current_reclaim_state(sc.gfp_mask);
>   reclaim_state.reclaimed_slab = 0;
>   p->reclaim_state = &reclaim_state;
>  
> -- 
> 2.11.0
> 

-- 
Michal Hocko
SUSE Labs


[PATCH v2 02/10] usb: musb: Add quirk to avoid skb reserve in gadget mode

2017-05-10 Thread Peter Ujfalusi
For tusb6010 the DMA functionality only possible if the buffer is 32bit
aligned (SYNC access to FIFO) since with ASYNC access the TX/RX offset
registers will corrupt eventually.
The MUSB_G_NO_SKB_RESERVE will set the quirk_avoids_skb_reserve flag in
usb_gadget struct to provide correctly aligned buffer.

Signed-off-by: Peter Ujfalusi 
---
 drivers/usb/musb/musb_core.c | 3 +++
 drivers/usb/musb/musb_core.h | 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 870da18f5077..87cbd56cc761 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -2224,6 +2224,9 @@ musb_init_controller(struct device *dev, int nIrq, void 
__iomem *ctrl)
musb->io.ep_select = musb_flat_ep_select;
}
 
+   if (musb->io.quirks & MUSB_G_NO_SKB_RESERVE)
+   musb->g.quirk_avoids_skb_reserve = 1;
+
/* At least tusb6010 has its own offsets */
if (musb->ops->ep_offset)
musb->io.ep_offset = musb->ops->ep_offset;
diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
index 3e98d4268a64..9f22c5b8ce37 100644
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -172,6 +172,7 @@ struct musb_io;
  */
 struct musb_platform_ops {
 
+#define MUSB_G_NO_SKB_RESERVE  BIT(9)
 #define MUSB_DA8XX BIT(8)
 #define MUSB_PRESERVE_SESSION  BIT(7)
 #define MUSB_DMA_UX500 BIT(6)
-- 
2.12.2



Vertrauenswürdiges Darlehen

2017-05-10 Thread YesGrowth Loans®
Schönen Tag,

   Ich bin Frau Rose Butler, die Exekutive eines gut anerkannten legitimen 
Kreditvergabe-Unternehmen bekannt als YesGrowth Darlehen. Hast du einen 
schlechten Kredit oder du brauchst Geld, um deine Rechnungen zu bezahlen? Unser 
Zinssatz beträgt 3%.

Füllen Sie das Formular unten aus, wenn interessiert.

Vollständiger Name:
Geschlecht:
Benötigte Menge:
Dauer:

Vielen Dank,
Frau Rose Butler


[PATCH v2 01/10] dmaengine: omap-dma: port_window support correction for both direction

2017-05-10 Thread Peter Ujfalusi
When the port_window support was verified it was done on setup where only
the MEM_TO_DEV direction was enabled. This got un-noticed and thus only
this direction worked.

Now that I have managed to get a setup to verify both direction it turned
out that the setup was incorrect:
omap_desc members are settings for the slave port while the omap_sg members
apply to the memory side of the sDMA setup.

Fixes: 527a27591312 ("dmaengine: omap-dma: Fix the port_window support")
Signed-off-by: Peter Ujfalusi 
Cc: Russell King 
Cc: dmaeng...@vger.kernel.org
Cc: dan.j.willi...@intel.com
Cc: vinod.k...@intel.com
---
 drivers/dma/omap-dma.c | 39 +++
 1 file changed, 15 insertions(+), 24 deletions(-)

diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
index eed745a598fa..4fc86d40b50c 100644
--- a/drivers/dma/omap-dma.c
+++ b/drivers/dma/omap-dma.c
@@ -948,12 +948,6 @@ static struct dma_async_tx_descriptor 
*omap_dma_prep_slave_sg(
return NULL;
}
 
-   /* When the port_window is used, one frame must cover the window */
-   if (port_window) {
-   burst = port_window;
-   port_window_bytes = port_window * es_bytes[es];
-   }
-
/* Now allocate and setup the descriptor. */
d = kzalloc(sizeof(*d) + sglen * sizeof(d->sg[0]), GFP_ATOMIC);
if (!d)
@@ -963,6 +957,21 @@ static struct dma_async_tx_descriptor 
*omap_dma_prep_slave_sg(
d->dev_addr = dev_addr;
d->es = es;
 
+   /* When the port_window is used, one frame must cover the window */
+   if (port_window) {
+   burst = port_window;
+   port_window_bytes = port_window * es_bytes[es];
+
+   d->ei = 1;
+   /*
+* One frame covers the port_window and by  configure
+* the source frame index to be -1 * (port_window - 1)
+* we instruct the sDMA that after a frame is processed
+* it should move back to the start of the window.
+*/
+   d->fi = -(port_window_bytes - 1);
+   }
+
d->ccr = c->ccr | CCR_SYNC_FRAME;
if (dir == DMA_DEV_TO_MEM) {
d->csdp = CSDP_DST_BURST_64 | CSDP_DST_PACKED;
@@ -987,14 +996,6 @@ static struct dma_async_tx_descriptor 
*omap_dma_prep_slave_sg(
d->ccr |= CCR_SRC_AMODE_POSTINC;
if (port_window) {
d->ccr |= CCR_DST_AMODE_DBLIDX;
-   d->ei = 1;
-   /*
-* One frame covers the port_window and by  configure
-* the source frame index to be -1 * (port_window - 1)
-* we instruct the sDMA that after a frame is processed
-* it should move back to the start of the window.
-*/
-   d->fi = -(port_window_bytes - 1);
 
if (port_window_bytes >= 64)
d->csdp |= CSDP_DST_BURST_64;
@@ -1050,16 +1051,6 @@ static struct dma_async_tx_descriptor 
*omap_dma_prep_slave_sg(
osg->addr = sg_dma_address(sgent);
osg->en = en;
osg->fn = sg_dma_len(sgent) / frame_bytes;
-   if (port_window && dir == DMA_DEV_TO_MEM) {
-   osg->ei = 1;
-   /*
-* One frame covers the port_window and by  configure
-* the source frame index to be -1 * (port_window - 1)
-* we instruct the sDMA that after a frame is processed
-* it should move back to the start of the window.
-*/
-   osg->fi = -(port_window_bytes - 1);
-   }
 
if (d->using_ll) {
osg->t2_desc = dma_pool_alloc(od->desc_pool, GFP_ATOMIC,
-- 
2.12.2



[PATCH v2 07/10] usb: musb: tusb6010_omap: Allocate DMA channels upfront

2017-05-10 Thread Peter Ujfalusi
Instead of requesting the DMA channel in tusb_omap_dma_allocate() do it
when the controller is created and in runtime work from the DMA channel
pool.

This change is needed for the DMAengine conversion of the driver since the
tusb_omap_dma_allocate() is called in interrupt context which might lead
to lock within the DMAengine API when requesting channel.

Signed-off-by: Peter Ujfalusi 
---
 drivers/usb/musb/tusb6010_omap.c | 184 +++
 1 file changed, 92 insertions(+), 92 deletions(-)

diff --git a/drivers/usb/musb/tusb6010_omap.c b/drivers/usb/musb/tusb6010_omap.c
index f1e58e15e5bb..2daeef7e572d 100644
--- a/drivers/usb/musb/tusb6010_omap.c
+++ b/drivers/usb/musb/tusb6010_omap.c
@@ -45,7 +45,7 @@ struct tusb_omap_dma_ch {
u8  tx;
struct musb_hw_ep   *hw_ep;
 
-   struct tusb_dma_datadma_data;
+   struct tusb_dma_data*dma_data;
 
struct tusb_omap_dma*tusb_dma;
 
@@ -62,7 +62,7 @@ struct tusb_omap_dma {
struct dma_controller   controller;
void __iomem*tbase;
 
-   struct tusb_dma_datadma_data;
+   struct tusb_dma_datadma_pool[MAX_DMAREQ];
unsignedmultichannel:1;
 };
 
@@ -120,10 +120,7 @@ static void tusb_omap_dma_cb(int lch, u16 ch_status, void 
*data)
 
spin_lock_irqsave(&musb->lock, flags);
 
-   if (tusb_dma->multichannel)
-   ch = chdat->dma_data.ch;
-   else
-   ch = tusb_dma->dma_data.ch;
+   ch = chdat->dma_data->ch;
 
if (ch_status != OMAP_DMA_BLOCK_IRQ)
printk(KERN_ERR "TUSB DMA error status: %i\n", ch_status);
@@ -247,7 +244,8 @@ static int tusb_omap_dma_program(struct dma_channel 
*channel, u16 packet_sz,
dma_remaining = TUSB_EP_CONFIG_XFR_SIZE(dma_remaining);
if (dma_remaining) {
dev_dbg(musb->controller, "Busy %s dma ch%i, not using: %08x\n",
-   chdat->tx ? "tx" : "rx", chdat->dma_data.ch,
+   chdat->tx ? "tx" : "rx",
+   chdat->dma_data ? chdat->dma_data->ch : -1,
dma_remaining);
return false;
}
@@ -259,11 +257,8 @@ static int tusb_omap_dma_program(struct dma_channel 
*channel, u16 packet_sz,
else
chdat->transfer_packet_sz = packet_sz;
 
-   if (tusb_dma->multichannel) {
-   dma_data = &chdat->dma_data;
-   } else {
-   dma_data = &tusb_dma->dma_data;
-
+   dma_data = chdat->dma_data;
+   if (!tusb_dma->multichannel) {
if (tusb_omap_use_shared_dmareq(chdat) != 0) {
dev_dbg(musb->controller, "could not get dma for 
ep%i\n", chdat->epnum);
return false;
@@ -275,10 +270,10 @@ static int tusb_omap_dma_program(struct dma_channel 
*channel, u16 packet_sz,
WARN_ON(1);
return false;
}
-
-   omap_set_dma_callback(dma_data->ch, tusb_omap_dma_cb, channel);
}
 
+   omap_set_dma_callback(dma_data->ch, tusb_omap_dma_cb, channel);
+
chdat->packet_sz = packet_sz;
chdat->len = len;
channel->actual_len = 0;
@@ -409,19 +404,9 @@ static int tusb_omap_dma_program(struct dma_channel 
*channel, u16 packet_sz,
 static int tusb_omap_dma_abort(struct dma_channel *channel)
 {
struct tusb_omap_dma_ch *chdat = to_chdat(channel);
-   struct tusb_omap_dma*tusb_dma = chdat->tusb_dma;
-   struct tusb_dma_data*dma_data = &tusb_dma->dma_data;
 
-   if (!tusb_dma->multichannel) {
-   if (dma_data->ch >= 0) {
-   omap_stop_dma(dma_data->ch);
-   omap_free_dma(dma_data->ch);
-   dma_data->ch = -1;
-   }
-
-   dma_data->dmareq = -1;
-   dma_data->sync_dev = -1;
-   }
+   if (chdat->dma_data)
+   omap_stop_dma(chdat->dma_data->ch);
 
channel->status = MUSB_DMA_STATUS_FREE;
 
@@ -433,15 +418,6 @@ static inline int tusb_omap_dma_allocate_dmareq(struct 
tusb_omap_dma_ch *chdat)
u32 reg = musb_readl(chdat->tbase, TUSB_DMA_EP_MAP);
int i, dmareq_nr = -1;
 
-   const int sync_dev[6] = {
-   OMAP24XX_DMA_EXT_DMAREQ0,
-   OMAP24XX_DMA_EXT_DMAREQ1,
-   OMAP242X_DMA_EXT_DMAREQ2,
-   OMAP242X_DMA_EXT_DMAREQ3,
-   OMAP242X_DMA_EXT_DMAREQ4,
-   OMAP242X_DMA_EXT_DMAREQ5,
-   };
-
for (i = 0; i < MAX_DMAREQ; i++) {
int cur = (reg & (0xf << (i * 5))) >> (i * 5);
if (cur == 0) {
@@ -458,8 +434,7 @@ static inline int tusb_omap_dma_allocate_dmareq(struct 
tusb_omap_dma_ch *chdat)
reg |= ((1 << 4) << (dmareq_nr * 5));
musb_writel(chdat->tbase, TUSB_DMA_EP_MAP, re

[PATCH v2 05/10] usb: musb: tusb6010_omap: Do not reset the other direction's packet size

2017-05-10 Thread Peter Ujfalusi
We have one register for each EP to set the maximum packet size for both
TX and RX.
If for example an RX programming would happen before the previous TX
transfer finishes we would reset the TX packet side.

To fix this issue, only modify the TX or RX part of the register.

Signed-off-by: Peter Ujfalusi 
---
 drivers/usb/musb/tusb6010_omap.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/musb/tusb6010_omap.c b/drivers/usb/musb/tusb6010_omap.c
index db2e4c379ccf..4e1a6e4a61b8 100644
--- a/drivers/usb/musb/tusb6010_omap.c
+++ b/drivers/usb/musb/tusb6010_omap.c
@@ -389,15 +389,19 @@ static int tusb_omap_dma_program(struct dma_channel 
*channel, u16 packet_sz,
 
if (chdat->tx) {
/* Send transfer_packet_sz packets at a time */
-   musb_writel(ep_conf, TUSB_EP_MAX_PACKET_SIZE_OFFSET,
-   chdat->transfer_packet_sz);
+   u32 psize = musb_readl(ep_conf, TUSB_EP_MAX_PACKET_SIZE_OFFSET);
+   psize &= ~0x7ff;
+   psize |= chdat->transfer_packet_sz;
+   musb_writel(ep_conf, TUSB_EP_MAX_PACKET_SIZE_OFFSET, psize);
 
musb_writel(ep_conf, TUSB_EP_TX_OFFSET,
TUSB_EP_CONFIG_XFR_SIZE(chdat->transfer_len));
} else {
/* Receive transfer_packet_sz packets at a time */
-   musb_writel(ep_conf, TUSB_EP_MAX_PACKET_SIZE_OFFSET,
-   chdat->transfer_packet_sz << 16);
+   u32 psize = musb_readl(ep_conf, TUSB_EP_MAX_PACKET_SIZE_OFFSET);
+   psize &= ~(0x7ff << 16);
+   psize |= (chdat->transfer_packet_sz << 16);
+   musb_writel(ep_conf, TUSB_EP_MAX_PACKET_SIZE_OFFSET, psize);
 
musb_writel(ep_conf, TUSB_EP_RX_OFFSET,
TUSB_EP_CONFIG_XFR_SIZE(chdat->transfer_len));
-- 
2.12.2



[PATCH v2 00/10] usb: musb: tusb6010_omap: Convert to DMAengine

2017-05-10 Thread Peter Ujfalusi
Hi,

Changes since v1:
- Fix the port_window support in omap-dma DMAengine driver
- MUSB_G_NO_SKB_RESERVE quirk flag addition to msub core
- packet size corruption fix for tusb6010
- Handle DMA completion for TX also in the DMA callback

The v1 series was tested with g_cdc where only the DMA was only enabled for TX
and because of that I have not noticed that the sDMA code was not correct for 
RX,
it only worked for TX case.

The ASYNC mode of tusb6010 is really unstable, we get corrupted TX/RX offset
register quite easily, but the SYNC mode is stable.

The series was tested on top of next-20170510 with g_ncm module since with this
we can use the quirk to avoid skb_reserve and get properly aligned buffers for
DMA.
The n810 is using nfsroot.

The device would not boot to prompt most of the time before patch 5 (packet size
reset fix).
With that patch in, the device would boot up fine most of the cases, but will
fail pretty fast with my stress test [1].
After the first 9 patch the legacy DMA mode is going to be stable with g_ncm, it
boots to prompt, and survives the stress test [1].

The last patch is going the DMAengine conversion and I have run the stress test
against it over 3 hours (ping-test.sh wrap count is 139).

[1] Running these in parallel:
ping -f 192.168.0.2
ping -f -s 2048 192.168.0.2
ping-test.sh 192.168.0.2 1

and (nfsroot) time to time:
scp root@192.168.0.1:/usr/portage/distfiles/thunderbird-52.1.0.source.tar.xz /

$ ls -alh /usr/portage/distfiles/thunderbird-52.1.0.source.tar.xz 
218M Apr 30 15:46 /usr/portage/distfiles/thunderbird-52.1.0.source.tar.xz

In essence copy 218M from my host back to the host.

ping-test.sh (modified version from Tony to show the wrap count):
#!/bin/bash

device=$1
size=$2
wraps=0

while [ 1 ]; do
#echo "Pinging with size $size"
if ! ping -w0 -c1 -s$size $device > /dev/null 2>&1; then
break;
fi
size=$(expr $size + 1)

if [ $size -gt 8192 ]; then
wraps=$(expr $wraps + 1)
echo "wrapping ($wraps) at $size"
size=1
fi
done
echo "Test ran up to $size"

Regards,
Peter

CC: dmaeng...@vger.kernel.org
I only send the cover letter and the DMAengine patch for the dmaengine list, the
rest can be checked - if there is interest - via lkml
---
Peter Ujfalusi (10):
  dmaengine: omap-dma: port_window support correction for both direction
  usb: musb: Add quirk to avoid skb reserve in gadget mode
  usb: musb: tusb6010: Add MUSB_G_NO_SKB_RESERVE to quirks
  usb: musb: tusb6010_omap: Use one musb_ep_select call in
tusb_omap_dma_program
  usb: musb: tusb6010_omap: Do not reset the other direction's packet
size
  usb: musb: tusb6010_omap: Create new struct for DMA data/parameters
  usb: musb: tusb6010_omap: Allocate DMA channels upfront
  usb: musb: tusb6010: Handle DMA TX completion in DMA callback as well
  ARM: OMAP2+: DMA: Add slave map entries for 24xx external request
lines
  usb: musb: tusb6010_omap: Convert to DMAengine API

 arch/arm/mach-omap2/dma.c|  24 +++
 drivers/dma/omap-dma.c   |  39 ++--
 drivers/usb/musb/musb_core.c |   3 +
 drivers/usb/musb/musb_core.h |   1 +
 drivers/usb/musb/tusb6010.c  |  21 +--
 drivers/usb/musb/tusb6010_omap.c | 391 ---
 6 files changed, 211 insertions(+), 268 deletions(-)

-- 
2.12.2



[PATCH v2 04/10] usb: musb: tusb6010_omap: Use one musb_ep_select call in tusb_omap_dma_program

2017-05-10 Thread Peter Ujfalusi
Having one musb_ep_select() instead the two calls in if/else is the same
thing, but makes the code a bit simpler to follow.

Signed-off-by: Peter Ujfalusi 
---
 drivers/usb/musb/tusb6010_omap.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/musb/tusb6010_omap.c b/drivers/usb/musb/tusb6010_omap.c
index 8b43c4b99f04..db2e4c379ccf 100644
--- a/drivers/usb/musb/tusb6010_omap.c
+++ b/drivers/usb/musb/tusb6010_omap.c
@@ -367,15 +367,14 @@ static int tusb_omap_dma_program(struct dma_channel 
*channel, u16 packet_sz,
/*
 * Prepare MUSB for DMA transfer
 */
+   musb_ep_select(mbase, chdat->epnum);
if (chdat->tx) {
-   musb_ep_select(mbase, chdat->epnum);
csr = musb_readw(hw_ep->regs, MUSB_TXCSR);
csr |= (MUSB_TXCSR_AUTOSET | MUSB_TXCSR_DMAENAB
| MUSB_TXCSR_DMAMODE | MUSB_TXCSR_MODE);
csr &= ~MUSB_TXCSR_P_UNDERRUN;
musb_writew(hw_ep->regs, MUSB_TXCSR, csr);
} else {
-   musb_ep_select(mbase, chdat->epnum);
csr = musb_readw(hw_ep->regs, MUSB_RXCSR);
csr |= MUSB_RXCSR_DMAENAB;
csr &= ~(MUSB_RXCSR_AUTOCLEAR | MUSB_RXCSR_DMAMODE);
-- 
2.12.2



[PATCH v2 09/10] ARM: OMAP2+: DMA: Add slave map entries for 24xx external request lines

2017-05-10 Thread Peter Ujfalusi
The external request lines are used by tusb6010 on OMAP24xx platforms.
Update the map so the driver can use dmaengine API to request the DMA
channel. At the same time add temporary map containing only the external
DMA request numbers for DT booted case on omap24xx since the tusb6010 stack
is not yet supports DT boot.

Signed-off-by: Peter Ujfalusi 
---
 arch/arm/mach-omap2/dma.c | 24 
 1 file changed, 24 insertions(+)

diff --git a/arch/arm/mach-omap2/dma.c b/arch/arm/mach-omap2/dma.c
index e58c13a9bea5..0b77a0176018 100644
--- a/arch/arm/mach-omap2/dma.c
+++ b/arch/arm/mach-omap2/dma.c
@@ -249,6 +249,24 @@ static const struct dma_slave_map omap24xx_sdma_map[] = {
{ "omap_uart.2", "rx", SDMA_FILTER_PARAM(54) },
{ "omap_hsmmc.0", "tx", SDMA_FILTER_PARAM(61) },
{ "omap_hsmmc.0", "rx", SDMA_FILTER_PARAM(62) },
+
+   /* external DMA requests when tusb6010 is used */
+   { "musb-tusb", "dmareq0", SDMA_FILTER_PARAM(2) },
+   { "musb-tusb", "dmareq1", SDMA_FILTER_PARAM(3) },
+   { "musb-tusb", "dmareq2", SDMA_FILTER_PARAM(14) }, /* OMAP2420 only */
+   { "musb-tusb", "dmareq3", SDMA_FILTER_PARAM(15) }, /* OMAP2420 only */
+   { "musb-tusb", "dmareq4", SDMA_FILTER_PARAM(16) }, /* OMAP2420 only */
+   { "musb-tusb", "dmareq5", SDMA_FILTER_PARAM(64) }, /* OMAP2420 only */
+};
+
+static const struct dma_slave_map omap24xx_sdma_dt_map[] = {
+   /* external DMA requests when tusb6010 is used */
+   { "musb-hdrc.1.auto", "dmareq0", SDMA_FILTER_PARAM(2) },
+   { "musb-hdrc.1.auto", "dmareq1", SDMA_FILTER_PARAM(3) },
+   { "musb-hdrc.1.auto", "dmareq2", SDMA_FILTER_PARAM(14) }, /* OMAP2420 
only */
+   { "musb-hdrc.1.auto", "dmareq3", SDMA_FILTER_PARAM(15) }, /* OMAP2420 
only */
+   { "musb-hdrc.1.auto", "dmareq4", SDMA_FILTER_PARAM(16) }, /* OMAP2420 
only */
+   { "musb-hdrc.1.auto", "dmareq5", SDMA_FILTER_PARAM(64) }, /* OMAP2420 
only */
 };
 
 static const struct dma_slave_map omap3xxx_sdma_map[] = {
@@ -346,6 +364,12 @@ static int __init omap2_system_dma_init_dev(struct 
omap_hwmod *oh, void *unused)
   __func__);
return -ENODEV;
}
+   } else {
+   if (soc_is_omap24xx()) {
+   /* DMA slave map for drivers not yet converted to DT */
+   p.slave_map = omap24xx_sdma_dt_map;
+   p.slavecnt = ARRAY_SIZE(omap24xx_sdma_dt_map);
+   }
}
 
pdev = omap_device_build(name, 0, oh, &p, sizeof(p));
-- 
2.12.2



[PATCH v2 06/10] usb: musb: tusb6010_omap: Create new struct for DMA data/parameters

2017-05-10 Thread Peter Ujfalusi
For the DMA we have ch (channel), dmareq and sync_dev parameters both
within the tusb_omap_dma_ch and tusb_omap_dma_ch struct.
By creating a common struct the code can be simplified when selecting
between the shared or multichannel DMA parameters.

Signed-off-by: Peter Ujfalusi 
---
 drivers/usb/musb/tusb6010_omap.c | 163 ---
 1 file changed, 84 insertions(+), 79 deletions(-)

diff --git a/drivers/usb/musb/tusb6010_omap.c b/drivers/usb/musb/tusb6010_omap.c
index 4e1a6e4a61b8..f1e58e15e5bb 100644
--- a/drivers/usb/musb/tusb6010_omap.c
+++ b/drivers/usb/musb/tusb6010_omap.c
@@ -31,6 +31,12 @@
 #define OMAP242X_DMA_EXT_DMAREQ4   16
 #define OMAP242X_DMA_EXT_DMAREQ5   64
 
+struct tusb_dma_data {
+   int ch;
+   s8  dmareq;
+   s8  sync_dev;
+};
+
 struct tusb_omap_dma_ch {
struct musb *musb;
void __iomem*tbase;
@@ -39,9 +45,7 @@ struct tusb_omap_dma_ch {
u8  tx;
struct musb_hw_ep   *hw_ep;
 
-   int ch;
-   s8  dmareq;
-   s8  sync_dev;
+   struct tusb_dma_datadma_data;
 
struct tusb_omap_dma*tusb_dma;
 
@@ -58,9 +62,7 @@ struct tusb_omap_dma {
struct dma_controller   controller;
void __iomem*tbase;
 
-   int ch;
-   s8  dmareq;
-   s8  sync_dev;
+   struct tusb_dma_datadma_data;
unsignedmultichannel:1;
 };
 
@@ -119,9 +121,9 @@ static void tusb_omap_dma_cb(int lch, u16 ch_status, void 
*data)
spin_lock_irqsave(&musb->lock, flags);
 
if (tusb_dma->multichannel)
-   ch = chdat->ch;
+   ch = chdat->dma_data.ch;
else
-   ch = tusb_dma->ch;
+   ch = tusb_dma->dma_data.ch;
 
if (ch_status != OMAP_DMA_BLOCK_IRQ)
printk(KERN_ERR "TUSB DMA error status: %i\n", ch_status);
@@ -140,8 +142,7 @@ static void tusb_omap_dma_cb(int lch, u16 ch_status, void 
*data)
/* HW issue #10: XFR_SIZE may get corrupt on DMA (both async & sync) */
if (unlikely(remaining > chdat->transfer_len)) {
dev_dbg(musb->controller, "Corrupt %s dma ch%i XFR_SIZE: 
0x%08lx\n",
-   chdat->tx ? "tx" : "rx", chdat->ch,
-   remaining);
+   chdat->tx ? "tx" : "rx", ch, remaining);
remaining = 0;
}
 
@@ -219,9 +220,7 @@ static int tusb_omap_dma_program(struct dma_channel 
*channel, u16 packet_sz,
u32 dma_remaining;
int src_burst, dst_burst;
u16 csr;
-   int ch;
-   s8  dmareq;
-   s8  sync_dev;
+   struct tusb_dma_data*dma_data;
 
if (unlikely(dma_addr & 0x1) || (len < 32) || (len > packet_sz))
return false;
@@ -248,7 +247,7 @@ static int tusb_omap_dma_program(struct dma_channel 
*channel, u16 packet_sz,
dma_remaining = TUSB_EP_CONFIG_XFR_SIZE(dma_remaining);
if (dma_remaining) {
dev_dbg(musb->controller, "Busy %s dma ch%i, not using: %08x\n",
-   chdat->tx ? "tx" : "rx", chdat->ch,
+   chdat->tx ? "tx" : "rx", chdat->dma_data.ch,
dma_remaining);
return false;
}
@@ -261,15 +260,15 @@ static int tusb_omap_dma_program(struct dma_channel 
*channel, u16 packet_sz,
chdat->transfer_packet_sz = packet_sz;
 
if (tusb_dma->multichannel) {
-   ch = chdat->ch;
-   dmareq = chdat->dmareq;
-   sync_dev = chdat->sync_dev;
+   dma_data = &chdat->dma_data;
} else {
+   dma_data = &tusb_dma->dma_data;
+
if (tusb_omap_use_shared_dmareq(chdat) != 0) {
dev_dbg(musb->controller, "could not get dma for 
ep%i\n", chdat->epnum);
return false;
}
-   if (tusb_dma->ch < 0) {
+   if (dma_data->ch < 0) {
/* REVISIT: This should get blocked earlier, happens
 * with MSC ErrorRecoveryTest
 */
@@ -277,10 +276,7 @@ static int tusb_omap_dma_program(struct dma_channel 
*channel, u16 packet_sz,
return false;
}
 
-   ch = tusb_dma->ch;
-   dmareq = tusb_dma->dmareq;
-   sync_dev = tusb_dma->sync_dev;
-   omap_set_dma_callback(ch, tusb_omap_dma_cb, channel);
+   omap_set_dma_callback(dma_data->ch, tusb

Re: kvm: warning in kvm_load_guest_fpu

2017-05-10 Thread Wanpeng Li
2017-05-10 9:48 GMT+08:00 Wanpeng Li :
> 2017-05-09 22:04 GMT+08:00 Andrey Konovalov :
>> Hi,
>>
>> I've got the following error report while fuzzing the kernel with syzkaller.
>>
>> On commit 2868b2513aa732a99ea4a0a6bf10dc93c1f3dac2 (4.11+).
>>
>> A reproducer and .config are attached.
>
> If there are beauty codes for testing?

It messes the MXCSR Register reserve bits, I will make a patch.

Regards,
Wanpeng Li

>
>>
>> [ cut here ]
>> WARNING: CPU: 0 PID: 4108 at ./arch/x86/include/asm/fpu/internal.h:169
>> kvm_load_guest_fpu.part.163+0x2a9/0x430
>> Modules linked in:
>> CPU: 0 PID: 4108 Comm: a.out Not tainted 4.11.0+ #331
>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>> task: 880068b2c200 task.stack: 88006921
>> RIP: 0010:copy_kernel_to_fxregs ./arch/x86/include/asm/fpu/internal.h:169
>> RIP: 0010:__copy_kernel_to_fpregs ./arch/x86/include/asm/fpu/internal.h:459
>> RIP: 0010:kvm_load_guest_fpu.part.163+0x2a9/0x430 arch/x86/kvm/x86.c:7596
>> RSP: 0018:8800692176e8 EFLAGS: 00010297
>> RAX: 880068b2c200 RBX: 11000d242edd RCX: 8800696fc5ec
>> RDX:  RSI:  RDI: 880068b2d4c5
>> RBP: 8800692177b0 R08: 11000d242ebf R09: f8f8
>> R10:  R11: 0002 R12: 8800696f8000
>> R13: 880069217788 R14: dc00 R15: 8800696f8000
>> FS:  7fb239f787c0() GS:88006ca0() knlGS:
>> CS:  0010 DS:  ES:  CR0: 80050033
>> CR2: 2001f000 CR3: 6c55a000 CR4: 26f0
>> Call Trace:
>>  kvm_load_guest_fpu arch/x86/kvm/x86.c:6737
>>  vcpu_enter_guest arch/x86/kvm/x86.c:6842
>>  vcpu_run arch/x86/kvm/x86.c:7030
>>  kvm_arch_vcpu_ioctl_run+0x1f61/0x4860 arch/x86/kvm/x86.c:7191
>>  kvm_vcpu_ioctl+0x673/0x1120 arch/x86/kvm/../../../virt/kvm/kvm_main.c:2568
>>  vfs_ioctl fs/ioctl.c:45
>>  do_vfs_ioctl+0x1bf/0x1660 fs/ioctl.c:685
>>  SYSC_ioctl fs/ioctl.c:700
>>  SyS_ioctl+0x8f/0xc0 fs/ioctl.c:691
>>  entry_SYSCALL_64_fastpath+0x1f/0xbe arch/x86/entry/entry_64.S:204
>> RIP: 0033:0x7fb23968bb79
>> RSP: 002b:7ffec57db2d8 EFLAGS: 0202 ORIG_RAX: 0010
>> RAX: ffda RBX: 7ffec57db480 RCX: 7fb23968bb79
>> RDX:  RSI: ae80 RDI: 0005
>> RBP: 00400b40 R08:  R09: 
>> R10:  R11: 0202 R12: 
>> R13: 7ffec57db480 R14:  R15: 
>> Code: 4e 00 65 ff 0d f9 72 f5 7e e9 5c fe ff ff e8 7f e4 4e 00 31 c0
>> 49 0f ae 8c 24 00 0b 00 00 85 c0 0f 84 35 fe ff ff e8 67 e4 4e 00 <0f>
>> ff e9 29 fe ff ff e8 5b e4 4e 00 65 ff 05 c4 72 f5 7e 4d 8d
>> ---[ end trace 7a89c6ce24f92b9b ]---


[PATCH v2 08/10] usb: musb: tusb6010: Handle DMA TX completion in DMA callback as well

2017-05-10 Thread Peter Ujfalusi
Handle the DMA TX in a similar way as we do for the RX: in the DMA
completion callback.

Since we are no longer using DMA completion interrupt for the TX we can as
wall keep these interrupts disabled, but keep the handler for debug
purposes.

Signed-off-by: Peter Ujfalusi 
---
 drivers/usb/musb/tusb6010.c  | 18 +++---
 drivers/usb/musb/tusb6010_omap.c | 34 +-
 2 files changed, 4 insertions(+), 48 deletions(-)

diff --git a/drivers/usb/musb/tusb6010.c b/drivers/usb/musb/tusb6010.c
index 4253bfb22043..4eb640c54f2c 100644
--- a/drivers/usb/musb/tusb6010.c
+++ b/drivers/usb/musb/tusb6010.c
@@ -881,26 +881,14 @@ static irqreturn_t tusb_musb_interrupt(int irq, void 
*__hci)
| TUSB_INT_SRC_ID_STATUS_CHNG))
idle_timeout = tusb_otg_ints(musb, int_src, tbase);
 
-   /* TX dma callback must be handled here, RX dma callback is
-* handled in tusb_omap_dma_cb.
+   /*
+* Just clear the DMA interrupt if it comes as the completion for both
+* TX and RX is handled by the DMA callback in tusb6010_omap
 */
if ((int_src & TUSB_INT_SRC_TXRX_DMA_DONE)) {
u32 dma_src = musb_readl(tbase, TUSB_DMA_INT_SRC);
-   u32 real_dma_src = musb_readl(tbase, TUSB_DMA_INT_MASK);
 
dev_dbg(musb->controller, "DMA IRQ %08x\n", dma_src);
-   real_dma_src = ~real_dma_src & dma_src;
-   if (tusb_dma_omap(musb) && real_dma_src) {
-   int tx_source = (real_dma_src & 0x);
-   int i;
-
-   for (i = 1; i <= 15; i++) {
-   if (tx_source & (1 << i)) {
-   dev_dbg(musb->controller, "completing 
ep%i %s\n", i, "tx");
-   musb_dma_completion(musb, i, 1);
-   }
-   }
-   }
musb_writel(tbase, TUSB_DMA_INT_CLEAR, dma_src);
}
 
diff --git a/drivers/usb/musb/tusb6010_omap.c b/drivers/usb/musb/tusb6010_omap.c
index 2daeef7e572d..34e0115a4629 100644
--- a/drivers/usb/musb/tusb6010_omap.c
+++ b/drivers/usb/musb/tusb6010_omap.c
@@ -173,13 +173,7 @@ static void tusb_omap_dma_cb(int lch, u16 ch_status, void 
*data)
 
channel->status = MUSB_DMA_STATUS_FREE;
 
-   /* Handle only RX callbacks here. TX callbacks must be handled based
-* on the TUSB DMA status interrupt.
-* REVISIT: Use both TUSB DMA status interrupt and OMAP DMA callback
-* interrupt for RX and TX.
-*/
-   if (!chdat->tx)
-   musb_dma_completion(musb, chdat->epnum, chdat->tx);
+   musb_dma_completion(musb, chdat->epnum, chdat->tx);
 
/* We must terminate short tx transfers manually by setting TXPKTRDY.
 * REVISIT: This same problem may occur with other MUSB dma as well.
@@ -463,22 +457,12 @@ tusb_omap_dma_allocate(struct dma_controller *c,
int ret, i;
struct tusb_omap_dma*tusb_dma;
struct musb *musb;
-   void __iomem*tbase;
struct dma_channel  *channel = NULL;
struct tusb_omap_dma_ch *chdat = NULL;
struct tusb_dma_data*dma_data = NULL;
-   u32 reg;
 
tusb_dma = container_of(c, struct tusb_omap_dma, controller);
musb = tusb_dma->controller.musb;
-   tbase = musb->ctrl_base;
-
-   reg = musb_readl(tbase, TUSB_DMA_INT_MASK);
-   if (tx)
-   reg &= ~(1 << hw_ep->epnum);
-   else
-   reg &= ~(1 << (hw_ep->epnum + 15));
-   musb_writel(tbase, TUSB_DMA_INT_MASK, reg);
 
/* REVISIT: Why does dmareq5 not work? */
if (hw_ep->epnum == 0) {
@@ -547,26 +531,10 @@ static void tusb_omap_dma_release(struct dma_channel 
*channel)
 {
struct tusb_omap_dma_ch *chdat = to_chdat(channel);
struct musb *musb = chdat->musb;
-   void __iomem*tbase = musb->ctrl_base;
-   u32 reg;
 
dev_dbg(musb->controller, "ep%i ch%i\n", chdat->epnum,
chdat->dma_data->ch);
 
-   reg = musb_readl(tbase, TUSB_DMA_INT_MASK);
-   if (chdat->tx)
-   reg |= (1 << chdat->epnum);
-   else
-   reg |= (1 << (chdat->epnum + 15));
-   musb_writel(tbase, TUSB_DMA_INT_MASK, reg);
-
-   reg = musb_readl(tbase, TUSB_DMA_INT_CLEAR);
-   if (chdat->tx)
-   reg |= (1 << chdat->epnum);
-   else
-   reg |= (1 << (chdat->epnum + 15));
-   musb_writel(tbase, TUSB_DMA_INT_CLEAR, reg);
-
channel->status = MUSB_DMA_STATUS_UNKNOWN;
 
omap_stop_dma(chdat->dma_data->ch);
-- 
2.12.2



Re: [PATCH] mm/vmscan: fix unsequenced modification and access warning

2017-05-10 Thread Nick Desaulniers
> You can add

Something that's not clear to me when advised to add, should I be
uploading a v3 with your acked by? I think I got that wrong the last
time I asked (which was my first patch to Linux).

> But I still do not understand which part of the code is undefined and
> why.

It's not immediately clear to me either, but it's super later here...

>  is this a bug in -Wunsequenced in Clang

Possibly, I think I already found one earlier tonight.

https://bugs.llvm.org/show_bug.cgi?id=32985

Tomorrow, I'll try to cut down a test case to see if this is indeed a
compiler bug.  Would you like me to change the commit message to call
this just a simple clean up, in the meantime?

Thanks,
~Nick


[PATCH v2 03/10] usb: musb: tusb6010: Add MUSB_G_NO_SKB_RESERVE to quirks

2017-05-10 Thread Peter Ujfalusi
When using the g_ncm for networking this flag will make sure that the
buffer is alligned to 32bit so the DMA can be used to offload the data
movement.

Signed-off-by: Peter Ujfalusi 
---
 drivers/usb/musb/tusb6010.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/musb/tusb6010.c b/drivers/usb/musb/tusb6010.c
index e85cc8e4e7a9..4253bfb22043 100644
--- a/drivers/usb/musb/tusb6010.c
+++ b/drivers/usb/musb/tusb6010.c
@@ -1181,7 +1181,8 @@ static int tusb_musb_exit(struct musb *musb)
 }
 
 static const struct musb_platform_ops tusb_ops = {
-   .quirks = MUSB_DMA_TUSB_OMAP | MUSB_IN_TUSB,
+   .quirks = MUSB_DMA_TUSB_OMAP | MUSB_IN_TUSB |
+ MUSB_G_NO_SKB_RESERVE,
.init   = tusb_musb_init,
.exit   = tusb_musb_exit,
 
-- 
2.12.2



Re: [PATCH v3 3/3] arm/arm64: signal SIBGUS and inject SEA Error

2017-05-10 Thread gengdongjiu
Hi James,
  thanks a lot for your answer.

On 2017/5/9 1:28, James Morse wrote:
> Hi gengdongjiu,
> 
> On 04/05/17 17:52, gengdongjiu wrote:
>> 2017-05-04 23:42 GMT+08:00 gengdongjiu :
>>> On 30/04/17 06:37, Dongjiu Geng wrote:
 diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
 index 105b6ab..a96594f 100644
 --- a/arch/arm/kvm/mmu.c
 +++ b/arch/arm/kvm/mmu.c
 +static void kvm_handle_bad_page(unsigned long address,
 + bool hugetlb, bool hwpoison)
 +{
 + /* handle both hwpoison and other synchronous external Abort */
 + if (hwpoison)
 + kvm_send_signal(address, hugetlb, true);
 + else
 + kvm_send_signal(address, hugetlb, false);
 +}
>>>
>>> Why the extra level of indirection? We only want to signal userspace like 
>>> this
>>> from KVM for hwpoison. Signals for RAS related reasons should come from the 
>>> bits
>>> of the kernel that decoded the error.
>>
>> For the SEA, the are maily two types:
>> 0b01 Synchronous External Abort on memory access.
>> 0b0101xx Synchronous External Abort on page table walk. DFSC[1:0]
>> encode the level.
> 
> (KVM shouldn't have to make decisions about this)
> 
> 
>> hwpoison should belong to the  "Synchronous External Abort on memory access"
>> if the SEA type is not hwpoison, such as page table walk, do you mean
>> KVM do not deliver the SIGBUS?
> 
> 
> The flow of events should be SEI/SEA from firmware to the hosts's APEI code. 
> KVM
> should only be involved to get us back to the host if we were running a guest.
> The APEI/hwpoison code may cause a set of processes to be sent signals. The 
> code
> in mm/memory-failure.c does this by walking the process rmaps using the 
> physical
> addresses in the CPER records.
> 
> We want user space to be sent signals as this can (and should) work in exactly
> the same way on arm64 as it does on x86 or any other architecture. If a
> web-browser can handle SIGBUS notifications for memory-corruption, it 
> shouldn't
> have to care what architecture it is running on.
 Ok, James, understand.

> 
> So what is that KVM+SIGBUS patch about?...
> 
>>> (hwpoison for KVM is a corner case as Qemu's memory effectively has two 
>>> users,
>>> Qemu and KVM. This isn't the example of how user-space gets signalled.)
> 
> KVM creates guests as if they were additional users of Qemu's memory. The code
> in mm/memory-failure.c may find that Qemu didn't have the affected page mapped
> to user-space - but it may have been in use by stage2.
> 
> The KVM+SIGBUS patch hides this difference, meaning Qemu gets a signal when 
> the
> guest touches the hwpoison page as if Qemu had touched the page itself.
> 
> Signals from KVM is a corner case, for firmware-first decisions should happen 
> in
> the APEI code based on CPER records.
> 
> 
>> If so, how the KVM handle the SEA type other than hwpoison?
> 
> To deliver to a guest? It shouldn't have to know, user space should use a KVM
> API to drive this.
> 
> When received from hardware? It shouldn't have to care, these things should be
> passed into the APEI code for handling. KVM just needs to put the host 
> registers
> back.
Recently I confirmed with the hardware team. they said almost all the SEA 
errors have the
Poison flag, so may be there is no need to consider other SEA errors other than 
 hwPoison.
only consider SEA hwpoison errors can be enough.

> 
> 
 diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
 index bb02909..1d2e2e7 100644
 --- a/include/uapi/linux/kvm.h
 +++ b/include/uapi/linux/kvm.h
 @@ -1306,6 +1306,7 @@ 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)
 +#define KVM_ARM_SEA   _IO(KVMIO,   0xb8)

  #define KVM_DEV_ASSIGN_ENABLE_IOMMU (1 << 0)
  #define KVM_DEV_ASSIGN_PCI_2_3 (1 << 1)

>>>
>>> Why do we need a userspace API for SEA? It can also be done by using
>>> KVM_{G,S}ET_ONE_REG to change the vcpu registers. The advantage of doing it 
>>> this
>>> way is you can choose which ESR value to use.
>>>
>>> Adding a new API call to do something you could do with an old one doesn't 
>>> look
>>> right.
>>
>> James, I considered your suggestion before that use the
>> KVM_{G,S}ET_ONE_REG to change the vcpu registers. but I found it does
>> not have difference to use the alread existed KVM API. 
> 
> (Only that is an in-kernel helper, not a published API)

yes, the kvm_inject_dabt is an in-kernel API.

> 
> 
>> so may be
>> changing the vcpu registers in qemu will duplicate with the KVM APIs.
> 
> That is true, but the alternative is a new API that doesn't do anything new, 
> its
> just more convenient.
> 
> Marc and Christoffer are the people to convince.
> I argue the existing API is sufficient.
> 
> 
>> injection a SEA is no more than setting some registers: elr_el1, PC,
>> PSTATE, SPSR_el1, far_el1, es

Re: [PATCH] staging: vc04_services: Fix bulk cache maintenance

2017-05-10 Thread Phil Elwell
On 04/05/2017 18:51, Stefan Wahren wrote:
> 
>> Phil Elwell  hat am 4. Mai 2017 um 11:58 geschrieben:
>>
>>
>> vchiq_arm supports transfers less than one page and at arbitrary
>> alignment, using the dma-mapping API to perform its cache maintenance
>> (even though the VPU drives the DMA hardware). Read (DMA_FROM_DEVICE)
>> operations use cache invalidation for speed, falling back to
>> clean+invalidate on partial cache lines, with writes (DMA_TO_DEVICE)
>> using flushes.
>>
>> If a read transfer has ends which aren't page-aligned, performing cache
>> maintenance as if they were whole pages can lead to memory corruption
>> since the partial cache lines at the ends (and any cache lines before or
>> after the transfer area) will be invalidated. This bug was masked until
>> the disabling of the cache flush in flush_dcache_page().
>>
>> Honouring the requested transfer start- and end-points prevents the
>> corruption.
>>
>> Fixes: cf9caf192988 ("staging: vc04_services: Replace dmac_map_area with 
>> dmac_map_sg")
>> Signed-off-by: Phil Elwell 
> 
> Reported-by: Stefan Wahren 
> Tested-by: Stefan Wahren 
> 
> In order to clarify the context of this issue:
> 
> http://lists.infradead.org/pipermail/linux-rpi-kernel/2017-April/006149.html

Thanks, Stefan.

Is there no feedback other on this patch? It's been in the rpi-4.11.y downstream
branch for a week now with favourable results - see issue
https://github.com/raspberrypi/linux/issues/1977 and pr
https://github.com/raspberrypi/linux/pull/1987.

Phil


Re: [PATCH v2 2/3] iio: adc: at91-sama5d2_adc: add hw trigger and buffer support

2017-05-10 Thread Eugen Hristev

Hello,

Thanks for the suggestions and reply.

Please see my answers inline

Eugen

On 07.05.2017 18:01, Jonathan Cameron wrote:

On 04/05/17 13:13, Eugen Hristev wrote:

Added support for the external hardware trigger on pin ADTRG,
integrated the three possible edge triggers into the subsystem
and created buffer management for data retrieval

Signed-off-by: Eugen Hristev 
---
  Changes in v2:
 - Moved buffer allocation and freeing into the preenable and postdisable
   callbacks.
   We have a total of scan bytes that can vary a lot depending on each channel
   enabled at a certain point.

How much?  Having dynamic allocations are likely to prove just as costly as 
you'll
almost always end up allocating a lot more than you ask for.

I'm counting worst case of 18x 2byte channels + timestamp = 48 bytes.
A quick google suggests you'll always allocate 32 bytes whatever with slab
(lower for slob). So not a huge saving...

Worth the hassle?



I will modify the allocation of scan_bytes to make it static.




 - made the at91 trigger list part of state structure
 - made the iio trigger list preallocated in state structure
 - moved irq enabling/disabling into the try_reenable callback

I'd have liked a little explanation on why we need to explicitly disable
the irq.  It will have little effect it if triggers too often as the
trigger consumers are all oneshot anyway.


Regarding the irq, the discussion is a bit longer.

As it is now, the interrupt is not a oneshot threaded one, because only
the top half handler is installed, and the threaded one is NULL.
Calling trigger_poll without disabling the interrupt will make the
handler loop, so that is the reason for disabling and reenabling
the interrupt.

One solution is to change it to oneshot threaded interrupt and call
trigger_poll_chained to make it a nested handling. This will make a
longer response time for conversions though.

One other option is to do irq_save and irq_restore before issuing the
trigger poll, but that limits the usability of the trigger by any
other device I guess.

I did the behavior with disabling and enabling the interrupt considering
the old at91 driver and the stm32-adc driver which looks to be fairly
similar with this implementation.

Can you please advise on which approach you think it's best for this
driver ?
So I can try that, and resend the patch.


 - on trigger disable must write disable registries as well

Basically looks good. A few questions inline.

Jonathan


 drivers/iio/adc/at91-sama5d2_adc.c | 231 -
 1 file changed, 228 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/adc/at91-sama5d2_adc.c 
b/drivers/iio/adc/at91-sama5d2_adc.c
index e10dca3..11f5570 100644
--- a/drivers/iio/adc/at91-sama5d2_adc.c
+++ b/drivers/iio/adc/at91-sama5d2_adc.c
@@ -23,8 +23,15 @@
 #include 
 #include 
 #include 
+#include 
+
 #include 
 #include 
+#include 
+#include 
+#include 
+#include 
+
 #include 

 /* Control Register */
@@ -132,6 +139,17 @@
 #define AT91_SAMA5D2_PRESSR0xbc
 /* Trigger Register */
 #define AT91_SAMA5D2_TRGR  0xc0
+/* Mask for TRGMOD field of TRGR register */
+#define AT91_SAMA5D2_TRGR_TRGMOD_MASK GENMASK(2, 0)
+/* No trigger, only software trigger can start conversions */
+#define AT91_SAMA5D2_TRGR_TRGMOD_NO_TRIGGER 0
+/* Trigger Mode external trigger rising edge */
+#define AT91_SAMA5D2_TRGR_TRGMOD_EXT_TRIG_RISE 1
+/* Trigger Mode external trigger falling edge */
+#define AT91_SAMA5D2_TRGR_TRGMOD_EXT_TRIG_FALL 2
+/* Trigger Mode external trigger any edge */
+#define AT91_SAMA5D2_TRGR_TRGMOD_EXT_TRIG_ANY 3
+
 /* Correction Select Register */
 #define AT91_SAMA5D2_COSR  0xd0
 /* Correction Value Register */
@@ -145,14 +163,20 @@
 /* Version Register */
 #define AT91_SAMA5D2_VERSION   0xfc

+#define AT91_SAMA5D2_HW_TRIG_CNT 3
+#define AT91_SAMA5D2_SINGLE_CHAN_CNT 12
+#define AT91_SAMA5D2_DIFF_CHAN_CNT 6
+
 #define AT91_SAMA5D2_CHAN_SINGLE(num, addr)\
{   \
.type = IIO_VOLTAGE,\
.channel = num, \
.address = addr,\
+   .scan_index = num,  \
.scan_type = {  \
.sign = 'u',\
.realbits = 12, \
+   .storagebits = 16,  \
},  \
.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),   \
.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),   \
@@ -168,9 +192,11 @@
.channel = num, \
.channel2 = num2,  

RE: [PATCH 1/4] hamradio: Combine two seq_printf() calls into one in yam_seq_show()

2017-05-10 Thread David Laight
From: SF Markus Elfring
> Sent: 09 May 2017 15:22
> A bit of data was put into a sequence by two separate function calls.
> Print the same data by a single function call instead.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring 
> ---
>  drivers/net/hamradio/yam.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/net/hamradio/yam.c b/drivers/net/hamradio/yam.c
> index b6891ada1d7b..542f1e511df1 100644
> --- a/drivers/net/hamradio/yam.c
> +++ b/drivers/net/hamradio/yam.c
> @@ -830,8 +830,7 @@ static int yam_seq_show(struct seq_file *seq, void *v)
>   seq_printf(seq, "  RxFrames %lu\n", dev->stats.rx_packets);
>   seq_printf(seq, "  TxInt%u\n", yp->nb_mdint);
>   seq_printf(seq, "  RxInt%u\n", yp->nb_rxint);
> - seq_printf(seq, "  RxOver   %lu\n", dev->stats.rx_fifo_errors);
> - seq_printf(seq, "\n");
> + seq_printf(seq, "  RxOver   %lu\n\n", dev->stats.rx_fifo_errors);
>   return 0;

The code was consistently (and probably deliberately) using one seq_printf()
call for each line so that the source looks 'a bit like' the output.

These changes are all stupid.

David



Re: [patch V2 24/24] cpu/hotplug: Convert hotplug locking to percpu rwsem

2017-05-10 Thread Thomas Gleixner
On Wed, 10 May 2017, Michael Ellerman wrote:

> Thomas Gleixner  writes:
> 
> > @@ -130,6 +130,7 @@ void __static_key_slow_inc(struct static
> >  * the all CPUs, for that to be serialized against CPU hot-plug
> >  * we need to avoid CPUs coming online.
> >  */
> > +   lockdep_assert_hotplug_held();
> > jump_label_lock();
> > if (atomic_read(&key->enabled) == 0) {
> > atomic_set(&key->enabled, -1);
> 
> I seem to be hitting this assert from the ftrace event selftests,
> enabled at boot with CONFIG_FTRACE_STARTUP_TEST=y, using next-20170509
> (on powerpc).
> 
> The stupidly obvious (or perhaps obviously stupid) patch below fixes it:

Kinda. There is more horror in that area lurking and I'm still trying to
figure out all the convoluted call pathes.

Thanks,

tglx

> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index daefdee9411a..5531f7ce8fa6 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -3241,9 +3241,19 @@ static __init void event_trace_self_tests(void)
>   continue;
>   }
>  
> + get_online_cpus();
> + mutex_lock(&event_mutex);
>   ftrace_event_enable_disable(file, 1);
> + mutex_unlock(&event_mutex);
> + put_online_cpus();
> +
>   event_test_stuff();
> +
> + get_online_cpus();
> + mutex_lock(&event_mutex);
>   ftrace_event_enable_disable(file, 0);
> + mutex_unlock(&event_mutex);
> + put_online_cpus();
>  
>   pr_cont("OK\n");
>   }
> 
> cheers
> 


[PATCH v2 10/10] usb: musb: tusb6010_omap: Convert to DMAengine API

2017-05-10 Thread Peter Ujfalusi
With the port_window support in DMAengine and the sDMA driver we can
convert the driver to DMAengine.

Signed-off-by: Peter Ujfalusi 
---
 drivers/usb/musb/tusb6010_omap.c | 201 ---
 1 file changed, 80 insertions(+), 121 deletions(-)

diff --git a/drivers/usb/musb/tusb6010_omap.c b/drivers/usb/musb/tusb6010_omap.c
index 34e0115a4629..0c2bd0befe72 100644
--- a/drivers/usb/musb/tusb6010_omap.c
+++ b/drivers/usb/musb/tusb6010_omap.c
@@ -15,7 +15,7 @@
 #include 
 #include 
 #include 
-#include 
+#include 
 
 #include "musb_core.h"
 #include "tusb6010.h"
@@ -24,17 +24,9 @@
 
 #define MAX_DMAREQ 5   /* REVISIT: Really 6, but req5 not OK */
 
-#define OMAP24XX_DMA_EXT_DMAREQ0   2
-#define OMAP24XX_DMA_EXT_DMAREQ1   3
-#define OMAP242X_DMA_EXT_DMAREQ2   14
-#define OMAP242X_DMA_EXT_DMAREQ3   15
-#define OMAP242X_DMA_EXT_DMAREQ4   16
-#define OMAP242X_DMA_EXT_DMAREQ5   64
-
 struct tusb_dma_data {
-   int ch;
s8  dmareq;
-   s8  sync_dev;
+   struct dma_chan *chan;
 };
 
 struct tusb_omap_dma_ch {
@@ -105,7 +97,7 @@ static inline void tusb_omap_free_shared_dmareq(struct 
tusb_omap_dma_ch *chdat)
  * See also musb_dma_completion in plat_uds.c and musb_g_[tx|rx]() in
  * musb_gadget.c.
  */
-static void tusb_omap_dma_cb(int lch, u16 ch_status, void *data)
+static void tusb_omap_dma_cb(void *data)
 {
struct dma_channel  *channel = (struct dma_channel *)data;
struct tusb_omap_dma_ch *chdat = to_chdat(channel);
@@ -116,18 +108,11 @@ static void tusb_omap_dma_cb(int lch, u16 ch_status, void 
*data)
void __iomem*ep_conf = hw_ep->conf;
void __iomem*mbase = musb->mregs;
unsigned long   remaining, flags, pio;
-   int ch;
 
spin_lock_irqsave(&musb->lock, flags);
 
-   ch = chdat->dma_data->ch;
-
-   if (ch_status != OMAP_DMA_BLOCK_IRQ)
-   printk(KERN_ERR "TUSB DMA error status: %i\n", ch_status);
-
-   dev_dbg(musb->controller, "ep%i %s dma callback ch: %i status: %x\n",
-   chdat->epnum, chdat->tx ? "tx" : "rx",
-   ch, ch_status);
+   dev_dbg(musb->controller, "ep%i %s dma callback\n",
+   chdat->epnum, chdat->tx ? "tx" : "rx");
 
if (chdat->tx)
remaining = musb_readl(ep_conf, TUSB_EP_TX_OFFSET);
@@ -138,8 +123,8 @@ static void tusb_omap_dma_cb(int lch, u16 ch_status, void 
*data)
 
/* HW issue #10: XFR_SIZE may get corrupt on DMA (both async & sync) */
if (unlikely(remaining > chdat->transfer_len)) {
-   dev_dbg(musb->controller, "Corrupt %s dma ch%i XFR_SIZE: 
0x%08lx\n",
-   chdat->tx ? "tx" : "rx", ch, remaining);
+   dev_dbg(musb->controller, "Corrupt %s XFR_SIZE: 0x%08lx\n",
+   chdat->tx ? "tx" : "rx", remaining);
remaining = 0;
}
 
@@ -206,12 +191,15 @@ static int tusb_omap_dma_program(struct dma_channel 
*channel, u16 packet_sz,
struct musb_hw_ep   *hw_ep = chdat->hw_ep;
void __iomem*mbase = musb->mregs;
void __iomem*ep_conf = hw_ep->conf;
-   dma_addr_t  fifo = hw_ep->fifo_sync;
-   struct omap_dma_channel_params  dma_params;
+   dma_addr_t  fifo_addr = hw_ep->fifo_sync;
u32 dma_remaining;
-   int src_burst, dst_burst;
u16 csr;
struct tusb_dma_data*dma_data;
+   struct dma_async_tx_descriptor  *dma_desc;
+   struct dma_slave_config dma_cfg;
+   enum dma_transfer_direction dma_dir;
+   u32 port_window;
+   int ret;
 
if (unlikely(dma_addr & 0x1) || (len < 32) || (len > packet_sz))
return false;
@@ -237,10 +225,8 @@ static int tusb_omap_dma_program(struct dma_channel 
*channel, u16 packet_sz,
 
dma_remaining = TUSB_EP_CONFIG_XFR_SIZE(dma_remaining);
if (dma_remaining) {
-   dev_dbg(musb->controller, "Busy %s dma ch%i, not using: %08x\n",
-   chdat->tx ? "tx" : "rx",
-   chdat->dma_data ? chdat->dma_data->ch : -1,
-   dma_remaining);
+   dev_dbg(musb->controller, "Busy %s dma, not using: %08x\n",
+   chdat->tx ? "tx" : "rx", dma_remaining);
return false;
}
 
@@ -257,7 +243,7 @@ static int tusb_omap_dma_program(struct dma_channel 
*channel, u16 packet_sz,
dev_dbg(musb->controller, "could not get dma for 
ep%i\n", chdat->epnum);
return false;
}
-   if (dma_data->ch < 0) {
+   if

DWC2 USB Host Mode Lockup 4.11

2017-05-10 Thread Tim Sander
Hi

I am currently seeing a error with the designware driver on
Intel/Altera ARM Cortex A9 Cyclone SOC V Hardware. The USB PHY is a
TUSB1210 without a hw reset line connected. The error only occurs on
plugging in of the device in host mode. Once the USB device is
enumerated i have not seen any errors. Ocassionally i get an error
that the USB Device is no longer enumerated. Even a reboot does not
help to recover to normal operation. This points IMHO to the PHY as
source of problem as all other components are getting a hw reset on
reboot. I have not worked with USB on a driver level so my knowledge
is a little thin. Nevertheless i tried to pin down the problem. I have
added the patch below to the 4.11 kernel. The observation is that when
the error has not been hit i see lots of "dwc2: STATUS EINPROGRESS"
messages. Which means the bug_on statement i added is not hit on
normal operation.

The usb hw-schematic looks like this:
https://rocketboards.org/foswiki/pub/Documentation/EBVSoCratesEvaluationBoard/SoCrates-Schematic.pdf

So my take is that for some reason the communication between PHY
and controller is broken in a way that either no request gets send to
the PHY or that the PHY is sending no reply.

Any idea how i can get this USB port back to normal operation?

Attached below is the patch which i added to produce the two output
dumps further below. The first output dump is the seldom error case,
the second is the success case.

Best regards
Tim

diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index a73722e27d07..1c18104e432f 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -38,6 +38,8 @@
  * This file contains the core HCD code, and implements the Linux hc_driver
  * API
  */
+#define DEBUG
+
 #include 
 #include 
 #include 
@@ -4663,6 +4665,7 @@ static int _dwc2_hcd_urb_enqueue(struct usb_hcd *hcd, 
struct urb *urb,
dwc2_urb->flags = tflags;
dwc2_urb->interval = urb->interval;
dwc2_urb->status = -EINPROGRESS;
+   printk("dwc2: STATUS EINPROGRESS\n");
 
for (i = 0; i < urb->number_of_packets; ++i)
dwc2_hcd_urb_set_iso_desc_params(dwc2_urb, i,
@@ -4773,6 +4776,7 @@ static int _dwc2_hcd_urb_dequeue(struct usb_hcd *hcd, 
struct urb *urb,
 
dev_dbg(hsotg->dev, "Called usb_hcd_giveback_urb()\n");
dev_dbg(hsotg->dev, "  urb->status = %d\n", urb->status);
+   BUG_ON(urb->status <0);
 out:
spin_unlock_irqrestore(&hsotg->lock, flags);

Here is the output in the error case:
[   11.245681] usbcore: registered new interface driver usbfs
[   11.254272] usbcore: registered new interface driver hub
[   11.262479] usbcore: registered new device driver usb
[   11.346143] dwc2 ffb0.usb: mapped PA ffb0 to VA 9155
[   11.346236] dwc2 ffb0.usb: Looking up vusb_d-supply from device tree
[   11.346254] dwc2 ffb0.usb: Looking up vusb_d-supply property in node 
/soc/usb@ffb0 failed
[   11.346273] dwc2 ffb0.usb: ffb0.usb supply vusb_d not found, using 
dummy regulator
[   11.354882] dwc2 ffb0.usb: Looking up vusb_a-supply from device tree
[   11.354897] dwc2 ffb0.usb: Looking up vusb_a-supply property in node 
/soc/usb@ffb0 failed
[   11.354909] dwc2 ffb0.usb: ffb0.usb supply vusb_a not found, using 
dummy regulator
[   11.363660] dwc2 ffb0.usb: registering common handler for irq43
[   11.363848] dwc2 ffb0.usb: Forcing mode to host
[   11.363868] dwc2 ffb0.usb: Core Release: 2.93a (snpsid=4f54293a)
[   11.363882] dwc2 ffb0.usb: Forcing mode to host
[   11.363909] dwc2 ffb0.usb: DWC OTG HCD INIT
[   11.363921] dwc2 ffb0.usb: hcfg=0200
[   11.363950] dwc2 ffb0.usb: dwc2_core_init(8481e010)
[   11.363962] dwc2 ffb0.usb: HS ULPI PHY selected
[   11.363974] dwc2 ffb0.usb: Internal DMA Mode
[   11.363987] dwc2 ffb0.usb: host_dma:1 dma_desc_enable:1
[   11.363998] dwc2 ffb0.usb: Using Descriptor DMA mode
[   11.364010] dwc2 ffb0.usb: Host Mode
[   11.375756] dwc2 ffb0.usb: DWC OTG Controller
[   11.380596] dwc2 ffb0.usb: new USB bus registered, assigned bus number 1
[   11.387883] dwc2 ffb0.usb: irq 43, io mem 0xffb0
[   11.393368] dwc2 ffb0.usb: DWC OTG HCD START
[   11.393389] dwc2 ffb0.usb: dwc2_core_host_init(8481e010)
[   11.393403] dwc2 ffb0.usb: Initializing HCFG.FSLSPClkSel to 
[   11.393417] dwc2 ffb0.usb: initial grxfsiz=2000
[   11.393429] dwc2 ffb0.usb: new grxfsiz=0200
[   11.393441] dwc2 ffb0.usb: initial gnptxfsiz=20002000
[   11.393454] dwc2 ffb0.usb: new gnptxfsiz=02000200
[   11.393465] dwc2 ffb0.usb: initial hptxfsiz=20004000
[   11.393477] dwc2 ffb0.usb: new hptxfsiz=02000400
[   11.393495] dwc2 ffb0.usb: Init: Port Power? op_state=9
[   11.393502] dwc2 ffb0.usb: Init: Power Port (0)
[   11.393508] dwc2 ffb0.usb: dwc2_enable_host_interrupts()
[   11.393519] dwc2 ffb0.usb: DWC OTG HCD Has Root Hub
[   11.393979] usb usb1: Ne

[PATCH v5 02/17] net: qca_framing: use u16 for frame offset

2017-05-10 Thread Stefan Wahren
It doesn't make sense to use a signed variable for offset here, so
fix it up.

Signed-off-by: Stefan Wahren 
---
 drivers/net/ethernet/qualcomm/qca_framing.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/qualcomm/qca_framing.h 
b/drivers/net/ethernet/qualcomm/qca_framing.h
index d5e795d..8b385e6 100644
--- a/drivers/net/ethernet/qualcomm/qca_framing.h
+++ b/drivers/net/ethernet/qualcomm/qca_framing.h
@@ -103,7 +103,7 @@ struct qcafrm_handle {
enum qcafrm_state state;
 
/* Offset in buffer (borrowed for length too) */
-   s16 offset;
+   u16 offset;
 
/* Frame length as kept by this module */
u16 len;
-- 
2.1.4



[PATCH v5 16/17] tty: serdev-ttyport: return actual baudrate from ttyport_set_baudrate

2017-05-10 Thread Stefan Wahren
Instead of returning the requested baudrate, we better return the
actual one because it isn't always the same.

Signed-off-by: Stefan Wahren 
Acked-by: Rob Herring 
---
 drivers/tty/serdev/serdev-ttyport.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serdev/serdev-ttyport.c 
b/drivers/tty/serdev/serdev-ttyport.c
index 487c88f..2cfdf34 100644
--- a/drivers/tty/serdev/serdev-ttyport.c
+++ b/drivers/tty/serdev/serdev-ttyport.c
@@ -151,7 +151,7 @@ static unsigned int ttyport_set_baudrate(struct 
serdev_controller *ctrl, unsigne
 
/* tty_set_termios() return not checked as it is always 0 */
tty_set_termios(tty, &ktermios);
-   return speed;
+   return ktermios.c_ospeed;
 }
 
 static void ttyport_set_flow_control(struct serdev_controller *ctrl, bool 
enable)
-- 
2.1.4



[PATCH v5 03/17] net: qca_7k: Use BIT macro

2017-05-10 Thread Stefan Wahren
Use the BIT macro for the CONFIG and INT register values.

Signed-off-by: Stefan Wahren 
---
 drivers/net/ethernet/qualcomm/qca_7k.h | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/qualcomm/qca_7k.h 
b/drivers/net/ethernet/qualcomm/qca_7k.h
index 1cad851..4047f0a 100644
--- a/drivers/net/ethernet/qualcomm/qca_7k.h
+++ b/drivers/net/ethernet/qualcomm/qca_7k.h
@@ -54,15 +54,15 @@
 #define SPI_REG_ACTION_CTRL 0x1B00
 
 /*   SPI_CONFIG register definition; */
-#define QCASPI_SLAVE_RESET_BIT (1 << 6)
+#define QCASPI_SLAVE_RESET_BIT  BIT(6)
 
 /*   INTR_CAUSE/ENABLE register definition.  */
-#define SPI_INT_WRBUF_BELOW_WM (1 << 10)
-#define SPI_INT_CPU_ON (1 << 6)
-#define SPI_INT_ADDR_ERR   (1 << 3)
-#define SPI_INT_WRBUF_ERR  (1 << 2)
-#define SPI_INT_RDBUF_ERR  (1 << 1)
-#define SPI_INT_PKT_AVLBL  (1 << 0)
+#define SPI_INT_WRBUF_BELOW_WM  BIT(10)
+#define SPI_INT_CPU_ON  BIT(6)
+#define SPI_INT_ADDR_ERRBIT(3)
+#define SPI_INT_WRBUF_ERR   BIT(2)
+#define SPI_INT_RDBUF_ERR   BIT(1)
+#define SPI_INT_PKT_AVLBL   BIT(0)
 
 void qcaspi_spi_error(struct qcaspi *qca);
 int qcaspi_read_register(struct qcaspi *qca, u16 reg, u16 *result);
-- 
2.1.4



[PATCH v5 09/17] net: qualcomm: rename qca_framing.c to qca_7k_common.c

2017-05-10 Thread Stefan Wahren
As preparation for the upcoming UART driver we need a module
which contains common functions for both interfaces. The module
qca_framing is a good candidate but renaming to qca_7k_common would
make it clear.

Signed-off-by: Stefan Wahren 
---
 drivers/net/ethernet/qualcomm/Makefile   | 2 +-
 drivers/net/ethernet/qualcomm/{qca_framing.c => qca_7k_common.c} | 2 +-
 drivers/net/ethernet/qualcomm/{qca_framing.h => qca_7k_common.h} | 0
 drivers/net/ethernet/qualcomm/qca_spi.c  | 2 +-
 drivers/net/ethernet/qualcomm/qca_spi.h  | 2 +-
 5 files changed, 4 insertions(+), 4 deletions(-)
 rename drivers/net/ethernet/qualcomm/{qca_framing.c => qca_7k_common.c} (99%)
 rename drivers/net/ethernet/qualcomm/{qca_framing.h => qca_7k_common.h} (100%)

diff --git a/drivers/net/ethernet/qualcomm/Makefile 
b/drivers/net/ethernet/qualcomm/Makefile
index aacb0a5..5e17bf1 100644
--- a/drivers/net/ethernet/qualcomm/Makefile
+++ b/drivers/net/ethernet/qualcomm/Makefile
@@ -3,6 +3,6 @@
 #
 
 obj-$(CONFIG_QCA7000) += qcaspi.o
-qcaspi-objs := qca_spi.o qca_framing.o qca_7k.o qca_debug.o
+qcaspi-objs := qca_spi.o qca_7k_common.o qca_7k.o qca_debug.o
 
 obj-y += emac/
diff --git a/drivers/net/ethernet/qualcomm/qca_framing.c 
b/drivers/net/ethernet/qualcomm/qca_7k_common.c
similarity index 99%
rename from drivers/net/ethernet/qualcomm/qca_framing.c
rename to drivers/net/ethernet/qualcomm/qca_7k_common.c
index 2341f2b..6d17fbd 100644
--- a/drivers/net/ethernet/qualcomm/qca_framing.c
+++ b/drivers/net/ethernet/qualcomm/qca_7k_common.c
@@ -23,7 +23,7 @@
 
 #include 
 
-#include "qca_framing.h"
+#include "qca_7k_common.h"
 
 u16
 qcafrm_create_header(u8 *buf, u16 length)
diff --git a/drivers/net/ethernet/qualcomm/qca_framing.h 
b/drivers/net/ethernet/qualcomm/qca_7k_common.h
similarity index 100%
rename from drivers/net/ethernet/qualcomm/qca_framing.h
rename to drivers/net/ethernet/qualcomm/qca_7k_common.h
diff --git a/drivers/net/ethernet/qualcomm/qca_spi.c 
b/drivers/net/ethernet/qualcomm/qca_spi.c
index deec70f..2bc3ba4 100644
--- a/drivers/net/ethernet/qualcomm/qca_spi.c
+++ b/drivers/net/ethernet/qualcomm/qca_spi.c
@@ -43,8 +43,8 @@
 #include 
 
 #include "qca_7k.h"
+#include "qca_7k_common.h"
 #include "qca_debug.h"
-#include "qca_framing.h"
 #include "qca_spi.h"
 
 #define MAX_DMA_BURST_LEN 5000
diff --git a/drivers/net/ethernet/qualcomm/qca_spi.h 
b/drivers/net/ethernet/qualcomm/qca_spi.h
index 064853d..fc4beb1 100644
--- a/drivers/net/ethernet/qualcomm/qca_spi.h
+++ b/drivers/net/ethernet/qualcomm/qca_spi.h
@@ -32,7 +32,7 @@
 #include 
 #include 
 
-#include "qca_framing.h"
+#include "qca_7k_common.h"
 
 #define QCASPI_DRV_VERSION "0.2.7-i"
 #define QCASPI_DRV_NAME"qcaspi"
-- 
2.1.4



[PATCH v5 05/17] net: qualcomm: Improve readability of length defines

2017-05-10 Thread Stefan Wahren
In order to avoid mixing things up, make the MTU and frame length
defines easier to read.

Signed-off-by: Stefan Wahren 
---
 drivers/net/ethernet/qualcomm/qca_framing.c |  2 +-
 drivers/net/ethernet/qualcomm/qca_framing.h |  8 
 drivers/net/ethernet/qualcomm/qca_spi.c | 12 ++--
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/qualcomm/qca_framing.c 
b/drivers/net/ethernet/qualcomm/qca_framing.c
index faa924c..2341f2b 100644
--- a/drivers/net/ethernet/qualcomm/qca_framing.c
+++ b/drivers/net/ethernet/qualcomm/qca_framing.c
@@ -117,7 +117,7 @@ qcafrm_fsm_decode(struct qcafrm_handle *handle, u8 *buf, 
u16 buf_len, u8 recv_by
break;
case QCAFRM_WAIT_RSVD_BYTE2:
len = handle->offset;
-   if (len > buf_len || len < QCAFRM_ETHMINLEN) {
+   if (len > buf_len || len < QCAFRM_MIN_LEN) {
ret = QCAFRM_INVLEN;
handle->state = QCAFRM_HW_LEN0;
} else {
diff --git a/drivers/net/ethernet/qualcomm/qca_framing.h 
b/drivers/net/ethernet/qualcomm/qca_framing.h
index 8b385e6..5df7c65 100644
--- a/drivers/net/ethernet/qualcomm/qca_framing.h
+++ b/drivers/net/ethernet/qualcomm/qca_framing.h
@@ -44,12 +44,12 @@
 #define QCAFRM_INVFRAME (QCAFRM_ERR_BASE - 4)
 
 /* Min/Max Ethernet MTU: 46/1500 */
-#define QCAFRM_ETHMINMTU (ETH_ZLEN - ETH_HLEN)
-#define QCAFRM_ETHMAXMTU ETH_DATA_LEN
+#define QCAFRM_MIN_MTU (ETH_ZLEN - ETH_HLEN)
+#define QCAFRM_MAX_MTU ETH_DATA_LEN
 
 /* Min/Max frame lengths */
-#define QCAFRM_ETHMINLEN (QCAFRM_ETHMINMTU + ETH_HLEN)
-#define QCAFRM_ETHMAXLEN (QCAFRM_ETHMAXMTU + VLAN_ETH_HLEN)
+#define QCAFRM_MIN_LEN (QCAFRM_MIN_MTU + ETH_HLEN)
+#define QCAFRM_MAX_LEN (QCAFRM_MAX_MTU + VLAN_ETH_HLEN)
 
 /* QCA7K header len */
 #define QCAFRM_HEADER_LEN 8
diff --git a/drivers/net/ethernet/qualcomm/qca_spi.c 
b/drivers/net/ethernet/qualcomm/qca_spi.c
index 5c79612..a0dbb92 100644
--- a/drivers/net/ethernet/qualcomm/qca_spi.c
+++ b/drivers/net/ethernet/qualcomm/qca_spi.c
@@ -69,7 +69,7 @@ static int qcaspi_pluggable = QCASPI_PLUGGABLE_MIN;
 module_param(qcaspi_pluggable, int, 0);
 MODULE_PARM_DESC(qcaspi_pluggable, "Pluggable SPI connection (yes/no).");
 
-#define QCASPI_MTU QCAFRM_ETHMAXMTU
+#define QCASPI_MTU QCAFRM_MAX_MTU
 #define QCASPI_TX_TIMEOUT (1 * HZ)
 #define QCASPI_QCA7K_REBOOT_TIME_MS 1000
 
@@ -402,7 +402,7 @@ qcaspi_tx_ring_has_space(struct tx_ring *txr)
if (txr->skb[txr->tail])
return 0;
 
-   return (txr->size + QCAFRM_ETHMAXLEN < QCASPI_HW_BUF_LEN) ? 1 : 0;
+   return (txr->size + QCAFRM_MAX_LEN < QCASPI_HW_BUF_LEN) ? 1 : 0;
 }
 
 /*   Flush the tx ring. This function is only safe to
@@ -666,8 +666,8 @@ qcaspi_netdev_xmit(struct sk_buff *skb, struct net_device 
*dev)
struct sk_buff *tskb;
u8 pad_len = 0;
 
-   if (skb->len < QCAFRM_ETHMINLEN)
-   pad_len = QCAFRM_ETHMINLEN - skb->len;
+   if (skb->len < QCAFRM_MIN_LEN)
+   pad_len = QCAFRM_MIN_LEN - skb->len;
 
if (qca->txr.skb[qca->txr.tail]) {
netdev_warn(qca->net_dev, "queue was unexpectedly full!\n");
@@ -804,8 +804,8 @@ qcaspi_netdev_setup(struct net_device *dev)
dev->tx_queue_len = 100;
 
/* MTU range: 46 - 1500 */
-   dev->min_mtu = QCAFRM_ETHMINMTU;
-   dev->max_mtu = QCAFRM_ETHMAXMTU;
+   dev->min_mtu = QCAFRM_MIN_MTU;
+   dev->max_mtu = QCAFRM_MAX_MTU;
 
qca = netdev_priv(dev);
memset(qca, 0, sizeof(struct qcaspi));
-- 
2.1.4



[PATCH v5 06/17] net: qca_spi: remove QCASPI_MTU

2017-05-10 Thread Stefan Wahren
There is no need for an additional MTU define.

Signed-off-by: Stefan Wahren 
---
 drivers/net/ethernet/qualcomm/qca_spi.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/qualcomm/qca_spi.c 
b/drivers/net/ethernet/qualcomm/qca_spi.c
index a0dbb92..a239ac4 100644
--- a/drivers/net/ethernet/qualcomm/qca_spi.c
+++ b/drivers/net/ethernet/qualcomm/qca_spi.c
@@ -69,7 +69,6 @@ static int qcaspi_pluggable = QCASPI_PLUGGABLE_MIN;
 module_param(qcaspi_pluggable, int, 0);
 MODULE_PARM_DESC(qcaspi_pluggable, "Pluggable SPI connection (yes/no).");
 
-#define QCASPI_MTU QCAFRM_MAX_MTU
 #define QCASPI_TX_TIMEOUT (1 * HZ)
 #define QCASPI_QCA7K_REBOOT_TIME_MS 1000
 
@@ -745,7 +744,7 @@ qcaspi_netdev_init(struct net_device *dev)
 {
struct qcaspi *qca = netdev_priv(dev);
 
-   dev->mtu = QCASPI_MTU;
+   dev->mtu = QCAFRM_MAX_MTU;
dev->type = ARPHRD_ETHER;
qca->clkspeed = qcaspi_clkspeed;
qca->burst_len = qcaspi_burst_len;
-- 
2.1.4



Re: [PATCH] irq_bcm2836: Send event when onlining sleeping cores

2017-05-10 Thread Marc Zyngier
On Wed, May 10 2017 at  9:27:10 am BST, Phil Elwell  
wrote:
> On 10/05/2017 08:42, Marc Zyngier wrote:
>> On 09/05/17 20:02, Phil Elwell wrote:
>>> On 09/05/2017 19:53, Marc Zyngier wrote:
 On 09/05/17 19:52, Phil Elwell wrote:
> On 09/05/2017 19:14, Marc Zyngier wrote:
>> On 09/05/17 19:08, Eric Anholt wrote:
>>> Marc Zyngier  writes:
>>>
 On 09/05/17 17:59, Eric Anholt wrote:
> Phil Elwell  writes:
>
>> In order to reduce power consumption and bus traffic, it is sensible
>> for secondary cores to enter a low-power idle state when waiting to
>> be started. The wfe instruction causes a core to wait until an event
>> or interrupt arrives before continuing to the next instruction.
>> The sev instruction sends a wakeup event to the other cores, so call
>> it from bcm2836_smp_boot_secondary, the function that wakes up the
>> waiting cores during booting.
>>
>> It is harmless to use this patch without the corresponding change
>> adding wfe to the ARMv7/ARMv8-32 stubs, but if the stubs are updated
>> and this patch is not applied then the other cores will sleep 
>> forever.
>>
>> See: https://github.com/raspberrypi/linux/issues/1989
>>
>> Signed-off-by: Phil Elwell 
>> ---
>>  drivers/irqchip/irq-bcm2836.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/irqchip/irq-bcm2836.c 
>> b/drivers/irqchip/irq-bcm2836.c
>> index e10597c..6dccdf9 100644
>> --- a/drivers/irqchip/irq-bcm2836.c
>> +++ b/drivers/irqchip/irq-bcm2836.c
>> @@ -248,6 +248,9 @@ static int __init 
>> bcm2836_smp_boot_secondary(unsigned int cpu,
>>  writel(secondary_startup_phys,
>> intc.base + LOCAL_MAILBOX3_SET0 + 16 * cpu);
>>
>> +dsb(sy); /* Ensure write has completed before waking the other 
>> CPUs */
>> +sev();
>> +
>>  return 0;
>>  }
>
> This is also the behavior that the standard arm64 spin-table
> method has,
> which we unfortunately can't quite use.

 And why is that so? Why do you have to reinvent the wheel (and hide the
 cloned wheel in an interrupt controller driver)?

 That doesn't seem right to me.
>>>
>>> The armv8 stubs (firmware-supplied code in the low page that do the
>>> spinning) do actually implement arm64's spin-table method.  It's the
>>> armv7 stubs that use these registers in the irqchip instead of plain
>>> addresses in system memory.
>>
>> Let's put ARMv7 aside for the time being. If your firmware already
>> implements spin-tables, why don't you simply use that at least on arm64?
>
> We do.

 Obviously not the way it is intended if you have to duplicate the core
 architectural code in the interrupt controller driver, which couldn't
 care less.
>>>
>>> If we were using this method on arm64 then the other cores would not start 
>>> up
>>> because armstub8.S has always included a wfe. Nothing in the commit mentions
>>> arm64 - this is an ARCH=arm fix.
>> 
>> Thanks for the clarification, which you could have added to the commit
>> message.
>> 
>> The question still remains: why do we have CPU bring-up code in an
>> interrupt controller, instead of having it in the architecture code?
>> 
>> The RPi-2 is the *only* platform to have its SMP bringup code outside of
>> arch/arm, so the first course of action would be to move that code where
>> it belongs.
>
> You were CC'd on the commit (41f4988cc287e5f836d3f6620c9f900bc9b560e9) that
> introduced bcm2836_smp_boot_secondary - it seems strange to start objecting
> now.

Well, I'm far from being perfect. If I had noticed it, I'd have NACKed
it.

> Yes, I think it is odd that it didn't go into arch/arm/mach-bcm, but in
> the interests of making changes in small, independent steps, do you have a
> problem with this commit?

On its own, no. I'm just not keen on adding more unrelated stuff to this
file, so let's start with dealing with the original bug, and you can
then add this fix on top.

Thanks,

M.
-- 
Jazz is not dead, it just smell funny.


[PATCH v5 01/17] net: qualcomm: remove unnecessary includes

2017-05-10 Thread Stefan Wahren
Most of the includes in qca_7k.c are unnecessary so we better remove them.

Signed-off-by: Stefan Wahren 
---
 drivers/net/ethernet/qualcomm/qca_7k.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/drivers/net/ethernet/qualcomm/qca_7k.c 
b/drivers/net/ethernet/qualcomm/qca_7k.c
index f0066fb..557d53c 100644
--- a/drivers/net/ethernet/qualcomm/qca_7k.c
+++ b/drivers/net/ethernet/qualcomm/qca_7k.c
@@ -23,11 +23,7 @@
  *   kernel-based SPI device.
  */
 
-#include 
-#include 
-#include 
 #include 
-#include 
 
 #include "qca_7k.h"
 
-- 
2.1.4



Re: [PATCH v2 2/8] drm: Add drm_crtc_mode_valid()

2017-05-10 Thread Jose Abreu
Hi Daniel,


On 10-05-2017 08:59, Daniel Vetter wrote:
> On Tue, May 09, 2017 at 06:00:09PM +0100, Jose Abreu wrote:
>> Add a new helper to call crtc->mode_valid callback.
>>
>> Suggested-by: Ville Syrjälä 
>> Signed-off-by: Jose Abreu 
>> Cc: Carlos Palminha 
>> Cc: Alexey Brodkin 
>> Cc: Ville Syrjälä 
>> Cc: Daniel Vetter 
>> Cc: Dave Airlie 
>> Cc: Andrzej Hajda 
>> Cc: Archit Taneja 
>> ---
>>  drivers/gpu/drm/drm_crtc.c  | 22 ++
>>  drivers/gpu/drm/drm_crtc_internal.h |  3 +++
>>  2 files changed, 25 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
>> index 5af25ce..07ae705 100644
>> --- a/drivers/gpu/drm/drm_crtc.c
>> +++ b/drivers/gpu/drm/drm_crtc.c
>> @@ -38,6 +38,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -741,3 +742,24 @@ int drm_mode_crtc_set_obj_prop(struct drm_mode_object 
>> *obj,
>>  
>>  return ret;
>>  }
>> +
>> +/**
>> + * drm_crtc_mode_valid - call crtc->mode_valid callback, if any.
>> + * @crtc: crtc
>> + * @mode: mode to be validated
>> + *
>> + * If no mode_valid callback is available this will return MODE_OK.
>> + *
>> + * Returns: drm_mode_status Enum
>> + */
>> +enum drm_mode_status drm_crtc_mode_valid(struct drm_crtc *crtc,
>> + const struct drm_display_mode *mode)
>> +{
>> +const struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private;
> This is clearly a helper func, but you place it into the core and
> EXPORT_SYMBOL it. Imo this should be entirely internal to the helpers,
> perhaps just stuff them all into drm_probe_helpers.c? Header file would be
> drm_crtc_helper_internal.h.

Yeah, at first I was not planning to export it but then I saw
that drm_bridge_mode_fixup() is exported (and is in drm_bridge.c)
so it kind of felt right to place this in drm_crtc.c. Anyway, I
will move them to drm_probe_helpers.c, indeed there is no point
in exporting this.

>
> That also means no need for kernel-doc (only the driver api is formally
> documented) and then these 3 patches are so tiny it's better to squash
> them into the patch that adds their users.

Ok, will remove the docs but I think its better to have a single
patch which adds all the helpers so that I can use the
suggested-by tag. Thanks!

Best regards,
Jose Miguel Abreu

>
> Thanks, Daniel
>> +
>> +if (!crtc_funcs || !crtc_funcs->mode_valid)
>> +return MODE_OK;
>> +
>> +return crtc_funcs->mode_valid(crtc, mode);
>> +}
>> +EXPORT_SYMBOL(drm_crtc_mode_valid);
>> diff --git a/drivers/gpu/drm/drm_crtc_internal.h 
>> b/drivers/gpu/drm/drm_crtc_internal.h
>> index d077c54..3800abd 100644
>> --- a/drivers/gpu/drm/drm_crtc_internal.h
>> +++ b/drivers/gpu/drm/drm_crtc_internal.h
>> @@ -45,6 +45,9 @@ int drm_crtc_check_viewport(const struct drm_crtc *crtc,
>>  
>>  struct dma_fence *drm_crtc_create_fence(struct drm_crtc *crtc);
>>  
>> +enum drm_mode_status drm_crtc_mode_valid(struct drm_crtc *crtc,
>> + const struct drm_display_mode *mode);
>> +
>>  /* IOCTLs */
>>  int drm_mode_getcrtc(struct drm_device *dev,
>>   void *data, struct drm_file *file_priv);
>> -- 
>> 1.9.1
>>
>>



[PATCH v5 15/17] dt-bindings: qca7000: append UART interface to binding

2017-05-10 Thread Stefan Wahren
This merges the serdev binding for the QCA7000 UART driver (Ethernet over
UART) into the existing document.

Signed-off-by: Stefan Wahren 
---
 .../devicetree/bindings/net/qca-qca7000.txt| 32 ++
 1 file changed, 32 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/qca-qca7000.txt 
b/Documentation/devicetree/bindings/net/qca-qca7000.txt
index a37f656..08364c3 100644
--- a/Documentation/devicetree/bindings/net/qca-qca7000.txt
+++ b/Documentation/devicetree/bindings/net/qca-qca7000.txt
@@ -54,3 +54,35 @@ ssp2: spi@80014000 {
local-mac-address = [ A0 B0 C0 D0 E0 F0 ];
};
 };
+
+(b) Ethernet over UART
+
+In order to use the QCA7000 as UART slave it must be defined as a child of a
+UART master in the device tree. It is possible to preconfigure the UART
+settings of the QCA7000 firmware, but it's not possible to change them during
+runtime.
+
+Required properties:
+- compatible: Should be "qca,qca7000-uart"
+
+Optional properties:
+- local-mac-address : see ./ethernet.txt
+- current-speed : current baud rate of QCA7000 which defaults to 115200
+ if absent, see also ../serial/slave-device.txt
+
+UART Example:
+
+/* Freescale i.MX28 UART */
+auart0: serial@8006a000 {
+   compatible = "fsl,imx28-auart", "fsl,imx23-auart";
+   reg = <0x8006a000 0x2000>;
+   pinctrl-names = "default";
+   pinctrl-0 = <&auart0_2pins_a>;
+   status = "okay";
+
+   qca7000: ethernet {
+   compatible = "qca,qca7000-uart";
+   local-mac-address = [ A0 B0 C0 D0 E0 F0 ];
+   current-speed = <38400>;
+   };
+};
-- 
2.1.4



[PATCH v5 14/17] dt-bindings: slave-device: add current-speed property

2017-05-10 Thread Stefan Wahren
This adds a new DT property to define the current baud rate of the
slave device.

Signed-off-by: Stefan Wahren 
---
 Documentation/devicetree/bindings/serial/slave-device.txt | 9 +
 1 file changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/serial/slave-device.txt 
b/Documentation/devicetree/bindings/serial/slave-device.txt
index f660379..40110e0 100644
--- a/Documentation/devicetree/bindings/serial/slave-device.txt
+++ b/Documentation/devicetree/bindings/serial/slave-device.txt
@@ -21,6 +21,15 @@ Optional Properties:
  can support. For example, a particular board has some signal
  quality issue or the host processor can't support higher
  baud rates.
+- current-speed: The current baud rate the device operates at. This 
should
+ only be present in case a driver has no chance to know
+ the baud rate of the slave device.
+ Examples:
+   * device supports auto-baud
+   * the rate is setup by a bootloader and there is no
+ way to reset the device
+   * device baud rate is configured by its firmware but
+ there is no way to request the actual settings
 
 Example:
 
-- 
2.1.4



Re: RFC v2: post-init-read-only protection for data allocated dynamically

2017-05-10 Thread Igor Stoppa
On 10/05/17 11:05, Michal Hocko wrote:
> On Fri 05-05-17 13:42:27, Igor Stoppa wrote:

[...]

>> ... in the case I have in mind, I have various, heterogeneous chunks of
>> data, coming from various subsystems, not necessarily page aligned.
>> And, even if they were page aligned, most likely they would be far
>> smaller than a page, even a 4k page.
> 
> This aspect of various sizes makes the SLAB allocator not optimal
> because it operates on caches (pools of pages) which manage objects of
> the same size.

Indeed, that's why I wasn't too excited about caches, but probably I was
not able to explain sufficiently well why in the RFC :-/

> You could use the maximum size of all objects and waste
> some memory but you would have to know this max in advance which would
> make this approach less practical. You could create more caches of
> course but that still requires to know those sizes in advance.

Yes, and even generic per-architecture or even per board profiling
wouldn't necessarily do much good: taking SE Linux as example, one could
have two identical boards with almost identical binaries installed, only
differing in the rules/policy.
That difference alone could easily lead to very different size
requirements for the sealable page pool.

> So it smells like a dedicated allocator which operates on a pool of
> pages might be a better option in the end.

ok

> This depends on what you
> expect from the allocator. NUMA awareness? Very effective hotpath? Very
> good fragmentation avoidance? CPU cache awareness? Special alignment
> requirements? Reasonable free()? Etc...

>From the perspective of selling the feature to as many subsystems as
possible, I'd say that as primary requirement, it shouldn't affect
runtime performance.
I suppose (but it's just my guess) that trading milliseconds-scale
boot-time slowdown, for additional integrity is acceptable in the vast
majority of cases.

The only alignment requirements that I can think of, are coming from the
MMU capability of dealing only with physical pages, when it comes to
write-protect them.

> To me it seems that this being an initialization mostly thingy a simple
> allocator which manages a pool of pages (one set of sealed and one for
> allocations) 

Shouldn't also the set of pages used for keeping track of the others be
sealed? Once one is ro, also the other should not change.

> and which only appends new objects as they fit to unsealed
> pages would be sufficient for starter.

Any "free" that might happen during the initialization transient, would
actually result in an untracked gap, right?

What about the size of the pool of pages?
No predefined size, instead request a new page, when the memory
remaining from the page currently in use is not enough to fit the latest
allocation request?

There are also two aspect we discussed earlier:

- livepatch: how to deal with it? Identify the page it wants to modify
and temporarily un-protect it?

- modules: unloading and reloading modules will eventually lead to
permanently lost pages, in increasing number.
Loading/unloading repeatedly the same module is probably not so common,
with a major exception being USB, where almost anything can show up.
And disappear.
This seems like a major showstopper for the linear allocator you propose.

My reasoning in pursuing the kmalloc approach was that it is already
equipped with mechanisms for dealing with these sort of cases, where
memory can be fragmented.
I also wouldn't risk introducing bugs with my homebrew allocator ...

The initial thought was that there could be a master toggle to
seal/unseal all the memory affected.

But you were not too excited, iirc :-D
Alternatively, kmalloc could be enhanced to unseal only the pages it
wants to modify.

I don't think much can be done for data that is placed together, in the
same page with something that needs to be altered.
But what is outside of that page could still enjoy the protection from
the seal.

--
igor


[PATCH v5 17/17] net: qualcomm: add QCA7000 UART driver

2017-05-10 Thread Stefan Wahren
This patch adds the Ethernet over UART driver for the
Qualcomm QCA7000 HomePlug GreenPHY.

Signed-off-by: Stefan Wahren 
---
 drivers/net/ethernet/qualcomm/Kconfig |  16 +
 drivers/net/ethernet/qualcomm/Makefile|   2 +
 drivers/net/ethernet/qualcomm/qca_7k_common.h |   6 +
 drivers/net/ethernet/qualcomm/qca_uart.c  | 423 ++
 4 files changed, 447 insertions(+)
 create mode 100644 drivers/net/ethernet/qualcomm/qca_uart.c

diff --git a/drivers/net/ethernet/qualcomm/Kconfig 
b/drivers/net/ethernet/qualcomm/Kconfig
index b4c369d..877675a 100644
--- a/drivers/net/ethernet/qualcomm/Kconfig
+++ b/drivers/net/ethernet/qualcomm/Kconfig
@@ -30,6 +30,22 @@ config QCA7000_SPI
  To compile this driver as a module, choose M here. The module
  will be called qcaspi.
 
+config QCA7000_UART
+   tristate "Qualcomm Atheros QCA7000 UART support"
+   select QCA7000
+   depends on SERIAL_DEV_BUS && OF
+   ---help---
+ This UART protocol driver supports the Qualcomm Atheros QCA7000.
+
+ Currently the driver assumes these device UART settings:
+   Data bits: 8
+   Parity: None
+   Stop bits: 1
+   Flow control: None
+
+ To compile this driver as a module, choose M here. The module
+ will be called qcauart.
+
 config QCOM_EMAC
tristate "Qualcomm Technologies, Inc. EMAC Gigabit Ethernet support"
depends on HAS_DMA && HAS_IOMEM
diff --git a/drivers/net/ethernet/qualcomm/Makefile 
b/drivers/net/ethernet/qualcomm/Makefile
index 65556ca..92fa7c4 100644
--- a/drivers/net/ethernet/qualcomm/Makefile
+++ b/drivers/net/ethernet/qualcomm/Makefile
@@ -5,5 +5,7 @@
 obj-$(CONFIG_QCA7000) += qca_7k_common.o
 obj-$(CONFIG_QCA7000_SPI) += qcaspi.o
 qcaspi-objs := qca_7k.o qca_debug.o qca_spi.o
+obj-$(CONFIG_QCA7000_UART) += qcauart.o
+qcauart-objs := qca_uart.o
 
 obj-y += emac/
diff --git a/drivers/net/ethernet/qualcomm/qca_7k_common.h 
b/drivers/net/ethernet/qualcomm/qca_7k_common.h
index 07bdd6c..928554f 100644
--- a/drivers/net/ethernet/qualcomm/qca_7k_common.h
+++ b/drivers/net/ethernet/qualcomm/qca_7k_common.h
@@ -122,6 +122,12 @@ static inline void qcafrm_fsm_init_spi(struct 
qcafrm_handle *handle)
handle->state = handle->init;
 }
 
+static inline void qcafrm_fsm_init_uart(struct qcafrm_handle *handle)
+{
+   handle->init = QCAFRM_WAIT_AA1;
+   handle->state = handle->init;
+}
+
 /*   Gather received bytes and try to extract a full Ethernet frame
  *   by following a simple state machine.
  *
diff --git a/drivers/net/ethernet/qualcomm/qca_uart.c 
b/drivers/net/ethernet/qualcomm/qca_uart.c
new file mode 100644
index 000..1f6514c
--- /dev/null
+++ b/drivers/net/ethernet/qualcomm/qca_uart.c
@@ -0,0 +1,423 @@
+/*
+ *   Copyright (c) 2011, 2012, Qualcomm Atheros Communications Inc.
+ *   Copyright (c) 2017, I2SE GmbH
+ *
+ *   Permission to use, copy, modify, and/or distribute this software
+ *   for any purpose with or without fee is hereby granted, provided
+ *   that the above copyright notice and this permission notice appear
+ *   in all copies.
+ *
+ *   THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL
+ *   WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED
+ *   WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
+ *   THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, INDIRECT, OR
+ *   CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+ *   LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+ *   NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
+ *   CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+/*   This module implements the Qualcomm Atheros UART protocol for
+ *   kernel-based UART device; it is essentially an Ethernet-to-UART
+ *   serial converter;
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "qca_7k_common.h"
+
+#define QCAUART_DRV_VERSION "0.1.0"
+#define QCAUART_DRV_NAME "qcauart"
+#define QCAUART_TX_TIMEOUT (1 * HZ)
+
+struct qcauart {
+   struct net_device *net_dev;
+   spinlock_t lock;/* transmit lock */
+   struct work_struct tx_work; /* Flushes transmit buffer   */
+
+   struct serdev_device *serdev;
+   struct qcafrm_handle frm_handle;
+   struct sk_buff *rx_skb;
+
+   unsigned char *tx_head; /* pointer to next XMIT byte */
+   int tx_left;/* bytes left in XMIT queue  */
+   unsigned char *tx_buffer;
+};
+
+static int
+qca_tty_receive(struct serdev_device *serdev, const unsigned char *data,
+   size_t count)
+{
+   struct qcauart *qca = serdev_device_get_drvdata(serdev);
+   struct net_device *netdev = qca->net_dev;
+   struct net_device_stats *n_stats = &netdev->stats;
+   size_t 

[PATCH v5 12/17] dt-bindings: qca7000-spi: Rework binding

2017-05-10 Thread Stefan Wahren
In preparation for the QCA7000 UART binding rework the binding document.

Signed-off-by: Stefan Wahren 
---
 .../devicetree/bindings/net/qca-qca7000-spi.txt| 49 +-
 1 file changed, 29 insertions(+), 20 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/qca-qca7000-spi.txt 
b/Documentation/devicetree/bindings/net/qca-qca7000-spi.txt
index c74989c..a37f656 100644
--- a/Documentation/devicetree/bindings/net/qca-qca7000-spi.txt
+++ b/Documentation/devicetree/bindings/net/qca-qca7000-spi.txt
@@ -1,29 +1,38 @@
-* Qualcomm QCA7000 (Ethernet over SPI protocol)
+* Qualcomm QCA7000
 
-Note: The QCA7000 is useable as a SPI device. In this case it must be defined
-as a child of a SPI master in the device tree.
+The QCA7000 is a serial-to-powerline bridge with a host interface which could
+be configured either as SPI or UART slave. This configuration is done by
+the QCA7000 firmware.
+
+(a) Ethernet over SPI
+
+In order to use the QCA7000 as SPI device it must be defined as a child of a
+SPI master in the device tree.
 
 Required properties:
-- compatible : Should be "qca,qca7000"
-- reg : Should specify the SPI chip select
-- interrupts : The first cell should specify the index of the source interrupt
-  and the second cell should specify the trigger type as rising edge
-- spi-cpha : Must be set
-- spi-cpol: Must be set
+- compatible   : Should be "qca,qca7000"
+- reg  : Should specify the SPI chip select
+- interrupts   : The first cell should specify the index of the source
+ interrupt and the second cell should specify the trigger
+ type as rising edge
+- spi-cpha : Must be set
+- spi-cpol : Must be set
 
 Optional properties:
-- interrupt-parent : Specify the pHandle of the source interrupt
+- interrupt-parent  : Specify the pHandle of the source interrupt
 - spi-max-frequency : Maximum frequency of the SPI bus the chip can operate at.
-  Numbers smaller than 100 or greater than 1600 are invalid. Missing
-  the property will set the SPI frequency to 800 Hertz.
-- local-mac-address: 6 bytes, MAC address
-- qca,legacy-mode : Set the SPI data transfer of the QCA7000 to legacy mode.
-  In this mode the SPI master must toggle the chip select between each data
-  word. In burst mode these gaps aren't necessary, which is faster.
-  This setting depends on how the QCA7000 is setup via GPIO pin strapping.
-  If the property is missing the driver defaults to burst mode.
-
-Example:
+ Numbers smaller than 100 or greater than 1600
+ are invalid. Missing the property will set the SPI
+ frequency to 800 Hertz.
+- local-mac-address : see ./ethernet.txt
+- qca,legacy-mode   : Set the SPI data transfer of the QCA7000 to legacy mode.
+ In this mode the SPI master must toggle the chip select
+ between each data word. In burst mode these gaps aren't
+ necessary, which is faster. This setting depends on how
+ the QCA7000 is setup via GPIO pin strapping. If the
+ property is missing the driver defaults to burst mode.
+
+SPI Example:
 
 /* Freescale i.MX28 SPI master*/
 ssp2: spi@80014000 {
-- 
2.1.4



Re: [PATCH v2 5/8] drm: Use new mode_valid() helpers in connector probe helper

2017-05-10 Thread Jose Abreu
Hi Daniel,


On 10-05-2017 09:01, Daniel Vetter wrote:
> On Tue, May 09, 2017 at 06:00:12PM +0100, Jose Abreu wrote:
>> This changes the connector probe helper function to use the new
>> encoder->mode_valid() and crtc->mode_valid() helper callbacks to
>> validate the modes.
>>
>> The new callbacks are optional so the behaviour remains the same
>> if they are not implemented. If they are, then the code loops
>> through all the connector's encodersXcrtcs and calls the
>> callback.
>>
>> If at least a valid encoderXcrtc combination is found which
>> accepts the mode then the function returns MODE_OK.
>>
>> Signed-off-by: Jose Abreu 
>> Cc: Carlos Palminha 
>> Cc: Alexey Brodkin 
>> Cc: Ville Syrjälä 
>> Cc: Daniel Vetter 
>> Cc: Dave Airlie 
>> Cc: Andrzej Hajda 
>> Cc: Archit Taneja 
>> ---
>>
>> Changes v1->v2:
>>  - Use new helpers suggested by Ville
>>  - Change documentation (Daniel)
>>
>>  drivers/gpu/drm/drm_probe_helper.c | 60 
>> --
>>  1 file changed, 57 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_probe_helper.c 
>> b/drivers/gpu/drm/drm_probe_helper.c
>> index 1b0c14a..de47413 100644
>> --- a/drivers/gpu/drm/drm_probe_helper.c
>> +++ b/drivers/gpu/drm/drm_probe_helper.c
>> @@ -39,6 +39,8 @@
>>  #include 
>>  #include 
>>  
>> +#include "drm_crtc_internal.h"
>> +
>>  /**
>>   * DOC: output probing helper overview
>>   *
>> @@ -80,6 +82,54 @@
>>  return MODE_OK;
>>  }
>>  
>> +static enum drm_mode_status
>> +drm_mode_validate_connector(struct drm_connector *connector,
>> +struct drm_display_mode *mode)
>> +{
>> +struct drm_device *dev = connector->dev;
>> +uint32_t *ids = connector->encoder_ids;
>> +enum drm_mode_status ret = MODE_OK;
>> +unsigned int i;
>> +
>> +/* Step 1: Validate against connector */
>> +ret = drm_connector_mode_valid(connector, mode);
>> +if (ret != MODE_OK)
>> +return ret;
>> +
>> +/* Step 2: Validate against encoders and crtcs */
>> +for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
>> +struct drm_encoder *encoder = drm_encoder_find(dev, ids[i]);
>> +struct drm_crtc *crtc;
>> +
>> +if (!encoder)
>> +continue;
>> +
>> +ret = drm_encoder_mode_valid(encoder, mode);
>> +if (ret != MODE_OK) {
>> +/* No point in continuing for crtc check as this encoder
>> + * will not accept the mode anyway. If all encoders
>> + * reject the mode then, at exit, ret will not be
>> + * MODE_OK. */
>> +continue;
>> +}
> One thing I've forgotten the last time around: Please also check
> bridge->mode_valid here. The encoder->bridge mapping is fixed.

Ok, will add in next version.

Best regards,
Jose Miguel Abreu

>
> Otherwise I think this looks good.
> -Daniel
>
>> +
>> +drm_for_each_crtc(crtc, dev) {
>> +if (!drm_encoder_crtc_ok(encoder, crtc))
>> +continue;
>> +
>> +ret = drm_crtc_mode_valid(crtc, mode);
>> +if (ret == MODE_OK) {
>> +/* If we get to this point there is at least
>> + * one combination of encoder+crtc that works
>> + * for this mode. Lets return now. */
>> +return ret;
>> +}
>> +}
>> +}
>> +
>> +return ret;
>> +}
>> +
>>  static int drm_helper_probe_add_cmdline_mode(struct drm_connector 
>> *connector)
>>  {
>>  struct drm_cmdline_mode *cmdline_mode;
>> @@ -284,7 +334,11 @@ void drm_kms_helper_poll_enable(struct drm_device *dev)
>>   *- drm_mode_validate_flag() checks the modes against basic connector
>>   *  capabilities (interlace_allowed,doublescan_allowed,stereo_allowed)
>>   *- the optional &drm_connector_helper_funcs.mode_valid helper can 
>> perform
>> - *  driver and/or hardware specific checks
>> + *  driver and/or sink specific checks
>> + *- the optional &drm_crtc_helper_funcs.mode_valid and
>> + *  &drm_encoder_helper_funcs.mode_valid helpers can perform driver 
>> and/or
>> + *  source specific checks which are also enforced by the modeset/atomic
>> + *  helpers
>>   *
>>   * 5. Any mode whose status is not OK is pruned from the connector's modes 
>> list,
>>   *accompanied by a debug message indicating the reason for the mode's
>> @@ -428,8 +482,8 @@ int drm_helper_probe_single_connector_modes(struct 
>> drm_connector *connector,
>>  if (mode->status == MODE_OK)
>>  mode->status = drm_mode_validate_flag(mode, mode_flags);
>>  
>> -if (mode->status == MODE_OK && connector_funcs->mode_valid)
>> -mode->status = connector_funcs->mode_valid(connector,
>> +if (mode->status == MODE_OK

[PATCH v5 04/17] net: qualcomm: use net_device_ops instead of direct call

2017-05-10 Thread Stefan Wahren
There is no need to export qcaspi_netdev_open and qcaspi_netdev_close
because they are also accessible via the net_device_ops.

Signed-off-by: Stefan Wahren 
---
 drivers/net/ethernet/qualcomm/qca_debug.c | 5 +++--
 drivers/net/ethernet/qualcomm/qca_spi.c   | 4 ++--
 drivers/net/ethernet/qualcomm/qca_spi.h   | 3 ---
 3 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/qualcomm/qca_debug.c 
b/drivers/net/ethernet/qualcomm/qca_debug.c
index d145df9..92b6be9 100644
--- a/drivers/net/ethernet/qualcomm/qca_debug.c
+++ b/drivers/net/ethernet/qualcomm/qca_debug.c
@@ -275,6 +275,7 @@ qcaspi_get_ringparam(struct net_device *dev, struct 
ethtool_ringparam *ring)
 static int
 qcaspi_set_ringparam(struct net_device *dev, struct ethtool_ringparam *ring)
 {
+   const struct net_device_ops *ops = dev->netdev_ops;
struct qcaspi *qca = netdev_priv(dev);
 
if ((ring->rx_pending) ||
@@ -283,13 +284,13 @@ qcaspi_set_ringparam(struct net_device *dev, struct 
ethtool_ringparam *ring)
return -EINVAL;
 
if (netif_running(dev))
-   qcaspi_netdev_close(dev);
+   ops->ndo_stop(dev);
 
qca->txr.count = max_t(u32, ring->tx_pending, TX_RING_MIN_LEN);
qca->txr.count = min_t(u16, qca->txr.count, TX_RING_MAX_LEN);
 
if (netif_running(dev))
-   qcaspi_netdev_open(dev);
+   ops->ndo_open(dev);
 
return 0;
 }
diff --git a/drivers/net/ethernet/qualcomm/qca_spi.c 
b/drivers/net/ethernet/qualcomm/qca_spi.c
index 8590109..5c79612 100644
--- a/drivers/net/ethernet/qualcomm/qca_spi.c
+++ b/drivers/net/ethernet/qualcomm/qca_spi.c
@@ -602,7 +602,7 @@ qcaspi_intr_handler(int irq, void *data)
return IRQ_HANDLED;
 }
 
-int
+static int
 qcaspi_netdev_open(struct net_device *dev)
 {
struct qcaspi *qca = netdev_priv(dev);
@@ -639,7 +639,7 @@ qcaspi_netdev_open(struct net_device *dev)
return 0;
 }
 
-int
+static int
 qcaspi_netdev_close(struct net_device *dev)
 {
struct qcaspi *qca = netdev_priv(dev);
diff --git a/drivers/net/ethernet/qualcomm/qca_spi.h 
b/drivers/net/ethernet/qualcomm/qca_spi.h
index 6e31a0e..064853d 100644
--- a/drivers/net/ethernet/qualcomm/qca_spi.h
+++ b/drivers/net/ethernet/qualcomm/qca_spi.h
@@ -108,7 +108,4 @@ struct qcaspi {
u16 burst_len;
 };
 
-int qcaspi_netdev_open(struct net_device *dev);
-int qcaspi_netdev_close(struct net_device *dev);
-
 #endif /* _QCA_SPI_H */
-- 
2.1.4



[PATCH v5 13/17] dt-bindings: qca7000: rename binding

2017-05-10 Thread Stefan Wahren
Before we can merge the QCA7000 UART binding the document needs to be
renamed.

Signed-off-by: Stefan Wahren 
---
 .../devicetree/bindings/net/{qca-qca7000-spi.txt => qca-qca7000.txt}  | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename Documentation/devicetree/bindings/net/{qca-qca7000-spi.txt => 
qca-qca7000.txt} (100%)

diff --git a/Documentation/devicetree/bindings/net/qca-qca7000-spi.txt 
b/Documentation/devicetree/bindings/net/qca-qca7000.txt
similarity index 100%
rename from Documentation/devicetree/bindings/net/qca-qca7000-spi.txt
rename to Documentation/devicetree/bindings/net/qca-qca7000.txt
-- 
2.1.4



[PATCH v5 10/17] net: qualcomm: prepare frame decoding for UART driver

2017-05-10 Thread Stefan Wahren
Unfortunately the frame format is not exactly identical between SPI
and UART. In case of SPI there is an additional HW length at the
beginning. So store the initial state to make the decoding state machine
more flexible and easy to extend for UART support.

Signed-off-by: Stefan Wahren 
---
 drivers/net/ethernet/qualcomm/qca_7k_common.c | 12 ++--
 drivers/net/ethernet/qualcomm/qca_7k_common.h |  8 ++--
 drivers/net/ethernet/qualcomm/qca_spi.c   |  2 +-
 3 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/qualcomm/qca_7k_common.c 
b/drivers/net/ethernet/qualcomm/qca_7k_common.c
index 6d17fbd..0d3daa9 100644
--- a/drivers/net/ethernet/qualcomm/qca_7k_common.c
+++ b/drivers/net/ethernet/qualcomm/qca_7k_common.c
@@ -83,7 +83,7 @@ qcafrm_fsm_decode(struct qcafrm_handle *handle, u8 *buf, u16 
buf_len, u8 recv_by
 
if (recv_byte != 0x00) {
/* first two bytes of length must be 0 */
-   handle->state = QCAFRM_HW_LEN0;
+   handle->state = handle->init;
}
break;
case QCAFRM_HW_LEN2:
@@ -97,7 +97,7 @@ qcafrm_fsm_decode(struct qcafrm_handle *handle, u8 *buf, u16 
buf_len, u8 recv_by
case QCAFRM_WAIT_AA4:
if (recv_byte != 0xAA) {
ret = QCAFRM_NOHEAD;
-   handle->state = QCAFRM_HW_LEN0;
+   handle->state = handle->init;
} else {
handle->state--;
}
@@ -119,7 +119,7 @@ qcafrm_fsm_decode(struct qcafrm_handle *handle, u8 *buf, 
u16 buf_len, u8 recv_by
len = handle->offset;
if (len > buf_len || len < QCAFRM_MIN_LEN) {
ret = QCAFRM_INVLEN;
-   handle->state = QCAFRM_HW_LEN0;
+   handle->state = handle->init;
} else {
handle->state = (enum qcafrm_state)(len + 1);
/* Remaining number of bytes. */
@@ -135,7 +135,7 @@ qcafrm_fsm_decode(struct qcafrm_handle *handle, u8 *buf, 
u16 buf_len, u8 recv_by
case QCAFRM_WAIT_551:
if (recv_byte != 0x55) {
ret = QCAFRM_NOTAIL;
-   handle->state = QCAFRM_HW_LEN0;
+   handle->state = handle->init;
} else {
handle->state = QCAFRM_WAIT_552;
}
@@ -143,11 +143,11 @@ qcafrm_fsm_decode(struct qcafrm_handle *handle, u8 *buf, 
u16 buf_len, u8 recv_by
case QCAFRM_WAIT_552:
if (recv_byte != 0x55) {
ret = QCAFRM_NOTAIL;
-   handle->state = QCAFRM_HW_LEN0;
+   handle->state = handle->init;
} else {
ret = handle->offset;
/* Frame is fully received. */
-   handle->state = QCAFRM_HW_LEN0;
+   handle->state = handle->init;
}
break;
}
diff --git a/drivers/net/ethernet/qualcomm/qca_7k_common.h 
b/drivers/net/ethernet/qualcomm/qca_7k_common.h
index 5df7c65..07bdd6c 100644
--- a/drivers/net/ethernet/qualcomm/qca_7k_common.h
+++ b/drivers/net/ethernet/qualcomm/qca_7k_common.h
@@ -61,6 +61,7 @@
 #define QCAFRM_ERR_BASE -1000
 
 enum qcafrm_state {
+   /* HW length is only available on SPI */
QCAFRM_HW_LEN0 = 0x8000,
QCAFRM_HW_LEN1 = QCAFRM_HW_LEN0 - 1,
QCAFRM_HW_LEN2 = QCAFRM_HW_LEN1 - 1,
@@ -101,6 +102,8 @@ enum qcafrm_state {
 struct qcafrm_handle {
/*  Current decoding state */
enum qcafrm_state state;
+   /* Initial state depends on connection type */
+   enum qcafrm_state init;
 
/* Offset in buffer (borrowed for length too) */
u16 offset;
@@ -113,9 +116,10 @@ u16 qcafrm_create_header(u8 *buf, u16 len);
 
 u16 qcafrm_create_footer(u8 *buf);
 
-static inline void qcafrm_fsm_init(struct qcafrm_handle *handle)
+static inline void qcafrm_fsm_init_spi(struct qcafrm_handle *handle)
 {
-   handle->state = QCAFRM_HW_LEN0;
+   handle->init = QCAFRM_HW_LEN0;
+   handle->state = handle->init;
 }
 
 /*   Gather received bytes and try to extract a full Ethernet frame
diff --git a/drivers/net/ethernet/qualcomm/qca_spi.c 
b/drivers/net/ethernet/qualcomm/qca_spi.c
index 2bc3ba4..b7073a9 100644
--- a/drivers/net/ethernet/qualcomm/qca_spi.c
+++ b/drivers/net/ethernet/qualcomm/qca_spi.c
@@ -637,7 +637,7 @@ qcaspi_netdev_open(struct net_device *dev)
qca->intr_req = 1;
qca->intr_svc = 0;
qca->sync = QCASPI_SYNC_UNKNOWN;
-   qcafrm_fsm_init(&qca->frm_handle);
+   qcafrm_fsm_init_spi(&qca->frm_handle);
 
qca->spi_thread = kthread_run((void *)qcaspi_spi_thread,
  qca, "%s", dev->name);
-- 
2.1.4



[PATCH v5 08/17] net: qca_spi: Clarify MODULE_DESCRIPTION

2017-05-10 Thread Stefan Wahren
Since this driver is specific to the QCA7000, we should make the module
description more precisely.

Signed-off-by: Stefan Wahren 
---
 drivers/net/ethernet/qualcomm/qca_spi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/qualcomm/qca_spi.c 
b/drivers/net/ethernet/qualcomm/qca_spi.c
index c727357..deec70f 100644
--- a/drivers/net/ethernet/qualcomm/qca_spi.c
+++ b/drivers/net/ethernet/qualcomm/qca_spi.c
@@ -997,7 +997,7 @@ static struct spi_driver qca_spi_driver = {
 };
 module_spi_driver(qca_spi_driver);
 
-MODULE_DESCRIPTION("Qualcomm Atheros SPI Driver");
+MODULE_DESCRIPTION("Qualcomm Atheros QCA7000 SPI Driver");
 MODULE_AUTHOR("Qualcomm Atheros Communications");
 MODULE_AUTHOR("Stefan Wahren ");
 MODULE_LICENSE("Dual BSD/GPL");
-- 
2.1.4



[PATCH v5 00/17] net: qualcomm: add QCA7000 UART driver

2017-05-10 Thread Stefan Wahren
The Qualcomm QCA7000 HomePlug GreenPHY supports two interfaces:
UART and SPI. This patch series adds the missing support for UART.

This driver based on the Qualcomm code [1], but contains some changes:
* use random MAC address per default
* use net_device_stats from device
* share frame decoding between SPI and UART driver
* improve error handling
* reimplement tty_wakeup with work queue (based on slcan)
* use new serial device bus instead of ldisc

The patches 1 - 3 are just for clean up and are not related to
the UART support. Patches 4 - 15 prepare the existing QCA7000
code for UART support. Patch 16 is a improvement for serial device
bus. Patch 17 contains the new driver.

Cherry picking of the dt-bindings and serdev patch is okay. The
UART driver functionally (not compile) depends on this unmerged
patch [2].

The code itself has been tested on a Freescale i.MX28 board and
a Raspberry Pi Zero.

Changes in v5:
  * rebase to current linux-next
  * fix alignment issues in rx path
  * fix buffer overrun with big ethernet frames
  * fix init of UART decoding fsm
  * add device UART settings to Kconfig help
  * add current-speed to slave-device binding
  * merge SPI and UART binding document
  * rename qca_common to qca_7k_common
  * drop patch for retrieving UART settings
  * several cleanups

Changes in v4:
  * rebase to current linux-next
  * use parameter -M for git format-patch
  * change order of local variables where possible
  * implement basic serdev support (without hardware flow control)

Changes in v3:
  * rebase to current net-next

Changes in v2:
  * fix build issue by using netif_trans_update() and dev_trans_start()

[1] - https://github.com/IoE/qca7000
[2] - http://marc.info/?l=linux-serial&m=149338017301309&w=2

Stefan Wahren (17):
  net: qualcomm: remove unnecessary includes
  net: qca_framing: use u16 for frame offset
  net: qca_7k: Use BIT macro
  net: qualcomm: use net_device_ops instead of direct call
  net: qualcomm: Improve readability of length defines
  net: qca_spi: remove QCASPI_MTU
  net: qualcomm: move qcaspi_tx_cmd to qca_spi.c
  net: qca_spi: Clarify MODULE_DESCRIPTION
  net: qualcomm: rename qca_framing.c to qca_7k_common.c
  net: qualcomm: prepare frame decoding for UART driver
  net: qualcomm: make qca_7k_common a separate kernel module
  dt-bindings: qca7000-spi: Rework binding
  dt-bindings: qca7000: rename binding
  dt-bindings: slave-device: add current-speed property
  dt-bindings: qca7000: append UART interface to binding
  tty: serdev-ttyport: return actual baudrate from ttyport_set_baudrate
  net: qualcomm: add QCA7000 UART driver

 .../devicetree/bindings/net/qca-qca7000-spi.txt|  47 ---
 .../devicetree/bindings/net/qca-qca7000.txt|  88 +
 .../devicetree/bindings/serial/slave-device.txt|   9 +
 drivers/net/ethernet/qualcomm/Kconfig  |  24 +-
 drivers/net/ethernet/qualcomm/Makefile |   7 +-
 drivers/net/ethernet/qualcomm/qca_7k.c |  28 --
 drivers/net/ethernet/qualcomm/qca_7k.h |  15 +-
 .../qualcomm/{qca_framing.c => qca_7k_common.c}|  26 +-
 .../qualcomm/{qca_framing.h => qca_7k_common.h}|  24 +-
 drivers/net/ethernet/qualcomm/qca_debug.c  |   5 +-
 drivers/net/ethernet/qualcomm/qca_spi.c|  47 ++-
 drivers/net/ethernet/qualcomm/qca_spi.h|   5 +-
 drivers/net/ethernet/qualcomm/qca_uart.c   | 423 +
 drivers/tty/serdev/serdev-ttyport.c|   2 +-
 14 files changed, 630 insertions(+), 120 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/net/qca-qca7000-spi.txt
 create mode 100644 Documentation/devicetree/bindings/net/qca-qca7000.txt
 rename drivers/net/ethernet/qualcomm/{qca_framing.c => qca_7k_common.c} (85%)
 rename drivers/net/ethernet/qualcomm/{qca_framing.h => qca_7k_common.h} (86%)
 create mode 100644 drivers/net/ethernet/qualcomm/qca_uart.c

-- 
2.1.4



Re: [PATCH v3 1/3] arm64: kvm: support kvmtool to detect RAS extension feature

2017-05-10 Thread gengdongjiu
Dear, James

On 2017/5/9 1:31, James Morse wrote:
> Hi gengdongjiu,
> 
> On 04/05/17 18:20, gengdongjiu wrote:
>>> On 30/04/17 06:37, Dongjiu Geng wrote:
 Handle kvmtool's detection for RAS extension, because sometimes
 the APP needs to know the CPU's capacity
>>>
 diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
 index d9e9697..1004039 100644
 --- a/arch/arm64/kvm/reset.c
 +++ b/arch/arm64/kvm/reset.c
 @@ -64,6 +64,14 @@ static bool cpu_has_32bit_el1(void)
   return !!(pfr0 & 0x20);
  }

 +static bool kvm_arm_support_ras_extension(void)
 +{
 + u64 pfr0;
 +
 + pfr0 = read_system_reg(SYS_ID_AA64PFR0_EL1);
 + return !!(pfr0 & 0x1000);
 +}
>>>
>>> Why are we telling user-space that the CPU has RAS extensions? EL0 can't do
>>> anything with this and the guest EL1 can detect it from the id registers.
>>>
>>>
>>> Are you using this to decide whether or not to generate a HEST for the 
>>> guest?
>>
>> James, yes, it is.  my current user-space qemu EL0 patches indeed will
>> check the RAS  extensions.
>> if has the RAS extensions. for SEA, userspace qemu will generate the
>> CPER and inject the SEA to guest;
>> for SEI,  userspace qemu sets the virtual SEI with the specified
>> Syndrome(set the HCR_EL2.VSE and vsesr_el2 );
>> if not have RAS extensions, Qemu does nothing
> 
> But you can use APEI in a guest on CPUs without the RAS extensions: the host 
> may
> signal memory errors to Qemu for any number of reasons, user-space shouldn't
> care how it knows. Examples are PCI-AER, any APEI event notified by polling or
> one of the flavours of irq.
> 
> I would expect Qemu to generate a HEST based on its abilities, i.e. if it
> supports any mechanism of notifying the guest about errors. Choosing the
> mechanism then depends on the type of error.
> 
> Ideally the Qemu code for HEST/GHES/CPER generation code using some of the 
> irqs
> and polling could be shared with x86, as these should be possible using common
> KVM APIs.
Ok, got it.

> 
> 
>>> If Qemu/kvmtool supports handling memory-failure notifications from signals 
>>> you
>>> should always generate a HEST. The GHES notification method could be 
>>> anything
>>> Qemu can deliver to the guest using the KVM APIs. Notifications from Qemu 
>>> to the
>>> guest don't depend on the RAS extensions. KVM has APIs for IRQ and SEA (you 
>>> can
>>> use KVM_SET_ONE_REG).
>>
>> I will consider your suggestion to  always generate a CPER instead of
> 
> (generate a HEST, CPER are the runtime records. There are too many acronyms in
> this space!)
  thanks James's correction.

> 
>> relying on the RAS extensions, thanks
> 
> 
> Thanks,
> 
> James
> 
> 
> .
> 



  1   2   3   4   5   6   7   >