Re: [PATCH v4 02/14] xen/asm-generic: introduce generic device.h

2023-11-30 Thread Oleksii
Hi Shawn,

On Wed, 2023-11-29 at 13:18 -0600, Shawn Anastasio wrote:
> On 11/29/23 6:49 AM, Oleksii wrote:
> > On Tue, 2023-11-28 at 15:28 -0600, Shawn Anastasio wrote:
> > > Hi Oleksii,
> > > 
> > > On 11/27/23 8:13 AM, Oleksii Kurochko wrote:
> > > > diff --git a/xen/arch/ppc/include/asm/irq.h
> > > > b/xen/arch/ppc/include/asm/irq.h
> > > > index 5c37d0cf25..49193fddff 100644
> > > > --- a/xen/arch/ppc/include/asm/irq.h
> > > > +++ b/xen/arch/ppc/include/asm/irq.h
> > > > @@ -3,7 +3,9 @@
> > > >  #define __ASM_PPC_IRQ_H__
> > > >  
> > > >  #include 
> > > > +#ifdef CONFIG_HAS_DEVICE_TREE
> > > 
> > > I realize that you were likely following PPC's device.h which
> > > also
> > > checks CONFIG_HAS_DEVICE_TREE, but I'm not sure that it makes
> > > sense
> > > to
> > > check this conditional in PPC code at all -- we will always have
> > > HAS_DEVICE_TREE (selected by PPC) and I can't imagine a scenario
> > > where
> > > this will ever not be the case.
> > What about case if ACPI is used? Does ACPI is supported by PPC?
> > 
> > But if you are sure that CONFIG_HAS_DEVICE_TREE will be always
> > selected
> > then I am OK to remove this change.
> > 
> 
> ACPI isn't supported by any PPC platform, we always use device tree.
Thanks for clarification.

Then we can remove useless #ifdef. I'll do it it in next patch version.

> 
> > > 
> > > Unless Jan (or someone else) disagrees, I'd rather this
> > > conditional
> > > be
> > > dropped inside of PPC code.
> > I'll double check but I think I had a compilation issue if it isn't
> > defined.
> > 
> 
> I'm not encountering any issues locally with the conditional dropped,
> but if you are able to reproduce them then let me know.
Sure. I am going to do it tomorrow or today evening if I'll face an
issue will write you.

~ Oleksii



Re: [PATCH v4 02/14] xen/asm-generic: introduce generic device.h

2023-11-29 Thread Shawn Anastasio
On 11/29/23 6:49 AM, Oleksii wrote:
> On Tue, 2023-11-28 at 15:28 -0600, Shawn Anastasio wrote:
>> Hi Oleksii,
>>
>> On 11/27/23 8:13 AM, Oleksii Kurochko wrote:
>>> diff --git a/xen/arch/ppc/include/asm/irq.h
>>> b/xen/arch/ppc/include/asm/irq.h
>>> index 5c37d0cf25..49193fddff 100644
>>> --- a/xen/arch/ppc/include/asm/irq.h
>>> +++ b/xen/arch/ppc/include/asm/irq.h
>>> @@ -3,7 +3,9 @@
>>>  #define __ASM_PPC_IRQ_H__
>>>  
>>>  #include 
>>> +#ifdef CONFIG_HAS_DEVICE_TREE
>>
>> I realize that you were likely following PPC's device.h which also
>> checks CONFIG_HAS_DEVICE_TREE, but I'm not sure that it makes sense
>> to
>> check this conditional in PPC code at all -- we will always have
>> HAS_DEVICE_TREE (selected by PPC) and I can't imagine a scenario
>> where
>> this will ever not be the case.
> What about case if ACPI is used? Does ACPI is supported by PPC?
> 
> But if you are sure that CONFIG_HAS_DEVICE_TREE will be always selected
> then I am OK to remove this change.
> 

ACPI isn't supported by any PPC platform, we always use device tree.

