Re: [Qemu-devel] [RFC][PATCH 09/15] memory: Introduce address_space_lookup_region

[Paolo Bonzini]

Il 06/05/2013 16:51, Jan Kiszka ha scritto:
> On 2013-05-06 16:39, Paolo Bonzini wrote:
>> Il 06/05/2013 16:26, Jan Kiszka ha scritto:
>>> This new service so far only replaces phys_page_find as public API. In a
>>> follow-up step, it will return the effective memory region for the
>>> specified address, i.e. after resolving what are currently sub-pages.
>>> Moreover, it will also once encapsulate locking and reference counting
>>> when we introduce BQL-free dispatching.
>>
>> In my IOMMU rebase I have a similar function:
>>
>> /* address_space_translate: translate an address range into an address space
>>  * into a MemoryRegionSection and an address range into that section.
>>  *
>>  * @as: #AddressSpace to be accessed
>>  * @addr: address within that address space
>>  * @xlat: pointer to address within the returned memory region section's
>>  * #MemoryRegion.
>>  * @len: pointer to length
>>  * @is_write: indicates the transfer direction
>>  */
>> MemoryRegionSection *address_space_translate(AddressSpace *as, hwaddr addr,
>>                                              hwaddr *xlat, hwaddr *len,
>>                                              bool is_write);
>>
>> It wraps (actually, replaces) both phys_page_find and
>> memory_region_section_addr.
> 
> Good, looks like we are heading in similar directions. What is the
> purpose of len? When does is_write matter?

Both matter when adding the IOMMU.  is_write is needed to check for
permissions, and len because the translation will be valid for one page
only (not for the full size of the MemoryRegion.

I can implement address_space_translate on top of
address_space_lookup_region and include your next patch too, so we're
fine here.

Paolo

> In a later step, this should become something like
> address_space_get_region_ref (to be paired with memory_region_unref,
> once done). So this one also takes care of incrementing the reference
> counter or acquiring the BQL, as necessary. Currently, it asks the
> caller to specify if the BQL is already held, but that will change.
> 
> Jan
>