On 01/12/17 10:09, Alex Williamson wrote:
> On Fri, 1 Dec 2017 08:56:42 +1100
> Alexey Kardashevskiy <a...@ozlabs.ru> wrote:
>
>> On 05/10/17 16:50, Alexey Kardashevskiy wrote:
>>> The new callback will be called when a new VFIO IOMMU group is added.
>>>
>>> This should cause no behavioral change, the next patch will.
>>>
>>> Signed-off-by: Alexey Kardashevskiy <a...@ozlabs.ru>
>>
>>
>> What about this one, any conclusion yet?
>
> Patchew reported that if failed to build and you never replied with an
> explanation, so I haven't looked at it.
Well, this ping was more for David as he is supposedly rethingking it...
>
>>> ---
>>>
>>> This could be at the higher level - in MemoryRegionOps, makes sense?
>>>
>>> ---
>>> include/exec/memory.h | 4 ++++
>>> hw/vfio/common.c | 15 +++++++++++++++
>>> 2 files changed, 19 insertions(+)
>>>
>>> diff --git a/include/exec/memory.h b/include/exec/memory.h
>>> index 5ed4042f87..64e0b4fc96 100644
>>> --- a/include/exec/memory.h
>>> +++ b/include/exec/memory.h
>>> @@ -210,6 +210,10 @@ typedef struct IOMMUMemoryRegionClass {
>>> IOMMUNotifierFlag new_flags);
>>> /* Set this up to provide customized IOMMU replay function */
>>> void (*replay)(IOMMUMemoryRegion *iommu, IOMMUNotifier *notifier);
>>> +
>>> + /* Notifies IOMMUMR about attached VFIO IOMMU group */
>>> + void (*add_vfio_group)(IOMMUMemoryRegion *iommu_mr, int vfio_kvm_fd,
>>> + int groupfd);
>
> If there's an "add" why is there no remove?
Because I do not have use for it now and I doubt there will be any ever.
"add" attaches an IOMMU group to a DMA window in KVM, and I cannot imagine
if we ever want to detach a group _and_ keep using it.
> Clearly a vfio specific callback is pretty much a failure as far as
> abstraction and layering is concerned.
This was one of the things I wanted David to discuss with you as he clearly
did not like another abstraction violation:
https://patchwork.kernel.org/patch/9853981/
>
>>> } IOMMUMemoryRegionClass;
>>>
>>> typedef struct CoalescedMemoryRange CoalescedMemoryRange;
>>> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
>>> index 7b2924c0ef..9e861e0393 100644
>>> --- a/hw/vfio/common.c
>>> +++ b/hw/vfio/common.c
>>> @@ -481,6 +481,21 @@ static void vfio_listener_region_add(MemoryListener
>>> *listener,
>>> VFIOGuestIOMMU *giommu;
>>> IOMMUMemoryRegion *iommu_mr = IOMMU_MEMORY_REGION(section->mr);
>>>
>>> +#ifdef CONFIG_KVM
>>> + if (kvm_enabled()) {
>>> + VFIOGroup *group;
>>> + IOMMUMemoryRegionClass *imrc =
>>> + IOMMU_MEMORY_REGION_GET_CLASS(iommu_mr);
>>> +
>>> + QLIST_FOREACH(group, &container->group_list, container_next) {
>>> + if (imrc->add_vfio_group) {
>>> + imrc->add_vfio_group(iommu_mr, vfio_kvm_device_fd,
>>> + group->fd);
>>> + }
>>> + }
>>> + }
>>> +#endif
>>> +
>>> trace_vfio_listener_region_add_iommu(iova, end);
>>> /*
>>> * FIXME: For VFIO iommu types which have KVM acceleration to
>>>
>>
>>
>
--
Alexey
