From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:56448)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <alex.williamson@redhat.com>) id 1TGw8p-0003Ac-0e
	for qemu-devel@nongnu.org; Wed, 26 Sep 2012 14:15:27 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <alex.williamson@redhat.com>) id 1TGw8j-0005tC-6j
	for qemu-devel@nongnu.org; Wed, 26 Sep 2012 14:15:22 -0400
Received: from mx1.redhat.com ([209.132.183.28]:37787)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <alex.williamson@redhat.com>) id 1TGw8i-0005rS-Ud
	for qemu-devel@nongnu.org; Wed, 26 Sep 2012 14:15:17 -0400
Message-ID: <1348683311.28860.215.camel@bling.home>
From: Alex Williamson <alex.williamson@redhat.com>
Date: Wed, 26 Sep 2012 12:15:11 -0600
In-Reply-To: <005e01cd9c11$7b967f10$72c37d30$@cs.wisc.edu>
References: <1348673453-3248-1-git-send-email-mjr@cs.wisc.edu>
	<1348676771.28860.199.camel@bling.home>
	<004401cd9c07$0b36d760$21a48620$@cs.wisc.edu>
	<1348678672.28860.204.camel@bling.home>
	<005901cd9c0f$4d3f9ef0$e7bedcd0$@cs.wisc.edu>
	<1348682122.28860.210.camel@bling.home>
	<005e01cd9c11$7b967f10$72c37d30$@cs.wisc.edu>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH v3] Align PCI capabilities in pci_find_space
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Matt Renzelmann <mjr@cs.wisc.edu>
Cc: qemu-devel@nongnu.org

On Wed, 2012-09-26 at 13:05 -0500, Matt Renzelmann wrote:
> > 
> > Mismatched uses of "size" here.  We need both the end of the range to
> > search and the size of the sub-range we're looking for.  Maybe start,
> > end, and size.  Thanks,
> > 
> 
> Ah of course, how's this:
> 
> static int pci_find_space(PCIDevice *pdev, uint32_t start,
>                           uint32_t end, uint32_t size)
> {
>     int offset = start;
>     int i;
>     uint32_t *dword_used = &pdev->used[start];
> 
>     /* This approach ensures the capability is dword-aligned, as 
>        required by the PCI and PCI-E specifications */
>     for (i = start; i < end; i += 4, dword_used++) { 
>         if (*dword_used)
>             offset = i + 4;
>         else if (i - offset + 4 >= size)
>             return offset;
>     }
> 
>     return 0;
> }

Looks reasonable to me

> static int pci_find_legacy_space(PCIDevice *pdev, uint8_t size) {
>     return pci_find_space(pdev, PCI_CONFIG_HEADER_SIZE,
>                           PCI_CONFIG_SPACE_SIZE, size);
> }
> 
> static int pci_find_express_space(PCIDevice *pdev, uint16_t size) {
>     assert (pci_config_size(pdev) >= PCIE_CONFIG_SPACE_SIZE);

This could be done generically in pci_find_space

assert(pci_config_size(pdev) >= end)

You could also add an alignment check

assert(!(start & 0x3))

Thanks,

Alex

>     return pci_find_space(pdev, PCI_CONFIG_SPACE_SIZE,
>                           PCIE_CONFIG_SPACE_SIZE, size);
> }
>