On 21 May 2013 11:57, Paolo Bonzini <pbonz...@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).
thanks
-- PMM
