diff mbox

[v2,16/17] x86/insn: remove pcommit

Message ID 146812115425.32932.3314855641604340233.stgit@dwillia2-desk3.amr.corp.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Dan Williams July 10, 2016, 3:25 a.m. UTC
The pcommit instruction is being deprecated in favor of either ADR
(asynchronous DRAM refresh: flush-on-power-fail) at the platform level, or
posted-write-queue flush addresses as defined by the ACPI 6.x NFIT (NVDIMM
Firmware Interface Table).

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 arch/x86/include/asm/cpufeatures.h                 |    1 
 arch/x86/include/asm/special_insns.h               |   46 --------------------
 arch/x86/lib/x86-opcode-map.txt                    |    2 -
 tools/objtool/arch/x86/insn/x86-opcode-map.txt     |    2 -
 tools/perf/arch/x86/tests/insn-x86-dat-32.c        |    2 -
 tools/perf/arch/x86/tests/insn-x86-dat-64.c        |    2 -
 tools/perf/arch/x86/tests/insn-x86-dat-src.c       |    4 --
 .../perf/util/intel-pt-decoder/x86-opcode-map.txt  |    2 -
 8 files changed, 3 insertions(+), 58 deletions(-)

Comments

Peter Zijlstra July 12, 2016, 2:57 p.m. UTC | #1
On Sat, Jul 09, 2016 at 08:25:54PM -0700, Dan Williams wrote:
> The pcommit instruction is being deprecated in favor of either ADR
> (asynchronous DRAM refresh: flush-on-power-fail) at the platform level, or
> posted-write-queue flush addresses as defined by the ACPI 6.x NFIT (NVDIMM
> Firmware Interface Table).

>  arch/x86/include/asm/cpufeatures.h                 |    1 
>  arch/x86/include/asm/special_insns.h               |   46 --------------------
>  arch/x86/lib/x86-opcode-map.txt                    |    2 -
>  tools/objtool/arch/x86/insn/x86-opcode-map.txt     |    2 -
>  tools/perf/arch/x86/tests/insn-x86-dat-32.c        |    2 -
>  tools/perf/arch/x86/tests/insn-x86-dat-64.c        |    2 -
>  tools/perf/arch/x86/tests/insn-x86-dat-src.c       |    4 --

Just deprecated, or is it completely eradicated, removed from history,
will never ever happen and we'll reissue the opcode for something else?

Because if its only deprecated then removing it from the instruction
decoders seems wrong, old binaries might still contain the opcode.
Dan Williams July 12, 2016, 10:12 p.m. UTC | #2
On Tue, Jul 12, 2016 at 7:57 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Sat, Jul 09, 2016 at 08:25:54PM -0700, Dan Williams wrote:
>> The pcommit instruction is being deprecated in favor of either ADR
>> (asynchronous DRAM refresh: flush-on-power-fail) at the platform level, or
>> posted-write-queue flush addresses as defined by the ACPI 6.x NFIT (NVDIMM
>> Firmware Interface Table).
>
>>  arch/x86/include/asm/cpufeatures.h                 |    1
>>  arch/x86/include/asm/special_insns.h               |   46 --------------------
>>  arch/x86/lib/x86-opcode-map.txt                    |    2 -
>>  tools/objtool/arch/x86/insn/x86-opcode-map.txt     |    2 -
>>  tools/perf/arch/x86/tests/insn-x86-dat-32.c        |    2 -
>>  tools/perf/arch/x86/tests/insn-x86-dat-64.c        |    2 -
>>  tools/perf/arch/x86/tests/insn-x86-dat-src.c       |    4 --
>
> Just deprecated, or is it completely eradicated, removed from history,
> will never ever happen and we'll reissue the opcode for something else?
>
> Because if its only deprecated then removing it from the instruction
> decoders seems wrong, old binaries might still contain the opcode.

Eradicated.

"The new instructions like CLWB and CLFLUSHOPT will be rolled into the
SDM but PCOMMIT will be removed from the Extensions doc and not rolled
into the SDM." [1]

Existing binaries are already gating their usage on the presence of
the cpu id flag, that flag and the instruction opcode are reserved
going forward.

