[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 05/14] hw/nvram/nrf51_nvm: Add nRF51 non-vola
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH v5 05/14] hw/nvram/nrf51_nvm: Add nRF51 non-volatile memories |
Date: |
Sun, 16 Dec 2018 06:20:24 +0000 |
User-agent: |
Mutt/1.10.1 (2018-07-13) |
On Mon, Nov 26, 2018 at 05:43:59PM +0000, Peter Maydell wrote:
> On Mon, 26 Nov 2018 at 00:24, Steffen Görtz <address@hidden> 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
- Re: [Qemu-devel] [PATCH v5 05/14] hw/nvram/nrf51_nvm: Add nRF51 non-volatile memories,
Stefan Hajnoczi <=