>>
>> Unless Jan (or someone else) disagrees, I'd rather this conditional
>> be
>> dropped inside of PPC code.
> I'll double check but I think I had a compilation issue if it isn't
> defined.
>

I'm not encountering any issues locally with the conditional dropped,
but if you are able to reproduce them then let me know.

> Thanks.
> 
> ~ Oleksii

Thanks,
Shawn



Re: [PATCH v4 02/14] xen/asm-generic: introduce generic device.h

2023-11-29 Thread Oleksii
On Tue, 2023-11-28 at 15:28 -0600, Shawn Anastasio wrote:
> Hi Oleksii,
> 
> On 11/27/23 8:13 AM, Oleksii Kurochko wrote:
> > diff --git a/xen/arch/ppc/include/asm/Makefile
> > b/xen/arch/ppc/include/asm/Makefile
> > index ece7fa66dd..df4c1ebb08 100644
> > --- a/xen/arch/ppc/include/asm/Makefile
> > +++ b/xen/arch/ppc/include/asm/Makefile
> > @@ -1,3 +1,4 @@
> >  # SPDX-License-Identifier: GPL-2.0-only
> > +generic-y += device.h
> >  generic-y += paging.h
> >  generic-y += vm_event.h
> > diff --git a/xen/arch/ppc/include/asm/device.h
> > b/xen/arch/ppc/include/asm/device.h
> > deleted file mode 100644
> > index 8253e61d51..00
> > --- a/xen/arch/ppc/include/asm/device.h
> > +++ /dev/null
> > @@ -1,53 +0,0 @@
> > -/* SPDX-License-Identifier: GPL-2.0-only */
> > -#ifndef __ASM_PPC_DEVICE_H__
> > -#define __ASM_PPC_DEVICE_H__
> > -
> > -enum device_type
> > -{
> > -    DEV_DT,
> > -    DEV_PCI,
> > -};
> > -
> > -struct device {
> > -    enum device_type type;
> > -#ifdef CONFIG_HAS_DEVICE_TREE
> > -    struct dt_device_node *of_node; /* Used by drivers imported
> > from Linux */
> > -#endif
> > -};
> > -
> > -enum device_class
> > -{
> > -    DEVICE_SERIAL,
> > -    DEVICE_IOMMU,
> > -    DEVICE_PCI_HOSTBRIDGE,
> > -    /* Use for error */
> > -    DEVICE_UNKNOWN,
> > -};
> > -
> > -struct device_desc {
> > -    /* Device name */
> > -    const char *name;
> > -    /* Device class */
> > -    enum device_class class;
> > -    /* List of devices supported by this driver */
> > -    const struct dt_device_match *dt_match;
> > -    /*
> > - * Device initialization.
> > - *
> > - * -EAGAIN is used to indicate that device probing is
> > deferred.
> > - */
> > -    int (*init)(struct dt_device_node *dev, const void *data);
> > -};
> > -
> > -typedef struct device device_t;
> > -
> > -#define DT_DEVICE_START(name_, namestr_,
> > class_)    \
> > -static const struct device_desc __dev_desc_##name_
> > __used   \
> > -__section(".dev.info") =
> > {  \
> > -    .name =
> > namestr_,   \
> > -    .class =
> > class_,    \
> > -
> > -#define
> > DT_DEVICE_END   \
> > -};
> > -
> > -#endif /* __ASM_PPC_DEVICE_H__ */
> > diff --git a/xen/arch/ppc/include/asm/irq.h
> > b/xen/arch/ppc/include/asm/irq.h
> > index 5c37d0cf25..49193fddff 100644
> > --- a/xen/arch/ppc/include/asm/irq.h
> > +++ b/xen/arch/ppc/include/asm/irq.h
> > @@ -3,7 +3,9 @@
> >  #define __ASM_PPC_IRQ_H__
> >  
> >  #include 
> > +#ifdef CONFIG_HAS_DEVICE_TREE
> 
> I realize that you were likely following PPC's device.h which also
> checks CONFIG_HAS_DEVICE_TREE, but I'm not sure that it makes sense
> to
> check this conditional in PPC code at all -- we will always have
> HAS_DEVICE_TREE (selected by PPC) and I can't imagine a scenario
> where
> this will ever not be the case.
What about case if ACPI is used? Does ACPI is supported by PPC?

