# HG changeset patch
# User Keir Fraser <keir.fraser@xxxxxxxxxx>
# Date 1210077138 -3600
# Node ID 01aa7c088e983cd54b61faeb3ff533581714a26f
# Parent e6f20d5ed5fe7e24eab12977d812bd499794ba30
SVM: clean up __get_instruction_length_from_list()
Remove unused arguments, fix its behaviour near page boundaries,
inject appropriate pagefaults, and inject #GP if the instruction is
not decodable or %eip is not pointing to valid RAM.
Signed-off-by: Tim Deegan <Tim.Deegan@xxxxxxxxxx>
Signed-off-by: Keir Fraser <keir.fraser@xxxxxxxxxx>
---
xen/arch/x86/hvm/svm/emulate.c | 115 +++++++++++++++++-----------------
xen/arch/x86/hvm/svm/svm.c | 68 ++++++++++----------
xen/include/asm-x86/hvm/svm/emulate.h | 11 +--
3 files changed, 100 insertions(+), 94 deletions(-)
diff -r e6f20d5ed5fe -r 01aa7c088e98 xen/arch/x86/hvm/svm/emulate.c
--- a/xen/arch/x86/hvm/svm/emulate.c Tue May 06 11:05:00 2008 +0100
+++ b/xen/arch/x86/hvm/svm/emulate.c Tue May 06 13:32:18 2008 +0100
@@ -28,18 +28,6 @@
#include <asm/hvm/svm/emulate.h>
#define MAX_INST_LEN 15
-
-static int inst_copy_from_guest(
- unsigned char *buf, unsigned long guest_eip, int inst_len)
-{
- struct vmcb_struct *vmcb = current->arch.hvm_svm.vmcb;
- uint32_t pfec = (vmcb->cpl == 3) ? PFEC_user_mode : 0;
- if ( (inst_len > MAX_INST_LEN) || (inst_len <= 0) )
- return 0;
- if ( hvm_fetch_from_guest_virt_nofault(buf, guest_eip, inst_len, pfec) )
- return 0;
- return inst_len;
-}
static unsigned int is_prefix(u8 opc)
{
@@ -73,12 +61,7 @@ static unsigned long svm_rip2pointer(str
return p;
}
-/*
- * Here's how it works:
- * First byte: Length.
- * Following bytes: Opcode bytes.
- * Special case: Last byte, if zero, doesn't need to match.
- */
+/* First byte: Length. Following bytes: Opcode bytes. */
#define MAKE_INSTR(nm, ...) static const u8 OPCODE_##nm[] = { __VA_ARGS__ }
MAKE_INSTR(INVD, 2, 0x0f, 0x08);
MAKE_INSTR(WBINVD, 2, 0x0f, 0x09);
@@ -101,70 +84,90 @@ static const u8 *opc_bytes[INSTR_MAX_COU
[INSTR_INT3] = OPCODE_INT3
};
+static int fetch(struct vcpu *v, u8 *buf, unsigned long addr, int len)
+{
+ uint32_t pfec = (v->arch.hvm_svm.vmcb->cpl == 3) ? PFEC_user_mode : 0;
+
+ switch ( hvm_fetch_from_guest_virt(buf, addr, len, pfec) )
+ {
+ case HVMCOPY_okay:
+ return 1;
+ case HVMCOPY_bad_gva_to_gfn:
+ /* OK just to give up; we'll have injected #PF already */
+ return 0;
+ case HVMCOPY_bad_gfn_to_mfn:
+ default:
+ /* Not OK: fetches from non-RAM pages are not supportable. */
+ gdprintk(XENLOG_WARNING, "Bad instruction fetch at %#lx (%#lx)\n",
+ (unsigned long) guest_cpu_user_regs()->eip, addr);
+ hvm_inject_exception(TRAP_gp_fault, 0, 0);
+ return 0;
+ }
+}
+
int __get_instruction_length_from_list(struct vcpu *v,
- enum instruction_index *list, unsigned int list_count,
- u8 *guest_eip_buf, enum instruction_index *match)
+ enum instruction_index *list, unsigned int list_count)
{
struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb;
unsigned int i, j, inst_len = 0;
- int found = 0;
enum instruction_index instr = 0;
- u8 buffer[MAX_INST_LEN];
- u8 *buf;
+ u8 buf[MAX_INST_LEN];
const u8 *opcode = NULL;
+ unsigned long fetch_addr;
+ unsigned int fetch_len;
- if ( guest_eip_buf )
+ /* Fetch up to the next page break; we'll fetch from the next page
+ * later if we have to. */
+ fetch_addr = svm_rip2pointer(v);
+ fetch_len = min_t(unsigned int, MAX_INST_LEN,
+ PAGE_SIZE - (fetch_addr & ~PAGE_MASK));
+ if ( !fetch(v, buf, fetch_addr, fetch_len) )
+ return 0;
+
+ while ( (inst_len < MAX_INST_LEN) && is_prefix(buf[inst_len]) )
{
- buf = guest_eip_buf;
- }
- else
- {
- if ( inst_copy_from_guest(buffer, svm_rip2pointer(v), MAX_INST_LEN)
- != MAX_INST_LEN )
- return 0;
- buf = buffer;
+ inst_len++;
+ if ( inst_len >= fetch_len )
+ {
+ if ( !fetch(v, buf + fetch_len, fetch_addr + fetch_len,
+ MAX_INST_LEN - fetch_len) )
+ return 0;
+ fetch_len = MAX_INST_LEN;
+ }
}
for ( j = 0; j < list_count; j++ )
{
instr = list[j];
opcode = opc_bytes[instr];
- ASSERT(opcode);
- while ( (inst_len < MAX_INST_LEN) &&
- is_prefix(buf[inst_len]) &&
- !is_prefix(opcode[1]) )
- inst_len++;
-
- ASSERT(opcode[0] <= 15); /* Make sure the table is correct. */
- found = 1;
-
- for ( i = 0; i < opcode[0]; i++ )
+ for ( i = 0; (i < opcode[0]) && ((inst_len + i) < MAX_INST_LEN); i++ )
{
- /* If the last byte is zero, we just accept it without checking */
- if ( (i == (opcode[0]-1)) && (opcode[i+1] == 0) )
- break;
+ if ( (inst_len + i) >= fetch_len )
+ {
+ if ( !fetch(v, buf + fetch_len,
+ fetch_addr + fetch_len,
+ MAX_INST_LEN - fetch_len) )
+ return 0;
+ fetch_len = MAX_INST_LEN;
+ }
if ( buf[inst_len+i] != opcode[i+1] )
- {
- found = 0;
- break;
- }
+ goto mismatch;
}
-
- if ( found )
- goto done;
+ goto done;
+ mismatch: ;
}
- printk("%s: Mismatch between expected and actual instruction bytes: "
- "eip = %lx\n", __func__, (unsigned long)vmcb->rip);
+ gdprintk(XENLOG_WARNING,
+ "%s: Mismatch between expected and actual instruction bytes: "
+ "eip = %lx\n", __func__, (unsigned long)vmcb->rip);
+ hvm_inject_exception(TRAP_gp_fault, 0, 0);
return 0;
done:
inst_len += opcode[0];
ASSERT(inst_len <= MAX_INST_LEN);
- if ( match )
- *match = instr;
return inst_len;
}
diff -r e6f20d5ed5fe -r 01aa7c088e98 xen/arch/x86/hvm/svm/svm.c
--- a/xen/arch/x86/hvm/svm/svm.c Tue May 06 11:05:00 2008 +0100
+++ b/xen/arch/x86/hvm/svm/svm.c Tue May 06 13:32:18 2008 +0100
@@ -84,7 +84,10 @@ static void inline __update_guest_eip(
{
struct vcpu *curr = current;
- if ( unlikely((inst_len == 0) || (inst_len > 15)) )
+ if ( unlikely(inst_len == 0) )
+ return;
+
+ if ( unlikely(inst_len > 15) )
{
gdprintk(XENLOG_ERR, "Bad instruction length %u\n", inst_len);
domain_crash(curr->domain);
@@ -907,8 +910,7 @@ static void svm_vmexit_do_cpuid(struct c
{
unsigned int eax, ebx, ecx, edx, inst_len;
- inst_len = __get_instruction_length(current, INSTR_CPUID, NULL);
- if ( inst_len == 0 )
+ if ( (inst_len = __get_instruction_length(current, INSTR_CPUID)) == 0 )
return;
eax = regs->eax;
@@ -1083,13 +1085,15 @@ static void svm_do_msr_access(struct cpu
if ( vmcb->exitinfo1 == 0 )
{
+ if ( (inst_len = __get_instruction_length(v, INSTR_RDMSR)) == 0 )
+ return;
rc = hvm_msr_read_intercept(regs);
- inst_len = __get_instruction_length(v, INSTR_RDMSR, NULL);
}
else
{
+ if ( (inst_len = __get_instruction_length(v, INSTR_WRMSR)) == 0 )
+ return;
rc = hvm_msr_write_intercept(regs);
- inst_len = __get_instruction_length(v, INSTR_WRMSR, NULL);
}
if ( rc == X86EMUL_OKAY )
@@ -1101,34 +1105,36 @@ static void svm_vmexit_do_hlt(struct vmc
{
unsigned int inst_len;
- inst_len = __get_instruction_length(current, INSTR_HLT, NULL);
+ if ( (inst_len = __get_instruction_length(current, INSTR_HLT)) == 0 )
+ return;
+ __update_guest_eip(regs, inst_len);
+
+ hvm_hlt(regs->eflags);
+}
+
+static void wbinvd_ipi(void *info)
+{
+ wbinvd();
+}
+
+static void svm_wbinvd_intercept(void)
+{
+ if ( !list_empty(&(domain_hvm_iommu(current->domain)->pdev_list)) )
+ on_each_cpu(wbinvd_ipi, NULL, 1, 1);
+}
+
+static void svm_vmexit_do_invalidate_cache(struct cpu_user_regs *regs)
+{
+ enum instruction_index list[] = { INSTR_INVD, INSTR_WBINVD };
+ int inst_len;
+
+ inst_len = __get_instruction_length_from_list(
+ current, list, ARRAY_SIZE(list));
if ( inst_len == 0 )
return;
- __update_guest_eip(regs, inst_len);
-
- hvm_hlt(regs->eflags);
-}
-
-static void wbinvd_ipi(void *info)
-{
- wbinvd();
-}
-
-static void svm_wbinvd_intercept(void)
-{
- if ( !list_empty(&(domain_hvm_iommu(current->domain)->pdev_list)) )
- on_each_cpu(wbinvd_ipi, NULL, 1, 1);
-}
-
-static void svm_vmexit_do_invalidate_cache(struct cpu_user_regs *regs)
-{
- enum instruction_index list[] = { INSTR_INVD, INSTR_WBINVD };
- int inst_len;
svm_wbinvd_intercept();
- inst_len = __get_instruction_length_from_list(
- current, list, ARRAY_SIZE(list), NULL, NULL);
__update_guest_eip(regs, inst_len);
}
@@ -1204,7 +1210,8 @@ asmlinkage void svm_vmexit_handler(struc
if ( !v->domain->debugger_attached )
goto exit_and_crash;
/* AMD Vol2, 15.11: INT3, INTO, BOUND intercepts do not update RIP. */
- inst_len = __get_instruction_length(v, INSTR_INT3, NULL);
+ if ( (inst_len = __get_instruction_length(v, INSTR_INT3)) == 0 )
+ break;
__update_guest_eip(regs, inst_len);
domain_pause_for_debugger();
break;
@@ -1281,8 +1288,7 @@ asmlinkage void svm_vmexit_handler(struc
break;
case VMEXIT_VMMCALL:
- inst_len = __get_instruction_length(v, INSTR_VMCALL, NULL);
- if ( inst_len == 0 )
+ if ( (inst_len = __get_instruction_length(v, INSTR_VMCALL)) == 0 )
break;
HVMTRACE_1D(VMMCALL, v, regs->eax);
rc = hvm_do_hypercall(regs);
diff -r e6f20d5ed5fe -r 01aa7c088e98 xen/include/asm-x86/hvm/svm/emulate.h
--- a/xen/include/asm-x86/hvm/svm/emulate.h Tue May 06 11:05:00 2008 +0100
+++ b/xen/include/asm-x86/hvm/svm/emulate.h Tue May 06 13:32:18 2008 +0100
@@ -34,15 +34,12 @@ enum instruction_index {
};
int __get_instruction_length_from_list(
- struct vcpu *v,
- enum instruction_index *list, unsigned int list_count,
- u8 *guest_eip_buf, enum instruction_index *match);
+ struct vcpu *v, enum instruction_index *list, unsigned int list_count);
-static inline int __get_instruction_length(struct vcpu *v,
- enum instruction_index instr, u8 *guest_eip_buf)
+static inline int __get_instruction_length(
+ struct vcpu *v, enum instruction_index instr)
{
- return __get_instruction_length_from_list(
- v, &instr, 1, guest_eip_buf, NULL);
+ return __get_instruction_length_from_list(v, &instr, 1);
}
#endif /* __ASM_X86_HVM_SVM_EMULATE_H__ */
_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog
|
Home