* [PATCH v3 1/3] x86/HVM: fix miscellaneous aspects of x2APIC emulation
2014-09-12 12:47 [PATCH v3 0/3] x86/HVM: fix various aspects of x2APIC emulation Jan Beulich
@ 2014-09-12 12:52 ` Jan Beulich
2014-09-12 12:53 ` [PATCH v3 2/3] x86/HVM: fix ID handling " Jan Beulich
2014-09-12 12:54 ` [PATCH v3 3/3] x86/vlapic: a few type adjustments Jan Beulich
2 siblings, 0 replies; 6+ messages in thread
From: Jan Beulich @ 2014-09-12 12:52 UTC (permalink / raw)
To: xen-devel; +Cc: Keir Fraser
[-- Attachment #1: Type: text/plain, Size: 9070 bytes --]
- generate #GP on invalid APIC base MSR transitions
- fail reads from the EOI and self-IPI registers (which are write-only)
- handle self-IPI writes and the ICR2 half of ICR writes largely in
hvm_x2apic_msr_write() and (for self-IPI only) vlapic_apicv_write()
- don't permit MMIO-based access in x2APIC mode
- filter writes to read-only registers in hvm_x2apic_msr_write(),
allowing conditionals to be dropped from vlapic_reg_write()
- don't ignore upper half of MSR-based write to ESR being non-zero
- VMX's EXIT_REASON_APIC_WRITE must not result in #GP (this exit being
trap-like, this exception would get raised on the wrong RIP)
- make hvm_x2apic_msr_read() produce X86EMUL_* return codes just like
hvm_x2apic_msr_write() does (benign to the only caller)
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v3: Also handle APIC_EOI in hvm_x2apic_msr_read() as pointed out by
Andrew. Filter MMIO-based accesses in vlapic_range() when in x2APIC
mode. Move x2APIC special casing from vlapic_reg_write() to
hvm_x2apic_msr_write(). Don't open-code vlapic_x2apic_mode().
v2: Split from main patch.
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4499,7 +4499,8 @@ int hvm_msr_write_intercept(unsigned int
break;
case MSR_IA32_APICBASE:
- vlapic_msr_set(vcpu_vlapic(v), msr_content);
+ if ( !vlapic_msr_set(vcpu_vlapic(v), msr_content) )
+ goto gp_fault;
break;
case MSR_IA32_TSC_DEADLINE:
--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -588,7 +588,7 @@ int hvm_x2apic_msr_read(struct vcpu *v,
uint32_t low, high = 0, offset = (msr - MSR_IA32_APICBASE_MSR) << 4;
if ( !vlapic_x2apic_mode(vlapic) )
- return 1;
+ return X86EMUL_UNHANDLEABLE;
vlapic_read_aligned(vlapic, offset, &low);
switch ( offset )
@@ -601,12 +601,15 @@ int hvm_x2apic_msr_read(struct vcpu *v,
vlapic_read_aligned(vlapic, APIC_ICR2, &high);
break;
+ case APIC_EOI:
case APIC_ICR2:
- return 1;
+ case APIC_SELF_IPI:
+ return X86EMUL_UNHANDLEABLE;
}
*msr_content = (((uint64_t)high) << 32) | low;
- return 0;
+
+ return X86EMUL_OKAY;
}
static void vlapic_pt_cb(struct vcpu *v, void *data)
@@ -630,10 +633,7 @@ static int vlapic_reg_write(struct vcpu
switch ( offset )
{
case APIC_ID:
- if ( !vlapic_x2apic_mode(vlapic) )
- vlapic_set_reg(vlapic, APIC_ID, val);
- else
- rc = X86EMUL_UNHANDLEABLE;
+ vlapic_set_reg(vlapic, APIC_ID, val);
break;
case APIC_TASKPRI:
@@ -645,17 +645,11 @@ static int vlapic_reg_write(struct vcpu
break;
case APIC_LDR:
- if ( !vlapic_x2apic_mode(vlapic) )
- vlapic_set_reg(vlapic, APIC_LDR, val & APIC_LDR_MASK);
- else
- rc = X86EMUL_UNHANDLEABLE;
+ vlapic_set_reg(vlapic, APIC_LDR, val & APIC_LDR_MASK);
break;
case APIC_DFR:
- if ( !vlapic_x2apic_mode(vlapic) )
- vlapic_set_reg(vlapic, APIC_DFR, val | 0x0FFFFFFF);
- else
- rc = X86EMUL_UNHANDLEABLE;
+ vlapic_set_reg(vlapic, APIC_DFR, val | 0x0FFFFFFF);
break;
case APIC_SPIV:
@@ -682,21 +676,6 @@ static int vlapic_reg_write(struct vcpu
}
break;
- case APIC_ESR:
- if ( vlapic_x2apic_mode(vlapic) && (val != 0) )
- {
- gdprintk(XENLOG_ERR, "Local APIC write ESR with non-zero %lx\n",
- val);
- rc = X86EMUL_UNHANDLEABLE;
- }
- break;
-
- case APIC_SELF_IPI:
- rc = vlapic_x2apic_mode(vlapic)
- ? vlapic_reg_write(v, APIC_ICR, 0x40000 | (val & 0xff))
- : X86EMUL_UNHANDLEABLE;
- break;
-
case APIC_ICR:
val &= ~(1 << 12); /* always clear the pending bit */
vlapic_ipi(vlapic, val, vlapic_get_reg(vlapic, APIC_ICR2));
@@ -704,9 +683,7 @@ static int vlapic_reg_write(struct vcpu
break;
case APIC_ICR2:
- if ( !vlapic_x2apic_mode(vlapic) )
- val &= 0xff000000;
- vlapic_set_reg(vlapic, APIC_ICR2, val);
+ vlapic_set_reg(vlapic, APIC_ICR2, val & 0xff000000);
break;
case APIC_LVTT: /* LVT Timer Reg */
@@ -851,8 +828,16 @@ static int vlapic_write(struct vcpu *v,
int vlapic_apicv_write(struct vcpu *v, unsigned int offset)
{
- uint32_t val = vlapic_get_reg(vcpu_vlapic(v), offset);
- return vlapic_reg_write(v, offset, val);
+ struct vlapic *vlapic = vcpu_vlapic(v);
+ uint32_t val = vlapic_get_reg(vlapic, offset);
+
+ if ( !vlapic_x2apic_mode(vlapic) )
+ return vlapic_reg_write(v, offset, val);
+
+ if ( offset != APIC_SELF_IPI )
+ return X86EMUL_UNHANDLEABLE;
+
+ return vlapic_reg_write(v, APIC_ICR, APIC_DEST_SELF | (uint8_t)val);
}
int hvm_x2apic_msr_write(struct vcpu *v, unsigned int msr, uint64_t msr_content)
@@ -865,16 +850,33 @@ int hvm_x2apic_msr_write(struct vcpu *v,
switch ( offset )
{
- int rc;
+ case APIC_TASKPRI:
+ case APIC_EOI:
+ case APIC_SPIV:
+ case APIC_CMCI:
+ case APIC_LVTT ... APIC_LVTERR:
+ case APIC_TMICT:
+ case APIC_TMCCT:
+ case APIC_TDCR:
+ break;
case APIC_ICR:
- rc = vlapic_reg_write(v, APIC_ICR2, (uint32_t)(msr_content >> 32));
- if ( rc )
- return rc;
+ vlapic_set_reg(vlapic, APIC_ICR2, msr_content >> 32);
break;
- case APIC_ICR2:
- return X86EMUL_UNHANDLEABLE;
+ case APIC_SELF_IPI:
+ offset = APIC_ICR;
+ msr_content = APIC_DEST_SELF | (uint8_t)msr_content;
+ break;
+
+ case APIC_ESR:
+ if ( msr_content )
+ {
+ printk(XENLOG_G_WARNING "%pv: non-zero (%lx) LAPIC ESR write\n",
+ v, msr_content);
+ default:
+ return X86EMUL_UNHANDLEABLE;
+ }
}
return vlapic_reg_write(v, offset, (uint32_t)msr_content);
@@ -884,7 +886,10 @@ static int vlapic_range(struct vcpu *v,
{
struct vlapic *vlapic = vcpu_vlapic(v);
unsigned long offset = addr - vlapic_base_address(vlapic);
- return (!vlapic_hw_disabled(vlapic) && (offset < PAGE_SIZE));
+
+ return !vlapic_hw_disabled(vlapic) &&
+ !vlapic_x2apic_mode(vlapic) &&
+ (offset < PAGE_SIZE);
}
const struct hvm_mmio_handler vlapic_mmio_handler = {
@@ -893,10 +898,12 @@ const struct hvm_mmio_handler vlapic_mmi
.write_handler = vlapic_write
};
-void vlapic_msr_set(struct vlapic *vlapic, uint64_t value)
+bool_t vlapic_msr_set(struct vlapic *vlapic, uint64_t value)
{
if ( (vlapic->hw.apic_base_msr ^ value) & MSR_IA32_APICBASE_ENABLE )
{
+ if ( unlikely(value & MSR_IA32_APICBASE_EXTD) )
+ return 0;
if ( value & MSR_IA32_APICBASE_ENABLE )
{
vlapic_reset(vlapic);
@@ -905,10 +912,15 @@ void vlapic_msr_set(struct vlapic *vlapi
}
else
{
+ if ( unlikely(vlapic_x2apic_mode(vlapic)) )
+ return 0;
vlapic->hw.disabled |= VLAPIC_HW_DISABLED;
pt_may_unmask_irq(vlapic_domain(vlapic), NULL);
}
}
+ else if ( !(value & MSR_IA32_APICBASE_ENABLE) &&
+ unlikely(value & MSR_IA32_APICBASE_EXTD) )
+ return 0;
vlapic->hw.apic_base_msr = value;
@@ -923,6 +935,8 @@ void vlapic_msr_set(struct vlapic *vlapi
HVM_DBG_LOG(DBG_LEVEL_VLAPIC,
"apic base msr is 0x%016"PRIx64, vlapic->hw.apic_base_msr);
+
+ return 1;
}
uint64_t vlapic_tdt_msr_get(struct vlapic *vlapic)
@@ -1206,6 +1220,10 @@ static int lapic_load_hidden(struct doma
if ( hvm_load_entry_zeroextend(LAPIC, h, &s->hw) != 0 )
return -EINVAL;
+ if ( !(s->hw.apic_base_msr & MSR_IA32_APICBASE_ENABLE) &&
+ unlikely(vlapic_x2apic_mode(s)) )
+ return -EINVAL;
+
vmx_vlapic_msr_changed(v);
return 0;
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -3096,8 +3096,7 @@ void vmx_vmexit_handler(struct cpu_user_
break;
case EXIT_REASON_APIC_WRITE:
- if ( vmx_handle_apic_write() )
- hvm_inject_hw_exception(TRAP_gp_fault, 0);
+ vmx_handle_apic_write();
break;
case EXIT_REASON_ACCESS_GDTR_OR_IDTR:
--- a/xen/include/asm-x86/hvm/vlapic.h
+++ b/xen/include/asm-x86/hvm/vlapic.h
@@ -106,7 +106,7 @@ void vlapic_destroy(struct vcpu *v);
void vlapic_reset(struct vlapic *vlapic);
-void vlapic_msr_set(struct vlapic *vlapic, uint64_t value);
+bool_t vlapic_msr_set(struct vlapic *vlapic, uint64_t value);
void vlapic_tdt_msr_set(struct vlapic *vlapic, uint64_t value);
uint64_t vlapic_tdt_msr_get(struct vlapic *vlapic);
[-- Attachment #2: x86-HVM-x2APIC-misc.patch --]
[-- Type: text/plain, Size: 9124 bytes --]
x86/HVM: fix miscellaneous aspects of x2APIC emulation
- generate #GP on invalid APIC base MSR transitions
- fail reads from the EOI and self-IPI registers (which are write-only)
- handle self-IPI writes and the ICR2 half of ICR writes largely in
hvm_x2apic_msr_write() and (for self-IPI only) vlapic_apicv_write()
- don't permit MMIO-based access in x2APIC mode
- filter writes to read-only registers in hvm_x2apic_msr_write(),
allowing conditionals to be dropped from vlapic_reg_write()
- don't ignore upper half of MSR-based write to ESR being non-zero
- VMX's EXIT_REASON_APIC_WRITE must not result in #GP (this exit being
trap-like, this exception would get raised on the wrong RIP)
- make hvm_x2apic_msr_read() produce X86EMUL_* return codes just like
hvm_x2apic_msr_write() does (benign to the only caller)
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v3: Also handle APIC_EOI in hvm_x2apic_msr_read() as pointed out by
Andrew. Filter MMIO-based accesses in vlapic_range() when in x2APIC
mode. Move x2APIC special casing from vlapic_reg_write() to
hvm_x2apic_msr_write(). Don't open-code vlapic_x2apic_mode().
v2: Split from main patch.
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4499,7 +4499,8 @@ int hvm_msr_write_intercept(unsigned int
break;
case MSR_IA32_APICBASE:
- vlapic_msr_set(vcpu_vlapic(v), msr_content);
+ if ( !vlapic_msr_set(vcpu_vlapic(v), msr_content) )
+ goto gp_fault;
break;
case MSR_IA32_TSC_DEADLINE:
--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -588,7 +588,7 @@ int hvm_x2apic_msr_read(struct vcpu *v,
uint32_t low, high = 0, offset = (msr - MSR_IA32_APICBASE_MSR) << 4;
if ( !vlapic_x2apic_mode(vlapic) )
- return 1;
+ return X86EMUL_UNHANDLEABLE;
vlapic_read_aligned(vlapic, offset, &low);
switch ( offset )
@@ -601,12 +601,15 @@ int hvm_x2apic_msr_read(struct vcpu *v,
vlapic_read_aligned(vlapic, APIC_ICR2, &high);
break;
+ case APIC_EOI:
case APIC_ICR2:
- return 1;
+ case APIC_SELF_IPI:
+ return X86EMUL_UNHANDLEABLE;
}
*msr_content = (((uint64_t)high) << 32) | low;
- return 0;
+
+ return X86EMUL_OKAY;
}
static void vlapic_pt_cb(struct vcpu *v, void *data)
@@ -630,10 +633,7 @@ static int vlapic_reg_write(struct vcpu
switch ( offset )
{
case APIC_ID:
- if ( !vlapic_x2apic_mode(vlapic) )
- vlapic_set_reg(vlapic, APIC_ID, val);
- else
- rc = X86EMUL_UNHANDLEABLE;
+ vlapic_set_reg(vlapic, APIC_ID, val);
break;
case APIC_TASKPRI:
@@ -645,17 +645,11 @@ static int vlapic_reg_write(struct vcpu
break;
case APIC_LDR:
- if ( !vlapic_x2apic_mode(vlapic) )
- vlapic_set_reg(vlapic, APIC_LDR, val & APIC_LDR_MASK);
- else
- rc = X86EMUL_UNHANDLEABLE;
+ vlapic_set_reg(vlapic, APIC_LDR, val & APIC_LDR_MASK);
break;
case APIC_DFR:
- if ( !vlapic_x2apic_mode(vlapic) )
- vlapic_set_reg(vlapic, APIC_DFR, val | 0x0FFFFFFF);
- else
- rc = X86EMUL_UNHANDLEABLE;
+ vlapic_set_reg(vlapic, APIC_DFR, val | 0x0FFFFFFF);
break;
case APIC_SPIV:
@@ -682,21 +676,6 @@ static int vlapic_reg_write(struct vcpu
}
break;
- case APIC_ESR:
- if ( vlapic_x2apic_mode(vlapic) && (val != 0) )
- {
- gdprintk(XENLOG_ERR, "Local APIC write ESR with non-zero %lx\n",
- val);
- rc = X86EMUL_UNHANDLEABLE;
- }
- break;
-
- case APIC_SELF_IPI:
- rc = vlapic_x2apic_mode(vlapic)
- ? vlapic_reg_write(v, APIC_ICR, 0x40000 | (val & 0xff))
- : X86EMUL_UNHANDLEABLE;
- break;
-
case APIC_ICR:
val &= ~(1 << 12); /* always clear the pending bit */
vlapic_ipi(vlapic, val, vlapic_get_reg(vlapic, APIC_ICR2));
@@ -704,9 +683,7 @@ static int vlapic_reg_write(struct vcpu
break;
case APIC_ICR2:
- if ( !vlapic_x2apic_mode(vlapic) )
- val &= 0xff000000;
- vlapic_set_reg(vlapic, APIC_ICR2, val);
+ vlapic_set_reg(vlapic, APIC_ICR2, val & 0xff000000);
break;
case APIC_LVTT: /* LVT Timer Reg */
@@ -851,8 +828,16 @@ static int vlapic_write(struct vcpu *v,
int vlapic_apicv_write(struct vcpu *v, unsigned int offset)
{
- uint32_t val = vlapic_get_reg(vcpu_vlapic(v), offset);
- return vlapic_reg_write(v, offset, val);
+ struct vlapic *vlapic = vcpu_vlapic(v);
+ uint32_t val = vlapic_get_reg(vlapic, offset);
+
+ if ( !vlapic_x2apic_mode(vlapic) )
+ return vlapic_reg_write(v, offset, val);
+
+ if ( offset != APIC_SELF_IPI )
+ return X86EMUL_UNHANDLEABLE;
+
+ return vlapic_reg_write(v, APIC_ICR, APIC_DEST_SELF | (uint8_t)val);
}
int hvm_x2apic_msr_write(struct vcpu *v, unsigned int msr, uint64_t msr_content)
@@ -865,16 +850,33 @@ int hvm_x2apic_msr_write(struct vcpu *v,
switch ( offset )
{
- int rc;
+ case APIC_TASKPRI:
+ case APIC_EOI:
+ case APIC_SPIV:
+ case APIC_CMCI:
+ case APIC_LVTT ... APIC_LVTERR:
+ case APIC_TMICT:
+ case APIC_TMCCT:
+ case APIC_TDCR:
+ break;
case APIC_ICR:
- rc = vlapic_reg_write(v, APIC_ICR2, (uint32_t)(msr_content >> 32));
- if ( rc )
- return rc;
+ vlapic_set_reg(vlapic, APIC_ICR2, msr_content >> 32);
break;
- case APIC_ICR2:
- return X86EMUL_UNHANDLEABLE;
+ case APIC_SELF_IPI:
+ offset = APIC_ICR;
+ msr_content = APIC_DEST_SELF | (uint8_t)msr_content;
+ break;
+
+ case APIC_ESR:
+ if ( msr_content )
+ {
+ printk(XENLOG_G_WARNING "%pv: non-zero (%lx) LAPIC ESR write\n",
+ v, msr_content);
+ default:
+ return X86EMUL_UNHANDLEABLE;
+ }
}
return vlapic_reg_write(v, offset, (uint32_t)msr_content);
@@ -884,7 +886,10 @@ static int vlapic_range(struct vcpu *v,
{
struct vlapic *vlapic = vcpu_vlapic(v);
unsigned long offset = addr - vlapic_base_address(vlapic);
- return (!vlapic_hw_disabled(vlapic) && (offset < PAGE_SIZE));
+
+ return !vlapic_hw_disabled(vlapic) &&
+ !vlapic_x2apic_mode(vlapic) &&
+ (offset < PAGE_SIZE);
}
const struct hvm_mmio_handler vlapic_mmio_handler = {
@@ -893,10 +898,12 @@ const struct hvm_mmio_handler vlapic_mmi
.write_handler = vlapic_write
};
-void vlapic_msr_set(struct vlapic *vlapic, uint64_t value)
+bool_t vlapic_msr_set(struct vlapic *vlapic, uint64_t value)
{
if ( (vlapic->hw.apic_base_msr ^ value) & MSR_IA32_APICBASE_ENABLE )
{
+ if ( unlikely(value & MSR_IA32_APICBASE_EXTD) )
+ return 0;
if ( value & MSR_IA32_APICBASE_ENABLE )
{
vlapic_reset(vlapic);
@@ -905,10 +912,15 @@ void vlapic_msr_set(struct vlapic *vlapi
}
else
{
+ if ( unlikely(vlapic_x2apic_mode(vlapic)) )
+ return 0;
vlapic->hw.disabled |= VLAPIC_HW_DISABLED;
pt_may_unmask_irq(vlapic_domain(vlapic), NULL);
}
}
+ else if ( !(value & MSR_IA32_APICBASE_ENABLE) &&
+ unlikely(value & MSR_IA32_APICBASE_EXTD) )
+ return 0;
vlapic->hw.apic_base_msr = value;
@@ -923,6 +935,8 @@ void vlapic_msr_set(struct vlapic *vlapi
HVM_DBG_LOG(DBG_LEVEL_VLAPIC,
"apic base msr is 0x%016"PRIx64, vlapic->hw.apic_base_msr);
+
+ return 1;
}
uint64_t vlapic_tdt_msr_get(struct vlapic *vlapic)
@@ -1206,6 +1220,10 @@ static int lapic_load_hidden(struct doma
if ( hvm_load_entry_zeroextend(LAPIC, h, &s->hw) != 0 )
return -EINVAL;
+ if ( !(s->hw.apic_base_msr & MSR_IA32_APICBASE_ENABLE) &&
+ unlikely(vlapic_x2apic_mode(s)) )
+ return -EINVAL;
+
vmx_vlapic_msr_changed(v);
return 0;
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -3096,8 +3096,7 @@ void vmx_vmexit_handler(struct cpu_user_
break;
case EXIT_REASON_APIC_WRITE:
- if ( vmx_handle_apic_write() )
- hvm_inject_hw_exception(TRAP_gp_fault, 0);
+ vmx_handle_apic_write();
break;
case EXIT_REASON_ACCESS_GDTR_OR_IDTR:
--- a/xen/include/asm-x86/hvm/vlapic.h
+++ b/xen/include/asm-x86/hvm/vlapic.h
@@ -106,7 +106,7 @@ void vlapic_destroy(struct vcpu *v);
void vlapic_reset(struct vlapic *vlapic);
-void vlapic_msr_set(struct vlapic *vlapic, uint64_t value);
+bool_t vlapic_msr_set(struct vlapic *vlapic, uint64_t value);
void vlapic_tdt_msr_set(struct vlapic *vlapic, uint64_t value);
uint64_t vlapic_tdt_msr_get(struct vlapic *vlapic);
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v3 2/3] x86/HVM: fix ID handling of x2APIC emulation
2014-09-12 12:47 [PATCH v3 0/3] x86/HVM: fix various aspects of x2APIC emulation Jan Beulich
2014-09-12 12:52 ` [PATCH v3 1/3] x86/HVM: fix miscellaneous " Jan Beulich
@ 2014-09-12 12:53 ` Jan Beulich
2014-09-12 14:49 ` Jan Beulich
2014-09-12 12:54 ` [PATCH v3 3/3] x86/vlapic: a few type adjustments Jan Beulich
2 siblings, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2014-09-12 12:53 UTC (permalink / raw)
To: xen-devel; +Cc: Keir Fraser
[-- Attachment #1: Type: text/plain, Size: 9612 bytes --]
- 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)
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. This post-migration fixup involves introducing
arch_domain_unpause(), needed here to fix up state after all
intended state setting was done (any suggestion as to how else to
accomplish this is very welcome).
--- 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)
{
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;
@@ -898,6 +892,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);
+ 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 )
@@ -925,11 +928,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));
@@ -1216,6 +1215,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;
@@ -1244,6 +1244,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;
@@ -1261,6 +1262,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.
+ */
+ 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
@@ -949,8 +949,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
@@ -499,6 +499,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))
/*
* 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__ */
[-- Attachment #2: x86-HVM-x2APIC-id.patch --]
[-- Type: text/plain, Size: 9656 bytes --]
x86/HVM: fix ID handling of x2APIC emulation
- 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)
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. This post-migration fixup involves introducing
arch_domain_unpause(), needed here to fix up state after all
intended state setting was done (any suggestion as to how else to
accomplish this is very welcome).
--- 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)
{
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;
@@ -898,6 +892,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);
+ 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 )
@@ -925,11 +928,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));
@@ -1216,6 +1215,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;
@@ -1244,6 +1244,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;
@@ -1261,6 +1262,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.
+ */
+ 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
@@ -949,8 +949,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
@@ -499,6 +499,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))
/*
* 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__ */
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v3 2/3] x86/HVM: fix ID handling of x2APIC emulation
2014-09-12 12:53 ` [PATCH v3 2/3] x86/HVM: fix ID handling " Jan Beulich
@ 2014-09-12 14:49 ` Jan Beulich
0 siblings, 0 replies; 6+ messages in thread
From: Jan Beulich @ 2014-09-12 14:49 UTC (permalink / raw)
To: xen-devel
Cc: Ian Campbell, Ian Jackson, Keir Fraser, Stefano Stabellini,
Tim Deegan
>>> On 12.09.14 at 14:53, <JBeulich@suse.com> 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)
>
> 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. This post-migration fixup involves introducing
> arch_domain_unpause(), needed here to fix up state after all
> intended state setting was done (any suggestion as to how else to
> accomplish this is very welcome).
Oops - just realized the quoted portion below needs a wider Cc list.
Jan
> --- 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/common/domain.c
> +++ b/xen/common/domain.c
> @@ -949,8 +949,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,
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3 3/3] x86/vlapic: a few type adjustments
2014-09-12 12:47 [PATCH v3 0/3] x86/HVM: fix various aspects of x2APIC emulation Jan Beulich
2014-09-12 12:52 ` [PATCH v3 1/3] x86/HVM: fix miscellaneous " Jan Beulich
2014-09-12 12:53 ` [PATCH v3 2/3] x86/HVM: fix ID handling " Jan Beulich
@ 2014-09-12 12:54 ` Jan Beulich
2014-09-15 15:15 ` Andrew Cooper
2 siblings, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2014-09-12 12:54 UTC (permalink / raw)
To: xen-devel; +Cc: Keir Fraser
[-- Attachment #1: Type: text/plain, Size: 2815 bytes --]
Constify a couple of pointer parameters, convert a boolean function
return type to bool_t, and clean up a printk() being touched anyway.
Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -173,9 +173,9 @@ uint32_t vlapic_set_ppr(struct vlapic *v
return ppr;
}
-static int vlapic_match_logical_addr(struct vlapic *vlapic, uint32_t mda)
+static bool_t vlapic_match_logical_addr(const struct vlapic *vlapic, uint32_t mda)
{
- int result = 0;
+ bool_t result = 0;
uint32_t logical_id = vlapic_get_reg(vlapic, APIC_LDR);
if ( vlapic_x2apic_mode(vlapic) )
@@ -196,9 +196,9 @@ static int vlapic_match_logical_addr(str
result = 1;
break;
default:
- gdprintk(XENLOG_WARNING, "Bad DFR value for lapic of vcpu %d: %08x\n",
- vlapic_vcpu(vlapic)->vcpu_id,
- vlapic_get_reg(vlapic, APIC_DFR));
+ printk(XENLOG_G_WARNING "%pv: bad LAPIC DFR value %08x\n",
+ const_vlapic_vcpu(vlapic),
+ vlapic_get_reg(vlapic, APIC_DFR));
break;
}
@@ -206,7 +206,7 @@ static int vlapic_match_logical_addr(str
}
bool_t vlapic_match_dest(
- struct vlapic *target, const struct vlapic *source,
+ const struct vlapic *target, const struct vlapic *source,
int short_hand, uint32_t dest, bool_t dest_mode)
{
HVM_DBG_LOG(DBG_LEVEL_VLAPIC, "target %p, source %p, dest %#x, "
--- a/xen/include/asm-x86/hvm/vlapic.h
+++ b/xen/include/asm-x86/hvm/vlapic.h
@@ -28,6 +28,8 @@
#define vcpu_vlapic(x) (&(x)->arch.hvm_vcpu.vlapic)
#define vlapic_vcpu(x) (container_of((x), struct vcpu, arch.hvm_vcpu.vlapic))
+#define const_vlapic_vcpu(x) (container_of((x), const struct vcpu, \
+ arch.hvm_vcpu.vlapic))
#define vlapic_domain(x) (vlapic_vcpu(x)->domain)
#define _VLAPIC_ID(vlapic, id) (vlapic_x2apic_mode(vlapic) \
@@ -85,7 +87,8 @@ struct vlapic {
/* vlapic's frequence is 100 MHz */
#define APIC_BUS_CYCLE_NS 10
-static inline uint32_t vlapic_get_reg(struct vlapic *vlapic, uint32_t reg)
+static inline uint32_t vlapic_get_reg(const struct vlapic *vlapic,
+ uint32_t reg)
{
return *((uint32_t *)(&vlapic->regs->data[reg]));
}
@@ -129,7 +132,7 @@ struct vlapic *vlapic_lowest_prio(
int short_hand, uint32_t dest, bool_t dest_mode);
bool_t vlapic_match_dest(
- struct vlapic *target, const struct vlapic *source,
+ const struct vlapic *target, const struct vlapic *source,
int short_hand, uint32_t dest, bool_t dest_mode);
void vlapic_domain_unpause(const struct domain *);
[-- Attachment #2: x86-HVM-x2APIC-types.patch --]
[-- Type: text/plain, Size: 2847 bytes --]
x86/vlapic: a few type adjustments
Constify a couple of pointer parameters, convert a boolean function
return type to bool_t, and clean up a printk() being touched anyway.
Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -173,9 +173,9 @@ uint32_t vlapic_set_ppr(struct vlapic *v
return ppr;
}
-static int vlapic_match_logical_addr(struct vlapic *vlapic, uint32_t mda)
+static bool_t vlapic_match_logical_addr(const struct vlapic *vlapic, uint32_t mda)
{
- int result = 0;
+ bool_t result = 0;
uint32_t logical_id = vlapic_get_reg(vlapic, APIC_LDR);
if ( vlapic_x2apic_mode(vlapic) )
@@ -196,9 +196,9 @@ static int vlapic_match_logical_addr(str
result = 1;
break;
default:
- gdprintk(XENLOG_WARNING, "Bad DFR value for lapic of vcpu %d: %08x\n",
- vlapic_vcpu(vlapic)->vcpu_id,
- vlapic_get_reg(vlapic, APIC_DFR));
+ printk(XENLOG_G_WARNING "%pv: bad LAPIC DFR value %08x\n",
+ const_vlapic_vcpu(vlapic),
+ vlapic_get_reg(vlapic, APIC_DFR));
break;
}
@@ -206,7 +206,7 @@ static int vlapic_match_logical_addr(str
}
bool_t vlapic_match_dest(
- struct vlapic *target, const struct vlapic *source,
+ const struct vlapic *target, const struct vlapic *source,
int short_hand, uint32_t dest, bool_t dest_mode)
{
HVM_DBG_LOG(DBG_LEVEL_VLAPIC, "target %p, source %p, dest %#x, "
--- a/xen/include/asm-x86/hvm/vlapic.h
+++ b/xen/include/asm-x86/hvm/vlapic.h
@@ -28,6 +28,8 @@
#define vcpu_vlapic(x) (&(x)->arch.hvm_vcpu.vlapic)
#define vlapic_vcpu(x) (container_of((x), struct vcpu, arch.hvm_vcpu.vlapic))
+#define const_vlapic_vcpu(x) (container_of((x), const struct vcpu, \
+ arch.hvm_vcpu.vlapic))
#define vlapic_domain(x) (vlapic_vcpu(x)->domain)
#define _VLAPIC_ID(vlapic, id) (vlapic_x2apic_mode(vlapic) \
@@ -85,7 +87,8 @@ struct vlapic {
/* vlapic's frequence is 100 MHz */
#define APIC_BUS_CYCLE_NS 10
-static inline uint32_t vlapic_get_reg(struct vlapic *vlapic, uint32_t reg)
+static inline uint32_t vlapic_get_reg(const struct vlapic *vlapic,
+ uint32_t reg)
{
return *((uint32_t *)(&vlapic->regs->data[reg]));
}
@@ -129,7 +132,7 @@ struct vlapic *vlapic_lowest_prio(
int short_hand, uint32_t dest, bool_t dest_mode);
bool_t vlapic_match_dest(
- struct vlapic *target, const struct vlapic *source,
+ const struct vlapic *target, const struct vlapic *source,
int short_hand, uint32_t dest, bool_t dest_mode);
void vlapic_domain_unpause(const struct domain *);
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v3 3/3] x86/vlapic: a few type adjustments
2014-09-12 12:54 ` [PATCH v3 3/3] x86/vlapic: a few type adjustments Jan Beulich
@ 2014-09-15 15:15 ` Andrew Cooper
0 siblings, 0 replies; 6+ messages in thread
From: Andrew Cooper @ 2014-09-15 15:15 UTC (permalink / raw)
To: Jan Beulich, xen-devel; +Cc: Keir Fraser
[-- Attachment #1.1: Type: text/plain, Size: 3158 bytes --]
On 12/09/2014 13:54, Jan Beulich wrote:
> Constify a couple of pointer parameters, convert a boolean function
> return type to bool_t, and clean up a printk() being touched anyway.
>
> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
>
> --- a/xen/arch/x86/hvm/vlapic.c
> +++ b/xen/arch/x86/hvm/vlapic.c
> @@ -173,9 +173,9 @@ uint32_t vlapic_set_ppr(struct vlapic *v
> return ppr;
> }
>
> -static int vlapic_match_logical_addr(struct vlapic *vlapic, uint32_t mda)
> +static bool_t vlapic_match_logical_addr(const struct vlapic *vlapic, uint32_t mda)
> {
> - int result = 0;
> + bool_t result = 0;
> uint32_t logical_id = vlapic_get_reg(vlapic, APIC_LDR);
>
> if ( vlapic_x2apic_mode(vlapic) )
> @@ -196,9 +196,9 @@ static int vlapic_match_logical_addr(str
> result = 1;
> break;
> default:
> - gdprintk(XENLOG_WARNING, "Bad DFR value for lapic of vcpu %d: %08x\n",
> - vlapic_vcpu(vlapic)->vcpu_id,
> - vlapic_get_reg(vlapic, APIC_DFR));
> + printk(XENLOG_G_WARNING "%pv: bad LAPIC DFR value %08x\n",
> + const_vlapic_vcpu(vlapic),
> + vlapic_get_reg(vlapic, APIC_DFR));
> break;
> }
>
> @@ -206,7 +206,7 @@ static int vlapic_match_logical_addr(str
> }
>
> bool_t vlapic_match_dest(
> - struct vlapic *target, const struct vlapic *source,
> + const struct vlapic *target, const struct vlapic *source,
> int short_hand, uint32_t dest, bool_t dest_mode)
> {
> HVM_DBG_LOG(DBG_LEVEL_VLAPIC, "target %p, source %p, dest %#x, "
> --- a/xen/include/asm-x86/hvm/vlapic.h
> +++ b/xen/include/asm-x86/hvm/vlapic.h
> @@ -28,6 +28,8 @@
>
> #define vcpu_vlapic(x) (&(x)->arch.hvm_vcpu.vlapic)
> #define vlapic_vcpu(x) (container_of((x), struct vcpu, arch.hvm_vcpu.vlapic))
> +#define const_vlapic_vcpu(x) (container_of((x), const struct vcpu, \
> + arch.hvm_vcpu.vlapic))
> #define vlapic_domain(x) (vlapic_vcpu(x)->domain)
>
> #define _VLAPIC_ID(vlapic, id) (vlapic_x2apic_mode(vlapic) \
> @@ -85,7 +87,8 @@ struct vlapic {
> /* vlapic's frequence is 100 MHz */
> #define APIC_BUS_CYCLE_NS 10
>
> -static inline uint32_t vlapic_get_reg(struct vlapic *vlapic, uint32_t reg)
> +static inline uint32_t vlapic_get_reg(const struct vlapic *vlapic,
> + uint32_t reg)
> {
> return *((uint32_t *)(&vlapic->regs->data[reg]));
> }
> @@ -129,7 +132,7 @@ struct vlapic *vlapic_lowest_prio(
> int short_hand, uint32_t dest, bool_t dest_mode);
>
> bool_t vlapic_match_dest(
> - struct vlapic *target, const struct vlapic *source,
> + const struct vlapic *target, const struct vlapic *source,
> int short_hand, uint32_t dest, bool_t dest_mode);
>
> void vlapic_domain_unpause(const struct domain *);
>
>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
[-- Attachment #1.2: Type: text/html, Size: 3967 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread