Re: [Xen-devel] [v3][PATCH 03/16] xen/vtd: create RMRR mapping

["Chen, Tiejun" <tiejun.chen@xxxxxxxxx>]

On 2015/6/11 17:14, Tian, Kevin wrote:
> > From: Chen, Tiejun
> > Sent: Thursday, June 11, 2015 9:15 AM
> >
> > RMRR reserved regions must be setup in the pfn space with an identity
> > mapping to reported mfn. However existing code has problem to setup
> > correct mapping when VT-d shares EPT page table, so lead to problem
> > when assigning devices (e.g GPU) with RMRR reported. So instead, this
> > patch aims to setup identity mapping in p2m layer, regardless of
> > whether EPT is shared or not. And we still keep creating VT-d table.
> >
> > Signed-off-by: Tiejun Chen <tiejun.chen@xxxxxxxxx>
> > ---
> >   xen/arch/x86/mm/p2m.c               | 10 ++++++++--
> >   xen/drivers/passthrough/vtd/iommu.c |  3 +--
> >   2 files changed, 9 insertions(+), 4 deletions(-)
> >
> > diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
> > index a6db236..c7198a5 100644
> > --- a/xen/arch/x86/mm/p2m.c
> > +++ b/xen/arch/x86/mm/p2m.c
> > @@ -927,10 +927,16 @@ int set_identity_p2m_entry(struct domain *d, unsigned long
> > gfn,
> >           }
> >
> >           gfn_unlock(p2m, gfn, 0);
> > -        return ret;
> >       }
> > +    else
> > +        ret = 0;
> >
> > -    return 0;
> > +    if( ret == 0 )
> > +    {
> > +        ret = iommu_map_page(d, gfn, gfn, IOMMUF_readable|IOMMUF_writable);
> > +    }
> > +
> > +    return ret;
> p2m_set_entry will setup IOMMU pages already. You don't need
> another explicit iommu map here.
Right.

> >   }
> >
> >   /* Returns: 0 for success, -errno for failure */
> > diff --git a/xen/drivers/passthrough/vtd/iommu.c
> > b/xen/drivers/passthrough/vtd/iommu.c
> > index 6a37624..31ce1af 100644
> > --- a/xen/drivers/passthrough/vtd/iommu.c
> > +++ b/xen/drivers/passthrough/vtd/iommu.c
> > @@ -1856,8 +1856,7 @@ static int rmrr_identity_mapping(struct domain *d, bool_t 
> > map,
> >
> >       while ( base_pfn < end_pfn )
> >       {
> > -        int err = intel_iommu_map_page(d, base_pfn, base_pfn,
> > -                                       IOMMUF_readable|IOMMUF_writable);
> > +        int err = set_identity_p2m_entry(d, base_pfn, p2m_access_rw);
> >
> >           if ( err )
> >               return err;
> Tim has another comment to replace earlier unmap with
Yes, I knew this.

> guest_physmap_remove_page() which will call iommu
> unmap internally. Please include this change too.
But,

guest_physmap_remove_page()
    |
    + p2m_remove_page()
        |
        + iommu_unmap_page()
        |
        + p2m_set_entry(p2m, gfn, _mfn(INVALID_MFN), xxx)

I think this already remove these pages both on ept/vt-d sides, right?

Or I'm misunderstand what you guys mean?

Thanks
Tiejun

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