Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 26 Jul 2022 08:32:27 -0700
From:   Jerry Snitselaar <jsnitsel@redhat.com>
To:     Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Cc:     linux-kernel@vger.kernel.org, iommu@lists.linux.dev,
        joro@8bytes.org, will@kernel.org, vasant.hegde@amd.com,
        jon.grimm@amd.com
Subject: Re: [PATCH 2/2] iommu/amd: Add support for AVIC when SNP is enabled
Message-ID: <20220726153227.caqf6hagmwu727ls@cantor>
References: <20220726134348.6438-1-suravee.suthikulpanit@amd.com>
 <20220726134348.6438-3-suravee.suthikulpanit@amd.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220726134348.6438-3-suravee.suthikulpanit@amd.com>
Precedence: bulk

On Tue, Jul 26, 2022 at 08:43:48AM -0500, Suravee Suthikulpanit wrote:
> In order to support AVIC on SNP-enabled system, The IOMMU driver needs to
> check EFR2[SNPAVICSup] and enables the support by setting SNPAVICEn bit
> in the IOMMU control register (MMIO offset 18h).
> 
> For detail, please see section "SEV-SNP Guest Virtual APIC Support" of the
> AMD I/O Virtualization Technology (IOMMU) Specification.
> (https://www.amd.com/system/files/TechDocs/48882_IOMMU.pdf)
> 
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>

Is this a typo in the 3.06 spec for SNPAVIC_EN?

"001b-111b = Reserved."

Or I guess maybe there is a newer revision that isn't available yet
that describes 001b?

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

> ---
>  drivers/iommu/amd/amd_iommu_types.h |  7 +++++++
>  drivers/iommu/amd/init.c            | 11 ++++++++++-
>  2 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
> index 3c1205ba636a..5b1019dab328 100644
> --- a/drivers/iommu/amd/amd_iommu_types.h
> +++ b/drivers/iommu/amd/amd_iommu_types.h
> @@ -103,6 +103,12 @@
>  #define FEATURE_GLXVAL_SHIFT	14
>  #define FEATURE_GLXVAL_MASK	(0x03ULL << FEATURE_GLXVAL_SHIFT)
>  
> +/* Extended Feature 2 Bits */
> +#define FEATURE_SNPAVICSUP_SHIFT	5
> +#define FEATURE_SNPAVICSUP_MASK		(0x07ULL << FEATURE_SNPAVICSUP_SHIFT)
> +#define FEATURE_SNPAVICSUP_GAM(x) \
> +	((x & FEATURE_SNPAVICSUP_MASK) >> FEATURE_SNPAVICSUP_SHIFT == 0x1)
> +
>  /* Note:
>   * The current driver only support 16-bit PASID.
>   * Currently, hardware only implement upto 16-bit PASID
> @@ -165,6 +171,7 @@
>  #define CONTROL_GAINT_EN	29
>  #define CONTROL_XT_EN		50
>  #define CONTROL_INTCAPXT_EN	51
> +#define CONTROL_SNPAVIC_EN	61
>  
>  #define CTRL_INV_TO_MASK	(7 << CONTROL_INV_TIMEOUT)
>  #define CTRL_INV_TO_NONE	0
> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> index 4cd94d716122..6bbaf6b971e8 100644
> --- a/drivers/iommu/amd/init.c
> +++ b/drivers/iommu/amd/init.c
> @@ -2794,13 +2794,22 @@ static void enable_iommus_vapic(void)
>  		return;
>  	}
>  
> -	/* Enabling GAM support */
> +	if (amd_iommu_snp_en &&
> +	    !FEATURE_SNPAVICSUP_GAM(amd_iommu_efr2)) {
> +		pr_warn("Force to disable Virtual APIC due to SNP\n");
> +		amd_iommu_guest_ir = AMD_IOMMU_GUEST_IR_LEGACY_GA;
> +		return;
> +	}
> +
> +	/* Enabling GAM and SNPAVIC support */
>  	for_each_iommu(iommu) {
>  		if (iommu_init_ga_log(iommu) ||
>  		    iommu_ga_log_enable(iommu))
>  			return;
>  
>  		iommu_feature_enable(iommu, CONTROL_GAM_EN);
> +		if (amd_iommu_snp_en)
> +			iommu_feature_enable(iommu, CONTROL_SNPAVIC_EN);
>  	}
>  
>  	amd_iommu_irq_ops.capability |= (1 << IRQ_POSTING_CAP);
> -- 
> 2.34.1
>