mbox series

[0/4] xen/arm: Unbreak ACPI

Message ID 20200926205542.9261-1-julien@xen.org (mailing list archive)
Headers show
Series xen/arm: Unbreak ACPI | expand

Message

Julien Grall Sept. 26, 2020, 8:55 p.m. UTC
From: Julien Grall <jgrall@amazon.com>

Hi all,

Xen on ARM has been broken for quite a while on ACPI systems. This
series aims to fix it.

Unfortunately I don't have a system with ACPI v6.0 or later (QEMU seems
to only support 5.1). So I did only some light testing.

I have only build tested the x86 side so far.

Cheers,

*** BLURB HERE ***

Julien Grall (4):
  xen/acpi: Rework acpi_os_map_memory() and acpi_os_unmap_memory()
  xen/arm: acpi: The fixmap area should always be cleared during
    failure/unmap
  xen/arm: Check if the platform is not using ACPI before initializing
    Dom0less
  xen/arm: Introduce fw_unreserved_regions() and use it

 xen/arch/arm/acpi/lib.c     | 79 ++++++++++++++++++++++++++++++-------
 xen/arch/arm/kernel.c       |  2 +-
 xen/arch/arm/setup.c        | 25 +++++++++---
 xen/arch/x86/acpi/lib.c     | 18 +++++++++
 xen/drivers/acpi/osl.c      | 34 ++++++++--------
 xen/include/asm-arm/setup.h |  2 +-
 xen/include/xen/acpi.h      |  1 +
 7 files changed, 123 insertions(+), 38 deletions(-)

Comments

Elliott Mitchell Sept. 27, 2020, 1:47 a.m. UTC | #1
On Sat, Sep 26, 2020 at 09:55:38PM +0100, Julien Grall wrote:
> Xen on ARM has been broken for quite a while on ACPI systems. This
> series aims to fix it.
> 
> Unfortunately I don't have a system with ACPI v6.0 or later (QEMU seems
> to only support 5.1). So I did only some light testing.
> 
> I have only build tested the x86 side so far.

On the ARM device I'm trying to get operational the boot got distinctly
further along so appears to be a distinct ACPI improvement here.
Masami Hiramatsu Sept. 28, 2020, 6:47 a.m. UTC | #2
Hello,

This made progress with my Xen boot on DeveloperBox (
https://www.96boards.org/product/developerbox/ ) with ACPI.

Thank you,


2020年9月27日(日) 5:56 Julien Grall <julien@xen.org>:

>
> From: Julien Grall <jgrall@amazon.com>
>
> Hi all,
>
> Xen on ARM has been broken for quite a while on ACPI systems. This
> series aims to fix it.
>
> Unfortunately I don't have a system with ACPI v6.0 or later (QEMU seems
> to only support 5.1). So I did only some light testing.
>
> I have only build tested the x86 side so far.
>
> Cheers,
>
> *** BLURB HERE ***
>
> Julien Grall (4):
>   xen/acpi: Rework acpi_os_map_memory() and acpi_os_unmap_memory()
>   xen/arm: acpi: The fixmap area should always be cleared during
>     failure/unmap
>   xen/arm: Check if the platform is not using ACPI before initializing
>     Dom0less
>   xen/arm: Introduce fw_unreserved_regions() and use it
>
>  xen/arch/arm/acpi/lib.c     | 79 ++++++++++++++++++++++++++++++-------
>  xen/arch/arm/kernel.c       |  2 +-
>  xen/arch/arm/setup.c        | 25 +++++++++---
>  xen/arch/x86/acpi/lib.c     | 18 +++++++++
>  xen/drivers/acpi/osl.c      | 34 ++++++++--------
>  xen/include/asm-arm/setup.h |  2 +-
>  xen/include/xen/acpi.h      |  1 +
>  7 files changed, 123 insertions(+), 38 deletions(-)
>
> --
> 2.17.1
>


--
Masami Hiramatsu
Masami Hiramatsu Sept. 28, 2020, 12:41 p.m. UTC | #3
Hi,

With Julien's ACPI fix, I found my DeveloperBox (
https://www.96boards.org/product/developerbox/ ) still failed
to boot bcause of SPCR issue. According to the UEFI EDK2
implementation, the SCPR table will not be made if user
chooses graphical console instead of serial.
In such case, we should ignore the SPCR missing error.

Compared with Linux kernel implementation, this doesn't check
the STAO (status override) table, becaue it seems STAO (or XENV)
will be provided by Xen itself.
Or, should we check STAO too?

Thank you,

---

Masami Hiramatsu (1):
      xen: acpi: Hide UART address only if SPCR exists


 xen/arch/arm/acpi/domain_build.c |   19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

--
Masami Hiramatsu (Linaro) <masami.hiramatsu@linaro.org>
Masami Hiramatsu Sept. 28, 2020, 1 p.m. UTC | #4
Hi,

I've missed the explanation of the attached patch. This prototype
patch was also needed for booting up the Xen on my box (for the system
which has no SPCR).

Thank you,

2020年9月28日(月) 15:47 Masami Hiramatsu <masami.hiramatsu@linaro.org>:
>
> Hello,
>
> This made progress with my Xen boot on DeveloperBox (
> https://www.96boards.org/product/developerbox/ ) with ACPI.
>
> Thank you,
>
>
> 2020年9月27日(日) 5:56 Julien Grall <julien@xen.org>:
>
> >
> > From: Julien Grall <jgrall@amazon.com>
> >
> > Hi all,
> >
> > Xen on ARM has been broken for quite a while on ACPI systems. This
> > series aims to fix it.
> >
> > Unfortunately I don't have a system with ACPI v6.0 or later (QEMU seems
> > to only support 5.1). So I did only some light testing.
> >
> > I have only build tested the x86 side so far.
> >
> > Cheers,
> >
> > *** BLURB HERE ***
> >
> > Julien Grall (4):
> >   xen/acpi: Rework acpi_os_map_memory() and acpi_os_unmap_memory()
> >   xen/arm: acpi: The fixmap area should always be cleared during
> >     failure/unmap
> >   xen/arm: Check if the platform is not using ACPI before initializing
> >     Dom0less
> >   xen/arm: Introduce fw_unreserved_regions() and use it
> >
> >  xen/arch/arm/acpi/lib.c     | 79 ++++++++++++++++++++++++++++++-------
> >  xen/arch/arm/kernel.c       |  2 +-
> >  xen/arch/arm/setup.c        | 25 +++++++++---
> >  xen/arch/x86/acpi/lib.c     | 18 +++++++++
> >  xen/drivers/acpi/osl.c      | 34 ++++++++--------
> >  xen/include/asm-arm/setup.h |  2 +-
> >  xen/include/xen/acpi.h      |  1 +
> >  7 files changed, 123 insertions(+), 38 deletions(-)
> >
> > --
> > 2.17.1
> >
>
>
> --
> Masami Hiramatsu
Rahul Singh Sept. 29, 2020, 11:10 a.m. UTC | #5
Hello,

> On 26 Sep 2020, at 9:55 pm, Julien Grall <julien@xen.org> wrote:
> 
> From: Julien Grall <jgrall@amazon.com>
> 
> Hi all,
> 
> Xen on ARM has been broken for quite a while on ACPI systems. This
> series aims to fix it.

We also observed the panic after enabling the ACPI for ARM N1SDP board earlier.

(XEN) Xen call trace:
(XEN)    [<0000000000271b24>] set_fixmap+0x28/0x2c (PC)
(XEN)    [<0000000000271b18>] set_fixmap+0x1c/0x2c (LR)
(XEN)    [<0000000000267d58>] __acpi_map_table+0x38/0x94
(XEN)    [<00000000002501a4>] acpi_os_map_memory+0x18/0x64

After applying this patch series panic is fixed and XEN is able to boot with ACPI enabled. 

> 
> Unfortunately I don't have a system with ACPI v6.0 or later (QEMU seems
> to only support 5.1). So I did only some light testing.
> 
> I have only build tested the x86 side so far.
> 
> Cheers,
> 
> *** BLURB HERE ***
> 
> Julien Grall (4):
>  xen/acpi: Rework acpi_os_map_memory() and acpi_os_unmap_memory()
>  xen/arm: acpi: The fixmap area should always be cleared during
>    failure/unmap
>  xen/arm: Check if the platform is not using ACPI before initializing
>    Dom0less
>  xen/arm: Introduce fw_unreserved_regions() and use it
> 
> xen/arch/arm/acpi/lib.c     | 79 ++++++++++++++++++++++++++++++-------
> xen/arch/arm/kernel.c       |  2 +-
> xen/arch/arm/setup.c        | 25 +++++++++---
> xen/arch/x86/acpi/lib.c     | 18 +++++++++
> xen/drivers/acpi/osl.c      | 34 ++++++++--------
> xen/include/asm-arm/setup.h |  2 +-
> xen/include/xen/acpi.h      |  1 +
> 7 files changed, 123 insertions(+), 38 deletions(-)
> 
> -- 
> 2.17.1
> 
>
Elliott Mitchell Sept. 29, 2020, 3:28 p.m. UTC | #6
On Sat, Sep 26, 2020 at 06:47:08PM -0700, Elliott Mitchell wrote:
> On the ARM device I'm trying to get operational the boot got distinctly
> further along so appears to be a distinct ACPI improvement here.

Just in case it wasn't clear, that does amount to:

Tested-by: Elliott Mitchell <ehem+xen@m5p.com>

Now to look over a different set of shoulders and find out whether the
next problem is what they've been running into, or a distinct one...
Alex Bennée Sept. 29, 2020, 3:29 p.m. UTC | #7
Julien Grall <julien@xen.org> writes:

> From: Julien Grall <jgrall@amazon.com>
>
> Hi all,
>
> Xen on ARM has been broken for quite a while on ACPI systems. This
> series aims to fix it.
>
> Unfortunately I don't have a system with ACPI v6.0 or later (QEMU seems
> to only support 5.1). So I did only some light testing.

I was hoping to get more diagnostics out to get it working under QEMU
TCG so I think must of missed a step:

  Loading Xen 4.15-unstable ...
  Loading Linux 4.19.0-11-arm64 ...
  Loading initial ramdisk ...
  Using modules provided by bootloader in FDT
  Xen 4.15-unstable (c/s Sat Sep 26 21:55:42 2020 +0100 git:72f3d495d0) EFI loader
  ...silence...

I have a grub installed from testing on a buster base:

  dpkg --status grub-arm64-efi
  Version: 2.04-8

With:

  GRUB_CMDLINE_LINUX_DEFAULT=""
  GRUB_CMDLINE_LINUX="console=ttyAMA0"
  GRUB_CMDLINE_LINUX_XEN_REPLACE="console=hvc0 earlyprintk=xen"
  GRUB_CMDLINE_XEN="loglvl=all guest_loglvl=all com1=115200,8n1,0x3e8,5console=com1,vg"

And I built Xen with --enable-systemd and tweaked the hypervisor .config:

  CONFIG_EXPERT=y
  CONFIG_ACPI=y

So any pointers to make it more verbose would be helpful.