But if you are sure that CONFIG_HAS_DEVICE_TREE will be always selected
then I am OK to remove this change.

> 
> Unless Jan (or someone else) disagrees, I'd rather this conditional
> be
> dropped inside of PPC code.
I'll double check but I think I had a compilation issue if it isn't
defined.

> 
> >  #include 
> > +#endif
> >  #include 
> >  
> >  /* TODO */
> > @@ -25,9 +27,11 @@ static inline void arch_move_irqs(struct vcpu
> > *v)
> >  BUG_ON("unimplemented");
> >  }
> >  
> > +#ifdef CONFIG_HAS_DEVICE_TREE
> 
> Ditto.
> 
> >  static inline int platform_get_irq(const struct dt_device_node
> > *device, int index)
> >  {
> >  BUG_ON("unimplemented");
> >  }
> > +#endif
> >  
> >  #endif /* __ASM_PPC_IRQ_H__ */

Thanks.

~ Oleksii



Re: [PATCH v4 02/14] xen/asm-generic: introduce generic device.h

2023-11-29 Thread Jan Beulich
On 28.11.2023 22:28, Shawn Anastasio wrote:
> On 11/27/23 8:13 AM, Oleksii Kurochko wrote:
>> --- a/xen/arch/ppc/include/asm/irq.h
>> +++ b/xen/arch/ppc/include/asm/irq.h
>> @@ -3,7 +3,9 @@
>>  #define __ASM_PPC_IRQ_H__
>>  
>>  #include 
>> +#ifdef CONFIG_HAS_DEVICE_TREE
> 
> I realize that you were likely following PPC's device.h which also
> checks CONFIG_HAS_DEVICE_TREE, but I'm not sure that it makes sense to
> check this conditional in PPC code at all -- we will always have
> HAS_DEVICE_TREE (selected by PPC) and I can't imagine a scenario where
> this will ever not be the case.
> 
> Unless Jan (or someone else) disagrees, I'd rather this conditional be
> dropped inside of PPC code.

No, pointless #ifdef are indeed better avoided.

Jan



Re: [PATCH v4 02/14] xen/asm-generic: introduce generic device.h

2023-11-28 Thread Shawn Anastasio
Hi Oleksii,

On 11/27/23 1:46 PM, Oleksii wrote:
> On Mon, 2023-11-27 at 15:31 +0100, Jan Beulich wrote:
>> On 27.11.2023 15:13, Oleksii Kurochko wrote:
>>> --- a/xen/arch/ppc/include/asm/irq.h
>>> +++ b/xen/arch/ppc/include/asm/irq.h
>>> @@ -3,7 +3,9 @@
>>>  #define __ASM_PPC_IRQ_H__
>>>  
>>>  #include 
>>> +#ifdef CONFIG_HAS_DEVICE_TREE
>>>  #include 
>>> +#endif
>>>  #include 
>>
>> Why would this #ifdef not cover the public header as well? (Otherwise
>> I'd
>> be inclined to ask that the conditional be moved inside that header.)
> In that header is defined only consts without additional header
> inclusion. At that moment it looked to me pretty save to ifdef only
> xen/device_tree.h but you are right we can move incluion of the public
> header inside #ifdef OR just remove as xen/device_tree.h already
> includes it.
>

