Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1232865 > unrolled thread

[PATCH v1 0/8] xen/arm: vgic: Support 32-bit access for 64-bit register

Started byJulien Grall <julien.grall@citrix.com>
First post2015-09-25 17:00 +0200
Last post2015-09-25 17:00 +0200
Articles 3 — 1 participant

Back to article view | Back to linux.kernel

Contents

  [PATCH v1 0/8] xen/arm: vgic: Support 32-bit access for 64-bit register Julien Grall <julien.grall@citrix.com> - 2015-09-25 17:00 +0200
    [PATCH v1 3/8] xen/arm: Support sign-extension for every read access Julien Grall <julien.grall@citrix.com> - 2015-09-25 17:00 +0200
    Re: [PATCH v1 0/8] xen/arm: vgic: Support 32-bit access for 64-bit  register Julien Grall <julien.grall@citrix.com> - 2015-09-25 17:00 +0200

#1232865 — [PATCH v1 0/8] xen/arm: vgic: Support 32-bit access for 64-bit register

FromJulien Grall <julien.grall@citrix.com>
Date2015-09-25 17:00 +0200
Subject[PATCH v1 0/8] xen/arm: vgic: Support 32-bit access for 64-bit register
Message-ID<qcCDD-4wT-3@gated-at.bofh.it>
Hi all,

This series aims to fix the 32-bit access on 64-bit register. Some guest
OS such as FreeBSD and Linux (only in the ITS) use 32-bit access and will
crash at boot time.

I took the opportunity to go further and optimize the way Xen is storing
registers such as GICD_IPRIORITYR, GICD_ITARGETR and GICD_IROUTER.

Major changes in v2:
    - Use the helpers in GICv2
    - Optimize the assembly input for vgic_regN_* helpers on arm32
    - Add support for sign-extension generically
    - Store GICD_{IPRIORITYR, ITARGETSR, IROUTER} in a better way

For all the changes see in each patch.

A branch has been pushed based on the lastest staging:

git://xenbits.xen.org/people/julieng/xen-unstable.git branch gicv3-32bit-v1

Sincerely yours,

Julien Grall (8):
  xen/arm: io: remove mmio_check_t typedef
  xen/arm: io: Extend write/read handler to pass the register in
    parameter
  xen/arm: Support sign-extension for every read access
  xen/arm: vgic: ctlr stores a 32-bit hardware register so use uint32_t
  xen/arm: vgic: Optimize the way to store GICD_IPRIORITYR in the rank
  xen/arm: vgic: Optimize the way to store the target vCPU in the rank
  xen/arm: vgic: Introduce helpers to read/write/clear/set vGIC register
    ...
  xen/arm: vgic-v3: Support 32-bit access for 64-bit registers

 xen/arch/arm/io.c            |  42 +++++-
 xen/arch/arm/vgic-v2.c       | 295 +++++++++++++++++++++---------------
 xen/arch/arm/vgic-v3.c       | 352 +++++++++++++++++++++++++------------------
 xen/arch/arm/vgic.c          |  72 ++++++++-
 xen/arch/arm/vuart.c         |  16 +-
 xen/include/asm-arm/domain.h |   2 +-
 xen/include/asm-arm/mmio.h   |   5 +-
 xen/include/asm-arm/vgic.h   | 148 +++++++++++++++---
 8 files changed, 617 insertions(+), 315 deletions(-)

-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1232866 — [PATCH v1 3/8] xen/arm: Support sign-extension for every read access

FromJulien Grall <julien.grall@citrix.com>
Date2015-09-25 17:00 +0200
Subject[PATCH v1 3/8] xen/arm: Support sign-extension for every read access
Message-ID<qcCDE-4wT-27@gated-at.bofh.it>
In reply to#1232865
The guest may try to load data from the emulated MMIO region using
instruction with Sign-Extension (i.e ldrs*). This can happen for any
access smaller than the register size (byte/half-word for aarch32,
byte/half-word/word for aarch64).

The support of sign-extension was limited for byte access in vGIG
emulation. Although there is no reason to not have it generically.

So move the support just after we get the data from the MMIO emulation.

Signed-off-by: Julien Grall <julien.grall@citrix.com>

---

I was thinking to completely drop the sign-extension support in Xen as
it will be very unlikely to use ldrs* instruction to access MMIO.
Although the code is fairly small, so it doesn't harm to keep it
generically.

    Changes in v2:
        - Patch added
