Re: [Xen-devel] [PATCH v6 06/14] iommu: track reserved ranges using a rangeset

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: 07 September 2018 11:40
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> Cc: Kevin Tian <kevin.tian@xxxxxxxxx>; xen-devel <xen-
> devel@xxxxxxxxxxxxxxxxxxxx>
> Subject: Re: [PATCH v6 06/14] iommu: track reserved ranges using a rangeset
> 
> >>> On 23.08.18 at 11:47, <paul.durrant@xxxxxxxxxx> wrote:
> > @@ -248,12 +252,16 @@ int iommu_construct(struct domain *d)
> >
> >  void iommu_domain_destroy(struct domain *d)
> >  {
> > -    if ( !iommu_enabled || !dom_iommu(d)->platform_ops )
> > +    const struct domain_iommu *hd = dom_iommu(d);
> > +
> > +    if ( !iommu_enabled || !hd->platform_ops )
> >          return;
> >
> >      iommu_teardown(d);
> >
> >      arch_iommu_domain_destroy(d);
> > +
> > +    rangeset_destroy(hd->reserved_ranges);
> 
> For idempotency reasons perhaps better to store NULL after
> the call?

Ok.

> 
> > --- a/xen/drivers/passthrough/vtd/x86/vtd.c
> > +++ b/xen/drivers/passthrough/vtd/x86/vtd.c
> > @@ -154,8 +154,21 @@ void __hwdom_init
> vtd_set_hwdom_mapping(struct domain *d)
> >
> >          rc = iommu_map_page(d, _bfn(pfn), _mfn(pfn),
> >                         IOMMUF_readable | IOMMUF_writable);
> > +
> > +        /*
> > +         * The only reason a reserved page would be mapped is that
> > +         * iommu_inclusive_mapping is set, in which case it needs to be
> > +         * marked as reserved in the IOMMU.
> > +         */
> > +        if ( !rc && page_is_ram_type(pfn, RAM_TYPE_RESERVED) )
> > +        {
> > +            ASSERT(iommu_inclusive_mapping);
> > +
> > +            rc = rangeset_add_singleton(dom_iommu(d)->reserved_ranges,
> pfn);
> > +        }
> 
> Why would this be restricted to the E820 reserved type? I think this
> should cover everything that gets mapped with iommu_inclusive_mapping
> set, but not mapped with the flag clear.
> 

When PV-IOMMU is enabled it clearly needs to be modify the domain's memory 
mappings (otherwise what's the point of the API) so I could change this range 
to include everything mapped for the domain except ranges explicitly identified 
as RAM.

  Paul

> Jan
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel