From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60146) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZINOY-0001JK-1q for qemu-devel@nongnu.org; Thu, 23 Jul 2015 16:47:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZINOX-0001ND-1D for qemu-devel@nongnu.org; Thu, 23 Jul 2015 16:47:10 -0400 Message-ID: <1437684406.7562.62.camel@kernel.crashing.org> From: Benjamin Herrenschmidt Date: Fri, 24 Jul 2015 06:46:46 +1000 In-Reply-To: <1437675858-14070-1-git-send-email-lvivier@redhat.com> References: <1437566099-10004-1-git-send-email-lvivier@redhat.com> <1437675858-14070-1-git-send-email-lvivier@redhat.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH v2] pci: allow 0 address for PCI IO/MEM regions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Laurent Vivier Cc: qemu-devel@nongnu.org, David Gibson , qemu-ppc@nongnu.org, Michael Roth , "Michael S. Tsirkin" On Thu, 2015-07-23 at 20:24 +0200, Laurent Vivier wrote: > From: Michael Roth > > Some kernels program a 0 address for io regions. PCI 3.0 spec > section 6.2.5.1 doesn't seem to disallow this. > > Signed-off-by: Michael Roth > [lvivier: add pci_allow_0_addr in MachineClass to conditionally > allow addr 0 for pseries, as this can break other architectures] > Signed-off-by: Laurent Vivier > --- Why would it break other architectures ? The PCI bus will forward address 0 just fine and some devices will decode it just fine too, regardless of the architecture they are put on. I don't see why having BARs capable of decoding it would break anything... > v2: move flag from PCIBus to MachineClass, rename it pci_allow_0_address > 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;