Re: [PATCH V2 2/5] iommu/vt-d: Rename device_def_domain_type() to intel_iommu_dev_def_domain_type()
Hi, On 2020/2/23 7:59, Prakhya, Sai Praneeth wrote: On 2020/2/17 5:57, Sai Praneeth Prakhya wrote: The functionality needed for iommu_ops->dev_def_domain_type() is already provided by device_def_domain_type() in intel_iommu.c. But, every call back function in intel_iommu_ops starts with intel_iommu prefix, hence rename device_def_domain_type() to intel_iommu_dev_def_domain_type() so that it follows the same semantics. How about keep device_def_domain_type() and call it in the new intel_iommu_dev_def_domain_type()? Sure! I could but could you please explain the advantages we might get by doing so? Less number of changes is what I could think of.. any other reasons? device_def_domain_type() is a quirk list for devices that must use a specified domain type. intel_iommu_dev_def_domain_type() tells the upper layer whether the device could switch to another type of domain. Put them in separated functions will make it easier for maintenance. Best regards, baolu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH V2 5/5] iommu: Document usage of "/sys/kernel/iommu_groups//type" file
Hi, On 2020/2/17 5:57, Sai Praneeth Prakhya wrote: The default domain type of an iommu group can be changed using file "/sys/kernel/iommu_groups//type". Hence, document it's usage and more importantly spell out it's limitations. Cc: Christoph Hellwig Cc: Joerg Roedel Cc: Ashok Raj Cc: Will Deacon Cc: Lu Baolu Cc: Sohil Mehta Cc: Robin Murphy Cc: Jacob Pan Signed-off-by: Sai Praneeth Prakhya --- .../ABI/testing/sysfs-kernel-iommu_groups | 29 ++ 1 file changed, 29 insertions(+) diff --git a/Documentation/ABI/testing/sysfs-kernel-iommu_groups b/Documentation/ABI/testing/sysfs-kernel-iommu_groups index 017f5bc3920c..808a9507b281 100644 --- a/Documentation/ABI/testing/sysfs-kernel-iommu_groups +++ b/Documentation/ABI/testing/sysfs-kernel-iommu_groups @@ -33,3 +33,32 @@ Description:In case an RMRR is used only by graphics or USB devices it is now exposed as "direct-relaxable" instead of "direct". In device assignment use case, for instance, those RMRR are considered to be relaxable and safe. + +What: /sys/kernel/iommu_groups//type +Date: February 2020 +KernelVersion: v5.6 +Contact: Sai Praneeth Prakhya +Description: Lets the user know the type of default domain in use by iommu + for this group. A privileged user could request kernel to change Let the users know the default domain type of the IOMMU group by reading this file. A privileged user could request to change it by writing to this file. + the group type by writing to this file. Presently, only three + types are supported + 1. DMA: All the DMA transactions from the devices in this group + are translated by the iommu. + 2. identity: All the DMA transactions from the devices in this +group are *not* translated by the iommu. + 3. auto: Kernel decides one among DMA/identity mode and hence +when the user reads the file he would never see "auto". Just out of curiosity, when could a user reads "auto" from this file? +This is just a write only value. + Note: + - + A group type could be modified only when *all* the devices in group's default domain type (not group type). + the group are not binded to any device driver. So, the user has bound + to first unbind the appropriate drivers and then change the + default domain type. + Caution: + + Unbinding a device driver will take away the drivers control + over the device and if done on devices that host root file + system could lead to catastrophic effects (the user might + need to reboot the machine to get it to normal state). So, it's + expected that the user understands what he is doing. Best regards, baolu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH V2 3/5] iommu: Add support to change default domain of an iommu_group
Hi,
On 2020/2/17 5:57, Sai Praneeth Prakhya wrote:
Presently, the default domain of an iommu_group is allocated during boot
time (i.e. when a device is being added to a group) and it cannot be
changed later. So, the device would typically be either in identity (also
known as pass_through) mode (controlled by "iommu=pt" kernel command line
The default domain type is initialized according to the kernel build
option, and could be overrided by several kernel commands like iommu=pt
and iommu.passthrough.
argument) or the device would be in DMA mode as long as the machine is up
and running. There is no way to change the default domain type dynamically
i.e. after booting, a device cannot switch between identity mode and DMA
mode.
But, assume a use case wherein the user trusts the device and also believes
that the OS is secure enough and hence wants *only* this device to bypass
IOMMU (so that it could be high performing) whereas all the other devices
to go through IOMMU (so that the system is protected). Presently, this use
case is not supported. Hence it will be helpful if there is some way to
change the default domain of a B:D.F dynamically. Since, linux iommu
subsystem prefers to deal at iommu_group level instead of B:D.F level, it
might be helpful if there is some way to change the default domain of a
*iommu_group* dynamically. Hence, add such support.
A privileged user could request the kernel to change the default domain
type of a iommu_group by writing to
"/sys/kernel/iommu_groups//type"
file. Presently, only three values are supported
1. identity: all the DMA transactions from the devices in this group are
*not* translated by the iommu
2. DMA: all the DMA transactions from the devices in this group are
translated by the iommu
3. auto: kernel decides one among DMA/identity
Also please note that a group type could be modified only when *all* the
devices in the group are not binded to any device driver. Please see
"Documentation/ABI/testing/sysfs-kernel-iommu_groups" for more information.
Cc: Christoph Hellwig
Cc: Joerg Roedel
Cc: Ashok Raj
Cc: Will Deacon
Cc: Lu Baolu
Cc: Sohil Mehta
Cc: Robin Murphy
Cc: Jacob Pan
Signed-off-by: Sai Praneeth Prakhya
---
drivers/iommu/iommu.c | 227 +-
1 file changed, 226 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 3e3528436e0b..56a29076871f 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -225,6 +225,8 @@ static int __iommu_attach_group(struct iommu_domain *domain,
struct iommu_group *group);
static void __iommu_detach_group(struct iommu_domain *domain,
struct iommu_group *group);
+static ssize_t iommu_group_store_type(struct iommu_group *group,
+ const char *buf, size_t count);
static int __init iommu_set_def_domain_type(char *str)
{
@@ -448,7 +450,8 @@ static IOMMU_GROUP_ATTR(name, S_IRUGO,
iommu_group_show_name, NULL);
static IOMMU_GROUP_ATTR(reserved_regions, 0444,
iommu_group_show_resv_regions, NULL);
-static IOMMU_GROUP_ATTR(type, 0444, iommu_group_show_type, NULL);
+static IOMMU_GROUP_ATTR(type, 0644, iommu_group_show_type,
+ iommu_group_store_type);
static void iommu_group_release(struct kobject *kobj)
{
@@ -2654,3 +2657,225 @@ int iommu_sva_get_pasid(struct iommu_sva *handle)
return ops->sva_get_pasid(handle);
}
EXPORT_SYMBOL_GPL(iommu_sva_get_pasid);
+
+/*
+ * Changes the default domain of a group
+ *
+ * @group: The group for which the default domain should be changed
+ * @prv_dom: The previous domain that is being switched from
The previous domain is still kept in group->default_domain, so it's
unnecessary to pass it as a parameter, right?
+ * @type: The type of the new default domain that gets associated with the
group
+ *
+ * Returns 0 on success and error code on failure
+ */
+static int iommu_group_change_def_domain(struct iommu_group *group,
+struct iommu_domain *prv_dom,
+int type)
+{
+ struct group_device *grp_dev, *temp;
+ struct iommu_domain *new_domain;
+ int ret;
+
+ /*
+* iommu_domain_alloc() takes "struct bus_type" as an argument which is
+* a member in "struct device". Changing a group's default domain type
+* deals at iommu_group level rather than device level and hence there
+* is no straight forward way to get "bus_type" of an iommu_group that
+* could be passed to iommu_domain_alloc(). So, instead of directly
+* calling iommu_domain_alloc(), use iommu_ops from previous default
+* domain.
+*/
+ if (!prv_dom || !prv_dom->ops || !prv_dom->ops->domain_alloc || !type)
+ return -EINVAL;
+
+ /* Allocate a new domain of
RE: [PATCH V2 2/5] iommu/vt-d: Rename device_def_domain_type() to intel_iommu_dev_def_domain_type()
> > On 2020/2/17 5:57, Sai Praneeth Prakhya wrote: > > The functionality needed for iommu_ops->dev_def_domain_type() is > > already provided by device_def_domain_type() in intel_iommu.c. But, > > every call back function in intel_iommu_ops starts with intel_iommu > > prefix, hence rename > > device_def_domain_type() to intel_iommu_dev_def_domain_type() so that > > it follows the same semantics. > > How about keep device_def_domain_type() and call it in the new > intel_iommu_dev_def_domain_type()? Sure! I could but could you please explain the advantages we might get by doing so? Less number of changes is what I could think of.. any other reasons? Regards, Sai ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH V2 2/5] iommu/vt-d: Rename device_def_domain_type() to intel_iommu_dev_def_domain_type()
Hi,
On 2020/2/17 5:57, Sai Praneeth Prakhya wrote:
The functionality needed for iommu_ops->dev_def_domain_type() is already
provided by device_def_domain_type() in intel_iommu.c. But, every call back
function in intel_iommu_ops starts with intel_iommu prefix, hence rename
device_def_domain_type() to intel_iommu_dev_def_domain_type() so that it
follows the same semantics.
How about keep device_def_domain_type() and call it in the new
intel_iommu_dev_def_domain_type()?
Best regards,
baolu
Cc: Christoph Hellwig
Cc: Joerg Roedel
Cc: Ashok Raj
Cc: Will Deacon
Cc: Lu Baolu
Cc: Sohil Mehta
Cc: Robin Murphy
Cc: Jacob Pan
Signed-off-by: Sai Praneeth Prakhya
---
drivers/iommu/intel-iommu.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 64ddccf1d5fe..68f10d271ac0 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -3034,7 +3034,7 @@ static bool device_is_rmrr_locked(struct device *dev)
* - IOMMU_DOMAIN_IDENTITY: device requires an identical mapping domain
* - 0: both identity and dynamic domains work for this device
*/
-static int device_def_domain_type(struct device *dev)
+static int intel_iommu_dev_def_domain_type(struct device *dev)
{
if (dev_is_pci(dev)) {
struct pci_dev *pdev = to_pci_dev(dev);
@@ -5796,7 +5796,8 @@ static int intel_iommu_add_device(struct device *dev)
domain = iommu_get_domain_for_dev(dev);
dmar_domain = to_dmar_domain(domain);
if (domain->type == IOMMU_DOMAIN_DMA) {
- if (device_def_domain_type(dev) == IOMMU_DOMAIN_IDENTITY) {
+ if (intel_iommu_dev_def_domain_type(dev) ==
+ IOMMU_DOMAIN_IDENTITY) {
ret = iommu_request_dm_for_dev(dev);
if (ret) {
dmar_remove_one_dev_info(dev);
@@ -5807,7 +5808,7 @@ static int intel_iommu_add_device(struct device *dev)
}
}
} else {
- if (device_def_domain_type(dev) == IOMMU_DOMAIN_DMA) {
+ if (intel_iommu_dev_def_domain_type(dev) == IOMMU_DOMAIN_DMA) {
ret = iommu_request_dma_domain_for_dev(dev);
if (ret) {
dmar_remove_one_dev_info(dev);
@@ -6194,6 +6195,7 @@ const struct iommu_ops intel_iommu_ops = {
.dev_enable_feat= intel_iommu_dev_enable_feat,
.dev_disable_feat = intel_iommu_dev_disable_feat,
.is_attach_deferred = intel_iommu_is_attach_deferred,
+ .dev_def_domain_type= intel_iommu_dev_def_domain_type,
.pgsize_bitmap = INTEL_IOMMU_PGSIZES,
};
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH V2 0/5] iommu: Add support to change default domain of a group
Hi Joerg, > Presently, the default domain of a group is allocated during boot time and it > cannot be changed later. So, the device would typically be either in identity > (pass_through) mode or the device would be in DMA mode as long as the system > is up and running. There is no way to change the default domain type > dynamically i.e. after booting, a device cannot switch between identity mode > and DMA mode. > > Assume a use case wherein the privileged user would want to use the device in > pass-through mode when the device is used for host so that it would be high > performing. Presently, this is not supported. Hence add support to change the > default domain of a group dynamically. > > Support this through a sysfs file, namely > "/sys/kernel/iommu_groups//type". > > Hi Joerg, > Sorry! for _huge_ delay in posting a V2 of this patch set. Was stuck with some > internal PoC work. Will be consistent from now. > > Changes from V1: > > 1. V1 patch set wasn't updating dma_ops for some vendors (Eg: AMD), hence, >change the logic of updating default domain as below (because adding a > device >to iommu_group automatically updates dma_ops) >a. Allocate a new domain >b. For every device in the group > i. Remove the device from the group > ii. Add the device back to the group >c. Free previous domain > 2. Drop 1st patch of V1 (iommu/vt-d: Modify device_def_domain_type() to use > at >runtime) because "iommu=pt" has no effect on this function anymore. > 3. Added a patch to take/release lock while reading iommu_group- > >default_domain->type >because it can be changed any time by user. > 4. Before changing default domain type of a group, check if the group is >directly assigned for user level access. If so, abort. > 5. Sanitize return path (using ternary operator) in iommu_group_store_type() > 6. > Split 2nd patch of V1 (iommu: Add device_def_domain_type() call back function >to iommu_ops) into two patches such that iommu generic changes are now in > 1st >patch of V2 and vt-d specific changes are in 2nd patch of V2. > 7. Rename device_def_domain_type() to dev_def_domain_type() 8. Remove > example from documentation 9. Change the value written to file > "/sys/kernel/iommu_groups//type" >from "dma" to "DMA". Just a gentle reminder, could you please review this patch set and let me know your comments? Regards, Sai ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH V2 1/5] iommu: Add dev_def_domain_type() call back function to iommu_ops
>
> On 2020/2/17 5:57, Sai Praneeth Prakhya wrote:
> > When user requests kernel to change the default domain type of a group
> > through sysfs, kernel has to make sure that it's ok to change the
> > domain type of every device in the group to the requested domain
> > (every device may not support both the domain types i.e. DMA and
> > identity). Hence, add a call back function that could be implemented
> > per architecture that performs the above check.
> >
> > Cc: Christoph Hellwig
> > Cc: Joerg Roedel
> > Cc: Ashok Raj
> > Cc: Will Deacon
> > Cc: Lu Baolu
> > Cc: Sohil Mehta
> > Cc: Robin Murphy
> > Cc: Jacob Pan
> > Signed-off-by: Sai Praneeth Prakhya
> > ---
> > include/linux/iommu.h | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/include/linux/iommu.h b/include/linux/iommu.h index
> > d1b5f4d98569..3f4aaad0aeb7 100644
> > --- a/include/linux/iommu.h
> > +++ b/include/linux/iommu.h
> > @@ -248,6 +248,7 @@ struct iommu_iotlb_gather {
> >* @cache_invalidate: invalidate translation caches
> >* @sva_bind_gpasid: bind guest pasid and mm
> >* @sva_unbind_gpasid: unbind guest pasid and mm
> > + * @dev_def_domain_type: Return the required default domain type for
> > + a device
>
> Can you please define the return value of this callback?
Sure! I will add it in the next version
Regards,
Sai
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH V2 1/5] iommu: Add dev_def_domain_type() call back function to iommu_ops
Hi Sai,
On 2020/2/17 5:57, Sai Praneeth Prakhya wrote:
When user requests kernel to change the default domain type of a group
through sysfs, kernel has to make sure that it's ok to change the domain
type of every device in the group to the requested domain (every device may
not support both the domain types i.e. DMA and identity). Hence, add a call
back function that could be implemented per architecture that performs the
above check.
Cc: Christoph Hellwig
Cc: Joerg Roedel
Cc: Ashok Raj
Cc: Will Deacon
Cc: Lu Baolu
Cc: Sohil Mehta
Cc: Robin Murphy
Cc: Jacob Pan
Signed-off-by: Sai Praneeth Prakhya
---
include/linux/iommu.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index d1b5f4d98569..3f4aaad0aeb7 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -248,6 +248,7 @@ struct iommu_iotlb_gather {
* @cache_invalidate: invalidate translation caches
* @sva_bind_gpasid: bind guest pasid and mm
* @sva_unbind_gpasid: unbind guest pasid and mm
+ * @dev_def_domain_type: Return the required default domain type for a device
Can you please define the return value of this callback?
Best regards,
baolu
* @pgsize_bitmap: bitmap of all possible supported page sizes
* @owner: Driver module providing these ops
*/
@@ -318,6 +319,8 @@ struct iommu_ops {
int (*sva_unbind_gpasid)(struct device *dev, int pasid);
+ int (*dev_def_domain_type)(struct device *dev);
+
unsigned long pgsize_bitmap;
struct module *owner;
};
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 2/2] virtio: let virtio use DMA API when guest RAM is protected
On Fri, Feb 21, 2020 at 03:33:40PM +0100, Halil Pasic wrote: > AFAIU you have a positive attitude towards the idea, that > !F_VIRTIO_PLATFORM implies 'no DMA API is used by virtio' > should be scrapped. > > I would like to accomplish that without adverse effects to virtio-ccw > (because caring for virtio-ccw is a part of job description). > > Regards, > Halil It is possible, in theory. IIRC the main challenge is that DMA API has overhead of indirect function calls even when all it does it return back the PA without changes. So that will lead to a measureable performance degradation. That might be fixable, possibly using some kind of logic along the lines of if (iova is pa) return pa else indirect call and for unmapping, we might need an API that says "unmap is a nop, safe to skip" so we don't maintain the dma address until unmap. -- MST ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu/vt-d: Fix a bug in intel_iommu_iova_to_phys() for huge page
On Thu, Feb 20, 2020 at 11:44:31AM -0800, Yonghyun Hwang wrote: > intel_iommu_iova_to_phys() has a bug when it translates an IOVA for a huge > page onto its corresponding physical address. This commit fixes the bug by > accomodating the level of page entry for the IOVA and adds IOVA's lower > address to the physical address. > > Signed-off-by: Yonghyun Hwang Reviewed-by: Moritz Fischer > --- > > Changes from v1: > - level cannot be 0. So, the condition, "if (level > 1)", is removed, which > results in a simple code. > - a macro, BIT_MASK, is used to have a bit mask > > --- > drivers/iommu/intel-iommu.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c > index 932267f49f9a..4fd5c6287b6d 100644 > --- a/drivers/iommu/intel-iommu.c > +++ b/drivers/iommu/intel-iommu.c > @@ -5554,7 +5554,9 @@ static phys_addr_t intel_iommu_iova_to_phys(struct > iommu_domain *domain, > > pte = pfn_to_dma_pte(dmar_domain, iova >> VTD_PAGE_SHIFT, ); > if (pte) > - phys = dma_pte_addr(pte); > + phys = dma_pte_addr(pte) + > + (iova & (BIT_MASK(level_to_offset_bits(level) + > + VTD_PAGE_SHIFT) - 1)); > > return phys; > } > -- > 2.25.0.265.gbab2e86ba0-goog > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu/vt-d: Fix a bug in intel_iommu_iova_to_phys() for huge page
Hi, On 2020/2/22 21:05, Lu Baolu wrote: Hi, On 2020/2/21 3:44, Yonghyun Hwang wrote: intel_iommu_iova_to_phys() has a bug when it translates an IOVA for a huge page onto its corresponding physical address. This commit fixes the bug by accomodating the level of page entry for the IOVA and adds IOVA's lower address to the physical address. Signed-off-by: Yonghyun Hwang This fix looks good to me. Cc: # As far back as possible The email address should be: sta...@vger.kernel.org. Sorry about the typo. Best regards, baolu Acked-by: Lu Baolu Best regards, baolu --- Changes from v1: - level cannot be 0. So, the condition, "if (level > 1)", is removed, which results in a simple code. - a macro, BIT_MASK, is used to have a bit mask --- drivers/iommu/intel-iommu.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c index 932267f49f9a..4fd5c6287b6d 100644 --- a/drivers/iommu/intel-iommu.c +++ b/drivers/iommu/intel-iommu.c @@ -5554,7 +5554,9 @@ static phys_addr_t intel_iommu_iova_to_phys(struct iommu_domain *domain, pte = pfn_to_dma_pte(dmar_domain, iova >> VTD_PAGE_SHIFT, ); if (pte) - phys = dma_pte_addr(pte); + phys = dma_pte_addr(pte) + + (iova & (BIT_MASK(level_to_offset_bits(level) + + VTD_PAGE_SHIFT) - 1)); return phys; } ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu/vt-d: Fix a bug in intel_iommu_iova_to_phys() for huge page
Hi, On 2020/2/21 3:44, Yonghyun Hwang wrote: intel_iommu_iova_to_phys() has a bug when it translates an IOVA for a huge page onto its corresponding physical address. This commit fixes the bug by accomodating the level of page entry for the IOVA and adds IOVA's lower address to the physical address. Signed-off-by: Yonghyun Hwang This fix looks good to me. Cc: # As far back as possible Acked-by: Lu Baolu Best regards, baolu --- Changes from v1: - level cannot be 0. So, the condition, "if (level > 1)", is removed, which results in a simple code. - a macro, BIT_MASK, is used to have a bit mask --- drivers/iommu/intel-iommu.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c index 932267f49f9a..4fd5c6287b6d 100644 --- a/drivers/iommu/intel-iommu.c +++ b/drivers/iommu/intel-iommu.c @@ -5554,7 +5554,9 @@ static phys_addr_t intel_iommu_iova_to_phys(struct iommu_domain *domain, pte = pfn_to_dma_pte(dmar_domain, iova >> VTD_PAGE_SHIFT, ); if (pte) - phys = dma_pte_addr(pte); + phys = dma_pte_addr(pte) + + (iova & (BIT_MASK(level_to_offset_bits(level) + + VTD_PAGE_SHIFT) - 1)); return phys; } ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
