Re: [Xen-devel] [PATCH 1/2] iommu VT-d: separate rmrr addition function

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

> From: elena.ufimtseva@xxxxxxxxxx [mailto:elena.ufimtseva@xxxxxxxxxx]
> Sent: Monday, March 09, 2015 10:43 PM
> 
> From: Elena Ufimtseva <elena.ufimtseva@xxxxxxxxxx>
> 
> In preparation for auxiliary RMRR data provided on Xen
> command line, make RMRR adding a separate function.
> No code changes.
> 
> Signed-off-by: Elena Ufimtseva <elena.ufimtseva@xxxxxxxxxx>

Acked-by: Kevin Tian <kevin.tian@xxxxxxxxx>, with one small comment.

> ---
>  xen/drivers/passthrough/vtd/dmar.c |  122
> +++++++++++++++++++-----------------
>  xen/drivers/passthrough/vtd/dmar.h |    1 +
>  2 files changed, 66 insertions(+), 57 deletions(-)
> 
> diff --git a/xen/drivers/passthrough/vtd/dmar.c
> b/xen/drivers/passthrough/vtd/dmar.c
> index 1152c3a..4c85732 100644
> --- a/xen/drivers/passthrough/vtd/dmar.c
> +++ b/xen/drivers/passthrough/vtd/dmar.c
> @@ -567,6 +567,66 @@ out:
>      return ret;
>  }
> 
> +int register_one_rmrr(struct acpi_rmrr_unit *rmrru)
> +{
> +    u8 b, d, f;
> +    bool_t ignore = 0;
> +    unsigned int i = 0;
> +    int ret = 0;
> +
> +    /* Skip checking if segment is not accessible yet. */
> +    if ( !pci_known_segment(rmrru->segment) )
> +        i = UINT_MAX;
> +
> +    for ( ; i < rmrru->scope.devices_cnt; i++ )
> +    {
> +        b = PCI_BUS(rmrru->scope.devices[i]);
> +        d = PCI_SLOT(rmrru->scope.devices[i]);
> +        f = PCI_FUNC(rmrru->scope.devices[i]);
> +
> +        if ( pci_device_detect(rmrru->segment, b, d, f) == 0 )
> +        {
> +            dprintk(XENLOG_WARNING VTDPREFIX,
> +                    " Non-existent device (%04x:%02x:%02x.%u) is
> reported"
> +                    " in RMRR (%"PRIx64", %"PRIx64")'s scope!\n",
> +                    rmrru->segment, b, d, f,
> +                    rmrru->base_address, rmrru->end_address);
> +            ignore = 1;
> +        }
> +        else
> +        {
> +            ignore = 0;
> +            break;
> +        }
> +    }
> +
> +    if ( ignore )
> +    {
> +        dprintk(XENLOG_WARNING VTDPREFIX,
> +            "  Ignore the RMRR (%"PRIx64", %"PRIx64") due to "
> +            "devices under its scope are not PCI discoverable!\n",
> +            rmrru->base_address, rmrru->end_address);
> +        ret = -EFAULT;
> +    }
> +    else if ( rmrru->base_address > rmrru->end_address )
> +    {
> +        dprintk(XENLOG_WARNING VTDPREFIX,
> +            "  The RMRR (%"PRIx64", %"PRIx64") is incorrect!\n",
> +            rmrru->base_address, rmrru->end_address);
> +        ret = -EFAULT;
> +    }
> +    else
> +    {
> +        if ( iommu_verbose )
> +            dprintk(VTDPREFIX,
> +                    "  RMRR region: base_addr %"PRIx64
> +                    " end_address %"PRIx64"\n",
> +                    rmrru->base_address, rmrru->end_address);
> +        acpi_register_rmrr_unit(rmrru);
> +    }
> +    return ret;
> +}
> +
>  static int __init
>  acpi_parse_one_rmrr(struct acpi_dmar_header *header)
>  {
> @@ -618,66 +678,14 @@ acpi_parse_one_rmrr(struct acpi_dmar_header
> *header)
>                                 &rmrru->scope, RMRR_TYPE,
> rmrr->segment);
> 
>      if ( ret || (rmrru->scope.devices_cnt == 0) )
> -        xfree(rmrru);
> -    else
>      {
> -        u8 b, d, f;
> -        bool_t ignore = 0;
> -        unsigned int i = 0;
> -
> -        /* Skip checking if segment is not accessible yet. */
> -        if ( !pci_known_segment(rmrr->segment) )
> -            i = UINT_MAX;
> -
> -        for ( ; i < rmrru->scope.devices_cnt; i++ )
> -        {
> -            b = PCI_BUS(rmrru->scope.devices[i]);
> -            d = PCI_SLOT(rmrru->scope.devices[i]);
> -            f = PCI_FUNC(rmrru->scope.devices[i]);
> -
> -            if ( pci_device_detect(rmrr->segment, b, d, f) == 0 )
> -            {
> -                dprintk(XENLOG_WARNING VTDPREFIX,
> -                        " Non-existent device (%04x:%02x:%02x.%u) is
> reported"
> -                        " in RMRR (%"PRIx64", %"PRIx64")'s scope!\n",
> -                        rmrr->segment, b, d, f,
> -                        rmrru->base_address, rmrru->end_address);
> -                ignore = 1;
> -            }
> -            else
> -            {
> -                ignore = 0;
> -                break;
> -            }
> -        }
> -
> -        if ( ignore )
> -        {
> -            dprintk(XENLOG_WARNING VTDPREFIX,
> -                "  Ignore the RMRR (%"PRIx64", %"PRIx64") due to "
> -                "devices under its scope are not PCI discoverable!\n",
> -                rmrru->base_address, rmrru->end_address);
> -            xfree(rmrru);
> -        }
> -        else if ( base_addr > end_addr )
> -        {
> -            dprintk(XENLOG_WARNING VTDPREFIX,
> -                "  The RMRR (%"PRIx64", %"PRIx64") is incorrect!\n",
> -                rmrru->base_address, rmrru->end_address);
> -            xfree(rmrru);
> -            ret = -EFAULT;
> -        }
> -        else
> -        {
> -            if ( iommu_verbose )
> -                dprintk(VTDPREFIX,
> -                        "  RMRR region: base_addr %"PRIx64
> -                        " end_address %"PRIx64"\n",
> -                        rmrru->base_address, rmrru->end_address);
> -            acpi_register_rmrr_unit(rmrru);
> -        }
> +        xfree(rmrru);
> +        return ret;
>      }
> 
> +    ret = register_one_rmrr(rmrru);
> +    if ( ret )
> +        xfree(rmrru);

a bit simplification here:

      if ( !ret && (rmrru->scope.devices_cnt != 0) )
                        ret = register_one_rmrr(rmrru);
                if ( ret )
                        xfree(rmrru);

>      return ret;
>  }
> 
> diff --git a/xen/drivers/passthrough/vtd/dmar.h
> b/xen/drivers/passthrough/vtd/dmar.h
> index af1feef..060aa04 100644
> --- a/xen/drivers/passthrough/vtd/dmar.h
> +++ b/xen/drivers/passthrough/vtd/dmar.h
> @@ -131,5 +131,6 @@ int vtd_hw_check(void);
>  void disable_pmr(struct iommu *iommu);
>  int is_usb_device(u16 seg, u8 bus, u8 devfn);
>  int is_igd_drhd(struct acpi_drhd_unit *drhd);
> +int register_one_rmrr(struct acpi_rmrr_unit *rmrru);
> 
>  #endif /* _DMAR_H_ */
> --
> 1.7.10.4


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