Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1398343
| From | Robin Murphy <robin.murphy@arm.com> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH v9 5/7] vfio/type1: also check IRQ remapping capability at msi domain |
| Date | 2016-05-10 19:30 +0200 |
| Message-ID | <rxjDP-75T-5@gated-at.bofh.it> (permalink) |
| References | <rv3Db-38F-1@gated-at.bofh.it> <rv3Dc-38F-3@gated-at.bofh.it> <rx29Y-6uv-13@gated-at.bofh.it> <rxiy6-5Zu-19@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
Hi Eric,
On 10/05/16 17:10, Eric Auger wrote:
> Hi Alex,
> On 05/10/2016 12:49 AM, Alex Williamson wrote:
>> On Wed, 4 May 2016 11:54:16 +0000
>> Eric Auger <eric.auger@linaro.org> wrote:
>>
>>> On x86 IRQ remapping is abstracted by the IOMMU. On ARM this is abstracted
>>> by the msi controller. vfio_safe_irq_domain allows to check whether
>>> interrupts are "safe" for a given device. They are if the device does
>>> not use MSI
>>
>> Are we sure we're not opening a security hole here? An MSI is simply a
>> DMA write, so really whether or not a device uses MSI is irrelevant.
>> If it can generate a DMA to the MSI doorbell then we need to be
>> protected and I think we pretty much need to assume that devices are
>> DMA capable. Do the MSI domain checks cover this?
> Let me try to rephrase: we check the device is not attached to an MSI
> controller (I think this is the semantic of dev_get_msi_domain(dev)).
>
> If it is not, we don't have to care about MSI isolation: there will be
> no IOMMU binding between the device and any MSI doorbell. If it is we
> check the msi domain is backed by an MSI controller able to perform MSI
> isolation.
>
> So effectively "usage of MSIs" is improper - since it is decided after
> the group attachment anyway - and the commit message should rather
> state "if the device is linked to an MSI controller" (dt msi-parent
> notion I think).
Hmm, I think Alex has a point here - on a GICv2m I can happily fire
arbitrary MSIs from _a shell_ (using /dev/mem), and the CPUs definitely
aren't in an MSI domain, so I don't think it's valid to assume that a
device using only wired interrupts, therefore with no connection to any
MSI controller, isn't still capable of maliciously spewing DMA all over
any and every doorbell region in the system.
Robin.
> Does it sound better?
>
> Regards
>
> Eric
>>
>>> or if the device uses MSI and the msi-parent controller
>>> supports IRQ remapping.
>>>
>>> Then we check at group level if all devices have safe interrupts: if not,
>>> we only allow the group to be attached if allow_unsafe_interrupts is set.
>>>
>>> At this point ARM sMMU still advertises IOMMU_CAP_INTR_REMAP. This is
>>> changed in next patch.
>>>
>>> Signed-off-by: Eric Auger <eric.auger@linaro.org>
>>>
>>> ---
>>> v3 -> v4:
>>> - rename vfio_msi_parent_irq_remapping_capable into vfio_safe_irq_domain
>>> and irq_remapping into safe_irq_domains
>>>
>>> v2 -> v3:
>>> - protect vfio_msi_parent_irq_remapping_capable with
>>> CONFIG_GENERIC_MSI_IRQ_DOMAIN
>>> ---
>>> drivers/vfio/vfio_iommu_type1.c | 44 +++++++++++++++++++++++++++++++++++++++--
>>> 1 file changed, 42 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
>>> index 4d3a6f1..2fc8197 100644
>>> --- a/drivers/vfio/vfio_iommu_type1.c
>>> +++ b/drivers/vfio/vfio_iommu_type1.c
>>> @@ -37,6 +37,8 @@
>>> #include <linux/vfio.h>
>>> #include <linux/workqueue.h>
>>> #include <linux/msi-iommu.h>
>>> +#include <linux/irqdomain.h>
>>> +#include <linux/msi.h>
>>>
>>> #define DRIVER_VERSION "0.2"
>>> #define DRIVER_AUTHOR "Alex Williamson <alex.williamson@redhat.com>"
>>> @@ -777,6 +779,33 @@ static int vfio_bus_type(struct device *dev, void *data)
>>> return 0;
>>> }
>>>
>>> +/**
>>> + * vfio_safe_irq_domain: returns whether the irq domain
>>> + * the device is attached to is safe with respect to MSI isolation.
>>> + * If the irq domain is not an MSI domain, we return it is safe.
>>> + *
>>> + * @dev: device handle
>>> + * @data: unused
>>> + * returns 0 if the irq domain is safe, -1 if not.
>>> + */
>>> +static int vfio_safe_irq_domain(struct device *dev, void *data)
>>> +{
>>> +#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
>>> + struct irq_domain *domain;
>>> + struct msi_domain_info *info;
>>> +
>>> + domain = dev_get_msi_domain(dev);
>>> + if (!domain)
>>> + return 0;
>>> +
>>> + info = msi_get_domain_info(domain);
>>> +
>>> + if (!(info->flags & MSI_FLAG_IRQ_REMAPPING))
>>> + return -1;
>>> +#endif
>>> + return 0;
>>> +}
>>> +
>>> static int vfio_iommu_replay(struct vfio_iommu *iommu,
>>> struct vfio_domain *domain)
>>> {
>>> @@ -870,7 +899,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
>>> struct vfio_group *group, *g;
>>> struct vfio_domain *domain, *d;
>>> struct bus_type *bus = NULL;
>>> - int ret;
>>> + int ret, safe_irq_domains;
>>>
>>> mutex_lock(&iommu->lock);
>>>
>>> @@ -893,6 +922,13 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
>>>
>>> group->iommu_group = iommu_group;
>>>
>>> + /*
>>> + * Determine if all the devices of the group have a safe irq domain
>>> + * with respect to MSI isolation
>>> + */
>>> + safe_irq_domains = !iommu_group_for_each_dev(iommu_group, &bus,
>>> + vfio_safe_irq_domain);
>>> +
>>> /* Determine bus_type in order to allocate a domain */
>>> ret = iommu_group_for_each_dev(iommu_group, &bus, vfio_bus_type);
>>> if (ret)
>>> @@ -920,8 +956,12 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
>>> INIT_LIST_HEAD(&domain->group_list);
>>> list_add(&group->next, &domain->group_list);
>>>
>>> + /*
>>> + * to advertise safe interrupts either the IOMMU or the MSI controllers
>>> + * must support IRQ remapping/interrupt translation
>>> + */
>>> if (!allow_unsafe_interrupts &&
>>> - !iommu_capable(bus, IOMMU_CAP_INTR_REMAP)) {
>>> + (!iommu_capable(bus, IOMMU_CAP_INTR_REMAP) && !safe_irq_domains)) {
>>> pr_warn("%s: No interrupt remapping support. Use the module param \"allow_unsafe_interrupts\" to enable VFIO IOMMU support on this platform\n",
>>> __func__);
>>> ret = -EPERM;
>>
>
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH v9 0/7] KVM PCIe/MSI passthrough on ARM/ARM64: kernel part 3/3: vfio changes Eric Auger <eric.auger@linaro.org> - 2016-05-04 14:00 +0200
[PATCH v9 5/7] vfio/type1: also check IRQ remapping capability at msi domain Eric Auger <eric.auger@linaro.org> - 2016-05-04 14:00 +0200
Re: [PATCH v9 5/7] vfio/type1: also check IRQ remapping capability at msi domain "Chalamarla, Tirumalesh" <Tirumalesh.Chalamarla@caviumnetworks.com> - 2016-05-05 21:30 +0200
Re: [PATCH v9 5/7] vfio/type1: also check IRQ remapping capability at msi domain Eric Auger <eric.auger@linaro.org> - 2016-05-09 10:10 +0200
Re: [PATCH v9 5/7] vfio/type1: also check IRQ remapping capability at msi domain Alex Williamson <alex.williamson@redhat.com> - 2016-05-10 00:50 +0200
Re: [PATCH v9 5/7] vfio/type1: also check IRQ remapping capability at msi domain Eric Auger <eric.auger@linaro.org> - 2016-05-10 18:20 +0200
Re: [PATCH v9 5/7] vfio/type1: also check IRQ remapping capability at msi domain Robin Murphy <robin.murphy@arm.com> - 2016-05-10 19:30 +0200
Re: [PATCH v9 5/7] vfio/type1: also check IRQ remapping capability at msi domain Eric Auger <eric.auger@linaro.org> - 2016-05-11 10:50 +0200
Re: [PATCH v9 5/7] vfio/type1: also check IRQ remapping capability at msi domain Robin Murphy <robin.murphy@arm.com> - 2016-05-11 11:40 +0200
Re: [PATCH v9 5/7] vfio/type1: also check IRQ remapping capability at msi domain Eric Auger <eric.auger@linaro.org> - 2016-05-11 11:50 +0200
Re: [PATCH v9 5/7] vfio/type1: also check IRQ remapping capability at msi domain Robin Murphy <robin.murphy@arm.com> - 2016-05-11 15:50 +0200
Re: [PATCH v9 5/7] vfio/type1: also check IRQ remapping capability at msi domain Eric Auger <eric.auger@linaro.org> - 2016-05-11 16:40 +0200
[PATCH v9 6/7] iommu/arm-smmu: do not advertise IOMMU_CAP_INTR_REMAP Eric Auger <eric.auger@linaro.org> - 2016-05-04 14:00 +0200
[PATCH v9 1/7] vfio: introduce a vfio_dma type field Eric Auger <eric.auger@linaro.org> - 2016-05-04 14:00 +0200
[PATCH v9 7/7] vfio/type1: return MSI geometry through VFIO_IOMMU_GET_INFO capability chains Eric Auger <eric.auger@linaro.org> - 2016-05-04 14:00 +0200
Re: [PATCH v9 7/7] vfio/type1: return MSI geometry through VFIO_IOMMU_GET_INFO capability chains Eric Auger <eric.auger@linaro.org> - 2016-05-04 14:10 +0200
Re: [PATCH v9 7/7] vfio/type1: return MSI geometry through VFIO_IOMMU_GET_INFO capability chains Alex Williamson <alex.williamson@redhat.com> - 2016-05-10 01:10 +0200
Re: [PATCH v9 7/7] vfio/type1: return MSI geometry through VFIO_IOMMU_GET_INFO capability chains Eric Auger <eric.auger@linaro.org> - 2016-05-10 19:00 +0200
Re: [PATCH v9 7/7] vfio/type1: return MSI geometry through VFIO_IOMMU_GET_INFO capability chains Alex Williamson <alex.williamson@redhat.com> - 2016-05-10 01:00 +0200
Re: [PATCH v9 7/7] vfio/type1: return MSI geometry through VFIO_IOMMU_GET_INFO capability chains Eric Auger <eric.auger@linaro.org> - 2016-05-10 18:40 +0200
[PATCH v9 2/7] vfio/type1: vfio_find_dma accepting a type argument Eric Auger <eric.auger@linaro.org> - 2016-05-04 14:00 +0200
Re: [PATCH v9 2/7] vfio/type1: vfio_find_dma accepting a type argument Alex Williamson <alex.williamson@redhat.com> - 2016-05-10 00:50 +0200
Re: [PATCH v9 2/7] vfio/type1: vfio_find_dma accepting a type argument Eric Auger <eric.auger@linaro.org> - 2016-05-10 17:00 +0200
[PATCH v9 4/7] vfio: allow reserved msi iova registration Eric Auger <eric.auger@linaro.org> - 2016-05-04 14:00 +0200
Re: [PATCH v9 4/7] vfio: allow reserved msi iova registration "Chalamarla, Tirumalesh" <Tirumalesh.Chalamarla@caviumnetworks.com> - 2016-05-05 21:40 +0200
Re: [PATCH v9 4/7] vfio: allow reserved msi iova registration Eric Auger <eric.auger@linaro.org> - 2016-05-09 10:00 +0200
Re: [PATCH v9 4/7] vfio: allow reserved msi iova registration Alex Williamson <alex.williamson@redhat.com> - 2016-05-10 17:40 +0200
Re: [PATCH v9 4/7] vfio: allow reserved msi iova registration Eric Auger <eric.auger@linaro.org> - 2016-05-10 17:40 +0200
[PATCH v9 3/7] vfio/type1: bypass unmap/unpin and replay for VFIO_IOVA_RESERVED slots Eric Auger <eric.auger@linaro.org> - 2016-05-04 14:00 +0200
Re: [PATCH v9 3/7] vfio/type1: bypass unmap/unpin and replay for VFIO_IOVA_RESERVED slots Alex Williamson <alex.williamson@redhat.com> - 2016-05-10 00:50 +0200
Re: [PATCH v9 3/7] vfio/type1: bypass unmap/unpin and replay for VFIO_IOVA_RESERVED slots Eric Auger <eric.auger@linaro.org> - 2016-05-11 15:00 +0200
csiph-web