Re: [PATCH v2 11/17] kvm: arm64: Configure VTCR per VM
Hi Suzuki,
On 27/03/18 14:15, Suzuki K Poulose wrote:
> We set VTCR_EL2 very early during the stage2 init and don't
> touch it ever. This is fine as we had a fixed IPA size. This
> patch changes the behavior to set the VTCR for a given VM,
> depending on its stage2 table. The common configuration for
> VTCR is still performed during the early init as we have to
> retain the hardware access flag update bits (VTCR_EL2_HA)
> per CPU (as they are only set for the CPUs which are capabile).
(Nit: capable)
> The bits defining the number of levels in the page table (SL0)
> and and the size of the Input address to the translation (T0SZ)
> are programmed for each VM upon entry to the guest.
> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> index 870f4b1..5ccd3ae 100644
> --- a/arch/arm64/kvm/hyp/switch.c
> +++ b/arch/arm64/kvm/hyp/switch.c
> @@ -164,6 +164,12 @@ static void __hyp_text __deactivate_traps(struct
> kvm_vcpu *vcpu)
> static void __hyp_text __activate_vm(struct kvm_vcpu *vcpu)
> {
> struct kvm *kvm = kern_hyp_va(vcpu->kvm);
> + u64 vtcr = read_sysreg(vtcr_el2);
> +
> + vtcr &= ~VTCR_EL2_PRIVATE_MASK;
> + vtcr |= VTCR_EL2_SL0(kvm_stage2_levels(kvm)) |
> + VTCR_EL2_T0SZ(kvm_phys_shift(kvm));
> + write_sysreg(vtcr, vtcr_el2);
> write_sysreg(kvm->arch.vttbr, vttbr_el2);
> }
Do we need to set this register for tlb maintenance too?
e.g. tlbi for a 3-level-stage2 vm when a 2-level-stage2 vm's vtcr is loaded...
(The ARM-ARM has 'Any of the bits of VTCR_EL2 are permitted to be cached in a
TLB'.)
Thanks,
James
Re: [PATCH v2 1/3] locking: Document the semantics of spin_is_locked()
On Tue, Apr 03, 2018 at 04:43:00PM +0200, Peter Zijlstra wrote: > On Tue, Apr 03, 2018 at 07:17:18AM -0700, Paul E. McKenney wrote: > > Suggestions for a fix? Clearly great care is required when using it > > in things like WARN_ON()... > > Yeah, don't use it there, use lockdep_assert_held(). Good point, -ETOOEARLY. ;-) > As I stated before in this thread, ideally we'd make *_is_locked() go > away entirely. After being reminded of the issues on UP systems, I now have much more sympathy for that view... Thanx, Paul
[PATCH v1] perf stat: enable 1ms interval for printing event counters values
Currently print count interval for performance counters values is
limited by 10ms so reading the values at frequencies higher than 100Hz
is restricted by the tool.
This change makes perf stat -I possible on frequencies up to 1KHz and,
to some extent, makes perf stat -I to be on-par with perf record
sampling profiling.
When running perf stat -I for monitoring e.g. PCIe uncore counters and
at the same time profiling some I/O workload by perf record e.g. for
cpu-cycles and context switches, it is then possible to observe
consolidated CPU/OS/IO(Uncore) performance picture for that workload.
Tool overhead warning printed when specifying -v option can be missed
due to screen scrolling in case you have output to the console
so message is moved into help available by running perf stat -h.
Signed-off-by: Alexey Budankov
---
tools/perf/builtin-stat.c | 14 ++
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index f5c454855908..147a27e8c937 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -1943,7 +1943,8 @@ static const struct option stat_options[] = {
OPT_STRING(0, "post", _cmd, "command",
"command to run after to the measured command"),
OPT_UINTEGER('I', "interval-print", _config.interval,
- "print counts at regular interval in ms (>= 10)"),
+ "print counts at regular interval in ms "
+ "(overhead is possible for values <= 100ms)"),
OPT_INTEGER(0, "interval-count", _config.times,
"print counts for fixed number of times"),
OPT_UINTEGER(0, "timeout", _config.timeout,
@@ -2923,17 +2924,6 @@ int cmd_stat(int argc, const char **argv)
}
}
- if (interval && interval < 100) {
- if (interval < 10) {
- pr_err("print interval must be >= 10ms\n");
- parse_options_usage(stat_usage, stat_options, "I", 1);
- goto out;
- } else
- pr_warning("print interval < 100ms. "
- "The overhead percentage could be high in
some cases. "
- "Please proceed with caution.\n");
- }
-
if (stat_config.times && interval)
interval_count = true;
else if (stat_config.times && !interval) {
Re: [RFC PATCH v3 1/3] ima: extend clone() with IMA namespace support
On Mon, Apr 02, 2018 at 07:20:54AM -0400, Stefan Berger wrote: Good morning to everyone. > On 03/29/2018 01:44 PM, Dr. Greg Wettstein wrote: > >On Mar 28, 8:44am, Stefan Berger wrote: > >} Subject: Re: [RFC PATCH v3 1/3] ima: extend clone() with IMA namespace > >sup > > > >Good morning, I hope the week is going well for everyone. > > > >>On 03/28/2018 08:14 AM, Dr. Greg Wettstein wrote: > >>>On Wed, Mar 28, 2018 at 07:10:12AM -0400, Stefan Berger wrote: > >>> > >>>Good morning, I hope the day is starting out well for everyone. > >>> > On 03/27/2018 07:01 PM, Eric W. Biederman wrote: > >Stefan Berger writes: > > > >>From: Yuqiong Sun > >> > >>Add new CONFIG_IMA_NS config option. Let clone() create a new IMA > >>namespace upon CLONE_NEWUSER flag. Attach the ima_ns data structure > >>to user_namespace. ima_ns is allocated and freed upon IMA namespace > >>creation and exit, which is tied to USER namespace creation and exit. > >>Currently, the ima_ns contains no useful IMA data but only a dummy > >>interface. This patch creates the framework for namespacing the > >>different > >>aspects of IMA (eg. IMA-audit, IMA-measurement, IMA-appraisal). > >Tying IMA to the user namespace is far better than tying IMA > >to the mount namespace. It may even be the proper answer. > > > >You had asked what it would take to unstick this so you won't have > >problems next time you post and I did not get as far as answering. > > > >I had a conversation a while back with Mimi and I believe what was > >agreed was that IMA to start doing it's thing early needs a write > >to securityfs/imafs. > Above you say 'proper answer' for user namespace. Now this sounds like > making it independent. > > >As such I expect the best way to create the ima namespace is by simply > >writing to securityfs/imafs. Possibly before the user namespace is > >even unshared. That would allow IMA to keep track of things from > >before a container is created. > So you are saying to not tie it to user namespace but make it an > independent namespace and to not use a clone flag (0x1000) but use > the filesystem to spawn a new namespace. Should that be an IMA > specific file or a file that can be shared with other subsystems? > >>>We've been platforming solutions for about 18 months now on top of a > >>>namespaced IMA implementation that we developed and carry against the > >>>4.4.x kernel. Technically its not an IMA namespace, but rather a > >>>behavioral namespace, since we implement information exchange event > >>>modeling, conceptually though its all the same and its origins were > >>>IMA. > >>Are you intending to make this publicly available and/or contribute > >>it ? > >An interesting question and one that will be the subject of a meeting > >this afternoon. > What is the outcome of this meeting ? After extensive discussion a decision was made that another meeting was needed :-) > >The namespace implementation is probably of limited value but the > >modeling engine would arguably be utility in open-source form. We > >built the ima/behavior namespace implementation since it is a very > >practical requirement for a deterministically modeled os/application > >stack. > It's hard to figure out from what you are saying to determine what > you doing in user space and what is done in the kernel. The kernel exports the characteristics of the information exchange event, ie. Actor(process)/Subject(inode) identity attributes, the userspace implementation processes those according to policy directives and maintains a contour map of the current behavior of the system. Most commonly it compares this map to a map that is supplied with the policy directives to determine if the behavior of the system is 'on-or-off contour'. > >The current namespace work will satisfy a very broad constituency > >but we needed an implementation 18 months ago. Consensus and > >expediency are always conflicting goals. We are only trying to > >offer some insight from practical experience if the community ever > >wants to consider a vision larger then file integrity. > I think you may have to become a bit more concrete for others to see > what you are doing. Mimi invited us to present our work at the Linux Security Summit in Seattle the year before last. You can get the paper that we wrote to support the presentation at the following link: ftp://ftp.idfusion.net/pub/idfusion/iso-identity.pdf I think it is also still available at kernsec.org. The paper discussed a specific hardware implementation but has a full discussion of the theory we use to implement deterministically modeled application platforms. Our work has advanced significantly from where things were at when we wrote that paper. Most specifically, almost everything has moved to userspace in order to be hosted in a trusted execution environment. The paper also does not cover
Re: call/normal switch was Re: omap4-droid4: voice call support was
* Tony Lindgren [180402 15:59]: > * Dan Williams [180402 15:51]: > > On Sun, 2018-04-01 at 10:30 -0700, Tony Lindgren wrote: > > > * Tony Lindgren [180401 15:38]: > > > Found it! Here's what I need to do over n_gsm: > > > > > > ngsm 1 "AT+CFUN=1" > > > ngsm 1 "AT+CFUN?" > > > ngsm 2 "AT+EACC=3,0"# enable mic > > > ngsm 2 "AT+CLVL=4" # set speaker volume > > > ngsm 2 "AT+CMUT=0" # unmute mic > > > > I tried to look through the QMI dumps we have in libqmi from 2013 > > (latest Qualcomm posted) and couldn't find anything to do with mic > > control, speaker volume, or anything like that. > > > > If the modem supports the AT service (which I think it does? Not > > looking at the libqmi dumps right now) then it could potentially tunnel > > these AT commands through QMI too. > > > > Perhaps Qualcomm added something to the Voice service after 2013, or > > perhaps there are other services that might control speaker/mic that we > > don't have public dumps for yet though. > > OK thanks for checking. So probably only n_gsm channel 1 is for normal > Qualcomm at commands, and then channel 2 and others are commands > implemented by Motorola on the mdm6600. > > I guess we'd have to add support for reading and writing to > /dev/gsmtty2 at least as it looks like these cannot be accessed > via /dev/ttyUSB4. Or at least I have not figured out any other > way to access them. Hmm or maybe there is some way to select where qmi to tunnels the AT commands to? Some kind of channel type paramenter like n_gsm has? Anyways, for trying to figure out things for alsamixer, I tested that ngsm 2 "AT+EACC=3,0" is not related to cpcap audio register settings and the mic is enabled during the call with Pavel's patch with alsamixer "Mode" set to either "Call" or "Normal". Also speaker keeps working during the call toggling between "Call" and "Normal". "Voice" in alsamixer does also control the speaker volume during the call. And setting the second "Speaker" from "Voice" to "HiFi" mutes the speaker. Then alsamixer capture settings for "Mic2" do change the mic gain as expected, and setting "Right" to "Off" mutes the mic. And then setting "Left" to "Mic 2" turns on the mic again. So my guess is that ngsm 2 "AT+EACC=3,0" and ngsm 2 "AT+CLVL=4" might control some external amp over the mdm6600 gpio pins? With "AT+CLVL" values being 0 7 it sounds like 3 gpios for that? Regards, Tony
Re: [PATCH 00/15] ARM: pxa: switch to DMA slave maps
On 2 April 2018 at 16:26, Robert Jarzmik wrote: > Hi, > > This serie is aimed at removing the dmaengine slave compat use, and transfer > knowledge of the DMA requestors into architecture code. > > This was discussed/advised by Arnd a couple of years back, it's almost time. > > The serie is divided in 3 phasees : > - phase 1 : patch 1/15 and patch 2/15 >=> this is the preparation work > - phase 2 : patches 3/15 .. 10/15 >=> this is the switch of all the drivers >=> this one will require either an Ack of the maintainers or be taken by > them > once phase 1 is merged > - phase 3 : patches 11/15 >=> this is the last part, cleanup and removal of export of the DMA filter > function > > As this looks like a patch bomb, each maintainer expressing for his tree > either > an Ack or "I want to take through my tree" will be spared in the next > iterations > of this serie. Perhaps an option is to send this hole series as PR for 3.17 rc1, that would removed some churns and make this faster/easier? Well, if you receive the needed acks of course. For the mmc change: Acked-by: Ulf Hansson Kind regards Uffe > > Several of these changes have been tested on actual hardware, including : > - pxamci > - pxa_camera > - smc* > - ASoC and SSP > > Happy review. > > Robert Jarzmik (15): > dmaengine: pxa: use a dma slave map > ARM: pxa: add dma slave map > mmc: pxamci: remove the dmaengine compat need > media: pxa_camera: remove the dmaengine compat need > mtd: nand: pxa3xx: remove the dmaengine compat need > net: smc911x: remove the dmaengine compat need > net: smc91x: remove the dmaengine compat need > ASoC: pxa: remove the dmaengine compat need > net: irda: pxaficp_ir: remove the dmaengine compat need > ata: pata_pxa: remove the dmaengine compat need > dmaengine: pxa: document pxad_param > dmaengine: pxa: make the filter function internal > ARM: pxa: remove the DMA IO resources > ARM: pxa: change SSP devices allocation > ARM: pxa: change SSP DMA channels allocation > > arch/arm/mach-pxa/devices.c | 269 > ++ > arch/arm/mach-pxa/devices.h | 14 +- > arch/arm/mach-pxa/include/mach/audio.h| 12 ++ > arch/arm/mach-pxa/pxa25x.c| 4 +- > arch/arm/mach-pxa/pxa27x.c| 4 +- > arch/arm/mach-pxa/pxa3xx.c| 5 +- > arch/arm/plat-pxa/ssp.c | 50 +- > drivers/ata/pata_pxa.c| 10 +- > drivers/dma/pxa_dma.c | 13 +- > drivers/media/platform/pxa_camera.c | 22 +-- > drivers/mmc/host/pxamci.c | 29 +--- > drivers/mtd/nand/pxa3xx_nand.c| 10 +- > drivers/net/ethernet/smsc/smc911x.c | 16 +- > drivers/net/ethernet/smsc/smc91x.c| 12 +- > drivers/net/ethernet/smsc/smc91x.h| 1 - > drivers/staging/irda/drivers/pxaficp_ir.c | 14 +- > include/linux/dma/pxa-dma.h | 20 +-- > include/linux/platform_data/mmp_dma.h | 4 + > include/linux/pxa2xx_ssp.h| 4 +- > sound/arm/pxa2xx-ac97.c | 14 +- > sound/arm/pxa2xx-pcm-lib.c| 6 +- > sound/soc/pxa/pxa-ssp.c | 5 +- > sound/soc/pxa/pxa2xx-ac97.c | 32 +--- > 23 files changed, 196 insertions(+), 374 deletions(-) > > -- > 2.11.0 >
Re: [GIT PULL] Kernel lockdown for secure boot
[re-added cc's, I think. Sorry, I think I failed to use the gmane
gateway correctly there.]
On Tue, Apr 3, 2018 at 12:06 AM, David Howells wrote:
> Andy Lutomirski wrote:
>
>> This is an attempt at a review. I'm replying here because I can't find the
>> actual relevant patch emails.
>
> This was the latest post:
>
> https://lkml.org/lkml/2017/11/9/660
>
> and they were posted multiple times before that, plus distributions, such as
> Fedora, have been carrying them for a long while.
>
>> For the rest of this review, I'm going to pretend that you actually want two
>> features: "try-prevent-root-from-corrupting-the-kernel" and
>> "try-to-prevent-root-from-reading-kernel-memory".
>
> It theoretically boils down into those two, but the line is blurrier than you
> think.
>
> Further, some of the vectors that can be used to do one can potentially do the
> other also and it starts getting to be a lot of extra work to distinguish the
> two.
>
>> I do *not* see why the mere act of using Secure Boot should have this
>> effect.
>
> To be able to pass secure boot mode over kexec, you have to make sure that the
> kernel image doesn't get corrupted, lest someone blacklist your signing key in
> the bootloader.
Can you explain that much more clearly? I'm asking why booting via
UEFI Secure Boot should enable lockdown, and I don't see what this has
to do with kexec. And "someone blacklist[ing] your key in the
bootloader" sounds like a political issue, not a technical issue.
What is the actual purpose of these patches?
>
>> In particular, UEFI Secure Boot should *not* enable
>> "try-to-prevent-root-from-reading-kernel-memory", which means that, unless
>> you actually implement the split, you should drop a bunch of the patches.
>
> Yes it should. If someone can read your kernel image, they can steal the
> crypto keys you use to encrypt your filesystem.
Can you please explain the actual attack that is avoided by doing this?
Suppose I'm a bad guy attacking someone's laptop. If I just have
normal uid!=0 access, then these patches have no effect. Instead,
we're talking about an attacker who is somehow able to become global
root and bypass all LSM restrictions but has not gained kernel code
execution. It is indeed the case that your patches make it harder to
simply read the dm-crypt encryption key out of main memory. But root
can attack the disk encryption in many other ways. They can
persistently compromise the machine by adding services or user
accounts or intentionally misconfiguring something. They can directly
read the entire contents of the disk. They can modify the initrd so
that the next time the machine reboots and the user types the
password, the attacker gets the key (unless the TPM is involved, but
getting *that* right on a standard distro is difficult or impossible).
And I'm not even sure why an attacker who manages to become root wants
your disk encryption key. That key is worth nothing unless the
attacker makes its attack persistent, but, if the attacker can install
a persistent user-level backdoor, then they can read the cleartext off
your disk just as easily as they can read the ciphertext.
>
>> "Restrict /dev/{mem,kmem,port} when the kernel is locked down": this should
>> probably split into one restriction for read and one for write.
>
> Not so for /dev/port. Read & Write here are _not_ the same as Read & Write
> on, say, /dev/mem. In fact, if /dev/mem gives you access to mmio ports, then
> the same applies there. Btw, Fedora hasn't even provided /dev/kmem for a
> while.
Then split /dev/mem and turn off /dev/port for all locked-down modes.
>
>> "bpf: Restrict kernel image access functions when the kernel is locked down":
>> This patch just sucks in general.
>
> Yes - but that's what Alexei Starovoitov specified. bpf kind of sucks since
> it gives you unrestricted access to the kernel.
bpf, in certain contexts, gives you unrestricted access to *reading*
kernel memory. bpf should, under no circumstances, let you write to
the kernel unless you're using fault injection or similar.
I'm surprised that Alexei acked this patch. If something like XDP or
bpfilter starts becoming widely used, this patch will require a lot of
reworking to avoid breaking standard distros.
Re: Signal handling in a page fault handler
On Tue, Apr 03, 2018 at 07:48:29AM -0700, Matthew Wilcox wrote:
> On Tue, Apr 03, 2018 at 03:12:35PM +0200, Thomas Hellstrom wrote:
> > I think the TTM page fault handler originally set the standard for this.
> > First, IMO any critical section that waits for the GPU (like typically the
> > page fault handler does), should be locked at least killable. The need for
> > interruptible locks came from the X server's silken mouse relying on signals
> > for smooth mouse operations: You didn't want the X server to be stuck in the
> > kernel waiting for GPU completion when it should handle the cursor move
> > request.. Now that doesn't seem to be the case anymore but to reiterate
> > Chris' question, why would the signal persist once returned to user-space?
>
> Yeah, you graphics people have had to deal with much more recalcitrant
> hardware than most of the rest of us ... and less reasonable user
> expectations ("My graphics card was doing something and I expected
> everything else to keep going" vs "My hard drive died and my kernel
> paniced, oh well.")
>
> I don't know exactly how the signal code works at the delivery end;
> I'm not sure when TIF_SIGPENDING gets cleared. I just get concerned
> when I see one bit of kernel code doing things in a very complicated
> and careful manner and another bit of kernel code blithely assuming
> that everything's going to be OK.
I think you last line pretty much sums up the proper attitude when writing
gpu drivers:
https://i.imgflip.com/27nm7w.jpg
Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Re: [PATCH v3] nvmet: fix nvmet_execute_write_zeroes function
On 4/2/2018 8:38 PM, Keith Busch wrote: Thanks, I've applied the patch with a simpler changelog explaining the bug. Thanks Rodrigo and Keith, I've tested with/w.o the patch and it works well (with the fix only). -Max. ___ Linux-nvme mailing list linux-n...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme
Re: [PATCH RFC tools/memory-model] Add s390.{cfg,cat}
On Tue, Apr 03, 2018 at 09:50:19AM -0400, Alan Stern wrote: > On Mon, 2 Apr 2018, Paul E. McKenney wrote: > > > > > I will look at this more later, reaching end of both battery and useful > > > > attention span... > > > > Like the following, perhaps? > > > > Thanx, Paul > > > > > > > > s390 > > > > include "fences.cat" > > include "cos.cat" > > > > (* Fundamental coherence ordering *) > > let com = rf | co | fr > > acyclic po-loc | com as coherence > > > > (* Atomic *) > > empty rmw & (fre;coe) as atom > > > > (* Fences *) > > let mb = [M] ; fencerel(Mb) ; [M] > > > > (* TSO with multicopy atomicity *) > > let po-ghb = ([R] ; po ; [M]) | ([M] ; po ; [W]) > > acyclic mb | po-ghb | fr | rf | co as sc > > Yes, that should work okay (apart from issues related to ordering of > atomic accesses). > > By the way, what does that last "sc" stand for? Surely not Sequential > Consistency! You might consider renaming it to "tso-mca". Good point, fixed. But it is the closest to SC in commercial computing systems, for whatever that is worth. ;-) Thanx, Paul
Re: [PATCH v3 09/14] s390: vfio-ap: sysfs interfaces to configure domains
On Tue, 3 Apr 2018 11:12:45 -0400
Tony Krowiak wrote:
> On 04/03/2018 07:17 AM, Cornelia Huck wrote:
> > On Wed, 14 Mar 2018 14:25:49 -0400
> > Tony Krowiak wrote:
> >
> >> Provides the sysfs interfaces for assigning AP domains to
> >> and unassigning AP domains from a mediated matrix device.
> >>
> >> An AP domain ID corresponds to an AP queue index (APQI). For
> >> each domain assigned to the mediated matrix device, its
> >> corresponging APQI is stored in an AP queue mask (AQM).
> >> The bits in the AQM, from most significant to least
> >> significant bit, correspond to AP domain numbers 0 to 255.
> >> When a domain is assigned, the bit corresponding to its
> >> APQI will be set in the AQM. Likewise, when a domain is
> >> unassigned, the bit corresponding to its APQI will be
> >> cleared from the AQM.
> >>
> >> The relevant sysfs structures are:
> >>
> >> /sys/devices/vfio_ap
> >> ... [matrix]
> >> .. [mdev_supported_types]
> >> . [vfio_ap-passthrough]
> >> [devices]
> >> ...[$uuid]
> >> .. assign_domain
> >> .. unassign_domain
> >>
> >> To assign a domain to the $uuid mediated matrix device,
> >> write the domain's ID to the assign_domain file. To
> >> unassign a domain, write the domain's ID to the
> >> unassign_domain file. The ID is specified using
> >> conventional semantics: If it begins with 0x, the number
> >> will be parsed as a hexadecimal (case insensitive) number;
> >> otherwise, it will be parsed as a decimal number.
> >>
> >> For example, to assign domain 173 (0xad) to the mediated matrix
> >> device $uuid:
> >>
> >>echo 173 > assign_domain
> >>
> >>or
> >>
> >>echo 0xad > assign_domain
> >>
> >> To unassign domain 173 (0xad):
> >>
> >>echo 173 > unassign_domain
> >>
> >>or
> >>
> >>echo 0xad > unassign_domain
> >>
> >> The assignment will be rejected:
> >>
> >> * If the domain ID exceeds the maximum value for an AP domain:
> >>
> >>* If the AP Extended Addressing (APXA) facility is installed,
> >> the max value is 255
> >>
> >>* Else the max value is 15
> >>
> >> * If no AP adapters have yet been assigned and there are
> >>no AP queues reserved by the VFIO AP driver that have an APQN
> >>with an APQI matching that of the AP domain number being
> >>assigned.
> >>
> >> * If any of the APQNs that can be derived from the intersection
> >>of the APQI being assigned and the AP adapter ID (APID) of
> >>each of the AP adapters previously assigned can not be matched
> >>with an APQN of an AP queue device reserved by the VFIO AP
> >>driver.
> >>
> >> Signed-off-by: Tony Krowiak
> >> ---
> >> arch/s390/include/asm/kvm-ap.h|1 +
> >> drivers/s390/crypto/vfio_ap_ops.c | 215
> >> -
> >> 2 files changed, 215 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/drivers/s390/crypto/vfio_ap_ops.c
> >> b/drivers/s390/crypto/vfio_ap_ops.c
> >> index 90512a6..c448835 100644
> >> --- a/drivers/s390/crypto/vfio_ap_ops.c
> >> +++ b/drivers/s390/crypto/vfio_ap_ops.c
> >> @@ -377,10 +377,223 @@ static ssize_t unassign_adapter_store(struct device
> >> *dev,
> >> }
> >> DEVICE_ATTR_WO(unassign_adapter);
> >>
> >> +/**
> >> + * vfio_ap_validate_queues_for_apqi
> >> + *
> >> + * @ap_matrix: the matrix device
> >> + * @matrix_mdev: the mediated matrix device
> >> + * @apqi: an AP queue index (APQI) - corresponds to a domain ID
> >> + *
> >> + * Verifies that each APQN that is derived from the intersection of @apqi
> >> and
> >> + * each AP adapter ID (APID) corresponding to an AP domain assigned to the
> >> + * @matrix_mdev matches the APQN of an AP queue reserved by the VFIO AP
> >> device
> >> + * driver.
> >> + *
> >> + * Returns 0 if validation succeeds; otherwise, returns an error.
> >> + */
> >> +static int vfio_ap_validate_queues_for_apqi(struct ap_matrix *ap_matrix,
> >> + struct ap_matrix_mdev *matrix_mdev,
> >> + unsigned long apqi)
> >> +{
> >> + int ret;
> >> + struct vfio_ap_qid_match qid_match;
> >> + unsigned long apid;
> >> + struct device_driver *drv = ap_matrix->device.driver;
> >> +
> >> + /**
> >> + * Examine each APQN with the specified APQI
> >> + */
> >> + for_each_set_bit_inv(apid, matrix_mdev->matrix->apm,
> >> + matrix_mdev->matrix->apm_max) {
> >> + qid_match.qid = AP_MKQID(apid, apqi);
> >> + qid_match.dev = NULL;
> >> +
> >> + ret = driver_for_each_device(drv, NULL, _match,
> >> + vfio_ap_queue_match);
> >> + if (ret)
> >> + return ret;
> > Hm, I'm wondering whether jumping out of the outer loop is the correct
> > thing to do here - and if yes, whether we should log an error?
> If you look at the vfio_ap_queue_match() function which is passed to the
> driver_for_each_device() function, it never
Re: [PATCH 02/15] ARM: pxa: add dma slave map
Arnd Bergmann writes:
>> + { "smc911x.0", "rx", PDMA_FILTER_PARAM(LOWEST, -1) },
>> + { "smc911x.0", "tx", PDMA_FILTER_PARAM(LOWEST, -1) },
>> + { "smc91x.0", "data", PDMA_FILTER_PARAM(LOWEST, -1) },
>
> This one is interesting, as you are dealing with an off-chip device,
> and the channel number is '-'1. How does this even work? Does it
> mean
This relies on pxa_dma, in which the "-1" for a requestor line means "no
requestor" or said in another way "always requesting". As a consequence, as soon
as the DMA descriptors are queued, the transfer begins, and it is supposed
implicitely that the FIFO output availability is at least as quick as the system
bus and the DMA size is perfectly fit for the FIFO available bytes.
This is what has been the underlying of DMA transfers of smc91x(x) on the PXA
platforms, where the smc91x(s) are directly wired on the system bus (the same
bus having DRAM, SRAM, IO-mapped devices).
>
>> + /* PXA25x specific map */
>> + { "pxa25x-ssp.0", "rx", PDMA_FILTER_PARAM(LOWEST, 13) },
>> + { "pxa25x-ssp.0", "tx", PDMA_FILTER_PARAM(LOWEST, 14) },
>> + { "pxa25x-nssp.1", "rx", PDMA_FILTER_PARAM(LOWEST, 15) },
>> + { "pxa25x-nssp.1", "tx", PDMA_FILTER_PARAM(LOWEST, 16) },
>> + { "pxa25x-nssp.2", "rx", PDMA_FILTER_PARAM(LOWEST, 23) },
>> + { "pxa25x-nssp.2", "tx", PDMA_FILTER_PARAM(LOWEST, 24) },
>> + { "pxa-pcm-audio", "nssp2_rx", PDMA_FILTER_PARAM(LOWEST, 15) },
>> + { "pxa-pcm-audio", "nssp2_tx", PDMA_FILTER_PARAM(LOWEST, 16) },
>> + { "pxa-pcm-audio", "nssp3_rx", PDMA_FILTER_PARAM(LOWEST, 23) },
>> + { "pxa-pcm-audio", "nssp3_tx", PDMA_FILTER_PARAM(LOWEST, 24) },
>> +
>> + /* PXA27x specific map */
>> + { "pxa-pcm-audio", "ssp3_rx", PDMA_FILTER_PARAM(LOWEST, 66) },
>> + { "pxa-pcm-audio", "ssp3_tx", PDMA_FILTER_PARAM(LOWEST, 67) },
>> + { "pxa27x-camera.0", "CI_Y", PDMA_FILTER_PARAM(HIGHEST, 68) },
>> + { "pxa27x-camera.0", "CI_U", PDMA_FILTER_PARAM(HIGHEST, 69) },
>> + { "pxa27x-camera.0", "CI_V", PDMA_FILTER_PARAM(HIGHEST, 70) },
>> +
>> + /* PXA3xx specific map */
>> + { "pxa-pcm-audio", "ssp4_rx", PDMA_FILTER_PARAM(LOWEST, 2) },
>> + { "pxa-pcm-audio", "ssp4_tx", PDMA_FILTER_PARAM(LOWEST, 3) },
>> + { "pxa2xx-mci.1", "rx", PDMA_FILTER_PARAM(LOWEST, 93) },
>> + { "pxa2xx-mci.1", "tx", PDMA_FILTER_PARAM(LOWEST, 94) },
>> + { "pxa3xx-nand", "data", PDMA_FILTER_PARAM(LOWEST, 97) },
>> + { "pxa2xx-mci.2", "rx", PDMA_FILTER_PARAM(LOWEST, 100) },
>> + { "pxa2xx-mci.2", "tx", PDMA_FILTER_PARAM(LOWEST, 101) },
>> +};
>
> Since more than half the entries in here are chip specific, maybe it would be
> better to split that table into three and have a copy for each one in
> arch/arm/mach-pxa/pxa{25x.27x.3xx}.c?
Mmmh, today the split is :
- 16 common entries
- 10 pxa25x specific entries
- 5 pxa27x specific entries
- 7 pxa3xx specific entries
=> total of 38 lines
After the split we'll have :
- 26 pxa25x specific entries
- 21 pxa27x specific entries
- 23 pxa3xx specific entries
=> total of 70 lines
That doubles the number of lines, not counting the declarations, and amending of
pxa2xx_set_dmac_info().
If you think it's worth it, what is the driving benefit behind ?
> Does that mean it's actually a memory-to-memory transfer with a device being
> on the external SRAM interface?
I'm taking this is the follow up to the "-1" question :0
Cheers.
--
Robert
Re: [PATCH v1] perf stat: enable 1ms interval for printing event counters values
On Tue, Apr 03, 2018 at 06:04:13PM +0300, Alexey Budankov wrote:
>
> Currently print count interval for performance counters values is
> limited by 10ms so reading the values at frequencies higher than 100Hz
> is restricted by the tool.
>
> This change makes perf stat -I possible on frequencies up to 1KHz and,
> to some extent, makes perf stat -I to be on-par with perf record
> sampling profiling.
>
> When running perf stat -I for monitoring e.g. PCIe uncore counters and
> at the same time profiling some I/O workload by perf record e.g. for
> cpu-cycles and context switches, it is then possible to observe
> consolidated CPU/OS/IO(Uncore) performance picture for that workload.
>
> Tool overhead warning printed when specifying -v option can be missed
> due to screen scrolling in case you have output to the console
> so message is moved into help available by running perf stat -h.
>
> Signed-off-by: Alexey Budankov
> ---
> tools/perf/builtin-stat.c | 14 ++
> 1 file changed, 2 insertions(+), 12 deletions(-)
I don't mind taking out the limit, but please update also
the stat man page, it mentiones the 10ms minimum
thanks,
jirka
>
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index f5c454855908..147a27e8c937 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -1943,7 +1943,8 @@ static const struct option stat_options[] = {
> OPT_STRING(0, "post", _cmd, "command",
> "command to run after to the measured command"),
> OPT_UINTEGER('I', "interval-print", _config.interval,
> - "print counts at regular interval in ms (>= 10)"),
> + "print counts at regular interval in ms "
> + "(overhead is possible for values <= 100ms)"),
> OPT_INTEGER(0, "interval-count", _config.times,
> "print counts for fixed number of times"),
> OPT_UINTEGER(0, "timeout", _config.timeout,
> @@ -2923,17 +2924,6 @@ int cmd_stat(int argc, const char **argv)
> }
> }
>
> - if (interval && interval < 100) {
> - if (interval < 10) {
> - pr_err("print interval must be >= 10ms\n");
> - parse_options_usage(stat_usage, stat_options, "I", 1);
> - goto out;
> - } else
> - pr_warning("print interval < 100ms. "
> -"The overhead percentage could be high in
> some cases. "
> -"Please proceed with caution.\n");
> - }
> -
> if (stat_config.times && interval)
> interval_count = true;
> else if (stat_config.times && !interval) {
Re: [PATCH v2 1/3] locking: Document the semantics of spin_is_locked()
Andrea Parri wrote: > Sorry, but I don't understand your objection: are you suggesting to add > something like "Always return 0 on !SMP" to the comment? what else? Something like that, possibly along with a warning that this might not be what you want. You might actually want it to return true on !SMP, it depends on what you're using it for. David
Re: [PATCH] locking/hung_task: Show all hung tasks before panic
On Mon, Apr 2, 2018 at 5:35 PM, Paul E. McKenney
wrote:
> On Mon, Apr 02, 2018 at 11:12:04PM +0900, Tetsuo Handa wrote:
>> When we get a hung task it can often be valuable to see _all_ the hung
>> tasks on the system before calling panic().
>>
>> Quoting from
>> https://syzkaller.appspot.com/text?tag=CrashReport=5412451675799552
>>
>> INFO: task syz-executor3:13421 blocked for more than 120 seconds.
>> Not tainted 4.16.0-rc7+ #9
>> "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
>> syz-executor3 D24672 13421 4481 0x0004
>> Call Trace:
>> context_switch kernel/sched/core.c:2862 [inline]
>> __schedule+0x8fb/0x1ec0 kernel/sched/core.c:3440
>> schedule+0xf5/0x430 kernel/sched/core.c:3499
>> __rwsem_down_read_failed_common kernel/locking/rwsem-xadd.c:269 [inline]
>> rwsem_down_read_failed+0x401/0x6e0 kernel/locking/rwsem-xadd.c:286
>> call_rwsem_down_read_failed+0x18/0x30 arch/x86/lib/rwsem.S:94
>> __down_read arch/x86/include/asm/rwsem.h:83 [inline]
>> down_read+0xa4/0x150 kernel/locking/rwsem.c:26
>> __get_super.part.9+0x1d3/0x280 fs/super.c:663
>> __get_super include/linux/spinlock.h:310 [inline]
>> get_super+0x2d/0x40 fs/super.c:692
>> fsync_bdev+0x19/0x80 fs/block_dev.c:468
>> invalidate_partition+0x35/0x60 block/genhd.c:1566
>> drop_partitions.isra.12+0xcd/0x1d0 block/partition-generic.c:440
>> rescan_partitions+0x72/0x900 block/partition-generic.c:513
>> __blkdev_reread_part+0x15f/0x1e0 block/ioctl.c:173
>> blkdev_reread_part+0x26/0x40 block/ioctl.c:193
>> loop_reread_partitions+0x12f/0x1a0 drivers/block/loop.c:619
>> loop_set_status+0x9bb/0xf60 drivers/block/loop.c:1161
>> loop_set_status64+0x9d/0x110 drivers/block/loop.c:1271
>> lo_ioctl+0xd86/0x1b70 drivers/block/loop.c:1381
>> (...snipped...)
>> Showing all locks held in the system:
>> (...snipped...)
>> 3 locks held by syz-executor3/13421:
>> #0: (>lo_ctl_mutex/1){+.+.}, at: [<834f78af>]
>> lo_ioctl+0x8b/0x1b70 drivers/block/loop.c:1355 /*
>> mutex_lock_nested(>lo_ctl_mutex, 1); */
>> #1: (>bd_mutex){+.+.}, at: [<03605603>]
>> blkdev_reread_part+0x1e/0x40 block/ioctl.c:192
>> #2: (>s_umount_key#77){.+.+}, at: [<77701649>]
>> __get_super.part.9+0x1d3/0x280 fs/super.c:663 /* down_read(>s_umount); */
>> (...snipped...)
>> 2 locks held by syz-executor0/13428:
>> #0: (>s_umount_key#76/1){+.+.}, at: [] alloc_super
>> fs/super.c:211 [inline]
>> #0: (>s_umount_key#76/1){+.+.}, at: []
>> sget_userns+0x3a1/0xe40 fs/super.c:502 /* down_write_nested(>s_umount,
>> SINGLE_DEPTH_NESTING); */
>> #1: (>lo_ctl_mutex/1){+.+.}, at: [<834f78af>]
>> lo_ioctl+0x8b/0x1b70 drivers/block/loop.c:1355 /*
>> mutex_lock_nested(>lo_ctl_mutex, 1); */
>>
>>
>> In addition to showing hashed address of lock instances, it would be
>> nice if trace of 13428 is printed as well as 13421.
>>
>> Showing hung tasks up to /proc/sys/kernel/hung_task_warnings could delay
>> calling panic() but normally there should not be so many hung tasks.
>>
>> Signed-off-by: Tetsuo Handa
>> Cc: Vegard Nossum
>> Cc: Andrew Morton
>> Cc: Linus Torvalds
>> Cc: Mandeep Singh Baines
>> Cc: Paul E. McKenney
>
> I just know that I am going to regret this the first time this happens
> on a low-speed console port, but...
>
> Acked-by: Paul E. McKenney
Thanks! I think getting these last bits of debugging tools to be more
useful is very important in the context of syzbot. So:
Acked-by: Dmitry Vyukov
>> Cc: Peter Zijlstra
>> Cc: Thomas Gleixner
>> Cc: Ingo Molnar
>> ---
>> kernel/hung_task.c | 11 +++
>> 1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/kernel/hung_task.c b/kernel/hung_task.c
>> index 751593e..32b4794 100644
>> --- a/kernel/hung_task.c
>> +++ b/kernel/hung_task.c
>> @@ -44,6 +44,7 @@
>>
>> static int __read_mostly did_panic;
>> static bool hung_task_show_lock;
>> +static bool hung_task_call_panic;
>>
>> static struct task_struct *watchdog_task;
>>
>> @@ -127,10 +128,8 @@ static void check_hung_task(struct task_struct *t,
>> unsigned long timeout)
>> touch_nmi_watchdog();
>>
>> if (sysctl_hung_task_panic) {
>> - if (hung_task_show_lock)
>> - debug_show_all_locks();
>> - trigger_all_cpu_backtrace();
>> - panic("hung_task: blocked tasks");
>> + hung_task_show_lock = true;
>> + hung_task_call_panic = true;
>> }
>> }
>>
>> @@ -193,6 +192,10 @@ static void check_hung_uninterruptible_tasks(unsigned
>> long timeout)
>> rcu_read_unlock();
>> if (hung_task_show_lock)
>> debug_show_all_locks();
>> + if (hung_task_call_panic) {
>> + trigger_all_cpu_backtrace();
>> + panic("hung_task: blocked tasks");
>> + }
>> }
>>
>> static long hung_timeout_jiffies(unsigned long last_checked,
Re: [GIT PULL] siginfo fix for v4.16-rc5
On Apr 3, 2018, at 10:27 AM, Eric W. Biederman wrote: > Geert Uytterhoeven writes: > >> On Mon, Apr 2, 2018 at 10:17 PM, Eric W. Biederman >> wrote: >> >>> A 2-byte alignment for 4 byte pointers. That is a new one to me. >> >> Not just for pointers, also for int and long. > > The smallest I have seen previously has been 64bit integers having > 32bit alignment. 32bit entities having only 16bit alignment on a 32bit > arch was simply a surprise. Even when it works there tend to be good > reasons not to do that by default. The 68K architecture began as 16-bit with the 68000. Rather than tightening requirements, the 68020 not only maintained compatibility with 16-bit alignment, but also forgave byte-misaligned data accesses (albeit with a performance penalty). Jumping to an odd address is still an error, though. Josh
Re: [PATCH v2 2/6] i2c: i2c-stm32f7: Add slave support
On Mon, Mar 26, 2018 at 10:41:51AM +0200, Pierre Yves MORDRET wrote: > > > On 03/25/2018 08:16 PM, Wolfram Sang wrote: > > On Wed, Mar 21, 2018 at 05:48:56PM +0100, Pierre-Yves MORDRET wrote: > >> This patch adds slave support for I2C controller embedded in STM32F7 SoC > >> > >> Signed-off-by: M'boumba Cedric Madianga > >> Signed-off-by: Pierre-Yves MORDRET > > > > Looks OK from a first look. What kind of tests did you do? > > As mentioned for 10-bit, I'm using 2 I2C instances from the SoC. > Here are the tests I dit: > - Master/slave send in 7 and 10 bits > - master/slave recv in 7 and 10 bits > - E2PROM Read/Write As far as I understand the code now, both instances can be master / slave simultanously on the same bus? signature.asc Description: PGP signature
[PATCH v2 0/6] spi: Add support for DMA transfers in sun4i SPI driver
The following patchset provides corrections for PIO-mode and support for DMA transfers in sun4i SPI driver. Changes in v2: 1) Restored processing of 3/4 FIFO full interrupt. 2) Debug log enhancements. Sergey Suloev (6): spi: core: handle timeout error from transfer_one() spi: sun4i: restrict transfer length in PIO-mode spi: sun4i: coding style/readability improvements spi: sun4i: use completion provided by SPI core driver spi: sun4i: introduce register set/unset helpers spi: sun4i: add DMA transfers support drivers/spi/spi-sun4i.c | 442 +--- drivers/spi/spi.c | 5 +- 2 files changed, 347 insertions(+), 100 deletions(-) -- 2.16.2
[PATCH v2 2/6] spi: sun4i: restrict transfer length in PIO-mode
There is no need to handle the 3/4 FIFO empty interrupt
as the maximum supported transfer length in PIO mode
is 64 bytes.
As long as a problem was reported previously with filling FIFO
on A10s we want to stick with 63 bytes depth.
Changes in v2:
1) Restored processing of 3/4 FIFO full interrupt.
Signed-off-by: Sergey Suloev
---
drivers/spi/spi-sun4i.c | 37 ++---
1 file changed, 10 insertions(+), 27 deletions(-)
diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
index 4141003..08fd007 100644
--- a/drivers/spi/spi-sun4i.c
+++ b/drivers/spi/spi-sun4i.c
@@ -22,7 +22,12 @@
#include
-#define SUN4I_FIFO_DEPTH 64
+/*
+ * FIFO length is 64 bytes
+ * But filling the FIFO fully might cause a timeout
+ * on some devices, for example on spi2 on A10s
+ */
+#define SUN4I_FIFO_DEPTH 63
#define SUN4I_RXDATA_REG 0x00
@@ -202,7 +207,7 @@ static void sun4i_spi_set_cs(struct spi_device *spi, bool
enable)
static size_t sun4i_spi_max_transfer_size(struct spi_device *spi)
{
- return SUN4I_FIFO_DEPTH - 1;
+ return SUN4I_FIFO_DEPTH;
}
static int sun4i_spi_transfer_one(struct spi_master *master,
@@ -216,11 +221,8 @@ static int sun4i_spi_transfer_one(struct spi_master
*master,
int ret = 0;
u32 reg;
- /* We don't support transfer larger than the FIFO */
- if (tfr->len > SUN4I_MAX_XFER_SIZE)
- return -EMSGSIZE;
-
- if (tfr->tx_buf && tfr->len >= SUN4I_MAX_XFER_SIZE)
+ /* We don't support transfers larger than FIFO depth */
+ if (tfr->len > SUN4I_FIFO_DEPTH)
return -EMSGSIZE;
reinit_completion(>done);
@@ -313,17 +315,12 @@ static int sun4i_spi_transfer_one(struct spi_master
*master,
/*
* Fill the TX FIFO
-* Filling the FIFO fully causes timeout for some reason
-* at least on spi2 on A10s
*/
- sun4i_spi_fill_fifo(sspi, SUN4I_FIFO_DEPTH - 1);
+ sun4i_spi_fill_fifo(sspi, SUN4I_FIFO_DEPTH);
/* Enable the interrupts */
sun4i_spi_enable_interrupt(sspi, SUN4I_INT_CTL_TC |
SUN4I_INT_CTL_RF_F34);
- /* Only enable Tx FIFO interrupt if we really need it */
- if (tx_len > SUN4I_FIFO_DEPTH)
- sun4i_spi_enable_interrupt(sspi, SUN4I_INT_CTL_TF_E34);
/* Start the transfer */
reg = sun4i_spi_read(sspi, SUN4I_CTL_REG);
@@ -371,20 +368,6 @@ static irqreturn_t sun4i_spi_handler(int irq, void *dev_id)
return IRQ_HANDLED;
}
- /* Transmit FIFO 3/4 empty */
- if (status & SUN4I_INT_CTL_TF_E34) {
- sun4i_spi_fill_fifo(sspi, SUN4I_FIFO_DEPTH);
-
- if (!sspi->len)
- /* nothing left to transmit */
- sun4i_spi_disable_interrupt(sspi, SUN4I_INT_CTL_TF_E34);
-
- /* Only clear the interrupt _after_ re-seeding the FIFO */
- sun4i_spi_write(sspi, SUN4I_INT_STA_REG, SUN4I_INT_CTL_TF_E34);
-
- return IRQ_HANDLED;
- }
-
return IRQ_NONE;
}
--
2.16.2
[PATCH v2 6/6] spi: sun4i: add DMA transfers support
DMA transfers are now available for sun4i-family SoCs.
The DMA mode is used automatically as soon as requested
transfer length is more than FIFO length.
Changes in v2:
1) Debug log enhancements.
Signed-off-by: Sergey Suloev
---
drivers/spi/spi-sun4i.c | 299
1 file changed, 277 insertions(+), 22 deletions(-)
diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
index d81d31c..dda7922 100644
--- a/drivers/spi/spi-sun4i.c
+++ b/drivers/spi/spi-sun4i.c
@@ -14,6 +14,8 @@
#include
#include
#include
+#include
+#include
#include
#include
#include
@@ -39,6 +41,7 @@
#define SUN4I_CTL_CPHA BIT(2)
#define SUN4I_CTL_CPOL BIT(3)
#define SUN4I_CTL_CS_ACTIVE_LOWBIT(4)
+#define SUN4I_CTL_DMA_DEDICATEDBIT(5)
#define SUN4I_CTL_LMTF BIT(6)
#define SUN4I_CTL_TF_RST BIT(8)
#define SUN4I_CTL_RF_RST BIT(9)
@@ -58,6 +61,8 @@
#define SUN4I_INT_STA_REG 0x10
#define SUN4I_DMA_CTL_REG 0x14
+#define SUN4I_CTL_DMA_RF_READY BIT(0)
+#define SUN4I_CTL_DMA_TF_NOT_FULL BIT(10)
#define SUN4I_WAIT_REG 0x18
@@ -169,6 +174,13 @@ static inline void sun4i_spi_fill_fifo(struct sun4i_spi
*sspi, int len)
}
}
+static bool sun4i_spi_can_dma(struct spi_master *master,
+ struct spi_device *spi,
+ struct spi_transfer *tfr)
+{
+ return tfr->len > SUN4I_FIFO_DEPTH;
+}
+
static void sun4i_spi_set_cs(struct spi_device *spi, bool enable)
{
struct sun4i_spi *sspi = spi_master_get_devdata(spi->master);
@@ -208,6 +220,11 @@ static void sun4i_spi_set_cs(struct spi_device *spi, bool
enable)
static size_t sun4i_spi_max_transfer_size(struct spi_device *spi)
{
+ struct spi_master *master = spi->master;
+
+ if (master->can_dma)
+ return SUN4I_MAX_XFER_SIZE;
+
return SUN4I_FIFO_DEPTH;
}
@@ -235,6 +252,164 @@ static int sun4i_spi_wait_for_transfer(struct spi_device
*spi,
return 0;
}
+static void sun4i_spi_dma_callback(void *param)
+{
+ struct spi_master *master = param;
+
+ dev_dbg(>dev, "DMA transfer complete\n");
+ spi_finalize_current_transfer(master);
+}
+
+static int sun4i_spi_dmap_prep_tx(struct spi_master *master,
+ struct spi_transfer *tfr,
+ dma_cookie_t *cookie)
+{
+ struct dma_async_tx_descriptor *chan_desc = NULL;
+
+ chan_desc = dmaengine_prep_slave_sg(master->dma_tx,
+ tfr->tx_sg.sgl, tfr->tx_sg.nents,
+ DMA_TO_DEVICE,
+ DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+ if (!chan_desc) {
+ dev_err(>dev,
+ "Couldn't prepare TX DMA slave\n");
+ return -EIO;
+ }
+
+ chan_desc->callback = sun4i_spi_dma_callback;
+ chan_desc->callback_param = master;
+
+ *cookie = dmaengine_submit(chan_desc);
+ dma_async_issue_pending(master->dma_tx);
+
+ return 0;
+}
+
+static int sun4i_spi_dmap_prep_rx(struct spi_master *master,
+ struct spi_transfer *tfr,
+ dma_cookie_t *cookie)
+{
+ struct dma_async_tx_descriptor *chan_desc = NULL;
+
+ chan_desc = dmaengine_prep_slave_sg(master->dma_rx,
+ tfr->rx_sg.sgl, tfr->rx_sg.nents,
+ DMA_FROM_DEVICE,
+ DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+ if (!chan_desc) {
+ dev_err(>dev,
+ "Couldn't prepare RX DMA slave\n");
+ return -EIO;
+ }
+
+ chan_desc->callback = sun4i_spi_dma_callback;
+ chan_desc->callback_param = master;
+
+ *cookie = dmaengine_submit(chan_desc);
+ dma_async_issue_pending(master->dma_rx);
+
+ return 0;
+}
+
+static int sun4i_spi_transfer_one_dma(struct spi_device *spi,
+ struct spi_transfer *tfr)
+{
+ struct spi_master *master = spi->master;
+ struct sun4i_spi *sspi = spi_master_get_devdata(master);
+ dma_cookie_t tx_cookie = 0, rx_cookie = 0;
+ enum dma_status status;
+ int ret;
+ u32 reg = 0;
+
+ dev_dbg(>dev, "Using DMA mode for transfer\n");
+
+ /* Disable interrupts */
+ sun4i_spi_write(sspi, SUN4I_INT_CTL_REG, 0);
+
+ if (sspi->tx_buf) {
+ ret = sun4i_spi_dmap_prep_tx(master, tfr, _cookie);
+ if (ret)
+ goto out;
+
+ reg |= SUN4I_CTL_DMA_TF_NOT_FULL;
+ }
+
+ if (sspi->rx_buf) {
+ ret =
[PATCH v2 5/6] spi: sun4i: introduce register set/unset helpers
Two helper functions were added in order to set/unset
specified flags in registers.
Signed-off-by: Sergey Suloev
---
drivers/spi/spi-sun4i.c | 40 +++-
1 file changed, 19 insertions(+), 21 deletions(-)
diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
index 9d1bc20..d81d31c 100644
--- a/drivers/spi/spi-sun4i.c
+++ b/drivers/spi/spi-sun4i.c
@@ -107,29 +107,29 @@ static inline void sun4i_spi_write(struct sun4i_spi
*sspi, u32 reg, u32 value)
writel(value, sspi->base_addr + reg);
}
-static inline u32 sun4i_spi_get_tx_fifo_count(struct sun4i_spi *sspi)
+static inline void sun4i_spi_set(struct sun4i_spi *sspi, u32 addr, u32 val)
{
- u32 reg = sun4i_spi_read(sspi, SUN4I_FIFO_STA_REG);
-
- reg >>= SUN4I_FIFO_STA_TF_CNT_BITS;
+ u32 reg = sun4i_spi_read(sspi, addr);
- return reg & SUN4I_FIFO_STA_TF_CNT_MASK;
+ reg |= val;
+ sun4i_spi_write(sspi, addr, reg);
}
-static inline void sun4i_spi_enable_interrupt(struct sun4i_spi *sspi, u32 mask)
+static inline void sun4i_spi_unset(struct sun4i_spi *sspi, u32 addr, u32 val)
{
- u32 reg = sun4i_spi_read(sspi, SUN4I_INT_CTL_REG);
+ u32 reg = sun4i_spi_read(sspi, addr);
- reg |= mask;
- sun4i_spi_write(sspi, SUN4I_INT_CTL_REG, reg);
+ reg &= ~val;
+ sun4i_spi_write(sspi, addr, reg);
}
-static inline void sun4i_spi_disable_interrupt(struct sun4i_spi *sspi, u32
mask)
+static inline u32 sun4i_spi_get_tx_fifo_count(struct sun4i_spi *sspi)
{
- u32 reg = sun4i_spi_read(sspi, SUN4I_INT_CTL_REG);
+ u32 reg = sun4i_spi_read(sspi, SUN4I_FIFO_STA_REG);
- reg &= ~mask;
- sun4i_spi_write(sspi, SUN4I_INT_CTL_REG, reg);
+ reg >>= SUN4I_FIFO_STA_TF_CNT_BITS;
+
+ return reg & SUN4I_FIFO_STA_TF_CNT_MASK;
}
static inline void sun4i_spi_drain_fifo(struct sun4i_spi *sspi, int len)
@@ -256,13 +256,12 @@ static int sun4i_spi_transfer_one(struct spi_master
*master,
/* Clear pending interrupts */
sun4i_spi_write(sspi, SUN4I_INT_STA_REG, ~0);
+ /* Reset FIFOs */
+ sun4i_spi_set(sspi, SUN4I_CTL_REG,
+ SUN4I_CTL_RF_RST | SUN4I_CTL_TF_RST);
reg = sun4i_spi_read(sspi, SUN4I_CTL_REG);
- /* Reset FIFOs */
- sun4i_spi_write(sspi, SUN4I_CTL_REG,
- reg | SUN4I_CTL_RF_RST | SUN4I_CTL_TF_RST);
-
/*
* Setup the transfer control register: Chip Select,
* polarities, etc.
@@ -342,12 +341,11 @@ static int sun4i_spi_transfer_one(struct spi_master
*master,
sun4i_spi_fill_fifo(sspi, SUN4I_FIFO_DEPTH);
/* Enable the interrupts */
- sun4i_spi_enable_interrupt(sspi, SUN4I_INT_CTL_TC |
-SUN4I_INT_CTL_RF_F34);
+ sun4i_spi_set(sspi, SUN4I_INT_CTL_REG, SUN4I_INT_CTL_TC |
+ SUN4I_INT_CTL_RF_F34);
/* Start the transfer */
- reg = sun4i_spi_read(sspi, SUN4I_CTL_REG);
- sun4i_spi_write(sspi, SUN4I_CTL_REG, reg | SUN4I_CTL_XCH);
+ sun4i_spi_set(sspi, SUN4I_CTL_REG, SUN4I_CTL_XCH);
ret = sun4i_spi_wait_for_transfer(spi, tfr);
--
2.16.2
[PATCH v2 4/6] spi: sun4i: use completion provided by SPI core driver
As long as the completion already provided by the SPI core
then there is no need to waste extra-memory on this.
Also a waiting function was added to avoid code duplication.
Signed-off-by: Sergey Suloev
---
drivers/spi/spi-sun4i.c | 62 -
1 file changed, 35 insertions(+), 27 deletions(-)
diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
index 899e956..9d1bc20 100644
--- a/drivers/spi/spi-sun4i.c
+++ b/drivers/spi/spi-sun4i.c
@@ -92,8 +92,6 @@ struct sun4i_spi {
struct clk *hclk;
struct clk *mclk;
- struct completion done;
-
const u8*tx_buf;
u8 *rx_buf;
int len;
@@ -213,13 +211,36 @@ static size_t sun4i_spi_max_transfer_size(struct
spi_device *spi)
return SUN4I_FIFO_DEPTH;
}
+static int sun4i_spi_wait_for_transfer(struct spi_device *spi,
+ struct spi_transfer *tfr)
+{
+ struct spi_master *master = spi->master;
+ unsigned int start, end, tx_time;
+ unsigned int timeout;
+
+ /* calc required timeout from given speed & len values */
+ tx_time = max(tfr->len * 8 * 2 / (tfr->speed_hz / 1000), 100U);
+ start = jiffies;
+ timeout = wait_for_completion_timeout(>xfer_completion,
+ msecs_to_jiffies(tx_time));
+ end = jiffies;
+ if (!timeout) {
+ dev_warn(>dev,
+"%s: timeout transferring %u bytes@%iHz for %i(%i)ms",
+dev_name(>dev), tfr->len, tfr->speed_hz,
+jiffies_to_msecs(end - start), tx_time);
+ return -ETIMEDOUT;
+ }
+
+ return 0;
+}
+
static int sun4i_spi_transfer_one(struct spi_master *master,
struct spi_device *spi,
struct spi_transfer *tfr)
{
struct sun4i_spi *sspi = spi_master_get_devdata(master);
- unsigned int mclk_rate, div, timeout;
- unsigned int start, end, tx_time;
+ unsigned int mclk_rate, div;
unsigned int tx_len = 0;
int ret = 0;
u32 reg;
@@ -228,7 +249,6 @@ static int sun4i_spi_transfer_one(struct spi_master *master,
if (tfr->len > SUN4I_FIFO_DEPTH)
return -EMSGSIZE;
- reinit_completion(>done);
sspi->tx_buf = tfr->tx_buf;
sspi->rx_buf = tfr->rx_buf;
sspi->len = tfr->len;
@@ -329,22 +349,8 @@ static int sun4i_spi_transfer_one(struct spi_master
*master,
reg = sun4i_spi_read(sspi, SUN4I_CTL_REG);
sun4i_spi_write(sspi, SUN4I_CTL_REG, reg | SUN4I_CTL_XCH);
- tx_time = max(tfr->len * 8 * 2 / (tfr->speed_hz / 1000), 100U);
- start = jiffies;
- timeout = wait_for_completion_timeout(>done,
- msecs_to_jiffies(tx_time));
- end = jiffies;
- if (!timeout) {
- dev_warn(>dev,
-"%s: timeout transferring %u bytes@%iHz for %i(%i)ms",
-dev_name(>dev), tfr->len, tfr->speed_hz,
-jiffies_to_msecs(end - start), tx_time);
- ret = -ETIMEDOUT;
- goto out;
- }
-
+ ret = sun4i_spi_wait_for_transfer(spi, tfr);
-out:
sun4i_spi_write(sspi, SUN4I_INT_CTL_REG, 0);
return ret;
@@ -352,14 +358,18 @@ out:
static irqreturn_t sun4i_spi_handler(int irq, void *dev_id)
{
- struct sun4i_spi *sspi = dev_id;
- u32 status = sun4i_spi_read(sspi, SUN4I_INT_STA_REG);
+ struct spi_master *master = dev_id;
+ struct sun4i_spi *sspi = spi_master_get_devdata(master);
+ u32 status;
+
+ status = sun4i_spi_read(sspi, SUN4I_INT_STA_REG);
/* Transfer complete */
if (status & SUN4I_INT_CTL_TC) {
- sun4i_spi_write(sspi, SUN4I_INT_STA_REG, SUN4I_INT_CTL_TC);
+ sun4i_spi_write(sspi, SUN4I_INT_STA_REG,
+ SUN4I_INT_CTL_TC);
sun4i_spi_drain_fifo(sspi, SUN4I_FIFO_DEPTH);
- complete(>done);
+ spi_finalize_current_transfer(master);
return IRQ_HANDLED;
}
@@ -456,7 +466,7 @@ static int sun4i_spi_probe(struct platform_device *pdev)
}
ret = devm_request_irq(>dev, irq, sun4i_spi_handler,
- 0, dev_name(>dev), sspi);
+ 0, dev_name(>dev), master);
if (ret) {
dev_err(>dev, "Cannot request IRQ\n");
goto err_free_master;
@@ -476,8 +486,6 @@ static int sun4i_spi_probe(struct platform_device *pdev)
goto err_free_master;
}
- init_completion(>done);
-
/*
* This wake-up/shutdown pattern is to be able to have the
* device woken up, even if runtime_pm is disabled
--
[PATCH v2 3/6] spi: sun4i: coding style/readability improvements
Minor changes to fulfill the coding style and
improve the readability.
Signed-off-by: Sergey Suloev
---
drivers/spi/spi-sun4i.c | 32 +---
1 file changed, 17 insertions(+), 15 deletions(-)
diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
index 08fd007..899e956 100644
--- a/drivers/spi/spi-sun4i.c
+++ b/drivers/spi/spi-sun4i.c
@@ -83,8 +83,11 @@
#define SUN4I_FIFO_STA_TF_CNT_MASK 0x7f
#define SUN4I_FIFO_STA_TF_CNT_BITS 16
+#define SUN4I_SPI_MAX_SPEED_HZ 100 * 1000 * 1000
+#define SUN4I_SPI_MIN_SPEED_HZ 3 * 1000
+#define SUN4I_SPI_MODE_BITS(SPI_CPOL | SPI_CPHA | SPI_CS_HIGH |
SPI_LSB_FIRST)
+
struct sun4i_spi {
- struct spi_master *master;
void __iomem*base_addr;
struct clk *hclk;
struct clk *mclk;
@@ -418,12 +421,23 @@ static int sun4i_spi_probe(struct platform_device *pdev)
struct resource *res;
int ret = 0, irq;
- master = spi_alloc_master(>dev, sizeof(struct sun4i_spi));
+ master = spi_alloc_master(>dev, sizeof(*sspi));
if (!master) {
dev_err(>dev, "Unable to allocate SPI Master\n");
return -ENOMEM;
}
+ master->max_speed_hz = SUN4I_SPI_MAX_SPEED_HZ;
+ master->min_speed_hz = SUN4I_SPI_MIN_SPEED_HZ;
+ master->num_chipselect = 4;
+ master->mode_bits = SUN4I_SPI_MODE_BITS;
+ master->bits_per_word_mask = SPI_BPW_MASK(8);
+ master->set_cs = sun4i_spi_set_cs;
+ master->transfer_one = sun4i_spi_transfer_one;
+ master->max_transfer_size = sun4i_spi_max_transfer_size;
+ master->dev.of_node = pdev->dev.of_node;
+ master->auto_runtime_pm = true;
+
platform_set_drvdata(pdev, master);
sspi = spi_master_get_devdata(master);
@@ -442,24 +456,12 @@ static int sun4i_spi_probe(struct platform_device *pdev)
}
ret = devm_request_irq(>dev, irq, sun4i_spi_handler,
- 0, "sun4i-spi", sspi);
+ 0, dev_name(>dev), sspi);
if (ret) {
dev_err(>dev, "Cannot request IRQ\n");
goto err_free_master;
}
- sspi->master = master;
- master->max_speed_hz = 100 * 1000 * 1000;
- master->min_speed_hz = 3 * 1000;
- master->set_cs = sun4i_spi_set_cs;
- master->transfer_one = sun4i_spi_transfer_one;
- master->num_chipselect = 4;
- master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH | SPI_LSB_FIRST;
- master->bits_per_word_mask = SPI_BPW_MASK(8);
- master->dev.of_node = pdev->dev.of_node;
- master->auto_runtime_pm = true;
- master->max_transfer_size = sun4i_spi_max_transfer_size;
-
sspi->hclk = devm_clk_get(>dev, "ahb");
if (IS_ERR(sspi->hclk)) {
dev_err(>dev, "Unable to acquire AHB clock\n");
--
2.16.2
[PATCH v2 1/6] spi: core: handle timeout error from transfer_one()
As long as sun4i/sun6i SPI drivers have overriden the default
"wait for completion" procedure then we need to properly
handle -ETIMEDOUT error from transfer_one().
Signed-off-by: Sergey Suloev
---
drivers/spi/spi.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index b33a727..2dcd4f6 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1028,7 +1028,7 @@ static int spi_transfer_one_message(struct spi_controller
*ctlr,
reinit_completion(>xfer_completion);
ret = ctlr->transfer_one(ctlr, msg->spi, xfer);
- if (ret < 0) {
+ if (ret < 0 && ret != -ETIMEDOUT) {
SPI_STATISTICS_INCREMENT_FIELD(statm,
errors);
SPI_STATISTICS_INCREMENT_FIELD(stats,
@@ -1051,7 +1051,7 @@ static int spi_transfer_one_message(struct spi_controller
*ctlr,
msecs_to_jiffies(ms));
}
- if (ms == 0) {
+ if (ms == 0 || ret == -ETIMEDOUT) {
SPI_STATISTICS_INCREMENT_FIELD(statm,
timedout);
SPI_STATISTICS_INCREMENT_FIELD(stats,
@@ -1059,6 +1059,7 @@ static int spi_transfer_one_message(struct spi_controller
*ctlr,
dev_err(>spi->dev,
"SPI transfer timed out\n");
msg->status = -ETIMEDOUT;
+ ret = 0;
}
} else {
if (xfer->len)
--
2.16.2
Re: [PATCH v2 3/6] i2c: i2c-stm32f7: Add initial SMBus protocols support
> >> All SMBus protocols are implemented except SMBus-specific protocols. > > > > What does that mean? > > It miss SMBus Host Notification and SMBBus Alert. They are almost ready but > I'm > struggling to put them back to operational state after recent changes related > to > SMBust Host Notification. A more "classic" interrupt base solution has been > put > in place but I fail to use implement it in my side. > Another patch set is going to be delivered for these 2 commands. This is totally fine to implement it incrementally. Please just update the commit message with the more detailed explanation above. > > That is quite some complexity considering we have I2C_FUNC_SMBUS_EMUL. I > > don't mind, but you really want that? > > > > All SMBBus commands are implemented as such. I never try to emulation > commands. > Should we use emulation SMBus commands or real commands... Don't know. You won't see any difference on the wire. I don't know your HW. It might be that SMBus mode is more "automatic" and uses less interrupts. Or stuff like Alert or HostNotification only works in this mode. If you and the driver maintainers think it is worth the added complexity, I am fine, too. signature.asc Description: PGP signature
[GIT] Sparc
1) Add support for ADI (Application Data Integrity) found in more recent sparc64 cpus. Essentially this is keyed based access to virtual memory, and if the key encoded in the virual address is wrong you get a trap. The mm changes were reviewed by Andrew Morton and others. Work by Khalid Aziz. 2) Validate DAX completion index range properly, from Rob Gardner. 3) Add proper Kconfig deps for DAX driver. From Guenter Roeck. Please pull, thanks a lot. The following changes since commit 8f5fd927c3a7576d57248a2d7a0861c3f2795973: Merge tag 'for-4.16-rc5-tag' of git://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux (2018-03-16 13:37:42 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/davem/sparc-next.git for you to fetch changes up to d13864b68e41c11e4231de90cf358658f6ecea45: sparc64: Make atomic_xchg() an inline function rather than a macro. (2018-04-03 08:24:35 -0700) David S. Miller (3): Merge branch 'sparc64-ADI' Merge git://git.kernel.org/.../davem/sparc sparc64: Make atomic_xchg() an inline function rather than a macro. Guenter Roeck (1): sparc64: Oracle DAX driver depends on SPARC64 Khalid Aziz (12): signals, sparc: Add signal codes for ADI violations mm, swap: Add infrastructure for saving page metadata on swap sparc64: Add support for ADI register fields, ASIs and traps sparc64: Add HV fault type handlers for ADI related faults sparc64: Add handler for "Memory Corruption Detected" trap sparc64: Add auxiliary vectors to report platform ADI properties mm: Add address parameter to arch_validate_prot() mm: Clear arch specific VM flags on protection change mm: Allow arch code to override copy_highpage() sparc64: Add support for ADI (Application Data Integrity) sparc64: Update signal delivery to use new helper functions sparc: Make auxiliary vectors for ADI available on 32-bit as well Rob Gardner (1): sparc64: Properly range check DAX completion index Documentation/sparc/adi.txt | 278 arch/powerpc/include/asm/mman.h | 4 +- arch/powerpc/kernel/syscalls.c | 2 +- arch/sparc/include/asm/adi.h| 6 ++ arch/sparc/include/asm/adi_64.h | 47 + arch/sparc/include/asm/atomic_64.h | 6 +- arch/sparc/include/asm/elf_64.h | 5 ++ arch/sparc/include/asm/hypervisor.h | 2 + arch/sparc/include/asm/mman.h | 84 ++- arch/sparc/include/asm/mmu_64.h | 17 + arch/sparc/include/asm/mmu_context_64.h | 51 ++ arch/sparc/include/asm/page_64.h| 6 ++ arch/sparc/include/asm/pgtable_64.h | 48 ++ arch/sparc/include/asm/thread_info_64.h | 2 +- arch/sparc/include/asm/trap_block.h | 2 + arch/sparc/include/asm/ttable.h | 10 +++ arch/sparc/include/uapi/asm/asi.h | 5 ++ arch/sparc/include/uapi/asm/auxvec.h| 9 ++- arch/sparc/include/uapi/asm/mman.h | 2 + arch/sparc/include/uapi/asm/pstate.h| 10 +++ arch/sparc/kernel/Makefile | 1 + arch/sparc/kernel/adi_64.c | 397 + arch/sparc/kernel/entry.h | 3 + arch/sparc/kernel/etrap_64.S| 27 +++- arch/sparc/kernel/head_64.S | 1 + arch/sparc/kernel/mdesc.c | 2 + arch/sparc/kernel/process_64.c | 25 +++ arch/sparc/kernel/rtrap_64.S| 33 - arch/sparc/kernel/setup_64.c| 2 + arch/sparc/kernel/sun4v_mcd.S | 18 + arch/sparc/kernel/traps_64.c| 130 ++-- arch/sparc/kernel/ttable_64.S | 6 +- arch/sparc/kernel/urtt_fill.S | 7 +- arch/sparc/kernel/vmlinux.lds.S | 5 ++ arch/sparc/mm/gup.c | 37 +++ arch/sparc/mm/hugetlbpage.c | 14 +++- arch/sparc/mm/init_64.c | 69 +++ arch/sparc/mm/tsb.c | 21 ++ arch/x86/kernel/signal_compat.c | 2 +- drivers/sbus/char/Kconfig | 3 +- drivers/sbus/char/oradax.c | 2 +- include/asm-generic/pgtable.h | 36 ++ include/linux/highmem.h | 4 ++ include/linux/mm.h | 9 +++ include/linux/mman.h| 2 +- include/uapi/asm-generic/siginfo.h | 5 +- mm/ksm.c| 4 ++ mm/memory.c | 1 + mm/mprotect.c | 4 +- mm/rmap.c | 14 50 files changed, 1451
Re: [PATCH 14/15] ARM: pxa: change SSP devices allocation
Arnd Bergmann writes:
chop chop ... removed several mail recipients to leave only the ASoC / PXA
subset ...
> On Mon, Apr 2, 2018 at 4:26 PM, Robert Jarzmik wrote:
>
>>
>> +static struct pxa_ssp_info pxa_ssp_infos[] = {
>> + { .dma_chan_rx_name = "ssp1_rx", .dma_chan_tx_name = "ssp1_tx", },
>> + { .dma_chan_rx_name = "ssp1_rx", .dma_chan_tx_name = "ssp1_tx", },
>> + { .dma_chan_rx_name = "ssp2_rx", .dma_chan_tx_name = "ssp2_tx", },
>> + { .dma_chan_rx_name = "ssp2_rx", .dma_chan_tx_name = "ssp2_tx", },
>> + { .dma_chan_rx_name = "ssp3_rx", .dma_chan_tx_name = "ssp3_tx", },
>> + { .dma_chan_rx_name = "ssp3_rx", .dma_chan_tx_name = "ssp3_tx", },
>> + { .dma_chan_rx_name = "ssp4_rx", .dma_chan_tx_name = "ssp4_tx", },
>> + { .dma_chan_rx_name = "ssp4_rx", .dma_chan_tx_name = "ssp4_tx", },
>> +};
>
> This part looks odd to me, you're adding an extra level of indirection to
> do two stages of lookups in some form of platform data.
That's unfortunately right.
> Why can't you just always use "rx" and "tx" as the names here?
Well I couldn't. I'll explain you why, and maybe you'll find a better solution.
That all is related to how ASoC and SSP interact.
If I remember correctly, here is how it works :
- the DMA channel is requested in sound/arm/pxa2xx-pcm-lib.c:128
return snd_dmaengine_pcm_open(
substream, dma_request_slave_channel(rtd->platform->dev,
The trick is that the device here is _not_ the SSP one, it's if memory serves
me well the pxa-pcm-audio one.
As a consequence, the device cannot be used to differenciate which SSP
exactly is providing the sound samples stream. This information is
nevertheless required to choose the correct requestor line, which is a 1-to-1
match to the SSP port.
The indirection in the channel name is used to choose the correct requestor
line for a given SSP port providing the samples.
It also must be underlined that this dma request serves both AC97 and SSP as
sample providers.
> (also, I don't see why each line is duplicated, but I'm sure there's
> an easy answer for that).
Ahh that is an unfortunate rebase most probably :)
Cheers.
--
Robert
[PATCH v2 1/2] dt-bindings: iio: afe: add current-sense-cuicuit and voltage-divider
An ADC is often used to measure other quantities indirectly. These
bindings describe two cases, a current through a sense resistor, and
a "big" voltage measured with the help of a voltage divider.
Signed-off-by: Peter Rosin
---
.../bindings/iio/afe/current-sense-circuit.txt | 45 ++
.../bindings/iio/afe/voltage-divider.txt | 45 ++
MAINTAINERS| 7
3 files changed, 97 insertions(+)
create mode 100644
Documentation/devicetree/bindings/iio/afe/current-sense-circuit.txt
create mode 100644
Documentation/devicetree/bindings/iio/afe/voltage-divider.txt
diff --git
a/Documentation/devicetree/bindings/iio/afe/current-sense-circuit.txt
b/Documentation/devicetree/bindings/iio/afe/current-sense-circuit.txt
new file mode 100644
index ..0bc7d89387c0
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/afe/current-sense-circuit.txt
@@ -0,0 +1,45 @@
+Current Sense Curcuit
+=
+
+When an io-channel measures the voltage over a current sense resistor,
+the interesting mesaurement is often the current through the resistor,
+not the voltage over it. This binding describes such a current sense
+curcuit.
+
+Required properties:
+- compatible : "current-sense-circuit"
+- io-channels : Channel node of a voltage io-channel.
+
+Optional properties:
+- numerator : The io-channel scale is multiplied by this value (default 1).
+- denominator : The io-channel scale is divided by this value (default 1).
+
+Example:
+The system current is measured by measuring the voltage over a
+3.3 ohms sense resistor.
+
+sysi {
+ compatible = "current-sense-circuit";
+ io-channels = < 0>;
+
+ /* Divide the ADC voltage by 33/10 (i.e. 3.3) to get the current. */
+ numerator = <10>;
+ denominator = <33>;
+};
+
+ {
+ tiadc: adc@48 {
+ compatible = "ti,ads1015";
+ reg = <0x48>;
+ #io-channel-cells = <1>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ channel@0 { /* IN0,IN1 differential */
+ reg = <0>;
+ ti,gain = <1>;
+ ti,datarate = <4>;
+ };
+ };
+};
diff --git a/Documentation/devicetree/bindings/iio/afe/voltage-divider.txt
b/Documentation/devicetree/bindings/iio/afe/voltage-divider.txt
new file mode 100644
index ..fd4a215d9e6d
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/afe/voltage-divider.txt
@@ -0,0 +1,45 @@
+Voltage divider
+===
+
+When an io-channel measures the midpoint of a voltage divider, the
+interesting voltage is often the voltage over the full resistance
+of the divider. This binding describes the voltage divider in such
+a curcuit.
+
+Required properties:
+- compatible : "voltage-divider"
+- io-channels : Channel node of a voltage io-channel.
+
+Optional properties:
+- numerator : The io-channel scale is multiplied by this value (default 1).
+- denominator : The io-channel scale is divided by this value (default 1).
+
+Example:
+The system voltage is circa 12V, but divided down with a 22/200
+voltage divider to adjust it to the ADC range.
+
+SYSVADC GND
+ + + +
+ | .-. | .. |
+ '--| 200 |-+-| 22 |--'
+ '-' ''
+
+sysv {
+ compatible = "voltage-divider";
+ io-channels = < 1>;
+
+ /* Multiply the ADC voltage by 222/22 to get the system voltage. */
+ numerator = <222>; /* 200 + 22 */
+ denominator = <22>;
+};
+
+ {
+ maxadc: adc@0 {
+ compatible = "maxim,max1027";
+ reg = <0>;
+ #io-channel-cells = <1>;
+ interrupt-parent = <>;
+ interrupts = <15 IRQ_TYPE_EDGE_RISING>;
+ spi-max-frequency = <100>;
+ };
+};
diff --git a/MAINTAINERS b/MAINTAINERS
index 36a28e979e9a..9dbe5019c6bd 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6889,6 +6889,13 @@ F: drivers/staging/iio/
F: include/linux/iio/
F: tools/iio/
+IIO UNIT CONVERTER
+M: Peter Rosin
+L: linux-...@vger.kernel.org
+S: Maintained
+F: Documentation/devicetree/bindings/iio/afe/current-sense-circuit.txt
+F: Documentation/devicetree/bindings/iio/afe/voltage-divider.txt
+
IKANOS/ADI EAGLE ADSL USB DRIVER
M: Matthieu Castet
M: Stanislaw Gruszka
--
2.11.0
[PATCH v2 2/2] iio: afe: unit-converter: new driver
If an ADC channel measures the midpoint of a voltage divider, the
interesting voltage is often the voltage over the full resistance.
E.g. if the full voltage is too big for the ADC to handle.
Likewise, if an ADC channel measures the voltage across a resistor,
the interesting value is often the current through the resistor.
This driver solves both problems by allowing to linearly scale a channel
and by allowing changes to the type of the channel. Or both.
Signed-off-by: Peter Rosin
---
MAINTAINERS | 1 +
drivers/iio/Kconfig | 1 +
drivers/iio/Makefile | 1 +
drivers/iio/afe/Kconfig | 18 +++
drivers/iio/afe/Makefile | 6 +
drivers/iio/afe/iio-unit-converter.c | 257 +++
6 files changed, 284 insertions(+)
create mode 100644 drivers/iio/afe/Kconfig
create mode 100644 drivers/iio/afe/Makefile
create mode 100644 drivers/iio/afe/iio-unit-converter.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 9dbe5019c6bd..f9835521eec6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6895,6 +6895,7 @@ L:linux-...@vger.kernel.org
S: Maintained
F: Documentation/devicetree/bindings/iio/afe/current-sense-circuit.txt
F: Documentation/devicetree/bindings/iio/afe/voltage-divider.txt
+F: drivers/iio/afe/iio-unit-converter.c
IKANOS/ADI EAGLE ADSL USB DRIVER
M: Matthieu Castet
diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
index b3c8c6ef0dff..d69e85a8bdc3 100644
--- a/drivers/iio/Kconfig
+++ b/drivers/iio/Kconfig
@@ -70,6 +70,7 @@ config IIO_TRIGGERED_EVENT
source "drivers/iio/accel/Kconfig"
source "drivers/iio/adc/Kconfig"
+source "drivers/iio/afe/Kconfig"
source "drivers/iio/amplifiers/Kconfig"
source "drivers/iio/chemical/Kconfig"
source "drivers/iio/common/Kconfig"
diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
index b16b2e9ddc40..d8cba9c229c0 100644
--- a/drivers/iio/Makefile
+++ b/drivers/iio/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_IIO_TRIGGERED_EVENT) +=
industrialio-triggered-event.o
obj-y += accel/
obj-y += adc/
+obj-y += afe/
obj-y += amplifiers/
obj-y += buffer/
obj-y += chemical/
diff --git a/drivers/iio/afe/Kconfig b/drivers/iio/afe/Kconfig
new file mode 100644
index ..75acbe7eed15
--- /dev/null
+++ b/drivers/iio/afe/Kconfig
@@ -0,0 +1,18 @@
+#
+# Analog Front End drivers
+#
+# When adding new entries keep the list in alphabetical order
+
+menu "Analog Front Ends"
+
+config IIO_UNIT_CONVERTER
+ tristate "IIO unit converter"
+ depends on OF || COMPILE_TEST
+ help
+ Say yes here to build support for the IIO unit converter
+ that handles voltage dividers and current sense circuits.
+
+ To compile this driver as a module, choose M here: the
+ module will be called iio-unit-converter.
+
+endmenu
diff --git a/drivers/iio/afe/Makefile b/drivers/iio/afe/Makefile
new file mode 100644
index ..7691cc5b809a
--- /dev/null
+++ b/drivers/iio/afe/Makefile
@@ -0,0 +1,6 @@
+#
+# Makefile for industrial I/O Analog Front Ends (AFE)
+#
+
+# When adding new entries keep the list in alphabetical order
+obj-$(CONFIG_IIO_UNIT_CONVERTER) += iio-unit-converter.o
diff --git a/drivers/iio/afe/iio-unit-converter.c
b/drivers/iio/afe/iio-unit-converter.c
new file mode 100644
index ..43429543cc29
--- /dev/null
+++ b/drivers/iio/afe/iio-unit-converter.c
@@ -0,0 +1,257 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * IIO unit converter
+ *
+ * Copyright (C) 2018 Axentia Technologies AB
+ *
+ * Author: Peter Rosin
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+struct unit_converter_cfg {
+ enum iio_chan_type type;
+};
+
+enum unit_converter_variant {
+ CURRENT_SENSE_CIRCUIT,
+ VOLTAGE_DIVIDER,
+};
+
+static const struct unit_converter_cfg unit_converter_cfg[] = {
+ [CURRENT_SENSE_CIRCUIT] = {
+ .type = IIO_CURRENT,
+ },
+ [VOLTAGE_DIVIDER] = {
+ .type = IIO_VOLTAGE,
+ },
+};
+
+struct unit_converter {
+ const struct unit_converter_cfg *cfg;
+ struct iio_channel *source;
+ struct iio_dev *indio_dev;
+ struct iio_chan_spec chan;
+ struct iio_chan_spec_ext_info *ext_info;
+ s32 numerator;
+ s32 denominator;
+};
+
+static int unit_converter_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long mask)
+{
+ struct unit_converter *uc = iio_priv(indio_dev);
+ unsigned long long tmp;
+ int ret;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ return iio_read_channel_raw(uc->source, val);
+
+ case IIO_CHAN_INFO_SCALE:
+ ret = iio_read_channel_scale(uc->source, val, val2);
+ switch (ret) {
+ case IIO_VAL_FRACTIONAL:
+
[PATCH v2 0/2] iio: add unit converter
Hi! This driver implements support for voltage dividers and current sense circuits. It's pretty generic and should be easily adaptable to other linear scaling purposes... The driver is still named "unit converter", because it was not clear to me that there was a real problem with the driver being named that. I got the impression that the naming discussion in v1 was mainly about the category, and that it kind of looked odd and non-specific with unit-converter in the DT bindings, but what do I know? Cheers, Peter Changes since v1:https://lkml.org/lkml/2018/3/19/801 - Put the driver in the new afe category (Analog Front Ends) and do not move the iio-mux driver. - Do not refer to the source channel as "parent", use "source" instead. - Have the DT compatible drive the target unit, instead of relying on a "type" DT-property for that. - In the DT bindings, use an unnamed source channel. - Do not set up writes to _RAW (sorry Phil) as I don't need it and have not tested it. It's easy to add back if needed. - Fail if the source channel does not support _RAW or _SCALE. - Fix various spelling issues. - Fix various code style issues. Peter Rosin (2): dt-bindings: iio: afe: add current-sense-cuicuit and voltage-divider iio: afe: unit-converter: new driver .../bindings/iio/afe/current-sense-circuit.txt | 45 .../bindings/iio/afe/voltage-divider.txt | 45 MAINTAINERS| 8 + drivers/iio/Kconfig| 1 + drivers/iio/Makefile | 1 + drivers/iio/afe/Kconfig| 18 ++ drivers/iio/afe/Makefile | 6 + drivers/iio/afe/iio-unit-converter.c | 257 + 8 files changed, 381 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/afe/current-sense-circuit.txt create mode 100644 Documentation/devicetree/bindings/iio/afe/voltage-divider.txt create mode 100644 drivers/iio/afe/Kconfig create mode 100644 drivers/iio/afe/Makefile create mode 100644 drivers/iio/afe/iio-unit-converter.c -- 2.11.0
Re: [PATCH v2 5/6] i2c: i2c-stm32f7: Add DMA support
> +#define STM32F7_I2C_DMA_LEN_MIN 0x1
...
> + if (i2c_dev->dma && f7_msg->count >= STM32F7_I2C_DMA_LEN_MIN) {
Are you using DMA for every message with a length >= 1? The setup of
that might be more expensive than the DMA gain, if so.
signature.asc
Description: PGP signature
Re: [PATCH 02/15] ARM: pxa: add dma slave map
On Tue, Apr 3, 2018 at 5:18 PM, Robert Jarzmik wrote:
> Arnd Bergmann writes:
>
>>> + { "smc911x.0", "rx", PDMA_FILTER_PARAM(LOWEST, -1) },
>>> + { "smc911x.0", "tx", PDMA_FILTER_PARAM(LOWEST, -1) },
>>> + { "smc91x.0", "data", PDMA_FILTER_PARAM(LOWEST, -1) },
>>
>> This one is interesting, as you are dealing with an off-chip device,
>> and the channel number is '-'1. How does this even work? Does it
>> mean
>
> This relies on pxa_dma, in which the "-1" for a requestor line means "no
> requestor" or said in another way "always requesting". As a consequence, as
> soon
> as the DMA descriptors are queued, the transfer begins, and it is supposed
> implicitely that the FIFO output availability is at least as quick as the
> system
> bus and the DMA size is perfectly fit for the FIFO available bytes.
>
> This is what has been the underlying of DMA transfers of smc91x(x) on the PXA
> platforms, where the smc91x(s) are directly wired on the system bus (the same
> bus having DRAM, SRAM, IO-mapped devices).
Ok, I looked at the driver in more detail now and found the scary parts.
So it's using the async DMA interface to do synchronous DMA in
interrupt context in order to transfer the rx data faster than an readsl()
would, correct?
It still feels odd to me that there is an entry in the slave map for
a device that does not have a request line. However, it also seems
that the entire code in those two drivers that deals with DMA is specific
to PXA anyway, so maybe it can be done differently: instead of
calling dma_request_slave_channel_compat() or dma_request_chan()
with a fake request line, how about calling dma_request_channel()
with an NULL filter function and data, and have the driver handle
the empty data case the same way as the rq=-1 case today?
>>> + /* PXA25x specific map */
>>> + { "pxa25x-ssp.0", "rx", PDMA_FILTER_PARAM(LOWEST, 13) },
>>> + { "pxa25x-ssp.0", "tx", PDMA_FILTER_PARAM(LOWEST, 14) },
>>> + { "pxa25x-nssp.1", "rx", PDMA_FILTER_PARAM(LOWEST, 15) },
>>> + { "pxa25x-nssp.1", "tx", PDMA_FILTER_PARAM(LOWEST, 16) },
>>> + { "pxa25x-nssp.2", "rx", PDMA_FILTER_PARAM(LOWEST, 23) },
>>> + { "pxa25x-nssp.2", "tx", PDMA_FILTER_PARAM(LOWEST, 24) },
>>> + { "pxa-pcm-audio", "nssp2_rx", PDMA_FILTER_PARAM(LOWEST, 15) },
>>> + { "pxa-pcm-audio", "nssp2_tx", PDMA_FILTER_PARAM(LOWEST, 16) },
>>> + { "pxa-pcm-audio", "nssp3_rx", PDMA_FILTER_PARAM(LOWEST, 23) },
>>> + { "pxa-pcm-audio", "nssp3_tx", PDMA_FILTER_PARAM(LOWEST, 24) },
>>> +
>>> + /* PXA27x specific map */
>>> + { "pxa-pcm-audio", "ssp3_rx", PDMA_FILTER_PARAM(LOWEST, 66) },
>>> + { "pxa-pcm-audio", "ssp3_tx", PDMA_FILTER_PARAM(LOWEST, 67) },
>>> + { "pxa27x-camera.0", "CI_Y", PDMA_FILTER_PARAM(HIGHEST, 68) },
>>> + { "pxa27x-camera.0", "CI_U", PDMA_FILTER_PARAM(HIGHEST, 69) },
>>> + { "pxa27x-camera.0", "CI_V", PDMA_FILTER_PARAM(HIGHEST, 70) },
>>> +
>>> + /* PXA3xx specific map */
>>> + { "pxa-pcm-audio", "ssp4_rx", PDMA_FILTER_PARAM(LOWEST, 2) },
>>> + { "pxa-pcm-audio", "ssp4_tx", PDMA_FILTER_PARAM(LOWEST, 3) },
>>> + { "pxa2xx-mci.1", "rx", PDMA_FILTER_PARAM(LOWEST, 93) },
>>> + { "pxa2xx-mci.1", "tx", PDMA_FILTER_PARAM(LOWEST, 94) },
>>> + { "pxa3xx-nand", "data", PDMA_FILTER_PARAM(LOWEST, 97) },
>>> + { "pxa2xx-mci.2", "rx", PDMA_FILTER_PARAM(LOWEST, 100) },
>>> + { "pxa2xx-mci.2", "tx", PDMA_FILTER_PARAM(LOWEST, 101) },
>>> +};
>>
>> Since more than half the entries in here are chip specific, maybe it would be
>> better to split that table into three and have a copy for each one in
>> arch/arm/mach-pxa/pxa{25x.27x.3xx}.c?
> Mmmh, today the split is :
> - 16 common entries
> - 10 pxa25x specific entries
> - 5 pxa27x specific entries
> - 7 pxa3xx specific entries
> => total of 38 lines
>
> After the split we'll have :
> - 26 pxa25x specific entries
> - 21 pxa27x specific entries
> - 23 pxa3xx specific entries
> => total of 70 lines
>
> That doubles the number of lines, not counting the declarations, and amending
> of
> pxa2xx_set_dmac_info().
>
> If you think it's worth it, what is the driving benefit behind ?
It seems a bit cleaner to only register the tables for the dma lines that
are actually present on a given chip.
>> Does that mean it's actually a memory-to-memory transfer with a device being
>> on the external SRAM interface?
> I'm taking this is the follow up to the "-1" question :0
Right.
Arnd
Re: [GIT PULL] Kernel lockdown for secure boot
On Tue, Apr 03, 2018 at 08:11:07AM -0700, Andy Lutomirski wrote: > > > >> "bpf: Restrict kernel image access functions when the kernel is locked > >> down": > >> This patch just sucks in general. > > > > Yes - but that's what Alexei Starovoitov specified. bpf kind of sucks since > > it gives you unrestricted access to the kernel. > > bpf, in certain contexts, gives you unrestricted access to *reading* > kernel memory. bpf should, under no circumstances, let you write to > the kernel unless you're using fault injection or similar. > > I'm surprised that Alexei acked this patch. If something like XDP or > bpfilter starts becoming widely used, this patch will require a lot of > reworking to avoid breaking standard distros. my understanding was that this lockdown set attemps to disallow _reads_ of kernel memory from anything, so first version of patch was adding run-time checks for bpf_probe_read() which is no-go and without this helper the bpf for tracing is losing a lot of its power, so the easiest is to disable it all. I think lockdown suppose to disable xdp, bpfilter, nflog, raw sockets + pcap too otherwise even cap_net_admin can see traffic coming into host. Similarly kprobe, perf_event, ftrace should be off as well?
Re: [PATCH v3 09/14] s390: vfio-ap: sysfs interfaces to configure domains
On 04/03/2018 11:19 AM, Cornelia Huck wrote:
On Tue, 3 Apr 2018 11:12:45 -0400
Tony Krowiak wrote:
On 04/03/2018 07:17 AM, Cornelia Huck wrote:
On Wed, 14 Mar 2018 14:25:49 -0400
Tony Krowiak wrote:
Provides the sysfs interfaces for assigning AP domains to
and unassigning AP domains from a mediated matrix device.
An AP domain ID corresponds to an AP queue index (APQI). For
each domain assigned to the mediated matrix device, its
corresponging APQI is stored in an AP queue mask (AQM).
The bits in the AQM, from most significant to least
significant bit, correspond to AP domain numbers 0 to 255.
When a domain is assigned, the bit corresponding to its
APQI will be set in the AQM. Likewise, when a domain is
unassigned, the bit corresponding to its APQI will be
cleared from the AQM.
The relevant sysfs structures are:
/sys/devices/vfio_ap
... [matrix]
.. [mdev_supported_types]
. [vfio_ap-passthrough]
[devices]
...[$uuid]
.. assign_domain
.. unassign_domain
To assign a domain to the $uuid mediated matrix device,
write the domain's ID to the assign_domain file. To
unassign a domain, write the domain's ID to the
unassign_domain file. The ID is specified using
conventional semantics: If it begins with 0x, the number
will be parsed as a hexadecimal (case insensitive) number;
otherwise, it will be parsed as a decimal number.
For example, to assign domain 173 (0xad) to the mediated matrix
device $uuid:
echo 173 > assign_domain
or
echo 0xad > assign_domain
To unassign domain 173 (0xad):
echo 173 > unassign_domain
or
echo 0xad > unassign_domain
The assignment will be rejected:
* If the domain ID exceeds the maximum value for an AP domain:
* If the AP Extended Addressing (APXA) facility is installed,
the max value is 255
* Else the max value is 15
* If no AP adapters have yet been assigned and there are
no AP queues reserved by the VFIO AP driver that have an APQN
with an APQI matching that of the AP domain number being
assigned.
* If any of the APQNs that can be derived from the intersection
of the APQI being assigned and the AP adapter ID (APID) of
each of the AP adapters previously assigned can not be matched
with an APQN of an AP queue device reserved by the VFIO AP
driver.
Signed-off-by: Tony Krowiak
---
arch/s390/include/asm/kvm-ap.h|1 +
drivers/s390/crypto/vfio_ap_ops.c | 215
-
2 files changed, 215 insertions(+), 1 deletions(-)
diff --git a/drivers/s390/crypto/vfio_ap_ops.c
b/drivers/s390/crypto/vfio_ap_ops.c
index 90512a6..c448835 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -377,10 +377,223 @@ static ssize_t unassign_adapter_store(struct device *dev,
}
DEVICE_ATTR_WO(unassign_adapter);
+/**
+ * vfio_ap_validate_queues_for_apqi
+ *
+ * @ap_matrix: the matrix device
+ * @matrix_mdev: the mediated matrix device
+ * @apqi: an AP queue index (APQI) - corresponds to a domain ID
+ *
+ * Verifies that each APQN that is derived from the intersection of @apqi and
+ * each AP adapter ID (APID) corresponding to an AP domain assigned to the
+ * @matrix_mdev matches the APQN of an AP queue reserved by the VFIO AP device
+ * driver.
+ *
+ * Returns 0 if validation succeeds; otherwise, returns an error.
+ */
+static int vfio_ap_validate_queues_for_apqi(struct ap_matrix *ap_matrix,
+ struct ap_matrix_mdev *matrix_mdev,
+ unsigned long apqi)
+{
+ int ret;
+ struct vfio_ap_qid_match qid_match;
+ unsigned long apid;
+ struct device_driver *drv = ap_matrix->device.driver;
+
+ /**
+* Examine each APQN with the specified APQI
+*/
+ for_each_set_bit_inv(apid, matrix_mdev->matrix->apm,
+matrix_mdev->matrix->apm_max) {
+ qid_match.qid = AP_MKQID(apid, apqi);
+ qid_match.dev = NULL;
+
+ ret = driver_for_each_device(drv, NULL, _match,
+vfio_ap_queue_match);
+ if (ret)
+ return ret;
Hm, I'm wondering whether jumping out of the outer loop is the correct
thing to do here - and if yes, whether we should log an error?
If you look at the vfio_ap_queue_match() function which is passed to the
driver_for_each_device() function, it never returns an error. The
driver_for_each_device() function only returns an error if the function
passed in returns an error, so in reality, the value of *ret *will never
be anything but 0. Having said that, there are no guarantees that the
vfio_ap_queue_match() function will never change, so it would probably
be a good idea to log an error if *ret *is not 0.**I think returning at
this point is valid because a non-zero is returned from
Re: [PATCH v2 11/17] kvm: arm64: Configure VTCR per VM
On 03/04/18 15:58, James Morse wrote:
Hi Suzuki,
On 27/03/18 14:15, Suzuki K Poulose wrote:
We set VTCR_EL2 very early during the stage2 init and don't
touch it ever. This is fine as we had a fixed IPA size. This
patch changes the behavior to set the VTCR for a given VM,
depending on its stage2 table. The common configuration for
VTCR is still performed during the early init as we have to
retain the hardware access flag update bits (VTCR_EL2_HA)
per CPU (as they are only set for the CPUs which are capabile).
(Nit: capable)
Thanks for spotting, will fix it.
The bits defining the number of levels in the page table (SL0)
and and the size of the Input address to the translation (T0SZ)
are programmed for each VM upon entry to the guest.
diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index 870f4b1..5ccd3ae 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -164,6 +164,12 @@ static void __hyp_text __deactivate_traps(struct kvm_vcpu
*vcpu)
static void __hyp_text __activate_vm(struct kvm_vcpu *vcpu)
{
struct kvm *kvm = kern_hyp_va(vcpu->kvm);
+ u64 vtcr = read_sysreg(vtcr_el2);
+
+ vtcr &= ~VTCR_EL2_PRIVATE_MASK;
+ vtcr |= VTCR_EL2_SL0(kvm_stage2_levels(kvm)) |
+ VTCR_EL2_T0SZ(kvm_phys_shift(kvm));
+ write_sysreg(vtcr, vtcr_el2);
write_sysreg(kvm->arch.vttbr, vttbr_el2);
}
Do we need to set this register for tlb maintenance too?
e.g. tlbi for a 3-level-stage2 vm when a 2-level-stage2 vm's vtcr is loaded...
(The ARM-ARM has 'Any of the bits of VTCR_EL2 are permitted to be cached in a
TLB'.)
You're right. We need to set the VTCR for the tlb operations. I think
we can do this by hooking it to the __tlb_switch_to_guest() routine.
Will address it in the next version.
Cheers
Suzuki
Re: [PATCH v2] x86/xen/efi: Initialize UEFI secure boot state during dom0 boot
On Tue, 2018-04-03 at 16:39 +0200, Daniel Kiper wrote:
> Initialize UEFI secure boot state during dom0 boot. Otherwise the
> kernel
> may not even know that it runs on secure boot enabled platform.
>
> Signed-off-by: Daniel Kiper
> ---
> arch/x86/xen/efi.c| 57
> +
> drivers/firmware/efi/libstub/secureboot.c |3 ++
> 2 files changed, 60 insertions(+)
>
> diff --git a/arch/x86/xen/efi.c b/arch/x86/xen/efi.c
> index a18703b..1804b27 100644
> --- a/arch/x86/xen/efi.c
> +++ b/arch/x86/xen/efi.c
> @@ -115,6 +115,61 @@ static efi_system_table_t __init
> *xen_efi_probe(void)
> return _systab_xen;
> }
>
> +/*
> + * Determine whether we're in secure boot mode.
> + *
> + * Please keep the logic in sync with
> + * drivers/firmware/efi/libstub/secureboot.c:efi_get_secureboot().
> + */
> +static enum efi_secureboot_mode xen_efi_get_secureboot(void)
> +{
> + static efi_guid_t efi_variable_guid =
> EFI_GLOBAL_VARIABLE_GUID;
> + static efi_guid_t shim_guid = EFI_SHIM_LOCK_GUID;
> + efi_status_t status;
> + u8 moksbstate, secboot, setupmode;
> + unsigned long size;
> +
> + size = sizeof(secboot);
> + status = efi.get_variable(L"SecureBoot", _variable_guid,
> + NULL, , );
> +
> + if (status == EFI_NOT_FOUND)
> + return efi_secureboot_mode_disabled;
> +
> + if (status != EFI_SUCCESS)
> + goto out_efi_err;
> +
> + size = sizeof(setupmode);
> + status = efi.get_variable(L"SetupMode", _variable_guid,
> + NULL, , );
> +
> + if (status != EFI_SUCCESS)
> + goto out_efi_err;
> +
> + if (secboot == 0 || setupmode == 1)
> + return efi_secureboot_mode_disabled;
> +
> + /* See if a user has put the shim into insecure mode. */
> + size = sizeof(moksbstate);
> + status = efi.get_variable(L"MokSBStateRT", _guid,
> + NULL, , );
> +
> + /* If it fails, we don't care why. Default to secure. */
> + if (status != EFI_SUCCESS)
> + goto secure_boot_enabled;
> +
> + if (moksbstate == 1)
> + return efi_secureboot_mode_disabled;
> +
> + secure_boot_enabled:
> + pr_info("UEFI Secure Boot is enabled.\n");
> + return efi_secureboot_mode_enabled;
> +
> + out_efi_err:
> + pr_err("Could not determine UEFI Secure Boot status.\n");
> + return efi_secureboot_mode_unknown;
> +}
> +
This looks like a bad idea: you're duplicating the secure boot check in
drivers/firmware/efi/libstub/secureboot.c
Which is an implementation of policy. If we have to have policy in the
kernel, it should really only be in one place to prevent drift; why
can't you simply use the libstub efi_get_secureboot() so we're not
duplicating the implementation of policy?
James
[PATCH v3 6/6] spi: sun6i: add DMA transfers support
DMA transfers are now available for sun6i and sun8i SoCs.
The DMA mode is used automatically as soon as requested
transfer length is more than FIFO length.
Changes in v3:
1) Debug log enhancements.
Signed-off-by: Sergey Suloev
---
drivers/spi/spi-sun6i.c | 331 ++--
1 file changed, 294 insertions(+), 37 deletions(-)
diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c
index 2fa9d6e..7f41871 100644
--- a/drivers/spi/spi-sun6i.c
+++ b/drivers/spi/spi-sun6i.c
@@ -14,6 +14,8 @@
#include
#include
#include
+#include
+#include
#include
#include
#include
@@ -55,17 +57,20 @@
#define SUN6I_FIFO_CTL_REG 0x18
#define SUN6I_FIFO_CTL_RF_RDY_TRIG_LEVEL_MASK 0xff
-#define SUN6I_FIFO_CTL_RF_RDY_TRIG_LEVEL_BITS 0
+#define SUN6I_FIFO_CTL_RF_RDY_TRIG_LEVEL_POS 0
+#define SUN6I_FIFO_CTL_RF_DRQ_EN BIT(8)
#define SUN6I_FIFO_CTL_RF_RST BIT(15)
#define SUN6I_FIFO_CTL_TF_ERQ_TRIG_LEVEL_MASK 0xff
-#define SUN6I_FIFO_CTL_TF_ERQ_TRIG_LEVEL_BITS 16
+#define SUN6I_FIFO_CTL_TF_ERQ_TRIG_LEVEL_POS 16
+#define SUN6I_FIFO_CTL_TF_DRQ_EN BIT(24)
#define SUN6I_FIFO_CTL_TF_RST BIT(31)
+#define SUN6I_FIFO_CTL_DMA_DEDICATEBIT(9)|BIT(25)
#define SUN6I_FIFO_STA_REG 0x1c
#define SUN6I_FIFO_STA_RF_CNT_MASK 0x7f
-#define SUN6I_FIFO_STA_RF_CNT_BITS 0
+#define SUN6I_FIFO_STA_RF_CNT_POS 0
#define SUN6I_FIFO_STA_TF_CNT_MASK 0x7f
-#define SUN6I_FIFO_STA_TF_CNT_BITS 16
+#define SUN6I_FIFO_STA_TF_CNT_POS 16
#define SUN6I_CLK_CTL_REG 0x24
#define SUN6I_CLK_CTL_CDR2_MASK0xff
@@ -135,7 +140,7 @@ static inline u32 sun6i_spi_get_tx_fifo_count(struct
sun6i_spi *sspi)
{
u32 reg = sun6i_spi_read(sspi, SUN6I_FIFO_STA_REG);
- reg >>= SUN6I_FIFO_STA_TF_CNT_BITS;
+ reg >>= SUN6I_FIFO_STA_TF_CNT_POS;
return reg & SUN6I_FIFO_STA_TF_CNT_MASK;
}
@@ -148,7 +153,7 @@ static inline void sun6i_spi_drain_fifo(struct sun6i_spi
*sspi, int len)
/* See how much data is available */
reg = sun6i_spi_read(sspi, SUN6I_FIFO_STA_REG);
reg &= SUN6I_FIFO_STA_RF_CNT_MASK;
- cnt = reg >> SUN6I_FIFO_STA_RF_CNT_BITS;
+ cnt = reg >> SUN6I_FIFO_STA_RF_CNT_POS;
if (len > cnt)
len = cnt;
@@ -177,6 +182,15 @@ static inline void sun6i_spi_fill_fifo(struct sun6i_spi
*sspi, int len)
}
}
+static bool sun6i_spi_can_dma(struct spi_master *master,
+ struct spi_device *spi,
+ struct spi_transfer *tfr)
+{
+ struct sun6i_spi *sspi = spi_master_get_devdata(master);
+
+ return tfr->len > sspi->fifo_depth;
+}
+
static void sun6i_spi_set_cs(struct spi_device *spi, bool enable)
{
struct sun6i_spi *sspi = spi_master_get_devdata(spi->master);
@@ -208,6 +222,9 @@ static size_t sun6i_spi_max_transfer_size(struct spi_device
*spi)
struct spi_master *master = spi->master;
struct sun6i_spi *sspi = spi_master_get_devdata(master);
+ if (master->can_dma)
+ return SUN6I_MAX_XFER_SIZE;
+
return sspi->fifo_depth;
}
@@ -268,16 +285,187 @@ static int sun6i_spi_wait_for_transfer(struct spi_device
*spi,
return 0;
}
+static void sun6i_spi_dma_callback(void *param)
+{
+ struct spi_master *master = param;
+
+ dev_dbg(>dev, "DMA transfer complete\n");
+ spi_finalize_current_transfer(master);
+}
+
+static int sun6i_spi_dmap_prep_tx(struct spi_master *master,
+ struct spi_transfer *tfr,
+ dma_cookie_t *cookie)
+{
+ struct dma_async_tx_descriptor *chan_desc = NULL;
+
+ chan_desc = dmaengine_prep_slave_sg(master->dma_tx,
+ tfr->tx_sg.sgl, tfr->tx_sg.nents,
+ DMA_TO_DEVICE,
+ DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+ if (!chan_desc) {
+ dev_err(>dev,
+ "Couldn't prepare TX DMA slave\n");
+ return -EIO;
+ }
+
+ chan_desc->callback = sun6i_spi_dma_callback;
+ chan_desc->callback_param = master;
+
+ *cookie = dmaengine_submit(chan_desc);
+ dma_async_issue_pending(master->dma_tx);
+
+ return 0;
+}
+
+static int sun6i_spi_dmap_prep_rx(struct spi_master *master,
+ struct spi_transfer *tfr,
+ dma_cookie_t *cookie)
+{
+ struct dma_async_tx_descriptor *chan_desc = NULL;
+
+ chan_desc = dmaengine_prep_slave_sg(master->dma_rx,
+ tfr->rx_sg.sgl, tfr->rx_sg.nents,
+ DMA_FROM_DEVICE,
+ DMA_PREP_INTERRUPT |
[PATCH v3 5/6] spi: sun6i: introduce register set/unset helpers
Two helper functions were added in order to set/unset
specified flags in registers.
Signed-off-by: Sergey Suloev
---
drivers/spi/spi-sun6i.c | 37 -
1 file changed, 16 insertions(+), 21 deletions(-)
diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c
index 0912404..2fa9d6e 100644
--- a/drivers/spi/spi-sun6i.c
+++ b/drivers/spi/spi-sun6i.c
@@ -115,29 +115,29 @@ static inline void sun6i_spi_write(struct sun6i_spi
*sspi, u32 reg, u32 value)
writel(value, sspi->base_addr + reg);
}
-static inline u32 sun6i_spi_get_tx_fifo_count(struct sun6i_spi *sspi)
+static inline void sun6i_spi_set(struct sun6i_spi *sspi, u32 addr, u32 val)
{
- u32 reg = sun6i_spi_read(sspi, SUN6I_FIFO_STA_REG);
-
- reg >>= SUN6I_FIFO_STA_TF_CNT_BITS;
+ u32 reg = sun6i_spi_read(sspi, addr);
- return reg & SUN6I_FIFO_STA_TF_CNT_MASK;
+ reg |= val;
+ sun6i_spi_write(sspi, addr, reg);
}
-static inline void sun6i_spi_enable_interrupt(struct sun6i_spi *sspi, u32 mask)
+static inline void sun6i_spi_unset(struct sun6i_spi *sspi, u32 addr, u32 val)
{
- u32 reg = sun6i_spi_read(sspi, SUN6I_INT_CTL_REG);
+ u32 reg = sun6i_spi_read(sspi, addr);
- reg |= mask;
- sun6i_spi_write(sspi, SUN6I_INT_CTL_REG, reg);
+ reg &= ~val;
+ sun6i_spi_write(sspi, addr, reg);
}
-static inline void sun6i_spi_disable_interrupt(struct sun6i_spi *sspi, u32
mask)
+static inline u32 sun6i_spi_get_tx_fifo_count(struct sun6i_spi *sspi)
{
- u32 reg = sun6i_spi_read(sspi, SUN6I_INT_CTL_REG);
+ u32 reg = sun6i_spi_read(sspi, SUN6I_FIFO_STA_REG);
- reg &= ~mask;
- sun6i_spi_write(sspi, SUN6I_INT_CTL_REG, reg);
+ reg >>= SUN6I_FIFO_STA_TF_CNT_BITS;
+
+ return reg & SUN6I_FIFO_STA_TF_CNT_MASK;
}
static inline void sun6i_spi_drain_fifo(struct sun6i_spi *sspi, int len)
@@ -310,18 +310,14 @@ static int sun6i_spi_transfer_one(struct spi_master
*master,
sun6i_spi_write(sspi, SUN6I_FIFO_CTL_REG,
(trig_level << SUN6I_FIFO_CTL_RF_RDY_TRIG_LEVEL_BITS));
-
- reg = sun6i_spi_read(sspi, SUN6I_TFR_CTL_REG);
/*
* If it's a TX only transfer, we don't want to fill the RX
* FIFO with bogus data
*/
if (sspi->rx_buf)
- reg &= ~SUN6I_TFR_CTL_DHB;
+ sun6i_spi_unset(sspi, SUN6I_TFR_CTL_REG, SUN6I_TFR_CTL_DHB);
else
- reg |= SUN6I_TFR_CTL_DHB;
-
- sun6i_spi_write(sspi, SUN6I_TFR_CTL_REG, reg);
+ sun6i_spi_set(sspi, SUN6I_TFR_CTL_REG, SUN6I_TFR_CTL_DHB);
/* Ensure that we have a parent clock fast enough */
@@ -376,8 +372,7 @@ static int sun6i_spi_transfer_one(struct spi_master *master,
SUN6I_INT_CTL_RF_RDY);
/* Start the transfer */
- reg = sun6i_spi_read(sspi, SUN6I_TFR_CTL_REG);
- sun6i_spi_write(sspi, SUN6I_TFR_CTL_REG, reg | SUN6I_TFR_CTL_XCH);
+ sun6i_spi_set(sspi, SUN6I_TFR_CTL_REG, SUN6I_TFR_CTL_XCH);
/* Wait for completion */
ret = sun6i_spi_wait_for_transfer(spi, tfr);
--
2.16.2
[PATCH] rapidio: use a reference count for struct mport_dma_req
Once the dma request is passed to the DMA engine, the DMA
subsystem would hold a pointer to this structure and could
call the completion callback after do_dma_request() has
timed out.
The current code deals with this by putting timed out SYNC
requests to a pending list and freeing them later, when the
mport cdev device is released. But this still does not
guarantee that the DMA subsystem is really done with those
transfers, so in theory dma_xfer_callback/dma_req_free
could be called after mport_cdev_release_dma and could
potentially access already freed memory.
This patch simplifies the current handling by using a kref
in the mport dma request structure, so that it gets freed
only when nobody uses it anymore.
This also simplifies the code a bit, as FAF transfers are
now handled in the same way as SYNC and ASYNC transfers.
There is no need anymore for the pending list and for the
dma workqueue which was used in case of FAF transfers, so
we remove them both.
Signed-off-by: Ioan Nicu
---
drivers/rapidio/devices/rio_mport_cdev.c | 122 +--
1 file changed, 18 insertions(+), 104 deletions(-)
diff --git a/drivers/rapidio/devices/rio_mport_cdev.c
b/drivers/rapidio/devices/rio_mport_cdev.c
index cfb54e01d758..9d27016c899e 100644
--- a/drivers/rapidio/devices/rio_mport_cdev.c
+++ b/drivers/rapidio/devices/rio_mport_cdev.c
@@ -212,7 +212,6 @@ struct mport_cdev_priv {
#ifdef CONFIG_RAPIDIO_DMA_ENGINE
struct dma_chan *dmach;
struct list_headasync_list;
- struct list_headpend_list;
spinlock_t req_lock;
struct mutexdma_lock;
struct kref dma_ref;
@@ -258,8 +257,6 @@ static DECLARE_WAIT_QUEUE_HEAD(mport_cdev_wait);
static struct class *dev_class;
static dev_t dev_number;
-static struct workqueue_struct *dma_wq;
-
static void mport_release_mapping(struct kref *ref);
static int rio_mport_maint_rd(struct mport_cdev_priv *priv, void __user *arg,
@@ -539,6 +536,7 @@ static int maint_comptag_set(struct mport_cdev_priv *priv,
void __user *arg)
#ifdef CONFIG_RAPIDIO_DMA_ENGINE
struct mport_dma_req {
+ struct kref refcount;
struct list_head node;
struct file *filp;
struct mport_cdev_priv *priv;
@@ -554,11 +552,6 @@ struct mport_dma_req {
struct completion req_comp;
};
-struct mport_faf_work {
- struct work_struct work;
- struct mport_dma_req *req;
-};
-
static void mport_release_def_dma(struct kref *dma_ref)
{
struct mport_dev *md =
@@ -578,8 +571,10 @@ static void mport_release_dma(struct kref *dma_ref)
complete(>comp);
}
-static void dma_req_free(struct mport_dma_req *req)
+static void dma_req_free(struct kref *ref)
{
+ struct mport_dma_req *req = container_of(ref, struct mport_dma_req,
+ refcount);
struct mport_cdev_priv *priv = req->priv;
unsigned int i;
@@ -611,30 +606,7 @@ static void dma_xfer_callback(void *param)
req->status = dma_async_is_tx_complete(priv->dmach, req->cookie,
NULL, NULL);
complete(>req_comp);
-}
-
-static void dma_faf_cleanup(struct work_struct *_work)
-{
- struct mport_faf_work *work = container_of(_work,
- struct mport_faf_work, work);
- struct mport_dma_req *req = work->req;
-
- dma_req_free(req);
- kfree(work);
-}
-
-static void dma_faf_callback(void *param)
-{
- struct mport_dma_req *req = (struct mport_dma_req *)param;
- struct mport_faf_work *work;
-
- work = kmalloc(sizeof(*work), GFP_ATOMIC);
- if (!work)
- return;
-
- INIT_WORK(>work, dma_faf_cleanup);
- work->req = req;
- queue_work(dma_wq, >work);
+ kref_put(>refcount, dma_req_free);
}
/*
@@ -765,16 +737,14 @@ static int do_dma_request(struct mport_dma_req *req,
goto err_out;
}
- if (sync == RIO_TRANSFER_FAF)
- tx->callback = dma_faf_callback;
- else
- tx->callback = dma_xfer_callback;
+ tx->callback = dma_xfer_callback;
tx->callback_param = req;
req->dmach = chan;
req->sync = sync;
req->status = DMA_IN_PROGRESS;
init_completion(>req_comp);
+ kref_get(>refcount);
cookie = dmaengine_submit(tx);
req->cookie = cookie;
@@ -785,6 +755,7 @@ static int do_dma_request(struct mport_dma_req *req,
if (dma_submit_error(cookie)) {
rmcd_error("submit err=%d (addr:0x%llx len:0x%llx)",
cookie, xfer->rio_addr, xfer->length);
+ kref_put(>refcount, dma_req_free);
ret = -EIO;
goto err_out;
}
@@ -860,6 +831,8 @@ rio_dma_transfer(struct file *filp, u32 transfer_mode,
if (!req)
return -ENOMEM;
+ kref_init(>refcount);
+
[PATCH v3 3/6] spi: sun6i: restrict transfer length in PIO-mode
There is no need to handle 3/4 empty interrupt as the maximum
supported transfer length in PIO mode is equal to FIFO depth,
i.e. 128 bytes for sun6i and 64 bytes for sun8i SoCs.
Changes in v3:
1) Restored processing of 3/4 FIFO full interrupt.
Signed-off-by: Sergey Suloev
---
drivers/spi/spi-sun6i.c | 41 +
1 file changed, 17 insertions(+), 24 deletions(-)
diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c
index 78acc1f..c09ad10 100644
--- a/drivers/spi/spi-sun6i.c
+++ b/drivers/spi/spi-sun6i.c
@@ -207,7 +207,10 @@ static void sun6i_spi_set_cs(struct spi_device *spi, bool
enable)
static size_t sun6i_spi_max_transfer_size(struct spi_device *spi)
{
- return SUN6I_MAX_XFER_SIZE - 1;
+ struct spi_master *master = spi->master;
+ struct sun6i_spi *sspi = spi_master_get_devdata(master);
+
+ return sspi->fifo_depth;
}
static int sun6i_spi_prepare_message(struct spi_master *master,
@@ -255,8 +258,14 @@ static int sun6i_spi_transfer_one(struct spi_master
*master,
int ret = 0;
u32 reg;
- if (tfr->len > SUN6I_MAX_XFER_SIZE)
- return -EINVAL;
+ /* A zero length transfer never finishes if programmed
+ in the hardware */
+ if (!tfr->len)
+ return 0;
+
+ /* Don't support transfer larger than the FIFO */
+ if (tfr->len > sspi->fifo_depth)
+ return -EMSGSIZE;
reinit_completion(>done);
sspi->tx_buf = tfr->tx_buf;
@@ -278,8 +287,7 @@ static int sun6i_spi_transfer_one(struct spi_master *master,
*/
trig_level = sspi->fifo_depth / 4 * 3;
sun6i_spi_write(sspi, SUN6I_FIFO_CTL_REG,
- (trig_level << SUN6I_FIFO_CTL_RF_RDY_TRIG_LEVEL_BITS) |
- (trig_level << SUN6I_FIFO_CTL_TF_ERQ_TRIG_LEVEL_BITS));
+ (trig_level << SUN6I_FIFO_CTL_RF_RDY_TRIG_LEVEL_BITS));
reg = sun6i_spi_read(sspi, SUN6I_TFR_CTL_REG);
@@ -343,11 +351,8 @@ static int sun6i_spi_transfer_one(struct spi_master
*master,
sun6i_spi_fill_fifo(sspi, sspi->fifo_depth);
/* Enable the interrupts */
- sun6i_spi_write(sspi, SUN6I_INT_CTL_REG, SUN6I_INT_CTL_TC);
sun6i_spi_enable_interrupt(sspi, SUN6I_INT_CTL_TC |
SUN6I_INT_CTL_RF_RDY);
- if (tx_len > sspi->fifo_depth)
- sun6i_spi_enable_interrupt(sspi, SUN6I_INT_CTL_TF_ERQ);
/* Start the transfer */
reg = sun6i_spi_read(sspi, SUN6I_TFR_CTL_REG);
@@ -376,7 +381,9 @@ out:
static irqreturn_t sun6i_spi_handler(int irq, void *dev_id)
{
struct sun6i_spi *sspi = dev_id;
- u32 status = sun6i_spi_read(sspi, SUN6I_INT_STA_REG);
+ u32 status;
+
+ status = sun6i_spi_read(sspi, SUN6I_INT_STA_REG);
/* Transfer complete */
if (status & SUN6I_INT_CTL_TC) {
@@ -388,26 +395,12 @@ static irqreturn_t sun6i_spi_handler(int irq, void
*dev_id)
/* Receive FIFO 3/4 full */
if (status & SUN6I_INT_CTL_RF_RDY) {
- sun6i_spi_drain_fifo(sspi, SUN6I_FIFO_DEPTH);
+ sun6i_spi_drain_fifo(sspi, sspi->fifo_depth);
/* Only clear the interrupt _after_ draining the FIFO */
sun6i_spi_write(sspi, SUN6I_INT_STA_REG, SUN6I_INT_CTL_RF_RDY);
return IRQ_HANDLED;
}
- /* Transmit FIFO 3/4 empty */
- if (status & SUN6I_INT_CTL_TF_ERQ) {
- sun6i_spi_fill_fifo(sspi, SUN6I_FIFO_DEPTH);
-
- if (!sspi->len)
- /* nothing left to transmit */
- sun6i_spi_disable_interrupt(sspi, SUN6I_INT_CTL_TF_ERQ);
-
- /* Only clear the interrupt _after_ re-seeding the FIFO */
- sun6i_spi_write(sspi, SUN6I_INT_STA_REG, SUN6I_INT_CTL_TF_ERQ);
-
- return IRQ_HANDLED;
- }
-
return IRQ_NONE;
}
--
2.16.2
[PATCH v3 0/6] spi: Add support for DMA transfers in sun6i SPI driver
The following patchset provides corrections for PIO-mode and support for DMA transfers in sun6i SPI driver. Changes in v2: 1) Fixed issue with misplacing a piece of code that requires access to the transfer structure into sun6i_spi_prepare_message() function where the transfer structure is not available. 2) Fixed issue with passing an invalid argument into devm_request_irq() function. Changes in v3: 1) Restored processing of 3/4 FIFO full interrupt. 2) Debug log enhancements. Sergey Suloev (6): spi: sun6i: coding style/readability improvements spi: sun6i: handle chip select polarity flag spi: sun6i: restrict transfer length in PIO-mode spi: sun6i: use completion provided by SPI core spi: sun6i: introduce register set/unset helpers spi: sun6i: add DMA transfers support drivers/spi/spi-sun6i.c | 526 1 file changed, 402 insertions(+), 124 deletions(-) -- 2.16.2
[PATCH v3 4/6] spi: sun6i: use completion provided by SPI core
As long as the completion is already provided by the SPI core
then there is no need to waste extra-memory on this.
Also a waiting function was added to avoid code duplication.
Changes in v2:
1) Fixed issue with passing an invalid argument into devm_request_irq()
function.
Signed-off-by: Sergey Suloev
---
drivers/spi/spi-sun6i.c | 52 -
1 file changed, 30 insertions(+), 22 deletions(-)
diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c
index c09ad10..0912404 100644
--- a/drivers/spi/spi-sun6i.c
+++ b/drivers/spi/spi-sun6i.c
@@ -99,8 +99,6 @@ struct sun6i_spi {
struct clk *mclk;
struct reset_control*rstc;
- struct completion done;
-
const u8*tx_buf;
u8 *rx_buf;
int len;
@@ -246,6 +244,30 @@ static int sun6i_spi_prepare_message(struct spi_master
*master,
return 0;
}
+static int sun6i_spi_wait_for_transfer(struct spi_device *spi,
+ struct spi_transfer *tfr)
+{
+ struct spi_master *master = spi->master;
+ unsigned int start, end, tx_time;
+ unsigned int timeout;
+
+ /* smart wait for completion */
+ tx_time = max(tfr->len * 8 * 2 / (tfr->speed_hz / 1000), 100U);
+ start = jiffies;
+ timeout = wait_for_completion_timeout(>xfer_completion,
+ msecs_to_jiffies(tx_time));
+ end = jiffies;
+ if (!timeout) {
+ dev_warn(>dev,
+"%s: timeout transferring %u bytes@%iHz for %i(%i)ms",
+dev_name(>dev), tfr->len, tfr->speed_hz,
+jiffies_to_msecs(end - start), tx_time);
+ return -ETIMEDOUT;
+ }
+
+ return 0;
+}
+
static int sun6i_spi_transfer_one(struct spi_master *master,
struct spi_device *spi,
struct spi_transfer *tfr)
@@ -267,7 +289,6 @@ static int sun6i_spi_transfer_one(struct spi_master *master,
if (tfr->len > sspi->fifo_depth)
return -EMSGSIZE;
- reinit_completion(>done);
sspi->tx_buf = tfr->tx_buf;
sspi->rx_buf = tfr->rx_buf;
sspi->len = tfr->len;
@@ -358,21 +379,9 @@ static int sun6i_spi_transfer_one(struct spi_master
*master,
reg = sun6i_spi_read(sspi, SUN6I_TFR_CTL_REG);
sun6i_spi_write(sspi, SUN6I_TFR_CTL_REG, reg | SUN6I_TFR_CTL_XCH);
- tx_time = max(tfr->len * 8 * 2 / (tfr->speed_hz / 1000), 100U);
- start = jiffies;
- timeout = wait_for_completion_timeout(>done,
- msecs_to_jiffies(tx_time));
- end = jiffies;
- if (!timeout) {
- dev_warn(>dev,
-"%s: timeout transferring %u bytes@%iHz for %i(%i)ms",
-dev_name(>dev), tfr->len, tfr->speed_hz,
-jiffies_to_msecs(end - start), tx_time);
- ret = -ETIMEDOUT;
- goto out;
- }
+ /* Wait for completion */
+ ret = sun6i_spi_wait_for_transfer(spi, tfr);
-out:
sun6i_spi_write(sspi, SUN6I_INT_CTL_REG, 0);
return ret;
@@ -380,7 +389,8 @@ out:
static irqreturn_t sun6i_spi_handler(int irq, void *dev_id)
{
- struct sun6i_spi *sspi = dev_id;
+ struct spi_master *master = dev_id;
+ struct sun6i_spi *sspi = spi_master_get_devdata(master);
u32 status;
status = sun6i_spi_read(sspi, SUN6I_INT_STA_REG);
@@ -389,7 +399,7 @@ static irqreturn_t sun6i_spi_handler(int irq, void *dev_id)
if (status & SUN6I_INT_CTL_TC) {
sun6i_spi_write(sspi, SUN6I_INT_STA_REG, SUN6I_INT_CTL_TC);
sun6i_spi_drain_fifo(sspi, sspi->fifo_depth);
- complete(>done);
+ spi_finalize_current_transfer(master);
return IRQ_HANDLED;
}
@@ -496,7 +506,7 @@ static int sun6i_spi_probe(struct platform_device *pdev)
}
ret = devm_request_irq(>dev, irq, sun6i_spi_handler,
- 0, dev_name(>dev), sspi);
+ 0, dev_name(>dev), master);
if (ret) {
dev_err(>dev, "Cannot request IRQ\n");
goto err_free_master;
@@ -518,8 +528,6 @@ static int sun6i_spi_probe(struct platform_device *pdev)
goto err_free_master;
}
- init_completion(>done);
-
sspi->rstc = devm_reset_control_get_exclusive(>dev, NULL);
if (IS_ERR(sspi->rstc)) {
dev_err(>dev, "Couldn't get reset controller\n");
--
2.16.2
[PATCH v3 2/6] spi: sun6i: handle chip select polarity flag
The chip select polarity flag is declared as supported but is not handled in the code. Signed-off-by: Sergey Suloev --- drivers/spi/spi-sun6i.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c index 88ad45e..78acc1f 100644 --- a/drivers/spi/spi-sun6i.c +++ b/drivers/spi/spi-sun6i.c @@ -193,6 +193,12 @@ static void sun6i_spi_set_cs(struct spi_device *spi, bool enable) else reg &= ~SUN6I_TFR_CTL_CS_LEVEL; + /* Handle chip select "reverse" polarity */ + if (spi->mode & SPI_CS_HIGH) + reg &= ~SUN6I_TFR_CTL_SPOL; + else + reg |= SUN6I_TFR_CTL_SPOL; + /* We want to control the chip select manually */ reg |= SUN6I_TFR_CTL_CS_MANUAL; -- 2.16.2
[PATCH v3 1/6] spi: sun6i: coding style/readability improvements
Minor changes to fulfill the coding style and improve
the readability of the code.
Changes in v2:
1) Fixed issue with misplacing a piece of code that requires access
to the transfer structure into sun6i_spi_prepare_message() function
where the transfer structure is not available.
Signed-off-by: Sergey Suloev
---
drivers/spi/spi-sun6i.c | 97 +
1 file changed, 58 insertions(+), 39 deletions(-)
diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c
index 8533f4e..88ad45e 100644
--- a/drivers/spi/spi-sun6i.c
+++ b/drivers/spi/spi-sun6i.c
@@ -88,8 +88,12 @@
#define SUN6I_TXDATA_REG 0x200
#define SUN6I_RXDATA_REG 0x300
+#define SUN6I_SPI_MODE_BITS(SPI_CPOL | SPI_CPHA | SPI_CS_HIGH |
SPI_LSB_FIRST)
+
+#define SUN6I_SPI_MAX_SPEED_HZ 1
+#define SUN6I_SPI_MIN_SPEED_HZ 3000
+
struct sun6i_spi {
- struct spi_master *master;
void __iomem*base_addr;
struct clk *hclk;
struct clk *mclk;
@@ -189,6 +193,9 @@ static void sun6i_spi_set_cs(struct spi_device *spi, bool
enable)
else
reg &= ~SUN6I_TFR_CTL_CS_LEVEL;
+ /* We want to control the chip select manually */
+ reg |= SUN6I_TFR_CTL_CS_MANUAL;
+
sun6i_spi_write(sspi, SUN6I_TFR_CTL_REG, reg);
}
@@ -197,6 +204,39 @@ static size_t sun6i_spi_max_transfer_size(struct
spi_device *spi)
return SUN6I_MAX_XFER_SIZE - 1;
}
+static int sun6i_spi_prepare_message(struct spi_master *master,
+struct spi_message *msg)
+{
+ struct spi_device *spi = msg->spi;
+ struct sun6i_spi *sspi = spi_master_get_devdata(master);
+ u32 reg;
+
+ /*
+* Setup the transfer control register: Chip Select,
+* polarities, etc.
+*/
+ reg = sun6i_spi_read(sspi, SUN6I_TFR_CTL_REG);
+
+ if (spi->mode & SPI_CPOL)
+ reg |= SUN6I_TFR_CTL_CPOL;
+ else
+ reg &= ~SUN6I_TFR_CTL_CPOL;
+
+ if (spi->mode & SPI_CPHA)
+ reg |= SUN6I_TFR_CTL_CPHA;
+ else
+ reg &= ~SUN6I_TFR_CTL_CPHA;
+
+ if (spi->mode & SPI_LSB_FIRST)
+ reg |= SUN6I_TFR_CTL_FBS;
+ else
+ reg &= ~SUN6I_TFR_CTL_FBS;
+
+ sun6i_spi_write(sspi, SUN6I_TFR_CTL_REG, reg);
+
+ return 0;
+}
+
static int sun6i_spi_transfer_one(struct spi_master *master,
struct spi_device *spi,
struct spi_transfer *tfr)
@@ -235,27 +275,8 @@ static int sun6i_spi_transfer_one(struct spi_master
*master,
(trig_level << SUN6I_FIFO_CTL_RF_RDY_TRIG_LEVEL_BITS) |
(trig_level << SUN6I_FIFO_CTL_TF_ERQ_TRIG_LEVEL_BITS));
- /*
-* Setup the transfer control register: Chip Select,
-* polarities, etc.
-*/
- reg = sun6i_spi_read(sspi, SUN6I_TFR_CTL_REG);
-
- if (spi->mode & SPI_CPOL)
- reg |= SUN6I_TFR_CTL_CPOL;
- else
- reg &= ~SUN6I_TFR_CTL_CPOL;
-
- if (spi->mode & SPI_CPHA)
- reg |= SUN6I_TFR_CTL_CPHA;
- else
- reg &= ~SUN6I_TFR_CTL_CPHA;
-
- if (spi->mode & SPI_LSB_FIRST)
- reg |= SUN6I_TFR_CTL_FBS;
- else
- reg &= ~SUN6I_TFR_CTL_FBS;
+ reg = sun6i_spi_read(sspi, SUN6I_TFR_CTL_REG);
/*
* If it's a TX only transfer, we don't want to fill the RX
* FIFO with bogus data
@@ -265,11 +286,9 @@ static int sun6i_spi_transfer_one(struct spi_master
*master,
else
reg |= SUN6I_TFR_CTL_DHB;
- /* We want to control the chip select manually */
- reg |= SUN6I_TFR_CTL_CS_MANUAL;
-
sun6i_spi_write(sspi, SUN6I_TFR_CTL_REG, reg);
+
/* Ensure that we have a parent clock fast enough */
mclk_rate = clk_get_rate(sspi->mclk);
if (mclk_rate < (2 * tfr->speed_hz)) {
@@ -442,12 +461,24 @@ static int sun6i_spi_probe(struct platform_device *pdev)
struct resource *res;
int ret = 0, irq;
- master = spi_alloc_master(>dev, sizeof(struct sun6i_spi));
+ master = spi_alloc_master(>dev, sizeof(*sspi));
if (!master) {
dev_err(>dev, "Unable to allocate SPI Master\n");
return -ENOMEM;
}
+ master->max_speed_hz = SUN6I_SPI_MAX_SPEED_HZ;
+ master->min_speed_hz = SUN6I_SPI_MIN_SPEED_HZ;
+ master->num_chipselect = 4;
+ master->mode_bits = SUN6I_SPI_MODE_BITS;
+ master->bits_per_word_mask = SPI_BPW_MASK(8);
+ master->set_cs = sun6i_spi_set_cs;
+ master->prepare_message = sun6i_spi_prepare_message;
+ master->transfer_one = sun6i_spi_transfer_one;
+ master->max_transfer_size = sun6i_spi_max_transfer_size;
+ master->dev.of_node = pdev->dev.of_node;
+
Re: [PATCH v3 2/4] mfd: add Gateworks System Controller core driver
On Wed, Mar 28, 2018 at 8:14 AM, Tim Harvey wrote:
> The Gateworks System Controller (GSC) is an I2C slave controller
> implemented with an MSP430 micro-controller whose firmware embeds the
> following features:
> - I/O expander (16 GPIO's) using PCA955x protocol
> - Real Time Clock using DS1672 protocol
> - User EEPROM using AT24 protocol
> - HWMON using custom protocol
> - Interrupt controller with tamper detect, user pushbotton
> - Watchdog controller capable of full board power-cycle
> - Power Control capable of full board power-cycle
>
> see http://trac.gateworks.com/wiki/gsc for more details
>
> +
> +/*
> + * gsc_powerdown - API to use GSC to power down board for a specific time
> + *
> + * secs - number of seconds to remain powered off
> + */
> +static int gsc_powerdown(struct gsc_dev *gsc, unsigned long secs)
> +{
> + int ret;
> + unsigned char regs[4];
> +
> + dev_info(>i2c->dev, "GSC powerdown for %ld seconds\n",
> +secs);
> + regs[0] = secs & 0xff;
> + regs[1] = (secs >> 8) & 0xff;
> + regs[2] = (secs >> 16) & 0xff;
> + regs[3] = (secs >> 24) & 0xff;
> + ret = regmap_bulk_write(gsc->regmap, GSC_TIME_ADD, regs, 4);
> +
> + return ret;
> +}
Any feedback on the 'powerdown' sysfs attribute that hooks to this
function? This allows the GSC to disable the board primary power
supply for 2^32 seconds and is often used to 'reset' the board
although it could also be used to put the board in a power down state
longer. I'm wondering if there is a more appropriate API for this in
the kernel that I don't know about.
I would also like to register a restart handler using this but I
believe that ARM restart handlers currently can not use I2C - is that
correct?
Regards,
Tim
Re: [alsa-devel] [PATCH] ASoC: atmel_ssc_dai: fix spelling mistake: "Stoping" -> "Stopping"
On Tue, 2018-04-03 at 15:45 +0200, Ladislav Michl wrote:
> On Fri, Mar 30, 2018 at 04:44:20PM +0100, Colin King wrote:
> > From: Colin Ian King
>
> Hello Colin,
>
> > Trivial fix to spelling mistake in pr_debug message text
>
> would you mind making this patch a bit less non-trivial and
> change pr_debug to dev_dbg dropping Atmel_ssc_dai prefix?
Presumably, then all the pr_ calls should be changed.
Something like:
---
sound/soc/atmel/atmel_ssc_dai.c | 38 --
1 file changed, 16 insertions(+), 22 deletions(-)
diff --git a/sound/soc/atmel/atmel_ssc_dai.c b/sound/soc/atmel/atmel_ssc_dai.c
index a1e2c5682dcd..594228156b2d 100644
--- a/sound/soc/atmel/atmel_ssc_dai.c
+++ b/sound/soc/atmel/atmel_ssc_dai.c
@@ -291,11 +291,10 @@ static int atmel_ssc_startup(struct snd_pcm_substream
*substream,
int dir, dir_mask;
int ret;
- pr_debug("atmel_ssc_startup: SSC_SR=0x%x\n",
- ssc_readl(ssc_p->ssc->regs, SR));
+ dev_dbg(dai->dev, "SSC_SR=0x%x\n", ssc_readl(ssc_p->ssc->regs, SR));
/* Enable PMC peripheral clock for this SSC */
- pr_debug("atmel_ssc_dai: Starting clock\n");
+ dev_dbg(dai->dev, "Starting clock\n");
clk_enable(ssc_p->ssc->clk);
ssc_p->mck_rate = clk_get_rate(ssc_p->ssc->clk);
@@ -385,7 +384,7 @@ static void atmel_ssc_shutdown(struct snd_pcm_substream
*substream,
spin_unlock_irq(_p->lock);
/* Shutdown the SSC clock. */
- pr_debug("atmel_ssc_dai: Stopping clock\n");
+ dev_dbg(dai->dev, "Stopping clock\n");
clk_disable(ssc_p->ssc->clk);
}
@@ -794,13 +793,12 @@ static int atmel_ssc_hw_params(struct snd_pcm_substream
*substream,
break;
default:
- printk(KERN_WARNING "atmel_ssc_dai: unsupported DAI format
0x%x\n",
- ssc_p->daifmt);
+ dev_warn(dai->dev, "unsupported DAI format 0x%x\n",
+ssc_p->daifmt);
return -EINVAL;
}
- pr_debug("atmel_ssc_hw_params: "
- "RCMR=%08x RFMR=%08x TCMR=%08x TFMR=%08x\n",
- rcmr, rfmr, tcmr, tfmr);
+ dev_dbg(dai->dev, "RCMR=%08x RFMR=%08x TCMR=%08x TFMR=%08x\n",
+ rcmr, rfmr, tcmr, tfmr);
if (!ssc_p->initialized) {
if (!ssc_p->ssc->pdata->use_dma) {
@@ -818,9 +816,8 @@ static int atmel_ssc_hw_params(struct snd_pcm_substream
*substream,
ret = request_irq(ssc_p->ssc->irq, atmel_ssc_interrupt, 0,
ssc_p->name, ssc_p);
if (ret < 0) {
- printk(KERN_WARNING
- "atmel_ssc_dai: request_irq failure\n");
- pr_debug("Atmel_ssc_dai: Stoping clock\n");
+ dev_warn(dai->dev, "request_irq failure\n");
+ dev_dbg(dai->dev, "Stopping clock\n");
clk_disable(ssc_p->ssc->clk);
return ret;
}
@@ -839,7 +836,7 @@ static int atmel_ssc_hw_params(struct snd_pcm_substream
*substream,
ssc_writel(ssc_p->ssc->regs, TCMR, tcmr);
ssc_writel(ssc_p->ssc->regs, TFMR, tfmr);
- pr_debug("atmel_ssc_dai,hw_params: SSC initialized\n");
+ dev_dbg(dai->dev, "SSC initialized\n");
return 0;
}
@@ -862,9 +859,9 @@ static int atmel_ssc_prepare(struct snd_pcm_substream
*substream,
ssc_writel(ssc_p->ssc->regs, CR, dma_params->mask->ssc_disable);
ssc_writel(ssc_p->ssc->regs, IDR, dma_params->mask->ssc_error);
- pr_debug("%s enabled SSC_SR=0x%08x\n",
- dir ? "receive" : "transmit",
- ssc_readl(ssc_p->ssc->regs, SR));
+ dev_dbg(dai->dev, "%s enabled SSC_SR=0x%08x\n",
+ dir ? "receive" : "transmit",
+ ssc_readl(ssc_p->ssc->regs, SR));
return 0;
}
@@ -1050,21 +1047,18 @@ static void asoc_ssc_exit(struct device *dev)
int atmel_ssc_set_audio(int ssc_id)
{
struct ssc_device *ssc;
- int ret;
/* If we can grab the SSC briefly to parent the DAI device off it */
ssc = ssc_request(ssc_id);
if (IS_ERR(ssc)) {
pr_err("Unable to parent ASoC SSC DAI on SSC: %ld\n",
- PTR_ERR(ssc));
+ PTR_ERR(ssc));
return PTR_ERR(ssc);
- } else {
- ssc_info[ssc_id].ssc = ssc;
}
- ret = asoc_ssc_init(>pdev->dev);
+ ssc_info[ssc_id].ssc = ssc;
- return ret;
+ return asoc_ssc_init(>pdev->dev);
}
EXPORT_SYMBOL_GPL(atmel_ssc_set_audio);
Re: call/normal switch was Re: omap4-droid4: voice call support was
Hi! > > OK thanks for checking. So probably only n_gsm channel 1 is for normal > > Qualcomm at commands, and then channel 2 and others are commands > > implemented by Motorola on the mdm6600. > > > > I guess we'd have to add support for reading and writing to > > /dev/gsmtty2 at least as it looks like these cannot be accessed > > via /dev/ttyUSB4. Or at least I have not figured out any other > > way to access them. > > Hmm or maybe there is some way to select where qmi to tunnels the > AT commands to? Some kind of channel type paramenter like n_gsm > has? > > Anyways, for trying to figure out things for alsamixer, I tested > that ngsm 2 "AT+EACC=3,0" is not related to cpcap audio register > settings and the mic is enabled during the call with Pavel's patch > with alsamixer "Mode" set to either "Call" or "Normal". Also speaker > keeps working during the call toggling between "Call" and "Normal". > "Voice" in alsamixer does also control the speaker volume during > the call. And setting the second "Speaker" from "Voice" to "HiFi" > mutes the speaker. Take a look at the patch. Selecting "call" does something at hardware level, selecting "normal" is a nop. Sorry for confusion. Yes, that patch needs more work. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Re: [PATCH 14/15] ARM: pxa: change SSP devices allocation
On Tue, Apr 3, 2018 at 5:32 PM, Robert Jarzmik wrote:
> Arnd Bergmann writes:
>
> chop chop ... removed several mail recipients to leave only the ASoC / PXA
> subset ...
>
>> On Mon, Apr 2, 2018 at 4:26 PM, Robert Jarzmik
>> wrote:
>>
>>>
>>> +static struct pxa_ssp_info pxa_ssp_infos[] = {
>>> + { .dma_chan_rx_name = "ssp1_rx", .dma_chan_tx_name = "ssp1_tx", },
>>> + { .dma_chan_rx_name = "ssp1_rx", .dma_chan_tx_name = "ssp1_tx", },
>>> + { .dma_chan_rx_name = "ssp2_rx", .dma_chan_tx_name = "ssp2_tx", },
>>> + { .dma_chan_rx_name = "ssp2_rx", .dma_chan_tx_name = "ssp2_tx", },
>>> + { .dma_chan_rx_name = "ssp3_rx", .dma_chan_tx_name = "ssp3_tx", },
>>> + { .dma_chan_rx_name = "ssp3_rx", .dma_chan_tx_name = "ssp3_tx", },
>>> + { .dma_chan_rx_name = "ssp4_rx", .dma_chan_tx_name = "ssp4_tx", },
>>> + { .dma_chan_rx_name = "ssp4_rx", .dma_chan_tx_name = "ssp4_tx", },
>>> +};
>>
>> This part looks odd to me, you're adding an extra level of indirection to
>> do two stages of lookups in some form of platform data.
> That's unfortunately right.
>
>> Why can't you just always use "rx" and "tx" as the names here?
> Well I couldn't. I'll explain you why, and maybe you'll find a better
> solution.
>
> That all is related to how ASoC and SSP interact.
> If I remember correctly, here is how it works :
> - the DMA channel is requested in sound/arm/pxa2xx-pcm-lib.c:128
> return snd_dmaengine_pcm_open(
> substream, dma_request_slave_channel(rtd->platform->dev,
>The trick is that the device here is _not_ the SSP one, it's if memory
> serves
>me well the pxa-pcm-audio one.
>
>As a consequence, the device cannot be used to differenciate which SSP
>exactly is providing the sound samples stream. This information is
>nevertheless required to choose the correct requestor line, which is a
> 1-to-1
>match to the SSP port.
>
>The indirection in the channel name is used to choose the correct requestor
>line for a given SSP port providing the samples.
>
>It also must be underlined that this dma request serves both AC97 and SSP
> as
>sample providers.
I'm still unable to follow through that code, but I understand now that
the device you pass to dma_request_slave_channel() is not the one
we'd like it to be here.
Where exactly does that call to dma_request_chan() happen? Is this
the one in dmaengine_pcm_new()? Could we perhaps put a
pointer to the SSP device into snd_dmaengine_dai_dma_data?
Arnd
Re: [PATCH v2 1/6] spi: core: handle timeout error from transfer_one()
On Tue, Apr 03, 2018 at 06:29:00PM +0300, Sergey Suloev wrote: > As long as sun4i/sun6i SPI drivers have overriden the default > "wait for completion" procedure then we need to properly > handle -ETIMEDOUT error from transfer_one(). Why is this connected to those drivers specifically? signature.asc Description: PGP signature
Re: [PATCH] memcg, thp: do not invoke oom killer on thp charges
On Tue 03-04-18 10:58:53, Johannes Weiner wrote:
> On Wed, Mar 21, 2018 at 09:59:28PM +0100, Michal Hocko wrote:
> > From: Michal Hocko
> >
> > David has noticed that THP memcg charge can trigger the oom killer
> > since 2516035499b9 ("mm, thp: remove __GFP_NORETRY from khugepaged and
> > madvised allocations"). We have used an explicit __GFP_NORETRY
> > previously which ruled the OOM killer automagically.
> >
> > Memcg charge path should be semantically compliant with the allocation
> > path and that means that if we do not trigger the OOM killer for costly
> > orders which should do the same in the memcg charge path as well.
> > Otherwise we are forcing callers to distinguish the two and use
> > different gfp masks which is both non-intuitive and bug prone. Not to
> > mention the maintenance burden.
> >
> > Teach mem_cgroup_oom to bail out on costly order requests to fix the THP
> > issue as well as any other costly OOM eligible allocations to be added
> > in future.
> >
> > Fixes: 2516035499b9 ("mm, thp: remove __GFP_NORETRY from khugepaged and
> > madvised allocations")
> > Reported-by: David Rientjes
> > Signed-off-by: Michal Hocko
>
> I also prefer this fix over having separate OOM behaviors (which is
> user-visible, and not just about technical ability to satisfy the
> allocation) between the allocator and memcg.
>
> Acked-by: Johannes Weiner
I will repost the patch with the currently merged THP specific handling
reverted (see below). While 9d3c3354bb85 might have been an appropriate
quick fix, we shouldn't keep it longterm for 4.17+ IMHO.
Does you ack apply to that patch as well?
---
>From 3e1b127dd6107a0e51654e90d9faef7f196f5a10 Mon Sep 17 00:00:00 2001
From: Michal Hocko
Date: Wed, 21 Mar 2018 10:10:37 +0100
Subject: [PATCH] memcg, thp: do not invoke oom killer on thp charges
David has noticed that THP memcg charge can trigger the oom killer
since 2516035499b9 ("mm, thp: remove __GFP_NORETRY from khugepaged and
madvised allocations"). We have used an explicit __GFP_NORETRY
previously which ruled the OOM killer automagically.
Memcg charge path should be semantically compliant with the allocation
path and that means that if we do not trigger the OOM killer for
costly orders which should do the same in the memcg charge path as
well. Otherwise we are forcing callers to distinguish the two and use
different gfp masks which is both non-intuitive and bug prone. As soon
as we get a costly high order kmalloc user we even do not have any means
to tell the memcg specific gfp mask to prevent from OOM because the
charging is deep within guts of the slab allocator.
The unexpected memcg OOM on THP has already been fixed upstream by
9d3c3354bb85 ("mm, thp: do not cause memcg oom for thp") but this is
one-off fix rather than a generic solution. Teach mem_cgroup_oom to bail
out on costly order requests to fix the THP issue as well as any other
costly OOM eligible allocations to be added in future.
Also revert 9d3c3354bb85 because special gfp for THP is no longer
needed.
Fixes: 2516035499b9 ("mm, thp: remove __GFP_NORETRY from khugepaged and
madvised allocations")
Reported-by: David Rientjes
Signed-off-by: Michal Hocko
---
mm/huge_memory.c | 5 ++---
mm/khugepaged.c | 8 ++--
mm/memcontrol.c | 2 +-
3 files changed, 5 insertions(+), 10 deletions(-)
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 2297dd9cc7c3..0cc62405de9c 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -555,8 +555,7 @@ static int __do_huge_pmd_anonymous_page(struct vm_fault
*vmf, struct page *page,
VM_BUG_ON_PAGE(!PageCompound(page), page);
- if (mem_cgroup_try_charge(page, vma->vm_mm, gfp | __GFP_NORETRY, ,
- true)) {
+ if (mem_cgroup_try_charge(page, vma->vm_mm, gfp, , true)) {
put_page(page);
count_vm_event(THP_FAULT_FALLBACK);
return VM_FAULT_FALLBACK;
@@ -1317,7 +1316,7 @@ int do_huge_pmd_wp_page(struct vm_fault *vmf, pmd_t
orig_pmd)
}
if (unlikely(mem_cgroup_try_charge(new_page, vma->vm_mm,
- huge_gfp | __GFP_NORETRY, , true))) {
+ huge_gfp, , true))) {
put_page(new_page);
split_huge_pmd(vma, vmf->pmd, vmf->address);
if (page)
diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 214e614b62b0..b7e2268dfc9a 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -960,9 +960,7 @@ static void collapse_huge_page(struct mm_struct *mm,
goto out_nolock;
}
- /* Do not oom kill for khugepaged charges */
- if (unlikely(mem_cgroup_try_charge(new_page, mm, gfp | __GFP_NORETRY,
- , true))) {
+ if (unlikely(mem_cgroup_try_charge(new_page, mm, gfp, , true))) {
result = SCAN_CGROUP_CHARGE_FAIL;
goto out_nolock;
}
@@ -1321,9 +1319,7 @@ static void
Re: [PATCH] usbip: vhc_hcd: prevent module being removed while device are attached
On 04/03/2018 12:56 AM, Greg KH wrote:
> On Mon, Apr 02, 2018 at 02:52:31PM -0600, Shuah Khan wrote:
>> vhci_hcd module can be removed even when devices are attached. Fix to
>> prevent module removal when devices are still attached.
>>
>> Signed-off-by: Shuah Khan
>> ---
>> drivers/usb/usbip/vhci_sysfs.c | 25 +
>> 1 file changed, 21 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/usb/usbip/vhci_sysfs.c b/drivers/usb/usbip/vhci_sysfs.c
>> index 48808388ec33..6a54b9aa92be 100644
>> --- a/drivers/usb/usbip/vhci_sysfs.c
>> +++ b/drivers/usb/usbip/vhci_sysfs.c
>> @@ -9,6 +9,7 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> #include "usbip_common.h"
>> #include "vhci.h"
>> @@ -252,6 +253,8 @@ static ssize_t detach_store(struct device *dev, struct
>> device_attribute *attr,
>> if (ret < 0)
>> return -EINVAL;
>>
>> +module_put(THIS_MODULE);
>> +
>> usbip_dbg_vhci_sysfs("Leave\n");
>>
>> return count;
>> @@ -302,7 +305,7 @@ static ssize_t attach_store(struct device *dev, struct
>> device_attribute *attr,
>> struct vhci_hcd *vhci_hcd;
>> struct vhci_device *vdev;
>> struct vhci *vhci;
>> -int err;
>> +int err, ret;
>> unsigned long flags;
>>
>> /*
>> @@ -339,10 +342,18 @@ static ssize_t attach_store(struct device *dev, struct
>> device_attribute *attr,
>> else
>> vdev = >vhci_hcd_hs->vdev[rhport];
>>
>> +/* get module ref to avoid being removed with active attached devs */
>> +if (!try_module_get(THIS_MODULE)) {
>> +ret = -EAGAIN;
>> +goto module_get_err;
>> +}
>
> That's really not a good idea, trying to grab your own module reference
> is considered racy and we stopped adding that code pattern to the kernel
> a long time ago.
Thanks. Good to know.
>
> What's wrong if you remove the vhci module if devices are attached? You
> can do that today for any other USB host driver, this shouldn't be
> "special".
This is a virtual device associated to a real physical device on a different
system. My concern is that if the module gets removed accidentally then it
could disrupt access to the remote device. The remote nature of the device
with several players involved makes this scenario a bit more complex than
a regular usb case when device is physically on the system. It is probably one
of the minor problems that could happen with a remote device.
I found a few modules that hold reference to themselves in the kernel. Block,
crypto, net, md, vfio, nfs.
md for example holds reference to itself when it creates a blank device.
vfio get regerence to itself it when opens pci and mediated devices.
Some network drivers do the same for handling virtual functions.
nfs does this for rpc handling.
I am not making a case for adding one more, however I am curious if this
vhci_hcd falls into a similar special case of handling virtual connections
and devices.
>
> Also, kernel modules are never automatically removed by the system, so
> someone has to do this by hand, so they knew what they were doing :)
>
Yes. This will not be a usual scenario. The question is whether not kernel
should detect driver is in use and prevent it.
thanks,
-- Shuah
Re: [PATCH V3 4/4] genirq/affinity: irq vector spread among online CPUs as far as possible
On Tue, Apr 03, 2018 at 03:32:21PM +0200, Thomas Gleixner wrote:
> On Thu, 8 Mar 2018, Ming Lei wrote:
> > 1) before 84676c1f21 ("genirq/affinity: assign vectors to all possible
> > CPUs")
> > irq 39, cpu list 0
> > irq 40, cpu list 1
> > irq 41, cpu list 2
> > irq 42, cpu list 3
> >
> > 2) after 84676c1f21 ("genirq/affinity: assign vectors to all possible CPUs")
> > irq 39, cpu list 0-2
> > irq 40, cpu list 3-4,6
> > irq 41, cpu list 5
> > irq 42, cpu list 7
> >
> > 3) after applying this patch against V4.15+:
> > irq 39, cpu list 0,4
> > irq 40, cpu list 1,6
> > irq 41, cpu list 2,5
> > irq 42, cpu list 3,7
>
> That's more or less window dressing. If the device is already in use when
> the offline CPUs get hot plugged, then the interrupts still stay on cpu 0-3
> because the effective affinity of interrupts on X86 (and other
> architectures) is always a single CPU.
>
> So this only might move interrupts to the hotplugged CPUs when the device
> is initialized after CPU hotplug and the actual vector allocation moves an
> interrupt out to the higher numbered CPUs if they have less vectors
> allocated than the lower numbered ones.
It works for blk-mq devices, such as NVMe.
Now NVMe driver creates num_possible_cpus() hw queues, and each
hw queue is assigned one msix irq vector.
Storage is Client/Server model, that means the interrupt is only
delivered to CPU after one IO request is submitted to hw queue and
it is completed by this hw queue.
When CPUs is hotplugged, and there will be IO submitted from these
CPUs, then finally IOs complete and irq events are generated from
hw queues, and notify these submission CPU by IRQ finally.
Thanks,
Ming
Re: [PATCH v2 1/6] spi: core: handle timeout error from transfer_one()
On 04/03/2018 06:52 PM, Mark Brown wrote: On Tue, Apr 03, 2018 at 06:29:00PM +0300, Sergey Suloev wrote: As long as sun4i/sun6i SPI drivers have overriden the default "wait for completion" procedure then we need to properly handle -ETIMEDOUT error from transfer_one(). Why is this connected to those drivers specifically? These 2 drivers have their own "waiting" code and not using the code from SPI core.
Re: [PATCH v17 01/10] LIB: Introduce a generic PIO mapping method
+int logic_pio_register_range(struct logic_pio_hwaddr *new_range)
+{
+ struct logic_pio_hwaddr *range;
+ resource_size_t start = new_range->hw_start;
+ resource_size_t end = new_range->hw_start + new_range->size;
+ resource_size_t mmio_sz = 0;
+ resource_size_t iio_sz = MMIO_UPPER_LIMIT;
+ int ret = 0;
+
+ if (!new_range || !new_range->fwnode || !new_range->size)
+ return -EINVAL;
+
+ mutex_lock(_range_mutex);
+ list_for_each_entry_rcu(range, _range_list, list) {
+ if (range->fwnode == new_range->fwnode) {
+ /* range already there */
+ ret = -EFAULT;
+ goto end_register;
+ }
Hi Thierry,
This is the -EFAULT that propagates to pci-tegra.c's ->probe() and fails
to bind the driver.
I'm not exactly sure what's causing the duplicate here because it's
rather difficult to get at something useful from just the ->fwnode, but
I'm fairly sure that the reason this breaks is because the Tegra driver
will defer probe due to some regulators that aren't available on the
first try. Given the above code and the rest of this file, I can't see a
way to "fix" the driver and remove the I/O range on failure.
This is doubly bad because this doesn't only leak the ranges on probe
deferral, but also on driver unload, and we just added support for
building the Tegra driver as a loadable module, so these are actually
cases that can happen in regular uses of the driver.
I have no idea on how to fix this. Anyone know of a quick fix to restore
PCI for Tegra other than reverting all of these changes?
I suppose an API could be added to unregister the range, but the calling
sequence is rather obfuscated, so removing the range will look totally
asymmetric, I'm afraid.
Here's the call stack:
tegra_pcie_probe()
tegra_pcie_parse_dt()
of_pci_range_to_resource()
pci_register_io_range()
logic_pio_register_range()
So the range here is registered as part of a resource parsing function,
which is supposed to not have any side-effects. There's no equivalent of
that parsing routine (i.e. no "unparse" function that would undo the
effects of parsing).
Perhaps a cleaner way would be to decouple the parsing from the actual
request step that has the side-effect.
This could be added if we agreed that it would be useful.
Going back in history a little, it looks like even before this commit
the I/O range registration was triggered by the parsing code and even
the range leak was there, except that it caused pci_register_io_range()
to return 0 rather than -EFAULT. Perhaps the quickest fix for this would
be to do the same in the new code and restore drivers that accidentally
depend on this behaviour.
I can confirm that the following fixes the issue for me, though I don't
think it's a very clean fix given that the range will remain requested
forever, even if the driver is gone. But since that's already been the
case for quite a while, probably something that can be fixed separately.
Right, there was no way to deregister the range previously. From
looking at the history here I see no reason to not support it.
As for this patch, as you said, the only difference is that we fault on
trying to register the same range again. So this solution seems reasonable.
On another point, for the tegra driver, is it possible to defer earlier
in the probe, before these currently irreversible steps are taken?
Thanks very much,
John
Cc'ing linux-tegra for visibility.
Thierry
--- >8 ---
diff --git a/lib/logic_pio.c b/lib/logic_pio.c
index 29cedeadb397..4664b87e1c5f 100644
--- a/lib/logic_pio.c
+++ b/lib/logic_pio.c
@@ -46,7 +46,6 @@ int logic_pio_register_range(struct logic_pio_hwaddr
*new_range)
list_for_each_entry_rcu(range, _range_list, list) {
if (range->fwnode == new_range->fwnode) {
/* range already there */
- ret = -EFAULT;
goto end_register;
}
if (range->flags == LOGIC_PIO_CPU_MMIO &&
Re: [PATCH v5 3/3] MIPS: use generic GCC library routines from lib/
On Tue, Apr 03, 2018 at 10:24:26AM +0100, Matt Redfearn wrote:
> From: Antony Pavlov
>
> The commit b35cd9884fa5 ("lib: Add shared copies of some GCC library
> routines") makes it possible to share generic GCC library routines by
> several architectures.
>
> This commit removes several generic GCC library routines from
> arch/mips/lib/ in favour of similar routines from lib/.
>
> Signed-off-by: Antony Pavlov
> [Matt Redfearn] Use GENERIC_LIB_* named Kconfig entries
> Signed-off-by: Matt Redfearn
> Cc: Palmer Dabbelt
> Cc: Matt Redfearn
> Cc: James Hogan
> Cc: Ralf Baechle
> Cc: linux-m...@linux-mips.org
> Cc: linux-kernel@vger.kernel.org
ci20_defconfig:
make[1]: *** No rule to make target 'arch/mips/lib/ashldi3.c', needed by
'arch/mips/boot/compressed/ashldi3.c'. Stop.
make[1]: *** Waiting for unfinished jobs
make: *** [arch/mips/Makefile +395 : vmlinuz] Error 2
same for db1xxx_defconfig (and possibly others).
Thanks
James
signature.asc
Description: Digital signature
Re: [PATCH v2] f2fs: remain written times to update inode during fsync
On 04/03, Chao Yu wrote:
> On 2018/4/3 13:23, Jaegeuk Kim wrote:
> > On 04/03, Chao Yu wrote:
> >> On 2018/3/31 0:30, Jaegeuk Kim wrote:
> >>> Change log from v1:
> >>> - add more description
> >>>
> >>> This fixes xfstests/generic/392.
> >>>
> >>> The failure was caused by different times between 1) one marked in the
> >>> last
> >>> fsync(2) call and 2) the other given by roll-forward recovery after
> >>> power-cut.
> >>> The reason was that we skipped updating inode block at 1), since its
> >>> i_size
> >>> was recoverable along with 4KB-aligned data writes, which was fixed by:
> >>> "f2fs: fix a wrong condition in f2fs_skip_inode_update"
> >>>
> >>> Signed-off-by: Jaegeuk Kim
> >>> ---
> >>> fs/f2fs/f2fs.h | 15 +++
> >>> fs/f2fs/inode.c | 4
> >>> 2 files changed, 19 insertions(+)
> >>>
> >>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> >>> index 000f93f6767e..675c39d85111 100644
> >>> --- a/fs/f2fs/f2fs.h
> >>> +++ b/fs/f2fs/f2fs.h
> >>> @@ -664,6 +664,7 @@ struct f2fs_inode_info {
> >>> kprojid_t i_projid; /* id for project quota */
> >>> int i_inline_xattr_size;/* inline xattr size */
> >>> struct timespec i_crtime; /* inode creation time */
> >>> + struct timespec i_disk_time[4]; /* inode disk times */
> >>> };
> >>>
> >>> static inline void get_extent_info(struct extent_info *ext,
> >>> @@ -2457,6 +2458,11 @@ static inline void clear_file(struct inode *inode,
> >>> int type)
> >>> f2fs_mark_inode_dirty_sync(inode, true);
> >>> }
> >>>
> >>> +static inline bool time_equal(struct timespec a, struct timespec b)
> >>> +{
> >>> + return (a.tv_sec == b.tv_sec) && (a.tv_nsec == b.tv_nsec);
> >>> +}
> >>
> >> We can use existing timespec_equal in instead of defining a
> >> duplicated one?
> >
> > It is defined with const parameters.
>
> I didn't get it, const keyword can used to protect parameter not to be changed
> in that function, that will be more safe, so it will be better to use that
> one?
Oh, I just made a mistake when testing timespec_equal().
I really need more recess time. :P
Change log from v3:
- use timespec_equal()
This fixes xfstests/generic/392.
The failure was caused by different times between 1) one marked in the last
fsync(2) call and 2) the other given by roll-forward recovery after power-cut.
The reason was that we skipped updating inode block at 1), since its i_size
was recoverable along with 4KB-aligned data writes, which was fixed by:
"f2fs: fix a wrong condition in f2fs_skip_inode_update"
Signed-off-by: Jaegeuk Kim
---
fs/f2fs/f2fs.h | 11 +++
fs/f2fs/inode.c | 8
2 files changed, 19 insertions(+)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 7102d3965fea..1df7f10476d6 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -664,6 +664,7 @@ struct f2fs_inode_info {
kprojid_t i_projid; /* id for project quota */
int i_inline_xattr_size;/* inline xattr size */
struct timespec i_crtime; /* inode creation time */
+ struct timespec i_disk_time[4]; /* inode disk times */
};
static inline void get_extent_info(struct extent_info *ext,
@@ -2474,6 +2475,16 @@ static inline bool f2fs_skip_inode_update(struct inode
*inode, int dsync)
i_size_read(inode) & ~PAGE_MASK)
return false;
+ if (!timespec_equal(F2FS_I(inode)->i_disk_time, >i_atime))
+ return false;
+ if (!timespec_equal(F2FS_I(inode)->i_disk_time + 1, >i_ctime))
+ return false;
+ if (!timespec_equal(F2FS_I(inode)->i_disk_time + 2, >i_mtime))
+ return false;
+ if (!timespec_equal(F2FS_I(inode)->i_disk_time + 3,
+ _I(inode)->i_crtime))
+ return false;
+
down_read(_I(inode)->i_sem);
ret = F2FS_I(inode)->last_disk_size == i_size_read(inode);
up_read(_I(inode)->i_sem);
diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
index 401f09ccce7e..e0d9e8f27ed2 100644
--- a/fs/f2fs/inode.c
+++ b/fs/f2fs/inode.c
@@ -284,6 +284,10 @@ static int do_read_inode(struct inode *inode)
fi->i_crtime.tv_nsec = le32_to_cpu(ri->i_crtime_nsec);
}
+ F2FS_I(inode)->i_disk_time[0] = inode->i_atime;
+ F2FS_I(inode)->i_disk_time[1] = inode->i_ctime;
+ F2FS_I(inode)->i_disk_time[2] = inode->i_mtime;
+ F2FS_I(inode)->i_disk_time[3] = F2FS_I(inode)->i_crtime;
f2fs_put_page(node_page, 1);
stat_inc_inline_xattr(inode);
@@ -444,6 +448,10 @@ void update_inode(struct inode *inode, struct page
*node_page)
if (inode->i_nlink == 0)
clear_inline_node(node_page);
+ F2FS_I(inode)->i_disk_time[0] = inode->i_atime;
+ F2FS_I(inode)->i_disk_time[1] = inode->i_ctime;
+ F2FS_I(inode)->i_disk_time[2] = inode->i_mtime;
+ F2FS_I(inode)->i_disk_time[3] = F2FS_I(inode)->i_crtime;
}
void update_inode_page(struct inode
Re: [PATCH v2] x86/xen/efi: Initialize UEFI secure boot state during dom0 boot
On Tue, Apr 03, 2018 at 08:44:41AM -0700, James Bottomley wrote:
> On Tue, 2018-04-03 at 16:39 +0200, Daniel Kiper wrote:
> > Initialize UEFI secure boot state during dom0 boot. Otherwise the
> > kernel
> > may not even know that it runs on secure boot enabled platform.
> >
> > Signed-off-by: Daniel Kiper
> > ---
> > arch/x86/xen/efi.c| 57
> > +
> > drivers/firmware/efi/libstub/secureboot.c |3 ++
> > 2 files changed, 60 insertions(+)
> >
> > diff --git a/arch/x86/xen/efi.c b/arch/x86/xen/efi.c
> > index a18703b..1804b27 100644
> > --- a/arch/x86/xen/efi.c
> > +++ b/arch/x86/xen/efi.c
> > @@ -115,6 +115,61 @@ static efi_system_table_t __init
> > *xen_efi_probe(void)
> > return _systab_xen;
> > }
> >
> > +/*
> > + * Determine whether we're in secure boot mode.
> > + *
> > + * Please keep the logic in sync with
> > + * drivers/firmware/efi/libstub/secureboot.c:efi_get_secureboot().
> > + */
> > +static enum efi_secureboot_mode xen_efi_get_secureboot(void)
> > +{
> > + static efi_guid_t efi_variable_guid =
> > EFI_GLOBAL_VARIABLE_GUID;
> > + static efi_guid_t shim_guid = EFI_SHIM_LOCK_GUID;
> > + efi_status_t status;
> > + u8 moksbstate, secboot, setupmode;
> > + unsigned long size;
> > +
> > + size = sizeof(secboot);
> > + status = efi.get_variable(L"SecureBoot", _variable_guid,
> > + NULL, , );
> > +
> > + if (status == EFI_NOT_FOUND)
> > + return efi_secureboot_mode_disabled;
> > +
> > + if (status != EFI_SUCCESS)
> > + goto out_efi_err;
> > +
> > + size = sizeof(setupmode);
> > + status = efi.get_variable(L"SetupMode", _variable_guid,
> > + NULL, , );
> > +
> > + if (status != EFI_SUCCESS)
> > + goto out_efi_err;
> > +
> > + if (secboot == 0 || setupmode == 1)
> > + return efi_secureboot_mode_disabled;
> > +
> > + /* See if a user has put the shim into insecure mode. */
> > + size = sizeof(moksbstate);
> > + status = efi.get_variable(L"MokSBStateRT", _guid,
> > + NULL, , );
> > +
> > + /* If it fails, we don't care why. Default to secure. */
> > + if (status != EFI_SUCCESS)
> > + goto secure_boot_enabled;
> > +
> > + if (moksbstate == 1)
> > + return efi_secureboot_mode_disabled;
> > +
> > + secure_boot_enabled:
> > + pr_info("UEFI Secure Boot is enabled.\n");
> > + return efi_secureboot_mode_enabled;
> > +
> > + out_efi_err:
> > + pr_err("Could not determine UEFI Secure Boot status.\n");
> > + return efi_secureboot_mode_unknown;
> > +}
> > +
>
> This looks like a bad idea: you're duplicating the secure boot check in
>
> drivers/firmware/efi/libstub/secureboot.c
>
> Which is an implementation of policy. If we have to have policy in the
> kernel, it should really only be in one place to prevent drift; why
> can't you simply use the libstub efi_get_secureboot() so we're not
> duplicating the implementation of policy?
Well, here is the first version of this patch:
https://lkml.org/lkml/2018/1/9/496
Ard did not like it. I was not happy too. In general both approaches are not
perfect.
More you can find in the discussion around this patchset. If you have better
idea
how to do that I am happy to implement it.
Daniel
Re: [PATCH v2 1/3] locking: Document the semantics of spin_is_locked()
On Tue, Apr 03, 2018 at 08:03:56AM -0700, Paul E. McKenney wrote: > On Tue, Apr 03, 2018 at 04:43:00PM +0200, Peter Zijlstra wrote: > > On Tue, Apr 03, 2018 at 07:17:18AM -0700, Paul E. McKenney wrote: > > > Suggestions for a fix? Clearly great care is required when using it > > > in things like WARN_ON()... > > > > Yeah, don't use it there, use lockdep_assert_held(). > > Good point, -ETOOEARLY. ;-) > > > As I stated before in this thread, ideally we'd make *_is_locked() go > > away entirely. > > After being reminded of the issues on UP systems, I now have much more > sympathy for that view... And so the main remaining use case is debug prints on !PROVE_LOCKING builds. Which need some thought about the UP case. Or am I missing something here? Thanx, Paul
Re: [PATCH v1] kernel/trace:check the val against the available mem
On Tue 03-04-18 10:17:53, Steven Rostedt wrote:
> On Tue, 3 Apr 2018 15:56:07 +0200
> Michal Hocko wrote:
[...]
> > I simply do not see the difference between the two. Both have the same
> > deadly effect in the end. The direct OOM has an arguable advantage that
> > the effect is immediate rather than subtle with potential performance
> > side effects until the machine OOMs after crawling for quite some time.
>
> The difference is if the allocation succeeds or not. If it doesn't
> succeed, we free all memory that we tried to allocate. If it succeeds
> and causes issues, then yes, that's the admins fault.
What am I trying to say is that this is so extremely time and workload
sensitive that you can hardly have a stable behavior. It will become a
pure luck whether the failure happens.
> I'm worried about
> the accidental putting in too big of a number, either by an admin by
> mistake, or some stupid script that just thinks the current machines
> has terabytes of memory.
I would argue that stupid scripts should have no business calling root
only interfaces which can allocate a lot of memory and cause OOMs.
> I'm under the assumption that if I allocate an allocation of 32 pages
> with RETRY_MAYFAIL, and there's 2 pages available, but not 32, and
> while my allocation is reclaiming memory, and another task comes in and
> asks for a single page, it can still succeed. This would be why I would
> be using RETRY_MAYFAIL with higher orders of pages, that it doesn't
> take all memory in the system if it fails. Is this assumption incorrect?
Yes. There is no guarantee that the allocation will get the memory it
reclaimed in the direct reclaim. Pages are simply freed back into the
pool and it is a matter of timing who gets them.
> The current approach of allocating 1 page at a time with RETRY_MAYFAIL
> is that it will succeed to get any pages that are available, until
> there are none, and if some unlucky task asks for memory during that
> time, it is guaranteed to fail its allocation triggering an OOM.
>
> I was thinking of doing something like:
>
> large_pages = nr_pages / 32;
> if (large_pages) {
> pages = alloc_pages_node(cpu_to_node(cpu),
> GFP_KERNEL | __GFP_RETRY_MAYFAIL, 5);
> if (pages)
> /* break up pages */
> else
> /* try to allocate with NORETRY */
> }
You can do so, of course. In fact it would have some advantages over
single pages because you would fragment the memory less but this is not
a reliable prevention from OOM killing and the complete memory
depletion if you allow arbitrary trace buffer sizes.
> Now it will allocate memory in 32 page chunks using reclaim. If it
> fails to allocate them, it would not have taken up any smaller chunks
> that were available, leaving them for other users. It would then go
> back to singe pages, allocating with RETRY. Or I could just say screw
> it, and make the allocation of the ring buffer always be 32 page chunks
> (or at least make it user defined).
yes a fallback is questionable. Whether to make the batch size
configuration is a matter of how much internal details you want to
expose to userspace.
--
Michal Hocko
SUSE Labs
Re: [PATCH v8 11/14] iommu/rockchip: Use OF_IOMMU to attach devices automatically
Hi Jeffy,
Sorry for delayed response.
On Mon, Mar 26, 2018 at 1:58 AM JeffyChen wrote:
> Hi Daniel,
> Thanks for your reply.
> On 03/26/2018 02:31 PM, Daniel Kurtz wrote:
> >> >+struct rk_iommudata {
> >> >+ struct rk_iommu *iommu;
> >> >+};
> > Why do we need this struct? Can't we just assign a pointer to struct
> > rk_iommu directly to dev->archdata.iommu?
> >
> hmmm, i was trying to add more device related data in patch[13]:
>struct rk_iommudata {
> + struct device_link *link; /* runtime PM link from IOMMU to master
*/
> struct rk_iommu *iommu;
>};
> >
Can't you just add link to rk_iommu directly?
Re: [PATCH v2 1/6] spi: core: handle timeout error from transfer_one()
On Tue, Apr 03, 2018 at 07:00:55PM +0300, Sergey Suloev wrote: > On 04/03/2018 06:52 PM, Mark Brown wrote: > > On Tue, Apr 03, 2018 at 06:29:00PM +0300, Sergey Suloev wrote: > > > As long as sun4i/sun6i SPI drivers have overriden the default > > > "wait for completion" procedure then we need to properly > > > handle -ETIMEDOUT error from transfer_one(). > > Why is this connected to those drivers specifically? > These 2 drivers have their own "waiting" code and not using the code from > SPI core. Does this not apply to any other driver - why is this something we only have to do when these drivers do it? That's what's setting off alarm bells. signature.asc Description: PGP signature
Re: [PATCH 1/1] taging: fbtft: fix memory leak
On Tue, 2018-04-03 at 21:33 +0800, Xidong Wang wrote:
> From: Xidong Wang <2711406...@qq.com>
>
> In function fbtft_framebuffer_alloc(), the memory allocated by
> framebuffer_alloc() is not released on the error path that txbuflen > 0
> and txbuf, which holds the return value of devm_kzalloc(), is NULL.
> This will result in a memory leak bug.
[]
> diff --git a/drivers/staging/fbtft/fbtft-core.c
> b/drivers/staging/fbtft/fbtft-core.c
[]
> @@ -836,7 +836,7 @@ struct fb_info *fbtft_framebuffer_alloc(struct
> fbtft_display *display,
> if (txbuflen > 0) {
> txbuf = devm_kzalloc(par->info->device, txbuflen, GFP_KERNEL);
> if (!txbuf)
> - goto alloc_fail;
> + goto err_info;
> par->txbuf.buf = txbuf;
> par->txbuf.len = txbuflen;
> }
> @@ -872,6 +872,9 @@ struct fb_info *fbtft_framebuffer_alloc(struct
> fbtft_display *display,
>
> return info;
>
> +err_info:
> + framebuffer_release(info);
> +
> alloc_fail:
> vfree(vmem);
What about the
if (par->gamma.curves && gamma) {
if (fbtft_gamma_parse_str(par,
par->gamma.curves, gamma, strlen(gamma)))
goto alloc_fail;
}
a little above this?
Presumable then it should goto err_info too.
Re: [PATCH net 2/2] net: systemport: Fix sparse warnings in bcm_sysport_insert_tsb()
On Mon, Apr 02, 2018 at 03:58:56PM -0700, Florian Fainelli wrote: > skb->protocol is a __be16 which we would be calling htons() against, > while this is not wrong per-se as it correctly results in swapping the > value on LE hosts, this still upsets sparse. Adopt a similar pattern to > what other drivers do and just assign ip_ver to skb->protocol, and then > use htons() against the different constants such that the compiler can > resolve the values at build time. This is completely bogus. What sparse is complaining about is htons used to convert from big-endian to host-endian. Which is what ntohs is for. IOW, instead of all that crap just - ip_ver = htons(skb->protocol); + ip_ver = ntohs(skb->protocol); and be done with that. Same in other drivers with the same problem.
Re: [PATCH v2 1/6] spi: core: handle timeout error from transfer_one()
On 04/03/2018 07:18 PM, Mark Brown wrote: On Tue, Apr 03, 2018 at 07:00:55PM +0300, Sergey Suloev wrote: On 04/03/2018 06:52 PM, Mark Brown wrote: On Tue, Apr 03, 2018 at 06:29:00PM +0300, Sergey Suloev wrote: As long as sun4i/sun6i SPI drivers have overriden the default "wait for completion" procedure then we need to properly handle -ETIMEDOUT error from transfer_one(). Why is this connected to those drivers specifically? These 2 drivers have their own "waiting" code and not using the code from SPI core. Does this not apply to any other driver - why is this something we only have to do when these drivers do it? That's what's setting off alarm bells. sun4i/sun6i drivers have let's say "smart" waiting while SPI core uses a fixed interval to wait. I can't say for every SPI driver in kernel, that's outside of my area of expertise.
Re: [GIT PULL] Kernel lockdown for secure boot
On Tue, Apr 3, 2018 at 8:41 AM, Alexei Starovoitov wrote: > On Tue, Apr 03, 2018 at 08:11:07AM -0700, Andy Lutomirski wrote: >> > >> >> "bpf: Restrict kernel image access functions when the kernel is locked >> >> down": >> >> This patch just sucks in general. >> > >> > Yes - but that's what Alexei Starovoitov specified. bpf kind of sucks >> > since >> > it gives you unrestricted access to the kernel. >> >> bpf, in certain contexts, gives you unrestricted access to *reading* >> kernel memory. bpf should, under no circumstances, let you write to >> the kernel unless you're using fault injection or similar. >> >> I'm surprised that Alexei acked this patch. If something like XDP or >> bpfilter starts becoming widely used, this patch will require a lot of >> reworking to avoid breaking standard distros. > > my understanding was that this lockdown set attemps to disallow _reads_ > of kernel memory from anything, so first version of patch was adding > run-time checks for bpf_probe_read() which is no-go > and without this helper the bpf for tracing is losing a lot of its power, > so the easiest is to disable it all. Fair enough. > I think lockdown suppose to disable xdp, bpfilter, nflog, raw sockets + pcap > too > otherwise even cap_net_admin can see traffic coming into host. > Similarly kprobe, perf_event, ftrace should be off as well? > I'm reasonably sure that lockdown is not intended to be this far reaching. cap_net_admin can see traffic coming into the host, and I don't think lockdown is intended to change that. David, I think this is exactly why you need to define what "lockdown" means. As it stands, the best argument I've seen involves "blacklisting", but that's a political thing and almost no one involved has any ability to evaluate it. Right now there's a series of patches that check for "lockdown" and seem to disable things that make someone uncomfortable. That's not a good way to design a security feature.
Re: [PATCH net 1/2] net: bcmgenet: Fix sparse warnings in bcmgenet_put_tx_csum()
On Mon, Apr 02, 2018 at 03:58:55PM -0700, Florian Fainelli wrote:
> skb->protocol is a __be16 which we would be calling htons() against,
> while this is not wrong per-se as it correctly results in swapping the
> value on LE hosts, this still upsets sparse. Adopt a similar pattern to
> what other drivers do and just assign ip_ver to skb->protocol, and then
> use htons() against the different constants such that the compiler can
> resolve the values at build time.
Fuck that and fuck other drivers sharing that "pattern". It's not hard
to remember:
host-endian to net-endian short is htons
host-endian to net-endian long is htonl
net-endian to host-endian short is ntohs
net-endian to host-endian long is ntohl
That's *it*. No convoluted changes needed, just use the right primitive.
You are turning trivial one-liners ("conversions between host- and net-endian
are the same in both directions, so the current code works even though we
are using the wrong primitive, confusing the readers. Use the right one")
which are absolutely self-contained and safe into much more massive changes
that are harder to follow and which are *not* self-contained at all.
Don't do it.
Re: [GIT PULL] Kernel lockdown for secure boot
On Tue, Apr 3, 2018 at 8:11 AM Andy Lutomirski wrote: > Can you explain that much more clearly? I'm asking why booting via > UEFI Secure Boot should enable lockdown, and I don't see what this has > to do with kexec. And "someone blacklist[ing] your key in the > bootloader" sounds like a political issue, not a technical issue. A kernel that allows users arbitrary access to ring 0 is just an overfeatured bootloader. Why would you want secure boot in that case?
Re: [PATCH v8 25/42] ARM: davinci: dm644x: add new clock init using common clock framework
On 04/03/2018 05:26 AM, Sekhar Nori wrote:
On Friday 16 March 2018 08:22 AM, David Lechner wrote:
+static struct resource dm644x_pll1_resources[] = {
+ {
+ .start = DAVINCI_PLL1_BASE,
+ .end= DAVINCI_PLL1_BASE + SZ_4K - 1,
The .end should be DAVINCI_PLL1_BASE + SZ_1K - 1, otherwise it prevents
PLL2 from getting registered.
+ .flags = IORESOURCE_MEM,
+ },
+};
+
+static struct platform_device dm644x_pll1_device = {
+ .name = "dm644x-pll1",
+ .id = -1,
+ .resource = dm644x_pll1_resources,
+ .num_resources = ARRAY_SIZE(dm644x_pll1_resources),
+};
+
+static struct resource dm644x_pll2_resources[] = {
+ {
+ .start = DAVINCI_PLL2_BASE,
+ .end= DAVINCI_PLL2_BASE + SZ_4K - 1,
And this too should be fixed, else it prevents the PSC from getting
registered.
+ .flags = IORESOURCE_MEM,
+ },
+};
With these fixed, I still had to enable 'clk_ignore_unused' on DM644x
EVM to get to NFS boot. I think root of the problem is that pm_runtime()
APIs are not working in the legacy boot mode.
This can be seen even on the DA850 LCDK in legacy boot. pm_genpd_summary
in debugfs shows all domains are off and there are no devices registered
under the "da850-psc1: emac" domain. NFS mounting still works on the
DA850 LCDK because clk_summary shows enable and prepare count of 4 for
emac. Not sure how that's happening. But on DM644x EVM, the emac clock
enable count is 0.
Still looking at whats going wrong here. I am testing your v8 branch
with clk-davinci branch from clk-next merged to get the fixes Stephen made.
In legacy mode, genpd is not being used. I didn't see any mechanism for
genpd lookup without device tree. So, we are still relying on the
matching in arch/arm/mach-davinci/pm_domain.c.
I suspect we need to fix the clock lookups in
drivers/clk/davinci/psc-dm644x.c.
LPSC_CLKDEV2(emac_clkdev, NULL, "davinci_emac.1",
"fck","davinci_mdio.0");
NULL might need to be changed to "fck" to be picked up by pm matching
and "davinci_emac.1" should be verified that it matches the actual EMAC
device name.
[GIT PULL 1/2] Kbuild updates for 4.17
Hi Linus, Please pull Kbuild updates for v4.17. You will see a merge conflict in arch/blackfin/kernel/bfin_ksyms.c This file was already removed since the blackfin support was entirely dropped. So, please remove it. The following changes since commit 0c8efd610b58cb23cefdfa12015799079aef94ae: Linux 4.16-rc5 (2018-03-11 17:25:09 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git tags/kbuild-v4.17 for you to fetch changes up to a95b37e20db9a2b05354eec009b2188523a21c8e: kbuild: get out of (2018-03-31 12:22:38 +0900) Kbuild updates for v4.17 - add a shell script to get Clang version - improve portability of build scripts - drop always-enabled CONFIG_THIN_ARCHIVE and remove unused code - rename built-in.o which is now thin archive to built-in.a - process clean/build targets one by one to get along with -j option - simplify ld-option - improve building with CONFIG_TRIM_UNUSED_KSYMS - define KBUILD_MODNAME even for objects shared among multiple modules - avoid linking multiple instances of same objects from composite objects - move to c_flags to include it only for C files - clean-up various Makefiles Cao jin (1): kbuild: fix modname for composite modules Masahiro Yamada (23): kbuild: process mixture of clean/build targets one by one kbuild: simplify ld-option implementation kbuild: remove command line interface LDFLAGS_MODULE from makefiles.txt kbuild: remove internally used LDFLAGS_vmlinux from kbuild.txt kbuild: clear LDFLAGS in the top Makefile kbuild: remove wrong 'touch' in adjust_autoksyms.sh kbuild: move 'scripts' target below kbuild: restore autoksyms.h touch to the top Makefile kbuild: move CONFIG_TRIM_UNUSED_KSYMS code unneeded for external module kbuild: move include/config/ksym/* to include/ksym/* kbuild: link vmlinux only once for CONFIG_TRIM_UNUSED_KSYMS kbuild: remove unnecessary $(subst $(obj)/, , ...) in modname-multi kbuild: define KBUILD_MODNAME even if multiple modules share objects kbuild: simplify modname calculation kbuild: move modname and modname-multi close to modname_flags kbuild: rename real-objs-y/m to real-obj-y/m kbuild: link $(real-obj-y) instead of $(obj-y) into built-in.a lib: zstd: clean up Makefile for simpler composite object handling net: liquidio: clean up Makefile for simpler composite object handling kbuild: remove partial section mismatch detection for built-in.a kbuild: clean up archive rule of built-in.a kbuild: clean up link rule of composite modules kbuild: get out of Michael Forney (2): kbuild: Improve portability of some sed invocations kbuild: Use ls(1) instead of stat(1) to obtain file size Nicholas Piggin (2): kbuild: remove incremental linking option kbuild: rename built-in.o to built-in.a Sami Tolvanen (1): kbuild: add clang-version.sh .gitignore| 1 + Documentation/kbuild/kbuild.txt | 4 -- Documentation/kbuild/makefiles.txt| 28 - Documentation/process/changes.rst | 2 +- Makefile | 89 ++- arch/Kconfig | 6 -- arch/arm/boot/deflate_xip_data.sh | 2 +- arch/blackfin/kernel/bfin_ksyms.c | 2 +- arch/powerpc/boot/wrapper | 2 +- arch/powerpc/kernel/Makefile | 2 +- drivers/net/ethernet/cavium/liquidio/Makefile | 51 +--- drivers/s390/Makefile | 2 +- include/linux/kconfig.h | 3 - lib/Kconfig.debug | 4 +- lib/zstd/Makefile | 17 ++ scripts/Kbuild.include| 6 +- scripts/Makefile.build| 97 ++ scripts/Makefile.lib | 47 +++ scripts/adjust_autoksyms.sh | 9 ++- scripts/basic/fixdep.c| 8 +-- scripts/clang-version.sh | 33 ++ scripts/file-size.sh | 4 ++ scripts/gen_initramfs_list.sh | 2 +- scripts/headers_install.sh| 10 ++-- scripts/kconfig/Makefile | 2 - scripts/link-vmlinux.sh | 103 scripts/namespace.pl | 2 +- usr/initramfs_data.S | 2 +- 28 files changed, 235 insertions(+), 305 deletions(-) create mode 100755 scripts/clang-version.sh create mode 100755 scripts/file-size.sh -- Best
Re: stable-rc build: 4 warnings 0 failures (stable-rc/v4.15.15)
On Tue, Apr 03, 2018 at 04:47:31PM +0200, Arnd Bergmann wrote:
> On Sat, Mar 31, 2018 at 7:49 PM, Olof's autobuilder wrote:
>
> > Warnings:
> >
> > arm64.allmodconfig:
> > WARNING: modpost: missing MODULE_LICENSE() in
> > drivers/phy/qualcomm/phy-qcom-ufs.o
>
> This needs a backport of
>
> 59fba0869aca ("phy: qcom-ufs: add MODULE_LICENSE tag")
Now added, thanks!
greg k-h
Re: [PATCH net 1/2] net: bcmgenet: Fix sparse warnings in bcmgenet_put_tx_csum()
From: Al Viro
Date: Tue, 3 Apr 2018 17:29:33 +0100
> On Mon, Apr 02, 2018 at 03:58:55PM -0700, Florian Fainelli wrote:
>> skb->protocol is a __be16 which we would be calling htons() against,
>> while this is not wrong per-se as it correctly results in swapping the
>> value on LE hosts, this still upsets sparse. Adopt a similar pattern to
>> what other drivers do and just assign ip_ver to skb->protocol, and then
>> use htons() against the different constants such that the compiler can
>> resolve the values at build time.
>
> Fuck that and fuck other drivers sharing that "pattern". It's not hard
> to remember:
> host-endian to net-endian short is htons
> host-endian to net-endian long is htonl
> net-endian to host-endian short is ntohs
> net-endian to host-endian long is ntohl
>
> That's *it*. No convoluted changes needed, just use the right primitive.
> You are turning trivial one-liners ("conversions between host- and net-endian
> are the same in both directions, so the current code works even though we
> are using the wrong primitive, confusing the readers. Use the right one")
> which are absolutely self-contained and safe into much more massive changes
> that are harder to follow and which are *not* self-contained at all.
>
> Don't do it.
Yes Al, however the pattern choosen here is probably cheaper on little endian:
__beXX val = x;
switch (val) {
case htons(ETH_P_FOO):
...
}
This way only the compiler byte swaps the constants at compile time,
no code is actually generated to do it.
Re: [RFC PATCH for 4.17 02/21] rseq: Introduce restartable sequences system call (v12)
- On Apr 2, 2018, at 11:33 AM, Mathieu Desnoyers mathieu.desnoy...@efficios.com wrote: > - On Apr 1, 2018, at 12:13 PM, One Thousand Gnomes > gno...@lxorguk.ukuu.org.uk wrote: > >> On Tue, 27 Mar 2018 12:05:23 -0400 >> Mathieu Desnoyers wrote: >> >>> Expose a new system call allowing each thread to register one userspace >>> memory area to be used as an ABI between kernel and user-space for two >>> purposes: user-space restartable sequences and quick access to read the >>> current CPU number value from user-space. >> >> What is the *worst* case timing achievable by using the atomics ? What >> does it do to real time performance requirements ? > > Given that there are two system calls introduced in this series (rseq and > cpu_opv), can you clarify which system call you refer to in the two questions > above ? > > For rseq, given that its userspace works pretty much like a read seqlock > (it retries on failure), it has no impact whatsoever on scheduler behavior. > So characterizing its worst case timing does not appear to be relevant. > >> For cpu_opv you now >> give an answer but your answer is assuming there isn't another thread >> actively thrashing the cache or store buffers, and that the user didn't >> sneakily pass in a page of uncacheable memory (eg framebuffer, or GPU >> space). > > Are those considered as device pages ? > >> >> I don't see anything that restricts it to cached pages. With that check >> in place for x86 at least it would probably be ok and I think the sneaky >> attacks to make it uncacheable would fail becuase you've got the pages >> locked so trying to give them to an accelerator will block until you are >> done. >> >> I still like the idea it's just the latencies concern me. > > Indeed, cpu_opv touches pages that are shared with user-space with > preemption off, so this one affects the scheduler latency. The worse-case > timings I measured for cpu_opv were with cache-cold memory. So I expect that > another thread actively trashing the cache would be in the same ballpark > figure. It does not account for a concurrent thread thrashing the store > buffers though. > > The checks enforcing which pages can be touched by cpu_opv operations are > done within cpu_op_check_page(). is_zone_device_page() is used to ensure no > device page is touched with preempt disabled. I understand that you would > prefer to disallow pages of uncacheable memory as well, which I'm fine with. > Is there an API similar to is_zone_device_page() to check whether a page is > uncacheable ? Looking into this a bit more, I notice the following: The pgprot_noncached (_PAGE_NOCACHE on x86) pgprot is part of the vma->vm_page_prot. Therefore, in order to have userspace provide pointers to noncached pages as input to cpu_opv, they need to be part of a userspace vma which has a pgprot_noncached vm_page_prot. The cpu_opv system call uses get_user_pages_fast() to grab the struct page from the userspace addresses, and then passes those pages to vm_map_ram(), with a PAGE_KERNEL pgprot. This creates a temporary kernel mapping to those pages, which is then used to read/write from/to those pages with preemption disabled. Therefore, with the proposed cpu_opv implementation, the kernel is not touching noncached mappings with preemption disabled, which should take care of your latency concern. Am I missing something ? Thanks, Mathieu -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com
Re: [PATCH v17 01/10] LIB: Introduce a generic PIO mapping method
On Tue, Apr 03, 2018 at 05:01:37PM +0100, John Garry wrote:
> > > > +int logic_pio_register_range(struct logic_pio_hwaddr *new_range)
> > > > +{
> > > > + struct logic_pio_hwaddr *range;
> > > > + resource_size_t start = new_range->hw_start;
> > > > + resource_size_t end = new_range->hw_start + new_range->size;
> > > > + resource_size_t mmio_sz = 0;
> > > > + resource_size_t iio_sz = MMIO_UPPER_LIMIT;
> > > > + int ret = 0;
> > > > +
> > > > + if (!new_range || !new_range->fwnode || !new_range->size)
> > > > + return -EINVAL;
> > > > +
> > > > + mutex_lock(_range_mutex);
> > > > + list_for_each_entry_rcu(range, _range_list, list) {
> > > > + if (range->fwnode == new_range->fwnode) {
> > > > + /* range already there */
> > > > + ret = -EFAULT;
> > > > + goto end_register;
> > > > + }
> > >
>
> Hi Thierry,
>
> > > This is the -EFAULT that propagates to pci-tegra.c's ->probe() and fails
> > > to bind the driver.
> > >
> > > I'm not exactly sure what's causing the duplicate here because it's
> > > rather difficult to get at something useful from just the ->fwnode, but
> > > I'm fairly sure that the reason this breaks is because the Tegra driver
> > > will defer probe due to some regulators that aren't available on the
> > > first try. Given the above code and the rest of this file, I can't see a
> > > way to "fix" the driver and remove the I/O range on failure.
> > >
> > > This is doubly bad because this doesn't only leak the ranges on probe
> > > deferral, but also on driver unload, and we just added support for
> > > building the Tegra driver as a loadable module, so these are actually
> > > cases that can happen in regular uses of the driver.
> > >
> > > I have no idea on how to fix this. Anyone know of a quick fix to restore
> > > PCI for Tegra other than reverting all of these changes?
> > >
> > > I suppose an API could be added to unregister the range, but the calling
> > > sequence is rather obfuscated, so removing the range will look totally
> > > asymmetric, I'm afraid.
> > >
> > > Here's the call stack:
> > >
> > > tegra_pcie_probe()
> > > tegra_pcie_parse_dt()
> > > of_pci_range_to_resource()
> > > pci_register_io_range()
> > > logic_pio_register_range()
> > >
> > > So the range here is registered as part of a resource parsing function,
> > > which is supposed to not have any side-effects. There's no equivalent of
> > > that parsing routine (i.e. no "unparse" function that would undo the
> > > effects of parsing).
> > >
> > > Perhaps a cleaner way would be to decouple the parsing from the actual
> > > request step that has the side-effect.
>
> This could be added if we agreed that it would be useful.
I guess in most cases these ranges will be static at least during one
boot. But it still feels like this should be removed when the driver
goes away. While this may not depend on data by the driver, and hence
won't cause a crash or anything, it just seems wrong to leave it
around when the driver no longer isn't.
> > > Going back in history a little, it looks like even before this commit
> > > the I/O range registration was triggered by the parsing code and even
> > > the range leak was there, except that it caused pci_register_io_range()
> > > to return 0 rather than -EFAULT. Perhaps the quickest fix for this would
> > > be to do the same in the new code and restore drivers that accidentally
> > > depend on this behaviour.
> >
> > I can confirm that the following fixes the issue for me, though I don't
> > think it's a very clean fix given that the range will remain requested
> > forever, even if the driver is gone. But since that's already been the
> > case for quite a while, probably something that can be fixed separately.
> >
>
> Right, there was no way to deregister the range previously. From looking at
> the history here I see no reason to not support it.
>
> As for this patch, as you said, the only difference is that we fault on
> trying to register the same range again. So this solution seems reasonable.
Okay, I can turn this into a proper patch to fix this up. I suspect that
other drivers may be subject to the same regression. For the longer term
I think it'd be better to properly undo the registration on failure and
removal, but I suspect that it'd be quite a bit of work and not suitable
for v4.17 anymore.
> On another point, for the tegra driver, is it possible to defer earlier in
> the probe, before these currently irreversible steps are taken?
I'm sure it'd be possible. But it would be quite involved, I think. The
reason the code is the way it is is because parsing the DT didn't use to
have side-effects.
Also, I don't think it would buy us much because the probe can still
defer (or at least fail) as late as pci_scan_root_bus_bridge(). Even if
we work around the probe deferral by moving the DT parsing
Re: [PATCH v4 13/14] dt-bindings: cpufreq: Document operating-points-v2-kryo-cpu
Hi Viresh, On 4/3/2018 9:53 AM, Viresh Kumar wrote: > On 03-04-18, 08:11, Sricharan R wrote: >> Right, i was adding a similar one for krait cores [1]. There is code common >> in the >> init sequence across both (little). Do you suggest to make them common ? > > It may make sense as we are talking about one SoC family here :) > ok. So either of us can merge, depending upon which one goes first. Regards, Sricharan -- "QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
[GIT PULL 2/2] Kconfig updates for 4.17
Hi Linus, Please pull Kconfig updates for 4.17. The following changes since commit 0c8efd610b58cb23cefdfa12015799079aef94ae: Linux 4.16-rc5 (2018-03-11 17:25:09 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git tags/kconfig-v4.17 for you to fetch changes up to 18492685e479fd4d8e1dca836f57c11b6800f083: kconfig: use yylineno option instead of manual lineno increments (2018-03-26 02:04:07 +0900) Kconfig updates for v4.17 - improve checkpatch for more precise Kconfig code checking - clarify effective selects by grouping reverse dependencies in help - do not write out '# CONFIG_FOO is not set' from invisible symbols - make oldconfig as silent as it should be - rename 'silentoldconfig' to 'syncconfig' - add unit-test framework and several test cases - warn unmet dependency of tristate symbols - make unmet dependency warnings readable, removing false positives - improve recursive include detection - use yylineno to simplify the line number tracking - misc cleanups Eugeniu Rosca (1): kconfig: Print reverse dependencies in groups Masahiro Yamada (24): kconfig: clean-up reverse dependency help implementation kconfig: do not call check_conf() for olddefconfig kconfig: remove unneeded input_mode test in conf() kconfig: remove redundant input_mode test for check_conf() loop kconfig: hide irrelevant sub-menus for oldconfig kconfig: invoke oldconfig instead of silentoldconfig from local*config kconfig: rename silentoldconfig to syncconfig kbuild: add PYTHON2 and PYTHON3 variables kconfig: tests: add framework for Kconfig unit testing kconfig: tests: add basic choice tests kconfig: tests: test automatic submenu creation kconfig: tests: test if new symbols in choice are asked kconfig: tests: check unneeded "is not set" with unmet dependency kconfig: tests: check visibility of tristate choice values in y choice kconfig: tests: test defconfig when two choices interact kconfig: tests: test randconfig for choice in choice kconfig: tests: test if recursive dependencies are detected kconfig: tests: test if recursive inclusion is detected kconfig: warn unmet direct dependency of tristate symbols selected by y kconfig: make unmet dependency warnings readable kconfig: do not include both curses.h and ncurses.h for nconfig kconfig: remove duplicated file name and lineno of recursive inclusion kconfig: detect recursive inclusion earlier kconfig: use yylineno option instead of manual lineno increments Ulf Magnusson (5): checkpatch: kconfig: recognize more prompts when checking help texts checkpatch: kconfig: check help texts for menuconfig and choice checkpatch: kconfig: prefer 'help' over '---help---' kconfig: only write '# CONFIG_FOO is not set' for visible symbols kconfig: remove redundant streamline_config.pl prerequisite Documentation/kbuild/kconfig.txt | 2 +- Documentation/networking/i40e.txt | 2 +- Makefile | 6 +- scripts/checkpatch.pl | 21 +- scripts/kconfig/Makefile | 29 ++- scripts/kconfig/conf.c | 41 ++-- scripts/kconfig/expr.c | 86 +++ scripts/kconfig/expr.h | 4 +- scripts/kconfig/lkc.h | 1 + scripts/kconfig/menu.c | 12 +- scripts/kconfig/nconf.h| 4 +- scripts/kconfig/symbol.c | 40 ++-- scripts/kconfig/tests/auto_submenu/Kconfig | 50 scripts/kconfig/tests/auto_submenu/__init__.py | 12 + scripts/kconfig/tests/auto_submenu/expected_stdout | 10 + scripts/kconfig/tests/choice/Kconfig | 54 + scripts/kconfig/tests/choice/__init__.py | 40 scripts/kconfig/tests/choice/alldef_expected_config| 5 + scripts/kconfig/tests/choice/allmod_expected_config| 9 + scripts/kconfig/tests/choice/allno_expected_config | 5 + scripts/kconfig/tests/choice/allyes_expected_config| 9 + scripts/kconfig/tests/choice/oldask0_expected_stdout | 10 + scripts/kconfig/tests/choice/oldask1_config| 2 + scripts/kconfig/tests/choice/oldask1_expected_stdout | 15 ++ scripts/kconfig/tests/choice_value_with_m_dep/Kconfig | 19 ++ .../kconfig/tests/choice_value_with_m_dep/__init__.py | 15 ++ scripts/kconfig/tests/choice_value_with_m_dep/config | 2 + .../tests/choice_value_with_m_dep/expected_config | 3 +
Re: [PATCH net 1/2] net: bcmgenet: Fix sparse warnings in bcmgenet_put_tx_csum()
On Tue, Apr 03, 2018 at 12:33:05PM -0400, David Miller wrote:
> Yes Al, however the pattern choosen here is probably cheaper on little endian:
>
> __beXX val = x;
> switch (val) {
> case htons(ETH_P_FOO):
>...
> }
>
> This way only the compiler byte swaps the constants at compile time,
> no code is actually generated to do it.
That's not obvious, actually - depends upon how sparse the switch ends
up being. You can easily lose more than a single byteswap insn on
worse cascase of comparisons.
Re: [GIT PULL] Kernel lockdown for secure boot
On Tue, Apr 3, 2018 at 9:29 AM, Matthew Garrett wrote: > On Tue, Apr 3, 2018 at 8:11 AM Andy Lutomirski wrote: >> Can you explain that much more clearly? I'm asking why booting via >> UEFI Secure Boot should enable lockdown, and I don't see what this has >> to do with kexec. And "someone blacklist[ing] your key in the >> bootloader" sounds like a political issue, not a technical issue. > > A kernel that allows users arbitrary access to ring 0 is just an > overfeatured bootloader. Why would you want secure boot in that case? To get a chain of trust. I can provision a system with some public keys, stored in UEFI authenticated variables, such that the system will only boot a signed image. That signed image, can, in turn, load a signed (or hashed or otherwise verfified) kernel and a verified initramfs. The initramfs can run a full system from a verified (using dm-verity or similar) filesystem, for example. Now it's very hard to persistently attack this system. Chromium OS does something very much like this, except that it doesn't use UEFI as far as I know. So does iOS, and so do some Android versions. None of this requires lockdown, or even a separation between usermode and kernelmode, to work correctly. One could even do this on an MMU-less system if one really cared to. More usefully, someone probably has done this using a unikernel. If I had to guess at a motivation that makes this patchset work, it would be that there is an uneasy truce between Microsoft and the various vendors of signed Linux bootloaders. That truce could conceivably require that the signed bootloaders not knowingly ship a system that allows a non-physically-present user to chainload Windows. If so, the patchset should say that loud and clear in its description and the parts that block bpf should go away.
Re: [PATCH v3 2/4] mfd: add Gateworks System Controller core driver
On Tue, Apr 03, 2018 at 08:48:27AM -0700, Tim Harvey wrote:
> On Wed, Mar 28, 2018 at 8:14 AM, Tim Harvey wrote:
> > The Gateworks System Controller (GSC) is an I2C slave controller
> > implemented with an MSP430 micro-controller whose firmware embeds the
> > following features:
> > - I/O expander (16 GPIO's) using PCA955x protocol
> > - Real Time Clock using DS1672 protocol
> > - User EEPROM using AT24 protocol
> > - HWMON using custom protocol
> > - Interrupt controller with tamper detect, user pushbotton
> > - Watchdog controller capable of full board power-cycle
> > - Power Control capable of full board power-cycle
> >
> > see http://trac.gateworks.com/wiki/gsc for more details
> >
>
> > +
> > +/*
> > + * gsc_powerdown - API to use GSC to power down board for a specific time
> > + *
> > + * secs - number of seconds to remain powered off
> > + */
> > +static int gsc_powerdown(struct gsc_dev *gsc, unsigned long secs)
> > +{
> > + int ret;
> > + unsigned char regs[4];
> > +
> > + dev_info(>i2c->dev, "GSC powerdown for %ld seconds\n",
> > +secs);
> > + regs[0] = secs & 0xff;
> > + regs[1] = (secs >> 8) & 0xff;
> > + regs[2] = (secs >> 16) & 0xff;
> > + regs[3] = (secs >> 24) & 0xff;
> > + ret = regmap_bulk_write(gsc->regmap, GSC_TIME_ADD, regs, 4);
> > +
> > + return ret;
> > +}
>
> Any feedback on the 'powerdown' sysfs attribute that hooks to this
> function? This allows the GSC to disable the board primary power
> supply for 2^32 seconds and is often used to 'reset' the board
> although it could also be used to put the board in a power down state
> longer. I'm wondering if there is a more appropriate API for this in
> the kernel that I don't know about.
Hi Tim
RTC can cause wakeup when an alarm is set. It looks like the DS1672
does not have this. But you are emulating the DS1672 right? You could
add a second emulated RTC which does support an alarm? DS3232?
> I would also like to register a restart handler using this but I
> believe that ARM restart handlers currently can not use I2C - is that
> correct?
There are plenty which use GPIOs, or UARTs. Not seen any which use
i2c. What do you think does not work at this point?
Andrew
Re: [LINUX PATCH v8 2/2] memory: pl353: Add driver for arm pl353 static memory controller
Hello-
On Wed, Mar 14, 2018 at 04:14:34PM +0530, nagasureshkumarre...@gmail.com wrote:
> From: Naga Sureshkumar Relli
I'm pleased to see this patchset revived and resubmitted!
It would be easier to follow if you constructed your two patchsets with
git format-patch --thread.
Or, merge them into a single patchset, especially considering the
dependency between patches.
Some code review comments below:
> Add driver for arm pl353 static memory controller. This controller is
> used in xilinx zynq soc for interfacing the nand and nor/sram memory
> devices.
>
> Signed-off-by: Naga Sureshkumar Relli
> ---
> Changes in v8:
> - None
> Changes in v7:
> - Corrected the kconfig to use tristate selection
> - Corrected the GPL licence ident
> - Added boundary checks for nand timing parameters
> Changes in v6:
> - Fixed checkpatch.pl reported warnings
> Changes in v5:
> - Added pl353_smc_get_clkrate function, made pl353_smc_set_cycles as public
> API
> - Removed nand timing parameter initialization and moved it to nand driver
> Changes in v4:
> - Modified driver to support multiple instances
> - Used sleep instaed of busywait for delay
> Changes in v3:
> - None
> Changes in v2:
> - Since now the timing parameters are in nano seconds, added logic to convert
> them to the cycles
> ---
> drivers/memory/Kconfig | 7 +
> drivers/memory/Makefile | 1 +
> drivers/memory/pl353-smc.c | 548
>
> include/linux/platform_data/pl353-smc.h | 34 ++
> 4 files changed, 590 insertions(+)
> create mode 100644 drivers/memory/pl353-smc.c
> create mode 100644 include/linux/platform_data/pl353-smc.h
>
> diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig
> index 19a0e83..d70d6db 100644
> --- a/drivers/memory/Kconfig
> +++ b/drivers/memory/Kconfig
> @@ -152,6 +152,13 @@ config DA8XX_DDRCTL
> This driver is for the DDR2/mDDR Memory Controller present on
> Texas Instruments da8xx SoCs. It's used to tweak various memory
> controller configuration options.
> +config PL35X_SMC
> + bool "ARM PL35X Static Memory Controller(SMC) driver"
Is there any reason why this can't be tristate?
[..]
> +++ b/drivers/memory/pl353-smc.c
[..]
> +/**
> + * struct pl353_smc_data - Private smc driver structure
> + * @devclk: Pointer to the peripheral clock
> + * @aperclk: Pointer to the APER clock
> + */
> +struct pl353_smc_data {
> + struct clk *memclk;
> + struct clk *aclk;
> +};
> +
> +/* SMC virtual register base */
> +static void __iomem *pl353_smc_base;
While it's true that the Zynq chips only have a single instance of this
IP, there is no real reason why an SoC can't instantiate more than one.
I'm a bit uncomfortable with the fact that this has been designed out.
> +
> +/**
> + * pl353_smc_set_buswidth - Set memory buswidth
> + * @bw: Memory buswidth (8 | 16)
> + * Return: 0 on success or negative errno.
> + */
> +int pl353_smc_set_buswidth(unsigned int bw)
> +{
> +
> + if (bw != PL353_SMC_MEM_WIDTH_8 && bw != PL353_SMC_MEM_WIDTH_16)
> + return -EINVAL;
> +
> + writel(bw, pl353_smc_base + PL353_SMC_SET_OPMODE_OFFS);
There should be no reason not to use the _relaxed() accessor variants.
> + writel(PL353_SMC_DC_UPT_NAND_REGS, pl353_smc_base +
> +PL353_SMC_DIRECT_CMD_OFFS);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(pl353_smc_set_buswidth);
> +
> +/**
> + * pl353_smc_set_cycles - Set memory timing parameters
> + * @t0: t_rcread cycle time
> + * @t1: t_wcwrite cycle time
> + * @t2: t_rea/t_ceoeoutput enable assertion delay
> + * @t3: t_wpwrite enable deassertion delay
> + * @t4: t_clr/t_pc page cycle time
> + * @t5: t_ar/t_ta ID read time/turnaround time
> + * @t6: t_rrbusy to RE timing
> + *
> + * Sets NAND chip specific timing parameters.
> + */
> +static void pl353_smc_set_cycles(u32 t0, u32 t1, u32 t2, u32 t3, u32
> + t4, u32 t5, u32 t6)
> +{
> + t0 &= PL353_SMC_SET_CYCLES_T0_MASK;
> + t1 = (t1 & PL353_SMC_SET_CYCLES_T1_MASK) <<
> + PL353_SMC_SET_CYCLES_T1_SHIFT;
> + t2 = (t2 & PL353_SMC_SET_CYCLES_T2_MASK) <<
> + PL353_SMC_SET_CYCLES_T2_SHIFT;
> + t3 = (t3 & PL353_SMC_SET_CYCLES_T3_MASK) <<
> + PL353_SMC_SET_CYCLES_T3_SHIFT;
> + t4 = (t4 & PL353_SMC_SET_CYCLES_T4_MASK) <<
> + PL353_SMC_SET_CYCLES_T4_SHIFT;
> + t5 = (t5 & PL353_SMC_SET_CYCLES_T5_MASK) <<
> + PL353_SMC_SET_CYCLES_T5_SHIFT;
> + t6 = (t6 & PL353_SMC_SET_CYCLES_T6_MASK) <<
> + PL353_SMC_SET_CYCLES_T6_SHIFT;
> +
> + t0 |= t1 | t2 | t3 | t4 | t5 | t6;
> +
> + writel(t0, pl353_smc_base + PL353_SMC_SET_CYCLES_OFFS);
> + writel(PL353_SMC_DC_UPT_NAND_REGS, pl353_smc_base +
> +
RE: [PATCH v5 03/14] PCI: Add pcie_bandwidth_capable() to compute max supported link bandwidth
> -Original Message- > From: Bjorn Helgaas [mailto:helg...@kernel.org] > Sent: Tuesday, April 03, 2018 7:06 AM > To: Jacob Keller > Cc: Tal Gilboa ; Tariq Toukan ; > Keller, Jacob E ; Ariel Elior > ; > Ganesh Goudar ; Kirsher, Jeffrey T > ; everest-linux...@cavium.com; intel-wired- > l...@lists.osuosl.org; net...@vger.kernel.org; linux-kernel@vger.kernel.org; > linux-...@vger.kernel.org > Subject: Re: [PATCH v5 03/14] PCI: Add pcie_bandwidth_capable() to compute > max supported link bandwidth > > On Mon, Apr 02, 2018 at 05:30:54PM -0700, Jacob Keller wrote: > > On Mon, Apr 2, 2018 at 7:05 AM, Bjorn Helgaas wrote: > > > +/* PCIe speed to Mb/s reduced by encoding overhead */ > > > +#define PCIE_SPEED2MBS_ENC(speed) \ > > > + ((speed) == PCIE_SPEED_16_0GT ? (16000*(128/130)) : \ > > > +(speed) == PCIE_SPEED_8_0GT ? (8000*(128/130)) : \ > > > +(speed) == PCIE_SPEED_5_0GT ? (5000*(8/10)) : \ > > > +(speed) == PCIE_SPEED_2_5GT ? (2500*(8/10)) : \ > > > +0) > > > + > > > > Should this be "(speed * x ) / y" instead? wouldn't they calculate > > 128/130 and truncate that to zero before multiplying by the speed? Or > > are compilers smart enough to do this the other way to avoid the > > losses? > > Yep, thanks for saving me yet more embarrassment. That's what patch review is for :D Thanks, Jake
Re: [RFC PATCH v1] fw_lockdown: new micro LSM module to prevent loading unsigned firmware
On Mon, Apr 02, 2018 at 05:42:22PM -0700, Andy Lutomirski wrote: > On 11/10/2017 01:02 PM, Mimi Zohar wrote: > > If the kernel is locked down and IMA-appraisal is not enabled, prevent > > loading of unsigned firmware. > > > diff --git a/security/fw_lockdown/Kconfig b/security/fw_lockdown/Kconfig > > new file mode 100644 > > index ..d6aef6ce8fee > > --- /dev/null > > +++ b/security/fw_lockdown/Kconfig > > @@ -0,0 +1,6 @@ > > +config SECURITY_FW_LOCKDOWN > > + bool "Prevent loading unsigned firmware" > > + depends on LOCK_DOWN_KERNEL > > + default y > > + help > > + Prevent loading unsigned firmware in lockdown mode, > > Please be honest about what this does. This option makes your system > useless if you don't use IMA-Appraisal and it offers a particular security > benefit if you do you IMA-Appraisal. How about making it depend on > IMA-Appraisal? Change the name to SECURITY_ONLY_LOAD_IMA_APPRAISED_FIRMWARE > and adjust the text accordingly, please. Mimi is on vacation right now so I'll address this. All the above makes sense, but just keep in mind the patch was posted just as an illustration of how IMA *can* live up to the original intent proposed by David Howells on kernel lockdown. David's old kernel lockdown documentation stipulated that a requirement was to also prevent loading unsigned firmware. Does the kernel lockdown documentation still have a section indicating signed firmware is a requirement? Or was that removed in the end? Mimi's RFC was at a time when we still had on our roadmap the prospect of adding generic signed firmware support into Linux. Mimi's point and RFC patch was an illustration then on how IMA users can already meet these assumed requirements on 'kernel lockdown', and that if we used a micro LSM hook this could be easily managed, given the firmware signing support we intended on adding would not be needed for IMA systems. Her point was that with IMA you can already meet the definition and IMA systems would not have to wait for "generic firmware signing" to be merged. The biggest thing which has changed since then is that we decided to *not* support or streamline generic firmware signing (non-IMA) for now for a few reasons [0] [1] which are important to re-iterate as these are easy to forget, and AFAICT not documented anywhere. One was that my own requirement for it was already done -- cfg80211 already open codes firmware signing checking on its own through the new CONFIG_CFG80211_REQUIRE_SIGNED_REGDB thereby also not requiring CRDA anymore, the userspace component which used to read the signed regulatory database and then send regulatory content to the wireless subsystem. So CRDA is no longer needed if you enable CONFIG_CFG80211_REQUIRE_SIGNED_REGDB. Second, is is that the whole UEFI movement pushed hardware vendors to start embracing requiring signed firmware on peripherals themselves. So a lot of hardware these days *does* do firmware signing already and a generic kernel firmware signing facility would then just make us do double work then. There are exceptions and there are a few reasons for this. For instance USB devices are not allowed to pee on random memory (thanks for the analogy Alan!), so not all USB devices have support for checking their own firmware signature. This is one reason also why why some operating systems do not allow users to use random USB devices. If companies *really* need to vet for these they have a few options, one is to use IMA, the other is to have the driver open code the firmware signing component just as we did with cfg80211 through the new CONFIG_CFG80211_REQUIRE_SIGNED_REGDB. If we end up later with enough open coded firmware checking users we can later revisit adding a generic firmware signing facility. Third is that there is no strong evidence of any security issue with having firmware in /lib/firmware not be signed. I can attest to this, in my research of all the history of this, I cannot find a single incident which calls bloody murder for unsigned firmware. There are concerns and known backdoors on firmware for storage drives for instance, but these are not /lib/firmware files, for instance. If you also consider the second reason above, it may also help explain why perhaps there is also any serious threat on this front. Granted, there are known cases know of a third party using some signing key to sign malicious firmware and using that to exploit an OS, but if they did that and we trusted each vendor signing key as well we'd be just as vulnerable. Fourth, if you want signed /lib/firmware, you might want signed binaries, and if you want signed binaries, we already have IMA. So.. until there is no real strong reason to support and maintain a generic firmware signing mechanism, no need to add it. We can wait for the open coded boom, and if that really does happen we'll revisit later. So the kernel lockdown definition may want to revise all the above, review its documentation and if the "firmware
[PATCH 1/3] Documentation/features: Add script that refreshes the arch support status files in place
Suggested-by: Ingo Molnar Signed-off-by: Andrea Parri --- Documentation/features/scripts/features-refresh.sh | 55 ++ 1 file changed, 55 insertions(+) create mode 100755 Documentation/features/scripts/features-refresh.sh diff --git a/Documentation/features/scripts/features-refresh.sh b/Documentation/features/scripts/features-refresh.sh new file mode 100755 index 0..ae3e9d5d3f262 --- /dev/null +++ b/Documentation/features/scripts/features-refresh.sh @@ -0,0 +1,55 @@ +# +# Small script that refreshes the kernel feature support status in place. +# + +for F_FILE in Documentation/features/*/*/arch-support.txt; do + K=$(grep "^# Kconfig:" "$F_FILE" | cut -c26-) + K_VALID="false" # K is 'valid' iff there exists a Kconfig file + # (for some arch) containing K. + + for ARCH_DIR in arch/*/; do + K_FILES=$(find $ARCH_DIR -name "Kconfig*") + + K_GREP=$(grep "$K" $K_FILES) + if [ ! -z "$K_GREP" ]; then + K_VALID="true" + break + fi + done + + if [ "$K_VALID" = "false" ]; then + printf "WARNING: '%s' is not a valid Kconfig\n" "$K" + fi + + T_FILE="$F_FILE.tmp" + + grep "^#" $F_FILE > $T_FILE + echo "---" >> $T_FILE + echo "| arch |status|" >> $T_FILE + echo "---" >> $T_FILE + + for ARCH_DIR in arch/*/; do + ARCH=$(echo $ARCH_DIR | sed -e 's/arch//g' | sed -e 's/\///g') + K_FILES=$(find $ARCH_DIR -name "Kconfig*") + + K_GREP=$(grep "$K" $K_FILES) + if [ ! -z "$K_GREP" ]; then + # K is 'supported by a given arch', if there exists + # a Kconfig file for this arch containing K. + printf "|%12s: | ok |\n" "$ARCH" >> $T_FILE + else + # ... Otherwise: Keep the original status (if any); + # default to "not yet supported". + S=$(grep -v "^#" "$F_FILE" | grep " $ARCH:") + if [ ! -z "$S" ]; then + echo "$S" >> $T_FILE + else + printf "|%12s: | TODO |\n" "$ARCH" \ + >> $T_FILE + fi + fi + done + + echo "---" >> $T_FILE + mv $T_FILE $F_FILE +done -- 2.7.4
[RFC PATCH 0/3] Documentation/features: Provide and apply "features-refresh.sh"
In Ingo's words [1]: "[...] what should be done instead is to write a script that refreshes all the arch-support.txt files in-place. [...] It's OK for the script to have various quirks for weirdly implemented features and exceptions: i.e. basically whenever it gets a feature wrong, we can just tweak the script with quirks to make it all work out of box. [...] But in the end there should only be a single new script: Documentation/features/scripts/features-refresh.sh ... which operates on the arch-support.txt files and refreshes them in place, and which, after all the refreshes have been committed, should produce an empty 'git diff' result." "[...] New features can then be added by basically just creating a header-only arch-support.txt file, such as: triton:~/tip/Documentation/features> cat foo/bar/arch-support.txt # # Feature name: shiny new fubar kernel feature # Kconfig: ARCH_USE_FUBAR # description: arch supports the fubar feature # And running Documentation/features/scripts/features-refresh.sh would auto-generate the arch support matrix. [...] This way we soft- decouple the refreshing of the entries from the introduction of the features, while still making it all easy to keep sync and to extend." This RFC presents a first attempt to implement such a feature/script, and applies it script on top of Arnd's: git://git.kernel.org/pub/scm/linux/kernel/git/arnd/asm-generic.git arch-removal Patch 1/3 provides the "features-refresh.sh" script. Patch 2/3 removes the "BPF-JIT" feature file and it creates header-only files for "cBPF-JIT" and "eBPF-JIT". Patch 3/3 presents the results of running the script; this run also printed to standard output the following warnings: WARNING: '__HAVE_ARCH_STRNCASECMP' is not a valid Kconfig WARNING: 'Optimized asm/rwsem.h' is not a valid Kconfig WARNING: '!ARCH_USES_GETTIMEOFFSET' is not a valid Kconfig WARNING: '__HAVE_ARCH_PTE_SPECIAL' is not a valid Kconfig (I'm sending these patches with empty commit messagges, for early feedback: I'll fill in these messages in subsequent versions if this makes sense...) Cheers, Andrea Andrea Parri (3): Documentation/features: Add script that refreshes the arch support status files in place Documentation/features/core: Add arch support status files for 'cBPF-JIT' and 'eBPF-JIT' Documentation/features: Refresh and auto-generate the arch support status files in place .../features/core/BPF-JIT/arch-support.txt | 31 .../features/core/cBPF-JIT/arch-support.txt| 32 + .../features/core/eBPF-JIT/arch-support.txt| 32 + .../core/generic-idle-thread/arch-support.txt | 3 +- .../features/core/jump-labels/arch-support.txt | 1 + .../features/core/tracehook/arch-support.txt | 1 + .../features/debug/KASAN/arch-support.txt | 3 +- .../debug/gcov-profile-all/arch-support.txt| 1 + Documentation/features/debug/kgdb/arch-support.txt | 3 +- .../debug/kprobes-on-ftrace/arch-support.txt | 1 + .../features/debug/kprobes/arch-support.txt| 3 +- .../features/debug/kretprobes/arch-support.txt | 3 +- .../features/debug/optprobes/arch-support.txt | 3 +- .../features/debug/stackprotector/arch-support.txt | 1 + .../features/debug/uprobes/arch-support.txt| 5 +- .../debug/user-ret-profiler/arch-support.txt | 1 + .../features/io/dma-api-debug/arch-support.txt | 1 + .../features/io/dma-contiguous/arch-support.txt| 3 +- .../features/io/sg-chain/arch-support.txt | 1 + .../features/lib/strncasecmp/arch-support.txt | 1 + .../locking/cmpxchg-local/arch-support.txt | 3 +- .../features/locking/lockdep/arch-support.txt | 3 +- .../locking/queued-rwlocks/arch-support.txt| 9 ++-- .../locking/queued-spinlocks/arch-support.txt | 7 +-- .../locking/rwsem-optimized/arch-support.txt | 1 + .../features/perf/kprobes-event/arch-support.txt | 5 +- .../features/perf/perf-regs/arch-support.txt | 3 +- .../features/perf/perf-stackdump/arch-support.txt | 3 +- .../sched/membarrier-sync-core/arch-support.txt| 1 + .../features/sched/numa-balancing/arch-support.txt | 5 +- Documentation/features/scripts/features-refresh.sh | 55 ++ .../seccomp/seccomp-filter/arch-support.txt| 5 +- .../time/arch-tick-broadcast/arch-support.txt | 3 +- .../features/time/clockevents/arch-support.txt | 3 +- .../time/context-tracking/arch-support.txt | 1 + .../features/time/irq-time-acct/arch-support.txt | 3 +- .../time/modern-timekeeping/arch-support.txt | 1 + .../features/time/virt-cpuacct/arch-support.txt| 1 + .../features/vm/ELF-ASLR/arch-support.txt | 3 +- .../features/vm/PG_uncached/arch-support.txt | 1 +
[PATCH 2/3] Documentation/features/core: Add arch support status files for 'cBPF-JIT' and 'eBPF-JIT'
Signed-off-by: Andrea Parri --- .../features/core/BPF-JIT/arch-support.txt | 31 -- .../features/core/cBPF-JIT/arch-support.txt| 5 .../features/core/eBPF-JIT/arch-support.txt| 5 3 files changed, 10 insertions(+), 31 deletions(-) delete mode 100644 Documentation/features/core/BPF-JIT/arch-support.txt create mode 100644 Documentation/features/core/cBPF-JIT/arch-support.txt create mode 100644 Documentation/features/core/eBPF-JIT/arch-support.txt diff --git a/Documentation/features/core/BPF-JIT/arch-support.txt b/Documentation/features/core/BPF-JIT/arch-support.txt deleted file mode 100644 index 0b96b4e1e7d4a..0 --- a/Documentation/features/core/BPF-JIT/arch-support.txt +++ /dev/null @@ -1,31 +0,0 @@ -# -# Feature name: BPF-JIT -# Kconfig: HAVE_BPF_JIT -# description: arch supports BPF JIT optimizations -# ---- -| arch |status| ---- -| alpha: | TODO | -| arc: | TODO | -| arm: | ok | -| arm64: | ok | -| c6x: | TODO | -| h8300: | TODO | -| hexagon: | TODO | -|ia64: | TODO | -|m68k: | TODO | -| microblaze: | TODO | -|mips: | ok | -| nios2: | TODO | -|openrisc: | TODO | -| parisc: | TODO | -| powerpc: | ok | -|s390: | ok | -| sh: | TODO | -| sparc: | ok | -| um: | TODO | -| unicore32: | TODO | -| x86: | ok | -| xtensa: | TODO | ---- diff --git a/Documentation/features/core/cBPF-JIT/arch-support.txt b/Documentation/features/core/cBPF-JIT/arch-support.txt new file mode 100644 index 0..2ae2a7106e67d --- /dev/null +++ b/Documentation/features/core/cBPF-JIT/arch-support.txt @@ -0,0 +1,5 @@ +# +# Feature name: cBPF-JIT +# Kconfig: HAVE_CBPF_JIT +# description: arch supports cBPF JIT optimizations +# diff --git a/Documentation/features/core/eBPF-JIT/arch-support.txt b/Documentation/features/core/eBPF-JIT/arch-support.txt new file mode 100644 index 0..c8b0b458b9cec --- /dev/null +++ b/Documentation/features/core/eBPF-JIT/arch-support.txt @@ -0,0 +1,5 @@ +# +# Feature name: eBPF-JIT +# Kconfig: HAVE_EBPF_JIT +# description: arch supports eBPF JIT optimizations +# -- 2.7.4
[PATCH 3/3] Documentation/features: Refresh and auto-generate the arch support status files in place
Signed-off-by: Andrea Parri --- .../features/core/cBPF-JIT/arch-support.txt| 27 ++ .../features/core/eBPF-JIT/arch-support.txt| 27 ++ .../core/generic-idle-thread/arch-support.txt | 3 ++- .../features/core/jump-labels/arch-support.txt | 1 + .../features/core/tracehook/arch-support.txt | 1 + .../features/debug/KASAN/arch-support.txt | 3 ++- .../debug/gcov-profile-all/arch-support.txt| 1 + Documentation/features/debug/kgdb/arch-support.txt | 3 ++- .../debug/kprobes-on-ftrace/arch-support.txt | 1 + .../features/debug/kprobes/arch-support.txt| 3 ++- .../features/debug/kretprobes/arch-support.txt | 3 ++- .../features/debug/optprobes/arch-support.txt | 3 ++- .../features/debug/stackprotector/arch-support.txt | 1 + .../features/debug/uprobes/arch-support.txt| 5 ++-- .../debug/user-ret-profiler/arch-support.txt | 1 + .../features/io/dma-api-debug/arch-support.txt | 1 + .../features/io/dma-contiguous/arch-support.txt| 3 ++- .../features/io/sg-chain/arch-support.txt | 1 + .../features/lib/strncasecmp/arch-support.txt | 1 + .../locking/cmpxchg-local/arch-support.txt | 3 ++- .../features/locking/lockdep/arch-support.txt | 3 ++- .../locking/queued-rwlocks/arch-support.txt| 9 .../locking/queued-spinlocks/arch-support.txt | 7 +++--- .../locking/rwsem-optimized/arch-support.txt | 1 + .../features/perf/kprobes-event/arch-support.txt | 5 ++-- .../features/perf/perf-regs/arch-support.txt | 3 ++- .../features/perf/perf-stackdump/arch-support.txt | 3 ++- .../sched/membarrier-sync-core/arch-support.txt| 1 + .../features/sched/numa-balancing/arch-support.txt | 5 ++-- .../seccomp/seccomp-filter/arch-support.txt| 5 ++-- .../time/arch-tick-broadcast/arch-support.txt | 3 ++- .../features/time/clockevents/arch-support.txt | 3 ++- .../time/context-tracking/arch-support.txt | 1 + .../features/time/irq-time-acct/arch-support.txt | 3 ++- .../time/modern-timekeeping/arch-support.txt | 1 + .../features/time/virt-cpuacct/arch-support.txt| 1 + .../features/vm/ELF-ASLR/arch-support.txt | 3 ++- .../features/vm/PG_uncached/arch-support.txt | 1 + Documentation/features/vm/THP/arch-support.txt | 1 + Documentation/features/vm/TLB/arch-support.txt | 1 + .../features/vm/huge-vmap/arch-support.txt | 1 + .../features/vm/ioremap_prot/arch-support.txt | 1 + .../features/vm/numa-memblock/arch-support.txt | 3 ++- .../features/vm/pte_special/arch-support.txt | 1 + 44 files changed, 127 insertions(+), 31 deletions(-) diff --git a/Documentation/features/core/cBPF-JIT/arch-support.txt b/Documentation/features/core/cBPF-JIT/arch-support.txt index 2ae2a7106e67d..6b829e27c268a 100644 --- a/Documentation/features/core/cBPF-JIT/arch-support.txt +++ b/Documentation/features/core/cBPF-JIT/arch-support.txt @@ -3,3 +3,30 @@ # Kconfig: HAVE_CBPF_JIT # description: arch supports cBPF JIT optimizations # +--- +| arch |status| +--- +| alpha: | TODO | +| arc: | TODO | +| arm: | TODO | +| arm64: | TODO | +| c6x: | TODO | +| h8300: | TODO | +| hexagon: | TODO | +|ia64: | TODO | +|m68k: | TODO | +| microblaze: | TODO | +|mips: | ok | +| nios2: | TODO | +|openrisc: | TODO | +| parisc: | TODO | +| powerpc: | ok | +| riscv: | TODO | +|s390: | TODO | +| sh: | TODO | +| sparc: | ok | +| um: | TODO | +| unicore32: | TODO | +| x86: | TODO | +| xtensa: | TODO | +--- diff --git a/Documentation/features/core/eBPF-JIT/arch-support.txt b/Documentation/features/core/eBPF-JIT/arch-support.txt index c8b0b458b9cec..4a4ab34ee293a 100644 --- a/Documentation/features/core/eBPF-JIT/arch-support.txt +++ b/Documentation/features/core/eBPF-JIT/arch-support.txt @@ -3,3 +3,30 @@ # Kconfig: HAVE_EBPF_JIT # description: arch supports eBPF JIT optimizations # +--- +| arch |status| +--- +| alpha: | TODO | +| arc: | TODO | +| arm: | ok | +| arm64: | ok | +| c6x: | TODO | +| h8300: | TODO | +| hexagon: | TODO | +|ia64: | TODO | +|m68k: | TODO | +| microblaze: | TODO | +|mips: | ok | +| nios2: | TODO | +|openrisc: | TODO | +| parisc: | TODO | +| powerpc: | ok | +| riscv: | TODO | +|s390: | ok | +| sh: | TODO | +|
Re: Looking for way to program external MMU from userspace (or viable alternative)
On Tue, Apr 03, 2018 at 01:27:36AM +, Simon Que wrote: > Hi kernel community, > > We have an external PCIe board with a custom coprocessor on it. We also > have code for a kernel driver for it. We have thought about upstreaming it, > but we realized that we can instead convert the driver to a userspace > driver using UIO. > > However, there's one aspect of the system and driver that doesn't seem to > be covered by UIO. The external board has a MMU and a DMA interface that > allows it to copy data between the host system's RAM and its own internal > memory. > > The current kernel driver code looks up the physical address of a page of > user-allocated memory by traversing the page table, and then writing the > physical address to the external MMU. If we were to move the driver to > userspace, this procedure would require exposing the physical address to > user space, which insecure and thus a no-go. > > What possibilities are there for programming the MMU from a userspace > driver? > > For reference, here is the existing kernel driver code -- start from > apex_driver.c. > https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/981313 AFAIK not do-able and not something you want to do. UIO is for very very basic device, no MMU or DMA. Allowing any such thing from user space is wide opening the door to random DMA exploitation especialy if there is no IOMMU. Cheers, Jérôme
Re: [PATCH v1] kernel/trace:check the val against the available mem
On Tue, 3 Apr 2018 18:11:19 +0200 Michal Hocko wrote: > yes a fallback is questionable. Whether to make the batch size > configuration is a matter of how much internal details you want to > expose to userspace. Well, it is tracing the guts of the kernel, so internal details are generally exposed to userspace ;-) -- Steve
[PATCH v2] ARM: dts: tpc: Device tree description of the iMX6Q TPC board
This commit adds device tree description of Kieback & Peter GmbH
iMX6Q TPC board.
Signed-off-by: Lukasz Majewski
---
Changes for v2:
- SDPX license identifiers used
- Separate regulators
- Proper beeper driver
- Use of the lcd panel (with compatible) instead of timings provided in
device tree
- Add IRQ types (like IRQ_TYPE_EDGE_FALLING) and GPIO active levels (like
GPIO_ACTIVE_HIGH)
- Remove not needed nodes
- Make W=1 dtbs compilation with no errors
---
.../devicetree/bindings/vendor-prefixes.txt| 1 +
arch/arm/boot/dts/Makefile | 1 +
arch/arm/boot/dts/imx6q-kp-tpc.dts | 23 ++
arch/arm/boot/dts/imx6q-kp.dtsi| 460 +
4 files changed, 485 insertions(+)
create mode 100644 arch/arm/boot/dts/imx6q-kp-tpc.dts
create mode 100644 arch/arm/boot/dts/imx6q-kp.dtsi
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt
b/Documentation/devicetree/bindings/vendor-prefixes.txt
index ae850d6c0ad3..8ff7eadc8bef 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -181,6 +181,7 @@ karoKa-Ro electronics GmbH
keithkoep Keith & Koep GmbH
keymileKeymile GmbH
khadas Khadas
+kiebackpeterKieback & Peter GmbH
kinetic Kinetic Technologies
kingnovel Kingnovel Technology Co., Ltd.
kosagi Sutajio Ko-Usagi PTE Ltd.
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index ade7a38543dc..c148c4cf28f2 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -459,6 +459,7 @@ dtb-$(CONFIG_SOC_IMX6Q) += \
imx6q-icore-ofcap10.dtb \
imx6q-icore-ofcap12.dtb \
imx6q-icore-rqs.dtb \
+ imx6q-kp-tpc.dtb \
imx6q-marsboard.dtb \
imx6q-mccmon6.dtb \
imx6q-nitrogen6x.dtb \
diff --git a/arch/arm/boot/dts/imx6q-kp-tpc.dts
b/arch/arm/boot/dts/imx6q-kp-tpc.dts
new file mode 100644
index ..b5646040b516
--- /dev/null
+++ b/arch/arm/boot/dts/imx6q-kp-tpc.dts
@@ -0,0 +1,23 @@
+/*
+ * Copyright 2018
+ * Lukasz Majewski, DENX Software Engineering, lu...@denx.de
+ *
+ * SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+ */
+
+/dts-v1/;
+
+#include "imx6q-kp.dtsi"
+
+/ {
+ model = "Freescale i.MX6 Qwuad K+P TPC Board";
+ compatible = "kiebackpeter,imx6q-tpc", "fsl,imx6q";
+
+ memory: memory@1000 {
+ reg = <0x1000 0x4000>;
+ };
+};
+
+_di0_disp0 {
+ remote-endpoint = <_display_in>;
+};
diff --git a/arch/arm/boot/dts/imx6q-kp.dtsi b/arch/arm/boot/dts/imx6q-kp.dtsi
new file mode 100644
index ..acf6d40ca227
--- /dev/null
+++ b/arch/arm/boot/dts/imx6q-kp.dtsi
@@ -0,0 +1,460 @@
+/*
+ * Copyright 2018
+ * Lukasz Majewski, DENX Software Engineering, lu...@denx.de
+ *
+ * SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+ */
+
+/dts-v1/;
+
+#include "imx6q.dtsi"
+
+#include
+#include
+#include
+
+/ {
+ backlight_lcd: backlight-lcd {
+ compatible = "pwm-backlight";
+ pwms = < 0 500>;
+ brightness-levels = < 0 1 2 3 4 5 6 7 8 9
+ 10 11 12 13 14 15 16 17 18 19
+ 20 21 22 23 24 25 26 27 28 29
+ 30 31 32 33 34 35 36 37 38 39
+ 40 41 42 43 44 45 46 47 48 49
+ 50 51 52 53 54 55 56 57 58 59
+ 60 61 62 63 64 65 66 67 68 69
+ 70 71 72 73 74 75 76 77 78 79
+ 80 81 82 83 84 85 86 87 88 89
+ 90 91 92 93 94 95 96 97 98 99
+100 101 102 103 104 105 106 107 108 109
+110 111 112 113 114 115 116 117 118 119
+120 121 122 123 124 125 126 127 128 129
+130 131 132 133 134 135 136 137 138 139
+140 141 142 143 144 145 146 147 148 149
+150 151 152 153 154 155 156 157 158 159
+160 161 162 163 164 165 166 167 168 169
+170 171 172 173 174 175 176 177 178 179
+180 181 182 183 184 185 186 187 188 189
+190 191 192 193 194 195 196 197 198 199
+200 201 202 203 204 205 206 207 208 209
+210 211 212 213 214 215 216 217 218 219
+220 221 222 223 224 225 226 227 228 229
+230 231 232 233 234 235 236 237 238 239
+240 241 242 243
Re: [PATCH v6 3/9] pinctrl: actions: Add Actions S900 pinctrl driver
Hi Andy,
On Sat, Mar 31, 2018 at 12:16:49AM +0300, Andy Shevchenko wrote:
> On Wed, Mar 28, 2018 at 8:46 PM, Manivannan Sadhasivam
> wrote:
> > Add pinctrl driver for Actions Semi S900 SoC. The driver supports
> > pinctrl, pinmux and pinconf functionalities through a range of registers
> > common to both gpio driver and pinctrl driver.
> >
> > Pinmux functionality is available only for the pin groups while the
> > pinconf functionality is available for both pin groups and individual
> > pins.
>
> > +static void owl_update_bits(void __iomem *base, u32 mask, u32 val)
> > +{
> > + u32 reg_val;
> > +
> > + reg_val = readl_relaxed(base);
> > +
> > + reg_val &= ~mask;
> > + reg_val |= val;
>
> Usual pattern here is
>
> reg_val = (reg_val & ~mask) | (val & mask);
>
> This will allow to avoid possible overflow.
>
Ack.
> > +
> > + writel_relaxed(reg_val, base);
> > +}
>
> > + tmp = readl_relaxed(pctrl->base + reg);
> > + mask = (1 << width) - 1;
> > + arg = (tmp >> bit) & mask;
>
> This looks like a candidate for a helper function. You have at least
> one more same code.
>
> Something like
>
> ..._read_field(reg, mask, shift)
>
>
Okay. Will add owl_read_field helper function.
> > + mask = (1 << width) - 1;
> > + mask = mask << bit;
> > +
> > + owl_update_bits(pctrl->base + reg, mask, (arg << bit));
>
> Similar here,
>
> ..._write_field(regm mask, shift, arg)
>
Will add owl_write_field helper function.
> > + tmp = readl_relaxed(pctrl->base + reg);
> > + mask = (1 << width) - 1;
> > + arg = (tmp >> bit) & mask;
>
> > + mask = (1 << width) - 1;
> > + mask = mask << bit;
> > +
> > + owl_update_bits(pctrl->base + reg, mask, (arg << bit));
>
> > +static const struct pinconf_ops owl_pinconf_ops = {
> > + .is_generic = true,
> > + .pin_config_get = owl_pin_config_get,
> > + .pin_config_set = owl_pin_config_set,
> > + .pin_config_group_get = owl_group_config_get,
> > + .pin_config_group_set = owl_group_config_set
>
> It's still good idea to leave comma here...
>
I'm confused. What is the criteria for removing/keeping comma for last member
of struct? I followed your gpio driver suggestion.
Thanks,
Mani
> > +};
> > +
> > +static struct pinctrl_desc owl_pinctrl_desc = {
> > + .pctlops = _pinctrl_ops,
> > + .pmxops = _pinmux_ops,
> > + .confops = _pinconf_ops,
> > + .owner = THIS_MODULE
>
> ...and here, and in all similar places.
>
> > +};
> > +
>
> --
> With Best Regards,
> Andy Shevchenko
KASAN: global-out-of-bounds Write in string
Hello, syzbot hit the following crash on upstream commit 642e7fd23353e22290e3d51719fcb658dc252342 (Tue Apr 3 04:22:12 2018 +) Merge branch 'syscalls-next' of git://git.kernel.org/pub/scm/linux/kernel/git/brodo/linux syzbot dashboard link: https://syzkaller.appspot.com/bug?extid=b890b3335a4d8c608963 syzkaller reproducer: https://syzkaller.appspot.com/x/repro.syz?id=4698588919627776 Raw console output: https://syzkaller.appspot.com/x/log.txt?id=5456053378482176 Kernel config: https://syzkaller.appspot.com/x/.config?id=-6874493495260513980 compiler: gcc (GCC) 7.1.1 20170620 IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+b890b3335a4d8c608...@syzkaller.appspotmail.com It will help syzbot understand when the bug is fixed. See footer for details. If you forward the report, please keep this part and the footer. IPVS: ftp: loaded support on port[0] = 21 REISERFS warning (device loop0): sh-2021 reiserfs_fill_super: can not find reiserfs on loop0 syz-executor0 (4503) used greatest stack depth: 15560 bytes left REISERFS warning (device loop0): sh-2021 reiserfs_fill_super: can not find reiserfs on loop0 == BUG: KASAN: global-out-of-bounds in string+0x1cb/0x200 lib/vsprintf.c:598 Write of size 1 at addr 89e166a0 by task syz-executor0/4522 CPU: 1 PID: 4522 Comm: syz-executor0 Not tainted 4.16.0+ #12 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:17 [inline] dump_stack+0x1a7/0x27d lib/dump_stack.c:53 print_address_description+0x178/0x250 mm/kasan/report.c:256 kasan_report_error mm/kasan/report.c:354 [inline] kasan_report+0x23c/0x360 mm/kasan/report.c:412 __asan_report_store1_noabort+0x17/0x20 mm/kasan/report.c:435 string+0x1cb/0x200 lib/vsprintf.c:598 vsnprintf+0x863/0x1900 lib/vsprintf.c:2282 vsprintf+0x2a/0x40 lib/vsprintf.c:2462 prepare_error_buf+0x1d2/0x1820 fs/reiserfs/prints.c:240 __reiserfs_warning+0xc8/0x1a0 fs/reiserfs/prints.c:267 reiserfs_getopt fs/reiserfs/super.c:1044 [inline] reiserfs_parse_options+0x11e5/0x24e0 fs/reiserfs/super.c:1194 reiserfs_fill_super+0x520/0x33a0 fs/reiserfs/super.c:1946 mount_bdev+0x2b7/0x370 fs/super.c:1119 get_super_block+0x34/0x40 fs/reiserfs/super.c:2605 mount_fs+0x66/0x2d0 fs/super.c:1222 vfs_kern_mount.part.26+0xc6/0x4a0 fs/namespace.c:1037 vfs_kern_mount fs/namespace.c:2514 [inline] do_new_mount fs/namespace.c:2517 [inline] do_mount+0xea4/0x2b90 fs/namespace.c:2847 ksys_mount+0xab/0x120 fs/namespace.c:3063 SYSC_mount fs/namespace.c:3077 [inline] SyS_mount+0x39/0x50 fs/namespace.c:3074 do_syscall_64+0x281/0x940 arch/x86/entry/common.c:287 entry_SYSCALL_64_after_hwframe+0x42/0xb7 RIP: 0033:0x457d0a RSP: 002b:7f0655cddbb8 EFLAGS: 0246 ORIG_RAX: 00a5 RAX: ffda RBX: 2000 RCX: 00457d0a RDX: 2000 RSI: 2100 RDI: 7f0655cddc00 RBP: 0004 R08: 20013f00 R09: 2000 R10: R11: 0246 R12: 0005 R13: 066d R14: 006fcad8 R15: 0001 The buggy address belongs to the variable: error_buf+0x400/0x420 Memory state around the buggy address: 89e16580: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 89e16600: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 89e16680: 00 00 00 00 fa fa fa fa 04 fa fa fa fa fa fa fa ^ 89e16700: 00 fa fa fa fa fa fa fa 00 fa fa fa fa fa fa fa 89e16780: 00 fa fa fa fa fa fa fa 00 fa fa fa fa fa fa fa == --- This bug is generated by a dumb bot. It may contain errors. See https://goo.gl/tpsmEJ for details. Direct all questions to syzkal...@googlegroups.com. syzbot will keep track of this bug report. If you forgot to add the Reported-by tag, once the fix for this bug is merged into any tree, please reply to this email with: #syz fix: exact-commit-title If you want to test a patch for this bug, please reply with: #syz test: git://repo/address.git branch and provide the patch inline or as an attachment. To mark this as a duplicate of another syzbot report, please reply with: #syz dup: exact-subject-of-another-report If it's a one-off invalid bug report, please reply with: #syz invalid Note: if the crash happens again, it will cause creation of a new bug report. Note: all commands must start from beginning of the line in the email body.
Re: [PATCH v2] x86/xen/efi: Initialize UEFI secure boot state during dom0 boot
On Tue, 2018-04-03 at 18:07 +0200, Daniel Kiper wrote:
> On Tue, Apr 03, 2018 at 08:44:41AM -0700, James Bottomley wrote:
> >
> > On Tue, 2018-04-03 at 16:39 +0200, Daniel Kiper wrote:
> > >
> > > Initialize UEFI secure boot state during dom0 boot. Otherwise the
> > > kernel may not even know that it runs on secure boot enabled
> > > platform.
> > >
> > > Signed-off-by: Daniel Kiper
> > > ---
> > > arch/x86/xen/efi.c| 57
> > > +
> > > drivers/firmware/efi/libstub/secureboot.c |3 ++
> > > 2 files changed, 60 insertions(+)
> > >
> > > diff --git a/arch/x86/xen/efi.c b/arch/x86/xen/efi.c
> > > index a18703b..1804b27 100644
> > > --- a/arch/x86/xen/efi.c
> > > +++ b/arch/x86/xen/efi.c
> > > @@ -115,6 +115,61 @@ static efi_system_table_t __init
> > > *xen_efi_probe(void)
> > > return _systab_xen;
> > > }
> > >
> > > +/*
> > > + * Determine whether we're in secure boot mode.
> > > + *
> > > + * Please keep the logic in sync with
> > > + *
> > > drivers/firmware/efi/libstub/secureboot.c:efi_get_secureboot().
> > > + */
> > > +static enum efi_secureboot_mode xen_efi_get_secureboot(void)
> > > +{
> > > + static efi_guid_t efi_variable_guid =
> > > EFI_GLOBAL_VARIABLE_GUID;
> > > + static efi_guid_t shim_guid = EFI_SHIM_LOCK_GUID;
> > > + efi_status_t status;
> > > + u8 moksbstate, secboot, setupmode;
> > > + unsigned long size;
> > > +
> > > + size = sizeof(secboot);
> > > + status = efi.get_variable(L"SecureBoot",
> > > _variable_guid,
> > > + NULL, , );
> > > +
> > > + if (status == EFI_NOT_FOUND)
> > > + return efi_secureboot_mode_disabled;
> > > +
> > > + if (status != EFI_SUCCESS)
> > > + goto out_efi_err;
> > > +
> > > + size = sizeof(setupmode);
> > > + status = efi.get_variable(L"SetupMode",
> > > _variable_guid,
> > > + NULL, , );
> > > +
> > > + if (status != EFI_SUCCESS)
> > > + goto out_efi_err;
> > > +
> > > + if (secboot == 0 || setupmode == 1)
> > > + return efi_secureboot_mode_disabled;
> > > +
> > > + /* See if a user has put the shim into insecure mode. */
> > > + size = sizeof(moksbstate);
> > > + status = efi.get_variable(L"MokSBStateRT", _guid,
> > > + NULL, , );
> > > +
> > > + /* If it fails, we don't care why. Default to secure. */
> > > + if (status != EFI_SUCCESS)
> > > + goto secure_boot_enabled;
> > > +
> > > + if (moksbstate == 1)
> > > + return efi_secureboot_mode_disabled;
> > > +
> > > + secure_boot_enabled:
> > > + pr_info("UEFI Secure Boot is enabled.\n");
> > > + return efi_secureboot_mode_enabled;
> > > +
> > > + out_efi_err:
> > > + pr_err("Could not determine UEFI Secure Boot
> > > status.\n");
> > > + return efi_secureboot_mode_unknown;
> > > +}
> > > +
> >
> > This looks like a bad idea: you're duplicating the secure boot
> > check in
> >
> > drivers/firmware/efi/libstub/secureboot.c
> >
> > Which is an implementation of policy. If we have to have policy in
> > the kernel, it should really only be in one place to prevent drift;
> > why can't you simply use the libstub efi_get_secureboot() so we're
> > not duplicating the implementation of policy?
>
> Well, here is the first version of this patch:
> https://lkml.org/lkml/2018/1/9/496 Ard did not like it. I was not
> happy too. In general both approaches are not perfect. More you can
> find in the discussion around this patchset. If you have better idea
> how to do that I am happy to implement it.
One way might be simply to have the pre exit-boot-services code lay
down a variable containing the state which you pick up, rather than you
calling efi code separately and trying to use the insecure RT
variables. That way there's a uniform view of the internal kernel
secure boot state that everyone can use.
James
KASAN: use-after-free Read in xfs_inobt_init_key_from_rec
Hello, syzbot hit the following crash on upstream commit 642e7fd23353e22290e3d51719fcb658dc252342 (Tue Apr 3 04:22:12 2018 +) Merge branch 'syscalls-next' of git://git.kernel.org/pub/scm/linux/kernel/git/brodo/linux syzbot dashboard link: https://syzkaller.appspot.com/bug?extid=08ab33be0178b76851c8 C reproducer: https://syzkaller.appspot.com/x/repro.c?id=5951352765153280 syzkaller reproducer: https://syzkaller.appspot.com/x/repro.syz?id=5417464171069440 Raw console output: https://syzkaller.appspot.com/x/log.txt?id=6712144213049344 Kernel config: https://syzkaller.appspot.com/x/.config?id=-6874493495260513980 compiler: gcc (GCC) 7.1.1 20170620 IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+08ab33be0178b7685...@syzkaller.appspotmail.com It will help syzbot understand when the bug is fixed. See footer for details. If you forward the report, please keep this part and the footer. XFS (loop0): Mounting V4 Filesystem XFS (loop0): totally zeroed log == BUG: KASAN: use-after-free in xfs_inobt_init_key_from_rec+0x6a/0x70 fs/xfs/libxfs/xfs_ialloc_btree.c:195 Read of size 4 at addr 8801af98cf00 by task syzkaller185183/4464 CPU: 0 PID: 4464 Comm: syzkaller185183 Not tainted 4.16.0+ #12 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:17 [inline] dump_stack+0x1a7/0x27d lib/dump_stack.c:53 print_address_description+0x73/0x250 mm/kasan/report.c:256 kasan_report_error mm/kasan/report.c:354 [inline] kasan_report+0x23c/0x360 mm/kasan/report.c:412 __asan_report_load4_noabort+0x14/0x20 mm/kasan/report.c:432 xfs_inobt_init_key_from_rec+0x6a/0x70 fs/xfs/libxfs/xfs_ialloc_btree.c:195 xfs_lookup_get_search_key+0x7e/0xc0 fs/xfs/libxfs/xfs_btree.c:1858 xfs_btree_lookup+0x932/0x1180 fs/xfs/libxfs/xfs_btree.c:1948 xfs_inobt_lookup fs/xfs/libxfs/xfs_ialloc.c:75 [inline] xfs_imap_lookup+0x216/0x640 fs/xfs/libxfs/xfs_ialloc.c:2235 xfs_imap+0x78f/0x960 fs/xfs/libxfs/xfs_ialloc.c:2364 xfs_iread+0xc7/0x770 fs/xfs/libxfs/xfs_inode_buf.c:556 xfs_iget_cache_miss fs/xfs/xfs_icache.c:475 [inline] xfs_iget+0xcec/0x3060 fs/xfs/xfs_icache.c:626 xfs_mountfs+0x139a/0x2690 fs/xfs/xfs_mount.c:881 xfs_fs_fill_super+0xc8d/0x1250 fs/xfs/xfs_super.c:1703 mount_bdev+0x2b7/0x370 fs/super.c:1119 xfs_fs_mount+0x34/0x40 fs/xfs/xfs_super.c:1770 mount_fs+0x66/0x2d0 fs/super.c:1222 vfs_kern_mount.part.26+0xc6/0x4a0 fs/namespace.c:1037 vfs_kern_mount fs/namespace.c:2514 [inline] do_new_mount fs/namespace.c:2517 [inline] do_mount+0xea4/0x2b90 fs/namespace.c:2847 ksys_mount+0xab/0x120 fs/namespace.c:3063 SYSC_mount fs/namespace.c:3077 [inline] SyS_mount+0x39/0x50 fs/namespace.c:3074 do_syscall_64+0x281/0x940 arch/x86/entry/common.c:287 entry_SYSCALL_64_after_hwframe+0x42/0xb7 RIP: 0033:0x44371a RSP: 002b:7fff43e464c8 EFLAGS: 0202 ORIG_RAX: 00a5 RAX: ffda RBX: 2840 RCX: 0044371a RDX: 2000 RSI: 2100 RDI: 7fff43e464d0 RBP: 0003 R08: 20018900 R09: 000a R10: R11: 0202 R12: 0004 R13: 00402610 R14: R15: Allocated by task 4462: save_stack+0x43/0xd0 mm/kasan/kasan.c:447 set_track mm/kasan/kasan.c:459 [inline] kasan_kmalloc+0xad/0xe0 mm/kasan/kasan.c:552 kasan_slab_alloc+0x12/0x20 mm/kasan/kasan.c:489 kmem_cache_alloc+0x12e/0x760 mm/slab.c:3542 getname_flags+0xcb/0x580 fs/namei.c:138 getname+0x19/0x20 fs/namei.c:209 do_sys_open+0x2e7/0x6d0 fs/open.c:1095 SYSC_open fs/open.c:1119 [inline] SyS_open+0x2d/0x40 fs/open.c:1114 do_syscall_64+0x281/0x940 arch/x86/entry/common.c:287 entry_SYSCALL_64_after_hwframe+0x42/0xb7 Freed by task 4462: save_stack+0x43/0xd0 mm/kasan/kasan.c:447 set_track mm/kasan/kasan.c:459 [inline] __kasan_slab_free+0x11a/0x170 mm/kasan/kasan.c:520 kasan_slab_free+0xe/0x10 mm/kasan/kasan.c:527 __cache_free mm/slab.c:3486 [inline] kmem_cache_free+0x83/0x2a0 mm/slab.c:3744 putname+0xee/0x130 fs/namei.c:258 do_sys_open+0x31b/0x6d0 fs/open.c:1110 SYSC_open fs/open.c:1119 [inline] SyS_open+0x2d/0x40 fs/open.c:1114 do_syscall_64+0x281/0x940 arch/x86/entry/common.c:287 entry_SYSCALL_64_after_hwframe+0x42/0xb7 The buggy address belongs to the object at 8801af98c600 which belongs to the cache names_cache of size 4096 The buggy address is located 2304 bytes inside of 4096-byte region [8801af98c600, 8801af98d600) The buggy address belongs to the page: page:ea0006be6300 count:1 mapcount:0 mapping:8801af98c600 index:0x0 compound_mapcount: 0 flags: 0x2fffc008100(slab|head) raw: 02fffc008100 8801af98c600 00010001 raw: ea0006be7aa0 ea0006be5f20 8801da5ea600 page dumped because: kasan: bad access detected Memory
Re: [PATCH net] net: dsa: mt7530: Use NULL instead of plain integer
On 04/02/2018 07:18 PM, Sean Wang wrote: > On Mon, 2018-04-02 at 16:24 -0700, Florian Fainelli wrote: >> We would be passing 0 instead of NULL as the rsp argument to >> mt7530_fdb_cmd(), fix that. >> > > Acked-by: Sean Wang > > BTW, does the part of the commit message should be updated with "passing > NULL instead of 0"? I don't follow you, the commit message indicates what we were doing which implies it was wrong and fixes it. Would you want me to reword that part? -- Florian
Re: 4.15.14 crash with iscsi target and dvd
On Sun, 2018-04-01 at 14:27 -0400, Wakko Warner wrote:
> Wakko Warner wrote:
> > Wakko Warner wrote:
> > > I tested 4.14.32 last night with the same oops. 4.9.91 works fine.
> > > From the initiator, if I do cat /dev/sr1 > /dev/null it works. If I mount
> > > /dev/sr1 and then do find -type f | xargs cat > /dev/null the target
> > > crashes. I'm using the builtin iscsi target with pscsi. I can burn from
> > > the initiator with out problems. I'll test other kernels between 4.9 and
> > > 4.14.
> >
> > So I've tested 4.x.y where x one of 10 11 12 14 15 and y is the latest patch
> > (except for 4.15 which was 1 behind)
> > Each of these kernels crash within seconds or immediate of doing find -type
> > f | xargs cat > /dev/null from the initiator.
>
> I tried 4.10.0. It doesn't completely lockup the system, but the device
> that was used hangs. So from the initiator, it's /dev/sr1 and from the
> target it's /dev/sr0. Attempting to read /dev/sr0 after the oops causes the
> process to hang in D state.
Hello Wakko,
Thank you for having narrowed down this further. I think that you encountered
a regression either in the block layer core or in the SCSI core. Unfortunately
the number of changes between kernel versions v4.9 and v4.10 in these two
subsystems is huge. I see two possible ways forward:
- Either that you perform a bisect to identify the patch that introduced this
regression. However, I'm not sure whether you are familiar with the bisect
process.
- Or that you identify the command that triggers this crash such that others
can reproduce this issue without needing access to your setup.
How about reproducing this crash with the below patch applied on top of
kernel v4.15.x? The additional output sent by this patch to the system log
should allow us to reproduce this issue by submitting the same SCSI command
with sg_raw.
Thanks,
Bart.
Subject: [PATCH] Report commands with no physical segments in the system log
---
drivers/scsi/scsi_lib.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 6b6a6705f6e5..74a39db57d49 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1093,8 +1093,10 @@ int scsi_init_io(struct scsi_cmnd *cmd)
bool is_mq = (rq->mq_ctx != NULL);
int error = BLKPREP_KILL;
- if (WARN_ON_ONCE(!blk_rq_nr_phys_segments(rq)))
+ if (WARN_ON_ONCE(!blk_rq_nr_phys_segments(rq))) {
+ scsi_print_command(cmd);
goto err_exit;
+ }
error = scsi_init_sgtable(rq, >sdb);
if (error)
Re: [PATCH v17 01/10] LIB: Introduce a generic PIO mapping method
On 03/04/2018 17:37, Thierry Reding wrote:
On Tue, Apr 03, 2018 at 05:01:37PM +0100, John Garry wrote:
+int logic_pio_register_range(struct logic_pio_hwaddr *new_range)
+{
+ struct logic_pio_hwaddr *range;
+ resource_size_t start = new_range->hw_start;
+ resource_size_t end = new_range->hw_start + new_range->size;
+ resource_size_t mmio_sz = 0;
+ resource_size_t iio_sz = MMIO_UPPER_LIMIT;
+ int ret = 0;
+
+ if (!new_range || !new_range->fwnode || !new_range->size)
+ return -EINVAL;
+
+ mutex_lock(_range_mutex);
+ list_for_each_entry_rcu(range, _range_list, list) {
+ if (range->fwnode == new_range->fwnode) {
+ /* range already there */
+ ret = -EFAULT;
+ goto end_register;
+ }
Hi Thierry,
This is the -EFAULT that propagates to pci-tegra.c's ->probe() and fails
to bind the driver.
I'm not exactly sure what's causing the duplicate here because it's
rather difficult to get at something useful from just the ->fwnode, but
I'm fairly sure that the reason this breaks is because the Tegra driver
will defer probe due to some regulators that aren't available on the
first try. Given the above code and the rest of this file, I can't see a
way to "fix" the driver and remove the I/O range on failure.
This is doubly bad because this doesn't only leak the ranges on probe
deferral, but also on driver unload, and we just added support for
building the Tegra driver as a loadable module, so these are actually
cases that can happen in regular uses of the driver.
I have no idea on how to fix this. Anyone know of a quick fix to restore
PCI for Tegra other than reverting all of these changes?
I suppose an API could be added to unregister the range, but the calling
sequence is rather obfuscated, so removing the range will look totally
asymmetric, I'm afraid.
Here's the call stack:
tegra_pcie_probe()
tegra_pcie_parse_dt()
of_pci_range_to_resource()
pci_register_io_range()
logic_pio_register_range()
So the range here is registered as part of a resource parsing function,
which is supposed to not have any side-effects. There's no equivalent of
that parsing routine (i.e. no "unparse" function that would undo the
effects of parsing).
Perhaps a cleaner way would be to decouple the parsing from the actual
request step that has the side-effect.
This could be added if we agreed that it would be useful.
I guess in most cases these ranges will be static at least during one
boot. But it still feels like this should be removed when the driver
goes away. While this may not depend on data by the driver, and hence
won't cause a crash or anything, it just seems wrong to leave it
around when the driver no longer isn't.
That sounds reasonable, considering we do unmap the iospace when we
release - so it looks like currently we're leaving some IO range
reserved which does not have a mapping.
However this change seems non-trivial, considering we're now even
coupling the PIO range registration into DT parsing.
Going back in history a little, it looks like even before this commit
the I/O range registration was triggered by the parsing code and even
the range leak was there, except that it caused pci_register_io_range()
to return 0 rather than -EFAULT. Perhaps the quickest fix for this would
be to do the same in the new code and restore drivers that accidentally
depend on this behaviour.
I can confirm that the following fixes the issue for me, though I don't
think it's a very clean fix given that the range will remain requested
forever, even if the driver is gone. But since that's already been the
case for quite a while, probably something that can be fixed separately.
Right, there was no way to deregister the range previously. From looking at
the history here I see no reason to not support it.
As for this patch, as you said, the only difference is that we fault on
trying to register the same range again. So this solution seems reasonable.
Okay, I can turn this into a proper patch to fix this up. I suspect that
other drivers may be subject to the same regression. For the longer term
I think it'd be better to properly undo the registration on failure and
removal, but I suspect that it'd be quite a bit of work and not suitable
for v4.17 anymore.
Thanks, I had started to put the patch together but if you're happy to
continue then that's fine. Please let me know.
On another point, for the tegra driver, is it possible to defer earlier in
the probe, before these currently irreversible steps are taken?
I'm sure it'd be possible. But it would be quite involved, I think. The
reason the code is the way it is is because parsing the DT didn't use to
have side-effects.
Also, I don't think it would buy us much because the probe can still
defer (or at least fail) as late as pci_scan_root_bus_bridge(). Even if
we work
Re: [PATCH v6 3/9] pinctrl: actions: Add Actions S900 pinctrl driver
Hi Mani,
Am 03.04.2018 um 19:00 schrieb Manivannan Sadhasivam:
> On Sat, Mar 31, 2018 at 12:16:49AM +0300, Andy Shevchenko wrote:
>> On Wed, Mar 28, 2018 at 8:46 PM, Manivannan Sadhasivam
>> wrote:
>>> +static const struct pinconf_ops owl_pinconf_ops = {
>>> + .is_generic = true,
>>> + .pin_config_get = owl_pin_config_get,
>>> + .pin_config_set = owl_pin_config_set,
>>> + .pin_config_group_get = owl_group_config_get,
>>> + .pin_config_group_set = owl_group_config_set
>>
>> It's still good idea to leave comma here...
>>
>
> I'm confused. What is the criteria for removing/keeping comma for last member
> of struct? I followed your gpio driver suggestion.
No comma for list terminator ("{}") because nothing goes after it; comma
whenever it allows adding a new line without touching the old one, i.e.
keeping future diff small.
Cheers,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Re: [RFC PATCH v1] fw_lockdown: new micro LSM module to prevent loading unsigned firmware
On Tue, Apr 3, 2018 at 9:56 AM, Luis R. Rodriguez wrote: > The biggest thing which has changed since then is that we decided to *not* > support or streamline generic firmware signing (non-IMA) for now for a few > reasons [0] [1] which are important to re-iterate as these are easy to forget, > and AFAICT not documented anywhere. And the URLs... [0] https://lkml.kernel.org/r/20171204195155.gu...@wotan.suse.de [1] https://lkml.kernel.org/r/20171207153209.5da771a9@alans-desktop Luis
Re: [PATCH 4/8] dma-buf: add peer2peer flag
On Tue, Apr 03, 2018 at 11:09:09AM +0200, Daniel Vetter wrote: > On Thu, Mar 29, 2018 at 01:34:24PM +0200, Christian König wrote: > > Am 29.03.2018 um 08:57 schrieb Daniel Vetter: > > > On Sun, Mar 25, 2018 at 12:59:56PM +0200, Christian König wrote: > > > > Add a peer2peer flag noting that the importer can deal with device > > > > resources which are not backed by pages. > > > > > > > > Signed-off-by: Christian König > > > Um strictly speaking they all should, but ttm never bothered to use the > > > real interfaces but just hacked around the provided sg list, grabbing the > > > underlying struct pages, then rebuilding the sg list again. > > > > Actually that isn't correct. TTM converts them to a dma address array > > because drivers need it like this (at least nouveau, radeon and amdgpu). > > > > I've fixed radeon and amdgpu to be able to deal without it and mailed with > > Ben about nouveau, but the outcome is they don't really know. > > > > TTM itself doesn't have any need for the pages on imported BOs (you can't > > mmap them anyway), the real underlying problem is that sg tables doesn't > > provide what drivers need. > > > > I think we could rather easily fix sg tables, but that is a totally separate > > task. > > Looking at patch 8, the sg table seems perfectly sufficient to convey the > right dma addresses to the importer. Ofcourse the exporter has to set up > the right kind of iommu mappings to make this work. > > > > The entire point of using sg lists was exactly to allow this use case of > > > peer2peer dma (or well in general have special exporters which managed > > > memory/IO ranges not backed by struct page). So essentially you're having > > > a "I'm totally not broken flag" here. > > > > No, independent of needed struct page pointers we need to note if the > > exporter can handle peer2peer stuff from the hardware side in general. > > > > So what I've did is just to set peer2peer allowed on the importer because of > > the driver needs and clear it in the exporter if the hardware can't handle > > that. > > The only thing the importer seems to do is call the > pci_peer_traffic_supported, which the exporter could call too. What am I > missing (since the sturct_page stuff sounds like it's fixed already by > you)? > -Daniel AFAIK Logan patchset require to register and initialize struct page for the device memory you want to map (export from exporter point of view). With GPU this isn't something we want, struct page is >~= 2^6 so for 4GB GPU = 2^6*2^32/2^12 = 2^26 = 64MB of RAM 8GB GPU = 2^6*2^33/2^12 = 2^27 = 128MB of RAM 16GB GPU = 2^6*2^34/2^12 = 2^28 = 256MB of RAM 32GB GPU = 2^6*2^34/2^12 = 2^29 = 512MB of RAM All this is mostly wasted as only a small sub-set (that can not be constraint to specific range) will ever be exported at any point in time. For GPU work load this is hardly justifiable, even for HMM i do not plan to register all those pages. Hence why i argue that dma_map_resource() like use by Christian is good enough for us. People that care about SG can fix that but i rather not have to depend on that and waste system memory. Cheers, Jérôme
Re: NO_HZ_FULL and tick running within a reasonable amount of time
On Tue, Apr 03, 2018 at 01:41:31PM +0200, Frederic Weisbecker wrote:
> On Mon, Apr 02, 2018 at 03:04:38PM -0700, Paul E. McKenney wrote:
> > Hello!
> >
> > I am hitting the following on today's mainline under rcutorture, but
> > only on scenarios built with CONFIG_NO_HZ_FULL=y:
> >
> >
> >
> > WARNING: CPU: 0 PID: 7 at
> > /home/paulmck/public_git/linux-rcu/kernel/sched/core.c:3124
> > sched_tick_remote+0x113/0x120
> > Modules linked in:
> > CPU: 0 PID: 7 Comm: kworker/u4:0 Not tainted 4.16.0+ #1
> > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> > Ubuntu-1.8.2-1ubuntu1 04/01/2014
> > Workqueue: events_unbound sched_tick_remote
> > RIP: 0010:sched_tick_remote+0x113/0x120
> > RSP: 0018:94d540103e20 EFLAGS: 00010002
> > RAX: 00012e9bb357 RBX: 8f95dfd21840 RCX: 001f
> > RDX: b2d05e00 RSI: RDI: 8f95dfd21858
> > RBP: 94d540103e48 R08: f6499019 R09: f6499000
> > R10: b163d33b R11: a5c8c212 R12: 8f95dfd25518
> > R13: 8f95de9e4200 R14: 3402 R15: 8f95dfd21858
> > FS: () GS:8f95dfc0() knlGS:
> > CS: 0010 DS: ES: CR0: 80050033
> > CR2: 0a015b40 CR3: 1de14000 CR4: 06f0
> > Call Trace:
> > process_one_work+0x1d9/0x6a0
> > worker_thread+0x42/0x420
> > kthread+0xf3/0x130
> > ? rescuer_thread+0x340/0x340
> > ? kthread_delayed_work_timer_fn+0x80/0x80
> > ret_from_fork+0x3a/0x50
> > Code: ff 48 8b 83 80 0b 00 00 48 85 c0 0f 85 41 ff ff ff e9 45 ff ff ff be
> > ff ff ff ff 4c 89 ff e8 55 44 02 00 85 c0 75 87 0f 0b eb 83 <0f> 0b eb 97
> > 66 0f 1f 84 00 00 00 00 00 41 57 41 56 41 55 41 54
> > ---[ end trace fbdcbe529a8ae799 ]--
> >
> >
> >
> > The WARN_ON_ONCE() triggering is this guy:
> >
> > delta = rq_clock_task(rq) - curr->se.exec_start;
> > WARN_ON_ONCE(delta > (u64)NSEC_PER_SEC * 3);
>
> Weird. Can you try to print up those values and see how much they drift?
>
> if (WARN_ON_ONCE(delta > (u64)NSEC_PER_SEC * 3))
>printk_once("clock_task: %lld exec_start: %lld\n",
> rq_clock_task(rq), curr->se.exec_start);
Here you go!
Thanx, Paul
WARNING: CPU: 0 PID: 7 at
/home/paulmck/public_git/linux-rcu/kernel/sched/core.c:3124
sched_tick_remote+0xdb/0x100
Modules linked in:
CPU: 0 PID: 7 Comm: kworker/u4:0 Not tainted 4.16.0+ #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
Ubuntu-1.8.2-1ubuntu1 04/01/2014
Workqueue: events_unbound sched_tick_remote
RIP: 0010:sched_tick_remote+0xdb/0x100
RSP: 0018:a2c440103e60 EFLAGS: 00010006
RAX: b2d05e00 RBX: 96f0dfd20980 RCX: 00016a8de322
RDX: 0d33a301 RSI: 000177c18623 RDI: fffeb0bf0e53
RBP: 96f0dfd24328 R08: R09: 0001
R10: R11: R12: 96f0de9d2640
R13: R14: 096f0de81700 R15: 96f0de96f540
FS: () GS:96f0dfc0() knlGS:
CS: 0010 DS: ES: CR0: 80050033
CR2: f7ec6ca3 CR3: 1e03e000 CR4: 06f0
Call Trace:
process_one_work+0x139/0x3e0
worker_thread+0x42/0x420
kthread+0xf3/0x130
? create_worker+0x190/0x190
? kthread_destroy_worker+0x40/0x40
ret_from_fork+0x35/0x40
Code: 89 8b 43 04 85 c0 0f 85 75 ff ff ff 48 8b 83 e0 0a 00 00 48 85 c0 0f 85
65 ff ff ff e9 69 ff ff ff 48 89 df e8 87 fe ff ff eb 8d <0f> 0b 80 3d 7d 57 2b
01 00 75 a8 48 c7 c7 e8 e1 fd a4 c6 05 6d
---[ end trace f0c6a1afa55d130d ]---
clock_task: 6304138787 exec_start: 221487873
[PATCH] i8042: Enable MUX on Sony VAIO VGN-CS series to fix touchpad
The touch sensor buttons on Sony VAIO VGN-CS series laptops (e.g.
VGN-CS31S) are a separate PS/2 device. As the MUX is disabled for all
VAIO machines by the nomux blacklist, the data from touch sensor
buttons and touchpad are combined. The protocol used by the buttons is
probably similar to the touchpad protocol (both are Synaptics) so both
devices get enabled. The controller combines the data, creating a mess
which results in random button clicks, touchpad stopping working and
lost sync error messages:
psmouse serio1: TouchPad at isa0060/serio1/input0 lost sync at byte 4
psmouse serio1: TouchPad at isa0060/serio1/input0 lost sync at byte 1
psmouse serio1: TouchPad at isa0060/serio1/input0 lost sync at byte 1
psmouse serio1: TouchPad at isa0060/serio1/input0 lost sync at byte 1
psmouse serio1: TouchPad at isa0060/serio1/input0 lost sync at byte 1
psmouse serio1: issuing reconnect request
Add a new i8042_dmi_forcemux_table whitelist with VGN-CS.
With MUX enabled, touch sensor buttons are detected as separate device
(and left disabled as there's currently no driver), fixing all touchpad
problems.
Signed-off-by: Ondrej Zary
---
drivers/input/serio/i8042-x86ia64io.h | 17 +
1 file changed, 17 insertions(+)
diff --git a/drivers/input/serio/i8042-x86ia64io.h
b/drivers/input/serio/i8042-x86ia64io.h
index 6cbbdc6e9687..56644c74828c 100644
--- a/drivers/input/serio/i8042-x86ia64io.h
+++ b/drivers/input/serio/i8042-x86ia64io.h
@@ -530,6 +530,20 @@ static const struct dmi_system_id __initconst
i8042_dmi_nomux_table[] = {
{ }
};
+static const struct dmi_system_id __initconst i8042_dmi_forcemux_table[] = {
+ {
+ /*
+* Sony Vaio VGN-CS series require MUX or the touch sensor
+* buttons will disturb touchpad operation
+*/
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Sony Corporation"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "VGN-CS"),
+ },
+ },
+ { }
+};
+
/*
* On some Asus laptops, just running self tests cause problems.
*/
@@ -1163,6 +1177,9 @@ static int __init i8042_platform_init(void)
if (dmi_check_system(i8042_dmi_nomux_table))
i8042_nomux = true;
+ if (dmi_check_system(i8042_dmi_forcemux_table))
+ i8042_nomux = false;
+
if (dmi_check_system(i8042_dmi_notimeout_table))
i8042_notimeout = true;
--
Ondrej Zary
