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

Re: [Xen-devel] [PATCH] xen/arm: vgic-v3: Fix the typo of GICD IRQ active status range

To: Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>
From: Wei Xu <xuwei5@xxxxxxxxxxxxx>
Date: Tue, 7 Jan 2020 17:48:37 +0800
Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, prime.zeng@xxxxxxxxxxxxx, shameerali.kolothum.thodi@xxxxxxxxxx, linuxarm@xxxxxxxxxx
Delivery-date: Tue, 07 Jan 2020 09:48:55 +0000
List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

Hi Julien,

On 2020/1/7 17:10, Julien Grall wrote:

On 07/01/2020 08:39, Wei Xu wrote:
Hi Stefano,

On 2020/1/7 6:01, Stefano Stabellini wrote:
On Sat, 28 Dec 2019, Wei Xu wrote:
Hi Julien,

On 2019/12/28 16:09, Julien Grall wrote:
Hi,

On 28/12/2019 03:08, Wei Xu wrote:
This patch fixes the typo about the active status range of an IRQ
via GICD. Otherwise it will be failed to handle the mmio access and
inject a data abort.
I have seen a patch similar from NXP a month ago and I disagreedon the
approach.
If you look at the context you modifed, it says that readingACTIVER is notsupported. While I agree the behavior is not consistent accrossACTIVER,injecting a data abort is a perfectly fine behavior to me (thoughnot spec
compliant) as we don't implement the registers correctly.
I guess you are sending this patch, because you tried Linux 5.4(or later)on Xen, right? Linux has recently began to read ACTIVER to checkwhether anIRQ is active at the HW level during the synchronizing of theIRQS. From myunderstanding, this is used because there is a window where theinterrupt isactive at the HW level but the Linux IRQ subsystem is not aware ofit.
While the patch below will allow Linux 5.4 to not crash, it is notgoing tomake it fly very far because of the above. So I am rather nothappy with
persuing with returning 0.
Yes, I am using Linux 5.5-rc2 :)
Got it and thanks for the explanation.
I am not insistent on this and OK to wait for the update.
Thanks and have a very happy new year!
Hi Wei,

what do you do to reproduce the issue? Are you just booting Linux
5.5-rc2 as dom0 and seeing the issue during boot, or are you doing
something specific?

.
I directly tested the mainline kernel with defconfig.
And the 5.5-rc5 kernel booting log is as below:

     root@ubuntu:~# dmesg | more
[ 0.000000] Booting Linux on physical CPU 0x0000000000[0x481fd010][ 0.000000] Linux version 5.5.0-rc5 (joyx@Turing-Arch-b) (gccversion 4.9.1 20140505 (prerelease) (crosstool-NG linaro-1.13.1-4.9-2014.05 -Linaro GCC 4.9-20
     14.05)) #132 SMP PREEMPT Tue Jan 7 15:43:06 CST 2020
     [    0.000000] Xen XEN_VERSION.XEN_SUBVERSION support found
     [    0.000000] efi: Getting EFI parameters from FDT:
     [    0.000000] efi: EFI v2.50 by Xen
     [    0.000000] efi:  ACPI 2.0=0x181d0e70
     [    0.000000] cma: Reserved 32 MiB at 0x000000007e000000
     [    0.000000] ACPI: Early table checksum verification disabled
     [    0.000000] ACPI: RSDP 0x00000000181D0E70 000024 (v02 HISI  )
[ 0.000000] ACPI: XSDT 0x00000000181D0DB0 0000BC (v01 HISIHIP08 0000000
     0      01000013)
Is that the full log from Linux? If not, can you post it in full?
But to boot with ACPI on our hardware, except above change I havealso done some hacking based on
XEN 4.13 as below:
I haven't booted Xen on any ACPI systems recently so there might bebugs in the code. Your changes below is definitely a call to look moreinto details what's wrong.


Yes, my target is to make Xen booting with ACPI firstly.

     diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
     index d028ec9..215a291 100644
     --- a/xen/arch/arm/traps.c
     +++ b/xen/arch/arm/traps.c
     @@ -1856,8 +1856,8 @@ static bool try_map_mmio(gfn_t gfn)
              return false;

          /* The hardware domain can only map permitted MMIO regions */
     -    if ( !iomem_access_permitted(d, mfn_x(mfn), mfn_x(mfn) + 1) )
     -        return false;
+ /* if ( !iomem_access_permitted(d, mfn_x(mfn), mfn_x(mfn) +1) ) */
     +    /*     return false; */
Dom0 should be able to map nearly all the address space through thisfunction. The only thing not allowed is the GIC and UART (seeacpi_iomem_deny_access).
So why do you want this change? What sort of address Dom0 is trying tomap and fail?


Yes, it is the UART address 0x3f00002f8.

Without this, during DOM0 UART initialization, the mem_serial_in in thekernel side will be failed and reported a unhandled fault at0xffff80001006d2f9(gva)

because of mem abort.

The Xen printed "HSR=0x930100005 pc=0xffff800010645d94gva=0xffff80001006d2f9 gpa=0x000003f00002f9" in traps.c.

          return !map_regions_p2mt(d, gfn, 1, mfn, p2m_mmio_direct_c);
      }


     diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
     index 4d6c971..c626f9e 100644
     --- a/xen/arch/arm/mm.c
     +++ b/xen/arch/arm/mm.c
@@ -1095,7 +1095,7 @@ static bool xen_pt_check_entry(lpae_tentry, mfn_t mfn, unsigned int flags)
              {
mm_printk("Changing MFN for a valid entry is notallowed (%#"PRI_mfn" -> %#"PRI_mfn").\n",
                            mfn_x(lpae_get_mfn(entry)), mfn_x(mfn));
     -            return false;
     +            return true;
There is a pretty good reason to prevent modifying the MFN on a validentry. Indeed, the PT code is not handling of the Break-Before-Makesequence (requires when updating certain entry) as this is a can ofworms.
However, during my testing I never found a place where an valid entryis modified (other than the permissions part). So can you give moredetails how you came up here?


In the full log, I found the RSDP(0x39de0) replaced by XSDT(0x39dd0).
But I did not know why :(

              }
          }
          /* Sanity check when removing a page. */


     diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
     index 3c899cd..1e83351 100644
     --- a/xen/arch/arm/setup.c
     +++ b/xen/arch/arm/setup.c

@@ -852,7 +852,8 @@ void __init start_xen(unsigned longboot_phys_offset,

          else
          {
              printk("Booting using ACPI\n");
     -        device_tree_flattened = NULL;
     +        device_tree_flattened = relocate_fdt(fdt_paddr, fdt_size);
     +        dt_unflatten_host_device_tree();


When using ACPI, the DT should not be used. So why do you need this?


I have tried not passing DT with grub-2.04 but also to load DOM0 kernel.
The log is as below:

    (XEN) *** LOADING DOMAIN 0 ***
    (XEN) Loading d0 kernel from boot module @ 0000000016221000
    (XEN) Allocating 1:1 mappings totalling 4096MB for dom0:
    (XEN) BANK[0] 0x00000008000000-0x00000010000000 (128MB)
    (XEN) BANK[1] 0x00000020000000-0x00000038000000 (384MB)
    (XEN) BANK[2] 0x00000050000000-0x00000080000000 (768MB)
    (XEN) BANK[3] 0x00202000000000-0x00202080000000 (2048MB)
    (XEN) BANK[4] 0x002020b0000000-0x002020c0000000 (256MB)
    (XEN) BANK[5] 0x00202600000000-0x00202620000000 (512MB)
    (XEN) Grant table range: 0x000000181da000-0x0000001821a000
    (XEN) Allocating PPI 16 for event channel interrupt

(XEN) Loading zImage from 0000000016221000 to0000000008080000-00000000099caa00

    (XEN) Loading d0 DTB to 0x000000000fe00000-0x000000000fe0025b
    (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 switchinput)

    (XEN) Data Abort Trap. Syndrome=0x6
    (XEN) Walking Hypervisor VA 0x38 on CPU0 via TTBR 0x000000001831a000
    (XEN) 0TH[0x0] = 0x000000001831df7f
    (XEN) 1ST[0x0] = 0x000000001831bf7f
    (XEN) 2ND[0x0] = 0x0000000000000000
    (XEN) CPU0: Unexpected Trap: Data Abort
    (XEN) ----[ Xen-4.13.0-rc  arm64  debug=y   Not tainted ]----
    (XEN) CPU:    0
    (XEN) PC:     00000000002c6398 create_domUs+0x20/0x208
    (XEN) LR:     00000000002c6398
    (XEN) SP:     0000000000307d60
    (XEN) CPSR:   60000249 MODE:64-bit EL2h (Hypervisor, handler)

(XEN) X0: 0000000000000000 X1: 0000000000000003 X2:0000000000000000(XEN) X3: 0000000000000000 X4: 0000000000000000 X5:0000000000000024(XEN) X6: 0080808080808080 X7: fefefefefefeff09 X8:7f7f7f7f7f7f7f7f(XEN) X9: 731f646b61606d54 X10: 7f7f7f7f7f7f7f7f X11:0101010101010101(XEN) X12: 0000000000000008 X13: 00000000002871b8 X14:0000000000000020(XEN) X15: 00000000004002f8 X16: 00000000002b2000 X17:00000000002b2000(XEN) X18: 00000000002b2000 X19: 000080662f3d7000 X20:00000000002b1480(XEN) X21: 0000000000348430 X22: 0000000000000080 X23:00000000002a4240(XEN) X24: 0000000000000080 X25: 0000000000348000 X26:00000000002e9078(XEN) X27: 000020279c000000 X28: 00000000002e83f0 FP:0000000000307d60

    (XEN)
    (XEN)   VTCR_EL2: 800d3590
    (XEN)  VTTBR_EL2: 0000000000000000
    (XEN)
    (XEN)  SCTLR_EL2: 30cd183d
    (XEN)    HCR_EL2: 000000008000003a
    (XEN)  TTBR0_EL2: 000000001831a000
    (XEN)
    (XEN)    ESR_EL2: 96000006
    (XEN)  HPFAR_EL2: 0000000000000000
    (XEN)    FAR_EL2: 0000000000000038
    (XEN)
    (XEN) Xen stack trace from sp=0000000000307d60:

(XEN) 0000000000307de0 00000000002cb5f0 000080662f3d700000000000002b1480(XEN) 0000000000348430 0000000000000080 00000000002a42400000000000000080(XEN) 0000000080000000 6d681f6762736876 0000000000307dc000000000002bc570(XEN) 0000000000307de0 00000000002cb5e0 000080662f3d700000000000002b1480(XEN) 000000003f14a780 00000000002001b8 00000000181da0000000000017fda000(XEN) 000000001a637000 0000000000000000 00000000004002f8000000001828cdc8(XEN) 0000000000001500 0000000000000001 0000000000000001000000001828cdc0(XEN) 0000000000000000 0000000000003000 000000001a6370000000003700000000(XEN) 0000000000000000 0000000f86db1000 00000000000000000000000000000000(XEN) 0000000000000000 0000000300000000 000000800000000000000040ffffffff(XEN) 00000002ffffffff 0000000000000280 00000000000000000000000000000000(XEN) 0000000000000000 0000000000000000 00000000000000000000000000000000(XEN) 0000000000000000 0000000000000000 00000000000000000000000000000000(XEN) 0000000000000000 0000000000000000 00000000000000000000000000000000(XEN) 0000000000000000 0000000000000000 00000000000000000000000000000000(XEN) 0000000000000000 0000000000000000 00000000000000000000000000000000(XEN) 0000000000000000 0000000000000000 00000000000000000000000000000000(XEN) 0000000000000000 0000000000000000 00000000000000000000000000000000(XEN) 0000000000000000 0000000000000000 00000000000000000000000000000000(XEN) 0000000000000000 0000000000000000 00000000000000000000000000000000(XEN) 0000000000000000 0000000000000000 00000000000000000000000000000000

    (XEN) Xen call trace:
    (XEN)    [<00000000002c6398>] create_domUs+0x20/0x208 (PC)
    (XEN)    [<00000000002c6398>] create_domUs+0x20/0x208 (LR)
    (XEN)    [<00000000002cb5f0>] start_xen+0xc34/0xcbc
    (XEN)    [<00000000002001b8>] arm64/head.o#primary_switched+0x10/0x30
    (XEN)
    (XEN) debugtrace_dump() global buffer starting
    1 cpupool_add_domain(dom=0,pool=0) n_dom 1 rc 0
    (XEN) wrap: 0
    (XEN) debugtrace_dump() global buffer finished
    (XEN)
    (XEN) ****************************************
    (XEN) Panic on CPU 0:
    (XEN) CPU0: Unexpected Trap: Data Abort
    (XEN) ****************************************
    (XEN)
    (XEN) Reboot in five seconds...

So I passed DT to the Xen with grub-2.02 and hacked above code becausein the create_domUs

will report a bug if chosen node  can not be find.

Thanks!

Best Regards,
Wei


Cheers,




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

Follow-Ups:
- Re: [Xen-devel] [PATCH] xen/arm: vgic-v3: Fix the typo of GICD IRQ active status range
  - From: Julien Grall

References:
- Re: [Xen-devel] [PATCH] xen/arm: vgic-v3: Fix the typo of GICD IRQ active status range
  - From: Stefano Stabellini
- Re: [Xen-devel] [PATCH] xen/arm: vgic-v3: Fix the typo of GICD IRQ active status range
  - From: Wei Xu
- Re: [Xen-devel] [PATCH] xen/arm: vgic-v3: Fix the typo of GICD IRQ active status range
  - From: Julien Grall

Prev by Date: [Xen-devel] [libvirt test] 145710: regressions - FAIL
Next by Date: Re: [Xen-devel] [RFC PATCH 3/3] xen/netback: Fix grant copy across page boundary with KASAN
Previous by thread: Re: [Xen-devel] [PATCH] xen/arm: vgic-v3: Fix the typo of GICD IRQ active status range
Next by thread: Re: [Xen-devel] [PATCH] xen/arm: vgic-v3: Fix the typo of GICD IRQ active status range
Index(es):
- Date
- Thread

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