From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wei Wang Subject: Re: [PATCH 2 of 6 V6] amd iommu: call guest_iommu_set_base from hvmloader Date: Mon, 15 Oct 2012 15:41:46 +0200 Message-ID: <507C129A.2010203@amd.com> References: <5063155A.4070307@amd.com> <50642A2A020000780009E32D@nat28.tlf.novell.com> <507BDED1.6020005@amd.com> <507BFD7802000078000A1526@nat28.tlf.novell.com> <507C004B.20003@amd.com> <507C233702000078000A1670@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <507C233702000078000A1670@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: Keir Fraser , "xen-devel@lists.xensource.com" , Ian Jackson , IanCampbell List-Id: xen-devel@lists.xenproject.org On 10/15/2012 02:52 PM, Jan Beulich wrote: >>>> On 15.10.12 at 14:23, Wei Wang wrote: >> On 10/15/2012 12:11 PM, Jan Beulich wrote: >>>>>> On 15.10.12 at 12:00, Wei Wang wrote: >>>> On 09/27/2012 10:27 AM, Jan Beulich wrote: >>>>>>>> On 26.09.12 at 16:46, Wei Wang wrote: >>>>>> @@ -3834,6 +3835,9 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE(void) >>>> arg) >>>>>> case HVM_PARAM_BUFIOREQ_EVTCHN: >>>>>> rc = -EINVAL; >>>>>> break; >>>>>> + case HVM_PARAM_IOMMU_BASE: >>>>>> + rc = guest_iommu_set_base(d, a.value); >>>>> >>>>> This suggests that you're allowing for only a single IOMMU per >>>>> guest - is that not going to become an issue sooner or later? >>>> >>>> I think that one iommu per guest is probably enough. Because guest IVRS >>>> table is totally virtual, it does not reflect any pci relationship of >>>> real systems. Even if qemu supports multi pci buses, we can still >>>> virtually group them together into one virtual IVRS table. It might be >>>> an issue if qemu uses multi pci segments, but so far even hardware iommu >>>> only uses segment 0. Additionally, the guest iommu is only used by ats >>>> capable GPUs. Normal passthrough device should not make use of it. So,, >>>> What do you think? >>> >>> Especially the multi-segment aspect makes me think that the >>> interface should allow for multiple IOMMUs, even if the >>> implementation supports only one for now. >> >> Ok, then I will rework the interface to take iommu segment as an >> additional parameter. > > That'll likely make the interface even more ugly than the more > flexible variant allowing for multiple IOMMUs independent of > the segment they're on/for. But let's see what you come up > with... Hi Jan My idea is to make the interface looks like that guest_iommu_set_base(d, iommu_base, iommu_seg) This will allow hvmloader to choose a non-zero iommu segment. if iommu_seg > 0, then I will add a new iommu to guest iommu list and update iommu_segment field accordingly. But I seem to see no reason to add multiple guest iommus to pci segment 0. Thanks, Wei > Jan > >