linux-kernel - Re: [PATCH] iommu/amd: Add sanity check for interrupt remapping table length macros

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]

Message-ID: <871rfxtvr8.fsf@redhat.com>
Date:   Thu, 10 Dec 2020 10:29:31 -0700
From:   Jerry Snitselaar <jsnitsel@...hat.com>
To:     Suravee Suthikulpanit <suravee.suthikulpanit@....com>
Cc:     linux-kernel@...r.kernel.org, iommu@...ts.linux-foundation.org,
        joro@...tes.org, will@...nel.org, Jon.Grimm@....com,
        thomas.lendacky@....com
Subject: Re: [PATCH] iommu/amd: Add sanity check for interrupt remapping
 table length macros


Suravee Suthikulpanit @ 2020-12-10 09:24 MST:

> Currently, macros related to the interrupt remapping table length are
> defined separately. This has resulted in an oversight in which one of
> the macros were missed when changing the length. To prevent this,
> redefine the macros to add built-in sanity check.
>
> Also, rename macros to use the name of the DTE[IntTabLen] field as
> specified in the AMD IOMMU specification. There is no functional change.
>
> Suggested-by: Linus Torvalds <torvalds@...ux-foundation.org>
> Reviewed-by: Tom Lendacky <thomas.lendacky@....com>
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@....com>
> Cc: Will Deacon <will@...nel.org>
> Cc: Jerry Snitselaar <jsnitsel@...hat.com>
> Cc: Joerg Roedel <joro@...tes.org>
> ---
>  drivers/iommu/amd/amd_iommu_types.h | 19 ++++++++++---------
>  drivers/iommu/amd/init.c            |  6 +++---
>  drivers/iommu/amd/iommu.c           |  2 +-
>  3 files changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
> index 494b42a31b7a..899ce62df3f0 100644
> --- a/drivers/iommu/amd/amd_iommu_types.h
> +++ b/drivers/iommu/amd/amd_iommu_types.h
> @@ -255,11 +255,19 @@
>  /* Bit value definition for dte irq remapping fields*/
>  #define DTE_IRQ_PHYS_ADDR_MASK	(((1ULL << 45)-1) << 6)
>  #define DTE_IRQ_REMAP_INTCTL_MASK	(0x3ULL << 60)
> -#define DTE_IRQ_TABLE_LEN_MASK	(0xfULL << 1)
>  #define DTE_IRQ_REMAP_INTCTL    (2ULL << 60)
> -#define DTE_IRQ_TABLE_LEN       (9ULL << 1)
>  #define DTE_IRQ_REMAP_ENABLE    1ULL
>  
> +/*
> + * AMD IOMMU hardware only support 512 IRTEs despite
> + * the architectural limitation of 2048 entries.
> + */
> +#define DTE_INTTAB_ALIGNMENT    128
> +#define DTE_INTTABLEN_VALUE     9ULL
> +#define DTE_INTTABLEN           (DTE_INTTABLEN_VALUE << 1)
> +#define DTE_INTTABLEN_MASK      (0xfULL << 1)
> +#define MAX_IRQS_PER_TABLE      (1 << DTE_INTTABLEN_VALUE)
> +
>  #define PAGE_MODE_NONE    0x00
>  #define PAGE_MODE_1_LEVEL 0x01
>  #define PAGE_MODE_2_LEVEL 0x02
> @@ -409,13 +417,6 @@ extern bool amd_iommu_np_cache;
>  /* Only true if all IOMMUs support device IOTLBs */
>  extern bool amd_iommu_iotlb_sup;
>  
> -/*
> - * AMD IOMMU hardware only support 512 IRTEs despite
> - * the architectural limitation of 2048 entries.
> - */
> -#define MAX_IRQS_PER_TABLE	512
> -#define IRQ_TABLE_ALIGNMENT	128
> -
>  struct irq_remap_table {
>  	raw_spinlock_t lock;
>  	unsigned min_index;
> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> index 23a790f8f550..6bec8913d064 100644
> --- a/drivers/iommu/amd/init.c
> +++ b/drivers/iommu/amd/init.c
> @@ -989,10 +989,10 @@ static bool copy_device_table(void)
>  
>  		irq_v = old_devtb[devid].data[2] & DTE_IRQ_REMAP_ENABLE;
>  		int_ctl = old_devtb[devid].data[2] & DTE_IRQ_REMAP_INTCTL_MASK;
> -		int_tab_len = old_devtb[devid].data[2] & DTE_IRQ_TABLE_LEN_MASK;
> +		int_tab_len = old_devtb[devid].data[2] & DTE_INTTABLEN_MASK;
>  		if (irq_v && (int_ctl || int_tab_len)) {
>  			if ((int_ctl != DTE_IRQ_REMAP_INTCTL) ||
> -			    (int_tab_len != DTE_IRQ_TABLE_LEN)) {
> +			    (int_tab_len != DTE_INTTABLEN)) {
>  				pr_err("Wrong old irq remapping flag: %#x\n", devid);
>  				return false;
>  			}
> @@ -2674,7 +2674,7 @@ static int __init early_amd_iommu_init(void)
>  			remap_cache_sz = MAX_IRQS_PER_TABLE * (sizeof(u64) * 2);
>  		amd_iommu_irq_cache = kmem_cache_create("irq_remap_cache",
>  							remap_cache_sz,
> -							IRQ_TABLE_ALIGNMENT,
> +							DTE_INTTAB_ALIGNMENT,
>  							0, NULL);
>  		if (!amd_iommu_irq_cache)
>  			goto out;
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index b9cf59443843..f7abf16d1e3a 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -3191,7 +3191,7 @@ static void set_dte_irq_entry(u16 devid, struct irq_remap_table *table)
>  	dte	&= ~DTE_IRQ_PHYS_ADDR_MASK;
>  	dte	|= iommu_virt_to_phys(table->table);
>  	dte	|= DTE_IRQ_REMAP_INTCTL;
> -	dte	|= DTE_IRQ_TABLE_LEN;
> +	dte	|= DTE_INTTABLEN;
>  	dte	|= DTE_IRQ_REMAP_ENABLE;
>  
>  	amd_iommu_dev_table[devid].data[2] = dte;


Reviewed-by: Jerry Snitselaar <jsnitsel@...hat.com>