Re: [PATCH v1] x86/nestedsvm: remove dead code for shadow-paging L1 hosts

[Jan Beulich <jbeulich@xxxxxxxx>]

On 20.10.2026 05:05, chunjie.zhu@xxxxxxxxxx wrote:
> --- a/xen/arch/x86/hvm/svm/nestedsvm.c
> +++ b/xen/arch/x86/hvm/svm/nestedsvm.c
> @@ -304,21 +304,12 @@ static int nsvm_vcpu_hostrestore(struct vcpu *v, struct 
> cpu_user_regs *regs)
>          /* host nested paging + guest nested paging. */
>          /* hvm_set_cr3() below sets v->arch.hvm.guest_cr[3] for us. */
>      }
> -    else if ( paging_mode_hap(v->domain) )
> -    {
> -        /* host nested paging + guest shadow paging. */
> -        /* hvm_set_cr3() below sets v->arch.hvm.guest_cr[3] for us. */
> -    }
>      else
>      {
> -        /* host shadow paging + guest shadow paging. */
> -
> -        /* Reset MMU context  -- XXX (hostrestore) not yet working*/
> -        if ( !pagetable_is_null(v->arch.guest_table) )
> -            put_page(pagetable_get_page(v->arch.guest_table));
> -        v->arch.guest_table = pagetable_null();
> -        /* hvm_set_cr3() below sets v->arch.hvm.guest_cr[3] for us. */
> +        /* host shadow paging + guest shadow paging cannot happen */
> +        BUG_ON(paging_mode_shadow(v->domain));

How's "guest shadow paging" relevant here; the BUG_ON() solely depends on
"host", doesn't it? IOW I think the comment wants wording differently.

Also, nit: Please correct comment style while touching comments.

Furthermore: Why would the check be relevant only on the "else" path? It
could equally well live ahead of it, with the comments in if() and "else"
then talking of just the guest mode.

(All comments apply elsewhere as well.)

> @@ -1341,12 +1319,14 @@ nestedsvm_check_intercepts(struct vcpu *v, struct 
> cpu_user_regs *regs,
>              /* l2 guest intercepts #PF unnecessarily */
>              return NESTEDHVM_VMEXIT_INJECT;
>          }
> -        if ( !paging_mode_hap(v->domain) )
> -            /* host shadow paging + guest shadow paging */
> -            return NESTEDHVM_VMEXIT_HOST;
> +        else
> +        {
> +            /* host shadow paging + guest shadow paging cannot happen */
> +            BUG_ON(!paging_mode_hap(v->domain));
>  
> -        /* host nested paging + guest shadow paging */
> -        return NESTEDHVM_VMEXIT_INJECT;
> +            /* host nested paging + guest shadow paging */
> +            return NESTEDHVM_VMEXIT_INJECT;
> +        }
>      case VMEXIT_VMMCALL:
>          /* Always let the guest handle VMMCALL/VMCALL */
>          return NESTEDHVM_VMEXIT_INJECT;

This results in (apparently) un-annotated fall-through. As both if() and
"else" now return NESTEDHVM_VMEXIT_INJECT, those return-s want folding.
Then also please add a blank line between return and next case.

Jan