From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55095) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZJddX-0006Hc-5N for qemu-devel@nongnu.org; Mon, 27 Jul 2015 04:19:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZJddT-00033G-VR for qemu-devel@nongnu.org; Mon, 27 Jul 2015 04:19:51 -0400 References: <1437566099-10004-1-git-send-email-lvivier@redhat.com> <1437726913-4534-1-git-send-email-lvivier@redhat.com> From: Laurent Vivier Message-ID: <55B5E99F.5090901@redhat.com> Date: Mon, 27 Jul 2015 10:19:43 +0200 MIME-Version: 1.0 In-Reply-To: <1437726913-4534-1-git-send-email-lvivier@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH v3] pci: allow 0 address for PCI IO/MEM regions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: qemu-devel@nongnu.org, David Gibson , qemu-ppc@nongnu.org, Michael Roth , "Michael S. Tsirkin" Alex, could you ACK this patch ? It's not perfect and it will be removed later, but for the moment it allows to hotplug PCI card in pseries. Laurent On 24/07/2015 10:35, Laurent Vivier wrote: > Some kernels program a 0 address for io regions. PCI 3.0 spec > section 6.2.5.1 doesn't seem to disallow this. > > based on patch by Michael Roth > > Add pci_allow_0_addr in MachineClass to conditionally > allow addr 0 for pseries, as this can break other architectures. > > This patch allows to hotplug PCI card in pseries machine, as the first > added card BAR0 is always set to 0 address. > > This as a temporary hack, waiting to fix PCI memory priorities for more > machine types... > > Signed-off-by: Laurent Vivier > --- > v2: move flag from PCIBus to MachineClass, rename it pci_allow_0_address > v3: change author, update commit message. > hw/pci/pci.c | 12 +++++++++--- > hw/ppc/spapr.c | 1 + > include/hw/boards.h | 3 ++- > 3 files changed, 12 insertions(+), 4 deletions(-) > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > index a017614..9f57aea 100644 > --- a/hw/pci/pci.c > +++ b/hw/pci/pci.c > @@ -38,6 +38,7 @@ > #include "hw/pci/msix.h" > #include "exec/address-spaces.h" > #include "hw/hotplug.h" > +#include "hw/boards.h" > > //#define DEBUG_PCI > #ifdef DEBUG_PCI > @@ -1065,6 +1066,10 @@ static pcibus_t pci_bar_address(PCIDevice *d, > pcibus_t new_addr, last_addr; > int bar = pci_bar(d, reg); > uint16_t cmd = pci_get_word(d->config + PCI_COMMAND); > + Object *machine = qdev_get_machine(); > + ObjectClass *oc = object_get_class(machine); > + MachineClass *mc = MACHINE_CLASS(oc); > + bool allow_0_address = mc->pci_allow_0_address; > > if (type & PCI_BASE_ADDRESS_SPACE_IO) { > if (!(cmd & PCI_COMMAND_IO)) { > @@ -1075,7 +1080,8 @@ static pcibus_t pci_bar_address(PCIDevice *d, > /* Check if 32 bit BAR wraps around explicitly. > * TODO: make priorities correct and remove this work around. > */ > - if (last_addr <= new_addr || new_addr == 0 || last_addr >= UINT32_MAX) { > + if (last_addr <= new_addr || last_addr >= UINT32_MAX || > + (!allow_0_address && new_addr == 0)) { > return PCI_BAR_UNMAPPED; > } > return new_addr; > @@ -1099,8 +1105,8 @@ static pcibus_t pci_bar_address(PCIDevice *d, > /* XXX: as we cannot support really dynamic > mappings, we handle specific values as invalid > mappings. */ > - if (last_addr <= new_addr || new_addr == 0 || > - last_addr == PCI_BAR_UNMAPPED) { > + if (last_addr <= new_addr || last_addr == PCI_BAR_UNMAPPED || > + (!allow_0_address && new_addr == 0)) { > return PCI_BAR_UNMAPPED; > } > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index a6f1947..bf0c64f 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -1835,6 +1835,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data) > mc->default_ram_size = 512 * M_BYTE; > mc->kvm_type = spapr_kvm_type; > mc->has_dynamic_sysbus = true; > + mc->pci_allow_0_address = true; > > fwc->get_dev_path = spapr_get_fw_dev_path; > nc->nmi_monitor_handler = spapr_nmi; > diff --git a/include/hw/boards.h b/include/hw/boards.h > index 2aec9cb..3f84afd 100644 > --- a/include/hw/boards.h > +++ b/include/hw/boards.h > @@ -100,7 +100,8 @@ struct MachineClass { > no_cdrom:1, > no_sdcard:1, > has_dynamic_sysbus:1, > - no_tco:1; > + no_tco:1, > + pci_allow_0_address:1; > int is_default; > const char *default_machine_opts; > const char *default_boot_order; >