Re: [Xen-devel] [PATCH 2/2] AMD IOMMU: use ioremap()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 10.07.13 at 19:12, Suravee Suthikulanit <suravee.suthikulpanit@xxxxxxx>
wrote:
> On 7/10/2013 5:49 AM, Jan Beulich wrote:
>> There's no point in using the fixmap here, and it gets
>> map_iommu_mmio_region() in line with unmap_iommu_mmio_region(), which
>> was already using iounmap() (thus crashing if actually used).
>>
>> Signed-off-by: Jan Beulich<jbeulich@xxxxxxxx>
>>
>> --- a/xen/drivers/passthrough/amd/iommu_init.c
>> +++ b/xen/drivers/passthrough/amd/iommu_init.c
>> @@ -48,19 +48,10 @@ static int iommu_has_ht_flag(struct amd_
>>   
>>   static int __init map_iommu_mmio_region(struct amd_iommu *iommu)
>>   {
>> -    unsigned long mfn;
>> -
>> -    if ( nr_amd_iommus > MAX_AMD_IOMMUS )
>> -    {
>> -        AMD_IOMMU_DEBUG("nr_amd_iommus %d > MAX_IOMMUS\n", nr_amd_iommus);
>> +    iommu->mmio_base = ioremap(iommu->mmio_base_phys,
>> +                               IOMMU_MMIO_REGION_LENGTH);
>> +    if ( iommu->mmio_base )
> This should have been "if ( !iommu->mmio_base )".

Oops, of course.

While changing this I also noticed that the corresponding VT-d code
had no error checking at all, so I'll send a v2 for both.

>>           return -ENOMEM;
>> -    }
>> -
>> -    iommu->mmio_base = (void *)fix_to_virt(
>> -        FIX_IOMMU_MMIO_BASE_0 + nr_amd_iommus * MMIO_PAGES_PER_IOMMU);
>> -    mfn = (unsigned long)(iommu->mmio_base_phys >> PAGE_SHIFT);
>> -    map_pages_to_xen((unsigned long)iommu->mmio_base, mfn,
>> -                     MMIO_PAGES_PER_IOMMU, PAGE_HYPERVISOR_NOCACHE);
>>   
>>       memset(iommu->mmio_base, 0, IOMMU_MMIO_REGION_LENGTH);
> Once I changed the above logic, IOMMU is initialized correctly. I have 
> tested pass-though a NIC and thing looks fine.

Please explicitly send an ack (or which other tags to apply) for the
v2 that I'll send out in a minute.

Thanks, Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel