Re: [Xen-devel] [PATCH] xen/arm: vgic-v3: Fix the typo of GICD IRQ active status range
Hi Julien, On 2020/1/7 23:12, Julien Grall wrote: On 07/01/2020 12:55, Wei Xu wrote: Hi Julien, As only one entity should manage the UART (i.e Xen or Dom0), we today assume this will be managed by Xen. Xen should expose a partial virtual UART (only a few registers are emulating) to dom0 in replacement. This is usually done by the UART driver. Looking at the log you pasted in a separate e-mail: (XEN) Platform: Generic System (XEN) Unable to initialize acpi uart: -9 (XEN) Bad console= option 'dtuart' So Xen didn't manage to initialize the uart. The -9 suggests, Xen didn't find a driver for your UART. At the moment, Xen is only able to detect pl011, sbsa, sbsa32 UART for ACPI. What is the type of the UART used on your platform? Thanks! Got it. Our UART is 8250. You would need to teach the 8250 driver how to initialize the UART with ACPI. It is not very difficult to do it, have a look at the pl011 version. Thanks! It is not working even I changed the condition to " if ( acpi_disabled ) ". Doh, thank you for spotting the extra !. My grub 2.04 configuration is as below: xen_hypervisor /xen dom0_mem=4G acpi=force loglvl=all guest_loglvl=all xen_module /Image rdinit=/init acpi=force noinitrd root=/dev/sdb1 rw The log with the condition " if ( acpi_disabled ) " is as following: (XEN) Adding cpu 126 to runqueue 0 (XEN) Adding cpu 127 to runqueue 0 (XEN) alternatives: Patching with alt table 002d4f48 -> 002d5764 (XEN) *** LOADING DOMAIN 0 *** (XEN) Loading d0 kernel from boot module @ 16257000 (XEN) Allocating 1:1 mappings totalling 4096MB for dom0: (XEN) BANK[0] 0x000800-0x001000 (128MB) (XEN) BANK[1] 0x002000-0x003800 (384MB) (XEN) BANK[2] 0x005000-0x008000 (768MB) (XEN) BANK[3] 0x002020-0x0020208000 (2048MB) (XEN) BANK[4] 0x002020b000-0x002020c000 (256MB) (XEN) BANK[5] 0x002026-0x0020262000 (512MB) (XEN) Grant table range: 0x00181c7000-0x0018207000 (XEN) Allocating PPI 16 for event channel interrupt (XEN) Loading zImage from 16257000 to 0808-09981200 (XEN) Loading d0 DTB to 0x0fe0-0x0fe0025b (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) Data Abort Trap. Syndrome=0x6 (XEN) Walking Hypervisor VA 0x10 on CPU0 via TTBR 0x182ff000 (XEN) 0TH[0x0] = 0x18302f7f (XEN) 1ST[0x0] = 0x18300f7f (XEN) 2ND[0x0] = 0x (XEN) CPU0: Unexpected Trap: Data Abort (XEN) [ Xen-4.13.0-rc arm64 debug=y Not tainted ] (XEN) CPU:0 (XEN) PC: 002b65c8 002b65c8 Can you look with addr2line what this PC refers to? Sorry for the late reply! The PC refers to fdt_num_mem_rsv during init_done. But the device_tree_flattened is NULL that the data abort happened. So I added below changes and the XEN dom0 can be booted. diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c index 1e83351..1ab80a1 100644 --- a/xen/arch/arm/setup.c +++ b/xen/arch/arm/setup.c @@ -392,7 +392,8 @@ void __init discard_initial_modules(void) !mfn_valid(maddr_to_mfn(e)) ) continue; -dt_unreserved_regions(s, e, init_domheap_pages, 0); + if( acpi_disabled ) + dt_unreserved_regions(s, e, init_domheap_pages, 0); } Thank you so much! Best Regards, Wei Cheers, ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen/arm: vgic-v3: Fix the typo of GICD IRQ active status range
On 07/01/2020 12:55, Wei Xu wrote: Hi Julien, As only one entity should manage the UART (i.e Xen or Dom0), we today assume this will be managed by Xen. Xen should expose a partial virtual UART (only a few registers are emulating) to dom0 in replacement. This is usually done by the UART driver. Looking at the log you pasted in a separate e-mail: (XEN) Platform: Generic System (XEN) Unable to initialize acpi uart: -9 (XEN) Bad console= option 'dtuart' So Xen didn't manage to initialize the uart. The -9 suggests, Xen didn't find a driver for your UART. At the moment, Xen is only able to detect pl011, sbsa, sbsa32 UART for ACPI. What is the type of the UART used on your platform? Thanks! Got it. Our UART is 8250. You would need to teach the 8250 driver how to initialize the UART with ACPI. It is not very difficult to do it, have a look at the pl011 version. Thanks! It is not working even I changed the condition to " if ( acpi_disabled ) ". Doh, thank you for spotting the extra !. My grub 2.04 configuration is as below: xen_hypervisor /xen dom0_mem=4G acpi=force loglvl=all guest_loglvl=all xen_module /Image rdinit=/init acpi=force noinitrd root=/dev/sdb1 rw The log with the condition " if ( acpi_disabled ) " is as following: (XEN) Adding cpu 126 to runqueue 0 (XEN) Adding cpu 127 to runqueue 0 (XEN) alternatives: Patching with alt table 002d4f48 -> 002d5764 (XEN) *** LOADING DOMAIN 0 *** (XEN) Loading d0 kernel from boot module @ 16257000 (XEN) Allocating 1:1 mappings totalling 4096MB for dom0: (XEN) BANK[0] 0x000800-0x001000 (128MB) (XEN) BANK[1] 0x002000-0x003800 (384MB) (XEN) BANK[2] 0x005000-0x008000 (768MB) (XEN) BANK[3] 0x002020-0x0020208000 (2048MB) (XEN) BANK[4] 0x002020b000-0x002020c000 (256MB) (XEN) BANK[5] 0x002026-0x0020262000 (512MB) (XEN) Grant table range: 0x00181c7000-0x0018207000 (XEN) Allocating PPI 16 for event channel interrupt (XEN) Loading zImage from 16257000 to 0808-09981200 (XEN) Loading d0 DTB to 0x0fe0-0x0fe0025b (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) Data Abort Trap. Syndrome=0x6 (XEN) Walking Hypervisor VA 0x10 on CPU0 via TTBR 0x182ff000 (XEN) 0TH[0x0] = 0x18302f7f (XEN) 1ST[0x0] = 0x18300f7f (XEN) 2ND[0x0] = 0x (XEN) CPU0: Unexpected Trap: Data Abort (XEN) [ Xen-4.13.0-rc arm64 debug=y Not tainted ] (XEN) CPU: 0 (XEN) PC: 002b65c8 002b65c8 Can you look with addr2line what this PC refers to? Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen/arm: vgic-v3: Fix the typo of GICD IRQ active status range
Hi Julien, On 2020/1/7 19:42, Julien Grall wrote: Hi, On 07/01/2020 09:48, Wei Xu wrote: 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 disagreed on the approach. If you look at the context you modifed, it says that reading ACTIVER is not supported. While I agree the behavior is not consistent accross ACTIVER, injecting a data abort is a perfectly fine behavior to me (though not 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 check whether an IRQ is active at the HW level during the synchronizing of the IRQS. From my understanding, this is used because there is a window where the interrupt is active at the HW level but the Linux IRQ subsystem is not aware of it. While the patch below will allow Linux 5.4 to not crash, it is not going to make it fly very far because of the above. So I am rather not happy 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.00] Booting Linux on physical CPU 0x00 [0x481fd010] [0.00] Linux version 5.5.0-rc5 (joyx@Turing-Arch-b) (gcc version 4.9.1 2 0140505 (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.00] Xen XEN_VERSION.XEN_SUBVERSION support found [0.00] efi: Getting EFI parameters from FDT: [0.00] efi: EFI v2.50 by Xen [0.00] efi: ACPI 2.0=0x181d0e70 [0.00] cma: Reserved 32 MiB at 0x7e00 [0.00] ACPI: Early table checksum verification disabled [0.00] ACPI: RSDP 0x181D0E70 24 (v02 HISI ) [0.00] ACPI: XSDT 0x181D0DB0 BC (v01 HISI HIP08000 0 0113) 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 have also done some hacking based on XEN 4.13 as below: I haven't booted Xen on any ACPI systems recently so there might be bugs in the code. Your changes below is definitely a call to look more into 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 this function. The only thing not allowed is the GIC and UART (see acpi_iomem_deny_access). So why do you want this change? What sort of address Dom0 is trying to map and fail? Yes, it is the UART address 0x3f2f8. Without this, during DOM0 UART initialization, the mem_serial_in in the kernel side will be failed and reported a unhandled fault at 0x80001006d2f9(gva) because of mem abort. The Xen printed "HSR=0x93015 pc=0x800010645d94 gva=0x80001006d2f9 gpa=0x03f2f9" in traps.c. I assume this is your primary address as specified in the SPCR, right? Yes. As only one entity should manage the UART (i.e Xen or Dom0), we today assume this will be managed by Xen. Xen should expose a partial virtual UART (only a few registers are emulating) to dom0 in replacement. This is usually done by the UART driver. Looking at the log you pasted in a separate e-mail: (XEN) Platform: Generic System (XEN) Unable to initialize acpi uart: -9 (XEN) Bad console= option 'dtuart' So Xen didn't manage to initialize the uart. The -9 suggests, Xen didn't find a driver for your UART. At the moment, Xen is only able to detect pl011, sbsa, sbsa32 UART for ACPI. What is
Re: [Xen-devel] [PATCH] xen/arm: vgic-v3: Fix the typo of GICD IRQ active status range
Hi, On 07/01/2020 09:48, Wei Xu wrote: 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 disagreed on the approach. If you look at the context you modifed, it says that reading ACTIVER is not supported. While I agree the behavior is not consistent accross ACTIVER, injecting a data abort is a perfectly fine behavior to me (though not 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 check whether an IRQ is active at the HW level during the synchronizing of the IRQS. From my understanding, this is used because there is a window where the interrupt is active at the HW level but the Linux IRQ subsystem is not aware of it. While the patch below will allow Linux 5.4 to not crash, it is not going to make it fly very far because of the above. So I am rather not happy 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.00] Booting Linux on physical CPU 0x00 [0x481fd010] [ 0.00] Linux version 5.5.0-rc5 (joyx@Turing-Arch-b) (gcc version 4.9.1 2 0140505 (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.00] Xen XEN_VERSION.XEN_SUBVERSION support found [ 0.00] efi: Getting EFI parameters from FDT: [ 0.00] efi: EFI v2.50 by Xen [ 0.00] efi: ACPI 2.0=0x181d0e70 [ 0.00] cma: Reserved 32 MiB at 0x7e00 [ 0.00] ACPI: Early table checksum verification disabled [ 0.00] ACPI: RSDP 0x181D0E70 24 (v02 HISI ) [ 0.00] ACPI: XSDT 0x181D0DB0 BC (v01 HISI HIP08 000 0 0113) 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 have also done some hacking based on XEN 4.13 as below: I haven't booted Xen on any ACPI systems recently so there might be bugs in the code. Your changes below is definitely a call to look more into 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 this function. The only thing not allowed is the GIC and UART (see acpi_iomem_deny_access). So why do you want this change? What sort of address Dom0 is trying to map and fail? Yes, it is the UART address 0x3f2f8. Without this, during DOM0 UART initialization, the mem_serial_in in the kernel side will be failed and reported a unhandled fault at 0x80001006d2f9(gva) because of mem abort. The Xen printed "HSR=0x93015 pc=0x800010645d94 gva=0x80001006d2f9 gpa=0x03f2f9" in traps.c. I assume this is your primary address as specified in the SPCR, right? As only one entity should manage the UART (i.e Xen or Dom0), we today assume this will be managed by Xen. Xen should expose a partial virtual UART (only a few registers are emulating) to dom0 in replacement. This is usually done by the UART driver. Looking at the log you pasted in a separate e-mail: (XEN) Platform: Generic System (XEN) Unable to initialize acpi uart: -9 (XEN) Bad console= option 'dtuart' So Xen didn't manage to initialize the uart. The -9 suggests, Xen didn't find a driver for your UART. At the moment, Xen is only able to detect pl011, sbsa, sbsa32 UART for ACPI. What is the type of the UART used on your platform?
Re: [Xen-devel] [PATCH] xen/arm: vgic-v3: Fix the typo of GICD IRQ active status range
On 07/01/2020 10:56, Wei Xu wrote: Maybe I am missing something, but Linux seems to be running fine and I can't spot any error related to read the active status register. By any chance, did you build Xen with your patch? Yes, I built Xen with my patch. The reason I asked the log is to see what is the splat you receive from Linux without your patch. So can you send the splat you have without your patch? Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen/arm: vgic-v3: Fix the typo of GICD IRQ active status range
Hi Julien, On 2020/1/7 18:51, Julien Grall wrote: On 07/01/2020 09:28, Wei Xu wrote: 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 disagreed on the approach. If you look at the context you modifed, it says that reading ACTIVER is not supported. While I agree the behavior is not consistent accross ACTIVER, injecting a data abort is a perfectly fine behavior to me (though not 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 check whether an IRQ is active at the HW level during the synchronizing of the IRQS. From my understanding, this is used because there is a window where the interrupt is active at the HW level but the Linux IRQ subsystem is not aware of it. While the patch below will allow Linux 5.4 to not crash, it is not going to make it fly very far because of the above. So I am rather not happy 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.00] Booting Linux on physical CPU 0x00 [0x481fd010] [0.00] Linux version 5.5.0-rc5 (joyx@Turing-Arch-b) (gcc version 4.9.1 2 0140505 (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.00] Xen XEN_VERSION.XEN_SUBVERSION support found [0.00] efi: Getting EFI parameters from FDT: [0.00] efi: EFI v2.50 by Xen [0.00] efi: ACPI 2.0=0x181d0e70 [0.00] cma: Reserved 32 MiB at 0x7e00 [0.00] ACPI: Early table checksum verification disabled [0.00] ACPI: RSDP 0x181D0E70 24 (v02 HISI ) [0.00] ACPI: XSDT 0x181D0DB0 BC (v01 HISI HIP08000 0 0113) Is that the full log from Linux? If not, can you post it in full? I will reply the other questions in another mail since the below log is too big. Maybe I am missing something, but Linux seems to be running fine and I can't spot any error related to read the active status register. By any chance, did you build Xen with your patch? Yes, I built Xen with my patch. Best Regards, Wei Cheers, ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen/arm: vgic-v3: Fix the typo of GICD IRQ active status range
On 07/01/2020 09:28, Wei Xu wrote: 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 disagreed on the approach. If you look at the context you modifed, it says that reading ACTIVER is not supported. While I agree the behavior is not consistent accross ACTIVER, injecting a data abort is a perfectly fine behavior to me (though not 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 check whether an IRQ is active at the HW level during the synchronizing of the IRQS. From my understanding, this is used because there is a window where the interrupt is active at the HW level but the Linux IRQ subsystem is not aware of it. While the patch below will allow Linux 5.4 to not crash, it is not going to make it fly very far because of the above. So I am rather not happy 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.00] Booting Linux on physical CPU 0x00 [0x481fd010] [ 0.00] Linux version 5.5.0-rc5 (joyx@Turing-Arch-b) (gcc version 4.9.1 2 0140505 (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.00] Xen XEN_VERSION.XEN_SUBVERSION support found [ 0.00] efi: Getting EFI parameters from FDT: [ 0.00] efi: EFI v2.50 by Xen [ 0.00] efi: ACPI 2.0=0x181d0e70 [ 0.00] cma: Reserved 32 MiB at 0x7e00 [ 0.00] ACPI: Early table checksum verification disabled [ 0.00] ACPI: RSDP 0x181D0E70 24 (v02 HISI ) [ 0.00] ACPI: XSDT 0x181D0DB0 BC (v01 HISI HIP08 000 0 0113) Is that the full log from Linux? If not, can you post it in full? I will reply the other questions in another mail since the below log is too big. Maybe I am missing something, but Linux seems to be running fine and I can't spot any error related to read the active status register. By any chance, did you build Xen with your patch? Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen/arm: vgic-v3: Fix the typo of GICD IRQ active status range
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 disagreed
on the
approach.
If you look at the context you modifed, it says that reading
ACTIVER is not
supported. While I agree the behavior is not consistent accross
ACTIVER,
injecting a data abort is a perfectly fine behavior to me (though
not 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 check
whether an
IRQ is active at the HW level during the synchronizing of the
IRQS. From my
understanding, this is used because there is a window where the
interrupt is
active at the HW level but the Linux IRQ subsystem is not aware of
it.
While the patch below will allow Linux 5.4 to not crash, it is not
going to
make it fly very far because of the above. So I am rather not
happy 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.00] Booting Linux on physical CPU 0x00
[0x481fd010]
[0.00] Linux version 5.5.0-rc5 (joyx@Turing-Arch-b) (gcc
version 4.9.1 2
0140505 (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.00] Xen XEN_VERSION.XEN_SUBVERSION support found
[0.00] efi: Getting EFI parameters from FDT:
[0.00] efi: EFI v2.50 by Xen
[0.00] efi: ACPI 2.0=0x181d0e70
[0.00] cma: Reserved 32 MiB at 0x7e00
[0.00] ACPI: Early table checksum verification disabled
[0.00] ACPI: RSDP 0x181D0E70 24 (v02 HISI )
[0.00] ACPI: XSDT 0x181D0DB0 BC (v01 HISI
HIP08000
0 0113)
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 have
also done some hacking based on
XEN 4.13 as below:
I haven't booted Xen on any ACPI systems recently so there might be
bugs in the code. Your changes below is definitely a call to look more
into 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 this
function. The only thing not allowed is the GIC and UART (see
acpi_iomem_deny_access).
So why do you want this change? What sort of address Dom0 is trying to
map and fail?
Yes, it is the UART address 0x3f2f8.
Without this, during DOM0 UART initialization, the mem_serial_in in the
kernel side will be failed and reported a unhandled fault at
0x80001006d2f9(gva)
because of mem abort.
The Xen printed "HSR=0x93015 pc=0x800010645d94
gva=0x80001006d2f9 gpa=0x03f2f9" 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_t
entry, mfn_t mfn, unsigned int flags)
{
mm_printk("Changing MFN for a valid entry is not
allowed (%#"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 valid
entry. Indeed, the PT code is not handling of the Break-Before-Make
sequence (requires when updating
Re: [Xen-devel] [PATCH] xen/arm: vgic-v3: Fix the typo of GICD IRQ active status range
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 disagreed on the
approach.
If you look at the context you modifed, it says that reading ACTIVER
is not
supported. While I agree the behavior is not consistent accross
ACTIVER,
injecting a data abort is a perfectly fine behavior to me (though
not 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 check
whether an
IRQ is active at the HW level during the synchronizing of the IRQS.
From my
understanding, this is used because there is a window where the
interrupt is
active at the HW level but the Linux IRQ subsystem is not aware of it.
While the patch below will allow Linux 5.4 to not crash, it is not
going to
make it fly very far because of the above. So I am rather not happy
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.00] Booting Linux on physical CPU 0x00 [0x481fd010]
[ 0.00] Linux version 5.5.0-rc5 (joyx@Turing-Arch-b) (gcc
version 4.9.1 2
0140505 (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.00] Xen XEN_VERSION.XEN_SUBVERSION support found
[ 0.00] efi: Getting EFI parameters from FDT:
[ 0.00] efi: EFI v2.50 by Xen
[ 0.00] efi: ACPI 2.0=0x181d0e70
[ 0.00] cma: Reserved 32 MiB at 0x7e00
[ 0.00] ACPI: Early table checksum verification disabled
[ 0.00] ACPI: RSDP 0x181D0E70 24 (v02 HISI )
[ 0.00] ACPI: XSDT 0x181D0DB0 BC (v01 HISI
HIP08 000
0 0113)
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 have also
done some hacking based on
XEN 4.13 as below:
I haven't booted Xen on any ACPI systems recently so there might be bugs
in the code. Your changes below is definitely a call to look more into
details what's wrong.
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 this
function. The only thing not allowed is the GIC and UART (see
acpi_iomem_deny_access).
So why do you want this change? What sort of address Dom0 is trying to
map and fail?
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_t entry,
mfn_t mfn, unsigned int flags)
{
mm_printk("Changing MFN for a valid entry is not
allowed (%#"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 valid
entry. Indeed, the PT code is not handling of the Break-Before-Make
sequence (requires when updating certain entry) as this is a can of worms.
However, during my testing I never found a place where an valid entry is
modified (other than the permissions part). So can you give more details
how you came up here?
}
}
/* 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
+++
Re: [Xen-devel] [PATCH] xen/arm: vgic-v3: Fix the typo of GICD IRQ active status range
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 disagreed on the
approach.
If you look at the context you modifed, it says that reading ACTIVER is not
supported. While I agree the behavior is not consistent accross ACTIVER,
injecting a data abort is a perfectly fine behavior to me (though not 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 check whether an
IRQ is active at the HW level during the synchronizing of the IRQS. From my
understanding, this is used because there is a window where the interrupt is
active at the HW level but the Linux IRQ subsystem is not aware of it.
While the patch below will allow Linux 5.4 to not crash, it is not going to
make it fly very far because of the above. So I am rather not happy 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.00] Booting Linux on physical CPU 0x00 [0x481fd010]
[0.00] Linux version 5.5.0-rc5 (joyx@Turing-Arch-b) (gcc
version 4.9.1 2
0140505 (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.00] Xen XEN_VERSION.XEN_SUBVERSION support found
[0.00] efi: Getting EFI parameters from FDT:
[0.00] efi: EFI v2.50 by Xen
[0.00] efi: ACPI 2.0=0x181d0e70
[0.00] cma: Reserved 32 MiB at 0x7e00
[0.00] ACPI: Early table checksum verification disabled
[0.00] ACPI: RSDP 0x181D0E70 24 (v02 HISI )
[0.00] ACPI: XSDT 0x181D0DB0 BC (v01 HISI
HIP08000
0 0113)
But to boot with ACPI on our hardware, except above change I have also
done some hacking based on
XEN 4.13 as below:
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; */
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_t entry,
mfn_t mfn, unsigned int flags)
{
mm_printk("Changing MFN for a valid entry is not
allowed (%#"PRI_mfn" -> %#"PRI_mfn").\n",
mfn_x(lpae_get_mfn(entry)), mfn_x(mfn));
-return false;
+return true;
}
}
/* 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 long
boot_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();
}
init_IRQ();
Best Regards,
Wei
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen/arm: vgic-v3: Fix the typo of GICD IRQ active status range
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 disagreed on the > > approach. > > > > If you look at the context you modifed, it says that reading ACTIVER is not > > supported. While I agree the behavior is not consistent accross ACTIVER, > > injecting a data abort is a perfectly fine behavior to me (though not 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 check whether an > > IRQ is active at the HW level during the synchronizing of the IRQS. From my > > understanding, this is used because there is a window where the interrupt is > > active at the HW level but the Linux IRQ subsystem is not aware of it. > > > > While the patch below will allow Linux 5.4 to not crash, it is not going to > > make it fly very far because of the above. So I am rather not happy 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? ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen/arm: vgic-v3: Fix the typo of GICD IRQ active status range
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 disagreed on
the approach.
If you look at the context you modifed, it says that reading ACTIVER
is not supported. While I agree the behavior is not consistent accross
ACTIVER, injecting a data abort is a perfectly fine behavior to me
(though not 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
check whether an IRQ is active at the HW level during the
synchronizing of the IRQS. From my understanding, this is used because
there is a window where the interrupt is active at the HW level but
the Linux IRQ subsystem is not aware of it.
While the patch below will allow Linux 5.4 to not crash, it is not
going to make it fly very far because of the above. So I am rather not
happy 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!
Best Regards,
Wei
@Stefano, you mention you will look at implementing ACTIVER. What's
the state?
>
Fixes: a2b83f95bfad ("xen/arm: vgic: Properly emulate the full
register")
Signed-off-by: Wei Xu
---
xen/arch/arm/vgic-v3.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index 422b94f..e802f20 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -706,7 +706,7 @@ static int __vgic_v3_distr_common_mmio_read(const
char *name, struct vcpu *v,
goto read_as_zero;
/* Read the active status of an IRQ via GICD/GICR is not
supported */
-case VRANGE32(GICD_ISACTIVER, GICD_ISACTIVER):
+case VRANGE32(GICD_ISACTIVER, GICD_ISACTIVERN):
case VRANGE32(GICD_ICACTIVER, GICD_ICACTIVERN):
goto read_as_zero;
Cheers,
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen/arm: vgic-v3: Fix the typo of GICD IRQ active status range
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 disagreed on the
approach.
If you look at the context you modifed, it says that reading ACTIVER is
not supported. While I agree the behavior is not consistent accross
ACTIVER, injecting a data abort is a perfectly fine behavior to me
(though not 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 check
whether an IRQ is active at the HW level during the synchronizing of the
IRQS. From my understanding, this is used because there is a window
where the interrupt is active at the HW level but the Linux IRQ
subsystem is not aware of it.
While the patch below will allow Linux 5.4 to not crash, it is not going
to make it fly very far because of the above. So I am rather not happy
with persuing with returning 0.
@Stefano, you mention you will look at implementing ACTIVER. What's the
state?
>
Fixes: a2b83f95bfad ("xen/arm: vgic: Properly emulate the full register")
Signed-off-by: Wei Xu
---
xen/arch/arm/vgic-v3.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index 422b94f..e802f20 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -706,7 +706,7 @@ static int __vgic_v3_distr_common_mmio_read(const char
*name, struct vcpu *v,
goto read_as_zero;
/* Read the active status of an IRQ via GICD/GICR is not supported */
-case VRANGE32(GICD_ISACTIVER, GICD_ISACTIVER):
+case VRANGE32(GICD_ISACTIVER, GICD_ISACTIVERN):
case VRANGE32(GICD_ICACTIVER, GICD_ICACTIVERN):
goto read_as_zero;
Cheers,
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] xen/arm: vgic-v3: Fix the typo of GICD IRQ active status range
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.
Fixes: a2b83f95bfad ("xen/arm: vgic: Properly emulate the full register")
Signed-off-by: Wei Xu
---
xen/arch/arm/vgic-v3.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index 422b94f..e802f20 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -706,7 +706,7 @@ static int __vgic_v3_distr_common_mmio_read(const char
*name, struct vcpu *v,
goto read_as_zero;
/* Read the active status of an IRQ via GICD/GICR is not supported */
-case VRANGE32(GICD_ISACTIVER, GICD_ISACTIVER):
+case VRANGE32(GICD_ISACTIVER, GICD_ISACTIVERN):
case VRANGE32(GICD_ICACTIVER, GICD_ICACTIVERN):
goto read_as_zero;
--
2.8.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