>
> I have only build tested the x86 side so far.
>
> Cheers,
>
> *** BLURB HERE ***
>
> Julien Grall (4):
>   xen/acpi: Rework acpi_os_map_memory() and acpi_os_unmap_memory()
>   xen/arm: acpi: The fixmap area should always be cleared during
>     failure/unmap
>   xen/arm: Check if the platform is not using ACPI before initializing
>     Dom0less
>   xen/arm: Introduce fw_unreserved_regions() and use it
>
>  xen/arch/arm/acpi/lib.c     | 79 ++++++++++++++++++++++++++++++-------
>  xen/arch/arm/kernel.c       |  2 +-
>  xen/arch/arm/setup.c        | 25 +++++++++---
>  xen/arch/x86/acpi/lib.c     | 18 +++++++++
>  xen/drivers/acpi/osl.c      | 34 ++++++++--------
>  xen/include/asm-arm/setup.h |  2 +-
>  xen/include/xen/acpi.h      |  1 +
>  7 files changed, 123 insertions(+), 38 deletions(-)
Julien Grall Sept. 29, 2020, 5:07 p.m. UTC | #8
Hi Alex,

On 29/09/2020 16:29, Alex Bennée wrote:
> 
> Julien Grall <julien@xen.org> writes:
> 
>> From: Julien Grall <jgrall@amazon.com>
>>
>> Hi all,
>>
>> Xen on ARM has been broken for quite a while on ACPI systems. This
>> series aims to fix it.
>>
>> Unfortunately I don't have a system with ACPI v6.0 or later (QEMU seems
>> to only support 5.1). So I did only some light testing.
> 
> I was hoping to get more diagnostics out to get it working under QEMU
> TCG so I think must of missed a step:
> 
>    Loading Xen 4.15-unstable ...
>    Loading Linux 4.19.0-11-arm64 ...
>    Loading initial ramdisk ...
>    Using modules provided by bootloader in FDT
>    Xen 4.15-unstable (c/s Sat Sep 26 21:55:42 2020 +0100 git:72f3d495d0) EFI loader
>    ...silence...
> 
> I have a grub installed from testing on a buster base:
> 
>    dpkg --status grub-arm64-efi
>    Version: 2.04-8
> 
> With:
> 
>    GRUB_CMDLINE_LINUX_DEFAULT=""
>    GRUB_CMDLINE_LINUX="console=ttyAMA0"
>    GRUB_CMDLINE_LINUX_XEN_REPLACE="console=hvc0 earlyprintk=xen"
>    GRUB_CMDLINE_XEN="loglvl=all guest_loglvl=all com1=115200,8n1,0x3e8,5console=com1,vg"
> 
> And I built Xen with --enable-systemd and tweaked the hypervisor .config:
> 
>    CONFIG_EXPERT=y
>    CONFIG_ACPI=y
> 
> So any pointers to make it more verbose would be helpful.

The error is hapenning before Xen setup the console. You can get early 
output on QEMU if you rebuild Xen with the following .config options:

CONFIG_DEBUG=y
CONFIG_EARLY_UART_CHOICE_PL011=y
CONFIG_EARLY_UART_PL011=y
CONFIG_EARLY_PRINTK=y
CONFIG_EARLY_UART_BASE_ADDRESS=0x09000000
CONFIG_EARLY_UART_PL011_BAUD_RATE=0
CONFIG_EARLY_PRINTK_INC="debug-pl011.inc"

Cheers,
Alex Bennée Sept. 29, 2020, 9:11 p.m. UTC | #9
Julien Grall <julien@xen.org> writes:

> Hi Alex,
>
> On 29/09/2020 16:29, Alex Bennée wrote:
>> 
>> Julien Grall <julien@xen.org> writes:
>> 
>>> From: Julien Grall <jgrall@amazon.com>
>>>
>>> Hi all,
>>>
>>> Xen on ARM has been broken for quite a while on ACPI systems. This
>>> series aims to fix it.
>>>
>>> Unfortunately I don't have a system with ACPI v6.0 or later (QEMU seems
>>> to only support 5.1). So I did only some light testing.
>> 
>> I was hoping to get more diagnostics out to get it working under QEMU
>> TCG so I think must of missed a step:
>> 
>>    Loading Xen 4.15-unstable ...
>>    Loading Linux 4.19.0-11-arm64 ...
>>    Loading initial ramdisk ...
>>    Using modules provided by bootloader in FDT
>>    Xen 4.15-unstable (c/s Sat Sep 26 21:55:42 2020 +0100 git:72f3d495d0) EFI loader
>>    ...silence...
>> 
>> I have a grub installed from testing on a buster base:
>> 
>>    dpkg --status grub-arm64-efi
>>    Version: 2.04-8
>> 
>> With:
>> 
>>    GRUB_CMDLINE_LINUX_DEFAULT=""
>>    GRUB_CMDLINE_LINUX="console=ttyAMA0"
>>    GRUB_CMDLINE_LINUX_XEN_REPLACE="console=hvc0 earlyprintk=xen"
>>    GRUB_CMDLINE_XEN="loglvl=all guest_loglvl=all com1=115200,8n1,0x3e8,5console=com1,vg"
>> 
>> And I built Xen with --enable-systemd and tweaked the hypervisor .config:
>> 
>>    CONFIG_EXPERT=y
>>    CONFIG_ACPI=y
>> 
>> So any pointers to make it more verbose would be helpful.
>
> The error is hapenning before Xen setup the console. You can get early 
> output on QEMU if you rebuild Xen with the following .config options:
>
> CONFIG_DEBUG=y
> CONFIG_EARLY_UART_CHOICE_PL011=y
> CONFIG_EARLY_UART_PL011=y
> CONFIG_EARLY_PRINTK=y
> CONFIG_EARLY_UART_BASE_ADDRESS=0x09000000
> CONFIG_EARLY_UART_PL011_BAUD_RATE=0
> CONFIG_EARLY_PRINTK_INC="debug-pl011.inc"

OK I can see it fails on the ACPI and then tries to fall back to FDT and
then fails to find the GIC:

  (XEN) CMDLINE[00000000f7bbe000]:chosen placeholder root=UUID=cf00cd3a-066b-4146-bedf-f811d3343077 ro console=hvc0 earlyprintk=xen
  (XEN)
  (XEN) Command line: placeholder loglvl=all guest_loglvl=all com1=115200,8n1,0x3e8,5console=com1,vg no-real-mode edd=off
  (XEN) parameter "placeholder" unknown!
  (XEN) parameter "no-real-mode" unknown!
  (XEN) parameter "edd" unknown!
  (XEN) ACPI: RSDP 138560000, 0024 (r2 BOCHS )
  (XEN) ACPI: XSDT 138550000, 004C (r1 BOCHS  BXPCFACP        1       1000013)
  (XEN) ACPI: FACP 138510000, 010C (r5 BOCHS  BXPCFACP        1 BXPC        1)
  (XEN) ACPI: DSDT 138520000, 14A6 (r2 BOCHS  BXPCDSDT        1 BXPC        1)
  (XEN) ACPI: APIC 138500000, 018C (r3 BOCHS  BXPCAPIC        1 BXPC        1)
  (XEN) ACPI: GTDT 1384F0000, 0060 (r2 BOCHS  BXPCGTDT        1 BXPC        1)
  (XEN) ACPI: MCFG 1384E0000, 003C (r1 BOCHS  BXPCMCFG        1 BXPC        1)
  (XEN) ACPI: SPCR 1384D0000, 0050 (r2 BOCHS  BXPCSPCR        1 BXPC        1)
  (XEN) Unsupported FADT revision 5.1, should be 6.0+, will disable ACPI
  (XEN) acpi_boot_table_init: FADT not found (-22)
  (XEN) Domain heap initialised
  (XEN) Booting using Device Tree
  (XEN) Platform: Generic System
  (XEN)
  (XEN) ****************************************
  (XEN) Panic on CPU 0:
  (XEN) Unable to find compatible GIC in the device tree
  (XEN) ****************************************
  (XEN)
  (XEN) Reboot in five seconds...

Despite saying it is going to reboot it never manages to. Any idea how
it is trying to reset the system?
Andre Przywara Sept. 29, 2020, 11:39 p.m. UTC | #10
On 29/09/2020 22:11, Alex Bennée wrote:

Hi,

> Julien Grall <julien@xen.org> writes:
> 
>> Hi Alex,
>>
>> On 29/09/2020 16:29, Alex Bennée wrote:
>>>
>>> Julien Grall <julien@xen.org> writes:
>>>
>>>> From: Julien Grall <jgrall@amazon.com>
>>>>
>>>> Hi all,
>>>>
>>>> Xen on ARM has been broken for quite a while on ACPI systems. This
>>>> series aims to fix it.
>>>>
>>>> Unfortunately I don't have a system with ACPI v6.0 or later (QEMU seems
>>>> to only support 5.1).

Does QEMU provide ACPI tables? Or is this actually EDK2 generating them?

> So I did only some light testing.

So about that v6.0 or later: it seems like the requirement comes from:
commit 1c9bd43019cd "arm/acpi: Parse FADT table and get PSCI flags":
"Since STAO table and the GIC version are introduced by ACPI 6.0, we
will check the version and only parse FADT table with version >= 6.0. If
firmware provides ACPI tables with ACPI version less than 6.0, OS will
be messed up with those information, so disable ACPI if we get an FADT
table with version less than 6.0."

STAO (and XENV) have indeed been introduced by ACPI v6.0, but those
tables are supposed to be *generated* by Xen and handed down to Dom0,
they will never be provided by firmware (or parsed) by the Xen
hypervisor. Checking the Linux code it seems to actually not (yet?)
support the STAO named list, and currently finds and uses the STAO (UART
block bit) regardless of the FADT version number.

I can't find anything GIC related in the FADT, the whole GIC information
is described in MADT.

Linux/arm64 seems to be happy with ACPI >= v5.1:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/kernel/acpi.c#n148

So can we relax the v6.0 requirement, to be actually >= v5.1? That is
among the first to support ARM anyway, IIRC.

I have only a smattering about ACPI, so happy to stand corrected.

Cheers,
Andre

