From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:39324)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <peterx@redhat.com>) id 1bNZ7F-0007R5-Bj
	for qemu-devel@nongnu.org; Thu, 14 Jul 2016 01:23:18 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <peterx@redhat.com>) id 1bNZ7B-0005kG-5S
	for qemu-devel@nongnu.org; Thu, 14 Jul 2016 01:23:17 -0400
Received: from mx1.redhat.com ([209.132.183.28]:34926)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <peterx@redhat.com>) id 1bNZ7B-0005jH-04
	for qemu-devel@nongnu.org; Thu, 14 Jul 2016 01:23:13 -0400
Date: Thu, 14 Jul 2016 13:23:05 +0800
From: Peter Xu <peterx@redhat.com>
Message-ID: <20160714052305.GA3065@pxdev.xzpeter.org>
References: <1467706769-12505-1-git-send-email-peterx@redhat.com>
	<1467706769-12505-15-git-send-email-peterx@redhat.com>
	<CABdVeACE_=mKx56LL8PmRNY8SrdzNSQMTXpQBPepN3ohjqYZ0w@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <CABdVeACE_=mKx56LL8PmRNY8SrdzNSQMTXpQBPepN3ohjqYZ0w@mail.gmail.com>
Subject: Re: [Qemu-devel] [PATCH v11 14/28] intel_iommu: Add support for PCI
 MSI remap
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: David Kiarie <davidkiarie4@gmail.com>
Cc: QEMU Developers <qemu-devel@nongnu.org>, imammedo@redhat.com, rth@twiddle.net, Eduardo Habkost <ehabkost@redhat.com>, jasowang@redhat.com, Marcel Apfelbaum <marcel@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, pbonzini@redhat.com, Jan Kiszka <jan.kiszka@web.de>, rkrcmar@redhat.com, Alex Williamson <alex.williamson@redhat.com>, wexu@redhat.com

On Wed, Jul 13, 2016 at 04:17:01PM +0300, David Kiarie wrote:

[...]

> > +static MemTxResult vtd_mem_ir_read(void *opaque, hwaddr addr,
> > +                                   uint64_t *data, unsigned size,
> > +                                   MemTxAttrs attrs)
> > +{
> > +    addr += VTD_INTERRUPT_ADDR_FIRST;
> > +
> > +    VTD_DPRINTF(IR, "read mem_ir addr 0x%"PRIx64 " size %u",
> > +                addr, size);
> > +
> > +    if (dma_memory_read(&address_space_memory, addr, &data, size)) {
> > +        VTD_DPRINTF(GENERAL, "error: fail to access 0x%"PRIx64, addr);
> > +        return MEMTX_ERROR;
> > +    }
> > +
> > +    return MEMTX_OK;
> > +}
> 
> I'm looking at this and wondering whether dma_memory_read expected a
> double pointer as the third argument. (??)

Right. It's dangerous and should never be touched. Maybe I should keep
this an empty function, just like what APIC is doing. Read at
address_space_memory is odd here in all cases...

Thanks, David.

-- peterx