Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [RFC PATCH 11/13] x86/uintr: Introduce uintr_wait() syscall
To:     Thomas Gleixner <tglx@linutronix.de>,
        Andy Lutomirski <luto@kernel.org>,
        the arch/x86 maintainers <x86@kernel.org>
CC:     Tony Luck <tony.luck@intel.com>,
        Dave Hansen <dave.hansen@intel.com>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        "H. Peter Anvin" <hpa@zytor.com>, Jens Axboe <axboe@kernel.dk>,
        Christian Brauner <christian@brauner.io>,
        "Peter Zijlstra (Intel)" <peterz@infradead.org>,
        Shuah Khan <shuah@kernel.org>, Arnd Bergmann <arnd@arndb.de>,
        Jonathan Corbet <corbet@lwn.net>,
        Raj Ashok <ashok.raj@intel.com>,
        Jacob Pan <jacob.jun.pan@linux.intel.com>,
        Gayatri Kammela <gayatri.kammela@intel.com>,
        Zeng Guang <guang.zeng@intel.com>,
        "Williams, Dan J" <dan.j.williams@intel.com>,
        Randy E Witt <randy.e.witt@intel.com>,
        "Shankar, Ravi V" <ravi.v.shankar@intel.com>,
        Ramesh Thomas <ramesh.thomas@intel.com>,
        Linux API <linux-api@vger.kernel.org>,
        <linux-arch@vger.kernel.org>,
        "Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
        <linux-kselftest@vger.kernel.org>
References: <20210913200132.3396598-1-sohil.mehta@intel.com>
 <20210913200132.3396598-12-sohil.mehta@intel.com>
 <f5a971e4-6b0d-477f-992c-89110a2ceb03@www.fastmail.com>
 <c6e83d0e-6551-4e16-0822-0abbc4d656c4@intel.com>
 <fd54f257-fa02-4ec3-a81b-b5e60f24bf94@www.fastmail.com> <877dex7tgj.ffs@tglx>
 <b537a890-4b9f-462e-8c17-5c7aa9b60138@www.fastmail.com> <87tui162am.ffs@tglx>
 <25ba1e1f-c05b-4b67-b547-6b5dbc958a2f@www.fastmail.com> <87pmsp5aqx.ffs@tglx>
 <0364c572-4bc2-4538-8d65-485dbfa81f0d@www.fastmail.com> <875yug4eos.ffs@tglx>
From:   Sohil Mehta <sohil.mehta@intel.com>
Message-ID: <410ccb21-49d8-4ec0-3f23-07e37c694bbe@intel.com>
Date:   Fri, 1 Oct 2021 16:00:44 -0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101
 Firefox/78.0 Thunderbird/78.14.0
