Re: [PATCH v5 0/4] vfio: type1: support for ARM SMMUS with VFIO_IOMMU_TYPE1

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Good news Eric !

Here are more information about the NOEXEC flag. This flag is used to
set the XN flag on the PTE, which make the page non-executable (XN is
set if NOEXEC is set).

As it used to be an EXEC flag (XN was set by default, unset if EXEC
was set), this patch serie was mandatory for PL330 test, since it
executes some code from memory. Now that it has been reversed to
NOEXEC (XN is not set by default), every page is executable, so it
works but there may be a security issue.

I think it is better to have it if it is working, but for
simplification we can add it later (as the requirement dropped when
the flag has been reversed).

Best regards,
Baptiste


On Fri, Mar 6, 2015 at 2:17 PM, Eric Auger <eric.auger@xxxxxxxxxx> wrote:
> On 03/06/2015 12:27 PM, Baptiste Reynal wrote:
>> Hi everyone,
>>
>> Indeed, the NOEXEC flag is needed for our tests and VFIO should work
>> without it for some other devices (including xgmac). I think it is
>> reasonable to drop this patch serie for now.
>>
>> Still we have the IOMMU_CACHE issue. To answer Will, the SMMU page
>> tables are installed at stage-2. Currently, the dma-coherent flag is
>> not preserved to the guest, that may be the issue (As we don't have it
>> in the host in our case). "[RFC PATCH v3 0/3] vfio: platform: return
>> device properties for a platform device"
>> (http://lists.celinuxforum.org/pipermail/iommu/2014-December/011425.html)
>> may be a solution to figure out if the device is coherent or not.
>
> I confirm the problem is fixed with proper guest device-tree. Sorry for
> the noise and thanks for your support. indeed in the future the above
> series will help in getting the info dynamically.
>
> Best Regards
>
> Eric
>>
>>
>> On Fri, Mar 6, 2015 at 11:41 AM, Will Deacon <will.deacon@xxxxxxx> wrote:
>>> Hi Eric,
>>>
>>> On Fri, Mar 06, 2015 at 09:04:22AM +0000, Eric Auger wrote:
>>>> Yes to me "vfio: type1: support for ARM SMMUS with VFIO_IOMMU_TYPE1" is
>>>> not a direct dependency of vfio-platform driver although I understand
>>>> Baptiste needs it to test with PL330 device. On my side I can test
>>>> vfio-platform driver without it, assigning the Midway Calxeda xgmac.
>>>
>>> Realistically, I don't expect anybody using device passthrough for
>>> practical applications to be using a PL330. Your xgmac example is far
>>> more compelling.
>>>
>>>> Also if I am not wrong the title of the series now does not really
>>>> reflect anymore what the series aims at. since "[PATCH v3 2/6] vfio:
>>>> type1: support for ARM SMMUs"  withdrawal, the series now "only" aims at
>>>> supporting the VFIO_DMA_MAP_FLAG_NOEXEC flag.
>>>>
>>>> However with this issue of IOMMU_CACHE always set along with arm-smmu,
>>>> there is a need for an adaptation of either vfio_iommu_type1 or arm-smmu
>>>> since the integrated pieces are not functional.
>>>
>>> Well, I'm still trying to understand exactly what is happening in your case:
>>>
>>>   - Is the xgmac coherent or not? Does it have a "dma-coherent" property?
>>>   - Are you installing the SMMU page tables at stage-1 or stage-2?
>>>   - If it *is* coherent, then we should use IOMMU_CACHE mappings for the
>>>     DMA buffers and ensure that the guest knows it is coherent (by
>>>     preserving the "dma-coherent" flag).
>>>   - If it is *not* coherent, then the behaviour of IOMMU_CACHE depends
>>>     on the stage of translation:
>>>
>>>     * Stage-1: we will make the transactions cacheable, and you'll need
>>>       to tell the guest that the device is actually cache coherent
>>>     * Stage-2: IOMMU_CACHE won't actually have any effect, so everything
>>>       should work as non-coherent.
>>>
>>> In other words, I think you're probably just telling the guest the wrong
>>> thing.
>>>
>>>> On top of the dma-coherent property of the *master*, should not we also
>>>> query the cache-coherent property of the interconnect downstream to the
>>>> smmu?
>>>
>>> I don't think so... "dma-coherent" should only be set on a master if the
>>> interconnect is properly configured. It's supposed to be a "If you DMA now,
>>> it will snoop the CPU caches" flag as opposed to "If you write a random
>>> selection of MMIO registers in the SoC, then this device will be coherent".
>>>
>>>> How can we progress quickly on this topic? is it acceptable to return
>>>> false on arm_smmu_capable(IOMMU_CAP_CACHE_COHERENCY) as a quick hack?
>>>
>>> No; pretending that a device is not coherent when it actually is can lead
>>> to corruption of DMA buffers due to unnecessary cache invalidation.
>>>
>>>> As a longer term solution, would it make sense to add a user flag at VFIO
>>>> user API level to turn the IOMMU_CACHE on?
>>>
>>> I think userspace certainly needs a way to figure out if a device is
>>> coherent or not, otherwise it can't generate the correct device-tree
>>> properties for something like a KVM guest but the IOMMU_* setting should
>>> remain in the kernel IMO. Similarly for things like MSI pages, which would
>>> need to be mapped as device memory on ARM -- that should be exposed as a
>>> higher level "please map my MSI page here" ioctl as opposed to requiring
>>> userspace to supply the correct memory attributes.
>>>
>>> Will
>
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm




[Index of Archives]     [Linux KVM]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux