Re: [PATCH v3 09/12] x86/mpx: Map/unmap xsave area in in read_bndcfgu()

["Alejandro Vallejo" <alejandro.vallejo@xxxxxxxxx>]

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