linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] x86/apic: simplify disconnect_bsp_APIC setup of LVT{0/1}
@ 2020-01-27 17:57 Roger Pau Monne
  2020-02-19 12:36 ` Roger Pau Monné
  0 siblings, 1 reply; 3+ messages in thread
From: Roger Pau Monne @ 2020-01-27 17:57 UTC (permalink / raw)
  To: linux-kernel
  Cc: Roger Pau Monne, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	H. Peter Anvin, x86, Peter Zijlstra, Tony Luck, Jacob Pan,
	Kefeng Wang, Jan Beulich, Sean Christopherson

There's no need to read the current values of LVT{0/1} for the
purposes of the function, which seem to be to save the currently
selected vector: in the destination modes used (ExtINT and NMI) the
vector field is ignored and hence can be set to 0.

Note that clear_local_APIC as called by init_bsp_APIC would have
already wiped those registers by writing APIC_LVT_MASKED, and hence
there's nothing useful to preserve if that was the intent. Also note
that there are other places where LVT{0/1} is written to without doing
a read-modify-write (init_bsp_APIC and clear_local_APIC), so if
writing 0s to the reserved parts would cause issues they would be also
triggered by writes elsewhere.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Cc: Kefeng Wang <wangkefeng.wang@huawei.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Sean Christopherson <sean.j.christopherson@intel.com>
---
 arch/x86/kernel/apic/apic.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 28446fa6bf18..ce0c65340b4c 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -2292,12 +2292,7 @@ void disconnect_bsp_APIC(int virt_wire_setup)
 		 * For LVT0 make it edge triggered, active high,
 		 * external and enabled
 		 */
-		value = apic_read(APIC_LVT0);
-		value &= ~(APIC_MODE_MASK | APIC_SEND_PENDING |
-			APIC_INPUT_POLARITY | APIC_LVT_REMOTE_IRR |
-			APIC_LVT_LEVEL_TRIGGER | APIC_LVT_MASKED);
-		value |= APIC_LVT_REMOTE_IRR | APIC_SEND_PENDING;
-		value = SET_APIC_DELIVERY_MODE(value, APIC_MODE_EXTINT);
+		value = APIC_LVT_REMOTE_IRR | APIC_SEND_PENDING | APIC_DM_EXTINT;
 		apic_write(APIC_LVT0, value);
 	} else {
 		/* Disable LVT0 */
@@ -2308,12 +2303,7 @@ void disconnect_bsp_APIC(int virt_wire_setup)
 	 * For LVT1 make it edge triggered, active high,
 	 * nmi and enabled
 	 */
-	value = apic_read(APIC_LVT1);
-	value &= ~(APIC_MODE_MASK | APIC_SEND_PENDING |
-			APIC_INPUT_POLARITY | APIC_LVT_REMOTE_IRR |
-			APIC_LVT_LEVEL_TRIGGER | APIC_LVT_MASKED);
-	value |= APIC_LVT_REMOTE_IRR | APIC_SEND_PENDING;
-	value = SET_APIC_DELIVERY_MODE(value, APIC_MODE_NMI);
+	value = APIC_LVT_REMOTE_IRR | APIC_SEND_PENDING | APIC_DM_NMI;
 	apic_write(APIC_LVT1, value);
 }
 
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] x86/apic: simplify disconnect_bsp_APIC setup of LVT{0/1}
  2020-01-27 17:57 [PATCH] x86/apic: simplify disconnect_bsp_APIC setup of LVT{0/1} Roger Pau Monne
@ 2020-02-19 12:36 ` Roger Pau Monné
  2020-03-19 18:27   ` PING: " Roger Pau Monné
  0 siblings, 1 reply; 3+ messages in thread
From: Roger Pau Monné @ 2020-02-19 12:36 UTC (permalink / raw)
  To: linux-kernel
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin,
	x86, Peter Zijlstra, Tony Luck, Jacob Pan, Kefeng Wang,
	Jan Beulich, Sean Christopherson

Ping?

On Mon, Jan 27, 2020 at 06:57:58PM +0100, Roger Pau Monne wrote:
> There's no need to read the current values of LVT{0/1} for the
> purposes of the function, which seem to be to save the currently
> selected vector: in the destination modes used (ExtINT and NMI) the
> vector field is ignored and hence can be set to 0.
> 
> Note that clear_local_APIC as called by init_bsp_APIC would have
> already wiped those registers by writing APIC_LVT_MASKED, and hence
> there's nothing useful to preserve if that was the intent. Also note
> that there are other places where LVT{0/1} is written to without doing
> a read-modify-write (init_bsp_APIC and clear_local_APIC), so if
> writing 0s to the reserved parts would cause issues they would be also
> triggered by writes elsewhere.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: x86@kernel.org
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Tony Luck <tony.luck@intel.com>
> Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
> Cc: Kefeng Wang <wangkefeng.wang@huawei.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Sean Christopherson <sean.j.christopherson@intel.com>
> ---
>  arch/x86/kernel/apic/apic.c | 14 ++------------
>  1 file changed, 2 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> index 28446fa6bf18..ce0c65340b4c 100644
> --- a/arch/x86/kernel/apic/apic.c
> +++ b/arch/x86/kernel/apic/apic.c
> @@ -2292,12 +2292,7 @@ void disconnect_bsp_APIC(int virt_wire_setup)
>  		 * For LVT0 make it edge triggered, active high,
>  		 * external and enabled
>  		 */
> -		value = apic_read(APIC_LVT0);
> -		value &= ~(APIC_MODE_MASK | APIC_SEND_PENDING |
> -			APIC_INPUT_POLARITY | APIC_LVT_REMOTE_IRR |
> -			APIC_LVT_LEVEL_TRIGGER | APIC_LVT_MASKED);
> -		value |= APIC_LVT_REMOTE_IRR | APIC_SEND_PENDING;
> -		value = SET_APIC_DELIVERY_MODE(value, APIC_MODE_EXTINT);
> +		value = APIC_LVT_REMOTE_IRR | APIC_SEND_PENDING | APIC_DM_EXTINT;
>  		apic_write(APIC_LVT0, value);
>  	} else {
>  		/* Disable LVT0 */
> @@ -2308,12 +2303,7 @@ void disconnect_bsp_APIC(int virt_wire_setup)
>  	 * For LVT1 make it edge triggered, active high,
>  	 * nmi and enabled
>  	 */
> -	value = apic_read(APIC_LVT1);
> -	value &= ~(APIC_MODE_MASK | APIC_SEND_PENDING |
> -			APIC_INPUT_POLARITY | APIC_LVT_REMOTE_IRR |
> -			APIC_LVT_LEVEL_TRIGGER | APIC_LVT_MASKED);
> -	value |= APIC_LVT_REMOTE_IRR | APIC_SEND_PENDING;
> -	value = SET_APIC_DELIVERY_MODE(value, APIC_MODE_NMI);
> +	value = APIC_LVT_REMOTE_IRR | APIC_SEND_PENDING | APIC_DM_NMI;
>  	apic_write(APIC_LVT1, value);
>  }
>  
> -- 
> 2.25.0
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread
* PING: [PATCH] x86/apic: simplify disconnect_bsp_APIC setup of LVT{0/1}
  2020-02-19 12:36 ` Roger Pau Monné
