Re: [PATCH] x86/reboot: Add Dell Optiplex 7450 AIO reboot quirk

Re: [PATCH] x86/reboot: Add Dell Optiplex 7450 AIO reboot quirk

[Ingo Molnar]

* Alex Hung <alex.hung@canonical.com> wrote:

> Dell Optiplex 7450 AIO works with BOOT_ACPI; however, the quirk for
> "OptiPlex 745" changes its boot method to BOOT_BIOS and causes 7450 AIO
> hangs when rebooting; as a result, 7450 AIO is appended to overwrite
> BOOT_BIOS by BOOT_ACPI in order not to break the original 745 series
> 
> Signed-off-by: Alex Hung <alex.hung@canonical.com>
> ---
>  arch/x86/kernel/reboot.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
> index b7e1d5f..b404d05 100644
> --- a/arch/x86/kernel/reboot.c
> +++ b/arch/x86/kernel/reboot.c
> @@ -55,6 +55,19 @@ bool port_cf9_safe = false;
>   */
>  
>  /*
> + * Some machines require the "reboot=a" commandline options
> + */
> +static int __init set_acpi_reboot(const struct dmi_system_id *d)
> +{
> +	if (reboot_type != BOOT_ACPI) {
> +		reboot_type = BOOT_ACPI;
> +		pr_info("%s series board detected. Selecting %s-method for reboots.\n",
> +			d->ident, "ACPI");
> +	}
> +	return 0;
> +}
> +
> +/*
>   * Some machines require the "reboot=b" or "reboot=k"  commandline options,
>   * this quirk makes that automatic.
>   */
> @@ -395,6 +408,14 @@ static struct dmi_system_id __initdata reboot_dmi_table[] = {
>  			DMI_MATCH(DMI_PRODUCT_NAME, "Dell XPS710"),
>  		},
>  	},
> +	{	/* Handle problems with rebooting on Dell Optiplex 7450 AIO */
> +		.callback = set_acpi_reboot,
> +		.ident = "Dell OptiPlex 7450 AIO",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "OptiPlex 7450 AIO"),
> +		},
> +	},

I think it would be better to limit the original "Dell Optiplex 745" quirk to that 
machine alone instead of quirking the quirk on one of the (known) newer models 
with longer strings.

Thanks,

	Ingo

Re: [PATCH] x86/reboot: Add Dell Optiplex 7450 AIO reboot quirk

[Alex Hung]

Hi Ingo,

I agree with you but it can affect a lot of systems according to
163ea310b68bdde89b1ac633fbf8c0db290d3f86.

Do you think we should invert 163ea310 instead?

On Thu, Jul 14, 2016 at 8:58 PM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Alex Hung <alex.hung@canonical.com> wrote:
>
>> Dell Optiplex 7450 AIO works with BOOT_ACPI; however, the quirk for
>> "OptiPlex 745" changes its boot method to BOOT_BIOS and causes 7450 AIO
>> hangs when rebooting; as a result, 7450 AIO is appended to overwrite
>> BOOT_BIOS by BOOT_ACPI in order not to break the original 745 series
>>
>> Signed-off-by: Alex Hung <alex.hung@canonical.com>
>> ---
>>  arch/x86/kernel/reboot.c | 21 +++++++++++++++++++++
>>  1 file changed, 21 insertions(+)
>>
>> diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
>> index b7e1d5f..b404d05 100644
>> --- a/arch/x86/kernel/reboot.c
>> +++ b/arch/x86/kernel/reboot.c
>> @@ -55,6 +55,19 @@ bool port_cf9_safe = false;
>>   */
>>
>>  /*
>> + * Some machines require the "reboot=a" commandline options
>> + */
>> +static int __init set_acpi_reboot(const struct dmi_system_id *d)
>> +{
>> +     if (reboot_type != BOOT_ACPI) {
>> +             reboot_type = BOOT_ACPI;
>> +             pr_info("%s series board detected. Selecting %s-method for reboots.\n",
>> +                     d->ident, "ACPI");
>> +     }
>> +     return 0;
>> +}
>> +
>> +/*
>>   * Some machines require the "reboot=b" or "reboot=k"  commandline options,
>>   * this quirk makes that automatic.
>>   */
>> @@ -395,6 +408,14 @@ static struct dmi_system_id __initdata reboot_dmi_table[] = {
>>                       DMI_MATCH(DMI_PRODUCT_NAME, "Dell XPS710"),
>>               },
>>       },
>> +     {       /* Handle problems with rebooting on Dell Optiplex 7450 AIO */
>> +             .callback = set_acpi_reboot,
>> +             .ident = "Dell OptiPlex 7450 AIO",
>> +             .matches = {
>> +                     DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
>> +                     DMI_MATCH(DMI_PRODUCT_NAME, "OptiPlex 7450 AIO"),
>> +             },
>> +     },
>
> I think it would be better to limit the original "Dell Optiplex 745" quirk to that
> machine alone instead of quirking the quirk on one of the (known) newer models
> with longer strings.
>
> Thanks,
>
>         Ingo



-- 
Cheers,
Alex Hung

Re: [PATCH] x86/reboot: Add Dell Optiplex 7450 AIO reboot quirk

[Ingo Molnar]

* Alex Hung <alex.hung@canonical.com> wrote:

> Hi Ingo,
> 
> I agree with you but it can affect a lot of systems according to
> 163ea310b68bdde89b1ac633fbf8c0db290d3f86.
> 
> Do you think we should invert 163ea310 instead?

Reverting 163ea310 sounds like a bad idea, considering how old the commit is.

Ok, I guess we should do your patch after all. What a mess DMI based quirk gating 
is!

Thanks,

	Ingo