| Message ID | 20171219031703.23420-4-sameer.goel@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | SMMUv3 driver | expand |
On Mon, Dec 18, 2017 at 08:16:58PM -0700, Sameer Goel wrote: > For porting files directly from Linux it is useful to have a function mapping > definitions from Linux to Xen. This file adds common API functions and > other defines that are needed for porting arm SMMU drivers. > > Signed-off-by: Sameer Goel <sameer.goel@linaro.org> > --- > xen/include/xen/linux_compat.h | 81 ++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 81 insertions(+) > create mode 100644 xen/include/xen/linux_compat.h > > diff --git a/xen/include/xen/linux_compat.h b/xen/include/xen/linux_compat.h > new file mode 100644 > index 0000000000..174d0390e5 > --- /dev/null > +++ b/xen/include/xen/linux_compat.h > @@ -0,0 +1,81 @@ > +/****************************************************************************** > + * include/xen/linux_compat.h > + * > + * Compatibility defines for porting code from Linux to Xen > + * > + * 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 __XEN_LINUX_COMPAT_H__ > +#define __XEN_LINUX_COMPAT_H__ > + > +#include <asm/types.h> > + > +typedef paddr_t phys_addr_t; > +typedef paddr_t dma_addr_t; > + > +typedef unsigned int gfp_t; > +#define GFP_KERNEL 0 > +#define __GFP_ZERO 0 You might want to think carefully about this one. I haven't checked if Xen supports such flag, but it is probably a bad idea to fool the driver that it gets a zeroed allocation. > + > +/* 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 for IRQ functions */ > +#define free_irq release_irq > + > +enum irqreturn { > + IRQ_NONE = (0 << 0), > + IRQ_HANDLED = (1 << 0), > + IRQ_WAKE_THREAD = (2 << 0), Use 1U 2U please. > +}; > + > +typedef enum irqreturn irqreturn_t; > + > +/* Device logger functions */ > +#define dev_print(dev, lvl, fmt, ...) \ > + printk(lvl fmt, ## __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_info(dev, fmt, ...) dev_print(dev, XENLOG_INFO, fmt, ## __VA_ARGS__) > + > +#define dev_err_ratelimited(dev, fmt, ...) \ > + dev_print(dev, XENLOG_ERR, fmt, ## __VA_ARGS__) > + These are going to clash with the ones in drivers/passthrough/arm/smmu.c. Wei.
On Mon, Dec 18, 2017 at 08:16:58PM -0700, Sameer Goel wrote: > For porting files directly from Linux it is useful to have a function mapping > definitions from Linux to Xen. This file adds common API functions and > other defines that are needed for porting arm SMMU drivers. It would be good that you add things to linux_compat.h in the same patch that you import the code, or else reviewing whether this is needed or not is impossible. > Signed-off-by: Sameer Goel <sameer.goel@linaro.org> > --- > xen/include/xen/linux_compat.h | 81 ++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 81 insertions(+) > create mode 100644 xen/include/xen/linux_compat.h > > diff --git a/xen/include/xen/linux_compat.h b/xen/include/xen/linux_compat.h > new file mode 100644 > index 0000000000..174d0390e5 > --- /dev/null > +++ b/xen/include/xen/linux_compat.h > @@ -0,0 +1,81 @@ > +/****************************************************************************** > + * include/xen/linux_compat.h > + * > + * Compatibility defines for porting code from Linux to Xen > + * > + * 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 __XEN_LINUX_COMPAT_H__ > +#define __XEN_LINUX_COMPAT_H__ > + > +#include <asm/types.h> > + > +typedef paddr_t phys_addr_t; > +typedef paddr_t dma_addr_t; > + > +typedef unsigned int gfp_t; > +#define GFP_KERNEL 0 > +#define __GFP_ZERO 0 > + > +/* 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 The above looks pretty much ARM specific. I wonder whether it would make sense to also introduce an asm/linux_compat.h to place such things. > +/* Helpers for IRQ functions */ > +#define free_irq release_irq > + > +enum irqreturn { > + IRQ_NONE = (0 << 0), > + IRQ_HANDLED = (1 << 0), > + IRQ_WAKE_THREAD = (2 << 0), You don't need to set the explicit values, just using: enum irqreturn { IRQ_NONE, IRQ_HANDLED, IRQ_WAKE_THREAD, }; Will achieve exactly the same. > +}; > + > +typedef enum irqreturn irqreturn_t; > + > +/* Device logger functions */ > +#define dev_print(dev, lvl, fmt, ...) \ > + printk(lvl fmt, ## __VA_ARGS__) You don't need to pass dev here (it's not used AFAICT). > + > +#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_info(dev, fmt, ...) dev_print(dev, XENLOG_INFO, fmt, ## __VA_ARGS__) Neither here. In which case you can get rid of dev_print and simply use printk the in the above macros. Also you should make sure they are no longer than 80 cols. > + > +#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) Why use the '_' versions of malloc/zalloc? Do you really intend to force the alignment? > + > +/* Alias to Xen time functions */ > +#define ktime_t s_time_t > +#define ktime_add_us(t,i) (NOW() + MICROSECS(i)) > +#define ktime_compare(t,i) (NOW() > (i)) Er, the above is wrong, ktime_add_us adds 'i' to 't', not to NOW. Roger.
On 1/23/2018 9:51 AM, Roger Pau Monné wrote: > On Mon, Dec 18, 2017 at 08:16:58PM -0700, Sameer Goel wrote: >> For porting files directly from Linux it is useful to have a function mapping >> definitions from Linux to Xen. This file adds common API functions and >> other defines that are needed for porting arm SMMU drivers. > It would be good that you add things to linux_compat.h in the same > patch that you import the code, or else reviewing whether this is > needed or not is impossible. I'll do that. > >> Signed-off-by: Sameer Goel <sameer.goel@linaro.org> >> --- >> xen/include/xen/linux_compat.h | 81 ++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 81 insertions(+) >> create mode 100644 xen/include/xen/linux_compat.h >> >> diff --git a/xen/include/xen/linux_compat.h b/xen/include/xen/linux_compat.h >> new file mode 100644 >> index 0000000000..174d0390e5 >> --- /dev/null >> +++ b/xen/include/xen/linux_compat.h >> @@ -0,0 +1,81 @@ >> +/****************************************************************************** >> + * include/xen/linux_compat.h >> + * >> + * Compatibility defines for porting code from Linux to Xen >> + * >> + * 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 __XEN_LINUX_COMPAT_H__ >> +#define __XEN_LINUX_COMPAT_H__ >> + >> +#include <asm/types.h> >> + >> +typedef paddr_t phys_addr_t; >> +typedef paddr_t dma_addr_t; >> + >> +typedef unsigned int gfp_t; >> +#define GFP_KERNEL 0 >> +#define __GFP_ZERO 0 >> + >> +/* 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 > The above looks pretty much ARM specific. I wonder whether it would > make sense to also introduce an asm/linux_compat.h to place such > things. Yes at this point this is arm specific. Since, this is only being used for smmu specific code I'll move this out to the smmu specific header. >> +/* Helpers for IRQ functions */ >> +#define free_irq release_irq >> + >> +enum irqreturn { >> + IRQ_NONE = (0 << 0), >> + IRQ_HANDLED = (1 << 0), >> + IRQ_WAKE_THREAD = (2 << 0), > You don't need to set the explicit values, just using: Ok. > > enum irqreturn { > IRQ_NONE, > IRQ_HANDLED, > IRQ_WAKE_THREAD, > }; > > Will achieve exactly the same. > >> +}; >> + >> +typedef enum irqreturn irqreturn_t; >> + >> +/* Device logger functions */ >> +#define dev_print(dev, lvl, fmt, ...) \ >> + printk(lvl fmt, ## __VA_ARGS__) > You don't need to pass dev here (it's not used AFAICT). > >> + >> +#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_info(dev, fmt, ...) dev_print(dev, XENLOG_INFO, fmt, ## __VA_ARGS__) > Neither here. In which case you can get rid of dev_print and simply > use printk the in the above macros. Also you should make sure they are > no longer than 80 cols. Ok. > >> + >> +#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) > Why use the '_' versions of malloc/zalloc? Do you really intend to > force the alignment? No, it just that xmalloc and xzalloc don't take in a size. I can use xmalloc_bytes and xzalloc_bytes but the above defines worked fine in the upstreamed smmu driver and I just copied them over. > >> + >> +/* Alias to Xen time functions */ >> +#define ktime_t s_time_t >> +#define ktime_add_us(t,i) (NOW() + MICROSECS(i)) >> +#define ktime_compare(t,i) (NOW() > (i)) > Er, the above is wrong, ktime_add_us adds 'i' to 't', not to NOW. > > Roger. > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xenproject.org > https://lists.xenproject.org/mailman/listinfo/xen-devel
diff --git a/xen/include/xen/linux_compat.h b/xen/include/xen/linux_compat.h new file mode 100644 index 0000000000..174d0390e5 --- /dev/null +++ b/xen/include/xen/linux_compat.h @@ -0,0 +1,81 @@ +/****************************************************************************** + * include/xen/linux_compat.h + * + * Compatibility defines for porting code from Linux to Xen + * + * 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 __XEN_LINUX_COMPAT_H__ +#define __XEN_LINUX_COMPAT_H__ + +#include <asm/types.h> + +typedef paddr_t phys_addr_t; +typedef paddr_t dma_addr_t; + +typedef unsigned int gfp_t; +#define GFP_KERNEL 0 +#define __GFP_ZERO 0 + +/* 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 for IRQ functions */ +#define free_irq release_irq + +enum irqreturn { + IRQ_NONE = (0 << 0), + IRQ_HANDLED = (1 << 0), + IRQ_WAKE_THREAD = (2 << 0), +}; + +typedef enum irqreturn irqreturn_t; + +/* Device logger functions */ +#define dev_print(dev, lvl, fmt, ...) \ + printk(lvl fmt, ## __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_info(dev, fmt, ...) dev_print(dev, XENLOG_INFO, 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) + +/* Alias to Xen time functions */ +#define ktime_t s_time_t +#define ktime_add_us(t,i) (NOW() + MICROSECS(i)) +#define ktime_compare(t,i) (NOW() > (i)) + +#endif /* __XEN_LINUX_COMPAT_H__ */
For porting files directly from Linux it is useful to have a function mapping definitions from Linux to Xen. This file adds common API functions and other defines that are needed for porting arm SMMU drivers. Signed-off-by: Sameer Goel <sameer.goel@linaro.org> --- xen/include/xen/linux_compat.h | 81 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 xen/include/xen/linux_compat.h