> -----Original Message-----
> From: [email protected] [mailto:[email protected]] On Behalf Of
> George Dunlap
> Sent: 10 July 2018 14:47
> To: Paul Durrant <[email protected]>
> Cc: xen-devel <[email protected]>; Kevin Tian
> <[email protected]>; Stefano Stabellini <[email protected]>;
> Suravee Suthikulpanit <[email protected]>; Julien Grall
> <[email protected]>; Jan Beulich <[email protected]>
> Subject: Re: [Xen-devel] [PATCH v2 02/13] iommu: introduce the concept of
> BFN...
>
> On Sat, Jul 7, 2018 at 12:05 PM, Paul Durrant <[email protected]>
> wrote:
>
> > @@ -651,34 +651,34 @@ int amd_iommu_map_page(struct domain *d,
> unsigned long gfn, unsigned long mfn,
> > if ( rc )
> > {
> > spin_unlock(&hd->arch.mapping_lock);
> > - AMD_IOMMU_DEBUG("Root table alloc failed, gfn = %lx\n", gfn);
> > + AMD_IOMMU_DEBUG("Root table alloc failed, bfn = %lx\n", bfn);
> > domain_crash(d);
> > return rc;
> > }
> >
> > /* Since HVM domain is initialized with 2 level IO page table,
> > - * we might need a deeper page table for lager gfn now */
> > + * we might need a deeper page table for wider bfn now */
>
> Oh, come on, no lager gfns? ;-)
>
> > @@ -2737,10 +2737,12 @@ static void
> arm_smmu_iommu_domain_teardown(struct domain *d)
> > xfree(xen_domain);
> > }
> >
> > -static int __must_check arm_smmu_map_page(struct domain *d,
> unsigned long gfn,
> > +static int __must_check arm_smmu_map_page(struct domain *d,
> unsigned long bfn,
> > unsigned long mfn, unsigned int flags)
> > {
> > p2m_type_t t;
> > + unsigned long frame = bfn;
> > + unsigned long gfn;
>
> I don't understand what the poind of these temporary variables are.
> They don't seem to really add any information at this point.
I added these at Jan's request. I can defer introducing them to the later patch
introducing typesafe bfns and gfns if that's preferable. The eventual code will
be the same.
Paul
>
> >
> > /*
> > * Grant mappings can be used for DMA requests. The dev_bus_addr
> > @@ -2748,10 +2750,11 @@ static int __must_check
> arm_smmu_map_page(struct domain *d, unsigned long gfn,
> > * protected by an IOMMU, Xen needs to add a 1:1 mapping in the
> domain
> > * p2m to allow DMA request to work.
> > * This is only valid when the domain is directed mapped. Hence this
> > - * function should only be used by gnttab code with gfn == mfn.
> > + * function should only be used by gnttab code with gfn == mfn ==
> bfn.
> > */
> > BUG_ON(!is_domain_direct_mapped(d));
> > - BUG_ON(mfn != gfn);
> > + BUG_ON(mfn != frame);
> > + gfn = frame;
> >
> > /* We only support readable and writable flags */
> > if (!(flags & (IOMMUF_readable | IOMMUF_writable)))
> > @@ -2766,8 +2769,12 @@ static int __must_check
> arm_smmu_map_page(struct domain *d, unsigned long gfn,
> > return guest_physmap_add_entry(d, _gfn(gfn), _mfn(mfn), 0, t);
> > }
> >
> > -static int __must_check arm_smmu_unmap_page(struct domain *d,
> unsigned long gfn)
> > +static int __must_check arm_smmu_unmap_page(struct domain *d,
> unsigned long bfn)
> > {
> > + unsigned long frame = bfn;
> > + unsigned long gfn = frame;
> > + unsigned long mfn = frame;
>
> Same here.
>
> > diff --git a/xen/drivers/passthrough/iommu.c
> b/xen/drivers/passthrough/iommu.c
> > index 2c44fabf99..9a3bb6a43e 100644
> > --- a/xen/drivers/passthrough/iommu.c
> > +++ b/xen/drivers/passthrough/iommu.c
> > @@ -185,7 +185,8 @@ void __hwdom_init iommu_hwdom_init(struct
> domain *d)
> > page_list_for_each ( page, &d->page_list )
> > {
> > unsigned long mfn = mfn_x(page_to_mfn(page));
> > - unsigned long gfn = mfn_to_gmfn(d, mfn);
> > + unsigned long frame = mfn_to_gmfn(d, mfn);
> > + unsigned long bfn = frame;
>
> And here.
>
> Everything else looks good.
>
> -George
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
