From: "Roger Pau Monné" <roger.pau@citrix.com>
To: Paul Durrant <paul.durrant@citrix.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
Wei Liu <wei.liu2@citrix.com>,
Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
George Dunlap <George.Dunlap@eu.citrix.com>,
Andrew Cooper <andrew.cooper3@citrix.com>,
Ian Jackson <ian.jackson@eu.citrix.com>, Tim Deegan <tim@xen.org>,
Julien Grall <julien.grall@arm.com>,
Jan Beulich <jbeulich@suse.com>,
xen-devel@lists.xenproject.org
Subject: Re: [Xen-devel] [PATCH 4/5] iommu: introduce iommu_groups
Date: Wed, 15 May 2019 10:44:43 +0200 [thread overview]
Message-ID: <20190515084443.f4v3otqjqu2ofnrk@Air-de-Roger> (raw)
Message-ID: <20190515084443.KShGBBXQZeZSbsDvKRMfWah0a0XHZkIKfiBnbDvOFl4@z> (raw)
In-Reply-To: <20190508132403.1454-5-paul.durrant@citrix.com>
On Wed, May 08, 2019 at 02:24:02PM +0100, Paul Durrant wrote:
> Some devices may share a single PCIe initiator id, e.g. if they are actually
> legacy PCI devices behind a bridge, and hence DMA from such devices will
> be subject to the same address translation in the IOMMU. Hence these devices
> should be treated as a unit for the purposes of assignment. There are also
> other reasons why multiple devices should be treated as a unit, e.g. those
> subject to a shared RMRR or those downstream of a bridge that does not
> support ACS.
>
> This patch introduces a new struct iommu_group to act as a container for
> devices that should be treated as a unit, and builds a list of them as
> PCI devices are scanned. The iommu_ops already implement a method,
> get_device_group_id(), that is seemingly intended to return the initiator
> id for a given SBDF so use this as the mechanism for group assignment in
> the first instance. Assignment based on shared RMRR or lack of ACS will be
> dealt with in subsequent patches, as will modifications to the device
> assignment code.
>
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> ---
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: George Dunlap <George.Dunlap@eu.citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Julien Grall <julien.grall@arm.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Tim Deegan <tim@xen.org>
> Cc: Wei Liu <wei.liu2@citrix.com>
> ---
> xen/drivers/passthrough/iommu.c | 76 +++++++++++++++++++++++++++++++++++++++++
> xen/drivers/passthrough/pci.c | 3 ++
> xen/include/xen/iommu.h | 7 ++++
> xen/include/xen/pci.h | 3 ++
> 4 files changed, 89 insertions(+)
>
> diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
> index d3a6199b77..11319fbaae 100644
> --- a/xen/drivers/passthrough/iommu.c
> +++ b/xen/drivers/passthrough/iommu.c
> @@ -655,6 +655,82 @@ static void iommu_dump_p2m_table(unsigned char key)
> }
> }
>
> +#ifdef CONFIG_HAS_PCI
> +
> +struct iommu_group {
> + unsigned int id;
> + unsigned int index;
I'm not sure I see the point of the index field, isn't it enough to
just use the ID field?
The ID is already used as a unique key in the code below for the radix
tree operations.
> + struct list_head devs_list;
> +};
> +
> +static struct radix_tree_root iommu_groups;
> +
> +void __init iommu_groups_init(void)
> +{
> + radix_tree_init(&iommu_groups);
> +}
> +
> +static struct iommu_group *alloc_iommu_group(unsigned int id)
> +{
> + struct iommu_group *grp;
> + static unsigned int index;
> +
> + grp = xzalloc(struct iommu_group);
Can be moved with the declaration above.
> + if ( !grp )
> + return NULL;
> +
> + grp->id = id;
> + grp->index = index++;
AFAICT none of this is subject to races because it's always protected
by the pcidevs lock?
> + INIT_LIST_HEAD(&grp->devs_list);
> +
> + if ( radix_tree_insert(&iommu_groups, id, grp) )
> + {
> + xfree(grp);
> + grp = NULL;
Do you need to decrease index here, or is it fine to burn numbers on
failure?
> + }
> +
> + return grp;
> +}
> +
> +static struct iommu_group *get_iommu_group(unsigned int id)
> +{
> + struct iommu_group *grp = radix_tree_lookup(&iommu_groups, id);
> +
> + if ( !grp )
> + grp = alloc_iommu_group(id);
> +
> + return grp;
> +}
> +
> +int iommu_group_assign(struct pci_dev *pdev)
> +{
> + const struct iommu_ops *ops;
> + unsigned int id;
> + struct iommu_group *grp;
> +
> + ops = iommu_get_ops();
This initialization can be done at declaration time.
> + if ( !ops || !ops->get_device_group_id )
> + return 0;
> +
> + id = ops->get_device_group_id(pdev->seg, pdev->bus, pdev->devfn);
> + grp = get_iommu_group(id);
> +
> + if ( ! grp )
^ extra space
> + return -ENOMEM;
> +
> + if ( iommu_verbose )
> + printk(XENLOG_INFO "Assign %04x:%02x:%02x.%u -> IOMMU group %u\n",
> + pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn),
> + PCI_FUNC(pdev->devfn), grp->index);
Wouldn't it be more helpful to print the group ID rather than the Xen
internal index?
Thanks, Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
next prev parent reply other threads:[~2019-05-15 8:45 UTC|newest]
Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-08 13:23 [PATCH 0/5] iommu groups + cleanup Paul Durrant
2019-05-08 13:23 ` [Xen-devel] " Paul Durrant
2019-05-08 13:23 ` [PATCH 1/5] iommu: trivial re-organisation to avoid unnecessary test Paul Durrant
2019-05-08 13:23 ` [Xen-devel] " Paul Durrant
2019-05-13 15:22 ` Jan Beulich
2019-05-13 15:22 ` [Xen-devel] " Jan Beulich
2019-05-08 13:24 ` [PATCH 2/5] iommu / x86: move call to scan_pci_devices() out of vendor code Paul Durrant
2019-05-08 13:24 ` [Xen-devel] " Paul Durrant
2019-05-13 15:35 ` Jan Beulich
2019-05-13 15:35 ` [Xen-devel] " Jan Beulich
2019-05-14 16:13 ` Paul Durrant
2019-05-14 16:13 ` [Xen-devel] " Paul Durrant
2019-05-15 6:29 ` Jan Beulich
2019-05-15 6:29 ` [Xen-devel] " Jan Beulich
2019-05-08 13:24 ` [PATCH 3/5] iommu: move iommu_get_ops() into common code Paul Durrant
2019-05-08 13:24 ` [Xen-devel] " Paul Durrant
2019-05-13 16:11 ` Jan Beulich
2019-05-13 16:11 ` [Xen-devel] " Jan Beulich
2019-05-14 16:19 ` Paul Durrant
2019-05-14 16:19 ` [Xen-devel] " Paul Durrant
2019-05-14 21:36 ` Julien Grall
2019-05-14 21:36 ` [Xen-devel] " Julien Grall
2019-05-15 6:32 ` Jan Beulich
2019-05-15 6:32 ` [Xen-devel] " Jan Beulich
2019-05-08 13:24 ` [PATCH 4/5] iommu: introduce iommu_groups Paul Durrant
2019-05-08 13:24 ` [Xen-devel] " Paul Durrant
2019-05-15 8:44 ` Roger Pau Monné [this message]
2019-05-15 8:44 ` Roger Pau Monné
2019-05-31 13:48 ` Paul Durrant
2019-05-31 13:48 ` [Xen-devel] " Paul Durrant
2019-05-15 14:17 ` Jan Beulich
2019-05-15 14:17 ` [Xen-devel] " Jan Beulich
2019-05-31 13:55 ` Paul Durrant
2019-05-31 13:55 ` [Xen-devel] " Paul Durrant
2019-05-31 14:13 ` Jan Beulich
2019-05-31 14:13 ` [Xen-devel] " Jan Beulich
2019-05-31 14:21 ` Paul Durrant
2019-05-31 14:21 ` [Xen-devel] " Paul Durrant
2019-05-15 14:24 ` Jan Beulich
2019-05-15 14:24 ` [Xen-devel] " Jan Beulich
2019-05-08 13:24 ` [PATCH 5/5] iommu / pci: re-implement XEN_DOMCTL_get_device_group Paul Durrant
2019-05-08 13:24 ` [Xen-devel] " Paul Durrant
2019-05-15 9:06 ` Roger Pau Monné
2019-05-15 9:06 ` [Xen-devel] " Roger Pau Monné
2019-06-03 9:58 ` Paul Durrant
2019-06-03 9:58 ` [Xen-devel] " Paul Durrant
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=20190515084443.f4v3otqjqu2ofnrk@Air-de-Roger \
--to=roger.pau@citrix.com \
--cc=George.Dunlap@eu.citrix.com \
--cc=andrew.cooper3@citrix.com \
--cc=ian.jackson@eu.citrix.com \
--cc=jbeulich@suse.com \
--cc=julien.grall@arm.com \
--cc=konrad.wilk@oracle.com \
--cc=paul.durrant@citrix.com \
--cc=sstabellini@kernel.org \
--cc=tim@xen.org \
--cc=wei.liu2@citrix.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 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).