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;
Message-ID: <2d0ab154-596e-437f-1575-3d25fe421b86@oracle.com>
Date:   Tue, 9 May 2023 18:47:03 -0400
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.0
Subject: Re: [PATCH 3/5] iommu/amd: Introduce Disable IRTE Caching Support
Content-Language: en-US
To:     Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>,
        linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org
Cc:     joro@8bytes.org, joao.m.martins@oracle.com,
        boris.ostrovsky@oracle.com, jon.grimm@amd.com,
        santosh.shukla@amd.com, vasant.hegde@amd.com,
        kishon.vijayabraham@amd.com
References: <20230509111646.369661-1-suravee.suthikulpanit@amd.com>
 <20230509111646.369661-4-suravee.suthikulpanit@amd.com>
From:   Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
Organization: Oracle Corporation
In-Reply-To: <20230509111646.369661-4-suravee.suthikulpanit@amd.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?Nmx3UnI1eisyTjlFY1locE4rTzhqTTkxdWxOT3FkeGs0VklOck95U0ErQ1Na?=
 =?utf-8?B?bVVmSnpMdXZ6WEpmYTFlQjRqK1Q4a1p2eVdyMTlmay9hVXVzM0U4aU80and0?=
 =?utf-8?B?VU13c0VPQXNOaE8wWnVqditaQ0tJSTdUUFFTNDRzSjQ0anByUmZGcTdTZ1N2?=
 =?utf-8?B?T28xQWNTa0hEdTltc05yZlBRY1N4WllzcVpCTCtQd21mdkRQdWswMkFHR3FI?=
 =?utf-8?B?VkZONkV3cmFsSlUwSklYS0FOTlpyVTJTM2xORjVjOG13dDIzMU00cDFHaXly?=
 =?utf-8?B?TDZ0QmcyVWxNRXhiNW5tM1Y4QnE2S3duQ3U4em1pNXBDMGdhc0p6L3BuNTI0?=
 =?utf-8?B?aURwRzMrd1hnazlTMDh5VmNZdXNBYkR5dEhyNkgraDdGQ0l0UXQ1L3lOYWFI?=
 =?utf-8?B?OUVPa2xycS90K1lweUlwVWx3K1Nnd3Z3WmNyenFoUlRWaFQydDBLY0Zpbm9x?=
 =?utf-8?B?YlVNdzNFTk1IdWpXMm9za3VJdzVWYzNJdTUvVzRXTHQ2TzhRbHc5M3ZHM1or?=
 =?utf-8?B?V0dEUXVaY3FNcmNpaUVNQWxuTzhKanpIWE16TWZZMDJBYVUyakl4a1VaU3Vi?=
 =?utf-8?B?UnZnMHNOaGx6S2xKdTN4WEswc2Q3OGFmKzRlMk01SjUyMUVIaEozVXdXTFor?=
 =?utf-8?B?QUp2WkI2d1I5ODVqY29RcEdFUEFMSzJhelBEOUlhaVI1VVNvQVQ2WGd4TXJq?=
 =?utf-8?B?U3pVODY4OUpENm9aMGRuVXBmSnA0MUJzTjkreWVWUWVYWWQ1cWRIM09tVnlo?=
 =?utf-8?B?SklpNndVTFFGbXVYeVd3eGZNaFJuSnhyMU05dnIzaTBnQ0E1YzlUKzltVVdv?=
 =?utf-8?B?ODhEcENHOE54Z0VKdmM0QnhXM2UxL3JYOGVNV3ZRZ2x6bitVdzNxbUxZcXMy?=
 =?utf-8?B?ZWljQmttL0h4M2V1bzJIWGZZOFY1dDRGVzVUdk4xTTczQ0tIUmNGSjhibTFU?=
 =?utf-8?B?MXFDZFJZUEVSbjRKWmxoakVXYWhBdGh5MjV5eTQwY015ZUJkYnVUQjIwSWtV?=
 =?utf-8?B?U2R2ZngxQ09TZ0pja2hEVmYxN3VCcVJhMVl3SXBBajJuWEQrdHlMamdmSGti?=
 =?utf-8?B?Q0lvcXYrdlB5VWVCM2JCVDJsYXdYdjZPdDNmTytXdlpienR6SU04T3dGQVBI?=
 =?utf-8?B?WWdvT1MvNnFtUFBzMjEvS3ByWTZ3SWNOVkFxNkVxMEt1ZGJQUGtXQXFpeFRE?=
 =?utf-8?B?QStxU24zR2hFN3FSODJEM1ZCRUVZQkk5a1kwSUUxbEVJYldSYUlsOHgvN08v?=
 =?utf-8?B?ejMxZG1SanEyZHNuZ1B0dXFYUHp1NVhUZEx3WGF2a2hoeXdvUGFaRjU3N29L?=
 =?utf-8?B?TjlhMVRCTmtiVUFPdHpjNGE5RDNIMTRMYmMrZkNYUjFxZk5VK1JGckp0U3M5?=
 =?utf-8?B?TTArUEhGMXUwbFpmenFRdExnbU8xU2VOZVc4RmxnSzVoRkhsTm1GenNxTk9w?=
 =?utf-8?B?cEVFK0I4WkVEeittQmhDdlU0TlhVdU5IRXl0RjdybGpHUG1lUmZVcXEvV0Zq?=
 =?utf-8?B?dklQYUttbjlLMnhkSlBjQ05wa2xlMlNyMHV5WVJRNFJEeHYvYnd6Z3FkRkkx?=
 =?utf-8?B?bUJqbmg1WE83T2pMZ3pEeXlxZ2d6TTNaZkxVK2tNaUNyczRuT2t3dnBwMzNt?=
 =?utf-8?B?aDArNGxLSlRJL1V2T3k0VmlYbG9kem1ML0UzZ3FWZDhNRzN3bURHYnVXRXpo?=
 =?utf-8?B?azJvYnNBU2RIVk1nZEZoczU0SG5GUnNDbnNKVS9ldGZZNW04TUhFZ0tEZURq?=
 =?utf-8?B?bkNBYnZjeGxXZmliejlRclR4K04vR2huOE15S1o0N1U3T0dwWUduWHFiRUtp?=
 =?utf-8?B?M1Q4cXp0MEZPNjB6S3czcHloTFE2TlNrWTlRZnFZdTRmUjR3ZUtOQWpObnR2?=
 =?utf-8?B?RFg1Y1hkdWdRSHdkZTFEQ1RuMExUYk5peW9yRlNDUEU3c1VqQWFNK3BpeURr?=
 =?utf-8?B?WGFLbWVJZzJULzd4VUdSTWtUOXNuZmhzNHM4VStkRTcrSFJFZlVZcmozZ0h0?=
 =?utf-8?B?ZHBydlRGY0NuK01pNFFoYi9ORDdYRGp2TEo5c1hnalpDRGRiZWN0K3VOY3ky?=
 =?utf-8?B?SUVuYW9GMGRHbHcwOXZDSmhMcXBEN3hKaElURGdyWUxudm8vR1d3RkhtZ2Z1?=
 =?utf-8?B?ZFpEUTE5MTFCRmFCV3hLeFRUUmtvbHRhNXVoRndQV2VwSkhuZXh2ZFFUNGpB?=
 =?utf-8?B?SEE9PQ==?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: dy1LQL+icHy4N70AXYppXAexEJI/+urR4UtRusw9so3mtVAHHOtjGTQ/4BNnVAVbcET7XKYr+9j52Yklw9hf+NJkN1g3dVphafL7im2sNoiLiQP6z4MJt2Vw1eNdD+nEIm/atI/E5T+vNeFbK+1bTINkG/FmDolpdjpi/y7M0tV5/b8bOBLx26d7VhfMELPN4P3CVPHX/gcFN18u+GiuiA4UY9RGFh+lRtBX9N9qnQCr/ukFHME+UpK8cpivx1WZvJbwVf8Yk4r+MzuOjmUQTHXCkNPWVp97UHRKihr/zcw28sr4gw3h77OX+/Vt5YZQKztGgJWpWyN3K5AK8CpgguAR99AJwVFr2C4+iVe/bsZXU0cM+5rpn3K/7Gt/d7nO9jvzPSP18Kk7HyixlWgOf30SbdOcqFvWkGVqg+HzGiNdxcHM8kC06lavlr2q9mFgVLYcMS+PeE0fuI/PdKF64bzszvHfQjAdcBj8jVVnUfZBX6o0R19EiWHJMO5A/nJVt8dCF3Ylc0nFEJ3Y2ghZAkwyRMvD2PkgUzGcSXIWqrwykrjR5A0YEHfnBCrtVV0R5GZAlz0rXL+HTnEvU7YwVYN+J/N0gj52a8W1eBkkVF+zz54jTszBO5C3iBypPKnF3YnsiDibO7QIyx14Yd6DdVKv7QjGEZyeMMWTXQ2vlup7l90Vttc3puv6yOf7A5uPu/muoghAAyLeSCYGa6mYQ8t89dVPHfKVeevLQ2UrqINsqkDM0pVQbp7mAt5QXSjhjDFpgfM/sNTbQl1Ng0sDxUdN/GgjzSJtQHkcssOZNZAhZD8UOQAPKoPk0H9yYjlmAuL1RSdcuqJ3AZaCdfmAksxulDahXynQfjw+gwsCmCNEPMiJW6NshhVTHJ9ZHD+Z
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 587ab4b8-7737-4fff-c2a5-08db50df5379
X-MS-Exchange-CrossTenant-AuthSource: DS7PR10MB5280.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 May 2023 22:47:11.4502
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 55gGKV36cl5xpaaZRrKt/nUu1nBfKTo1VCsokPzVATDgxRX7JDfKtUlkDL9wFKIBQWUd8UrUHmw8wvNglt926PtKH1xoHMZ+YYgDeVxKl1w=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR10MB4394
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.254,Aquarius:18.0.942,Hydra:6.0.573,FMLib:17.11.170.22
 definitions=2023-05-09_14,2023-05-05_01,2023-02-09_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=999 malwarescore=0
 adultscore=0 spamscore=0 mlxscore=0 phishscore=0 bulkscore=0
 suspectscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2304280000 definitions=main-2305090185
