Re: [Xen-devel] [PATCH v3.1 08/15] x86/vtd: fix mapping of RMRR regions

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 04.11.16 at 14:03, <roger.pau@xxxxxxxxxx> wrote:
> On Fri, Nov 04, 2016 at 06:53:09AM -0600, Jan Beulich wrote:
>> >>> On 04.11.16 at 13:25, <roger.pau@xxxxxxxxxx> wrote:
>> > On Fri, Nov 04, 2016 at 04:34:58AM -0600, Jan Beulich wrote:
>> >> >>> On 04.11.16 at 10:45, <roger.pau@xxxxxxxxxx> wrote:
>> >> > case p2m_invalid:
>> >> > case p2m_mmio_dm:
>> >> >     ret = p2m_set_entry(p2m, gfn, _mfn(gfn), PAGE_ORDER_4K,
>> >> >                         p2m_mmio_direct, p2ma);
>> >> >     if ( ret )
>> >> >         break;
>> >> >     if ( !iommu_use_hap_pt(d) )
>> >> >         ret = iommu_map_page(d, gfn, gfn, 
> IOMMUF_readable|IOMMUF_writable);
>> >> >     break;
>> >> > case p2m_mmio_direct:
>> >> >     if ( a != p2ma || gfn != mfn )
>> >> >     {
>> >> >         printk(XENLOG_G_WARNING
>> >> >                "Cannot setup identity map d%d:%lx, already mapped with "
>> >> >                "different access type or mfn\n", d->domain_id, gfn);
>> >> >         ret = (flag & XEN_DOMCTL_DEV_RDM_RELAXED) ? 0 : -EBUSY;
>> >> >         break;
>> >> >     }
>> >> >     if ( !iommu_use_hap_pt(d) )
>> >> >         ret = iommu_map_page(d, gfn, gfn, 
> IOMMUF_readable|IOMMUF_writable);
>> >> 
>> >> Well, since according to what I've said above this code should
>> >> really not be here, I think the code structuring question is moot
>> >> now. The conditional call to iommu_map_page() really just needs
>> >> adding alongside the p2m_set_entry() call.
>> > 
>> > OK, so if the gfn is already mapped into the p2m we don't care whether it 
>> > has a valid IOMMU mapping or not?
>> 
>> We do care, but it is the responsibility of whoever established the
>> first mapping to make sure it's present in both P2M and IOMMU.
>> IOW if the GFN is already mapped, we should be able to imply that
>> it's mapped in both places.
> 
> But how is the first caller that established the mapping supposed to know if 
> it needs an IOMMU entry or not? (p2m_mmio_direct types don't get an IOMMU 
> mapping at all)

And it's that fact stated in parentheses which I'd like to question.
I don't see what's wrong with e.g. DMAing right into / out of a
video frame buffer.

> Are we expecting the first caller that setup the mapping to also know about 
> RMRR regions and add the IOMMU entry if needed?

This has nothing to do with RMRR regions: Whoever establishes
_some_ P2M mapping ought to also establish an IOMMU one, if
the tables aren't shared (unless there is an explicit reason not do
so).

Jan


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