Re: [PATCH v6 4/8] iommu: make map and unmap take a page count, similar to flush

[Oleksandr <olekstysh@xxxxxxxxx>]

On 11.09.20 10:59, Paul Durrant wrote:

Hi

> De-htmling...
>
> -----
> From: Oleksandr Tyshchenko <olekstysh@xxxxxxxxx>
> Sent: 10 September 2020 19:20
> To: Durrant, Paul <pdurrant@xxxxxxxxxxxx>
> Cc: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>; Paul Durrant <paul@xxxxxxx>; open list:X86 <xen-devel@xxxxxxxxxxxxxxxxxxxx>; Jan Beulich 
> <jbeulich@xxxxxxxx>; Andrew Cooper <andrew.cooper3@xxxxxxxxxx>; Wei Liu <wl@xxxxxxx>; Roger Pau Monné <roger.pau@xxxxxxxxxx>; George 
> Dunlap <george.dunlap@xxxxxxxxxx>; Ian Jackson <ian.jackson@xxxxxxxxxxxxx>; Julien Grall <julien@xxxxxxx>; Stefano Stabellini 
> <sstabellini@xxxxxxxxxx>; Jun Nakajima <jun.nakajima@xxxxxxxxx>; Kevin Tian <kevin.tian@xxxxxxxxx>
> Subject: Re: [PATCH v6 4/8] iommu: make map and unmap take a page count, 
> similar to flush
>
>
>
> On Thu, Sep 10, 2020 at 8:49 PM Durrant, Paul <mailto:pdurrant@xxxxxxxxxxxx> 
> wrote:
>
> Hi Paul
>
> [sorry for the possible format issue]
>
> > -----Original Message-----
> > From: Bertrand Marquis <mailto:Bertrand.Marquis@xxxxxxx>
> > Sent: 10 September 2020 17:46
> > To: Paul Durrant <mailto:paul@xxxxxxx>
> > Cc: open list:X86 <mailto:xen-devel@xxxxxxxxxxxxxxxxxxxx>; Durrant, Paul 
> > <mailto:pdurrant@xxxxxxxxxxxx>; Jan Beulich
> > <mailto:jbeulich@xxxxxxxx>; Andrew Cooper <mailto:andrew.cooper3@xxxxxxxxxx>; Wei Liu 
> > <mailto:wl@xxxxxxx>; Roger Pau Monné
> > <mailto:roger.pau@xxxxxxxxxx>; George Dunlap <mailto:george.dunlap@xxxxxxxxxx>; 
> > Ian Jackson
> > <mailto:ian.jackson@xxxxxxxxxxxxx>; Julien Grall <mailto:julien@xxxxxxx>; 
> > Stefano Stabellini
> > <mailto:sstabellini@xxxxxxxxxx>; Jun Nakajima <mailto:jun.nakajima@xxxxxxxxx>; Kevin 
> > Tian <mailto:kevin.tian@xxxxxxxxx>
> > Subject: RE: [EXTERNAL] [PATCH v6 4/8] iommu: make map and unmap take a page 
> > count, similar to flush
> >
> > CAUTION: This email originated from outside of the organization. Do not click 
> > links or open
> > attachments unless you can confirm the sender and know the content is safe.
> >
> >
> >
> > > > diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
> > > > index 1831dc66b0..13f68dc93d 100644
> > > > --- a/xen/include/xen/iommu.h
> > > > +++ b/xen/include/xen/iommu.h
> > > > @@ -146,23 +146,23 @@ enum
> > > > #define IOMMU_FLUSHF_modified (1u << _IOMMU_FLUSHF_modified)
> > > >
> > > > int __must_check iommu_map(struct domain *d, dfn_t dfn, mfn_t mfn,
> > > > -                           unsigned int page_order, unsigned int flags,
> > > > +                           unsigned long page_count, unsigned int flags,
> > > >                            unsigned int *flush_flags);
> > > > int __must_check iommu_unmap(struct domain *d, dfn_t dfn,
> > > > -                             unsigned int page_order,
> > > > +                             unsigned long page_count,
> > > >                              unsigned int *flush_flags);
> > > >
> > > > int __must_check iommu_legacy_map(struct domain *d, dfn_t dfn, mfn_t mfn,
> > > > -                                  unsigned int page_order,
> > > > +                                  unsigned long page_count,
> > > >                                   unsigned int flags);
> > > > int __must_check iommu_legacy_unmap(struct domain *d, dfn_t dfn,
> > > > -                                    unsigned int page_order);
> > > > +                                    unsigned long page_count);
> > > >
> > > > int __must_check iommu_lookup_page(struct domain *d, dfn_t dfn, mfn_t *mfn,
> > > >                                    unsigned int *flags);
> > > >
> > > > int __must_check iommu_iotlb_flush(struct domain *d, dfn_t dfn,
> > > > -                                   unsigned int page_count,
> > > > +                                   unsigned long page_count,
> > > >                                    unsigned int flush_flags);
> > > > int __must_check iommu_iotlb_flush_all(struct domain *d,
> > > >                                        unsigned int flush_flags);
> > > > @@ -281,7 +281,7 @@ struct iommu_ops {
> > > >     void (*share_p2m)(struct domain *d);
> > > >     void (*crash_shutdown)(void);
> > > >     int __must_check (*iotlb_flush)(struct domain *d, dfn_t dfn,
> > > > -                                    unsigned int page_count,
> > > > +                                    unsigned long page_count,
> > > This change will require to change the arm smmu code to have the right type for 
> > > page count:
> > > xen/drivers/passthrough/smmu.c:2536
> > > static int __must_check arm_smmu_iotlb_flush(struct domain *d, dfn_t dfn,
> > >                          unsigned int page_count,
> > >                          unsigned int flush_flags)
> > >
> > > Otherwise compilation is failing for arm.
> > >
> > > With that fixed i could compile and start an arm system with the complete serie 
> > > (but not one with an
> > arm SMMU).
> >
> > I should have specified because my test system right now does not have an 
> > SMMUv1.
> Thanks for spotting that; I did run a cross compilation on arm a while ago so 
> not sure how I managed to miss this. Will fix and send v7.
>
>
>   Probably ipmmu_iotlb_flush() in ipmmu-vmsa.c needs the same update as well (I 
> don't have the possibility to apply your series and re-check)? Please note, it 
> is still under CONFIG_EXPERT.
> -----
>
> Oh, that's new I guess?
Not so new, AFAIR it is in mainline since v4.13 or little bit earlier, 
but still under Tech Preview.

>   I'll go check.
Thank you.


--
Regards,

Oleksandr Tyshchenko