From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH] pci-assign: Fix multifunction support Date: Tue, 17 Jan 2012 14:08:33 +0100 Message-ID: <4F1572D1.5020704@siemens.com> References: <20120116171141.15543.17245.stgit@bling.home> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Cc: "kvm@vger.kernel.org" , "mtosatti@redhat.com" To: Alex Williamson Return-path: Received: from thoth.sbs.de ([192.35.17.2]:21432 "EHLO thoth.sbs.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753473Ab2AQNIh (ORCPT ); Tue, 17 Jan 2012 08:08:37 -0500 In-Reply-To: <20120116171141.15543.17245.stgit@bling.home> Sender: kvm-owner@vger.kernel.org List-ID: On 2012-01-16 18:11, Alex Williamson wrote: > The core PCI code sets the multifunction bit in the header before > calling the device initfn. For device assignment, we're blasting > that value with the actual hardware value, so nobody sees the > additional functions if the devices isn't physically multifunction. > Switch the HEADER_TYPE to a fully emulated field (all read-only > anyway) and add setting and clearing of the multifunction bit to > match qemu directive. > > Signed-off-by: Alex Williamson > --- > > hw/device-assignment.c | 8 +++++++- > 1 files changed, 7 insertions(+), 1 deletions(-) > > diff --git a/hw/device-assignment.c b/hw/device-assignment.c > index 2a9e66d..7f4a5ec 100644 > --- a/hw/device-assignment.c > +++ b/hw/device-assignment.c > @@ -540,6 +540,13 @@ again: > fprintf(stderr, "%s: read failed, errno = %d\n", __func__, errno); > } > > + /* Restore or clear multifunction, this is always controlled by qemu */ > + if (pci_dev->dev.cap_present & QEMU_PCI_CAP_MULTIFUNCTION) { > + pci_dev->dev.config[PCI_HEADER_TYPE] |= PCI_HEADER_TYPE_MULTI_FUNCTION; > + } else { > + pci_dev->dev.config[PCI_HEADER_TYPE] &= ~PCI_HEADER_TYPE_MULTI_FUNCTION; > + } > + Why have this in get_*real*_device? Why not fix this up at the caller site, i.e. in assigned_initfn? Just for consistency, not a functional issue. > /* Clear host resource mapping info. If we choose not to register a > * BAR, such as might be the case with the option ROM, we can get > * confusing, unwritable, residual addresses from the host here. */ > @@ -1575,7 +1582,6 @@ static int assigned_initfn(struct PCIDevice *pci_dev) > assigned_dev_direct_config_read(dev, PCI_CLASS_PROG, 3); > assigned_dev_direct_config_read(dev, PCI_CACHE_LINE_SIZE, 1); > assigned_dev_direct_config_read(dev, PCI_LATENCY_TIMER, 1); > - assigned_dev_direct_config_read(dev, PCI_HEADER_TYPE, 1); > assigned_dev_direct_config_read(dev, PCI_BIST, 1); > assigned_dev_direct_config_read(dev, PCI_CARDBUS_CIS, 4); > assigned_dev_direct_config_read(dev, PCI_SUBSYSTEM_VENDOR_ID, 2); > Looks good otherwise. Is it a regression of the access control refactoring? Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux