All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andre Przywara <andre.przywara@arm.com>
To: Eric Auger <eric.auger@linaro.org>,
	"eric.auger@st.com" <eric.auger@st.com>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	Marc Zyngier <Marc.Zyngier@arm.com>,
	"christoffer.dall@linaro.org" <christoffer.dall@linaro.org>,
	"kvmarm@lists.cs.columbia.edu" <kvmarm@lists.cs.columbia.edu>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>
Cc: "pbonzini@redhat.com" <pbonzini@redhat.com>,
	"p.fedin@samsung.com" <p.fedin@samsung.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 2/7] KVM: kvm_host: add kvm_extended_msi
Date: Thu, 2 Jul 2015 18:03:26 +0100	[thread overview]
Message-ID: <55956EDE.6030300@arm.com> (raw)
In-Reply-To: <1435592237-17924-3-git-send-email-eric.auger@linaro.org>

Hi Eric,

just played a bit with the code and I could make things easier by the
following change:

On 29/06/15 16:37, Eric Auger wrote:
> Add a new kvm_extended_msi struct to store the additional device ID
> specific to ARM. kvm_kernel_irq_routing_entry union now encompasses
> this new struct.
> 
> Signed-off-by: Eric Auger <eric.auger@linaro.org>
> 
> ---
> 
> RFC -> PATCH:
> - reword the commit message after change in first patch (uapi)
> ---
>  include/linux/kvm_host.h | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index ad45054..e1c1c0d 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -304,6 +304,13 @@ struct kvm_s390_adapter_int {
>  	u32 adapter_id;
>  };
>  
> +struct kvm_extended_msi {
> +	u32     address_lo;     /* low 32 bits of msi message address */
> +	u32     address_hi;     /* high 32 bits of msi message address */
> +	u32     data;           /* 16 bits of msi message data */
> +	u32	devid;		/* out-of-band device ID */
> +};
> +

I got rid of this structure at all, instead using:

 @@ -317,6 +324,7 @@ struct kvm_kernel_irq_routing_entry {
  		} irqchip;
-		struct msi_msg msi;
+		struct {
+			struct msi_msg msi;
+			u32 devid;
+		};
  		struct kvm_s390_adapter_int adapter;
  	};
  	struct hlist_node link;
  };

This re-uses the existing MSI fields in struct msi_msg, so all the extra
code you added in the next patches to set address and data could be
skipped. If needed we can add a flags field here as well to avoid that
extra type.
That simplified a lot for me.

Cheers,
André

WARNING: multiple messages have this Message-ID (diff)
From: andre.przywara@arm.com (Andre Przywara)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 2/7] KVM: kvm_host: add kvm_extended_msi
Date: Thu, 2 Jul 2015 18:03:26 +0100	[thread overview]
Message-ID: <55956EDE.6030300@arm.com> (raw)
In-Reply-To: <1435592237-17924-3-git-send-email-eric.auger@linaro.org>

Hi Eric,

just played a bit with the code and I could make things easier by the
following change:

On 29/06/15 16:37, Eric Auger wrote:
> Add a new kvm_extended_msi struct to store the additional device ID
> specific to ARM. kvm_kernel_irq_routing_entry union now encompasses
> this new struct.
> 
> Signed-off-by: Eric Auger <eric.auger@linaro.org>
> 
> ---
> 
> RFC -> PATCH:
> - reword the commit message after change in first patch (uapi)
> ---
>  include/linux/kvm_host.h | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index ad45054..e1c1c0d 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -304,6 +304,13 @@ struct kvm_s390_adapter_int {
>  	u32 adapter_id;
>  };
>  
> +struct kvm_extended_msi {
> +	u32     address_lo;     /* low 32 bits of msi message address */
> +	u32     address_hi;     /* high 32 bits of msi message address */
> +	u32     data;           /* 16 bits of msi message data */
> +	u32	devid;		/* out-of-band device ID */
> +};
> +

I got rid of this structure at all, instead using:

 @@ -317,6 +324,7 @@ struct kvm_kernel_irq_routing_entry {
  		} irqchip;
