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

Re: [Xen-devel] [RFC PATCH 02/10] arm64: Add hook to handle guest GICv3 sysreg accesses

On 01/25/2018 11:37 PM, Julien Grall wrote:

I forgot to mention one thing about the placement of do_fixup_vgic_errata.

On 16/01/18 15:42, mjaggi@xxxxxxxxxxxxxxxxxx wrote:
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index f6f6de3691..d4f0581d33 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2103,6 +2103,17 @@ void do_trap_guest_sync(struct cpu_user_regs *regs)
      const union hsr hsr = { .bits = regs->hsr };
+    int ret;
+    ret  = do_fixup_vgic_errata(regs,hsr);
+    if ( !ret )
+    {
+        advance_pc(regs, hsr);
+        return;

I am fully aware that I suggested this solution and still support that the vGIC errata should be fully separated. After all, it deals with hardware bug and the errata will just update the LRs as the hardware would do.

enter_hypervisor_head() will sync the LRs state to the internal vGIC state. leave_hypervisor_head() will process pending softirq and write/update the LRs based on the internal vGIC state.

As you rightfully did, the do_fixup_vgic_errata should be called before syncing the LRs. However, even if you return early here, you will still execute leave_hypervisor_tail(). This mean that pending softirqs will be processed and potentially the vCPU rescheduled. Because the LRs were not synced (enter_hypervisor_head()) was not called, then the vGIC state will not out-of-date and would lead to all sort of potential issues.

As the vGIC errata implies trapping the register such as IAR1 (reading interrupt), we want to get a fastpath for it (e.g not trying to execute softirq...). So I think we should bypass leave_hypervisor_tail(). I am not entirely sure how to do it nicely thought.

How about adding a check for group1_trap enable in leave_hypervisor_tail().

void leave_hypervisor_tail(void)
+if (group1_trap)
+   return;
    while (1)


+    }
        switch (hsr.ec) {
diff --git a/xen/include/asm-arm/arm64/traps.h b/xen/include/asm-arm/arm64/traps.h
index 2379b578cb..7378a1b022 100644
--- a/xen/include/asm-arm/arm64/traps.h
+++ b/xen/include/asm-arm/arm64/traps.h
@@ -5,6 +5,8 @@ void inject_undef64_exception(struct cpu_user_regs *regs, int instr_len);
    void do_sysreg(struct cpu_user_regs *regs,
                 const union hsr hsr);
+int do_fixup_vgic_errata(struct cpu_user_regs *regs,
+               const union hsr hsr);
    #endif /* __ASM_ARM64_TRAPS__ */

Xen-devel mailing list



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