Re: [BUG?] can not writeback more then 8 bytes memory
On 08/20/2010 12:12 PM, Wei Yongjun wrote: Hi all: I have the following patch to SIDT emulation instruction, but it does not work because we can not writeback more then 8 bytes memory, the SIDT under PROTO64 is 10 bytes. I change the code to write twice, first time write limit, then write address, and it does not work too. If I change the c-dst.bytes to only writeback 8 bytes, it will writeback 8 bytes. Is this a bug of KVM or it is the IO limit? It's complicated... if you write to real memory, then you can do c-dst.* = ...; writeback(); c-dst.* = ...; writeback(); However that will not work for mmio, since writeback() only schedules mmio writes which are retired later on. As a temporary fix we can extend writeback() to support 2+N byte writes (and fail them on mmio). See also the sse-mmio branch on kvm.git on how to support 8 byte mmio. Longer term I'd like a queue based approach so we can have any number of reads and writes for an instruction, and have multiple writes possible for mmio. It would look something like this: - every read or write is assigned a position in the queue starting from 0 - the queue is maintained across invocations of x86_emulate_insn() - if an operation position is already in the queue, we read the value from the queue, and ignore writes (e.g. a replay after exit to userspace) - if an operation position is not already in the queue, we add it. If we cannot satisfy it, we exit to userspace - need to check read-after-write for collision in the queue. There's some beginning of that already (see struct read_cache). For now we can live without SIDT to mmio, that means you can do the simple fix to writeback() above. But you will only be able to test is with realmode.flat, not with emulator.flat since that requires mmio. If test with realmode.flat, fix to writeback() does not need, because the size we need to writeback is 2 + 4 in realmode.flat. I tested used the following patch. diff --git a/x86/realmode.c b/x86/realmode.c index 8c771fc..26651a6 100644 --- a/x86/realmode.c +++ b/x86/realmode.c @@ -1242,6 +1242,25 @@ void test_lds_lss() outregs.ebx == desc.sel); } + +static struct { + u16 limit; + unsigned long base; +} __attribute__((packed)) idt_descr = { + 0x1234, + 0x5678, +}; + +void test_sidt() +{ + MK_INSN(sidt, sidt (%eax)\n\t); + + inregs = (struct regs){ .eax = (unsigned long)idt_descr }; + + exec_in_big_real_mode(insn_sidt); + report(sidt, 0, idt_descr.limit == 1023 idt_descr.base == 0); +} + void realmode_start(void) { test_null(); @@ -1274,6 +1293,7 @@ void realmode_start(void) test_cwd_cdq(); test_das(); test_lds_lss(); + test_sidt(); exit(0); } -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG?] can not writeback more then 8 bytes memory
On 08/23/2010 11:55 AM, Wei Yongjun wrote: If test with realmode.flat, fix to writeback() does not need, because the size we need to writeback is 2 + 4 in realmode.flat. I tested used the following patch. It will fail in 64-bit mode though. We can goto cannot_emulate, though it's a bit nasty. -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG?] can not writeback more then 8 bytes memory
On 08/20/2010 12:12 PM, Wei Yongjun wrote: Hi all: I have the following patch to SIDT emulation instruction, but it does not work because we can not writeback more then 8 bytes memory, the SIDT under PROTO64 is 10 bytes. I change the code to write twice, first time write limit, then write address, and it does not work too. If I change the c-dst.bytes to only writeback 8 bytes, it will writeback 8 bytes. Is this a bug of KVM or it is the IO limit? It's complicated... if you write to real memory, then you can do c-dst.* = ...; writeback(); c-dst.* = ...; writeback(); However that will not work for mmio, since writeback() only schedules mmio writes which are retired later on. As a temporary fix we can extend writeback() to support 2+N byte writes (and fail them on mmio). See also the sse-mmio branch on kvm.git on how to support 8 byte mmio. Longer term I'd like a queue based approach so we can have any number of reads and writes for an instruction, and have multiple writes possible for mmio. It would look something like this: - every read or write is assigned a position in the queue starting from 0 - the queue is maintained across invocations of x86_emulate_insn() - if an operation position is already in the queue, we read the value from the queue, and ignore writes (e.g. a replay after exit to userspace) - if an operation position is not already in the queue, we add it. If we cannot satisfy it, we exit to userspace - need to check read-after-write for collision in the queue. There's some beginning of that already (see struct read_cache). For now we can live without SIDT to mmio, that means you can do the simple fix to writeback() above. But you will only be able to test is with realmode.flat, not with emulator.flat since that requires mmio. -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[BUG?] can not writeback more then 8 bytes memory
Hi all: I have the following patch to SIDT emulation instruction, but it does not work because we can not writeback more then 8 bytes memory, the SIDT under PROTO64 is 10 bytes. I change the code to write twice, first time write limit, then write address, and it does not work too. If I change the c-dst.bytes to only writeback 8 bytes, it will writeback 8 bytes. Is this a bug of KVM or it is the IO limit? diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c index dc3a648..43a6819 100644 --- a/arch/x86/kvm/emulate.c +++ b/arch/x86/kvm/emulate.c @@ -2213,7 +2213,8 @@ static struct opcode group5[] = { }; static struct group_dual group7 = { { - N, N, D(ModRM | SrcMem | Priv), D(ModRM | SrcMem | Priv), + N, D(DstMem | SrcNone | ModRM | Op3264), + D(ModRM | SrcMem | Priv), D(ModRM | SrcMem | Priv), D(SrcNone | ModRM | DstMem | Mov), N, D(SrcMem16 | ModRM | Mov | Priv), D(SrcMem | ModRM | ByteOp | Priv | NoAccess), @@ -3282,6 +3283,7 @@ twobyte_insn: switch (c-modrm_reg) { u16 size; unsigned long address; + struct desc_ptr dt; case 0: /* vmcall */ if (c-modrm_mod != 3 || c-modrm_rm != 1) @@ -3296,6 +3298,11 @@ twobyte_insn: /* Disable writeback. */ c-dst.type = OP_NONE; break; + case 1: /* sidt */ + ops-get_idt(dt, ctxt-vcpu); + c-dst.bytes = c-op_bytes + 2; + memcpy(c-dst.valptr, dt, c-dst.bytes); + break; case 2: /* lgdt */ rc = read_descriptor(ctxt, ops, c-src.addr.mem, size, address, c-op_bytes); -- 1.7.0.4 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html