Re: [Qemu-devel] [PATCH 0/2] [RFC] 64-bit io paths

[Richard Henderson]

On 04/22/2010 01:01 PM, Blue Swirl wrote:
>> This is also fine.  Although by using NULL all you'd get is a qemu
>>  null pointer dereference; I suppose this might get caught and
>>  translated to an cpu exception, but I think it would be preferable
>>  long-term to be more explicit about this and fill in the entries
>>  with a function that would explicitly raise the exception.
> 
> Perhaps also the bus layer could do something here.

What do you mean?

> Interesting. Could you make a summary of the design for the benefit of the 
> list?

The basic device interface looks like

+/* Object based memory region registration.  */
+
+typedef struct MemoryRegion MemoryRegion;
+
+typedef uint32_t MemoryCallbackRead(MemoryRegion *, target_phys_addr_t ofs);
+typedef uint64_t MemoryCallbackRead64(MemoryRegion *, target_phys_addr_t ofs);
+typedef void MemoryCallbackWrite(MemoryRegion *, target_phys_addr_t ofs,
+                                 uint32_t val);
+typedef void MemoryCallbackWrite64(MemoryRegion *, target_phys_addr_t ofs,
+                                   uint64_t val);
+
+typedef struct MemoryCallbackInfo {
+    MemoryCallbackRead *read8;
+    MemoryCallbackRead *read16;
+    MemoryCallbackRead *read32;
+    MemoryCallbackRead64 *read64;
+
+    MemoryCallbackWrite *write8;
+    MemoryCallbackWrite *write16;
+    MemoryCallbackWrite *write32;
+    MemoryCallbackWrite64 *write64;
+
+    /* This describes RAM.  The callbacks above need not be used, and
+       the host memory backing the RAM begins at qemu_get_ram_ptr_r.  */
+    _Bool ram;
+
+    /* Legacy shim: propagate the IO_MEM_ROMD bit.  */
+    _Bool romd;
+} MemoryCallbackInfo;
+
+/* This structure describes the region.  It is logically opaque -- drivers
+   should not be peeking inside.  But in the interest of efficiency we want
+   to directly embed this structure in the driver's private strucure.  In
+   this way we can avoid having to have an extra table of opaque pointers
+   for consumption by the driver.  The intended use is
+
+    struct FooDeviceState {
+        DeviceState qdev;
+        MemoryRegion mem_region;
+        ...
+    };
+
+    static uint32_t foo_read8(MemoryRegion *region, target_phys_addr_t ofs)
+    {
+        FooDeviceState *s = container_of(region, FooDeviceState, mem_region);
+        ...
+    }
+*/
+
+struct MemoryRegion {
+    const MemoryCallbackInfo *cb;
+    target_phys_addr_t start;
+    target_phys_addr_t size;
+
+    /* If this value is not -1, this memory region is registred in
+       the io_mem_region array, used by softmmu_header.h to resolve
+       memory-mapped operations in the guest address space.  */
+    int io_index;
+};
+
+/* Register a memory region at START_ADDR/SIZE.  The REGION structure will
+   be initialized appropriately for DEV using CB as the operation set.  */
+extern void cpu_register_memory_region(MemoryRegion *region,
+                                       const MemoryCallbackInfo *cb,
+                                       target_phys_addr_t start_addr,
+                                       target_phys_addr_t size);
+
+/* Unregister a memory region.  */
+extern void cpu_unregister_memory_region(MemoryRegion *);
+
+/* Allocate ram for use with cpu_register_memory_region.  */
+extern const MemoryCallbackInfo *qemu_ram_alloc_r(ram_addr_t);
+extern void qemu_ram_free_r(const MemoryCallbackInfo *);

The Basic Idea is that we have a MemoryRegion object that describes
a contiguous mapping within the guest address space.  This object 
needs to handle RAM, ROM and devices.  The desire to handle memory
and devices the same comes from the wish to have PCI device BARs
show up as plain memory in the TLB as plain memory, and to be able
to handle all PCI device regions identically within sysbus.

There are a number of restrictions on the interface above what we
currently support:

  * Objects may not be mapped over the top of existing objects.
    This is most abused by pflash devices with their IO_MEM_ROMD thing.

  * Objects must be unmapped explicitly, rather than mapping 
    IO_MEM_UNASSIGNED over the top of an object.  This is most
    abused by the PCI subsystem.

The use of the MemoryRegion pointer doubling as the device's opaque
pointer means that it's easy to implement generic helpers such as

uint64_t composite_read64(MemoryRegion *region, target_phys_addr_t ofs)
{
   uint32_t v1 = region->cb->read32(region, ofs);
   uint32_t v2 = region->cb->read32(region, ofs + 4);

   return combine_for_target_endian(v1, v2);
}

or even

uin32_t invalid_read8(MemoryRegion *region, target_phys_addr_t ofs)
{
    do_unassigned_access(region->start + ofs, 0, 0, 0, 1);
    return 0;
}

without having to have supplementary opaque pointer caches, as we
currently do for e.g. subpage_t.

These can be entered into the device's MemoryCallbackInfo structure
so that we're sure that there's defined semantics for every access,
and the device need not invent its own invalid_read functions as
quite a few do now.

I will admit that I do not yet have a good replacement for IO_MEM_ROMD,
or toggling the read-only bit on a RAM region.  Or even for the way
that pc.c allocates RAM around the 640k-1M hole.


r~