From patchwork Wed Oct 16 09:20:45 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Michael S. Tsirkin" X-Patchwork-Id: 283889 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id A748B2C025E for ; Wed, 16 Oct 2013 20:19:30 +1100 (EST) Received: from localhost ([::1]:46034 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VWNGK-0006X6-AB for incoming@patchwork.ozlabs.org; Wed, 16 Oct 2013 05:19:28 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33950) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VWNFX-0006R2-BJ for qemu-devel@nongnu.org; Wed, 16 Oct 2013 05:19:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VWNF7-0004yS-2J for qemu-devel@nongnu.org; Wed, 16 Oct 2013 05:18:39 -0400 Received: from mx1.redhat.com ([209.132.183.28]:51076) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VWNF6-0004y1-QP for qemu-devel@nongnu.org; Wed, 16 Oct 2013 05:18:12 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r9G9IAlm017327 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 16 Oct 2013 05:18:10 -0400 Received: from redhat.com (vpn1-6-247.ams2.redhat.com [10.36.6.247]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with SMTP id r9G9I72F028333; Wed, 16 Oct 2013 05:18:07 -0400 Date: Wed, 16 Oct 2013 12:20:45 +0300 From: "Michael S. Tsirkin" To: Igor Mammedov Message-ID: <20131016092045.GA21233@redhat.com> References: <1381913354-8815-1-git-send-email-imammedo@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1381913354-8815-1-git-send-email-imammedo@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: armbru@redhat.com, qemu-devel@nongnu.org, blauwirbel@gmail.com, kraxel@redhat.com, aliguori@amazon.com, pbonzini@redhat.com, afaerber@suse.de Subject: Re: [Qemu-devel] [PATCH 0/4] pc: inform SeaBIOS where 64-bit PCI hole start and PCI mappings cleanup 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 Wed, Oct 16, 2013 at 10:49:10AM +0200, Igor Mammedov wrote: > * simplify PCI address space mapping into system address space, > replacing code duplication in piix/q53 PCs with helper function I think this does not go far enough. I was always wondering about PCI hole in QEMU. Some real PCs have a "PCI hole" where PCI masks real memory, but PIIX does not do this, instead PCI is whenever RAM does not mask it. So it looks like the hole concept is a left-over from when we didn't have priorities in the memory API. How about we remove them? See patch below. I did a quick boot test and it seems to work, of course it needs much more testing. It's on top of Marcel's series adding negative priorities, so works on top of the acpi branch or the pci branch. I'm also wondering about the smram region - it uses priority 1 but does not say why. Need to check what does it overlap with, and why. diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c index bad3953..988516a 100644 --- a/hw/pci-host/piix.c +++ b/hw/pci-host/piix.c @@ -102,8 +102,6 @@ struct PCII440FXState { MemoryRegion *system_memory; MemoryRegion *pci_address_space; MemoryRegion *ram_memory; - MemoryRegion pci_hole; - MemoryRegion pci_hole_64bit; PAMMemoryRegion pam_regions[13]; MemoryRegion smram_region; uint8_t smm_enabled; @@ -326,7 +324,6 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, PCII440FXState *f; unsigned i; I440FXState *i440fx; - uint64_t pci_hole64_size; dev = qdev_create(NULL, TYPE_I440FX_PCI_HOST_BRIDGE); s = PCI_HOST_BRIDGE(dev); @@ -354,23 +351,9 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, i440fx->pci_info.w32.begin = 0xe0000000; } - memory_region_init_alias(&f->pci_hole, OBJECT(d), "pci-hole", f->pci_address_space, - pci_hole_start, pci_hole_size); - memory_region_add_subregion(f->system_memory, pci_hole_start, &f->pci_hole); - - pci_hole64_size = pci_host_get_hole64_size(i440fx->pci_hole64_size); - - pc_init_pci64_hole(&i440fx->pci_info, 0x100000000ULL + above_4g_mem_size, - pci_hole64_size); - memory_region_init_alias(&f->pci_hole_64bit, OBJECT(d), "pci-hole64", - f->pci_address_space, - i440fx->pci_info.w64.begin, - pci_hole64_size); - if (pci_hole64_size) { - memory_region_add_subregion(f->system_memory, - i440fx->pci_info.w64.begin, - &f->pci_hole_64bit); - } + /* Set to lower priority than RAM */ + memory_region_add_subregion_overlap(f->system_memory, 0x0, + f->pci_address_space, -1); memory_region_init_alias(&f->smram_region, OBJECT(d), "smram-region", f->pci_address_space, 0xa0000, 0x20000); memory_region_add_subregion_overlap(f->system_memory, 0xa0000,