* x2apic_wrmsr_fence vs. Intel manual
@ 2020-03-02 16:11 Jan Kiszka
2020-03-02 16:20 ` Thomas Gleixner
2020-03-04 18:27 ` Dave Hansen
0 siblings, 2 replies; 6+ messages in thread
From: Jan Kiszka @ 2020-03-02 16:11 UTC (permalink / raw)
To: x86; +Cc: Linux Kernel Mailing List
Hi all,
as I generated a nice bug around fence vs. x2apic icr writes, I studied
the kernel code and the Intel manual in this regard more closely. But
there is a discrepancy:
arch/x86/include/asm/apic.h:
/*
* Make previous memory operations globally visible before
* sending the IPI through x2apic wrmsr. We need a serializing instruction or
* mfence for this.
*/
static inline void x2apic_wrmsr_fence(void)
{
asm volatile("mfence" : : : "memory");
}
Intel SDM, 10.12.3 MSR Access in x2APIC Mode:
"A WRMSR to an APIC register may complete before all preceding stores
are globally visible; software can prevent this by inserting a
serializing instruction or the sequence MFENCE;LFENCE before the WRMSR."
The former dates back to ce4e240c279a, but that commit does not mention
why lfence is not needed. Did the manual read differently back then? Or
why are we safe? To my reading of lfence, it also has a certain
instruction serializing effect that mfence does not have.
Jan
--
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: x2apic_wrmsr_fence vs. Intel manual
2020-03-02 16:11 x2apic_wrmsr_fence vs. Intel manual Jan Kiszka
@ 2020-03-02 16:20 ` Thomas Gleixner
2020-03-02 16:33 ` Jan Kiszka
2020-03-02 16:35 ` Peter Zijlstra
2020-03-04 18:27 ` Dave Hansen
1 sibling, 2 replies; 6+ messages in thread
From: Thomas Gleixner @ 2020-03-02 16:20 UTC (permalink / raw)
To: Jan Kiszka, x86; +Cc: Linux Kernel Mailing List
Jan Kiszka <jan.kiszka@siemens.com> writes:
> as I generated a nice bug around fence vs. x2apic icr writes, I studied
> the kernel code and the Intel manual in this regard more closely. But
> there is a discrepancy:
>
> arch/x86/include/asm/apic.h:
>
> /*
> * Make previous memory operations globally visible before
> * sending the IPI through x2apic wrmsr. We need a serializing instruction or
> * mfence for this.
> */
> static inline void x2apic_wrmsr_fence(void)
> {
> asm volatile("mfence" : : : "memory");
> }
>
> Intel SDM, 10.12.3 MSR Access in x2APIC Mode:
>
> "A WRMSR to an APIC register may complete before all preceding stores
> are globally visible; software can prevent this by inserting a
> serializing instruction or the sequence MFENCE;LFENCE before the WRMSR."
>
> The former dates back to ce4e240c279a, but that commit does not mention
> why lfence is not needed. Did the manual read differently back then? Or
> why are we safe? To my reading of lfence, it also has a certain
> instruction serializing effect that mfence does not have.
The 2011 SDM says:
A WRMSR to an APIC register may complete before all preceding stores
are globally visible; software can prevent this by inserting a
serializing instruction, an SFENCE, or an MFENCE before the WRMSR.
Sigh....
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: x2apic_wrmsr_fence vs. Intel manual
2020-03-02 16:20 ` Thomas Gleixner
@ 2020-03-02 16:33 ` Jan Kiszka
2020-03-02 16:35 ` Peter Zijlstra
1 sibling, 0 replies; 6+ messages in thread
From: Jan Kiszka @ 2020-03-02 16:33 UTC (permalink / raw)
To: Thomas Gleixner, x86; +Cc: Linux Kernel Mailing List
On 02.03.20 17:20, Thomas Gleixner wrote:
> Jan Kiszka <jan.kiszka@siemens.com> writes:
>> as I generated a nice bug around fence vs. x2apic icr writes, I studied
>> the kernel code and the Intel manual in this regard more closely. But
>> there is a discrepancy:
>>
>> arch/x86/include/asm/apic.h:
>>
>> /*
>> * Make previous memory operations globally visible before
>> * sending the IPI through x2apic wrmsr. We need a serializing instruction or
>> * mfence for this.
>> */
>> static inline void x2apic_wrmsr_fence(void)
>> {
>> asm volatile("mfence" : : : "memory");
>> }
>>
>> Intel SDM, 10.12.3 MSR Access in x2APIC Mode:
>>
>> "A WRMSR to an APIC register may complete before all preceding stores
>> are globally visible; software can prevent this by inserting a
>> serializing instruction or the sequence MFENCE;LFENCE before the WRMSR."
>>
>> The former dates back to ce4e240c279a, but that commit does not mention
>> why lfence is not needed. Did the manual read differently back then? Or
>> why are we safe? To my reading of lfence, it also has a certain
>> instruction serializing effect that mfence does not have.
>
> The 2011 SDM says:
>
> A WRMSR to an APIC register may complete before all preceding stores
> are globally visible; software can prevent this by inserting a
> serializing instruction, an SFENCE, or an MFENCE before the WRMSR.
>
> Sigh....
>
OK, that explains it. Curious since when (date and CPU generation) we
unknowingly started to play roulette here.
Jan
--
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: x2apic_wrmsr_fence vs. Intel manual
2020-03-02 16:20 ` Thomas Gleixner
2020-03-02 16:33 ` Jan Kiszka
@ 2020-03-02 16:35 ` Peter Zijlstra
1 sibling, 0 replies; 6+ messages in thread
From: Peter Zijlstra @ 2020-03-02 16:35 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Jan Kiszka, x86, Linux Kernel Mailing List, H. Peter Anvin
On Mon, Mar 02, 2020 at 05:20:26PM +0100, Thomas Gleixner wrote:
> Jan Kiszka <jan.kiszka@siemens.com> writes:
> > as I generated a nice bug around fence vs. x2apic icr writes, I studied
> > the kernel code and the Intel manual in this regard more closely. But
> > there is a discrepancy:
> >
> > arch/x86/include/asm/apic.h:
> >
> > /*
> > * Make previous memory operations globally visible before
> > * sending the IPI through x2apic wrmsr. We need a serializing instruction or
> > * mfence for this.
> > */
> > static inline void x2apic_wrmsr_fence(void)
> > {
> > asm volatile("mfence" : : : "memory");
> > }
> >
> > Intel SDM, 10.12.3 MSR Access in x2APIC Mode:
> >
> > "A WRMSR to an APIC register may complete before all preceding stores
> > are globally visible; software can prevent this by inserting a
> > serializing instruction or the sequence MFENCE;LFENCE before the WRMSR."
> >
> > The former dates back to ce4e240c279a, but that commit does not mention
> > why lfence is not needed. Did the manual read differently back then? Or
> > why are we safe? To my reading of lfence, it also has a certain
> > instruction serializing effect that mfence does not have.
>
> The 2011 SDM says:
>
> A WRMSR to an APIC register may complete before all preceding stores
> are globally visible; software can prevent this by inserting a
> serializing instruction, an SFENCE, or an MFENCE before the WRMSR.
>
> Sigh....
*groan*, The only way I can explain this is...
... because we changed the definition of LFENCE from:
- wait until completion of all prior LOADs
to
- wait until completion of all prior instructions
Because Spectre (and because apparently it was implemented that way,
mostly). It could be that MFENCE, which is basically a completion
barrier for all prior LOADs *AND* STOREs, is no longer a stict superset
of LFENCE anymore...
Which makes the otherwise perverted sequence: MFENCE;LFENCE, actually
mean something :/
la-la-la
Would be good to have that clarified though.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: x2apic_wrmsr_fence vs. Intel manual
2020-03-02 16:11 x2apic_wrmsr_fence vs. Intel manual Jan Kiszka
2020-03-02 16:20 ` Thomas Gleixner
@ 2020-03-04 18:27 ` Dave Hansen
2020-03-04 18:39 ` Jan Kiszka
1 sibling, 1 reply; 6+ messages in thread
From: Dave Hansen @ 2020-03-04 18:27 UTC (permalink / raw)
To: Jan Kiszka, x86; +Cc: Linux Kernel Mailing List
On 3/2/20 8:11 AM, Jan Kiszka wrote:
> The former dates back to ce4e240c279a, but that commit does not mention
> why lfence is not needed. Did the manual read differently back then? Or
> why are we safe? To my reading of lfence, it also has a certain
> instruction serializing effect that mfence does not have.
I asked around Intel about this.
The old "SFENCE, or MFENCE" recommendation was deemed insufficient
because it has no impact on the ordering of WRMSR since it is not a
"load or store instruction". LFENCE's instruction-ordering semantic is
needed because it ensures later ordering of all instructions, not just
loads and stores.
Jan, do you think you're seeing a bug resulting from WRMSR ordering?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: x2apic_wrmsr_fence vs. Intel manual
2020-03-04 18:27 ` Dave Hansen
@ 2020-03-04 18:39 ` Jan Kiszka
0 siblings, 0 replies; 6+ messages in thread
From: Jan Kiszka @ 2020-03-04 18:39 UTC (permalink / raw)
To: Dave Hansen, x86; +Cc: Linux Kernel Mailing List
On 04.03.20 19:27, Dave Hansen wrote:
> On 3/2/20 8:11 AM, Jan Kiszka wrote:
>> The former dates back to ce4e240c279a, but that commit does not mention
>> why lfence is not needed. Did the manual read differently back then? Or
>> why are we safe? To my reading of lfence, it also has a certain
>> instruction serializing effect that mfence does not have.
>
> I asked around Intel about this.
>
> The old "SFENCE, or MFENCE" recommendation was deemed insufficient
> because it has no impact on the ordering of WRMSR since it is not a
> "load or store instruction". LFENCE's instruction-ordering semantic is
> needed because it ensures later ordering of all instructions, not just
> loads and stores.
>
> Jan, do you think you're seeing a bug resulting from WRMSR ordering?
>
Nope, not so far. I'm hunting a race between two guests over Jailhouse
where the kick (sent as IPI) seems to come before the data, but changing
the fences didn't solve it, unfortunately. Along that, I was reading
code and manuals up and down and ran into this inconsistency. That's the
story.
Jan
--
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-03-04 18:40 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-03-02 16:11 x2apic_wrmsr_fence vs. Intel manual Jan Kiszka
2020-03-02 16:20 ` Thomas Gleixner
2020-03-02 16:33 ` Jan Kiszka
2020-03-02 16:35 ` Peter Zijlstra
2020-03-04 18:27 ` Dave Hansen
2020-03-04 18:39 ` Jan Kiszka
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox