Re: [PATCH v8 6/9] drivers: perf: hisi: Add support for Hisilicon Djtag driver

[Will Deacon <will.deacon@arm.com>]

On Fri, Jun 09, 2017 at 03:18:39PM +0100, John Garry wrote:
> On 08/06/2017 17:35, Mark Rutland wrote:
> >Hi,
> >
> >On Mon, May 22, 2017 at 08:48:32PM +0800, Shaokun Zhang wrote:
> >>+/*
> >>+ * hisi_djtag_lock_v2: djtag lock to avoid djtag access conflict b/w kernel
> >>+ * and UEFI.
> >
> >The mention of UEFI here worries me somewhat, and I have a number of
> >questions specifically relating to how we interact with UEFI here.
> >
> 
> Hi Mark,
> 
> This djtag locking mechanism is an advisory software-only policy. The
> problem is the hardware designers made an interface which does not consider
> multiple agents in the system concurrently accessing the djtag registers.
> 
> System wide, djtag is used as an interface to other HW modules, but we only
> use for perf HW in the kernel.
> 
> >When precisely does UEFI need to touch the djtag hardware? e.g. does
> >this happen in runtime services? ... or completely asynchronously?
> >
> 
> Actually it's trusted firmware which accesses for L3 cache management in CPU
> hotplug
> 
> >What does UEFI do with djtag when it holds the lock?
> >
> 
> As mentioned, cache management
> 
> >Are there other software agents (e.g. secure firmware) which try to
> >take this lock?
> >
> 
> No
> 
> >Can you explain how the locking scheme works? e.g. is this an advisory
> >software-only policy, or does the hardware prohibit accesses from other
> >agents somehow?
> >
> 
> The locking scheme is a software solution to spinlock. It's uses djtag
> module select register as the spinlock flag, to avoid using some shared
> memory.
> 
> The tricky part is that there is no test-and-set hardware support, so we use
> this algorithm:
> - precondition: flag initially set unlocked
> 
> a. agent reads flag
>     - if not unlocked, continues to poll
>     - otherwise, writes agent's unique lock value to flag
> b. agent waits defined amount of time *uninterrupted* and then checks the
> flag

How do you figure out this time period? Doesn't it need to be no shorter
than the longest critical section?

Will