On 2013-05-23 20:04, Peter Maydell wrote:
> On 21 May 2013 11:57, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> +bool address_space_valid(AddressSpace *as, hwaddr addr, int len, bool is_write)
>> +{
>> +    AddressSpaceDispatch *d = as->dispatch;
>> +    MemoryRegionSection *section;
>> +    int l;
>> +    hwaddr page;
>> +
>> +    while (len > 0) {
>> +        page = addr & TARGET_PAGE_MASK;
>> +        l = (page + TARGET_PAGE_SIZE) - addr;
>> +        if (l > len) {
>> +            l = len;
>> +        }
>> +        section = phys_page_find(d, addr >> TARGET_PAGE_BITS);
>> +        if (section->mr == &io_mem_unassigned ||
>> +            (is_write && section->mr->readonly)) {
> 
> It's kind of bogus that io_mem_unassigned is the only MemoryRegion
> that can be unreadable. Why is 'readonly' a MemoryRegion attribute
> and 'writeonly' not? Shouldn't we be calling the MemoryRegionOps
> accepts() callback here? What about access alignment constraints
> and access size restrictions? What if the validity of the range
> changes between the time you asked and when you actually do the
> access?
> 
> The whole API is kind of unconvincing really, especially since
> the only thing we seem to use it for is some parameter checking
> in spapr_llan.c (via a huge pile of intermediate wrappers).

I'll also have a use for it: replace isa_is_ioport_assigned.

Jan