xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Julien Grall <julien.grall@arm.com>
To: Oleksandr Tyshchenko <olekstysh@gmail.com>,
	xen-devel@lists.xenproject.org
Cc: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>,
	sstabellini@kernel.org
Subject: Re: [Xen-devel] [PATCH V2 2/6] iommu/arm: Add ability to handle deferred probing request
Date: Mon, 12 Aug 2019 12:11:12 +0100	[thread overview]
Message-ID: <3c6d54f2-06ad-6bd1-447b-0e4cbef3d391@arm.com> (raw)
In-Reply-To: <1564763985-20312-3-git-send-email-olekstysh@gmail.com>

Hi Oleksandr,

On 02/08/2019 17:39, Oleksandr Tyshchenko wrote:
> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> 
> This patch adds minimal required support to General IOMMU framework
> to be able to handle a case when IOMMU driver requesting deferred
> probing for a device.
> 
> In order not to pull Linux's error code (-EPROBE_DEFER) to Xen
> we have chosen -EAGAIN to be used for indicating that device
> probing is deferred.
> 
> This is needed for the upcoming IPMMU driver which may request
> deferred probing depending on what device will be probed the first
> (there is some dependency between these devices, Root device must be
> registered before Cache devices. If not the case, driver will deny
> further Cache device probes until Root device is registered).
> As we can't guarantee a fixed pre-defined order for the device nodes
> in DT, we need to be ready for the situation where devices being
> probed in "any" order.
> 
> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> ---
>   xen/common/device_tree.c            |  1 +
>   xen/drivers/passthrough/arm/iommu.c | 35 ++++++++++++++++++++++++++++++++++-
>   xen/include/asm-arm/device.h        |  6 +++++-
>   xen/include/xen/device_tree.h       |  1 +
>   4 files changed, 41 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> index e107c6f..6f37448 100644
> --- a/xen/common/device_tree.c
> +++ b/xen/common/device_tree.c
> @@ -1774,6 +1774,7 @@ static unsigned long __init unflatten_dt_node(const void *fdt,
>           /* By default the device is not protected */
>           np->is_protected = false;
>           INIT_LIST_HEAD(&np->domain_list);
> +        INIT_LIST_HEAD(&np->deferred_probe);

I am not entirely happy to add a new list_head field per node just for the 
benefits of boot code. Could we re-use domain_list (with a comment in the code 
and appropriate ASSERT)?

>   
>           if ( new_format )
>           {
> diff --git a/xen/drivers/passthrough/arm/iommu.c b/xen/drivers/passthrough/arm/iommu.c
> index 2135233..3195919 100644
> --- a/xen/drivers/passthrough/arm/iommu.c
> +++ b/xen/drivers/passthrough/arm/iommu.c
> @@ -20,6 +20,12 @@
>   #include <xen/device_tree.h>
>   #include <asm/device.h>
>   
> +/*
> + * Used to keep track of devices for which driver requested deferred probing
> + * (returns -EAGAIN).
> + */
> +static LIST_HEAD(deferred_probe_list);

This wants to be in init section as this is only used at boot.

> +
>   static const struct iommu_ops *iommu_ops;
>   
>   const struct iommu_ops *iommu_get_ops(void)
> @@ -42,7 +48,7 @@ void __init iommu_set_ops(const struct iommu_ops *ops)
>   
>   int __init iommu_hardware_setup(void)
>   {
> -    struct dt_device_node *np;
> +    struct dt_device_node *np, *tmp;
>       int rc;
>       unsigned int num_iommus = 0;
>   
> @@ -51,6 +57,33 @@ int __init iommu_hardware_setup(void)
>           rc = device_init(np, DEVICE_IOMMU, NULL);
>           if ( !rc )
>               num_iommus++;
> +        else if (rc == -EAGAIN)
> +            /*
> +             * Driver requested deferred probing, so add this device to
> +             * the deferred list for further processing.
> +             */
> +            list_add(&np->deferred_probe, &deferred_probe_list);
> +    }
> +
> +    /*
> +     * Process devices in the deferred list if at least one successfully
> +     * probed device is present.
> +     */

I think this can turn into an infinite loop if all device in deferred_probe_list 
still return -EDEFER_PROBE and num_iommus is a non-zero.

A better condition would be to check that at least one IOMMU is added at each 
loop. If not, then we should bail with an error because it likely means 
something is buggy.

> +    while ( !list_empty(&deferred_probe_list) && num_iommus )
> +    {
> +        list_for_each_entry_safe ( np, tmp, &deferred_probe_list,
> +                                   deferred_probe )
> +        {
> +            rc = device_init(np, DEVICE_IOMMU, NULL);
> +            if ( !rc )
> +                num_iommus++;
> +            if ( rc != -EAGAIN )
> +                /*
> +                 * Driver didn't request deferred probing, so remove this device
> +                 * from the deferred list.
> +                 */
> +                list_del_init(&np->deferred_probe);
> +        }
>       }
>   
>       return ( num_iommus > 0 ) ? 0 : -ENODEV;
> diff --git a/xen/include/asm-arm/device.h b/xen/include/asm-arm/device.h
> index 63a0f36..ee1c3bc 100644
> --- a/xen/include/asm-arm/device.h
> +++ b/xen/include/asm-arm/device.h
> @@ -44,7 +44,11 @@ struct device_desc {
>       enum device_class class;
>       /* List of devices supported by this driver */
>       const struct dt_device_match *dt_match;
> -    /* Device initialization */
> +    /*
> +     * Device initialization.
> +     *
> +     * -EAGAIN is used to indicate that device probing is deferred.
> +     */
>       int (*init)(struct dt_device_node *dev, const void *data);
>   };
>   
> diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
> index 8315629..71b0e47 100644
> --- a/xen/include/xen/device_tree.h
> +++ b/xen/include/xen/device_tree.h
> @@ -93,6 +93,7 @@ struct dt_device_node {
>       /* IOMMU specific fields */
>       bool is_protected;
>       struct list_head domain_list;
> +    struct list_head deferred_probe;
>   
>       struct device dev;
>   };
> 

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

  reply	other threads:[~2019-08-12 11:11 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-02 16:39 [Xen-devel] [PATCH V2 0/6] iommu/arm: Add Renesas IPMMU-VMSA support + Linux's iommu_fwspec Oleksandr Tyshchenko
2019-08-02 16:39 ` [Xen-devel] [PATCH V2 1/6] iommu/arm: Add iommu_helpers.c file to keep common for IOMMUs stuff Oleksandr Tyshchenko
2019-08-09 17:35   ` Julien Grall
2019-08-09 18:10     ` Oleksandr
2019-08-02 16:39 ` [Xen-devel] [PATCH V2 2/6] iommu/arm: Add ability to handle deferred probing request Oleksandr Tyshchenko
2019-08-12 11:11   ` Julien Grall [this message]
2019-08-12 12:01     ` Oleksandr
2019-08-12 19:46       ` Julien Grall
2019-08-13 12:35         ` Oleksandr
2019-08-14 17:34           ` Julien Grall
2019-08-14 19:25             ` Stefano Stabellini
2019-08-15  9:29               ` Julien Grall
2019-08-15 12:54                 ` Julien Grall
2019-08-15 13:14                   ` Oleksandr
2019-08-15 16:39                     ` Oleksandr
2019-08-02 16:39 ` [Xen-devel] [PATCH V2 3/6] [RFC] xen/common: Introduce _xrealloc function Oleksandr Tyshchenko
2019-08-05 10:02   ` Jan Beulich
2019-08-06 18:50     ` Oleksandr
2019-08-07  6:22       ` Jan Beulich
2019-08-07 17:31         ` Oleksandr
2019-08-06 19:51     ` Volodymyr Babchuk
2019-08-07  6:26       ` Jan Beulich
2019-08-07 18:36         ` Oleksandr
2019-08-08  6:08           ` Jan Beulich
2019-08-08  7:05           ` Jan Beulich
2019-08-08 11:05             ` Oleksandr
2019-08-02 16:39 ` [Xen-devel] [PATCH V2 4/6] iommu/arm: Add lightweight iommu_fwspec support Oleksandr Tyshchenko
2019-08-13 12:39   ` Julien Grall
2019-08-13 15:17     ` Oleksandr
2019-08-13 15:28       ` Julien Grall
2019-08-13 16:18         ` Oleksandr
2019-08-13 13:40   ` Julien Grall
2019-08-13 16:28     ` Oleksandr
2019-08-02 16:39 ` [Xen-devel] [PATCH V2 5/6] iommu/arm: Introduce iommu_add_dt_device API Oleksandr Tyshchenko
2019-08-13 13:49   ` Julien Grall
2019-08-13 16:05     ` Oleksandr
2019-08-13 17:13       ` Julien Grall
2019-08-02 16:39 ` [Xen-devel] [PATCH V2 6/6] iommu/arm: Add Renesas IPMMU-VMSA support Oleksandr Tyshchenko
2019-08-07  2:41   ` Yoshihiro Shimoda
2019-08-07 16:01     ` Oleksandr
2019-08-07 19:15       ` Julien Grall
2019-08-07 20:28         ` Oleksandr Tyshchenko
2019-08-08  9:05           ` Julien Grall
2019-08-08 10:14             ` Oleksandr
2019-08-08 12:44               ` Julien Grall
2019-08-08 15:04                 ` Oleksandr
2019-08-08 17:16                   ` Julien Grall
2019-08-08 19:29                     ` Oleksandr
2019-08-08 20:32                       ` Julien Grall
2019-08-08 23:32                         ` Oleksandr Tyshchenko
2019-08-09  9:56                           ` Julien Grall
2019-08-09 18:38                             ` Oleksandr
2019-08-08 12:28         ` Oleksandr
2019-08-08 14:23         ` Lars Kurth
2019-08-08  4:05       ` Yoshihiro Shimoda
2019-08-14 17:38   ` Julien Grall
2019-08-14 18:45     ` Oleksandr
2019-08-05  7:58 ` [Xen-devel] [PATCH V2 0/6] iommu/arm: Add Renesas IPMMU-VMSA support + Linux's iommu_fwspec Oleksandr
2019-08-05  8:29   ` Julien Grall

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3c6d54f2-06ad-6bd1-447b-0e4cbef3d391@arm.com \
    --to=julien.grall@arm.com \
    --cc=oleksandr_tyshchenko@epam.com \
    --cc=olekstysh@gmail.com \
    --cc=sstabellini@kernel.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).