|
[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
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:I have seen a patch similar from NXP a month ago and I disagreed on theThis 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.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 speccompliant) 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 withpersuing 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) (gcc version 4.9.1 2 0140505 (prerelease) (crosstool-NG linaro-1.13.1-4.9-2014.05 - Linaro GCC 4.9-2014.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 HISI HIP08 00000000 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 have also done some hacking based onXEN 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 0x3f00002f8.Without this, during DOM0 UART initialization, the mem_serial_in in the kernel side will be failed and reported a unhandled fault at 0xffff80001006d2f9(gva)because of mem abort.The Xen printed "HSR=0x930100005 pc=0xffff800010645d94 gva=0xffff80001006d2f9 gpa=0x000003f00002f9" 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? 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) On Arm, we requires the fixmap to be empty (by calling clear_fixmap()) before the fixmap can used for a different mapping. Looking at the implementation of acpi_os_unmap_memory(), the fixmap is not cleared and hence why you see the warning about changing the MFN on valid entry. IHMO, acpi_os_unmap_memory() ought to clear the fixmap. This will make it a much saner interface to use. Would you mind to have a look? } } /* 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, The function is used for creating multiple domains at boot time from Xen. It is very DT centric at the moment, but ACPI platform may not come with a DT (though the EFI stub may create one ATM). I can see other issue with dom0less and ACPI, so I think it would be best to just not call the function when using ACPI. Could you try following patch (not tested nor compiled):
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 3c899cd4a0..3e9dc8fe9f 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -985,7 +985,8 @@ void __init start_xen(unsigned long boot_phys_offset,
system_state = SYS_STATE_active;
- create_domUs();
+ if ( !acpi_disabled )
+ create_domUs();
domain_unpause_by_systemcontroller(dom0);
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |