Re: [PATCH v1] x86/nSVM: Check the L1 IOPM_BASE and MSRPM_BASE assigned physical addresses

[Jan Beulich <jbeulich@xxxxxxxx>]

On 28.07.2026 13:41, Abdelkareem Abdelsaamad wrote:
> --- a/xen/arch/x86/hvm/svm/nestedsvm.c
> +++ b/xen/arch/x86/hvm/svm/nestedsvm.c
> @@ -18,6 +18,8 @@
>  
>  #define NSVM_ERROR_VVMCB        1
>  #define NSVM_ERROR_VMENTRY      2
> +#define IOPM_MAX_PAGES_DIFF     2
> +#define MSRPM_MAX_PAGES_DIFF    1

I see little value in these constants, the more that from their names alone
one cannot infer what they're meant to represent. Furthermore the difference
between the two checks you add makes clear that in the latter one you open-
code MSRPM_MAX_PAGES_DIFF being 1.

> @@ -282,7 +284,7 @@ static int nsvm_vcpu_hostrestore(struct vcpu *v, struct 
> cpu_user_regs *regs)
>      return 0;
>  }
>  
> -static int nsvm_vmrun_permissionmap(struct vcpu *v, bool viopm)
> +static int nsvm_vmrun_permissionmap(struct vcpu *v)
>  {
>      struct svm_vcpu *arch_svm = &v->arch.hvm.svm;
>      struct nestedsvm *svm = &vcpu_nestedsvm(v);
> @@ -295,6 +297,23 @@ static int nsvm_vmrun_permissionmap(struct vcpu *v, bool 
> viopm)
>      unsigned long *ns_viomap;
>      bool ioport_80 = true, ioport_ed = true;
>  
> +    if ( (domain_get_maximum_gpfn(v->domain) < IOPM_MAX_PAGES_DIFF) ||

Leaving aside that it's questionable whether a domain with just two pages
can actually run sensibly, I think there's an off-by-one here, but I
further think the check isn't needed. After all ...

> +         (gfn_x(_gfn(ns_vmcb->_iopm_base_pa >> PAGE_SHIFT)) >

... the shift here guarantees that adding IOPM_MAX_PAGES_DIFF to the
result would not overflow (unless that constant was really huge).

Also please use gaddr_to_gfn() (or PFN_DOWN()) instead of open-coding.

> +          (domain_get_maximum_gpfn(v->domain) - IOPM_MAX_PAGES_DIFF)) )
> +    {
> +        gdprintk(XENLOG_ERR, "nsvm_vmrun_permissionmap invalid _iopm_base_pa"
> +                 " address (%#"PRIx64")\n", ns_vmcb->_iopm_base_pa);

When you want the function name in a log message, please use __func__.

Jan