Re: [RFC PATCH v4 7/8] bus/cdx: add cdx-MSI domain with gic-its domain as parent

From: Thomas Gleixner <tglx@linutronix.de>
To: Nipun Gupta <nipun.gupta@amd.com>,
	robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org,
	gregkh@linuxfoundation.org, rafael@kernel.org,
	eric.auger@redhat.com, alex.williamson@redhat.com,
	cohuck@redhat.com, puneet.gupta@amd.com,
	song.bao.hua@hisilicon.com, mchehab+huawei@kernel.org,
	maz@kernel.org, f.fainelli@gmail.com, jeffrey.l.hugo@gmail.com,
	saravanak@google.com, Michael.Srba@seznam.cz, mani@kernel.org,
	yishaih@nvidia.com, jgg@ziepe.ca, jgg@nvidia.com,
	robin.murphy@arm.com, will@kernel.org, joro@8bytes.org,
	masahiroy@kernel.org, ndesaulniers@google.com,
	linux-arm-kernel@lists.infradead.org,
	linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org, kvm@vger.kernel.org
Cc: okaya@kernel.org, harpreet.anand@amd.com, nikhil.agarwal@amd.com,
	michal.simek@amd.com, aleksandar.radovanovic@amd.com,
	git@amd.com, Nipun Gupta <nipun.gupta@amd.com>
Subject: Re: [RFC PATCH v4 7/8] bus/cdx: add cdx-MSI domain with gic-its domain as parent
Date: Thu, 17 Nov 2022 20:10:46 +0100	[thread overview]
Message-ID: <87r0y1l7m1.ffs@tglx> (raw)
In-Reply-To: <20221014044049.2557085-8-nipun.gupta@amd.com>

On Fri, Oct 14 2022 at 10:10, Nipun Gupta wrote:
> Since CDX devices are not linked to of node they need a
> separate MSI domain for handling device ID to be provided to
> the GIC ITS domain.
>
> This also introduces APIs to alloc and free IRQs for CDX domain.

This is not really much of an explanation.

> +#include "cdx.h"
> +
> +#define REQ_ID_SHIFT	10
> +
> +/*
> + * Convert an msi_desc to a globaly unique identifier (per-device
> + * reqid + msi_desc position in the msi_list).
> + */
> +static irq_hw_number_t cdx_domain_calc_hwirq(struct cdx_device *dev,
> +					     struct msi_desc *desc)
> +{
> +	return (dev->req_id << REQ_ID_SHIFT) | desc->msi_index;
> +}
> +
> +static void cdx_msi_set_desc(msi_alloc_info_t *arg,
> +			     struct msi_desc *desc)
> +{
> +	arg->desc = desc;
> +	arg->hwirq = cdx_domain_calc_hwirq(to_cdx_device(desc->dev), desc);
> +}
> +
> +static void cdx_msi_write_msg(struct irq_data *irq_data,
> +			      struct msi_msg *msg)
> +{
> +	struct msi_desc *msi_desc = irq_data_get_msi_desc(irq_data);
> +	struct cdx_device *cdx_dev = to_cdx_device(msi_desc->dev);
> +	struct cdx_controller_t *cdx = cdx_dev->cdx;
> +	uint64_t addr;
> +	int ret;
> +
> +	addr = ((uint64_t)(msi_desc->msg.address_hi) << 32) |
> +	       msi_desc->msg.address_lo;
> +
> +	ret = cdx->ops.write_msi(cdx, cdx_dev->bus_num, cdx_dev->dev_num,
> +				 msi_desc->msi_index, msi_desc->msg.data,
> +				 addr);
> +	if (ret)
> +		dev_err(&cdx_dev->dev, "Write MSI failed to CDX controller\n");
> +}

So this is yet another variant of platform-MSI which is about to be
kicked from the planet.

Please check:

     https://lore.kernel.org/r/20221111131813.914374272@linutronix.de
     https://lore.kernel.org/r/20221111133158.196269823@linutronix.de

and the ARM/GIC conversion in:

     https://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git/log/?h=devmsi-arm

The latter is WIP, but functional.

I'm sure you replace your yet another magic MSI domain which only
provides a device specific MSI write function with the new
infrastructure trivially.

Thanks,

        tglx