[Xen-devel,RFC,v4,3/8] xen/linux_compat: Add a Linux compat header

Message ID 20171219031703.23420-4-sameer.goel@linaro.org
State New
Headers show
Series
  • SMMUv3 driver
Related show

Commit Message

Sameer Goel Dec. 19, 2017, 3:16 a.m.
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

Comments

Wei Liu Jan. 23, 2018, 4:28 p.m. | #1
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.
Roger Pau Monné Jan. 23, 2018, 4:51 p.m. | #2
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.
Sameer Goel Feb. 9, 2018, 5:54 p.m. | #3
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

Patch

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__ */