[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH RFC 02/15] xen/arm: move a few guest related #defines to public/arch-arm.h



Hi,

On 02/07/2018 21:37, Stefano Stabellini wrote:
On Fri, 15 Jun 2018, Julien Grall wrote:
Hi Stefano,

On 06/14/2018 10:15 PM, Stefano Stabellini wrote:
On Thu, 14 Jun 2018, Julien Grall wrote:
On 13/06/18 23:15, Stefano Stabellini wrote:
Move a few constants defined by libxl_arm.c to
xen/include/public/arch-arm.h, so that they are together with the other
guest related #defines such as GUEST_GICD_BASE and GUEST_VPL011_SPI.
Also, this way they can be reused by hypervisor code.

All variables moved to arch-arm.h should be prefixed with GUEST_* to avoid
clash with the rest of Xen.

I'll do.


Signed-off-by: Stefano Stabellini <stefanos@xxxxxxxxxx>
CC: wei.liu2@xxxxxxxxxx
CC: ian.jackson@xxxxxxxxxxxxx
---
    tools/libxl/libxl_arm.c       | 26 --------------------------
    xen/include/public/arch-arm.h | 26 ++++++++++++++++++++++++++
    2 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
index 8af9f6f..89a417f 100644
--- a/tools/libxl/libxl_arm.c
+++ b/tools/libxl/libxl_arm.c
@@ -8,23 +8,6 @@
    #include <libfdt.h>
    #include <assert.h>
    -/**
- * IRQ line type.
- * DT_IRQ_TYPE_NONE            - default, unspecified type
- * DT_IRQ_TYPE_EDGE_RISING     - rising edge triggered
- * DT_IRQ_TYPE_EDGE_FALLING    - falling edge triggered
- * DT_IRQ_TYPE_EDGE_BOTH       - rising and falling edge triggered
- * DT_IRQ_TYPE_LEVEL_HIGH      - high level triggered
- * DT_IRQ_TYPE_LEVEL_LOW       - low level triggered
- */
-#define DT_IRQ_TYPE_NONE           0x00000000
-#define DT_IRQ_TYPE_EDGE_RISING    0x00000001
-#define DT_IRQ_TYPE_EDGE_FALLING   0x00000002
-#define DT_IRQ_TYPE_EDGE_BOTH                           \
-    (DT_IRQ_TYPE_EDGE_FALLING | DT_IRQ_TYPE_EDGE_RISING)
-#define DT_IRQ_TYPE_LEVEL_HIGH     0x00000004
-#define DT_IRQ_TYPE_LEVEL_LOW      0x00000008
-

Those defines have nothing to do with the guest itself. They are currently
define in Xen without the DT_ prefix.

Sounds like we want to get rid of the DT_IRQ_TYPE_* definitions
completely, move the IRQ_TYPE_* definitions from device_tree.h to here,
and start using them in tools/libxl/libxl_arm.c (which involves a
renaming s/DT_IRQ_TYPE/IRQ_TYPE/g).

Is that what you had in mind?

Even if DT is Arm only today, the DT code is in common code and therefore
header device_tree.h should contain every thing necessary to use a DT.

If we still want to share constant with libxl then I would prefer to introduce
a new header (similar to acpi/acconfig.h) that provide all the common values.

OK, I can do that. I'll introduce a new header file.


Note that the hypervisor one don't have the DT_ prefix because they are use to
describe IRQ for both DT and ACPI in Xen. It is not that nice, we might want
to introduce aliases in that case. So we keep DT_* in libxl.

With the new header file we'll be able to reuse the same #defines in
libxl and xen. I think it would be nicer to avoid the aliases and just
use the regular definitions in libxl too? Changing libxl to use
IRQ_TYPE_ directly only requires a small patch.

I really don't want that. As I explained, IRQ_TYPE have been conveniently chosen to avoid converting DT value to IRQ_TYPE. They are not meant to be use like that libxl (or any header publicly shared) and will provide more confusion that anything for other bits than the hypervisor.

So in the hypervisor case we want to:
        1) Define DT_* in a separate header
        2) Alias IRQ_TYPE_* to corresponding one.

In the libxl case we want to use the new header containing DT_*.

Cheers,

--
Julien Grall

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.