On 4/23/10, Richard Henderson <r...@twiddle.net> wrote:
> 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?
If we had a bus layer that handled conversion from bus width to device
width (discussed earlier), it could as well handle the cases where
instead of conversion, a bus error should be reported.
> > 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.
Perhaps with small changes we could make this design stackable, so
that a device providing a bus inside another bus could use the same
registration functions, only the bus handle would change. Like:
upa = bus_register_memory_region(cpu_bus,,);
pci = bus_register_memory_region(upa,,);
ebus = bus_register_memory_region(pci,,):
ide = bus_register_memory_region(ebus,,):
Currently "cpu_bus" is always implicit and pci has separate
registration functions.
I'd suppose that ROMD, subpages and unassigned memory handlers could
be implemented by stacking instead of special casing them.
> * 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.
Read-only bit could be implemented in the stackable design by creating
a simple bus which can suppress writes when configured so, The device
behind would be just RAM.
Likewise for PC RAM mapping, the memory controller provides a bus
where the addresses may be twisted as needed before passing to
underlying RAM.
However, the outcome of the Generic DMA discussions (last year?) was
that instead of read/write functions, a mapping API would be better
because then we have the possibility to do zero copy DMA. Perhaps the
same applies here, too. This would be a bigger change obviously but if
there will be widespread changes to devices, it would be nice to get
this right.
