Because the clamping was done against the MemoryRegion,
address_space_rw was effectively broken if a write spanned
multiple sections that are not linear in underlying memory
(with the memory not being under an IOMMU).

This is visible with the MIPS rc4030 IOMMU, which is implemented
as a series of alias memory regions that point to the actual RAM.
Tested-by: NHervé Poussineau <hpoussin@reactos.org>
Tested-by: NMark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

It is common for MMIO registers to overlap, for example a 4 byte register
at 0xcf8 (totally random choice... :)) and a 1 byte register at 0xcf9.
If these registers are implemented via separate MemoryRegions, it is
wrong to clamp the accesses as the value written would be truncated.

Hence for these regions the effects of commit 23820dbf (exec: Respect
as_translate_internal length clamp, 2015-03-16, previously applied as
commit c3c1bb99) must be skipped.
Tested-by: NHervé Poussineau <hpoussin@reactos.org>
Tested-by: NMark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

check the return value of the function it calls and error if it's non-0
Fixup qemu_rdma_init_one_block that is the only current caller,
  and rdma_add_block the only function it calls using it.

Pass the name of the ramblock to the function; helps in debugging.
Signed-off-by: NDr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: NDavid Gibson <david@gibson.dropbear.id.au>
Reviewed-by: NAmit Shah <amit.shah@redhat.com>
Reviewed-by: NMichael R. Hines <mrhines@us.ibm.com>
Reviewed-by: NJuan Quintela <quintela@redhat.com>
Signed-off-by: NJuan Quintela <quintela@redhat.com>

We create optional sections with this patch.  But we already have
optional subsections.  Instead of having two mechanism that do the
same, we can just generalize it.

For subsections we just change:

- Add a needed function to VMStateDescription
- Remove VMStateSubsection (after removal of the needed function
  it is just a VMStateDescription)
- Adjust the whole tree, moving the needed function to the corresponding
  VMStateDescription
Signed-off-by: NJuan Quintela <quintela@redhat.com>

The cpu_physical_memory_reset_dirty() function is sometimes used
together with cpu_physical_memory_get_dirty().  This is not atomic since
two separate accesses to the dirty memory bitmap are made.

Turn cpu_physical_memory_reset_dirty() and
cpu_physical_memory_clear_dirty_range_type() into the atomic
cpu_physical_memory_test_and_clear_dirty().
Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>
Message-Id: <1417519399-3166-6-git-send-email-stefanha@redhat.com>
Reviewed-by: NFam Zheng <famz@redhat.com>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

Most of the time, not all bitmaps have to be marked as dirty;
do not do anything if the interesting ones are already dirty.
Previously, any clean bitmap would have cause all the bitmaps to be
marked dirty.

In fact, unless running TCG most of the time bitmap operations need
not be done at all, because memory_region_is_logging returns zero.
In this case, skip the call to cpu_physical_memory_range_includes_clean
altogether as well.

With this patch, cpu_physical_memory_set_dirty_range is called
unconditionally, so there need not be anymore a separate call to
xen_modified_memory.
Reviewed-by: NFam Zheng <famz@redhat.com>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

This cuts in half the cost of bitmap operations (which will become more
expensive when made atomic) during migration on non-VRAM regions.
Reviewed-by: NFam Zheng <famz@redhat.com>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

The is_cpu_write_access argument is always 0, remove it.
Reviewed-by: NFam Zheng <famz@redhat.com>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

The memory API can now return the exact set of bitmaps that have to
be tracked.  Use it instead of the in_migration variable.

In the next patches, we will also use it to set only DIRTY_MEMORY_VGA
or DIRTY_MEMORY_MIGRATION if necessary.  This can make a difference
for dataplane, especially after the dirty bitmap is changed to use
more expensive atomic operations.

Of some interest is the change to stl_phys_notdirty.  When migration
was introduced, stl_phys_notdirty was changed to effectively behave
as stl_phys during migration.  In fact, if one looks at the function as it
was in the beginning (commit 8df1cd07, physical memory access functions,
2005-01-28), at the time the dirty bitmap was the equivalent of
DIRTY_MEMORY_CODE nowadays; hence, the function simply should not touch
the dirty code bits.  This patch changes it to do the intended thing.
Reviewed-by: NFam Zheng <famz@redhat.com>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

Invoke xen_modified_memory from cpu_physical_memory_set_dirty_range_nocode;
it is akin to DIRTY_MEMORY_MIGRATION, so set it together with that bitmap.
The remaining call from invalidate_and_set_dirty's "else" branch will go
away soon.

