linux-kernel - Re: [PATCH] irq/gic-its: gicv4: set VPENDING table as inner-shareable

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20191201180434.1dba3116@why>
Date:   Sun, 1 Dec 2019 18:04:34 +0000
From:   Marc Zyngier <maz@...nel.org>
To:     Heyi Guo <guoheyi@...wei.com>
Cc:     <linux-kernel@...r.kernel.org>, <wanghaibin.wang@...wei.com>,
        "Thomas Gleixner" <tglx@...utronix.de>,
        Jason Cooper <jason@...edaemon.net>
Subject: Re: [PATCH] irq/gic-its: gicv4: set VPENDING table as
 inner-shareable

On Sat, 30 Nov 2019 15:38:49 +0800
Heyi Guo <guoheyi@...wei.com> wrote:

> There is no special reason to set virtual LPI pending table as
> non-shareable. If we choose to hard code the shareability without
> probing, inner-shareable will be a better choice, for all the other
> ITS/GICR tables prefer to be inner-shareable.

One of the issues is that we have strictly no idea what the caches are
Inner Shareable with (I've been asking for such clarification for years
without getting anywhere). You can have as many disconnected inner
shareable domains as you want!

I suspect that in the grand scheme of things, the redistributors
ought to be in the same inner shareable domain, and that with a bit of
luck, the CPUs are there as well. Still, that's a massive guess.

> What's more, on Hisilicon hip08 it will trigger some kind of bus
> warning when mixing use of different shareabilities.

Do you have more information about what the bus is complaining about?
Is that because the CPUs have these pages mapped as inner shareable?

I'll give it a go on D05 (HIP07) to find out what changes there.

Thanks,

	M.

> Signed-off-by: Heyi Guo <guoheyi@...wei.com>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: Jason Cooper <jason@...edaemon.net>
> Cc: Marc Zyngier <maz@...nel.org>
> ---
>  drivers/irqchip/irq-gic-v3-its.c   | 2 +-
>  include/linux/irqchip/arm-gic-v3.h | 3 +++
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index 787e8eec9a7f..d31e863bc9ef 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -2831,7 +2831,7 @@ static void its_vpe_schedule(struct its_vpe *vpe)
>  	val  = virt_to_phys(page_address(vpe->vpt_page)) &
>  		GENMASK_ULL(51, 16);
>  	val |= GICR_VPENDBASER_RaWaWb;
> -	val |= GICR_VPENDBASER_NonShareable;
> +	val |= GICR_VPENDBASER_InnerShareable;
>  	/*
>  	 * There is no good way of finding out if the pending table is
>  	 * empty as we can race against the doorbell interrupt very
> diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h
> index 5cc10cf7cb3e..a184f875e451 100644
> --- a/include/linux/irqchip/arm-gic-v3.h
> +++ b/include/linux/irqchip/arm-gic-v3.h
> @@ -289,6 +289,9 @@
>  #define GICR_VPENDBASER_NonShareable					\
>  	GIC_BASER_SHAREABILITY(GICR_VPENDBASER, NonShareable)
>  
> +#define GICR_VPENDBASER_InnerShareable					\
> +	GIC_BASER_SHAREABILITY(GICR_VPENDBASER, InnerShareable)
> +
>  #define GICR_VPENDBASER_nCnB	GIC_BASER_CACHEABILITY(GICR_VPENDBASER, INNER, nCnB)
>  #define GICR_VPENDBASER_nC 	GIC_BASER_CACHEABILITY(GICR_VPENDBASER, INNER, nC)
>  #define GICR_VPENDBASER_RaWt	GIC_BASER_CACHEABILITY(GICR_VPENDBASER, INNER, RaWt)



-- 
Jazz is not dead. It just smells funny...