[PATCH] ARM64: kernel: implement ACPI parking protocol

[lorenzo.pieralisi@arm.com (Lorenzo Pieralisi)]

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