[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 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)

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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

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