---
 xen/arch/arm/io.c          | 29 ++++++++++++++++++++++++++++-
 xen/arch/arm/vgic-v2.c     | 10 +++++-----
 xen/arch/arm/vgic-v3.c     |  4 ++--
 xen/include/asm-arm/vgic.h |  8 +++-----
 4 files changed, 38 insertions(+), 13 deletions(-)

diff --git a/xen/arch/arm/io.c b/xen/arch/arm/io.c
index 32b2194..e1b03a2 100644
--- a/xen/arch/arm/io.c
+++ b/xen/arch/arm/io.c
@@ -23,6 +23,32 @@
 #include <asm/current.h>
 #include <asm/mmio.h>
 
+static int handle_read(mmio_read_t read_cb, struct vcpu *v,
+                       mmio_info_t *info, register_t *r)
+{
+    uint8_t size = (1 << info->dabt.size) * 8;
+
+    if ( !read_cb(v, info, r) )
+        return 0;
+
+    /*
+     * Extend the bit sign if required.
+     * Note that we expect the read handler to have zeroed the bit
+     * unused in the register.
+     */
+    if ( info->dabt.sign && (*r & (1UL << (size - 1)) ))
+    {
+        /*
+         * We are relying on register_t as the same size as
+         * an unsigned long or order to keep the 32bit some smaller
+         */
+        BUILD_BUG_ON(sizeof(register_t) != sizeof(unsigned long));
+        *r |= (~0UL) << size;
+    }
+
+    return 1;
+}
+
 int handle_mmio(mmio_info_t *info)
 {
     struct vcpu *v = current;
@@ -48,7 +74,8 @@ found:
     if ( info->dabt.write )
         return mmio_handler->mmio_handler_ops->write_handler(v, info, *r);
     else
-        return mmio_handler->mmio_handler_ops->read_handler(v, info, r);
+        return handle_read(mmio_handler->mmio_handler_ops->read_handler,
+                           v, info, r);
 }
 
 void register_mmio_handler(struct domain *d,
diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
index 3d0ce1d..47f9da9 100644
--- a/xen/arch/arm/vgic-v2.c
+++ b/xen/arch/arm/vgic-v2.c
@@ -129,7 +129,7 @@ static int vgic_v2_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
         *r = rank->v2.itargets[REG_RANK_INDEX(8, gicd_reg - GICD_ITARGETSR,
                                               DABT_WORD)];
         if ( dabt.size == DABT_BYTE )
-            *r = vgic_byte_read(*r, dabt.sign, gicd_reg);
+            *r = vgic_byte_read(*r, gicd_reg);
         vgic_unlock_rank(v, rank, flags);
         return 1;
 
@@ -142,7 +142,7 @@ static int vgic_v2_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
         *r = rank->ipriority[REG_RANK_INDEX(8, gicd_reg - GICD_IPRIORITYR,
                                             DABT_WORD)];
         if ( dabt.size == DABT_BYTE )
-            *r = vgic_byte_read(*r, dabt.sign, gicd_reg);
+            *r = vgic_byte_read(*r, gicd_reg);
         vgic_unlock_rank(v, rank, flags);
         return 1;
 
@@ -377,7 +377,7 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info,
 
             new_target = i % 8;
             old_target_mask = vgic_byte_read(rank->v2.itargets[REG_RANK_INDEX(8,
-                                             gicd_reg - GICD_ITARGETSR, DABT_WORD)], 0, i/8);
+                                             gicd_reg - GICD_ITARGETSR, DABT_WORD)], i/8);
             old_target = find_first_bit(&old_target_mask, 8);
 
             if ( new_target != old_target )
