linux-kernel - Re: [PATCH v7 6/7] iommu/amd: Introduce amd_iommu_get_num

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 12 Jan 2017 15:21:39 +0100
From:   Borislav Petkov <bp@...en8.de>
To:     Suravee Suthikulpanit <Suravee.Suthikulpanit@....com>
Cc:     linux-kernel@...r.kernel.org, iommu@...ts.linux-foundation.org,
        joro@...tes.org, peterz@...radead.org, mingo@...hat.com
Subject: Re: [PATCH v7 6/7] iommu/amd: Introduce amd_iommu_get_num_iommus()

On Mon, Jan 09, 2017 at 09:33:46PM -0600, Suravee Suthikulpanit wrote:
> This patch introduces amd_iommu_get_num_iommus(). This is intended for

There's that "This patch" again... but you get the idea :)

> Perf AMD IOMMU driver.
> 
> Cc: Joerg Roedel <joro@...tes.org>
> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@....com>
> ---
>  arch/x86/events/amd/iommu.h    | 2 ++
>  drivers/iommu/amd_iommu_init.c | 7 ++++++-
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/events/amd/iommu.h b/arch/x86/events/amd/iommu.h
> index 342716e..381f1c4 100644
> --- a/arch/x86/events/amd/iommu.h
> +++ b/arch/x86/events/amd/iommu.h
> @@ -25,6 +25,8 @@
>  #define PC_MAX_SPEC_CNTRS			16
>  
>  /* amd_iommu_init.c external support functions */
> +extern int amd_iommu_get_num_iommus(void);
> +
>  extern bool amd_iommu_pc_supported(void);
>  
>  extern u8 amd_iommu_pc_get_max_banks(uint idx);
> diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
> index c993c77..c3740cb 100644
> --- a/drivers/iommu/amd_iommu_init.c
> +++ b/drivers/iommu/amd_iommu_init.c
> @@ -1329,7 +1329,7 @@ static int __init init_iommu_one(struct amd_iommu *iommu, struct ivhd_header *h)
>  
>  	/* Add IOMMU to internal data structures */
>  	list_add_tail(&iommu->list, &amd_iommu_list);
> -	iommu->index             = amd_iommus_present++;
> +	iommu->index = amd_iommus_present++;
>  
>  	if (unlikely(iommu->index >= MAX_IOMMUS)) {
>  		WARN(1, "AMD-Vi: System has more IOMMUs than supported by this driver\n");
> @@ -2717,6 +2717,11 @@ static struct amd_iommu *get_amd_iommu(uint idx)
>  	return iommu;
>  }
>  
> +int amd_iommu_get_num_iommus(void)
> +{
> +	return amd_iommus_present;
> +}

So this is strange. This amd_iommus_present is used by other iommu code
but then you're adding a getter. IMO, it should be cleaner if *all* code
is converted to use the getter now and not the naked variable.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.