X-Proofpoint-GUID: Q5dE-tjADhR-S9K-T0mKQVofmCeyzxMZ
X-Proofpoint-ORIG-GUID: Q5dE-tjADhR-S9K-T0mKQVofmCeyzxMZ
X-Spam-Status: No, score=-3.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_LOW,
        SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Suravee,

A couple of additional comments below:

On 5/9/2023 7:16 AM, Suravee Suthikulpanit wrote:
> An Interrupt Remapping Table (IRT) stores interrupt remapping configuration
> for each device. In a normal operation, the AMD IOMMU caches the table
> to optimize subsequent data accesses. This requires the IOMMU driver to
> invalidate IRT whenever it updates the table. The invalidation process
> includes issuing an INVALIDATE_INTERRUPT_TABLE command following by
> a COMPLETION_WAIT command.
>
> However, there are cases in which the IRT is updated at a high rate.
> For example, for IOMMU AVIC, the IRTE[IsRun] bit is updated on every
> vcpu scheduling (i.e. amd_iommu_update_ga()). On system with large
> amount of vcpus and VFIO PCI pass-through devices, the invalidation
> process could potentially become a performance bottleneck.
>
> Introducing a new kernel boot option:
>
>      amd_iommu=irtcachedis
>
> which disables IRTE caching by setting the IRTCachedis bit in each IOMMU
> Control register, and bypass the IRT invalidation process.
>
> Co-developed-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
> [Awaiting sign-off-by Alejandro]
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> ---
>   .../admin-guide/kernel-parameters.txt         |  1 +
>   drivers/iommu/amd/amd_iommu_types.h           |  4 +++
>   drivers/iommu/amd/init.c                      | 25 +++++++++++++++++++
>   3 files changed, 30 insertions(+)
[snip]
> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> index fd487c33b28a..01d131e75de4 100644
> --- a/drivers/iommu/amd/init.c
> +++ b/drivers/iommu/amd/init.c
> @@ -160,6 +160,7 @@ static int amd_iommu_xt_mode = IRQ_REMAP_XAPIC_MODE;
>   static bool amd_iommu_detected;
>   static bool amd_iommu_disabled __initdata;
>   static bool amd_iommu_force_enable __initdata;
> +static bool amd_iommu_irtcachedis __initdata;
Lets drop the __initdata attribute above, since amd_iommu_irtcachedis is 
used by early_enable_iommus(), which is in .text (causes modpost warning).

