From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:51869) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TMEWX-0000ZH-GD for qemu-devel@nongnu.org; Thu, 11 Oct 2012 04:53:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TMEWV-0001mE-Rm for qemu-devel@nongnu.org; Thu, 11 Oct 2012 04:53:45 -0400 Received: from mx1.redhat.com ([209.132.183.28]:42517) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TMEWV-0001m4-Jv for qemu-devel@nongnu.org; Thu, 11 Oct 2012 04:53:43 -0400 Message-ID: <5076890D.4070506@redhat.com> Date: Thu, 11 Oct 2012 10:53:33 +0200 From: Avi Kivity MIME-Version: 1.0 References: <1349800368-15228-1-git-send-email-avi@redhat.com> <1349800368-15228-24-git-send-email-avi@redhat.com> <507684A3.3070605@redhat.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3 23/23] pci: honor PCI_COMMAND_MASTER List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: liu ping fan Cc: Blue Swirl , Paolo Bonzini , qemu-devel@nongnu.org, Anthony Liguori , "Michael S. Tsirkin" On 10/11/2012 10:49 AM, liu ping fan wrote: > On Thu, Oct 11, 2012 at 4:34 PM, Avi Kivity wrote: >> On 10/11/2012 05:38 AM, liu ping fan wrote: >>> On Wed, Oct 10, 2012 at 12:32 AM, Avi Kivity wrote: >>>> Currently we ignore PCI_COMMAND_MASTER completely: DMA succeeds even when >>>> the bit is clear. >>>> >>>> Honor PCI_COMMAND_MASTER by inserting a memory region into the device's >>>> bus master address space, and tying its enable status to PCI_COMMAND_MASTER. >>>> >>>> Tested using >>>> >>>> setpci -s 03 COMMAND=3 >>>> >>>> while a ping was running on a NIC in slot 3. The kernel (Linux) detected >>>> the stall and recovered after the command >>>> >>>> setpci -s 03 COMMAND=7 >>>> >>>> was issued. >>>> >>>> Signed-off-by: Avi Kivity >>>> --- >>>> hw/pci.c | 13 +++++++++++-- >>>> hw/pci.h | 1 + >>>> 2 files changed, 12 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/hw/pci.c b/hw/pci.c >>>> index 8e8e030..7adf61b 100644 >>>> --- a/hw/pci.c >>>> +++ b/hw/pci.c >>>> @@ -782,7 +782,11 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus, >>>> /* FIXME: Make dma_context_fn use MemoryRegions instead, so this path is >>>> * taken unconditionally */ >>>> /* FIXME: inherit memory region from bus creator */ >>>> - address_space_init(&pci_dev->bus_master_as, get_system_memory()); >>>> + memory_region_init_alias(&pci_dev->bus_master_enable_region, "bus master", >>>> + get_system_memory(), 0, >>>> + memory_region_size(get_system_memory())); >>> >>> Could we achieve that hiding special mr from some address space? I >>> think one approach is to limit the iommu's lookup table, but that is >>> the guest's willing. The other one is use dummy-mr to overlap some >>> piece of region of system_memory. >>> This method will require changing the render sequence of alias mr. >>> >>> diff --git a/memory.c b/memory.c >>> index 2f68d67..cf67c66 100644 >>> --- a/memory.c >>> +++ b/memory.c >>> @@ -499,6 +499,11 @@ static void render_memory_region(FlatView *view, >>> >>> clip = addrrange_intersection(tmp, clip); >>> >>> + /* Render subregions in priority order. */ >>> + QTAILQ_FOREACH(subregion, &mr->subregions, subregions_link) { >>> + render_memory_region(view, subregion, base, clip, readonly); >>> + } >>> + >>> if (mr->alias) { >>> int128_subfrom(&base, int128_make64(mr->alias->addr)); >>> int128_subfrom(&base, int128_make64(mr->alias_offset)); >>> @@ -506,11 +511,6 @@ static void render_memory_region(FlatView *view, >>> return; >>> } >>> >>> - /* Render subregions in priority order. */ >>> - QTAILQ_FOREACH(subregion, &mr->subregions, subregions_link) { >>> - render_memory_region(view, subregion, base, clip, readonly); >>> - } >>> - >>> >> >> I don't really follow. We don't have any memory region that is both a >> container and an alias, so this change has no effect. Can you describe >> what you intend? >> > Suppose iommuA takes over only a subsection of system_memory, and > forbidden to touch other space. How can we achieve this model after > adopting this series? I will soon post patches. Basically an iommu is just another memory region, and it can be anywhere in the memory hierarchy except it cannot be a leaf (like an alias). -- error compiling committee.c: too many arguments to function