linux-kernel - Re: [PATCH 1/1] irq-gic: add capability to set bypass flag in GIC

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:	Fri, 22 Nov 2013 11:37:08 -0800
From:	Feng Kan <fkan@....com>
To:	Marc Zyngier <marc.zyngier@....com>
Cc:	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"jcm@...hat.com" <jcm@...hat.com>,
	Catalin Marinas <Catalin.Marinas@....com>,
	Mark Rutland <Mark.Rutland@....com>
Subject: Re: [PATCH 1/1] irq-gic: add capability to set bypass flag in GIC

>>       set_handle_irq(gic_handle_irq);
>>
>> +     gic->bypass_flag = (bypass_val & 0xf) << 4;

>Beware, the top 2 bits are reserved on GICv1, and shouldn't be messed with.

The only time those bits are touched are when the dts is modified with the
bypass flag. Otherwise those bits remain untouched as before.


>> -     gic_init_bases(gic_cnt, -1, dist_base, cpu_base, percpu_offset, node);
>> +     if (of_property_read_u32(node, "bypass-flags", &bypass_val))
>> +             bypass_val = 0;
>
> [adding Mark on Cc, so he can comment on the DT parts]
>
> Where's the DT documentation update? Also, as this is an
> implementation-specific quirk, you should consider using a separate
> compatible string and move the handling of this issue into some X-Gene
> specific code.

Yes, the only way to do that nicely would be to add another irq-xgene file.
Since I see little of gic specific implementation in the irq-gic file. To add
another irq-xgene file for one change seems excessive at the time. Given
that in v2 these bypass bits are added, is it better to check for v2 dts
property before modify the CPU_CTRL register?

>
>> +     gic_init_bases(gic_cnt, -1, dist_base, cpu_base, percpu_offset, bypass_val, node);
>>
>>       if (parent) {
>>               irq = irq_of_parse_and_map(node, 0);
>> diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
>> index 0e5d9ec..999515b 100644
>> --- a/include/linux/irqchip/arm-gic.h
>> +++ b/include/linux/irqchip/arm-gic.h
>> @@ -64,14 +64,14 @@ struct device_node;
>>  extern struct irq_chip gic_arch_extn;
>>
>>  void gic_init_bases(unsigned int, int, void __iomem *, void __iomem *,
>> -                 u32 offset, struct device_node *);
>> +                 u32 offset, u32 bypass_val, struct device_node *);
>>  void gic_cascade_irq(unsigned int gic_nr, unsigned int irq);
>>  void gic_cpu_if_down(void);
>>
>>  static inline void gic_init(unsigned int nr, int start,
>>                           void __iomem *dist , void __iomem *cpu)
>>  {
>> -     gic_init_bases(nr, start, dist, cpu, 0, NULL);
>> +     gic_init_bases(nr, start, dist, cpu, 0, 0, NULL);
>>  }
>>
>>  #endif /* __ASSEMBLY */
>>
>
> Cheers,
>
>         M.
> --
> Jazz is not dead. It just smells funny...
>
--
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/