From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wei Yongjun Subject: Re: [BUG?] can not writeback more then 8 bytes memory Date: Mon, 23 Aug 2010 16:55:29 +0800 Message-ID: <4C723781.9090500@cn.fujitsu.com> References: <4C6E46E0.4080409@cn.fujitsu.com> <4C70F75A.104@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=GB2312 Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org To: Avi Kivity Return-path: Received: from cn.fujitsu.com ([222.73.24.84]:64850 "EHLO song.cn.fujitsu.com" rhost-flags-OK-FAIL-OK-OK) by vger.kernel.org with ESMTP id S1753457Ab0HWI5K (ORCPT ); Mon, 23 Aug 2010 04:57:10 -0400 In-Reply-To: <4C70F75A.104@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: > 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); }