From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH v2 2/2] x86/HVM: fix ID handling of x2APIC emulation Date: Thu, 11 Sep 2014 17:28:42 +0100 Message-ID: <5411CDBA.2090401@citrix.com> References: <541070390200007800033519@mail.emea.novell.com> <541071DF0200007800033534@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============8250752557471909694==" Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1XS7En-0006TP-Ku for xen-devel@lists.xenproject.org; Thu, 11 Sep 2014 16:28:49 +0000 In-Reply-To: <541071DF0200007800033534@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich , xen-devel Cc: Ian Campbell , Ian Jackson , Keir Fraser , Tim Deegan List-Id: xen-devel@lists.xenproject.org --===============8250752557471909694== Content-Type: multipart/alternative; boundary="------------090503040007080201050406" --------------090503040007080201050406 Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: quoted-printable On 10/09/14 14:44, Jan Beulich wrote: > - properly change ID when switching into x2APIC mode (instead of > mimicking necessary behavior in hvm_x2apic_msr_read()) > - correctly (meaningfully) set LDR (so far it ended up being 1 on all > vCPU-s) > - even if we don't support more than 128 vCPU-s in a HVM guest for now,= > we should properly handle IDs as 32-bit values (i.e. not ignore the > top 24 bits) > - with that, properly do cluster ID and bit mask check in > vlapic_match_logical_addr() > - slightly adjust other parameter types of vlapic_match_dest() and > vlapic_lowest_prio() (and related local variable ones) I think the addition of arch_domain_unpause() at least needs mentioning in the commit message, although... > > Signed-off-by: Jan Beulich > --- > v2: Some changes broken out to separate patch. Correct ID and > LDR after domain restore (if necessary); as stated previously the > only compatibility problem this creates is when migrating a VM _to_= > an unfixed (i.e. old) hypervisor, a scenario which supposedly isn't= > supported. > > --- a/xen/arch/x86/hvm/vlapic.c > +++ b/xen/arch/x86/hvm/vlapic.c > @@ -173,18 +173,17 @@ uint32_t vlapic_set_ppr(struct vlapic *v > return ppr; > } > =20 > -static int vlapic_match_logical_addr(struct vlapic *vlapic, uint8_t md= a) > +static int vlapic_match_logical_addr(struct vlapic *vlapic, uint32_t m= da) > { > int result =3D 0; > - uint32_t logical_id; > + uint32_t logical_id =3D vlapic_get_reg(vlapic, APIC_LDR); > =20 > if ( vlapic_x2apic_mode(vlapic) ) > - { > - logical_id =3D vlapic_get_reg(vlapic, APIC_LDR); > - return !!(logical_id & mda); > - } > + return ((logical_id >> 16) =3D=3D (mda >> 16)) && > + (uint16_t)(logical_id & mda); > =20 > - logical_id =3D GET_xAPIC_LOGICAL_ID(vlapic_get_reg(vlapic, APIC_LD= R)); > + logical_id =3D GET_xAPIC_LOGICAL_ID(logical_id); > + mda =3D (uint8_t)mda; > =20 > switch ( vlapic_get_reg(vlapic, APIC_DFR) ) > { > @@ -207,8 +206,8 @@ static int vlapic_match_logical_addr(str > } > =20 > bool_t vlapic_match_dest( > - struct vlapic *target, struct vlapic *source, > - int short_hand, uint8_t dest, uint8_t dest_mode) > + struct vlapic *target, const struct vlapic *source, > + int short_hand, uint32_t dest, bool_t dest_mode) target should be const as well, and looks as if it can be by pushing const-ness down into vlapic_match_logical_addr() and vlapic_get_reg(). > { > HVM_DBG_LOG(DBG_LEVEL_VLAPIC, "target %p, source %p, dest %#x, " > "dest_mode %#x, short_hand %#x", > @@ -219,7 +218,8 @@ bool_t vlapic_match_dest( > case APIC_DEST_NOSHORT: > if ( dest_mode ) > return vlapic_match_logical_addr(target, dest); > - return ((dest =3D=3D 0xFF) || (dest =3D=3D VLAPIC_ID(target)))= ; > + return (dest =3D=3D _VLAPIC_ID(target, 0xffffffff)) || > + (dest =3D=3D VLAPIC_ID(target)); > =20 > case APIC_DEST_SELF: > return (target =3D=3D source); > @@ -286,7 +286,7 @@ static void vlapic_init_sipi_action(unsi > uint32_t icr =3D vcpu_vlapic(origin)->init_sipi.icr; > uint32_t dest =3D vcpu_vlapic(origin)->init_sipi.dest; > uint32_t short_hand =3D icr & APIC_SHORT_MASK; > - uint32_t dest_mode =3D !!(icr & APIC_DEST_MASK); > + bool_t dest_mode =3D !!(icr & APIC_DEST_MASK); > struct vcpu *v; > =20 > if ( icr =3D=3D 0 ) > @@ -352,8 +352,8 @@ static void vlapic_accept_irq(struct vcp > } > =20 > struct vlapic *vlapic_lowest_prio( > - struct domain *d, struct vlapic *source, > - int short_hand, uint8_t dest, uint8_t dest_mode) > + struct domain *d, const struct vlapic *source, > + int short_hand, uint32_t dest, bool_t dest_mode) > { > int old =3D d->arch.hvm_domain.irq.round_robin_prev_vcpu; > uint32_t ppr, target_ppr =3D UINT_MAX; > @@ -414,13 +414,11 @@ void vlapic_ipi( > { > unsigned int dest; > unsigned int short_hand =3D icr_low & APIC_SHORT_MASK; > - unsigned int dest_mode =3D !!(icr_low & APIC_DEST_MASK); > + bool_t dest_mode =3D !!(icr_low & APIC_DEST_MASK); > =20 > HVM_DBG_LOG(DBG_LEVEL_VLAPIC, "icr =3D 0x%08x:%08x", icr_high, icr= _low); > =20 > - dest =3D (vlapic_x2apic_mode(vlapic) > - ? icr_high > - : GET_xAPIC_DEST_FIELD(icr_high)); > + dest =3D _VLAPIC_ID(vlapic, icr_high); > =20 > switch ( icr_low & APIC_MODE_MASK ) > { > @@ -593,10 +591,6 @@ int hvm_x2apic_msr_read(struct vcpu *v,=20 > vlapic_read_aligned(vlapic, offset, &low); > switch ( offset ) > { > - case APIC_ID: > - low =3D GET_xAPIC_ID(low); > - break; > - > case APIC_ICR: > vlapic_read_aligned(vlapic, APIC_ICR2, &high); > break; > @@ -891,6 +885,15 @@ const struct hvm_mmio_handler vlapic_mmi > .write_handler =3D vlapic_write > }; > =20 > +static void set_x2apic_id(struct vlapic *vlapic) > +{ > + u32 id =3D vlapic_vcpu(vlapic)->vcpu_id; > + u32 ldr =3D ((id & ~0xf) << 12) | (1 << (id & 0xf)); > + > + vlapic_set_reg(vlapic, APIC_ID, id * 2); I know this mimics the existing behaviour, but I should point out that it is an Intel-ism (which assumes no hyperthreads) which is wrong on AMD systems, and confuses algorithms which following the BIOS/Systems guides. I do plan to fix it as part of my cpuid/feature levelling fixes.= > + vlapic_set_reg(vlapic, APIC_LDR, ldr); > +} > + > bool_t vlapic_msr_set(struct vlapic *vlapic, uint64_t value) > { > if ( (vlapic->hw.apic_base_msr ^ value) & MSR_IA32_APICBASE_ENABLE= ) > @@ -918,11 +921,7 @@ bool_t vlapic_msr_set(struct vlapic *vla > vlapic->hw.apic_base_msr =3D value; > =20 > if ( vlapic_x2apic_mode(vlapic) ) > - { > - u32 id =3D vlapic_get_reg(vlapic, APIC_ID); > - u32 ldr =3D ((id & ~0xf) << 16) | (1 << (id & 0xf)); > - vlapic_set_reg(vlapic, APIC_LDR, ldr); > - } > + set_x2apic_id(vlapic); > =20 > vmx_vlapic_msr_changed(vlapic_vcpu(vlapic)); > =20 > @@ -1209,6 +1208,7 @@ static int lapic_load_hidden(struct doma > return -EINVAL; > } > s =3D vcpu_vlapic(v); > + s->loaded =3D 1; > =20 > if ( hvm_load_entry_zeroextend(LAPIC, h, &s->hw) !=3D 0 )=20 > return -EINVAL; > @@ -1237,6 +1237,7 @@ static int lapic_load_regs(struct domain > return -EINVAL; > } > s =3D vcpu_vlapic(v); > + s->loaded =3D 1; > =20 > if ( hvm_load_entry(LAPIC_REGS, h, s->regs) !=3D 0 )=20 > return -EINVAL; > @@ -1254,6 +1255,29 @@ HVM_REGISTER_SAVE_RESTORE(LAPIC, lapic_s > HVM_REGISTER_SAVE_RESTORE(LAPIC_REGS, lapic_save_regs, lapic_load_regs= , > 1, HVMSR_PER_VCPU); > =20 > +void vlapic_domain_unpause(const struct domain *d) > +{ > + /* > + * Following lapic_load_hidden()/lapic_load_regs() we may need to > + * correct ID and LDR when they come from an old, broken hyperviso= r. > + */ This seems like aweful overhead for the domain_{,un}pause() path. Why can't it be fixed up once in lapic_load_{regs,hidden}(), or possibly deferred to the end of hvm_load()? > + struct vcpu *v; > + > + for_each_vcpu ( d, v ) > + { > + struct vlapic *vlapic =3D vcpu_vlapic(v); > + u32 id =3D vlapic_get_reg(vlapic, APIC_ID); > + > + if ( vlapic->loaded && vlapic_x2apic_mode(vlapic) && > + id && GET_xAPIC_ID(id) =3D=3D v->vcpu_id * 2 && > + id =3D=3D SET_xAPIC_ID(GET_xAPIC_ID(id)) && > + vlapic_get_reg(vlapic, APIC_LDR) =3D=3D 1 ) > + set_x2apic_id(vlapic); > + > + vlapic->loaded =3D 0; > + } > +} > + > int vlapic_init(struct vcpu *v) > { > struct vlapic *vlapic =3D vcpu_vlapic(v); > --- a/xen/common/domain.c > +++ b/xen/common/domain.c > @@ -952,8 +952,11 @@ void domain_unpause(struct domain *d) > struct vcpu *v; > =20 > if ( atomic_dec_and_test(&d->pause_count) ) > + { > + arch_domain_unpause(d); > for_each_vcpu( d, v ) > vcpu_wake(v); > + } > } > =20 > int __domain_pause_by_systemcontroller(struct domain *d, > --- a/xen/include/asm-arm/domain.h > +++ b/xen/include/asm-arm/domain.h > @@ -241,6 +241,8 @@ struct arch_vcpu > void vcpu_show_execution_state(struct vcpu *); > void vcpu_show_registers(const struct vcpu *); > =20 > +#define arch_domain_unpause(d) ((void)(d)) > + > #endif /* __ASM_DOMAIN_H__ */ > =20 > /* > --- a/xen/include/asm-x86/domain.h > +++ b/xen/include/asm-x86/domain.h > @@ -500,6 +500,12 @@ void domain_cpuid(struct domain *d, > unsigned int *ecx, > unsigned int *edx); > =20 > +#define arch_domain_unpause(d) ({ \ > + const struct domain *d_ =3D (d); \ > + if ( is_hvm_domain(d_) ) \ > + vlapic_domain_unpause(d_); \ > +}) > + > #endif /* __ASM_DOMAIN_H__ */ > =20 > /* > --- a/xen/include/asm-x86/hvm/vlapic.h > +++ b/xen/include/asm-x86/hvm/vlapic.h > @@ -30,8 +30,9 @@ > #define vlapic_vcpu(x) (container_of((x), struct vcpu, arch.hvm_vcpu= =2Evlapic)) > #define vlapic_domain(x) (vlapic_vcpu(x)->domain) > =20 > -#define VLAPIC_ID(vlapic) \ > - (GET_xAPIC_ID(vlapic_get_reg((vlapic), APIC_ID))) > +#define _VLAPIC_ID(vlapic, id) (vlapic_x2apic_mode(vlapic) \ > + ? (id) : GET_xAPIC_ID(id)) > +#define VLAPIC_ID(vlapic) _VLAPIC_ID(vlapic, vlapic_get_reg(vlapic, AP= IC_ID)) Some comment regarding the difference between these two? ~Andrew > =20 > /* > * APIC can be disabled in two ways: > @@ -70,6 +71,7 @@ > struct vlapic { > struct hvm_hw_lapic hw; > struct hvm_hw_lapic_regs *regs; > + bool_t loaded; > struct periodic_time pt; > s_time_t timer_last_update; > struct page_info *regs_page; > @@ -123,11 +125,13 @@ void vlapic_ipi(struct vlapic *vlapic, u > int vlapic_apicv_write(struct vcpu *v, unsigned int offset); > =20 > struct vlapic *vlapic_lowest_prio( > - struct domain *d, struct vlapic *source, > - int short_hand, uint8_t dest, uint8_t dest_mode); > + struct domain *d, const struct vlapic *source, > + int short_hand, uint32_t dest, bool_t dest_mode); > =20 > bool_t vlapic_match_dest( > - struct vlapic *target, struct vlapic *source, > - int short_hand, uint8_t dest, uint8_t dest_mode); > + struct vlapic *target, const struct vlapic *source, > + int short_hand, uint32_t dest, bool_t dest_mode); > + > +void vlapic_domain_unpause(const struct domain *); > =20 > #endif /* __ASM_X86_HVM_VLAPIC_H__ */ > > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel --------------090503040007080201050406 Content-Type: text/html; charset="ISO-8859-1" Content-Transfer-Encoding: 8bit
On 10/09/14 14:44, Jan Beulich wrote:
- properly change ID when switching into x2APIC mode (instead of
  mimicking necessary behavior in hvm_x2apic_msr_read())
