Re: [PATCH v3 1/4] amd-iommu: add flush iommu_ops

[Paul Durrant <Paul.Durrant@citrix.com>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: 05 December 2018 13:13
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: Brian Woods <brian.woods@amd.com>; Suravee Suthikulpanit
> <suravee.suthikulpanit@amd.com>; Andrew Cooper
> <Andrew.Cooper3@citrix.com>; Roger Pau Monne <roger.pau@citrix.com>; Wei
> Liu <wei.liu2@citrix.com>; xen-devel <xen-devel@lists.xenproject.org>
> Subject: RE: [PATCH v3 1/4] amd-iommu: add flush iommu_ops
> 
> >>> On 05.12.18 at 13:58, <Paul.Durrant@citrix.com> wrote:
> >> From: Paul Durrant
> >> Sent: 05 December 2018 12:57
> >>
> >> > From: Jan Beulich [mailto:JBeulich@suse.com]
> >> > Sent: 05 December 2018 11:46
> >> >
> >> > >>> On 05.12.18 at 12:29, <paul.durrant@citrix.com> wrote:
> >> > > --- a/xen/drivers/passthrough/amd/iommu_map.c
> >> > > +++ b/xen/drivers/passthrough/amd/iommu_map.c
> >> > > @@ -634,6 +634,56 @@ int amd_iommu_unmap_page(struct domain *d,
> dfn_t
> >> > dfn)
> >> > >      spin_unlock(&hd->arch.mapping_lock);
> >> > >
> >> > >      amd_iommu_flush_pages(d, dfn_x(dfn), 0);
> >> > > +    return 0;
> >> > > +}
> >> > > +
> >> > > +static unsigned long flush_count(unsigned long dfn, unsigned int
> >> > page_count,
> >> > > +                                 unsigned int order)
> >> > > +{
> >> > > +    unsigned long start = dfn >> order;
> >> > > +    unsigned long end = (dfn + page_count - 1) >> 1;
> >> >
> >> > Granted in my earlier reply I had a missing opening parenthesis,
> >> > but the above is pretty clearly the wrong way of addressing
> >> > the resulting build error.
> >>
> >> Sorry, that's way too cryptic. What build error?
> >
> > I agree there is a typo there, in that the '>> 1' should be '>>
> order'... Is
> > that what you're getting at?
> 
> In the reply to v2 I did suggest
> 
>     unsigned long start = dfn >> order;
>     unsigned long end = (dfn + page_count - 1) >> order) + 1;
> 
> which of course needs to be
> 
>     unsigned long start = dfn >> order;
>     unsigned long end = ((dfn + page_count - 1) >> order) + 1;
> 
> whereas I was guessing that you might have noticed the build
> error with the original suggestion, but then wrongly dropped the
> "order) + " part.

Ah ok... no, I got what you meant and just completely typo-ed it. I'll send v4 unless you're happy to fix on commit.

  Paul

> 
> Jan
> 


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