[PATCH] iommu/arm-smmu-v3: Use str_read_write helper w/ logs

[PATCH] iommu/arm-smmu-v3: Use str_read_write helper w/ logs

[Pranjal Shrivastava]

Adopt the `str_read_write` helper in event logging as highlighted [1].

[1] https://lore.kernel.org/all/20250107130053.GC6991@willie-the-truck/

Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 34b424c87be0..0e4cbb2c64d7 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1907,7 +1907,7 @@ static void arm_smmu_dump_event(struct arm_smmu_device *smmu, u64 *raw,
 		dev_err(smmu->dev, "%s %s %s %s \"%s\"%s%s stag: %#x",
 			evt->privileged ? "priv" : "unpriv",
 			evt->instruction ? "inst" : "data",
-			evt->read ? "read" : "write",
+			str_read_write(evt->read),
 			evt->s2 ? "s2" : "s1", event_class_str[evt->class],
 			evt->class_tt ? (evt->ttrnw ? " ttd_read" : " ttd_write") : "",
 			evt->stall ? " stall" : "", evt->stag);
-- 
2.47.1.613.gc27f4b7a9f-goog

Re: [PATCH] iommu/arm-smmu-v3: Use str_read_write helper w/ logs

[Nicolin Chen]

On Tue, Jan 07, 2025 at 04:51:00PM +0000, Pranjal Shrivastava wrote:
> Adopt the `str_read_write` helper in event logging as highlighted [1].
> 
> [1] https://lore.kernel.org/all/20250107130053.GC6991@willie-the-truck/
> 
> Signed-off-by: Pranjal Shrivastava <praan@google.com>

Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>

Re: [PATCH] iommu/arm-smmu-v3: Use str_read_write helper w/ logs

[Will Deacon]

On Tue, 07 Jan 2025 16:51:00 +0000, Pranjal Shrivastava wrote:
> Adopt the `str_read_write` helper in event logging as highlighted [1].
> 
> [1] https://lore.kernel.org/all/20250107130053.GC6991@willie-the-truck/
> 
> 

Applied to iommu (arm/smmu/updates), thanks!

[1/1] iommu/arm-smmu-v3: Use str_read_write helper w/ logs
      https://git.kernel.org/iommu/c/f2c77f6e41e6

Cheers,
-- 
Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev

Re: [PATCH] iommu/arm-smmu-v3: Use str_read_write helper w/ logs

[Marek Szyprowski]

On 07.01.2025 17:51, Pranjal Shrivastava wrote:
> Adopt the `str_read_write` helper in event logging as highlighted [1].
>
> [1] https://lore.kernel.org/all/20250107130053.GC6991@willie-the-truck/
>
> Signed-off-by: Pranjal Shrivastava <praan@google.com>

This change landed in today's linux-next as commit f2c77f6e41e6 
("iommu/arm-smmu-v3: Use str_read_write helper w/ logs"). It fails to 
build here with arm64/defconfig on my setup. It looks that it misses to 
add the following include:

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c 
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 0e4cbb2c64d7..a0e395da8a88 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -26,6 +26,7 @@
  #include <linux/pci.h>
  #include <linux/pci-ats.h>
  #include <linux/platform_device.h>
+#include <linux/string_choices.h>
  #include <kunit/visibility.h>
  #include <uapi/linux/iommufd.h>

> ---
>   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index 34b424c87be0..0e4cbb2c64d7 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -1907,7 +1907,7 @@ static void arm_smmu_dump_event(struct arm_smmu_device *smmu, u64 *raw,
>   		dev_err(smmu->dev, "%s %s %s %s \"%s\"%s%s stag: %#x",
>   			evt->privileged ? "priv" : "unpriv",
>   			evt->instruction ? "inst" : "data",
> -			evt->read ? "read" : "write",
> +			str_read_write(evt->read),
>   			evt->s2 ? "s2" : "s1", event_class_str[evt->class],
>   			evt->class_tt ? (evt->ttrnw ? " ttd_read" : " ttd_write") : "",
>   			evt->stall ? " stall" : "", evt->stag);

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH] iommu/arm-smmu-v3: Use str_read_write helper w/ logs

[Will Deacon]

On Fri, Jan 10, 2025 at 12:18:02PM +0100, Marek Szyprowski wrote:
> On 07.01.2025 17:51, Pranjal Shrivastava wrote:
> > Adopt the `str_read_write` helper in event logging as highlighted [1].
> >
> > [1] https://lore.kernel.org/all/20250107130053.GC6991@willie-the-truck/
> >
> > Signed-off-by: Pranjal Shrivastava <praan@google.com>
> 
> This change landed in today's linux-next as commit f2c77f6e41e6 
> ("iommu/arm-smmu-v3: Use str_read_write helper w/ logs"). It fails to 
> build here with arm64/defconfig on my setup. It looks that it misses to 
> add the following include:
> 
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c 
> b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index 0e4cbb2c64d7..a0e395da8a88 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -26,6 +26,7 @@
>   #include <linux/pci.h>
>   #include <linux/pci-ats.h>
>   #include <linux/platform_device.h>
> +#include <linux/string_choices.h>
>   #include <kunit/visibility.h>
>   #include <uapi/linux/iommufd.h>

Hmm, I build and test my branches before I push them so this is a bit
surprising. I just confirmed that it's building with arm64/defconfig
for me, so lemme dig into this some more...

Will

Re: [PATCH] iommu/arm-smmu-v3: Use str_read_write helper w/ logs

[Will Deacon]

On Fri, Jan 10, 2025 at 12:53:43PM +0000, Will Deacon wrote:
> On Fri, Jan 10, 2025 at 12:18:02PM +0100, Marek Szyprowski wrote:
> > On 07.01.2025 17:51, Pranjal Shrivastava wrote:
> > > Adopt the `str_read_write` helper in event logging as highlighted [1].
> > >
> > > [1] https://lore.kernel.org/all/20250107130053.GC6991@willie-the-truck/
> > >
> > > Signed-off-by: Pranjal Shrivastava <praan@google.com>
> > 
> > This change landed in today's linux-next as commit f2c77f6e41e6 
> > ("iommu/arm-smmu-v3: Use str_read_write helper w/ logs"). It fails to 
> > build here with arm64/defconfig on my setup. It looks that it misses to 
> > add the following include:
> > 
> > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c 
> > b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > index 0e4cbb2c64d7..a0e395da8a88 100644
> > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > @@ -26,6 +26,7 @@
> >   #include <linux/pci.h>
> >   #include <linux/pci-ats.h>
> >   #include <linux/platform_device.h>
> > +#include <linux/string_choices.h>
> >   #include <kunit/visibility.h>
> >   #include <uapi/linux/iommufd.h>
> 
> Hmm, I build and test my branches before I push them so this is a bit
> surprising. I just confirmed that it's building with arm64/defconfig
> for me, so lemme dig into this some more...

Aha, there seems to be a transitive include of string_choices.h via
linux/acpi.h. So while I still think the arm64 defconfig should build,
if I go in and disable CONFIG_ACPI then I see the failure.

So for my test builds:
allnoconfig:	Builds as there isn't an SMMU driver
defconfig:	Builds because CONFIG_ACPI=y
allmodconfig:	Same as above

I'll add the missing #include.

Will

Re: [PATCH] iommu/arm-smmu-v3: Use str_read_write helper w/ logs

[Will Deacon]

On Fri, Jan 10, 2025 at 01:17:25PM +0000, Will Deacon wrote:
> On Fri, Jan 10, 2025 at 12:53:43PM +0000, Will Deacon wrote:
> > On Fri, Jan 10, 2025 at 12:18:02PM +0100, Marek Szyprowski wrote:
> > > On 07.01.2025 17:51, Pranjal Shrivastava wrote:
> > > > Adopt the `str_read_write` helper in event logging as highlighted [1].
> > > >
> > > > [1] https://lore.kernel.org/all/20250107130053.GC6991@willie-the-truck/
> > > >
> > > > Signed-off-by: Pranjal Shrivastava <praan@google.com>
> > > 
> > > This change landed in today's linux-next as commit f2c77f6e41e6 
> > > ("iommu/arm-smmu-v3: Use str_read_write helper w/ logs"). It fails to 
> > > build here with arm64/defconfig on my setup. It looks that it misses to 
> > > add the following include:
> > > 
> > > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c 
> > > b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > > index 0e4cbb2c64d7..a0e395da8a88 100644
> > > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > > @@ -26,6 +26,7 @@
> > >   #include <linux/pci.h>
> > >   #include <linux/pci-ats.h>
> > >   #include <linux/platform_device.h>
> > > +#include <linux/string_choices.h>
> > >   #include <kunit/visibility.h>
> > >   #include <uapi/linux/iommufd.h>
> > 
> > Hmm, I build and test my branches before I push them so this is a bit
> > surprising. I just confirmed that it's building with arm64/defconfig
> > for me, so lemme dig into this some more...
> 
> Aha, there seems to be a transitive include of string_choices.h via
> linux/acpi.h. So while I still think the arm64 defconfig should build,
> if I go in and disable CONFIG_ACPI then I see the failure.
> 
> So for my test builds:
> allnoconfig:	Builds as there isn't an SMMU driver
> defconfig:	Builds because CONFIG_ACPI=y
> allmodconfig:	Same as above
> 
> I'll add the missing #include.

I pushed the fix to arm/smmu/updates [1] which should appear in -next
when Joerg next rebuilds the branch.

Cheers,

Will

[1] https://git.kernel.org/pub/scm/linux/kernel/git/iommu/linux.git/commit/?h=arm/smmu/updates

Re: [PATCH] iommu/arm-smmu-v3: Use str_read_write helper w/ logs

[Pranjal Shrivastava]

> > > On Fri, Jan 10, 2025 at 12:18:02PM +0100, Marek Szyprowski wrote:
> > > > On 07.01.2025 17:51, Pranjal Shrivastava wrote:
> > > > > Adopt the `str_read_write` helper in event logging as highlighted [1].
> > > > >
> > > > > [1] https://lore.kernel.org/all/20250107130053.GC6991@willie-the-truck/
> > > > >
> > > > > Signed-off-by: Pranjal Shrivastava <praan@google.com>
> > > > 
> > > > This change landed in today's linux-next as commit f2c77f6e41e6 
> > > > ("iommu/arm-smmu-v3: Use str_read_write helper w/ logs"). It fails to 
> > > > build here with arm64/defconfig on my setup. It looks that it misses to 
> > > > add the following include:
> > > > 
> > > > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c 
> > > > b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > > > index 0e4cbb2c64d7..a0e395da8a88 100644
> > > > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > > > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > > > @@ -26,6 +26,7 @@
> > > >   #include <linux/pci.h>
> > > >   #include <linux/pci-ats.h>
> > > >   #include <linux/platform_device.h>
> > > > +#include <linux/string_choices.h>
> > > >   #include <kunit/visibility.h>
> > > >   #include <uapi/linux/iommufd.h>
> > > 
> > > Hmm, I build and test my branches before I push them so this is a bit
> > > surprising. I just confirmed that it's building with arm64/defconfig
> > > for me, so lemme dig into this some more...
> > 
> > Aha, there seems to be a transitive include of string_choices.h via
> > linux/acpi.h. So while I still think the arm64 defconfig should build,
> > if I go in and disable CONFIG_ACPI then I see the failure.
> > 
> > So for my test builds:
> > allnoconfig:	Builds as there isn't an SMMU driver
> > defconfig:	Builds because CONFIG_ACPI=y
> > allmodconfig:	Same as above
> > 

Ahh, apologies for not building with CONFIG_ACPI=n, I'll be more
careful next time!

> > I'll add the missing #include.
> 
> I pushed the fix to arm/smmu/updates [1] which should appear in -next
> when Joerg next rebuilds the branch.

Thanks for fixing this!
Regards,
Praan

Re: [PATCH] iommu/arm-smmu-v3: Use str_read_write helper w/ logs

[Guenter Roeck]

On Tue, Jan 07, 2025 at 04:51:00PM +0000, Pranjal Shrivastava wrote:
> Adopt the `str_read_write` helper in event logging as highlighted [1].
> 
> [1] https://lore.kernel.org/all/20250107130053.GC6991@willie-the-truck/
> 
> Signed-off-by: Pranjal Shrivastava <praan@google.com>

This may result in build errors such as:

Error log:
In file included from include/linux/device.h:15,
                 from include/linux/acpi.h:14,
                 from drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c:12:
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c: In function 'arm_smmu_dump_event':
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c:1910:25: error: implicit declaration of function 'str_read_write'; did you mean 'sb_end_write'?

since the necessary include file is not included directly.

Guenter

> ---
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index 34b424c87be0..0e4cbb2c64d7 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -1907,7 +1907,7 @@ static void arm_smmu_dump_event(struct arm_smmu_device *smmu, u64 *raw,
>  		dev_err(smmu->dev, "%s %s %s %s \"%s\"%s%s stag: %#x",
>  			evt->privileged ? "priv" : "unpriv",
>  			evt->instruction ? "inst" : "data",
> -			evt->read ? "read" : "write",
> +			str_read_write(evt->read),
>  			evt->s2 ? "s2" : "s1", event_class_str[evt->class],
>  			evt->class_tt ? (evt->ttrnw ? " ttd_read" : " ttd_write") : "",
>  			evt->stall ? " stall" : "", evt->stag);
> -- 
> 2.47.1.613.gc27f4b7a9f-goog
>