| Message ID | 20210419144524.32608-1-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | x86/shim: Simplify compat handling in write_start_info() | expand |
On 19.04.2021 16:45, Andrew Cooper wrote: > Factor out a compat boolean to remove the lfence overhead from multiple > is_pv_32bit_domain() calls. > > For a compat guest, the upper 32 bits of rdx are zero, so there is no need to > have any conditional logic at all when mapping the start info page. Iirc the contents of the upper halves hold unspecified contents after a switch from compat to 64-bit mode. Therefore only with this part of the change dropped ... > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> Jan
On 19/04/2021 16:55, Jan Beulich wrote: > On 19.04.2021 16:45, Andrew Cooper wrote: >> Factor out a compat boolean to remove the lfence overhead from multiple >> is_pv_32bit_domain() calls. >> >> For a compat guest, the upper 32 bits of rdx are zero, so there is no need to >> have any conditional logic at all when mapping the start info page. > Iirc the contents of the upper halves hold unspecified contents after > a switch from compat to 64-bit mode. Therefore only with this part of > the change dropped ... But we're shim, so will never ever mix compat and non-compat guests. ~Andrew > >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > Reviewed-by: Jan Beulich <jbeulich@suse.com> > > Jan
On 19.04.2021 17:57, Andrew Cooper wrote: > On 19/04/2021 16:55, Jan Beulich wrote: >> On 19.04.2021 16:45, Andrew Cooper wrote: >>> Factor out a compat boolean to remove the lfence overhead from multiple >>> is_pv_32bit_domain() calls. >>> >>> For a compat guest, the upper 32 bits of rdx are zero, so there is no need to >>> have any conditional logic at all when mapping the start info page. >> Iirc the contents of the upper halves hold unspecified contents after >> a switch from compat to 64-bit mode. Therefore only with this part of >> the change dropped ... > > But we're shim, so will never ever mix compat and non-compat guests. That's not the point: A compat guest will still cause the CPU to transition back and forth between 64-bit and compat modes. It is this transitioning which leaves the upper halves of all GPRs in undefined state (even if in reality a CPU would likely need to go through extra hoops to prevent them from being zero if they were written to in compat mode). Jan
On 19/04/2021 17:00, Jan Beulich wrote: > On 19.04.2021 17:57, Andrew Cooper wrote: >> On 19/04/2021 16:55, Jan Beulich wrote: >>> On 19.04.2021 16:45, Andrew Cooper wrote: >>>> Factor out a compat boolean to remove the lfence overhead from multiple >>>> is_pv_32bit_domain() calls. >>>> >>>> For a compat guest, the upper 32 bits of rdx are zero, so there is no need to >>>> have any conditional logic at all when mapping the start info page. >>> Iirc the contents of the upper halves hold unspecified contents after >>> a switch from compat to 64-bit mode. Therefore only with this part of >>> the change dropped ... >> But we're shim, so will never ever mix compat and non-compat guests. > That's not the point: A compat guest will still cause the CPU to > transition back and forth between 64-bit and compat modes. It is > this transitioning which leaves the upper halves of all GPRs in > undefined state (even if in reality a CPU would likely need to go > through extra hoops to prevent them from being zero if they were > written to in compat mode). Hmm. That's awkward. So real behaviour (I've checked with some contacts) is that upper bits are preserved until the next write to the register, after which the upper bits are zeroed. I wonder whether I'll have any luck formally asking AMD and Intel for a tweak to this effect in the manuals. ~Andrew
diff --git a/xen/arch/x86/pv/shim.c b/xen/arch/x86/pv/shim.c index d16c0048c0..533c194eef 100644 --- a/xen/arch/x86/pv/shim.c +++ b/xen/arch/x86/pv/shim.c @@ -280,12 +280,12 @@ void __init pv_shim_setup_dom(struct domain *d, l4_pgentry_t *l4start, static void write_start_info(struct domain *d) { struct cpu_user_regs *regs = guest_cpu_user_regs(); - start_info_t *si = map_domain_page(_mfn(is_pv_32bit_domain(d) ? regs->edx - : regs->rdx)); + bool compat = is_pv_32bit_domain(d); + start_info_t *si = map_domain_page(_mfn(regs->rdx)); uint64_t param; snprintf(si->magic, sizeof(si->magic), "xen-3.0-x86_%s", - is_pv_32bit_domain(d) ? "32p" : "64"); + compat ? "32p" : "64"); si->nr_pages = domain_tot_pages(d); si->shared_info = virt_to_maddr(d->shared_info); si->flags = 0; @@ -300,7 +300,7 @@ static void write_start_info(struct domain *d) &si->console.domU.mfn) ) BUG(); - if ( is_pv_32bit_domain(d) ) + if ( compat ) xlat_start_info(si, XLAT_start_info_console_domU); unmap_domain_page(si);
Factor out a compat boolean to remove the lfence overhead from multiple is_pv_32bit_domain() calls. For a compat guest, the upper 32 bits of rdx are zero, so there is no need to have any conditional logic at all when mapping the start info page. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Roger Pau Monné <roger.pau@citrix.com> CC: Wei Liu <wl@xen.org> --- xen/arch/x86/pv/shim.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)