Re: [Xen-devel] [PATCH V2 1/6] iommu/arm: Add iommu_helpers.c file to keep common for IOMMUs stuff

[Oleksandr <olekstysh@xxxxxxxxx>]

Hi, Julien


> On 02/08/2019 17:39, Oleksandr Tyshchenko wrote:
> > From: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
> >
> > Introduce a separate file to keep various helpers which could be used
> > by more than one IOMMU driver in order not to duplicate code.
> >
> > The first condidates to be moved to the new file are SMMU driver's
> NIT: s/condidates/candidates/
ok


> > "map_page/unmap_page" callbacks. There callbacks neither contain any
> > SMMU specific info nor perform any SMMU specific actions and are going
> > to be the same across all IOMMU drivers which H/W IP shares P2M
> > with the CPU like SMMU does.
> >
> > So, move callbacks to iommu_helpers.c for the upcoming IPMMU driver
> > to be able to re-use them.
> >
> > Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
> > ---
> >   xen/drivers/passthrough/arm/Makefile        |  2 +-
> >   xen/drivers/passthrough/arm/iommu_helpers.c | 78 
> > +++++++++++++++++++++++++++++
> >   xen/drivers/passthrough/arm/smmu.c          | 48 +-----------------
> >   xen/include/asm-arm/iommu.h                 |  7 +++
> >   4 files changed, 88 insertions(+), 47 deletions(-)
> >   create mode 100644 xen/drivers/passthrough/arm/iommu_helpers.c
> >
> > diff --git a/xen/drivers/passthrough/arm/Makefile 
> > b/xen/drivers/passthrough/arm/Makefile
> > index b3efcfd..4abb87a 100644
> > --- a/xen/drivers/passthrough/arm/Makefile
> > +++ b/xen/drivers/passthrough/arm/Makefile
> > @@ -1,2 +1,2 @@
> > -obj-y += iommu.o
> > +obj-y += iommu.o iommu_helpers.o
> >   obj-$(CONFIG_ARM_SMMU) += smmu.o
> > diff --git a/xen/drivers/passthrough/arm/iommu_helpers.c 
> > b/xen/drivers/passthrough/arm/iommu_helpers.c
> > new file mode 100644
> > index 0000000..53e8daa
> > --- /dev/null
> > +++ b/xen/drivers/passthrough/arm/iommu_helpers.c
> > @@ -0,0 +1,78 @@
> > +/*
> > + * xen/drivers/passthrough/arm/iommu_helpers.c
> > + *
> > + * Contains various helpers to be used by IOMMU drivers.
> > + *
> > + * Copyright (C) 2019 EPAM Systems Inc.
> You mostly moved the code from the SMMU code, so the copyright there 
> should be retain. As this is Xen modification the copyright here 
> should be:
>  * Copyright (C) 2014 Linaro Limited.
Oh, yes. Sorry, forgot about it. Will add.


> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms and conditions of the GNU General Public
> > + * License, version 2, as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > + * General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public
> > + * License along with this program; If not, see 
> > <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#include <xen/lib.h>
> > +#include <xen/sched.h>
> > +#include <xen/iommu.h>
> Could you order the headers above alphabetical please?
>
> And also as a extra NIT a newline before 'xen' headers and 'asm' one :).
Yes, will follow this rule.


> > +#include <asm/device.h>
> The rest of the good looks good to me.
>
> Cheers,
--
Regards,

Oleksandr Tyshchenko


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