Rafael J. Wysocki wrote:
On Monday 23 March 2009, Tian, Kevin wrote:
And then Xen jumps in to finish remaining steps. From this angle,
Xen is not a completely new platform and, well, S3 is more like a
'S1' type from dom0's p.o.v with a different trigger method. Then is
it overkilled to introduce a new set of ops with 99% content
duplicated?
IMO, no, it isn't.
Hm. Well, lets take acpi_suspend_enter() as a specific example. The
Xen change here is:
@@ -240,11 +240,20 @@ static int acpi_suspend_enter(suspend_state_t pm_state)
barrier();
status = acpi_enter_sleep_state(acpi_state);
break;
case ACPI_STATE_S3:
- do_suspend_lowlevel();
+ if (!xen_pv_domain())
+ do_suspend_lowlevel();
+ else {
+ /*
+ * Xen will save and restore CPU context, so
+ * we can skip that and just go straight to
+ * the suspend.
+ */
+ acpi_enter_sleep_state(acpi_state);
+ }
break;
}
/* If ACPI is not enabled by the BIOS, we need to enable it here. */
if (set_sci_en_on_resume)
Which is, functionally, adding one if() and a new line of code, in the
middle of a ~70 line function.
Are you suggesting that it would be best to copy this whole function so
that I can put one line of Xen-specific code in the middle, rather than
just making this change?
Some other functions, the Xen vs. non-Xen changes are larger;
acpi_sleep_prepare() could reasonably have a Xen-specific variant
because a big chunk of it is setting up the wakeup vector (which is
unnecessary under Xen), and the rest can be easily pulled into common
code. But unfortunately acpi_sleep_prepare isn't itself an operation,
and is only called at the bottom of 2-3 level deep callchains.
I think that rather than having a separate xen-acpi
platform_suspend_ops, it would make more sense to have a acpi_ops within
acpi/sleep.c and handle the differences that way. I'll see how it turns
out.
J
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
