Re: [PATCH 15/15] Revert "x86: default to reboot via ACPI"

Re: [PATCH 15/15] Revert "x86: default to reboot via ACPI"

[Len Brown]

> > This reverts commit c7ffa6c26277b403920e2255d10df849bd613380.

I agree that the 2.6.27 default was changed to ACPI for the wrong reason.

However, I think it was the right thing to do,
and if you didn't propose it, I would.

My expectation is that with the ACPI default, our problem
is working around a finite list of old machines that don't work;
while with the default KBD, our problem is working around
a potentially unbounded list of yet to be shipped machines
who may only be tested and work using the ACPI method.

So I recommend leaving the default as ACPI for a while to
see how it goes.

thanks,
-Len

ps. please cc: linux-acpi@vger.kernel.org on ACPI related changes.

Re: [PATCH 15/15] Revert "x86: default to reboot via ACPI"

[Matthew Garrett]

On Thu, Nov 06, 2008 at 02:50:06PM -0500, Len Brown wrote:

> My expectation is that with the ACPI default, our problem
> is working around a finite list of old machines that don't work;
> while with the default KBD, our problem is working around
> a potentially unbounded list of yet to be shipped machines
> who may only be tested and work using the ACPI method.

Does Windows default to using the ACPI method now?

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 15/15] Revert "x86: default to reboot via ACPI"

[Len Brown]

> > My expectation is that with the ACPI default, our problem
> > is working around a finite list of old machines that don't work;
> > while with the default KBD, our problem is working around
> > a potentially unbounded list of yet to be shipped machines
> > who may only be tested and work using the ACPI method.
> 
> Does Windows default to using the ACPI method now?

That is my guess, based on the fact that we've seen
newer machines that don't reboot w/o using the ACPI reset reg.

-Len

Re: [PATCH 15/15] Revert "x86: default to reboot via ACPI"

[Matthew Garrett]

On Thu, Nov 06, 2008 at 05:17:25PM -0500, Len Brown wrote:
> > Does Windows default to using the ACPI method now?
> 
> That is my guess, based on the fact that we've seen
> newer machines that don't reboot w/o using the ACPI reset reg.

We've seen machines that won't reboot for a variety of reasons in the 
past. In some cases this has seemed to be due to hardware being in a 
state that the BIOS didn't expect. Hitting the SMI trap behind the ACPI 
register might work around this, but I suspect that in many cases we 
could achieve the same effect by spending more time trying to work out 
whether there's any common themes in the failures.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 15/15] Revert "x86: default to reboot via ACPI"

[Zhao Yakui]

On Fri, 2008-11-07 at 06:17 +0800, Len Brown wrote:
> 
> > > My expectation is that with the ACPI default, our problem
> > > is working around a finite list of old machines that don't work;
> > > while with the default KBD, our problem is working around
> > > a potentially unbounded list of yet to be shipped machines
> > > who may only be tested and work using the ACPI method.
> > 
> > Does Windows default to using the ACPI method now?
> 
> That is my guess, based on the fact that we've seen
> newer machines that don't reboot w/o using the ACPI reset reg.
With the help of KVM I find that the windows will be rebooted by writing
RESET_VALUE to RESET_REG I/O port if the RESET_REG_SUP bit is not
zero(It indicates whether ACPI reboot is supported).
IMO maybe the ACPI reboot is the first choice. If it can't, then it will
fall back to other mode.

Thanks.
> 
> -Len
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 15/15] Revert "x86: default to reboot via ACPI"

[Matthew Garrett]

On Fri, Nov 07, 2008 at 09:01:19AM +0800, Zhao Yakui wrote:

> With the help of KVM I find that the windows will be rebooted by writing
> RESET_VALUE to RESET_REG I/O port if the RESET_REG_SUP bit is not
> zero(It indicates whether ACPI reboot is supported).
> IMO maybe the ACPI reboot is the first choice. If it can't, then it will
> fall back to other mode.

Hmm. But we're seeing some machines that end up very confused if 
rebooted via ACPI. I guess we need to run Vista on them to find out how 
they behave. What OSI strings did your KVM setup expose? We know that 
Windows changes behaviour under various circumstances depending on which 
OS the firmware requests, so it's almost possible that this is another 
of those cases.

-- 
Matthew Garrett | mjg59-1xO5oi07KQx4cg9Nei1l7Q@public.gmane.org

Re: [PATCH 15/15] Revert "x86: default to reboot via ACPI"

[Avi Kivity]

Matthew Garrett wrote:
> Hmm. But we're seeing some machines that end up very confused if 
> rebooted via ACPI. I guess we need to run Vista on them to find out how 
> they behave. What OSI strings did your KVM setup expose? We know that 
> Windows changes behaviour under various circumstances depending on which 
> OS the firmware requests, so it's almost possible that this is another 
> of those cases.
>   

Isn't it the other way around?  The firmware changes behavior depending 
on how the OS identifies itself?

Reboot is a fixed feature IIRC, so it cannot change depending on 
identification strings.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 15/15] Revert "x86: default to reboot via ACPI"

[Matthew Garrett]

On Sun, Nov 09, 2008 at 12:11:03PM +0200, Avi Kivity wrote:
> Matthew Garrett wrote:
> >Hmm. But we're seeing some machines that end up very confused if 
> >rebooted via ACPI. I guess we need to run Vista on them to find out how 
> >they behave. What OSI strings did your KVM setup expose? We know that 
> >Windows changes behaviour under various circumstances depending on which 
> >OS the firmware requests, so it's almost possible that this is another 
> >of those cases.
> >  
> 
> Isn't it the other way around?  The firmware changes behavior depending 
> on how the OS identifies itself?

That also happens, yes.

> Reboot is a fixed feature IIRC, so it cannot change depending on 
> identification strings.