[snip]
>   
> +static void iommu_enable_irtcachedis(struct amd_iommu *iommu)
> +{
> +	u64 ctrl;
> +
> +	if (amd_iommu_irtcachedis) {
> +		/*
> +		 * Note:
> +		 * The support for IRTCacheDis feature is dertermined by
> +		 * checking if the bit is writable.
> +		 */
> +		iommu_feature_enable(iommu, CONTROL_IRTCACHEDIS);
> +		ctrl = readq(iommu->mmio_base +  MMIO_CONTROL_OFFSET);
> +		ctrl &= (1ULL << CONTROL_IRTCACHEDIS);
> +		if (ctrl)
> +			iommu->irtcachedis_enabled = true;
> +		pr_info("iommu%d (%#06x) : IRT cache is %s\n",
> +			iommu->index, iommu->devid,
> +			iommu->irtcachedis_enabled ? "disabled" : "enabled");
> +	}
> +}
> +
>   static void early_enable_iommu(struct amd_iommu *iommu)
>   {
>   	iommu_disable(iommu);
> @@ -2710,6 +2732,7 @@ static void early_enable_iommu(struct amd_iommu *iommu)
>   	iommu_set_exclusion_range(iommu);
>   	iommu_enable_ga(iommu);
>   	iommu_enable_xt(iommu);
> +	iommu_enable_irtcachedis(iommu);
>   	iommu_enable(iommu);
>   	iommu_flush_all_caches(iommu);
>   }
I need to understand better the code flow around kdump, and it is not 
clear from my reading of the spec that this is required, but shouldn't 
iommu_enable_irtcachedis() also be called in the else{} block of 
early_enable_iommus()?

Thank you,
Alejandro