[PATCH 1/1] x86/hyper-V: Allocate the IDT entry early in boot

[PATCH 1/1] x86/hyper-V: Allocate the IDT entry early in boot

[K. Y. Srinivasan]

Allocate the hypervisor callback IDT entry early in the boot sequence.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 arch/x86/kernel/cpu/mshyperv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
index 3b3f713e15e5..236324e83a3a 100644
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -59,8 +59,6 @@ void hyperv_vector_handler(struct pt_regs *regs)
 void hv_setup_vmbus_irq(void (*handler)(void))
 {
 	vmbus_handler = handler;
-	/* Setup the IDT for hypervisor callback */
-	alloc_intr_gate(HYPERVISOR_CALLBACK_VECTOR, hyperv_callback_vector);
 }
 
 void hv_remove_vmbus_irq(void)
@@ -251,6 +249,8 @@ static void __init ms_hyperv_init_platform(void)
 	 */
 	x86_platform.apic_post_init = hyperv_init;
 	hyperv_setup_mmu_ops();
+	/* Setup the IDT for hypervisor callback */
+	alloc_intr_gate(HYPERVISOR_CALLBACK_VECTOR, hyperv_callback_vector);
 #endif
 }
 
-- 
2.14.1

Re: [PATCH 1/1] x86/hyper-V: Allocate the IDT entry early in boot

[Thomas Gleixner]

On Fri, 8 Sep 2017, K. Y. Srinivasan wrote:

> Allocate the hypervisor callback IDT entry early in the boot sequence.
> 
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> ---
>  arch/x86/kernel/cpu/mshyperv.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> index 3b3f713e15e5..236324e83a3a 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -59,8 +59,6 @@ void hyperv_vector_handler(struct pt_regs *regs)
>  void hv_setup_vmbus_irq(void (*handler)(void))
>  {
>  	vmbus_handler = handler;
> -	/* Setup the IDT for hypervisor callback */
> -	alloc_intr_gate(HYPERVISOR_CALLBACK_VECTOR, hyperv_callback_vector);
>  }
>  
>  void hv_remove_vmbus_irq(void)
> @@ -251,6 +249,8 @@ static void __init ms_hyperv_init_platform(void)
>  	 */
>  	x86_platform.apic_post_init = hyperv_init;
>  	hyperv_setup_mmu_ops();
> +	/* Setup the IDT for hypervisor callback */
> +	alloc_intr_gate(HYPERVISOR_CALLBACK_VECTOR, hyperv_callback_vector);
>  #endif
>  }

Nice. So we can make all of that gate/idt stuff __init now.

That makes my previous fix obsolete, right?

Thanks,

	tglx

RE: [PATCH 1/1] x86/hyper-V: Allocate the IDT entry early in boot

[KY Srinivasan]

> -----Original Message-----
> From: Thomas Gleixner [mailto:tglx@linutronix.de]
> Sent: Saturday, September 9, 2017 2:40 AM
> To: KY Srinivasan <kys@microsoft.com>
> Cc: x86@kernel.org; gregkh@linuxfoundation.org; linux-
> kernel@vger.kernel.org; devel@linuxdriverproject.org; olaf@aepfle.de;
> apw@canonical.com; jasowang@redhat.com; hpa@zytor.com;
> mingo@kernel.org
> Subject: Re: [PATCH 1/1] x86/hyper-V: Allocate the IDT entry early in boot
> 
> On Fri, 8 Sep 2017, K. Y. Srinivasan wrote:
> 
> > Allocate the hypervisor callback IDT entry early in the boot sequence.
> >
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > ---
> >  arch/x86/kernel/cpu/mshyperv.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/x86/kernel/cpu/mshyperv.c
> b/arch/x86/kernel/cpu/mshyperv.c
> > index 3b3f713e15e5..236324e83a3a 100644
> > --- a/arch/x86/kernel/cpu/mshyperv.c
> > +++ b/arch/x86/kernel/cpu/mshyperv.c
> > @@ -59,8 +59,6 @@ void hyperv_vector_handler(struct pt_regs *regs)
> >  void hv_setup_vmbus_irq(void (*handler)(void))
> >  {
> >  	vmbus_handler = handler;
> > -	/* Setup the IDT for hypervisor callback */
> > -	alloc_intr_gate(HYPERVISOR_CALLBACK_VECTOR,
> hyperv_callback_vector);
> >  }
> >
> >  void hv_remove_vmbus_irq(void)
> > @@ -251,6 +249,8 @@ static void __init ms_hyperv_init_platform(void)
> >  	 */
> >  	x86_platform.apic_post_init = hyperv_init;
> >  	hyperv_setup_mmu_ops();
> > +	/* Setup the IDT for hypervisor callback */
> > +	alloc_intr_gate(HYPERVISOR_CALLBACK_VECTOR,
> hyperv_callback_vector);
> >  #endif
> >  }
> 
> Nice. So we can make all of that gate/idt stuff __init now.
> 
> That makes my previous fix obsolete, right?

