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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

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.

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