| Message ID | 20180209031055.21926-7-sameer.goel@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | SMMUv3 driver | expand |
On Thu, Feb 08, 2018 at 08:10:54PM -0700, Sameer Goel wrote: > Pull common defines for SMMU drives in a local header. > > Signed-off-by: Sameer Goel <sameer.goel@linaro.org> > --- > xen/drivers/passthrough/arm/arm_smmu.h | 125 +++++++++++++++++++++++++++++++++ > xen/drivers/passthrough/arm/smmu-v3.c | 96 +------------------------ > xen/drivers/passthrough/arm/smmu.c | 104 +-------------------------- > 3 files changed, 128 insertions(+), 197 deletions(-) > create mode 100644 xen/drivers/passthrough/arm/arm_smmu.h > > diff --git a/xen/drivers/passthrough/arm/arm_smmu.h b/xen/drivers/passthrough/arm/arm_smmu.h > new file mode 100644 > index 0000000000..f49dceb5b4 > --- /dev/null > +++ b/xen/drivers/passthrough/arm/arm_smmu.h > @@ -0,0 +1,125 @@ > +/****************************************************************************** > + * ./arm_smmu.h > + * > + * Common compatibility defines and data_structures for porting arm smmu > + * drivers from Linux. > + * > + * Copyright (c) 2017 Linaro Limited > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * 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/>. > + */ > + > +#ifndef __ARM_SMMU_H__ > +#define __ARM_SMMU_H__ > + > + > +/* Alias to Xen device tree helpers */ > +#define device_node dt_device_node > +#define of_phandle_args dt_phandle_args > +#define of_device_id dt_device_match > +#define of_match_node dt_match_node > +#define of_property_read_u32(np, pname, out) (!dt_property_read_u32(np, pname, out)) Break the line. > +#define of_property_read_bool dt_property_read_bool > +#define of_parse_phandle_with_args dt_parse_phandle_with_args > + > +/* Helpers to get device MMIO and IRQs */ > +struct resource { > + u64 addr; > + u64 size; uint64_t is preferred rather than u64. > + unsigned int type; > +}; > + > +#define resource_size(res) ((res)->size) > + > +#define platform_device device > + > +#define IORESOURCE_MEM 0 > +#define IORESOURCE_IRQ 1 > + > +/* Stub out DMA domain related functions */ > +#define iommu_get_dma_cookie(dom) 0 > +#define iommu_put_dma_cookie(dom) > + > +#define VA_BITS 0 /* Only used for configuring stage-1 input size */ > + > +#define MODULE_DEVICE_TABLE(type, name) > +#define module_param_named(name, value, type, perm) > +#define MODULE_PARM_DESC(_parm, desc) > + > +#define dma_set_mask_and_coherent(d, b) 0 > +#define of_dma_is_coherent(n) 0 > + > +static void __iomem *devm_ioremap_resource(struct device *dev, > + struct resource *res) Aligment, please use spaces. Also, is __iomem needed here at all? > +{ > + void __iomem *ptr; Same question about __iomem attribute. > + > + if ( !res || res->type != IORESOURCE_MEM ) > + { > + dev_err(dev, "Invalid resource\n"); > + return ERR_PTR(-EINVAL); > + } > + > + ptr = ioremap_nocache(res->addr, res->size); > + if ( !ptr ) > + { > + dev_err(dev, "ioremap failed (addr 0x%"PRIx64" size 0x%"PRIx64")\n", > + res->addr, res->size); > + return ERR_PTR(-ENOMEM); > + } > + > + return ptr; > +} > + > +/* > + * Domain type definitions. Not really needed for Xen, defining to port > + * Linux code as-is > + */ > +#define IOMMU_DOMAIN_UNMANAGED 0 > +#define IOMMU_DOMAIN_DMA 1 > +#define IOMMU_DOMAIN_IDENTITY 2 > + > +/* Xen: Compatibility define for iommu_domain_geometry.*/ > +struct iommu_domain_geometry { > + dma_addr_t aperture_start; /* First address that can be mapped */ > + dma_addr_t aperture_end; /* Last address that can be mapped */ > + bool force_aperture; /* DMA only allowed in mappable range? */ > +}; > + > +/* Xen: Dummy iommu_domain */ > +struct iommu_domain { > + /* Runtime SMMU configuration for this iommu_domain */ > + struct arm_smmu_domain *priv; > + unsigned int type; > + > + /* Dummy compatibility defines */ > + unsigned long pgsize_bitmap; > + struct iommu_domain_geometry geometry; > + > + atomic_t ref; > + /* Used to link iommu_domain contexts for a same domain. > + * There is at least one per-SMMU to used by the domain. > + */ > + struct list_head list; No tabs please. And if you want to align the fields, please do so uniformly for all the structs in the file. > +}; > + > +/* Xen: Describes information required for a Xen domain */ > +struct arm_smmu_xen_domain { > + spinlock_t lock; > + /* List of iommu domains associated to this domain */ > + struct list_head contexts; > +}; Tabs. > + > +#endif /* __ARM_SMMU_H__ */ > + > diff --git a/xen/drivers/passthrough/arm/smmu-v3.c b/xen/drivers/passthrough/arm/smmu-v3.c > index f43485fe6e..f0a61521fb 100644 > --- a/xen/drivers/passthrough/arm/smmu-v3.c > +++ b/xen/drivers/passthrough/arm/smmu-v3.c > @@ -49,28 +49,7 @@ > #include <asm/io.h> > #include <asm/platform.h> > > -/* Alias to Xen device tree helpers */ > -#define device_node dt_device_node > -#define of_phandle_args dt_phandle_args > -#define of_device_id dt_device_match > -#define of_match_node dt_match_node > -#define of_property_read_u32(np, pname, out) (!dt_property_read_u32(np, pname, out)) > -#define of_property_read_bool dt_property_read_bool > -#define of_parse_phandle_with_args dt_parse_phandle_with_args > - > -/* Xen: Helpers to get device MMIO and IRQs */ > -struct resource { > - u64 addr; > - u64 size; > - unsigned int type; > -}; > - > -#define resource_size(res) ((res)->size) > - > -#define platform_device device > - > -#define IORESOURCE_MEM 0 > -#define IORESOURCE_IRQ 1 You introduce the above code in patch 5, and remove it in patch 6, is this really needed? Ie: why not simply introduce this code directly in this patch? > diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c > index ad956d5b8d..4c04391e21 100644 > --- a/xen/drivers/passthrough/arm/smmu.c > +++ b/xen/drivers/passthrough/arm/smmu.c > @@ -41,6 +41,7 @@ > #include <xen/irq.h> > #include <xen/lib.h> > #include <xen/list.h> > +#include <xen/linux_compat.h> > #include <xen/mm.h> > #include <xen/vmap.h> > #include <xen/rbtree.h> > @@ -51,36 +52,13 @@ > #include <asm/io.h> > #include <asm/platform.h> > > +#include "arm_smmu.h" /* Not a self contained header. So last in the list */ > /* Xen: The below defines are redefined within the file. Undef it */ > #undef SCTLR_AFE > #undef SCTLR_TRE > #undef SCTLR_M > #undef TTBCR_EAE > > -/* Alias to Xen device tree helpers */ > -#define device_node dt_device_node > -#define of_phandle_args dt_phandle_args > -#define of_device_id dt_device_match > -#define of_match_node dt_match_node > -#define of_property_read_u32(np, pname, out) (!dt_property_read_u32(np, pname, out)) > -#define of_property_read_bool dt_property_read_bool > -#define of_parse_phandle_with_args dt_parse_phandle_with_args > - > -/* Xen: Helpers to get device MMIO and IRQs */ > -struct resource > -{ > - u64 addr; > - u64 size; > - unsigned int type; > -}; > - > -#define resource_size(res) (res)->size; > - > -#define platform_device device > - > -#define IORESOURCE_MEM 0 > -#define IORESOURCE_IRQ 1 > - > static struct resource *platform_get_resource(struct platform_device *pdev, > unsigned int type, > unsigned int num) > @@ -118,58 +96,6 @@ static struct resource *platform_get_resource(struct platform_device *pdev, > > /* Xen: Helpers for IRQ functions */ > #define request_irq(irq, func, flags, name, dev) request_irq(irq, flags, func, name, dev) > -#define free_irq release_irq > - > -enum irqreturn { > - IRQ_NONE = (0 << 0), > - IRQ_HANDLED = (1 << 0), > -}; > - > -typedef enum irqreturn irqreturn_t; You remove the irqreturn enum without adding any replacement, is this really unused? Thanks, Roger.
Hi, On 02/09/2018 10:43 AM, Roger Pau Monné wrote: >> + unsigned int type; >> +}; >> + >> +#define resource_size(res) ((res)->size) >> + >> +#define platform_device device >> + >> +#define IORESOURCE_MEM 0 >> +#define IORESOURCE_IRQ 1 >> + >> +/* Stub out DMA domain related functions */ >> +#define iommu_get_dma_cookie(dom) 0 >> +#define iommu_put_dma_cookie(dom) >> + >> +#define VA_BITS 0 /* Only used for configuring stage-1 input size */ >> + >> +#define MODULE_DEVICE_TABLE(type, name) >> +#define module_param_named(name, value, type, perm) >> +#define MODULE_PARM_DESC(_parm, desc) >> + >> +#define dma_set_mask_and_coherent(d, b) 0 >> +#define of_dma_is_coherent(n) 0 >> + >> +static void __iomem *devm_ioremap_resource(struct device *dev, >> + struct resource *res) > > Aligment, please use spaces. > > Also, is __iomem needed here at all? On Arm, we tend to add keep __iomem on pointer dealing with MMIO. [...] >> + >> +#endif /* __ARM_SMMU_H__ */ >> + >> diff --git a/xen/drivers/passthrough/arm/smmu-v3.c b/xen/drivers/passthrough/arm/smmu-v3.c >> index f43485fe6e..f0a61521fb 100644 >> --- a/xen/drivers/passthrough/arm/smmu-v3.c >> +++ b/xen/drivers/passthrough/arm/smmu-v3.c >> @@ -49,28 +49,7 @@ >> #include <asm/io.h> >> #include <asm/platform.h> >> >> -/* Alias to Xen device tree helpers */ >> -#define device_node dt_device_node >> -#define of_phandle_args dt_phandle_args >> -#define of_device_id dt_device_match >> -#define of_match_node dt_match_node >> -#define of_property_read_u32(np, pname, out) (!dt_property_read_u32(np, pname, out)) >> -#define of_property_read_bool dt_property_read_bool >> -#define of_parse_phandle_with_args dt_parse_phandle_with_args >> - >> -/* Xen: Helpers to get device MMIO and IRQs */ >> -struct resource { >> - u64 addr; >> - u64 size; >> - unsigned int type; >> -}; >> - >> -#define resource_size(res) ((res)->size) >> - >> -#define platform_device device >> - >> -#define IORESOURCE_MEM 0 >> -#define IORESOURCE_IRQ 1 > > You introduce the above code in patch 5, and remove it in patch 6, is > this really needed? > > Ie: why not simply introduce this code directly in this patch? See https://lists.xen.org/archives/html/xen-devel/2018-01/msg02066.html > >> diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c >> index ad956d5b8d..4c04391e21 100644 >> --- a/xen/drivers/passthrough/arm/smmu.c >> +++ b/xen/drivers/passthrough/arm/smmu.c >> @@ -41,6 +41,7 @@ >> #include <xen/irq.h> >> #include <xen/lib.h> >> #include <xen/list.h> >> +#include <xen/linux_compat.h> >> #include <xen/mm.h> >> #include <xen/vmap.h> >> #include <xen/rbtree.h> >> @@ -51,36 +52,13 @@ >> #include <asm/io.h> >> #include <asm/platform.h> >> >> +#include "arm_smmu.h" /* Not a self contained header. So last in the list */ >> /* Xen: The below defines are redefined within the file. Undef it */ >> #undef SCTLR_AFE >> #undef SCTLR_TRE >> #undef SCTLR_M >> #undef TTBCR_EAE >> >> -/* Alias to Xen device tree helpers */ >> -#define device_node dt_device_node >> -#define of_phandle_args dt_phandle_args >> -#define of_device_id dt_device_match >> -#define of_match_node dt_match_node >> -#define of_property_read_u32(np, pname, out) (!dt_property_read_u32(np, pname, out)) >> -#define of_property_read_bool dt_property_read_bool >> -#define of_parse_phandle_with_args dt_parse_phandle_with_args >> - >> -/* Xen: Helpers to get device MMIO and IRQs */ >> -struct resource >> -{ >> - u64 addr; >> - u64 size; >> - unsigned int type; >> -}; >> - >> -#define resource_size(res) (res)->size; >> - >> -#define platform_device device >> - >> -#define IORESOURCE_MEM 0 >> -#define IORESOURCE_IRQ 1 >> - >> static struct resource *platform_get_resource(struct platform_device *pdev, >> unsigned int type, >> unsigned int num) >> @@ -118,58 +96,6 @@ static struct resource *platform_get_resource(struct platform_device *pdev, >> >> /* Xen: Helpers for IRQ functions */ >> #define request_irq(irq, func, flags, name, dev) request_irq(irq, flags, func, name, dev) >> -#define free_irq release_irq >> - >> -enum irqreturn { >> - IRQ_NONE = (0 << 0), >> - IRQ_HANDLED = (1 << 0), >> -}; >> - >> -typedef enum irqreturn irqreturn_t; > > You remove the irqreturn enum without adding any replacement, is this > really unused? It is used, so looks like the SMMU driver has not been build test it. Sameer, please at least build test the changes you made in the SMMU driver. Cheers,
On Fri, Feb 09, 2018 at 10:51:01AM +0000, Julien Grall wrote: > Hi, > > On 02/09/2018 10:43 AM, Roger Pau Monné wrote: > > > + unsigned int type; > > > +}; > > > + > > > +#define resource_size(res) ((res)->size) > > > + > > > +#define platform_device device > > > + > > > +#define IORESOURCE_MEM 0 > > > +#define IORESOURCE_IRQ 1 > > > + > > > +/* Stub out DMA domain related functions */ > > > +#define iommu_get_dma_cookie(dom) 0 > > > +#define iommu_put_dma_cookie(dom) > > > + > > > +#define VA_BITS 0 /* Only used for configuring stage-1 input size */ > > > + > > > +#define MODULE_DEVICE_TABLE(type, name) > > > +#define module_param_named(name, value, type, perm) > > > +#define MODULE_PARM_DESC(_parm, desc) > > > + > > > +#define dma_set_mask_and_coherent(d, b) 0 > > > +#define of_dma_is_coherent(n) 0 > > > + > > > +static void __iomem *devm_ioremap_resource(struct device *dev, > > > + struct resource *res) > > > > Aligment, please use spaces. > > > > Also, is __iomem needed here at all? > > On Arm, we tend to add keep __iomem on pointer dealing with MMIO. I understand that you keep it when directly importing code from Linux, but this is Xen code, so unless this is done merely for consistency it seems quite pointless (__iomem is defined to nothing AFAICT). > > [...] > > > > + > > > +#endif /* __ARM_SMMU_H__ */ > > > + > > > diff --git a/xen/drivers/passthrough/arm/smmu-v3.c b/xen/drivers/passthrough/arm/smmu-v3.c > > > index f43485fe6e..f0a61521fb 100644 > > > --- a/xen/drivers/passthrough/arm/smmu-v3.c > > > +++ b/xen/drivers/passthrough/arm/smmu-v3.c > > > @@ -49,28 +49,7 @@ > > > #include <asm/io.h> > > > #include <asm/platform.h> > > > -/* Alias to Xen device tree helpers */ > > > -#define device_node dt_device_node > > > -#define of_phandle_args dt_phandle_args > > > -#define of_device_id dt_device_match > > > -#define of_match_node dt_match_node > > > -#define of_property_read_u32(np, pname, out) (!dt_property_read_u32(np, pname, out)) > > > -#define of_property_read_bool dt_property_read_bool > > > -#define of_parse_phandle_with_args dt_parse_phandle_with_args > > > - > > > -/* Xen: Helpers to get device MMIO and IRQs */ > > > -struct resource { > > > - u64 addr; > > > - u64 size; > > > - unsigned int type; > > > -}; > > > - > > > -#define resource_size(res) ((res)->size) > > > - > > > -#define platform_device device > > > - > > > -#define IORESOURCE_MEM 0 > > > -#define IORESOURCE_IRQ 1 > > > > You introduce the above code in patch 5, and remove it in patch 6, is > > this really needed? > > > > Ie: why not simply introduce this code directly in this patch? > > See https://lists.xen.org/archives/html/xen-devel/2018-01/msg02066.html Hm, OK, I'm not sure I follow that. AFAICT the above code is added in patch 5 so that the driver can be hooked up into the build system. Could we just hold off hooking the driver to the build system until patch 6, in order to avoid such addition and removal of code? Thanks, Roger.
Hi, On 02/09/2018 11:02 AM, Roger Pau Monné wrote: > On Fri, Feb 09, 2018 at 10:51:01AM +0000, Julien Grall wrote: >> Hi, >> >> On 02/09/2018 10:43 AM, Roger Pau Monné wrote: >>>> + unsigned int type; >>>> +}; >>>> + >>>> +#define resource_size(res) ((res)->size) >>>> + >>>> +#define platform_device device >>>> + >>>> +#define IORESOURCE_MEM 0 >>>> +#define IORESOURCE_IRQ 1 >>>> + >>>> +/* Stub out DMA domain related functions */ >>>> +#define iommu_get_dma_cookie(dom) 0 >>>> +#define iommu_put_dma_cookie(dom) >>>> + >>>> +#define VA_BITS 0 /* Only used for configuring stage-1 input size */ >>>> + >>>> +#define MODULE_DEVICE_TABLE(type, name) >>>> +#define module_param_named(name, value, type, perm) >>>> +#define MODULE_PARM_DESC(_parm, desc) >>>> + >>>> +#define dma_set_mask_and_coherent(d, b) 0 >>>> +#define of_dma_is_coherent(n) 0 >>>> + >>>> +static void __iomem *devm_ioremap_resource(struct device *dev, >>>> + struct resource *res) >>> >>> Aligment, please use spaces. >>> >>> Also, is __iomem needed here at all? >> >> On Arm, we tend to add keep __iomem on pointer dealing with MMIO. > > I understand that you keep it when directly importing code from Linux, > but this is Xen code, so unless this is done merely for consistency it > seems quite pointless (__iomem is defined to nothing AFAICT). We do it quite consistenly in Xen Arm code :). Have a look at the return of ioremap_* or read*/write* parameters. While it might be defined to nothing today, I'd like to keep it because it could easily be defined to check the address space used. > >> >> [...] >> >>>> + >>>> +#endif /* __ARM_SMMU_H__ */ >>>> + >>>> diff --git a/xen/drivers/passthrough/arm/smmu-v3.c b/xen/drivers/passthrough/arm/smmu-v3.c >>>> index f43485fe6e..f0a61521fb 100644 >>>> --- a/xen/drivers/passthrough/arm/smmu-v3.c >>>> +++ b/xen/drivers/passthrough/arm/smmu-v3.c >>>> @@ -49,28 +49,7 @@ >>>> #include <asm/io.h> >>>> #include <asm/platform.h> >>>> -/* Alias to Xen device tree helpers */ >>>> -#define device_node dt_device_node >>>> -#define of_phandle_args dt_phandle_args >>>> -#define of_device_id dt_device_match >>>> -#define of_match_node dt_match_node >>>> -#define of_property_read_u32(np, pname, out) (!dt_property_read_u32(np, pname, out)) >>>> -#define of_property_read_bool dt_property_read_bool >>>> -#define of_parse_phandle_with_args dt_parse_phandle_with_args >>>> - >>>> -/* Xen: Helpers to get device MMIO and IRQs */ >>>> -struct resource { >>>> - u64 addr; >>>> - u64 size; >>>> - unsigned int type; >>>> -}; >>>> - >>>> -#define resource_size(res) ((res)->size) >>>> - >>>> -#define platform_device device >>>> - >>>> -#define IORESOURCE_MEM 0 >>>> -#define IORESOURCE_IRQ 1 >>> >>> You introduce the above code in patch 5, and remove it in patch 6, is >>> this really needed? >>> >>> Ie: why not simply introduce this code directly in this patch? >> >> See https://lists.xen.org/archives/html/xen-devel/2018-01/msg02066.html > > Hm, OK, I'm not sure I follow that. > > AFAICT the above code is added in patch 5 so that the driver can be > hooked up into the build system. Could we just hold off hooking the > driver to the build system until patch 6, in order to avoid such > addition and removal of code? What matters is to know what is common between SMMUv2 and SMMUv3 driver. So it can be pulled in a separate headers. IHMO, the both yours and his way are valid. TBH, I would have done a third way and move that patch before #5. But at this stage, this is a matter of taste, hence way I didn't push to reshuffle the series. Cheers,
On 2/9/2018 3:51 AM, Julien Grall wrote: > >> >>> diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c >>> index ad956d5b8d..4c04391e21 100644 >>> --- a/xen/drivers/passthrough/arm/smmu.c >>> +++ b/xen/drivers/passthrough/arm/smmu.c >>> @@ -41,6 +41,7 @@ >>> #include <xen/irq.h> >>> #include <xen/lib.h> >>> #include <xen/list.h> >>> +#include <xen/linux_compat.h> The above header included for the first time. >>> #include <xen/mm.h> >>> #include <xen/vmap.h> >>> #include <xen/rbtree.h> >>> @@ -51,36 +52,13 @@ >>> #include <asm/io.h> >>> #include <asm/platform.h> >>> +#include "arm_smmu.h" /* Not a self contained header. So last in the list */ >>> /* Xen: The below defines are redefined within the file. Undef it */ >>> #undef SCTLR_AFE >>> #undef SCTLR_TRE >>> #undef SCTLR_M >>> #undef TTBCR_EAE >>> -/* Alias to Xen device tree helpers */ >>> -#define device_node dt_device_node >>> -#define of_phandle_args dt_phandle_args >>> -#define of_device_id dt_device_match >>> -#define of_match_node dt_match_node >>> -#define of_property_read_u32(np, pname, out) (!dt_property_read_u32(np, pname, out)) >>> -#define of_property_read_bool dt_property_read_bool >>> -#define of_parse_phandle_with_args dt_parse_phandle_with_args >>> - >>> -/* Xen: Helpers to get device MMIO and IRQs */ >>> -struct resource >>> -{ >>> - u64 addr; >>> - u64 size; >>> - unsigned int type; >>> -}; >>> - >>> -#define resource_size(res) (res)->size; >>> - >>> -#define platform_device device >>> - >>> -#define IORESOURCE_MEM 0 >>> -#define IORESOURCE_IRQ 1 >>> - >>> static struct resource *platform_get_resource(struct platform_device *pdev, >>> unsigned int type, >>> unsigned int num) >>> @@ -118,58 +96,6 @@ static struct resource *platform_get_resource(struct platform_device *pdev, >>> /* Xen: Helpers for IRQ functions */ >>> #define request_irq(irq, func, flags, name, dev) request_irq(irq, flags, func, name, dev) >>> -#define free_irq release_irq >>> - >>> -enum irqreturn { >>> - IRQ_NONE = (0 << 0), >>> - IRQ_HANDLED = (1 << 0), >>> -}; >>> - >>> -typedef enum irqreturn irqreturn_t; >> >> You remove the irqreturn enum without adding any replacement, is this >> really unused? > > It is used, so looks like the SMMU driver has not been build test it. Sameer, please at least build test the changes you made in the SMMU driver. It is build tested. The above defined now come from linux_compat.h. I introduced this with the smmu-v3 code changes as recommended by Roger on the RFC. > > Cheers, > Thanks, Sameer
Hi, On 09/02/18 03:10, Sameer Goel wrote: > Pull common defines for SMMU drives in a local header. s/drivers/ > > Signed-off-by: Sameer Goel <sameer.goel@linaro.org> > --- > xen/drivers/passthrough/arm/arm_smmu.h | 125 +++++++++++++++++++++++++++++++++ > xen/drivers/passthrough/arm/smmu-v3.c | 96 +------------------------ > xen/drivers/passthrough/arm/smmu.c | 104 +-------------------------- > 3 files changed, 128 insertions(+), 197 deletions(-) > create mode 100644 xen/drivers/passthrough/arm/arm_smmu.h > > diff --git a/xen/drivers/passthrough/arm/arm_smmu.h b/xen/drivers/passthrough/arm/arm_smmu.h > new file mode 100644 > index 0000000000..f49dceb5b4 > --- /dev/null > +++ b/xen/drivers/passthrough/arm/arm_smmu.h > @@ -0,0 +1,125 @@ > +/****************************************************************************** > + * ./arm_smmu.h xen/drivers/passthrough/arm/arm_smmu.h > + * > + * Common compatibility defines and data_structures for porting arm smmu > + * drivers from Linux. > + * > + * Copyright (c) 2017 Linaro Limited > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * 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/>. > + */ > + > +#ifndef __ARM_SMMU_H__ > +#define __ARM_SMMU_H__ > + > + No need for 2 newline > +/* Alias to Xen device tree helpers */ > +#define device_node dt_device_node > +#define of_phandle_args dt_phandle_args > +#define of_device_id dt_device_match > +#define of_match_node dt_match_node > +#define of_property_read_u32(np, pname, out) (!dt_property_read_u32(np, pname, out)) > +#define of_property_read_bool dt_property_read_bool > +#define of_parse_phandle_with_args dt_parse_phandle_with_args > + > +/* Helpers to get device MMIO and IRQs */ > +struct resource { > + u64 addr; > + u64 size; Please take the oppoturnity to switch to uint64_t. > + unsigned int type; > +}; > + > +#define resource_size(res) ((res)->size) > + > +#define platform_device device > + > +#define IORESOURCE_MEM 0 > +#define IORESOURCE_IRQ 1 > + > +/* Stub out DMA domain related functions */ > +#define iommu_get_dma_cookie(dom) 0 > +#define iommu_put_dma_cookie(dom) Please avoid to implement empty macro. Use do { } while (0) > + > +#define VA_BITS 0 /* Only used for configuring stage-1 input size */ This seems to only be dropped in the SMMUv2 driver. > + > +#define MODULE_DEVICE_TABLE(type, name) > +#define module_param_named(name, value, type, perm) > +#define MODULE_PARM_DESC(_parm, desc) Should not this belong to the linux-compat.h? > + > +#define dma_set_mask_and_coherent(d, b) 0 > +#define of_dma_is_coherent(n) 0 Those 2 don't exist in the SMMUv2 driver. > + > +static void __iomem *devm_ioremap_resource(struct device *dev, > + struct resource *res) > +{ > + void __iomem *ptr; > + > + if ( !res || res->type != IORESOURCE_MEM ) > + { > + dev_err(dev, "Invalid resource\n"); > + return ERR_PTR(-EINVAL); > + } > + > + ptr = ioremap_nocache(res->addr, res->size); > + if ( !ptr ) > + { > + dev_err(dev, "ioremap failed (addr 0x%"PRIx64" size 0x%"PRIx64")\n", > + res->addr, res->size); > + return ERR_PTR(-ENOMEM); > + } > + > + return ptr; > +} > + > +/* > + * Domain type definitions. Not really needed for Xen, defining to port > + * Linux code as-is > + */ > +#define IOMMU_DOMAIN_UNMANAGED 0 > +#define IOMMU_DOMAIN_DMA 1 > +#define IOMMU_DOMAIN_IDENTITY 2 > + > +/* Xen: Compatibility define for iommu_domain_geometry.*/ > +struct iommu_domain_geometry { > + dma_addr_t aperture_start; /* First address that can be mapped */ > + dma_addr_t aperture_end; /* Last address that can be mapped */ > + bool force_aperture; /* DMA only allowed in mappable range? */ > +}; > + > +/* Xen: Dummy iommu_domain */ > +struct iommu_domain { > + /* Runtime SMMU configuration for this iommu_domain */ > + struct arm_smmu_domain *priv; > + unsigned int type; > + > + /* Dummy compatibility defines */ > + unsigned long pgsize_bitmap; > + struct iommu_domain_geometry geometry; > + > + atomic_t ref; > + /* Used to link iommu_domain contexts for a same domain. > + * There is at least one per-SMMU to used by the domain. > + */ > + struct list_head list; > +}; > + > +/* Xen: Describes information required for a Xen domain */ > +struct arm_smmu_xen_domain { > + spinlock_t lock; > + /* List of iommu domains associated to this domain */ > + struct list_head contexts; > +}; > + > +#endif /* __ARM_SMMU_H__ */ > + > diff --git a/xen/drivers/passthrough/arm/smmu-v3.c b/xen/drivers/passthrough/arm/smmu-v3.c > index f43485fe6e..f0a61521fb 100644 > --- a/xen/drivers/passthrough/arm/smmu-v3.c > +++ b/xen/drivers/passthrough/arm/smmu-v3.c > @@ -49,28 +49,7 @@ > #include <asm/io.h> > #include <asm/platform.h> > > -/* Alias to Xen device tree helpers */ > -#define device_node dt_device_node > -#define of_phandle_args dt_phandle_args > -#define of_device_id dt_device_match > -#define of_match_node dt_match_node > -#define of_property_read_u32(np, pname, out) (!dt_property_read_u32(np, pname, out)) > -#define of_property_read_bool dt_property_read_bool > -#define of_parse_phandle_with_args dt_parse_phandle_with_args > - > -/* Xen: Helpers to get device MMIO and IRQs */ > -struct resource { > - u64 addr; > - u64 size; > - unsigned int type; > -}; > - > -#define resource_size(res) ((res)->size) > - > -#define platform_device device > - > -#define IORESOURCE_MEM 0 > -#define IORESOURCE_IRQ 1 > +#include "arm_smmu.h" > > static struct resource *platform_get_resource(struct platform_device *pdev, > unsigned int type, > @@ -200,79 +179,6 @@ static void dmam_free_coherent(struct device *dev, size_t size, void *vaddr, > xfree(vaddr); > } > > -/* Xen: Stub out DMA domain related functions */ > -#define iommu_get_dma_cookie(dom) 0 > -#define iommu_put_dma_cookie(dom) > - > -/* Xen: Stub out module param related function */ > -#define module_param_named(a, b, c, d) > -#define MODULE_PARM_DESC(a, b) > - > -#define dma_set_mask_and_coherent(d, b) 0 > - > -#define of_dma_is_coherent(n) 0 > - > -#define MODULE_DEVICE_TABLE(type, name) > - > -static void __iomem *devm_ioremap_resource(struct device *dev, > - struct resource *res) > -{ > - void __iomem *ptr; > - > - if (!res || res->type != IORESOURCE_MEM) { > - dev_err(dev, "Invalid resource\n"); > - return ERR_PTR(-EINVAL); > - } > - > - ptr = ioremap_nocache(res->addr, res->size); > - if (!ptr) { > - dev_err(dev, > - "ioremap failed (addr 0x%"PRIx64" size 0x%"PRIx64")\n", > - res->addr, res->size); > - return ERR_PTR(-ENOMEM); > - } > - > - return ptr; > -} > - > -/* Xen: Compatibility define for iommu_domain_geometry.*/ > -struct iommu_domain_geometry { > - dma_addr_t aperture_start; /* First address that can be mapped */ > - dma_addr_t aperture_end; /* Last address that can be mapped */ > - bool force_aperture; /* DMA only allowed in mappable range? */ > -}; > - > - > -/* Xen: Type definitions for iommu_domain */ > -#define IOMMU_DOMAIN_UNMANAGED 0 > -#define IOMMU_DOMAIN_DMA 1 > -#define IOMMU_DOMAIN_IDENTITY 2 > - > -/* Xen: Dummy iommu_domain */ > -struct iommu_domain { > - /* Runtime SMMU configuration for this iommu_domain */ > - struct arm_smmu_domain *priv; > - unsigned int type; > - > - /* Dummy compatibility defines */ > - unsigned long pgsize_bitmap; > - struct iommu_domain_geometry geometry; > - > - atomic_t ref; > - /* > - * Used to link iommu_domain contexts for a same domain. > - * There is at least one per-SMMU to used by the domain. > - */ > - struct list_head list; > -}; > - > -/* Xen: Describes information required for a Xen domain */ > -struct arm_smmu_xen_domain { > - spinlock_t lock; > - /* List of iommu domains associated to this domain */ > - struct list_head contexts; > -}; > - > /* > * Xen: Information about each device stored in dev->archdata.iommu > * > diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c > index ad956d5b8d..4c04391e21 100644 > --- a/xen/drivers/passthrough/arm/smmu.c > +++ b/xen/drivers/passthrough/arm/smmu.c > @@ -41,6 +41,7 @@ > #include <xen/irq.h> > #include <xen/lib.h> > #include <xen/list.h> > +#include <xen/linux_compat.h> I don't think this refactoring belongs to this patch. > #include <xen/mm.h> > #include <xen/vmap.h> > #include <xen/rbtree.h> > @@ -51,36 +52,13 @@ > #include <asm/io.h> > #include <asm/platform.h> > > +#include "arm_smmu.h" /* Not a self contained header. So last in the list */ > /* Xen: The below defines are redefined within the file. Undef it */ > #undef SCTLR_AFE > #undef SCTLR_TRE > #undef SCTLR_M > #undef TTBCR_EAE > > -/* Alias to Xen device tree helpers */ > -#define device_node dt_device_node > -#define of_phandle_args dt_phandle_args > -#define of_device_id dt_device_match > -#define of_match_node dt_match_node > -#define of_property_read_u32(np, pname, out) (!dt_property_read_u32(np, pname, out)) > -#define of_property_read_bool dt_property_read_bool > -#define of_parse_phandle_with_args dt_parse_phandle_with_args > - > -/* Xen: Helpers to get device MMIO and IRQs */ > -struct resource > -{ > - u64 addr; > - u64 size; > - unsigned int type; > -}; > - > -#define resource_size(res) (res)->size; > - > -#define platform_device device > - > -#define IORESOURCE_MEM 0 > -#define IORESOURCE_IRQ 1 > - > static struct resource *platform_get_resource(struct platform_device *pdev, > unsigned int type, > unsigned int num) > @@ -118,58 +96,6 @@ static struct resource *platform_get_resource(struct platform_device *pdev, > > /* Xen: Helpers for IRQ functions */ > #define request_irq(irq, func, flags, name, dev) request_irq(irq, flags, func, name, dev) > -#define free_irq release_irq > - > -enum irqreturn { > - IRQ_NONE = (0 << 0), > - IRQ_HANDLED = (1 << 0), > -}; > - > -typedef enum irqreturn irqreturn_t; > - > -/* Device logger functions > - * TODO: Handle PCI > - */ > -#define dev_print(dev, lvl, fmt, ...) \ > - printk(lvl "smmu: %s: " fmt, dt_node_full_name(dev_to_dt(dev)), ## __VA_ARGS__) > - > -#define dev_dbg(dev, fmt, ...) dev_print(dev, XENLOG_DEBUG, fmt, ## __VA_ARGS__) > -#define dev_notice(dev, fmt, ...) dev_print(dev, XENLOG_INFO, fmt, ## __VA_ARGS__) > -#define dev_warn(dev, fmt, ...) dev_print(dev, XENLOG_WARNING, fmt, ## __VA_ARGS__) > -#define dev_err(dev, fmt, ...) dev_print(dev, XENLOG_ERR, fmt, ## __VA_ARGS__) > - > -#define dev_err_ratelimited(dev, fmt, ...) \ > - dev_print(dev, XENLOG_ERR, fmt, ## __VA_ARGS__) > - > -#define dev_name(dev) dt_node_full_name(dev_to_dt(dev)) > - > -/* Alias to Xen allocation helpers */ > -#define kfree xfree > -#define kmalloc(size, flags) _xmalloc(size, sizeof(void *)) > -#define kzalloc(size, flags) _xzalloc(size, sizeof(void *)) > -#define devm_kzalloc(dev, size, flags) _xzalloc(size, sizeof(void *)) > -#define kmalloc_array(size, n, flags) _xmalloc_array(size, sizeof(void *), n) > - > -static void __iomem *devm_ioremap_resource(struct device *dev, > - struct resource *res) > -{ > - void __iomem *ptr; > - > - if (!res || res->type != IORESOURCE_MEM) { > - dev_err(dev, "Invalid resource\n"); > - return ERR_PTR(-EINVAL); > - } > - > - ptr = ioremap_nocache(res->addr, res->size); > - if (!ptr) { > - dev_err(dev, > - "ioremap failed (addr 0x%"PRIx64" size 0x%"PRIx64")\n", > - res->addr, res->size); > - return ERR_PTR(-ENOMEM); > - } > - > - return ptr; > -} > > /* Xen doesn't handle IOMMU fault */ > #define report_iommu_fault(...) 1 > @@ -196,32 +122,6 @@ static inline int pci_for_each_dma_alias(struct pci_dev *pdev, > #define PHYS_MASK_SHIFT PADDR_BITS > typedef paddr_t phys_addr_t; > > -#define VA_BITS 0 /* Only used for configuring stage-1 input size */ > - > -#define MODULE_DEVICE_TABLE(type, name) > -#define module_param_named(name, value, type, perm) > -#define MODULE_PARM_DESC(_parm, desc) > - > -/* Xen: Dummy iommu_domain */ > -struct iommu_domain > -{ > - /* Runtime SMMU configuration for this iommu_domain */ > - struct arm_smmu_domain *priv; > - > - atomic_t ref; > - /* Used to link iommu_domain contexts for a same domain. > - * There is at least one per-SMMU to used by the domain. > - * */ > - struct list_head list; > -}; > - > -/* Xen: Describes informations required for a Xen domain */ > -struct arm_smmu_xen_domain { > - spinlock_t lock; > - /* List of context (i.e iommu_domain) associated to this domain */ > - struct list_head contexts; > -}; > - > /* > * Xen: Information about each device stored in dev->archdata.iommu > * > Cheers,
On 2/9/2018 4:02 AM, Roger Pau Monné wrote: > On Fri, Feb 09, 2018 at 10:51:01AM +0000, Julien Grall wrote: >> Hi, >> >> On 02/09/2018 10:43 AM, Roger Pau Monné wrote: >>>> + unsigned int type; >>>> +}; >>>> + >>>> +#define resource_size(res) ((res)->size) >>>> + >>>> +#define platform_device device >>>> + >>>> +#define IORESOURCE_MEM 0 >>>> +#define IORESOURCE_IRQ 1 >>>> + >>>> +/* Stub out DMA domain related functions */ >>>> +#define iommu_get_dma_cookie(dom) 0 >>>> +#define iommu_put_dma_cookie(dom) >>>> + >>>> +#define VA_BITS 0 /* Only used for configuring stage-1 input size */ >>>> + >>>> +#define MODULE_DEVICE_TABLE(type, name) >>>> +#define module_param_named(name, value, type, perm) >>>> +#define MODULE_PARM_DESC(_parm, desc) >>>> + >>>> +#define dma_set_mask_and_coherent(d, b) 0 >>>> +#define of_dma_is_coherent(n) 0 >>>> + >>>> +static void __iomem *devm_ioremap_resource(struct device *dev, >>>> + struct resource *res) >>> Aligment, please use spaces. >>> >>> Also, is __iomem needed here at all? >> On Arm, we tend to add keep __iomem on pointer dealing with MMIO. > I understand that you keep it when directly importing code from Linux, > but this is Xen code, so unless this is done merely for consistency it > seems quite pointless (__iomem is defined to nothing AFAICT). > >> [...] >> >>>> + >>>> +#endif /* __ARM_SMMU_H__ */ >>>> + >>>> diff --git a/xen/drivers/passthrough/arm/smmu-v3.c b/xen/drivers/passthrough/arm/smmu-v3.c >>>> index f43485fe6e..f0a61521fb 100644 >>>> --- a/xen/drivers/passthrough/arm/smmu-v3.c >>>> +++ b/xen/drivers/passthrough/arm/smmu-v3.c >>>> @@ -49,28 +49,7 @@ >>>> #include <asm/io.h> >>>> #include <asm/platform.h> >>>> -/* Alias to Xen device tree helpers */ >>>> -#define device_node dt_device_node >>>> -#define of_phandle_args dt_phandle_args >>>> -#define of_device_id dt_device_match >>>> -#define of_match_node dt_match_node >>>> -#define of_property_read_u32(np, pname, out) (!dt_property_read_u32(np, pname, out)) >>>> -#define of_property_read_bool dt_property_read_bool >>>> -#define of_parse_phandle_with_args dt_parse_phandle_with_args >>>> - >>>> -/* Xen: Helpers to get device MMIO and IRQs */ >>>> -struct resource { >>>> - u64 addr; >>>> - u64 size; >>>> - unsigned int type; >>>> -}; >>>> - >>>> -#define resource_size(res) ((res)->size) >>>> - >>>> -#define platform_device device >>>> - >>>> -#define IORESOURCE_MEM 0 >>>> -#define IORESOURCE_IRQ 1 >>> You introduce the above code in patch 5, and remove it in patch 6, is >>> this really needed? >>> >>> Ie: why not simply introduce this code directly in this patch? >> See https://lists.xen.org/archives/html/xen-devel/2018-01/msg02066.html > Hm, OK, I'm not sure I follow that. > > AFAICT the above code is added in patch 5 so that the driver can be > hooked up into the build system. Could we just hold off hooking the > driver to the build system until patch 6, in order to avoid such > addition and removal of code? I just wanted this patch to be the unifying change between the SMMUv2 and SMMUv3 jargon. This allows me to keep some variable names as is from Linux kernel for the first checkin. I agree that I can shuffle around some variables but since I was introducing this patch I refrained from it. > > Thanks, Roger. > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xenproject.org > https://lists.xenproject.org/mailman/listinfo/xen-devel
diff --git a/xen/drivers/passthrough/arm/arm_smmu.h b/xen/drivers/passthrough/arm/arm_smmu.h new file mode 100644 index 0000000000..f49dceb5b4 --- /dev/null +++ b/xen/drivers/passthrough/arm/arm_smmu.h @@ -0,0 +1,125 @@ +/****************************************************************************** + * ./arm_smmu.h + * + * Common compatibility defines and data_structures for porting arm smmu + * drivers from Linux. + * + * Copyright (c) 2017 Linaro Limited + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * 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/>. + */ + +#ifndef __ARM_SMMU_H__ +#define __ARM_SMMU_H__ + + +/* Alias to Xen device tree helpers */ +#define device_node dt_device_node +#define of_phandle_args dt_phandle_args +#define of_device_id dt_device_match +#define of_match_node dt_match_node +#define of_property_read_u32(np, pname, out) (!dt_property_read_u32(np, pname, out)) +#define of_property_read_bool dt_property_read_bool +#define of_parse_phandle_with_args dt_parse_phandle_with_args + +/* Helpers to get device MMIO and IRQs */ +struct resource { + u64 addr; + u64 size; + unsigned int type; +}; + +#define resource_size(res) ((res)->size) + +#define platform_device device + +#define IORESOURCE_MEM 0 +#define IORESOURCE_IRQ 1 + +/* Stub out DMA domain related functions */ +#define iommu_get_dma_cookie(dom) 0 +#define iommu_put_dma_cookie(dom) + +#define VA_BITS 0 /* Only used for configuring stage-1 input size */ + +#define MODULE_DEVICE_TABLE(type, name) +#define module_param_named(name, value, type, perm) +#define MODULE_PARM_DESC(_parm, desc) + +#define dma_set_mask_and_coherent(d, b) 0 +#define of_dma_is_coherent(n) 0 + +static void __iomem *devm_ioremap_resource(struct device *dev, + struct resource *res) +{ + void __iomem *ptr; + + if ( !res || res->type != IORESOURCE_MEM ) + { + dev_err(dev, "Invalid resource\n"); + return ERR_PTR(-EINVAL); + } + + ptr = ioremap_nocache(res->addr, res->size); + if ( !ptr ) + { + dev_err(dev, "ioremap failed (addr 0x%"PRIx64" size 0x%"PRIx64")\n", + res->addr, res->size); + return ERR_PTR(-ENOMEM); + } + + return ptr; +} + +/* + * Domain type definitions. Not really needed for Xen, defining to port + * Linux code as-is + */ +#define IOMMU_DOMAIN_UNMANAGED 0 +#define IOMMU_DOMAIN_DMA 1 +#define IOMMU_DOMAIN_IDENTITY 2 + +/* Xen: Compatibility define for iommu_domain_geometry.*/ +struct iommu_domain_geometry { + dma_addr_t aperture_start; /* First address that can be mapped */ + dma_addr_t aperture_end; /* Last address that can be mapped */ + bool force_aperture; /* DMA only allowed in mappable range? */ +}; + +/* Xen: Dummy iommu_domain */ +struct iommu_domain { + /* Runtime SMMU configuration for this iommu_domain */ + struct arm_smmu_domain *priv; + unsigned int type; + + /* Dummy compatibility defines */ + unsigned long pgsize_bitmap; + struct iommu_domain_geometry geometry; + + atomic_t ref; + /* Used to link iommu_domain contexts for a same domain. + * There is at least one per-SMMU to used by the domain. + */ + struct list_head list; +}; + +/* Xen: Describes information required for a Xen domain */ +struct arm_smmu_xen_domain { + spinlock_t lock; + /* List of iommu domains associated to this domain */ + struct list_head contexts; +}; + +#endif /* __ARM_SMMU_H__ */ + diff --git a/xen/drivers/passthrough/arm/smmu-v3.c b/xen/drivers/passthrough/arm/smmu-v3.c index f43485fe6e..f0a61521fb 100644 --- a/xen/drivers/passthrough/arm/smmu-v3.c +++ b/xen/drivers/passthrough/arm/smmu-v3.c @@ -49,28 +49,7 @@ #include <asm/io.h> #include <asm/platform.h> -/* Alias to Xen device tree helpers */ -#define device_node dt_device_node -#define of_phandle_args dt_phandle_args -#define of_device_id dt_device_match -#define of_match_node dt_match_node -#define of_property_read_u32(np, pname, out) (!dt_property_read_u32(np, pname, out)) -#define of_property_read_bool dt_property_read_bool -#define of_parse_phandle_with_args dt_parse_phandle_with_args - -/* Xen: Helpers to get device MMIO and IRQs */ -struct resource { - u64 addr; - u64 size; - unsigned int type; -}; - -#define resource_size(res) ((res)->size) - -#define platform_device device - -#define IORESOURCE_MEM 0 -#define IORESOURCE_IRQ 1 +#include "arm_smmu.h" static struct resource *platform_get_resource(struct platform_device *pdev, unsigned int type, @@ -200,79 +179,6 @@ static void dmam_free_coherent(struct device *dev, size_t size, void *vaddr, xfree(vaddr); } -/* Xen: Stub out DMA domain related functions */ -#define iommu_get_dma_cookie(dom) 0 -#define iommu_put_dma_cookie(dom) - -/* Xen: Stub out module param related function */ -#define module_param_named(a, b, c, d) -#define MODULE_PARM_DESC(a, b) - -#define dma_set_mask_and_coherent(d, b) 0 - -#define of_dma_is_coherent(n) 0 - -#define MODULE_DEVICE_TABLE(type, name) - -static void __iomem *devm_ioremap_resource(struct device *dev, - struct resource *res) -{ - void __iomem *ptr; - - if (!res || res->type != IORESOURCE_MEM) { - dev_err(dev, "Invalid resource\n"); - return ERR_PTR(-EINVAL); - } - - ptr = ioremap_nocache(res->addr, res->size); - if (!ptr) { - dev_err(dev, - "ioremap failed (addr 0x%"PRIx64" size 0x%"PRIx64")\n", - res->addr, res->size); - return ERR_PTR(-ENOMEM); - } - - return ptr; -} - -/* Xen: Compatibility define for iommu_domain_geometry.*/ -struct iommu_domain_geometry { - dma_addr_t aperture_start; /* First address that can be mapped */ - dma_addr_t aperture_end; /* Last address that can be mapped */ - bool force_aperture; /* DMA only allowed in mappable range? */ -}; - - -/* Xen: Type definitions for iommu_domain */ -#define IOMMU_DOMAIN_UNMANAGED 0 -#define IOMMU_DOMAIN_DMA 1 -#define IOMMU_DOMAIN_IDENTITY 2 - -/* Xen: Dummy iommu_domain */ -struct iommu_domain { - /* Runtime SMMU configuration for this iommu_domain */ - struct arm_smmu_domain *priv; - unsigned int type; - - /* Dummy compatibility defines */ - unsigned long pgsize_bitmap; - struct iommu_domain_geometry geometry; - - atomic_t ref; - /* - * Used to link iommu_domain contexts for a same domain. - * There is at least one per-SMMU to used by the domain. - */ - struct list_head list; -}; - -/* Xen: Describes information required for a Xen domain */ -struct arm_smmu_xen_domain { - spinlock_t lock; - /* List of iommu domains associated to this domain */ - struct list_head contexts; -}; - /* * Xen: Information about each device stored in dev->archdata.iommu * diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c index ad956d5b8d..4c04391e21 100644 --- a/xen/drivers/passthrough/arm/smmu.c +++ b/xen/drivers/passthrough/arm/smmu.c @@ -41,6 +41,7 @@ #include <xen/irq.h> #include <xen/lib.h> #include <xen/list.h> +#include <xen/linux_compat.h> #include <xen/mm.h> #include <xen/vmap.h> #include <xen/rbtree.h> @@ -51,36 +52,13 @@ #include <asm/io.h> #include <asm/platform.h> +#include "arm_smmu.h" /* Not a self contained header. So last in the list */ /* Xen: The below defines are redefined within the file. Undef it */ #undef SCTLR_AFE #undef SCTLR_TRE #undef SCTLR_M #undef TTBCR_EAE -/* Alias to Xen device tree helpers */ -#define device_node dt_device_node -#define of_phandle_args dt_phandle_args -#define of_device_id dt_device_match -#define of_match_node dt_match_node -#define of_property_read_u32(np, pname, out) (!dt_property_read_u32(np, pname, out)) -#define of_property_read_bool dt_property_read_bool -#define of_parse_phandle_with_args dt_parse_phandle_with_args - -/* Xen: Helpers to get device MMIO and IRQs */ -struct resource -{ - u64 addr; - u64 size; - unsigned int type; -}; - -#define resource_size(res) (res)->size; - -#define platform_device device - -#define IORESOURCE_MEM 0 -#define IORESOURCE_IRQ 1 - static struct resource *platform_get_resource(struct platform_device *pdev, unsigned int type, unsigned int num) @@ -118,58 +96,6 @@ static struct resource *platform_get_resource(struct platform_device *pdev, /* Xen: Helpers for IRQ functions */ #define request_irq(irq, func, flags, name, dev) request_irq(irq, flags, func, name, dev) -#define free_irq release_irq - -enum irqreturn { - IRQ_NONE = (0 << 0), - IRQ_HANDLED = (1 << 0), -}; - -typedef enum irqreturn irqreturn_t; - -/* Device logger functions - * TODO: Handle PCI - */ -#define dev_print(dev, lvl, fmt, ...) \ - printk(lvl "smmu: %s: " fmt, dt_node_full_name(dev_to_dt(dev)), ## __VA_ARGS__) - -#define dev_dbg(dev, fmt, ...) dev_print(dev, XENLOG_DEBUG, fmt, ## __VA_ARGS__) -#define dev_notice(dev, fmt, ...) dev_print(dev, XENLOG_INFO, fmt, ## __VA_ARGS__) -#define dev_warn(dev, fmt, ...) dev_print(dev, XENLOG_WARNING, fmt, ## __VA_ARGS__) -#define dev_err(dev, fmt, ...) dev_print(dev, XENLOG_ERR, fmt, ## __VA_ARGS__) - -#define dev_err_ratelimited(dev, fmt, ...) \ - dev_print(dev, XENLOG_ERR, fmt, ## __VA_ARGS__) - -#define dev_name(dev) dt_node_full_name(dev_to_dt(dev)) - -/* Alias to Xen allocation helpers */ -#define kfree xfree -#define kmalloc(size, flags) _xmalloc(size, sizeof(void *)) -#define kzalloc(size, flags) _xzalloc(size, sizeof(void *)) -#define devm_kzalloc(dev, size, flags) _xzalloc(size, sizeof(void *)) -#define kmalloc_array(size, n, flags) _xmalloc_array(size, sizeof(void *), n) - -static void __iomem *devm_ioremap_resource(struct device *dev, - struct resource *res) -{ - void __iomem *ptr; - - if (!res || res->type != IORESOURCE_MEM) { - dev_err(dev, "Invalid resource\n"); - return ERR_PTR(-EINVAL); - } - - ptr = ioremap_nocache(res->addr, res->size); - if (!ptr) { - dev_err(dev, - "ioremap failed (addr 0x%"PRIx64" size 0x%"PRIx64")\n", - res->addr, res->size); - return ERR_PTR(-ENOMEM); - } - - return ptr; -} /* Xen doesn't handle IOMMU fault */ #define report_iommu_fault(...) 1 @@ -196,32 +122,6 @@ static inline int pci_for_each_dma_alias(struct pci_dev *pdev, #define PHYS_MASK_SHIFT PADDR_BITS typedef paddr_t phys_addr_t; -#define VA_BITS 0 /* Only used for configuring stage-1 input size */ - -#define MODULE_DEVICE_TABLE(type, name) -#define module_param_named(name, value, type, perm) -#define MODULE_PARM_DESC(_parm, desc) - -/* Xen: Dummy iommu_domain */ -struct iommu_domain -{ - /* Runtime SMMU configuration for this iommu_domain */ - struct arm_smmu_domain *priv; - - atomic_t ref; - /* Used to link iommu_domain contexts for a same domain. - * There is at least one per-SMMU to used by the domain. - * */ - struct list_head list; -}; - -/* Xen: Describes informations required for a Xen domain */ -struct arm_smmu_xen_domain { - spinlock_t lock; - /* List of context (i.e iommu_domain) associated to this domain */ - struct list_head contexts; -}; - /* * Xen: Information about each device stored in dev->archdata.iommu *
Pull common defines for SMMU drives in a local header. Signed-off-by: Sameer Goel <sameer.goel@linaro.org> --- xen/drivers/passthrough/arm/arm_smmu.h | 125 +++++++++++++++++++++++++++++++++ xen/drivers/passthrough/arm/smmu-v3.c | 96 +------------------------ xen/drivers/passthrough/arm/smmu.c | 104 +-------------------------- 3 files changed, 128 insertions(+), 197 deletions(-) create mode 100644 xen/drivers/passthrough/arm/arm_smmu.h