linux-kernel - Re: [PATCH] cdx: add MSI support for CDX bus

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]

Date:   Tue, 09 May 2023 10:01:34 +0200
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Nipun Gupta <nipun.gupta@....com>, gregkh@...uxfoundation.org,
        maz@...nel.org, jgg@...pe.ca, linux-kernel@...r.kernel.org
Cc:     git@....com, harpreet.anand@....com,
        pieter.jansen-van-vuuren@....com, nikhil.agarwal@....com,
        michal.simek@....com, abhijit.gangurde@....com,
        pablo.cascon@....com, Nipun Gupta <nipun.gupta@....com>
Subject: Re: [PATCH] cdx: add MSI support for CDX bus

On Mon, May 08 2023 at 19:39, Nipun Gupta wrote:
> Add CDX-MSI domain with gic-its domain as parent, to support MSI
> for CDX devices. CDX devices allocate MSIs from the CDX domain.
> Also, introduce APIs to alloc and free IRQs for CDX domain.

This lacks any information why this needs to have a separate irq domain
and what this is about. Changelogs need to be self explanatory and
providing a link to some RFC series which might have more information
does not cut it.

Just for the record. I complained about the useless changelog in that
RFC series already.

> Signed-off-by: Nipun Gupta <nipun.gupta@....com>
> Signed-off-by: Nikhil Agarwal <nikhil.agarwal@....com>
> Signed-off-by: Abhijit Gangurde <abhijit.gangurde@....com>

This Signed-off-by chain is broken. If this is intended to denote
co-authorship, then please follow:

  https://www.kernel.org/doc/html/latest/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by

> +static struct irq_chip cdx_msi_irq_chip = {
> +	.name			= "CDX-MSI",
> +	.irq_mask		= irq_chip_mask_parent,
> +	.irq_unmask		= irq_chip_unmask_parent,
> +	.irq_eoi		= irq_chip_eoi_parent,
> +	.irq_set_affinity	= msi_domain_set_affinity,
> +	.irq_write_msi_msg	= cdx_msi_write_msg
> +};

The only real CDX specific functionality here is a CDX specific
irq_write_msi_msg() callback, right?

And I gave you a pointer how this should be handled, but instead of
helping this effort along you go off and implement it differently just
because. Sigh!

Thanks,

        tglx