[PATCH 0/2] xen/arm: static event channel

[PATCH 0/2] xen/arm: static event channel

[Rahul Singh]

This patch series fix issues related to static event channel series.

Rahul Singh (2):
  xen: Add static event channel in SUPPORT.md on ARM
  xen/arm: fix booting ACPI based system after static evtchn series

 SUPPORT.md           | 7 +++++++
 xen/arch/arm/setup.c | 5 +++--
 2 files changed, 10 insertions(+), 2 deletions(-)

-- 
2.25.1

[PATCH 1/2] xen: Add static event channel in SUPPORT.md on ARM

[Rahul Singh]

Static event channel support is tech preview, which shall be documented
in SUPPORT.md

Signed-off-by: Rahul Singh <rahul.singh@arm.com>
---
 SUPPORT.md | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/SUPPORT.md b/SUPPORT.md
index 8ebd63ad82..29f74ac506 100644
--- a/SUPPORT.md
+++ b/SUPPORT.md
@@ -922,6 +922,13 @@ bootscrub=off are passed as Xen command line parameters. (Memory should
 be scrubbed with bootscrub=idle.) No XSAs will be issues due to
 unscrubbed memory.
 
+## Static Event Channel
+
+Allow to setup the static event channel on dom0less system, enabling domains
+to send/receive notifications.
+
+    Status, ARM: Tech Preview
+
 # Format and definitions
 
 This file contains prose, and machine-readable fragments.
-- 
2.25.1

Re: [PATCH 1/2] xen: Add static event channel in SUPPORT.md on ARM

[Ayan Kumar Halder]

On 23/09/2022 12:02, Rahul Singh wrote:
> CAUTION: This message has originated from an External Source. Please use proper judgment and caution when opening attachments, clicking links, or responding to this email.
>
>
> Static event channel support is tech preview, which shall be documented
> in SUPPORT.md
>
> Signed-off-by: Rahul Singh <rahul.singh@arm.com>
Reviewed-by: Ayan Kumar Halder <ayankuma@amd.com>
> ---
>   SUPPORT.md | 7 +++++++
>   1 file changed, 7 insertions(+)
>
> diff --git a/SUPPORT.md b/SUPPORT.md
> index 8ebd63ad82..29f74ac506 100644
> --- a/SUPPORT.md
> +++ b/SUPPORT.md
> @@ -922,6 +922,13 @@ bootscrub=off are passed as Xen command line parameters. (Memory should
>   be scrubbed with bootscrub=idle.) No XSAs will be issues due to
>   unscrubbed memory.
>
> +## Static Event Channel
> +
> +Allow to setup the static event channel on dom0less system, enabling domains
> +to send/receive notifications.
> +
> +    Status, ARM: Tech Preview
> +
>   # Format and definitions
>
>   This file contains prose, and machine-readable fragments.
> --
> 2.25.1
>
>

Re: [PATCH 1/2] xen: Add static event channel in SUPPORT.md on ARM

[Bertrand Marquis]

Hi Rahul,

> On 23 Sep 2022, at 13:02, Rahul Singh <Rahul.Singh@arm.com> wrote:
> 
> Static event channel support is tech preview, which shall be documented
> in SUPPORT.md
> 
> Signed-off-by: Rahul Singh <rahul.singh@arm.com>

Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

Cheers
Bertrand

> ---
> SUPPORT.md | 7 +++++++
> 1 file changed, 7 insertions(+)
> 
> diff --git a/SUPPORT.md b/SUPPORT.md
> index 8ebd63ad82..29f74ac506 100644
> --- a/SUPPORT.md
> +++ b/SUPPORT.md
> @@ -922,6 +922,13 @@ bootscrub=off are passed as Xen command line parameters. (Memory should
> be scrubbed with bootscrub=idle.) No XSAs will be issues due to
> unscrubbed memory.
> 
> +## Static Event Channel
> +
> +Allow to setup the static event channel on dom0less system, enabling domains
> +to send/receive notifications.
> +
> +    Status, ARM: Tech Preview
> +
> # Format and definitions
> 
> This file contains prose, and machine-readable fragments.
> -- 
> 2.25.1
> 
> 

