linux-kernel - Re: [RFC PATCH 5/9] apple-gmux: Use GMSP acpi method for interrupt clear

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230211104034.53e6f8ac@redecorated-mbp>
Date:   Sat, 11 Feb 2023 10:40:34 +1100
From:   Orlando Chamberlain <orlandoch.dev@...il.com>
To:     Hans de Goede <hdegoede@...hat.com>
Cc:     platform-driver-x86@...r.kernel.org, amd-gfx@...ts.freedesktop.org,
        linux-kernel@...r.kernel.org, alsa-devel@...a-project.org,
        Alex Deucher <alexander.deucher@....com>,
        Christian König <christian.koenig@....com>,
        "Pan, Xinhui" <Xinhui.Pan@....com>,
        David Airlie <airlied@...il.com>,
        Daniel Vetter <daniel@...ll.ch>,
        Mark Gross <markgross@...nel.org>,
        Jaroslav Kysela <perex@...ex.cz>,
        Takashi Iwai <tiwai@...e.com>,
        Hawking Zhang <Hawking.Zhang@....com>,
        Lijo Lazar <lijo.lazar@....com>,
        YiPeng Chai <YiPeng.Chai@....com>,
        Somalapuram Amaranath <Amaranath.Somalapuram@....com>,
        Mario Limonciello <mario.limonciello@....com>,
        Bokun Zhang <Bokun.Zhang@....com>,
        Jack Xiao <Jack.Xiao@....com>,
        Kai Vehmanen <kai.vehmanen@...ux.intel.com>,
        Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>,
        Rander Wang <rander.wang@...el.com>,
        Ranjani Sridharan <ranjani.sridharan@...ux.intel.com>,
        Amadeusz Sławiński 
        <amadeuszx.slawinski@...ux.intel.com>,
        Yong Zhi <yong.zhi@...el.com>, Evan Quan <evan.quan@....com>,
        Kerem Karabay <kekrby@...il.com>,
        Aditya Garg <gargaditya08@...e.com>,
        Aun-Ali Zaidi <admin@...eit.net>
Subject: Re: [RFC PATCH 5/9] apple-gmux: Use GMSP acpi method for interrupt
 clear

On Fri, 10 Feb 2023 20:43:58 +0100
Hans de Goede <hdegoede@...hat.com> wrote:

> Hi,
> 
> On 2/10/23 05:48, Orlando Chamberlain wrote:
> > This is needed for interrupts to be cleared correctly on MMIO based
> > gmux's. It is untested if this helps/hinders other gmux types, but I
> > have seen the GMSP method in the acpi tables of a MacBook with an
> > indexed gmux.
> > 
> > If this turns out to break support for older gmux's, this can
> > instead be only done on MMIO gmux's.
> > 
> > There is also a "GMLV" acpi method, and the "GMSP" method can be
> > called with 1 as its argument, but the purposes of these aren't
> > known and they don't seem to be needed.
> > 
> > Signed-off-by: Orlando Chamberlain <orlandoch.dev@...il.com>
> > ---
> >  drivers/platform/x86/apple-gmux.c | 26 +++++++++++++++++++++++++-
> >  1 file changed, 25 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/platform/x86/apple-gmux.c
> > b/drivers/platform/x86/apple-gmux.c index
> > 760434a527c1..c605f036ea0b 100644 ---
> > a/drivers/platform/x86/apple-gmux.c +++
> > b/drivers/platform/x86/apple-gmux.c @@ -494,8 +494,29 @@ static
> > const struct apple_gmux_config apple_gmux_index = {
> >   * MCP79, on all following generations it's GPIO pin 6 of the
> > Intel PCH.
> >   * The GPE merely signals that an interrupt occurred, the actual
> > type of event
> >   * is identified by reading a gmux register.
> > + *
> > + * On MMIO gmux's, we also need to call the acpi method GMSP to
> > properly clear
> > + * interrupts. TODO: Do other types need this? Does this break
> > other types? */
> >  
> > +static int gmux_call_acpi_gmsp(struct apple_gmux_data *gmux_data,
> > int arg) +{
> > +	acpi_status status = AE_OK;
> > +	union acpi_object arg0 = { ACPI_TYPE_INTEGER };
> > +	struct acpi_object_list arg_list = { 1, &arg0 };
> > +
> > +	arg0.integer.value = arg;
> > +
> > +	status = acpi_evaluate_object(gmux_data->dhandle, "GMSP",
> > &arg_list, NULL);
> > +	if (ACPI_FAILURE(status)) {
> > +		pr_err("GMSP call failed: %s\n",
> > +		       acpi_format_exception(status));
> > +		return -ENODEV;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  static inline void gmux_disable_interrupts(struct apple_gmux_data
> > *gmux_data) {
> >  	gmux_write8(gmux_data, GMUX_PORT_INTERRUPT_ENABLE,
> > @@ -519,7 +540,10 @@ static void gmux_clear_interrupts(struct
> > apple_gmux_data *gmux_data) 
> >  	/* to clear interrupts write back current status */
> >  	status = gmux_interrupt_get_status(gmux_data);
> > -	gmux_write8(gmux_data, GMUX_PORT_INTERRUPT_STATUS, status);
> > +	if (status) {
> > +		gmux_write8(gmux_data, GMUX_PORT_INTERRUPT_STATUS,
> > status);
> > +		gmux_call_acpi_gmsp(gmux_data, 0);  
> 
> Ugh no, please don't go around calling random ACPI methods from
> untested firmware revisions / device models.
> 
> ACPI code (even Apple's I have learned) tends to be full of bugs. If
> we did not need to call GMSP before then please lets keep not calling
> it on the older models. Just because it is there does not mean that
> calling it is useful, it might even be harmful.

I'll make it only use this ACPI method on MMIO gmux's in v2 then.

> 
> Regards,
> 
> Hans
> 
> 
> 
> 
> 
> 
> > +	}
> >  }
> >  
> >  static void gmux_notify_handler(acpi_handle device, u32 value,
> > void *context)  
>