-		struct msi_msg msi;
+		struct {
+			struct msi_msg msi;
+			u32 devid;
+		};
  		struct kvm_s390_adapter_int adapter;
  	};
  	struct hlist_node link;
  };

This re-uses the existing MSI fields in struct msi_msg, so all the extra
code you added in the next patches to set address and data could be
skipped. If needed we can add a flags field here as well to avoid that
extra type.
That simplified a lot for me.

Cheers,
Andr?

WARNING: multiple messages have this Message-ID (diff)
From: Andre Przywara <andre.przywara@arm.com>
To: Eric Auger <eric.auger@linaro.org>,
	"eric.auger@st.com" <eric.auger@st.com>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	Marc Zyngier <Marc.Zyngier@arm.com>,
	"christoffer.dall@linaro.org" <christoffer.dall@linaro.org>,
	"kvmarm@lists.cs.columbia.edu" <kvmarm@lists.cs.columbia.edu>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"p.fedin@samsung.com" <p.fedin@samsung.com>,
	"pbonzini@redhat.com" <pbonzini@redhat.com>
Subject: Re: [PATCH 2/7] KVM: kvm_host: add kvm_extended_msi
Date: Thu, 2 Jul 2015 18:03:26 +0100	[thread overview]
Message-ID: <55956EDE.6030300@arm.com> (raw)
In-Reply-To: <1435592237-17924-3-git-send-email-eric.auger@linaro.org>

Hi Eric,

just played a bit with the code and I could make things easier by the
following change:

On 29/06/15 16:37, Eric Auger wrote:
> Add a new kvm_extended_msi struct to store the additional device ID
> specific to ARM. kvm_kernel_irq_routing_entry union now encompasses
> this new struct.
> 
> Signed-off-by: Eric Auger <eric.auger@linaro.org>
> 
> ---
> 
> RFC -> PATCH:
> - reword the commit message after change in first patch (uapi)
> ---
>  include/linux/kvm_host.h | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index ad45054..e1c1c0d 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -304,6 +304,13 @@ struct kvm_s390_adapter_int {
>  	u32 adapter_id;
>  };
>  
> +struct kvm_extended_msi {
> +	u32     address_lo;     /* low 32 bits of msi message address */
> +	u32     address_hi;     /* high 32 bits of msi message address */
> +	u32     data;           /* 16 bits of msi message data */
> +	u32	devid;		/* out-of-band device ID */
> +};
> +

I got rid of this structure at all, instead using:

 @@ -317,6 +324,7 @@ struct kvm_kernel_irq_routing_entry {
  		} irqchip;
-		struct msi_msg msi;
+		struct {
+			struct msi_msg msi;
+			u32 devid;
+		};
  		struct kvm_s390_adapter_int adapter;
  	};
  	struct hlist_node link;
  };

This re-uses the existing MSI fields in struct msi_msg, so all the extra
code you added in the next patches to set address and data could be
skipped. If needed we can add a flags field here as well to avoid that
extra type.
That simplified a lot for me.

Cheers,
André

  reply	other threads:[~2015-07-02 16:52 UTC|newest]

