On 28.08.2025 17:03, Andrew Cooper wrote:
> In principle, the following can also be used for read_registers()
>
> diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
> index 5799770a2f71..0b0fdf2c5ac4 100644
> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -125,16 +125,21 @@ static void read_registers(struct extra_state
> *state)
> state->cr3 = read_cr3();
> state->cr4 = read_cr4();
>
> - if ( !(state->cr4 & X86_CR4_FRED) && (state->cr4 & X86_CR4_FSGSBASE)
> )
> + if ( state->cr4 & X86_CR4_FSGSBASE )
> {
> state->fsb = __rdfsbase();
> state->gsb = __rdgsbase();
> +
> + if ( state->cr4 & X86_CR4_FRED )
> + goto gskern_fred;
> +
> state->gss = __rdgskern();
I'm irritated by this patch context here vs ...
> --- a/xen/arch/x86/include/asm/fsgsbase.h
> +++ b/xen/arch/x86/include/asm/fsgsbase.h
> @@ -79,7 +79,9 @@ static inline unsigned long read_gs_base(void)
>
> static inline unsigned long read_gs_shadow(void)
> {
> - if ( read_cr4() & X86_CR4_FSGSBASE )
> + unsigned long cr4 = read_cr4();
> +
> + if ( !(cr4 & X86_CR4_FRED) && (cr4 & X86_CR4_FSGSBASE) )
> return __rdgs_shadow();
... the one here. Was the former (and the subject of the patch) not updated
yet (kern => shadow)? On the assumption that that's what has happened, and
hence preferably with the subject also adjusted
Reviewed-by: Jan Beulich <jbeul...@suse.com>
As to the alternative, I in particular don't overly like the goto there. I
would consider that an option only if in turn a simplification elsewhere
resulted.
Jan