>>>
>>> I was hoping to get more diagnostics out to get it working under QEMU
>>> TCG so I think must of missed a step:
>>>
>>>    Loading Xen 4.15-unstable ...
>>>    Loading Linux 4.19.0-11-arm64 ...
>>>    Loading initial ramdisk ...
>>>    Using modules provided by bootloader in FDT
>>>    Xen 4.15-unstable (c/s Sat Sep 26 21:55:42 2020 +0100 git:72f3d495d0) EFI loader
>>>    ...silence...
>>>
>>> I have a grub installed from testing on a buster base:
>>>
>>>    dpkg --status grub-arm64-efi
>>>    Version: 2.04-8
>>>
>>> With:
>>>
>>>    GRUB_CMDLINE_LINUX_DEFAULT=""
>>>    GRUB_CMDLINE_LINUX="console=ttyAMA0"
>>>    GRUB_CMDLINE_LINUX_XEN_REPLACE="console=hvc0 earlyprintk=xen"
>>>    GRUB_CMDLINE_XEN="loglvl=all guest_loglvl=all com1=115200,8n1,0x3e8,5console=com1,vg"
>>>
>>> And I built Xen with --enable-systemd and tweaked the hypervisor .config:
>>>
>>>    CONFIG_EXPERT=y
>>>    CONFIG_ACPI=y
>>>
>>> So any pointers to make it more verbose would be helpful.
>>
>> The error is hapenning before Xen setup the console. You can get early 
>> output on QEMU if you rebuild Xen with the following .config options:
>>
>> CONFIG_DEBUG=y
>> CONFIG_EARLY_UART_CHOICE_PL011=y
>> CONFIG_EARLY_UART_PL011=y
>> CONFIG_EARLY_PRINTK=y
>> CONFIG_EARLY_UART_BASE_ADDRESS=0x09000000
>> CONFIG_EARLY_UART_PL011_BAUD_RATE=0
>> CONFIG_EARLY_PRINTK_INC="debug-pl011.inc"
> 
> OK I can see it fails on the ACPI and then tries to fall back to FDT and
> then fails to find the GIC:
> 
>   (XEN) CMDLINE[00000000f7bbe000]:chosen placeholder root=UUID=cf00cd3a-066b-4146-bedf-f811d3343077 ro console=hvc0 earlyprintk=xen
>   (XEN)
>   (XEN) Command line: placeholder loglvl=all guest_loglvl=all com1=115200,8n1,0x3e8,5console=com1,vg no-real-mode edd=off
>   (XEN) parameter "placeholder" unknown!
>   (XEN) parameter "no-real-mode" unknown!
>   (XEN) parameter "edd" unknown!
>   (XEN) ACPI: RSDP 138560000, 0024 (r2 BOCHS )
>   (XEN) ACPI: XSDT 138550000, 004C (r1 BOCHS  BXPCFACP        1       1000013)
>   (XEN) ACPI: FACP 138510000, 010C (r5 BOCHS  BXPCFACP        1 BXPC        1)
>   (XEN) ACPI: DSDT 138520000, 14A6 (r2 BOCHS  BXPCDSDT        1 BXPC        1)
>   (XEN) ACPI: APIC 138500000, 018C (r3 BOCHS  BXPCAPIC        1 BXPC        1)
>   (XEN) ACPI: GTDT 1384F0000, 0060 (r2 BOCHS  BXPCGTDT        1 BXPC        1)
>   (XEN) ACPI: MCFG 1384E0000, 003C (r1 BOCHS  BXPCMCFG        1 BXPC        1)
>   (XEN) ACPI: SPCR 1384D0000, 0050 (r2 BOCHS  BXPCSPCR        1 BXPC        1)
>   (XEN) Unsupported FADT revision 5.1, should be 6.0+, will disable ACPI
>   (XEN) acpi_boot_table_init: FADT not found (-22)
>   (XEN) Domain heap initialised
>   (XEN) Booting using Device Tree
>   (XEN) Platform: Generic System
>   (XEN)
>   (XEN) ****************************************
>   (XEN) Panic on CPU 0:
>   (XEN) Unable to find compatible GIC in the device tree
>   (XEN) ****************************************
>   (XEN)
>   (XEN) Reboot in five seconds...
> 
> Despite saying it is going to reboot it never manages to. Any idea how
> it is trying to reset the system?
>
Alex Bennée Sept. 30, 2020, 8:51 a.m. UTC | #11
André Przywara <andre.przywara@arm.com> writes:

> On 29/09/2020 22:11, Alex Bennée wrote:
>
> Hi,
>
>> Julien Grall <julien@xen.org> writes:
>> 
>>> Hi Alex,
>>>
>>> On 29/09/2020 16:29, Alex Bennée wrote:
>>>>
>>>> Julien Grall <julien@xen.org> writes:
>>>>
>>>>> From: Julien Grall <jgrall@amazon.com>
>>>>>
>>>>> Hi all,
>>>>>
>>>>> Xen on ARM has been broken for quite a while on ACPI systems. This
>>>>> series aims to fix it.
>>>>>
>>>>> Unfortunately I don't have a system with ACPI v6.0 or later (QEMU seems
>>>>> to only support 5.1).
>
> Does QEMU provide ACPI tables? Or is this actually EDK2 generating
> them?

It certainly does - whether EDK2 mangles them afterwards is another
question. For the -M virt machine we currently generate:

  /* FADT */
  static void build_fadt_rev5(GArray *table_data, BIOSLinker *linker,
                              VirtMachineState *vms, unsigned dsdt_tbl_offset)
  {
      /* ACPI v5.1 */
      AcpiFadtData fadt = {
          .rev = 5,
          .minor_ver = 1,
          .flags = 1 << ACPI_FADT_F_HW_REDUCED_ACPI,
          .xdsdt_tbl_offset = &dsdt_tbl_offset,
      };

      switch (vms->psci_conduit) {
      case QEMU_PSCI_CONDUIT_DISABLED:
          fadt.arm_boot_arch = 0;
          break;
      case QEMU_PSCI_CONDUIT_HVC:
          fadt.arm_boot_arch = ACPI_FADT_ARM_PSCI_COMPLIANT |
                               ACPI_FADT_ARM_PSCI_USE_HVC;
          break;
      case QEMU_PSCI_CONDUIT_SMC:
          fadt.arm_boot_arch = ACPI_FADT_ARM_PSCI_COMPLIANT;
          break;
      default:
          g_assert_not_reached();
      }

      build_fadt(table_data, linker, &fadt, NULL, NULL);
  }

Looking through the code it seems we stop around 5.0 - even the new
fancy x86 microvm ACPI tables are described as /* ACPI 5.0: 4.1
Hardware-Reduced ACPI */.

>
>> So I did only some light testing.
>
> So about that v6.0 or later: it seems like the requirement comes from:
> commit 1c9bd43019cd "arm/acpi: Parse FADT table and get PSCI flags":
> "Since STAO table and the GIC version are introduced by ACPI 6.0, we
> will check the version and only parse FADT table with version >= 6.0. If
> firmware provides ACPI tables with ACPI version less than 6.0, OS will
> be messed up with those information, so disable ACPI if we get an FADT
> table with version less than 6.0."
>
> STAO (and XENV) have indeed been introduced by ACPI v6.0, but those
> tables are supposed to be *generated* by Xen and handed down to Dom0,
> they will never be provided by firmware (or parsed) by the Xen
> hypervisor. Checking the Linux code it seems to actually not (yet?)
> support the STAO named list, and currently finds and uses the STAO (UART
> block bit) regardless of the FADT version number.
>
> I can't find anything GIC related in the FADT, the whole GIC information
> is described in MADT.

I think we are generating full data for both GIC2 and GIC3:

  /* MADT */
  static void
  build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
  {
      VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
      int madt_start = table_data->len;
      const MemMapEntry *memmap = vms->memmap;
      const int *irqmap = vms->irqmap;
      AcpiMadtGenericDistributor *gicd;
      AcpiMadtGenericMsiFrame *gic_msi;
      int i;

      acpi_data_push(table_data, sizeof(AcpiMultipleApicTable));

      gicd = acpi_data_push(table_data, sizeof *gicd);
      gicd->type = ACPI_APIC_GENERIC_DISTRIBUTOR;
      gicd->length = sizeof(*gicd);
      gicd->base_address = cpu_to_le64(memmap[VIRT_GIC_DIST].base);
      gicd->version = vms->gic_version;

      for (i = 0; i < vms->smp_cpus; i++) {
          AcpiMadtGenericCpuInterface *gicc = acpi_data_push(table_data,
                                                             sizeof(*gicc));
          ARMCPU *armcpu = ARM_CPU(qemu_get_cpu(i));

          gicc->type = ACPI_APIC_GENERIC_CPU_INTERFACE;
          gicc->length = sizeof(*gicc);
          if (vms->gic_version == 2) {
              gicc->base_address = cpu_to_le64(memmap[VIRT_GIC_CPU].base);
              gicc->gich_base_address = cpu_to_le64(memmap[VIRT_GIC_HYP].base);
              gicc->gicv_base_address = cpu_to_le64(memmap[VIRT_GIC_VCPU].base);
          }
          gicc->cpu_interface_number = cpu_to_le32(i);
          gicc->arm_mpidr = cpu_to_le64(armcpu->mp_affinity);
          gicc->uid = cpu_to_le32(i);
          gicc->flags = cpu_to_le32(ACPI_MADT_GICC_ENABLED);

          if (arm_feature(&armcpu->env, ARM_FEATURE_PMU)) {
              gicc->performance_interrupt = cpu_to_le32(PPI(VIRTUAL_PMU_IRQ));
          }
          if (vms->virt) {
              gicc->vgic_interrupt = cpu_to_le32(PPI(ARCH_GIC_MAINT_IRQ));
          }
      }

      if (vms->gic_version == 3) {
          AcpiMadtGenericTranslator *gic_its;
          int nb_redist_regions = virt_gicv3_redist_region_count(vms);
          AcpiMadtGenericRedistributor *gicr = acpi_data_push(table_data,
                                                           sizeof *gicr);

          gicr->type = ACPI_APIC_GENERIC_REDISTRIBUTOR;
          gicr->length = sizeof(*gicr);
          gicr->base_address = cpu_to_le64(memmap[VIRT_GIC_REDIST].base);
          gicr->range_length = cpu_to_le32(memmap[VIRT_GIC_REDIST].size);

          if (nb_redist_regions == 2) {
              gicr = acpi_data_push(table_data, sizeof(*gicr));
              gicr->type = ACPI_APIC_GENERIC_REDISTRIBUTOR;
              gicr->length = sizeof(*gicr);
              gicr->base_address =
                  cpu_to_le64(memmap[VIRT_HIGH_GIC_REDIST2].base);
              gicr->range_length =
                  cpu_to_le32(memmap[VIRT_HIGH_GIC_REDIST2].size);
          }

          if (its_class_name() && !vmc->no_its) {
              gic_its = acpi_data_push(table_data, sizeof *gic_its);
              gic_its->type = ACPI_APIC_GENERIC_TRANSLATOR;
              gic_its->length = sizeof(*gic_its);
              gic_its->translation_id = 0;
              gic_its->base_address = cpu_to_le64(memmap[VIRT_GIC_ITS].base);
          }
      } else {
          gic_msi = acpi_data_push(table_data, sizeof *gic_msi);
          gic_msi->type = ACPI_APIC_GENERIC_MSI_FRAME;
          gic_msi->length = sizeof(*gic_msi);
          gic_msi->gic_msi_frame_id = 0;
          gic_msi->base_address = cpu_to_le64(memmap[VIRT_GIC_V2M].base);
          gic_msi->flags = cpu_to_le32(1);
          gic_msi->spi_count = cpu_to_le16(NUM_GICV2M_SPIS);
          gic_msi->spi_base = cpu_to_le16(irqmap[VIRT_GIC_V2M] + ARM_SPI_BASE);
      }

      build_header(linker, table_data,
                   (void *)(table_data->data + madt_start), "APIC",
                   table_data->len - madt_start, 3, NULL, NULL);
  }


>
> Linux/arm64 seems to be happy with ACPI >= v5.1:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/kernel/acpi.c#n148
>
> So can we relax the v6.0 requirement, to be actually >= v5.1? That is
> among the first to support ARM anyway, IIRC.
>
> I have only a smattering about ACPI, so happy to stand corrected.

I'd certainly be happy with that as reduces the number of components
that need to be uprevved to get support.

