On 15/09/2017 09:30, Auger Eric wrote:
> Hi Tomasz,
>
> On 14/09/2017 16:43, Tomasz Nowicki wrote:
>> On 14.09.2017 16:31, Tomasz Nowicki wrote:
>>> Hi Eric,
>>>
>>> On 14.09.2017 11:27, Linu Cherian wrote:
>>>> Hi Eric,
>>>>
>>>> On Fri Sep 01, 2017 at 07:21:16PM +0200, Eric Auger wrote:
>>>>> memory_region_iommu_replay() is used for VFIO integration.
>>>>>
>>>>> However its default implementation is not adapted to SMMUv3
>>>>> IOMMU memory region. Indeed the input address range is too
>>>>> huge and its execution is too slow as it calls the translate()
>>>>> callback on each granule.
>>>>>
>>>>> Let's implement the replay callback which hierarchically walk
>>>>> over the page table structure and notify only the segments
>>>>> that are populated with valid entries.
>>>>>
>>>>> Signed-off-by: Eric Auger <eric.au...@redhat.com>
>>>>> ---
>>>>> hw/arm/smmuv3.c | 36 ++++++++++++++++++++++++++++++++++++
>>>>> hw/arm/trace-events | 1 +
>>>>> 2 files changed, 37 insertions(+)
>>>>>
>>>>> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
>>>>> index 8e7d10d..c43bd93 100644
>>>>> --- a/hw/arm/smmuv3.c
>>>>> +++ b/hw/arm/smmuv3.c
>>>>> @@ -657,6 +657,41 @@ static int smmuv3_notify_entry(IOMMUTLBEntry
>>>>> *entry, void *private)
>>>>> return 0;
>>>>> }
>>>>> +/* Unmap the whole notifier's range */
>>>>> +static void smmuv3_unmap_notifier_range(IOMMUNotifier *n)
>>>>> +{
>>>>> + IOMMUTLBEntry entry;
>>>>> + hwaddr size = n->end - n->start + 1;
>>>>> +
>>>>> + entry.target_as = &address_space_memory;
>>>>> + entry.iova = n->start & ~(size - 1);
>>>>> + entry.perm = IOMMU_NONE;
>>>>> + entry.addr_mask = size - 1;
>>>>> +
>>>>> + memory_region_notify_one(n, &entry);
>>>>> +}
>>>>> +
>>>>> +static void smmuv3_replay(IOMMUMemoryRegion *mr, IOMMUNotifier *n)
>>>>> +{
>>>>> + SMMUTransCfg cfg = {};
>>>>> + int ret;
>>>>> +
>>>>> + trace_smmuv3_replay(mr->parent_obj.name, n, n->start, n->end);
>>>>> + smmuv3_unmap_notifier_range(n);
>>>>> +
>>>>> + ret = smmuv3_decode_config(mr, &cfg);
>>>>> + if (ret) {
>>>>> + error_report("%s error decoding the configuration for iommu
>>>>> mr=%s",
>>>>> + __func__, mr->parent_obj.name);
>>>>> + }
>>>>>
>>>>
>>>> On an invalid config being found, shouldnt we return rather than
>>>> proceeding with
>>>> page table walk. For example on an invalid Stream table entry.
>>>
>>> Indeed, without return here vhost case is not working for me.
>>
>> I was just lucky one time. return here has no influence. Vhost still not
>> working. Sorry for noise.
>
> As far as I understand the replay() callback only is called in VFIO use
> case. So this shouldn't impact vhost.
Forget that, that's potentially called from some invalidation commands
also in vhost case.
Thanks
Eric
>
> I can't reproduce your vhost issue on my side. I will review the
> invalidate code again and compare against the last version.
>
> What is the page size used by your guest?
>
> Thanks
>
> Eric
>>
>> Tomasz
>>
>>>
>>> Thanks,
>>> Tomasz
>>>
>>>>
>>>> +
>>>>> + if (cfg.disabled || cfg.bypassed) {
>>>>> + return;
>>>>> + }
>>>>> + /* walk the page tables and replay valid entries */
>>>>> + smmu_page_walk(&cfg, 0, (1ULL << (64 - cfg.tsz)) - 1, false,
>>>>> + smmuv3_notify_entry, n);
>>>>> +}
>>>>> static void smmuv3_notify_iova_range(IOMMUMemoryRegion *mr,
>>>>> IOMMUNotifier *n,
>>>>> uint64_t iova, size_t size)
>>>>> {
>>>>> @@ -1095,6 +1130,7 @@ static void
>>>>> smmuv3_iommu_memory_region_class_init(ObjectClass *klass,
>>>>> imrc->translate = smmuv3_translate;
>>>>> imrc->notify_flag_changed = smmuv3_notify_flag_changed;
>>>>> + imrc->replay = smmuv3_replay;
>>>>> }
>>>>> static const TypeInfo smmuv3_type_info = {
>>>>> diff --git a/hw/arm/trace-events b/hw/arm/trace-events
>>>>> index 4ac264d..15f84d6 100644
>>>>> --- a/hw/arm/trace-events
>>>>> +++ b/hw/arm/trace-events
>>>>> @@ -46,5 +46,6 @@ smmuv3_cfg_stage(int s, uint32_t oas, uint32_t
>>>>> tsz, uint64_t ttbr, bool aa64, ui
>>>>> smmuv3_notify_flag_add(const char *iommu) "ADD SMMUNotifier node
>>>>> for iommu mr=%s"
>>>>> smmuv3_notify_flag_del(const char *iommu) "DEL SMMUNotifier node
>>>>> for iommu mr=%s"
>>>>> smmuv3_replay_mr(const char *name) "iommu mr=%s"
>>>>> +smmuv3_replay(const char *name, void *n, hwaddr start, hwaddr end)
>>>>> "iommu mr=%s notifier=%p [0x%"PRIx64",0x%"PRIx64"]"
>>>>> smmuv3_notify_entry(hwaddr iova, hwaddr pa, hwaddr mask, int perm)
>>>>> "iova=0x%"PRIx64" pa=0x%" PRIx64" mask=0x%"PRIx64" perm=%d"
>>>>> smmuv3_notify_iova_range(const char *name, uint64_t iova, size_t
>>>>> size, void *n) "iommu mr=%s iova=0x%"PRIx64" size=0x%lx n=%p"
>>>>> --
>>>>> 2.5.5
>>>>>
>>>>>
>>>>
