Re: [PATCH v2 09/13] x86/emulator: Refactor FXSAVE_AREA to use wrappers

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

On Mon Dec 16, 2024 at 12:01 PM GMT, Jan Beulich wrote:
> On 16.12.2024 12:58, Alejandro Vallejo wrote:
> > On Mon Dec 9, 2024 at 4:26 PM GMT, Jan Beulich wrote:
> >> On 05.11.2024 15:33, Alejandro Vallejo wrote:
> >>> --- a/xen/arch/x86/x86_emulate/blk.c
> >>> +++ b/xen/arch/x86/x86_emulate/blk.c
> >>> @@ -11,9 +11,12 @@
> >>>      !defined(X86EMUL_NO_SIMD)
> >>>  # ifdef __XEN__
> >>>  #  include <asm/xstate.h>
> >>> -#  define FXSAVE_AREA ((void *)&current->arch.xsave_area->fpu_sse)
> >>> +/* has a fastpath for `current`, so there's no actual map */
> >>> +#  define FXSAVE_AREA ((void *)VCPU_MAP_XSAVE_AREA(current))
> >>> +#  define UNMAP_FXSAVE_AREA(x) VCPU_UNMAP_XSAVE_AREA(currt ent, x)
> >>
> >> The typo of the first argument strongly suggests that the macro should
> >> already now evaluate its parameters, also pleasing Misra.
> > 
> > Not an unreasonable takeaway. I can expand as follows instead:
> > 
> > #define VCPU_UNMAP_XSAVE_AREA(v, x) ({ (void)(v); x; })
> > 
> > Thoughts?
>
> Why would x not also be cast to void?
>
> Jan

it should have the "x = NULL", in fact.

  #define VCPU_UNMAP_XSAVE_AREA(v, x) do { (void)(v); (x) = NULL; } while(0)

Regardless, the important bit was adding a separate statement for (v) casted to
void. Leaving the rest as-is.

Cheers,
Alejandro