From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: Re: [PATCH 2/2] AMD IOMMU: use ioremap() Date: Thu, 11 Jul 2013 09:14:20 +0100 Message-ID: <51DE857C02000078000E408F@nat28.tlf.novell.com> References: <51DD555102000078000E3D11@nat28.tlf.novell.com> <51DD585702000078000E3D40@nat28.tlf.novell.com> <51DD95FA.9060106@amd.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <51DD95FA.9060106@amd.com> Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Suravee Suthikulanit Cc: Jacob Shin , xiantao.zhang@intel.com, xen-devel List-Id: xen-devel@lists.xenproject.org >>> On 10.07.13 at 19:12, Suravee Suthikulanit 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 >> >> --- 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