[Qemu-devel] [PATCH V2 0/7] execute code from mmio area

[Qemu-devel] [PATCH V2 0/7] execute code from mmio area

[fred.konrad]

From: KONRAD Frederic <fred.konrad@greensocs.com>

This series allows to execute code from mmio areas.
The main goal of this is to be able to run code for example from an SPI device.

The three first patch fixes the way get_page_addr_code fills the TLB.

The fourth patch implements the mmio execution helpers: the device must
implement the request_ptr callback of the MemoryRegion and will be notified when
the guest wants to execute code from it.

The fifth patch introduces mmio_interface device which allows to dynamically
map a host pointer somewhere into the memory.

The sixth patch implements the execution from the SPI memories in the
xilinx_spips model.

Thanks,
Fred

V1 -> V2:
  * Fix the DPRINTF error.
RFC -> V1:
  * Use an interface (mmio-interface) to fix any reference leak issue.

KONRAD Frederic (7):
  cputlb: cleanup get_page_addr_code to use VICTIM_TLB_HIT
  cputlb: move get_page_addr_code
  cputlb: fix the way get_page_addr_code fills the tlb
  exec: allow to get a pointer for some mmio memory region
  qdev: add MemoryRegion property
  introduce mmio_interface
  xilinx_spips: allow mmio execution

 cputlb.c                         |  81 ++++++++++++++-----------
 hw/misc/Makefile.objs            |   1 +
 hw/misc/mmio_interface.c         | 128 +++++++++++++++++++++++++++++++++++++++
 hw/ssi/xilinx_spips.c            |  74 ++++++++++++++++------
 include/exec/memory.h            |  35 +++++++++++
 include/hw/misc/mmio_interface.h |  49 +++++++++++++++
 include/hw/qdev-properties.h     |   2 +
 memory.c                         |  57 +++++++++++++++++
 8 files changed, 372 insertions(+), 55 deletions(-)
 create mode 100644 hw/misc/mmio_interface.c
 create mode 100644 include/hw/misc/mmio_interface.h

-- 
1.8.3.1

[Qemu-devel] [PATCH V2 1/7] cputlb: cleanup get_page_addr_code to use VICTIM_TLB_HIT

[fred.konrad]

From: KONRAD Frederic <fred.konrad@greensocs.com>

This replaces env1 and page_index variables by env and index
so we can use VICTIM_TLB_HIT macro later.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
 cputlb.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/cputlb.c b/cputlb.c
index 6c39927..665caea 100644
--- a/cputlb.c
+++ b/cputlb.c
@@ -457,21 +457,21 @@ static void report_bad_exec(CPUState *cpu, target_ulong addr)
  * is actually a ram_addr_t (in system mode; the user mode emulation
  * version of this function returns a guest virtual address).
  */
-tb_page_addr_t get_page_addr_code(CPUArchState *env1, target_ulong addr)
+tb_page_addr_t get_page_addr_code(CPUArchState *env, target_ulong addr)
 {
-    int mmu_idx, page_index, pd;
+    int mmu_idx, index, pd;
     void *p;
     MemoryRegion *mr;
-    CPUState *cpu = ENV_GET_CPU(env1);
+    CPUState *cpu = ENV_GET_CPU(env);
     CPUIOTLBEntry *iotlbentry;
 
-    page_index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
-    mmu_idx = cpu_mmu_index(env1, true);
-    if (unlikely(env1->tlb_table[mmu_idx][page_index].addr_code !=
+    index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
+    mmu_idx = cpu_mmu_index(env, true);
+    if (unlikely(env->tlb_table[mmu_idx][index].addr_code !=
                  (addr & TARGET_PAGE_MASK))) {
-        cpu_ldub_code(env1, addr);
+        cpu_ldub_code(env, addr);
     }
-    iotlbentry = &env1->iotlb[mmu_idx][page_index];
+    iotlbentry = &env->iotlb[mmu_idx][index];
     pd = iotlbentry->addr & ~TARGET_PAGE_MASK;
     mr = iotlb_to_region(cpu, pd, iotlbentry->attrs);
     if (memory_region_is_unassigned(mr)) {
@@ -484,7 +484,7 @@ tb_page_addr_t get_page_addr_code(CPUArchState *env1, target_ulong addr)
             exit(1);
         }
     }
-    p = (void *)((uintptr_t)addr + env1->tlb_table[mmu_idx][page_index].addend);
+    p = (void *)((uintptr_t)addr + env->tlb_table[mmu_idx][index].addend);
     return qemu_ram_addr_from_host_nofail(p);
 }
 
-- 
1.8.3.1

[Qemu-devel] [PATCH V2 2/7] cputlb: move get_page_addr_code

[fred.konrad]

From: KONRAD Frederic <fred.konrad@greensocs.com>

This just moves the code before VICTIM_TLB_HIT macro definition
so we can use it.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
 cputlb.c | 72 ++++++++++++++++++++++++++++++++--------------------------------
 1 file changed, 36 insertions(+), 36 deletions(-)

diff --git a/cputlb.c b/cputlb.c
index 665caea..b3a5f47 100644
--- a/cputlb.c
+++ b/cputlb.c
@@ -452,42 +452,6 @@ static void report_bad_exec(CPUState *cpu, target_ulong addr)
     log_cpu_state_mask(LOG_GUEST_ERROR, cpu, CPU_DUMP_FPU | CPU_DUMP_CCOP);
 }
 
-/* NOTE: this function can trigger an exception */
-/* NOTE2: the returned address is not exactly the physical address: it
- * is actually a ram_addr_t (in system mode; the user mode emulation
- * version of this function returns a guest virtual address).
- */
-tb_page_addr_t get_page_addr_code(CPUArchState *env, target_ulong addr)
-{
-    int mmu_idx, index, pd;
-    void *p;
-    MemoryRegion *mr;
-    CPUState *cpu = ENV_GET_CPU(env);
-    CPUIOTLBEntry *iotlbentry;
-
-    index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
-    mmu_idx = cpu_mmu_index(env, true);
-    if (unlikely(env->tlb_table[mmu_idx][index].addr_code !=
-                 (addr & TARGET_PAGE_MASK))) {
-        cpu_ldub_code(env, addr);
-    }
-    iotlbentry = &env->iotlb[mmu_idx][index];
-    pd = iotlbentry->addr & ~TARGET_PAGE_MASK;
-    mr = iotlb_to_region(cpu, pd, iotlbentry->attrs);
-    if (memory_region_is_unassigned(mr)) {
-        CPUClass *cc = CPU_GET_CLASS(cpu);
-
-        if (cc->do_unassigned_access) {
-            cc->do_unassigned_access(cpu, addr, false, true, 0, 4);
-        } else {
-            report_bad_exec(cpu, addr);
-            exit(1);
-        }
-    }
-    p = (void *)((uintptr_t)addr + env->tlb_table[mmu_idx][index].addend);
-    return qemu_ram_addr_from_host_nofail(p);
-}
-
 static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry *iotlbentry,
                          target_ulong addr, uintptr_t retaddr, int size)
 {
@@ -554,6 +518,42 @@ static bool victim_tlb_hit(CPUArchState *env, size_t mmu_idx, size_t index,
   victim_tlb_hit(env, mmu_idx, index, offsetof(CPUTLBEntry, TY), \
                  (ADDR) & TARGET_PAGE_MASK)
 
+/* NOTE: this function can trigger an exception */
+/* NOTE2: the returned address is not exactly the physical address: it
+ * is actually a ram_addr_t (in system mode; the user mode emulation
+ * version of this function returns a guest virtual address).
+ */
+tb_page_addr_t get_page_addr_code(CPUArchState *env, target_ulong addr)
+{
+    int mmu_idx, index, pd;
+    void *p;
+    MemoryRegion *mr;
+    CPUState *cpu = ENV_GET_CPU(env);
+    CPUIOTLBEntry *iotlbentry;
+
+    index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
+    mmu_idx = cpu_mmu_index(env, true);
+    if (unlikely(env->tlb_table[mmu_idx][index].addr_code !=
+                 (addr & TARGET_PAGE_MASK))) {
+        cpu_ldub_code(env, addr);
+    }
+    iotlbentry = &env->iotlb[mmu_idx][index];
+    pd = iotlbentry->addr & ~TARGET_PAGE_MASK;
+    mr = iotlb_to_region(cpu, pd, iotlbentry->attrs);
+    if (memory_region_is_unassigned(mr)) {
+        CPUClass *cc = CPU_GET_CLASS(cpu);
+
+        if (cc->do_unassigned_access) {
+            cc->do_unassigned_access(cpu, addr, false, true, 0, 4);
+        } else {
+            report_bad_exec(cpu, addr);
+            exit(1);
+        }
+    }
+    p = (void *)((uintptr_t)addr + env->tlb_table[mmu_idx][index].addend);
+    return qemu_ram_addr_from_host_nofail(p);
+}
+
 /* Probe for whether the specified guest write access is permitted.
  * If it is not permitted then an exception will be taken in the same
  * way as if this were a real write access (and we will not return).
-- 
1.8.3.1

[Qemu-devel] [PATCH V2 3/7] cputlb: fix the way get_page_addr_code fills the tlb

[fred.konrad]

From: KONRAD Frederic <fred.konrad@greensocs.com>

get_page_addr_code(..) does a cpu_ldub_code to fill the tlb:
This can lead to some side effects if a device is mapped at this address.

So this patch replaces the cpu_memory_ld by a tlb_fill.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
 cputlb.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/cputlb.c b/cputlb.c
index b3a5f47..846341e 100644
--- a/cputlb.c
+++ b/cputlb.c
@@ -534,8 +534,10 @@ tb_page_addr_t get_page_addr_code(CPUArchState *env, target_ulong addr)
     index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
     mmu_idx = cpu_mmu_index(env, true);
     if (unlikely(env->tlb_table[mmu_idx][index].addr_code !=
-                 (addr & TARGET_PAGE_MASK))) {
-        cpu_ldub_code(env, addr);
+                 (addr & (TARGET_PAGE_MASK | TLB_INVALID_MASK)))) {
+        if (!VICTIM_TLB_HIT(addr_read, addr)) {
+            tlb_fill(ENV_GET_CPU(env), addr, MMU_INST_FETCH, mmu_idx, 0);
+        }
     }
     iotlbentry = &env->iotlb[mmu_idx][index];
     pd = iotlbentry->addr & ~TARGET_PAGE_MASK;
-- 
1.8.3.1

[Qemu-devel] [PATCH V2 4/7] exec: allow to get a pointer for some mmio memory region

[fred.konrad]

From: KONRAD Frederic <fred.konrad@greensocs.com>

This introduces a special callback which allows to run code from some MMIO
devices.

SysBusDevice with a MemoryRegion which implements the request_ptr callback will
be notified when the guest try to execute code from their offset. Then it will
be able to eg: pre-load some code from an SPI device or ask a pointer from an
external simulator, etc..

When the pointer or the data in it are no longer valid the device has to
invalidate it.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>

RFC -> V1:
  * Use mmio-interface instead of directly creating the subregion.
---
 cputlb.c              |  7 +++++++
 include/exec/memory.h | 35 +++++++++++++++++++++++++++++++
 memory.c              | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 99 insertions(+)

diff --git a/cputlb.c b/cputlb.c
index 846341e..9077247 100644
--- a/cputlb.c
+++ b/cputlb.c
@@ -545,6 +545,13 @@ tb_page_addr_t get_page_addr_code(CPUArchState *env, target_ulong addr)
     if (memory_region_is_unassigned(mr)) {
         CPUClass *cc = CPU_GET_CLASS(cpu);
 
+        if (memory_region_request_mmio_ptr(mr, addr)) {
+            /* A MemoryRegion is potentially added so re-run the
+             * get_page_addr_code.
+             */
+            return get_page_addr_code(env, addr);
+        }
+
         if (cc->do_unassigned_access) {
             cc->do_unassigned_access(cpu, addr, false, true, 0, 4);
         } else {
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 987f925..36b0eec 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -120,6 +120,15 @@ struct MemoryRegionOps {
                                     uint64_t data,
                                     unsigned size,
                                     MemTxAttrs attrs);
+    /* Instruction execution pre-callback:
+     * @addr is the address of the access relative to the @mr.
+     * @size is the size of the area returned by the callback.
+     * @offset is the location of the pointer inside @mr.
+     *
+     * Returns a pointer to a location which contains guest code.
+     */
+    void *(*request_ptr)(void *opaque, hwaddr addr, unsigned *size,
+                         unsigned *offset);
 
     enum device_endian endianness;
     /* Guest-visible constraints: */
@@ -1253,6 +1262,32 @@ void memory_global_dirty_log_stop(void);
 void mtree_info(fprintf_function mon_printf, void *f, bool flatview);
 
 /**
+ * memory_region_request_mmio_ptr: request a pointer to an mmio
+ * MemoryRegion. If it is possible map a RAM MemoryRegion with this pointer.
+ * When the device wants to invalidate the pointer it will call
+ * memory_region_invalidate_mmio_ptr.
+ *
+ * @mr: #MemoryRegion to check
+ * @addr: address within that region
+ *
+ * Returns true on success, false otherwise.
+ */
+bool memory_region_request_mmio_ptr(MemoryRegion *mr, hwaddr addr);
+
+/**
+ * memory_region_invalidate_mmio_ptr: invalidate the pointer to an mmio
+ * previously requested.
+ * In the end that means that if something wants to execute from this area it
+ * will need to request the pointer again.
+ *
+ * @mr: #MemoryRegion associated to the pointer.
+ * @addr: address within that region
+ * @size: size of that area.
+ */
+void memory_region_invalidate_mmio_ptr(MemoryRegion *mr, hwaddr offset,
+                                       unsigned size);
+
+/**
  * memory_region_dispatch_read: perform a read directly to the specified
  * MemoryRegion.
  *
diff --git a/memory.c b/memory.c
index 6c58373..a605250 100644
--- a/memory.c
+++ b/memory.c
@@ -30,6 +30,8 @@
 #include "exec/ram_addr.h"
 #include "sysemu/kvm.h"
 #include "sysemu/sysemu.h"
+#include "hw/misc/mmio_interface.h"
+#include "hw/qdev-properties.h"
 
 //#define DEBUG_UNASSIGNED
 
@@ -2375,6 +2377,61 @@ void memory_listener_unregister(MemoryListener *listener)
     QTAILQ_REMOVE(&listener->address_space->listeners, listener, link_as);
 }
 
+bool memory_region_request_mmio_ptr(MemoryRegion *mr, hwaddr addr)
+{
+    void *host;
+    unsigned size = 0;
+    unsigned offset = 0;
+    Object *new_interface;
+
+    if (!mr || !mr->ops->request_ptr) {
+        return false;
+    }
+
+    /*
+     * Avoid an update if the request_ptr call
+     * memory_region_invalidate_mmio_ptr which seems to be likely when we use
+     * a cache.
+     */
+    memory_region_transaction_begin();
+
+    host = mr->ops->request_ptr(mr->opaque, addr - mr->addr, &size, &offset);
+
+    if (!host || !size) {
+        memory_region_transaction_commit();
+        return false;
+    }
+
+    new_interface = object_new("mmio_interface");
+    qdev_prop_set_uint64(DEVICE(new_interface), "start", offset);
+    qdev_prop_set_uint64(DEVICE(new_interface), "end", offset + size - 1);
+    qdev_prop_set_bit(DEVICE(new_interface), "ro", true);
+    qdev_prop_set_ptr(DEVICE(new_interface), "host_ptr", host);
+    qdev_prop_set_ptr(DEVICE(new_interface), "subregion", mr);
+    object_property_set_bool(OBJECT(new_interface), true, "realized", NULL);
+
+    memory_region_transaction_commit();
+    return true;
+}
+
+void memory_region_invalidate_mmio_ptr(MemoryRegion *mr, hwaddr offset,
+                                       unsigned size)
+{
+    MemoryRegionSection section = memory_region_find(mr, offset, size);
+
+    if (section.mr != mr) {
+        /* memory_region_find add a ref on section.mr */
+        memory_region_unref(section.mr);
+        if (!MMIO_INTERFACE(section.mr->owner)) {
+            return;
+        }
+        /* We found the interface just drop it. */
+        object_property_set_bool(section.mr->owner, false, "realized", NULL);
+        object_unref(section.mr->owner);
+        object_unparent(section.mr->owner);
+    }
+}
+
 void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name)
 {
     memory_region_ref(root);
-- 
1.8.3.1

[Qemu-devel] [PATCH V2 5/7] qdev: add MemoryRegion property

[fred.konrad]

From: KONRAD Frederic <fred.konrad@greensocs.com>

We need to pass a pointer to a MemoryRegion for mmio_interface.
So this just adds that.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
 include/hw/qdev-properties.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 7ac3153..babb258 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -171,6 +171,8 @@ extern PropertyInfo qdev_prop_arraylen;
     DEFINE_PROP_DEFAULT(_n, _s, _f, 0, qdev_prop_blocksize, uint16_t)
 #define DEFINE_PROP_PCI_HOST_DEVADDR(_n, _s, _f) \
     DEFINE_PROP(_n, _s, _f, qdev_prop_pci_host_devaddr, PCIHostDeviceAddress)
+#define DEFINE_PROP_MEMORY_REGION(_n, _s, _f)             \
+    DEFINE_PROP(_n, _s, _f, qdev_prop_ptr, MemoryRegion *)
 
 #define DEFINE_PROP_END_OF_LIST()               \
     {}
-- 
1.8.3.1

[Qemu-devel] [PATCH V2 6/7] introduce mmio_interface

[fred.konrad]

From: KONRAD Frederic <fred.konrad@greensocs.com>

This introduces mmio_interface object which contains a MemoryRegion
and can be hotplugged/hotunplugged.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>

V1 -> V2:
  * Fix the qemu_log format.
---
 hw/misc/Makefile.objs            |   1 +
 hw/misc/mmio_interface.c         | 128 +++++++++++++++++++++++++++++++++++++++
 include/hw/misc/mmio_interface.h |  49 +++++++++++++++
 3 files changed, 178 insertions(+)
 create mode 100644 hw/misc/mmio_interface.c
 create mode 100644 include/hw/misc/mmio_interface.h

diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index 898e4cc..91580a3 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -55,3 +55,4 @@ obj-$(CONFIG_EDU) += edu.o
 obj-$(CONFIG_HYPERV_TESTDEV) += hyperv_testdev.o
 obj-$(CONFIG_AUX) += auxbus.o
 obj-$(CONFIG_ASPEED_SOC) += aspeed_scu.o aspeed_sdmc.o
+obj-y += mmio_interface.o
\ No newline at end of file
diff --git a/hw/misc/mmio_interface.c b/hw/misc/mmio_interface.c
new file mode 100644
index 0000000..6f004d2
--- /dev/null
+++ b/hw/misc/mmio_interface.c
@@ -0,0 +1,128 @@
+/*
+ * mmio_interface.c
+ *
+ *  Copyright (C) 2017 : GreenSocs
+ *      http://www.greensocs.com/ , email: info@greensocs.com
+ *
+ *  Developed by :
+ *  Frederic Konrad   <fred.konrad@greensocs.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option)any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "trace.h"
+#include "hw/qdev-properties.h"
+#include "hw/misc/mmio_interface.h"
+#include "qapi/error.h"
+
+#ifndef DEBUG_MMIO_INTERFACE
+#define DEBUG_MMIO_INTERFACE 0
+#endif
+
+static uint64_t mmio_interface_counter;
+
+#define DPRINTF(fmt, ...) do {                                                 \
+    if (DEBUG_MMIO_INTERFACE) {                                                \
+        qemu_log("mmio_interface: 0x%" PRIX64 ": " fmt, s->id, ## __VA_ARGS__);\
+    }                                                                          \
+} while (0);
+
+static void mmio_interface_init(Object *obj)
+{
+    MMIOInterface *s = MMIO_INTERFACE(obj);
+
+    if (DEBUG_MMIO_INTERFACE) {
+        s->id = mmio_interface_counter++;
+    }
+
+    DPRINTF("interface created\n");
+    s->host_ptr = 0;
+    s->subregion = 0;
+}
+
+static void mmio_interface_realize(DeviceState *dev, Error **errp)
+{
+    MMIOInterface *s = MMIO_INTERFACE(dev);
+
+    DPRINTF("realize from 0x%" PRIX64 " to 0x%" PRIX64 " map host pointer"
+            " %p\n", s->start, s->end, s->host_ptr);
+
+    if (!s->host_ptr) {
+        error_setg(errp, "host_ptr property must be set");
+    }
+
+    if (!s->subregion) {
+        error_setg(errp, "subregion property must be set");
+    }
+
+    memory_region_init_ram_ptr(&s->ram_mem, OBJECT(s), "ram",
+                               s->end - s->start + 1, s->host_ptr);
+    memory_region_set_readonly(&s->ram_mem, s->ro);
+    memory_region_add_subregion(s->subregion, s->start, &s->ram_mem);
+}
+
+static void mmio_interface_unrealize(DeviceState *dev, Error **errp)
+{
+    MMIOInterface *s = MMIO_INTERFACE(dev);
+
+    DPRINTF("unrealize from 0x%" PRIX64 " to 0x%" PRIX64 " map host pointer"
+            " %p\n", s->start, s->end, s->host_ptr);
+    memory_region_del_subregion(s->subregion, &s->ram_mem);
+}
+
+static void mmio_interface_finalize(Object *obj)
+{
+    MMIOInterface *s = MMIO_INTERFACE(obj);
+
+    DPRINTF("finalize from 0x%" PRIX64 " to 0x%" PRIX64 " map host pointer"
+            " %p\n", s->start, s->end, s->host_ptr);
+    object_unparent(OBJECT(&s->ram_mem));
+}
+
+static Property mmio_interface_properties[] = {
+    DEFINE_PROP_UINT64("start", MMIOInterface, start, 0),
+    DEFINE_PROP_UINT64("end", MMIOInterface, end, 0),
+    DEFINE_PROP_PTR("host_ptr", MMIOInterface, host_ptr),
+    DEFINE_PROP_BOOL("ro", MMIOInterface, ro, false),
+    DEFINE_PROP_MEMORY_REGION("subregion", MMIOInterface, subregion),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void mmio_interface_class_init(ObjectClass *oc, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(oc);
+
+    dc->realize = mmio_interface_realize;
+    dc->unrealize = mmio_interface_unrealize;
+    dc->props = mmio_interface_properties;
+}
+
+static const TypeInfo mmio_interface_info = {
+    .name          = TYPE_MMIO_INTERFACE,
+    .parent        = TYPE_DEVICE,
+    .instance_size = sizeof(MMIOInterface),
+    .instance_init = mmio_interface_init,
+    .instance_finalize = mmio_interface_finalize,
+    .class_init    = mmio_interface_class_init,
+};
+
+static void mmio_interface_register_types(void)
+{
+    type_register_static(&mmio_interface_info);
+}
+
+type_init(mmio_interface_register_types)
diff --git a/include/hw/misc/mmio_interface.h b/include/hw/misc/mmio_interface.h
new file mode 100644
index 0000000..90d34fb
--- /dev/null
+++ b/include/hw/misc/mmio_interface.h
@@ -0,0 +1,49 @@
+/*
+ * mmio_interface.h
+ *
+ *  Copyright (C) 2017 : GreenSocs
+ *      http://www.greensocs.com/ , email: info@greensocs.com
+ *
+ *  Developed by :
+ *  Frederic Konrad   <fred.konrad@greensocs.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option)any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#ifndef MMIO_INTERFACE_H
+#define MMIO_INTERFACE_H
+
+#include "exec/memory.h"
+
+#define TYPE_MMIO_INTERFACE "mmio_interface"
+#define MMIO_INTERFACE(obj) OBJECT_CHECK(MMIOInterface, (obj),                 \
+                                         TYPE_MMIO_INTERFACE)
+
+typedef struct MMIOInterface {
+    DeviceState parent_obj;
+
+    MemoryRegion *subregion;
+    MemoryRegion ram_mem;
+    uint64_t start;
+    uint64_t end;
+    bool ro;
+    uint64_t id;
+    void *host_ptr;
+} MMIOInterface;
+
+void mmio_interface_map(MMIOInterface *s);
+void mmio_interface_unmap(MMIOInterface *s);
+
+#endif /* MMIO_INTERFACE_H */
-- 
1.8.3.1

[Qemu-devel] [PATCH V2 7/7] xilinx_spips: allow mmio execution

[fred.konrad]

From: KONRAD Frederic <fred.konrad@greensocs.com>

This allows to execute from the lqspi area.

When the request_ptr is called the device loads 1024bytes from the SPI device.
Then this code can be executed by the guest.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
 hw/ssi/xilinx_spips.c | 74 ++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 55 insertions(+), 19 deletions(-)

diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c
index da8adfa..e833028 100644
--- a/hw/ssi/xilinx_spips.c
+++ b/hw/ssi/xilinx_spips.c
@@ -496,6 +496,18 @@ static const MemoryRegionOps spips_ops = {
     .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
+static void xilinx_qspips_invalidate_mmio_ptr(XilinxQSPIPS *q)
+{
+    XilinxSPIPS *s = &q->parent_obj;
+
+    if (q->lqspi_cached_addr != ~0ULL) {
+        /* Invalidate the current mapped mmio */
+        memory_region_invalidate_mmio_ptr(&s->mmlqspi, q->lqspi_cached_addr,
+                                          LQSPI_CACHE_SIZE);
+        q->lqspi_cached_addr = ~0ULL;
+    }
+}
+
 static void xilinx_qspips_write(void *opaque, hwaddr addr,
                                 uint64_t value, unsigned size)
 {
@@ -505,7 +517,7 @@ static void xilinx_qspips_write(void *opaque, hwaddr addr,
     addr >>= 2;
 
     if (addr == R_LQSPI_CFG) {
-        q->lqspi_cached_addr = ~0ULL;
+        xilinx_qspips_invalidate_mmio_ptr(q);
     }
 }
 
@@ -517,27 +529,20 @@ static const MemoryRegionOps qspips_ops = {
 
 #define LQSPI_CACHE_SIZE 1024
 
-static uint64_t
-lqspi_read(void *opaque, hwaddr addr, unsigned int size)
+static void lqspi_load_cache(void *opaque, hwaddr addr)
 {
-    int i;
     XilinxQSPIPS *q = opaque;
     XilinxSPIPS *s = opaque;
-    uint32_t ret;
-
-    if (addr >= q->lqspi_cached_addr &&
-            addr <= q->lqspi_cached_addr + LQSPI_CACHE_SIZE - 4) {
-        uint8_t *retp = &q->lqspi_buf[addr - q->lqspi_cached_addr];
-        ret = cpu_to_le32(*(uint32_t *)retp);
-        DB_PRINT_L(1, "addr: %08x, data: %08x\n", (unsigned)addr,
-                   (unsigned)ret);
-        return ret;
-    } else {
-        int flash_addr = (addr / num_effective_busses(s));
-        int slave = flash_addr >> LQSPI_ADDRESS_BITS;
-        int cache_entry = 0;
-        uint32_t u_page_save = s->regs[R_LQSPI_STS] & ~LQSPI_CFG_U_PAGE;
-
+    int i;
+    int flash_addr = ((addr & ~(LQSPI_CACHE_SIZE - 1))
+                   / num_effective_busses(s));
+    int slave = flash_addr >> LQSPI_ADDRESS_BITS;
+    int cache_entry = 0;
+    uint32_t u_page_save = s->regs[R_LQSPI_STS] & ~LQSPI_CFG_U_PAGE;
+
+    if (addr < q->lqspi_cached_addr ||
+            addr > q->lqspi_cached_addr + LQSPI_CACHE_SIZE - 4) {
+        xilinx_qspips_invalidate_mmio_ptr(q);
         s->regs[R_LQSPI_STS] &= ~LQSPI_CFG_U_PAGE;
         s->regs[R_LQSPI_STS] |= slave ? LQSPI_CFG_U_PAGE : 0;
 
@@ -589,12 +594,43 @@ lqspi_read(void *opaque, hwaddr addr, unsigned int size)
         xilinx_spips_update_cs_lines(s);
 
         q->lqspi_cached_addr = flash_addr * num_effective_busses(s);
+    }
+}
+
+static void *lqspi_request_mmio_ptr(void *opaque, hwaddr addr, unsigned *size,
+                                    unsigned *offset)
+{
+    XilinxQSPIPS *q = opaque;
+    hwaddr offset_within_the_region = addr & ~(LQSPI_CACHE_SIZE - 1);
+
+    lqspi_load_cache(opaque, offset_within_the_region);
+    *size = LQSPI_CACHE_SIZE;
+    *offset = offset_within_the_region;
+    return q->lqspi_buf;
+}
+
+static uint64_t
+lqspi_read(void *opaque, hwaddr addr, unsigned int size)
+{
+    XilinxQSPIPS *q = opaque;
+    uint32_t ret;
+
+    if (addr >= q->lqspi_cached_addr &&
+            addr <= q->lqspi_cached_addr + LQSPI_CACHE_SIZE - 4) {
+        uint8_t *retp = &q->lqspi_buf[addr - q->lqspi_cached_addr];
+        ret = cpu_to_le32(*(uint32_t *)retp);
+        DB_PRINT_L(1, "addr: %08x, data: %08x\n", (unsigned)addr,
+                   (unsigned)ret);
+        return ret;
+    } else {
+        lqspi_load_cache(opaque, addr);
         return lqspi_read(opaque, addr, size);
     }
 }
 
 static const MemoryRegionOps lqspi_ops = {
     .read = lqspi_read,
+    .request_ptr = lqspi_request_mmio_ptr,
     .endianness = DEVICE_NATIVE_ENDIAN,
     .valid = {
         .min_access_size = 1,
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH V2 0/7] execute code from mmio area

[KONRAD Frederic]

Ping!

Would be nice for us if we can get this into 2.9.

Thanks,
Fred

Le 17/02/2017 à 21:17, fred.konrad@greensocs.com a écrit :
> From: KONRAD Frederic <fred.konrad@greensocs.com>
>
> This series allows to execute code from mmio areas.
> The main goal of this is to be able to run code for example from an SPI device.
>
> The three first patch fixes the way get_page_addr_code fills the TLB.
>
> The fourth patch implements the mmio execution helpers: the device must
> implement the request_ptr callback of the MemoryRegion and will be notified when
> the guest wants to execute code from it.
>
> The fifth patch introduces mmio_interface device which allows to dynamically
> map a host pointer somewhere into the memory.
>
> The sixth patch implements the execution from the SPI memories in the
> xilinx_spips model.
>
> Thanks,
> Fred
>
> V1 -> V2:
>   * Fix the DPRINTF error.
> RFC -> V1:
>   * Use an interface (mmio-interface) to fix any reference leak issue.
>
> KONRAD Frederic (7):
>   cputlb: cleanup get_page_addr_code to use VICTIM_TLB_HIT
>   cputlb: move get_page_addr_code
>   cputlb: fix the way get_page_addr_code fills the tlb
>   exec: allow to get a pointer for some mmio memory region
>   qdev: add MemoryRegion property
>   introduce mmio_interface
>   xilinx_spips: allow mmio execution
>
>  cputlb.c                         |  81 ++++++++++++++-----------
>  hw/misc/Makefile.objs            |   1 +
>  hw/misc/mmio_interface.c         | 128 +++++++++++++++++++++++++++++++++++++++
>  hw/ssi/xilinx_spips.c            |  74 ++++++++++++++++------
>  include/exec/memory.h            |  35 +++++++++++
>  include/hw/misc/mmio_interface.h |  49 +++++++++++++++
>  include/hw/qdev-properties.h     |   2 +
>  memory.c                         |  57 +++++++++++++++++
>  8 files changed, 372 insertions(+), 55 deletions(-)
>  create mode 100644 hw/misc/mmio_interface.c
>  create mode 100644 include/hw/misc/mmio_interface.h
>

Re: [Qemu-devel] [PATCH V2 0/7] execute code from mmio area

[Frederic Konrad]

Hi All,

Any feedback for the 4 last patches?

Thanks,
Fred

On 02/17/2017 09:17 PM, fred.konrad@greensocs.com wrote:
> From: KONRAD Frederic <fred.konrad@greensocs.com>
> 
> This series allows to execute code from mmio areas.
> The main goal of this is to be able to run code for example from an SPI device.
> 
> The three first patch fixes the way get_page_addr_code fills the TLB.
> 
> The fourth patch implements the mmio execution helpers: the device must
> implement the request_ptr callback of the MemoryRegion and will be notified when
> the guest wants to execute code from it.
> 
> The fifth patch introduces mmio_interface device which allows to dynamically
> map a host pointer somewhere into the memory.
> 
> The sixth patch implements the execution from the SPI memories in the
> xilinx_spips model.
> 
> Thanks,
> Fred
> 
> V1 -> V2:
>   * Fix the DPRINTF error.
> RFC -> V1:
>   * Use an interface (mmio-interface) to fix any reference leak issue.
> 
> KONRAD Frederic (7):
>   cputlb: cleanup get_page_addr_code to use VICTIM_TLB_HIT
>   cputlb: move get_page_addr_code
>   cputlb: fix the way get_page_addr_code fills the tlb
>   exec: allow to get a pointer for some mmio memory region
>   qdev: add MemoryRegion property
>   introduce mmio_interface
>   xilinx_spips: allow mmio execution
> 
>  cputlb.c                         |  81 ++++++++++++++-----------
>  hw/misc/Makefile.objs            |   1 +
>  hw/misc/mmio_interface.c         | 128 +++++++++++++++++++++++++++++++++++++++
>  hw/ssi/xilinx_spips.c            |  74 ++++++++++++++++------
>  include/exec/memory.h            |  35 +++++++++++
>  include/hw/misc/mmio_interface.h |  49 +++++++++++++++
>  include/hw/qdev-properties.h     |   2 +
>  memory.c                         |  57 +++++++++++++++++
>  8 files changed, 372 insertions(+), 55 deletions(-)
>  create mode 100644 hw/misc/mmio_interface.c
>  create mode 100644 include/hw/misc/mmio_interface.h
> 

Re: [Qemu-devel] [PATCH V2 0/7] execute code from mmio area

[Paolo Bonzini]

On 21/02/2017 09:51, KONRAD Frederic wrote:
> Ping!
> 
> Would be nice for us if we can get this into 2.9.

Sorry, I've been much busier with icount than I would have liked. :(

Paolo

> Thanks,
> Fred
> 
> Le 17/02/2017 à 21:17, fred.konrad@greensocs.com a écrit :
>> From: KONRAD Frederic <fred.konrad@greensocs.com>
>>
>> This series allows to execute code from mmio areas.
>> The main goal of this is to be able to run code for example from an
>> SPI device.
>>
>> The three first patch fixes the way get_page_addr_code fills the TLB.
>>
>> The fourth patch implements the mmio execution helpers: the device must
>> implement the request_ptr callback of the MemoryRegion and will be
>> notified when
>> the guest wants to execute code from it.
>>
>> The fifth patch introduces mmio_interface device which allows to
>> dynamically
>> map a host pointer somewhere into the memory.
>>
>> The sixth patch implements the execution from the SPI memories in the
>> xilinx_spips model.
>>
>> Thanks,
>> Fred
>>
>> V1 -> V2:
>>   * Fix the DPRINTF error.
>> RFC -> V1:
>>   * Use an interface (mmio-interface) to fix any reference leak issue.
>>
>> KONRAD Frederic (7):
>>   cputlb: cleanup get_page_addr_code to use VICTIM_TLB_HIT
>>   cputlb: move get_page_addr_code
>>   cputlb: fix the way get_page_addr_code fills the tlb
>>   exec: allow to get a pointer for some mmio memory region
>>   qdev: add MemoryRegion property
>>   introduce mmio_interface
>>   xilinx_spips: allow mmio execution
>>
>>  cputlb.c                         |  81 ++++++++++++++-----------
>>  hw/misc/Makefile.objs            |   1 +
>>  hw/misc/mmio_interface.c         | 128
>> +++++++++++++++++++++++++++++++++++++++
>>  hw/ssi/xilinx_spips.c            |  74 ++++++++++++++++------
>>  include/exec/memory.h            |  35 +++++++++++
>>  include/hw/misc/mmio_interface.h |  49 +++++++++++++++
>>  include/hw/qdev-properties.h     |   2 +
>>  memory.c                         |  57 +++++++++++++++++
>>  8 files changed, 372 insertions(+), 55 deletions(-)
>>  create mode 100644 hw/misc/mmio_interface.c
>>  create mode 100644 include/hw/misc/mmio_interface.h
>>

Re: [Qemu-devel] [PATCH V2 4/7] exec: allow to get a pointer for some mmio memory region

[Edgar E. Iglesias]

On Fri, Feb 17, 2017 at 09:17:10PM +0100, fred.konrad@greensocs.com wrote:
> From: KONRAD Frederic <fred.konrad@greensocs.com>
> 
> This introduces a special callback which allows to run code from some MMIO
> devices.
> 
> SysBusDevice with a MemoryRegion which implements the request_ptr callback will
> be notified when the guest try to execute code from their offset. Then it will
> be able to eg: pre-load some code from an SPI device or ask a pointer from an
> external simulator, etc..
> 
> When the pointer or the data in it are no longer valid the device has to
> invalidate it.
> 
> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
> 
> RFC -> V1:
>   * Use mmio-interface instead of directly creating the subregion.

Hi Fred,




> ---
>  cputlb.c              |  7 +++++++
>  include/exec/memory.h | 35 +++++++++++++++++++++++++++++++
>  memory.c              | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 99 insertions(+)
> 
> diff --git a/cputlb.c b/cputlb.c
> index 846341e..9077247 100644
> --- a/cputlb.c
> +++ b/cputlb.c
> @@ -545,6 +545,13 @@ tb_page_addr_t get_page_addr_code(CPUArchState *env, target_ulong addr)
>      if (memory_region_is_unassigned(mr)) {
>          CPUClass *cc = CPU_GET_CLASS(cpu);
>  
> +        if (memory_region_request_mmio_ptr(mr, addr)) {
> +            /* A MemoryRegion is potentially added so re-run the
> +             * get_page_addr_code.
> +             */
> +            return get_page_addr_code(env, addr);
> +        }
> +
>          if (cc->do_unassigned_access) {
>              cc->do_unassigned_access(cpu, addr, false, true, 0, 4);
>          } else {
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 987f925..36b0eec 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -120,6 +120,15 @@ struct MemoryRegionOps {
>                                      uint64_t data,
>                                      unsigned size,
>                                      MemTxAttrs attrs);
> +    /* Instruction execution pre-callback:
> +     * @addr is the address of the access relative to the @mr.
> +     * @size is the size of the area returned by the callback.
> +     * @offset is the location of the pointer inside @mr.
> +     *
> +     * Returns a pointer to a location which contains guest code.
> +     */
> +    void *(*request_ptr)(void *opaque, hwaddr addr, unsigned *size,
> +                         unsigned *offset);
>  
>      enum device_endian endianness;
>      /* Guest-visible constraints: */
> @@ -1253,6 +1262,32 @@ void memory_global_dirty_log_stop(void);
>  void mtree_info(fprintf_function mon_printf, void *f, bool flatview);
>  
>  /**
> + * memory_region_request_mmio_ptr: request a pointer to an mmio
> + * MemoryRegion. If it is possible map a RAM MemoryRegion with this pointer.
> + * When the device wants to invalidate the pointer it will call
> + * memory_region_invalidate_mmio_ptr.
> + *
> + * @mr: #MemoryRegion to check
> + * @addr: address within that region
> + *
> + * Returns true on success, false otherwise.
> + */
> +bool memory_region_request_mmio_ptr(MemoryRegion *mr, hwaddr addr);
> +
> +/**
> + * memory_region_invalidate_mmio_ptr: invalidate the pointer to an mmio
> + * previously requested.
> + * In the end that means that if something wants to execute from this area it
> + * will need to request the pointer again.
> + *
> + * @mr: #MemoryRegion associated to the pointer.
> + * @addr: address within that region
> + * @size: size of that area.
> + */
> +void memory_region_invalidate_mmio_ptr(MemoryRegion *mr, hwaddr offset,
> +                                       unsigned size);
> +
> +/**
>   * memory_region_dispatch_read: perform a read directly to the specified
>   * MemoryRegion.
>   *
> diff --git a/memory.c b/memory.c
> index 6c58373..a605250 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -30,6 +30,8 @@
>  #include "exec/ram_addr.h"
>  #include "sysemu/kvm.h"
>  #include "sysemu/sysemu.h"
> +#include "hw/misc/mmio_interface.h"
> +#include "hw/qdev-properties.h"
>  
>  //#define DEBUG_UNASSIGNED
>  
> @@ -2375,6 +2377,61 @@ void memory_listener_unregister(MemoryListener *listener)
>      QTAILQ_REMOVE(&listener->address_space->listeners, listener, link_as);
>  }
>  
> +bool memory_region_request_mmio_ptr(MemoryRegion *mr, hwaddr addr)
> +{
> +    void *host;
> +    unsigned size = 0;
> +    unsigned offset = 0;
> +    Object *new_interface;
> +
> +    if (!mr || !mr->ops->request_ptr) {
> +        return false;
> +    }
> +
> +    /*
> +     * Avoid an update if the request_ptr call
> +     * memory_region_invalidate_mmio_ptr which seems to be likely when we use
> +     * a cache.
> +     */
> +    memory_region_transaction_begin();
> +
> +    host = mr->ops->request_ptr(mr->opaque, addr - mr->addr, &size, &offset);
> +
> +    if (!host || !size) {
> +        memory_region_transaction_commit();
> +        return false;
> +    }
> +
> +    new_interface = object_new("mmio_interface");
> +    qdev_prop_set_uint64(DEVICE(new_interface), "start", offset);
> +    qdev_prop_set_uint64(DEVICE(new_interface), "end", offset + size - 1);
> +    qdev_prop_set_bit(DEVICE(new_interface), "ro", true);
> +    qdev_prop_set_ptr(DEVICE(new_interface), "host_ptr", host);
> +    qdev_prop_set_ptr(DEVICE(new_interface), "subregion", mr);
> +    object_property_set_bool(OBJECT(new_interface), true, "realized", NULL);
> +
> +    memory_region_transaction_commit();
> +    return true;
> +}
> +
> +void memory_region_invalidate_mmio_ptr(MemoryRegion *mr, hwaddr offset,
> +                                       unsigned size)
> +{
> +    MemoryRegionSection section = memory_region_find(mr, offset, size);
> +
> +    if (section.mr != mr) {
> +        /* memory_region_find add a ref on section.mr */
> +        memory_region_unref(section.mr);
> +        if (!MMIO_INTERFACE(section.mr->owner)) {

Is MMIO_INTERFACE defined yet?
This may break bisection...

Cheers,
Edgar


> +            return;
> +        }
> +        /* We found the interface just drop it. */
> +        object_property_set_bool(section.mr->owner, false, "realized", NULL);
> +        object_unref(section.mr->owner);
> +        object_unparent(section.mr->owner);
> +    }
> +}
> +
>  void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name)
>  {
>      memory_region_ref(root);
> -- 
> 1.8.3.1
> 
> 

Re: [Qemu-devel] [PATCH V2 0/7] execute code from mmio area

[Edgar E. Iglesias]

On Fri, Feb 17, 2017 at 09:17:06PM +0100, fred.konrad@greensocs.com wrote:
> From: KONRAD Frederic <fred.konrad@greensocs.com>
> 
> This series allows to execute code from mmio areas.
> The main goal of this is to be able to run code for example from an SPI device.
> 
> The three first patch fixes the way get_page_addr_code fills the TLB.
> 
> The fourth patch implements the mmio execution helpers: the device must
> implement the request_ptr callback of the MemoryRegion and will be notified when
> the guest wants to execute code from it.
> 
> The fifth patch introduces mmio_interface device which allows to dynamically
> map a host pointer somewhere into the memory.
> 
> The sixth patch implements the execution from the SPI memories in the
> xilinx_spips model.


I had a comment on the possible break of bisection, but the series looks
good to me. If you fix up the ordering:

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

Cheers,
Edgar



> 
> Thanks,
> Fred
> 
> V1 -> V2:
>   * Fix the DPRINTF error.
> RFC -> V1:
>   * Use an interface (mmio-interface) to fix any reference leak issue.
> 
> KONRAD Frederic (7):
>   cputlb: cleanup get_page_addr_code to use VICTIM_TLB_HIT
>   cputlb: move get_page_addr_code
>   cputlb: fix the way get_page_addr_code fills the tlb
>   exec: allow to get a pointer for some mmio memory region
>   qdev: add MemoryRegion property
>   introduce mmio_interface
>   xilinx_spips: allow mmio execution
> 
>  cputlb.c                         |  81 ++++++++++++++-----------
>  hw/misc/Makefile.objs            |   1 +
>  hw/misc/mmio_interface.c         | 128 +++++++++++++++++++++++++++++++++++++++
>  hw/ssi/xilinx_spips.c            |  74 ++++++++++++++++------
>  include/exec/memory.h            |  35 +++++++++++
>  include/hw/misc/mmio_interface.h |  49 +++++++++++++++
>  include/hw/qdev-properties.h     |   2 +
>  memory.c                         |  57 +++++++++++++++++
>  8 files changed, 372 insertions(+), 55 deletions(-)
>  create mode 100644 hw/misc/mmio_interface.c
>  create mode 100644 include/hw/misc/mmio_interface.h
> 
> -- 
> 1.8.3.1
> 
> 

Re: [Qemu-devel] [PATCH V2 4/7] exec: allow to get a pointer for some mmio memory region

[Frederic Konrad]

On 03/03/2017 02:44 PM, Edgar E. Iglesias wrote:
> On Fri, Feb 17, 2017 at 09:17:10PM +0100, fred.konrad@greensocs.com wrote:
>> From: KONRAD Frederic <fred.konrad@greensocs.com>
>>
>> This introduces a special callback which allows to run code from some MMIO
>> devices.
>>
>> SysBusDevice with a MemoryRegion which implements the request_ptr callback will
>> be notified when the guest try to execute code from their offset. Then it will
>> be able to eg: pre-load some code from an SPI device or ask a pointer from an
>> external simulator, etc..
>>
>> When the pointer or the data in it are no longer valid the device has to
>> invalidate it.
>>
>> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
>>
>> RFC -> V1:
>>   * Use mmio-interface instead of directly creating the subregion.
> 
> Hi Fred,
> 
> 
> 
> 
>> ---
>>  cputlb.c              |  7 +++++++
>>  include/exec/memory.h | 35 +++++++++++++++++++++++++++++++
>>  memory.c              | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 99 insertions(+)
>>
>> diff --git a/cputlb.c b/cputlb.c
>> index 846341e..9077247 100644
>> --- a/cputlb.c
>> +++ b/cputlb.c
>> @@ -545,6 +545,13 @@ tb_page_addr_t get_page_addr_code(CPUArchState *env, target_ulong addr)
>>      if (memory_region_is_unassigned(mr)) {
>>          CPUClass *cc = CPU_GET_CLASS(cpu);
>>  
>> +        if (memory_region_request_mmio_ptr(mr, addr)) {
>> +            /* A MemoryRegion is potentially added so re-run the
>> +             * get_page_addr_code.
>> +             */
>> +            return get_page_addr_code(env, addr);
>> +        }
>> +
>>          if (cc->do_unassigned_access) {
>>              cc->do_unassigned_access(cpu, addr, false, true, 0, 4);
>>          } else {
>> diff --git a/include/exec/memory.h b/include/exec/memory.h
>> index 987f925..36b0eec 100644
>> --- a/include/exec/memory.h
>> +++ b/include/exec/memory.h
>> @@ -120,6 +120,15 @@ struct MemoryRegionOps {
>>                                      uint64_t data,
>>                                      unsigned size,
>>                                      MemTxAttrs attrs);
>> +    /* Instruction execution pre-callback:
>> +     * @addr is the address of the access relative to the @mr.
>> +     * @size is the size of the area returned by the callback.
>> +     * @offset is the location of the pointer inside @mr.
>> +     *
>> +     * Returns a pointer to a location which contains guest code.
>> +     */
>> +    void *(*request_ptr)(void *opaque, hwaddr addr, unsigned *size,
>> +                         unsigned *offset);
>>  
>>      enum device_endian endianness;
>>      /* Guest-visible constraints: */
>> @@ -1253,6 +1262,32 @@ void memory_global_dirty_log_stop(void);
>>  void mtree_info(fprintf_function mon_printf, void *f, bool flatview);
>>  
>>  /**
>> + * memory_region_request_mmio_ptr: request a pointer to an mmio
>> + * MemoryRegion. If it is possible map a RAM MemoryRegion with this pointer.
>> + * When the device wants to invalidate the pointer it will call
>> + * memory_region_invalidate_mmio_ptr.
>> + *
>> + * @mr: #MemoryRegion to check
>> + * @addr: address within that region
>> + *
>> + * Returns true on success, false otherwise.
>> + */
>> +bool memory_region_request_mmio_ptr(MemoryRegion *mr, hwaddr addr);
>> +
>> +/**
>> + * memory_region_invalidate_mmio_ptr: invalidate the pointer to an mmio
>> + * previously requested.
>> + * In the end that means that if something wants to execute from this area it
>> + * will need to request the pointer again.
>> + *
>> + * @mr: #MemoryRegion associated to the pointer.
>> + * @addr: address within that region
>> + * @size: size of that area.
>> + */
>> +void memory_region_invalidate_mmio_ptr(MemoryRegion *mr, hwaddr offset,
>> +                                       unsigned size);
>> +
>> +/**
>>   * memory_region_dispatch_read: perform a read directly to the specified
>>   * MemoryRegion.
>>   *
>> diff --git a/memory.c b/memory.c
>> index 6c58373..a605250 100644
>> --- a/memory.c
>> +++ b/memory.c
>> @@ -30,6 +30,8 @@
>>  #include "exec/ram_addr.h"
>>  #include "sysemu/kvm.h"
>>  #include "sysemu/sysemu.h"
>> +#include "hw/misc/mmio_interface.h"
>> +#include "hw/qdev-properties.h"
>>  
>>  //#define DEBUG_UNASSIGNED
>>  
>> @@ -2375,6 +2377,61 @@ void memory_listener_unregister(MemoryListener *listener)
>>      QTAILQ_REMOVE(&listener->address_space->listeners, listener, link_as);
>>  }
>>  
>> +bool memory_region_request_mmio_ptr(MemoryRegion *mr, hwaddr addr)
>> +{
>> +    void *host;
>> +    unsigned size = 0;
>> +    unsigned offset = 0;
>> +    Object *new_interface;
>> +
>> +    if (!mr || !mr->ops->request_ptr) {
>> +        return false;
>> +    }
>> +
>> +    /*
>> +     * Avoid an update if the request_ptr call
>> +     * memory_region_invalidate_mmio_ptr which seems to be likely when we use
>> +     * a cache.
>> +     */
>> +    memory_region_transaction_begin();
>> +
>> +    host = mr->ops->request_ptr(mr->opaque, addr - mr->addr, &size, &offset);
>> +
>> +    if (!host || !size) {
>> +        memory_region_transaction_commit();
>> +        return false;
>> +    }
>> +
>> +    new_interface = object_new("mmio_interface");
>> +    qdev_prop_set_uint64(DEVICE(new_interface), "start", offset);
>> +    qdev_prop_set_uint64(DEVICE(new_interface), "end", offset + size - 1);
>> +    qdev_prop_set_bit(DEVICE(new_interface), "ro", true);
>> +    qdev_prop_set_ptr(DEVICE(new_interface), "host_ptr", host);
>> +    qdev_prop_set_ptr(DEVICE(new_interface), "subregion", mr);
>> +    object_property_set_bool(OBJECT(new_interface), true, "realized", NULL);
>> +
>> +    memory_region_transaction_commit();
>> +    return true;
>> +}
>> +
>> +void memory_region_invalidate_mmio_ptr(MemoryRegion *mr, hwaddr offset,
>> +                                       unsigned size)
>> +{
>> +    MemoryRegionSection section = memory_region_find(mr, offset, size);
>> +
>> +    if (section.mr != mr) {
>> +        /* memory_region_find add a ref on section.mr */
>> +        memory_region_unref(section.mr);
>> +        if (!MMIO_INTERFACE(section.mr->owner)) {
> 
> Is MMIO_INTERFACE defined yet?
> This may break bisection...

OOPS, your right :(..
I'll resend with the right order!

Thanks!
Fred

> 
> Cheers,
> Edgar
> 
> 
>> +            return;
>> +        }
>> +        /* We found the interface just drop it. */
>> +        object_property_set_bool(section.mr->owner, false, "realized", NULL);
>> +        object_unref(section.mr->owner);
>> +        object_unparent(section.mr->owner);
>> +    }
>> +}
>> +
>>  void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name)
>>  {
>>      memory_region_ref(root);
>> -- 
>> 1.8.3.1
>>
>>