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: <5044217c-9380-f50a-2809-0e3dc80f7bd2@intel.com>
Date:   Fri, 16 Jun 2023 11:06:24 +0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.11.2
Subject: Re: [PATCH] iommu/vt-d: Fix to flush cache of PASID directory table
Content-Language: en-CA
To:     Baolu Lu <baolu.lu@linux.intel.com>, <dwmw2@infradead.org>,
        <joro@8bytes.org>, <will@kernel.org>, <robin.murphy@arm.com>
CC:     <iommu@lists.linux.dev>, <linux-kernel@vger.kernel.org>
References: <20230615071613.690639-1-yanfei.xu@intel.com>
 <3f60b35a-683c-5e18-c9d5-964df9331f0f@linux.intel.com>
From:   Yanfei Xu <yanfei.xu@intel.com>
In-Reply-To: <3f60b35a-683c-5e18-c9d5-964df9331f0f@linux.intel.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?YURPQWNDd0VSM0RtdThXMWpBT1hTVDBjS0JNazBpVTFwK20rTWY4aG9aanA5?=
 =?utf-8?B?OTZiMzg4TlhmU0lVQkFtTHNlOW5iVENiWTg2M2Rxb2JBQmo3cEQ3VExBRVNh?=
 =?utf-8?B?NHh2bE1ZTzdyRENBQ0lJYzkyQ01kUHREOEFYcldxbGxaN1o4RWZlMFQyMXYz?=
 =?utf-8?B?UDFOQ1hDamtqNFkwRzh3RXI5TGN6SmRTMHNjNkJEeU9GNWlQWVkrUSt0clEw?=
 =?utf-8?B?eld3Y3NBUkZHYldWa2pVY3g5SUFpVXFtQ3cvWW1nTlhKZ1FVWFVNQmJMa2tR?=
 =?utf-8?B?NDM5Smx6anYrbkgyK1UxUHQ3VlZtSU9EZm9LalV0Mnp3VWVlQ2kwbURVRHh5?=
 =?utf-8?B?dmluQkxKWFdLQkVzSXZhZVpNSUFqeWZnZ3laMmhnOVJCRVZMc0VDdU9HTlhP?=
 =?utf-8?B?RmNQV0dKU0hnZHZzRVdUQWtkZ0VQYzVGL3BwK0ozT3BNRUNLeFA1SmdtY3Ey?=
 =?utf-8?B?YUN5OXQrK2RaRFhnblJITEh0L3RSZHlHUmVLWXE4WnBCaGsvTWd1S2w3aWlO?=
 =?utf-8?B?L3UyRHZLS3RYSmlYT0VvYmY4cmE4K3VMaC94VVVjS3VaL01iZnU5MncyenMr?=
 =?utf-8?B?M3BtNStlVUl6YkYwUERZVm1zVGh5WGk4c0V3ZXNqMFBLeS8rS3cyQnhzWWd6?=
 =?utf-8?B?cThtcWNPWllVdUlhbjNlUlhleTJMR0VObGVFaVJTR0xxemZkR0tsL0JhUEla?=
 =?utf-8?B?TDVaM1hUb3hrVWQwNUF2RjdFeTNhaEMxdjlZS0cvdTV5b0dTaXdJZlNCaFJ0?=
 =?utf-8?B?NWJmdEFJWTRqNmRHSE0rZThJdVB4OUFxVGxYdXIyK3NCOElMb1JFdzRnTlBh?=
 =?utf-8?B?aHNiYXZGSXNZaGhYMW9FS21FdE44NzJIYXlQNFJzRS9KOUZXN3Z6aDMrK2pN?=
 =?utf-8?B?WWhMSGs2czB6T0FEU20zOFZZaEVtZmNjVHUxOXlKbThjZW9xLzFQbmxic0Vt?=
 =?utf-8?B?dVc1UGdZMURnMmcrV0ljdytCcmJ1R2FqNUo1SmJXZ3hGZDF1enArMUs2cXhP?=
 =?utf-8?B?MWRpTmVUbGZEVHZTT01xa0RsR0NpYUVvVlB1cmxrVHA5M3VSdVU3cTk1NXV0?=
 =?utf-8?B?U0U4TjJZcjhMMzZkRldhUVJ5Z255bS9DNVk2TExwaDJBLzZka21zQndSbThO?=
 =?utf-8?B?MHptQXBnZDZJZjEyY25qOUdmam5kUEpqSkNuVFNFT2M1UXhpMFBOOTQ0QWdu?=
 =?utf-8?B?VnR1Z2tHbjZwVUg5ZnFEQm0rZ0kxd1hWNnlQR0cwaWxmZm1FWlRSY0puVjRJ?=
 =?utf-8?B?OGU0NmhoOFVmM2V2aFJiVUd4Y1B0cjR0aUFrTndaM29pd2JDRzcxZXBnTDEy?=
 =?utf-8?B?V2NZNFJLOVRqem5kVlFhQ0xYOWU2TExLYnN0eHNKZ2xHQUV2WUplNkZnbnM4?=
 =?utf-8?B?WDVEREV5YkhaaGczKzZRZW5kTHlva0VCSUtDU1lkdlpzQXV0Y2JFZlFWQ2NO?=
 =?utf-8?B?ei9VbUdpWmFJWmFRRW94Q2Y5cXhDU1VNUEZJcWtnTXR2cUI2bzRKYjBPdTQ2?=
 =?utf-8?B?STRrUDVmajNUcGpHOFZpbXN4RkExeWNzYVRmditHdDgrUjlnSHR6R1hNT3R2?=
 =?utf-8?B?SU5WTTlqME9wTkdTQlU1U1QxZG1XSk5jRkdSa2NwS1pUd0haRFpYOTMxYVRU?=
 =?utf-8?B?UjF0UExmVmdzU1RxelFWdGtLQjhMRlI0ZkcxM0phZ1E0NGhUOFpoT1VrRkJu?=
 =?utf-8?B?M05Yb2Z1YmtQVlFBWkl3TzVJS09aZmNyQm8zb1RlWnZPTVR6aHFIT1o2WUwz?=
 =?utf-8?B?eUcwYktoNkszT1VCbmN3WmExVldZQTJZbmloRGlySE9tUWkwdnJVcTd0TzRa?=
 =?utf-8?B?ajdlaVVQdVhKdnNSaFMySmRkL2FNais5Ry84SlRySnFxd21zZHNEZk0yMmlM?=
 =?utf-8?B?bzZCcmNXaHVsY1lWMkRZeml4SXRvR0xaUXlKZy8xTm1TWUtzeTdnQUJsckts?=
 =?utf-8?B?T2VPRE1pY21OMytJRHpvdU1KdmF0N3Vma244MGhjM29lT2ZFZWpNUmxCUWVC?=
 =?utf-8?B?OVRNUjFid0NDU0xDNWZtTDVxQ2tiY00zODBvbGFNdnlCaHNXN3M3WGh1ais1?=
 =?utf-8?B?c3gxT3NrTGpJbDR6cUVpNFZ2cHczSEJ6eUFsSUxLbmc1bFB5aVlDdWNqTDA3?=
 =?utf-8?Q?ZEvEjGDVtU9yHzC9FoLfpGAT5?=
