xen/arch/x86/pv/iret.c | 25 ++++++++++++++++++++++++- xen/arch/x86/traps.c | 24 ------------------------ xen/include/asm-x86/traps.h | 2 -- 3 files changed, 24 insertions(+), 27 deletions(-)
All callers are in pv/iret.c. Move the function and make it static.
Even before the pinning cleanup, there was nothing which is specific to
operating on curr, so rename the variable back to v.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wl@xen.org>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Juergen Gross <jgross@suse.com>
Discovered while reviewing/prodding Juergen's pinning removal patches.
---
xen/arch/x86/pv/iret.c | 25 ++++++++++++++++++++++++-
xen/arch/x86/traps.c | 24 ------------------------
xen/include/asm-x86/traps.h | 2 --
3 files changed, 24 insertions(+), 27 deletions(-)
diff --git a/xen/arch/x86/pv/iret.c b/xen/arch/x86/pv/iret.c
index c359a1dbfd..ae1c33612b 100644
--- a/xen/arch/x86/pv/iret.c
+++ b/xen/arch/x86/pv/iret.c
@@ -22,7 +22,30 @@
#include <xen/sched.h>
#include <asm/current.h>
-#include <asm/traps.h>
+
+static void async_exception_cleanup(struct vcpu *v)
+{
+ unsigned int trap;
+
+ if ( !v->async_exception_mask )
+ return;
+
+ if ( !(v->async_exception_mask & (v->async_exception_mask - 1)) )
+ trap = __scanbit(v->async_exception_mask, VCPU_TRAP_NONE);
+ else
+ for ( trap = VCPU_TRAP_NONE + 1; trap <= VCPU_TRAP_LAST; ++trap )
+ if ( (v->async_exception_mask ^
+ v->async_exception_state(trap).old_mask) == (1u << trap) )
+ break;
+ if ( unlikely(trap > VCPU_TRAP_LAST) )
+ {
+ ASSERT_UNREACHABLE();
+ return;
+ }
+
+ /* Restore previous asynchronous exception mask. */
+ v->async_exception_mask = v->async_exception_state(trap).old_mask;
+}
unsigned long do_iret(void)
{
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 08d7edc568..38d12013db 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -1593,30 +1593,6 @@ static void pci_serr_softirq(void)
outb(inb(0x61) & 0x0b, 0x61); /* re-enable the PCI SERR error line. */
}
-void async_exception_cleanup(struct vcpu *curr)
-{
- int trap;
-
- if ( !curr->async_exception_mask )
- return;
-
- if ( !(curr->async_exception_mask & (curr->async_exception_mask - 1)) )
- trap = __scanbit(curr->async_exception_mask, VCPU_TRAP_NONE);
- else
- for ( trap = VCPU_TRAP_NONE + 1; trap <= VCPU_TRAP_LAST; ++trap )
- if ( (curr->async_exception_mask ^
- curr->async_exception_state(trap).old_mask) == (1 << trap) )
- break;
- if ( unlikely(trap > VCPU_TRAP_LAST) )
- {
- ASSERT_UNREACHABLE();
- return;
- }
-
- /* Restore previous asynchronous exception mask. */
- curr->async_exception_mask = curr->async_exception_state(trap).old_mask;
-}
-
static void nmi_hwdom_report(unsigned int reason_idx)
{
struct domain *d = hardware_domain;
diff --git a/xen/include/asm-x86/traps.h b/xen/include/asm-x86/traps.h
index b88f2a4f2f..ec23d3a70b 100644
--- a/xen/include/asm-x86/traps.h
+++ b/xen/include/asm-x86/traps.h
@@ -19,8 +19,6 @@
#ifndef ASM_TRAP_H
#define ASM_TRAP_H
-void async_exception_cleanup(struct vcpu *);
-
const char *trapstr(unsigned int trapnr);
#endif /* ASM_TRAP_H */
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-develOn 23.07.19 21:58, Andrew Cooper wrote: > All callers are in pv/iret.c. Move the function and make it static. > > Even before the pinning cleanup, there was nothing which is specific to > operating on curr, so rename the variable back to v. > > No functional change. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Juergen Gross <jgross@suse.com> Juergen _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
On 23.07.2019 21:58, Andrew Cooper wrote: > All callers are in pv/iret.c. Move the function and make it static. > > Even before the pinning cleanup, there was nothing which is specific to > operating on curr, so rename the variable back to v. I'm not in full agreement with this: The implication here was (and afaict still is) that uses of / updates to involved vCPU fields are race free. Feel free to add my ack if you revert back to curr. Otherwise I'd first like to hear your contrary opinion. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
On 24/07/2019 11:22, Jan Beulich wrote: > On 23.07.2019 21:58, Andrew Cooper wrote: >> All callers are in pv/iret.c. Move the function and make it static. >> >> Even before the pinning cleanup, there was nothing which is specific to >> operating on curr, so rename the variable back to v. > I'm not in full agreement with this: The implication here was (and afaict > still is) that uses of / updates to involved vCPU fields are race free.z > Feel free to add my ack if you revert back to curr. Otherwise I'd first > like to hear your contrary opinion. We still call this v in plenty of other cases. If it wanted to be strictly limited to current then it should ASSERT(curr == current) as it doesn't derive curr itself, but with the move and being static, this is very obviously a pointless check, given the two callers. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
On 24.07.2019 13:04, Andrew Cooper wrote: > On 24/07/2019 11:22, Jan Beulich wrote: >> On 23.07.2019 21:58, Andrew Cooper wrote: >>> All callers are in pv/iret.c. Move the function and make it static. >>> >>> Even before the pinning cleanup, there was nothing which is specific to >>> operating on curr, so rename the variable back to v. >> I'm not in full agreement with this: The implication here was (and afaict >> still is) that uses of / updates to involved vCPU fields are race free.z >> Feel free to add my ack if you revert back to curr. Otherwise I'd first >> like to hear your contrary opinion. > > We still call this v in plenty of other cases. And we still call "curr" "v" in many cases where we shouldn't. I think it is helpful to know when there are such assumptions, ... > If it wanted to be strictly limited to current then it should > ASSERT(curr == current) as it doesn't derive curr itself, but with the > move and being static, this is very obviously a pointless check, given > the two callers. ... even when it's only a static helper. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
© 2016 - 2026 Red Hat, Inc.