On 2013-05-24 08:13, Jan Kiszka wrote:
> On 2013-05-23 20:04, Peter Maydell wrote:
>> 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).
>
> I'll also have a use for it: replace isa_is_ioport_assigned.
But for this use case, something like
memory_region_access_valid(MemoryRegion *, ...) would be more helpful
because pci_address_space_io returns a memory region, not an address
space. Can we change it? dma_memory_valid could simply pass the address
space's root region.
Jan
--
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux
