| 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
| ||
|
Message-ID: <e2296121-1f99-5bd2-2a80-a24b029bf4f3@intel.com>
Date: Thu, 1 Sep 2022 14:02:44 -0700
From: "Ceraolo Spurio, Daniele" <daniele.ceraolospurio@...el.com>
To: Tomas Winkler <tomas.winkler@...el.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
David Airlie <airlied@...ux.ie>,
Daniel Vetter <daniel@...ll.ch>
CC: Jani Nikula <jani.nikula@...ux.intel.com>,
Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>,
Rodrigo Vivi <rodrigo.vivi@...el.com>,
Tvrtko Ursulin <tvrtko.ursulin@...ux.intel.com>,
<intel-gfx@...ts.freedesktop.org>, <linux-kernel@...r.kernel.org>,
"Alexander Usyskin" <alexander.usyskin@...el.com>,
Vitaly Lubart <vitaly.lubart@...el.com>
Subject: Re: [PATCH v7 11/15] mei: gsc: setup gsc extended operational memory
On 8/6/2022 5:26 AM, Tomas Winkler wrote:
> 1. Retrieve extended operational memory physical pointers from the
> auxiliary device info.
> 2. Setup memory registers.
> 3. Notify firmware that the memory is ready by sending the memory
> ready command.
> 4. Disable PXP device if GSC is not in PXP mode.
>
> V7:
> 1. Add kdoc to mei_aux
> 2. rename pxp_isready() to pxp_is_ready()
>
> CC: Daniele Ceraolo Spurio <daniele.ceraolospurio@...el.com>
> Signed-off-by: Tomas Winkler <tomas.winkler@...el.com>
> Signed-off-by: Alexander Usyskin <alexander.usyskin@...el.com>
> ---
> drivers/misc/mei/bus-fixup.c | 70 ++++++++++++++++++++++++++++++++++-
> drivers/misc/mei/gsc-me.c | 16 ++++++++
> drivers/misc/mei/hw-me-regs.h | 7 ++++
> drivers/misc/mei/hw-me.c | 28 +++++++++++++-
> drivers/misc/mei/mei_dev.h | 10 +++++
> include/linux/mei_aux.h | 3 ++
> 6 files changed, 131 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/misc/mei/bus-fixup.c b/drivers/misc/mei/bus-fixup.c
> index 811c94ebf250..b350cc8d500c 100644
> --- a/drivers/misc/mei/bus-fixup.c
> +++ b/drivers/misc/mei/bus-fixup.c
> @@ -188,6 +188,19 @@ static int mei_fwver(struct mei_cl_device *cldev)
> return ret;
> }
>
> +static int mei_gfx_memory_ready(struct mei_cl_device *cldev)
> +{
> + struct mkhi_gfx_mem_ready req = {0};
> + unsigned int mode = MEI_CL_IO_TX_INTERNAL;
> +
> + req.hdr.group_id = MKHI_GROUP_ID_GFX;
> + req.hdr.command = MKHI_GFX_MEMORY_READY_CMD_REQ;
> + req.flags = MKHI_GFX_MEM_READY_PXP_ALLOWED;
> +
> + dev_dbg(&cldev->dev, "Sending memory ready command\n");
> + return __mei_cl_send(cldev->cl, (u8 *)&req, sizeof(req), 0, mode);
> +}
> +
> static void mei_mkhi_fix(struct mei_cl_device *cldev)
> {
> int ret;
> @@ -234,6 +247,39 @@ static void mei_gsc_mkhi_ver(struct mei_cl_device *cldev)
> dev_err(&cldev->dev, "FW version command failed %d\n", ret);
> mei_cldev_disable(cldev);
> }
> +
> +static void mei_gsc_mkhi_fix_ver(struct mei_cl_device *cldev)
> +{
> + int ret;
> +
> + /* No need to enable the client if nothing is needed from it */
> + if (!cldev->bus->fw_f_fw_ver_supported &&
> + cldev->bus->pxp_mode != MEI_DEV_PXP_INIT)
> + return;
> +
> + ret = mei_cldev_enable(cldev);
> + if (ret)
> + return;
> +
> + if (cldev->bus->pxp_mode == MEI_DEV_PXP_INIT) {
> + ret = mei_gfx_memory_ready(cldev);
> + if (ret < 0)
> + dev_err(&cldev->dev, "memory ready command failed %d\n", ret);
> + else
> + dev_dbg(&cldev->dev, "memory ready command sent\n");
> + /* we go to reset after that */
> + cldev->bus->pxp_mode = MEI_DEV_PXP_SETUP;
> + goto out;
> + }
> +
> + ret = mei_fwver(cldev);
> + if (ret < 0)
> + dev_err(&cldev->dev, "FW version command failed %d\n",
> + ret);
> +out:
> + mei_cldev_disable(cldev);
> +}
> +
> /**
> * mei_wd - wd client on the bus, change protocol version
> * as the API has changed.
> @@ -473,6 +519,26 @@ static void vt_support(struct mei_cl_device *cldev)
> cldev->do_match = 1;
> }
>
> +/**
> + * pxp_is_ready - enable bus client if pxp is ready
> + *
> + * @cldev: me clients device
> + */
> +static void pxp_is_ready(struct mei_cl_device *cldev)
> +{
> + struct mei_device *bus = cldev->bus;
> +
> + switch (bus->pxp_mode) {
> + case MEI_DEV_PXP_READY:
> + case MEI_DEV_PXP_DEFAULT:
> + cldev->do_match = 1;
Can you explain why you set do_match = 1 with MEI_DEV_PXP_DEFAULT ?
AFAIU MEI_DEV_PXP_DEFAULT means that we haven't even started to
initialize PXP, so it seems weird to match in that scenario.
Apart from this, the patch LGTM.
Daniele
> + break;
> + default:
> + cldev->do_match = 0;
> + break;
> + }
> +}
> +
> #define MEI_FIXUP(_uuid, _hook) { _uuid, _hook }
>
> static struct mei_fixup {
> @@ -486,10 +552,10 @@ static struct mei_fixup {
> MEI_FIXUP(MEI_UUID_WD, mei_wd),
> MEI_FIXUP(MEI_UUID_MKHIF_FIX, mei_mkhi_fix),
> MEI_FIXUP(MEI_UUID_IGSC_MKHI, mei_gsc_mkhi_ver),
> - MEI_FIXUP(MEI_UUID_IGSC_MKHI_FIX, mei_gsc_mkhi_ver),
> + MEI_FIXUP(MEI_UUID_IGSC_MKHI_FIX, mei_gsc_mkhi_fix_ver),
> MEI_FIXUP(MEI_UUID_HDCP, whitelist),
> MEI_FIXUP(MEI_UUID_ANY, vt_support),
> - MEI_FIXUP(MEI_UUID_PAVP, whitelist),
> + MEI_FIXUP(MEI_UUID_PAVP, pxp_is_ready),
> };
>
> /**
> diff --git a/drivers/misc/mei/gsc-me.c b/drivers/misc/mei/gsc-me.c
> index bfa6154b93e2..6b22726aed55 100644
> --- a/drivers/misc/mei/gsc-me.c
> +++ b/drivers/misc/mei/gsc-me.c
> @@ -32,6 +32,17 @@ static int mei_gsc_read_hfs(const struct mei_device *dev, int where, u32 *val)
> return 0;
> }
>
> +static void mei_gsc_set_ext_op_mem(const struct mei_me_hw *hw, struct resource *mem)
> +{
> + u32 low = lower_32_bits(mem->start);
> + u32 hi = upper_32_bits(mem->start);
> + u32 limit = (resource_size(mem) / SZ_4K) | GSC_EXT_OP_MEM_VALID;
> +
> + iowrite32(low, hw->mem_addr + H_GSC_EXT_OP_MEM_BASE_ADDR_LO_REG);
> + iowrite32(hi, hw->mem_addr + H_GSC_EXT_OP_MEM_BASE_ADDR_HI_REG);
> + iowrite32(limit, hw->mem_addr + H_GSC_EXT_OP_MEM_LIMIT_REG);
> +}
> +
> static int mei_gsc_probe(struct auxiliary_device *aux_dev,
> const struct auxiliary_device_id *aux_dev_id)
> {
> @@ -67,6 +78,11 @@ static int mei_gsc_probe(struct auxiliary_device *aux_dev,
>
> dev_set_drvdata(device, dev);
>
> + if (adev->ext_op_mem.start) {
> + mei_gsc_set_ext_op_mem(hw, &adev->ext_op_mem);
> + dev->pxp_mode = MEI_DEV_PXP_INIT;
> + }
> +
> /* use polling */
> if (mei_me_hw_use_polling(hw)) {
> mei_disable_interrupts(dev);
> diff --git a/drivers/misc/mei/hw-me-regs.h b/drivers/misc/mei/hw-me-regs.h
> index 15e8e2b322b1..8049f288f74c 100644
> --- a/drivers/misc/mei/hw-me-regs.h
> +++ b/drivers/misc/mei/hw-me-regs.h
> @@ -127,6 +127,8 @@
> # define PCI_CFG_HFS_3_FW_SKU_SPS 0x00000060
> #define PCI_CFG_HFS_4 0x64
> #define PCI_CFG_HFS_5 0x68
> +# define GSC_CFG_HFS_5_BOOT_TYPE_MSK 0x00000003
> +# define GSC_CFG_HFS_5_BOOT_TYPE_PXP 3
> #define PCI_CFG_HFS_6 0x6C
>
> /* MEI registers */
> @@ -143,6 +145,11 @@
> /* H_D0I3C - D0I3 Control */
> #define H_D0I3C 0x800
>
> +#define H_GSC_EXT_OP_MEM_BASE_ADDR_LO_REG 0x100
> +#define H_GSC_EXT_OP_MEM_BASE_ADDR_HI_REG 0x104
> +#define H_GSC_EXT_OP_MEM_LIMIT_REG 0x108
> +#define GSC_EXT_OP_MEM_VALID BIT(31)
> +
> /* register bits of H_CSR (Host Control Status register) */
> /* Host Circular Buffer Depth - maximum number of 32-bit entries in CB */
> #define H_CBD 0xFF000000
> diff --git a/drivers/misc/mei/hw-me.c b/drivers/misc/mei/hw-me.c
> index 6a8e8f3cea7e..c86d6018bc20 100644
> --- a/drivers/misc/mei/hw-me.c
> +++ b/drivers/misc/mei/hw-me.c
> @@ -433,6 +433,29 @@ static bool mei_me_hw_is_resetting(struct mei_device *dev)
> return (mecsr & ME_RST_HRA) == ME_RST_HRA;
> }
>
> +/**
> + * mei_gsc_pxp_check - check for gsc firmware entering pxp mode
> + *
> + * @dev: the device structure
> + */
> +static void mei_gsc_pxp_check(struct mei_device *dev)
> +{
> + struct mei_me_hw *hw = to_me_hw(dev);
> + u32 fwsts5 = 0;
> +
> + if (dev->pxp_mode == MEI_DEV_PXP_DEFAULT)
> + return;
> +
> + hw->read_fws(dev, PCI_CFG_HFS_5, &fwsts5);
> + trace_mei_pci_cfg_read(dev->dev, "PCI_CFG_HFS_5", PCI_CFG_HFS_5, fwsts5);
> + if ((fwsts5 & GSC_CFG_HFS_5_BOOT_TYPE_MSK) == GSC_CFG_HFS_5_BOOT_TYPE_PXP) {
> + dev_dbg(dev->dev, "pxp mode is ready 0x%08x\n", fwsts5);
> + dev->pxp_mode = MEI_DEV_PXP_READY;
> + } else {
> + dev_dbg(dev->dev, "pxp mode is not ready 0x%08x\n", fwsts5);
> + }
> +}
> +
> /**
> * mei_me_hw_ready_wait - wait until the me(hw) has turned ready
> * or timeout is reached
> @@ -452,6 +475,8 @@ static int mei_me_hw_ready_wait(struct mei_device *dev)
> return -ETIME;
> }
>
> + mei_gsc_pxp_check(dev);
> +
> mei_me_hw_reset_release(dev);
> dev->recvd_hw_ready = false;
> return 0;
> @@ -1270,7 +1295,8 @@ irqreturn_t mei_me_irq_thread_handler(int irq, void *dev_id)
>
> /* check if ME wants a reset */
> if (!mei_hw_is_ready(dev) && dev->dev_state != MEI_DEV_RESETTING) {
> - dev_warn(dev->dev, "FW not ready: resetting.\n");
> + dev_warn(dev->dev, "FW not ready: resetting: dev_state = %d pxp = %d\n",
> + dev->dev_state, dev->pxp_mode);
> if (dev->dev_state == MEI_DEV_POWERING_DOWN ||
> dev->dev_state == MEI_DEV_POWER_DOWN)
> mei_cl_all_disconnect(dev);
> diff --git a/drivers/misc/mei/mei_dev.h b/drivers/misc/mei/mei_dev.h
> index 16f59b3a45fc..7c508bca9a00 100644
> --- a/drivers/misc/mei/mei_dev.h
> +++ b/drivers/misc/mei/mei_dev.h
> @@ -62,6 +62,14 @@ enum mei_dev_state {
> MEI_DEV_POWER_UP
> };
>
> +/* MEI PXP mode state */
> +enum mei_dev_pxp_mode {
> + MEI_DEV_PXP_DEFAULT = 0,
> + MEI_DEV_PXP_INIT = 1,
> + MEI_DEV_PXP_SETUP = 2,
> + MEI_DEV_PXP_READY = 3,
> +};
> +
> const char *mei_dev_state_str(int state);
>
> enum mei_file_transaction_states {
> @@ -454,6 +462,7 @@ struct mei_dev_timeouts {
> * @reset_count : number of consecutive resets
> * @dev_state : device state
> * @hbm_state : state of host bus message protocol
> + * @pxp_mode : PXP device mode
> * @init_clients_timer : HBM init handshake timeout
> *
> * @pg_event : power gating event
> @@ -537,6 +546,7 @@ struct mei_device {
> unsigned long reset_count;
> enum mei_dev_state dev_state;
> enum mei_hbm_state hbm_state;
> + enum mei_dev_pxp_mode pxp_mode;
> u16 init_clients_timer;
>
> /*
> diff --git a/include/linux/mei_aux.h b/include/linux/mei_aux.h
> index 4894d8bf4159..506912ad363b 100644
> --- a/include/linux/mei_aux.h
> +++ b/include/linux/mei_aux.h
> @@ -12,6 +12,8 @@
> * @aux_dev: - auxiliary device object
> * @irq: interrupt driving the mei auxiliary device
> * @bar: mmio resource bar reserved to mei auxiliary device
> + * @ext_op_mem: resource for extend operational memory
> + * used in graphics PXP mode.
> * @slow_firmware: The device has slow underlying firmware.
> * Such firmware will require to use larger operation timeouts.
> */
> @@ -19,6 +21,7 @@ struct mei_aux_device {
> struct auxiliary_device aux_dev;
> int irq;
> struct resource bar;
> + struct resource ext_op_mem;
> bool slow_firmware;
> };
>
Powered by blists - more mailing lists