Re: [Xen-devel] Re: Paravirtualizing bits of acpi access

[Jeremy Fitzhardinge <jeremy@xxxxxxxx>]

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

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel