On Mon, Nov 26, 2018 at 05:43:59PM +0000, Peter Maydell wrote: > On Mon, 26 Nov 2018 at 00:24, Steffen Görtz <m...@steffen-goertz.de> wrote: > > > > Hi Peter, > > > > thank you for your remarks! > > > > >> +}; > > >> + > > >> +static uint64_t ficr_read(void *opaque, hwaddr offset > > > > > >> + value &= ~(NRF51_PAGE_SIZE - 1); > > >> + if (value < (s->flash_size - NRF51_PAGE_SIZE)) { > > >> + memset(s->storage + value / 4, 0xFF, NRF51_PAGE_SIZE); > > > > > > Can the guest try to execute from the flash storage? If so > > > then just updating the backing storage directly like this is > > > not sufficient to ensure that QEMU discards any now-stale > > > translated code blocks from the affected memory. > > > > What else is necessary to invalidate stale blocks? > > You need an AddressSpace that points to the MemoryRegion(s) > you're altering, and you need to use the operations on > the AddressSpace like address_space_write(). These will > under the hood do the right thing with TB invalidation.
I'm not sure about this. The memory region looks like this: {parent_obj = {class = 0x5555565ee350, free = 0x0, Python Exception <class 'gdb.error'> There is no member named keys.: properties = 0x55555672f860, ref = 1, parent = 0x5555566620f0}, romd_mode = true, ram = false, subpage = false, readonly = false, nonvolatile = false, rom_device = true, flush_coalesced_mmio = false, global_locking = true, dirty_log_mask = 0 '\000', is_iommu = false, ram_block = 0x555556768b40, owner = 0x5555566620f0, ops = 0x55555615d360 <flash_ops>, opaque = 0x5555566620f0, container = 0x0, size = 262144, addr = 0, destructor = 0x555555893f00 <memory_region_destructor_ram>, align = 2097152, terminates = true, ram_device = false, enabled = true, warning_printed = false, vga_logging_count = 0 '\000', alias = 0x0, alias_offset = 0, priority = 0, subregions = { tqh_first = 0x0, tqh_last = 0x555556662778}, subregions_link = {tqe_next = 0x0, tqe_prev = 0x0}, coalesced = {tqh_first = 0x0, tqh_last = 0x555556662798}, name = 0x5555568033d0 "nrf51_soc.flash", ioeventfd_nb = 0, ioeventfds = 0x0} I see nothing that invalidates TBs in the address_space_write() code for MMIO memory regions (not RAM). Only the RAM case calls invalidate_and_set_dirty(). There are a few complications with this device: 1. Stores from the CPU are only honored when the NRF51_NVMC_CONFIG_WEN write enable bit is set. NRF51_NVMC_ERASEPCRx and NRF51_NVMC_ERASEALL commands use a separate erase enable bit (NRF51_NVMC_CONFIG_EEN) and are therefore different from normal writes. 2. Stores from the CPU can only flip 1s to 0s (this is NOR flash). When we erase a page of flash memory it must be set to 0xff (i.e. flip 0s to 1s). 3. nrf51_nvm.c:flash_write() does not mark the page dirty for live migration. My questions: 1. Is the current rom+mmio device approach okay or should it be modelled differently? 2. Erase operations cannot use ordinary address_space_write() for the reasons mentioned above. Should this device directly call cpu_physical_memory_set_dirty_range() and tb_invalidate_phys_range()?
signature.asc
Description: PGP signature