No functional change.

Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
---
v2:
  * s/ret/bndcfgu
---
 xen/arch/x86/xstate.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/xstate.c b/xen/arch/x86/xstate.c
index 6db7ec2ea6a9..9ecbef760277 100644
--- a/xen/arch/x86/xstate.c
+++ b/xen/arch/x86/xstate.c
@@ -1022,9 +1022,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);
@@ -1050,7 +1051,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)
-- 
2.47.0

On 05.11.2024 15:33, Alejandro Vallejo wrote:
> --- a/xen/arch/x86/xstate.c
> +++ b/xen/arch/x86/xstate.c
> @@ -1022,9 +1022,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()];

Can this be pointer-to-const? Certainly right now, so the question is rather
meant to be forward looking.

> +    struct xsave_struct *xstate = VCPU_MAP_XSAVE_AREA(v);

This certainly can be pointer-to-const, just like ...

>      const struct xstate_bndcsr *bndcsr;

... this is.

Jan

On Mon Dec 9, 2024 at 4:30 PM GMT, Jan Beulich wrote:
> On 05.11.2024 15:33, Alejandro Vallejo wrote:
> > --- a/xen/arch/x86/xstate.c
> > +++ b/xen/arch/x86/xstate.c
> > @@ -1022,9 +1022,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()];
>
> Can this be pointer-to-const? Certainly right now, so the question is rather
> meant to be forward looking.
>
> > +    struct xsave_struct *xstate = VCPU_MAP_XSAVE_AREA(v);
>
> This certainly can be pointer-to-const, just like ...
>
> >      const struct xstate_bndcsr *bndcsr;
>
> ... this is.
>
> Jan

Yes, those retained non-const because of the now missing patch to zero-out
bndcfgu.

Cheers,
Alejandro

On 16.12.2024 13:00, Alejandro Vallejo wrote:
> On Mon Dec 9, 2024 at 4:30 PM GMT, Jan Beulich wrote:
>> On 05.11.2024 15:33, Alejandro Vallejo wrote:
>>> --- a/xen/arch/x86/xstate.c
>>> +++ b/xen/arch/x86/xstate.c
>>> @@ -1022,9 +1022,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()];
>>
>> Can this be pointer-to-const? Certainly right now, so the question is rather
>> meant to be forward looking.
>>
>>> +    struct xsave_struct *xstate = VCPU_MAP_XSAVE_AREA(v);
>>
>> This certainly can be pointer-to-const, just like ...
>>
>>>      const struct xstate_bndcsr *bndcsr;
>>
>> ... this is.
> 
> Yes, those retained non-const because of the now missing patch to zero-out
> bndcfgu.

I'm afraid this reply is ambiguous as to what's going to happen in the next
version. I can read both "will change" and "needs to stay" into it.

Jan

On Mon Dec 16, 2024 at 12:03 PM GMT, Jan Beulich wrote:
> On 16.12.2024 13:00, Alejandro Vallejo wrote:
> > On Mon Dec 9, 2024 at 4:30 PM GMT, Jan Beulich wrote:
> >> On 05.11.2024 15:33, Alejandro Vallejo wrote:
> >>> --- a/xen/arch/x86/xstate.c
> >>> +++ b/xen/arch/x86/xstate.c
> >>> @@ -1022,9 +1022,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()];
> >>
> >> Can this be pointer-to-const? Certainly right now, so the question is rather
> >> meant to be forward looking.
> >>
> >>> +    struct xsave_struct *xstate = VCPU_MAP_XSAVE_AREA(v);
> >>
> >> This certainly can be pointer-to-const, just like ...
> >>
> >>>      const struct xstate_bndcsr *bndcsr;
> >>
> >> ... this is.
> > 
> > Yes, those retained non-const because of the now missing patch to zero-out
> > bndcfgu.
>
> I'm afraid this reply is ambiguous as to what's going to happen in the next
> version. I can read both "will change" and "needs to stay" into it.
>
> Jan

It was an answer to "Can this be pointer to const?". Yes, I'll put the const
back.

Cheers,
Alejandro

On Mon Dec 16, 2024 at 2:02 PM GMT, Alejandro Vallejo wrote:
> On Mon Dec 16, 2024 at 12:03 PM GMT, Jan Beulich wrote:
> > On 16.12.2024 13:00, Alejandro Vallejo wrote:
> > > On Mon Dec 9, 2024 at 4:30 PM GMT, Jan Beulich wrote:
> > >> On 05.11.2024 15:33, Alejandro Vallejo wrote:
> > >>> --- a/xen/arch/x86/xstate.c
> > >>> +++ b/xen/arch/x86/xstate.c
> > >>> @@ -1022,9 +1022,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()];
> > >>
> > >> Can this be pointer-to-const? Certainly right now, so the question is rather
> > >> meant to be forward looking.
> > >>
> > >>> +    struct xsave_struct *xstate = VCPU_MAP_XSAVE_AREA(v);
> > >>
> > >> This certainly can be pointer-to-const, just like ...
> > >>
> > >>>      const struct xstate_bndcsr *bndcsr;
> > >>
> > >> ... this is.
> > > 
> > > Yes, those retained non-const because of the now missing patch to zero-out
> > > bndcfgu.
> >
> > I'm afraid this reply is ambiguous as to what's going to happen in the next
> > version. I can read both "will change" and "needs to stay" into it.
> >
> > Jan
>
> It was an answer to "Can this be pointer to const?". Yes, I'll put the const
> back.
>
> Cheers,
> Alejandro

Actually, xstate is the target of the assembly, so it will be written to
(though not from C). It seems like asking for trouble modifying the target of a
pointer to non-volatile const from inline assembly.

I'll leave it as-is in the interest of safety.

Cheers,
Alejandro