RE: [Xen-devel] expose MWAIT to dom0

["Jan Beulich" <JBeulich@xxxxxxxxxx>]

>>> On 19.08.11 at 14:55, Jan Beulich wrote:
> >>> On 19.08.11 at 11:39, "Tian, Kevin" <kevin.tian@xxxxxxxxx> wrote:
> >>  From: Jan Beulich [mailto:JBeulich@xxxxxxxxxx] 
> >> Sent: Friday, August 19, 2011 5:32 PM
> >> 
> >> >>> On 19.08.11 at 10:34, "Tian, Kevin" <kevin.tian@xxxxxxxxx> wrote:
> >> >>  From: Jan Beulich [mailto:JBeulich@xxxxxxxxxx] 
> >> >> Sent: Friday, August 19, 2011 4:11 PM
> >> >>
> >> >> >>> On 19.08.11 at 03:31, "Tian, Kevin" <kevin.tian@xxxxxxxxx> wrote:
> >> >> > OK, to avoid the regression, if it's really cared, then we may change 
> >> >> > Xen
> >> >> > to support entering C-state by mwait with interrupt enabled.
> >> >>
> >> >> I don't think that's worth it.
> >> >>
> >> >> > But the next question is whether it's really worthy of enabling Xen PM
> >> >> > such way:
> >> >> >       - I think native Linux only supports mwait with 
> >> >> > break-on-interrupt
> >> >> > extension too. You may confirm on such machines which I think should
> >> >> > have no C2/C3 available. It's less likely for a customer to try PM on 
> >> >> > a
> >> >> > platform where native Linux fails to do that
> >> >>
> >> >> Looking at the code, I can't see why Linux wouldn't use the I/O method
> >> >> in this case instead.
> >> >
> >> > acpi_processor_ffh_cstate_probe_cpu:
> >> >         /* mwait ecx extensions INTERRUPT_BREAK should be supported
> >> for
> >> > C2/C3 */
> >> >         if (!(ecx & CPUID5_ECX_EXTENSIONS_SUPPORTED) ||
> >> >             !(ecx & CPUID5_ECX_INTERRUPT_BREAK)) {
> >> >                 retval = -1;
> >> >                 goto out;
> >> >         }
> >> >
> >> > arch_acpi_set_pdc_bits:
> >> >         /*
> >> >          * If mwait/monitor is unsupported, C2/C3_FFH will be disabled
> >> >          */
> >> >         if (!cpu_has(c, X86_FEATURE_MWAIT))
> >> >                 buf[2] &= ~(ACPI_PDC_C_C2C3_FFH);
> >> >
> >> 
> >> Right, this precludes the use of MWAIT, but it doesn't preclude using
> >> the I/O method.
> > 
> > In your special machine which has mwait but no break-on-interrupt extension:
> > 
> > arch_acpi_set_pdc_bits tells BIOS that mwait should be used.
> > 
> > BIOS then report _CST table with each entry filled with mwait style info
> > 
> > later when Linux verifies _CST entry by acpi_processor_ffh_cstate_probe_cpu,
> > it simply fails. No fallback to i/o method as BIOS is only notified in ACPI 
> > init time.
> 
> Oh, right, I just stumbled across that too (and didn't pay close enough
> attention before). The function really should be checking for the
> extension bits.
> 
> >> 
> >> >>
> >> >> >       - using mwait with interrupt enabled lacks the trace capability,
> >> >> > while w/o trace I don't think any customer would enable Xen PM w/o a
> >> >> > verification process.
> >> >> >
> >> >> > Another approach, if we really want to keep original I/O style, is to
> >> >> > reveal Xen's mwait related conditions in shared info page, say a 
> >> >> > simple
> >> >> > flag to indicate whether mwait bit should be set by pvops cpuid hook.
> >> >> > Xen will check mwait extension earlier before dom0 is launched, 
> >> >> > instead
> >> >> > of current point where dom0 registers cx info. This way there's no Xen
> >> >> > implementation detail encoded in dom0, while concerned regression
> >> >> > could be removed.
> >> >>
> >> >> The concept sounds reasonable, just that the shared info page probably
> >> >> isn't the right mechanism (after all this is Dom0-only information that
> >> >> we want to expose). A new platform sub-hypercall would probably be
> >> >> the better route.
> >> >>
> >> >
> >> > yes, that's a better choice.
> >> 
> >> Yet another idea - why don't we simply pass the buffer passed to
> >> arch_acpi_set_pdc_bits() down to Xen, rather than fiddling with the
> >> bits in Dom0? That would at once allow to not set ACPI_PDC_T_FFH
> >> (which I don't think Xen really supports at present).
> >> 
> >> Or really, depending on who controls what, the P, C, and T bits should
> >> be set by either Dom0 or Xen (so e.g. let Dom0 do what it currently
> >> does, and then let Xen override the bits it ought to control).
> > 
> > _PDC is encoded in AML language, and requires an ACPI parser which
> > is one thing we avoid in Xen. If Xen want to override those bits, then
> > whole ACPI component needs move down to Xen too.
> 
> No, I'm not saying the evaluation should be happening there. Below is
> a draft hypervisor patch (only compile tested so far).

Attached a patch that actually works (with a minimal Dom0 addition).

Jan

Attachment: acpi-set-pdc-bits.patch
Description: Text document

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