[PATCH] coresight: etm4x: Enable ETE device accessed via MMIO

[PATCH] coresight: etm4x: Enable ETE device accessed via MMIO

[Ruidong Tian]

The ETM4X driver now assume that all ETE as CPU system instructions
accessed device, in fact the ETE device on some machines also accessed
via MMIO.

Signed-off-by: Ruidong Tian <tianruidong@linux.alibaba.com>
---
 drivers/hwtracing/coresight/coresight-etm4x-core.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 285539104bcc..ad298c9cc87e 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1103,8 +1103,9 @@ static bool etm4_init_iomem_access(struct etmv4_drvdata *drvdata,
 	 * with MMIO. But we cannot touch the OSLK until we are
 	 * sure this is an ETM. So rely only on the TRCDEVARCH.
 	 */
-	if ((devarch & ETM_DEVARCH_ID_MASK) != ETM_DEVARCH_ETMv4x_ARCH) {
-		pr_warn_once("TRCDEVARCH doesn't match ETMv4 architecture\n");
+	if ((devarch & ETM_DEVARCH_ID_MASK) != ETM_DEVARCH_ETMv4x_ARCH &&
+		(devarch & ETM_DEVARCH_ID_MASK) != ETM_DEVARCH_ETE_ARCH) {
+		pr_warn_once("TRCDEVARCH doesn't match ETMv4/ETE architecture\n");
 		return false;
 	}
 
-- 
2.33.1

Re: [PATCH] coresight: etm4x: Enable ETE device accessed via MMIO

[Suzuki K Poulose]

On 18/10/2023 08:05, Ruidong Tian wrote:
> The ETM4X driver now assume that all ETE as CPU system instructions
> accessed device, in fact the ETE device on some machines also accessed
> via MMIO.
> 
> Signed-off-by: Ruidong Tian <tianruidong@linux.alibaba.com>

Why are we going backwards to MMIO from system instructions ? Is it 
because of an "unfriendly" hypervisor preventing access ?

As such, without a sufficiently acceptable explanation, I am reluctant
to make this change

Suzuki

> ---
>   drivers/hwtracing/coresight/coresight-etm4x-core.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 285539104bcc..ad298c9cc87e 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -1103,8 +1103,9 @@ static bool etm4_init_iomem_access(struct etmv4_drvdata *drvdata,
>   	 * with MMIO. But we cannot touch the OSLK until we are
>   	 * sure this is an ETM. So rely only on the TRCDEVARCH.
>   	 */
> -	if ((devarch & ETM_DEVARCH_ID_MASK) != ETM_DEVARCH_ETMv4x_ARCH) {
> -		pr_warn_once("TRCDEVARCH doesn't match ETMv4 architecture\n");
> +	if ((devarch & ETM_DEVARCH_ID_MASK) != ETM_DEVARCH_ETMv4x_ARCH &&
> +		(devarch & ETM_DEVARCH_ID_MASK) != ETM_DEVARCH_ETE_ARCH) {
> +		pr_warn_once("TRCDEVARCH doesn't match ETMv4/ETE architecture\n");
>   		return false;
>   	}
>   

Re: [PATCH] coresight: etm4x: Enable ETE device accessed via MMIO

[Ruidong Tian]

Hi Suzuki,

Now ETM4X driver use MMIO or system instruction depends on this check in 
function etm4_init_csdev_access:

         if (drvdata->base)
                 return etm4_init_iomem_access(drvdata, csa);

This check always true if firmware provides a address range in ACPI
table of ETE, and as a result, the ETE device in this case cannot be
successfully probed.

I think OS should be compatible with this situation, no matter firmware
how to organize ETE information in ACPI table. How do you feel about
it?

Thank you

Ruidong Tian
在 2023/10/18 16:28, Suzuki K Poulose 写道:
> On 18/10/2023 08:05, Ruidong Tian wrote:
>> The ETM4X driver now assume that all ETE as CPU system instructions
>> accessed device, in fact the ETE device on some machines also accessed
>> via MMIO.
>>
>> Signed-off-by: Ruidong Tian <tianruidong@linux.alibaba.com>
>
> Why are we going backwards to MMIO from system instructions ? Is it 
> because of an "unfriendly" hypervisor preventing access ?
>
> As such, without a sufficiently acceptable explanation, I am reluctant
> to make this change
>
> Suzuki
>
>> ---
>>   drivers/hwtracing/coresight/coresight-etm4x-core.c | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c 
>> b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> index 285539104bcc..ad298c9cc87e 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> @@ -1103,8 +1103,9 @@ static bool etm4_init_iomem_access(struct 
>> etmv4_drvdata *drvdata,
>>        * with MMIO. But we cannot touch the OSLK until we are
>>        * sure this is an ETM. So rely only on the TRCDEVARCH.
>>        */
>> -    if ((devarch & ETM_DEVARCH_ID_MASK) != ETM_DEVARCH_ETMv4x_ARCH) {
>> -        pr_warn_once("TRCDEVARCH doesn't match ETMv4 architecture\n");
>> +    if ((devarch & ETM_DEVARCH_ID_MASK) != ETM_DEVARCH_ETMv4x_ARCH &&
>> +        (devarch & ETM_DEVARCH_ID_MASK) != ETM_DEVARCH_ETE_ARCH) {
>> +        pr_warn_once("TRCDEVARCH doesn't match ETMv4/ETE 
>> architecture\n");
>>           return false;
>>       }

Re: [PATCH] coresight: etm4x: Enable ETE device accessed via MMIO

[Suzuki K Poulose]

Hi

On 18/10/2023 10:30, Ruidong Tian wrote:
> Hi Suzuki,
> 
> Now ETM4X driver use MMIO or system instruction depends on this check in 
> function etm4_init_csdev_access:
> 
>          if (drvdata->base)
>                  return etm4_init_iomem_access(drvdata, csa);
> 
> This check always true if firmware provides a address range in ACPI
> table of ETE, and as a result, the ETE device in this case cannot be
> successfully probed.
> 
> I think OS should be compatible with this situation, no matter firmware
> how to organize ETE information in ACPI table. How do you feel about
> it?

My question is not about "What the patch does". But, why can't we use
system instructions on your system, when ETE was designed to be used
with that in the first place and get rid of the MMIO.

Suzuki

> 
> Thank you
> 
> Ruidong Tian
> 在 2023/10/18 16:28, Suzuki K Poulose 写道:
>> On 18/10/2023 08:05, Ruidong Tian wrote:
>>> The ETM4X driver now assume that all ETE as CPU system instructions
>>> accessed device, in fact the ETE device on some machines also accessed
>>> via MMIO.
>>>
>>> Signed-off-by: Ruidong Tian <tianruidong@linux.alibaba.com>
>>
>> Why are we going backwards to MMIO from system instructions ? Is it 
>> because of an "unfriendly" hypervisor preventing access ?
>>
>> As such, without a sufficiently acceptable explanation, I am reluctant
>> to make this change
>>
>> Suzuki
>>
>>> ---
>>>   drivers/hwtracing/coresight/coresight-etm4x-core.c | 5 +++--
>>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c 
>>> b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>>> index 285539104bcc..ad298c9cc87e 100644
>>> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
>>> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>>> @@ -1103,8 +1103,9 @@ static bool etm4_init_iomem_access(struct 
>>> etmv4_drvdata *drvdata,
>>>        * with MMIO. But we cannot touch the OSLK until we are
>>>        * sure this is an ETM. So rely only on the TRCDEVARCH.
>>>        */
>>> -    if ((devarch & ETM_DEVARCH_ID_MASK) != ETM_DEVARCH_ETMv4x_ARCH) {
>>> -        pr_warn_once("TRCDEVARCH doesn't match ETMv4 architecture\n");
>>> +    if ((devarch & ETM_DEVARCH_ID_MASK) != ETM_DEVARCH_ETMv4x_ARCH &&
>>> +        (devarch & ETM_DEVARCH_ID_MASK) != ETM_DEVARCH_ETE_ARCH) {
>>> +        pr_warn_once("TRCDEVARCH doesn't match ETMv4/ETE 
>>> architecture\n");
>>>           return false;
>>>       }

Re: [PATCH] coresight: etm4x: Enable ETE device accessed via MMIO

[Ruidong Tian]

Hi Suzuki

You are right, I review armv9 Spec again, and find that ETE only
support system instructions access.  This patch is meaningless and
need to drop it.

Ruidong

在 2023/10/18 17:36, Suzuki K Poulose 写道:
> Hi
>
> On 18/10/2023 10:30, Ruidong Tian wrote:
>> Hi Suzuki,
>>
>> Now ETM4X driver use MMIO or system instruction depends on this check 
>> in function etm4_init_csdev_access:
>>
>>          if (drvdata->base)
>>                  return etm4_init_iomem_access(drvdata, csa);
>>
>> This check always true if firmware provides a address range in ACPI
>> table of ETE, and as a result, the ETE device in this case cannot be
>> successfully probed.
>>
>> I think OS should be compatible with this situation, no matter firmware
>> how to organize ETE information in ACPI table. How do you feel about
>> it?
>
> My question is not about "What the patch does". But, why can't we use
> system instructions on your system, when ETE was designed to be used
> with that in the first place and get rid of the MMIO.
>
> Suzuki
>
>>
>> Thank you
>>
>> Ruidong Tian
>> 在 2023/10/18 16:28, Suzuki K Poulose 写道:
>>> On 18/10/2023 08:05, Ruidong Tian wrote:
>>>> The ETM4X driver now assume that all ETE as CPU system instructions
>>>> accessed device, in fact the ETE device on some machines also accessed
>>>> via MMIO.
>>>>
>>>> Signed-off-by: Ruidong Tian <tianruidong@linux.alibaba.com>
>>>
>>> Why are we going backwards to MMIO from system instructions ? Is it 
>>> because of an "unfriendly" hypervisor preventing access ?
>>>
>>> As such, without a sufficiently acceptable explanation, I am reluctant
>>> to make this change
>>>
>>> Suzuki
>>>
>>>> ---
>>>>   drivers/hwtracing/coresight/coresight-etm4x-core.c | 5 +++--
>>>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c 
>>>> b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>>>> index 285539104bcc..ad298c9cc87e 100644
>>>> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
>>>> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>>>> @@ -1103,8 +1103,9 @@ static bool etm4_init_iomem_access(struct 
>>>> etmv4_drvdata *drvdata,
>>>>        * with MMIO. But we cannot touch the OSLK until we are
>>>>        * sure this is an ETM. So rely only on the TRCDEVARCH.
>>>>        */
>>>> -    if ((devarch & ETM_DEVARCH_ID_MASK) != ETM_DEVARCH_ETMv4x_ARCH) {
>>>> -        pr_warn_once("TRCDEVARCH doesn't match ETMv4 
>>>> architecture\n");
>>>> +    if ((devarch & ETM_DEVARCH_ID_MASK) != ETM_DEVARCH_ETMv4x_ARCH &&
>>>> +        (devarch & ETM_DEVARCH_ID_MASK) != ETM_DEVARCH_ETE_ARCH) {
>>>> +        pr_warn_once("TRCDEVARCH doesn't match ETMv4/ETE 
>>>> architecture\n");
>>>>           return false;
>>>>       }