From: "Raj, Ashok" <email@example.com> To: Andy Lutomirski <firstname.lastname@example.org> Cc: Ravi V Shankar <email@example.com>, Peter Zijlstra <firstname.lastname@example.org>, Dave Hansen <email@example.com>, H Peter Anvin <firstname.lastname@example.org>, Jean-Philippe Brucker <email@example.com>, Dave Jiang <firstname.lastname@example.org>, Ashok Raj <email@example.com>, x86 <firstname.lastname@example.org>, amd-gfx <email@example.com>, Christoph Hellwig <firstname.lastname@example.org>, Ingo Molnar <email@example.com>, Fenghua Yu <firstname.lastname@example.org>, Borislav Petkov <email@example.com>, Thomas Gleixner <firstname.lastname@example.org>, Tony Luck <email@example.com>, Felix Kuehling <Felix.Kuehling@amd.com>, linux-kernel <firstname.lastname@example.org>, iommu <email@example.com>, Jacob Jun Pan <firstname.lastname@example.org>, David Woodhouse <email@example.com> Subject: Re: [PATCH v6 12/12] x86/traps: Fix up invalid PASID Date: Mon, 3 Aug 2020 08:19:21 -0700 [thread overview] Message-ID: <20200803151921.GA387967@otc-nc-03> (raw) In-Reply-To: <CALCETrXLFwzCzoE8ZjciBO_WSK8StyTfO1yXVm4v2qFQZpfasg@mail.gmail.com> On Mon, Aug 03, 2020 at 08:12:18AM -0700, Andy Lutomirski wrote: > On Mon, Aug 3, 2020 at 8:03 AM Dave Hansen <firstname.lastname@example.org> wrote: > > > > On 7/31/20 4:34 PM, Andy Lutomirski wrote: > > >> Thomas suggested to provide a reason for the #GP caused by executing ENQCMD > > >> without a valid PASID value programmed. #GP error codes are 16 bits and all > > >> 16 bits are taken. Refer to SDM Vol 3, Chapter 16.13 for details. The other > > >> choice was to reflect the error code in an MSR. ENQCMD can also cause #GP > > >> when loading from the source operand, so its not fully comprehending all > > >> the reasons. Rather than special case the ENQCMD, in future Intel may > > >> choose a different fault mechanism for such cases if recovery is needed on > > >> #GP. > > > Decoding the user instruction is ugly and sets a bad architecture > > > precedent, but we already do it in #GP for UMIP. So I'm unconvinced. > > > > I'll try to do one more bit of convincing. :) > > > > In the end, we need a way to figure out if the #GP was from a known "OK" > > source that we can fix up. You're right that we could fire up the > > instruction decoder to help answer that question. But, it (also) > > doesn't easily yield a perfect answer as to the source of the #GP, it > > always involves a user copy, and it's a larger code impact than what > > we've got. > > > > I think I went and looked at fixup_umip_exception(), and compared it to > > the alternative which is essentially just these three lines of code: > > > > > + /* > > > + * If the current task already has a valid PASID in the MSR, > > > + * the #GP must be for some other reason. > > > + */ > > > + if (current->has_valid_pasid) > > > + return false; > > ...> + /* Now the current task has a valid PASID in the MSR. */ > > > + current->has_valid_pasid = 1; > > > > and *I* was convinced that instruction decoding wasn't worth it. > > > > There's a lot of stuff that fixup_umip_exception() does which we don't > > have to duplicate, but it's going to be really hard to get it anywhere > > near as compact as what we've got. > > > > I could easily be convinced that the PASID fixup is so trivial and so > obviously free of misfiring in a way that causes an infinite loop that > this code is fine. But I think we first need to answer the bigger > question of why we're doing a lazy fixup in the first place. We choose lazy fixup for 2 reasons. - If some threads were already created before the MSR is programmed, then we need to fixup those in a race free way. scheduling some task-work etc. We did do that early on, but decided it was ugly. - Not all threads need to submit ENQCMD, force feeding the MSR probably isn't even required for all. Yes the overhead isn't probably big, but might not even be required for all threads. We needed to fixup MSR in two different way. To keep the code simple, the choice was to only fixup on #GP, that eliminated the extra code we need to support case1. Cheers, Ashok _______________________________________________ iommu mailing list email@example.com https://lists.linuxfoundation.org/mailman/listinfo/iommu
next prev parent reply other threads:[~2020-08-03 15:19 UTC|newest] Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-07-13 23:47 [PATCH v6 00/12] x86: tag application address space for devices Fenghua Yu 2020-07-13 23:47 ` [PATCH v6 01/12] iommu: Change type of pasid to u32 Fenghua Yu 2020-07-14 2:45 ` Liu, Yi L 2020-07-14 13:54 ` Fenghua Yu 2020-07-14 13:56 ` Liu, Yi L 2020-07-22 14:03 ` Joerg Roedel 2020-07-22 17:21 ` Fenghua Yu 2020-07-13 23:47 ` [PATCH v6 02/12] iommu/vt-d: Change flags type to unsigned int in binding mm Fenghua Yu 2020-07-13 23:47 ` [PATCH v6 03/12] docs: x86: Add documentation for SVA (Shared Virtual Addressing) Fenghua Yu 2020-07-14 3:25 ` Liu, Yi L 2020-07-15 23:32 ` Fenghua Yu 2020-07-13 23:47 ` [PATCH v6 04/12] x86/cpufeatures: Enumerate ENQCMD and ENQCMDS instructions Fenghua Yu 2020-07-13 23:48 ` [PATCH v6 05/12] x86/fpu/xstate: Add supervisor PASID state for ENQCMD feature Fenghua Yu 2020-07-13 23:48 ` [PATCH v6 06/12] x86/msr-index: Define IA32_PASID MSR Fenghua Yu 2020-07-13 23:48 ` [PATCH v6 07/12] mm: Define pasid in mm Fenghua Yu 2020-07-13 23:48 ` [PATCH v6 08/12] fork: Clear PASID for new mm Fenghua Yu 2021-02-24 10:19 ` Jean-Philippe Brucker 2021-02-25 22:17 ` Fenghua Yu 2021-03-01 23:00 ` Jacob Pan 2021-03-02 10:43 ` Jean-Philippe Brucker 2020-07-13 23:48 ` [PATCH v6 09/12] x86/process: Clear PASID state for a newly forked/cloned thread Fenghua Yu 2020-08-01 1:44 ` Andy Lutomirski 2020-07-13 23:48 ` [PATCH v6 10/12] x86/mmu: Allocate/free PASID Fenghua Yu 2020-07-13 23:48 ` [PATCH v6 11/12] sched: Define and initialize a flag to identify valid PASID in the task Fenghua Yu 2020-07-13 23:48 ` [PATCH v6 12/12] x86/traps: Fix up invalid PASID Fenghua Yu 2020-07-31 23:34 ` Andy Lutomirski 2020-08-01 0:42 ` Fenghua Yu 2020-08-03 15:03 ` Dave Hansen 2020-08-03 15:12 ` Andy Lutomirski 2020-08-03 15:19 ` Raj, Ashok [this message] 2020-08-03 16:36 ` Dave Hansen 2020-08-03 17:16 ` Andy Lutomirski 2020-08-03 17:34 ` Dave Hansen 2020-08-03 19:24 ` Andy Lutomirski 2020-08-01 1:28 ` Andy Lutomirski 2020-08-03 17:19 ` Fenghua Yu
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200803151921.GA387967@otc-nc-03 \ --firstname.lastname@example.org \ --cc=Felix.Kuehling@amd.com \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --subject='Re: [PATCH v6 12/12] x86/traps: Fix up invalid PASID' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).