[PATCH] ARM64: kernel: implement ACPI parking protocol

[Mark Salter]

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.