@@ -503,7 +503,7 @@ static struct vcpu *vgic_v2_get_target_vcpu(struct vcpu *v, unsigned int irq)
     ASSERT(spin_is_locked(&rank->lock));
 
     target = vgic_byte_read(rank->v2.itargets[REG_RANK_INDEX(8,
-                                              irq, DABT_WORD)], 0, irq & 0x3);
+                                              irq, DABT_WORD)], irq & 0x3);
 
     /* 1-N SPI should be delivered as pending to all the vcpus in the
      * mask, but here we just return the first vcpu for simplicity and
@@ -521,7 +521,7 @@ static int vgic_v2_get_irq_priority(struct vcpu *v, unsigned int irq)
 
     ASSERT(spin_is_locked(&rank->lock));
     priority = vgic_byte_read(rank->ipriority[REG_RANK_INDEX(8,
-                                              irq, DABT_WORD)], 0, irq & 0x3);
+                                              irq, DABT_WORD)], irq & 0x3);
 
     return priority;
 }
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index 94f1a5c..c013200 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -336,7 +336,7 @@ static int __vgic_v3_distr_common_mmio_read(const char *name, struct vcpu *v,
         *r = rank->ipriority[REG_RANK_INDEX(8, reg - GICD_IPRIORITYR,
                                             DABT_WORD)];
         if ( dabt.size == DABT_BYTE )
-            *r = vgic_byte_read(*r, dabt.sign, reg);
+            *r = vgic_byte_read(*r, reg);
         vgic_unlock_rank(v, rank, flags);
         return 1;
     case GICD_ICFGR ... GICD_ICFGRN:
@@ -1062,7 +1062,7 @@ static int vgic_v3_get_irq_priority(struct vcpu *v, unsigned int irq)
 
     ASSERT(spin_is_locked(&rank->lock));
     priority = vgic_byte_read(rank->ipriority[REG_RANK_INDEX(8,
-                                              irq, DABT_WORD)], 0, irq & 0x3);
+                                              irq, DABT_WORD)], irq & 0x3);
 
     return priority;
 }
diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
index 96839f0..354c0d4 100644
--- a/xen/include/asm-arm/vgic.h
+++ b/xen/include/asm-arm/vgic.h
@@ -158,15 +158,13 @@ static inline int REG_RANK_NR(int b, uint32_t n)
     }
 }
 
-static inline uint32_t vgic_byte_read(uint32_t val, int sign, int offset)
+static inline uint32_t vgic_byte_read(uint32_t val, int offset)
 {
     int byte = offset & 0x3;
 
     val = val >> (8*byte);
-    if ( sign && (val & 0x80) )
-        val |= 0xffffff00;
-    else
-        val &= 0x000000ff;
+    val &= 0x000000ff;
+
     return val;
 }
 
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1232868 — Re: [PATCH v1 0/8] xen/arm: vgic: Support 32-bit access for 64-bit register

FromJulien Grall <julien.grall@citrix.com>
Date2015-09-25 17:00 +0200
SubjectRe: [PATCH v1 0/8] xen/arm: vgic: Support 32-bit access for 64-bit register
Message-ID<qcCDF-4wT-37@gated-at.bofh.it>
In reply to#1232865
Please ignore this version, I've sent it to the wrong mailing list.

Sorry for the noise.

On 25/09/15 15:50, Julien Grall wrote:
> Hi all,
> 
> This series aims to fix the 32-bit access on 64-bit register. Some guest
> OS such as FreeBSD and Linux (only in the ITS) use 32-bit access and will
> crash at boot time.
> 
> I took the opportunity to go further and optimize the way Xen is storing
> registers such as GICD_IPRIORITYR, GICD_ITARGETR and GICD_IROUTER.
> 
> Major changes in v2:
>     - Use the helpers in GICv2
>     - Optimize the assembly input for vgic_regN_* helpers on arm32
>     - Add support for sign-extension generically
>     - Store GICD_{IPRIORITYR, ITARGETSR, IROUTER} in a better way
> 
> For all the changes see in each patch.
> 
> A branch has been pushed based on the lastest staging:
> 
> git://xenbits.xen.org/people/julieng/xen-unstable.git branch gicv3-32bit-v1
> 
> Sincerely yours,
> 
> Julien Grall (8):
>   xen/arm: io: remove mmio_check_t typedef
>   xen/arm: io: Extend write/read handler to pass the register in
>     parameter
>   xen/arm: Support sign-extension for every read access
>   xen/arm: vgic: ctlr stores a 32-bit hardware register so use uint32_t
>   xen/arm: vgic: Optimize the way to store GICD_IPRIORITYR in the rank
>   xen/arm: vgic: Optimize the way to store the target vCPU in the rank
>   xen/arm: vgic: Introduce helpers to read/write/clear/set vGIC register
>     ...
>   xen/arm: vgic-v3: Support 32-bit access for 64-bit registers
> 
>  xen/arch/arm/io.c            |  42 +++++-
>  xen/arch/arm/vgic-v2.c       | 295 +++++++++++++++++++++---------------
>  xen/arch/arm/vgic-v3.c       | 352 +++++++++++++++++++++++++------------------
>  xen/arch/arm/vgic.c          |  72 ++++++++-
>  xen/arch/arm/vuart.c         |  16 +-
>  xen/include/asm-arm/domain.h |   2 +-
>  xen/include/asm-arm/mmio.h   |   5 +-
>  xen/include/asm-arm/vgic.h   | 148 +++++++++++++++---
>  8 files changed, 617 insertions(+), 315 deletions(-)
> 


-- 
Julien Grall
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web