Second, fix the second argument to the function in the
cpu_physical_memory_set_dirty_lebitmap call site.  That function is only used
by KVM, but it is better to be clean anyway.
Acked-by: NStefano Stabellini <stefano.stabellini@eu.citrix.com>
Reviewed-by: NFam Zheng <famz@redhat.com>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

phys_page_set_level is writing zeroes to a struct that has just been
filled in by phys_map_node_alloc.  Instead, tell phys_map_node_alloc
whether to fill in the page "as a leaf" or "as a non-leaf".

memcpy is faster than struct assignment, which copies each bitfield
individually.  A compiler bug (https://gcc.gnu.org/PR66391), and
small memcpys like this one are special-cased anyway, and optimized
to a register move, so just use the memcpy.

This cuts the cost of phys_page_set_level from 25% to 5% when
booting qboot.
Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

Once address_space_translate will be called outside the BQL, the returned
MemoryRegion might disappear as soon as the RCU read-side critical section
ends.  Avoid this by moving the critical section to the callers.
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
Message-Id: <1426684909-95030-3-git-send-email-pbonzini@redhat.com>

address_space_translate_internal will clamp the *plen length argument
based on the size of the memory region being queried. The iommu walker
logic in addresss_space_translate was ignoring this by discarding the
post fn call value of *plen. Fix by just always using *plen as the
length argument throughout the fn, removing the len local variable.

This fixes a bootloader bug when a single elf section spans multiple
QEMU memory regions.
Signed-off-by: NPeter Crosthwaite <peter.crosthwaite@xilinx.com>
Message-Id: <1426570554-15940-1-git-send-email-peter.crosthwaite@xilinx.com>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

If DMA's owning thread cancels the IO while the bounce buffer's owning thread
is notifying the "cpu client list", a use-after-free happens:

     continue_after_map_failure               dma_aio_cancel
     ------------------------------------------------------------------
     aio_bh_new
                                              qemu_bh_delete
     qemu_bh_schedule (use after free)

Also, the old code doesn't run the bh in the right AioContext.

Fix both problems by passing a QEMUBH to cpu_register_map_client.
Signed-off-by: NFam Zheng <famz@redhat.com>
Reviewed-by: NPaolo Bonzini <pbonzini@redhat.com>
Message-Id: <1426496617-10702-6-git-send-email-famz@redhat.com>
[Remove unnecessary forward declaration. - Paolo]
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

The caller's workflow is like

    if (!address_space_map()) {
        ...
        cpu_register_map_client();
    }

If bounce buffer became available after address_space_map() but before
cpu_register_map_client(), the caller could miss it and has to wait for the
next bounce buffer notify, which may never happen in the worse case.

Just notify the list in cpu_register_map_client().
Signed-off-by: NFam Zheng <famz@redhat.com>
Message-Id: <1426496617-10702-5-git-send-email-famz@redhat.com>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

So that accesses from multiple threads are safe.
Signed-off-by: NFam Zheng <famz@redhat.com>
Message-Id: <1426496617-10702-4-git-send-email-famz@redhat.com>
[Remove #if from cpu_exec_init_all. - Paolo]
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

There could be a race condition when two processes call
address_space_map concurrently and both want to use the bounce buffer.

Add an in_use flag in BounceBuffer to sync it.
Signed-off-by: NFam Zheng <famz@redhat.com>
Message-Id: <1426496617-10702-2-git-send-email-famz@redhat.com>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

Capture the memory attributes for the transaction which triggered
a watchpoint; this allows CPU specific code to implement features
like ARM's "user-mode only WPs also hit for LDRT/STRT accesses
made from privileged code". This change also correctly passes
through the memory attributes to the underlying device when
a watchpoint access doesn't hit.
Signed-off-by: NPeter Maydell <peter.maydell@linaro.org>
Reviewed-by: NAlex Bennée <alex.bennee@linaro.org>

Add new address_space_ld*/st* functions which allow transaction
attributes and error reporting for basic load and stores. These
are named to be in line with the address_space_read/write/rw
buffer operations.

The existing ld/st*_phys functions are now wrappers around
the new functions.
Signed-off-by: NPeter Maydell <peter.maydell@linaro.org>
Reviewed-by: NEdgar E. Iglesias <edgar.iglesias@xilinx.com>
Reviewed-by: NAlex Bennée <alex.bennee@linaro.org>

Make address_space_rw take transaction attributes, rather
than always using the 'unspecified' attributes.
Signed-off-by: NPeter Maydell <peter.maydell@linaro.org>
Reviewed-by: NPaolo Bonzini <pbonzini@redhat.com>
Reviewed-by: NEdgar E. Iglesias <edgar.iglesias@xilinx.com>
Reviewed-by: NAlex Bennée <alex.bennee@linaro.org>

Convert the subpage memory ops to _with_attrs; this will allow
us to pass the attributes through to the underlying access
functions. (Nothing uses the attributes yet.)
Signed-off-by: NPeter Maydell <peter.maydell@linaro.org>
Reviewed-by: NPaolo Bonzini <pbonzini@redhat.com>
Reviewed-by: NEdgar E. Iglesias <edgar.iglesias@xilinx.com>

Rather than retaining io_mem_read/write as simple wrappers around
the memory_region_dispatch_read/write functions, make the latter
public and change all the callers to use them, since we need to
touch all the callsites anyway to add MemTxAttrs and MemTxResult
support. Delete io_mem_read and io_mem_write entirely.

(All the callers currently pass MEMTXATTRS_UNSPECIFIED
and convert the return value back to bool or ignore it.)
Signed-off-by: NPeter Maydell <peter.maydell@linaro.org>
Reviewed-by: NAlex Bennée <alex.bennee@linaro.org>

This reverts commit c3c1bb99.
It causes problems with boards that declare memory regions shorter
than the registers they contain.
Reported-by: NZoltan Balaton <balaton@eik.bme.hu>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

It is not necessary to munmap an area before remapping it with MAP_FIXED;
if the memory region specified by addr and len overlaps pages of any
existing mapping, then the overlapped part of the existing mapping will
be discarded.

On the other hand, if QEMU does munmap the pages, there is a small
probability that another mmap sneaks in and catches the just-freed
portion of the address space.  In effect, munmap followed by
mmap(MAP_FIXED) is a use-after-free error, and Coverity flags it
as such.  Fix it.
Reviewed-by: NGonglei <arei.gonglei@huawei.com>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

address_space_translate_internal will clamp the *plen length argument
based on the size of the memory region being queried. The iommu walker
logic in addresss_space_translate was ignoring this by discarding the
post fn call value of *plen. Fix by just always using *plen as the
length argument throughout the fn, removing the len local variable.

This fixes a bootloader bug when a single elf section spans multiple
QEMU memory regions.
Signed-off-by: NPeter Crosthwaite <peter.crosthwaite@xilinx.com>
Message-Id: <1426570554-15940-1-git-send-email-peter.crosthwaite@xilinx.com>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

As noted by Andreas, hw/boards.h shouldn't be used outside softmmu code.
Include it conditionally, and drop the (now unnecessary) ifdef guards in
hw/boards.h
Reported-by: NAndreas Färber <afaerber@suse.de>
Cc: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: NMichael S. Tsirkin <mst@redhat.com>
Reviewed-by: NAndreas Färber <afaerber@suse.de>

Running
    qemu-bin ... -machine pc,mem-merge=on
leads to crash:
    x86_64-softmmu/qemu-system-x86_64 -machine pc,dump-guest-core=on
    qemu-system-x86_64: qemu/util/qemu-option.c:387: qemu_opt_get_bool_helper:
    Assertion `opt->desc && opt->desc->type == QEMU_OPT_BOOL' failed.  Aborted
    (core dumped)

This happens because the commit e79d5a6 ("machine: remove qemu_machine_opts
global list") removed the global option descriptions and moved them to
MachineState's QOM properties.

Fix this by querying machine properties through designated wrappers.
Signed-off-by: NMarcel Apfelbaum <marcel@redhat.com>
Acked-by: NMichael S. Tsirkin <mst@redhat.com>
Signed-off-by: NMichael S. Tsirkin <mst@redhat.com>
Reviewed-by: NPaolo Bonzini <pbonzini@redhat.com>

Running
    qemu-bin ... -machine pc,dump-guest-core=on
leads to crash:
    x86_64-softmmu/qemu-system-x86_64 -machine pc,dump-guest-core=on
    qemu-system-x86_64: qemu/util/qemu-option.c:387: qemu_opt_get_bool_helper:
    Assertion `opt->desc && opt->desc->type == QEMU_OPT_BOOL' failed.  Aborted
    (core dumped)

This happens because the commit e79d5a6 ("machine: remove qemu_machine_opts
global list") removed the global option descriptions and moved them to
MachineState's QOM properties.

Fix this by querying machine properties through designated wrappers.
Signed-off-by: NMarcel Apfelbaum <marcel@redhat.com>
Acked-by: NMichael S. Tsirkin <mst@redhat.com>
Signed-off-by: NMichael S. Tsirkin <mst@redhat.com>
Reviewed-by: NPaolo Bonzini <pbonzini@redhat.com>

This fixes a NULL pointer dereference in s390x-softmmu.

On pretty much all other architectures, creating an MMIO region calls
cpu_reload_memory_map.  On s390, however, there are no MMIO regions
and everything is done via hypercalls.

Fixes: 9d82b5a7Reported-by: NChristian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

Signed-off-by: NGonglei <arei.gonglei@huawei.com>
Reviewed-by: NMarkus Armbruster <armbru@redhat.com>
Signed-off-by: NMichael Tokarev <mjt@tls.msk.ru>

Block size must fundamentally be a multiple of target page size.
Aligning automatically removes need to worry about the alignment
from callers.

Note: the only caller of qemu_ram_resize (acpi) already happens to have
size padded to a power of 2, but we would like to drop the padding in
ACPI core, and don't want to expose target page size knowledge to ACPI.
Signed-off-by: NMichael S. Tsirkin <mst@redhat.com>
Acked-by: NPaolo Bonzini <ponzini@redhat.com>

Allow "unlocked" reads of the ram_list by using an RCU-enabled QLIST.

The ramlist mutex is kept.  call_rcu callbacks are run with the iothread
lock taken, but that may change in the future.  Writers still take the
ramlist mutex, but they no longer need to assume that the iothread lock
is taken.

Readers of the list, instead, no longer require either the iothread
or ramlist mutex, but they need to use rcu_read_lock() and
rcu_read_unlock().

One place in arch_init.c was downgrading from write side to read side
like this:

    qemu_mutex_lock_iothread()
    qemu_mutex_lock_ramlist()
    ...
    qemu_mutex_unlock_iothread()
    ...
    qemu_mutex_unlock_ramlist()

and the equivalent idiom is:

    qemu_mutex_lock_ramlist()
    rcu_read_lock()
    ...
    qemu_mutex_unlock_ramlist()
    ...
    rcu_read_unlock()
Reviewed-by: NFam Zheng <famz@redhat.com>
Signed-off-by: NMike Day <ncmike@ncultra.org>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

QLIST has RCU-friendly primitives, so switch to it.
Reviewed-by: NFam Zheng <famz@redhat.com>
Signed-off-by: NMike Day <ncmike@ncultra.org>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

Reviewed-by: NFam Zheng <famz@redhat.com>
Signed-off-by: NMike Day <ncmike@ncultra.org>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

Hence, freeing a RAMBlock has to be switched to call_rcu.
Reviewed-by: NFam Zheng <famz@redhat.com>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

Note that even after this patch, most callers of address_space_*
functions must still be under the big QEMU lock, otherwise the memory
region returned by address_space_translate can disappear as soon as
address_space_translate returns.  This will be fixed in the next part
of this series.
Reviewed-by: NFam Zheng <famz@redhat.com>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

After the previous patch, TLBs will be flushed on every change to
the memory mapping.  This patch augments that with synchronization
of the MemoryRegionSections referred to in the iotlb array.

With this change, it is guaranteed that iotlb_to_region will access
the correct memory map, even once the TLB will be accessed outside
the BQL.
Reviewed-by: NFam Zheng <famz@redhat.com>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

This for now is a simple TLB flush.  This can change later for two
reasons:

1) an AddressSpaceDispatch will be cached in the CPUState object

2) it will not be possible to do tlb_flush once the TCG-generated code
runs outside the BQL.
Reviewed-by: NFam Zheng <famz@redhat.com>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

address_space_destroy_dispatch is called from an RCU callback and hence
outside the iothread mutex (BQL).  However, after address_space_destroy
no new accesses can hit the destroyed AddressSpace so it is not necessary
to observe changes to the memory map.  Move the memory_listener_unregister
call earlier, to make it thread-safe again.
Reported-by: NAlex Williamson <alex.williamson@redhat.com>
Fixes: 374f2981Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
Signed-off-by: NAlex Williamson <alex.williamson@redhat.com>

Coverity flags this as "dereference after null check".  Not quite a
dereference, since it will just EFAULT, but still nice to fix.
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>