[PATCHv2] soc: qcom: llcc: Support chipsets that can write to llcc registers
From: "Isaac J. Manjarres"
Older chipsets may not be allowed to configure certain LLCC registers
as that is handled by the secure side software. However, this is not
the case for newer chipsets and they must configure these registers
according to the contents of the SCT table, while keeping in mind that
older targets may not have these capabilities. So add support to allow
such configuration of registers to enable capacity based allocation
and power collapse retention for capable chipsets.
Signed-off-by: Isaac J. Manjarres
(sai: use table instead of dt property and minor commit msg change)
Signed-off-by: Sai Prakash Ranjan
---
Changes in v2:
* Fix build errors reported by kernel test robot.
---
drivers/soc/qcom/llcc-qcom.c | 27 +++
1 file changed, 27 insertions(+)
diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
index 429b5a60a1ba..865f607cf502 100644
--- a/drivers/soc/qcom/llcc-qcom.c
+++ b/drivers/soc/qcom/llcc-qcom.c
@@ -45,6 +45,9 @@
#define LLCC_TRP_ATTR0_CFGn(n)(0x21000 + SZ_8 * n)
#define LLCC_TRP_ATTR1_CFGn(n)(0x21004 + SZ_8 * n)
+#define LLCC_TRP_SCID_DIS_CAP_ALLOC 0x21F00
+#define LLCC_TRP_PCB_ACT 0x21F04
+
#define BANK_OFFSET_STRIDE 0x8
/**
@@ -318,6 +321,11 @@ size_t llcc_get_slice_size(struct llcc_slice_desc *desc)
}
EXPORT_SYMBOL_GPL(llcc_get_slice_size);
+static const struct of_device_id __maybe_unused qcom_llcc_configure_of_match[]
= {
+ { .compatible = "qcom,sc7180-llcc" },
+ { }
+};
+
static int qcom_llcc_cfg_program(struct platform_device *pdev)
{
int i;
@@ -327,13 +335,17 @@ static int qcom_llcc_cfg_program(struct platform_device
*pdev)
u32 attr0_val;
u32 max_cap_cacheline;
u32 sz;
+ u32 disable_cap_alloc = 0, retain_pc = 0;
int ret = 0;
const struct llcc_slice_config *llcc_table;
struct llcc_slice_desc desc;
+ const struct of_device_id *llcc_configure;
sz = drv_data->cfg_size;
llcc_table = drv_data->cfg;
+ llcc_configure = of_match_node(qcom_llcc_configure_of_match,
pdev->dev.of_node);
+
for (i = 0; i < sz; i++) {
attr1_cfg = LLCC_TRP_ATTR1_CFGn(llcc_table[i].slice_id);
attr0_cfg = LLCC_TRP_ATTR0_CFGn(llcc_table[i].slice_id);
@@ -369,6 +381,21 @@ static int qcom_llcc_cfg_program(struct platform_device
*pdev)
attr0_val);
if (ret)
return ret;
+
+ if (llcc_configure) {
+ disable_cap_alloc |= llcc_table[i].dis_cap_alloc <<
llcc_table[i].slice_id;
+ ret = regmap_write(drv_data->bcast_regmap,
+ LLCC_TRP_SCID_DIS_CAP_ALLOC,
disable_cap_alloc);
+ if (ret)
+ return ret;
+
+ retain_pc |= llcc_table[i].retain_on_pc <<
llcc_table[i].slice_id;
+ ret = regmap_write(drv_data->bcast_regmap,
+ LLCC_TRP_PCB_ACT, retain_pc);
+ if (ret)
+ return ret;
+ }
+
if (llcc_table[i].activate_on_init) {
desc.slice_id = llcc_table[i].slice_id;
ret = llcc_slice_activate(&desc);
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH] soc: qcom: llcc: Support chipsets that can write to llcc registers
On 2020-08-17 18:13, kernel test robot wrote:
Hi Sai,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linux/master]
[also build test WARNING on linus/master v5.9-rc1 next-20200817]
[cannot apply to agross-msm/qcom/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Sai-Prakash-Ranjan/soc-qcom-llcc-Support-chipsets-that-can-write-to-llcc-registers/20200817-161342
base:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
bcf876870b95592b52519ed4aafcf9d95999bc9c
config: mips-randconfig-r006-20200817 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project
de71b46a519db014ce906a39f8a0e1b235ef1568)
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross
-O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install mips cross compiling tool for clang build
# apt-get install binutils-mips-linux-gnu
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross
ARCH=mips
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All warnings (new ones prefixed by >>):
drivers/soc/qcom/llcc-qcom.c:343:28: warning: unused variable 'np'
[-Wunused-variable]
const struct device_node *np = dev_of_node(&pdev->dev);
^
drivers/soc/qcom/llcc-qcom.c:324:34: warning: unused variable
'qcom_llcc_configure_of_match' [-Wunused-const-variable]
static const struct of_device_id qcom_llcc_configure_of_match[] = {
^
2 warnings generated.
Ok, W=1 build and CONFIG_OF=n, so I need __maybe_unused for
qcom_llcc_configure_of_match.
Will add and send v2.
Thanks,
Sai
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member
of Code Aurora Forum, hosted by The Linux Foundation
[PATCH] soc: qcom: llcc: Support chipsets that can write to llcc registers
From: "Isaac J. Manjarres"
Older chipsets may not be allowed to configure certain LLCC registers
as that is handled by the secure side software. However, this is not
the case for newer chipsets and they must configure these registers
according to the contents of the SCT table, while keeping in mind that
older targets may not have these capabilities. So add support to allow
such configuration of registers to enable capacity based allocation
and power collapse retention for capable chipsets.
Signed-off-by: Isaac J. Manjarres
(sai: use table instead of dt property and minor commit msg change)
Signed-off-by: Sai Prakash Ranjan
---
drivers/soc/qcom/llcc-qcom.c | 28
1 file changed, 28 insertions(+)
diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
index 429b5a60a1ba..20619d15ecba 100644
--- a/drivers/soc/qcom/llcc-qcom.c
+++ b/drivers/soc/qcom/llcc-qcom.c
@@ -45,6 +45,9 @@
#define LLCC_TRP_ATTR0_CFGn(n)(0x21000 + SZ_8 * n)
#define LLCC_TRP_ATTR1_CFGn(n)(0x21004 + SZ_8 * n)
+#define LLCC_TRP_SCID_DIS_CAP_ALLOC 0x21F00
+#define LLCC_TRP_PCB_ACT 0x21F04
+
#define BANK_OFFSET_STRIDE 0x8
/**
@@ -318,6 +321,11 @@ size_t llcc_get_slice_size(struct llcc_slice_desc *desc)
}
EXPORT_SYMBOL_GPL(llcc_get_slice_size);
+static const struct of_device_id qcom_llcc_configure_of_match[] = {
+ { .compatible = "qcom,sc7180-llcc" },
+ { }
+};
+
static int qcom_llcc_cfg_program(struct platform_device *pdev)
{
int i;
@@ -327,13 +335,18 @@ static int qcom_llcc_cfg_program(struct platform_device
*pdev)
u32 attr0_val;
u32 max_cap_cacheline;
u32 sz;
+ u32 disable_cap_alloc = 0, retain_pc = 0;
int ret = 0;
const struct llcc_slice_config *llcc_table;
struct llcc_slice_desc desc;
+ const struct of_device_id *llcc_configure;
+ const struct device_node *np = dev_of_node(&pdev->dev);
sz = drv_data->cfg_size;
llcc_table = drv_data->cfg;
+ llcc_configure = of_match_node(qcom_llcc_configure_of_match, np);
+
for (i = 0; i < sz; i++) {
attr1_cfg = LLCC_TRP_ATTR1_CFGn(llcc_table[i].slice_id);
attr0_cfg = LLCC_TRP_ATTR0_CFGn(llcc_table[i].slice_id);
@@ -369,6 +382,21 @@ static int qcom_llcc_cfg_program(struct platform_device
*pdev)
attr0_val);
if (ret)
return ret;
+
+ if (llcc_configure) {
+ disable_cap_alloc |= llcc_table[i].dis_cap_alloc <<
llcc_table[i].slice_id;
+ ret = regmap_write(drv_data->bcast_regmap,
+ LLCC_TRP_SCID_DIS_CAP_ALLOC,
disable_cap_alloc);
+ if (ret)
+ return ret;
+
+ retain_pc |= llcc_table[i].retain_on_pc <<
llcc_table[i].slice_id;
+ ret = regmap_write(drv_data->bcast_regmap,
+ LLCC_TRP_PCB_ACT, retain_pc);
+ if (ret)
+ return ret;
+ }
+
if (llcc_table[i].activate_on_init) {
desc.slice_id = llcc_table[i].slice_id;
ret = llcc_slice_activate(&desc);
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
[PATCH] arm64: dts: qcom: sc7180: Fix the LLCC base register size
There is only one LLCC logical bank on SC7180 SoC of size
0x5(320KB) not 2MB, so correct the size and fix copy
paste mistake from SDM845 which had 4 logical banks.
Fixes: 7cee5c742899 ("arm64: dts: qcom: sc7180: Fix node order")
Fixes: c831fa26 ("arm64: dts: qcom: sc7180: Add Last level cache controller
node")
Signed-off-by: Sai Prakash Ranjan
---
arch/arm64/boot/dts/qcom/sc7180.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi
b/arch/arm64/boot/dts/qcom/sc7180.dtsi
index d46b3833e52f..e875f6c3b663 100644
--- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
@@ -2618,7 +2618,7 @@ dc_noc: interconnect@916 {
system-cache-controller@920 {
compatible = "qcom,sc7180-llcc";
- reg = <0 0x0920 0 0x20>, <0 0x0960 0
0x5>;
+ reg = <0 0x0920 0 0x5>, <0 0x0960 0
0x5>;
reg-names = "llcc_base", "llcc_broadcast_base";
interrupts = ;
};
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH] arm64: Add KRYO4XX gold CPU core to spectre-v2 safe list
On 2020-08-14 13:47, Marc Zyngier wrote: On 2020-08-14 05:34, Sai Prakash Ranjan wrote: On 2020-08-13 23:29, Marc Zyngier wrote: [...] We'd need to disable the late onlining of CPUs that would change the mitigation state, and this is... ugly. Ugh, yes indeed and here I was thinking that these things are straightforward :( It never is. Big-little is a broken concept. I'll try and think of something next week, as I'm pretty swamped at the moment. Sure no problem, thanks. -Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH] soc: qcom: socinfo: add SC7180 entry to soc_id array
On 2020-08-13 20:33, Douglas Anderson wrote:
Add an entry for SC7180 SoC.
Signed-off-by: Douglas Anderson
---
drivers/soc/qcom/socinfo.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/soc/qcom/socinfo.c b/drivers/soc/qcom/socinfo.c
index e19102f46302..e56eea29615c 100644
--- a/drivers/soc/qcom/socinfo.c
+++ b/drivers/soc/qcom/socinfo.c
@@ -223,6 +223,7 @@ static const struct soc_id soc_id[] = {
{ 321, "SDM845" },
{ 341, "SDA845" },
{ 356, "SM8250" },
+ { 425, "SC7180" },
};
static const char *socinfo_machine(struct device *dev, unsigned int
id)
From the chipinfo document that I have at hand, this is correct soc id
for SC7180, so
Reviewed-by: Sai Prakash Ranjan
Thanks,
Sai
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member
of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH] arm64: Add KRYO4XX gold CPU core to spectre-v2 safe list
On 2020-08-13 23:29, Marc Zyngier wrote:
On 2020-08-13 13:33, Sai Prakash Ranjan wrote:
On 2020-08-13 16:09, Marc Zyngier wrote:
On 2020-08-13 10:40, Will Deacon wrote:
On Thu, Aug 13, 2020 at 02:49:37PM +0530, Sai Prakash Ranjan wrote:
On 2020-08-13 14:33, Will Deacon wrote:
> On Thu, Aug 13, 2020 at 01:48:34PM +0530, Sai Prakash Ranjan wrote:
> > KRYO4XX gold/big CPU cores are based on Cortex-A76 which has CSV2
> > bits set and are spectre-v2 safe. But on big.LITTLE systems where
> > they are coupled with other CPU cores such as the KRYO4XX silver
> > based on Cortex-A55 which are spectre-v2 safe but do not have CSV2
> > bits set, the system wide safe value will be set to the lowest value
> > of CSV2 bits as per FTR_LOWER_SAFE defined for CSV2 bits of register
> > ID_AA64PFR0_EL1.
> >
> > This is a problem when booting a guest kernel on gold CPU cores
> > where it will incorrectly report ARM_SMCCC_ARCH_WORKAROUND_1 warning
> > and consider them as vulnerable for Spectre variant 2 due to system
> > wide safe value which is used in kvm emulation code when reading id
> > registers. One wrong way of fixing this is to set the FTR_HIGHER_SAFE
> > for CSV2 bits, so instead add the KRYO4XX gold CPU core to the safe
> > list which will be consulted even when the sanitised read reports
> > that CSV2 bits are not set for KRYO4XX gold cores.
> >
> > Reported-by: Stephen Boyd
> > Signed-off-by: Sai Prakash Ranjan
> > ---
> > arch/arm64/kernel/cpu_errata.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/arm64/kernel/cpu_errata.c
> > b/arch/arm64/kernel/cpu_errata.c
> > index 6bd1d3ad037a..6cbdd2d98a2a 100644
> > --- a/arch/arm64/kernel/cpu_errata.c
> > +++ b/arch/arm64/kernel/cpu_errata.c
> > @@ -545,6 +545,7 @@ static const struct midr_range
> > spectre_v2_safe_list[] = {
> > MIDR_ALL_VERSIONS(MIDR_HISI_TSV110),
> > MIDR_ALL_VERSIONS(MIDR_QCOM_KRYO_3XX_SILVER),
> > MIDR_ALL_VERSIONS(MIDR_QCOM_KRYO_4XX_SILVER),
> > + MIDR_ALL_VERSIONS(MIDR_QCOM_KRYO_4XX_GOLD),
>
> We shouldn't be putting CPUs in the safe list when they have CSV2
> reporting
> that they are mitigated in hardware, so I don't think this is the right
> approach.
>
Ok but the only thing I find wrong in this approach is that it is a
redundant
information because CSV2 is already advertising the mitigation, but
again
CSV2 check is done first so it doesn't really hurt to add it to the
safe
list because we already know that it is safe.
It simply doesn't scale. That's why CSV2 exists in the first place,
so we
don't have to modify the kernel everytime a new CPU is invented.
> Sounds more like KVM should advertise CSV2 for the vCPUs if all of the
> physical CPUs without CSV2 set are on the safe list. But then again, KVM
> has always been slightly in denial about big.LITTLE because you can't
> sensibly expose it to a guest if there are detectable differences...
>
Sorry but I don't see how the guest kernel will see the CSV2 bits
set for
gold CPU cores without actually adding them to the safe list or
reading the
not sanitised value of ID_AA64PFR0_EL1 ?
Well that's for somebody to figure out in the patch. I'm just saying
that
adding cores to the safe list when they already have a CSV2 field
conveying
the same information is the wrong approach. The right appproach is
for KVM
to expose CSV2 as set when the system is not affected by the
erratum.
A sensible way to fix this would be with something like that:
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 077293b5115f..2735db21ff0d 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1131,6 +1131,9 @@ static u64 read_id_reg(const struct kvm_vcpu
*vcpu,
if (!vcpu_has_sve(vcpu))
val &= ~(0xfUL << ID_AA64PFR0_SVE_SHIFT);
val &= ~(0xfUL << ID_AA64PFR0_AMU_SHIFT);
+ if (!(val & (0xfUL << ID_AA64PFR0_CSV2_SHIFT)) &&
+ get_spectre_v2_workaround_state() ==
ARM64_BP_HARDEN_NOT_REQUIRED)
+ val |= (1UL << ID_AA64PFR0_CSV2_SHIFT);
} else if (id == SYS_ID_AA64ISAR1_EL1 && !vcpu_has_ptrauth(vcpu)) {
val &= ~((0xfUL << ID_AA64ISAR1_APA_SHIFT) |
(0xfUL << ID_AA64ISAR1_API_SHIFT) |
Thanks Marc, I gave this a go on SC7180 where the issue was seen and
this fix is good.
Tested-by: Sai Prakash Ranjan
There is still a problem with this approach. A late CPU could
come up after a guest has been started. If that CPU identified
as vulnerable by the host kernel, get_spectre_v2_workaround_state()
will return a different valu
Re: [PATCH] arm64: Add KRYO4XX gold CPU core to spectre-v2 safe list
On 2020-08-13 16:09, Marc Zyngier wrote:
On 2020-08-13 10:40, Will Deacon wrote:
On Thu, Aug 13, 2020 at 02:49:37PM +0530, Sai Prakash Ranjan wrote:
On 2020-08-13 14:33, Will Deacon wrote:
> On Thu, Aug 13, 2020 at 01:48:34PM +0530, Sai Prakash Ranjan wrote:
> > KRYO4XX gold/big CPU cores are based on Cortex-A76 which has CSV2
> > bits set and are spectre-v2 safe. But on big.LITTLE systems where
> > they are coupled with other CPU cores such as the KRYO4XX silver
> > based on Cortex-A55 which are spectre-v2 safe but do not have CSV2
> > bits set, the system wide safe value will be set to the lowest value
> > of CSV2 bits as per FTR_LOWER_SAFE defined for CSV2 bits of register
> > ID_AA64PFR0_EL1.
> >
> > This is a problem when booting a guest kernel on gold CPU cores
> > where it will incorrectly report ARM_SMCCC_ARCH_WORKAROUND_1 warning
> > and consider them as vulnerable for Spectre variant 2 due to system
> > wide safe value which is used in kvm emulation code when reading id
> > registers. One wrong way of fixing this is to set the FTR_HIGHER_SAFE
> > for CSV2 bits, so instead add the KRYO4XX gold CPU core to the safe
> > list which will be consulted even when the sanitised read reports
> > that CSV2 bits are not set for KRYO4XX gold cores.
> >
> > Reported-by: Stephen Boyd
> > Signed-off-by: Sai Prakash Ranjan
> > ---
> > arch/arm64/kernel/cpu_errata.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/arm64/kernel/cpu_errata.c
> > b/arch/arm64/kernel/cpu_errata.c
> > index 6bd1d3ad037a..6cbdd2d98a2a 100644
> > --- a/arch/arm64/kernel/cpu_errata.c
> > +++ b/arch/arm64/kernel/cpu_errata.c
> > @@ -545,6 +545,7 @@ static const struct midr_range
> > spectre_v2_safe_list[] = {
> > MIDR_ALL_VERSIONS(MIDR_HISI_TSV110),
> > MIDR_ALL_VERSIONS(MIDR_QCOM_KRYO_3XX_SILVER),
> > MIDR_ALL_VERSIONS(MIDR_QCOM_KRYO_4XX_SILVER),
> > + MIDR_ALL_VERSIONS(MIDR_QCOM_KRYO_4XX_GOLD),
>
> We shouldn't be putting CPUs in the safe list when they have CSV2
> reporting
> that they are mitigated in hardware, so I don't think this is the right
> approach.
>
Ok but the only thing I find wrong in this approach is that it is a
redundant
information because CSV2 is already advertising the mitigation, but
again
CSV2 check is done first so it doesn't really hurt to add it to the
safe
list because we already know that it is safe.
It simply doesn't scale. That's why CSV2 exists in the first place, so
we
don't have to modify the kernel everytime a new CPU is invented.
> Sounds more like KVM should advertise CSV2 for the vCPUs if all of the
> physical CPUs without CSV2 set are on the safe list. But then again, KVM
> has always been slightly in denial about big.LITTLE because you can't
> sensibly expose it to a guest if there are detectable differences...
>
Sorry but I don't see how the guest kernel will see the CSV2 bits set
for
gold CPU cores without actually adding them to the safe list or
reading the
not sanitised value of ID_AA64PFR0_EL1 ?
Well that's for somebody to figure out in the patch. I'm just saying
that
adding cores to the safe list when they already have a CSV2 field
conveying
the same information is the wrong approach. The right appproach is for
KVM
to expose CSV2 as set when the system is not affected by the erratum.
A sensible way to fix this would be with something like that:
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 077293b5115f..2735db21ff0d 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1131,6 +1131,9 @@ static u64 read_id_reg(const struct kvm_vcpu
*vcpu,
if (!vcpu_has_sve(vcpu))
val &= ~(0xfUL << ID_AA64PFR0_SVE_SHIFT);
val &= ~(0xfUL << ID_AA64PFR0_AMU_SHIFT);
+ if (!(val & (0xfUL << ID_AA64PFR0_CSV2_SHIFT)) &&
+ get_spectre_v2_workaround_state() ==
ARM64_BP_HARDEN_NOT_REQUIRED)
+ val |= (1UL << ID_AA64PFR0_CSV2_SHIFT);
} else if (id == SYS_ID_AA64ISAR1_EL1 && !vcpu_has_ptrauth(vcpu)) {
val &= ~((0xfUL << ID_AA64ISAR1_APA_SHIFT) |
(0xfUL << ID_AA64ISAR1_API_SHIFT) |
Thanks Marc, I gave this a go on SC7180 where the issue was seen and
this fix is good.
Tested-by: Sai Prakash Ranjan
Thanks,
Sai
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member
of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH] arm64: Add KRYO4XX gold CPU core to spectre-v2 safe list
On 2020-08-13 14:33, Will Deacon wrote:
On Thu, Aug 13, 2020 at 01:48:34PM +0530, Sai Prakash Ranjan wrote:
KRYO4XX gold/big CPU cores are based on Cortex-A76 which has CSV2
bits set and are spectre-v2 safe. But on big.LITTLE systems where
they are coupled with other CPU cores such as the KRYO4XX silver
based on Cortex-A55 which are spectre-v2 safe but do not have CSV2
bits set, the system wide safe value will be set to the lowest value
of CSV2 bits as per FTR_LOWER_SAFE defined for CSV2 bits of register
ID_AA64PFR0_EL1.
This is a problem when booting a guest kernel on gold CPU cores
where it will incorrectly report ARM_SMCCC_ARCH_WORKAROUND_1 warning
and consider them as vulnerable for Spectre variant 2 due to system
wide safe value which is used in kvm emulation code when reading id
registers. One wrong way of fixing this is to set the FTR_HIGHER_SAFE
for CSV2 bits, so instead add the KRYO4XX gold CPU core to the safe
list which will be consulted even when the sanitised read reports
that CSV2 bits are not set for KRYO4XX gold cores.
Reported-by: Stephen Boyd
Signed-off-by: Sai Prakash Ranjan
---
arch/arm64/kernel/cpu_errata.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/kernel/cpu_errata.c
b/arch/arm64/kernel/cpu_errata.c
index 6bd1d3ad037a..6cbdd2d98a2a 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -545,6 +545,7 @@ static const struct midr_range
spectre_v2_safe_list[] = {
MIDR_ALL_VERSIONS(MIDR_HISI_TSV110),
MIDR_ALL_VERSIONS(MIDR_QCOM_KRYO_3XX_SILVER),
MIDR_ALL_VERSIONS(MIDR_QCOM_KRYO_4XX_SILVER),
+ MIDR_ALL_VERSIONS(MIDR_QCOM_KRYO_4XX_GOLD),
We shouldn't be putting CPUs in the safe list when they have CSV2
reporting
that they are mitigated in hardware, so I don't think this is the right
approach.
Ok but the only thing I find wrong in this approach is that it is a
redundant
information because CSV2 is already advertising the mitigation, but
again
CSV2 check is done first so it doesn't really hurt to add it to the safe
list because we already know that it is safe.
Sounds more like KVM should advertise CSV2 for the vCPUs if all of the
physical CPUs without CSV2 set are on the safe list. But then again,
KVM
has always been slightly in denial about big.LITTLE because you can't
sensibly expose it to a guest if there are detectable differences...
Sorry but I don't see how the guest kernel will see the CSV2 bits set
for
gold CPU cores without actually adding them to the safe list or reading
the
not sanitised value of ID_AA64PFR0_EL1 ?
Thanks,
Sai
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member
of Code Aurora Forum, hosted by The Linux Foundation
[PATCH] arm64: Add KRYO4XX gold CPU core to spectre-v2 safe list
KRYO4XX gold/big CPU cores are based on Cortex-A76 which has CSV2
bits set and are spectre-v2 safe. But on big.LITTLE systems where
they are coupled with other CPU cores such as the KRYO4XX silver
based on Cortex-A55 which are spectre-v2 safe but do not have CSV2
bits set, the system wide safe value will be set to the lowest value
of CSV2 bits as per FTR_LOWER_SAFE defined for CSV2 bits of register
ID_AA64PFR0_EL1.
This is a problem when booting a guest kernel on gold CPU cores
where it will incorrectly report ARM_SMCCC_ARCH_WORKAROUND_1 warning
and consider them as vulnerable for Spectre variant 2 due to system
wide safe value which is used in kvm emulation code when reading id
registers. One wrong way of fixing this is to set the FTR_HIGHER_SAFE
for CSV2 bits, so instead add the KRYO4XX gold CPU core to the safe
list which will be consulted even when the sanitised read reports
that CSV2 bits are not set for KRYO4XX gold cores.
Reported-by: Stephen Boyd
Signed-off-by: Sai Prakash Ranjan
---
arch/arm64/kernel/cpu_errata.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index 6bd1d3ad037a..6cbdd2d98a2a 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -545,6 +545,7 @@ static const struct midr_range spectre_v2_safe_list[] = {
MIDR_ALL_VERSIONS(MIDR_HISI_TSV110),
MIDR_ALL_VERSIONS(MIDR_QCOM_KRYO_3XX_SILVER),
MIDR_ALL_VERSIONS(MIDR_QCOM_KRYO_4XX_SILVER),
+ MIDR_ALL_VERSIONS(MIDR_QCOM_KRYO_4XX_GOLD),
{ /* sentinel */ }
};
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
[PATCH] scsi: ufs-qcom: Remove unused msm bus scaling apis
MSM bus scaling has moved on to use interconnect framework
and downstream bus scaling apis like msm_bus_scale*() does
not exist anymore in the kernel. Currently they are guarded
by a config which also does not exist and hence there are no
build failures reported. Remove these unused apis as they
are currently no-op anyways and the scaling support that may
be added in future will use interconnect apis.
Signed-off-by: Sai Prakash Ranjan
---
drivers/scsi/ufs/ufs-qcom.c | 225 +---
drivers/scsi/ufs/ufs-qcom.h | 11 --
2 files changed, 1 insertion(+), 235 deletions(-)
diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
index d0d75527830e..d78f2f8181e8 100644
--- a/drivers/scsi/ufs/ufs-qcom.c
+++ b/drivers/scsi/ufs/ufs-qcom.c
@@ -621,218 +621,6 @@ static int ufs_qcom_resume(struct ufs_hba *hba, enum
ufs_pm_op pm_op)
return 0;
}
-#ifdef CONFIG_MSM_BUS_SCALING
-static int ufs_qcom_get_bus_vote(struct ufs_qcom_host *host,
- const char *speed_mode)
-{
- struct device *dev = host->hba->dev;
- struct device_node *np = dev->of_node;
- int err;
- const char *key = "qcom,bus-vector-names";
-
- if (!speed_mode) {
- err = -EINVAL;
- goto out;
- }
-
- if (host->bus_vote.is_max_bw_needed && !!strcmp(speed_mode, "MIN"))
- err = of_property_match_string(np, key, "MAX");
- else
- err = of_property_match_string(np, key, speed_mode);
-
-out:
- if (err < 0)
- dev_err(dev, "%s: Invalid %s mode %d\n",
- __func__, speed_mode, err);
- return err;
-}
-
-static void ufs_qcom_get_speed_mode(struct ufs_pa_layer_attr *p, char *result)
-{
- int gear = max_t(u32, p->gear_rx, p->gear_tx);
- int lanes = max_t(u32, p->lane_rx, p->lane_tx);
- int pwr;
-
- /* default to PWM Gear 1, Lane 1 if power mode is not initialized */
- if (!gear)
- gear = 1;
-
- if (!lanes)
- lanes = 1;
-
- if (!p->pwr_rx && !p->pwr_tx) {
- pwr = SLOWAUTO_MODE;
- snprintf(result, BUS_VECTOR_NAME_LEN, "MIN");
- } else if (p->pwr_rx == FAST_MODE || p->pwr_rx == FASTAUTO_MODE ||
-p->pwr_tx == FAST_MODE || p->pwr_tx == FASTAUTO_MODE) {
- pwr = FAST_MODE;
- snprintf(result, BUS_VECTOR_NAME_LEN, "%s_R%s_G%d_L%d", "HS",
-p->hs_rate == PA_HS_MODE_B ? "B" : "A", gear, lanes);
- } else {
- pwr = SLOW_MODE;
- snprintf(result, BUS_VECTOR_NAME_LEN, "%s_G%d_L%d",
-"PWM", gear, lanes);
- }
-}
-
-static int __ufs_qcom_set_bus_vote(struct ufs_qcom_host *host, int vote)
-{
- int err = 0;
-
- if (vote != host->bus_vote.curr_vote) {
- err = msm_bus_scale_client_update_request(
- host->bus_vote.client_handle, vote);
- if (err) {
- dev_err(host->hba->dev,
- "%s: msm_bus_scale_client_update_request()
failed: bus_client_handle=0x%x, vote=%d, err=%d\n",
- __func__, host->bus_vote.client_handle,
- vote, err);
- goto out;
- }
-
- host->bus_vote.curr_vote = vote;
- }
-out:
- return err;
-}
-
-static int ufs_qcom_update_bus_bw_vote(struct ufs_qcom_host *host)
-{
- int vote;
- int err = 0;
- char mode[BUS_VECTOR_NAME_LEN];
-
- ufs_qcom_get_speed_mode(&host->dev_req_params, mode);
-
- vote = ufs_qcom_get_bus_vote(host, mode);
- if (vote >= 0)
- err = __ufs_qcom_set_bus_vote(host, vote);
- else
- err = vote;
-
- if (err)
- dev_err(host->hba->dev, "%s: failed %d\n", __func__, err);
- else
- host->bus_vote.saved_vote = vote;
- return err;
-}
-
-static int ufs_qcom_set_bus_vote(struct ufs_hba *hba, bool on)
-{
- struct ufs_qcom_host *host = ufshcd_get_variant(hba);
- int vote, err;
-
- /*
-* In case ufs_qcom_init() is not yet done, simply ignore.
-* This ufs_qcom_set_bus_vote() shall be called from
-* ufs_qcom_init() after init is done.
-*/
- if (!host)
- return 0;
-
- if (on) {
- vote = host->bus_vote.saved_vote;
- if (vote == host->bus_vote.min_bw_vote)
- ufs_qcom_update_bus_bw_vote(host);
- } else {
- vote = host->bus_vote.min_bw_vote;
- }
-
- err = __ufs_qcom_set_bus_vote(ho
[PATCH 2/2] drm/msm/mdp5: Remove unused downstream bus scaling apis
MSM bus scaling has moved on to use interconnect framework
and downstream bus scaling apis are not present anymore.
Remove them as they are nop anyways in the current code,
no functional change.
Signed-off-by: Sai Prakash Ranjan
---
.../gpu/drm/msm/disp/mdp5/mdp5_cmd_encoder.c | 24 ---
drivers/gpu/drm/msm/disp/mdp5/mdp5_encoder.c | 68 ---
2 files changed, 92 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cmd_encoder.c
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cmd_encoder.c
index eeef41fcd4e1..ff2c1d583c79 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cmd_encoder.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cmd_encoder.c
@@ -14,27 +14,6 @@ static struct mdp5_kms *get_kms(struct drm_encoder *encoder)
return to_mdp5_kms(to_mdp_kms(priv->kms));
}
-#ifdef DOWNSTREAM_CONFIG_MSM_BUS_SCALING
-#include
-#include
-#include
-
-static void bs_set(struct mdp5_encoder *mdp5_cmd_enc, int idx)
-{
- if (mdp5_cmd_enc->bsc) {
- DBG("set bus scaling: %d", idx);
- /* HACK: scaling down, and then immediately back up
-* seems to leave things broken (underflow).. so
-* never disable:
-*/
- idx = 1;
- msm_bus_scale_client_update_request(mdp5_cmd_enc->bsc, idx);
- }
-}
-#else
-static void bs_set(struct mdp5_encoder *mdp5_cmd_enc, int idx) {}
-#endif
-
#define VSYNC_CLK_RATE 1920
static int pingpong_tearcheck_setup(struct drm_encoder *encoder,
struct drm_display_mode *mode)
@@ -146,8 +125,6 @@ void mdp5_cmd_encoder_disable(struct drm_encoder *encoder)
mdp5_ctl_set_encoder_state(ctl, pipeline, false);
mdp5_ctl_commit(ctl, pipeline, mdp_ctl_flush_mask_encoder(intf), true);
- bs_set(mdp5_cmd_enc, 0);
-
mdp5_cmd_enc->enabled = false;
}
@@ -161,7 +138,6 @@ void mdp5_cmd_encoder_enable(struct drm_encoder *encoder)
if (WARN_ON(mdp5_cmd_enc->enabled))
return;
- bs_set(mdp5_cmd_enc, 1);
if (pingpong_tearcheck_enable(encoder))
return;
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_encoder.c
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_encoder.c
index f48827283c2b..79d67c495780 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_encoder.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_encoder.c
@@ -16,72 +16,9 @@ static struct mdp5_kms *get_kms(struct drm_encoder *encoder)
return to_mdp5_kms(to_mdp_kms(priv->kms));
}
-#ifdef DOWNSTREAM_CONFIG_MSM_BUS_SCALING
-#include
-#include
-#include
-#define MDP_BUS_VECTOR_ENTRY(ab_val, ib_val) \
- { \
- .src = MSM_BUS_MASTER_MDP_PORT0,\
- .dst = MSM_BUS_SLAVE_EBI_CH0, \
- .ab = (ab_val), \
- .ib = (ib_val), \
- }
-
-static struct msm_bus_vectors mdp_bus_vectors[] = {
- MDP_BUS_VECTOR_ENTRY(0, 0),
- MDP_BUS_VECTOR_ENTRY(20, 20),
-};
-static struct msm_bus_paths mdp_bus_usecases[] = { {
- .num_paths = 1,
- .vectors = &mdp_bus_vectors[0],
-}, {
- .num_paths = 1,
- .vectors = &mdp_bus_vectors[1],
-} };
-static struct msm_bus_scale_pdata mdp_bus_scale_table = {
- .usecase = mdp_bus_usecases,
- .num_usecases = ARRAY_SIZE(mdp_bus_usecases),
- .name = "mdss_mdp",
-};
-
-static void bs_init(struct mdp5_encoder *mdp5_encoder)
-{
- mdp5_encoder->bsc = msm_bus_scale_register_client(
- &mdp_bus_scale_table);
- DBG("bus scale client: %08x", mdp5_encoder->bsc);
-}
-
-static void bs_fini(struct mdp5_encoder *mdp5_encoder)
-{
- if (mdp5_encoder->bsc) {
- msm_bus_scale_unregister_client(mdp5_encoder->bsc);
- mdp5_encoder->bsc = 0;
- }
-}
-
-static void bs_set(struct mdp5_encoder *mdp5_encoder, int idx)
-{
- if (mdp5_encoder->bsc) {
- DBG("set bus scaling: %d", idx);
- /* HACK: scaling down, and then immediately back up
-* seems to leave things broken (underflow).. so
-* never disable:
-*/
- idx = 1;
- msm_bus_scale_client_update_request(mdp5_encoder->bsc, idx);
- }
-}
-#else
-static void bs_init(struct mdp5_encoder *mdp5_encoder) {}
-static void bs_fini(struct mdp5_encoder *mdp5_encoder) {}
-static void bs_set(struct mdp5_encoder *mdp5_encoder, int idx) {}
-#endif
-
static void mdp5_encoder_destroy(struct drm_encoder *encoder)
{
struct mdp5_encoder *mdp5_encoder = to_mdp5_encoder(encoder);
- bs_fini(mdp5_encoder);
drm_encoder_cleanup(encoder);
kfree(mdp5_encoder);
}
@@ -222,8 +159,6 @@ static void mdp5_vid_encoder_disable(struct
[PATCH 1/2] drm/msm/mdp4: Remove unused downstream bus scaling apis
MSM bus scaling has moved on to use interconnect framework
and downstream bus scaling apis are not present anymore.
Remove them as they are nop anyways in the current code,
no functional change.
Signed-off-by: Sai Prakash Ranjan
---
.../gpu/drm/msm/disp/mdp4/mdp4_dtv_encoder.c | 51 ---
drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h | 13 -
.../gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c | 47 -
3 files changed, 111 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_dtv_encoder.c
b/drivers/gpu/drm/msm/disp/mdp4/mdp4_dtv_encoder.c
index 5d8956055286..88645dbc3785 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_dtv_encoder.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_dtv_encoder.c
@@ -25,54 +25,9 @@ static struct mdp4_kms *get_kms(struct drm_encoder *encoder)
return to_mdp4_kms(to_mdp_kms(priv->kms));
}
-#ifdef DOWNSTREAM_CONFIG_MSM_BUS_SCALING
-#include
-/* not ironically named at all.. no, really.. */
-static void bs_init(struct mdp4_dtv_encoder *mdp4_dtv_encoder)
-{
- struct drm_device *dev = mdp4_dtv_encoder->base.dev;
- struct lcdc_platform_data *dtv_pdata = mdp4_find_pdata("dtv.0");
-
- if (!dtv_pdata) {
- DRM_DEV_ERROR(dev->dev, "could not find dtv pdata\n");
- return;
- }
-
- if (dtv_pdata->bus_scale_table) {
- mdp4_dtv_encoder->bsc = msm_bus_scale_register_client(
- dtv_pdata->bus_scale_table);
- DBG("bus scale client: %08x", mdp4_dtv_encoder->bsc);
- DBG("lcdc_power_save: %p", dtv_pdata->lcdc_power_save);
- if (dtv_pdata->lcdc_power_save)
- dtv_pdata->lcdc_power_save(1);
- }
-}
-
-static void bs_fini(struct mdp4_dtv_encoder *mdp4_dtv_encoder)
-{
- if (mdp4_dtv_encoder->bsc) {
- msm_bus_scale_unregister_client(mdp4_dtv_encoder->bsc);
- mdp4_dtv_encoder->bsc = 0;
- }
-}
-
-static void bs_set(struct mdp4_dtv_encoder *mdp4_dtv_encoder, int idx)
-{
- if (mdp4_dtv_encoder->bsc) {
- DBG("set bus scaling: %d", idx);
- msm_bus_scale_client_update_request(mdp4_dtv_encoder->bsc, idx);
- }
-}
-#else
-static void bs_init(struct mdp4_dtv_encoder *mdp4_dtv_encoder) {}
-static void bs_fini(struct mdp4_dtv_encoder *mdp4_dtv_encoder) {}
-static void bs_set(struct mdp4_dtv_encoder *mdp4_dtv_encoder, int idx) {}
-#endif
-
static void mdp4_dtv_encoder_destroy(struct drm_encoder *encoder)
{
struct mdp4_dtv_encoder *mdp4_dtv_encoder =
to_mdp4_dtv_encoder(encoder);
- bs_fini(mdp4_dtv_encoder);
drm_encoder_cleanup(encoder);
kfree(mdp4_dtv_encoder);
}
@@ -162,8 +117,6 @@ static void mdp4_dtv_encoder_disable(struct drm_encoder
*encoder)
clk_disable_unprepare(mdp4_dtv_encoder->hdmi_clk);
clk_disable_unprepare(mdp4_dtv_encoder->mdp_clk);
- bs_set(mdp4_dtv_encoder, 0);
-
mdp4_dtv_encoder->enabled = false;
}
@@ -185,8 +138,6 @@ static void mdp4_dtv_encoder_enable(struct drm_encoder
*encoder)
MDP4_DMA_CONFIG_PACK(0x21));
mdp4_crtc_set_intf(encoder->crtc, INTF_LCDC_DTV, 1);
- bs_set(mdp4_dtv_encoder, 1);
-
DBG("setting mdp_clk=%lu", pc);
ret = clk_set_rate(mdp4_dtv_encoder->mdp_clk, pc);
@@ -252,8 +203,6 @@ struct drm_encoder *mdp4_dtv_encoder_init(struct drm_device
*dev)
goto fail;
}
- bs_init(mdp4_dtv_encoder);
-
return encoder;
fail:
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h
b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h
index 18933bd81c77..e8ee92ab7956 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h
@@ -222,17 +222,4 @@ static inline struct clk *mpd4_lvds_pll_init(struct
drm_device *dev)
}
#endif
-#ifdef DOWNSTREAM_CONFIG_MSM_BUS_SCALING
-/* bus scaling data is associated with extra pointless platform devices,
- * "dtv", etc.. this is a bit of a hack, but we need a way for encoders
- * to find their pdata to make the bus-scaling stuff work.
- */
-static inline void *mdp4_find_pdata(const char *devname)
-{
- struct device *dev;
- dev = bus_find_device_by_name(&platform_bus_type, NULL, devname);
- return dev ? dev->platform_data : NULL;
-}
-#endif
-
#endif /* __MDP4_KMS_H__ */
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c
b/drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c
index 871f3514ef69..10eb3e5b218e 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c
@@ -30,51 +30,10 @@ static struct mdp4_kms *get_kms(struct drm_encoder *encoder)
return to_mdp4_kms(to_mdp_kms(priv->kms));
}
-#ifdef DOWNSTREAM_CONFIG_MSM_BUS_SCALING
-#include
-stat
[PATCH 0/2] Remove unused downstream bus scaling apis
MSM bus scaling has moved on to use interconnect framework and downstream bus scaling apis are not present anymore. Remove them as they are nop anyways in the current code, no functional change. Sai Prakash Ranjan (2): drm/msm/mdp4: Remove unused downstream bus scaling apis drm/msm/mdp5: Remove unused downstream bus scaling apis .../gpu/drm/msm/disp/mdp4/mdp4_dtv_encoder.c | 51 -- drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h | 13 .../gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c | 47 - .../gpu/drm/msm/disp/mdp5/mdp5_cmd_encoder.c | 24 --- drivers/gpu/drm/msm/disp/mdp5/mdp5_encoder.c | 68 --- 5 files changed, 203 deletions(-) -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH] coresight: etm4x: Add Support for HiSilicon ETM device
Hi Qi Liu,
On 2020-08-03 19:05, Qi Liu wrote:
Add ETMv4 periperhal ID for HiSilicon Hip08 and Hip09 platform. Hip08
contains ETMv4.2 device and Hip09 contains ETMv4.5 device.
Signed-off-by: Qi Liu
---
drivers/hwtracing/coresight/coresight-etm4x.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c
b/drivers/hwtracing/coresight/coresight-etm4x.c
index 0c35cd5e..4a4f0bd 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x.c
@@ -1561,6 +1561,8 @@ static struct amba_cs_uci_id uci_id_etm4[] = {
};
static const struct amba_id etm4_ids[] = {
+ CS_AMBA_ID(0x000b6d02), /* HiSilicon-Hip09 */
+ CS_AMBA_ID(0x000b6d01), /* HiSilicon-Hip08 */
CS_AMBA_ID(0x000bb95d), /* Cortex-A53 */
CS_AMBA_ID(0x000bb95e), /* Cortex-A57 */
CS_AMBA_ID(0x000bb95a), /* Cortex-A72 */
--
2.8.1
You have missed adding the maintainers/reviewers of coresight(Mathieu
and Mike),
you can use scripts/get_maintainer.pl on the drivers to get the
maintainers and
reviewers for the corresponding drivers, same on other patch as well.
Thanks,
Sai
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member
of Code Aurora Forum, hosted by The Linux Foundation
[PATCHv3] coresight: etm4x: Fix etm4_count race by moving cpuhp callbacks to init
etm4_count keeps track of number of ETMv4 registered and on some systems,
a race is observed on etm4_count variable which can lead to multiple calls
to cpuhp_setup_state_nocalls_cpuslocked(). This function internally calls
cpuhp_store_callbacks() which prevents multiple registrations of callbacks
for a given state and due to this race, it returns -EBUSY leading to ETM
probe failures like below.
coresight-etm4x: probe of 704.etm failed with error -16
This race can easily be triggered with async probe by setting probe type
as PROBE_PREFER_ASYNCHRONOUS and with ETM power management property
"arm,coresight-loses-context-with-cpu".
Prevent this race by moving cpuhp callbacks to etm driver init since the
cpuhp callbacks doesn't have to depend on the etm4_count and can be once
setup during driver init. Similarly we move cpu_pm notifier registration
to driver init and completely remove etm4_count usage. Also now we can
use non cpuslocked version of cpuhp callbacks with this movement.
Fixes: 9b6a3f3633a5 ("coresight: etmv4: Fix CPU power management setup in
probe() function")
Fixes: 58eb457be028 ("hwtracing/coresight-etm4x: Convert to hotplug state
machine")
Suggested-by: Suzuki K Poulose
Signed-off-by: Sai Prakash Ranjan
---
Changes in v3:
* Minor cleanups from v2 and change to device_initcall (Stephen Boyd)
* Move to non cpuslocked cpuhp callbacks and rename to etm_pm_setup() (Mike
Leach)
Changes in v2:
* Rearrange cpuhp callbacks and move them to driver init (Suzuki K Poulose)
---
drivers/hwtracing/coresight/coresight-etm4x.c | 65 +--
1 file changed, 31 insertions(+), 34 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c
b/drivers/hwtracing/coresight/coresight-etm4x.c
index 6d7d2169bfb2..fddfd93b9a7b 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x.c
@@ -48,8 +48,6 @@ module_param(pm_save_enable, int, 0444);
MODULE_PARM_DESC(pm_save_enable,
"Save/restore state on power down: 1 = never, 2 = self-hosted");
-/* The number of ETMv4 currently registered */
-static int etm4_count;
static struct etmv4_drvdata *etmdrvdata[NR_CPUS];
static void etm4_set_default_config(struct etmv4_config *config);
static int etm4_set_event_filters(struct etmv4_drvdata *drvdata,
@@ -1398,28 +1396,25 @@ static struct notifier_block etm4_cpu_pm_nb = {
.notifier_call = etm4_cpu_pm_notify,
};
-/* Setup PM. Called with cpus locked. Deals with error conditions and counts */
-static int etm4_pm_setup_cpuslocked(void)
+/* Setup PM. Deals with error conditions and counts */
+static int __init etm4_pm_setup(void)
{
int ret;
- if (etm4_count++)
- return 0;
-
ret = cpu_pm_register_notifier(&etm4_cpu_pm_nb);
if (ret)
- goto reduce_count;
+ return ret;
- ret =
cpuhp_setup_state_nocalls_cpuslocked(CPUHP_AP_ARM_CORESIGHT_STARTING,
- "arm/coresight4:starting",
- etm4_starting_cpu,
etm4_dying_cpu);
+ ret = cpuhp_setup_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING,
+ "arm/coresight4:starting",
+ etm4_starting_cpu, etm4_dying_cpu);
if (ret)
goto unregister_notifier;
- ret = cpuhp_setup_state_nocalls_cpuslocked(CPUHP_AP_ONLINE_DYN,
- "arm/coresight4:online",
- etm4_online_cpu, NULL);
+ ret = cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN,
+ "arm/coresight4:online",
+ etm4_online_cpu, NULL);
/* HP dyn state ID returned in ret on success */
if (ret > 0) {
@@ -1428,21 +1423,15 @@ static int etm4_pm_setup_cpuslocked(void)
}
/* failed dyn state - remove others */
- cpuhp_remove_state_nocalls_cpuslocked(CPUHP_AP_ARM_CORESIGHT_STARTING);
+ cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING);
unregister_notifier:
cpu_pm_unregister_notifier(&etm4_cpu_pm_nb);
-
-reduce_count:
- --etm4_count;
return ret;
}
-static void etm4_pm_clear(void)
+static void __init etm4_pm_clear(void)
{
- if (--etm4_count != 0)
- return;
-
cpu_pm_unregister_notifier(&etm4_cpu_pm_nb);
cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING);
if (hp_online) {
@@ -1498,22 +1487,12 @@ static int etm4_probe(struct amba_device *adev, const
struct amba_id *id)
if (!desc.name)
return -ENOMEM;
- cpus_read_lock();
etmdrvdata[drvdata->cpu] = drvdata;
if (smp_call_function_single(drvdata->cpu,
Re: [PATCHv2] coresight: etm4x: Fix etm4_count race by moving cpuhp callbacks to init
Hi Mike,
On 2020-07-29 01:46, Mike Leach wrote:
Hi Sai,
On Tue, 28 Jul 2020 at 08:51, Sai Prakash Ranjan
wrote:
etm4_count keeps track of number of ETMv4 registered and on some
systems,
a race is observed on etm4_count variable which can lead to multiple
calls
to cpuhp_setup_state_nocalls_cpuslocked(). This function internally
calls
cpuhp_store_callbacks() which prevents multiple registrations of
callbacks
for a given state and due to this race, it returns -EBUSY leading to
ETM
probe failures like below.
coresight-etm4x: probe of 704.etm failed with error -16
This race can easily be triggered with async probe by setting probe
type
as PROBE_PREFER_ASYNCHRONOUS and with ETM power management property
"arm,coresight-loses-context-with-cpu".
Prevent this race by moving cpuhp callbacks to etm driver init since
the
cpuhp callbacks doesn't have to depend on the etm4_count and can be
once
setup during driver init. Similarly we move cpu_pm notifier
registration
to driver init and completely remove etm4_count usage.
Fixes: 9b6a3f3633a5 ("coresight: etmv4: Fix CPU power management setup
in probe() function")
Fixes: 58eb457be028 ("hwtracing/coresight-etm4x: Convert to hotplug
state machine")
Suggested-by: Suzuki K Poulose
Signed-off-by: Sai Prakash Ranjan
---
Changes in v2:
* Rearrange cpuhp callbacks and move them to driver init (Suzuki K
Poulose)
---
drivers/hwtracing/coresight/coresight-etm4x.c | 51
++-
1 file changed, 27 insertions(+), 24 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c
b/drivers/hwtracing/coresight/coresight-etm4x.c
index 6d7d2169bfb2..adb71987a1e3 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x.c
@@ -48,8 +48,6 @@ module_param(pm_save_enable, int, 0444);
MODULE_PARM_DESC(pm_save_enable,
"Save/restore state on power down: 1 = never, 2 =
self-hosted");
-/* The number of ETMv4 currently registered */
-static int etm4_count;
static struct etmv4_drvdata *etmdrvdata[NR_CPUS];
static void etm4_set_default_config(struct etmv4_config *config);
static int etm4_set_event_filters(struct etmv4_drvdata *drvdata,
@@ -1403,12 +1401,9 @@ static int etm4_pm_setup_cpuslocked(void)
{
consider renaming this to etm4_pm_setup() and handing any cpu locking
inside the function.
In the circumstances - as part of the driver init rather than probe it
may be sufficient to call the cpuhp_setup functions without the
_cpuslocked suffix and allow the calls to lock the cpus as they are
made.
i.e. cpuhp_setup_state_nocalls_cpuslocked() =>
cpuhp_setup_state_nocalls()
Sure, will make this change.
int ret;
- if (etm4_count++)
- return 0;
-
ret = cpu_pm_register_notifier(&etm4_cpu_pm_nb);
if (ret)
- goto reduce_count;
+ return ret;
ret =
cpuhp_setup_state_nocalls_cpuslocked(CPUHP_AP_ARM_CORESIGHT_STARTING,
"arm/coresight4:starting",
@@ -1432,17 +1427,11 @@ static int etm4_pm_setup_cpuslocked(void)
unregister_notifier:
cpu_pm_unregister_notifier(&etm4_cpu_pm_nb);
-
-reduce_count:
- --etm4_count;
return ret;
}
static void etm4_pm_clear(void)
{
- if (--etm4_count != 0)
- return;
-
cpu_pm_unregister_notifier(&etm4_cpu_pm_nb);
cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING);
if (hp_online) {
@@ -1498,22 +1487,12 @@ static int etm4_probe(struct amba_device
*adev, const struct amba_id *id)
if (!desc.name)
return -ENOMEM;
- cpus_read_lock();
etmdrvdata[drvdata->cpu] = drvdata;
if (smp_call_function_single(drvdata->cpu,
etm4_init_arch_data, drvdata, 1))
dev_err(dev, "ETM arch init failed\n");
- ret = etm4_pm_setup_cpuslocked();
- cpus_read_unlock();
-
- /* etm4_pm_setup_cpuslocked() does its own cleanup - exit on
error */
- if (ret) {
- etmdrvdata[drvdata->cpu] = NULL;
- return ret;
- }
-
if (etm4_arch_supported(drvdata->arch) == false) {
ret = -EINVAL;
goto err_arch_supported;
@@ -1560,7 +1539,6 @@ static int etm4_probe(struct amba_device *adev,
const struct amba_id *id)
err_arch_supported:
etmdrvdata[drvdata->cpu] = NULL;
- etm4_pm_clear();
return ret;
}
@@ -1598,4 +1576,29 @@ static struct amba_driver etm4x_driver = {
.probe = etm4_probe,
.id_table = etm4_ids,
};
-builtin_amba_driver(etm4x_driver);
+
+static int __init etm4x_init(void)
+{
+ int ret;
+
+ cpus_read_lock();
+ ret = etm4_pm_setup_cpuslocked();
+ cpus_read_unlock();
See my comment above about rename and use of cpus_read_
Re: [PATCHv2] coresight: etm4x: Fix etm4_count race by moving cpuhp callbacks to init
On 2020-07-28 13:59, Stephen Boyd wrote:
Quoting Sai Prakash Ranjan (2020-07-28 00:51:02)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c
b/drivers/hwtracing/coresight/coresight-etm4x.c
index 6d7d2169bfb2..adb71987a1e3 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x.c
@@ -48,8 +48,6 @@ module_param(pm_save_enable, int, 0444);
MODULE_PARM_DESC(pm_save_enable,
"Save/restore state on power down: 1 = never, 2 =
self-hosted");
-/* The number of ETMv4 currently registered */
-static int etm4_count;
static struct etmv4_drvdata *etmdrvdata[NR_CPUS];
static void etm4_set_default_config(struct etmv4_config *config);
static int etm4_set_event_filters(struct etmv4_drvdata *drvdata,
@@ -1403,12 +1401,9 @@ static int etm4_pm_setup_cpuslocked(void)
Is this only called from __init now? If so please mark it as __init
then.
Yes, will change it.
{
int ret;
- if (etm4_count++)
- return 0;
-
ret = cpu_pm_register_notifier(&etm4_cpu_pm_nb);
if (ret)
- goto reduce_count;
+ return ret;
ret =
cpuhp_setup_state_nocalls_cpuslocked(CPUHP_AP_ARM_CORESIGHT_STARTING,
"arm/coresight4:starting",
@@ -1432,17 +1427,11 @@ static int etm4_pm_setup_cpuslocked(void)
unregister_notifier:
cpu_pm_unregister_notifier(&etm4_cpu_pm_nb);
-
-reduce_count:
- --etm4_count;
return ret;
}
static void etm4_pm_clear(void)
This is __init too?
Will change.
{
- if (--etm4_count != 0)
- return;
-
cpu_pm_unregister_notifier(&etm4_cpu_pm_nb);
cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING);
if (hp_online) {
@@ -1598,4 +1576,29 @@ static struct amba_driver etm4x_driver = {
.probe = etm4_probe,
.id_table = etm4_ids,
};
-builtin_amba_driver(etm4x_driver);
+
+static int __init etm4x_init(void)
+{
+ int ret;
+
+ cpus_read_lock();
+ ret = etm4_pm_setup_cpuslocked();
+ cpus_read_unlock();
+
+ /* etm4_pm_setup_cpuslocked() does its own cleanup - exit on
error */
+ if (ret)
+ return ret;
+
+ ret = amba_driver_register(&etm4x_driver);
+ if (ret) {
+ pr_info("Error registering etm4x driver\n");
Use pr_err() please.
Yes indeed, will change.
+ goto err_init;
+ }
+
+ return ret;
+
+err_init:
Why is this a goto?
+ etm4_pm_clear();
+ return ret;
Instead of just putting this in the if (ret) arm?
Will change.
+}
+module_init(etm4x_init);
It was device_initcall before with builtin_amba_driver(), best to not
change that.
Sure.
I will wait to see if there are any more comments on this patch and then
post a v3.
Thanks for the review Stephen.
Thanks,
Sai
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member
of Code Aurora Forum, hosted by The Linux Foundation
[PATCHv2] coresight: etm4x: Fix etm4_count race by moving cpuhp callbacks to init
etm4_count keeps track of number of ETMv4 registered and on some systems,
a race is observed on etm4_count variable which can lead to multiple calls
to cpuhp_setup_state_nocalls_cpuslocked(). This function internally calls
cpuhp_store_callbacks() which prevents multiple registrations of callbacks
for a given state and due to this race, it returns -EBUSY leading to ETM
probe failures like below.
coresight-etm4x: probe of 704.etm failed with error -16
This race can easily be triggered with async probe by setting probe type
as PROBE_PREFER_ASYNCHRONOUS and with ETM power management property
"arm,coresight-loses-context-with-cpu".
Prevent this race by moving cpuhp callbacks to etm driver init since the
cpuhp callbacks doesn't have to depend on the etm4_count and can be once
setup during driver init. Similarly we move cpu_pm notifier registration
to driver init and completely remove etm4_count usage.
Fixes: 9b6a3f3633a5 ("coresight: etmv4: Fix CPU power management setup in
probe() function")
Fixes: 58eb457be028 ("hwtracing/coresight-etm4x: Convert to hotplug state
machine")
Suggested-by: Suzuki K Poulose
Signed-off-by: Sai Prakash Ranjan
---
Changes in v2:
* Rearrange cpuhp callbacks and move them to driver init (Suzuki K Poulose)
---
drivers/hwtracing/coresight/coresight-etm4x.c | 51 ++-
1 file changed, 27 insertions(+), 24 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c
b/drivers/hwtracing/coresight/coresight-etm4x.c
index 6d7d2169bfb2..adb71987a1e3 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x.c
@@ -48,8 +48,6 @@ module_param(pm_save_enable, int, 0444);
MODULE_PARM_DESC(pm_save_enable,
"Save/restore state on power down: 1 = never, 2 = self-hosted");
-/* The number of ETMv4 currently registered */
-static int etm4_count;
static struct etmv4_drvdata *etmdrvdata[NR_CPUS];
static void etm4_set_default_config(struct etmv4_config *config);
static int etm4_set_event_filters(struct etmv4_drvdata *drvdata,
@@ -1403,12 +1401,9 @@ static int etm4_pm_setup_cpuslocked(void)
{
int ret;
- if (etm4_count++)
- return 0;
-
ret = cpu_pm_register_notifier(&etm4_cpu_pm_nb);
if (ret)
- goto reduce_count;
+ return ret;
ret =
cpuhp_setup_state_nocalls_cpuslocked(CPUHP_AP_ARM_CORESIGHT_STARTING,
"arm/coresight4:starting",
@@ -1432,17 +1427,11 @@ static int etm4_pm_setup_cpuslocked(void)
unregister_notifier:
cpu_pm_unregister_notifier(&etm4_cpu_pm_nb);
-
-reduce_count:
- --etm4_count;
return ret;
}
static void etm4_pm_clear(void)
{
- if (--etm4_count != 0)
- return;
-
cpu_pm_unregister_notifier(&etm4_cpu_pm_nb);
cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING);
if (hp_online) {
@@ -1498,22 +1487,12 @@ static int etm4_probe(struct amba_device *adev, const
struct amba_id *id)
if (!desc.name)
return -ENOMEM;
- cpus_read_lock();
etmdrvdata[drvdata->cpu] = drvdata;
if (smp_call_function_single(drvdata->cpu,
etm4_init_arch_data, drvdata, 1))
dev_err(dev, "ETM arch init failed\n");
- ret = etm4_pm_setup_cpuslocked();
- cpus_read_unlock();
-
- /* etm4_pm_setup_cpuslocked() does its own cleanup - exit on error */
- if (ret) {
- etmdrvdata[drvdata->cpu] = NULL;
- return ret;
- }
-
if (etm4_arch_supported(drvdata->arch) == false) {
ret = -EINVAL;
goto err_arch_supported;
@@ -1560,7 +1539,6 @@ static int etm4_probe(struct amba_device *adev, const
struct amba_id *id)
err_arch_supported:
etmdrvdata[drvdata->cpu] = NULL;
- etm4_pm_clear();
return ret;
}
@@ -1598,4 +1576,29 @@ static struct amba_driver etm4x_driver = {
.probe = etm4_probe,
.id_table = etm4_ids,
};
-builtin_amba_driver(etm4x_driver);
+
+static int __init etm4x_init(void)
+{
+ int ret;
+
+ cpus_read_lock();
+ ret = etm4_pm_setup_cpuslocked();
+ cpus_read_unlock();
+
+ /* etm4_pm_setup_cpuslocked() does its own cleanup - exit on error */
+ if (ret)
+ return ret;
+
+ ret = amba_driver_register(&etm4x_driver);
+ if (ret) {
+ pr_info("Error registering etm4x driver\n");
+ goto err_init;
+ }
+
+ return ret;
+
+err_init:
+ etm4_pm_clear();
+ return ret;
+}
+module_init(etm4x_init);
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH 2/4] arm64: dts: qcom: sc7180: Add iommus property to ETR
On 2020-07-28 11:58, Bjorn Andersson wrote: On Mon 27 Jul 21:40 PDT 2020, Sai Prakash Ranjan wrote: On 2020-07-28 02:28, Bjorn Andersson wrote: > On Tue 23 Jun 23:56 PDT 2020, Sai Prakash Ranjan wrote: > > > Hi Bjorn, > > > > On 2020-06-21 13:39, Sai Prakash Ranjan wrote: > > > Hi Bjorn, > > > > > > On 2020-06-21 12:52, Bjorn Andersson wrote: > > > > On Tue 09 Jun 06:30 PDT 2020, Sai Prakash Ranjan wrote: > > > > > > > > > Define iommus property for Coresight ETR component in > > > > > SC7180 SoC with the SID and mask to enable SMMU > > > > > translation for this master. > > > > > > > > > > > > > We don't have &apps_smmu in linux-next, as we've yet to figure out how > > > > to disable the boot splash or support the stream mapping handover. > > > > > > > > So I'm not able to apply this. > > > > > > > > > > This is for SC7180 which has apps_smmu not SM8150. > > > > > > > Please let me know if this needs further explanation. > > > > I must have commented on the wrong patch, sorry about that. The SM8150 > patch in this series does not compile due to the lack of &apps_smmu. > > I've picked the other 3 patches. > Thanks Bjorn, I can resend SM8150 coresight change when SMMU support lands for it since coresight ETR won't work without it on android bootloaders. As for the other 3 patches, Patch 1 and Patch 2 will apply cleanly to the right coresight nodes but due to the missing unique context in Patch 3, it could be applied to some other node. We had to upload this change 3 times in chromium tree to get it applied to the right replicator node :) and this property in Patch 3 is important to fix a hard lockup. I'm not sure why this patch is missing the proper context :/ I couldn't find the changes yet in qcom/for-next or other branches to see if it is applied to right replicator node. In case you haven't applied it yet, Patch 3 change should be applied to "replicator@6b06000" node. Thanks for pointing that out, I've fixed up the incorrectly applied change. (Still not published the branch) For the future I believe you can pass -U to git format-patch to get number of lines of context. Making that bigger than the default 3 should help for the coresight patches. Thanks Bjorn, will use this option going forward. Thanks, Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH 2/4] arm64: dts: qcom: sc7180: Add iommus property to ETR
On 2020-07-28 02:28, Bjorn Andersson wrote: On Tue 23 Jun 23:56 PDT 2020, Sai Prakash Ranjan wrote: Hi Bjorn, On 2020-06-21 13:39, Sai Prakash Ranjan wrote: > Hi Bjorn, > > On 2020-06-21 12:52, Bjorn Andersson wrote: > > On Tue 09 Jun 06:30 PDT 2020, Sai Prakash Ranjan wrote: > > > > > Define iommus property for Coresight ETR component in > > > SC7180 SoC with the SID and mask to enable SMMU > > > translation for this master. > > > > > > > We don't have &apps_smmu in linux-next, as we've yet to figure out how > > to disable the boot splash or support the stream mapping handover. > > > > So I'm not able to apply this. > > > > This is for SC7180 which has apps_smmu not SM8150. > Please let me know if this needs further explanation. I must have commented on the wrong patch, sorry about that. The SM8150 patch in this series does not compile due to the lack of &apps_smmu. I've picked the other 3 patches. Thanks Bjorn, I can resend SM8150 coresight change when SMMU support lands for it since coresight ETR won't work without it on android bootloaders. As for the other 3 patches, Patch 1 and Patch 2 will apply cleanly to the right coresight nodes but due to the missing unique context in Patch 3, it could be applied to some other node. We had to upload this change 3 times in chromium tree to get it applied to the right replicator node :) and this property in Patch 3 is important to fix a hard lockup. I'm not sure why this patch is missing the proper context :/ I couldn't find the changes yet in qcom/for-next or other branches to see if it is applied to right replicator node. In case you haven't applied it yet, Patch 3 change should be applied to "replicator@6b06000" node. Thanks, Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH] coresight: etm4x: Fix etm4_count race using atomic variable
On 2020-07-27 15:09, Suzuki K Poulose wrote:
On 07/27/2020 07:07 AM, Sai Prakash Ranjan wrote:
etm4_count keeps track of number of ETMv4 registered and on some
systems, a race is observed on etm4_count variable which can
lead to multiple calls to cpuhp_setup_state_nocalls_cpuslocked().
This function internally calls cpuhp_store_callbacks() which
prevents multiple registrations of callbacks for a given state
and due to this race, it returns -EBUSY leading to ETM probe
failures like below.
coresight-etm4x: probe of 704.etm failed with error -16
This race can easily be triggered with async probe by setting
probe type as PROBE_PREFER_ASYNCHRONOUS and with ETM power
management property "arm,coresight-loses-context-with-cpu".
Prevent this race by converting etm4_count variable to atomic.
Fixes: 9b6a3f3633a5 ("coresight: etmv4: Fix CPU power management setup
in probe() function")
Fixes: 58eb457be028 ("hwtracing/coresight-etm4x: Convert to hotplug
state machine")
Suggested-by: Mike Leach
(Mike: Rootcause and context for commit message)
Signed-off-by: Sai Prakash Ranjan
Please could we leave the hotplug notifier installed with the driver
init and don't worry about this at all ? We bail out early in the
notifier anyways, if the CPU is not registered with its ETM.
Sure thing, sorry for not taking the cue from earlier discussion.
Thanks,
Sai
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member
of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH] coresight: etm4x: Fix etm4_count race using atomic variable
On 2020-07-27 11:37, Sai Prakash Ranjan wrote:
etm4_count keeps track of number of ETMv4 registered and on some
systems, a race is observed on etm4_count variable which can
lead to multiple calls to cpuhp_setup_state_nocalls_cpuslocked().
This function internally calls cpuhp_store_callbacks() which
prevents multiple registrations of callbacks for a given state
and due to this race, it returns -EBUSY leading to ETM probe
failures like below.
coresight-etm4x: probe of 704.etm failed with error -16
This race can easily be triggered with async probe by setting
probe type as PROBE_PREFER_ASYNCHRONOUS and with ETM power
management property "arm,coresight-loses-context-with-cpu".
Prevent this race by converting etm4_count variable to atomic.
Fixes: 9b6a3f3633a5 ("coresight: etmv4: Fix CPU power management setup
in probe() function")
Fixes: 58eb457be028 ("hwtracing/coresight-etm4x: Convert to hotplug
state machine")
Suggested-by: Mike Leach
(Mike: Rootcause and context for commit message)
Signed-off-by: Sai Prakash Ranjan
---
drivers/hwtracing/coresight/coresight-etm4x.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c
b/drivers/hwtracing/coresight/coresight-etm4x.c
index 6d7d2169bfb2..f256ea744c51 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x.c
@@ -49,7 +49,7 @@ MODULE_PARM_DESC(pm_save_enable,
"Save/restore state on power down: 1 = never, 2 = self-hosted");
/* The number of ETMv4 currently registered */
-static int etm4_count;
+static atomic_t etm4_count;
static struct etmv4_drvdata *etmdrvdata[NR_CPUS];
static void etm4_set_default_config(struct etmv4_config *config);
static int etm4_set_event_filters(struct etmv4_drvdata *drvdata,
@@ -1403,7 +1403,7 @@ static int etm4_pm_setup_cpuslocked(void)
{
int ret;
- if (etm4_count++)
+ if (atomic_inc_return(&etm4_count))
return 0;
Sorry, I messed up here, will send a next version fixing this.
Thanks,
Sai
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member
of Code Aurora Forum, hosted by The Linux Foundation
[PATCH] coresight: etm4x: Fix etm4_count race using atomic variable
etm4_count keeps track of number of ETMv4 registered and on some
systems, a race is observed on etm4_count variable which can
lead to multiple calls to cpuhp_setup_state_nocalls_cpuslocked().
This function internally calls cpuhp_store_callbacks() which
prevents multiple registrations of callbacks for a given state
and due to this race, it returns -EBUSY leading to ETM probe
failures like below.
coresight-etm4x: probe of 704.etm failed with error -16
This race can easily be triggered with async probe by setting
probe type as PROBE_PREFER_ASYNCHRONOUS and with ETM power
management property "arm,coresight-loses-context-with-cpu".
Prevent this race by converting etm4_count variable to atomic.
Fixes: 9b6a3f3633a5 ("coresight: etmv4: Fix CPU power management setup in
probe() function")
Fixes: 58eb457be028 ("hwtracing/coresight-etm4x: Convert to hotplug state
machine")
Suggested-by: Mike Leach
(Mike: Rootcause and context for commit message)
Signed-off-by: Sai Prakash Ranjan
---
drivers/hwtracing/coresight/coresight-etm4x.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c
b/drivers/hwtracing/coresight/coresight-etm4x.c
index 6d7d2169bfb2..f256ea744c51 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x.c
@@ -49,7 +49,7 @@ MODULE_PARM_DESC(pm_save_enable,
"Save/restore state on power down: 1 = never, 2 = self-hosted");
/* The number of ETMv4 currently registered */
-static int etm4_count;
+static atomic_t etm4_count;
static struct etmv4_drvdata *etmdrvdata[NR_CPUS];
static void etm4_set_default_config(struct etmv4_config *config);
static int etm4_set_event_filters(struct etmv4_drvdata *drvdata,
@@ -1403,7 +1403,7 @@ static int etm4_pm_setup_cpuslocked(void)
{
int ret;
- if (etm4_count++)
+ if (atomic_inc_return(&etm4_count))
return 0;
ret = cpu_pm_register_notifier(&etm4_cpu_pm_nb);
@@ -1434,13 +1434,13 @@ static int etm4_pm_setup_cpuslocked(void)
cpu_pm_unregister_notifier(&etm4_cpu_pm_nb);
reduce_count:
- --etm4_count;
+ atomic_dec(&etm4_count);
return ret;
}
static void etm4_pm_clear(void)
{
- if (--etm4_count != 0)
+ if (atomic_dec_return(&etm4_count) != 0)
return;
cpu_pm_unregister_notifier(&etm4_cpu_pm_nb);
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
[PATCH] arm64: dts: qcom: sdm845: Support ETMv4 power management
Add "arm,coresight-loses-context-with-cpu" property to coresight
ETM nodes to avoid failure of trace session because of losing
context on entering deep idle states.
Signed-off-by: Sai Prakash Ranjan
---
arch/arm64/boot/dts/qcom/sdm845.dtsi | 8
1 file changed, 8 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi
b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index e506793407d8..0b5f063dcaea 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -3016,6 +3016,7 @@ etm@704 {
clocks = <&aoss_qmp>;
clock-names = "apb_pclk";
+ arm,coresight-loses-context-with-cpu;
out-ports {
port {
@@ -3035,6 +3036,7 @@ etm@714 {
clocks = <&aoss_qmp>;
clock-names = "apb_pclk";
+ arm,coresight-loses-context-with-cpu;
out-ports {
port {
@@ -3054,6 +3056,7 @@ etm@724 {
clocks = <&aoss_qmp>;
clock-names = "apb_pclk";
+ arm,coresight-loses-context-with-cpu;
out-ports {
port {
@@ -3073,6 +3076,7 @@ etm@734 {
clocks = <&aoss_qmp>;
clock-names = "apb_pclk";
+ arm,coresight-loses-context-with-cpu;
out-ports {
port {
@@ -3092,6 +3096,7 @@ etm@744 {
clocks = <&aoss_qmp>;
clock-names = "apb_pclk";
+ arm,coresight-loses-context-with-cpu;
out-ports {
port {
@@ -3111,6 +3116,7 @@ etm@754 {
clocks = <&aoss_qmp>;
clock-names = "apb_pclk";
+ arm,coresight-loses-context-with-cpu;
out-ports {
port {
@@ -3130,6 +3136,7 @@ etm@764 {
clocks = <&aoss_qmp>;
clock-names = "apb_pclk";
+ arm,coresight-loses-context-with-cpu;
out-ports {
port {
@@ -3149,6 +3156,7 @@ etm@774 {
clocks = <&aoss_qmp>;
clock-names = "apb_pclk";
+ arm,coresight-loses-context-with-cpu;
out-ports {
port {
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH] i2c: i2c-qcom-geni: Fix DMA transfer race
On 2020-07-21 05:54, Douglas Anderson wrote:
When I have KASAN enabled on my kernel and I start stressing the
touchscreen my system tends to hang. The touchscreen is one of the
only things that does a lot of big i2c transfers and ends up hitting
the DMA paths in the geni i2c driver. It appears that KASAN adds
enough delay in my system to tickle a race condition in the DMA setup
code.
When the system hangs, I found that it was running the geni_i2c_irq()
over and over again. It had these:
m_stat = 0x0480
rx_st= 0x3011
dm_tx_st = 0x
dm_rx_st = 0x
dma = 0x0001
Notably we're in DMA mode but are getting M_RX_IRQ_EN and
M_RX_FIFO_WATERMARK_EN over and over again.
Putting some traces in geni_i2c_rx_one_msg() showed that when we
failed we were getting to the start of geni_i2c_rx_one_msg() but were
never executing geni_se_rx_dma_prep().
I believe that the problem here is that we are writing the transfer
length and setting up the geni command before we run
geni_se_rx_dma_prep(). If a transfer makes it far enough before we do
that then we get into the state I have observed. Let's change the
order, which seems to work fine.
Fixes: 37692de5d523 ("i2c: i2c-qcom-geni: Add bus driver for the
Qualcomm GENI I2C controller")
Signed-off-by: Douglas Anderson
---
drivers/i2c/busses/i2c-qcom-geni.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-qcom-geni.c
b/drivers/i2c/busses/i2c-qcom-geni.c
index 18d1e4fd4cf3..21e27f10510a 100644
--- a/drivers/i2c/busses/i2c-qcom-geni.c
+++ b/drivers/i2c/busses/i2c-qcom-geni.c
@@ -366,15 +366,15 @@ static int geni_i2c_rx_one_msg(struct
geni_i2c_dev *gi2c, struct i2c_msg *msg,
else
geni_se_select_mode(se, GENI_SE_FIFO);
- writel_relaxed(len, se->base + SE_I2C_RX_TRANS_LEN);
- geni_se_setup_m_cmd(se, I2C_READ, m_param);
-
if (dma_buf && geni_se_rx_dma_prep(se, dma_buf, len, &rx_dma)) {
geni_se_select_mode(se, GENI_SE_FIFO);
i2c_put_dma_safe_msg_buf(dma_buf, msg, false);
dma_buf = NULL;
}
+ writel_relaxed(len, se->base + SE_I2C_RX_TRANS_LEN);
+ geni_se_setup_m_cmd(se, I2C_READ, m_param);
+
time_left = wait_for_completion_timeout(&gi2c->done, XFER_TIMEOUT);
if (!time_left)
geni_i2c_abort_xfer(gi2c);
Tested-by: Sai Prakash Ranjan
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member
of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH] drm/msm: dsi: dev_pm_opp_put_clkname() only when an opp_table exists
On 2020-07-20 17:31, Rajendra Nayak wrote:
Its possible for msm_dsi_host_init() to fail early, before
dev_pm_opp_set_clkname() is called. In such cases, unconditionally
calling dev_pm_opp_put_clkname() in msm_dsi_host_destroy() results
in a crash. Put an additional check so that dev_pm_opp_put_clkname()
is called only when an opp_table exists.
Fixes: f99131fa7a23 ("drm/msm: dsi: Use OPP API to set clk/perf state")
Reported-by: Sai Prakash Ranjan
Signed-off-by: Rajendra Nayak
---
drivers/gpu/drm/msm/dsi/dsi_host.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 0a14c4a..4f580f7 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -1936,7 +1936,8 @@ void msm_dsi_host_destroy(struct mipi_dsi_host
*host)
if (msm_host->has_opp_table)
dev_pm_opp_of_remove_table(&msm_host->pdev->dev);
- dev_pm_opp_put_clkname(msm_host->opp_table);
+ if (msm_host->opp_table)
+ dev_pm_opp_put_clkname(msm_host->opp_table);
pm_runtime_disable(&msm_host->pdev->dev);
}
Tested-by: Sai Prakash Ranjan
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member
of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH] drm/msm/dpu: dev_pm_opp_put_clkname() only when an opp_table exists
On 2020-07-20 17:47, Rajendra Nayak wrote:
Its possible that dpu_bind() fails early enough before
dev_pm_opp_set_clkname() is called. In such cases, unconditionally
calling dev_pm_opp_put_clkname() in dpu_unbind() can result in
a crash. Put an additional check so that dev_pm_opp_put_clkname()
is called only when an opp_table exists.
Fixes: aa3950767d05 ("drm/msm/dpu: Use OPP API to set clk/perf state")
Signed-off-by: Rajendra Nayak
---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index f2bbce4..843a1c1 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -1079,7 +1079,8 @@ static void dpu_unbind(struct device *dev,
struct device *master, void *data)
if (dpu_kms->has_opp_table)
dev_pm_opp_of_remove_table(dev);
- dev_pm_opp_put_clkname(dpu_kms->opp_table);
+ if (dpu_kms->opp_table)
+ dev_pm_opp_put_clkname(dpu_kms->opp_table);
}
static const struct component_ops dpu_ops = {
Tested-by: Sai Prakash Ranjan
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member
of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH V2] soc: qcom: geni: Fix NULL pointer dereference
On 2020-07-20 15:06, Akash Asthana wrote:
pdev struct doesn't exists for the devices whose status are disabled
from DT node, in such cases NULL is returned from
'of_find_device_by_node'
Later when we try to get drvdata from pdev struct NULL pointer
dereference
is triggered.
Add a NULL check for return values to fix the issue.
We were hitting this issue when one of QUP is disabled.
Fixes: 048eb908a1f2 ("soc: qcom-geni-se: Add interconnect support to
fix earlycon crash")
Reported-by: Sai Prakash Ranjan
Reviewed-by: Matthias Kaehlcke
Signed-off-by: Akash Asthana
Nit: my codeaurora mail address is .
I don't think you have to resend for this small change, hopefully
maintainers
are ok to fix this when applying.
Reported-by: Sai Prakash Ranjan
Tested-by: Sai Prakash Ranjan
-Sai
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member
of Code Aurora Forum, hosted by The Linux Foundation
Re: [Freedreno] [PATCH] iommu/arm-smmu: Add a init_context_bank implementation hook
On 2020-07-14 00:43, Jordan Crouse wrote: On Mon, Jul 13, 2020 at 08:03:32PM +0100, Will Deacon wrote: On Mon, Jul 13, 2020 at 11:00:32AM -0600, Jordan Crouse wrote: > On Mon, Jul 13, 2020 at 04:11:23PM +0100, Will Deacon wrote: > > On Thu, Jun 11, 2020 at 04:36:56PM -0600, Jordan Crouse wrote: > > > Add a new implementation hook to allow the implementation specific code > > > to tweek the context bank configuration just before it gets written. > > > The first user will be the Adreno GPU implementation to turn on > > > SCTLR.HUPCF to ensure that a page fault doesn't terminating pending > > > transactions. Doing so could hang the GPU if one of the terminated > > > transactions is a CP read. > > > > > > This depends on the arm-smmu adreno SMMU implementation [1]. > > > > > > [1] https://patchwork.kernel.org/patch/11600943/ > > > > > > Signed-off-by: Jordan Crouse > > > --- > > > > > > drivers/iommu/arm-smmu-qcom.c | 13 + > > > drivers/iommu/arm-smmu.c | 28 +--- > > > drivers/iommu/arm-smmu.h | 11 +++ > > > 3 files changed, 37 insertions(+), 15 deletions(-) > > > > This looks straightforward enough, but I don't want to merge this without > > a user and Sai's series has open questions afaict. > > Not sure what you mean by a user in this context? > Are you referring to https://patchwork.kernel.org/patch/11628541/? Right, this post was just a single patch in isolation, whereas it was reposted over at: https://lore.kernel.org/r/cdcc6a1c95a84e774790389dc8b3b7f490dc.1593344119.git.saiprakash.ran...@codeaurora.org so I'll ignore this one. Sorry, I'm just really struggling to keep track of what is targetting 5.9, and I don't have tonnes of time to sift through the backlog of duplicate postings :( Yeah, that is our fault. There are too many cooks in the kitchen. We need to pick either system cache or split pagetable and serialize the other on top of it to get the impl code going and then build from there. This particular patch can happily hang out in the background until the rest is resolved. My bad, sorry. Let us get split pagetable support reviewed first, then I can post system cache support on top of it. As jordan said, this patch can hibernate until those get resolved. Thanks, Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH] arm64: Add missing sentinel to erratum_1463225
Hi Florian,
On 2020-07-09 10:43, Florian Fainelli wrote:
When the erratum_1463225 array was introduced a sentinel at the end was
missing thus causing a KASAN: global-out-of-bounds in
is_affected_midr_range_list on arm64 error.
Link:
https://lore.kernel.org/linux-arm-kernel/ca+g9fys3eavpu89-rtqfqq9ggxamgmak7jiivrfp0yxj5s+...@mail.gmail.com/
Fixes: a9e821b89daa ("arm64: Add KRYO4XX gold CPU cores to erratum
list 1463225 and 1418040")
Signed-off-by: Florian Fainelli
---
arch/arm64/kernel/cpu_errata.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/kernel/cpu_errata.c
b/arch/arm64/kernel/cpu_errata.c
index 8e302dc093d0..79728bfb5351 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -782,6 +782,7 @@ static const struct midr_range erratum_1463225[] =
{
MIDR_RANGE(MIDR_CORTEX_A76, 0, 0, 3, 1),
/* Kryo4xx Gold (rcpe to rfpf) => (r0p0 to r3p1) */
MIDR_RANGE(MIDR_QCOM_KRYO_4XX_GOLD, 0xc, 0xe, 0xf, 0xf),
+ {},
};
#endif
My bad missing this, thanks for the fix.
Reviewed-by: Sai Prakash Ranjan
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member
of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCHv3 7/7] drm/msm/a6xx: Add support for using system cache(LLC)
On 2020-07-03 21:34, Rob Clark wrote: On Fri, Jul 3, 2020 at 7:53 AM Sai Prakash Ranjan wrote: Hi Will, On 2020-07-03 19:07, Will Deacon wrote: > On Mon, Jun 29, 2020 at 09:22:50PM +0530, Sai Prakash Ranjan wrote: >> diff --git a/drivers/gpu/drm/msm/msm_iommu.c >> b/drivers/gpu/drm/msm/msm_iommu.c >> index f455c597f76d..bd1d58229cc2 100644 >> --- a/drivers/gpu/drm/msm/msm_iommu.c >> +++ b/drivers/gpu/drm/msm/msm_iommu.c >> @@ -218,6 +218,9 @@ static int msm_iommu_map(struct msm_mmu *mmu, >> uint64_t iova, >> iova |= GENMASK_ULL(63, 49); >> >> >> +if (mmu->features & MMU_FEATURE_USE_SYSTEM_CACHE) >> +prot |= IOMMU_SYS_CACHE_ONLY; > > Given that I think this is the only user of IOMMU_SYS_CACHE_ONLY, then > it > looks like it should actually be a property on the domain because we > never > need to configure it on a per-mapping basis within a domain, and > therefore > it shouldn't be exposed by the IOMMU API as a prot flag. > > Do you agree? > GPU being the only user is for now, but there are other clients which can use this. Plus how do we set the memory attributes if we do not expose this as prot flag? It does appear that the downstream kgsl driver sets this for basically all mappings.. well there is some conditional stuff around DOMAIN_ATTR_USE_LLC_NWA but it seems based on the property of the domain. (Jordan may know more about what that is about.) But looks like there are a lot of different paths into iommu_map in kgsl so I might have missed something. Assuming there isn't some case where we specifically don't want to use the system cache for some mapping, I think it could be a domain attribute that sets an io_pgtable_cfg::quirks flag Ok then we are good to remove unused sys cache prot flag which Will has posted. Thanks, Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH] iommu: Remove unused IOMMU_SYS_CACHE_ONLY flag
On 2020-07-03 21:55, Will Deacon wrote: The IOMMU_SYS_CACHE_ONLY flag was never exposed via the DMA API and has no in-tree users. Remove it. Cc: Robin Murphy Cc: "Isaac J. Manjarres" Cc: Joerg Roedel Cc: Christoph Hellwig Cc: Sai Prakash Ranjan Cc: Rob Clark Signed-off-by: Will Deacon --- As discussed in [1], sounds like this should be a domain attribute anyway when it's needed by the GPU. [1] https://lore.kernel.org/r/caf6aegscrovtsi2r7_aukmh9luoc_gumr0w0kujc2cegpfj...@mail.gmail.com Reviewed-by: Sai Prakash Ranjan QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCHv3 7/7] drm/msm/a6xx: Add support for using system cache(LLC)
Hi Will, On 2020-07-03 19:07, Will Deacon wrote: On Mon, Jun 29, 2020 at 09:22:50PM +0530, Sai Prakash Ranjan wrote: diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c index f455c597f76d..bd1d58229cc2 100644 --- a/drivers/gpu/drm/msm/msm_iommu.c +++ b/drivers/gpu/drm/msm/msm_iommu.c @@ -218,6 +218,9 @@ static int msm_iommu_map(struct msm_mmu *mmu, uint64_t iova, iova |= GENMASK_ULL(63, 49); + if (mmu->features & MMU_FEATURE_USE_SYSTEM_CACHE) + prot |= IOMMU_SYS_CACHE_ONLY; Given that I think this is the only user of IOMMU_SYS_CACHE_ONLY, then it looks like it should actually be a property on the domain because we never need to configure it on a per-mapping basis within a domain, and therefore it shouldn't be exposed by the IOMMU API as a prot flag. Do you agree? GPU being the only user is for now, but there are other clients which can use this. Plus how do we set the memory attributes if we do not expose this as prot flag? Thanks, Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH 3/3] arm64: Add KRYO4XX silver CPU cores to erratum list 1530923 and 1024718
Hi Will, On 2020-07-03 19:25, Will Deacon wrote: On Tue, Jun 30, 2020 at 11:30:55PM +0530, Sai Prakash Ranjan wrote: KRYO4XX silver/LITTLE CPU cores with revision r1p0 are affected by erratum 1530923 and 1024718, so add them to the respective list. The variant and revision bits are implementation defined and are different from the their Cortex CPU counterparts on which they are based on, i.e., r1p0 is equivalent to rdpe. So just to confirm, revisions prior to rdpe are unaffected, or do those parts simply not exist? There is one revision prior to this r0p1(r7pc) which has a different part number and are used in v1 of SoCs which are limited to only internal test platforms in the early stages of bringup and not used in actual devices out there, so I did not add it to the list but they are affected. Plus we would need to add another MIDR_QCOM_KRYO_4XX_SILVER_V1 if we are supporting them which I thought was not worth it when devices with those CPUs are not available. Thanks, Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH v9 0/7] iommu/arm-smmu: Enable split pagetable support
Hi Will, Robin, On 2020-06-27 01:30, Jordan Crouse wrote: Another iteration of the split-pagetable support for arm-smmu and the Adreno GPU SMMU. After email discussions [1] we opted to make a arm-smmu implementation for specifically for the Adreno GPU and use that to enable split pagetable support and later other implementation specific bits that we need. On the hardware side this is very close to the same code from before [2] only the TTBR1 quirk is turned on by the implementation and not a domain attribute. In drm/msm we use the returned size of the aperture as a clue to let us know which virtual address space we should use for global memory objects. There are two open items that you should be aware of. First, in the implementation specific code we have to check the compatible string of the device so that we only enable TTBR1 for the GPU (SID 0) and not the GMU (SID 4). I went back and forth trying to decide if I wanted to use the compatible string or the SID as the filter and settled on the compatible string but I could be talked out of it. The other open item is that in drm/msm the hardware only uses 49 bits of the address space but arm-smmu expects the address to be sign extended all the way to 64 bits. This isn't a problem normally unless you look at the hardware registers that contain a IOVA and then the upper bits will be zero. I opted to restrict the internal drm/msm IOVA range to only 49 bits and then sign extend right before calling iommu_map / iommu_unmap. This is a bit wonky but I thought that matching the hardware would be less confusing when debugging a hang. v9: Fix bot-detected merge conflict v7: Add attached device to smmu_domain to pass to implementation specific functions [1] https://lists.linuxfoundation.org/pipermail/iommu/2020-May/044537.html [2] https://patchwork.kernel.org/patch/11482591/ Jordan Crouse (7): iommu/arm-smmu: Pass io-pgtable config to implementation specific function iommu/arm-smmu: Add support for split pagetables dt-bindings: arm-smmu: Add compatible string for Adreno GPU SMMU iommu/arm-smmu: Add a pointer to the attached device to smmu_domain iommu/arm-smmu: Add implementation for the adreno GPU SMMU drm/msm: Set the global virtual address range from the IOMMU domain arm: dts: qcom: sm845: Set the compatible string for the GPU SMMU .../devicetree/bindings/iommu/arm,smmu.yaml | 4 ++ arch/arm64/boot/dts/qcom/sdm845.dtsi | 2 +- drivers/gpu/drm/msm/adreno/adreno_gpu.c | 13 +- drivers/gpu/drm/msm/msm_iommu.c | 7 +++ drivers/iommu/arm-smmu-impl.c | 6 ++- drivers/iommu/arm-smmu-qcom.c | 45 ++- drivers/iommu/arm-smmu.c | 38 +++- drivers/iommu/arm-smmu.h | 30 ++--- 8 files changed, 120 insertions(+), 25 deletions(-) Any chance reviewing this? Thanks, Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
[PATCH 0/3] Add Kryo4xx gold and silver cores to applicable errata list
This series adds the Kryo4xx gold/big and silver/LITTLE CPU cores to the errata list which are applicable to them based on the revisions of the Cortex CPU cores on which they are based on. Patch 1 adds the MIDR value for Kryo4xx gold CPU cores. Patch 2 adds Kryo4xx gold CPU cores to erratum list 1463225 and 1418040. Patch 3 adds Kryo4xx silver CPU cores to erratum list 1530923 and 1024718. Sai Prakash Ranjan (3): arm64: Add MIDR value for KRYO4XX gold CPU cores arm64: Add KRYO4XX gold CPU cores to erratum list 1463225 and 1418040 arm64: Add KRYO4XX silver CPU cores to erratum list 1530923 and 1024718 Documentation/arm64/silicon-errata.rst | 8 arch/arm64/include/asm/cputype.h | 2 ++ arch/arm64/kernel/cpu_errata.c | 21 +++-- arch/arm64/kernel/cpufeature.c | 2 ++ 4 files changed, 27 insertions(+), 6 deletions(-) -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
[PATCH 2/3] arm64: Add KRYO4XX gold CPU cores to erratum list 1463225 and 1418040
KRYO4XX gold/big CPU core revisions r0p0 to r3p1 are affected by
erratum 1463225 and 1418040, so add them to the respective list.
The variant and revision bits are implementation defined and are
different from the their Cortex CPU counterparts on which they are
based on, i.e., (r0p0 to r3p1) is equivalent to (rcpe to rfpf).
Signed-off-by: Sai Prakash Ranjan
---
Documentation/arm64/silicon-errata.rst | 4
arch/arm64/kernel/cpu_errata.c | 19 +--
2 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/Documentation/arm64/silicon-errata.rst
b/Documentation/arm64/silicon-errata.rst
index 936cf2a59ca4..f3c0c4393e7e 100644
--- a/Documentation/arm64/silicon-errata.rst
+++ b/Documentation/arm64/silicon-errata.rst
@@ -147,6 +147,10 @@ stable kernels.
++-+-+-+
| Qualcomm Tech. | Falkor v{1,2} | E1041 |
QCOM_FALKOR_ERRATUM_1041|
++-+-+-+
+| Qualcomm Tech. | Kryo4xx Gold| N/A | ARM64_ERRATUM_1463225
|
+++-+-+-+
+| Qualcomm Tech. | Kryo4xx Gold| N/A | ARM64_ERRATUM_1418040
|
+++-+-+-+
++-+-+-+
| Fujitsu| A64FX | E#010001| FUJITSU_ERRATUM_010001
|
++-+-+-+
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index cf50c53e9357..044f1d7aebdf 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -472,12 +472,7 @@ static bool
has_cortex_a76_erratum_1463225(const struct arm64_cpu_capabilities *entry,
int scope)
{
- u32 midr = read_cpuid_id();
- /* Cortex-A76 r0p0 - r3p1 */
- struct midr_range range = MIDR_RANGE(MIDR_CORTEX_A76, 0, 0, 3, 1);
-
- WARN_ON(scope != SCOPE_LOCAL_CPU || preemptible());
- return is_midr_in_range(midr, &range) && is_kernel_in_hyp_mode();
+ return is_affected_midr_range_list(entry, scope) &&
is_kernel_in_hyp_mode();
}
#endif
@@ -728,6 +723,8 @@ static const struct midr_range erratum_1418040_list[] = {
MIDR_RANGE(MIDR_CORTEX_A76, 0, 0, 3, 1),
/* Neoverse-N1 r0p0 to r3p1 */
MIDR_RANGE(MIDR_NEOVERSE_N1, 0, 0, 3, 1),
+ /* Kryo4xx Gold (rcpe to rfpf) => (r0p0 to r3p1) */
+ MIDR_RANGE(MIDR_QCOM_KRYO_4XX_GOLD, 0xc, 0xe, 0xf, 0xf),
{},
};
#endif
@@ -777,6 +774,15 @@ static const struct midr_range
erratum_speculative_at_list[] = {
};
#endif
+#ifdef CONFIG_ARM64_ERRATUM_1463225
+static const struct midr_range erratum_1463225[] = {
+ /* Cortex-A76 r0p0 - r3p1 */
+ MIDR_RANGE(MIDR_CORTEX_A76, 0, 0, 3, 1),
+ /* Kryo4xx Gold (rcpe to rfpf) => (r0p0 to r3p1) */
+ MIDR_RANGE(MIDR_QCOM_KRYO_4XX_GOLD, 0xc, 0xe, 0xf, 0xf),
+};
+#endif
+
const struct arm64_cpu_capabilities arm64_errata[] = {
#ifdef CONFIG_ARM64_WORKAROUND_CLEAN_CACHE
{
@@ -916,6 +922,7 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
.capability = ARM64_WORKAROUND_1463225,
.type = ARM64_CPUCAP_LOCAL_CPU_ERRATUM,
.matches = has_cortex_a76_erratum_1463225,
+ .midr_range_list = erratum_1463225,
},
#endif
#ifdef CONFIG_CAVIUM_TX2_ERRATUM_219
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
[PATCH 3/3] arm64: Add KRYO4XX silver CPU cores to erratum list 1530923 and 1024718
KRYO4XX silver/LITTLE CPU cores with revision r1p0 are affected by
erratum 1530923 and 1024718, so add them to the respective list.
The variant and revision bits are implementation defined and are
different from the their Cortex CPU counterparts on which they are
based on, i.e., r1p0 is equivalent to rdpe.
Signed-off-by: Sai Prakash Ranjan
---
Documentation/arm64/silicon-errata.rst | 4
arch/arm64/kernel/cpu_errata.c | 2 ++
arch/arm64/kernel/cpufeature.c | 2 ++
3 files changed, 8 insertions(+)
diff --git a/Documentation/arm64/silicon-errata.rst
b/Documentation/arm64/silicon-errata.rst
index f3c0c4393e7e..3f7c3a7e8a2b 100644
--- a/Documentation/arm64/silicon-errata.rst
+++ b/Documentation/arm64/silicon-errata.rst
@@ -151,6 +151,10 @@ stable kernels.
++-+-+-+
| Qualcomm Tech. | Kryo4xx Gold| N/A | ARM64_ERRATUM_1418040
|
++-+-+-+
+| Qualcomm Tech. | Kryo4xx Silver | N/A | ARM64_ERRATUM_1530923
|
+++-+-+-+
+| Qualcomm Tech. | Kryo4xx Silver | N/A | ARM64_ERRATUM_1024718
|
+++-+-+-+
++-+-+-+
| Fujitsu| A64FX | E#010001| FUJITSU_ERRATUM_010001
|
++-+-+-+
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index 044f1d7aebdf..8e302dc093d0 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -769,6 +769,8 @@ static const struct midr_range
erratum_speculative_at_list[] = {
#ifdef CONFIG_ARM64_ERRATUM_1530923
/* Cortex A55 r0p0 to r2p0 */
MIDR_RANGE(MIDR_CORTEX_A55, 0, 0, 2, 0),
+ /* Kryo4xx Silver (rdpe => r1p0) */
+ MIDR_REV(MIDR_QCOM_KRYO_4XX_SILVER, 0xd, 0xe),
#endif
{},
};
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 9f63053a63a9..9fae0efc80c1 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1408,6 +1408,8 @@ static bool cpu_has_broken_dbm(void)
static const struct midr_range cpus[] = {
#ifdef CONFIG_ARM64_ERRATUM_1024718
MIDR_RANGE(MIDR_CORTEX_A55, 0, 0, 1, 0), // A55 r0p0 -r1p0
+ /* Kryo4xx Silver (rdpe => r1p0) */
+ MIDR_REV(MIDR_QCOM_KRYO_4XX_SILVER, 0xd, 0xe),
#endif
{},
};
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
[PATCH 1/3] arm64: Add MIDR value for KRYO4XX gold CPU cores
Add MIDR value for KRYO4XX gold/big CPU cores which are used in Qualcomm Technologies, Inc. SoCs. This will be used to identify and apply erratum which are applicable for these CPU cores. Signed-off-by: Sai Prakash Ranjan --- arch/arm64/include/asm/cputype.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h index a87a93f67671..7219cddeba66 100644 --- a/arch/arm64/include/asm/cputype.h +++ b/arch/arm64/include/asm/cputype.h @@ -86,6 +86,7 @@ #define QCOM_CPU_PART_FALKOR 0xC00 #define QCOM_CPU_PART_KRYO 0x200 #define QCOM_CPU_PART_KRYO_3XX_SILVER 0x803 +#define QCOM_CPU_PART_KRYO_4XX_GOLD0x804 #define QCOM_CPU_PART_KRYO_4XX_SILVER 0x805 #define NVIDIA_CPU_PART_DENVER 0x003 @@ -114,6 +115,7 @@ #define MIDR_QCOM_FALKOR MIDR_CPU_MODEL(ARM_CPU_IMP_QCOM, QCOM_CPU_PART_FALKOR) #define MIDR_QCOM_KRYO MIDR_CPU_MODEL(ARM_CPU_IMP_QCOM, QCOM_CPU_PART_KRYO) #define MIDR_QCOM_KRYO_3XX_SILVER MIDR_CPU_MODEL(ARM_CPU_IMP_QCOM, QCOM_CPU_PART_KRYO_3XX_SILVER) +#define MIDR_QCOM_KRYO_4XX_GOLD MIDR_CPU_MODEL(ARM_CPU_IMP_QCOM, QCOM_CPU_PART_KRYO_4XX_GOLD) #define MIDR_QCOM_KRYO_4XX_SILVER MIDR_CPU_MODEL(ARM_CPU_IMP_QCOM, QCOM_CPU_PART_KRYO_4XX_SILVER) #define MIDR_NVIDIA_DENVER MIDR_CPU_MODEL(ARM_CPU_IMP_NVIDIA, NVIDIA_CPU_PART_DENVER) #define MIDR_NVIDIA_CARMEL MIDR_CPU_MODEL(ARM_CPU_IMP_NVIDIA, NVIDIA_CPU_PART_CARMEL) -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH] arm64: dts: qcom: sc7180: Drop the unused non-MSA SID
On 2020-06-30 13:49, Sibi Sankar wrote:
Having a non-MSA (Modem Self-Authentication) SID bypassed breaks modem
sandboxing i.e if a transaction were to originate from it, the hardware
memory protections units (XPUs) would fail to flag them (any
transaction
originating from modem are historically termed as an MSA transaction).
Drop the unused non-MSA modem SID on SC7180 SoCs and cheza so that SMMU
continues to block them.
Fixes: bec71ba243e95 ("arm64: dts: qcom: sc7180: Update Q6V5 MSS node")
Fixes: 68aee4af5f620 ("arm64: dts: qcom: sdm845-cheza: Add iommus
property")
Cc: sta...@vger.kernel.org
Reported-by: Sai Prakash Ranjan
Signed-off-by: Sibi Sankar
---
arch/arm64/boot/dts/qcom/sc7180-idp.dts| 2 +-
arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sc7180-idp.dts
b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
index 39dbfc89689e8..141de49a1b7d6 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-idp.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
@@ -312,7 +312,7 @@ &qupv3_id_1 {
&remoteproc_mpss {
status = "okay";
compatible = "qcom,sc7180-mss-pil";
- iommus = <&apps_smmu 0x460 0x1>, <&apps_smmu 0x444 0x3>;
+ iommus = <&apps_smmu 0x461 0x0>, <&apps_smmu 0x444 0x3>;
memory-region = <&mba_mem &mpss_mem>;
};
diff --git a/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
b/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
index 70466cc4b4055..64fc1bfd66fad 100644
--- a/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
@@ -634,7 +634,7 @@ &mdss_mdp {
};
&mss_pil {
- iommus = <&apps_smmu 0x780 0x1>,
+ iommus = <&apps_smmu 0x781 0x0>,
<&apps_smmu 0x724 0x3>;
};
Reviewed-by: Sai Prakash Ranjan
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member
of Code Aurora Forum, hosted by The Linux Foundation
[PATCHv3 4/7] iommu: arm-smmu-impl: Remove unwanted extra blank lines
There are few places in arm-smmu-impl where there are
extra blank lines, remove them and while at it fix the
checkpatch warning for space required before the open
parenthesis.
Signed-off-by: Sai Prakash Ranjan
---
drivers/iommu/arm-smmu-impl.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/iommu/arm-smmu-impl.c b/drivers/iommu/arm-smmu-impl.c
index 309675cf6699..8fbab9c38b61 100644
--- a/drivers/iommu/arm-smmu-impl.c
+++ b/drivers/iommu/arm-smmu-impl.c
@@ -9,10 +9,9 @@
#include "arm-smmu.h"
-
static int arm_smmu_gr0_ns(int offset)
{
- switch(offset) {
+ switch (offset) {
case ARM_SMMU_GR0_sCR0:
case ARM_SMMU_GR0_sACR:
case ARM_SMMU_GR0_sGFSR:
@@ -47,7 +46,6 @@ static const struct arm_smmu_impl calxeda_impl = {
.write_reg = arm_smmu_write_ns,
};
-
struct cavium_smmu {
struct arm_smmu_device smmu;
u32 id_base;
@@ -103,7 +101,6 @@ static struct arm_smmu_device *cavium_smmu_impl_init(struct
arm_smmu_device *smm
return &cs->smmu;
}
-
#define ARM_MMU500_ACTLR_CPRE (1 << 1)
#define ARM_MMU500_ACR_CACHE_LOCK (1 << 26)
@@ -148,7 +145,6 @@ static const struct arm_smmu_impl arm_mmu500_impl = {
.reset = arm_mmu500_reset,
};
-
struct arm_smmu_device *arm_smmu_impl_init(struct arm_smmu_device *smmu)
{
const struct device_node *np = smmu->dev->of_node;
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
[PATCHv3 0/7] System Cache support for GPU and required SMMU support
Some hardware variants contain a system cache or the last level cache(llc). This cache is typically a large block which is shared by multiple clients on the SOC. GPU uses the system cache to cache both the GPU data buffers(like textures) as well the SMMU pagetables. This helps with improved render performance as well as lower power consumption by reducing the bus traffic to the system memory. The system cache architecture allows the cache to be split into slices which then be used by multiple SOC clients. This patch series is an effort to enable and use two of those slices perallocated for the GPU, one for the GPU data buffers and another for the GPU SMMU hardware pagetables. Patch 1 adds a init_context_bank implementation hook to set SCTLR.HUPCF. Patch 2,3,6,7 adds system cache support in SMMU and GPU driver. Patch 4 and 5 are minor cleanups for arm-smmu impl. Changes in v3: * Fix domain attribute setting to before iommu_attach_device() * Fix few code style and checkpatch warnings * Rebase on top of Jordan's latest split pagetables and per-instance pagetables support [1][2] Changes in v2: * Addressed review comments and rebased on top of Jordan's split pagetables series [1] https://lore.kernel.org/patchwork/cover/1264446/ [2] https://lore.kernel.org/patchwork/cover/1264460/ Jordan Crouse (1): iommu/arm-smmu: Add a init_context_bank implementation hook Sai Prakash Ranjan (4): iommu/io-pgtable-arm: Add support to use system cache iommu/arm-smmu: Add domain attribute for system cache iommu: arm-smmu-impl: Remove unwanted extra blank lines iommu: arm-smmu-impl: Convert to use of_match_node() for qcom impl Sharat Masetty (2): drm/msm: rearrange the gpu_rmw() function drm/msm/a6xx: Add support for using system cache(LLC) drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 82 + drivers/gpu/drm/msm/adreno/a6xx_gpu.h | 3 + drivers/gpu/drm/msm/adreno/adreno_gpu.c | 23 ++- drivers/gpu/drm/msm/msm_drv.c | 8 +++ drivers/gpu/drm/msm/msm_drv.h | 1 + drivers/gpu/drm/msm/msm_gpu.h | 5 +- drivers/gpu/drm/msm/msm_iommu.c | 3 + drivers/gpu/drm/msm/msm_mmu.h | 4 ++ drivers/iommu/arm-smmu-impl.c | 13 ++-- drivers/iommu/arm-smmu-qcom.c | 13 drivers/iommu/arm-smmu.c| 46 +- drivers/iommu/arm-smmu.h| 13 drivers/iommu/io-pgtable-arm.c | 7 ++- include/linux/io-pgtable.h | 4 ++ include/linux/iommu.h | 1 + 15 files changed, 198 insertions(+), 28 deletions(-) -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
[PATCHv3 5/7] iommu: arm-smmu-impl: Convert to use of_match_node() for qcom impl
Use of_match_node() to match qcom implementation instead
of multiple of_device_compatible() calls for each qcom
implementation.
Signed-off-by: Sai Prakash Ranjan
---
drivers/iommu/arm-smmu-impl.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/arm-smmu-impl.c b/drivers/iommu/arm-smmu-impl.c
index 8fbab9c38b61..42020d50ce12 100644
--- a/drivers/iommu/arm-smmu-impl.c
+++ b/drivers/iommu/arm-smmu-impl.c
@@ -9,6 +9,12 @@
#include "arm-smmu.h"
+static const struct of_device_id __maybe_unused qcom_smmu_impl_of_match[] = {
+ { .compatible = "qcom,sc7180-smmu-500" },
+ { .compatible = "qcom,sdm845-smmu-500" },
+ { }
+};
+
static int arm_smmu_gr0_ns(int offset)
{
switch (offset) {
@@ -168,8 +174,7 @@ struct arm_smmu_device *arm_smmu_impl_init(struct
arm_smmu_device *smmu)
if (of_property_read_bool(np, "calxeda,smmu-secure-config-access"))
smmu->impl = &calxeda_impl;
- if (of_device_is_compatible(np, "qcom,sdm845-smmu-500") ||
- of_device_is_compatible(np, "qcom,sc7180-smmu-500"))
+ if (of_match_node(qcom_smmu_impl_of_match, np))
return qcom_smmu_impl_init(smmu);
if (of_device_is_compatible(smmu->dev->of_node, "qcom,adreno-smmu"))
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
[PATCHv3 7/7] drm/msm/a6xx: Add support for using system cache(LLC)
From: Sharat Masetty
The last level system cache can be partitioned to 32 different
slices of which GPU has two slices preallocated. One slice is
used for caching GPU buffers and the other slice is used for
caching the GPU SMMU pagetables. This talks to the core system
cache driver to acquire the slice handles, configure the SCID's
to those slices and activates and deactivates the slices upon
GPU power collapse and restore.
Some support from the IOMMU driver is also needed to make use
of the system cache. IOMMU_SYS_CACHE_ONLY is a buffer protection
flag which enables caching GPU data buffers in the system cache
with memory attributes such as outer cacheable, read-allocate,
write-allocate for buffers. The GPU then has the ability to
override a few cacheability parameters which it does to override
write-allocate to write-no-allocate as the GPU hardware does not
benefit much from it.
Similarly DOMAIN_ATTR_SYS_CACHE is another domain level attribute
used by the IOMMU driver to set the right attributes to cache the
hardware pagetables into the system cache.
Signed-off-by: Sharat Masetty
(sai: fix to set attr before device attach to IOMMU and rebase)
Signed-off-by: Sai Prakash Ranjan
---
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 82 +
drivers/gpu/drm/msm/adreno/a6xx_gpu.h | 3 +
drivers/gpu/drm/msm/adreno/adreno_gpu.c | 23 ++-
drivers/gpu/drm/msm/msm_iommu.c | 3 +
drivers/gpu/drm/msm/msm_mmu.h | 4 ++
5 files changed, 114 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 6bee70853ea8..c33cd2a588e6 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -9,6 +9,8 @@
#include "a6xx_gmu.xml.h"
#include
+#include
+#include
#define GPU_PAS_ID 13
@@ -808,6 +810,79 @@ static const u32
a6xx_register_offsets[REG_ADRENO_REGISTER_MAX] = {
REG_ADRENO_DEFINE(REG_ADRENO_CP_RB_CNTL, REG_A6XX_CP_RB_CNTL),
};
+static void a6xx_llc_rmw(struct a6xx_gpu *a6xx_gpu, u32 reg, u32 mask, u32 or)
+{
+ return msm_rmw(a6xx_gpu->llc_mmio + (reg << 2), mask, or);
+}
+
+static void a6xx_llc_write(struct a6xx_gpu *a6xx_gpu, u32 reg, u32 value)
+{
+ return msm_writel(value, a6xx_gpu->llc_mmio + (reg << 2));
+}
+
+static void a6xx_llc_deactivate(struct a6xx_gpu *a6xx_gpu)
+{
+ llcc_slice_deactivate(a6xx_gpu->llc_slice);
+ llcc_slice_deactivate(a6xx_gpu->htw_llc_slice);
+}
+
+static void a6xx_llc_activate(struct a6xx_gpu *a6xx_gpu)
+{
+ u32 cntl1_regval = 0;
+
+ if (IS_ERR(a6xx_gpu->llc_mmio))
+ return;
+
+ if (!llcc_slice_activate(a6xx_gpu->llc_slice)) {
+ u32 gpu_scid = llcc_get_slice_id(a6xx_gpu->llc_slice);
+
+ gpu_scid &= 0x1f;
+ cntl1_regval = (gpu_scid << 0) | (gpu_scid << 5) | (gpu_scid <<
10) |
+ (gpu_scid << 15) | (gpu_scid << 20);
+ }
+
+ if (!llcc_slice_activate(a6xx_gpu->htw_llc_slice)) {
+ u32 gpuhtw_scid = llcc_get_slice_id(a6xx_gpu->htw_llc_slice);
+
+ gpuhtw_scid &= 0x1f;
+ cntl1_regval |= FIELD_PREP(GENMASK(29, 25), gpuhtw_scid);
+ }
+
+ if (cntl1_regval) {
+ /*
+* Program the slice IDs for the various GPU blocks and GPU MMU
+* pagetables
+*/
+ a6xx_llc_write(a6xx_gpu, REG_A6XX_CX_MISC_SYSTEM_CACHE_CNTL_1,
cntl1_regval);
+
+ /*
+* Program cacheability overrides to not allocate cache lines on
+* a write miss
+*/
+ a6xx_llc_rmw(a6xx_gpu, REG_A6XX_CX_MISC_SYSTEM_CACHE_CNTL_0,
0xF, 0x03);
+ }
+}
+
+static void a6xx_llc_slices_destroy(struct a6xx_gpu *a6xx_gpu)
+{
+ llcc_slice_putd(a6xx_gpu->llc_slice);
+ llcc_slice_putd(a6xx_gpu->htw_llc_slice);
+}
+
+static void a6xx_llc_slices_init(struct platform_device *pdev,
+ struct a6xx_gpu *a6xx_gpu)
+{
+ a6xx_gpu->llc_mmio = msm_ioremap(pdev, "cx_mem", "gpu_cx");
+ if (IS_ERR(a6xx_gpu->llc_mmio))
+ return;
+
+ a6xx_gpu->llc_slice = llcc_slice_getd(LLCC_GPU);
+ a6xx_gpu->htw_llc_slice = llcc_slice_getd(LLCC_GPUHTW);
+
+ if (IS_ERR(a6xx_gpu->llc_slice) && IS_ERR(a6xx_gpu->htw_llc_slice))
+ a6xx_gpu->llc_mmio = ERR_PTR(-EINVAL);
+}
+
static int a6xx_pm_resume(struct msm_gpu *gpu)
{
struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
@@ -822,6 +897,8 @@ static int a6xx_pm_resume(struct msm_gpu *gpu)
msm_gpu_resume_devfreq(gpu);
+ a6xx_llc_activate(a6xx_gpu);
+
return 0;
}
@@ -830,6 +907,8 @@ static int a6xx_pm_suspend(struct msm_gpu *gpu)
stru
[PATCHv3 1/7] iommu/arm-smmu: Add a init_context_bank implementation hook
From: Jordan Crouse
Add a new implementation hook to allow the implementation specific code
to tweek the context bank configuration just before it gets written.
The first user will be the Adreno GPU implementation to turn on
SCTLR.HUPCF to ensure that a page fault doesn't terminating pending
transactions. Doing so could hang the GPU if one of the terminated
transactions is a CP read.
This depends on the arm-smmu adreno SMMU implementation [1].
[1] https://lore.kernel.org/patchwork/patch/1264452/
Signed-off-by: Jordan Crouse
Signed-off-by: Sai Prakash Ranjan
---
drivers/iommu/arm-smmu-qcom.c | 13 +
drivers/iommu/arm-smmu.c | 29 +
drivers/iommu/arm-smmu.h | 12
3 files changed, 38 insertions(+), 16 deletions(-)
diff --git a/drivers/iommu/arm-smmu-qcom.c b/drivers/iommu/arm-smmu-qcom.c
index cb2acb6b19dd..6462fb00f493 100644
--- a/drivers/iommu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm-smmu-qcom.c
@@ -17,6 +17,18 @@ static bool qcom_adreno_smmu_is_gpu_device(struct
arm_smmu_domain *smmu_domain)
return of_device_is_compatible(smmu_domain->dev->of_node,
"qcom,adreno");
}
+static void qcom_adreno_smmu_init_context_bank(struct arm_smmu_domain
*smmu_domain,
+ struct arm_smmu_cb *cb)
+{
+ /*
+* On the GPU device we want to process subsequent transactions after a
+* fault to keep the GPU from hanging
+*/
+
+ if (qcom_adreno_smmu_is_gpu_device(smmu_domain))
+ cb->sctlr |= ARM_SMMU_SCTLR_HUPCF;
+}
+
static int qcom_adreno_smmu_init_context(struct arm_smmu_domain *smmu_domain,
struct io_pgtable_cfg *pgtbl_cfg)
{
@@ -92,6 +104,7 @@ static const struct arm_smmu_impl qcom_adreno_smmu_impl = {
.init_context = qcom_adreno_smmu_init_context,
.def_domain_type = qcom_smmu_def_domain_type,
.reset = qcom_smmu500_reset,
+ .init_context_bank = qcom_adreno_smmu_init_context_bank,
};
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 4bd247dfd703..b2564f93d685 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -86,14 +86,6 @@ struct arm_smmu_smr {
boolvalid;
};
-struct arm_smmu_cb {
- u64 ttbr[2];
- u32 tcr[2];
- u32 mair[2];
- struct arm_smmu_cfg *cfg;
- atomic_taux;
-};
-
struct arm_smmu_master_cfg {
struct arm_smmu_device *smmu;
s16 smendx[];
@@ -580,6 +572,18 @@ static void arm_smmu_init_context_bank(struct
arm_smmu_domain *smmu_domain,
cb->mair[1] = pgtbl_cfg->arm_lpae_s1_cfg.mair >> 32;
}
}
+
+ cb->sctlr = ARM_SMMU_SCTLR_CFIE | ARM_SMMU_SCTLR_CFRE |
ARM_SMMU_SCTLR_AFE |
+ ARM_SMMU_SCTLR_TRE | ARM_SMMU_SCTLR_M;
+
+ if (stage1)
+ cb->sctlr |= ARM_SMMU_SCTLR_S1_ASIDPNE;
+ if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN))
+ cb->sctlr |= ARM_SMMU_SCTLR_E;
+
+ /* Give the implementation a chance to adjust the configuration */
+ if (smmu_domain->smmu->impl &&
smmu_domain->smmu->impl->init_context_bank)
+ smmu_domain->smmu->impl->init_context_bank(smmu_domain, cb);
}
static void arm_smmu_write_context_bank(struct arm_smmu_device *smmu, int idx)
@@ -658,14 +662,7 @@ static void arm_smmu_write_context_bank(struct
arm_smmu_device *smmu, int idx)
}
/* SCTLR */
- reg = ARM_SMMU_SCTLR_CFIE | ARM_SMMU_SCTLR_CFRE | ARM_SMMU_SCTLR_AFE |
- ARM_SMMU_SCTLR_TRE | ARM_SMMU_SCTLR_M;
- if (stage1)
- reg |= ARM_SMMU_SCTLR_S1_ASIDPNE;
- if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN))
- reg |= ARM_SMMU_SCTLR_E;
-
- arm_smmu_cb_write(smmu, idx, ARM_SMMU_CB_SCTLR, reg);
+ arm_smmu_cb_write(smmu, idx, ARM_SMMU_CB_SCTLR, cb->sctlr);
}
/*
diff --git a/drivers/iommu/arm-smmu.h b/drivers/iommu/arm-smmu.h
index 79d441024043..4a335ef3d97a 100644
--- a/drivers/iommu/arm-smmu.h
+++ b/drivers/iommu/arm-smmu.h
@@ -142,6 +142,7 @@ enum arm_smmu_cbar_type {
#define ARM_SMMU_CB_SCTLR 0x0
#define ARM_SMMU_SCTLR_S1_ASIDPNE BIT(12)
+#define ARM_SMMU_SCTLR_HUPCF BIT(8)
#define ARM_SMMU_SCTLR_CFCFG BIT(7)
#define ARM_SMMU_SCTLR_CFIEBIT(6)
#define ARM_SMMU_SCTLR_CFREBIT(5)
@@ -349,6 +350,15 @@ struct arm_smmu_domain {
boolaux;
};
+struct arm_smmu_cb {
+ u64 ttbr[2];
+ u32 tcr[2];
+ u32 mair[2];
+ u32 sctlr;
+ struct arm_smmu_cfg *cfg;
+ atomic_taux;
+};
+
st
[PATCHv3 3/7] iommu/arm-smmu: Add domain attribute for system cache
Add iommu domain attribute for using system cache aka last level
cache by client drivers like GPU to set right attributes for caching
the hardware pagetables into the system cache.
Signed-off-by: Sai Prakash Ranjan
---
drivers/iommu/arm-smmu.c | 17 +
drivers/iommu/arm-smmu.h | 1 +
include/linux/iommu.h| 1 +
3 files changed, 19 insertions(+)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index b2564f93d685..71b6f7038423 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -897,6 +897,9 @@ static int arm_smmu_init_domain_context(struct iommu_domain
*domain,
goto out_unlock;
}
+ if (smmu_domain->sys_cache)
+ pgtbl_cfg.quirks |= IO_PGTABLE_QUIRK_SYS_CACHE;
+
if (smmu_domain->non_strict)
pgtbl_cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT;
@@ -1732,6 +1735,9 @@ static int arm_smmu_domain_get_attr(struct iommu_domain
*domain,
case DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE:
*(int *)data = smmu_domain->non_strict;
return 0;
+ case DOMAIN_ATTR_SYS_CACHE:
+ *((int *)data) = smmu_domain->sys_cache;
+ return 0;
default:
return -ENODEV;
}
@@ -1763,6 +1769,17 @@ static int arm_smmu_domain_set_attr(struct iommu_domain
*domain,
else
smmu_domain->stage = ARM_SMMU_DOMAIN_S1;
break;
+ case DOMAIN_ATTR_SYS_CACHE:
+ if (smmu_domain->smmu) {
+ ret = -EPERM;
+ goto out_unlock;
+ }
+
+ if (*((int *)data))
+ smmu_domain->sys_cache = true;
+ else
+ smmu_domain->sys_cache = false;
+ break;
default:
ret = -ENODEV;
}
diff --git a/drivers/iommu/arm-smmu.h b/drivers/iommu/arm-smmu.h
index 4a335ef3d97a..bbae48bdc022 100644
--- a/drivers/iommu/arm-smmu.h
+++ b/drivers/iommu/arm-smmu.h
@@ -348,6 +348,7 @@ struct arm_smmu_domain {
struct iommu_domain domain;
struct device *dev; /* Device attached to this
domain */
boolaux;
+ boolsys_cache;
};
struct arm_smmu_cb {
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 2388117641f1..a48edbebe3cb 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -125,6 +125,7 @@ enum iommu_attr {
DOMAIN_ATTR_NESTING,/* two stages of translation */
DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE,
DOMAIN_ATTR_PGTABLE_CFG,
+ DOMAIN_ATTR_SYS_CACHE,
DOMAIN_ATTR_MAX,
};
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
[PATCHv3 6/7] drm/msm: rearrange the gpu_rmw() function
From: Sharat Masetty
The register read-modify-write construct is generic enough
that it can be used by other subsystems as needed, create
a more generic rmw() function and have the gpu_rmw() use
this new function.
Signed-off-by: Sharat Masetty
Reviewed-by: Jordan Crouse
Signed-off-by: Sai Prakash Ranjan
---
drivers/gpu/drm/msm/msm_drv.c | 8
drivers/gpu/drm/msm/msm_drv.h | 1 +
drivers/gpu/drm/msm/msm_gpu.h | 5 +
3 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 0c219b954943..5aa070929220 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -166,6 +166,14 @@ u32 msm_readl(const void __iomem *addr)
return val;
}
+void msm_rmw(void __iomem *addr, u32 mask, u32 or)
+{
+ u32 val = msm_readl(addr);
+
+ val &= ~mask;
+ msm_writel(val | or, addr);
+}
+
struct msm_vblank_work {
struct work_struct work;
int crtc_id;
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index 983a8b7e5a74..5bb02ccb863a 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -417,6 +417,7 @@ void __iomem *msm_ioremap(struct platform_device *pdev,
const char *name,
const char *dbgname);
void msm_writel(u32 data, void __iomem *addr);
u32 msm_readl(const void __iomem *addr);
+void msm_rmw(void __iomem *addr, u32 mask, u32 or);
struct msm_gpu_submitqueue;
int msm_submitqueue_init(struct drm_device *drm, struct msm_file_private *ctx);
diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
index f1762b77bea8..3519777c8cb2 100644
--- a/drivers/gpu/drm/msm/msm_gpu.h
+++ b/drivers/gpu/drm/msm/msm_gpu.h
@@ -232,10 +232,7 @@ static inline u32 gpu_read(struct msm_gpu *gpu, u32 reg)
static inline void gpu_rmw(struct msm_gpu *gpu, u32 reg, u32 mask, u32 or)
{
- uint32_t val = gpu_read(gpu, reg);
-
- val &= ~mask;
- gpu_write(gpu, reg, val | or);
+ msm_rmw(gpu->mmio + (reg << 2), mask, or);
}
static inline u64 gpu_read64(struct msm_gpu *gpu, u32 lo, u32 hi)
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
[PATCHv3 2/7] iommu/io-pgtable-arm: Add support to use system cache
Add a quirk IO_PGTABLE_QUIRK_SYS_CACHE to override the
attributes set in TCR for the page table walker when
using system cache.
Signed-off-by: Sai Prakash Ranjan
---
drivers/iommu/io-pgtable-arm.c | 7 ++-
include/linux/io-pgtable.h | 4
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
index 04fbd4bf0ff9..0a6cb82fd98a 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c
@@ -792,7 +792,8 @@ arm_64_lpae_alloc_pgtable_s1(struct io_pgtable_cfg *cfg,
void *cookie)
if (cfg->quirks & ~(IO_PGTABLE_QUIRK_ARM_NS |
IO_PGTABLE_QUIRK_NON_STRICT |
- IO_PGTABLE_QUIRK_ARM_TTBR1))
+ IO_PGTABLE_QUIRK_ARM_TTBR1 |
+ IO_PGTABLE_QUIRK_SYS_CACHE))
return NULL;
data = arm_lpae_alloc_pgtable(cfg);
@@ -804,6 +805,10 @@ arm_64_lpae_alloc_pgtable_s1(struct io_pgtable_cfg *cfg,
void *cookie)
tcr->sh = ARM_LPAE_TCR_SH_IS;
tcr->irgn = ARM_LPAE_TCR_RGN_WBWA;
tcr->orgn = ARM_LPAE_TCR_RGN_WBWA;
+ } else if (cfg->quirks & IO_PGTABLE_QUIRK_SYS_CACHE) {
+ tcr->sh = ARM_LPAE_TCR_SH_OS;
+ tcr->irgn = ARM_LPAE_TCR_RGN_NC;
+ tcr->orgn = ARM_LPAE_TCR_RGN_WBWA;
} else {
tcr->sh = ARM_LPAE_TCR_SH_OS;
tcr->irgn = ARM_LPAE_TCR_RGN_NC;
diff --git a/include/linux/io-pgtable.h b/include/linux/io-pgtable.h
index bbed1d3925ba..23114b3fe2a5 100644
--- a/include/linux/io-pgtable.h
+++ b/include/linux/io-pgtable.h
@@ -86,6 +86,9 @@ struct io_pgtable_cfg {
*
* IO_PGTABLE_QUIRK_ARM_TTBR1: (ARM LPAE format) Configure the table
* for use in the upper half of a split address space.
+*
+* IO_PGTABLE_QUIRK_SYS_CACHE: Override the attributes set in TCR for
+* the page table walker when using system cache.
*/
#define IO_PGTABLE_QUIRK_ARM_NS BIT(0)
#define IO_PGTABLE_QUIRK_NO_PERMS BIT(1)
@@ -93,6 +96,7 @@ struct io_pgtable_cfg {
#define IO_PGTABLE_QUIRK_ARM_MTK_EXTBIT(3)
#define IO_PGTABLE_QUIRK_NON_STRICT BIT(4)
#define IO_PGTABLE_QUIRK_ARM_TTBR1 BIT(5)
+ #define IO_PGTABLE_QUIRK_SYS_CACHE BIT(6)
unsigned long quirks;
unsigned long pgsize_bitmap;
unsigned intias;
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCHv3 0/2] Convert QCOM watchdog timer bindings to YAML
On 2020-06-26 02:48, Guenter Roeck wrote: On Fri, Jun 26, 2020 at 12:52:31AM +0530, Sai Prakash Ranjan wrote: > > > I don't think the watchdog mailing list has been copied on this series, > meaning I don't have a copy that I could apply if I wanted to. I kept you in CC for all the revisions but missed adding watchdog list. Will resend with the appropriate lists added. I use patchwork to track patches, tags, and my responses. No patchwork, no patch, no tags, and no tracking. Now resent with watchdog list added - https://lore.kernel.org/patchwork/cover/1263944/ Thanks, Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
[RESEND PATCHv3 1/2] dt-bindings: watchdog: Convert QCOM watchdog timer bindings to YAML
Convert QCOM watchdog timer bindings to DT schema format using
json-schema.
Signed-off-by: Sai Prakash Ranjan
Reviewed-by: Stephen Boyd
Reviewed-by: Rob Herring
---
.../devicetree/bindings/watchdog/qcom-wdt.txt | 28
.../bindings/watchdog/qcom-wdt.yaml | 44 +++
2 files changed, 44 insertions(+), 28 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/watchdog/qcom-wdt.txt
create mode 100644 Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml
diff --git a/Documentation/devicetree/bindings/watchdog/qcom-wdt.txt
b/Documentation/devicetree/bindings/watchdog/qcom-wdt.txt
deleted file mode 100644
index 41aeaa2ff0f8..
--- a/Documentation/devicetree/bindings/watchdog/qcom-wdt.txt
+++ /dev/null
@@ -1,28 +0,0 @@
-Qualcomm Krait Processor Sub-system (KPSS) Watchdog
-
-Required properties :
-- compatible : shall contain only one of the following:
-
- "qcom,kpss-wdt-msm8960"
- "qcom,kpss-wdt-apq8064"
- "qcom,kpss-wdt-ipq8064"
- "qcom,kpss-wdt-ipq4019"
- "qcom,kpss-timer"
- "qcom,scss-timer"
- "qcom,kpss-wdt"
-
-- reg : shall contain base register location and length
-- clocks : shall contain the input clock
-
-Optional properties :
-- timeout-sec : shall contain the default watchdog timeout in seconds,
-if unset, the default timeout is 30 seconds
-
-Example:
- watchdog@208a038 {
- compatible = "qcom,kpss-wdt-ipq8064";
- reg = <0x0208a038 0x40>;
- clocks = <&sleep_clk>;
- timeout-sec = <10>;
- };
diff --git a/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml
b/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml
new file mode 100644
index ..5448cc537a03
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml
@@ -0,0 +1,44 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/watchdog/qcom-wdt.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Krait Processor Sub-system (KPSS) Watchdog timer
+
+maintainers:
+ - Sai Prakash Ranjan
+
+allOf:
+ - $ref: watchdog.yaml#
+
+properties:
+ compatible:
+enum:
+ - qcom,kpss-timer
+ - qcom,kpss-wdt
+ - qcom,kpss-wdt-apq8064
+ - qcom,kpss-wdt-ipq4019
+ - qcom,kpss-wdt-ipq8064
+ - qcom,kpss-wdt-msm8960
+ - qcom,scss-timer
+
+ reg:
+maxItems: 1
+
+ clocks:
+maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - clocks
+
+examples:
+ - |
+watchdog@208a038 {
+ compatible = "qcom,kpss-wdt-ipq8064";
+ reg = <0x0208a038 0x40>;
+ clocks = <&sleep_clk>;
+ timeout-sec = <10>;
+};
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
[RESEND PATCHv3 2/2] dt-bindings: watchdog: Add compatible for QCS404, SC7180, SDM845, SM8150
Add missing compatible for watchdog timer on QCS404, SC7180, SDM845 and SM8150 SoCs. Signed-off-by: Sai Prakash Ranjan Reviewed-by: Stephen Boyd Acked-by: Rob Herring --- Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml | 4 1 file changed, 4 insertions(+) diff --git a/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml b/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml index 5448cc537a03..0709ddf0b6a5 100644 --- a/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml +++ b/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml @@ -15,6 +15,10 @@ allOf: properties: compatible: enum: + - qcom,apss-wdt-qcs404 + - qcom,apss-wdt-sc7180 + - qcom,apss-wdt-sdm845 + - qcom,apss-wdt-sm8150 - qcom,kpss-timer - qcom,kpss-wdt - qcom,kpss-wdt-apq8064 -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
[RESEND PATCHv3 0/2] Convert QCOM watchdog timer bindings to YAML
This series converts QCOM watchdog timer bindings to YAML. Also it adds the missing SoC-specific compatible for QCS404, SC7180, SDM845 and SM8150 SoCs. v1: https://lore.kernel.org/lkml/cover.1576211720.git.saiprakash.ran...@codeaurora.org/ v2: https://lore.kernel.org/lkml/cover.1580570160.git.saiprakash.ran...@codeaurora.org/ Resending this series with watchdog mailing list added. Changes since v2: * Add missing compatibles to enum. Changes since v1: As per Rob's suggestion: * Replaced oneOf+const with enum. * Removed timeout-sec and included watchdog.yaml. * Removed repeated use of const:qcom,kpss-wdt and made use of enum. Sai Prakash Ranjan (2): dt-bindings: watchdog: Convert QCOM watchdog timer bindings to YAML dt-bindings: watchdog: Add compatible for QCS404, SC7180, SDM845, SM8150 .../devicetree/bindings/watchdog/qcom-wdt.txt | 28 --- .../bindings/watchdog/qcom-wdt.yaml | 48 +++ 2 files changed, 48 insertions(+), 28 deletions(-) delete mode 100644 Documentation/devicetree/bindings/watchdog/qcom-wdt.txt create mode 100644 Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCHv3 0/2] Convert QCOM watchdog timer bindings to YAML
On 2020-06-25 21:30, Guenter Roeck wrote: On Mon, Jun 22, 2020 at 11:50:52AM +0530, Sai Prakash Ranjan wrote: On 2020-06-21 13:03, Bjorn Andersson wrote: > On Tue 16 Jun 23:56 PDT 2020, Sai Prakash Ranjan wrote: > > > Hi Bjorn, > > > > Hi Sai, > > > On 2020-02-12 03:54, Sai Prakash Ranjan wrote: > > > This series converts QCOM watchdog timer bindings to YAML. Also > > > it adds the missing SoC-specific compatible for QCS404, SC7180, > > > SDM845 and SM8150 SoCs. > > > > > > v1: > > > https://lore.kernel.org/lkml/cover.1576211720.git.saiprakash.ran...@codeaurora.org/ > > > v2: > > > https://lore.kernel.org/lkml/cover.1580570160.git.saiprakash.ran...@codeaurora.org/ > > > > > > Changes since v2: > > > * Add missing compatibles to enum. > > > > > > Changes since v1: > > > As per Rob's suggestion: > > > * Replaced oneOf+const with enum. > > > * Removed timeout-sec and included watchdog.yaml. > > > * Removed repeated use of const:qcom,kpss-wdt and made use of enum. > > > > > > Sai Prakash Ranjan (2): > > > dt-bindings: watchdog: Convert QCOM watchdog timer bindings to YAML > > > dt-bindings: watchdog: Add compatible for QCS404, SC7180, SDM845, > > > SM8150 > > > > > > .../devicetree/bindings/watchdog/qcom-wdt.txt | 28 --- > > > .../bindings/watchdog/qcom-wdt.yaml | 48 +++ > > > 2 files changed, 48 insertions(+), 28 deletions(-) > > > delete mode 100644 > > > Documentation/devicetree/bindings/watchdog/qcom-wdt.txt > > > create mode 100644 > > > Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml > > > > > > Gentle ping! > > > > This should better go through the watchdog tree, so I believe Guenter > would be the one to pick this up. > Ah right, then a gentle ping for Guenter. I don't think the watchdog mailing list has been copied on this series, meaning I don't have a copy that I could apply if I wanted to. I kept you in CC for all the revisions but missed adding watchdog list. Will resend with the appropriate lists added. I also see no evidence for a Reviewed-by: tag from Rob or any other DT maintainer. Not sure why you think I would add reviewed-by tag from Rob without him giving one. But here's the evidence - https://lore.kernel.org/lkml/cover.1580570160.git.saiprakash.ran...@codeaurora.org/ This link is also given v3 cover letter. Thanks, Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
[PATCH] arm64: Add KRYO{3,4}XX silver CPU cores to SSB safelist
QCOM KRYO{3,4}XX silver/LITTLE CPU cores are based on
Cortex-A55 and are SSB safe, hence add them to SSB
safelist -> arm64_ssb_cpus[].
Reported-by: Stephen Boyd
Signed-off-by: Sai Prakash Ranjan
---
arch/arm64/kernel/cpu_errata.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index ad06d6802d2e..cf50c53e9357 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -460,6 +460,8 @@ static const struct midr_range arm64_ssb_cpus[] = {
MIDR_ALL_VERSIONS(MIDR_CORTEX_A53),
MIDR_ALL_VERSIONS(MIDR_CORTEX_A55),
MIDR_ALL_VERSIONS(MIDR_BRAHMA_B53),
+ MIDR_ALL_VERSIONS(MIDR_QCOM_KRYO_3XX_SILVER),
+ MIDR_ALL_VERSIONS(MIDR_QCOM_KRYO_4XX_SILVER),
{},
};
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
[PATCH] arm64: kpti: Add KRYO{3,4}XX silver CPU cores to kpti safelist
QCOM KRYO{3,4}XX silver/LITTLE CPU cores are based on Cortex-A55
and are meltdown safe, hence add them to kpti_safe_list[].
Signed-off-by: Sai Prakash Ranjan
---
arch/arm64/kernel/cpufeature.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 4ae41670c2e6..9f63053a63a9 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1290,6 +1290,8 @@ static bool unmap_kernel_at_el0(const struct
arm64_cpu_capabilities *entry,
MIDR_ALL_VERSIONS(MIDR_CORTEX_A73),
MIDR_ALL_VERSIONS(MIDR_HISI_TSV110),
MIDR_ALL_VERSIONS(MIDR_NVIDIA_CARMEL),
+ MIDR_ALL_VERSIONS(MIDR_QCOM_KRYO_3XX_SILVER),
+ MIDR_ALL_VERSIONS(MIDR_QCOM_KRYO_4XX_SILVER),
{ /* sentinel */ }
};
char const *str = "kpti command line option";
base-commit: cfafde3c949cae39483639c03c5da5fd91bb234e
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH 2/4] arm64: dts: qcom: sc7180: Add iommus property to ETR
Hi Bjorn, On 2020-06-21 13:39, Sai Prakash Ranjan wrote: Hi Bjorn, On 2020-06-21 12:52, Bjorn Andersson wrote: On Tue 09 Jun 06:30 PDT 2020, Sai Prakash Ranjan wrote: Define iommus property for Coresight ETR component in SC7180 SoC with the SID and mask to enable SMMU translation for this master. We don't have &apps_smmu in linux-next, as we've yet to figure out how to disable the boot splash or support the stream mapping handover. So I'm not able to apply this. This is for SC7180 which has apps_smmu not SM8150. Please let me know if this needs further explanation. Thanks, Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCHv3] coresight: tmc: Add shutdown callback for TMC ETR
On 2020-06-23 22:55, Mathieu Poirier wrote: On Tue, Jun 16, 2020 at 10:26:23AM +0530, Sai Prakash Ranjan wrote: Implement a shutdown callback to ensure ETR hardware is properly shutdown in reboot/shutdown path. This is required for ETR which has SMMU address translation enabled like on SC7180 SoC and few others. If the hardware is still accessing memory after SMMU translation is disabled as part of SMMU shutdown callback in system reboot or shutdown path, then IOVAs(I/O virtual address) which it was using will go on the bus as the physical addresses which might result in unknown crashes (NoC/interconnect errors). So we make sure from this shutdown callback that the ETR is shutdown before SMMU translation is disabled and device_link in SMMU driver will take care of ordering of shutdown callbacks such that SMMU shutdown callback is not called before any of its consumer shutdown callbacks. Signed-off-by: Sai Prakash Ranjan --- I have applied your patch. Thanks Mathieu. -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH] venus: core: add shutdown callback for venus
Hi Mansur,
On 2020-06-13 16:03, Mansur Alisha Shaik wrote:
After the SMMU translation is disabled in the
arm-smmu shutdown callback during reboot, if
any subsystem are still alive then IOVAs they
are using will become PAs on bus, which may
lead to crash.
Below are the consumers of smmu from venus
arm-smmu: consumer: aa0.video-codec supplier=1500.iommu
arm-smmu: consumer: video-firmware.0 supplier=1500.iommu
So implemented shutdown callback, which detach iommu maps.
Change-Id: I0f0f331056e0b84b92f1d86f66618d4b1caaa24a
Signed-off-by: Mansur Alisha Shaik
---
drivers/media/platform/qcom/venus/core.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/media/platform/qcom/venus/core.c
b/drivers/media/platform/qcom/venus/core.c
index 30d4b9e..acf798c 100644
--- a/drivers/media/platform/qcom/venus/core.c
+++ b/drivers/media/platform/qcom/venus/core.c
@@ -371,6 +371,14 @@ static int venus_remove(struct platform_device
*pdev)
return ret;
}
+static void venus_core_shutdown(struct platform_device *pdev)
+{
+ int ret;
+
+ ret = venus_remove(pdev);
+ WARN_ON(ret < 0);
I don't think you should warn here, its shutdown path and you can't
do anything with this WARN unlike remove callback where you have
to be sure to cleanup properly so that you are able to reload module.
But if you still want a hint about this failure, then just add a
dev_err()
to indicate the failure instead of a big stack trace spamming kernel
log.
Thanks,
Sai
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member
of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCHv3 0/2] Convert QCOM watchdog timer bindings to YAML
On 2020-06-21 13:03, Bjorn Andersson wrote: On Tue 16 Jun 23:56 PDT 2020, Sai Prakash Ranjan wrote: Hi Bjorn, Hi Sai, On 2020-02-12 03:54, Sai Prakash Ranjan wrote: > This series converts QCOM watchdog timer bindings to YAML. Also > it adds the missing SoC-specific compatible for QCS404, SC7180, > SDM845 and SM8150 SoCs. > > v1: > https://lore.kernel.org/lkml/cover.1576211720.git.saiprakash.ran...@codeaurora.org/ > v2: > https://lore.kernel.org/lkml/cover.1580570160.git.saiprakash.ran...@codeaurora.org/ > > Changes since v2: > * Add missing compatibles to enum. > > Changes since v1: > As per Rob's suggestion: > * Replaced oneOf+const with enum. > * Removed timeout-sec and included watchdog.yaml. > * Removed repeated use of const:qcom,kpss-wdt and made use of enum. > > Sai Prakash Ranjan (2): > dt-bindings: watchdog: Convert QCOM watchdog timer bindings to YAML > dt-bindings: watchdog: Add compatible for QCS404, SC7180, SDM845, > SM8150 > > .../devicetree/bindings/watchdog/qcom-wdt.txt | 28 --- > .../bindings/watchdog/qcom-wdt.yaml | 48 +++ > 2 files changed, 48 insertions(+), 28 deletions(-) > delete mode 100644 > Documentation/devicetree/bindings/watchdog/qcom-wdt.txt > create mode 100644 > Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml Gentle ping! This should better go through the watchdog tree, so I believe Guenter would be the one to pick this up. Ah right, then a gentle ping for Guenter. Thanks, Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH 2/4] arm64: dts: qcom: sc7180: Add iommus property to ETR
Hi Bjorn, On 2020-06-21 12:52, Bjorn Andersson wrote: On Tue 09 Jun 06:30 PDT 2020, Sai Prakash Ranjan wrote: Define iommus property for Coresight ETR component in SC7180 SoC with the SID and mask to enable SMMU translation for this master. We don't have &apps_smmu in linux-next, as we've yet to figure out how to disable the boot splash or support the stream mapping handover. So I'm not able to apply this. This is for SC7180 which has apps_smmu not SM8150. Thanks, Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCHv3 0/2] Convert QCOM watchdog timer bindings to YAML
Hi Bjorn, On 2020-02-12 03:54, Sai Prakash Ranjan wrote: This series converts QCOM watchdog timer bindings to YAML. Also it adds the missing SoC-specific compatible for QCS404, SC7180, SDM845 and SM8150 SoCs. v1: https://lore.kernel.org/lkml/cover.1576211720.git.saiprakash.ran...@codeaurora.org/ v2: https://lore.kernel.org/lkml/cover.1580570160.git.saiprakash.ran...@codeaurora.org/ Changes since v2: * Add missing compatibles to enum. Changes since v1: As per Rob's suggestion: * Replaced oneOf+const with enum. * Removed timeout-sec and included watchdog.yaml. * Removed repeated use of const:qcom,kpss-wdt and made use of enum. Sai Prakash Ranjan (2): dt-bindings: watchdog: Convert QCOM watchdog timer bindings to YAML dt-bindings: watchdog: Add compatible for QCS404, SC7180, SDM845, SM8150 .../devicetree/bindings/watchdog/qcom-wdt.txt | 28 --- .../bindings/watchdog/qcom-wdt.yaml | 48 +++ 2 files changed, 48 insertions(+), 28 deletions(-) delete mode 100644 Documentation/devicetree/bindings/watchdog/qcom-wdt.txt create mode 100644 Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml Gentle ping! Thanks, Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
[PATCHv3] coresight: tmc: Add shutdown callback for TMC ETR
Implement a shutdown callback to ensure ETR hardware is
properly shutdown in reboot/shutdown path. This is required
for ETR which has SMMU address translation enabled like on
SC7180 SoC and few others. If the hardware is still accessing
memory after SMMU translation is disabled as part of SMMU
shutdown callback in system reboot or shutdown path, then
IOVAs(I/O virtual address) which it was using will go on the
bus as the physical addresses which might result in unknown
crashes (NoC/interconnect errors). So we make sure from this
shutdown callback that the ETR is shutdown before SMMU translation
is disabled and device_link in SMMU driver will take care of
ordering of shutdown callbacks such that SMMU shutdown callback
is not called before any of its consumer shutdown callbacks.
Signed-off-by: Sai Prakash Ranjan
---
Changes since v2:
* Remove ETF/ETB disable as suggested by Mathieu and Mike since they are
not really affected.
* Remove coresight and misc device unregister since it is not required
for shutdown callback unlike remove callback and userspace is long gone
by this time.
Changes since v1:
* Use mode flag and drop enable flag as Mike suggested.
* Use spinlock before tmc hw disable as Mike suggested.
---
.../hwtracing/coresight/coresight-tmc-etr.c | 2 +-
drivers/hwtracing/coresight/coresight-tmc.c | 23 +++
drivers/hwtracing/coresight/coresight-tmc.h | 1 +
3 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c
b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index 625882bc8b08..b29c2db94d96 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -1110,7 +1110,7 @@ static void __tmc_etr_disable_hw(struct tmc_drvdata
*drvdata)
}
-static void tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
+void tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
{
__tmc_etr_disable_hw(drvdata);
/* Disable CATU device if this ETR is connected to one */
diff --git a/drivers/hwtracing/coresight/coresight-tmc.c
b/drivers/hwtracing/coresight/coresight-tmc.c
index 39fba1d16e6e..b13ce0daa572 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.c
+++ b/drivers/hwtracing/coresight/coresight-tmc.c
@@ -538,6 +538,28 @@ static int tmc_probe(struct amba_device *adev, const
struct amba_id *id)
return ret;
}
+static void tmc_shutdown(struct amba_device *adev)
+{
+ unsigned long flags;
+ struct tmc_drvdata *drvdata = amba_get_drvdata(adev);
+
+ spin_lock_irqsave(&drvdata->spinlock, flags);
+
+ if (drvdata->mode == CS_MODE_DISABLED)
+ goto out;
+
+ if (drvdata->config_type == TMC_CONFIG_TYPE_ETR)
+ tmc_etr_disable_hw(drvdata);
+
+ /*
+* We do not care about coresight unregister here unlike remove
+* callback which is required for making coresight modular since
+* the system is going down after this.
+*/
+out:
+ spin_unlock_irqrestore(&drvdata->spinlock, flags);
+}
+
static const struct amba_id tmc_ids[] = {
CS_AMBA_ID(0x000bb961),
/* Coresight SoC 600 TMC-ETR/ETS */
@@ -556,6 +578,7 @@ static struct amba_driver tmc_driver = {
.suppress_bind_attrs = true,
},
.probe = tmc_probe,
+ .shutdown = tmc_shutdown,
.id_table = tmc_ids,
};
builtin_amba_driver(tmc_driver);
diff --git a/drivers/hwtracing/coresight/coresight-tmc.h
b/drivers/hwtracing/coresight/coresight-tmc.h
index 71de978575f3..6e8d2dc33d17 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.h
+++ b/drivers/hwtracing/coresight/coresight-tmc.h
@@ -268,6 +268,7 @@ ssize_t tmc_etb_get_sysfs_trace(struct tmc_drvdata *drvdata,
/* ETR functions */
int tmc_read_prepare_etr(struct tmc_drvdata *drvdata);
int tmc_read_unprepare_etr(struct tmc_drvdata *drvdata);
+void tmc_etr_disable_hw(struct tmc_drvdata *drvdata);
extern const struct coresight_ops tmc_etr_cs_ops;
ssize_t tmc_etr_get_sysfs_trace(struct tmc_drvdata *drvdata,
loff_t pos, size_t len, char **bufpp);
base-commit: 059e38815950dbec65beafe03757bce9436e89a4
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
[PATCH] coresight: tmc: Fix TMC mode read in tmc_read_unprepare_etb()
Reading TMC mode register without proper coresight power
management can lead to exceptions like the one in the call
trace below in tmc_read_unprepare_etb() when the trace data
is read after the sink is disabled. So fix this by having
a check for coresight sysfs mode before reading TMC mode
management register in tmc_read_unprepare_etb() similar to
tmc_read_prepare_etb().
SError Interrupt on CPU6, code 0xbe000411 -- SError
pstate: 80400089 (Nzcv daIf +PAN -UAO)
pc : tmc_read_unprepare_etb+0x74/0x108
lr : tmc_read_unprepare_etb+0x54/0x108
sp : ff80d9507c30
x29: ff80d9507c30 x28: ff80b3569a0c
x27: x26: 000a0001
x25: ff80cbae9550 x24: 0010
x23: ffd07296b0f0 x22: ffd0109ee028
x21: x20: ff80d19e70e0
x19: ff80d19e7080 x18:
x17: x16:
x15: x14:
x13: x12:
x11: x10: dfd1
x9 : x8 : 0002
x7 : ffd071d0fe78 x6 :
x5 : 0080 x4 : 0001
x3 : ffd071d0fe98 x2 :
x1 : 0004 x0 : 0001
Kernel panic - not syncing: Asynchronous SError Interrupt
Fixes: 4525412a5046 ("coresight: tmc: making prepare/unprepare functions
generic")
Reported-by: Mike Leach
Signed-off-by: Sai Prakash Ranjan
---
drivers/hwtracing/coresight/coresight-tmc-etf.c | 13 ++---
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c
b/drivers/hwtracing/coresight/coresight-tmc-etf.c
index 36cce2bfb744..6375504ba8b0 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
@@ -639,15 +639,14 @@ int tmc_read_unprepare_etb(struct tmc_drvdata *drvdata)
spin_lock_irqsave(&drvdata->spinlock, flags);
- /* There is no point in reading a TMC in HW FIFO mode */
- mode = readl_relaxed(drvdata->base + TMC_MODE);
- if (mode != TMC_MODE_CIRCULAR_BUFFER) {
- spin_unlock_irqrestore(&drvdata->spinlock, flags);
- return -EINVAL;
- }
-
/* Re-enable the TMC if need be */
if (drvdata->mode == CS_MODE_SYSFS) {
+ /* There is no point in reading a TMC in HW FIFO mode */
+ mode = readl_relaxed(drvdata->base + TMC_MODE);
+ if (mode != TMC_MODE_CIRCULAR_BUFFER) {
+ spin_unlock_irqrestore(&drvdata->spinlock, flags);
+ return -EINVAL;
+ }
/*
* The trace run will continue with the same allocated trace
* buffer. As such zero-out the buffer so that we don't end
base-commit: 3d439a6c349778f129de19595db564a8366c3634
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH 2/2] coresight: tmc: Add shutdown callback for TMC ETR/ETF
Hi Mathieu, On 2020-06-09 20:57, Mathieu Poirier wrote: On Mon, 8 Jun 2020 at 08:07, Sai Prakash Ranjan wrote: Hi Mathieu, Mike On 2020-06-04 12:57, Sai Prakash Ranjan wrote: > [...] >> >> Robin has a point - user space is long gone at this time. As such the >> first >> question to ask is what kind of CS session was running at the time the >> system >> was shutting down. Was it a perf session of a sysfs session? >> >> I'm guessing it was a sysfs session because user space has been blown >> away a >> while back and part of that process should have killed all perf >> sessions. > > I was enabling trace via sysfs. > >> >> If I am correct then simply switching off the ETR HW in the shutdown() >> amba bus >> callback should be fine - otherwise Mike's approach is mandatory. >> There is >> also the exchange between Robin and Sai about removing the SMMU >> shutdown >> callback, but that thread is still incomplete. >> > > If Robin is hinting at removing SMMU shutdown callback, then I think > adding > all these shutdown callbacks to all clients of SMMU can be avoided. Git > blaming > the thing shows it was added to avoid some kexec memory corruption. > I think I misread the cryptic hint from Robin and it is not right to remove SMMU shutdown callback. For more details on why that was a bad idea and would break kexec, please refer to [1]. As for the coresight, can I disable the ETR only in the tmc shutdown callback or are we still concerned about the userspace coming into picture? User space isn't a concern, especially after you've confirmed the problem occured during an ongoing sysfs session. Will post v3 with comments addressed after 5.8-rc1 is out. Thanks, Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH 2/2] arm64: dts: qcom: sc7180: Add support to skip powering up of ETM
Hi Bjorn,
On 2020-05-15 16:21, Sai Prakash Ranjan wrote:
Add "qcom,skip-power-up" property to skip powering up ETM
on SC7180 SoC to workaround a hardware errata where CPU
watchdog counter is stopped when ETM power up bit is set
(i.e., when TRCPDCR.PU = 1).
Signed-off-by: Sai Prakash Ranjan
---
Depends on ETM driver change here -
https://lore.kernel.org/patchwork/cover/1242100/
---
arch/arm64/boot/dts/qcom/sc7180.dtsi | 8
1 file changed, 8 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi
b/arch/arm64/boot/dts/qcom/sc7180.dtsi
index 8b3707347547..de4bae4ec224 100644
--- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
@@ -1657,6 +1657,7 @@
clocks = <&aoss_qmp>;
clock-names = "apb_pclk";
arm,coresight-loses-context-with-cpu;
+ qcom,skip-power-up;
out-ports {
port {
@@ -1676,6 +1677,7 @@
clocks = <&aoss_qmp>;
clock-names = "apb_pclk";
arm,coresight-loses-context-with-cpu;
+ qcom,skip-power-up;
out-ports {
port {
@@ -1695,6 +1697,7 @@
clocks = <&aoss_qmp>;
clock-names = "apb_pclk";
arm,coresight-loses-context-with-cpu;
+ qcom,skip-power-up;
out-ports {
port {
@@ -1714,6 +1717,7 @@
clocks = <&aoss_qmp>;
clock-names = "apb_pclk";
arm,coresight-loses-context-with-cpu;
+ qcom,skip-power-up;
out-ports {
port {
@@ -1733,6 +1737,7 @@
clocks = <&aoss_qmp>;
clock-names = "apb_pclk";
arm,coresight-loses-context-with-cpu;
+ qcom,skip-power-up;
out-ports {
port {
@@ -1752,6 +1757,7 @@
clocks = <&aoss_qmp>;
clock-names = "apb_pclk";
arm,coresight-loses-context-with-cpu;
+ qcom,skip-power-up;
out-ports {
port {
@@ -1771,6 +1777,7 @@
clocks = <&aoss_qmp>;
clock-names = "apb_pclk";
arm,coresight-loses-context-with-cpu;
+ qcom,skip-power-up;
out-ports {
port {
@@ -1790,6 +1797,7 @@
clocks = <&aoss_qmp>;
clock-names = "apb_pclk";
arm,coresight-loses-context-with-cpu;
+ qcom,skip-power-up;
out-ports {
port {
I have sent this patch as a part of other coresight changes
to keep all coresight DT changes together[1], we can drop this
patch now.
[1] - https://lore.kernel.org/patchwork/cover/1253969/
Thanks,
Sai
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member
of Code Aurora Forum, hosted by The Linux Foundation
[PATCH 1/4] arm64: dts: qcom: sc7180: Add support to skip powering up of ETM
Add "qcom,skip-power-up" property to skip powering up ETM
on SC7180 SoC to workaround a hardware errata where CPU
watchdog counter is stopped when ETM power up bit is set
(i.e., when TRCPDCR.PU = 1).
Signed-off-by: Sai Prakash Ranjan
---
Depends on ETM driver change here:
-
https://git.linaro.org/kernel/coresight.git/commit/?h=next-v5.8-rc1&id=159e248e75b1b548276b6571d7740a35cab1f5be
---
arch/arm64/boot/dts/qcom/sc7180.dtsi | 8
1 file changed, 8 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi
b/arch/arm64/boot/dts/qcom/sc7180.dtsi
index 7c2b79dda3d7..f684a0b87848 100644
--- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
@@ -1810,6 +1810,7 @@
clocks = <&aoss_qmp>;
clock-names = "apb_pclk";
arm,coresight-loses-context-with-cpu;
+ qcom,skip-power-up;
out-ports {
port {
@@ -1829,6 +1830,7 @@
clocks = <&aoss_qmp>;
clock-names = "apb_pclk";
arm,coresight-loses-context-with-cpu;
+ qcom,skip-power-up;
out-ports {
port {
@@ -1848,6 +1850,7 @@
clocks = <&aoss_qmp>;
clock-names = "apb_pclk";
arm,coresight-loses-context-with-cpu;
+ qcom,skip-power-up;
out-ports {
port {
@@ -1867,6 +1870,7 @@
clocks = <&aoss_qmp>;
clock-names = "apb_pclk";
arm,coresight-loses-context-with-cpu;
+ qcom,skip-power-up;
out-ports {
port {
@@ -1886,6 +1890,7 @@
clocks = <&aoss_qmp>;
clock-names = "apb_pclk";
arm,coresight-loses-context-with-cpu;
+ qcom,skip-power-up;
out-ports {
port {
@@ -1905,6 +1910,7 @@
clocks = <&aoss_qmp>;
clock-names = "apb_pclk";
arm,coresight-loses-context-with-cpu;
+ qcom,skip-power-up;
out-ports {
port {
@@ -1924,6 +1930,7 @@
clocks = <&aoss_qmp>;
clock-names = "apb_pclk";
arm,coresight-loses-context-with-cpu;
+ qcom,skip-power-up;
out-ports {
port {
@@ -1943,6 +1950,7 @@
clocks = <&aoss_qmp>;
clock-names = "apb_pclk";
arm,coresight-loses-context-with-cpu;
+ qcom,skip-power-up;
out-ports {
port {
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
[PATCH 3/4] arm64: dts: qcom: sc7180: Add support for context losing replicator
Add "qcom,replicator-loses-context" property to the replicator
in Always-on domain in SC7180 SoC to enable coresight replicator
driver to handle this variation of replicator designs.
Signed-off-by: Sai Prakash Ranjan
---
Depends on coresight replicator change here:
-
https://git.linaro.org/kernel/coresight.git/commit/?h=next-v5.8-rc1&id=1b6cddfb7ebb5ed293124698f147e914b15315a1
---
arch/arm64/boot/dts/qcom/sc7180.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi
b/arch/arm64/boot/dts/qcom/sc7180.dtsi
index 9b38867740ca..0cbe322a30c9 100644
--- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
@@ -1784,6 +1784,7 @@
clocks = <&aoss_qmp>;
clock-names = "apb_pclk";
+ qcom,replicator-loses-context;
out-ports {
port {
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
[PATCH 4/4] arm64: dts: qcom: sm8150: Add Coresight support
Add coresight components found on Qualcomm SM8150 SoC.
Signed-off-by: Sai Prakash Ranjan
---
Depends on following coresight driver and SM8150 SMMU support:
-
https://git.linaro.org/kernel/coresight.git/commit/?h=next-v5.8-rc1&id=159e248e75b1b548276b6571d7740a35cab1f5be
-
https://git.linaro.org/kernel/coresight.git/commit/?h=next-v5.8-rc1&id=1b6cddfb7ebb5ed293124698f147e914b15315a1
- https://lore.kernel.org/lkml/20200524023815.21789-2-jonat...@marek.ca/
---
arch/arm64/boot/dts/qcom/sm8150.dtsi | 591 +++
1 file changed, 591 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi
b/arch/arm64/boot/dts/qcom/sm8150.dtsi
index 141c21dfa68c..a2fc77211cc3 100644
--- a/arch/arm64/boot/dts/qcom/sm8150.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi
@@ -538,6 +538,597 @@
};
};
+ stm@6002000 {
+ compatible = "arm,coresight-stm", "arm,primecell";
+ reg = <0 0x06002000 0 0x1000>,
+ <0 0x1628 0 0x18>;
+ reg-names = "stm-base", "stm-stimulus-base";
+
+ clocks = <&aoss_qmp>;
+ clock-names = "apb_pclk";
+
+ out-ports {
+ port {
+ stm_out: endpoint {
+ remote-endpoint =
<&funnel0_in7>;
+ };
+ };
+ };
+ };
+
+ funnel@6041000 {
+ compatible = "arm,coresight-dynamic-funnel",
"arm,primecell";
+ reg = <0 0x06041000 0 0x1000>;
+
+ clocks = <&aoss_qmp>;
+ clock-names = "apb_pclk";
+
+ out-ports {
+ port {
+ funnel0_out: endpoint {
+ remote-endpoint =
<&merge_funnel_in0>;
+ };
+ };
+ };
+
+ in-ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@7 {
+ reg = <7>;
+ funnel0_in7: endpoint {
+ remote-endpoint = <&stm_out>;
+ };
+ };
+ };
+ };
+
+ funnel@6042000 {
+ compatible = "arm,coresight-dynamic-funnel",
"arm,primecell";
+ reg = <0 0x06042000 0 0x1000>;
+
+ clocks = <&aoss_qmp>;
+ clock-names = "apb_pclk";
+
+ out-ports {
+ port {
+ funnel1_out: endpoint {
+ remote-endpoint =
<&merge_funnel_in1>;
+ };
+ };
+ };
+
+ in-ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@4 {
+ reg = <4>;
+ funnel1_in4: endpoint {
+ remote-endpoint =
<&swao_replicator_out>;
+ };
+ };
+ };
+ };
+
+ funnel@6043000 {
+ compatible = "arm,coresight-dynamic-funnel",
"arm,primecell";
+ reg = <0 0x06043000 0 0x1000>;
+
+ clocks = <&aoss_qmp>;
+ clock-names = "apb_pclk";
+
+ out-ports {
+ port {
+ funnel2_out: endpoint {
+ remote-endpoint =
<&merge_funnel_in2>;
+ };
+ };
+ };
+
+ in-ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@2 {
+
[PATCH 2/4] arm64: dts: qcom: sc7180: Add iommus property to ETR
Define iommus property for Coresight ETR component in
SC7180 SoC with the SID and mask to enable SMMU
translation for this master.
Signed-off-by: Sai Prakash Ranjan
---
arch/arm64/boot/dts/qcom/sc7180.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi
b/arch/arm64/boot/dts/qcom/sc7180.dtsi
index f684a0b87848..9b38867740ca 100644
--- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
@@ -1711,6 +1711,7 @@
etr@6048000 {
compatible = "arm,coresight-tmc", "arm,primecell";
reg = <0 0x06048000 0 0x1000>;
+ iommus = <&apps_smmu 0x04a0 0x20>;
clocks = <&aoss_qmp>;
clock-names = "apb_pclk";
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
[PATCH 0/4] Add coresight support for SM8150 and few changes to SC7180
This series adds coresight support for SM8150 SoC and some coresight DT changes to SC7180 SoC. Patch 1 depends on coresight etm driver change: - https://git.linaro.org/kernel/coresight.git/commit/?h=next-v5.8-rc1&id=159e248e75b1b548276b6571d7740a35cab1f5be Patch 3 depends on coresight replicator driver change: - https://git.linaro.org/kernel/coresight.git/commit/?h=next-v5.8-rc1&id=1b6cddfb7ebb5ed293124698f147e914b15315a1 Patch 4 depends on following coresight driver changes and SMMU DT node for SM8150 - https://git.linaro.org/kernel/coresight.git/commit/?h=next-v5.8-rc1&id=159e248e75b1b548276b6571d7740a35cab1f5be - https://git.linaro.org/kernel/coresight.git/commit/?h=next-v5.8-rc1&id=1b6cddfb7ebb5ed293124698f147e914b15315a1 - https://lore.kernel.org/lkml/20200524023815.21789-2-jonat...@marek.ca/ Tested this series on SM8150 and SC7180. Sai Prakash Ranjan (4): arm64: dts: qcom: sc7180: Add support to skip powering up of ETM arm64: dts: qcom: sc7180: Add iommus property to ETR arm64: dts: qcom: sc7180: Add support for context losing replicator arm64: dts: qcom: sm8150: Add Coresight support arch/arm64/boot/dts/qcom/sc7180.dtsi | 10 + arch/arm64/boot/dts/qcom/sm8150.dtsi | 591 +++ 2 files changed, 601 insertions(+) base-commit: 98cfcf1a9c542d6bec7b29915d838caaf72da737 -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH 2/2] coresight: tmc: Add shutdown callback for TMC ETR/ETF
Hi Mathieu, Mike On 2020-06-04 12:57, Sai Prakash Ranjan wrote: [...] Robin has a point - user space is long gone at this time. As such the first question to ask is what kind of CS session was running at the time the system was shutting down. Was it a perf session of a sysfs session? I'm guessing it was a sysfs session because user space has been blown away a while back and part of that process should have killed all perf sessions. I was enabling trace via sysfs. If I am correct then simply switching off the ETR HW in the shutdown() amba bus callback should be fine - otherwise Mike's approach is mandatory. There is also the exchange between Robin and Sai about removing the SMMU shutdown callback, but that thread is still incomplete. If Robin is hinting at removing SMMU shutdown callback, then I think adding all these shutdown callbacks to all clients of SMMU can be avoided. Git blaming the thing shows it was added to avoid some kexec memory corruption. I think I misread the cryptic hint from Robin and it is not right to remove SMMU shutdown callback. For more details on why that was a bad idea and would break kexec, please refer to [1]. As for the coresight, can I disable the ETR only in the tmc shutdown callback or are we still concerned about the userspace coming into picture? [1] https://lore.kernel.org/patchwork/patch/1253131/ Thanks, Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [RFC PATCH] iommu/arm-smmu: Remove shutdown callback
Hi Will, On 2020-06-08 17:08, Will Deacon wrote: On Mon, Jun 08, 2020 at 02:43:03PM +0530, Sai Prakash Ranjan wrote: On 2020-06-08 13:48, Will Deacon wrote: > On Sun, Jun 07, 2020 at 04:39:18PM +0530, Sai Prakash Ranjan wrote: > > Remove SMMU shutdown callback since it seems to cause more > > problems than benefits. With this callback, we need to make > > sure that all clients/consumers of SMMU do not perform any > > DMA activity once the SMMU is shutdown and translation is > > disabled. In other words we need to add shutdown callbacks > > for all those clients to make sure they do not perform any > > DMA or else we see all kinds of weird crashes during reboot > > or shutdown. This is clearly not scalable as the number of > > clients of SMMU would vary across SoCs and we would need to > > add shutdown callbacks to almost all drivers eventually. > > This callback was added for kexec usecase where it was known > > to cause memory corruptions when SMMU was not shutdown but > > that does not directly relate to SMMU because the memory > > corruption could be because of the client of SMMU which is > > not shutdown properly before booting into new kernel. So in > > that case, we need to identify the client of SMMU causing > > the memory corruption and add appropriate shutdown callback > > to the client rather than to the SMMU. > > > > Signed-off-by: Sai Prakash Ranjan > > --- > > drivers/iommu/arm-smmu-v3.c | 6 -- > > drivers/iommu/arm-smmu.c| 6 -- > > 2 files changed, 12 deletions(-) > > This feels like a giant bodge to me and I think that any driver which > continues to perform DMA after its ->shutdown() function has been > invoked > is buggy. Wouldn't that cause problems with kexec(), for example? > Yes it is definitely a bug in the client driver if DMA is performed after shutdown callback of that respective driver is invoked and it must be fixed in that driver. But here the problem I was describing is not that, most of the drivers do not have a shutdown callback to begin with and adding it just because of shutdown dependency on SMMU doesn't seem so well because we can have many more such clients in the future and then we have to just go on adding the shutdown callbacks everywhere. I'm not sure why you're trying to treat these cases differently. It's also not "just because of SMMU", is it? Like I said, kexec() would be broken regardless. The bottom line is that after running ->shutdown() (or skipping it if it's not implemented) for a driver, then the device must no longer perform DMA. Yes it's not just because of SMMU. Now I understand that we indeed need to shutdown SMMU like any other driver and we need to fix the client drivers as well. > There's a clear shutdown dependency ordering, where the clients of the > SMMU need to shutdown before the SMMU itself, but that's not really > the SMMU driver's problem to solve. > The problem with kexec may not be directly related to SMMU as you said above if DMA is performed after the client shutdown callback, then its a bug in the client driver, so that needs to be fixed in the client driver, not the SMMU. So is there any point in having the SMMU shutdown callback? Given that the SMMU mediates DMA transactions for all upstream masters based on in-memory data (e.g. page tables), then I think it's a /very/ good idea to tear that down as part of the shutdown callback before the memory is effectively free()d. One thing I would be in favour of is changing the ->shutdown() code to honour disable_bypass=1 so that we put the SMMU in an aborting state instead of passthrough. Would that help at all? It would at least avoid the memory corruption on missing shutdown callback. This would be good, however this would then mask the fact that it is client drivers who are buggy and if we stop seeing issues, then no one will bother fixing the client drivers. So we better go ahead and fix the client drivers first and I will drop this patch. As you see, with this SMMU shutdown callback, we need to add shutdown callbacks in all the client drivers. I don't see the problem with that. Why is it a problem? Not a problem, I wanted to confirm if kexec issue was indeed only due to client driver bugs or SMMU has also some contribution. We have already started adding client driver shutdown callbacks [1][2], but I also wanted to get this clarified, so sent this patch as RFC. [1] - https://lore.kernel.org/lkml/1591009402-681-1-git-send-email-mkri...@codeaurora.org/ [2] - https://lore.kernel.org/lkml/28123d1e19f235f97555ee36a5ed8b52d20cbdea.1590947174.git.saiprakash.ran...@codeaurora.org/ Thanks, Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [RFC PATCH] iommu/arm-smmu: Remove shutdown callback
Hi Robin, On 2020-06-08 16:56, Robin Murphy wrote: On 2020-06-08 10:13, Sai Prakash Ranjan wrote: Hi Will, On 2020-06-08 13:48, Will Deacon wrote: On Sun, Jun 07, 2020 at 04:39:18PM +0530, Sai Prakash Ranjan wrote: Remove SMMU shutdown callback since it seems to cause more problems than benefits. With this callback, we need to make sure that all clients/consumers of SMMU do not perform any DMA activity once the SMMU is shutdown and translation is disabled. In other words we need to add shutdown callbacks for all those clients to make sure they do not perform any DMA or else we see all kinds of weird crashes during reboot or shutdown. This is clearly not scalable as the number of clients of SMMU would vary across SoCs and we would need to add shutdown callbacks to almost all drivers eventually. This callback was added for kexec usecase where it was known to cause memory corruptions when SMMU was not shutdown but that does not directly relate to SMMU because the memory corruption could be because of the client of SMMU which is not shutdown properly before booting into new kernel. So in that case, we need to identify the client of SMMU causing the memory corruption and add appropriate shutdown callback to the client rather than to the SMMU. Signed-off-by: Sai Prakash Ranjan ---  drivers/iommu/arm-smmu-v3.c | 6 --  drivers/iommu/arm-smmu.c   | 6 --  2 files changed, 12 deletions(-) This feels like a giant bodge to me and I think that any driver which continues to perform DMA after its ->shutdown() function has been invoked is buggy. Wouldn't that cause problems with kexec(), for example? Yes it is definitely a bug in the client driver if DMA is performed after shutdown callback of that respective driver is invoked and it must be fixed in that driver. But here the problem I was describing is not that, most of the drivers do not have a shutdown callback to begin with and adding it just because of shutdown dependency on SMMU doesn't seem so well because we can have many more such clients in the future and then we have to just go on adding the shutdown callbacks everywhere. Yes, indeed we do. Like it or not, kexec is a thing, and about 98% of drivers will have been written without considering it. If fixing one problem exposes lots of other problems, can you honestly say that the best solution is to go back and re-break the original thing? No, definitely not. I was under the wrong impression that *kexec thing* was more due to the client driver bugs rather than the SMMU. There's a clear shutdown dependency ordering, where the clients of the SMMU need to shutdown before the SMMU itself, but that's not really the SMMU driver's problem to solve. The problem with kexec may not be directly related to SMMU as you said above if DMA is performed after the client shutdown callback, then its a bug in the client driver, so that needs to be fixed in the client driver, not the SMMU. So is there any point in having the SMMU shutdown callback? The point is that kexec needs to return the system to a state as close to reset as possible. The SMMU is at least as relevant as any other device in that regard, if not far more so - consider if the first kernel *did* leave it enabled with whatever left-over translations in place, and kexec'ed into another OS that wasn't SMMU-aware... Yes understood. I wrongly assumed that the kexec problem was more of a client driver bugs rather than SMMU. But I see that we indeed need to shutdown SMMU as any other driver. As you see, with this SMMU shutdown callback, we need to add shutdown callbacks in all the client drivers. Yes. And if you really want to argue against that, then at least be consistent and propose removing shutdown from *all* the IOMMU drivers. And then probably propose removing kexec support from all their respective architectures... ;) Not my intention to break or remove kexec, hence the RFC tag :) As for the consistent part, out of 18 iommu drivers only 5 have shutdown callbacks, so nothing much to remove there and kexec is broken there probably. However I got your point and will drop this patch. Thanks, Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [RFC PATCH] iommu/arm-smmu: Remove shutdown callback
Hi Will, On 2020-06-08 13:48, Will Deacon wrote: On Sun, Jun 07, 2020 at 04:39:18PM +0530, Sai Prakash Ranjan wrote: Remove SMMU shutdown callback since it seems to cause more problems than benefits. With this callback, we need to make sure that all clients/consumers of SMMU do not perform any DMA activity once the SMMU is shutdown and translation is disabled. In other words we need to add shutdown callbacks for all those clients to make sure they do not perform any DMA or else we see all kinds of weird crashes during reboot or shutdown. This is clearly not scalable as the number of clients of SMMU would vary across SoCs and we would need to add shutdown callbacks to almost all drivers eventually. This callback was added for kexec usecase where it was known to cause memory corruptions when SMMU was not shutdown but that does not directly relate to SMMU because the memory corruption could be because of the client of SMMU which is not shutdown properly before booting into new kernel. So in that case, we need to identify the client of SMMU causing the memory corruption and add appropriate shutdown callback to the client rather than to the SMMU. Signed-off-by: Sai Prakash Ranjan --- drivers/iommu/arm-smmu-v3.c | 6 -- drivers/iommu/arm-smmu.c| 6 -- 2 files changed, 12 deletions(-) This feels like a giant bodge to me and I think that any driver which continues to perform DMA after its ->shutdown() function has been invoked is buggy. Wouldn't that cause problems with kexec(), for example? Yes it is definitely a bug in the client driver if DMA is performed after shutdown callback of that respective driver is invoked and it must be fixed in that driver. But here the problem I was describing is not that, most of the drivers do not have a shutdown callback to begin with and adding it just because of shutdown dependency on SMMU doesn't seem so well because we can have many more such clients in the future and then we have to just go on adding the shutdown callbacks everywhere. There's a clear shutdown dependency ordering, where the clients of the SMMU need to shutdown before the SMMU itself, but that's not really the SMMU driver's problem to solve. The problem with kexec may not be directly related to SMMU as you said above if DMA is performed after the client shutdown callback, then its a bug in the client driver, so that needs to be fixed in the client driver, not the SMMU. So is there any point in having the SMMU shutdown callback? As you see, with this SMMU shutdown callback, we need to add shutdown callbacks in all the client drivers. Thanks, Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
[RFC PATCH] iommu/arm-smmu: Remove shutdown callback
Remove SMMU shutdown callback since it seems to cause more
problems than benefits. With this callback, we need to make
sure that all clients/consumers of SMMU do not perform any
DMA activity once the SMMU is shutdown and translation is
disabled. In other words we need to add shutdown callbacks
for all those clients to make sure they do not perform any
DMA or else we see all kinds of weird crashes during reboot
or shutdown. This is clearly not scalable as the number of
clients of SMMU would vary across SoCs and we would need to
add shutdown callbacks to almost all drivers eventually.
This callback was added for kexec usecase where it was known
to cause memory corruptions when SMMU was not shutdown but
that does not directly relate to SMMU because the memory
corruption could be because of the client of SMMU which is
not shutdown properly before booting into new kernel. So in
that case, we need to identify the client of SMMU causing
the memory corruption and add appropriate shutdown callback
to the client rather than to the SMMU.
Signed-off-by: Sai Prakash Ranjan
---
drivers/iommu/arm-smmu-v3.c | 6 --
drivers/iommu/arm-smmu.c| 6 --
2 files changed, 12 deletions(-)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 8a908c50c306..634da02fef78 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -4142,11 +4142,6 @@ static int arm_smmu_device_remove(struct platform_device
*pdev)
return 0;
}
-static void arm_smmu_device_shutdown(struct platform_device *pdev)
-{
- arm_smmu_device_remove(pdev);
-}
-
static const struct of_device_id arm_smmu_of_match[] = {
{ .compatible = "arm,smmu-v3", },
{ },
@@ -4161,7 +4156,6 @@ static struct platform_driver arm_smmu_driver = {
},
.probe = arm_smmu_device_probe,
.remove = arm_smmu_device_remove,
- .shutdown = arm_smmu_device_shutdown,
};
module_platform_driver(arm_smmu_driver);
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 243bc4cb2705..4d80516c789f 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -2276,11 +2276,6 @@ static int arm_smmu_device_remove(struct platform_device
*pdev)
return 0;
}
-static void arm_smmu_device_shutdown(struct platform_device *pdev)
-{
- arm_smmu_device_remove(pdev);
-}
-
static int __maybe_unused arm_smmu_runtime_resume(struct device *dev)
{
struct arm_smmu_device *smmu = dev_get_drvdata(dev);
@@ -2335,7 +2330,6 @@ static struct platform_driver arm_smmu_driver = {
},
.probe = arm_smmu_device_probe,
.remove = arm_smmu_device_remove,
- .shutdown = arm_smmu_device_shutdown,
};
module_platform_driver(arm_smmu_driver);
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH 1/6] arm64: dts: qcom: sm8150: add apps_smmu node
On 2020-06-05 20:21, Nicolas Dechesne wrote:
On Fri, Jun 5, 2020 at 4:39 PM Sai Prakash Ranjan
wrote:
Hi Nico,
On 2020-06-05 20:01, Nicolas Dechesne wrote:
> On Fri, Jun 5, 2020 at 4:14 PM Sai Prakash Ranjan
> wrote:
>>
>> On 2020-06-05 19:40, Jonathan Marek wrote:
>> > On 6/5/20 10:03 AM, Sai Prakash Ranjan wrote:
>> >> On 2020-05-29 08:45, Bjorn Andersson wrote:
>> >>> On Thu 28 May 20:02 PDT 2020, Jonathan Marek wrote:
>> >>>
>> >>>>
>> >>>>
>> >>>> On 5/28/20 10:52 PM, Bjorn Andersson wrote:
>> >>>> > On Sat 23 May 19:38 PDT 2020, Jonathan Marek wrote:
>> >>>> >
>> >>>> > > Add the apps_smmu node for sm8150. Note that adding the iommus
field for
>> >>>> > > UFS is required because initializing the iommu removes the bypass
mapping
>> >>>> > > that created by the bootloader.
>> >>>> > >
>> >>>> >
>> >>>> > Unrelated to the patch itself; how do you disable the splash screen on
>> >>>> > 8150? "fastboot oem select-display-panel none" doesn't seem to work
for
>> >>>> > me on the MTP - and hence this would prevent my device from booting.
>> >>>> >
>> >>>> > Thanks,
>> >>>> > Bjorn
>> >>>> >
>> >>>>
>> >>>> I don't have a MTP, but on HDK855, "fastboot oem
>> >>>> select-display-panel none"
>> >>>> combined with setting the physical switch to HDMI mode (which
>> >>>> switches off
>> >>>> the 1440x2560 panel) gets it to not setup the display at all (just
>> >>>> the
>> >>>> fastboot command isn't enough).
>> >>>>
>> >>>
>> >>> Okay, I don't think we have anything equivalent on the MTP, but good
>> >>> to
>> >>> know.
>> >>>
>> >>
>> >> Actually I tried out this in SM8150 MTP and it works fine for me,
>> >>
>> >> "fastboot set_active a; fastboot set_active b; fastboot set_active a;
>> >> fastboot oem select-display-panel none; fastboot reboot bootloader;
>> >> fastboot boot boot-sm8150.img"
>> >>
>> >> Also I need to switch slots everytime like above, otherwise I always
>> >> see some error
>> >> while loading the boot image.
>> >>
>> >
>> > What is the error? If it is "FAILED (remote: Failed to
>> > load/authenticate boot image: Load Error)" then flashing/erasing
>> > boot_a will make it go away ("fastboot erase boot_a") for the next 6
>> > or so "failed" boots.
>> >
>>
>> Yes this exact error.
>
> The bootloader maintains a 'boot status' in one of the partition
> attributes. After a certain amount of 'failed' boot , it will switch
> to the other boot partition. It's the same thing on RB3/DB845c. In our
> release for DB845c, we are patching the bootloader so that this
> behavior is bypassed. On typical 'product' there is a user space
> application that will come and set the partition attribute to indicate
> the boot was successful.
>
> For the record, this is the patch we use on 845c:
>
https://git.linaro.org/landing-teams/working/qualcomm/abl.git/commit/?h=release/LE.UM.2.3.7-09200-sda845.0&id=e3dc60213234ed626161a568ba587ddac63c5158
>
> rebuilding EDK2/ABL requires access to signing tools.. so it might not
> be possible for everyone. but in case you can, it should be
> straightforward to reuse this patch.
>
Thank you for these details and the patch, it's very useful.
I do have access to ABL code and the signing tools and can build one.
Good. Then the next problem you will likely face is that building QCOM
ABL is far from being straightforward. Why would it be? ;)
That's the script we use to build it ourselves:
https://git.linaro.org/ci/job/configs.git/tree/lt-qcom-bootloader/dragonboard845c/builders.sh#n61
It has a reference to sectools which we have (internally) access to,
but you have it too, and you should be able to leverage most of the
script.
Looks like a cool tool, will definitely try it out :) Also internally we
have another
tool to build ABL(if you are aware of kdev then you will know what this
is called, guess ;))
which takes care of cloning and building and signing all the things
required
(although very weirdly it clones sectools everytime which should be
fixed, I just comment
that part out when I build) and all it takes is one command "make" :)
Thanks,
Sai
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member
of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH 1/6] arm64: dts: qcom: sm8150: add apps_smmu node
Hi Nico,
On 2020-06-05 20:01, Nicolas Dechesne wrote:
On Fri, Jun 5, 2020 at 4:14 PM Sai Prakash Ranjan
wrote:
On 2020-06-05 19:40, Jonathan Marek wrote:
> On 6/5/20 10:03 AM, Sai Prakash Ranjan wrote:
>> On 2020-05-29 08:45, Bjorn Andersson wrote:
>>> On Thu 28 May 20:02 PDT 2020, Jonathan Marek wrote:
>>>
>>>>
>>>>
>>>> On 5/28/20 10:52 PM, Bjorn Andersson wrote:
>>>> > On Sat 23 May 19:38 PDT 2020, Jonathan Marek wrote:
>>>> >
>>>> > > Add the apps_smmu node for sm8150. Note that adding the iommus field
for
>>>> > > UFS is required because initializing the iommu removes the bypass
mapping
>>>> > > that created by the bootloader.
>>>> > >
>>>> >
>>>> > Unrelated to the patch itself; how do you disable the splash screen on
>>>> > 8150? "fastboot oem select-display-panel none" doesn't seem to work for
>>>> > me on the MTP - and hence this would prevent my device from booting.
>>>> >
>>>> > Thanks,
>>>> > Bjorn
>>>> >
>>>>
>>>> I don't have a MTP, but on HDK855, "fastboot oem
>>>> select-display-panel none"
>>>> combined with setting the physical switch to HDMI mode (which
>>>> switches off
>>>> the 1440x2560 panel) gets it to not setup the display at all (just
>>>> the
>>>> fastboot command isn't enough).
>>>>
>>>
>>> Okay, I don't think we have anything equivalent on the MTP, but good
>>> to
>>> know.
>>>
>>
>> Actually I tried out this in SM8150 MTP and it works fine for me,
>>
>> "fastboot set_active a; fastboot set_active b; fastboot set_active a;
>> fastboot oem select-display-panel none; fastboot reboot bootloader;
>> fastboot boot boot-sm8150.img"
>>
>> Also I need to switch slots everytime like above, otherwise I always
>> see some error
>> while loading the boot image.
>>
>
> What is the error? If it is "FAILED (remote: Failed to
> load/authenticate boot image: Load Error)" then flashing/erasing
> boot_a will make it go away ("fastboot erase boot_a") for the next 6
> or so "failed" boots.
>
Yes this exact error.
The bootloader maintains a 'boot status' in one of the partition
attributes. After a certain amount of 'failed' boot , it will switch
to the other boot partition. It's the same thing on RB3/DB845c. In our
release for DB845c, we are patching the bootloader so that this
behavior is bypassed. On typical 'product' there is a user space
application that will come and set the partition attribute to indicate
the boot was successful.
For the record, this is the patch we use on 845c:
https://git.linaro.org/landing-teams/working/qualcomm/abl.git/commit/?h=release/LE.UM.2.3.7-09200-sda845.0&id=e3dc60213234ed626161a568ba587ddac63c5158
rebuilding EDK2/ABL requires access to signing tools.. so it might not
be possible for everyone. but in case you can, it should be
straightforward to reuse this patch.
Thank you for these details and the patch, it's very useful.
I do have access to ABL code and the signing tools and can build one.
Thanks,
Sai
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member
of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH 1/6] arm64: dts: qcom: sm8150: add apps_smmu node
On 2020-05-25 15:07, Sai Prakash Ranjan wrote:
Hi Jonathan,
On 2020-05-24 08:08, Jonathan Marek wrote:
Add the apps_smmu node for sm8150. Note that adding the iommus field
for
UFS is required because initializing the iommu removes the bypass
mapping
that created by the bootloader.
Signed-off-by: Jonathan Marek
---
arch/arm64/boot/dts/qcom/sm8150.dtsi | 91
1 file changed, 91 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi
b/arch/arm64/boot/dts/qcom/sm8150.dtsi
index a36512d1f6a1..acb839427b12 100644
--- a/arch/arm64/boot/dts/qcom/sm8150.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi
@@ -442,6 +442,8 @@ ufs_mem_hc: ufshc@1d84000 {
resets = <&gcc GCC_UFS_PHY_BCR>;
reset-names = "rst";
+ iommus = <&apps_smmu 0x300 0>;
+
clock-names =
"core_clk",
"bus_aggr_clk",
@@ -706,6 +708,7 @@ usb_1_dwc3: dwc3@a60 {
compatible = "snps,dwc3";
reg = <0 0x0a60 0 0xcd00>;
interrupts = ;
+ iommus = <&apps_smmu 0x140 0>;
snps,dis_u2_susphy_quirk;
snps,dis_enblslpm_quirk;
phys = <&usb_1_hsphy>, <&usb_1_ssphy>;
@@ -742,6 +745,94 @@ spmi_bus: spmi@c44 {
cell-index = <0>;
};
+ apps_smmu: iommu@1500 {
+ compatible = "qcom,sdm845-smmu-500", "arm,mmu-500";
This should be qcom,sm8150-smmu-500 and also you need to update the
arm-smmu
binding with this compatible in a separate patch.
I tested out this series with my coresight patches for enabling SMMU
translation
for ETR on SM8150, it works fine. With this above comment addressed and
with
Bjorn's comments on commit description addressed,
Reviewed-by: Sai Prakash Ranjan
Tested-by: Sai Prakash Ranjan
Thanks,
Sai
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member
of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH 1/6] arm64: dts: qcom: sm8150: add apps_smmu node
On 2020-06-05 19:40, Jonathan Marek wrote:
On 6/5/20 10:03 AM, Sai Prakash Ranjan wrote:
On 2020-05-29 08:45, Bjorn Andersson wrote:
On Thu 28 May 20:02 PDT 2020, Jonathan Marek wrote:
On 5/28/20 10:52 PM, Bjorn Andersson wrote:
> On Sat 23 May 19:38 PDT 2020, Jonathan Marek wrote:
>
> > Add the apps_smmu node for sm8150. Note that adding the iommus field for
> > UFS is required because initializing the iommu removes the bypass mapping
> > that created by the bootloader.
> >
>
> Unrelated to the patch itself; how do you disable the splash screen on
> 8150? "fastboot oem select-display-panel none" doesn't seem to work for
> me on the MTP - and hence this would prevent my device from booting.
>
> Thanks,
> Bjorn
>
I don't have a MTP, but on HDK855, "fastboot oem
select-display-panel none"
combined with setting the physical switch to HDMI mode (which
switches off
the 1440x2560 panel) gets it to not setup the display at all (just
the
fastboot command isn't enough).
Okay, I don't think we have anything equivalent on the MTP, but good
to
know.
Actually I tried out this in SM8150 MTP and it works fine for me,
"fastboot set_active a; fastboot set_active b; fastboot set_active a;
fastboot oem select-display-panel none; fastboot reboot bootloader;
fastboot boot boot-sm8150.img"
Also I need to switch slots everytime like above, otherwise I always
see some error
while loading the boot image.
What is the error? If it is "FAILED (remote: Failed to
load/authenticate boot image: Load Error)" then flashing/erasing
boot_a will make it go away ("fastboot erase boot_a") for the next 6
or so "failed" boots.
Yes this exact error.
-Sai
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member
of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH 1/6] arm64: dts: qcom: sm8150: add apps_smmu node
On 2020-05-29 08:45, Bjorn Andersson wrote: On Thu 28 May 20:02 PDT 2020, Jonathan Marek wrote: On 5/28/20 10:52 PM, Bjorn Andersson wrote: > On Sat 23 May 19:38 PDT 2020, Jonathan Marek wrote: > > > Add the apps_smmu node for sm8150. Note that adding the iommus field for > > UFS is required because initializing the iommu removes the bypass mapping > > that created by the bootloader. > > > > Unrelated to the patch itself; how do you disable the splash screen on > 8150? "fastboot oem select-display-panel none" doesn't seem to work for > me on the MTP - and hence this would prevent my device from booting. > > Thanks, > Bjorn > I don't have a MTP, but on HDK855, "fastboot oem select-display-panel none" combined with setting the physical switch to HDMI mode (which switches off the 1440x2560 panel) gets it to not setup the display at all (just the fastboot command isn't enough). Okay, I don't think we have anything equivalent on the MTP, but good to know. Actually I tried out this in SM8150 MTP and it works fine for me, "fastboot set_active a; fastboot set_active b; fastboot set_active a; fastboot oem select-display-panel none; fastboot reboot bootloader; fastboot boot boot-sm8150.img" Also I need to switch slots everytime like above, otherwise I always see some error while loading the boot image. Thanks, Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH 2/2] coresight: tmc: Add shutdown callback for TMC ETR/ETF
Hi Mathieu, +Will On 2020-06-03 23:14, Mathieu Poirier wrote: On Wed, Jun 03, 2020 at 02:34:10PM +0100, Robin Murphy wrote: On 2020-06-03 14:22, Mike Leach wrote: > Hi Sai, > > On Wed, 3 Jun 2020 at 13:14, Sai Prakash Ranjan > wrote: > > > > Hi Mike, > > > > On 2020-06-03 16:57, Mike Leach wrote: > > > Hi, > > > > > > On Wed, 3 Jun 2020 at 11:24, Sai Prakash Ranjan > > > wrote: > > > > > > > > Hi Mike, > > > > > > > > Thanks again for looking at this. > > > > > > > > On 2020-06-03 03:42, Mike Leach wrote: > > > > [...] > > > > > > > > > > > > > > > > SMMU/IOMMU won't be able to do much here as it is the client's > > > > > > responsiblity to > > > > > > properly shutdown and SMMU device link just makes sure that > > > > > > SMMU(supplier) shutdown is > > > > > > called only after its consumers shutdown callbacks are called. > > > > > > > > > > I think this use case can be handled slightly differently than the > > > > > general requirements for modular CoreSight drivers. > > > > > > > > > > What is needed here is a way of stopping the underlying ETR hardware > > > > > from issuing data to the SMMU, until the entire device has been shut > > > > > down, in a way that does not remove the driver, breaking existing > > > > > references and causing a system crash. > > > > > > > > > > We could introduce a new mode to the ETR driver - e.g. > > > > > CS_MODE_SHUTDOWN. > > > > > > > > > > At the end of the block tmc_shutdown(struct amba_device *adev), set > > > > > drvdata->mode to CS_MODE_SHUTDOWN & remove the coresight_unregister(). > > > > > This new mode can be used to prevent the underlying hardware from > > > > > being able to restart until the device is re-powered. > > > > > > > > > > This mode can be detected in the code that enables / disables the ETR > > > > > and handled appropriately (updates to tmc_enable_etr_sink and > > > > > tmc_disable_etr_sink). > > > > > This mode will persist until the device is re-started - but because we > > > > > are on the device shutdown path this is not an issue. > > > > > > > > > > This should leave the CoreSight infrastructure stable until the > > > > > drivers are shut down normally as part of the device power down > > > > > process. > > > > > > > > > > > > > Sounds good to me, but if the coresight_unregister() is the trouble > > > > point > > > > causing these crashes, then can't we just remove that from > > > > tmc_shutdown() > > > > callback? This would be like maintaining the same behaviour as now > > > > where > > > > on reboot/shutdown we basically don't do anything except for disabling > > > > ETR. > > > > > > No - the new mode prevents race conditions where the thread shutting > > > down the SMMU does the ETR shutdown, but then another thread happens > > > to be trying to start trace and restarts the ETR. > > > It also prevents the condition Mathieu discussed where a thread might > > > be attempting to shutdown trace - this could try to disable the > > > hardware again re-releasing resources/ re-flushing and waiting for > > > stop. > > > > > > > I do not think there will a race between SMMU shutdown and ETR shutdown. > > Driver core takes care of calling SMMU shutdown after its consumer > > shutdown callbacks via device link, otherwise there would already be > > bugs in all other client drivers. > > > > I am not saying there could be a race between tmc_shutdowm and > Smmu_shutdown - there may be a case if the coresight_disable_path > sequence is running and gets to the point of disabling the ETR after > the SMMU callback has disabled it. I'm confused now - there is no "SMMU callback", we're talking about the system-wide cleanup from kernel_shutdown_prepare() or kernel_restart_prepare(). As far as I'm aware userspace should be long gone by that point, so although trace may have been left running, the chance of racing against other driver operations seems pretty unlikely. Robin has a point - user space is long gone at this time. As such the first question to ask is what kind of CS session was running at the time the system was shutting down. Was it a perf session of a sysfs session? I'm guessing it was a sysfs session because user space has been blown away a while back and part of that process should have killed all perf sessions. I was enabling trace via sysfs. If I am correct then simply switching off the ETR HW in the shutdown() amba bus callback should be fine - otherwise Mike's approach is mandatory. There is also the exchange between Robin and Sai about removing the SMMU shutdown callback, but that thread is still incomplete. If Robin is hinting at removing SMMU shutdown callback, then I think adding all these shutdown callbacks to all clients of SMMU can be avoided. Git blaming the thing shows it was added to avoid some kexec memory corruption. Thanks, Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH 08/10] clk: qcom: Add graphics clock controller driver for SM8250
Hi Bjorn, On 2020-06-03 23:39, Bjorn Andersson wrote: On Thu 28 May 23:56 PDT 2020, Sai Prakash Ranjan wrote: Hi Bjorn, On 2020-05-29 06:41, Bjorn Andersson wrote: > On Mon 25 May 02:47 PDT 2020, Sai Prakash Ranjan wrote: > > > Hi Jonathan, > > > > On 2020-05-25 02:36, Jonathan Marek wrote: > > > Add support for the graphics clock controller found on SM8250 > > > based devices. This would allow graphics drivers to probe and > > > control their clocks. > > > > > > This is copied from the downstream kernel, adapted for upstream. > > > For example, GDSCs have been added. > > > > > > Signed-off-by: Jonathan Marek > > > > Since this is taken from downstream, maintain the original author's > > signed-off and add yourself as the co-developer if you have done > > any modifications. Same applies to all other patches. > > > > I disagree with this. > > As expressed in the commit message, this patch is based on the > downstream driver, not the individual patch. As such, the _patch_ is > prepared by Jonathan and by his Signed-off-by certifies the origin of > the contribution per section 11.a or 11.b of submitting-patches.rst. > I lost at the downstream driver vs the individual patch here. So the downstream driver is also an individual patch right or did I get something completely wrong. The downstream driver is the result of a series of patches, by various people, whom all use their Signed-off-by to denote that what they add is conforming to the given license and that they have permission to contribute to the project. So if someone prepares a patch and includes a commit description saying it is taken from downstream, does it mean he is the author of that patch? No, but I think the wording here is wrong. The patch is not taken from downstream, it's based on downstream code. Shouldn't the author be included in "From: Author" and his signed-off appear first before the submitter's(also a contributor) signed-off? It should, in the case that what is contributed is the forwarding of a patch found somewhere. But as I said before, Jonathan does through his S-o-b state that his patch is based on previous work that is covered under appropriate open source license and that he has the right under that license to contribute said work. As such, his patch is meeting the requirements. The other part is how to give credit to authors of the original work, Jonathan does that by stating that it's based on work in the downstream kernel - which is quite typical to how it's done. Or is it because these clock data is auto generated and it doesnt really matter? No. The author and s-o-b relates to license compliance, as such the person who committed the auto generated work will sign off that the content is license compliant and he/she is allowed to contribute it to the project. Thanks for these nice explanations. Regards, Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH 2/2] coresight: tmc: Add shutdown callback for TMC ETR/ETF
Hi Mike, On 2020-06-03 19:21, Mike Leach wrote: Hi, On Wed, 3 Jun 2020 at 14:34, Robin Murphy wrote: On 2020-06-03 14:22, Mike Leach wrote: > Hi Sai, > > On Wed, 3 Jun 2020 at 13:14, Sai Prakash Ranjan > wrote: >> >> Hi Mike, >> >> On 2020-06-03 16:57, Mike Leach wrote: >>> Hi, >>> >>> On Wed, 3 Jun 2020 at 11:24, Sai Prakash Ranjan >>> wrote: >>>> >>>> Hi Mike, >>>> >>>> Thanks again for looking at this. >>>> >>>> On 2020-06-03 03:42, Mike Leach wrote: >>>> [...] >>>> >>>>>> >>>>>> SMMU/IOMMU won't be able to do much here as it is the client's >>>>>> responsiblity to >>>>>> properly shutdown and SMMU device link just makes sure that >>>>>> SMMU(supplier) shutdown is >>>>>> called only after its consumers shutdown callbacks are called. >>>>> >>>>> I think this use case can be handled slightly differently than the >>>>> general requirements for modular CoreSight drivers. >>>>> >>>>> What is needed here is a way of stopping the underlying ETR hardware >>>>> from issuing data to the SMMU, until the entire device has been shut >>>>> down, in a way that does not remove the driver, breaking existing >>>>> references and causing a system crash. >>>>> >>>>> We could introduce a new mode to the ETR driver - e.g. >>>>> CS_MODE_SHUTDOWN. >>>>> >>>>> At the end of the block tmc_shutdown(struct amba_device *adev), set >>>>> drvdata->mode to CS_MODE_SHUTDOWN & remove the coresight_unregister(). >>>>> This new mode can be used to prevent the underlying hardware from >>>>> being able to restart until the device is re-powered. >>>>> >>>>> This mode can be detected in the code that enables / disables the ETR >>>>> and handled appropriately (updates to tmc_enable_etr_sink and >>>>> tmc_disable_etr_sink). >>>>> This mode will persist until the device is re-started - but because we >>>>> are on the device shutdown path this is not an issue. >>>>> >>>>> This should leave the CoreSight infrastructure stable until the >>>>> drivers are shut down normally as part of the device power down >>>>> process. >>>>> >>>> >>>> Sounds good to me, but if the coresight_unregister() is the trouble >>>> point >>>> causing these crashes, then can't we just remove that from >>>> tmc_shutdown() >>>> callback? This would be like maintaining the same behaviour as now >>>> where >>>> on reboot/shutdown we basically don't do anything except for disabling >>>> ETR. >>> >>> No - the new mode prevents race conditions where the thread shutting >>> down the SMMU does the ETR shutdown, but then another thread happens >>> to be trying to start trace and restarts the ETR. >>> It also prevents the condition Mathieu discussed where a thread might >>> be attempting to shutdown trace - this could try to disable the >>> hardware again re-releasing resources/ re-flushing and waiting for >>> stop. >>> >> >> I do not think there will a race between SMMU shutdown and ETR shutdown. >> Driver core takes care of calling SMMU shutdown after its consumer >> shutdown callbacks via device link, otherwise there would already be >> bugs in all other client drivers. >> > > I am not saying there could be a race between tmc_shutdowm and > Smmu_shutdown - there may be a case if the coresight_disable_path > sequence is running and gets to the point of disabling the ETR after > the SMMU callback has disabled it. I'm confused now - there is no "SMMU callback", we're talking about the system-wide cleanup from kernel_shutdown_prepare() or kernel_restart_prepare(). As far as I'm aware userspace should be long gone by that point, so although trace may have been left running || ((offset >= TRCCIDCVRn(0)) && (offset <= TRCVMIDCVRn(7)), the chance of racing against other driver operations seems pretty unlikely. Sorry - bad choice of terminology. I was referring to the SMMU ensuring that it had all its clients shut-down before if shut down. To quote Sai... SMMU device link just makes sure that >>>>>> SMMU(supplier) shutdown is >>>>>> called only after
Re: [PATCH 2/2] coresight: tmc: Add shutdown callback for TMC ETR/ETF
Hi Robin, On 2020-06-03 19:10, Robin Murphy wrote: On 2020-06-03 13:26, Sai Prakash Ranjan wrote: Hi Robin, On 2020-06-03 17:51, Robin Murphy wrote: On 2020-06-03 13:00, Sai Prakash Ranjan wrote: Hi Robin, Mathieu On 2020-06-03 17:07, Robin Murphy wrote: On 2020-06-01 22:28, Mathieu Poirier wrote: That being said I'm sure that dependencies on an IOMMU isn't a problem confined to coresight. I am adding Robin Murphy, who added this commit [1], to the thread in the hope that he can provide guidance on the right way to do this. Right, it's not specific to CoreSight, and it's not even specific to IOMMUs really. In short, blame kexec ;) Yes it is not specific to coresight, we are targeting this for all consumers/clients of SMMU(atleast on SC7180 SoC). We have display throwing NoC/interconnect errors[1] during reboot after SMMU is disabled. This is also not specific to kexec either as you explained here [2] about a case with display which is exacly what is happening in our system [1]. Sure, but those instances are begging the question of why the SMMU is disabled at reboot in the first place ;) That is what happens in SMMU shutdown callback right? It is the reboot/shutdown flow. Yes, that's where it happens, but my point is *why* it happens at all. hint: `git log --grep=shutdown drivers/iommu/` Ah my change :) If we could assume the system is always about to be powered off or reset, we wouldn't need to do anything to the SMMU either ;) Are you hinting at removing SMMU shutdown callback altogether ;) Thanks, Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH 2/2] coresight: tmc: Add shutdown callback for TMC ETR/ETF
Hi Mike, On 2020-06-03 19:04, Robin Murphy wrote: On 2020-06-03 14:22, Mike Leach wrote: Hi Sai, On Wed, 3 Jun 2020 at 13:14, Sai Prakash Ranjan wrote: Hi Mike, On 2020-06-03 16:57, Mike Leach wrote: Hi, On Wed, 3 Jun 2020 at 11:24, Sai Prakash Ranjan wrote: Hi Mike, Thanks again for looking at this. On 2020-06-03 03:42, Mike Leach wrote: [...] SMMU/IOMMU won't be able to do much here as it is the client's responsiblity to properly shutdown and SMMU device link just makes sure that SMMU(supplier) shutdown is called only after its consumers shutdown callbacks are called. I think this use case can be handled slightly differently than the general requirements for modular CoreSight drivers. What is needed here is a way of stopping the underlying ETR hardware from issuing data to the SMMU, until the entire device has been shut down, in a way that does not remove the driver, breaking existing references and causing a system crash. We could introduce a new mode to the ETR driver - e.g. CS_MODE_SHUTDOWN. At the end of the block tmc_shutdown(struct amba_device *adev), set drvdata->mode to CS_MODE_SHUTDOWN & remove the coresight_unregister(). This new mode can be used to prevent the underlying hardware from being able to restart until the device is re-powered. This mode can be detected in the code that enables / disables the ETR and handled appropriately (updates to tmc_enable_etr_sink and tmc_disable_etr_sink). This mode will persist until the device is re-started - but because we are on the device shutdown path this is not an issue. This should leave the CoreSight infrastructure stable until the drivers are shut down normally as part of the device power down process. Sounds good to me, but if the coresight_unregister() is the trouble point causing these crashes, then can't we just remove that from tmc_shutdown() callback? This would be like maintaining the same behaviour as now where on reboot/shutdown we basically don't do anything except for disabling ETR. No - the new mode prevents race conditions where the thread shutting down the SMMU does the ETR shutdown, but then another thread happens to be trying to start trace and restarts the ETR. It also prevents the condition Mathieu discussed where a thread might be attempting to shutdown trace - this could try to disable the hardware again re-releasing resources/ re-flushing and waiting for stop. I do not think there will a race between SMMU shutdown and ETR shutdown. Driver core takes care of calling SMMU shutdown after its consumer shutdown callbacks via device link, otherwise there would already be bugs in all other client drivers. I am not saying there could be a race between tmc_shutdowm and Smmu_shutdown - there may be a case if the coresight_disable_path sequence is running and gets to the point of disabling the ETR after the SMMU callback has disabled it. I'm confused now - there is no "SMMU callback", we're talking about the system-wide cleanup from kernel_shutdown_prepare() or kernel_restart_prepare(). As far as I'm aware userspace should be long gone by that point, so although trace may have been left running, the chance of racing against other driver operations seems pretty unlikely. As Robin said, it is not SMMU callback but the normal reboot/shutdown flow and race is unlikely at that point. tmc_shutdown() platform_drv_shutdown() device_shutdown() kernel_restart_prepare() kernel_restart() If I am not clear enough, first all the consumer shutdown callbacks of SMMU are called like above tmc_shutdown() and then we call the arm_smmu_device_shutdown(), this ordering is ensured by the device links. Thanks, Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH 2/2] coresight: tmc: Add shutdown callback for TMC ETR/ETF
Hi Robin, On 2020-06-03 17:51, Robin Murphy wrote: On 2020-06-03 13:00, Sai Prakash Ranjan wrote: Hi Robin, Mathieu On 2020-06-03 17:07, Robin Murphy wrote: On 2020-06-01 22:28, Mathieu Poirier wrote: That being said I'm sure that dependencies on an IOMMU isn't a problem confined to coresight. I am adding Robin Murphy, who added this commit [1], to the thread in the hope that he can provide guidance on the right way to do this. Right, it's not specific to CoreSight, and it's not even specific to IOMMUs really. In short, blame kexec ;) Yes it is not specific to coresight, we are targeting this for all consumers/clients of SMMU(atleast on SC7180 SoC). We have display throwing NoC/interconnect errors[1] during reboot after SMMU is disabled. This is also not specific to kexec either as you explained here [2] about a case with display which is exacly what is happening in our system [1]. Sure, but those instances are begging the question of why the SMMU is disabled at reboot in the first place ;) That is what happens in SMMU shutdown callback right? It is the reboot/shutdown flow. arm_smmu_device_shutdown() platform_drv_shutdown() device_shutdown() kernel_restart_prepare() kernel_restart() Thanks, Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH 2/2] coresight: tmc: Add shutdown callback for TMC ETR/ETF
Hi Mike, On 2020-06-03 16:57, Mike Leach wrote: Hi, On Wed, 3 Jun 2020 at 11:24, Sai Prakash Ranjan wrote: Hi Mike, Thanks again for looking at this. On 2020-06-03 03:42, Mike Leach wrote: [...] >> >> SMMU/IOMMU won't be able to do much here as it is the client's >> responsiblity to >> properly shutdown and SMMU device link just makes sure that >> SMMU(supplier) shutdown is >> called only after its consumers shutdown callbacks are called. > > I think this use case can be handled slightly differently than the > general requirements for modular CoreSight drivers. > > What is needed here is a way of stopping the underlying ETR hardware > from issuing data to the SMMU, until the entire device has been shut > down, in a way that does not remove the driver, breaking existing > references and causing a system crash. > > We could introduce a new mode to the ETR driver - e.g. > CS_MODE_SHUTDOWN. > > At the end of the block tmc_shutdown(struct amba_device *adev), set > drvdata->mode to CS_MODE_SHUTDOWN & remove the coresight_unregister(). > This new mode can be used to prevent the underlying hardware from > being able to restart until the device is re-powered. > > This mode can be detected in the code that enables / disables the ETR > and handled appropriately (updates to tmc_enable_etr_sink and > tmc_disable_etr_sink). > This mode will persist until the device is re-started - but because we > are on the device shutdown path this is not an issue. > > This should leave the CoreSight infrastructure stable until the > drivers are shut down normally as part of the device power down > process. > Sounds good to me, but if the coresight_unregister() is the trouble point causing these crashes, then can't we just remove that from tmc_shutdown() callback? This would be like maintaining the same behaviour as now where on reboot/shutdown we basically don't do anything except for disabling ETR. No - the new mode prevents race conditions where the thread shutting down the SMMU does the ETR shutdown, but then another thread happens to be trying to start trace and restarts the ETR. It also prevents the condition Mathieu discussed where a thread might be attempting to shutdown trace - this could try to disable the hardware again re-releasing resources/ re-flushing and waiting for stop. I do not think there will a race between SMMU shutdown and ETR shutdown. Driver core takes care of calling SMMU shutdown after its consumer shutdown callbacks via device link, otherwise there would already be bugs in all other client drivers. Thanks, Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH 2/2] coresight: tmc: Add shutdown callback for TMC ETR/ETF
Hi Robin, Mathieu On 2020-06-03 17:07, Robin Murphy wrote: On 2020-06-01 22:28, Mathieu Poirier wrote: That being said I'm sure that dependencies on an IOMMU isn't a problem confined to coresight. I am adding Robin Murphy, who added this commit [1], to the thread in the hope that he can provide guidance on the right way to do this. Right, it's not specific to CoreSight, and it's not even specific to IOMMUs really. In short, blame kexec ;) Yes it is not specific to coresight, we are targeting this for all consumers/clients of SMMU(atleast on SC7180 SoC). We have display throwing NoC/interconnect errors[1] during reboot after SMMU is disabled. This is also not specific to kexec either as you explained here [2] about a case with display which is exacly what is happening in our system [1]. [1] https://lore.kernel.org/lkml/1591009402-681-1-git-send-email-mkri...@codeaurora.org/ [2] https://lore.kernel.org/lkml/5858bdac-b7f9-ac26-0c0d-c9653cef8...@arm.com/ The fundamental thing is that devices should stop any DMA activity at shutdown. For a normal poweroff you can typically get away without doing so, but over kexec, ongoing DMA traffic may corrupt memory in the new kernel (at worst, I think even DMA reads could potentially cause unexpected cache behaviour that might lead to mishaps, given the right combination of memory attributes). IOMMUs merely help to make the situation more serious. For similar kexec reasons, they need to disable any existing translations at shutdown (imagine if the second kernel didn't have an IOMMU driver). And at that point, even the normal poweroff case becomes problematic, because any device DMA that hasn't been shut down beforehand is now not necessarily going benignly to memory as it would in the no-IOMMU case above, but potentially to random physical addresses, with all the hilarity ensuing that you would expect from that. Thanks, Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH 2/2] coresight: tmc: Add shutdown callback for TMC ETR/ETF
Hi Mike,
Thanks again for looking at this.
On 2020-06-03 03:42, Mike Leach wrote:
[...]
SMMU/IOMMU won't be able to do much here as it is the client's
responsiblity to
properly shutdown and SMMU device link just makes sure that
SMMU(supplier) shutdown is
called only after its consumers shutdown callbacks are called.
I think this use case can be handled slightly differently than the
general requirements for modular CoreSight drivers.
What is needed here is a way of stopping the underlying ETR hardware
from issuing data to the SMMU, until the entire device has been shut
down, in a way that does not remove the driver, breaking existing
references and causing a system crash.
We could introduce a new mode to the ETR driver - e.g.
CS_MODE_SHUTDOWN.
At the end of the block tmc_shutdown(struct amba_device *adev), set
drvdata->mode to CS_MODE_SHUTDOWN & remove the coresight_unregister().
This new mode can be used to prevent the underlying hardware from
being able to restart until the device is re-powered.
This mode can be detected in the code that enables / disables the ETR
and handled appropriately (updates to tmc_enable_etr_sink and
tmc_disable_etr_sink).
This mode will persist until the device is re-started - but because we
are on the device shutdown path this is not an issue.
This should leave the CoreSight infrastructure stable until the
drivers are shut down normally as part of the device power down
process.
Sounds good to me, but if the coresight_unregister() is the trouble
point
causing these crashes, then can't we just remove that from
tmc_shutdown()
callback? This would be like maintaining the same behaviour as now where
on reboot/shutdown we basically don't do anything except for disabling
ETR.
This way, we do not have to introduce any new mode as well. To be exact,
in
tmc_shutdown() we just disable ETR and then return without unregistering
which should not cause any issues since this is shutdown not the remove
callback which is a requirement for making coresight modular like below:
static void tmc_shutdown(struct amba_device *adev)
{
unsigned long flags;
struct tmc_drvdata *drvdata = amba_get_drvdata(adev);
spin_lock_irqsave(&drvdata->spinlock, flags);
if (drvdata->mode == CS_MODE_DISABLED)
goto out;
if (drvdata->config_type == TMC_CONFIG_TYPE_ETR)
tmc_etr_disable_hw(drvdata);
/*
* We do not care about coresight unregister here unlike remove
* callback which is required for making coresight modular
since
* the system is going down after this.
*/
out:
spin_unlock_irqrestore(&drvdata->spinlock, flags);
}
Thanks,
Sai
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member
of Code Aurora Forum, hosted by The Linux Foundation
Re: [v2] drm/msm: add shutdown support for display platform_driver
Hi Emil, On 2020-06-02 21:09, Emil Velikov wrote: On Tue, 2 Jun 2020 at 15:49, Sai Prakash Ranjan wrote: Hi Emil, On 2020-06-02 19:43, Emil Velikov wrote: > Hi Krishna, > > On Tue, 2 Jun 2020 at 08:17, Krishna Manikandan > wrote: >> >> Define shutdown callback for display drm driver, >> so as to disable all the CRTCS when shutdown >> notification is received by the driver. >> >> This change will turn off the timing engine so >> that no display transactions are requested >> while mmu translations are getting disabled >> during reboot sequence. >> >> Signed-off-by: Krishna Manikandan >> > AFAICT atomics is setup in msm_drm_ops::bind and shutdown in > msm_drm_ops::unbind. > > Are you saying that unbind never triggers? If so, then we should > really fix that instead, since this patch seems more like a > workaround. > Which path do you suppose that the unbind should be called from, remove callback? Here we are talking about the drivers which are builtin, where remove callbacks are not called from the driver core during reboot/shutdown, instead shutdown callbacks are called which needs to be defined in order to trigger unbind. So AFAICS there is nothing to be fixed. Interesting - in drm effectively only drm panels implement .shutdown. So my naive assumption was that .remove was used implicitly by core, as part of the shutdown process. Yet that's not the case, so it seems that many drivers could use some fixes. Then again, that's an existing problem which is irrelevant for msm. -Emil To give more context, we are actually targeting the clients/consumers of SMMU/IOMMU here because we have to make sure that before the supplier (SMMU) shuts down, its consumers/clients need to be shutdown properly. Now the ordering of this is taken care in the SMMU driver via device_link which makes sure that consumer shutdown callbacks are called first, but we need to define shutdown callbacks for all its consumers to make sure we actually shutdown the clients or else it would invite the crashes during reboot which in this case was seen for display. Thanks, Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [v2] drm/msm: add shutdown support for display platform_driver
Hi Emil, On 2020-06-02 19:43, Emil Velikov wrote: Hi Krishna, On Tue, 2 Jun 2020 at 08:17, Krishna Manikandan wrote: Define shutdown callback for display drm driver, so as to disable all the CRTCS when shutdown notification is received by the driver. This change will turn off the timing engine so that no display transactions are requested while mmu translations are getting disabled during reboot sequence. Signed-off-by: Krishna Manikandan AFAICT atomics is setup in msm_drm_ops::bind and shutdown in msm_drm_ops::unbind. Are you saying that unbind never triggers? If so, then we should really fix that instead, since this patch seems more like a workaround. Which path do you suppose that the unbind should be called from, remove callback? Here we are talking about the drivers which are builtin, where remove callbacks are not called from the driver core during reboot/shutdown, instead shutdown callbacks are called which needs to be defined in order to trigger unbind. So AFAICS there is nothing to be fixed. msm_pdev_shutdown() platform_drv_shutdown() device_shutdown() kernel_restart_prepare() kernel_restart() __arm64_sys_reboot() Thanks, Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH 2/2] coresight: tmc: Add shutdown callback for TMC ETR/ETF
Hi Mathieu,
Thanks for taking your time for review.
On 2020-06-02 02:58, Mathieu Poirier wrote:
Hi Sai,
On top of the comments already privided by Mike, I have the following:
On Mon, Jun 01, 2020 at 01:32:26PM +0530, Sai Prakash Ranjan wrote:
Implement a shutdown callback to ensure ETR/ETF hardware is
properly shutdown in reboot/shutdown path. This is required
for ETR/ETF which has SMMU address translation enabled like
on SC7180 SoC and few others. If the hardware is still accessing
memory after SMMU translation is disabled as part of SMMU
shutdown callback in system reboot or shutdown path, then
IOVAs(I/O virtual address) which it was using will go on the bus
as the physical addresses which might result in unknown crashes
(NoC/interconnect errors). So we make sure from this shutdown
callback that the ETR/ETF is shutdown before SMMU translation is
disabled and device_link in SMMU driver will take care of ordering
of shutdown callbacks such that SMMU shutdown callback is not
called before any of its consumer shutdown callbacks.
Signed-off-by: Sai Prakash Ranjan
---
.../hwtracing/coresight/coresight-tmc-etf.c | 4 +--
.../hwtracing/coresight/coresight-tmc-etr.c | 2 +-
drivers/hwtracing/coresight/coresight-tmc.c | 29
+++
drivers/hwtracing/coresight/coresight-tmc.h | 3 ++
4 files changed, 35 insertions(+), 3 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c
b/drivers/hwtracing/coresight/coresight-tmc-etf.c
index 36cce2bfb744..cba3e7592820 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
@@ -85,7 +85,7 @@ static void __tmc_etb_disable_hw(struct tmc_drvdata
*drvdata)
CS_LOCK(drvdata->base);
}
-static void tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
+void tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
{
__tmc_etb_disable_hw(drvdata);
coresight_disclaim_device(drvdata->base);
@@ -118,7 +118,7 @@ static int tmc_etf_enable_hw(struct tmc_drvdata
*drvdata)
return 0;
}
-static void tmc_etf_disable_hw(struct tmc_drvdata *drvdata)
+void tmc_etf_disable_hw(struct tmc_drvdata *drvdata)
{
CS_UNLOCK(drvdata->base);
Why do we care about ETB and ETF when they both use RAM internal to the
device?
Moreover, the system RAM they use is not dedicated and as such falls
back to the
kernel's memory pool.
Actually we don't, I added the disable for ETF/ETB for completeness
since we are
adding shutdown callback for TMC devices and not just ETR although this
issue applies
only for ETR and it doesn't hurt to disable these devices in shutdown
path.
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c
b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index 625882bc8b08..b29c2db94d96 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -1110,7 +1110,7 @@ static void __tmc_etr_disable_hw(struct
tmc_drvdata *drvdata)
}
-static void tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
+void tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
{
__tmc_etr_disable_hw(drvdata);
/* Disable CATU device if this ETR is connected to one */
diff --git a/drivers/hwtracing/coresight/coresight-tmc.c
b/drivers/hwtracing/coresight/coresight-tmc.c
index 5a271ebc4585..7e687a356fe0 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.c
+++ b/drivers/hwtracing/coresight/coresight-tmc.c
@@ -540,6 +540,34 @@ static int tmc_probe(struct amba_device *adev,
const struct amba_id *id)
return ret;
}
+static void tmc_shutdown(struct amba_device *adev)
+{
+ struct tmc_drvdata *drvdata = amba_get_drvdata(adev);
+
+ if (!drvdata->enable)
+ goto out;
+
+ /*
+* We do not care about the active trace sessions here
+* since the system is going down unlike remove callback,
+* just make sure that the hardware is shutdown.
+*/
+ switch (drvdata->config_type) {
+ case TMC_CONFIG_TYPE_ETB:
+ tmc_etb_disable_hw(drvdata);
+ break;
+ case TMC_CONFIG_TYPE_ETF:
+ tmc_etf_disable_hw(drvdata);
+ break;
+ case TMC_CONFIG_TYPE_ETR:
+ tmc_etr_disable_hw(drvdata);
+ }
+
+out:
+ misc_deregister(&drvdata->miscdev);
+ coresight_unregister(drvdata->csdev);
If a session is active when tmc_shutdown() is called, unregistering the
ETF/ETR
will result in a kernel crash if the session is stopped before the
kernel has
had the opportunity to shutdown. It is the problem as trying to make
coresight
drivers modular.
For this to really work the ongoing session would need to be stopped.
That
would teardown the path and stop the sink.
I have tested this with and without active trace sessions multiple times
on 2 devices
and did not observe a single crash. The crash should be easily tri
[PATCHv2] coresight: tmc: Add shutdown callback for TMC ETR/ETF
Implement a shutdown callback to ensure ETR/ETF hardware is
properly shutdown in reboot/shutdown path. This is required
for ETR/ETF which has SMMU address translation enabled like
on SC7180 SoC and few others. If the hardware is still accessing
memory after SMMU translation is disabled as part of SMMU
shutdown callback in system reboot or shutdown path, then
IOVAs(I/O virtual address) which it was using will go on the bus
as the physical addresses which might result in unknown crashes
(NoC/interconnect errors). So we make sure from this shutdown
callback that the ETR/ETF is shutdown before SMMU translation is
disabled and device_link in SMMU driver will take care of ordering
of shutdown callbacks such that SMMU shutdown callback is not
called before any of its consumer shutdown callbacks.
Signed-off-by: Sai Prakash Ranjan
---
Changes since v1:
* Use mode flag and drop enable flag as Mike suggested.
* Use spinlock before tmc hw disable as Mike suggested.
---
.../hwtracing/coresight/coresight-tmc-etf.c | 4 +--
.../hwtracing/coresight/coresight-tmc-etr.c | 2 +-
drivers/hwtracing/coresight/coresight-tmc.c | 33 +++
drivers/hwtracing/coresight/coresight-tmc.h | 3 ++
4 files changed, 39 insertions(+), 3 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c
b/drivers/hwtracing/coresight/coresight-tmc-etf.c
index 36cce2bfb744..cba3e7592820 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
@@ -85,7 +85,7 @@ static void __tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
CS_LOCK(drvdata->base);
}
-static void tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
+void tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
{
__tmc_etb_disable_hw(drvdata);
coresight_disclaim_device(drvdata->base);
@@ -118,7 +118,7 @@ static int tmc_etf_enable_hw(struct tmc_drvdata *drvdata)
return 0;
}
-static void tmc_etf_disable_hw(struct tmc_drvdata *drvdata)
+void tmc_etf_disable_hw(struct tmc_drvdata *drvdata)
{
CS_UNLOCK(drvdata->base);
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c
b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index 625882bc8b08..b29c2db94d96 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -1110,7 +1110,7 @@ static void __tmc_etr_disable_hw(struct tmc_drvdata
*drvdata)
}
-static void tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
+void tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
{
__tmc_etr_disable_hw(drvdata);
/* Disable CATU device if this ETR is connected to one */
diff --git a/drivers/hwtracing/coresight/coresight-tmc.c
b/drivers/hwtracing/coresight/coresight-tmc.c
index 39fba1d16e6e..c7902ce0bd3b 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.c
+++ b/drivers/hwtracing/coresight/coresight-tmc.c
@@ -538,6 +538,38 @@ static int tmc_probe(struct amba_device *adev, const
struct amba_id *id)
return ret;
}
+static void tmc_shutdown(struct amba_device *adev)
+{
+ unsigned long flags;
+ struct tmc_drvdata *drvdata = amba_get_drvdata(adev);
+
+ spin_lock_irqsave(&drvdata->spinlock, flags);
+
+ if (drvdata->mode == CS_MODE_DISABLED)
+ goto out;
+
+ /*
+* We do not care about the active trace sessions here
+* since the system is going down unlike remove callback,
+* just make sure that the hardware is shutdown.
+*/
+ switch (drvdata->config_type) {
+ case TMC_CONFIG_TYPE_ETB:
+ tmc_etb_disable_hw(drvdata);
+ break;
+ case TMC_CONFIG_TYPE_ETF:
+ tmc_etf_disable_hw(drvdata);
+ break;
+ case TMC_CONFIG_TYPE_ETR:
+ tmc_etr_disable_hw(drvdata);
+ }
+
+out:
+ spin_unlock_irqrestore(&drvdata->spinlock, flags);
+ misc_deregister(&drvdata->miscdev);
+ coresight_unregister(drvdata->csdev);
+}
+
static const struct amba_id tmc_ids[] = {
CS_AMBA_ID(0x000bb961),
/* Coresight SoC 600 TMC-ETR/ETS */
@@ -556,6 +588,7 @@ static struct amba_driver tmc_driver = {
.suppress_bind_attrs = true,
},
.probe = tmc_probe,
+ .shutdown = tmc_shutdown,
.id_table = tmc_ids,
};
builtin_amba_driver(tmc_driver);
diff --git a/drivers/hwtracing/coresight/coresight-tmc.h
b/drivers/hwtracing/coresight/coresight-tmc.h
index 71de978575f3..0a77f2f25090 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.h
+++ b/drivers/hwtracing/coresight/coresight-tmc.h
@@ -260,6 +260,8 @@ u32 tmc_get_memwidth_mask(struct tmc_drvdata *drvdata);
/* ETB/ETF functions */
int tmc_read_prepare_etb(struct tmc_drvdata *drvdata);
int tmc_read_unprepare_etb(struct tmc_drvdata *drvdata);
+void tmc_etb_disable_hw(struct tmc_drvdata *drvdata);
+void tmc_etf_disable_hw(stru
Re: [PATCH 2/2] coresight: tmc: Add shutdown callback for TMC ETR/ETF
Hi Mike,
Thanks for the review.
On 2020-06-01 19:05, Mike Leach wrote:
Hi,
On Mon, 1 Jun 2020 at 09:02, Sai Prakash Ranjan
wrote:
Implement a shutdown callback to ensure ETR/ETF hardware is
properly shutdown in reboot/shutdown path. This is required
for ETR/ETF which has SMMU address translation enabled like
on SC7180 SoC and few others. If the hardware is still accessing
memory after SMMU translation is disabled as part of SMMU
shutdown callback in system reboot or shutdown path, then
IOVAs(I/O virtual address) which it was using will go on the bus
as the physical addresses which might result in unknown crashes
(NoC/interconnect errors). So we make sure from this shutdown
callback that the ETR/ETF is shutdown before SMMU translation is
disabled and device_link in SMMU driver will take care of ordering
of shutdown callbacks such that SMMU shutdown callback is not
called before any of its consumer shutdown callbacks.
Signed-off-by: Sai Prakash Ranjan
---
.../hwtracing/coresight/coresight-tmc-etf.c | 4 +--
.../hwtracing/coresight/coresight-tmc-etr.c | 2 +-
drivers/hwtracing/coresight/coresight-tmc.c | 29
+++
drivers/hwtracing/coresight/coresight-tmc.h | 3 ++
4 files changed, 35 insertions(+), 3 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c
b/drivers/hwtracing/coresight/coresight-tmc-etf.c
index 36cce2bfb744..cba3e7592820 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
@@ -85,7 +85,7 @@ static void __tmc_etb_disable_hw(struct tmc_drvdata
*drvdata)
CS_LOCK(drvdata->base);
}
-static void tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
+void tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
{
__tmc_etb_disable_hw(drvdata);
coresight_disclaim_device(drvdata->base);
@@ -118,7 +118,7 @@ static int tmc_etf_enable_hw(struct tmc_drvdata
*drvdata)
return 0;
}
-static void tmc_etf_disable_hw(struct tmc_drvdata *drvdata)
+void tmc_etf_disable_hw(struct tmc_drvdata *drvdata)
{
CS_UNLOCK(drvdata->base);
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c
b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index 625882bc8b08..b29c2db94d96 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -1110,7 +1110,7 @@ static void __tmc_etr_disable_hw(struct
tmc_drvdata *drvdata)
}
-static void tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
+void tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
{
__tmc_etr_disable_hw(drvdata);
/* Disable CATU device if this ETR is connected to one */
diff --git a/drivers/hwtracing/coresight/coresight-tmc.c
b/drivers/hwtracing/coresight/coresight-tmc.c
index 5a271ebc4585..7e687a356fe0 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.c
+++ b/drivers/hwtracing/coresight/coresight-tmc.c
@@ -540,6 +540,34 @@ static int tmc_probe(struct amba_device *adev,
const struct amba_id *id)
return ret;
}
+static void tmc_shutdown(struct amba_device *adev)
+{
+ struct tmc_drvdata *drvdata = amba_get_drvdata(adev);
+
Take drvdata->spinlock here? The tmc_xxx_disable_hw functions are
normally called with the spinlock claimed.
Sure will take spinlock here.
+ if (!drvdata->enable)
As per previous patch drvdata->mode can be used here.
Yes, will use mode here and drop enable flag in the next version.
Thanks,
Sai
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member
of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH 1/2] coresight: tmc: Add enable flag to indicate the status of ETR/ETF
Hi Mike,
Thanks for the review.
On 2020-06-01 18:57, Mike Leach wrote:
Hi,
On Mon, 1 Jun 2020 at 09:02, Sai Prakash Ranjan
wrote:
Add a flag to check whether TMC ETR/ETF is enabled or not.
This is later used in shutdown callback to determine if
we require to disable ETR/ETF.
Signed-off-by: Sai Prakash Ranjan
---
drivers/hwtracing/coresight/coresight-tmc.c | 2 ++
drivers/hwtracing/coresight/coresight-tmc.h | 2 ++
2 files changed, 4 insertions(+)
diff --git a/drivers/hwtracing/coresight/coresight-tmc.c
b/drivers/hwtracing/coresight/coresight-tmc.c
index 39fba1d16e6e..5a271ebc4585 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.c
+++ b/drivers/hwtracing/coresight/coresight-tmc.c
@@ -62,11 +62,13 @@ void tmc_flush_and_stop(struct tmc_drvdata
*drvdata)
void tmc_enable_hw(struct tmc_drvdata *drvdata)
{
+ drvdata->enable = true;
writel_relaxed(TMC_CTL_CAPT_EN, drvdata->base + TMC_CTL);
}
void tmc_disable_hw(struct tmc_drvdata *drvdata)
{
+ drvdata->enable = false;
writel_relaxed(0x0, drvdata->base + TMC_CTL);
}
diff --git a/drivers/hwtracing/coresight/coresight-tmc.h
b/drivers/hwtracing/coresight/coresight-tmc.h
index 71de978575f3..d156860495c7 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.h
+++ b/drivers/hwtracing/coresight/coresight-tmc.h
@@ -184,6 +184,7 @@ struct etr_buf {
* @idr_mutex: Access serialisation for idr.
* @sysfs_buf: SYSFS buffer for ETR.
* @perf_buf: PERF buffer for ETR.
+ * @enable:Indicates whether ETR/ETF is enabled.
*/
struct tmc_drvdata {
void __iomem*base;
@@ -207,6 +208,7 @@ struct tmc_drvdata {
struct mutexidr_mutex;
struct etr_buf *sysfs_buf;
struct etr_buf *perf_buf;
+ boolenable;
Is this flag needed?
For TMC, drvdata->mode indicates if the device is in use.
Yes we can use mode flag, will make this change in the next version.
Thanks,
Sai
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member
of Code Aurora Forum, hosted by The Linux Foundation
[PATCH 2/2] coresight: tmc: Add shutdown callback for TMC ETR/ETF
Implement a shutdown callback to ensure ETR/ETF hardware is
properly shutdown in reboot/shutdown path. This is required
for ETR/ETF which has SMMU address translation enabled like
on SC7180 SoC and few others. If the hardware is still accessing
memory after SMMU translation is disabled as part of SMMU
shutdown callback in system reboot or shutdown path, then
IOVAs(I/O virtual address) which it was using will go on the bus
as the physical addresses which might result in unknown crashes
(NoC/interconnect errors). So we make sure from this shutdown
callback that the ETR/ETF is shutdown before SMMU translation is
disabled and device_link in SMMU driver will take care of ordering
of shutdown callbacks such that SMMU shutdown callback is not
called before any of its consumer shutdown callbacks.
Signed-off-by: Sai Prakash Ranjan
---
.../hwtracing/coresight/coresight-tmc-etf.c | 4 +--
.../hwtracing/coresight/coresight-tmc-etr.c | 2 +-
drivers/hwtracing/coresight/coresight-tmc.c | 29 +++
drivers/hwtracing/coresight/coresight-tmc.h | 3 ++
4 files changed, 35 insertions(+), 3 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c
b/drivers/hwtracing/coresight/coresight-tmc-etf.c
index 36cce2bfb744..cba3e7592820 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
@@ -85,7 +85,7 @@ static void __tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
CS_LOCK(drvdata->base);
}
-static void tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
+void tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
{
__tmc_etb_disable_hw(drvdata);
coresight_disclaim_device(drvdata->base);
@@ -118,7 +118,7 @@ static int tmc_etf_enable_hw(struct tmc_drvdata *drvdata)
return 0;
}
-static void tmc_etf_disable_hw(struct tmc_drvdata *drvdata)
+void tmc_etf_disable_hw(struct tmc_drvdata *drvdata)
{
CS_UNLOCK(drvdata->base);
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c
b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index 625882bc8b08..b29c2db94d96 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -1110,7 +1110,7 @@ static void __tmc_etr_disable_hw(struct tmc_drvdata
*drvdata)
}
-static void tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
+void tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
{
__tmc_etr_disable_hw(drvdata);
/* Disable CATU device if this ETR is connected to one */
diff --git a/drivers/hwtracing/coresight/coresight-tmc.c
b/drivers/hwtracing/coresight/coresight-tmc.c
index 5a271ebc4585..7e687a356fe0 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.c
+++ b/drivers/hwtracing/coresight/coresight-tmc.c
@@ -540,6 +540,34 @@ static int tmc_probe(struct amba_device *adev, const
struct amba_id *id)
return ret;
}
+static void tmc_shutdown(struct amba_device *adev)
+{
+ struct tmc_drvdata *drvdata = amba_get_drvdata(adev);
+
+ if (!drvdata->enable)
+ goto out;
+
+ /*
+* We do not care about the active trace sessions here
+* since the system is going down unlike remove callback,
+* just make sure that the hardware is shutdown.
+*/
+ switch (drvdata->config_type) {
+ case TMC_CONFIG_TYPE_ETB:
+ tmc_etb_disable_hw(drvdata);
+ break;
+ case TMC_CONFIG_TYPE_ETF:
+ tmc_etf_disable_hw(drvdata);
+ break;
+ case TMC_CONFIG_TYPE_ETR:
+ tmc_etr_disable_hw(drvdata);
+ }
+
+out:
+ misc_deregister(&drvdata->miscdev);
+ coresight_unregister(drvdata->csdev);
+}
+
static const struct amba_id tmc_ids[] = {
CS_AMBA_ID(0x000bb961),
/* Coresight SoC 600 TMC-ETR/ETS */
@@ -558,6 +586,7 @@ static struct amba_driver tmc_driver = {
.suppress_bind_attrs = true,
},
.probe = tmc_probe,
+ .shutdown = tmc_shutdown,
.id_table = tmc_ids,
};
builtin_amba_driver(tmc_driver);
diff --git a/drivers/hwtracing/coresight/coresight-tmc.h
b/drivers/hwtracing/coresight/coresight-tmc.h
index d156860495c7..f4f56c474e58 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.h
+++ b/drivers/hwtracing/coresight/coresight-tmc.h
@@ -262,6 +262,8 @@ u32 tmc_get_memwidth_mask(struct tmc_drvdata *drvdata);
/* ETB/ETF functions */
int tmc_read_prepare_etb(struct tmc_drvdata *drvdata);
int tmc_read_unprepare_etb(struct tmc_drvdata *drvdata);
+void tmc_etb_disable_hw(struct tmc_drvdata *drvdata);
+void tmc_etf_disable_hw(struct tmc_drvdata *drvdata);
extern const struct coresight_ops tmc_etb_cs_ops;
extern const struct coresight_ops tmc_etf_cs_ops;
@@ -270,6 +272,7 @@ ssize_t tmc_etb_get_sysfs_trace(struct tmc_drvdata *drvdata,
/* ETR functions */
int tmc_read_prepare_etr(struct tmc_drvdata *drvdata);
int tmc_read_unprepa
[PATCH 0/2] coresight: tmc: Add shutdown callback for TMC ETR/ETF
This series adds a shutdown callback to TMC ETR/ETF to ensure that it is properly shutdown in reboot/shutdown path. This is required for ETR/ETF which has SMMU address translation enabled like on SC7180 SoC and few others. If the hardware is still accessing memory after SMMU translation is disabled as part of SMMU shutdown callback in system reboot or shutdown path, then IOVAs(I/O virtual address) which it was using will go on the bus as the physical addresses which might result in unknown crashes (NoC/interconnect errors). So we make sure from this shutdown callback that the ETR/ETF is shutdown before SMMU translation is disabled and device_link in SMMU driver will take care of ordering of shutdown callbacks such that SMMU shutdown callback is not called before any of its consumer shutdown callbacks. Sai Prakash Ranjan (2): coresight: tmc: Add enable flag to indicate the status of ETR/ETF coresight: tmc: Add shutdown callback for TMC ETR/ETF .../hwtracing/coresight/coresight-tmc-etf.c | 4 +-- .../hwtracing/coresight/coresight-tmc-etr.c | 2 +- drivers/hwtracing/coresight/coresight-tmc.c | 31 +++ drivers/hwtracing/coresight/coresight-tmc.h | 5 +++ 4 files changed, 39 insertions(+), 3 deletions(-) base-commit: 059e38815950dbec65beafe03757bce9436e89a4 -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
[PATCH 1/2] coresight: tmc: Add enable flag to indicate the status of ETR/ETF
Add a flag to check whether TMC ETR/ETF is enabled or not.
This is later used in shutdown callback to determine if
we require to disable ETR/ETF.
Signed-off-by: Sai Prakash Ranjan
---
drivers/hwtracing/coresight/coresight-tmc.c | 2 ++
drivers/hwtracing/coresight/coresight-tmc.h | 2 ++
2 files changed, 4 insertions(+)
diff --git a/drivers/hwtracing/coresight/coresight-tmc.c
b/drivers/hwtracing/coresight/coresight-tmc.c
index 39fba1d16e6e..5a271ebc4585 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.c
+++ b/drivers/hwtracing/coresight/coresight-tmc.c
@@ -62,11 +62,13 @@ void tmc_flush_and_stop(struct tmc_drvdata *drvdata)
void tmc_enable_hw(struct tmc_drvdata *drvdata)
{
+ drvdata->enable = true;
writel_relaxed(TMC_CTL_CAPT_EN, drvdata->base + TMC_CTL);
}
void tmc_disable_hw(struct tmc_drvdata *drvdata)
{
+ drvdata->enable = false;
writel_relaxed(0x0, drvdata->base + TMC_CTL);
}
diff --git a/drivers/hwtracing/coresight/coresight-tmc.h
b/drivers/hwtracing/coresight/coresight-tmc.h
index 71de978575f3..d156860495c7 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.h
+++ b/drivers/hwtracing/coresight/coresight-tmc.h
@@ -184,6 +184,7 @@ struct etr_buf {
* @idr_mutex: Access serialisation for idr.
* @sysfs_buf: SYSFS buffer for ETR.
* @perf_buf: PERF buffer for ETR.
+ * @enable:Indicates whether ETR/ETF is enabled.
*/
struct tmc_drvdata {
void __iomem*base;
@@ -207,6 +208,7 @@ struct tmc_drvdata {
struct mutexidr_mutex;
struct etr_buf *sysfs_buf;
struct etr_buf *perf_buf;
+ boolenable;
};
struct etr_buf_operations {
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH 08/10] clk: qcom: Add graphics clock controller driver for SM8250
Hi Bjorn, On 2020-05-29 06:41, Bjorn Andersson wrote: On Mon 25 May 02:47 PDT 2020, Sai Prakash Ranjan wrote: Hi Jonathan, On 2020-05-25 02:36, Jonathan Marek wrote: > Add support for the graphics clock controller found on SM8250 > based devices. This would allow graphics drivers to probe and > control their clocks. > > This is copied from the downstream kernel, adapted for upstream. > For example, GDSCs have been added. > > Signed-off-by: Jonathan Marek Since this is taken from downstream, maintain the original author's signed-off and add yourself as the co-developer if you have done any modifications. Same applies to all other patches. I disagree with this. As expressed in the commit message, this patch is based on the downstream driver, not the individual patch. As such, the _patch_ is prepared by Jonathan and by his Signed-off-by certifies the origin of the contribution per section 11.a or 11.b of submitting-patches.rst. I lost at the downstream driver vs the individual patch here. So the downstream driver is also an individual patch right or did I get something completely wrong. So if someone prepares a patch and includes a commit description saying it is taken from downstream, does it mean he is the author of that patch? Shouldn't the author be included in "From: Author" and his signed-off appear first before the submitter's(also a contributor) signed-off? Or is it because these clock data is auto generated and it doesnt really matter? Regarding co-developed-by; this should not be used when "forwarding" an existing patch. Per section 11.c the contributor should add their Signed-off-by to certify the origin of the patch. Any modifications should be documented in immediately proceeding the s-o-b, as described later in section 11. Yes makes sense to not have co-developed-by for forwarding patch. Thanks, Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
