Jan Beulich
2008-Feb-01 08:45 UTC
[Xen-devel] [PATCH] x86: adjust reserved bit page fault handling
One could even debate whether reserved bit faults are always fatal (and
should never be propagated to the guest)...
Signed-off-by: Jan Beulich <jbeulich@novell.com>
Index: 2008-01-28/xen/arch/x86/traps.c
==================================================================---
2008-01-28.orig/xen/arch/x86/traps.c 2008-01-28 11:31:44.000000000 +0100
+++ 2008-01-28/xen/arch/x86/traps.c 2008-01-30 11:47:39.000000000 +0100
@@ -823,6 +823,17 @@ asmlinkage void do_machine_check(struct
machine_check_vector(regs, regs->error_code);
}
+static inline void reserved_bit_page_fault(int guest, unsigned long addr,
+ struct cpu_user_regs *regs)
+{
+ if ( guest )
+ gdprintk(XENLOG_ERR, "reserved bit in page table entry: ");
+ else
+ dprintk(XENLOG_ERR, "reserved bit in page table entry: ");
+ show_page_walk(addr);
+ show_execution_state(regs);
+}
+
void propagate_page_fault(unsigned long addr, u16 error_code)
{
struct trap_info *ti;
@@ -852,6 +863,8 @@ void propagate_page_fault(unsigned long
v->domain->domain_id, v->vcpu_id, error_code);
show_page_walk(addr);
}
+ else if ( unlikely(error_code & PFEC_reserved_bit) )
+ reserved_bit_page_fault(1, addr, guest_cpu_user_regs());
}
static int handle_gdt_ldt_mapping_fault(
@@ -1034,8 +1047,10 @@ static int fixup_page_fault(unsigned lon
struct vcpu *v = current;
struct domain *d = v->domain;
- /* No fixups in interrupt context or when interrupts are disabled. */
- if ( in_irq() || !(regs->eflags & X86_EFLAGS_IF) )
+ /* No fixups in interrupt context, when interrupts are disabled, or
+ * when a reserved bit was found to be set. */
+ if ( in_irq() || !(regs->eflags & X86_EFLAGS_IF) ||
+ (regs->error_code & PFEC_reserved_bit) )
return 0;
if ( unlikely(IN_HYPERVISOR_RANGE(addr)) )
@@ -1101,6 +1116,8 @@ asmlinkage void do_page_fault(struct cpu
if ( likely((fixup = search_exception_table(regs->eip)) != 0) )
{
perfc_incr(copy_user_faults);
+ if ( unlikely(regs->error_code & PFEC_reserved_bit) )
+ reserved_bit_page_fault(0, addr, regs);
regs->eip = fixup;
return;
}
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Tim Deegan
2008-Feb-01 09:28 UTC
Re: [Xen-devel] [PATCH] x86: adjust reserved bit page fault handling
At 08:45 +0000 on 01 Feb (1201855522), Jan Beulich wrote:> @@ -1034,8 +1047,10 @@ static int fixup_page_fault(unsigned lon > struct vcpu *v = current; > struct domain *d = v->domain; > > - /* No fixups in interrupt context or when interrupts are disabled. */ > - if ( in_irq() || !(regs->eflags & X86_EFLAGS_IF) ) > + /* No fixups in interrupt context, when interrupts are disabled, or > + * when a reserved bit was found to be set. */ > + if ( in_irq() || !(regs->eflags & X86_EFLAGS_IF) || > + (regs->error_code & PFEC_reserved_bit) ) > return 0; > > if ( unlikely(IN_HYPERVISOR_RANGE(addr)) )The shadow pagetable code deliberately introduces invalid pagetable entries as part of its fast-path treatment of MMIO and not-present entries in the guest tables, so paging_fault needs to be called for PFEC_reserved_bit faults. Cheers, Tim. -- Tim Deegan <Tim.Deegan@citrix.com> Principal Software Engineer, Citrix Systems (R&D) Ltd. [Company #02300071, SL9 0DZ, UK.] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jan Beulich
2008-Feb-01 10:38 UTC
Re: [Xen-devel] [PATCH] x86: adjust reserved bit page fault handling
>>> Tim Deegan <Tim.Deegan@citrix.com> 01.02.08 10:28 >>> >At 08:45 +0000 on 01 Feb (1201855522), Jan Beulich wrote: >> @@ -1034,8 +1047,10 @@ static int fixup_page_fault(unsigned lon >> struct vcpu *v = current; >> struct domain *d = v->domain; >> >> - /* No fixups in interrupt context or when interrupts are disabled. */ >> - if ( in_irq() || !(regs->eflags & X86_EFLAGS_IF) ) >> + /* No fixups in interrupt context, when interrupts are disabled, or >> + * when a reserved bit was found to be set. */ >> + if ( in_irq() || !(regs->eflags & X86_EFLAGS_IF) || >> + (regs->error_code & PFEC_reserved_bit) ) >> return 0; >> >> if ( unlikely(IN_HYPERVISOR_RANGE(addr)) ) > >The shadow pagetable code deliberately introduces invalid pagetable >entries as part of its fast-path treatment of MMIO and not-present >entries in the guest tables, so paging_fault needs to be called for >PFEC_reserved_bit faults.Here''s a replacement patch then, it would still be questionable whether reserved bit faults should ever be propagated to a guest... Signed-off-by: Jan Beulich <jbeulich@novell.com> Index: 2008-01-28/xen/arch/x86/traps.c ==================================================================--- 2008-01-28.orig/xen/arch/x86/traps.c 2008-01-28 11:31:44.000000000 +0100 +++ 2008-01-28/xen/arch/x86/traps.c 2008-02-01 11:25:49.000000000 +0100 @@ -823,6 +823,18 @@ asmlinkage void do_machine_check(struct machine_check_vector(regs, regs->error_code); } +static inline void reserved_bit_page_fault(int guest, unsigned long addr, + struct cpu_user_regs *regs) +{ + if ( guest ) + gdprintk(XENLOG_ERR, ""); + else + dprintk(XENLOG_ERR, ""); + printk("reserved bit in page table (ec=%04X): ", regs->error_code); + show_page_walk(addr); + show_execution_state(regs); +} + void propagate_page_fault(unsigned long addr, u16 error_code) { struct trap_info *ti; @@ -852,6 +864,8 @@ void propagate_page_fault(unsigned long v->domain->domain_id, v->vcpu_id, error_code); show_page_walk(addr); } + else if ( unlikely(error_code & PFEC_reserved_bit) ) + reserved_bit_page_fault(1, addr, guest_cpu_user_regs()); } static int handle_gdt_ldt_mapping_fault( @@ -1047,7 +1061,8 @@ static int fixup_page_fault(unsigned lon trace_trap_two_addr(TRC_PV_PAGING_FIXUP, regs->eip, addr); return ret; } - if ( (addr >= GDT_LDT_VIRT_START) && (addr < GDT_LDT_VIRT_END) ) + if ( !(regs->error_code & PFEC_reserved_bit) && + (addr >= GDT_LDT_VIRT_START) && (addr < GDT_LDT_VIRT_END) ) return handle_gdt_ldt_mapping_fault( addr - GDT_LDT_VIRT_START, regs); return 0; @@ -1057,7 +1072,8 @@ static int fixup_page_fault(unsigned lon guest_kernel_mode(v, regs) && /* Do not check if access-protection fault since the page may legitimately be not present in shadow page tables */ - ((regs->error_code & PFEC_write_access) == PFEC_write_access) && + ((regs->error_code & (PFEC_write_access|PFEC_reserved_bit)) =+ PFEC_write_access) && ptwr_do_page_fault(v, addr, regs) ) return EXCRET_fault_fixed; @@ -1101,6 +1117,8 @@ asmlinkage void do_page_fault(struct cpu if ( likely((fixup = search_exception_table(regs->eip)) != 0) ) { perfc_incr(copy_user_faults); + if ( unlikely(regs->error_code & PFEC_reserved_bit) ) + reserved_bit_page_fault(0, addr, regs); regs->eip = fixup; return; } _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2008-Feb-01 10:54 UTC
Re: [Xen-devel] [PATCH] x86: adjust reserved bit page fault handling
On 1/2/08 10:38, "Jan Beulich" <jbeulich@novell.com> wrote:>> The shadow pagetable code deliberately introduces invalid pagetable >> entries as part of its fast-path treatment of MMIO and not-present >> entries in the guest tables, so paging_fault needs to be called for >> PFEC_reserved_bit faults. > > Here''s a replacement patch then, it would still be questionable whether > reserved bit faults should ever be propagated to a guest... > > Signed-off-by: Jan Beulich <jbeulich@novell.com>What problem does this patch solve? -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jan Beulich
2008-Feb-01 11:24 UTC
Re: [Xen-devel] [PATCH] x86: adjust reserved bit page fault handling
>>> Keir Fraser <Keir.Fraser@cl.cam.ac.uk> 01.02.08 11:54 >>> >On 1/2/08 10:38, "Jan Beulich" <jbeulich@novell.com> wrote: > >>> The shadow pagetable code deliberately introduces invalid pagetable >>> entries as part of its fast-path treatment of MMIO and not-present >>> entries in the guest tables, so paging_fault needs to be called for >>> PFEC_reserved_bit faults. >> >> Here''s a replacement patch then, it would still be questionable whether >> reserved bit faults should ever be propagated to a guest... >> >> Signed-off-by: Jan Beulich <jbeulich@novell.com> > >What problem does this patch solve?When debugging the xen_change_pte_range() problem I mistakenly interpreted a page fault error code 15 as being decimal (it really was hex) and looked at the reserved bit fault handling. The patch is the outcome of this; despite it turned out to not be a fault of this kind I thought that tightening the handling (and adding the printing, since reserved bit faults are [except in the case mentioned by Tim] always bad) wouldn''t be a bad idea. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel