|
Hello Julien,
On 10/27/2018 12:55 AM, Julien Grall wrote:
The functions below does not exist in latest Xen. So are you sure this based on mainline?
Yep, I've put a wrong diff into the email, it is for XEN 4.10.
Please find the patch for XEN 4.12-unstable on my github [1].
How many domain do you have run at that time?
Only one domain. In my setup I'm running a BSP release under XEN as Dom0.
Under which condition this is hapenning?
Under no specific conditions. During the system boot, during console operations, during glmark2 [2] running.
Also, what is your Xen command line?
`dom0_mem=1G
console=dtuart dtuart=serial0 dom0_max_vcpus=4 bootscrub=0 loglvl=all cpufreq=none`
I assume you didn't disable serrors, right? So we should have plenty of dsb(sy) and isb() in the path to there.
I didn't disable serrors.
So I would rule out a missing barrier here.
Those prints I treat as LR content change (state PENDING->INVALID)
without exiting from hyp to guest. Unfortunately, I did not find a kind
of explanation to this effect in ARM IHI 0048B.b document.
I have GICv2 on the die.
I appreciate you find some time to look at this and express your opinion.
---
diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index 511c8d7..0b9aa2d 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -171,6 +171,14 @@ static unsigned int gicv2_cpu_mask(const cpumask_t
*cpumask)
return mask;
}
+static void gicv2_store_lrs(void)
+{
+ int i;
+
+ for ( i = 0; i < gicv2_info.nr_lrs; i++ )
+ current->arch.gic.v2.lr[i] = readl_gich(GICH_LR + i * 4);
+}
+
static void gicv2_save_state(struct vcpu *v)
{
int i;
@@ -446,11 +454,13 @@ static void gicv2_update_lr(int lr, const struct
pending_irq *p,
<< GICH_V2_LR_PHYSICAL_SHIFT);
writel_gich(lr_reg, GICH_LR + lr * 4);
+ current->arch.gic.v2.lr[lr] = lr_reg;
}
static void gicv2_clear_lr(int lr)
{
writel_gich(0, GICH_LR + lr * 4);
+ current->arch.gic.v2.lr[lr] = 0;
}
static void gicv2_read_lr(int lr, struct gic_lr *lr_reg)
@@ -458,6 +468,9 @@ static void gicv2_read_lr(int lr, struct gic_lr *lr_reg)
uint32_t lrv;
lrv = readl_gich(GICH_LR + lr * 4);
+ if ( lrv != current->arch.gic.v2.lr[lr])
+ printk("Stored LR is stale: stored 0x%"PRIX32" read 0x%"PRIX32"\n",
+ current->arch.gic.v2.lr[lr], lrv);
lr_reg->pirq = (lrv >> GICH_V2_LR_PHYSICAL_SHIFT) &
GICH_V2_LR_PHYSICAL_MASK;
lr_reg->virq = (lrv >> GICH_V2_LR_VIRTUAL_SHIFT) &
GICH_V2_LR_VIRTUAL_MASK;
lr_reg->priority = (lrv >> GICH_V2_LR_PRIORITY_SHIFT) &
GICH_V2_LR_PRIORITY_MASK;
@@ -481,6 +494,7 @@ static void gicv2_write_lr(int lr, const struct
gic_lr *lr_reg)
((uint32_t)(lr_reg->grp & GICH_V2_LR_GRP_MASK) <<
GICH_V2_LR_GRP_SHIFT) );
writel_gich(lrv, GICH_LR + lr * 4);
+ current->arch.gic.v2.lr[lr] = lrv;
}
static void gicv2_hcr_status(uint32_t flag, bool status)
@@ -1232,6 +1246,7 @@ const static struct gic_hw_operations gicv2_ops = {
.info = &gicv2_info,
.init = gicv2_init,
.secondary_init = gicv2_secondary_cpu_init,
+ .store_lrs = gicv2_store_lrs,
.save_state = gicv2_save_state,
.restore_state = gicv2_restore_state,
.dump_state = gicv2_dump_state,
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index ed363f6..a05c518 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -594,6 +594,12 @@ static void gic_update_one_lr(struct vcpu *v, int i)
}
}
+void gic_store_lrs(void)
+{
+ if(gic_hw_ops->store_lrs)
+ gic_hw_ops->store_lrs();
+}
+
void gic_clear_lrs(struct vcpu *v)
{
int i = 0;
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index f6f6de3..985192b 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2095,6 +2095,7 @@ static void enter_hypervisor_head(struct
cpu_user_regs *regs)
if ( current->arch.hcr_el2 & HCR_VA )
current->arch.hcr_el2 = READ_SYSREG(HCR_EL2);
+ gic_store_lrs();
gic_clear_lrs(...) may rewrite the LRs. To confirm this stall LRs are not due by this function, can you move that after gic_clear_lrs()?
Right you are, gic_clear_lrs(), in 4.10 codebase, rewrites LRs. But it does not change PENDING to INVALID under no circumstances from one hand. From another hand, all changes to LRs are made through gic specific operations gic_hw_ops->... which are tracked
by me. You can see, in the code above, that I copy all updates to the physical LR issued by hypervisor into the stored LRs. So it not the case. But I'll check on Monday.
gic_clear_lrs(current);
}
}
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index d3d7bda..6fe3fdb 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -293,6 +293,7 @@ extern unsigned int gic_number_lines(void);
/* IRQ translation function for the device tree */
int gic_irq_xlate(const u32 *intspec, unsigned int intsize,
unsigned int *out_hwirq, unsigned int *out_type);
+void gic_store_lrs(void);
void gic_clear_lrs(struct vcpu *v);
struct gic_info {
@@ -314,6 +315,8 @@ struct gic_hw_operations {
/* Initialize the GIC and the boot CPU */
int (*init)(void);
/* Save GIC registers */
+ void (*store_lrs)(void);
+ /* Save GIC registers */
void (*save_state)(struct vcpu *);
/* Restore GIC registers */
void (*restore_state)(const struct vcpu *);
Cheers,
[1]
https://github.com/aanisov/xen/commit/b1c28be38d30eb460ed0339526c4987b2cf1708c
[2] https://github.com/glmark2/glmark2
|
