Re: [PATCH] Revert "ata: ahci-platform: add reset control support"
HI, On 09-04-18 03:32, Kunihiko Hayashi wrote: This reverts commit f0f56716fc3e5d547fd7811eb218a30ed0695605. According to Thierry's view, https://www.spinics.net/lists/linux-ide/msg55357.html some hardware-specific drivers already use their own resets, and the common reset might make a path to occur double controls of resets. For now, revert the commit that adds reset control support to ahci-platform, and hold until the solution is confirmed not be affect all hardware-specific drivers. Fixes: f0f56716fc3e ("ata: ahci-platform: add reset control support") Reported-by: Thierry Reding Suggested-by: Hans de Goede Signed-off-by: Kunihiko Hayashi Acked-by: Hans de Goede Regards, Hans --- .../devicetree/bindings/ata/ahci-platform.txt | 1 - drivers/ata/ahci.h | 1 - drivers/ata/libahci_platform.c | 24 +++--- 3 files changed, 3 insertions(+), 23 deletions(-) diff --git a/Documentation/devicetree/bindings/ata/ahci-platform.txt b/Documentation/devicetree/bindings/ata/ahci-platform.txt index f4006d3..c760ecb 100644 --- a/Documentation/devicetree/bindings/ata/ahci-platform.txt +++ b/Documentation/devicetree/bindings/ata/ahci-platform.txt @@ -30,7 +30,6 @@ compatible: Optional properties: - dma-coherent : Present if dma operations are coherent - clocks: a list of phandle + clock specifier pairs -- resets: a list of phandle + reset specifier pairs - target-supply : regulator for SATA target power - phys : reference to the SATA PHY node - phy-names : must be "sata-phy" diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h index 4356ef1..a9d996e 100644 --- a/drivers/ata/ahci.h +++ b/drivers/ata/ahci.h @@ -350,7 +350,6 @@ struct ahci_host_priv { u32 em_msg_type;/* EM message type */ boolgot_runtime_pm; /* Did we do pm_runtime_get? */ struct clk *clks[AHCI_MAX_CLKS]; /* Optional */ - struct reset_control*rsts; /* Optional */ struct regulator**target_pwrs; /* Optional */ /* * If platform uses PHYs. There is a 1:1 relation between the port number and diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c index 46a7624..30cc8f1 100644 --- a/drivers/ata/libahci_platform.c +++ b/drivers/ata/libahci_platform.c @@ -25,7 +25,6 @@ #include #include #include -#include #include "ahci.h" static void ahci_host_stop(struct ata_host *host); @@ -196,8 +195,7 @@ EXPORT_SYMBOL_GPL(ahci_platform_disable_regulators); * following order: * 1) Regulator * 2) Clocks (through ahci_platform_enable_clks) - * 3) Resets - * 4) Phys + * 3) Phys * * If resource enabling fails at any point the previous enabled resources * are disabled in reverse order. @@ -217,19 +215,12 @@ int ahci_platform_enable_resources(struct ahci_host_priv *hpriv) if (rc) goto disable_regulator; - rc = reset_control_deassert(hpriv->rsts); - if (rc) - goto disable_clks; - rc = ahci_platform_enable_phys(hpriv); if (rc) - goto disable_resets; + goto disable_clks; return 0; -disable_resets: - reset_control_assert(hpriv->rsts); - disable_clks: ahci_platform_disable_clks(hpriv); @@ -248,15 +239,12 @@ EXPORT_SYMBOL_GPL(ahci_platform_enable_resources); * following order: * 1) Phys * 2) Clocks (through ahci_platform_disable_clks) - * 3) Resets - * 4) Regulator + * 3) Regulator */ void ahci_platform_disable_resources(struct ahci_host_priv *hpriv) { ahci_platform_disable_phys(hpriv); - reset_control_assert(hpriv->rsts); - ahci_platform_disable_clks(hpriv); ahci_platform_disable_regulators(hpriv); @@ -405,12 +393,6 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev) hpriv->clks[i] = clk; } - hpriv->rsts = devm_reset_control_array_get_optional_shared(dev); - if (IS_ERR(hpriv->rsts)) { - rc = PTR_ERR(hpriv->rsts); - goto err_out; - } - hpriv->nports = child_nodes = of_get_child_count(dev->of_node); /*
[PATCH net] net: dsa: mv88e6xxx: Fix receive time stamp race condition.
The DSA stack passes received PTP frames to this driver via mv88e6xxx_port_rxtstamp() for deferred delivery. The driver then queues the frame and kicks the worker thread. The work callback reads out the latched receive time stamp and then works through the queue, delivering any non-matching frames without a time stamp. If a new frame arrives after the worker thread has read out the time stamp register but enters the queue before the worker finishes processing the queue, that frame will be delivered without a time stamp. This patch fixes the race by moving the queue onto a list on the stack before reading out the latched time stamp value. Fixes: c6fe0ad2c3499 ("net: dsa: mv88e6xxx: add rx/tx timestamping support") Signed-off-by: Richard Cochran --- drivers/net/dsa/mv88e6xxx/hwtstamp.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/net/dsa/mv88e6xxx/hwtstamp.c b/drivers/net/dsa/mv88e6xxx/hwtstamp.c index ac7694c71266..a036c490b7ce 100644 --- a/drivers/net/dsa/mv88e6xxx/hwtstamp.c +++ b/drivers/net/dsa/mv88e6xxx/hwtstamp.c @@ -285,10 +285,18 @@ static void mv88e6xxx_get_rxts(struct mv88e6xxx_chip *chip, struct sk_buff_head *rxq) { u16 buf[4] = { 0 }, status, seq_id; - u64 ns, timelo, timehi; struct skb_shared_hwtstamps *shwt; + struct sk_buff_head received; + u64 ns, timelo, timehi; + unsigned long flags; int err; + /* The latched timestamp belongs to one of the received frames. */ + __skb_queue_head_init(&received); + spin_lock_irqsave(&rxq->lock, flags); + skb_queue_splice_tail_init(rxq, &received); + spin_unlock_irqrestore(&rxq->lock, flags); + mutex_lock(&chip->reg_lock); err = mv88e6xxx_port_ptp_read(chip, ps->port_id, reg, buf, ARRAY_SIZE(buf)); @@ -311,7 +319,7 @@ static void mv88e6xxx_get_rxts(struct mv88e6xxx_chip *chip, /* Since the device can only handle one time stamp at a time, * we purge any extra frames from the queue. */ - for ( ; skb; skb = skb_dequeue(rxq)) { + for ( ; skb; skb = __skb_dequeue(&received)) { if (mv88e6xxx_ts_valid(status) && seq_match(skb, seq_id)) { ns = timehi << 16 | timelo; -- 2.11.0
Re: [PATCH v2 0/5] allow override of bus format in bridges
On 2018-04-04 14:35, Peter Rosin wrote: > On 2018-04-04 11:07, Laurent Pinchart wrote: >> Hi Daniel, >> >> On Wednesday, 4 April 2018 09:34:41 EEST Daniel Vetter wrote: >>> On Wed, Apr 4, 2018 at 12:28 AM, Laurent Pinchart wrote: On Wednesday, 28 March 2018 10:08:26 EEST Daniel Vetter wrote: > On Mon, Mar 26, 2018 at 11:24:42PM +0200, Peter Rosin wrote: >> Hi! >> >> [I got to v2 sooner than expected] >> >> I have an Atmel sama5d31 hooked up to an lvds encoder and then >> on to an lvds panel. Which seems like something that has been >> done one or two times before... >> >> The problem is that the bus_format of the SoC and the panel do >> not agree. The SoC driver (atmel-hlcdc) can handle the >> rgb444, rgb565, rgb666 and rgb888 bus formats. The hardware is >> wired for the rgb565 case. The lvds encoder supports rgb888 on >> its input side with the LSB wires for each color simply pulled >> down internally in the encoder in my case which means that the >> rgb565 bus_format is the format that works best. And the panel >> is expecting lvds (vesa-24), which is what the encoder outputs. >> >> The reason I "blame" the bus_format of the drm_connector is that >> with the below DT snippet, things do not work *exactly* due to >> that. At least, it starts to work if I hack the panel-lvds driver >> to report the rgb565 bus_format instead of vesa-24. >> >> panel: panel { >> compatible = "panel-lvds"; >> >> width-mm = <304>; >> height-mm = <228; >> >> data-mapping = "vesa-24"; >> >> panel-timing { >> // 1024x768 @ 60Hz (typical) >> clock-frequency = <5214 6500 7110>; >> hactive = <1024>; >> vactive = <768>; >> hfront-porch = <48 88 88>; >> hback-porch = <96 168 168>; >> hsync-len = <32 64 64>; >> vfront-porch = <8 13 14>; >> vback-porch = <8 13 14>; >> vsync-len = <8 12 14>; >> }; >> >> port { >> panel_input: endpoint { >> remote-endpoint = <&lvds_encoder_output>; >> }; >> }; >> }; >> >> lvds-encoder { >> compatible = "ti,ds90c185", "lvds-encoder"; >> >> ports { >> #address-cells = <1>; >> #size-cells = <0>; >> >> port@0 { >> reg = <0>; >> >> lvds_encoder_input: endpoint { >> remote-endpoint = <&hlcdc_output>; >> }; >> }; >> >> port@1 { >> reg = <1>; >> >> lvds_encoder_output: endpoint { >> remote-endpoint = <&panel_input>; >> }; >> }; >> }; >> }; >> >> But, instead of perverting the panel-lvds driver with support >> for a totally fake non-lvds bus_format, I intruduce an API that allows >> display controller drivers to query the required bus_format of any >> intermediate bridges, and match up with that instead of the formats >> given by the drm_connector. I trigger this with this addition to the >> >> lvds-encoder DT node: >> interface-pix-fmt = "rgb565"; >> >> Naming is hard though, so I'm not sure if that's good? >> >> I threw in the first patch, since that is the actual lvds encoder >> I have in this case. >> >> Suggestions welcome. > > Took a quick look, feels rather un-atomic. And there's beend discussing > for other bridge related state that we might want to track (like the full > adjusted_mode that might need to be adjusted at each stage in the chain). > So here's my suggestions: > > - Add an optional per-bridge internal state struct using the support in > https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#handling-driver-> > >> private-state > > Yes it says "driver private", but since bridge is just helper stuff > that's all included. "driver private" == "not exposed as uapi" here. > Include all the usual convenience wrappers to get at the state for a > bridge. > > - Then stuff your bus_format into that new drm_bridge_stat
Re: [PATCH 0/3] syscalls: clean up stub naming convention
* Ingo Molnar wrote: > > * Dominik Brodowski wrote: > > > On Sun, Apr 08, 2018 at 10:35:50AM +0200, Ingo Molnar wrote: > > > - _sys_waitid() # ridiculous number of underscores? > > > - __sys_waitid() # too generic sounding? > > > > ... and we'd need to rename internal helpers in net/ > > > > > - __inline_sys_waitid() # too long? > > > > sounds acceptable, though a bit long (especially for the compat case, though > > it doesn't really matter in the case of > > __inline_compat_sys_sched_rr_get_interval) > > So as per the previous mail this is not just an inline function, but an > active > type conversion wrapper that sign-extends 32-bit ints to longs, which is > important > on some 64-bit architectures. > > And that's a really non-obvious property IMO, and the name should probably > reflect > _that_ non-obvious property, not the inlining property which is really just a > small detail. > > I.e. how about: > > __se_sys_waitid() > > ... where 'se' stands for sign-extended, with a comment in the macro that > explains > the prefix? (The historical abbreviation for sign extension is 'sext', which > I > think wouldn't really be suitable these days.) Ok, so I got confused there: I think it's the do_sys_waitid() intermediate that is actually doing the sign-extension - and the inlined helper is what is in the syscall definition body. So it's all still somewhat of a confusing misnomer: the 'do' named function is actually the sign-extension function variant - and the '_il' variant actually 'does' the real work ... I.e., old naming: 810f08d0 t kernel_waitid# common C function (see kernel/exit.c) __il_sys_waitid# inlined helper doing the actual work # (takes parameters as declared) 810f1aa0 T __do_sys_waitid# C function calling inlined helper # (takes parameters of type long; casts # them to the declared type) 810f1aa0 Tsys_waitid# alias to __do_sys_waitid() (taking # parameters as declared), to be included # in syscall table New suggested naming: 810f08d0 t kernel_waitid# common C function (see kernel/exit.c) __do_sys_waitid# inlined helper doing the actual work # (takes original parameters as declared) 810f1aa0 T __se_sys_waitid# sign-extending C function calling inlined # helper (takes parameters of type long; # casts them to the declared type) 810f1aa0 Tsys_waitid# alias to __se_sys_waitid() (but taking # original parameters as declared), to be # included in syscall table Agreed? Thanks, Ingo
Re: [PATCH 1/2] perf: riscv: preliminary RISC-V support
On Thu, Apr 05, 2018 at 09:47:50AM -0700, Palmer Dabbelt wrote: > On Mon, 26 Mar 2018 00:57:54 PDT (-0700), alan...@andestech.com wrote: > >This patch provide a basic PMU, riscv_base_pmu, which supports two > >general hardware event, instructions and cycles. Furthermore, this > >PMU serves as a reference implementation to ease the portings in > >the future. > > > >riscv_base_pmu should be able to run on any RISC-V machine that > >conforms to the Priv-Spec. Note that the latest qemu model hasn't > >fully support a proper behavior of Priv-Spec 1.10 yet, but work > >around should be easy with very small fixes. Please check > >https://github.com/riscv/riscv-qemu/pull/115 for future updates. > > > >Cc: Nick Hu > >Cc: Greentime Hu > >Signed-off-by: Alan Kao > > We should really be able to detect PMU types at runtime (via a device tree > entry) rather than requiring that a single PMU is built in to the kernel. > This will require a handful of modifications to how this patch works, which > I'll try to list below. > >+menu "PMU type" > >+depends on PERF_EVENTS > >+ > >+config RISCV_BASE_PMU > >+bool "Base Performance Monitoring Unit" > >+def_bool y > >+help > >+ A base PMU that serves as a reference implementation and has limited > >+ feature of perf. > >+ > >+endmenu > >+ > > Rather than a menu where a single PMU can be selected, there should be > options to enable or disable support for each PMU type -- this is just like > how all our other drivers work. > I see. Sure. The descriptions and implementation will be refined in v3. > >+struct pmu * __weak __init riscv_init_platform_pmu(void) > >+{ > >+riscv_pmu = &riscv_base_pmu; > >+return riscv_pmu->pmu; > >+} > > Rather than relying on a weak symbol that gets overridden by other PMU > types, this should look through the device tree for a compatible PMU (in the > case of just the base PMU it could be any RISC-V hart) and install a PMU > handler for it. There'd probably be some sort of priority scheme here, like > there are for other driver subsystems, where we'd pick the best PMU driver > that's compatible with the PMUs on every hart. > > >+ > >+int __init init_hw_perf_events(void) > >+{ > >+struct pmu *pmu = riscv_init_platform_pmu(); > >+ > >+perf_irq = NULL; > >+perf_pmu_register(pmu, "cpu", PERF_TYPE_RAW); > >+return 0; > >+} > >+arch_initcall(init_hw_perf_events); > > Since we only have a single PMU type right now this isn't critical to handle > right away, but we will have to refactor this before adding another PMU. I see. My rough plan is to do the device tree parsing here, and if no specific PMU string is found then just register the base PMU proposed in this patch. How about this idea? Thanks.
[PATCH v2] vhost-net: set packet weight of tx polling to 2 * vq size
handle_tx will delay rx for tens or even hundreds of milliseconds when tx busy polling udp packets with small length(e.g. 1byte udp payload), because setting VHOST_NET_WEIGHT takes into account only sent-bytes but no single packet length. Ping-Latencies shown below were tested between two Virtual Machines using netperf (UDP_STREAM, len=1), and then another machine pinged the client: vq size=256 Packet-Weight Ping-Latencies(millisecond) min avg max Origin 3.319 18.48957.303 64 1.6432.021 2.552 128 1.8252.600 3.224 256 1.9972.710 4.295 512 1.8603.171 4.631 1024 2.0024.173 9.056 2048 2.2575.650 9.688 4096 2.0938.50815.943 vq size=512 Packet-Weight Ping-Latencies(millisecond) min avg max Origin 6.537 29.17766.245 64 2.7983.614 4.403 128 2.8613.820 4.775 256 3.0084.018 4.807 512 3.2544.523 5.824 1024 3.0795.335 7.747 2048 3.9448.20112.762 4096 4.158 11.05719.985 Ring size is a hint from device about a burst size it can tolerate. Based on benchmarks, set the weight to 2 * vq size. To evaluate this change, another tests were done using netperf(RR, TX) between two machines with Intel(R) Xeon(R) Gold 6133 CPU @ 2.50GHz, and vq size was tweaked through qemu. Results shown below does not show obvious changes. vq size=256 TCP_RRvq size=512 TCP_RR size/sessions/+thu%/+normalize% size/sessions/+thu%/+normalize% 1/ 1/ -7%/-2% 1/ 1/ 0%/-2% 1/ 4/ +1%/ 0% 1/ 4/ +1%/ 0% 1/ 8/ +1%/-2% 1/ 8/ 0%/+1% 64/ 1/ -6%/ 0% 64/ 1/ +7%/+3% 64/ 4/ 0%/+2% 64/ 4/ -1%/+1% 64/ 8/ 0%/ 0% 64/ 8/ -1%/-2% 256/ 1/ -3%/-4%256/ 1/ -4%/-2% 256/ 4/ +3%/+4%256/ 4/ +1%/+2% 256/ 8/ +2%/ 0%256/ 8/ +1%/-1% vq size=256 UDP_RRvq size=512 UDP_RR size/sessions/+thu%/+normalize% size/sessions/+thu%/+normalize% 1/ 1/ -5%/+1% 1/ 1/ -3%/-2% 1/ 4/ +4%/+1% 1/ 4/ -2%/+2% 1/ 8/ -1%/-1% 1/ 8/ -1%/ 0% 64/ 1/ -2%/-3% 64/ 1/ +1%/+1% 64/ 4/ -5%/-1% 64/ 4/ +2%/ 0% 64/ 8/ 0%/-1% 64/ 8/ -2%/+1% 256/ 1/ +7%/+1%256/ 1/ -7%/ 0% 256/ 4/ +1%/+1%256/ 4/ -3%/-4% 256/ 8/ +2%/+2%256/ 8/ +1%/+1% vq size=256 TCP_STREAMvq size=512 TCP_STREAM size/sessions/+thu%/+normalize% size/sessions/+thu%/+normalize% 64/ 1/ 0%/-3% 64/ 1/ 0%/ 0% 64/ 4/ +3%/-1% 64/ 4/ -2%/+4% 64/ 8/ +9%/-4% 64/ 8/ -1%/+2% 256/ 1/ +1%/-4%256/ 1/ +1%/+1% 256/ 4/ -1%/-1%256/ 4/ -3%/ 0% 256/ 8/ +7%/+5%256/ 8/ -3%/ 0% 512/ 1/ +1%/ 0%512/ 1/ -1%/-1% 512/ 4/ +1%/-1%512/ 4/ 0%/ 0% 512/ 8/ +7%/-5%512/ 8/ +6%/-1% 1024/ 1/ 0%/-1% 1024/ 1/ 0%/+1% 1024/ 4/ +3%/ 0% 1024/ 4/ +1%/ 0% 1024/ 8/ +8%/+5% 1024/ 8/ -1%/ 0% 2048/ 1/ +2%/+2% 2048/ 1/ -1%/ 0% 2048/ 4/ +1%/ 0% 2048/ 4/ 0%/-1% 2048/ 8/ -2%/ 0% 2048/ 8/ 5%/-1% 4096/ 1/ -2%/ 0% 4096/ 1/ -2%/ 0% 4096/ 4/ +2%/ 0% 4096/ 4/ 0%/ 0% 4096/ 8/ +9%/-2% 4096/ 8/ -5%/-1% Signed-off-by: Haibin Zhang Signed-off-by: Yunfang Tai Signed-off-by: Lidong Chen --- drivers/vhost/net.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c index 8139bc70ad7d..3563a305cc0a 100644 --- a/drivers/vhost/net.c +++ b/drivers/vhost/net.c @@ -44,6 +44,10 @@ MODULE_PARM_DESC(experimental_zcopytx, "Enable Zero Copy TX;" * Using this limit prevents one virtqueue from starving others. */ #define VHOST_NET_WEIGHT 0x8 +/* Max number of packets transferred before requeueing the job. + * Using this limit prevents one virtqueue from starving rx. */ +#define VHOST_N
Re: [PATCH 0/3] syscalls: clean up stub naming convention
On Mon, Apr 09, 2018 at 09:06:11AM +0200, Ingo Molnar wrote: > > * Ingo Molnar wrote: > > > > > * Dominik Brodowski wrote: > > > > > On Sun, Apr 08, 2018 at 10:35:50AM +0200, Ingo Molnar wrote: > > > > - _sys_waitid() # ridiculous number of underscores? > > > > - __sys_waitid() # too generic sounding? > > > > > > ... and we'd need to rename internal helpers in net/ > > > > > > > - __inline_sys_waitid() # too long? > > > > > > sounds acceptable, though a bit long (especially for the compat case, > > > though > > > it doesn't really matter in the case of > > > __inline_compat_sys_sched_rr_get_interval) > > > > So as per the previous mail this is not just an inline function, but an > > active > > type conversion wrapper that sign-extends 32-bit ints to longs, which is > > important > > on some 64-bit architectures. > > > > And that's a really non-obvious property IMO, and the name should probably > > reflect > > _that_ non-obvious property, not the inlining property which is really just > > a > > small detail. > > > > I.e. how about: > > > > __se_sys_waitid() > > > > ... where 'se' stands for sign-extended, with a comment in the macro that > > explains > > the prefix? (The historical abbreviation for sign extension is 'sext', > > which I > > think wouldn't really be suitable these days.) > > Ok, so I got confused there: I think it's the do_sys_waitid() intermediate > that > is actually doing the sign-extension - and the inlined helper is what is in > the > syscall definition body. > > So it's all still somewhat of a confusing misnomer: the 'do' named function > is > actually the sign-extension function variant - and the '_il' variant actually > 'does' the real work ... > > I.e., old naming: > > 810f08d0 t kernel_waitid # common C function (see kernel/exit.c) > > __il_sys_waitid # inlined helper doing the actual work > # (takes parameters as declared) > > 810f1aa0 T __do_sys_waitid # C function calling inlined helper > # (takes parameters of type long; casts > # them to the declared type) > > 810f1aa0 Tsys_waitid # alias to __do_sys_waitid() (taking > # parameters as declared), to be included > # in syscall table > > > New suggested naming: > > 810f08d0 t kernel_waitid # common C function (see kernel/exit.c) > > __do_sys_waitid # inlined helper doing the actual work > # (takes original parameters as declared) > > 810f1aa0 T __se_sys_waitid # sign-extending C function calling inlined > # helper (takes parameters of type long; > # casts them to the declared type) > > 810f1aa0 Tsys_waitid # alias to __se_sys_waitid() (but taking > # original parameters as declared), to be > # included in syscall table > > Agreed? Yes. Thanks, Dominik
Re: [PATCH] mm/memblock: introduce PHYS_ADDR_MAX
On Fri 06-04-18 23:38:09, Stefan Agner wrote: > So far code was using ULLONG_MAX and type casting to obtain a > phys_addr_t with all bits set. The typecast is necessary to > silence compiler warnings on 32-bit platforms. > > Use the simpler but still type safe approach "~(phys_addr_t)0" > to create a preprocessor define for all bits set. > > Suggested-by: Linus Torvalds > Signed-off-by: Stefan Agner Acked-by: Michal Hocko > --- > Hi, > > There are about a dozen other instances of (phys_addr_t)ULLONG_MAX > accross the tree. Should I address them too? Yes, please. Maybe wait until the merge window sattles (rc1). > -- > Stefan > > include/linux/kernel.h | 1 + > mm/memblock.c | 22 +++--- > 2 files changed, 12 insertions(+), 11 deletions(-) > > diff --git a/include/linux/kernel.h b/include/linux/kernel.h > index 3fd291503576..1ba9e2d71bc9 100644 > --- a/include/linux/kernel.h > +++ b/include/linux/kernel.h > @@ -29,6 +29,7 @@ > #define LLONG_MIN(-LLONG_MAX - 1) > #define ULLONG_MAX (~0ULL) > #define SIZE_MAX (~(size_t)0) > +#define PHYS_ADDR_MAX(~(phys_addr_t)0) > > #define U8_MAX ((u8)~0U) > #define S8_MAX ((s8)(U8_MAX>>1)) > diff --git a/mm/memblock.c b/mm/memblock.c > index 696829a198ba..957587178b36 100644 > --- a/mm/memblock.c > +++ b/mm/memblock.c > @@ -67,7 +67,7 @@ ulong __init_memblock choose_memblock_flags(void) > /* adjust *@size so that (@base + *@size) doesn't overflow, return new size > */ > static inline phys_addr_t memblock_cap_size(phys_addr_t base, phys_addr_t > *size) > { > - return *size = min(*size, (phys_addr_t)ULLONG_MAX - base); > + return *size = min(*size, PHYS_ADDR_MAX - base); > } > > /* > @@ -924,7 +924,7 @@ void __init_memblock __next_mem_range(u64 *idx, int nid, > ulong flags, > r = &type_b->regions[idx_b]; > r_start = idx_b ? r[-1].base + r[-1].size : 0; > r_end = idx_b < type_b->cnt ? > - r->base : (phys_addr_t)ULLONG_MAX; > + r->base : PHYS_ADDR_MAX; > > /* >* if idx_b advanced past idx_a, > @@ -1040,7 +1040,7 @@ void __init_memblock __next_mem_range_rev(u64 *idx, int > nid, ulong flags, > r = &type_b->regions[idx_b]; > r_start = idx_b ? r[-1].base + r[-1].size : 0; > r_end = idx_b < type_b->cnt ? > - r->base : (phys_addr_t)ULLONG_MAX; > + r->base : PHYS_ADDR_MAX; > /* >* if idx_b advanced past idx_a, >* break out to advance idx_a > @@ -1543,13 +1543,13 @@ phys_addr_t __init_memblock memblock_end_of_DRAM(void) > > static phys_addr_t __init_memblock __find_max_addr(phys_addr_t limit) > { > - phys_addr_t max_addr = (phys_addr_t)ULLONG_MAX; > + phys_addr_t max_addr = PHYS_ADDR_MAX; > struct memblock_region *r; > > /* >* translate the memory @limit size into the max address within one of >* the memory memblock regions, if the @limit exceeds the total size > - * of those regions, max_addr will keep original value ULLONG_MAX > + * of those regions, max_addr will keep original value PHYS_ADDR_MAX >*/ > for_each_memblock(memory, r) { > if (limit <= r->size) { > @@ -1564,7 +1564,7 @@ static phys_addr_t __init_memblock > __find_max_addr(phys_addr_t limit) > > void __init memblock_enforce_memory_limit(phys_addr_t limit) > { > - phys_addr_t max_addr = (phys_addr_t)ULLONG_MAX; > + phys_addr_t max_addr = PHYS_ADDR_MAX; > > if (!limit) > return; > @@ -1572,14 +1572,14 @@ void __init memblock_enforce_memory_limit(phys_addr_t > limit) > max_addr = __find_max_addr(limit); > > /* @limit exceeds the total size of the memory, do nothing */ > - if (max_addr == (phys_addr_t)ULLONG_MAX) > + if (max_addr == PHYS_ADDR_MAX) > return; > > /* truncate both memory and reserved regions */ > memblock_remove_range(&memblock.memory, max_addr, > - (phys_addr_t)ULLONG_MAX); > + PHYS_ADDR_MAX); > memblock_remove_range(&memblock.reserved, max_addr, > - (phys_addr_t)ULLONG_MAX); > + PHYS_ADDR_MAX); > } > > void __init memblock_cap_memory_range(phys_addr_t base, phys_addr_t size) > @@ -1607,7 +1607,7 @@ void __init memblock_cap_memory_range(phys_addr_t base, > phys_addr_t size) > /* truncate the reserved regions */ > memblock_remove_range(&memblock.reserved, 0, base); > memblock_remove_range(&memblock.reserved, > - base + size, (phys_addr_t)ULLONG_MAX); > + base + size, PHYS_ADDR_MAX); > } > > void __init memblock_
Re: [PATCH] mm: shmem: enable thp migration (Re: [PATCH v1] mm: consider non-anonymous thp as unmovable page)
On Fri, Apr 06, 2018 at 09:08:15AM +0200, Michal Hocko wrote: > On Fri 06-04-18 05:14:53, Naoya Horiguchi wrote: > > On Fri, Apr 06, 2018 at 03:07:11AM +, Horiguchi Naoya(堀口 直也) wrote: > > ... > > > - > > > From e31ec037701d1cc76b26226e4b66d8c783d40889 Mon Sep 17 00:00:00 2001 > > > From: Naoya Horiguchi > > > Date: Fri, 6 Apr 2018 10:58:35 +0900 > > > Subject: [PATCH] mm: enable thp migration for shmem thp > > > > > > My testing for the latest kernel supporting thp migration showed an > > > infinite loop in offlining the memory block that is filled with shmem > > > thps. We can get out of the loop with a signal, but kernel should > > > return with failure in this case. > > > > > > What happens in the loop is that scan_movable_pages() repeats returning > > > the same pfn without any progress. That's because page migration always > > > fails for shmem thps. > > > > > > In memory offline code, memory blocks containing unmovable pages should > > > be prevented from being offline targets by has_unmovable_pages() inside > > > start_isolate_page_range(). So it's possible to change migratability > > > for non-anonymous thps to avoid the issue, but it introduces more complex > > > and thp-specific handling in migration code, so it might not good. > > > > > > So this patch is suggesting to fix the issue by enabling thp migration > > > for shmem thp. Both of anon/shmem thp are migratable so we don't need > > > precheck about the type of thps. > > > > > > Fixes: commit 72b39cfc4d75 ("mm, memory_hotplug: do not fail offlining > > > too early") > > > Signed-off-by: Naoya Horiguchi > > > Cc: sta...@vger.kernel.org # v4.15+ > > > > ... oh, I don't think this is suitable for stable. > > Michal's fix in another email can come first with "CC: stable", > > then this one. > > Anyway I want to get some feedback on the change of this patch. > > My patch is indeed much simpler but it depends on [1] and that doesn't > sound like a stable material as well because it depends on onether 2 > patches. Maybe we need some other hack for 4.15 if we really care enough. > > [1] http://lkml.kernel.org/r/20180103082555.14592-4-mho...@kernel.org OK, so I like just giving up sending to stable.
[PATCH v5 0/3] perf/core: expose thread context switch out event type to user space
Implement preempting context switch out event as a part of PERF_RECORD_SWITCH[_CPU_WIDE] record. The event is treated as preemption one when task->state value of the thread being switched out is TASK_RUNNING; Percentage of preempting and non-preempting context switches help understanding the nature of workloads (CPU or IO bound) that are running on the machine; Event type encoding is implemented using a new PERF_RECORD_MISC_SWITCH_OUT_PREEMPT bit in misc field of event record header; Perf tool report and script commands have been extended to decode new PREEMPT bit and the updated output looks like this: tools/perf/perf report -D -i perf.data | grep _SWITCH 0 768361415226 0x27f076 [0x28]: PERF_RECORD_SWITCH_CPU_WIDE IN prev pid/tid: 8/8 4 768362216813 0x28f45e [0x28]: PERF_RECORD_SWITCH_CPU_WIDE OUT next pid/tid: 0/0 4 768362217824 0x28f486 [0x28]: PERF_RECORD_SWITCH_CPU_WIDE IN prev pid/tid: 4073/4073 0 768362414027 0x27f0ce [0x28]: PERF_RECORD_SWITCH_CPU_WIDE OUT preempt next pid/tid: 8/8 0 768362414367 0x27f0f6 [0x28]: PERF_RECORD_SWITCH_CPU_WIDE IN prev pid/tid: 0/0 perf script --show-switch-events -F +misc -I -i perf.data: hdparm 4073 [004] U 762.198265: 380194 cycles:ppp: 7faf727f5a23 strchr (/usr/lib64/ld-2.26.so) hdparm 4073 [004] K 762.198366: 441572 cycles:ppp: b9218435 alloc_set_pte (/lib/modules/4.16.0-rc6+/build/vmlinux) hdparm 4073 [004] S 762.198391: PERF_RECORD_SWITCH_CPU_WIDE OUT next pid/tid: 0/0 swapper 0 [004] 762.198392: PERF_RECORD_SWITCH_CPU_WIDE IN prev pid/tid: 4073/4073 swapper 0 [004] Sp 762.198477: PERF_RECORD_SWITCH_CPU_WIDE OUT preempt next pid/tid: 4073/4073 hdparm 4073 [004] 762.198478: PERF_RECORD_SWITCH_CPU_WIDE IN prev pid/tid: 0/0 swapper 0 [007] K 762.198514:2303073 cycles:ppp: b98b0c66 intel_idle (/lib/modules/4.16.0-rc6+/build/vmlinux) swapper 0 [007] Sp 762.198561: PERF_RECORD_SWITCH_CPU_WIDE OUT preempt next pid/tid: 1134/1134 kworker/u16:18 1134 [007] 762.198562: PERF_RECORD_SWITCH_CPU_WIDE IN prev pid/tid: 0/0 kworker/u16:18 1134 [007] S 762.198567: PERF_RECORD_SWITCH_CPU_WIDE OUT next pid/tid: 0/0 The documentation has been updated to mention PREEMPT switch out events and its decoding symbols in perf script output. The changes have been manually tested on Fedora 27 with the patched kernel: git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git perf/core --- Alexey Budankov (3): perf/core: store context switch out type into Perf trace perf report: extend raw dump (-D) out with switch out event type perf script: extend misc field decoding with switch out event type include/uapi/linux/perf_event.h | 18 +++--- kernel/events/core.c | 4 tools/include/uapi/linux/perf_event.h| 18 +++--- tools/perf/Documentation/perf-script.txt | 17 + tools/perf/builtin-script.c | 5 - tools/perf/util/event.c | 4 +++- 6 files changed, 50 insertions(+), 16 deletions(-)
[PATCH RESEND v2] vhost-net: set packet weight of tx polling to 2 * vq size
handle_tx will delay rx for tens or even hundreds of milliseconds when tx busy polling udp packets with small length(e.g. 1byte udp payload), because setting VHOST_NET_WEIGHT takes into account only sent-bytes but no single packet length. Ping-Latencies shown below were tested between two Virtual Machines using netperf (UDP_STREAM, len=1), and then another machine pinged the client: vq size=256 Packet-Weight Ping-Latencies(millisecond) min avg max Origin 3.319 18.48957.303 64 1.6432.021 2.552 128 1.8252.600 3.224 256 1.9972.710 4.295 512 1.8603.171 4.631 1024 2.0024.173 9.056 2048 2.2575.650 9.688 4096 2.0938.50815.943 vq size=512 Packet-Weight Ping-Latencies(millisecond) min avg max Origin 6.537 29.17766.245 64 2.7983.614 4.403 128 2.8613.820 4.775 256 3.0084.018 4.807 512 3.2544.523 5.824 1024 3.0795.335 7.747 2048 3.9448.20112.762 4096 4.158 11.05719.985 Seems pretty consistent, a small dip at 2 VQ sizes. Ring size is a hint from device about a burst size it can tolerate. Based on benchmarks, set the weight to 2 * vq size. To evaluate this change, another tests were done using netperf(RR, TX) between two machines with Intel(R) Xeon(R) Gold 6133 CPU @ 2.50GHz, and vq size was tweaked through qemu. Results shown below does not show obvious changes. vq size=256 TCP_RRvq size=512 TCP_RR size/sessions/+thu%/+normalize% size/sessions/+thu%/+normalize% 1/ 1/ -7%/-2% 1/ 1/ 0%/-2% 1/ 4/ +1%/ 0% 1/ 4/ +1%/ 0% 1/ 8/ +1%/-2% 1/ 8/ 0%/+1% 64/ 1/ -6%/ 0% 64/ 1/ +7%/+3% 64/ 4/ 0%/+2% 64/ 4/ -1%/+1% 64/ 8/ 0%/ 0% 64/ 8/ -1%/-2% 256/ 1/ -3%/-4%256/ 1/ -4%/-2% 256/ 4/ +3%/+4%256/ 4/ +1%/+2% 256/ 8/ +2%/ 0%256/ 8/ +1%/-1% vq size=256 UDP_RRvq size=512 UDP_RR size/sessions/+thu%/+normalize% size/sessions/+thu%/+normalize% 1/ 1/ -5%/+1% 1/ 1/ -3%/-2% 1/ 4/ +4%/+1% 1/ 4/ -2%/+2% 1/ 8/ -1%/-1% 1/ 8/ -1%/ 0% 64/ 1/ -2%/-3% 64/ 1/ +1%/+1% 64/ 4/ -5%/-1% 64/ 4/ +2%/ 0% 64/ 8/ 0%/-1% 64/ 8/ -2%/+1% 256/ 1/ +7%/+1%256/ 1/ -7%/ 0% 256/ 4/ +1%/+1%256/ 4/ -3%/-4% 256/ 8/ +2%/+2%256/ 8/ +1%/+1% vq size=256 TCP_STREAMvq size=512 TCP_STREAM size/sessions/+thu%/+normalize% size/sessions/+thu%/+normalize% 64/ 1/ 0%/-3% 64/ 1/ 0%/ 0% 64/ 4/ +3%/-1% 64/ 4/ -2%/+4% 64/ 8/ +9%/-4% 64/ 8/ -1%/+2% 256/ 1/ +1%/-4%256/ 1/ +1%/+1% 256/ 4/ -1%/-1%256/ 4/ -3%/ 0% 256/ 8/ +7%/+5%256/ 8/ -3%/ 0% 512/ 1/ +1%/ 0%512/ 1/ -1%/-1% 512/ 4/ +1%/-1%512/ 4/ 0%/ 0% 512/ 8/ +7%/-5%512/ 8/ +6%/-1% 1024/ 1/ 0%/-1% 1024/ 1/ 0%/+1% 1024/ 4/ +3%/ 0% 1024/ 4/ +1%/ 0% 1024/ 8/ +8%/+5% 1024/ 8/ -1%/ 0% 2048/ 1/ +2%/+2% 2048/ 1/ -1%/ 0% 2048/ 4/ +1%/ 0% 2048/ 4/ 0%/-1% 2048/ 8/ -2%/ 0% 2048/ 8/ 5%/-1% 4096/ 1/ -2%/ 0% 4096/ 1/ -2%/ 0% 4096/ 4/ +2%/ 0% 4096/ 4/ 0%/ 0% 4096/ 8/ +9%/-2% 4096/ 8/ -5%/-1% Acked-by: Michael S. Tsirkin Signed-off-by: Haibin Zhang Signed-off-by: Yunfang Tai Signed-off-by: Lidong Chen --- drivers/vhost/net.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c index 8139bc70ad7d..3563a305cc0a 100644 --- a/drivers/vhost/net.c +++ b/drivers/vhost/net.c @@ -44,6 +44,10 @@ MODULE_PARM_DESC(experimental_zcopytx, "Enable Zero Copy TX;" * Using this limit prevents one virtqueue from starving others. */ #define VHOST_NET_WEIGHT 0x8 +/* Max number of packets transferred before requeueing the job.
[PATCH v5 1/3] perf/core: store context switch out type into Perf trace
Store preempting context switch out event into Perf trace as a part of PERF_RECORD_SWITCH[_CPU_WIDE] record. Percentage of preempting and non-preempting context switches help understanding the nature of workloads (CPU or IO bound) that are running on a machine; The event is treated as preemption one when task->state value of the thread being switched out is TASK_RUNNING. Event type encoding is implemented using PERF_RECORD_MISC_SWITCH_OUT_PREEMPT bit; Signed-off-by: Alexey Budankov --- include/uapi/linux/perf_event.h | 18 +++--- kernel/events/core.c | 4 tools/include/uapi/linux/perf_event.h | 18 +++--- 3 files changed, 34 insertions(+), 6 deletions(-) diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h index 912b85b52344..b8e288a1f740 100644 --- a/include/uapi/linux/perf_event.h +++ b/include/uapi/linux/perf_event.h @@ -650,11 +650,23 @@ struct perf_event_mmap_page { #define PERF_RECORD_MISC_COMM_EXEC (1 << 13) #define PERF_RECORD_MISC_SWITCH_OUT(1 << 13) /* - * Indicates that the content of PERF_SAMPLE_IP points to - * the actual instruction that triggered the event. See also - * perf_event_attr::precise_ip. + * These PERF_RECORD_MISC_* flags below are safely reused + * for the following events: + * + * PERF_RECORD_MISC_EXACT_IP - PERF_RECORD_SAMPLE of precise events + * PERF_RECORD_MISC_SWITCH_OUT_PREEMPT - PERF_RECORD_SWITCH* events + * + * + * PERF_RECORD_MISC_EXACT_IP: + * Indicates that the content of PERF_SAMPLE_IP points to + * the actual instruction that triggered the event. See also + * perf_event_attr::precise_ip. + * + * PERF_RECORD_MISC_SWITCH_OUT_PREEMPT: + * Indicates that thread was preempted in TASK_RUNNING state. */ #define PERF_RECORD_MISC_EXACT_IP (1 << 14) +#define PERF_RECORD_MISC_SWITCH_OUT_PREEMPT(1 << 14) /* * Reserve the last bit to indicate some extended misc field */ diff --git a/kernel/events/core.c b/kernel/events/core.c index fc1c330c6bd6..872a5aaa77eb 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -7584,6 +7584,10 @@ static void perf_event_switch(struct task_struct *task, }, }; + if (!sched_in && task->state == TASK_RUNNING) + switch_event.event_id.header.misc |= + PERF_RECORD_MISC_SWITCH_OUT_PREEMPT; + perf_iterate_sb(perf_event_switch_output, &switch_event, NULL); diff --git a/tools/include/uapi/linux/perf_event.h b/tools/include/uapi/linux/perf_event.h index 912b85b52344..b8e288a1f740 100644 --- a/tools/include/uapi/linux/perf_event.h +++ b/tools/include/uapi/linux/perf_event.h @@ -650,11 +650,23 @@ struct perf_event_mmap_page { #define PERF_RECORD_MISC_COMM_EXEC (1 << 13) #define PERF_RECORD_MISC_SWITCH_OUT(1 << 13) /* - * Indicates that the content of PERF_SAMPLE_IP points to - * the actual instruction that triggered the event. See also - * perf_event_attr::precise_ip. + * These PERF_RECORD_MISC_* flags below are safely reused + * for the following events: + * + * PERF_RECORD_MISC_EXACT_IP - PERF_RECORD_SAMPLE of precise events + * PERF_RECORD_MISC_SWITCH_OUT_PREEMPT - PERF_RECORD_SWITCH* events + * + * + * PERF_RECORD_MISC_EXACT_IP: + * Indicates that the content of PERF_SAMPLE_IP points to + * the actual instruction that triggered the event. See also + * perf_event_attr::precise_ip. + * + * PERF_RECORD_MISC_SWITCH_OUT_PREEMPT: + * Indicates that thread was preempted in TASK_RUNNING state. */ #define PERF_RECORD_MISC_EXACT_IP (1 << 14) +#define PERF_RECORD_MISC_SWITCH_OUT_PREEMPT(1 << 14) /* * Reserve the last bit to indicate some extended misc field */
Re: [RFC v2 2/2] base: dma-mapping: Postpone page_to_pfn() on mmap()
Hi Robin, Laurent, a long time passed, sorry about this. On Wed, Nov 15, 2017 at 01:38:23PM +, Robin Murphy wrote: > On 14/11/17 17:08, Jacopo Mondi wrote: > >On SH4 architecture, with SPARSEMEM memory model, translating page to > >pfn hangs the CPU. Post-pone translation to pfn after > >dma_mmap_from_dev_coherent() function call as it succeeds and make page > >translation not necessary. > > > >This patch was suggested by Laurent Pinchart and he's working to submit > >a proper fix mainline. Not sending for inclusion at the moment. > > Y'know, I think this patch does have some merit by itself - until we know > that cpu_addr *doesn't* represent some device-private memory which is not > guaranteed to be backed by a struct page, calling virt_to_page() on it is > arguably semantically incorrect, even if it might happen to be benign in > most cases. I still need to carry this patch in my trees to have a working dma memory mapping on SH4 platforms. My understanding from your comment is that there may be a way forward for this patch, do you still think the same? Have you got any suggestion on how to improve this eventually for inclusion? Thanks j > > Robin. > > >Suggested-by: Laurent Pinchart > >Signed-off-by: Jacopo Mondi > >--- > > drivers/base/dma-mapping.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > >diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c > >index e584edd..73d64d3 100644 > >--- a/drivers/base/dma-mapping.c > >+++ b/drivers/base/dma-mapping.c > >@@ -227,8 +227,8 @@ int dma_common_mmap(struct device *dev, struct > >vm_area_struct *vma, > > #ifndef CONFIG_ARCH_NO_COHERENT_DMA_MMAP > > unsigned long user_count = vma_pages(vma); > > unsigned long count = PAGE_ALIGN(size) >> PAGE_SHIFT; > >-unsigned long pfn = page_to_pfn(virt_to_page(cpu_addr)); > > unsigned long off = vma->vm_pgoff; > >+unsigned long pfn; > > > > vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); > > > >@@ -236,6 +236,7 @@ int dma_common_mmap(struct device *dev, struct > >vm_area_struct *vma, > > return ret; > > > > if (off < count && user_count <= (count - off)) { > >+pfn = page_to_pfn(virt_to_page(cpu_addr)); > > ret = remap_pfn_range(vma, vma->vm_start, > > pfn + off, > > user_count << PAGE_SHIFT, > >-- > >2.7.4 > > > >___ > >iommu mailing list > >io...@lists.linux-foundation.org > >https://lists.linuxfoundation.org/mailman/listinfo/iommu > > signature.asc Description: PGP signature
[PATCH v5 2/3] perf report: extend raw dump (-D) out with switch out event type
Print additional 'preempt' tag for PERF_RECORD_SWITCH[_CPU_WIDE] OUT records when event header misc field contains PERF_RECORD_MISC_SWITCH_OUT_PREEMPT bit set designating preemption context switch out event: tools/perf/perf report -D -i perf.data | grep _SWITCH 0 768361415226 0x27f076 [0x28]: PERF_RECORD_SWITCH_CPU_WIDE IN prev pid/tid: 8/8 4 768362216813 0x28f45e [0x28]: PERF_RECORD_SWITCH_CPU_WIDE OUT next pid/tid: 0/0 4 768362217824 0x28f486 [0x28]: PERF_RECORD_SWITCH_CPU_WIDE IN prev pid/tid: 4073/4073 0 768362414027 0x27f0ce [0x28]: PERF_RECORD_SWITCH_CPU_WIDE OUT preempt next pid/tid: 8/8 0 768362414367 0x27f0f6 [0x28]: PERF_RECORD_SWITCH_CPU_WIDE IN prev pid/tid: 0/0 Signed-off-by: Alexey Budankov --- tools/perf/util/event.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c index f0a6cbd033cc..98ff3a6a3d50 100644 --- a/tools/perf/util/event.c +++ b/tools/perf/util/event.c @@ -1421,7 +1421,9 @@ size_t perf_event__fprintf_itrace_start(union perf_event *event, FILE *fp) size_t perf_event__fprintf_switch(union perf_event *event, FILE *fp) { bool out = event->header.misc & PERF_RECORD_MISC_SWITCH_OUT; - const char *in_out = out ? "OUT" : "IN "; + const char *in_out = !out ? "IN " : + !(event->header.misc & PERF_RECORD_MISC_SWITCH_OUT_PREEMPT) ? + "OUT" : "OUT preempt"; if (event->header.type == PERF_RECORD_SWITCH) return fprintf(fp, " %s\n", in_out);
[PATCH v5 3/3] perf script: extend misc field decoding with switch out event type
Append 'p' sign to 'S' tag designating the type of context switch out event so 'Sp' means preemption context switch. Documentation is extended to cover new presentation changes. perf script --show-switch-events -F +misc -I -i perf.data: hdparm 4073 [004] U 762.198265: 380194 cycles:ppp: 7faf727f5a23 strchr (/usr/lib64/ld-2.26.so) hdparm 4073 [004] K 762.198366: 441572 cycles:ppp: b9218435 alloc_set_pte (/lib/modules/4.16.0-rc6+/build/vmlinux) hdparm 4073 [004] S 762.198391: PERF_RECORD_SWITCH_CPU_WIDE OUT next pid/tid: 0/0 swapper 0 [004] 762.198392: PERF_RECORD_SWITCH_CPU_WIDE IN prev pid/tid: 4073/4073 swapper 0 [004] Sp 762.198477: PERF_RECORD_SWITCH_CPU_WIDE OUT preempt next pid/tid: 4073/4073 hdparm 4073 [004] 762.198478: PERF_RECORD_SWITCH_CPU_WIDE IN prev pid/tid: 0/0 swapper 0 [007] K 762.198514:2303073 cycles:ppp: b98b0c66 intel_idle (/lib/modules/4.16.0-rc6+/build/vmlinux) swapper 0 [007] Sp 762.198561: PERF_RECORD_SWITCH_CPU_WIDE OUT preempt next pid/tid: 1134/1134 kworker/u16:18 1134 [007] 762.198562: PERF_RECORD_SWITCH_CPU_WIDE IN prev pid/tid: 0/0 kworker/u16:18 1134 [007] S 762.198567: PERF_RECORD_SWITCH_CPU_WIDE OUT next pid/tid: 0/0 Signed-off-by: Alexey Budankov --- tools/perf/Documentation/perf-script.txt | 17 + tools/perf/builtin-script.c | 5 - 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt index 36ec0257f8d3..afdafe2110a1 100644 --- a/tools/perf/Documentation/perf-script.txt +++ b/tools/perf/Documentation/perf-script.txt @@ -228,14 +228,15 @@ OPTIONS For sample events it's possible to display misc field with -F +misc option, following letters are displayed for each bit: - PERF_RECORD_MISC_KERNELK - PERF_RECORD_MISC_USER U - PERF_RECORD_MISC_HYPERVISORH - PERF_RECORD_MISC_GUEST_KERNEL G - PERF_RECORD_MISC_GUEST_USERg - PERF_RECORD_MISC_MMAP_DATA*M - PERF_RECORD_MISC_COMM_EXEC E - PERF_RECORD_MISC_SWITCH_OUTS + PERF_RECORD_MISC_KERNEL K + PERF_RECORD_MISC_USER U + PERF_RECORD_MISC_HYPERVISOR H + PERF_RECORD_MISC_GUEST_KERNEL G + PERF_RECORD_MISC_GUEST_USER g + PERF_RECORD_MISC_MMAP_DATA* M + PERF_RECORD_MISC_COMM_EXECE + PERF_RECORD_MISC_SWITCH_OUT S + PERF_RECORD_MISC_SWITCH_OUT_PREEMPT Sp $ perf script -F +misc ... sched-messaging 1414 K 28690.636582: 4590 cycles ... diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c index 313c42423393..0f916e8e1835 100644 --- a/tools/perf/builtin-script.c +++ b/tools/perf/builtin-script.c @@ -657,8 +657,11 @@ static int perf_sample__fprintf_start(struct perf_sample *sample, break; case PERF_RECORD_SWITCH: case PERF_RECORD_SWITCH_CPU_WIDE: - if (has(SWITCH_OUT)) + if (has(SWITCH_OUT)) { ret += fprintf(fp, "S"); + if (sample->misc & PERF_RECORD_MISC_SWITCH_OUT_PREEMPT) + ret += fprintf(fp, "p"); + } default: break; }
Re: [PATCH v2 1/3] stacktrace: move arch_within_stack_frames from thread_info.h
Hi Sahara, Thank you for the patch! Yet something to improve: [auto build test ERROR on linus/master] [also build test ERROR on v4.16 next-20180406] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/kpark3469-gmail-com/usercopy-reimplement-arch_within_stack_frames/20180409-144349 config: i386-randconfig-x071-201814 (attached as .config) compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 reproduce: # save the attached .config to linux build tree make ARCH=i386 All errors (new ones prefixed by >>): from include/linux/time.h:6, from include/linux/ktime.h:24, from include/linux/rcutiny.h:28, from include/linux/rcupdate.h:211, from include/linux/rculist.h:11, from include/linux/pid.h:5, from include/linux/sched.h:14, from include/linux/uaccess.h:5, from arch/x86/include/asm/stacktrace.h:10, from include/linux/stacktrace.h:6, from include/linux/lockdep.h:29, from include/linux/spinlock_types.h:18, from kernel/bounds.c:14: include/linux/spinlock.h:297:24: error: unknown type name 'raw_spinlock_t' static __always_inline raw_spinlock_t *spinlock_check(spinlock_t *lock) ^~ include/linux/spinlock.h:297:55: error: unknown type name 'spinlock_t'; did you mean 'clock_t'? static __always_inline raw_spinlock_t *spinlock_check(spinlock_t *lock) ^~ clock_t include/linux/spinlock.h:308:39: error: unknown type name 'spinlock_t'; did you mean 'clock_t'? static __always_inline void spin_lock(spinlock_t *lock) ^~ clock_t include/linux/spinlock.h:313:42: error: unknown type name 'spinlock_t'; did you mean 'clock_t'? static __always_inline void spin_lock_bh(spinlock_t *lock) ^~ clock_t include/linux/spinlock.h:318:41: error: unknown type name 'spinlock_t'; did you mean 'clock_t'? static __always_inline int spin_trylock(spinlock_t *lock) ^~ clock_t include/linux/spinlock.h:333:43: error: unknown type name 'spinlock_t'; did you mean 'clock_t'? static __always_inline void spin_lock_irq(spinlock_t *lock) ^~ clock_t include/linux/spinlock.h:348:41: error: unknown type name 'spinlock_t'; did you mean 'clock_t'? static __always_inline void spin_unlock(spinlock_t *lock) ^~ clock_t include/linux/spinlock.h:353:44: error: unknown type name 'spinlock_t'; did you mean 'clock_t'? static __always_inline void spin_unlock_bh(spinlock_t *lock) ^~ clock_t include/linux/spinlock.h:358:45: error: unknown type name 'spinlock_t'; did you mean 'clock_t'? static __always_inline void spin_unlock_irq(spinlock_t *lock) ^~ clock_t include/linux/spinlock.h:363:52: error: unknown type name 'spinlock_t'; did you mean 'clock_t'? static __always_inline void spin_unlock_irqrestore(spinlock_t *lock, unsigned long flags) ^~ clock_t include/linux/spinlock.h:368:44: error: unknown type name 'spinlock_t'; did you mean 'clock_t'? static __always_inline int spin_trylock_bh(spinlock_t *lock) ^~ clock_t include/linux/spinlock.h:373:45: error: unknown type name 'spinlock_t'; did you mean 'clock_t'? static __always_inline int spin_trylock_irq(spinlock_t *lock) ^~ clock_t include/linux/spinlock.h:383:43: error: unknown type name 'spinlock_t'; did you mean 'clock_t'? static __always_inline int spin_is_locked
Re: [PATCH] HID: input: fix battery level reporting on BT mice
On Tue, 3 Apr 2018, Dmitry Torokhov wrote: > The commit 581c4484769e ("HID: input: map digitizer battery usage") > assumed that devices having input (qas opposed to feature) report for > battery strength would report the data on their own, without the need to > be polled by the kernel; unfortunately it is not so. Many wireless mice > do not send unsolicited reports with battery strength data and have to > be polled explicitly. As a complication, stylus devices on digitizers > are not normally connected to the base and thus can not be polled - the > base can only determine battery strength in the stylus when it is in > proximity. > > To solve this issue, we add a special flag that tells the kernel > to avoid polling the device (and expect unsolicited reports) and set it > when report field with physical usage of digitizer stylus (HID_DG_STYLUS). > Unless this flag is set, and we have not seen the unsolicited reports, > the kernel will attempt to poll the device when userspace attempts to > read "capacity" and "state" attributes of power_supply object > corresponding to the devices battery. > > Fixes: 581c4484769e ("HID: input: map digitizer battery usage") > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=198095 > Cc: sta...@vger.kernel.org > Signed-off-by: Dmitry Torokhov Applied for 4.17, thanks. -- Jiri Kosina SUSE Labs
Re: [PATCH 2/2] HID: i2c-hid: Fix resume issue on Raydium touchscreen device
On Tue, 3 Apr 2018, Aaron Ma wrote: > Hi Jiri: > > This patch is pending for long time. > > Could you merge this single patch to upstream? I don't this I am seeing it in my inbox. Could you please resend? Thanks, -- Jiri Kosina SUSE Labs
Re: [PATCH v2 9/9] perf probe: Support SDT markers having reference counter (semaphore)
Hi Ravi, On Wed, 4 Apr 2018 14:01:10 +0530 Ravi Bangoria wrote: > With this, perf buildid-cache will save SDT markers with reference > counter in probe cache. Perf probe will be able to probe markers > having reference counter. Ex, > > # readelf -n /tmp/tick | grep -A1 loop2 > Name: loop2 > ... Semaphore: 0x10020036 > > # ./perf buildid-cache --add /tmp/tick > # ./perf probe sdt_tick:loop2 > # ./perf stat -e sdt_tick:loop2 /tmp/tick > hi: 0 > hi: 1 > hi: 2 > ^C > Performance counter stats for '/tmp/tick': > 3 sdt_tick:loop2 >2.561851452 seconds time elapsed > > Signed-off-by: Ravi Bangoria > --- > tools/perf/util/probe-event.c | 18 ++--- > tools/perf/util/probe-event.h | 1 + > tools/perf/util/probe-file.c | 34 ++-- > tools/perf/util/probe-file.h | 1 + > tools/perf/util/symbol-elf.c | 46 > --- > tools/perf/util/symbol.h | 7 +++ > 6 files changed, 86 insertions(+), 21 deletions(-) > > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c > index e1dbc98..b3a1330 100644 > --- a/tools/perf/util/probe-event.c > +++ b/tools/perf/util/probe-event.c > @@ -1832,6 +1832,12 @@ int parse_probe_trace_command(const char *cmd, struct > probe_trace_event *tev) > tp->offset = strtoul(fmt2_str, NULL, 10); > } > > + if (tev->uprobes) { > + fmt2_str = strchr(p, '('); > + if (fmt2_str) > + tp->ref_ctr_offset = strtoul(fmt2_str + 1, NULL, 0); > + } > + > tev->nargs = argc - 2; > tev->args = zalloc(sizeof(struct probe_trace_arg) * tev->nargs); > if (tev->args == NULL) { > @@ -2054,15 +2060,21 @@ char *synthesize_probe_trace_command(struct > probe_trace_event *tev) > } > > /* Use the tp->address for uprobes */ > - if (tev->uprobes) > + if (tev->uprobes) { > err = strbuf_addf(&buf, "%s:0x%lx", tp->module, tp->address); > - else if (!strncmp(tp->symbol, "0x", 2)) > + if (uprobe_ref_ctr_is_supported() && > + tp->ref_ctr_offset && > + err >= 0) > + err = strbuf_addf(&buf, "(0x%lx)", tp->ref_ctr_offset); If the kernel doesn't support uprobe_ref_ctr but the event requires to increment uprobe_ref_ctr, I think we should (at least) warn user here. > + } else if (!strncmp(tp->symbol, "0x", 2)) { > /* Absolute address. See try_to_find_absolute_address() */ > err = strbuf_addf(&buf, "%s%s0x%lx", tp->module ?: "", > tp->module ? ":" : "", tp->address); > - else > + } else { > err = strbuf_addf(&buf, "%s%s%s+%lu", tp->module ?: "", > tp->module ? ":" : "", tp->symbol, tp->offset); > + } > + > if (err) > goto error; > > diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h > index 45b14f0..15a98c3 100644 > --- a/tools/perf/util/probe-event.h > +++ b/tools/perf/util/probe-event.h > @@ -27,6 +27,7 @@ struct probe_trace_point { > char*symbol;/* Base symbol */ > char*module;/* Module name */ > unsigned long offset; /* Offset from symbol */ > + unsigned long ref_ctr_offset; /* SDT reference counter offset */ > unsigned long address;/* Actual address of the trace point */ > boolretprobe; /* Return probe flag */ > }; > diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c > index 4ae1123..ca0e524 100644 > --- a/tools/perf/util/probe-file.c > +++ b/tools/perf/util/probe-file.c > @@ -697,8 +697,16 @@ int probe_cache__add_entry(struct probe_cache *pcache, > #ifdef HAVE_GELF_GETNOTE_SUPPORT > static unsigned long long sdt_note__get_addr(struct sdt_note *note) > { > - return note->bit32 ? (unsigned long long)note->addr.a32[0] > - : (unsigned long long)note->addr.a64[0]; > + return note->bit32 ? > + (unsigned long long)note->addr.a32[SDT_NOTE_IDX_LOC] : > + (unsigned long long)note->addr.a64[SDT_NOTE_IDX_LOC]; > +} > + > +static unsigned long long sdt_note__get_ref_ctr_offset(struct sdt_note *note) > +{ > + return note->bit32 ? > + (unsigned long long)note->addr.a32[SDT_NOTE_IDX_REFCTR] : > + (unsigned long long)note->addr.a64[SDT_NOTE_IDX_REFCTR]; > } > > static const char * const type_to_suffix[] = { > @@ -776,14 +784,21 @@ static char *synthesize_sdt_probe_command(struct > sdt_note *note, > { > struct strbuf buf; > char *ret = NULL, **args; > - int i, args_count; > + int i, args_count, err; > + unsigned long long ref_ctr_offset; > > if (strbuf_init(&buf, 32) < 0) > return NULL; > > - if (strbuf_addf(&buf, "p:%s/%s %s:0x%llx
Re: [PATCH] hidraw: Fix crash on HIDIOCGFEATURE with a destroyed device
On Fri, 6 Apr 2018, Rodrigo Rivas Costa wrote: > Doing `ioctl(HIDIOCGFEATURE)` in a tight loop on a hidraw device > and then disconnecting the device, or unloading the driver, can > cause a NULL pointer dereference. > > When a hidraw device is destroyed it sets 0 to `dev->exist`. > Most functions check 'dev->exist' before doing its work, but > `hidraw_get_report()` was missing that check. > > Signed-off-by: Rodrigo Rivas Costa Applied, thank you. -- Jiri Kosina SUSE Labs
Re: [PATCH 2/2] HID: i2c-hid: Fix resume issue on Raydium touchscreen device
Hi Jiri: Re-send this patch. Thanks, Aaron On 01/03/2018 01:30 AM, Aaron Ma wrote: > When Rayd touchscreen resumed from S3, it issues too many errors like: > i2c_hid i2c-RAYD0001:00: i2c_hid_get_input: incomplete report (58/5442) > > And all the report data are corrupted, touchscreen is unresponsive. > > Fix this by re-sending report description command after resume. > Add device ID as a quirk. > > Cc: sta...@vger.kernel.org > Signed-off-by: Aaron Ma > --- > drivers/hid/hid-ids.h | 3 +++ > drivers/hid/i2c-hid/i2c-hid.c | 13 + > 2 files changed, 16 insertions(+) > > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h > index 5da3d6256d25..753cc10aa699 100644 > --- a/drivers/hid/hid-ids.h > +++ b/drivers/hid/hid-ids.h > @@ -516,6 +516,9 @@ > #define I2C_VENDOR_ID_HANTICK0x0911 > #define I2C_PRODUCT_ID_HANTICK_5288 0x5288 > > +#define I2C_VENDOR_ID_RAYD 0x2386 > +#define I2C_PRODUCT_ID_RAYD_3118 0x3118 > + > #define USB_VENDOR_ID_HANWANG0x0b57 > #define USB_DEVICE_ID_HANWANG_TABLET_FIRST 0x5000 > #define USB_DEVICE_ID_HANWANG_TABLET_LAST0x8fff > diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c > index 09404ffdb08b..57a447a9d40e 100644 > --- a/drivers/hid/i2c-hid/i2c-hid.c > +++ b/drivers/hid/i2c-hid/i2c-hid.c > @@ -47,6 +47,7 @@ > /* quirks to control the device */ > #define I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV BIT(0) > #define I2C_HID_QUIRK_NO_IRQ_AFTER_RESET BIT(1) > +#define I2C_HID_QUIRK_RESEND_REPORT_DESCRBIT(2) > > /* flags */ > #define I2C_HID_STARTED 0 > @@ -171,6 +172,8 @@ static const struct i2c_hid_quirks { > I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV }, > { I2C_VENDOR_ID_HANTICK, I2C_PRODUCT_ID_HANTICK_5288, > I2C_HID_QUIRK_NO_IRQ_AFTER_RESET }, > + { I2C_VENDOR_ID_RAYD, I2C_PRODUCT_ID_RAYD_3118, > + I2C_HID_QUIRK_RESEND_REPORT_DESCR }, > { 0, 0 } > }; > > @@ -1211,6 +1214,16 @@ static int i2c_hid_resume(struct device *dev) > if (ret) > return ret; > > + /* RAYDIUM device (2386:3118) need to re-send report descr cmd > + * after resume, after this it will be back normal. > + * otherwise it issues too many incomplete reports. > + */ > + if (ihid->quirks & I2C_HID_QUIRK_RESEND_REPORT_DESCR) { > + ret = i2c_hid_command(client, &hid_report_descr_cmd, NULL, 0); > + if (!ret) > + return ret; > + } > + > if (hid->driver && hid->driver->reset_resume) { > ret = hid->driver->reset_resume(hid); > return ret; > -- 2.14.3 >
Re: __GFP_LOW
On Sat 07-04-18 21:27:09, Matthew Wilcox wrote: > On Fri, Apr 06, 2018 at 08:09:53AM +0200, Michal Hocko wrote: > > OK, we already split the documentation into these categories. So we got > > at least the structure right ;) > > Yes, this part of the documentation makes sense to me :-) > > > > - What kind of memory to allocate (DMA, NORMAL, HIGHMEM) > > > - Where to get the pages from > > >- Local node only (THISNODE) > > >- Only in compliance with cpuset policy (HARDWALL) > > >- Spread the pages between zones (WRITE) > > >- The movable zone (MOVABLE) > > >- The reclaimable zone (RECLAIMABLE) > > > - What you are willing to do if no free memory is available: > > >- Nothing at all (NOWAIT) > > >- Use my own time to free memory (DIRECT_RECLAIM) > > > - But only try once (NORETRY) > > > - Can call into filesystems (FS) > > > - Can start I/O (IO) > > > - Can sleep (!ATOMIC) > > >- Steal time from other processes to free memory (KSWAPD_RECLAIM) > > > > What does that mean? If I drop the flag, do not steal? Well I do because > > they will hit direct reclaim sooner... > > If they allocate memory, sure. A process which stays in its working > set won't, unless it's preempted by kswapd. Well, I was probably not clear here. KSWAPD_RECLAIM is not something you want to drop because this is a cooperative flag. If you do not use it then you are effectivelly pushing others to the direct reclaim because the kswapd won't be woken up and won't do the background work. Your working make it sound as a good thing to drop. > > >- Kill other processes to get their memory (!RETRY_MAYFAIL) > > > > Not really for costly orders. > > Yes, need to be more precise there. > > > >- All of the above, and wait forever (NOFAIL) > > >- Take from emergency reserves (HIGH) > > >- ... but not the last parts of the regular reserves (LOW) > > > > What does that mean and how it is different from NOWAIT? Is this about > > the low watermark and if yes do we want to teach users about this and > > make the whole thing even more complicated? Does it wake > > kswapd? What is the eagerness ordering? LOW, NOWAIT, NORETRY, > > RETRY_MAYFAIL, NOFAIL? > > LOW doesn't quite fit into the eagerness scale with the other flags; > instead it's composable with them. So you can specify NOWAIT | LOW, > NORETRY | LOW, NOFAIL | LOW, etc. All I have in mind is something > like this: > > if (alloc_flags & ALLOC_HIGH) > min -= min / 2; > + if (alloc_flags & ALLOC_LOW) > + min += min / 2; > > The idea is that a GFP_KERNEL | __GFP_LOW allocation cannot force a > GFP_KERNEL allocation into an OOM situation because it cannot take > the last pages of memory before the watermark. So what are we going to do if the LOW watermark cannot succeed? > It can still make a > GFP_KERNEL allocation *more likely* to hit OOM (just like any other kind > of allocation can), but it can't do it by itself. So who would be a user of __GFP_LOW? > --- > > I've been wondering about combining the DIRECT_RECLAIM, NORETRY, > RETRY_MAYFAIL and NOFAIL flags together into a single field: > 0 => RECLAIM_NEVER, /* !DIRECT_RECLAIM */ > 1 => RECLAIM_ONCE,/* NORETRY */ > 2 => RECLAIM_PROGRESS,/* RETRY_MAYFAIL */ > 3 => RECLAIM_FOREVER, /* NOFAIL */ > > The existance of __GFP_RECLAIM makes this a bit tricky. I honestly don't > know what this code is asking for: I am not sure I follow here. Is the RECLAIM_ an internal thing to the allocator? > kernel/power/swap.c: __get_free_page(__GFP_RECLAIM | > __GFP_HIGH); > but I suspect I'll have to find out. There's about 60 places to look at. Well, it would be more understandable if this was written as (GFP_KERNEL | __GFP_HIGH) & ~(__GFP_FS|__GFP_IO) > I also want to add __GFP_KILL (to be part of the GFP_KERNEL definition). What does __GFP_KILL means? > That way, each bit that you set in the GFP mask increases the things the > page allocator can do to get memory for you. At the moment, RETRY_MAYFAIL > subtracts the ability to kill other tasks, which is unusual. Well, it is not all that great because some flags add capabilities while some remove them but, well, life is hard when you try to extend an interface which was not all that great from the very beginning. > For example, > this test in kvmalloc_node: > > WARN_ON_ONCE((flags & GFP_KERNEL) != GFP_KERNEL); > > doesn't catch RETRY_MAYFAIL being set. It doesn't really have to. We want to catch obviously broken gfp flags here. That means mostly GFP_NO{FS,IO} because those might simply deadlock. RETRY_MAYFAIL is even supported to some extend. -- Michal Hocko SUSE Labs
Re: [linux-sunxi] [PATCH v3] ARM: sun8i: v40: enable USB host ports for Banana Pi M2 Berry
On Fri, Apr 6, 2018 at 10:03 PM, Icenowy Zheng wrote: > Banana Pi M2 Berry has an on-board USB Hub that provides 4 USB Type-A > ports, and it's connected to the USB1 port of the SoC. > > Enable it. > > Signed-off-by: Icenowy Zheng Verified against public schematics. Reviewed-by: Chen-Yu Tsai
Re: linux-next: build failure after merge of the nvdimm tree
On Mon, Apr 9, 2018 at 1:38 PM, Oliver wrote: > On Mon, Apr 9, 2018 at 1:16 PM, Stephen Rothwell > wrote: >> Hi Dan, >> >> After merging the nvdimm tree, today's linux-next build (x86_64 >> allmodconfig) failed like this: >> >> ERROR: "of_node_to_nid" [drivers/nvdimm/of_pmem.ko] undefined! >> >> Caused by commit >> >> 717197608952 ("libnvdimm: Add device-tree based driver") >> >> X86 seems to not have a version of of_node_to_nid() even though CONFIG_OF >> and CONFIG_NUMA are both 'y' in this build. > > It's a side effect of a driver selecting CONFIG_OF accidently. There's > a patch to fix this in the drm-misc-next tree: > > https://lkml.org/lkml/2018/4/3/17 > >> I have used the nvdimm tree from next-20180406 for today. > > That works too. Hmm, on closer inspection this is actually a bug in the of driver. The patch above fixes the specific problem that we hit on ia64 due to CONFIG_OF being selected by accident, but the underlying issue will affect any platform that doesn't provide an implementation of of_node_to_nid(). The fundamental problem is that the various actual implementations of of_node_to_nid (ppc, sparc and the generic one in of_numa.c) export the symbol. While the fallback implementation does not because it is defined as a weak symbol. As a result we'll get this build failure iff there's a call to of_node_to_nid() in a module. The one-line fix is just to delete the call to of_node_to_nid() in of_pmem.c. I have a patch that adds a Kconfig options and removes the weak symbol games. That needs Acks from the ppc, sparc and DT maintainers though so it'll take longer to organise. Oliver
Re: [PATCH 0/3] syscalls: clean up stub naming convention
* Dominik Brodowski wrote: > > New suggested naming: > > > > 810f08d0 t kernel_waitid# common C function (see kernel/exit.c) > > > > __do_sys_waitid# inlined helper doing the actual work > > # (takes original parameters as declared) > > > > 810f1aa0 T __se_sys_waitid# sign-extending C function calling > > inlined > > # helper (takes parameters of type long; > > # casts them to the declared type) > > > > 810f1aa0 Tsys_waitid# alias to __se_sys_waitid() (but taking > > # original parameters as declared), to be > > # included in syscall table > > > > Agreed? > > Yes. Ok, great. Since these re-renames look complex enough, mind re-sending the series with these changes incorporated and the 4/3 patch incorporated as well (and the image shrinking script left out)? The base patches are looking good here so I plan sending these to Linus tomorrow-ish. (The renames don't affect functionality so they don't need as much testing.) Thanks, Ingo
Re: [PATCH] sched: support dynamiQ cluster
Hi Morten, On 6 April 2018 at 14:58, Morten Rasmussen wrote: > On Thu, Apr 05, 2018 at 06:22:48PM +0200, Vincent Guittot wrote: >> Hi Morten, >> >> On 5 April 2018 at 17:46, Morten Rasmussen wrote: >> > On Wed, Apr 04, 2018 at 03:43:17PM +0200, Vincent Guittot wrote: >> >> On 4 April 2018 at 12:44, Valentin Schneider >> >> wrote: [snip] >> >> > What I meant was that if the task composition changes, IOW we mix >> >> > "small" >> >> > tasks (e.g. periodic stuff) and "big" tasks (performance-sensitive >> >> > stuff like >> >> > sysbench threads), we shouldn't assume all of those require to run on a >> >> > big >> >> > CPU. The thing is, ASYM_PACKING can't make the difference between >> >> > those, so >> >> >> >> That's the 1st point where I tend to disagree: why big cores are only >> >> for long running task and periodic stuff can't need to run on big >> >> cores to get max compute capacity ? >> >> You make the assumption that only long running tasks need high compute >> >> capacity. This patch wants to always provide max compute capacity to >> >> the system and not only long running task >> > >> > There is no way we can tell if a periodic or short-running tasks >> > requires the compute capacity of a big core or not based on utilization >> > alone. The utilization can only tell us if a task could potentially use >> > more compute capacity, i.e. the utilization approaches the compute >> > capacity of its current cpu. >> > >> > How we handle low utilization tasks comes down to how we define >> > "performance" and if we care about the cost of "performance" (e.g. >> > energy consumption). >> > >> > Placing a low utilization task on a little cpu should always be fine >> > from _throughput_ point of view. As long as the cpu has spare cycles it >> >> I disagree, throughput is not only a matter of spare cycle it's also a >> matter of how fast you compute the work like with IO activity as an >> example > > From a cpu centric point of view it is, but I agree that from a > application/user point of view completion time might impact throughput > too. For example of if your throughput depends on how fast you can > offload work to some peripheral device (GPU for example). > > However, as I said in the beginning we don't know what the task does. I agree but that's not what you do with misfit as you assume long running task has higher priority but not shorter running tasks > >> > means that work isn't piling up faster than it can be processed. >> > However, from a _latency_ (completion time) point of view it might be a >> > problem, and for latency sensitive tasks I can agree that going for max >> > capacity might be better choice. >> > >> > The misfit patches places tasks based on utilization to ensure that >> > tasks get the _throughput_ they need if possible. This is in line with >> > the placement policy we have in select_task_rq_fair() already. >> > >> > We shouldn't forget that what we are discussing here is the default >> > behaviour when we don't have sufficient knowledge about the tasks in the >> > scheduler. So we are looking a reasonable middle-of-the-road policy that >> > doesn't kill your performance or the battery. If user-space has its own >> >> But misfit task kills performance and might also kills your battery as >> it doesn't prevent small task to run on big cores > > As I said it is not perfect for all use-cases, it is middle-of-the-road > approach. But I strongly disagree that it is always a bad choice for mmh ... I never said that it's always a bad choice; I said that it can also easily make bad choice and kills performance and / or battery. In fact, we can't really predict the behavior of the system as short running tasks can be randomly put on big or little cores and random behavior are impossible to predict and mitigate. > both energy and performance as you suggest. ASYM_PACKING doesn't > guarantee max "throughput" (by your definition) either as you may fill > up your big cores with smaller tasks leaving the big tasks behind on > little cpus. You didn't understand the point here. Asym ensures the max throughput to the system because it will provide the max compute capacity per seconds to the whole system and not only to some specific tasks. You assume that long running tasks must run on big cores and not short running tasks. But why filling a big core with long running task and filling a little core with short running tasks is the best choice ? Why the opposite should not be better as long as the big core is fully used ? The goal is to keep big CPU used whatever the type of tasks. then, there are other mechanism like cgroup to help sorting groups of tasks. You try to partially do 2 things at the same time > >> The default behavior of the scheduler is to provide max _throughput_ >> not middle performance and then side activity can mitigate the power >> impact like frequency scaling or like EAS which tries to optimize the >> usage of energy when system is not overloaded. > > That view
Re: uprobes/perf: KASAN: use-after-free in uprobe_perf_close
On Tue, Mar 06, 2018 at 06:49:10PM +0900, Prashant Bhole wrote: > Sorry for late reply. I tried these changes. It didn't fix the problem. With He, sorry for completely forgetting about this one :/ > these changes, the use-after-free access of task_struct occurs at > _free_event() for the last remaining event. > > In your changes, I tried keeping get/put_task_struct() in > perf_alloc_context()/put_ctx() intact and The problem did not occur. Changes > are mentioned below. Yes, I think you're right in that this is the cleanest solution; it adds reference counting to the exact pointer we're using. > diff --git a/kernel/events/core.c b/kernel/events/core.c > index c98cce4ceebd..65889d2b5ae2 100644 > --- a/kernel/events/core.c > +++ b/kernel/events/core.c > @@ -4109,6 +4109,8 @@ static void _free_event(struct perf_event *event) > > if (event->ctx) > put_ctx(event->ctx); > + if (event->hw.target) > + put_task_struct(event->hw.target); > > exclusive_event_destroy(event); > module_put(event->pmu->module); > @@ -9593,6 +9595,7 @@ perf_event_alloc(struct perf_event_attr *attr, int > cpu, >* and we cannot use the ctx information because we need the >* pmu before we get a ctx. >*/ > + get_task_struct(task); > event->hw.target = task; > } > > @@ -9708,6 +9711,8 @@ perf_event_alloc(struct perf_event_attr *attr, int > cpu, > perf_detach_cgroup(event); > if (event->ns) > put_pid_ns(event->ns); > + if (task) Should this not too be 'event->hw.target', for consistency and clarity? > + put_task_struct(task); > kfree(event); > > return ERR_PTR(err);
WARNING in notify_change
Hello, syzbot hit the following crash on upstream commit 3fd14cdcc05a682b03743683ce3a726898b20555 (Fri Apr 6 19:15:41 2018 +) Merge tag 'mtd/for-4.17' of git://git.infradead.org/linux-mtd syzbot dashboard link: https://syzkaller.appspot.com/bug?extid=2b74da47f048a5046135 So far this crash happened 3 times on upstream. C reproducer: https://syzkaller.appspot.com/x/repro.c?id=5084712116682752 syzkaller reproducer: https://syzkaller.appspot.com/x/repro.syz?id=6394049002995712 Raw console output: https://syzkaller.appspot.com/x/log.txt?id=4711531803574272 Kernel config: https://syzkaller.appspot.com/x/.config?id=-5813481738265533882 compiler: gcc (GCC) 8.0.1 20180301 (experimental) IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+2b74da47f048a5046...@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. WARNING: CPU: 1 PID: 4470 at fs/attr.c:213 notify_change+0xd94/0x10c0 fs/attr.c:213 Kernel panic - not syncing: panic_on_warn set ... CPU: 1 PID: 4470 Comm: syzkaller570118 Not tainted 4.16.0+ #4 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0x1b9/0x294 lib/dump_stack.c:113 panic+0x22f/0x4de kernel/panic.c:183 __warn.cold.8+0x163/0x1a3 kernel/panic.c:547 report_bug+0x252/0x2d0 lib/bug.c:186 fixup_bug arch/x86/kernel/traps.c:178 [inline] do_error_trap+0x1de/0x490 arch/x86/kernel/traps.c:296 do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:315 invalid_op+0x1b/0x40 arch/x86/entry/entry_64.S:991 RIP: 0010:notify_change+0xd94/0x10c0 fs/attr.c:213 RSP: 0018:8801ad76f860 EFLAGS: 00010293 RAX: 8801d9f66680 RBX: 8801d32d4630 RCX: 81c36578 RDX: RSI: 81c371b4 RDI: 0007 RBP: 8801ad76f930 R08: 8801d9f66680 R09: ed003a65a8e2 R10: ed003a65a8e2 R11: 8801d32d4717 R12: 4200 R13: 8801ad76f970 R14: 8801d5260ce0 R15: 110035aedf18 __remove_privs fs/inode.c:1813 [inline] file_remove_privs+0x2bf/0x530 fs/inode.c:1835 __generic_file_write_iter+0x169/0x5e0 mm/filemap.c:3209 blkdev_write_iter+0x233/0x420 fs/block_dev.c:1908 call_write_iter include/linux/fs.h:1782 [inline] new_sync_write fs/read_write.c:474 [inline] __vfs_write+0x5bc/0x880 fs/read_write.c:487 vfs_write+0x1f8/0x560 fs/read_write.c:549 ksys_write+0xf9/0x250 fs/read_write.c:598 SYSC_write fs/read_write.c:610 [inline] SyS_write+0x24/0x30 fs/read_write.c:607 do_syscall_64+0x29e/0x9d0 arch/x86/entry/common.c:287 entry_SYSCALL_64_after_hwframe+0x42/0xb7 RIP: 0033:0x445c59 RSP: 002b:7f76203b7d38 EFLAGS: 0293 ORIG_RAX: 0001 RAX: ffda RBX: 006dac24 RCX: 00445c59 RDX: 0050 RSI: 2400 RDI: 0008 RBP: 0005 R08: R09: R10: R11: 0293 R12: 006dac20 R13: 0006 R14: 0030656c69662f2e R15: Dumping ftrace buffer: (ftrace buffer empty) Kernel Offset: disabled Rebooting in 86400 seconds.. --- 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.
[PATCH] HID: i2c-hid: Fix resume issue on Raydium touchscreen device
When Rayd touchscreen resumed from S3, it issues too many errors like: i2c_hid i2c-RAYD0001:00: i2c_hid_get_input: incomplete report (58/5442) And all the report data are corrupted, touchscreen is unresponsive. Fix this by re-sending report description command after resume. Add device ID as a quirk. Cc: sta...@vger.kernel.org Signed-off-by: Aaron Ma --- drivers/hid/hid-ids.h | 3 +++ drivers/hid/i2c-hid/i2c-hid.c | 13 + 2 files changed, 16 insertions(+) diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h index 5a3a7ead3012..0b5cc910f62e 100644 --- a/drivers/hid/hid-ids.h +++ b/drivers/hid/hid-ids.h @@ -525,6 +525,9 @@ #define I2C_VENDOR_ID_HANTICK 0x0911 #define I2C_PRODUCT_ID_HANTICK_52880x5288 +#define I2C_VENDOR_ID_RAYD 0x2386 +#define I2C_PRODUCT_ID_RAYD_3118 0x3118 + #define USB_VENDOR_ID_HANWANG 0x0b57 #define USB_DEVICE_ID_HANWANG_TABLET_FIRST 0x5000 #define USB_DEVICE_ID_HANWANG_TABLET_LAST 0x8fff diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c index 97689e98e53f..615a91ac93bd 100644 --- a/drivers/hid/i2c-hid/i2c-hid.c +++ b/drivers/hid/i2c-hid/i2c-hid.c @@ -47,6 +47,7 @@ /* quirks to control the device */ #define I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV BIT(0) #define I2C_HID_QUIRK_NO_IRQ_AFTER_RESET BIT(1) +#define I2C_HID_QUIRK_RESEND_REPORT_DESCR BIT(2) /* flags */ #define I2C_HID_STARTED0 @@ -171,6 +172,8 @@ static const struct i2c_hid_quirks { I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV }, { I2C_VENDOR_ID_HANTICK, I2C_PRODUCT_ID_HANTICK_5288, I2C_HID_QUIRK_NO_IRQ_AFTER_RESET }, + { I2C_VENDOR_ID_RAYD, I2C_PRODUCT_ID_RAYD_3118, + I2C_HID_QUIRK_RESEND_REPORT_DESCR }, { 0, 0 } }; @@ -1220,6 +1223,16 @@ static int i2c_hid_resume(struct device *dev) if (ret) return ret; + /* RAYDIUM device (2386:3118) need to re-send report descr cmd +* after resume, after this it will be back normal. +* otherwise it issues too many incomplete reports. +*/ + if (ihid->quirks & I2C_HID_QUIRK_RESEND_REPORT_DESCR) { + ret = i2c_hid_command(client, &hid_report_descr_cmd, NULL, 0); + if (!ret) + return ret; + } + if (hid->driver && hid->driver->reset_resume) { ret = hid->driver->reset_resume(hid); return ret; -- 2.14.3
Re: Race-free unlinking of directory entries
Hi! I would like to remind this my older email about race free unlinking. Is there any plan to provide such support? On Wednesday 20 December 2017 20:18:44 Pali Rohár wrote: > Hi! > > Linux kernel currently does not provide any race-free way for calling > unlink() syscall on file entry which points to opened file descriptor. > > On the other hand Linux kernel already provides race-free way for > creating file entry by linkat() syscall with AT_EMPTY_PATH or > AT_SYMLINK_FOLLOW flags. unlinkat() does not. > > There was already discussion about unlink issue in bugzilla: > https://bugzilla.kernel.org/show_bug.cgi?id=93441 > > Because file descriptor describes inode number which can be stored in > more directories as hard links, there is a proposed funlinkat() syscall > with following API: > > int funlinkat(int fd, int dirfd, const char *pathname, int flags); > > It should atomically check if file descriptor fd and pathname (according > to dirfd) are same, and if then just unlinkat(dirfd, pathname, flags). > If are not same, throw error. > > What userspace application basically needs: > > Open file, test it stat (or probably content) and based on test result > decide if file needs to be removed or not. > > Or delete a file behind a file descriptor opened with O_PATH. > > Both cases are currently not possible without introducing race condition > between open/stat and unlink. Between those two calls, some other > process can exchange files. -- Pali Rohár pali.ro...@gmail.com
[PATCH] drivers/of: Introduce ARCH_HAS_OWN_OF_NUMA
Some OF platforms (pseries and some SPARC systems) has their own implementations of NUMA affinity detection rather than using the generic OF_NUMA driver, which mainly exists for arm64. For other platforms one of two fallbacks provided by the base OF driver are used depending on CONFIG_NUMA. In the CONFIG_NUMA=n case the fallback is an inline function in of.h. In the =y case the fallback is a real function which is defined as a weak symbol so that it may be overwritten by the architecture if desired. The problem with this arrangement is that the real implementations all export of_node_to_nid(). Unfortunately it's not possible to export the fallback since it would clash with the non-weak version. As a result we get build failures when: a) CONFIG_NUMA=y && CONFIG_OF=y, and b) The platform doesn't implement of_node_to_nid(), and c) A module uses of_node_to_nid() Given b) will be true for most platforms this is fairly easy to hit and has been observed on ia64 and x86. This patch remedies the problem by introducing the ARCH_HAS_OWN_OF_NUMA Kconfig option which is selected if an architecture provides an implementation of of_node_to_nid(). If a platform does not use it's own, or the generic OF_NUMA, then always use the inline fallback in of.h so we don't need to futz around with exports. Cc: devicet...@vger.kernel.org Cc: sparcli...@vger.kernel.org Cc: linuxppc-...@lists.ozlabs.org Fixes: 298535c00a2c ("of, numa: Add NUMA of binding implementation.") Signed-off-by: Oliver O'Halloran --- arch/powerpc/Kconfig | 1 + arch/sparc/Kconfig | 1 + drivers/of/Kconfig | 3 +++ drivers/of/base.c| 7 --- include/linux/of.h | 2 +- 5 files changed, 6 insertions(+), 8 deletions(-) diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index c32a181a7cbb..74ce5f3564ae 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -625,6 +625,7 @@ config NUMA bool "NUMA support" depends on PPC64 default y if SMP && PPC_PSERIES + select ARCH_HAS_OWN_OF_NUMA config NODES_SHIFT int diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig index 8767e45f1b2b..f8071f1c3edb 100644 --- a/arch/sparc/Kconfig +++ b/arch/sparc/Kconfig @@ -299,6 +299,7 @@ config GENERIC_LOCKBREAK config NUMA bool "NUMA support" depends on SPARC64 && SMP + select ARCH_HAS_OWN_OF_NUMA config NODES_SHIFT int "Maximum NUMA Nodes (as a power of 2)" diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig index ad3fcad4d75b..01c62b747b25 100644 --- a/drivers/of/Kconfig +++ b/drivers/of/Kconfig @@ -103,4 +103,7 @@ config OF_OVERLAY config OF_NUMA bool +config ARCH_HAS_OWN_OF_NUMA + bool + endif # OF diff --git a/drivers/of/base.c b/drivers/of/base.c index 848f549164cd..82a9584bb0e2 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -84,13 +84,6 @@ int of_n_size_cells(struct device_node *np) } EXPORT_SYMBOL(of_n_size_cells); -#ifdef CONFIG_NUMA -int __weak of_node_to_nid(struct device_node *np) -{ - return NUMA_NO_NODE; -} -#endif - static struct device_node **phandle_cache; static u32 phandle_cache_mask; diff --git a/include/linux/of.h b/include/linux/of.h index 4d25e4f952d9..9bb42dac5e65 100644 --- a/include/linux/of.h +++ b/include/linux/of.h @@ -942,7 +942,7 @@ static inline int of_cpu_node_to_id(struct device_node *np) #define of_node_cmp(s1, s2)strcasecmp((s1), (s2)) #endif -#if defined(CONFIG_OF) && defined(CONFIG_NUMA) +#if defined(CONFIG_OF_NUMA) || defined(CONFIG_ARCH_HAS_OWN_OF_NUMA) extern int of_node_to_nid(struct device_node *np); #else static inline int of_node_to_nid(struct device_node *device) -- 2.9.5
Re: [PATCH] AF_ALG: register completely initialized request in list
On Sun, Apr 8, 2018 at 7:57 PM, Stephan Müller wrote: > Hi, > > May I ask to check whether this patch fixes the issue? I cannot re-create > the issue with the reproducter. Yet, as far as I understand, you try to > induce errors which shall validate whether the error code paths are correct. You can ask syzbot to test by replying to its report email with a test command, see: https://github.com/google/syzkaller/blob/master/docs/syzbot.md#communication-with-syzbot Note that all testing of KMSAN bugs needs to go to KMSAN tree, for details see: https://github.com/google/syzkaller/blob/master/docs/syzbot.md#kmsan-bugs > The fix below should ensure this now. > > Thanks a lot. > > ---8<--- > > From 8f083e7b0684a9f91c186d7b46eec34e439689c3 Mon Sep 17 00:00:00 2001 > From: Stephan Mueller > Date: Sun, 8 Apr 2018 19:53:59 +0200 > Subject: [PATCH] AF_ALG: Initialize sg_num_bytes in error code path > > The RX SGL in processing is already registered with the RX SGL tracking > list to support proper cleanup. The cleanup code path uses the > sg_num_bytes variable which must therefore be always initialized, even > in the error code path. > > Signed-off-by: Stephan Mueller > Reported-by: syzbot+9c251bdd09f83b92b...@syzkaller.appspotmail.com > --- > crypto/af_alg.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/crypto/af_alg.c b/crypto/af_alg.c > index c49766b03165..0d555c072669 100644 > --- a/crypto/af_alg.c > +++ b/crypto/af_alg.c > @@ -1156,8 +1156,10 @@ int af_alg_get_rsgl(struct sock *sk, struct msghdr > *msg, int flags, > > /* make one iovec available as scatterlist */ > err = af_alg_make_sg(&rsgl->sgl, &msg->msg_iter, seglen); > - if (err < 0) > + if (err < 0) { > + rsgl->sg_num_bytes = 0; > return err; > + } > > /* chain the new scatterlist with previous one */ > if (areq->last_rsgl) > -- > 2.14.3 > > > > > > -- > You received this message because you are subscribed to the Google Groups > "syzkaller-bugs" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to syzkaller-bugs+unsubscr...@googlegroups.com. > To view this discussion on the web visit > https://groups.google.com/d/msgid/syzkaller-bugs/3337259.MW9pfDCdka%40positron.chronox.de. > For more options, visit https://groups.google.com/d/optout.
Re: [PATCH] AF_ALG: register completely initialized request in list
Am Montag, 9. April 2018, 09:51:13 CEST schrieb Dmitry Vyukov: Hi Dmitry, > You can ask syzbot to test by replying to its report email with a test > command, see: > https://github.com/google/syzkaller/blob/master/docs/syzbot.md#communication > -with-syzbot > > Note that all testing of KMSAN bugs needs to go to KMSAN tree, for details > see: > https://github.com/google/syzkaller/blob/master/docs/syzbot.md#kmsan-bugs Thank you. I will resend the patch later today with the proper tags. Ciao Stephan
Re: [PATCH] drm: clarify adjusted_mode for a bridge connected to a crtc
On Fri, Apr 06, 2018 at 03:28:27PM +, Philippe CORNU wrote: > Hi Laurent, > > On 04/06/2018 04:53 PM, Laurent Pinchart wrote: > > Hi Philippe, > > > > Thank you for the patch. > > > > On Monday, 26 February 2018 14:16:04 EEST Philippe Cornu wrote: > >> This patch clarifies the adjusted_mode documentation > >> for a bridge directly connected to a crtc. > >> > >> Signed-off-by: Philippe Cornu > >> --- > >> This patch is linked to the discussion https://lkml.org/lkml/2018/1/25/367 > >> > >> include/drm/drm_bridge.h | 3 ++- > >> 1 file changed, 2 insertions(+), 1 deletion(-) > >> > >> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h > >> index 3270fec46979..b5f3c070467c 100644 > >> --- a/include/drm/drm_bridge.h > >> +++ b/include/drm/drm_bridge.h > >> @@ -177,7 +177,8 @@ struct drm_bridge_funcs { > >> * pipeline has been called already. If the bridge is the first element > >> * then this would be &drm_encoder_helper_funcs.mode_set. The display > >> * pipe (i.e. clocks and timing signals) is off when this function is > >> - * called. > >> + * called. If the bridge is connected to the crtc, the adjusted_mode > >> + * parameter is the one defined in &drm_crtc_state.adjusted_mode. > > > > Unless I'm mistaken this will always be the mode stored in > > &drm_crtc_state.adjusted_mode (at least for atomic drivers), regardless of > > whether the bridge is the first in the chain (connected to the CRTC) or not. > > What is important to document is that we have a single adjusted_mode for the > > whole chain of bridges, and that it corresponds to the mode output by the > > CRTC > > for the first bridge. Bridges further in the chain can look at that mode, > > although there will probably be nothing of interest to them there. > > > > How about the following text ? > > > > /** > > * @mode_set: > > * > > * This callback should set the given mode on the bridge. It is called > > * after the @mode_set callback for the preceding element in the > > display > > * pipeline has been called already. If the bridge is the first element > > * then this would be &drm_encoder_helper_funcs.mode_set. The display > > * pipe (i.e. clocks and timing signals) is off when this function is > > * called. > > * > > * The adjusted_mode parameter corresponds to the mode output by the > > CRTC > > * for the first bridge in the chain. It can be different from the mode > > * parameter that contains the desired mode for the connector at the > > end > > * of the bridges chain, for instance when the first bridge in the > > chain > > * performs scaling. The adjusted mode is mostly useful for the first > > * bridge in the chain and is likely irrelevant for the other bridges. > > * > > * For atomic drivers the adjusted_mode is the mode stored in > > * &drm_crtc_state.adjusted_mode. > > * > > * NOTE: > > * > > * If a need arises to store and access modes adjusted for other > > locations > > * than the connection between the CRTC and the first bridge, the DRM > > * framework will have to be extended with DRM bridge states. > > */ > > > > Then I think we should also update the documentation of > > drm_crtc_state.adjusted_mode accordingly: > > > > /* > > * @adjusted_mode: > > * > > * Internal display timings which can be used by the driver to handle > > * differences between the mode requested by userspace in @mode and > > what > > * is actually programmed into the hardware. > > * > > * For drivers using drm_bridge, this store the hardware display > > timings > > * used between the CRTC and the first bridge. For other drivers, the > > * meaning of the adjusted_mode field is purely driver implementation > > * defined information, and will usually be used to store the hardware > > * display timings used between the CRTC and encoder blocks. > > */ > > > > Your proposal is very clear and understandable. I will make a new patch > version based on it. Just to avoid confusion: Needs to be a fully new patch on top of latest drm-misc-next, since no rebasing in a group maintained tree. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH 3/7] PM / Domain: Add struct device to genpd
On 22-03-18, 11:18, Ulf Hansson wrote: > On 22 March 2018 at 10:59, Viresh Kumar wrote: > > On 22-03-18, 10:30, Ulf Hansson wrote: > >> On 22 December 2017 at 08:26, Viresh Kumar wrote: > >> > + ret = device_add(&genpd->dev); > >> > >> What's the point of adding the device? Can we skip this step, as I > >> guess the opp core is only using the device as cookie rather actual > >> using it? No? > > > > We also use it for the OPP debugfs stuff, so that would be required I > > believe. > > Right, however, isn't that only using the dev_name(dev), which you > don't need to add the device to make use of. > > Or maybe I missing something around this... So I tested this bit. The code works fine even if the device isn't added (registered), but this looks a bit sloppy to attempt. Please let me know what would you prefer in this case, add the device or not. -- viresh
Re: [PATCH] crypto: DRBG - guard uninstantion by lock
On Mon, Apr 9, 2018 at 7:40 AM, Stephan Mueller wrote: > Am Montag, 9. April 2018, 00:46:03 CEST schrieb Theodore Y. Ts'o: > > Hi Theodore, >> >> So the syzbot will run while the patch goes through the normal e-mail >> review process, which is kind of neat. :-) > > Thank you very much for the hint. That is a neat feature indeed. > > As I came late to the party and I missed the original mails, I am wondering > about which GIT repo was used and which branch of it. With that, I would be > happy to resubmit with the test line. All syzbot reported bugs are available here: https://groups.google.com/forum/#!searchin/syzkaller-bugs/"WARNING$20in$20kmem_cache_free"; and here: https://syzkaller.appspot.com/ But unfortunately testing won't work in this case, because I manually extracted a reproducer and syzbot does not know about it. This bug seems to lead to assorted silent heap corruptions and different manifestations each time, so it's difficult for syzbot to attribute a reproducer to the bug. When we debug it, it would be nice to understand why the heap corruption is silent and is not detected by KASAN and anything else, to prevent such unpleasant cases in future. I've tested it manually, but unfortunately kernel still crashed within a minute: $ git status HEAD detached at f2d285669aae Changes not staged for commit: (use "git add ..." to update what will be committed) (use "git checkout -- ..." to discard changes in working directory) modified: crypto/drbg.c $ git diff diff --git a/crypto/drbg.c b/crypto/drbg.c index 4faa2781c964..68c1949a253f 100644 --- a/crypto/drbg.c +++ b/crypto/drbg.c @@ -1510,8 +1510,8 @@ static int drbg_instantiate(struct drbg_state *drbg, struct drbg_string *pers, return ret; free_everything: - mutex_unlock(&drbg->drbg_mutex); drbg_uninstantiate(drbg); + mutex_unlock(&drbg->drbg_mutex); return ret; } # ./a.out ... [ 183.647874] FAULT_INJECTION: forcing a failure. [ 183.647874] name failslab, interval 1, probability 0, space 0, times 0 [ 183.648287] Call Trace: [ 183.648297] dump_stack+0x1b9/0x29f [ 183.648306] ? arch_local_irq_restore+0x52/0x52 [ 183.648318] ? __save_stack_trace+0x7e/0xd0 [ 183.651848] should_fail.cold.4+0xa/0x1a [ 183.652411] ? fault_create_debugfs_attr+0x1f0/0x1f0 [ 183.653138] ? kasan_kmalloc+0xc4/0xe0 [ 183.653694] ? __kmalloc+0x14e/0x760 [ 183.654206] ? drbg_kcapi_seed+0x776/0x12e0 [ 183.654798] ? crypto_rng_reset+0x7c/0x130 [ 183.655379] ? rng_setkey+0x25/0x30 [ 183.655882] ? alg_setsockopt+0x306/0x3b0 [ 183.656450] ? graph_lock+0x170/0x170 [ 183.656975] ? entry_SYSENTER_compat+0x70/0x7f [ 183.657606] ? find_held_lock+0x36/0x1c0 [ 183.658164] ? __lock_is_held+0xb5/0x140 [ 183.658728] ? check_same_owner+0x320/0x320 [ 183.659321] ? rcu_note_context_switch+0x710/0x710 [ 183.66] should_failslab+0x124/0x180 [ 183.660561] __kmalloc+0x2c8/0x760 [ 183.661046] ? graph_lock+0x170/0x170 [ 183.661569] ? drbg_kcapi_seed+0x882/0x12e0 [ 183.662161] drbg_kcapi_seed+0x882/0x12e0 [ 183.662731] ? drbg_seed+0x10a0/0x10a0 [ 183.663267] ? lock_downgrade+0x8e0/0x8e0 [ 183.663833] ? lock_acquire+0x1dc/0x520 [ 183.664385] ? lock_release+0xa10/0xa10 [ 183.664934] ? check_same_owner+0x320/0x320 [ 183.665530] ? __sanitizer_cov_trace_const_cmp8+0x18/0x20 [ 183.666292] ? __check_object_size+0x95/0x5d9 [ 183.666904] ? sock_kmalloc+0x14e/0x1d0 [ 183.667444] ? mark_held_locks+0xc9/0x160 [ 183.668020] ? __might_sleep+0x95/0x190 [ 183.668567] crypto_rng_reset+0x7c/0x130 [ 183.669124] rng_setkey+0x25/0x30 [ 183.669598] ? rng_sock_destruct+0x90/0x90 [ 183.670176] alg_setsockopt+0x306/0x3b0 [ 183.670724] __compat_sys_setsockopt+0x315/0x7c0 [ 183.671375] ? __compat_sys_getsockopt+0x7f0/0x7f0 [ 183.672057] ? __sanitizer_cov_trace_const_cmp4+0x16/0x20 [ 183.672813] ? ksys_write+0x1a6/0x250 [ 183.67] ? SyS_read+0x30/0x30 [ 183.673811] compat_SyS_setsockopt+0x34/0x50 [ 183.674416] ? scm_detach_fds_compat+0x440/0x440 [ 183.675079] do_fast_syscall_32+0x41f/0x10dc [ 183.675725] ? do_page_fault+0xee/0x8a7 [ 183.676284] ? do_int80_syscall_32+0xa70/0xa70 [ 183.676925] ? trace_hardirqs_on_thunk+0x1a/0x1c [ 183.677590] ? __sanitizer_cov_trace_const_cmp4+0x16/0x20 [ 183.678348] ? syscall_return_slowpath+0x30f/0x5c0 [ 183.679026] ? sysret32_from_system_call+0x5/0x3c [ 183.679694] ? trace_hardirqs_off_thunk+0x1a/0x1c [ 183.680380] entry_SYSENTER_compat+0x70/0x7f [ 183.681000] RIP: 0023:0xf7f0ecb9 [ 183.681488] RSP: 002b:ffeb1e9c EFLAGS: 0296 ORIG_RAX: 016e [ 183.682606] RAX: ffda RBX: 0003 RCX: 0117 [ 183.683620] RDX: 0001 RSI: 205b1fd0 RDI: [ 183.684602] RBP: 2040 R08: R09: [ 183.685622] R10: R11: R12: [ 183.686642] R13: R14:
Re: [RfC PATCH] Add udmabuf misc device
On Fri, Apr 06, 2018 at 12:54:22PM +0200, Gerd Hoffmann wrote: > On Fri, Apr 06, 2018 at 10:52:21AM +0100, Daniel Stone wrote: > > Hi Gerd, > > > > On 14 March 2018 at 08:03, Gerd Hoffmann wrote: > > >> Either mlock account (because it's mlocked defacto), and get_user_pages > > >> won't do that for you. > > >> > > >> Or you write the full-blown userptr implementation, including > > >> mmu_notifier > > >> support (see i915 or amdgpu), but that also requires Christian Königs > > >> latest ->invalidate_mapping RFC for dma-buf (since atm exporting userptr > > >> buffers is a no-go). > > > > > > I guess I'll look at mlock accounting for starters then. Easier for > > > now, and leaves the door open to switch to userptr later as this should > > > be transparent to userspace. > > > > Out of interest, do you have usecases for full userptr support? Maybe > > another way would be to allow creation of dmabufs from memfds. > > I have two things in mind. > > One is vga emulation. I have virtual pci memory bar for the virtual > vga. qemu backs vga memory with anonymous pages right now, switching > that to shmem should be easy though if that makes things easier. Guest > places the framebuffer somewhere in the pci bar, and I want export the > chunk which represents the framebuffer as dma-buf to display it on the > host without copying around data. Framebuffer is linear in guest > physical memory, so a single block only. That is the simpler case. > > The more difficuilt one is virtio-gpu ressources. virtio-gpu resources > live in host memory (guest has no direct access). The guest can > optionally specify guest memory pages as backing storage for the > resource. Guest backing storage is allowed to be scattered. Commands > exist to copy both ways between host storage and guest backing. > > With virgl (opengl acceleration) enabled the guest will send rendering > commands to fill the framebuffer ressource, so there is no need to copy > content to the framebuffer ressource. The guest may fill other > resources such as textures used for rendering with copy commands. > > Without acceleration the guest does software-rendering to the backing > storage, then sends a command to copy the framebuffer content from guest > backing storage to host ressource. > > Now it would be useful to allow a shared mapping, so no copying between > guest backing storage and host resource is needed, especially for the > software rendering case (i.e. dumb gem buffers). Being able to export > guest dumb buffers to other host processes would be useful too, for > example to display guest windows seamlessly on the host wayland server. > > So getting a dma-buf for the guest backing storage via udmabuf looked > like a useful approach. We can export the guest gem buffers to other > host processes that way. qemu itself could map it too, to get a linear > representation of the scattered guest backing storage. > > The other obvious approach would be to do it the other way around and > allow the guest map the host resource somehow. On the host side qemu > could use vgem to allocate resource memory, so it'll be a gem object > already. Mapping that into the guest isn't that straight-forward > though. The guest manages its physical address space, so the guest > would need to find a free spot and ask the host to place the resource > there. Then the guest needs page structs covering the mapped resource, > so it can work with it. Didn't investigate how difficuilt that is. Use > memory hotplug maybe? Can we easily unmap the resource then? Also I > think updating the guests physical memory layout (which we would need to > do on every resource map/unmap) isn't an exactly cheap operation ... Generally we try to cache mappings as much as possible. And wrt finding a slot: Create a sufficiently sized BAR on the virgl device, just for that? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH v3 4/4] zram: introduce zram memory tracking
On Mon, Apr 09, 2018 at 02:54:35PM +0900, Minchan Kim wrote: > zRam as swap is useful for small memory device. However, swap means > those pages on zram are mostly cold pages due to VM's LRU algorithm. > Especially, once init data for application are touched for launching, > they tend to be not accessed any more and finally swapped out. > zRAM can store such cold pages as compressed form but it's pointless > to keep in memory. Better idea is app developers free them directly > rather than remaining them on heap. > > This patch tell us last access time of each block of zram via > "cat /sys/kernel/debug/zram/zram0/block_state". > > The output is as follows, > 30075.033841 .wh > 30163.806904 s.. > 30263.806919 ..h > > First column is zram's block index and 3rh one represents symbol > (s: same page w: written page to backing store h: huge page) of the > block state. Second column represents usec time unit of the block > was last accessed. So above example means the 300th block is accessed > at 75.033851 second and it was huge so it was written to the backing > store. > > Admin can leverage this information to catch cold|incompressible pages > of process with *pagemap* once part of heaps are swapped out. > > Cc: Greg KH > Signed-off-by: Minchan Kim > --- > Documentation/blockdev/zram.txt | 24 ++ > drivers/block/zram/Kconfig | 9 +++ > drivers/block/zram/zram_drv.c | 139 +--- > drivers/block/zram/zram_drv.h | 5 ++ > 4 files changed, 166 insertions(+), 11 deletions(-) > > diff --git a/Documentation/blockdev/zram.txt b/Documentation/blockdev/zram.txt > index 78db38d02bc9..45509c7d5716 100644 > --- a/Documentation/blockdev/zram.txt > +++ b/Documentation/blockdev/zram.txt > @@ -243,5 +243,29 @@ to backing storage rather than keeping it in memory. > User should set up backing device via /sys/block/zramX/backing_dev > before disksize setting. > > += memory tracking > + > +With CONFIG_ZRAM_MEMORY_TRACKING, user can know information of the > +zram block. It could be useful to catch cold or incompressible > +pages of the proess with*pagemap. > +If you enable the feature, you could see block state via > +/sys/kernel/debug/zram/zram0/block_state". The output is as follows, > + > + 30075.033841 .wh > + 30163.806904 s.. > + 30263.806919 ..h > + > +First column is zram's block index. > +Second column is access time. > +Third column is state of the block. > +(s: same page > +w: written page to backing store > +h: huge page) > + > +First line of above example says 300th block is accessed at 75.033841sec > +and the block's state is huge so it is written back to the backing > +storage. It's a debugging feature so anyone shouldn't rely on it to work > +properly. > + > Nitin Gupta > ngu...@vflare.org > diff --git a/drivers/block/zram/Kconfig b/drivers/block/zram/Kconfig > index ac3a31d433b2..efe60c82d8ec 100644 > --- a/drivers/block/zram/Kconfig > +++ b/drivers/block/zram/Kconfig > @@ -26,3 +26,12 @@ config ZRAM_WRITEBACK >/sys/block/zramX/backing_dev. > >See zram.txt for more infomration. > + > +config ZRAM_MEMORY_TRACKING > + bool "Tracking zram block status" > + depends on ZRAM > + select DEBUG_FS Select? Shouldn't you depend on this instead? Selecting is a pain to try to track down what is keeping an option enabled. > + help > + With this feature, admin can track the state of allocated block > + of zRAM. Admin could see the information via > + /sys/kernel/debug/zram/zramX/block_state. A short note here as to where to find the documentation for what that info is (i.e. in the file you wrote above?) > +#else > +static void zram_debugfs_create(void) {}; > +static void zram_debugfs_destroy(void) {}; > +static void zram_accessed(struct zram *zram, u32 index) {}; > +static void zram_reset_access(struct zram *zram, u32 index) {}; > +static void zram_debugfs_register(struct zram *zram) {}; > +static void zram_debugfs_unregister(struct zram *zram) {}; > +#endif Much nicer, thanks! The above was only very minor nits, no need to change anything if you don't want to. Acked-by: Greg Kroah-Hartman
[PATCH] usb: typec: ucsi: fix tracepoint related build error
The ucsi driver defines several tracepoints, but the header file with the tracepoint definition trace.h is only conditionally built depending on CONFIG_FTRACE. This leads to the following build error with CONFIG_FTRACE=n and CONFIG_TYPEC_UCSI=m: ERROR: "__tracepoint_ucsi_command" [drivers/usb/typec/ucsi/typec_ucsi.ko] undefined! ERROR: "__tracepoint_ucsi_register_port" [drivers/usb/typec/ucsi/typec_ucsi.ko] undefined! ERROR: "__tracepoint_ucsi_notify" [drivers/usb/typec/ucsi/typec_ucsi.ko] undefined! ERROR: "__tracepoint_ucsi_reset_ppm" [drivers/usb/typec/ucsi/typec_ucsi.ko] undefined! ERROR: "__tracepoint_ucsi_run_command" [drivers/usb/typec/ucsi/typec_ucsi.ko] undefined! ERROR: "__tracepoint_ucsi_ack" [drivers/usb/typec/ucsi/typec_ucsi.ko] undefined! ERROR: "__tracepoint_ucsi_connector_change" [drivers/usb/typec/ucsi/typec_ucsi.ko] undefined! With CONFIG_TYPEC_UCSI=y the build fails with several link errors. Fix this by changing the Makefile to unconditionally build trace.o. Tracepints are a runtime contruct and no other user of tracepoints depends on CONFIG_FTRACE. Fixes: c1b0bc2dabfa ("usb: typec: Add support for UCSI interface") Signed-off-by: Tobias Regnery --- drivers/usb/typec/ucsi/Makefile | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/usb/typec/ucsi/Makefile b/drivers/usb/typec/ucsi/Makefile index b57891c1fd31..971befbbc2a8 100644 --- a/drivers/usb/typec/ucsi/Makefile +++ b/drivers/usb/typec/ucsi/Makefile @@ -3,8 +3,6 @@ CFLAGS_trace.o := -I$(src) obj-$(CONFIG_TYPEC_UCSI) += typec_ucsi.o -typec_ucsi-y := ucsi.o - -typec_ucsi-$(CONFIG_FTRACE)+= trace.o +typec_ucsi-y := ucsi.o trace.o obj-$(CONFIG_UCSI_ACPI)+= ucsi_acpi.o -- 2.16.3
Re: [PATCH v3 3/5] platform/x86: Rename silead_dmi to touchscreen_dmi
On Sun, Apr 8, 2018 at 8:40 PM, Hans de Goede wrote: > Not only silead touchscreens need some extra info not available in the > ACPI tables to work properly. X86 devices with a Chipone ICN8505 chip also > need some DMI based extra configuration. > > There is no reason to have separate dmi config code per touchscreen > controller vendor. This commit renames silead_dmi to a more generic > touchscreen_dmi name (and Kconfig option) in preparation of adding > info for tablets with an ICN8505 based touchscreen. > > Note there are no functional changes all code changes are limited to > removing references to silead where these are no longer applicable. > I have no objections from my side, though consider the following: - I would like to be in sync with Darren on this - make oldconfig will be broken after your change for existing users - the usual pattern in kernel that we don't rename drivers; I guess here we are on the safe side b/c this driver is used standalone Taking above into attention, and assuming it will go via some other tree, Acked-by: Andy Shevchenko > Signed-off-by: Hans de Goede > --- > MAINTAINERS | 2 +- > drivers/platform/x86/Kconfig | 16 ++--- > drivers/platform/x86/Makefile | 2 +- > .../x86/{silead_dmi.c => touchscreen_dmi.c} | 66 +-- > 4 files changed, 43 insertions(+), 43 deletions(-) > rename drivers/platform/x86/{silead_dmi.c => touchscreen_dmi.c} (87%) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 0d5c55daeeba..99dd47e3b0dd 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -12618,7 +12618,7 @@ L: linux-in...@vger.kernel.org > L: platform-driver-...@vger.kernel.org > S: Maintained > F: drivers/input/touchscreen/silead.c > -F: drivers/platform/x86/silead_dmi.c > +F: drivers/platform/x86/touchscreen_dmi.c > > SILICON MOTION SM712 FRAME BUFFER DRIVER > M: Sudip Mukherjee > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig > index 1868aab0282a..b836576f0fe4 100644 > --- a/drivers/platform/x86/Kconfig > +++ b/drivers/platform/x86/Kconfig > @@ -1194,16 +1194,16 @@ config INTEL_TURBO_MAX_3 > This driver is only required when the system is not using Hardware > P-States (HWP). In HWP mode, priority can be read from ACPI tables. > > -config SILEAD_DMI > - bool "Tablets with Silead touchscreens" > +config TOUCHSCREEN_DMI > + bool "DMI based touchscreen configuration info" > depends on ACPI && DMI && I2C=y && TOUCHSCREEN_SILEAD > ---help--- > - Certain ACPI based tablets with Silead touchscreens do not have > - enough data in ACPI tables for the touchscreen driver to handle > - the touchscreen properly, as OEMs expected the data to be baked > - into the tablet model specific version of the driver shipped > - with the OS-image for the device. This option supplies the missing > - information. Enable this for x86 tablets with Silead touchscreens. > + Certain ACPI based tablets with e.g. Silead or Chipone touchscreens > + do not have enough data in ACPI tables for the touchscreen driver to > + handle the touchscreen properly, as OEMs expect the data to be baked > + into the tablet model specific version of the driver shipped with > the > + the OS-image for the device. This option supplies the missing info. > + Enable this for x86 tablets with Silead or Chipone touchscreens. > > config INTEL_CHTDC_TI_PWRBTN > tristate "Intel Cherry Trail Dollar Cove TI power button driver" > diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile > index 2ba6cb795338..8d9477114fb5 100644 > --- a/drivers/platform/x86/Makefile > +++ b/drivers/platform/x86/Makefile > @@ -78,7 +78,7 @@ obj-$(CONFIG_INTEL_SMARTCONNECT) += > intel-smartconnect.o > obj-$(CONFIG_PVPANIC) += pvpanic.o > obj-$(CONFIG_ALIENWARE_WMI)+= alienware-wmi.o > obj-$(CONFIG_INTEL_PMC_IPC)+= intel_pmc_ipc.o > -obj-$(CONFIG_SILEAD_DMI) += silead_dmi.o > +obj-$(CONFIG_TOUCHSCREEN_DMI) += touchscreen_dmi.o > obj-$(CONFIG_SURFACE_PRO3_BUTTON) += surfacepro3_button.o > obj-$(CONFIG_SURFACE_3_BUTTON) += surface3_button.o > obj-$(CONFIG_INTEL_PUNIT_IPC) += intel_punit_ipc.o > diff --git a/drivers/platform/x86/silead_dmi.c > b/drivers/platform/x86/touchscreen_dmi.c > similarity index 87% > rename from drivers/platform/x86/silead_dmi.c > rename to drivers/platform/x86/touchscreen_dmi.c > index 452aacabaa8e..87fc839b28f7 100644 > --- a/drivers/platform/x86/silead_dmi.c > +++ b/drivers/platform/x86/touchscreen_dmi.c > @@ -1,5 +1,5 @@ > /* > - * Silead touchscreen driver DMI based configuration code > + * Touchscreen driver DMI based configuration code > * > * Copyright (c) 2017 Red Hat Inc. > * > @@ -20,7 +20,7 @@ > #include > #include > > -struct silead_ts_dmi_data { > +struct ts_dmi_dat
Re: [PATCH v2 4/5] platform/x86: touchscreen_dmi: Add EFI embedded firmware info support
On Sun, Apr 8, 2018 at 8:26 PM, Hans de Goede wrote: > Sofar we have been unable to get permission from the vendors to put the > firmware for touchscreens listed in touchscreen_dmi in linux-firmware. > > Some of the tablets with such a touchscreen have a touchscreen driver, and > thus a copy of the firmware, as part of their EFI code. > > This commit adds the necessary info for the new EFI embedded-firmware code > to extract these firmwares, making the touchscreen work OOTB without the > user needing to manually add the firmware. > Acked-by: Andy Shevchenko for PDx86 bits only. > Signed-off-by: Hans de Goede > --- > drivers/firmware/efi/embedded-firmware.c | 3 +++ > drivers/platform/x86/Kconfig | 1 + > drivers/platform/x86/touchscreen_dmi.c | 26 +++- > include/linux/efi_embedded_fw.h | 2 ++ > 4 files changed, 31 insertions(+), 1 deletion(-) > > diff --git a/drivers/firmware/efi/embedded-firmware.c > b/drivers/firmware/efi/embedded-firmware.c > index cb57225a340d..26101ac1a282 100644 > --- a/drivers/firmware/efi/embedded-firmware.c > +++ b/drivers/firmware/efi/embedded-firmware.c > @@ -23,6 +23,9 @@ struct embedded_fw { > static LIST_HEAD(found_fw_list); > > static const struct dmi_system_id * const embedded_fw_table[] = { > +#ifdef CONFIG_TOUCHSCREEN_DMI > + touchscreen_dmi_table, > +#endif > NULL > }; > > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig > index b836576f0fe4..5bb0f5edd7f2 100644 > --- a/drivers/platform/x86/Kconfig > +++ b/drivers/platform/x86/Kconfig > @@ -1197,6 +1197,7 @@ config INTEL_TURBO_MAX_3 > config TOUCHSCREEN_DMI > bool "DMI based touchscreen configuration info" > depends on ACPI && DMI && I2C=y && TOUCHSCREEN_SILEAD > + select EFI_EMBEDDED_FIRMWARE if EFI_STUB > ---help--- > Certain ACPI based tablets with e.g. Silead or Chipone touchscreens > do not have enough data in ACPI tables for the touchscreen driver to > diff --git a/drivers/platform/x86/touchscreen_dmi.c > b/drivers/platform/x86/touchscreen_dmi.c > index 87fc839b28f7..6488cd50ba79 100644 > --- a/drivers/platform/x86/touchscreen_dmi.c > +++ b/drivers/platform/x86/touchscreen_dmi.c > @@ -15,12 +15,15 @@ > #include > #include > #include > +#include > #include > #include > #include > #include > > struct ts_dmi_data { > + /* The EFI embedded-fw code expects this to be the first member! */ > + struct efi_embedded_fw_desc embedded_fw; > const char *acpi_name; > const struct property_entry *properties; > }; > @@ -31,10 +34,17 @@ static const struct property_entry > cube_iwork8_air_props[] = { > PROPERTY_ENTRY_BOOL("touchscreen-swapped-x-y"), > PROPERTY_ENTRY_STRING("firmware-name", "gsl3670-cube-iwork8-air.fw"), > PROPERTY_ENTRY_U32("silead,max-fingers", 10), > + PROPERTY_ENTRY_BOOL("efi-embedded-firmware"), > { } > }; > > static const struct ts_dmi_data cube_iwork8_air_data = { > + .embedded_fw = { > + .name = "silead/gsl3670-cube-iwork8-air.fw", > + .prefix = { 0xf0, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00 }, > + .length = 38808, > + .crc= 0xfecde51f, > + }, > .acpi_name = "MSSL1680:00", > .properties = cube_iwork8_air_props, > }; > @@ -119,10 +129,17 @@ static const struct property_entry pipo_w2s_props[] = { > PROPERTY_ENTRY_BOOL("touchscreen-swapped-x-y"), > PROPERTY_ENTRY_STRING("firmware-name", > "gsl1680-pipo-w2s.fw"), > + PROPERTY_ENTRY_BOOL("efi-embedded-firmware"), > { } > }; > > static const struct ts_dmi_data pipo_w2s_data = { > + .embedded_fw = { > + .name = "silead/gsl1680-pipo-w2s.fw", > + .prefix = { 0xf0, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00 }, > + .length = 39072, > + .crc= 0x28d5dc6c, > + }, > .acpi_name = "MSSL1680:00", > .properties = pipo_w2s_props, > }; > @@ -162,10 +179,17 @@ static const struct property_entry > chuwi_hi8_pro_props[] = { > PROPERTY_ENTRY_BOOL("touchscreen-swapped-x-y"), > PROPERTY_ENTRY_STRING("firmware-name", "gsl3680-chuwi-hi8-pro.fw"), > PROPERTY_ENTRY_BOOL("silead,home-button"), > + PROPERTY_ENTRY_BOOL("efi-embedded-firmware"), > { } > }; > > static const struct ts_dmi_data chuwi_hi8_pro_data = { > + .embedded_fw = { > + .name = "silead/gsl3680-chuwi-hi8-pro.fw", > + .prefix = { 0xf0, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00 }, > + .length = 39864, > + .crc= 0xfe2bedba, > + }, > .acpi_name = "MSSL1680:00", > .properties = chuwi_hi8_pro_props, > }; > @@ -277,7 +301,7 @@ static const struct ts_dmi_data teclast_x3_plus_data = { > .properties
Re: [PATCH v3 5/5] platform/x86: touchscreen_dmi: Add info for the Chuwi Vi8 Plus tablet
On Sun, Apr 8, 2018 at 8:40 PM, Hans de Goede wrote: > Add touchscreen info for the Chuwi Vi8 Plus tablet. This tablet uses a > Chipone ICN8505 touchscreen controller, with the firmware used by the > touchscreen embedded in the EFI firmware. > Acked-by: Andy Shevchenko > Signed-off-by: Hans de Goede > --- > drivers/platform/x86/touchscreen_dmi.c | 25 + > 1 file changed, 25 insertions(+) > > diff --git a/drivers/platform/x86/touchscreen_dmi.c > b/drivers/platform/x86/touchscreen_dmi.c > index 6488cd50ba79..5fdb6fe878f4 100644 > --- a/drivers/platform/x86/touchscreen_dmi.c > +++ b/drivers/platform/x86/touchscreen_dmi.c > @@ -301,6 +301,22 @@ static const struct ts_dmi_data teclast_x3_plus_data = { > .properties = teclast_x3_plus_props, > }; > > +static const struct property_entry efi_embedded_fw_props[] = { > + PROPERTY_ENTRY_BOOL("efi-embedded-firmware"), > + { } > +}; > + > +static const struct ts_dmi_data chuwi_vi8_plus_data = { > + .embedded_fw = { > + .name = "chipone/icn8505-HAMP0002.fw", > + .prefix = { 0xb0, 0x07, 0x00, 0x00, 0xe4, 0x07, 0x00, 0x00 }, > + .length = 35012, > + .crc= 0x74dfd3fc, > + }, > + .acpi_name = "CHPN0001:00", > + .properties = efi_embedded_fw_props, > +}; > + > const struct dmi_system_id touchscreen_dmi_table[] = { > { > /* CUBE iwork8 Air */ > @@ -487,6 +503,15 @@ const struct dmi_system_id touchscreen_dmi_table[] = { > DMI_MATCH(DMI_PRODUCT_NAME, "Y8W81"), > }, > }, > + { > + /* Chuwi Vi8 Plus (CWI506) */ > + .driver_data = (void *)&chuwi_vi8_plus_data, > + .matches = { > + DMI_MATCH(DMI_SYS_VENDOR, "Hampoo"), > + DMI_MATCH(DMI_PRODUCT_NAME, "D2D3_Vi8A1"), > + DMI_MATCH(DMI_BOARD_NAME, "Cherry Trail CR"), > + }, > + }, > { }, > }; > > -- > 2.17.0 > -- With Best Regards, Andy Shevchenko
Re: [RfC PATCH] Add udmabuf misc device
On Thu, Apr 05, 2018 at 05:11:17PM -0700, Matt Roper wrote: > On Thu, Apr 05, 2018 at 10:32:04PM +0200, Daniel Vetter wrote: > > Pulling this out of the shadows again. > > > > We now also have xen-zcopy from Oleksandr and the hyper dmabuf stuff > > from Matt and Dongwong. > > > > At least from the intel side there seems to be the idea to just have 1 > > special device that can handle cross-gues/host sharing for all kinds > > of hypervisors, so I guess you all need to work together :-) > > > > Or we throw out the idea that hyper dmabuf will be cross-hypervisor > > (not sure how useful/reasonable that is, someone please convince me > > one way or the other). > > > > Cheers, Daniel > > Dongwon (DW) is the one doing all the real work on hyper_dmabuf, but I'm > familiar with the use cases he's trying to address, and I think there > are a couple high-level goals of his work that are worth calling out as > we discuss the various options for sharing buffers produced in one VM > with a consumer running in another VM: > > * We should try to keep the interface/usage separate from the >underlying hypervisor implementation details. I.e., in DW's design >the sink/source drivers that handle the actual buffer passing in the >two VM's should provide a generic interface that does not depend on a >specific hypervisor. Behind the scenes there could be various >implementations for specific hypervisors (Xen, KVM, ACRN, etc.), and >some of those backends may have additional restrictions, but it would >be best if userspace didn't have to know the specific hypervisor >running on the system and could just query the general capabilities >available to it. We've already got projects in flight that are >wanting this functionality on Xen and ACRN today. Two comments on this: - Just because it's in drivers/gpu doesn't mean you can't use it for anything else. E.g. the xen-zcopy driver can very much be used for any dma-buf, there's nothing gpu specific with it - well besides that it resuses some useful DRM ioctls, but if that annoys you just do a #define TOTALLY_GENERIC DRM and be done :-) - Especially the kvm memory and hypervisor model seems totally different from other hypervisors, e.g. no real use for guest-guest sharing (which doesn't go through the host) and other cases. So trying to make something 100% generic seems like a bad idea. Wrt making it generic: Just use generic interfaces - if you can somehow use xen-front for the display sharing, then a) no need for hyper-dmabuf and b) already fully generic since it looks like a normal drm device to the guest userspace. > * The general interface should be able to express sharing from any >guest:guest, not just guest:host. Arbitrary G:G sharing might be >something some hypervisors simply aren't able to support, but the >userspace API itself shouldn't make assumptions or restrict that. I >think ideally the sharing API would include some kind of >query_targets interface that would return a list of VM's that your >current OS is allowed to share with; that list would be depend on the >policy established by the system integrator, but obviously wouldn't >include targets that the hypervisor itself wouldn't be capable of >handling. Uh ... has a proper security architect analyzed this idea? > * A lot of the initial use cases are in the realm of graphics, but this >shouldn't be a graphics-specific API. Buffers might contain other >types of content as well (e.g., audio). Really the content producer >could potentially be any driver (or userspace) running in the VM that >knows how to import/export dma_buf's (or maybe just import given >danvet's suggestion that we should make the sink driver do all the >actual memory allocation for any buffers that may be shared). See above, just because it uses drm ioctls doesn't make it gfx specific. Otoh making it even more graphics specific might be even better, i.e. just sharing the backend tech (grant tables or whatever), but having dedicated front-ents for each use-case so there's less code to type. > * We need to be able to handle cross-VM coordination of buffer usage as >well, so I think we'd want to include fence forwarding support in the >API as well to signal back and forth about production/consumption >completion. And of course document really well what should happen >if, for example, the entire VM you're sharing with/from dies. Implicit fencing has been proven to be a bad idea. Please do explicit passing of dma_fences (plus assorted protocol). > * The sharing API could be used to share multiple kinds of content in a >single system. The sharing sink driver running in the content >producer's VM should accept some additional metadata that will be >passed over to the target VM as well. The sharing source driver >running in the content consumer's VM would then be able to use t
Re: [PATCH 3.18 00/93] 3.18.103-stable review
On Sun, Apr 08, 2018 at 10:26:18PM +0200, Greg Kroah-Hartman wrote: > On Sun, Apr 08, 2018 at 05:13:32PM +0200, Greg Kroah-Hartman wrote: > > On Sun, Apr 08, 2018 at 07:07:26AM -0700, Guenter Roeck wrote: > > > On Fri, Apr 06, 2018 at 03:22:29PM +0200, Greg Kroah-Hartman wrote: > > > > This is the start of the stable review cycle for the 3.18.103 release. > > > > There are 93 patches in this series, all will be posted as a response > > > > to this one. If anyone has any issues with these being applied, please > > > > let me know. > > > > > > > > Responses should be made by Sun Apr 8 08:42:04 UTC 2018. > > > > Anything received after that time might be too late. > > > > > > > > > > Not full results this time, sorry (my build system is corrupted), > > > > Ah, I was wondering what happened to your web results, sorry about this :( > > > > > but there are various build failures in 3.18.y. > > > > > > drivers/net/ethernet/freescale/fec_main.c: In function 'fec_drv_remove': > > > drivers/net/ethernet/freescale/fec_main.c:3342:2: error: implicit > > > declaration of function 'pm_runtime_put' > > > > Gotta love how 0-day doesn't catch these :( > > kernel.ci caught it :( > > > I thought I fixed this up for the 4.4.y tree already, let me dig... > > No, I didn't fix this for 4.4.y, that was something else. Should be > easy for me to resolve for the next 3.18.y release. Ok, added the needed .h file. thanks, greg k-h
Re: [GIT PULL] Kernel lockdown for secure boot
On 04/09/2018 05:40 AM, Alexei Starovoitov wrote: > On Sun, Apr 08, 2018 at 04:07:42PM +0800, joeyli wrote: [...] >>> If the only thing that folks are paranoid about is reading >>> arbitrary kernel memory with bpf_probe_read() helper >>> then preferred patch would be to disable it during verification >>> when in lockdown mode >> >> Sorry for I didn't fully understand your idea... >> Do you mean that using bpf verifier to filter out bpf program that >> uses bpf_probe_read()? > > Take a look bpf_get_trace_printk_proto(). > Similarly we can add bpf_get_probe_read_proto() that > will return NULL if lockdown is on. > Then programs with bpf_probe_read() will be rejected by the verifier. Fully agree with the above. For the two helpers, something like the below would be sufficient to reject progs at verification time. diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index d88e96d..51a6c2e 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -117,6 +117,11 @@ static const struct bpf_func_proto bpf_probe_read_proto = { .arg3_type = ARG_ANYTHING, }; +static const struct bpf_func_proto *bpf_get_probe_read_proto(void) +{ + return kernel_is_locked_down("BPF") ? NULL : &bpf_probe_read_proto; +} + BPF_CALL_3(bpf_probe_write_user, void *, unsafe_ptr, const void *, src, u32, size) { @@ -282,6 +287,9 @@ static const struct bpf_func_proto bpf_trace_printk_proto = { const struct bpf_func_proto *bpf_get_trace_printk_proto(void) { + if (kernel_is_locked_down("BPF")) + return NULL; + /* * this program might be calling bpf_trace_printk, * so allocate per-cpu printk buffers @@ -535,7 +543,7 @@ tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) case BPF_FUNC_map_delete_elem: return &bpf_map_delete_elem_proto; case BPF_FUNC_probe_read: - return &bpf_probe_read_proto; + return bpf_get_probe_read_proto(); case BPF_FUNC_ktime_get_ns: return &bpf_ktime_get_ns_proto; case BPF_FUNC_tail_call:
Re: [PATCH AUTOSEL for 4.15 019/189] printk: Add console owner and waiter logic to load balance console writes
On Mon 2018-04-09 00:16:59, Sasha Levin wrote: > From: "Steven Rostedt (VMware)" > > [ Upstream commit dbdda842fe96f8932bae554f0adf463c27c42bc7 ] > > This patch implements what I discussed in Kernel Summit. I added > lockdep annotation (hopefully correctly), and it hasn't had any splats > (since I fixed some bugs in the first iterations). It did catch > problems when I had the owner covering too much. But now that the owner > is only set when actively calling the consoles, lockdep has stayed > quiet. I do not think that this is a material for stable backports. Yes, it is a fix but it is not trivial. There are already 3 follow up commits: c162d5b4338d72deed6 ("printk: Hide console waiter logic into helpers") fd5f7cde1b85d4c8e09 ("printk: Never set console_may_schedule in console_trylock()") c14376de3a1befa70d9 ("printk: Wake klogd when passing console_lock owner") One is just a code clean up but the other two are changes/fixes that should go together with Steven's patch. These changes tries to prevent softlockups. It is a problem that is being discussed for years. We are still waiting for feedback to see if more changes will be necessary. IMHO, there is no reason to hurry and backport it everywhere. Best Regards, Petr
Re: [PATCH v3 0/4] backlight: pwm_bl: support linear interpolation and brightness to human eye
On Thu, 08 Feb 2018, Enric Balletbo i Serra wrote: > Dear all, > > This series is a third patchset integrating the requested changes. > > The first and second patch what tries to solve is the problem of > granularity for high resolution PWMs. The idea is simple interpolate > between 2 brightness values so we can have a high PWM duty cycle (a > 16 bits PWM is up to 65535 possible steps) without having to list > out every possible value in the dts. I think that this patch is > required to not break backward compability, to be more flexible and > also extend the functionality to be able to use high resolution PWM > with enough steps to have a good UI experience in userspace. > > The thirth and fourth patch is a bit more ambicious, the idea is let > decide the driver the brightness-levels required in function of the PWM > resolution. To do this create a brightness-levels table filled with the > CIE 1931 algorithm values to convert brightness to PWM duty cycle. > > More detailed info is available in the commit message of every patch. > > Both functionalities were tested on a Samsung Chromebook Plus (that has > a 16 bits PWM) and a SL50 device (with a 8 bits PWM) > > Waiting for your feedback. Looks like you now have some positive feedback. :) Could you please collect all of your received Acks and re-post this set as a [RESEND] please? > Best regards, > > Enric Balletbo i Serra (4): > backlight: pwm_bl: linear interpolation between brightness-levels > dt-bindings: pwm-backlight: add a num-interpolation-steps property. > backlight: pwm_bl: compute brightness of LED linearly to human eye. > dt-bindings: pwm-backlight: move brightness-levels to optional. > > .../bindings/leds/backlight/pwm-backlight.txt | 34 ++- > drivers/video/backlight/pwm_bl.c | 232 > +++-- > 2 files changed, 246 insertions(+), 20 deletions(-) -- Lee Jones [李琼斯] Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
Re: [PATCH v2] Add udmabuf misc device
On Fri, Apr 06, 2018 at 02:24:46PM +0200, Christian König wrote: > Am 06.04.2018 um 11:33 schrieb Gerd Hoffmann: > >Hi, > > > > > The pages backing a DMA-buf are not allowed to move (at least not without > > > a > > > patch set I'm currently working on), but for certain MM operations to work > > > correctly you must be able to modify the page tables entries and move the > > > pages backing them around. > > > > > > For example try to use fork() with some copy on write pages with this > > > approach. You will find that you have only two options to correctly handle > > > this. > > The fork() issue should go away with shared memory pages (no cow). > > I guess this is the reason why vgem is internally backed by shmem. > > Yes, exactly that is also an approach which should work fine. Just don't try > to get this working with get_user_pages(). > > > > > Hmm. So I could try to limit the udmabuf driver to shmem too (i.e. > > have the ioctl take a shmem filehandle and offset instead of a virtual > > address). > > > > But maybe it is better then to just extend vgem, i.e. add support to > > create gem objects from existing shmem. > > > > Comments? > > Yes, extending vgem instead of creating something new sounds like a good > idea to me as well. +1 on adding a vgem "import from shmem/memfd" ioctl. Sounds like a good idea, and generally useful. We might want to limit to memfd though for semantic reasons: dma-buf have invariant size, shmem not so much. memfd can be locked down to not change their size anymore. And iirc the core mm page invalidation protocol around truncate() is about as bad as get_user_pages vs cow :-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH AUTOSEL for 4.14 015/161] printk: Add console owner and waiter logic to load balance console writes
On Mon 2018-04-09 00:19:53, Sasha Levin wrote: > From: "Steven Rostedt (VMware)" > > [ Upstream commit dbdda842fe96f8932bae554f0adf463c27c42bc7 ] > > This patch implements what I discussed in Kernel Summit. I added > lockdep annotation (hopefully correctly), and it hasn't had any splats > (since I fixed some bugs in the first iterations). It did catch > problems when I had the owner covering too much. But now that the owner > is only set when actively calling the consoles, lockdep has stayed > quiet. Same here. I do not thing that this is a material for stable backport. More details can be found in my reply to the patch for 4.15, see https://lkml.kernel.org/r/20180409081535.dq7p5bfnpvd3x...@pathway.suse.cz Best Regards, Petr PS: I wonder how much time you give people to react before releasing this. The number of autosel mails is increasing and I am involved only in very small amount of them. I wonder if some other people gets overwhelmed by this.
Re: [PATCH 8/8] drm/arm/malidp: Added the late system pm functions
On Fri, Apr 06, 2018 at 08:02:16PM +0100, Ayan Halder wrote: > On Tue, Mar 27, 2018 at 01:09:36PM +0200, Daniel Vetter wrote: > > On Tue, Mar 27, 2018 at 11:59 AM, Ayan Halder wrote: > > > On Tue, Mar 27, 2018 at 10:29:03AM +0200, Daniel Vetter wrote: > > >> On Mon, Mar 26, 2018 at 06:03:20PM +0100, Ayan Kumar Halder wrote: > > >> > malidp_pm_suspend_late checks if the runtime status is not suspended > > >> > and if so, invokes malidp_runtime_pm_suspend which disables the > > >> > display engine/core interrupts and the clocks. It sets the runtime > > >> > status > > >> > as suspended. Subsequently, malidp_pm_resume_early will invoke > > >> > malidp_runtime_pm_resume which enables the clocks and the interrupts > > >> > (previously disabled) and sets the runtime status as active. > > >> > > > >> > Signed-off-by: Ayan Kumar Halder > > >> > Change-Id: I5f8c3d28f076314a1c9da2a46760a9c37039ccda > > >> > > >> Why exactly do you need late/early hooks? If you have dependencies with > > >> other devices, pls consider adding device_links instead. This here > > >> shouldn't be necessary. > > >> -Daniel > > > We need to late/early hooks to disable malidp interrupts and the > > > clocks. > > > > Yes, but why this ordering constraint? Why can't you just disable the > > interrupts/clocks in the normal suspend code. I see that the patch > > does this, I want to understand why it does it. > > -Daniel > Apologies for my delayed response on this. > > With reference to https://lwn.net/Articles/505683/ :- > 1. "suspend() should leave the device in a quiescent state." We invoke > drm_mode_config_helper_suspend() which deactivates the crtc. I > understand that this is the quiescent state. > > 2. "suspend_late() can often be the same as runtime_suspend()." We > invoke runtime suspend/resume calls in late/early hooks. This article is from 2012. That's not really recommended best practices anymore. device_links have only been added a few years ago, so ofc an article from 2012 can't tell you that you should use those instead :-) That's why I brought this up, we have much better ways to handle device dependencies now. Also, you still haven't explained what exactly the dependency is. -Daniel > > > >> > --- > > >> > drivers/gpu/drm/arm/malidp_drv.c | 17 + > > >> > 1 file changed, 17 insertions(+) > > >> > > > >> > diff --git a/drivers/gpu/drm/arm/malidp_drv.c > > >> > b/drivers/gpu/drm/arm/malidp_drv.c > > >> > index bd44a6d..f6124d8 100644 > > >> > --- a/drivers/gpu/drm/arm/malidp_drv.c > > >> > +++ b/drivers/gpu/drm/arm/malidp_drv.c > > >> > @@ -766,8 +766,25 @@ static int __maybe_unused malidp_pm_resume(struct > > >> > device *dev) > > >> > return 0; > > >> > } > > >> > > > >> > +static int __maybe_unused malidp_pm_suspend_late(struct device *dev) > > >> > +{ > > >> > + if (!pm_runtime_status_suspended(dev)) { > > >> > + malidp_runtime_pm_suspend(dev); > > >> > + pm_runtime_set_suspended(dev); > > >> > + } > > >> > + return 0; > > >> > +} > > >> > + > > >> > +static int __maybe_unused malidp_pm_resume_early(struct device *dev) > > >> > +{ > > >> > + malidp_runtime_pm_resume(dev); > > >> > + pm_runtime_set_active(dev); > > >> > + return 0; > > >> > +} > > >> > + > > >> > static const struct dev_pm_ops malidp_pm_ops = { > > >> > SET_SYSTEM_SLEEP_PM_OPS(malidp_pm_suspend, malidp_pm_resume) \ > > >> > + SET_LATE_SYSTEM_SLEEP_PM_OPS(malidp_pm_suspend_late, > > >> > malidp_pm_resume_early) \ > > >> > SET_RUNTIME_PM_OPS(malidp_runtime_pm_suspend, > > >> > malidp_runtime_pm_resume, NULL) > > >> > }; > > >> > > > >> > -- > > >> > 2.7.4 > > >> > > > >> > ___ > > >> > dri-devel mailing list > > >> > dri-de...@lists.freedesktop.org > > >> > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > >> > > >> -- > > >> Daniel Vetter > > >> Software Engineer, Intel Corporation > > >> http://blog.ffwll.ch > > >> ___ > > >> dri-devel mailing list > > >> dri-de...@lists.freedesktop.org > > >> https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > IMPORTANT NOTICE: The contents of this email and any attachments are > > > confidential and may also be privileged. If you are not the intended > > > recipient, please notify the sender immediately and do not disclose the > > > contents to any other person, use it for any purpose, or store or copy > > > the information in any medium. Thank you. > > > ___ > > > dri-devel mailing list > > > dri-de...@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > > > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH v5 07/13] ARM: smp: Add initialization of CNTVOFF
On Wed, Apr 4, 2018 at 4:30 PM, Marc Zyngier wrote: > On Wed, 04 Apr 2018 14:59:09 +0100, > Mylčne Josserand wrote: [Marc: stuck in ISO-8859-1? ;-] >> > It'd be good to take this opportunity to refactor the shmobile code. >> >> I can do it in this series but I do not have any shmobile platforms so >> I will not be able to test my modifications (only compilation). >> >> If someone can test it for me (who?), it is okay for me to refactor this >> code :) > > I guess you could Cc the shmobile folks (Geert Uytterhoeven, Simon > Horman), and get them to review/test the changes. Correct. I can test on a remote R-Car E2 ALT board that needs it. P.S. Interestingly, none of the Renesas CA15 SoCs seem to suffer from it, only CA7. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re: [PATCH 1/1] drm/xen-zcopy: Add Xen zero-copy helper DRM driver
On Fri, Apr 06, 2018 at 02:25:08PM +0300, Oleksandr Andrushchenko wrote: > On 04/03/2018 12:47 PM, Daniel Vetter wrote: > > On Thu, Mar 29, 2018 at 04:19:31PM +0300, Oleksandr Andrushchenko wrote: > > > From: Oleksandr Andrushchenko > > > +static int to_refs_grant_foreign_access(struct xen_gem_object *xen_obj) > > > +{ > > > + grant_ref_t priv_gref_head; > > > + int ret, j, cur_ref, num_pages; > > > + struct sg_page_iter sg_iter; > > > + > > > + ret = gnttab_alloc_grant_references(xen_obj->num_pages, > > > + &priv_gref_head); > > > + if (ret < 0) { > > > + DRM_ERROR("Cannot allocate grant references\n"); > > > + return ret; > > > + } > > > + > > > + j = 0; > > > + num_pages = xen_obj->num_pages; > > > + for_each_sg_page(xen_obj->sgt->sgl, &sg_iter, xen_obj->sgt->nents, 0) { > > > + struct page *page; > > > + > > > + page = sg_page_iter_page(&sg_iter); > > Quick drive-by: You can't assume that an sgt is struct page backed. > Do you mean that someone could give me sgt which never > seen sg_assign_page for its entries? Yes. > What are the other use-cases for that to happen? Sharing vram or other resources which are not backed by a struct page. See Christian König's recent work to accomplish just that for admgpu. > > And you probably want to check this at import/attach time. > The check you mean is to make sure that when I call > page = sg_page_iter_page(&sg_iter); > I have to make sure that I get a valid page? Yup. > > -Daniel > Thank you, > Oleksandr Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH v2 9/9] perf probe: Support SDT markers having reference counter (semaphore)
Hi Masami, On 04/09/2018 12:58 PM, Masami Hiramatsu wrote: > Hi Ravi, > > On Wed, 4 Apr 2018 14:01:10 +0530 > Ravi Bangoria wrote: > >> @@ -2054,15 +2060,21 @@ char *synthesize_probe_trace_command(struct >> probe_trace_event *tev) >> } >> >> /* Use the tp->address for uprobes */ >> -if (tev->uprobes) >> +if (tev->uprobes) { >> err = strbuf_addf(&buf, "%s:0x%lx", tp->module, tp->address); >> -else if (!strncmp(tp->symbol, "0x", 2)) >> +if (uprobe_ref_ctr_is_supported() && >> +tp->ref_ctr_offset && >> +err >= 0) >> +err = strbuf_addf(&buf, "(0x%lx)", tp->ref_ctr_offset); > If the kernel doesn't support uprobe_ref_ctr but the event requires > to increment uprobe_ref_ctr, I think we should (at least) warn user here. pr_debug("A semaphore is associated with %s:%s and seems your kernel doesn't support it.\n" tev->group, tev->event); Looks good? >> @@ -776,14 +784,21 @@ static char *synthesize_sdt_probe_command(struct >> sdt_note *note, >> { >> struct strbuf buf; >> char *ret = NULL, **args; >> -int i, args_count; >> +int i, args_count, err; >> +unsigned long long ref_ctr_offset; >> >> if (strbuf_init(&buf, 32) < 0) >> return NULL; >> >> -if (strbuf_addf(&buf, "p:%s/%s %s:0x%llx", >> -sdtgrp, note->name, pathname, >> -sdt_note__get_addr(note)) < 0) >> +err = strbuf_addf(&buf, "p:%s/%s %s:0x%llx", >> +sdtgrp, note->name, pathname, >> +sdt_note__get_addr(note)); >> + >> +ref_ctr_offset = sdt_note__get_ref_ctr_offset(note); >> +if (uprobe_ref_ctr_is_supported() && ref_ctr_offset && err >= 0) >> +err = strbuf_addf(&buf, "(0x%llx)", ref_ctr_offset); > We don't have to care about uprobe_ref_ctr support here, because > this information will be just cached, not directly written to > uprobe_events. Sure, will remove the check. Thanks for the review :). Ravi
Re: [RFC 1/3] drm: Add DAMAGE_CLIPS property to plane
On Thu, Apr 05, 2018 at 11:07:19PM +, Deepak Singh Rawat wrote: > > > > On Wed, Apr 04, 2018 at 04:49:06PM -0700, Deepak Rawat wrote: > > > From: Lukasz Spintzyk > > > > > > Optional plane property to mark damaged regions on the plane in > > > framebuffer coordinates of the framebuffer attached to the plane. > > > > > > The layout of blob data is simply an array of drm_mode_rect with > > maximum > > > array size limited by DRM_MODE_FB_DIRTY_MAX_CLIPS. Unlike plane src > > > coordinates, damage clips are not in 16.16 fixed point. > > > > > > Damage clips are a hint to kernel as which area of framebuffer has > > > changed since last page-flip. This should be helpful for some drivers > > > especially for virtual devices where each framebuffer change needs to > > > be transmitted over network, usb, etc. > > > > > > Driver which are interested in enabling DAMAGE_CLIPS property for a > > > plane should enable this property using drm_plane_enable_damage_clips. > > > > > > Signed-off-by: Lukasz Spintzyk > > > Signed-off-by: Deepak Rawat > > > > The property uapi section is missing, see: > > > > https://urldefense.proofpoint.com/v2/url?u=https- > > 3A__dri.freedesktop.org_docs_drm_gpu_drm-2Dkms.html-23plane- > > 2Dcomposition- > > 2Dproperties&d=DwIBAg&c=uilaK90D4TOVoH58JNXRgQ&r=zOOG28inJK0762 > > SxAf- > > cyZdStnd2NQpRu98lJP2iYGw&m=ELAUsNTjD0ICwUEDFjPW4jsmy2A5AkhS5Q > > z_3vlEC9Q&s=nH-HNXPczoJQMj1iwHiGfrhXz4-00b0M8-3kirjm-EY&e= > > > > Plane composition feels like the best place to put this. Please use that > > section to tie all the various bits together, including the helpers you're > > adding in the following patches for drivers to use. > > > > Bunch of nitpicks below, but overall I'm agreeing now with just going with > > fb coordinate damage rects. > > > > Like you say, the thing needed here now is userspace + driver actually > > implementing this. I'd also say the compat helper to map the legacy > > fb->dirty to this new atomic way of doing things should be included here > > (gives us a lot more testing for these new paths). > > > > Icing on the cake would be an igt to make sure kernel rejects invalid clip > > rects correctly. > > > > > --- > > > drivers/gpu/drm/drm_atomic.c| 42 > > + > > > drivers/gpu/drm/drm_atomic_helper.c | 4 > > > drivers/gpu/drm/drm_mode_config.c | 5 + > > > drivers/gpu/drm/drm_plane.c | 12 +++ > > > include/drm/drm_mode_config.h | 15 + > > > include/drm/drm_plane.h | 16 ++ > > > include/uapi/drm/drm_mode.h | 15 + > > > 7 files changed, 109 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/drm_atomic.c > > b/drivers/gpu/drm/drm_atomic.c > > > index 7d25c42..9226d24 100644 > > > --- a/drivers/gpu/drm/drm_atomic.c > > > +++ b/drivers/gpu/drm/drm_atomic.c > > > @@ -669,6 +669,40 @@ static void drm_atomic_crtc_print_state(struct > > drm_printer *p, > > > } > > > > > > /** > > > + * drm_atomic_set_damage_for_plane - sets the damage clips property > > to plane > > > + * @state: plane state > > > + * @blob: damage clips in framebuffer coordinates > > > + * > > > + * Returns: > > > + * > > > + * Zero on success, error code on failure. > > > + */ > > > +static int drm_atomic_set_damage_for_plane(struct drm_plane_state > > *state, > > > +struct drm_property_blob *blob) > > > +{ > > > + if (blob == state->damage_clips) > > > + return 0; > > > + > > > + drm_property_blob_put(state->damage_clips); > > > + state->damage_clips = NULL; > > > + > > > + if (blob) { > > > + uint32_t count = blob->length/sizeof(struct drm_rect); > > > + > > > + if (count > DRM_MODE_FB_DIRTY_MAX_CLIPS) > > > + return -EINVAL; > > > + > > > + state->damage_clips = drm_property_blob_get(blob); > > > + state->num_clips = count; > > > + } else { > > > + state->damage_clips = NULL; > > > + state->num_clips = 0; > > > + } > > > + > > > + return 0; > > > +} > > > + > > > +/** > > > * drm_atomic_get_plane_state - get plane state > > > * @state: global atomic state object > > > * @plane: plane to get state object for > > > @@ -793,6 +827,12 @@ static int drm_atomic_plane_set_property(struct > > drm_plane *plane, > > > state->color_encoding = val; > > > } else if (property == plane->color_range_property) { > > > state->color_range = val; > > > + } else if (property == config->prop_damage_clips) { > > > + struct drm_property_blob *blob = > > > + drm_property_lookup_blob(dev, val); > > > + int ret = drm_atomic_set_damage_for_plane(state, blob); > > > > There's already a helper with size-checking built-in, see > > drm_atomic_replace_property_blob_from_id(). Wrt computing num_clips > > I'd > > just provide a little inline helper that does the > > blob->length/sizeof(drm_rect) conversion (it's just a shift, so
[RESEND PATCH v3 4/4] dt-bindings: pwm-backlight: move brightness-levels to optional.
The patch 'backlight: pwm_bl: compute brightness of LED linearly to human eye' introduced a default brightness-levels table that is used when brightness-levels is not available in the dts. So move brightness-levels and default-brightness-level to be optional. Signed-off-by: Enric Balletbo i Serra Reviewed-by: Rob Herring Acked-by: Daniel Thompson --- .../devicetree/bindings/leds/backlight/pwm-backlight.txt | 15 --- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt index ce9b5746b774..64fa2fbd98c9 100644 --- a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt +++ b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt @@ -3,13 +3,6 @@ pwm-backlight bindings Required properties: - compatible: "pwm-backlight" - pwms: OF device-tree PWM specification (see PWM binding[0]) - - brightness-levels: Array of distinct brightness levels. Typically these - are in the range from 0 to 255, but any range starting at 0 will do. - The actual brightness level (PWM duty cycle) will be interpolated - from these values. 0 means a 0% duty cycle (darkest/off), while the - last value in the array represents a 100% duty cycle (brightest). - - default-brightness-level: the default brightness level (index into the - array defined by the "brightness-levels" property) - power-supply: regulator for supply voltage Optional properties: @@ -21,6 +14,14 @@ Optional properties: and enabling the backlight using GPIO. - pwm-off-delay-ms: Delay in ms between disabling the backlight using GPIO and setting PWM value to 0. + - brightness-levels: Array of distinct brightness levels. Typically these + are in the range from 0 to 255, but any range starting at + 0 will do. The actual brightness level (PWM duty cycle) + will be interpolated from these values. 0 means a 0% duty + cycle (darkest/off), while the last value in the array + represents a 100% duty cycle (brightest). + - default-brightness-level: The default brightness level (index into the + array defined by the "brightness-levels" property). - num-interpolated-steps: Number of interpolated steps between each value of brightness-levels table. This way a high resolution pwm duty cycle can be used without -- 2.16.3
[RESEND PATCH v3 0/4] backlight: pwm_bl: support linear interpolation and brightness to human eye
Dear all, This series is a third patchset (resend )integrating the requested changes. The first and second patch what tries to solve is the problem of granularity for high resolution PWMs. The idea is simple interpolate between 2 brightness values so we can have a high PWM duty cycle (a 16 bits PWM is up to 65535 possible steps) without having to list out every possible value in the dts. I think that this patch is required to not break backward compatibility, to be more flexible and also extend the functionality to be able to use high resolution PWM with enough steps to have a good UI experience in userspace. The thirth and fourth patch is a bit more ambicious, the idea is let decide the driver the brightness-levels required in function of the PWM resolution. To do this create a brightness-levels table filled with the CIE 1931 algorithm values to convert brightness to PWM duty cycle. More detailed info is available in the commit message of every patch. Both functionalities were tested on a Samsung Chromebook Plus (that has a 16 bits PWM) and a SL50 device (with a 8 bits PWM) Waiting for your feedback. Best regards Enric Balletbo i Serra (4): backlight: pwm_bl: linear interpolation between brightness-levels dt-bindings: pwm-backlight: add a num-interpolation-steps property. backlight: pwm_bl: compute brightness of LED linearly to human eye. dt-bindings: pwm-backlight: move brightness-levels to optional. .../bindings/leds/backlight/pwm-backlight.txt | 34 ++- drivers/video/backlight/pwm_bl.c | 232 +++-- 2 files changed, 246 insertions(+), 20 deletions(-) -- 2.16.3
[RESEND PATCH v3 1/4] backlight: pwm_bl: linear interpolation between brightness-levels
Setting num-interpolated-steps in the dts will allow you to have linear interpolation between values of brightness-levels. This way a high resolution pwm duty cycle can be used without having to list out every possible value in the dts. This system also allows for gamma corrected values. The most simple example is interpolate between two brightness values a number of steps, this can be done setting the following in the dts: brightness-levels = <0 65535>; num-interpolated-steps = <1024>; default-brightness-level = <512>; This will create a brightness-level table with the following values: <0 63 126 189 252 315 378 441 ... 64260 64323 64386 64449 65535> Another use case can be describe a gamma corrected curve, as we have better sensitivity at low luminance than high luminance we probably want have smaller steps for low brightness levels values and bigger steps for high brightness levels values. This can be achieved with the following in the dts: brightness-levels = <0 4096 65535>; num-interpolated-steps = <1024>; default-brightness-level = <512>; This will create a brightness-levels table with the following values: <0 4 8 12 16 20 ... 4096 4156 4216 4276 ... 65535> Signed-off-by: Enric Balletbo i Serra Acked-by: Daniel Thompson --- drivers/video/backlight/pwm_bl.c | 83 1 file changed, 83 insertions(+) diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c index 8e3f1245f5c5..f0a108ab570a 100644 --- a/drivers/video/backlight/pwm_bl.c +++ b/drivers/video/backlight/pwm_bl.c @@ -147,7 +147,11 @@ static int pwm_backlight_parse_dt(struct device *dev, struct platform_pwm_backlight_data *data) { struct device_node *node = dev->of_node; + unsigned int num_levels = 0; + unsigned int levels_count; + unsigned int num_steps; struct property *prop; + unsigned int *table; int length; u32 value; int ret; @@ -167,6 +171,7 @@ static int pwm_backlight_parse_dt(struct device *dev, /* read brightness levels from DT property */ if (data->max_brightness > 0) { size_t size = sizeof(*data->levels) * data->max_brightness; + unsigned int i, j, n = 0; data->levels = devm_kzalloc(dev, size, GFP_KERNEL); if (!data->levels) @@ -184,6 +189,84 @@ static int pwm_backlight_parse_dt(struct device *dev, return ret; data->dft_brightness = value; + + /* +* This property is optional, if is set enables linear +* interpolation between each of the values of brightness levels +* and creates a new pre-computed table. +*/ + of_property_read_u32(node, "num-interpolated-steps", +&num_steps); + + /* +* Make sure that there is at least two entries in the +* brightness-levels table, otherwise we can't interpolate +* between two points. +*/ + if (num_steps) { + if (data->max_brightness < 2) { + dev_err(dev, "can't interpolate\n"); + return -EINVAL; + } + + /* +* Recalculate the number of brightness levels, now +* taking in consideration the number of interpolated +* steps between two levels. +*/ + for (i = 0; i < data->max_brightness - 1; i++) { + if ((data->levels[i + 1] - data->levels[i]) / + num_steps) + num_levels += num_steps; + else + num_levels++; + } + num_levels++; + dev_dbg(dev, "new number of brightness levels: %d\n", + num_levels); + + /* +* Create a new table of brightness levels with all the +* interpolated steps. +*/ + size = sizeof(*table) * num_levels; + table = devm_kzalloc(dev, size, GFP_KERNEL); + if (!table) + return -ENOMEM; + + /* Fill the interpolated table. */ + levels_count = 0; + for (i = 0; i < data->max_brightness - 1; i++) { + value = data->levels[i]; + n = (data->levels[i + 1] - value) / num_steps; + if (n > 0) { + for (j = 0
[RESEND PATCH v3 3/4] backlight: pwm_bl: compute brightness of LED linearly to human eye.
When you want to change the brightness using a PWM signal, one thing you need to consider is how human perceive the brightness. Human perceive the brightness change non-linearly, we have better sensitivity at low luminance than high luminance, so to achieve perceived linear dimming, the brightness must be matches to the way our eyes behave. The CIE 1931 lightness formula is what actually describes how we perceive light. This patch computes a default table with the brightness levels filled with the numbers provided by the CIE 1931 algorithm, the number of the brightness levels is calculated based on the PWM resolution. The calculation of the table using the CIE 1931 algorithm is enabled by default when you do not define the 'brightness-levels' propriety in your device tree. Signed-off-by: Enric Balletbo i Serra Acked-by: Daniel Thompson --- drivers/video/backlight/pwm_bl.c | 149 +++ 1 file changed, 136 insertions(+), 13 deletions(-) diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c index f0a108ab570a..d047d875f251 100644 --- a/drivers/video/backlight/pwm_bl.c +++ b/drivers/video/backlight/pwm_bl.c @@ -143,6 +143,107 @@ static const struct backlight_ops pwm_backlight_ops = { }; #ifdef CONFIG_OF +#define PWM_LUMINANCE_SCALE1 /* luminance scale */ + +/* An integer based power function */ +static u64 int_pow(u64 base, int exp) +{ + u64 result = 1; + + while (exp) { + if (exp & 1) + result *= base; + exp >>= 1; + base *= base; + } + + return result; +} + +/* + * CIE lightness to PWM conversion. + * + * The CIE 1931 lightness formula is what actually describes how we perceive + * light: + * Y = (L* / 902.3) if L* ≤ 0.08856 + * Y = ((L* + 16) / 116)^3if L* > 0.08856 + * + * Where Y is the luminance, the amount of light coming out of the screen, and + * is a number between 0.0 and 1.0; and L* is the lightness, how bright a human + * perceives the screen to be, and is a number between 0 and 100. + * + * The following function does the fixed point maths needed to implement the + * above formula. + */ +static u64 cie1931(unsigned int lightness, unsigned int scale) +{ + u64 retval; + + lightness *= 100; + if (lightness <= (8 * scale)) { + retval = DIV_ROUND_CLOSEST_ULL(lightness * 10, 9023); + } else { + retval = int_pow((lightness + (16 * scale)) / 116, 3); + retval = DIV_ROUND_CLOSEST_ULL(retval, (scale * scale)); + } + + return retval; +} + +/* + * Create a default correction table for PWM values to create linear brightness + * for LED based backlights using the CIE1931 algorithm. + */ +static +int pwm_backlight_brightness_default(struct device *dev, +struct platform_pwm_backlight_data *data, +unsigned int period) +{ + unsigned int counter = 0; + unsigned int i, n; + u64 retval; + + /* +* Count the number of bits needed to represent the period number. The +* number of bits is used to calculate the number of levels used for the +* brightness-levels table, the purpose of this calculation is have a +* pre-computed table with enough levels to get linear brightness +* perception. The period is divided by the number of bits so for a +* 8-bit PWM we have 255 / 8 = 32 brightness levels or for a 16-bit PWM +* we have 65535 / 16 = 4096 brightness levels. +* +* Note that this method is based on empirical testing on different +* devices with PWM of 8 and 16 bits of resolution. +*/ + n = period; + while (n) { + counter += n % 2; + n >>= 1; + } + + data->max_brightness = DIV_ROUND_UP(period, counter); + data->levels = devm_kcalloc(dev, data->max_brightness, + sizeof(*data->levels), GFP_KERNEL); + if (!data->levels) + return -ENOMEM; + + /* Fill the table using the cie1931 algorithm */ + for (i = 0; i < data->max_brightness; i++) { + retval = cie1931((i * PWM_LUMINANCE_SCALE) / +data->max_brightness, PWM_LUMINANCE_SCALE) * +period; + retval = DIV_ROUND_CLOSEST_ULL(retval, PWM_LUMINANCE_SCALE); + if (retval > UINT_MAX) + return -EINVAL; + data->levels[i] = (unsigned int)retval; + } + + data->dft_brightness = data->max_brightness / 2; + data->max_brightness--; + + return 0; +} + static int pwm_backlight_parse_dt(struct device *dev, struct platform_pwm_backlight_data *data) { @@ -161,10 +262,13 @@ static int pwm_backlight_parse_dt(struct device *dev,
Re: [RFC 2/3] drm: Add helper iterator functions to iterate over plane damage.
On Thu, Apr 05, 2018 at 11:59:57PM +, Deepak Singh Rawat wrote: > > plane damage. > > > > On 04/05/2018 09:52 AM, Daniel Vetter wrote: > > > > > > TYPE_PLANE I have no idea who needs that. I suggest we just drop it. > > > > I'm assuming CRTC plane coordinates here. They are used for uploading > > contents of hardware planes. Like, in the simplest case, cursor images. > > Yes they are CRTC plane coordinates, so is TYPE_PLANE naming confusing ? > And should be named to TYPE_CRTC_PLANE but then it is confusing with > TYPE_CRTC. Yeah, I think TYPE_PLANE is really confusing, and too much aimied at your vmwgfx special case (where the virtual hw requires that this all lines up properly). I think providing FB coordinates, and doing the vmwgfx-specific remapping in vmwgfx code is better. And someone else can then figure out how to handle CRTC overall damage for physical devices. As mentioned by me (and Rob Clark too), most hw only allows for 1 (or maybe 2) overall damage rects, so that helper would need to combine all the damge rects into 1. Plus take stuff like gamma/ctm/alpha into account too. Better we leave that to someone who needs it. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH v2 0/3] Update reset and poll logic for GDSCs
[v2] * Addressed review comments given in v1 series This series implements the below logic for the GDSCs 1. logic to reset the AON logic before or assert/deassert the block control reset removing the clamp io for few GDSCs on SDM845 SoC. 2. It also introduces the requirement to poll for higher timeout values for few of the GDSCs. 3. There is a new poll register for the GDSCs on SDM845 SoCs which needs to be polled for the correct hardware status of the GDSCs. Amit Nischal (3): clk: qcom: gdsc: Add support to reset AON and block reset logic clk: qcom: gdsc: Add support to poll for higher timeout value clk: qcom: gdsc: Add support to poll CFG register to check GDSC state drivers/clk/qcom/gdsc.c | 63 +++-- drivers/clk/qcom/gdsc.h | 5 +++- 2 files changed, 60 insertions(+), 8 deletions(-) -- Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member of the Code Aurora Forum, hosted by the Linux Foundation.
[PATCH v2] kvm: nVMX: Introduce KVM_CAP_STATE
From: Jim Mattson For nested virtualization L0 KVM is managing a bit of state for L2 guests, this state can not be captured through the currently available IOCTLs. In fact the state captured through all of these IOCTLs is usually a mix of L1 and L2 state. It is also dependent on whether the L2 guest was running at the moment when the process was interrupted to save its state. With this capability, there are two new vcpu ioctls: KVM_GET_VMX_STATE and KVM_SET_VMX_STATE. These can be used for saving and restoring a VM that is in VMX operation. Cc: Paolo Bonzini Cc: Radim Krčmář Cc: Thomas Gleixner Cc: Ingo Molnar Cc: H. Peter Anvin Cc: x...@kernel.org Cc: k...@vger.kernel.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Jim Mattson [karahmed@ - rename structs and functions and make them ready for AMD and address previous comments. - rebase & a bit of refactoring. - Merge 7/8 and 8/8 into one patch. - Force a VMExit from L2 after reading the kvm_state to avoid mixed state between L1 and L2 on resurrecting the instance. ] Signed-off-by: KarimAllah Ahmed --- v1 -> v2: - rename structs and functions and make them ready for AMD and address previous comments. - rebase & a bit of refactoring. - Merge 7/8 and 8/8 into one patch. - Force a VMExit from L2 after reading the kvm_state to avoid mixed state between L1 and L2 on resurrecting the instance. --- Documentation/virtual/kvm/api.txt | 46 ++ arch/x86/include/asm/kvm_host.h | 7 ++ arch/x86/include/uapi/asm/kvm.h | 38 arch/x86/kvm/vmx.c| 189 +- arch/x86/kvm/x86.c| 21 + include/uapi/linux/kvm.h | 5 + 6 files changed, 302 insertions(+), 4 deletions(-) diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt index d6b3ff5..3ed56df 100644 --- a/Documentation/virtual/kvm/api.txt +++ b/Documentation/virtual/kvm/api.txt @@ -3516,6 +3516,52 @@ Returns: 0 on success; -1 on error This ioctl can be used to unregister the guest memory region registered with KVM_MEMORY_ENCRYPT_REG_REGION ioctl above. +4.112 KVM_GET_STATE + +Capability: KVM_CAP_STATE +Architectures: x86 +Type: vcpu ioctl +Parameters: struct kvm_state (in/out) +Returns: 0 on success, -1 on error +Errors: + E2BIG: the data size exceeds the value of 'size' specified by + the user (the size required will be written into size). + +struct kvm_state { + __u16 flags; + __u16 format; + __u32 size; + union { + struct kvm_vmx_state vmx; + struct kvm_svm_state svm; + __u8 pad[120]; + }; + __u8 data[0]; +}; + +This ioctl copies the vcpu's kvm_state struct from the kernel to userspace. + +4.113 KVM_SET_STATE + +Capability: KVM_CAP_STATE +Architectures: x86 +Type: vcpu ioctl +Parameters: struct kvm_state (in) +Returns: 0 on success, -1 on error + +struct kvm_state { + __u16 flags; + __u16 format; + __u32 size; + union { + struct kvm_vmx_state vmx; + struct kvm_svm_state svm; + __u8 pad[120]; + }; + __u8 data[0]; +}; + +This copies the vcpu's kvm_state struct from userspace to the kernel. 5. The kvm_run structure diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index fad4d46..902db9e 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -73,6 +73,7 @@ #define KVM_REQ_HV_RESET KVM_ARCH_REQ(20) #define KVM_REQ_HV_EXITKVM_ARCH_REQ(21) #define KVM_REQ_HV_STIMER KVM_ARCH_REQ(22) +#define KVM_REQ_GET_VMCS12_PAGES KVM_ARCH_REQ(23) #define CR0_RESERVED_BITS \ (~(unsigned long)(X86_CR0_PE | X86_CR0_MP | X86_CR0_EM | X86_CR0_TS \ @@ -1090,6 +1091,12 @@ struct kvm_x86_ops { void (*setup_mce)(struct kvm_vcpu *vcpu); + int (*get_state)(struct kvm_vcpu *vcpu, +struct kvm_state __user *user_kvm_state); + int (*set_state)(struct kvm_vcpu *vcpu, +struct kvm_state __user *user_kvm_state); + void (*get_vmcs12_pages)(struct kvm_vcpu *vcpu); + int (*smi_allowed)(struct kvm_vcpu *vcpu); int (*pre_enter_smm)(struct kvm_vcpu *vcpu, char *smstate); int (*pre_leave_smm)(struct kvm_vcpu *vcpu, u64 smbase); diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h index f3a9604..1d1cd26 100644 --- a/arch/x86/include/uapi/asm/kvm.h +++ b/arch/x86/include/uapi/asm/kvm.h @@ -361,4 +361,42 @@ struct kvm_sync_regs { #define KVM_X86_QUIRK_LINT0_REENABLED (1 << 0) #define KVM_X86_QUIRK_CD_NW_CLEARED(1 << 1) +#define KVM_STATE_GUEST_MODE 0x0001 +#define KVM_STATE_RUN_PENDING 0x0002 +#define KVM_STATE_GIF 0x0004 + +str
[PATCH v2 3/3] clk: qcom: gdsc: Add support to poll CFG register to check GDSC state
From: Amit Nischal The default behavior of the GDSC enable/disable sequence is to poll the status bits of either the actual GDSCR or the corresponding HW_CTRL registers. On targets which have support for a CFG_GDSCR register, the status bits might not show the correct state of the GDSC, especially in the disable sequence, where the status bit will be cleared even before the core is completely power collapsed. On targets with this issue, poll the power on/off bits in the CFG_GDSCR register instead to correctly determine the GDSC state. Signed-off-by: Amit Nischal Signed-off-by: Taniya Das --- drivers/clk/qcom/gdsc.c | 39 +++ drivers/clk/qcom/gdsc.h | 1 + 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c index cb61c15..2dda2d5 100644 --- a/drivers/clk/qcom/gdsc.c +++ b/drivers/clk/qcom/gdsc.c @@ -33,6 +33,11 @@ #define GMEM_CLAMP_IO_MASK BIT(0) #define GMEM_RESET_MASKBIT(4) +/* CFG_GDSCR */ +#define GDSC_POWER_UP_COMPLETE BIT(16) +#define GDSC_POWER_DOWN_COMPLETE BIT(15) +#define CFG_GDSCR_OFFSET 0x4 + /* Wait 2^n CXO cycles between all states. Here, n=2 (4 cycles). */ #define EN_REST_WAIT_VAL (0x2 << 20) #define EN_FEW_WAIT_VAL(0x8 << 16) @@ -64,18 +69,43 @@ static int gdsc_hwctrl(struct gdsc *sc, bool en) return regmap_update_bits(sc->regmap, sc->gdscr, HW_CONTROL_MASK, val); } +static int gdsc_is_enabled_by_poll_cfg_reg(struct gdsc *sc, bool en) +{ + u32 val; + int ret; + + ret = regmap_read(sc->regmap, sc->gdscr + CFG_GDSCR_OFFSET, &val); + if (ret) + return ret; + + if (en) + return !!(val & GDSC_POWER_UP_COMPLETE); + + return !(val & GDSC_POWER_DOWN_COMPLETE); +} + static int gdsc_poll_status(struct gdsc *sc, unsigned int reg, bool en) { ktime_t start; start = ktime_get(); do { - if (gdsc_is_enabled(sc, reg) == en) - return 0; + if (sc->flags & POLL_CFG_GDSCR) { + if (gdsc_is_enabled_by_poll_cfg_reg(sc, en) == en) + return 0; + } else { + if (gdsc_is_enabled(sc, reg) == en) + return 0; + } } while (ktime_us_delta(ktime_get(), start) < TIMEOUT_US); - if (gdsc_is_enabled(sc, reg) == en) - return 0; + if (sc->flags & POLL_CFG_GDSCR) { + if (gdsc_is_enabled_by_poll_cfg_reg(sc, en) == en) + return 0; + } else { + if (gdsc_is_enabled(sc, reg) == en) + return 0; + } return -ETIMEDOUT; } @@ -254,6 +284,7 @@ static int gdsc_disable(struct generic_pm_domain *domain) udelay(1); reg = sc->gds_hw_ctrl ? sc->gds_hw_ctrl : sc->gdscr; + ret = gdsc_poll_status(sc, reg, true); if (ret) return ret; diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h index 9279278..0f992e8 100644 --- a/drivers/clk/qcom/gdsc.h +++ b/drivers/clk/qcom/gdsc.h @@ -55,6 +55,7 @@ struct gdsc { #define HW_CTRLBIT(2) #define SW_RESET BIT(3) #define AON_RESET BIT(4) +#define POLL_CFG_GDSCR BIT(5) struct reset_controller_dev *rcdev; unsigned int*resets; unsigned intreset_count; -- Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member of the Code Aurora Forum, hosted by the Linux Foundation.
[PATCH v2 0/3] Update reset and poll logic for GDSCs
[v2] * Addressed review comments given in v1 series This series implements the below logic for the GDSCs 1. logic to reset the AON logic before or assert/deassert the block control reset removing the clamp io for few GDSCs on SDM845 SoC. 2. It also introduces the requirement to poll for higher timeout values for few of the GDSCs. 3. There is a new poll register for the GDSCs on SDM845 SoCs which needs to be polled for the correct hardware status of the GDSCs. Amit Nischal (3): clk: qcom: gdsc: Add support to reset AON and block reset logic clk: qcom: gdsc: Add support to poll for higher timeout value clk: qcom: gdsc: Add support to poll CFG register to check GDSC state drivers/clk/qcom/gdsc.c | 63 +++-- drivers/clk/qcom/gdsc.h | 5 +++- 2 files changed, 60 insertions(+), 8 deletions(-) -- Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member of the Code Aurora Forum, hosted by the Linux Foundation.
Re: [RFC 3/3] drm: Add helper to validate damage during modeset_check
On Thu, Apr 05, 2018 at 11:55:29PM +, Deepak Singh Rawat wrote: > > > > On Wed, Apr 04, 2018 at 04:49:08PM -0700, Deepak Rawat wrote: > > > This patch adds a helper which should be called by driver which enable > > > damage (by calling drm_plane_enable_damage_clips) from atomic_check > > > hook. This helper for now set the damage to NULL for the planes on crtc > > > which need full modeset. > > > > > > The driver also need to check for other crtc properties which can > > > affect damage in atomic_check hook, like gamma, ctm, etc. Plane related > > > properties which affect damage can be handled in damage iterator. > > > > > > Signed-off-by: Deepak Rawat > > > --- > > > drivers/gpu/drm/drm_atomic_helper.c | 47 > > + > > > include/drm/drm_atomic_helper.h | 2 ++ > > > 2 files changed, 49 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > > b/drivers/gpu/drm/drm_atomic_helper.c > > > index 355b514..23f44ab 100644 > > > --- a/drivers/gpu/drm/drm_atomic_helper.c > > > +++ b/drivers/gpu/drm/drm_atomic_helper.c > > > @@ -3987,3 +3987,50 @@ drm_atomic_helper_damage_iter_next(struct > > drm_atomic_helper_damage_iter *iter, > > > return true; > > > } > > > EXPORT_SYMBOL(drm_atomic_helper_damage_iter_next); > > > + > > > +/** > > > + * drm_atomic_helper_check_damage - validate state object for damage > > changes > > > + * @dev: DRM device > > > + * @state: the driver state object > > > + * > > > + * Check the state object if damage is required or not. In case damage is > > not > > > + * required e.g. need modeset, the damage blob is set to NULL. > > > > Why is that needed? > > > > I can imagine that drivers unconditionally upload everything for a > > modeset, and hence don't need special damage tracking. But for that it's > > imo better to have a clear_damage() helper. > > Don't we need a generic helper which all drivers can use to see if anything > in drm_atomic_state will result in full update? My intention for calling that > function from atomic_check hook was because state access can > return -EDEADLK. > > I think for now having drm_damage_helper_clear_damage helper and > calling it from atomic_check seems better option. In future once I have > clear idea, a generic function can be done. Yeah, if some of the future helpers need to e.g. allocate memory, then we need to do any necessary prep steps from ->atomic_check. But this isn't just prep, it clears stuff, so giving it a name that indicates better what it does seems like a good idea to me. Just make it clear that this should be called from ->atomic_check callbacks. > > But even for modesets (e.g. resolution changes) I'd expect that virtual > > drivers don't want to upload unecessary amounts of data. Manual upload > > hw drivers probably need to upload everything, because the panel will have > > forgotten all the old data once power cycled. > > AFAIK current vmwgfx will do full upload for resolution change. > > > > > And if you think this is really the right thing, then we need to rename > > the function to tell what it does, e.g. > > > > drm_damage_helper_clear_damage_on_modeset() > > > > drm_damage_helper because I think we should stuff this all into > > drm_damage_helper.c, see previous patch. > > > > But I think a > > > > drm_damage_helper_clear_damage(crtc_state) > > > > which you can use in your crtc->atomic_check function like > > > > crtc_atomic_check(state) > > { > > if (drm_atomic_crtc_needs_modeset(state)) > > drm_damage_helper_clear_damage(state); > > } > > > > is more flexible and useful for drivers. There might be other cases where > > clearing damage is the right thing to do. Also, there's the question of > > whether no damage clips == full damage or not, so maybe we should call > > this helper full_damage() instead. > > In my opinion if via proper documentation it is conveyed that no damage > means full-update the clear_damage makes sense. Documentation is the worst documentation. Function names, or just outright implemented behaviour is much better (e.g. runtime checks). For full damage if there's no clip rect I think the iterator should implement that for us. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[RESEND PATCH v3 2/4] dt-bindings: pwm-backlight: add a num-interpolation-steps property.
The num-interpolated-steps property specifies the number of interpolated steps between each value of brightness-level table. This is useful for high resolution PWMs to not have to list out every possible value in the brightness-level array. Signed-off-by: Enric Balletbo i Serra Acked-by: Daniel Thompson Reviewed-by: Rob Herring --- .../bindings/leds/backlight/pwm-backlight.txt | 19 +++ 1 file changed, 19 insertions(+) diff --git a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt index 310810906613..ce9b5746b774 100644 --- a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt +++ b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt @@ -21,6 +21,11 @@ Optional properties: and enabling the backlight using GPIO. - pwm-off-delay-ms: Delay in ms between disabling the backlight using GPIO and setting PWM value to 0. + - num-interpolated-steps: Number of interpolated steps between each value +of brightness-levels table. This way a high +resolution pwm duty cycle can be used without +having to list out every possible value in the +brightness-level array. [0]: Documentation/devicetree/bindings/pwm/pwm.txt [1]: Documentation/devicetree/bindings/gpio/gpio.txt @@ -39,3 +44,17 @@ Example: post-pwm-on-delay-ms = <10>; pwm-off-delay-ms = <10>; }; + +Example using num-interpolation-steps: + + backlight { + compatible = "pwm-backlight"; + pwms = <&pwm 0 500>; + + brightness-levels = <0 2048 4096 8192 16384 65535>; + num-interpolated-steps = <2048>; + default-brightness-level = <4096>; + + power-supply = <&vdd_bl_reg>; + enable-gpios = <&gpio 58 0>; + }; -- 2.16.3
[PATCH v2 2/3] clk: qcom: gdsc: Add support to poll for higher timeout value
From: Amit Nischal For some gdscs, it might take longer time up to 500us for updating their status. Update the timeout value for all GDSC polling status. Signed-off-by: Amit Nischal Signed-off-by: Taniya Das --- drivers/clk/qcom/gdsc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c index 266fefa..cb61c15 100644 --- a/drivers/clk/qcom/gdsc.c +++ b/drivers/clk/qcom/gdsc.c @@ -41,7 +41,7 @@ #define RETAIN_MEM BIT(14) #define RETAIN_PERIPH BIT(13) -#define TIMEOUT_US 100 +#define TIMEOUT_US 500 #define domain_to_gdsc(domain) container_of(domain, struct gdsc, pd) -- Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member of the Code Aurora Forum, hosted by the Linux Foundation.
Re: [PATCH 2/2] crypto: ccree: enable support for hardware keys
On Tue, Apr 3, 2018 at 1:19 PM, Herbert Xu wrote: > On Sat, Mar 31, 2018 at 08:30:46PM +0300, Gilad Ben-Yossef wrote: >> >> However, as it uses the exact same mechanism of the regular xts-aes-ccree >> but takes the key from another source, I've marked it with a test of >> alg_test_null() on the premise that if the xts-aes-ccree works, so must this. > > Well it appears to be stubbed out as cc_is_hw_key always returns > false. Please look again. The stub version of cc_is_hw_key() doing that is being replaced in this patch. >> > Are there other crypto drivers doing this? >> >> I thought the exact same thing until I ran into a presentation about the s390 >> secure keys implementation. I basically imitated their use (or abuse?) >> of the Crypto API >> assuming it is the way to go. >> >> Take a look at arch/s390/crypto/paes_s390.c > > It's always nice to discover code that was never reviewed. :-) > > The general approach seems sane though. > >> Anyway, if this is not the way to go I'd be more than happy to do whatever >> work is needed to create the right interface. >> >> PS. I'd be out of the office and away from email access to the coming week, >> so >> kindly excuse any delay in response. > > I think it's fine. However, I don't really like the idea of everyone > coming up with their own "paes", i.e., your patch's use of "haes". > It should be OK to just use paes for everyone, no? The s390 key and the cryptocell keys are not the same: Their is, I believe, is an AES key encrypted by some internal key/algorithm. The cryptocell "key" is a token, which is internally comprised of one or two indexes, referencing slots in the internal memory in the hardware, and a key size, that describe the size of the key. I thought it would be confusing to use "paes" to describe both, since they are not interchangeable. You would not be able to feed an paes key that works with the s390 version to cryptocell and vice verse and get it work. Having said, if you prefer to have "paes" simply designate "implementation specific token for an AES key" I'm perfectly fine with that. > > As to your patch specifically, there is one issue where you're > directly dereferencing the key as a struct. This is a no-no because > the key may have come from user-space. You must treat it as a > binary blob. The s390 code seems to do this correctly. > As noted above, the haes "key" is really a token encoding 3 different pieces of information: 1. The index of the first key slot to use 2. Potentially, the index of a 2nd key slot to use (for XTS) 3. a key size, denoting the actual key size used, not the slot number I than have to parse this information encoded in the token and feed it to the HW (via 3 different registers) Therefore, I do not see how I can avoid parsing the token. I hope I've managed to explain things better now. Thanks, Gilad -- Gilad Ben-Yossef Chief Coffee Drinker "If you take a class in large-scale robotics, can you end up in a situation where the homework eats your dog?" -- Jean-Baptiste Queru
[PATCH v2 1/3] clk: qcom: gdsc: Add support to reset AON and block reset logic
From: Amit Nischal For some of the gdsc power domains, there could be need to reset the AON logic or assert/deassert the block control reset before removing the clamp_io. Add support for the same by introducing new flags SW_RESET and AON_RESET. Both SW reset and AON reset requires to be asserted for at least 1us before being de-asserted. Signed-off-by: Taniya Das Signed-off-by: Amit Nischal --- drivers/clk/qcom/gdsc.c | 22 -- drivers/clk/qcom/gdsc.h | 4 +++- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c index a4f3580..266fefa 100644 --- a/drivers/clk/qcom/gdsc.c +++ b/drivers/clk/qcom/gdsc.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, The Linux Foundation. All rights reserved. + * Copyright (c) 2015, 2017-2018, The Linux Foundation. All rights reserved. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 and @@ -31,6 +31,7 @@ #define HW_CONTROL_MASKBIT(1) #define SW_COLLAPSE_MASK BIT(0) #define GMEM_CLAMP_IO_MASK BIT(0) +#define GMEM_RESET_MASKBIT(4) /* Wait 2^n CXO cycles between all states. Here, n=2 (4 cycles). */ #define EN_REST_WAIT_VAL (0x2 << 20) @@ -166,6 +167,14 @@ static inline void gdsc_assert_clamp_io(struct gdsc *sc) GMEM_CLAMP_IO_MASK, 1); } +static inline void gdsc_assert_reset_aon(struct gdsc *sc) +{ + regmap_update_bits(sc->regmap, sc->clamp_io_ctrl, + GMEM_RESET_MASK, 1); + udelay(1); + regmap_update_bits(sc->regmap, sc->clamp_io_ctrl, + GMEM_RESET_MASK, 0); +} static int gdsc_enable(struct generic_pm_domain *domain) { struct gdsc *sc = domain_to_gdsc(domain); @@ -174,8 +183,17 @@ static int gdsc_enable(struct generic_pm_domain *domain) if (sc->pwrsts == PWRSTS_ON) return gdsc_deassert_reset(sc); - if (sc->flags & CLAMP_IO) + if (sc->flags & SW_RESET) { + gdsc_assert_reset(sc); + udelay(1); + gdsc_deassert_reset(sc); + } + + if (sc->flags & CLAMP_IO) { + if (sc->flags & AON_RESET) + gdsc_assert_reset_aon(sc); gdsc_deassert_clamp_io(sc); + } ret = gdsc_toggle_logic(sc, true); if (ret) diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h index 3964834..9279278 100644 --- a/drivers/clk/qcom/gdsc.h +++ b/drivers/clk/qcom/gdsc.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, The Linux Foundation. All rights reserved. + * Copyright (c) 2015, 2017-2018, The Linux Foundation. All rights reserved. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 and @@ -53,6 +53,8 @@ struct gdsc { #define VOTABLEBIT(0) #define CLAMP_IO BIT(1) #define HW_CTRLBIT(2) +#define SW_RESET BIT(3) +#define AON_RESET BIT(4) struct reset_controller_dev *rcdev; unsigned int*resets; unsigned intreset_count; -- Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member of the Code Aurora Forum, hosted by the Linux Foundation.
Re: [PATCH 2/3] clk: qcom: gdsc: Add support to poll for higher timeout value
Hello Stephen, Thanks for the review comments. On 4/6/2018 4:54 AM, Stephen Boyd wrote: Quoting Taniya Das (2018-04-02 03:45:44) From: Amit Nischal For some gdscs, it might take longer time up to 500us for updating their status. So add support for the same by defining a new flag 'GDS_TIMEOUT' to mark such gdsc in order to poll their status for longer timeout value. Signed-off-by: Amit Nischal Signed-off-by: Taniya Das --- Let's just increase the timeout to 500 if it's required? This is a timeout, so we're not really expecting to hit it anyway so optimizing the uncommon case is not useful. -- Will fix this in the next series. To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation. --
[PATCH v2] mfd: twl-core: Fix clock initialization
When looking up the clock we must use the client->dev as device since that is the one which is probed via DT. Signed-off-by: Peter Ujfalusi Cc: sta...@vger.kernel.org # 4.16+ --- Hi, Changes since v1: - Removed the Fixes line and add tag only for v4.16 to avoid possible breakage in pre v4.16. Regards, Peter drivers/mfd/twl-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c index d3133a371e27..c649344fd7f2 100644 --- a/drivers/mfd/twl-core.c +++ b/drivers/mfd/twl-core.c @@ -1177,7 +1177,7 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id) twl_priv->ready = true; /* setup clock framework */ - clocks_init(&pdev->dev, pdata ? pdata->clock : NULL); + clocks_init(&client->dev, pdata ? pdata->clock : NULL); /* read TWL IDCODE Register */ if (twl_class_is_4030()) { -- Peter Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Re: [PATCH] openrisc: define mb() as its mandatory
On Sun, Apr 08, 2018 at 09:07:29AM +0900, Stafford Horne wrote: > On Sat, Apr 07, 2018 at 11:09:05AM +0200, Peter Zijlstra wrote: > > On Sat, Apr 07, 2018 at 05:58:49AM +0900, Stafford Horne wrote: > > > Following Peter Z's patch ("asm-generic: Disallow no-op mb() for SMP > > > systems") which makes mb() mandatory for SMP architectures we define it > > > as l.msync. On OpenRISC this will flush the current cores write buffer > > > and trigger remote cores to invalidate their caches of the written > > > memory. > > > > > > Signed-off-by: Stafford Horne > > > Link: https://lkml.org/lkml/2018/1/31/254 > > > Cc: Peter Zijlstra > > > Cc: Will Deacon > > > --- > > > > > > Notes: > > > - Sorry, its been a while since we discussed this patch is the parent > > > to this > > > still going in Peter? > > > > Oops.. yes it should. It seems I also lost track of it. Thanks for the > > reminder! > > No Problem, > > If you think this patch makes sense I can just put it into my OpenRISC queue > for > 4.17. I dont see any reason to wait for yours. Any thoughts? I'm fine with you taking the patch, but I just saw the build robot found two failure cases: PARISC and 32-bit SPARC. I send patches for both, if their respective maintainers agree you could do the lot.
Re: 答复: [RFC PATCH] x86/acpi: Prevent x2apic id -1 from being accounted
RongQing, At 04/09/2018 02:38 PM, Li,Rongqing wrote: -邮件原件- 发件人: Dou Liyang [mailto:douly.f...@cn.fujitsu.com] 发送时间: 2018年4月9日 13:38 收件人: Li,Rongqing ; linux-kernel@vger.kernel.org; t...@linutronix.de; mi...@redhat.com; h...@zytor.com; jgr...@suse.com; x...@kernel.org; pet...@infradead.org 主题: Re: [RFC PATCH] x86/acpi: Prevent x2apic id -1 from being accounted Hi RongQing, Is there an local x2apic whose ID is in your machine? I think no [...] [0.00] ACPI: X2APIC (apic_id[0x] uid[0x00] disabled) [0.00] ACPI: X2APIC (apic_id[0x] uid[0x01] disabled) [0.00] ACPI: X2APIC (apic_id[0x] uid[0x02] disabled) Ah, sure enough! [...] At 04/08/2018 07:38 PM, Li RongQing wrote: local_apic_id of acpi_madt_local_x2apic is u32, it is converted to int when checked by default_apic_id_valid() and return true if it is larger than 0x7fff, this is wrong For x2apic enabled systems, - the byte length of X2APIC ID is 4, and it can be larger than 0x7fff in theory Yes - the ->apic_id_valid points to x2apic_apic_id_valid(), which always return _ture_ , not default_apic_id_valid(). To this machine, the apic is apic_flat I see, I am sorry the title and changelog make me misunderstand. Here, actually, we prevent all the X2APIC Id from being parsed in non-x2apic mode, not just 0x. because the values of x2APIC ID must be 255 and greater in ACPI MADT. -RongQing Thanks, dou and if local_apic_id is invalid, we should prevent it from being accounted > This fixes a bug that Purley platform displays too many possible cpu Signed-off-by: Li RongQing Cc: Peter Zijlstra Cc: Thomas Gleixner Cc: Dou Liyang --- arch/x86/include/asm/apic.h | 4 ++-- arch/x86/kernel/acpi/boot.c | 10 ++ arch/x86/kernel/apic/apic_common.c | 2 +- arch/x86/kernel/apic/apic_numachip.c | 2 +- arch/x86/kernel/apic/x2apic.h| 2 +- arch/x86/kernel/apic/x2apic_phys.c | 2 +- arch/x86/kernel/apic/x2apic_uv_x.c | 2 +- arch/x86/xen/apic.c | 2 +- 8 files changed, 14 insertions(+), 12 deletions(-) diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h index 40a3d3642f3a..08acd954f00e 100644 --- a/arch/x86/include/asm/apic.h +++ b/arch/x86/include/asm/apic.h @@ -313,7 +313,7 @@ struct apic { /* Probe, setup and smpboot functions */ int (*probe)(void); int (*acpi_madt_oem_check)(char *oem_id, char *oem_table_id); - int (*apic_id_valid)(int apicid); + int (*apic_id_valid)(u32 apicid); int (*apic_id_registered)(void); bool(*check_apicid_used)(physid_mask_t *map, int apicid); @@ -486,7 +486,7 @@ static inline unsigned int read_apic_id(void) return apic->get_apic_id(reg); } -extern int default_apic_id_valid(int apicid); +extern int default_apic_id_valid(u32 apicid); extern int default_acpi_madt_oem_check(char *, char *); extern void default_setup_apic_routing(void); diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c index 7a37d9357bc4..7412564dc2a7 100644 --- a/arch/x86/kernel/acpi/boot.c +++ b/arch/x86/kernel/acpi/boot.c @@ -200,7 +200,7 @@ acpi_parse_x2apic(struct acpi_subtable_header *header, const unsigned long end) { struct acpi_madt_local_x2apic *processor = NULL; #ifdef CONFIG_X86_X2APIC - int apic_id; + u32 apic_id; u8 enabled; #endif @@ -222,10 +222,12 @@ acpi_parse_x2apic(struct acpi_subtable_header *header, const unsigned long end) * to not preallocating memory for all NR_CPUS * when we use CPU hotplug. */ - if (!apic->apic_id_valid(apic_id) && enabled) + if (!apic->apic_id_valid(apic_id)) { printk(KERN_WARNING PREFIX "x2apic entry ignored\n"); Due to the disable APIC entries may not exist physically, please just printk the warning if the APIC ID is enabled. if (!apic->apic_id_valid(apic_id)) { if(enabled) printk... return 0; } Thanks, dou
Re: [PATCH 3/3] clk: qcom: gdsc: Add support to poll CFG register to check GDSC state
Hello Stephen, Thanks for the review comments. On 4/6/2018 10:10 PM, Stephen Boyd wrote: Quoting Taniya Das (2018-04-02 03:45:45) diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c index e89584e..e0c83ba 100644 --- a/drivers/clk/qcom/gdsc.c +++ b/drivers/clk/qcom/gdsc.c @@ -83,6 +88,38 @@ static int gdsc_poll_status(struct gdsc *sc, unsigned int reg, bool en) return -ETIMEDOUT; } +static int gdsc_is_enabled_by_poll_cfg_reg(struct gdsc *sc, bool en) +{ + u32 val; + int ret; + + ret = regmap_read(sc->regmap, sc->gdscr + CFG_GDSCR_OFFSET, &val); + if (ret) + return ret; + + if (en) + return !!(val & GDSC_POWER_UP_COMPLETE); + else + return !(val & GDSC_POWER_DOWN_COMPLETE); Make this into if (en) return ... return ... Will fix this in the next series. +} + +static int gdsc_poll_cfg_status(struct gdsc *sc, bool en) +{ + ktime_t start = ktime_get(); + ktime_t timeout = + (sc->flags & GDS_TIMEOUT) ? TIMEOUT_US_GDS : TIMEOUT_US; + + do { + if (gdsc_is_enabled_by_poll_cfg_reg(sc, en) == en) + return 0; + } while (ktime_us_delta(ktime_get(), start) < timeout); + + if (gdsc_is_enabled_by_poll_cfg_reg(sc, en) == en) + return 0; + + return -ETIMEDOUT; +} + static int gdsc_toggle_logic(struct gdsc *sc, bool en) { int ret; @@ -106,6 +143,9 @@ static int gdsc_toggle_logic(struct gdsc *sc, bool en) return 0; } + if (sc->flags & POLL_CFG_GDSCR) + return gdsc_poll_cfg_status(sc, en); + if (sc->gds_hw_ctrl) { status_reg = sc->gds_hw_ctrl; /* @@ -258,8 +298,12 @@ static int gdsc_disable(struct generic_pm_domain *domain) */ udelay(1); - reg = sc->gds_hw_ctrl ? sc->gds_hw_ctrl : sc->gdscr; - ret = gdsc_poll_status(sc, reg, true); + if (sc->flags & POLL_CFG_GDSCR) { + ret = gdsc_poll_cfg_status(sc, true); + } else { + reg = sc->gds_hw_ctrl ? sc->gds_hw_ctrl : sc->gdscr; + ret = gdsc_poll_status(sc, reg, true); + } Maybe this can be pushed into the gdsc_poll_status() function so that we can keep the "how do we poll status bit" logic in one place. Yes, I will push the logic in the gdsc_poll_status() in the next series. if (ret) return ret; } -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation. --
Re: [PATCH 2/2] crypto: ccree: enable support for hardware keys
On Tue, Apr 3, 2018 at 3:22 PM, Milan Broz wrote: > On 03/31/2018 07:30 PM, Gilad Ben-Yossef wrote: > ... >>> Are there other crypto drivers doing this? >> >> I thought the exact same thing until I ran into a presentation about the s390 >> secure keys implementation. I basically imitated their use (or abuse?) >> of the Crypto API >> assuming it is the way to go. >> >> Take a look at arch/s390/crypto/paes_s390.c >> >> The slide for the presentation describing this is here: >> http://schd.ws/hosted_files/ossna2017/89/LC2017SecKeyDmCryptV5.pdf >> >> And they seem to even have support for it in the DM-Crypt tools, which at >> the time they claimed to be in the process of getting it up-streamed. > > It is "in the process", but definitely not accepted. > > We are just discussing how to integrate paes wrapped keys in cryptsetup and > it will definitely not be the way presented in the slides above. > > If you plan more such ciphers, I would welcome some unified way in crypto API > how to handle these HSM keys flavors. That would be good. Note however the fine difference - the s390 usage is a wrapped key. Ours is a token for a key (a slot number really). Probably makes no difference for any practical sense, but I thought it is worth mentioning it. > > For kernel dm-crypt, there is no change needed (dmcrypt just treats it as a > normal cipher key). > (I would say that it is not the best idea either, IMHO it would be better to > use > kernel keyring reference instead and somehow handle hw keys through keyring.) > I am all for the keyring approach. In fact, that was the way I wanted to to go to introduce this feature for cryptocell when I discovered that was already upstream code using a different approach. Any suggestion how this would work vis a vis the crypto API usage? e.g. - have a parallel setkey variant added to crypto APi that takes a kernel keyring object rather than actual key? Thanks, Gilad -- Gilad Ben-Yossef Chief Coffee Drinker "If you take a class in large-scale robotics, can you end up in a situation where the homework eats your dog?" -- Jean-Baptiste Queru
RE: [PATCH v2] xen/privcmd: add IOCTL_PRIVCMD_MMAP_RESOURCE
> -Original Message- > From: Boris Ostrovsky [mailto:boris.ostrov...@oracle.com] > Sent: 05 April 2018 23:34 > To: Paul Durrant ; x...@kernel.org; xen- > de...@lists.xenproject.org; linux-kernel@vger.kernel.org > Cc: Juergen Gross ; Thomas Gleixner > ; Ingo Molnar > Subject: Re: [PATCH v2] xen/privcmd: add > IOCTL_PRIVCMD_MMAP_RESOURCE > > On 04/05/2018 11:42 AM, Paul Durrant wrote: > > My recent Xen patch series introduces a new HYPERVISOR_memory_op to > > support direct priv-mapping of certain guest resources (such as ioreq > > pages, used by emulators) by a tools domain, rather than having to access > > such resources via the guest P2M. > > > > This patch adds the necessary infrastructure to the privcmd driver and > > Xen MMU code to support direct resource mapping. > > > > NOTE: The adjustment in the MMU code is partially cosmetic. Xen will now > > allow a PV tools domain to map guest pages either by GFN or MFN, thus > > the term 'mfn' has been swapped for 'pfn' in the lower layers of the > > remap code. > > > > Signed-off-by: Paul Durrant > > --- > > Cc: Boris Ostrovsky > > Cc: Juergen Gross > > Cc: Thomas Gleixner > > Cc: Ingo Molnar > > > > v2: > > - Fix bug when mapping multiple pages of a resource > > > Only a few nits below. > > > --- > > arch/x86/xen/mmu.c | 50 +++- > > drivers/xen/privcmd.c | 130 > + > > include/uapi/xen/privcmd.h | 11 > > include/xen/interface/memory.h | 66 + > > include/xen/interface/xen.h| 7 ++- > > include/xen/xen-ops.h | 24 +++- > > 6 files changed, 270 insertions(+), 18 deletions(-) > > > > diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c > > index d33e7dbe3129..8453d7be415c 100644 > > --- a/arch/x86/xen/mmu.c > > +++ b/arch/x86/xen/mmu.c > > @@ -65,37 +65,42 @@ static void xen_flush_tlb_all(void) > > #define REMAP_BATCH_SIZE 16 > > > > struct remap_data { > > - xen_pfn_t *mfn; > > + xen_pfn_t *pfn; > > bool contiguous; > > + bool no_translate; > > pgprot_t prot; > > struct mmu_update *mmu_update; > > }; > > > > -static int remap_area_mfn_pte_fn(pte_t *ptep, pgtable_t token, > > +static int remap_area_pfn_pte_fn(pte_t *ptep, pgtable_t token, > > unsigned long addr, void *data) > > { > > struct remap_data *rmd = data; > > - pte_t pte = pte_mkspecial(mfn_pte(*rmd->mfn, rmd->prot)); > > + pte_t pte = pte_mkspecial(mfn_pte(*rmd->pfn, rmd->prot)); > > > > /* If we have a contiguous range, just update the mfn itself, > >else update pointer to be "next mfn". */ > > This probably also needs to be updated (and while at it, comment style > fixed) > Ok. > > if (rmd->contiguous) > > - (*rmd->mfn)++; > > + (*rmd->pfn)++; > > else > > - rmd->mfn++; > > + rmd->pfn++; > > > > - rmd->mmu_update->ptr = virt_to_machine(ptep).maddr | > MMU_NORMAL_PT_UPDATE; > > + rmd->mmu_update->ptr = virt_to_machine(ptep).maddr; > > + rmd->mmu_update->ptr |= rmd->no_translate ? > > + MMU_PT_UPDATE_NO_TRANSLATE : > > + MMU_NORMAL_PT_UPDATE; > > rmd->mmu_update->val = pte_val_ma(pte); > > rmd->mmu_update++; > > > > return 0; > > } > > > > -static int do_remap_gfn(struct vm_area_struct *vma, > > +static int do_remap_pfn(struct vm_area_struct *vma, > > unsigned long addr, > > - xen_pfn_t *gfn, int nr, > > + xen_pfn_t *pfn, int nr, > > int *err_ptr, pgprot_t prot, > > - unsigned domid, > > + unsigned int domid, > > + bool no_translate, > > struct page **pages) > > { > > int err = 0; > > @@ -106,11 +111,12 @@ static int do_remap_gfn(struct vm_area_struct > *vma, > > > > BUG_ON(!((vma->vm_flags & (VM_PFNMAP | VM_IO)) == > (VM_PFNMAP | VM_IO))); > > > > - rmd.mfn = gfn; > > + rmd.pfn = pfn; > > rmd.prot = prot; > > /* We use the err_ptr to indicate if there we are doing a contiguous > > * mapping or a discontigious mapping. */ > > Style. > I'm not modifying this comment but I'll fix it. > > rmd.contiguous = !err_ptr; > > + rmd.no_translate = no_translate; > > > > while (nr) { > > int index = 0; > > @@ -121,7 +127,7 @@ static int do_remap_gfn(struct vm_area_struct > *vma, > > > > rmd.mmu_update = mmu_update; > > err = apply_to_page_range(vma->vm_mm, addr, range, > > - remap_area_mfn_pte_fn, &rmd); > > + remap_area_pfn_pte_fn, &rmd); > > if (err) > > goto out; > > > > @@ -175,7 +181,8 @@ int xen_remap_domain_gfn_range(struct > vm_area_struct *vma, > > if (xen_feature(XENFEAT_auto_translated_physmap)) > > return -EOPNOTSUPP; > > > > - return do_re
Re: [PATCH] sched/fair: schedutil: update only with all info available
Hi Patrick On 6 April 2018 at 19:28, Patrick Bellasi wrote: > Schedutil is not properly updated when the first FAIR task wakes up on a > CPU and when a RQ is (un)throttled. This is mainly due to the current > integration strategy, which relies on updates being triggered implicitly > each time a cfs_rq's utilization is updated. > > Those updates are currently provided (mainly) via >cfs_rq_util_change() > which is used in: > - update_cfs_rq_load_avg() >when the utilization of a cfs_rq is updated > - {attach,detach}_entity_load_avg() > This is done based on the idea that "we should callback schedutil > frequently enough" to properly update the CPU frequency at every > utilization change. > > Since this recent schedutil update: > > commit 8f111bc357aa ("cpufreq/schedutil: Rewrite CPUFREQ_RT support") > > we use additional RQ information to properly account for FAIR tasks > utilization. Specifically, cfs_rq::h_nr_running has to be non-zero > in sugov_aggregate_util() to sum up the cfs_rq's utilization. Isn't the use of cfs_rq::h_nr_running, the root cause of the problem ? I can now see a lot a frequency changes on my hikey with this new condition in sugov_aggregate_util(). With a rt-app UC that creates a periodic cfs task, I have a lot of frequency changes instead of staying at the same frequency Peter, what was your goal with adding the condition "if (rq->cfs.h_nr_running)" for the aggragation of CFS utilization Thanks Vincent > > However, cfs_rq::h_nr_running is usually updated as: > > enqueue_entity() >... >update_load_avg() > ... > cfs_rq_util_change ==> trigger schedutil update > ... > cfs_rq->h_nr_running += number_of_tasks > > both in enqueue_task_fair() as well as in unthrottle_cfs_rq(). > A similar pattern is used also in dequeue_task_fair() and > throttle_cfs_rq() to remove tasks. > > This means that we are likely to see a zero cfs_rq utilization when we > enqueue a task on an empty CPU, or a non-zero cfs_rq utilization when > instead, for example, we are throttling all the FAIR tasks of a CPU. > > While the second issue is less important, since we are less likely to > reduce frequency when CPU utilization decreases, the first issue can > instead impact performance. Indeed, we potentially introduce a not desired > latency between a task enqueue on a CPU and its frequency increase. > > Another possible unwanted side effect is the iowait boosting of a CPU > when we enqueue a task into a throttled cfs_rq. > > Moreover, the current schedutil integration has these other downsides: > > - schedutil updates are triggered by RQ's load updates, which makes >sense in general but it does not allow to know exactly which other RQ >related information has been updated (e.g. h_nr_running). > > - increasing the chances to update schedutil does not always correspond >to provide the most accurate information for a proper frequency >selection, thus we can skip some updates. > > - we don't know exactly at which point a schedutil update is triggered, >and thus potentially a frequency change started, and that's because >the update is a side effect of cfs_rq_util_changed instead of an >explicit call from the most suitable call path. > > - cfs_rq_util_change() is mainly a wrapper function for an already >existing "public API", cpufreq_update_util(), to ensure we actually >update schedutil only when we are updating a root RQ. Thus, especially >when task groups are in use, most of the calls to this wrapper >function are really not required. > > - the usage of a wrapper function is not completely consistent across >fair.c, since we still need sometimes additional explicit calls to >cpufreq_update_util(), for example to support the IOWAIT boot flag in >the wakeup path > > - it makes it hard to integrate new features since it could require to >change other function prototypes just to pass in an additional flag, >as it happened for example here: > > commit ea14b57e8a18 ("sched/cpufreq: Provide migration hint") > > All the above considered, let's try to make schedutil updates more > explicit in fair.c by: > > - removing the cfs_rq_util_change() wrapper function to use the >cpufreq_update_util() public API only when root cfs_rq is updated > > - remove indirect and side-effect (sometimes not required) schedutil >updates when the cfs_rq utilization is updated > > - call cpufreq_update_util() explicitly in the few call sites where it >really makes sense and all the required information has been updated > > By doing so, this patch mainly removes code and adds explicit calls to > schedutil only when we: > - {enqueue,dequeue}_task_fair() a task to/from the root cfs_rq > - (un)throttle_cfs_rq() a set of tasks up to the root cfs_rq > - task_tick_fair() to update the utilization of the root cfs_rq > > All the other code paths, currently _indirectly_ covered by a call to > update_load_
Re: [PATCH v2 11/21] stack-protector: test compiler capability in Kconfig and drop AUTO mode
2018-03-28 20:18 GMT+09:00 Kees Cook : > On Mon, Mar 26, 2018 at 10:29 PM, Masahiro Yamada > wrote: >> Move the test for -fstack-protector(-strong) option to Kconfig. >> >> If the compiler does not support the option, the corresponding menu >> is automatically hidden. If _STRONG is not supported, it will fall >> back to _REGULAR. If _REGULAR is not supported, it will be disabled. >> This means, _AUTO is implicitly handled by the dependency solver of >> Kconfig, hence removed. >> >> I also turned the 'choice' into only two boolean symbols. The use of >> 'choice' is not a good idea here, because all of all{yes,mod,no}config >> would choose the first visible value, while we want allnoconfig to >> disable as many features as possible. >> >> X86 has additional shell scripts in case the compiler supports the >> option, but generates broken code. I added CC_HAS_SANE_STACKPROTECTOR >> to test this. I had to add -m32 to gcc-x86_32-has-stack-protector.sh >> to make it work correctly. >> >> Signed-off-by: Masahiro Yamada > > This looks really good. Notes below... > >> --- >> >> Changes in v2: >> - Describe $(cc-option ...) directly in depends on context >> >> Makefile | 93 >> ++- >> arch/Kconfig | 29 +++--- >> arch/x86/Kconfig | 8 ++- >> scripts/gcc-x86_32-has-stack-protector.sh | 7 +-- >> scripts/gcc-x86_64-has-stack-protector.sh | 5 -- >> 5 files changed, 22 insertions(+), 120 deletions(-) >> >> diff --git a/Makefile b/Makefile >> index 5c395ed..5cadffa 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -675,55 +675,11 @@ ifneq ($(CONFIG_FRAME_WARN),0) >> KBUILD_CFLAGS += $(call cc-option,-Wframe-larger-than=${CONFIG_FRAME_WARN}) >> endif >> >> -# This selects the stack protector compiler flag. Testing it is delayed >> -# until after .config has been reprocessed, in the prepare-compiler-check >> -# target. >> -ifdef CONFIG_CC_STACKPROTECTOR_AUTO >> - stackp-flag := $(call cc-option,-fstack-protector-strong,$(call >> cc-option,-fstack-protector)) >> - stackp-name := AUTO >> -else >> -ifdef CONFIG_CC_STACKPROTECTOR_REGULAR >> - stackp-flag := -fstack-protector >> - stackp-name := REGULAR >> -else >> -ifdef CONFIG_CC_STACKPROTECTOR_STRONG >> - stackp-flag := -fstack-protector-strong >> - stackp-name := STRONG >> -else >> - # If either there is no stack protector for this architecture or >> - # CONFIG_CC_STACKPROTECTOR_NONE is selected, we're done, and >> $(stackp-name) >> - # is empty, skipping all remaining stack protector tests. >> - # >> - # Force off for distro compilers that enable stack protector by default. >> - KBUILD_CFLAGS += $(call cc-option, -fno-stack-protector) >> -endif >> -endif >> -endif >> -# Find arch-specific stack protector compiler sanity-checking script. >> -ifdef stackp-name >> -ifneq ($(stackp-flag),) >> - stackp-path := >> $(srctree)/scripts/gcc-$(SRCARCH)_$(BITS)-has-stack-protector.sh >> - stackp-check := $(wildcard $(stackp-path)) >> - # If the wildcard test matches a test script, run it to check >> functionality. >> - ifdef stackp-check >> -ifneq ($(shell $(CONFIG_SHELL) $(stackp-check) $(CC) $(KBUILD_CPPFLAGS) >> $(biarch)),y) >> - stackp-broken := y >> -endif >> - endif >> - ifndef stackp-broken >> -# If the stack protector is functional, enable code that depends on it. >> -KBUILD_CPPFLAGS += -DCONFIG_CC_STACKPROTECTOR >> -# Either we've already detected the flag (for AUTO) or we'll fail the >> -# build in the prepare-compiler-check rule (for specific flag). >> -KBUILD_CFLAGS += $(stackp-flag) >> - else >> -# We have to make sure stack protector is unconditionally disabled if >> -# the compiler is broken (in case we're going to continue the build in >> -# AUTO mode). > > Let's keep this comment (slightly rewritten) since the reason for > setting this flag isn't obvious. Will move this to help of arch/x86/Kconfig >> -KBUILD_CFLAGS += $(call cc-option, -fno-stack-protector) >> - endif >> -endif >> -endif >> +stackp-flags-y := -fno-stack-protector > > This is a (minor?) regression in my testing. Making this unconditional > may break for a compiler built without stack-protector. It should be > rare, but it's technically possible. Perhaps: > > stackp-flags-y := ($call cc-option, -fno-stack-protector) Will add CONFIG_CC_HAS_STACKPROTECTOR_NONE >> +stackp-flags-$(CONFIG_CC_STACKPROTECTOR) := -fstack-protector >> +stackp-flags-$(CONFIG_CC_STACKPROTECTOR_STRONG):= >> -fstack-protector-strong >> + >> +KBUILD_CFLAGS += $(stackp-flags-y) >> [...] >> diff --git a/arch/Kconfig b/arch/Kconfig >> index 8e0d665..b42378d 100644 >> --- a/arch/Kconfig >> +++ b/arch/Kconfig >> @@ -535,13 +535,13 @@ config HAVE_CC_STACKPROTECTOR >> bool >> help >> An arch should select this symbol if: >> - - its compiler supports t
Re: 答复: Re: 答复: Re: [PATCH v2] scsi: Introduce sdev_printk_ratelimited tothrottlefrequent printk
On Mon 2018-04-09 10:13:43, wen.yan...@zte.com.cn wrote: > That's a good idea, but it only solves part of the problem. > loopping printks under spinlock, there's two path: > one path is: > scsi_request_fn --> loop -> blk_peek_request-> scsi_prep_fn -> > scsi_prep_state_check -> sdev_printk > another path is: > scsi_request_fn --> loop -> sdev_printk Is this message redundant? It seems to be printed in the same situations as the messages in scsi_prep_state_check(). In fact, there seems to be a mismatch. scsi_request_fn() prints about offline device also for sdev->sdev_state == SDEV_DEL. While scsi_prep_state_check() prints about a dead device in this case. Would it make sense to remove the redundant dev_printk() from scsi_request_fn()? Then we could add a flag into struct scsi_device that would remember if we already printed the error in scsi_prep_state_check() for the given device. It could be used to print the error only once. The flag might be scsi_device_state value for which we printed the error last time. We would need to reset it scsi_prep_state_check() see another state. It is a bit hairy but it really does not make much sense to print the same error message thousand times. Best Regards, Petr PS: Your mail was again strangely formatted. Please, send mails in plain text format (no html).
Re: [PATCH v2 1/3] stacktrace: move arch_within_stack_frames from thread_info.h
Hi Sahara, Thank you for the patch! Yet something to improve: [auto build test ERROR on linus/master] [also build test ERROR on v4.16 next-20180406] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/kpark3469-gmail-com/usercopy-reimplement-arch_within_stack_frames/20180409-144349 config: x86_64-randconfig-x013-201814 (attached as .config) compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 reproduce: # save the attached .config to linux build tree make ARCH=x86_64 All errors (new ones prefixed by >>): from include/linux/uaccess.h:5, from arch/x86/include/asm/stacktrace.h:10, from include/linux/stacktrace.h:6, from include/linux/lockdep.h:29, from include/linux/spinlock_types.h:18, from kernel/bounds.c:14: include/linux/spinlock.h:297:24: error: unknown type name 'raw_spinlock_t' static __always_inline raw_spinlock_t *spinlock_check(spinlock_t *lock) ^~ include/linux/spinlock.h:297:55: error: unknown type name 'spinlock_t'; did you mean 'clock_t'? static __always_inline raw_spinlock_t *spinlock_check(spinlock_t *lock) ^~ clock_t include/linux/spinlock.h:308:39: error: unknown type name 'spinlock_t'; did you mean 'clock_t'? static __always_inline void spin_lock(spinlock_t *lock) ^~ clock_t include/linux/spinlock.h:313:42: error: unknown type name 'spinlock_t'; did you mean 'clock_t'? static __always_inline void spin_lock_bh(spinlock_t *lock) ^~ clock_t include/linux/spinlock.h:318:41: error: unknown type name 'spinlock_t'; did you mean 'clock_t'? static __always_inline int spin_trylock(spinlock_t *lock) ^~ clock_t include/linux/spinlock.h:333:43: error: unknown type name 'spinlock_t'; did you mean 'clock_t'? static __always_inline void spin_lock_irq(spinlock_t *lock) ^~ clock_t include/linux/spinlock.h:348:41: error: unknown type name 'spinlock_t'; did you mean 'clock_t'? static __always_inline void spin_unlock(spinlock_t *lock) ^~ clock_t include/linux/spinlock.h:353:44: error: unknown type name 'spinlock_t'; did you mean 'clock_t'? static __always_inline void spin_unlock_bh(spinlock_t *lock) ^~ clock_t include/linux/spinlock.h:358:45: error: unknown type name 'spinlock_t'; did you mean 'clock_t'? static __always_inline void spin_unlock_irq(spinlock_t *lock) ^~ clock_t include/linux/spinlock.h:363:52: error: unknown type name 'spinlock_t'; did you mean 'clock_t'? static __always_inline void spin_unlock_irqrestore(spinlock_t *lock, unsigned long flags) ^~ clock_t include/linux/spinlock.h:368:44: error: unknown type name 'spinlock_t'; did you mean 'clock_t'? static __always_inline int spin_trylock_bh(spinlock_t *lock) ^~ clock_t include/linux/spinlock.h:373:45: error: unknown type name 'spinlock_t'; did you mean 'clock_t'? static __always_inline int spin_trylock_irq(spinlock_t *lock) ^~ clock_t include/linux/spinlock.h:383:43: error: unknown type name 'spinlock_t'; did you mean 'clock_t'? static __always_inline int spin_is_locked(spinlock_t *lock) ^~ clock_t include/linux/spinlock.h:388:46: error: unknown type name 'spinlock_t'; did you mean 'clock_t'? static __always_inline int spin_is_contended(spinlock_t *lock)
Re: [PATCH v2] locking/hung_task: Show all hung tasks before panic
On Sat, Apr 7, 2018 at 6:24 PM, Tetsuo Handa wrote: > Dmitry Vyukov wrote: >> On Sat, Apr 7, 2018 at 5:39 PM, Peter Zijlstra wrote: >> > On Sat, Apr 07, 2018 at 09:31:19PM +0900, Tetsuo Handa wrote: >> >> are for replacing debug_show_all_locks() in check_hung_task() for cases >> >> like >> >> https://syzkaller.appspot.com/bug?id=26aa22915f5e3b7ca2cfca76a939f12c25d624db >> >> because we are interested in only threads holding locks. >> >> >> >> SysRq-t is too much but SysRq-w is useless for killable/interruptible >> >> threads... >> > >> > Or use a script to process the sysrq-t output? I mean, we can add all >> > sorts, but where does it end? > > Maybe allow khungtaskd to call call_usermode_helper() to run arbitrary > operations > instead of just calling panic()? This would probably work for syzbot too. >> Good question. >> We are talking about few dozen more stacks, right? >> >> Not all kernel bugs are well reproducible, so it's not always possible >> to go back and hit sysrq-t. And this come up in the context of syzbot, >> which is an automated system. It reported a bunch of hangs and most of >> them are real bugs, but not all of them are easily actionable. >> Can it be a config or a command line argument, which will make syzbot >> capture more useful context for each such hang? >> > > It will be nice if syzbot testing is done with kdump configured, and the > result of automated scripting on vmcore (such as "foreach bt -s -l") is > available. kdump's popped up several times already (https://github.com/google/syzkaller/issues/491). But this will require some non-trivial amount of work to pipe it through the whole system (starting from investigation/testing, second kernel to storing them and exposing).
Re: [PATCH v2 2/2] mailbox: add STMicroelectronics STM32 IPCC driver
On 06/04/18 18:20, Jassi Brar wrote: > On Fri, Apr 6, 2018 at 8:35 PM, Fabien DESSENNE > wrote: >> On 06/04/18 14:56, Jassi Brar wrote: >>> On Fri, Apr 6, 2018 at 5:59 PM, Fabien DESSENNE >>> wrote: Hi On 05/04/18 11:38, Jassi Brar wrote: > On Mon, Mar 12, 2018 at 4:28 PM, Fabien Dessenne > wrote: > >> + >> + /* irq */ >> + for (i = 0; i < IPCC_IRQ_NUM; i++) { >> + ipcc->irqs[i] = of_irq_get_byname(dev->of_node, >> irq_name[i]); >> + if (ipcc->irqs[i] < 0) { >> + dev_err(dev, "no IRQ specified %s\n", >> irq_name[i]); >> + ret = ipcc->irqs[i]; >> + goto err_clk; >> + } >> + >> + ret = devm_request_threaded_irq(dev, ipcc->irqs[i], NULL, >> + irq_thread[i], >> IRQF_ONESHOT, >> + dev_name(dev), ipcc); >> > In your interrupt handlers you don't do anything that could block. > Threads only adds some delay to your message handling. > So maybe use devm_request_irq() ? The interrupt handlers call mbox_chan_received_data() / mbox_chan_txdone(), which call in turn client's rx_callback() / tx_done() / tx_prepare() which behavior may be unsafe. Hence, using a threaded irq here seems to be a good choice. >>> rx_callback() is supposed to be atomic. >> I am worried with this atomic part (and honestly I did not note that the >> callbacks were expected to be) >> >> In my case, remoteproc->virtio->rpmsg is the mailbox client defining the >> rx_callback. >> If I follow your suggestion, I shall make this rx_callback Atomic in >> remoteproc (or in virtio or rpmsg). And this does not seem to be so >> simple (add a worker in the middle of somewhere?). Bjorn, feel free to >> comment this part. >> >> An alternate implementation consists in using a threaded IRQ for the >> mailbox interrupt. >> This option is not only simple, but also ensures to split bottom & half >> parts at the irq level which is IMHO a general good practice. >> >> I can see that some mailbox clients implement callbacks that are NOT >> atomic and I suspect this is the reason why some mailbox drivers use >> threaded_irq (rockchip mailbox splits the bottom & half parts). >> >> Would it be acceptable to consider the "atomic client callback" as a >> non-strict rule ? >> > Of course you can traverse atomic path from sleepable context (but not > vice-versa). So, to be sure we understand each other, I can use threaded_irq, right? > Please send in the final revision. > > Thanks.
Linux 4.9.93
Hi, After this patchset, a kernel built with CFI fails. Disabling UNMAP_KERNEL_AT_EL0 fix the issue obviously. Wondering if there is one of the test suite used on the review patchset that covers the CFI usecase. Best regards, [0.249191] CPU features: detected feature: GIC system register CPU interface [0.256391] CPU features: detected feature: Privileged Access Never [0.262719] CPU features: detected feature: User Access Override [0.268791] CPU features: detected feature: 32-bit EL0 Support [0.274683] CPU features: detected feature: Kernel page table isolation (KPTI) [0.282166] CFI failure: [0.282169] CFI failure: [0.282172] CFI failure: [0.282173] CFI failure: [0.282175] CFI failure: [0.282176] CFI failure: [0.282177] CFI failure: [0.282178] CFI failure: [0.282188] [ cut here ] [0.282189] [ cut here ] [0.282190] [ cut here ] [0.282191] [ cut here ] [0.282193] [ cut here ] [0.282196] kernel BUG at kernel/cfi.c:32! [0.282198] [ cut here ] [0.282201] kernel BUG at kernel/cfi.c:32! [0.282202] [ cut here ] [0.282204] kernel BUG at kernel/cfi.c:32! [0.282207] kernel BUG at kernel/cfi.c:32! [0.282209] kernel BUG at kernel/cfi.c:32! [0.282211] kernel BUG at kernel/cfi.c:32! [0.282214] kernel BUG at kernel/cfi.c:32! [0.282215] [ cut here ] [0.282216] kernel BUG at kernel/cfi.c:32! [0.282218] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP [0.282224] Modules linked in: [0.282230] CPU: 2 PID: 25 Comm: migration/2 Not tainted 4.9.93-perf+ #39 [0.282232] Hardware name: [0.282235] task: fffbb3b36580 task.stack: fffbb30cc000 [0.282250] PC is at __cfi_check_fail+0x14/0x1c [0.282253] LR is at __cfi_check_fail+0x14/0x1c [0.282255] pc : [] lr : [] pstate: 60c00085 [0.282256] sp : fffbb30cfc30 [0.282259] x29: fffbb30cfc30 x28: ff93b6415000 [0.282261] x27: 0013b65c1000 x26: ff93b5ce6000 [0.282264] x25: ff93b5ce6000 x24: ff93b6419000 [0.282266] x23: ff93b65c1000 x22: ff93b65c4000 [0.282268] x21: 9d12f8172cb2f296 x20: 8180e3e0 [0.282271] x19: x18: 002c [0.282274] x17: 000fd054 x16: [0.282276] x15: ff93b65ec000 x14: 000c [0.282279] x13: 0004 x12: [0.282281] x11: x10: 01440144 [0.282283] x9 : 260822e8751d5000 x8 : 260822e8751d5000 [0.282286] x7 : x6 : fffbbac75b60 [0.282288] x5 : x4 : [0.282290] x3 : 3a657275 x2 : [0.282292] x1 : x0 : 000c [0.282294] [0.282294] PC: 0xff93b3f03d50: [0.282308] 3d50 b9001ac8 f94002c8 370ffec8 17be d421 1400 aa1603e0 f90007e8 [0.282315] 3d70 94536017 f94007e8 17e2 a9bf7bfd 910003fd d000d100 913ee400 94533cc7 [0.282322] 3d90 d421 1400 b0013788 2a1f03e0 f901c51f d65f03c0 f940406b 2a0203e8 [0.282329] 3db0 2a0103e9 aa0003ea b48b f9000145 f94000cb b40001ab a9bf7bfd 910003fd [0.282330] [0.282330] LR: 0xff93b3f03d50: [0.282336] 3d50 b9001ac8 f94002c8 370ffec8 17be d421 1400 aa1603e0 f90007e8 [0.282343] 3d70 94536017 f94007e8 17e2 a9bf7bfd 910003fd d000d100 913ee400 94533cc7 [0.282350] 3d90 d421 1400 b0013788 2a1f03e0 f901c51f d65f03c0 f940406b 2a0203e8 [0.282357] 3db0 2a0103e9 aa0003ea b48b f9000145 f94000cb b40001ab a9bf7bfd 910003fd [0.282358] [0.282358] SP: 0xfffbb30cfbf0: [0.282365] fbf0 b3f03d90 ff93 b30cfc30 fffb b3f03d90 ff93 60c00085 [0.282372] fc10 b6415000 ff93 b642fa00 ff93 b3f03d90 ff93 [0.282378] fc30 b30cfc70 fffb b3d458c0 ff93 0080 0001 [0.282385] fc50 b65c4000 ff93 b64420f0 ff93 8180e3e0 0002 [0.282387] Process migration/2 (pid: 25, stack limit = 0xfffbb30cc000) [0.282389] Call trace: [0.282391] Exception stack(0xfffbb30cfb00 to 0xfffbb30cfc30) [0.282395] fb00: 000c 3a657275 [0.282397] fb20: fffbbac75b60 [0.282400] fb40: 260822e8751d5000 260822e8751d5000 01440144 [0.282403] fb60: 0004 000c ff93b65ec000 [0.282405] fb80: 000fd054 002c [0.282408] fba0: 8180e3e0 9d12f8172cb2f296 ff93b65c4000 ff93b65c1000 [0.282411] fbc0
Re: [PATCH v5 07/13] ARM: smp: Add initialization of CNTVOFF
On 09/04/18 09:24, Geert Uytterhoeven wrote: > On Wed, Apr 4, 2018 at 4:30 PM, Marc Zyngier wrote: >> On Wed, 04 Apr 2018 14:59:09 +0100, >> Mylčne Josserand wrote: > > [Marc: stuck in ISO-8859-1? ;-] I have no idea what Wanderlust does (that's what I use on my laptop). But Thunderbird definitely interprets the original posting as 'è', which should work with a 8859-1. I need to have a look at how to get UTF-8 to be the default... (I hate email clients). > It'd be good to take this opportunity to refactor the shmobile code. >>> >>> I can do it in this series but I do not have any shmobile platforms so >>> I will not be able to test my modifications (only compilation). >>> >>> If someone can test it for me (who?), it is okay for me to refactor this >>> code :) >> >> I guess you could Cc the shmobile folks (Geert Uytterhoeven, Simon >> Horman), and get them to review/test the changes. > > Correct. I can test on a remote R-Car E2 ALT board that needs it. > > P.S. Interestingly, none of the Renesas CA15 SoCs seem to suffer from it, > only CA7. I suspect A15 has the courtesy of resetting CNTVOFF to zero, and A7 doesn't. But the letter of the architecture is that it has an "UNKNOWN reset value". Thanks, M. -- Jazz is not dead. It just smells funny...
Re: [PATCH v3 1/3] resource: Use list_head to link resource sibling
The struct resource uses singly linked list to link siblings. It's not easy to do reverse iteration on sibling list. So replace it with list_head. And code refactoring makes codes in kernel/resource.c more readable than pointer operation. Besides, type of member variables of struct resource, sibling and child, are changed from 'struct resource *' to 'struct list_head'. Kernel size will increase because of those statically defined struct resource instances. Signed-off-by: Baoquan He --- v2->v3: Make sibling() and first_child() global so that they can be called out of kernel/resource.c to simplify code. Fix several code bugs found by kbuild test robot. Got report from lkp that kernel size increased. It's on purpose since the type change of sibling and child inside struct resource{}. For each struct resource variable, it will cost another 16 bytes on x86 64. arch/sparc/kernel/ioport.c | 2 +- drivers/gpu/drm/drm_memory.c| 3 +- drivers/gpu/drm/gma500/gtt.c| 5 +- drivers/hv/vmbus_drv.c | 52 drivers/input/joystick/iforce/iforce-main.c | 4 +- drivers/nvdimm/e820.c | 2 +- drivers/nvdimm/namespace_devs.c | 6 +- drivers/nvdimm/nd.h | 5 +- drivers/of/address.c| 4 +- drivers/parisc/lba_pci.c| 4 +- drivers/pci/host/vmd.c | 8 +- drivers/pci/probe.c | 2 + drivers/pci/setup-bus.c | 2 +- include/linux/ioport.h | 6 +- kernel/resource.c | 193 ++-- 15 files changed, 149 insertions(+), 149 deletions(-) diff --git a/arch/sparc/kernel/ioport.c b/arch/sparc/kernel/ioport.c index 3bcef9ce74df..4e91fbbbedcc 100644 --- a/arch/sparc/kernel/ioport.c +++ b/arch/sparc/kernel/ioport.c @@ -669,7 +669,7 @@ static int sparc_io_proc_show(struct seq_file *m, void *v) struct resource *root = m->private, *r; const char *nm; - for (r = root->child; r != NULL; r = r->sibling) { + list_for_each_entry(r, &root->child, sibling) { if ((nm = r->name) == NULL) nm = "???"; seq_printf(m, "%016llx-%016llx: %s\n", (unsigned long long)r->start, diff --git a/drivers/gpu/drm/drm_memory.c b/drivers/gpu/drm/drm_memory.c index 3c54044214db..53e300a993dc 100644 --- a/drivers/gpu/drm/drm_memory.c +++ b/drivers/gpu/drm/drm_memory.c @@ -155,9 +155,8 @@ u64 drm_get_max_iomem(void) struct resource *tmp; resource_size_t max_iomem = 0; - for (tmp = iomem_resource.child; tmp; tmp = tmp->sibling) { + list_for_each_entry(tmp, &iomem_resource.child, sibling) max_iomem = max(max_iomem, tmp->end); - } return max_iomem; } diff --git a/drivers/gpu/drm/gma500/gtt.c b/drivers/gpu/drm/gma500/gtt.c index 3949b0990916..addd3bc009af 100644 --- a/drivers/gpu/drm/gma500/gtt.c +++ b/drivers/gpu/drm/gma500/gtt.c @@ -565,7 +565,7 @@ int psb_gtt_init(struct drm_device *dev, int resume) int psb_gtt_restore(struct drm_device *dev) { struct drm_psb_private *dev_priv = dev->dev_private; - struct resource *r = dev_priv->gtt_mem->child; + struct resource *r; struct gtt_range *range; unsigned int restored = 0, total = 0, size = 0; @@ -573,14 +573,13 @@ int psb_gtt_restore(struct drm_device *dev) mutex_lock(&dev_priv->gtt_mutex); psb_gtt_init(dev, 1); - while (r != NULL) { + list_for_each_entry(r, &dev_priv->gtt_mem->child, sibling) { range = container_of(r, struct gtt_range, resource); if (range->pages) { psb_gtt_insert(dev, range, 1); size += range->resource.end - range->resource.start; restored++; } - r = r->sibling; total++; } mutex_unlock(&dev_priv->gtt_mutex); diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index bc65c4d79c1f..7ba8a25520d9 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -1413,9 +1413,8 @@ static acpi_status vmbus_walk_resources(struct acpi_resource *res, void *ctx) { resource_size_t start = 0; resource_size_t end = 0; - struct resource *new_res; + struct resource *new_res, *tmp; struct resource **old_res = &hyperv_mmio; - struct resource **prev_res = NULL; switch (res->type) { @@ -1462,44 +1461,36 @@ static acpi_status vmbus_walk_resources(struct acpi_resource *res, void *ctx) /* * If two ranges are adjacent, merge them. */ - do { - if (!*old_res) { - *old_res = new_res; - break; - } - - if (((*old_res)->end + 1)
[PATCH v2 1/9] Input: synaptics - add Lenovo 80 series ids to SMBus
This time, Lenovo decided to go with different pieces in its latest series of Thinkpads. For those we have been able to test: - the T480 is using Synaptics with an IBM trackpoint -> it behaves properly with or without intertouch, there is no point not using RMI4 - the X1 Carbon 6th gen is using Synaptics with an IBM trackpoint -> the touchpad doesn't behave properly under PS/2 so we have to switch it to RMI4 if we do not want to have disappointed users - the X280 is using Synaptics with an ALPS trackpoint -> the recent fixes in the trackpoint handling fixed it so upstream now works fine with or without RMI4, and there is no point not using RMI4 - the T480s is using an Elan touchpad, so that's a different story Cc: # v4.14.x, v4.15.x, v4.16.x Signed-off-by: Benjamin Tissoires --- no changes in v2 drivers/input/mouse/synaptics.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c index 60f2c463d1cc..14a1188561aa 100644 --- a/drivers/input/mouse/synaptics.c +++ b/drivers/input/mouse/synaptics.c @@ -173,6 +173,9 @@ static const char * const smbus_pnp_ids[] = { "LEN0046", /* X250 */ "LEN004a", /* W541 */ "LEN200f", /* T450s */ + "LEN0071", /* T480 */ + "LEN0092", /* X1 Carbon 6th gen */ + "LEN0097", /* X280 -> ALPS trackpoint */ NULL }; -- 2.14.3
[PATCH v2 3/9] Input: elan_i2c - add trackstick report
The Elan touchpads over I2C/SMBus also can handle a trackstick. Unfortunately, nothing tells us if the device supports trackstick (the information lies in the PS/2 node), so rely on a platform data to enable or not the trackstick node. Link: https://bugzilla.redhat.com/show_bug.cgi?id=1313939 Signed-off-by: Benjamin Tissoires --- changes in v2: - use of generic device property instead of platform data so device tree can also make use of it --- .../devicetree/bindings/input/elan_i2c.txt | 1 + drivers/input/mouse/elan_i2c_core.c| 90 +- 2 files changed, 88 insertions(+), 3 deletions(-) diff --git a/Documentation/devicetree/bindings/input/elan_i2c.txt b/Documentation/devicetree/bindings/input/elan_i2c.txt index ee3242c4ba67..d80a83583238 100644 --- a/Documentation/devicetree/bindings/input/elan_i2c.txt +++ b/Documentation/devicetree/bindings/input/elan_i2c.txt @@ -14,6 +14,7 @@ Optional properties: - pinctrl-0: a phandle pointing to the pin settings for the device (see pinctrl binding [1]). - vcc-supply: a phandle for the regulator supplying 3.3V power. +- elan,trackpoint: touchpad can support a trackpoint (boolean) [0]: Documentation/devicetree/bindings/interrupt-controller/interrupts.txt [1]: Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt diff --git a/drivers/input/mouse/elan_i2c_core.c b/drivers/input/mouse/elan_i2c_core.c index 75e757520ef0..44e970931926 100644 --- a/drivers/input/mouse/elan_i2c_core.c +++ b/drivers/input/mouse/elan_i2c_core.c @@ -36,6 +36,7 @@ #include #include #include +#include #include #include @@ -51,6 +52,7 @@ #define ETP_MAX_FINGERS5 #define ETP_FINGER_DATA_LEN5 #define ETP_REPORT_ID 0x5D +#define ETP_TP_REPORT_ID 0x5E #define ETP_REPORT_ID_OFFSET 2 #define ETP_TOUCH_INFO_OFFSET 3 #define ETP_FINGER_DATA_OFFSET 4 @@ -61,6 +63,7 @@ struct elan_tp_data { struct i2c_client *client; struct input_dev*input; + struct input_dev*tp_input; /* trackpoint input node */ struct regulator*vcc; const struct elan_transport_ops *ops; @@ -930,6 +933,34 @@ static void elan_report_absolute(struct elan_tp_data *data, u8 *packet) input_sync(input); } +static void elan_report_trackpoint(struct elan_tp_data *data, u8 *report) +{ + struct input_dev *input = data->tp_input; + u8 *packet = &report[ETP_REPORT_ID_OFFSET + 1]; + int x, y; + + if (!data->tp_input) { + dev_warn_once(&data->client->dev, + "received a trackpoint report while no trackpoint device has been created.\n" + "Please report upstream.\n"); + return; + } + + input_report_key(input, BTN_LEFT, packet[0] & 0x01); + input_report_key(input, BTN_RIGHT, packet[0] & 0x02); + input_report_key(input, BTN_MIDDLE, packet[0] & 0x04); + + if ((packet[3] & 0x0F) == 0x06) { + x = packet[4] - (int)((packet[1]^0x80) << 1); + y = (int)((packet[2]^0x80) << 1) - packet[5]; + + input_report_rel(input, REL_X, x); + input_report_rel(input, REL_Y, y); + } + + input_sync(input); +} + static irqreturn_t elan_isr(int irq, void *dev_id) { struct elan_tp_data *data = dev_id; @@ -951,11 +982,17 @@ static irqreturn_t elan_isr(int irq, void *dev_id) if (error) goto out; - if (report[ETP_REPORT_ID_OFFSET] != ETP_REPORT_ID) + switch (report[ETP_REPORT_ID_OFFSET]) { + case ETP_REPORT_ID: + elan_report_absolute(data, report); + break; + case ETP_TP_REPORT_ID: + elan_report_trackpoint(data, report); + break; + default: dev_err(dev, "invalid report id data (%x)\n", report[ETP_REPORT_ID_OFFSET]); - else - elan_report_absolute(data, report); + } out: return IRQ_HANDLED; @@ -966,6 +1003,37 @@ static irqreturn_t elan_isr(int irq, void *dev_id) * Elan initialization functions ** */ + +static int elan_setup_trackpoint_input_device(struct elan_tp_data *data) +{ + struct device *dev = &data->client->dev; + struct input_dev *input; + + input = devm_input_allocate_device(dev); + if (!input) + return -ENOMEM; + + input->name = "Elan TrackPoint"; + input->id.bustype = BUS_I2C; + input->id.vendor = ELAN_VENDOR_ID; + input->id.product = data->product_id; + input_set_drvdata(input, data); + + input->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_REL); + input->relbit[BIT_WORD(REL_X)] = + BIT_MASK(REL_X) | BIT_MASK(REL_Y); + input->keybit[BIT_WORD(BTN_LEFT)] = + BIT_MASK(BTN_LEFT) | BIT_MASK(BTN_MIDDLE)
[PATCH v2 6/9] Input: elantech - query the resolution in query_info
The command ETP_RESOLUTION_QUERY also contains the bus information. It is better to fetch it once, while we are querying for device information. Signed-off-by: Benjamin Tissoires --- no changes in v2 drivers/input/mouse/elantech.c | 27 +++ drivers/input/mouse/elantech.h | 2 ++ 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c index 980dfd7e861e..a2a14a31edb5 100644 --- a/drivers/input/mouse/elantech.c +++ b/drivers/input/mouse/elantech.c @@ -1179,7 +1179,6 @@ static int elantech_set_input_params(struct psmouse *psmouse) struct elantech_data *etd = psmouse->private; struct elantech_device_info *info = &etd->info; unsigned int x_min = 0, y_min = 0, x_max = 0, y_max = 0, width = 0; - unsigned int x_res = 31, y_res = 31; if (elantech_set_range(psmouse, &x_min, &y_min, &x_max, &y_max, &width)) return -1; @@ -1232,13 +1231,6 @@ static int elantech_set_input_params(struct psmouse *psmouse) break; case 4: - if (elantech_get_resolution_v4(psmouse, &x_res, &y_res)) { - /* -* if query failed, print a warning and leave the values -* zero to resemble synaptics.c behavior. -*/ - psmouse_warn(psmouse, "couldn't query resolution data.\n"); - } elantech_set_buttonpad_prop(psmouse); __set_bit(BTN_TOOL_QUADTAP, dev->keybit); /* For X to recognize me as touchpad. */ @@ -1267,11 +1259,11 @@ static int elantech_set_input_params(struct psmouse *psmouse) break; } - input_abs_set_res(dev, ABS_X, x_res); - input_abs_set_res(dev, ABS_Y, y_res); + input_abs_set_res(dev, ABS_X, info->x_res); + input_abs_set_res(dev, ABS_Y, info->y_res); if (info->hw_version > 1) { - input_abs_set_res(dev, ABS_MT_POSITION_X, x_res); - input_abs_set_res(dev, ABS_MT_POSITION_Y, y_res); + input_abs_set_res(dev, ABS_MT_POSITION_X, info->x_res); + input_abs_set_res(dev, ABS_MT_POSITION_Y, info->y_res); } etd->y_max = y_max; @@ -1720,6 +1712,17 @@ static int elantech_query_info(struct psmouse *psmouse, /* The MSB indicates the presence of the trackpoint */ info->has_trackpoint = (info->capabilities[0] & 0x80) == 0x80; + info->x_res = 31; + info->y_res = 31; + if (info->hw_version == 4) { + if (elantech_get_resolution_v4(psmouse, + &info->x_res, + &info->y_res)) { + psmouse_warn(psmouse, +"failed to query resolution data.\n"); + } + } + return 0; } diff --git a/drivers/input/mouse/elantech.h b/drivers/input/mouse/elantech.h index d8ac27fe4597..851df4ce6232 100644 --- a/drivers/input/mouse/elantech.h +++ b/drivers/input/mouse/elantech.h @@ -120,6 +120,8 @@ struct elantech_device_info { unsigned char debug; unsigned char hw_version; unsigned int fw_version; + unsigned int x_res; + unsigned int y_res; bool paritycheck; bool jumpy_cursor; bool reports_pressure; -- 2.14.3
[PATCH v2 9/9] input: psmouse-smbus: allow to control psmouse_deactivate
This seems to be Synaptics specific, as some Elan touchpads are not correctly switching to SMBus if we call deactivate before switching to SMBus on cold boot and on resume. Tested with the T480s Signed-off-by: Benjamin Tissoires --- changes in v2: - rebased on top of previous --- drivers/input/mouse/elantech.c | 2 +- drivers/input/mouse/psmouse-smbus.c | 13 ++--- drivers/input/mouse/psmouse.h | 1 + drivers/input/mouse/synaptics.c | 2 +- 4 files changed, 13 insertions(+), 5 deletions(-) diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c index 07e40a58e66c..fb4d902c4403 100644 --- a/drivers/input/mouse/elantech.c +++ b/drivers/input/mouse/elantech.c @@ -1774,7 +1774,7 @@ static int elantech_create_smbus(struct psmouse *psmouse, if (info->has_trackpoint) smbus_board.properties = i2c_properties; - return psmouse_smbus_init(psmouse, &smbus_board, NULL, 0, + return psmouse_smbus_init(psmouse, &smbus_board, NULL, 0, false, leave_breadcrumbs); } diff --git a/drivers/input/mouse/psmouse-smbus.c b/drivers/input/mouse/psmouse-smbus.c index c8a3b1f35ce3..852d4b486ddb 100644 --- a/drivers/input/mouse/psmouse-smbus.c +++ b/drivers/input/mouse/psmouse-smbus.c @@ -23,6 +23,7 @@ struct psmouse_smbus_dev { struct i2c_client *client; struct list_head node; bool dead; + bool need_deactivate; }; static LIST_HEAD(psmouse_smbus_list); @@ -118,7 +119,10 @@ static psmouse_ret_t psmouse_smbus_process_byte(struct psmouse *psmouse) static int psmouse_smbus_reconnect(struct psmouse *psmouse) { - psmouse_deactivate(psmouse); + struct psmouse_smbus_dev *smbdev = psmouse->private; + + if (smbdev->need_deactivate) + psmouse_deactivate(psmouse); return 0; } @@ -225,6 +229,7 @@ void psmouse_smbus_cleanup(struct psmouse *psmouse) int psmouse_smbus_init(struct psmouse *psmouse, const struct i2c_board_info *board, const void *pdata, size_t pdata_size, + bool need_deactivate, bool leave_breadcrumbs) { struct psmouse_smbus_dev *smbdev; @@ -236,6 +241,7 @@ int psmouse_smbus_init(struct psmouse *psmouse, smbdev->psmouse = psmouse; smbdev->board = *board; + smbdev->need_deactivate = need_deactivate; if (pdata) { smbdev->board.platform_data = kmemdup(pdata, pdata_size, @@ -246,6 +252,9 @@ int psmouse_smbus_init(struct psmouse *psmouse, } } + if (need_deactivate) + psmouse_deactivate(psmouse); + psmouse->private = smbdev; psmouse->protocol_handler = psmouse_smbus_process_byte; psmouse->reconnect = psmouse_smbus_reconnect; @@ -253,8 +262,6 @@ int psmouse_smbus_init(struct psmouse *psmouse, psmouse->disconnect = psmouse_smbus_disconnect; psmouse->resync_time = 0; - psmouse_deactivate(psmouse); - mutex_lock(&psmouse_smbus_mutex); list_add_tail(&smbdev->node, &psmouse_smbus_list); mutex_unlock(&psmouse_smbus_mutex); diff --git a/drivers/input/mouse/psmouse.h b/drivers/input/mouse/psmouse.h index dd4ec1f602d7..64c3a5d3fb3e 100644 --- a/drivers/input/mouse/psmouse.h +++ b/drivers/input/mouse/psmouse.h @@ -225,6 +225,7 @@ struct i2c_board_info; int psmouse_smbus_init(struct psmouse *psmouse, const struct i2c_board_info *board, const void *pdata, size_t pdata_size, + bool need_deactivate, bool leave_breadcrumbs); void psmouse_smbus_cleanup(struct psmouse *psmouse); diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c index 14a1188561aa..6b1ed53a8b03 100644 --- a/drivers/input/mouse/synaptics.c +++ b/drivers/input/mouse/synaptics.c @@ -1751,7 +1751,7 @@ static int synaptics_create_intertouch(struct psmouse *psmouse, }; return psmouse_smbus_init(psmouse, &intertouch_board, - &pdata, sizeof(pdata), + &pdata, sizeof(pdata), true, leave_breadcrumbs); } -- 2.14.3
[PATCH v2 8/9] Input: elantech - detect new ICs and setup Host Notify for them
New ICs are using a different scheme for the alternate bus parameter. Given that they are new and are only using either PS2 only or PS2 + SMBus Host Notify, we force those new ICs to use the SMBus solution for enhanced reporting. This allows the touchpad found on the Lenovo T480s to report 5 fingers every 8 ms, instead of having a limit of 2 every 8 ms. Signed-off-by: Benjamin Tissoires --- no changes in v2 drivers/input/mouse/elantech.c | 11 +++ drivers/input/mouse/elantech.h | 15 +++ 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c index 510e7c0622d3..07e40a58e66c 100644 --- a/drivers/input/mouse/elantech.c +++ b/drivers/input/mouse/elantech.c @@ -1793,11 +1793,11 @@ static int elantech_setup_smbus(struct psmouse *psmouse, if (elantech_smbus == ELANTECH_SMBUS_NOT_SET) { /* -* FIXME: -* constraint the I2C capable devices by using FW version, -* board version, or by using DMI matching +* New ICs are enabled by default. +* Old ICs are up to the user to decide. */ - return -ENXIO; + if (!ETP_NEW_IC_SMBUS_HOST_NOTIFY(info->fw_version)) + return -ENXIO; } psmouse_info(psmouse, "Trying to set up SMBus access\n"); @@ -1818,6 +1818,9 @@ static int elantech_setup_smbus(struct psmouse *psmouse, static bool elantech_use_host_notify(struct psmouse *psmouse, struct elantech_device_info *info) { + if (ETP_NEW_IC_SMBUS_HOST_NOTIFY(info->fw_version)) + return true; + switch (info->bus) { case ETP_BUS_PS2_ONLY: /* expected case */ diff --git a/drivers/input/mouse/elantech.h b/drivers/input/mouse/elantech.h index f9b1c485e8d9..119727085a60 100644 --- a/drivers/input/mouse/elantech.h +++ b/drivers/input/mouse/elantech.h @@ -115,6 +115,21 @@ #define ETP_BUS_PS2_SMB_ALERT 3 #define ETP_BUS_PS2_SMB_HST_NTFY 4 +/* + * New ICs are either using SMBus Host Notify or just plain PS2. + * + * ETP_FW_VERSION_QUERY is: + * Byte 1: + * - bit 0..3: IC BODY + * Byte 2: + * - bit 4: HiddenButton + * - bit 5: PS2_SMBUS_NOTIFY + * - bit 6: PS2CRCCheck + */ +#define ETP_NEW_IC_SMBUS_HOST_NOTIFY(fw_version) \ + fw_version) & 0x0f2000) == 0x0f2000) && \ +((fw_version) & 0xff) > 0) + /* * The base position for one finger, v4 hardware */ -- 2.14.3
[PATCH v2 7/9] Input: elantech - add support for SMBus devices
Many of the Elantech devices are connected through PS/2 and a different bus (SMBus or plain I2C). To not break any existing device, we only enable SMBus based on a module parameter. If some laptops require the quirk to be set, we will have to rely on a list of PNPIds or MDI matching to individually expose those hardware over SMBus. the parameter mentioned above is elantech_smbus from the psmouse module. Signed-off-by: Benjamin Tissoires --- changes in v2: - use of device property instead of platform data --- drivers/input/mouse/Kconfig | 12 +++ drivers/input/mouse/elantech.c | 188 +++- drivers/input/mouse/elantech.h | 24 + drivers/input/mouse/psmouse-base.c | 21 +++- drivers/input/mouse/psmouse-smbus.c | 11 ++- drivers/input/mouse/psmouse.h | 1 + 6 files changed, 246 insertions(+), 11 deletions(-) diff --git a/drivers/input/mouse/Kconfig b/drivers/input/mouse/Kconfig index 89ebb8f39fee..f27f23f2d99a 100644 --- a/drivers/input/mouse/Kconfig +++ b/drivers/input/mouse/Kconfig @@ -133,6 +133,18 @@ config MOUSE_PS2_ELANTECH If unsure, say N. +config MOUSE_PS2_ELANTECH_SMBUS + bool "Elantech PS/2 SMbus companion" if EXPERT + default y + depends on MOUSE_PS2 && MOUSE_PS2_ELANTECH + depends on I2C=y || I2C=MOUSE_PS2 + select MOUSE_PS2_SMBUS + help + Say Y here if you have a Elantech touchpad connected to + to an SMBus, but enumerated through PS/2. + + If unsure, say Y. + config MOUSE_PS2_SENTELIC bool "Sentelic Finger Sensing Pad PS/2 protocol extension" depends on MOUSE_PS2 diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c index a2a14a31edb5..510e7c0622d3 100644 --- a/drivers/input/mouse/elantech.c +++ b/drivers/input/mouse/elantech.c @@ -14,13 +14,16 @@ #include #include #include +#include #include #include +#include #include #include #include #include "psmouse.h" #include "elantech.h" +#include "elan_i2c.h" #define elantech_debug(fmt, ...) \ do {\ @@ -1084,7 +1087,8 @@ static unsigned int elantech_convert_res(unsigned int val) static int elantech_get_resolution_v4(struct psmouse *psmouse, unsigned int *x_res, - unsigned int *y_res) + unsigned int *y_res, + unsigned int *bus) { unsigned char param[3]; @@ -1093,6 +1097,7 @@ static int elantech_get_resolution_v4(struct psmouse *psmouse, *x_res = elantech_convert_res(param[1] & 0x0f); *y_res = elantech_convert_res((param[1] & 0xf0) >> 4); + *bus = param[2]; return 0; } @@ -1474,6 +1479,12 @@ static void elantech_disconnect(struct psmouse *psmouse) { struct elantech_data *etd = psmouse->private; + /* +* We might have left a breadcrumb when trying to +* set up SMbus companion. +*/ + psmouse_smbus_cleanup(psmouse); + if (etd->tp_dev) input_unregister_device(etd->tp_dev); sysfs_remove_group(&psmouse->ps2dev.serio->dev.kobj, @@ -1659,6 +1670,8 @@ static int elantech_query_info(struct psmouse *psmouse, { unsigned char param[3]; + memset(info, 0, sizeof(*info)); + /* * Do the version query again so we can store the result */ @@ -1717,7 +1730,8 @@ static int elantech_query_info(struct psmouse *psmouse, if (info->hw_version == 4) { if (elantech_get_resolution_v4(psmouse, &info->x_res, - &info->y_res)) { + &info->y_res, + &info->bus)) { psmouse_warn(psmouse, "failed to query resolution data.\n"); } @@ -1726,6 +1740,129 @@ static int elantech_query_info(struct psmouse *psmouse, return 0; } +#if defined(CONFIG_MOUSE_PS2_ELANTECH_SMBUS) + +/* + * The newest Elantech device can use a secondary bus (over SMBus) which + * provides a better bandwidth and allow a better control of the touchpads. + * This is used to decide if we need to use this bus or not. + */ +enum { + ELANTECH_SMBUS_NOT_SET = -1, + ELANTECH_SMBUS_OFF, + ELANTECH_SMBUS_ON, +}; + +static int elantech_smbus = IS_ENABLED(CONFIG_MOUSE_ELAN_I2C_SMBUS) ? + ELANTECH_SMBUS_NOT_SET : ELANTECH_SMBUS_OFF; +module_param_named(elantech_smbus, elantech_smbus, int, 0644); +MODULE_PARM_DESC(elantech_smbus, "Use a secondary bus for the Elantech device."); + +static int elantech_create_smbus(struct psmouse *psmouse, +struct elantech_device_info *in
[PATCH v2 4/9] Input: elantech - split device info into a separate structure
In preparation for SMBus device support, move static device information that we query form the touchpad upon initialization into separate structure. This will allow us to query the device without allocating memory first. Signed-off-by: Benjamin Tissoires --- no changes in v2 drivers/input/mouse/elantech.c | 271 - drivers/input/mouse/elantech.h | 28 +++-- 2 files changed, 175 insertions(+), 124 deletions(-) diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c index db47a5e1d114..d485664f1563 100644 --- a/drivers/input/mouse/elantech.c +++ b/drivers/input/mouse/elantech.c @@ -24,7 +24,7 @@ #define elantech_debug(fmt, ...) \ do {\ - if (etd->debug) \ + if (etd->info.debug)\ psmouse_printk(KERN_DEBUG, psmouse, \ fmt, ##__VA_ARGS__);\ } while (0) @@ -105,7 +105,7 @@ static int elantech_read_reg(struct psmouse *psmouse, unsigned char reg, if (reg > 0x11 && reg < 0x20) return -1; - switch (etd->hw_version) { + switch (etd->info.hw_version) { case 1: if (ps2_sliced_command(&psmouse->ps2dev, ETP_REGISTER_READ) || ps2_sliced_command(&psmouse->ps2dev, reg) || @@ -137,7 +137,7 @@ static int elantech_read_reg(struct psmouse *psmouse, unsigned char reg, if (rc) psmouse_err(psmouse, "failed to read register 0x%02x.\n", reg); - else if (etd->hw_version != 4) + else if (etd->info.hw_version != 4) *val = param[0]; else *val = param[1]; @@ -160,7 +160,7 @@ static int elantech_write_reg(struct psmouse *psmouse, unsigned char reg, if (reg > 0x11 && reg < 0x20) return -1; - switch (etd->hw_version) { + switch (etd->info.hw_version) { case 1: if (ps2_sliced_command(&psmouse->ps2dev, ETP_REGISTER_WRITE) || ps2_sliced_command(&psmouse->ps2dev, reg) || @@ -237,7 +237,7 @@ static void elantech_report_absolute_v1(struct psmouse *psmouse) unsigned char *packet = psmouse->packet; int fingers; - if (etd->fw_version < 0x02) { + if (etd->info.fw_version < 0x02) { /* * byte 0: D U p1 p2 1 p3 R L * byte 1: f 0 th tw x9 x8 y9 y8 @@ -252,7 +252,7 @@ static void elantech_report_absolute_v1(struct psmouse *psmouse) fingers = (packet[0] & 0xc0) >> 6; } - if (etd->jumpy_cursor) { + if (etd->info.jumpy_cursor) { if (fingers != 1) { etd->single_finger_reports = 0; } else if (etd->single_finger_reports < 2) { @@ -282,8 +282,8 @@ static void elantech_report_absolute_v1(struct psmouse *psmouse) psmouse_report_standard_buttons(dev, packet[0]); - if (etd->fw_version < 0x02 && - (etd->capabilities[0] & ETP_CAP_HAS_ROCKER)) { + if (etd->info.fw_version < 0x02 && + (etd->info.capabilities[0] & ETP_CAP_HAS_ROCKER)) { /* rocker up */ input_report_key(dev, BTN_FORWARD, packet[0] & 0x40); /* rocker down */ @@ -391,7 +391,7 @@ static void elantech_report_absolute_v2(struct psmouse *psmouse) input_report_key(dev, BTN_TOOL_TRIPLETAP, fingers == 3); input_report_key(dev, BTN_TOOL_QUADTAP, fingers == 4); psmouse_report_standard_buttons(dev, packet[0]); - if (etd->reports_pressure) { + if (etd->info.reports_pressure) { input_report_abs(dev, ABS_PRESSURE, pres); input_report_abs(dev, ABS_TOOL_WIDTH, width); } @@ -444,7 +444,7 @@ static void elantech_report_trackpoint(struct psmouse *psmouse, default: /* Dump unexpected packet sequences if debug=1 (default) */ - if (etd->debug == 1) + if (etd->info.debug == 1) elantech_packet_dump(psmouse); break; @@ -523,7 +523,7 @@ static void elantech_report_absolute_v3(struct psmouse *psmouse, input_report_key(dev, BTN_TOOL_TRIPLETAP, fingers == 3); /* For clickpads map both buttons to BTN_LEFT */ - if (etd->fw_version & 0x001000) + if (etd->info.fw_version & 0x001000) input_report_key(dev, BTN_LEFT, packet[0] & 0x03); else psmouse_report_standard_buttons(dev, packet[0]); @@ -541,7 +541,7 @@ static void elantech_input_sync_v4(struct psmouse *psmouse) unsigned char *packet = psmouse->packet; /* For clickpads map both buttons to BTN_LEFT */ - if (etd->fw_
Re: [PATCH 2/2] crypto: ccree: enable support for hardware keys
On 04/03/2018 12:19 PM, Herbert Xu wrote: > On Sat, Mar 31, 2018 at 08:30:46PM +0300, Gilad Ben-Yossef wrote: >> However, as it uses the exact same mechanism of the regular xts-aes-ccree >> but takes the key from another source, I've marked it with a test of >> alg_test_null() on the premise that if the xts-aes-ccree works, so must this. > Well it appears to be stubbed out as cc_is_hw_key always returns > false. > >>> Are there other crypto drivers doing this? >> I thought the exact same thing until I ran into a presentation about the s390 >> secure keys implementation. I basically imitated their use (or abuse?) >> of the Crypto API >> assuming it is the way to go. >> >> Take a look at arch/s390/crypto/paes_s390.c > It's always nice to discover code that was never reviewed. > > The general approach seems sane though. > >> Anyway, if this is not the way to go I'd be more than happy to do whatever >> work is needed to create the right interface. >> >> PS. I'd be out of the office and away from email access to the coming week, >> so >> kindly excuse any delay in response. > I think it's fine. However, I don't really like the idea of everyone > coming up with their own "paes", i.e., your patch's use of "haes". > It should be OK to just use paes for everyone, no? > > As to your patch specifically, there is one issue where you're > directly dereferencing the key as a struct. This is a no-no because > the key may have come from user-space. You must treat it as a > binary blob. The s390 code seems to do this correctly. > > Cheers, I would be happy to have a generic kernel interface for some kind of key token as a binary blob. I am also open to use the kernel key ring for future extensions. But please understand we needed a way to support our hardware keys and I think the chosen design isn't so bad. regards Harald Freudenberger using the kernel key ring in future extensions.
[PATCH v2 5/9] Input: elantech_query_info() can be static
Signed-off-by: Fengguang Wu Signed-off-by: Benjamin Tissoires --- new in v2, raised by kbuild test robot --- drivers/input/mouse/elantech.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c index d485664f1563..980dfd7e861e 100644 --- a/drivers/input/mouse/elantech.c +++ b/drivers/input/mouse/elantech.c @@ -1662,8 +1662,8 @@ static int elantech_set_properties(struct elantech_device_info *info) return 0; } -int elantech_query_info(struct psmouse *psmouse, - struct elantech_device_info *info) +static int elantech_query_info(struct psmouse *psmouse, + struct elantech_device_info *info) { unsigned char param[3]; -- 2.14.3
[PATCH v2 0/9] Input: support for latest Lenovo thinkpads (series 80)
Hi Dmitry, Here is the v2 of the Lenovo 80 series. Changes from v1: - included patch to convert a function to static from build bot - use of device property instead of platform data (thus the new device tree binding) BTW, KT, if you want to add your Signed-off-by on the patches, feel free to do so. You were of tremendous help :) Cheers, Benjamin Benjamin Tissoires (9): Input: synaptics - add Lenovo 80 series ids to SMBus input: elan_i2c_smbus - fix corrupted stack Input: elan_i2c - add trackstick report Input: elantech - split device info into a separate structure Input: elantech_query_info() can be static Input: elantech - query the resolution in query_info Input: elantech - add support for SMBus devices Input: elantech - detect new ICs and setup Host Notify for them input: psmouse-smbus: allow to control psmouse_deactivate .../devicetree/bindings/input/elan_i2c.txt | 1 + drivers/input/mouse/Kconfig| 12 + drivers/input/mouse/elan_i2c_core.c| 90 +++- drivers/input/mouse/elan_i2c_smbus.c | 22 +- drivers/input/mouse/elantech.c | 479 +++-- drivers/input/mouse/elantech.h | 69 ++- drivers/input/mouse/psmouse-base.c | 21 +- drivers/input/mouse/psmouse-smbus.c| 24 +- drivers/input/mouse/psmouse.h | 2 + drivers/input/mouse/synaptics.c| 5 +- 10 files changed, 565 insertions(+), 160 deletions(-) -- 2.14.3
[PATCH v2 2/9] input: elan_i2c_smbus - fix corrupted stack
New ICs (like the one on the Lenovo T480s) answer to ETP_SMBUS_IAP_VERSION_CMD 4 bytes instead of 3. This corrupts the stack as i2c_smbus_read_block_data() uses the values returned by the i2c device to know how many data it need to return. i2c_smbus_read_block_data() can read up to 32 bytes (I2C_SMBUS_BLOCK_MAX) and there is no safeguard on how many bytes are provided in the return value. Ensure we always have enough space for any future firmware. Also 0-initialize the values to prevent any access to uninitialized memory. Cc: # v4.4.x, v4.9.x, v4.14.x, v4.15.x, v4.16.x Signed-off-by: Benjamin Tissoires --- no changes in v2 drivers/input/mouse/elan_i2c_smbus.c | 22 +++--- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/drivers/input/mouse/elan_i2c_smbus.c b/drivers/input/mouse/elan_i2c_smbus.c index 29f99529b187..cfcb32559925 100644 --- a/drivers/input/mouse/elan_i2c_smbus.c +++ b/drivers/input/mouse/elan_i2c_smbus.c @@ -130,7 +130,7 @@ static int elan_smbus_get_baseline_data(struct i2c_client *client, bool max_baseline, u8 *value) { int error; - u8 val[3]; + u8 val[I2C_SMBUS_BLOCK_MAX] = {0}; error = i2c_smbus_read_block_data(client, max_baseline ? @@ -149,7 +149,7 @@ static int elan_smbus_get_version(struct i2c_client *client, bool iap, u8 *version) { int error; - u8 val[3]; + u8 val[I2C_SMBUS_BLOCK_MAX] = {0}; error = i2c_smbus_read_block_data(client, iap ? ETP_SMBUS_IAP_VERSION_CMD : @@ -170,7 +170,7 @@ static int elan_smbus_get_sm_version(struct i2c_client *client, u8 *clickpad) { int error; - u8 val[3]; + u8 val[I2C_SMBUS_BLOCK_MAX] = {0}; error = i2c_smbus_read_block_data(client, ETP_SMBUS_SM_VERSION_CMD, val); @@ -188,7 +188,7 @@ static int elan_smbus_get_sm_version(struct i2c_client *client, static int elan_smbus_get_product_id(struct i2c_client *client, u16 *id) { int error; - u8 val[3]; + u8 val[I2C_SMBUS_BLOCK_MAX] = {0}; error = i2c_smbus_read_block_data(client, ETP_SMBUS_UNIQUEID_CMD, val); @@ -205,7 +205,7 @@ static int elan_smbus_get_checksum(struct i2c_client *client, bool iap, u16 *csum) { int error; - u8 val[3]; + u8 val[I2C_SMBUS_BLOCK_MAX] = {0}; error = i2c_smbus_read_block_data(client, iap ? ETP_SMBUS_FW_CHECKSUM_CMD : @@ -226,7 +226,7 @@ static int elan_smbus_get_max(struct i2c_client *client, { int ret; int error; - u8 val[3]; + u8 val[I2C_SMBUS_BLOCK_MAX] = {0}; ret = i2c_smbus_read_block_data(client, ETP_SMBUS_RANGE_CMD, val); if (ret != 3) { @@ -246,7 +246,7 @@ static int elan_smbus_get_resolution(struct i2c_client *client, { int ret; int error; - u8 val[3]; + u8 val[I2C_SMBUS_BLOCK_MAX] = {0}; ret = i2c_smbus_read_block_data(client, ETP_SMBUS_RESOLUTION_CMD, val); if (ret != 3) { @@ -267,7 +267,7 @@ static int elan_smbus_get_num_traces(struct i2c_client *client, { int ret; int error; - u8 val[3]; + u8 val[I2C_SMBUS_BLOCK_MAX] = {0}; ret = i2c_smbus_read_block_data(client, ETP_SMBUS_XY_TRACENUM_CMD, val); if (ret != 3) { @@ -294,7 +294,7 @@ static int elan_smbus_iap_get_mode(struct i2c_client *client, { int error; u16 constant; - u8 val[3]; + u8 val[I2C_SMBUS_BLOCK_MAX] = {0}; error = i2c_smbus_read_block_data(client, ETP_SMBUS_IAP_CTRL_CMD, val); if (error < 0) { @@ -345,7 +345,7 @@ static int elan_smbus_prepare_fw_update(struct i2c_client *client) int len; int error; enum tp_mode mode; - u8 val[3]; + u8 val[I2C_SMBUS_BLOCK_MAX] = {0}; u8 cmd[4] = {0x0F, 0x78, 0x00, 0x06}; u16 password; @@ -419,7 +419,7 @@ static int elan_smbus_write_fw_block(struct i2c_client *client, struct device *dev = &client->dev; int error; u16 result; - u8 val[3]; + u8 val[I2C_SMBUS_BLOCK_MAX] = {0}; /* * Due to the limitation of smbus protocol limiting -- 2.14.3
Re: [PATCH 0/1] drm: Add crtc_queue_syncobj and crtc_get_syncobj ioctls
On Fri, Apr 06, 2018 at 04:56:48PM -0700, Keith Packard wrote: > (This is an RFC on whether this pair of ioctls seems reasonable. The > code compiles, but I haven't tested it as I'm away from home this > weekend.) > > I'm rewriting my implementation of the Vulkan EXT_display_control > extension, which provides a way to signal a Vulkan fence at vblank > time. I had implemented it using events, but that isn't great as the > Vulkan API includes the ability to wait for any of a set of fences to > be signaled. As the other Vulkan fences are implemented using > dma_fences in the kernel, and (eventually) using syncobj up in user > space, it seems reasonable to use syncobjs for everything and hook > vblank to those. > > In any case, I'm proposing two new syncobj/vblank ioctls (the names > aren't great; suggestions welcome, as usual): > > DRM_IOCTL_CRTC_QUEUE_SYNCOBJ > > Create a new syncobj that will be signaled at (or after) the > specified vblank sequence value. This uses the same parameters > to specify the target sequence as > DRM_IOCTL_CRTC_QUEUE_SEQUENCE. My understanding of drm_syncobj is that you: - Create a syncobj with the drm_syncobj_create ioctl. - Pass it around to various driver callbacks who update the attached dma_fence pointer using drm_syncobj_replace_fence(). Yes amdgpu does this a bit differently, but that seems to be the exception. >From that pov I'd massage the uapi to just extend drm_crtc_queue_sequence_ioctl with a new syncobj flag. Syncobj we can just add at the bottom of struct drm_crtc_queue_sequence (drm structs can be extended like that, it's part of the uapi). Or we reuse user_data, but that's a bit a hack. We also don't need a new event type, vblank code simply singals event->fence, which we'll arrange to be the fence behind the syncobj. > DRM_IOCTL_CRTC_GET_SYNCOBJ > > Once the above syncobj has been signaled, this ioctl allows > the application to find out when that happened, returning both > the vblank sequence count and time (in ns). The android syncpt stuff already had the concept of a fence timestamp, and we carried it over as part of struct dma_fence.timestamp. It's just not exposed yet as proper uapi. I think we should aim a bit more into that direction with something like the below sketch: - Add a dma_fence_signal_ts, which allows us to set the timestamp from a hw clock. - Use that in the vblank code. - Add new drm_syncobj ioctl to query the timestamp of the attached fence (if it's signalled). That would entirely avoid the special-case ioctl just for vblank syncobj here. Also, this might be useful in your implementation of VK_GOOGLE_display_timing, since the current query timestamp you're using won't take into account irq wakeup latency. Using fence->timestamp will still miss the irq->atomic worker wakupe latency, but should be a lot better already. > I'd like to hear comments on whether this seems reasonable, or whether > I should go in some other direction. Besides a few bikesheds to align better with other stuff going around I think this looks good. -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH] swiotlb: use dma_direct_supported for swiotlb_ops
swiotlb_alloc calls dma_direct_alloc, which can satisfy lower than 32-bit dma mask requests using GFP_DMA if the architecture supports it. Various x86 drivers rely on that, so we need to support that. At the same time the whole kernel expects 32-bit dma mask to just work, so the other magic in swiotlb_dma_support isn't actually needed either. Reported-by: Dominik Brodowski Fixes: 6e4bf5867783 ("x86/dma: Use generic swiotlb_ops") Signed-off-by: Christoph Hellwig --- lib/swiotlb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/swiotlb.c b/lib/swiotlb.c index 47aeb04c1997..32aacd0d56a8 100644 --- a/lib/swiotlb.c +++ b/lib/swiotlb.c @@ -1087,6 +1087,6 @@ const struct dma_map_ops swiotlb_dma_ops = { .unmap_sg = swiotlb_unmap_sg_attrs, .map_page = swiotlb_map_page, .unmap_page = swiotlb_unmap_page, - .dma_supported = swiotlb_dma_supported, + .dma_supported = dma_direct_supported, }; #endif /* CONFIG_DMA_DIRECT_OPS */ -- 2.16.3
fix x86 swiotlb regression
Hi all, this patch fixes a regression in the x86 swiotlb conversion. This mostly happend because swiotlb_dma_support does the wrong thing (and did so for a long time) and we switched x86 to use it. There are a few others users of swiotlb_dma_supported that also look rather broken, but I'll take care of those for the next merge window.
Re: [PATCH AUTOSEL for 4.9 078/293] firmware: dmi_scan: Check DMI structure length
On Mon, 9 Apr 2018 00:23:55 +, Sasha Levin wrote: > From: Jean Delvare > > [ Upstream commit a814c3597a6b6040e2ef9459748081a6d5b7312d ] > > Before accessing DMI data to record it for later, we should ensure > that the DMI structures are large enough to contain the data in > question. > > Signed-off-by: Jean Delvare > Reviewed-by: Mika Westerberg > Cc: Dmitry Torokhov > Cc: Andy Shevchenko > Cc: Linus Walleij > Signed-off-by: Sasha Levin > --- > drivers/firmware/dmi_scan.c | 23 --- > 1 file changed, 16 insertions(+), 7 deletions(-) > (...) > @@ -191,13 +191,14 @@ static void __init dmi_save_ident(const struct > dmi_header *dm, int slot, > static void __init dmi_save_uuid(const struct dmi_header *dm, int slot, > int index) > { > - const u8 *d = (u8 *) dm + index; > + const u8 *d; > char *s; > int is_ff = 1, is_00 = 1, i; > > - if (dmi_ident[slot]) > + if (dmi_ident[slot] || dm->length <= index + 16) I'm afraid this check is off by one and nobody noticed :-( I'll send a fix-up patch. Probably harmless in practice as I have never seen a system with a DMI type 1 structure of exactly 24 bytes (would be 8 bytes for very old implementations and at least 25 for anything even remotely recent), but still not good. Sorry about that. > return; > > + d = (u8 *) dm + index; > for (i = 0; i < 16 && (is_ff || is_00); i++) { > if (d[i] != 0x00) > is_00 = 0; -- Jean Delvare SUSE L3 Support
Re: [PATCH v5 08/13] ARM: sunxi: Add initialization of CNTVOFF
On Sun, Apr 08, 2018 at 11:09:32AM +0200, Mylène Josserand wrote: > Hello Maxime, > > On Wed, 4 Apr 2018 09:45:15 +0200 > Maxime Ripard wrote: > > > On Tue, Apr 03, 2018 at 10:06:28PM +0200, Mylène Josserand wrote: > > > Hello, > > > > > > Thank you for the review. > > > > > > On Tue, 3 Apr 2018 11:12:18 +0200 > > > Maxime Ripard wrote: > > > > > > > On Tue, Apr 03, 2018 at 08:18:31AM +0200, Mylène Josserand wrote: > > > > > Add the initialization of CNTVOFF for sun8i-a83t. > > > > > > > > > > For boot CPU, Create a new machine that handles this > > > > > function's call in an "init_early" callback. > > > > > For secondary CPUs, add this function into secondary_startup > > > > > assembly entry. > > > > > > > > > > Signed-off-by: Mylène Josserand > > > > > --- > > > > > arch/arm/mach-sunxi/headsmp.S | 1 + > > > > > arch/arm/mach-sunxi/sunxi.c | 18 +- > > > > > 2 files changed, 18 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/arch/arm/mach-sunxi/headsmp.S > > > > > b/arch/arm/mach-sunxi/headsmp.S > > > > > index 79890fbe5613..b586b7cf803a 100644 > > > > > --- a/arch/arm/mach-sunxi/headsmp.S > > > > > +++ b/arch/arm/mach-sunxi/headsmp.S > > > > > @@ -71,6 +71,7 @@ ENDPROC(sunxi_mc_smp_cluster_cache_enable) > > > > > > > > > > ENTRY(sunxi_mc_smp_secondary_startup) > > > > > bl sunxi_mc_smp_cluster_cache_enable > > > > > + bl smp_init_cntvoff > > > > > b secondary_startup > > > > > ENDPROC(sunxi_mc_smp_secondary_startup) > > > > > > > > > > diff --git a/arch/arm/mach-sunxi/sunxi.c b/arch/arm/mach-sunxi/sunxi.c > > > > > index 5e9602ce1573..090784108c0a 100644 > > > > > --- a/arch/arm/mach-sunxi/sunxi.c > > > > > +++ b/arch/arm/mach-sunxi/sunxi.c > > > > > @@ -16,6 +16,7 @@ > > > > > #include > > > > > > > > > > #include > > > > > +#include > > > > > > > > > > static const char * const sunxi_board_dt_compat[] = { > > > > > "allwinner,sun4i-a10", > > > > > @@ -62,7 +63,6 @@ MACHINE_END > > > > > static const char * const sun8i_board_dt_compat[] = { > > > > > "allwinner,sun8i-a23", > > > > > "allwinner,sun8i-a33", > > > > > - "allwinner,sun8i-a83t", > > > > > "allwinner,sun8i-h2-plus", > > > > > "allwinner,sun8i-h3", > > > > > "allwinner,sun8i-r40", > > > > > @@ -75,6 +75,22 @@ DT_MACHINE_START(SUN8I_DT, "Allwinner sun8i > > > > > Family") > > > > > .dt_compat = sun8i_board_dt_compat, > > > > > MACHINE_END > > > > > > > > > > +void __init sun8i_cntvoff_init(void) > > > > > +{ > > > > > + smp_init_cntvoff(); > > > > > > > > Can't this be moved to the SMP setup code? > > > > > > I tried to put it in the first lines of "sunxi_mc_smp_init" function > > > but it did not work. I tried to find some callbacks to have an > > > early "init" and I only found the "init_early"'s one. There is probably > > > another way to handle that so do not hesitate to tell me any ideas. > > > > It's hard to say without more context about why it doesn't work. Have > > you checked the order between early_initcall, the timer initialization > > function and init_early? > > > > Yes, I tested it. I wanted to test it again to give more context. Here > is the boot log: http://code.bulix.org/n1x864-315948?raw > I added printk in the 3 functions (with "===>"). > > The "init_early" and "sun6i_timer" are executed before > "arch_timer_of_init" (which is the function that parses the DT property > that we used previously "arm,cpu-registers-not-fw-configured"). > The "sunxi_mc_smp_init" function is called later so I guess that is why > it is not working in that case. Ok. Make sure to mention that in your commit log. Maxime -- Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature