Hi,
On 09/28/2015 11:55 AM, Marc Zyngier wrote:
> On 28/09/15 11:11, Markos Chandras wrote:
>> From: Alex Smith <alex.smith@xxxxxxxxxx>
>>
>> The GIC provides a "user-mode visible" section containing a mirror of
>> the counter registers which can be mapped into user memory. This will
>> be used by the VDSO time function implementations, so provide a
>> function to map it in.
>>
>> When the GIC is not enabled in Kconfig a dummy inline version of this
>> function is provided, along with "#define gic_present 0", so that we
>> don't have to litter the VDSO code with ifdefs.
>>
>> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
>> Cc: Jason Cooper <jason@xxxxxxxxxxxxxx>
>> Cc: Marc Zyngier <marc.zyngier@xxxxxxx>
>> Cc: linux-kernel@xxxxxxxxxxxxxxx
>> Signed-off-by: Alex Smith <alex.smith@xxxxxxxxxx>
>> Signed-off-by: Markos Chandras <markos.chandras@xxxxxxxxxx>
>> ---
>> drivers/irqchip/irq-mips-gic.c | 27 +++++++++++++++++++++------
>> include/linux/irqchip/mips-gic.h | 24 ++++++++++++++++++++++--
>> 2 files changed, 43 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-mips-gic.c b/drivers/irqchip/irq-mips-gic.c
>> index af2f16bb8a94..c995b199ca32 100644
>> --- a/drivers/irqchip/irq-mips-gic.c
>> +++ b/drivers/irqchip/irq-mips-gic.c
>> @@ -13,6 +13,7 @@
>> #include <linux/irq.h>
>> #include <linux/irqchip.h>
>> #include <linux/irqchip/mips-gic.h>
>> +#include <linux/mm.h>
>> #include <linux/of_address.h>
>> #include <linux/sched.h>
>> #include <linux/smp.h>
>> @@ -29,6 +30,7 @@ struct gic_pcpu_mask {
>> DECLARE_BITMAP(pcpu_mask, GIC_MAX_INTRS);
>> };
>>
>> +static unsigned long gic_base_addr;
>> static void __iomem *gic_base;
>> static struct gic_pcpu_mask pcpu_masks[NR_CPUS];
>> static DEFINE_SPINLOCK(gic_lock);
>> @@ -301,6 +303,19 @@ int gic_get_c0_fdc_int(void)
>> GIC_LOCAL_TO_HWIRQ(GIC_LOCAL_INT_FDC));
>> }
>>
>> +int gic_map_user_section(struct vm_area_struct *vma, unsigned long base,
>> + unsigned long size)
>> +{
>> + unsigned long pfn;
>> +
>> + BUG_ON(!gic_present);
>
> Why do you have a BUG() here, while you're just returning -1 in the case
> where CONFIG_MIPS_GIC is not refined? This feels overly harsh to me.
I suppose i could change that to return -1 if git_present is not true.
>
>> + BUG_ON(size > USM_VISIBLE_SECTION_SIZE);
>
> Same here.
But I think this is different. The size of mapping has to be less than
USM_VISIBLE_SECTION_SIZE because that's the maximum data size exposed by
the GIC chip for userspace use. So if that's not true, then BUG_ON seems
like a sensible thing to do.
>
> - Does this code have to be in the irqchip driver? It really feels out
> of place, and I'd rather see a function that returns the mappable range
> to the VDSO code, where the mapping would occur.
>
> Thanks,
>
That does seem like a good idea. I will have a look
--
markos
