linux-kernel - Re: [PATCH V3] irqchip:gic: change access of gicc

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]

Date:	Mon, 8 Sep 2014 09:57:01 -0700
From:	Feng Kan <fkan@....com>
To:	Arun Chandran <achandran@...sta.com>
Cc:	"tglx@...utronix.de" <tglx@...utronix.de>,
	patches <patches@....com>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Marc Zyngier <marc.zyngier@....com>,
	Vinayak Kale <vkale@....com>
Subject: Re: [PATCH V3] irqchip:gic: change access of gicc_ctrl register to
 read modify write.

On Mon, Sep 8, 2014 at 1:52 AM, Arun Chandran <achandran@...sta.com> wrote:
> Hi Feng,
>
> Is this patch still needed to fix perf calltrace?
Yes, if the someone decide to run the secure mode of the GIC, which is
what is present in
the kernel tree now.

> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/265814.html
>
> On Fri, Feb 28, 2014 at 3:06 AM, Feng Kan <fkan@....com> wrote:
>> This change is made to preserve the GIC v2 releated bits in the
>> GIC_CPU_CTRL register (also known as the GICC_CTLR register in spec).
>> The original code only set the enable/disable group bit in this register.
>> This code will preserve the bypass bits configured by the bootload except
>> the enable/disable bit. The main reason for this change is to allow the
>> bypass bits specified in the v2 spec to remain untouched by the current
>> GIC code. In the X-Gene platform, the bypass functionality is not used
>> and bypass must be disabled at all time.
>>
>> Signed-off-by: Vinayak Kale <vkale@....com>
>> Acked-by: Anup Patel <apatel@....com>
>> Signed-off-by: Feng Kan <fkan@....com>
>> ---
>> V3 Changes:
>>         - Sorry, forgot to change the mask for cpu_init path
>>           assumed bootloader setup bits correctly.
>> V2 Changes:
>>         - only mask off v2 bypass bits
>>  drivers/irqchip/irq-gic.c |   32 +++++++++++++++++++++++++++++---
>>  1 files changed, 29 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
>> index 341c601..3ca7995 100644
>> --- a/drivers/irqchip/irq-gic.c
>> +++ b/drivers/irqchip/irq-gic.c
>> @@ -418,6 +418,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
>>         void __iomem *dist_base = gic_data_dist_base(gic);
>>         void __iomem *base = gic_data_cpu_base(gic);
>>         unsigned int cpu_mask, cpu = smp_processor_id();
>> +       unsigned int ctrl_mask;
>>         int i;
>>
>>         /*
>> @@ -449,13 +450,29 @@ static void gic_cpu_init(struct gic_chip_data *gic)
>>                 writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4 / 4);
>>
>>         writel_relaxed(0xf0, base + GIC_CPU_PRIMASK);
>> -       writel_relaxed(1, base + GIC_CPU_CTRL);
>> +
>> +       ctrl_mask = readl(base + GIC_CPU_CTRL);
>
> readl_relaxed() here?
>
>> +
>> +       /* Mask out the gic v2 bypass bits */
>> +       ctrl_mask &= 0x1e0;
>> +
>> +       /* Enable group 0 */
>> +       ctrl_mask |= 0x1;
>> +       writel_relaxed(ctrl_mask, base + GIC_CPU_CTRL);
>>  }
>>
>>  void gic_cpu_if_down(void)
>>  {
>> +       unsigned int ctrl_mask;
>>         void __iomem *cpu_base = gic_data_cpu_base(&gic_data[0]);
>> -       writel_relaxed(0, cpu_base + GIC_CPU_CTRL);
>> +
>> +       ctrl_mask = readl(cpu_base + GIC_CPU_CTRL);
> readl_relaxed() here?
>
>> +       /*
>> +        * Disable grp enable bit, leave the bypass bits alone as changing
>> +        * them could leave the system unstable
>> +        */
>> +       ctrl_mask &= 0x1e0;
>> +       writel_relaxed(ctrl_mask, cpu_base + GIC_CPU_CTRL);
>>  }
>>
>>  #ifdef CONFIG_CPU_PM
>> @@ -566,6 +583,7 @@ static void gic_cpu_restore(unsigned int gic_nr)
>>  {
>>         int i;
>>         u32 *ptr;
>> +       unsigned int ctrl_mask;
>>         void __iomem *dist_base;
>>         void __iomem *cpu_base;
>>
>> @@ -590,7 +608,15 @@ static void gic_cpu_restore(unsigned int gic_nr)
>>                 writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4);
>>
>>         writel_relaxed(0xf0, cpu_base + GIC_CPU_PRIMASK);
>> -       writel_relaxed(1, cpu_base + GIC_CPU_CTRL);
>> +
>> +       ctrl_mask = readl(cpu_base + GIC_CPU_CTRL);
>
> readl_relaxed() here?
Trying to make sure read is completed before write back out to gic.
>
>> +
>> +       /* Mask out the gic v2 bypass bits */
>> +       ctrl_mask &= 0x1e0;
>> +
>> +       /* Enable group 0 */
>> +       ctrl_mask |= 0x1;
>> +       writel_relaxed(ctrl_mask, cpu_base + GIC_CPU_CTRL);
>>  }
>>
>
> --Arun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/