On 9/25/23 19:20, Jinoh Kang wrote:
> 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.
>
&
ee 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
On 9/16/23 23:00, Andrew Cooper wrote:
> On 16/09/2023 8:36 am, Jinoh Kang wrote:
(snip)
>> These two hunks look like a behavioral change in singlestep mode.
>>
>> This is actually a fix, assuming the emulator previously did not handle
>> 'rep {in,out}s' i
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
ent in the common case, but retain the prior behaviour if a debugger is
> attached.
>
> Signed-off-by: Andrew Cooper
> ---
> CC: Jan Beulich
> CC: Roger Pau Monné
> CC: Wei Liu
> CC: Jinoh Kang
>
> v2:
> * Split pieces out into an earlier patch.
> * Extend do_d
>
> Signed-off-by: Andrew Cooper
> ---
> CC: Jan Beulich
> CC: Roger Pau Monné
> CC: Wei Liu
> CC: Jinoh Kang
>
> v2:
> * Split out of prior patch.
> ---
> xen/arch/x86/include/asm/domain.h | 18 --
> xen/arch/x86/include/asm/hvm/hvm.
that.
(Ideally all the HWBP handling should be part of the emulator logic, but
I don't see an easy way to generalize the PV-specific logic. It could
be its own patch anyway.)
--
Sincerely,
Jinoh Kang
On 9/15/23 21:20, Jinoh Kang wrote:
> On 9/13/23 08:21, Andrew Cooper wrote:
>> diff --git a/xen/arch/x86/x86_emulate/x86_emulate.h
>> b/xen/arch/x86/x86_emulate/x86_emulate.h
>> index 698750267a90..f0e74d23c378 100644
>> --- a/xen/arch/x86/x86_emulate/x86_emul
t; bool sti:1; /* Instruction sets STI irq shadow. */
> bool unblock_nmi:1; /* Instruction clears NMI blocking. */
> -bool singlestep:1; /* Singlestepping was active. */
> +bool singlestep:1; /* Singlestepping was active. (TODO, merge
> into pending_dbg) */
> };
> } retire;
>
--
Sincerely,
Jinoh Kang
On 8/30/23 22:41, Jan Beulich wrote:
> On 24.08.2023 17:25, Jinoh Kang wrote:
>> Prepare for an upcoming patch that overloads the 'cr2' field for #DB.
>
> Seeing the subsequent change and the fact that earlier on Andrew didn't
> need such an adjustment, I'
: Andrew Cooper
Reviewed-by: Roger Pau Monné
Reviewed-by: Jan Beulich
[ jinoh: Rebase onto staging, move constants to x86-defns.h ]
Signed-off-by: Jinoh Kang
---
CC: Andrew Cooper
CC: Jan Beulich
CC: Wei Liu
CC: Roger Pau Monné
CC: Jun Nakajima
CC: Kevin Tian
v1 -> v2: [S-o-b fi
From: Andrew Cooper
Replace the few remaining uses with X86_DR6_* constants.
Signed-off-by: Andrew Cooper
Reviewed-by: Roger Pau Monné
Acked-by: Jan Beulich
[ jinoh: Rebase onto staging ]
Signed-off-by: Jinoh Kang
---
CC: Andrew Cooper
CC: Jan Beulich
CC: Wei Liu
CC: Roger Pau Monné
v1
From: Andrew Cooper
Signed-off-by: Andrew Cooper
Reviewed-by: Roger Pau Monné
Extracted comments only, and then s/from emulation/from monitor/;
originally "x86/hvm: RFC - PROBABLY BROKEN - Defer all debugging/monitor
actions to {svm,vmx}_inject_event()"
Signed-off-by: Jinoh Ka
Ensure that we pass the correct pending_dbg value to
hvm_monitor_interrupt().
Signed-off-by: Jinoh Kang
---
CC: Andrew Cooper
CC: Jan Beulich
CC: Wei Liu
CC: Roger Pau Monné
CC: Jun Nakajima
CC: Kevin Tian
CC: Tamas K Lengyel
CC: Alexandru Isaila
CC: Petre Pircalabu
v1 -> v2: new pa
viewed-by: Roger Pau Monné
Reviewed-by: Jan Beulich
[ jinoh: Rebase onto staging, forward declare struct vcpu ]
Signed-off-by: Jinoh Kang
---
CC: Andrew Cooper
CC: Jan Beulich
CC: Wei Liu
CC: Roger Pau Monné
CC: Jun Nakajima
CC: Kevin Tian
CC: Tim Deegan
CC: Tamas K Lengyel
CC: Alexand
Prepare for an upcoming patch that overloads the 'cr2' field for #DB.
Signed-off-by: Jinoh Kang
---
CC: Andrew Cooper
CC: Jan Beulich
CC: Wei Liu
CC: Roger Pau Monné
CC: Tamas K Lengyel
CC: Alexandru Isaila
CC: Petre Pircalabu
---
xen/arch/x86/hvm/hvm.c | 9 +++--
1 file
e the guest maximum policy,
which will be more permissive with respect to the RTM feature.
Signed-off-by: Andrew Cooper
Reviewed-by: Roger Pau Monné
[ jinoh: Rebase onto staging, along with some fixes ]
Signed-off-by: Jinoh Kang
---
CC: Andrew Cooper
CC: Jan Beulich
CC: Wei Liu
CC: Roger Pau
ernel mode
- Commit message fixes
Andrew Cooper (5):
x86: Fix calculation of %dr6/7 reserved bits
x86/emul: Add pending_dbg field to x86_event
x86/hvm: Add comments about #DB exception behavior to
{svm,vmx}_inject_event()
x86: Fix merging of new status bits into %dr6
x86/dbg: Cleanup
to take care of them.
Somehow I assumed that Andrew's branch had already contained the relevant
updates, which was clearly wrong and was a sloppy thinking on my part.
Also, rushing to submission didn't _really_ help.
Again, apologies for wasting your time.
--
Sincerely,
Jinoh Kang
On 8/22/23 23:54, Jan Beulich wrote:
> On 21.08.2023 18:12, Jinoh Kang wrote:
>> On 8/22/23 00:56, Jinoh Kang wrote:
>>> From: Andrew Cooper
>>>
>>> The reserved bit calculations for %dr6 and %dr7 depend on whether the VM has
>>> the Restricted Tr
thing
else w.r.t. tagging rules.
>
> Jan
--
Sincerely,
Jinoh Kang
On 8/22/23 01:34, Tamas K Lengyel wrote:
> On Mon, Aug 21, 2023 at 5:57 PM Jinoh Kang wrote:
>> This is RFC because it probably breaks introspection, as injection replies
>> from the introspection engine will (probably, but I haven't confirmed)
>> trigger
>> ne
On 8/22/23 00:56, Jinoh Kang wrote:
> From: Andrew Cooper
>
> The reserved bit calculations for %dr6 and %dr7 depend on whether the VM has
> the Restricted Transnational Memory feature available.
s/Transnational/Transactional/.
It was in the original review, but I missed the change
of RTM and reserved fields.
For PV guests, move the ad-hoc updating of %dr6 into pv_inject_event(). This
in principle breaks the handing of RTM in do_debug(), but PV guests can't
actually enable MSR_DEBUGCTL.RTM yet, so this doesn't matter in practice.
Signed-off-by: Jinoh Kang
---
CC:
: Jinoh Kang
---
CC: Jan Beulich
CC: Wei Liu
CC: Roger Pau Monné
CC: Jun Nakajima
CC: Kevin Tian
---
xen/arch/x86/hvm/svm/svm.c | 3 ++-
xen/arch/x86/hvm/vmx/vmx.c | 3 ++-
xen/arch/x86/include/asm/debugreg.h | 30 +++-
xen/arch/x86/include/asm
From: Andrew Cooper
Replace the few remaining uses with X86_DR6_* constants.
Signed-off-by: Jinoh Kang
---
CC: Jan Beulich
CC: Wei Liu
CC: Roger Pau Monné
---
xen/arch/x86/hvm/vmx/vmx.c | 2 +-
xen/arch/x86/include/asm/debugreg.h | 17 -
xen/arch/x86/pv/emul-priv
level, and
everything implemented at the injection level. Amongst other things, this
means that the monitor subsystem will pick up debug actions from emulated
events.
Signed-off-by: Jinoh Kang
---
CC: Jan Beulich
CC: Wei Liu
CC: Roger Pau Monné
CC: Jun Nakajima
CC: Kevin Tian
CC: Razvan
guest maximum policy,
which will be more permissive with respect to the RTM feature.
Signed-off-by: Jinoh Kang
---
CC: Jan Beulich
CC: Wei Liu
CC: Roger Pau Monné
---
xen/arch/x86/domain.c | 7 +++--
xen/arch/x86/hvm/hvm.c | 6 ++--
xen/arch/x86/include/asm
This is a rebased version of Andrew Cooper's debugging facilities patch:
https://lore.kernel.org/xen-devel/1528120755-17455-1-git-send-email-andrew.coop...@citrix.com/
> So this started as a small fix for the vmentry failure (penultimate patch),
> and has snowballed...
>
> I'm fairly confident tha
ne something wrong, your debug-fixes-v1 when rebased to master
seems to fix my problem, so I guess your patchset is the superset.
--
Sincerely,
Jinoh Kang
On 8/19/23 00:47, Jinoh Kang wrote:
> Today, when a HVM (or PVH) guest triggers a hardware breakpoint while
> EFLAGS.TF is set, Xen incorrectly assumes that this is a single stepping
> exception and sets DR_STEP in dr6 in addition to DR_TRAP.
>
> This causes problems with Linu
Fixes: 9864841914c2 ("x86/vm_event: add support for VM_EVENT_REASON_INTERRUPT")
Signed-off-by: Jinoh Kang
---
xen/arch/x86/hvm/svm/svm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 01dd592d9b83..be
Commit 21867648033d ("x86/debug: Plumb pending_dbg through the monitor
and devicemodel interfaces") introduced pending_dbg, but did not
actually populate or use the field.
Signed-off-by: Jinoh Kang
---
xen/arch/x86/hvm/svm/svm.c | 34 +++---
xen/arch/x
Add pv_inject_debug_exception() helper and use it wherever
applicable.
This helper corresponds to hvm_inject_debug_exception() in HVM.
Signed-off-by: Jinoh Kang
---
xen/arch/x86/include/asm/domain.h | 12
xen/arch/x86/pv/emulate.c | 5 +
xen/arch/x86/pv/ro-page
nt.
Fixes: 8b831f4189 ("x86: single step after instruction emulation")
Signed-off-by: Jinoh Kang
---
xen/arch/x86/hvm/emulate.c | 3 ++-
xen/arch/x86/hvm/svm/svm.c | 6 +++---
xen/arch/x86/hvm/vmx/vmx.c | 6 +++---
xen/arch/x86/include/as
Signed-off-by: Jinoh Kang
---
xen/arch/x86/pv/ro-page-fault.c | 4
1 file changed, 4 insertions(+)
diff --git a/xen/arch/x86/pv/ro-page-fault.c b/xen/arch/x86/pv/ro-page-fault.c
index cad28ef928ad..238bfbeb4ac4 100644
--- a/xen/arch/x86/pv/ro-page-fault.c
+++ b/xen/arch/x86/pv/ro-page
n
preparation for an upcoming patch that uses it to actually populate dr6.
Signed-off-by: Jinoh Kang
---
xen/arch/x86/hvm/dm.c | 2 +-
xen/arch/x86/hvm/hvm.c | 4 ++--
xen/arch/x86/hvm/svm/nestedsvm.c | 2 +-
xen/arch/x86/hvm/svm/svm.c
Prepare for an upcoming patch that overloads the 'cr2' field for #DB.
Signed-off-by: Jinoh Kang
---
xen/arch/x86/hvm/hvm.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 3a99c0ff20be..48a77524f198 10
KEUSER, pid, (void *)offsetof(struct user,
u_debugreg[7]), (void *)0L));
/* Continue execution to let child process exit */
ABORT_ON_ERR(ptrace(PTRACE_CONT, pid, NULL, (unsigned long)0));
if (waitpid(pid, &wstatus, 0) != pid) abort();
if (!(WIFEXITED(wstatus) && WEXITST
39 matches
Mail list logo