Thread overview: 147+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-29 15:37 [PATCH 0/7] KVM: arm/arm64: gsi routing support Eric Auger
2015-06-29 15:37 ` Eric Auger
2015-06-29 15:37 ` Eric Auger
2015-06-29 15:37 ` [PATCH 1/7] KVM: api: add kvm_irq_routing_extended_msi Eric Auger
2015-06-29 15:37   ` Eric Auger
2015-06-29 15:37   ` Eric Auger
2015-07-02  7:26   ` Pavel Fedin
2015-07-02  7:26     ` Pavel Fedin
2015-07-02  7:26     ` Pavel Fedin
2015-07-02  8:41     ` Pavel Fedin
2015-07-02  8:41       ` Pavel Fedin
2015-07-02 14:50       ` Eric Auger
2015-07-02 14:50         ` Eric Auger
2015-07-02 14:50         ` Eric Auger
2015-07-02 14:49     ` Eric Auger
2015-07-02 14:49       ` Eric Auger
2015-07-02 14:49       ` Eric Auger
2015-07-02 15:14       ` Andre Przywara
2015-07-02 15:14         ` Andre Przywara
2015-07-02 15:14         ` Andre Przywara
2015-07-02 15:22         ` Eric Auger
2015-07-02 15:22           ` Eric Auger
2015-07-02 15:39           ` Pavel Fedin
2015-07-02 15:39             ` Pavel Fedin
2015-07-02 15:39             ` Pavel Fedin
2015-07-02 15:41             ` Eric Auger
2015-07-02 15:41               ` Eric Auger
2015-07-03 15:29               ` Pavel Fedin
2015-07-03 15:29                 ` Pavel Fedin
2015-07-03 15:29                 ` Pavel Fedin
2015-07-03 15:42                 ` Eric Auger
2015-07-03 15:42                   ` Eric Auger
2015-07-03 15:42                   ` Eric Auger
2015-07-03  9:05     ` Andre Przywara
2015-07-03  9:05       ` Andre Przywara
2015-07-03  9:05       ` Andre Przywara
2015-07-03 15:53       ` Andre Przywara
2015-07-03 15:53         ` Andre Przywara
2015-07-03 15:53         ` Andre Przywara
2015-07-06  6:42       ` Pavel Fedin
2015-07-06  6:42         ` Pavel Fedin
2015-07-06  6:42         ` Pavel Fedin
2015-07-06  8:30         ` Andre Przywara
2015-07-06  8:30           ` Andre Przywara
2015-07-06  8:30           ` Andre Przywara
2015-07-06  9:30           ` Christoffer Dall
2015-07-06  9:30             ` Christoffer Dall
2015-07-06  9:30             ` Christoffer Dall
2015-07-06 10:05             ` Andre Przywara
2015-07-06 10:05               ` Andre Przywara
2015-07-06 10:05               ` Andre Przywara
2015-07-06 10:37               ` Christoffer Dall
2015-07-06 10:37                 ` Christoffer Dall
2015-07-06 10:37                 ` Christoffer Dall
2015-07-06 11:07                 ` Paolo Bonzini
2015-07-06 11:07                   ` Paolo Bonzini
2015-07-06 11:23                   ` Andre Przywara
2015-07-06 11:23                     ` Andre Przywara
2015-07-06 11:23                     ` Andre Przywara
2015-07-06 11:51                     ` Paolo Bonzini
2015-07-06 11:51                       ` Paolo Bonzini
2015-07-06 13:32                       ` Pavel Fedin
2015-07-06 13:32                         ` Pavel Fedin
2015-07-06 15:01                         ` Eric Auger
2015-07-06 15:01                           ` Eric Auger
2015-07-06 15:01                           ` Eric Auger
2015-07-06 15:52                           ` Andre Przywara
2015-07-06 15:52                             ` Andre Przywara
2015-07-06 15:52                             ` Andre Przywara
2015-07-06 17:02                             ` Eric Auger
2015-07-06 17:02                               ` Eric Auger
2015-07-07  7:23                             ` Pavel Fedin
2015-07-07  7:23                               ` Pavel Fedin
2015-07-07  7:23                               ` Pavel Fedin
2015-07-07  7:43                               ` Eric Auger
2015-07-07  7:43                                 ` Eric Auger
2015-07-07  7:43                                 ` Eric Auger
2015-07-06 15:37                         ` Andre Przywara
2015-07-06 15:37                           ` Andre Przywara
2015-07-06 15:37                           ` Andre Przywara
2015-07-06 15:54                           ` Paolo Bonzini
2015-07-06 15:54                             ` Paolo Bonzini
2015-07-06 15:54                             ` Paolo Bonzini
2015-07-06 16:08                             ` Andre Przywara
2015-07-06 16:08                               ` Andre Przywara
2015-07-06 16:08                               ` Andre Przywara
2015-07-07  7:16                           ` Pavel Fedin
2015-07-07  7:16                             ` Pavel Fedin
2015-07-07  7:16                             ` Pavel Fedin
2015-07-07 10:02                             ` Andre Przywara
2015-07-07 10:02                               ` Andre Przywara
2015-07-07 10:02                               ` Andre Przywara
2015-07-07 10:57                               ` Pavel Fedin
2015-07-07 10:57                                 ` Pavel Fedin
2015-07-07 10:57                                 ` Pavel Fedin
2015-07-06 12:08                     ` Christoffer Dall
2015-07-06 12:08                       ` Christoffer Dall
2015-07-06 13:33                       ` Pavel Fedin
2015-07-06 13:33                         ` Pavel Fedin
2015-07-06 13:33                         ` Pavel Fedin
2015-07-06 15:09                       ` Andre Przywara
2015-07-06 15:09                         ` Andre Przywara
2015-07-06 15:09                         ` Andre Przywara
2015-06-29 15:37 ` [PATCH 2/7] KVM: kvm_host: add kvm_extended_msi Eric Auger
2015-06-29 15:37   ` Eric Auger
2015-06-29 15:37   ` Eric Auger
2015-07-02 17:03   ` Andre Przywara [this message]
2015-07-02 17:03     ` Andre Przywara
2015-07-02 17:03     ` Andre Przywara
2015-06-29 15:37 ` [PATCH 3/7] KVM: irqchip: convey devid to kvm_set_msi Eric Auger
2015-06-29 15:37   ` Eric Auger
2015-06-29 15:37   ` Eric Auger
2015-06-29 15:37 ` [PATCH 4/7] KVM: arm/arm64: enable irqchip routing Eric Auger
2015-06-29 15:37   ` Eric Auger
2015-06-29 15:37   ` Eric Auger
2015-06-30 13:39   ` Andre Przywara
2015-06-30 13:39     ` Andre Przywara
2015-06-30 13:39     ` Andre Przywara
2015-06-30 14:02     ` Eric Auger
2015-06-30 14:02       ` Eric Auger
2015-06-30 14:02       ` Eric Auger
2015-06-29 15:37 ` [PATCH 5/7] KVM: arm/arm64: build a default routing table Eric Auger
2015-06-29 15:37   ` Eric Auger
2015-06-29 15:37   ` Eric Auger
2015-06-29 15:37 ` [PATCH 6/7] KVM: arm/arm64: enable MSI routing Eric Auger
2015-06-29 15:37   ` Eric Auger
2015-06-29 15:37   ` Eric Auger
2015-06-29 15:37 ` [PATCH 7/7] KVM: arm: implement kvm_set_msi by gsi direct mapping Eric Auger
2015-06-29 15:37   ` Eric Auger
2015-06-29 15:37   ` Eric Auger
2015-07-02  7:53   ` Pavel Fedin
2015-07-02  7:53     ` Pavel Fedin
2015-07-02 15:02     ` Eric Auger
2015-07-02 15:02       ` Eric Auger
2015-07-02 15:02       ` Eric Auger
2015-07-02 15:37       ` Pavel Fedin
2015-07-02 15:37         ` Pavel Fedin
2015-07-02 15:37         ` Pavel Fedin
2015-07-02 17:10   ` Andre Przywara
2015-07-02 17:10     ` Andre Przywara
2015-07-02 17:10     ` Andre Przywara
2015-07-03  5:34     ` Eric Auger
2015-07-03  5:34       ` Eric Auger
2015-07-03  5:34       ` Eric Auger
2015-07-05 19:40 ` [PATCH 0/7] KVM: arm/arm64: gsi routing support Christoffer Dall
2015-07-05 19:40   ` Christoffer Dall
2015-07-05 19:40   ` Christoffer Dall

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=55956EDE.6030300@arm.com \
    --to=andre.przywara@arm.com \
    --cc=Marc.Zyngier@arm.com \
    --cc=christoffer.dall@linaro.org \
    --cc=eric.auger@linaro.org \
    --cc=eric.auger@st.com \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.cs.columbia.edu \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=p.fedin@samsung.com \
    --cc=pbonzini@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.