[Xen-devel] Ping: [PATCH] AMD-Vi: allocate root table on demand

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 03.03.17 at 15:29, <JBeulich@xxxxxxxx> wrote:
> This was my originally intended fix for the AMD side of XSA-207:
> There's no need to unconditionally allocate the root table, and with
> that there's then also no way to leak it when a guest has no devices
> assigned.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> --- a/xen/drivers/passthrough/amd/iommu_map.c
> +++ b/xen/drivers/passthrough/amd/iommu_map.c
> @@ -636,11 +636,10 @@ int amd_iommu_map_page(struct domain *d,
>  {
>      bool_t need_flush = 0;
>      struct domain_iommu *hd = dom_iommu(d);
> +    int rc;
>      unsigned long pt_mfn[7];
>      unsigned int merge_level;
>  
> -    BUG_ON( !hd->arch.root_table );
> -
>      if ( iommu_use_hap_pt(d) )
>          return 0;
>  
> @@ -648,6 +647,13 @@ int amd_iommu_map_page(struct domain *d,
>  
>      spin_lock(&hd->arch.mapping_lock);
>  
> +    rc = amd_iommu_alloc_root(hd);
> +    if ( rc )
> +    {
> +        spin_unlock(&hd->arch.mapping_lock);
> +        return rc;
> +    }
> +
>      /* Since HVM domain is initialized with 2 level IO page table,
>       * we might need a deeper page table for lager gfn now */
>      if ( is_hvm_domain(d) )
> @@ -717,8 +723,6 @@ int amd_iommu_unmap_page(struct domain *
>      unsigned long pt_mfn[7];
>      struct domain_iommu *hd = dom_iommu(d);
>  
> -    BUG_ON( !hd->arch.root_table );
> -
>      if ( iommu_use_hap_pt(d) )
>          return 0;
>  
> @@ -726,6 +730,12 @@ int amd_iommu_unmap_page(struct domain *
>  
>      spin_lock(&hd->arch.mapping_lock);
>  
> +    if ( !hd->arch.root_table )
> +    {
> +        spin_unlock(&hd->arch.mapping_lock);
> +        return 0;
> +    }
> +
>      /* Since HVM domain is initialized with 2 level IO page table,
>       * we might need a deeper page table for lager gfn now */
>      if ( is_hvm_domain(d) )
> --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
> +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
> @@ -222,23 +222,29 @@ int __init amd_iov_detect(void)
>      return scan_pci_devices();
>  }
>  
> -static int allocate_domain_resources(struct domain_iommu *hd)
> +int amd_iommu_alloc_root(struct domain_iommu *hd)
>  {
> -    /* allocate root table */
> -    spin_lock(&hd->arch.mapping_lock);
> -    if ( !hd->arch.root_table )
> +    if ( unlikely(!hd->arch.root_table) )
>      {
>          hd->arch.root_table = alloc_amd_iommu_pgtable();
>          if ( !hd->arch.root_table )
> -        {
> -            spin_unlock(&hd->arch.mapping_lock);
>              return -ENOMEM;
> -        }
>      }
> -    spin_unlock(&hd->arch.mapping_lock);
> +
>      return 0;
>  }
>  
> +static int __must_check allocate_domain_resources(struct domain_iommu *hd)
> +{
> +    int rc;
> +
> +    spin_lock(&hd->arch.mapping_lock);
> +    rc = amd_iommu_alloc_root(hd);
> +    spin_unlock(&hd->arch.mapping_lock);
> +
> +    return rc;
> +}
> +
>  static int get_paging_mode(unsigned long entries)
>  {
>      int level = 1;
> @@ -259,14 +265,6 @@ static int amd_iommu_domain_init(struct
>  {
>      struct domain_iommu *hd = dom_iommu(d);
>  
> -    /* allocate page directroy */
> -    if ( allocate_domain_resources(hd) != 0 )
> -    {
> -        if ( hd->arch.root_table )
> -            free_domheap_page(hd->arch.root_table);
> -        return -ENOMEM;
> -    }
> -
>      /* For pv and dom0, stick with get_paging_mode(max_page)
>       * For HVM dom0, use 2 level page table at first */
>      hd->arch.paging_mode = is_hvm_domain(d) ?
> @@ -280,6 +278,9 @@ static void __hwdom_init amd_iommu_hwdom
>      unsigned long i; 
>      const struct amd_iommu *iommu;
>  
> +    if ( allocate_domain_resources(dom_iommu(d)) )
> +        BUG();
> +
>      if ( !iommu_passthrough && !need_iommu(d) )
>      {
>          int rc = 0;
> @@ -363,7 +364,7 @@ static int reassign_device(struct domain
>                             u8 devfn, struct pci_dev *pdev)
>  {
>      struct amd_iommu *iommu;
> -    int bdf;
> +    int bdf, rc;
>      struct domain_iommu *t = dom_iommu(target);
>  
>      bdf = PCI_BDF2(pdev->bus, pdev->devfn);
> @@ -385,10 +386,9 @@ static int reassign_device(struct domain
>          pdev->domain = target;
>      }
>  
> -    /* IO page tables might be destroyed after pci-detach the last device
> -     * In this case, we have to re-allocate root table for next pci-attach.*/
> -    if ( t->arch.root_table == NULL )
> -        allocate_domain_resources(t);
> +    rc = allocate_domain_resources(t);
> +    if ( rc )
> +        return rc;
>  
>      amd_iommu_setup_domain_device(target, iommu, devfn, pdev);
>      AMD_IOMMU_DEBUG("Re-assign %04x:%02x:%02x.%u from dom%d to dom%d\n",
> --- a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
> +++ b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
> @@ -56,6 +56,7 @@ int __must_check amd_iommu_map_page(stru
>                                      unsigned long mfn, unsigned int flags);
>  int __must_check amd_iommu_unmap_page(struct domain *d, unsigned long gfn);
>  u64 amd_iommu_get_next_table_from_pte(u32 *entry);
> +int __must_check amd_iommu_alloc_root(struct domain_iommu *hd);
>  int amd_iommu_reserve_domain_unity_map(struct domain *domain,
>                                         u64 phys_addr, unsigned long size,
>                                         int iw, int ir);



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel