On 14 September 2011 11:02, Avi Kivity<address@hidden> wrote:
> On 09/14/2011 12:56 PM, Peter Maydell wrote:
>>
>> On 14 September 2011 10:23, Avi Kivity<address@hidden> wrote:
>> > This patchset introduces memory_region_set_enabled() and
>> > memory_region_set_address() to avoid the requirement on memory
>> > routers to track the internal state of the memory API (so they know
>> > whether they need to add or remove a region). Instead, they can
>> > simply copy the state of the region from the guest-exposed register
>> > to the memory core, via the new mutator functions.
>> >
>> > Please review. Do we need a memory_region_set_size() as well?
>>
>> Would set_size() allow things like omap_gpmc() to avoid the need
>> to create an intermediate container subregion to enforce size
>> clipping on the child region it's trying to map?
>
> I'd recommend not calling _set_size() on somebody else's region - this
> quickly leads to confusion. Only call set_size() if you also called _init()
> and will call _destroy().
>
> Can you point me at the code in question?
hw/omap_gpmc.c:omap_gpmc_cs_map(). For each of the 8 children you
can connect to it, the GPMC has a base and mask register. The
hardware logic is effectively
if ((address& mask) == base) { send transaction to this child }
(complicated only slightly by the register for base only having
bits [29:24] with the others implied-zero, and the register for
mask only having bits [27:24].) The effect is that you can use
the mask value to set the size of the area the child is mapped in.
(Silly mask settings with "holes" are discouraged by the TRM,
and the current code doesn't handle them.)
The repeated-in-the-space effect happens if the child is smaller
than the space it's in: the child hardware just ignores the higher
bits of the address so appears multiple times.
>> (Strictly speaking what omap_gpmc() wants is not merely clipping
>> to a guest-specified size but also wrapping, so you can take a
>> 16MB child region and map the bottom 4MB of it repeating into
>> a 32MB chunk of address space, say. But that would require a lot
>> of playing games with aliases to implement a bizarre corner
>> case that nobody uses in practice.)
> That's best done in the memory core, the rendering loop can be adjusted to
> do this replication.
That would be nice, although as I say nobody is actually relying
on it so probably not worth the effort unless there's another user
for it.