| Message ID | 1350481786-4969-3-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: > In the past it used to point to 'sidt' (native_store_idt) operation > which is a non-privileged operation. This resulted in the > 'struct desc_ptr' value containing the address of Xen's IDT table, > instead of the IDT table that Linux thinks its using. The end result > is that doing: > > store_idt(&desc); > load_idt(&desc); > > would blow up b/c xen_load_idt would try to parse the IDT contents > (desc) and de-reference a virtual address that is outside Linux's > __va (it is in Xen's virtual address). > > With this patch we are providing the last written IDT address. > OK... this seems like another excellent set of pvops calls that should be nukable to Kingdom Come. There is no reason, ever, to read the IDT and GDT from the kernel... the kernel already knows what they should be! -hpa
On Wed, Oct 17, 2012 at 04:51:17PM -0700, H. Peter Anvin wrote: > On 10/17/2012 06:49 AM, Konrad Rzeszutek Wilk wrote: > >In the past it used to point to 'sidt' (native_store_idt) operation > >which is a non-privileged operation. This resulted in the > >'struct desc_ptr' value containing the address of Xen's IDT table, > >instead of the IDT table that Linux thinks its using. The end result > >is that doing: > > > > store_idt(&desc); > > load_idt(&desc); > > > >would blow up b/c xen_load_idt would try to parse the IDT contents > >(desc) and de-reference a virtual address that is outside Linux's > >__va (it is in Xen's virtual address). > > > >With this patch we are providing the last written IDT address. > > > > OK... this seems like another excellent set of pvops calls that > should be nukable to Kingdom Come. There is no reason, ever, to > read the IDT and GDT from the kernel... the kernel already knows > what they should be! Even during suspend and resume cycle? There are the sequence of sidt/lidt calls being done there. And we do need to filter at least the sidt call - and in the case of ACPI suspend path, the lidt. > > -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:45 AM, Konrad Rzeszutek Wilk wrote: >> >> OK... this seems like another excellent set of pvops calls that >> should be nukable to Kingdom Come. There is no reason, ever, to >> read the IDT and GDT from the kernel... the kernel already knows >> what they should be! > > Even during suspend and resume cycle? There are the sequence of > sidt/lidt calls being done there. And we do need to filter at > least the sidt call - and in the case of ACPI suspend path, > the lidt. Yes, I am pretty sure we can make static guarantees on the IDT and GDTs. -hpa
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c index e3497f2..f29d6d6 100644 --- a/arch/x86/xen/enlighten.c +++ b/arch/x86/xen/enlighten.c @@ -777,7 +777,13 @@ static void xen_load_idt(const struct desc_ptr *desc) spin_unlock(&lock); } +static void xen_store_idt(struct desc_ptr *dtr) +{ + const struct desc_ptr *desc = &__get_cpu_var(idt_desc); + dtr->address = desc->address; + dtr->size = desc->size; +} /* Write a GDT descriptor entry. Ignore LDT descriptors, since they're handled differently. */ static void xen_write_gdt_entry(struct desc_struct *dt, int entry, @@ -1200,7 +1206,7 @@ static const struct pv_cpu_ops xen_cpu_ops __initconst = { .free_ldt = xen_free_ldt, .store_gdt = native_store_gdt, - .store_idt = native_store_idt, + .store_idt = xen_store_idt, .store_tr = xen_store_tr, .write_ldt_entry = xen_write_ldt_entry,
In the past it used to point to 'sidt' (native_store_idt) operation which is a non-privileged operation. This resulted in the 'struct desc_ptr' value containing the address of Xen's IDT table, instead of the IDT table that Linux thinks its using. The end result is that doing: store_idt(&desc); load_idt(&desc); would blow up b/c xen_load_idt would try to parse the IDT contents (desc) and de-reference a virtual address that is outside Linux's __va (it is in Xen's virtual address). With this patch we are providing the last written IDT address. Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> --- arch/x86/xen/enlighten.c | 8 +++++++- 1 files changed, 7 insertions(+), 1 deletions(-)