[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-arm] [PATCH 2/2] hw/arm/smmuv3: Implement dummy replay
From: |
Auger Eric |
Subject: |
Re: [Qemu-arm] [PATCH 2/2] hw/arm/smmuv3: Implement dummy replay |
Date: |
Fri, 14 Jun 2019 15:40:15 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 |
Hi Peter,
On 6/14/19 3:26 PM, Peter Maydell wrote:
> On Tue, 11 Jun 2019 at 15:29, Eric Auger <address@hidden> wrote:
>>
>> On ARM we currently do not support VFIO-PCI devices protected
>> by the IOMMU. Any attempt to run such use case results in this
>> kind of warning:
>>
>> "-device vfio-pci,host=0004:01:00.0,id=hostdev0,bus=pci.1,addr=0x0:
>> warning: SMMUv3 does not support notification on MAP: device vfio-pci
>> will not function properly".
>>
>> However this is just a warning and this should not prevent the
>> guest from booting in a reasonable amount of time. This does not
>> happen currently.
>>
>> This is due to the fact the VFIO vfio_listener_region_add() calls
>> memory_region_iommu_replay(). As the SMMUv3 IOMMUMemoryRegionClass
>> currently does not implement the replay() callback, the default
>> memory_region_iommu_replay() implementation is used. This latter
>> loops on the whole notifier's range (48b address space), translates
>> each page and call the notifier on the resulting entry. This totally
>> freezes the guest.
>>
>> The Intel IOMMU implements the replay() function which only
>> notifies valid page table entries.
>>
>> In the looming SMMUv3 nested stage VFIO integration, there will be
>> no need to replay() anything as there will not be any shadow page
>> tables: the stage 1 page tables are owned by the guest.
>>
>> So let's implement a void replay() which satisfies both cases.
>>
>> Signed-off-by: Eric Auger <address@hidden>
>> ---
>> hw/arm/smmuv3.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
>> index e2f07d2864..1f578365ef 100644
>> --- a/hw/arm/smmuv3.c
>> +++ b/hw/arm/smmuv3.c
>> @@ -1489,6 +1489,11 @@ static void
>> smmuv3_notify_flag_changed(IOMMUMemoryRegion *iommu,
>> }
>> }
>>
>> +static inline void
>> +smmuv3_replay(IOMMUMemoryRegion *iommu_mr, IOMMUNotifier *n)
>> +{
>> +}
>
> This doesn't seem like a valid implementation of the replay
> method to me. The API doc comment says
> * The default implementation of memory_region_iommu_replay() is to
> * call the IOMMU translate method for every page in the address space
> * with flag == IOMMU_NONE and then call the notifier if translate
> * returns a valid mapping. If this method is implemented then it
> * overrides the default behaviour, and must provide the full semantics
> * of memory_region_iommu_replay(), by calling @notifier for every
> * translation present in the IOMMU.
>
> This empty function is definitely not going to call the notifier
> for every IOMMU translation...
The situation is a bit odd. SMMUv3 is not integrated with VFIO so VFIO
devices will not work anyway (we are not able to notify on MAP). There
is a warning already reporting the issue. However the default
implementation of memory_region_iommu_replay() prevents the guest from
booting. So what would you advise?
Thanks
Eric
>
> thanks
> -- PMM
>
[Qemu-arm] [PATCH 1/2] hw/arm/smmuv3: Remove spurious error messages on IOVA invalidations, Eric Auger, 2019/06/11