From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41623) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YdFkv-0000SA-OW for qemu-devel@nongnu.org; Wed, 01 Apr 2015 06:20:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YdFkq-00032d-4Y for qemu-devel@nongnu.org; Wed, 01 Apr 2015 06:20:17 -0400 Received: from mx1.redhat.com ([209.132.183.28]:57459) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YdFkp-00032D-Tz for qemu-devel@nongnu.org; Wed, 01 Apr 2015 06:20:12 -0400 Date: Wed, 1 Apr 2015 12:20:02 +0200 From: "Michael S. Tsirkin" Message-ID: <20150401121819-mutt-send-email-mst@redhat.com> References: <1427876112-12615-1-git-send-email-jasowang@redhat.com> <1427876112-12615-18-git-send-email-jasowang@redhat.com> <20150401110531-mutt-send-email-mst@redhat.com> <1427883138.20573.0@smtp.corp.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1427883138.20573.0@smtp.corp.redhat.com> Subject: Re: [Qemu-devel] [PATCH V5 17/18] pci: remove hard-coded bar size in msix_init_exclusive_bar() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jason Wang Cc: cornelia.huck@de.ibm.com, Keith Busch , Kevin Wolf , qemu-devel@nongnu.org, Stefan Hajnoczi On Wed, Apr 01, 2015 at 06:12:18PM +0800, Jason Wang wrote: > >> diff --git a/hw/pci/msix.c b/hw/pci/msix.c > >> index 24de260..8c6d8f3 100644 > >> --- a/hw/pci/msix.c > >> +++ b/hw/pci/msix.c > >> @@ -291,33 +291,44 @@ int msix_init(struct PCIDevice *dev, unsigned > >>short nentries, > >> } > >> int msix_init_exclusive_bar(PCIDevice *dev, unsigned short > >>nentries, > >> - uint8_t bar_nr) > >> + uint8_t bar_nr, bool legacy_layout) > >> { > >> int ret; > >> char *name; > >> - > >> - /* > >> - * Migration compatibility dictates that this remains a 4k > >> - * BAR with the vector table in the lower half and PBA in > >> - * the upper half. Do not use these elsewhere! > >> - */ > >> -#define MSIX_EXCLUSIVE_BAR_SIZE 4096 > >> -#define MSIX_EXCLUSIVE_BAR_TABLE_OFFSET 0 > >> -#define MSIX_EXCLUSIVE_BAR_PBA_OFFSET (MSIX_EXCLUSIVE_BAR_SIZE / 2) > >> -#define MSIX_EXCLUSIVE_CAP_OFFSET 0 > >> - > >> - if (nentries * PCI_MSIX_ENTRY_SIZE > > >>MSIX_EXCLUSIVE_BAR_PBA_OFFSET) { > >> - return -EINVAL; > >> + uint32_t bar_size; > >> + uint32_t bar_pba_offset; > >> + > >> + if (legacy_layout) { > >> + /* > >> + * Migration compatibility dictates that this remains a 4k > >> + * BAR with the vector table in the lower half and PBA in > >> + * the upper half. Do not use these elsewhere! > >> + */ > >> + bar_size = 4096; > >> + bar_pba_offset = bar_size / 2; > >> + > >> + if (nentries * PCI_MSIX_ENTRY_SIZE > bar_pba_offset) { > >> + return -EINVAL; > >> + } > > > >So this takes pains to behave in a compatible > >way when more than 128 vectors are requested. > >But since we only had up to 64 VQs, why does > >some a configuration make sense? > > This question could also be asked for the code even without this patch. Spec > does not clarify this and if we think vectors>=128 is not a valid > configuration with only 64 VQs, qemu should fail and quit instead of a > warning. Unfortunately we don't do this and leave a chance for user to use > it. I agree with this as a general design principle. And it's typically even better to just support what the user requested, even if it doesn't make sense. -- MST