| 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: <67afd398-f039-4937-a859-cc4d5b0a53d5@suse.de>
Date: Mon, 31 Mar 2025 09:53:27 +0200
From: Thomas Zimmermann <tzimmermann@...e.de>
To: Binbin Zhou <zhoubb.aaron@...il.com>
Cc: Binbin Zhou <zhoubinbin@...ngson.cn>, Huacai Chen
<chenhuacai@...ngson.cn>, Lee Jones <lee@...nel.org>,
Corey Minyard <minyard@....org>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>, David Airlie <airlied@...il.com>,
Simona Vetter <simona@...ll.ch>, Huacai Chen <chenhuacai@...nel.org>,
linux-kernel@...r.kernel.org, openipmi-developer@...ts.sourceforge.net,
dri-devel@...ts.freedesktop.org, Xuerui Wang <kernel@...0n.name>,
loongarch@...ts.linux.dev, Chong Qiao <qiaochong@...ngson.cn>
Subject: Re: [PATCH v1 4/4] drm/ls2kbmc: Add Loongson-2K BMC reset function
support
Hi
Am 11.02.25 um 12:27 schrieb Binbin Zhou:
> Hi Thomas:
>
> Sorry for my late reply and thanks for your advice.
Apologies, I missed your email.
>
> On Wed, Jan 15, 2025 at 2:57 PM Thomas Zimmermann <tzimmermann@...e.de> wrote:
>> Hi
>>
>>
>> Am 30.12.24 um 10:31 schrieb Binbin Zhou:
>>> Since the display is a sub-function of the Loongson-2K BMC, when the
>>> BMC reset, the entire BMC PCIe is disconnected, including the display
>>> which is interrupted.
>> To me, that's a strong indicator to set up the entire thing from scratch.
>>
>>> Not only do you need to save/restore the relevant PCIe registers
>>> throughout the reset process, but you also need to re-push the display
>>> to the monitor at the end.
>>>
>>> Co-developed-by: Chong Qiao <qiaochong@...ngson.cn>
>>> Signed-off-by: Chong Qiao <qiaochong@...ngson.cn>
>>> Signed-off-by: Binbin Zhou <zhoubinbin@...ngson.cn>
>>> ---
>>> drivers/gpu/drm/tiny/ls2kbmc.c | 284 ++++++++++++++++++++++++++++++++-
>>> 1 file changed, 283 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/tiny/ls2kbmc.c b/drivers/gpu/drm/tiny/ls2kbmc.c
>>> index 909d6c687193..4b440f20cb4d 100644
>>> --- a/drivers/gpu/drm/tiny/ls2kbmc.c
>>> +++ b/drivers/gpu/drm/tiny/ls2kbmc.c
>> Move all the reset detection into the BMC core driver. When you see a
>> reset, remove the display's platform device via
>> platform_device_unregister(). This will release the device and the DRM
>> driver on top. We do this for simpledrm/efifb/etc. Hence user-space code
>> is able to deal with it. Then set up a new platform device when the new
>> framebuffer is available. Your DRM driver will bind to it and user-space
>> code will continue with the new DRM device.
> I tried to refactor the bmc restart part according to your scheme. I'm
> not quite sure if the experimental results are exactly right.
>
> Key part:
>
> New solution:
> 1. platform_device_unregister(simpledrm)
> 2. wait and detect if the BMC reboot is complete;
> 3. platform_device_register_resndata(simpledrm)
You are doing this in the MFD driver, right?
Apologies again, I find it very confusing which component does what.
If the MFD driver created the initial graphics device, it should also
remove it on BMC resets. And then establish a new graphics device when
the reset has been complete. It shouldn't matter if the driver is
simpledrm or a custom driver for your hardware.
>
> Original solution:
> 1. wait and detect if the BMC reboot is complete;
> 2. ls2kbmc_push_drm_mode() pushes display data such as crtc.
>
> When the BMC reboot is completed, the display in the new solution will
> lose all the information of the previous desktop and redisplay the
> system login interface, while the original solution will keep the
> desktop information.
>
> Is this normal for our new solution, or is there something wrong with
> my implementation?
The desktop should mostly remain as-is. Maybe flicker, but not go back
to the login screen. Is there any error in the log files of the desktop
or compositor?
Best regards
Thomas
>
>> Best regards
>> Thomas
>>
>>> @@ -8,10 +8,12 @@
>>> */
>>>
>>> #include <linux/aperture.h>
>>> +#include <linux/delay.h>
>>> #include <linux/minmax.h>
>>> #include <linux/pci.h>
>>> #include <linux/platform_data/simplefb.h>
>>> #include <linux/platform_device.h>
>>> +#include <linux/stop_machine.h>
>>>
>>> #include <drm/drm_atomic.h>
>>> #include <drm/drm_atomic_state_helper.h>
>>> @@ -32,9 +34,27 @@
>>> #include <drm/drm_panic.h>
>>> #include <drm/drm_probe_helper.h>
>>>
>>> +#define BMC_RESET_DELAY (60 * HZ)
>>> +#define BMC_RESET_WAIT 10000
>>> +
>>> +static const u32 index[] = { 0x4, 0x10, 0x14, 0x18, 0x1c, 0x20, 0x24,
>>> + 0x30, 0x3c, 0x54, 0x58, 0x78, 0x7c, 0x80 };
>>> +static const u32 cindex[] = { 0x4, 0x10, 0x3c };
>>> +
>>> +struct ls2kbmc_pci_data {
>>> + u32 d80c;
>>> + u32 d71c;
>>> + u32 data[14];
>>> + u32 cdata[3];
>>> +};
>>> +
>>> struct ls2kbmc_pdata {
>>> struct pci_dev *pdev;
>>> + struct drm_device *ddev;
>>> + struct work_struct bmc_work;
>>> + unsigned long reset_time;
>>> struct simplefb_platform_data pd;
>>> + struct ls2kbmc_pci_data pci_data;
>>> };
>>>
>>> /*
>>> @@ -583,6 +603,265 @@ static struct ls2kbmc_device *ls2kbmc_device_create(struct drm_driver *drv,
>>> return sdev;
>>> }
>>>
>>> +static bool ls2kbmc_bar0_addr_is_set(struct pci_dev *ppdev)
>>> +{
>>> + u32 addr;
>>> +
>>> + pci_read_config_dword(ppdev, PCI_BASE_ADDRESS_0, &addr);
>>> + addr &= PCI_BASE_ADDRESS_MEM_MASK;
>>> +
>>> + return addr ? true : false;
>>> +}
>>> +
>>> +static void ls2kbmc_save_pci_data(struct ls2kbmc_pdata *priv)
>>> +{
>>> + struct pci_dev *pdev = priv->pdev;
>>> + struct pci_dev *parent = pdev->bus->self;
>>> + int i;
>>> +
>>> + for (i = 0; i < ARRAY_SIZE(index); i++)
>>> + pci_read_config_dword(parent, index[i], &priv->pci_data.data[i]);
>>> +
>>> + for (i = 0; i < ARRAY_SIZE(cindex); i++)
>>> + pci_read_config_dword(pdev, cindex[i], &priv->pci_data.cdata[i]);
>>> +
>>> + pci_read_config_dword(parent, 0x80c, &priv->pci_data.d80c);
>>> + priv->pci_data.d80c = (priv->pci_data.d80c & ~(3 << 17)) | (1 << 17);
>>> +
>>> + pci_read_config_dword(parent, 0x71c, &priv->pci_data.d71c);
>>> + priv->pci_data.d71c |= 1 << 26;
>>> +}
>>> +
>>> +static bool ls2kbmc_check_pcie_connected(struct pci_dev *parent, struct drm_device *dev)
>>> +{
>>> + void __iomem *mmio;
>>> + int sts, timeout = 10000;
>>> +
>>> + mmio = pci_iomap(parent, 0, 0x100);
>>> + if (!mmio)
>>> + return false;
>>> +
>>> + writel(readl(mmio) | 0x8, mmio);
>>> + while (timeout) {
>>> + sts = readl(mmio + 0xc);
>>> + if ((sts & 0x11) == 0x11)
>>> + break;
>>> + mdelay(1);
>>> + timeout--;
>>> + }
>>> +
>>> + pci_iounmap(parent, mmio);
>>> +
>>> + if (!timeout) {
>>> + drm_err(dev, "pcie train failed status=0x%x\n", sts);
>>> + return false;
>>> + }
>>> +
>>> + return true;
>>> +}
>>> +
>>> +static int ls2kbmc_recove_pci_data(void *data)
>>> +{
>>> + struct ls2kbmc_pdata *priv = data;
>>> + struct pci_dev *pdev = priv->pdev;
>>> + struct drm_device *dev = priv->ddev;
>>> + struct pci_dev *parent = pdev->bus->self;
>>> + u32 i, timeout, retry = 0;
>>> + bool ready;
>>> +
>>> + pci_write_config_dword(parent, PCI_BASE_ADDRESS_2, 0);
>>> + pci_write_config_dword(parent, PCI_BASE_ADDRESS_3, 0);
>>> + pci_write_config_dword(parent, PCI_BASE_ADDRESS_4, 0);
>>> +
>>> + timeout = 10000;
>>> + while (timeout) {
>>> + ready = ls2kbmc_bar0_addr_is_set(parent);
>>> + if (!ready)
>>> + break;
>>> + mdelay(1);
>>> + timeout--;
>>> + };
>>> +
>>> + if (!timeout)
>>> + drm_warn(dev, "bar not clear 0\n");
>>> +
>>> +retrain:
>>> + for (i = 0; i < ARRAY_SIZE(index); i++)
>>> + pci_write_config_dword(parent, index[i], priv->pci_data.data[i]);
>>> +
>>> + pci_write_config_dword(parent, 0x80c, priv->pci_data.d80c);
>>> + pci_write_config_dword(parent, 0x71c, priv->pci_data.d71c);
>>> +
>>> + /* Check if the pcie is connected */
>>> + ready = ls2kbmc_check_pcie_connected(parent, dev);
>>> + if (!ready)
>>> + return ready;
>>> +
>>> + for (i = 0; i < ARRAY_SIZE(cindex); i++)
>>> + pci_write_config_dword(pdev, cindex[i], priv->pci_data.cdata[i]);
>>> +
>>> + drm_info(dev, "pcie recovered done\n");
>>> +
>>> + if (!retry) {
>>> + /*wait u-boot ddr config */
>>> + mdelay(BMC_RESET_WAIT);
>>> + ready = ls2kbmc_bar0_addr_is_set(parent);
>>> + if (!ready) {
>>> + retry = 1;
>>> + goto retrain;
>>> + }
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int ls2kbmc_push_drm_mode(struct drm_device *dev)
>>> +{
>>> + struct ls2kbmc_device *sdev = ls2kbmc_device_of_dev(dev);
>>> + struct drm_crtc *crtc = &sdev->crtc;
>>> + struct drm_plane *plane = crtc->primary;
>>> + struct drm_connector *connector = &sdev->connector;
>>> + struct drm_framebuffer *fb = NULL;
>>> + struct drm_mode_set set;
>>> + struct drm_modeset_acquire_ctx ctx;
>>> + int ret;
>>> +
>>> + mutex_lock(&dev->mode_config.mutex);
>>> + connector->funcs->fill_modes(connector, 4096, 4096);
>>> + mutex_unlock(&dev->mode_config.mutex);
>>> +
>>> + DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx,
>>> + DRM_MODESET_ACQUIRE_INTERRUPTIBLE, ret);
>>> +
>>> + if (plane->state)
>>> + fb = plane->state->fb;
>>> + else
>>> + fb = plane->fb;
>>> +
>>> + if (!fb) {
>>> + drm_dbg(dev, "CRTC doesn't have current FB\n");
>>> + ret = -EINVAL;
>>> + goto out;
>>> + }
>>> +
>>> + drm_framebuffer_get(fb);
>>> +
>>> + set.crtc = crtc;
>>> + set.x = 0;
>>> + set.y = 0;
>>> + set.mode = &sdev->mode;
>>> + set.connectors = &connector;
>>> + set.num_connectors = 1;
>>> + set.fb = fb;
>>> +
>>> + ret = crtc->funcs->set_config(&set, &ctx);
>>> +
>>> +out:
>>> + DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static void ls2kbmc_events_fn(struct work_struct *work)
>>> +{
>>> + struct ls2kbmc_pdata *priv = container_of(work, struct ls2kbmc_pdata, bmc_work);
>>> +
>>> + /*
>>> + * The pcie is lost when the BMC resets,
>>> + * at which point access to the pcie from other CPUs
>>> + * is suspended to prevent a crash.
>>> + */
>>> + stop_machine(ls2kbmc_recove_pci_data, priv, NULL);
>>> +
>>> + drm_info(priv->ddev, "redraw console\n");
>>> +
>>> + /* We need to re-push the display due to previous pcie loss. */
>>> + ls2kbmc_push_drm_mode(priv->ddev);
>>> +}
>>> +
>>> +static irqreturn_t ls2kbmc_interrupt(int irq, void *arg)
>>> +{
>>> + struct ls2kbmc_pdata *priv = arg;
>>> +
>>> + if (system_state != SYSTEM_RUNNING)
>>> + return IRQ_HANDLED;
>>> +
>>> + /* skip interrupt in BMC_RESET_DELAY */
>>> + if (time_after(jiffies, priv->reset_time + BMC_RESET_DELAY))
>>> + schedule_work(&priv->bmc_work);
>>> +
>>> + priv->reset_time = jiffies;
>>> +
>>> + return IRQ_HANDLED;
>>> +}
>>> +
>>> +#define BMC_RESET_GPIO 14
>>> +#define LOONGSON_GPIO_REG_BASE 0x1fe00500
>>> +#define LOONGSON_GPIO_REG_SIZE 0x18
>>> +#define LOONGSON_GPIO_OEN 0x0
>>> +#define LOONGSON_GPIO_FUNC 0x4
>>> +#define LOONGSON_GPIO_INTPOL 0x10
>>> +#define LOONGSON_GPIO_INTEN 0x14
>>> +
>>> +/* The gpio interrupt is a watchdog interrupt that is triggered when the BMC resets. */
>>> +static int ls2kbmc_gpio_reset_handler(struct ls2kbmc_pdata *priv)
>>> +{
>>> + int irq, ret = 0;
>>> + int gsi = 16 + (BMC_RESET_GPIO & 7);
>>> + void __iomem *gpio_base;
>>> +
>>> + /* Since Loongson-3A hardware does not support GPIO interrupt cascade,
>>> + * chip->gpio_to_irq() cannot be implemented,
>>> + * here acpi_register_gsi() is used to get gpio irq.
>>> + */
>>> + irq = acpi_register_gsi(NULL, gsi, ACPI_EDGE_SENSITIVE, ACPI_ACTIVE_LOW);
>>> + if (irq < 0)
>>> + return irq;
>>> +
>>> + gpio_base = ioremap(LOONGSON_GPIO_REG_BASE, LOONGSON_GPIO_REG_SIZE);
>>> + if (!gpio_base) {
>>> + acpi_unregister_gsi(gsi);
>>> + return PTR_ERR(gpio_base);
>>> + }
>>> +
>>> + writel(readl(gpio_base + LOONGSON_GPIO_OEN) | BIT(BMC_RESET_GPIO),
>>> + gpio_base + LOONGSON_GPIO_OEN);
>>> + writel(readl(gpio_base + LOONGSON_GPIO_FUNC) & ~BIT(BMC_RESET_GPIO),
>>> + gpio_base + LOONGSON_GPIO_FUNC);
>>> + writel(readl(gpio_base + LOONGSON_GPIO_INTPOL) & ~BIT(BMC_RESET_GPIO),
>>> + gpio_base + LOONGSON_GPIO_INTPOL);
>>> + writel(readl(gpio_base + LOONGSON_GPIO_INTEN) | BIT(BMC_RESET_GPIO),
>>> + gpio_base + LOONGSON_GPIO_INTEN);
>>> +
>>> + ret = request_irq(irq, ls2kbmc_interrupt, IRQF_SHARED | IRQF_TRIGGER_FALLING,
>>> + "ls2kbmc gpio", priv);
>>> +
>>> + acpi_unregister_gsi(gsi);
>>> + iounmap(gpio_base);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static int ls2kbmc_pdata_initial(struct platform_device *pdev, struct ls2kbmc_pdata *priv)
>>> +{
>>> + int ret;
>>> +
>>> + priv->pdev = *(struct pci_dev **)dev_get_platdata(&pdev->dev);
>>> +
>>> + ls2kbmc_save_pci_data(priv);
>>> +
>>> + INIT_WORK(&priv->bmc_work, ls2kbmc_events_fn);
>>> +
>>> + ret = request_irq(priv->pdev->irq, ls2kbmc_interrupt,
>>> + IRQF_SHARED | IRQF_TRIGGER_RISING, "ls2kbmc pcie", priv);
>>> + if (ret) {
>>> + pr_err("request_irq(%d) failed\n", priv->pdev->irq);
>>> + return ret;
>>> + }
>>> +
>>> + return ls2kbmc_gpio_reset_handler(priv);
>>> +}
>>> +
>>> /*
>>> * Platform driver
>>> */
>>> @@ -598,12 +877,15 @@ static int ls2kbmc_probe(struct platform_device *pdev)
>>> if (IS_ERR(priv))
>>> return -ENOMEM;
>>>
>>> - priv->pdev = *(struct pci_dev **)dev_get_platdata(&pdev->dev);
>>> + ret = ls2kbmc_pdata_initial(pdev, priv);
>>> + if (ret)
>>> + return ret;
>>>
>>> sdev = ls2kbmc_device_create(&ls2kbmc_driver, pdev, priv);
>>> if (IS_ERR(sdev))
>>> return PTR_ERR(sdev);
>>> dev = &sdev->dev;
>>> + priv->ddev = &sdev->dev;
>>>
>>> ret = drm_dev_register(dev, 0);
>>> if (ret)
>> --
>> --
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Frankenstrasse 146, 90461 Nuernberg, Germany
>> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
>> HRB 36809 (AG Nuernberg)
>>
>
> --
> Thanks.
> Binbin
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
Powered by blists - more mailing lists