From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51223) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZNHbL-0002T0-9t for qemu-devel@nongnu.org; Thu, 06 Aug 2015 05:36:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZNHbF-0002N4-TP for qemu-devel@nongnu.org; Thu, 06 Aug 2015 05:36:39 -0400 References: <1437566099-10004-1-git-send-email-lvivier@redhat.com> <1437726913-4534-1-git-send-email-lvivier@redhat.com> <55B5E99F.5090901@redhat.com> From: Laurent Vivier Message-ID: <55C315E3.60701@redhat.com> Date: Thu, 6 Aug 2015 10:08:03 +0200 MIME-Version: 1.0 In-Reply-To: <55B5E99F.5090901@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: Michael Roth , "Michael S. Tsirkin" , qemu-ppc@nongnu.org, qemu-devel@nongnu.org, David Gibson Ping ? On 27/07/2015 10:19, Laurent Vivier wrote: > 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; >> >