[PATCH] tools/virtio: pipe assertion in vring_test.c
From: Yunseong Kim The virtio_device need to fail checking when create the geust/host pipe. Signed-off-by: Yunseong Kim --- tools/virtio/vringh_test.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/virtio/vringh_test.c b/tools/virtio/vringh_test.c index 98ff808d6f0c..b1af8807c02a 100644 --- a/tools/virtio/vringh_test.c +++ b/tools/virtio/vringh_test.c @@ -161,8 +161,8 @@ static int parallel_test(u64 features, host_map = mmap(NULL, mapsize, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0); guest_map = mmap(NULL, mapsize, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0); - pipe(to_guest); - pipe(to_host); + assert(pipe(to_guest) == 0); + assert(pipe(to_host) == 0); CPU_ZERO(&cpu_set); find_cpus(&first_cpu, &last_cpu); -- 2.34.1
Re: [PATCH] tools/virtio: pipe assertion in vring_test.c
On Mon, May 27, 2024 at 04:13:31PM +0900, ysk...@gmail.com wrote: > From: Yunseong Kim > > The virtio_device need to fail checking when create the geust/host pipe. typo > > Signed-off-by: Yunseong Kim I guess ... > --- > tools/virtio/vringh_test.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/tools/virtio/vringh_test.c b/tools/virtio/vringh_test.c > index 98ff808d6f0c..b1af8807c02a 100644 > --- a/tools/virtio/vringh_test.c > +++ b/tools/virtio/vringh_test.c > @@ -161,8 +161,8 @@ static int parallel_test(u64 features, > host_map = mmap(NULL, mapsize, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0); > guest_map = mmap(NULL, mapsize, PROT_READ|PROT_WRITE, MAP_SHARED, fd, > 0); > > - pipe(to_guest); > - pipe(to_host); > + assert(pipe(to_guest) == 0); > + assert(pipe(to_host) == 0); I don't like == 0, prefer ! . Also, calling pipe outside assert is preferable, since in theory assert can be compiled out. Not an issue here but people tend to copy/paste text. > CPU_ZERO(&cpu_set); > find_cpus(&first_cpu, &last_cpu); > -- > 2.34.1
Re: [PATCH] tools/virtio: pipe assertion in vring_test.c
On 5/27/24 4:52 오후, Michael S. Tsirkin wrote: > On Mon, May 27, 2024 at 04:13:31PM +0900, ysk...@gmail.com wrote: >> From: Yunseong Kim >> >> The virtio_device need to fail checking when create the geust/host pipe. > > typo Thank you for code review Michael. Sorry, there was a typo in my message. I'll fix it and send you patch version 2. >> >> Signed-off-by: Yunseong Kim > > > I guess ... > >> --- >> tools/virtio/vringh_test.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/tools/virtio/vringh_test.c b/tools/virtio/vringh_test.c >> index 98ff808d6f0c..b1af8807c02a 100644 >> --- a/tools/virtio/vringh_test.c >> +++ b/tools/virtio/vringh_test.c >> @@ -161,8 +161,8 @@ static int parallel_test(u64 features, >> host_map = mmap(NULL, mapsize, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0); >> guest_map = mmap(NULL, mapsize, PROT_READ|PROT_WRITE, MAP_SHARED, fd, >> 0); >> >> -pipe(to_guest); >> -pipe(to_host); >> +assert(pipe(to_guest) == 0); >> +assert(pipe(to_host) == 0); > > > I don't like == 0, prefer ! . > Also, calling pipe outside assert is preferable, since in theory > assert can be compiled out. > Not an issue here but people tend to copy/paste text. I agree, it's uncomfortable even if I did it. I'll fix it as you suggested and send it to patch 2. Thank you! Warm Regards, Yunseong Kim >> CPU_ZERO(&cpu_set); >> find_cpus(&first_cpu, &last_cpu); >> -- >> 2.34.1 >
[PATCH v2 0/5] arm64: qcom: sa8775p: enable remoteprocs - ADSP, CDSP and GPDSP
Add DT bindings, relevant DT defines, DTS nodes and driver changes required to enable the remoteprocs on sa8775p. Signed-off-by: Bartosz Golaszewski --- Changes in v2: - Move the DT bindings for sa8775p-pas into a separate file - Link to v1: https://lore.kernel.org/r/20240522-topic-lemans-iot-remoteproc-v1-0-af9fab7b2...@linaro.org --- Bartosz Golaszewski (2): dt-bindings: remoteproc: qcom,sa8775p-pas: Document the SA8775p ADSP, CDSP and GPDSP arm64: dts: qcom: sa8775p-ride: enable remoteprocs Tengfei Fan (3): dt-bindings: mailbox: qcom-ipcc: Add GPDSP0 and GPDSP1 clients remoteproc: qcom_q6v5_pas: Add support for SA8775p ADSP, CDSP and GPDSP arm64: dts: qcom: sa8775p: add ADSP, CDSP and GPDSP nodes .../bindings/remoteproc/qcom,sa8775p-pas.yaml | 177 +++ arch/arm64/boot/dts/qcom/sa8775p-ride.dts | 25 ++ arch/arm64/boot/dts/qcom/sa8775p.dtsi | 332 + drivers/remoteproc/qcom_q6v5_pas.c | 92 ++ include/dt-bindings/mailbox/qcom-ipcc.h| 2 + 5 files changed, 628 insertions(+) --- base-commit: 3689b0ef08b70e4e03b82ebd37730a03a672853a change-id: 20240507-topic-lemans-iot-remoteproc-6647b50281e2 Best regards, -- Bartosz Golaszewski
[PATCH v2 1/5] dt-bindings: remoteproc: qcom,sa8775p-pas: Document the SA8775p ADSP, CDSP and GPDSP
From: Bartosz Golaszewski Document the components used to boot the ADSP, CDSP0, CDSP1, GPDSP0 and GPDSP1 on the SA8775p SoC. Co-developed-by: Tengfei Fan Signed-off-by: Bartosz Golaszewski --- .../bindings/remoteproc/qcom,sa8775p-pas.yaml | 177 + 1 file changed, 177 insertions(+) diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,sa8775p-pas.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,sa8775p-pas.yaml new file mode 100644 index ..77a5e11555de --- /dev/null +++ b/Documentation/devicetree/bindings/remoteproc/qcom,sa8775p-pas.yaml @@ -0,0 +1,177 @@ +# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/remoteproc/qcom,sa8775p-pas.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Qualcomm SA8775p Peripheral Authentication Service + +maintainers: + - Bartosz Golaszewski + +description: + Qualcomm SA8775p SoC Peripheral Authentication Service loads and boots firmware + on the Qualcomm DSP Hexagon cores. + +properties: + compatible: +enum: + - qcom,sa8775p-adsp-pas + - qcom,sa8775p-cdsp0-pas + - qcom,sa8775p-cdsp1-pas + - qcom,sa8775p-gpdsp0-pas + - qcom,sa8775p-gpdsp1-pas + + reg: +maxItems: 1 + + clocks: +items: + - description: XO clock + + clock-names: +items: + - const: xo + + qcom,qmp: +$ref: /schemas/types.yaml#/definitions/phandle +description: Reference to the AOSS side-channel message RAM. + + firmware-name: +$ref: /schemas/types.yaml#/definitions/string-array +items: + - description: Firmware name of the Hexagon core + + memory-region: +items: + - description: Memory region for main Firmware authentication + + interrupts: +maxItems: 5 + + interrupt-names: +maxItems: 5 + +required: + - compatible + - reg + - memory-region + +allOf: + - $ref: /schemas/remoteproc/qcom,pas-common.yaml# + + - if: + properties: +compatible: + enum: +- qcom,sa8775p-adsp-pas +then: + properties: +power-domains: + items: +- description: LCX power domain +- description: LMX power domain +power-domain-names: + items: +- const: lcx +- const: lmx + + - if: + properties: +compatible: + enum: +- qcom,sa8775p-cdsp-pas +then: + properties: +power-domains: + items: +- description: CX power domain +- description: MXC power domain +- description: NSP0 power domain +power-domain-names: + items: +- const: cx +- const: mxc +- const: nsp0 + + - if: + properties: +compatible: + enum: +- qcom,sa8775p-cdsp1-pas +then: + properties: +power-domains: + items: +- description: CX power domain +- description: MXC power domain +- description: NSP1 power domain +power-domain-names: + items: +- const: cx +- const: mxc +- const: nsp1 + + - if: + properties: +compatible: + enum: +- qcom,sa8775p-gpdsp0-pas +- qcom,sa8775p-gpdsp1-pas +then: + properties: +power-domains: + items: +- description: CX power domain +- description: MXC power domain +power-domain-names: + items: +- const: cx +- const: mxc + +unevaluatedProperties: false + +examples: + - | +#include +#include +#include +#include +#include + +remoteproc@3000 { +compatible = "qcom,sa8775p-adsp-pas"; +reg = <0x3000 0x100>; + +interrupts-extended = <&pdc 6 IRQ_TYPE_EDGE_RISING>, + <&smp2p_adsp_in 0 IRQ_TYPE_EDGE_RISING>, + <&smp2p_adsp_in 2 IRQ_TYPE_EDGE_RISING>, + <&smp2p_adsp_in 1 IRQ_TYPE_EDGE_RISING>, + <&smp2p_adsp_in 3 IRQ_TYPE_EDGE_RISING>; +interrupt-names = "wdog", "fatal", "ready", "handover", "stop-ack"; + +clocks = <&rpmhcc RPMH_CXO_CLK>; +clock-names = "xo"; + +power-domains = <&rpmhpd RPMHPD_LCX>, <&rpmhpd RPMHPD_LMX>; +power-domain-names = "lcx", "lmx"; + +interconnects = <&lpass_ag_noc MASTER_LPASS_PROC 0 &mc_virt SLAVE_EBI1 0>; + +memory-region = <&pil_adsp_mem>; + +qcom,qmp = <&aoss_qmp>; + +qcom,smem-states = <&smp2p_adsp_out 0>; +qcom,smem-state-names = "stop"; + +glink-edge { +interrupts-extended = <&ipcc IPCC_CLIENT_LPASS + IPCC_MPROC_SIGNAL_GLINK_QMP + IRQ_TYPE_EDGE_RISING>; +mboxes = <&ipcc IPCC_CLIENT_LPASS IPCC_MPROC_SIGNAL_GLIN
[PATCH v2 2/5] dt-bindings: mailbox: qcom-ipcc: Add GPDSP0 and GPDSP1 clients
From: Tengfei Fan Add GPDSP0 and GPDSP1 clients for SA8775p platform. Signed-off-by: Tengfei Fan Acked-by: Krzysztof Kozlowski Signed-off-by: Bartosz Golaszewski --- include/dt-bindings/mailbox/qcom-ipcc.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/dt-bindings/mailbox/qcom-ipcc.h b/include/dt-bindings/mailbox/qcom-ipcc.h index fbfa3febc66d..fd85a79381b3 100644 --- a/include/dt-bindings/mailbox/qcom-ipcc.h +++ b/include/dt-bindings/mailbox/qcom-ipcc.h @@ -33,5 +33,7 @@ #define IPCC_CLIENT_NSP1 18 #define IPCC_CLIENT_TME23 #define IPCC_CLIENT_WPSS 24 +#define IPCC_CLIENT_GPDSP0 31 +#define IPCC_CLIENT_GPDSP1 32 #endif -- 2.43.0
[PATCH v2 3/5] remoteproc: qcom_q6v5_pas: Add support for SA8775p ADSP, CDSP and GPDSP
From: Tengfei Fan Add support for PIL loading on ADSP, CDSP0, CDSP1, GPDSP0 and GPDSP1 on SA8775p SoCs. Signed-off-by: Tengfei Fan Co-developed-by: Bartosz Golaszewski Signed-off-by: Bartosz Golaszewski --- drivers/remoteproc/qcom_q6v5_pas.c | 92 ++ 1 file changed, 92 insertions(+) diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c index 54d8005d40a3..16053aa99298 100644 --- a/drivers/remoteproc/qcom_q6v5_pas.c +++ b/drivers/remoteproc/qcom_q6v5_pas.c @@ -820,6 +820,23 @@ static const struct adsp_data adsp_resource_init = { .ssctl_id = 0x14, }; +static const struct adsp_data sa8775p_adsp_resource = { + .crash_reason_smem = 423, + .firmware_name = "adsp.mdt", + .pas_id = 1, + .minidump_id = 5, + .auto_boot = true, + .proxy_pd_names = (char*[]){ + "lcx", + "lmx", + NULL + }, + .load_state = "adsp", + .ssr_name = "lpass", + .sysmon_name = "adsp", + .ssctl_id = 0x14, +}; + static const struct adsp_data sdm845_adsp_resource_init = { .crash_reason_smem = 423, .firmware_name = "adsp.mdt", @@ -933,6 +950,42 @@ static const struct adsp_data cdsp_resource_init = { .ssctl_id = 0x17, }; +static const struct adsp_data sa8775p_cdsp0_resource = { + .crash_reason_smem = 601, + .firmware_name = "cdsp0.mdt", + .pas_id = 18, + .minidump_id = 7, + .auto_boot = true, + .proxy_pd_names = (char*[]){ + "cx", + "mxc", + "nsp0", + NULL + }, + .load_state = "cdsp", + .ssr_name = "cdsp", + .sysmon_name = "cdsp", + .ssctl_id = 0x17, +}; + +static const struct adsp_data sa8775p_cdsp1_resource = { + .crash_reason_smem = 633, + .firmware_name = "cdsp1.mdt", + .pas_id = 30, + .minidump_id = 20, + .auto_boot = true, + .proxy_pd_names = (char*[]){ + "cx", + "mxc", + "nsp1", + NULL + }, + .load_state = "nsp", + .ssr_name = "cdsp1", + .sysmon_name = "cdsp1", + .ssctl_id = 0x20, +}; + static const struct adsp_data sdm845_cdsp_resource_init = { .crash_reason_smem = 601, .firmware_name = "cdsp.mdt", @@ -1074,6 +1127,40 @@ static const struct adsp_data sm8350_cdsp_resource = { .ssctl_id = 0x17, }; +static const struct adsp_data sa8775p_gpdsp0_resource = { + .crash_reason_smem = 640, + .firmware_name = "gpdsp0.mdt", + .pas_id = 39, + .minidump_id = 21, + .auto_boot = true, + .proxy_pd_names = (char*[]){ + "cx", + "mxc", + NULL + }, + .load_state = "gpdsp0", + .ssr_name = "gpdsp0", + .sysmon_name = "gpdsp0", + .ssctl_id = 0x21, +}; + +static const struct adsp_data sa8775p_gpdsp1_resource = { + .crash_reason_smem = 641, + .firmware_name = "gpdsp1.mdt", + .pas_id = 40, + .minidump_id = 22, + .auto_boot = true, + .proxy_pd_names = (char*[]){ + "cx", + "mxc", + NULL + }, + .load_state = "gpdsp1", + .ssr_name = "gpdsp1", + .sysmon_name = "gpdsp1", + .ssctl_id = 0x22, +}; + static const struct adsp_data mpss_resource_init = { .crash_reason_smem = 421, .firmware_name = "modem.mdt", @@ -1315,6 +1402,11 @@ static const struct of_device_id adsp_of_match[] = { { .compatible = "qcom,qcs404-adsp-pas", .data = &adsp_resource_init }, { .compatible = "qcom,qcs404-cdsp-pas", .data = &cdsp_resource_init }, { .compatible = "qcom,qcs404-wcss-pas", .data = &wcss_resource_init }, + { .compatible = "qcom,sa8775p-adsp-pas", .data = &sa8775p_adsp_resource}, + { .compatible = "qcom,sa8775p-cdsp0-pas", .data = &sa8775p_cdsp0_resource}, + { .compatible = "qcom,sa8775p-cdsp1-pas", .data = &sa8775p_cdsp1_resource}, + { .compatible = "qcom,sa8775p-gpdsp0-pas", .data = &sa8775p_gpdsp0_resource}, + { .compatible = "qcom,sa8775p-gpdsp1-pas", .data = &sa8775p_gpdsp1_resource}, { .compatible = "qcom,sc7180-adsp-pas", .data = &sm8250_adsp_resource}, { .compatible = "qcom,sc7180-mpss-pas", .data = &mpss_resource_init}, { .compatible = "qcom,sc7280-adsp-pas", .data = &sm8350_adsp_resource}, -- 2.43.0
[PATCH v2 4/5] arm64: dts: qcom: sa8775p: add ADSP, CDSP and GPDSP nodes
From: Tengfei Fan Add nodes for remoteprocs: ADSP, CDSP0, CDSP1, GPDSP0 and GPDSP1 for SA8775p SoCs. Signed-off-by: Tengfei Fan Co-developed-by: Bartosz Golaszewski Signed-off-by: Bartosz Golaszewski --- arch/arm64/boot/dts/qcom/sa8775p.dtsi | 332 ++ 1 file changed, 332 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/sa8775p.dtsi b/arch/arm64/boot/dts/qcom/sa8775p.dtsi index 31de73594839..5c0b61a5624b 100644 --- a/arch/arm64/boot/dts/qcom/sa8775p.dtsi +++ b/arch/arm64/boot/dts/qcom/sa8775p.dtsi @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -544,6 +545,121 @@ cpucp_fw_mem: cpucp-fw@db20 { }; }; + smp2p-adsp { + compatible = "qcom,smp2p"; + qcom,smem = <443>, <429>; + interrupts-extended = <&ipcc IPCC_CLIENT_LPASS +IPCC_MPROC_SIGNAL_SMP2P +IRQ_TYPE_EDGE_RISING>; + mboxes = <&ipcc IPCC_CLIENT_LPASS IPCC_MPROC_SIGNAL_SMP2P>; + + qcom,local-pid = <0>; + qcom,remote-pid = <2>; + + smp2p_adsp_out: master-kernel { + qcom,entry-name = "master-kernel"; + #qcom,smem-state-cells = <1>; + }; + + smp2p_adsp_in: slave-kernel { + qcom,entry-name = "slave-kernel"; + interrupt-controller; + #interrupt-cells = <2>; + }; + }; + + smp2p-cdsp0 { + compatible = "qcom,smp2p"; + qcom,smem = <94>, <432>; + interrupts-extended = <&ipcc IPCC_CLIENT_CDSP +IPCC_MPROC_SIGNAL_SMP2P +IRQ_TYPE_EDGE_RISING>; + mboxes = <&ipcc IPCC_CLIENT_CDSP IPCC_MPROC_SIGNAL_SMP2P>; + + qcom,local-pid = <0>; + qcom,remote-pid = <5>; + + smp2p_cdsp0_out: master-kernel { + qcom,entry-name = "master-kernel"; + #qcom,smem-state-cells = <1>; + }; + + smp2p_cdsp0_in: slave-kernel { + qcom,entry-name = "slave-kernel"; + interrupt-controller; + #interrupt-cells = <2>; + }; + }; + + smp2p-cdsp1 { + compatible = "qcom,smp2p"; + qcom,smem = <617>, <616>; + interrupts-extended = <&ipcc IPCC_CLIENT_NSP1 +IPCC_MPROC_SIGNAL_SMP2P +IRQ_TYPE_EDGE_RISING>; + mboxes = <&ipcc IPCC_CLIENT_NSP1 IPCC_MPROC_SIGNAL_SMP2P>; + + qcom,local-pid = <0>; + qcom,remote-pid = <12>; + + smp2p_cdsp1_out: master-kernel { + qcom,entry-name = "master-kernel"; + #qcom,smem-state-cells = <1>; + }; + + smp2p_cdsp1_in: slave-kernel { + qcom,entry-name = "slave-kernel"; + interrupt-controller; + #interrupt-cells = <2>; + }; + }; + + smp2p-gpdsp0 { + compatible = "qcom,smp2p"; + qcom,smem = <617>, <616>; + interrupts-extended = <&ipcc IPCC_CLIENT_GPDSP0 +IPCC_MPROC_SIGNAL_SMP2P +IRQ_TYPE_EDGE_RISING>; + mboxes = <&ipcc IPCC_CLIENT_GPDSP0 IPCC_MPROC_SIGNAL_SMP2P>; + + qcom,local-pid = <0>; + qcom,remote-pid = <17>; + + smp2p_gpdsp0_out: master-kernel { + qcom,entry-name = "master-kernel"; + #qcom,smem-state-cells = <1>; + }; + + smp2p_gpdsp0_in: slave-kernel { + qcom,entry-name = "slave-kernel"; + interrupt-controller; + #interrupt-cells = <2>; + }; + }; + + smp2p-gpdsp1 { + compatible = "qcom,smp2p"; + qcom,smem = <617>, <616>; + interrupts-extended = <&ipcc IPCC_CLIENT_GPDSP1 +IPCC_MPROC_SIGNAL_SMP2P +IRQ_TYPE_EDGE_RISING>; + mboxes = <&ipcc IPCC_CLIENT_GPDSP1 IPCC_MPROC_SIGNAL_SMP2P>; + + qcom,local-pid = <0>; + qcom,remote-pid = <18>; + + smp2p_gpdsp1_out: master-kernel { + qcom,entry-name = "master-kernel"; + #qcom,smem-state-cells = <1>; + }; + + smp2p_gpdsp1_in: slave-kernel { + qcom,entry-name = "slave-kernel"; +
[PATCH v2 5/5] arm64: dts: qcom: sa8775p-ride: enable remoteprocs
From: Bartosz Golaszewski Enable all remoteproc nodes on the sa8775p-ride board and point to the appropriate firmware files. Signed-off-by: Bartosz Golaszewski --- arch/arm64/boot/dts/qcom/sa8775p-ride.dts | 25 + 1 file changed, 25 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/sa8775p-ride.dts b/arch/arm64/boot/dts/qcom/sa8775p-ride.dts index 26ad05bd3b3f..071fcaf09364 100644 --- a/arch/arm64/boot/dts/qcom/sa8775p-ride.dts +++ b/arch/arm64/boot/dts/qcom/sa8775p-ride.dts @@ -727,6 +727,31 @@ &pcie1_phy { status = "okay"; }; +&remoteproc_adsp { + firmware-name = "qcom/sa8775p/adsp.mbn"; + status = "okay"; +}; + +&remoteproc_cdsp0 { + firmware-name = "qcom/sa8775p/cdsp0.mbn"; + status = "okay"; +}; + +&remoteproc_cdsp1 { + firmware-name = "qcom/sa8775p/cdsp1.mbn"; + status = "okay"; +}; + +&remoteproc_gpdsp0 { + firmware-name = "qcom/sa8775p/gpdsp0.mbn"; + status = "okay"; +}; + +&remoteproc_gpdsp1 { + firmware-name = "qcom/sa8775p/gpdsp1.mbn"; + status = "okay"; +}; + &uart10 { compatible = "qcom,geni-debug-uart"; pinctrl-0 = <&qup_uart10_default>; -- 2.43.0
Re: [PATCH v2 1/5] dt-bindings: remoteproc: qcom,sa8775p-pas: Document the SA8775p ADSP, CDSP and GPDSP
On 27/05/2024 10:43, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski > > Document the components used to boot the ADSP, CDSP0, CDSP1, GPDSP0 and > GPDSP1 on the SA8775p SoC. > > Co-developed-by: Tengfei Fan Missing SoB. > Signed-off-by: Bartosz Golaszewski ... > + > +allOf: > + - $ref: /schemas/remoteproc/qcom,pas-common.yaml# > + > + - if: > + properties: > +compatible: > + enum: > +- qcom,sa8775p-adsp-pas > +then: > + properties: > +power-domains: > + items: > +- description: LCX power domain > +- description: LMX power domain > +power-domain-names: > + items: > +- const: lcx > +- const: lmx > + > + - if: > + properties: > +compatible: > + enum: > +- qcom,sa8775p-cdsp-pas cdsp0 > +then: > + properties: > +power-domains: > + items: > +- description: CX power domain > +- description: MXC power domain > +- description: NSP0 power domain > +power-domain-names: > + items: > +- const: cx > +- const: mxc > +- const: nsp0 Shouldn't this be just nsp, so both cdsp0 and cdsp1 entries can be unified? That's the power domain from the device point of view, so the device expects to be in some NSP domain, not explicitly NSPn. Best regards, Krzysztof
Re: [PATCH v2 3/5] remoteproc: qcom_q6v5_pas: Add support for SA8775p ADSP, CDSP and GPDSP
On Mon, May 27, 2024 at 10:43:50AM +0200, Bartosz Golaszewski wrote: > From: Tengfei Fan > > Add support for PIL loading on ADSP, CDSP0, CDSP1, GPDSP0 and GPDSP1 on > SA8775p SoCs. > > Signed-off-by: Tengfei Fan > Co-developed-by: Bartosz Golaszewski > Signed-off-by: Bartosz Golaszewski > --- > drivers/remoteproc/qcom_q6v5_pas.c | 92 > ++ > 1 file changed, 92 insertions(+) > > diff --git a/drivers/remoteproc/qcom_q6v5_pas.c > b/drivers/remoteproc/qcom_q6v5_pas.c > index 54d8005d40a3..16053aa99298 100644 > --- a/drivers/remoteproc/qcom_q6v5_pas.c > +++ b/drivers/remoteproc/qcom_q6v5_pas.c > @@ -820,6 +820,23 @@ static const struct adsp_data adsp_resource_init = { > .ssctl_id = 0x14, > }; > > +static const struct adsp_data sa8775p_adsp_resource = { > + .crash_reason_smem = 423, > + .firmware_name = "adsp.mdt", mbn please. Other than that LGTM > + .pas_id = 1, > + .minidump_id = 5, > + .auto_boot = true, > + .proxy_pd_names = (char*[]){ > + "lcx", > + "lmx", > + NULL > + }, > + .load_state = "adsp", > + .ssr_name = "lpass", > + .sysmon_name = "adsp", > + .ssctl_id = 0x14, > +}; > + > static const struct adsp_data sdm845_adsp_resource_init = { > .crash_reason_smem = 423, > .firmware_name = "adsp.mdt", > @@ -933,6 +950,42 @@ static const struct adsp_data cdsp_resource_init = { > .ssctl_id = 0x17, > }; > > +static const struct adsp_data sa8775p_cdsp0_resource = { > + .crash_reason_smem = 601, > + .firmware_name = "cdsp0.mdt", > + .pas_id = 18, > + .minidump_id = 7, > + .auto_boot = true, > + .proxy_pd_names = (char*[]){ > + "cx", > + "mxc", > + "nsp0", > + NULL > + }, > + .load_state = "cdsp", > + .ssr_name = "cdsp", > + .sysmon_name = "cdsp", > + .ssctl_id = 0x17, > +}; > + > +static const struct adsp_data sa8775p_cdsp1_resource = { > + .crash_reason_smem = 633, > + .firmware_name = "cdsp1.mdt", > + .pas_id = 30, > + .minidump_id = 20, > + .auto_boot = true, > + .proxy_pd_names = (char*[]){ > + "cx", > + "mxc", > + "nsp1", > + NULL > + }, > + .load_state = "nsp", > + .ssr_name = "cdsp1", > + .sysmon_name = "cdsp1", > + .ssctl_id = 0x20, > +}; > + > static const struct adsp_data sdm845_cdsp_resource_init = { > .crash_reason_smem = 601, > .firmware_name = "cdsp.mdt", > @@ -1074,6 +1127,40 @@ static const struct adsp_data sm8350_cdsp_resource = { > .ssctl_id = 0x17, > }; > > +static const struct adsp_data sa8775p_gpdsp0_resource = { > + .crash_reason_smem = 640, > + .firmware_name = "gpdsp0.mdt", > + .pas_id = 39, > + .minidump_id = 21, > + .auto_boot = true, > + .proxy_pd_names = (char*[]){ > + "cx", > + "mxc", > + NULL > + }, > + .load_state = "gpdsp0", > + .ssr_name = "gpdsp0", > + .sysmon_name = "gpdsp0", > + .ssctl_id = 0x21, > +}; > + > +static const struct adsp_data sa8775p_gpdsp1_resource = { > + .crash_reason_smem = 641, > + .firmware_name = "gpdsp1.mdt", > + .pas_id = 40, > + .minidump_id = 22, > + .auto_boot = true, > + .proxy_pd_names = (char*[]){ > + "cx", > + "mxc", > + NULL > + }, > + .load_state = "gpdsp1", > + .ssr_name = "gpdsp1", > + .sysmon_name = "gpdsp1", > + .ssctl_id = 0x22, > +}; > + > static const struct adsp_data mpss_resource_init = { > .crash_reason_smem = 421, > .firmware_name = "modem.mdt", > @@ -1315,6 +1402,11 @@ static const struct of_device_id adsp_of_match[] = { > { .compatible = "qcom,qcs404-adsp-pas", .data = &adsp_resource_init }, > { .compatible = "qcom,qcs404-cdsp-pas", .data = &cdsp_resource_init }, > { .compatible = "qcom,qcs404-wcss-pas", .data = &wcss_resource_init }, > + { .compatible = "qcom,sa8775p-adsp-pas", .data = > &sa8775p_adsp_resource}, > + { .compatible = "qcom,sa8775p-cdsp0-pas", .data = > &sa8775p_cdsp0_resource}, > + { .compatible = "qcom,sa8775p-cdsp1-pas", .data = > &sa8775p_cdsp1_resource}, > + { .compatible = "qcom,sa8775p-gpdsp0-pas", .data = > &sa8775p_gpdsp0_resource}, > + { .compatible = "qcom,sa8775p-gpdsp1-pas", .data = > &sa8775p_gpdsp1_resource}, > { .compatible = "qcom,sc7180-adsp-pas", .data = &sm8250_adsp_resource}, > { .compatible = "qcom,sc7180-mpss-pas", .data = &mpss_resource_init}, > { .compatible = "qcom,sc7280-adsp-pas", .data = &sm8350_adsp_resource}, > > -- > 2.43.0 > -- With best wishes Dmitry
Re: [PATCH v2 4/5] arm64: dts: qcom: sa8775p: add ADSP, CDSP and GPDSP nodes
On Mon, May 27, 2024 at 10:43:51AM +0200, Bartosz Golaszewski wrote: > From: Tengfei Fan > > Add nodes for remoteprocs: ADSP, CDSP0, CDSP1, GPDSP0 and GPDSP1 for > SA8775p SoCs. > > Signed-off-by: Tengfei Fan > Co-developed-by: Bartosz Golaszewski > Signed-off-by: Bartosz Golaszewski > --- > arch/arm64/boot/dts/qcom/sa8775p.dtsi | 332 > ++ > 1 file changed, 332 insertions(+) > With nsp0 vs nsp1 vs nsp sorted out: Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH v2 5/5] arm64: dts: qcom: sa8775p-ride: enable remoteprocs
On Mon, May 27, 2024 at 10:43:52AM +0200, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski > > Enable all remoteproc nodes on the sa8775p-ride board and point to the > appropriate firmware files. > > Signed-off-by: Bartosz Golaszewski > --- > arch/arm64/boot/dts/qcom/sa8775p-ride.dts | 25 + > 1 file changed, 25 insertions(+) > Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH 2/2] ring-buffer: Fix a race between readers and resize checks
On 5/20/24 15:50, Steven Rostedt wrote: > On Fri, 17 May 2024 15:40:08 +0200 > Petr Pavlu wrote: > >> The reader code in rb_get_reader_page() swaps a new reader page into the >> ring buffer by doing cmpxchg on old->list.prev->next to point it to the >> new page. Following that, if the operation is successful, >> old->list.next->prev gets updated too. This means the underlying >> doubly-linked list is temporarily inconsistent, page->prev->next or >> page->next->prev might not be equal back to page for some page in the >> ring buffer. >> >> The resize operation in ring_buffer_resize() can be invoked in parallel. >> It calls rb_check_pages() which can detect the described inconsistency >> and stop further tracing: >> >> [ 190.271762] [ cut here ] >> [ 190.271771] WARNING: CPU: 1 PID: 6186 at kernel/trace/ring_buffer.c:1467 >> rb_check_pages.isra.0+0x6a/0xa0 >> [ 190.271789] Modules linked in: [...] >> [ 190.271991] Unloaded tainted modules: intel_uncore_frequency(E):1 >> skx_edac(E):1 >> [ 190.272002] CPU: 1 PID: 6186 Comm: cmd.sh Kdump: loaded Tainted: G >> E 6.9.0-rc6-default #5 158d3e1e6d0b091c34c3b96bfd99a1c58306d79f >> [ 190.272011] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS >> rel-1.16.0-0-gd239552c-rebuilt.opensuse.org 04/01/2014 >> [ 190.272015] RIP: 0010:rb_check_pages.isra.0+0x6a/0xa0 >> [ 190.272023] Code: [...] >> [ 190.272028] RSP: 0018:9c37463abb70 EFLAGS: 00010206 >> [ 190.272034] RAX: 8eba04b6cb80 RBX: 0007 RCX: >> 8eba01f13d80 >> [ 190.272038] RDX: 8eba01f130c0 RSI: 8eba04b6cd00 RDI: >> 8eba0004c700 >> [ 190.272042] RBP: 8eba0004c700 R08: 00010002 R09: >> >> [ 190.272045] R10: 7f52 R11: 8eba7f60 R12: >> 8eba0004c720 >> [ 190.272049] R13: 8eba00223a00 R14: 0008 R15: >> 8eba067a8000 >> [ 190.272053] FS: 7f1bd64752c0() GS:8eba7f68() >> knlGS: >> [ 190.272057] CS: 0010 DS: ES: CR0: 80050033 >> [ 190.272061] CR2: 7f1bd6662590 CR3: 00010291e001 CR4: >> 00370ef0 >> [ 190.272070] DR0: DR1: DR2: >> >> [ 190.272073] DR3: DR6: fffe0ff0 DR7: >> 0400 >> [ 190.272077] Call Trace: >> [ 190.272098] >> [ 190.272189] ring_buffer_resize+0x2ab/0x460 >> [ 190.272199] __tracing_resize_ring_buffer.part.0+0x23/0xa0 >> [ 190.272206] tracing_resize_ring_buffer+0x65/0x90 >> [ 190.272216] tracing_entries_write+0x74/0xc0 >> [ 190.272225] vfs_write+0xf5/0x420 >> [ 190.272248] ksys_write+0x67/0xe0 >> [ 190.272256] do_syscall_64+0x82/0x170 >> [ 190.272363] entry_SYSCALL_64_after_hwframe+0x76/0x7e >> [ 190.272373] RIP: 0033:0x7f1bd657d263 >> [ 190.272381] Code: [...] >> [ 190.272385] RSP: 002b:7ffe72b643f8 EFLAGS: 0246 ORIG_RAX: >> 0001 >> [ 190.272391] RAX: ffda RBX: 0002 RCX: >> 7f1bd657d263 >> [ 190.272395] RDX: 0002 RSI: 555a6eb538e0 RDI: >> 0001 >> [ 190.272398] RBP: 555a6eb538e0 R08: 000a R09: >> >> [ 190.272401] R10: 555a6eb55190 R11: 0246 R12: >> 7f1bd6662500 >> [ 190.272404] R13: 0002 R14: 7f1bd6667c00 R15: >> 0002 >> [ 190.272412] >> [ 190.272414] ---[ end trace ]--- >> >> Note that ring_buffer_resize() calls rb_check_pages() only if the parent >> trace_buffer has recording disabled. Recent commit d78ab792705c >> ("tracing: Stop current tracer when resizing buffer") causes that it is >> now always the case which makes it more likely to experience this issue. >> >> The window to hit this race is nonetheless very small. To help >> reproducing it, one can add a delay loop in rb_get_reader_page(): >> >> ret = rb_head_page_replace(reader, cpu_buffer->reader_page); >> if (!ret) >> goto spin; >> for (unsigned i = 0; i < 1U << 26; i++) /* inserted delay loop */ >> __asm__ __volatile__ ("" : : : "memory"); >> rb_list_head(reader->list.next)->prev = &cpu_buffer->reader_page->list; >> >> .. and then run the following commands on the target system: >> >> echo 1 > /sys/kernel/tracing/events/sched/sched_switch/enable >> while true; do >> echo 16 > /sys/kernel/tracing/buffer_size_kb; sleep 0.1 >> echo 8 > /sys/kernel/tracing/buffer_size_kb; sleep 0.1 >> done & >> while true; do >> for i in /sys/kernel/tracing/per_cpu/*; do >> timeout 0.1 cat $i/trace_pipe; sleep 0.2 >> done >> done >> >> To fix the problem, make sure ring_buffer_resize() doesn't invoke >> rb_check_pages() concurrently with a reader operating on the same >> ring_buffer_per_cpu by taking its cpu_buffer->reader_lock. > > Definitely a bug. Thanks for catching it. But... > >> >> Fixes: 659f451ff213 ("ring-buffer: Add integrity check at end of iter read") >
[PATCH v2] tracing/probes: fix error check in parse_btf_field()
btf_find_struct_member() might return NULL or an error via the ERR_PTR() macro. However, its caller in parse_btf_field() only checks for the NULL condition. Fix this by using IS_ERR() and returning the error up the stack. Fixes: c440adfbe3025 ("tracing/probes: Support BTF based data structure field access") Signed-off-by: Carlos López --- v2: added call to trace_probe_log_err() kernel/trace/trace_probe.c | 4 1 file changed, 4 insertions(+) diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c index 5e263c141574..39877c80d6cb 100644 --- a/kernel/trace/trace_probe.c +++ b/kernel/trace/trace_probe.c @@ -554,6 +554,10 @@ static int parse_btf_field(char *fieldname, const struct btf_type *type, anon_offs = 0; field = btf_find_struct_member(ctx->btf, type, fieldname, &anon_offs); + if (IS_ERR(field)) { + trace_probe_log_err(ctx->offset, BAD_BTF_TID); + return PTR_ERR(field); + } if (!field) { trace_probe_log_err(ctx->offset, NO_BTF_FIELD); return -ENOENT; -- 2.35.3
[PATCH v2] tools/virtio: creating pipe assertion in vringh_test
From: Yunseong Kim The parallel_test() function in vringh_test needs to verify the creation of the guest/host pipe. Signed-off-by: Yunseong Kim --- tools/virtio/vringh_test.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tools/virtio/vringh_test.c b/tools/virtio/vringh_test.c index 98ff808d6f0c..43d3a6aa1dcf 100644 --- a/tools/virtio/vringh_test.c +++ b/tools/virtio/vringh_test.c @@ -139,7 +139,7 @@ static int parallel_test(u64 features, bool fast_vringh) { void *host_map, *guest_map; - int fd, mapsize, to_guest[2], to_host[2]; + int pipe_ret, fd, mapsize, to_guest[2], to_host[2]; unsigned long xfers = 0, notifies = 0, receives = 0; unsigned int first_cpu, last_cpu; cpu_set_t cpu_set; @@ -161,8 +161,11 @@ static int parallel_test(u64 features, host_map = mmap(NULL, mapsize, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0); guest_map = mmap(NULL, mapsize, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0); - pipe(to_guest); - pipe(to_host); + pipe_ret = pipe(to_guest); + assert(!pipe_ret); + + pipe_ret = pipe(to_host); + assert(!pipe_ret); CPU_ZERO(&cpu_set); find_cpus(&first_cpu, &last_cpu); -- 2.34.1
[PATCH v2] ftrace: Fix stack trace entry generated by ftrace_pid_func()
On setting set_ftrace_pid, a extra entry generated by ftrace_pid_func() is shown on stack trace(CONFIG_UNWINDER_FRAME_POINTER=y). [004] .68.459382: => 0xa00090af => ksys_read => __x64_sys_read => x64_sys_call => do_syscall_64 => entry_SYSCALL_64_after_hwframe To resolve this issue, increment skip count in function_stack_trace_call() if pids are set. Signed-off-by: Tatsuya S --- Changes in v2: - Fix build warnings reported by kernel test robot - Link to v1: https://lore.kernel.org/linux-trace-kernel/20240526112658.46740-1-tatsuya.s2...@gmail.com/ include/linux/ftrace.h | 2 ++ kernel/trace/ftrace.c | 2 +- kernel/trace/trace_functions.c | 7 ++- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h index 800995c425e0..0855dfe768eb 100644 --- a/include/linux/ftrace.h +++ b/include/linux/ftrace.h @@ -913,6 +913,8 @@ static inline bool is_ftrace_trampoline(unsigned long addr) /* totally disable ftrace - can not re-enable after this */ void ftrace_kill(void); +bool ftrace_pids_enabled(struct ftrace_ops *ops); + static inline void tracer_disable(void) { #ifdef CONFIG_FUNCTION_TRACER diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c index 65208d3b5ed9..e8ddd56d1e55 100644 --- a/kernel/trace/ftrace.c +++ b/kernel/trace/ftrace.c @@ -99,7 +99,7 @@ struct ftrace_ops *function_trace_op __read_mostly = &ftrace_list_end; /* What to set function_trace_op to */ static struct ftrace_ops *set_function_trace_op; -static bool ftrace_pids_enabled(struct ftrace_ops *ops) +bool ftrace_pids_enabled(struct ftrace_ops *ops) { struct trace_array *tr; diff --git a/kernel/trace/trace_functions.c b/kernel/trace/trace_functions.c index 9f1bfbe105e8..455c9a880199 100644 --- a/kernel/trace/trace_functions.c +++ b/kernel/trace/trace_functions.c @@ -223,6 +223,7 @@ function_stack_trace_call(unsigned long ip, unsigned long parent_ip, long disabled; int cpu; unsigned int trace_ctx; + int skip = STACK_SKIP; if (unlikely(!tr->function_enabled)) return; @@ -239,7 +240,11 @@ function_stack_trace_call(unsigned long ip, unsigned long parent_ip, if (likely(disabled == 1)) { trace_ctx = tracing_gen_ctx_flags(flags); trace_function(tr, ip, parent_ip, trace_ctx); - __trace_stack(tr, trace_ctx, STACK_SKIP); +#ifdef CONFIG_UNWINDER_FRAME_POINTER + if (ftrace_pids_enabled(op)) + skip++; +#endif + __trace_stack(tr, trace_ctx, skip); } atomic_dec(&data->disabled); -- 2.45.1
Re: [PATCH 01/12] soc: qcom: add firmware name helper
On Thu, 23 May 2024 at 01:48, Bjorn Andersson wrote: > > On Tue, May 21, 2024 at 03:08:31PM +0200, Dmitry Baryshkov wrote: > > On Tue, 21 May 2024 at 13:20, Kalle Valo wrote: > > > > > > Dmitry Baryshkov writes: > > > > > > > On Tue, 21 May 2024 at 12:52, wrote: > > > >> > > > >> On 21/05/2024 11:45, Dmitry Baryshkov wrote: > > > >> > Qualcomm platforms have different sets of the firmware files, which > > > >> > differ from platform to platform (and from board to board, due to the > > > >> > embedded signatures). Rather than listing all the firmware files, > > > >> > including full paths, in the DT, provide a way to determine firmware > > > >> > path based on the root DT node compatible. > > > >> > > > >> Ok this looks quite over-engineered but necessary to handle the legacy, > > > >> but I really think we should add a way to look for a board-specific > > > >> path > > > >> first and fallback to those SoC specific paths. > > > > > > > > Again, CONFIG_FW_LOADER_USER_HELPER => delays. > > > > > > To me this also looks like very over-engineered, can you elaborate more > > > why this is needed? Concrete examples would help to understand better. > > > > Sure. During the meeting last week Arnd suggested evaluating if we can > > drop firmware-name from the board DT files. Several reasons for that: > > - DT should describe the hardware, not the Linux-firmware locations > > - having firmware name in DT complicates updating the tree to use > > different firmware API (think of mbn vs mdt vs any other format) > > - If the DT gets supplied by the vendor (e.g. for > > SystemReady-certified devices), there should be a sync between the > > vendor's DT, linux kernel and the rootfs. Dropping firmware names from > > DT solves that by removing one piece of the equation > > > > Now for the complexity of the solution. Each SoC family has their own > > firmware set. This includes firmware for the DSPs, for modem, WiFi > > bits, GPU shader, etc. > > For the development boards these devices are signed by the testing key > > and the actual signature is not validated against the root of trust > > certificate. > > For the end-user devices the signature is actually validated against > > the bits fused to the SoC during manufacturing process. CA certificate > > (and thus the fuses) differ from vendor to vendor (and from the device > > to device) > > > > Not all of the firmware files are a part of the public linux-firmware > > tree. However we need to support the rootfs bundled with the firmware > > for different platforms (both public and vendor). The non-signed files > > come from the Adreno GPU and can be shared between platforms. All > > other files are SoC-specific and in some cases device-specific. > > > > So for example the SDM845 db845c (open device) loads following firmware > > files: > > Not signed: > > - qcom/a630_sqe.fw > > - qcom/a630_gmu.bin > > > > Signed, will work for any non-secured sdm845 device: > > - qcom/sdm845/a630_zap.mbn > > - qcom/sdm845/adsp.mbn > > - qcom/sdm845/cdsp.mbn > > - qcom/sdm485/mba.mbn > > - qcom/sdm845/modem.mbn > > - qcom/sdm845/wlanmdsp.mbn (loaded via TQFTP) > > - qcom/venus-5.2/venus.mbn > > > > Signed, works only for DB845c. > > - qcom/sdm845/Thundercomm/db845c/slpi.mbn > > > > In comparison, the SDM845 Pixel-3 phone (aka blueline) should load the > > following firmware files: > > - qcom/a630_sqe.fw (the same, non-signed file) > > - qcom/a630_gmu.bin (the same, non-signed file) > > - qcom/sdm845/Google/blueline/a630_zap.mbn > > How do you get from "a630_zap.mbn" to this? By extending the lookup > table for every target, or what am I missing? More or less so. Matching the root OF node gives us the firmware location, then it gets prepended to all firmware targets. Not an ideal solution, as there is no fallback support, but at least it gives us some points to discuss (and to decide whether to move to some particular direction or to abandon the idea completely, making Arnd unhappy again). > > Regards, > Bjorn > > > - qcom/sdm845/Google/blueline/adsp.mbn > > - qcom/sdm845/Google/blueline/cdsp.mbn > > - qcom/sdm845/Google/blueline/ipa_fws.mbn > > - qcom/sdm845/Google/blueline/mba.mbn > > - qcom/sdm845/Google/blueline/modem.mbn > > - qcom/sdm845/Google/blueline/venus.mbn > > - qcom/sdm845/Google/blueline/wlanmdsp.mbn > > - qcom/sdm845/Google/blueline/slpi.mbn > > > > The Lenovo Yoga C630 WoS laptop (SDM850 is a variant of SDM845) uses > > another set of files: > > - qcom/a630_sqe.fw (the same, non-signed file) > > - qcom/a630_gmu.bin (the same, non-signed file) > > - qcom/sdm850/LENOVO/81JL/qcdxkmsuc850.mbn > > - qcom/sdm850/LENOVO/81JL/qcadsp850.mbn > > - qcom/sdm850/LENOVO/81JL/qccdsp850.mbn > > - qcom/sdm850/LENOVO/81JL/ipa_fws.elf > > - qcom/sdm850/LENOVO/81JL/qcdsp1v2850.mbn > > - qcom/sdm850/LENOVO/81JL/qcdsp2850.mbn > > - qcom/sdm850/LENOVO/81JL/qcvss850.mbn > > - qcom/sdm850/LENOVO/81JL/wlanmdsp.mbn > > - qcom/sdm850/LENOVO/81JL/qcslpi850.mbn > > > > If we look at one of the recent p
Re: [PATCH v2] tracing/probes: fix error check in parse_btf_field()
On Mon, 27 May 2024 11:43:52 +0200 Carlos López wrote: > btf_find_struct_member() might return NULL or an error via the > ERR_PTR() macro. However, its caller in parse_btf_field() only checks > for the NULL condition. Fix this by using IS_ERR() and returning the > error up the stack. > Thanks for updating! This version looks good to me. Let me pick this to probes/fixes. Thank you, > Fixes: c440adfbe3025 ("tracing/probes: Support BTF based data structure field > access") > Signed-off-by: Carlos López > --- > v2: added call to trace_probe_log_err() > > kernel/trace/trace_probe.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c > index 5e263c141574..39877c80d6cb 100644 > --- a/kernel/trace/trace_probe.c > +++ b/kernel/trace/trace_probe.c > @@ -554,6 +554,10 @@ static int parse_btf_field(char *fieldname, const struct > btf_type *type, > anon_offs = 0; > field = btf_find_struct_member(ctx->btf, type, > fieldname, > &anon_offs); > + if (IS_ERR(field)) { > + trace_probe_log_err(ctx->offset, BAD_BTF_TID); > + return PTR_ERR(field); > + } > if (!field) { > trace_probe_log_err(ctx->offset, NO_BTF_FIELD); > return -ENOENT; > -- > 2.35.3 > -- Masami Hiramatsu (Google)
Re: Bug in Kernel 6.8.x, 6.9.x Causing Trace/Panic During Shutdown/Reboot
Hi Steven, I took some time and bisected the 6.8.9 - 6.8.10 and git gave the panic inducing commit: 414fb08628143 (tracefs: Reset permissions on remount if permissions are options) I reverted that commit to 6.9.2 and now it only serves the trace but the panic is gone. But I can live with it. --Ilkka On Sun, May 26, 2024 at 8:42 PM Ilkka Naulapää wrote: > > hi, > > I took 6.9.2 and applied that 0bcfd9aa4dafa to it. Now the kernel is > serving me both problems; the trace and the panic as the pic shows. > > > To understand this, did you do anything with tracing? Before shutting down, > > is there anything in /sys/kernel/tracing/instances directory? > > Were any of the files/directories permissions in /sys/kernel/tracing > > changed? > > And to answer your question, I did not do any tracing or so and the > /sys/kernel/tracing is empty. > Just plain boot-up, no matter if in full desktop or in bare rescue > mode, ends up the same way. > > --Ilkka > > On Fri, May 24, 2024 at 8:19 PM Steven Rostedt wrote: > > > > On Fri, 24 May 2024 12:50:08 +0200 > > "Linux regression tracking (Thorsten Leemhuis)" > > wrote: > > > > > > - Affected Versions: Before kernel version 6.8.10, the bug caused a > > > > quick display of a kernel trace dump before the shutdown/reboot > > > > completed. Starting from version 6.8.10 and continuing into version > > > > 6.9.0 and 6.9.1, this issue has escalated to a kernel panic, > > > > preventing the shutdown or reboot from completing and leaving the > > > > machine stuck. > > > > Ah, I bet it was this commit: baa23a8d4360d ("tracefs: Reset permissions on > > remount if permissions are options"), which added a "iput" callback to the > > dentry without calling iput, leaving stale inodes around. > > > > This is fixed with: > > > > 0bcfd9aa4dafa ("tracefs: Clear EVENT_INODE flag in tracefs_drop_inode()") > > > > Try adding just that patch. It will at least make it go back to what was > > happening before 6.8.10 (I hope!). > > > > -- Steve
[ANNOUNCE] 5.10.217-rt109
Hello RT-list! I'm pleased to announce the 5.10.217-rt109 stable release. This release is just an update to the new stable 5.10.217 version and no other changes have been performed. You can get this release via the git tree at: git://git.kernel.org/pub/scm/linux/kernel/git/rt/linux-stable-rt.git branch: v5.10-rt Head SHA1: a4119312fd0c5672efda3c8487c92e7962c8c212 Or to build 5.10.217-rt109 directly, the following patches should be applied: https://www.kernel.org/pub/linux/kernel/v5.x/linux-5.10.tar.xz https://www.kernel.org/pub/linux/kernel/v5.x/patch-5.10.217.xz https://www.kernel.org/pub/linux/kernel/projects/rt/5.10/older/patch-5.10.217-rt109.patch.xz Signing key fingerprint: 9354 0649 9972 8D31 D464 D140 F394 A423 F8E6 7C26 All keys used for the above files and repositories can be found on the following git repository: git://git.kernel.org/pub/scm/docs/kernel/pgpkeys.git Enjoy! Luis
[PATCH net-next 2/5] net/ipv4/sysctl: constify ctl_table arguments of utility functions
The sysctl core is preparing to only expose instances of struct ctl_table as "const". This will also affect the ctl_table argument of sysctl handlers. As the function prototype of all sysctl handlers throughout the tree needs to stay consistent that change will be done in one commit. To reduce the size of that final commit, switch utility functions which are not bound by "typedef proc_handler" to "const struct ctl_table". No functional change. Signed-off-by: Thomas Weißschuh --- net/ipv4/sysctl_net_ipv4.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c index 162a0a3b6ba5..d7892f34a15b 100644 --- a/net/ipv4/sysctl_net_ipv4.c +++ b/net/ipv4/sysctl_net_ipv4.c @@ -130,7 +130,8 @@ static int ipv4_privileged_ports(struct ctl_table *table, int write, return ret; } -static void inet_get_ping_group_range_table(struct ctl_table *table, kgid_t *low, kgid_t *high) +static void inet_get_ping_group_range_table(const struct ctl_table *table, + kgid_t *low, kgid_t *high) { kgid_t *data = table->data; struct net *net = @@ -145,7 +146,8 @@ static void inet_get_ping_group_range_table(struct ctl_table *table, kgid_t *low } /* Update system visible IP port range */ -static void set_ping_group_range(struct ctl_table *table, kgid_t low, kgid_t high) +static void set_ping_group_range(const struct ctl_table *table, +kgid_t low, kgid_t high) { kgid_t *data = table->data; struct net *net = -- 2.45.1
[PATCH net-next 3/5] net/ipv6/addrconf: constify ctl_table arguments of utility functions
The sysctl core is preparing to only expose instances of struct ctl_table as "const". This will also affect the ctl_table argument of sysctl handlers. As the function prototype of all sysctl handlers throughout the tree needs to stay consistent that change will be done in one commit. To reduce the size of that final commit, switch utility functions which are not bound by "typedef proc_handler" to "const struct ctl_table". No functional change. Signed-off-by: Thomas Weißschuh --- net/ipv6/addrconf.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c index 5c424a0e7232..1e69756d53d9 100644 --- a/net/ipv6/addrconf.c +++ b/net/ipv6/addrconf.c @@ -863,7 +863,7 @@ static void addrconf_forward_change(struct net *net, __s32 newf) } } -static int addrconf_fixup_forwarding(struct ctl_table *table, int *p, int newf) +static int addrconf_fixup_forwarding(const struct ctl_table *table, int *p, int newf) { struct net *net; int old; @@ -931,7 +931,7 @@ static void addrconf_linkdown_change(struct net *net, __s32 newf) } } -static int addrconf_fixup_linkdown(struct ctl_table *table, int *p, int newf) +static int addrconf_fixup_linkdown(const struct ctl_table *table, int *p, int newf) { struct net *net; int old; @@ -6378,7 +6378,7 @@ static void addrconf_disable_change(struct net *net, __s32 newf) } } -static int addrconf_disable_ipv6(struct ctl_table *table, int *p, int newf) +static int addrconf_disable_ipv6(const struct ctl_table *table, int *p, int newf) { struct net *net = (struct net *)table->extra2; int old; @@ -6669,7 +6669,7 @@ void addrconf_disable_policy_idev(struct inet6_dev *idev, int val) } static -int addrconf_disable_policy(struct ctl_table *ctl, int *valp, int val) +int addrconf_disable_policy(const struct ctl_table *ctl, int *valp, int val) { struct net *net = (struct net *)ctl->extra2; struct inet6_dev *idev; -- 2.45.1
[PATCH net-next 4/5] net/ipv6/ndisc: constify ctl_table arguments of utility function
The sysctl core is preparing to only expose instances of struct ctl_table as "const". This will also affect the ctl_table argument of sysctl handlers. As the function prototype of all sysctl handlers throughout the tree needs to stay consistent that change will be done in one commit. To reduce the size of that final commit, switch utility functions which are not bound by "typedef proc_handler" to "const struct ctl_table". No functional change. Signed-off-by: Thomas Weißschuh --- net/ipv6/ndisc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c index d914b23256ce..254b192c5705 100644 --- a/net/ipv6/ndisc.c +++ b/net/ipv6/ndisc.c @@ -1936,7 +1936,7 @@ static struct notifier_block ndisc_netdev_notifier = { }; #ifdef CONFIG_SYSCTL -static void ndisc_warn_deprecated_sysctl(struct ctl_table *ctl, +static void ndisc_warn_deprecated_sysctl(const struct ctl_table *ctl, const char *func, const char *dev_name) { static char warncomm[TASK_COMM_LEN]; -- 2.45.1
[PATCH net-next 0/5] net: constify ctl_table arguments of utility functions
The sysctl core is preparing to only expose instances of struct ctl_table as "const". This will also affect the ctl_table argument of sysctl handlers. As the function prototype of all sysctl handlers throughout the tree needs to stay consistent that change will be done in one commit. To reduce the size of that final commit, switch utility functions which are not bound by "typedef proc_handler" to "const struct ctl_table". No functional change. This patch(set) is meant to be applied through your subsystem tree. Or at your preference through the sysctl tree. Motivation == Moving structures containing function pointers into unmodifiable .rodata prevents attackers or bugs from corrupting and diverting those pointers. Also the "struct ctl_table" exposed by the sysctl core were never meant to be mutated by users. For this goal changes to both the sysctl core and "const" qualifiers for various sysctl APIs are necessary. Full Process * Drop ctl_table modifications from the sysctl core ([0], in mainline) * Constify arguments to ctl_table_root::{set_ownership,permissions} ([1], in mainline) * Migrate users of "ctl_table_header::ctl_table_arg" to "const". (in mainline) * Afterwards convert "ctl_table_header::ctl_table_arg" itself to const. (in mainline) * Prepare helpers used to implement proc_handlers throughout the tree to use "const struct ctl_table *". ([2], in progress, this patch) * Afterwards switch over all proc_handlers callbacks to use "const struct ctl_table *" in one commit. ([2], in progress) Only custom handlers will be affected, the big commit avoids a disruptive and messy transition phase. * Switch over the internals of the sysctl core to "const struct ctl_table *" (to be done) * Switch include/linux/sysctl.h to "const struct ctl_table *" (to be done) * Transition instances of "struct ctl_table" through the tree to const (to be done) A work-in-progress view containing all the outlined changes can be found at https://git.sr.ht/~t-8ch/linux sysctl-constfy [0] https://lore.kernel.org/lkml/20240322-sysctl-empty-dir-v2-0-e559cf8ec...@weissschuh.net/ [1] https://lore.kernel.org/lkml/20240315-sysctl-const-ownership-v3-0-b86680eae...@weissschuh.net/ [2] https://lore.kernel.org/lkml/20240423-sysctl-const-handler-v3-0-e0beccb83...@weissschuh.net/ --- Thomas Weißschuh (5): net/neighbour: constify ctl_table arguments of utility function net/ipv4/sysctl: constify ctl_table arguments of utility functions net/ipv6/addrconf: constify ctl_table arguments of utility functions net/ipv6/ndisc: constify ctl_table arguments of utility function ipvs: constify ctl_table arguments of utility functions net/core/neighbour.c | 2 +- net/ipv4/sysctl_net_ipv4.c | 6 -- net/ipv6/addrconf.c| 8 net/ipv6/ndisc.c | 2 +- net/netfilter/ipvs/ip_vs_ctl.c | 7 --- 5 files changed, 14 insertions(+), 11 deletions(-) --- base-commit: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0 change-id: 20240523-sysctl-const-handler-net-824d4ad5a15a Best regards, -- Thomas Weißschuh
[PATCH net-next 5/5] ipvs: constify ctl_table arguments of utility functions
The sysctl core is preparing to only expose instances of struct ctl_table as "const". This will also affect the ctl_table argument of sysctl handlers. As the function prototype of all sysctl handlers throughout the tree needs to stay consistent that change will be done in one commit. To reduce the size of that final commit, switch utility functions which are not bound by "typedef proc_handler" to "const struct ctl_table". No functional change. Signed-off-by: Thomas Weißschuh --- net/netfilter/ipvs/ip_vs_ctl.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c index b6d0dcf3a5c3..78a1cc72dc38 100644 --- a/net/netfilter/ipvs/ip_vs_ctl.c +++ b/net/netfilter/ipvs/ip_vs_ctl.c @@ -1924,7 +1924,8 @@ proc_do_sync_ports(struct ctl_table *table, int write, return rc; } -static int ipvs_proc_est_cpumask_set(struct ctl_table *table, void *buffer) +static int ipvs_proc_est_cpumask_set(const struct ctl_table *table, +void *buffer) { struct netns_ipvs *ipvs = table->extra2; cpumask_var_t *valp = table->data; @@ -1962,8 +1963,8 @@ static int ipvs_proc_est_cpumask_set(struct ctl_table *table, void *buffer) return ret; } -static int ipvs_proc_est_cpumask_get(struct ctl_table *table, void *buffer, -size_t size) +static int ipvs_proc_est_cpumask_get(const struct ctl_table *table, +void *buffer, size_t size) { struct netns_ipvs *ipvs = table->extra2; cpumask_var_t *valp = table->data; -- 2.45.1
[PATCH net-next 1/5] net/neighbour: constify ctl_table arguments of utility function
The sysctl core is preparing to only expose instances of struct ctl_table as "const". This will also affect the ctl_table argument of sysctl handlers. As the function prototype of all sysctl handlers throughout the tree needs to stay consistent that change will be done in one commit. To reduce the size of that final commit, switch utility functions which are not bound by "typedef proc_handler" to "const struct ctl_table". No functional change. Signed-off-by: Thomas Weißschuh --- net/core/neighbour.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/core/neighbour.c b/net/core/neighbour.c index 45fd88405b6b..277751375b0a 100644 --- a/net/core/neighbour.c +++ b/net/core/neighbour.c @@ -3578,7 +3578,7 @@ static void neigh_copy_dflt_parms(struct net *net, struct neigh_parms *p, rcu_read_unlock(); } -static void neigh_proc_update(struct ctl_table *ctl, int write) +static void neigh_proc_update(const struct ctl_table *ctl, int write) { struct net_device *dev = ctl->extra1; struct neigh_parms *p = ctl->extra2; -- 2.45.1
Re: [PATCH v2 2/3] arm64: dts: qcom: pm7250b: Add a TCPM description
On Fri, Mar 29, 2024 at 01:26:20PM GMT, Luca Weiss wrote: > Type-C port management functionality lives inside of the PMIC block on > pm7250b. > > The Type-C port management logic controls orientation detection, > vbus/vconn sense and to send/receive Type-C Power Domain messages. > pm7250b is found in devices where USB Type-C port management is performed in firmware, presumably using this hardware block. As such, it seems reasonable to leave this node disabled and only enable it on the targets that doesn't do this in firmware. Regards, Bjorn > Reviewed-by: Bryan O'Donoghue > Reviewed-by: Konrad Dybcio > Signed-off-by: Luca Weiss > --- > arch/arm64/boot/dts/qcom/pm7250b.dtsi | 39 > +++ > 1 file changed, 39 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/pm7250b.dtsi > b/arch/arm64/boot/dts/qcom/pm7250b.dtsi > index 4faed25a787f..0205c2669093 100644 > --- a/arch/arm64/boot/dts/qcom/pm7250b.dtsi > +++ b/arch/arm64/boot/dts/qcom/pm7250b.dtsi > @@ -51,6 +51,45 @@ pm7250b_vbus: usb-vbus-regulator@1100 { > status = "disabled"; > }; > > + pm7250b_typec: typec@1500 { > + compatible = "qcom,pm7250b-typec", "qcom,pm8150b-typec"; > + reg = <0x1500>, > + <0x1700>; > + interrupts = IRQ_TYPE_EDGE_RISING>, > + , > + IRQ_TYPE_EDGE_RISING>, > + , > + IRQ_TYPE_EDGE_RISING>, > + IRQ_TYPE_EDGE_RISING>, > + , > + IRQ_TYPE_EDGE_RISING>, > + IRQ_TYPE_EDGE_RISING>, > + IRQ_TYPE_EDGE_RISING>, > + IRQ_TYPE_EDGE_RISING>, > + IRQ_TYPE_EDGE_RISING>, > + IRQ_TYPE_EDGE_RISING>, > + IRQ_TYPE_EDGE_RISING>, > + IRQ_TYPE_EDGE_RISING>, > + IRQ_TYPE_EDGE_RISING>; > + interrupt-names = "or-rid-detect-change", > + "vpd-detect", > + "cc-state-change", > + "vconn-oc", > + "vbus-change", > + "attach-detach", > + "legacy-cable-detect", > + "try-snk-src-detect", > + "sig-tx", > + "sig-rx", > + "msg-tx", > + "msg-rx", > + "msg-tx-failed", > + "msg-tx-discarded", > + "msg-rx-discarded", > + "fr-swap"; > + vdd-vbus-supply = <&pm7250b_vbus>; > + }; > + > pm7250b_temp: temp-alarm@2400 { > compatible = "qcom,spmi-temp-alarm"; > reg = <0x2400>; > > -- > 2.44.0 >
Re: [PATCH v2] sched/rt: Clean up usage of rt_task()
On 05/21/24 13:00, Sebastian Andrzej Siewior wrote: > On 2024-05-15 23:05:36 [+0100], Qais Yousef wrote: > > rt_task() checks if a task has RT priority. But depends on your > > dictionary, this could mean it belongs to RT class, or is a 'realtime' > > task, which includes RT and DL classes. > > > > Since this has caused some confusion already on discussion [1], it > > seemed a clean up is due. > > > > I define the usage of rt_task() to be tasks that belong to RT class. > > Make sure that it returns true only for RT class and audit the users and > > replace the ones required the old behavior with the new realtime_task() > > which returns true for RT and DL classes. Introduce similar > > realtime_prio() to create similar distinction to rt_prio() and update > > the users that required the old behavior to use the new function. > > > > Move MAX_DL_PRIO to prio.h so it can be used in the new definitions. > > > > Document the functions to make it more obvious what is the difference > > between them. PI-boosted tasks is a factor that must be taken into > > account when choosing which function to use. > > > > Rename task_is_realtime() to realtime_task_policy() as the old name is > > confusing against the new realtime_task(). > > I *think* everyone using rt_task() means to include DL tasks. And > everyone means !SCHED-people since they know when the difference matters. yes, this makes sense > > > No functional changes were intended. > > > > [1] > > https://lore.kernel.org/lkml/20240506100509.gl40...@noisy.programming.kicks-ass.net/ > > > > Reviewed-by: Phil Auld > > Signed-off-by: Qais Yousef > > --- > > > > Changes since v1: > > > > * Use realtime_task_policy() instead task_has_realtime_policy() (Peter) > > * Improve commit message readability about replace some rt_task() > > users. > > > > v1 discussion: > > https://lore.kernel.org/lkml/20240514234112.792989-1-qyou...@layalina.io/ > > > > fs/select.c | 2 +- > > fs/bcachefs/six.c > six_owner_running() has rt_task(). But imho should have realtime_task() > to consider DL. But I think it is way worse that it has its own locking > rather than using what everyone else but then again it wouldn't be the > new hot thing… I think I missed this one. Converted now. Thanks! > > > include/linux/ioprio.h| 2 +- > > include/linux/sched/deadline.h| 6 -- > > include/linux/sched/prio.h| 1 + > > include/linux/sched/rt.h | 27 ++- > > kernel/locking/rtmutex.c | 4 ++-- > > kernel/locking/rwsem.c| 4 ++-- > > kernel/locking/ww_mutex.h | 2 +- > > kernel/sched/core.c | 6 +++--- > > kernel/time/hrtimer.c | 6 +++--- > > kernel/trace/trace_sched_wakeup.c | 2 +- > > mm/page-writeback.c | 4 ++-- > > mm/page_alloc.c | 2 +- > > 13 files changed, 48 insertions(+), 20 deletions(-) > … > > diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c > > index 70625dff62ce..08b95e0a41ab 100644 > > --- a/kernel/time/hrtimer.c > > +++ b/kernel/time/hrtimer.c > > @@ -1996,7 +1996,7 @@ static void __hrtimer_init_sleeper(struct > > hrtimer_sleeper *sl, > > * expiry. > > */ > > if (IS_ENABLED(CONFIG_PREEMPT_RT)) { > > - if (task_is_realtime(current) && !(mode & HRTIMER_MODE_SOFT)) > > + if (realtime_task_policy(current) && !(mode & > > HRTIMER_MODE_SOFT)) > > mode |= HRTIMER_MODE_HARD; > > } > > > > @@ -2096,7 +2096,7 @@ long hrtimer_nanosleep(ktime_t rqtp, const enum > > hrtimer_mode mode, > > u64 slack; > > > > slack = current->timer_slack_ns; > > - if (rt_task(current)) > > + if (realtime_task(current)) > > slack = 0; > > > > hrtimer_init_sleeper_on_stack(&t, clockid, mode); > > @@ -2301,7 +2301,7 @@ schedule_hrtimeout_range_clock(ktime_t *expires, u64 > > delta, > > * Override any slack passed by the user if under > > * rt contraints. > > */ > > - if (rt_task(current)) > > + if (realtime_task(current)) > > delta = 0; > > I know this is just converting what is already here but… > __hrtimer_init_sleeper() looks at the policy to figure out if the task > is realtime do decide if should expire in HARD-IRQ context. This is > correct, a boosted task should not sleep. > > hrtimer_nanosleep() + schedule_hrtimeout_range_clock() is looking at > priority to decide if slack should be removed. This should also look at > policy since a boosted task shouldn't sleep. I have to admit I never dug deep enough into this code. Happy to convert these users. I'll add that as a separate patch as this is somewhat changing behavior which this patch intends to do a clean up only. > > In order to be PI-boosted you need to acquire a lock and the only lock > you can sleep while acquired without generating a warning is a mutex_t > (or equivalent sleeping lock) on PREEMPT_RT. No
Re: [PATCH v2] sched/rt: Clean up usage of rt_task()
On 05/23/24 11:45, Steven Rostedt wrote: > On Wed, 15 May 2024 23:05:36 +0100 > Qais Yousef wrote: > > diff --git a/include/linux/sched/deadline.h b/include/linux/sched/deadline.h > > index df3aca89d4f5..5cb88b748ad6 100644 > > --- a/include/linux/sched/deadline.h > > +++ b/include/linux/sched/deadline.h > > @@ -10,8 +10,6 @@ > > > > #include > > > > -#define MAX_DL_PRIO0 > > - > > static inline int dl_prio(int prio) > > { > > if (unlikely(prio < MAX_DL_PRIO)) > > @@ -19,6 +17,10 @@ static inline int dl_prio(int prio) > > return 0; > > } > > > > +/* > > + * Returns true if a task has a priority that belongs to DL class. > > PI-boosted > > + * tasks will return true. Use dl_policy() to ignore PI-boosted tasks. > > + */ > > static inline int dl_task(struct task_struct *p) > > { > > return dl_prio(p->prio); > > diff --git a/include/linux/sched/prio.h b/include/linux/sched/prio.h > > index ab83d85e1183..6ab43b4f72f9 100644 > > --- a/include/linux/sched/prio.h > > +++ b/include/linux/sched/prio.h > > @@ -14,6 +14,7 @@ > > */ > > > > #define MAX_RT_PRIO100 > > +#define MAX_DL_PRIO0 > > > > #define MAX_PRIO (MAX_RT_PRIO + NICE_WIDTH) > > #define DEFAULT_PRIO (MAX_RT_PRIO + NICE_WIDTH / 2) > > diff --git a/include/linux/sched/rt.h b/include/linux/sched/rt.h > > index b2b9e6eb9683..a055dd68a77c 100644 > > --- a/include/linux/sched/rt.h > > +++ b/include/linux/sched/rt.h > > @@ -7,18 +7,43 @@ > > struct task_struct; > > > > static inline int rt_prio(int prio) > > +{ > > + if (unlikely(prio < MAX_RT_PRIO && prio >= MAX_DL_PRIO)) > > + return 1; > > + return 0; > > +} > > + > > +static inline int realtime_prio(int prio) > > { > > if (unlikely(prio < MAX_RT_PRIO)) > > return 1; > > return 0; > > } > > I'm thinking we should change the above to bool (separate patch), as > returning an int may give one the impression that it returns the actual > priority number. Having it return bool will clear that up. > > In fact, if we are touching theses functions, might as well change all of > them to bool when returning true/false. Just to make it easier to > understand what they are doing. I can add a patch on top, sure. > > > > > +/* > > + * Returns true if a task has a priority that belongs to RT class. > > PI-boosted > > + * tasks will return true. Use rt_policy() to ignore PI-boosted tasks. > > + */ > > static inline int rt_task(struct task_struct *p) > > { > > return rt_prio(p->prio); > > } > > > > -static inline bool task_is_realtime(struct task_struct *tsk) > > +/* > > + * Returns true if a task has a priority that belongs to RT or DL classes. > > + * PI-boosted tasks will return true. Use realtime_task_policy() to ignore > > + * PI-boosted tasks. > > + */ > > +static inline int realtime_task(struct task_struct *p) > > +{ > > + return realtime_prio(p->prio); > > +} > > + > > +/* > > + * Returns true if a task has a policy that belongs to RT or DL classes. > > + * PI-boosted tasks will return false. > > + */ > > +static inline bool realtime_task_policy(struct task_struct *tsk) > > { > > int policy = tsk->policy; > > > > > > > diff --git a/kernel/trace/trace_sched_wakeup.c > > b/kernel/trace/trace_sched_wakeup.c > > index 0469a04a355f..19d737742e29 100644 > > --- a/kernel/trace/trace_sched_wakeup.c > > +++ b/kernel/trace/trace_sched_wakeup.c > > @@ -545,7 +545,7 @@ probe_wakeup(void *ignore, struct task_struct *p) > > * - wakeup_dl handles tasks belonging to sched_dl class only. > > */ > > if (tracing_dl || (wakeup_dl && !dl_task(p)) || > > - (wakeup_rt && !dl_task(p) && !rt_task(p)) || > > + (wakeup_rt && !realtime_task(p)) || > > (!dl_task(p) && (p->prio >= wakeup_prio || p->prio >= > > current->prio))) > > return; > > > > Reviewed-by: Steven Rostedt (Google) Thanks! -- Qais Yousef > >
Re: [PATCH] ipvs: Avoid unnecessary calls to skb_is_gso_sctp
Hello, On Thu, 23 May 2024, Ismael Luceno wrote: > In the context of the SCTP SNAT/DNAT handler, these calls can only > return true. > > Ref: e10d3ba4d434 ("ipvs: Fix checksumming on GSO of SCTP packets") checkpatch.pl prefers to see the "commit" word: Ref: commit e10d3ba4d434 ("ipvs: Fix checksumming on GSO of SCTP packets") > Signed-off-by: Ismael Luceno Looks good to me for nf-next, thanks! Acked-by: Julian Anastasov > CC: Pablo Neira Ayuso > CC: Michal Kubeček > CC: Simon Horman > CC: Julian Anastasov > CC: lvs-de...@vger.kernel.org > CC: netfilter-de...@vger.kernel.org > CC: net...@vger.kernel.org > CC: coret...@netfilter.org > --- > net/netfilter/ipvs/ip_vs_proto_sctp.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/net/netfilter/ipvs/ip_vs_proto_sctp.c > b/net/netfilter/ipvs/ip_vs_proto_sctp.c > index 1e689c714127..83e452916403 100644 > --- a/net/netfilter/ipvs/ip_vs_proto_sctp.c > +++ b/net/netfilter/ipvs/ip_vs_proto_sctp.c > @@ -126,7 +126,7 @@ sctp_snat_handler(struct sk_buff *skb, struct > ip_vs_protocol *pp, > if (sctph->source != cp->vport || payload_csum || > skb->ip_summed == CHECKSUM_PARTIAL) { > sctph->source = cp->vport; > - if (!skb_is_gso(skb) || !skb_is_gso_sctp(skb)) > + if (!skb_is_gso(skb)) > sctp_nat_csum(skb, sctph, sctphoff); > } else { > skb->ip_summed = CHECKSUM_UNNECESSARY; > @@ -175,7 +175,7 @@ sctp_dnat_handler(struct sk_buff *skb, struct > ip_vs_protocol *pp, > (skb->ip_summed == CHECKSUM_PARTIAL && >!(skb_dst(skb)->dev->features & NETIF_F_SCTP_CRC))) { > sctph->dest = cp->dport; > - if (!skb_is_gso(skb) || !skb_is_gso_sctp(skb)) > + if (!skb_is_gso(skb)) > sctp_nat_csum(skb, sctph, sctphoff); > } else if (skb->ip_summed != CHECKSUM_PARTIAL) { > skb->ip_summed = CHECKSUM_UNNECESSARY; > -- > 2.44.0 Regards -- Julian Anastasov
Re: Bug in Kernel 6.8.x, 6.9.x Causing Trace/Panic During Shutdown/Reboot
On Mon, May 27, 2024 at 07:40:21PM +0300, Ilkka Naulapää wrote: > Hi Steven, > > I took some time and bisected the 6.8.9 - 6.8.10 and git gave the > panic inducing commit: > > 414fb08628143 (tracefs: Reset permissions on remount if permissions are > options) > > I reverted that commit to 6.9.2 and now it only serves the trace but > the panic is gone. But I can live with it. Steven, should we revert that? Or is there some other change that we should take to resolve this? thanks, greg k-h
Re: Bug in Kernel 6.8.x, 6.9.x Causing Trace/Panic During Shutdown/Reboot
On Mon, 27 May 2024 20:14:42 +0200 Greg KH wrote: > On Mon, May 27, 2024 at 07:40:21PM +0300, Ilkka Naulapää wrote: > > Hi Steven, > > > > I took some time and bisected the 6.8.9 - 6.8.10 and git gave the > > panic inducing commit: > > > > 414fb08628143 (tracefs: Reset permissions on remount if permissions are > > options) > > > > I reverted that commit to 6.9.2 and now it only serves the trace but > > the panic is gone. But I can live with it. > > Steven, should we revert that? > > Or is there some other change that we should take to resolve this? > Before we revert it (as it may be a bug in mainline), Ilkka, can you test v6.10-rc1? If it exists there, it will let me know whether or not I missed something. Thanks, -- Steve
Re: Bug in Kernel 6.8.x, 6.9.x Causing Trace/Panic During Shutdown/Reboot
On Fri, 24 May 2024 12:50:08 +0200 "Linux regression tracking (Thorsten Leemhuis)" wrote: > > - Affected Versions: Before kernel version 6.8.10, the bug caused a > > quick display of a kernel trace dump before the shutdown/reboot > > completed. Starting from version 6.8.10 and continuing into version > > 6.9.0 and 6.9.1, this issue has escalated to a kernel panic, > > preventing the shutdown or reboot from completing and leaving the > > machine stuck. You state "Before kernel version 6.8.10, the bug caused ...". Does that mean that a bug was happening before v6.8.10? But did not cause a panic? I just noticed your second screen shot from your report, and it has: "cache_from_obj: Wrong slab cache, tracefs_inode_cache but object is from inode_cache" So somehow an tracefs_inode was allocated from the inode_cache and is now being freed by the tracefs_inode logic? Did this happen before 6.8.10? If so, this code could just be triggering an issue from an unrelated bug. Thanks, -- Steve
Re: [PATCH 0/3] tracing: Fix some selftest issues
On Sun, 26 May 2024 19:10:57 +0900 "Masami Hiramatsu (Google)" wrote: > Hi, > > Here is a series of some fixes/improvements for the test modules and boot > time selftest of kprobe events. I found a WARNING message with some boot > time selftest configuration, which came from the combination of embedded > kprobe generate API tests module and ftrace boot-time selftest. So the main > problem is that the test module should not be built-in. But I also think > this WARNING message is useless (because there are warning messages already) > and the cleanup code is redundant. This series fixes those issues. Note, when I enable trace tests as builtin instead of modules, I just disable the bootup self tests when it detects this. This helps with doing tests via config options than having to add user space code that loads modules. Could you do something similar? -- Steve > > Thank you, > > --- > > Masami Hiramatsu (Google) (3): > tracing: Build event generation tests only as modules > tracing/kprobe: Remove unneeded WARN_ON_ONCE() in selftests > tracing/kprobe: Remove cleanup code unrelated to selftest >
Re: [PATCH 2/2] ring-buffer: Fix a race between readers and resize checks
On Mon, 27 May 2024 11:36:55 +0200 Petr Pavlu wrote: > >> static void rb_check_pages(struct ring_buffer_per_cpu *cpu_buffer) > >> { > >> @@ -2200,8 +2205,13 @@ int ring_buffer_resize(struct trace_buffer *buffer, > >> unsigned long size, > >> */ > >>synchronize_rcu(); > >>for_each_buffer_cpu(buffer, cpu) { > >> + unsigned long flags; > >> + > >>cpu_buffer = buffer->buffers[cpu]; > >> + raw_spin_lock_irqsave(&cpu_buffer->reader_lock, flags); > >>rb_check_pages(cpu_buffer); > >> + raw_spin_unlock_irqrestore(&cpu_buffer->reader_lock, > >> + flags); > > > > Putting my RT hat on, I really don't like the above fix. The > > rb_check_pages() iterates all subbuffers which makes the time interrupts > > are disabled non-deterministic. > > I see, this applies also to the same rb_check_pages() validation invoked > from ring_buffer_read_finish(). > > > > > Instead, I would rather have something where we disable readers while we do > > the check, and re-enable them. > > > > raw_spin_lock_irqsave(&cpu_buffer->reader_lock, flags); > > cpu_buffer->read_disabled++; > > raw_spin_unlock_irqrestore(&cpu_buffer->reader_lock, > > flags); > > > > // Also, don't put flags on a new line. We are allow to go 100 characters > > now. > > Noted. > > > > > > > rb_check_pages(cpu_buffer); > > raw_spin_lock_irqsave(&cpu_buffer->reader_lock, flags); > > cpu_buffer->read_disabled--; > > raw_spin_unlock_irqrestore(&cpu_buffer->reader_lock, > > flags); > > > > Or something like that. Yes, that also requires creating a new > > "read_disabled" field in the ring_buffer_per_cpu code. > > I think this would work but I'm personally not immediately sold on this > approach. If I understand the idea correctly, readers should then check > whether cpu_buffer->read_disabled is set and bail out with some error if > that is the case. The rb_check_pages() function is only a self-check > code and as such, I feel it doesn't justify disrupting readers with > a new error condition and adding more complex locking. Honestly, this code was never made for more than one reader per cpu_buffer. I'm perfectly fine if all check_pages() causes other readers to the same per_cpu buffer to get -EBUSY. Do you really see this being a problem? What use case is there for hitting the check_pages() and reading the same cpu buffer at the same time? -- Steve
[PATCH v3 0/3] Clean up usage of rt_task()
Make rt_task() return true only for RT class and add new realtime_task() to return true for RT and DL classes to avoid some confusion the old API can cause. No functional changes intended in patch 1. Patch 2 changes hrtimer users as suggested by Sebastian. Patch 3 cleans up the return type as suggested by Steve. Changes since v2: * Fix one user that should use realtime_task() but remained using rt_task() (Sebastian) * New patch to convert all hrtimer users to use realtime_task_policy() (Sebastian) * Add a new patch to convert return type to bool (Steve) * Rebase on tip/sched/core and handle a conflict with code shuffle to syscalls.c * Add Reviewed-by Steve Changes since v1: * Use realtime_task_policy() instead task_has_realtime_policy() (Peter) * Improve commit message readability about replace some rt_task() users. v1 discussion: https://lore.kernel.org/lkml/20240514234112.792989-1-qyou...@layalina.io/ v2 discussion: https://lore.kernel.org/lkml/20240515220536.823145-1-qyou...@layalina.io/ Qais Yousef (3): sched/rt: Clean up usage of rt_task() hrtimer: Convert realtime_task() to realtime_task_policy() sched/rt, dl: Convert functions to return bool fs/bcachefs/six.c | 2 +- fs/select.c | 2 +- include/linux/ioprio.h| 2 +- include/linux/sched/deadline.h| 10 ++ include/linux/sched/prio.h| 1 + include/linux/sched/rt.h | 31 --- kernel/locking/rtmutex.c | 4 ++-- kernel/locking/rwsem.c| 4 ++-- kernel/locking/ww_mutex.h | 2 +- kernel/sched/core.c | 4 ++-- kernel/sched/syscalls.c | 2 +- kernel/time/hrtimer.c | 6 +++--- kernel/trace/trace_sched_wakeup.c | 2 +- mm/page-writeback.c | 4 ++-- mm/page_alloc.c | 2 +- 15 files changed, 53 insertions(+), 25 deletions(-) -- 2.34.1
[PATCH v3 1/3] sched/rt: Clean up usage of rt_task()
rt_task() checks if a task has RT priority. But depends on your dictionary, this could mean it belongs to RT class, or is a 'realtime' task, which includes RT and DL classes. Since this has caused some confusion already on discussion [1], it seemed a clean up is due. I define the usage of rt_task() to be tasks that belong to RT class. Make sure that it returns true only for RT class and audit the users and replace the ones required the old behavior with the new realtime_task() which returns true for RT and DL classes. Introduce similar realtime_prio() to create similar distinction to rt_prio() and update the users that required the old behavior to use the new function. Move MAX_DL_PRIO to prio.h so it can be used in the new definitions. Document the functions to make it more obvious what is the difference between them. PI-boosted tasks is a factor that must be taken into account when choosing which function to use. Rename task_is_realtime() to realtime_task_policy() as the old name is confusing against the new realtime_task(). No functional changes were intended. [1] https://lore.kernel.org/lkml/20240506100509.gl40...@noisy.programming.kicks-ass.net/ Reviewed-by: Phil Auld Reviewed-by: Steven Rostedt (Google) Signed-off-by: Qais Yousef --- fs/bcachefs/six.c | 2 +- fs/select.c | 2 +- include/linux/ioprio.h| 2 +- include/linux/sched/deadline.h| 6 -- include/linux/sched/prio.h| 1 + include/linux/sched/rt.h | 27 ++- kernel/locking/rtmutex.c | 4 ++-- kernel/locking/rwsem.c| 4 ++-- kernel/locking/ww_mutex.h | 2 +- kernel/sched/core.c | 4 ++-- kernel/sched/syscalls.c | 2 +- kernel/time/hrtimer.c | 6 +++--- kernel/trace/trace_sched_wakeup.c | 2 +- mm/page-writeback.c | 4 ++-- mm/page_alloc.c | 2 +- 15 files changed, 49 insertions(+), 21 deletions(-) diff --git a/fs/bcachefs/six.c b/fs/bcachefs/six.c index 3a494c5d1247..b30870bf7e4a 100644 --- a/fs/bcachefs/six.c +++ b/fs/bcachefs/six.c @@ -335,7 +335,7 @@ static inline bool six_owner_running(struct six_lock *lock) */ rcu_read_lock(); struct task_struct *owner = READ_ONCE(lock->owner); - bool ret = owner ? owner_on_cpu(owner) : !rt_task(current); + bool ret = owner ? owner_on_cpu(owner) : !realtime_task(current); rcu_read_unlock(); return ret; diff --git a/fs/select.c b/fs/select.c index 9515c3fa1a03..8d5c1419416c 100644 --- a/fs/select.c +++ b/fs/select.c @@ -82,7 +82,7 @@ u64 select_estimate_accuracy(struct timespec64 *tv) * Realtime tasks get a slack of 0 for obvious reasons. */ - if (rt_task(current)) + if (realtime_task(current)) return 0; ktime_get_ts64(&now); diff --git a/include/linux/ioprio.h b/include/linux/ioprio.h index db1249cd9692..75859b78d540 100644 --- a/include/linux/ioprio.h +++ b/include/linux/ioprio.h @@ -40,7 +40,7 @@ static inline int task_nice_ioclass(struct task_struct *task) { if (task->policy == SCHED_IDLE) return IOPRIO_CLASS_IDLE; - else if (task_is_realtime(task)) + else if (realtime_task_policy(task)) return IOPRIO_CLASS_RT; else return IOPRIO_CLASS_BE; diff --git a/include/linux/sched/deadline.h b/include/linux/sched/deadline.h index df3aca89d4f5..5cb88b748ad6 100644 --- a/include/linux/sched/deadline.h +++ b/include/linux/sched/deadline.h @@ -10,8 +10,6 @@ #include -#define MAX_DL_PRIO0 - static inline int dl_prio(int prio) { if (unlikely(prio < MAX_DL_PRIO)) @@ -19,6 +17,10 @@ static inline int dl_prio(int prio) return 0; } +/* + * Returns true if a task has a priority that belongs to DL class. PI-boosted + * tasks will return true. Use dl_policy() to ignore PI-boosted tasks. + */ static inline int dl_task(struct task_struct *p) { return dl_prio(p->prio); diff --git a/include/linux/sched/prio.h b/include/linux/sched/prio.h index ab83d85e1183..6ab43b4f72f9 100644 --- a/include/linux/sched/prio.h +++ b/include/linux/sched/prio.h @@ -14,6 +14,7 @@ */ #define MAX_RT_PRIO100 +#define MAX_DL_PRIO0 #define MAX_PRIO (MAX_RT_PRIO + NICE_WIDTH) #define DEFAULT_PRIO (MAX_RT_PRIO + NICE_WIDTH / 2) diff --git a/include/linux/sched/rt.h b/include/linux/sched/rt.h index b2b9e6eb9683..a055dd68a77c 100644 --- a/include/linux/sched/rt.h +++ b/include/linux/sched/rt.h @@ -7,18 +7,43 @@ struct task_struct; static inline int rt_prio(int prio) +{ + if (unlikely(prio < MAX_RT_PRIO && prio >= MAX_DL_PRIO)) + return 1; + return 0; +} + +static inline int realtime_prio(int prio) { if (unlikely(prio < MAX_RT_PRIO)) return 1; return 0; } +/* + * Returns true if a task h
[PATCH v3 2/3] hrtimer: Convert realtime_task() to realtime_task_policy()
As Sebastian explained in [1], We need only look at the policy to decide if we need to remove the slack because PI-boosted tasks should not sleep. [1] https://lore.kernel.org/lkml/20240521110035.kriwl...@linutronix.de/ Suggested-by: Sebastian Andrzej Siewior Signed-off-by: Qais Yousef --- kernel/time/hrtimer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c index 89d4da59059d..36086ab46d08 100644 --- a/kernel/time/hrtimer.c +++ b/kernel/time/hrtimer.c @@ -2073,7 +2073,7 @@ long hrtimer_nanosleep(ktime_t rqtp, const enum hrtimer_mode mode, u64 slack; slack = current->timer_slack_ns; - if (realtime_task(current)) + if (realtime_task_policy(current)) slack = 0; hrtimer_init_sleeper_on_stack(&t, clockid, mode); @@ -2278,7 +2278,7 @@ schedule_hrtimeout_range_clock(ktime_t *expires, u64 delta, * Override any slack passed by the user if under * rt contraints. */ - if (realtime_task(current)) + if (realtime_task_policy(current)) delta = 0; hrtimer_init_sleeper_on_stack(&t, clock_id, mode); -- 2.34.1
[PATCH v3 3/3] sched/rt, dl: Convert functions to return bool
{rt, realtime, dl}_{task, prio}() functions return value is actually a bool. Convert their return type to reflect that. Suggested-by: Steven Rostedt (Google) Signed-off-by: Qais Yousef --- include/linux/sched/deadline.h | 4 ++-- include/linux/sched/rt.h | 8 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/include/linux/sched/deadline.h b/include/linux/sched/deadline.h index 5cb88b748ad6..87d2370dd3db 100644 --- a/include/linux/sched/deadline.h +++ b/include/linux/sched/deadline.h @@ -10,7 +10,7 @@ #include -static inline int dl_prio(int prio) +static inline bool dl_prio(int prio) { if (unlikely(prio < MAX_DL_PRIO)) return 1; @@ -21,7 +21,7 @@ static inline int dl_prio(int prio) * Returns true if a task has a priority that belongs to DL class. PI-boosted * tasks will return true. Use dl_policy() to ignore PI-boosted tasks. */ -static inline int dl_task(struct task_struct *p) +static inline bool dl_task(struct task_struct *p) { return dl_prio(p->prio); } diff --git a/include/linux/sched/rt.h b/include/linux/sched/rt.h index a055dd68a77c..78da97cdac48 100644 --- a/include/linux/sched/rt.h +++ b/include/linux/sched/rt.h @@ -6,14 +6,14 @@ struct task_struct; -static inline int rt_prio(int prio) +static inline bool rt_prio(int prio) { if (unlikely(prio < MAX_RT_PRIO && prio >= MAX_DL_PRIO)) return 1; return 0; } -static inline int realtime_prio(int prio) +static inline bool realtime_prio(int prio) { if (unlikely(prio < MAX_RT_PRIO)) return 1; @@ -24,7 +24,7 @@ static inline int realtime_prio(int prio) * Returns true if a task has a priority that belongs to RT class. PI-boosted * tasks will return true. Use rt_policy() to ignore PI-boosted tasks. */ -static inline int rt_task(struct task_struct *p) +static inline bool rt_task(struct task_struct *p) { return rt_prio(p->prio); } @@ -34,7 +34,7 @@ static inline int rt_task(struct task_struct *p) * PI-boosted tasks will return true. Use realtime_task_policy() to ignore * PI-boosted tasks. */ -static inline int realtime_task(struct task_struct *p) +static inline bool realtime_task(struct task_struct *p) { return realtime_prio(p->prio); } -- 2.34.1
Re: [PATCH v2] ftrace: Fix stack trace entry generated by ftrace_pid_func()
On Mon, 27 May 2024 18:44:56 +0900 Tatsuya S wrote: > On setting set_ftrace_pid, a extra entry generated by ftrace_pid_func() > is shown on stack trace(CONFIG_UNWINDER_FRAME_POINTER=y). > > [004] .68.459382: > => 0xa00090af > => ksys_read > => __x64_sys_read > => x64_sys_call > => do_syscall_64 > => entry_SYSCALL_64_after_hwframe > > To resolve this issue, increment skip count > in function_stack_trace_call() if pids are set. Just a note, this isn't a "fix" but simply an improvement in output. I'm happy to take this (after testing and more reviewing), but it will be for the next merge window, and with a different subject line. "ftrace: Hide one more entry in stack trace when ftrace_pid is enabled" Or something like that. Thanks, -- Steve
[PATCH] ftrace: adding the missing parameter descriptions of unregister_ftrace_direct
Adding the description of the parameters addr and free_filters of the function unregister_ftrace_direct. Signed-off-by: MarileneGarcia --- Hello, These changes fix the following compiler warnings of the function unregister_ftrace_direct. The warnings happen using GCC compiler, enabling the ftrace related configs and using the command 'make W=1'. kernel/trace/ftrace.c:5489: warning: Function parameter or struct member 'addr' not described in 'unregister_ftrace_direct' kernel/trace/ftrace.c:5489: warning: Function parameter or struct member 'free_filters' not described in 'unregister_ftrace_direct' kernel/trace/ftrace.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c index 65208d3b5ed9..6062e4ce1957 100644 --- a/kernel/trace/ftrace.c +++ b/kernel/trace/ftrace.c @@ -5475,6 +5475,8 @@ EXPORT_SYMBOL_GPL(register_ftrace_direct); * unregister_ftrace_direct - Remove calls to custom trampoline * previously registered by register_ftrace_direct for @ops object. * @ops: The address of the struct ftrace_ops object + * @addr: The address of the trampoline to call at @ops functions + * @free_filters: non zero to remove all filters for the ftrace_ops * * This is used to remove a direct calls to @addr from the nop locations * of the functions registered in @ops (with by ftrace_set_filter_ip -- 2.34.1
Re: [PATCH v2 2/2] selftests/user_events: Add non-spacing separator check
On Tue, 23 Apr 2024 16:23:38 + Beau Belgrave wrote: > The ABI documentation indicates that field separators do not need a > space between them, only a ';'. When no spacing is used, the register > must work. Any subsequent register, with or without spaces, must match > and not return -EADDRINUSE. > > Add a non-spacing separator case to our self-test register case to ensure > it works going forward. > Looks good to me. Acked-by: Masami Hiramatsu (Google) Thanks! > Signed-off-by: Beau Belgrave > --- > tools/testing/selftests/user_events/ftrace_test.c | 8 > 1 file changed, 8 insertions(+) > > diff --git a/tools/testing/selftests/user_events/ftrace_test.c > b/tools/testing/selftests/user_events/ftrace_test.c > index dcd7509fe2e0..0bb46793dcd4 100644 > --- a/tools/testing/selftests/user_events/ftrace_test.c > +++ b/tools/testing/selftests/user_events/ftrace_test.c > @@ -261,6 +261,12 @@ TEST_F(user, register_events) { > ASSERT_EQ(0, ioctl(self->data_fd, DIAG_IOCSREG, ®)); > ASSERT_EQ(0, reg.write_index); > > + /* Register without separator spacing should still match */ > + reg.enable_bit = 29; > + reg.name_args = (__u64)"__test_event u32 field1;u32 field2"; > + ASSERT_EQ(0, ioctl(self->data_fd, DIAG_IOCSREG, ®)); > + ASSERT_EQ(0, reg.write_index); > + > /* Multiple registers to same name but different args should fail */ > reg.enable_bit = 29; > reg.name_args = (__u64)"__test_event u32 field1;"; > @@ -288,6 +294,8 @@ TEST_F(user, register_events) { > ASSERT_EQ(0, ioctl(self->data_fd, DIAG_IOCSUNREG, &unreg)); > unreg.disable_bit = 30; > ASSERT_EQ(0, ioctl(self->data_fd, DIAG_IOCSUNREG, &unreg)); > + unreg.disable_bit = 29; > + ASSERT_EQ(0, ioctl(self->data_fd, DIAG_IOCSUNREG, &unreg)); > > /* Delete should have been auto-done after close and unregister */ > close(self->data_fd); > -- > 2.34.1 > -- Masami Hiramatsu (Google)
Re: [PATCH v2] ftrace: Fix stack trace entry generated by ftrace_pid_func()
On Mon, May 27, 2024 at 07:49:14PM GMT, Steven Rostedt wrote: > On Mon, 27 May 2024 18:44:56 +0900 > Tatsuya S wrote: > > > On setting set_ftrace_pid, a extra entry generated by ftrace_pid_func() > > is shown on stack trace(CONFIG_UNWINDER_FRAME_POINTER=y). > > > > [004] .68.459382: > > => 0xa00090af > > => ksys_read > > => __x64_sys_read > > => x64_sys_call > > => do_syscall_64 > > => entry_SYSCALL_64_after_hwframe > > > > To resolve this issue, increment skip count > > in function_stack_trace_call() if pids are set. > > Just a note, this isn't a "fix" but simply an improvement in output. > I'm happy to take this (after testing and more reviewing), but it will > be for the next merge window, and with a different subject line. > > "ftrace: Hide one more entry in stack trace when ftrace_pid is enabled" > > Or something like that. > > Thanks, > > -- Steve I will send patch with new subject line. Thank you. Tatsuya
Re: [PATCH v2 1/5] dt-bindings: remoteproc: qcom,sa8775p-pas: Document the SA8775p ADSP, CDSP and GPDSP
On 5/27/2024 4:56 PM, Krzysztof Kozlowski wrote: On 27/05/2024 10:43, Bartosz Golaszewski wrote: From: Bartosz Golaszewski Document the components used to boot the ADSP, CDSP0, CDSP1, GPDSP0 and GPDSP1 on the SA8775p SoC. Co-developed-by: Tengfei Fan Missing SoB. Signed-off-by: Bartosz Golaszewski ... + +allOf: + - $ref: /schemas/remoteproc/qcom,pas-common.yaml# + + - if: + properties: +compatible: + enum: +- qcom,sa8775p-adsp-pas +then: + properties: +power-domains: + items: +- description: LCX power domain +- description: LMX power domain +power-domain-names: + items: +- const: lcx +- const: lmx + + - if: + properties: +compatible: + enum: +- qcom,sa8775p-cdsp-pas cdsp0 +then: + properties: +power-domains: + items: +- description: CX power domain +- description: MXC power domain +- description: NSP0 power domain +power-domain-names: + items: +- const: cx +- const: mxc +- const: nsp0 Shouldn't this be just nsp, so both cdsp0 and cdsp1 entries can be unified? That's the power domain from the device point of view, so the device expects to be in some NSP domain, not explicitly NSPn. Both cdsp0 and cdsp1 entries can uniformly use nsp. Best regards, Krzysztof -- Thx and BRs, Tengfei Fan
Re: [PATCH v2 3/5] remoteproc: qcom_q6v5_pas: Add support for SA8775p ADSP, CDSP and GPDSP
On 5/27/2024 4:43 PM, Bartosz Golaszewski wrote: From: Tengfei Fan Add support for PIL loading on ADSP, CDSP0, CDSP1, GPDSP0 and GPDSP1 on SA8775p SoCs. Signed-off-by: Tengfei Fan Co-developed-by: Bartosz Golaszewski Signed-off-by: Bartosz Golaszewski --- drivers/remoteproc/qcom_q6v5_pas.c | 92 ++ 1 file changed, 92 insertions(+) diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c index 54d8005d40a3..16053aa99298 100644 --- a/drivers/remoteproc/qcom_q6v5_pas.c +++ b/drivers/remoteproc/qcom_q6v5_pas.c @@ -820,6 +820,23 @@ static const struct adsp_data adsp_resource_init = { .ssctl_id = 0x14, }; +static const struct adsp_data sa8775p_adsp_resource = { + .crash_reason_smem = 423, + .firmware_name = "adsp.mdt", + .pas_id = 1, + .minidump_id = 5, + .auto_boot = true, + .proxy_pd_names = (char*[]){ + "lcx", + "lmx", + NULL + }, + .load_state = "adsp", + .ssr_name = "lpass", + .sysmon_name = "adsp", + .ssctl_id = 0x14, +}; + static const struct adsp_data sdm845_adsp_resource_init = { .crash_reason_smem = 423, .firmware_name = "adsp.mdt", @@ -933,6 +950,42 @@ static const struct adsp_data cdsp_resource_init = { .ssctl_id = 0x17, }; +static const struct adsp_data sa8775p_cdsp0_resource = { + .crash_reason_smem = 601, + .firmware_name = "cdsp0.mdt", + .pas_id = 18, + .minidump_id = 7, + .auto_boot = true, + .proxy_pd_names = (char*[]){ + "cx", + "mxc", + "nsp0", s/nsp0/nsp/ + NULL + }, + .load_state = "cdsp", + .ssr_name = "cdsp", + .sysmon_name = "cdsp", + .ssctl_id = 0x17, +}; + +static const struct adsp_data sa8775p_cdsp1_resource = { + .crash_reason_smem = 633, + .firmware_name = "cdsp1.mdt", + .pas_id = 30, + .minidump_id = 20, + .auto_boot = true, + .proxy_pd_names = (char*[]){ + "cx", + "mxc", + "nsp1", s/nsp1/nsp/ + NULL + }, + .load_state = "nsp", + .ssr_name = "cdsp1", + .sysmon_name = "cdsp1", + .ssctl_id = 0x20, +}; + static const struct adsp_data sdm845_cdsp_resource_init = { .crash_reason_smem = 601, .firmware_name = "cdsp.mdt", @@ -1074,6 +1127,40 @@ static const struct adsp_data sm8350_cdsp_resource = { .ssctl_id = 0x17, }; +static const struct adsp_data sa8775p_gpdsp0_resource = { + .crash_reason_smem = 640, + .firmware_name = "gpdsp0.mdt", + .pas_id = 39, + .minidump_id = 21, + .auto_boot = true, + .proxy_pd_names = (char*[]){ + "cx", + "mxc", + NULL + }, + .load_state = "gpdsp0", + .ssr_name = "gpdsp0", + .sysmon_name = "gpdsp0", + .ssctl_id = 0x21, +}; + +static const struct adsp_data sa8775p_gpdsp1_resource = { + .crash_reason_smem = 641, + .firmware_name = "gpdsp1.mdt", + .pas_id = 40, + .minidump_id = 22, + .auto_boot = true, + .proxy_pd_names = (char*[]){ + "cx", + "mxc", + NULL + }, + .load_state = "gpdsp1", + .ssr_name = "gpdsp1", + .sysmon_name = "gpdsp1", + .ssctl_id = 0x22, +}; + static const struct adsp_data mpss_resource_init = { .crash_reason_smem = 421, .firmware_name = "modem.mdt", @@ -1315,6 +1402,11 @@ static const struct of_device_id adsp_of_match[] = { { .compatible = "qcom,qcs404-adsp-pas", .data = &adsp_resource_init }, { .compatible = "qcom,qcs404-cdsp-pas", .data = &cdsp_resource_init }, { .compatible = "qcom,qcs404-wcss-pas", .data = &wcss_resource_init }, + { .compatible = "qcom,sa8775p-adsp-pas", .data = &sa8775p_adsp_resource}, + { .compatible = "qcom,sa8775p-cdsp0-pas", .data = &sa8775p_cdsp0_resource}, + { .compatible = "qcom,sa8775p-cdsp1-pas", .data = &sa8775p_cdsp1_resource}, + { .compatible = "qcom,sa8775p-gpdsp0-pas", .data = &sa8775p_gpdsp0_resource}, + { .compatible = "qcom,sa8775p-gpdsp1-pas", .data = &sa8775p_gpdsp1_resource}, { .compatible = "qcom,sc7180-adsp-pas", .data = &sm8250_adsp_resource}, { .compatible = "qcom,sc7180-mpss-pas", .data = &mpss_resource_init}, { .compatible = "qcom,sc7280-adsp-pas", .data = &sm8350_adsp_resource}, -- Thx and BRs, Tengfei Fan
Re: [PATCH v2 4/5] arm64: dts: qcom: sa8775p: add ADSP, CDSP and GPDSP nodes
On 5/27/2024 4:43 PM, Bartosz Golaszewski wrote: From: Tengfei Fan Add nodes for remoteprocs: ADSP, CDSP0, CDSP1, GPDSP0 and GPDSP1 for SA8775p SoCs. Signed-off-by: Tengfei Fan Co-developed-by: Bartosz Golaszewski Signed-off-by: Bartosz Golaszewski --- arch/arm64/boot/dts/qcom/sa8775p.dtsi | 332 ++ 1 file changed, 332 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/sa8775p.dtsi b/arch/arm64/boot/dts/qcom/sa8775p.dtsi index 31de73594839..5c0b61a5624b 100644 --- a/arch/arm64/boot/dts/qcom/sa8775p.dtsi +++ b/arch/arm64/boot/dts/qcom/sa8775p.dtsi @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -544,6 +545,121 @@ cpucp_fw_mem: cpucp-fw@db20 { }; }; + smp2p-adsp { + compatible = "qcom,smp2p"; + qcom,smem = <443>, <429>; + interrupts-extended = <&ipcc IPCC_CLIENT_LPASS +IPCC_MPROC_SIGNAL_SMP2P +IRQ_TYPE_EDGE_RISING>; + mboxes = <&ipcc IPCC_CLIENT_LPASS IPCC_MPROC_SIGNAL_SMP2P>; + + qcom,local-pid = <0>; + qcom,remote-pid = <2>; + + smp2p_adsp_out: master-kernel { + qcom,entry-name = "master-kernel"; + #qcom,smem-state-cells = <1>; + }; + + smp2p_adsp_in: slave-kernel { + qcom,entry-name = "slave-kernel"; + interrupt-controller; + #interrupt-cells = <2>; + }; + }; + + smp2p-cdsp0 { + compatible = "qcom,smp2p"; + qcom,smem = <94>, <432>; + interrupts-extended = <&ipcc IPCC_CLIENT_CDSP +IPCC_MPROC_SIGNAL_SMP2P +IRQ_TYPE_EDGE_RISING>; + mboxes = <&ipcc IPCC_CLIENT_CDSP IPCC_MPROC_SIGNAL_SMP2P>; + + qcom,local-pid = <0>; + qcom,remote-pid = <5>; + + smp2p_cdsp0_out: master-kernel { + qcom,entry-name = "master-kernel"; + #qcom,smem-state-cells = <1>; + }; + + smp2p_cdsp0_in: slave-kernel { + qcom,entry-name = "slave-kernel"; + interrupt-controller; + #interrupt-cells = <2>; + }; + }; + + smp2p-cdsp1 { + compatible = "qcom,smp2p"; + qcom,smem = <617>, <616>; + interrupts-extended = <&ipcc IPCC_CLIENT_NSP1 +IPCC_MPROC_SIGNAL_SMP2P +IRQ_TYPE_EDGE_RISING>; + mboxes = <&ipcc IPCC_CLIENT_NSP1 IPCC_MPROC_SIGNAL_SMP2P>; + + qcom,local-pid = <0>; + qcom,remote-pid = <12>; + + smp2p_cdsp1_out: master-kernel { + qcom,entry-name = "master-kernel"; + #qcom,smem-state-cells = <1>; + }; + + smp2p_cdsp1_in: slave-kernel { + qcom,entry-name = "slave-kernel"; + interrupt-controller; + #interrupt-cells = <2>; + }; + }; + + smp2p-gpdsp0 { + compatible = "qcom,smp2p"; + qcom,smem = <617>, <616>; + interrupts-extended = <&ipcc IPCC_CLIENT_GPDSP0 +IPCC_MPROC_SIGNAL_SMP2P +IRQ_TYPE_EDGE_RISING>; + mboxes = <&ipcc IPCC_CLIENT_GPDSP0 IPCC_MPROC_SIGNAL_SMP2P>; + + qcom,local-pid = <0>; + qcom,remote-pid = <17>; + + smp2p_gpdsp0_out: master-kernel { + qcom,entry-name = "master-kernel"; + #qcom,smem-state-cells = <1>; + }; + + smp2p_gpdsp0_in: slave-kernel { + qcom,entry-name = "slave-kernel"; + interrupt-controller; + #interrupt-cells = <2>; + }; + }; + + smp2p-gpdsp1 { + compatible = "qcom,smp2p"; + qcom,smem = <617>, <616>; + interrupts-extended = <&ipcc IPCC_CLIENT_GPDSP1 +IPCC_MPROC_SIGNAL_SMP2P +IRQ_TYPE_EDGE_RISING>; + mboxes = <&ipcc IPCC_CLIENT_GPDSP1 IPCC_MPROC_SIGNAL_SMP2P>; + + qcom,local-pid = <0>; + qcom,remote-pid = <18>; + + smp2p_gpdsp1_out: master-kernel { + qcom,entry-name = "master-kernel"; + #qcom,smem-state-cells = <1>; + }; + + smp2p_gpdsp1_in: slave-kernel { +
[PATCH v3] ftrace: Hide one more entry in stack trace when ftrace_pid is enabled
On setting set_ftrace_pid, a extra entry generated by ftrace_pid_func() is shown on stack trace(CONFIG_UNWINDER_FRAME_POINTER=y). [004] .68.459382: => 0xa00090af => ksys_read => __x64_sys_read => x64_sys_call => do_syscall_64 => entry_SYSCALL_64_after_hwframe To resolve this issue, increment skip count in function_stack_trace_call() if pids are set. Signed-off-by: Tatsuya S --- Changes in v3: - New subject line - Link to v2: https://lore.kernel.org/linux-trace-kernel/20240527100916.5737-2-tatsuya.s2...@gmail.com Changes in v2: - Fix build warnings reported by kernel test robot - Link to v1: https://lore.kernel.org/linux-trace-kernel/20240526112658.46740-1-tatsuya.s2...@gmail.com/ include/linux/ftrace.h | 2 ++ kernel/trace/ftrace.c | 2 +- kernel/trace/trace_functions.c | 7 ++- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h index 800995c425e0..0855dfe768eb 100644 --- a/include/linux/ftrace.h +++ b/include/linux/ftrace.h @@ -913,6 +913,8 @@ static inline bool is_ftrace_trampoline(unsigned long addr) /* totally disable ftrace - can not re-enable after this */ void ftrace_kill(void); +bool ftrace_pids_enabled(struct ftrace_ops *ops); + static inline void tracer_disable(void) { #ifdef CONFIG_FUNCTION_TRACER diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c index 65208d3b5ed9..e8ddd56d1e55 100644 --- a/kernel/trace/ftrace.c +++ b/kernel/trace/ftrace.c @@ -99,7 +99,7 @@ struct ftrace_ops *function_trace_op __read_mostly = &ftrace_list_end; /* What to set function_trace_op to */ static struct ftrace_ops *set_function_trace_op; -static bool ftrace_pids_enabled(struct ftrace_ops *ops) +bool ftrace_pids_enabled(struct ftrace_ops *ops) { struct trace_array *tr; diff --git a/kernel/trace/trace_functions.c b/kernel/trace/trace_functions.c index 9f1bfbe105e8..455c9a880199 100644 --- a/kernel/trace/trace_functions.c +++ b/kernel/trace/trace_functions.c @@ -223,6 +223,7 @@ function_stack_trace_call(unsigned long ip, unsigned long parent_ip, long disabled; int cpu; unsigned int trace_ctx; + int skip = STACK_SKIP; if (unlikely(!tr->function_enabled)) return; @@ -239,7 +240,11 @@ function_stack_trace_call(unsigned long ip, unsigned long parent_ip, if (likely(disabled == 1)) { trace_ctx = tracing_gen_ctx_flags(flags); trace_function(tr, ip, parent_ip, trace_ctx); - __trace_stack(tr, trace_ctx, STACK_SKIP); +#ifdef CONFIG_UNWINDER_FRAME_POINTER + if (ftrace_pids_enabled(op)) + skip++; +#endif + __trace_stack(tr, trace_ctx, skip); } atomic_dec(&data->disabled); -- 2.45.1
Re: [PATCH v4 0/4] MSM8976 MDSS/GPU/WCNSS support
On Wed, 08 May 2024 18:34:33 +0200, Adam Skladowski wrote: > This patch series provide support for display subsystem, gpu > and also adds wireless connectivity subsystem support. > > Changes since v3 > > 1. Minor styling fixes > 2. Converted qcom,ipc into mailbox on wcnss patch > > [...] Applied, thanks! [1/4] arm64: dts: qcom: msm8976: Add IOMMU nodes commit: 418c2ffd7df9bfc25c21172bd881b78d7569fb4d [2/4] arm64: dts: qcom: msm8976: Add MDSS nodes commit: b0516dbf8e218dede2fd2837ca82dccd9cdcdafc [3/4] arm64: dts: qcom: msm8976: Add Adreno GPU commit: 00e67d8e80f06bb848a3dd516d06e2f040b7d8f2 [4/4] arm64: dts: qcom: msm8976: Add WCNSS node commit: 45878973229a93f0f42aa048ac8c6223af010082 Best regards, -- Bjorn Andersson
Re: (subset) [PATCH v3 0/2] Add samsung-milletwifi
On Mon, 19 Feb 2024 22:43:15 +0100, Bryant Mairs wrote: > This series adds support for samsung-milletwifi, the smaller cousin > to samsung-matisselte. I've used the manufacturer's naming convention > for consistency. > > Hardware currently supported: > - Display > - Cover detection > - Physical buttons > - Touchscreen and touchkeys > - Accelerometer > > [...] Applied, thanks! [2/2] ARM: dts: qcom: Add support for Samsung Galaxy Tab 4 8.0 Wi-Fi commit: 49b9981a0ecae2bbb298d8b0c2b8058220038691 Best regards, -- Bjorn Andersson
Re: (subset) [PATCH 0/2] Add basic APR sound support for SC7280 SoC
On Fri, 10 May 2024 14:27:07 +0200, Luca Weiss wrote: > Validated on Fairphone 5 (QCM6490) smartphone by using DisplayPort over > USB-C audio, connected to a TV, with a basic UCM to enable > 'DISPLAY_PORT_RX Audio Mixer MultiMedia1': > https://gitlab.com/postmarketOS/pmaports/-/tree/master/device/testing/device-fairphone-fp5/ucm > > Unfortunately all the device-specific things can't be enabled yet > upstream as detailed in the second patch, but the SoC parts should be > good to go. > > [...] Applied, thanks! [1/2] arm64: dts: qcom: sc7280: Add APR nodes for sound commit: f44da5d8722de348ff2eb8b206c69b52809c1772 Best regards, -- Bjorn Andersson
Re: (subset) [PATCH v2 0/2] Add Samsung Galaxy Note 3 support
On Thu, 14 Mar 2024 20:00:13 +0100, Luca Weiss wrote: > Add the dts for "hlte" which is a phablet from 2013. > > Applied, thanks! [2/2] ARM: dts: qcom: msm8974: Add Samsung Galaxy Note 3 commit: b4f6c63bf34d8da1b769483bb1f4a603c53896ce Best regards, -- Bjorn Andersson
Re: Bug in Kernel 6.8.x, 6.9.x Causing Trace/Panic During Shutdown/Reboot
I tried 6.10-rc1 and it still ends up to panic --Ilkka On Tue, May 28, 2024 at 12:44 AM Steven Rostedt wrote: > > On Mon, 27 May 2024 20:14:42 +0200 > Greg KH wrote: > > > On Mon, May 27, 2024 at 07:40:21PM +0300, Ilkka Naulapää wrote: > > > Hi Steven, > > > > > > I took some time and bisected the 6.8.9 - 6.8.10 and git gave the > > > panic inducing commit: > > > > > > 414fb08628143 (tracefs: Reset permissions on remount if permissions are > > > options) > > > > > > I reverted that commit to 6.9.2 and now it only serves the trace but > > > the panic is gone. But I can live with it. > > > > Steven, should we revert that? > > > > Or is there some other change that we should take to resolve this? > > > > Before we revert it (as it may be a bug in mainline), Ilkka, can you > test v6.10-rc1? If it exists there, it will let me know whether or not > I missed something. > > Thanks, > > -- Steve
Re: Bug in Kernel 6.8.x, 6.9.x Causing Trace/Panic During Shutdown/Reboot
yeah, the cache_from_obj tracing bug (without panic) has been displayed quite some time now - maybe even since 6.7.x or so. I could try checking a few versions back for this and try bisecting it if I can find when this started. --Ilkka On Tue, May 28, 2024 at 1:31 AM Steven Rostedt wrote: > > On Fri, 24 May 2024 12:50:08 +0200 > "Linux regression tracking (Thorsten Leemhuis)" > wrote: > > > > - Affected Versions: Before kernel version 6.8.10, the bug caused a > > > quick display of a kernel trace dump before the shutdown/reboot > > > completed. Starting from version 6.8.10 and continuing into version > > > 6.9.0 and 6.9.1, this issue has escalated to a kernel panic, > > > preventing the shutdown or reboot from completing and leaving the > > > machine stuck. > > You state "Before kernel version 6.8.10, the bug caused ...". Does that > mean that a bug was happening before v6.8.10? But did not cause a panic? > > I just noticed your second screen shot from your report, and it has: > > "cache_from_obj: Wrong slab cache, tracefs_inode_cache but object is from > inode_cache" > > So somehow an tracefs_inode was allocated from the inode_cache and is > now being freed by the tracefs_inode logic? Did this happen before > 6.8.10? If so, this code could just be triggering an issue from an > unrelated bug. > > Thanks, > > -- Steve