linux-kernel - Re: [PATCH v6 5/5] virt: vmgenid: add support for devicetree bindings

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

Message-ID: <CAHmME9qHY_1JGDMyvEGB7XDu2DKaYQcPE3UfSfb4aVZkgC8Q=g@mail.gmail.com>
Date: Wed, 17 Apr 2024 17:16:50 +0200
From: "Jason A. Donenfeld" <Jason@...c4.com>
To: sudanl@...zon.com, Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Cc: tytso@....edu, olivia@...enic.com, herbert@...dor.apana.org.au, 
	robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org, 
	linux-crypto@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org, graf@...zon.de, xmarcalx@...zon.co.uk, 
	dwmw@...zon.co.uk, Alexander Graf <graf@...zon.com>, Babis Chalios <bchalios@...zon.es>
Subject: Re: [PATCH v6 5/5] virt: vmgenid: add support for devicetree bindings

Hi Sudan & especially Krzysztof,

On Wed, Apr 17, 2024 at 12:43 PM Babis Chalios <bchalios@...zon.es> wrote:
>  struct vmgenid_state {
>         u8 *next_id;
>         u8 this_id[VMGENID_SIZE];
> +       int irq;

This is only ever used inside of one function. Why not just keep it on
the stack?

>  };
>
>  static void vmgenid_notify(struct device *device)
> @@ -43,6 +45,14 @@ vmgenid_acpi_handler(acpi_handle __always_unused handle,
>         vmgenid_notify(dev);
>  }
>
> +static irqreturn_t
> +vmgenid_of_irq_handler(int __always_unused irq, void *dev)
> +{
> +       vmgenid_notify(dev);
> +
> +       return IRQ_HANDLED;
> +}

Is there a reason the of code isn't conditional on CONFIG_OF? I'm not
super familiar with these drivers, but this seems like it would be a
thing to do, and then we could do `depends on OF || ACPI` in the
Kconfig.

After the whole Babis authorship debacle, I'm just fixing various
things up in my own tree and I'll send out a v+1. But Krzysztof, I
would really appreciate your review of this before I apply it to
random-next.

Jason