Re: [Xen-devel] [PATCH 1/3] x86/pv: Introduce and use x86emul_read_dr()
>>> On 16.04.18 at 18:44, wrote: > On 16/04/18 13:32, Jan Beulich wrote: > On 16.04.18 at 14:18, wrote: >>> On 13/04/18 12:39, Jan Beulich wrote: >>> On 13.04.18 at 13:17, wrote: > On 13/04/18 09:31, Jan Beulich wrote: > On 12.04.18 at 18:55, wrote: >>> do_get_debugreg() has several bugs: >>> >>> * The %cr4.de condition is inverted. %dr4/5 should be accessible only >>> when >>>%cr4.de is disabled. >>> * When %cr4.de is disabled, emulation should yield #UD rather than >>> complete >>>with zero. >>> * Using -EINVAL for errors is a broken ABI, as it overlaps with valid > values >>>near the top of the address space. >>> >>> Introduce a common x86emul_read_dr() handler (as we will eventually >>> want to >>> add HVM support) which separates its success/failure indication from >>> the > data >>> value, and have do_get_debugreg() call into the handler. >> The HVM part here is sort of questionable because of your use of >> curr->arch.pv_vcpu.ctrlreg[4]. > That is what the "needs further plumbing" refers to, as well as needing > hooks to get/modify %dr6/7 from the VMCB/VMCS. > > However, we are gaining an increasing amount of common x86 code which > needs to read control register values, and I've got a plan to refactor > across the board to v->arch.cr4 (and similar). There is no point having > identical information in different parts of sub-unions. I agree. >> This is appropriate for the NULL ctxt case, >> but it's already a layering violation for the use of the function in >> priv_op_ops, where the read_cr() hook should be used instead. > Hmm - doing this, while probably the better long temr course of action, > would require passing the ops structures down into the callbacks. That doesn't sound like a problem, though - the hypercall path would pass NULL there as well. >> This ... >> >>> +int x86emul_read_dr(unsigned int reg, unsigned long *val, >>> +struct x86_emulate_ctxt *ctxt) >>> +{ >>> +struct vcpu *curr = current; >>> + >>> +/* HVM support requires a bit more plumbing before it will work. */ >>> +ASSERT(is_pv_vcpu(curr)); >>> + >>> +switch ( reg ) >>> +{ >>> +case 0 ... 3: >>> +case 6: >>> +*val = curr->arch.debugreg[reg]; >>> +break; >>> + >>> +case 7: >>> +*val = (curr->arch.debugreg[7] | >>> +curr->arch.debugreg[5]); >>> +break; >>> + >>> +case 4 ... 5: >>> +if ( !(curr->arch.pv_vcpu.ctrlreg[4] & X86_CR4_DE) ) >>> +{ >>> +*val = curr->arch.debugreg[reg + 2]; >>> +break; >> Once at it, wouldn't you better also fix the missing ORing of [5] into >> the > DR7 (really >> DR5) value here? > [5] is zero when %cr4.de is clear (subject to a bugfix in the subsequent > patch), as IO breakpoints are only valid to use when %cr4.de is enabled. Oh, right you are. >>> So, are your comments suitably addressed? It is unclear whether you >>> want any changes to be made. >> ... is what I'd prefer to be taken care of without delaying to the time when >> we make this work for HVM as well. Unless you feel really strongly about it >> being better the way you have it, in which case you may feel free to add >> my ack. > > In all PV cases (hypercall and emulation), the current code functions > correctly, because DE is active in context. > > In principle, the emulation case would be better if it used the > read_cr() hook, but that is invasive to arrange (which is why I chose > not to at this point), and still needs special casing for the hypercall > case anyway. Well, okay then: Acked-by: Jan Beulich Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/3] x86/pv: Introduce and use x86emul_read_dr()
On 16/04/18 13:32, Jan Beulich wrote: On 16.04.18 at 14:18, wrote: >> On 13/04/18 12:39, Jan Beulich wrote: >> On 13.04.18 at 13:17, wrote: On 13/04/18 09:31, Jan Beulich wrote: On 12.04.18 at 18:55, wrote: >> do_get_debugreg() has several bugs: >> >> * The %cr4.de condition is inverted. %dr4/5 should be accessible only >> when >>%cr4.de is disabled. >> * When %cr4.de is disabled, emulation should yield #UD rather than >> complete >>with zero. >> * Using -EINVAL for errors is a broken ABI, as it overlaps with valid >> values >>near the top of the address space. >> >> Introduce a common x86emul_read_dr() handler (as we will eventually want >> to >> add HVM support) which separates its success/failure indication from the >> data >> value, and have do_get_debugreg() call into the handler. > The HVM part here is sort of questionable because of your use of > curr->arch.pv_vcpu.ctrlreg[4]. That is what the "needs further plumbing" refers to, as well as needing hooks to get/modify %dr6/7 from the VMCB/VMCS. However, we are gaining an increasing amount of common x86 code which needs to read control register values, and I've got a plan to refactor across the board to v->arch.cr4 (and similar). There is no point having identical information in different parts of sub-unions. >>> I agree. >>> > This is appropriate for the NULL ctxt case, > but it's already a layering violation for the use of the function in > priv_op_ops, where the read_cr() hook should be used instead. Hmm - doing this, while probably the better long temr course of action, would require passing the ops structures down into the callbacks. >>> That doesn't sound like a problem, though - the hypercall path would >>> pass NULL there as well. > This ... > >> +int x86emul_read_dr(unsigned int reg, unsigned long *val, >> +struct x86_emulate_ctxt *ctxt) >> +{ >> +struct vcpu *curr = current; >> + >> +/* HVM support requires a bit more plumbing before it will work. */ >> +ASSERT(is_pv_vcpu(curr)); >> + >> +switch ( reg ) >> +{ >> +case 0 ... 3: >> +case 6: >> +*val = curr->arch.debugreg[reg]; >> +break; >> + >> +case 7: >> +*val = (curr->arch.debugreg[7] | >> +curr->arch.debugreg[5]); >> +break; >> + >> +case 4 ... 5: >> +if ( !(curr->arch.pv_vcpu.ctrlreg[4] & X86_CR4_DE) ) >> +{ >> +*val = curr->arch.debugreg[reg + 2]; >> +break; > Once at it, wouldn't you better also fix the missing ORing of [5] into > the DR7 (really > DR5) value here? [5] is zero when %cr4.de is clear (subject to a bugfix in the subsequent patch), as IO breakpoints are only valid to use when %cr4.de is enabled. >>> Oh, right you are. >> So, are your comments suitably addressed? It is unclear whether you >> want any changes to be made. > ... is what I'd prefer to be taken care of without delaying to the time when > we make this work for HVM as well. Unless you feel really strongly about it > being better the way you have it, in which case you may feel free to add > my ack. In all PV cases (hypercall and emulation), the current code functions correctly, because DE is active in context. In principle, the emulation case would be better if it used the read_cr() hook, but that is invasive to arrange (which is why I chose not to at this point), and still needs special casing for the hypercall case anyway. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/3] x86/pv: Introduce and use x86emul_read_dr()
>>> On 16.04.18 at 14:18, wrote: > On 13/04/18 12:39, Jan Beulich wrote: > On 13.04.18 at 13:17, wrote: >>> On 13/04/18 09:31, Jan Beulich wrote: >>> On 12.04.18 at 18:55, wrote: > do_get_debugreg() has several bugs: > > * The %cr4.de condition is inverted. %dr4/5 should be accessible only > when >%cr4.de is disabled. > * When %cr4.de is disabled, emulation should yield #UD rather than > complete >with zero. > * Using -EINVAL for errors is a broken ABI, as it overlaps with valid > values >near the top of the address space. > > Introduce a common x86emul_read_dr() handler (as we will eventually want > to > add HVM support) which separates its success/failure indication from the > data > value, and have do_get_debugreg() call into the handler. The HVM part here is sort of questionable because of your use of curr->arch.pv_vcpu.ctrlreg[4]. >>> That is what the "needs further plumbing" refers to, as well as needing >>> hooks to get/modify %dr6/7 from the VMCB/VMCS. >>> >>> However, we are gaining an increasing amount of common x86 code which >>> needs to read control register values, and I've got a plan to refactor >>> across the board to v->arch.cr4 (and similar). There is no point having >>> identical information in different parts of sub-unions. >> I agree. >> This is appropriate for the NULL ctxt case, but it's already a layering violation for the use of the function in priv_op_ops, where the read_cr() hook should be used instead. >>> Hmm - doing this, while probably the better long temr course of action, >>> would require passing the ops structures down into the callbacks. >> That doesn't sound like a problem, though - the hypercall path would >> pass NULL there as well. This ... > +int x86emul_read_dr(unsigned int reg, unsigned long *val, > +struct x86_emulate_ctxt *ctxt) > +{ > +struct vcpu *curr = current; > + > +/* HVM support requires a bit more plumbing before it will work. */ > +ASSERT(is_pv_vcpu(curr)); > + > +switch ( reg ) > +{ > +case 0 ... 3: > +case 6: > +*val = curr->arch.debugreg[reg]; > +break; > + > +case 7: > +*val = (curr->arch.debugreg[7] | > +curr->arch.debugreg[5]); > +break; > + > +case 4 ... 5: > +if ( !(curr->arch.pv_vcpu.ctrlreg[4] & X86_CR4_DE) ) > +{ > +*val = curr->arch.debugreg[reg + 2]; > +break; Once at it, wouldn't you better also fix the missing ORing of [5] into the DR7 (really DR5) value here? >>> [5] is zero when %cr4.de is clear (subject to a bugfix in the subsequent >>> patch), as IO breakpoints are only valid to use when %cr4.de is enabled. >> Oh, right you are. > > So, are your comments suitably addressed? It is unclear whether you > want any changes to be made. ... is what I'd prefer to be taken care of without delaying to the time when we make this work for HVM as well. Unless you feel really strongly about it being better the way you have it, in which case you may feel free to add my ack. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/3] x86/pv: Introduce and use x86emul_read_dr()
On 13/04/18 12:39, Jan Beulich wrote: On 13.04.18 at 13:17, wrote: >> On 13/04/18 09:31, Jan Beulich wrote: >> On 12.04.18 at 18:55, wrote: do_get_debugreg() has several bugs: * The %cr4.de condition is inverted. %dr4/5 should be accessible only when %cr4.de is disabled. * When %cr4.de is disabled, emulation should yield #UD rather than complete with zero. * Using -EINVAL for errors is a broken ABI, as it overlaps with valid values near the top of the address space. Introduce a common x86emul_read_dr() handler (as we will eventually want to add HVM support) which separates its success/failure indication from the data value, and have do_get_debugreg() call into the handler. >>> The HVM part here is sort of questionable because of your use of >>> curr->arch.pv_vcpu.ctrlreg[4]. >> That is what the "needs further plumbing" refers to, as well as needing >> hooks to get/modify %dr6/7 from the VMCB/VMCS. >> >> However, we are gaining an increasing amount of common x86 code which >> needs to read control register values, and I've got a plan to refactor >> across the board to v->arch.cr4 (and similar). There is no point having >> identical information in different parts of sub-unions. > I agree. > >>> This is appropriate for the NULL ctxt case, >>> but it's already a layering violation for the use of the function in >>> priv_op_ops, where the read_cr() hook should be used instead. >> Hmm - doing this, while probably the better long temr course of action, >> would require passing the ops structures down into the callbacks. > That doesn't sound like a problem, though - the hypercall path would > pass NULL there as well. > +int x86emul_read_dr(unsigned int reg, unsigned long *val, +struct x86_emulate_ctxt *ctxt) +{ +struct vcpu *curr = current; + +/* HVM support requires a bit more plumbing before it will work. */ +ASSERT(is_pv_vcpu(curr)); + +switch ( reg ) +{ +case 0 ... 3: +case 6: +*val = curr->arch.debugreg[reg]; +break; + +case 7: +*val = (curr->arch.debugreg[7] | +curr->arch.debugreg[5]); +break; + +case 4 ... 5: +if ( !(curr->arch.pv_vcpu.ctrlreg[4] & X86_CR4_DE) ) +{ +*val = curr->arch.debugreg[reg + 2]; +break; >>> Once at it, wouldn't you better also fix the missing ORing of [5] into the >>> DR7 (really >>> DR5) value here? >> [5] is zero when %cr4.de is clear (subject to a bugfix in the subsequent >> patch), as IO breakpoints are only valid to use when %cr4.de is enabled. > Oh, right you are. So, are your comments suitably addressed? It is unclear whether you want any changes to be made. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/3] x86/pv: Introduce and use x86emul_read_dr()
>>> On 13.04.18 at 13:17, wrote: > On 13/04/18 09:31, Jan Beulich wrote: > On 12.04.18 at 18:55, wrote: >>> do_get_debugreg() has several bugs: >>> >>> * The %cr4.de condition is inverted. %dr4/5 should be accessible only when >>>%cr4.de is disabled. >>> * When %cr4.de is disabled, emulation should yield #UD rather than complete >>>with zero. >>> * Using -EINVAL for errors is a broken ABI, as it overlaps with valid >>> values >>>near the top of the address space. >>> >>> Introduce a common x86emul_read_dr() handler (as we will eventually want to >>> add HVM support) which separates its success/failure indication from the >>> data >>> value, and have do_get_debugreg() call into the handler. >> The HVM part here is sort of questionable because of your use of >> curr->arch.pv_vcpu.ctrlreg[4]. > > That is what the "needs further plumbing" refers to, as well as needing > hooks to get/modify %dr6/7 from the VMCB/VMCS. > > However, we are gaining an increasing amount of common x86 code which > needs to read control register values, and I've got a plan to refactor > across the board to v->arch.cr4 (and similar). There is no point having > identical information in different parts of sub-unions. I agree. >> This is appropriate for the NULL ctxt case, >> but it's already a layering violation for the use of the function in >> priv_op_ops, where the read_cr() hook should be used instead. > > Hmm - doing this, while probably the better long temr course of action, > would require passing the ops structures down into the callbacks. That doesn't sound like a problem, though - the hypercall path would pass NULL there as well. >>> +int x86emul_read_dr(unsigned int reg, unsigned long *val, >>> +struct x86_emulate_ctxt *ctxt) >>> +{ >>> +struct vcpu *curr = current; >>> + >>> +/* HVM support requires a bit more plumbing before it will work. */ >>> +ASSERT(is_pv_vcpu(curr)); >>> + >>> +switch ( reg ) >>> +{ >>> +case 0 ... 3: >>> +case 6: >>> +*val = curr->arch.debugreg[reg]; >>> +break; >>> + >>> +case 7: >>> +*val = (curr->arch.debugreg[7] | >>> +curr->arch.debugreg[5]); >>> +break; >>> + >>> +case 4 ... 5: >>> +if ( !(curr->arch.pv_vcpu.ctrlreg[4] & X86_CR4_DE) ) >>> +{ >>> +*val = curr->arch.debugreg[reg + 2]; >>> +break; >> Once at it, wouldn't you better also fix the missing ORing of [5] into the >> DR7 (really >> DR5) value here? > > [5] is zero when %cr4.de is clear (subject to a bugfix in the subsequent > patch), as IO breakpoints are only valid to use when %cr4.de is enabled. Oh, right you are. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/3] x86/pv: Introduce and use x86emul_read_dr()
On 13/04/18 09:31, Jan Beulich wrote: On 12.04.18 at 18:55, wrote: >> do_get_debugreg() has several bugs: >> >> * The %cr4.de condition is inverted. %dr4/5 should be accessible only when >>%cr4.de is disabled. >> * When %cr4.de is disabled, emulation should yield #UD rather than complete >>with zero. >> * Using -EINVAL for errors is a broken ABI, as it overlaps with valid values >>near the top of the address space. >> >> Introduce a common x86emul_read_dr() handler (as we will eventually want to >> add HVM support) which separates its success/failure indication from the data >> value, and have do_get_debugreg() call into the handler. > The HVM part here is sort of questionable because of your use of > curr->arch.pv_vcpu.ctrlreg[4]. That is what the "needs further plumbing" refers to, as well as needing hooks to get/modify %dr6/7 from the VMCB/VMCS. However, we are gaining an increasing amount of common x86 code which needs to read control register values, and I've got a plan to refactor across the board to v->arch.cr4 (and similar). There is no point having identical information in different parts of sub-unions. > This is appropriate for the NULL ctxt case, > but it's already a layering violation for the use of the function in > priv_op_ops, where the read_cr() hook should be used instead. Hmm - doing this, while probably the better long temr course of action, would require passing the ops structures down into the callbacks. > >> +int x86emul_read_dr(unsigned int reg, unsigned long *val, >> +struct x86_emulate_ctxt *ctxt) >> +{ >> +struct vcpu *curr = current; >> + >> +/* HVM support requires a bit more plumbing before it will work. */ >> +ASSERT(is_pv_vcpu(curr)); >> + >> +switch ( reg ) >> +{ >> +case 0 ... 3: >> +case 6: >> +*val = curr->arch.debugreg[reg]; >> +break; >> + >> +case 7: >> +*val = (curr->arch.debugreg[7] | >> +curr->arch.debugreg[5]); >> +break; >> + >> +case 4 ... 5: >> +if ( !(curr->arch.pv_vcpu.ctrlreg[4] & X86_CR4_DE) ) >> +{ >> +*val = curr->arch.debugreg[reg + 2]; >> +break; > Once at it, wouldn't you better also fix the missing ORing of [5] into the > DR7 (really > DR5) value here? [5] is zero when %cr4.de is clear (subject to a bugfix in the subsequent patch), as IO breakpoints are only valid to use when %cr4.de is enabled. I haven't yet investigated the native behaviour of selecting an IO breakpoint, then disabling Debug Extensions. That said, one of my TODO items is to allow PV guests to use General Detect (because its trivial to implement), at which point [5] will turn from an IO breakpoint shadow, into a more general %dr7 shadow, and ORing it in here will matter. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/3] x86/pv: Introduce and use x86emul_read_dr()
>>> On 12.04.18 at 18:55, wrote: > do_get_debugreg() has several bugs: > > * The %cr4.de condition is inverted. %dr4/5 should be accessible only when >%cr4.de is disabled. > * When %cr4.de is disabled, emulation should yield #UD rather than complete >with zero. > * Using -EINVAL for errors is a broken ABI, as it overlaps with valid values >near the top of the address space. > > Introduce a common x86emul_read_dr() handler (as we will eventually want to > add HVM support) which separates its success/failure indication from the data > value, and have do_get_debugreg() call into the handler. The HVM part here is sort of questionable because of your use of curr->arch.pv_vcpu.ctrlreg[4]. This is appropriate for the NULL ctxt case, but it's already a layering violation for the use of the function in priv_op_ops, where the read_cr() hook should be used instead. > The ABI of do_get_debugreg() remains broken, but switches from -EINVAL to > -ENODEV for compatibility with the changes in the following patch. > > Take the opportunity to add a missing local variable block to x86_emulate.c I don't think such a block can ever be "missing" - it shouldn't really be a requirement for one to be there; note how ./CODING_STYLE says "is permitted". Of course I don't mind its addition here. > +int x86emul_read_dr(unsigned int reg, unsigned long *val, > +struct x86_emulate_ctxt *ctxt) > +{ > +struct vcpu *curr = current; > + > +/* HVM support requires a bit more plumbing before it will work. */ > +ASSERT(is_pv_vcpu(curr)); > + > +switch ( reg ) > +{ > +case 0 ... 3: > +case 6: > +*val = curr->arch.debugreg[reg]; > +break; > + > +case 7: > +*val = (curr->arch.debugreg[7] | > +curr->arch.debugreg[5]); > +break; > + > +case 4 ... 5: > +if ( !(curr->arch.pv_vcpu.ctrlreg[4] & X86_CR4_DE) ) > +{ > +*val = curr->arch.debugreg[reg + 2]; > +break; Once at it, wouldn't you better also fix the missing ORing of [5] into the DR7 (really DR5) value here? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 1/3] x86/pv: Introduce and use x86emul_read_dr()
do_get_debugreg() has several bugs: * The %cr4.de condition is inverted. %dr4/5 should be accessible only when %cr4.de is disabled. * When %cr4.de is disabled, emulation should yield #UD rather than complete with zero. * Using -EINVAL for errors is a broken ABI, as it overlaps with valid values near the top of the address space. Introduce a common x86emul_read_dr() handler (as we will eventually want to add HVM support) which separates its success/failure indication from the data value, and have do_get_debugreg() call into the handler. The ABI of do_get_debugreg() remains broken, but switches from -EINVAL to -ENODEV for compatibility with the changes in the following patch. Take the opportunity to add a missing local variable block to x86_emulate.c Signed-off-by: Andrew Cooper --- CC: Jan Beulich CC: Wei Liu CC: Roger Pau Monné CC: Juergen Gross --- xen/arch/x86/pv/emul-priv-op.c | 15 +-- xen/arch/x86/pv/misc-hypercalls.c | 18 +++-- xen/arch/x86/x86_emulate.c | 49 ++ xen/arch/x86/x86_emulate/x86_emulate.h | 3 +++ 4 files changed, 56 insertions(+), 29 deletions(-) diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c index b456408..61702d9 100644 --- a/xen/arch/x86/pv/emul-priv-op.c +++ b/xen/arch/x86/pv/emul-priv-op.c @@ -794,19 +794,6 @@ static int write_cr(unsigned int reg, unsigned long val, return X86EMUL_UNHANDLEABLE; } -static int read_dr(unsigned int reg, unsigned long *val, - struct x86_emulate_ctxt *ctxt) -{ -unsigned long res = do_get_debugreg(reg); - -if ( IS_ERR_VALUE(res) ) -return X86EMUL_UNHANDLEABLE; - -*val = res; - -return X86EMUL_OKAY; -} - static int write_dr(unsigned int reg, unsigned long val, struct x86_emulate_ctxt *ctxt) { @@ -1305,7 +1292,7 @@ static const struct x86_emulate_ops priv_op_ops = { .read_segment= read_segment, .read_cr = read_cr, .write_cr= write_cr, -.read_dr = read_dr, +.read_dr = x86emul_read_dr, .write_dr= write_dr, .write_xcr = x86emul_write_xcr, .read_msr= read_msr, diff --git a/xen/arch/x86/pv/misc-hypercalls.c b/xen/arch/x86/pv/misc-hypercalls.c index 5862130..1619be7 100644 --- a/xen/arch/x86/pv/misc-hypercalls.c +++ b/xen/arch/x86/pv/misc-hypercalls.c @@ -30,22 +30,10 @@ long do_set_debugreg(int reg, unsigned long value) unsigned long do_get_debugreg(int reg) { -struct vcpu *curr = current; +unsigned long val; +int res = x86emul_read_dr(reg, &val, NULL); -switch ( reg ) -{ -case 0 ... 3: -case 6: -return curr->arch.debugreg[reg]; -case 7: -return (curr->arch.debugreg[7] | -curr->arch.debugreg[5]); -case 4 ... 5: -return ((curr->arch.pv_vcpu.ctrlreg[4] & X86_CR4_DE) ? -curr->arch.debugreg[reg + 2] : 0); -} - -return -EINVAL; +return res == X86EMUL_OKAY ? val : -ENODEV; } long do_fpu_taskswitch(int set) diff --git a/xen/arch/x86/x86_emulate.c b/xen/arch/x86/x86_emulate.c index 0729edc..d260bdc 100644 --- a/xen/arch/x86/x86_emulate.c +++ b/xen/arch/x86/x86_emulate.c @@ -87,3 +87,52 @@ int x86emul_write_xcr(unsigned int reg, uint64_t val, return X86EMUL_OKAY; } + +/* Called with NULL ctxt in hypercall context. */ +int x86emul_read_dr(unsigned int reg, unsigned long *val, +struct x86_emulate_ctxt *ctxt) +{ +struct vcpu *curr = current; + +/* HVM support requires a bit more plumbing before it will work. */ +ASSERT(is_pv_vcpu(curr)); + +switch ( reg ) +{ +case 0 ... 3: +case 6: +*val = curr->arch.debugreg[reg]; +break; + +case 7: +*val = (curr->arch.debugreg[7] | +curr->arch.debugreg[5]); +break; + +case 4 ... 5: +if ( !(curr->arch.pv_vcpu.ctrlreg[4] & X86_CR4_DE) ) +{ +*val = curr->arch.debugreg[reg + 2]; +break; +} + +/* Fallthrough */ +default: +if ( ctxt ) +x86_emul_hw_exception(TRAP_invalid_op, X86_EVENT_NO_EC, ctxt); + +return X86EMUL_EXCEPTION; +} + +return X86EMUL_OKAY; +} + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * tab-width: 4 + * indent-tabs-mode: nil + * End: + */ diff --git a/xen/arch/x86/x86_emulate/x86_emulate.h b/xen/arch/x86/x86_emulate/x86_emulate.h index 13385b0..2ae0518 100644 --- a/xen/arch/x86/x86_emulate/x86_emulate.h +++ b/xen/arch/x86/x86_emulate/x86_emulate.h @@ -722,6 +722,9 @@ int x86emul_read_xcr(unsigned int reg, uint64_t *val, int x86emul_write_xcr(unsigned int reg, uint64_t val, struct x86_emulate_ctxt *ctxt); +int x86emul_read_dr(unsigned int reg, unsigned long *val, +struct x86_emulate_ctxt *ctxt); + #e