| Message ID | 1350481786-4969-2-git-send-email-konrad.wilk@oracle.com (mailing list archive) |
|---|---|
| State | RFC, archived |
| Headers | show |
On 10/17/2012 06:49 AM, Konrad Rzeszutek Wilk wrote: > We check the TSS descriptor before we try to dereference it. > Also fix up the value to use the #defines. > > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > --- > arch/x86/power/cpu.c | 7 +++++-- > 1 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/power/cpu.c b/arch/x86/power/cpu.c > index 218cdb1..c17370e 100644 > --- a/arch/x86/power/cpu.c > +++ b/arch/x86/power/cpu.c > @@ -133,7 +133,9 @@ static void fix_processor_context(void) > { > int cpu = smp_processor_id(); > struct tss_struct *t = &per_cpu(init_tss, cpu); > - > +#ifdef CONFIG_X86_64 > + struct desc_struct *desc = get_cpu_gdt_table(cpu); > +#endif > set_tss_desc(cpu, t); /* > * This just modifies memory; should not be > * necessary. But... This is necessary, because > @@ -142,7 +144,8 @@ static void fix_processor_context(void) > */ > > #ifdef CONFIG_X86_64 > - get_cpu_gdt_table(cpu)[GDT_ENTRY_TSS].type = 9; > + if (!desc_empty(&desc[GDT_ENTRY_TSS])) > + desc[GDT_ENTRY_TSS].type = DESC_TSS; > > syscall_init(); /* This sets MSR_*STAR and related */ > #endif > Why is this patch necessary? Presumably there is something further down the line which depends on the TSS descriptor being empty, but if so, what? -hpa
On Wed, Oct 17, 2012 at 05:03:09PM -0700, H. Peter Anvin wrote: > On 10/17/2012 06:49 AM, Konrad Rzeszutek Wilk wrote: > >We check the TSS descriptor before we try to dereference it. > >Also fix up the value to use the #defines. > > > >Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > >--- > > arch/x86/power/cpu.c | 7 +++++-- > > 1 files changed, 5 insertions(+), 2 deletions(-) > > > >diff --git a/arch/x86/power/cpu.c b/arch/x86/power/cpu.c > >index 218cdb1..c17370e 100644 > >--- a/arch/x86/power/cpu.c > >+++ b/arch/x86/power/cpu.c > >@@ -133,7 +133,9 @@ static void fix_processor_context(void) > > { > > int cpu = smp_processor_id(); > > struct tss_struct *t = &per_cpu(init_tss, cpu); > >- > >+#ifdef CONFIG_X86_64 > >+ struct desc_struct *desc = get_cpu_gdt_table(cpu); > >+#endif > > set_tss_desc(cpu, t); /* > > * This just modifies memory; should not be > > * necessary. But... This is necessary, because > >@@ -142,7 +144,8 @@ static void fix_processor_context(void) > > */ > > > > #ifdef CONFIG_X86_64 > >- get_cpu_gdt_table(cpu)[GDT_ENTRY_TSS].type = 9; > >+ if (!desc_empty(&desc[GDT_ENTRY_TSS])) > >+ desc[GDT_ENTRY_TSS].type = DESC_TSS; > > > > syscall_init(); /* This sets MSR_*STAR and related */ > > #endif > > > > Why is this patch necessary? Presumably there is something further > down the line which depends on the TSS descriptor being empty, but > if so, what? I could not find it. This write has been in the code since the initial git history. Is the pre-git bitkeeper tree somewhere available? > > -hpa > > -- > H. Peter Anvin, Intel Open Source Technology Center > I work for Intel. I don't speak on their behalf. > -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/18/2012 07:47 AM, Konrad Rzeszutek Wilk wrote: >> >> Why is this patch necessary? Presumably there is something further >> down the line which depends on the TSS descriptor being empty, but >> if so, what? > > I could not find it. This write has been in the code since the initial > git history. Is the pre-git bitkeeper tree somewhere available? I didn't ask why the write was necessary, but why you need it to be conditional. I know why the write is necessary: it is (presumably) there to clear the TSS busy bit. However, as for older records: -bk era history (2.5.0-2.6.12-rc2): git://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git Ancient history (pre-2.4.0): git://git.kernel.org/pub/scm/linux/kernel/git/davej/history.git There doesn't seem to be any git trees known for the bridge between 2.4.0 and 2.5.0. -hpa
On Wed, Oct 17, 2012 at 05:03:09PM -0700, H. Peter Anvin wrote: > On 10/17/2012 06:49 AM, Konrad Rzeszutek Wilk wrote: > >We check the TSS descriptor before we try to dereference it. > >Also fix up the value to use the #defines. > > > >Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > >--- > > arch/x86/power/cpu.c | 7 +++++-- > > 1 files changed, 5 insertions(+), 2 deletions(-) > > > >diff --git a/arch/x86/power/cpu.c b/arch/x86/power/cpu.c > >index 218cdb1..c17370e 100644 > >--- a/arch/x86/power/cpu.c > >+++ b/arch/x86/power/cpu.c > >@@ -133,7 +133,9 @@ static void fix_processor_context(void) > > { > > int cpu = smp_processor_id(); > > struct tss_struct *t = &per_cpu(init_tss, cpu); > >- > >+#ifdef CONFIG_X86_64 > >+ struct desc_struct *desc = get_cpu_gdt_table(cpu); > >+#endif > > set_tss_desc(cpu, t); /* > > * This just modifies memory; should not be > > * necessary. But... This is necessary, because > >@@ -142,7 +144,8 @@ static void fix_processor_context(void) > > */ > > > > #ifdef CONFIG_X86_64 > >- get_cpu_gdt_table(cpu)[GDT_ENTRY_TSS].type = 9; > >+ if (!desc_empty(&desc[GDT_ENTRY_TSS])) > >+ desc[GDT_ENTRY_TSS].type = DESC_TSS; > > > > syscall_init(); /* This sets MSR_*STAR and related */ > > #endif > > > > Why is this patch necessary? Presumably there is something further > down the line which depends on the TSS descriptor being empty, but > if so, what? Ah, so at least on Xen, that desc is marked as RO b/c it has been given to the hypervisor. And the hypervisor adds its own entries. Lastly on 64-bit, the TSS for PV guests is not used - as the PV kernel and user-space both run in the user-space ring. So there is nothing in that entry. The issue I stumbled upon was just a page-fault b/c of trying to modify a RO region. The other way of fixing this (without knowing that the TSS entry is not set), was to use the functions/macros, as such: if (alternative_tss) { memcpy(&tss, &desc[GDT_ENTRY_TSS], sizeof(tss_desc)); tss.type = DESC_TSS; write_gdt_entry(desc, GDT_ENTRY_TSS, &tss, DESC_TSS); } else { if (!desc_empty(&desc[GDT_ENTRY_TSS])) { desc[GDT_ENTRY_TSS].type = DESC_TSS; } } which works as well and does not throw the person from trying to figure out why the TSS descriptor is empty (or not). > > -hpa > > -- > H. Peter Anvin, Intel Open Source Technology Center > I work for Intel. I don't speak on their behalf. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/power/cpu.c b/arch/x86/power/cpu.c index 218cdb1..c17370e 100644 --- a/arch/x86/power/cpu.c +++ b/arch/x86/power/cpu.c @@ -133,7 +133,9 @@ static void fix_processor_context(void) { int cpu = smp_processor_id(); struct tss_struct *t = &per_cpu(init_tss, cpu); - +#ifdef CONFIG_X86_64 + struct desc_struct *desc = get_cpu_gdt_table(cpu); +#endif set_tss_desc(cpu, t); /* * This just modifies memory; should not be * necessary. But... This is necessary, because @@ -142,7 +144,8 @@ static void fix_processor_context(void) */ #ifdef CONFIG_X86_64 - get_cpu_gdt_table(cpu)[GDT_ENTRY_TSS].type = 9; + if (!desc_empty(&desc[GDT_ENTRY_TSS])) + desc[GDT_ENTRY_TSS].type = DESC_TSS; syscall_init(); /* This sets MSR_*STAR and related */ #endif
We check the TSS descriptor before we try to dereference it. Also fix up the value to use the #defines. Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> --- arch/x86/power/cpu.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-)