[PATCH] ARM64: kernel: implement ACPI parking protocol

[msalter@redhat.com (Mark Salter)]

On Mon, 2015-08-24 at 18:13 +0100, Lorenzo Pieralisi wrote:
> Hi Mark,
> 
> On Thu, Jul 16, 2015 at 07:23:46PM +0100, Mark Salter wrote:
> > On Thu, 2015-07-16 at 12:05 -0600, Al Stone wrote:
> > 
> > > On 07/16/2015 11:12 AM, Lorenzo Pieralisi wrote:
> > 
> > > > On Thu, Jul 16, 2015 at 05:17:11PM +0100, Mark Salter wrote:
> > 
> > > > > On Wed, 2015-07-15 at 12:33 +0100, Lorenzo Pieralisi wrote:
> > > > > 
> > 
> > > > > > The SBBR and ACPI specifications allow ACPI based systems that 
> > > > > > do not
> > > > > > implement PSCI (eg systems with no EL3) to boot through the 
> > > > > > ACPI parking
> > > > > > protocol specification[1].
> > > > > > 
> > > > > > This patch implements the ACPI parking protocol CPU operations, 
> > > > > > and adds
> > > > > > code that eases parsing the parking protocol data structures to 
> > > > > > the
> > > > > > ARM64 SMP initializion carried out at the same time as cpus 
> > > > > > enumeration.
> > > > > > 
> > > > > > To wake-up the CPUs from the parked state, this patch 
> > > > > > implements a
> > > > > > wakeup IPI for ARM64 (ie arch_send_wakeup_ipi_mask()) that 
> > > > > > mirrors the
> > > > > > ARM one, so that a specific IPI is sent for wake-up purpose in 
> > > > > > order
> > > > > > to distinguish it from other IPI sources.
> > > > > > 
> > > > > > Given the current ACPI MADT parsing API, the patch implements a 
> > > > > > glue
> > > > > > layer that helps passing MADT GICC data structure from SMP 
> > > > > > initialization
> > > > > 
> > > > > Somewhat off topic, but this reminds once again, that it might be
> > > > > better to generalize the ACPI_MADT_TYPE_GENERIC_INTERRUPT so that 
> > > > > it
> > > > > could be done in one pass. Currently, the SMP code and the GIC 
> > > > > code
> > > > > need boot-time info from ACPI_MADT_TYPE_GENERIC_INTERRUPT tables. 
> > > > > This
> > > > > patch adds parking protocol, and this patch:
> > > > > 
> > > > >  https://lkml.org/lkml/2015/5/1/203
> > > > > 
> > > > > need to get the PMU irq from the same table. I've been thinking 
> > > > > of
> > > > > something like a single loop through the table in setup.c with
> > > > > callouts to registered users of the various bits of data.
> > > > 
> > > > It is not off topic at all, it is bang on topic. I hate the code
> > > > as it stands forcing parsing the MADT in multiple places at 
> > > > different
> > > > times, that's why I added hooks to set the parking protocol entries
> > > > from smp.c and I know that's ugly, I posted it like this on purpose
> > > > to get feedback.
> > > > 
> > 
> > > > > Those users could register a handler function with something like 
> > > > > an
> > > > > ACPI_MADT_GIC_DECLARE() macro which would add a handler to a
> > > > > special linker section.
> > > > > 
> > > > > I could work up a separate patch if others think it a worthwhile
> > > > > thing to do.
> > > > 
> > > > Something simpler ? Like stashing the GICC entries (I know we need
> > > > permanent table mappings for that to work unless we create data
> > > > structures out of the MADT entries with the fields we are 
> > > > interested in)
> > > > for possible CPUS ?
> > > 
> > > Right -- it seems like it would be pretty straightforward to traverse
> > > the MADT once, and capture all the GICC subtables in a list of 
> > > pointers,
> > > or even make a copy of the contents of all of them.  Each user of the
> > > content would still have to traverse the list, though, unless data is
> > > reduced to only those things that are needed and the rest tossed. 
> > >  Even
> > > if only keep the info needed, I've still got a list of entries, one 
> > > for
> > > each CPU, I think, that I would still have to traverse.
> > > 
> > > If I have to register a handler to gather a specific bit of data 
> > > needed,
> > > I'm not understanding how that's any less complicated than what I 
> > > need
> > > to do today -- call acpi_parse_table_madt() with my GICC handler. 
> > >  Wouldn't
> > > both GICC subtable handlers be just about the same code?
> > > 
> > > I'm probably missing something obvious, but I'm not understanding 
> > > what problem
> > > is being solved....
> > > 
> > 
> > The difference is that GIC code and SMP code each loop
> > through the tables getting the info they need. If they
> > registered a handler, there is only one loop through
> > the tables regardless of how many handlers get registered.
> > Having each loop through as currently done isn't really
> > a performance issue (its boot-time only right now), but
> > there is duplicated code wrt validating the table entry
> > and calling the acpi API to do it. All of that could be
> > done in one place instead of duplicating it in different
> > places.
> 
> Did you have time to put the patch together ? I would like
> to post a v2 for this set soon.
> 
> Thank you !
> Lorenzo

No. The original idea I had wasn't going to work out because the
parsing had to happen at different times during the boot. I'm not
sure if there's a better way than what we're doing now.