>
> Cheers,
> Andre
>
>>>>
>>>> I was hoping to get more diagnostics out to get it working under QEMU
>>>> TCG so I think must of missed a step:
>>>>
>>>>    Loading Xen 4.15-unstable ...
>>>>    Loading Linux 4.19.0-11-arm64 ...
>>>>    Loading initial ramdisk ...
>>>>    Using modules provided by bootloader in FDT
>>>>    Xen 4.15-unstable (c/s Sat Sep 26 21:55:42 2020 +0100 git:72f3d495d0) EFI loader
>>>>    ...silence...
>>>>
>>>> I have a grub installed from testing on a buster base:
>>>>
>>>>    dpkg --status grub-arm64-efi
>>>>    Version: 2.04-8
>>>>
>>>> With:
>>>>
>>>>    GRUB_CMDLINE_LINUX_DEFAULT=""
>>>>    GRUB_CMDLINE_LINUX="console=ttyAMA0"
>>>>    GRUB_CMDLINE_LINUX_XEN_REPLACE="console=hvc0 earlyprintk=xen"
>>>>    GRUB_CMDLINE_XEN="loglvl=all guest_loglvl=all com1=115200,8n1,0x3e8,5console=com1,vg"
>>>>
>>>> And I built Xen with --enable-systemd and tweaked the hypervisor .config:
>>>>
>>>>    CONFIG_EXPERT=y
>>>>    CONFIG_ACPI=y
>>>>
>>>> So any pointers to make it more verbose would be helpful.
>>>
>>> The error is hapenning before Xen setup the console. You can get early 
>>> output on QEMU if you rebuild Xen with the following .config options:
>>>
>>> CONFIG_DEBUG=y
>>> CONFIG_EARLY_UART_CHOICE_PL011=y
>>> CONFIG_EARLY_UART_PL011=y
>>> CONFIG_EARLY_PRINTK=y
>>> CONFIG_EARLY_UART_BASE_ADDRESS=0x09000000
>>> CONFIG_EARLY_UART_PL011_BAUD_RATE=0
>>> CONFIG_EARLY_PRINTK_INC="debug-pl011.inc"
>> 
>> OK I can see it fails on the ACPI and then tries to fall back to FDT and
>> then fails to find the GIC:
>> 
>>   (XEN) CMDLINE[00000000f7bbe000]:chosen placeholder root=UUID=cf00cd3a-066b-4146-bedf-f811d3343077 ro console=hvc0 earlyprintk=xen
>>   (XEN)
>>   (XEN) Command line: placeholder loglvl=all guest_loglvl=all com1=115200,8n1,0x3e8,5console=com1,vg no-real-mode edd=off
>>   (XEN) parameter "placeholder" unknown!
>>   (XEN) parameter "no-real-mode" unknown!
>>   (XEN) parameter "edd" unknown!
>>   (XEN) ACPI: RSDP 138560000, 0024 (r2 BOCHS )
>>   (XEN) ACPI: XSDT 138550000, 004C (r1 BOCHS  BXPCFACP        1       1000013)
>>   (XEN) ACPI: FACP 138510000, 010C (r5 BOCHS  BXPCFACP        1 BXPC        1)
>>   (XEN) ACPI: DSDT 138520000, 14A6 (r2 BOCHS  BXPCDSDT        1 BXPC        1)
>>   (XEN) ACPI: APIC 138500000, 018C (r3 BOCHS  BXPCAPIC        1 BXPC        1)
>>   (XEN) ACPI: GTDT 1384F0000, 0060 (r2 BOCHS  BXPCGTDT        1 BXPC        1)
>>   (XEN) ACPI: MCFG 1384E0000, 003C (r1 BOCHS  BXPCMCFG        1 BXPC        1)
>>   (XEN) ACPI: SPCR 1384D0000, 0050 (r2 BOCHS  BXPCSPCR        1 BXPC        1)
>>   (XEN) Unsupported FADT revision 5.1, should be 6.0+, will disable ACPI
>>   (XEN) acpi_boot_table_init: FADT not found (-22)
>>   (XEN) Domain heap initialised
>>   (XEN) Booting using Device Tree
>>   (XEN) Platform: Generic System
>>   (XEN)
>>   (XEN) ****************************************
>>   (XEN) Panic on CPU 0:
>>   (XEN) Unable to find compatible GIC in the device tree
>>   (XEN) ****************************************
>>   (XEN)
>>   (XEN) Reboot in five seconds...
>> 
>> Despite saying it is going to reboot it never manages to. Any idea how
>> it is trying to reset the system?
>>
Julien Grall Sept. 30, 2020, 9:42 a.m. UTC | #12
Hi Alex,

On 29/09/2020 22:11, Alex Bennée wrote:
> 
> Julien Grall <julien@xen.org> writes:
> 
>> Hi Alex,
>>
>> On 29/09/2020 16:29, Alex Bennée wrote:
>>>
>>> Julien Grall <julien@xen.org> writes:
>>>
>>>> From: Julien Grall <jgrall@amazon.com>
>>>>
>>>> Hi all,
>>>>
>>>> Xen on ARM has been broken for quite a while on ACPI systems. This
>>>> series aims to fix it.
>>>>
>>>> Unfortunately I don't have a system with ACPI v6.0 or later (QEMU seems
>>>> to only support 5.1). So I did only some light testing.
>>>
>>> I was hoping to get more diagnostics out to get it working under QEMU
>>> TCG so I think must of missed a step:
>>>
>>>     Loading Xen 4.15-unstable ...
>>>     Loading Linux 4.19.0-11-arm64 ...
>>>     Loading initial ramdisk ...
>>>     Using modules provided by bootloader in FDT
>>>     Xen 4.15-unstable (c/s Sat Sep 26 21:55:42 2020 +0100 git:72f3d495d0) EFI loader
>>>     ...silence...
>>>
>>> I have a grub installed from testing on a buster base:
>>>
>>>     dpkg --status grub-arm64-efi
>>>     Version: 2.04-8
>>>
>>> With:
>>>
>>>     GRUB_CMDLINE_LINUX_DEFAULT=""
>>>     GRUB_CMDLINE_LINUX="console=ttyAMA0"
>>>     GRUB_CMDLINE_LINUX_XEN_REPLACE="console=hvc0 earlyprintk=xen"
>>>     GRUB_CMDLINE_XEN="loglvl=all guest_loglvl=all com1=115200,8n1,0x3e8,5console=com1,vg"
>>>
>>> And I built Xen with --enable-systemd and tweaked the hypervisor .config:
>>>
>>>     CONFIG_EXPERT=y
>>>     CONFIG_ACPI=y
>>>
>>> So any pointers to make it more verbose would be helpful.
>>
>> The error is hapenning before Xen setup the console. You can get early
>> output on QEMU if you rebuild Xen with the following .config options:
>>
>> CONFIG_DEBUG=y
>> CONFIG_EARLY_UART_CHOICE_PL011=y
>> CONFIG_EARLY_UART_PL011=y
>> CONFIG_EARLY_PRINTK=y
>> CONFIG_EARLY_UART_BASE_ADDRESS=0x09000000
>> CONFIG_EARLY_UART_PL011_BAUD_RATE=0
>> CONFIG_EARLY_PRINTK_INC="debug-pl011.inc"
> 
> OK I can see it fails on the ACPI and then tries to fall back to FDT and
> then fails to find the GIC:
> 
>    (XEN) CMDLINE[00000000f7bbe000]:chosen placeholder root=UUID=cf00cd3a-066b-4146-bedf-f811d3343077 ro console=hvc0 earlyprintk=xen
>    (XEN)
>    (XEN) Command line: placeholder loglvl=all guest_loglvl=all com1=115200,8n1,0x3e8,5console=com1,vg no-real-mode edd=off
>    (XEN) parameter "placeholder" unknown!
>    (XEN) parameter "no-real-mode" unknown!
>    (XEN) parameter "edd" unknown!
>    (XEN) ACPI: RSDP 138560000, 0024 (r2 BOCHS )
>    (XEN) ACPI: XSDT 138550000, 004C (r1 BOCHS  BXPCFACP        1       1000013)
>    (XEN) ACPI: FACP 138510000, 010C (r5 BOCHS  BXPCFACP        1 BXPC        1)
>    (XEN) ACPI: DSDT 138520000, 14A6 (r2 BOCHS  BXPCDSDT        1 BXPC        1)
>    (XEN) ACPI: APIC 138500000, 018C (r3 BOCHS  BXPCAPIC        1 BXPC        1)
>    (XEN) ACPI: GTDT 1384F0000, 0060 (r2 BOCHS  BXPCGTDT        1 BXPC        1)
>    (XEN) ACPI: MCFG 1384E0000, 003C (r1 BOCHS  BXPCMCFG        1 BXPC        1)
>    (XEN) ACPI: SPCR 1384D0000, 0050 (r2 BOCHS  BXPCSPCR        1 BXPC        1)
>    (XEN) Unsupported FADT revision 5.1, should be 6.0+, will disable ACPI
>    (XEN) acpi_boot_table_init: FADT not found (-22)
>    (XEN) Domain heap initialised
>    (XEN) Booting using Device Tree
>    (XEN) Platform: Generic System
>    (XEN)
>    (XEN) ****************************************
>    (XEN) Panic on CPU 0:
>    (XEN) Unable to find compatible GIC in the device tree
>    (XEN) ****************************************
>    (XEN)
>    (XEN) Reboot in five seconds...
> 
> Despite saying it is going to reboot it never manages to. Any idea how
> it is trying to reset the system?

This is a bit of chicken and eggs problem. To know the reset method, you 
need to parse the ACPI tables. As we can't parse then we don't know the 
reset method. So, Xen will just do an infinite loop.

It would probably be good to be more forthcoming with the users and say 
it will not reboot.

Also, IIRC, the time subsystem is not yet initialized. So it might be 
possible to mdelay() doesn't work properly.

Cheers,
Julien Grall Sept. 30, 2020, 10:35 a.m. UTC | #13
(Removing the x86 folks from the CC list)

Hi André,

On 30/09/2020 00:39, André Przywara wrote:
> On 29/09/2020 22:11, Alex Bennée wrote:
> 
> Hi,
> 
>> Julien Grall <julien@xen.org> writes:
>>
>>> Hi Alex,
>>>
>>> On 29/09/2020 16:29, Alex Bennée wrote:
>>>>
>>>> Julien Grall <julien@xen.org> writes:
>>>>
>>>>> From: Julien Grall <jgrall@amazon.com>
>>>>>
>>>>> Hi all,
>>>>>
>>>>> Xen on ARM has been broken for quite a while on ACPI systems. This
>>>>> series aims to fix it.
>>>>>
>>>>> Unfortunately I don't have a system with ACPI v6.0 or later (QEMU seems
>>>>> to only support 5.1).
> 
> Does QEMU provide ACPI tables? Or is this actually EDK2 generating them?
> 
>> So I did only some light testing.
> 
> So about that v6.0 or later: it seems like the requirement comes from:
> commit 1c9bd43019cd "arm/acpi: Parse FADT table and get PSCI flags":
> "Since STAO table and the GIC version are introduced by ACPI 6.0, we
> will check the version and only parse FADT table with version >= 6.0. If
> firmware provides ACPI tables with ACPI version less than 6.0, OS will
> be messed up with those information, so disable ACPI if we get an FADT
> table with version less than 6.0."
> 
> STAO (and XENV) have indeed been introduced by ACPI v6.0, but those
> tables are supposed to be *generated* by Xen and handed down to Dom0,
> they will never be provided by firmware (or parsed) by the Xen
> hypervisor. Checking the Linux code it seems to actually not (yet?)
> support the STAO named list,

I may be because we don't yet use the named list in Xen. I am not sure 
if other hypervisor ever used the STAO.

> and currently finds and uses the STAO (UART
> block bit) regardless of the FADT version number.

IIRC, the concern at the time is an OS may decide to bail out if it 
finds a table that is not meant to exist.

> I can't find anything GIC related in the FADT, the whole GIC information
> is described in MADT.

My memory is quite fuzzy... IIRC the problem is (was?) related to how 
Linux used to check the size of the MADT.