Removing the include of  from ppc's asm/irq.h breaks
the build, because of platform_get_irq's usage of the `struct
dt_device_node` type:

  CC  common/cpu.o
In file included from ./include/xen/irq.h:80,
 from ./include/xen/pci.h:13,
 from ./include/xen/iommu.h:25,
 from ./include/xen/sched.h:12,
 from ./include/xen/event.h:12,
 from common/cpu.c:3:
./arch/ppc/include/asm/irq.h:28:49: error: ‘struct dt_device_node’
declared inside parameter list will not be visible outside of this
definition or declaration [-Werror]
   28 | static inline int platform_get_irq(const struct dt_device_node
*device, int index)

I wouldn't be opposed to a forward declaration of that type, but as I
indicated in my other reply to this patch, I'd like to avoid the
addition of these essentially always-dead CONFIG_HAS_DEVICE_TREE checks
to PPC code anyways, so dropping this hunk entirely from the patch might
be the way forward.

> ~ Oleksii

Thanks,
Shawn



Re: [PATCH v4 02/14] xen/asm-generic: introduce generic device.h

2023-11-28 Thread Shawn Anastasio
Hi Oleksii,

On 11/27/23 8:13 AM, Oleksii Kurochko wrote:
> diff --git a/xen/arch/ppc/include/asm/Makefile 
> b/xen/arch/ppc/include/asm/Makefile
> index ece7fa66dd..df4c1ebb08 100644
> --- a/xen/arch/ppc/include/asm/Makefile
> +++ b/xen/arch/ppc/include/asm/Makefile
> @@ -1,3 +1,4 @@
>  # SPDX-License-Identifier: GPL-2.0-only
> +generic-y += device.h
>  generic-y += paging.h
>  generic-y += vm_event.h
> diff --git a/xen/arch/ppc/include/asm/device.h 
> b/xen/arch/ppc/include/asm/device.h
> deleted file mode 100644
> index 8253e61d51..00
> --- a/xen/arch/ppc/include/asm/device.h
> +++ /dev/null
> @@ -1,53 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0-only */
> -#ifndef __ASM_PPC_DEVICE_H__
> -#define __ASM_PPC_DEVICE_H__
> -
> -enum device_type
> -{
> -DEV_DT,
> -DEV_PCI,
> -};
> -
> -struct device {
> -enum device_type type;
> -#ifdef CONFIG_HAS_DEVICE_TREE
> -struct dt_device_node *of_node; /* Used by drivers imported from Linux */
> -#endif
> -};
> -
> -enum device_class
> -{
> -DEVICE_SERIAL,
> -DEVICE_IOMMU,
> -DEVICE_PCI_HOSTBRIDGE,
> -/* Use for error */
> -DEVICE_UNKNOWN,
> -};
> -
> -struct device_desc {
> -/* Device name */
> -const char *name;
> -/* Device class */
> -enum device_class class;
> -/* List of devices supported by this driver */
> -const struct dt_device_match *dt_match;
> -/*
> - * Device initialization.
> - *
> - * -EAGAIN is used to indicate that device probing is deferred.
> - */
> -int (*init)(struct dt_device_node *dev, const void *data);
> -};
> -
> -typedef struct device device_t;
> -
> -#define DT_DEVICE_START(name_, namestr_, class_)\
> -static const struct device_desc __dev_desc_##name_ __used   \
> -__section(".dev.info") = {  \
> -.name = namestr_,   \
> -.class = class_,\
> -
> -#define DT_DEVICE_END   \
> -};
> -
> -#endif /* __ASM_PPC_DEVICE_H__ */
> diff --git a/xen/arch/ppc/include/asm/irq.h b/xen/arch/ppc/include/asm/irq.h
> index 5c37d0cf25..49193fddff 100644
> --- a/xen/arch/ppc/include/asm/irq.h
> +++ b/xen/arch/ppc/include/asm/irq.h
> @@ -3,7 +3,9 @@
>  #define __ASM_PPC_IRQ_H__
>  
>  #include 
> +#ifdef CONFIG_HAS_DEVICE_TREE

I realize that you were likely following PPC's device.h which also
checks CONFIG_HAS_DEVICE_TREE, but I'm not sure that it makes sense to
check this conditional in PPC code at all -- we will always have
HAS_DEVICE_TREE (selected by PPC) and I can't imagine a scenario where
this will ever not be the case.

Unless Jan (or someone else) disagrees, I'd rather this conditional be
dropped inside of PPC code.

>  #include 
> +#endif
>  #include 
>  
>  /* TODO */
> @@ -25,9 +27,11 @@ static inline void arch_move_irqs(struct vcpu *v)
>  BUG_ON("unimplemented");
>  }
>  
> +#ifdef CONFIG_HAS_DEVICE_TREE

Ditto.

>  static inline int platform_get_irq(const struct dt_device_node *device, int 
> index)
>  {
>  BUG_ON("unimplemented");
>  }
> +#endif
>  
>  #endif /* __ASM_PPC_IRQ_H__ */

Thanks,
Shawn



Re: [PATCH v4 02/14] xen/asm-generic: introduce generic device.h

2023-11-27 Thread Jan Beulich
On 27.11.2023 20:46, Oleksii wrote:
> On Mon, 2023-11-27 at 15:31 +0100, Jan Beulich wrote:
>> On 27.11.2023 15:13, Oleksii Kurochko wrote:
>>> --- a/xen/arch/ppc/include/asm/irq.h
>>> +++ b/xen/arch/ppc/include/asm/irq.h
>>> @@ -3,7 +3,9 @@
>>>  #define __ASM_PPC_IRQ_H__
>>>  
>>>  #include 
>>> +#ifdef CONFIG_HAS_DEVICE_TREE
>>>  #include 
>>> +#endif
>>>  #include 
>>
>> Why would this #ifdef not cover the public header as well? (Otherwise
>> I'd
>> be inclined to ask that the conditional be moved inside that header.)
> In that header is defined only consts without additional header
> inclusion. At that moment it looked to me pretty save to ifdef only
> xen/device_tree.h but you are right we can move incluion of the public
> header inside #ifdef OR just remove as xen/device_tree.h already
> includes it.

Oh, yes, dropping the redundant #include would be even better then. Yet
that furthers the desire to have the #ifdef inside that other header, to
improve how things look as use sites like the one here.

Jan



Re: [PATCH v4 02/14] xen/asm-generic: introduce generic device.h

2023-11-27 Thread Oleksii
On Mon, 2023-11-27 at 15:31 +0100, Jan Beulich wrote:
> On 27.11.2023 15:13, Oleksii Kurochko wrote:
> > Arm, PPC and RISC-V use the same device.h thereby device.h
> > was moved to asm-generic. Arm's device.h was taken as a base with
> > the following changes:
> >  - #ifdef PCI related things.
> >  - #ifdef ACPI related things.
> >  - Rename #ifdef guards.
> >  - Add SPDX tag.
> >  - #ifdef CONFIG_HAS_DEVICE_TREE related things.
> >  - #ifdef-ing iommu related things with CONFIG_HAS_PASSTHROUGH.
> > 
> > Also Arm and PPC are switched to asm-generic version of device.h
> > 
> > Signed-off-by: Oleksii Kurochko 
> > ---
> > Changes in V4:
> >  - Updated the commit message
> >  - Switched Arm and PPC to asm-generic version of device.h
> >  - Replaced HAS_PCI with CONFIG_HAS_PCI
> >  - ifdef-ing iommu filed of dev_archdata struct with
> > CONFIG_HAS_PASSTHROUGH
> >  - ifdef-ing iommu_fwspec of device struct with
> > CONFIG_HAS_PASSTHROUGH
> >  - ifdef-ing DT related things with CONFIG_HAS_DEVICE_TREE
> >  - Updated the commit message ( remove a note with question about
> >    if device.h should be in asm-generic or not )
> >  - Replaced DEVICE_IC with DEVICE_INTERRUPT_CONTROLLER
> >  - Rationalized usage of CONFIG_HAS_* in device.h
> >  - Fixed indents for ACPI_DEVICE_START and ACPI_DEVICE_END
> > ---
> > Changes in V3:
> >  - ifdef device tree related things.
> >  - update the commit message
> > ---
> > Changes in V2:
> > - take ( as common ) device.h from Arm as PPC and RISC-V
> > use it as a base.
> > - #ifdef PCI related things.
> > - #ifdef ACPI related things.
> > - rename DEVICE_GIC to DEVIC_IC.
> > - rename #ifdef guards.
> > - switch Arm and PPC to generic device.h
> > - add SPDX tag
> > - update the commit message
> > 
> > ---
> >  xen/arch/arm/device.c |  15 ++-
> >  xen/arch/arm/domain_build.c   |   2 +-
> >  xen/arch/arm/gic-v2.c |   4 +-
> >  xen/arch/arm/gic-v3.c |   6 +-
> >  xen/arch/arm/gic.c    |   4 +-
> >  xen/arch/arm/include/asm/Makefile |   1 +
> >  xen/arch/ppc/include/asm/Makefile |   1 +
> >  xen/arch/ppc/include/asm/device.h |  53 
> >  xen/arch/ppc/include/asm/irq.h    |   4 +
> >  .../asm => include/asm-generic}/device.h  | 125 +++---
> > 
> >  xen/include/headers++.chk.new |   0
> >  11 files changed, 106 insertions(+), 109 deletions(-)
> >  delete mode 100644 xen/arch/ppc/include/asm/device.h
> >  rename xen/{arch/arm/include/asm => include/asm-generic}/device.h
> > (79%)
> >  create mode 100644 xen/include/headers++.chk.new
> 
> Stray new file, presumably because of a missing entry in .gitignore?
Yeah, I don't have such entry in .gitignore.
I will remove this file in next version of the patch.

> 
> Overall I think there are too many changes done all in one go here.
> But it's mostly Arm which is affected, so I'll leave judging about
> that
> to the Arm maintainers.
> 
> > --- a/xen/arch/ppc/include/asm/irq.h
> > +++ b/xen/arch/ppc/include/asm/irq.h
> > @@ -3,7 +3,9 @@
> >  #define __ASM_PPC_IRQ_H__
> >  
> >  #include 
> > +#ifdef CONFIG_HAS_DEVICE_TREE
> >  #include 
> > +#endif
> >  #include 
> 
> Why would this #ifdef not cover the public header as well? (Otherwise
> I'd
> be inclined to ask that the conditional be moved inside that header.)
In that header is defined only consts without additional header
inclusion. At that moment it looked to me pretty save to ifdef only
xen/device_tree.h but you are right we can move incluion of the public
header inside #ifdef OR just remove as xen/device_tree.h already
includes it.

~ Oleksii



Re: [PATCH v4 02/14] xen/asm-generic: introduce generic device.h

2023-11-27 Thread Jan Beulich
On 27.11.2023 15:13, Oleksii Kurochko wrote:
> Arm, PPC and RISC-V use the same device.h thereby device.h
> was moved to asm-generic. Arm's device.h was taken as a base with
> the following changes:
>  - #ifdef PCI related things.
>  - #ifdef ACPI related things.
>  - Rename #ifdef guards.
>  - Add SPDX tag.
>  - #ifdef CONFIG_HAS_DEVICE_TREE related things.
>  - #ifdef-ing iommu related things with CONFIG_HAS_PASSTHROUGH.
> 
> Also Arm and PPC are switched to asm-generic version of device.h
> 
> Signed-off-by: Oleksii Kurochko 
> ---
> Changes in V4:
>  - Updated the commit message
>  - Switched Arm and PPC to asm-generic version of device.h
>  - Replaced HAS_PCI with CONFIG_HAS_PCI
>  - ifdef-ing iommu filed of dev_archdata struct with CONFIG_HAS_PASSTHROUGH
>  - ifdef-ing iommu_fwspec of device struct with CONFIG_HAS_PASSTHROUGH
>  - ifdef-ing DT related things with CONFIG_HAS_DEVICE_TREE
>  - Updated the commit message ( remove a note with question about
>if device.h should be in asm-generic or not )
>  - Replaced DEVICE_IC with DEVICE_INTERRUPT_CONTROLLER
>  - Rationalized usage of CONFIG_HAS_* in device.h
>  - Fixed indents for ACPI_DEVICE_START and ACPI_DEVICE_END
> ---
> Changes in V3:
>  - ifdef device tree related things.
>  - update the commit message
> ---
> Changes in V2:
>   - take ( as common ) device.h from Arm as PPC and RISC-V use it as a 
> base.
>   - #ifdef PCI related things.
>   - #ifdef ACPI related things.
>   - rename DEVICE_GIC to DEVIC_IC.
>   - rename #ifdef guards.
>   - switch Arm and PPC to generic device.h
>   - add SPDX tag
>   - update the commit message
> 
> ---
>  xen/arch/arm/device.c |  15 ++-
>  xen/arch/arm/domain_build.c   |   2 +-
>  xen/arch/arm/gic-v2.c |   4 +-
>  xen/arch/arm/gic-v3.c |   6 +-
>  xen/arch/arm/gic.c|   4 +-
>  xen/arch/arm/include/asm/Makefile |   1 +
>  xen/arch/ppc/include/asm/Makefile |   1 +
>  xen/arch/ppc/include/asm/device.h |  53 
>  xen/arch/ppc/include/asm/irq.h|   4 +
>  .../asm => include/asm-generic}/device.h  | 125 +++---
>  xen/include/headers++.chk.new |   0
>  11 files changed, 106 insertions(+), 109 deletions(-)
>  delete mode 100644 xen/arch/ppc/include/asm/device.h
>  rename xen/{arch/arm/include/asm => include/asm-generic}/device.h (79%)
>  create mode 100644 xen/include/headers++.chk.new

Stray new file, presumably because of a missing entry in .gitignore?

Overall I think there are too many changes done all in one go here.
But it's mostly Arm which is affected, so I'll leave judging about that
to the Arm maintainers.

> --- a/xen/arch/ppc/include/asm/irq.h
> +++ b/xen/arch/ppc/include/asm/irq.h
> @@ -3,7 +3,9 @@
>  #define __ASM_PPC_IRQ_H__
>  
>  #include 
> +#ifdef CONFIG_HAS_DEVICE_TREE
>  #include 
> +#endif
>  #include 

Why would this #ifdef not cover the public header as well? (Otherwise I'd
be inclined to ask that the conditional be moved inside that header.)

Jan



[PATCH v4 02/14] xen/asm-generic: introduce generic device.h

2023-11-27 Thread Oleksii Kurochko
Arm, PPC and RISC-V use the same device.h thereby device.h
was moved to asm-generic. Arm's device.h was taken as a base with
the following changes:
 - #ifdef PCI related things.
 - #ifdef ACPI related things.
 - Rename #ifdef guards.
 - Add SPDX tag.
 - #ifdef CONFIG_HAS_DEVICE_TREE related things.
 - #ifdef-ing iommu related things with CONFIG_HAS_PASSTHROUGH.

Also Arm and PPC are switched to asm-generic version of device.h

Signed-off-by: Oleksii Kurochko 
---
Changes in V4:
 - Updated the commit message
 - Switched Arm and PPC to asm-generic version of device.h
 - Replaced HAS_PCI with CONFIG_HAS_PCI
 - ifdef-ing iommu filed of dev_archdata struct with CONFIG_HAS_PASSTHROUGH
 - ifdef-ing iommu_fwspec of device struct with CONFIG_HAS_PASSTHROUGH
 - ifdef-ing DT related things with CONFIG_HAS_DEVICE_TREE
 - Updated the commit message ( remove a note with question about
   if device.h should be in asm-generic or not )
 - Replaced DEVICE_IC with DEVICE_INTERRUPT_CONTROLLER
 - Rationalized usage of CONFIG_HAS_* in device.h
 - Fixed indents for ACPI_DEVICE_START and ACPI_DEVICE_END
---
Changes in V3:
 - ifdef device tree related things.
 - update the commit message
---
Changes in V2:
- take ( as common ) device.h from Arm as PPC and RISC-V use it as a 
base.
- #ifdef PCI related things.
- #ifdef ACPI related things.
- rename DEVICE_GIC to DEVIC_IC.
- rename #ifdef guards.
- switch Arm and PPC to generic device.h
- add SPDX tag
- update the commit message

---
 xen/arch/arm/device.c |  15 ++-
 xen/arch/arm/domain_build.c   |   2 +-
 xen/arch/arm/gic-v2.c |   4 +-
 xen/arch/arm/gic-v3.c |   6 +-
 xen/arch/arm/gic.c|   4 +-
 xen/arch/arm/include/asm/Makefile |   1 +
 xen/arch/ppc/include/asm/Makefile |   1 +
 xen/arch/ppc/include/asm/device.h |  53 
 xen/arch/ppc/include/asm/irq.h|   4 +
 .../asm => include/asm-generic}/device.h  | 125 +++---
 xen/include/headers++.chk.new |   0
 11 files changed, 106 insertions(+), 109 deletions(-)
 delete mode 100644 xen/arch/ppc/include/asm/device.h
 rename xen/{arch/arm/include/asm => include/asm-generic}/device.h (79%)
 create mode 100644 xen/include/headers++.chk.new

diff --git a/xen/arch/arm/device.c b/xen/arch/arm/device.c
index 1f631d3274..affbe79f9a 100644
--- a/xen/arch/arm/device.c
+++ b/xen/arch/arm/device.c
@@ -16,7 +16,10 @@
 #include 
 
 extern const struct device_desc _sdevice[], _edevice[];
+
+#ifdef CONFIG_ACPI
 extern const struct acpi_device_desc _asdevice[], _aedevice[];
+#endif
 
 int __init device_init(struct dt_device_node *dev, enum device_class class,
const void *data)
@@ -45,6 +48,7 @@ int __init device_init(struct dt_device_node *dev, enum 
device_class class,
 return -EBADF;
 }
 
+#ifdef CONFIG_ACPI
 int __init acpi_device_init(enum device_class class, const void *data, int 
class_type)
 {
 const struct acpi_device_desc *desc;
@@ -61,6 +65,7 @@ int __init acpi_device_init(enum device_class class, const 
void *data, int class
 
 return -EBADF;
 }
+#endif
 
 enum device_class device_get_class(const struct dt_device_node *dev)
 {
@@ -329,9 +334,13 @@ int handle_device(struct domain *d, struct dt_device_node 
*dev, p2m_type_t p2mt,
 struct map_range_data mr_data = {
 .d = d,
 .p2mt = p2mt,
-.skip_mapping = !own_device ||
-(is_pci_passthrough_enabled() &&
-(device_get_class(dev) == DEVICE_PCI_HOSTBRIDGE)),
+.skip_mapping =
+!own_device
+#ifdef CONFIG_HAS_PCI
+|| (is_pci_passthrough_enabled() &&
+(device_get_class(dev) == DEVICE_PCI_HOSTBRIDGE))
+#endif
+,
 .iomem_ranges = iomem_ranges,
 .irq_ranges = irq_ranges
 };
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 2dd2926b41..a26cbff68e 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2361,7 +2361,7 @@ static int __init handle_node(struct domain *d, struct 
kernel_info *kinfo,
  * Replace these nodes with our own. Note that the original may be
  * used_by DOMID_XEN so this check comes first.
  */
-if ( device_get_class(node) == DEVICE_GIC )
+if ( device_get_class(node) == DEVICE_INTERRUPT_CONTROLLER )
 return make_gic_node(d, kinfo->fdt, node);
 if ( dt_match_node(timer_matches, node) )
 return make_timer_node(kinfo);
diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index cf392bfd1c..5d6885e389 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -1366,7 +1366,7 @@ static const struct dt_device_match gicv2_dt_match[] 
__initconst =
 { /* sentinel */ },
 };
 
-DT_DEVICE_START(gicv2,