Re: [Xen-devel] [PATCH 07/17] x86/traps: put PV code handlers under CONFIG_PV

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 04.10.18 at 17:43, <wei.liu2@xxxxxxxxxx> wrote:
> The trap handlers in hypervisor need to forward events to PV guests if
> necessary. If there is no PV guest relevant code should be excluded.
> 
> Put code under CONFIG_PV and add ASSERT_UNREACHABLE.

-ETOOMANYIFDEFS

What's wrong with an inline stub for pv_inject_event()? This won't
eliminate all #ifdef-s, but quite a few of them afaics.

> @@ -1140,6 +1152,7 @@ static int handle_ldt_mapping_fault(unsigned int offset,
>          if ( !guest_mode(regs) )
>              return 0;
>  
> +#ifdef CONFIG_PV
>          /* Access would have become non-canonical? Pass #GP[sel] back. */
>          if ( unlikely(!is_canonical_address(curr->arch.pv.ldt_base + 
> offset)) )
>          {
> @@ -1151,6 +1164,9 @@ static int handle_ldt_mapping_fault(unsigned int offset,
>              /* else pass the #PF back, with adjusted %cr2. */
>              pv_inject_page_fault(regs->error_code,
>                                   curr->arch.pv.ldt_base + offset);
> +#else
> +        ASSERT_UNREACHABLE();
> +#endif

I think here it should be the call site to gain #ifdef (around the
entire if()), and the function as a whole should then be put in
another #ifdef, or be moved to pv/traps.c.

> @@ -1494,7 +1514,9 @@ void __init do_early_page_fault(struct cpu_user_regs 
> *regs)
>  
>  void do_general_protection(struct cpu_user_regs *regs)
>  {
> +#ifdef CONFIG_PV
>      struct vcpu *v = current;
> +#endif

I think this would better be resolved by moving the declaration into
the block which has multiple references, and use plain "current" in
the subsequent "else if()". But it's a matter of taste, I agree.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel