linux-kernel - Re: [PATCH v8 2/2] misc: Add a mechanism to detect stalls on guest vCPUs

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>] [day] [month] [year] [list]

Date:   Tue, 28 Jun 2022 14:11:36 +0000
From:   Sebastian Ene <sebastianene@...gle.com>
To:     Guenter Roeck <linux@...ck-us.net>
Cc:     linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
        Wim Van Sebroeck <wim@...ux-watchdog.org>,
        Rob Herring <robh+dt@...nel.org>, qperret@...gle.com,
        maz@...nel.org, linux-watchdog@...r.kernel.org, will@...nel.org,
        vdonnefort@...gle.com
Subject: Re: [PATCH v8 2/2] misc: Add a mechanism to detect stalls on guest
 vCPUs

On Tue, Jun 28, 2022 at 07:00:05AM -0700, Guenter Roeck wrote:
> On 6/28/22 02:54, Sebastian Ene wrote:
> [ ... ]
> > > > +static struct platform_device *virt_dev;
> > > > +

Hi,

> > > 
> > > virt_dev is only used to call platform_set_drvdata() and platform_get_drvdata()
> > > on it. Why not just have a static variable named vm_stall_detect ?
> > > 
> > 
> > I think this should also work. I wanted to make use of the provided APIs
> > like platform_set/platform_get.
> > 
> 
> That doesn't mean such APIs should be used just to get used, though.
> 

I will remove these calls and keep it static.

> > > > +
> > > > +	vm_stall_detect = (struct vm_stall_detect_s __percpu *)
> > > > +		platform_get_drvdata(virt_dev);
> > > 
> > > platform_get_drvdata() returns void *; typecast to it is unnecessary.
> > > 
> > > 
> > 
> > I needed this typecast because the variable is per-cpu and some
> > compilers(eg. gcc for ARCH=h8300) complain if we don't specify this
> > hint.
> > 
> Hmm, interesting. I didn't know that. We live and learn.
> Though h8300 is gone now :-)

I had some Intel robot complaining about this in my previous series(v5) and
I fixed the warnings by adding these compiler hints.

> 
> Did you reply in private on purpose ?
> 

I misused my CC list but I will fix this in my reply. 

> Thanks,
> Guenter

Thanks for the response,
Seb