[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v8 1/2] IOMMU/VT-d: wire common device reserved memory API



On Thu, Sep 29, 2022 at 03:33:12PM +0200, Marek Marczykowski-Górecki wrote:
> Re-use rmrr= parameter handling code to handle common device reserved
> memory.
> 
> Move MAX_USER_RMRR_PAGES limit enforcement to apply only to
> user-configured ranges, but not those from internal callers.
> 
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
> Reviewed-by: Kevin Tian <kevin.tian@xxxxxxxxx>

Henry, can this be included in 4.17? The AMD counterpart went in
earlier, but due to late review on Intel part, this one didn't. 

> ---
> Changes in v8:
> - move add_one_user_rmrr() function earlier
> - extend commit message
> Changes in v3:
> - make MAX_USER_RMRR_PAGES applicable only to user-configured RMRR
> ---
>  xen/drivers/passthrough/vtd/dmar.c | 196 +++++++++++++++++-------------
>  1 file changed, 114 insertions(+), 82 deletions(-)
> 
> diff --git a/xen/drivers/passthrough/vtd/dmar.c 
> b/xen/drivers/passthrough/vtd/dmar.c
> index 367304c8739c..78c8bad1515a 100644
> --- a/xen/drivers/passthrough/vtd/dmar.c
> +++ b/xen/drivers/passthrough/vtd/dmar.c
> @@ -861,113 +861,136 @@ static struct user_rmrr __initdata 
> user_rmrrs[MAX_USER_RMRR];
>  
>  /* Macro for RMRR inclusive range formatting. */
>  #define ERMRRU_FMT "[%lx-%lx]"
> -#define ERMRRU_ARG(eru) eru.base_pfn, eru.end_pfn
> +#define ERMRRU_ARG base_pfn, end_pfn
>  
> -static int __init add_user_rmrr(void)
> +/* Returns 1 on success, 0 when ignoring and < 0 on error. */
> +static int __init add_one_user_rmrr(unsigned long base_pfn,
> +                                    unsigned long end_pfn,
> +                                    unsigned int dev_count,
> +                                    uint32_t *sbdf)
>  {
>      struct acpi_rmrr_unit *rmrr, *rmrru;
> -    unsigned int idx, seg, i;
> -    unsigned long base, end;
> +    unsigned int idx, seg;
> +    unsigned long base_iter;
>      bool overlap;
>  
> -    for ( i = 0; i < nr_rmrr; i++ )
> +    if ( iommu_verbose )
> +        printk(XENLOG_DEBUG VTDPREFIX
> +               "Adding RMRR for %d device ([0]: %#x) range "ERMRRU_FMT"\n",
> +               dev_count, sbdf[0], ERMRRU_ARG);
> +
> +    if ( base_pfn > end_pfn )
>      {
> -        base = user_rmrrs[i].base_pfn;
> -        end = user_rmrrs[i].end_pfn;
> +        printk(XENLOG_ERR VTDPREFIX
> +               "Invalid RMRR Range "ERMRRU_FMT"\n",
> +               ERMRRU_ARG);
> +        return 0;
> +    }
>  
> -        if ( base > end )
> +    overlap = false;
> +    list_for_each_entry(rmrru, &acpi_rmrr_units, list)
> +    {
> +        if ( pfn_to_paddr(base_pfn) <= rmrru->end_address &&
> +             rmrru->base_address <= pfn_to_paddr(end_pfn) )
>          {
>              printk(XENLOG_ERR VTDPREFIX
> -                   "Invalid RMRR Range "ERMRRU_FMT"\n",
> -                   ERMRRU_ARG(user_rmrrs[i]));
> -            continue;
> +                   "Overlapping RMRRs: "ERMRRU_FMT" and [%lx-%lx]\n",
> +                   ERMRRU_ARG,
> +                   paddr_to_pfn(rmrru->base_address),
> +                   paddr_to_pfn(rmrru->end_address));
> +            overlap = true;
> +            break;
>          }
> +    }
> +    /* Don't add overlapping RMRR. */
> +    if ( overlap )
> +        return 0;
>  
> -        if ( (end - base) >= MAX_USER_RMRR_PAGES )
> +    base_iter = base_pfn;
> +    do
> +    {
> +        if ( !mfn_valid(_mfn(base_iter)) )
>          {
>              printk(XENLOG_ERR VTDPREFIX
> -                   "RMRR range "ERMRRU_FMT" exceeds "\
> -                   __stringify(MAX_USER_RMRR_PAGES)" pages\n",
> -                   ERMRRU_ARG(user_rmrrs[i]));
> -            continue;
> +                   "Invalid pfn in RMRR range "ERMRRU_FMT"\n",
> +                   ERMRRU_ARG);
> +            break;
>          }
> +    } while ( base_iter++ < end_pfn );
>  
> -        overlap = false;
> -        list_for_each_entry(rmrru, &acpi_rmrr_units, list)
> -        {
> -            if ( pfn_to_paddr(base) <= rmrru->end_address &&
> -                 rmrru->base_address <= pfn_to_paddr(end) )
> -            {
> -                printk(XENLOG_ERR VTDPREFIX
> -                       "Overlapping RMRRs: "ERMRRU_FMT" and [%lx-%lx]\n",
> -                       ERMRRU_ARG(user_rmrrs[i]),
> -                       paddr_to_pfn(rmrru->base_address),
> -                       paddr_to_pfn(rmrru->end_address));
> -                overlap = true;
> -                break;
> -            }
> -        }
> -        /* Don't add overlapping RMRR. */
> -        if ( overlap )
> -            continue;
> +    /* Invalid pfn in range as the loop ended before end_pfn was reached. */
> +    if ( base_iter <= end_pfn )
> +        return 0;
>  
> -        do
> -        {
> -            if ( !mfn_valid(_mfn(base)) )
> -            {
> -                printk(XENLOG_ERR VTDPREFIX
> -                       "Invalid pfn in RMRR range "ERMRRU_FMT"\n",
> -                       ERMRRU_ARG(user_rmrrs[i]));
> -                break;
> -            }
> -        } while ( base++ < end );
> +    rmrr = xzalloc(struct acpi_rmrr_unit);
> +    if ( !rmrr )
> +        return -ENOMEM;
>  
> -        /* Invalid pfn in range as the loop ended before end_pfn was 
> reached. */
> -        if ( base <= end )
> -            continue;
> +    rmrr->scope.devices = xmalloc_array(u16, dev_count);
> +    if ( !rmrr->scope.devices )
> +    {
> +        xfree(rmrr);
> +        return -ENOMEM;
> +    }
>  
> -        rmrr = xzalloc(struct acpi_rmrr_unit);
> -        if ( !rmrr )
> -            return -ENOMEM;
> +    seg = 0;
> +    for ( idx = 0; idx < dev_count; idx++ )
> +    {
> +        rmrr->scope.devices[idx] = sbdf[idx];
> +        seg |= PCI_SEG(sbdf[idx]);
> +    }
> +    if ( seg != PCI_SEG(sbdf[0]) )
> +    {
> +        printk(XENLOG_ERR VTDPREFIX
> +               "Segments are not equal for RMRR range "ERMRRU_FMT"\n",
> +               ERMRRU_ARG);
> +        scope_devices_free(&rmrr->scope);
> +        xfree(rmrr);
> +        return 0;
> +    }
>  
> -        rmrr->scope.devices = xmalloc_array(u16, user_rmrrs[i].dev_count);
> -        if ( !rmrr->scope.devices )
> -        {
> -            xfree(rmrr);
> -            return -ENOMEM;
> -        }
> +    rmrr->segment = seg;
> +    rmrr->base_address = pfn_to_paddr(base_pfn);
> +    /* Align the end_address to the end of the page */
> +    rmrr->end_address = pfn_to_paddr(end_pfn) | ~PAGE_MASK;
> +    rmrr->scope.devices_cnt = dev_count;
>  
> -        seg = 0;
> -        for ( idx = 0; idx < user_rmrrs[i].dev_count; idx++ )
> -        {
> -            rmrr->scope.devices[idx] = user_rmrrs[i].sbdf[idx];
> -            seg |= PCI_SEG(user_rmrrs[i].sbdf[idx]);
> -        }
> -        if ( seg != PCI_SEG(user_rmrrs[i].sbdf[0]) )
> -        {
> -            printk(XENLOG_ERR VTDPREFIX
> -                   "Segments are not equal for RMRR range "ERMRRU_FMT"\n",
> -                   ERMRRU_ARG(user_rmrrs[i]));
> -            scope_devices_free(&rmrr->scope);
> -            xfree(rmrr);
> -            continue;
> -        }
> +    if ( register_one_rmrr(rmrr) )
> +        printk(XENLOG_ERR VTDPREFIX
> +               "Could not register RMMR range "ERMRRU_FMT"\n",
> +               ERMRRU_ARG);
>  
> -        rmrr->segment = seg;
> -        rmrr->base_address = pfn_to_paddr(user_rmrrs[i].base_pfn);
> -        /* Align the end_address to the end of the page */
> -        rmrr->end_address = pfn_to_paddr(user_rmrrs[i].end_pfn) | ~PAGE_MASK;
> -        rmrr->scope.devices_cnt = user_rmrrs[i].dev_count;
> +    return 1;
> +}
>  
> -        if ( register_one_rmrr(rmrr) )
> -            printk(XENLOG_ERR VTDPREFIX
> -                   "Could not register RMMR range "ERMRRU_FMT"\n",
> -                   ERMRRU_ARG(user_rmrrs[i]));
> -    }
> +static int __init add_user_rmrr(void)
> +{
> +    unsigned int i;
> +    int ret;
>  
> +    for ( i = 0; i < nr_rmrr; i++ )
> +    {
> +        ret = add_one_user_rmrr(user_rmrrs[i].base_pfn,
> +                                user_rmrrs[i].end_pfn,
> +                                user_rmrrs[i].dev_count,
> +                                user_rmrrs[i].sbdf);
> +        if ( ret < 0 )
> +            return ret;
> +    }
>      return 0;
>  }
>  
> +static int __init cf_check add_one_extra_rmrr(xen_pfn_t start, xen_ulong_t 
> nr, u32 id, void *ctxt)
> +{
> +    u32 sbdf_array[] = { id };
> +    return add_one_user_rmrr(start, start+nr, 1, sbdf_array);
> +}
> +
> +static int __init add_extra_rmrr(void)
> +{
> +    return iommu_get_extra_reserved_device_memory(add_one_extra_rmrr, NULL);
> +}
> +
>  #include <asm/tboot.h>
>  /* ACPI tables may not be DMA protected by tboot, so use DMAR copy */
>  /* SINIT saved in SinitMleData in TXT heap (which is DMA protected) */
> @@ -1010,7 +1033,7 @@ int __init acpi_dmar_init(void)
>      {
>          iommu_init_ops = &intel_iommu_init_ops;
>  
> -        return add_user_rmrr();
> +        return add_user_rmrr() || add_extra_rmrr();
>      }
>  
>      return ret;
> @@ -1108,6 +1131,15 @@ static int __init cf_check parse_rmrr_param(const char 
> *str)
>          else
>              end = start;
>  
> +        if ( (end - start) >= MAX_USER_RMRR_PAGES )
> +        {
> +            printk(XENLOG_ERR VTDPREFIX
> +                    "RMRR range "ERMRRU_FMT" exceeds "\
> +                    __stringify(MAX_USER_RMRR_PAGES)" pages\n",
> +                    start, end);
> +            return -E2BIG;
> +        }
> +
>          user_rmrrs[nr_rmrr].base_pfn = start;
>          user_rmrrs[nr_rmrr].end_pfn = end;
>  
> -- 
> git-series 0.9.1
> 

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

Attachment: signature.asc
Description: PGP signature


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.