Yes.

K. Y
> 
> Thanks,
> 
> 	tglx
> 

[PATCH 1/1] x86/hyper-V: Allocate the IDT entry early in boot

[kys]

From: "K. Y. Srinivasan" <kys@microsoft.com>

Allocate the hypervisor callback IDT entry early in the boot sequence.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 arch/x86/kernel/cpu/mshyperv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
index 3b3f713e15e5..236324e83a3a 100644
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -59,8 +59,6 @@ void hyperv_vector_handler(struct pt_regs *regs)
 void hv_setup_vmbus_irq(void (*handler)(void))
 {
 	vmbus_handler = handler;
-	/* Setup the IDT for hypervisor callback */
-	alloc_intr_gate(HYPERVISOR_CALLBACK_VECTOR, hyperv_callback_vector);
 }
 
 void hv_remove_vmbus_irq(void)
@@ -251,6 +249,8 @@ static void __init ms_hyperv_init_platform(void)
 	 */
 	x86_platform.apic_post_init = hyperv_init;
 	hyperv_setup_mmu_ops();
+	/* Setup the IDT for hypervisor callback */
+	alloc_intr_gate(HYPERVISOR_CALLBACK_VECTOR, hyperv_callback_vector);
 #endif
 }
 
-- 
2.14.1

Re: [PATCH 1/1] x86/hyper-V: Allocate the IDT entry early in boot

[Dan Carpenter]

On Fri, Sep 08, 2017 at 04:15:57PM -0700, kys@exchange.microsoft.com wrote:
> From: "K. Y. Srinivasan" <kys@microsoft.com>
> 
> Allocate the hypervisor callback IDT entry early in the boot sequence.
> 

I'm guessing this fixes a NULL dereference or something?  The changelog
doesn't really say why we are doing this.

regards,
dan carpenter

RE: [PATCH 1/1] x86/hyper-V: Allocate the IDT entry early in boot

[KY Srinivasan]

> -----Original Message-----
> From: Dan Carpenter [mailto:dan.carpenter@oracle.com]
> Sent: Saturday, September 9, 2017 4:04 AM
> To: KY Srinivasan <kys@microsoft.com>
> Cc: x86@kernel.org; gregkh@linuxfoundation.org; linux-
> kernel@vger.kernel.org; devel@linuxdriverproject.org; olaf@aepfle.de;
> apw@canonical.com; jasowang@redhat.com; tglx@linutronix.de;
> hpa@zytor.com; mingo@kernel.org
> Subject: Re: [PATCH 1/1] x86/hyper-V: Allocate the IDT entry early in boot
> 
> On Fri, Sep 08, 2017 at 04:15:57PM -0700, kys@exchange.microsoft.com
> wrote:
> > From: "K. Y. Srinivasan" <kys@microsoft.com>
> >
> > Allocate the hypervisor callback IDT entry early in the boot sequence.
> >
> 
> I'm guessing this fixes a NULL dereference or something?  The changelog
> doesn't really say why we are doing this.

The changelog does say what we are doing - allocating the IDT entry early in the boot sequence.
The current code would allocate the entry as part of registering the handler when vmbus driver
loaded and this caused a problem for the cleanup Thomas had implemented.

K. Y
> 
> regards,
> dan carpenter

