Re: [Qemu-devel] [PATCH] vfio: add check for memory region overflow condition

[Bandan Das <bsd@redhat.com>]

Alex Williamson <alex.williamson@redhat.com> writes:
...
>> 
>> And it does. If we fix this assert, then vfio_dma_map() attempts mapping
>> this direct mapped address range starting from 0 and prints a 
>> warning message; happens for the whole range and goes on for ever.
>> The overflow check seemed to me like something we should fix, but now
>> I am more confused then ever!
>
> Is the MemoryRegion memory_region_is_iommu() such that you're calling
> vfio_dma_map() from vfio_iommu_map_notify()?  If so then we should

Yes, that is correct. This all started after we added the iommu mapping
replay changes but I was wrong about the vfio_dma_map part. Please see
below.

> probably be using 128bit helpers for doing sanity checking and go ahead
> and let something assert if we get to the vfio_dma_map() in
> vfio_listener_region_add() with a 2^64 size.  Then if you're taking the
> memory_region_is_iommu() path, vfio_dma_map() is going to be called
> with translations within that 2^64 bit address space, not mapping the
> entire space, right?  Thanks,

The 128 bit operations make sense...

The error message comes from:
if (!memory_region_is_ram(mr)) {
        error_report("iommu map to non memory area %"HWADDR_PRIx"",
                     xlat);
        goto out;
    }
in vfio_iommu_map_notify() before we even get to vfio_dma_map().

This gets attempted for the entire range because dmar isn't enabled yet and
vtd_iommu_translate() does this direct mapping in 4k increments in the translate
path :
...

    if (!s->dmar_enabled) {
        /* DMAR disabled, passthrough, use 4k-page*/
        ret.iova = addr & VTD_PAGE_MASK_4K;
        ret.translated_addr = addr & VTD_PAGE_MASK_4K;
        ret.addr_mask = ~VTD_PAGE_MASK_4K;
        ret.perm = IOMMU_RW;
        return ret;
    }

I am not sure yet who actually uses it though.
memory_region_iommu_replay() does the whole iteration
if perm != IOMMU_NONE:

void memory_region_iommu_replay(MemoryRegion *mr, Notifier *n,
                                hwaddr granularity, bool is_write)
{
    hwaddr addr;
    IOMMUTLBEntry iotlb;

    for (addr = 0; addr < memory_region_size(mr); addr += granularity) {
        iotlb = mr->iommu_ops->translate(mr, addr, is_write);
        if (iotlb.perm != IOMMU_NONE) {
            n->notify(n, &iotlb);
        }
...

> Alex