| Message ID | 20241029094254.38659-1-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | x86/pv: gate setting per-domain slot to XPTI being active for the domain | expand |
On 29/10/2024 9:42 am, Roger Pau Monne wrote: > It's possible to have XPTI not active for all PV domains (active for domUs, > inactive for dom0), hence don't gate setting the per-domain slot on the > presence of the per-pCPU shadow root page-table. Instead set the slot based on > whether XPTI is active for the domain. This avoid pointlessly setting the > per-domain slot if the shadow root page-table won't be used by the domain. > > Fixes: 0d3e7f0b6bf0 ('xen/x86: support per-domain flag for xpti') > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > --- > xen/arch/x86/domain.c | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c > index 78a13e6812c9..fd6bb3663027 100644 > --- a/xen/arch/x86/domain.c > +++ b/xen/arch/x86/domain.c > @@ -1902,12 +1902,11 @@ void cf_check paravirt_ctxt_switch_from(struct vcpu *v) > > void cf_check paravirt_ctxt_switch_to(struct vcpu *v) > { > - root_pgentry_t *root_pgt = this_cpu(root_pgt); > + const struct domain *d = v->domain; > > - if ( root_pgt ) > - root_pgt[root_table_offset(PERDOMAIN_VIRT_START)] = > - l4e_from_page(v->domain->arch.perdomain_l3_pg, > - __PAGE_HYPERVISOR_RW); > + if ( d->arch.pv.xpti ) > + this_cpu(root_pgt)[root_table_offset(PERDOMAIN_VIRT_START)] = > + l4e_from_page(d->arch.perdomain_l3_pg, __PAGE_HYPERVISOR_RW); I'm not sure this is a wise change. You're only optimising away a single PTE calculation/write, at the cost of retaining a mapping to the wrong domain's perdom area; whichever the last domain to schedule on this pCPU was. ~Andrew
On Tue, Oct 29, 2024 at 09:48:26AM +0000, Andrew Cooper wrote: > On 29/10/2024 9:42 am, Roger Pau Monne wrote: > > It's possible to have XPTI not active for all PV domains (active for domUs, > > inactive for dom0), hence don't gate setting the per-domain slot on the > > presence of the per-pCPU shadow root page-table. Instead set the slot based on > > whether XPTI is active for the domain. This avoid pointlessly setting the > > per-domain slot if the shadow root page-table won't be used by the domain. > > > > Fixes: 0d3e7f0b6bf0 ('xen/x86: support per-domain flag for xpti') > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > > --- > > xen/arch/x86/domain.c | 9 ++++----- > > 1 file changed, 4 insertions(+), 5 deletions(-) > > > > diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c > > index 78a13e6812c9..fd6bb3663027 100644 > > --- a/xen/arch/x86/domain.c > > +++ b/xen/arch/x86/domain.c > > @@ -1902,12 +1902,11 @@ void cf_check paravirt_ctxt_switch_from(struct vcpu *v) > > > > void cf_check paravirt_ctxt_switch_to(struct vcpu *v) > > { > > - root_pgentry_t *root_pgt = this_cpu(root_pgt); > > + const struct domain *d = v->domain; > > > > - if ( root_pgt ) > > - root_pgt[root_table_offset(PERDOMAIN_VIRT_START)] = > > - l4e_from_page(v->domain->arch.perdomain_l3_pg, > > - __PAGE_HYPERVISOR_RW); > > + if ( d->arch.pv.xpti ) > > + this_cpu(root_pgt)[root_table_offset(PERDOMAIN_VIRT_START)] = > > + l4e_from_page(d->arch.perdomain_l3_pg, __PAGE_HYPERVISOR_RW); > > I'm not sure this is a wise change. > > You're only optimising away a single PTE calculation/write, at the cost > of retaining a mapping to the wrong domain's perdom area; whichever the > last domain to schedule on this pCPU was. If XPTI is not used by the next vCPU there will be plenty of stale mappings in the shadow root_pgt, as it won't be updated on return to guest context because it's not used. All guest controlled slots will be outdated. Note that currently if the next vCPU is HVM the slot won't get updated either, as context switching to an HVM vCPU won't call paravirt_ctxt_switch_to(). I don't think updating the per-domain slot makes any sense if the root_pgt won't be used by the vCPU. Thanks, Roger.
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c index 78a13e6812c9..fd6bb3663027 100644 --- a/xen/arch/x86/domain.c +++ b/xen/arch/x86/domain.c @@ -1902,12 +1902,11 @@ void cf_check paravirt_ctxt_switch_from(struct vcpu *v) void cf_check paravirt_ctxt_switch_to(struct vcpu *v) { - root_pgentry_t *root_pgt = this_cpu(root_pgt); + const struct domain *d = v->domain; - if ( root_pgt ) - root_pgt[root_table_offset(PERDOMAIN_VIRT_START)] = - l4e_from_page(v->domain->arch.perdomain_l3_pg, - __PAGE_HYPERVISOR_RW); + if ( d->arch.pv.xpti ) + this_cpu(root_pgt)[root_table_offset(PERDOMAIN_VIRT_START)] = + l4e_from_page(d->arch.perdomain_l3_pg, __PAGE_HYPERVISOR_RW); if ( unlikely(v->arch.dr7 & DR7_ACTIVE_MASK) ) activate_debugregs(v);
It's possible to have XPTI not active for all PV domains (active for domUs, inactive for dom0), hence don't gate setting the per-domain slot on the presence of the per-pCPU shadow root page-table. Instead set the slot based on whether XPTI is active for the domain. This avoid pointlessly setting the per-domain slot if the shadow root page-table won't be used by the domain. Fixes: 0d3e7f0b6bf0 ('xen/x86: support per-domain flag for xpti') Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- xen/arch/x86/domain.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)