On 9/16/23 21:56, Andrew Cooper wrote:
>> We don't reset DR6 after reading it, and there is no guarantee that the PV
>> guest
>> will reset it either, so it doesn't match PENDING_DBG exactly IIRC.
>>
>> On the other hand, nothing will probably care about its double-accumulating
>> quirk except perhaps monitor users.
>>
>> Do we want to document that, shadow DR6 for PV (which seems a little overkill
>> if we don't trap DR6 access from PV already), or just live with that?
>
> Different DR6's.
>
> This is Xen responding to a real #DB (most likely from a PV guest, but
> maybe from debugging activity in Xen itself), and in ...
>
>>>
>>> /*
>>> * At the time of writing (March 2018), on the subject of %dr6:
>
> ... this piece of context missing from the patch, Xen always writes
> X86_DR6_DEFAULT back in order to clear the sticky bits.
Oh, that'll do.
How come have I not seen this? Maybe I was in dire need of morning coffee.
I assumed absence just from a missing context...
>
> This behaviour hasn't changed. Xen always sees a "clean" DR6 on every
> new #DB.
>
> For the PV guest, what matters is ...
>
(snip)
>>> - /* Save debug status register where guest OS can peek at it */
>>> - v->arch.dr6 |= (dr6 & ~X86_DR6_DEFAULT);
>>> - v->arch.dr6 &= (dr6 | ~X86_DR6_DEFAULT);
>>> -
>>> if ( guest_kernel_mode(v, regs) && v->domain->debugger_attached )
>>> {
>>> + /* Save debug status register where gdbsx can peek at it */
>>> + v->arch.dr6 = x86_merge_dr6(v->domain->arch.cpu_policy,
>>> + v->arch.dr6, pending_dbg);
>>> domain_pause_for_debugger();
>>> return;
>>> }
>>>
>>> - pv_inject_hw_exception(X86_EXC_DB, X86_EVENT_NO_EC);
>>> + pv_inject_DB(pending_dbg);
>
> ... this, which will merge all new pending bits into the guest's DR6.
>
> If the guest chooses not to clean out DR6 each time, then it will see
> content accumulate.
>
> To your earlier point of shadowing, we already have to do that. DR6 is
> just one of many registers we need to context switch for the vCPU.
>
> PV guests, being ring-deprivieged, trap into Xen for every DR access,
> and Xen handles every one of their #DB exceptions. This is one reason
> why I split the work into several parts. PV guests are easier to get
> sorted properly, and patch 1,2,5,6 are all common improvements relevant
> to HVM too.
That confirms my knowledge. Honestly if I got such a foolish remark
I would question the reviewer's understanding of PV mode (not that you did
anything wrong). Sorry for wasting your time.
>
> ~Andrew
--
Sincerely,
Jinoh Kang