[1]: https://lists.01.org/pipermail/linux-nvdimm/2016-June/005923.html
Dan Williams July 22, 2016, 3:55 p.m. UTC | #3
On Tue, Jul 12, 2016 at 3:12 PM, Dan Williams <dan.j.williams@intel.com> wrote:
> On Tue, Jul 12, 2016 at 7:57 AM, Peter Zijlstra <peterz@infradead.org> wrote:
>> On Sat, Jul 09, 2016 at 08:25:54PM -0700, Dan Williams wrote:
>>> The pcommit instruction is being deprecated in favor of either ADR
>>> (asynchronous DRAM refresh: flush-on-power-fail) at the platform level, or
>>> posted-write-queue flush addresses as defined by the ACPI 6.x NFIT (NVDIMM
>>> Firmware Interface Table).
>>
>>>  arch/x86/include/asm/cpufeatures.h                 |    1
>>>  arch/x86/include/asm/special_insns.h               |   46 --------------------
>>>  arch/x86/lib/x86-opcode-map.txt                    |    2 -
>>>  tools/objtool/arch/x86/insn/x86-opcode-map.txt     |    2 -
>>>  tools/perf/arch/x86/tests/insn-x86-dat-32.c        |    2 -
>>>  tools/perf/arch/x86/tests/insn-x86-dat-64.c        |    2 -
>>>  tools/perf/arch/x86/tests/insn-x86-dat-src.c       |    4 --
>>
>> Just deprecated, or is it completely eradicated, removed from history,
>> will never ever happen and we'll reissue the opcode for something else?
>>
>> Because if its only deprecated then removing it from the instruction
>> decoders seems wrong, old binaries might still contain the opcode.
>
> Eradicated.
>
> "The new instructions like CLWB and CLFLUSHOPT will be rolled into the
> SDM but PCOMMIT will be removed from the Extensions doc and not rolled
> into the SDM." [1]
>
> Existing binaries are already gating their usage on the presence of
> the cpu id flag, that flag and the instruction opcode are reserved
> going forward.
>
> [1]: https://lists.01.org/pipermail/linux-nvdimm/2016-June/005923.html

x86 maintainers, I have the other patches in this series queued in
-next. Please ack this one and I'll add it for v4.8-rc1, or otherwise
let me know how you want to handle this patch.
Ingo Molnar July 22, 2016, 4:52 p.m. UTC | #4
* Dan Williams <dan.j.williams@intel.com> wrote:

> On Tue, Jul 12, 2016 at 3:12 PM, Dan Williams <dan.j.williams@intel.com> wrote:
> > On Tue, Jul 12, 2016 at 7:57 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> >> On Sat, Jul 09, 2016 at 08:25:54PM -0700, Dan Williams wrote:
> >>> The pcommit instruction is being deprecated in favor of either ADR
> >>> (asynchronous DRAM refresh: flush-on-power-fail) at the platform level, or
> >>> posted-write-queue flush addresses as defined by the ACPI 6.x NFIT (NVDIMM
> >>> Firmware Interface Table).
> >>
> >>>  arch/x86/include/asm/cpufeatures.h                 |    1
> >>>  arch/x86/include/asm/special_insns.h               |   46 --------------------
> >>>  arch/x86/lib/x86-opcode-map.txt                    |    2 -
> >>>  tools/objtool/arch/x86/insn/x86-opcode-map.txt     |    2 -
> >>>  tools/perf/arch/x86/tests/insn-x86-dat-32.c        |    2 -
> >>>  tools/perf/arch/x86/tests/insn-x86-dat-64.c        |    2 -
> >>>  tools/perf/arch/x86/tests/insn-x86-dat-src.c       |    4 --
> >>
> >> Just deprecated, or is it completely eradicated, removed from history,
> >> will never ever happen and we'll reissue the opcode for something else?
> >>
> >> Because if its only deprecated then removing it from the instruction
> >> decoders seems wrong, old binaries might still contain the opcode.
> >
> > Eradicated.
> >
> > "The new instructions like CLWB and CLFLUSHOPT will be rolled into the
> > SDM but PCOMMIT will be removed from the Extensions doc and not rolled
> > into the SDM." [1]
> >
> > Existing binaries are already gating their usage on the presence of
> > the cpu id flag, that flag and the instruction opcode are reserved
> > going forward.
> >
> > [1]: https://lists.01.org/pipermail/linux-nvdimm/2016-June/005923.html
> 
> x86 maintainers, I have the other patches in this series queued in -next. Please 
> ack this one and I'll add it for v4.8-rc1, or otherwise let me know how you want 
> to handle this patch.

Since it's just a removal AFAICS that the rest of your series should not depend 
on, can you submit it to the x86 tree?

Thanks,

	Ingo
