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

Re: [Xen-devel] [PATCH V5 05/10] xen/arm64: gicv3: Use AFF1 when translating ICC_SGI1R_EL1 to cpumask





On 31/05/2015 16:25, Chen Baozi wrote:
On May 31, 2015, at 21:14, Julien Grall <julien.grall@xxxxxxxxxx> wrote:
On 30/05/2015 12:07, Chen Baozi wrote:
From: Chen Baozi <baozich@xxxxxxxxx>

To support more than 16 vCPUs, we have to calculate cpumask with AFF1
field value in ICC_SGI1R_EL1.

Signed-off-by: Chen Baozi <baozich@xxxxxxxxx>
---
  xen/arch/arm/vgic-v3.c            | 9 ++++++++-
  xen/include/asm-arm/gic_v3_defs.h | 3 +++
  2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index a283c8c..21d8d3f 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -976,10 +976,17 @@ static inline void gicv3_sgir_to_cpumask(cpumask_t 
*cpumask,
                                           const register_t sgir)
  {
      unsigned long target_list;
+    int aff1;

unsigned int.


      target_list = sgir & ICH_SGI_TARGETLIST_MASK;
-    bitmap_copy(cpumask_bits(cpumask), &target_list, ICH_SGI_TARGET_BITS);
+    /* We assume that only AFF1 is used in ICC_SGI1R_EL1. */
+    aff1 = (sgir >> ICH_SGI_AFFINITY_LEVEL(1)) & ICH_SGI_AFFx_MASK;

+    BUILD_BUG_ON(sizeof(cpumask_t)*8 < MAX_VIRT_CPUS);

Ah, here is the BUILD_BUG_ON. This is not vgic-v3 specific but generic to all 
the vgic. It would have been more logical to put it in the function vgic_to_sgi 
in the previous patch (i.e #4).

+    BUG_ON(((aff1+1) * ICH_SGI_TARGET_BITS) > NR_CPUS);

NACK. This value is passed by the guest. With this a malicious guest could take 
down Xen.

+
+    memcpy((uint16_t *)cpumask + aff1, &target_list,

That's hackhish. You can't assume that the bitmap will be at the beginning of 
cpumask_t.

Sorry, I’m not quite understand this. Why can not?

cpumask_t is a structure containing a bitmap. You are assuming that the address of cpumask_t is always equal to cpumask_t.bitmap.
There is helper to get the correct address (see cpumask_bits(cpumask)).

Furthermore, there is function to copy a bitmap (see bitmap_copy). Although it will require more than 1 line of code.

If you don't want to use it please use a direct assignation (it's only a 16 bits data) which will always be faster than a memcpy.

Regards,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

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