On Mon, 2021-04-12 at 00:13 +0300, Alexander Monakov wrote:
> print_iommu_info prints the EFR register and then the decoded list of
> features on a separate line:
>
> pci 0000:00:00.2: AMD-Vi: Extended features (0x206d73ef22254ade):
> PPR X2APIC NX GT IA GA PC GA_vAPIC
>
> The second line is emitted via 'pr_cont', which causes it to have a
> different ('warn') loglevel compared to the previous line ('info').
>
> Commit 9a295ff0ffc9 attempted to rectify this by removing the newline
> from the pci_info format string, but this doesn't work, as pci_info
> calls implicitly append a newline anyway.
>
> Printing the decoded features on the same line would make it quite long.
> Instead, change pci_info() to pr_info() to omit PCI bus location info,
> which is shown in the preceding message anyway. This results in:
>
> pci 0000:00:00.2: AMD-Vi: Found IOMMU cap 0x40
> AMD-Vi: Extended features (0x206d73ef22254ade): PPR X2APIC NX GT IA GA PC
> GA_vAPIC
> AMD-Vi: Interrupt remapping enabled
>
> Fixes: 9a295ff0ffc9 ("iommu/amd: Print extended features in one line to fix
> divergent log levels")
> Link:
> https://lore.kernel.org/lkml/[email protected]
> Signed-off-by: Alexander Monakov <[email protected]>
> Cc: Paul Menzel <[email protected]>
> Cc: Joerg Roedel <[email protected]>
> Cc: Suravee Suthikulpanit <[email protected]>
> Cc: [email protected]
> ---
>
> v2: avoid pr_info(""), change pci_info() to pr_info() for a nicer
> solution
>
> drivers/iommu/amd/init.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> index 596d0c413473..62913f82a21f 100644
> --- a/drivers/iommu/amd/init.c
> +++ b/drivers/iommu/amd/init.c
> @@ -1929,8 +1929,8 @@ static void print_iommu_info(void)
> pci_info(pdev, "Found IOMMU cap 0x%hx\n", iommu->cap_ptr);
>
>
> if (iommu->cap & (1 << IOMMU_CAP_EFR)) {
> - pci_info(pdev, "Extended features (%#llx):",
> - iommu->features);
> + pr_info("Extended features (%#llx):", iommu->features);
> +
> for (i = 0; i < ARRAY_SIZE(feat_str); ++i) {
> if (iommu_feature(iommu, (1ULL << i)))
> pr_cont(" %s", feat_str[i]);
How about avoiding all of this by using a temporary buffer
and a single pci_info.
Miscellanea:
o Move the feat_str and i declarations into the if block for locality
---
drivers/iommu/amd/init.c | 29 ++++++++++++++++++-----------
1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index 321f5906e6ed..0d219044726e 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -1943,30 +1943,37 @@ static int __init iommu_init_pci(struct amd_iommu
*iommu)
static void print_iommu_info(void)
{
- static const char * const feat_str[] = {
- "PreF", "PPR", "X2APIC", "NX", "GT", "[5]",
- "IA", "GA", "HE", "PC"
- };
struct amd_iommu *iommu;
for_each_iommu(iommu) {
struct pci_dev *pdev = iommu->dev;
- int i;
pci_info(pdev, "Found IOMMU cap 0x%x\n", iommu->cap_ptr);
if (iommu->cap & (1 << IOMMU_CAP_EFR)) {
- pci_info(pdev, "Extended features (%#llx):",
- iommu->features);
+ static const char * const feat_str[] = {
+ "PreF", "PPR", "X2APIC", "NX", "GT", "[5]",
+ "IA", "GA", "HE", "PC"
+ };
+ int i;
+ char features[128] = "";
+ int len = 0;
+
for (i = 0; i < ARRAY_SIZE(feat_str); ++i) {
- if (iommu_feature(iommu, (1ULL << i)))
- pr_cont(" %s", feat_str[i]);
+ if (!iommu_feature(iommu, BIT_ULL(i)))
+ continue;
+ len += scnprintf(features + len,
+ sizeof(features) - len,
+ " %s", feat_str[i]);
}
if (iommu->features & FEATURE_GAM_VAPIC)
- pr_cont(" GA_vAPIC");
+ len += scnprintf(features + len,
+ sizeof(features) - len,
+ " %s", "GA_vPIC");
- pr_cont("\n");
+ pci_info(pdev, "Extended features (%#llx):%s\n",
+ iommu->features, features);
}
}
if (irq_remapping_enabled) {
