[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH 11/13] intel_iommu: provide its own replay()
From: |
Peter Xu |
Subject: |
Re: [Qemu-devel] [RFC PATCH 11/13] intel_iommu: provide its own replay() callback |
Date: |
Fri, 30 Dec 2016 17:18:50 +0800 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
On Fri, Dec 30, 2016 at 04:39:49AM +0000, Liu, Yi L wrote:
> > -----Original Message-----
> > From: Peter Xu [mailto:address@hidden
> > Sent: Friday, December 30, 2016 11:44 AM
> > To: Liu, Yi L <address@hidden>
> > Cc: Tian, Kevin <address@hidden>; Lan, Tianyu <address@hidden>; 'qemu-
> > address@hidden' <address@hidden>; 'address@hidden'
> > <address@hidden>
> > Subject: Re: [Qemu-devel] [RFC PATCH 11/13] intel_iommu: provide its own
> > replay()
> > callback
> >
> > On Thu, Dec 29, 2016 at 10:23:21AM +0000, Liu, Yi L wrote:
> >
> > [...]
> >
> > > > >
> > > > > Hereby, I have a minor concern on this part.
> > > > > The replay would be performed in vfio_listener_region_add() when
> > > > > device is
> > > > > assigned. Since guest is just started, so there will be no valid
> > > > > context entry for
> > the
> > > > > assigned device. So there will be no vtd_page_walk. Thus no change to
> > > > > SL page
> > > > > table in host. The SL page table would still be a IOVA->HPA mapping.
> > > > > It's true
> > > > > that the gIOVA->HPA mapping can be built by page map/unmap when guest
> > > > > trying to issue dma. So it is just ok without relpay in the
> > > > > beginning. Hot plug
> > should
> > > > > be all the same. I guess it is the design here?
> > > >
> > > > I am not sure whether I get your point here - I think it's by design.
> > > > We don't really need replay if we are sure that the IOMMU address
> > > > space has totally no mapping (i.e., when the guest init). Replay is
> > > > only useful when there are existing mappings in the IOMMU address
> > > > space.
> > > yes, if we are sure no existing mapping, it is ok. There is another case
> > > which may
> > > complain it. If a device is hotplug in, then there should be existing
> > > mapping. Just
> > > looked code again. If corresponding context entry is not present, the
> > > vtd_page_walk should be bypassed.
> >
> > Yes, if the context entry is not present, the replay will be bypassed.
> > And if it exists, then the replay will work (along with the page walk
> > process). Shouldn't that the behavior we want?
> >
> > Please clarify if I misunderstood your question.
> I was just assuming a replay should be performed when a new device is plug in.
> However, according to your statements, it is not required. Is this the
> conclusion in
> the previous discussion?
Hmm, I mean whether the replay is needed for the hotplugged device
should depend on which iommu domain it joins when it is plugged. I
think it should belong to nowhere when plugged but I am not quite
sure... IIUC that'll depend on whether the context entry of that
plugged device is valid - and I think it should be an invalid one.
>
> > [...]
> >
> > > > >
> > > > > However, it may be incompatible with a future work in which we expose
> > > > > an
> > > > > vIOMMU to guest but guest disable IOVA usage. In this scenario, the
> > > > > SL page
> > > > > table in host would be a GPA->HPA instead of a gIOVA->HPA mapping. It
> > > > > would
> > > > > rely on memory_replay to build a GPA->HPA mapping. If I'm correct, I
> > > > > think it
> > > > > would be expected to have a replay in the time device is assigned.
> > > >
> > > > If you need a GPA -> HPA mapping (AFAICT you mean virtualized SVM and
> > > > the so-called first level translation), IMHO the default mapping
> > > > behavior of vfio_listener_region_add() suites here (when
> > > > memory_region_is_iommu(section->mr) == false), which maps the whole
> > > > guest physical address space, just like the case when we use vfio
> > > > devices without vIOMMU.
> > > Yes, I mean SVM virtualizaion. To virtualize SVM, an vIOMMU would be
> > > exposed to
> > > guest. So memory_region_is_iommu(section->mr) would be true. Then, the
> > original
> > > mapping the whole guest physical address space would not run. Thus no
> > > GPA->HPA
> > > mapping would be built. This is what I really concern here.
> >
> > What I meant above was, we might be able to leverage existing
> > no-viommu logic to build the SLPT for virtualized SVM. For example, we
> > can switch the if block from:
> >
> > if (memory_region_is_iommu(section->mr))
> >
> > into something similiar like:
> >
> > if (memory_region_is_iommu(section->mr) &&
> > !FIRST_LEVEL_TRANSLATION_ENABLED(section->mr))
> >
> > Just a quick thought, not sure whether that would be an applicable
> > idea. Thanks,
> If I remember correctly, with vIOMMU exposed, section->mr here should
> not be a ram, the logic would fail in the following code. I agree with you
> that map the whole guest physical address space should work with SVM
> virtualization. May need to think more on how to make it happen.
>
> hw/i386/common.c, a snippet:
> /* Here we assume that memory_region_is_ram(section->mr)==true */
>
> vaddr = memory_region_get_ram_ptr(section->mr) +
> section->offset_within_region +
> (iova - section->offset_within_address_space);
Right. That code won't work if only with a change in the if() clause -
I wasn't showing any workable codes (even it's not pesudo), only to
show what I meant. :)
-- peterx