From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qc0-f170.google.com ([209.85.216.170]:39350 "EHLO mail-qc0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751886AbaGKSAp (ORCPT ); Fri, 11 Jul 2014 14:00:45 -0400 Received: by mail-qc0-f170.google.com with SMTP id c9so1322637qcz.29 for ; Fri, 11 Jul 2014 11:00:41 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: <20140611060118.GB3169@yanx> <20140702210736.GB28852@google.com> <20140703131526.GG28852@google.com> <20140703221152.GH28852@google.com> From: Bjorn Helgaas Date: Fri, 11 Jul 2014 12:00:21 -0600 Message-ID: Subject: Re: [PATCH] pci: Allow very large resource windows To: Yinghai Lu Cc: Linus Torvalds , Greg Kroah-Hartman , Guo Chao , "linux-pci@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-pci-owner@vger.kernel.org List-ID: On Thu, Jul 10, 2014 at 7:12 PM, Yinghai Lu wrote: > On Thu, Jul 3, 2014 at 3:11 PM, Bjorn Helgaas wrote: >> On Thu, Jul 03, 2014 at 12:59:21PM -0700, Yinghai Lu wrote: > >>> when we try to size it, means that bar is not assigned. with r->flags >>> = 0, means >>> we will ignore it all the way. >> >> With "r->flags = 0", we will not try to assign resources to the BAR, >> but the hardware BAR still exists and contains some address. If the >> device has other memory BARs, pci_enable_resources() will turn on >> PCI_COMMAND_MEMORY. Now the "r->flags == 0" BAR is enabled but the >> address it contains might conflict with other devices. That's the >> problem. >> >> To fix this, I think we need to do "r->flags |= IORESOURCE_UNSET | >> IORESOURCE_DISABLED". > > No, that is not right. > > Current that r->flags = 0 and reset_resource() cover the bug in > pci_enable_resources() when assign and reassign resource fail. > > In pci_enable_resources() if there is one resource with IO BAR or SRIOV BAR > is not assigned and have UNSET, it will prevent device from being enabled. > > Most drivers could work without io port. > > If you change r->flags = 0 to > r->flags |= IORESOURCE_UNSET | IORESOURCE_DISABLED > > Those drivers would not work anymore. If a driver doesn't need IO ports, it should use pci_enable_device_mem(). Then pci_enable_device_flags() will build a BAR bitmask that does not include the IO BARs, and pci_enable_resources() will not look at the resources corresponding to those BARs at all (so it doesn't matter what r->flags is), and it will not turn on PCI_COMMAND_IO. > I also know one driver that could work with pref mmio, but mmio is not assigned. > --- silicon bug, that it exposes non-needed mmio bar. The driver might work, but turning on PCI_COMMAND_MEMORY when an MMIO BAR is not assigned is dangerous and could cause bus conflicts. If we can't safely turn on PCI_COMMAND_MEMORY and this driver stops working, well, I think that's tough luck. I explained why I think it is dangerous in the paragraph "With 'r->flags = 0', ..." above. This a question of how the hardware behaves, so if you think I'm wrong, you need to point out something in the spec that says the hardware works differently than how I think it does. > BTW I think we may need to clear the BAR in pci_claim_resource(). > > Maybe Linus or Greg have more idea about this. > Do we need to get more strict? will only enable PCI_COMMAND_MEMORY > unless all mmio BARs get assigned value? I think we should turn on PCI_COMMAND_MEMORY only when the driver requests IORESOURCE_MEM and all MMIO BARs are assigned. Bjorn