| Message ID | 20231021200518.30125-5-phil@philjordan.eu (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hvf x86 correctness and efficiency improvements part 1 | expand |
On Sat, Oct 21, 2023 at 10:05:18PM +0200, Phil Dennis-Jordan wrote: > When using x86 macOS Hypervisor.framework as accelerator, detection of > dirty memory regions is implemented by marking logged memory region > slots as read-only in the EPT, then setting the dirty flag when a > guest write causes a fault. The area marked dirty should then be marked > writable in order for subsequent writes to succeed without a VM exit. > > However, dirty bits are tracked on a per-page basis, whereas the fault > handler was marking the whole logged memory region as writable. This > change fixes the fault handler so only the protection of the single > faulting page is marked as dirty. > > (Note: the dirty page tracking appeared to work despite this error > because HVF’s hv_vcpu_run() function generated unnecessary EPT fault > exits, which ended up causing the dirty marking handler to run even > when the memory region had been marked RW. When using > hv_vcpu_run_until(), a change planned for a subsequent commit, these > spurious exits no longer occur, so dirty memory tracking malfunctions.) > > Additionally, the dirty page is set to permit code execution, the same > as all other guest memory; changing memory protection from RX to RW not > RWX appears to have been an oversight. > Hi Phil, I don't observe a problem with SVGA if I apply CPU kick patch on top of it. Thanks for fixing this, Reviewed-by: Roman Bolshakov <roman@roolebo.dev> Tested-by: Roman Bolshakov <roman@roolebo.dev> Regards, Roman
diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c index 43d64574ad..a15ee469c3 100644 --- a/target/i386/hvf/hvf.c +++ b/target/i386/hvf/hvf.c @@ -135,9 +135,10 @@ static bool ept_emulation_fault(hvf_slot *slot, uint64_t gpa, uint64_t ept_qual) if (write && slot) { if (slot->flags & HVF_SLOT_LOG) { + uint64_t dirty_page_start = gpa & ~(TARGET_PAGE_SIZE - 1u); memory_region_set_dirty(slot->region, gpa - slot->start, 1); - hv_vm_protect((hv_gpaddr_t)slot->start, (size_t)slot->size, - HV_MEMORY_READ | HV_MEMORY_WRITE); + hv_vm_protect(dirty_page_start, TARGET_PAGE_SIZE, + HV_MEMORY_READ | HV_MEMORY_WRITE | HV_MEMORY_EXEC); } }
When using x86 macOS Hypervisor.framework as accelerator, detection of dirty memory regions is implemented by marking logged memory region slots as read-only in the EPT, then setting the dirty flag when a guest write causes a fault. The area marked dirty should then be marked writable in order for subsequent writes to succeed without a VM exit. However, dirty bits are tracked on a per-page basis, whereas the fault handler was marking the whole logged memory region as writable. This change fixes the fault handler so only the protection of the single faulting page is marked as dirty. (Note: the dirty page tracking appeared to work despite this error because HVF’s hv_vcpu_run() function generated unnecessary EPT fault exits, which ended up causing the dirty marking handler to run even when the memory region had been marked RW. When using hv_vcpu_run_until(), a change planned for a subsequent commit, these spurious exits no longer occur, so dirty memory tracking malfunctions.) Additionally, the dirty page is set to permit code execution, the same as all other guest memory; changing memory protection from RX to RW not RWX appears to have been an oversight. Signed-off-by: Phil Dennis-Jordan <phil@philjordan.eu> --- target/i386/hvf/hvf.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)