diff mbox

[RFC,1/2] Increased the size of requester_id field from MemTxAttrs

Message ID 1490276212-32008-2-git-send-email-diana.craciun@nxp.com
State New
Headers show

Commit Message

Diana Craciun March 23, 2017, 1:36 p.m. UTC
The PCI requester ID field is 16 bits. The requester_id field
from MemTxAttrs is used for MSIs to specify the device ID for
the platforms where this device ID is needed (e.g virt machine + GICv3
ITS). However, if more entities that uses MSIs in the system are used,
16 bit is no longer enough to represent the device ID. Increased the size
of this field to 24 bits in order to accomodate 256 entities.
Also the name requester_id does no longer reflect the content, so
the name was changed to stream_id.

Signed-off-by: Diana Craciun <diana.craciun@nxp.com>
---
 hw/intc/arm_gicv3_its_common.c         | 2 +-
 hw/intc/arm_gicv3_its_kvm.c            | 2 +-
 hw/pci/msi.c                           | 2 +-
 include/exec/memattrs.h                | 4 ++--
 include/hw/intc/arm_gicv3_its_common.h | 2 +-
 5 files changed, 6 insertions(+), 6 deletions(-)

Comments

Peter Maydell April 4, 2017, 12:27 p.m. UTC | #1
On 23 March 2017 at 13:36, Diana Craciun <diana.craciun@nxp.com> wrote:
> The PCI requester ID field is 16 bits. The requester_id field
> from MemTxAttrs is used for MSIs to specify the device ID for
> the platforms where this device ID is needed (e.g virt machine + GICv3
> ITS). However, if more entities that uses MSIs in the system are used,
> 16 bit is no longer enough to represent the device ID. Increased the size
> of this field to 24 bits in order to accomodate 256 entities.
> Also the name requester_id does no longer reflect the content, so
> the name was changed to stream_id.

This name change would be reverting the name change in
commit a05f686ff3, which says:
    MemTxAttrs.stream_id also renamed to requester_id in order to better
    reflect semantics of the field.

I don't have a strong opinion about the name myself, but flipping
backwards and forwards doesn't seem like a great plan :-)

thanks
-- PMM
Diana Craciun May 23, 2017, 10:45 a.m. UTC | #2
On 04/04/2017 03:27 PM, Peter Maydell wrote:
> On 23 March 2017 at 13:36, Diana Craciun <diana.craciun@nxp.com> wrote:
>> The PCI requester ID field is 16 bits. The requester_id field
>> from MemTxAttrs is used for MSIs to specify the device ID for
>> the platforms where this device ID is needed (e.g virt machine + GICv3
>> ITS). However, if more entities that uses MSIs in the system are used,
>> 16 bit is no longer enough to represent the device ID. Increased the size
>> of this field to 24 bits in order to accomodate 256 entities.
>> Also the name requester_id does no longer reflect the content, so
>> the name was changed to stream_id.
> This name change would be reverting the name change in
> commit a05f686ff3, which says:
>     MemTxAttrs.stream_id also renamed to requester_id in order to better
>     reflect semantics of the field.
>
> I don't have a strong opinion about the name myself, but flipping
> backwards and forwards doesn't seem like a great plan :-)

The reason I have changed the name is that the requester_id is a PCI
related terminology and this patch is extending the usage of that field
to other devices also. Naming it requester_id might be confusing.

I will shortly send a v2 for the patches, I didn't change the name in
the v2, but if there are strong opinions regarding this naming, I will
change it.

Thanks,

Diana
diff mbox

Patch

diff --git a/hw/intc/arm_gicv3_its_common.c b/hw/intc/arm_gicv3_its_common.c
index 9d67c5c..efdb1b3 100644
--- a/hw/intc/arm_gicv3_its_common.c
+++ b/hw/intc/arm_gicv3_its_common.c
@@ -66,7 +66,7 @@  static MemTxResult gicv3_its_trans_write(void *opaque, hwaddr offset,
     if (offset == 0x0040 && ((size == 2) || (size == 4))) {
         GICv3ITSState *s = ARM_GICV3_ITS_COMMON(opaque);
         GICv3ITSCommonClass *c = ARM_GICV3_ITS_COMMON_GET_CLASS(s);
-        int ret = c->send_msi(s, le64_to_cpu(value), attrs.requester_id);
+        int ret = c->send_msi(s, le64_to_cpu(value), attrs.stream_id);
 
         if (ret <= 0) {
             qemu_log_mask(LOG_GUEST_ERROR,
diff --git a/hw/intc/arm_gicv3_its_kvm.c b/hw/intc/arm_gicv3_its_kvm.c
index bd4f3aa..9006907 100644
--- a/hw/intc/arm_gicv3_its_kvm.c
+++ b/hw/intc/arm_gicv3_its_kvm.c
@@ -29,7 +29,7 @@ 
 #define TYPE_KVM_ARM_ITS "arm-its-kvm"
 #define KVM_ARM_ITS(obj) OBJECT_CHECK(GICv3ITSState, (obj), TYPE_KVM_ARM_ITS)
 
-static int kvm_its_send_msi(GICv3ITSState *s, uint32_t value, uint16_t devid)
+static int kvm_its_send_msi(GICv3ITSState *s, uint32_t value, uint32_t devid)
 {
     struct kvm_msi msi;
 
diff --git a/hw/pci/msi.c b/hw/pci/msi.c
index a87b227..7925851 100644
--- a/hw/pci/msi.c
+++ b/hw/pci/msi.c
@@ -336,7 +336,7 @@  void msi_send_message(PCIDevice *dev, MSIMessage msg)
 {
     MemTxAttrs attrs = {};
 
-    attrs.requester_id = pci_requester_id(dev);
+    attrs.stream_id = pci_requester_id(dev);
     address_space_stl_le(&dev->bus_master_as, msg.address, msg.data,
                          attrs, NULL);
 }
diff --git a/include/exec/memattrs.h b/include/exec/memattrs.h
index e601061..b13e1b8 100644
--- a/include/exec/memattrs.h
+++ b/include/exec/memattrs.h
@@ -35,8 +35,8 @@  typedef struct MemTxAttrs {
     unsigned int secure:1;
     /* Memory access is usermode (unprivileged) */
     unsigned int user:1;
-    /* Requester ID (for MSI for example) */
-    unsigned int requester_id:16;
+    /* Stream ID (for MSI for example) */
+    unsigned int stream_id:24;
 } MemTxAttrs;
 
 /* Bus masters which don't specify any attributes will get this,
diff --git a/include/hw/intc/arm_gicv3_its_common.h b/include/hw/intc/arm_gicv3_its_common.h
index 1ba1894..6140fc6 100644
--- a/include/hw/intc/arm_gicv3_its_common.h
+++ b/include/hw/intc/arm_gicv3_its_common.h
@@ -68,7 +68,7 @@  struct GICv3ITSCommonClass {
     SysBusDeviceClass parent_class;
     /*< public >*/
 
-    int (*send_msi)(GICv3ITSState *s, uint32_t data, uint16_t devid);
+    int (*send_msi)(GICv3ITSState *s, uint32_t data, uint32_t devid);
     void (*pre_save)(GICv3ITSState *s);
     void (*post_load)(GICv3ITSState *s);
 };