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/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
