Luca Tettamanti wrote: > Il Fri, Jun 15, 2007 at 12:06:50PM +0300, Avi Kivity ha scritto: > >>> After a bit of thinking: it's correct but removes an optimization; >>> furthermore it may miss other instructions that write to memory mapped >>> areas. >>> A more proper fix should be "force the writeback if dst.ptr is in some >>> kind of mmio area". >>> >>> >> I think we can just disable the optimization, and (in a separate patch) >> add it back in emulator_write_phys(), where we know we're modifying >> memory and not hitting mmio. >> > > Problem is that in emulator_write_phys() we've already lost track of the > previous (dst.orig_val) value. I moved up the decision in >
Actually we haven't; just before the memcpy(), we can put a memcmp() to guard the kvm_mmu_pte_write(), which is the really expensive operation, especially with guest smp. > cmpxchg_emulated; unfortunately this means that the non-locked write > path (write_emulated) can't do this optimization, unless I change its > signature to include the old value. > > The first patch makes the writeback step uncoditional whenever we have a > destination operand (the mov check (d & Mov) may be superfluous, yes?). > The write-to-registry path still has the optimization that skips the > write if possible. > The mov check is in done since the destination is not read for moves; so the check for change would read uninitialized memory. I think we can simply remove the if (). For the register case, the check is more expensive that the write; for mmio, we don't want it; and for memory writes, we can put it in emulator_write_phys(). > Next one: I've splitted emulator_write_phys into emulator_write_phys_mem > (for normal RAM) and emulator_write_phys_mmio (for the rest). The > This is in order to properly emulate cmpxchg(), right? I don't think it's necessary since all that memory is write protected and the modifications happen under kvm->lock. > > I'm a bit confused about this test, found in emulator_write_phys > (original code): > > if (((gpa + bytes - 1) >> PAGE_SHIFT) != (gpa >> PAGE_SHIFT)) > return 0; > > AFAICT is makes the emulator skip the write if the modified area spans > across two different (physical) pages. When this happens write_emulated > does a MMIO write. I'd expect the function to load the 2 pages and do the > memory write on both instead. > This isn't skipping the write; instead it uses the mmio path to update memory instead of the direct path, as I was too lazy to code split page writes. It's also wrong in case someone uses nonaligned writes to update page tables, but no-one does that. -- Do not meddle in the internals of kernels, for they are subtle and quick to panic. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