[PATCH 2/2] xen/arm: fix booting ACPI based system after static evtchn series

[Rahul Singh]

When ACPI is enabled and the system booted with ACPI, BUG() is observed
after merging the static event channel series. As there is not DT when
booted with ACPI there will be no chosen node because of that
"BUG_ON(chosen == NULL)" will be hit.

(XEN) Xen BUG at arch/arm/domain_build.c:3578

Move call to alloc_static_evtchn() under acpi_disabled check to fix the
issue.

Fixes: 1fe16b3ed78a (xen/arm: introduce xen-evtchn dom0less property)
Signed-off-by: Rahul Singh <rahul.singh@arm.com>
---
 xen/arch/arm/setup.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 61b4f258a0..4395640019 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -1166,9 +1166,10 @@ void __init start_xen(unsigned long boot_phys_offset,
         printk(XENLOG_INFO "Xen dom0less mode detected\n");
 
     if ( acpi_disabled )
+    {
         create_domUs();
-
-    alloc_static_evtchn();
+        alloc_static_evtchn();
+    }
 
     /*
      * This needs to be called **before** heap_init_late() so modules
-- 
2.25.1

Re: [PATCH 2/2] xen/arm: fix booting ACPI based system after static evtchn series

[Ayan Kumar Halder]

Hi Rahul,

On 23/09/2022 12:02, Rahul Singh wrote:
> CAUTION: This message has originated from an External Source. Please use proper judgment and caution when opening attachments, clicking links, or responding to this email.
>
>
> When ACPI is enabled and the system booted with ACPI, BUG() is observed
> after merging the static event channel series. As there is not DT when
[NIT] : s/not/no
> booted with ACPI there will be no chosen node because of that
> "BUG_ON(chosen == NULL)" will be hit.
>
> (XEN) Xen BUG at arch/arm/domain_build.c:3578
Is the bug seen on the gitlab ci ?
>
> Move call to alloc_static_evtchn() under acpi_disabled check to fix the
> issue.
>
> Fixes: 1fe16b3ed78a (xen/arm: introduce xen-evtchn dom0less property)
> Signed-off-by: Rahul Singh <rahul.singh@arm.com>
> ---
>   xen/arch/arm/setup.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 61b4f258a0..4395640019 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -1166,9 +1166,10 @@ void __init start_xen(unsigned long boot_phys_offset,
>           printk(XENLOG_INFO "Xen dom0less mode detected\n");
>
>       if ( acpi_disabled )
> +    {
>           create_domUs();
> -
> -    alloc_static_evtchn();
> +        alloc_static_evtchn();

Can the code in alloc_static_evtchn() be guarded with "#ifndef 
CONFIG_ACPI ... endif" ?

- Ayan

> +    }
>
>       /*
>        * This needs to be called **before** heap_init_late() so modules
> --
> 2.25.1
>
>

Re: [PATCH 2/2] xen/arm: fix booting ACPI based system after static evtchn series

[Rahul Singh]

Hi Ayan,
 
> On 23 Sep 2022, at 1:10 pm, Ayan Kumar Halder <ayankuma@amd.com> wrote:
> 
> Hi Rahul,
> 
> On 23/09/2022 12:02, Rahul Singh wrote:
>> CAUTION: This message has originated from an External Source. Please use proper judgment and caution when opening attachments, clicking links, or responding to this email.
>> 
>> 
>> When ACPI is enabled and the system booted with ACPI, BUG() is observed
>> after merging the static event channel series. As there is not DT when
> [NIT] : s/not/no

Ack. 
>> booted with ACPI there will be no chosen node because of that
>> "BUG_ON(chosen == NULL)" will be hit.
>> 
>> (XEN) Xen BUG at arch/arm/domain_build.c:3578
> Is the bug seen on the gitlab ci ?

No, I found the issue while testing the ACPI boot. But going forward we will add this in our internal ci.
>> 
>> Move call to alloc_static_evtchn() under acpi_disabled check to fix the
>> issue.
>> 
>> Fixes: 1fe16b3ed78a (xen/arm: introduce xen-evtchn dom0less property)
>> Signed-off-by: Rahul Singh <rahul.singh@arm.com>
>> ---
>>  xen/arch/arm/setup.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>> 
>> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
>> index 61b4f258a0..4395640019 100644
>> --- a/xen/arch/arm/setup.c
>> +++ b/xen/arch/arm/setup.c
>> @@ -1166,9 +1166,10 @@ void __init start_xen(unsigned long boot_phys_offset,
>>          printk(XENLOG_INFO "Xen dom0less mode detected\n");
>> 
>>      if ( acpi_disabled )
>> +    {
>>          create_domUs();
>> -
>> -    alloc_static_evtchn();
>> +        alloc_static_evtchn();
> 
> Can the code in alloc_static_evtchn() be guarded with "#ifndef CONFIG_ACPI ... endif" ?

Not required as acpi_disabled will take care of that. acpi_disabled variable is used to avoid the CONFIG_ACPI.

Regards,
Rahul

Re: [PATCH 2/2] xen/arm: fix booting ACPI based system after static evtchn series

[Jan Beulich]

On 23.09.2022 14:10, Ayan Kumar Halder wrote:
> On 23/09/2022 12:02, Rahul Singh wrote:
>> --- a/xen/arch/arm/setup.c
>> +++ b/xen/arch/arm/setup.c
>> @@ -1166,9 +1166,10 @@ void __init start_xen(unsigned long boot_phys_offset,
>>           printk(XENLOG_INFO "Xen dom0less mode detected\n");
>>
>>       if ( acpi_disabled )
>> +    {
>>           create_domUs();
>> -
>> -    alloc_static_evtchn();
>> +        alloc_static_evtchn();
> 
> Can the code in alloc_static_evtchn() be guarded with "#ifndef 
> CONFIG_ACPI ... endif" ?

This wouldn't help the issue, but at best code size when !CONFIG_ACPI. When
CONFIG_ACPI=y, acpi_disabled might still be true, and hence the function
may still need skipping. Apart from this I'd also consider it odd to have
a non-ACPI function have such an #ifdef ...

Jan

Re: [PATCH 2/2] xen/arm: fix booting ACPI based system after static evtchn series

[Ayan Kumar Halder]

On 26/09/2022 08:38, Jan Beulich wrote:
> On 23.09.2022 14:10, Ayan Kumar Halder wrote:
>> On 23/09/2022 12:02, Rahul Singh wrote:
>>> --- a/xen/arch/arm/setup.c
>>> +++ b/xen/arch/arm/setup.c
>>> @@ -1166,9 +1166,10 @@ void __init start_xen(unsigned long boot_phys_offset,
>>>            printk(XENLOG_INFO "Xen dom0less mode detected\n");
>>>
>>>        if ( acpi_disabled )
>>> +    {
>>>            create_domUs();
>>> -
>>> -    alloc_static_evtchn();
>>> +        alloc_static_evtchn();
>> Can the code in alloc_static_evtchn() be guarded with "#ifndef
>> CONFIG_ACPI ... endif" ?
> This wouldn't help the issue, but at best code size when !CONFIG_ACPI. When
> CONFIG_ACPI=y, acpi_disabled might still be true, and hence the function
> may still need skipping. Apart from this I'd also consider it odd to have
> a non-ACPI function have such an #ifdef ...

I think this makes sense. Also Rahul's previous comments looks 
reasonable to me.

Reviewed-by: Ayan Kumar Halder <ayankuma@amd.com>

>
> Jan

Re: [PATCH 2/2] xen/arm: fix booting ACPI based system after static evtchn series

[Bertrand Marquis]

Hi,

> On 23 Sep 2022, at 13:02, Rahul Singh <Rahul.Singh@arm.com> wrote:
> 
> When ACPI is enabled and the system booted with ACPI, BUG() is observed
> after merging the static event channel series. As there is not DT when
> booted with ACPI there will be no chosen node because of that
> "BUG_ON(chosen == NULL)" will be hit.
> 
> (XEN) Xen BUG at arch/arm/domain_build.c:3578
> 
> Move call to alloc_static_evtchn() under acpi_disabled check to fix the
> issue.
> 
> Fixes: 1fe16b3ed78a (xen/arm: introduce xen-evtchn dom0less property)
> Signed-off-by: Rahul Singh <rahul.singh@arm.com>

Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

This is needed to fix ACPI build issues and was release-acked already.

Cheers
Bertrand

> ---
> xen/arch/arm/setup.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 61b4f258a0..4395640019 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -1166,9 +1166,10 @@ void __init start_xen(unsigned long boot_phys_offset,
>         printk(XENLOG_INFO "Xen dom0less mode detected\n");
> 
>     if ( acpi_disabled )
> +    {
>         create_domUs();
> -
> -    alloc_static_evtchn();
> +        alloc_static_evtchn();
> +    }
> 
>     /*
>      * This needs to be called **before** heap_init_late() so modules
> -- 
> 2.25.1
> 

Re: [PATCH 0/2] xen/arm: static event channel

[Bertrand Marquis]

Hi Rahul,

Please give the necessary justification for inclusion in 4.17:
- severity of the bug fixed
- probability and impact of potential issues that the patch could add.

thanks
Bertrand

> On 23 Sep 2022, at 13:02, Rahul Singh <Rahul.Singh@arm.com> wrote:
> 
> This patch series fix issues related to static event channel series.
> 
> Rahul Singh (2):
>  xen: Add static event channel in SUPPORT.md on ARM
>  xen/arm: fix booting ACPI based system after static evtchn series
> 
> SUPPORT.md           | 7 +++++++
> xen/arch/arm/setup.c | 5 +++--
> 2 files changed, 10 insertions(+), 2 deletions(-)
> 
> -- 
> 2.25.1
> 

Re: [PATCH 0/2] xen/arm: static event channel

[Rahul Singh]

Hi All,

> On 26 Sep 2022, at 1:12 pm, Bertrand Marquis <Bertrand.Marquis@arm.com> wrote:
> 
> Hi Rahul,
> 
> Please give the necessary justification for inclusion in 4.17:
> - severity of the bug fixed

The severity of the bug is high as without this fixed system with ACPI support will fail to boot.

> - probability and impact of potential issues that the patch could add.

As we are not supporting the static event channel for ACPI, it is okay to move
alloc_static_evtchn() under acpi_disabled check.

Regards,
Rahul

RE: [PATCH 0/2] xen/arm: static event channel

[Henry Wang]

Hi Rahul and all,

> -----Original Message-----
> From: Rahul Singh <Rahul.Singh@arm.com>
> Subject: Re: [PATCH 0/2] xen/arm: static event channel
> 
> Hi All,
> > On 26 Sep 2022, at 1:12 pm, Bertrand Marquis
> <Bertrand.Marquis@arm.com> wrote:
> > Hi Rahul,
> >
> > Please give the necessary justification for inclusion in 4.17:
> > - severity of the bug fixed
> The severity of the bug is high as without this fixed system with ACPI support
> will fail to boot.

I agree on that, dom0 boot from ACPI on Arm is what we currently supported.
So I think we need to fix the current issue for 4.17.

Besides the bugfix patch, the other patch in this series is a pure doc change
so it should be safe.

I think we still need Arm maintainers' R-b or ack for this series, and as long as
This series is properly reviewed:

Release-acked-by: Henry Wang <Henry.Wang@arm.com>

> 
> > - probability and impact of potential issues that the patch could add.
> 
> As we are not supporting the static event channel for ACPI, it is okay to move
> alloc_static_evtchn() under acpi_disabled check.

Agreed.

Kind regards,
Henry

> 
> Regards,
> Rahul
>