On Thu, 16 Sept 2021 at 15:04, Alexander Graf <ag...@csgraf.de> wrote: > > > On 16.09.21 13:59, Peter Maydell wrote: > > On Wed, 15 Sept 2021 at 19:10, Alexander Graf <ag...@csgraf.de> wrote: > >> Hvf's permission bitmap during and after dirty logging does not include > >> the HV_MEMORY_EXEC permission. At least on Apple Silicon, this leads to > >> instruction faults once dirty logging was enabled. > >> > >> Add the bit to make it work properly. > >> > >> Signed-off-by: Alexander Graf <ag...@csgraf.de> > >> --- > >> accel/hvf/hvf-accel-ops.c | 4 ++-- > >> 1 file changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/accel/hvf/hvf-accel-ops.c b/accel/hvf/hvf-accel-ops.c > >> index d1691be989..71cc2fa70f 100644 > >> --- a/accel/hvf/hvf-accel-ops.c > >> +++ b/accel/hvf/hvf-accel-ops.c > >> @@ -239,12 +239,12 @@ static void > >> hvf_set_dirty_tracking(MemoryRegionSection *section, bool on) > >> if (on) { > >> slot->flags |= HVF_SLOT_LOG; > >> hv_vm_protect((uintptr_t)slot->start, (size_t)slot->size, > >> - HV_MEMORY_READ); > >> + HV_MEMORY_READ | HV_MEMORY_EXEC); > >> /* stop tracking region*/ > >> } else { > >> slot->flags &= ~HVF_SLOT_LOG; > >> hv_vm_protect((uintptr_t)slot->start, (size_t)slot->size, > >> - HV_MEMORY_READ | HV_MEMORY_WRITE); > >> + HV_MEMORY_READ | HV_MEMORY_WRITE | HV_MEMORY_EXEC); > >> } > >> } > > Makes sense -- this matches the premissions we set initially > > for memory regions in hvf_set_phys_mem(). > > > > Reviewed-by: Peter Maydell <peter.mayd...@linaro.org> > > > > Should we change also the hv_vm_protect() call in > > target/i386/hvf/hvf.c:ept_emulation_fault(), for consistency ? > > > The x86 hvf code seems to deal just fine with !X mappings, so I'd leave > it as is as part of the arm enablement series - especially because I > have limited testing capabilities for x86 hvf.
Yeah, I should have been clearer -- that would be best as a separate x86-specific patch. -- PMM