There are two issues there: When hvm_map_guest_frame_rw() yields a r/o
page mapping, we fail to indicate the failure to the guest, and we fall
over the NULL pointer in nvcpu->nv_vvmcx when subsequently invoking
nvmx_set_vmcs_pointer() (if no earlier VMPTRLD put in place a valid
value).

Fixes: 5dbbaa0fe121 ("x86/vvmx: fix I/O and MSR bitmaps mapping")
Reported-by: Manuel Andreas <manuel.andreas@tum.de>
Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -1820,7 +1820,7 @@ static int nvmx_handle_vmptrld(struct cp
                 vvmcx = NULL;
             }
         }
-        else
+        if ( !vvmcx )
         {
             vmfail(regs, VMX_INSN_VMPTRLD_INVALID_PHYADDR);
             goto out;

On 02/06/2025 4:37 pm, Jan Beulich wrote:
> There are two issues there: When hvm_map_guest_frame_rw() yields a r/o
> page mapping, we fail to indicate the failure to the guest, and we fall
> over the NULL pointer in nvcpu->nv_vvmcx when subsequently invoking
> nvmx_set_vmcs_pointer() (if no earlier VMPTRLD put in place a valid
> value).
>
> Fixes: 5dbbaa0fe121 ("x86/vvmx: fix I/O and MSR bitmaps mapping")
> Reported-by: Manuel Andreas <manuel.andreas@tum.de>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/arch/x86/hvm/vmx/vvmx.c
> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
> @@ -1820,7 +1820,7 @@ static int nvmx_handle_vmptrld(struct cp
>                  vvmcx = NULL;
>              }
>          }
> -        else
> +        if ( !vvmcx )
>          {
>              vmfail(regs, VMX_INSN_VMPTRLD_INVALID_PHYADDR);
>              goto out;

To go back to one of Manuel's original questions, there is no such thing
as a read-only page in real hardware (write ignore is the closest), but
I'm certain that hardware will give us VMPTRLD_INVALID_PHYADDR for e.g.
trying to overlay the APIC MMIO page, so I think it is a reasonable to
use for this purpose.

But, please insert a blank line.  I've spent 5 minutes thinking this
patch was broken and trying to figure out how you'd covered the r/o case
before realising I was reading it wrong.

With that, Reviewed-By: Andrew Cooper <andrew.cooper3@citrix.com>

~Andrew