From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:33716)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <jan.kiszka@siemens.com>) id 1UfpEu-00016e-7V
	for qemu-devel@nongnu.org; Fri, 24 May 2013 06:28:49 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <jan.kiszka@siemens.com>) id 1UfpEt-00081b-1q
	for qemu-devel@nongnu.org; Fri, 24 May 2013 06:28:48 -0400
Received: from thoth.sbs.de ([192.35.17.2]:24307)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <jan.kiszka@siemens.com>) id 1UfpEs-00080w-Nx
	for qemu-devel@nongnu.org; Fri, 24 May 2013 06:28:46 -0400
Message-ID: <519F40D1.1020600@siemens.com>
Date: Fri, 24 May 2013 12:28:33 +0200
From: Jan Kiszka <jan.kiszka@siemens.com>
MIME-Version: 1.0
References: <1369133851-1894-1-git-send-email-pbonzini@redhat.com>
	<1369133851-1894-16-git-send-email-pbonzini@redhat.com>
	<CAFEAcA-LTgN_W_2kSKUoBPtgyuEV0h=GDABAh+gqo6FHHQTshw@mail.gmail.com>
	<519F050C.7020903@web.de>
In-Reply-To: <519F050C.7020903@web.de>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH 15/30] memory: add address_space_valid
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>, qemu-devel@nongnu.org, David Gibson <david@gibson.dropbear.id.au>

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 <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.

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