- correctly (meaningfully) set LDR (so far it ended up being 1 on all
  vCPU-s)
- even if we don't support more than 128 vCPU-s in a HVM guest for now,
  we should properly handle IDs as 32-bit values (i.e. not ignore the
  top 24 bits)
- with that, properly do cluster ID and bit mask check in
  vlapic_match_logical_addr()
- slightly adjust other parameter types of vlapic_match_dest() and
  vlapic_lowest_prio() (and related local variable ones)

I think the addition of arch_domain_unpause() at least needs mentioning in the commit message, although...


Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: Some changes broken out to separate patch. Correct ID and
    LDR after domain restore (if necessary); as stated previously the
    only compatibility problem this creates is when migrating a VM _to_
    an unfixed (i.e. old) hypervisor, a scenario which supposedly isn't
    supported.

--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -173,18 +173,17 @@ uint32_t vlapic_set_ppr(struct vlapic *v
    return ppr;
 }
 
-static int vlapic_match_logical_addr(struct vlapic *vlapic, uint8_t mda)
+static int vlapic_match_logical_addr(struct vlapic *vlapic, uint32_t mda)
 {
     int result = 0;
-    uint32_t logical_id;
+    uint32_t logical_id = vlapic_get_reg(vlapic, APIC_LDR);
 
     if ( vlapic_x2apic_mode(vlapic) )
-    {
-        logical_id = vlapic_get_reg(vlapic, APIC_LDR);
-        return !!(logical_id & mda);
-    }
+        return ((logical_id >> 16) == (mda >> 16)) &&
+               (uint16_t)(logical_id & mda);
 
-    logical_id = GET_xAPIC_LOGICAL_ID(vlapic_get_reg(vlapic, APIC_LDR));
+    logical_id = GET_xAPIC_LOGICAL_ID(logical_id);
+    mda = (uint8_t)mda;
 
     switch ( vlapic_get_reg(vlapic, APIC_DFR) )
     {
@@ -207,8 +206,8 @@ static int vlapic_match_logical_addr(str
 }
 
 bool_t vlapic_match_dest(
-    struct vlapic *target, struct vlapic *source,
-    int short_hand, uint8_t dest, uint8_t dest_mode)
+    struct vlapic *target, const struct vlapic *source,
+    int short_hand, uint32_t dest, bool_t dest_mode)

target should be const as well, and looks as if it can be by pushing const-ness down into vlapic_match_logical_addr() and vlapic_get_reg().

 {
     HVM_DBG_LOG(DBG_LEVEL_VLAPIC, "target %p, source %p, dest %#x, "
                 "dest_mode %#x, short_hand %#x",
@@ -219,7 +218,8 @@ bool_t vlapic_match_dest(
     case APIC_DEST_NOSHORT:
         if ( dest_mode )
             return vlapic_match_logical_addr(target, dest);
-        return ((dest == 0xFF) || (dest == VLAPIC_ID(target)));
+        return (dest == _VLAPIC_ID(target, 0xffffffff)) ||
+               (dest == VLAPIC_ID(target));
 
     case APIC_DEST_SELF:
         return (target == source);
@@ -286,7 +286,7 @@ static void vlapic_init_sipi_action(unsi
     uint32_t icr = vcpu_vlapic(origin)->init_sipi.icr;
     uint32_t dest = vcpu_vlapic(origin)->init_sipi.dest;
     uint32_t short_hand = icr & APIC_SHORT_MASK;
-    uint32_t dest_mode  = !!(icr & APIC_DEST_MASK);
+    bool_t dest_mode  = !!(icr & APIC_DEST_MASK);
     struct vcpu *v;
 
     if ( icr == 0 )
@@ -352,8 +352,8 @@ static void vlapic_accept_irq(struct vcp
 }
 
 struct vlapic *vlapic_lowest_prio(
-    struct domain *d, struct vlapic *source,
-    int short_hand, uint8_t dest, uint8_t dest_mode)
+    struct domain *d, const struct vlapic *source,
+    int short_hand, uint32_t dest, bool_t dest_mode)
 {
     int old = d->arch.hvm_domain.irq.round_robin_prev_vcpu;
     uint32_t ppr, target_ppr = UINT_MAX;
@@ -414,13 +414,11 @@ void vlapic_ipi(
 {
     unsigned int dest;
     unsigned int short_hand = icr_low & APIC_SHORT_MASK;
-    unsigned int dest_mode  = !!(icr_low & APIC_DEST_MASK);
+    bool_t dest_mode = !!(icr_low & APIC_DEST_MASK);
 
     HVM_DBG_LOG(DBG_LEVEL_VLAPIC, "icr = 0x%08x:%08x", icr_high, icr_low);
 
-    dest = (vlapic_x2apic_mode(vlapic)
-            ? icr_high
-            : GET_xAPIC_DEST_FIELD(icr_high));
+    dest = _VLAPIC_ID(vlapic, icr_high);
 
     switch ( icr_low & APIC_MODE_MASK )
     {
@@ -593,10 +591,6 @@ int hvm_x2apic_msr_read(struct vcpu *v, 
     vlapic_read_aligned(vlapic, offset, &low);
     switch ( offset )
     {
-    case APIC_ID:
-        low = GET_xAPIC_ID(low);
-        break;
-
     case APIC_ICR:
         vlapic_read_aligned(vlapic, APIC_ICR2, &high);
         break;
@@ -891,6 +885,15 @@ const struct hvm_mmio_handler vlapic_mmi
     .write_handler = vlapic_write
 };
 
+static void set_x2apic_id(struct vlapic *vlapic)
+{
+    u32 id = vlapic_vcpu(vlapic)->vcpu_id;
+    u32 ldr = ((id & ~0xf) << 12) | (1 << (id & 0xf));
+
+    vlapic_set_reg(vlapic, APIC_ID, id * 2);

I know this mimics the existing behaviour, but I should point out that it is an Intel-ism (which assumes no hyperthreads) which is wrong on AMD systems, and confuses algorithms which following the BIOS/Systems guides.  I do plan to fix it as part of my cpuid/feature levelling fixes.

+    vlapic_set_reg(vlapic, APIC_LDR, ldr);
+}
+
 bool_t vlapic_msr_set(struct vlapic *vlapic, uint64_t value)
 {
     if ( (vlapic->hw.apic_base_msr ^ value) & MSR_IA32_APICBASE_ENABLE )
@@ -918,11 +921,7 @@ bool_t vlapic_msr_set(struct vlapic *vla
     vlapic->hw.apic_base_msr = value;
 
     if ( vlapic_x2apic_mode(vlapic) )
-    {
-        u32 id = vlapic_get_reg(vlapic, APIC_ID);
-        u32 ldr = ((id & ~0xf) << 16) | (1 << (id & 0xf));
-        vlapic_set_reg(vlapic, APIC_LDR, ldr);
-    }
+        set_x2apic_id(vlapic);
 
     vmx_vlapic_msr_changed(vlapic_vcpu(vlapic));
 
@@ -1209,6 +1208,7 @@ static int lapic_load_hidden(struct doma
         return -EINVAL;
     }
     s = vcpu_vlapic(v);
+    s->loaded = 1;
     
     if ( hvm_load_entry_zeroextend(LAPIC, h, &s->hw) != 0 ) 
         return -EINVAL;
@@ -1237,6 +1237,7 @@ static int lapic_load_regs(struct domain
         return -EINVAL;
     }
     s = vcpu_vlapic(v);
+    s->loaded = 1;
     
     if ( hvm_load_entry(LAPIC_REGS, h, s->regs) != 0 ) 
         return -EINVAL;
@@ -1254,6 +1255,29 @@ HVM_REGISTER_SAVE_RESTORE(LAPIC, lapic_s
 HVM_REGISTER_SAVE_RESTORE(LAPIC_REGS, lapic_save_regs, lapic_load_regs,
                           1, HVMSR_PER_VCPU);
 
+void vlapic_domain_unpause(const struct domain *d)
+{
+    /*
+     * Following lapic_load_hidden()/lapic_load_regs() we may need to
+     * correct ID and LDR when they come from an old, broken hypervisor.
+     */

This seems like aweful overhead for the domain_{,un}pause() path.  Why can't it be fixed up once in lapic_load_{regs,hidden}(), or possibly deferred to the end of hvm_load()?

+    struct vcpu *v;
+
+    for_each_vcpu ( d, v )
+    {
+        struct vlapic *vlapic = vcpu_vlapic(v);
+        u32 id = vlapic_get_reg(vlapic, APIC_ID);
+
+        if ( vlapic->loaded && vlapic_x2apic_mode(vlapic) &&
+             id && GET_xAPIC_ID(id) == v->vcpu_id * 2 &&
+             id == SET_xAPIC_ID(GET_xAPIC_ID(id)) &&
+             vlapic_get_reg(vlapic, APIC_LDR) == 1 )
+            set_x2apic_id(vlapic);
+
+        vlapic->loaded = 0;
+    }
+}
+
 int vlapic_init(struct vcpu *v)
 {
     struct vlapic *vlapic = vcpu_vlapic(v);
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -952,8 +952,11 @@ void domain_unpause(struct domain *d)
     struct vcpu *v;
 
     if ( atomic_dec_and_test(&d->pause_count) )
+    {
+        arch_domain_unpause(d);
         for_each_vcpu( d, v )
             vcpu_wake(v);
+    }
 }
 
 int __domain_pause_by_systemcontroller(struct domain *d,
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -241,6 +241,8 @@ struct arch_vcpu
 void vcpu_show_execution_state(struct vcpu *);
 void vcpu_show_registers(const struct vcpu *);
 
+#define arch_domain_unpause(d) ((void)(d))
+
 #endif /* __ASM_DOMAIN_H__ */
 
 /*
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -500,6 +500,12 @@ void domain_cpuid(struct domain *d,
                   unsigned int  *ecx,
                   unsigned int  *edx);
 
+#define arch_domain_unpause(d) ({  \
+    const struct domain *d_ = (d); \
+    if ( is_hvm_domain(d_) )       \
+        vlapic_domain_unpause(d_); \
+})
+
 #endif /* __ASM_DOMAIN_H__ */
 
 /*
--- a/xen/include/asm-x86/hvm/vlapic.h
+++ b/xen/include/asm-x86/hvm/vlapic.h
@@ -30,8 +30,9 @@
 #define vlapic_vcpu(x)   (container_of((x), struct vcpu, arch.hvm_vcpu.vlapic))
 #define vlapic_domain(x) (vlapic_vcpu(x)->domain)
 
-#define VLAPIC_ID(vlapic)   \
-    (GET_xAPIC_ID(vlapic_get_reg((vlapic), APIC_ID)))
+#define _VLAPIC_ID(vlapic, id) (vlapic_x2apic_mode(vlapic) \
+                                ? (id) : GET_xAPIC_ID(id))
+#define VLAPIC_ID(vlapic) _VLAPIC_ID(vlapic, vlapic_get_reg(vlapic, APIC_ID))

Some comment regarding the difference between these two?

~Andrew

 
 /*
  * APIC can be disabled in two ways:
@@ -70,6 +71,7 @@
 struct vlapic {
     struct hvm_hw_lapic      hw;
     struct hvm_hw_lapic_regs *regs;
+    bool_t                   loaded;
     struct periodic_time     pt;
     s_time_t                 timer_last_update;
     struct page_info         *regs_page;
@@ -123,11 +125,13 @@ void vlapic_ipi(struct vlapic *vlapic, u
 int vlapic_apicv_write(struct vcpu *v, unsigned int offset);
 
 struct vlapic *vlapic_lowest_prio(
-    struct domain *d, struct vlapic *source,
-    int short_hand, uint8_t dest, uint8_t dest_mode);
+    struct domain *d, const struct vlapic *source,
+    int short_hand, uint32_t dest, bool_t dest_mode);
 
 bool_t vlapic_match_dest(
-    struct vlapic *target, struct vlapic *source,
-    int short_hand, uint8_t dest, uint8_t dest_mode);
+    struct vlapic *target, const struct vlapic *source,
+    int short_hand, uint32_t dest, bool_t dest_mode);
+
+void vlapic_domain_unpause(const struct domain *);
 
 #endif /* __ASM_X86_HVM_VLAPIC_H__ */




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

--------------090503040007080201050406-- --===============8250752557471909694== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============8250752557471909694==--