From mboxrd@z Thu Jan  1 00:00:00 1970
From: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Subject: Re: [PATCH 2/4] AMD IOMMU: cover all functions of a
 device even if ACPI only tells us of func 0
Date: Fri, 15 Feb 2013 12:21:40 -0500
Message-ID: <511E6EA4.9050009@oracle.com>
References: <511E6CAD02000078000BED19@nat28.tlf.novell.com>
	<511E6E5F02000078000BED4E@nat28.tlf.novell.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <511E6E5F02000078000BED4E@nat28.tlf.novell.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Jan Beulich <JBeulich@suse.com>
Cc: Sherry Hurwitz <sherry.hurwitz@amd.com>, xen-devel <xen-devel@lists.xen.org>
List-Id: xen-devel@lists.xenproject.org

On 02/15/2013 11:20 AM, Jan Beulich wrote:
>   
>   static int __init get_last_bdf_acpi(struct acpi_table_header *table)
>   {
>       const struct acpi_ivrs_header *ivrs_block;
>       unsigned long length = sizeof(struct acpi_table_ivrs);
> +    int last_bdf = 0;
>   
>       while ( table->length > (length + sizeof(*ivrs_block)) )
>       {
>           ivrs_block = (struct acpi_ivrs_header *)((u8 *)table + length);
>           if ( table->length < (length + ivrs_block->length) )
>               return -ENODEV;
> -        if ( ivrs_block->type == ACPI_IVRS_TYPE_HARDWARE &&
> -             get_last_bdf_ivhd(
> +        if ( ivrs_block->type == ACPI_IVRS_TYPE_HARDWARE )
> +        {
> +            int ret = get_last_bdf_ivhd(
>                    container_of(ivrs_block, const struct acpi_ivrs_hardware,
> -                              header)) != 0 )
> -            return -ENODEV;
> +                              header));
> +
> +            if ( ret < 0 )
> +                return ret;
> +            UPDATE_LAST_BDF(ret);

Why do we need UPDATE_LAST_BDF () here? It is updated in 
get_last_bdf_ivhd () above.

> +        }
>           length += ivrs_block->length;
>       }
> -   return 0;
> +
> +    return last_bdf;
>   }

...

> --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
> +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
> @@ -28,12 +28,38 @@
>   #include <asm/hvm/svm/amd-iommu-proto.h>
>   #include "../ats.h"
>   
> +static bool_t __read_mostly init_done;
> +
>   struct amd_iommu *find_iommu_for_device(int seg, int bdf)
>   {
>       struct ivrs_mappings *ivrs_mappings = get_ivrs_mappings(seg);
>   
> -    return ivrs_mappings && bdf < ivrs_bdf_entries ? ivrs_mappings[bdf].iommu
> -                                                   : NULL;
> +    if ( !ivrs_mappings || bdf >= ivrs_bdf_entries )
> +        return NULL;
> +
> +    if ( unlikely(!ivrs_mappings[bdf].iommu) && likely(init_done) )
> +    {
> +        unsigned int bd0 = bdf & ~PCI_FUNC(~0);
> +
> +        if ( ivrs_mappings[bd0].iommu )
> +        {
> +            struct ivrs_mappings tmp = ivrs_mappings[bd0];
> +
> +            tmp.iommu = NULL;
> +            if ( tmp.dte_requestor_id == bd0 )
> +                tmp.dte_requestor_id = bdf;

Is it possible to have tmp.dte_requestor_id != bd0


-boris