Re: [PATCH 1/1] x86/hyper-V: Allocate the IDT entry early in boot

[Ingo Molnar]

* KY Srinivasan <kys@microsoft.com> wrote:

> 
> 
> > -----Original Message-----
> > From: Dan Carpenter [mailto:dan.carpenter@oracle.com]
> > Sent: Saturday, September 9, 2017 4:04 AM
> > To: KY Srinivasan <kys@microsoft.com>
> > Cc: x86@kernel.org; gregkh@linuxfoundation.org; linux-
> > kernel@vger.kernel.org; devel@linuxdriverproject.org; olaf@aepfle.de;
> > apw@canonical.com; jasowang@redhat.com; tglx@linutronix.de;
> > hpa@zytor.com; mingo@kernel.org
> > Subject: Re: [PATCH 1/1] x86/hyper-V: Allocate the IDT entry early in boot
> > 
> > On Fri, Sep 08, 2017 at 04:15:57PM -0700, kys@exchange.microsoft.com
> > wrote:
> > > From: "K. Y. Srinivasan" <kys@microsoft.com>
> > >
> > > Allocate the hypervisor callback IDT entry early in the boot sequence.
> > >
> > 
> > I'm guessing this fixes a NULL dereference or something?  The changelog
> > doesn't really say why we are doing this.
> 
> The changelog does say what we are doing - allocating the IDT entry early in the boot sequence.

But the question was the 'why', not the 'what' - so Dan's question is
fully justified ...

> The current code would allocate the entry as part of registering the handler 
> when vmbus driver loaded and this caused a problem for the cleanup Thomas had 
> implemented.

I've put this explanation into the changelog.

Thanks,

	Ingo

RE: [PATCH 1/1] x86/hyper-V: Allocate the IDT entry early in boot

[KY Srinivasan]

> -----Original Message-----
> From: Ingo Molnar [mailto:mingo.kernel.org@gmail.com] On Behalf Of Ingo
> Molnar
> Sent: Wednesday, September 13, 2017 2:01 AM
> To: KY Srinivasan <kys@microsoft.com>
> Cc: Dan Carpenter <dan.carpenter@oracle.com>; x86@kernel.org;
> gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> jasowang@redhat.com; tglx@linutronix.de; hpa@zytor.com
> Subject: Re: [PATCH 1/1] x86/hyper-V: Allocate the IDT entry early in boot
> 
> 
> * KY Srinivasan <kys@microsoft.com> wrote:
> 
> >
> >
> > > -----Original Message-----
> > > From: Dan Carpenter [mailto:dan.carpenter@oracle.com]
> > > Sent: Saturday, September 9, 2017 4:04 AM
> > > To: KY Srinivasan <kys@microsoft.com>
> > > Cc: x86@kernel.org; gregkh@linuxfoundation.org; linux-
> > > kernel@vger.kernel.org; devel@linuxdriverproject.org; olaf@aepfle.de;
> > > apw@canonical.com; jasowang@redhat.com; tglx@linutronix.de;
> > > hpa@zytor.com; mingo@kernel.org
> > > Subject: Re: [PATCH 1/1] x86/hyper-V: Allocate the IDT entry early in boot
> > >
> > > On Fri, Sep 08, 2017 at 04:15:57PM -0700, kys@exchange.microsoft.com
> > > wrote:
> > > > From: "K. Y. Srinivasan" <kys@microsoft.com>
> > > >
> > > > Allocate the hypervisor callback IDT entry early in the boot sequence.
> > > >
> > >
> > > I'm guessing this fixes a NULL dereference or something?  The changelog
> > > doesn't really say why we are doing this.
> >
> > The changelog does say what we are doing - allocating the IDT entry early in
> the boot sequence.
> 
> But the question was the 'why', not the 'what' - so Dan's question is
> fully justified ...
> 
> > The current code would allocate the entry as part of registering the handler
> > when vmbus driver loaded and this caused a problem for the cleanup
> Thomas had
> > implemented.
> 
> I've put this explanation into the changelog.

You are right. Thanks, Ingo.

K. Y
> 
> Thanks,
> 
> 	Ingo