Dan Williams July 23, 2016, 12:54 a.m. UTC | #5
On Fri, Jul 22, 2016 at 9:52 AM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Dan Williams <dan.j.williams@intel.com> wrote:
>
>> On Tue, Jul 12, 2016 at 3:12 PM, Dan Williams <dan.j.williams@intel.com> wrote:
>> > On Tue, Jul 12, 2016 at 7:57 AM, Peter Zijlstra <peterz@infradead.org> wrote:
>> >> On Sat, Jul 09, 2016 at 08:25:54PM -0700, Dan Williams wrote:
>> >>> The pcommit instruction is being deprecated in favor of either ADR
>> >>> (asynchronous DRAM refresh: flush-on-power-fail) at the platform level, or
>> >>> posted-write-queue flush addresses as defined by the ACPI 6.x NFIT (NVDIMM
>> >>> Firmware Interface Table).
>> >>
>> >>>  arch/x86/include/asm/cpufeatures.h                 |    1
>> >>>  arch/x86/include/asm/special_insns.h               |   46 --------------------
>> >>>  arch/x86/lib/x86-opcode-map.txt                    |    2 -
>> >>>  tools/objtool/arch/x86/insn/x86-opcode-map.txt     |    2 -
>> >>>  tools/perf/arch/x86/tests/insn-x86-dat-32.c        |    2 -
>> >>>  tools/perf/arch/x86/tests/insn-x86-dat-64.c        |    2 -
>> >>>  tools/perf/arch/x86/tests/insn-x86-dat-src.c       |    4 --
>> >>
>> >> Just deprecated, or is it completely eradicated, removed from history,
>> >> will never ever happen and we'll reissue the opcode for something else?
>> >>
>> >> Because if its only deprecated then removing it from the instruction
>> >> decoders seems wrong, old binaries might still contain the opcode.
>> >
>> > Eradicated.
>> >
>> > "The new instructions like CLWB and CLFLUSHOPT will be rolled into the
>> > SDM but PCOMMIT will be removed from the Extensions doc and not rolled
>> > into the SDM." [1]
>> >
>> > Existing binaries are already gating their usage on the presence of
>> > the cpu id flag, that flag and the instruction opcode are reserved
>> > going forward.
>> >
>> > [1]: https://lists.01.org/pipermail/linux-nvdimm/2016-June/005923.html
>>
>> x86 maintainers, I have the other patches in this series queued in -next. Please
>> ack this one and I'll add it for v4.8-rc1, or otherwise let me know how you want
>> to handle this patch.
>
> Since it's just a removal AFAICS that the rest of your series should not depend
> on, can you submit it to the x86 tree?

This patch depends on the previous patches in the series removing
calls to pcommit_sfence().
Ingo Molnar July 23, 2016, 7:49 a.m. UTC | #6
* Dan Williams <dan.j.williams@intel.com> wrote:

> On Fri, Jul 22, 2016 at 9:52 AM, Ingo Molnar <mingo@kernel.org> wrote:
> >
> > * Dan Williams <dan.j.williams@intel.com> wrote:
> >
> >> On Tue, Jul 12, 2016 at 3:12 PM, Dan Williams <dan.j.williams@intel.com> wrote:
> >> > On Tue, Jul 12, 2016 at 7:57 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> >> >> On Sat, Jul 09, 2016 at 08:25:54PM -0700, Dan Williams wrote:
> >> >>> The pcommit instruction is being deprecated in favor of either ADR
> >> >>> (asynchronous DRAM refresh: flush-on-power-fail) at the platform level, or
> >> >>> posted-write-queue flush addresses as defined by the ACPI 6.x NFIT (NVDIMM
> >> >>> Firmware Interface Table).
> >> >>
> >> >>>  arch/x86/include/asm/cpufeatures.h                 |    1
> >> >>>  arch/x86/include/asm/special_insns.h               |   46 --------------------
> >> >>>  arch/x86/lib/x86-opcode-map.txt                    |    2 -
> >> >>>  tools/objtool/arch/x86/insn/x86-opcode-map.txt     |    2 -
> >> >>>  tools/perf/arch/x86/tests/insn-x86-dat-32.c        |    2 -
> >> >>>  tools/perf/arch/x86/tests/insn-x86-dat-64.c        |    2 -
> >> >>>  tools/perf/arch/x86/tests/insn-x86-dat-src.c       |    4 --
> >> >>
> >> >> Just deprecated, or is it completely eradicated, removed from history,
> >> >> will never ever happen and we'll reissue the opcode for something else?
> >> >>
> >> >> Because if its only deprecated then removing it from the instruction
> >> >> decoders seems wrong, old binaries might still contain the opcode.
> >> >
> >> > Eradicated.
> >> >
> >> > "The new instructions like CLWB and CLFLUSHOPT will be rolled into the
> >> > SDM but PCOMMIT will be removed from the Extensions doc and not rolled
> >> > into the SDM." [1]
> >> >
> >> > Existing binaries are already gating their usage on the presence of
> >> > the cpu id flag, that flag and the instruction opcode are reserved
> >> > going forward.
> >> >
> >> > [1]: https://lists.01.org/pipermail/linux-nvdimm/2016-June/005923.html
> >>
> >> x86 maintainers, I have the other patches in this series queued in -next. Please
> >> ack this one and I'll add it for v4.8-rc1, or otherwise let me know how you want
> >> to handle this patch.
> >
> > Since it's just a removal AFAICS that the rest of your series should not depend
> > on, can you submit it to the x86 tree?
> 
> This patch depends on the previous patches in the series removing
> calls to pcommit_sfence().

