AMD-GFX Archive on lore.kernel.org
 help / color / Atom feed
From: Andy Lutomirski <luto@amacapital.net>
To: Dave Hansen <dave.hansen@intel.com>
Cc: Ravi V Shankar <ravi.v.shankar@intel.com>,
	Peter Zijlstra <peterz@infradead.org>,
	H Peter Anvin <hpa@zytor.com>,
	Jean-Philippe Brucker <jean-philippe@linaro.org>,
	Dave Jiang <dave.jiang@intel.com>,
	Ashok Raj <ashok.raj@intel.com>, Joerg Roedel <joro@8bytes.org>,
	x86 <x86@kernel.org>, amd-gfx <amd-gfx@lists.freedesktop.org>,
	Christoph Hellwig <hch@infradead.org>,
	Ingo Molnar <mingo@redhat.com>, Fenghua Yu <fenghua.yu@intel.com>,
	Borislav Petkov <bp@alien8.de>, Andy Lutomirski <luto@kernel.org>,
	Sohil Mehta <sohil.mehta@intel.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Tony Luck <tony.luck@intel.com>,
	Felix Kuehling <Felix.Kuehling@amd.com>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	iommu <iommu@lists.linux-foundation.org>,
	Jacob Jun Pan <jacob.jun.pan@intel.com>,
	David Woodhouse <dwmw2@infradead.org>,
	Lu Baolu <baolu.lu@linux.intel.com>
Subject: Re: [PATCH v6 12/12] x86/traps: Fix up invalid PASID
Date: Mon, 3 Aug 2020 12:24:44 -0700
Message-ID: <56C96132-6F42-4E50-B712-C7F31CB01C9D@amacapital.net> (raw)
In-Reply-To: <92a14516-1f63-5b5f-7f30-8870fe343c8e@intel.com>




> On Aug 3, 2020, at 10:34 AM, Dave Hansen <dave.hansen@intel.com> wrote:
> 
> On 8/3/20 10:16 AM, Andy Lutomirski wrote:
>> - TILE: genuinely per-thread, but it's expensive so it's
>> lazy-loadable.  But the lazy-load mechanism reuses #NM, and it's not
>> fully disambiguated from the other use of #NM.  So it sort of works,
>> but it's gross.
> 
> For those playing along at home, there's a new whitepaper out from Intel
> about some new CPU features which are going to be fun:
> 
>> https://software.intel.com/content/dam/develop/public/us/en/documents/architecture-instruction-set-extensions-programming-reference.pdf
> 
> Which part were you worried about?  I thought it was fully disambuguated
> from this:
> 
>> When XFD causes an instruction to generate #NM, the processor loads
>> the IA32_XFD_ERR MSR to identify the disabled state component(s).
>> Specifically, the MSR is loaded with the logical AND of the IA32_XFD
>> MSR and the bitmap corresponding to the state components required by
>> the faulting instruction.
>> 
>> Device-not-available exceptions that are not due to XFD — those 
>> resulting from setting CR0.TS to 1 — do not modify the IA32_XFD_ERR
>> MSR.
> 
> So if you always make sure to *clear* IA32_XFD_ERR after handing and XFD
> exception, any #NM's with a clear IA32_XFD_ERR are from "legacy"
> CR0.TS=1.  Any bits set in IA32_XFD_ERR mean a new-style XFD exception.
> 
> Am I missing something?

I don’t think you’re missing anything, but this mechanism seems to be solidly in the category of “just barely works”, kind of like #DB and DR6, which also just barely works.

And this PASID vs #GP mess is just sad. We’re trying to engineer around an issue that has no need to exist in the first place.  For some reason we have two lazy-loading-fault mechanisms showing up in x86 in rapid succession, one of them is maybe 3/4-baked, and the other is totally separate and maybe 1/4 baked.

If ENQCMD instead raise #NM, this would be trivial. (And it even makes sense — the error is, literally, “an instruction tried to use something in XSTATE that isn’t initialized”.). Or if someone had noticed that, kind of like PKRU, PASID didn’t really belong in XSTATE, we wouldn’t have this mess.

Yes, obviously Linux can get all this stuff to work, but maybe Intel could aspire to design features that are straightforward to use well instead of merely possible to use well?
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

  reply index

Thread overview: 32+ 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
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
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 [this message]
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=56C96132-6F42-4E50-B712-C7F31CB01C9D@amacapital.net \
    --to=luto@amacapital.net \
    --cc=Felix.Kuehling@amd.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=ashok.raj@intel.com \
    --cc=baolu.lu@linux.intel.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@intel.com \
    --cc=dave.jiang@intel.com \
    --cc=dwmw2@infradead.org \
    --cc=fenghua.yu@intel.com \
    --cc=hch@infradead.org \
    --cc=hpa@zytor.com \
    --cc=iommu@lists.linux-foundation.org \
    --cc=jacob.jun.pan@intel.com \
    --cc=jean-philippe@linaro.org \
    --cc=joro@8bytes.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=ravi.v.shankar@intel.com \
    --cc=sohil.mehta@intel.com \
    --cc=tglx@linutronix.de \
    --cc=tony.luck@intel.com \
    --cc=x86@kernel.org \
    /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

AMD-GFX Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/amd-gfx/0 amd-gfx/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 amd-gfx amd-gfx/ https://lore.kernel.org/amd-gfx \
		amd-gfx@lists.freedesktop.org
	public-inbox-index amd-gfx

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.freedesktop.lists.amd-gfx


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git