From: "Tian, Kevin" <kevin.tian@intel.com>
To: "Liu, Yi L" <yi.l.liu@intel.com>,
Robin Murphy <robin.murphy@arm.com>,
Jean-Philippe Brucker <jean-philippe@linaro.org>
Cc: "alex.williamson@redhat.com" <alex.williamson@redhat.com>,
"eric.auger@redhat.com" <eric.auger@redhat.com>,
"baolu.lu@linux.intel.com" <baolu.lu@linux.intel.com>,
"joro@8bytes.org" <joro@8bytes.org>,
"jacob.jun.pan@linux.intel.com" <jacob.jun.pan@linux.intel.com>,
"Raj, Ashok" <ashok.raj@intel.com>,
"Tian, Jun J" <jun.j.tian@intel.com>,
"Sun, Yi Y" <yi.y.sun@intel.com>,
"peterx@redhat.com" <peterx@redhat.com>,
"Wu, Hao" <hao.wu@intel.com>,
"iommu@lists.linux-foundation.org"
<iommu@lists.linux-foundation.org>,
"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Will Deacon <will@kernel.org>
Subject: RE: [PATCH v3 02/14] iommu: Report domain nesting info
Date: Tue, 30 Jun 2020 01:20:34 +0000 [thread overview]
Message-ID: <MWHPR11MB16456F2C16A88BB6F84076C78C6F0@MWHPR11MB1645.namprd11.prod.outlook.com> (raw)
In-Reply-To: <DM5PR11MB1435D877048ACCC1B8EAAFDBC3900@DM5PR11MB1435.namprd11.prod.outlook.com>
> From: Liu, Yi L <yi.l.liu@intel.com>
> Sent: Saturday, June 27, 2020 2:53 PM
>
> Hi Robin,
>
> > From: Robin Murphy <robin.murphy@arm.com>
> > Sent: Saturday, June 27, 2020 12:05 AM
> >
> > On 2020-06-26 08:47, Jean-Philippe Brucker wrote:
> > > On Wed, Jun 24, 2020 at 01:55:15AM -0700, Liu Yi L wrote:
> > >> IOMMUs that support nesting translation needs report the capability
> > >> info to userspace, e.g. the format of first level/stage paging structures.
> > >>
> > >> This patch reports nesting info by DOMAIN_ATTR_NESTING. Caller can
> > >> get nesting info after setting DOMAIN_ATTR_NESTING.
> > >>
> > >> v2 -> v3:
> > >> *) remvoe cap/ecap_mask in iommu_nesting_info.
> > >> *) reuse DOMAIN_ATTR_NESTING to get nesting info.
> > >> *) return an empty iommu_nesting_info for SMMU drivers per Jean'
> > >> suggestion.
> > >>
> > >> Cc: Kevin Tian <kevin.tian@intel.com>
> > >> CC: Jacob Pan <jacob.jun.pan@linux.intel.com>
> > >> Cc: Alex Williamson <alex.williamson@redhat.com>
> > >> Cc: Eric Auger <eric.auger@redhat.com>
> > >> Cc: Jean-Philippe Brucker <jean-philippe@linaro.org>
> > >> Cc: Joerg Roedel <joro@8bytes.org>
> > >> Cc: Lu Baolu <baolu.lu@linux.intel.com>
> > >> Signed-off-by: Liu Yi L <yi.l.liu@intel.com>
> > >> Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
> > >> ---
> > >> drivers/iommu/arm-smmu-v3.c | 29 ++++++++++++++++++++--
> > >> drivers/iommu/arm-smmu.c | 29 ++++++++++++++++++++--
> > >
> > > Looks reasonable to me. Please move the SMMU changes to a separate
> > > patch and Cc the SMMU maintainers:
> >
> > Cheers Jean, I'll admit I've been skipping over a lot of these patches lately :)
> >
> > A couple of comments below...
> >
> > >
> > > Cc: Will Deacon <will@kernel.org>
> > > Cc: Robin Murphy <robin.murphy@arm.com>
> > >
> > > Thanks,
> > > Jean
> > >
> > >> include/uapi/linux/iommu.h | 59
> > +++++++++++++++++++++++++++++++++++++++++++++
> > >> 3 files changed, 113 insertions(+), 4 deletions(-)
> > >>
> > >> diff --git a/drivers/iommu/arm-smmu-v3.c
> > >> b/drivers/iommu/arm-smmu-v3.c index f578677..0c45d4d 100644
> > >> --- a/drivers/iommu/arm-smmu-v3.c
> > >> +++ b/drivers/iommu/arm-smmu-v3.c
> > >> @@ -3019,6 +3019,32 @@ static struct iommu_group
> > *arm_smmu_device_group(struct device *dev)
> > >> return group;
> > >> }
> > >>
> > >> +static int arm_smmu_domain_nesting_info(struct arm_smmu_domain
> > *smmu_domain,
> > >> + void *data)
> > >> +{
> > >> + struct iommu_nesting_info *info = (struct iommu_nesting_info *)
> data;
> > >> + u32 size;
> > >> +
> > >> + if (!info || smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
> > >> + return -ENODEV;
> > >> +
> > >> + size = sizeof(struct iommu_nesting_info);
> > >> +
> > >> + /*
> > >> + * if provided buffer size is not equal to the size, should
> > >> + * return 0 and also the expected buffer size to caller.
> > >> + */
> > >> + if (info->size != size) {
> > >> + info->size = size;
> > >> + return 0;
> > >> + }
> > >> +
> > >> + /* report an empty iommu_nesting_info for now */
> > >> + memset(info, 0x0, size);
> > >> + info->size = size;
> > >> + return 0;
> > >> +}
> > >> +
> > >> static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
> > >> enum iommu_attr attr, void *data)
> > >> {
> > >> @@ -3028,8 +3054,7 @@ static int arm_smmu_domain_get_attr(struct
> > iommu_domain *domain,
> > >> case IOMMU_DOMAIN_UNMANAGED:
> > >> switch (attr) {
> > >> case DOMAIN_ATTR_NESTING:
> > >> - *(int *)data = (smmu_domain->stage ==
> > ARM_SMMU_DOMAIN_NESTED);
> > >> - return 0;
> > >> + return
> arm_smmu_domain_nesting_info(smmu_domain,
> > data);
> > >> default:
> > >> return -ENODEV;
> > >> }
> > >> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> > >> index 243bc4c..908607d 100644
> > >> --- a/drivers/iommu/arm-smmu.c
> > >> +++ b/drivers/iommu/arm-smmu.c
> > >> @@ -1506,6 +1506,32 @@ static struct iommu_group
> > *arm_smmu_device_group(struct device *dev)
> > >> return group;
> > >> }
> > >>
> > >> +static int arm_smmu_domain_nesting_info(struct arm_smmu_domain
> > *smmu_domain,
> > >> + void *data)
> > >> +{
> > >> + struct iommu_nesting_info *info = (struct iommu_nesting_info *)
> data;
> > >> + u32 size;
> > >> +
> > >> + if (!info || smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
> > >> + return -ENODEV;
> > >> +
> > >> + size = sizeof(struct iommu_nesting_info);
> > >> +
> > >> + /*
> > >> + * if provided buffer size is not equal to the size, should
> > >> + * return 0 and also the expected buffer size to caller.
> > >> + */
> > >> + if (info->size != size) {
> > >> + info->size = size;
> > >> + return 0;
> > >> + }
> > >> +
> > >> + /* report an empty iommu_nesting_info for now */
> > >> + memset(info, 0x0, size);
> > >> + info->size = size;
> > >> + return 0;
> > >> +}
> > >> +
> > >> static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
> > >> enum iommu_attr attr, void *data)
> > >> {
> > >> @@ -1515,8 +1541,7 @@ static int arm_smmu_domain_get_attr(struct
> > iommu_domain *domain,
> > >> case IOMMU_DOMAIN_UNMANAGED:
> > >> switch (attr) {
> > >> case DOMAIN_ATTR_NESTING:
> > >> - *(int *)data = (smmu_domain->stage ==
> > ARM_SMMU_DOMAIN_NESTED);
> > >> - return 0;
> > >> + return
> arm_smmu_domain_nesting_info(smmu_domain,
> > data);
> > >> default:
> > >> return -ENODEV;
> > >> }
> > >> diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
> > >> index 1afc661..898c99a 100644
> > >> --- a/include/uapi/linux/iommu.h
> > >> +++ b/include/uapi/linux/iommu.h
> > >> @@ -332,4 +332,63 @@ struct iommu_gpasid_bind_data {
> > >> } vendor;
> > >> };
> > >>
> > >> +/*
> > >> + * struct iommu_nesting_info - Information for nesting-capable
> IOMMU.
> > >> + * user space should check it before using
> > >> + * nesting capability.
> > >> + *
> > >> + * @size: size of the whole structure
> > >> + * @format: PASID table entry format, the same definition with
> > >> + * @format of struct iommu_gpasid_bind_data.
> > >> + * @features: supported nesting features.
> > >> + * @flags: currently reserved for future extension.
> > >> + * @data: vendor specific cap info.
> > >> + *
> > >> + * +---------------+----------------------------------------------------+
> > >> + * | feature | Notes |
> > >> + *
> > >>
> > ++===============+============================================
> > =======
> > >> +=+
> > >> + * | SYSWIDE_PASID | Kernel manages PASID in system wide, PASIDs
> used |
> > >> + * | | in the system should be allocated by host kernel |
> > >> + * +---------------+----------------------------------------------------+
> > >> + * | BIND_PGTBL | bind page tables to host PASID, the PASID could |
> > >> + * | | either be a host PASID passed in bind request or |
> > >> + * | | default PASIDs (e.g. default PASID of aux-domain) |
> > >> + * +---------------+----------------------------------------------------+
> > >> + * | CACHE_INVLD | mandatory feature for nesting capable IOMMU
> |
> > >> + *
> > >> ++---------------+---------------------------------------------------
> > >> +-+
> > >> + *
> > >> + */
> > >> +struct iommu_nesting_info {
> > >> + __u32 size;
> > >> + __u32 format;
> > >> + __u32 features;
> > >> +#define IOMMU_NESTING_FEAT_SYSWIDE_PASID (1 << 0)
> > >> +#define IOMMU_NESTING_FEAT_BIND_PGTBL (1 << 1)
> > >> +#define IOMMU_NESTING_FEAT_CACHE_INVLD (1 << 2)
> > >> + __u32 flags;
> > >> + __u8 data[];
> > >> +};
> > >> +
> > >> +/*
> > >> + * struct iommu_nesting_info_vtd - Intel VT-d specific nesting info
> > >> + *
> > >> + *
> > >> + * @flags: VT-d specific flags. Currently reserved for future
> > >> + * extension.
> > >> + * @addr_width: The output addr width of first level/stage translation
> > >> + * @pasid_bits: Maximum supported PASID bits, 0 represents no
> PASID
> > >> + * support.
> > >> + * @cap_reg: Describe basic capabilities as defined in VT-d
> capability
> > >> + * register.
> > >> + * @ecap_reg: Describe the extended capabilities as defined in VT-d
> > >> + * extended capability register.
> > >> + */
> > >> +struct iommu_nesting_info_vtd {
> > >> + __u32 flags;
> > >> + __u16 addr_width;
> >
> > I think this might be worth promoting to a generic feature - Arm has the
> same
> > notion of intermediate address size, and I'd imagine that pretty much any
> other
> > two-stage translation system would as well (either explicitly or implicitly).
> > It also
> > comes close to something the DPDK folks raised where they wanted parity
> with a
> > feature that currently scrapes AGAW out of some VT-d-specific place, so
> > abstracting it to completely generic code, in a way that could eventually be
> > generalised to reporting info for non-nested domains too, would be really
> nice.
>
> got you. I can do that.
>
> > What would also be cool is if the user was able to pass in a structure with
> > preferred values for the address size and other capabilities when they
> request
> > nesting in the first place. Right now we'll always set up the maximum
> possible
> > sized page table for any domain, but if we knew ahead of time how many
> bits the
> > user actually cared about then we could potentially be more efficient (e.g.
> use
> > fewer levels of pagetable or a different translation granule).
>
> agreed, and I guess only the configurable caps (like the addr_width, domain
> could have different addr_width per user request). I think it may be an
> optimization afterward. Here, we report all the nesting related caps to user,
> thus user could either do pre-check or expose correct capability to guest per
> hardware support. This is necesary as nesting requires guest to maintain
> page
> tables per hw supporting.
>
yes, this likely requires a new uAPI thus it could come as an incremental patch
later. We may reuse the same structure as defined here for communicating
preferred values.
Thanks
Kevin
next prev parent reply other threads:[~2020-06-30 1:20 UTC|newest]
Thread overview: 55+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-24 8:55 [PATCH v3 00/14] vfio: expose virtual Shared Virtual Addressing to VMs Liu Yi L
2020-06-24 8:55 ` [PATCH v3 01/14] vfio/type1: Refactor vfio_iommu_type1_ioctl() Liu Yi L
2020-07-02 21:21 ` Alex Williamson
2020-07-03 3:46 ` Liu, Yi L
2020-06-24 8:55 ` [PATCH v3 02/14] iommu: Report domain nesting info Liu Yi L
2020-06-26 7:47 ` Jean-Philippe Brucker
2020-06-26 16:04 ` Robin Murphy
2020-06-27 6:53 ` Liu, Yi L
2020-06-30 1:20 ` Tian, Kevin [this message]
2020-06-27 6:14 ` Liu, Yi L
2020-06-29 9:24 ` Stefan Hajnoczi
2020-06-29 12:23 ` Liu, Yi L
2020-06-30 2:00 ` Tian, Kevin
2020-06-30 3:45 ` Liu, Yi L
2020-07-03 9:59 ` Stefan Hajnoczi
2020-07-02 17:54 ` Alex Williamson
2020-07-03 3:53 ` Liu, Yi L
2020-06-24 8:55 ` [PATCH v3 03/14] vfio/type1: Report iommu nesting info to userspace Liu Yi L
2020-07-02 18:38 ` Alex Williamson
2020-07-03 6:05 ` Liu, Yi L
2020-07-03 13:03 ` Liu, Yi L
2020-06-24 8:55 ` [PATCH v3 04/14] vfio: Add PASID allocation/free support Liu Yi L
2020-07-02 21:17 ` Alex Williamson
2020-07-03 6:08 ` Liu, Yi L
2020-06-24 8:55 ` [PATCH v3 05/14] iommu/vt-d: Support setting ioasid set to domain Liu Yi L
2020-06-24 8:55 ` [PATCH v3 06/14] vfio/type1: Add VFIO_IOMMU_PASID_REQUEST (alloc/free) Liu Yi L
2020-07-02 21:18 ` Alex Williamson
2020-07-03 6:28 ` Liu, Yi L
2020-07-08 8:16 ` Liu, Yi L
2020-07-08 19:54 ` Alex Williamson
2020-07-09 0:32 ` Liu, Yi L
2020-07-09 1:56 ` Tian, Kevin
2020-07-09 2:08 ` Liu, Yi L
2020-07-09 2:18 ` Tian, Kevin
2020-07-09 2:26 ` Liu, Yi L
2020-07-09 7:16 ` Liu, Yi L
2020-07-09 14:27 ` Alex Williamson
2020-07-09 18:05 ` Jacob Pan
2020-07-10 5:39 ` Liu, Yi L
2020-07-10 12:55 ` Alex Williamson
2020-07-10 13:03 ` Liu, Yi L
2020-06-24 8:55 ` [PATCH v3 07/14] iommu: Pass domain to sva_unbind_gpasid() Liu Yi L
2020-06-24 8:55 ` [PATCH v3 08/14] iommu/vt-d: Check ownership for PASIDs from user-space Liu Yi L
2020-06-24 8:55 ` [PATCH v3 09/14] vfio/type1: Support binding guest page tables to PASID Liu Yi L
2020-07-02 21:19 ` Alex Williamson
2020-07-03 6:46 ` Liu, Yi L
2020-06-24 8:55 ` [PATCH v3 10/14] vfio/type1: Allow invalidating first-level/stage IOMMU cache Liu Yi L
2020-07-02 21:19 ` Alex Williamson
2020-07-03 3:47 ` Liu, Yi L
2020-06-24 8:55 ` [PATCH v3 11/14] vfio/type1: Add vSVA support for IOMMU-backed mdevs Liu Yi L
2020-06-24 8:55 ` [PATCH v3 12/14] vfio/pci: Expose PCIe PASID capability to guest Liu Yi L
2020-06-24 8:55 ` [PATCH v3 13/14] vfio: Document dual stage control Liu Yi L
2020-06-29 9:21 ` Stefan Hajnoczi
2020-06-29 9:24 ` Liu, Yi L
2020-06-24 8:55 ` [PATCH v3 14/14] iommu/vt-d: Support reporting nesting capability info Liu Yi L
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=MWHPR11MB16456F2C16A88BB6F84076C78C6F0@MWHPR11MB1645.namprd11.prod.outlook.com \
--to=kevin.tian@intel.com \
--cc=alex.williamson@redhat.com \
--cc=ashok.raj@intel.com \
--cc=baolu.lu@linux.intel.com \
--cc=eric.auger@redhat.com \
--cc=hao.wu@intel.com \
--cc=iommu@lists.linux-foundation.org \
--cc=jacob.jun.pan@linux.intel.com \
--cc=jean-philippe@linaro.org \
--cc=joro@8bytes.org \
--cc=jun.j.tian@intel.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=peterx@redhat.com \
--cc=robin.murphy@arm.com \
--cc=will@kernel.org \
--cc=yi.l.liu@intel.com \
--cc=yi.y.sun@intel.com \
/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).