The ID strings that the firmware requests give a good idea about which 
operating systems the machine has been tested with. If Vista uses the 
ACPI method then having the firmware request the OSI string for Vista 
gives us a good indication that it's safe to use the ACPI method.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 15/15] Revert "x86: default to reboot via ACPI"

[Robert Hancock]

Matthew Garrett wrote:
> On Fri, Nov 07, 2008 at 09:01:19AM +0800, Zhao Yakui wrote:
> 
>> With the help of KVM I find that the windows will be rebooted by writing
>> RESET_VALUE to RESET_REG I/O port if the RESET_REG_SUP bit is not
>> zero(It indicates whether ACPI reboot is supported).
>> IMO maybe the ACPI reboot is the first choice. If it can't, then it will
>> fall back to other mode.
> 
> Hmm. But we're seeing some machines that end up very confused if 
> rebooted via ACPI. I guess we need to run Vista on them to find out how 
> they behave. What OSI strings did your KVM setup expose? We know that 
> Windows changes behaviour under various circumstances depending on which 
> OS the firmware requests, so it's almost possible that this is another 
> of those cases.

Given that Windows behavior, this patch seems suspicious:

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=8fd145917fb62368a9b80db59562c20576238f5a

This patch ignores the RESET_REG_SUP flag and just tries using the reset 
register anyway if it thinks it's valid. So we may attempt ACPI reset on 
machines which don't indicate it's supported.

The patch description mentioned that some machines didn't reboot after 
S3 suspend without this patch. However, we recently had this patch merged:

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=a68823ee5285e65b51ceb96f8b13a5b4f99a6888

Is it possible that the problem fixed there is the true cause of this 
reboot after S3 problem?

Re: [PATCH 15/15] Revert "x86: default to reboot via ACPI"

[Matthew Garrett]

On Thu, Nov 06, 2008 at 07:30:43PM -0600, Robert Hancock wrote:

> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=8fd145917fb62368a9b80db59562c20576238f5a
> 
> This patch ignores the RESET_REG_SUP flag and just tries using the reset 
> register anyway if it thinks it's valid. So we may attempt ACPI reset on 
> machines which don't indicate it's supported.

Yeah, that sounds very wrong.

> The patch description mentioned that some machines didn't reboot after 
> S3 suspend without this patch. However, we recently had this patch merged:
> 
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=a68823ee5285e65b51ceb96f8b13a5b4f99a6888
> 
> Is it possible that the problem fixed there is the true cause of this 
> reboot after S3 problem?

Oh, yeah, could be. Given the From:, I should really have thought of 
that :)

-- 
Matthew Garrett | mjg59-1xO5oi07KQx4cg9Nei1l7Q@public.gmane.org

Re: [PATCH 15/15] Revert "x86: default to reboot via ACPI"

[Len Brown]

On Fri, 7 Nov 2008, Matthew Garrett wrote:

> On Thu, Nov 06, 2008 at 07:30:43PM -0600, Robert Hancock wrote:
> 
> > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=8fd145917fb62368a9b80db59562c20576238f5a
> > 
> > This patch ignores the RESET_REG_SUP flag and just tries using the reset 
> > register anyway if it thinks it's valid. So we may attempt ACPI reset on 
> > machines which don't indicate it's supported.
> 
> Yeah, that sounds very wrong.

As it turns out, it was an incorrect guess on our part on how to be "bug 
compatible" and I'm reverting it per the regression report here:

http://bugzilla.kernel.org/show_bug.cgi?id=11942

-Len

> > The patch description mentioned that some machines didn't reboot after 
> > S3 suspend without this patch. However, we recently had this patch merged:
> > 
> > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=a68823ee5285e65b51ceb96f8b13a5b4f99a6888
> > 
> > Is it possible that the problem fixed there is the true cause of this 
> > reboot after S3 problem?
> 
> Oh, yeah, could be. Given the From:, I should really have thought of 
> that :)
> 
> -- 
> Matthew Garrett | mjg59@srcf.ucam.org
> 

Re: [PATCH 15/15] Revert "x86: default to reboot via ACPI"

[Rafael J. Wysocki]

On Friday, 7 of November 2008, Robert Hancock wrote:
> Matthew Garrett wrote:
> > On Fri, Nov 07, 2008 at 09:01:19AM +0800, Zhao Yakui wrote:
> > 
> >> With the help of KVM I find that the windows will be rebooted by writing
> >> RESET_VALUE to RESET_REG I/O port if the RESET_REG_SUP bit is not
> >> zero(It indicates whether ACPI reboot is supported).
> >> IMO maybe the ACPI reboot is the first choice. If it can't, then it will
> >> fall back to other mode.
> > 
> > Hmm. But we're seeing some machines that end up very confused if 
> > rebooted via ACPI. I guess we need to run Vista on them to find out how 
> > they behave. What OSI strings did your KVM setup expose? We know that 
> > Windows changes behaviour under various circumstances depending on which 
> > OS the firmware requests, so it's almost possible that this is another 
> > of those cases.
> 
> Given that Windows behavior, this patch seems suspicious:
> 
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=8fd145917fb62368a9b80db59562c20576238f5a
> 
> This patch ignores the RESET_REG_SUP flag and just tries using the reset 
> register anyway if it thinks it's valid. So we may attempt ACPI reset on 
> machines which don't indicate it's supported.
> 
> The patch description mentioned that some machines didn't reboot after 
> S3 suspend without this patch. However, we recently had this patch merged:
> 
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=a68823ee5285e65b51ceb96f8b13a5b4f99a6888
> 
> Is it possible that the problem fixed there is the true cause of this 
> reboot after S3 problem?

Generally, it is.

Should it regarded as -stable material, BTW, or is it already in -stable?

Matthew?

Thanks,
Rafael