linux-kernel - Re: [PATCH v4 2/4] irqchip/gic: common: add support to device tree based quirks

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] [day] [month] [year] [list]

Date:   Mon, 10 Dec 2018 13:19:52 +0000
From:   Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
To:     Marc Zyngier <marc.zyngier@....com>
Cc:     sudeep.holla@....com, tglx@...utronix.de, jason@...edaemon.net,
        linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        rnayak@...eaurora.org, sboyd@...nel.org,
        bjorn.andersson@...aro.org, nicolas.dechesne@...aro.org,
        ctatlor97@...il.com, vkoul@...nel.org, robh+dt@...nel.org,
        devicetree@...r.kernel.org
Subject: Re: [PATCH v4 2/4] irqchip/gic: common: add support to device tree
 based quirks



On 10/12/18 10:55, Marc Zyngier wrote:
>>   
>> +void gic_enable_of_quirks(const struct device_node *np,
>> +			  const struct gic_quirk *quirks, void *data)
>> +{
>> +	for (; quirks->desc; quirks++) {
> So you expect quirks->desc to be NULL at some point, just like for any
> other quirk table we have.
> 
>> +		if (!of_device_is_compatible(np, quirks->compatible))
>> +			continue;
>> +		if (quirks->init(data))
>> +			pr_info("GIC: enabling workaround for %s\n",
>> +				quirks->desc);
>> +	}
>> +}
>> +
>>   void gic_enable_quirks(u32 iidr, const struct gic_quirk *quirks,
>>   		void *data)
>>   {
>> diff --git a/drivers/irqchip/irq-gic-common.h b/drivers/irqchip/irq-gic-common.h
>> index 3919cd7c5285..97e58fb6c232 100644
>> --- a/drivers/irqchip/irq-gic-common.h
>> +++ b/drivers/irqchip/irq-gic-common.h
>> @@ -23,6 +23,7 @@
>>   
>>   struct gic_quirk {
>>   	const char *desc;
>> +	const char *compatible;
>>   	bool (*init)(void *data);
>>   	u32 iidr;
>>   	u32 mask;
>> @@ -35,6 +36,8 @@ void gic_dist_config(void __iomem *base, int gic_irqs,
>>   void gic_cpu_config(void __iomem *base, void (*sync_access)(void));
>>   void gic_enable_quirks(u32 iidr, const struct gic_quirk *quirks,
>>   		void *data);
>> +void gic_enable_of_quirks(const struct device_node *np,
>> +			  const struct gic_quirk *quirks, void *data);
>>   
>>   void gic_set_kvm_info(const struct gic_kvm_info *info);
>>   
>> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
>> index 29e9d47be97d..c95796fa4de6 100644
>> --- a/drivers/irqchip/irq-gic-v3.c
>> +++ b/drivers/irqchip/irq-gic-v3.c
>> @@ -1271,6 +1271,9 @@ static void __init gic_of_setup_kvm_info(struct device_node *node)
>>   	gic_set_kvm_info(&gic_v3_kvm_info);
>>   }
>>   
>> +static const struct gic_quirk gic_quirks[] = {
>> +};
> ... and yet here you provide an empty table. That's not going to work
> very well. You definitely need to have an empty entry at the end of the
> array, always.
> 
> I guess you want to test your code on a non affected platform, and I'm
> pretty sure you'll see it exploding.

Yes, Should have carefully looked at this!!
We need an empty entry at the end!
I will fix it and send a new version!

Thanks,
srini
>