Re: [PATCH v6 1/2] x86/sev: Make sure IRQs are disabled while GHCB is active

2021-06-18 Thread Peter Zijlstra 
On Fri, Jun 18, 2021 at 10:17:54AM +0200, Joerg Roedel wrote:
> On Thu, Jun 17, 2021 at 05:38:46PM +0200, Peter Zijlstra wrote:
> > I'm getting (with all of v6.1 applied):
> > 
> > vmlinux.o: warning: objtool: __sev_es_nmi_complete()+0x1bf: call to panic() 
> > leaves .noinstr.text section
> > 
> > $ ./scripts/faddr2line defconfig-build/vmlinux __sev_es_nmi_complete+0x1bf
> > __sev_es_nmi_complete+0x1bf/0x1d0:
> > __sev_get_ghcb at arch/x86/kernel/sev.c:223
> > (inlined by) __sev_es_nmi_complete at arch/x86/kernel/sev.c:519
> 
> I see where this is coming from, but can't create the same warning. I
> did run 'objtool check -n vmlinux'. Is there more to do to get the full
> check?

You get those when you enable CONFIG_DEBUG_ENTRY=y (make sure to have
PARAVIRT=n, I so need to go fix that :/).
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v6 1/2] x86/sev: Make sure IRQs are disabled while GHCB is active

2021-06-18 Thread Joerg Roedel 
On Thu, Jun 17, 2021 at 05:00:48PM +0200, Peter Zijlstra wrote:
> I think this is broken, at this point RCU is quite dead on this CPU and
> local_irq_save/restore include tracing and all sorts.
> 
> Also, shouldn't IRQs already be disabled by the time we get here?

Yes it is, I removed these calls, made __sev_get/put_ghcb() noinstr
instead of __always_inline and put instrumentation_begin()/end() around
the panic() call in __sev_get_ghcb().

Regards,

Joerg
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v6 1/2] x86/sev: Make sure IRQs are disabled while GHCB is active

2021-06-18 Thread Joerg Roedel 
On Thu, Jun 17, 2021 at 05:38:46PM +0200, Peter Zijlstra wrote:
> I'm getting (with all of v6.1 applied):
> 
> vmlinux.o: warning: objtool: __sev_es_nmi_complete()+0x1bf: call to panic() 
> leaves .noinstr.text section
> 
> $ ./scripts/faddr2line defconfig-build/vmlinux __sev_es_nmi_complete+0x1bf
> __sev_es_nmi_complete+0x1bf/0x1d0:
> __sev_get_ghcb at arch/x86/kernel/sev.c:223
> (inlined by) __sev_es_nmi_complete at arch/x86/kernel/sev.c:519

I see where this is coming from, but can't create the same warning. I
did run 'objtool check -n vmlinux'. Is there more to do to get the full
check?

Regards,

Joerg
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v6 1/2] x86/sev: Make sure IRQs are disabled while GHCB is active

2021-06-17 Thread Peter Zijlstra 


On Wed, Jun 16, 2021 at 08:49:12PM +0200, Joerg Roedel wrote:
> @@ -514,7 +523,7 @@ void noinstr __sev_es_nmi_complete(void)
>   struct ghcb_state state;
>   struct ghcb *ghcb;
>  
> - ghcb = sev_es_get_ghcb();
> + ghcb = __sev_get_ghcb();
>  
>   vc_ghcb_invalidate(ghcb);
>   ghcb_set_sw_exit_code(ghcb, SVM_VMGEXIT_NMI_COMPLETE);
> @@ -524,7 +533,7 @@ void noinstr __sev_es_nmi_complete(void)
>   sev_es_wr_ghcb_msr(__pa_nodebug(ghcb));
>   VMGEXIT();
>  
> - sev_es_put_ghcb();
> + __sev_put_ghcb();
>  }

I'm getting (with all of v6.1 applied):

vmlinux.o: warning: objtool: __sev_es_nmi_complete()+0x1bf: call to panic() 
leaves .noinstr.text section

$ ./scripts/faddr2line defconfig-build/vmlinux __sev_es_nmi_complete+0x1bf
__sev_es_nmi_complete+0x1bf/0x1d0:
__sev_get_ghcb at arch/x86/kernel/sev.c:223
(inlined by) __sev_es_nmi_complete at arch/x86/kernel/sev.c:519


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v6 1/2] x86/sev: Make sure IRQs are disabled while GHCB is active

2021-06-17 Thread Peter Zijlstra 
On Wed, Jun 16, 2021 at 08:49:12PM +0200, Joerg Roedel wrote:

>  static void sev_es_ap_hlt_loop(void)
>  {
>   struct ghcb_state state;
> + unsigned long flags;
>   struct ghcb *ghcb;
>  
> - ghcb = sev_es_get_ghcb();
> + local_irq_save(flags);
> +
> + ghcb = __sev_get_ghcb();
>  
>   while (true) {
>   vc_ghcb_invalidate(ghcb);
> @@ -692,7 +704,9 @@ static void sev_es_ap_hlt_loop(void)
>   break;
>   }
>  
> - sev_es_put_ghcb();
> + __sev_put_ghcb();
> +
> + local_irq_restore(flags);
>  }

I think this is broken, at this point RCU is quite dead on this CPU and
local_irq_save/restore include tracing and all sorts.

Also, shouldn't IRQs already be disabled by the time we get here?
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization