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.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-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 HISI
HIP08 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 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
+++ 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();
When using ACPI, the DT should not be used. So why do you need this?
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
