Re: [Qemu-devel] [RFC][PATCH 08/45] Introduce MSIMessage structure

2011-10-18 Thread Jan Kiszka
On 2011-10-17 15:01, Michael S. Tsirkin wrote:
 On Mon, Oct 17, 2011 at 02:09:46PM +0200, Jan Kiszka wrote:
 On 2011-10-17 14:04, Michael S. Tsirkin wrote:
 On Mon, Oct 17, 2011 at 01:51:00PM +0200, Jan Kiszka wrote:
 On 2011-10-17 13:46, Michael S. Tsirkin wrote:
 On Mon, Oct 17, 2011 at 11:27:42AM +0200, Jan Kiszka wrote:
 Will be used for generating and distributing MSI messages, both in
 emulation mode and under KVM.

 Signed-off-by: Jan Kiszka jan.kis...@siemens.com

 I would add

 uint64_t msix_get_address(dev, vector)
 uint64_t msix_get_data(dev, vector)

 and same for msi.

 this would minimise the changes while still making it
 possible to avoid code duplication in kvm.

 I'm introducing msi[x]_message_from_vector for that purpose later on. Or
 what do you mean?

 Jan

 It does not look like everyone actually wants the structure,
 users seem to put it on stack and then immediately
 unwrap it to get at the address/data.
 So two accessorts get_data + get_address instead of one, will
 remove the need to rework all code to use the structure.

 The idea of this patch is to start handling MSI messages as a single
 blob. There should be no need to ask a device for parts of that blobs
 this way.
 
 There should be no need to look at the message at all.
 devices really only care about vector numbers.
 So we are left with msix.c msi.c and kvm as the only users.
 kvm has a cache of messages so it needs a struct of these,
 msix/msi don't.

MSIMessages is primarily designed for path from the device to the
interrupt controller. And there are multiple stops, already in the path
after this patch set. Interrupt remapping, e.g., would add another stop.

 
 If you see use cases in this series, though, let me know.

 Jan
 
 Yes, I see them. msix_notify is one example. msi_notify is another.
 
 E.g. msi_notify would IMO look nicer as:
 stl_le_phys(msi_get_address(dev, vector), msi_get_data(dev, vector));

This line does not exist anymore at the end of this series. See
pc_msi_deliver.

Jan



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [RFC][PATCH 08/45] Introduce MSIMessage structure

2011-10-17 Thread Michael S. Tsirkin
On Mon, Oct 17, 2011 at 11:27:42AM +0200, Jan Kiszka wrote:
 Will be used for generating and distributing MSI messages, both in
 emulation mode and under KVM.
 
 Signed-off-by: Jan Kiszka jan.kis...@siemens.com

I would add

uint64_t msix_get_address(dev, vector)
uint64_t msix_get_data(dev, vector)

and same for msi.

this would minimise the changes while still making it
possible to avoid code duplication in kvm.

 ---
  hw/msi.h  |5 +
  qemu-common.h |1 +
  2 files changed, 6 insertions(+), 0 deletions(-)
 
 diff --git a/hw/msi.h b/hw/msi.h
 index e5e821f..22e3932 100644
 --- a/hw/msi.h
 +++ b/hw/msi.h
 @@ -24,6 +24,11 @@
  #include qemu-common.h
  #include pci.h
  
 +struct MSIMessage {
 +uint64_t address;
 +uint32_t data;
 +};
 +
  extern bool msi_supported;
  
  bool msi_enabled(const PCIDevice *dev);
 diff --git a/qemu-common.h b/qemu-common.h
 index 5e87bdf..d3901bd 100644
 --- a/qemu-common.h
 +++ b/qemu-common.h
 @@ -15,6 +15,7 @@ typedef struct QEMUTimer QEMUTimer;
  typedef struct QEMUFile QEMUFile;
  typedef struct QEMUBH QEMUBH;
  typedef struct DeviceState DeviceState;
 +typedef struct MSIMessage MSIMessage;
  
  struct Monitor;
  typedef struct Monitor Monitor;
 -- 
 1.7.3.4



Re: [Qemu-devel] [RFC][PATCH 08/45] Introduce MSIMessage structure

2011-10-17 Thread Jan Kiszka
On 2011-10-17 13:46, Michael S. Tsirkin wrote:
 On Mon, Oct 17, 2011 at 11:27:42AM +0200, Jan Kiszka wrote:
 Will be used for generating and distributing MSI messages, both in
 emulation mode and under KVM.

 Signed-off-by: Jan Kiszka jan.kis...@siemens.com
 
 I would add
 
 uint64_t msix_get_address(dev, vector)
 uint64_t msix_get_data(dev, vector)
 
 and same for msi.
 
 this would minimise the changes while still making it
 possible to avoid code duplication in kvm.

