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] [day] [month] [year] [list]
Message-ID: <CAMpQs4JXKu4GDD79Mdkq9vASSDE_5SQsjsg4htfaZ5yGm3=k5g@mail.gmail.com>
Date: Tue, 11 Feb 2025 17:27:40 +0600
From: Binbin Zhou <zhoubb.aaron@...il.com>
To: Thomas Zimmermann <tzimmermann@...e.de>
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 Thomas:

Sorry for my late reply and thanks for your advice.

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)

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?

>
> 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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