All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stefano Stabellini <sstabellini@kernel.org>
To: Julien Grall <julien@xen.org>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	 xen-devel@lists.xenproject.org, Bertrand.Marquis@arm.com,
	 Volodymyr_Babchuk@epam.com, rahul.singh@arm.com,
	brian.woods@xilinx.com,
	 Stefano Stabellini <stefano.stabellini@xilinx.com>
Subject: Re: [PATCH v5 2/4] xen: do not return -EEXIST if iommu_add_dt_device is called twice
Date: Fri, 30 Jul 2021 14:57:34 -0700 (PDT)	[thread overview]
Message-ID: <alpine.DEB.2.21.2107301444100.10122@sstabellini-ThinkPad-T480s> (raw)
In-Reply-To: <aa7be3ad-8cbe-a187-3ebc-e1c5d2d11091@xen.org>

On Mon, 26 Jul 2021, Julien Grall wrote:
> Hi Stefano,
> 
> On 23/07/2021 21:16, Stefano Stabellini wrote:
> > On Fri, 23 Jul 2021, Julien Grall wrote:
> > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> > > > ---
> > > > Changes in v5:
> > > > - new patch
> > > > ---
> > > >    xen/drivers/passthrough/device_tree.c | 9 +++++++--
> > > >    1 file changed, 7 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/xen/drivers/passthrough/device_tree.c
> > > > b/xen/drivers/passthrough/device_tree.c
> > > > index 999b831d90..32526ecabb 100644
> > > > --- a/xen/drivers/passthrough/device_tree.c
> > > > +++ b/xen/drivers/passthrough/device_tree.c
> > > > @@ -140,8 +140,13 @@ int iommu_add_dt_device(struct dt_device_node *np)
> > > >        if ( !ops )
> > > >            return -EINVAL;
> > > >    +    /*
> > > > +     * Some Device Trees may expose both legacy SMMU and generic
> > > > +     * IOMMU bindings together. If both are present, the device
> > > > +     * can be already added.
> > > 
> > > Wouldn't this also happen when there is just generic bindings? If so,
> > > shouldn't this patch be first in the series to avoid breaking bisection?
> > 
> > No, both need to be present; if there is just the generic bindings we
> > don't need this change. I can still move it to the beginning of the
> > series anyway if you prefer.
> 
> Sorry but I am having some trouble to understand why this is not a problem for
> just the legacy binding.
> 
> If I look at patch #1, the dev->iommu_fspec will be allocated in
> register_smmu_master(). If I am not mistaken, this is called when the SMMU is
> initialized.
> 
> So the call to iommu_add_dt_device() in handle_device() should return -EEXIST
> (dev_iommu_fwspec_get() will return a non-NULL pointer).
> 
> What did I miss?

I checked. It is true that we need this check with the legacy bindings,
even when alone.

Like you said, dev->iommu_fspec is allocated early by
register_smmu_master. Then, handle_device, or handle_passthrough_prop
for dom0less guests, calls iommu_add_dt_device a second time.

On the other hand with only the generic bindings register_smmu_master
doesn't call iommu_add_dt_device, so iommu_add_dt_device is called only
once by handle_device or handle_passthrough_prop.


The comment I proposed is not correct. What about this one?

    /*
     * In case of legacy SMMU bindings, register_smmu_master might have
     * already initialized struct iommu_fwspec for this device.
     */


  reply	other threads:[~2021-07-30 21:57 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-22 23:35 [PATCH v5 0/4] Generic SMMU Bindings Stefano Stabellini
2021-07-22 23:36 ` [PATCH v5 1/4] arm,smmu: switch to using iommu_fwspec functions Stefano Stabellini
2021-07-22 23:36 ` [PATCH v5 2/4] xen: do not return -EEXIST if iommu_add_dt_device is called twice Stefano Stabellini
2021-07-23  6:31   ` Jan Beulich
2021-07-23  9:28     ` Julien Grall
2021-07-23 13:02       ` Jan Beulich
2021-07-26 15:45         ` Julien Grall
2021-08-03  6:57           ` Jan Beulich
2021-08-03  9:31             ` Julien Grall
2021-07-23  9:13   ` Julien Grall
2021-07-23 20:16     ` Stefano Stabellini
2021-07-26 15:53       ` Julien Grall
2021-07-30 21:57         ` Stefano Stabellini [this message]
2021-08-02 15:05           ` Julien Grall
2021-08-03  0:18             ` Stefano Stabellini
2021-07-22 23:36 ` [PATCH v5 3/4] arm,smmu: restructure code in preparation to new bindings support Stefano Stabellini
2021-07-22 23:36 ` [PATCH v5 4/4] arm,smmu: add support for generic DT bindings. Implement add_device and dt_xlate Stefano Stabellini

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=alpine.DEB.2.21.2107301444100.10122@sstabellini-ThinkPad-T480s \
    --to=sstabellini@kernel.org \
    --cc=Bertrand.Marquis@arm.com \
    --cc=Volodymyr_Babchuk@epam.com \
    --cc=brian.woods@xilinx.com \
    --cc=julien@xen.org \
    --cc=rahul.singh@arm.com \
    --cc=stefano.stabellini@xilinx.com \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.