In-Reply-To: <875yug4eos.ffs@tglx>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Content-Language: en-US
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?TlliNllSbmM2bWtHdFJFUXRLV3U5V0VEWG5Bc2xKMSt2UzdoNDlYYW9xNjhF?=
 =?utf-8?B?b0lNRTI4K200L1JqNEVkcFRKcGE0WUVGYkI0cndFb2ZNZ3pJRWpJRFZ0bEZM?=
 =?utf-8?B?WGh0VldacnpWMXFwMW10OUd3K2dzWDZQREdYeEFyNlpkR01RajhmbnZuVjVO?=
 =?utf-8?B?N0l4UmUxZjNjdGFnWHVCeUl3OUFLb3gzbmVUZEh5RDdOeDJ4M3c0L1dsY0Ji?=
 =?utf-8?B?N0hhK1Q2RDZUY2h2Tmp6RVYrS3E5cUhUdTVKNlNpUEI3ZmdSdUZHUTVNM0xj?=
 =?utf-8?B?NkNENW5MbE5pR1MxSmpFTnN6VUEwUE5naDkrRml5d05MUlZsZDFpaVBKR01w?=
 =?utf-8?B?eDQzWnBUWXdobWxnOUh5NWpRdExHb2xpYUxscHdEdHB5S3h2aEtYUW1jT2tN?=
 =?utf-8?B?N0dJYW9aOWlqM3N5U2twQU1mOEgva01MOHhGU1h0RUo0cHBsVEczQmI5cUJa?=
 =?utf-8?B?ODVOdkg1Qm9QZ1gyYlFzZXMwS3JnR1BGOGhZL2llY1RiYUpTanlQS2VLa1Rm?=
 =?utf-8?B?M0RaVUhRRkZLTGtzY3pkZmE4MHg2L1hBRDlhM2FNN1B6UnNGUjNnQTl2SDVP?=
 =?utf-8?B?RmRNa2czWklHb1MvNmJJOVNqR1lxUk5WWkpMVENYVjhqRXpZZm0zK3UwVlZi?=
 =?utf-8?B?bE1TaDVjSU50ckkyZnBYYVVxK21iNGNmQ1p2MWNpNldmT0pNTUlqWjE5UFBh?=
 =?utf-8?B?Q1FLK1k0UWhobFNtQ2hwM1BpTi9SbUlsQjcxN1dSQ1RXZVBLS3U4bXRTL0cw?=
 =?utf-8?B?RUI4Qmc3ZFVhOUkrbWE1Q0tsa3ZBMWNPd1BEd2RKU2ttOUtxNUh2bnEwdnJE?=
 =?utf-8?B?clVoVFNnQm5GNFQzRUlEZXVja0FFU3FseHluU0FJb0tuM003d2RPQ2ZEMDVZ?=
 =?utf-8?B?NHQ4TlBrSGpla3dEUGFVWU1LTVozbnBWODNUQmtOLy9KNGpXN0wxUUNkbWUv?=
 =?utf-8?B?YnYzZEFsTVNUWkN5aC94REg2UkNIb1ZUZENTaWgzd0RMaEJqNmpTSVZLUEZS?=
 =?utf-8?B?dklaSjQrQ0JaR0NZVXlBbS9jalN3T1o3UVVoUjRPMGhTT3lpSVdvdGNQZENY?=
 =?utf-8?B?anZZbS8zRGpoNnhZeUMzRGluemx5aXU0dUQvN2NmMXBsMTZPckZsdWFMQW9M?=
 =?utf-8?B?UUliNmJZMHo2dXZzKzRtc0R6RTkwTkxTemZmMjZpclJIVnIwRnlISXZrQWlB?=
 =?utf-8?B?dTNFd0lLN2dXM29rTnRHcU5ITUwzNTlwMW9Yc3JBb0o2REJ4enR4WSsxSUtM?=
 =?utf-8?B?SnAzNkhxZ3dVVlFMNTYySlJIVUE2c29tZ0pFSHVtMGdKSFNZL2pVQ2hsK3hn?=
 =?utf-8?B?VlRPMldYU25zaEg0V3R1cDJ1NmdnOTRCQnQ1OGhwMnM5MmhlaDd4ekg3WTc2?=
 =?utf-8?B?dTV4aWp6Q1JHK1Fid1VtSUtkcHJuZG4vOTRxU05pR3d1UWZqTHZWU1dKNXds?=
 =?utf-8?B?TUtxaWhrM3lhbEN1YjRoekpENm82VlVQOU1BTytaOFhTVks0VkdrSUI3VU92?=
 =?utf-8?B?Sk03aVk4dzBjdU5JQytGNjRmb3FVOTR4ekgzZUpDTmE2cW9YdWFrZGZaekVv?=
 =?utf-8?B?S212SFRnY0tyaHNWN0NtMWJUa1hnTUFQYnByUkJYQjd2TFl3c1JpTW9MdFhT?=
 =?utf-8?B?WTNJYjEzNlZnOWs0cHpLcFVMa29XWXhJMXB5S0pIZFZPaVk0L3dwYWZaeWRy?=
 =?utf-8?B?cWZhTkQvZVYxbUZYWFNRanZjQVVuK1NrZVpDaHZHSGJrb3hOdlI0SWgxYlRC?=
 =?utf-8?Q?k3AudYM0x1hERRaObsE7NS9cGFn8BkmlhuARK6l?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 3e2f4270-7af8-4f9f-4ba6-08d9852f4e39
X-MS-Exchange-CrossTenant-AuthSource: BYAPR11MB3320.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 01 Oct 2021 23:00:47.4219
 (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: xxEBNaSe70ajGWMRih/t3d4/y1/OzOOxBDuvGHTrBjbVMz33VTobzpVrnKlF7IpKOhVn4mOlsUs9CN0sE1mKTQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR11MB5037
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 10/1/2021 2:29 PM, Thomas Gleixner wrote:
> So we'd end up with two XSAVES on context switch. We can simply do:
>          XSAVES();
>          fpu.state.xtsate.uintr.uinv = 0;


I am a bit confused. Do we need to set UINV to 0 explicitly?

If XSAVES gets called twice during context switch then the UINV in the 
XSTATE buffer automatically gets set to 0. Since XSAVES saves the 
current UINV value in the MISC_MSR which was already set to 0 by the 
previous XSAVES.

Though, this probably happens due to pure luck than intentional design :)

> which allows to do as many XRSTORS in a row as we want. Only the final
> one on the way to user space will have to restore the real vector if the
> register state is not valid:
>
>         if (fpu_state_valid()) {
>              if (needs_uinv(current)
>                 wrmsrl(UINV, vector);
>         } else {
>              if (needs_uinv(current)
>                 fpu.state.xtsate.uintr.uinv = vector;
>              XRSTORS();
>         }

I might have missed some subtle difference. Has this logic changed from 
what you previously suggested for arch_exit_to_user_mode_prepare()?

        if (xrstors_pending)) {
             // Update the saved xstate for xrstors
             // Unconditionally update the UINV since it could have been 
overwritten by calling XSAVES twice.
                current->xstate.uintr.uinv = UINTR_NOTIFICATION_VECTOR;
                 current->xstate.uintr.uirr |= pir;
         } else {
                 // Manually restore UIRR and UINV
                 rdmsrl(IA32_UINTR_RR, uirr);
                 wrmsrl(IA32_UINTR_RR, uirr | pir);

             misc.val64 = 0;
                 misc.uittsz = current->uintr->uittsz;
                 misc.uinv = UINTR_NOTIFICATION_VECTOR;
                 wrmsrl(IA32_UINTR_MISC, misc.val64);
         }

> Hmm?


The one case I can see this failing is if there was another XRSTORS 
after the "final" restore in arch_exit_to_user_mode_prepare()? I think 
that is not possible but I am not an expert on this. Did I misunderstand 
something?

Thanks,
Sohil