From patchwork Fri Aug 9 10:13:44 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alexey Kardashevskiy X-Patchwork-Id: 265979 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (unknown [IPv6:2001:4830:134:3::12]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id F17212C00D3 for ; Fri, 9 Aug 2013 20:14:21 +1000 (EST) Received: from localhost ([::1]:58913 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V7ji7-0007pW-QG for incoming@patchwork.ozlabs.org; Fri, 09 Aug 2013 06:14:19 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49760) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V7jhi-0007pD-Ny for qemu-devel@nongnu.org; Fri, 09 Aug 2013 06:13:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V7jhd-0003rX-If for qemu-devel@nongnu.org; Fri, 09 Aug 2013 06:13:54 -0400 Received: from mail-pa0-f47.google.com ([209.85.220.47]:43841) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V7jhd-0003rP-8p for qemu-devel@nongnu.org; Fri, 09 Aug 2013 06:13:49 -0400 Received: by mail-pa0-f47.google.com with SMTP id kl13so4767991pab.6 for ; Fri, 09 Aug 2013 03:13:48 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-gm-message-state:message-id:date:from:user-agent:mime-version:to :cc:subject:references:in-reply-to:content-type :content-transfer-encoding; bh=si42YlP7emNM9EGCUEPHy7Zj8lKbSv+/OCpsC3MhU2E=; b=NOthfFOlkL9o7vSVaGIPgi3CTYU92odBYazznLd7f5K2XroFobw/YuIcnlmAtQkI/Y 46KrV88cocNJbrC3S0PHbRpQpwG4AabBWGzjw4I+lCHSAPRs803I4YxRvalgM8kpQcze 7uVXX6uip49SLZjtwuIK24yG8jQLYid4RKwa0HXkTNwDhLl3nZNZiaZz7/pOayfabTUW vxK+/zZxAFM47RlxpXv44kZb/v9f7wCt5JZitE6Vq0hgpnSigaKPfYPIou89E1Dj7D8s NIwscODlERxmSnUHtcK4a/iahx4wKMfGDoLSJl+ALZpOtFqT1piW3MYwtzmPAd8pKhzo wpLw== X-Gm-Message-State: ALoCoQnDp21cxk7Jy3V3piY2KJzKxpPY5T7X32i06Md9eIwLgmmBkv5Ye/32re4rb1085kBiedJr X-Received: by 10.68.252.194 with SMTP id zu2mr10710111pbc.58.1376043228468; Fri, 09 Aug 2013 03:13:48 -0700 (PDT) Received: from aik.ozlabs.ibm.com (ibmaus65.lnk.telstra.net. [165.228.126.9]) by mx.google.com with ESMTPSA id nj9sm19314942pbc.13.2013.08.09.03.13.45 for (version=TLSv1 cipher=RC4-SHA bits=128/128); Fri, 09 Aug 2013 03:13:47 -0700 (PDT) Message-ID: <5204C0D8.5010205@ozlabs.ru> Date: Fri, 09 Aug 2013 20:13:44 +1000 From: Alexey Kardashevskiy User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130625 Thunderbird/17.0.7 MIME-Version: 1.0 To: Paolo Bonzini References: <1376038150-14527-1-git-send-email-aik@ozlabs.ru> <5204B8F9.5050201@redhat.com> <5204BAE0.8070507@ozlabs.ru> <5204BC34.8030405@redhat.com> In-Reply-To: <5204BC34.8030405@redhat.com> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.85.220.47 Cc: Anthony Liguori , "Michael S . Tsirkin" , qemu-devel@nongnu.org, David Gibson Subject: Re: [Qemu-devel] [PATCH v2] pci: Introduce helper to retrieve a PCI device's DMA address space X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org On 08/09/2013 07:53 PM, Paolo Bonzini wrote: > Il 09/08/2013 11:48, Alexey Kardashevskiy ha scritto: >> On 08/09/2013 07:40 PM, Paolo Bonzini wrote: >>> Il 09/08/2013 10:49, Alexey Kardashevskiy ha scritto: >>>> A PCI device's DMA address space (possibly an IOMMU) is returned by a >>>> method on the PCIBus. At the moment that only has one caller, so the >>>> method is simply open coded. We'll need another caller for VFIO, so >>>> this patch introduces a helper/wrapper function. >>>> >>>> Signed-off-by: David Gibson >>>> [aik: added inheritance from parent if iommu is not set for the current bus] >>>> Signed-off-by: Alexey Kardashevskiy >>>> >>>> --- >>>> Changes: >>>> v2: >>>> * added inheritance, needed for a pci-bridge on spapr-ppc64 >>>> * pci_iommu_as renamed to pci_device_iommu_address_space >>>> --- >>>> hw/pci/pci.c | 22 ++++++++++++++++------ >>>> include/hw/pci/pci.h | 1 + >>>> 2 files changed, 17 insertions(+), 6 deletions(-) >>>> >>>> diff --git a/hw/pci/pci.c b/hw/pci/pci.c >>>> index 4c004f5..0072b54 100644 >>>> --- a/hw/pci/pci.c >>>> +++ b/hw/pci/pci.c >>>> @@ -812,12 +812,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus, >>>> } >>>> >>>> pci_dev->bus = bus; >>>> - if (bus->iommu_fn) { >>>> - dma_as = bus->iommu_fn(bus, bus->iommu_opaque, devfn); >>>> - } else { >>>> - /* FIXME: inherit memory region from bus creator */ >>>> - dma_as = &address_space_memory; >>>> - } >>>> + dma_as = pci_device_iommu_address_space(pci_dev); >>>> >>>> memory_region_init_alias(&pci_dev->bus_master_enable_region, >>>> OBJECT(pci_dev), "bus master", >>>> @@ -2239,6 +2234,21 @@ static void pci_device_class_init(ObjectClass *klass, void *data) >>>> k->props = pci_props; >>>> } >>>> >>>> +AddressSpace *pci_device_iommu_address_space(PCIDevice *dev) >>>> +{ >>>> + PCIBus *bus = PCI_BUS(dev->bus); >>>> + >>>> + if (bus->iommu_fn) { >>>> + return bus->iommu_fn(bus, bus->iommu_opaque, dev->devfn); >>>> + } >>>> + >>>> + if (bus->parent_dev) { >>>> + return pci_device_iommu_address_space(bus->parent_dev); >>>> + } >>> >>> No, this would fail if bus->parent_dev is not NULL but not a PCI device >>> either. >> >> parent_dev is of the PCIDevice* type, how can it be not a PCI device? :-/ > > Doh, I misread the code, I thought it was the "parent" field in > BusState. Why do we have parent_dev at all? The code is too old? Don't know. >>> You can use object_dynamic_cast to convert the parent_dev to >>> PCIDevice, and if the cast succeeds you call the new function. >>> >>> Perhaps you could make the new function take a PCIBus instead. >>> Accessing the PCIDevice's IOMMU address space (as opposed to the >>> bus-master address space) doesn't make much sense, VFIO is really a >>> special case. Putting the new API on the bus side instead looks better. >>> >>> (BTW, do you need to enable bus-master DMA on PCI bridges, to do DMA for >>> devices sitting on the secondary bus?) >> >> It happens naturally I guess when linux enables devices. > > Yes, but then using the IOMMU address space would be wrong; you would > have to use the bus-master address space as a base for the child's > bus-master address space. Like this? Works too. diff --git a/hw/pci/pci.c b/hw/pci/pci.c index 8bdcedc..a4c70e6 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -2247,23 +2247,23 @@ AddressSpace *pci_device_iommu_address_space(PCIDevice *dev) { PCIBus *bus = PCI_BUS(dev->bus); if (bus->iommu_fn) { return bus->iommu_fn(bus, bus->iommu_opaque, dev->devfn); } if (bus->parent_dev) { return pci_device_iommu_address_space(bus->parent_dev); } - return &address_space_memory; + return &dev->bus_master_as; } > Also, we would have to fix the x86 firmware. > > Paolo >