* [PATCH v1 1/8] xen/arm: io: remove mmio_check_t typedef
[not found] <1443192667-16112-1-git-send-email-julien.grall@citrix.com>
@ 2015-09-25 14:51 ` Julien Grall
2015-09-25 14:51 ` [PATCH v1 2/8] xen/arm: io: Extend write/read handler to pass the register in parameter Julien Grall
` (7 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Julien Grall @ 2015-09-25 14:51 UTC (permalink / raw)
To: xen-devel
Cc: linux-kernel, Julien Grall, ian.campbell, linux-arm-kernel,
stefano.stabellini
From: Julien Grall <julien.grall@linaro.org>
This typedef is a left-over of the previous MMIO emulation
implementation.
Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
Changes in v2:
- Patch added
---
xen/include/asm-arm/mmio.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/xen/include/asm-arm/mmio.h b/xen/include/asm-arm/mmio.h
index 0160f09..7278bce 100644
--- a/xen/include/asm-arm/mmio.h
+++ b/xen/include/asm-arm/mmio.h
@@ -34,7 +34,6 @@ typedef struct
typedef int (*mmio_read_t)(struct vcpu *v, mmio_info_t *info);
typedef int (*mmio_write_t)(struct vcpu *v, mmio_info_t *info);
-typedef int (*mmio_check_t)(struct vcpu *v, paddr_t addr);
struct mmio_handler_ops {
mmio_read_t read_handler;
--
2.1.4
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v1 2/8] xen/arm: io: Extend write/read handler to pass the register in parameter
[not found] <1443192667-16112-1-git-send-email-julien.grall@citrix.com>
2015-09-25 14:51 ` [PATCH v1 1/8] xen/arm: io: remove mmio_check_t typedef Julien Grall
@ 2015-09-25 14:51 ` Julien Grall
2015-09-25 14:51 ` [PATCH v1 3/8] xen/arm: Support sign-extension for every read access Julien Grall
` (6 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Julien Grall @ 2015-09-25 14:51 UTC (permalink / raw)
To: xen-devel
Cc: ian.campbell, stefano.stabellini, Julien Grall, linux-kernel,
Julien Grall, linux-arm-kernel
From: Julien Grall <julien.grall@linaro.org>
Rather than letting each handler to retrieve the register used by the
I/O access, add a new parameter to pass the register in parameter.
This will help to implement generic register manipulation on I/O access
such as sign-extension and endianess.
Read handlers need to modify the value of the register, so a pointer to
it is given in argument. Write handlers shouldn't modify the register,
therfore only a plain value is given.
Signed-off-by: Julien Grall <julien.grall@citrix.com>
---
Changes in v2:
- Patch added
---
xen/arch/arm/io.c | 15 ++++---
xen/arch/arm/vgic-v2.c | 46 ++++++++++----------
xen/arch/arm/vgic-v3.c | 105 +++++++++++++++++++++------------------------
xen/arch/arm/vuart.c | 16 +++----
xen/include/asm-arm/mmio.h | 4 +-
5 files changed, 88 insertions(+), 98 deletions(-)
diff --git a/xen/arch/arm/io.c b/xen/arch/arm/io.c
index 8e55d49..32b2194 100644
--- a/xen/arch/arm/io.c
+++ b/xen/arch/arm/io.c
@@ -29,6 +29,9 @@ int handle_mmio(mmio_info_t *info)
int i;
const struct mmio_handler *mmio_handler;
const struct io_handler *io_handlers = &v->domain->arch.io_handlers;
+ struct hsr_dabt dabt = info->dabt;
+ struct cpu_user_regs *regs = guest_cpu_user_regs();
+ register_t *r = select_user_reg(regs, dabt.reg);
for ( i = 0; i < io_handlers->num_entries; i++ )
{
@@ -36,14 +39,16 @@ int handle_mmio(mmio_info_t *info)
if ( (info->gpa >= mmio_handler->addr) &&
(info->gpa < (mmio_handler->addr + mmio_handler->size)) )
- {
- return info->dabt.write ?
- mmio_handler->mmio_handler_ops->write_handler(v, info) :
- mmio_handler->mmio_handler_ops->read_handler(v, info);
- }
+ goto found;
}
return 0;
+
+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);
}
void register_mmio_handler(struct domain *d,
diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
index fa71598..3d0ce1d 100644
--- a/xen/arch/arm/vgic-v2.c
+++ b/xen/arch/arm/vgic-v2.c
@@ -50,11 +50,10 @@ void vgic_v2_setup_hw(paddr_t dbase, paddr_t cbase, paddr_t vbase)
vgic_v2_hw.vbase = vbase;
}
-static int vgic_v2_distr_mmio_read(struct vcpu *v, mmio_info_t *info)
+static int vgic_v2_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
+ register_t *r)
{
struct hsr_dabt dabt = info->dabt;
- struct cpu_user_regs *regs = guest_cpu_user_regs();
- register_t *r = select_user_reg(regs, dabt.reg);
struct vgic_irq_rank *rank;
int gicd_reg = (int)(info->gpa - v->domain->arch.vgic.dbase);
unsigned long flags;
@@ -247,11 +246,10 @@ static int vgic_v2_to_sgi(struct vcpu *v, register_t sgir)
return vgic_to_sgi(v, sgir, sgi_mode, virq, &target);
}
-static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
+static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info,
+ register_t r)
{
struct hsr_dabt dabt = info->dabt;
- struct cpu_user_regs *regs = guest_cpu_user_regs();
- register_t *r = select_user_reg(regs, dabt.reg);
struct vgic_irq_rank *rank;
int gicd_reg = (int)(info->gpa - v->domain->arch.vgic.dbase);
uint32_t tr;
@@ -265,7 +263,7 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
if ( dabt.size != DABT_WORD ) goto bad_width;
/* Ignore all but the enable bit */
vgic_lock(v);
- v->domain->arch.vgic.ctlr = (*r) & GICD_CTL_ENABLE;
+ v->domain->arch.vgic.ctlr = r & GICD_CTL_ENABLE;
vgic_unlock(v);
return 1;
@@ -289,11 +287,11 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
if ( rank == NULL) goto write_ignore;
vgic_lock_rank(v, rank, flags);
tr = rank->ienable;
- rank->ienable |= *r;
+ rank->ienable |= r;
/* The virtual irq is derived from register offset.
* The register difference is word difference. So divide by 2(DABT_WORD)
* to get Virtual irq number */
- vgic_enable_irqs(v, (*r) & (~tr),
+ vgic_enable_irqs(v, r & (~tr),
(gicd_reg - GICD_ISENABLER) >> DABT_WORD);
vgic_unlock_rank(v, rank, flags);
return 1;
@@ -304,11 +302,11 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
if ( rank == NULL) goto write_ignore;
vgic_lock_rank(v, rank, flags);
tr = rank->ienable;
- rank->ienable &= ~*r;
+ rank->ienable &= ~r;
/* The virtual irq is derived from register offset.
* The register difference is word difference. So divide by 2(DABT_WORD)
* to get Virtual irq number */
- vgic_disable_irqs(v, (*r) & tr,
+ vgic_disable_irqs(v, r & tr,
(gicd_reg - GICD_ICENABLER) >> DABT_WORD);
vgic_unlock_rank(v, rank, flags);
return 1;
@@ -317,28 +315,28 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
if ( dabt.size != DABT_WORD ) goto bad_width;
printk(XENLOG_G_ERR
"%pv: vGICD: unhandled word write %#"PRIregister" to ISPENDR%d\n",
- v, *r, gicd_reg - GICD_ISPENDR);
+ v, r, gicd_reg - GICD_ISPENDR);
return 0;
case GICD_ICPENDR ... GICD_ICPENDRN:
if ( dabt.size != DABT_WORD ) goto bad_width;
printk(XENLOG_G_ERR
"%pv: vGICD: unhandled word write %#"PRIregister" to ICPENDR%d\n",
- v, *r, gicd_reg - GICD_ICPENDR);
+ v, r, gicd_reg - GICD_ICPENDR);
return 0;
case GICD_ISACTIVER ... GICD_ISACTIVERN:
if ( dabt.size != DABT_WORD ) goto bad_width;
printk(XENLOG_G_ERR
"%pv: vGICD: unhandled word write %#"PRIregister" to ISACTIVER%d\n",
- v, *r, gicd_reg - GICD_ISACTIVER);
+ v, r, gicd_reg - GICD_ISACTIVER);
return 0;
case GICD_ICACTIVER ... GICD_ICACTIVERN:
if ( dabt.size != DABT_WORD ) goto bad_width;
printk(XENLOG_G_ERR
"%pv: vGICD: unhandled word write %#"PRIregister" to ICACTIVER%d\n",
- v, *r, gicd_reg - GICD_ICACTIVER);
+ v, r, gicd_reg - GICD_ICACTIVER);
return 0;
case GICD_ITARGETSR ... GICD_ITARGETSR + 7:
@@ -360,7 +358,7 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
target = target | (target << 8) | (target << 16) | (target << 24);
else
target = (target << (8 * (gicd_reg & 0x3)));
- target &= *r;
+ target &= r;
/* ignore zero writes */
if ( !target )
goto write_ignore;
@@ -408,10 +406,10 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
vgic_lock_rank(v, rank, flags);
if ( dabt.size == DABT_WORD )
rank->ipriority[REG_RANK_INDEX(8, gicd_reg - GICD_IPRIORITYR,
- DABT_WORD)] = *r;
+ DABT_WORD)] = r;
else
vgic_byte_write(&rank->ipriority[REG_RANK_INDEX(8,
- gicd_reg - GICD_IPRIORITYR, DABT_WORD)], *r, gicd_reg);
+ gicd_reg - GICD_IPRIORITYR, DABT_WORD)], r, gicd_reg);
vgic_unlock_rank(v, rank, flags);
return 1;
@@ -425,7 +423,7 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
rank = vgic_rank_offset(v, 2, gicd_reg - GICD_ICFGR, DABT_WORD);
if ( rank == NULL) goto write_ignore;
vgic_lock_rank(v, rank, flags);
- rank->icfg[REG_RANK_INDEX(2, gicd_reg - GICD_ICFGR, DABT_WORD)] = *r;
+ rank->icfg[REG_RANK_INDEX(2, gicd_reg - GICD_ICFGR, DABT_WORD)] = r;
vgic_unlock_rank(v, rank, flags);
return 1;
@@ -435,20 +433,20 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
case GICD_SGIR:
if ( dabt.size != DABT_WORD ) goto bad_width;
- return vgic_v2_to_sgi(v, *r);
+ return vgic_v2_to_sgi(v, r);
case GICD_CPENDSGIR ... GICD_CPENDSGIRN:
if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
printk(XENLOG_G_ERR
"%pv: vGICD: unhandled %s write %#"PRIregister" to ICPENDSGIR%d\n",
- v, dabt.size ? "word" : "byte", *r, gicd_reg - GICD_CPENDSGIR);
+ v, dabt.size ? "word" : "byte", r, gicd_reg - GICD_CPENDSGIR);
return 0;
case GICD_SPENDSGIR ... GICD_SPENDSGIRN:
if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
printk(XENLOG_G_ERR
"%pv: vGICD: unhandled %s write %#"PRIregister" to ISPENDSGIR%d\n",
- v, dabt.size ? "word" : "byte", *r, gicd_reg - GICD_SPENDSGIR);
+ v, dabt.size ? "word" : "byte", r, gicd_reg - GICD_SPENDSGIR);
return 0;
/* Implementation defined -- write ignored */
@@ -475,14 +473,14 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
default:
printk(XENLOG_G_ERR
"%pv: vGICD: unhandled write r%d=%"PRIregister" offset %#08x\n",
- v, dabt.reg, *r, gicd_reg);
+ v, dabt.reg, r, gicd_reg);
return 0;
}
bad_width:
printk(XENLOG_G_ERR
"%pv: vGICD: bad write width %d r%d=%"PRIregister" offset %#08x\n",
- v, dabt.size, dabt.reg, *r, gicd_reg);
+ v, dabt.size, dabt.reg, r, gicd_reg);
domain_crash_synchronous();
return 0;
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index f1c482d..94f1a5c 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -104,11 +104,10 @@ static struct vcpu *vgic_v3_get_target_vcpu(struct vcpu *v, unsigned int irq)
}
static int __vgic_v3_rdistr_rd_mmio_read(struct vcpu *v, mmio_info_t *info,
- uint32_t gicr_reg)
+ uint32_t gicr_reg,
+ register_t *r)
{
struct hsr_dabt dabt = info->dabt;
- struct cpu_user_regs *regs = guest_cpu_user_regs();
- register_t *r = select_user_reg(regs, dabt.reg);
uint64_t aff;
switch ( gicr_reg )
@@ -215,11 +214,10 @@ read_as_zero_32:
}
static int __vgic_v3_rdistr_rd_mmio_write(struct vcpu *v, mmio_info_t *info,
- uint32_t gicr_reg)
+ uint32_t gicr_reg,
+ register_t r)
{
struct hsr_dabt dabt = info->dabt;
- struct cpu_user_regs *regs = guest_cpu_user_regs();
- register_t *r = select_user_reg(regs, dabt.reg);
switch ( gicr_reg )
{
@@ -276,7 +274,7 @@ static int __vgic_v3_rdistr_rd_mmio_write(struct vcpu *v, mmio_info_t *info,
bad_width:
printk(XENLOG_G_ERR
"%pv: vGICR: bad write width %d r%d=%"PRIregister" offset %#08x\n",
- v, dabt.size, dabt.reg, *r, gicr_reg);
+ v, dabt.size, dabt.reg, r, gicr_reg);
domain_crash_synchronous();
return 0;
@@ -290,11 +288,10 @@ write_ignore_32:
}
static int __vgic_v3_distr_common_mmio_read(const char *name, struct vcpu *v,
- mmio_info_t *info, uint32_t reg)
+ mmio_info_t *info, uint32_t reg,
+ register_t *r)
{
struct hsr_dabt dabt = info->dabt;
- struct cpu_user_regs *regs = guest_cpu_user_regs();
- register_t *r = select_user_reg(regs, dabt.reg);
struct vgic_irq_rank *rank;
unsigned long flags;
@@ -369,11 +366,10 @@ read_as_zero:
}
static int __vgic_v3_distr_common_mmio_write(const char *name, struct vcpu *v,
- mmio_info_t *info, uint32_t reg)
+ mmio_info_t *info, uint32_t reg,
+ register_t r)
{
struct hsr_dabt dabt = info->dabt;
- struct cpu_user_regs *regs = guest_cpu_user_regs();
- register_t *r = select_user_reg(regs, dabt.reg);
struct vgic_irq_rank *rank;
uint32_t tr;
unsigned long flags;
@@ -389,9 +385,9 @@ static int __vgic_v3_distr_common_mmio_write(const char *name, struct vcpu *v,
if ( rank == NULL ) goto write_ignore;
vgic_lock_rank(v, rank, flags);
tr = rank->ienable;
- rank->ienable |= *r;
+ rank->ienable |= r;
/* The irq number is extracted from offset. so shift by register size */
- vgic_enable_irqs(v, (*r) & (~tr), (reg - GICD_ISENABLER) >> DABT_WORD);
+ vgic_enable_irqs(v, r & (~tr), (reg - GICD_ISENABLER) >> DABT_WORD);
vgic_unlock_rank(v, rank, flags);
return 1;
case GICD_ICENABLER ... GICD_ICENABLERN:
@@ -400,37 +396,37 @@ static int __vgic_v3_distr_common_mmio_write(const char *name, struct vcpu *v,
if ( rank == NULL ) goto write_ignore;
vgic_lock_rank(v, rank, flags);
tr = rank->ienable;
- rank->ienable &= ~*r;
+ rank->ienable &= ~r;
/* The irq number is extracted from offset. so shift by register size */
- vgic_disable_irqs(v, (*r) & tr, (reg - GICD_ICENABLER) >> DABT_WORD);
+ vgic_disable_irqs(v, r & tr, (reg - GICD_ICENABLER) >> DABT_WORD);
vgic_unlock_rank(v, rank, flags);
return 1;
case GICD_ISPENDR ... GICD_ISPENDRN:
if ( dabt.size != DABT_WORD ) goto bad_width;
printk(XENLOG_G_ERR
"%pv: %s: unhandled word write %#"PRIregister" to ISPENDR%d\n",
- v, name, *r, reg - GICD_ISPENDR);
+ v, name, r, reg - GICD_ISPENDR);
return 0;
case GICD_ICPENDR ... GICD_ICPENDRN:
if ( dabt.size != DABT_WORD ) goto bad_width;
printk(XENLOG_G_ERR
"%pv: %s: unhandled word write %#"PRIregister" to ICPENDR%d\n",
- v, name, *r, reg - GICD_ICPENDR);
+ v, name, r, reg - GICD_ICPENDR);
return 0;
case GICD_ISACTIVER ... GICD_ISACTIVERN:
if ( dabt.size != DABT_WORD ) goto bad_width;
printk(XENLOG_G_ERR
"%pv: %s: unhandled word write %#"PRIregister" to ISACTIVER%d\n",
- v, name, *r, reg - GICD_ISACTIVER);
+ v, name, r, reg - GICD_ISACTIVER);
return 0;
case GICD_ICACTIVER ... GICD_ICACTIVERN:
if ( dabt.size != DABT_WORD ) goto bad_width;
printk(XENLOG_G_ERR
"%pv: %s: unhandled word write %#"PRIregister" to ICACTIVER%d\n",
- v, name, *r, reg - GICD_ICACTIVER);
+ v, name, r, reg - GICD_ICACTIVER);
return 0;
case GICD_IPRIORITYR ... GICD_IPRIORITYRN:
@@ -440,10 +436,10 @@ static int __vgic_v3_distr_common_mmio_write(const char *name, struct vcpu *v,
vgic_lock_rank(v, rank, flags);
if ( dabt.size == DABT_WORD )
rank->ipriority[REG_RANK_INDEX(8, reg - GICD_IPRIORITYR,
- DABT_WORD)] = *r;
+ DABT_WORD)] = r;
else
vgic_byte_write(&rank->ipriority[REG_RANK_INDEX(8,
- reg - GICD_IPRIORITYR, DABT_WORD)], *r, reg);
+ reg - GICD_IPRIORITYR, DABT_WORD)], r, reg);
vgic_unlock_rank(v, rank, flags);
return 1;
case GICD_ICFGR: /* Restricted to configure SGIs */
@@ -455,20 +451,20 @@ static int __vgic_v3_distr_common_mmio_write(const char *name, struct vcpu *v,
rank = vgic_rank_offset(v, 2, reg - GICD_ICFGR, DABT_WORD);
if ( rank == NULL ) goto write_ignore;
vgic_lock_rank(v, rank, flags);
- rank->icfg[REG_RANK_INDEX(2, reg - GICD_ICFGR, DABT_WORD)] = *r;
+ rank->icfg[REG_RANK_INDEX(2, reg - GICD_ICFGR, DABT_WORD)] = r;
vgic_unlock_rank(v, rank, flags);
return 1;
default:
printk(XENLOG_G_ERR
"%pv: %s: unhandled write r%d=%"PRIregister" offset %#08x\n",
- v, name, dabt.reg, *r, reg);
+ v, name, dabt.reg, r, reg);
return 0;
}
bad_width:
printk(XENLOG_G_ERR
"%pv: %s: bad write width %d r%d=%"PRIregister" offset %#08x\n",
- v, name, dabt.size, dabt.reg, *r, reg);
+ v, name, dabt.size, dabt.reg, r, reg);
domain_crash_synchronous();
return 0;
@@ -479,11 +475,9 @@ write_ignore:
}
static int vgic_v3_rdistr_sgi_mmio_read(struct vcpu *v, mmio_info_t *info,
- uint32_t gicr_reg)
+ uint32_t gicr_reg, register_t *r)
{
struct hsr_dabt dabt = info->dabt;
- struct cpu_user_regs *regs = guest_cpu_user_regs();
- register_t *r = select_user_reg(regs, dabt.reg);
switch ( gicr_reg )
{
@@ -502,7 +496,7 @@ static int vgic_v3_rdistr_sgi_mmio_read(struct vcpu *v, mmio_info_t *info,
* So handle in common with GICD handling
*/
return __vgic_v3_distr_common_mmio_read("vGICR: SGI", v, info,
- gicr_reg);
+ gicr_reg, r);
/* Read the pending status of an SGI is via GICR is not supported */
case GICR_ISPENDR0:
@@ -533,11 +527,9 @@ read_as_zero:
}
static int vgic_v3_rdistr_sgi_mmio_write(struct vcpu *v, mmio_info_t *info,
- uint32_t gicr_reg)
+ uint32_t gicr_reg, register_t r)
{
struct hsr_dabt dabt = info->dabt;
- struct cpu_user_regs *regs = guest_cpu_user_regs();
- register_t *r = select_user_reg(regs, dabt.reg);
switch ( gicr_reg )
{
@@ -556,19 +548,19 @@ static int vgic_v3_rdistr_sgi_mmio_write(struct vcpu *v, mmio_info_t *info,
* So handle common with GICD handling
*/
return __vgic_v3_distr_common_mmio_write("vGICR: SGI", v,
- info, gicr_reg);
+ info, gicr_reg, r);
case GICR_ISPENDR0:
if ( dabt.size != DABT_WORD ) goto bad_width;
printk(XENLOG_G_ERR
"%pv: vGICR: SGI: unhandled word write %#"PRIregister" to ISPENDR0\n",
- v, *r);
+ v, r);
return 0;
case GICR_ICPENDR0:
if ( dabt.size != DABT_WORD ) goto bad_width;
printk(XENLOG_G_ERR
"%pv: vGICR: SGI: unhandled word write %#"PRIregister" to ICPENDR0\n",
- v, *r);
+ v, r);
return 0;
case GICR_NSACR:
@@ -584,7 +576,7 @@ static int vgic_v3_rdistr_sgi_mmio_write(struct vcpu *v, mmio_info_t *info,
bad_width:
printk(XENLOG_G_ERR
"%pv: vGICR: SGI: bad write width %d r%d=%"PRIregister" offset %#08x\n",
- v, dabt.size, dabt.reg, *r, gicr_reg);
+ v, dabt.size, dabt.reg, r, gicr_reg);
domain_crash_synchronous();
return 0;
@@ -634,7 +626,8 @@ static inline struct vcpu *get_vcpu_from_rdist(paddr_t gpa,
return d->vcpu[vcpu_id];
}
-static int vgic_v3_rdistr_mmio_read(struct vcpu *v, mmio_info_t *info)
+static int vgic_v3_rdistr_mmio_read(struct vcpu *v, mmio_info_t *info,
+ register_t *r)
{
uint32_t offset;
@@ -645,9 +638,9 @@ static int vgic_v3_rdistr_mmio_read(struct vcpu *v, mmio_info_t *info)
return 0;
if ( offset < SZ_64K )
- return __vgic_v3_rdistr_rd_mmio_read(v, info, offset);
+ return __vgic_v3_rdistr_rd_mmio_read(v, info, offset, r);
else if ( (offset >= SZ_64K) && (offset < 2 * SZ_64K) )
- return vgic_v3_rdistr_sgi_mmio_read(v, info, (offset - SZ_64K));
+ return vgic_v3_rdistr_sgi_mmio_read(v, info, (offset - SZ_64K), r);
else
printk(XENLOG_G_WARNING
"%pv: vGICR: unknown gpa read address %"PRIpaddr"\n",
@@ -656,7 +649,8 @@ static int vgic_v3_rdistr_mmio_read(struct vcpu *v, mmio_info_t *info)
return 0;
}
-static int vgic_v3_rdistr_mmio_write(struct vcpu *v, mmio_info_t *info)
+static int vgic_v3_rdistr_mmio_write(struct vcpu *v, mmio_info_t *info,
+ register_t r)
{
uint32_t offset;
@@ -667,9 +661,9 @@ static int vgic_v3_rdistr_mmio_write(struct vcpu *v, mmio_info_t *info)
return 0;
if ( offset < SZ_64K )
- return __vgic_v3_rdistr_rd_mmio_write(v, info, offset);
+ return __vgic_v3_rdistr_rd_mmio_write(v, info, offset, r);
else if ( (offset >= SZ_64K) && (offset < 2 * SZ_64K) )
- return vgic_v3_rdistr_sgi_mmio_write(v, info, (offset - SZ_64K));
+ return vgic_v3_rdistr_sgi_mmio_write(v, info, (offset - SZ_64K), r);
else
printk(XENLOG_G_WARNING
"%pv: vGICR: unknown gpa write address %"PRIpaddr"\n",
@@ -678,11 +672,10 @@ static int vgic_v3_rdistr_mmio_write(struct vcpu *v, mmio_info_t *info)
return 0;
}
-static int vgic_v3_distr_mmio_read(struct vcpu *v, mmio_info_t *info)
+static int vgic_v3_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
+ register_t *r)
{
struct hsr_dabt dabt = info->dabt;
- struct cpu_user_regs *regs = guest_cpu_user_regs();
- register_t *r = select_user_reg(regs, dabt.reg);
struct vgic_irq_rank *rank;
unsigned long flags;
int gicd_reg = (int)(info->gpa - v->domain->arch.vgic.dbase);
@@ -745,7 +738,7 @@ static int vgic_v3_distr_mmio_read(struct vcpu *v, mmio_info_t *info)
* Above all register are common with GICR and GICD
* Manage in common
*/
- return __vgic_v3_distr_common_mmio_read("vGICD", v, info, gicd_reg);
+ return __vgic_v3_distr_common_mmio_read("vGICD", v, info, gicd_reg, r);
case GICD_IROUTER ... GICD_IROUTER31:
/* SGI/PPI is RES0 */
goto read_as_zero_64;
@@ -835,11 +828,10 @@ read_as_zero:
return 1;
}
-static int vgic_v3_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
+static int vgic_v3_distr_mmio_write(struct vcpu *v, mmio_info_t *info,
+ register_t r)
{
struct hsr_dabt dabt = info->dabt;
- struct cpu_user_regs *regs = guest_cpu_user_regs();
- register_t *r = select_user_reg(regs, dabt.reg);
struct vgic_irq_rank *rank;
unsigned long flags;
uint64_t new_irouter, old_irouter;
@@ -855,7 +847,7 @@ static int vgic_v3_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
vgic_lock(v);
/* Only EnableGrp1A can be changed */
- if ( *r & GICD_CTLR_ENABLE_G1A )
+ if ( r & GICD_CTLR_ENABLE_G1A )
v->domain->arch.vgic.ctlr |= GICD_CTLR_ENABLE_G1A;
else
v->domain->arch.vgic.ctlr &= ~GICD_CTLR_ENABLE_G1A;
@@ -901,7 +893,8 @@ static int vgic_v3_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
case GICD_ICFGR ... GICD_ICFGRN:
/* Above registers are common with GICR and GICD
* Manage in common */
- return __vgic_v3_distr_common_mmio_write("vGICD", v, info, gicd_reg);
+ return __vgic_v3_distr_common_mmio_write("vGICD", v, info,
+ gicd_reg, r);
case GICD_IROUTER ... GICD_IROUTER31:
/* SGI/PPI is RES0 */
goto write_ignore_64;
@@ -910,7 +903,7 @@ static int vgic_v3_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
rank = vgic_rank_offset(v, 64, gicd_reg - GICD_IROUTER,
DABT_DOUBLE_WORD);
if ( rank == NULL ) goto write_ignore;
- new_irouter = *r;
+ new_irouter = r;
vgic_lock_rank(v, rank, flags);
old_irouter = rank->v3.irouter[REG_RANK_INDEX(64,
@@ -923,7 +916,7 @@ static int vgic_v3_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
{
printk(XENLOG_G_DEBUG
"%pv: vGICD: wrong irouter at offset %#08x val %#"PRIregister,
- v, gicd_reg, *r);
+ v, gicd_reg, r);
vgic_unlock_rank(v, rank, flags);
/*
* TODO: Don't inject a fault to the guest when the MPIDR is
@@ -969,14 +962,14 @@ static int vgic_v3_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
default:
printk(XENLOG_G_ERR
"%pv: vGICD: unhandled write r%d=%"PRIregister" offset %#08x\n",
- v, dabt.reg, *r, gicd_reg);
+ v, dabt.reg, r, gicd_reg);
return 0;
}
bad_width:
printk(XENLOG_G_ERR
"%pv: vGICD: bad write width %d r%d=%"PRIregister" offset %#08x\n",
- v, dabt.size, dabt.reg, *r, gicd_reg);
+ v, dabt.size, dabt.reg, r, gicd_reg);
domain_crash_synchronous();
return 0;
diff --git a/xen/arch/arm/vuart.c b/xen/arch/arm/vuart.c
index d9f4249..5b8409c 100644
--- a/xen/arch/arm/vuart.c
+++ b/xen/arch/arm/vuart.c
@@ -45,8 +45,8 @@
#define domain_has_vuart(d) ((d)->arch.vuart.info != NULL)
-static int vuart_mmio_read(struct vcpu *v, mmio_info_t *info);
-static int vuart_mmio_write(struct vcpu *v, mmio_info_t *info);
+static int vuart_mmio_read(struct vcpu *v, mmio_info_t *info, register_t *r);
+static int vuart_mmio_write(struct vcpu *v, mmio_info_t *info, register_t r);
static const struct mmio_handler_ops vuart_mmio_handler = {
.read_handler = vuart_mmio_read,
@@ -105,12 +105,9 @@ static void vuart_print_char(struct vcpu *v, char c)
spin_unlock(&uart->lock);
}
-static int vuart_mmio_read(struct vcpu *v, mmio_info_t *info)
+static int vuart_mmio_read(struct vcpu *v, mmio_info_t *info, register_t *r)
{
struct domain *d = v->domain;
- struct hsr_dabt dabt = info->dabt;
- struct cpu_user_regs *regs = guest_cpu_user_regs();
- register_t *r = select_user_reg(regs, dabt.reg);
paddr_t offset = info->gpa - d->arch.vuart.info->base_addr;
perfc_incr(vuart_reads);
@@ -125,19 +122,16 @@ static int vuart_mmio_read(struct vcpu *v, mmio_info_t *info)
return 1;
}
-static int vuart_mmio_write(struct vcpu *v, mmio_info_t *info)
+static int vuart_mmio_write(struct vcpu *v, mmio_info_t *info, register_t r)
{
struct domain *d = v->domain;
- struct hsr_dabt dabt = info->dabt;
- struct cpu_user_regs *regs = guest_cpu_user_regs();
- register_t *r = select_user_reg(regs, dabt.reg);
paddr_t offset = info->gpa - d->arch.vuart.info->base_addr;
perfc_incr(vuart_writes);
if ( offset == d->arch.vuart.info->data_off )
/* ignore any status bits */
- vuart_print_char(v, *r & 0xFF);
+ vuart_print_char(v, r & 0xFF);
return 1;
}
diff --git a/xen/include/asm-arm/mmio.h b/xen/include/asm-arm/mmio.h
index 7278bce..b511334 100644
--- a/xen/include/asm-arm/mmio.h
+++ b/xen/include/asm-arm/mmio.h
@@ -32,8 +32,8 @@ typedef struct
paddr_t gpa;
} mmio_info_t;
-typedef int (*mmio_read_t)(struct vcpu *v, mmio_info_t *info);
-typedef int (*mmio_write_t)(struct vcpu *v, mmio_info_t *info);
+typedef int (*mmio_read_t)(struct vcpu *v, mmio_info_t *info, register_t *r);
+typedef int (*mmio_write_t)(struct vcpu *v, mmio_info_t *info, register_t r);
struct mmio_handler_ops {
mmio_read_t read_handler;
--
2.1.4
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v1 3/8] xen/arm: Support sign-extension for every read access
[not found] <1443192667-16112-1-git-send-email-julien.grall@citrix.com>
2015-09-25 14:51 ` [PATCH v1 1/8] xen/arm: io: remove mmio_check_t typedef Julien Grall
2015-09-25 14:51 ` [PATCH v1 2/8] xen/arm: io: Extend write/read handler to pass the register in parameter Julien Grall
@ 2015-09-25 14:51 ` Julien Grall
2015-09-25 14:51 ` [PATCH v1 4/8] xen/arm: vgic: ctlr stores a 32-bit hardware register so use uint32_t Julien Grall
` (5 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Julien Grall @ 2015-09-25 14:51 UTC (permalink / raw)
To: xen-devel
Cc: Julien Grall, linux-kernel, ian.campbell, linux-arm-kernel,
stefano.stabellini
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
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v1 4/8] xen/arm: vgic: ctlr stores a 32-bit hardware register so use uint32_t
[not found] <1443192667-16112-1-git-send-email-julien.grall@citrix.com>
` (2 preceding siblings ...)
2015-09-25 14:51 ` [PATCH v1 3/8] xen/arm: Support sign-extension for every read access Julien Grall
@ 2015-09-25 14:51 ` Julien Grall
2015-09-25 14:51 ` [PATCH v1 5/8] xen/arm: vgic: Optimize the way to store GICD_IPRIORITYR in the rank Julien Grall
` (4 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Julien Grall @ 2015-09-25 14:51 UTC (permalink / raw)
To: xen-devel
Cc: Julien Grall, linux-kernel, ian.campbell, linux-arm-kernel,
stefano.stabellini
Signed-off-by: Julien Grall <julien.grall@citrix.com>
---
Changes in v2:
- Patch added
---
xen/include/asm-arm/domain.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index c3f5a95..1a5bfd7 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -90,7 +90,7 @@ struct arch_domain
* rank order.
*/
spinlock_t lock;
- int ctlr;
+ uint32_t ctlr;
int nr_spis; /* Number of SPIs */
unsigned long *allocated_irqs; /* bitmap of IRQs allocated */
struct vgic_irq_rank *shared_irqs;
--
2.1.4
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v1 5/8] xen/arm: vgic: Optimize the way to store GICD_IPRIORITYR in the rank
[not found] <1443192667-16112-1-git-send-email-julien.grall@citrix.com>
` (3 preceding siblings ...)
2015-09-25 14:51 ` [PATCH v1 4/8] xen/arm: vgic: ctlr stores a 32-bit hardware register so use uint32_t Julien Grall
@ 2015-09-25 14:51 ` Julien Grall
2015-09-25 14:51 ` [PATCH v1 6/8] xen/arm: vgic: Optimize the way to store the target vCPU " Julien Grall
` (3 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Julien Grall @ 2015-09-25 14:51 UTC (permalink / raw)
To: xen-devel
Cc: Julien Grall, linux-kernel, ian.campbell, linux-arm-kernel,
stefano.stabellini
Xen is currently directly storing the value of register GICD_IPRIORITYR
in the rank. This makes emulation of the register access very simple
but makes the code to get the priority for a given IRQ more complex.
While the priority of an IRQ is retrieved everytime an IRQ is injected
to the guest, the access to register occurs less often.
So the data structure should be optimized for the most common case
rather than the inverse.
This patch introduces the usage of an array to store the priority for
every interrupt in the rank. This will make the code to get the priority
very quick. The emulation code will now have to generate the GICD_PRIORITYR
register for read access and split it to store in a convenient way.
Signed-off-by: Julien Grall <julien.grall@citrix.com>
---
The real reason is I'd like to drop vgic_byte_* helpers in favors of more
generic access helper. Although it would make the code to retrieve the
priority more complex. So reworking the way to get the priority was the
best solution.
Changes in v2:
- Patch added
---
xen/arch/arm/vgic-v2.c | 24 ++++++++++++++----------
xen/arch/arm/vgic-v3.c | 27 ++++++++++++++++-----------
xen/arch/arm/vgic.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
xen/include/asm-arm/vgic.h | 18 +++++++++++++++++-
4 files changed, 93 insertions(+), 22 deletions(-)
diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
index 47f9da9..23d1982 100644
--- a/xen/arch/arm/vgic-v2.c
+++ b/xen/arch/arm/vgic-v2.c
@@ -139,8 +139,8 @@ static int vgic_v2_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
if ( rank == NULL) goto read_as_zero;
vgic_lock_rank(v, rank, flags);
- *r = rank->ipriority[REG_RANK_INDEX(8, gicd_reg - GICD_IPRIORITYR,
- DABT_WORD)];
+ /* Recreate the IPRIORITYR register */
+ *r = vgic_generate_ipriorityr(rank, gicd_reg - GICD_IPRIORITYR);
if ( dabt.size == DABT_BYTE )
*r = vgic_byte_read(*r, gicd_reg);
vgic_unlock_rank(v, rank, flags);
@@ -400,18 +400,25 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info,
}
case GICD_IPRIORITYR ... GICD_IPRIORITYRN:
+ {
+ uint32_t ipriorityr;
+
if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
rank = vgic_rank_offset(v, 8, gicd_reg - GICD_IPRIORITYR, DABT_WORD);
if ( rank == NULL) goto write_ignore;
vgic_lock_rank(v, rank, flags);
if ( dabt.size == DABT_WORD )
- rank->ipriority[REG_RANK_INDEX(8, gicd_reg - GICD_IPRIORITYR,
- DABT_WORD)] = r;
+ ipriorityr = r;
else
- vgic_byte_write(&rank->ipriority[REG_RANK_INDEX(8,
- gicd_reg - GICD_IPRIORITYR, DABT_WORD)], r, gicd_reg);
+ {
+ ipriorityr = vgic_generate_ipriorityr(rank,
+ gicd_reg - GICD_IPRIORITYR);
+ vgic_byte_write(&ipriorityr, r, gicd_reg);
+ }
+ vgic_store_ipriorityr(rank, gicd_reg - GICD_IPRIORITYR, ipriorityr);
vgic_unlock_rank(v, rank, flags);
return 1;
+ }
case GICD_ICFGR: /* SGIs */
goto write_ignore_32;
@@ -516,14 +523,11 @@ static struct vcpu *vgic_v2_get_target_vcpu(struct vcpu *v, unsigned int irq)
static int vgic_v2_get_irq_priority(struct vcpu *v, unsigned int irq)
{
- int priority;
struct vgic_irq_rank *rank = vgic_rank_irq(v, irq);
ASSERT(spin_is_locked(&rank->lock));
- priority = vgic_byte_read(rank->ipriority[REG_RANK_INDEX(8,
- irq, DABT_WORD)], irq & 0x3);
- return priority;
+ return rank->priority[irq & INTERRUPT_RANK_MASK];
}
static int vgic_v2_vcpu_init(struct vcpu *v)
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index c013200..2787507 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -333,8 +333,8 @@ static int __vgic_v3_distr_common_mmio_read(const char *name, struct vcpu *v,
if ( rank == NULL ) goto read_as_zero;
vgic_lock_rank(v, rank, flags);
- *r = rank->ipriority[REG_RANK_INDEX(8, reg - GICD_IPRIORITYR,
- DABT_WORD)];
+ /* Recreate the IPRIORITYR register */
+ *r = vgic_generate_ipriorityr(rank, reg - GICD_IPRIORITYR);
if ( dabt.size == DABT_BYTE )
*r = vgic_byte_read(*r, reg);
vgic_unlock_rank(v, rank, flags);
@@ -430,18 +430,26 @@ static int __vgic_v3_distr_common_mmio_write(const char *name, struct vcpu *v,
return 0;
case GICD_IPRIORITYR ... GICD_IPRIORITYRN:
+ {
+ uint32_t ipriorityr;
+
if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
rank = vgic_rank_offset(v, 8, reg - GICD_IPRIORITYR, DABT_WORD);
- if ( rank == NULL ) goto write_ignore;
+ if ( rank == NULL) goto write_ignore;
+
vgic_lock_rank(v, rank, flags);
if ( dabt.size == DABT_WORD )
- rank->ipriority[REG_RANK_INDEX(8, reg - GICD_IPRIORITYR,
- DABT_WORD)] = r;
+ ipriorityr = r;
else
- vgic_byte_write(&rank->ipriority[REG_RANK_INDEX(8,
- reg - GICD_IPRIORITYR, DABT_WORD)], r, reg);
+ {
+ ipriorityr = vgic_generate_ipriorityr(rank, reg - GICD_IPRIORITYR);
+ vgic_byte_write(&ipriorityr, r, reg);
+ }
+ vgic_store_ipriorityr(rank, reg - GICD_IPRIORITYR, ipriorityr);
vgic_unlock_rank(v, rank, flags);
return 1;
+ }
+
case GICD_ICFGR: /* Restricted to configure SGIs */
goto write_ignore_32;
case GICD_ICFGR + 4 ... GICD_ICFGRN: /* PPI + SPIs */
@@ -1057,14 +1065,11 @@ static const struct mmio_handler_ops vgic_distr_mmio_handler = {
static int vgic_v3_get_irq_priority(struct vcpu *v, unsigned int irq)
{
- int priority;
struct vgic_irq_rank *rank = vgic_rank_irq(v, irq);
ASSERT(spin_is_locked(&rank->lock));
- priority = vgic_byte_read(rank->ipriority[REG_RANK_INDEX(8,
- irq, DABT_WORD)], irq & 0x3);
- return priority;
+ return rank->priority[irq & INTERRUPT_RANK_MASK];
}
static int vgic_v3_vcpu_init(struct vcpu *v)
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index a6835a8..50ad360 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -61,6 +61,52 @@ struct vgic_irq_rank *vgic_rank_irq(struct vcpu *v, unsigned int irq)
return vgic_get_rank(v, rank);
}
+#define NR_PRIORITY_PER_REG 4U
+#define NR_BIT_PER_PRIORITY 8U
+
+/*
+ * Generate the associated IPRIORITYR register based on an offset in the rank.
+ * Note the offset will be round down to match a real HW register.
+ */
+uint32_t vgic_generate_ipriorityr(struct vgic_irq_rank *rank,
+ unsigned int offset)
+{
+ uint32_t reg = 0;
+ unsigned int i;
+
+ ASSERT(spin_is_locked(&rank->lock));
+
+ offset &= INTERRUPT_RANK_MASK;
+ offset &= ~(NR_PRIORITY_PER_REG - 1);
+
+ for ( i = 0; i < NR_PRIORITY_PER_REG; i++, offset++ )
+ reg |= rank->priority[offset] << (i * NR_BIT_PER_PRIORITY);
+
+ return reg;
+}
+
+/*
+ * Store a IPRIORITYR register in a convenient way.
+ * Note the offset will be round down to match a real HW register.
+ */
+void vgic_store_ipriorityr(struct vgic_irq_rank *rank,
+ unsigned int offset,
+ uint32_t reg)
+{
+ unsigned int i;
+
+ ASSERT(spin_is_locked(&rank->lock));
+
+ offset &= INTERRUPT_RANK_MASK;
+ offset &= ~(NR_PRIORITY_PER_REG - 1);
+
+ for ( i = 0; i < NR_PRIORITY_PER_REG; i++, offset++ )
+ {
+ rank->priority[offset] = reg & ((1 << NR_BIT_PER_PRIORITY) - 1);
+ reg >>= NR_BIT_PER_PRIORITY;
+ }
+}
+
static void vgic_init_pending_irq(struct pending_irq *p, unsigned int virq)
{
INIT_LIST_HEAD(&p->inflight);
diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
index 354c0d4..ce9970e 100644
--- a/xen/include/asm-arm/vgic.h
+++ b/xen/include/asm-arm/vgic.h
@@ -82,12 +82,21 @@ struct pending_irq
struct list_head lr_queue;
};
+#define NR_INTERRUPT_PER_RANK 32
+#define INTERRUPT_RANK_MASK (NR_INTERRUPT_PER_RANK - 1)
+
/* Represents state corresponding to a block of 32 interrupts */
struct vgic_irq_rank {
spinlock_t lock; /* Covers access to all other members of this struct */
uint32_t ienable;
uint32_t icfg[2];
- uint32_t ipriority[8];
+
+ /*
+ * It's more convenient to store one priority per interrupt than
+ * the register IPRIORITYR itself
+ */
+ uint8_t priority[32];
+
union {
struct {
uint32_t itargets[8];
@@ -244,6 +253,13 @@ void vgic_v3_setup_hw(paddr_t dbase,
uint32_t rdist_stride);
#endif
+/* Helpers to handle the GICD_IPRIORITY register */
+uint32_t vgic_generate_ipriorityr(struct vgic_irq_rank *rank,
+ unsigned int offset);
+void vgic_store_ipriorityr(struct vgic_irq_rank *rank,
+ unsigned int offset,
+ uint32_t reg);
+
#endif /* __ASM_ARM_VGIC_H__ */
/*
--
2.1.4
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v1 6/8] xen/arm: vgic: Optimize the way to store the target vCPU in the rank
[not found] <1443192667-16112-1-git-send-email-julien.grall@citrix.com>
` (4 preceding siblings ...)
2015-09-25 14:51 ` [PATCH v1 5/8] xen/arm: vgic: Optimize the way to store GICD_IPRIORITYR in the rank Julien Grall
@ 2015-09-25 14:51 ` Julien Grall
2015-09-25 14:51 ` [PATCH v1 7/8] xen/arm: vgic: Introduce helpers to read/write/clear/set vGIC register Julien Grall
` (2 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Julien Grall @ 2015-09-25 14:51 UTC (permalink / raw)
To: xen-devel
Cc: Julien Grall, linux-kernel, ian.campbell, linux-arm-kernel,
stefano.stabellini
Xen is currently directly storing the value of register GICD_ITARGETSR
(for GICv2) and GICD_IROUTER (for GICv3) in the rank. This makes the
emulation of the registers access very simple but makes the code to get
the target vCPU for a given IRQ more complex.
While the target vCPU of an IRQ is retrieved everytime an IRQ is
injected to the guest, the access to the register occurs less often.
So the data structure should be optimized for the most common case
rather than the inverse.
This patch introduce the usage of an array to store the target vCPU for
every interrupt in the rank. This will make the code to get the target
very quick. The emulation code will now have to generate the GICD_ITARGETSR
and GICD_IROUTER register for read access and split it to store in a
convenient way.
Note that with these changes, any read to those registers will list only
the target vCPU used by Xen. This is fine because the GIC spec doesn't
require to return exactly the value written and it can be seen as if we
decide to implement the register read-only.
Finally take the opportunity to fix coding style in section we are
modifying.
Signed-off-by: Julien Grall <julien.grall@citrix.com>
---
The real reason is I'd like to drop vgic_byte_* helpers in favor of more
generic access helpers. Although it would make the code to retrieve the
priority more complex. So reworking the code to get the target vCPU
was the best solution.
Changes in v2:
- Patch added
---
xen/arch/arm/vgic-v2.c | 172 ++++++++++++++++++++++++++-------------------
xen/arch/arm/vgic-v3.c | 121 +++++++++++++++++--------------
xen/arch/arm/vgic.c | 28 ++++++--
xen/include/asm-arm/vgic.h | 13 ++--
4 files changed, 197 insertions(+), 137 deletions(-)
diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
index 23d1982..b6db64f 100644
--- a/xen/arch/arm/vgic-v2.c
+++ b/xen/arch/arm/vgic-v2.c
@@ -50,6 +50,90 @@ void vgic_v2_setup_hw(paddr_t dbase, paddr_t cbase, paddr_t vbase)
vgic_v2_hw.vbase = vbase;
}
+#define NR_TARGET_PER_REG 4U
+#define NR_BIT_PER_TARGET 8U
+
+/*
+ * Generate the associated ITARGETSR register based on the offset from
+ * ITARGETSR0. Only one vCPU will be listed for a given IRQ.
+ *
+ * Note the offset will be round down to match a real HW register.
+ */
+static uint32_t vgic_generate_itargetsr(struct vgic_irq_rank *rank,
+ unsigned int offset)
+{
+ uint32_t reg = 0;
+ unsigned int i;
+
+ ASSERT(spin_is_locked(&rank->lock));
+
+ offset &= INTERRUPT_RANK_MASK;
+ offset &= ~(NR_TARGET_PER_REG - 1);
+
+ for ( i = 0; i < NR_TARGET_PER_REG; i++, offset++ )
+ reg |= (1 << rank->vcpu[offset]) << (i * NR_BIT_PER_TARGET);
+
+ return reg;
+}
+
+/*
+ * Store a ITARGETSR register in a convenient way and migrate the IRQ
+ * if necessary.
+ * Note the offset will be round down to match a real HW register.
+ */
+static void vgic_store_itargetsr(struct domain *d, struct vgic_irq_rank *rank,
+ unsigned int offset, uint32_t itargetsr)
+{
+ unsigned int i, rank_idx;
+
+ ASSERT(spin_is_locked(&rank->lock));
+
+ rank_idx = offset / NR_INTERRUPT_PER_RANK;
+
+ offset &= INTERRUPT_RANK_MASK;
+ offset &= ~(NR_TARGET_PER_REG - 1);
+
+ for ( i = 0; i < NR_TARGET_PER_REG; i++, offset++ )
+ {
+ unsigned int new_target, old_target;
+
+ /*
+ * SPI are using the 1-N model (see 1.4.3 in ARM IHI 0048B).
+ * While the interrupt could be set pending to all the vCPUs in
+ * target list, it's not guarantee by the spec.
+ * For simplicity, always route the IRQ to the first interrupt
+ * in the target list
+ */
+ new_target = ffs(itargetsr & ((1 << NR_BIT_PER_TARGET) - 1));
+ old_target = rank->vcpu[offset];
+
+ itargetsr >>= NR_BIT_PER_TARGET;
+
+ /*
+ * Ignore the write request for this interrupt if the new target
+ * is invalid.
+ */
+ if ( !new_target || (new_target > d->max_vcpus) )
+ continue;
+
+ /* The vCPU ID always start from 0 */
+ new_target--;
+
+ /* Only migrate the IRQ if the target vCPU has changed */
+ if ( new_target != old_target )
+ {
+ unsigned int irq = rank_idx * NR_INTERRUPT_PER_RANK + offset;
+
+ vgic_migrate_irq(d->vcpu[old_target],
+ d->vcpu[new_target],
+ irq);
+ }
+
+ rank->vcpu[offset] = new_target;
+ }
+
+}
+
static int vgic_v2_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
register_t *r)
{
@@ -126,8 +210,8 @@ static int vgic_v2_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
rank = vgic_rank_offset(v, 8, gicd_reg - GICD_ITARGETSR, DABT_WORD);
if ( rank == NULL) goto read_as_zero;
vgic_lock_rank(v, rank, flags);
- *r = rank->v2.itargets[REG_RANK_INDEX(8, gicd_reg - GICD_ITARGETSR,
- DABT_WORD)];
+ /* The recreate the ITARGETSR register */
+ *r = vgic_generate_itargetsr(rank, gicd_reg - GICD_ITARGETSR);
if ( dabt.size == DABT_BYTE )
*r = vgic_byte_read(*r, gicd_reg);
vgic_unlock_rank(v, rank, flags);
@@ -345,56 +429,23 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info,
case GICD_ITARGETSR + 8 ... GICD_ITARGETSRN:
{
+ uint32_t itargetsr;
+
/* unsigned long needed for find_next_bit */
- unsigned long target;
- int i;
if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
rank = vgic_rank_offset(v, 8, gicd_reg - GICD_ITARGETSR, DABT_WORD);
if ( rank == NULL) goto write_ignore;
- /* 8-bit vcpu mask for this domain */
- BUG_ON(v->domain->max_vcpus > 8);
- target = (1 << v->domain->max_vcpus) - 1;
- if ( dabt.size == 2 )
- target = target | (target << 8) | (target << 16) | (target << 24);
- else
- target = (target << (8 * (gicd_reg & 0x3)));
- target &= r;
- /* ignore zero writes */
- if ( !target )
- goto write_ignore;
- /* For word reads ignore writes where any single byte is zero */
- if ( dabt.size == 2 &&
- !((target & 0xff) && (target & (0xff << 8)) &&
- (target & (0xff << 16)) && (target & (0xff << 24))))
- goto write_ignore;
vgic_lock_rank(v, rank, flags);
- i = 0;
- while ( (i = find_next_bit(&target, 32, i)) < 32 )
- {
- unsigned int irq, new_target, old_target;
- unsigned long old_target_mask;
- struct vcpu *v_target, *v_old;
-
- new_target = i % 8;
- old_target_mask = vgic_byte_read(rank->v2.itargets[REG_RANK_INDEX(8,
- gicd_reg - GICD_ITARGETSR, DABT_WORD)], i/8);
- old_target = find_first_bit(&old_target_mask, 8);
-
- if ( new_target != old_target )
- {
- irq = gicd_reg - GICD_ITARGETSR + (i / 8);
- v_target = v->domain->vcpu[new_target];
- v_old = v->domain->vcpu[old_target];
- vgic_migrate_irq(v_old, v_target, irq);
- }
- i += 8 - new_target;
- }
if ( dabt.size == DABT_WORD )
- rank->v2.itargets[REG_RANK_INDEX(8, gicd_reg - GICD_ITARGETSR,
- DABT_WORD)] = target;
+ itargetsr = r;
else
- vgic_byte_write(&rank->v2.itargets[REG_RANK_INDEX(8,
- gicd_reg - GICD_ITARGETSR, DABT_WORD)], target, gicd_reg);
+ {
+ itargetsr = vgic_generate_itargetsr(rank,
+ gicd_reg - GICD_ITARGETSR);
+ vgic_byte_write(&itargetsr, r, gicd_reg);
+ }
+ vgic_store_itargetsr(v->domain, rank, gicd_reg - GICD_ITARGETSR,
+ itargetsr);
vgic_unlock_rank(v, rank, flags);
return 1;
}
@@ -504,21 +555,11 @@ static const struct mmio_handler_ops vgic_v2_distr_mmio_handler = {
static struct vcpu *vgic_v2_get_target_vcpu(struct vcpu *v, unsigned int irq)
{
- unsigned long target;
- struct vcpu *v_target;
struct vgic_irq_rank *rank = vgic_rank_irq(v, irq);
- ASSERT(spin_is_locked(&rank->lock));
- target = vgic_byte_read(rank->v2.itargets[REG_RANK_INDEX(8,
- irq, DABT_WORD)], irq & 0x3);
+ ASSERT(spin_is_locked(&rank->lock));
- /* 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
- * because it would be too slow to do otherwise. */
- target = find_first_bit(&target, 8);
- ASSERT(target >= 0 && target < v->domain->max_vcpus);
- v_target = v->domain->vcpu[target];
- return v_target;
+ return v->domain->vcpu[rank->vcpu[irq & INTERRUPT_RANK_MASK]];
}
static int vgic_v2_get_irq_priority(struct vcpu *v, unsigned int irq)
@@ -532,22 +573,14 @@ static int vgic_v2_get_irq_priority(struct vcpu *v, unsigned int irq)
static int vgic_v2_vcpu_init(struct vcpu *v)
{
- int i;
-
- /* For SGI and PPI the target is always this CPU */
- for ( i = 0 ; i < 8 ; i++ )
- v->arch.vgic.private_irqs->v2.itargets[i] =
- (1<<(v->vcpu_id+0))
- | (1<<(v->vcpu_id+8))
- | (1<<(v->vcpu_id+16))
- | (1<<(v->vcpu_id+24));
+ /* Nothing specific to initialize for this driver */
return 0;
}
static int vgic_v2_domain_init(struct domain *d)
{
- int i, ret;
+ int ret;
/*
* The hardware domain gets the hardware address.
@@ -586,11 +619,6 @@ static int vgic_v2_domain_init(struct domain *d)
if ( ret )
return ret;
- /* By default deliver to CPU0 */
- for ( i = 0; i < DOMAIN_NR_RANKS(d); i++ )
- memset(d->arch.vgic.shared_irqs[i].v2.itargets, 0x1,
- sizeof(d->arch.vgic.shared_irqs[i].v2.itargets));
-
register_mmio_handler(d, &vgic_v2_distr_mmio_handler, d->arch.vgic.dbase,
PAGE_SIZE);
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index 2787507..b247043 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -89,18 +89,77 @@ static struct vcpu *vgic_v3_irouter_to_vcpu(struct domain *d, uint64_t irouter)
return d->vcpu[vcpu_id];
}
+#define NR_BYTE_PER_IROUTER 8U
+
+/*
+ * Generate the associated IROUTER register based on the offset from IROUTERO.
+ * Only one vCPU will be listed for a given IRQ.
+ *
+ * Note the offset will be round down to match a real HW register
+ */
+static uint64_t vgic_generate_irouter(struct vgic_irq_rank *rank,
+ unsigned int offset)
+{
+ ASSERT(spin_is_locked(&rank->lock));
+
+ /* There is exactly 1 IRQ per IROUTER */
+ offset /= NR_BYTE_PER_IROUTER;
+
+ /* Get the index in the rank */
+ offset &= INTERRUPT_RANK_MASK;
+
+ return vcpuid_to_vaffinity(rank->vcpu[offset]);
+}
+
+/*
+ * Store a IROUTER register in a convenient way and migrate the IRQ
+ * if necessary.
+ * Note the offset will be round down to match a real HW register.
+ */
+static void vgic_store_irouter(struct domain *d, struct vgic_irq_rank *rank,
+ unsigned int offset, uint64_t irouter)
+{
+ struct vcpu *new_vcpu, *old_vcpu;
+ unsigned int rank_idx, irq;
+
+
+ /* There is 1 IRQ per IROUTER */
+ irq = offset / NR_BYTE_PER_IROUTER;
+
+ rank_idx = irq / NR_INTERRUPT_PER_RANK;
+
+ /* Get the index in the rank */
+ offset &= irq & INTERRUPT_RANK_MASK;
+
+ new_vcpu = vgic_v3_irouter_to_vcpu(d, irouter);
+ old_vcpu = d->vcpu[rank->vcpu[offset]];
+
+ /*
+ * From the spec (see 8.9.13 in IHI 0069A), any write with an
+ * invalid vCPU will lead to ignore the interrupt.
+ *
+ * But the current code to inject an IRQ is not able to cope with
+ * invalid vCPU. So for now, just ignore the write.
+ *
+ * TODO: Respect the spec
+ */
+ if ( !new_vcpu )
+ return;
+
+ /* Only migrate the IRQ if the target vCPU has changed */
+ if ( new_vcpu != old_vcpu )
+ vgic_migrate_irq(old_vcpu, new_vcpu, irq);
+
+ rank->vcpu[offset] = new_vcpu->vcpu_id;
+}
+
static struct vcpu *vgic_v3_get_target_vcpu(struct vcpu *v, unsigned int irq)
{
- struct vcpu *v_target;
struct vgic_irq_rank *rank = vgic_rank_irq(v, irq);
ASSERT(spin_is_locked(&rank->lock));
- v_target = vgic_v3_irouter_to_vcpu(v->domain, rank->v3.irouter[irq % 32]);
-
- ASSERT(v_target != NULL);
-
- return v_target;
+ return v->domain->vcpu[rank->vcpu[irq & INTERRUPT_RANK_MASK]];
}
static int __vgic_v3_rdistr_rd_mmio_read(struct vcpu *v, mmio_info_t *info,
@@ -756,8 +815,7 @@ static int vgic_v3_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
DABT_DOUBLE_WORD);
if ( rank == NULL ) goto read_as_zero;
vgic_lock_rank(v, rank, flags);
- *r = rank->v3.irouter[REG_RANK_INDEX(64,
- (gicd_reg - GICD_IROUTER), DABT_DOUBLE_WORD)];
+ *r = vgic_generate_irouter(rank, gicd_reg - GICD_IROUTER);
vgic_unlock_rank(v, rank, flags);
return 1;
case GICD_NSACR ... GICD_NSACRN:
@@ -842,8 +900,6 @@ static int vgic_v3_distr_mmio_write(struct vcpu *v, mmio_info_t *info,
struct hsr_dabt dabt = info->dabt;
struct vgic_irq_rank *rank;
unsigned long flags;
- uint64_t new_irouter, old_irouter;
- struct vcpu *old_vcpu, *new_vcpu;
int gicd_reg = (int)(info->gpa - v->domain->arch.vgic.dbase);
perfc_incr(vgicd_writes);
@@ -911,32 +967,8 @@ static int vgic_v3_distr_mmio_write(struct vcpu *v, mmio_info_t *info,
rank = vgic_rank_offset(v, 64, gicd_reg - GICD_IROUTER,
DABT_DOUBLE_WORD);
if ( rank == NULL ) goto write_ignore;
- new_irouter = r;
vgic_lock_rank(v, rank, flags);
-
- old_irouter = rank->v3.irouter[REG_RANK_INDEX(64,
- (gicd_reg - GICD_IROUTER),
- DABT_DOUBLE_WORD)];
- old_vcpu = vgic_v3_irouter_to_vcpu(v->domain, old_irouter);
- new_vcpu = vgic_v3_irouter_to_vcpu(v->domain, new_irouter);
-
- if ( !new_vcpu )
- {
- printk(XENLOG_G_DEBUG
- "%pv: vGICD: wrong irouter at offset %#08x val %#"PRIregister,
- v, gicd_reg, r);
- vgic_unlock_rank(v, rank, flags);
- /*
- * TODO: Don't inject a fault to the guest when the MPIDR is
- * not valid. From the spec, the interrupt should be
- * ignored.
- */
- return 0;
- }
- rank->v3.irouter[REG_RANK_INDEX(64, (gicd_reg - GICD_IROUTER),
- DABT_DOUBLE_WORD)] = new_irouter;
- if ( old_vcpu != new_vcpu )
- vgic_migrate_irq(old_vcpu, new_vcpu, (gicd_reg - GICD_IROUTER)/8);
+ vgic_store_irouter(v->domain, rank, gicd_reg - GICD_IROUTER, r);
vgic_unlock_rank(v, rank, flags);
return 1;
case GICD_NSACR ... GICD_NSACRN:
@@ -1075,7 +1107,6 @@ static int vgic_v3_get_irq_priority(struct vcpu *v, unsigned int irq)
static int vgic_v3_vcpu_init(struct vcpu *v)
{
int i;
- uint64_t affinity;
paddr_t rdist_base;
struct vgic_rdist_region *region;
unsigned int last_cpu;
@@ -1084,15 +1115,6 @@ static int vgic_v3_vcpu_init(struct vcpu *v)
struct domain *d = v->domain;
uint32_t rdist_stride = d->arch.vgic.rdist_stride;
- /* For SGI and PPI the target is always this CPU */
- affinity = (MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 3) << 32 |
- MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 2) << 16 |
- MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 1) << 8 |
- MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 0));
-
- for ( i = 0 ; i < 32 ; i++ )
- v->arch.vgic.private_irqs->v3.irouter[i] = affinity;
-
/*
* Find the region where the re-distributor lives. For this purpose,
* we look one region ahead as we have only the first CPU in hand.
@@ -1137,7 +1159,7 @@ static int vgic_v3_vcpu_init(struct vcpu *v)
static int vgic_v3_domain_init(struct domain *d)
{
- int i, idx;
+ int i;
/*
* Domain 0 gets the hardware address.
@@ -1189,13 +1211,6 @@ static int vgic_v3_domain_init(struct domain *d)
d->arch.vgic.rdist_regions[0].first_cpu = 0;
}
- /* By default deliver to CPU0 */
- for ( i = 0; i < DOMAIN_NR_RANKS(d); i++ )
- {
- for ( idx = 0; idx < 32; idx++ )
- d->arch.vgic.shared_irqs[i].v3.irouter[idx] = 0;
- }
-
/* Register mmio handle for the Distributor */
register_mmio_handler(d, &vgic_distr_mmio_handler, d->arch.vgic.dbase,
SZ_64K);
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 50ad360..cce1bc2 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -65,7 +65,8 @@ struct vgic_irq_rank *vgic_rank_irq(struct vcpu *v, unsigned int irq)
#define NR_BIT_PER_PRIORITY 8U
/*
- * Generate the associated IPRIORITYR register based on an offset in the rank.
+ * Generate the associated IPRIORITYR register based on the offset from
+ * IPRIORITYR0.
* Note the offset will be round down to match a real HW register.
*/
uint32_t vgic_generate_ipriorityr(struct vgic_irq_rank *rank,
@@ -114,6 +115,23 @@ static void vgic_init_pending_irq(struct pending_irq *p, unsigned int virq)
p->irq = virq;
}
+static void vgic_rank_init(struct vgic_irq_rank *rank, unsigned int vcpu)
+{
+ unsigned int i;
+
+ /*
+ * Make sure that the type chosen to store the target is able to
+ * store an VCPU ID between 0 and the maximum of virtual CPUs
+ * supported.
+ */
+ BUILD_BUG_ON((1 << (sizeof(rank->vcpu[0]) * 8)) < MAX_VIRT_CPUS);
+
+ spin_lock_init(&rank->lock);
+
+ for ( i = 0; i < NR_INTERRUPT_PER_RANK; i++ )
+ rank->vcpu[i] = vcpu;
+}
+
int domain_vgic_init(struct domain *d, unsigned int nr_spis)
{
int i;
@@ -160,8 +178,9 @@ int domain_vgic_init(struct domain *d, unsigned int nr_spis)
for (i=0; i<d->arch.vgic.nr_spis; i++)
vgic_init_pending_irq(&d->arch.vgic.pending_irqs[i], i + 32);
- for (i=0; i<DOMAIN_NR_RANKS(d); i++)
- spin_lock_init(&d->arch.vgic.shared_irqs[i].lock);
+ /* SPIs are routed to VCPU0 by default */
+ for ( i = 0; i < DOMAIN_NR_RANKS(d); i++ )
+ vgic_rank_init(&d->arch.vgic.shared_irqs[i], 0);
ret = d->arch.vgic.handler->domain_init(d);
if ( ret )
@@ -215,7 +234,8 @@ int vcpu_vgic_init(struct vcpu *v)
if ( v->arch.vgic.private_irqs == NULL )
return -ENOMEM;
- spin_lock_init(&v->arch.vgic.private_irqs->lock);
+ /* SGIs/PPIs are routed to this VCPU by default */
+ vgic_rank_init(v->arch.vgic.private_irqs, v->vcpu_id);
v->domain->arch.vgic.handler->vcpu_init(v);
diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
index ce9970e..0cb8029 100644
--- a/xen/include/asm-arm/vgic.h
+++ b/xen/include/asm-arm/vgic.h
@@ -97,14 +97,11 @@ struct vgic_irq_rank {
*/
uint8_t priority[32];
- union {
- struct {
- uint32_t itargets[8];
- }v2;
- struct {
- uint64_t irouter[32];
- }v3;
- };
+ /*
+ * It's more convenient to store a target VCPU per interrupt
+ * than the register ITARGETSR/IROUTER itself
+ */
+ uint8_t vcpu[32];
};
struct sgi_target {
--
2.1.4
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v1 7/8] xen/arm: vgic: Introduce helpers to read/write/clear/set vGIC register ...
[not found] <1443192667-16112-1-git-send-email-julien.grall@citrix.com>
` (5 preceding siblings ...)
2015-09-25 14:51 ` [PATCH v1 6/8] xen/arm: vgic: Optimize the way to store the target vCPU " Julien Grall
@ 2015-09-25 14:51 ` Julien Grall
2015-09-25 14:51 ` [PATCH v1 8/8] xen/arm: vgic-v3: Support 32-bit access for 64-bit registers Julien Grall
2015-09-25 14:52 ` [PATCH v1 0/8] xen/arm: vgic: Support 32-bit access for 64-bit register Julien Grall
8 siblings, 0 replies; 11+ messages in thread
From: Julien Grall @ 2015-09-25 14:51 UTC (permalink / raw)
To: xen-devel
Cc: Julien Grall, linux-kernel, ian.campbell, linux-arm-kernel,
stefano.stabellini
and use them in the vGIC emulation.
The GIC registers may support different access sizes. Rather than open
coding the access for every registers, provide a set of helpers to access
them.
The caller will have to call vgic_regN_* where N is the size of the
emulated registers.
The new helpers supports any access size and expect the caller to
validate the access size supported by the emulated registers.
Finally, take the opportunity to fix the coding style in section we are
modifying.
Signed-off-by: Julien Grall <julien.grall@citrix.com>
---
I've only done quick tests. I sent them in order to help Vijay
supported multiple access size.
Changes in v2:
- Make use of the new helpers in the vGICv2 code
- Drop vgic_byte_*
- Use unsigned long rather than uint64_t to optimize the code
on ARM. (~about 40% less instruction per helpers).
---
xen/arch/arm/vgic-v2.c | 99 ++++++++++++++++++++++++----------------
xen/arch/arm/vgic-v3.c | 111 +++++++++++++++++++++++++++++----------------
xen/include/asm-arm/vgic.h | 111 +++++++++++++++++++++++++++++++++++++++++----
3 files changed, 234 insertions(+), 87 deletions(-)
diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
index b6db64f..cdc0825 100644
--- a/xen/arch/arm/vgic-v2.c
+++ b/xen/arch/arm/vgic-v2.c
@@ -149,24 +149,31 @@ static int vgic_v2_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
case GICD_CTLR:
if ( dabt.size != DABT_WORD ) goto bad_width;
vgic_lock(v);
- *r = v->domain->arch.vgic.ctlr;
+ *r = vgic_reg32_read(v->domain->arch.vgic.ctlr, info);
vgic_unlock(v);
return 1;
case GICD_TYPER:
+ {
+ uint32_t typer;
+
if ( dabt.size != DABT_WORD ) goto bad_width;
/* No secure world support for guests. */
vgic_lock(v);
- *r = ( ((v->domain->max_vcpus - 1) << GICD_TYPE_CPUS_SHIFT) )
+ typer = ((v->domain->max_vcpus - 1) << GICD_TYPE_CPUS_SHIFT)
| DIV_ROUND_UP(v->domain->arch.vgic.nr_spis, 32);
vgic_unlock(v);
+
+ *r = vgic_reg32_read(typer, info);
+
return 1;
+ }
case GICD_IIDR:
if ( dabt.size != DABT_WORD ) goto bad_width;
/*
* XXX Do we need a JEP106 manufacturer ID?
* Just use the physical h/w value for now
*/
- *r = 0x0000043b;
+ *r = vgic_reg32_read(0x0000043b, info);
return 1;
/* Implementation defined -- read as zero */
@@ -182,7 +189,7 @@ static int vgic_v2_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ISENABLER, DABT_WORD);
if ( rank == NULL) goto read_as_zero;
vgic_lock_rank(v, rank, flags);
- *r = rank->ienable;
+ *r = vgic_reg32_read(rank->ienable, info);
vgic_unlock_rank(v, rank, flags);
return 1;
@@ -191,7 +198,7 @@ static int vgic_v2_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ICENABLER, DABT_WORD);
if ( rank == NULL) goto read_as_zero;
vgic_lock_rank(v, rank, flags);
- *r = rank->ienable;
+ *r = vgic_reg32_read(rank->ienable, info);
vgic_unlock_rank(v, rank, flags);
return 1;
@@ -206,38 +213,55 @@ static int vgic_v2_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
goto read_as_zero;
case GICD_ITARGETSR ... GICD_ITARGETSRN:
+ {
+ uint32_t itargetsr;
+
if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
rank = vgic_rank_offset(v, 8, gicd_reg - GICD_ITARGETSR, DABT_WORD);
if ( rank == NULL) goto read_as_zero;
vgic_lock_rank(v, rank, flags);
/* The recreate the ITARGETSR register */
- *r = vgic_generate_itargetsr(rank, gicd_reg - GICD_ITARGETSR);
- if ( dabt.size == DABT_BYTE )
- *r = vgic_byte_read(*r, gicd_reg);
+ itargetsr = vgic_generate_itargetsr(rank,
+ gicd_reg - GICD_ITARGETSR);
vgic_unlock_rank(v, rank, flags);
+ *r = vgic_reg32_read(itargetsr, info);
+
return 1;
+ }
case GICD_IPRIORITYR ... GICD_IPRIORITYRN:
+ {
+ uint32_t ipriorityr;
+
if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
rank = vgic_rank_offset(v, 8, gicd_reg - GICD_IPRIORITYR, DABT_WORD);
if ( rank == NULL) goto read_as_zero;
vgic_lock_rank(v, rank, flags);
/* Recreate the IPRIORITYR register */
- *r = vgic_generate_ipriorityr(rank, gicd_reg - GICD_IPRIORITYR);
- if ( dabt.size == DABT_BYTE )
- *r = vgic_byte_read(*r, gicd_reg);
+ ipriorityr = vgic_generate_ipriorityr(rank,
+ gicd_reg - GICD_IPRIORITYR);
vgic_unlock_rank(v, rank, flags);
+ *r = vgic_reg32_read(ipriorityr, info);
+
return 1;
+ }
case GICD_ICFGR ... GICD_ICFGRN:
+ {
+ uint32_t icfgr;
+
if ( dabt.size != DABT_WORD ) goto bad_width;
rank = vgic_rank_offset(v, 2, gicd_reg - GICD_ICFGR, DABT_WORD);
if ( rank == NULL) goto read_as_zero;
vgic_lock_rank(v, rank, flags);
- *r = rank->icfg[REG_RANK_INDEX(2, gicd_reg - GICD_ICFGR, DABT_WORD)];
+ icfgr = rank->icfg[REG_RANK_INDEX(2, gicd_reg - GICD_ICFGR, DABT_WORD)];
vgic_unlock_rank(v, rank, flags);
+
+ *r = vgic_reg32_read(icfgr, info);
+
return 1;
+ }
case GICD_NSACR ... GICD_NSACRN:
/* We do not implement security extensions for guests, read zero */
@@ -347,7 +371,8 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info,
if ( dabt.size != DABT_WORD ) goto bad_width;
/* Ignore all but the enable bit */
vgic_lock(v);
- v->domain->arch.vgic.ctlr = r & GICD_CTL_ENABLE;
+ vgic_reg32_write(&v->domain->arch.vgic.ctlr, r, info);
+ v->domain->arch.vgic.ctlr &= GICD_CTL_ENABLE;
vgic_unlock(v);
return 1;
@@ -371,11 +396,13 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info,
if ( rank == NULL) goto write_ignore;
vgic_lock_rank(v, rank, flags);
tr = rank->ienable;
- rank->ienable |= r;
- /* The virtual irq is derived from register offset.
+ vgic_reg32_setbit(&rank->ienable, r, info);
+ /*
+ * The virtual irq is derived from register offset.
* The register difference is word difference. So divide by 2(DABT_WORD)
- * to get Virtual irq number */
- vgic_enable_irqs(v, r & (~tr),
+ * to get Virtual irq number
+ */
+ vgic_enable_irqs(v, (rank->ienable) & (~tr),
(gicd_reg - GICD_ISENABLER) >> DABT_WORD);
vgic_unlock_rank(v, rank, flags);
return 1;
@@ -386,12 +413,14 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info,
if ( rank == NULL) goto write_ignore;
vgic_lock_rank(v, rank, flags);
tr = rank->ienable;
- rank->ienable &= ~r;
- /* The virtual irq is derived from register offset.
+ vgic_reg32_clearbit(&rank->ienable, r, info);
+ /*
+ * The virtual irq is derived from register offset.
* The register difference is word difference. So divide by 2(DABT_WORD)
- * to get Virtual irq number */
- vgic_disable_irqs(v, r & tr,
- (gicd_reg - GICD_ICENABLER) >> DABT_WORD);
+ * to get Virtual irq number
+ */
+ vgic_disable_irqs(v, (~rank->ienable) & tr,
+ (gicd_reg - GICD_ICENABLER) >> DABT_WORD);
vgic_unlock_rank(v, rank, flags);
return 1;
@@ -436,14 +465,9 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info,
rank = vgic_rank_offset(v, 8, gicd_reg - GICD_ITARGETSR, DABT_WORD);
if ( rank == NULL) goto write_ignore;
vgic_lock_rank(v, rank, flags);
- if ( dabt.size == DABT_WORD )
- itargetsr = r;
- else
- {
- itargetsr = vgic_generate_itargetsr(rank,
- gicd_reg - GICD_ITARGETSR);
- vgic_byte_write(&itargetsr, r, gicd_reg);
- }
+ itargetsr = vgic_generate_itargetsr(rank,
+ gicd_reg - GICD_ITARGETSR);
+ vgic_reg32_write(&itargetsr, r, info);
vgic_store_itargetsr(v->domain, rank, gicd_reg - GICD_ITARGETSR,
itargetsr);
vgic_unlock_rank(v, rank, flags);
@@ -458,14 +482,9 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info,
rank = vgic_rank_offset(v, 8, gicd_reg - GICD_IPRIORITYR, DABT_WORD);
if ( rank == NULL) goto write_ignore;
vgic_lock_rank(v, rank, flags);
- if ( dabt.size == DABT_WORD )
- ipriorityr = r;
- else
- {
- ipriorityr = vgic_generate_ipriorityr(rank,
- gicd_reg - GICD_IPRIORITYR);
- vgic_byte_write(&ipriorityr, r, gicd_reg);
- }
+ ipriorityr = vgic_generate_ipriorityr(rank,
+ gicd_reg - GICD_IPRIORITYR);
+ vgic_reg32_write(&ipriorityr, r, info);
vgic_store_ipriorityr(rank, gicd_reg - GICD_IPRIORITYR, ipriorityr);
vgic_unlock_rank(v, rank, flags);
return 1;
@@ -481,7 +500,9 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info,
rank = vgic_rank_offset(v, 2, gicd_reg - GICD_ICFGR, DABT_WORD);
if ( rank == NULL) goto write_ignore;
vgic_lock_rank(v, rank, flags);
- rank->icfg[REG_RANK_INDEX(2, gicd_reg - GICD_ICFGR, DABT_WORD)] = r;
+ vgic_reg32_write(&rank->icfg[REG_RANK_INDEX(2, gicd_reg - GICD_ICFGR,
+ DABT_WORD)],
+ r, info);
vgic_unlock_rank(v, rank, flags);
return 1;
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index b247043..8da695f 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -167,7 +167,6 @@ static int __vgic_v3_rdistr_rd_mmio_read(struct vcpu *v, mmio_info_t *info,
register_t *r)
{
struct hsr_dabt dabt = info->dabt;
- uint64_t aff;
switch ( gicr_reg )
{
@@ -176,21 +175,27 @@ static int __vgic_v3_rdistr_rd_mmio_read(struct vcpu *v, mmio_info_t *info,
goto read_as_zero_32;
case GICR_IIDR:
if ( dabt.size != DABT_WORD ) goto bad_width;
- *r = GICV3_GICR_IIDR_VAL;
+ *r = vgic_reg32_read(GICV3_GICR_IIDR_VAL, info);
return 1;
case GICR_TYPER:
+ {
+ uint64_t typer, aff;
+
if ( dabt.size != DABT_DOUBLE_WORD ) goto bad_width;
/* TBD: Update processor id in [23:8] when ITS support is added */
aff = (MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 3) << 56 |
MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 2) << 48 |
MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 1) << 40 |
MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 0) << 32);
- *r = aff;
+ typer = aff;
if ( v->arch.vgic.flags & VGIC_V3_RDIST_LAST )
- *r |= GICR_TYPER_LAST;
+ typer |= GICR_TYPER_LAST;
+
+ *r = vgic_reg64_read(typer, info);
return 1;
+ }
case GICR_STATUSR:
/* Not implemented */
goto read_as_zero_32;
@@ -219,7 +224,7 @@ static int __vgic_v3_rdistr_rd_mmio_read(struct vcpu *v, mmio_info_t *info,
case GICR_SYNCR:
if ( dabt.size != DABT_WORD ) goto bad_width;
/* RO . But when read it always returns busy bito bit[0] */
- *r = GICR_SYNCR_NOT_BUSY;
+ *r = vgic_reg32_read(GICR_SYNCR_NOT_BUSY, info);
return 1;
case GICR_MOVLPIR:
/* WO Read as zero */
@@ -229,22 +234,22 @@ static int __vgic_v3_rdistr_rd_mmio_read(struct vcpu *v, mmio_info_t *info,
goto read_as_zero_64;
case GICR_PIDR0:
if ( dabt.size != DABT_WORD ) goto bad_width;
- *r = GICV3_GICR_PIDR0;
+ *r = vgic_reg32_read(GICV3_GICR_PIDR0, info);
return 1;
case GICR_PIDR1:
if ( dabt.size != DABT_WORD ) goto bad_width;
- *r = GICV3_GICR_PIDR1;
+ *r = vgic_reg32_read(GICV3_GICR_PIDR1, info);
return 1;
case GICR_PIDR2:
if ( dabt.size != DABT_WORD ) goto bad_width;
- *r = GICV3_GICR_PIDR2;
+ *r = vgic_reg32_read(GICV3_GICR_PIDR2, info);
return 1;
case GICR_PIDR3:
/* Manufacture/customer defined */
goto read_as_zero_32;
case GICR_PIDR4:
if ( dabt.size != DABT_WORD ) goto bad_width;
- *r = GICV3_GICR_PIDR4;
+ *r = vgic_reg32_read(GICV3_GICR_PIDR4, info);
return 1;
case GICR_PIDR5 ... GICR_PIDR7:
/* Reserved0 */
@@ -365,7 +370,7 @@ static int __vgic_v3_distr_common_mmio_read(const char *name, struct vcpu *v,
rank = vgic_rank_offset(v, 1, reg - GICD_ISENABLER, DABT_WORD);
if ( rank == NULL ) goto read_as_zero;
vgic_lock_rank(v, rank, flags);
- *r = rank->ienable;
+ *r = vgic_reg32_read(rank->ienable, info);
vgic_unlock_rank(v, rank, flags);
return 1;
case GICD_ICENABLER ... GICD_ICENABLERN:
@@ -373,7 +378,7 @@ static int __vgic_v3_distr_common_mmio_read(const char *name, struct vcpu *v,
rank = vgic_rank_offset(v, 1, reg - GICD_ICENABLER, DABT_WORD);
if ( rank == NULL ) goto read_as_zero;
vgic_lock_rank(v, rank, flags);
- *r = rank->ienable;
+ *r = vgic_reg32_read(rank->ienable, info);
vgic_unlock_rank(v, rank, flags);
return 1;
/* Read the pending status of an IRQ via GICD/GICR is not supported */
@@ -387,25 +392,38 @@ static int __vgic_v3_distr_common_mmio_read(const char *name, struct vcpu *v,
goto read_as_zero;
case GICD_IPRIORITYR ... GICD_IPRIORITYRN:
+ {
+ uint32_t ipriorityr;
+
if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
rank = vgic_rank_offset(v, 8, reg - GICD_IPRIORITYR, DABT_WORD);
if ( rank == NULL ) goto read_as_zero;
vgic_lock_rank(v, rank, flags);
/* Recreate the IPRIORITYR register */
- *r = vgic_generate_ipriorityr(rank, reg - GICD_IPRIORITYR);
- if ( dabt.size == DABT_BYTE )
- *r = vgic_byte_read(*r, reg);
+ ipriorityr = vgic_generate_ipriorityr(rank, reg - GICD_IPRIORITYR);
vgic_unlock_rank(v, rank, flags);
+
+ *r = vgic_reg32_read(ipriorityr, info);
+
return 1;
+ }
+
case GICD_ICFGR ... GICD_ICFGRN:
+ {
+ uint32_t icfgr;
+
if ( dabt.size != DABT_WORD ) goto bad_width;
rank = vgic_rank_offset(v, 2, reg - GICD_ICFGR, DABT_WORD);
if ( rank == NULL ) goto read_as_zero;
vgic_lock_rank(v, rank, flags);
- *r = rank->icfg[REG_RANK_INDEX(2, reg - GICD_ICFGR, DABT_WORD)];
+ icfgr = rank->icfg[REG_RANK_INDEX(2, reg - GICD_ICFGR, DABT_WORD)];
vgic_unlock_rank(v, rank, flags);
+
+ *r = vgic_reg32_read(icfgr, info);
+
return 1;
+ }
default:
printk(XENLOG_G_ERR
"%pv: %s: unhandled read r%d offset %#08x\n",
@@ -444,9 +462,10 @@ static int __vgic_v3_distr_common_mmio_write(const char *name, struct vcpu *v,
if ( rank == NULL ) goto write_ignore;
vgic_lock_rank(v, rank, flags);
tr = rank->ienable;
- rank->ienable |= r;
+ vgic_reg32_setbit(&rank->ienable, r, info);
/* The irq number is extracted from offset. so shift by register size */
- vgic_enable_irqs(v, r & (~tr), (reg - GICD_ISENABLER) >> DABT_WORD);
+ vgic_enable_irqs(v, (rank->ienable) & (~tr),
+ (reg - GICD_ISENABLER) >> DABT_WORD);
vgic_unlock_rank(v, rank, flags);
return 1;
case GICD_ICENABLER ... GICD_ICENABLERN:
@@ -455,9 +474,10 @@ static int __vgic_v3_distr_common_mmio_write(const char *name, struct vcpu *v,
if ( rank == NULL ) goto write_ignore;
vgic_lock_rank(v, rank, flags);
tr = rank->ienable;
- rank->ienable &= ~r;
+ vgic_reg32_clearbit(&rank->ienable, r, info);
/* The irq number is extracted from offset. so shift by register size */
- vgic_disable_irqs(v, r & tr, (reg - GICD_ICENABLER) >> DABT_WORD);
+ vgic_disable_irqs(v, (~rank->ienable) & tr,
+ (reg - GICD_ICENABLER) >> DABT_WORD);
vgic_unlock_rank(v, rank, flags);
return 1;
case GICD_ISPENDR ... GICD_ISPENDRN:
@@ -497,13 +517,8 @@ static int __vgic_v3_distr_common_mmio_write(const char *name, struct vcpu *v,
if ( rank == NULL) goto write_ignore;
vgic_lock_rank(v, rank, flags);
- if ( dabt.size == DABT_WORD )
- ipriorityr = r;
- else
- {
- ipriorityr = vgic_generate_ipriorityr(rank, reg - GICD_IPRIORITYR);
- vgic_byte_write(&ipriorityr, r, reg);
- }
+ ipriorityr = vgic_generate_ipriorityr(rank, reg - GICD_IPRIORITYR);
+ vgic_reg32_write(&ipriorityr, r, info);
vgic_store_ipriorityr(rank, reg - GICD_IPRIORITYR, ipriorityr);
vgic_unlock_rank(v, rank, flags);
return 1;
@@ -518,7 +533,9 @@ static int __vgic_v3_distr_common_mmio_write(const char *name, struct vcpu *v,
rank = vgic_rank_offset(v, 2, reg - GICD_ICFGR, DABT_WORD);
if ( rank == NULL ) goto write_ignore;
vgic_lock_rank(v, rank, flags);
- rank->icfg[REG_RANK_INDEX(2, reg - GICD_ICFGR, DABT_WORD)] = r;
+ vgic_reg32_write(&rank->icfg[REG_RANK_INDEX(2, reg - GICD_ICFGR,
+ DABT_WORD)],
+ r, info);
vgic_unlock_rank(v, rank, flags);
return 1;
default:
@@ -754,7 +771,7 @@ static int vgic_v3_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
case GICD_CTLR:
if ( dabt.size != DABT_WORD ) goto bad_width;
vgic_lock(v);
- *r = v->domain->arch.vgic.ctlr;
+ *r = vgic_reg32_read(v->domain->arch.vgic.ctlr, info);
vgic_unlock(v);
return 1;
case GICD_TYPER:
@@ -769,13 +786,16 @@ static int vgic_v3_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
* bit is zero. The maximum is 8.
*/
unsigned int ncpus = min_t(unsigned int, v->domain->max_vcpus, 8);
+ uint32_t typer;
if ( dabt.size != DABT_WORD ) goto bad_width;
/* No secure world support for guests. */
- *r = ((ncpus - 1) << GICD_TYPE_CPUS_SHIFT |
- DIV_ROUND_UP(v->domain->arch.vgic.nr_spis, 32));
+ typer = ((ncpus - 1) << GICD_TYPE_CPUS_SHIFT |
+ DIV_ROUND_UP(v->domain->arch.vgic.nr_spis, 32));
- *r |= (irq_bits - 1) << GICD_TYPE_ID_BITS_SHIFT;
+ typer |= (irq_bits - 1) << GICD_TYPE_ID_BITS_SHIFT;
+
+ *r = vgic_reg32_read(typer, info);
return 1;
}
@@ -787,7 +807,7 @@ static int vgic_v3_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
goto read_as_zero_32;
case GICD_IIDR:
if ( dabt.size != DABT_WORD ) goto bad_width;
- *r = GICV3_GICD_IIDR_VAL;
+ *r = vgic_reg32_read(GICV3_GICD_IIDR_VAL, info);
return 1;
case 0x020 ... 0x03c:
case 0xc000 ... 0xffcc:
@@ -810,14 +830,22 @@ static int vgic_v3_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
/* SGI/PPI is RES0 */
goto read_as_zero_64;
case GICD_IROUTER32 ... GICD_IROUTERN:
+ {
+ uint64_t irouter;
+
if ( dabt.size != DABT_DOUBLE_WORD ) goto bad_width;
rank = vgic_rank_offset(v, 64, gicd_reg - GICD_IROUTER,
DABT_DOUBLE_WORD);
if ( rank == NULL ) goto read_as_zero;
vgic_lock_rank(v, rank, flags);
- *r = vgic_generate_irouter(rank, gicd_reg - GICD_IROUTER);
+ irouter = vgic_generate_irouter(rank, gicd_reg - GICD_IROUTER);
vgic_unlock_rank(v, rank, flags);
+
+ *r = vgic_reg64_read(irouter, info);
+
return 1;
+ }
+
case GICD_NSACR ... GICD_NSACRN:
/* We do not implement security extensions for guests, read zero */
goto read_as_zero_32;
@@ -833,17 +861,17 @@ static int vgic_v3_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
case GICD_PIDR0:
/* GICv3 identification value */
if ( dabt.size != DABT_WORD ) goto bad_width;
- *r = GICV3_GICD_PIDR0;
+ *r = vgic_reg32_read(GICV3_GICD_PIDR0, info);
return 1;
case GICD_PIDR1:
/* GICv3 identification value */
if ( dabt.size != DABT_WORD ) goto bad_width;
- *r = GICV3_GICD_PIDR1;
+ *r = vgic_reg32_read(GICV3_GICD_PIDR1, info);
return 1;
case GICD_PIDR2:
/* GICv3 identification value */
if ( dabt.size != DABT_WORD ) goto bad_width;
- *r = GICV3_GICD_PIDR2;
+ *r = vgic_reg32_read(GICV3_GICD_PIDR2, info);
return 1;
case GICD_PIDR3:
/* GICv3 identification value. Manufacturer/Customer defined */
@@ -851,7 +879,7 @@ static int vgic_v3_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
case GICD_PIDR4:
/* GICv3 identification value */
if ( dabt.size != DABT_WORD ) goto bad_width;
- *r = GICV3_GICD_PIDR4;
+ *r = vgic_reg32_read(GICV3_GICD_PIDR4, info);
return 1;
case GICD_PIDR5 ... GICD_PIDR7:
/* Reserved0 */
@@ -963,14 +991,21 @@ static int vgic_v3_distr_mmio_write(struct vcpu *v, mmio_info_t *info,
/* SGI/PPI is RES0 */
goto write_ignore_64;
case GICD_IROUTER32 ... GICD_IROUTERN:
+ {
+ uint64_t irouter;
+
if ( dabt.size != DABT_DOUBLE_WORD ) goto bad_width;
rank = vgic_rank_offset(v, 64, gicd_reg - GICD_IROUTER,
DABT_DOUBLE_WORD);
if ( rank == NULL ) goto write_ignore;
vgic_lock_rank(v, rank, flags);
- vgic_store_irouter(v->domain, rank, gicd_reg - GICD_IROUTER, r);
+ irouter = vgic_generate_irouter(rank, gicd_reg - GICD_IROUTER);
+ vgic_reg64_write(&irouter, r, info);
+ vgic_store_irouter(v->domain, rank, gicd_reg - GICD_IROUTER, irouter);
vgic_unlock_rank(v, rank, flags);
return 1;
+ }
+
case GICD_NSACR ... GICD_NSACRN:
/* We do not implement security extensions for guests, write ignore */
goto write_ignore_32;
diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
index 0cb8029..6d4c938 100644
--- a/xen/include/asm-arm/vgic.h
+++ b/xen/include/asm-arm/vgic.h
@@ -19,6 +19,7 @@
#define __ASM_ARM_VGIC_H__
#include <xen/bitops.h>
+#include <asm/mmio.h>
struct pending_irq
{
@@ -164,26 +165,116 @@ static inline int REG_RANK_NR(int b, uint32_t n)
}
}
-static inline uint32_t vgic_byte_read(uint32_t val, int offset)
+#define VGIC_REG_MASK(size) ((~0UL) >> (BITS_PER_LONG - ((1 << (size)) * 8)))
+
+/*
+ * The check on the size supported by the register has to be done by
+ * the caller of vgic_regN_*.
+ *
+ * vgic_reg_* should never be called directly. Instead use the vgic_regN_*
+ * according to size of the emulated register
+ *
+ * Note that the alignment fault will always be taken in the guest
+ * (see B3.12.7 DDI0406.b).
+ */
+static inline register_t vgic_reg_read(unsigned long reg,
+ unsigned int offset,
+ enum dabt_size size)
+{
+ reg >>= 8 * offset;
+ reg &= VGIC_REG_MASK(size);
+
+ return reg;
+}
+
+static inline void vgic_reg_write(unsigned long *reg, register_t val,
+ unsigned int offset,
+ enum dabt_size size)
{
- int byte = offset & 0x3;
+ unsigned long mask = VGIC_REG_MASK(size);
+ int shift = offset * 8;
- val = val >> (8*byte);
- val &= 0x000000ff;
+ *reg &= ~(mask << shift);
+ *reg |= ((unsigned long)val & mask) << shift;
+}
+
+static inline void vgic_reg_setbit(unsigned long *reg, register_t bits,
+ unsigned int offset,
+ enum dabt_size size)
+{
+ unsigned long mask = VGIC_REG_MASK(size);
+ int shift = offset * 8;
- return val;
+ *reg |= ((unsigned long)bits & mask) << shift;
}
-static inline void vgic_byte_write(uint32_t *reg, uint32_t var, int offset)
+static inline void vgic_reg_clearbit(unsigned long *reg, register_t bits,
+ unsigned int offset,
+ enum dabt_size size)
{
- int byte = offset & 0x3;
+ unsigned long mask = VGIC_REG_MASK(size);
+ int shift = offset * 8;
- var &= 0xff;
+ *reg &= ~(((unsigned long)bits & mask) << shift);
+}
- *reg &= ~(0xff << (8*byte));
- *reg |= (var << (8*byte));
+/* N-bit register helpers */
+#define VGIC_REG_HELPERS(sz, offmask) \
+static inline register_t vgic_reg##sz##_read(uint##sz##_t reg, \
+ const mmio_info_t *info) \
+{ \
+ return vgic_reg_read(reg, info->gpa & offmask, \
+ info->dabt.size); \
+} \
+ \
+static inline void vgic_reg##sz##_write(uint##sz##_t *reg, \
+ register_t val, \
+ const mmio_info_t *info) \
+{ \
+ unsigned long tmp = *reg; \
+ \
+ vgic_reg_write(&tmp, val, info->gpa & offmask, \
+ info->dabt.size); \
+ \
+ *reg = tmp; \
+} \
+ \
+static inline void vgic_reg##sz##_setbit(uint##sz##_t *reg, \
+ register_t bits, \
+ const mmio_info_t *info) \
+{ \
+ unsigned long tmp = *reg; \
+ \
+ vgic_reg_setbit(&tmp, bits, info->gpa & offmask, \
+ info->dabt.size); \
+ \
+ *reg = tmp; \
+} \
+ \
+static inline void vgic_reg##sz##_clearbit(uint##sz##_t *reg, \
+ register_t bits, \
+ const mmio_info_t *info) \
+{ \
+ unsigned long tmp = *reg; \
+ \
+ vgic_reg_clearbit(&tmp, bits, info->gpa & offmask, \
+ info->dabt.size); \
+ \
+ *reg = tmp; \
}
+/*
+ * 64 bits register are only supported on platform with 64-bit long.
+ * This is also allow us to optimize the 32 bit case by using
+ * unsigned long rather than uint64_t
+ */
+#if BITS_PER_LONG == 64
+VGIC_REG_HELPERS(64, 0x7);
+#endif
+VGIC_REG_HELPERS(32, 0x3);
+
+#undef VGIC_REG_HELPERS
+
enum gic_sgi_mode;
/*
--
2.1.4
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v1 8/8] xen/arm: vgic-v3: Support 32-bit access for 64-bit registers
[not found] <1443192667-16112-1-git-send-email-julien.grall@citrix.com>
` (6 preceding siblings ...)
2015-09-25 14:51 ` [PATCH v1 7/8] xen/arm: vgic: Introduce helpers to read/write/clear/set vGIC register Julien Grall
@ 2015-09-25 14:51 ` Julien Grall
2015-09-25 14:52 ` [PATCH v1 0/8] xen/arm: vgic: Support 32-bit access for 64-bit register Julien Grall
8 siblings, 0 replies; 11+ messages in thread
From: Julien Grall @ 2015-09-25 14:51 UTC (permalink / raw)
To: xen-devel
Cc: Julien Grall, linux-kernel, ian.campbell, linux-arm-kernel,
stefano.stabellini
Based on 8.1.3 (IHI 0069A), unless stated otherwise, the 64-bit registers
supports both 32-bit and 64-bits access.
All the registers we properly emulate (i.e not RAZ/WI) supports 32-bit access.
For RAZ/WI, it's also seems to be the case but I'm not 100% sure. Anyway,
emulating 32-bit access for them doesn't hurt. Note that we would need
some extra care when they will be implemented (for instance GICR_PROPBASER).
Signed-off-by: Julien Grall <julien.grall@citrix.com>
---
This is technically fixing boot of FreeBSD ARM64 guest with GICv3.
AFAICT, Linux is not using 32-bit access in the GICv3 code expect
for the ITS (which we don't support yet).
So this patch is a good candiate for Xen 4.6. Although this patch is
heavily depend on previous patches. It may be possible to shuffle
and move the "opmitization" patches towards the end. I haven't yet
done that because I feel this series makes more sense in the current
order.
Also, I haven't move vgic_reg64_check_access in vgic.h because there
is no usage in this series outside of vgic-v3.c and the helpers is
GICv3 oriented.
Changes in v2:
- Support 32bit access on the most significant word of
GICR_TYPER
---
xen/arch/arm/vgic-v3.c | 24 +++++++++++++++++-------
1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index 8da695f..46b5ad8 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -162,6 +162,15 @@ static struct vcpu *vgic_v3_get_target_vcpu(struct vcpu *v, unsigned int irq)
return v->domain->vcpu[rank->vcpu[irq & INTERRUPT_RANK_MASK]];
}
+static inline bool vgic_reg64_check_access(struct hsr_dabt dabt)
+{
+ /*
+ * 64 bits registers can be accessible using 32-bit and 64-bit unless
+ * stated otherwise (See 8.1.3 ARM IHI 0069A).
+ */
+ return ( dabt.size == DABT_DOUBLE_WORD || dabt.size == DABT_WORD );
+}
+
static int __vgic_v3_rdistr_rd_mmio_read(struct vcpu *v, mmio_info_t *info,
uint32_t gicr_reg,
register_t *r)
@@ -178,10 +187,11 @@ static int __vgic_v3_rdistr_rd_mmio_read(struct vcpu *v, mmio_info_t *info,
*r = vgic_reg32_read(GICV3_GICR_IIDR_VAL, info);
return 1;
case GICR_TYPER:
+ case GICR_TYPER + 4:
{
uint64_t typer, aff;
- if ( dabt.size != DABT_DOUBLE_WORD ) goto bad_width;
+ if ( !vgic_reg64_check_access(dabt) ) goto bad_width;
/* TBD: Update processor id in [23:8] when ITS support is added */
aff = (MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 3) << 56 |
MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 2) << 48 |
@@ -267,7 +277,7 @@ bad_width:
return 0;
read_as_zero_64:
- if ( dabt.size != DABT_DOUBLE_WORD ) goto bad_width;
+ if ( !vgic_reg64_check_access(dabt) ) goto bad_width;
*r = 0;
return 1;
@@ -343,7 +353,7 @@ bad_width:
return 0;
write_ignore_64:
- if ( dabt.size != DABT_DOUBLE_WORD ) goto bad_width;
+ if ( vgic_reg64_check_access(dabt) ) goto bad_width;
return 1;
write_ignore_32:
@@ -833,7 +843,7 @@ static int vgic_v3_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
{
uint64_t irouter;
- if ( dabt.size != DABT_DOUBLE_WORD ) goto bad_width;
+ if ( !vgic_reg64_check_access(dabt) ) goto bad_width;
rank = vgic_rank_offset(v, 64, gicd_reg - GICD_IROUTER,
DABT_DOUBLE_WORD);
if ( rank == NULL ) goto read_as_zero;
@@ -908,7 +918,7 @@ bad_width:
return 0;
read_as_zero_64:
- if ( dabt.size != DABT_DOUBLE_WORD ) goto bad_width;
+ if ( vgic_reg64_check_access(dabt) ) goto bad_width;
*r = 0;
return 1;
@@ -994,7 +1004,7 @@ static int vgic_v3_distr_mmio_write(struct vcpu *v, mmio_info_t *info,
{
uint64_t irouter;
- if ( dabt.size != DABT_DOUBLE_WORD ) goto bad_width;
+ if ( !vgic_reg64_check_access(dabt) ) goto bad_width;
rank = vgic_rank_offset(v, 64, gicd_reg - GICD_IROUTER,
DABT_DOUBLE_WORD);
if ( rank == NULL ) goto write_ignore;
@@ -1053,7 +1063,7 @@ write_ignore_32:
return 1;
write_ignore_64:
- if ( dabt.size != DABT_DOUBLE_WORD ) goto bad_width;
+ if ( vgic_reg64_check_access(dabt) ) goto bad_width;
return 1;
write_ignore:
--
2.1.4
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v1 0/8] xen/arm: vgic: Support 32-bit access for 64-bit register
[not found] <1443192667-16112-1-git-send-email-julien.grall@citrix.com>
` (7 preceding siblings ...)
2015-09-25 14:51 ` [PATCH v1 8/8] xen/arm: vgic-v3: Support 32-bit access for 64-bit registers Julien Grall
@ 2015-09-25 14:52 ` Julien Grall
8 siblings, 0 replies; 11+ messages in thread
From: Julien Grall @ 2015-09-25 14:52 UTC (permalink / raw)
To: xen-devel
Cc: linux-kernel, ian.campbell, linux-arm-kernel, stefano.stabellini
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
^ permalink raw reply [flat|nested] 11+ messages in thread