Re: [PATCH 5/7] x86: Introduce x86_merge_dr6()

[Jinoh Kang <jinoh.kang.kr@xxxxxxxxx>]

On 9/25/23 16:30, Jan Beulich wrote:
> On 22.09.2023 18:11, Andrew Cooper wrote:
>> On 18/09/2023 12:37 pm, Jan Beulich wrote:
>>> On 15.09.2023 22:36, Andrew Cooper wrote:
>>>> The current logic used to update %dr6 when injecting #DB is buggy.
>>>>
>>>> The SDM/APM documention on %dr6 updates is far from ideal, but does at 
>>>> least
>>>> make clear that it's non-trivial.  The actual behaviour is to overwrite
>>>> B{0..3} and accumulate all other bits.
>>> As mentioned before, I'm okay to ack this patch provided it is explicitly 
>>> said
>>> where the information is coming from.
>>
>> The information *is* coming from the relevant paragraph of the relevant
>> chapters of the relevant manuals.
>>
>> I don't need to teach programmers how to suck eggs.  Nor am I going to
>> quote buggy manuals (corrections are pending for both) as a
>> justification for restating several paragraphs of information as a oneliner.
> 
> Earlier on you said this to my original request:
> 
> 'SDM Vol3 18.2.3 Debug Status Register (DR6) says
> 
>  "Certain debug exceptions may clear bits 0-3. The remaining contents of
>  the DR6 register are never cleared by the processor."'
> 
> "Certain" and "may" do not describe the behavior that your change implements.
> Hence imo there's still a need to clarify where the extra information is
> coming from. Pending corrections are of course appreciated; in case you have
> been told the new wording already, perhaps you could quote that?

As an outsider's perspective, I think this kind of thing is where selftests
really shine.  I got the impression that Xen will need to rely on numerous other
platform oddities, the documentation of which are often unavailable.

Of course, adding a whole new test infrastructure in code freeze is not viable.
Maybe I have missed something, but I only see three paths forward here:

1. Reference the most relevant paragraph in SDM/APM, but don't quote it.
   Keep the current explanation, and state that the manual is vague anyway.

2. Acknowledge that SDM/APM is incomplete, and completely abandon the manual
   as the *authoritative* source of information.  Perhaps embed a sample test
   program that demonstrates the behavior, if it isn't too long.

3. Actually assert in runtime that DR6 behaves as expected.

> 
> Jan

-- 
Sincerely,
Jinoh Kang