Re: [PATCH] 9p: prevent read overrun in protocol dump tracepoint
Steven Rostedt wrote on Sat, Dec 02, 2023 at 11:15:24PM -0500: > > Also, for custom tracepoints e.g. bpftrace the program needs to know how > > many bytes can be read safely even if it's just for dumping -- unless > > dynamic_array is a "fat pointer" that conveys its own size? > > (Sorry didn't take the time to check) > > Yes, there's also a __get_dynamic_array_len(line) that will return the > allocated length of the line. Is that what you need? Yes, thanks! So the lower two bytes of the field are its position in the entry and the higher two bytes its size; ok. It doesn't look like bpftrace has any helper for it but that can probably be sorted out if someone wants to dump data there. Let's update the event to use a dynamic array and have printk fomrat to use %*ph with that length. JP Kobryn, does that sound good to you? I'm not sure what you were trying to do in the first place. Do you want to send a v2 or shall I? -- Dominique Martinet | Asmadeus
Re: (subset) [PATCH 1/2] dt-bindings: arm: qcom: Add Huawei Honor 5X / GR5 (2016)
On Sat, 21 Oct 2023 16:30:24 +0200, Lukas Walter wrote: > Add a compatible for Huawei Honor 5X / GR5 (2016). > > Applied, thanks! [1/2] dt-bindings: arm: qcom: Add Huawei Honor 5X / GR5 (2016) commit: 01a3c3739183003640f8468ecf75d7eeb15f808a [2/2] arm64: dts: qcom: msm8939-huawei-kiwi: Add initial device tree commit: cff9a76f306bfb6262153c0da2029071036b9a04 Best regards, -- Bjorn Andersson
Re: [PATCH v3 0/3] Add support for HTC One Mini 2 smartphone
On Sat, 25 Nov 2023 13:05:32 +0100, Luca Weiss wrote: > Add support for this smartphone from HTC which is based on the MSM8926 > SoC and codenamed "memul". > > Depends on, runtime-only, bootloader enables watchdog so we need to pet > it to stay alive: > https://lore.kernel.org/linux-arm-msm/20231011-msm8226-msm8974-watchdog-v1-0-2c472818f...@z3ntu.xyz/T/ > > [...] Applied, thanks! [1/3] dt-bindings: vendor-prefixes: document HTC Corporation commit: d69e34675a8be0affe8c55dbf50f795dac521933 [2/3] dt-bindings: arm: qcom: Add HTC One Mini 2 commit: bfccc195192ea6ae72a4a49a85c94f1ad8ee7a13 [3/3] ARM: dts: qcom: Add support for HTC One Mini 2 commit: be0061dcbac1b6a5a1cf681f7cabbb2681ab0e2c Best regards, -- Bjorn Andersson
Re: (subset) [PATCH v3 1/2] arm64: dts: qcom: sm8250-xiaomi-elish: Fix typos
On Sun, 26 Nov 2023 10:28:48 +0800, Jianhua Lu wrote: > There are two typos in this dtsi, so fix it. > classis -> chassis. > 8070 -> 8060 > > Applied, thanks! [1/2] arm64: dts: qcom: sm8250-xiaomi-elish: Fix typos commit: 608168b4d6079f2c43944bdfd64fd6c405d9a767 [2/2] arm64: dts: qcom: sm8250-xiaomi-elish: Add pm8150b type-c node and enable usb otg commit: 69652787279d64b0b0cc350fdfb34c503e40653c Best regards, -- Bjorn Andersson
Re: [PATCH v2] arm64: dts: qcom: sdm632-fairphone-fp3: Enable WiFi/Bluetooth
On Mon, 27 Nov 2023 22:55:38 +0100, Luca Weiss wrote: > Configure and enable the WCNSS which provides WiFi and Bluetooth on this > device using the WCN3680B chip. > > Applied, thanks! [1/1] arm64: dts: qcom: sdm632-fairphone-fp3: Enable WiFi/Bluetooth commit: 5b006a82a2bbc0ce18bc6b084fc8d8d9cc110001 Best regards, -- Bjorn Andersson
Re: (subset) [PATCH 0/3] Add watchdog nodes to msm8226 & msm8974
On Wed, 11 Oct 2023 18:33:12 +0200, Luca Weiss wrote: > Document the compatible for the watchdog found on both SoCs, and add > them to the SoC dtsi file. And especially for the case where the > bootloader has already enabled the watchdog we need to start petting it > on time, otherwise the system gets rebooted. > > It's worth noting that the watchdog behaves a bit unexpectedly. > It appears the watchdog counts down significantly slower when there's no > load on the system and can last far longer than 30 seconds until they > bark. Only when putting load on the system, e.g. with stress-ng does the > watchdog interrupt fire and kill the system within an expected amount of > time. > > [...] Applied, thanks! [3/3] ARM: dts: qcom: msm8974: Add watchdog node commit: 95053f6bc8ffca438a261400d7c06bd74e3f106e Best regards, -- Bjorn Andersson
Re: (subset) [PATCH v2 0/2] Small dtsi fixes for msm8953 SoC
On Sat, 25 Nov 2023 13:19:26 +0100, Luca Weiss wrote: > Fix some small things in the qcom/msm8953.dtsi file to make dtbs_check > happier than before. > > Applied, thanks! [2/2] arm64: dts: qcom: msm8953: Use non-deprecated qcom,domain in LPASS commit: 2e0dcbf164fb02d2558bd08b9609a30ef5935912 Best regards, -- Bjorn Andersson
Re: [PATCH] arm64: dts: qcom: sdm632-fairphone-fp3: Enable LPASS
On Sun, 15 Oct 2023 22:06:56 +0200, Luca Weiss wrote: > Enable the LPASS/ADSP found on the phone. > > Applied, thanks! [1/1] arm64: dts: qcom: sdm632-fairphone-fp3: Enable LPASS commit: 2dee68e77cb5322d7cfe44f3c84ff8ae2eaf4aee Best regards, -- Bjorn Andersson
Re: [PATCH 0/4] Add Fairphone 5 thermals (PMK7325, PM7250B, PM7325)
On Fri, 13 Oct 2023 10:09:52 +0200, Luca Weiss wrote: > Configure the necessary components to register some thermal zones in > Linux for the different thermistors found on the Fairphone 5. > > The names for the thermal zones and ADCs were taken from the downstream > kernel but double checked against hardware schematics. > > > [...] Applied, thanks! [1/4] iio: adc: Add PM7325 PMIC7 ADC bindings commit: 18c74d56fe6070c7c38058d7b43ccf2102abebcd [2/4] arm64: dts: qcom: qcm6490-fairphone-fp5: Add PM7250B thermals commit: 4c343fe9b68adeca1aa3a851bd06e62ecdaed180 [3/4] arm64: dts: qcom: qcm6490-fairphone-fp5: Add PMK7325 thermals commit: 46a2f77e1eb81990d303a94ab62f1bf79d0c9926 [4/4] arm64: dts: qcom: qcm6490-fairphone-fp5: Add PM7325 thermals commit: ae1122c375707a36c8fecebba745421a1e0ff93f Best regards, -- Bjorn Andersson
Re: [PATCH v2] arm64: dts: qcom: msm8939-longcheer-l9100: Add proximity-near-level
On Sun, 26 Nov 2023 22:46:20 +0100, André Apitzsch wrote: > Consider an object near to the sensor when their distance is about 4 cm > or below. > > Applied, thanks! [1/1] arm64: dts: qcom: msm8939-longcheer-l9100: Add proximity-near-level commit: fbe0870c48ac84f117860096048055a4f078a976 Best regards, -- Bjorn Andersson
Re: [PATCH v2] arm64: dts: qcom: qcm6490-fairphone-fp5: Enable UFS
On Mon, Oct 02, 2023 at 02:30:41PM +0200, Luca Weiss wrote: > Enable the UFS phy and controller so that we can access the internal > storage of the phone. > > At the same time we need to bump the minimum voltage used for UFS VCC, > otherwise it doesn't initialize properly. The 2.952V is taken from the > vcc-voltage-level property downstream. > > See also the following link for more information about the VCCQ/VCCQ2: > https://gerrit-public.fairphone.software/plugins/gitiles/kernel/msm-extra/devicetree/+/1590a3739e7dc29d2597307881553236d492f188/fp5/yupik-idp-pm7250b.dtsi#207 > > Signed-off-by: Luca Weiss > --- > Depends on: > https://lore.kernel.org/linux-arm-msm/20230927081858.15961-1-quic_nitir...@quicinc.com/ I'd love to merge this patch, but this dependency doesn't seem to make progress, please consider fixing up the outstanding feedback and posting v5. Regards, Bjorn
Re: [PATCH v2 1/2] arm64: dts: qcom: msm8953: Set initial address for memory
On Sat, Nov 25, 2023 at 01:19:27PM +0100, Luca Weiss wrote:
> The dtbs_check really doesn't like having memory without reg set.
>
> The base address depends on the amount of RAM you have:
>
> <= 2.00 GiB RAM: 0x8000
>= 3.00 GiB RAM: 0x4000
>= 3.75 GiB RAM: 0x1000
> (more does not fit into the 32-bit physical address space)
>
> So, let's pick one of the values, 0x1000 which is used on devices
> with 3.75 GiB RAM. Since the bootloader will update it to what's present
> on the device it doesn't matter too much.
>
> Signed-off-by: Luca Weiss
> ---
> arch/arm64/boot/dts/qcom/msm8953.dtsi | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8953.dtsi
> b/arch/arm64/boot/dts/qcom/msm8953.dtsi
> index e7de7632669a..a3ba24ca599b 100644
> --- a/arch/arm64/boot/dts/qcom/msm8953.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8953.dtsi
> @@ -174,10 +174,10 @@ scm: scm {
> };
> };
>
> - memory {
Wouldn't it be sufficient to add @0 here, to please dtbs_check?
Regards,
Bjorn
> + memory@1000 {
> device_type = "memory";
> /* We expect the bootloader to fill in the reg */
> - reg = <0 0 0 0>;
> + reg = <0 0x1000 0 0>;
> };
>
> pmu {
>
> --
> 2.43.0
>
Re: [PATCH] 9p: prevent read overrun in protocol dump tracepoint
On Sun, 3 Dec 2023 10:33:32 +0900
Dominique Martinet wrote:
> > TP_printk("clnt %lu %s(tag = %d)\n%.3x: %16ph\n%.3x: %16ph\n",
> > (unsigned long)__entry->clnt,
> > show_9p_op(__entry->type),
> > __entry->tag, 0, __get_dynamic_array(line), 16,
> > __get_dynamic_array(line) + 16)
>
> This was just printing garbage in the previous version but %16ph with a
> dynamic alloc would be out of range (even the start of the next buffer,
> _get_dynamic_array(line) + 16, can be out of range)
>
> Also, for custom tracepoints e.g. bpftrace the program needs to know how
> many bytes can be read safely even if it's just for dumping -- unless
> dynamic_array is a "fat pointer" that conveys its own size?
> (Sorry didn't take the time to check)
Yes, there's also a __get_dynamic_array_len(line) that will return the
allocated length of the line. Is that what you need?
-- Steve
Re: [PATCH] 9p: prevent read overrun in protocol dump tracepoint
Steven Rostedt wrote on Sat, Dec 02, 2023 at 08:14:09PM -0500:
> > AFAICS __entry is a local variable on stack, and array __entry->line not
> > intialized with zeros, i.e. the dump would contain trash at the end. Maybe
> > prepending memset() before memcpy()?
Well spotted!
Now I'm thinking about it we weren't initializing the source buffer
either back when we had (>32) msize allocations, so these already had
been printing garbage, but might as well get this sorted out while we're
here.
> __entry is a macro that points into the ring buffer that gets allocated
> before this is called. TRACE_EVENT() has a __dynamic_array() field that
> can handle variable length arrays. What you can do is turn this into
> something like:
>
> TRACE_EVENT(9p_protocol_dump,
> TP_PROTO(struct p9_client *clnt, struct p9_fcall *pdu),
>
> TP_ARGS(clnt, pdu),
>
> TP_STRUCT__entry(
> __field(void *, clnt
> )
> __field(__u8, type
> )
> __field(__u16, tag
> )
> __dynamic_array(unsigned char, line, min(pdu->capacity,
> P9_PROTO_DUMP_SZ) )
> ),
>
> TP_fast_assign(
> __entry->clnt = clnt;
> __entry->type = pdu->id;
> __entry->tag= pdu->tag;
> memcpy(__get_dynamic_array(line), pdu->sdata,
> min(pdu->capacity, P9_PROTO_DUMP_SZ));
> ),
> TP_printk("clnt %lu %s(tag = %d)\n%.3x: %16ph\n%.3x: %16ph\n",
> (unsigned long)__entry->clnt, show_9p_op(__entry->type),
> __entry->tag, 0, __get_dynamic_array(line), 16,
> __get_dynamic_array(line) + 16)
This was just printing garbage in the previous version but %16ph with a
dynamic alloc would be out of range (even the start of the next buffer,
_get_dynamic_array(line) + 16, can be out of range)
Also, for custom tracepoints e.g. bpftrace the program needs to know how
many bytes can be read safely even if it's just for dumping -- unless
dynamic_array is a "fat pointer" that conveys its own size?
(Sorry didn't take the time to check)
So I see two ways forward:
- We can give up on the 16 bytes split here, add the size in one of the
fields, and print with %*ph using that size.
- Or just give up and zero the tail; I'm surprised there's no "memcpy
up to x bytes and zero up to y bytes if required" helper but Christian's
suggestion of always doing memset first is probably not that bad
performance-wise if someone's dumping these out already.
I don't have a hard preference here, what do you think?
--
Dominique Martinet | Asmadeus
Re: [PATCH] 9p: prevent read overrun in protocol dump tracepoint
On Sat, 02 Dec 2023 14:05:24 +0100
Christian Schoenebeck wrote:
> > > --- a/include/trace/events/9p.h
> > > +++ b/include/trace/events/9p.h
> > > @@ -185,7 +185,8 @@ TRACE_EVENT(9p_protocol_dump,
> > > __entry->clnt = clnt;
> > > __entry->type = pdu->id;
> > > __entry->tag= pdu->tag;
> > > - memcpy(__entry->line, pdu->sdata, P9_PROTO_DUMP_SZ);
> > > + memcpy(__entry->line, pdu->sdata,
> > > + min(pdu->capacity, P9_PROTO_DUMP_SZ));
> > > ),
> > > TP_printk("clnt %lu %s(tag = %d)\n%.3x: %16ph\n%.3x: %16ph\n",
> > > (unsigned long)__entry->clnt, show_9p_op(__entry->type),
>
> AFAICS __entry is a local variable on stack, and array __entry->line not
> intialized with zeros, i.e. the dump would contain trash at the end. Maybe
> prepending memset() before memcpy()?
__entry is a macro that points into the ring buffer that gets allocated
before this is called. TRACE_EVENT() has a __dynamic_array() field that
can handle variable length arrays. What you can do is turn this into
something like:
TRACE_EVENT(9p_protocol_dump,
TP_PROTO(struct p9_client *clnt, struct p9_fcall *pdu),
TP_ARGS(clnt, pdu),
TP_STRUCT__entry(
__field(void *, clnt
)
__field(__u8, type
)
__field(__u16, tag
)
__dynamic_array(unsigned char, line, min(pdu->capacity,
P9_PROTO_DUMP_SZ) )
),
TP_fast_assign(
__entry->clnt = clnt;
__entry->type = pdu->id;
__entry->tag= pdu->tag;
memcpy(__get_dynamic_array(line), pdu->sdata,
min(pdu->capacity, P9_PROTO_DUMP_SZ));
),
TP_printk("clnt %lu %s(tag = %d)\n%.3x: %16ph\n%.3x: %16ph\n",
(unsigned long)__entry->clnt, show_9p_op(__entry->type),
__entry->tag, 0, __get_dynamic_array(line), 16,
__get_dynamic_array(line) + 16)
);
-- Steve
Re: [PATCH vhost 0/7] vdpa/mlx5: Add support for resumable vqs
On Fri, Dec 01, 2023 at 12:48:50PM +0200, Dragos Tatulea wrote: > Add support for resumable vqs in the driver. This is a firmware feature > that can be used for the following benefits: > - Full device .suspend/.resume. > - .set_map doesn't need to destroy and create new vqs anymore just to > update the map. When resumable vqs are supported it is enough to > suspend the vqs, set the new maps, and then resume the vqs. > > The first patch exposes the relevant bits in mlx5_ifc.h. That means it > needs to be applied to the mlx5-vhost tree [0] first. I didn't get this. Why does this need to go through that tree? Is there a dependency on some other commit from that tree? > Once applied > there, the change has to be pulled from mlx5-vhost into the vhost tree > and only then the remaining patches can be applied. Same flow as the vq > descriptor mappings patchset [1]. > > To be able to use resumable vqs properly, support for selectively modifying > vq parameters was needed. This is what the middle part of the series > consists of. > > [0] > https://git.kernel.org/pub/scm/linux/kernel/git/mellanox/linux.git/log/?h=mlx5-vhost > [1] > https://lore.kernel.org/virtualization/20231018171456.1624030-2-dtatu...@nvidia.com/ > > Dragos Tatulea (7): > vdpa/mlx5: Expose resumable vq capability > vdpa/mlx5: Split function into locked and unlocked variants > vdpa/mlx5: Allow modifying multiple vq fields in one modify command > vdpa/mlx5: Introduce per vq and device resume > vdpa/mlx5: Mark vq addrs for modification in hw vq > vdpa/mlx5: Mark vq state for modification in hw vq > vdpa/mlx5: Use vq suspend/resume during .set_map > > drivers/vdpa/mlx5/core/mr.c| 31 +++--- > drivers/vdpa/mlx5/net/mlx5_vnet.c | 172 + > include/linux/mlx5/mlx5_ifc.h | 3 +- > include/linux/mlx5/mlx5_ifc_vdpa.h | 4 + > 4 files changed, 174 insertions(+), 36 deletions(-) > > -- > 2.42.0
Re: [PATCH net-next v5 2/3] virtio/vsock: send credit update during setting SO_RCVLOWAT
On 02.12.2023 23:22, Michael S. Tsirkin wrote:
> On Fri, Dec 01, 2023 at 01:40:41PM +0300, Arseniy Krasnov wrote:
>>
>>
>> On 01.12.2023 12:48, Stefano Garzarella wrote:
>>> On Fri, Dec 01, 2023 at 11:35:56AM +0300, Arseniy Krasnov wrote:
On 01.12.2023 11:27, Stefano Garzarella wrote:
> On Thu, Nov 30, 2023 at 12:40:43PM -0500, Michael S. Tsirkin wrote:
>> On Thu, Nov 30, 2023 at 03:11:19PM +0100, Stefano Garzarella wrote:
>>> On Thu, Nov 30, 2023 at 08:58:58AM -0500, Michael S. Tsirkin wrote:
On Thu, Nov 30, 2023 at 04:43:34PM +0300, Arseniy Krasnov wrote:
>
>
> On 30.11.2023 16:42, Michael S. Tsirkin wrote:
>> On Thu, Nov 30, 2023 at 04:08:39PM +0300, Arseniy Krasnov wrote:
>>> Send credit update message when SO_RCVLOWAT is updated and it is
>>> bigger
>>> than number of bytes in rx queue. It is needed, because 'poll()'
>>> will
>>> wait until number of bytes in rx queue will be not smaller than
>>> SO_RCVLOWAT, so kick sender to send more data. Otherwise mutual
>>> hungup
>>> for tx/rx is possible: sender waits for free space and receiver is
>>> waiting data in 'poll()'.
>>>
>>> Signed-off-by: Arseniy Krasnov
>>> ---
>>> Changelog:
>>> v1 -> v2:
>>> * Update commit message by removing 'This patch adds XXX' manner.
>>> * Do not initialize 'send_update' variable - set it directly
>>> during
>>> first usage.
>>> v3 -> v4:
>>> * Fit comment in 'virtio_transport_notify_set_rcvlowat()' to 80
>>> chars.
>>> v4 -> v5:
>>> * Do not change callbacks order in transport structures.
>>>
>>> drivers/vhost/vsock.c | 1 +
>>> include/linux/virtio_vsock.h | 1 +
>>> net/vmw_vsock/virtio_transport.c | 1 +
>>> net/vmw_vsock/virtio_transport_common.c | 27
>>> +
>>> net/vmw_vsock/vsock_loopback.c | 1 +
>>> 5 files changed, 31 insertions(+)
>>>
>>> diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
>>> index f75731396b7e..4146f80db8ac 100644
>>> --- a/drivers/vhost/vsock.c
>>> +++ b/drivers/vhost/vsock.c
>>> @@ -451,6 +451,7 @@ static struct virtio_transport vhost_transport
>>> = {
>>> .notify_buffer_size =
>>> virtio_transport_notify_buffer_size,
>>>
>>> .read_skb = virtio_transport_read_skb,
>>> + .notify_set_rcvlowat =
>>> virtio_transport_notify_set_rcvlowat
>>> },
>>>
>>> .send_pkt = vhost_transport_send_pkt,
>>> diff --git a/include/linux/virtio_vsock.h
>>> b/include/linux/virtio_vsock.h
>>> index ebb3ce63d64d..c82089dee0c8 100644
>>> --- a/include/linux/virtio_vsock.h
>>> +++ b/include/linux/virtio_vsock.h
>>> @@ -256,4 +256,5 @@ void virtio_transport_put_credit(struct
>>> virtio_vsock_sock *vvs, u32 credit);
>>> void virtio_transport_deliver_tap_pkt(struct sk_buff *skb);
>>> int virtio_transport_purge_skbs(void *vsk, struct sk_buff_head
>>> *list);
>>> int virtio_transport_read_skb(struct vsock_sock *vsk,
>>> skb_read_actor_t read_actor);
>>> +int virtio_transport_notify_set_rcvlowat(struct vsock_sock *vsk,
>>> int val);
>>> #endif /* _LINUX_VIRTIO_VSOCK_H */
>>> diff --git a/net/vmw_vsock/virtio_transport.c
>>> b/net/vmw_vsock/virtio_transport.c
>>> index af5bab1acee1..8007593a3a93 100644
>>> --- a/net/vmw_vsock/virtio_transport.c
>>> +++ b/net/vmw_vsock/virtio_transport.c
>>> @@ -539,6 +539,7 @@ static struct virtio_transport virtio_transport
>>> = {
>>> .notify_buffer_size =
>>> virtio_transport_notify_buffer_size,
>>>
>>> .read_skb = virtio_transport_read_skb,
>>> + .notify_set_rcvlowat =
>>> virtio_transport_notify_set_rcvlowat
>>> },
>>>
>>> .send_pkt = virtio_transport_send_pkt,
>>> diff --git a/net/vmw_vsock/virtio_transport_common.c
>>> b/net/vmw_vsock/virtio_transport_common.c
>>> index f6dc896bf44c..1cb556ad4597 100644
>>> --- a/net/vmw_vsock/virtio_transport_common.c
>>> +++ b/net/vmw_vsock/virtio_transport_common.c
>>> @@ -1684,6 +1684,33 @@ int virtio_transport_read_skb(struct
>>> vsock_sock *vsk, skb_read_actor_t recv_acto
>>> }
>>> EXPORT_SYMBOL_GPL(virtio_transport_read_skb);
>>>
>>> +int virtio_transport_notify_set_rcvlowat(struct vsock_sock *vsk,
>>> int val)
>>> +{
Re: [PATCH net-next v5 2/3] virtio/vsock: send credit update during setting SO_RCVLOWAT
On Fri, Dec 01, 2023 at 01:40:41PM +0300, Arseniy Krasnov wrote:
>
>
> On 01.12.2023 12:48, Stefano Garzarella wrote:
> > On Fri, Dec 01, 2023 at 11:35:56AM +0300, Arseniy Krasnov wrote:
> >>
> >>
> >> On 01.12.2023 11:27, Stefano Garzarella wrote:
> >>> On Thu, Nov 30, 2023 at 12:40:43PM -0500, Michael S. Tsirkin wrote:
> On Thu, Nov 30, 2023 at 03:11:19PM +0100, Stefano Garzarella wrote:
> > On Thu, Nov 30, 2023 at 08:58:58AM -0500, Michael S. Tsirkin wrote:
> > > On Thu, Nov 30, 2023 at 04:43:34PM +0300, Arseniy Krasnov wrote:
> > > >
> > > >
> > > > On 30.11.2023 16:42, Michael S. Tsirkin wrote:
> > > > > On Thu, Nov 30, 2023 at 04:08:39PM +0300, Arseniy Krasnov wrote:
> > > > >> Send credit update message when SO_RCVLOWAT is updated and it is
> > > > >> bigger
> > > > >> than number of bytes in rx queue. It is needed, because 'poll()'
> > > > >> will
> > > > >> wait until number of bytes in rx queue will be not smaller than
> > > > >> SO_RCVLOWAT, so kick sender to send more data. Otherwise mutual
> > > > >> hungup
> > > > >> for tx/rx is possible: sender waits for free space and receiver
> > > > >> is
> > > > >> waiting data in 'poll()'.
> > > > >>
> > > > >> Signed-off-by: Arseniy Krasnov
> > > > >> ---
> > > > >> Changelog:
> > > > >> v1 -> v2:
> > > > >> * Update commit message by removing 'This patch adds XXX'
> > > > >>manner.
> > > > >> * Do not initialize 'send_update' variable - set it directly
> > > > >>during
> > > > >> first usage.
> > > > >> v3 -> v4:
> > > > >> * Fit comment in 'virtio_transport_notify_set_rcvlowat()' to
> > > > >>80 chars.
> > > > >> v4 -> v5:
> > > > >> * Do not change callbacks order in transport structures.
> > > > >>
> > > > >> drivers/vhost/vsock.c | 1 +
> > > > >> include/linux/virtio_vsock.h | 1 +
> > > > >> net/vmw_vsock/virtio_transport.c | 1 +
> > > > >> net/vmw_vsock/virtio_transport_common.c | 27
> > > > >>+
> > > > >> net/vmw_vsock/vsock_loopback.c | 1 +
> > > > >> 5 files changed, 31 insertions(+)
> > > > >>
> > > > >> diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> > > > >> index f75731396b7e..4146f80db8ac 100644
> > > > >> --- a/drivers/vhost/vsock.c
> > > > >> +++ b/drivers/vhost/vsock.c
> > > > >> @@ -451,6 +451,7 @@ static struct virtio_transport
> > > > >> vhost_transport = {
> > > > >> .notify_buffer_size =
> > > > >>virtio_transport_notify_buffer_size,
> > > > >>
> > > > >> .read_skb = virtio_transport_read_skb,
> > > > >> + .notify_set_rcvlowat =
> > > > >> virtio_transport_notify_set_rcvlowat
> > > > >> },
> > > > >>
> > > > >> .send_pkt = vhost_transport_send_pkt,
> > > > >> diff --git a/include/linux/virtio_vsock.h
> > > > >> b/include/linux/virtio_vsock.h
> > > > >> index ebb3ce63d64d..c82089dee0c8 100644
> > > > >> --- a/include/linux/virtio_vsock.h
> > > > >> +++ b/include/linux/virtio_vsock.h
> > > > >> @@ -256,4 +256,5 @@ void virtio_transport_put_credit(struct
> > > > >> virtio_vsock_sock *vvs, u32 credit);
> > > > >> void virtio_transport_deliver_tap_pkt(struct sk_buff *skb);
> > > > >> int virtio_transport_purge_skbs(void *vsk, struct sk_buff_head
> > > > >>*list);
> > > > >> int virtio_transport_read_skb(struct vsock_sock *vsk,
> > > > >>skb_read_actor_t read_actor);
> > > > >> +int virtio_transport_notify_set_rcvlowat(struct vsock_sock
> > > > >> *vsk, int val);
> > > > >> #endif /* _LINUX_VIRTIO_VSOCK_H */
> > > > >> diff --git a/net/vmw_vsock/virtio_transport.c
> > > > >> b/net/vmw_vsock/virtio_transport.c
> > > > >> index af5bab1acee1..8007593a3a93 100644
> > > > >> --- a/net/vmw_vsock/virtio_transport.c
> > > > >> +++ b/net/vmw_vsock/virtio_transport.c
> > > > >> @@ -539,6 +539,7 @@ static struct virtio_transport
> > > > >> virtio_transport = {
> > > > >> .notify_buffer_size =
> > > > >>virtio_transport_notify_buffer_size,
> > > > >>
> > > > >> .read_skb = virtio_transport_read_skb,
> > > > >> + .notify_set_rcvlowat =
> > > > >> virtio_transport_notify_set_rcvlowat
> > > > >> },
> > > > >>
> > > > >> .send_pkt = virtio_transport_send_pkt,
> > > > >> diff --git a/net/vmw_vsock/virtio_transport_common.c
> > > > >> b/net/vmw_vsock/virtio_transport_common.c
> > > > >> index f6dc896bf44c..1cb556ad4597 100644
> > > > >> --- a/net/vmw_vsock/virtio_transport_common.c
> > > > >> +++ b/net/vmw_vsock/virtio_transport_common.c
> > > > >> @@ -1684,6 +1684,33 @@ int virtio_transport_read_skb(struct
> > > > >> vsock_sock *vsk, skb_read_actor_t
Re: ARM Ftrace Function Graph Fails With UNWINDER_FRAME_POINTER
On 12/2/2023 1:26 AM, Ard Biesheuvel wrote:
On Sat, 2 Dec 2023 at 09:49, Justin Chen wrote:
On 12/1/23 10:53 PM, Ard Biesheuvel wrote:
On Fri, 1 Dec 2023 at 23:59, Justin Chen wrote:
On 12/1/23 10:07 AM, Steven Rostedt wrote:
On Fri, 1 Dec 2023 09:25:59 -0800
Justin Chen wrote:
It appears the sub instruction at 0x6dd0 correctly accounts for the
extra 8 bytes, so the frame pointer is valid. So it is our assumption
that there are no gaps between the stack frames is invalid.
Thanks for the assistance. The gap between the stack frame depends on
the function. Most do not have a gap. Some have 8 (as shown above), some
have 12. A single assumption here is not going to work. I'm having a
hard time finding out the reasoning for this gap. I tried disabling a
bunch of gcc flags as well as -O2 and the gap still exists.
That code was originally added because of some strange things that gcc did
with mcount (for example, it made a copy of the stack frame that it passed
to mcount, where the function graph tracer replaced the copy of the return
stack making the shadow stack go out of sync and crash). This was very hard
to debug and I added this code to detect it if it happened again.
Well it's been over a decade since that happened (2009).
71e308a239c09 ("function-graph: add stack frame test")
I'm happy assuming that the compiler folks are aware of our tricks with
hijacking return calls and I don't expect it to happen again. We can just
rip out those checks. That is, if it's only causing false positives, I
don't think it's worth keeping around.
Has it detected any real issues on the Arm platforms?
-- Steve
I am not familiar enough to make a call. But from my limited testing
with ARM, I didn't see any issues. If you would like me to, I can submit
a patch to remove the check entirely. Or maybe only disable it for ARM?
Please try the fix I proposed first.
Just tested it. Seems to do the trick.
Thanks
Either solution works for me.
Given that this discussion is taking place in the context of the
report of an issue identified by GRAPH_FP_TEST, I don't see how
removing that would be a reasonable conclusion.
Fair enough. I will submit a patch. Thanks for the help.
FWIW I also experimented with LLVM, looks like function_graph just
crashes regardless of the issue being discussed. The disassemble of
LLVM[1] does something completely different.
LLVM does not support CONFIG_UNWINDER_FRAME_POINTER so the fact that
the prologue looks different is expected.
In the case below, the FP {r11} is correctly made to point to a {r11,
lr} tuple on the stack, so the codegen is correct AFAICT. But IIRC we
rely on unwind information for ftrace in this case, not the frame
pointer.
Could you be more specific about the crash?
It just hangs with no prints. I can instrument the hang and see what I
can find.
Justin
[1]
LLVM dump
c0c6faa0 :
c0c6faa0: f0 4f 2d e9 push{r4, r5, r6, r7, r8, r9, r10, r11, lr}
c0c6faa4: 1c b0 8d e2 add r11, sp, #28
c0c6faa8: ac d0 4d e2 sub sp, sp, #172
c0c6faac: 00 70 a0 e1 mov r7, r0
c0c6fab0: c8 0c 04 e3 movwr0, #19656
c0c6fab4: 80 02 4c e3 movtr0, #49792
c0c6fab8: 03 50 a0 e1 mov r5, r3
c0c6fabc: 00 00 90 e5 ldr r0, [r0]
c0c6fac0: 02 a0 a0 e1 mov r10, r2
c0c6fac4: 20 00 0b e5 str r0, [r11, #-32]
c0c6fac8: 00 40 2d e9 stmdb sp!, {lr}
c0c6facc: 4b 8b d6 eb bl 0xc0212800 <__gnu_mcount_nc> @ imm =
#-10867412
smime.p7s
Description: S/MIME Cryptographic Signature
[PATCH RESEND] mac802154: Fix uninit-value access in ieee802154_hdr_push_sechdr
The syzkaller reported an issue:
BUG: KMSAN: uninit-value in ieee802154_hdr_push_sechdr
net/ieee802154/header_ops.c:54 [inline]
BUG: KMSAN: uninit-value in ieee802154_hdr_push+0x971/0xb90
net/ieee802154/header_ops.c:108
ieee802154_hdr_push_sechdr net/ieee802154/header_ops.c:54 [inline]
ieee802154_hdr_push+0x971/0xb90 net/ieee802154/header_ops.c:108
ieee802154_header_create+0x9c0/0xc00 net/mac802154/iface.c:396
wpan_dev_hard_header include/net/cfg802154.h:494 [inline]
dgram_sendmsg+0xd1d/0x1500 net/ieee802154/socket.c:677
ieee802154_sock_sendmsg+0x91/0xc0 net/ieee802154/socket.c:96
sock_sendmsg_nosec net/socket.c:725 [inline]
sock_sendmsg net/socket.c:748 [inline]
sys_sendmsg+0x9c2/0xd60 net/socket.c:2494
___sys_sendmsg+0x28d/0x3c0 net/socket.c:2548
__sys_sendmsg+0x225/0x3c0 net/socket.c:2577
__compat_sys_sendmsg net/compat.c:346 [inline]
__do_compat_sys_sendmsg net/compat.c:353 [inline]
__se_compat_sys_sendmsg net/compat.c:350 [inline]
We found hdr->key_id_mode is uninitialized in mac802154_set_header_security()
which indicates hdr.fc.security_enabled should be 0. However, it is set to be
cb->secen before.
Later, ieee802154_hdr_push_sechdr is invoked, causing KMSAN complains
uninit-value issue.
Since mac802154_set_header_security() sets hdr.fc.security_enabled based on the
variables
ieee802154_sub_if_data *sdata and ieee802154_mac_cb *cb in a collaborative
manner.
Therefore, we should not set security_enabled prior to
mac802154_set_header_security().
Fixed it by removing the line that sets the hdr.fc.security_enabled.
Syzkaller don't provide repro, and I provide a syz repro like:
r0 = syz_init_net_socket$802154_dgram(0x24, 0x2, 0x0)
setsockopt$WPAN_SECURITY(r0, 0x0, 0x1, &(0x7f00)=0x2, 0x4)
setsockopt$WPAN_SECURITY(r0, 0x0, 0x1, &(0x7f80), 0x4)
sendmsg$802154_dgram(r0, &(0x7f000100)={&(0x7f40)={0x24, @short},
0x14, &(0x7fc0)={0x0}}, 0x0)
Fixes: 32edc40ae65c ("ieee802154: change _cb handling slightly")
Signed-off-by: Zhang Shurong
---
net/mac802154/iface.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/net/mac802154/iface.c b/net/mac802154/iface.c
index c0e2da5072be..c99b6e40a5db 100644
--- a/net/mac802154/iface.c
+++ b/net/mac802154/iface.c
@@ -368,7 +368,6 @@ static int ieee802154_header_create(struct sk_buff *skb,
memset(, 0, sizeof(hdr.fc));
hdr.fc.type = cb->type;
- hdr.fc.security_enabled = cb->secen;
hdr.fc.ack_request = cb->ackreq;
hdr.seq = atomic_inc_return(>ieee802154_ptr->dsn) & 0xFF;
--
2.30.2
Re: [PATCH] 9p: prevent read overrun in protocol dump tracepoint
On Saturday, December 2, 2023 5:35:18 AM CET asmad...@codewreck.org wrote:
> JP Kobryn wrote on Fri, Dec 01, 2023 at 07:04:10PM -0800:
> > An out of bounds read can occur within the tracepoint 9p_protocol_dump().
> > In the fast assign, there is a memcpy that uses a constant size of 32
> > (macro definition as P9_PROTO_DUMP_SZ). When the copy is invoked, the
> > source buffer is not guaranteed match this size. It was found that in some
> > cases the source buffer size is less than 32, resulting in a read that
> > overruns.
> >
> > The size of the source buffer seems to be known at the time of the
> > tracepoint being invoked. The allocations happen within p9_fcall_init(),
> > where the capacity field is set to the allocated size of the payload
> > buffer. This patch tries to fix the overrun by using the minimum of that
> > field (size of source buffer) and the size of destination buffer when
> > performing the copy.
>
> Good catch; this is a regression due to a semi-recent optimization in
> commit 60ece0833b6c ("net/9p: allocate appropriate reduced message
> buffers")
Indeed, didn't have this one on screen! Thanks!
> For some reason I thought we rounded up small messages alloc to 4k but
> I've just confirmed we don't, so these overruns are quite frequent.
> I'll add the fixes tag and cc to stable if there's no other comment.
Yeah, in p9_msg_buf_size() [net/9p/protocol.c] the smallest allocation size
for message types known to be small (at compile-time) is hard coded to 4k.
However for all variable-size message types the size is calculated at runtime
exactly as needed for that particular message being sent. So these 9p message
types can trigger this case (<32). They are currently never rounded up.
[...]
> > diff --git a/include/trace/events/9p.h b/include/trace/events/9p.h
> > index 4dfa6d7f83ba..8690a7086252 100644
> > --- a/include/trace/events/9p.h
> > +++ b/include/trace/events/9p.h
> > @@ -185,7 +185,8 @@ TRACE_EVENT(9p_protocol_dump,
> > __entry->clnt = clnt;
> > __entry->type = pdu->id;
> > __entry->tag= pdu->tag;
> > - memcpy(__entry->line, pdu->sdata, P9_PROTO_DUMP_SZ);
> > + memcpy(__entry->line, pdu->sdata,
> > + min(pdu->capacity, P9_PROTO_DUMP_SZ));
> > ),
> > TP_printk("clnt %lu %s(tag = %d)\n%.3x: %16ph\n%.3x: %16ph\n",
> > (unsigned long)__entry->clnt, show_9p_op(__entry->type),
AFAICS __entry is a local variable on stack, and array __entry->line not
intialized with zeros, i.e. the dump would contain trash at the end. Maybe
prepending memset() before memcpy()?
/Christian
[PATCH v3 2/5] dt-bindings: input/touchscreen: Add compatible for IST3038B
From: Markuss Broks Imagis IST3038B is a variant (firmware?) of Imagis IST3038 IC, add the compatible for it to the IST3038C bindings. Signed-off-by: Markuss Broks Signed-off-by: Karel Balej --- .../devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml b/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml index 0d6b033fd5fb..b5372c4eae56 100644 --- a/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml +++ b/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml @@ -18,6 +18,7 @@ properties: compatible: enum: + - imagis,ist3038b - imagis,ist3038c reg: -- 2.43.0
[PATCH v3 0/5] input/touchscreen: imagis: add support for IST3032C
From: Karel Balej This patch series generalizes the Imagis touchscreen driver to support other Imagis chips, namely IST3038B, which use a slightly different protocol. It also adds necessary information to the driver so that the IST3032C touchscreen can be used with it. The motivation for this is the samsung,coreprimevelte smartphone with which this series has been tested. However, the support for this device is not yet in-tree, the effort is happening at [1]. In particular, the driver for the regulator needed by the touchscreen on this device has not been rewritten for mainline yet. Note that this is a prerequisite for this patch [2] which implements support for touch keys for Imagis touchscreens that have it. [1] https://lore.kernel.org/all/20231102-pxa1908-lkml-v7-0-cabb1a0cb...@skole.hr/ [2] https://lore.kernel.org/all/20231112194124.24916-1-duje.mihano...@skole.hr/ --- v3: - Rebase to v6.7-rc3. - v2: https://lore.kernel.org/all/20231003133440.4696-1-kar...@gimli.ms.mff.cuni.cz/ v2: - Do not rename the driver. - Do not hardcode voltage required by the IST3032C. - Use Markuss' series which generalizes the driver. Link to the original series: https://lore.kernel.org/all/20220504152406.8730-1-markuss.br...@gmail.com/ - Separate bindings into separate patch. - v1: https://lore.kernel.org/all/20230926173531.18715-1-bal...@matfyz.cz/ --- Karel Balej (2): dt-bindings: input/touchscreen: imagis: add compatible for IST3032C input/touchscreen: imagis: add support for IST3032C Markuss Broks (3): input/touchscreen: imagis: Correct the maximum touch area value dt-bindings: input/touchscreen: Add compatible for IST3038B input/touchscreen: imagis: Add support for Imagis IST3038B .../input/touchscreen/imagis,ist3038c.yaml| 2 + drivers/input/touchscreen/imagis.c| 70 +++ 2 files changed, 60 insertions(+), 12 deletions(-) -- 2.43.0
[PATCH v3 3/5] input/touchscreen: imagis: Add support for Imagis IST3038B
From: Markuss Broks
Imagis IST3038B is another variant of Imagis IST3038 IC, which has
a different register interface from IST3038C (possibly firmware defined).
This should also work for IST3044B (though untested), however other
variants using this interface/protocol(IST3026, IST3032, IST3026B,
IST3032B) have a different format for coordinates, and they'd need
additional effort to be supported by this driver.
Signed-off-by: Markuss Broks
Signed-off-by: Karel Balej
---
drivers/input/touchscreen/imagis.c | 58 --
1 file changed, 47 insertions(+), 11 deletions(-)
diff --git a/drivers/input/touchscreen/imagis.c
b/drivers/input/touchscreen/imagis.c
index e67fd3011027..84a02672ac47 100644
--- a/drivers/input/touchscreen/imagis.c
+++ b/drivers/input/touchscreen/imagis.c
@@ -13,7 +13,7 @@
#define IST3038C_HIB_ACCESS(0x800B << 16)
#define IST3038C_DIRECT_ACCESS BIT(31)
-#define IST3038C_REG_CHIPID0x40001000
+#define IST3038C_REG_CHIPID(0x40001000 | IST3038C_DIRECT_ACCESS)
#define IST3038C_REG_HIB_BASE 0x3100
#define IST3038C_REG_TOUCH_STATUS (IST3038C_REG_HIB_BASE |
IST3038C_HIB_ACCESS)
#define IST3038C_REG_TOUCH_COORD (IST3038C_REG_HIB_BASE |
IST3038C_HIB_ACCESS | 0x8)
@@ -31,8 +31,21 @@
#define IST3038C_FINGER_COUNT_SHIFT12
#define IST3038C_FINGER_STATUS_MASKGENMASK(9, 0)
+#define IST3038B_REG_STATUS0x20
+#define IST3038B_REG_CHIPID0x30
+#define IST3038B_WHOAMI0x30380b
+
+struct imagis_properties {
+ unsigned int interrupt_msg_cmd;
+ unsigned int touch_coord_cmd;
+ unsigned int whoami_cmd;
+ unsigned int whoami_val;
+ bool protocol_b;
+};
+
struct imagis_ts {
struct i2c_client *client;
+ const struct imagis_properties *tdata;
struct input_dev *input_dev;
struct touchscreen_properties prop;
struct regulator_bulk_data supplies[2];
@@ -84,8 +97,7 @@ static irqreturn_t imagis_interrupt(int irq, void *dev_id)
int i;
int error;
- error = imagis_i2c_read_reg(ts, IST3038C_REG_INTR_MESSAGE,
- _message);
+ error = imagis_i2c_read_reg(ts, ts->tdata->interrupt_msg_cmd,
_message);
if (error) {
dev_err(>client->dev,
"failed to read the interrupt message: %d\n", error);
@@ -104,9 +116,13 @@ static irqreturn_t imagis_interrupt(int irq, void *dev_id)
finger_pressed = intr_message & IST3038C_FINGER_STATUS_MASK;
for (i = 0; i < finger_count; i++) {
- error = imagis_i2c_read_reg(ts,
- IST3038C_REG_TOUCH_COORD + (i * 4),
- _status);
+ if (ts->tdata->protocol_b)
+ error = imagis_i2c_read_reg(ts,
+ ts->tdata->touch_coord_cmd,
_status);
+ else
+ error = imagis_i2c_read_reg(ts,
+ ts->tdata->touch_coord_cmd
+ (i * 4),
+ _status);
if (error) {
dev_err(>client->dev,
"failed to read coordinates for finger %d:
%d\n",
@@ -261,6 +277,12 @@ static int imagis_probe(struct i2c_client *i2c)
ts->client = i2c;
+ ts->tdata = device_get_match_data(dev);
+ if (!ts->tdata) {
+ dev_err(dev, "missing chip data\n");
+ return -EINVAL;
+ }
+
error = imagis_init_regulators(ts);
if (error) {
dev_err(dev, "regulator init error: %d\n", error);
@@ -279,15 +301,13 @@ static int imagis_probe(struct i2c_client *i2c)
return error;
}
- error = imagis_i2c_read_reg(ts,
- IST3038C_REG_CHIPID | IST3038C_DIRECT_ACCESS,
- _id);
+ error = imagis_i2c_read_reg(ts, ts->tdata->whoami_cmd, _id);
if (error) {
dev_err(dev, "chip ID read failure: %d\n", error);
return error;
}
- if (chip_id != IST3038C_WHOAMI) {
+ if (chip_id != ts->tdata->whoami_val) {
dev_err(dev, "unknown chip ID: 0x%x\n", chip_id);
return -EINVAL;
}
@@ -343,9 +363,25 @@ static int imagis_resume(struct device *dev)
static DEFINE_SIMPLE_DEV_PM_OPS(imagis_pm_ops, imagis_suspend, imagis_resume);
+static const struct imagis_properties imagis_3038b_data = {
+ .interrupt_msg_cmd = IST3038B_REG_STATUS,
+ .touch_coord_cmd = IST3038B_REG_STATUS,
+ .whoami_cmd = IST3038B_REG_CHIPID,
+ .whoami_val = IST3038B_WHOAMI,
+ .protocol_b = true,
+};
+
+static const struct imagis_properties imagis_3038c_data = {
+ .interrupt_msg_cmd = IST3038C_REG_INTR_MESSAGE,
+
[PATCH v3 5/5] input/touchscreen: imagis: add support for IST3032C
From: Karel Balej
IST3032C is a touchscreen chip used for instance in the
samsung,coreprimevelte smartphone, with which this was tested. Add the
chip specific information to the driver.
Signed-off-by: Karel Balej
---
drivers/input/touchscreen/imagis.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/input/touchscreen/imagis.c
b/drivers/input/touchscreen/imagis.c
index 84a02672ac47..41f28e6e9cb1 100644
--- a/drivers/input/touchscreen/imagis.c
+++ b/drivers/input/touchscreen/imagis.c
@@ -35,6 +35,8 @@
#define IST3038B_REG_CHIPID0x30
#define IST3038B_WHOAMI0x30380b
+#define IST3032C_WHOAMI0x32c
+
struct imagis_properties {
unsigned int interrupt_msg_cmd;
unsigned int touch_coord_cmd;
@@ -363,6 +365,13 @@ static int imagis_resume(struct device *dev)
static DEFINE_SIMPLE_DEV_PM_OPS(imagis_pm_ops, imagis_suspend, imagis_resume);
+static const struct imagis_properties imagis_3032c_data = {
+ .interrupt_msg_cmd = IST3038C_REG_INTR_MESSAGE,
+ .touch_coord_cmd = IST3038C_REG_TOUCH_COORD,
+ .whoami_cmd = IST3038C_REG_CHIPID,
+ .whoami_val = IST3032C_WHOAMI,
+};
+
static const struct imagis_properties imagis_3038b_data = {
.interrupt_msg_cmd = IST3038B_REG_STATUS,
.touch_coord_cmd = IST3038B_REG_STATUS,
@@ -380,6 +389,7 @@ static const struct imagis_properties imagis_3038c_data = {
#ifdef CONFIG_OF
static const struct of_device_id imagis_of_match[] = {
+ { .compatible = "imagis,ist3032c", .data = _3032c_data },
{ .compatible = "imagis,ist3038b", .data = _3038b_data },
{ .compatible = "imagis,ist3038c", .data = _3038c_data },
{ },
--
2.43.0
[PATCH v3 4/5] dt-bindings: input/touchscreen: imagis: add compatible for IST3032C
From: Karel Balej Document possible usage of the Imagis driver with the IST3032C touchscreen. Signed-off-by: Karel Balej --- .../devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml b/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml index b5372c4eae56..2af71cbcc97d 100644 --- a/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml +++ b/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml @@ -18,6 +18,7 @@ properties: compatible: enum: + - imagis,ist3032c - imagis,ist3038b - imagis,ist3038c -- 2.43.0
[PATCH v3 1/5] input/touchscreen: imagis: Correct the maximum touch area value
From: Markuss Broks
As specified in downstream IST3038B driver and proved by testing,
the correct maximum reported value of touch area is 16.
Signed-off-by: Markuss Broks
Signed-off-by: Karel Balej
---
drivers/input/touchscreen/imagis.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/touchscreen/imagis.c
b/drivers/input/touchscreen/imagis.c
index 07111ca24455..e67fd3011027 100644
--- a/drivers/input/touchscreen/imagis.c
+++ b/drivers/input/touchscreen/imagis.c
@@ -210,7 +210,7 @@ static int imagis_init_input_dev(struct imagis_ts *ts)
input_set_capability(input_dev, EV_ABS, ABS_MT_POSITION_X);
input_set_capability(input_dev, EV_ABS, ABS_MT_POSITION_Y);
- input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR, 0, 255, 0, 0);
+ input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR, 0, 16, 0, 0);
touchscreen_parse_properties(input_dev, true, >prop);
if (!ts->prop.max_x || !ts->prop.max_y) {
--
2.43.0
Re: ARM Ftrace Function Graph Fails With UNWINDER_FRAME_POINTER
On Sat, 2 Dec 2023 at 09:49, Justin Chen wrote:
>
>
>
> On 12/1/23 10:53 PM, Ard Biesheuvel wrote:
> > On Fri, 1 Dec 2023 at 23:59, Justin Chen wrote:
> >>
> >>
> >>
> >> On 12/1/23 10:07 AM, Steven Rostedt wrote:
> >>> On Fri, 1 Dec 2023 09:25:59 -0800
> >>> Justin Chen wrote:
> >>>
> > It appears the sub instruction at 0x6dd0 correctly accounts for the
> > extra 8 bytes, so the frame pointer is valid. So it is our assumption
> > that there are no gaps between the stack frames is invalid.
>
> Thanks for the assistance. The gap between the stack frame depends on
> the function. Most do not have a gap. Some have 8 (as shown above), some
> have 12. A single assumption here is not going to work. I'm having a
> hard time finding out the reasoning for this gap. I tried disabling a
> bunch of gcc flags as well as -O2 and the gap still exists.
> >>>
> >>> That code was originally added because of some strange things that gcc did
> >>> with mcount (for example, it made a copy of the stack frame that it passed
> >>> to mcount, where the function graph tracer replaced the copy of the return
> >>> stack making the shadow stack go out of sync and crash). This was very
> >>> hard
> >>> to debug and I added this code to detect it if it happened again.
> >>>
> >>> Well it's been over a decade since that happened (2009).
> >>>
> >>> 71e308a239c09 ("function-graph: add stack frame test")
> >>>
> >>> I'm happy assuming that the compiler folks are aware of our tricks with
> >>> hijacking return calls and I don't expect it to happen again. We can just
> >>> rip out those checks. That is, if it's only causing false positives, I
> >>> don't think it's worth keeping around.
> >>>
> >>> Has it detected any real issues on the Arm platforms?
> >>>
> >>> -- Steve
> >>
> >> I am not familiar enough to make a call. But from my limited testing
> >> with ARM, I didn't see any issues. If you would like me to, I can submit
> >> a patch to remove the check entirely. Or maybe only disable it for ARM?
> >>
> >
> > Please try the fix I proposed first.
>
> Just tested it. Seems to do the trick.
Thanks
> Either solution works for me.
>
Given that this discussion is taking place in the context of the
report of an issue identified by GRAPH_FP_TEST, I don't see how
removing that would be a reasonable conclusion.
> FWIW I also experimented with LLVM, looks like function_graph just
> crashes regardless of the issue being discussed. The disassemble of
> LLVM[1] does something completely different.
>
LLVM does not support CONFIG_UNWINDER_FRAME_POINTER so the fact that
the prologue looks different is expected.
In the case below, the FP {r11} is correctly made to point to a {r11,
lr} tuple on the stack, so the codegen is correct AFAICT. But IIRC we
rely on unwind information for ftrace in this case, not the frame
pointer.
Could you be more specific about the crash?
>
> [1]
> LLVM dump
> c0c6faa0 :
> c0c6faa0: f0 4f 2d e9 push{r4, r5, r6, r7, r8, r9, r10, r11, lr}
> c0c6faa4: 1c b0 8d e2 add r11, sp, #28
> c0c6faa8: ac d0 4d e2 sub sp, sp, #172
> c0c6faac: 00 70 a0 e1 mov r7, r0
> c0c6fab0: c8 0c 04 e3 movwr0, #19656
> c0c6fab4: 80 02 4c e3 movtr0, #49792
> c0c6fab8: 03 50 a0 e1 mov r5, r3
> c0c6fabc: 00 00 90 e5 ldr r0, [r0]
> c0c6fac0: 02 a0 a0 e1 mov r10, r2
> c0c6fac4: 20 00 0b e5 str r0, [r11, #-32]
> c0c6fac8: 00 40 2d e9 stmdb sp!, {lr}
> c0c6facc: 4b 8b d6 eb bl 0xc0212800 <__gnu_mcount_nc> @ imm =
> #-10867412
Re: ARM Ftrace Function Graph Fails With UNWINDER_FRAME_POINTER
On 12/1/23 10:53 PM, Ard Biesheuvel wrote:
On Fri, 1 Dec 2023 at 23:59, Justin Chen wrote:
On 12/1/23 10:07 AM, Steven Rostedt wrote:
On Fri, 1 Dec 2023 09:25:59 -0800
Justin Chen wrote:
It appears the sub instruction at 0x6dd0 correctly accounts for the
extra 8 bytes, so the frame pointer is valid. So it is our assumption
that there are no gaps between the stack frames is invalid.
Thanks for the assistance. The gap between the stack frame depends on
the function. Most do not have a gap. Some have 8 (as shown above), some
have 12. A single assumption here is not going to work. I'm having a
hard time finding out the reasoning for this gap. I tried disabling a
bunch of gcc flags as well as -O2 and the gap still exists.
That code was originally added because of some strange things that gcc did
with mcount (for example, it made a copy of the stack frame that it passed
to mcount, where the function graph tracer replaced the copy of the return
stack making the shadow stack go out of sync and crash). This was very hard
to debug and I added this code to detect it if it happened again.
Well it's been over a decade since that happened (2009).
71e308a239c09 ("function-graph: add stack frame test")
I'm happy assuming that the compiler folks are aware of our tricks with
hijacking return calls and I don't expect it to happen again. We can just
rip out those checks. That is, if it's only causing false positives, I
don't think it's worth keeping around.
Has it detected any real issues on the Arm platforms?
-- Steve
I am not familiar enough to make a call. But from my limited testing
with ARM, I didn't see any issues. If you would like me to, I can submit
a patch to remove the check entirely. Or maybe only disable it for ARM?
Please try the fix I proposed first.
Just tested it. Seems to do the trick. Either solution works for me.
FWIW I also experimented with LLVM, looks like function_graph just
crashes regardless of the issue being discussed. The disassemble of
LLVM[1] does something completely different.
Thanks,
Justin
[1]
LLVM dump
c0c6faa0 :
c0c6faa0: f0 4f 2d e9 push{r4, r5, r6, r7, r8, r9, r10, r11, lr}
c0c6faa4: 1c b0 8d e2 add r11, sp, #28
c0c6faa8: ac d0 4d e2 sub sp, sp, #172
c0c6faac: 00 70 a0 e1 mov r7, r0
c0c6fab0: c8 0c 04 e3 movwr0, #19656
c0c6fab4: 80 02 4c e3 movtr0, #49792
c0c6fab8: 03 50 a0 e1 mov r5, r3
c0c6fabc: 00 00 90 e5 ldr r0, [r0]
c0c6fac0: 02 a0 a0 e1 mov r10, r2
c0c6fac4: 20 00 0b e5 str r0, [r11, #-32]
c0c6fac8: 00 40 2d e9 stmdb sp!, {lr}
c0c6facc: 4b 8b d6 eb bl 0xc0212800 <__gnu_mcount_nc> @ imm =
#-10867412
smime.p7s
Description: S/MIME Cryptographic Signature
