Re: [RFC PATCH v2 2/5] iommu/virtio-iommu: Add probe request

From: Auger Eric <eric.auger@redhat.com>
To: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>,
	"iommu@lists.linux-foundation.org"
	<iommu@lists.linux-foundation.org>,
	"devel@acpica.org" <devel@acpica.org>,
	"linux-acpi@vger.kernel.org" <linux-acpi@vger.kernel.org>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"kvmarm@lists.cs.columbia.edu" <kvmarm@lists.cs.columbia.edu>,
	"virtualization@lists.linux-foundation.org"
	<virtualization@lists.linux-foundation.org>,
	"virtio-dev@lists.oasis-open.org"
	<virtio-dev@lists.oasis-open.org>
Cc: "Jayachandran.Nair@cavium.com" <Jayachandran.Nair@cavium.com>,
	Lorenzo Pieralisi <Lorenzo.Pieralisi@arm.com>,
	"ashok.raj@intel.com" <ashok.raj@intel.com>,
	"mst@redhat.com" <mst@redhat.com>,
	Marc Zyngier <Marc.Zyngier@arm.com>,
	Will Deacon <Will.Deacon@arm.com>,
	"rjw@rjwysocki.net" <rjw@rjwysocki.net>,
	"robert.moore@intel.com" <robert.moore@intel.com>,
	"lv.zheng@intel.com" <lv.zheng@intel.com>,
	Sudeep Holla <Sudeep.Holla@arm.com>,
	"lenb@kernel.org" <lenb@kernel.org>,
	Robin Murphy <Robin.Murphy@arm.com>,
	"joro@8bytes.org" <joro@8bytes.org>,
	"hanjun.guo@linaro.org" <hanjun.guo@linaro.org>
Subject: Re: [RFC PATCH v2 2/5] iommu/virtio-iommu: Add probe request
Date: Fri, 19 Jan 2018 18:22:30 +0100	[thread overview]
Message-ID: <bfefc181-4c25-8798-b304-dce2c68b1ba8__6325.59163864261$1516382493$gmane$org@redhat.com> (raw)
In-Reply-To: <b9ede9e9-25a6-2848-e152-ffc61d560be8@arm.com>

Hi Jean-Philippe,

On 19/01/18 17:21, Jean-Philippe Brucker wrote:
> On 16/01/18 23:26, Auger Eric wrote:
> [...]
>>> +	switch (mem->subtype) {
>>> +	case VIRTIO_IOMMU_RESV_MEM_T_MSI:
>>> +		region = iommu_alloc_resv_region(addr, size, prot,
>>> +						 IOMMU_RESV_MSI);
>> if (!region)
>> 	return -ENOMEM;
>>> +		break;
>>> +	case VIRTIO_IOMMU_RESV_MEM_T_RESERVED:
>>> +	default:
>>> +		region = iommu_alloc_resv_region(addr, size, 0,
>>> +						 IOMMU_RESV_RESERVED);
>> same.
> 
> I'll add them, thanks
> 
>> There is another issue related to the exclusion of iovas belonging to
>> reserved regions. Typically on x86, when attempting to run
>> virtio-blk-pci with iommu I eventually saw the driver using iova
>> belonging to the IOAPIC regions to map phys addr and this stalled qemu
>> with a drown trace:
>>
>> "virtio: bogus descriptor or out of resources"
>>
>> those regions need to be excluded from the iova allocator. This was
>> resolved by adding
>> if (iommu_dma_init_domain(domain,
>> 			  vdev->viommu->geometry.aperture_start,
>> 			  vdev->viommu->geometry.aperture_end,
>> 			  dev))
>> in viommu_attach_dev()
> 
> The most recent hack for x86 [1] does call iommu_dma_init_domain() in
> attach_dev(). Is it buggy?

Oh OK. Actually I have never used that branch and in my version the last
arg of iommu_dma_init_domain was NULL hence the issue. But I was
thinking more generally that care must be taken to exclude all those
regions.

Thanks

Eric
> 
> We probably shouldn't call iommu_dma_init_domain() unconditionally
> (outside of CONFIG_X86 that is), since it's normally done by the arch
> (arch/arm64/mm/dma-mapping.c)
> 
> Thanks,
> Jean
> 
> [1]
> http://www.linux-arm.org/git?p=linux-jpb.git;a=commitdiff;h=e910e224b58712151dda06df595a53ff07edef63
> on branch virtio-iommu/v0.5-x86
> 