> On Oct 27, 2014, at 16:37, Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote: > > > > On 09/30/2014 07:49 PM, Nadav Amit wrote: >> GP and SS exceptions deliver as error-code the segment selector if the >> exception occurred when the segment is loaded. However, if the exception >> occurs during the memory access itself, due to limit violations, the error-code >> should be zero. Fix it. >> >> Signed-off-by: Nadav Amit <namit@xxxxxxxxxxxxxxxxx> >> --- >> arch/x86/kvm/emulate.c | 20 +++++++++++--------- >> 1 file changed, 11 insertions(+), 9 deletions(-) >> >> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c >> index a46207a..13a1c76 100644 >> --- a/arch/x86/kvm/emulate.c >> +++ b/arch/x86/kvm/emulate.c >> @@ -621,7 +621,7 @@ static int __linearize(struct x86_emulate_ctxt *ctxt, >> bool usable; >> ulong la; >> u32 lim; >> - u16 sel; >> + u16 sel, error_code = 0; >> unsigned cpl; >> >> la = seg_base(ctxt, addr.seg) + addr.ea; >> @@ -634,14 +634,14 @@ static int __linearize(struct x86_emulate_ctxt *ctxt, >> usable = ctxt->ops->get_segment(ctxt, &sel, &desc, NULL, >> addr.seg); >> if (!usable) >> - goto bad; >> + goto bad_sel; > > This can only happen because of a NULL selector, which means the error > code is zero anyway. > >> /* code segment in protected mode or read-only data segment */ >> if ((((ctxt->mode != X86EMUL_MODE_REAL) && (desc.type & 8)) >> || !(desc.type & 2)) && write) >> - goto bad; >> + goto bad_sel; > > This is not "detected while loading a segment descriptor", so the error > code should be zero. > >> /* unreadable code segment */ >> if (!fetch && (desc.type & 8) && !(desc.type & 2)) >> - goto bad; >> + goto bad_sel; > > Same here. > >> lim = desc_limit_scaled(&desc); >> if ((ctxt->mode == X86EMUL_MODE_REAL) && !fetch && >> (ctxt->d & NoBigReal)) { >> @@ -664,15 +664,15 @@ static int __linearize(struct x86_emulate_ctxt *ctxt, >> if (!(desc.type & 8)) { >> /* data segment */ >> if (cpl > desc.dpl) >> - goto bad; >> + goto bad_sel; >> } else if ((desc.type & 8) && !(desc.type & 4)) { >> /* nonconforming code segment */ >> if (cpl != desc.dpl) >> - goto bad; >> + goto bad_sel; >> } else if ((desc.type & 8) && (desc.type & 4)) { >> /* conforming code segment */ >> if (cpl < desc.dpl) >> - goto bad; >> + goto bad_sel; > > These three should be deleted, as you pointed out in patch 5. > > So I've dropped this patch, and posted a simpler alternative that just > uses error code 0 in __linearize. Can you look at it? It looks fine. I noticed these mistakes also... Please wait with the other emulator fixes I posted. I run some further tests, since I am annoyed of making mistakes and redoing patches. Regards, Nadav -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html