Message ID | 20250110132823.24348-10-alejandro.vallejo@cloud.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | x86: Address Space Isolation FPU preparations | expand |
On 10.01.2025 14:28, Alejandro Vallejo wrote: > --- a/xen/arch/x86/xstate.c > +++ b/xen/arch/x86/xstate.c > @@ -1024,9 +1024,10 @@ int handle_xsetbv(u32 index, u64 new_bv) > > uint64_t read_bndcfgu(void) > { > + uint64_t bndcfgu = 0; > unsigned long cr0 = read_cr0(); > - struct xsave_struct *xstate > - = idle_vcpu[smp_processor_id()]->arch.xsave_area; > + struct vcpu *v = idle_vcpu[smp_processor_id()]; Question on this one remains: Can it be pointer-to-const (in the longer run; certainly in can be right now)? > + struct xsave_struct *xstate = VCPU_MAP_XSAVE_AREA(v); I realize my similar remark on this one was actually wrong; the asm()s clearly modify what is being pointed top. Jan
Hi, Thanks for reviewing this and the other patches. On Mon Jan 27, 2025 at 10:57 AM GMT, Jan Beulich wrote: > On 10.01.2025 14:28, Alejandro Vallejo wrote: > > --- a/xen/arch/x86/xstate.c > > +++ b/xen/arch/x86/xstate.c > > @@ -1024,9 +1024,10 @@ int handle_xsetbv(u32 index, u64 new_bv) > > > > uint64_t read_bndcfgu(void) > > { > > + uint64_t bndcfgu = 0; > > unsigned long cr0 = read_cr0(); > > - struct xsave_struct *xstate > > - = idle_vcpu[smp_processor_id()]->arch.xsave_area; > > + struct vcpu *v = idle_vcpu[smp_processor_id()]; > > Question on this one remains: Can it be pointer-to-const (in the longer > run; certainly in can be right now)? I have no idea where I got the idea the C constness was transitive (quite likely from Rust, as its illegal to grab a &mut from a &). Const being non-transitive means I can constify the vcpu as you suggest/ask. The rationale was that getting a pointer to non-const from a pointer to const seemed wrong. But C is bizarre and its finer details have a way of biting. Oh well. FWIW, this ought to hold even in the long run. There won't be anything special in the map/unmap macros besides some indirection, so it'll work in a very similar fashion as it does now. I'll adjust it as you propose. > > > + struct xsave_struct *xstate = VCPU_MAP_XSAVE_AREA(v); > > I realize my similar remark on this one was actually wrong; the asm()s > clearly modify what is being pointed top. Indeed, xstate can definitely not be const. > Jan Cheers, Alejandro
diff --git a/xen/arch/x86/xstate.c b/xen/arch/x86/xstate.c index 3d249518a1b7..2003ba664594 100644 --- a/xen/arch/x86/xstate.c +++ b/xen/arch/x86/xstate.c @@ -1024,9 +1024,10 @@ int handle_xsetbv(u32 index, u64 new_bv) uint64_t read_bndcfgu(void) { + uint64_t bndcfgu = 0; unsigned long cr0 = read_cr0(); - struct xsave_struct *xstate - = idle_vcpu[smp_processor_id()]->arch.xsave_area; + struct vcpu *v = idle_vcpu[smp_processor_id()]; + struct xsave_struct *xstate = VCPU_MAP_XSAVE_AREA(v); const struct xstate_bndcsr *bndcsr; ASSERT(cpu_has_mpx); @@ -1052,7 +1053,12 @@ uint64_t read_bndcfgu(void) if ( cr0 & X86_CR0_TS ) write_cr0(cr0); - return xstate->xsave_hdr.xstate_bv & X86_XCR0_BNDCSR ? bndcsr->bndcfgu : 0; + if ( xstate->xsave_hdr.xstate_bv & X86_XCR0_BNDCSR ) + bndcfgu = bndcsr->bndcfgu; + + VCPU_UNMAP_XSAVE_AREA(v, xstate); + + return bndcfgu; } void xstate_set_init(uint64_t mask)
No functional change. Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com> --- v2->v3: * No change v1->v2: * s/ret/bndcfgu --- xen/arch/x86/xstate.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)