On 2026-10-07 16:50, Akihiko Odaki wrote:
On 2026/10/07 18:30, Philippe Mathieu-Daudé wrote:
From: Mark Burton <[email protected]>
Dirty-page logging handles a RAM write fault by marking the page dirty,
restoring write permission, and retrying the guest instruction.
The handler also retried when the resolved RAM region had no dirty-log
client. This path neither restores permission nor advances the PC, so
the instruction can repeatedly exit to QEMU.
Retry only after marking the page dirty and restoring write permission.
Otherwise, use the existing emulation path to complete the access and
advance the guest PC.
I can identify three cases of RAM write faults. The existing code
handles all three, but this patch breaks the third:
1. The RAM is read-only. assert(!mr->readonly) trips in this case.
2. Dirty logging is enabled. The vCPU marks the page dirty, restores
write permission, and retries the write.
3. Dirty logging is enabled when the fault occurs, but another thread
disables it before the vCPU acquires the BQL. The vCPU then observes
a zero dirty-log mask. The write still needs to be retried because
write permission has already been restored. The existing code does
this correctly, but this patch falls through to emulation instead.
This patch is useful only if there is another case that the existing
code does not handle correctly. Please describe that case and preserve
the retry behavior in case 3.
Thanks for the deep review.
The reporter said:
We reproduced is a level-2 translation fault, DFSC 0x06, on a
dynamic DMI RAM leaf that has no corresponding HVF mapping.
address_space_translate() resolves the leaf as RAM, but the dirty-log
mask is zero, no page is marked or unprotected, and the unconditional
retry makes no progress.
That would be another case:
4. FlatView says RAM, but HVF has no mapping (nothing has changed
to make retry succeed).
So to prevent retrying a known translation miss that cannot make any
progress checking DFSC could be sufficient:
-- >8 --
diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
index b00a7e1a84..4faa21caa1 100644
case EC_DATAABORT: {
bool isv = FIELD_EX32(syndrome, DABORT_ISS, ISV);
bool iswrite = FIELD_EX32(syndrome, DABORT_ISS, WNR);
bool s1ptw = FIELD_EX32(syndrome, DABORT_ISS, S1PTW);
bool sse = FIELD_EX32(syndrome, DABORT_ISS, SSE);
uint32_t sas = FIELD_EX32(syndrome, DABORT_ISS, SAS);
uint32_t len = 1 << sas;
uint32_t srt = FIELD_EX32(syndrome, DABORT_ISS, SRT);
uint32_t cm = FIELD_EX32(syndrome, DABORT_ISS, CM);
+ uint32_t fsc = FIELD_EX32(syndrome, DABORT_ISS, DFSC);
+ bool l2_translation_fault = fsc == 0b000110;
uint64_t val = 0;
uint64_t ipa = excp->physical_address;
AddressSpace *as = cpu_get_address_space(cpu, ARMASIdx_NS);
trace_hvf_data_abort(excp->virtual_address, ipa, isv,
iswrite, s1ptw, len, srt);
if (cm) {
/* We don't cache MMIO regions */
advance_pc = true;
break;
}
/* Handle dirty page logging for ram. */
if (iswrite) {
hwaddr xlat;
MemoryRegion *mr = address_space_translate(as, ipa, &xlat,
NULL, true,
MEMTXATTRS_UNSPECIFIED);
- if (memory_region_is_ram(mr)) {
+ if (memory_region_is_ram(mr) && !s1ptw &&
!l2_translation_fault) {
uintptr_t page_size = qemu_real_host_page_size();
intptr_t page_mask = -(intptr_t)page_size;
uint64_t ipa_page = ipa & page_mask;
/* TODO: Inject exception to the guest. */
assert(!mr->readonly);
if (memory_region_get_dirty_log_mask(mr)) {
memory_region_set_dirty(mr, xlat, page_size);
hvf_unprotect_dirty_range(ipa_page, page_size);
}
/* Retry with page writes enabled. */
break;
}
}
/*
* TODO: If s1ptw, this is an error in the guest os page tables.
* Inject the exception into the guest.
*/
assert(!s1ptw);
---
(the !s1ptw guard is independent of the zero-mask race,
this is to ensure we cannot accidentally take the dirty-log retry
path when the page-table memory is RAM).
But be wary because I'm way out of area of understanding here...
Anyway I asked the reporter for more info and possible reproducer;
and might add comments to the code to better follow the 1/2/3/4 paths.
But after some sleep :)
