* Re: [PATCH] iommu/arm-smmu-v3: Use str_read_write helper w/ logs
2025-01-07 16:51 ` [PATCH] iommu/arm-smmu-v3: Use str_read_write helper w/ logs Pranjal Shrivastava
@ 2025-01-07 22:21 ` Nicolin Chen
2025-01-08 16:38 ` Will Deacon
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Nicolin Chen @ 2025-01-07 22:21 UTC (permalink / raw)
To: Pranjal Shrivastava
Cc: Joerg Roedel, Will Deacon, Robin Murphy, Mostafa Saleh, iommu,
Daniel Mentz
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>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] iommu/arm-smmu-v3: Use str_read_write helper w/ logs
2025-01-07 16:51 ` [PATCH] iommu/arm-smmu-v3: Use str_read_write helper w/ logs Pranjal Shrivastava
2025-01-07 22:21 ` Nicolin Chen
@ 2025-01-08 16:38 ` Will Deacon
2025-01-10 11:18 ` Marek Szyprowski
2025-01-14 19:08 ` Guenter Roeck
3 siblings, 0 replies; 9+ messages in thread
From: Will Deacon @ 2025-01-08 16:38 UTC (permalink / raw)
To: Joerg Roedel, Robin Murphy, Pranjal Shrivastava
Cc: catalin.marinas, kernel-team, Will Deacon, Mostafa Saleh, iommu,
Daniel Mentz
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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] iommu/arm-smmu-v3: Use str_read_write helper w/ logs
2025-01-07 16:51 ` [PATCH] iommu/arm-smmu-v3: Use str_read_write helper w/ logs Pranjal Shrivastava
2025-01-07 22:21 ` Nicolin Chen
2025-01-08 16:38 ` Will Deacon
@ 2025-01-10 11:18 ` Marek Szyprowski
2025-01-10 12:53 ` Will Deacon
2025-01-14 19:08 ` Guenter Roeck
3 siblings, 1 reply; 9+ messages in thread
From: Marek Szyprowski @ 2025-01-10 11:18 UTC (permalink / raw)
To: Pranjal Shrivastava, Joerg Roedel, Will Deacon, Robin Murphy
Cc: Mostafa Saleh, iommu, Daniel Mentz
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
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH] iommu/arm-smmu-v3: Use str_read_write helper w/ logs
2025-01-10 11:18 ` Marek Szyprowski
@ 2025-01-10 12:53 ` Will Deacon
2025-01-10 13:17 ` Will Deacon
0 siblings, 1 reply; 9+ messages in thread
From: Will Deacon @ 2025-01-10 12:53 UTC (permalink / raw)
To: Marek Szyprowski
Cc: Pranjal Shrivastava, Joerg Roedel, Robin Murphy, Mostafa Saleh,
iommu, Daniel Mentz
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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] iommu/arm-smmu-v3: Use str_read_write helper w/ logs
2025-01-10 12:53 ` Will Deacon
@ 2025-01-10 13:17 ` Will Deacon
2025-01-10 16:25 ` Will Deacon
0 siblings, 1 reply; 9+ messages in thread
From: Will Deacon @ 2025-01-10 13:17 UTC (permalink / raw)
To: Marek Szyprowski
Cc: Pranjal Shrivastava, Joerg Roedel, Robin Murphy, Mostafa Saleh,
iommu, Daniel Mentz
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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] iommu/arm-smmu-v3: Use str_read_write helper w/ logs
2025-01-10 13:17 ` Will Deacon
@ 2025-01-10 16:25 ` Will Deacon
2025-01-10 17:47 ` Pranjal Shrivastava
0 siblings, 1 reply; 9+ messages in thread
From: Will Deacon @ 2025-01-10 16:25 UTC (permalink / raw)
To: Marek Szyprowski
Cc: Pranjal Shrivastava, Joerg Roedel, Robin Murphy, Mostafa Saleh,
iommu, Daniel Mentz
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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] iommu/arm-smmu-v3: Use str_read_write helper w/ logs
2025-01-10 16:25 ` Will Deacon
@ 2025-01-10 17:47 ` Pranjal Shrivastava
0 siblings, 0 replies; 9+ messages in thread
From: Pranjal Shrivastava @ 2025-01-10 17:47 UTC (permalink / raw)
To: Will Deacon
Cc: Marek Szyprowski, Joerg Roedel, Robin Murphy, Mostafa Saleh,
iommu, Daniel Mentz
> > > 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] iommu/arm-smmu-v3: Use str_read_write helper w/ logs
2025-01-07 16:51 ` [PATCH] iommu/arm-smmu-v3: Use str_read_write helper w/ logs Pranjal Shrivastava
` (2 preceding siblings ...)
2025-01-10 11:18 ` Marek Szyprowski
@ 2025-01-14 19:08 ` Guenter Roeck
3 siblings, 0 replies; 9+ messages in thread
From: Guenter Roeck @ 2025-01-14 19:08 UTC (permalink / raw)
To: Pranjal Shrivastava
Cc: Joerg Roedel, Will Deacon, Robin Murphy, Mostafa Saleh, iommu,
Daniel Mentz
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
>
^ permalink raw reply [flat|nested] 9+ messages in thread