linux-kernel - Re: [PATCH] drm/komeda: Adds error event print functionality

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]

Date:   Thu, 18 Jul 2019 11:23:50 -0400
From:   Sean Paul <sean@...rly.run>
To:     Liviu Dudau <Liviu.Dudau@....com>
Cc:     "Lowry Li (Arm Technology China)" <Lowry.Li@....com>,
        "james qian wang (Arm Technology China)" <james.qian.wang@....com>,
        "maarten.lankhorst@...ux.intel.com" 
        <maarten.lankhorst@...ux.intel.com>,
        "seanpaul@...omium.org" <seanpaul@...omium.org>,
        "airlied@...ux.ie" <airlied@...ux.ie>,
        Brian Starkey <Brian.Starkey@....com>,
        "Julien Yin (Arm Technology China)" <Julien.Yin@....com>,
        "Jonathan Chai (Arm Technology China)" <Jonathan.Chai@....com>,
        Ayan Halder <Ayan.Halder@....com>,
        "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        nd <nd@....com>
Subject: Re: [PATCH] drm/komeda: Adds error event print functionality

On Thu, Jul 18, 2019 at 02:17:37PM +0100, Liviu Dudau wrote:
> On Thu, Jun 27, 2019 at 04:10:36AM +0100, Lowry Li (Arm Technology China) wrote:

/snip

> > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> > index 647bce5..1462bac 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> > @@ -47,6 +47,8 @@ static irqreturn_t komeda_kms_irq_handler(int irq, void *data)
> >  	memset(&evts, 0, sizeof(evts));
> >  	status = mdev->funcs->irq_handler(mdev, &evts);
> >  
> > +	komeda_print_events(&evts);
> 
> Calling this function from the IRQ handler is a bad idea. We should use debugfs
> if you really want to have a trace of the events, but I personally don't see
> value in having this functionality in the kernel at all. You can expose the
> value of the evts->global and evts->pipes[] as integers and decode that in
> userspace or as a debugfs entry.

Alternatively, consider using kernel trace events. They allow you to selectively
turn on/off certain events and also allow you to customize which data is
recorded and how it's formatted. Seems like a good fit from the quick scan I've
done.

Sean

> 
> Best regards,
> Liviu
> 
> > +
> >  	/* Notify the crtc to handle the events */
> >  	for (i = 0; i < kms->n_crtcs; i++)
> >  		komeda_crtc_handle_event(&kms->crtcs[i], &evts);
> > -- 
> > 1.9.1
> > 
> 
> -- 
> ====================
> | I would like to |
> | fix the world,  |
> | but they're not |
> | giving me the   |
>  \ source code!  /
>   ---------------
>     ¯\_(ツ)_/¯

-- 
Sean Paul, Software Engineer, Google / Chromium OS