For those who want to test it, here is the slightly update patch.
NOTE: this version doesn't solve the potential infinite loop
which Alex is suspecting about.
IA64: fix emulation of fp emulation
This patch fixes bug reported as
http://bugzilla.xensource.com/bugzilla/show_bug.cgi?id=1392
When pv domain case, FPSWA hypercall isn't implemented properly.
So avoid the injecting floating point fault/trap at this moment.
However this may cause infinite loop depending on dtlb cache.
The right fix is to implement the hypercall properly however
it wouldn't be very straight forward because the argument
of fpswa is large and includes pointers.
When hvm domain case, floating point trap case iip was incremented
improperly. removed the increment
Signed-off-by: Isaku Yamahata <yamahata@xxxxxxxxxxxxx>
diff --git a/xen/arch/ia64/vmx/vmx_fault.c b/xen/arch/ia64/vmx/vmx_fault.c
--- a/xen/arch/ia64/vmx/vmx_fault.c
+++ b/xen/arch/ia64/vmx/vmx_fault.c
@@ -130,10 +130,8 @@ void vmx_reflect_interruption(u64 ifa, u
status = handle_fpu_swa(0, regs, isr);
if (!status)
return;
- else if (IA64_RETRY == status) {
- vcpu_decrement_iip(vcpu);
+ else if (IA64_RETRY == status)
return;
- }
break;
case 29: // IA64_DEBUG_VECTOR
diff --git a/xen/arch/ia64/xen/faults.c b/xen/arch/ia64/xen/faults.c
--- a/xen/arch/ia64/xen/faults.c
+++ b/xen/arch/ia64/xen/faults.c
@@ -318,6 +318,7 @@ handle_fpu_swa(int fp_fault, struct pt_r
IA64_BUNDLE bundle;
unsigned long fault_ip;
fpswa_ret_t ret;
+ unsigned long rc;
fault_ip = regs->cr_iip;
/*
@@ -328,16 +329,15 @@ handle_fpu_swa(int fp_fault, struct pt_r
if (!fp_fault && (ia64_psr(regs)->ri == 0))
fault_ip -= 16;
- if (VMX_DOMAIN(current)) {
- if (IA64_RETRY == __vmx_get_domain_bundle(fault_ip, &bundle))
- return IA64_RETRY;
- } else
- bundle = __get_domain_bundle(fault_ip);
-
- if (!bundle.i64[0] && !bundle.i64[1]) {
- printk("%s: floating-point bundle at 0x%lx not mapped\n",
- __FUNCTION__, fault_ip);
- return -1;
+ if (VMX_DOMAIN(current))
+ rc = __vmx_get_domain_bundle(fault_ip, &bundle);
+ else
+ rc = __get_domain_bundle(fault_ip, &bundle);
+ if (rc == IA64_RETRY) {
+ gdprintk(XENLOG_DEBUG,
+ "%s(%s): floating-point bundle at 0x%lx not mapped\n",
+ __FUNCTION__, fp_fault ? "fault" : "trap", fault_ip);
+ return IA64_RETRY;
}
ret = fp_emulate(fp_fault, &bundle, ®s->cr_ipsr, ®s->ar_fpsr,
diff --git a/xen/arch/ia64/xen/vcpu.c b/xen/arch/ia64/xen/vcpu.c
--- a/xen/arch/ia64/xen/vcpu.c
+++ b/xen/arch/ia64/xen/vcpu.c
@@ -1325,6 +1325,16 @@ static TR_ENTRY *vcpu_tr_lookup(VCPU * v
return NULL;
}
+unsigned long
+__get_domain_bundle(unsigned long iip, IA64_BUNDLE *bundle)
+{
+ *bundle = __get_domain_bundle_asm(iip);
+ if (!bundle->i64[0] && !bundle->i64[1])
+ return IA64_RETRY;
+
+ return 0;
+}
+
// return value
// 0: failure
// 1: success
@@ -1335,6 +1345,7 @@ vcpu_get_domain_bundle(VCPU * vcpu, REGS
u64 gpip; // guest pseudo phyiscal ip
unsigned long vaddr;
struct page_info *page;
+ unsigned long rc;
again:
#if 0
@@ -1387,11 +1398,11 @@ vcpu_get_domain_bundle(VCPU * vcpu, REGS
if (swap_rr0) {
set_virtual_rr0();
}
- *bundle = __get_domain_bundle(gip);
+ rc = __get_domain_bundle(gip, bundle);
if (swap_rr0) {
set_metaphysical_rr0();
}
- if (bundle->i64[0] == 0 && bundle->i64[1] == 0) {
+ if (rc == IA64_RETRY) {
dprintk(XENLOG_INFO, "%s gip 0x%lx\n", __func__, gip);
return 0;
}
diff --git a/xen/arch/ia64/xen/xenasm.S b/xen/arch/ia64/xen/xenasm.S
--- a/xen/arch/ia64/xen/xenasm.S
+++ b/xen/arch/ia64/xen/xenasm.S
@@ -389,7 +389,7 @@ END(ia64_prepare_handle_reflection)
END(ia64_prepare_handle_reflection)
#endif
-GLOBAL_ENTRY(__get_domain_bundle)
+GLOBAL_ENTRY(__get_domain_bundle_asm)
EX(.failure_in_get_bundle,ld8 r8=[r32],8)
;;
EX(.failure_in_get_bundle,ld8 r9=[r32])
@@ -403,7 +403,7 @@ GLOBAL_ENTRY(__get_domain_bundle)
;;
br.ret.sptk.many rp
;;
-END(__get_domain_bundle)
+END(__get_domain_bundle_asm)
/* derived from linux/arch/ia64/hp/sim/boot/boot_head.S */
GLOBAL_ENTRY(pal_emulator_static)
diff --git a/xen/include/asm-ia64/bundle.h b/xen/include/asm-ia64/bundle.h
--- a/xen/include/asm-ia64/bundle.h
+++ b/xen/include/asm-ia64/bundle.h
@@ -225,7 +225,8 @@ typedef union U_INST64 {
#ifdef __XEN__
extern unsigned long __vmx_get_domain_bundle(unsigned long iip, IA64_BUNDLE
*pbundle);
-extern IA64_BUNDLE __get_domain_bundle(unsigned long iip);
+extern IA64_BUNDLE __get_domain_bundle_asm(unsigned long iip);
+extern unsigned long __get_domain_bundle(unsigned long iip, IA64_BUNDLE
*bundle);
#endif
#define MASK_41 ((unsigned long)0x1ffffffffff)
On Fri, Dec 05, 2008 at 02:29:32PM +0900, Isaku Yamahata wrote:
> On Thu, Dec 04, 2008 at 09:10:48AM -0700, Alex Williamson wrote:
> > On Thu, 2008-12-04 at 15:29 +0900, Isaku Yamahata wrote:
> > > Although I also replied by the bugzilla,
> > > I also send the patch to the list for those who doesn't
> > > watch on the bug report.
> > > I hope this patch fixes it, please try this.
> >
> > Hi Isaku,
> >
> > Thanks for looking at this. With your patch, it doesn't fail, but I
> > regularly see cases where it doesn't complete, at least not in a
> > reasonable time frame. I imagine it's getting stuck in an endless loop
> > of retries.
>
> Hmm, I have been afraid of that. But I haven't observed it myself yet.
> It's probably because I've tested it without any other workloads.
> Injecting floating point fault/trap into pv guest and letting the guest
> retrieve the bundle/issue fpswa hypercall would solve that issue.
>
> However FPSWA hypercall isn't implemented properly which
> currently returns random value. This is the root cause.
> Please see fw_hypercall_fpswa()/ in xen/arch/ia64/xen/hypercall.c
> which was implemented by Kanno-san as 10158:9d52a66c7499.
> I can't find any place which sets arch_vcpu::fpswa_ret.
> Kanno-san, Any comment?
>
> from 10158:9d52a66c7499
>
> +static fpswa_ret_t
> +fw_hypercall_fpswa (struct vcpu *v)
> +{
> + return PSCBX(v, fpswa_ret);
> +}
> +
> ...
> @@ -109,6 +114,7 @@ struct arch_vcpu {
> //for phycial emulation
> unsigned long old_rsc;
> int mode_flags;
> + fpswa_ret_t fpswa_ret; /* save return values of FPSWA emulation */
> struct arch_vmx_struct arch_vmx; /* Virtual Machine Extensions */
> };
>
>
> > On bare metal the test takes about 1.3s (1.66GHz Montvale).
> > The few cases I've seen it complete running in dom0, it takes about 3.1s
> > (1.6GHz Montecito). Thanks,
>
> It's quite slow. more optimization is necessary?
>
--
yamahata
_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ia64-devel
|
Home