From mboxrd@z Thu Jan  1 00:00:00 1970
From: Will Deacon <will.deacon@arm.com>
Subject: Re: [PATCH 2/4] kvmtool: prepare for instantiating different IRQ
 chip devices
Date: Fri, 20 Jun 2014 18:55:19 +0100
Message-ID: <20140620175519.GF30656@arm.com>
References: <1403181879-29908-1-git-send-email-andre.przywara@arm.com>
 <1403181879-29908-3-git-send-email-andre.przywara@arm.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: "kvmarm@lists.cs.columbia.edu" <kvmarm@lists.cs.columbia.edu>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	Marc Zyngier <Marc.Zyngier@arm.com>
To: Andre Przywara <andre.przywara@arm.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from cam-admin0.cambridge.arm.com ([217.140.96.50]:50336 "EHLO
	cam-admin0.cambridge.arm.com" rhost-flags-OK-OK-OK-OK)
	by vger.kernel.org with ESMTP id S1752229AbaFTRzV (ORCPT
	<rfc822;kvm@vger.kernel.org>); Fri, 20 Jun 2014 13:55:21 -0400
Content-Disposition: inline
In-Reply-To: <1403181879-29908-3-git-send-email-andre.przywara@arm.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

Hi Andre,

On Thu, Jun 19, 2014 at 01:44:37PM +0100, Andre Przywara wrote:
> Extend the vGIC handling code to deal with different IRQ chip devices
> instead of hard-coding the GICv2 in.

Minor comment, but there are a few stray pr_infos in here which I don't
think are especially useful.

> +		if (!ioctl(gic_fd, KVM_HAS_DEVICE_ATTR, &offset_attr)) {
> +			err = ioctl(gic_fd, KVM_GET_DEVICE_ATTR, &offset_attr);
> +			if (err)
> +				return err;
> +		}
> +
> +		cpu_if_addr += offset;
> +
> +		err = ioctl(gic_fd, KVM_SET_DEVICE_ATTR, &cpu_if_attr);
>  		if (err)
>  			return err;
> +		pr_info("creating GICv2 KVM device");

One here...

> +	switch (type) {
> +	case KVM_DEV_TYPE_ARM_VGIC_V2:
> +		compatible = "arm,cortex-a15-gic";
> +		pr_info("creating FDT for a GICv2");

... and here.

> diff --git a/tools/kvm/virtio/mmio.c b/tools/kvm/virtio/mmio.c
> index afae6a7..35dc113 100644
> --- a/tools/kvm/virtio/mmio.c
> +++ b/tools/kvm/virtio/mmio.c
> @@ -293,7 +293,7 @@ int virtio_mmio_init(struct kvm *kvm, void *dev, struct virtio_device *vdev,
>  	 *
>  	 * virtio_mmio.devices=0x200@0xd2000000:5,0x200@0xd2000200:6
>  	 */
> -	pr_info("virtio-mmio.devices=0x%x@0x%x:%d\n", VIRTIO_MMIO_IO_SIZE, vmmio->addr, line);
> +	pr_info("virtio-mmio.devices=0x%x@0x%x:%d", VIRTIO_MMIO_IO_SIZE, vmmio->addr, line);

Huh?

Anyway, the general idea looks ok to me.

Will