From: Michael S. Tsirkin on
On Tue, Jun 01, 2010 at 11:10:45AM +0300, Avi Kivity wrote:
> On 05/31/2010 08:10 PM, Michael S. Tsirkin wrote:
>> On Mon, May 31, 2010 at 02:50:29PM +0300, Avi Kivity wrote:
>>
>>> On 05/30/2010 05:53 PM, Michael S. Tsirkin wrote:
>>>
>>>> So what I suggested is failing any kind of access until iommu
>>>> is assigned.
>>>>
>>>>
>>> So, the kernel driver must be aware of the iommu. In which case it may
>>> as well program it.
>>>
>> It's a kernel driver anyway. Point is that
>> the *device* driver is better off not programming iommu,
>> this way we do not need to reprogram it for each device.
>>
>
> The device driver is in userspace.

I mean the kernel driver that grants userspace the access.

> It can't program the iommu.
> What
> the patch proposes is that userspace tells vfio about the needed
> mappings, and vfio programs the iommu.

There seems to be some misunderstanding. The userspace interface
proposed forces a separate domain per device and forces userspace to
repeat iommu programming for each device. We are better off sharing a
domain between devices and programming the iommu once.

The natural way to do this is to have an iommu driver for programming
iommu.

This likely means we will have to pass the domain to 'vfio' or uio or
whatever the driver that gives userspace the access to device is called,
but this is only for security, there's no need to support programming
iommu there.

And using this design means the uio framework changes
required would be minor, so we won't have to duplicate code.

> --
> error compiling committee.c: too many arguments to function
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Avi Kivity on
On 06/01/2010 12:55 PM, Michael S. Tsirkin wrote:
>
>> It can't program the iommu.
>> What
>> the patch proposes is that userspace tells vfio about the needed
>> mappings, and vfio programs the iommu.
>>
> There seems to be some misunderstanding. The userspace interface
> proposed forces a separate domain per device and forces userspace to
> repeat iommu programming for each device. We are better off sharing a
> domain between devices and programming the iommu once.
>

iommufd = open(/dev/iommu);
ioctl(iommufd, IOMMUFD_ASSIGN_RANGE, ...)
ioctl(vfiofd, VFIO_SET_IOMMU, iommufd)

?

If so, I agree.

> The natural way to do this is to have an iommu driver for programming
> iommu.
>
> This likely means we will have to pass the domain to 'vfio' or uio or
> whatever the driver that gives userspace the access to device is called,
> but this is only for security, there's no need to support programming
> iommu there.
>
> And using this design means the uio framework changes
> required would be minor, so we won't have to duplicate code.
>

Since vfio would be the only driver, there would be no duplication. But
a separate object for the iommu mapping is a good thing. Perhaps we can
even share it with vhost (without actually using the mmu, since vhost is
software only).

--
error compiling committee.c: too many arguments to function

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Michael S. Tsirkin on
On Tue, Jun 01, 2010 at 01:28:48PM +0300, Avi Kivity wrote:
> On 06/01/2010 12:55 PM, Michael S. Tsirkin wrote:
>>
>>> It can't program the iommu.
>>> What
>>> the patch proposes is that userspace tells vfio about the needed
>>> mappings, and vfio programs the iommu.
>>>
>> There seems to be some misunderstanding. The userspace interface
>> proposed forces a separate domain per device and forces userspace to
>> repeat iommu programming for each device. We are better off sharing a
>> domain between devices and programming the iommu once.
>>
>
> iommufd = open(/dev/iommu);
> ioctl(iommufd, IOMMUFD_ASSIGN_RANGE, ...)
> ioctl(vfiofd, VFIO_SET_IOMMU, iommufd)
>
> ?

Yes.

> If so, I agree.

Good.

>> The natural way to do this is to have an iommu driver for programming
>> iommu.
>>
>> This likely means we will have to pass the domain to 'vfio' or uio or
>> whatever the driver that gives userspace the access to device is called,
>> but this is only for security, there's no need to support programming
>> iommu there.
>>
>> And using this design means the uio framework changes
>> required would be minor, so we won't have to duplicate code.
>>
>
> Since vfio would be the only driver, there would be no duplication. But
> a separate object for the iommu mapping is a good thing. Perhaps we can
> even share it with vhost (without actually using the mmu, since vhost is
> software only).

Main difference is that vhost works fine with unlocked
memory, paging it in on demand. iommu needs to unmap
memory when it is swapped out or relocated.

> --
> error compiling committee.c: too many arguments to function
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Avi Kivity on
On 06/01/2010 01:46 PM, Michael S. Tsirkin wrote:
>
>> Since vfio would be the only driver, there would be no duplication. But
>> a separate object for the iommu mapping is a good thing. Perhaps we can
>> even share it with vhost (without actually using the mmu, since vhost is
>> software only).
>>
> Main difference is that vhost works fine with unlocked
> memory, paging it in on demand. iommu needs to unmap
> memory when it is swapped out or relocated.
>
>

So you'd just take the memory map and not pin anything. This way you
can reuse the memory map.

But no, it doesn't handle the dirty bitmap, so no go.

--
error compiling committee.c: too many arguments to function

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Tom Lyon on
On Monday 31 May 2010 10:17:35 am Alan Cox wrote:
>
> Does look like it needs a locking audit, some memory and error checks
> reviewing and some further review of the ioctl security and
> overflows/trusted values.
Yes. Thanks for the detailed look.
>
> Rather a nice way of attacking the user space PCI problem.
And thanks for that!

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/