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: <d85d91f2-25e7-f706-094b-56c417becf1c@intel.com>
Date:   Fri, 2 Dec 2022 13:16:47 -0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Firefox/102.0 Thunderbird/102.5.0
Subject: Re: [patch 33/33] irqchip: Add IDXD Interrupt Message Store driver
Content-Language: en-US
To:     Thomas Gleixner <tglx@linutronix.de>,
        LKML <linux-kernel@vger.kernel.org>
CC:     <x86@kernel.org>, Joerg Roedel <joro@8bytes.org>,
        Will Deacon <will@kernel.org>, <linux-pci@vger.kernel.org>,
        Bjorn Helgaas <bhelgaas@google.com>,
        Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
        "Marc Zyngier" <maz@kernel.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Jason Gunthorpe <jgg@mellanox.com>,
        Dave Jiang <dave.jiang@intel.com>,
        "Alex Williamson" <alex.williamson@redhat.com>,
        Kevin Tian <kevin.tian@intel.com>,
        Dan Williams <dan.j.williams@intel.com>,
        Logan Gunthorpe <logang@deltatee.com>,
        Ashok Raj <ashok.raj@intel.com>, Jon Mason <jdmason@kudzu.us>,
        Allen Hubbe <allenbh@gmail.com>,
        "Ahmed S. Darwish" <darwi@linutronix.de>
References: <20221111133158.196269823@linutronix.de>
 <20221111135207.141746268@linutronix.de>
 <4a15c569-0545-20ac-e74c-ae17f7eb067d@intel.com> <87sfhxoa7i.ffs@tglx>
From:   Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <87sfhxoa7i.ffs@tglx>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?T1BWMlBjUUxkR2had1dxUkpmRHdrL2xJNjdyN2dCdlRZbDhmeHAwdjJlMHRp?=
 =?utf-8?B?eUhSRUZ0cGkrbHZIZHIxYitVUCtMc2pIaVJNK1I0eTFZbUFvUHByR25jcTVU?=
 =?utf-8?B?RnlJRkc0Rml4KzRrL3BqNTRQY2dSL0Y2Rmt3Q3RWZ2FmQncrVFZPeHBSQld6?=
 =?utf-8?B?Y051NHAvNUpSdFUzOU1oUVU0T3JmMmRQK3JhTHUyb1BmUm1KYjlEVEpGMGFy?=
 =?utf-8?B?OTVIWW1BdGhhRGlqMUJtclVhNHFNL1pPeGRsWlJ2dVVWcG51eUdmbUNyUng0?=
 =?utf-8?B?UXEyWmQvd0gvY3VpRDVNbFF5L2tGcTI1OWxTNklPaVIvdGw0ZCtmdnpUbENq?=
 =?utf-8?B?Q2JneVlqSWZxVEpTOE04OVVlZ2N2dUJhM1MySEVTZDhNQnFhblVXY1Vqc1pz?=
 =?utf-8?B?U1ZCQlcxTUZ6dTg2TkNsZHltWmgybUFFdEVnUUUrV09WdFJiMGJlcjIvK3RZ?=
 =?utf-8?B?UlZ2MUVYc2dxSUtVWjZ3ZkNPV2o4VXZyc3pSSnhjRk5uUVVzMFc0ZkNBNnJZ?=
 =?utf-8?B?cngxVmw2M05HTzZ3Z2FJaVVFTHlVL0pISTZBUjhVUE43OFBuRzNtWmJ1NUFL?=
 =?utf-8?B?YU16cU5xVlRmSDI5WU1wZnEwWDJUaDR3R09MTmRyM3hvd1dUTytLNWQxQWp1?=
 =?utf-8?B?Z0F4VE40NVREaCt0MERoR2dVVkxRMGthR3ZUUERPSnBKTUFoM29jWDBOYmU4?=
 =?utf-8?B?ZDJSeUNHUmU1M0NtVVcrc2RKWWJIeUlTK1l3K0FjZzk3SGxqMGdVMFYraWti?=
 =?utf-8?B?akRoUGN3clRxR3JKUTA0WVFEaXJ3Q1dTMFVwQWVRNWdKQUJab25wVDBsTmxR?=
 =?utf-8?B?WXJ6YkpodkVXS1ZiRmh2cy9RYllrdWJudCs1enYvanVsVEdKZEtKRTR3YStu?=
 =?utf-8?B?S0l6V1JLano0a256c0RtQ3lvREdMZ21YdTFUcHJIcGxLMUlDRWxDd1JDdHRt?=
 =?utf-8?B?RzJRQU00SGxQZTFzNU4yY1pURnBhTVhHNzZkWmdBYUJscTl6NlVhdzFXcklY?=
 =?utf-8?B?bHZoMm16ZDBzSzJIWEN2Nmt4V0VJUjVYM3ZheWZnTTZjUGZIeVNGamExWHhn?=
 =?utf-8?B?M0N5WlBOMGlwYVpyQ1lQUG1KdVpuM3VoYjRqWTNDVGVaeXJKWU42SHM0amg1?=
 =?utf-8?B?UVVDRVhRemt1K0psNGk2ZXByQWk2WnVqSkh3eHlQQWdaUFhOZ3c5d3VPSTVZ?=
 =?utf-8?B?aVJuamJ2WXY1UndtOXJUWHpsbWdOUWFkWldyc2pGR0hYVWlxWWxBT0xxRHQz?=
 =?utf-8?B?Mjl4U2MzUUpVbDk1YzV1ZndXUEhqVjVnU1dnb1NnajYxQ2ZORStNVUcyb3Za?=
 =?utf-8?B?K2c4c3lScXc2WFZuWDhxQnQrVDRleER3SlRnL2Z3VS9DdkZPd2tLNS9EdllJ?=
 =?utf-8?B?WDdRTEVJZFRsMVlzMnFSTnlzN3RERStrWGZ3enJnOVczOXJ5M3BIbkdYck95?=
 =?utf-8?B?V3JlZGxyektnWldFVFBjMSt1bDdtOVI4cnFXVytWbTFSK3l0OUFDMS9hTTY4?=
 =?utf-8?B?Nkx2UDF2WDc1Yld4V2t5bEwrVXNEenA3UExvL3pvWWFIMDJTWFVFTWpsU3ZX?=
 =?utf-8?B?K1hrdzRPMGVGbHlyaWZHdWRBZStQcnR4UUU1c2FTM2hzTVdSQmltWTR0U2dM?=
 =?utf-8?B?bmxjclk3V3dxZ2Z0N0wySXF2UG9Ha1FKdU1UWUpWTHExL2hDVVYyV3Y2ODZP?=
 =?utf-8?B?eUJ0TExxUjFzaW9GZVlubWdKOE1uaEs1QkYyL0ZOSmI2aU81cWY0ek1Ma0Jl?=
 =?utf-8?B?TlhjRFNNMklhY1ljWXB0eW5FUUppUk1UMWtzSXhMVzBhUzloZ0pKckU1MU4z?=
 =?utf-8?B?WmpmMGFkN2FEUDdTR2dSUmV1aENRaTlCT2daT0puWTdpTFY0VS9peS9ucC9j?=
 =?utf-8?B?bTJNVDdPRnlUN1ozZFM0am9ob2tQTmNLMHBJLy9zU1FQY0Y2TE1ob2E4YnR2?=
 =?utf-8?B?QVBkQnFlbGh5YnQ1cWcvOGpEYVgvWnRmZTdmVWtjODV3WGhkc1k1bjZ0dlZj?=
 =?utf-8?B?Y3JBR0VQemh1M0ZTNEkrenBheVdWNE4zaFVsNUhJS3NvK05maXNjUWpxQVRE?=
 =?utf-8?B?dnNUSXFGckJzZUJWMkI2bkk1ZkhVUW1XSGhGOEtrTVRramFlQVRUMXF2Y3Vz?=
 =?utf-8?B?OVZFYUVVbG9PN2dMOHVkTS81WnRXTEs4a0RGdHhZbk5Vb2JhRk1za1VRTkdT?=
 =?utf-8?B?MXc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 8739833d-3f2f-4e99-5734-08dad4aa8736
