All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alex Williamson <alex.williamson@redhat.com>
To: "Roedel, Joerg" <Joerg.Roedel@amd.com>
Cc: Florian Dazinger <florian@dazinger.net>,
	linux-kernel@vger.kernel.org,
	iommu <iommu@lists.linux-foundation.org>
Subject: Re: 3.6-rc7 boot crash + bisection
Date: Wed, 26 Sep 2012 10:21:10 -0600	[thread overview]
Message-ID: <1348676470.28860.197.camel@bling.home> (raw)
In-Reply-To: <20120926151044.GE10549@amd.com>

On Wed, 2012-09-26 at 17:10 +0200, Roedel, Joerg wrote:
> On Wed, Sep 26, 2012 at 08:35:59AM -0600, Alex Williamson wrote:
> > Hmm, that throws a kink in iommu groups.  So perhaps we need to make an
> > alias interface to iommu groups.  Seems like this could just be an extra
> > parameter to iommu_group_get and iommu_group_add_device (empty in the
> > typical case).  Then we have the problem of what's the type for an
> > alias?  For AMI-Vi, it's a u16, but we need to be more generic than
> > that.  Maybe iommu groups should just treat it as a void* so iommus can
> > use a pointer to some structure or a fixed value like a u16 bus:slot.
> > Thoughts?
> 
> Good question. The iommu-groups are part of the IOMMU-API, with an
> interface to the IOMMU drivers and one to the users of IOMMU-API. So the
> alias handling itself should be a function of the interface to the IOMMU
> driver. In general the interface should not be bus specific.
> 
> So a void pointer seems the only logical choice then. But I would not
> limit its scope to alias handling. How about making it a bus-private
> pointer where IOMMU driver store bus-specific information. That way we
> make sure that there is one struct per bus-type for this pointer, and
> not one structure per IOMMU driver.

I thought of another approach that may actually be more 3.6 worthy.
What if we just make the iommu driver handle it?  For instance,
amd_iommu can walk the alias table looking for entries that use the same
alias and get the device via pci_get_bus_and_slot.  If it finds a device
with an iommu group, it attaches the new device to the same group,
hiding anything about aliases from the group layer.  It just groups all
devices within the range.  I think the only complication is making sure
we're safe around device hotplug while we're doing this.  Thanks,

Alex



WARNING: multiple messages have this Message-ID (diff)
From: Alex Williamson <alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
To: "Roedel, Joerg" <Joerg.Roedel-5C7GfCeVMHo@public.gmane.org>
Cc: Florian Dazinger
	<florian-Q0TRQrZM+Zzk1uMJSBkQmQ@public.gmane.org>,
	iommu
	<iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org>,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: 3.6-rc7 boot crash + bisection
Date: Wed, 26 Sep 2012 10:21:10 -0600	[thread overview]
Message-ID: <1348676470.28860.197.camel@bling.home> (raw)
In-Reply-To: <20120926151044.GE10549-5C7GfCeVMHo@public.gmane.org>

On Wed, 2012-09-26 at 17:10 +0200, Roedel, Joerg wrote:
> On Wed, Sep 26, 2012 at 08:35:59AM -0600, Alex Williamson wrote:
> > Hmm, that throws a kink in iommu groups.  So perhaps we need to make an
> > alias interface to iommu groups.  Seems like this could just be an extra
> > parameter to iommu_group_get and iommu_group_add_device (empty in the
> > typical case).  Then we have the problem of what's the type for an
> > alias?  For AMI-Vi, it's a u16, but we need to be more generic than
> > that.  Maybe iommu groups should just treat it as a void* so iommus can
> > use a pointer to some structure or a fixed value like a u16 bus:slot.
> > Thoughts?
> 
> Good question. The iommu-groups are part of the IOMMU-API, with an
> interface to the IOMMU drivers and one to the users of IOMMU-API. So the
> alias handling itself should be a function of the interface to the IOMMU
> driver. In general the interface should not be bus specific.
> 
> So a void pointer seems the only logical choice then. But I would not
> limit its scope to alias handling. How about making it a bus-private
> pointer where IOMMU driver store bus-specific information. That way we
> make sure that there is one struct per bus-type for this pointer, and
> not one structure per IOMMU driver.

I thought of another approach that may actually be more 3.6 worthy.
What if we just make the iommu driver handle it?  For instance,
amd_iommu can walk the alias table looking for entries that use the same
alias and get the device via pci_get_bus_and_slot.  If it finds a device
with an iommu group, it attaches the new device to the same group,
hiding anything about aliases from the group layer.  It just groups all
devices within the range.  I think the only complication is making sure
we're safe around device hotplug while we're doing this.  Thanks,

Alex

  reply	other threads:[~2012-09-26 16:21 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-24 19:03 3.6-rc7 boot crash + bisection Florian Dazinger
2012-09-25 18:32 ` Alex Williamson
2012-09-25 18:42   ` Alex Williamson
2012-09-25 18:54   ` Florian Dazinger
2012-09-25 19:43     ` Alex Williamson
2012-09-25 19:43       ` Alex Williamson
2012-09-25 23:01       ` Florian Dazinger
2012-09-26  3:12         ` Alex Williamson
2012-09-26  3:12           ` Alex Williamson
2012-09-26 14:43         ` Roedel, Joerg
2012-09-26 14:43           ` Roedel, Joerg
2012-09-26 14:52           ` Alex Williamson
2012-09-26 14:52             ` Alex Williamson
2012-09-26 15:04             ` Roedel, Joerg
2012-09-26 15:04               ` Roedel, Joerg
2012-09-26 16:13               ` Alex Williamson
2012-09-26 16:13                 ` Alex Williamson
2012-09-26 16:43               ` Florian Dazinger
2012-09-26 16:43                 ` Florian Dazinger
2012-09-26 17:47               ` Florian Dazinger
2012-09-26 17:47                 ` Florian Dazinger
2012-09-26 13:20       ` Roedel, Joerg
2012-09-26 13:20         ` Roedel, Joerg
2012-09-26 14:35         ` Alex Williamson
2012-09-26 14:35           ` Alex Williamson
2012-09-26 15:10           ` Roedel, Joerg
2012-09-26 15:10             ` Roedel, Joerg
2012-09-26 16:21             ` Alex Williamson [this message]
2012-09-26 16:21               ` Alex Williamson
2012-09-26 19:50               ` Alex Williamson
2012-09-26 19:50                 ` Alex Williamson
2012-09-26 22:04                 ` Alex Williamson
2012-09-26 22:04                   ` Alex Williamson
2012-09-27 16:22                   ` Florian Dazinger
2012-09-27 16:22                     ` Florian Dazinger
2012-09-28 13:58                   ` Roedel, Joerg
2012-09-28 13:58                     ` Roedel, Joerg

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=1348676470.28860.197.camel@bling.home \
    --to=alex.williamson@redhat.com \
    --cc=Joerg.Roedel@amd.com \
    --cc=florian@dazinger.net \
    --cc=iommu@lists.linux-foundation.org \
    --cc=linux-kernel@vger.kernel.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.