Before commit [1], Linux was checking that the MADT entry was matching 
the ACPI version. As Xen generates the MADT using the ACPI 6.0 layout, 
it wouldn't be possible to boot Linux.

> 
> Linux/arm64 seems to be happy with ACPI >= v5.1:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/kernel/acpi.c#n148
> 
> So can we relax the v6.0 requirement, to be actually >= v5.1? That is
> among the first to support ARM anyway, IIRC.

I remember trying to relax the requiring in the past. However, I can't 
remember why I didn't upstream it. There was possibly some issues I 
could not get around?

I think supporting ACPI 5.1 would be useful. So I would suggest to 
attempt it again and see what breaks.

Cheers,

[1]
commit 9eb1c92b47c73249465d388eaa394fe436a3b489
Author: Jeremy Linton <jeremy.linton@arm.com>
Date:   Tue Nov 27 17:59:12 2018 +0000

     arm64: acpi: Prepare for longer MADTs

     The BAD_MADT_GICC_ENTRY check is a little too strict because
     it rejects MADT entries that don't match the currently known
     lengths. We should remove this restriction to avoid problems
     if the table length changes. Future code which might depend on
     additional fields should be written to validate those fields
     before using them, rather than trying to globally check
     known MADT version lengths.

     Link: 
https://lkml.kernel.org/r/20181012192937.3819951-1-jeremy.linton@arm.com
     Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
     [lorenzo.pieralisi@arm.com: added MADT macro comments]
     Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
     Acked-by: Sudeep Holla <sudeep.holla@arm.com>
     Cc: Will Deacon <will.deacon@arm.com>
     Cc: Catalin Marinas <catalin.marinas@arm.com>
     Cc: Al Stone <ahs3@redhat.com>
     Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
     Signed-off-by: Will Deacon <will.deacon@arm.com>
Alex Bennée Sept. 30, 2020, 10:38 a.m. UTC | #14
Julien Grall <julien@xen.org> writes:

> Hi Alex,
>
> On 29/09/2020 22:11, Alex Bennée wrote:
>> 
>> Julien Grall <julien@xen.org> writes:
>> 
>>> Hi Alex,
>>>
>>> On 29/09/2020 16:29, Alex Bennée wrote:
>>>>
>>>> Julien Grall <julien@xen.org> writes:
>>>>
>>>>> From: Julien Grall <jgrall@amazon.com>
>>>>>
>>>>> Hi all,
>>>>>
>>>>> Xen on ARM has been broken for quite a while on ACPI systems. This
>>>>> series aims to fix it.
>>>>>
>>>>> Unfortunately I don't have a system with ACPI v6.0 or later (QEMU seems
>>>>> to only support 5.1). So I did only some light testing.
>>>>
>>>> I was hoping to get more diagnostics out to get it working under QEMU
>>>> TCG so I think must of missed a step:
>>>>
>>>>     Loading Xen 4.15-unstable ...
>>>>     Loading Linux 4.19.0-11-arm64 ...
>>>>     Loading initial ramdisk ...
>>>>     Using modules provided by bootloader in FDT
>>>>     Xen 4.15-unstable (c/s Sat Sep 26 21:55:42 2020 +0100 git:72f3d495d0) EFI loader
>>>>     ...silence...
>>>>
>>>> I have a grub installed from testing on a buster base:
>>>>
>>>>     dpkg --status grub-arm64-efi
>>>>     Version: 2.04-8
>>>>
>>>> With:
>>>>
>>>>     GRUB_CMDLINE_LINUX_DEFAULT=""
>>>>     GRUB_CMDLINE_LINUX="console=ttyAMA0"
>>>>     GRUB_CMDLINE_LINUX_XEN_REPLACE="console=hvc0 earlyprintk=xen"
>>>>     GRUB_CMDLINE_XEN="loglvl=all guest_loglvl=all com1=115200,8n1,0x3e8,5console=com1,vg"
>>>>
>>>> And I built Xen with --enable-systemd and tweaked the hypervisor .config:
>>>>
>>>>     CONFIG_EXPERT=y
>>>>     CONFIG_ACPI=y
>>>>
>>>> So any pointers to make it more verbose would be helpful.
>>>
>>> The error is hapenning before Xen setup the console. You can get early
>>> output on QEMU if you rebuild Xen with the following .config options:
>>>
>>> CONFIG_DEBUG=y
>>> CONFIG_EARLY_UART_CHOICE_PL011=y
>>> CONFIG_EARLY_UART_PL011=y
>>> CONFIG_EARLY_PRINTK=y
>>> CONFIG_EARLY_UART_BASE_ADDRESS=0x09000000
>>> CONFIG_EARLY_UART_PL011_BAUD_RATE=0
>>> CONFIG_EARLY_PRINTK_INC="debug-pl011.inc"
>> 
>> OK I can see it fails on the ACPI and then tries to fall back to FDT and
>> then fails to find the GIC:
>> 
>>    (XEN) CMDLINE[00000000f7bbe000]:chosen placeholder root=UUID=cf00cd3a-066b-4146-bedf-f811d3343077 ro console=hvc0 earlyprintk=xen
>>    (XEN)
>>    (XEN) Command line: placeholder loglvl=all guest_loglvl=all com1=115200,8n1,0x3e8,5console=com1,vg no-real-mode edd=off
>>    (XEN) parameter "placeholder" unknown!
>>    (XEN) parameter "no-real-mode" unknown!
>>    (XEN) parameter "edd" unknown!
>>    (XEN) ACPI: RSDP 138560000, 0024 (r2 BOCHS )
>>    (XEN) ACPI: XSDT 138550000, 004C (r1 BOCHS  BXPCFACP        1       1000013)
>>    (XEN) ACPI: FACP 138510000, 010C (r5 BOCHS  BXPCFACP        1 BXPC        1)
>>    (XEN) ACPI: DSDT 138520000, 14A6 (r2 BOCHS  BXPCDSDT        1 BXPC        1)
>>    (XEN) ACPI: APIC 138500000, 018C (r3 BOCHS  BXPCAPIC        1 BXPC        1)
>>    (XEN) ACPI: GTDT 1384F0000, 0060 (r2 BOCHS  BXPCGTDT        1 BXPC        1)
>>    (XEN) ACPI: MCFG 1384E0000, 003C (r1 BOCHS  BXPCMCFG        1 BXPC        1)
>>    (XEN) ACPI: SPCR 1384D0000, 0050 (r2 BOCHS  BXPCSPCR        1 BXPC        1)
>>    (XEN) Unsupported FADT revision 5.1, should be 6.0+, will disable ACPI
>>    (XEN) acpi_boot_table_init: FADT not found (-22)
>>    (XEN) Domain heap initialised
>>    (XEN) Booting using Device Tree
>>    (XEN) Platform: Generic System
>>    (XEN)
>>    (XEN) ****************************************
>>    (XEN) Panic on CPU 0:
>>    (XEN) Unable to find compatible GIC in the device tree
>>    (XEN) ****************************************
>>    (XEN)
>>    (XEN) Reboot in five seconds...
>> 
>> Despite saying it is going to reboot it never manages to. Any idea how
>> it is trying to reset the system?
>
> This is a bit of chicken and eggs problem. To know the reset method, you 
> need to parse the ACPI tables. As we can't parse then we don't know the 
> reset method. So, Xen will just do an infinite loop.

Well you do get some ACPI tables - downgrading the minimum at least
restores the reset method detection. I wonder if it would be worth
defaulting to PSCI if you don't know rather than hang indefinitely?

FWIW the failure after that is failing to find the GIC - I'm just
looking at the MADT table parsing now. Why am I getting a sense of
DejaVu?

> It would probably be good to be more forthcoming with the users and say 
> it will not reboot.
>
> Also, IIRC, the time subsystem is not yet initialized. So it might be 
> possible to mdelay() doesn't work properly.

Surely that's an architectural subsystem so there is no reason that
couldn't be up and running.

>
> Cheers,
Julien Grall Sept. 30, 2020, 11:10 a.m. UTC | #15
Hi Alex,

On 30/09/2020 11:38, Alex Bennée wrote:
> 
> Julien Grall <julien@xen.org> writes:
> 
>> Hi Alex,
>>
>> On 29/09/2020 22:11, Alex Bennée wrote:
>>>
>>> Julien Grall <julien@xen.org> writes:
>>>
>>>> Hi Alex,
>>>>
>>>> On 29/09/2020 16:29, Alex Bennée wrote:
>>>>>
>>>>> Julien Grall <julien@xen.org> writes:
>>>>>
>>>>>> From: Julien Grall <jgrall@amazon.com>
>>>>>>
>>>>>> Hi all,
>>>>>>
>>>>>> Xen on ARM has been broken for quite a while on ACPI systems. This
>>>>>> series aims to fix it.
>>>>>>
>>>>>> Unfortunately I don't have a system with ACPI v6.0 or later (QEMU seems
>>>>>> to only support 5.1). So I did only some light testing.
>>>>>
>>>>> I was hoping to get more diagnostics out to get it working under QEMU
>>>>> TCG so I think must of missed a step:
>>>>>
>>>>>      Loading Xen 4.15-unstable ...
>>>>>      Loading Linux 4.19.0-11-arm64 ...
>>>>>      Loading initial ramdisk ...
>>>>>      Using modules provided by bootloader in FDT
>>>>>      Xen 4.15-unstable (c/s Sat Sep 26 21:55:42 2020 +0100 git:72f3d495d0) EFI loader
>>>>>      ...silence...
>>>>>
>>>>> I have a grub installed from testing on a buster base:
>>>>>
>>>>>      dpkg --status grub-arm64-efi
>>>>>      Version: 2.04-8
>>>>>
>>>>> With:
>>>>>
>>>>>      GRUB_CMDLINE_LINUX_DEFAULT=""
>>>>>      GRUB_CMDLINE_LINUX="console=ttyAMA0"
>>>>>      GRUB_CMDLINE_LINUX_XEN_REPLACE="console=hvc0 earlyprintk=xen"
>>>>>      GRUB_CMDLINE_XEN="loglvl=all guest_loglvl=all com1=115200,8n1,0x3e8,5console=com1,vg"
>>>>>
>>>>> And I built Xen with --enable-systemd and tweaked the hypervisor .config:
>>>>>
>>>>>      CONFIG_EXPERT=y
>>>>>      CONFIG_ACPI=y
>>>>>
>>>>> So any pointers to make it more verbose would be helpful.
>>>>
>>>> The error is hapenning before Xen setup the console. You can get early
>>>> output on QEMU if you rebuild Xen with the following .config options:
>>>>
>>>> CONFIG_DEBUG=y
>>>> CONFIG_EARLY_UART_CHOICE_PL011=y
>>>> CONFIG_EARLY_UART_PL011=y
>>>> CONFIG_EARLY_PRINTK=y
>>>> CONFIG_EARLY_UART_BASE_ADDRESS=0x09000000
>>>> CONFIG_EARLY_UART_PL011_BAUD_RATE=0
>>>> CONFIG_EARLY_PRINTK_INC="debug-pl011.inc"
>>>
>>> OK I can see it fails on the ACPI and then tries to fall back to FDT and
>>> then fails to find the GIC:
>>>
>>>     (XEN) CMDLINE[00000000f7bbe000]:chosen placeholder root=UUID=cf00cd3a-066b-4146-bedf-f811d3343077 ro console=hvc0 earlyprintk=xen
>>>     (XEN)
>>>     (XEN) Command line: placeholder loglvl=all guest_loglvl=all com1=115200,8n1,0x3e8,5console=com1,vg no-real-mode edd=off
>>>     (XEN) parameter "placeholder" unknown!
>>>     (XEN) parameter "no-real-mode" unknown!
>>>     (XEN) parameter "edd" unknown!
>>>     (XEN) ACPI: RSDP 138560000, 0024 (r2 BOCHS )
>>>     (XEN) ACPI: XSDT 138550000, 004C (r1 BOCHS  BXPCFACP        1       1000013)
>>>     (XEN) ACPI: FACP 138510000, 010C (r5 BOCHS  BXPCFACP        1 BXPC        1)
>>>     (XEN) ACPI: DSDT 138520000, 14A6 (r2 BOCHS  BXPCDSDT        1 BXPC        1)
>>>     (XEN) ACPI: APIC 138500000, 018C (r3 BOCHS  BXPCAPIC        1 BXPC        1)
>>>     (XEN) ACPI: GTDT 1384F0000, 0060 (r2 BOCHS  BXPCGTDT        1 BXPC        1)
>>>     (XEN) ACPI: MCFG 1384E0000, 003C (r1 BOCHS  BXPCMCFG        1 BXPC        1)
>>>     (XEN) ACPI: SPCR 1384D0000, 0050 (r2 BOCHS  BXPCSPCR        1 BXPC        1)
>>>     (XEN) Unsupported FADT revision 5.1, should be 6.0+, will disable ACPI
>>>     (XEN) acpi_boot_table_init: FADT not found (-22)
>>>     (XEN) Domain heap initialised
>>>     (XEN) Booting using Device Tree
>>>     (XEN) Platform: Generic System
>>>     (XEN)
>>>     (XEN) ****************************************
>>>     (XEN) Panic on CPU 0:
>>>     (XEN) Unable to find compatible GIC in the device tree
>>>     (XEN) ****************************************
>>>     (XEN)
>>>     (XEN) Reboot in five seconds...
>>>
>>> Despite saying it is going to reboot it never manages to. Any idea how
>>> it is trying to reset the system?
>>
>> This is a bit of chicken and eggs problem. To know the reset method, you
>> need to parse the ACPI tables. As we can't parse then we don't know the
>> reset method. So, Xen will just do an infinite loop.
> 
> Well you do get some ACPI tables - downgrading the minimum at least
> restores the reset method detection. I wonder if it would be worth
> defaulting to PSCI if you don't know rather than hang indefinitely?