I'm introducing msi[x]_message_from_vector for that purpose later on. Or
what do you mean?

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [RFC][PATCH 08/45] Introduce MSIMessage structure

2011-10-17 Thread Michael S. Tsirkin
On Mon, Oct 17, 2011 at 01:51:00PM +0200, Jan Kiszka wrote:
 On 2011-10-17 13:46, Michael S. Tsirkin wrote:
  On Mon, Oct 17, 2011 at 11:27:42AM +0200, Jan Kiszka wrote:
  Will be used for generating and distributing MSI messages, both in
  emulation mode and under KVM.
 
  Signed-off-by: Jan Kiszka jan.kis...@siemens.com
  
  I would add
  
  uint64_t msix_get_address(dev, vector)
  uint64_t msix_get_data(dev, vector)
  
  and same for msi.
  
  this would minimise the changes while still making it
  possible to avoid code duplication in kvm.
 
 I'm introducing msi[x]_message_from_vector for that purpose later on. Or
 what do you mean?
 
 Jan

It does not look like everyone actually wants the structure,
users seem to put it on stack and then immediately
unwrap it to get at the address/data.
So two accessorts get_data + get_address instead of one, will
remove the need to rework all code to use the structure.

 -- 
 Siemens AG, Corporate Technology, CT T DE IT 1
 Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [RFC][PATCH 08/45] Introduce MSIMessage structure

2011-10-17 Thread Jan Kiszka
On 2011-10-17 14:04, Michael S. Tsirkin wrote:
 On Mon, Oct 17, 2011 at 01:51:00PM +0200, Jan Kiszka wrote:
 On 2011-10-17 13:46, Michael S. Tsirkin wrote:
 On Mon, Oct 17, 2011 at 11:27:42AM +0200, Jan Kiszka wrote:
 Will be used for generating and distributing MSI messages, both in
 emulation mode and under KVM.

 Signed-off-by: Jan Kiszka jan.kis...@siemens.com

 I would add

 uint64_t msix_get_address(dev, vector)
 uint64_t msix_get_data(dev, vector)

 and same for msi.

 this would minimise the changes while still making it
 possible to avoid code duplication in kvm.

 I'm introducing msi[x]_message_from_vector for that purpose later on. Or
 what do you mean?

 Jan
 
 It does not look like everyone actually wants the structure,
 users seem to put it on stack and then immediately
 unwrap it to get at the address/data.
 So two accessorts get_data + get_address instead of one, will
 remove the need to rework all code to use the structure.

The idea of this patch is to start handling MSI messages as a single
blob. There should be no need to ask a device for parts of that blobs
this way. If you see use cases in this series, though, let me know.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [RFC][PATCH 08/45] Introduce MSIMessage structure

2011-10-17 Thread Michael S. Tsirkin
On Mon, Oct 17, 2011 at 02:09:46PM +0200, Jan Kiszka wrote:
 On 2011-10-17 14:04, Michael S. Tsirkin wrote:
  On Mon, Oct 17, 2011 at 01:51:00PM +0200, Jan Kiszka wrote:
  On 2011-10-17 13:46, Michael S. Tsirkin wrote:
  On Mon, Oct 17, 2011 at 11:27:42AM +0200, Jan Kiszka wrote:
  Will be used for generating and distributing MSI messages, both in
  emulation mode and under KVM.
 
  Signed-off-by: Jan Kiszka jan.kis...@siemens.com
 
  I would add
 
  uint64_t msix_get_address(dev, vector)
  uint64_t msix_get_data(dev, vector)
 
  and same for msi.
 
  this would minimise the changes while still making it
  possible to avoid code duplication in kvm.
 
  I'm introducing msi[x]_message_from_vector for that purpose later on. Or
  what do you mean?
 
  Jan
  
  It does not look like everyone actually wants the structure,
  users seem to put it on stack and then immediately
  unwrap it to get at the address/data.
  So two accessorts get_data + get_address instead of one, will
  remove the need to rework all code to use the structure.
 
 The idea of this patch is to start handling MSI messages as a single
 blob. There should be no need to ask a device for parts of that blobs
 this way.

There should be no need to look at the message at all.
devices really only care about vector numbers.
So we are left with msix.c msi.c and kvm as the only users.
kvm has a cache of messages so it needs a struct of these,
msix/msi don't.

 If you see use cases in this series, though, let me know.
 
 Jan

Yes, I see them. msix_notify is one example. msi_notify is another.

E.g. msi_notify would IMO look nicer as:
stl_le_phys(msi_get_address(dev, vector), msi_get_data(dev, vector));



 -- 
 Siemens AG, Corporate Technology, CT T DE IT 1
 Corporate Competence Center Embedded Linux