2017-09-08 23:17:32

by kys

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

From: "K. Y. Srinivasan" <[email protected]>

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

Signed-off-by: K. Y. Srinivasan <[email protected]>
---
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


2017-09-09 11:04:50

by Dan Carpenter

[permalink] [raw]
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, [email protected] wrote:
> From: "K. Y. Srinivasan" <[email protected]>
>
> 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

2017-09-09 16:00:54

by KY Srinivasan

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



> -----Original Message-----
> From: Dan Carpenter [mailto:[email protected]]
> Sent: Saturday, September 9, 2017 4:04 AM
> To: KY Srinivasan <[email protected]>
> Cc: [email protected]; [email protected]; linux-
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]
> 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, [email protected]
> wrote:
> > From: "K. Y. Srinivasan" <[email protected]>
> >
> > 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


2017-09-13 09:01:03

by Ingo Molnar

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


* KY Srinivasan <[email protected]> wrote:

>
>
> > -----Original Message-----
> > From: Dan Carpenter [mailto:[email protected]]
> > Sent: Saturday, September 9, 2017 4:04 AM
> > To: KY Srinivasan <[email protected]>
> > Cc: [email protected]; [email protected]; linux-
> > [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]
> > 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, [email protected]
> > wrote:
> > > From: "K. Y. Srinivasan" <[email protected]>
> > >
> > > 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

Subject: [tip:x86/urgent] x86/hyper-V: Allocate the IDT entry early in boot

Commit-ID: 213ff44ae4eb5224010166db2f851e4eea068268
Gitweb: http://git.kernel.org/tip/213ff44ae4eb5224010166db2f851e4eea068268
Author: K. Y. Srinivasan <[email protected]>
AuthorDate: Fri, 8 Sep 2017 16:15:57 -0700
Committer: Ingo Molnar <[email protected]>
CommitDate: Wed, 13 Sep 2017 11:02:26 +0200

x86/hyper-V: Allocate the IDT entry early in boot

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

The previous code would allocate the entry as part of registering the handler
when the vmbus driver loaded, and this caused a problem for the IDT cleanup
that Thomas is working on for v4.15.

Signed-off-by: K. Y. Srinivasan <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
---
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 3b3f713..236324e8 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
}


2017-09-13 18:05:09

by KY Srinivasan

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



> -----Original Message-----
> From: Ingo Molnar [mailto:[email protected]] On Behalf Of Ingo
> Molnar
> Sent: Wednesday, September 13, 2017 2:01 AM
> To: KY Srinivasan <[email protected]>
> Cc: Dan Carpenter <[email protected]>; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]
> Subject: Re: [PATCH 1/1] x86/hyper-V: Allocate the IDT entry early in boot
>
>
> * KY Srinivasan <[email protected]> wrote:
>
> >
> >
> > > -----Original Message-----
> > > From: Dan Carpenter [mailto:[email protected]]
> > > Sent: Saturday, September 9, 2017 4:04 AM
> > > To: KY Srinivasan <[email protected]>
> > > Cc: [email protected]; [email protected]; linux-
> > > [email protected]; [email protected]; [email protected];
> > > [email protected]; [email protected]; [email protected];
> > > [email protected]; [email protected]
> > > 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, [email protected]
> > > wrote:
> > > > From: "K. Y. Srinivasan" <[email protected]>
> > > >
> > > > 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