Re: [Xen-devel] [PATCH] x86/hvm: implement save/restore for posted interrupts

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 09/10/14 08:24, Zhang, Yang Z wrote:

Olaf Hering wrote on 2014-10-08:

On Wed, Oct 08, Zhang, Yang Z wrote:

Olaf Hering wrote on 2014-10-08:

On Wed, Oct 08, Zhang, Yang Z wrote:

Can you reproduce it with APICv disabled? You can add apicv=0
into your grub to disable it.

Using 'apicv=0' with staging helps, the guest resumes properly.

Does 'resumes properly' mean there are two sysrq in dmesg after resume?

Yes.

Will now try the patch you mentioned in the other mail.

Thanks. I am set upping the environment for testing now.

That patch (Message-ID: <20140804075007.GA11609@xxxxxxxxx>) does not
help.

Sorry. I forget to tell that that patch must combine with one fixing from your patch.
I rebased it based on latest Xen and the attached patch includes all fixings. Could you have a try? It works on my side, hope it helps.

diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
index 99ae1be..e702ed3 100644
--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -1259,6 +1259,9 @@ static int lapic_save_regs(struct domain *d, hvm_domain_context_t *h)
 
     for_each_vcpu ( d, v )
     {
+        if ( hvm_funcs.sync_pir_to_irr )
+            hvm_funcs.sync_pir_to_irr(v);
+
         s = vcpu_vlapic(v);
         if ( (rc = hvm_save_entry(LAPIC_REGS, v->vcpu_id, h, s->regs)) != 0 )
             break;
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 304aeea..7c4d796 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -1584,6 +1584,8 @@ static void vmx_process_isr(int isr, struct vcpu *v)
 {
     unsigned long status;
     u8 old;
+    int vector;

unsigned vector; especially as it is used in some pointer arithmetic hidden inside the vlapic_test_vector() macro.

+    struct vlapic *s = vcpu_vlapic(v);

's' seems to be a strange choice of variable name.  Prevailing use in this file is "struct vlapic *vlapic = vcpu_vlapic(v);"

 
     if ( isr < 0 )
         isr = 0;
@@ -1597,6 +1599,14 @@ static void vmx_process_isr(int isr, struct vcpu *v)
         status |= isr << VMX_GUEST_INTR_STATUS_SVI_OFFSET;
         __vmwrite(GUEST_INTR_STATUS, status);
     }
+    for ( vector = 0; vector < NR_VECTORS; vector++ )

Vectors 0 to 0x0f are strictly reserved.  Any reason not to start vector at 0x10 ?

+        if (vlapic_test_vector(vector, &s->regs->data[APIC_TMR]))
+            set_bit(vector,  v->arch.hvm_vmx.eoi_exit_bitmap);

Can't this loop be optimised somewhat to using word-sized |= operations?

~Andrew

+        
+    __vmwrite(0x201c, v->arch.hvm_vmx.eoi_exit_bitmap[0]);
+    __vmwrite(0x201e, v->arch.hvm_vmx.eoi_exit_bitmap[1]);
+    __vmwrite(0x2020, v->arch.hvm_vmx.eoi_exit_bitmap[2]);
+    __vmwrite(0x2022, v->arch.hvm_vmx.eoi_exit_bitmap[3]);
     vmx_vmcs_exit(v);
 }
 
diff --git a/xen/include/asm-x86/hvm/vlapic.h b/xen/include/asm-x86/hvm/vlapic.h
index bf59b95..fc8d131 100644
--- a/xen/include/asm-x86/hvm/vlapic.h
+++ b/xen/include/asm-x86/hvm/vlapic.h
@@ -61,6 +61,8 @@
 
 #define VEC_POS(v) ((v) % 32)
 #define REG_POS(v) (((v) / 32) * 0x10)
+#define vlapic_test_vector(vec, bitmap)                         \
+    test_bit(VEC_POS(vec), (uint32_t *)((bitmap) + REG_POS(vec)))
 #define vlapic_test_and_set_vector(vec, bitmap)                         \
     test_and_set_bit(VEC_POS(vec), (uint32_t *)((bitmap) + REG_POS(vec)))
 #define vlapic_test_and_clear_vector(vec, bitmap)  

Olaf

Best regards,
Yang

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

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