Paths

Table of Contentst

-
sys/x86/acpica/
-
x86/
-
acpica/
5
acpi_wakeup.c

Allocate non-contiguous pages for the ACPI wakeup handler
ClosedPublic
Actions

Authored by markj on Jul 8 2016, 12:46 AM.

Details

Reviewers

kib
jkim

Commits

rS302793: Allow ACPI wakeup code and page tables to be stored in non-contiguous pages.

Summary

This implements the suggestion in D6945. With this change, we allocate
four pages. The first is used for the wakeup code, and on amd64 the last
three are used for page tables. This change makes it easier for the kernel
to satisfy the allocation request(s), without adding much complexity.

Test Plan

ACPI S3 suspend/resume on my Lenovo G580 laptop

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 4457
Build 4508: arc lint + arc unit

Event Timeline

markj updated this revision to Diff 18216.Jul 8 2016, 12:46 AM

markj retitled this revision from to Allocate non-contiguous pages for the ACPI wakeup handler.

markj edited the test plan for this revision. (Show Details)

markj updated this object.

Herald added a subscriber: imp. · View Herald TranscriptJul 8 2016, 12:46 AM

markj updated this object.Jul 8 2016, 12:50 AM

markj edited the test plan for this revision. (Show Details)

markj added reviewers: kib, jkim.

kib accepted this revision.Jul 8 2016, 2:11 AM

kib edited edge metadata.

kib added inline comments.

sys/x86/acpica/acpi_wakeup.c
348	For this to work, you should clear all pages in array before doing contigmalloc() loop.

This revision is now accepted and ready to land.Jul 8 2016, 2:11 AM

markj added inline comments.Jul 8 2016, 2:16 AM

sys/x86/acpica/acpi_wakeup.c
348	Yikes, thanks. This array was static in an earlier revision, so I didn't bother.

Zero out the page array to fix cleanup code.

This revision now requires review to proceed.Jul 8 2016, 2:27 AM

WAKECODE_PADDR() and WAKECODE_VADDR() macros do nothing now. Please replace them with sc->acpi_wakephys and sc->acpi_wakeaddr.

This revision now requires changes to proceed.Jul 8 2016, 4:11 PM

ACPI_PAGETABLES does nothing now. Please replace it with ACPI_WAKEPAGES, e.g.,

#ifdef __amd64__
#define ACPI_WAKEPAGES       4
#else
#define ACPI_WAKEPAGES       1
#endif

Remove WAKECODE_{P,V}ADDR and ACPI_PAGETABLES.

Are there any further comments?

In D7154#149808, @markj wrote:

Are there any further comments?

I am not sure the page tables are correct. I'll take a closer look when I find some free time today.

Looks good. Please commit.

This revision is now accepted and ready to land.Jul 13 2016, 5:57 PM

Closed by commit rS302793: Allow ACPI wakeup code and page tables to be stored in non-contiguous pages. (authored by markj). · Explain WhyJul 14 2016, 12:38 AM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Path

Size

sys/

x86/

acpica/

acpi_wakeup.c

70 lines

Diff 18219

View Options

sys/x86/acpica/acpi_wakeup.c

	Show First 20 Lines • Show All 42 Lines • ▼ Show 20 Lines
	#include <sys/cpuset.h>			#include <sys/cpuset.h>
	#include <sys/lock.h>			#include <sys/lock.h>
	#include <sys/mutex.h>			#include <sys/mutex.h>

	#include <vm/vm.h>			#include <vm/vm.h>
	#include <vm/pmap.h>			#include <vm/pmap.h>

	#include <machine/bus.h>			#include <machine/bus.h>
				#include <machine/cpu.h>
	#include <machine/intr.h>			#include <machine/intr.h>

	#include "pic_if.h"			#include "pic_if.h"

	#include "gic_v3_reg.h"			#include "gic_v3_reg.h"
	#include "gic_v3_var.h"			#include "gic_v3_var.h"

	/* Device and PIC methods */			/* Device and PIC methods */
	▲ Show 20 Lines • Show All 166 Lines • ▼ Show 20 Lines
	* PIC interface.			* PIC interface.
	*/			*/
	static void			static void
	gic_v3_dispatch(device_t dev, struct trapframe *frame)			gic_v3_dispatch(device_t dev, struct trapframe *frame)
	{			{
	uint64_t active_irq;			uint64_t active_irq;

	while (1) {			while (1) {
				if (CPU_MATCH_ERRATA_CAVIUM_THUNDER_1_1) {
				/*
				* Hardware: Cavium ThunderX
				* Chip revision: Pass 1.0, Pass 1.1
				* ERRATUM: 22978, 23154
				impUnsubmitted Not Done Inline Actions So both Pass 1.0 and 1.1 have this errata. Is Cavium really going to market with Pass 1.1? that likely means we'd have to keep this around for a long time. If that's just the first wave and there will be a Pass 2.0 which fixes this, that would be better. imp: So both Pass 1.0 and 1.1 have this errata. Is Cavium really going to market with Pass 1.1? that…
				emasteUnsubmitted Not Done Inline Actions As far as we know right now, yes. emaste: As far as we know right now, yes.
				*/
				__asm __volatile("nop;nop;nop;nop;nop;nop;nop;nop;\n"
				"mrs %0, ICC_IAR1_EL1\n"
				"nop;nop;nop;nop;dsb sy;" : "=&r" (active_irq));
				} else {
	active_irq = gic_icc_read(IAR1);			active_irq = gic_icc_read(IAR1);
				andrewUnsubmitted Not Done Inline Actions Quoting https://lkml.org/lkml/2015/7/6/247 If you need an instruction sequence to be consecutive, then it _must_ be one single asm(). I would also like a comment explaining why all the nops are needed, and on what hardware the errata applies to. There have been a few places with other architectures where they needed a collection of nops, but nobody can remember why, or knew if they are still needed. andrew: Quoting https://lkml.org/lkml/2015/7/6/247 If you need an instruction sequence to be…
				wma_semihalf.comAuthorUnsubmitted Not Done Inline Actions Fixed, comments added. Unfortunately, these are the only information than can go public. wma_semihalf.com: Fixed, comments added. Unfortunately, these are the only information than can go public.
				emasteUnsubmitted Not Done Inline Actions Is it possible to expand on the meaning of `Pass 1.0/1.1`? emaste: Is it possible to expand on the meaning of `Pass 1.0/1.1`?
				}

	if (__predict_false(active_irq == ICC_IAR1_EL1_SPUR))			if (__predict_false(active_irq == ICC_IAR1_EL1_SPUR))
	break;			break;

	if (__predict_true((active_irq >= GIC_FIRST_PPI &&			if (__predict_true((active_irq >= GIC_FIRST_PPI &&
	active_irq <= GIC_LAST_SPI) \|\| active_irq >= GIC_FIRST_LPI)) {			active_irq <= GIC_LAST_SPI) \|\| active_irq >= GIC_FIRST_LPI)) {
	arm_dispatch_intr(active_irq, frame);			arm_dispatch_intr(active_irq, frame);
	continue;			continue;
	▲ Show 20 Lines • Show All 342 Lines • Show Last 20 Lines

Allocate non-contiguous pages for the ACPI wakeup handlerClosedPublicActions