[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:
Hi,

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 };
  +#ifdef CONFIG_VGIC_ERRATA
+    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)


Cheers,

+    }
+#endif
+
      enter_hypervisor_head(regs);
        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
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

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