X-MS-Exchange-CrossTenant-AuthSource: CY4PR11MB1862.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 Dec 2022 21:16:50.6235
 (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: FO4DhAa3gr3ZvAR99N/5H2JVPlYbjPQxiyy7ZzEmS5fZ5JRtakn/DwBpD67/vXZPIhtnDodgd6q/frCYdX7BbCUzagF5Mg+X0atcIJjoTME=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN9PR11MB5276
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.7 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 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 Thomas,

On 12/2/2022 11:51 AM, Thomas Gleixner wrote:
> On Fri, Dec 02 2022 at 09:55, Reinette Chatre wrote:
>> On 11/11/2022 5:59 AM, Thomas Gleixner wrote:

>> @@ -33,6 +34,8 @@ struct ims_slot {
>>  #define CTRL_PASID_ENABLE	BIT(3)
>>  /* Position of PASID.LSB in the control word */
>>  #define CTRL_PASID_SHIFT	12
>> +/* Valid PASID is 20 bits */
>> +#define CTRL_PASID_VALID	GENMASK(19, 0)
>>  
>>  static inline void iowrite32_and_flush(u32 value, void __iomem *addr)
>>  {
>> @@ -93,12 +96,17 @@ static void idxd_prepare_desc(struct irq_domain *domain, msi_alloc_info_t *arg,
>>  	/* Mask the interrupt for paranoia sake */
>>  	iowrite32_and_flush(CTRL_VECTOR_MASKBIT, &slot->ctrl);
>>  
>> -	/*
>> -	 * The caller provided PASID. Shift it to the proper position
>> -	 * and set the PASID enable bit.
>> -	 */
>> -	desc->data.icookie.value <<= CTRL_PASID_SHIFT;
>> -	desc->data.icookie.value |= CTRL_PASID_ENABLE;
>> +	if (pasid_valid((ioasid_t)desc->data.icookie.value)) {
>> +		/*
>> +		 * The caller provided PASID. Shift it to the proper position
>> +		 * and set the PASID enable bit.
>> +		 */
>> +		desc->data.icookie.value &= CTRL_PASID_VALID;
>> +		desc->data.icookie.value <<= CTRL_PASID_SHIFT;
>> +		desc->data.icookie.value |= CTRL_PASID_ENABLE;
>> +	} else {
>> +		desc->data.icookie.value = 0;
>> +	}
> 
> Looks about right. But that needs some sanity measures at the call sites
> so that we don't end up with an invalid PASID in cases where a valid
> PASID is truly required.

I will take a closer look at this. Current call site is explicit to
set an invalid PASID when PASID use is disabled. I still need to do testing
with valid PASID to learn those flows.

Reinette