Ok, and the patch looks harmless:

Acked-by: Ingo Molnar <mingo@kernel.org>

Thanks,

	Ingo
diff mbox

Patch

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 4a413485f9eb..700d97df7d28 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -225,7 +225,6 @@ 
 #define X86_FEATURE_RDSEED	( 9*32+18) /* The RDSEED instruction */
 #define X86_FEATURE_ADX		( 9*32+19) /* The ADCX and ADOX instructions */
 #define X86_FEATURE_SMAP	( 9*32+20) /* Supervisor Mode Access Prevention */
-#define X86_FEATURE_PCOMMIT	( 9*32+22) /* PCOMMIT instruction */
 #define X86_FEATURE_CLFLUSHOPT	( 9*32+23) /* CLFLUSHOPT instruction */
 #define X86_FEATURE_CLWB	( 9*32+24) /* CLWB instruction */
 #define X86_FEATURE_AVX512PF	( 9*32+26) /* AVX-512 Prefetch */
diff --git a/arch/x86/include/asm/special_insns.h b/arch/x86/include/asm/special_insns.h
index d96d04377765..587d7914ea4b 100644
--- a/arch/x86/include/asm/special_insns.h
+++ b/arch/x86/include/asm/special_insns.h
@@ -253,52 +253,6 @@  static inline void clwb(volatile void *__p)
 		: [pax] "a" (p));
 }
 
-/**
- * pcommit_sfence() - persistent commit and fence
- *
- * The PCOMMIT instruction ensures that data that has been flushed from the
- * processor's cache hierarchy with CLWB, CLFLUSHOPT or CLFLUSH is accepted to
- * memory and is durable on the DIMM.  The primary use case for this is
- * persistent memory.
- *
- * This function shows how to properly use CLWB/CLFLUSHOPT/CLFLUSH and PCOMMIT
- * with appropriate fencing.
- *
- * Example:
- * void flush_and_commit_buffer(void *vaddr, unsigned int size)
- * {
- *         unsigned long clflush_mask = boot_cpu_data.x86_clflush_size - 1;
- *         void *vend = vaddr + size;
- *         void *p;
- *
- *         for (p = (void *)((unsigned long)vaddr & ~clflush_mask);
- *              p < vend; p += boot_cpu_data.x86_clflush_size)
- *                 clwb(p);
- *
- *         // SFENCE to order CLWB/CLFLUSHOPT/CLFLUSH cache flushes
- *         // MFENCE via mb() also works
- *         wmb();
- *
- *         // PCOMMIT and the required SFENCE for ordering
- *         pcommit_sfence();
- * }
- *
- * After this function completes the data pointed to by 'vaddr' has been
- * accepted to memory and will be durable if the 'vaddr' points to persistent
- * memory.
- *
- * PCOMMIT must always be ordered by an MFENCE or SFENCE, so to help simplify
- * things we include both the PCOMMIT and the required SFENCE in the
- * alternatives generated by pcommit_sfence().
- */
-static inline void pcommit_sfence(void)
-{
-	alternative(ASM_NOP7,
-		    ".byte 0x66, 0x0f, 0xae, 0xf8\n\t" /* pcommit */
-		    "sfence",
-		    X86_FEATURE_PCOMMIT);
-}
-
 #define nop() asm volatile ("nop")
 
 
diff --git a/arch/x86/lib/x86-opcode-map.txt b/arch/x86/lib/x86-opcode-map.txt
index d388de72eaca..28632ee68377 100644
--- a/arch/x86/lib/x86-opcode-map.txt
+++ b/arch/x86/lib/x86-opcode-map.txt
@@ -947,7 +947,7 @@  GrpTable: Grp15
 4: XSAVE
 5: XRSTOR | lfence (11B)
 6: XSAVEOPT | clwb (66) | mfence (11B)
