| Message ID | 1443624989-24346-3-git-send-email-Bharat.Bhushan@freescale.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 2015-09-30 at 20:26 +0530, Bharat Bhushan wrote: > This patch allows the user-space to know whether msi-pages > are automatically mapped with some magic iova or not. > > Even if the msi-pages are automatically mapped, still user-space > wants to over-ride the automatic iova selection for msi-mapping. > For this user-space need to know whether it is allowed to change > the automatic mapping or not and this API provides this mechanism. > Follow up patches will provide how to over-ride this. > > Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com> > --- > drivers/vfio/vfio_iommu_type1.c | 32 ++++++++++++++++++++++++++++++++ > include/uapi/linux/vfio.h | 3 +++ > 2 files changed, 35 insertions(+) > > diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c > index fa5d3e4..3315fb6 100644 > --- a/drivers/vfio/vfio_iommu_type1.c > +++ b/drivers/vfio/vfio_iommu_type1.c > @@ -59,6 +59,7 @@ struct vfio_iommu { > struct rb_root dma_list; > bool v2; > bool nesting; > + bool allow_msi_reconfig; > struct list_head reserved_iova_list; > }; > > @@ -1117,6 +1118,23 @@ static int vfio_domains_have_iommu_cache(struct vfio_iommu *iommu) > return ret; > } > > +static > +int vfio_domains_get_msi_maps(struct vfio_iommu *iommu, > + struct iommu_domain_msi_maps *msi_maps) > +{ > + struct vfio_domain *d; > + int ret; > + > + mutex_lock(&iommu->lock); > + /* All domains have same msi-automap property, pick first */ > + d = list_first_entry(&iommu->domain_list, struct vfio_domain, next); > + ret = iommu_domain_get_attr(d->domain, DOMAIN_ATTR_MSI_MAPPING, > + msi_maps); > + mutex_unlock(&iommu->lock); > + > + return ret; > +} > + > static long vfio_iommu_type1_ioctl(void *iommu_data, > unsigned int cmd, unsigned long arg) > { > @@ -1138,6 +1156,8 @@ static long vfio_iommu_type1_ioctl(void *iommu_data, > } > } else if (cmd == VFIO_IOMMU_GET_INFO) { > struct vfio_iommu_type1_info info; > + struct iommu_domain_msi_maps msi_maps; > + int ret; > > minsz = offsetofend(struct vfio_iommu_type1_info, iova_pgsizes); > > @@ -1149,6 +1169,18 @@ static long vfio_iommu_type1_ioctl(void *iommu_data, > > info.flags = 0; > > + ret = vfio_domains_get_msi_maps(iommu, &msi_maps); > + if (ret) > + return ret; And now ioctl(VFIO_IOMMU_GET_INFO) no longer works for any IOMMU implementing domain_get_attr but not supporting DOMAIN_ATTR_MSI_MAPPING. > + > + if (msi_maps.override_automap) { > + info.flags |= VFIO_IOMMU_INFO_MSI_ALLOW_RECONFIG; > + iommu->allow_msi_reconfig = true; > + } > + > + if (msi_maps.automap) > + info.flags |= VFIO_IOMMU_INFO_MSI_AUTOMAP; > + > info.iova_pgsizes = vfio_pgsize_bitmap(iommu); > > return copy_to_user((void __user *)arg, &info, minsz); > diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h > index 1abd1a9..9998f6e 100644 > --- a/include/uapi/linux/vfio.h > +++ b/include/uapi/linux/vfio.h > @@ -391,6 +391,9 @@ struct vfio_iommu_type1_info { > __u32 argsz; > __u32 flags; > #define VFIO_IOMMU_INFO_PGSIZES (1 << 0) /* supported page sizes info */ > +#define VFIO_IOMMU_INFO_MSI_AUTOMAP (1 << 1) /* MSI pages are auto-mapped > + in iommu */ > +#define VFIO_IOMMU_INFO_MSI_ALLOW_RECONFIG (1 << 2) /* Allows reconfig automap*/ > __u64 iova_pgsizes; /* Bitmap of supported page sizes */ > }; > Once again, exposing interfaces to the user before they actually do anything is backwards. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Alex Williamson [mailto:alex.williamson@redhat.com] > Sent: Saturday, October 03, 2015 4:16 AM > To: Bhushan Bharat-R65777 <Bharat.Bhushan@freescale.com> > Cc: kvmarm@lists.cs.columbia.edu; kvm@vger.kernel.org; > christoffer.dall@linaro.org; eric.auger@linaro.org; pranavkumar@linaro.org; > marc.zyngier@arm.com; will.deacon@arm.com > Subject: Re: [RFC PATCH 3/6] vfio: Extend iommu-info to return MSIs > automap state > > On Wed, 2015-09-30 at 20:26 +0530, Bharat Bhushan wrote: > > This patch allows the user-space to know whether msi-pages are > > automatically mapped with some magic iova or not. > > > > Even if the msi-pages are automatically mapped, still user-space wants > > to over-ride the automatic iova selection for msi-mapping. > > For this user-space need to know whether it is allowed to change the > > automatic mapping or not and this API provides this mechanism. > > Follow up patches will provide how to over-ride this. > > > > Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com> > > --- > > drivers/vfio/vfio_iommu_type1.c | 32 > ++++++++++++++++++++++++++++++++ > > include/uapi/linux/vfio.h | 3 +++ > > 2 files changed, 35 insertions(+) > > > > diff --git a/drivers/vfio/vfio_iommu_type1.c > > b/drivers/vfio/vfio_iommu_type1.c index fa5d3e4..3315fb6 100644 > > --- a/drivers/vfio/vfio_iommu_type1.c > > +++ b/drivers/vfio/vfio_iommu_type1.c > > @@ -59,6 +59,7 @@ struct vfio_iommu { > > struct rb_root dma_list; > > bool v2; > > bool nesting; > > + bool allow_msi_reconfig; > > struct list_head reserved_iova_list; > > }; > > > > @@ -1117,6 +1118,23 @@ static int > vfio_domains_have_iommu_cache(struct vfio_iommu *iommu) > > return ret; > > } > > > > +static > > +int vfio_domains_get_msi_maps(struct vfio_iommu *iommu, > > + struct iommu_domain_msi_maps *msi_maps) { > > + struct vfio_domain *d; > > + int ret; > > + > > + mutex_lock(&iommu->lock); > > + /* All domains have same msi-automap property, pick first */ > > + d = list_first_entry(&iommu->domain_list, struct vfio_domain, next); > > + ret = iommu_domain_get_attr(d->domain, > DOMAIN_ATTR_MSI_MAPPING, > > + msi_maps); > > + mutex_unlock(&iommu->lock); > > + > > + return ret; > > +} > > + > > static long vfio_iommu_type1_ioctl(void *iommu_data, > > unsigned int cmd, unsigned long arg) { @@ > -1138,6 +1156,8 @@ > > static long vfio_iommu_type1_ioctl(void *iommu_data, > > } > > } else if (cmd == VFIO_IOMMU_GET_INFO) { > > struct vfio_iommu_type1_info info; > > + struct iommu_domain_msi_maps msi_maps; > > + int ret; > > > > minsz = offsetofend(struct vfio_iommu_type1_info, > iova_pgsizes); > > > > @@ -1149,6 +1169,18 @@ static long vfio_iommu_type1_ioctl(void > > *iommu_data, > > > > info.flags = 0; > > > > + ret = vfio_domains_get_msi_maps(iommu, &msi_maps); > > + if (ret) > > + return ret; > > And now ioctl(VFIO_IOMMU_GET_INFO) no longer works for any IOMMU > implementing domain_get_attr but not supporting > DOMAIN_ATTR_MSI_MAPPING. With this current patch version this will get the default assumed behavior as you commented on previous patch. > > > + > > + if (msi_maps.override_automap) { > > + info.flags |= > VFIO_IOMMU_INFO_MSI_ALLOW_RECONFIG; > > + iommu->allow_msi_reconfig = true; > > + } > > + > > + if (msi_maps.automap) > > + info.flags |= VFIO_IOMMU_INFO_MSI_AUTOMAP; > > + > > info.iova_pgsizes = vfio_pgsize_bitmap(iommu); > > > > return copy_to_user((void __user *)arg, &info, minsz); diff -- > git > > a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h index > > 1abd1a9..9998f6e 100644 > > --- a/include/uapi/linux/vfio.h > > +++ b/include/uapi/linux/vfio.h > > @@ -391,6 +391,9 @@ struct vfio_iommu_type1_info { > > __u32 argsz; > > __u32 flags; > > #define VFIO_IOMMU_INFO_PGSIZES (1 << 0) /* supported page > sizes info */ > > +#define VFIO_IOMMU_INFO_MSI_AUTOMAP (1 << 1) /* MSI pages > are auto-mapped > > + in iommu */ > > +#define VFIO_IOMMU_INFO_MSI_ALLOW_RECONFIG (1 << 2) /* Allows > > +reconfig automap*/ > > __u64 iova_pgsizes; /* Bitmap of supported page sizes */ > > }; > > > > Once again, exposing interfaces to the user before they actually do anything > is backwards. Will change the order. Thanks -Bharat
On Mon, 2015-10-05 at 06:00 +0000, Bhushan Bharat wrote: > > -1138,6 +1156,8 @@ > > > static long vfio_iommu_type1_ioctl(void *iommu_data, > > > } > > > } else if (cmd == VFIO_IOMMU_GET_INFO) { > > > struct vfio_iommu_type1_info info; > > > + struct iommu_domain_msi_maps msi_maps; > > > + int ret; > > > > > > minsz = offsetofend(struct vfio_iommu_type1_info, > > iova_pgsizes); > > > > > > @@ -1149,6 +1169,18 @@ static long vfio_iommu_type1_ioctl(void > > > *iommu_data, > > > > > > info.flags = 0; > > > > > > + ret = vfio_domains_get_msi_maps(iommu, &msi_maps); > > > + if (ret) > > > + return ret; > > > > And now ioctl(VFIO_IOMMU_GET_INFO) no longer works for any IOMMU > > implementing domain_get_attr but not supporting > > DOMAIN_ATTR_MSI_MAPPING. > > With this current patch version this will get the default assumed behavior as you commented on previous patch. How so? + msi_maps->automap = true; + msi_maps->override_automap = false; + + if (domain->ops->domain_get_attr) + ret = domain->ops->domain_get_attr(domain, attr, data); If domain_get_attr is implemented, but DOMAIN_ATTR_MSI_MAPPING is not, ret should be an error code. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Alex Williamson [mailto:alex.williamson@redhat.com] > Sent: Tuesday, October 06, 2015 4:15 AM > To: Bhushan Bharat-R65777 <Bharat.Bhushan@freescale.com> > Cc: kvmarm@lists.cs.columbia.edu; kvm@vger.kernel.org; > christoffer.dall@linaro.org; eric.auger@linaro.org; pranavkumar@linaro.org; > marc.zyngier@arm.com; will.deacon@arm.com > Subject: Re: [RFC PATCH 3/6] vfio: Extend iommu-info to return MSIs > automap state > > On Mon, 2015-10-05 at 06:00 +0000, Bhushan Bharat wrote: > > > -1138,6 +1156,8 @@ > > > > static long vfio_iommu_type1_ioctl(void *iommu_data, > > > > } > > > > } else if (cmd == VFIO_IOMMU_GET_INFO) { > > > > struct vfio_iommu_type1_info info; > > > > + struct iommu_domain_msi_maps msi_maps; > > > > + int ret; > > > > > > > > minsz = offsetofend(struct vfio_iommu_type1_info, > > > iova_pgsizes); > > > > > > > > @@ -1149,6 +1169,18 @@ static long vfio_iommu_type1_ioctl(void > > > > *iommu_data, > > > > > > > > info.flags = 0; > > > > > > > > + ret = vfio_domains_get_msi_maps(iommu, &msi_maps); > > > > + if (ret) > > > > + return ret; > > > > > > And now ioctl(VFIO_IOMMU_GET_INFO) no longer works for any > IOMMU > > > implementing domain_get_attr but not supporting > > > DOMAIN_ATTR_MSI_MAPPING. > > > > With this current patch version this will get the default assumed behavior > as you commented on previous patch. > > How so? You are right, the ioctl will return failure. But that should be ok, right? > > + msi_maps->automap = true; > + msi_maps->override_automap = false; > + > + if (domain->ops->domain_get_attr) > + ret = domain->ops->domain_get_attr(domain, attr, > + data); > > If domain_get_attr is implemented, but DOMAIN_ATTR_MSI_MAPPING is > not, ret should be an error code. Currently it returns same error code returned by domain->ops->domain_get_attr(). I do not think we want to complicate that we return an error to user-space that msi's probably cannot be used but user-space can continue with Legacy interrupt, or you want that? Thanks -Bharat
On Tue, 2015-10-06 at 08:53 +0000, Bhushan Bharat wrote: > > > > -----Original Message----- > > From: Alex Williamson [mailto:alex.williamson@redhat.com] > > Sent: Tuesday, October 06, 2015 4:15 AM > > To: Bhushan Bharat-R65777 <Bharat.Bhushan@freescale.com> > > Cc: kvmarm@lists.cs.columbia.edu; kvm@vger.kernel.org; > > christoffer.dall@linaro.org; eric.auger@linaro.org; pranavkumar@linaro.org; > > marc.zyngier@arm.com; will.deacon@arm.com > > Subject: Re: [RFC PATCH 3/6] vfio: Extend iommu-info to return MSIs > > automap state > > > > On Mon, 2015-10-05 at 06:00 +0000, Bhushan Bharat wrote: > > > > -1138,6 +1156,8 @@ > > > > > static long vfio_iommu_type1_ioctl(void *iommu_data, > > > > > } > > > > > } else if (cmd == VFIO_IOMMU_GET_INFO) { > > > > > struct vfio_iommu_type1_info info; > > > > > + struct iommu_domain_msi_maps msi_maps; > > > > > + int ret; > > > > > > > > > > minsz = offsetofend(struct vfio_iommu_type1_info, > > > > iova_pgsizes); > > > > > > > > > > @@ -1149,6 +1169,18 @@ static long vfio_iommu_type1_ioctl(void > > > > > *iommu_data, > > > > > > > > > > info.flags = 0; > > > > > > > > > > + ret = vfio_domains_get_msi_maps(iommu, &msi_maps); > > > > > + if (ret) > > > > > + return ret; > > > > > > > > And now ioctl(VFIO_IOMMU_GET_INFO) no longer works for any > > IOMMU > > > > implementing domain_get_attr but not supporting > > > > DOMAIN_ATTR_MSI_MAPPING. > > > > > > With this current patch version this will get the default assumed behavior > > as you commented on previous patch. > > > > How so? > > You are right, the ioctl will return failure. But that should be ok, right? Not remotely. ioctl(VFIO_IOMMU_GET_INFO) can't suddenly stop working on some platforms. > > > > + msi_maps->automap = true; > > + msi_maps->override_automap = false; > > + > > + if (domain->ops->domain_get_attr) > > + ret = domain->ops->domain_get_attr(domain, attr, > > + data); > > > > If domain_get_attr is implemented, but DOMAIN_ATTR_MSI_MAPPING is > > not, ret should be an error code. > > Currently it returns same error code returned by domain->ops->domain_get_attr(). > I do not think we want to complicate that we return an error to user-space that msi's probably cannot be used but user-space can continue with Legacy interrupt, or you want that? I can't really parse your statement, but ioctl(VFIO_IOMMU_GET_INFO) works today and it must work with your changes. Your change should only affect whether some flags are visible, MSI has worked just fine up to this point on other platforms. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c index fa5d3e4..3315fb6 100644 --- a/drivers/vfio/vfio_iommu_type1.c +++ b/drivers/vfio/vfio_iommu_type1.c @@ -59,6 +59,7 @@ struct vfio_iommu { struct rb_root dma_list; bool v2; bool nesting; + bool allow_msi_reconfig; struct list_head reserved_iova_list; }; @@ -1117,6 +1118,23 @@ static int vfio_domains_have_iommu_cache(struct vfio_iommu *iommu) return ret; } +static +int vfio_domains_get_msi_maps(struct vfio_iommu *iommu, + struct iommu_domain_msi_maps *msi_maps) +{ + struct vfio_domain *d; + int ret; + + mutex_lock(&iommu->lock); + /* All domains have same msi-automap property, pick first */ + d = list_first_entry(&iommu->domain_list, struct vfio_domain, next); + ret = iommu_domain_get_attr(d->domain, DOMAIN_ATTR_MSI_MAPPING, + msi_maps); + mutex_unlock(&iommu->lock); + + return ret; +} + static long vfio_iommu_type1_ioctl(void *iommu_data, unsigned int cmd, unsigned long arg) { @@ -1138,6 +1156,8 @@ static long vfio_iommu_type1_ioctl(void *iommu_data, } } else if (cmd == VFIO_IOMMU_GET_INFO) { struct vfio_iommu_type1_info info; + struct iommu_domain_msi_maps msi_maps; + int ret; minsz = offsetofend(struct vfio_iommu_type1_info, iova_pgsizes); @@ -1149,6 +1169,18 @@ static long vfio_iommu_type1_ioctl(void *iommu_data, info.flags = 0; + ret = vfio_domains_get_msi_maps(iommu, &msi_maps); + if (ret) + return ret; + + if (msi_maps.override_automap) { + info.flags |= VFIO_IOMMU_INFO_MSI_ALLOW_RECONFIG; + iommu->allow_msi_reconfig = true; + } + + if (msi_maps.automap) + info.flags |= VFIO_IOMMU_INFO_MSI_AUTOMAP; + info.iova_pgsizes = vfio_pgsize_bitmap(iommu); return copy_to_user((void __user *)arg, &info, minsz); diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h index 1abd1a9..9998f6e 100644 --- a/include/uapi/linux/vfio.h +++ b/include/uapi/linux/vfio.h @@ -391,6 +391,9 @@ struct vfio_iommu_type1_info { __u32 argsz; __u32 flags; #define VFIO_IOMMU_INFO_PGSIZES (1 << 0) /* supported page sizes info */ +#define VFIO_IOMMU_INFO_MSI_AUTOMAP (1 << 1) /* MSI pages are auto-mapped + in iommu */ +#define VFIO_IOMMU_INFO_MSI_ALLOW_RECONFIG (1 << 2) /* Allows reconfig automap*/ __u64 iova_pgsizes; /* Bitmap of supported page sizes */ };
This patch allows the user-space to know whether msi-pages are automatically mapped with some magic iova or not. Even if the msi-pages are automatically mapped, still user-space wants to over-ride the automatic iova selection for msi-mapping. For this user-space need to know whether it is allowed to change the automatic mapping or not and this API provides this mechanism. Follow up patches will provide how to over-ride this. Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com> --- drivers/vfio/vfio_iommu_type1.c | 32 ++++++++++++++++++++++++++++++++ include/uapi/linux/vfio.h | 3 +++ 2 files changed, 35 insertions(+)