X-MS-Exchange-CrossTenant-Network-Message-Id: f0a750ae-257e-48cb-d05b-08db6e16b094
X-MS-Exchange-CrossTenant-AuthSource: DM6PR11MB4138.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Jun 2023 03:06:33.7923
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: sWtnUy+qltNxe1q9H4iMsQAehglLxcGT6P0NL88ve+dhOIkgkk8srlzY1Y10CzK+faEpfTjX8h+IJJE52dT38A==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR11MB6480
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.5 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_MED,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE,
        URIBL_BLOCKED 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


On 6/16/2023 10:01 AM, Baolu Lu wrote:
> On 6/15/23 3:16 PM, Yanfei Xu wrote:
>> Even the PCI devices don't support pasid capability, PASID
>> table is mandatory for a PCI device in scalable mode. However
>> flushing cache of pasid directory table for these devices are
>> not taken after pasid table is allocated as the "size" of
>> table is zero. Fix to assign it with a page size.
>
> Documentation/process/submitting-patches.rst
>
> Please add more information about
>
> - Describe your problem.
> - Any background of the problem?
> - How your change fixes the problem.
> ...
>
Got it! Will improve these in commit message.

Just noticed this when reading the iommu code, no actual problem 
encountered.

>>
>> Fixes: 194b3348bdbb ("iommu/vt-d: Fix PASID directory pointer 
>> coherency")
>
> Do you need a Cc stable?

Yes, will add Cc: <stable@vger.kernel.org>

>
>> Signed-off-by: Yanfei Xu <yanfei.xu@intel.com>
>> ---
>>   drivers/iommu/intel/pasid.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
>> index c5d479770e12..bde7df055865 100644
>> --- a/drivers/iommu/intel/pasid.c
>> +++ b/drivers/iommu/intel/pasid.c
>> @@ -115,7 +115,9 @@ int intel_pasid_alloc_table(struct device *dev)
>>                     intel_pasid_max_id);
>>         size = max_pasid >> (PASID_PDE_SHIFT - 3);
>> -    order = size ? get_order(size) : 0;
>> +    if (!size)
>> +        size = PAGE_SIZE;
>> +    order = get_order(size);
>>       pages = alloc_pages_node(info->iommu->node,
>>                    GFP_KERNEL | __GFP_ZERO, order);
>>       if (!pages) {
>
> Is it similar to
A little difference that real size may less than memory size calculated by
order. But it is no effect. I think this is simpler.

Thanks,
Yanfei

>
> diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
> index c5d479770e12..49fc5a038a14 100644
> --- a/drivers/iommu/intel/pasid.c
> +++ b/drivers/iommu/intel/pasid.c
> @@ -129,7 +129,7 @@ int intel_pasid_alloc_table(struct device *dev)
>         info->pasid_table = pasid_table;
>
>         if (!ecap_coherent(info->iommu->ecap))
> -               clflush_cache_range(pasid_table->table, size);
> +               clflush_cache_range(pasid_table->table, (1 << order) * 
> PAGE_SIZE);
>
>         return 0;
>  }
>
> ?
>
> Best regards,
> baolu