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

["Jan Beulich" <JBeulich@xxxxxxxx>]

Suravee? Or alternatively, Sherry, is Suravee unavailable for an
extended period of time?

Jan

>>> On 10.03.17 at 17:19, <JBeulich@xxxxxxxx> wrote:
>>>> 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 



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