The risk is probably low enough to try to use PSCI even on platform not 
supporting it.

Although, it might be worth to check if EL3 is present to avoid 
panicking again and again on XGene.

> 
> FWIW the failure after that is failing to find the GIC - I'm just
> looking at the MADT table parsing now. Why am I getting a sense of
> DejaVu?

The ACPI code in Xen is based on the first ACPI implementation in Linux. 
So it is quite possible you encountered the bug there :).

>> It would probably be good to be more forthcoming with the users and say
>> it will not reboot.
>>
>> Also, IIRC, the time subsystem is not yet initialized. So it might be
>> possible to mdelay() doesn't work properly.
> 
> Surely that's an architectural subsystem so there is no reason that
> couldn't be up and running.

In theory yes, but the code is also catering some interesting/weird 
platforms behavior:
    1) There are (were?) platform where CNTFREQ was not set correctly
    2) Some platforms, such as the one with Exynos 5, (used to?) require 
specific code to enable the arch timer.

We are still using the Arndale for automated testing. So we would need 
to keep the hacks.

But it would be possible to rework the code and try to make the timer 
available earlier for well-behaved platforms.

Cheers,
Elliott Mitchell Oct. 8, 2020, 6:39 p.m. UTC | #16
On Mon, Sep 28, 2020 at 03:47:52PM +0900, Masami Hiramatsu wrote:
> This made progress with my Xen boot on DeveloperBox (
> https://www.96boards.org/product/developerbox/ ) with ACPI.

Adding your patch on top of Julien Grall's patch appears to push the Xen
boot of my target device (Raspberry PI 4B with Tianocore) further.  Still
yet to see any output attributable to the Domain 0 kernel though.

(XEN) *** LOADING DOMAIN 0 ***
(XEN) Loading d0 kernel from boot module @ 0000000032234000
(XEN) Loading ramdisk from boot module @ 0000000031747000
(XEN) Allocating 1:1 mappings totalling 2048MB for dom0:
(XEN) BANK[0] 0x00000020000000-0x00000030000000 (256MB)
(XEN) BANK[1] 0x00000040000000-0x000000b0000000 (1792MB)
(XEN) Grant table range: 0x000000315f3000-0x00000031633000
(XEN) Allocating PPI 16 for event channel interrupt
(XEN) Loading zImage from 0000000032234000 to 0000000020080000-0000000021359200
(XEN) Loading d0 initrd from 0000000031747000 to 0x0000000028200000-0x0000000028cebfe4
(XEN) Loading d0 DTB to 0x0000000028000000-0x0000000028000273
(XEN) Initial low memory virq threshold set at 0x4000 pages.
(XEN) Scrubbing Free RAM in background
(XEN) Std. Loglevel: All
(XEN) Guest Loglevel: All
(XEN) *** Serial input to DOM0 (type 'CTRL-a' three times to switch input)
(XEN) Freed 396kB init memory.

The line, "Loading d0 DTB to 0x0000000028000000-0x0000000028000273" seems
rather suspicious as I would expect Domain 0 to see ACPI tables, not a
device tree.

Your (Masami Hiramatsu) patch seems plausible, but things haven't
progressed enough for me to endorse it.  Looks like something closer to
the core of ACPI still needs further work, Julien Grall?
Julien Grall Oct. 9, 2020, 9:39 a.m. UTC | #17
Hello,

On 08/10/2020 19:39, Elliott Mitchell wrote:
> On Mon, Sep 28, 2020 at 03:47:52PM +0900, Masami Hiramatsu wrote:
>> This made progress with my Xen boot on DeveloperBox (
>> https://www.96boards.org/product/developerbox/ ) with ACPI.
> 
> Adding your patch on top of Julien Grall's patch appears to push the Xen
> boot of my target device (Raspberry PI 4B with Tianocore) further.  Still
> yet to see any output attributable to the Domain 0 kernel though.
> 
> (XEN) *** LOADING DOMAIN 0 ***
> (XEN) Loading d0 kernel from boot module @ 0000000032234000
> (XEN) Loading ramdisk from boot module @ 0000000031747000
> (XEN) Allocating 1:1 mappings totalling 2048MB for dom0:
> (XEN) BANK[0] 0x00000020000000-0x00000030000000 (256MB)
> (XEN) BANK[1] 0x00000040000000-0x000000b0000000 (1792MB)
> (XEN) Grant table range: 0x000000315f3000-0x00000031633000
> (XEN) Allocating PPI 16 for event channel interrupt
> (XEN) Loading zImage from 0000000032234000 to 0000000020080000-0000000021359200
> (XEN) Loading d0 initrd from 0000000031747000 to 0x0000000028200000-0x0000000028cebfe4
> (XEN) Loading d0 DTB to 0x0000000028000000-0x0000000028000273
> (XEN) Initial low memory virq threshold set at 0x4000 pages.
> (XEN) Scrubbing Free RAM in background
> (XEN) Std. Loglevel: All
> (XEN) Guest Loglevel: All
> (XEN) *** Serial input to DOM0 (type 'CTRL-a' three times to switch input)
> (XEN) Freed 396kB init memory.
> 
> The line, "Loading d0 DTB to 0x0000000028000000-0x0000000028000273" seems
> rather suspicious as I would expect Domain 0 to see ACPI tables, not a
> device tree.

This is normal, we are creating a small device-tree to pass some 
information to dom0 (such as the ACPI tables, command line, initrd).

> 
> Your (Masami Hiramatsu) patch seems plausible, but things haven't
> progressed enough for me to endorse it.  Looks like something closer to
> the core of ACPI still needs further work, Julien Grall?

I didn't go very far during my testing because QEMU is providing ACPI 
5.1 (Xen only supports 6.0+ so far).

For your log above, Xen finished to boot and now dom0 should start 
booting. The lack of console output may be because of a crash in Linux 
during earlyboot.

Do you have the early console enabled Linux? This can be done by adding 
earlycon=xenboot on the Linux command line.

Cheers,
Elliott Mitchell Oct. 9, 2020, 2:22 p.m. UTC | #18
On Fri, Oct 09, 2020 at 10:39:26AM +0100, Julien Grall wrote:
> On 08/10/2020 19:39, Elliott Mitchell wrote:
> > Your (Masami Hiramatsu) patch seems plausible, but things haven't
> > progressed enough for me to endorse it.  Looks like something closer to
> > the core of ACPI still needs further work, Julien Grall?
> 
> I didn't go very far during my testing because QEMU is providing ACPI 
> 5.1 (Xen only supports 6.0+ so far).
> 
> For your log above, Xen finished to boot and now dom0 should start 
> booting. The lack of console output may be because of a crash in Linux 
> during earlyboot.
> 
> Do you have the early console enabled Linux? This can be done by adding 
> earlycon=xenboot on the Linux command line.

Finding all the command-line console settings can be a challenge.  I had
thought it was supposed to be "console=hvc0 earlycon=hvc0".

With that though I finally have some output which claims to come from the
Linux kernel (yay! finally hit this point!).  As we were both guessing,
very early kernel panic:

[    0.000000] efi: Getting EFI parameters from FDT:
[    0.000000] efi: Can't find 'System Table' in device tree!
[    0.000000] cma: Failed to reserve 64 MiB
[    0.000000] Kernel panic - not syncing: Failed to allocate page table page

I don't know whether this is a problem with the mini-DT which was passed
in versus ACPI tables.  I note a complete lack of ACPI table information.
The kernel is from a 5.6-based kernel tree.  I'm unsure which portion to
try updating next.
Julien Grall Oct. 9, 2020, 6:15 p.m. UTC | #19
Hi Elliott,

On 09/10/2020 15:22, Elliott Mitchell wrote:
> On Fri, Oct 09, 2020 at 10:39:26AM +0100, Julien Grall wrote:
>> On 08/10/2020 19:39, Elliott Mitchell wrote:
>>> Your (Masami Hiramatsu) patch seems plausible, but things haven't
>>> progressed enough for me to endorse it.  Looks like something closer to
>>> the core of ACPI still needs further work, Julien Grall?
>>
>> I didn't go very far during my testing because QEMU is providing ACPI
>> 5.1 (Xen only supports 6.0+ so far).
>>
>> For your log above, Xen finished to boot and now dom0 should start
>> booting. The lack of console output may be because of a crash in Linux
>> during earlyboot.
>>
>> Do you have the early console enabled Linux? This can be done by adding
>> earlycon=xenboot on the Linux command line.
> 
> Finding all the command-line console settings can be a challenge.  I had
> thought it was supposed to be "console=hvc0 earlycon=hvc0".
> 
> With that though I finally have some output which claims to come from the
> Linux kernel (yay! finally hit this point!).  As we were both guessing,
> very early kernel panic:
> 
> [    0.000000] efi: Getting EFI parameters from FDT:
> [    0.000000] efi: Can't find 'System Table' in device tree!

Thank you for sending part of the log. Looking at Linux 5.6 code, the 
error message is printed by efi_get_fdt_params() (see 
drivers/firmware/efi.c) when one of the property is missing.

'System Table' suggests that Linux wasn't enable to find 
"linux,uefi-system-table" or "xen,uefi-system-table".

Xen will only create later. Would it be possible to add some code in 
__find_uefi_params() to confirm which property Linux thinks is missing?

> [    0.000000] cma: Failed to reserve 64 MiB
> [    0.000000] Kernel panic - not syncing: Failed to allocate page table page
> 
> I don't know whether this is a problem with the mini-DT which was passed
> in versus ACPI tables.  I note a complete lack of ACPI table information.

I think this is normal because, IIRC, the ACPI root pointer will be 
found using the System Table.

> The kernel is from a 5.6-based kernel tree.  I'm unsure which portion to
> try updating next.
> 
> 

Cheers,
Elliott Mitchell Oct. 9, 2020, 9:49 p.m. UTC | #20
On Mon, Sep 28, 2020 at 03:47:52PM +0900, Masami Hiramatsu wrote:
> This made progress with my Xen boot on DeveloperBox (
> https://www.96boards.org/product/developerbox/ ) with ACPI.

Now things have progressed a bit more and I can confirm the patch
provides useful progress.  I cannot say whether there is a better way
since I'm not familiar enough with the code.

As such, for both Masami Hiramatsu's "Hide UART address only if STAO..."
and Julien Grall's set of four:

Tested-by: Elliott Mitchell <ehem+xen@m5p.com>
Elliott Mitchell Oct. 9, 2020, 10:36 p.m. UTC | #21
On Fri, Oct 09, 2020 at 07:15:20PM +0100, Julien Grall wrote:
> On 09/10/2020 15:22, Elliott Mitchell wrote:
> > Finding all the command-line console settings can be a challenge.  I had
> > thought it was supposed to be "console=hvc0 earlycon=hvc0".
> > 
> > With that though I finally have some output which claims to come from the
> > Linux kernel (yay! finally hit this point!).  As we were both guessing,
> > very early kernel panic:
> > 
> > [    0.000000] efi: Getting EFI parameters from FDT:
> > [    0.000000] efi: Can't find 'System Table' in device tree!
> 
> Thank you for sending part of the log. Looking at Linux 5.6 code, the 
> error message is printed by efi_get_fdt_params() (see 
> drivers/firmware/efi.c) when one of the property is missing.
> 
> 'System Table' suggests that Linux wasn't enable to find 
> "linux,uefi-system-table" or "xen,uefi-system-table".
> 
> Xen will only create later. Would it be possible to add some code in 
> __find_uefi_params() to confirm which property Linux thinks is missing?

Trying to rebuild a configuration long after the prior build can be
entertaining.  Finally identified one patch appears to be for 4.19, but
breaks 5.x.  With that and an appropriate adjustment:

[    0.000000] efi: __find_uefi_params(): Missing "linux,uefi-system-table"
[    0.000000] efi: __find_uefi_params(): Missing "linux,uefi-system-table"


Good news is I had been meaning to try a later kernel for a while and
thus tried building a 5.8 kernel.  Once built, the 5.8 kernel booted
successfully.  As such the next issue is the driver for the graphics chip
doesn't function under Xen as of the 5.8 kernel and Xen 4.14.

I can now state I've gotten success with a combination of Julien Grall's
patches and Masami Hiramatsu.  Certainly qualifies for:

Tested-by: Elliott Mitchell <ehem+xen@m5p.com>

For all 5 patches.
Julien Grall Oct. 10, 2020, 11:02 a.m. UTC | #22
On 28/09/2020 07:47, Masami Hiramatsu wrote:
> Hello,

Hi Masami,

> This made progress with my Xen boot on DeveloperBox (
> https://www.96boards.org/product/developerbox/ ) with ACPI.
> 

I have reviewed the patch attached and I have a couple of remarks about it.

The STAO table was originally created to allow an hypervisor to hide 
devices from a controller domain (such as Dom0). If this table is not 
present, then it means the OS/hypervisor can use any device listed in 
the ACPI table.

Additionally, the STAO table should never be present in the host ACPI table.

Therefore, I think the code should not try to find the STAO. Instead, it 
should check whether the SPCR table is present.

Cheers,
Stefano Stabellini Oct. 12, 2020, 7:02 p.m. UTC | #23
On Sat, 10 Oct 2020, Julien Grall wrote:
> On 28/09/2020 07:47, Masami Hiramatsu wrote:
> > Hello,
> 
> Hi Masami,
> 
> > This made progress with my Xen boot on DeveloperBox (
> > https://www.96boards.org/product/developerbox/ ) with ACPI.
> > 
> 
> I have reviewed the patch attached and I have a couple of remarks about it.
> 
> The STAO table was originally created to allow an hypervisor to hide devices
> from a controller domain (such as Dom0). If this table is not present, then it
> means the OS/hypervisor can use any device listed in the ACPI table.
> 
> Additionally, the STAO table should never be present in the host ACPI table.
> 
> Therefore, I think the code should not try to find the STAO. Instead, it
> should check whether the SPCR table is present.

Yes, that makes sense, but that brings me to the next question.

SPCR seems to be required by SBBR, however, Masami wrote that he could
boot on a system without SPCR, which gets me very confused for two
reasons:

1) Why there is no SPCR? Isn't it supposed to be mandatory? Is it
because there no UART on Masami's system?

2) If there is no SPCR, how did Masami manage to boot Xen?
I take without any serial output? Just with the framebuffer?
Elliott Mitchell Oct. 12, 2020, 9:34 p.m. UTC | #24
On Mon, Oct 12, 2020 at 12:02:14PM -0700, Stefano Stabellini wrote:
> On Sat, 10 Oct 2020, Julien Grall wrote:
> > Therefore, I think the code should not try to find the STAO. Instead, it
> > should check whether the SPCR table is present.
> 
> Yes, that makes sense, but that brings me to the next question.
> 
> SPCR seems to be required by SBBR, however, Masami wrote that he could
> boot on a system without SPCR, which gets me very confused for two
> reasons:
> 
> 1) Why there is no SPCR? Isn't it supposed to be mandatory? Is it
> because there no UART on Masami's system?

I'm on different hardware, but some folks have setup Tianocore for it.
According to Documentation/arm64/acpi_object_usage.rst,
"Required: DSDT, FADT, GTDT, MADT, MCFG, RSDP, SPCR, XSDT".  Yet when
booting a Linux kernel directly on the hardware it lists APIC, BGRT,
CSRT, DSDT, DBG2, FACP, GTDT, PPTT, RSDP, and XSDT.

I don't know whether Linux's ACPI code omits mention of some required
tables and merely panics if they're absent.  Yet I'm speculating the list
of required tables has shrunk, SPCR is no longer required, and the
documentation is out of date.  Perhaps SPCR was required in early Linux
ACPI implementations, but more recent ones removed that requirement?

> 2) If there is no SPCR, how did Masami manage to boot Xen?
> I take without any serial output? Just with the framebuffer?

On my board the provided tables are sufficient to let Linux identify
ttyAMA0.  Linux's efifb driver finds the framebuffer and apparently has
it usable.
Stefano Stabellini Oct. 14, 2020, 1:06 a.m. UTC | #25
On Mon, 12 Oct 2020, Elliott Mitchell wrote:
> On Mon, Oct 12, 2020 at 12:02:14PM -0700, Stefano Stabellini wrote:
> > On Sat, 10 Oct 2020, Julien Grall wrote:
> > > Therefore, I think the code should not try to find the STAO. Instead, it
> > > should check whether the SPCR table is present.
> > 
> > Yes, that makes sense, but that brings me to the next question.
> > 
> > SPCR seems to be required by SBBR, however, Masami wrote that he could
> > boot on a system without SPCR, which gets me very confused for two
> > reasons:
> > 
> > 1) Why there is no SPCR? Isn't it supposed to be mandatory? Is it
> > because there no UART on Masami's system?
> 
> I'm on different hardware, but some folks have setup Tianocore for it.
> According to Documentation/arm64/acpi_object_usage.rst,
> "Required: DSDT, FADT, GTDT, MADT, MCFG, RSDP, SPCR, XSDT".  Yet when
> booting a Linux kernel directly on the hardware it lists APIC, BGRT,
> CSRT, DSDT, DBG2, FACP, GTDT, PPTT, RSDP, and XSDT.
> 
> I don't know whether Linux's ACPI code omits mention of some required
> tables and merely panics if they're absent.  Yet I'm speculating the list
> of required tables has shrunk, SPCR is no longer required, and the
> documentation is out of date.  Perhaps SPCR was required in early Linux
> ACPI implementations, but more recent ones removed that requirement?

I have just checked and SPCR is still a mandatory table in the latest
SBBR specification. It is probably one of those cases where the firmware
claims to be SBBR compliant, but it is not, and it happens to work with
Linux.
Elliott Mitchell Oct. 14, 2020, 1:37 a.m. UTC | #26
On Tue, Oct 13, 2020 at 06:06:26PM -0700, Stefano Stabellini wrote:
> On Mon, 12 Oct 2020, Elliott Mitchell wrote:
> > I'm on different hardware, but some folks have setup Tianocore for it.
> > According to Documentation/arm64/acpi_object_usage.rst,
> > "Required: DSDT, FADT, GTDT, MADT, MCFG, RSDP, SPCR, XSDT".  Yet when
> > booting a Linux kernel directly on the hardware it lists APIC, BGRT,
> > CSRT, DSDT, DBG2, FACP, GTDT, PPTT, RSDP, and XSDT.
> > 
> > I don't know whether Linux's ACPI code omits mention of some required
> > tables and merely panics if they're absent.  Yet I'm speculating the list
> > of required tables has shrunk, SPCR is no longer required, and the
> > documentation is out of date.  Perhaps SPCR was required in early Linux
> > ACPI implementations, but more recent ones removed that requirement?
> 
> I have just checked and SPCR is still a mandatory table in the latest
> SBBR specification. It is probably one of those cases where the firmware
> claims to be SBBR compliant, but it is not, and it happens to work with
> Linux.

Is meeting the SBBR specification supposed to be a requirement of running
Xen-ARM?

I don't seen any mention of such.
`find docs xen/arch/arm -type f -print0 | xargs -0 grep -eSBBR` produces
no output.

Perhaps you've been adding this as a presumptive requirement since
previously the only hardware capable of running Xen due to an
appropriately unlocked bootloader was SBBR compliant?  If so, it seems
time to either add this as an explicit requirement and document it, or
else remove this implicit requirement and start acting as such.

The Raspberry PI 4B has a UEFI implementation available which is based on
Tianocore.  No statement has been made of it qualifying as SBBR.  Yet it
is clearly mostly able to boot Xen, just this is exposing issues.
Julien Grall Oct. 14, 2020, 5:44 p.m. UTC | #27
Hi,

On 12/10/2020 20:02, Stefano Stabellini wrote:
> On Sat, 10 Oct 2020, Julien Grall wrote:
>> On 28/09/2020 07:47, Masami Hiramatsu wrote:
>>> Hello,
>>
>> Hi Masami,
>>
>>> This made progress with my Xen boot on DeveloperBox (
>>> https://www.96boards.org/product/developerbox/ ) with ACPI.
>>>
>>
>> I have reviewed the patch attached and I have a couple of remarks about it.
>>
>> The STAO table was originally created to allow an hypervisor to hide devices
>> from a controller domain (such as Dom0). If this table is not present, then it
>> means the OS/hypervisor can use any device listed in the ACPI table.
>>
>> Additionally, the STAO table should never be present in the host ACPI table.
>>
>> Therefore, I think the code should not try to find the STAO. Instead, it
>> should check whether the SPCR table is present.
> 
> Yes, that makes sense, but that brings me to the next question.
> 
> SPCR seems to be required by SBBR, however, Masami wrote that he could
> boot on a system without SPCR, which gets me very confused for two
> reasons:
> 
> 1) Why there is no SPCR? Isn't it supposed to be mandatory? Is it
> because there no UART on Masami's system?

