CPU_BASED_CR3_LOAD_EXITING |
HVM guests cannot run without either HAP or shadow enabled. While
paging_mode_shadow() is compile-time-constant when SHADOW_PAGING=n,
paging_mode_hap() isn't. Hence the former is preferred to leverage DCE.
In svm_update_guest_cr() combine two adjacent conditionals.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/hvm/dom0_build.c
+++ b/xen/arch/x86/hvm/dom0_build.c
@@ -473,7 +473,7 @@ static int __init pvh_populate_p2m(struc
}
}
- if ( using_vmx() && paging_mode_hap(d) && !vmx_unrestricted_guest(v) )
+ if ( using_vmx() && !paging_mode_shadow(d) && !vmx_unrestricted_guest(v) )
{
/*
* Since Dom0 cannot be migrated, we will only setup the
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -8,7 +8,7 @@
#include <asm/hvm/support.h>
#include <asm/hvm/svm.h>
#include <asm/hvm/nestedhvm.h>
-#include <asm/paging.h> /* paging_mode_hap */
+#include <asm/paging.h> /* paging_mode_(hap|shadow)() */
#include <asm/event.h> /* for local_event_delivery_(en|dis)able */
#include <asm/p2m.h> /* p2m_get_pagetable, p2m_get_nestedp2m */
#include <asm/x86_emulate.h>
@@ -243,7 +243,7 @@ static int nsvm_vcpu_hostrestore(struct
/* 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) )
+ else if ( !paging_mode_shadow(v->domain) )
{
/* host nested paging + guest shadow paging. */
/* hvm_set_cr3() below sets v->arch.hvm.guest_cr[3] for us. */
@@ -525,7 +525,7 @@ static int nsvm_vmcb_prepare4vmrun(struc
if ( rc != X86EMUL_OKAY )
gdprintk(XENLOG_ERR, "hvm_set_cr3 failed, rc: %u\n", rc);
}
- else if ( paging_mode_hap(v->domain) )
+ else if ( !paging_mode_shadow(v->domain) )
{
/* host nested paging + guest shadow paging. */
vmcb_set_np(n2vmcb, true);
@@ -1033,7 +1033,7 @@ nsvm_vmcb_prepare4vmexit(struct vcpu *v,
* unshadowed guest h_cr3 is kept in ns_vmcb->h_cr3,
* hence we keep the ns_vmcb->h_cr3 value. */
}
- else if ( paging_mode_hap(v->domain) )
+ else if ( !paging_mode_shadow(v->domain) )
{
/* host nested paging + guest shadow paging. */
vmcb_set_np(ns_vmcb, false);
@@ -1260,7 +1260,7 @@ nestedsvm_check_intercepts(struct vcpu *
/* host nested paging + guest nested paging */
return NESTEDHVM_VMEXIT_HOST;
}
- if ( paging_mode_hap(v->domain) )
+ if ( !paging_mode_shadow(v->domain) )
{
if ( is_intercepted )
return NESTEDHVM_VMEXIT_FATALERROR;
@@ -1559,7 +1559,7 @@ void svm_nested_features_on_efer_update(
if ( nsvm_efer_svm_enabled(v) )
{
if ( !vmcb->virt_ext.fields.vloadsave_enable &&
- paging_mode_hap(v->domain) &&
+ !paging_mode_shadow(v->domain) &&
cpu_has_svm_vloadsave )
{
vmcb->virt_ext.fields.vloadsave_enable = 1;
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -113,7 +113,8 @@ static void cf_check svm_update_guest_cr
switch ( cr )
{
case 0:
- if ( paging_mode_hap(v->domain) )
+ value = v->arch.hvm.guest_cr[0];
+ if ( !paging_mode_shadow(v->domain) )
{
uint32_t intercepts = vmcb_get_cr_intercepts(vmcb);
@@ -122,9 +123,7 @@ static void cf_check svm_update_guest_cr
monitor_ctrlreg_bitmask(VM_EVENT_X86_CR3) )
vmcb_set_cr_intercepts(vmcb, intercepts | CR_INTERCEPT_CR3_WRITE);
}
-
- value = v->arch.hvm.guest_cr[0];
- if ( paging_mode_shadow(v->domain) )
+ else
value |= X86_CR0_PG | X86_CR0_WP;
vmcb_set_cr0(vmcb, value);
break;
@@ -148,7 +147,7 @@ static void cf_check svm_update_guest_cr
break;
case 4:
value = HVM_CR4_HOST_MASK;
- if ( paging_mode_hap(v->domain) )
+ if ( !paging_mode_shadow(v->domain) )
value &= ~X86_CR4_PAE;
value |= v->arch.hvm.guest_cr[4];
@@ -418,7 +417,7 @@ static int svm_vmcb_restore(struct vcpu
svm_update_guest_cr(v, 0, 0);
svm_update_guest_cr(v, 4, 0);
- if ( paging_mode_hap(v->domain) )
+ if ( !paging_mode_shadow(v->domain) )
{
vmcb_set_np(vmcb, true);
vmcb_set_g_pat(vmcb, MSR_IA32_CR_PAT_RESET /* guest PAT */);
@@ -2519,7 +2518,7 @@ void asmlinkage svm_vmexit_handler(void)
regs, !(vmcb_get_efer(vmcb) & EFER_LMA) || !(vmcb->cs.l));
v->arch.hvm.guest_cr[2] = vmcb_get_cr2(vmcb);
- if ( paging_mode_hap(v->domain) )
+ if ( !paging_mode_shadow(v->domain) )
{
v->arch.hvm.guest_cr[0] = vmcb_get_cr0(vmcb);
v->arch.hvm.guest_cr[3] = v->arch.hvm.hw_cr[3] = vmcb_get_cr3(vmcb);
--- a/xen/arch/x86/hvm/svm/vmcb.c
+++ b/xen/arch/x86/hvm/svm/vmcb.c
@@ -143,7 +143,7 @@ static int construct_vmcb(struct vcpu *v
vmcb->_exception_intercepts = HVM_TRAP_MASK;
- if ( paging_mode_hap(v->domain) )
+ if ( !paging_mode_shadow(v->domain) )
{
vmcb_set_np(vmcb, true); /* enable nested paging */
vmcb->_g_pat = MSR_IA32_CR_PAT_RESET; /* guest PAT */
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -1141,7 +1141,7 @@ static int construct_vmcs(struct vcpu *v
SECONDARY_EXEC_ENABLE_VM_FUNCTIONS |
SECONDARY_EXEC_ENABLE_VIRT_EXCEPTIONS);
- if ( paging_mode_hap(d) )
+ if ( !paging_mode_shadow(d) )
{
v->arch.hvm.vmx.exec_control &= ~(CPU_BASED_INVLPG_EXITING |
CPU_BASED_CR3_LOAD_EXITING |
@@ -1203,7 +1203,7 @@ static int construct_vmcs(struct vcpu *v
vmx_clear_msr_intercept(v, MSR_IA32_SYSENTER_CS, VMX_MSR_RW);
vmx_clear_msr_intercept(v, MSR_IA32_SYSENTER_ESP, VMX_MSR_RW);
vmx_clear_msr_intercept(v, MSR_IA32_SYSENTER_EIP, VMX_MSR_RW);
- if ( paging_mode_hap(d) && (!is_iommu_enabled(d) || iommu_snoop) )
+ if ( !paging_mode_shadow(d) && (!is_iommu_enabled(d) || iommu_snoop) )
vmx_clear_msr_intercept(v, MSR_IA32_CR_PAT, VMX_MSR_RW);
if ( (vmexit_ctl & VM_EXIT_CLEAR_BNDCFGS) &&
(vmentry_ctl & VM_ENTRY_LOAD_BNDCFGS) )
@@ -1327,7 +1327,7 @@ static int construct_vmcs(struct vcpu *v
__vmwrite(VMCS_LINK_POINTER, ~0UL);
v->arch.hvm.vmx.exception_bitmap = HVM_TRAP_MASK
- | (paging_mode_hap(d) ? 0 : (1U << X86_EXC_PF));
+ | (!paging_mode_shadow(d) ? 0 : (1U << X86_EXC_PF));
if ( cpu_has_vmx_notify_vm_exiting )
__vmwrite(NOTIFY_WINDOW, vm_notify_window);
@@ -1347,7 +1347,7 @@ static int construct_vmcs(struct vcpu *v
__vmwrite(TPR_THRESHOLD, 0);
}
- if ( paging_mode_hap(d) )
+ if ( !paging_mode_shadow(d) )
{
struct p2m_domain *p2m = p2m_get_hostp2m(d);
struct ept_data *ept = &p2m->ept;
@@ -1377,7 +1377,7 @@ static int construct_vmcs(struct vcpu *v
vmx_vlapic_msr_changed(v);
- if ( opt_l1d_flush && paging_mode_hap(d) )
+ if ( opt_l1d_flush && !paging_mode_shadow(d) )
rc = vmx_add_msr(v, MSR_FLUSH_CMD, FLUSH_CMD_L1D,
VMX_MSR_GUEST_LOADONLY);
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -1707,7 +1707,7 @@ static void cf_check vmx_update_guest_cr
if ( paging_mode_shadow(v->domain) )
hw_cr0_mask |= X86_CR0_WP;
- if ( paging_mode_hap(v->domain) )
+ if ( !paging_mode_shadow(v->domain) )
{
/* Manage GUEST_CR3 when CR0.PE=0. */
uint32_t old_ctls = v->arch.hvm.vmx.exec_control;
@@ -1772,7 +1772,7 @@ static void cf_check vmx_update_guest_cr
/* Fallthrough: Changing CR0 can change some bits in real CR4. */
case 4:
v->arch.hvm.hw_cr[4] = HVM_CR4_HOST_MASK;
- if ( paging_mode_hap(v->domain) )
+ if ( !paging_mode_shadow(v->domain) )
v->arch.hvm.hw_cr[4] &= ~X86_CR4_PAE;
if ( !nestedhvm_vcpu_in_guestmode(v) )
@@ -1792,7 +1792,7 @@ static void cf_check vmx_update_guest_cr
* two subtly complicated cases.
*/
- if ( paging_mode_hap(v->domain) )
+ if ( !paging_mode_shadow(v->domain) )
{
/*
* On hardware lacking the Unrestricted Guest feature (or with
@@ -1824,7 +1824,7 @@ static void cf_check vmx_update_guest_cr
* unconditionally trapping more CR4 bits, at which point the
* performance benefit of doing this is quite dubious.
*/
- if ( paging_mode_hap(v->domain) )
+ if ( !paging_mode_shadow(v->domain) )
{
/*
* Update CR4 host mask to only trap when the guest tries to set
@@ -1860,7 +1860,7 @@ static void cf_check vmx_update_guest_cr
break;
case 3:
- if ( paging_mode_hap(v->domain) )
+ if ( !paging_mode_shadow(v->domain) )
{
if ( !hvm_paging_enabled(v) && !vmx_unrestricted_guest(v) )
v->arch.hvm.hw_cr[3] =
@@ -4190,7 +4190,7 @@ void asmlinkage vmx_vmexit_handler(struc
hvm_sanitize_regs_fields(regs, !(cs_ar_bytes & X86_SEG_AR_CS_LM_ACTIVE));
- if ( paging_mode_hap(v->domain) )
+ if ( !paging_mode_shadow(v->domain) )
{
/*
* Xen allows the guest to modify some CR4 bits directly, update cached
@@ -4977,7 +4977,7 @@ bool asmlinkage vmx_vmenter_helper(const
if ( unlikely(need_flush) )
vpid_sync_all();
- if ( paging_mode_hap(curr->domain) )
+ if ( !paging_mode_shadow(curr->domain) )
{
struct ept_data *ept = &p2m_get_hostp2m(currd)->ept;
unsigned int cpu = smp_processor_id();
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -2458,7 +2458,7 @@ int nvmx_n2_vmexit_handler(struct cpu_us
*/
if ( vector == X86_EXC_PF )
{
- if ( paging_mode_hap(v->domain) )
+ if ( !paging_mode_shadow(v->domain) )
nvcpu->nv_vmexit_pending = 1;
}
else if ( (intr_info & valid_mask) == valid_mask )On Mon Sep 21, 2026 at 3:19 PM CEST, Jan Beulich wrote:
> HVM guests cannot run without either HAP or shadow enabled. While
> paging_mode_shadow() is compile-time-constant when SHADOW_PAGING=n,
> paging_mode_hap() isn't. Hence the former is preferred to leverage DCE.
>
> In svm_update_guest_cr() combine two adjacent conditionals.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Yes, please.
Reviewed-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
A couple of nits below. Take them or leave them.
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -113,7 +113,8 @@ static void cf_check svm_update_guest_cr
> switch ( cr )
> {
> case 0:
> - if ( paging_mode_hap(v->domain) )
> + value = v->arch.hvm.guest_cr[0];
> + if ( !paging_mode_shadow(v->domain) )
> {
> uint32_t intercepts = vmcb_get_cr_intercepts(vmcb);
>
> @@ -122,9 +123,7 @@ static void cf_check svm_update_guest_cr
> monitor_ctrlreg_bitmask(VM_EVENT_X86_CR3) )
> vmcb_set_cr_intercepts(vmcb, intercepts | CR_INTERCEPT_CR3_WRITE);
> }
> -
> - value = v->arch.hvm.guest_cr[0];
> - if ( paging_mode_shadow(v->domain) )
> + else
> value |= X86_CR0_PG | X86_CR0_WP;
nit: This would be clearer with the polarity reversed. Check shadow
first and have hap later. It'd also make the diff (marginally) smaller too.
> vmcb_set_cr0(vmcb, value);
> break;
> --- a/xen/arch/x86/hvm/vmx/vmcs.c
> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
[snip]
> v->arch.hvm.vmx.exception_bitmap = HVM_TRAP_MASK
> - | (paging_mode_hap(d) ? 0 : (1U << X86_EXC_PF));
> + | (!paging_mode_shadow(d) ? 0 : (1U << X86_EXC_PF));
nit: Shouldn't | be on the prior line? It was there before, but seeing
how you're adjusting indentation might as well move that char.
In the same vein as before, it'd be a bit clearer with the polarity
inverted (shadow() ? bit : 0 )
Cheers,
Alejandro
On 21.09.2026 15:45, Alejandro Vallejo wrote:
> On Mon Sep 21, 2026 at 3:19 PM CEST, Jan Beulich wrote:
>> HVM guests cannot run without either HAP or shadow enabled. While
>> paging_mode_shadow() is compile-time-constant when SHADOW_PAGING=n,
>> paging_mode_hap() isn't. Hence the former is preferred to leverage DCE.
>>
>> In svm_update_guest_cr() combine two adjacent conditionals.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> Yes, please.
>
> Reviewed-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
Thanks.
>> --- a/xen/arch/x86/hvm/svm/svm.c
>> +++ b/xen/arch/x86/hvm/svm/svm.c
>> @@ -113,7 +113,8 @@ static void cf_check svm_update_guest_cr
>> switch ( cr )
>> {
>> case 0:
>> - if ( paging_mode_hap(v->domain) )
>> + value = v->arch.hvm.guest_cr[0];
>> + if ( !paging_mode_shadow(v->domain) )
>> {
>> uint32_t intercepts = vmcb_get_cr_intercepts(vmcb);
>>
>> @@ -122,9 +123,7 @@ static void cf_check svm_update_guest_cr
>> monitor_ctrlreg_bitmask(VM_EVENT_X86_CR3) )
>> vmcb_set_cr_intercepts(vmcb, intercepts | CR_INTERCEPT_CR3_WRITE);
>> }
>> -
>> - value = v->arch.hvm.guest_cr[0];
>> - if ( paging_mode_shadow(v->domain) )
>> + else
>> value |= X86_CR0_PG | X86_CR0_WP;
>
> nit: This would be clearer with the polarity reversed. Check shadow
> first and have hap later. It'd also make the diff (marginally) smaller too.
Not sure about diff size, but I deliberately didn't want to inverse
polarities anywhere. Switching the predicate I was hoping to be
sufficiently uncontroversial (except, as Andrew points out, it having
a slightly negative doc effect in a few places). Inverting polarities
of if/else-like constructs, otoh, can affect code gen, and we would
likely not want to favor shadow over HAP now that shadow is off by
default.
Jan
On Mon, Sep 21, 2026 at 03:45:29PM +0200, Alejandro Vallejo wrote:
> On Mon Sep 21, 2026 at 3:19 PM CEST, Jan Beulich wrote:
> > HVM guests cannot run without either HAP or shadow enabled. While
> > paging_mode_shadow() is compile-time-constant when SHADOW_PAGING=n,
> > paging_mode_hap() isn't. Hence the former is preferred to leverage DCE.
> >
> > In svm_update_guest_cr() combine two adjacent conditionals.
> >
> > Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> Yes, please.
>
> Reviewed-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
>
> A couple of nits below. Take them or leave them.
>
> > --- a/xen/arch/x86/hvm/svm/svm.c
> > +++ b/xen/arch/x86/hvm/svm/svm.c
> > @@ -113,7 +113,8 @@ static void cf_check svm_update_guest_cr
> > switch ( cr )
> > {
> > case 0:
> > - if ( paging_mode_hap(v->domain) )
> > + value = v->arch.hvm.guest_cr[0];
> > + if ( !paging_mode_shadow(v->domain) )
> > {
> > uint32_t intercepts = vmcb_get_cr_intercepts(vmcb);
> >
> > @@ -122,9 +123,7 @@ static void cf_check svm_update_guest_cr
> > monitor_ctrlreg_bitmask(VM_EVENT_X86_CR3) )
> > vmcb_set_cr_intercepts(vmcb, intercepts | CR_INTERCEPT_CR3_WRITE);
> > }
> > -
> > - value = v->arch.hvm.guest_cr[0];
> > - if ( paging_mode_shadow(v->domain) )
> > + else
> > value |= X86_CR0_PG | X86_CR0_WP;
>
> nit: This would be clearer with the polarity reversed. Check shadow
> first and have hap later. It'd also make the diff (marginally) smaller too.
>
> > vmcb_set_cr0(vmcb, value);
> > break;
> > --- a/xen/arch/x86/hvm/vmx/vmcs.c
> > +++ b/xen/arch/x86/hvm/vmx/vmcs.c
>
> [snip]
>
> > v->arch.hvm.vmx.exception_bitmap = HVM_TRAP_MASK
> > - | (paging_mode_hap(d) ? 0 : (1U << X86_EXC_PF));
> > + | (!paging_mode_shadow(d) ? 0 : (1U << X86_EXC_PF));
>
> nit: Shouldn't | be on the prior line? It was there before, but seeing
> how you're adjusting indentation might as well move that char.
>
> In the same vein as before, it'd be a bit clearer with the polarity
> inverted (shadow() ? bit : 0 )
I agree with both suggestions in principle, always better if we can
avoid negations.
Acked-by: Roger Pau Monné <roger@xenproject.org>
Thanks, Roger.
© 2016 - 2026 Red Hat, Inc.