-7: clflush | clflushopt (66) | sfence (11B) | pcommit (66),(11B)
+7: clflush | clflushopt (66) | sfence (11B)
 EndTable
 
 GrpTable: Grp16
diff --git a/tools/objtool/arch/x86/insn/x86-opcode-map.txt b/tools/objtool/arch/x86/insn/x86-opcode-map.txt
index d388de72eaca..28632ee68377 100644
--- a/tools/objtool/arch/x86/insn/x86-opcode-map.txt
+++ b/tools/objtool/arch/x86/insn/x86-opcode-map.txt
@@ -947,7 +947,7 @@  GrpTable: Grp15
 4: XSAVE
 5: XRSTOR | lfence (11B)
 6: XSAVEOPT | clwb (66) | mfence (11B)
-7: clflush | clflushopt (66) | sfence (11B) | pcommit (66),(11B)
+7: clflush | clflushopt (66) | sfence (11B)
 EndTable
 
 GrpTable: Grp16
diff --git a/tools/perf/arch/x86/tests/insn-x86-dat-32.c b/tools/perf/arch/x86/tests/insn-x86-dat-32.c
index 3b491cfe204e..38a48daed154 100644
--- a/tools/perf/arch/x86/tests/insn-x86-dat-32.c
+++ b/tools/perf/arch/x86/tests/insn-x86-dat-32.c
@@ -654,5 +654,3 @@ 
 "0f c7 1d 78 56 34 12 \txrstors 0x12345678",},
 {{0x0f, 0xc7, 0x9c, 0xc8, 0x78, 0x56, 0x34, 0x12, }, 8, 0, "", "",
 "0f c7 9c c8 78 56 34 12 \txrstors 0x12345678(%eax,%ecx,8)",},
-{{0x66, 0x0f, 0xae, 0xf8, }, 4, 0, "", "",
-"66 0f ae f8          \tpcommit ",},
diff --git a/tools/perf/arch/x86/tests/insn-x86-dat-64.c b/tools/perf/arch/x86/tests/insn-x86-dat-64.c
index 4fe7cce179c4..1f11ea85b60f 100644
--- a/tools/perf/arch/x86/tests/insn-x86-dat-64.c
+++ b/tools/perf/arch/x86/tests/insn-x86-dat-64.c
@@ -764,5 +764,3 @@ 
 "0f c7 9c c8 78 56 34 12 \txrstors 0x12345678(%rax,%rcx,8)",},
 {{0x41, 0x0f, 0xc7, 0x9c, 0xc8, 0x78, 0x56, 0x34, 0x12, }, 9, 0, "", "",
 "41 0f c7 9c c8 78 56 34 12 \txrstors 0x12345678(%r8,%rcx,8)",},
-{{0x66, 0x0f, 0xae, 0xf8, }, 4, 0, "", "",
-"66 0f ae f8          \tpcommit ",},
diff --git a/tools/perf/arch/x86/tests/insn-x86-dat-src.c b/tools/perf/arch/x86/tests/insn-x86-dat-src.c
index 41b1b1c62660..033b8a6fdab9 100644
--- a/tools/perf/arch/x86/tests/insn-x86-dat-src.c
+++ b/tools/perf/arch/x86/tests/insn-x86-dat-src.c
@@ -866,10 +866,6 @@  int main(void)
 
 #endif /* #ifndef __x86_64__ */
 
-	/* pcommit */
-
-	asm volatile("pcommit");
-
 	/* Following line is a marker for the awk script - do not change */
 	asm volatile("rdtsc"); /* Stop here */
 
diff --git a/tools/perf/util/intel-pt-decoder/x86-opcode-map.txt b/tools/perf/util/intel-pt-decoder/x86-opcode-map.txt
index d388de72eaca..28632ee68377 100644
--- a/tools/perf/util/intel-pt-decoder/x86-opcode-map.txt
+++ b/tools/perf/util/intel-pt-decoder/x86-opcode-map.txt
@@ -947,7 +947,7 @@  GrpTable: Grp15
 4: XSAVE
 5: XRSTOR | lfence (11B)
 6: XSAVEOPT | clwb (66) | mfence (11B)
-7: clflush | clflushopt (66) | sfence (11B) | pcommit (66),(11B)
+7: clflush | clflushopt (66) | sfence (11B)
 EndTable
 
 GrpTable: Grp16