From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MvA16-0000Xs-OI for qemu-devel@nongnu.org; Tue, 06 Oct 2009 09:23:48 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MvA11-0000Tk-GP for qemu-devel@nongnu.org; Tue, 06 Oct 2009 09:23:47 -0400 Received: from [199.232.76.173] (port=47205 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MvA11-0000Tc-DB for qemu-devel@nongnu.org; Tue, 06 Oct 2009 09:23:43 -0400 Received: from mx1.redhat.com ([209.132.183.28]:57075) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MvA10-0002RR-TW for qemu-devel@nongnu.org; Tue, 06 Oct 2009 09:23:43 -0400 Date: Tue, 6 Oct 2009 15:21:38 +0200 From: "Michael S. Tsirkin" Message-ID: <20091006132138.GA9681@redhat.com> References: <1254737223-16129-1-git-send-email-yamahata@valinux.co.jp> <1254737223-16129-17-git-send-email-yamahata@valinux.co.jp> <20091005114120.GA30477@redhat.com> <20091006100259.GE32367%yamahata@valinux.co.jp> <20091006105701.GC8916@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20091006105701.GC8916@redhat.com> Subject: [Qemu-devel] Re: [PATCH 16/23] pci: pcie host and mmcfg support. List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Isaku Yamahata Cc: qemu-devel@nongnu.org On Tue, Oct 06, 2009 at 12:57:01PM +0200, Michael S. Tsirkin wrote: > On Tue, Oct 06, 2009 at 07:02:59PM +0900, Isaku Yamahata wrote: > > On Mon, Oct 05, 2009 at 01:41:21PM +0200, Michael S. Tsirkin wrote: > > > On Mon, Oct 05, 2009 at 07:06:56PM +0900, Isaku Yamahata wrote: > > > > @@ -1052,9 +1242,10 @@ PCIDevice *pci_create_simple(PCIBus *bus, int devfn, const char *name) > > > > > > > > static int pci_find_space(PCIDevice *pdev, uint8_t size) > > > > { > > > > + int config_size = pcie_config_size(pdev); > > > > int offset = PCI_CONFIG_HEADER_SIZE; > > > > int i; > > > > - for (i = PCI_CONFIG_HEADER_SIZE; i < PCI_CONFIG_SPACE_SIZE; ++i) > > > > + for (i = PCI_CONFIG_HEADER_SIZE; i < config_size; ++i) > > > > if (pdev->used[i]) > > > > offset = i + 1; > > > > else if (i - offset + 1 == size) > > > > diff --git a/hw/pci.h b/hw/pci.h > > > > index 00f2b78..1f402d2 100644 > > > > --- a/hw/pci.h > > > > +++ b/hw/pci.h > > > > @@ -175,20 +175,26 @@ enum { > > > > QEMU_PCI_CAP_MSIX = 0x1, > > > > }; > > > > > > > > +/* Size of the standart PCIe config space: 4KB */ > > > > +#define PCIE_CONFIG_SPACE_SIZE 0x1000 > > > > +#define PCIE_EXT_CONFIG_SPACE_SIZE \ > > > > + (PCIE_CONFIG_SPACE_SIZE - PCI_CONFIG_SPACE_SIZE) > > > > + > > > > struct PCIDevice { > > > > DeviceState qdev; > > > > + > > > > /* PCI config space */ > > > > - uint8_t config[PCI_CONFIG_SPACE_SIZE]; > > > > + uint8_t *config; > > > > > > > > /* Used to enable config checks on load. Note that writeable bits are > > > > * never checked even if set in cmask. */ > > > > - uint8_t cmask[PCI_CONFIG_SPACE_SIZE]; > > > > + uint8_t *cmask; > > > > > > > > /* Used to implement R/W bytes */ > > > > - uint8_t wmask[PCI_CONFIG_SPACE_SIZE]; > > > > + uint8_t *wmask; > > > > > > > > /* Used to allocate config space for capabilities. */ > > > > - uint8_t used[PCI_CONFIG_SPACE_SIZE]; > > > > + uint8_t *used; > > > > > > > > /* the following fields are read only */ > > > > PCIBus *bus; > > > > > > > > > So I thought about this some more, and I think that this change > > > in unnecessary. PCI Express adds support for extended 4K > > > configuration space, but the only thing that must reside > > > there is the optional advanced error reporting capability, > > > which I don't think we'll need to implement, ever. > > > > > > Everything else can reside in the first 256 bytes, just as for regular > > > PCI. And pci code already returns 0 for accesses outside the first 256 > > > bytes, so express specific code is necessary. > > > > I agree with you for emulated PCI express device > > (which doesn't exist for now). However I oppose it for other reason. > > > > My purpose is to direct attach PCIe device to a guest including > > AER emulation somehow. > > For now, if I were you, I would just ignore AER. > > > When direct attaching PCIe native device to a guest, > > we don't have any control on how its configuration space is used. > > When an error is reported on host via AER, I'd like to pass > > the error to guest in some manner. So I want AER too in a sense. > > Since what you will want to do is forward stuff to a physical device, > you likely won't need to keep anything in memory at all. > So express code might just do > if (offset > 256) > write_to_physical_device(); > But, let's take these things one at a time. > For one, device assignment is not upstream at all. The point being, we'll know what's the best way to implement extended memory space when we see some code using it. > By the way, a general question: how was this tested? Did you manage to > make guest generate mmconfig transactions? Which guests do this? > > > -- > > yamahata