[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [RFC 16/16] gic: vgic: align frequently accessed data by cache line size





On 28/11/2018 21:32, Andrii Anisov wrote:
From: Andrii Anisov <andrii_anisov@xxxxxxxx>

Cache line size assumed 64 byte as for H3. Align the `struct
pending_irq` and allocate lrs shadow aligned to cache line size.

The arch_vcpu is already aligned to a cache size. So how does it improve the performance by making lr also aligned to a cache line?

I can believe that pending_irq would be nice to have it aligned to a cache line (or half of it). But you need to explain the trade-off as you are going to use more memory.


Signed-off-by: Andrii Anisov <andrii_anisov@xxxxxxxx>
---
  xen/arch/arm/domain.c        | 4 ++++
  xen/arch/arm/vgic.c          | 9 +++++++++
  xen/include/asm-arm/config.h | 2 +-
  xen/include/asm-arm/gic.h    | 2 +-
  xen/include/asm-arm/vgic.h   | 2 +-
  5 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 8e886b7..8bcb667 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -558,6 +558,10 @@ int arch_vcpu_create(struct vcpu *v)
      v->arch.saved_context.sp = (register_t)v->arch.cpu_info;
      v->arch.saved_context.pc = (register_t)continue_new_vcpu;
+ v->arch.gic.v2.lr = xzalloc_bytes(sizeof(uint32_t) * gic_number_lines());
+    if ( v->arch.gic.v2.lr == NULL )
+        return -ENOMEM;
+
      /* Idle VCPUs don't need the rest of this setup */
      if ( is_idle_vcpu(v) )
          return rc;
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 7691310..bedfb99 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -166,6 +166,15 @@ int domain_vgic_init(struct domain *d, unsigned int 
nr_spis)
d->arch.vgic.pending_irqs =
          xzalloc_array(struct pending_irq, d->arch.vgic.nr_spis);
+
+    if ( sizeof(struct pending_irq) != dcache_line_bytes )
+    {
+        printk ("sizeof(struct pending_irq) = %lu  is not equal to cacheline"
+                "size %lu. Is it expected?\n", sizeof(struct pending_irq),
+                dcache_line_bytes);
+        WARN();
+    }
What could happen if pending_irq is not aligned to your cacheline? If your cacheline is smaller than pending_irq, then you are going to use multiple cache line. So nothing to worry.

If your cacheline is bigger, then it is a potential concern as you would share the cacheline with something else. This may be an issue, but I would like to see numbers first.

+
      if ( d->arch.vgic.pending_irqs == NULL )
          return -ENOMEM;
diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
index bc89e84..4f3669f 100644
--- a/xen/include/asm-arm/config.h
+++ b/xen/include/asm-arm/config.h
@@ -28,7 +28,7 @@
#define CONFIG_ARM 1 -#define CONFIG_ARM_L1_CACHE_SHIFT 7 /* XXX */
+#define CONFIG_ARM_L1_CACHE_SHIFT 6 /* XXX */

That's not acceptable, this value is based on the biggest cache line used by Arm processors. If you want to use 64 bytes cache line, then you either make sure the structure fits in 64-bytes by adding padding.

#define CONFIG_SMP 1 diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index add2566..fe44d3a 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -186,7 +186,7 @@ struct gic_v2 {
      uint32_t hcr;
      uint32_t vmcr;
      uint32_t apr;
-    uint32_t lr[64];
+    uint32_t *lr;
      uint64_t lr_update_mask;
  };
diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
index a27a1a9..d4ec96f 100644
--- a/xen/include/asm-arm/vgic.h
+++ b/xen/include/asm-arm/vgic.h
@@ -99,7 +99,7 @@ struct pending_irq
       * TODO: when implementing irq migration, taking only the current
       * vgic lock is not going to be enough. */
      struct list_head lr_queue;
-};
+}__cacheline_aligned;
#define NR_INTERRUPT_PER_RANK 32
  #define INTERRUPT_RANK_MASK (NR_INTERRUPT_PER_RANK - 1)


Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.