Received-SPF: pass (google.com: domain of linux-kernel+bounces-189034-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Fri, 24 May 2024 20:00:42 +0100
From: Conor Dooley <conor@kernel.org>
To: Sebastian Ene <sebastianene@google.com>
Cc: arnd@arndb.de, gregkh@linuxfoundation.org, will@kernel.org,
	maz@kernel.org, Rob Herring <robh+dt@kernel.org>,
	Dragan Cvetic <dragan.cvetic@xilinx.com>,
	Guenter Roeck <linux@roeck-us.net>, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org, kernel-team@android.com
Subject: Re: [PATCH 2/2] misc: Register a PPI for the vcpu stall detection
 virtual device
Message-ID: <20240524-joyous-fantastic-2747ab94e83d@spud>
References: <20240523160413.868830-1-sebastianene@google.com>
 <20240523160413.868830-3-sebastianene@google.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
	protocol="application/pgp-signature"; boundary="U4biA59HViDJpe97"
Content-Disposition: inline
In-Reply-To: <20240523160413.868830-3-sebastianene@google.com>


--U4biA59HViDJpe97
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Thu, May 23, 2024 at 04:04:13PM +0000, Sebastian Ene wrote:
> Request a PPI for each vCPU during probe which will be used by the host
> to communicate a stall detected event on the vCPU. When the host raises
> this interrupt from the virtual machine monitor, the guest is expected to
> handle the interrupt and panic.
>=20
> Signed-off-by: Sebastian Ene <sebastianene@google.com>
> ---
>  drivers/misc/vcpu_stall_detector.c | 41 ++++++++++++++++++++++++++++--
>  1 file changed, 39 insertions(+), 2 deletions(-)
>=20
> diff --git a/drivers/misc/vcpu_stall_detector.c b/drivers/misc/vcpu_stall=
_detector.c
> index e2015c87f03f..c580cd7fd225 100644
> --- a/drivers/misc/vcpu_stall_detector.c
> +++ b/drivers/misc/vcpu_stall_detector.c
> @@ -32,6 +32,7 @@
>  struct vcpu_stall_detect_config {
>  	u32 clock_freq_hz;
>  	u32 stall_timeout_sec;
> +	int ppi_irq;
> =20
>  	void __iomem *membase;
>  	struct platform_device *dev;
> @@ -77,6 +78,12 @@ vcpu_stall_detect_timer_fn(struct hrtimer *hrtimer)
>  	return HRTIMER_RESTART;
>  }
> =20
> +static irqreturn_t vcpu_stall_detector_irq(int irq, void *dev)
> +{
> +	panic("vCPU stall detector");
> +	return IRQ_HANDLED;
> +}
> +
>  static int start_stall_detector_cpu(unsigned int cpu)
>  {
>  	u32 ticks, ping_timeout_ms;
> @@ -132,7 +139,7 @@ static int stop_stall_detector_cpu(unsigned int cpu)
> =20
>  static int vcpu_stall_detect_probe(struct platform_device *pdev)
>  {
> -	int ret;
> +	int ret, irq, num_irqs;
>  	struct resource *r;
>  	void __iomem *membase;
>  	u32 clock_freq_hz =3D VCPU_STALL_DEFAULT_CLOCK_HZ;
> @@ -169,9 +176,32 @@ static int vcpu_stall_detect_probe(struct platform_d=
evice *pdev)
>  	vcpu_stall_config =3D (struct vcpu_stall_detect_config) {
>  		.membase		=3D membase,
>  		.clock_freq_hz		=3D clock_freq_hz,
> -		.stall_timeout_sec	=3D stall_timeout_sec
> +		.stall_timeout_sec	=3D stall_timeout_sec,
> +		.ppi_irq		=3D -1,
>  	};
> =20
> +	num_irqs =3D platform_irq_count(pdev);
> +	if (num_irqs < 0) {
> +		dev_err(&pdev->dev, "Failed to get irqs\n");

platform_irq_count() either returns a number or EPROBE_DEFER, I don't
think emitting an error on deferred probe is the correct thing to do
here?

> +		ret =3D num_irqs;
> +		goto err;
> +	} else if (num_irqs > 1) {
> +		dev_err(&pdev->dev, "Multipple irqs detected\n");

Typo. I don't really see why you're going to this level of complexity
though, why aren't you just doing a single get_irq_optional()?

> +		ret =3D -EINVAL;
> +		goto err;
> +	} else if (num_irqs =3D=3D 1) {
> +		irq =3D platform_get_irq(pdev, 0);
> +		if ((irq > 0) && irq_is_percpu_devid(irq)) {
> +			ret =3D request_percpu_irq(irq,
> +						 vcpu_stall_detector_irq,
> +						 "vcpu_stall_detector",
> +						 vcpu_stall_detectors);
> +			if (!ret)
> +				vcpu_stall_config.ppi_irq =3D irq;
> +
> +		}
> +	}
> +
>  	ret =3D cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
>  				"virt/vcpu_stall_detector:online",
>  				start_stall_detector_cpu,
> @@ -184,6 +214,9 @@ static int vcpu_stall_detect_probe(struct platform_de=
vice *pdev)
>  	vcpu_stall_config.hp_online =3D ret;
>  	return 0;
>  err:
> +	if (vcpu_stall_config.ppi_irq > 0)
> +		free_percpu_irq(vcpu_stall_config.ppi_irq,
> +				vcpu_stall_detectors);
>  	return ret;
>  }
> =20
> @@ -193,6 +226,10 @@ static void vcpu_stall_detect_remove(struct platform=
_device *pdev)
> =20
>  	cpuhp_remove_state(vcpu_stall_config.hp_online);
> =20
> +	if (vcpu_stall_config.ppi_irq > 0)
> +		free_percpu_irq(vcpu_stall_config.ppi_irq,
> +				vcpu_stall_detectors);
> +
>  	for_each_possible_cpu(cpu)
>  		stop_stall_detector_cpu(cpu);
>  }
> --=20
> 2.45.1.288.g0e0cd299f1-goog
>=20
>=20

--U4biA59HViDJpe97
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYIAB0WIQRh246EGq/8RLhDjO14tDGHoIJi0gUCZlDj2gAKCRB4tDGHoIJi
0pKCAQCh1w4ktNbvOBnvKlPyT8kVEaqv6qVAvawgu15jGC1guwEAoBW32iJ63XM/
ox+um5tZltvEvtfyuCM4wVclCNrmHwI=
=dzas
-----END PGP SIGNATURE-----

--U4biA59HViDJpe97--