I can't comment specically on Masami's system, but I can make two broad 
comments:
    1) Not all the systems have to be compliant with the SBBR. But in 
theory, only systems passing the SBBR test can be claimed to be 
complained. This brings to the second point...
    2) "Mandatory" is what the specs aim to enforce. In my experience, 
some of the vendors may bend the rule and still claim they are compliant.

Even Linux documentation says that the SPCR is mandatory. But in the 
reality, the implementation will just ignore it.

 From my understanding of the code, Linux will only use it to discover 
the preferred console and enable earlycon. Without it, Linux will 
require the user to specify console=<...> or earlycon=<...>.

The UART will then be discovered via the DSDT.

> 
> 2) If there is no SPCR, how did Masami manage to boot Xen?
> I take without any serial output? Just with the framebuffer?

After his patch, Xen can boot without the SPCR. You will just see no 
logs. But I belive, he enabled earlyprintk for his platform.

Cheers,
Julien Grall Oct. 14, 2020, 5:47 p.m. UTC | #28
Hi Elliot,

On 14/10/2020 02:37, Elliott Mitchell wrote:
> On Tue, Oct 13, 2020 at 06:06:26PM -0700, Stefano Stabellini wrote:
>> On Mon, 12 Oct 2020, Elliott Mitchell wrote:
>>> I'm on different hardware, but some folks have setup Tianocore for it.
>>> According to Documentation/arm64/acpi_object_usage.rst,
>>> "Required: DSDT, FADT, GTDT, MADT, MCFG, RSDP, SPCR, XSDT".  Yet when
>>> booting a Linux kernel directly on the hardware it lists APIC, BGRT,
>>> CSRT, DSDT, DBG2, FACP, GTDT, PPTT, RSDP, and XSDT.
>>>
>>> I don't know whether Linux's ACPI code omits mention of some required
>>> tables and merely panics if they're absent.  Yet I'm speculating the list
>>> of required tables has shrunk, SPCR is no longer required, and the
>>> documentation is out of date.  Perhaps SPCR was required in early Linux
>>> ACPI implementations, but more recent ones removed that requirement?
>>
>> I have just checked and SPCR is still a mandatory table in the latest
>> SBBR specification. It is probably one of those cases where the firmware
>> claims to be SBBR compliant, but it is not, and it happens to work with
>> Linux.
> 
> Is meeting the SBBR specification supposed to be a requirement of running
> Xen-ARM?

This is not my goal. We should try to get Xen running everywhere as long 
as this doesn't require a lot of extra code. IOW, don't ask me to 
review/accept a port of Xen to RPI3 ;).

Cheers,
Stefano Stabellini Oct. 15, 2020, 6 p.m. UTC | #29
On Wed, 14 Oct 2020, Julien Grall wrote:
> Hi Elliot,
> 
> On 14/10/2020 02:37, Elliott Mitchell wrote:
> > On Tue, Oct 13, 2020 at 06:06:26PM -0700, Stefano Stabellini wrote:
> > > On Mon, 12 Oct 2020, Elliott Mitchell wrote:
> > > > I'm on different hardware, but some folks have setup Tianocore for it.
> > > > According to Documentation/arm64/acpi_object_usage.rst,
> > > > "Required: DSDT, FADT, GTDT, MADT, MCFG, RSDP, SPCR, XSDT".  Yet when
> > > > booting a Linux kernel directly on the hardware it lists APIC, BGRT,
> > > > CSRT, DSDT, DBG2, FACP, GTDT, PPTT, RSDP, and XSDT.
> > > > 
> > > > I don't know whether Linux's ACPI code omits mention of some required
> > > > tables and merely panics if they're absent.  Yet I'm speculating the
> > > > list
> > > > of required tables has shrunk, SPCR is no longer required, and the
> > > > documentation is out of date.  Perhaps SPCR was required in early Linux
> > > > ACPI implementations, but more recent ones removed that requirement?
> > > 
> > > I have just checked and SPCR is still a mandatory table in the latest
> > > SBBR specification. It is probably one of those cases where the firmware
> > > claims to be SBBR compliant, but it is not, and it happens to work with
> > > Linux.
> > 
> > Is meeting the SBBR specification supposed to be a requirement of running
> > Xen-ARM?
> 
> This is not my goal. We should try to get Xen running everywhere as long as
> this doesn't require a lot of extra code. IOW, don't ask me to review/accept a
> port of Xen to RPI3 ;).

I agree with Julien's statement.

For context, my previous comment in regards to SBBR was because I am
positive that Masami's platform is expected to be SBBR compliant.
Elliott Mitchell Oct. 16, 2020, 10:33 p.m. UTC | #30
On Mon, Sep 28, 2020 at 10:00:35PM +0900, Masami Hiramatsu wrote:
> I've missed the explanation of the attached patch. This prototype
> patch was also needed for booting up the Xen on my box (for the system
> which has no SPCR).

I'm pretty sure of this on the hardware I'm dealing with, but don't know
about the hardware you're dealing with.

Does your device have a framebuffer?  Have you ever tried/succeeded
booting your device with the framebuffer disabled? (booted with a
HDMI/DVI cable disconnected or the device on the other end *completely*
powered down)

Based upon the back and forth both on xen-devel and some messages which
were split off due to not being general.  An observation I had made a
while ago finally caused me to try recreating it.

On the device I'm on (Raspberry PI 4B with Tianocore -> GRUB -> Xen) I
discovered a SPCR table shows up if I boot with the device the output is
plugged into is powered down.  I'm guessing this causes Tianocore to
advise GRUB/Linux/Xen to boot with a serial console (presenting a Serial
Port Console Redirect table), whereas if the display device is
functioning the absense of SPCR is supposed to indicate console on
framebuffer.

This means the ACPI_FAILURE case in acpi_iomem_deny_access() simply needs
to be filled in similar to how it likely is on x86.  Allocate a serial
port for Xen's use as console, present it to domain 0 as hvc0, and hide
it from domain 0.



Next issue for me will be getting the framebuffer operational.
Apparently the Xen-ARM EFI implementation doesn't provide any EFI
variables to domain 0?  Jan Beulich, your name was mentioned as person
likely to have ideas for getting Linux's efifb code operational Xen-ARM.
Elliott Mitchell Oct. 17, 2020, 5:12 a.m. UTC | #31
On Fri, Oct 16, 2020 at 03:33:23PM -0700, Elliott Mitchell wrote:
> On the device I'm on (Raspberry PI 4B with Tianocore -> GRUB -> Xen) I
> discovered a SPCR table shows up if I boot with the device the output is
> plugged into is powered down.  I'm guessing this causes Tianocore to
> advise GRUB/Linux/Xen to boot with a serial console (presenting a Serial
> Port Console Redirect table), whereas if the display device is
> functioning the absense of SPCR is supposed to indicate console on
> framebuffer.
> 
> This means the ACPI_FAILURE case in acpi_iomem_deny_access() simply needs
> to be filled in similar to how it likely is on x86.  Allocate a serial
> port for Xen's use as console, present it to domain 0 as hvc0, and hide
> it from domain 0.

Looks like things are worse than I thought.

Upon examining some of my `dmesg` copies it looks like Linux interprets
the ignore_uart field in STAO tables as applying strictly to the UART
referenced in the SPCR table.  As such, when booted with the framebuffer
available, Linux thinks it can freely access the UART found in the
hardware table.  The specification for the STAO table is apparently
garbage since it only allows a hypervisor to tell a VM to ignore a
single UART.  Instead it really needs to be possible to mask arbitrary
devices.  :-(

As such, for ARM devices which can include framebuffers, I'm guessing Xen
will need to either pass a modified table to domain 0, or simulate the
device sufficiently to prevent concurrent access.  This could be as
simple as simulating a MMIO page which discards all writes.





> Next issue for me will be getting the framebuffer operational.
> Apparently the Xen-ARM EFI implementation doesn't provide any EFI
> variables to domain 0?  Jan Beulich, your name was mentioned as person
> likely to have ideas for getting Linux's efifb code operational Xen-ARM.

There may be multiple pieces to this.

For the framebuffer this might be as simple as parsing the BGRT table and
ensuring the addresses are directly mapped to domain 0.  I just noticed
in dmesg, "efi_bgrt: Ignoring BGRT: invalid image address".  I'm guessing
one thing got remapped, but a second didn't.

The other need for EFI variable access is for modifying EFI's boot
process.  While I suspect it may be feasible for most users to reboot to
a kernel directly on hardware to update GRUB/other bootloader, adding an
extra step increases the potential for a failure at the Wrong Time.
Elliott Mitchell Jan. 19, 2021, 7:25 a.m. UTC | #32
On Mon, Sep 28, 2020 at 09:41:39PM +0900, Masami Hiramatsu wrote:
> With Julien's ACPI fix, I found my DeveloperBox (
> https://www.96boards.org/product/developerbox/ ) still failed
> to boot bcause of SPCR issue. According to the UEFI EDK2
> implementation, the SCPR table will not be made if user
> chooses graphical console instead of serial.
> In such case, we should ignore the SPCR missing error.

You weren't the only person running into this.  Since you didn't get
back sooner I ended up submitting a pretty similar patch (I kept the
printk() due to informational value).

861f0c110976fa8879b7bf63d9478b6be83d4ab6 on master (switching to "main"
in the future?)

> Compared with Linux kernel implementation, this doesn't check
> the STAO (status override) table, becaue it seems STAO (or XENV)
> will be provided by Xen itself.
> Or, should we check STAO too?

Until we start seeing super-duper-visors Xen won't need to look for STAO.
Linux needs to look for STAO since it expects to potentially run under a
hypervisor, whereas Xen this isn't expected.

Graphics are a problem due to limits of ACPI table support.  Due to
graphics being a high priority for me, I'm temporarily using
device-trees.  :-(
Julien Grall Jan. 20, 2021, 6:05 p.m. UTC | #33
Hi,

On 19/01/2021 07:25, Elliott Mitchell wrote:
> On Mon, Sep 28, 2020 at 09:41:39PM +0900, Masami Hiramatsu wrote:
>> With Julien's ACPI fix, I found my DeveloperBox (
>> https://www.96boards.org/product/developerbox/ ) still failed
>> to boot bcause of SPCR issue. According to the UEFI EDK2
>> implementation, the SCPR table will not be made if user
>> chooses graphical console instead of serial.
>> In such case, we should ignore the SPCR missing error.
> 
> You weren't the only person running into this.  Since you didn't get
> back sooner I ended up submitting a pretty similar patch (I kept the
> printk() due to informational value).

Right, a cut-down version of the patch submitted has been merged.

@Masami, I believe all the ACPI patches for Arm went in. Would you mind 
to try the latest staging branch?

> 
> 861f0c110976fa8879b7bf63d9478b6be83d4ab6 on master (switching to "main"
> in the future?)

There was a discussion about it a few months ago. The last input we were 
waiting on github to make the change and adopt the same name.

Feel free to give a nudge on the thread [1].

Cheers,

[1] <24307.31637.214096.240023@mariner.uk.xensource.com>