WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [PATCH 6 of 6] amd iommu: enable ats devices

To: "Wei Wang" <wei.wang2@xxxxxxx>
Subject: Re: [Xen-devel] [PATCH 6 of 6] amd iommu: enable ats devices
From: "Jan Beulich" <JBeulich@xxxxxxxx>
Date: Wed, 02 Nov 2011 13:56:55 +0000
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Wed, 02 Nov 2011 06:58:35 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <b84967db0efb875bca4d.1319548058@xxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <patchbomb.1319548052@xxxxxxxxxxxx> <b84967db0efb875bca4d.1319548058@xxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
>>> On 25.10.11 at 15:07, Wei Wang <wei.wang2@xxxxxxx> wrote:
> # HG changeset patch
> # User Wei Wang <wei.wang2@xxxxxxx>
> # Date 1319472702 -7200
> # Node ID b84967db0efb875bca4d95f47fc77b46cd065665
> # Parent  8ec947b278afaf89acadf905237c95ba7b64524a
> amd iommu: enable ats devices
> 
> Signed-off-by: Wei Wang <wei.wang2@xxxxxxx 
> 
> diff -r 8ec947b278af -r b84967db0efb xen/drivers/passthrough/amd/iommu_map.c
> --- a/xen/drivers/passthrough/amd/iommu_map.c Mon Oct 24 18:11:40 2011 +0200
> +++ b/xen/drivers/passthrough/amd/iommu_map.c Mon Oct 24 18:11:42 2011 +0200
> @@ -369,6 +369,17 @@ void amd_iommu_set_root_page_table(
>      dte[0] = entry;
>  }
>  
> +void iommu_dte_set_iotlb(u32 *dte, u8 i)
> +{
> +    u32 entry;
> +
> +    entry = dte[3];
> +    set_field_in_reg_u32(!!i, entry,
> +                         IOMMU_DEV_TABLE_IOTLB_SUPPORT_MASK,
> +                         IOMMU_DEV_TABLE_IOTLB_SUPPORT_SHIFT, &entry);
> +    dte[3] = entry;
> +}
> +
>  void __init amd_iommu_set_intremap_table(
>      u32 *dte, u64 intremap_ptr, u8 int_valid)
>  {
> diff -r 8ec947b278af -r b84967db0efb 
> xen/drivers/passthrough/amd/pci_amd_iommu.c
> --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c     Mon Oct 24 18:11:40 
> 2011 +0200
> +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c     Mon Oct 24 18:11:42 
> 2011 
> +0200
> @@ -25,6 +25,7 @@
>  #include <asm/hvm/iommu.h>
>  #include <asm/amd-iommu.h>
>  #include <asm/hvm/svm/amd-iommu-proto.h>
> +#include "../x86/ats.h"
>  
>  struct amd_iommu *find_iommu_for_device(int seg, int bdf)
>  {
> @@ -81,11 +82,12 @@ static void disable_translation(u32 *dte
>  }
>  
>  static void amd_iommu_setup_domain_device(
> -    struct domain *domain, struct amd_iommu *iommu, int bdf)
> +    struct domain *domain, struct amd_iommu *iommu, u8 bus, u8 devfn)
>  {
>      void *dte;
>      unsigned long flags;
>      int req_id, valid = 1;
> +    int dte_i = 0;
>  
>      struct hvm_iommu *hd = domain_hvm_iommu(domain);
>  
> @@ -94,8 +96,11 @@ static void amd_iommu_setup_domain_devic
>      if ( iommu_passthrough && (domain->domain_id == 0) )
>          valid = 0;
>  
> +    if ( ats_enabled )
> +        dte_i = 1;
> +
>      /* get device-table entry */
> -    req_id = get_dma_requestor_id(iommu->seg, bdf);
> +    req_id = get_dma_requestor_id(iommu->seg, (bus << 8) | devfn);
>      dte = iommu->dev_table.buffer + (req_id * IOMMU_DEV_TABLE_ENTRY_SIZE);
>  
>      spin_lock_irqsave(&iommu->lock, flags);
> @@ -107,6 +112,9 @@ static void amd_iommu_setup_domain_devic
>              (u32 *)dte, page_to_maddr(hd->root_table), hd->domain_id,
>              hd->paging_mode, valid);
>  
> +        if ( pci_ats_device(iommu->seg, bus, devfn) && iommu->iotlb_support )
> +            iommu_dte_set_iotlb((u32 *)dte, dte_i);
> +
>          invalidate_dev_table_entry(iommu, req_id);
>          flush_command_buffer(iommu);
>  
> @@ -118,6 +126,21 @@ static void amd_iommu_setup_domain_devic
>      }
>  
>      spin_unlock_irqrestore(&iommu->lock, flags);
> +
> +    ASSERT(spin_is_locked(&pcidevs_lock));
> +
> +    if ( pci_ats_device(iommu->seg, bus, devfn) && ats_enabled &&

Iirc pci_ats_device() returning non-zero already implies ats_enabled.

> +         !pci_ats_enabled(iommu->seg, bus, devfn) )
> +    {
> +        struct pci_dev* pdev;

Formatting (need a newline here, plus asterisk and space again should
switch places).

> +        enable_ats_device(iommu->seg, bus, devfn);
> +
> +        ASSERT(spin_is_locked(&pcidevs_lock));
> +        pdev = pci_get_pdev(iommu->seg, bus, devfn);
> +
> +        if ( pdev )

Is it actually possible/valid for pdev to be NULL here? If not, you'd
want ASSERT()/BUG_ON() instead of if() here.

> +            amd_iommu_flush_iotlb(pdev, INV_IOMMU_ALL_PAGES_ADDRESS, 0);
> +    }
>  }
>  
>  static void __init amd_iommu_setup_dom0_device(struct pci_dev *pdev)
> @@ -126,7 +149,8 @@ static void __init amd_iommu_setup_dom0_
>      struct amd_iommu *iommu = find_iommu_for_device(pdev->seg, bdf);
>  
>      if ( likely(iommu != NULL) )
> -        amd_iommu_setup_domain_device(pdev->domain, iommu, bdf);
> +        amd_iommu_setup_domain_device(pdev->domain, iommu, pdev->bus, 
> +                                      pdev->devfn);
>      else
>          AMD_IOMMU_DEBUG("No iommu for device %04x:%02x:%02x.%u\n",
>                          pdev->seg, pdev->bus,
> @@ -261,21 +285,26 @@ static void __init amd_iommu_dom0_init(s
>      setup_dom0_pci_devices(d, amd_iommu_setup_dom0_device);
>  }
>  
> -static void amd_iommu_disable_domain_device(
> -    struct domain *domain, struct amd_iommu *iommu, int bdf)
> +void amd_iommu_disable_domain_device(struct domain *domain, 
> +                                     struct amd_iommu *iommu, u8 bus, u8 
> devfn)

If you really want to do this, then ...

>  {
>      void *dte;
>      unsigned long flags;
>      int req_id;
>  
>      BUG_ON ( iommu->dev_table.buffer == NULL );
> -    req_id = get_dma_requestor_id(iommu->seg, bdf);
> +
> +    req_id = get_dma_requestor_id(iommu->seg, (bus << 8) | devfn);

... you want to use PCI_BDF2() here.

>      dte = iommu->dev_table.buffer + (req_id * IOMMU_DEV_TABLE_ENTRY_SIZE);
>  
>      spin_lock_irqsave(&iommu->lock, flags);
>      if ( is_translation_valid((u32 *)dte) )
>      {
>          disable_translation((u32 *)dte);
> +
> +        if ( pci_ats_device(iommu->seg, bus, devfn) && iommu->iotlb_support )
> +            iommu_dte_set_iotlb((u32 *)dte, 0);
> +
>          invalidate_dev_table_entry(iommu, req_id);
>          flush_command_buffer(iommu);
>          AMD_IOMMU_DEBUG("Disable: device id = 0x%04x, "
> @@ -284,6 +313,12 @@ static void amd_iommu_disable_domain_dev
>                          domain_hvm_iommu(domain)->paging_mode);
>      }
>      spin_unlock_irqrestore(&iommu->lock, flags);
> +
> +    ASSERT(spin_is_locked(&pcidevs_lock));
> +
> +    if ( pci_ats_device(iommu->seg, bus, devfn) && ats_enabled && 

See above.

> +         pci_ats_enabled(iommu->seg, bus, devfn) )
> +       disable_ats_device(iommu->seg, bus, devfn); 
>  }
>  
>  static int reassign_device( struct domain *source, struct domain *target,
> @@ -310,7 +345,7 @@ static int reassign_device( struct domai
>          return -ENODEV;
>      }
>  
> -    amd_iommu_disable_domain_device(source, iommu, bdf);
> +    amd_iommu_disable_domain_device(source, iommu, bus, devfn);
>  
>      list_move(&pdev->domain_list, &target->arch.pdev_list);
>      pdev->domain = target;
> @@ -320,7 +355,7 @@ static int reassign_device( struct domai
>      if ( t->root_table == NULL )
>          allocate_domain_resources(t);
>  
> -    amd_iommu_setup_domain_device(target, iommu, bdf);
> +    amd_iommu_setup_domain_device(target, iommu, bus, devfn);
>      AMD_IOMMU_DEBUG("Re-assign %04x:%02x:%02x.%u from dom%d to dom%d\n",
>                      seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
>                      source->domain_id, target->domain_id);
> @@ -432,7 +467,8 @@ static int amd_iommu_add_device(struct p
>          return -ENODEV;
>      }
>  
> -    amd_iommu_setup_domain_device(pdev->domain, iommu, bdf);
> +    amd_iommu_setup_domain_device(pdev->domain, iommu, 
> +                                  pdev->bus, pdev->devfn);
>      return 0;
>  }
>  
> @@ -454,7 +490,8 @@ static int amd_iommu_remove_device(struc
>          return -ENODEV;
>      }
>  
> -    amd_iommu_disable_domain_device(pdev->domain, iommu, bdf);
> +    amd_iommu_disable_domain_device(pdev->domain, iommu, 
> +                                    pdev->bus, pdev->devfn);
>      return 0;
>  }
>  
> diff -r 8ec947b278af -r b84967db0efb 
> xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
> --- a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h   Mon Oct 24 18:11:40 
> 2011 +0200
> +++ b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h   Mon Oct 24 18:11:42 
> 2011 
> +0200
> @@ -75,6 +75,7 @@ void amd_iommu_set_intremap_table(
>      u32 *dte, u64 intremap_ptr, u8 int_valid);
>  void amd_iommu_set_root_page_table(
>      u32 *dte, u64 root_ptr, u16 domain_id, u8 paging_mode, u8 valid);
> +void iommu_dte_set_iotlb(u32 *dte, u8 i);
>  void invalidate_dev_table_entry(struct amd_iommu *iommu, u16 devic_id);
>  
>  /* send cmd to iommu */


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

<Prev in Thread] Current Thread [Next in Thread>
  • Re: [Xen-devel] [PATCH 6 of 6] amd iommu: enable ats devices, Jan Beulich <=