Re: [RFC GIT PULL, v2] RCU changes for v4.12
* 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
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
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
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
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.
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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?
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()
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
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
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
>>> 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
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
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
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
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
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
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
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
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
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
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"
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
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
* 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
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
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
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"
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
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
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
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
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
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
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
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
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
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
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
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
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 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
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
> 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
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
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
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
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()
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
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
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
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
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
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
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
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
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
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
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
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()
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
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
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
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
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
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
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
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
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
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
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
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
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 > > > . >