@ 2020-03-19 18:27   ` Roger Pau Monné
  0 siblings, 0 replies; 3+ messages in thread
From: Roger Pau Monné @ 2020-03-19 18:27 UTC (permalink / raw)
  To: linux-kernel
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin,
	x86, Peter Zijlstra, Tony Luck, Jacob Pan, Kefeng Wang,
	Jan Beulich, Sean Christopherson

On Wed, Feb 19, 2020 at 01:36:54PM +0100, Roger Pau Monné wrote:
> Ping?

Ping x2.

Thanks.

> On Mon, Jan 27, 2020 at 06:57:58PM +0100, Roger Pau Monne wrote:
> > There's no need to read the current values of LVT{0/1} for the
> > purposes of the function, which seem to be to save the currently
> > selected vector: in the destination modes used (ExtINT and NMI) the
> > vector field is ignored and hence can be set to 0.
> > 
> > Note that clear_local_APIC as called by init_bsp_APIC would have
> > already wiped those registers by writing APIC_LVT_MASKED, and hence
> > there's nothing useful to preserve if that was the intent. Also note
> > that there are other places where LVT{0/1} is written to without doing
> > a read-modify-write (init_bsp_APIC and clear_local_APIC), so if
> > writing 0s to the reserved parts would cause issues they would be also
> > triggered by writes elsewhere.
> > 
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > ---
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Ingo Molnar <mingo@redhat.com>
> > Cc: Borislav Petkov <bp@alien8.de>
> > Cc: "H. Peter Anvin" <hpa@zytor.com>
> > Cc: x86@kernel.org
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Tony Luck <tony.luck@intel.com>
> > Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
> > Cc: Kefeng Wang <wangkefeng.wang@huawei.com>
> > Cc: Jan Beulich <jbeulich@suse.com>
> > Cc: Sean Christopherson <sean.j.christopherson@intel.com>
> > ---
> >  arch/x86/kernel/apic/apic.c | 14 ++------------
> >  1 file changed, 2 insertions(+), 12 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> > index 28446fa6bf18..ce0c65340b4c 100644
> > --- a/arch/x86/kernel/apic/apic.c
> > +++ b/arch/x86/kernel/apic/apic.c
> > @@ -2292,12 +2292,7 @@ void disconnect_bsp_APIC(int virt_wire_setup)
> >  		 * For LVT0 make it edge triggered, active high,
> >  		 * external and enabled
> >  		 */
> > -		value = apic_read(APIC_LVT0);
> > -		value &= ~(APIC_MODE_MASK | APIC_SEND_PENDING |
> > -			APIC_INPUT_POLARITY | APIC_LVT_REMOTE_IRR |
> > -			APIC_LVT_LEVEL_TRIGGER | APIC_LVT_MASKED);
> > -		value |= APIC_LVT_REMOTE_IRR | APIC_SEND_PENDING;
> > -		value = SET_APIC_DELIVERY_MODE(value, APIC_MODE_EXTINT);
> > +		value = APIC_LVT_REMOTE_IRR | APIC_SEND_PENDING | APIC_DM_EXTINT;
> >  		apic_write(APIC_LVT0, value);
> >  	} else {
> >  		/* Disable LVT0 */
> > @@ -2308,12 +2303,7 @@ void disconnect_bsp_APIC(int virt_wire_setup)
> >  	 * For LVT1 make it edge triggered, active high,
> >  	 * nmi and enabled
> >  	 */
> > -	value = apic_read(APIC_LVT1);
> > -	value &= ~(APIC_MODE_MASK | APIC_SEND_PENDING |
> > -			APIC_INPUT_POLARITY | APIC_LVT_REMOTE_IRR |
> > -			APIC_LVT_LEVEL_TRIGGER | APIC_LVT_MASKED);
> > -	value |= APIC_LVT_REMOTE_IRR | APIC_SEND_PENDING;
> > -	value = SET_APIC_DELIVERY_MODE(value, APIC_MODE_NMI);
> > +	value = APIC_LVT_REMOTE_IRR | APIC_SEND_PENDING | APIC_DM_NMI;
> >  	apic_write(APIC_LVT1, value);
> >  }
> >  
> > -- 
> > 2.25.0
> > 

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-03-19 18:27 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-27 17:57 [PATCH] x86/apic: simplify disconnect_bsp_APIC setup of LVT{0/1} Roger Pau Monne
2020-02-19 12:36 ` Roger Pau Monné
2020-03-19 18:27   ` PING: " Roger Pau Monné

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).