Re: [kvm-unit-tests PATCH v3 01/27] x86: replace irq_{enable|disable}() with sti()/cli()

[Maxim Levitsky <mlevitsk@redhat.com>]

On Thu, 2022-12-01 at 14:46 +0100, Emanuele Giuseppe Esposito wrote:
> 
> Am 22/11/2022 um 17:11 schrieb Maxim Levitsky:
> > This removes a layer of indirection which is strictly
> > speaking not needed since its x86 code anyway.
> > 
> > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> > ---
> >  lib/x86/processor.h       | 19 +++++-----------
> >  lib/x86/smp.c             |  2 +-
> >  x86/apic.c                |  2 +-
> >  x86/asyncpf.c             |  6 ++---
> >  x86/eventinj.c            | 22 +++++++++---------
> >  x86/hyperv_connections.c  |  2 +-
> >  x86/hyperv_stimer.c       |  4 ++--
> >  x86/hyperv_synic.c        |  6 ++---
> >  x86/intel-iommu.c         |  2 +-
> >  x86/ioapic.c              | 14 ++++++------
> >  x86/pmu.c                 |  4 ++--
> >  x86/svm.c                 |  4 ++--
> >  x86/svm_tests.c           | 48 +++++++++++++++++++--------------------
> >  x86/taskswitch2.c         |  4 ++--
> >  x86/tscdeadline_latency.c |  4 ++--
> >  x86/vmexit.c              | 18 +++++++--------
> >  x86/vmx_tests.c           | 42 +++++++++++++++++-----------------
> >  17 files changed, 98 insertions(+), 105 deletions(-)
> > 
> > diff --git a/lib/x86/processor.h b/lib/x86/processor.h
> > index 7a9e8c82..b89f6a7c 100644
> > --- a/lib/x86/processor.h
> > +++ b/lib/x86/processor.h
> > @@ -653,11 +653,17 @@ static inline void pause(void)
> >  	asm volatile ("pause");
> >  }
> >  
> > +/* Disable interrupts as per x86 spec */
> >  static inline void cli(void)
> >  {
> >  	asm volatile ("cli");
> >  }
> >  
> > +/*
> > + * Enable interrupts.
> > + * Note that next instruction after sti will not have interrupts
> > + * evaluated due to concept of 'interrupt shadow'
> > + */
> >  static inline void sti(void)
> >  {
> >  	asm volatile ("sti");
> > @@ -732,19 +738,6 @@ static inline void wrtsc(u64 tsc)
> >  	wrmsr(MSR_IA32_TSC, tsc);
> >  }
> >  
> > -static inline void irq_disable(void)
> > -{
> > -	asm volatile("cli");
> > -}
> > -
> > -/* Note that irq_enable() does not ensure an interrupt shadow due
> > - * to the vagaries of compiler optimizations.  If you need the
> > - * shadow, use a single asm with "sti" and the instruction after it.
> Minor nitpick: instead of a new doc comment, why not use this same
> above? Looks clearer to me.
> 
> Regardless,
> Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> 

I am not 100% sure what you mean.
Note that cli() doesn't have the same interrupt window thing as sti().

Best regards,
	Maxim Levitsky