On 06/06/18 15:20, Jan Beulich wrote:
>>>> On 04.06.18 at 15:59, <andrew.coop...@citrix.com> wrote:
>> No functional change (as curr->arch.debugreg[5] is zero when DE is clear),
>> but
>> this change simplifies the following patch.
> A comment to this effect would be helpful ...
>
>> --- a/xen/arch/x86/x86_emulate.c
>> +++ b/xen/arch/x86/x86_emulate.c
>> @@ -101,23 +101,29 @@ int x86emul_read_dr(unsigned int reg, unsigned long
>> *val,
>> switch ( reg )
>> {
>> case 0 ... 3:
>> - case 6:
>> *val = curr->arch.debugreg[reg];
>> break;
>>
>> + case 4:
>> + if ( curr->arch.pv_vcpu.ctrlreg[4] & X86_CR4_DE )
>> + goto ud_fault;
>> +
>> + /* Fallthrough */
>> + case 6:
>> + *val = curr->arch.debugreg[6];
>> + break;
>> +
>> + case 5:
>> + if ( curr->arch.pv_vcpu.ctrlreg[4] & X86_CR4_DE )
>> + goto ud_fault;
>> +
>> + /* Fallthrough */
>> case 7:
>> *val = (curr->arch.debugreg[7] |
>> curr->arch.debugreg[5]);
> ... somewhere above here. With that
> Acked-by: Jan Beulich <jbeul...@suse.com>
Even in light of the rename turning this into
*val = curr->arch.dr7 | curr->arch.pv_vcpu.dr7_emul;
?
The only reason I made the comment was to justify the "no functional
change" part of the commit message, but PV guests' questionable use of
debugreg[5] in the first place has no bearing on the correctness of this
code.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
